From 6a94e98c75cd4499e6d0921bb4afed4bd85c0b38 Mon Sep 17 00:00:00 2001 From: Rajah James Date: Thu, 9 Jul 2026 11:49:50 -0400 Subject: [PATCH 1/8] fix(engine): reanimate the enchanted creature for Animate Dead / Dance of the Dead (#4767) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes GitHub issue phase-rs/phase#4767. Animate Dead's ETB reanimation never worked because of two compounding bugs: the ETB effect text was never recognized by the parser (fell to Unimplemented / was absorbed into the wrong clause), and even once fixed, the Aura could never attach to its own graveyard-zone cast-time target in the first place because two resolution-path checks hardcoded "target must be on the battlefield" instead of consulting the Aura's own zone-scoped Enchant filter. Three layers: 1. Runtime: a new TargetFilter::OriginalSource variant names an ability's pre-forward_result-rebind source identity, concretized eagerly in-place at the one point in effects/mod.rs's forward_result handling where the pre-rebind ability.source_id and the about-to-mutate sub-ability clone coexist (CR 608.2c). A companion fix in snapshot_transient_modifications concretizes Keyword::Enchant(ParentTarget) to a concrete SpecificObject (CR 613.1f). Sacrifice's controller-scope guard now lets "that creature's controller sacrifices it" be performed by the creature's CURRENT controller even if control changed since the delayed trigger was created (CR 701.21a). A related delayed_trigger.rs fix makes parent_target_snapshot prefer the node's own propagated targets over the TriggeringSource fallback when the root chain is empty (CR 608.2c) — without it the delayed leaves-battlefield sacrifice snapshotted the Aura instead of the reanimated creature. 2. Parser: a new whole-body, fail-closed recognizer (try_parse_reanimator_aura_etb_effect) builds the 4-node ChangeZone -> GenericEffect -> Attach -> CreateDelayedTrigger chain directly from Oracle text, generalized over the class's verb/ destination axis (Animate Dead's "return ... to the battlefield" and Dance of the Dead's "put ... onto the battlefield tapped"). 3. Initial-attach prerequisite: ability_utils.rs's target re-validation and stack.rs's Aura-spell-resolution attach guard both now consult the Aura's own Keyword::Enchant filter (via the existing aura_enchant_filter / is_valid_attachment_target authorities) instead of hardcoding battlefield-only, fixing every zone-scoped Enchant Aura (Animate Dead, Dance of the Dead, Spellweaver Volute), not just this card. Corrected a mislabeled CR 303.4f citation on the touched stack.rs block to CR 608.3c. Includes a class-general parser test for both cards plus discriminating runtime tests driving the real cast -> resolve-spell -> resolve-ETB pipeline, including a hostile control-change-before-sacrifice fixture. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XbgwGxbU9NHN9kou9isp8K --- crates/engine/src/game/ability_rw.rs | 16 +- crates/engine/src/game/ability_scan.rs | 3 + crates/engine/src/game/ability_utils.rs | 45 ++ crates/engine/src/game/casting_tests.rs | 388 ++++++++++++++++++ crates/engine/src/game/cost_payability.rs | 4 + crates/engine/src/game/coverage.rs | 10 +- .../src/game/effects/delayed_trigger.rs | 18 +- crates/engine/src/game/effects/effect.rs | 24 ++ crates/engine/src/game/effects/mod.rs | 20 + crates/engine/src/game/effects/sacrifice.rs | 15 +- crates/engine/src/game/filter.rs | 19 + crates/engine/src/game/stack.rs | 26 +- crates/engine/src/game/trigger_matchers.rs | 1 + crates/engine/src/parser/oracle_effect/mod.rs | 190 +++++++++ crates/engine/src/parser/oracle_trigger.rs | 28 +- .../engine/src/parser/oracle_trigger_tests.rs | 220 ++++++++++ crates/engine/src/types/ability.rs | 31 ++ crates/mtgish-import/src/convert/condition.rs | 1 + 18 files changed, 1044 insertions(+), 15 deletions(-) diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index 8c8a224d4c..1fef48c875 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -1325,7 +1325,11 @@ enum WriteScope { /// Exhaustive & wildcard-free: a future `TargetFilter` variant must be classified. fn scope_of(target: &TargetFilter, chain_root: Option) -> WriteScope { match target { - TargetFilter::SelfRef | TargetFilter::SourceOrPaired => WriteScope::SelfSource, + // CR 608.2c: `OriginalSource` denotes the ability's own (pre-rebind) source + // object — a write to it lands on the source, exactly like `SelfRef`. + TargetFilter::SelfRef | TargetFilter::SourceOrPaired | TargetFilter::OriginalSource => { + WriteScope::SelfSource + } TargetFilter::TriggeringSource => WriteScope::EventObject, TargetFilter::ParentTarget | TargetFilter::ParentTargetSlot { .. } => { chain_root.unwrap_or(WriteScope::EventObject) @@ -2234,6 +2238,10 @@ fn legacy_target_filter(f: &TargetFilter) -> bool { | TargetFilter::ExiledCardByIndex { .. } | TargetFilter::SourceChosenPlayer | TargetFilter::OriginalController + // CR 201.5a: `OriginalSource` is not one of the 12 frozen event-context + // tags — it is concretized to `SpecificObject` at resolution (mirrors + // `SpecificObject`, its concretized form). + | TargetFilter::OriginalSource | TargetFilter::DefendingPlayer | TargetFilter::HasChosenName | TargetFilter::Named { .. } @@ -2433,6 +2441,9 @@ fn member_bound_target_filter(f: &TargetFilter) -> bool { // legacy-12 tags (`legacy_batch_prompt`), resolution-local refs, and // uniformity-/owner-partition-invariant refs — all documented above. TargetFilter::SelfRef + // CR 608.2c: source carrier (writes_self/reads_src) — source-invariant, + // not per-member-bound; concretized to SpecificObject before this walk. + | TargetFilter::OriginalSource | TargetFilter::SourceOrPaired | TargetFilter::TriggeringSource | TargetFilter::ParentTarget @@ -6080,6 +6091,9 @@ fn rw_target_filter(x: &TargetFilter) -> RwProfile { | TargetFilter::ExiledCardByIndex { .. } | TargetFilter::SourceChosenPlayer | TargetFilter::OriginalController + // CR 201.5a: `OriginalSource` is a read-free object selector (concretized + // to `SpecificObject` at resolution). + | TargetFilter::OriginalSource | TargetFilter::DefendingPlayer | TargetFilter::HasChosenName | TargetFilter::Named { .. } diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index 830c6fcfde..955972338a 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -2384,6 +2384,9 @@ fn scan_target_filter(x: &TargetFilter) -> Axes { // CR 201.5a: a source-relative object ref (the granting object), like // SelfRef — no event/sibling/projected resource axis. TargetFilter::GrantingObject => Axes::NONE, + // CR 608.2c: source-relative object ref (concretized to SpecificObject), + // like SelfRef — no event/sibling/projected resource axis. + TargetFilter::OriginalSource => Axes::NONE, TargetFilter::SourceOrPaired => Axes::NONE, TargetFilter::Typed(..) => Axes::CONSERVATIVE, TargetFilter::Not { filter } => { diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index 0de41e2cc5..7db12b5b0e 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -1472,6 +1472,16 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) - }) .collect() } else if let Effect::Attach { attachment, target } = &validated.effect { + // NOTE (phase#4767): both SelfRef and ParentTarget are excluded from needing + // a target slot here, so `kept` can end up `[]` for an Attach node whose + // chain is otherwise target-less end-to-end — harmless today only because + // stack.rs's `!original_targets.is_empty()` gate then routes such chains + // through the unvalidated branch instead of calling this function at all + // (confirmed for the reanimator-Aura Attach shape: Animate Dead / Dance of + // the Dead). A future chain combining Attach with a genuinely-targeted + // sibling node would need this branch to preserve rather than drop live + // SelfRef/ParentTarget entries — flag for separate review if that pattern + // arises. let mut kept = Vec::new(); let mut target_iter = validated.targets.iter(); for (is_attachment, filter) in [(true, attachment), (false, target)] { @@ -1653,6 +1663,41 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) - { validated.targets.clone() } + // CR 303.4a + CR 608.2b: A plain Aura spell has no separate on-cast + // effect — its resolving `Effect` is the `Effect::Unimplemented` + // placeholder built in `casting.rs`, so `extract_target_filter_from_effect` + // returns `None` and lands here. Its legal targets are defined by its + // enchant ability (`Keyword::Enchant`), NOT by the placeholder effect, + // and that filter may be zone-scoped (e.g. Animate Dead's "creature + // card in a graveyard"). Re-validate against the Enchant filter with + // the SAME machinery cast-time targeting uses, so a graveyard-zone host + // is not fizzle-filtered by the hardcoded battlefield check below. + // Gated on `Effect::Unimplemented` specifically (not on Aura-ness): the + // `None` arm also legitimately serves `Effect::Sacrifice`/`UnattachAll`/ + // `Bounce { selection: AtResolution }`, which must keep the plain + // battlefield fizzle-check. + None if matches!(validated.effect, Effect::Unimplemented { .. }) => { + match crate::game::effects::change_targets::aura_enchant_filter( + state, + validated.source_id, + ) { + Some(filter) => targeting::validate_targets_for_ability( + state, + &validated.targets, + &filter, + &validated, + ), + None => validated + .targets + .iter() + .filter(|target| match target { + TargetRef::Object(object_id) => state.battlefield.contains(object_id), + TargetRef::Player(_) => true, + }) + .cloned() + .collect(), + } + } None => validated .targets .iter() diff --git a/crates/engine/src/game/casting_tests.rs b/crates/engine/src/game/casting_tests.rs index feefe54fe8..d20ecb2031 100644 --- a/crates/engine/src/game/casting_tests.rs +++ b/crates/engine/src/game/casting_tests.rs @@ -35462,6 +35462,394 @@ fn animate_dead_enchant_filter_enumerates_graveyard_targets() { } } +/// CR 608.3c + CR 303.4a regression (issue #4767): a zone-scoped-Enchant Aura +/// (Animate Dead, "enchant creature card in a graveyard") must NOT fizzle when it +/// resolves, and must attach to its graveyard host. Drives the real pipeline: +/// `handle_cast_spell` (auto-target onto the stack) → `stack::resolve_top`, which +/// runs the CR 608.2b fizzle check through `validate_targets_in_chain` (Site A) +/// and then the Aura-attach block (Site B). +/// +/// Reverting Site A alone flips assertion (a): the generic `None` fizzle arm's +/// hardcoded `state.battlefield.contains` drops the graveyard target, the spell +/// fizzles to the graveyard, and the Aura never reaches the battlefield. +/// Reverting Site B alone (with Site A in place) flips assertion (b): the spell +/// now reaches the battlefield but the hardcoded `state.battlefield.contains` +/// attach guard rejects the graveyard host, so it never attaches. +#[test] +fn animate_dead_aura_spell_resolves_and_attaches_to_graveyard_creature() { + use std::str::FromStr; + + let mut state = setup_game_at_main_phase(); + + let aura_id = create_object( + &mut state, + CardId(601), + PlayerId(0), + "Animate Dead".to_string(), + Zone::Hand, + ); + { + let obj = state.objects.get_mut(&aura_id).unwrap(); + obj.card_types.core_types.push(CoreType::Enchantment); + obj.card_types.subtypes.push("Aura".to_string()); + // Same MTGJSON-parameterized construction path as + // `animate_dead_enchant_filter_enumerates_graveyard_targets` — not a + // hand-rolled filter. + let kw = Keyword::from_str("Enchant:creature card in a graveyard").unwrap(); + obj.keywords.push(kw); + obj.mana_cost = ManaCost::Cost { + shards: vec![ManaCostShard::Black], + generic: 0, + }; + } + add_mana(&mut state, PlayerId(0), ManaType::Black, 1); + + let creature_id = create_object( + &mut state, + CardId(602), + PlayerId(1), + "Grizzly Bears".to_string(), + Zone::Graveyard, + ); + state + .objects + .get_mut(&creature_id) + .unwrap() + .card_types + .core_types + .push(CoreType::Creature); + + let mut events = Vec::new(); + let result = + handle_cast_spell(&mut state, PlayerId(0), aura_id, CardId(601), &mut events).unwrap(); + assert!( + matches!(result, WaitingFor::Priority { .. }), + "expected auto-target onto the stack; got {result:?}" + ); + assert_eq!(state.stack.len(), 1, "Aura spell must be on the stack"); + + // Resolve ONLY the Aura spell itself. Any resulting ETB-trigger stack entry + // (the separately-implemented reanimation chain) is left unresolved — this + // test isolates the attach prerequisite. + stack::resolve_top(&mut state, &mut events); + + // (a) The Aura did NOT fizzle — it reached the battlefield. Fails today at + // Site A before the fix. + assert!( + state.battlefield.contains(&aura_id), + "Animate Dead must resolve onto the battlefield, not fizzle to the graveyard" + ); + // (b) Site B accepted the graveyard host and attached the Aura. + assert_eq!( + state.objects[&aura_id].attached_to, + Some(crate::game::game_object::AttachTarget::Object(creature_id)), + "Aura must be attached to the graveyard creature it targeted" + ); + // (c) The creature is STILL in the graveyard — this test does NOT exercise + // the separate ETB reanimation chain. + assert_eq!( + state.objects[&creature_id].zone, + Zone::Graveyard, + "creature must remain in the graveyard (ETB reanimation is out of scope here)" + ); +} + +/// Verbatim Animate Dead Oracle text (matches `crates/engine/tests/fixtures/ +/// integration_cards.json` — the repo's canonical corpus form, which uses the +/// self-reference "this Aura"). Reused across the end-to-end reanimation tests. +const ANIMATE_DEAD_ORACLE_FULL: &str = "Enchant creature card in a graveyard\nWhen this Aura enters, if it's on the battlefield, it loses \"enchant creature card in a graveyard\" and gains \"enchant creature put onto the battlefield with this Aura.\" Return enchanted creature card to the battlefield under your control and attach this Aura to it. When this Aura leaves the battlefield, that creature's controller sacrifices it.\nEnchanted creature gets -1/-0."; + +/// Drives Animate Dead's FULL end-to-end reanimation pipeline (issue #4767): +/// parse the complete Oracle text through the real `parse_oracle_text` pipeline +/// (so the ETB reanimation trigger is produced by `try_parse_reanimator_aura_etb_effect` +/// and attached to the object), cast the Aura, resolve the spell (Site A/B — +/// attach to the graveyard host), fire the resulting ETB trigger via the real +/// `process_triggers` path, then resolve the 4-node reanimation chain. +/// +/// Returns `(state, aura_id, creature_id)` for the reanimated board so the +/// delayed-sacrifice and control-change tests can build on it. `evaluate_layers` +/// has been run, so the keyword swap and the -1/-0 static are applied. +fn reanimate_grizzly_via_animate_dead() -> (GameState, ObjectId, ObjectId) { + use crate::parser::oracle::parse_oracle_text; + use std::str::FromStr; + + let mut state = setup_game_at_main_phase(); + + let aura_id = create_object( + &mut state, + CardId(601), + PlayerId(0), + "Animate Dead".to_string(), + Zone::Hand, + ); + // Real parser output — same construction path a fresh card-data export uses. + let parsed = parse_oracle_text( + ANIMATE_DEAD_ORACLE_FULL, + "Animate Dead", + &[], + &["Enchantment".to_string()], + &["Aura".to_string()], + ); + // Reach-guard: the live parser MUST attach the reanimator ETB trigger. If the + // recognizer's dispatch is reverted this fails here, so no downstream + // assertion can pass vacuously. + assert!( + !parsed.triggers.is_empty(), + "parser must produce the reanimator ETB trigger; got none" + ); + { + let obj = state.objects.get_mut(&aura_id).unwrap(); + obj.card_types.core_types.push(CoreType::Enchantment); + obj.card_types.subtypes.push("Aura".to_string()); + obj.base_card_types = obj.card_types.clone(); + // MTGJSON-parameterized Enchant keyword (mirrors the sibling test). + let enchant = Keyword::from_str("Enchant:creature card in a graveyard").unwrap(); + obj.base_keywords.push(enchant.clone()); + obj.keywords.push(enchant); + // Printed baseline abilities/triggers/statics from the live parser. The + // base_* fields are the layer system's single source of truth + // (`evaluate_layers` re-derives the live fields from them each pass), so + // both must be set for the trigger/static to survive layer evaluation. + obj.base_abilities = Arc::new(parsed.abilities.clone()); + obj.abilities = Arc::new(parsed.abilities.clone()); + obj.base_trigger_definitions = Arc::new(parsed.triggers.clone()); + obj.trigger_definitions = parsed.triggers.clone().into(); + obj.base_static_definitions = Arc::new(parsed.statics.clone()); + obj.static_definitions = parsed.statics.clone().into(); + obj.mana_cost = ManaCost::Cost { + shards: vec![ManaCostShard::Black], + generic: 0, + }; + obj.base_mana_cost = obj.mana_cost.clone(); + } + add_mana(&mut state, PlayerId(0), ManaType::Black, 1); + + // Grizzly Bears (2/2) in the OPPONENT's graveyard, so reanimation genuinely + // moves control to the caster. + let creature_id = create_object( + &mut state, + CardId(602), + PlayerId(1), + "Grizzly Bears".to_string(), + Zone::Graveyard, + ); + { + let obj = state.objects.get_mut(&creature_id).unwrap(); + obj.card_types.core_types.push(CoreType::Creature); + obj.base_card_types = obj.card_types.clone(); + obj.power = Some(2); + obj.toughness = Some(2); + obj.base_power = Some(2); + obj.base_toughness = Some(2); + } + + let mut events = Vec::new(); + let result = + handle_cast_spell(&mut state, PlayerId(0), aura_id, CardId(601), &mut events).unwrap(); + assert!( + matches!(result, WaitingFor::Priority { .. }), + "expected auto-target onto the stack; got {result:?}" + ); + assert_eq!(state.stack.len(), 1, "Aura spell must be on the stack"); + + // (1) Resolve the Aura spell → Site A/B: it resolves onto the battlefield and + // attaches to the graveyard host. + stack::resolve_top(&mut state, &mut events); + assert!( + state.battlefield.contains(&aura_id), + "Aura must resolve onto the battlefield (Site A)" + ); + + // (2) Fire the ETB reanimation trigger through the real trigger pipeline. + crate::game::triggers::process_triggers(&mut state, &events); + assert_eq!( + state.stack.len(), + 1, + "the reanimator ETB trigger must be on the stack after process_triggers" + ); + + // (3) Resolve the 4-node reanimation chain + // (ChangeZone -> GenericEffect keyword-swap -> Attach -> CreateDelayedTrigger). + let mut etb_events = Vec::new(); + stack::resolve_top(&mut state, &mut etb_events); + crate::game::layers::evaluate_layers(&mut state); + + (state, aura_id, creature_id) +} + +/// CR 608.2c + CR 613.1f + CR 701.3a regression (issue #4767, "Animate Dead don't +/// bring back the creature to the battlefield"): the ETB reanimation chain must +/// move the enchanted creature card out of the graveyard onto the battlefield +/// under the caster's control, re-attach the Aura to it, swap the Aura's Enchant +/// restriction so the Aura is not immediately re-graveyarded by SBAs (CR 704.5m), +/// and apply the -1/-0 static. +/// +/// LIVE-REVERT EVIDENCE: commenting out the `try_parse_reanimator_aura_etb_effect` +/// dispatch in `dispatch_reanimator_aura_etb` (parser) leaves the ETB body as an +/// `Effect::Unimplemented`, so the reanimation ChangeZone never runs and assertion +/// (a) fails (creature stays in the graveyard). +#[test] +fn animate_dead_full_pipeline_reanimates_and_reattaches() { + use crate::game::game_object::AttachTarget; + + let (mut state, aura_id, creature_id) = reanimate_grizzly_via_animate_dead(); + + // (a) The creature was reanimated: it left the graveyard for the battlefield. + assert_eq!( + state.objects[&creature_id].zone, + Zone::Battlefield, + "reanimated creature must be on the battlefield, not the graveyard" + ); + assert!( + state.battlefield.contains(&creature_id), + "battlefield must contain the reanimated creature" + ); + assert!( + !state.players[1].graveyard.contains(&creature_id), + "reanimated creature must no longer be in its owner's graveyard" + ); + + // (b) Under the CASTER's control (CR: return ... under your control), not the + // owner's. + assert_eq!( + state.objects[&creature_id].controller, + PlayerId(0), + "reanimated creature must be controlled by the caster" + ); + + // (c) The Aura is attached to the SPECIFIC reanimated creature (direction + // matters — not the creature attached to the Aura). + assert_eq!( + state.objects[&aura_id].attached_to, + Some(AttachTarget::Object(creature_id)), + "Aura must be attached to the reanimated creature" + ); + + // (d) The keyword swap re-targeted the Aura's Enchant restriction, so an + // explicit SBA pass does NOT re-graveyard the Aura (CR 704.5m). + let mut sba_events = Vec::new(); + crate::game::sba::check_state_based_actions(&mut state, &mut sba_events); + assert!( + state.battlefield.contains(&aura_id), + "Aura must survive SBAs (keyword swap re-targeted its Enchant restriction)" + ); + assert_eq!( + state.objects[&aura_id].attached_to, + Some(AttachTarget::Object(creature_id)), + "Aura must stay attached to the reanimated creature after SBAs" + ); + assert_eq!( + state.objects[&creature_id].zone, + Zone::Battlefield, + "reanimated creature must remain on the battlefield after SBAs" + ); + + // (e) The -1/-0 static is applied (base 2/2 -> 1/2). + assert_eq!( + state.objects[&creature_id].power, + Some(1), + "reanimated creature must have -1/-0 applied (2 -> 1 power)" + ); + assert_eq!( + state.objects[&creature_id].toughness, + Some(2), + "reanimated creature toughness unchanged by -1/-0" + ); +} + +/// CR 701.21a + CR 603.7c regression (issue #4767): the delayed "When ~ leaves the +/// battlefield, that creature's controller sacrifices it" trigger must sacrifice +/// the reanimated creature when the Aura leaves the battlefield. +/// +/// LIVE-REVERT EVIDENCE: removing the `CreateDelayedTrigger` node from +/// `build_reanimator_aura_etb_chain` leaves no delayed trigger, so destroying the +/// Aura no longer puts a sacrifice ability on the stack and the final assertion +/// fails (creature stays on the battlefield). +#[test] +fn animate_dead_delayed_sacrifice_when_aura_leaves() { + let (mut state, aura_id, creature_id) = reanimate_grizzly_via_animate_dead(); + // Baseline reach-guard: the creature is on the battlefield before we remove + // the Aura, so the sacrifice assertion below is not vacuous. + assert_eq!(state.objects[&creature_id].zone, Zone::Battlefield); + + // Remove the Aura from the battlefield → fires the delayed leaves-play trigger. + let mut events = Vec::new(); + zones::move_to_zone(&mut state, aura_id, Zone::Graveyard, &mut events); + crate::game::triggers::check_delayed_triggers(&mut state, &events); + assert_eq!( + state.stack.len(), + 1, + "the delayed leaves-battlefield sacrifice must be on the stack" + ); + + // Resolve the sacrifice. + let mut sac_events = Vec::new(); + stack::resolve_top(&mut state, &mut sac_events); + + assert!( + !state.battlefield.contains(&creature_id), + "reanimated creature must be sacrificed when the Aura leaves the battlefield" + ); + assert_eq!( + state.objects[&creature_id].zone, + Zone::Graveyard, + "sacrificed creature must go to its owner's graveyard" + ); +} + +/// CR 701.21a regression (issue #4767, `sacrifice.rs` controller-scope relaxation): +/// "that creature's controller sacrifices it" must be performed by the creature's +/// CURRENT controller even if control changed after Animate Dead reanimated it and +/// before the delayed leaves-battlefield trigger fires. +/// +/// LIVE-REVERT EVIDENCE: restoring the unconditional `if obj.controller != +/// ability.controller { continue; }` guard in `sacrifice.rs` makes this fail — +/// the delayed trigger's controller (the Aura's controller, P0) no longer matches +/// the creature's new controller (P1), so the `continue` skips the sacrifice and +/// the creature stays on the battlefield. +#[test] +fn animate_dead_delayed_sacrifice_follows_new_controller() { + let (mut state, aura_id, creature_id) = reanimate_grizzly_via_animate_dead(); + + // Reanimated under the caster (P0). Transfer control to the OTHER player (P1) + // AFTER reanimation but BEFORE the Aura leaves — the delayed trigger was + // created while P0 controlled the creature. + assert_eq!( + state.objects[&creature_id].controller, + PlayerId(0), + "precondition: creature reanimated under the caster" + ); + { + let obj = state.objects.get_mut(&creature_id).unwrap(); + obj.controller = PlayerId(1); + obj.base_controller = Some(PlayerId(1)); + } + + // Remove the Aura → fire + resolve the delayed sacrifice. + let mut events = Vec::new(); + zones::move_to_zone(&mut state, aura_id, Zone::Graveyard, &mut events); + crate::game::triggers::check_delayed_triggers(&mut state, &events); + assert_eq!( + state.stack.len(), + 1, + "the delayed leaves-battlefield sacrifice must be on the stack" + ); + let mut sac_events = Vec::new(); + stack::resolve_top(&mut state, &mut sac_events); + + // Sacrificed by its NEW/current controller (P1), even though the delayed + // trigger's controller is P0. + assert!( + !state.battlefield.contains(&creature_id), + "creature must be sacrificed by its current controller after control change" + ); + assert_eq!( + state.objects[&creature_id].zone, + Zone::Graveyard, + "sacrificed creature must go to its owner's graveyard" + ); +} + /// CR 702.103b regression: drives the full cast pipeline end-to-end — /// `handle_cast_spell` → `AlternativeCastChoice(Bestow)` → /// `handle_bestow_cost_choice` (Alternative) — and asserts the spell on the stack still has the diff --git a/crates/engine/src/game/cost_payability.rs b/crates/engine/src/game/cost_payability.rs index ccca208204..e56b9eb31a 100644 --- a/crates/engine/src/game/cost_payability.rs +++ b/crates/engine/src/game/cost_payability.rs @@ -98,6 +98,8 @@ pub(crate) fn target_filter_has_pitch_bound_x(filter: &TargetFilter) -> bool { | TargetFilter::Owner // CR 201.5a: a granter self-ref carries no pitch-bound X. | TargetFilter::GrantingObject + // CR 608.2c: source-relative object ref carries no pitch-bound X. + | TargetFilter::OriginalSource | TargetFilter::AllPlayers => false, } } @@ -174,6 +176,8 @@ pub(crate) fn relax_pitch_bound_x_filter(filter: &TargetFilter) -> TargetFilter | TargetFilter::Owner // CR 201.5a: no pitch-bound X constraint to relax. | TargetFilter::GrantingObject + // CR 608.2c: source-relative object ref — nothing to relax. + | TargetFilter::OriginalSource | TargetFilter::AllPlayers => filter.clone(), } } diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 60bde2d01a..a3bc96b370 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -545,6 +545,8 @@ fn fmt_target(filter: &TargetFilter) -> String { TargetFilter::SelfRef => "self".into(), // CR 201.5a: a granted body's by-name reference to its granting object. TargetFilter::GrantingObject => "granting object".into(), + // CR 608.2c: the ability's pre-rebind source (reanimator-Aura keyword swap). + TargetFilter::OriginalSource => "original source".into(), TargetFilter::SourceOrPaired => "source or paired creature".into(), TargetFilter::ExiledCardByIndex { index } => format!("exiled card {index}"), TargetFilter::StackAbility { tag: Some(tag), .. } => format!("{tag:?} ability on stack"), @@ -9315,8 +9317,12 @@ fn line_has_condition_text(lower: &str) -> Option<&'static str> { // "if it doesn't have" / "if it had no" — state check on result object || lower.contains("if it doesn't have") || lower.contains("if it had no") - // "if it's on the battlefield" — zone check at resolution - || lower.contains("if it's on the battlefield") + // NOTE (phase#4767): "if it's on the battlefield" was previously listed + // here as an unparsed gap. It is now parsed as a source-scoped + // `TriggerCondition::SourceInZone { Battlefield }` (see + // `oracle_trigger.rs::extract_if_condition_with_card_name`), so it must + // NOT be flagged as an unsupported gap any longer (Animate Dead / + // Dance of the Dead reanimator-Aura ETB trigger). // "this way" — resolve-time checks on what happened during resolution // "if you reveal a creature card this way" / "if a card is put into a graveyard this way" || lower.contains("this way") diff --git a/crates/engine/src/game/effects/delayed_trigger.rs b/crates/engine/src/game/effects/delayed_trigger.rs index 03c530eeaf..a8f0027077 100644 --- a/crates/engine/src/game/effects/delayed_trigger.rs +++ b/crates/engine/src/game/effects/delayed_trigger.rs @@ -186,14 +186,28 @@ pub fn resolve( /// tail clause carries only its own local slot, so an inner delayed /// `ParentTargetSlot { index }` anaphor pointing at an earlier slot would index /// out of range and degrade to `Any`. Flattening the root chain exposes every -/// declared slot in order so the indexed anaphor resolves. Only when the root -/// chain is empty do we fall back to the triggering source (unchanged). +/// declared slot in order so the indexed anaphor resolves. +/// +/// CR 608.2c (phase#4767): When the root chain exposes NO concrete slot — because +/// the parent target was injected at runtime by a `forward_result` zone-change +/// rather than declared as an explicit chain slot (Animate Dead / Dance of the +/// Dead: the reanimated creature is the moved object, bound into the sub-chain's +/// `targets` by `effects/mod.rs`'s forward_result block, never a declared slot) — +/// the node's OWN propagated `targets` are the resolved parent target. Prefer them +/// over the triggering-source fallback, which would otherwise snapshot the +/// triggering object (the Aura) instead of "that creature". Only when BOTH the +/// root chain and the node's own targets are empty do we fall back to the +/// triggering source (unchanged). fn parent_target_snapshot(state: &GameState, ability: &ResolvedAbility) -> Vec { let root_chain = crate::game::targeting::parent_chain_targets_from_root(state, ability); if !root_chain.is_empty() { return root_chain; } + if !ability.targets.is_empty() { + return ability.targets.clone(); + } + crate::game::targeting::resolve_event_context_target( state, &TargetFilter::TriggeringSource, diff --git a/crates/engine/src/game/effects/effect.rs b/crates/engine/src/game/effects/effect.rs index 4296f2e43e..e5e70b8c40 100644 --- a/crates/engine/src/game/effects/effect.rs +++ b/crates/engine/src/game/effects/effect.rs @@ -570,6 +570,30 @@ fn snapshot_transient_modifications( value: resolve_quantity_with_targets(state, value, ability), } } + // CR 613.1f + CR 608.2c: the reanimator-Aura's granted Enchant restriction + // ("enchant creature put onto the battlefield with this Aura") is a layer-6 + // ability grant (CR 613.1f) whose anaphoric ParentTarget must bind to the + // SPECIFIC creature just reanimated (CR 608.2c: read the whole sentence and + // bind each anaphor to its referent), not stay a live/unresolved + // ParentTarget reference — concretize it once, here, the same way dynamic + // P/T values are concretized above. + ContinuousModification::AddKeyword { + keyword: crate::types::keywords::Keyword::Enchant(filter), + } if matches!( + filter, + TargetFilter::ParentTarget | TargetFilter::ParentTargetSlot { .. } + ) => + { + let ids = + crate::game::targeting::resolved_object_ids_for_filter(state, ability, filter); + ContinuousModification::AddKeyword { + keyword: crate::types::keywords::Keyword::Enchant( + ids.first() + .map(|id| TargetFilter::SpecificObject { id: *id }) + .unwrap_or_else(|| filter.clone()), + ), + } + } _ => modification.clone(), }) .collect() diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index e51be1a1e3..4092453f60 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -7726,6 +7726,26 @@ fn resolve_chain_body( .insert(0, TargetRef::Object(forwarded_objects[0])); } } + // CR 608.2c: OriginalSource names the ability's TRUE pre-rebind source — the + // reanimator-Aura's own identity — surviving the forward_result rebind above + // that would otherwise point source_id at the just-reanimated creature + // instead of the Aura whose own keyword text this static is rewriting + // (Animate Dead / Dance of the Dead class: "it loses ... and gains ..."). + // Concretized here, eagerly, in-place — never persisted as a ResolvedAbility + // field — because this is the ONE point in the whole chain where the + // pre-rebind `ability.source_id` and the about-to-be-mutated clone coexist. + if let Effect::GenericEffect { + static_abilities, .. + } = &mut sub_with_context.effect + { + for sd in static_abilities.iter_mut() { + if sd.affected == Some(TargetFilter::OriginalSource) { + sd.affected = Some(TargetFilter::SpecificObject { + id: ability.source_id, + }); + } + } + } } apply_parent_chain_context( &mut sub_with_context, diff --git a/crates/engine/src/game/effects/sacrifice.rs b/crates/engine/src/game/effects/sacrifice.rs index 66e5715fe2..769a68ba91 100644 --- a/crates/engine/src/game/effects/sacrifice.rs +++ b/crates/engine/src/game/effects/sacrifice.rs @@ -357,7 +357,20 @@ pub fn resolve( // they control. The primary fix is that Sacrifice no longer creates // target slots (see extract_target_filter_from_effect), but if this // path is ever reached, enforce controller ownership. - if obj.controller != ability.controller { + // + // CR 701.21a: "To sacrifice a permanent, its controller moves it..." — for an + // explicit anaphoric target (ParentTarget/ParentTargetSlot, e.g. Animate + // Dead's "that creature's controller sacrifices it"), the acting player is + // the object's OWN current controller, unconditionally, even if control + // changed since the ability (e.g. a delayed leaves-battlefield trigger) was + // created. The equality check below remains a valid defense-in-depth guard + // for every OTHER filter shape reaching this path. + if obj.controller != ability.controller + && !matches!( + filter, + TargetFilter::ParentTarget | TargetFilter::ParentTargetSlot { .. } + ) + { continue; } diff --git a/crates/engine/src/game/filter.rs b/crates/engine/src/game/filter.rs index d0e9cc3536..6ea2b5e94a 100644 --- a/crates/engine/src/game/filter.rs +++ b/crates/engine/src/game/filter.rs @@ -91,6 +91,9 @@ pub(crate) fn affected_filter_uses_object_population(filter: &TargetFilter) -> b | TargetFilter::ParentTargetOwner | TargetFilter::SourceChosenPlayer | TargetFilter::OriginalController + // CR 201.5a: a fixed source-relative object ref (concretized to + // SpecificObject before runtime) — never whole-board population. + | TargetFilter::OriginalSource | TargetFilter::PostReplacementSourceController | TargetFilter::PostReplacementDamageTarget | TargetFilter::PostReplacementDamageTargetOwner @@ -304,6 +307,9 @@ pub(crate) fn entered_object_perturbs_affected_filter( | TargetFilter::ParentTargetOwner | TargetFilter::SourceChosenPlayer | TargetFilter::OriginalController + // CR 201.5a: a fixed source-relative object ref (concretized to + // SpecificObject before runtime) — never whole-board population. + | TargetFilter::OriginalSource | TargetFilter::PostReplacementSourceController | TargetFilter::PostReplacementDamageTarget | TargetFilter::PostReplacementDamageTargetOwner @@ -1522,6 +1528,10 @@ fn filter_inner_for_object( TargetFilter::SourceChosenPlayer => false, TargetFilter::ScopedPlayer => false, // ScopedPlayer is a player, not an object TargetFilter::SelfRef => object_id == source_id, + // CR 608.2c: the original (pre-rebind) source object; concretized to + // SpecificObject before runtime, so this arm is defense-in-depth and + // mirrors SelfRef's source-identity semantics. + TargetFilter::OriginalSource => object_id == source_id, TargetFilter::SourceOrPaired => state .objects .get(&source_id) @@ -2019,6 +2029,9 @@ fn zone_change_filter_inner( TargetFilter::SourceChosenPlayer => false, TargetFilter::ScopedPlayer => false, TargetFilter::SelfRef => record.object_id == source_id, + // CR 608.2c: the original (pre-rebind) source object; concretized to + // SpecificObject before runtime — mirrors SelfRef's source identity. + TargetFilter::OriginalSource => record.object_id == source_id, TargetFilter::SourceOrPaired => false, TargetFilter::Typed(TypedFilter { type_filters, @@ -2463,6 +2476,9 @@ pub fn spell_record_matches_filter( | TargetFilter::AllPlayers | TargetFilter::Controller | TargetFilter::OriginalController + // CR 201.5a: source-relative object ref, concretized to SpecificObject + // before runtime — inapplicable to a spell-cast history record. + | TargetFilter::OriginalSource | TargetFilter::ScopedPlayer | TargetFilter::SelfRef | TargetFilter::SourceOrPaired @@ -2770,6 +2786,9 @@ fn spell_object_matches_filter_inner( | TargetFilter::AllPlayers | TargetFilter::Controller | TargetFilter::OriginalController + // CR 201.5a: source-relative object ref, concretized to SpecificObject + // before runtime — inapplicable to a spell-cast history record. + | TargetFilter::OriginalSource | TargetFilter::ScopedPlayer | TargetFilter::SelfRef | TargetFilter::SourceOrPaired diff --git a/crates/engine/src/game/stack.rs b/crates/engine/src/game/stack.rs index 5e77776e8a..4e3c3846a2 100644 --- a/crates/engine/src/game/stack.rs +++ b/crates/engine/src/game/stack.rs @@ -1268,7 +1268,10 @@ pub fn resolve_top(state: &mut GameState, events: &mut Vec) { } } - // CR 303.4f: Aura resolving to battlefield attaches to its target. + // CR 608.3c: An Aura spell resolving becomes a permanent put onto the + // battlefield attached to the player or object it was targeting. + // (NOT CR 303.4f, which explicitly governs Auras entering "by any means + // other than by resolving as an Aura spell.") if spell_in_zone(state, entry.id, Zone::Battlefield) { let is_aura = state .objects @@ -1277,19 +1280,26 @@ pub fn resolve_top(state: &mut GameState, events: &mut Vec) { .unwrap_or(false); if is_aura { match spell_targets.first() { - // CR 303.4f + CR 608.2b: Object Aura — verify the target is - // still on the battlefield (last-known-information check); a - // gone target leaves the Aura unattached and SBA - // (CR 704.5m) cleans it up at the next checkpoint. + // CR 608.3c + CR 608.2b: Object Aura — verify the target is + // still a legal host per the Aura's own zone-scoped enchant + // ability (`is_valid_attachment_target`, the single legality + // authority shared with `attach::resolve` and the SBA + // re-check). A battlefield-only Enchant filter still requires + // battlefield presence; a graveyard-scoped filter (Animate + // Dead) legally accepts a graveyard host. A now-illegal target + // leaves the Aura unattached and SBA (CR 704.5m) cleans it up + // at the next checkpoint. Some(crate::types::ability::TargetRef::Object(target_id)) - if state.battlefield.contains(target_id) => + if crate::game::sba::is_valid_attachment_target( + state, entry.id, *target_id, + ) => { effects::attach::attach_to(state, entry.id, *target_id); } Some(crate::types::ability::TargetRef::Object(_)) => { - // Target left the battlefield — SBA cleanup follows. + // Target is no longer a legal host — SBA cleanup follows. } - // CR 303.4f + CR 702.5d: Player Aura (Curse cycle, Faith's + // CR 608.3c + CR 702.5d: Player Aura (Curse cycle, Faith's // Fetters-class). Validity check is "player still in game" // — `attach_to_player` makes no liveness check itself, but // `check_unattached_auras` (CR 303.4c) will detach + grave diff --git a/crates/engine/src/game/trigger_matchers.rs b/crates/engine/src/game/trigger_matchers.rs index e2e0aea9b9..3190e51a3f 100644 --- a/crates/engine/src/game/trigger_matchers.rs +++ b/crates/engine/src/game/trigger_matchers.rs @@ -789,6 +789,7 @@ pub(super) fn target_filter_matches_object( // CR 201.5a: a source-relative object ref, concretized to SpecificObject // before any trigger evaluates; delegates like the other object refs. | TargetFilter::GrantingObject + | TargetFilter::OriginalSource | TargetFilter::SourceOrPaired | TargetFilter::Typed(_) | TargetFilter::Not { .. } diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 7ceef4bdfb..b8ccaf8d25 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -11366,6 +11366,196 @@ pub(crate) fn try_parse_grant_graveyard_keyword_to_target( Some(AbilityDefinition::new(kind, effect)) } +/// CR 608.2c + CR 613.1f + CR 701.3a + CR 701.21a: Whole-body, fail-closed recognizer +/// for the reanimator-Aura ETB effect shared by Animate Dead and Dance of the Dead. +/// The chain it builds implements a reanimation instruction (CR 608.2c), a layer-6 +/// keyword swap (CR 613.1f), an attach (CR 701.3a), and a delayed sacrifice +/// (CR 701.21a). Note: CR 303.4f does NOT apply — the Aura is not entering the +/// battlefield "by a means other than resolving as an Aura spell"; it resolved +/// normally and this triggered ability then specifies the object to enchant. +/// +/// By the time this runs, the trigger condition ("When ~ enters") and the +/// intervening-if ("if it's on the battlefield, ") have already been peeled by +/// `oracle_trigger`, and every "this Aura" self-reference has been normalized to +/// `~`. The remaining effect body (character-for-character, lowercased) is: +/// +/// ```text +/// it loses "enchant <...>" and gains "enchant <...>" enchanted creature +/// card under your control and attach ~ to it. when ~ leaves the +/// battlefield, that creature's controller sacrifices it +/// ``` +/// +/// where `` is `return `/`put ` and `` is ` to the battlefield` +/// (untapped) or ` onto the battlefield tapped`. The quoted keyword swap is +/// structurally fixed for this card class, so the two quoted spans are only shape- +/// checked ("both present, both open with `enchant`") — never semantically parsed — +/// and a fixed `RemoveKeyword`/`AddKeyword` pair is always emitted. +/// +/// Returns the fully-constructed 4-node effect chain directly, bypassing the +/// per-clause chunker. Declines (returns `None`) unless the ENTIRE body matches, +/// so any card whose text deviates stays an honest `Effect::Unimplemented`. +/// +/// See `build_reanimator_aura_etb_chain` for why each node uses the anaphor it +/// does (source-rebind interplay with `ChangeZone { forward_result: true }`). +pub(crate) fn try_parse_reanimator_aura_etb_effect( + text: &str, + kind: AbilityKind, +) -> Option { + let stripped = super::oracle_util::strip_reminder_text(text); + let lower = stripped.to_lowercase(); + let tap_state = + super::oracle_nom::bridge::nom_parse_lower(&lower, parse_reanimator_aura_etb_body)?; + Some(build_reanimator_aura_etb_chain(kind, tap_state)) +} + +/// Shape-check one `"enchant <...>"` quoted keyword-grant span, discarding its +/// content. The swap is fixed for the card class, so only the opening `enchant` +/// and the balanced quotes are verified. +fn parse_quoted_enchant_clause(i: &str) -> OracleResult<'_, ()> { + let (i, _) = tag("\"enchant ").parse(i)?; + let (i, _) = take_until("\"").parse(i)?; + let (i, _) = tag("\"").parse(i)?; + Ok((i, ())) +} + +/// Consume the whole reanimator-Aura body, returning the entry tap state derived +/// from the destination phrase. `all_consuming` (via trailing `eof`) makes it +/// fail-closed: a partial match yields no chain. +fn parse_reanimator_aura_etb_body(i: &str) -> OracleResult<'_, crate::types::zones::EtbTapState> { + use crate::types::zones::EtbTapState; + let (i, _) = tag("it loses ").parse(i)?; + let (i, _) = parse_quoted_enchant_clause(i)?; + let (i, _) = tag(" and gains ").parse(i)?; + let (i, _) = parse_quoted_enchant_clause(i)?; + let (i, _) = space1.parse(i)?; + // CR 608.2c: verb axis — follow the written instruction; "return" (from + // graveyard) vs "put" (Dance of the Dead) both move the creature card. + let (i, _) = alt((tag("return "), tag("put "))).parse(i)?; + let (i, _) = tag("enchanted creature card").parse(i)?; + // CR 614.1 / CR 110.5b: destination axis carries the sole variable — entry tap state. + let (i, tap) = alt(( + value(EtbTapState::Unspecified, tag(" to the battlefield")), + value(EtbTapState::Tapped, tag(" onto the battlefield tapped")), + )) + .parse(i)?; + let (i, _) = tag( + " under your control and attach ~ to it. when ~ leaves the battlefield, \ + that creature's controller sacrifices it", + ) + .parse(i)?; + let (i, _) = multispace0.parse(i)?; + let (i, _) = eof.parse(i)?; + Ok((i, tap)) +} + +/// Build the reanimator-Aura ETB effect chain (see `try_parse_reanimator_aura_etb_effect`). +/// +/// Structure — ChangeZone at the ROOT, with the other three nodes nested +/// progressively deeper — is a deliberate source-rebind choice: +/// +/// * `ChangeZone { forward_result: true }` returns the enchanted creature card and +/// rebinds the sub-chain's `source_id` to that creature, so trailing anaphora +/// bind to it. It is the ONLY forwarding node in the chain. +/// * `GenericEffect` is ChangeZone's direct sub, so its `source_id` IS rebound to +/// the creature. Its keyword-swap static must therefore reference the Aura via +/// `TargetFilter::OriginalSource` (concretized to the pre-rebind source in +/// `effects/mod.rs`'s forward_result block), NOT `SelfRef`. +/// * `Attach` is `GenericEffect`'s sub, not `ChangeZone`'s. Because `GenericEffect` +/// itself never sets `forward_result`, this hop forwards no objects, so Attach's +/// `source_id` is never rebound — `attachment: SelfRef` correctly stays the Aura. +/// `target: ParentTarget` binds to the creature via `.targets` propagation +/// (CR 701.3a: the Aura attaches to the reanimated creature — the #4767 bug). +/// * `CreateDelayedTrigger` likewise keeps `SelfRef` (never rebound) so the +/// leaves-battlefield condition watches the Aura, and its `Sacrifice { ParentTarget }` +/// snapshots the creature at delayed-trigger creation time (CR 701.21a / +/// CR 603.7c: "that creature's controller sacrifices it"). +fn build_reanimator_aura_etb_chain( + kind: AbilityKind, + tap_state: crate::types::zones::EtbTapState, +) -> AbilityDefinition { + // CR 701.21a + CR 603.7c: "When ~ leaves the battlefield, that creature's + // controller sacrifices it." — delayed leaves-battlefield sacrifice of the + // reanimated creature (ParentTarget, snapshotted at creation). + let sacrifice = AbilityDefinition::new( + kind, + Effect::Sacrifice { + target: TargetFilter::ParentTarget, + count: QuantityExpr::Fixed { value: 1 }, + min_count: 0, + }, + ); + let delayed = AbilityDefinition::new( + kind, + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::WhenLeavesPlayFiltered { + filter: TargetFilter::SelfRef, + }, + effect: Box::new(sacrifice), + uses_tracked_set: false, + }, + ); + // CR 701.3a + CR 701.3b: "attach ~ to it" — the Aura (SelfRef, unrebound at + // this hop) attaches to the reanimated creature (ParentTarget). + let attach = AbilityDefinition::new( + kind, + Effect::Attach { + attachment: TargetFilter::SelfRef, + target: TargetFilter::ParentTarget, + }, + ) + .sub_ability(delayed); + // CR 613.1f + CR 611.2a: the Aura's own keyword swap ("it loses ... and + // gains ...") is a layer-6 ability-removing/adding continuous effect (CR + // 613.1f) with no stated duration, so it lasts until end of game (CR 611.2a). + // `OriginalSource` references the Aura even though this node's + // source_id is rebound to the creature by the parent ChangeZone's + // forward_result. `duration: None` — an unstated-duration continuous grant. + let generic = AbilityDefinition::new( + kind, + Effect::GenericEffect { + static_abilities: vec![StaticDefinition::continuous() + .affected(TargetFilter::OriginalSource) + .modifications(vec![ + ContinuousModification::RemoveKeyword { + keyword: Keyword::Enchant(TargetFilter::Any), + }, + ContinuousModification::AddKeyword { + keyword: Keyword::Enchant(TargetFilter::ParentTarget), + }, + ])], + duration: None, + target: None, + }, + ) + .sub_ability(attach); + // CR 608.2c + CR 400.7: ROOT — return/put the enchanted creature card to the + // battlefield under the caster's control (follow the instruction, CR 608.2c). + // The card becomes a NEW object on arrival (CR 400.7), which is why + // `forward_result` rebinds the sub-chain source to the reanimated creature; + // set explicitly (not by auto-detection). + let mut root = AbilityDefinition::new( + kind, + Effect::ChangeZone { + origin: Some(Zone::Graveyard), + destination: Zone::Battlefield, + target: TargetFilter::AttachedTo, + owner_library: false, + enter_transformed: false, + enters_under: Some(ControllerRef::You), + enter_tapped: tap_state, + enters_attacking: false, + up_to: false, + enter_with_counters: vec![], + conditional_enter_with_counters: vec![], + face_down_profile: None, + enters_modified_if: None, + }, + ) + .sub_ability(generic); + root.forward_result = true; + root +} + /// The two equalization verbs Balance / Restore Balance / Balancing Act use. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum EqualizeVerb { diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index bb53ea989a..129ed01494 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -10,7 +10,7 @@ use nom::Parser; use super::oracle_effect::{ condition_text_is_rehomeable, lower_effect_chain_ir, parse_effect_chain_ir, try_parse_each_player_copy_chosen, try_parse_exile_top_each_library_with_collection_counter, - try_parse_grant_graveyard_keyword_to_target, + try_parse_grant_graveyard_keyword_to_target, try_parse_reanimator_aura_etb_effect, }; use super::oracle_ir::context::ParseContext; use super::oracle_ir::trigger::{FirstTimeLimit, TriggerBody, TriggerIr, TriggerModifiers}; @@ -1172,6 +1172,17 @@ pub(crate) fn parse_trigger_line_with_index_ir( try_parse_each_player_copy_chosen(&effect_for_parse, AbilityKind::Spell) .map(|ability| TriggerBody::PreLowered(Box::new(ability))) }) + .or_else(|| { + // CR 608.2c + CR 613.1f + CR 701.3a + CR 701.21a: whole-body + // reanimator-Aura ETB effect (Animate Dead / Dance of the Dead) — + // "it loses ... and gains ...", return/put the enchanted creature + // card to the battlefield under your control, attach the Aura to + // it, and register the leaves-battlefield sacrifice. Fail-closed: + // declines unless the entire body matches, so a deviating card + // stays an honest Unimplemented rather than misparsing. + try_parse_reanimator_aura_etb_effect(&effect_for_parse, AbilityKind::Spell) + .map(|ability| TriggerBody::PreLowered(Box::new(ability))) + }) .or_else(|| { // CR 700.2 + CR 608.2d: Inline modal trigger body — "choose one — // mode1; or mode2" on a single line (no bullet-line modes). Grenzo, @@ -4213,6 +4224,21 @@ fn extract_if_condition_with_card_name( // CR 508.1 / CR 603.4: attacking state. ("if it's attacking", TriggerCondition::SourceIsAttacking), ("if it is attacking", TriggerCondition::SourceIsAttacking), + // CR 113.6b: source-scoped "if it's on the battlefield" — the ETB + // trigger's own source (a reanimator Aura: Animate Dead / Dance of + // the Dead) resolves only while it is still on the battlefield. + ( + "if it's on the battlefield", + TriggerCondition::SourceInZone { + zone: Zone::Battlefield, + }, + ), + ( + "if it is on the battlefield", + TriggerCondition::SourceInZone { + zone: Zone::Battlefield, + }, + ), // CR 508.1 + CR 603.4: source-scoped "if ~ attacked this turn" — // the trigger resolves only if the ability's own source creature // declared as an attacker this turn (Riders of the Mark, Taigam, diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index feae5c7f35..76de208423 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -22152,3 +22152,223 @@ fn opponent_attacks_you_with_two_or_more_dinosaurs_carries_type_filter() { ), } } + +// --- phase#4767: reanimator-Aura ETB whole-body recognizer (Animate Dead / Dance of the Dead) --- + +/// Verbatim Oracle text (Scryfall, 2026-07). The `enter_tapped` axis is the sole +/// difference in the trigger sentence: "Return ... to the battlefield" (Animate +/// Dead) vs. "Put ... onto the battlefield tapped" (Dance of the Dead). +const ANIMATE_DEAD_ORACLE: &str = "Enchant creature card in a graveyard\nWhen this Aura enters, if it's on the battlefield, it loses \"enchant creature card in a graveyard\" and gains \"enchant creature put onto the battlefield with this Aura.\" Return enchanted creature card to the battlefield under your control and attach this Aura to it. When this Aura leaves the battlefield, that creature's controller sacrifices it.\nEnchanted creature gets -1/-0."; + +const DANCE_OF_THE_DEAD_ORACLE: &str = "Enchant creature card in a graveyard\nWhen this Aura enters, if it's on the battlefield, it loses \"enchant creature card in a graveyard\" and gains \"enchant creature put onto the battlefield with this Aura.\" Put enchanted creature card onto the battlefield tapped under your control and attach this Aura to it. When this Aura leaves the battlefield, that creature's controller sacrifices it.\nEnchanted creature gets +1/+1 and doesn't untap during its controller's untap step.\nAt the beginning of the upkeep of enchanted creature's controller, that player may pay {1}{B}. If the player does, untap that creature."; + +/// SHAPE test — assert the reanimator-Aura ETB trigger lowers to the exact 4-node +/// chain (ChangeZone -> GenericEffect -> Attach -> CreateDelayedTrigger) with the +/// field values the runtime source-rebind mechanism depends on. Runtime behavior +/// is exercised separately in `casting_tests.rs`; this pins parser structure only. +fn assert_reanimator_chain(oracle: &str, card_name: &str, expect_tapped: bool) { + use crate::parser::oracle::parse_oracle_text; + use crate::types::zones::Zone; + let parsed = parse_oracle_text( + oracle, + card_name, + &[], + &["Enchantment".to_string()], + &["Aura".to_string()], + ); + let etb = parsed + .triggers + .iter() + .find(|t| t.mode == TriggerMode::ChangesZone && t.destination == Some(Zone::Battlefield)) + .unwrap_or_else(|| { + panic!( + "{card_name}: expected an enters-battlefield trigger, got {:?}", + parsed.triggers + ) + }); + let root = etb + .execute + .as_deref() + .unwrap_or_else(|| panic!("{card_name}: Enters trigger has no execute body")); + + // Zero Unimplemented anywhere in the chain (fail-closed recognizer honesty). + fn assert_no_unimplemented(def: &AbilityDefinition, card_name: &str) { + assert!( + !matches!(def.effect.as_ref(), Effect::Unimplemented { .. }), + "{card_name}: unexpected Effect::Unimplemented in chain: {:?}", + def.effect + ); + if let Some(sub) = def.sub_ability.as_deref() { + assert_no_unimplemented(sub, card_name); + } + if let Effect::CreateDelayedTrigger { effect, .. } = def.effect.as_ref() { + assert_no_unimplemented(effect, card_name); + } + } + assert_no_unimplemented(root, card_name); + + // Node 1: ChangeZone at the root, forward_result set explicitly. + assert!( + root.forward_result, + "{card_name}: root ChangeZone must set forward_result", + ); + let Effect::ChangeZone { + origin, + destination, + target, + enters_under, + enter_tapped, + .. + } = root.effect.as_ref() + else { + panic!( + "{card_name}: expected root Effect::ChangeZone, got {:?}", + root.effect + ); + }; + assert_eq!(*origin, Some(Zone::Graveyard), "{card_name}: origin"); + assert_eq!(*destination, Zone::Battlefield, "{card_name}: destination"); + assert_eq!( + *target, + TargetFilter::AttachedTo, + "{card_name}: ChangeZone target" + ); + assert_eq!( + *enters_under, + Some(ControllerRef::You), + "{card_name}: enters_under" + ); + assert_eq!( + enter_tapped.is_tapped(), + expect_tapped, + "{card_name}: enter_tapped state ({enter_tapped:?})", + ); + + // Node 2: GenericEffect keyword swap referencing OriginalSource, no duration. + let generic = root + .sub_ability + .as_deref() + .unwrap_or_else(|| panic!("{card_name}: ChangeZone has no GenericEffect sub")); + let Effect::GenericEffect { + static_abilities, + duration, + .. + } = generic.effect.as_ref() + else { + panic!( + "{card_name}: expected GenericEffect, got {:?}", + generic.effect + ); + }; + assert_eq!( + *duration, None, + "{card_name}: keyword-swap grant has no stated duration" + ); + assert_eq!( + static_abilities.len(), + 1, + "{card_name}: one keyword-swap static" + ); + let sd = &static_abilities[0]; + assert_eq!( + sd.affected, + Some(TargetFilter::OriginalSource), + "{card_name}: keyword swap must target OriginalSource (the Aura), not SelfRef", + ); + assert_eq!( + sd.modifications, + vec![ + ContinuousModification::RemoveKeyword { + keyword: Keyword::Enchant(TargetFilter::Any), + }, + ContinuousModification::AddKeyword { + keyword: Keyword::Enchant(TargetFilter::ParentTarget), + }, + ], + "{card_name}: keyword swap modifications", + ); + + // Node 3: Attach — SelfRef (the Aura) onto ParentTarget (the reanimated creature). + let attach = generic + .sub_ability + .as_deref() + .unwrap_or_else(|| panic!("{card_name}: GenericEffect has no Attach sub")); + let Effect::Attach { attachment, target } = attach.effect.as_ref() else { + panic!("{card_name}: expected Attach, got {:?}", attach.effect); + }; + assert_eq!( + *attachment, + TargetFilter::SelfRef, + "{card_name}: attach attachment" + ); + assert_eq!( + *target, + TargetFilter::ParentTarget, + "{card_name}: attach host" + ); + + // Node 4: CreateDelayedTrigger — WhenLeavesPlayFiltered{SelfRef} -> Sacrifice{ParentTarget}. + let delayed = attach + .sub_ability + .as_deref() + .unwrap_or_else(|| panic!("{card_name}: Attach has no CreateDelayedTrigger sub")); + let Effect::CreateDelayedTrigger { + condition, + effect, + uses_tracked_set, + } = delayed.effect.as_ref() + else { + panic!( + "{card_name}: expected CreateDelayedTrigger, got {:?}", + delayed.effect + ); + }; + assert!( + !uses_tracked_set, + "{card_name}: delayed trigger uses no tracked set" + ); + assert_eq!( + *condition, + DelayedTriggerCondition::WhenLeavesPlayFiltered { + filter: TargetFilter::SelfRef, + }, + "{card_name}: delayed leaves-battlefield condition on the Aura (SelfRef)", + ); + let Effect::Sacrifice { target, count, .. } = effect.effect.as_ref() else { + panic!("{card_name}: expected Sacrifice, got {:?}", effect.effect); + }; + assert_eq!( + *target, + TargetFilter::ParentTarget, + "{card_name}: sacrifice targets the creature" + ); + assert_eq!( + *count, + QuantityExpr::Fixed { value: 1 }, + "{card_name}: sacrifice count" + ); + assert!( + delayed.sub_ability.is_none(), + "{card_name}: chain ends at the delayed trigger" + ); +} + +#[test] +fn animate_dead_etb_lowers_to_reanimator_chain_untapped_4767() { + assert_reanimator_chain( + ANIMATE_DEAD_ORACLE, + "Animate Dead", + /* expect_tapped */ false, + ); +} + +#[test] +fn dance_of_the_dead_etb_lowers_to_reanimator_chain_tapped_4767() { + // Dance of the Dead's extra clauses (P/T rider, upkeep untap) must not + // prevent the isomorphic tapped chain from lowering. + assert_reanimator_chain( + DANCE_OF_THE_DEAD_ORACLE, + "Dance of the Dead", + /* expect_tapped */ true, + ); +} diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 417b0c1b80..335286a91f 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -4444,6 +4444,33 @@ pub enum TargetFilter { /// (parent's target's controller) and `PostReplacementSourceController` /// (replacement-context source's controller). OriginalController, + /// CR 613.1f + CR 608.2c: Resolves to the ability's *original*, pre-rebind + /// source identity — the object that put this ability on the stack — even + /// inside a `ChangeZone { forward_result: true }` sub-ability chain where + /// the runtime has rebound `ResolvedAbility::source_id` to the just-moved + /// object. + /// + /// Motivation (reanimator-Aura class: Animate Dead / Dance of the Dead): + /// the ETB effect returns the enchanted creature card to the battlefield + /// via `ChangeZone { forward_result: true }`, which rebinds `source_id` to + /// the reanimated creature so trailing `SelfRef`/`ParentTarget` anaphora + /// (the attach, the leaves-battlefield sacrifice) bind to that creature. + /// But the same effect ALSO rewrites the Aura's OWN enchant restriction + /// ("it loses \"enchant creature card in a graveyard\" and gains \"enchant + /// creature put onto the battlefield with this Aura\""). That keyword swap + /// must reference the Aura itself (the original source), not the rebound + /// creature — CR 613.1f governs this layer-6 keyword swap of the Aura's own + /// enchant ability (defined per CR 303.4a), and CR 608.2c requires reading + /// the whole sentence to bind each anaphor to its true referent at resolution. + /// + /// Never persisted as a `ResolvedAbility` field: it is concretized eagerly, + /// in place, to `SpecificObject { id }` in the `forward_result` block of + /// `effects/mod.rs`, at the one point where the pre-rebind `source_id` and + /// the about-to-be-mutated sub-ability clone coexist. + /// + /// Distinct from `SelfRef` (rebound to the moved object under + /// `forward_result`) and from `OriginalController` (a player reference). + OriginalSource, /// CR 615.5 + CR 609.7: Resolves to the controller of the *prevented event's* /// damage source. Used by prevention follow-up sentences such as "the source's /// controller draws cards equal to the damage prevented this way" (Swans of @@ -12819,6 +12846,10 @@ impl TargetFilter { | TargetFilter::SourceOrPaired | TargetFilter::Controller | TargetFilter::OriginalController + // CR 608.2c: the reanimator-Aura's pre-rebind source identity is + // resolved (concretized to SpecificObject) during resolution, never + // declared as a player-chosen target slot. + | TargetFilter::OriginalSource | TargetFilter::ScopedPlayer | TargetFilter::TriggeringSpellController | TargetFilter::TriggeringSpellOwner diff --git a/crates/mtgish-import/src/convert/condition.rs b/crates/mtgish-import/src/convert/condition.rs index a804ba0f1b..9acdf1a682 100644 --- a/crates/mtgish-import/src/convert/condition.rs +++ b/crates/mtgish-import/src/convert/condition.rs @@ -1062,6 +1062,7 @@ fn target_filter_variant_name(f: &TargetFilter) -> &'static str { TargetFilter::AllPlayers => "AllPlayers", TargetFilter::Controller => "Controller", TargetFilter::OriginalController => "OriginalController", + TargetFilter::OriginalSource => "OriginalSource", TargetFilter::ScopedPlayer => "ScopedPlayer", TargetFilter::SelfRef => "SelfRef", TargetFilter::GrantingObject => "GrantingObject", From eea4f8f7a0574f204b3260c6fd5ef5c420812b86 Mon Sep 17 00:00:00 2001 From: Rajah James Date: Thu, 9 Jul 2026 12:43:44 -0400 Subject: [PATCH 2/8] fix(engine): handle OriginalSource in usable_disjunctive_permission_filter Merging origin/main pulled in a new exhaustive TargetFilter match in oracle_static/restriction.rs (added after this branch was cut) that didn't know about the OriginalSource variant from this branch's #4767 fix. OriginalSource is a resolved-once identity reference, not a broad usable filter, so it belongs in the same false bucket as OriginalController. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XbgwGxbU9NHN9kou9isp8K --- crates/engine/src/parser/oracle_static/restriction.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/engine/src/parser/oracle_static/restriction.rs b/crates/engine/src/parser/oracle_static/restriction.rs index 2637ceed3d..7710a19f8f 100644 --- a/crates/engine/src/parser/oracle_static/restriction.rs +++ b/crates/engine/src/parser/oracle_static/restriction.rs @@ -1956,6 +1956,7 @@ fn usable_disjunctive_permission_filter(filter: &TargetFilter) -> bool { | TargetFilter::ParentTargetOwner | TargetFilter::SourceChosenPlayer | TargetFilter::OriginalController + | TargetFilter::OriginalSource | TargetFilter::PostReplacementSourceController | TargetFilter::PostReplacementDamageTarget | TargetFilter::PostReplacementDamageTargetOwner From 923e277999746702df63c1ba0eccc4c4baa986f9 Mon Sep 17 00:00:00 2001 From: Rajah James Date: Thu, 9 Jul 2026 14:06:38 -0400 Subject: [PATCH 3/8] fix(engine): guard conjunctive intervening-if conditions; preserve propagated Attach targets Addresses matthewevans' review on PR #5449. 1. try_extract_simple_condition matched a pattern anywhere as a substring, so a conjunctive intervening-if ("if it's on the battlefield and you control 9 or fewer creatures named ~, roll a 20-sided die." - "Name Sticker" Goblin) had only its leading half consumed, corrupting the residual "and ..." text into the effect body and dropping the RollDie effect entirely (confirmed by the PR's own coverage-parse-diff). Require the matched phrase to be immediately followed by a clause boundary (comma/period/end of string) so a conjunctive form falls through to the general condition parser instead of being half-matched. 2. validate_targets_in_chain's Effect::Attach branch reconstructed `kept` from scratch across the attachment/target loop, silently dropping any target entries beyond what this node's own two operands consume - a real hazard for a chain where an Attach node propagates extra targets through for a downstream sibling (e.g. a chained CreateDelayedTrigger reading the same ParentTarget), per Gemini's review comment. Append any un-claimed target_iter entries to kept so they pass through unchanged. Both fixes verified via live revert-and-rerun: each new test fails without its corresponding fix and passes with it restored. Full cargo test -p engine suite green (0 failures), clippy clean, parser combinator gate clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XbgwGxbU9NHN9kou9isp8K --- crates/engine/src/game/ability_utils.rs | 64 ++++++++++++++++--- crates/engine/src/parser/oracle_trigger.rs | 15 +++++ .../engine/src/parser/oracle_trigger_tests.rs | 56 ++++++++++++++++ 3 files changed, 125 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index a14f4eb493..2e975479f3 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -1472,16 +1472,16 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) - }) .collect() } else if let Effect::Attach { attachment, target } = &validated.effect { - // NOTE (phase#4767): both SelfRef and ParentTarget are excluded from needing - // a target slot here, so `kept` can end up `[]` for an Attach node whose - // chain is otherwise target-less end-to-end — harmless today only because - // stack.rs's `!original_targets.is_empty()` gate then routes such chains - // through the unvalidated branch instead of calling this function at all - // (confirmed for the reanimator-Aura Attach shape: Animate Dead / Dance of - // the Dead). A future chain combining Attach with a genuinely-targeted - // sibling node would need this branch to preserve rather than drop live - // SelfRef/ParentTarget entries — flag for separate review if that pattern - // arises. + // CR 608.2b (phase#4767 review): `attachment`/`target` context-refs + // (SelfRef, ParentTarget, ...) don't need their own target slot and + // are skipped below — but `validated.targets` can carry MORE entries + // than this Attach node's own two operands consume, propagated + // through for a downstream sibling in the chain (e.g. a + // CreateDelayedTrigger sub-ability reading the same ParentTarget). + // Only the entries this node's own filters actually claim get + // re-validated here; any remaining, un-claimed entries must pass + // through UNCHANGED rather than being silently dropped, or a + // sibling relying on them downstream loses its target. let mut kept = Vec::new(); let mut target_iter = validated.targets.iter(); for (is_attachment, filter) in [(true, attachment), (false, target)] { @@ -1503,6 +1503,7 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) - kept.push(legal); } } + kept.extend(target_iter.cloned()); kept } else if let Effect::Fight { subject, target } = &validated.effect { // CR 608.2b + CR 701.14a: Dual-fighter fights validate each chosen @@ -7137,6 +7138,49 @@ mod tests { ); } + /// CR 608.2b (phase-rs/phase#5449 review): an `Effect::Attach` node whose + /// `attachment`/`target` are both context-refs (SelfRef/ParentTarget — + /// neither needs its own target slot) must not have its `.targets` wiped + /// to `[]` when the node carries MORE entries than its own two operands + /// consume — those extra entries are propagated through for a downstream + /// sibling (e.g. a chained `CreateDelayedTrigger` reading the same + /// `ParentTarget`), not this node's own operands, and must survive + /// re-validation unchanged. + #[test] + fn validate_targets_in_chain_attach_preserves_unclaimed_propagated_targets() { + let format = FormatConfig::duel_commander(); + let mut state = GameState::new(format, 2, 2); + let creature = create_object( + &mut state, + CardId(0), + PlayerId(1), + "Grizzly Bears".to_string(), + Zone::Battlefield, + ); + + // Attach{SelfRef, ParentTarget} — neither operand needs a slot — but + // `.targets` carries the propagated creature id for a downstream + // sibling, not for this node's own attachment/target resolution. + let ability = ResolvedAbility::new( + Effect::Attach { + attachment: TargetFilter::SelfRef, + target: TargetFilter::ParentTarget, + }, + vec![TargetRef::Object(creature)], + ObjectId(99), + PlayerId(0), + ); + + let validated = validate_targets_in_chain(&state, &ability); + assert_eq!( + validated.targets, + vec![TargetRef::Object(creature)], + "an Attach node's un-claimed propagated targets must pass through \ + re-validation unchanged, not be dropped just because neither of \ + this node's own operands needed a target slot" + ); + } + /// CR 608.2c + CR 608.2h + CR 704.5d (issue #1582): Recoil reads "Return /// target permanent to its owner's hand. Then that player discards a card." /// When the bounced permanent is a token, it ceases to exist as a diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 2e8ce73d8e..c9ae5b1d90 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -5578,6 +5578,16 @@ fn try_extract_cast_variant_paid_condition( /// /// For source-referential conditions that cannot be `StaticCondition`s and don't need /// dynamic parsing — just a fixed pattern mapping to a fixed `TriggerCondition`. +/// +/// CR 603.4 + CR 608.2c: a pattern here must match the WHOLE condition clause, +/// not just a prefix of it — a conjunctive condition ("if it's on the +/// battlefield and you control 9 or fewer creatures, ...", "Name Sticker" +/// Goblin) has its own composite meaning that this table cannot express. +/// Matching only the leading half and treating the residual "and ..." text as +/// part of the effect body silently corrupts the trigger (phase-rs/phase#5449 +/// review). Require the match to be immediately followed by a clause boundary +/// (`,`/`.`/end of string) so a conjunctive form falls through to the general +/// condition parser instead. fn try_extract_simple_condition( tp: &TextPair<'_>, text: &str, @@ -5585,6 +5595,11 @@ fn try_extract_simple_condition( ) -> Option<(String, Option)> { for (pattern, condition) in patterns { if let Some(pos) = tp.find(pattern) { + let after = &tp.lower[pos + pattern.len()..]; + let is_whole_clause = after.is_empty() || after.starts_with([',', '.']); + if !is_whole_clause { + continue; + } return Some(( strip_condition_clause(text, pos, pattern.len()), Some(condition.clone()), diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 2ba71f0612..402bb3b5f5 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -627,6 +627,62 @@ fn trigger_etb_self_from_anywhere_other_than_graveyard_or_exile() { assert!(def.execute.is_some()); } +// CR 603.4 (phase-rs/phase#5449 review regression): a conjunctive +// intervening-if ("if it's on the battlefield AND you control 9 or fewer +// creatures named ~, roll a 20-sided die.") must not be corrupted by the +// simple pattern table's "if it's on the battlefield" entry (added for the +// reanimator-Aura class in the same PR) matching only the leading half and +// feeding the residual "and you control..." text into effect parsing. +// `try_extract_simple_condition` now requires the matched phrase to be +// immediately followed by a clause boundary (`,`/`.`/end), so a compound +// condition like this one correctly declines the simple-table match and +// falls through — the RollDie effect must parse fully intact, with no +// stray fragment leaking from the un-consumed condition tail. +#[test] +fn trigger_conjunctive_battlefield_condition_does_not_corrupt_roll_die_effect() { + let def = parse_trigger_line( + "When this creature enters from anywhere other than a graveyard or exile, \ + if it's on the battlefield and you control 9 or fewer creatures named \ + \"Name Sticker\" Goblin, roll a 20-sided die.", + "\"Name Sticker\" Goblin", + ); + let Some(execute) = &def.execute else { + panic!("expected a RollDie execute ability, got None"); + }; + assert_eq!( + *execute.effect, + Effect::RollDie { + count: QuantityExpr::Fixed { value: 1 }, + sides: 20, + results: vec![], + modifier: None, + }, + "the conjunctive condition's residual \"and you control...\" text must not \ + leak into the effect body and corrupt the RollDie parse" + ); +} + +// Sibling: Animate Dead's own genuine (non-conjunctive) "if it's on the +// battlefield" condition, immediately followed by a comma, must still match +// the simple-table entry — the boundary check must not be so strict that it +// breaks the reanimator-Aura class this entry exists for. +#[test] +fn trigger_bare_battlefield_condition_still_matches_simple_table() { + let def = parse_trigger_line( + "When this Aura enters, if it's on the battlefield, it loses \ + \"enchant creature card in a graveyard\" and gains \"enchant creature \ + put onto the battlefield with this Aura.\" Return enchanted creature \ + card to the battlefield under your control and attach this Aura to it. \ + When this Aura leaves the battlefield, that creature's controller \ + sacrifices it.", + "Animate Dead", + ); + assert!( + def.execute.is_some(), + "the reanimator-Aura ETB effect must still parse via the whole-body recognizer" + ); +} + /// CR 603.10a + CR 603.4 (issue #4521): Kishla Skimmer — "Whenever a card /// leaves your graveyard during your turn, draw a card. This ability /// triggers only once each turn." The singular leaves-a-graveyard form must From 0ebea2c1bab580a62ebcf41019c1a88c9994bd8f Mon Sep 17 00:00:00 2001 From: Rajah James Date: Thu, 9 Jul 2026 15:01:49 -0400 Subject: [PATCH 4/8] fix(engine): map "attacking the player with the most life" to Dethrone's typed condition Addresses matthewevans' second review comment on PR #5449. The clause-boundary fix in the prior commit correctly stopped "if it's attacking the player with the most life or tied for most life" (Scourge of the Throne) from matching the bare "if it's attacking" simple-table entry, but that left the condition dropping to None entirely -- which evaluates as unconditionally true, making the ability's UntapAll + extra combat phase fire regardless of who's being attacked. That's a real gameplay regression, not an honest gap: strictly worse than the original bug, which merely failed to do anything useful. CR 702.105a's "attacks the player with the most life or tied for most life" is not novel -- Dethrone's own synthesized trigger (build_dethrone_trigger, database/synthesis.rs) already models it as TriggerCondition::QuantityComparison(DefendingPlayer LifeTotal >= AllPlayers::Max LifeTotal). Added a nom-based recognizer (scan_preceded + tag/alt, ordered before the plain "if it's attacking" entry so the longer phrase is never partially shadowed) that maps the intervening-if phrasing to the identical typed condition, so a card expressing this as plain trigger text gets the same correct behavior as one using the Dethrone keyword. Verified via live revert-and-rerun: the new test fails without the recognizer and passes with it restored. Full cargo test -p engine green, clippy clean, parser combinator gate clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XbgwGxbU9NHN9kou9isp8K --- crates/engine/src/parser/oracle_trigger.rs | 46 ++++++++++++++- .../engine/src/parser/oracle_trigger_tests.rs | 56 +++++++++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index c9ae5b1d90..d7c3d4e108 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -38,7 +38,7 @@ use super::oracle_util::{ use crate::parser::oracle_ir::diagnostic::OracleDiagnostic; use crate::types::ability::ManaProduction; use crate::types::ability::{ - AbilityCost, AbilityDefinition, AbilityKind, AbilityTag, AttachmentKind, + AbilityCost, AbilityDefinition, AbilityKind, AbilityTag, AggregateFunction, AttachmentKind, AttackersDeclaredCountSubject, CastManaObjectScope, CastManaSpentMetric, CastVariantPaid, CoinFlipResult, Comparator, ControllerRef, CountScope, CounterTriggerFilter, DamageKindFilter, DestinationConstraint, DieResultFilter, Effect, FilterProp, ObjectScope, OriginConstraint, @@ -4197,6 +4197,50 @@ fn extract_if_condition_with_card_name( ); } + // CR 603.4 + CR 702.105a: "if it's/is attacking the player with the most + // life or tied for most life" — the same defending-player-life-total + // predicate Dethrone's own synthesized trigger uses (`build_dethrone_trigger`, + // database/synthesis.rs), reused here so a card phrasing the identical CR + // 702.105a idiom as a plain intervening-if (Scourge of the Throne: "if + // it's attacking the player with the most life or tied for most life, + // untap all attacking creatures") gets the SAME typed condition instead of + // either the wrong bare `SourceIsAttacking` (dropping the life-total + // qualifier entirely — the phase-rs/phase#5449 review's second finding) or + // an unconditional `None` (which would make the ability fire regardless of + // who's being attacked — strictly worse than the original bug). Ordered + // before the plain "if it's attacking" simple-table entry so the longer, + // more specific phrase is never partially shadowed by the shorter one. + if let Some((before, _, rest)) = scan_preceded(&lower, |i| { + ( + tag::<_, _, OracleError<'_>>("if it"), + alt((tag("'s"), tag(" is"))), + tag(" attacking the player with the most life or tied for most life"), + ) + .parse(i) + }) { + let pat_start = before.len(); + let clause_len = lower.len() - before.len() - rest.len(); + return ( + strip_condition_clause(text, pat_start, clause_len), + Some(TriggerCondition::QuantityComparison { + lhs: QuantityExpr::Ref { + qty: QuantityRef::LifeTotal { + player: PlayerScope::DefendingPlayer, + }, + }, + comparator: Comparator::GE, + rhs: QuantityExpr::Ref { + qty: QuantityRef::LifeTotal { + player: PlayerScope::AllPlayers { + aggregate: AggregateFunction::Max, + exclude: None, + }, + }, + }, + }), + ); + } + // CR 603.4 + CR 702.138b: "unless it escaped" — the trigger fires unless // the source permanent was cast from a graveyard with its escape ability. // Phlage, Titan of Fire's Fury: "sacrifice it unless it escaped." The diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 402bb3b5f5..0929ef3bde 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -662,6 +662,62 @@ fn trigger_conjunctive_battlefield_condition_does_not_corrupt_roll_die_effect() ); } +// CR 603.4 + CR 702.105a (phase-rs/phase#5449 review, second finding): +// Scourge of the Throne's "if it's attacking the player with the most life or +// tied for most life" must map to the SAME typed QuantityComparison condition +// Dethrone's own synthesized trigger uses (build_dethrone_trigger, +// database/synthesis.rs) — not the wrong bare SourceIsAttacking (dropping the +// life-total qualifier, the original regression) and not an unconditional +// None (which would make UntapAll + the extra combat phase fire regardless of +// who's being attacked — strictly worse than the original bug, since the +// ability would then work "too well" under the wrong conditions). +#[test] +fn trigger_attacking_highest_life_player_condition_maps_to_dethrone_shape() { + let def = parse_trigger_line( + "Whenever this creature attacks for the first time each turn, if it's \ + attacking the player with the most life or tied for most life, untap \ + all attacking creatures.", + "Scourge of the Throne", + ); + assert_eq!( + def.condition, + Some(TriggerCondition::QuantityComparison { + lhs: QuantityExpr::Ref { + qty: QuantityRef::LifeTotal { + player: PlayerScope::DefendingPlayer, + }, + }, + comparator: Comparator::GE, + rhs: QuantityExpr::Ref { + qty: QuantityRef::LifeTotal { + player: PlayerScope::AllPlayers { + aggregate: AggregateFunction::Max, + exclude: None, + }, + }, + }, + }), + "must map to the same typed life-total comparison Dethrone uses, not \ + a bare SourceIsAttacking and not an unconditional None" + ); + let Some(execute) = &def.execute else { + panic!("expected an UntapAll execute ability, got None"); + }; + assert_eq!( + *execute.effect, + Effect::SetTapState { + target: TargetFilter::Typed(TypedFilter { + type_filters: vec![TypeFilter::Creature], + controller: None, + properties: vec![FilterProp::Attacking { defender: None }], + }), + scope: EffectScope::All, + state: TapStateChange::Untap, + }, + "the effect body must not be corrupted by the intervening-if extraction" + ); +} + // Sibling: Animate Dead's own genuine (non-conjunctive) "if it's on the // battlefield" condition, immediately followed by a comma, must still match // the simple-table entry — the boundary check must not be so strict that it From 8d243a83f0a4ccd9e042409a3d56a5bc9b68e314 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 9 Jul 2026 12:29:42 -0700 Subject: [PATCH 5/8] fix(PR-5449): preserve Hex source-zone trigger parsing --- crates/engine/src/parser/oracle_trigger.rs | 4 ++ .../engine/src/parser/oracle_trigger_tests.rs | 65 ++++++++++++++++++- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index d7c3d4e108..4d1cdc849f 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -5637,8 +5637,12 @@ fn try_extract_simple_condition( text: &str, patterns: &[(&str, TriggerCondition)], ) -> Option<(String, Option)> { + let first_if = tp.find("if "); // allow-noncombinator: structural first-if anchor for trigger-level intervening-if extraction for (pattern, condition) in patterns { if let Some(pos) = tp.find(pattern) { + if Some(pos) != first_if { + continue; + } let after = &tp.lower[pos + pattern.len()..]; let is_whole_clause = after.is_empty() || after.starts_with([',', '.']); if !is_whole_clause { diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 0929ef3bde..4d5af27ba9 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -9,9 +9,9 @@ use crate::types::ability::{ Comparator, ContinuousModification, ControllerRef, CountScope, DamageChannel, DamageModification, DamageSource, DelayedTriggerCondition, DiscardSelfScope, Duration, Effect, EffectScope, FilterProp, ManaContribution, ManaProduction, ManaSpendPermission, ObjectScope, - PlayerFilter, PlayerScope, PtStat, PtValue, PtValueScope, QuantityExpr, QuantityRef, - SharedQuality, TapStateChange, TargetFilter, TriggerCondition, TypeFilter, TypedFilter, - ZoneRef, + PerpetualModification, PlayerFilter, PlayerScope, PtStat, PtValue, PtValueScope, QuantityExpr, + QuantityRef, SharedQuality, TapStateChange, TargetFilter, TriggerCondition, TypeFilter, + TypedFilter, ZoneRef, }; use crate::types::counter::{CounterMatch, CounterType}; use crate::types::game_state::WaitingFor; @@ -739,6 +739,65 @@ fn trigger_bare_battlefield_condition_still_matches_simple_table() { ); } +// Digital-only Alchemy (no CR entry) + CR 113.6b (phase-rs/phase#5449 review, +// third finding): Hex's leading source-zone intervening-if is +// "if ~ is on the battlefield or in exile", and its later "Then if it's on the +// battlefield" clause is part of the effect body. The simple source-zone table +// must not scan past the leading condition and hoist the later clause instead, +// which would narrow the trigger to battlefield-only and turn the perpetual +// effect into a regular Pump. +#[test] +fn trigger_hex_companion_keeps_exile_or_battlefield_perpetual_condition() { + let def = parse_trigger_line( + "Whenever you cast an Adventure spell, if Hex, Kellan's Companion is on \ + the battlefield or in exile, it perpetually gets +1/+1. Then if it's \ + on the battlefield, exile it with a fetch counter on it.", + "Hex, Kellan's Companion", + ); + + let Some(TriggerCondition::Or { conditions }) = &def.condition else { + panic!( + "expected battlefield-or-exile source-zone condition, got {:?}", + def.condition + ); + }; + assert!( + conditions.contains(&TriggerCondition::SourceInZone { + zone: Zone::Battlefield + }), + "Hex trigger must still function from the battlefield, got {conditions:?}" + ); + assert!( + conditions.contains(&TriggerCondition::SourceInZone { zone: Zone::Exile }), + "Hex trigger must still function from exile, got {conditions:?}" + ); + assert!( + def.trigger_zones.contains(&Zone::Battlefield), + "trigger scanner must include battlefield, got {:?}", + def.trigger_zones + ); + assert!( + def.trigger_zones.contains(&Zone::Exile), + "trigger scanner must include exile, got {:?}", + def.trigger_zones + ); + + let execute = def.execute.as_deref().expect("Hex trigger execute body"); + match execute.effect.as_ref() { + Effect::ApplyPerpetual { + modification: + PerpetualModification::ModifyPowerToughness { + power_delta, + toughness_delta, + }, + .. + } => { + assert_eq!((*power_delta, *toughness_delta), (1, 1)); + } + other => panic!("expected ApplyPerpetual +1/+1, got {other:?}"), + } +} + /// CR 603.10a + CR 603.4 (issue #4521): Kishla Skimmer — "Whenever a card /// leaves your graveyard during your turn, draw a card. This ability /// triggers only once each turn." The singular leaves-a-graveyard form must From 180926058d12ee0e251629b478ff0b13498834da Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 9 Jul 2026 12:50:16 -0700 Subject: [PATCH 6/8] fix(PR-5449): correct sacrifice CR annotations --- crates/engine/src/game/casting_tests.rs | 4 +-- crates/engine/src/game/effects/sacrifice.rs | 28 +++++++++---------- crates/engine/src/parser/oracle_effect/mod.rs | 26 ++++++++--------- crates/engine/src/parser/oracle_trigger.rs | 2 +- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/crates/engine/src/game/casting_tests.rs b/crates/engine/src/game/casting_tests.rs index 695fd7f216..6c8203c46f 100644 --- a/crates/engine/src/game/casting_tests.rs +++ b/crates/engine/src/game/casting_tests.rs @@ -35925,7 +35925,7 @@ fn animate_dead_full_pipeline_reanimates_and_reattaches() { ); } -/// CR 701.21a + CR 603.7c regression (issue #4767): the delayed "When ~ leaves the +/// CR 701.17a + CR 603.7c regression (issue #4767): the delayed "When ~ leaves the /// battlefield, that creature's controller sacrifices it" trigger must sacrifice /// the reanimated creature when the Aura leaves the battlefield. /// @@ -35965,7 +35965,7 @@ fn animate_dead_delayed_sacrifice_when_aura_leaves() { ); } -/// CR 701.21a regression (issue #4767, `sacrifice.rs` controller-scope relaxation): +/// CR 701.17a regression (issue #4767, `sacrifice.rs` controller-scope relaxation): /// "that creature's controller sacrifices it" must be performed by the creature's /// CURRENT controller even if control changed after Animate Dead reanimated it and /// before the delayed leaves-battlefield trigger fires. diff --git a/crates/engine/src/game/effects/sacrifice.rs b/crates/engine/src/game/effects/sacrifice.rs index 769a68ba91..34bec046c9 100644 --- a/crates/engine/src/game/effects/sacrifice.rs +++ b/crates/engine/src/game/effects/sacrifice.rs @@ -13,12 +13,12 @@ use crate::types::zones::Zone; /// Resolve the set of players whose permanents are eligible for a sacrifice /// effect, derived from the target filter's `ControllerRef`. /// -/// CR 701.21a: A player can only sacrifice a permanent they control. +/// CR 701.17a: A player can only sacrifice a permanent they control. /// /// - `You` (or no controller clause): only the ability controller sacrifices /// (the historical default). /// - `Opponent`: each player other than the ability controller may be asked to -/// sacrifice. Per CR 701.21a, each affected player can only sacrifice their +/// sacrifice. Per CR 701.17a, each affected player can only sacrifice their /// own permanent; this resolver handles the single-opponent two-player case /// by routing both filter scope and chooser to that opponent. /// - `ScopedPlayer`: an event-context player such as the active player for @@ -127,7 +127,7 @@ fn trigger_event_scoped_player(state: &GameState, ability: &ResolvedAbility) -> }) } -/// CR 701.21a: To sacrifice a permanent, its controller moves it to its owner's graveyard. +/// CR 701.17a: To sacrifice a permanent, its controller moves it to its owner's graveyard. pub fn resolve( state: &mut GameState, ability: &ResolvedAbility, @@ -197,7 +197,7 @@ pub fn resolve( }; if targeted_objects.is_empty() { - // CR 701.21a: Derive the player(s) whose permanents are in scope from + // CR 701.17a: Derive the player(s) whose permanents are in scope from // the target filter's ControllerRef. Defaults to `[ability.controller]` // when no controller clause is present (historical "you sacrifice" // default). For `Opponent` / `TargetPlayer`, each affected player is @@ -268,7 +268,7 @@ pub fn resolve( return Ok(()); } - // CR 701.21a + CR 609.3: When the resolved count is at least the + // CR 701.17a + CR 609.3: When the resolved count is at least the // eligible pool and the sacrifice is mandatory, sacrifice every // eligible permanent — the effect does as much as possible. Fast-path // this rather than round-tripping through EffectZoneChoice. @@ -285,7 +285,7 @@ pub fn resolve( Err(_) => {} } } - // CR 701.21a + CR 603.10a + CR 608.2f: every eligible permanent was + // CR 701.17a + CR 603.10a + CR 608.2f: every eligible permanent was // sacrificed as part of the same resolution event, so co-departing // sacrifice/LTB observers (Blood Artist) observe each other. // `departed_subset` drops any permanent that didn't actually leave @@ -302,7 +302,7 @@ pub fn resolve( return Ok(()); } - // CR 701.21a: "Sacrifice N permanents" — the affected player picks + // CR 701.17a: "Sacrifice N permanents" — the affected player picks // which `count` permanents out of the eligible pool. Clamped to pool // size for safety; the branch above handles the mandatory-all case. let choice_count = count.min(eligible.len()); @@ -348,17 +348,17 @@ pub fn resolve( continue; } - // CR 701.21a: A player can't sacrifice something that isn't a permanent. + // CR 701.17a: A player can't sacrifice something that isn't a permanent. if obj.zone != Zone::Battlefield { continue; } - // CR 701.21a: Defense-in-depth — a player can only sacrifice permanents + // CR 701.17a: Defense-in-depth — a player can only sacrifice permanents // they control. The primary fix is that Sacrifice no longer creates // target slots (see extract_target_filter_from_effect), but if this // path is ever reached, enforce controller ownership. // - // CR 701.21a: "To sacrifice a permanent, its controller moves it..." — for an + // CR 701.17a: "To sacrifice a permanent, its controller moves it..." — for an // explicit anaphoric target (ParentTarget/ParentTargetSlot, e.g. Animate // Dead's "that creature's controller sacrifices it"), the acting player is // the object's OWN current controller, unconditionally, even if control @@ -583,7 +583,7 @@ mod tests { /// OTHER creature, the mandatory sacrifice auto-resolves onto it and the /// source survives. /// - /// CR 701.21a: sacrifice moves the chosen permanent to its owner's + /// CR 701.17a: sacrifice moves the chosen permanent to its owner's /// graveyard. `FilterProp::Another` is evaluated via /// `FilterContext::from_ability` (source excluded). The paired negative /// (source survives) is made non-vacuous by asserting the OTHER creature was @@ -834,7 +834,7 @@ mod tests { assert!(state.cost_payment_failed_flag); } - // CR 701.21a: When the target filter scopes sacrifice to opponents + // CR 701.17a: When the target filter scopes sacrifice to opponents // (ControllerRef::Opponent) or a target player (ControllerRef::TargetPlayer), // the affected player — not the ability controller — both provides the // eligible permanent pool and makes the choice. @@ -1095,7 +1095,7 @@ mod tests { } } - /// CR 701.21a: Even if the targeted path is reached (defense-in-depth), + /// CR 701.17a: Even if the targeted path is reached (defense-in-depth), /// sacrifice must skip permanents not controlled by the ability controller. #[test] fn targeted_path_skips_opponent_permanents() { @@ -1511,7 +1511,7 @@ mod tests { /// planeswalker with the greatest mana value among creatures and /// planeswalkers they control." /// - /// CR 202.3 + CR 608.2h + CR 701.21a: each opponent's eligible pool must + /// CR 202.3 + CR 608.2h + CR 701.17a: each opponent's eligible pool must /// be restricted to *that opponent's* permanents tied for the greatest /// mana value among their own creatures/planeswalkers — never a global /// battlefield maximum. diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 47223530cf..dbe5231022 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -7284,7 +7284,7 @@ fn parse_effect_clause_inner(text: &str, ctx: &mut ParseContext) -> ParsedEffect return parsed_clause(Effect::RingTemptsYou); } - // CR 101.4 + CR 701.21a: "For each player, you choose from among the permanents that + // CR 101.4 + CR 701.17a: "For each player, you choose from among the permanents that // player controls an artifact, a creature, ..." — Tragic Arrogance pattern where // the spell's controller chooses for all players. if let Ok((after_prefix, _)) = @@ -11430,11 +11430,11 @@ pub(crate) fn try_parse_grant_graveyard_keyword_to_target( Some(AbilityDefinition::new(kind, effect)) } -/// CR 608.2c + CR 613.1f + CR 701.3a + CR 701.21a: Whole-body, fail-closed recognizer +/// CR 608.2c + CR 613.1f + CR 701.3a + CR 701.17a: Whole-body, fail-closed recognizer /// for the reanimator-Aura ETB effect shared by Animate Dead and Dance of the Dead. /// The chain it builds implements a reanimation instruction (CR 608.2c), a layer-6 /// keyword swap (CR 613.1f), an attach (CR 701.3a), and a delayed sacrifice -/// (CR 701.21a). Note: CR 303.4f does NOT apply — the Aura is not entering the +/// (CR 701.17a). Note: CR 303.4f does NOT apply — the Aura is not entering the /// battlefield "by a means other than resolving as an Aura spell"; it resolved /// normally and this triggered ability then specifies the object to enchant. /// @@ -11531,13 +11531,13 @@ fn parse_reanimator_aura_etb_body(i: &str) -> OracleResult<'_, crate::types::zon /// (CR 701.3a: the Aura attaches to the reanimated creature — the #4767 bug). /// * `CreateDelayedTrigger` likewise keeps `SelfRef` (never rebound) so the /// leaves-battlefield condition watches the Aura, and its `Sacrifice { ParentTarget }` -/// snapshots the creature at delayed-trigger creation time (CR 701.21a / +/// snapshots the creature at delayed-trigger creation time (CR 701.17a / /// CR 603.7c: "that creature's controller sacrifices it"). fn build_reanimator_aura_etb_chain( kind: AbilityKind, tap_state: crate::types::zones::EtbTapState, ) -> AbilityDefinition { - // CR 701.21a + CR 603.7c: "When ~ leaves the battlefield, that creature's + // CR 701.17a + CR 603.7c: "When ~ leaves the battlefield, that creature's // controller sacrifices it." — delayed leaves-battlefield sacrifice of the // reanimated creature (ParentTarget, snapshotted at creation). let sacrifice = AbilityDefinition::new( @@ -11639,7 +11639,7 @@ enum EqualizeVerb { /// minimum) would mis-clamp a positive disposal count instead of yielding 0. fn balance_clause_effect(verb: EqualizeVerb, filter: TargetFilter) -> Effect { match verb { - // CR 701.21a: battlefield clause — "lands they control" / "creatures + // CR 701.17a: battlefield clause — "lands they control" / "creatures // they control". `filter` carries `controller: You`. The sacrifice // `target` and the RIGHT `ControlledByEachPlayer` Min keep `You`; the // LEFT per-player count is re-scoped to `ScopedPlayer` so it reads the @@ -12678,7 +12678,7 @@ fn thread_for_each_subject(effect: Effect, original: &str, ctx: &mut ParseContex amount, player: target, }, - // CR 115.1a/c + CR 701.21a + CR 608.2c: "Target opponent/player sacrifices + // CR 115.1a/c + CR 701.17a + CR 608.2c: "Target opponent/player sacrifices // a [typed] permanent ... for each X" (Urborg Justice, Din of the Fireherd, // Rakdos Riteknife). The for-each interception strips the dynamic count // early, so the fixed-count `inject_subject_target` Sacrifice arm never ran @@ -13461,7 +13461,7 @@ fn try_parse_verb_and_target<'a>( rem, )); } - // CR 107.1c + CR 701.21a: "sacrifice one or more / any number of + // CR 107.1c + CR 701.17a: "sacrifice one or more / any number of // " — delegate to the single variable-count-sacrifice // authority so the dynamic `UpTo(ObjectCount)` ceiling and the // matched `min_count` (1 vs 0) are produced consistently with the @@ -17401,7 +17401,7 @@ fn player_filter_as_controller_ref(filter: &TargetFilter) -> Option Some(ControllerRef::You), // CR 608.2d + CR 101.4: "any player may sacrifice … of their choice" — @@ -27084,7 +27084,7 @@ fn try_fold_token_repeat_into_count(effect: &mut Effect, qty: &QuantityExpr) -> /// [`ThisWayCause`] the tracked-set consumer should bind to (`caused_by`): /// /// - "exiled this way" → `Some(Exiled)` (CR 701.13a — exile). -/// - "sacrificed this way" → `Some(Sacrificed)` (CR 701.21a). +/// - "sacrificed this way" → `Some(Sacrificed)` (CR 701.17a). /// - "destroyed this way" → `Some(Destroyed)` (CR 701.8a). /// - "milled this way" → `Some(Milled)` (CR 701.17a). /// - "discarded this way" → `Some(Discarded)` (CR 701.9a). diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 4d1cdc849f..53246b1a76 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -1174,7 +1174,7 @@ pub(crate) fn parse_trigger_line_with_index_ir( .map(|ability| TriggerBody::PreLowered(Box::new(ability))) }) .or_else(|| { - // CR 608.2c + CR 613.1f + CR 701.3a + CR 701.21a: whole-body + // CR 608.2c + CR 613.1f + CR 701.3a + CR 701.17a: whole-body // reanimator-Aura ETB effect (Animate Dead / Dance of the Dead) — // "it loses ... and gains ...", return/put the enchanted creature // card to the battlefield under your control, attach the Aura to From 5a0078a607624ffe670a82c051940d1e7a39bbd7 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 9 Jul 2026 13:06:45 -0700 Subject: [PATCH 7/8] fix(PR-5449): restore current sacrifice CR annotations --- crates/engine/src/game/casting_tests.rs | 4 +-- crates/engine/src/game/effects/sacrifice.rs | 28 +++++++-------- crates/engine/src/parser/oracle_effect/mod.rs | 34 +++++++++---------- crates/engine/src/parser/oracle_trigger.rs | 10 +++--- 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/crates/engine/src/game/casting_tests.rs b/crates/engine/src/game/casting_tests.rs index 6c8203c46f..695fd7f216 100644 --- a/crates/engine/src/game/casting_tests.rs +++ b/crates/engine/src/game/casting_tests.rs @@ -35925,7 +35925,7 @@ fn animate_dead_full_pipeline_reanimates_and_reattaches() { ); } -/// CR 701.17a + CR 603.7c regression (issue #4767): the delayed "When ~ leaves the +/// CR 701.21a + CR 603.7c regression (issue #4767): the delayed "When ~ leaves the /// battlefield, that creature's controller sacrifices it" trigger must sacrifice /// the reanimated creature when the Aura leaves the battlefield. /// @@ -35965,7 +35965,7 @@ fn animate_dead_delayed_sacrifice_when_aura_leaves() { ); } -/// CR 701.17a regression (issue #4767, `sacrifice.rs` controller-scope relaxation): +/// CR 701.21a regression (issue #4767, `sacrifice.rs` controller-scope relaxation): /// "that creature's controller sacrifices it" must be performed by the creature's /// CURRENT controller even if control changed after Animate Dead reanimated it and /// before the delayed leaves-battlefield trigger fires. diff --git a/crates/engine/src/game/effects/sacrifice.rs b/crates/engine/src/game/effects/sacrifice.rs index 34bec046c9..769a68ba91 100644 --- a/crates/engine/src/game/effects/sacrifice.rs +++ b/crates/engine/src/game/effects/sacrifice.rs @@ -13,12 +13,12 @@ use crate::types::zones::Zone; /// Resolve the set of players whose permanents are eligible for a sacrifice /// effect, derived from the target filter's `ControllerRef`. /// -/// CR 701.17a: A player can only sacrifice a permanent they control. +/// CR 701.21a: A player can only sacrifice a permanent they control. /// /// - `You` (or no controller clause): only the ability controller sacrifices /// (the historical default). /// - `Opponent`: each player other than the ability controller may be asked to -/// sacrifice. Per CR 701.17a, each affected player can only sacrifice their +/// sacrifice. Per CR 701.21a, each affected player can only sacrifice their /// own permanent; this resolver handles the single-opponent two-player case /// by routing both filter scope and chooser to that opponent. /// - `ScopedPlayer`: an event-context player such as the active player for @@ -127,7 +127,7 @@ fn trigger_event_scoped_player(state: &GameState, ability: &ResolvedAbility) -> }) } -/// CR 701.17a: To sacrifice a permanent, its controller moves it to its owner's graveyard. +/// CR 701.21a: To sacrifice a permanent, its controller moves it to its owner's graveyard. pub fn resolve( state: &mut GameState, ability: &ResolvedAbility, @@ -197,7 +197,7 @@ pub fn resolve( }; if targeted_objects.is_empty() { - // CR 701.17a: Derive the player(s) whose permanents are in scope from + // CR 701.21a: Derive the player(s) whose permanents are in scope from // the target filter's ControllerRef. Defaults to `[ability.controller]` // when no controller clause is present (historical "you sacrifice" // default). For `Opponent` / `TargetPlayer`, each affected player is @@ -268,7 +268,7 @@ pub fn resolve( return Ok(()); } - // CR 701.17a + CR 609.3: When the resolved count is at least the + // CR 701.21a + CR 609.3: When the resolved count is at least the // eligible pool and the sacrifice is mandatory, sacrifice every // eligible permanent — the effect does as much as possible. Fast-path // this rather than round-tripping through EffectZoneChoice. @@ -285,7 +285,7 @@ pub fn resolve( Err(_) => {} } } - // CR 701.17a + CR 603.10a + CR 608.2f: every eligible permanent was + // CR 701.21a + CR 603.10a + CR 608.2f: every eligible permanent was // sacrificed as part of the same resolution event, so co-departing // sacrifice/LTB observers (Blood Artist) observe each other. // `departed_subset` drops any permanent that didn't actually leave @@ -302,7 +302,7 @@ pub fn resolve( return Ok(()); } - // CR 701.17a: "Sacrifice N permanents" — the affected player picks + // CR 701.21a: "Sacrifice N permanents" — the affected player picks // which `count` permanents out of the eligible pool. Clamped to pool // size for safety; the branch above handles the mandatory-all case. let choice_count = count.min(eligible.len()); @@ -348,17 +348,17 @@ pub fn resolve( continue; } - // CR 701.17a: A player can't sacrifice something that isn't a permanent. + // CR 701.21a: A player can't sacrifice something that isn't a permanent. if obj.zone != Zone::Battlefield { continue; } - // CR 701.17a: Defense-in-depth — a player can only sacrifice permanents + // CR 701.21a: Defense-in-depth — a player can only sacrifice permanents // they control. The primary fix is that Sacrifice no longer creates // target slots (see extract_target_filter_from_effect), but if this // path is ever reached, enforce controller ownership. // - // CR 701.17a: "To sacrifice a permanent, its controller moves it..." — for an + // CR 701.21a: "To sacrifice a permanent, its controller moves it..." — for an // explicit anaphoric target (ParentTarget/ParentTargetSlot, e.g. Animate // Dead's "that creature's controller sacrifices it"), the acting player is // the object's OWN current controller, unconditionally, even if control @@ -583,7 +583,7 @@ mod tests { /// OTHER creature, the mandatory sacrifice auto-resolves onto it and the /// source survives. /// - /// CR 701.17a: sacrifice moves the chosen permanent to its owner's + /// CR 701.21a: sacrifice moves the chosen permanent to its owner's /// graveyard. `FilterProp::Another` is evaluated via /// `FilterContext::from_ability` (source excluded). The paired negative /// (source survives) is made non-vacuous by asserting the OTHER creature was @@ -834,7 +834,7 @@ mod tests { assert!(state.cost_payment_failed_flag); } - // CR 701.17a: When the target filter scopes sacrifice to opponents + // CR 701.21a: When the target filter scopes sacrifice to opponents // (ControllerRef::Opponent) or a target player (ControllerRef::TargetPlayer), // the affected player — not the ability controller — both provides the // eligible permanent pool and makes the choice. @@ -1095,7 +1095,7 @@ mod tests { } } - /// CR 701.17a: Even if the targeted path is reached (defense-in-depth), + /// CR 701.21a: Even if the targeted path is reached (defense-in-depth), /// sacrifice must skip permanents not controlled by the ability controller. #[test] fn targeted_path_skips_opponent_permanents() { @@ -1511,7 +1511,7 @@ mod tests { /// planeswalker with the greatest mana value among creatures and /// planeswalkers they control." /// - /// CR 202.3 + CR 608.2h + CR 701.17a: each opponent's eligible pool must + /// CR 202.3 + CR 608.2h + CR 701.21a: each opponent's eligible pool must /// be restricted to *that opponent's* permanents tied for the greatest /// mana value among their own creatures/planeswalkers — never a global /// battlefield maximum. diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index dbe5231022..5dd941df99 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -7284,7 +7284,7 @@ fn parse_effect_clause_inner(text: &str, ctx: &mut ParseContext) -> ParsedEffect return parsed_clause(Effect::RingTemptsYou); } - // CR 101.4 + CR 701.17a: "For each player, you choose from among the permanents that + // CR 101.4 + CR 701.21a: "For each player, you choose from among the permanents that // player controls an artifact, a creature, ..." — Tragic Arrogance pattern where // the spell's controller chooses for all players. if let Ok((after_prefix, _)) = @@ -7646,7 +7646,7 @@ fn parse_effect_clause_inner(text: &str, ctx: &mut ParseContext) -> ParsedEffect return clause; } - // CR 121.1 + CR 701.17a: "{verb} cards equal to {quantity}" — dynamic count + // CR 121.1 + CR 701.21a: "{verb} cards equal to {quantity}" — dynamic count // from game state for draw or mill. if let Some(clause) = try_parse_equal_to_quantity_effect(tp) { return clause; @@ -9523,7 +9523,7 @@ enum EqualToQtyVerb { Draw, } -/// Parse "{verb} cards equal to {quantity_ref}" patterns (CR 121.1 + CR 701.17a). +/// Parse "{verb} cards equal to {quantity_ref}" patterns (CR 121.1 + CR 701.21a). /// /// Handles verbs whose count field is `QuantityExpr` (mill, draw). fn try_parse_equal_to_quantity_effect(tp: TextPair) -> Option { @@ -9551,7 +9551,7 @@ fn try_parse_equal_to_quantity_effect(tp: TextPair) -> Option Some(parsed_clause(Effect::Mill { count: qty, - // CR 701.17a: No subject → controller mills. + // CR 701.21a: No subject → controller mills. target: TargetFilter::Controller, destination: Zone::Graveyard, })), @@ -11430,11 +11430,11 @@ pub(crate) fn try_parse_grant_graveyard_keyword_to_target( Some(AbilityDefinition::new(kind, effect)) } -/// CR 608.2c + CR 613.1f + CR 701.3a + CR 701.17a: Whole-body, fail-closed recognizer +/// CR 608.2c + CR 613.1f + CR 701.3a + CR 701.21a: Whole-body, fail-closed recognizer /// for the reanimator-Aura ETB effect shared by Animate Dead and Dance of the Dead. /// The chain it builds implements a reanimation instruction (CR 608.2c), a layer-6 /// keyword swap (CR 613.1f), an attach (CR 701.3a), and a delayed sacrifice -/// (CR 701.17a). Note: CR 303.4f does NOT apply — the Aura is not entering the +/// (CR 701.21a). Note: CR 303.4f does NOT apply — the Aura is not entering the /// battlefield "by a means other than resolving as an Aura spell"; it resolved /// normally and this triggered ability then specifies the object to enchant. /// @@ -11531,13 +11531,13 @@ fn parse_reanimator_aura_etb_body(i: &str) -> OracleResult<'_, crate::types::zon /// (CR 701.3a: the Aura attaches to the reanimated creature — the #4767 bug). /// * `CreateDelayedTrigger` likewise keeps `SelfRef` (never rebound) so the /// leaves-battlefield condition watches the Aura, and its `Sacrifice { ParentTarget }` -/// snapshots the creature at delayed-trigger creation time (CR 701.17a / +/// snapshots the creature at delayed-trigger creation time (CR 701.21a / /// CR 603.7c: "that creature's controller sacrifices it"). fn build_reanimator_aura_etb_chain( kind: AbilityKind, tap_state: crate::types::zones::EtbTapState, ) -> AbilityDefinition { - // CR 701.17a + CR 603.7c: "When ~ leaves the battlefield, that creature's + // CR 701.21a + CR 603.7c: "When ~ leaves the battlefield, that creature's // controller sacrifices it." — delayed leaves-battlefield sacrifice of the // reanimated creature (ParentTarget, snapshotted at creation). let sacrifice = AbilityDefinition::new( @@ -11639,7 +11639,7 @@ enum EqualizeVerb { /// minimum) would mis-clamp a positive disposal count instead of yielding 0. fn balance_clause_effect(verb: EqualizeVerb, filter: TargetFilter) -> Effect { match verb { - // CR 701.17a: battlefield clause — "lands they control" / "creatures + // CR 701.21a: battlefield clause — "lands they control" / "creatures // they control". `filter` carries `controller: You`. The sacrifice // `target` and the RIGHT `ControlledByEachPlayer` Min keep `You`; the // LEFT per-player count is re-scoped to `ScopedPlayer` so it reads the @@ -12678,7 +12678,7 @@ fn thread_for_each_subject(effect: Effect, original: &str, ctx: &mut ParseContex amount, player: target, }, - // CR 115.1a/c + CR 701.17a + CR 608.2c: "Target opponent/player sacrifices + // CR 115.1a/c + CR 701.21a + CR 608.2c: "Target opponent/player sacrifices // a [typed] permanent ... for each X" (Urborg Justice, Din of the Fireherd, // Rakdos Riteknife). The for-each interception strips the dynamic count // early, so the fixed-count `inject_subject_target` Sacrifice arm never ran @@ -13461,7 +13461,7 @@ fn try_parse_verb_and_target<'a>( rem, )); } - // CR 107.1c + CR 701.17a: "sacrifice one or more / any number of + // CR 107.1c + CR 701.21a: "sacrifice one or more / any number of // " — delegate to the single variable-count-sacrifice // authority so the dynamic `UpTo(ObjectCount)` ceiling and the // matched `min_count` (1 vs 0) are produced consistently with the @@ -17401,7 +17401,7 @@ fn player_filter_as_controller_ref(filter: &TargetFilter) -> Option Some(ControllerRef::You), // CR 608.2d + CR 101.4: "any player may sacrifice … of their choice" — @@ -27084,9 +27084,9 @@ fn try_fold_token_repeat_into_count(effect: &mut Effect, qty: &QuantityExpr) -> /// [`ThisWayCause`] the tracked-set consumer should bind to (`caused_by`): /// /// - "exiled this way" → `Some(Exiled)` (CR 701.13a — exile). -/// - "sacrificed this way" → `Some(Sacrificed)` (CR 701.17a). +/// - "sacrificed this way" → `Some(Sacrificed)` (CR 701.21a). /// - "destroyed this way" → `Some(Destroyed)` (CR 701.8a). -/// - "milled this way" → `Some(Milled)` (CR 701.17a). +/// - "milled this way" → `Some(Milled)` (CR 701.21a). /// - "discarded this way" → `Some(Discarded)` (CR 701.9a). /// - "returned this way" / "put … onto the battlefield this way" → /// `Some(Returned)` (CR 608.2c + CR 400.7 — a one-shot put-onto-battlefield diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 53246b1a76..e8f73580c3 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -1174,7 +1174,7 @@ pub(crate) fn parse_trigger_line_with_index_ir( .map(|ability| TriggerBody::PreLowered(Box::new(ability))) }) .or_else(|| { - // CR 608.2c + CR 613.1f + CR 701.3a + CR 701.17a: whole-body + // CR 608.2c + CR 613.1f + CR 701.3a + CR 701.21a: whole-body // reanimator-Aura ETB effect (Animate Dead / Dance of the Dead) — // "it loses ... and gains ...", return/put the enchanted creature // card to the battlefield under your control, attach the Aura to @@ -7302,7 +7302,7 @@ enum PlayerEventVerbRoute { /// CR 603.4: Recognize a player-subject event-verb head and report which /// downstream parser handles it. ViaEvent verbs (CR 121.1 draw, CR 119.3 life, -/// CR 701.17a mill) are parsed by `try_parse_event` on `verb_rest`; +/// CR 701.21a mill) are parsed by `try_parse_event` on `verb_rest`; /// ViaPlayerTrigger verbs (CR 601.2 cast, CR 701.21 sacrifice, CR 701.9 /// discard) are parsed only by `try_parse_player_trigger`. fn parse_player_event_verb_head(input: &str) -> OracleResult<'_, PlayerEventVerbRoute> { @@ -8113,7 +8113,7 @@ fn append_trigger_condition( } } -/// CR 701.17a: Parse the milled-card filter from the predicate tail of an +/// CR 701.21a: Parse the milled-card filter from the predicate tail of an /// active-voice mill trigger ("mills **a nonland card**", "mills **one or more /// creature cards**"). Optionally consumes a leading "one or more " quantifier /// (semantically redundant — `valid_card` matching is per-object), then @@ -8962,7 +8962,7 @@ fn try_parse_event( return Some((TriggerMode::Destroyed, def)); } - // CR 701.17a: "is milled" / "are milled" — passive-voice mill trigger. + // CR 701.21a: "is milled" / "are milled" — passive-voice mill trigger. // "For a player to mill a number of cards, that player puts that many cards // from the top of their library into their graveyard." Mirrors the `is exiled` // / `is sacrificed` arms above. The passive subject (parsed by @@ -8984,7 +8984,7 @@ fn try_parse_event( return Some((TriggerMode::Milled, def)); } - // CR 701.17a + CR 603.2c: "mills " — active-voice mill trigger + // CR 701.21a + CR 603.2c: "mills " — active-voice mill trigger // ("a player mills a nonland card", "an opponent mills a nonland card", // "a player mills one or more creature cards"). Here the parsed subject is // the *milling player*, not the milled card; the milled card is the typed From 3048094c68cf8a04e20f8608e3d523bb828bb511 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 9 Jul 2026 13:07:16 -0700 Subject: [PATCH 8/8] fix(PR-5449): undo CR annotation correction --- crates/engine/src/parser/oracle_effect/mod.rs | 8 ++++---- crates/engine/src/parser/oracle_trigger.rs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 5dd941df99..47223530cf 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -7646,7 +7646,7 @@ fn parse_effect_clause_inner(text: &str, ctx: &mut ParseContext) -> ParsedEffect return clause; } - // CR 121.1 + CR 701.21a: "{verb} cards equal to {quantity}" — dynamic count + // CR 121.1 + CR 701.17a: "{verb} cards equal to {quantity}" — dynamic count // from game state for draw or mill. if let Some(clause) = try_parse_equal_to_quantity_effect(tp) { return clause; @@ -9523,7 +9523,7 @@ enum EqualToQtyVerb { Draw, } -/// Parse "{verb} cards equal to {quantity_ref}" patterns (CR 121.1 + CR 701.21a). +/// Parse "{verb} cards equal to {quantity_ref}" patterns (CR 121.1 + CR 701.17a). /// /// Handles verbs whose count field is `QuantityExpr` (mill, draw). fn try_parse_equal_to_quantity_effect(tp: TextPair) -> Option { @@ -9551,7 +9551,7 @@ fn try_parse_equal_to_quantity_effect(tp: TextPair) -> Option Some(parsed_clause(Effect::Mill { count: qty, - // CR 701.21a: No subject → controller mills. + // CR 701.17a: No subject → controller mills. target: TargetFilter::Controller, destination: Zone::Graveyard, })), @@ -27086,7 +27086,7 @@ fn try_fold_token_repeat_into_count(effect: &mut Effect, qty: &QuantityExpr) -> /// - "exiled this way" → `Some(Exiled)` (CR 701.13a — exile). /// - "sacrificed this way" → `Some(Sacrificed)` (CR 701.21a). /// - "destroyed this way" → `Some(Destroyed)` (CR 701.8a). -/// - "milled this way" → `Some(Milled)` (CR 701.21a). +/// - "milled this way" → `Some(Milled)` (CR 701.17a). /// - "discarded this way" → `Some(Discarded)` (CR 701.9a). /// - "returned this way" / "put … onto the battlefield this way" → /// `Some(Returned)` (CR 608.2c + CR 400.7 — a one-shot put-onto-battlefield diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index e8f73580c3..4d1cdc849f 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -7302,7 +7302,7 @@ enum PlayerEventVerbRoute { /// CR 603.4: Recognize a player-subject event-verb head and report which /// downstream parser handles it. ViaEvent verbs (CR 121.1 draw, CR 119.3 life, -/// CR 701.21a mill) are parsed by `try_parse_event` on `verb_rest`; +/// CR 701.17a mill) are parsed by `try_parse_event` on `verb_rest`; /// ViaPlayerTrigger verbs (CR 601.2 cast, CR 701.21 sacrifice, CR 701.9 /// discard) are parsed only by `try_parse_player_trigger`. fn parse_player_event_verb_head(input: &str) -> OracleResult<'_, PlayerEventVerbRoute> { @@ -8113,7 +8113,7 @@ fn append_trigger_condition( } } -/// CR 701.21a: Parse the milled-card filter from the predicate tail of an +/// CR 701.17a: Parse the milled-card filter from the predicate tail of an /// active-voice mill trigger ("mills **a nonland card**", "mills **one or more /// creature cards**"). Optionally consumes a leading "one or more " quantifier /// (semantically redundant — `valid_card` matching is per-object), then @@ -8962,7 +8962,7 @@ fn try_parse_event( return Some((TriggerMode::Destroyed, def)); } - // CR 701.21a: "is milled" / "are milled" — passive-voice mill trigger. + // CR 701.17a: "is milled" / "are milled" — passive-voice mill trigger. // "For a player to mill a number of cards, that player puts that many cards // from the top of their library into their graveyard." Mirrors the `is exiled` // / `is sacrificed` arms above. The passive subject (parsed by @@ -8984,7 +8984,7 @@ fn try_parse_event( return Some((TriggerMode::Milled, def)); } - // CR 701.21a + CR 603.2c: "mills " — active-voice mill trigger + // CR 701.17a + CR 603.2c: "mills " — active-voice mill trigger // ("a player mills a nonland card", "an opponent mills a nonland card", // "a player mills one or more creature cards"). Here the parsed subject is // the *milling player*, not the milled card; the milled card is the typed