Refactor the engine to make machine learning work easier (#5)

Author: pronovic

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "apologies"
-version = "0.1.10"
+version = "0.1.11"
 description = "Python code to play a game similar to Sorry"
 authors = ["Kenneth J. Pronovici <pronovic@ieee.org>"]
 license = "Apache-2.0"

src/apologies/engine.py

@@ -10,7 +10,7 @@
 
 import attr
 
-from .game import Game, GameMode, PlayerColor, PlayerView
+from .game import Game, GameMode, Player, PlayerColor, PlayerView
 from .rules import Move, Rules
 from .source import CharacterInputSource
 from .util import CircularQueue
@@ -54,6 +54,17 @@ class Engine:
     """
     Game engine that coordinates character actions in a game.
 
+    Normally, playing a game via an engine is as simple as::
+
+        engine.start_game()
+        while not engine.completed:
+          state = engine.play_next()
+
+    This plays a turn for each player, one after another, until the
+    game is complete.  Other, more fine-grained methods exist if you
+    need to structure game play differently for your purposes (for
+    instance, to train a machine learning model).
+
     Attributes:
         mode(GameMode): The game mode
         characters(List[Character]): Characters playing the game
@@ -102,6 +113,11 @@ def state(self) -> str:
         else:
             return "Game waiting to start"
 
+    @property
+    def game(self) -> Game:
+        """A reference to the underlying game."""
+        return self._game
+
     @property
     def started(self) -> bool:
         """Whether the game is started."""
@@ -112,6 +128,11 @@ def completed(self) -> bool:
         """Whether the game is completed."""
         return self._game.completed
 
+    def reset(self) -> Game:
+        """Reset game state."""
+        self._game = self._default_game()
+        return self._game
+
     def start_game(self) -> Game:
         """
         Start the game, returning game state.
@@ -131,57 +152,66 @@ def play_next(self) -> Game:
         """
         if self.completed:
             raise ValueError("Game is complete")
+
         saved = self._game.copy()
         try:
             color = self._queue.next()
-            if self.mode == GameMode.ADULT:
-                self._play_next_adult(color)
-            else:
-                self._play_next_standard(color)
+            character = self._map[color]
+
+            done = False
+            while not done:
+                view = self._game.create_player_view(color)
+                move = self.choose_next_move(character, view)
+                done = self.execute_move(color, move)
+
             return self._game
         except Exception as e:
             self._game = saved  # put back original so a failed call is idempotent
             raise e
 
-    def _play_next_standard(self, color: PlayerColor) -> None:
-        """Play the next turn under the rules for standard mode."""
-        card = self._game.deck.draw()
-        player = self._game.players[color]
-        character = self._map[color]
-        view = self._game.create_player_view(color)
-        legal_moves = self._rules.construct_legal_moves(view, card=card)
+    def construct_legal_moves(self, view: PlayerView) -> List[Move]:
+        """Construct the legal moves based on a player view."""
+        return self._rules.construct_legal_moves(view, card=None if self.mode == GameMode.ADULT else self._game.deck.draw())
+
+    def choose_next_move(self, character: Character, view: PlayerView) -> Move:
+        """Choose the next move for a character based on a player view."""
+        legal_moves = self.construct_legal_moves(view)
         move = character.choose_move(self.mode, view, legal_moves[:], Rules.evaluate_move)
         if move not in legal_moves:  # an illegal move is ignored and we choose randomly for the character
-            self._game.track("Illegal move: a random legal move will be chosen", player)
+            self._game.track("Illegal move: a random legal move will be chosen", view.player)
             move = random.choice(legal_moves)
-        if not move.actions:  # a move with no actions is a forfeit
+        return move
+
+    def execute_move(self, color: PlayerColor, move: Move) -> bool:
+        """Execute a move for a player, returning True if the player's turn is done."""
+        player = self._game.players[color]
+        if self.mode == GameMode.ADULT:
+            return self._execute_move_adult(player, move)
+        else:
+            return self._execute_move_standard(player, move)
+
+    def _execute_move_standard(self, player: Player, move: Move) -> bool:
+        """Play the next turn under the rules for standard mode, returning True if the player's turn is done."""
+        if not move.actions:
             self._game.deck.discard(move.card)
             self._game.track("Turn is forfeit; discarded card %s" % move.card.cardtype.value, player)
+            return True  # player's turn is done if they forfeit
         else:
             self._rules.execute_move(self._game, player, move)  # tracks history, potentially completes game
             self._game.deck.discard(move.card)
-            if not self.completed and self._rules.draw_again(move.card):
-                self._play_next_standard(color)  # recursive call for next move
+            return self.completed or not self._rules.draw_again(move.card)  # player's turn is done unless they can draw again
 
-    def _play_next_adult(self, color: PlayerColor) -> None:
-        """Play the next move under the rules for adult mode."""
-        player = self._game.players[color]
-        character = self._map[color]
-        view = self._game.create_player_view(color)
-        legal_moves = self._rules.construct_legal_moves(view, card=None)
-        move = character.choose_move(self.mode, view, legal_moves[:], Rules.evaluate_move)
-        if move not in legal_moves:  # an illegal move is ignored and we choose randomly for the character
-            self._game.track("Illegal move: a random legal move will be chosen", player)
-            move = random.choice(legal_moves)
-        if not move.actions:  # a move with no actions is a forfeit
+    def _execute_move_adult(self, player: Player, move: Move) -> bool:
+        """Play the next move under the rules for adult mode, returning True if the player's turn is done."""
+        if not move.actions:
             player.hand.remove(move.card)
             self._game.deck.discard(move.card)
             player.hand.append(self._game.deck.draw())
             self._game.track("Turn is forfeit; discarded card %s" % move.card.cardtype.value, player)
+            return True  # player's turn is done if they forfeit
         else:
             self._rules.execute_move(self._game, player, move)  # tracks history, potentially completes game
             player.hand.remove(move.card)
             self._game.deck.discard(move.card)
             player.hand.append(self._game.deck.draw())
-            if not self.completed and self._rules.draw_again(move.card):
-                self._play_next_adult(color)  # recursive call for next move
+            return self.completed or not self._rules.draw_again(move.card)  # player's turn is done unless they can draw again

src/apologies/rules.py

@@ -5,6 +5,7 @@
 Implements rules related to game play.
 """
 
+import uuid
 from enum import Enum
 from typing import List, Optional
 
@@ -69,11 +70,17 @@ class Move:
         card(Card): The card that is being played by this move
         actions(List[Action]): List of actions to execute
         side_effects(List[Action]): List of side effects that occurred as a result of the actions
+        id(str): Identifier for this move, which must be unique among all legal moves this move is grouped with
     """
 
     card = attr.ib(type=Card)
     actions = attr.ib(type=List[Action])
     side_effects = attr.ib(type=List[Action])
+    id = attr.ib(type=str)
+
+    @id.default
+    def _default_id(self) -> str:
+        return uuid.uuid4().hex
 
     @side_effects.default
     def _default_side_effects(self) -> List[Action]:

tests/test_engine.py

@@ -51,6 +51,7 @@ def test_constructor(self):
         assert engine.started is False
         assert engine.completed is False
         assert engine.state == "Game waiting to start"
+        assert engine.game is engine._game
         assert len(engine._game.players) == 2
         assert engine._queue.entries == [PlayerColor.RED, PlayerColor.YELLOW]
         assert engine._rules.mode == GameMode.STANDARD
@@ -76,6 +77,12 @@ def test_completed(self):
             assert engine.completed is True
             assert engine.state == "Game completed"
 
+    def test_reset(self):
+        engine = TestEngine._create_engine()
+        saved = engine._game
+        engine.reset()
+        assert engine._game is not None and engine._game is not saved and not engine._game.started
+
     def test_start_game(self):
         engine = TestEngine._create_engine()
         engine._rules.start_game = MagicMock()

tests/test_rules.py

@@ -2,13 +2,15 @@
 # vim: set ft=python ts=4 sw=4 expandtab:
 # pylint: disable=no-self-use,protected-access,too-many-locals,too-many-statements
 
-from unittest.mock import MagicMock, call
+from unittest.mock import MagicMock, call, patch
 
 import pytest
 
 from apologies.game import ADULT_HAND, DECK_SIZE, PAWNS, Card, CardType, Game, GameMode, Pawn, PlayerColor, Position
 from apologies.rules import Action, ActionType, BoardRules, Move, Rules
 
+_UUID = MagicMock(return_value=MagicMock(hex="uuid"))  # any call to get a random UUID returns a UUID with hex value "uuid"
+
 
 class TestAction:
     def test_constructor(self):
@@ -21,12 +23,22 @@ def test_constructor(self):
 
 
 class TestMove:
-    def test_constructor(self):
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
+    def test_constructor_uuid(self):
         card = Card(3, CardType.CARD_12)
         actions = [Action(ActionType.MOVE_TO_START, pawn=Pawn(PlayerColor.BLUE, 1, "whatever"))]
         move = Move(card, actions)
         assert move.card is card
         assert move.actions == actions
+        assert move.id == "uuid"
+
+    def test_constructor_explicit(self):
+        card = Card(3, CardType.CARD_12)
+        actions = [Action(ActionType.MOVE_TO_START, pawn=Pawn(PlayerColor.BLUE, 1, "whatever"))]
+        move = Move(card, actions, id="whatever")
+        assert move.card is card
+        assert move.actions == actions
+        assert move.id == "whatever"
 
 
 class TestRules:
@@ -84,6 +96,7 @@ def test_start_game_adult(self):
         assert game.players[PlayerColor.BLUE].pawns[0].position.square == 19
         assert len(game.players[PlayerColor.BLUE].hand) == ADULT_HAND
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_no_moves_with_card(self):
         card = MagicMock()
 
@@ -114,6 +127,7 @@ def test_construct_legal_moves_no_moves_with_card(self):
             [call(PlayerColor.RED, card, pawn1, all_pawns), call(PlayerColor.RED, card, pawn2, all_pawns)]
         )
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_no_moves_no_card(self):
         card = None
 
