Restructure premove to use PremoveModel

src/cli_chess/core/game/game_model_base.py

@@ -16,6 +16,7 @@
 from cli_chess.modules.board import BoardModel
 from cli_chess.modules.move_list import MoveListModel
 from cli_chess.modules.material_difference import MaterialDifferenceModel
+from cli_chess.modules.premove import PremoveModel
 from cli_chess.utils import EventManager, log
 from chess import Color, WHITE, COLOR_NAMES, InvalidMoveError, IllegalMoveError, AmbiguousMoveError
 from random import getrandbits
@@ -109,7 +110,10 @@ class PlayableGameModelBase(GameModelBase, ABC):
     def __init__(self, play_as_color: str, variant="standard", fen=""):
         self.my_color = self._get_side_to_play_as(play_as_color)
         self.game_in_progress = False
+
         super().__init__(orientation=self.my_color, variant=variant, fen=fen)
+        self.premove_model = PremoveModel(self.board_model)
+        self._assoc_models = self._assoc_models + [self.premove_model]
 
     def is_my_turn(self) -> bool:
         """Return True if it's our turn"""
@@ -125,42 +129,14 @@ def _get_side_to_play_as(color: str) -> Color:
         else:  # Get random color to play as
             return Color(getrandbits(1))
 
-    def make_premove(self, move: str):
-        """Make a premove on the board"""
-        if self.game_in_progress and not self.is_my_turn():
-            try:
-                if self.board_model.board.is_game_over():
-                    self.game_in_progress = False
-                    raise Warning("Game has already ended")
-                if self.board_model.get_premove() is not None:
-                    raise Warning("You already have a premove set")
-                move = move.strip()
-                if not move:
-                    raise Warning("No move specified")
-
-                tmp_board = self.board_model.board.copy()
-                tmp_board.turn = not tmp_board.turn
-                try:
-                    move = tmp_board.push_san(move).uci()
-
-                except Exception as e:
-                    if isinstance(e, InvalidMoveError):
-                        raise ValueError(f"Invalid premove: {move}")
-                    elif isinstance(e, IllegalMoveError):
-                        raise ValueError(f"Illegal premove: {move}")
-                    elif isinstance(e, AmbiguousMoveError):
-                        raise ValueError(f"Ambiguous premove: {move}")
-                    else:
-                        raise e
-
-                self.board_model.set_premove(move)
-            except Exception:
-                raise
-
     @abstractmethod
     def make_move(self, move: str) -> None:
         pass
 
+    @abstractmethod
+    def set_premove(self, move) -> None:
+        pass
+
     @abstractmethod
     def propose_takeback(self) -> None:
         pass

src/cli_chess/core/game/game_presenter_base.py

@@ -66,7 +66,7 @@ def exit(self) -> None:
 
 class PlayableGamePresenterBase(GamePresenterBase, ABC):
     def __init__(self, model: PlayableGameModelBase):
-        self.premove_presenter = PremovePresenter(model)
+        self.premove_presenter = PremovePresenter(model.premove_model)
         super().__init__(model)
         self.model = model
 
@@ -91,33 +91,27 @@ def user_input_received(self, inpt: str) -> None:
         """Respond to the users input. This input can either be the
            move input, or game actions (such as resign)
         """
-        inpt_lower = inpt.lower()
-        if inpt_lower == "resign" or inpt_lower == "quit" or inpt_lower == "exit":
-            self.resign()
-        elif inpt_lower == "draw" or inpt_lower == "offer draw":
-            self.offer_draw()
-        elif inpt_lower == "takeback" or inpt_lower == "back" or inpt_lower == "undo":
-            self.propose_takeback()
-        elif self.model.is_my_turn():
-            self.make_move(inpt)
-        else:
-            self.make_premove(inpt)
-
-    def make_move(self, move: str) -> None:
-        """Make the passed in move on the board"""
         try:
-            move = move.strip()
-            if move:
-                self.model.make_move(move)
+            inpt_lower = inpt.lower()
+            if inpt_lower == "resign" or inpt_lower == "quit" or inpt_lower == "exit":
+                self.resign()
+            elif inpt_lower == "draw" or inpt_lower == "offer draw":
+                self.offer_draw()
+            elif inpt_lower == "takeback" or inpt_lower == "back" or inpt_lower == "undo":
+                self.propose_takeback()
+            elif self.model.is_my_turn():
+                self.make_move(inpt)
+            else:
+                self.model.set_premove(inpt)
         except Exception as e:
             self.view.alert.show_alert(str(e))
 
-    def make_premove(self, move: str) -> None:
-        """Make a premove"""
+    def make_move(self, move: str) -> None:
+        """Make the passed in move on the board"""
         try:
             move = move.strip()
             if move:
-                self.model.make_premove(move)
+                self.model.make_move(move)
         except Exception as e:
             self.view.alert.show_alert(str(e))
 

src/cli_chess/core/game/game_view_base.py

@@ -159,7 +159,7 @@ def _get_function_bar_fragments() -> StyleAndTextTuples:
                     fragments.extend(self._draw_fb_fragments())
 
                 fragments.extend(self._resign_fb_fragments())
