Merge pull request #625 from creftos/master

[dice]Fix empty & invalid argument, close #624 & close #626

willie/modules/dice.py

@@ -117,7 +117,7 @@ def get_number_of_faces(self):
         return len(self.dice) + len(self.dropped)
 
 
-def _roll_dice(dice_expression):
+def _roll_dice(bot, dice_expression):
     result = re.search(
         r"""
         (?P<dice_num>\d*)
@@ -131,11 +131,17 @@ def _roll_dice(dice_expression):
     dice_num = int(result.group('dice_num') or 1)
     dice_type = int(result.group('dice_type'))
 
+    # Dice can't have zero or a negative number of sides.
+    if dice_type <= 0:
+        bot.reply("I don't have any dice with %d sides. =(" % dice_type)
+        return None # Signal there was a problem
+
     # Upper limit for dice should be at most a million. Creating a dict with
     # more than a million elements already takes a noticeable amount of time
     # on a fast computer and ~55kB of memory.
     if dice_num > 1000:
-        return None
+        bot.reply('I only have 1000 dice. =(')
+        return None # Signal there was a problem
 
     dice = DicePouch(dice_num, dice_type, 0)
 
@@ -171,13 +177,18 @@ def roll(bot, trigger):
     # Get a list of all dice expressions, evaluate them and then replace the
     # expressions in the original string with the results. Replacing is done
     # using string formatting, so %-characters must be escaped.
+    if not trigger.group(2):
+        return bot.reply("No dice to roll.")
     arg_str = trigger.group(2)
     dice_expressions = re.findall(dice_regexp, arg_str)
     arg_str = arg_str.replace("%", "%%")
     arg_str = re.sub(dice_regexp, "%s", arg_str)
-    dice = list(map(_roll_dice, dice_expressions))
+
+    f = lambda dice_expr: _roll_dice  (bot, dice_expr)
+    dice = list(map(f, dice_expressions))
+
     if None in dice:
-        bot.reply("I only have 1000 dice. =(")
+        # Stop computing roll if there was a problem rolling dice.
         return
 
     def _get_eval_str(dice):