Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
3 changes: 3 additions & 0 deletions crates/engine/src/game/effects/change_zone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ fn resolve_forward_result_search_attach_host(
let targets = forward_result_attach_host_targets(ability);
match target {
TargetFilter::SelfRef => Some(AttachTarget::Object(ability.source_id)),
// CR 303.4b + CR 109.4: "attached to you" (Lynde, Cheerful Tormentor)
// binds the Aura host to the resolving ability's controller.
TargetFilter::Controller => Some(AttachTarget::Player(ability.controller)),
TargetFilter::ParentTarget => targets
.first()
.map(|target| match target {
Expand Down
19 changes: 17 additions & 2 deletions crates/engine/src/game/effects/delayed_trigger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub fn resolve(
)
.map(|t| vec![t])
.unwrap_or_default()
} else if super::effect_refs_parent_target(&delayed_ability.effect) {
} else if super::ability_refs_parent_target(&delayed_ability) {
parent_target_snapshot(state, ability)
} else if effect_references_last_created(&delayed_ability.effect)
&& !state.last_created_token_ids.is_empty()
Expand Down Expand Up @@ -170,13 +170,28 @@ pub fn resolve(
// and activated-ability sources may not already carry trigger provenance;
// capture their current incarnation at creation rather than later rebinding
// the stored ObjectId. CR 400.7.
//
// CR 400.7 + CR 603.7c: when the delayed ability's source is still the same
// ObjectId, refresh zone + incarnation to that object's creation-time
// location. The parent trigger often latched while the source was on the
// battlefield (Gift of Immortality's dies trigger), but SBAs may already
// have moved that Aura to the graveyard (bumping incarnation) before the
// delayed return is created. SelfRef resolution requires a zone+incarnation
// match (`source_is_current_via_zone_match`); keeping the BF/pre-move stamp
// would make the end-step SelfRef ChangeZone no-op.
let source_context = ability.trigger_source.clone().or_else(|| {
state
.objects
.get(&ability.source_id)
.map(|source| super::super::triggers::trigger_source_context_for_latch(state, source))
});
if let Some(source_context) = source_context {
if let Some(mut source_context) = source_context {
if source_context.identity.reference.object_id == delayed_ability.source_id {
if let Some(obj) = state.objects.get(&delayed_ability.source_id) {
source_context.identity.expected_zone = obj.zone;
source_context.identity.reference.incarnation = obj.incarnation;
}
}
delayed_ability.set_trigger_source_recursive(source_context);
}

Expand Down
53 changes: 51 additions & 2 deletions crates/engine/src/game/effects/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5553,6 +5553,22 @@ fn ability_refs_triggering_source(ability: &ResolvedAbility) -> bool {
.is_some_and(ability_refs_triggering_source)
}

/// True when any effect in the ability chain references `ParentTarget`
/// (including nested sub/else abilities). Used by delayed-trigger snapshotting
/// so an Attach host on a ChangeZone sub-chain (Gift of Immortality #4956) still
/// freezes the parent referent at creation time.
fn ability_refs_parent_target(ability: &ResolvedAbility) -> bool {
effect_refs_parent_target(&ability.effect)
|| ability
.sub_ability
.as_deref()
.is_some_and(ability_refs_parent_target)
|| ability
.else_ability
.as_deref()
.is_some_and(ability_refs_parent_target)
}

/// CR 603.7 + CR 109.5: Replace the first `TargetRef::Object` in a target
/// slice with the supplied object id. Used by the `repeat_for: TrackedSetSize`
/// per-iteration rebind so the i-th iteration's parent reference (e.g.,
Expand Down Expand Up @@ -10292,15 +10308,48 @@ fn resolve_chain_body(
// isn't an implicit tracked-set consumer), prepend the moved
// card as a target so `ParentTarget` consumers downstream
// resolve to it.
if !forwarded_objects.is_empty() {
// CR 303.4g + CR 303.4i (#4956): `forward_result` Attach is realized via
// `enter_attached_to` on the zone move. If that move Remained, there is
// no ZoneChanged event — running Attach would stamp `attached_to` onto
// an Aura that never left its origin zone.
if ability.forward_result
&& forwarded_objects.is_empty()
&& matches!(
ability.effect,
Effect::ChangeZone {
destination: Zone::Battlefield,
..
}
)
&& matches!(sub.effect, Effect::Attach { .. })
{
if let Some(trailing) = sub.sub_ability.as_ref() {
let mut trailing_resolved = trailing.as_ref().clone();
apply_parent_chain_context(
&mut trailing_resolved,
ability,
effect_context_object.as_ref(),
state,
);
resolve_ability_chain(state, &trailing_resolved, events, depth + 1)?;
}
} else if !forwarded_objects.is_empty() {
let mut sub_with_context = sub.as_ref().clone();
// CR 707.10: `CopySpell { SelfRef }` copies the resolving spell
// itself (Sevinne's Reclamation, Chain cycle). `forward_result`
// rebinding `source_id` to the just-moved permanent would make
// `copy_spell::resolve` look up the wrong stack entry after
// `resolve_top` has popped the spell (issue #2860).
//
// CR 603.7c + CR 201.5: `CreateDelayedTrigger` must keep the
// creating ability's source. SelfRef inside the delayed body means
// "this card" (Gift of Immortality #4956), not the just-returned
// host. ParentTarget anaphora still receive the moved object via
// `targets` below (snapshot at delayed-trigger creation).
if !copy_spell_self_ref_keeps_resolving_spell_source(sub) {
sub_with_context.source_id = forwarded_objects[0];
if !matches!(sub.effect, Effect::CreateDelayedTrigger { .. }) {
sub_with_context.source_id = forwarded_objects[0];
}
if matches!(
ability.effect,
Effect::Conjure {
Expand Down
3 changes: 3 additions & 0 deletions crates/engine/src/game/zone_pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2952,6 +2952,9 @@ fn execute_zone_move_with_applied_terminal(
}
}
}
// CR 303.4i specified-host Remain is handled after delivery when
// `attach_to` fails / SBA (CR 704.5m). Pre-move filter checks while
// the Aura is still in GY falsely Remained legal Gift/Lynde hosts.
}
if let Some((controller, aura_id, legal_targets)) = pending_aura_choice {
let delivery_start = events.len();
Expand Down
38 changes: 32 additions & 6 deletions crates/engine/src/parser/oracle_effect/assembly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1893,9 +1893,31 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition {
// `parse_target_with_ctx` during chunk parse, so a targeted "of their
// choice" routes target selection to the scoped (upkeep) player.
def.target_chooser = clause_ir.target_chooser.clone();
// CR 701.3a + CR 303.4f: ChangeZone→Battlefield with an Attach sub
// (Gift of Immortality delayed reattach) must nest Attach on the
// ChangeZone with `forward_result` BEFORE delayed wrapping. Sibling-
// pushing then wrapping each unit would yield CDT{ChangeZone} +
// CDT{Attach} instead of CDT{ChangeZone[+Attach]}. TargetOnly already
// nests before wrap; mirror that for this attach-on-return shape.
let clause_sub = if is_target_only {
def.sub_ability = clause_ir.parsed.sub_ability.clone();
None
} else if matches!(
&*def.effect,
Effect::ChangeZone {
destination: Zone::Battlefield,
..
}
) && matches!(
clause_ir.parsed.sub_ability.as_deref().map(|s| &*s.effect),
Some(Effect::Attach { .. })
) {
// CR 303.4f + CR 608.2c: forward the moved Aura into Attach so
// `resolve_forward_result_search_attach_host` stamps `attach_to`
// and skips the CR 303.4f host-choice consult.
def.forward_result = true;
def.sub_ability = clause_ir.parsed.sub_ability.clone();
None
} else {
clause_ir.parsed.sub_ability.clone()
};
Expand Down Expand Up @@ -2252,12 +2274,16 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition {
},
),
);
// CR 608.2c: Lift condition/optional/repeat/player_scope to outer wrapper.
let lifted_condition = inner.condition.clone();
let lifted_optional = inner.optional;
let lifted_optional_for = inner.optional_for;
let lifted_repeat_for = inner.repeat_for.clone();
let lifted_player_scope = inner.player_scope.clone();
// CR 608.2c: Lift condition/optional/repeat/player_scope to the
// outer wrapper — move, don't clone. Leaving
// `OptionalEffectPerformed` on the delayed payload (Next of Kin
// #4956) would re-check that creation-time "if you do" signal when
// the delayed trigger fires at end step and skip the return.
let lifted_condition = std::mem::take(&mut inner.condition);
let lifted_optional = std::mem::replace(&mut inner.optional, false);
let lifted_optional_for = std::mem::take(&mut inner.optional_for);
let lifted_repeat_for = std::mem::take(&mut inner.repeat_for);
let lifted_player_scope = std::mem::take(&mut inner.player_scope);
// CR 608.2c: The `CreateDelayedTrigger` wrapper — not its payload —
// is the node that occupies this clause's slot in the parent's
// `sub_ability` chain, so it must carry the clause's `sub_link`
Expand Down
57 changes: 53 additions & 4 deletions crates/engine/src/parser/oracle_effect/imperative.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1890,12 +1890,13 @@ pub(super) fn parse_targeted_action_ast(
rest
};
let rest_lower = &lower[lower.len() - rest.len()..];
let (trailing_target_text, trailing_dest) = super::strip_return_destination_ext(rest);
let (trailing_target_text, trailing_dest, trailing_dest_remainder) =
super::strip_return_destination_ext_with_remainder(rest);
let (leading_target_text, leading_dest) = super::strip_leading_return_destination_ext(rest);
let (target_text, dest) = if leading_dest.is_some() {
(leading_target_text, leading_dest)
let (target_text, dest, dest_remainder) = if leading_dest.is_some() {
(leading_target_text, leading_dest, "")
} else {
(trailing_target_text, trailing_dest)
(trailing_target_text, trailing_dest, trailing_dest_remainder)
};
let (is_mass, target_text) = if let Some((_, rest)) =
nom_on_lower(target_text, &target_text.to_ascii_lowercase(), |input| {
Expand Down Expand Up @@ -2015,6 +2016,9 @@ pub(super) fn parse_targeted_action_ast(
enter_with_counters: d.enter_with_counters,
})
} else {
// CR 701.3a + CR 303.4f: "return … to the battlefield attached
// to <host>" (Gift of Immortality, Next of Kin, Lynde).
let attach_host = super::sequence::parse_search_attach_host(dest_remainder);
Some(TargetedImperativeAst::ReturnToBattlefield {
target,
origin,
Expand All @@ -2024,6 +2028,7 @@ pub(super) fn parse_targeted_action_ast(
enters_attacking: d.enters_attacking,
enter_with_counters: d.enter_with_counters,
face_down: d.face_down,
attach_host,
})
}
}
Expand Down Expand Up @@ -2299,6 +2304,8 @@ pub(super) fn lower_targeted_action_ast(ast: TargetedImperativeAst) -> Effect {
count,
},
// CR 400.7: Return to battlefield is a zone change, not a bounce.
// CR 701.3a: `attach_host` is an ability-level sub_ability (Attach under
// ChangeZone with forward_result), recovered in `lower_imperative_family_ast`.
TargetedImperativeAst::ReturnToBattlefield {
target,
origin,
Expand All @@ -2308,6 +2315,7 @@ pub(super) fn lower_targeted_action_ast(ast: TargetedImperativeAst) -> Effect {
enters_attacking,
enter_with_counters,
face_down,
attach_host: _,
} => Effect::ChangeZone {
origin,
destination: Zone::Battlefield,
Expand Down Expand Up @@ -12094,6 +12102,47 @@ pub(super) fn lower_imperative_family_ast(ast: ImperativeFamilyAst) -> ParsedEff
clause.multi_target = multi_target;
clause
}
// CR 701.3a + CR 303.4f: "return … to the battlefield attached to <host>"
// (Gift of Immortality, Next of Kin, Lynde). Bare Effect lowering cannot
// carry the Attach sub-chain — nest it here (Cloak / SearchLibrary pattern)
// so assembly can wrap a single ChangeZone[+Attach] unit in CreateDelayedTrigger.
ImperativeFamilyAst::Structured(ImperativeAst::Targeted(
TargetedImperativeAst::ReturnToBattlefield {
target,
origin,
enter_transformed,
enters_under,
enter_tapped,
enters_attacking,
enter_with_counters,
face_down,
attach_host: Some(host),
},
)) => {
let mut clause = parsed_clause(lower_targeted_action_ast(
TargetedImperativeAst::ReturnToBattlefield {
target,
origin,
enter_transformed,
enters_under,
enter_tapped,
enters_attacking,
enter_with_counters,
face_down,
attach_host: None,
},
));
// CR 701.3a: the returning Aura/permanent is SelfRef; the host is the
// anaphor captured on the IR (`that creature` / `you`).
clause.sub_ability = Some(Box::new(AbilityDefinition::new(
AbilityKind::Spell,
Effect::Attach {
attachment: TargetFilter::SelfRef,
target: host,
},
)));
clause
}
// All other arms produce a bare Effect with no sub_ability chain.
other => parsed_clause(lower_imperative_family_effect(other)),
}
Expand Down
3 changes: 3 additions & 0 deletions crates/engine/src/parser/oracle_effect/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6284,6 +6284,9 @@ fn strip_trailing_battlefield_riders(after_destination: &str) -> (&str, Battlefi
}

/// Detect "return ... to <zone>" destination phrase, including "transformed" flag.
/// Thin wrapper over [`strip_return_destination_ext_with_remainder`] for call sites
/// that discard the attach-host remainder (unit tests + legacy helpers).
#[allow(dead_code)] // exercised from `oracle_effect/tests.rs` (cfg(test) sibling)
pub(super) fn strip_return_destination_ext(text: &str) -> (&str, Option<ReturnDestination>) {
let (target, dest, _) = strip_return_destination_ext_with_remainder(text);
(target, dest)
Expand Down
6 changes: 6 additions & 0 deletions crates/engine/src/parser/oracle_effect/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15772,6 +15772,11 @@ fn try_parse_verb_and_target<'a>(
rem,
))
} else {
// CR 701.3a + CR 303.4f: consume "attached to <host>" from the
// destination remainder so it does not fall through as an
// Unimplemented follow-up clause (Gift of Immortality #4956).
let attach_host = sequence::parse_search_attach_host(rem);
let rem = if attach_host.is_some() { "" } else { rem };
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Some((
TargetedImperativeAst::ReturnToBattlefield {
target,
Expand All @@ -15782,6 +15787,7 @@ fn try_parse_verb_and_target<'a>(
enters_attacking: d.enters_attacking,
enter_with_counters: d.enter_with_counters,
face_down: d.face_down,
attach_host,
},
rem,
))
Expand Down
4 changes: 3 additions & 1 deletion crates/engine/src/parser/oracle_effect/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ fn parse_search_attach_host_phrase(input: &str) -> OracleResult<'_, TargetFilter
.parse(input)
}

fn parse_search_attach_host(text: &str) -> Option<TargetFilter> {
/// CR 701.3a + CR 303.4f: Parse an "attached to <host>" rider from return /
/// search destination remainders (Gift of Immortality, Next of Kin, Lynde).
pub(crate) fn parse_search_attach_host(text: &str) -> Option<TargetFilter> {
let lower = text.to_ascii_lowercase();
nom_on_lower(text, &lower, |input| {
let (input, _) = take_until("attached to").parse(input)?;
Expand Down
6 changes: 6 additions & 0 deletions crates/engine/src/parser/oracle_ir/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,12 @@ pub(crate) enum TargetedImperativeAst {
/// a Forest land."). Lowered to a default vanilla-2/2 `face_down_profile`,
/// refined by a trailing "It's a <type>" `FaceDownProfileSpec`.
face_down: bool,
/// CR 701.3a + CR 303.4f/i: Optional "attached to <host>" rider on the
/// return (Gift of Immortality, Next of Kin, Lynde). When set, lowering
/// nests `Effect::Attach { SelfRef → host }` under the ChangeZone with
/// `forward_result` so the Aura enters attached and skips the CR 303.4f
/// host-choice consult.
attach_host: Option<TargetFilter>,
},
/// CR 400.6: Return to a specific non-hand, non-battlefield zone (zone change).
ReturnToZone {
Expand Down
4 changes: 4 additions & 0 deletions crates/engine/src/parser/oracle_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2204,6 +2204,9 @@ pub fn normalize_card_name_refs(text: &str, card_name: &str) -> String {
let short_name = &effective_name[..of_pos];
let lower_short = short_name.to_lowercase();
// structural: not dispatch — guarding single-word short names only
// CR 201.5 / CR 201.5c: "Next of Kin" short name "Next" must not
// rewrite temporal "the next end step" → "the ~ end step" (Gift of
// Immortality peer class; issue #4956).
let is_common_english_word = !short_name.contains(' ')
&& matches!(
lower_short.as_str(),
Expand All @@ -2223,6 +2226,7 @@ pub fn normalize_card_name_refs(text: &str, card_name: &str) -> String {
| "back"
| "away"
| "off"
| "next"
);
// CR 201.3a: a card's "of"-derived short name normalizes to `~`
// (interchangeable name reference). Suppress this ONLY when the
Expand Down
Loading
Loading