-                if self.presenter.premove_presenter.get_premove():
+                if self.presenter.premove_presenter.is_premove_set():
                     fragments.extend(self._clear_premove_fb_fragments())
             else:
                 fragments.extend(self._exit_fb_fragments())
@@ -192,7 +192,7 @@ def _(event):
         def _(event): # noqa
             self.presenter.exit()
 
-        @bindings.add(Keys.Escape, filter=Condition(self.presenter.premove_presenter.get_premove), eager=True)
+        @bindings.add(Keys.Escape, filter=Condition(self.presenter.premove_presenter.is_premove_set), eager=True)
         def _(event):
             self.presenter.premove_presenter.clear_premove()
 

src/cli_chess/core/game/offline_game/offline_game_model.py

@@ -49,25 +49,27 @@ def make_move(self, move: str):
 
                 if not self.is_my_turn():
                     raise Warning("Not your turn")
-                move = move.strip()
-                if not move:
-                    raise Warning("No move specified")
-                # clean premove
-                self.board_model.set_premove(None)
-                self.board_model.make_move(move)
+
+                self.board_model.make_move(move.strip())
+                self.premove_model.clear_premove()
 
             except Exception:
                 raise
         else:
             log.warning("Attempted to make a move in a game that's not in progress")
             raise Warning("Game has already ended")
 
+    def set_premove(self, move: str) -> None:
+        """Sets the premove"""
+        self.premove_model.set_premove(move)
+
     def propose_takeback(self) -> None:
         """Take back the previous move"""
         try:
             if self.board_model.board.is_game_over():
                 raise Warning("Game has already ended")
 
+            self.premove_model.clear_premove()
             self.board_model.takeback(self.my_color)
         except Exception as e:
             log.error(f"Takeback failed - {e}")

src/cli_chess/core/game/offline_game/offline_game_presenter.py

@@ -49,15 +49,16 @@ def update(self, **kwargs) -> None:
         super().update(**kwargs)
         if "offlineGameOver" in kwargs:
             self._parse_and_present_game_over()
+            self.premove_presenter.clear_premove()
 
     def make_move(self, move: str) -> None:
         """Make the users move on the board"""
         try:
-            if self.model.is_my_turn():
+            if self.model.is_my_turn() and move:
                 self.model.make_move(move)
                 self.make_engine_move()
             else:
-                self.model.make_premove(move)
+                self.premove_presenter.set_premove(move)
         except Exception as e:
             self.view.alert.show_alert(str(e))
 
@@ -75,12 +76,12 @@ def make_engine_move(self) -> None:
                 move = engine_move.move.uci()
                 log.debug(f"Received move ({move}) from engine.")
                 self.board_presenter.make_move(move)
-                # After engine move, if premove is set, make it
-                if self.model.board_model.get_premove():
-                    self.make_move(self.model.board_model.get_premove())
+
+                # After the engine moves, make the premove if set
+                self.make_move(self.premove_presenter.pop_premove())
         except Exception as e:
-            log.error(f"Engine error {e}")
-            self.view.alert.show_alert(f"Engine error: {e}")
+            log.error(e)
+            self.view.alert.show_alert(str(e))
 
     def _parse_and_present_game_over(self) -> None:
         """Triages game over status for parsing and sending to the view for display"""

src/cli_chess/core/game/online_game/online_game_model.py

@@ -121,12 +121,17 @@ def handle_game_state_dispatcher_event(self, **kwargs) -> None:
             # and our local board are in sync (eg. takebacks, moves played on website, etc)
             self.board_model.reset(notify=False)
             self.board_model.make_moves_from_list(event.get('moves', []).split())
-            # if board_model.premove is set, make the move
-            if self.is_my_turn() and self.board_model.get_premove() is not None:
+
+            if self.is_my_turn():
+                premove = self.premove_model.pop_premove()
                 try:
-                    self.make_move(self.board_model.get_premove())
-                except Exception:
-                    self.board_model.set_premove(None)
+                    if premove:
+                        self.make_move(premove)
+                except Exception as e:
+                    if isinstance(e, ValueError):
+                        log.debug(f"The premove set was invalid in the new context, skipping: {e}")
+                    else:
+                        log.exception(e)
 
             if kwargs['gameOver']:
                 self._report_game_over(status=event.get('status'), winner=event.get('winner', ""))
