Skip to content

fix(parser): Per-opponent 'they' anaphor in 'each opponent ... the life they lost this turn' emits L - #3334

Merged
matthewevans merged 2 commits into
phase-rs:mainfrom
ntindle:fix/who-misparse-11-per-opponent-they-anaphor
Jun 15, 2026
Merged

fix(parser): Per-opponent 'they' anaphor in 'each opponent ... the life they lost this turn' emits L#3334
matthewevans merged 2 commits into
phase-rs:mainfrom
ntindle:fix/who-misparse-11-per-opponent-they-anaphor

Conversation

@ntindle

@ntindle ntindle commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes a parser misparse affecting 1 card(s) in the Doctor Who Commander precons.

Root cause: Per-opponent 'they' anaphor in 'each opponent ... the life they lost this turn' emits LifeLostThisTurn{player:Controller} instead of ScopedPlayer, so resolve_quantity reads the printed controller's life loss for every iterated opponent.

Cards corrected

  • Wound Reflection

Fix

Implemented the approved plan for WHO cluster #11 (per-opponent / targeted "the life they lost this turn" anaphor) with two surgical leaf changes plus tests, exactly as specified.

Change A (crates/engine/src/parser/oracle_nom/quantity.rs): the five bare third-person life-lost anaphor arms in parse_life_lost_ref ("the life that player lost this turn", "the life they lost this turn", "the amount of life they lost this turn", "the life that player lost", "the life they lost") now emit PlayerScope::Target instead of PlayerScope::Controller. Replaced the stale comment (which cited the nonexistent "CR 608.2k") with a verified CR block citing CR 115.1 + CR 115.10 + CR 119.3 + CR 608.2c. First-person ("you"/"you've") and "your opponents" aggregate arms left unchanged.

Change B (crates/engine/src/parser/oracle_effect/mod.rs): added one QuantityRef::LifeLostThisTurn { Target } -> ScopedPlayer rewrite arm in rewrite_player_scope_refs, immediately after the analogous HandSize { Target } arm, so under a lifted per-opponent player_scope the Target form rebinds to ScopedPlayer (mirroring the existing LifeTotal/HandSize Betor/Blood Tribute path). A purely targeted clause (Blitzwing, no player_scope) never reaches this walker, so its Target survives.

imperative.rs: doc-comment correction only (the comment claimed the bare arms map to Controller; they now map to Target). No logic change.

Tests added at three levels: leaf (quantity.rs), rewrite-walker (oracle_effect/mod.rs, 3 tests modeled on nested_each_opponent_loses_half_life_uses_scoped_player), and runtime end-to-end (life.rs, 3-player per-opponent ScopedPlayer with a controller trap value).

Verification: cargo fmt clean; cargo clippy -p engine --all-targets -D warnings exit 0 (no warnings); full cargo test -p engine green (0 failures across all binaries); 6 new tests pass; 63 life_lost/scoped_player tests pass. Regenerated card-data.json (35373 cards). Spot-check confirms all 5 cards: Wound Reflection / Archfiend of Despair / Warlock Class -> LifeLostThisTurn{ScopedPlayer}; Blitzwing -> LifeLostThisTurn{Target}; Astarion (Feed) -> LifeLostThisTurn{Target} unchanged. Scope tally across all cards is healthy (Controller 6, Opponent 21, AllPlayers 2, ScopedPlayer 4, Target 2) with no Controller-form regression; False Cure still has no LifeLostThisTurn ref (untouched). The fix corrected a fourth previously-broken sibling beyond the three named, a build-for-the-class win.

Parser diff gate: PASS on my changes (zero contains/starts_with/find/ends_with introduced in any modified file).

One test-shape correction vs the plan (not a logic deviation): the plan's leaf test for "the amount of life they lost this turn" is unreachable via parse_quantity_ref because parse_life_lost_ref's leading opt("the amount of ") strip consumes the gloss and there is no bare "life they lost" arm (the "amount of" tag arm is pre-existing dead code). That form was and remains routed through parse_target_relative_life_change_this_turn (imperative.rs), which already yields Target. I scoped the leaf test to the two reachable article-only forms (the actual card phrasings) and documented why. Runtime behavior matches the plan exactly. No code logic differs from the plan.

Files changed

  • crates/engine/src/parser/oracle_nom/quantity.rs
  • crates/engine/src/parser/oracle_effect/mod.rs
  • crates/engine/src/parser/oracle_effect/imperative.rs
  • crates/engine/src/game/effects/life.rs

CR references

  • CR 115.1 (target declared as part of putting ability on stack; read from ability.targets) - verified docs/MagicCompRules.txt:836
  • CR 115.10 (spells/abilities can affect players they don't target) - verified docs/MagicCompRules.txt:884
  • CR 115.10a (being affected does not make a player a target unless identified by 'target') - verified docs/MagicCompRules.txt:886
  • CR 119.3 (gain/lose life adjusts that player's life total) - verified docs/MagicCompRules.txt:1063
  • CR 608.2c (read the whole text and apply the rules of English; corrects the plan's nonexistent 'CR 608.2k') - verified docs/MagicCompRules.txt:2789
  • CR 109.5 ('you'/'your' = the object's controller) - verified docs/MagicCompRules.txt:610

Verification

  • cargo fmt --all — clean (no changes)
  • ./scripts/check-parser-combinators.sh <upstream/main merge-base cb4990504> — clean (exit 0)
  • cargo clippy -p engine --all-targets -- -D warnings — clean (exit 0, no warnings)
  • cargo test -p engine — clean (exit 0, 0 failed)
  • cargo run --profile tool --features cli --bin oracle-gen -- data --filter "wound reflection" — passed (after freeing disk space by removing regenerable target/debug/incremental cache)
    Cards confirmed re-parsed correctly: Wound Reflection

🤖 Generated with Claude Code

@ntindle
ntindle requested a review from matthewevans as a code owner June 15, 2026 01:18
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@matthewevans matthewevans self-assigned this Jun 15, 2026
@matthewevans matthewevans added the bug Bug fix label Jun 15, 2026

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I re-read the diff against origin/main and verified the important seams: the third-person life-lost quantity now parses as Target, per-opponent rewrite converts that target scope to ScopedPlayer, and the runtime resolver reads the scoped player's life_lost_this_turn rather than the controller trap value.

Local verification passed:

  • cargo fmt --all -- --check
  • ./scripts/check-parser-combinators.sh origin/main
  • CARGO_TARGET_DIR=/Users/matt/dev/forge.rs-pr-target cargo test -p engine --lib life_lost -- --nocapture
  • CARGO_TARGET_DIR=/Users/matt/dev/forge.rs-pr-target cargo test -p engine --lib they_lost -- --nocapture
  • CARGO_TARGET_DIR=/Users/matt/dev/forge.rs-pr-target cargo test -p engine --lib that_player_lost -- --nocapture

GitHub check rollup is green on head 901baccaeb4ae8de69faeb874717c756dd07f3c6.

@matthewevans
matthewevans added this pull request to the merge queue Jun 15, 2026
@matthewevans matthewevans removed their assignment Jun 15, 2026
Merged via the queue into phase-rs:main with commit 135d12e Jun 15, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants