From 7cdabeffdaf05e9b8ca3b5497d3e189a855c7a8b Mon Sep 17 00:00:00 2001 From: traemyn Date: Tue, 11 Aug 2026 18:22:51 -0500 Subject: [PATCH 1/5] fix(engine): preserve Memory Jar delayed end-step scope --- .../src/game/effects/delayed_trigger.rs | 148 +++++++++++++-- .../src/parser/oracle_effect/imperative.rs | 14 +- crates/engine/src/parser/oracle_effect/mod.rs | 87 +++++++-- .../engine/src/parser/oracle_effect/tests.rs | 59 ++++++ crates/engine/src/parser/oracle_target.rs | 22 +++ crates/engine/tests/integration/main.rs | 1 + .../memory_jar_delayed_end_step.rs | 172 ++++++++++++++++++ 7 files changed, 479 insertions(+), 24 deletions(-) create mode 100644 crates/engine/tests/integration/memory_jar_delayed_end_step.rs diff --git a/crates/engine/src/game/effects/delayed_trigger.rs b/crates/engine/src/game/effects/delayed_trigger.rs index 087f00d7f4..76fa5f06c8 100644 --- a/crates/engine/src/game/effects/delayed_trigger.rs +++ b/crates/engine/src/game/effects/delayed_trigger.rs @@ -993,15 +993,10 @@ fn bind_tracked_set_to_effect(effect: &mut Effect, real_id: TrackedSetId) { Effect::ChangeZoneAll { origin: _, target, .. } => { - // Resolve target filter - match target { - TargetFilter::TrackedSet { - id: TrackedSetId(0), - } - | TargetFilter::Any => { - *target = TargetFilter::TrackedSet { id: real_id }; - } - _ => {} + if matches!(target, TargetFilter::Any) { + *target = TargetFilter::TrackedSet { id: real_id }; + } else { + target.rebind_tracked_set_sentinel(real_id); } } // CR 603.7c + CR 608.2c: Pin the tracked-set sentinel `TrackedSetId(0)` to @@ -1012,8 +1007,7 @@ fn bind_tracked_set_to_effect(effect: &mut Effect, real_id: TrackedSetId) { // Maddening Imp cross-resolution collision). Reuses the existing // `TargetFilter::rebind_tracked_set_sentinel` (types/ability.rs) — the // single authority for rewriting `TrackedSet{0}`/`TrackedSetFiltered{0}` → - // concrete inside a filter (recursing And/Or/Not) — rather than open-coding - // the two-variant rewrite the `ChangeZoneAll` arm above does inline. + // concrete inside a filter (recursing And/Or/Not). Effect::DestroyAll { target, .. } => target.rebind_tracked_set_sentinel(real_id), // Upgrade ChangeZone → ChangeZoneAll: ChangeZone uses ability.targets (empty for // delayed triggers), so it would move nothing. ChangeZoneAll scans by filter. @@ -1051,8 +1045,24 @@ fn bind_tracked_set_to_effect(effect: &mut Effect, real_id: TrackedSetId) { } } +fn bind_tracked_set_to_ability_definition(ability: &mut AbilityDefinition, real_id: TrackedSetId) { + bind_tracked_set_to_effect(&mut ability.effect, real_id); + if let Effect::CreateDelayedTrigger { effect, .. } = &mut *ability.effect { + bind_tracked_set_to_ability_definition(effect, real_id); + } + if let Some(sub_ability) = ability.sub_ability.as_mut() { + bind_tracked_set_to_ability_definition(sub_ability, real_id); + } + if let Some(else_ability) = ability.else_ability.as_mut() { + bind_tracked_set_to_ability_definition(else_ability, real_id); + } +} + fn bind_tracked_set_to_ability_chain(ability: &mut ResolvedAbility, real_id: TrackedSetId) { bind_tracked_set_to_effect(&mut ability.effect, real_id); + if let Effect::CreateDelayedTrigger { effect, .. } = &mut ability.effect { + bind_tracked_set_to_ability_definition(effect, real_id); + } if let Some(sub_ability) = ability.sub_ability.as_mut() { bind_tracked_set_to_ability_chain(sub_ability, real_id); } @@ -2143,6 +2153,58 @@ mod tests { } } + #[test] + fn uses_tracked_set_rebinds_filtered_change_zone_all() { + let mut state = GameState::new_two_player(42); + state + .tracked_object_sets + .insert(TrackedSetId(1), vec![ObjectId(10)]); + state.next_tracked_set_id = 2; + + let effect_def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::ChangeZoneAll { + origin: Some(Zone::Exile), + destination: Zone::Hand, + target: TargetFilter::TrackedSetFiltered { + id: TrackedSetId(0), + filter: Box::new(TargetFilter::Any), + caused_by: None, + }, + enters_under: None, + enter_tapped: crate::types::zones::EtbTapState::Unspecified, + enter_with_counters: vec![], + face_down_profile: None, + library_position: None, + random_order: false, + }, + ); + let ability = ResolvedAbility::new( + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::AtNextPhase { phase: Phase::End }, + effect: Box::new(effect_def), + uses_tracked_set: true, + }, + vec![], + ObjectId(5), + PlayerId(0), + ); + let mut events = Vec::new(); + + resolve(&mut state, &ability, &mut events).expect("resolve must succeed"); + + let Effect::ChangeZoneAll { target, .. } = &state.delayed_triggers[0].ability.effect else { + panic!("expected delayed ChangeZoneAll effect"); + }; + assert!(matches!( + target, + TargetFilter::TrackedSetFiltered { + id: TrackedSetId(1), + .. + } + )); + } + #[test] fn uses_tracked_set_binds_sub_ability_effects() { let mut state = GameState::new_two_player(42); @@ -2209,6 +2271,70 @@ mod tests { } } + #[test] + fn uses_tracked_set_binds_nested_delayed_effects() { + let mut state = GameState::new_two_player(42); + state + .tracked_object_sets + .insert(TrackedSetId(1), vec![ObjectId(10)]); + state.next_tracked_set_id = 2; + + let nested_payload = AbilityDefinition::new( + AbilityKind::Spell, + Effect::ChangeZoneAll { + origin: Some(Zone::Exile), + destination: Zone::Hand, + target: TargetFilter::TrackedSetFiltered { + id: TrackedSetId(0), + filter: Box::new(TargetFilter::Any), + caused_by: None, + }, + enters_under: None, + enter_tapped: crate::types::zones::EtbTapState::Unspecified, + enter_with_counters: vec![], + face_down_profile: None, + library_position: None, + random_order: false, + }, + ); + let nested_delayed = AbilityDefinition::new( + AbilityKind::Spell, + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::AtNextPhase { phase: Phase::End }, + effect: Box::new(nested_payload), + uses_tracked_set: true, + }, + ); + let ability = ResolvedAbility::new( + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::AtNextPhase { phase: Phase::End }, + effect: Box::new(nested_delayed), + uses_tracked_set: true, + }, + vec![], + ObjectId(5), + PlayerId(0), + ); + let mut events = Vec::new(); + + resolve(&mut state, &ability, &mut events).expect("resolve must succeed"); + + let Effect::CreateDelayedTrigger { effect, .. } = &state.delayed_triggers[0].ability.effect + else { + panic!("expected nested delayed effect"); + }; + let Effect::ChangeZoneAll { target, .. } = &*effect.effect else { + panic!("expected nested delayed ChangeZoneAll effect"); + }; + assert!(matches!( + target, + TargetFilter::TrackedSetFiltered { + id: TrackedSetId(1), + .. + } + )); + } + #[test] fn uses_tracked_set_resolves_sentinel() { let mut state = GameState::new_two_player(42); diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index 99d6c4062f..1442ed651f 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -40,7 +40,8 @@ use crate::types::ability::{ GrantedAbilityScope, LibraryPosition, MultiTargetSpec, OutsideGameSourcePool, PlayerScope, PreventionAmount, PreventionScope, PtStat, PtValue, QuantityExpr, QuantityRef, ReassembleControlMode, SearchSelectionConstraint, StaticDefinition, StickerTicketCostPayment, - TapStateChange, TargetFilter, TargetSelectionMode, TypeFilter, TypedFilter, ZoneOwner, + TapStateChange, TargetFilter, TargetSelectionMode, ThisWayCause, TypeFilter, TypedFilter, + ZoneOwner, }; use crate::types::card_type::CoreType; use crate::types::phase::Phase; @@ -1994,7 +1995,16 @@ pub(super) fn parse_targeted_action_ast( BounceSelection::Targeted }; let is_mass = is_mass || count.is_some(); - let origin = super::infer_origin_zone(rest_lower); + let origin = super::infer_origin_zone(rest_lower).or_else(|| { + matches!( + target, + TargetFilter::TrackedSetFiltered { + caused_by: Some(ThisWayCause::Exiled), + .. + } + ) + .then_some(crate::types::zones::Zone::Exile) + }); // CR 400.7: A returned card's target filter must be scoped to its origin // zone. Without this, "return target ... card from your graveyard to the // battlefield" enumerates legal targets on the battlefield (the default diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 6ee4030bbf..2393c61576 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -16481,7 +16481,16 @@ fn try_parse_verb_and_target<'a>( if rem.is_empty() { rem = dest_remainder; } - let origin = infer_origin_zone(rest_lower); + let origin = infer_origin_zone(rest_lower).or_else(|| { + matches!( + target, + TargetFilter::TrackedSetFiltered { + caused_by: Some(ThisWayCause::Exiled), + .. + } + ) + .then_some(crate::types::zones::Zone::Exile) + }); let target = add_inferred_origin_constraints_to_target(target, origin, rest_lower); // CR 115.1: A bounce resolves at-resolution iff the Oracle text omitted // the word "target" AND the filter has a controller scope to enumerate @@ -26689,6 +26698,33 @@ fn mark_uses_tracked_set(def: &mut AbilityDefinition) { } } +fn ability_definition_uses_tracked_set(def: &AbilityDefinition) -> bool { + let mut effect = def.effect.as_ref().clone(); + let mut uses_tracked_set = false; + each_target_filter_mut(&mut effect, &mut |filter| { + uses_tracked_set |= matches!( + filter, + TargetFilter::TrackedSet { .. } | TargetFilter::TrackedSetFiltered { .. } + ); + }); + if let Effect::CreateDelayedTrigger { effect, .. } = def.effect.as_ref() { + uses_tracked_set |= ability_definition_uses_tracked_set(effect); + } + uses_tracked_set + || def + .sub_ability + .as_deref() + .is_some_and(ability_definition_uses_tracked_set) + || def + .else_ability + .as_deref() + .is_some_and(ability_definition_uses_tracked_set) + || def + .mode_abilities + .iter() + .any(ability_definition_uses_tracked_set) +} + fn tracked_set_filter() -> TargetFilter { TargetFilter::TrackedSet { id: TrackedSetId(0), @@ -27120,6 +27156,12 @@ pub(crate) fn each_quantity_expr_mut(effect: &mut Effect, f: &mut impl FnMut(&mu Effect::Discover { mana_value_limit, .. } => f(mana_value_limit), + // CR 603.7c: A delayed payload retains the player scope of the ability + // that creates it, so its nested quantity references need the same + // rewrite as direct effect fields. + Effect::CreateDelayedTrigger { effect, .. } => { + each_quantity_expr_mut(&mut effect.effect, f); + } _ => {} } } @@ -27212,6 +27254,12 @@ pub(crate) fn each_target_filter_mut(effect: &mut Effect, f: &mut impl FnMut(&mu target: Some(target), .. } => f(target), + // CR 603.7c: A delayed payload retains the player scope of the ability + // that creates it, so its nested target references need the same + // rewrite as direct effect fields. + Effect::CreateDelayedTrigger { effect, .. } => { + each_target_filter_mut(&mut effect.effect, f); + } _ => {} } } @@ -27506,6 +27554,12 @@ fn rewrite_player_scope_refs(def: &mut AbilityDefinition) { if let Some(condition) = def.condition.as_mut() { rewrite_condition(condition); } + if let Effect::CreateDelayedTrigger { + effect: delayed, .. + } = &mut *def.effect + { + rewrite_player_scope_refs(delayed); + } if let Some(sub) = def.sub_ability.as_mut() { rewrite_player_scope_refs(sub); } @@ -27529,6 +27583,12 @@ fn apply_player_scope_rewrites(def: &mut AbilityDefinition) { rewrite_player_scope_refs(def); return; } + if let Effect::CreateDelayedTrigger { + effect: delayed, .. + } = &mut *def.effect + { + apply_player_scope_rewrites(delayed); + } if let Some(sub) = def.sub_ability.as_mut() { apply_player_scope_rewrites(sub); } @@ -32183,17 +32243,11 @@ pub(crate) fn parse_effect_chain_ir( }); if let Some(prefix_condition) = prefix_delayed { let (inner_text, inner_multi_target) = strip_any_number_quantifier(text_after_prefix); - let inner_clause = parse_effect_clause(&inner_text, ctx); - let mut inner_def = AbilityDefinition::new(kind, inner_clause.effect); - if let Some(spec) = inner_multi_target.or(inner_clause.multi_target) { + let inner_ir = parse_effect_chain_ir(&inner_text, kind, ctx); + let mut inner_def = lower_effect_chain_ir(&inner_ir); + if let Some(spec) = inner_multi_target { inner_def = inner_def.multi_target(spec); } - if let Some(duration) = inner_clause.duration { - inner_def = inner_def.duration(duration); - } - if let Some(sub) = inner_clause.sub_ability { - inner_def.sub_ability = Some(sub); - } // CR 118.12a (issue #4369): a payment-unless on the delayed // instruction itself ("...next end step, sacrifice it unless you pay // {cost}" — Ashling, the Limitless; Satya, Aetherflux Genius) is @@ -32206,10 +32260,11 @@ pub(crate) fn parse_effect_chain_ir( inner_def.unless_pay = Some(up); } apply_where_x_ability_expression(&mut inner_def, where_x_expression.as_deref()); + let uses_tracked_set = ability_definition_uses_tracked_set(&inner_def); let delayed_effect = Effect::CreateDelayedTrigger { condition: prefix_condition.clone(), effect: Box::new(inner_def), - uses_tracked_set: false, + uses_tracked_set, }; let cascade_snap = super::swallow_check::CascadeSnapshot { is_optional, @@ -35214,6 +35269,16 @@ fn add_inferred_origin_constraints_to_target( let Some(zone) = origin else { return target; }; + if matches!( + target, + TargetFilter::TrackedSetFiltered { + caused_by: Some(ThisWayCause::Exiled), + .. + } + ) && zone == Zone::Exile + { + return target; + } // CR 400.7: A self-reference ("return this card from your graveyard") already // identifies one specific object, so an origin `InZone`/`Owned` constraint is // meaningless — and `add_filter_props` would corrupt it by wrapping the bare diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index 3af338ec0f..d9f727a540 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -9725,6 +9725,23 @@ fn return_leading_their_hand_all_exiled_with_source() { )); } +#[test] +fn return_each_card_they_exiled_this_way_uses_exiled_tracked_set() { + let e = parse_effect("returns to their hand each card they exiled this way"); + assert!(matches!( + e, + Effect::ChangeZoneAll { + origin: Some(Zone::Exile), + destination: Zone::Hand, + target: TargetFilter::TrackedSetFiltered { + caused_by: Some(ThisWayCause::Exiled), + .. + }, + .. + } + )); +} + /// CR 406.6 + CR 607.1/607.2a (#5577): Watcher for Tomorrow's Hideaway /// leaves-the-battlefield trigger — "put the exiled card into its owner's hand" /// — references the card THIS source exiled in a SEPARATE, earlier resolution @@ -17865,6 +17882,48 @@ fn temporal_prefix_in_effect_chain() { } } +#[test] +fn temporal_prefix_preserves_full_delayed_effect_chain() { + let def = parse_effect_chain( + "At the beginning of the next end step, each player discards their hand and returns to their hand each card they exiled this way.", + AbilityKind::Spell, + ); + let Effect::CreateDelayedTrigger { + effect, + uses_tracked_set: true, + .. + } = &*def.effect + else { + panic!("expected a tracked delayed trigger, got {:?}", def.effect); + }; + assert!(matches!( + effect.effect.as_ref(), + Effect::Discard { + count: QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::ScopedPlayer + } + }, + .. + } + )); + assert!(matches!( + effect + .sub_ability + .as_deref() + .map(|ability| ability.effect.as_ref()), + Some(Effect::ChangeZoneAll { + origin: Some(Zone::Exile), + destination: Zone::Hand, + target: TargetFilter::TrackedSetFiltered { + caused_by: Some(ThisWayCause::Exiled), + .. + }, + .. + }) + )); +} + #[test] fn non_targeted_multi_untap_chooses_at_resolution() { let def = parse_effect_chain( diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index d5d2b71bf0..d052b48d4c 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -1566,6 +1566,28 @@ pub fn parse_target_with_syntax<'a>( return (TargetFilter::ParentTarget, rest, syntax); } + // CR 608.2c + CR 603.7: "each card(s) they exiled this way" refers to + // the exiled members published by the preceding effect, not every card + // matching the generic `each card` descriptor. Preserve the producer + // action so a tracked set containing other object movements is excluded. + if let Ok((rest_lower, _)) = ( + opt(tag::<_, _, OracleError<'_>>("each ")), + alt((tag("cards"), tag("card"))), + tag(" they exiled this way"), + ) + .parse(lower.as_str()) + { + return ( + TargetFilter::TrackedSetFiltered { + id: TrackedSetId(0), + filter: Box::new(TargetFilter::Typed(TypedFilter::card())), + caused_by: Some(ThisWayCause::Exiled), + }, + &text[lower.len() - rest_lower.len()..], + syntax, + ); + } + // CR 601.2c: "each of target " is an exact-count multi-target // distribution (handled upstream by the counter.rs strip), NOT an all-matching // "each" filter. For any non-counter effect that reaches here, route the type diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index ef07130791..4af8e72e55 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -790,6 +790,7 @@ mod mauhur_swarming_of_moria; mod maze_of_ith_untap_bidirectional_prevent; mod mazemind_tome_existential_counter_state_trigger; mod mechtitan_core_return_exiled; +mod memory_jar_delayed_end_step; mod memory_plunder_free_cast_2884; mod mercenaries_any_player_activate_prevention_scope; mod merieke_ri_berit_cant_regenerate; diff --git a/crates/engine/tests/integration/memory_jar_delayed_end_step.rs b/crates/engine/tests/integration/memory_jar_delayed_end_step.rs new file mode 100644 index 0000000000..2512135f14 --- /dev/null +++ b/crates/engine/tests/integration/memory_jar_delayed_end_step.rs @@ -0,0 +1,172 @@ +//! Regression for Memory Jar's delayed per-player discard. +//! +//! The delayed end-step effect is part of the same "each player" ability as +//! the immediate hand exile and draw. Its quantity and target references must +//! retain the player being iterated when the delayed trigger resolves. + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::ability::{AbilityDefinition, Effect, QuantityExpr, QuantityRef, TargetFilter}; +use engine::types::identifiers::ObjectId; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; +use engine::types::zones::Zone; + +const MEMORY_JAR_ORACLE: &str = "{T}, Sacrifice this artifact: Each player exiles all cards from their hand face down and draws seven cards. At the beginning of the next end step, each player discards their hand and returns to their hand each card they exiled this way."; + +fn object_zone(runner: &engine::game::scenario::GameRunner, object: ObjectId) -> Zone { + runner.state().objects[&object].zone +} + +fn graveyard_contains( + runner: &engine::game::scenario::GameRunner, + player: PlayerId, + object: ObjectId, +) -> bool { + runner + .state() + .players + .iter() + .find(|candidate| candidate.id == player) + .is_some_and(|candidate| candidate.graveyard.contains(&object)) +} + +fn assert_no_unimplemented(effect: &Effect) { + match effect { + Effect::Unimplemented { name, .. } => { + panic!("Memory Jar parsed an unimplemented effect: {name}") + } + Effect::CreateDelayedTrigger { effect, .. } => { + assert_no_unimplemented_ability(effect); + } + _ => {} + } +} + +fn assert_no_unimplemented_ability(ability: &AbilityDefinition) { + assert_no_unimplemented(ability.effect.as_ref()); + if let Some(sub_ability) = ability.sub_ability.as_deref() { + assert_no_unimplemented_ability(sub_ability); + } + if let Some(else_ability) = ability.else_ability.as_deref() { + assert_no_unimplemented_ability(else_ability); + } +} + +fn find_delayed_ability(ability: &AbilityDefinition) -> Option<&AbilityDefinition> { + if matches!(ability.effect.as_ref(), Effect::CreateDelayedTrigger { .. }) { + return Some(ability); + } + ability + .sub_ability + .as_deref() + .and_then(find_delayed_ability) + .or_else(|| { + ability + .else_ability + .as_deref() + .and_then(find_delayed_ability) + }) +} + +#[test] +fn memory_jar_delayed_discard_stays_scoped_to_each_player() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + let jar = scenario + .add_creature_from_oracle(P0, "Memory Jar", 0, 0, MEMORY_JAR_ORACLE) + .as_artifact() + .id(); + + let p0_hand: Vec<_> = (0..7) + .map(|index| scenario.add_card_to_hand(P0, &format!("P0 original {index}"))) + .collect(); + let p1_hand: Vec<_> = (0..7) + .map(|index| scenario.add_card_to_hand(P1, &format!("P1 original {index}"))) + .collect(); + for index in 0..20 { + scenario.add_card_to_library_top(P0, &format!("P0 library padding {index}")); + scenario.add_card_to_library_top(P1, &format!("P1 library padding {index}")); + } + let p0_draws: Vec<_> = (0..7) + .map(|index| scenario.add_card_to_library_top(P0, &format!("P0 draw {index}"))) + .collect(); + let p1_draws: Vec<_> = (0..7) + .map(|index| scenario.add_card_to_library_top(P1, &format!("P1 draw {index}"))) + .collect(); + + let mut runner = scenario.build(); + let jar_ability = runner.state().objects[&jar].abilities[0].clone(); + assert_no_unimplemented_ability(&jar_ability); + let delayed_ability = find_delayed_ability(&jar_ability).expect("Memory Jar delayed ability"); + let Effect::CreateDelayedTrigger { + effect, + uses_tracked_set: true, + .. + } = delayed_ability.effect.as_ref() + else { + unreachable!("find_delayed_ability returned a non-delayed ability"); + }; + assert!(matches!( + effect.effect.as_ref(), + Effect::Discard { + count: QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: engine::types::ability::PlayerScope::ScopedPlayer + } + }, + .. + } + )); + let Some(return_effect) = effect.sub_ability.as_deref() else { + panic!("Memory Jar delayed return"); + }; + assert!(matches!( + return_effect.effect.as_ref(), + Effect::ChangeZoneAll { + origin: Some(Zone::Exile), + destination: Zone::Hand, + target: TargetFilter::TrackedSetFiltered { + caused_by: Some(engine::types::ability::ThisWayCause::Exiled), + .. + }, + .. + } + )); + + runner.activate(jar, 0).pay_with(&[jar]).resolve(); + + assert_eq!(runner.state().delayed_triggers.len(), 1); + for object in p0_hand.iter().chain(p1_hand.iter()) { + assert_eq!(object_zone(&runner, *object), Zone::Exile); + } + for object in p0_draws.iter().chain(p1_draws.iter()) { + assert_eq!(object_zone(&runner, *object), Zone::Hand); + } + + runner.advance_to_end_step(); + runner.advance_until_stack_empty(); + + for object in &p0_hand { + assert_eq!(object_zone(&runner, *object), Zone::Hand); + assert!(!graveyard_contains(&runner, P0, *object)); + } + for object in &p1_hand { + assert_eq!(object_zone(&runner, *object), Zone::Hand); + assert!(!graveyard_contains(&runner, P1, *object)); + } + for object in &p0_draws { + assert_eq!(object_zone(&runner, *object), Zone::Graveyard); + assert!(graveyard_contains(&runner, P0, *object)); + } + for object in &p1_draws { + assert_eq!(object_zone(&runner, *object), Zone::Graveyard); + assert!(graveyard_contains(&runner, P1, *object)); + } + + assert_eq!( + runner.state().players[0].graveyard.len(), + p0_draws.len() + 1 + ); + assert_eq!(runner.state().players[1].graveyard.len(), p1_draws.len()); +} From 98b95f601384ad7709b716d6ea677e56c92e9304 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 18:08:57 -0700 Subject: [PATCH 2/5] fix(PR-7257): preserve delayed tracked-set bindings --- .../src/game/effects/delayed_trigger.rs | 108 ++++++++++++++++-- crates/engine/src/parser/oracle_effect/mod.rs | 6 + .../engine/src/parser/oracle_effect/tests.rs | 48 +++++++- 3 files changed, 151 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/game/effects/delayed_trigger.rs b/crates/engine/src/game/effects/delayed_trigger.rs index 76fa5f06c8..00f412df48 100644 --- a/crates/engine/src/game/effects/delayed_trigger.rs +++ b/crates/engine/src/game/effects/delayed_trigger.rs @@ -1027,6 +1027,11 @@ fn bind_tracked_set_to_effect(effect: &mut Effect, real_id: TrackedSetId) { } | TargetFilter::Any => TargetFilter::TrackedSet { id: real_id }, TargetFilter::TrackedSet { id } => TargetFilter::TrackedSet { id: *id }, + TargetFilter::TrackedSetFiltered { .. } => { + let mut bound_target = target.clone(); + bound_target.rebind_tracked_set_sentinel(real_id); + bound_target + } _ => TargetFilter::TrackedSet { id: real_id }, }; *effect = Effect::ChangeZoneAll { @@ -1056,6 +1061,9 @@ fn bind_tracked_set_to_ability_definition(ability: &mut AbilityDefinition, real_ if let Some(else_ability) = ability.else_ability.as_mut() { bind_tracked_set_to_ability_definition(else_ability, real_id); } + for mode_ability in &mut ability.mode_abilities { + bind_tracked_set_to_ability_definition(mode_ability, real_id); + } } fn bind_tracked_set_to_ability_chain(ability: &mut ResolvedAbility, real_id: TrackedSetId) { @@ -2271,6 +2279,80 @@ mod tests { } } + #[test] + fn uses_tracked_set_binds_mode_ability_effects() { + let mut state = GameState::new_two_player(42); + state + .tracked_object_sets + .insert(TrackedSetId(1), vec![ObjectId(10)]); + state.next_tracked_set_id = 2; + + let mode = AbilityDefinition::new( + AbilityKind::Spell, + Effect::ChangeZoneAll { + origin: Some(Zone::Exile), + destination: Zone::Hand, + target: TargetFilter::TrackedSetFiltered { + id: TrackedSetId(0), + filter: Box::new(TargetFilter::Any), + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), + }, + enters_under: None, + enter_tapped: crate::types::zones::EtbTapState::Unspecified, + enter_with_counters: vec![], + face_down_profile: None, + library_position: None, + random_order: false, + }, + ); + let effect_def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ) + .with_modal( + crate::types::ability::ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 1, + ..Default::default() + }, + vec![mode], + ); + let ability = ResolvedAbility::new( + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::AtNextPhase { phase: Phase::End }, + effect: Box::new(effect_def), + uses_tracked_set: true, + }, + vec![], + ObjectId(5), + PlayerId(0), + ); + let mut events = Vec::new(); + + resolve(&mut state, &ability, &mut events).expect("resolve must succeed"); + + let mode = state.delayed_triggers[0] + .ability + .mode_abilities + .first() + .expect("delayed modal must retain its mode"); + assert!(matches!( + mode.effect.as_ref(), + Effect::ChangeZoneAll { + target: TargetFilter::TrackedSetFiltered { + id: TrackedSetId(1), + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), + .. + }, + .. + } + )); + } + #[test] fn uses_tracked_set_binds_nested_delayed_effects() { let mut state = GameState::new_two_player(42); @@ -2343,14 +2425,17 @@ mod tests { .insert(TrackedSetId(1), vec![ObjectId(10)]); state.next_tracked_set_id = 2; - // Parser emits ChangeZone with TrackedSetId(0) sentinel + // The ChangeZone upgrade must preserve the tracked-set filter and bind + // its sentinel, rather than dropping it to an unfiltered tracked set. let effect_def = AbilityDefinition::new( AbilityKind::Spell, Effect::ChangeZone { origin: None, destination: Zone::Battlefield, - target: TargetFilter::TrackedSet { + target: TargetFilter::TrackedSetFiltered { id: TrackedSetId(0), + filter: Box::new(TargetFilter::Any), + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), }, owner_library: false, enter_transformed: false, @@ -2379,8 +2464,9 @@ mod tests { let result = resolve(&mut state, &ability, &mut events); assert!(result.is_ok()); - // Should be upgraded to ChangeZoneAll with resolved TrackedSetId; origin - // stays unset so runtime derives member zones when firing. + // Should be upgraded to ChangeZoneAll with the filtered tracked set + // intact and its sentinel resolved; origin stays unset so runtime derives + // member zones when firing. match &state.delayed_triggers[0].ability.effect { Effect::ChangeZoneAll { origin, @@ -2390,12 +2476,14 @@ mod tests { } => { assert_eq!(*origin, None); assert_eq!(*destination, Zone::Battlefield); - assert_eq!( - *target, - TargetFilter::TrackedSet { - id: TrackedSetId(1) - } - ); + assert!(matches!( + target, + TargetFilter::TrackedSetFiltered { + id: TrackedSetId(1), + filter, + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), + } if matches!(filter.as_ref(), TargetFilter::Any) + )); } other => panic!("Expected ChangeZoneAll, got {:?}", other), } diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 2393c61576..b56868e58d 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -27566,6 +27566,9 @@ fn rewrite_player_scope_refs(def: &mut AbilityDefinition) { if let Some(else_branch) = def.else_ability.as_mut() { rewrite_player_scope_refs(else_branch); } + for mode_ability in &mut def.mode_abilities { + rewrite_player_scope_refs(mode_ability); + } } /// CR 608.2 + CR 109.5: Apply `rewrite_player_scope_refs` rooted at every def in @@ -27595,6 +27598,9 @@ fn apply_player_scope_rewrites(def: &mut AbilityDefinition) { if let Some(else_branch) = def.else_ability.as_mut() { apply_player_scope_rewrites(else_branch); } + for mode_ability in &mut def.mode_abilities { + apply_player_scope_rewrites(mode_ability); + } } /// CR 603.7c + CR 119.3 + CR 120.3: Rebind the event-bound player possessive diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index d9f727a540..2f1ac892b8 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -44,6 +44,52 @@ fn each_target_filter_mut_does_not_visit_shuffle() { ); } +#[test] +fn player_scope_rewrite_reaches_modal_mode_abilities() { + let mode = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Discard { + count: QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::Target, + }, + }, + target: TargetFilter::Controller, + }, + ); + let mut def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ) + .player_scope(PlayerFilter::All) + .with_modal( + ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 1, + ..Default::default() + }, + vec![mode], + ); + + apply_player_scope_rewrites(&mut def); + + assert!(matches!( + def.mode_abilities[0].effect.as_ref(), + Effect::Discard { + count: QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::ScopedPlayer, + }, + }, + target: TargetFilter::ScopedPlayer, + } + )); +} + #[test] fn do_the_same_type_rewrite_does_not_overwrite_logical_target_leaves() { let original = TargetFilter::And { @@ -17885,7 +17931,7 @@ fn temporal_prefix_in_effect_chain() { #[test] fn temporal_prefix_preserves_full_delayed_effect_chain() { let def = parse_effect_chain( - "At the beginning of the next end step, each player discards their hand and returns to their hand each card they exiled this way.", + "At the beginning of the next end step, each player discards their hand, then returns to their hand each card they exiled this way.", AbilityKind::Spell, ); let Effect::CreateDelayedTrigger { From b04174ce7825d6c92a0896203951a9adc670c415 Mon Sep 17 00:00:00 2001 From: traemyn Date: Tue, 11 Aug 2026 20:09:47 -0500 Subject: [PATCH 3/5] fix(engine): preserve nested delayed tracked-set filters --- .../src/game/effects/delayed_trigger.rs | 54 ++++++++++++++++++- .../engine/src/parser/oracle_effect/tests.rs | 2 +- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/game/effects/delayed_trigger.rs b/crates/engine/src/game/effects/delayed_trigger.rs index 76fa5f06c8..ac71d36698 100644 --- a/crates/engine/src/game/effects/delayed_trigger.rs +++ b/crates/engine/src/game/effects/delayed_trigger.rs @@ -1027,6 +1027,11 @@ fn bind_tracked_set_to_effect(effect: &mut Effect, real_id: TrackedSetId) { } | TargetFilter::Any => TargetFilter::TrackedSet { id: real_id }, TargetFilter::TrackedSet { id } => TargetFilter::TrackedSet { id: *id }, + TargetFilter::TrackedSetFiltered { .. } => { + let mut bound_target = (*target).clone(); + bound_target.rebind_tracked_set_sentinel(real_id); + bound_target + } _ => TargetFilter::TrackedSet { id: real_id }, }; *effect = Effect::ChangeZoneAll { @@ -1056,6 +1061,9 @@ fn bind_tracked_set_to_ability_definition(ability: &mut AbilityDefinition, real_ if let Some(else_ability) = ability.else_ability.as_mut() { bind_tracked_set_to_ability_definition(else_ability, real_id); } + for mode in &mut ability.mode_abilities { + bind_tracked_set_to_ability_definition(mode, real_id); + } } fn bind_tracked_set_to_ability_chain(ability: &mut ResolvedAbility, real_id: TrackedSetId) { @@ -1069,6 +1077,9 @@ fn bind_tracked_set_to_ability_chain(ability: &mut ResolvedAbility, real_id: Tra if let Some(else_ability) = ability.else_ability.as_mut() { bind_tracked_set_to_ability_chain(else_ability, real_id); } + for mode in &mut ability.mode_abilities { + bind_tracked_set_to_ability_definition(mode, real_id); + } } /// CR 400.7 + CR 603.7c: True when `filter` is an anaphor that names the @@ -2225,7 +2236,11 @@ mod tests { Effect::ChangeZone { origin: None, destination: Zone::Battlefield, - target: TargetFilter::Any, + target: TargetFilter::TrackedSetFiltered { + id: TrackedSetId(0), + filter: Box::new(TargetFilter::Any), + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), + }, owner_library: false, enter_transformed: false, enters_under: None, @@ -2238,6 +2253,22 @@ mod tests { enters_modified_if: None, }, ))); + effect_def.mode_abilities.push(AbilityDefinition::new( + AbilityKind::Spell, + Effect::ChangeZoneAll { + origin: Some(Zone::Exile), + destination: Zone::Hand, + target: TargetFilter::TrackedSet { + id: TrackedSetId(0), + }, + enters_under: None, + enter_tapped: crate::types::zones::EtbTapState::Unspecified, + enter_with_counters: vec![], + face_down_profile: None, + library_position: None, + random_order: false, + }, + )); let ability = ResolvedAbility::new( Effect::CreateDelayedTrigger { condition: DelayedTriggerCondition::AtNextPhase { phase: Phase::End }, @@ -2260,6 +2291,25 @@ mod tests { match &sub.effect { Effect::ChangeZoneAll { origin, target, .. } => { assert_eq!(*origin, None); + assert!(matches!( + target, + TargetFilter::TrackedSetFiltered { + id: TrackedSetId(1), + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), + .. + } + )); + } + other => panic!("Expected sub ChangeZoneAll, got {:?}", other), + } + + let mode = state.delayed_triggers[0] + .ability + .mode_abilities + .first() + .expect("mode ability must be preserved"); + match mode.effect.as_ref() { + Effect::ChangeZoneAll { target, .. } => { assert_eq!( *target, TargetFilter::TrackedSet { @@ -2267,7 +2317,7 @@ mod tests { } ); } - other => panic!("Expected sub ChangeZoneAll, got {:?}", other), + other => panic!("Expected mode ChangeZoneAll, got {:?}", other), } } diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index d9f727a540..20fb1d30e4 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -17885,7 +17885,7 @@ fn temporal_prefix_in_effect_chain() { #[test] fn temporal_prefix_preserves_full_delayed_effect_chain() { let def = parse_effect_chain( - "At the beginning of the next end step, each player discards their hand and returns to their hand each card they exiled this way.", + "At the beginning of the next end step, each player discards their hand, then returns to their hand each card they exiled this way.", AbilityKind::Spell, ); let Effect::CreateDelayedTrigger { From 9e3efb57806d3e3e1bf9588029d1dc2dbbca2b08 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 18:08:57 -0700 Subject: [PATCH 4/5] fix(PR-7257): preserve delayed tracked-set bindings --- .../src/game/effects/delayed_trigger.rs | 111 ++++++++++++++++-- crates/engine/src/parser/oracle_effect/mod.rs | 6 + .../engine/src/parser/oracle_effect/tests.rs | 48 +++++++- 3 files changed, 154 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/game/effects/delayed_trigger.rs b/crates/engine/src/game/effects/delayed_trigger.rs index 76fa5f06c8..5bc06c880d 100644 --- a/crates/engine/src/game/effects/delayed_trigger.rs +++ b/crates/engine/src/game/effects/delayed_trigger.rs @@ -1027,6 +1027,11 @@ fn bind_tracked_set_to_effect(effect: &mut Effect, real_id: TrackedSetId) { } | TargetFilter::Any => TargetFilter::TrackedSet { id: real_id }, TargetFilter::TrackedSet { id } => TargetFilter::TrackedSet { id: *id }, + TargetFilter::TrackedSetFiltered { .. } => { + let mut bound_target = target.clone(); + bound_target.rebind_tracked_set_sentinel(real_id); + bound_target + } _ => TargetFilter::TrackedSet { id: real_id }, }; *effect = Effect::ChangeZoneAll { @@ -1056,6 +1061,9 @@ fn bind_tracked_set_to_ability_definition(ability: &mut AbilityDefinition, real_ if let Some(else_ability) = ability.else_ability.as_mut() { bind_tracked_set_to_ability_definition(else_ability, real_id); } + for mode_ability in &mut ability.mode_abilities { + bind_tracked_set_to_ability_definition(mode_ability, real_id); + } } fn bind_tracked_set_to_ability_chain(ability: &mut ResolvedAbility, real_id: TrackedSetId) { @@ -1069,6 +1077,9 @@ fn bind_tracked_set_to_ability_chain(ability: &mut ResolvedAbility, real_id: Tra if let Some(else_ability) = ability.else_ability.as_mut() { bind_tracked_set_to_ability_chain(else_ability, real_id); } + for mode_ability in &mut ability.mode_abilities { + bind_tracked_set_to_ability_definition(mode_ability, real_id); + } } /// CR 400.7 + CR 603.7c: True when `filter` is an anaphor that names the @@ -2271,6 +2282,80 @@ mod tests { } } + #[test] + fn uses_tracked_set_binds_mode_ability_effects() { + let mut state = GameState::new_two_player(42); + state + .tracked_object_sets + .insert(TrackedSetId(1), vec![ObjectId(10)]); + state.next_tracked_set_id = 2; + + let mode = AbilityDefinition::new( + AbilityKind::Spell, + Effect::ChangeZoneAll { + origin: Some(Zone::Exile), + destination: Zone::Hand, + target: TargetFilter::TrackedSetFiltered { + id: TrackedSetId(0), + filter: Box::new(TargetFilter::Any), + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), + }, + enters_under: None, + enter_tapped: crate::types::zones::EtbTapState::Unspecified, + enter_with_counters: vec![], + face_down_profile: None, + library_position: None, + random_order: false, + }, + ); + let effect_def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ) + .with_modal( + crate::types::ability::ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 1, + ..Default::default() + }, + vec![mode], + ); + let ability = ResolvedAbility::new( + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::AtNextPhase { phase: Phase::End }, + effect: Box::new(effect_def), + uses_tracked_set: true, + }, + vec![], + ObjectId(5), + PlayerId(0), + ); + let mut events = Vec::new(); + + resolve(&mut state, &ability, &mut events).expect("resolve must succeed"); + + let mode = state.delayed_triggers[0] + .ability + .mode_abilities + .first() + .expect("delayed modal must retain its mode"); + assert!(matches!( + mode.effect.as_ref(), + Effect::ChangeZoneAll { + target: TargetFilter::TrackedSetFiltered { + id: TrackedSetId(1), + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), + .. + }, + .. + } + )); + } + #[test] fn uses_tracked_set_binds_nested_delayed_effects() { let mut state = GameState::new_two_player(42); @@ -2343,14 +2428,17 @@ mod tests { .insert(TrackedSetId(1), vec![ObjectId(10)]); state.next_tracked_set_id = 2; - // Parser emits ChangeZone with TrackedSetId(0) sentinel + // The ChangeZone upgrade must preserve the tracked-set filter and bind + // its sentinel, rather than dropping it to an unfiltered tracked set. let effect_def = AbilityDefinition::new( AbilityKind::Spell, Effect::ChangeZone { origin: None, destination: Zone::Battlefield, - target: TargetFilter::TrackedSet { + target: TargetFilter::TrackedSetFiltered { id: TrackedSetId(0), + filter: Box::new(TargetFilter::Any), + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), }, owner_library: false, enter_transformed: false, @@ -2379,8 +2467,9 @@ mod tests { let result = resolve(&mut state, &ability, &mut events); assert!(result.is_ok()); - // Should be upgraded to ChangeZoneAll with resolved TrackedSetId; origin - // stays unset so runtime derives member zones when firing. + // Should be upgraded to ChangeZoneAll with the filtered tracked set + // intact and its sentinel resolved; origin stays unset so runtime derives + // member zones when firing. match &state.delayed_triggers[0].ability.effect { Effect::ChangeZoneAll { origin, @@ -2390,12 +2479,14 @@ mod tests { } => { assert_eq!(*origin, None); assert_eq!(*destination, Zone::Battlefield); - assert_eq!( - *target, - TargetFilter::TrackedSet { - id: TrackedSetId(1) - } - ); + assert!(matches!( + target, + TargetFilter::TrackedSetFiltered { + id: TrackedSetId(1), + filter, + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), + } if matches!(filter.as_ref(), TargetFilter::Any) + )); } other => panic!("Expected ChangeZoneAll, got {:?}", other), } diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 2393c61576..b56868e58d 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -27566,6 +27566,9 @@ fn rewrite_player_scope_refs(def: &mut AbilityDefinition) { if let Some(else_branch) = def.else_ability.as_mut() { rewrite_player_scope_refs(else_branch); } + for mode_ability in &mut def.mode_abilities { + rewrite_player_scope_refs(mode_ability); + } } /// CR 608.2 + CR 109.5: Apply `rewrite_player_scope_refs` rooted at every def in @@ -27595,6 +27598,9 @@ fn apply_player_scope_rewrites(def: &mut AbilityDefinition) { if let Some(else_branch) = def.else_ability.as_mut() { apply_player_scope_rewrites(else_branch); } + for mode_ability in &mut def.mode_abilities { + apply_player_scope_rewrites(mode_ability); + } } /// CR 603.7c + CR 119.3 + CR 120.3: Rebind the event-bound player possessive diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index d9f727a540..2f1ac892b8 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -44,6 +44,52 @@ fn each_target_filter_mut_does_not_visit_shuffle() { ); } +#[test] +fn player_scope_rewrite_reaches_modal_mode_abilities() { + let mode = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Discard { + count: QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::Target, + }, + }, + target: TargetFilter::Controller, + }, + ); + let mut def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ) + .player_scope(PlayerFilter::All) + .with_modal( + ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 1, + ..Default::default() + }, + vec![mode], + ); + + apply_player_scope_rewrites(&mut def); + + assert!(matches!( + def.mode_abilities[0].effect.as_ref(), + Effect::Discard { + count: QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::ScopedPlayer, + }, + }, + target: TargetFilter::ScopedPlayer, + } + )); +} + #[test] fn do_the_same_type_rewrite_does_not_overwrite_logical_target_leaves() { let original = TargetFilter::And { @@ -17885,7 +17931,7 @@ fn temporal_prefix_in_effect_chain() { #[test] fn temporal_prefix_preserves_full_delayed_effect_chain() { let def = parse_effect_chain( - "At the beginning of the next end step, each player discards their hand and returns to their hand each card they exiled this way.", + "At the beginning of the next end step, each player discards their hand, then returns to their hand each card they exiled this way.", AbilityKind::Spell, ); let Effect::CreateDelayedTrigger { From 2ab09bfb365e76c26afbf7670f8f68630d45d616 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 18:08:57 -0700 Subject: [PATCH 5/5] fix(PR-7257): preserve delayed tracked-set bindings --- .../src/game/effects/delayed_trigger.rs | 100 ++++++++++++++++-- crates/engine/src/parser/oracle_effect/mod.rs | 6 ++ .../engine/src/parser/oracle_effect/tests.rs | 58 +++++++++- 3 files changed, 152 insertions(+), 12 deletions(-) diff --git a/crates/engine/src/game/effects/delayed_trigger.rs b/crates/engine/src/game/effects/delayed_trigger.rs index ac71d36698..e11e208931 100644 --- a/crates/engine/src/game/effects/delayed_trigger.rs +++ b/crates/engine/src/game/effects/delayed_trigger.rs @@ -2321,6 +2321,80 @@ mod tests { } } + #[test] + fn uses_tracked_set_binds_mode_ability_effects() { + let mut state = GameState::new_two_player(42); + state + .tracked_object_sets + .insert(TrackedSetId(1), vec![ObjectId(10)]); + state.next_tracked_set_id = 2; + + let mode = AbilityDefinition::new( + AbilityKind::Spell, + Effect::ChangeZoneAll { + origin: Some(Zone::Exile), + destination: Zone::Hand, + target: TargetFilter::TrackedSetFiltered { + id: TrackedSetId(0), + filter: Box::new(TargetFilter::Any), + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), + }, + enters_under: None, + enter_tapped: crate::types::zones::EtbTapState::Unspecified, + enter_with_counters: vec![], + face_down_profile: None, + library_position: None, + random_order: false, + }, + ); + let effect_def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ) + .with_modal( + crate::types::ability::ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 1, + ..Default::default() + }, + vec![mode], + ); + let ability = ResolvedAbility::new( + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::AtNextPhase { phase: Phase::End }, + effect: Box::new(effect_def), + uses_tracked_set: true, + }, + vec![], + ObjectId(5), + PlayerId(0), + ); + let mut events = Vec::new(); + + resolve(&mut state, &ability, &mut events).expect("resolve must succeed"); + + let mode = state.delayed_triggers[0] + .ability + .mode_abilities + .first() + .expect("delayed modal must retain its mode"); + assert!(matches!( + mode.effect.as_ref(), + Effect::ChangeZoneAll { + target: TargetFilter::TrackedSetFiltered { + id: TrackedSetId(1), + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), + .. + }, + .. + } + )); + } + #[test] fn uses_tracked_set_binds_nested_delayed_effects() { let mut state = GameState::new_two_player(42); @@ -2393,14 +2467,17 @@ mod tests { .insert(TrackedSetId(1), vec![ObjectId(10)]); state.next_tracked_set_id = 2; - // Parser emits ChangeZone with TrackedSetId(0) sentinel + // The ChangeZone upgrade must preserve the tracked-set filter and bind + // its sentinel, rather than dropping it to an unfiltered tracked set. let effect_def = AbilityDefinition::new( AbilityKind::Spell, Effect::ChangeZone { origin: None, destination: Zone::Battlefield, - target: TargetFilter::TrackedSet { + target: TargetFilter::TrackedSetFiltered { id: TrackedSetId(0), + filter: Box::new(TargetFilter::Any), + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), }, owner_library: false, enter_transformed: false, @@ -2429,8 +2506,9 @@ mod tests { let result = resolve(&mut state, &ability, &mut events); assert!(result.is_ok()); - // Should be upgraded to ChangeZoneAll with resolved TrackedSetId; origin - // stays unset so runtime derives member zones when firing. + // Should be upgraded to ChangeZoneAll with the filtered tracked set + // intact and its sentinel resolved; origin stays unset so runtime derives + // member zones when firing. match &state.delayed_triggers[0].ability.effect { Effect::ChangeZoneAll { origin, @@ -2440,12 +2518,14 @@ mod tests { } => { assert_eq!(*origin, None); assert_eq!(*destination, Zone::Battlefield); - assert_eq!( - *target, - TargetFilter::TrackedSet { - id: TrackedSetId(1) - } - ); + assert!(matches!( + target, + TargetFilter::TrackedSetFiltered { + id: TrackedSetId(1), + filter, + caused_by: Some(crate::types::ability::ThisWayCause::Exiled), + } if matches!(filter.as_ref(), TargetFilter::Any) + )); } other => panic!("Expected ChangeZoneAll, got {:?}", other), } diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 2393c61576..b56868e58d 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -27566,6 +27566,9 @@ fn rewrite_player_scope_refs(def: &mut AbilityDefinition) { if let Some(else_branch) = def.else_ability.as_mut() { rewrite_player_scope_refs(else_branch); } + for mode_ability in &mut def.mode_abilities { + rewrite_player_scope_refs(mode_ability); + } } /// CR 608.2 + CR 109.5: Apply `rewrite_player_scope_refs` rooted at every def in @@ -27595,6 +27598,9 @@ fn apply_player_scope_rewrites(def: &mut AbilityDefinition) { if let Some(else_branch) = def.else_ability.as_mut() { apply_player_scope_rewrites(else_branch); } + for mode_ability in &mut def.mode_abilities { + apply_player_scope_rewrites(mode_ability); + } } /// CR 603.7c + CR 119.3 + CR 120.3: Rebind the event-bound player possessive diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index 20fb1d30e4..6487731100 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -10,8 +10,8 @@ use crate::parser::parse_oracle_text; use crate::types::ability::CardPlayMode::{Cast, Play}; use crate::types::ability::CastFromZoneDriver::{DuringResolution, LingeringPermission}; use crate::types::ability::{ - AttachmentKind, CastManaObjectScope, CastManaSpentMetric, ExcessRecipient, - ForEachCategoryAction, PerpetualModification, SeatDirection, + AttachmentKind, CardSelectionMode, CastManaObjectScope, CastManaSpentMetric, ExcessRecipient, + ForEachCategoryAction, ModalChoice, PerpetualModification, SeatDirection, }; use crate::types::card_type::CoreType; use crate::types::mana::{ManaCost, ManaCostShard}; @@ -44,6 +44,60 @@ fn each_target_filter_mut_does_not_visit_shuffle() { ); } +#[test] +fn player_scope_rewrite_reaches_modal_mode_abilities() { + let mode = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Discard { + count: QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::Target, + }, + }, + target: TargetFilter::Typed(TypedFilter::card().controller(ControllerRef::You)), + selection: CardSelectionMode::Chosen, + unless_filter: None, + filter: None, + }, + ) + .player_scope(PlayerFilter::All); + let mut def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ) + .player_scope(PlayerFilter::All) + .with_modal( + ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 1, + ..Default::default() + }, + vec![mode], + ); + + apply_player_scope_rewrites(&mut def); + + assert!(matches!( + def.mode_abilities[0].effect.as_ref(), + Effect::Discard { + count: QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::ScopedPlayer, + }, + }, + target: TargetFilter::Typed(TypedFilter { + controller: Some(ControllerRef::ScopedPlayer), + .. + }), + .. + } + )); +} + #[test] fn do_the_same_type_rewrite_does_not_overwrite_logical_target_leaves() { let original = TargetFilter::And {