@@ -147,20 +152,14 @@ def make_move(self, move: str):
         """
         if self.game_in_progress:
             try:
-                if not self.is_my_turn():
-                    raise Warning("Not your turn")
-
-                move = move.strip()
                 if not move:
                     raise Warning("No move specified")
 
                 if move == "0000":
                     raise Warning("Null moves are not supported in online games")
 
-                move = self.board_model.verify_move(move)
+                move = self.board_model.verify_move(move.strip())
                 self.game_state_dispatcher.make_move(move)
-                # clean premove
-                self.board_model.set_premove(None)
             except Exception:
                 raise
         else:
@@ -170,23 +169,21 @@ def make_move(self, move: str):
             else:
                 raise Warning("Game has already ended")
 
-    def make_premove(self, move: str):
-        """Make a premove on the board"""
-        if self.game_in_progress:
-            try:
-                if move == "0000":
-                    raise Warning("Null moves are not supported in online games")
-            except Exception:
-                raise
-
-            super().make_premove(move)
+    def set_premove(self, move: str) -> None:
+        """Sets the premove. Raises an exception on an invalid premove"""
+        if self.game_in_progress and move and not self.is_my_turn():
+            if move == "0000":
+                raise Warning("Null moves are not supported in online games")
+            self.premove_model.set_premove(move)
 
     def propose_takeback(self) -> None:
         """Notifies the game state dispatcher to propose a takeback"""
         if self.game_in_progress:
             try:
                 if len(self.board_model.get_move_stack()) < 2:
                     raise Warning("Cannot send takeback with less than two moves")
+
+                self.premove_model.clear_premove()
                 self.game_state_dispatcher.send_takeback_request()
 
                 if not self.vs_ai:

src/cli_chess/core/game/online_game/online_game_presenter.py

@@ -48,6 +48,7 @@ def update(self, **kwargs) -> None:
             self.view.alert.clear_alert()
         if 'onlineGameOver' in kwargs:
             self._parse_and_present_game_over()
+            self.premove_presenter.clear_premove()
 
     def _parse_and_present_game_over(self) -> None:
         """Triages game over status for parsing and sending to the view for display"""

src/cli_chess/modules/board/board_model.py

@@ -27,7 +27,7 @@ def __init__(self, orientation: chess.Color = chess.WHITE, variant="standard", f
         self.initial_fen = self.board.fen()
         self.orientation = chess.WHITE if variant.lower() == "racingkings" else orientation
         self.highlight_move = chess.Move.null()
-        self.premove: str = None
+        self.premove_highlight = chess.Move.null()
         self._game_over_result: Optional[chess.Outcome] = None
         self._log_init_info()
 
@@ -204,15 +204,6 @@ def get_highlight_move(self) -> chess.Move:
         """
         return self.highlight_move
 
-    def get_premove(self) -> str:
-        """Returns the premove"""
-        return self.premove
-
-    def set_premove(self, move: str = None) -> None:
-        """Sets the premove"""
-        self.premove = move
-        self._notify_board_model_updated(successfulMoveMade=True)
-
     def set_board_orientation(self, color: chess.Color, notify=True) -> None:
         """Sets the board's orientation to the color passed in.
            If notify is false, a model update notification will not be sent.
@@ -338,6 +329,23 @@ def handle_resignation(self, color_resigning: chess.Color) -> None:
         self._game_over_result = chess.Outcome("resignation", not color_resigning)  # noqa
         self._notify_board_model_updated(isGameOver=True)
 
+    def set_premove_highlight(self, move: chess.Move) -> None:
+        """Sets the move that should be highlighted on the board.
+           indicating a premove. The board model itself does not
+           manage premoves but instead should be handled by an outside
+           class and passes to the board model for updating. This move
+           should never be popped from the board as it is a future
+           (possible) move.
+        """
+        if bool(move):
+            self.premove_highlight = move
+            self._notify_board_model_updated(premoveHighlightSet=True)
+
+    def clear_premove_highlight(self):
+        """Clears the set premove highlight"""
+        self.premove_highlight = chess.Move.null()
+        self._notify_board_model_updated(premoveHighlightCleared=True)
+
     def cleanup(self) -> None:
         """Handles model cleanup tasks. This should only ever
            be run when this model is no longer needed.

src/cli_chess/modules/board/board_presenter.py

@@ -170,26 +170,21 @@ def get_square_display_color(self, square: chess.Square) -> str:
 
         show_board_highlights = self.game_config_values[game_config.Keys.SHOW_BOARD_HIGHLIGHTS]
         if show_board_highlights:
+            # TODO: Lighten last move square color if on light square
             try:
                 last_move = self.model.get_highlight_move()
                 if bool(last_move) and (square == last_move.to_square or square == last_move.from_square):
                     square_color = "last-move"
-                    # TODO: Lighten last move square color if on light square
+
+                premove_highlight = self.model.premove_highlight
+                if bool(premove_highlight) and (square == premove_highlight.from_square or square == premove_highlight.to_square):
+                    square_color = "pre-move"
             except IndexError:
                 pass
 
             if self.model.is_square_in_check(square):
                 square_color = "in-check"
 
-            # premove highlight
-            if self.model.get_premove() is not None:
-                try:
-                    move = chess.Move.from_uci(self.model.get_premove())
-                    if square == move.from_square or square == move.to_square:
-                        square_color = "pre-move"
-                except Exception:
-                    pass
-
         return square_color
 
     def handle_resignation(self, color_resigning: chess.Color) -> None:

src/cli_chess/modules/premove/__init__.py

@@ -1,2 +1,3 @@
+from .premove_model import PremoveModel
 from .premove_view import PremoveView
 from .premove_presenter import PremovePresenter