diff --git a/backend/SYSTEM_REVIEW.md b/backend/SYSTEM_REVIEW.md index 159b1c2..f8ef4a0 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:** 788 tests passing, 94% coverage +**Status:** 789 tests passing, 94% coverage --- @@ -150,12 +150,18 @@ Additionally, a new `devolve` effect handler was added for removing evolution st ## Medium Priority Issues -### 9. Per-Ability Usage Tracking Flawed -**File:** `app/core/models/card.py:334` +### 9. ~~Per-Ability Usage Tracking Flawed~~ FIXED +**File:** `app/core/models/card.py:390` +**Status:** FIXED in current session -`CardInstance.ability_uses_this_turn` is a single integer, but a Pokemon can have multiple abilities with different `uses_per_turn` limits. The counter can't distinguish between them. +Changed `ability_uses_this_turn` from `int` to `dict[int, int]` to track usage per ability: +- `ability_uses_this_turn: dict[int, int]` maps ability index to use count +- `can_use_ability(ability, ability_index)` now requires the ability index +- Added `get_ability_uses(ability_index)` helper method +- Added `increment_ability_uses(ability_index)` helper method +- `reset_turn_state()` now clears the dict instead of setting to 0 -**Fix:** Change to `dict[int, int]` tracking uses per ability index. +This ensures Pokemon with multiple abilities (e.g., one limited to 1/turn, another to 2/turn) track each ability independently. --- @@ -297,7 +303,7 @@ Also added `GameEndReason.TURN_LIMIT` enum value to distinguish from `TIMEOUT` ( 6. ~~Add `SelectPrizeAction` executor~~ DONE (#11) 7. Fix stadium discard ownership 8. ~~Add turn limit check~~ DONE (#15) -9. Fix per-ability usage tracking +9. ~~Fix per-ability usage tracking~~ DONE (#9) ### Phase 3: Polish (Before Production) 10. Add CardDefinition field validation @@ -325,13 +331,14 @@ 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%** | **788 tests** | +| **TOTAL** | **94%** | **789 tests** | ### New Tests Added - `tests/core/test_evolution_stack.py` - 28 tests for evolution stack, devolve, knockout with attachments, find_card_instance - `tests/core/test_engine.py::TestSelectPrizeAction` - 3 tests for SelectPrizeAction execution - `tests/core/test_engine.py::TestTurnLimitCheck` - 5 tests for turn limit checking - `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 --- @@ -345,6 +352,17 @@ Also added `GameEndReason.TURN_LIMIT` enum value to distinguish from `TIMEOUT` ( ## Change Log +### 2026-01-26 - Per-Ability Usage Tracking (Issue #9) + +Fixed issue #9 - ability usage now tracked per-ability instead of globally: +- Changed `ability_uses_this_turn: int` to `dict[int, int]` +- `can_use_ability()` now requires `ability_index` parameter +- Added `get_ability_uses()` and `increment_ability_uses()` helper methods +- Updated engine, rules_validator, and all affected tests +- Added new test `test_can_use_ability_independent_tracking` to verify fix + +**Total: 789 tests passing** + ### 2026-01-26 - SelectPrizeAction and Turn Limit Check (Issues #11, #15) Fixed medium priority issues #11 and #15: diff --git a/backend/app/core/engine.py b/backend/app/core/engine.py index 61503d4..3d6a881 100644 --- a/backend/app/core/engine.py +++ b/backend/app/core/engine.py @@ -686,8 +686,8 @@ class GameEngine: ability = card_def.abilities[action.ability_index] - # Mark ability as used (increment counter) - pokemon.ability_uses_this_turn += 1 + # Mark ability as used (increment counter for this specific ability) + pokemon.increment_ability_uses(action.ability_index) # Execute ability effect (placeholder - would use effect system) return ActionResult( diff --git a/backend/app/core/models/card.py b/backend/app/core/models/card.py index 7bad025..54e52dd 100644 --- a/backend/app/core/models/card.py +++ b/backend/app/core/models/card.py @@ -356,8 +356,9 @@ class CardInstance(BaseModel): later indices are more recent evolutions. When the Pokemon is knocked out, all cards in this stack go to the discard pile. status_conditions: Active status conditions on this Pokemon. - ability_uses_this_turn: Number of times abilities have been used this turn. - Compared against Ability.uses_per_turn to determine if more uses allowed. + ability_uses_this_turn: Number of times each ability has been used this turn. + Maps ability index to use count. Compared against Ability.uses_per_turn + to determine if more uses of that specific ability are allowed. hp_modifier: Additive modifier to the Pokemon's base HP. Can be positive (e.g., from Tool cards) or negative (e.g., from effects). retreat_cost_modifier: Additive modifier to retreat cost. Negative values @@ -387,7 +388,7 @@ class CardInstance(BaseModel): attached_tools: list["CardInstance"] = Field(default_factory=list) cards_underneath: list["CardInstance"] = Field(default_factory=list) status_conditions: list[StatusCondition] = Field(default_factory=list) - ability_uses_this_turn: int = 0 + ability_uses_this_turn: dict[int, int] = Field(default_factory=dict) # Stat modifiers (applied by card effects, tools, abilities, etc.) hp_modifier: int = 0 @@ -498,23 +499,45 @@ class CardInstance(BaseModel): def reset_turn_state(self) -> None: """Reset per-turn state counters. Called at the start of each turn.""" - self.ability_uses_this_turn = 0 + self.ability_uses_this_turn.clear() - def can_use_ability(self, ability: Ability) -> bool: + def can_use_ability(self, ability: Ability, ability_index: int) -> bool: """Check if this Pokemon can use the given ability. - Compares ability_uses_this_turn against the ability's uses_per_turn limit. + Compares the use count for this specific ability against its uses_per_turn limit. If uses_per_turn is None, the ability has no usage limit. Args: ability: The Ability to check. + ability_index: The index of the ability in the Pokemon's ability list. Returns: True if the ability can be used (hasn't hit its per-turn limit). """ if ability.uses_per_turn is None: return True # Unlimited uses - return self.ability_uses_this_turn < ability.uses_per_turn + uses = self.ability_uses_this_turn.get(ability_index, 0) + return uses < ability.uses_per_turn + + def get_ability_uses(self, ability_index: int) -> int: + """Get the number of times a specific ability has been used this turn. + + Args: + ability_index: The index of the ability. + + Returns: + Number of times the ability has been used this turn. + """ + return self.ability_uses_this_turn.get(ability_index, 0) + + def increment_ability_uses(self, ability_index: int) -> None: + """Increment the usage counter for a specific ability. + + Args: + ability_index: The index of the ability that was used. + """ + current = self.ability_uses_this_turn.get(ability_index, 0) + self.ability_uses_this_turn[ability_index] = current + 1 def can_evolve_this_turn(self, current_turn: int) -> bool: """Check if this Pokemon can evolve this turn. diff --git a/backend/app/core/rules_validator.py b/backend/app/core/rules_validator.py index 12b3074..764cddc 100644 --- a/backend/app/core/rules_validator.py +++ b/backend/app/core/rules_validator.py @@ -705,12 +705,12 @@ def _validate_use_ability( ability = definition.abilities[action.ability_index] # Check ability usage limit - if not pokemon.can_use_ability(ability): + if not pokemon.can_use_ability(ability, action.ability_index): uses_limit = ability.uses_per_turn + current_uses = pokemon.get_ability_uses(action.ability_index) return ValidationResult( valid=False, - reason=f"Ability '{ability.name}' already used " - f"({pokemon.ability_uses_this_turn}/{uses_limit} this turn)", + reason=f"Ability '{ability.name}' already used ({current_uses}/{uses_limit} this turn)", ) return ValidationResult(valid=True) diff --git a/backend/tests/core/test_engine.py b/backend/tests/core/test_engine.py index b376668..309ce2f 100644 --- a/backend/tests/core/test_engine.py +++ b/backend/tests/core/test_engine.py @@ -2038,9 +2038,9 @@ class TestUseAbilityAction: assert result.success assert "Static" in result.message - # Ability should be recorded as used + # Ability should be recorded as used (ability index 0) active = game.players["player1"].get_active_pokemon() - assert active.ability_uses_this_turn >= 1 + assert active.get_ability_uses(0) >= 1 @pytest.mark.asyncio async def test_use_ability_pokemon_not_found( diff --git a/backend/tests/core/test_models/test_card.py b/backend/tests/core/test_models/test_card.py index e3705aa..4da8508 100644 --- a/backend/tests/core/test_models/test_card.py +++ b/backend/tests/core/test_models/test_card.py @@ -1403,11 +1403,11 @@ class TestCardInstanceTurnState: to be used again. """ instance = CardInstance(instance_id="uuid", definition_id="pikachu") - instance.ability_uses_this_turn = 3 + instance.ability_uses_this_turn = {0: 3, 1: 2} instance.reset_turn_state() - assert instance.ability_uses_this_turn == 0 + assert instance.ability_uses_this_turn == {} def test_can_use_ability_once_per_turn(self) -> None: """ @@ -1418,10 +1418,10 @@ class TestCardInstanceTurnState: instance = CardInstance(instance_id="uuid", definition_id="pikachu") ability = Ability(name="Static", effect_id="static_paralysis") # Default: uses_per_turn=1 - assert instance.can_use_ability(ability) + assert instance.can_use_ability(ability, ability_index=0) - instance.ability_uses_this_turn = 1 - assert not instance.can_use_ability(ability) + instance.increment_ability_uses(0) + assert not instance.can_use_ability(ability, ability_index=0) def test_can_use_ability_multiple_per_turn(self) -> None: """ @@ -1432,13 +1432,13 @@ class TestCardInstanceTurnState: instance = CardInstance(instance_id="uuid", definition_id="pikachu") ability = Ability(name="Double Draw", effect_id="draw_cards", uses_per_turn=2) - assert instance.can_use_ability(ability) + assert instance.can_use_ability(ability, ability_index=0) - instance.ability_uses_this_turn = 1 - assert instance.can_use_ability(ability) + instance.increment_ability_uses(0) + assert instance.can_use_ability(ability, ability_index=0) - instance.ability_uses_this_turn = 2 - assert not instance.can_use_ability(ability) + instance.increment_ability_uses(0) + assert not instance.can_use_ability(ability, ability_index=0) def test_can_use_ability_unlimited(self) -> None: """ @@ -1449,13 +1449,46 @@ class TestCardInstanceTurnState: instance = CardInstance(instance_id="uuid", definition_id="pikachu") ability = Ability(name="Energy Transfer", effect_id="move_energy", uses_per_turn=None) - assert instance.can_use_ability(ability) + assert instance.can_use_ability(ability, ability_index=0) - instance.ability_uses_this_turn = 10 - assert instance.can_use_ability(ability) + for _ in range(10): + instance.increment_ability_uses(0) + assert instance.can_use_ability(ability, ability_index=0) - instance.ability_uses_this_turn = 100 - assert instance.can_use_ability(ability) + for _ in range(90): + instance.increment_ability_uses(0) + assert instance.can_use_ability(ability, ability_index=0) + + def test_can_use_ability_independent_tracking(self) -> None: + """ + Verify that ability usage is tracked per-ability, not globally. + + A Pokemon with multiple abilities should track each one independently. + Using ability 0 should not affect the usage count for ability 1. + """ + instance = CardInstance(instance_id="uuid", definition_id="pikachu") + ability_0 = Ability(name="Static", effect_id="static_paralysis", uses_per_turn=1) + ability_1 = Ability(name="Double Draw", effect_id="draw_cards", uses_per_turn=2) + + # Both abilities should be usable initially + assert instance.can_use_ability(ability_0, ability_index=0) + assert instance.can_use_ability(ability_1, ability_index=1) + + # Use ability 0 once (hits its limit) + instance.increment_ability_uses(0) + assert not instance.can_use_ability(ability_0, ability_index=0) + # Ability 1 should still be usable (independent tracking) + assert instance.can_use_ability(ability_1, ability_index=1) + + # Use ability 1 twice (hits its limit) + instance.increment_ability_uses(1) + instance.increment_ability_uses(1) + assert not instance.can_use_ability(ability_1, ability_index=1) + + # Verify counts + assert instance.get_ability_uses(0) == 1 + assert instance.get_ability_uses(1) == 2 + assert instance.get_ability_uses(2) == 0 # Unused ability class TestCardInstanceJsonRoundTrip: diff --git a/backend/tests/core/test_models/test_game_state.py b/backend/tests/core/test_models/test_game_state.py index 9271bec..6995577 100644 --- a/backend/tests/core/test_models/test_game_state.py +++ b/backend/tests/core/test_models/test_game_state.py @@ -723,7 +723,7 @@ class TestPlayerStateTurnActions: # Add a pokemon with ability uses this turn pokemon = make_card_instance("pokemon-1") - pokemon.ability_uses_this_turn = 3 + pokemon.ability_uses_this_turn = {0: 3, 1: 1} player.active.add(pokemon) player.reset_turn_state() @@ -733,7 +733,7 @@ class TestPlayerStateTurnActions: assert player.stadiums_played_this_turn == 0 assert player.items_played_this_turn == 0 assert player.retreats_this_turn == 0 - assert pokemon.ability_uses_this_turn == 0 + assert pokemon.ability_uses_this_turn == {} # ============================================================================ diff --git a/backend/tests/core/test_rules_validator.py b/backend/tests/core/test_rules_validator.py index 235ca76..ecdcbba 100644 --- a/backend/tests/core/test_rules_validator.py +++ b/backend/tests/core/test_rules_validator.py @@ -1204,7 +1204,7 @@ class TestUseAbilityValidation: # Add Shaymin EX with ability shaymin = card_instance_factory("shaymin_ex_001", instance_id="shaymin") - shaymin.ability_uses_this_turn = 1 # Already used + shaymin.ability_uses_this_turn = {0: 1} # Already used ability index 0 player.bench.add(shaymin) action = UseAbilityAction(