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.
This commit is contained in:
Cal Corum
parent
9432499018
commit
554178dc6e
@ -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:
|
||||
|
||||
@ -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",
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -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,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user