diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index 75fe5d5bda..a160856922 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -2094,6 +2094,28 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) - // (the spell lives on the STACK). Re-validate against the source leaf // (`InZone Stack`-aware) instead, preserving the spell target. validate_pinned_targets(state, &validated.targets, &src_leaf, &validated) + } else if matches!( + &validated.effect, + Effect::ChangeZoneAll { target, .. } if crate::game::effects::filter_refs_parent_target(target) + ) { + // CR 115.1 + CR 608.2b: `ChangeZoneAll` is a resolution-time mass + // instruction when its filter carries a delayed `ParentTarget` snapshot + // inside a tracked set. Treating that internal filter as a target would + // fizzle a valid Exile -> Battlefield return before the mass resolver + // can inspect the tracked member. Ordinary ChangeZoneAll player filters + // remain declared targets and follow the generic validation below. + validated.targets.clone() + } else if matches!( + mass_all_target_filter(&validated.effect), + Some(TargetFilter::Player) + ) { + // CR 115.1 + CR 608.2b: A bare `Player` mass-operation filter (such as + // "exile target player's graveyard") is represented by a companion + // declared-player slot. The mass filter is normally a resolution-time + // population scan, so it has no `target_filter()` entry; validate this + // exceptional declared target against the same legal-player set used + // to build the slot. + validate_pinned_targets(state, &validated.targets, &TargetFilter::Player, &validated) } else { match triggers::extract_target_filter_from_effect(&validated.effect) { Some(filter) if matches!(validated.effect, Effect::PairWith { .. }) => { @@ -8661,6 +8683,55 @@ mod tests { ); } + /// CR 115.1 + CR 603.7c: delayed plural returns become `ChangeZoneAll` + /// with a tracked-set filter, but their snapshotted referent is still not a + /// chosen target. It must survive validation while it is in exile. + #[test] + fn validate_targets_in_chain_preserves_delayed_tracked_set_snapshot_off_battlefield() { + let format = FormatConfig::duel_commander(); + let mut state = GameState::new(format, 2, 2); + let victim = create_object( + &mut state, + CardId(0), + PlayerId(1), + "Grizzly Bears".to_string(), + Zone::Exile, + ); + let set_id = TrackedSetId(1); + state.tracked_object_sets.insert(set_id, vec![victim]); + + let ability = ResolvedAbility::new( + Effect::ChangeZoneAll { + origin: Some(Zone::Exile), + destination: Zone::Battlefield, + target: TargetFilter::TrackedSetFiltered { + id: set_id, + filter: Box::new(TargetFilter::ParentTarget), + 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, + }, + vec![TargetRef::Object(victim)], + ObjectId(99), + PlayerId(0), + ); + + let validated = validate_targets_in_chain(&state, &ability); + assert_eq!(validated.targets, ability.targets); + assert!( + !crate::game::targeting::check_fizzle( + &flatten_targets_in_chain(&ability), + &flatten_targets_in_chain(&validated), + ), + "a delayed tracked-set return must not fizzle before its mass resolver runs" + ); + } + /// 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 @@ -13293,6 +13364,44 @@ mod tests { .contains(&TargetRef::Player(PlayerId(1)))); } + /// CR 115.1 + CR 608.2b: a ChangeZoneAll player filter is a declared + /// player target, unlike an internal delayed ParentTarget filter. It must + /// be revalidated and cannot keep an eliminated player alive as a target. + #[test] + fn validate_targets_in_chain_drops_eliminated_change_zone_all_player_target() { + let mut state = GameState::new_two_player(42); + state.players[1].is_eliminated = true; + let ability = ResolvedAbility::new( + Effect::ChangeZoneAll { + origin: Some(Zone::Graveyard), + destination: Zone::Exile, + target: TargetFilter::Player, + enters_under: None, + enter_tapped: crate::types::zones::EtbTapState::Unspecified, + enter_with_counters: vec![], + face_down_profile: None, + library_position: None, + random_order: false, + }, + vec![TargetRef::Player(PlayerId(1))], + ObjectId(900), + PlayerId(0), + ); + + let validated = validate_targets_in_chain(&state, &ability); + assert!( + validated.targets.is_empty(), + "an eliminated player must not survive ChangeZoneAll target revalidation" + ); + assert!( + crate::game::targeting::check_fizzle( + &flatten_targets_in_chain(&ability), + &flatten_targets_in_chain(&validated), + ), + "a ChangeZoneAll ability whose sole player target is gone must fizzle" + ); + } + /// CR 109.4 + CR 115.1 + CR 506.2: Karazikar regression guard. /// /// "Whenever you attack a player, tap target creature that player controls diff --git a/crates/engine/src/game/effects/change_zone.rs b/crates/engine/src/game/effects/change_zone.rs index 081ad1e1ef..06162bf02a 100644 --- a/crates/engine/src/game/effects/change_zone.rs +++ b/crates/engine/src/game/effects/change_zone.rs @@ -206,6 +206,84 @@ fn tracked_set_member_zones(state: &GameState, filter: &TargetFilter) -> Option< (!zones.is_empty()).then_some(zones) } +/// CR 400.7 + CR 603.7c: A delayed tracked-set move retains an object-anaphor +/// member predicate until its creation-time pin has been recorded. At firing, +/// bind that predicate to the stored referent before the mass scan: the object +/// has already become the immediate exile successor of the pin, so the generic +/// `ParentTarget` matcher correctly rejects it as stale. +fn bind_delayed_parent_target_filter( + filter: &TargetFilter, + parent_targets: &[TargetRef], +) -> TargetFilter { + match filter { + TargetFilter::ParentTarget | TargetFilter::ParentTargetSlot { .. } => { + super::delayed_trigger::concrete_parent_target_filter(filter, parent_targets) + } + TargetFilter::TrackedSetFiltered { + id, + filter, + caused_by, + } => TargetFilter::TrackedSetFiltered { + id: *id, + filter: Box::new(bind_delayed_parent_target_filter(filter, parent_targets)), + caused_by: *caused_by, + }, + TargetFilter::And { filters } => TargetFilter::And { + filters: filters + .iter() + .map(|filter| bind_delayed_parent_target_filter(filter, parent_targets)) + .collect(), + }, + TargetFilter::Or { filters } => TargetFilter::Or { + filters: filters + .iter() + .map(|filter| bind_delayed_parent_target_filter(filter, parent_targets)) + .collect(), + }, + TargetFilter::Not { filter } => TargetFilter::Not { + filter: Box::new(bind_delayed_parent_target_filter(filter, parent_targets)), + }, + _ => filter.clone(), + } +} + +/// CR 400.7 + CR 603.7c: A delayed Exile → Battlefield return follows the +/// parent ability's own exile move. Its creation-time target pin is therefore +/// one incarnation behind the expected exile object; a later zone change creates +/// a further incarnation which must not be returned. +fn target_pin_is_current_or_delayed_exile_successor( + state: &GameState, + ability: &ResolvedAbility, + object_id: ObjectId, +) -> bool { + ability + .target_incarnations + .iter() + .find(|pin| pin.object_id == object_id) + .is_none_or(|pin| { + pin.is_current(state) + || state.objects.get(&object_id).is_some_and(|object| { + pin.incarnation.checked_add(1) == Some(object.incarnation) + }) + }) +} + +/// CR 400.7 + CR 603.7c: A delayed tracked-set move must validate its +/// incarnation pins when a nested set member filter names the creation-time +/// parent object. The anaphor walk is the shared recursive authority. +fn tracked_set_filter_names_parent_object(filter: &TargetFilter) -> bool { + match filter { + TargetFilter::TrackedSetFiltered { filter, .. } => { + super::delayed_trigger::filter_refs_parent_object_anaphor(filter) + } + TargetFilter::And { filters } | TargetFilter::Or { filters } => { + filters.iter().any(tracked_set_filter_names_parent_object) + } + TargetFilter::Not { filter } => tracked_set_filter_names_parent_object(filter), + _ => false, + } +} + /// CR 110.2a: Resolve the optional `enters_under` controller override to a /// concrete `PlayerId` for any battlefield-entry effect. Shared by `ChangeZone`, /// `ChangeZoneAll`, and `Manifest` so every entry path resolves the reference @@ -1646,6 +1724,22 @@ pub fn resolve_all( let effective_filter = crate::game::targeting::resolve_tracked_set_sentinel(state, effective_filter); + // CR 400.7 + CR 603.7c: This predicate must inspect the parser-preserved + // anaphor before the firing-time binding below replaces it with the delayed + // trigger's concrete referent. + let tracked_members_name_parent_object = tracked_set_filter_names_parent_object(&target_filter); + + // A delayed `ChangeZone` that named its parent object is upgraded to a + // tracked-set mass move. The pin distinguishes stale later incarnations; + // this binding supplies the one immediate successor that the delayed return + // is allowed to find in the tracked set. + let effective_filter = + if tracked_members_name_parent_object && !ability.target_incarnations.is_empty() { + bind_delayed_parent_target_filter(&effective_filter, &ability.targets) + } else { + effective_filter + }; + // CR 608.2c: Re-derive scan zones after the tracked-set sentinel binds — // the initial `origin`/`target` snapshot may have defaulted to the // battlefield before `chain_tracked_set_id` was populated (Zimone's @@ -1661,6 +1755,7 @@ pub fn resolve_all( } else { origin_zones }; + let delayed_exile_return = origin_zones == [Zone::Exile] && dest_zone == Zone::Battlefield; let track_exiled_by_source = crate::game::exile_links::should_track_exiled_by_source(state, ability.source_id, ability); @@ -1726,6 +1821,12 @@ pub fn resolve_all( .iter() .filter(|(&id, obj)| { origin_zones.contains(&obj.zone) + && (!tracked_members_name_parent_object + || if delayed_exile_return { + target_pin_is_current_or_delayed_exile_successor(state, ability, id) + } else { + ability.target_pin_is_current(id, state) + }) && crate::game::filter::matches_target_filter( state, id, diff --git a/crates/engine/src/game/effects/delayed_trigger.rs b/crates/engine/src/game/effects/delayed_trigger.rs index e11e208931..6bcad0e331 100644 --- a/crates/engine/src/game/effects/delayed_trigger.rs +++ b/crates/engine/src/game/effects/delayed_trigger.rs @@ -647,7 +647,7 @@ fn bind_parent_target_filter(filter: &mut TargetFilter, parent_targets: &[Target *filter = concrete_parent_target_filter(filter, parent_targets); } -fn concrete_parent_target_filter( +pub(crate) fn concrete_parent_target_filter( filter: &TargetFilter, parent_targets: &[TargetRef], ) -> TargetFilter { @@ -1032,6 +1032,13 @@ fn bind_tracked_set_to_effect(effect: &mut Effect, real_id: TrackedSetId) { bound_target.rebind_tracked_set_sentinel(real_id); bound_target } + TargetFilter::ParentTarget | TargetFilter::ParentTargetSlot { .. } => { + TargetFilter::TrackedSetFiltered { + id: real_id, + filter: Box::new(target.clone()), + caused_by: None, + } + } _ => TargetFilter::TrackedSet { id: real_id }, }; *effect = Effect::ChangeZoneAll { @@ -1105,7 +1112,7 @@ fn bind_tracked_set_to_ability_chain(ability: &mut ResolvedAbility, real_id: Tra /// /// `_ => false` IS CORRECT HERE. `TargetFilter` is a broad, open enum and the /// shipped template ends the same way. Do NOT try to exhaust it. -fn filter_refs_parent_object_anaphor(filter: &TargetFilter) -> bool { +pub(super) fn filter_refs_parent_object_anaphor(filter: &TargetFilter) -> bool { match filter { TargetFilter::ParentTarget | TargetFilter::ParentTargetSlot { .. } => true, // CR 608.2h + CR 108.3: these derive a PLAYER, not an object. @@ -1126,6 +1133,9 @@ fn filter_refs_parent_object_anaphor(filter: &TargetFilter) -> bool { filters.iter().any(filter_refs_parent_object_anaphor) } TargetFilter::Not { filter } => filter_refs_parent_object_anaphor(filter), + TargetFilter::TrackedSetFiltered { filter, .. } => { + filter_refs_parent_object_anaphor(filter) + } _ => false, } } @@ -2531,6 +2541,85 @@ mod tests { } } + /// CR 400.7 + CR 603.7c (issue #7100): tracked-set binding must retain a + /// parent-object anaphor long enough to capture its incarnation. A later + /// zone change creates a new object that the delayed member scan cannot + /// affect, while the original incarnation remains eligible. + #[test] + fn tracked_set_delayed_change_zone_preserves_parent_target_incarnation() { + let mut state = GameState::new_two_player(42); + let creature = crate::game::zones::create_object( + &mut state, + CardId(1), + PlayerId(0), + "Eerie Interlude target".to_string(), + Zone::Battlefield, + ); + let set_id = TrackedSetId(1); + state.tracked_object_sets.insert(set_id, vec![creature]); + state.chain_tracked_set_id = Some(set_id); + state.next_tracked_set_id = 2; + + let effect = AbilityDefinition::new( + AbilityKind::Spell, + Effect::ChangeZone { + origin: Some(Zone::Battlefield), + destination: Zone::Exile, + target: TargetFilter::ParentTarget, + owner_library: false, + enter_transformed: false, + enters_under: None, + enter_tapped: crate::types::zones::EtbTapState::Unspecified, + enters_attacking: false, + up_to: false, + enter_with_counters: vec![], + conditional_enter_with_counters: vec![], + face_down_profile: None, + enters_modified_if: None, + }, + ); + let create = ResolvedAbility::new( + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::AtNextPhase { phase: Phase::End }, + effect: Box::new(effect), + uses_tracked_set: true, + }, + vec![TargetRef::Object(creature)], + ObjectId(100), + PlayerId(0), + ); + let mut events = Vec::new(); + resolve(&mut state, &create, &mut events).expect("delayed trigger installs"); + + let delayed = state.delayed_triggers[0].ability.clone(); + assert_eq!(delayed.target_incarnations.len(), 1); + assert!(delayed.target_pin_is_current(creature, &state)); + assert!(matches!( + &delayed.effect, + Effect::ChangeZoneAll { + target: TargetFilter::TrackedSetFiltered { id, filter, .. }, + .. + } if *id == set_id && matches!(filter.as_ref(), TargetFilter::ParentTarget) + )); + + crate::game::effects::resolve_ability_chain(&mut state, &delayed, &mut events, 0) + .expect("current delayed trigger resolves"); + assert_eq!(state.objects[&creature].zone, Zone::Exile); + + crate::game::zones::move_to_zone(&mut state, creature, Zone::Graveyard, &mut events); + crate::game::zones::move_to_zone(&mut state, creature, Zone::Battlefield, &mut events); + let stale_delayed = state.delayed_triggers[0].ability.clone(); + assert!(!stale_delayed.target_pin_is_current(creature, &state)); + + crate::game::effects::resolve_ability_chain(&mut state, &stale_delayed, &mut events, 0) + .expect("stale delayed trigger resolves"); + assert_eq!( + state.objects[&creature].zone, + Zone::Battlefield, + "a later incarnation must not be matched through the tracked set" + ); + } + #[test] fn uses_tracked_set_binds_zone_change_condition_filter() { let mut state = GameState::new_two_player(42); diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index dc2127303a..03c7d6aaf8 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -5717,6 +5717,10 @@ fn optional_head_declined_all_object_targets(ability: &ResolvedAbility) -> bool /// "when you lose control of this, unattach it" trigger rebinds the per-source /// `attachment` through this hidden slot (Stolen Uniform, Ogre Geargrabber); /// without the arm the `_ => {}` fallback snapshots nothing and it resolves inert. +/// * `Effect::ChangeZoneAll` intentionally has no generic `target_filter()` slot: +/// its filter selects a mass operation rather than a declared target. It still +/// needs inspection here when it carries a delayed `ParentTarget` anaphor, so +/// the delayed trigger snapshots and pins that object before the mass scan. /// /// NOTE: the `_ => {}` arm means "no hidden object slot beyond `target_filter()`". /// Any FUTURE effect that hides an object slot behind `target_filter()` MUST add @@ -5736,6 +5740,9 @@ fn effect_parent_ref_slots(effect: &Effect) -> Vec<&TargetFilter> { Effect::UnattachAll { attachment, .. } if attachment.is_context_ref() => { slots.push(attachment) } + Effect::ChangeZoneAll { target, .. } if filter_refs_parent_target(target) => { + slots.push(target) + } _ => {} } slots @@ -5783,7 +5790,7 @@ fn effect_iterates_over_parent_target(effect: &Effect) -> bool { /// Recurse into compound filters so a wrapped `ParentTargetController` is /// detected wherever it appears (`Or { filters: [..., ParentTargetController, ...] }`). -fn filter_refs_parent_target(filter: &TargetFilter) -> bool { +pub(crate) fn filter_refs_parent_target(filter: &TargetFilter) -> bool { match filter { // CR 603.7c + CR 608.2c: a `ParentTargetSlot { index }` delayed effect // must snapshot the parent targets at creation, exactly like the broad @@ -5812,6 +5819,7 @@ fn filter_refs_parent_target(filter: &TargetFilter) -> bool { filters.iter().any(filter_refs_parent_target) } TargetFilter::Not { filter } => filter_refs_parent_target(filter), + TargetFilter::TrackedSetFiltered { filter, .. } => filter_refs_parent_target(filter), _ => false, } } diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index f04a586b78..87050a07e4 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -16105,11 +16105,10 @@ mod stage2_injector_tests { // three producers: `:6212/:6289/:9477 => :6228/:6305/:9493`. // shifts combine with #6958's paid-cast outcome exclusion and // #6976's conditional-branch exclusions. None creates an - // `OptionalEffect` prompt. #7268 removes two lines above all three, - // preserving the same producers at `:6298/:6375/:9570`. - "game/effects/mod.rs:6298".to_string(), - "game/effects/mod.rs:6375".to_string(), - "game/effects/mod.rs:9570".to_string(), + // `OptionalEffect` prompt. Re-pinned against the merged source. + "game/effects/mod.rs:6306".to_string(), + "game/effects/mod.rs:6383".to_string(), + "game/effects/mod.rs:9578".to_string(), // UNMOVED across the rebase, and that is itself evidence the SET did not // move: a census that had gained or lost a producer would not leave this // entry both byte-identical AND at the same coordinate. diff --git a/crates/engine/src/game/filter.rs b/crates/engine/src/game/filter.rs index fa6597cb02..5d67c3ab9e 100644 --- a/crates/engine/src/game/filter.rs +++ b/crates/engine/src/game/filter.rs @@ -3125,12 +3125,28 @@ fn filter_inner_for_object( // `ObjectScope::EventTarget`; inert (matches nothing) outside a trigger. TargetFilter::EventTarget => crate::game::quantity::triggering_event_target_object(state) .is_some_and(|damaged| damaged == object_id), - // ParentTarget/ParentTargetController/ParentTargetOwner/PostReplacementSourceController + // CR 400.7 + CR 603.7c: a parent object can be the member predicate of + // a tracked-set continuation. In that one scan-based path, match the + // creation-time target only while its recorded incarnation is current. + TargetFilter::ParentTarget => ability.is_some_and(|ability| { + !ability.target_incarnations.is_empty() + && ability.targets.iter().any(|target| { + matches!(target, TargetRef::Object(id) + if *id == object_id && ability.target_pin_is_current(*id, state)) + }) + }), + TargetFilter::ParentTargetSlot { index } => ability.is_some_and(|ability| { + !ability.target_incarnations.is_empty() + && matches!( + ability.targets.get(*index), + Some(TargetRef::Object(id)) + if *id == object_id && ability.target_pin_is_current(*id, state) + ) + }), + // ParentTargetController/ParentTargetOwner/PostReplacementSourceController // resolve at resolution time, not via object matching. ParentTargetOwner // mirrors ParentTargetController for the player-axis side of CR 108.3 vs CR 109.4. - TargetFilter::ParentTarget - | TargetFilter::ParentTargetSlot { .. } - | TargetFilter::ParentTargetController + TargetFilter::ParentTargetController | TargetFilter::ParentTargetOwner | TargetFilter::PostReplacementSourceController // CR 615.5: an object-typed resolution-time ref (the prevented event's