mantimon-tcg/backend/SYSTEM_REVIEW.md
Cal Corum 7fae1c61e8 Add CardDefinition validation for required fields (Issue #2)
- Add model_validator to enforce card-type-specific required fields
- Pokemon: require hp (positive), stage, pokemon_type
- Pokemon Stage 1/2 and VMAX/VSTAR: require evolves_from
- Trainer: require trainer_type
- Energy: require energy_type (auto-fills energy_provides)
- Update all test fixtures to include required fields
- Mark Issue #2 as FIXED in SYSTEM_REVIEW.md

765 tests passing
2026-01-26 10:28:37 -06:00

12 KiB

Mantimon TCG Core Engine - System Review

Date: 2026-01-25
Updated: 2026-01-26
Reviewed By: Multi-agent system review
Status: 765 tests passing, 94% coverage


Executive Summary

The core engine has a solid foundation with good separation of concerns, comprehensive documentation, and thorough test coverage. The review identified 8 critical issues - 5 have been fixed as part of the Energy/Evolution Stack refactor and CardDefinition validation work. The remaining 3 critical issues and several medium-priority gaps still need attention.

Fixed: #1, #2, #5, #7, #8
Still Open: #3 (end_turn knockouts), #4 (win condition timing), #6 (engine knockout processing)


Critical Issues (Must Fix)

1. Energy Zone Missing from Card Search FIXED

File: app/core/models/game_state.py:533-540
Severity: CRITICAL
Status: FIXED in commit 2b8fac4

The find_card_instance method now searches all zones including energy_zone, plus:

  • attached_energy - Energy cards attached to Pokemon in play
  • attached_tools - Tool cards attached to Pokemon in play
  • cards_underneath - Evolution stack (previous stages)
# Fixed - includes energy_zone and attached cards
for zone_name in ["deck", "hand", "active", "bench", "discard", "prizes", "energy_deck", "energy_zone"]:
# Also searches attached_energy, attached_tools, cards_underneath on Pokemon in play

2. No Validation That Pokemon Cards Have Required Fields FIXED

File: app/core/models/card.py:163-231
Severity: CRITICAL
Status: FIXED in commit TBD

Added a Pydantic model_validator to CardDefinition that enforces:

  • Pokemon cards require: hp (must be positive), stage, pokemon_type
  • Pokemon Stage 1/2 require: evolves_from
  • Pokemon VMAX/VSTAR require: evolves_from
  • Trainer cards require: trainer_type
  • Energy cards require: energy_type (auto-fills energy_provides if empty)

This prevents invalid card definitions at construction time rather than runtime errors later.


3. end_turn() Doesn't Process Knockouts

File: app/core/turn_manager.py:312-421
Severity: CRITICAL

end_turn() identifies knockouts from status damage and adds them to a knockouts list, but it never moves the KO'd Pokemon to discard. The comment says "should be handled by the game engine" but the engine also doesn't process them.

Impact: Pokemon knocked out by poison/burn damage remain in play.

Fix: Either:

  • Process knockouts fully in turn_manager.end_turn()
  • Or ensure GameEngine.end_turn() calls process_knockout() for each KO

4. Win Condition Checked Before Knockout Processing

File: app/core/turn_manager.py:398-401
Severity: CRITICAL

The code checks no_pokemon_in_play BEFORE the knockout is actually processed. The KO'd Pokemon is still in active.cards at this point.

Impact: Win condition check will always fail because the KO'd Pokemon is still technically "in play".

Fix: Move win condition check to AFTER knockout processing completes.


5. Energy Attachment Bug - Energy Card Disappears FIXED

File: app/core/engine.py:568
Severity: CRITICAL
Status: FIXED in commit 2b8fac4

The data model was changed to store full CardInstance objects:

  • attached_energy: list[CardInstance] (was list[str])
  • attached_tools: list[CardInstance] (was list[str])

Energy cards are now stored directly on the Pokemon they're attached to, not just as IDs. The find_card_instance method searches these attached cards, so they're always findable.

When a Pokemon is knocked out or retreats, attached energy moves to the owner's discard pile.


6. Status Knockouts Not Processed by Engine

File: app/core/engine.py:858-881
Severity: CRITICAL

GameEngine.end_turn() receives knockout information from TurnManager but doesn't process them:

def end_turn(self, game: GameState) -> ActionResult:
    result = self.turn_manager.end_turn(game, self.rng)
    if result.win_result:
        apply_win_result(game, result.win_result)
    return ActionResult(...)  # Knockouts not processed!

Fix: Add knockout processing loop that calls turn_manager.process_knockout() for each knockout in result.knockouts.


7. Confusion Status Not Handled in Attack FIXED

File: app/core/engine.py:676-726
Severity: CRITICAL
Status: FIXED in earlier commit

The _execute_attack method now handles Confusion status:

  • Checks if attacker has StatusCondition.CONFUSED
  • Flips a coin using the RNG
  • On tails: attack fails, Pokemon damages itself (configurable via rules.status.confusion_self_damage)
  • On heads: attack proceeds normally

The self-damage amount is configurable in RulesConfig.status.confusion_self_damage (default 30).


8. Energy Discard Handler Doesn't Move Cards FIXED

File: app/core/effects/handlers.py:479-517
Severity: CRITICAL
Status: FIXED in commit 2b8fac4

The discard_energy handler now:

  1. Finds the owner of the target Pokemon
  2. Calls detach_energy() which returns the CardInstance
  3. Adds the detached energy to the owner's discard pile

Additionally, a new devolve effect handler was added for removing evolution stages.


Medium Priority Issues

9. Per-Ability Usage Tracking Flawed

File: app/core/models/card.py:334

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.

Fix: Change to dict[int, int] tracking uses per ability index.


10. Double Knockout - Only One Forced Action

File: app/core/turn_manager.py:478-486

If both players' active Pokemon are KO'd simultaneously, only one forced_action can be set (last writer wins).

Fix: Support a queue of forced actions or handle simultaneous KOs specially.


11. No SelectPrizeAction Executor

File: app/core/engine.py:440-444

The SelectPrizeAction validator exists but there's no handler in _execute_action_internal. Prize card mode is broken.

Fix: Add _execute_select_prize method.


12. Stadium Discard Goes to Wrong Player

File: app/core/engine.py:608-624

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.

Fix: Track stadium ownership and discard to correct player.


13. No Knockout Detection After Damage Effects

File: app/core/effects/handlers.py

Neither deal_damage nor attack_damage check if the target is knocked out after applying damage.

Fix: Either return a knockout flag in EffectResult or document that knockout checking happens at engine level.


14. Exception Swallowing Hides Errors

File: app/core/effects/registry.py:97

except Exception as e:
    return EffectResult.failure(f"Effect '{effect_id}' failed: {e}")

This catches all exceptions including programming errors, making debugging difficult.

Fix: Log full traceback at ERROR level, or only catch expected exceptions.


15. Turn Limit Not Checked at Turn Start

File: app/core/engine.py:828-856

TurnManager.check_turn_limit() exists but GameEngine.start_turn() doesn't call it.

Fix: Add turn limit check before calling start_turn.


Low Priority / Observations

Models

  • ModifierMode enum not exported in __init__.py
  • ActionType enum exists but is unused (duplicates Literal values)
  • No maximum damage validation (negative damage possible)
  • Tools limit not validated at model level
  • Stadium ownership not tracked
  • No temporary effect system for "until end of turn" effects

Game Logic

  • Draw phase has no actions (may be intentional)
  • first_turn_of_game config exists in two places (FirstTurnConfig.can_evolve and EvolutionConfig.first_turn_of_game)
  • Prize card mode missing "taken" tracking vs "removed by effect"
  • Evolution doesn't validate stage progression (Stage 1 could "evolve" into Stage 1)
  • turn_number only increments on wraparound (could confuse turn limit calculations)

Effects System

  • No effect chaining for compound effects
  • No duration/timing system for temporary modifiers
  • No target selection callback for player-choice effects
  • Missing common effects: search_deck, switch_pokemon, force_switch, conditional_effect

Engine

  • Hardcoded hand size 7 in mulligan (should use rules.deck.starting_hand_size)
  • Async methods that don't need async (_execute_play_trainer, etc.)
  • No setup phase state machine
  • No get_legal_actions() method for UI/AI
  • No weakness/resistance in damage calculation
  • No undo/snapshot capability
  • Mulligan doesn't award opponent extra cards

Positive Observations

  1. Clean Architecture: Excellent separation of CardDefinition (immutable template) vs CardInstance (mutable state)

  2. Comprehensive Configuration: RulesConfig system supports multiple game variants (traditional, Pocket-style, custom)

  3. Well-Designed EffectContext: Rich helper methods for parameter extraction, coin flips, player/card access

  4. Testability: RandomProvider protocol with SeededRandom enables deterministic testing

  5. Zone Abstraction: Clean Zone class with rich set of methods for deck/hand manipulation

  6. Good Documentation: Comprehensive docstrings and type hints throughout

  7. Security Awareness: Visibility filter properly hides opponent hand, deck order, prizes


Phase 1: Critical Fixes (Before Any Testing) - 4/8 COMPLETE

  1. Fix find_card_instance to include energy_zone DONE
  2. Add CardDefinition field validation - STILL NEEDED
  3. Fix knockout processing in end_turn - STILL NEEDED
  4. Fix win condition check timing - STILL NEEDED
  5. Fix energy attachment to store cards properly DONE (full refactor to CardInstance)
  6. Fix engine to call process_knockout() for status KOs - STILL NEEDED
  7. Add confusion handling in attack execution DONE
  8. Fix energy discard handler to move cards DONE

Phase 2: Functionality Gaps (Before Feature Complete)

  1. Add SelectPrizeAction executor
  2. Fix stadium discard ownership
  3. Add turn limit check
  4. Fix per-ability usage tracking

Phase 3: Polish (Before Production)

  1. Add CardDefinition field validation
  2. Handle double knockouts
  3. Improve effect error handling
  4. Add missing effect handlers

Test Coverage Status

Module Coverage Notes
config.py 100% Complete
models/enums.py 100% Complete
models/card.py 100% Complete
models/actions.py 100% Complete
models/game_state.py 99% Near complete
effects/base.py 98% Near complete
effects/registry.py 100% Complete
effects/handlers.py 99% Near complete + devolve handler
rules_validator.py 94% Good
turn_manager.py 93% Good
visibility.py 95% Good
win_conditions.py 99% Near complete
engine.py 81% Gaps in error paths
rng.py 93% Good
TOTAL 94% 766 tests

New Test File Added

  • tests/core/test_evolution_stack.py - 28 tests for evolution stack, devolve, knockout with attachments, find_card_instance

Next Steps

  1. Review this document and prioritize fixes DONE
  2. Create GitHub issues for remaining critical items (#2, #3, #4, #6)
  3. Address Phase 1 fixes before continuing development 4/8 DONE
  4. Update tests as fixes are implemented DONE (766 tests)
  5. Re-run system review after remaining fixes

Change Log

2026-01-26 - Energy/Evolution Stack Refactor

Commits: 2b8fac4, dd2cadf

Fixed issues #1, #5, #7, #8 plus major enhancements:

Note: Issues #3 and #6 (knockout processing) were NOT fixed - we improved process_knockout() to handle attached cards correctly, but the engine still doesn't call it for status damage knockouts. That's a separate fix needed.

  • attached_energy and attached_tools changed from list[str] to list[CardInstance]
  • Added cards_underneath for evolution stack tracking
  • Added devolve effect handler
  • find_card_instance now searches attached cards and evolution stacks
  • Knockout processing discards all attached cards and evolution stack
  • Evolution clears status conditions (Pokemon TCG standard behavior)
  • 28 new comprehensive tests in test_evolution_stack.py

See PROJECT_PLAN_ENERGY_EVOLUTION.md for full implementation details.