Skip to content

fix(engine): reject stale object targets after zone changes #7235

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
matthewevans merged 1 commit into from
Aug 12, 2026
Merged

fix(engine): reject stale object targets after zone changes #7235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/engine/src/game/ability_rw.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3732,6 +3732,7 @@ fn walk_ability(
trigger_definition_ref: _, // exact trigger occurrence, no read/write effect
force_block_attacker: _, // exact force-block referent, no read/write effect
target_incarnations: _, // CR 400.7 pins on the referents, no read/write effect
selected_target_incarnations: _, // CR 400.7 selected-target pins, no read/write effect
controller: _,
original_controller: _,
scoped_player: _,
Expand Down
1 change: 1 addition & 0 deletions crates/engine/src/game/ability_scan.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ fn resolved_ability_axes(a: &ResolvedAbility, mode: ScanMode) -> Axes {
trigger_definition_ref: _, // exact trigger occurrence, no dynamic read
force_block_attacker: _, // exact force-block referent, no dynamic read
target_incarnations: _, // CR 400.7 referent pins, no dynamic read
selected_target_incarnations: _, // CR 400.7 selected-target pins, no dynamic read
controller: _, // player id
original_controller: _, // player id
scoped_player: _, // player id (iteration binding)
Expand Down
110 changes: 66 additions & 44 deletions crates/engine/src/game/ability_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1777,10 +1777,12 @@ pub fn assign_targets_in_chain(
) -> Result<(), EngineError> {
if is_per_opponent_target_fanout(ability) {
ability.targets = targets.to_vec();
ability.capture_target_incarnations_recursive(state);
return Ok(());
}
if !chain_has_target_sink(ability) {
ability.targets = targets.to_vec();
ability.capture_target_incarnations_recursive(state);
return Ok(());
}
let mut next_target = 0usize;
Expand All @@ -1790,6 +1792,7 @@ pub fn assign_targets_in_chain(
"Unused selected targets".to_string(),
));
}
ability.capture_target_incarnations_recursive(state);
Ok(())
}

Expand All @@ -1800,10 +1803,12 @@ pub fn assign_selected_slots_in_chain(
) -> Result<(), EngineError> {
if is_per_opponent_target_fanout(ability) {
ability.targets = selected_slots.iter().flatten().cloned().collect();
ability.capture_target_incarnations_recursive(state);
return Ok(());
}
if !chain_has_target_sink(ability) {
ability.targets = selected_slots.iter().flatten().cloned().collect();
ability.capture_target_incarnations_recursive(state);
return Ok(());
}
let mut next_slot = 0usize;
Expand All @@ -1813,6 +1818,7 @@ pub fn assign_selected_slots_in_chain(
"Unused selected target slots".to_string(),
));
}
ability.capture_target_incarnations_recursive(state);
Ok(())
}

Expand Down Expand Up @@ -1860,6 +1866,28 @@ pub fn distribution_targets(ability: &ResolvedAbility) -> Vec<TargetRef> {
}

/// CR 608.2b: Re-validate targets on resolution — remove any that are no longer legal.
fn target_is_current(ability: &ResolvedAbility, target: &TargetRef, state: &GameState) -> bool {
match target {
TargetRef::Object(id) => {
ability.target_pin_is_current(*id, state)
&& ability.selected_target_pin_is_current(*id, state)
}
TargetRef::Player(_) => true,
}
}

fn validate_pinned_targets(
state: &GameState,
targets: &[TargetRef],
filter: &TargetFilter,
ability: &ResolvedAbility,
) -> Vec<TargetRef> {
targeting::validate_targets_for_ability(state, targets, filter, ability)
.into_iter()
.filter(|target| target_is_current(ability, target, state))
.collect()
}

pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) -> ResolvedAbility {
let mut validated = ability.clone();
validated.targets = if is_per_opponent_target_fanout(&validated) {
Expand All @@ -1876,7 +1904,7 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) -
.filter(|filter| !filter.is_context_ref())
.zip(validated.targets.iter())
.filter_map(|(filter, target_ref)| {
let legal = targeting::validate_targets_for_ability(
let legal = validate_pinned_targets(
state,
std::slice::from_ref(target_ref),
filter,
Expand Down Expand Up @@ -1905,14 +1933,10 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) -
let Some(target_ref) = target_iter.next() else {
continue;
};
if let Some(legal) = targeting::validate_targets_for_ability(
state,
std::slice::from_ref(target_ref),
filter,
&validated,
)
.into_iter()
.next()
if let Some(legal) =
validate_pinned_targets(state, std::slice::from_ref(target_ref), filter, &validated)
.into_iter()
.next()
{
kept.push(legal);
}
Expand Down Expand Up @@ -1967,13 +1991,8 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) -
let Some(target_ref) = target_iter.next() else {
break;
};
if !targeting::validate_targets_for_ability(
state,
std::slice::from_ref(target_ref),
filter,
&validated,
)
.is_empty()
if !validate_pinned_targets(state, std::slice::from_ref(target_ref), filter, &validated)
.is_empty()
{
any_legal = true;
}
Expand All @@ -1998,7 +2017,7 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) -
let Some(target_ref) = target_iter.next() else {
continue;
};
if let Some(legal) = targeting::validate_targets_for_ability(
if let Some(legal) = validate_pinned_targets(
state,
std::slice::from_ref(target_ref),
filter,
Expand Down Expand Up @@ -2043,15 +2062,10 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) -
let explicit: Vec<TargetRef> = candidate_targets
.iter()
.filter(|t| {
targeting::validate_targets_for_ability(
state,
std::slice::from_ref(t),
target,
&validated,
)
.into_iter()
.next()
.is_some()
validate_pinned_targets(state, std::slice::from_ref(t), target, &validated)
.into_iter()
.next()
.is_some()
})
.cloned()
.collect();
Expand All @@ -2062,7 +2076,9 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) -
let TargetRef::Object(id) = t else {
return false;
};
!explicit.contains(t) && fight_creature_on_battlefield(state, *id)
!explicit.contains(t)
&& fight_creature_on_battlefield(state, *id)
&& target_is_current(&validated, t, state)
}) {
kept.push(ally.clone());
}
Expand All @@ -2077,15 +2093,18 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) -
// would fizzle-filter the spell to battlefield presence and drop it
// (the spell lives on the STACK). Re-validate against the source leaf
// (`InZone Stack`-aware) instead, preserving the spell target.
targeting::validate_targets_for_ability(state, &validated.targets, &src_leaf, &validated)
validate_pinned_targets(state, &validated.targets, &src_leaf, &validated)
} else {
match triggers::extract_target_filter_from_effect(&validated.effect) {
Some(filter) if matches!(validated.effect, Effect::PairWith { .. }) => {
let legal_choices = pair_with_legal_choices(state, &validated, filter);
validated
.targets
.iter()
.filter(|target| legal_choices.contains(target))
.filter(|target| {
legal_choices.contains(target)
&& target_is_current(&validated, target, state)
})
.cloned()
.collect()
}
Expand All @@ -2102,20 +2121,20 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) -
Some((_, rest)) => rest,
None => &[],
};
kept.extend(targeting::validate_targets_for_ability(
if let Some(companion) = kept.first() {
if !target_is_current(&validated, companion, state) {
kept.clear();
}
}
kept.extend(validate_pinned_targets(
state,
primary_targets,
filter,
&validated,
));
kept
}
Some(filter) => targeting::validate_targets_for_ability(
state,
&validated.targets,
filter,
&validated,
),
Some(filter) => validate_pinned_targets(state, &validated.targets, filter, &validated),
// CR 608.2b: A context-ref filter (`ParentTarget`,
// `TriggeringSource`, etc.) carries a resolution-time *snapshot*,
// not a player-chosen target. `extract_target_filter_from_effect`
Expand Down Expand Up @@ -2160,17 +2179,17 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) -
state,
validated.source_id,
) {
Some(filter) => targeting::validate_targets_for_ability(
state,
&validated.targets,
&filter,
&validated,
),
Some(filter) => {
validate_pinned_targets(state, &validated.targets, &filter, &validated)
}
None => validated
.targets
.iter()
.filter(|target| match target {
TargetRef::Object(object_id) => state.battlefield.contains(object_id),
TargetRef::Object(object_id) => {
state.battlefield.contains(object_id)
&& target_is_current(&validated, target, state)
}
TargetRef::Player(_) => true,
})
.cloned()
Expand All @@ -2181,7 +2200,10 @@ pub fn validate_targets_in_chain(state: &GameState, ability: &ResolvedAbility) -
.targets
.iter()
.filter(|target| match target {
TargetRef::Object(object_id) => state.battlefield.contains(object_id),
TargetRef::Object(object_id) => {
state.battlefield.contains(object_id)
&& target_is_current(&validated, target, state)
}
TargetRef::Player(_) => true,
})
.cloned()
Expand Down
1 change: 1 addition & 0 deletions crates/engine/src/game/effects/additional_phase.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ mod tests {
trigger_definition_ref: None,
force_block_attacker: None,
target_incarnations: Vec::new(),
selected_target_incarnations: Vec::new(),
targets: vec![],
kind: AbilityKind::Spell,
sub_ability: None,
Expand Down
16 changes: 16 additions & 0 deletions crates/engine/src/game/effects/change_targets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,24 @@ pub fn resolve(
// `vec![new_target]`, which would delete the untouched slot.
let updated =
forced_retarget_targets(state, &stack_ability, &current_targets, new_target);
let changed = updated
.iter()
.zip(current_targets.iter())
.find(|(updated, current)| {
stack_ability.retarget_target_requires_pin_refresh(current, updated, state)
})
.and_then(|(target, _)| match target {
TargetRef::Object(id) => state
.objects
.get(id)
.map(crate::types::identifiers::ObjectIncarnationRef::from_object),
TargetRef::Player(_) => None,
});
if let Some(stack_ability_mut) = state.stack[stack_entry_index].ability_mut() {
stack_ability_mut.targets = updated;
if let Some(pin) = changed {
stack_ability_mut.update_selected_target_incarnation(pin);
}
}
}
events.push(GameEvent::EffectResolved {
Expand Down
Loading
Loading