@@ -151,6 +165,7 @@ def test_construct_legal_moves_no_moves_no_card(self):
             ]
         )
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_with_moves_with_card(self):
         card = MagicMock()
 
@@ -187,6 +202,7 @@ def test_construct_legal_moves_with_moves_with_card(self):
             [call(PlayerColor.RED, card, pawn1, all_pawns), call(PlayerColor.RED, card, pawn2, all_pawns)]
         )
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_with_moves_no_card(self):
         card = None
 
@@ -498,6 +514,7 @@ def _legal_moves(color, game, index, cardtype):
 
 
 class TestLegalMoves:
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_card_1(self):
         # No legal moves if no pawn in start, on the board, or in safe
         game = _setup_game()
@@ -544,6 +561,7 @@ def test_construct_legal_moves_card_1(self):
         card, pawn, view, moves = _legal_moves(RED, game, 0, "1")
         assert moves == [Move(card, actions=[_square(pawn, 7)], side_effects=[_bump(view, GREEN, 1)])]
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_card_2(self):
         # No legal moves if no pawn in start, on the board, or in safe
         game = _setup_game()
@@ -590,6 +608,7 @@ def test_construct_legal_moves_card_2(self):
         card, pawn, view, moves = _legal_moves(RED, game, 0, "2")
         assert moves == [Move(card, actions=[_square(pawn, 8)], side_effects=[_bump(view, GREEN, 1)])]
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_card_3(self):
         # No legal moves if no pawn on the board, or in safe
         game = _setup_game()
