Skip to content

fix(engine): scan the Aura's own enchant-zone when choosing a host on non-cast entry#6072

Merged
matthewevans merged 1 commit into
phase-rs:mainfrom
rykerwilliams:fix/aura-attachment-graveyard-host-search
Jul 17, 2026
Merged

fix(engine): scan the Aura's own enchant-zone when choosing a host on non-cast entry#6072
matthewevans merged 1 commit into
phase-rs:mainfrom
rykerwilliams:fix/aura-attachment-graveyard-host-search

Conversation

@rykerwilliams

Copy link
Copy Markdown
Contributor

Summary

CR 303.4f: when an Aura enters the battlefield by any means other than resolving as a spell (e.g. returning from exile) and the effect putting it there doesn't specify a host, the controller chooses a legal object per the Aura's current Enchant ability. legal_aura_attachment_targets computed that candidate list from state.battlefield only, unconditionally — even though both call sites already had the Aura's enchant_filter in hand, and that filter is the single source of truth for which zone(s) it can legally target.

An Enchant ability that targets "a creature card in a graveyard" (Animate Dead, Dance of the Dead), "an instant card in a graveyard" (Spellweaver Volute), or "a card in your hand" (Don't Worry About It) could therefore never find a legal host through this path, even when one genuinely existed — the consult always saw zero candidates and the Aura stayed stranded in its current zone (CR 303.4g).

Fix: rewrite legal_aura_attachment_targets to resolve the search zone(s) from enchant_filter.extract_zones() (defaulting to the battlefield when the filter imposes no zone constraint — the common case, unaffected), flat-map zone_object_ids over those zones, and filter with matches_target_filter — mirroring the existing object_count_matching_ids pattern in game/quantity.rs. Uses matches_target_filter rather than the spell-targeting enumerator: CR 115.1b says an Aura permanent doesn't target anything, so hexproof/shroud must never remove a candidate here, only an actual Enchant restriction can. Preserves the pre-existing "Enchant player" branch, the CR 303.4d self-enchant exclusion, and the can_attach_to_object legality gate unchanged.

Incidentally also now excludes phased-out permanents from the default battlefield case (CR 702.26b) via zone_object_ids's existing filter — strictly more correct, not a behavior regression.

This is the fix that closes the final leg of the Worldgorger Dragon self-loop combo (mass-return mechanism fixed separately in #6055): the returning Animate Dead/Necromancy Aura can now find WGD sitting in the graveyard and re-attach to it, rather than staying stranded in exile.

Test plan

  • New test: a reanimator-class Aura re-entering from exile with a legal graveyard host finds it, attaches, and its ETB reanimation chain fires end-to-end (creature reaches the battlefield under the correct controller)
  • New test: an ordinary battlefield-scoped Aura is unaffected — attaches iff a legal battlefield host exists (both a has-host positive and a no-host negative in one fixture)
  • New test: a graveyard-scoped Aura with NO legal graveyard creature correctly stays in exile (CR 303.4g) — non-vacuous, a non-matching card is present and enumerated/rejected by the scan
  • Zero regression: full exile_return_tests (16/16), animate_dead-filtered casting tests (9/9), necromancy-filtered casting tests (5/5)
  • Live-revert-and-rerun proof (done independently by two people): reverting just the scan-logic rewrite reproduces the exact failure (Aura stuck in exile); restoring is green
  • cargo fmt --all, cargo clippy -p engine --all-targets -- -D warnings clean
  • CR-annotation gate clean (115.1b, 303.4d, 303.4f, 303.4g, 610.3a, 702.11, 702.18, 702.26b all verified against the Comprehensive Rules)

… non-cast entry

CR 303.4f: when an Aura enters the battlefield by any means other than
resolving as a spell (e.g. returning from exile) and the effect putting it
there doesn't specify a host, the controller chooses a legal object per the
Aura's current Enchant ability. legal_aura_attachment_targets computed that
candidate list from state.battlefield only, unconditionally - even though
both call sites already had the Aura's enchant_filter in hand, and that
filter is the single source of truth for which zone(s) it can legally
target.

An Enchant ability that targets "a creature card in a graveyard" (Animate
Dead, Dance of the Dead), "an instant card in a graveyard" (Spellweaver
Volute), or "a card in your hand" (Don't Worry About It) could therefore
never find a legal host through this path, even when one genuinely existed
- the consult always saw zero candidates and the Aura stayed stranded in
its current zone (CR 303.4g).

Rewrites legal_aura_attachment_targets to resolve the search zone(s) from
enchant_filter.extract_zones() (defaulting to the battlefield when the
filter imposes no zone constraint - the common case, unaffected), flat-map
zone_object_ids over those zones, and filter with matches_target_filter -
mirroring the existing object_count_matching_ids pattern in game/quantity.rs.
Uses matches_target_filter rather than the spell-targeting enumerator: CR
115.1b says an Aura permanent doesn't target anything, so hexproof/shroud
must never remove a candidate here, only an actual Enchant restriction can.
Preserves the pre-existing "Enchant player" branch, the CR 303.4d
self-enchant exclusion, and the can_attach_to_object legality gate
unchanged. Incidentally also now excludes phased-out permanents from the
default battlefield case (CR 702.26b) via zone_object_ids' existing filter -
strictly more correct, not a behavior regression.

This is the fix that closes the final leg of the Worldgorger Dragon self-loop
combo (mass-return mechanism fixed separately in phase-rs#6055): the returning
Animate Dead/Necromancy Aura can now find WGD sitting in the graveyard and
re-attach to it, rather than staying stranded in exile.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

rykerwilliams added a commit to rykerwilliams/phase that referenced this pull request Jul 17, 2026
@github-actions

Copy link
Copy Markdown

Parse changes introduced by this PR

✓ No card-parse changes detected.

@matthewevans matthewevans self-assigned this Jul 17, 2026

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maintainer review: correct zone-authority fix with discriminating exile-return coverage; required checks are green.

@matthewevans matthewevans added the bug Bug fix label Jul 17, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 17, 2026
@matthewevans matthewevans removed their assignment Jul 17, 2026
Merged via the queue into phase-rs:main with commit 0f69b87 Jul 17, 2026
13 checks passed
rykerwilliams added a commit to rykerwilliams/phase that referenced this pull request Jul 17, 2026
rykerwilliams added a commit to rykerwilliams/phase that referenced this pull request Jul 17, 2026
merge origin/main (via a regular merge, not rebase, to preserve this PR's
already-reviewed history) picks up phase-rs#6072, which fixed the returning Animate
Dead Aura's graveyard-host search - the "known gap" this test's SCOPE
section documented is now closed.

Replaces the "stays in Exile" assertion with the actual now-correct outcome:
the Aura reaches Zone::Battlefield and attaches to WGD-in-graveyard (CR
303.4f). Drives the rest of the pipeline to completion - the Aura's
re-entry ETB and Piranha Marsh's ETB co-trigger (CR 603.3b OrderTriggers),
Piranha's target selection and life-loss resolve first (CR 119.3, exact
life-total delta), then the Aura's ETB resolves and reanimates
Worldgorger Dragon a second time, re-attaching the Aura - closing one full
cycle of the "Immortal Worldgorger Dragon" combo end-to-end.

Also corrects a pre-existing CR citation: CR 610.3a ("the object doesn't
move" edge case) was cited for the plain-land/Piranha-Marsh "return to the
battlefield" behavior, which is the general CR 610.3 return rule, not the
610.3a edge case. Fixed in all 4 places this test cited it.
matthewevans added a commit to andriypolanski/phase that referenced this pull request Jul 17, 2026
…ability pass (phase-rs#6055)

* fix(engine): recognize mass-exile in the ETB-exile/LTB-return linked-ability pass

CR 607.1 + CR 607.2a + CR 406.6: the engine already models "exile X, return
X when the source leaves" generally (Journey to Nowhere / Oblivion Ring
class) via a CR 610.3 "until"-duration vehicle - detect_etb_exile_ltb_return
stamps Duration::UntilHostLeavesPlay onto the ETB exile at parse time, which
makes zone_pipeline create a persistent ExileLinkKind::UntilSourceLeaves
link per exiled object, and check_exile_returns (run every priority pass)
performs the automatic return when the source leaves the battlefield -
independent of whether the card's own printed LTB trigger ever resolves.

trigger_is_etb_exile_pending_duration only matched the single-target
Effect::ChangeZone->Exile, never the mass Effect::ChangeZoneAll->Exile that
Worldgorger Dragon's "exile all other permanents you control" (and Realm
Razer's "exile all lands") parses to. Their ETBs never got the duration
stamp, so the whole downstream machinery stayed dormant and their LTB
"return the exiled cards" triggers returned nothing.

Widens the matches! to accept both shapes. apply_etb_exile_ltb_return needs
no change - it only stamps execute.duration, never inspects execute.effect.
Blast radius is bounded: detect_etb_exile_ltb_return only pairs an ETB
exile with a card that also has a matching LTB-return trigger, so the
widened arm cannot fire on a card that just permanently exiles things.

Fixes the mass-return of non-Aura permanents in the Worldgorger Dragon +
Animate Dead self-loop combo (a land returns untapped; Piranha Marsh's
"target player loses 1 life" ETB refires) and fully fixes Realm Razer.
The final leg that would close the infinite loop - the returning Animate
Dead Aura re-attaching to Worldgorger-Dragon-in-graveyard - is NOT fixed
here: the as-enters aura-host scan (legal_aura_attachment_targets) is
battlefield-only and can't find a graveyard-resident host (CR 303.4f/g),
so the Aura's re-entry is denied. That graveyard-host aura-attach gap is a
separate, wider follow-up, tracked on the fork's internal worklist.

* fix(engine): don't synthesize an exile-link return when the LTB rider carries an entry modifier

CR 610.3: the automatic ExileLink::UntilSourceLeaves return path performs a
plain, unmodified zone move - it can't carry a printed entry rider like
"return the exiled cards to the battlefield tapped." Realm Razer's LTB
return has exactly that rider (enter_tapped: true), so pairing it with the
mass-exile synthesis from the previous commit would silently return its
lands untapped, contradicting its printed text.

trigger_is_ltb_return now also requires the return effect's entry modifiers
(enter_tapped, enters_attacking, enter_transformed, owner_library,
enters_under, enter_with_counters, conditional_enter_with_counters,
face_down_profile, enters_modified_if) to all be at their no-op default.
Realm Razer's ETB is therefore no longer paired and is left unstamped -
correctly excluded rather than silently dropping its tapped modifier - until
a modifier-carrying return mechanism exists. Worldgorger Dragon's LTB has no
modifiers and is unaffected; it remains fixed.

Adds a negative parser test asserting Realm Razer's mass-exile ETB is NOT
stamped UntilHostLeavesPlay, and narrows the existing positive test to
Worldgorger Dragon only. Updates the doc comments this touches (relation.rs,
oracle.rs, casting_tests.rs) to drop the now-inaccurate "Realm Razer is
fixed" claims and adds the CR 610.3 annotation prefix requested in review.

Addresses maintainer review feedback on phase-rs#6055 (matthewevans): the mass-exile
widening changed Realm Razer's required return semantics without carrying
its tapped rider through the automatic return path.

* test(engine): route the WGD test's off-zone keyword check through the state-scoped authority

CI's engine-authorities gate flagged a raw obj.keywords.iter() query on an
Exile-zone object - the object-scoped read is only authoritative for
battlefield objects and silently misses off-zone keyword grants for hand/
graveyard/exile.

Adds an explicit object_has_effective_keyword_kind(state, aura_id,
KeywordKind::Enchant) check (the off-zone-aware authority) confirming the
exiled Aura still carries an Enchant keyword at all, ahead of the existing
raw structural inspection of which Enchant variant it is (its inner
TargetFilter) - something no public authority exposes as a value, so that
one line keeps the // allow-raw-authority annotation the gate's own escape
hatch documents.

* test(engine): close the Worldgorger Dragon self-loop test's final leg

merge origin/main (via a regular merge, not rebase, to preserve this PR's
already-reviewed history) picks up phase-rs#6072, which fixed the returning Animate
Dead Aura's graveyard-host search - the "known gap" this test's SCOPE
section documented is now closed.

Replaces the "stays in Exile" assertion with the actual now-correct outcome:
the Aura reaches Zone::Battlefield and attaches to WGD-in-graveyard (CR
303.4f). Drives the rest of the pipeline to completion - the Aura's
re-entry ETB and Piranha Marsh's ETB co-trigger (CR 603.3b OrderTriggers),
Piranha's target selection and life-loss resolve first (CR 119.3, exact
life-total delta), then the Aura's ETB resolves and reanimates
Worldgorger Dragon a second time, re-attaching the Aura - closing one full
cycle of the "Immortal Worldgorger Dragon" combo end-to-end.

Also corrects a pre-existing CR citation: CR 610.3a ("the object doesn't
move" edge case) was cited for the plain-land/Piranha-Marsh "return to the
battlefield" behavior, which is the general CR 610.3 return rule, not the
610.3a edge case. Fixed in all 4 places this test cited it.

* fix(engine): mark a modifier-bearing linked return unsupported, not falsely green

CR 610.3: for the ETB-exile/LTB-return linked-ability class, the actual
return mechanism is the generic ExileLink::UntilSourceLeaves/
check_exile_returns path, driven purely by Duration::UntilHostLeavesPlay on
the ETB - it performs an unmodified zone move and never reads anything from
the LTB trigger's own printed effect. When the LTB return carries an entry
rider the automatic path can't apply (Realm Razer's "return the exiled
cards to the battlefield tapped"), the prior commit correctly declined to
stamp the duration - but left no trace that this card is now unsupported,
so it still parsed as a fully-formed, apparently-supported ETB/LTB pair.

Splits trigger_is_ltb_return into a shape-only check plus the existing
modifier check (trigger_is_ltb_return itself keeps its exact prior combined
behavior - zero change for Journey to Nowhere / Oblivion Ring / Worldgorger
Dragon), and adds trigger_is_ltb_return_with_entry_modifier for the
shape-matched-but-modifier-rejected class. Parameterizes
DocumentRelationIr::EtbExileLtbReturn with an outcome (DurationStamped, or
ModifierUnsupported carrying the return's verbatim fragment text - captured
at detect time since the applier has no access to the item list) rather
than adding a sibling relation variant, per this module's own documented
convention. When the outcome is ModifierUnsupported, attaches
Effect::unimplemented("modifier_bearing_linked_return", fragment) to the
LTB trigger's own execute chain, so coverage correctly reports the gap
instead of the card showing as falsely supported.

The new path is reachable only for a return the parser genuinely captured
an entry modifier on, so it cannot false-flag a card that's actually fully
supported - it only marks a class that was already broken (no duration
stamp, no return) as visibly, honestly broken.

Addresses maintainer review feedback on phase-rs#6055 (matthewevans, repeated
across 5 rounds): mark this modifier-bearing linked-return combination
unsupported through Effect::unimplemented and add a coverage-level
regression test proving the gap is visible to coverage, not just that a
duration field is None.

---------

Co-authored-by: Matt Evans <1388610+matthewevans@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants