Added error handling for (?most?) invalid Player responses

diff --git a/pyrisk/base.py b/pyrisk/base.py
index 295b27b1e3c69830313d8122c2f31ba5d2d9c823..7ac8b667ef4c262cb352578012833ae226e92ddf 100644 (file)
--- a/pyrisk/base.py
+++ b/pyrisk/base.py
@@ -432,12 +432,13 @@ class Player (NameMixin, ID_CmpMixin):
         report - another notification-only method
         """
         pass
-    def select_territory(self, world, log):
+    def select_territory(self, world, log, error=None):
         """Return the selected territory's name.
         """
         free_territories = [t for t in world.territories() if t.player == None]
         return random.sample(free_territories, 1)[0].name
-    def play_cards(self, world, log, play_required=True):
+    def play_cards(self, world, log, error=None,
+                   play_required=True):
         """Decide whether or not to turn in a set of cards.
 
         Return a list of cards to turn in or None.  If play_required
@@ -445,14 +446,16 @@ class Player (NameMixin, ID_CmpMixin):
         """
         if play_required == True:
             return random.sample(list(self.hand.possible()), 1)[0]
-    def place_armies(self, world, log, remaining=1, this_round=1):
+    def place_armies(self, world, log, error=None,
+                     remaining=1, this_round=1):
         """Both during setup and before each turn.
 
         Return {territory_name: num_armies, ...}
         """
         t = random.sample(list(self.border_territories(world)), 1)[0]
         return {t.name: this_round}
-    def attack_and_fortify(self, world, log, mode='attack'):
+    def attack_and_fortify(self, world, log, error=None,
+                           mode='attack'):
         """Return list of (source, target, armies) tuples.  Place None
         in the list to end this phase.
         """
@@ -467,7 +470,8 @@ class Player (NameMixin, ID_CmpMixin):
         if len(possible_attacks) == 0:
             return [None, None] # stop attack phase, then stop fortification phase
         return random.sample(possible_attacks, 1) # + [None]
-    def support_attack(self, world, log, source, target):
+    def support_attack(self, world, log, error,
+                       source, target):
         """Follow up on a conquest by moving additional armies.
         """
         return source.armies-1
@@ -546,16 +550,31 @@ class Engine (ID_CmpMixin):
     def select_territories(self):
         for t in self.world.territories():
             t.player = None
-        for i in range(len(list(self.world.territories()))):
+        num_terrs = len(list(self.world.territories()))
+        for i in range(num_terrs-1):
             p = self.players[i % len(self.players)]
-            t_name = p.select_territory(self.world, self.log)
-            t = self.world.territory_by_name(t_name)
-            if t.player != None:
-                raise PlayerError('Cannot select %s owned by %s'
-                                  % (t, t.player))
+            error = None
+            while True:
+                try:
+                    t_name = p.select_territory(self.world, self.log, error)
+                    try:
+                        t = self.world.territory_by_name(t_name)
+                    except KeyError:
+                        raise PlayerError('Invalid territory "%s"' % t_name)
+                    if t.player != None:
+                        raise PlayerError('Cannot select %s owned by %s'
+                                          % (t, t.player))
+                    break
+                except PlayerError, error:
+                    continue
             self.log('%s selects %s' % (p, t))
             t.player = p
             t.armies = 1
+        # last player has no choice.
+        p = self.players[(num_terrs-1) % len(self.players)]
+        t = [t for t in self.world.territories() if t.player == None][0]
+        t.player = p
+        t.armies = 1
     def place_initial_armies(self):
         already_placed = [len(list(p.territories(self.world))) for p in self.players]
         s = list(set(already_placed))
@@ -571,17 +590,27 @@ class Engine (ID_CmpMixin):
                 self.player_place_armies(p, remaining, 1)
             remaining -= 1
     def player_place_armies(self, player, remaining=1, this_round=1):
-        placements = player.place_armies(self.world, self.log, remaining, this_round)
-        if sum(placements.values()) != this_round:
-            raise PlayerError('Placing more than %d armies' % this_round)
-        for ter_name,armies in placements.items():
-            t = self.world.territory_by_name(ter_name)
-            if t.player != player:
-                raise PlayerError('Placing armies in %s owned by %s'
-                                  % (t, t.player))
-            if armies < 0:
-                raise PlayerError('Placing a negative number of armies (%d) in %s'
-                                  % (armies, t))
+        error = None
+        while True:
+            try:
+                placements = player.place_armies(self.world, self.log, error,
+                                                 remaining, this_round)
+                if sum(placements.values()) != this_round:
+                    raise PlayerError('Placing more than %d armies' % this_round)
+                for ter_name,armies in placements.items():
+                    try:
+                        t = self.world.territory_by_name(ter_name)
+                    except KeyError:
+                        raise PlayerError('Invalid territory "%s"' % t_name)
+                    if t.player != player:
+                        raise PlayerError('Placing armies in %s owned by %s'
+                                          % (t, t.player))
+                    if armies < 0:
+                        raise PlayerError('Placing a negative number of armies (%d) in %s'
+                                          % (armies, t))
+                break
+            except PlayerError, error:
+                continue
         self.log('%s places %s' % (player, placements))
         for terr_name,armies in placements.items():
             t = self.world.territory_by_name(terr_name)
@@ -595,14 +624,20 @@ class Engine (ID_CmpMixin):
         self.log('%s dealt %d cards' % (player, number))
     def play_cards_and_place_armies(self, player, additional_armies=0):
         cards_required = len(player.hand) >= 5
-        cards = player.play_cards(
-            self.world, self.log, play_required=cards_required)
-        if cards_required == True and cards == None:
-            raise PlayerError('You have %d >= 5 cards in your hand, you must play'
-                              % len(player.hand))
+        error = None
+        while True:
+            try:
+                cards = player.play_cards(
+                    self.world, self.log, error, play_required=cards_required)
+                if cards_required == True and cards == None:
+                    raise PlayerError('You have %d >= 5 cards in your hand, you must play'
+                                      % len(player.hand))
+                c_prod,c_terr_prod = self.deck.production(player, cards)
+                break
+            except PlayerError, error:
+                continue
         w_prod,w_terr_prod = self.world.production(player)
         self.log('%s earned %d armies from territories' % (player, w_prod))
-        c_prod,c_terr_prod = self.deck.production(player, cards)
         if c_prod > 0:
             self.log('%s played %s, earning %d armies'
                      % (player, cards, c_prod+sum(c_terr_prod.values())))
@@ -622,32 +657,42 @@ class Engine (ID_CmpMixin):
     def attack_and_fortify(self, player):
         captures = 0
         mode = 'attack'
+        error = None
         while True:
-            actions = player.attack_and_fortify(self.world, self.log, mode)
-            for action in actions:
-                if action == None:
+            try:
+                actions = player.attack_and_fortify(self.world, self.log, error, mode)
+                for action in actions:
+                    if action == None:
+                        if mode == 'attack':
+                            mode = 'fortify'
+                            continue
+                        else:
+                            assert mode == 'fortify', mode
+                            return captures
+                    source_name,target_name,armies = action
+                    try:
+                        source = self.world.territory_by_name(source_name)
+                    except KeyError:
+                        raise PlayerError('Invalid territory "%s"' % source_name)
+                    try:
+                        target = self.world.territory_by_name(target_name)
+                    except KeyError:
+                        raise PlayerError('Invalid territory "%s"' % targer_name)
+                    if not source.borders(target):
+                        raise PlayerError('Cannot reach %s from %s to %s'
+                                          % (target, source, mode))
                     if mode == 'attack':
-                        mode = 'fortify'
-                        continue
+                        tplayer = target.player
+                        capture = self.attack(source, target, armies)
+                        if capture == True:
+                            captures += 1
+                            if len(list(tplayer.territories(self.world))) == 0:
+                                self.player_killed(tplayer, killer=player)
                     else:
                         assert mode == 'fortify', mode
-                        return captures
-                source_name,target_name,armies = action
-                source = self.world.territory_by_name(source_name)
-                target = self.world.territory_by_name(target_name)
-                if not source.borders(target):
-                    raise PlayerError('Cannot reach %s from %s to %s'
-                                      % (target, source, mode))
-                if mode == 'attack':
-                    tplayer = target.player
-                    capture = self.attack(source, target, armies)
-                    if capture == True:
-                        captures += 1
-                        if len(list(tplayer.territories(self.world))) == 0:
-                            self.player_killed(tplayer, killer=player)
-                else:
-                    assert mode == 'fortify', mode
-                    self.fortify(source, target, armies)
+                        self.fortify(source, target, armies)
+            except PlayerError, error:
+                continue
     def attack(self, source, target, armies):
         if source.player == target.player:
             raise PlayerError('%s attacking %s, but you own both.'
@@ -688,12 +733,18 @@ class Engine (ID_CmpMixin):
         target.armies += remaining_attackers
         target.player = source.player
         if source.armies > 1:
-            support = source.player.support_attack(
-                self.world, self.log, source, target)
-            if support < 0 or support >= source.armies:
-                raise PlayerError(
-                    'Cannot support from %s to %s with %d armies, only %d available'
-                    % (source, target, support, source.armies-1))
+            error = None
+            while True:
+                try:
+                    support = source.player.support_attack(
+                        self.world, self.log, error, source, target)
+                    if support < 0 or support >= source.armies:
+                        raise PlayerError(
+                            'Cannot support from %s to %s with %d armies, only %d available'
+                            % (source, target, support, source.armies-1))
+                    break
+                except PlayerError, error:
+                    continue
             source.armies -= support
             target.armies += support
     def player_killed(self, player, killer):

diff --git a/pyrisk/player/email.py b/pyrisk/player/email.py
index 988f151eca9f48303bb0fda315c316b93556cbed..67bc7f9d14ae08187b1269e06abc5f2057b5f41d 100644 (file)
--- a/pyrisk/player/email.py
+++ b/pyrisk/player/email.py
@@ -68,7 +68,7 @@ class IncomingEmailDispatcher (object):
             self._cache.append((msg_tag, msg))
             msg = self._get_msg()
             msg_tag = self._msg_tag(msg['Subject'])
-        if verbose == True:
+        if self.verbose == True:
             print >> sys.stderr, msg
         return msg
     def _msg_tag(self, subject):
@@ -300,7 +300,7 @@ class EmailPlayer (Player):
             else:
                 body.append('  %s' % c)
         self._send_mail(world, log, 'Drawing cards', '\n'.join(body))
-    def select_territory(self, world, log):
+    def __select_territory(self, world, log, error=None):
         """Return the selected territory's name.
         """
         body = [
@@ -311,11 +311,15 @@ class EmailPlayer (Player):
         for t in world.territories():
             if t.player == None:
                 body.append('  %s' % t)
+        if error != None:
+            body.insert(0, '')
+            body.insert(0, str(error))
         tag = self._send_mail(world, log, 'Select territory', '\n'.join(body))
         body = self._get_mail(tag)
         name = body.splitlines()[0].strip()
         return name
-    def play_cards(self, world, log, play_required=True):
+    def play_cards(self, world, log, error=None,
+                   play_required=True):
         """Decide whether or not to turn in a set of cards.
 
         Return a list of cards to turn in or None.  If play_required
