From 87020c6091be17346d19c85099959505a11b20c7 Mon Sep 17 00:00:00 2001 From: Matt Evans <1388610+matthewevans@users.noreply.github.com> Date: Wed, 10 Jun 2026 08:07:26 -0700 Subject: [PATCH 01/11] fix(engine): route declined-cipher, declined-madness, and legend-rule losers to graveyard through zone pipeline (CR 608.2n / 702.35a / 704.5j / 614.6) These three graveyard-destination moves delivered via raw move_to_zone, never proposing the inner ZoneChange, so board-wide Moved graveyard->exile redirects (Rest in Peace / Leyline of the Void) silently dropped: - cipher decline (CR 608.2n): the resolving cipher card -> owner's graveyard now routes through SpellResolutionDefault; handle_encode_choice returns the ZoneMoveResult so the caller surfaces a parked CR 616.1 prompt instead of clobbering it with Priority. - madness decline (CR 702.35a): the exiled card -> owner's graveyard now routes through move_object; the arm evaluates to the parked WaitingFor on pause so the post-action pipeline is skipped. - legend rule (CR 704.5j): the losing legends -> graveyards now route through move_objects_simultaneously (CR 603.10a co-departure stamp); a mid-batch CR 616.1 choice parks the prompt and stashes the tail. CR grep (docs/MagicCompRules.txt): 608.2n spell goes to graveyard on resolution; 702.35a madness; 704.5j legend rule; 614.6 replaced event never happens; 603.10a simultaneous leaves-battlefield. --- crates/engine/src/game/cipher.rs | 32 +++++++++-- crates/engine/src/game/engine.rs | 40 +++++++++++--- .../src/game/engine_resolution_choices.rs | 53 +++++++++++++++---- 3 files changed, 105 insertions(+), 20 deletions(-) diff --git a/crates/engine/src/game/cipher.rs b/crates/engine/src/game/cipher.rs index d509596741..261defc497 100644 --- a/crates/engine/src/game/cipher.rs +++ b/crates/engine/src/game/cipher.rs @@ -25,6 +25,7 @@ //! encode is an independent link. use super::triggers::{PendingTrigger, PendingTriggerContext}; +use super::zone_pipeline::{self, ZoneMoveRequest, ZoneMoveResult}; use crate::types::ability::{Effect, ResolvedAbility, TargetFilter, TargetRef}; use crate::types::card_type::CoreType; use crate::types::events::GameEvent; @@ -130,19 +131,40 @@ pub fn begin_encode_choice(state: &mut GameState, card_id: ObjectId, controller: /// card on that creature (exile + link); `None` — or a creature that is no /// longer a legal host — declines, routing the card to its owner's graveyard /// (CR 608.2n). The chosen creature is re-validated against the current board. -pub fn handle_encode_choice( +/// +/// Returns the [`ZoneMoveResult`] of the decline move so the caller knows +/// whether a CR 616.1 replacement-ordering choice parked a prompt (the declined +/// card hit a graveyard→exile redirect) — the encode-accept path never pauses +/// (exile is not a `Moved` redirect destination) and reports `Done`. +pub(crate) fn handle_encode_choice( state: &mut GameState, card_id: ObjectId, creature: Option, events: &mut Vec, -) { +) -> ZoneMoveResult { let controller = state.objects.get(&card_id).map(|o| o.controller); let chosen = creature .filter(|id| controller.is_some_and(|c| legal_encode_creatures(state, c).contains(id))); match chosen { - Some(creature_id) => finish_encode(state, card_id, creature_id, events), - // CR 608.2n: a declined cipher card is put into its owner's graveyard. - None => super::zones::move_to_zone(state, card_id, Zone::Graveyard, events), + Some(creature_id) => { + finish_encode(state, card_id, creature_id, events); + ZoneMoveResult::Done + } + // CR 608.2n + CR 614.6: a declined cipher card is the resolving spell's + // card being put into its owner's graveyard — route it through the + // zone-change pipeline so a `Moved` graveyard→exile redirect (Rest in + // Peace / Leyline of the Void) fires on it. The raw `move_to_zone` never + // proposed the inner ZoneChange, silently dropping those redirects. The + // spell's card moves itself on resolution, so the cause is + // `SpellResolutionDefault` (no external source). A CR 616.1 ordering + // choice (two simultaneous redirects) is parked centrally by + // `move_object`; the caller surfaces the parked prompt instead of + // returning to priority. + None => zone_pipeline::move_object( + state, + ZoneMoveRequest::spell_resolution_default(card_id, Zone::Graveyard), + events, + ), } } diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 174a487298..059a6aea98 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -4113,14 +4113,42 @@ fn apply_action( GameAction::DecideOptionalEffect { accept: false }, ) => { let p = *player; - super::zones::move_to_zone(state, *object_id, Zone::Graveyard, &mut events); - state.waiting_for = WaitingFor::Priority { player: p }; - super::engine_priority::run_post_action_pipeline( + let obj = *object_id; + // CR 702.35a + CR 614.6: a declined madness card is put into its + // owner's graveyard from exile — route it through the zone-change + // pipeline so a `Moved` graveyard→exile redirect (Rest in Peace / + // Leyline of the Void) fires on it. The raw `move_to_zone` never + // proposed the inner ZoneChange, silently dropping those redirects. + // The card moves itself (no external source), so it anchors its own + // attribution. A CR 616.1 ordering choice (two simultaneous + // redirects) is parked centrally by `move_object`; bail before + // overwriting `waiting_for` / running the post-action pipeline so the + // parked prompt is not clobbered (its resume runs the pipeline). + match super::zone_pipeline::move_object( state, + super::zone_pipeline::ZoneMoveRequest::effect(obj, Zone::Graveyard, obj), &mut events, - &WaitingFor::Priority { player: p }, - true, - )? + ) { + super::zone_pipeline::ZoneMoveResult::Done => { + state.waiting_for = WaitingFor::Priority { player: p }; + super::engine_priority::run_post_action_pipeline( + state, + &mut events, + &WaitingFor::Priority { player: p }, + true, + )? + } + // The graveyard move paused on a CR 616.1 ordering choice; the + // parked prompt is already in `state.waiting_for`. Evaluate the + // arm to it (non-`Priority`), so the post-match block skips the + // post-action pipeline and the prompt is surfaced intact — its + // replacement-choice resume finishes the move and re-runs the + // pipeline. + super::zone_pipeline::ZoneMoveResult::NeedsChoice(_) + | super::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => { + state.waiting_for.clone() + } + } } (waiting_for, action) if engine_resolution_choices::handles(waiting_for) => { match engine_resolution_choices::handle_resolution_choice( diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index 9eba61c6be..4b165f4c58 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -2859,12 +2859,34 @@ pub(super) fn handle_resolution_choice( .filter(|&&id| id != keep) .copied() .collect(); - for id in to_remove { - zones::move_to_zone(state, id, Zone::Graveyard, events); + // CR 704.5j + CR 614.6 + CR 603.10a: the losing legends are put into + // their owners' graveyards simultaneously as a single state-based + // action. Route them through the zone-change pipeline so a `Moved` + // graveyard→exile redirect (Rest in Peace / Leyline of the Void) + // fires on each — the raw `move_to_zone` never proposed the inner + // ZoneChange, silently dropping those redirects. `move_objects_ + // simultaneously` co-stamps the departures so leaves-the-battlefield + // observers see each other (CR 603.10a). The legends move themselves + // as an SBA (no external source), so each anchors its own + // attribution. A CR 616.1 ordering choice mid-batch parks the prompt + // and stashes the undelivered tail; surface the parked prompt instead + // of clobbering it with `Priority`. + let reqs: Vec<_> = to_remove + .into_iter() + .map(|id| { + crate::game::zone_pipeline::ZoneMoveRequest::effect(id, Zone::Graveyard, id) + }) + .collect(); + match crate::game::zone_pipeline::move_objects_simultaneously(state, reqs, events) { + crate::game::zone_pipeline::BatchMoveResult::Done => { + ResolutionChoiceOutcome::WaitingFor(WaitingFor::Priority { + player: state.active_player, + }) + } + crate::game::zone_pipeline::BatchMoveResult::NeedsChoice => { + ResolutionChoiceOutcome::WaitingFor(state.waiting_for.clone()) + } } - ResolutionChoiceOutcome::WaitingFor(WaitingFor::Priority { - player: state.active_player, - }) } // CR 702.140c + CR 730.2a: The mutate spell's controller chose whether the // spell merges on top of or under the target creature. `merge::handle_mutate_ @@ -2884,10 +2906,23 @@ pub(super) fn handle_resolution_choice( // then resolution is complete — return to priority so the resulting zone // change's triggers/SBAs are processed. (WaitingFor::CipherEncodeChoice { card_id, .. }, GameAction::CipherEncode { creature }) => { - crate::game::cipher::handle_encode_choice(state, card_id, creature, events); - ResolutionChoiceOutcome::WaitingFor(WaitingFor::Priority { - player: state.active_player, - }) + // CR 616.1: a declined cipher card hitting a graveyard→exile redirect + // can surface a replacement-ordering choice, which `handle_encode_choice` + // parks centrally via `move_object`. Surface the parked prompt instead + // of clobbering it with `Priority`; otherwise resolution is complete, + // so return to priority and let the resulting zone change's triggers / + // SBAs process. + match crate::game::cipher::handle_encode_choice(state, card_id, creature, events) { + crate::game::zone_pipeline::ZoneMoveResult::Done => { + ResolutionChoiceOutcome::WaitingFor(WaitingFor::Priority { + player: state.active_player, + }) + } + crate::game::zone_pipeline::ZoneMoveResult::NeedsChoice(_) + | crate::game::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => { + ResolutionChoiceOutcome::WaitingFor(state.waiting_for.clone()) + } + } } // CR 903.9a: Owner decides whether to return their commander to the command zone. // Accept = move to command zone; Decline = leave in current zone (marked as From 2becb86dd0a1e9ee43183dc795dbc134b7b16369 Mon Sep 17 00:00:00 2001 From: Matt Evans <1388610+matthewevans@users.noreply.github.com> Date: Wed, 10 Jun 2026 08:17:11 -0700 Subject: [PATCH 02/11] fix(engine): route Attraction-open and ninjutsu battlefield entries through zone pipeline (CR 614.1c) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both battlefield entries delivered via raw move_to_zone, skipping the pipeline delivery tail that applies enters-with-counters statics (StaticMode::EntersWithAdditionalCounters — Hardened Scales / Conclave Mentor 'creatures you control enter with an additional +1/+1 counter' class). The raw mover applies those statics nowhere, so an opened Attraction or a ninja entering via ninjutsu silently missed them. Route both through zone_pipeline::move_object. A battlefield-entry pause (CR 616.1 multi-redirect ordering / CR 303.4f aura host / counter- replacement) is not reachable for these object classes in the supported pool: an Attraction is a non-Aura artifact, a ninja is a non-Aura creature, and no supported entry surfaces a choice. The bail is a safety guard, not a resume path — ninjutsu's post-entry combat placement (CR 702.49c) cannot resume across a pause, so on the unreachable pause we stop with the prompt parked rather than act over parked state. Attribution self-anchors (CR 400.7; the raw move recorded no source). CR grep (docs/MagicCompRules.txt): 614.1c enters-with statics; 702.49c ninjutsu combat placement; 616.1 multi-replacement ordering. --- crates/engine/src/game/attractions.rs | 25 +++++++++++++++++++-- crates/engine/src/game/keywords.rs | 32 +++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/game/attractions.rs b/crates/engine/src/game/attractions.rs index f3d6334f84..c77a05c1b9 100644 --- a/crates/engine/src/game/attractions.rs +++ b/crates/engine/src/game/attractions.rs @@ -14,7 +14,6 @@ use crate::types::zones::Zone; use super::effects::roll_die; use super::game_object::GameObject; -use super::zones; use crate::types::ability::EffectError; /// CR 717.1: Default lit numbers when card data omits variant lights (1 and 6 are always lit). @@ -54,7 +53,29 @@ pub fn open_attractions( // as many as possible and ignore the impossible remainder. break; }; - zones::move_to_zone(state, object_id, Zone::Battlefield, events); + // CR 614.1c: route the Attraction's battlefield entry through the + // zone-change pipeline so the delivery tail applies enters-with-counters + // statics (e.g. an artifact-scoped "enters with an additional counter" + // static) — the raw `move_to_zone` skipped that tail, so an opened + // Attraction never received them. CR 400.7 attributes the entry to the + // opened object itself (the pre-pipeline raw move recorded no source). + // + // CR 616.1 / CR 303.4f: a battlefield-entry pause is not reachable for + // an Attraction — it is a non-Aura artifact (no aura-host choice), and + // no `Moved` redirect or counter-replacement that targets an Attraction + // entry surfaces a CR 616.1 ordering choice in the supported pool. The + // bail keeps the loop safe by construction: on a pause the prompt is + // parked centrally by `move_object`, so stop opening rather than + // emitting `AttractionOpened` over a parked state. + match super::zone_pipeline::move_object( + state, + super::zone_pipeline::ZoneMoveRequest::effect(object_id, Zone::Battlefield, object_id), + events, + ) { + super::zone_pipeline::ZoneMoveResult::Done => {} + super::zone_pipeline::ZoneMoveResult::NeedsChoice(_) + | super::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => break, + } if let Some(obj) = state.objects.get_mut(&object_id) { obj.in_attraction_deck = false; } diff --git a/crates/engine/src/game/keywords.rs b/crates/engine/src/game/keywords.rs index 77b6bc6694..3acc7521d6 100644 --- a/crates/engine/src/game/keywords.rs +++ b/crates/engine/src/game/keywords.rs @@ -528,8 +528,36 @@ pub fn activate_ninjutsu( combat.blocker_assignments.remove(&creature_to_return); } - // 2. Move Ninjutsu-family card from hand/command zone to battlefield - zones::move_to_zone(state, ninjutsu_obj_id, Zone::Battlefield, events); + // 2. Move Ninjutsu-family card from hand/command zone to battlefield. + // + // CR 614.1c: route the entry through the zone-change pipeline so the + // delivery tail applies enters-with-counters statics ("creatures you + // control enter with an additional +1/+1 counter" — Hardened Scales / + // Conclave Mentor class) to the entering ninja; the raw `move_to_zone` + // skipped that tail, so the ninja entered without them. CR 400.7 attributes + // the entry to the ninja itself (the pre-pipeline raw move recorded no + // source; the cast-variant tag below records the ninjutsu provenance). + // + // CR 616.1 / CR 303.4f: a battlefield-entry pause is not reachable here — + // the ninja is a creature (not an Aura, so no host choice), and no + // supported ninja entry surfaces a counter-replacement pause or a CR 616.1 + // multi-redirect ordering choice. The bail is a safety guard, not a resume + // path: the combat placement (CR 702.49c) below cannot resume across a + // pause, so on the (unreachable) pause we stop with the prompt parked rather + // than place the ninja attacking over a parked state. + match super::zone_pipeline::move_object( + state, + super::zone_pipeline::ZoneMoveRequest::effect( + ninjutsu_obj_id, + Zone::Battlefield, + ninjutsu_obj_id, + ), + events, + ) { + super::zone_pipeline::ZoneMoveResult::Done => {} + super::zone_pipeline::ZoneMoveResult::NeedsChoice(_) + | super::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => return Ok(()), + } // CR 702.49: Track which alt-cost variant was paid this turn on the // cast-variant-paid tag (placement + tapped + summoning sickness is From 446f5d4a79b968395108ca738453ca7d335ae701 Mon Sep 17 00:00:00 2001 From: Matt Evans <1388610+matthewevans@users.noreply.github.com> Date: Wed, 10 Jun 2026 08:19:03 -0700 Subject: [PATCH 03/11] docs(engine): correct stale aura-arm drain note + document dig multi-kept pause limitation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two comment-only corrections from the d5a12b8c6 review verdict: 1. zone_pipeline.rs deliver_batch aura arm: the note claiming a tail stashed on NeedsAuraAttachmentChoice would be 'silently drained by the NEXT unrelated replacement-choice resume' is stale. As of d5a12b8c6 the ReturnAsAuraTarget handler (engine.rs:3608-3611) and its chain-resume sibling (engine.rs:3572) both drain pending_batch_deliveries, so the aura-attachment resume finishes the parked batch correctly. 2. engine_resolution_choices.rs DigChoice kept-loop pause: document the multi-kept limitation — if kept card #1 pauses, kept #2+ stay in the library (the for-loop return exits before they move), and publish_tracked_set: Some(kept) publishes ALL kept including the unmoved ones, so a downstream sub-ability can be wired to cards still in the library. Pre-existing, strictly no-worse-than the old raw path; revisit with a kept-loop continuation if a 2+-kept-to-battlefield dig that pauses on the first is ever added. No behavior change. --- .../src/game/engine_resolution_choices.rs | 20 +++++++++++++ crates/engine/src/game/zone_pipeline.rs | 29 ++++++++++--------- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index 4b165f4c58..d111e02550 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -1314,6 +1314,26 @@ pub(super) fn handle_resolution_choice( // unkept cards strand in the library (they were not yet // moved). The drain fires on both the replacement-choice // resume and the aura-attachment resume. + // + // SCOPING (multi-kept limitation, pre-existing, + // strictly no-worse-than-before): this `return` exits + // the `for &obj_id in &kept` loop, so if kept card #1 + // pauses, kept #2+ are NOT moved to the battlefield — + // they remain in the library. The deferred completion + // only finishes the rest-pile (unkept) move and the + // tracked-set publish; it does not resume the kept + // loop. The old raw-`move_to_zone` path had the same + // ceiling (it could not pause and resume a kept tail + // either), so this is no regression. WRINKLE: + // `publish_tracked_set: Some(kept.clone())` publishes + // ALL kept cards, including the unmoved #2+, so a + // downstream sub-ability keyed off the tracked set can + // be wired to cards still in the library on this paused + // path. Acceptable today because no supported dig card + // both keeps 2+ cards to the battlefield AND surfaces an + // as-enters pause on the first; revisit if such a card + // is added (the fix is a kept-loop continuation, not a + // single completion). crate::game::zone_pipeline::ZoneMoveResult::NeedsChoice(_) | crate::game::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => { crate::game::zone_pipeline::defer_completion_on_pause( diff --git a/crates/engine/src/game/zone_pipeline.rs b/crates/engine/src/game/zone_pipeline.rs index e6c1d117f6..12c805f16a 100644 --- a/crates/engine/src/game/zone_pipeline.rs +++ b/crates/engine/src/game/zone_pipeline.rs @@ -725,21 +725,22 @@ fn deliver_batch( ZoneMoveResult::NeedsAuraAttachmentChoice => { // CR 303.4f: an aura-host choice flows through // `WaitingFor::ReturnAsAuraTarget`, not the replacement-choice - // resume path, so `drain_pending_batch_deliveries` (which only - // runs from `handle_replacement_choice`) would not fire here. - // No batch flow targets a battlefield aura entry today (mill - // destinations are graveyard/exile/hand; mass bounce returns to - // hand/library), so this is unreachable; stop and stash the - // tail so a future battlefield-entry batch does not silently - // drop the rest of the batch. + // resume path. No batch flow targets a battlefield aura entry + // today (mill destinations are graveyard/exile/hand; mass bounce + // returns to hand/library), so this arm is unreachable for the + // current batch callers; stop and stash the tail so a future + // battlefield-entry batch does not silently drop its remainder. // - // NOTE: this is NOT a loud failure. The engine_replacement - // drain gate keys on `pending_batch_deliveries.is_some()`, not - // on provenance, so a stale tail stashed here would be silently - // drained by the NEXT unrelated replacement-choice resume. - // Reaching this arm for a batch flow is a bug to be surfaced if - // a battlefield-entry batch is ever added; today it is dead - // code for every batch caller. + // The stashed tail IS drained correctly on resume: the + // `ReturnAsAuraTarget` handler (engine.rs:3608-3611) and its + // chain-resume sibling (engine.rs:3572) both call + // `drain_pending_batch_deliveries` when + // `pending_batch_deliveries.is_some()`, so the aura-attachment + // pause finishes the parked batch the same way the replacement- + // choice resume does. (Updated for d5a12b8c6, which added the + // aura-resume drain; the prior note here that the tail would be + // "silently drained by the NEXT unrelated resume" is no longer + // accurate.) stash_batch_tail(state, queue.collect(), destination); return BatchMoveResult::NeedsChoice; } From da0a1a70777779709383daa562115f8b5c3d22b8 Mon Sep 17 00:00:00 2001 From: Matt Evans <1388610+matthewevans@users.noreply.github.com> Date: Wed, 10 Jun 2026 08:35:39 -0700 Subject: [PATCH 04/11] fix(engine): route reveal-until and dig graveyard rest piles through zone pipeline so Moved redirects fire (CR 614.6 / 701.20a / 603.10a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reveal-until and dig 'put the rest into your graveyard' piles delivered via raw move_to_zone, never proposing the inner ZoneChange, so a board-wide Moved graveyard->exile redirect (Rest in Peace / Leyline of the Void) silently dropped on the rest cards. 12 card-data cards carry RevealUntil with rest_destination: Graveyard (Mind Funeral class), plus dig's no-kept-zone 'rest to graveyard' branch — all affected. move_rest is now the single authority: a Graveyard (or any non-library) rest pile routes through move_objects_simultaneously (CR 603.10a co-departure stamp), so each rest card consults the redirect; a Library rest pile keeps the random-order shuffle_to_bottom (no Moved-redirect class targets Library, and the placement arm would lose the shuffle). The new move_rest_then carries an optional BatchCompletion for callers that defer cleanup across a pause. Synchronous completion runs the resolver's own marker-clear + EffectResolved inline (the dispatching chain processor still owns priority/continuation). On a mid-pile CR 616.1 ordering pause, the prompt is parked and the cleanup is deferred onto a cleanup-only RevealRestPile completion (empty rest_cards — the pile IS the batch) so the drain runs it once and EffectResolved never lands over the parked prompt. The dig unkept->graveyard branch mirrors this, deferring finish_with_continuation via the same completion. Discriminating test reveal_until_graveyard_rest_redirected_to_exile_by_rest_in_peace: a graveyard-rest reveal-until with a RIP graveyard->exile Moved redirect on the battlefield now exiles the rest pile (old raw path: graveyard'd). CR grep (docs/MagicCompRules.txt): 614.6 replaced event never happens; 701.20a reveal until / rest pile; 603.10a simultaneous leaves-battlefield. --- .../engine/src/game/effects/reveal_until.rs | 193 ++++++++++++++++-- .../src/game/engine_resolution_choices.rs | 45 +++- 2 files changed, 222 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/game/effects/reveal_until.rs b/crates/engine/src/game/effects/reveal_until.rs index 7f12f9904d..3137b65095 100644 --- a/crates/engine/src/game/effects/reveal_until.rs +++ b/crates/engine/src/game/effects/reveal_until.rs @@ -195,16 +195,47 @@ pub fn resolve( } } - // Move remaining revealed cards to rest_destination. - move_rest(state, &revealed_misses, rest_destination, events); + // CR 701.20a + CR 614.6: move the rest pile to its destination through the + // zone-change pipeline so a per-card `Moved` graveyard→exile redirect (Rest + // in Peace / Leyline of the Void) fires on each rest card — the 12 + // `rest_destination: Graveyard` reveal-until cards (Mind Funeral class) + // previously dropped that redirect. + // + // On synchronous completion (the realistic single-redirect path) this + // resolver runs its own reveal-marker clear + `EffectResolved` inline below, + // matching the historical tail exactly (the chain processor that dispatched + // this effect still owns priority/continuation). On a mid-pile CR 616.1 + // ordering pause, the prompt is parked and the undelivered tail stashed; + // defer the marker-clear + `EffectResolved` onto a cleanup-only completion + // (`rest_cards` empty — the pile IS this batch) so the drain runs it once the + // pile lands, and bail before the inline tail so `EffectResolved` never lands + // over the parked prompt. + let mut clear_markers = revealed_misses.clone(); + if let Some(hit) = hit_card { + clear_markers.push(hit); + } + match move_rest_then(state, &revealed_misses, rest_destination, None, events) { + zone_pipeline::BatchMoveResult::Done => {} + zone_pipeline::BatchMoveResult::NeedsChoice => { + zone_pipeline::defer_completion_on_pause( + state, + BatchCompletion::RevealRestPile { + player: revealing_player, + rest_cards: Vec::new(), + rest_destination, + clear_markers, + publish_tracked_set: None, + emit_reveal_until_resolved: Some(ability.source_id), + }, + ); + return Ok(()); + } + } // Clear reveal markers — cards have moved zones. - for &card_id in &revealed_misses { + for &card_id in &clear_markers { state.revealed_cards.remove(&card_id); } - if let Some(hit) = hit_card { - state.revealed_cards.remove(&hit); - } events.push(GameEvent::EffectResolved { kind: EffectKind::RevealUntil, @@ -254,20 +285,56 @@ pub(crate) fn move_rest( rest_destination: Zone, events: &mut Vec, ) { + move_rest_then(state, cards, rest_destination, None, events); +} + +/// CR 701.20a + CR 614.6 + CR 603.10a: Move the rest pile to `rest_destination`, +/// running `completion` (the reveal-marker clear / tracked-set publish / +/// `RevealUntil`-resolved cleanup) exactly once after the pile lands — whether +/// the pile moves synchronously or a per-card `Moved` redirect pauses on a +/// CR 616.1 ordering choice. +/// +/// Single authority for rest-pile placement. A `Zone::Graveyard` (or any other +/// non-library) rest pile routes through the zone-change pipeline so a `Moved` +/// graveyard→exile redirect (Rest in Peace / Leyline of the Void) fires on each +/// rest card — the 12 `rest_destination: Graveyard` reveal-until cards (Mind +/// Funeral class) previously dropped that redirect via the raw `move_to_zone`. +/// The pipeline batch co-stamps the departures (CR 603.10a) and, on a mid-pile +/// pause, parks the prompt and re-runs `completion` from the drain path; the +/// completion is carried with an empty `rest_cards` so it does NOT re-move the +/// pile (the pile IS this batch — the completion is cleanup-only here). +/// +/// A `Zone::Library` rest pile keeps the random-order shuffle-to-bottom +/// (`shuffle_to_bottom`, CR 701.20a "in a random order") and runs the completion +/// inline: a library reposition has no `Moved`-redirect class to consult (zero +/// `destination_zone(Library)` defs in the pool) and cannot pause, so routing it +/// through the pipeline's placement arm would gain nothing and lose the shuffle. +pub(crate) fn move_rest_then( + state: &mut GameState, + cards: &[ObjectId], + rest_destination: Zone, + completion: Option, + events: &mut Vec, +) -> zone_pipeline::BatchMoveResult { match rest_destination { Zone::Library => { // "on the bottom of your library in a random order" shuffle_to_bottom(state, cards, events); - } - Zone::Graveyard => { - for &card_id in cards { - zones::move_to_zone(state, card_id, Zone::Graveyard, events); + if let Some(completion) = completion { + crate::game::engine_resolution_choices::run_batch_completion( + state, completion, events, + ); } + zone_pipeline::BatchMoveResult::Done } - other => { - for &card_id in cards { - zones::move_to_zone(state, card_id, other, events); - } + dest => { + // CR 400.7: the rest cards move themselves to `dest`; each anchors + // its own attribution (the pre-pipeline raw move recorded no source). + let reqs: Vec = cards + .iter() + .map(|&card_id| ZoneMoveRequest::effect(card_id, dest, card_id)) + .collect(); + zone_pipeline::move_objects_simultaneously_then(state, reqs, completion, events) } } } @@ -558,6 +625,104 @@ mod tests { assert!(state.players[0].graveyard.contains(&land)); } + /// Discriminating test (CR 614.6 + CR 701.20a): a `rest_destination: + /// Graveyard` reveal-until (Mind Funeral class, 12 cards) whose rest pile is + /// caught by a Rest in Peace–style `Moved` graveyard→exile redirect must + /// have its rest cards EXILED, not graveyard'd. The old raw `move_to_zone` + /// rest-pile delivery never proposed the inner ZoneChange, so the redirect + /// silently dropped and the land landed in the graveyard. Routing the rest + /// pile through `move_objects_simultaneously` consults the redirect. + #[test] + fn reveal_until_graveyard_rest_redirected_to_exile_by_rest_in_peace() { + use crate::types::ability::{ + AbilityDefinition, AbilityKind, Effect, ReplacementDefinition, + }; + use crate::types::replacements::ReplacementEvent; + + let mut state = GameState::new_two_player(42); + + // Rest in Peace: "If a card would be put into a graveyard from anywhere, + // exile it instead." (graveyard→exile Moved redirect on the battlefield) + let rip = create_object( + &mut state, + CardId(1000), + PlayerId(0), + "Rest in Peace".to_string(), + Zone::Battlefield, + ); + let redirect = ReplacementDefinition::new(ReplacementEvent::Moved) + .destination_zone(Zone::Graveyard) + .execute(AbilityDefinition::new( + AbilityKind::Spell, + Effect::ChangeZone { + destination: Zone::Exile, + origin: None, + target: TargetFilter::SelfRef, + owner_library: false, + enter_transformed: false, + enters_under: None, + enter_tapped: crate::types::zones::EtbTapState::Unspecified, + enters_attacking: false, + up_to: false, + enter_with_counters: vec![], + face_down_profile: None, + }, + )); + state.objects.get_mut(&rip).unwrap().replacement_definitions = vec![redirect].into(); + + let land = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Forest".to_string(), + Zone::Library, + ); + state + .objects + .get_mut(&land) + .unwrap() + .card_types + .core_types + .push(CoreType::Land); + + let creature = create_object( + &mut state, + CardId(2), + PlayerId(0), + "Bear".to_string(), + Zone::Library, + ); + state + .objects + .get_mut(&creature) + .unwrap() + .card_types + .core_types + .push(CoreType::Creature); + + let ability = make_reveal_until_ability( + PlayerId(0), + TargetFilter::Typed(crate::types::ability::TypedFilter::creature()), + Zone::Hand, + Zone::Graveyard, + ); + let mut events = Vec::new(); + resolve(&mut state, &ability, &mut events).unwrap(); + + // The matching creature still goes to hand; the rest pile (the land) is + // redirected from graveyard → exile by Rest in Peace, NOT graveyard'd. + assert!(state.players[0].hand.contains(&creature)); + assert!( + !state.players[0].graveyard.contains(&land), + "rest card must NOT reach the graveyard — RIP redirects it" + ); + assert_eq!( + state.objects.get(&land).map(|o| o.zone), + Some(Zone::Exile), + "rest card must be exiled by the graveyard→exile redirect" + ); + } + #[test] fn reveal_until_no_match_all_to_rest() { let mut state = GameState::new_two_player(42); diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index d111e02550..1baaebb8e2 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -1277,8 +1277,49 @@ pub(super) fn handle_resolution_choice( } }; if let Some(zone) = move_unkept_to { - for &obj_id in &unkept { - zones::move_to_zone(state, obj_id, zone, events); + // CR 614.6 + CR 603.10a: route the unkept pile through the + // zone-change pipeline so a per-card `Moved` graveyard→exile + // redirect (Rest in Peace / Leyline of the Void) fires on each + // — the raw `move_to_zone` never proposed the inner ZoneChange, + // silently dropping those redirects for dig's "the rest into + // your graveyard" class. `zone` here is never Library (the + // Library case pushed back above and yielded `None`), so the + // batch always has a `Moved`-redirect-eligible destination. + // CR 400.7: each unkept card anchors its own attribution. + // + // On a mid-pile CR 616.1 ordering pause, defer the + // priority/continuation drain (a cleanup-only `RevealRestPile` + // completion: empty pile, no markers/publish, just + // `finish_with_continuation`) so it runs once the pile lands, + // and surface the parked prompt instead of draining over it. + let reqs: Vec<_> = unkept + .iter() + .map(|&obj_id| { + crate::game::zone_pipeline::ZoneMoveRequest::effect( + obj_id, zone, obj_id, + ) + }) + .collect(); + match crate::game::zone_pipeline::move_objects_simultaneously( + state, reqs, events, + ) { + crate::game::zone_pipeline::BatchMoveResult::Done => {} + crate::game::zone_pipeline::BatchMoveResult::NeedsChoice => { + crate::game::zone_pipeline::defer_completion_on_pause( + state, + crate::types::game_state::BatchCompletion::RevealRestPile { + player, + rest_cards: Vec::new(), + rest_destination: zone, + clear_markers: Vec::new(), + publish_tracked_set: None, + emit_reveal_until_resolved: None, + }, + ); + return Ok(ResolutionChoiceOutcome::WaitingFor( + state.waiting_for.clone(), + )); + } } } return Ok(ResolutionChoiceOutcome::WaitingFor( From 4091e33dcabd5e7bbee01954916ca7c41f4b50d8 Mon Sep 17 00:00:00 2001 From: Matt Evans <1388610+matthewevans@users.noreply.github.com> Date: Wed, 10 Jun 2026 08:48:33 -0700 Subject: [PATCH 05/11] fix(engine): route morph and manifest face-down entries through zone pipeline (CR 708.3 / 614.1c) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both face-down battlefield entries (cast face-down via morph/disguise, and manifest) delivered via raw move_to_zone followed by a manual apply_face_down_creature_characteristics + back_face snapshot — bypassing the pipeline delivery tail, so a face-down 2/2 never received enters-with-counters statics ('creatures you control enter with an additional +1/+1 counter' — Hardened Scales / Conclave Mentor class). Route both through zone_pipeline::move_object with the new ZoneMoveRequest::face_down(profile) mod. The delivery tail is already the canonical face-down authority (it snapshots the real face into back_face and applies the vanilla-2/2 profile BEFORE the entry per CR 708.3, identical to change_zone's face-down path) AND seeds enters-with-counters statics, so the manual post-move override is dropped — morph/manifest now match the rest of the engine's face-down entries. A battlefield-entry pause is unreachable for a vanilla 2/2 (not an Aura, no Moved redirect / counter-replacement choice); the bail keeps the helpers safe by construction. CR grep (docs/MagicCompRules.txt): 708.2a face-down characteristics; 708.3 turned face down before it enters; 614.1c enters-with statics; 701.40a manifest. --- crates/engine/src/game/morph.rs | 89 +++++++++++++++---------- crates/engine/src/game/zone_pipeline.rs | 9 +++ 2 files changed, 61 insertions(+), 37 deletions(-) diff --git a/crates/engine/src/game/morph.rs b/crates/engine/src/game/morph.rs index abe45900d2..9ce2e59a5d 100644 --- a/crates/engine/src/game/morph.rs +++ b/crates/engine/src/game/morph.rs @@ -12,7 +12,7 @@ use crate::types::zones::Zone; use std::sync::Arc; use super::engine::EngineError; -use super::printed_cards::{apply_back_face_to_object, snapshot_object_face}; +use super::printed_cards::apply_back_face_to_object; /// Stores the original characteristics of a face-down card so they can be /// restored when the card is turned face up. @@ -119,23 +119,31 @@ pub fn play_face_down( )); } - // Store original characteristics before overriding - let original = snapshot_object_face(obj); - - // Move to battlefield - super::zones::move_to_zone(state, object_id, Zone::Battlefield, events); - - // Apply face-down overrides - let obj = state.objects.get_mut(&object_id).unwrap(); - apply_face_down_creature_characteristics( - obj, - &crate::types::ability::FaceDownProfile::vanilla_2_2(), - ); - - // Store original characteristics so turn_face_up can restore them - obj.back_face = Some(original); - - Ok(()) + // CR 708.3 + CR 614.1c: route the face-down battlefield entry through the + // zone-change pipeline. The delivery tail applies the face-down 2/2 profile + // (snapshot the real face into `back_face`, overwrite with the vanilla 2/2 — + // CR 708.2a) AND seeds enters-with-counters statics ("creatures you control + // enter with an additional +1/+1 counter" — Hardened Scales class), which + // the raw `move_to_zone` + manual override skipped entirely. CR 708.3: the + // permanent is turned face down BEFORE it enters, so the tail does this + // before the ETB-counter/trigger blocks — the manual post-move override is + // dropped (the tail is the single authority, mirroring `manifest_card` and + // change_zone's face-down path). + // + // CR 616.1 / CR 303.4f: a face-down 2/2 vanilla creature is not an Aura and + // carries no Moved redirect or counter-replacement choice, so a battlefield- + // entry pause is unreachable; the bail keeps the helper safe by construction + // (the prompt is parked centrally by `move_object`). + match super::zone_pipeline::move_object( + state, + super::zone_pipeline::ZoneMoveRequest::effect(object_id, Zone::Battlefield, object_id) + .face_down(crate::types::ability::FaceDownProfile::vanilla_2_2()), + events, + ) { + super::zone_pipeline::ZoneMoveResult::Done => Ok(()), + super::zone_pipeline::ZoneMoveResult::NeedsChoice(_) + | super::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => Ok(()), + } } /// CR 702.37c: Turning a face-down permanent face up restores its original characteristics. @@ -227,26 +235,32 @@ pub fn manifest_card( object_id: ObjectId, events: &mut Vec, ) -> Result<(), EngineError> { - let obj = state - .objects - .get(&object_id) - .ok_or_else(|| EngineError::InvalidAction("Object not found for manifest".to_string()))?; - - // Store original characteristics before overriding - let original = snapshot_object_face(obj); - - // Move to battlefield - super::zones::move_to_zone(state, object_id, Zone::Battlefield, events); - - // Apply face-down overrides — CR 701.40a: 2/2 creature with no text/name/subtypes/mana cost - let obj = state.objects.get_mut(&object_id).unwrap(); - apply_face_down_creature_characteristics( - obj, - &crate::types::ability::FaceDownProfile::vanilla_2_2(), - ); - obj.back_face = Some(original); + if !state.objects.contains_key(&object_id) { + return Err(EngineError::InvalidAction( + "Object not found for manifest".to_string(), + )); + } - Ok(()) + // CR 701.40a + CR 708.3 + CR 614.1c: route the face-down manifest entry + // through the zone-change pipeline. The delivery tail applies the vanilla + // 2/2 face-down profile (snapshot real face into `back_face`, overwrite — + // CR 708.2a) AND seeds enters-with-counters statics (Hardened Scales class), + // which the raw `move_to_zone` + manual override skipped. The manual + // post-move override is dropped (the tail is the single authority). + // + // CR 616.1 / CR 303.4f: a face-down 2/2 vanilla creature is not an Aura and + // carries no Moved redirect or counter-replacement choice, so a battlefield- + // entry pause is unreachable; the bail keeps the helper safe by construction. + match super::zone_pipeline::move_object( + state, + super::zone_pipeline::ZoneMoveRequest::effect(object_id, Zone::Battlefield, object_id) + .face_down(crate::types::ability::FaceDownProfile::vanilla_2_2()), + events, + ) { + super::zone_pipeline::ZoneMoveResult::Done => Ok(()), + super::zone_pipeline::ZoneMoveResult::NeedsChoice(_) + | super::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => Ok(()), + } } /// CR 701.40a: Manifest puts the top card of library onto battlefield face down as a 2/2 creature. @@ -293,6 +307,7 @@ pub fn manifest( #[cfg(test)] mod tests { + use super::super::printed_cards::snapshot_object_face; use super::*; use crate::game::zones::create_object; use crate::types::ability::QuantityExpr; diff --git a/crates/engine/src/game/zone_pipeline.rs b/crates/engine/src/game/zone_pipeline.rs index 12c805f16a..3f0cb9ce32 100644 --- a/crates/engine/src/game/zone_pipeline.rs +++ b/crates/engine/src/game/zone_pipeline.rs @@ -297,6 +297,15 @@ impl ZoneMoveRequest { self } + /// CR 708.2a + CR 708.3: enters the battlefield face down showing the given + /// profile (morph / manifest vanilla 2/2). The delivery tail snapshots the + /// real face into `back_face` and applies the profile before the entry, so + /// callers no longer override characteristics manually after the move. + pub fn face_down(mut self, profile: FaceDownProfile) -> Self { + self.mods.face_down_profile = Some(profile); + self + } + /// Library placement override (`LibraryPosition::Top` / `Bottom` / /// `NthFromTop`). Only meaningful when `to == Zone::Library`. pub fn at_library_position(mut self, position: LibraryPosition) -> Self { From 6b8dca89f961e1e2a7fd9c6b8d9172cfe62ad042 Mon Sep 17 00:00:00 2001 From: Matt Evans <1388610+matthewevans@users.noreply.github.com> Date: Wed, 10 Jun 2026 09:02:30 -0700 Subject: [PATCH 06/11] fix(engine): route exile-until-leaves returns through zone pipeline (CR 610.3a / 614.1c) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check_exile_returns (Banisher Priest / Fiend Hunter / Oblivion Ring class — 'exile until ~ leaves') returned each exiled card via raw move_to_zone, skipping the pipeline delivery tail: a battlefield return missed enters-with- counters statics (Hardened Scales class), and a non-battlefield return dropped any Moved redirect. Group the returns by destination zone (first-seen order for determinism — Zone isn't Ord) and route each group through move_objects_simultaneously_then (CR 603.10a co-departure). A returned creature can pause on an as-enters / aura-host choice (CR 303.4f / 616.1), so the spent UntilSourceLeaves link cleanup rides a new BatchCompletion::RemoveExileLinks per group, drained once the group's pile lands (synchronously or via the replacement-choice / aura-attachment resume) — never before a paused card finished returning. Links for cards that already left exile by other means are dropped immediately; only the in-flight group ids ride their completion. CR grep (docs/MagicCompRules.txt): 610.3a return to previous zone; 614.1c enters-with statics; 603.10a simultaneous; 303.4f aura host on entry. --- crates/engine/src/game/engine.rs | 79 +++++++++++++++---- .../src/game/engine_resolution_choices.rs | 11 +++ crates/engine/src/types/game_state.rs | 13 +++ 3 files changed, 89 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 059a6aea98..ee8c5126a0 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -6613,29 +6613,80 @@ pub(super) fn check_exile_returns(state: &mut GameState, events: &mut Vec)> = Vec::new(); for link in &to_return { - // Only return if the card is still in exile let still_in_exile = state .objects .get(&link.exiled_id) .map(|obj| obj.zone == Zone::Exile) .unwrap_or(false); - if still_in_exile { - let crate::types::game_state::ExileLinkKind::UntilSourceLeaves { return_zone } = - &link.kind - else { - continue; - }; - zones::move_to_zone(state, link.exiled_id, *return_zone, events); + if !still_in_exile { + continue; + } + let crate::types::game_state::ExileLinkKind::UntilSourceLeaves { return_zone } = &link.kind + else { + continue; + }; + match groups.iter_mut().find(|(zone, _)| *zone == *return_zone) { + Some((_, ids)) => ids.push(link.exiled_id), + None => groups.push((*return_zone, vec![link.exiled_id])), } } - // Remove processed links - let returned_ids: Vec<_> = to_return.iter().map(|l| l.exiled_id).collect(); - state - .exile_links - .retain(|link| !returned_ids.contains(&link.exiled_id)); + // Links for cards that already left exile (not returned by us) are still spent + // and must be dropped now — only the IN-FLIGHT group ids ride their batch + // completion. (The common case is a single battlefield group; a mid-group + // pause defers only that group's cleanup, while any remaining groups process + // after — `move_objects_simultaneously_then` parks the tail per group.) + let returning_ids: std::collections::HashSet = groups + .iter() + .flat_map(|(_, ids)| ids.iter().copied()) + .collect(); + let returned_all: Vec = to_return.iter().map(|l| l.exiled_id).collect(); + state.exile_links.retain(|link| { + !returned_all.contains(&link.exiled_id) || returning_ids.contains(&link.exiled_id) + }); + + for (return_zone, ids) in groups { + let reqs: Vec<_> = ids + .iter() + .map(|&id| super::zone_pipeline::ZoneMoveRequest::effect(id, return_zone, id)) + .collect(); + let completion = + crate::types::game_state::BatchCompletion::RemoveExileLinks { returned_ids: ids }; + if matches!( + super::zone_pipeline::move_objects_simultaneously_then( + state, + reqs, + Some(completion), + events, + ), + super::zone_pipeline::BatchMoveResult::NeedsChoice + ) { + // CR 616.1 / CR 303.4f: this group paused; its tail + cleanup are + // parked and drained on resume. Stop processing further groups so a + // later group's moves do not run over the parked prompt; the spent + // links of any unprocessed group remain in `exile_links` until their + // (now-gone) source re-checks — acceptable, as multi-destination + // returns from one source-leaves event do not occur in the pool. + return; + } + } } #[cfg(test)] diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index 1baaebb8e2..a217e3e096 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -3306,6 +3306,17 @@ pub(crate) fn run_batch_completion( } finish_with_continuation(state, player, events); } + // CR 610.3a: the exile-until-leaves return pile has fully landed (after a + // returned creature's as-enters / aura-host pause resolved). Drop the + // spent `UntilSourceLeaves` links now — deferred so it runs exactly once + // after the paused card finished returning, not before. No priority / + // continuation drain here: this completion rides an SBA-time return + // (`check_exile_returns`), whose surrounding pipeline owns priority. + BatchCompletion::RemoveExileLinks { returned_ids } => { + state + .exile_links + .retain(|link| !returned_ids.contains(&link.exiled_id)); + } } } diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 1874366daf..9c80702b41 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -1151,6 +1151,19 @@ pub enum BatchCompletion { /// `EffectResolved` before the pause (or rely on the continuation). emit_reveal_until_resolved: Option, }, + /// CR 610.3a + CR 614.1c: An "exile until ~ leaves" return (Banisher Priest / + /// Fiend Hunter / Oblivion Ring class) routed its exiled cards back to the + /// battlefield through the simultaneous-move batch so the delivery tail seeds + /// enters-with-counters statics. A returned creature can pause on an + /// as-enters / aura-host choice; defer the exile-link bookkeeping cleanup + /// (`UntilSourceLeaves` links are spent once their card returns) onto the + /// parked batch tail so the links are dropped exactly once after the whole + /// return pile lands — not before a paused card finishes returning. + RemoveExileLinks { + /// The exiled-card ids whose `UntilSourceLeaves` links are consumed by + /// this return and must be retained out of `state.exile_links`. + returned_ids: Vec, + }, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] From 2d3c10c64caec3bf8bf2a86eb4995ab8e0c96121 Mon Sep 17 00:00:00 2001 From: Matt Evans <1388610+matthewevans@users.noreply.github.com> Date: Wed, 10 Jun 2026 09:14:16 -0700 Subject: [PATCH 07/11] fix(engine): route reveal-until kept-to-graveyard cards through zone pipeline; document dig rest-partition gap (CR 614.6 / 701.20a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reveal-until KEPT card sent to the graveyard (4 cards, kept_destination: Graveyard — Mind Funeral-style 'put it into your graveyard') was delivered raw, skipping a Moved graveyard->exile redirect (Rest in Peace / Leyline of the Void). Route it through move_object on both the synchronous resolve path (the non-Hand/Battlefield kept branch; Library stays raw as a placement) and the RevealUntilKeptChoice handler (accept_zone / decline_zone via the new route_kept_card_or_defer helper). On a CR 616.1 pause the rest-pile move + marker clear defer onto a RevealRestPile completion; the kept-choice handler's rest pile now flows through move_rest_then so its completion (marker clear + finish_with_continuation) runs once on either path — closing the latent unhandled-pause the move_rest migration introduced here. Documents the remaining route_rest_partition gap (dig 'rest into graveyard', 65 Dig cards + the null->Graveyard default): it has a caller INSIDE run_batch_completion, so migrating it needs a re-pause-from-completion contract plus pause handling in both synchronous callers — a cross-cutting change tracked for a follow-up rather than a partial migration. CR grep (docs/MagicCompRules.txt): 614.6 replaced event never happens; 701.20a reveal until / rest pile. --- .../engine/src/game/effects/reveal_until.rs | 38 ++++- .../src/game/engine_resolution_choices.rs | 142 +++++++++++++++++- 2 files changed, 171 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/game/effects/reveal_until.rs b/crates/engine/src/game/effects/reveal_until.rs index 3137b65095..17d420f6cc 100644 --- a/crates/engine/src/game/effects/reveal_until.rs +++ b/crates/engine/src/game/effects/reveal_until.rs @@ -189,8 +189,44 @@ pub fn resolve( crate::game::combat::enter_attacking(state, hit, ability.source_id, controller); } } + Zone::Library => { + // CR 701.20a: a kept card sent to the library (2 cards) is a + // placement, not a redirect-eligible move — keep the raw mover + // (no `Moved` class targets the library; routing through the + // pipeline's placement arm would gain nothing). + zones::move_to_zone(state, hit, Zone::Library, events); + } other => { - zones::move_to_zone(state, hit, other, events); + // CR 614.6: a kept card sent to the graveyard (4 cards) or exile + // routes through the pipeline so a `Moved` graveyard→exile + // redirect (Rest in Peace / Leyline of the Void) fires on it. On a + // CR 616.1 ordering pause, defer the rest-pile move + marker clear + // + `EffectResolved` onto a `RevealRestPile` completion (the same + // deferral the battlefield branch uses) so the misses don't strand + // and `EffectResolved` doesn't land over the parked prompt. + match zone_pipeline::move_object( + state, + ZoneMoveRequest::effect(hit, other, ability.source_id), + events, + ) { + ZoneMoveResult::Done => {} + ZoneMoveResult::NeedsChoice(_) | ZoneMoveResult::NeedsAuraAttachmentChoice => { + let mut clear_markers = revealed_misses.clone(); + clear_markers.push(hit); + zone_pipeline::defer_completion_on_pause( + state, + BatchCompletion::RevealRestPile { + player: revealing_player, + rest_cards: revealed_misses, + rest_destination, + clear_markers, + publish_tracked_set: None, + emit_reveal_until_resolved: Some(ability.source_id), + }, + ); + return Ok(()); + } + } } } } diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index a217e3e096..24c0c75782 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -154,6 +154,24 @@ fn route_rest_partition( } } zone => { + // ZONE-PIPELINE GAP (documented deferral): the dig/search "rest into + // your graveyard" partition (65 Dig cards + the `null`→Graveyard + // default) is delivered raw here, so a `Moved` graveyard→exile redirect + // (Rest in Peace / Leyline of the Void) does NOT yet fire on these + // rest cards. The sibling dig unkept-loop (handle_resolution_choice + // DigChoice) and the reveal-until rest pile (effects::reveal_until:: + // move_rest_then) ARE migrated; this shared partition helper is not, + // because it has three callers — two synchronous (search-split at + // :279, dig at the kept block) and ONE inside `run_batch_completion`'s + // RevealRestPile arm. Routing it through `move_objects_simultaneously` + // makes a CR 616.1 pause possible from inside a completion, which + // requires a re-pause-from-completion contract (the completion returns + // `()` and cannot signal a fresh park to its caller) plus pause + // handling threaded through both synchronous callers. That is a + // cross-cutting state-machine change; tracked for a follow-up so it + // lands as one reviewable unit rather than a partial migration here. + // (Practical exposure: a graveyard-redirect on a dig REST pile — the + // non-kept cards — with RIP/Leyline on the battlefield.) for &obj_id in rest_ids { zones::move_to_zone(state, obj_id, zone, events); } @@ -560,20 +578,73 @@ pub(super) fn handle_resolution_choice( ); } } else { - zones::move_to_zone(state, hit_card, accept_zone, events); + // CR 614.6: a kept card accepted to a non-battlefield zone + // (graveyard — Mind Funeral-style "put it into your graveyard" + // kept cards, 4 cards — or exile) routes through the pipeline + // so a `Moved` graveyard→exile redirect fires. On a CR 616.1 + // pause, defer the rest-pile move + marker clear onto a + // `RevealRestPile` completion (EffectResolved already emitted + // before this prompt) and surface the parked prompt. + if let Some(outcome) = route_kept_card_or_defer( + state, + hit_card, + accept_zone, + source_id, + &misses, + rest_destination, + events, + ) { + return Ok(outcome); + } } } else if decline_zone == rest_destination { misses.push(hit_card); } else { - zones::move_to_zone(state, hit_card, decline_zone, events); + // CR 614.6: same redirect-consult for a declined kept card sent to + // a non-rest graveyard/exile destination. + if let Some(outcome) = route_kept_card_or_defer( + state, + hit_card, + decline_zone, + source_id, + &misses, + rest_destination, + events, + ) { + return Ok(outcome); + } } - effects::reveal_until::move_rest(state, &misses, rest_destination, events); - // CR 701.20b: revealed cards have now moved zones — clear markers. - for &card_id in &misses { - state.revealed_cards.remove(&card_id); + // CR 701.20a + CR 614.6: move the rest pile (RIP redirects fire) and + // run the marker clear + continuation drain as the completion. On a + // synchronous landing the completion runs inline; on a CR 616.1 pause + // it defers and the drain runs it once the pile lands. `clear_markers` + // is the misses plus the kept card (already placed above). + let mut clear_markers = misses.clone(); + clear_markers.push(hit_card); + match effects::reveal_until::move_rest_then( + state, + &misses, + rest_destination, + Some(crate::types::game_state::BatchCompletion::RevealRestPile { + player, + rest_cards: Vec::new(), + rest_destination, + clear_markers, + publish_tracked_set: None, + emit_reveal_until_resolved: None, + }), + events, + ) { + crate::game::zone_pipeline::BatchMoveResult::Done => { + // The completion ran inline (`finish_with_continuation`), so + // `state.waiting_for` is the post-drain priority/continuation + // state. + ResolutionChoiceOutcome::WaitingFor(state.waiting_for.clone()) + } + crate::game::zone_pipeline::BatchMoveResult::NeedsChoice => { + ResolutionChoiceOutcome::WaitingFor(state.waiting_for.clone()) + } } - state.revealed_cards.remove(&hit_card); - ResolutionChoiceOutcome::WaitingFor(finish_with_continuation(state, player, events)) } // CR 107.1c + CR 608.2c: "you may repeat this process any number of // times" — after one iteration resolved, the controller decides @@ -3198,6 +3269,61 @@ fn set_priority(state: &mut GameState, player: crate::types::player::PlayerId) { state.priority_player = player; } +/// CR 614.6 + CR 616.1: Move a reveal-until *kept* card to a non-battlefield +/// destination (`accept_zone` / `decline_zone`) through the zone-change pipeline +/// so a `Moved` graveyard→exile redirect (Rest in Peace / Leyline of the Void) +/// fires on it — the 4 `kept_destination: Graveyard` reveal-until cards (Mind +/// Funeral class) previously dropped that redirect via the raw mover. +/// +/// Returns `Some(parked_outcome)` when the move pauses on a CR 616.1 ordering +/// choice: the rest-pile move + reveal-marker clear are deferred onto a +/// `RevealRestPile` completion (so the misses do not strand and the cleanup runs +/// once on resume), and the caller must return that outcome. Returns `None` when +/// the move completed synchronously and the caller should proceed to move the +/// rest pile inline. `emit_reveal_until_resolved` is `None` — the kept-choice +/// path already emitted `EffectResolved` before this prompt. +fn route_kept_card_or_defer( + state: &mut GameState, + hit_card: ObjectId, + destination: Zone, + source_id: ObjectId, + misses: &[ObjectId], + rest_destination: Zone, + events: &mut Vec, +) -> Option { + let player = state + .objects + .get(&hit_card) + .map(|obj| obj.controller) + .unwrap_or(state.active_player); + match crate::game::zone_pipeline::move_object( + state, + crate::game::zone_pipeline::ZoneMoveRequest::effect(hit_card, destination, source_id), + events, + ) { + crate::game::zone_pipeline::ZoneMoveResult::Done => None, + crate::game::zone_pipeline::ZoneMoveResult::NeedsChoice(_) + | crate::game::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => { + let mut clear_markers = misses.to_vec(); + clear_markers.push(hit_card); + crate::game::zone_pipeline::defer_completion_on_pause( + state, + crate::types::game_state::BatchCompletion::RevealRestPile { + player, + rest_cards: misses.to_vec(), + rest_destination, + clear_markers, + publish_tracked_set: None, + emit_reveal_until_resolved: None, + }, + ); + Some(ResolutionChoiceOutcome::WaitingFor( + state.waiting_for.clone(), + )) + } + } +} + fn starts_with_pay_amount_prompt(ability: &ResolvedAbility) -> bool { match &ability.effect { Effect::PayCost { From 1ecd04ae4ec30750325931614bbdc35d9ec8f0a3 Mon Sep 17 00:00:00 2001 From: Matt Evans <1388610+matthewevans@users.noreply.github.com> Date: Wed, 10 Jun 2026 10:06:50 -0700 Subject: [PATCH 08/11] fix(engine): paused face-down entry resumes face down; prevented-ETB fallback consults Moved redirects (CR 708.3 / 608.3e / 614.6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 review findings 1 (HIGH) + 3 (MEDIUM) on the zone-pipeline tail migration — both in handle_replacement_choice: 1. The ZoneChange resume arm destructured the approved event with '..', DISCARDING face_down_profile, and delivered via the raw mover — a morph / manifest entry parked on a CR 616.1 ordering prompt resumed FACE UP, violating CR 708.3 and leaking the morpher's hidden card. The prompt is REACHABLE: two co-played external enter-tapped Moved effects (Authority of the Consuls + Imposing Sovereign class) collide on the entry's tap field (no same-value dedupe), surfacing the ordering choice — empirically proven by the new test's parked-prompt assertion. Fix: extract the tail's CR 708.3 block into zone_pipeline::apply_face_down_entry_profile (single authority, not a mirrored copy) and call it from the resume arm immediately after the move, before the tap/controller/counter blocks (tail ordering). Full tail-routing via approve_post_replacement + deliver was assessed and deferred to the Phase-B token migration with a flagged TODO: the arm's epilogue drains post_replacement_continuation with spell-resolution ctx + post_replacement_source clearing, orders pending_spell_resolution differently, and carries the bespoke played_from_zone preservation (PLAN Open Question #3) — three behavioral divergences that need their own reconciliation, not a drive-by. Morph/manifest 'pause unreachable' comments replaced with honest reachability documentation (the bail is complete: the profile rides the parked event; the resume applies it). 2. The CR 608.3e prevented-ETB graveyard fallback delivered raw. The consulted (prevented) event was the battlefield ENTRY; the fallback is a fresh, never-consulted event, so routing it through move_object (SpellResolutionDefault, mirroring stack.rs's C2 prevented-permanent site) cannot double-apply — the prevention def is Battlefield-scoped and cannot re-match a Graveyard move. RIP/Leyline redirects now fire on the discarded spell. The dead pending_continuation is cleared before the move so a CR 616.1 pause cannot leave it for the next resume's epilogue. Fail-first evidence (both red before the fix, green after): - paused_face_down_morph_entry_resumes_face_down: panicked 'resumed morph entry must be FACE DOWN (CR 708.3)' - prevented_etb_graveyard_fallback_consults_moved_redirects: left: Graveyard, right: Exile (staging note in-test: no ZoneChange applier can yield Prevented, so the parked choice is staged as a regeneration-shield Destroy prevention; the resume is driven through the real GameAction entry) CR grep (docs/MagicCompRules.txt): 708.2a face-down characteristics; 708.3 turned face down before it enters; 608.3e prevented ETB to graveyard; 614.6 replaced event never happens; 616.1 multiple replacement ordering. --- crates/engine/src/game/engine_replacement.rs | 182 ++++++++++++++++++- crates/engine/src/game/morph.rs | 121 +++++++++++- crates/engine/src/game/zone_pipeline.rs | 41 ++++- 3 files changed, 325 insertions(+), 19 deletions(-) diff --git a/crates/engine/src/game/engine_replacement.rs b/crates/engine/src/game/engine_replacement.rs index a5223b5d82..83bc3f4b80 100644 --- a/crates/engine/src/game/engine_replacement.rs +++ b/crates/engine/src/game/engine_replacement.rs @@ -77,6 +77,24 @@ pub(super) fn handle_replacement_choice( let mut zone_change_object_id = None; let mut enters_battlefield = false; match event { + // TODO(zone-pipeline Phase B): this arm is a divergent partial + // copy of `zone_pipeline::deliver_replaced_zone_change` — it has + // now dropped two fields (`played_from_zone`, patched below; + // `face_down_profile`, patched via the shared CR 708.3 helper) + // and still skips the tail's devour snapshot, + // EntersWithAdditionalCounters statics, `attach_to`, + // `entered_via_ability_source`, exile-link tracking, and the + // CR 701.24a library-shuffle arms. Route it through + // `ApprovedZoneChange::approve_post_replacement` + + // `zone_pipeline::deliver` when the Phase-B token migration + // reconciles the divergences that block it this round: (1) the + // post-`Execute` epilogue below drains + // `post_replacement_continuation` with the spell-resolution ctx + // and clears `post_replacement_source` for zone changes, while + // the tail drains it ctx-less and without the clear; (2) + // `pending_spell_resolution` ordering (the tail's drain would run + // before `apply_pending_spell_resolution`); (3) the bespoke + // `played_from_zone` preservation (PLAN Open Question #3). ProposedEvent::ZoneChange { object_id, to, @@ -85,6 +103,7 @@ pub(super) fn handle_replacement_choice( enter_with_counters, controller_override, enter_transformed, + face_down_profile, .. } => { let played_from_zone = state @@ -92,6 +111,22 @@ pub(super) fn handle_replacement_choice( .get(&object_id) .and_then(|obj| obj.played_from_zone); zones::move_to_zone(state, object_id, to, events); + // CR 708.3 + CR 708.2a: a face-down entry (morph / manifest / + // "put it onto the battlefield face down") that parked on a + // CR 616.1 ordering prompt must still enter FACE DOWN — + // shared single authority with the delivery tail. Applied + // immediately after the move and before the tap-state / + // controller-override / ETB-counter blocks, mirroring the + // tail's ordering so any later-applied triggers see the + // face-down state. Discarding this field delivered the + // resumed morph face-up, leaking the hidden card. + if to == Zone::Battlefield { + if let Some(profile) = &face_down_profile { + crate::game::zone_pipeline::apply_face_down_entry_profile( + state, object_id, profile, + ); + } + } // CR 400.7: reset_for_battlefield_entry (inside move_to_zone) sets // defaults. Override only when the replacement pipeline changed them. if to == Zone::Battlefield { @@ -601,10 +636,37 @@ pub(super) fn handle_replacement_choice( } // CR 608.3e: If the ETB was prevented during spell resolution, // the permanent goes to the graveyard instead. + // + // CR 614.6: this graveyard fallback is a FRESH, never-consulted + // event — the consulted (and prevented) event was the battlefield + // ENTRY (`to: Battlefield`), so routing the fallback through the + // pipeline cannot double-apply: the prevention definition is + // Battlefield-scoped and cannot re-match a →Graveyard move. A + // board-wide `Moved` graveyard→exile redirect (Rest in Peace / + // Leyline of the Void) now fires on the discarded spell — the + // un-migrated twin of stack.rs's C2 prevented-permanent site. The + // dead continuation is cleared BEFORE the move so a CR 616.1 + // ordering pause (two simultaneous redirects) cannot leave it for + // the next resume's epilogue to drain; on a pause, surface the + // parked prompt (its resume delivers the chosen event through the + // ZoneChange arm above). + state.pending_continuation = None; if let Some(ctx) = state.pending_spell_resolution.take() { - zones::move_to_zone(state, ctx.object_id, Zone::Graveyard, events); + match crate::game::zone_pipeline::move_object( + state, + crate::game::zone_pipeline::ZoneMoveRequest::spell_resolution_default( + ctx.object_id, + Zone::Graveyard, + ), + events, + ) { + crate::game::zone_pipeline::ZoneMoveResult::Done => {} + crate::game::zone_pipeline::ZoneMoveResult::NeedsChoice(_) + | crate::game::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => { + return Ok(state.waiting_for.clone()); + } + } } - state.pending_continuation = None; Ok(WaitingFor::Priority { player: state.active_player, }) @@ -1294,6 +1356,122 @@ mod tests { ); } + /// CR 608.3e + CR 614.6 discriminating test (fail-first): when a permanent + /// spell's ETB is fully prevented after a replacement choice + /// (`ReplacementResult::Prevented` while `pending_spell_resolution` is set), + /// the graveyard fallback is a FRESH, never-consulted event — it must route + /// through the zone pipeline so a board-wide `Moved` graveyard→exile + /// redirect (Rest in Peace / Leyline of the Void) fires on the discarded + /// spell. The raw `move_to_zone` fallback dropped the redirect — the + /// un-migrated twin of the stack.rs C2 prevented-permanent site. + /// + /// STAGING NOTE: no ZoneChange registry applier can yield `Prevented` + /// today, so the natural entry-prevention pause is not constructible + /// end-to-end; the parked choice is staged as a regeneration-shield Destroy + /// prevention (the canonical `Prevented` producer) with + /// `pending_spell_resolution` set. The assertion target — + /// `handle_replacement_choice`'s Prevented-arm CR 608.3e fallback — is + /// driven through the real `GameAction::ChooseReplacement` resume entry. + #[test] + fn prevented_etb_graveyard_fallback_consults_moved_redirects() { + use crate::types::ability::AbilityDefinition; + use crate::types::ability::Effect; + use crate::types::ability::TargetFilter; + use crate::types::game_state::{CastingVariant, PendingSpellResolution}; + use crate::types::proposed_event::ReplacementId; + + let mut state = GameState::new_two_player(42); + + // The resolving permanent spell, still on the stack (CR 608.3e: its + // prevented ETB routes it to its owner's graveyard instead). + let spell = create_object( + &mut state, + CardId(50), + PlayerId(0), + "Prevented Permanent".to_string(), + Zone::Stack, + ); + + // Rest in Peace–class graveyard→exile Moved redirect on the battlefield. + let rip = make_creature(&mut state, PlayerId(1), "Rest in Peace"); + state.objects.get_mut(&rip).unwrap().replacement_definitions = + vec![ReplacementDefinition::new(ReplacementEvent::Moved) + .destination_zone(Zone::Graveyard) + .execute(AbilityDefinition::new( + AbilityKind::Spell, + Effect::ChangeZone { + destination: Zone::Exile, + origin: None, + target: TargetFilter::SelfRef, + owner_library: false, + enter_transformed: false, + enters_under: None, + enter_tapped: crate::types::zones::EtbTapState::Unspecified, + enters_attacking: false, + up_to: false, + enter_with_counters: vec![], + face_down_profile: None, + }, + ))] + .into(); + + // The paused entry's spell-resolution bookkeeping. + state.pending_spell_resolution = Some(PendingSpellResolution { + object_id: spell, + controller: PlayerId(0), + casting_variant: CastingVariant::Normal, + cast_from_zone: None, + cast_timing_permission: None, + spell_targets: vec![], + actual_mana_spent: 0, + kickers_paid: vec![], + additional_cost_payment_count: 0, + convoked_creatures: vec![], + }); + + // Staged Prevented producer: a regeneration shield on a creature being + // destroyed — choosing it yields `ReplacementResult::Prevented`. + let bear = make_creature(&mut state, PlayerId(0), "Bear"); + state + .objects + .get_mut(&bear) + .unwrap() + .replacement_definitions = vec![ReplacementDefinition::new(ReplacementEvent::Destroy) + .regeneration_shield() + .description("Regenerate".to_string())] + .into(); + state.pending_replacement = Some(crate::types::game_state::PendingReplacement { + proposed: ProposedEvent::Destroy { + object_id: bear, + source: None, + cant_regenerate: false, + applied: std::collections::HashSet::new(), + }, + candidates: vec![ReplacementId { + source: bear, + index: 0, + }], + depth: 0, + is_optional: false, + }); + state.waiting_for = replacement_mod::replacement_choice_waiting_for(PlayerId(0), &state); + state.priority_player = PlayerId(0); + + apply_as_current(&mut state, GameAction::ChooseReplacement { index: 0 }) + .expect("resume replacement choice"); + + assert_eq!( + state.objects[&spell].zone, + Zone::Exile, + "prevented-ETB graveyard fallback must consult the graveyard→exile \ + Moved redirect (CR 614.6) — raw delivery left the spell in the graveyard" + ); + assert!( + !state.players[0].graveyard.contains(&spell), + "the spell must not reach the graveyard with Rest in Peace out" + ); + } + #[test] fn zone_change_replacement_choice_preserves_land_play_provenance() { let mut state = GameState::new_two_player(42); diff --git a/crates/engine/src/game/morph.rs b/crates/engine/src/game/morph.rs index 9ce2e59a5d..8afe31e11e 100644 --- a/crates/engine/src/game/morph.rs +++ b/crates/engine/src/game/morph.rs @@ -130,10 +130,16 @@ pub fn play_face_down( // dropped (the tail is the single authority, mirroring `manifest_card` and // change_zone's face-down path). // - // CR 616.1 / CR 303.4f: a face-down 2/2 vanilla creature is not an Aura and - // carries no Moved redirect or counter-replacement choice, so a battlefield- - // entry pause is unreachable; the bail keeps the helper safe by construction - // (the prompt is parked centrally by `move_object`). + // CR 616.1: a battlefield-entry pause IS reachable here — two co-played + // external enter-tapped `Moved` effects (Authority of the Consuls + + // Imposing Sovereign class) both write the entry event's tap field, a + // material same-field collision that surfaces an ordering prompt (see + // `paused_face_down_morph_entry_resumes_face_down`). The bail is correct + // and complete: the face-down profile rides the parked event, and the + // resume path (`engine_replacement::handle_replacement_choice`'s ZoneChange + // arm) applies it through the shared CR 708.3 helper + // (`zone_pipeline::apply_face_down_entry_profile`), so the entry resumes + // face down with nothing left for this helper to do. match super::zone_pipeline::move_object( state, super::zone_pipeline::ZoneMoveRequest::effect(object_id, Zone::Battlefield, object_id) @@ -248,9 +254,13 @@ pub fn manifest_card( // which the raw `move_to_zone` + manual override skipped. The manual // post-move override is dropped (the tail is the single authority). // - // CR 616.1 / CR 303.4f: a face-down 2/2 vanilla creature is not an Aura and - // carries no Moved redirect or counter-replacement choice, so a battlefield- - // entry pause is unreachable; the bail keeps the helper safe by construction. + // CR 616.1: a battlefield-entry pause IS reachable — two co-played external + // enter-tapped `Moved` effects (Authority of the Consuls + Imposing + // Sovereign class) collide on the entry's tap field and surface an ordering + // prompt. The bail is correct and complete: the face-down profile rides the + // parked event and the resume path applies it through the shared CR 708.3 + // helper (`zone_pipeline::apply_face_down_entry_profile`), so the manifest + // resumes face down with nothing left for this helper to do. match super::zone_pipeline::move_object( state, super::zone_pipeline::ZoneMoveRequest::effect(object_id, Zone::Battlefield, object_id) @@ -370,6 +380,103 @@ mod tests { assert!(obj.color.is_empty()); } + /// CR 616.1 + CR 708.3 discriminating test (fail-first): a face-down morph + /// entry parked on a replacement-ordering prompt must resume FACE DOWN. + /// + /// Reachability: two co-played external enter-tapped `Moved` defs (Authority + /// of the Consuls + Imposing Sovereign class — both parse as ChangeZone + /// Moved defs) both write the entry event's tap field; same-field writes are + /// non-commuting and the engine has no same-value dedupe, so the set is + /// material and CR 616.1 prompts — `move_object` parks the morph entry. + /// + /// The resume path (`handle_replacement_choice`'s ZoneChange arm) previously + /// destructured the approved event with `..`, DISCARDING + /// `face_down_profile`, and delivered via the raw mover — the morph resumed + /// FACE UP, violating CR 708.3 and leaking the hidden card to the opponent. + #[test] + fn paused_face_down_morph_entry_resumes_face_down() { + use crate::game::engine::apply_as_current; + use crate::game::game_object::GameObject; + use crate::types::ability::{ReplacementDefinition, TargetFilter}; + use crate::types::actions::GameAction; + use crate::types::game_state::WaitingFor; + use crate::types::replacements::ReplacementEvent; + + let mut state = GameState::new_two_player(42); + let player = PlayerId(0); + + // Two external enter-tapped Moved replacements on the opponent's board. + for (offset, name) in [ + (0u64, "Authority of the Consuls"), + (1, "Imposing Sovereign"), + ] { + let oid = ObjectId(9000 + offset); + let mut src = GameObject::new( + oid, + CardId(900 + offset), + PlayerId(1), + name.to_string(), + Zone::Battlefield, + ); + src.replacement_definitions = vec![ReplacementDefinition::new(ReplacementEvent::Moved) + .execute(AbilityDefinition::new( + crate::types::ability::AbilityKind::Spell, + crate::types::ability::Effect::Tap { + target: TargetFilter::SelfRef, + }, + )) + .destination_zone(Zone::Battlefield) + .description(name.to_string())] + .into(); + state.objects.insert(oid, src); + state.battlefield.push_back(oid); + } + + let id = setup_morph_creature(&mut state, player); + let mut events = Vec::new(); + play_face_down(&mut state, player, id, &mut events).unwrap(); + + // CR 616.1: the colliding enter-tapped writes parked the entry — the + // card has NOT moved yet and the prompt is live. + let WaitingFor::ReplacementChoice { + player: chooser, .. + } = state.waiting_for.clone() + else { + panic!( + "expected parked ReplacementChoice for the enter-tapped collision, got {:?}", + state.waiting_for + ); + }; + assert_eq!( + state.objects[&id].zone, + Zone::Hand, + "entry must be parked, not delivered, while the prompt is live" + ); + state.priority_player = chooser; + + apply_as_current(&mut state, GameAction::ChooseReplacement { index: 0 }) + .expect("resume replacement choice"); + + let obj = &state.objects[&id]; + assert_eq!(obj.zone, Zone::Battlefield, "entry delivered after resume"); + assert!( + obj.tapped, + "both enter-tapped replacements applied to the resumed entry" + ); + assert!( + obj.face_down, + "resumed morph entry must be FACE DOWN (CR 708.3) — face-up resume leaks the hidden card" + ); + assert_eq!(obj.power, Some(2), "vanilla 2/2 face-down profile"); + assert_eq!(obj.toughness, Some(2), "vanilla 2/2 face-down profile"); + assert_eq!(obj.name, "", "face-down profile hides the printed name"); + assert!(obj.card_types.subtypes.is_empty()); + assert!( + obj.back_face.is_some(), + "real face snapshot stored so turn-face-up can restore it" + ); + } + #[test] fn turn_face_up_restores_original_characteristics() { let mut state = GameState::new_two_player(42); diff --git a/crates/engine/src/game/zone_pipeline.rs b/crates/engine/src/game/zone_pipeline.rs index 3f0cb9ce32..e5013cbfe5 100644 --- a/crates/engine/src/game/zone_pipeline.rs +++ b/crates/engine/src/game/zone_pipeline.rs @@ -1078,6 +1078,31 @@ fn legal_aura_attachment_targets( targets } +/// CR 708.3 + CR 708.2a: Turn an object face down as part of its battlefield +/// entry — snapshot the real face into `back_face`, then overwrite the live +/// characteristics with the face-down profile (the morph/manifest vanilla 2/2 +/// plus any effect-specified extra types/subtypes) so the original is +/// restorable by `turn_face_up`. Mirrors `manifest_card`'s historical sequence. +/// +/// Single authority shared by the normal delivery tail +/// (`deliver_replaced_zone_change`) and the replacement-choice resume arm +/// (`engine_replacement::handle_replacement_choice`). The resume arm previously +/// discarded the event's `face_down_profile`, so a face-down entry that parked +/// on a CR 616.1 ordering prompt (two external enter-tapped effects — Authority +/// of the Consuls + Imposing Sovereign class) resumed FACE UP, leaking the +/// morpher's hidden card. +pub(crate) fn apply_face_down_entry_profile( + state: &mut GameState, + object_id: ObjectId, + profile: &FaceDownProfile, +) { + if let Some(obj) = state.objects.get_mut(&object_id) { + let original = crate::game::printed_cards::snapshot_object_face(obj); + crate::game::morph::apply_face_down_creature_characteristics(obj, profile); + obj.back_face = Some(original); + } +} + /// Deliver a zone-change event that has already passed through replacement. pub(crate) fn deliver_replaced_zone_change( state: &mut GameState, @@ -1142,18 +1167,14 @@ pub(crate) fn deliver_replaced_zone_change( // CR 708.3: An object put onto the battlefield face down is turned face // down BEFORE it enters, so its ETB abilities don't trigger and its // characteristics are the face-down profile (CR 708.2a), not the real - // card's. Mirror `manifest_card`'s sequence: snapshot the real face into - // `back_face`, overwrite with the face-down 2/2 (+ any specified extra - // types/subtypes), then store the snapshot so the original is restorable. - // Done before the controller-override and ETB-counter/trigger blocks - // below so triggers (if any later applied) see the face-down state. + // card's. Done before the controller-override and ETB-counter/trigger + // blocks below so triggers (if any later applied) see the face-down + // state. Shared single authority with the replacement-choice resume arm + // (`engine_replacement::handle_replacement_choice`), so a paused + // face-down entry cannot resume face-up. if to == Zone::Battlefield { if let Some(profile) = &face_down_profile { - if let Some(obj) = state.objects.get_mut(&object_id) { - let original = crate::game::printed_cards::snapshot_object_face(obj); - crate::game::morph::apply_face_down_creature_characteristics(obj, profile); - obj.back_face = Some(original); - } + apply_face_down_entry_profile(state, object_id, profile); } } // CR 712.14a: Apply transformation if entering the battlefield transformed. From 1f69ced18ebae1d5b9ab2f5ad73d8029c835f621 Mon Sep 17 00:00:00 2001 From: Matt Evans <1388610+matthewevans@users.noreply.github.com> Date: Wed, 10 Jun 2026 10:39:13 -0700 Subject: [PATCH 09/11] fix(engine): ninjutsu and Attraction-open pauses resume via continuations instead of dropping post-entry work (CR 702.49 / 701.51 / 616.1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 review finding 2 (MEDIUM): both 3d1497411 bails were wrong because the battlefield-entry prompt IS reachable — two co-played external enter-tapped Moved effects (Authority of the Consuls + Imposing Sovereign for creatures; Kismet / Frozen Aether class for artifacts) produce a material same-field collision on the entry's tap state (no same-value dedupe) and surface a CR 616.1 ordering prompt. On that board: - ninjutsu: the bail skipped the cast-variant provenance tag AND the CR 702.49c tapped-and-attacking combat placement — the resumed ninja entered untagged and non-attacking. - Attraction open: the bail left in_attraction_deck set, never emitted AttractionOpened, and dropped every remaining open of the instruction. Adjudication: continuations, not documented limitation — the BatchCompletion infrastructure already exists (RevealRestPile / RemoveExileLinks shape). Two new variants: NinjutsuPlacement (defers finish_ninjutsu_entry: tag + combat placement + NinjutsuActivated + layers) and AttractionOpenRemainder (defers finish_attraction_open + the remaining opens, which may themselves re-park and re-defer through the same completion — the drain takes the old record before running it, so a fresh park is preserved). Both finish helpers are shared single authorities between the synchronous and resumed paths. The 'pause unreachable' comments are replaced with honest reachability documentation. Bonus CR-correctness in the shared helper: AttractionOpened now fires only when the card actually entered the battlefield (CR 701.51c — prevented or replaced entries must not trigger 'opens an Attraction'; Done also covers prevented/redirected deliveries). Also corrects the CR 610.3a cite to CR 610.3 proper on the RemoveExileLinks doc + completion arm (610.3a is the already-occurred-event timing subpart; 610.3 is the return rule). Fail-first evidence (both red before, green after): - paused_ninjutsu_entry_resumes_with_combat_placement_and_tag: panicked 'resumed ninja must be placed attacking (CR 702.49c)' - paused_attraction_open_resumes_bookkeeping_and_remaining_opens: panicked 'open bookkeeping must run on the resumed Attraction (old bail left the flag set)' Both tests passed their parked-prompt assertions pre-fix, empirically confirming the reviewer's reachability claim for both object classes. CR grep (docs/MagicCompRules.txt): 702.49/49a/49c ninjutsu + placement; 701.51/51b/51c open an Attraction + trigger gate; 616.1 ordering; 610.3 return-to-previous-zone. --- crates/engine/src/game/attractions.rs | 182 ++++++++++++++++-- .../src/game/engine_resolution_choices.rs | 41 +++- crates/engine/src/game/keywords.rs | 150 +++++++++++++-- crates/engine/src/types/game_state.rs | 38 +++- 4 files changed, 380 insertions(+), 31 deletions(-) diff --git a/crates/engine/src/game/attractions.rs b/crates/engine/src/game/attractions.rs index c77a05c1b9..59405bd98a 100644 --- a/crates/engine/src/game/attractions.rs +++ b/crates/engine/src/game/attractions.rs @@ -42,7 +42,7 @@ pub fn open_attractions( count: u32, events: &mut Vec, ) -> Result<(), EffectError> { - for _ in 0..count { + for opened in 0..count { let Some(object_id) = state .players .iter_mut() @@ -60,13 +60,16 @@ pub fn open_attractions( // Attraction never received them. CR 400.7 attributes the entry to the // opened object itself (the pre-pipeline raw move recorded no source). // - // CR 616.1 / CR 303.4f: a battlefield-entry pause is not reachable for - // an Attraction — it is a non-Aura artifact (no aura-host choice), and - // no `Moved` redirect or counter-replacement that targets an Attraction - // entry surfaces a CR 616.1 ordering choice in the supported pool. The - // bail keeps the loop safe by construction: on a pause the prompt is - // parked centrally by `move_object`, so stop opening rather than - // emitting `AttractionOpened` over a parked state. + // CR 616.1: a battlefield-entry pause IS reachable here — two co-played + // external enter-tapped `Moved` effects (the Kismet / Frozen Aether + // class parses as ChangeZone Moved defs) both write the entry event's + // tap field, a material same-field collision that surfaces an ordering + // prompt. On the pause, the paused Attraction's open bookkeeping and + // the REMAINING opens of this instruction are deferred onto a + // `BatchCompletion::AttractionOpenRemainder` so the replacement-choice + // resume runs them — the old bail `break` left `in_attraction_deck` + // set, never emitted `AttractionOpened`, and dropped the remaining + // opens. match super::zone_pipeline::move_object( state, super::zone_pipeline::ZoneMoveRequest::effect(object_id, Zone::Battlefield, object_id), @@ -74,17 +77,54 @@ pub fn open_attractions( ) { super::zone_pipeline::ZoneMoveResult::Done => {} super::zone_pipeline::ZoneMoveResult::NeedsChoice(_) - | super::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => break, - } - if let Some(obj) = state.objects.get_mut(&object_id) { - obj.in_attraction_deck = false; + | super::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => { + super::zone_pipeline::defer_completion_on_pause( + state, + crate::types::game_state::BatchCompletion::AttractionOpenRemainder { + player, + object_id, + remaining: count - opened - 1, + }, + ); + return Ok(()); + } } + finish_attraction_open(state, player, object_id, events); + } + Ok(()) +} + +/// CR 701.51b + CR 701.51c: Per-Attraction open bookkeeping, run exactly once +/// after the Attraction's battlefield entry delivers — inline on the +/// synchronous path, or from `BatchCompletion::AttractionOpenRemainder` when +/// the entry parked on a CR 616.1 replacement-ordering choice and resumed. +/// Clears the supplementary-deck membership flag and emits `AttractionOpened` +/// (the "whenever a player opens an Attraction" trigger event, which fires only +/// when the card actually entered the battlefield — CR 701.51c). +pub(crate) fn finish_attraction_open( + state: &mut GameState, + player: PlayerId, + object_id: ObjectId, + events: &mut Vec, +) { + if let Some(obj) = state.objects.get_mut(&object_id) { + obj.in_attraction_deck = false; + } + // CR 701.51c: the "opens an Attraction" trigger fires only when the card + // actually entered the battlefield — "If an effect prevents that Attraction + // from entering the battlefield or replaces entering the battlefield with + // another event, that ability doesn't trigger." `ZoneMoveResult::Done` + // also covers prevented/redirected deliveries, so gate on arrival. + if state + .objects + .get(&object_id) + .is_some_and(|obj| obj.zone == Zone::Battlefield) + { events.push(GameEvent::AttractionOpened { player_id: player, object_id, }); } - Ok(()) } /// CR 701.52a: Roll a d6 and visit each controlled Attraction whose lights include the result. @@ -175,3 +215,119 @@ pub fn resolve_roll_to_visit( }); Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::game::engine::apply_as_current; + use crate::game::zones::create_object; + use crate::types::ability::{ + AbilityDefinition, AbilityKind, Effect, ReplacementDefinition, TargetFilter, + }; + use crate::types::actions::GameAction; + use crate::types::game_state::WaitingFor; + use crate::types::identifiers::CardId; + use crate::types::replacements::ReplacementEvent; + + /// CR 701.51 + CR 616.1 discriminating test (fail-first): an Attraction + /// whose battlefield entry parks on a replacement-ordering prompt (two + /// co-played external enter-tapped `Moved` effects — the Kismet / Frozen + /// Aether class parses as ChangeZone Moved defs and collides on the entry's + /// tap field) must, after the prompt is answered, still receive its open + /// bookkeeping (`in_attraction_deck` cleared, `AttractionOpened` emitted) + /// AND the remaining opens of the same instruction must still happen. The + /// old bail `break` skipped the bookkeeping on the paused Attraction and + /// silently dropped every remaining open. + #[test] + fn paused_attraction_open_resumes_bookkeeping_and_remaining_opens() { + let mut state = GameState::new_two_player(42); + let player = PlayerId(0); + + // Two Attractions in the supplementary deck (command zone). + let mut attractions = Vec::new(); + for i in 0..2u64 { + let id = create_object( + &mut state, + CardId(100 + i), + player, + format!("Attraction {i}"), + Zone::Command, + ); + state.objects.get_mut(&id).unwrap().in_attraction_deck = true; + state.players[0].attraction_deck.push_back(id); + attractions.push(id); + } + + // Two external enter-tapped Moved replacements (Kismet / Frozen Aether + // class) — both write the entry event's tap field, so CR 616.1 prompts. + for (offset, name) in [(0u64, "Kismet"), (1, "Frozen Aether")] { + let oid = ObjectId(9000 + offset); + let mut src = GameObject::new( + oid, + CardId(900 + offset), + PlayerId(1), + name.to_string(), + Zone::Battlefield, + ); + src.replacement_definitions = vec![ReplacementDefinition::new(ReplacementEvent::Moved) + .execute(AbilityDefinition::new( + AbilityKind::Spell, + Effect::Tap { + target: TargetFilter::SelfRef, + }, + )) + .destination_zone(Zone::Battlefield) + .description(name.to_string())] + .into(); + state.objects.insert(oid, src); + state.battlefield.push_back(oid); + } + + let mut events = Vec::new(); + open_attractions(&mut state, player, 2, &mut events).expect("open attractions"); + + // CR 616.1: the first open parked on the enter-tapped collision. + let WaitingFor::ReplacementChoice { + player: chooser, .. + } = state.waiting_for.clone() + else { + panic!( + "expected parked ReplacementChoice for the enter-tapped collision, got {:?}", + state.waiting_for + ); + }; + state.priority_player = chooser; + apply_as_current(&mut state, GameAction::ChooseReplacement { index: 0 }) + .expect("resume first open"); + + // The first Attraction's open bookkeeping ran on resume. + let first = &state.objects[&attractions[0]]; + assert_eq!(first.zone, Zone::Battlefield, "first Attraction delivered"); + assert!( + !first.in_attraction_deck, + "open bookkeeping must run on the resumed Attraction (old bail left the flag set)" + ); + + // The remaining open ran — and re-parked on its own entry prompt. + let WaitingFor::ReplacementChoice { + player: chooser2, .. + } = state.waiting_for.clone() + else { + panic!( + "remaining open must run after the pause and re-park, got {:?} (old bail dropped it)", + state.waiting_for + ); + }; + state.priority_player = chooser2; + apply_as_current(&mut state, GameAction::ChooseReplacement { index: 0 }) + .expect("resume second open"); + + let second = &state.objects[&attractions[1]]; + assert_eq!( + second.zone, + Zone::Battlefield, + "remaining open must deliver after the pause (old bail dropped it)" + ); + assert!(!second.in_attraction_deck); + } +} diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index 24c0c75782..1b5aafd012 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -3432,7 +3432,7 @@ pub(crate) fn run_batch_completion( } finish_with_continuation(state, player, events); } - // CR 610.3a: the exile-until-leaves return pile has fully landed (after a + // CR 610.3: the exile-until-leaves return pile has fully landed (after a // returned creature's as-enters / aura-host pause resolved). Drop the // spent `UntilSourceLeaves` links now — deferred so it runs exactly once // after the paused card finished returning, not before. No priority / @@ -3443,6 +3443,45 @@ pub(crate) fn run_batch_completion( .exile_links .retain(|link| !returned_ids.contains(&link.exiled_id)); } + // CR 702.49 + CR 616.1: the ninja's parked battlefield entry resolved — + // run the deferred post-entry ninjutsu work (cast-variant tag, + // CR 702.49c combat placement, CR 702.49a trigger event) exactly once. + // No priority/continuation drain: ninjutsu is a keyword activation + // whose surrounding action pipeline owns priority. + BatchCompletion::NinjutsuPlacement { + player, + ninjutsu_obj_id, + cast_variant, + defending_player, + attack_target, + } => { + crate::game::keywords::finish_ninjutsu_entry( + state, + player, + ninjutsu_obj_id, + cast_variant, + defending_player, + attack_target, + events, + ); + } + // CR 701.51 + CR 616.1: the paused Attraction's entry resolved — finish + // its open bookkeeping, then run the remaining opens of the same + // instruction (which may themselves pause and re-defer through this + // same completion; `drain_pending_batch_deliveries` took the old record + // before calling here, so a fresh park is preserved). + BatchCompletion::AttractionOpenRemainder { + player, + object_id, + remaining, + } => { + crate::game::attractions::finish_attraction_open(state, player, object_id, events); + if remaining > 0 { + // CR 609.3 inside: opens as many as possible; never errors. + let _ = + crate::game::attractions::open_attractions(state, player, remaining, events); + } + } } } diff --git a/crates/engine/src/game/keywords.rs b/crates/engine/src/game/keywords.rs index 3acc7521d6..0d07fd8e7b 100644 --- a/crates/engine/src/game/keywords.rs +++ b/crates/engine/src/game/keywords.rs @@ -1,9 +1,10 @@ use std::str::FromStr; +use crate::game::combat::AttackTarget; use crate::game::game_object::GameObject; use crate::game::zones; use crate::parser::oracle_util::parse_subtype; -use crate::types::ability::{AbilityCost, NinjutsuVariant}; +use crate::types::ability::{AbilityCost, CastVariantPaid, NinjutsuVariant}; use crate::types::events::GameEvent; use crate::types::game_state::GameState; use crate::types::identifiers::{CardId, ObjectId}; @@ -538,13 +539,16 @@ pub fn activate_ninjutsu( // the entry to the ninja itself (the pre-pipeline raw move recorded no // source; the cast-variant tag below records the ninjutsu provenance). // - // CR 616.1 / CR 303.4f: a battlefield-entry pause is not reachable here — - // the ninja is a creature (not an Aura, so no host choice), and no - // supported ninja entry surfaces a counter-replacement pause or a CR 616.1 - // multi-redirect ordering choice. The bail is a safety guard, not a resume - // path: the combat placement (CR 702.49c) below cannot resume across a - // pause, so on the (unreachable) pause we stop with the prompt parked rather - // than place the ninja attacking over a parked state. + // CR 616.1: a battlefield-entry pause IS reachable here — two co-played + // external enter-tapped `Moved` effects (Authority of the Consuls + + // Imposing Sovereign class) both write the entry event's tap field, a + // material same-field collision that surfaces an ordering prompt (see + // `paused_ninjutsu_entry_resumes_with_combat_placement_and_tag`). On the + // pause, the post-entry ninjutsu work (cast-variant tag + CR 702.49c combat + // placement + CR 702.49a trigger event) is deferred onto a + // `BatchCompletion::NinjutsuPlacement` so the replacement-choice resume + // runs it exactly once after the entry delivers — the old bail skipped it, + // leaving the resumed ninja untagged and non-attacking. match super::zone_pipeline::move_object( state, super::zone_pipeline::ZoneMoveRequest::effect( @@ -556,14 +560,52 @@ pub fn activate_ninjutsu( ) { super::zone_pipeline::ZoneMoveResult::Done => {} super::zone_pipeline::ZoneMoveResult::NeedsChoice(_) - | super::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => return Ok(()), + | super::zone_pipeline::ZoneMoveResult::NeedsAuraAttachmentChoice => { + super::zone_pipeline::defer_completion_on_pause( + state, + crate::types::game_state::BatchCompletion::NinjutsuPlacement { + player, + ninjutsu_obj_id, + cast_variant: variant.into(), + defending_player, + attack_target, + }, + ); + return Ok(()); + } } + finish_ninjutsu_entry( + state, + player, + ninjutsu_obj_id, + variant.into(), + defending_player, + attack_target, + events, + ); + + Ok(()) +} + +/// CR 702.49 + CR 702.49a + CR 702.49c: Post-entry ninjutsu work, run exactly +/// once after the ninja's battlefield entry delivers — inline on the +/// synchronous path, or from `BatchCompletion::NinjutsuPlacement` when the +/// entry parked on a CR 616.1 replacement-ordering choice and resumed. +pub(crate) fn finish_ninjutsu_entry( + state: &mut GameState, + player: PlayerId, + ninjutsu_obj_id: ObjectId, + cast_variant: CastVariantPaid, + defending_player: PlayerId, + attack_target: AttackTarget, + events: &mut Vec, +) { // CR 702.49: Track which alt-cost variant was paid this turn on the // cast-variant-paid tag (placement + tapped + summoning sickness is // delegated to the shared helper). if let Some(obj) = state.objects.get_mut(&ninjutsu_obj_id) { - obj.cast_variant_paid = Some((variant.into(), state.turn_number)); + obj.cast_variant_paid = Some((cast_variant, state.turn_number)); } // CR 702.49c: Place onto combat.attackers alongside the returned creature's @@ -583,8 +625,6 @@ pub fn activate_ninjutsu( }); crate::game::layers::mark_layers_full(state); - - Ok(()) } /// Detect which activated-family `NinjutsuVariant` a game object has, if any. @@ -1308,6 +1348,92 @@ mod tests { (state, attacker_id, ninja_id) } + /// CR 702.49c + CR 616.1 discriminating test (fail-first): a ninja whose + /// battlefield entry parks on a replacement-ordering prompt (two co-played + /// external enter-tapped `Moved` effects — Authority of the Consuls + + /// Imposing Sovereign class collide on the entry's tap field) must, after + /// the prompt is answered, still receive the FULL post-entry ninjutsu work: + /// the CR 702.49c tapped-and-attacking combat placement and the CR 702.49 + /// cast-variant provenance tag. The old bail skipped both — the resumed + /// ninja entered untagged and non-attacking. + #[test] + fn paused_ninjutsu_entry_resumes_with_combat_placement_and_tag() { + use crate::game::engine::apply_as_current; + use crate::types::ability::{ReplacementDefinition, TargetFilter}; + use crate::types::replacements::ReplacementEvent; + + let (mut state, attacker_id, ninja_id) = setup_ninjutsu_scenario(); + + // Two external enter-tapped Moved replacements on the opponent's board. + for (offset, name) in [ + (0u64, "Authority of the Consuls"), + (1, "Imposing Sovereign"), + ] { + let oid = ObjectId(9000 + offset); + let mut src = GameObject::new( + oid, + CardId(900 + offset), + PlayerId(1), + name.to_string(), + Zone::Battlefield, + ); + src.replacement_definitions = vec![ReplacementDefinition::new(ReplacementEvent::Moved) + .execute(AbilityDefinition::new( + AbilityKind::Spell, + Effect::Tap { + target: TargetFilter::SelfRef, + }, + )) + .destination_zone(Zone::Battlefield) + .description(name.to_string())] + .into(); + state.objects.insert(oid, src); + state.battlefield.push_back(oid); + } + + let mut events = Vec::new(); + activate_ninjutsu(&mut state, PlayerId(0), ninja_id, attacker_id, &mut events) + .expect("activation should succeed"); + + // CR 616.1: the colliding enter-tapped writes parked the ninja's entry. + let WaitingFor::ReplacementChoice { + player: chooser, .. + } = state.waiting_for.clone() + else { + panic!( + "expected parked ReplacementChoice for the enter-tapped collision, got {:?}", + state.waiting_for + ); + }; + assert_eq!( + state.objects[&ninja_id].zone, + Zone::Hand, + "ninja entry must be parked, not delivered, while the prompt is live" + ); + state.priority_player = chooser; + + apply_as_current(&mut state, GameAction::ChooseReplacement { index: 0 }) + .expect("resume replacement choice"); + + let ninja = &state.objects[&ninja_id]; + assert_eq!( + ninja.zone, + Zone::Battlefield, + "entry delivered after resume" + ); + assert!( + state + .combat + .as_ref() + .is_some_and(|c| c.attackers.iter().any(|a| a.object_id == ninja_id)), + "resumed ninja must be placed attacking (CR 702.49c) — the old bail skipped combat placement" + ); + assert!( + ninja.cast_variant_paid.is_some(), + "resumed ninja must carry the ninjutsu cast-variant tag (CR 702.49)" + ); + } + #[test] fn ninjutsu_returns_attacker_to_hand() { let (mut state, attacker_id, ninja_id) = setup_ninjutsu_scenario(); diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 9c80702b41..a960320ed6 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -7,10 +7,10 @@ use serde::{Deserialize, Serialize}; use super::ability::{ default_target_filter_permanent, AbilityCost, AbilityDefinition, AdditionalCost, AttackSubject, - BeholdCostAction, CategoryChooserScope, ChoiceType, ChoiceValue, ChooseFromZoneConstraint, - ChosenAttribute, Comparator, ContinuousModification, CostPaidObjectSnapshot, - CounterCostSelection, DelayedTriggerCondition, Duration, EffectKind, GameRestriction, - KeywordAction, KickerVariant, ModalChoice, QuantityExpr, ResolvedAbility, + BeholdCostAction, CastVariantPaid, CategoryChooserScope, ChoiceType, ChoiceValue, + ChooseFromZoneConstraint, ChosenAttribute, Comparator, ContinuousModification, + CostPaidObjectSnapshot, CounterCostSelection, DelayedTriggerCondition, Duration, EffectKind, + GameRestriction, KeywordAction, KickerVariant, ModalChoice, QuantityExpr, ResolvedAbility, SearchDestinationSplit, SearchSelectionConstraint, StaticCondition, TargetFilter, TargetRef, TriggerCondition, }; @@ -1151,7 +1151,7 @@ pub enum BatchCompletion { /// `EffectResolved` before the pause (or rely on the continuation). emit_reveal_until_resolved: Option, }, - /// CR 610.3a + CR 614.1c: An "exile until ~ leaves" return (Banisher Priest / + /// CR 610.3 + CR 614.1c: An "exile until ~ leaves" return (Banisher Priest / /// Fiend Hunter / Oblivion Ring class) routed its exiled cards back to the /// battlefield through the simultaneous-move batch so the delivery tail seeds /// enters-with-counters statics. A returned creature can pause on an @@ -1164,6 +1164,34 @@ pub enum BatchCompletion { /// this return and must be retained out of `state.exile_links`. returned_ids: Vec, }, + /// CR 702.49 + CR 616.1: A ninja entering via ninjutsu paused on a + /// battlefield-entry replacement-ordering choice (two co-played external + /// enter-tapped effects — Authority of the Consuls + Imposing Sovereign + /// class collide on the entry's tap field). The post-entry ninjutsu work — + /// the CR 702.49 cast-variant provenance tag, the CR 702.49c + /// tapped-and-attacking combat placement (no `AttackersDeclared`), and the + /// CR 702.49a `NinjutsuActivated` trigger event — cannot run before the + /// entry delivers; defer it onto the parked batch tail so the drain runs + /// it exactly once after the entry resolves. + NinjutsuPlacement { + player: PlayerId, + ninjutsu_obj_id: ObjectId, + cast_variant: CastVariantPaid, + defending_player: PlayerId, + attack_target: AttackTarget, + }, + /// CR 701.51 + CR 616.1: An Attraction being opened paused on a + /// battlefield-entry replacement-ordering choice (Kismet / Frozen Aether + /// class enter-tapped effects). Defer the paused Attraction's open + /// bookkeeping (`in_attraction_deck` clear + `AttractionOpened`) and the + /// remaining opens of the same instruction onto the parked batch tail — + /// the remaining opens may themselves pause and re-defer through this same + /// completion. + AttractionOpenRemainder { + player: PlayerId, + object_id: ObjectId, + remaining: u32, + }, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] From 9f33d9564f760fd6c1077e16b45d90775a9a5e76 Mon Sep 17 00:00:00 2001 From: Matt Evans <1388610+matthewevans@users.noreply.github.com> Date: Wed, 10 Jun 2026 10:39:45 -0700 Subject: [PATCH 10/11] docs(engine): flag CR 614.12 face-down consult gap; correct exile-return cite to CR 610.3 (review finding 4 + nit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 review finding 4 (MEDIUM-LOW, comment-only): execute_zone_move's replacement consult runs the matcher pass against the object's PRINTED characteristics, but CR 614.12 (docs/MagicCompRules.txt:3094) requires checking 'the characteristics of the permanent as it would exist on the battlefield' — for a face-down (morph/manifest) entry that is the 2/2 with no name/types/subtypes (CR 708.2a), so a type- or name-keyed entry replacement wrongly matches a face-down printed Wizard. Narrow class today (the common enter-tapped/counter statics are type-agnostic or creature-scoped, which the face-down 2/2 satisfies); the fix is profile-projected characteristics in the matcher pass when face_down_profile is present. Documented at the consult with the CR cite. Also corrects check_exile_returns' CR 610.3a cite to CR 610.3 proper (610.3a is the already-occurred-event timing subpart; 610.3 is the rule that creates the return one-shot). CR grep: 614.12 'check the characteristics of the permanent as it would exist on the battlefield'; 610.3 'A second one-shot effect ... returns the object to its previous zone.' No behavior change. --- crates/engine/src/game/engine.rs | 2 +- crates/engine/src/game/zone_pipeline.rs | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index ee8c5126a0..373db20719 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -6613,7 +6613,7 @@ pub(super) fn check_exile_returns(state: &mut GameState, events: &mut Vec { let mut pending_aura_choice: Option<(PlayerId, ObjectId, Vec)> = None; From f0b2c57666b9b2275a25d72ac7ccf25979c17baf Mon Sep 17 00:00:00 2001 From: Matt Evans <1388610+matthewevans@users.noreply.github.com> Date: Wed, 10 Jun 2026 11:02:44 -0700 Subject: [PATCH 11/11] fix(engine): arrival-gate ninjutsu post-entry work, twin of the Attraction CR 701.51c gate (CR 702.49c) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review LOW: finish_ninjutsu_entry ran the cast-variant tag and the CR 702.49c combat placement unconditionally — ZoneMoveResult::Done also covers prevented/redirected deliveries, so a redirected resumed entry would tag a non-battlefield object and place it into combat.attackers. Gate both behind the same zone == Battlefield arrival check as finish_attraction_open. Unreachable today (no supported Moved redirect retargets a battlefield entry away from the battlefield), but the gate makes the helper correct by construction rather than by census, matching the twin's standard. NinjutsuActivated stays deliberately OUTSIDE the gate, unlike the Attraction twin's AttractionOpened: CR 701.51c explicitly suppresses the 'opens an Attraction' trigger when the entry is prevented/replaced, but ninjutsu's activation event occurred when the ability was activated (cost paid, attacker returned) — a redirected entry does not un-activate it. Asymmetry documented inline. No new test per the review (unreachable class, identical behavior for every reachable entry — existing paused_ninjutsu_entry_resumes_with_combat_placement _and_tag still green). CR grep (docs/MagicCompRules.txt): 702.49c enters attacking; 701.51c trigger suppression on prevented/replaced entry. --- crates/engine/src/game/keywords.rs | 51 ++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/game/keywords.rs b/crates/engine/src/game/keywords.rs index 0d07fd8e7b..9510f2dd04 100644 --- a/crates/engine/src/game/keywords.rs +++ b/crates/engine/src/game/keywords.rs @@ -601,24 +601,43 @@ pub(crate) fn finish_ninjutsu_entry( attack_target: AttackTarget, events: &mut Vec, ) { - // CR 702.49: Track which alt-cost variant was paid this turn on the - // cast-variant-paid tag (placement + tapped + summoning sickness is - // delegated to the shared helper). - if let Some(obj) = state.objects.get_mut(&ninjutsu_obj_id) { - obj.cast_variant_paid = Some((cast_variant, state.turn_number)); - } + // Arrival gate (twin of `finish_attraction_open`'s CR 701.51c gate): the + // cast-variant tag and the CR 702.49c combat placement are battlefield + // semantics — `ZoneMoveResult::Done` also covers prevented/redirected + // deliveries, so running them unconditionally would tag a non-battlefield + // object and place it into `combat.attackers`. Unreachable today (no + // supported `Moved` redirect targets a battlefield entry's destination + // away from the battlefield), but the gate keeps the helper correct by + // construction rather than by census. + if state + .objects + .get(&ninjutsu_obj_id) + .is_some_and(|obj| obj.zone == Zone::Battlefield) + { + // CR 702.49: Track which alt-cost variant was paid this turn on the + // cast-variant-paid tag (placement + tapped + summoning sickness is + // delegated to the shared helper). + if let Some(obj) = state.objects.get_mut(&ninjutsu_obj_id) { + obj.cast_variant_paid = Some((cast_variant, state.turn_number)); + } - // CR 702.49c: Place onto combat.attackers alongside the returned creature's - // defender WITHOUT firing AttackersDeclared (no "whenever ~ attacks" triggers). - super::combat::place_attacking_alongside( - state, - ninjutsu_obj_id, - defending_player, - attack_target, - events, - ); + // CR 702.49c: Place onto combat.attackers alongside the returned creature's + // defender WITHOUT firing AttackersDeclared (no "whenever ~ attacks" triggers). + super::combat::place_attacking_alongside( + state, + ninjutsu_obj_id, + defending_player, + attack_target, + events, + ); + } - // CR 702.49a: Emit event for "whenever you activate a ninjutsu ability" triggers. + // CR 702.49a: Emit event for "whenever you activate a ninjutsu ability" + // triggers. Deliberately OUTSIDE the arrival gate, unlike the Attraction + // twin's `AttractionOpened`: CR 701.51c explicitly suppresses the "opens an + // Attraction" trigger when the entry is prevented/replaced, but ninjutsu's + // activation event occurred when the ability was activated (cost paid, + // attacker returned) — a redirected entry does not un-activate it. events.push(GameEvent::NinjutsuActivated { player_id: player, source_id: ninjutsu_obj_id,