Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions crates/engine/src/analysis/ability_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1889,6 +1889,11 @@ fn build_nodes(faces: &[&CardFace]) -> Vec<AbilityNode> {
nodes.push(build_node(&face.name, def, trigger_axis(trigger)));
}
}
ContinuousModification::GrantReplacement { replacement } => {
if let Some(def) = &replacement.execute {
nodes.push(build_node(&face.name, def, None));
}
}
_ => {}
}
}
Expand Down
17 changes: 8 additions & 9 deletions crates/engine/src/database/unearth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,12 @@

use crate::types::ability::{
AbilityCost, AbilityDefinition, AbilityKind, ContinuousModification, DelayedTriggerCondition,
Duration, Effect, ReplacementDefinition, RestrictionExpiry, StaticDefinition, TargetFilter,
Duration, Effect, RestrictionExpiry, StaticDefinition, TargetFilter,
};
use crate::types::card::CardFace;
use crate::types::keywords::Keyword;
use crate::types::mana::ManaCost;
use crate::types::phase::Phase;
use crate::types::replacements::ReplacementEvent;
use crate::types::zones::Zone;

/// CR 702.84a: Synthesize the graveyard-activated reanimation ability for every
Expand Down Expand Up @@ -162,13 +161,13 @@ fn delayed_exile_step() -> AbilityDefinition {
/// reseeds, kept non-copiable (CR 707.2), and pruned on the host's battlefield
/// exit (CR 400.7) — see the variant doc in `types/ability.rs`.
fn leaves_battlefield_exile_step() -> AbilityDefinition {
let replacement = ReplacementDefinition::new(ReplacementEvent::Moved)
.valid_card(TargetFilter::SelfRef)
.expiry(RestrictionExpiry::UntilHostLeavesPlay)
.execute(AbilityDefinition::new(
AbilityKind::Spell,
exile_self_from_battlefield_effect(),
));
// Shares the single unstamped `leave_battlefield_exile_replacement` authority
// (#6538 / #6566); this site keeps its own `target: SelfRef` wrapper +
// description and composes the #6538 host-lifetime stamp itself. The stamp is
// applied per-consumer, not baked into the constructor, because the granted
// path (#6566) must NOT carry it — see the constructor doc.
let replacement = crate::parser::oracle_effect::leave_battlefield_exile_replacement()
.expiry(RestrictionExpiry::UntilHostLeavesPlay);

AbilityDefinition::new(
AbilityKind::Spell,
Expand Down
20 changes: 17 additions & 3 deletions crates/engine/src/game/ability_rw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ use crate::types::ability::FilterProp;
use crate::types::ability::{
AbilityCondition, AbilityDefinition, ContinuousModification, ControllerRef, Duration, Effect,
GuessSubject, ModalChoice, MultiTargetSpec, ObjectScope, PlayerFilter, PlayerScope,
QuantityExpr, QuantityRef, RepeatContinuation, ResolvedAbility, StaticCondition,
StaticDefinition, TargetFilter, TriggerCondition, TriggerDefinition, TypeFilter, TypedFilter,
ZoneRef,
QuantityExpr, QuantityRef, RepeatContinuation, ReplacementDefinition, ResolvedAbility,
StaticCondition, StaticDefinition, TargetFilter, TriggerCondition, TriggerDefinition,
TypeFilter, TypedFilter, ZoneRef,
};
use crate::types::game_state::TargetSelectionConstraint;
use crate::types::zones::Zone;
Expand Down Expand Up @@ -2695,6 +2695,12 @@ fn legacy_continuous_modification(m: &ContinuousModification) -> bool {
legacy_static_definition(definition)
}
ContinuousModification::GrantTrigger { trigger } => legacy_trigger_definition(trigger),
// A granted object-hosted replacement can nest a frozen tag in its
// execute body or its `valid_card` scope filter — distinct traversal from
// GrantTrigger (a ReplacementDefinition, not a TriggerDefinition).
ContinuousModification::GrantReplacement { replacement } => {
legacy_replacement_definition(replacement)
}
ContinuousModification::GrantAllActivatedAbilitiesOf { source, .. }
| ContinuousModification::GrantAllTriggeredAbilitiesOf { source } => {
legacy_target_filter(source)
Expand Down Expand Up @@ -2758,6 +2764,14 @@ fn legacy_continuous_modification(m: &ContinuousModification) -> bool {
}
}

/// A granted object-hosted `ReplacementDefinition` can carry a frozen tag in its
/// `execute` redirect body or its `valid_card` scope filter. Mirrors
/// `legacy_trigger_definition` for the replacement-granting layer-6 case.
fn legacy_replacement_definition(rd: &ReplacementDefinition) -> bool {
rd.execute.as_deref().is_some_and(legacy_definition)
|| rd.valid_card.as_ref().is_some_and(legacy_target_filter)
}

/// A granted / emblem `TriggerDefinition` can carry a frozen tag in its firing
/// filters (`valid_card`/`valid_source`), its intervening-if condition, or its
/// execute body.
Expand Down
4 changes: 4 additions & 0 deletions crates/engine/src/game/ability_scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5003,6 +5003,10 @@ fn scan_continuous_modification(m: &ContinuousModification, mode: ScanMode) -> A
// walker is a follow-up.
ContinuousModification::CopyValues { .. }
| ContinuousModification::GrantTrigger { .. }
// A granted object-hosted replacement's `ReplacementDefinition` execute
// is outside the scanner's traversal closure — fail-closed CONSERVATIVE,
// same class as GrantTrigger / GrantStaticAbility.
| ContinuousModification::GrantReplacement { .. }
| ContinuousModification::GrantAllActivatedAbilitiesOf { .. }
| ContinuousModification::GrantAllTriggeredAbilitiesOf { .. }
| ContinuousModification::AddStaticMode { .. }
Expand Down
20 changes: 20 additions & 0 deletions crates/engine/src/game/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4194,6 +4194,7 @@ fn fmt_modification(m: &crate::types::ability::ContinuousModification) -> String
format!("grant all triggered abilities of {}", fmt_target(source))
}
ContinuousModification::GrantTrigger { .. } => "grant trigger".into(),
ContinuousModification::GrantReplacement { .. } => "grant replacement".into(),
ContinuousModification::RemoveAllAbilities => "remove all abilities".into(),
ContinuousModification::AddType { core_type } => {
format!("add type {}", fmt_core_type(core_type))
Expand Down Expand Up @@ -4340,6 +4341,7 @@ fn static_details(stat: &StaticDefinition) -> Vec<(String, String)> {
m,
ContinuousModification::GrantTrigger { .. }
| ContinuousModification::GrantAbility { .. }
| ContinuousModification::GrantReplacement { .. }
)
})
.map(fmt_modification)
Expand Down Expand Up @@ -4529,6 +4531,11 @@ pub fn build_parse_details(
ContinuousModification::GrantAbility { definition } => {
children.push(build_ability_item(definition));
}
ContinuousModification::GrantReplacement { replacement } => {
if let Some(execute) = &replacement.execute {
children.push(build_ability_item(execute));
}
}
_ => {}
}
}
Expand Down Expand Up @@ -5902,6 +5909,10 @@ fn static_has_unimplemented_parts(def: &StaticDefinition) -> bool {
ContinuousModification::GrantTrigger { trigger } => {
trigger_has_unimplemented_parts(trigger)
}
ContinuousModification::GrantReplacement { replacement } => replacement
.execute
.as_deref()
.is_some_and(ability_definition_has_unimplemented_parts),
_ => false,
})
}
Expand Down Expand Up @@ -5996,6 +6007,11 @@ fn check_statics(
ContinuousModification::GrantTrigger { trigger } => {
check_trigger(trigger, trigger_registry, missing);
}
ContinuousModification::GrantReplacement { replacement } => {
if let Some(execute) = &replacement.execute {
collect_ability_missing_parts(execute, missing);
}
}
_ => {}
}
}
Expand Down Expand Up @@ -6915,6 +6931,10 @@ fn is_static_supported(
ContinuousModification::GrantTrigger { trigger } => {
is_trigger_supported(trigger, trigger_registry)
}
ContinuousModification::GrantReplacement { replacement } => replacement
.execute
.as_deref()
.is_none_or(is_ability_supported),
Comment on lines +6934 to +6937

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Traverse both executable branches of ReplacementDefinition.

All these paths inspect only execute and ignore ReplacementMode::Optional { decline }, causing false-green support/coverage results and incomplete legacy/X analysis.

  • crates/engine/src/game/coverage.rs#L6934-L6937: include decline support when computing is_static_supported.
  • crates/engine/src/game/coverage.rs#L4534-L4538: emit the decline ability as a parsed child.
  • crates/engine/src/game/coverage.rs#L5912-L5915: inspect decline for unimplemented parts.
  • crates/engine/src/game/coverage.rs#L6010-L6014: collect missing parts from decline.
  • crates/engine/src/game/ability_rw.rs#L2698-L2703: traverse decline for legacy references.
  • crates/engine/src/game/ability_rw.rs#L2767-L2773: extend the replacement helper to inspect decline.
  • crates/phase-ai/src/policies/x_reference.rs#L170-L172: detect X references in decline.

As per path instructions, reusable replacement-definition handling must cover the complete GrantReplacement contract, not only the current mandatory rider.

📍 Affects 3 files
  • crates/engine/src/game/coverage.rs#L6934-L6937 (this comment)
  • crates/engine/src/game/coverage.rs#L4534-L4538
  • crates/engine/src/game/coverage.rs#L5912-L5915
  • crates/engine/src/game/coverage.rs#L6010-L6014
  • crates/engine/src/game/ability_rw.rs#L2698-L2703
  • crates/engine/src/game/ability_rw.rs#L2767-L2773
  • crates/phase-ai/src/policies/x_reference.rs#L170-L172
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/engine/src/game/coverage.rs` around lines 6934 - 6937, Update
GrantReplacement handling to traverse both ReplacementDefinition branches:
execute and ReplacementMode::Optional { decline }. In
crates/engine/src/game/coverage.rs at lines 6934-6937, 4534-4538, 5912-5915, and
6010-6014, include decline in static support, parsed-child emission,
unimplemented-part checks, and missing-part collection; in
crates/engine/src/game/ability_rw.rs at lines 2698-2703 and 2767-2773, include
decline in legacy-reference traversal and replacement-helper inspection; and in
crates/phase-ai/src/policies/x_reference.rs at lines 170-172, detect X
references in decline as well as execute.

Source: Path instructions

_ => true,
})
}
Expand Down
34 changes: 32 additions & 2 deletions crates/engine/src/game/effects/effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,21 @@ pub fn resolve(
.or(duration.clone())
.unwrap_or(Duration::UntilEndOfTurn);

// CR 611.2a: The `UntilEndOfTurn` above is only a fallback for a
// stated-duration-less grant. A grant that states NO duration and grants
// ONLY an object-hosted replacement (the "it gains 'If ~ would leave the
// battlefield, exile it instead'" reanimation rider — Geth, Thane of
// Contracts; Llanowar Greenwidow; Realmbreaker; Spirit-Sister's Call)
// must last as long as the affected object exists, so promote that def to
// Duration::Permanent. It then survives its granting source leaving
// (`prune_host_left_effects` only prunes UntilHostLeavesPlay) and is
// cleaned up when the reanimated object itself leaves
// (`prune_affected_object_left_effects`). A STATED duration arrives on the
// wrapper (Elemental Expressionist's "Until end of turn" lives on the
// GenericEffect application, so `duration` is `Some` and the fallback is
// not taken) and is left EOT-confined, lapsing at cleanup (CR 514.2).
let duration_from_fallback = ability.duration.is_none() && duration.is_none();

for static_def in static_abilities {
// CR 611.2d: A continuous effect's variable (X) is determined once,
// on resolution. Snapshot resolution-context quantity refs (e.g.
Expand Down Expand Up @@ -94,6 +109,21 @@ pub fn resolve(
}
}
let static_def = &static_def;
// CR 611.2a: promote a fallback-duration replacement-only grant to
// Permanent (see `duration_from_fallback` above); every other def
// keeps the resolved `dur`. Mixed defs (a replacement grant alongside
// any other modification) are NOT promoted.
let def_dur = if duration_from_fallback
&& !static_def.modifications.is_empty()
&& static_def
.modifications
.iter()
.all(|m| matches!(m, ContinuousModification::GrantReplacement { .. }))
{
Duration::Permanent
} else {
dur.clone()
};
// CR 603.4 + CR 608.2h + CR 611.2d: An in-effect "if <condition>"
// carried by a `StaticDefinition` (Odric, Lunarch Marshal:
// "creatures you control gain first strike ... if a creature you
Expand All @@ -112,9 +142,9 @@ pub fn resolve(
}
let mut snapshotted = static_def.clone();
snapshotted.condition = None;
register_transient_effect(state, ability, &snapshotted, target.as_ref(), &dur);
register_transient_effect(state, ability, &snapshotted, target.as_ref(), &def_dur);
} else {
register_transient_effect(state, ability, static_def, target.as_ref(), &dur);
register_transient_effect(state, ability, static_def, target.as_ref(), &def_dur);
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions crates/engine/src/game/layers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6659,6 +6659,24 @@ fn apply_continuous_effect_filtered(
obj.static_definitions.push(*definition.clone());
}
}
// CR 614.1a + CR 614.6 + CR 613.1f: Grant an object-hosted replacement
// to the recipient. Mirror of `GrantStaticAbility` — push the cloned
// `ReplacementDefinition` onto `obj.replacement_definitions` so the
// granted replacement fires as a genuine replacement effect (its
// `valid_card: SelfRef` binds to this recipient object). Re-derived
// each layer pass (`obj.replacement_definitions` was reset to base at
// the start of the pass); structural-equality dedup keeps repeated
// grants (multiple sources, or a single static parsed twice)
// idempotent, matching the GrantTrigger / GrantStaticAbility invariant.
ContinuousModification::GrantReplacement { replacement } => {
if !obj
.replacement_definitions
.iter_all()
.any(|rd| rd == replacement.as_ref())
{
obj.replacement_definitions.push(*replacement.clone());
}
}
ContinuousModification::AddStaticMode { mode } => {
// CR 509.1b + CR 105.4 + CR 609.6 (issue #327): When the
// granted static mode carries an `IsChosenColor` filter prop,
Expand Down
3 changes: 3 additions & 0 deletions crates/engine/src/game/printed_cards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,9 @@ fn walk_continuous_mod(modification: &ContinuousModification, out: &mut Vec<Stri
match modification {
ContinuousModification::GrantAbility { definition } => walk_ability_def(definition, out),
ContinuousModification::GrantTrigger { trigger } => walk_trigger(trigger, out),
ContinuousModification::GrantReplacement { replacement } => {
walk_replacement(replacement, out)
}
ContinuousModification::GrantStaticAbility { definition } => walk_static(definition, out),
ContinuousModification::CopyValues { values, .. } => walk_copiable_values(values, out),
// Remaining modifications carry no nested ability/effect carriers.
Expand Down
3 changes: 3 additions & 0 deletions crates/engine/src/game/quantity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,9 @@ pub(crate) fn continuous_modification_dynamic_quantity(
| ContinuousModification::GrantAllActivatedAbilitiesOf { .. }
| ContinuousModification::GrantAllTriggeredAbilitiesOf { .. }
| ContinuousModification::GrantTrigger { .. }
// A granted object-hosted replacement carries no `QuantityExpr`
// magnitude — its `execute` (ChangeZone→Exile) has no dynamic value.
| ContinuousModification::GrantReplacement { .. }
| ContinuousModification::RemoveAllAbilities
| ContinuousModification::AddType { .. }
| ContinuousModification::RemoveType { .. }
Expand Down
4 changes: 4 additions & 0 deletions crates/engine/src/parser/oracle_effect/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9055,6 +9055,8 @@ fn apply_where_x_continuous_modification(
| ContinuousModification::AddColor { .. }
| ContinuousModification::AddStaticMode { .. }
| ContinuousModification::GrantStaticAbility { .. }
// Granted object-hosted replacement: no where-X / anaphoric magnitude.
| ContinuousModification::GrantReplacement { .. }
| ContinuousModification::SwitchPowerToughness
| ContinuousModification::AssignDamageFromToughness
| ContinuousModification::AssignDamageAsThoughUnblocked
Expand Down Expand Up @@ -9154,6 +9156,8 @@ fn rebind_target_anaphor_continuous_modification(modification: &mut ContinuousMo
| ContinuousModification::AddColor { .. }
| ContinuousModification::AddStaticMode { .. }
| ContinuousModification::GrantStaticAbility { .. }
// Granted object-hosted replacement: no where-X / anaphoric magnitude.
| ContinuousModification::GrantReplacement { .. }
| ContinuousModification::SwitchPowerToughness
| ContinuousModification::AssignDamageFromToughness
| ContinuousModification::AssignDamageAsThoughUnblocked
Expand Down
Loading
Loading