From cd3cc892f47db244820138aad3f33dab3f9a3e6a Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Thu, 29 Jan 2026 15:15:34 -0600 Subject: [PATCH] Implement GameService.execute_action enhancements (GS-003) Add forced action handling, turn boundary detection, and DB persistence: - Check for pending forced actions before allowing regular actions - Only specified player can act during forced action (except resign) - Only specified action type allowed during forced action - Detect turn boundaries (turn number OR current player change) - Persist to Postgres at turn boundaries for durability - Include pending_forced_action in GameActionResult for client New exceptions: ForcedActionRequiredError Tests: 11 new tests covering forced actions, turn boundaries, and pending action reporting. Total 47 tests for GameService. Co-Authored-By: Claude Opus 4.5 --- backend/app/core/config.py | 3 +- backend/app/core/turn_manager.py | 2 + backend/app/services/game_service.py | 92 ++++- .../project_plans/PHASE_4_GAME_SERVICE.json | 6 +- .../tests/unit/services/test_game_service.py | 363 +++++++++++++++++- 5 files changed, 454 insertions(+), 12 deletions(-) diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 70796b8..7b60920 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -100,8 +100,7 @@ class PrizeConfig(BaseModel): score points instead of taking prize cards. This simplifies the game while maintaining the knockout scoring mechanic. - Knockout points are determined by the Pokemon's variant (EX, V, GX, etc.), - not by its evolution stage. A Basic EX is worth the same as a Stage 2 EX. + Knockout points are determined by the Pokemon's variant (EX, V, GX, etc.). A Basic EX is worth the same as a Stage 2 EX. Attributes: count: Number of points needed to win (or prize cards if using classic rules). diff --git a/backend/app/core/turn_manager.py b/backend/app/core/turn_manager.py index 612bbfd..95f2ceb 100644 --- a/backend/app/core/turn_manager.py +++ b/backend/app/core/turn_manager.py @@ -401,6 +401,8 @@ class TurnManager: knockouts.append(active.instance_id) messages.append(f"{card_def.name} knocked out by status damage!") + # TODO: Must check for opponent's active status damage (e.g. poison, burn) which could cause rare double-KO; if both players meet win condition between turns, game ends in tie + # Process knockouts BEFORE checking win conditions # The opponent (who will be the next player) scores points for status KOs win_result = None diff --git a/backend/app/services/game_service.py b/backend/app/services/game_service.py index 03515da..77e8b5e 100644 --- a/backend/app/services/game_service.py +++ b/backend/app/services/game_service.py @@ -134,11 +134,48 @@ class GameCreationError(GameServiceError): super().__init__(f"Failed to create game: {reason}") +class ForcedActionRequiredError(GameServiceError): + """Raised when a forced action is required but a different action was attempted.""" + + def __init__( + self, + game_id: str, + player_id: str, + required_action_type: str, + attempted_action_type: str, + ) -> None: + self.game_id = game_id + self.player_id = player_id + self.required_action_type = required_action_type + self.attempted_action_type = attempted_action_type + super().__init__( + f"Forced action required: {required_action_type}, " + f"but {attempted_action_type} was attempted" + ) + + # ============================================================================= # Result Types # ============================================================================= +@dataclass +class PendingForcedAction: + """Information about a pending forced action for the client. + + Attributes: + player_id: The player who must take the action. + action_type: The type of action required. + reason: Human-readable explanation. + params: Additional parameters for the action. + """ + + player_id: str + action_type: str + reason: str + params: dict[str, Any] = field(default_factory=dict) + + @dataclass class GameActionResult: """Result of executing a game action. @@ -152,6 +189,9 @@ class GameActionResult: game_over: Whether the game ended as a result. winner_id: Winner's player ID if game ended with a winner. end_reason: Reason the game ended, if applicable. + turn_changed: Whether the turn changed as a result of this action. + current_player_id: The current player after action execution. + pending_forced_action: If set, the next action must be this forced action. """ success: bool @@ -162,6 +202,9 @@ class GameActionResult: game_over: bool = False winner_id: str | None = None end_reason: GameEndReason | None = None + turn_changed: bool = False + current_player_id: str | None = None + pending_forced_action: PendingForcedAction | None = None @dataclass @@ -455,19 +498,28 @@ class GameService: persists the updated state. The GameEngine is created using the rules stored in the game state. + Handles forced actions: if there's a pending forced action (e.g., + select new active after KO), only that action type from the + specified player is allowed. + + Persists to database at turn boundaries for durability. + Args: game_id: The game ID. player_id: The acting player's ID. action: The action to execute. Returns: - GameActionResult with success status and state changes. + GameActionResult with success status, state changes, and + any pending forced actions. Raises: GameNotFoundError: If game doesn't exist. PlayerNotInGameError: If player is not in the game. GameAlreadyEndedError: If game has already ended. NotPlayerTurnError: If it's not the player's turn. + ForcedActionRequiredError: If a forced action is required but + a different action was attempted. InvalidActionError: If the action is invalid. """ # Load game state @@ -481,10 +533,23 @@ class GameService: if state.winner_id is not None or state.end_reason is not None: raise GameAlreadyEndedError(game_id) - # Check it's player's turn (unless resignation, which can happen anytime) - if not isinstance(action, ResignAction) and state.current_player_id != player_id: + # Check for forced actions (except resignation, which is always allowed) + forced = state.get_current_forced_action() + if forced is not None and not isinstance(action, ResignAction): + # Only the specified player can act during a forced action + if player_id != forced.player_id: + raise NotPlayerTurnError(game_id, player_id, forced.player_id) + # Only the specified action type is allowed + if action.type != forced.action_type: + raise ForcedActionRequiredError(game_id, player_id, forced.action_type, action.type) + elif not isinstance(action, ResignAction) and state.current_player_id != player_id: + # Normal turn check (no forced action pending) raise NotPlayerTurnError(game_id, player_id, state.current_player_id) + # Track turn state before action for boundary detection + turn_before = state.turn_number + player_before = state.current_player_id + # Create engine with this game's rules via factory engine = self._engine_factory(state) @@ -494,11 +559,16 @@ class GameService: if not result.success: raise InvalidActionError(game_id, player_id, result.message) + # Detect turn change + turn_changed = state.turn_number != turn_before or state.current_player_id != player_before + # Save state to cache (fast path) await self._state_manager.save_to_cache(state) - # Check if turn ended - persist to DB at turn boundaries - # TODO: Implement turn boundary detection for DB persistence + # Persist to DB at turn boundaries for durability + if turn_changed: + await self._state_manager.persist_to_db(state) + logger.debug(f"Turn boundary: persisted game {game_id} to DB") # Build response action_result = GameActionResult( @@ -509,8 +579,20 @@ class GameService: state_changes={ "changes": result.state_changes, }, + turn_changed=turn_changed, + current_player_id=state.current_player_id, ) + # Include pending forced action if any + next_forced = state.get_current_forced_action() + if next_forced is not None: + action_result.pending_forced_action = PendingForcedAction( + player_id=next_forced.player_id, + action_type=next_forced.action_type, + reason=next_forced.reason, + params=next_forced.params, + ) + # Check for game over if result.win_result is not None: action_result.game_over = True diff --git a/backend/project_plans/PHASE_4_GAME_SERVICE.json b/backend/project_plans/PHASE_4_GAME_SERVICE.json index a2f2f33..6b7b586 100644 --- a/backend/project_plans/PHASE_4_GAME_SERVICE.json +++ b/backend/project_plans/PHASE_4_GAME_SERVICE.json @@ -9,7 +9,7 @@ "description": "Real-time gameplay infrastructure - WebSocket communication, game lifecycle management, reconnection handling, and turn timeout system", "totalEstimatedHours": 45, "totalTasks": 18, - "completedTasks": 7, + "completedTasks": 8, "status": "in_progress", "masterPlan": "../PROJECT_PLAN_MASTER.json" }, @@ -249,8 +249,8 @@ "description": "Validate and execute player actions, persist state, return results", "category": "services", "priority": 7, - "completed": false, - "tested": false, + "completed": true, + "tested": true, "dependencies": ["GS-002"], "files": [ {"path": "app/services/game_service.py", "status": "modify"} diff --git a/backend/tests/unit/services/test_game_service.py b/backend/tests/unit/services/test_game_service.py index 1cc891d..e68ee6e 100644 --- a/backend/tests/unit/services/test_game_service.py +++ b/backend/tests/unit/services/test_game_service.py @@ -18,10 +18,11 @@ import pytest from app.core.engine import ActionResult from app.core.enums import GameEndReason, TurnPhase -from app.core.models.actions import AttackAction, PassAction, ResignAction -from app.core.models.game_state import GameState, PlayerState +from app.core.models.actions import AttackAction, PassAction, ResignAction, SelectActiveAction +from app.core.models.game_state import ForcedAction, GameState, PlayerState from app.core.win_conditions import WinResult from app.services.game_service import ( + ForcedActionRequiredError, GameAlreadyEndedError, GameCreationError, GameNotFoundError, @@ -534,6 +535,352 @@ class TestExecuteAction: mock_state_manager.persist_to_db.assert_called_once() +class TestForcedActions: + """Tests for forced action enforcement in execute_action. + + When a forced action is pending (e.g., select new active after KO), + only the specified player can act and only with the specified action type. + """ + + @pytest.fixture + def game_state_with_forced_action(self) -> GameState: + """Create a game state with a pending forced action. + + The forced action requires player-1 to select a new active Pokemon, + simulating the situation after their active was knocked out. + """ + player1 = PlayerState(player_id="player-1") + player2 = PlayerState(player_id="player-2") + + state = GameState( + game_id="game-123", + players={"player-1": player1, "player-2": player2}, + current_player_id="player-2", # It's player-2's turn + turn_number=3, + phase=TurnPhase.MAIN, + ) + + # Add forced action for player-1 (select new active after KO) + state.forced_actions = [ + ForcedAction( + player_id="player-1", + action_type="select_active", + reason="Your active Pokemon was knocked out. Select a new active.", + params={"available_bench_ids": ["bench-pokemon-1", "bench-pokemon-2"]}, + ) + ] + + return state + + @pytest.mark.asyncio + async def test_forced_action_wrong_player_raises_error( + self, + game_service: GameService, + mock_state_manager: AsyncMock, + game_state_with_forced_action: GameState, + ) -> None: + """Test that wrong player attempting action during forced action raises error. + + When player-1 has a forced action pending, player-2 cannot act + (even though it's player-2's normal turn) except for resignation. + """ + mock_state_manager.load_state.return_value = game_state_with_forced_action + + # player-2 tries to pass, but player-1 has forced action pending + with pytest.raises(NotPlayerTurnError) as exc_info: + await game_service.execute_action("game-123", "player-2", PassAction()) + + # Error should indicate player-1 must act (forced action player) + assert exc_info.value.player_id == "player-2" + assert exc_info.value.current_player_id == "player-1" + + @pytest.mark.asyncio + async def test_forced_action_wrong_action_type_raises_error( + self, + game_service: GameService, + mock_state_manager: AsyncMock, + game_state_with_forced_action: GameState, + ) -> None: + """Test that wrong action type during forced action raises error. + + When a forced action requires 'select_active', attempting any other + action type (like 'pass') should raise ForcedActionRequiredError. + """ + mock_state_manager.load_state.return_value = game_state_with_forced_action + + # player-1 tries to pass instead of selecting active + with pytest.raises(ForcedActionRequiredError) as exc_info: + await game_service.execute_action("game-123", "player-1", PassAction()) + + assert exc_info.value.required_action_type == "select_active" + assert exc_info.value.attempted_action_type == "pass" + + @pytest.mark.asyncio + async def test_forced_action_correct_action_succeeds( + self, + game_service: GameService, + mock_state_manager: AsyncMock, + mock_engine: MagicMock, + game_state_with_forced_action: GameState, + ) -> None: + """Test that correct action during forced action succeeds. + + When player-1 submits the required select_active action, + it should be executed successfully. + """ + mock_state_manager.load_state.return_value = game_state_with_forced_action + mock_engine.execute_action.return_value = ActionResult( + success=True, + message="New active Pokemon selected", + ) + + action = SelectActiveAction(pokemon_id="bench-pokemon-1") + result = await game_service.execute_action("game-123", "player-1", action) + + assert result.success is True + assert result.action_type == "select_active" + + @pytest.mark.asyncio + async def test_resign_allowed_during_forced_action( + self, + game_service: GameService, + mock_state_manager: AsyncMock, + mock_engine: MagicMock, + game_state_with_forced_action: GameState, + ) -> None: + """Test that resignation is always allowed, even during forced action. + + A player should always be able to resign, regardless of forced + action state. This is a special exception to the forced action rule. + """ + mock_state_manager.load_state.return_value = game_state_with_forced_action + mock_engine.execute_action.return_value = ActionResult( + success=True, + message="Player resigned", + win_result=WinResult( + winner_id="player-2", + loser_id="player-1", + end_reason=GameEndReason.RESIGNATION, + reason="Player resigned", + ), + ) + + # player-1 resigns instead of selecting active + result = await game_service.execute_action("game-123", "player-1", ResignAction()) + + assert result.success is True + assert result.game_over is True + + @pytest.mark.asyncio + async def test_opponent_resign_allowed_during_forced_action( + self, + game_service: GameService, + mock_state_manager: AsyncMock, + mock_engine: MagicMock, + game_state_with_forced_action: GameState, + ) -> None: + """Test that opponent can resign while other player has forced action. + + When player-1 has a forced action pending (select active), player-2 + should still be able to resign. Resignation is always allowed for + any player at any time. + """ + mock_state_manager.load_state.return_value = game_state_with_forced_action + mock_engine.execute_action.return_value = ActionResult( + success=True, + message="Player resigned", + win_result=WinResult( + winner_id="player-1", + loser_id="player-2", + end_reason=GameEndReason.RESIGNATION, + reason="Player resigned", + ), + ) + + # player-2 resigns while player-1 has forced action pending + result = await game_service.execute_action("game-123", "player-2", ResignAction()) + + assert result.success is True + assert result.game_over is True + assert result.winner_id == "player-1" + + +class TestTurnBoundaryPersistence: + """Tests for turn boundary detection and database persistence. + + The game state should be persisted to the database when the turn + changes (either turn number or current player changes). + """ + + @pytest.mark.asyncio + async def test_turn_change_triggers_db_persistence( + self, + game_service: GameService, + mock_state_manager: AsyncMock, + mock_engine: MagicMock, + sample_game_state: GameState, + ) -> None: + """Test that turn change triggers database persistence. + + When an action causes the turn to change (e.g., pass action + ends turn), the state should be persisted to the database + for durability. + """ + mock_state_manager.load_state.return_value = sample_game_state + + def mock_execute(state, player_id, action): + # Simulate turn change: pass action ends turn + state.current_player_id = "player-2" + state.turn_number = 2 + return ActionResult(success=True, message="Turn ended") + + mock_engine.execute_action = AsyncMock(side_effect=mock_execute) + + result = await game_service.execute_action("game-123", "player-1", PassAction()) + + assert result.success is True + assert result.turn_changed is True + assert result.current_player_id == "player-2" + + # DB persistence should have been called due to turn change + mock_state_manager.persist_to_db.assert_called_once() + + @pytest.mark.asyncio + async def test_no_turn_change_skips_db_persistence( + self, + game_service: GameService, + mock_state_manager: AsyncMock, + mock_engine: MagicMock, + sample_game_state: GameState, + ) -> None: + """Test that actions without turn change don't persist to DB. + + For performance, we only persist to DB at turn boundaries. + Actions within the same turn only update the cache. + """ + mock_state_manager.load_state.return_value = sample_game_state + mock_engine.execute_action.return_value = ActionResult( + success=True, + message="Attack executed", + state_changes=[{"type": "damage", "amount": 30}], + ) + + result = await game_service.execute_action( + "game-123", "player-1", AttackAction(attack_index=0) + ) + + assert result.success is True + assert result.turn_changed is False + + # Cache should be updated, but DB should NOT be persisted + mock_state_manager.save_to_cache.assert_called_once() + mock_state_manager.persist_to_db.assert_not_called() + + @pytest.mark.asyncio + async def test_current_player_change_is_turn_change( + self, + game_service: GameService, + mock_state_manager: AsyncMock, + mock_engine: MagicMock, + sample_game_state: GameState, + ) -> None: + """Test that current player change counts as turn change. + + Turn boundaries are detected by either turn number change or + current player change (e.g., forced action completion might + return control to another player). + """ + mock_state_manager.load_state.return_value = sample_game_state + + def mock_execute(state, player_id, action): + # Only current player changes, turn number stays same + state.current_player_id = "player-2" + return ActionResult(success=True, message="Player changed") + + mock_engine.execute_action = AsyncMock(side_effect=mock_execute) + + result = await game_service.execute_action("game-123", "player-1", PassAction()) + + assert result.turn_changed is True + mock_state_manager.persist_to_db.assert_called_once() + + +class TestPendingForcedActionInResult: + """Tests for pending forced action inclusion in GameActionResult. + + After executing an action, if there's a new forced action pending, + it should be included in the result so the client knows what's next. + """ + + @pytest.mark.asyncio + async def test_pending_forced_action_included_in_result( + self, + game_service: GameService, + mock_state_manager: AsyncMock, + mock_engine: MagicMock, + sample_game_state: GameState, + ) -> None: + """Test that pending forced action is included in result. + + When an action results in a forced action being queued (e.g., + knockout triggers select_active), the result should include + the pending forced action details for the client. + """ + mock_state_manager.load_state.return_value = sample_game_state + + def mock_execute(state, player_id, action): + # Action execution adds a forced action (simulating KO) + state.forced_actions = [ + ForcedAction( + player_id="player-2", + action_type="select_active", + reason="Your active Pokemon was knocked out.", + params={"available_bench_ids": ["bench-1", "bench-2"]}, + ) + ] + return ActionResult( + success=True, + message="Attack knocked out opponent's Pokemon", + state_changes=[{"type": "knockout"}], + ) + + mock_engine.execute_action = AsyncMock(side_effect=mock_execute) + + result = await game_service.execute_action( + "game-123", "player-1", AttackAction(attack_index=0) + ) + + assert result.success is True + assert result.pending_forced_action is not None + assert result.pending_forced_action.player_id == "player-2" + assert result.pending_forced_action.action_type == "select_active" + assert result.pending_forced_action.reason == "Your active Pokemon was knocked out." + assert "bench-1" in result.pending_forced_action.params["available_bench_ids"] + + @pytest.mark.asyncio + async def test_no_pending_forced_action_when_queue_empty( + self, + game_service: GameService, + mock_state_manager: AsyncMock, + mock_engine: MagicMock, + sample_game_state: GameState, + ) -> None: + """Test that pending_forced_action is None when no forced action. + + Normal actions that don't trigger forced actions should return + None for the pending_forced_action field. + """ + mock_state_manager.load_state.return_value = sample_game_state + mock_engine.execute_action.return_value = ActionResult( + success=True, + message="Action executed normally", + ) + + result = await game_service.execute_action("game-123", "player-1", PassAction()) + + assert result.pending_forced_action is None + + class TestResignGame: """Tests for the resign_game convenience method.""" @@ -990,3 +1337,15 @@ class TestExceptionMessages: error = GameAlreadyEndedError("game-123") assert "game-123" in str(error) assert "ended" in str(error).lower() + + def test_forced_action_required_error_message(self) -> None: + """Test ForcedActionRequiredError has descriptive message. + + The error should clearly indicate what action was required + and what action was incorrectly attempted. + """ + error = ForcedActionRequiredError("game-123", "player-1", "select_active", "pass") + assert "select_active" in str(error) + assert "pass" in str(error) + assert error.required_action_type == "select_active" + assert error.attempted_action_type == "pass"