Fix per-ability usage tracking (Issue #9)
Changed ability_uses_this_turn from int to dict[int, int] to track each ability's usage independently: - ability_uses_this_turn: dict[int, int] maps ability index to use count - can_use_ability() now requires ability_index parameter - Added get_ability_uses() and increment_ability_uses() helper methods - reset_turn_state() clears the dict instead of setting to 0 This fixes the bug where using one ability could incorrectly block another ability on the same Pokemon from being used. 789 tests passing.
This commit is contained in:
Cal Corum
parent
0534c57430
commit
8e084d250a
@ -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:
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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.
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -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 == {}
|
||||
|
||||
|
||||
# ============================================================================
|
||||
|
||||
@ -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(
|
||||
|
||||
Loading…
Reference in New Issue
Block a user