fix(engine): bind Maze of Ith's bidirectional prevent anaphor to its own target#5484
Conversation
…own target
Maze of Ith ({T}: Untap target attacking creature. Prevent all combat
damage that would be dealt to and dealt by that creature this turn.)
parsed as Untap{target} with a child PreventDamage{target: Any} — the
"that creature" anaphor was dropped to the default Any instead of
binding to the same creature via TargetFilter::ParentTarget, and the
"dealt by" half (damage the creature deals) had no representation at
all.
Effect::PreventDamage's target + damage_source_filter fields combine
with AND semantics (game/replacement.rs), so "recipient==X OR
source==X" cannot be one shield. Splits the clause into two
PreventDamage nodes chained via SubAbilityLink::SequentialSibling: a
recipient-scoped "to" shield and a source-scoped-only "by" shield.
Two latent runtime gaps in game/effects/prevent_damage.rs, both
required for the split to actually work:
- resolve_source_filter had no arm for a bare ParentTarget damage
source filter (only ParentTargetSlot{index}); added one,
concretizing eagerly via first_object_target (promoted to
pub(crate)).
- source_scoped_prevent's gate only recognized the existing
And{ParentTargetSlot,...} shape, so a bare ParentTarget source
filter fell through to host_on_targets, which auto-filled
valid_card: SelfRef and silently re-imposed the recipient
constraint the source-only shield must not have. Widened the gate.
Anaphor recognition (parse_anaphoric_target_ref, oracle_target.rs)
delegates to the existing parse_target building block rather than
reimplementing the same grammar, gated on a new
parent_target_available: bool mirroring the established
ParseContext::parent_target_available convention.
Shipped scope: 7 cards sharing this exact singular-anaphor grammar
(Maze of Ith, Ebony Horse, Elvish Scout, Foxfire, Ith High Arcanist,
Maze of Shadows, Delirium). Deliberately deferred: a self-ref-activated
cohort ("this creature" — Deftblade Elite, Urborg Phantom, Moonlight
Geist), a plural-anaphor card (Energy Arc), and a multi-target/
unless-pay card (Winter's Chill) — verified via trace that the shipped
gate is a guaranteed no-op on Winter's Chill's inner clause, so no
guard was needed there.
Fixes phase-rs#1094.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbgwGxbU9NHN9kou9isp8K
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Parse changes introduced by this PR · 14 card(s), 3 signature(s) (baseline: main
|
matthewevans
left a comment
There was a problem hiding this comment.
APPROVE — maintainer sign-off.
This converts Maze of Ith's Prevent sentence from a single unbound PreventDamage { target: Any, scope: CombatDamage } (an effectively global combat shield) into a correctly-bound anaphor, at the right seam: the anaphor threads ParseContext.parent_target_available → UtilityImperativeAst::Prevent → parse_anaphoric_target_ref → TargetFilter::ParentTarget, and resolves at resolve() through the ability's own inherited target list (first_object_target(ability.targets)), never a board scan. CR 608.2c snapshot semantics hold.
Spot-verified against PR head 25c99c80:
parse_anaphoric_target_refreusesparse_targetand returnsSomeonly whenmatches!(filter, TargetFilter::ParentTarget)— it declines (returnsNone) on a fresh typed filter or theAnyfallback rather than guessing. It also short-circuitsNonewhenparent_target_availableis false.resolve_source_filter's bare-ParentTargetarm maps tofirst_object_target(ability_targets)→SpecificObject { id }(orNone) — the parent's own chosen object, no global rescan. Distinct fromparse_event_context_ref's "that creature" →TriggeringSourcetrigger path, which is correctly avoided in this non-trigger context.
Parse-diff confrontation (do not re-litigate the blast radius). Sticky baseline main 02bb03df, 14 cards / 3 signatures:
- 14 cards rebind
PreventDamage.targetfromany target→parent target. This is the whole prevent-with-anaphor class. - 7 cards additionally gained a
CombatDamage"by" node — precisely the bidirectional+combat cards: Maze of Ith, Ebony Horse, Elvish Scout, Delirium, Foxfire, Maze of Shadows, Ith High Arcanist ("...dealt to and dealt by [it/that creature]..."). - 1 card (Kiora, the Tide's Fury) gained an
AllDamage"by" node — it is bidirectional but all-damage ("Prevent all damage … dealt to and dealt by that permanent"). - Djeru's Resolve is single-direction ("dealt to it" only) and correctly received only the rebind, no "by" node — verifying the "by" node is gated on the "and dealt by" clause, not applied blanketly.
The blast radius exactly equals the stated class; every gained/changed card is explained.
Test discrimination. maze_of_ith_untaps_and_prevents_both_directions drives the real activation + combat pipeline and asserts BOTH shield directions (Mazed attacker and its blocker each take 0) AND that an un-Mazed attacker/blocker pair in the same combat still trades 2/2. Under the pre-fix global shield the un-Mazed pair would be shielded to 0/0, so those == 2 assertions FAIL on revert — a genuine negative-control discriminator. maze_of_ith_prevents_mazed_attacker_damage_to_defending_player asserts the defending player loses exactly the un-Mazed attacker's 3 (not 2+3=5), discriminating the source-scoped "by" shield.
Bidirectional "by" leg is a SequentialSibling with damage_source_filter = ParentTarget; recipient "to" leg is scoped to the parent-target object. Per-identity scoping, not a global shield. scenario.rs::add_land_from_oracle fills a real gap for the land-with-abilities class (existing helpers hardcode a mana ability, use Zone::Hand, or are creature-only). Fixtures use verbatim Oracle text.
All required checks green. Right seam, house idioms (reuses parse_target/ParentTarget rather than proliferating variants), discriminating runtime tests including a negative-control pair.
Summary
Fixes #1094 — [[Maze of Ith]] ({T}: Untap target attacking creature. Prevent all combat damage that would be dealt to and dealt by that creature this turn. — verified via Scryfall, from The Dark, 1994) doesn't untap the creature or prevent it from dealing or taking damage.
Root cause (originally diagnosed by @matthewevans in the issue's coverage-classifier comment): the ability parsed as
Untap { target } + child PreventDamage { target: Any }— the "that creature" anaphor was dropped to the defaultAnyinstead of binding to the same creature viaTargetFilter::ParentTarget. Separately, the clause is bidirectional ("dealt to and dealt by that creature") and the parser had no representation at all for the "dealt by" half (damage the creature deals, not just damage dealt to it).Effect::PreventDamage'starget+damage_source_filterfields combine with AND semantics (game/replacement.rs), so "recipient==X OR source==X" cannot be expressed as one shield. The fix splits the clause into twoPreventDamagenodes chained viaSubAbilityLink::SequentialSibling— a recipient-scoped "to" shield and a source-scoped-only "by" shield.Two latent runtime gaps in
game/effects/prevent_damage.rs, both required for the split to actually work, not scope creep:resolve_source_filterhad no arm for a bareParentTargetdamage-source filter (onlyParentTargetSlot{index}) — added one.source_scoped_prevent's gate only recognized the pre-existingAnd{ParentTargetSlot,...}shape, so a bareParentTargetsource filter fell through tohost_on_targets, which auto-filledvalid_card: SelfRefand silently re-imposed the recipient constraint the source-only shield must not have — widened the gate.Shipped scope
7 cards sharing this exact singular-anaphor grammar ("prevent...dealt to and dealt by
<that creature|it|the creature>"): Maze of Ith, Ebony Horse, Elvish Scout, Foxfire, Ith High Arcanist, Maze of Shadows, Delirium.Deliberately deferred (separate follow-up issues, not silently dropped):
TargetFilter::SelfRefrecipient-scoping carve-out inprevent_damage.rs::resolve(), currently untested and out of scope here.parent_target_availableis guaranteedfalsewhen its inner "prevent...that creature..." clause parses (its preceding sibling clause has a recognizedcondition.is_some(), which makes the backward target-referent walk bail before reaching the earlier typed target), so the new machinery is a guaranteed no-op there — no guard needed.Files changed
crates/engine/src/parser/oracle_target.rs—parse_anaphoric_target_ref, a thin wrapper delegating to the existingparse_targetbuilding block (gated on a newparent_target_available: bool).crates/engine/src/parser/oracle_ir/ast.rs—parent_target_available: boolfield onUtilityImperativeAst::Prevent.crates/engine/src/parser/oracle_effect/imperative.rs— threads the field through all 4 construction/consumption sites; factoredparse_prevention_amountout for shared reuse.crates/engine/src/parser/oracle_effect/lower.rs—try_parse_bidirectional_prevent, the sibling-splitting interceptor.crates/engine/src/game/effects/prevent_damage.rs— the two runtime fixes above.crates/engine/src/game/effects/mod.rs—first_object_targetpromoted topub(crate)for reuse.crates/engine/src/game/scenario.rs—add_land_from_oracle, a reusable test-harness builder (no existing helper places a land on the battlefield with parsed Oracle text).crates/engine/src/parser/oracle_tests.rs— unit tests for all 7 family cards + negative gate + sibling-clause survival (Foxfire's trailing draw trigger, Delirium's preceding tap/damage clauses).crates/engine/tests/integration/maze_of_ith_untap_bidirectional_prevent.rs(new) — discriminating runtime tests driving the real activate/combat pipeline, including a hostile un-Mazed attacker/blocker fixture and a defending-player-life-total fixture.CR references
CR 615, CR 615.1a, CR 615.11, CR 511.2, CR 608.2c, CR 302.6, CR 609.7, CR 701.26a/b — all verified against
docs/MagicCompRules.txt.Test plan
cargo fmt --allcleancargo clippy -p engine --lib -- -D warningsclean./scripts/check-parser-combinators.sh(Gate A): 0 violationscargo test -p engine --lib: 15964 passed, 0 failedcargo test -p engine --test integration: 2563 passed, 0 failed (includes both new Maze of Ith tests)resolve_source_filterfix, and thesource_scoped_preventgate widening — each flips both integration tests to failing; restored and re-confirmed greenparse_targetbuilding block (refactored to delegate)Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
https://claude.ai/code/session_01XbgwGxbU9NHN9kou9isp8K