From 554178dc6e83a8fe0b11b4a07d11787b395e610c Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Mon, 26 Jan 2026 11:38:38 -0600 Subject: [PATCH] Track stadium ownership for correct discard (Issue #12) Added stadium_owner_id field to GameState to track who played the stadium: - stadium_owner_id: str | None tracks the player who played the current stadium - When a stadium is replaced, old stadium discards to OWNER's pile (not current player) - Added stadium_owner_id to VisibleGameState for client visibility - Updated existing test and added 2 new tests for stadium ownership This fixes the bug where replacing an opponent's stadium would discard to the current player's pile instead of the opponent's. 797 tests passing. --- backend/SYSTEM_REVIEW.md | 29 +++++++--- backend/app/core/engine.py | 21 ++++---- backend/app/core/models/game_state.py | 2 + backend/app/core/visibility.py | 4 ++ backend/tests/core/test_engine.py | 77 ++++++++++++++++++++++++++- 5 files changed, 113 insertions(+), 20 deletions(-) diff --git a/backend/SYSTEM_REVIEW.md b/backend/SYSTEM_REVIEW.md index 57fc2d9..be2827a 100644 --- a/backend/SYSTEM_REVIEW.md +++ b/backend/SYSTEM_REVIEW.md @@ -3,7 +3,7 @@ **Date:** 2026-01-25 **Updated:** 2026-01-26 **Reviewed By:** Multi-agent system review -**Status:** 795 tests passing, 94% coverage +**Status:** 797 tests passing, 94% coverage --- @@ -195,12 +195,16 @@ Added complete prize card system: --- -### 12. Stadium Discard Goes to Wrong Player -**File:** `app/core/engine.py:608-624` +### 12. ~~Stadium Discard Goes to Wrong Player~~ FIXED +**File:** `app/core/engine.py:630-644` +**Status:** FIXED in current session -When a new stadium replaces an existing one, the old stadium is discarded to the current player's discard pile instead of its owner's. +Added `stadium_owner_id: str | None` to GameState to track who played the current stadium: +- When a stadium is played, both `stadium_in_play` and `stadium_owner_id` are set +- When a stadium is replaced, the old stadium is discarded to its owner's discard pile +- Added to VisibleGameState for client visibility -**Fix:** Track stadium ownership and discard to correct player. +Added 2 new tests for stadium ownership behavior. --- @@ -307,7 +311,7 @@ Also added `GameEndReason.TURN_LIMIT` enum value to distinguish from `TIMEOUT` ( ### Phase 2: Functionality Gaps (Before Feature Complete) 6. ~~Add `SelectPrizeAction` executor~~ DONE (#11) -7. Fix stadium discard ownership +7. ~~Fix stadium discard ownership~~ DONE (#12) 8. ~~Add turn limit check~~ DONE (#15) 9. ~~Fix per-ability usage tracking~~ DONE (#9) @@ -336,7 +340,7 @@ Also added `GameEndReason.TURN_LIMIT` enum value to distinguish from `TIMEOUT` ( | `win_conditions.py` | 99% | Near complete | | `engine.py` | 81% | Gaps in error paths | | `rng.py` | 93% | Good | -| **TOTAL** | **94%** | **795 tests** | +| **TOTAL** | **94%** | **797 tests** | ### New Tests Added - `tests/core/test_evolution_stack.py` - 28 tests for evolution stack, devolve, knockout with attachments, find_card_instance @@ -345,6 +349,7 @@ Also added `GameEndReason.TURN_LIMIT` enum value to distinguish from `TIMEOUT` ( - `tests/core/test_turn_manager.py::TestPrizeCardModeKnockout` - 4 tests for prize card mode in knockouts - `tests/core/test_models/test_card.py::TestCardInstanceTurnState::test_can_use_ability_independent_tracking` - 1 test for per-ability usage tracking - `tests/core/test_models/test_game_state.py::TestForcedActionQueue` - 8 tests for forced action queue management +- `tests/core/test_engine.py::TestPlayTrainerAction` - 2 new tests for stadium ownership --- @@ -358,6 +363,16 @@ Also added `GameEndReason.TURN_LIMIT` enum value to distinguish from `TIMEOUT` ( ## Change Log +### 2026-01-26 - Stadium Ownership Tracking (Issue #12) + +Added `stadium_owner_id` field to track who played the current stadium: +- When a stadium is played, `stadium_owner_id` is set to the player's ID +- When a stadium is replaced, old stadium discards to owner's pile (not current player) +- Added `stadium_owner_id` to VisibleGameState for client visibility +- Updated existing test and added 2 new tests for stadium ownership + +**Total: 797 tests passing** + ### 2026-01-26 - Forced Action Queue for Double Knockouts (Issue #10) Changed `forced_action` from single item to FIFO queue: diff --git a/backend/app/core/engine.py b/backend/app/core/engine.py index 984ddf6..a45c658 100644 --- a/backend/app/core/engine.py +++ b/backend/app/core/engine.py @@ -629,19 +629,18 @@ class GameEngine: player.supporters_played_this_turn += 1 elif card_def.trainer_type == TrainerType.STADIUM: player.stadiums_played_this_turn += 1 - # Handle stadium replacement - if game.stadium_in_play: + # Handle stadium replacement - discard to original owner + if game.stadium_in_play and game.stadium_owner_id: old_stadium = game.stadium_in_play - # Find stadium owner and discard - for p in game.players.values(): - if ( - old_stadium.instance_id in p.active - or old_stadium.instance_id in p.bench - ): - continue # Stadiums aren't on field - # Just discard old stadium - player.discard.add(old_stadium) + owner = game.players.get(game.stadium_owner_id) + if owner: + owner.discard.add(old_stadium) + else: + # Fallback if owner not found (shouldn't happen) + player.discard.add(old_stadium) + # Set new stadium and track owner game.stadium_in_play = card + game.stadium_owner_id = player.player_id return ActionResult( success=True, message="Stadium played", diff --git a/backend/app/core/models/game_state.py b/backend/app/core/models/game_state.py index 9aafecf..34c601c 100644 --- a/backend/app/core/models/game_state.py +++ b/backend/app/core/models/game_state.py @@ -372,6 +372,7 @@ class GameState(BaseModel): winner_id: ID of the winning player, if game has ended. end_reason: Why the game ended, if it has ended. stadium_in_play: The current Stadium card in play, if any. + stadium_owner_id: Player ID of who played the current stadium (for discard). turn_order: List of player IDs in turn order. first_turn_completed: Whether the very first turn of the game is done. forced_actions: Queue of ForcedAction items that must be completed before game proceeds. @@ -395,6 +396,7 @@ class GameState(BaseModel): # Shared game state stadium_in_play: CardInstance | None = None + stadium_owner_id: str | None = None # Turn order (for 2+ player support) turn_order: list[str] = Field(default_factory=list) diff --git a/backend/app/core/visibility.py b/backend/app/core/visibility.py index 861d991..3d50f66 100644 --- a/backend/app/core/visibility.py +++ b/backend/app/core/visibility.py @@ -121,6 +121,7 @@ class VisibleGameState(BaseModel): winner_id: Winner if game is over. end_reason: Why the game ended. stadium_in_play: Current stadium card (public). + stadium_owner_id: Player who played the current stadium (public). forced_action: Current forced action, if any. card_registry: Card definitions (needed to display cards). """ @@ -143,6 +144,7 @@ class VisibleGameState(BaseModel): # Shared state stadium_in_play: CardInstance | None = None + stadium_owner_id: str | None = None # Forced action (visible to know what's required) forced_action_player: str | None = None @@ -291,6 +293,7 @@ def get_visible_state(game: GameState, viewer_id: str) -> VisibleGameState: winner_id=game.winner_id, end_reason=game.end_reason, stadium_in_play=game.stadium_in_play, + stadium_owner_id=game.stadium_owner_id, forced_action_player=forced_action_player, forced_action_type=forced_action_type, forced_action_reason=forced_action_reason, @@ -385,6 +388,7 @@ def get_spectator_state(game: GameState) -> VisibleGameState: winner_id=game.winner_id, end_reason=game.end_reason, stadium_in_play=game.stadium_in_play, + stadium_owner_id=game.stadium_owner_id, forced_action_player=forced_action_player, forced_action_type=forced_action_type, forced_action_reason=forced_action_reason, diff --git a/backend/tests/core/test_engine.py b/backend/tests/core/test_engine.py index 30cdab7..7044ab5 100644 --- a/backend/tests/core/test_engine.py +++ b/backend/tests/core/test_engine.py @@ -1914,9 +1914,10 @@ class TestPlayTrainerAction: game.card_registry[different_stadium_def.id] = different_stadium_def p1 = game.players["player1"] - # Put an existing stadium in play + # Put an existing stadium in play (owned by player2) old_stadium = CardInstance(instance_id="old-stadium", definition_id=stadium_card_def.id) game.stadium_in_play = old_stadium + game.stadium_owner_id = "player2" # player2 owns the existing stadium # Add a different stadium to hand (different from old one) new_stadium = CardInstance( @@ -1930,9 +1931,81 @@ class TestPlayTrainerAction: assert result.success # New stadium should be in play assert game.stadium_in_play.instance_id == "new-stadium" - # Old stadium should be discarded + # New stadium should be owned by player1 + assert game.stadium_owner_id == "player1" + # Old stadium should be discarded to its OWNER's discard (player2) + p2 = game.players["player2"] + assert "old-stadium" in p2.discard + + @pytest.mark.asyncio + async def test_play_stadium_replace_own_stadium( + self, + engine: GameEngine, + game_for_trainer: tuple[GameState, dict], + stadium_card_def: CardDefinition, + different_stadium_def: CardDefinition, + ): + """ + Test that replacing own stadium discards to own pile. + + When a player replaces their own stadium, the old one goes to + their own discard pile (not the opponent's). + """ + game, _ = game_for_trainer + game.card_registry[different_stadium_def.id] = different_stadium_def + p1 = game.players["player1"] + + # Put player1's stadium in play + old_stadium = CardInstance(instance_id="old-stadium", definition_id=stadium_card_def.id) + game.stadium_in_play = old_stadium + game.stadium_owner_id = "player1" # player1 owns the existing stadium + + # Add a different stadium to hand + new_stadium = CardInstance( + instance_id="new-stadium", definition_id=different_stadium_def.id + ) + p1.hand.add(new_stadium) + + action = PlayTrainerAction(card_instance_id="new-stadium") + result = await engine.execute_action(game, "player1", action) + + assert result.success + assert game.stadium_in_play.instance_id == "new-stadium" + assert game.stadium_owner_id == "player1" + # Old stadium goes to player1's discard (the owner) assert "old-stadium" in p1.discard + @pytest.mark.asyncio + async def test_play_first_stadium_sets_owner( + self, + engine: GameEngine, + game_for_trainer: tuple[GameState, dict], + stadium_card_def: CardDefinition, + ): + """ + Test that playing the first stadium sets the owner. + + When no stadium is in play, playing one sets both the + stadium_in_play and stadium_owner_id. + """ + game, _ = game_for_trainer + p1 = game.players["player1"] + + # No stadium in play initially + assert game.stadium_in_play is None + assert game.stadium_owner_id is None + + # Add stadium to hand + stadium = CardInstance(instance_id="first-stadium", definition_id=stadium_card_def.id) + p1.hand.add(stadium) + + action = PlayTrainerAction(card_instance_id="first-stadium") + result = await engine.execute_action(game, "player1", action) + + assert result.success + assert game.stadium_in_play.instance_id == "first-stadium" + assert game.stadium_owner_id == "player1" + @pytest.mark.asyncio async def test_play_trainer_card_not_found( self,