@@ -617,6 +636,7 @@ def test_construct_legal_moves_card_3(self):
         card, pawn, view, moves = _legal_moves(RED, game, 0, "3")
         assert moves == [Move(card, actions=[_square(pawn, 9)], side_effects=[_bump(view, GREEN, 1)])]
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_card_4(self):
         # No legal moves if no pawn on the board, or in safe
         game = _setup_game()
@@ -644,6 +664,7 @@ def test_construct_legal_moves_card_4(self):
         card, pawn, view, moves = _legal_moves(RED, game, 0, "4")
         assert moves == [Move(card, actions=[_square(pawn, 2)], side_effects=[_bump(view, GREEN, 1)])]
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_card_5(self):
         # No legal moves if no pawn on the board, or in safe
         game = _setup_game()
@@ -671,6 +692,7 @@ def test_construct_legal_moves_card_5(self):
         card, pawn, view, moves = _legal_moves(RED, game, 0, "5")
         assert moves == [Move(card, actions=[_square(pawn, 11)], side_effects=[_bump(view, GREEN, 1)])]
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_card_7(self):
         # No legal moves if no pawn on the board, or in safe
         game = _setup_game()
@@ -742,6 +764,7 @@ def test_construct_legal_moves_card_7(self):
             Move(card, actions=[_square(pawn, 12), _square(other2, 56)], side_effects=[]),  # split (6, 1)
         ]
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_card_8(self):
         # No legal moves if no pawn on the board, or in safe
         game = _setup_game()
@@ -769,6 +792,7 @@ def test_construct_legal_moves_card_8(self):
         card, pawn, view, moves = _legal_moves(RED, game, 0, "8")
         assert moves == [Move(card, actions=[_square(pawn, 14)], side_effects=[_bump(view, GREEN, 1)])]
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_card_10(self):
         # No legal moves if no pawn on the board, or in safe
         game = _setup_game()
@@ -821,6 +845,7 @@ def test_construct_legal_moves_card_10(self):
             Move(card, actions=[_square(pawn, 4)], side_effects=[_bump(view, GREEN, 1)]),
         ]
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_card_11(self):
         # No legal moves if no pawn on the board, or in safe
         game = _setup_game()
@@ -866,6 +891,7 @@ def test_construct_legal_moves_card_11(self):
             Move(card, actions=[_square(pawn, 26)], side_effects=[]),
         ]
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_card_12(self):
         # No legal moves if no pawn on the board, or in safe
         game = _setup_game()
@@ -893,6 +919,7 @@ def test_construct_legal_moves_card_12(self):
         card, pawn, view, moves = _legal_moves(RED, game, 0, "12")
         assert moves == [Move(card, actions=[_square(pawn, 18)], side_effects=[_bump(view, GREEN, 1)])]
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_card_apologies(self):
         # No legal moves if no pawn in start
         game = _setup_game()
@@ -914,6 +941,7 @@ def test_construct_legal_moves_card_apologies(self):
             Move(card, actions=[_square(pawn, 19), _bump(view, BLUE, 1)], side_effects=[]),
         ]
 
+    @patch("apologies.rules.uuid.uuid4", new=_UUID)
     def test_construct_legal_moves_special(self):
         # Move pawn into safe zone
         game = _setup_game()