@@ -333,13 +337,17 @@ class EmailPlayer (Player):
             'blank to pass).  Available sets are:']
         for i,h in enumerate(possibles):
             body.append('  %d: %s' % (i, h))
+        if error != None:
+            body.insert(0, '')
+            body.insert(0, str(error))
         tag = self._send_mail(world, log, subject, '\n'.join(body))
         body = self._get_mail(tag)
         text = body.splitlines()[0].strip()
         if text == '':
             return None
         return possibles[int(text)]
-    def place_armies(self, world, log, remaining=1, this_round=1):
+    def place_armies(self, world, log, error=None,
+                     remaining=1, this_round=1):
         """Both during setup and before each turn.
 
         Return {territory_name: num_armies, ...}
@@ -360,6 +368,9 @@ class EmailPlayer (Player):
             'Your current disposition is:']
         for t in self.territories(world):
             body.append('  %d : %s' % (t.armies, t))
+        if error != None:
+            body.insert(0, '')
+            body.insert(0, str(error))
         tag = self._send_mail(world, log, subject, '\n'.join(body))
         body = self._get_mail(tag)
         placements = {}
@@ -367,10 +378,13 @@ class EmailPlayer (Player):
             line = line.strip()
             if len(line) == 0:
                 break
+            if line.count(':') != 1:
+                raise PlayerError('Invalid syntax "%s"' % line)
             armies,terr_name = [x.strip() for x in line.split(':')]
             placements[terr_name] = int(armies)
         return placements
-    def attack_and_fortify(self, world, log, mode='attack'):
+    def attack_and_fortify(self, world, log, error=None,
+                           mode='attack'):
         """Return list of (source, target, armies) tuples.  Place None
         in the list to end this phase.
         """
@@ -408,6 +422,9 @@ class EmailPlayer (Player):
                 '  ',
                 'or',
                 '  Pass']
+        if error != None:
+            body.insert(0, '')
+            body.insert(0, str(error))
         tag = self._send_mail(world, log, subject, '\n'.join(body))
         body = self._get_mail(tag)
         if mode == 'fortify':
@@ -432,7 +449,8 @@ class EmailPlayer (Player):
         elif line.lower() == 'pass' \
                 or (mode == 'fortify' and len(line) == 0):
             return None
-    def support_attack(self, world, log, source, target):
+    def support_attack(self, world, log, error,
+                       source, target):
         """Follow up on a conquest by moving additional armies.
         """
         subject = 'Support conquest of %s by %s' % (target, source)
@@ -445,6 +463,9 @@ class EmailPlayer (Player):
             'Reply with first line(s) of the body of your email set',
             'to "<number_of_armies>", or leave the first line blank',
             'to pass.']
+        if error != None:
+            body.insert(0, '')
+            body.insert(0, str(error))
         tag = self._send_mail(world, log, subject, '\n'.join(body))
         body = self._get_mail(tag)
         text = body.splitlines()[0].strip()