Fix Necromancy ETB reanimation (#640)#5778
Merged
matthewevans merged 1 commit intoJul 14, 2026
Merged
Conversation
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 14, 2026
Parse changes introduced by this PR · 1 card(s), 2 signature(s) (baseline: main
|
matthewevans
force-pushed
the
fix/necromancy-etb-reanimation
branch
from
July 14, 2026 04:04
6d4944f to
03b00f5
Compare
matthewevans
force-pushed
the
fix/necromancy-etb-reanimation
branch
from
July 14, 2026 04:20
03b00f5 to
74eb0e4
Compare
…ture (phase-rs#640) Necromancy's ETB ability was parsing to an empty Continuous static via the Animate Dead-class combinator, which only handles the "becomes an Aura" keyword-adoption clause and silently dropped the entire targeted reanimation sentence. Adds a new whole-body recognizer for the "grant" sub-class (plain Enchantment ETB grants Aura-hood and targets its own reanimation, as opposed to Animate Dead's cast-time Enchant-keyword targeting), generalizing the shared reanimator-Aura chain builder to cover both shapes via a parser-internal EnchantGrantShape selector. Also corrects several 701.17a (Mill) -> 701.21a (Sacrifice) CR mis-citations found in the touched functions. Closes phase-rs#640
matthewevans
force-pushed
the
fix/necromancy-etb-reanimation
branch
from
July 14, 2026 04:39
74eb0e4 to
16134f5
Compare
matthewevans
approved these changes
Jul 14, 2026
matthewevans
left a comment
Member
There was a problem hiding this comment.
Approved — Necromancy now uses its ETB trigger's real graveyard-creature target.
✅ Clean
- The change extends the existing reanimator-Aura parser seam and parameterizes the shared chain for the printed-Aura and grant-Aura shapes.
- The runtime tests exercise the parser, trigger, zone change, attachment, delayed sacrifice, cross-controller target, and fizzle paths; each would fail if the new dispatch were removed.
- The Oracle text and sacrifice citation were independently checked against Scryfall and the current Comprehensive Rules.
Recommendation: approved for the merge queue.
rykerwilliams
added a commit
to rykerwilliams/phase
that referenced
this pull request
Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Necromancy's ETB ability ("it becomes an Aura with "...". Put target creature card from a graveyard onto the battlefield under your control and attach this enchantment to it...") was parsing via the Animate Dead-class combinator (
try_parse_reanimator_aura_etb_effect), which only handles the quoted keyword-adoption clause and silently dropped the entire targeted-reanimation sentence — noTargetFilter, no zone reference, no reanimation effect node anywhere in the AST. This is exactly why #640 reports zero targetable creatures: no target-bearing effect was ever built.Root cause: Necromancy is printed as a plain Enchantment (no
Enchantkeyword line), unlike Animate Dead which is cast as an Aura and targets at cast time via its printed Enchant restriction. Necromancy's reanimation target is a genuine CR 603.3d stack target chosen when its own ETB triggered ability goes on the stack — a structurally different shape that needed its own recognizer, not a parameter tweak to the existing one.Fix
try_parse_reanimator_aura_grant_etb_effect+parse_reanimator_aura_grant_etb_shape, a new whole-body, fail-closed nom recognizer for the "grant" sub-class (plain Enchantment ETB grants Aura-hood + targets its own reanimation), wired intooracle_trigger.rs's dispatch chain alongside the existing Animate Dead arm.build_reanimator_aura_etb_chain→build_aura_attach_chain), parameterized by a new parser-internalEnchantGrantShapeenum (Swapfor Animate Dead's existing RemoveKeyword+AddKeyword behavior,GrantOnlyfor Necromancy's AddSubtype+AddKeyword — since Necromancy has no prior Aura subtype/Enchant restriction to swap) and aroot_target: TargetFilterparameter (Animate Dead keeps its existingAttachedToreference; Necromancy gets a genuinely-parsed target).701.17a(Mill) →701.21a(Sacrifice) CR mis-citations found in the touched functions.A confirmed 5-card class shares a similar "becomes an Aura with X, [producer effect], attach self to result" template (Necromancy, Last Voyage of the Uncharted, Cloudform, Lightform, Rageform). This PR scopes to Necromancy; the other 4 use an unquoted restriction clause and different producer/sacrifice phrasing, so they correctly stay
Unimplementedand are tracked separately for a future follow-up.CR references
Verification
cargo fmt --all— cleancargo clippy --all-targets -- -D warnings— cleanparse_targetbuilding-block regression test, 4 runtime pipeline tests covering full reanimation+attach+SBA-survival, cross-controller reanimation, delayed sacrifice, and CR 608.2b fizzle) — all passcargo semantic-audit/cargo coverage— not run locally (cold MTGJSON download didn't complete in this worktree); relying on CI for corpus-level signalCloses #640