diff --git a/crates/engine/src/game/cipher.rs b/crates/engine/src/game/cipher.rs index 4742cd469a..41a1de7cfd 100644 --- a/crates/engine/src/game/cipher.rs +++ b/crates/engine/src/game/cipher.rs @@ -127,12 +127,24 @@ pub(crate) fn finish_encode( } /// CR 702.99a: Begin the on-resolution encode offer for a Cipher spell. Returns -/// `true` when resolution paused for the choice (the caller must stop finalizing -/// the spell and return, leaving the card held off the stack like a mutating -/// spell), or `false` when there is no encode to offer — the spell isn't an -/// encodable cipher card, or the controller has no creature to host it — so the -/// caller routes the card normally (to its owner's graveyard). -pub fn begin_encode_choice(state: &mut GameState, card_id: ObjectId, controller: PlayerId) -> bool { +/// `true` when this hook has taken the card off the caller's hands — normally +/// because resolution paused for the choice (the caller stops finalizing the +/// spell and returns, leaving the card held off the stack like a mutating +/// spell), and in the one degenerate case below because the offer already +/// completed as a decline and routed the card itself. Returns `false` when there +/// is no encode to offer at all — the spell isn't an encodable cipher card, or +/// the controller has no creature to host it — so the caller routes the card +/// normally (to its owner's graveyard). +/// +/// Both `true` arms leave the caller with nothing to route, which is what makes +/// them one answer: the distinction that matters to a caller is whether the card +/// is still its responsibility. +pub fn begin_encode_choice( + state: &mut GameState, + card_id: ObjectId, + controller: PlayerId, + events: &mut Vec, +) -> bool { if !spell_can_encode(state, card_id) { return false; } @@ -140,14 +152,126 @@ pub fn begin_encode_choice(state: &mut GameState, card_id: ObjectId, controller: if creatures.is_empty() { return false; } - state.waiting_for = WaitingFor::CipherEncodeChoice { - player: controller, + let pending = crate::types::resolution::PendingCipherEncode { + stage: crate::types::resolution::CipherEncodeStage::Parked, card_id, + controller, creatures, }; + + // CR 702.99a: the encode is the spell's LAST instruction. When the spell's + // own effects are still paused on a player answer, this offer must not + // overwrite that live prompt (issue #7470) — it is parked BELOW the frame + // that owns the prompt and armed by `resume_resolution_frames` once that + // owner is consumed. Either way the caller's contract is the same: the + // resolution owes an answer, so the card is held off the stack. + park_encode_offer(state, pending, events); true } +/// Park the encode offer, arming it immediately only when nothing else owns the +/// current prompt. +/// +/// The offer always leaves this function accounted for: armed as the live +/// prompt, parked as a frame that will arm later, or — if the stack refuses a +/// prompt-less frame at all — completed as a decline. It is never dropped. +fn park_encode_offer( + state: &mut GameState, + pending: crate::types::resolution::PendingCipherEncode, + events: &mut Vec, +) { + // The question is not what SHAPE the top frame has — it is whether the + // resolution is currently asking the player anything at all. Keying this to + // `FrameGate::DirectChoice` missed the discard pause (Mental Vapors), whose + // frame owns a prompt without being a direct-choice owner. `waiting_for` + // is the engine's single answer to "is a question open", so ask it. + let resolution_paused = !matches!(state.waiting_for, WaitingFor::Priority { .. }); + if !resolution_paused { + let (player, card_id, creatures) = ( + pending.controller, + pending.card_id, + pending.creatures.clone(), + ); + // The frame and the prompt it may consume are installed as one step: + // a direct-choice owner that is visible with an unrelated `WaitingFor` + // is the very state #7470 left behind, and this authority makes the two + // unable to disagree. + let armed = crate::types::resolution::ResolutionFrame::CipherEncode( + crate::types::resolution::PendingCipherEncode { + stage: crate::types::resolution::CipherEncodeStage::Armed, + ..pending + }, + ); + if state + .install_direct_choice_frame( + armed, + WaitingFor::CipherEncodeChoice { + player, + card_id, + creatures, + }, + ) + .is_err() + { + // Same reasoning as the parked branch below: a refusal means the + // stack was already invalid, and the card still has to leave + // resolution by a legal route, so the offer completes as a decline + // (CR 608.2n) rather than being dropped. + handle_encode_choice(state, card_id, None, events); + } + return; + } + // Where a prompt-less frame may sit is a property of the stack's shape, not + // a guess this caller gets to make: an empty stack (a discard prompt owns no + // frame), the ordinary position below the active child, or outside a paused + // post-replacement/draw pair whose adjacency `validate` protects. The stack + // answers that itself, so no legal shape can refuse the offer. + let card_id = pending.card_id; + if state + .park_cipher_encode_beneath_live_prompt(pending) + .is_err() + { + // The stack rejected a frame that owns no prompt, which means it was + // already invalid before this offer existed. The card must still leave + // resolution by one of its two legal routes, so complete the offer the + // way a declined one completes (CR 608.2n: the card goes to its owner's + // graveyard) instead of dropping it and stranding the card off the + // stack. The live prompt is untouched either way — a decline moves a + // card, it does not ask a question. + handle_encode_choice(state, card_id, None, events); + } +} + +/// CR 702.99a: Arm a parked encode offer once it reaches the stack top, i.e. +/// after the spell's own effects have finished. Called from the exhaustive +/// frame-resume dispatch, which is what guarantees a parked offer is never +/// forgotten. +pub(crate) fn arm_parked_encode_offer(state: &mut GameState, events: &mut Vec) { + let Some(pending) = state.resolution_stack.active_cipher_encode() else { + return; + }; + // CR 702.99a: re-read legal hosts — the spell's own effects ran since the + // offer was parked and may have changed the board. + let creatures = legal_encode_creatures(state, pending.controller); + let (player, card_id) = (pending.controller, pending.card_id); + if creatures.is_empty() { + // No legal host left: consume the frame and route the card the way a + // declined offer does (CR 608.2n). + let _ = state.take_active_cipher_encode_frame(); + handle_encode_choice(state, card_id, None, events); + return; + } + if let Some(frame) = state.resolution_stack.active_cipher_encode_mut() { + frame.stage = crate::types::resolution::CipherEncodeStage::Armed; + frame.creatures = creatures.clone(); + } + state.waiting_for = WaitingFor::CipherEncodeChoice { + player, + card_id, + creatures, + }; +} + /// CR 702.99a–b: Resolve the encode choice. `creature = Some(id)` encodes the /// 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 diff --git a/crates/engine/src/game/cipher_tests.rs b/crates/engine/src/game/cipher_tests.rs index 9e70c2e35a..a230d0e21a 100644 --- a/crates/engine/src/game/cipher_tests.rs +++ b/crates/engine/src/game/cipher_tests.rs @@ -13,6 +13,7 @@ use crate::types::game_state::{GameState, WaitingFor}; use crate::types::identifiers::{CardId, ObjectId}; use crate::types::keywords::Keyword; use crate::types::player::PlayerId; +use crate::types::resolution::{CipherEncodeStage, PendingCipherEncode, ResolutionFrame}; use crate::types::zones::Zone; fn creature(state: &mut GameState, card: u64, owner: PlayerId, name: &str, zone: Zone) -> ObjectId { @@ -189,7 +190,12 @@ fn begin_encode_choice_pauses_then_accept_encodes() { let host = creature(&mut state, 1, PlayerId(0), "Host", Zone::Battlefield); let spell = cipher_spell(&mut state, 2, PlayerId(0)); - assert!(begin_encode_choice(&mut state, spell, PlayerId(0))); + assert!(begin_encode_choice( + &mut state, + spell, + PlayerId(0), + &mut Vec::new(), + )); match &state.waiting_for { WaitingFor::CipherEncodeChoice { player, @@ -214,7 +220,12 @@ fn begin_encode_choice_pauses_then_accept_encodes() { fn begin_encode_choice_skipped_without_host() { let mut state = GameState::new_two_player(1); let spell = cipher_spell(&mut state, 1, PlayerId(0)); - assert!(!begin_encode_choice(&mut state, spell, PlayerId(0))); + assert!(!begin_encode_choice( + &mut state, + spell, + PlayerId(0), + &mut Vec::new(), + )); } /// CR 608.2n: declining the encode puts the card into its owner's graveyard. @@ -223,13 +234,62 @@ fn handle_encode_choice_decline_routes_to_graveyard() { let mut state = GameState::new_two_player(1); let _host = creature(&mut state, 1, PlayerId(0), "Host", Zone::Battlefield); let spell = cipher_spell(&mut state, 2, PlayerId(0)); - assert!(begin_encode_choice(&mut state, spell, PlayerId(0))); + assert!(begin_encode_choice( + &mut state, + spell, + PlayerId(0), + &mut Vec::new(), + )); handle_encode_choice(&mut state, spell, None, &mut Vec::new()); assert_eq!(state.objects[&spell].zone, Zone::Graveyard); assert!(state.exile_links.is_empty()); } +/// CR 702.99a + CR 608.2n: A host that left while the spell's own prompt was +/// resolving leaves a parked offer with no legal target. The production resume +/// dispatcher must decline the card through its caller's event buffer, so the +/// zone move remains visible to the normal trigger pipeline. +#[test] +fn host_loss_before_arming_emits_the_decline_zone_change() { + let mut state = GameState::new_two_player(1); + let host = creature(&mut state, 1, PlayerId(0), "Host", Zone::Battlefield); + let spell = cipher_spell(&mut state, 2, PlayerId(0)); + state + .resolution_stack + .push_inner(ResolutionFrame::CipherEncode(PendingCipherEncode { + stage: CipherEncodeStage::Parked, + card_id: spell, + controller: PlayerId(0), + creatures: vec![host], + })); + + // This is the interleaving the parked frame represents: the host was legal + // when the offer parked, then the spell's remaining work removed it before + // `resume_resolution_frames` could arm the offer. + super::zones::move_to_zone(&mut state, host, Zone::Graveyard, &mut Vec::new()); + + let mut events = Vec::new(); + super::effects::resume_resolution_frames(&mut state, &mut events); + + assert!(state.resolution_stack.is_empty()); + assert_eq!(state.objects[&spell].zone, Zone::Graveyard); + assert!( + events.iter().any(|event| { + matches!( + event, + GameEvent::ZoneChanged { + object_id, + from: Some(Zone::Stack), + to: Zone::Graveyard, + .. + } if *object_id == spell + ) + }), + "the production resume path must publish the declined cipher card's zone move" + ); +} + // ── Combat-damage recast ────────────────────────────────────────────────── /// CR 702.99c: an encoded creature dealing combat damage to a player produces diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index 9dd6dd5ac7..97d7305dd3 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -988,6 +988,12 @@ pub(crate) fn resume_resolution_frames(state: &mut GameState, events: &mut Vec {} + // CR 702.99a: a Cipher encode offer parked under the spell's own prompt + // arms here — i.e. only once that prompt's owner is consumed, which is + // what puts the encode after the spell's other effects (issue #7470). + ResolutionFrame::CipherEncode(_) => { + crate::game::cipher::arm_parked_encode_offer(state, events); + } ResolutionFrame::RepeatFor(_) => drain_active_repeat_for(state, events), ResolutionFrame::RepeatUntil(_) => drain_active_repeat_until(state), ResolutionFrame::RepeatedOptionalPayment(_) => { diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index a54240e3ed..ea316d6376 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -19301,9 +19301,12 @@ mod stage2_injector_tests { // all MATCH). The round's other edits are the two new `#[cfg(test)]` // tests, which are below all three and mint no prompt, so the `in_test` // total is unchanged. - "game/effects/mod.rs:7261".to_string(), - "game/effects/mod.rs:7338".to_string(), - "game/effects/mod.rs:10618".to_string(), + // #7496's parked Cipher frame extends the exhaustive resume dispatch above + // all three producers. It adds six lines without assigning an optional-effect + // prompt, so the measured production producers move uniformly by `+6`. + "game/effects/mod.rs:7267".to_string(), + "game/effects/mod.rs:7344".to_string(), + "game/effects/mod.rs:10624".to_string(), // UNMOVED across the rebase, and that is itself evidence the SET did not // move: a census that had gained or lost a producer would not leave this // entry both byte-identical AND at the same coordinate. diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index 79d8e8cdf1..af4909b401 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -6675,6 +6675,21 @@ pub(super) fn handle_resolution_choice( // of clobbering it with `Priority`; otherwise resolution is complete, // so return to priority and let the resulting zone change's triggers / // SBAs process. + // CR 702.99a: the offer's own frame owns this prompt (issue #7470), + // so consume it BEFORE the card moves — the encode's zone change can + // park frames of its own, and a stale owner underneath them would + // fail `validate` at the next prompt. This holds for BOTH answers: + // a decline (`creature: None`) ends the offer just as an acceptance + // does, so it must consume the owner just as an acceptance does. + // + // The error is surfaced rather than swallowed: it means some other + // frame is sitting on top of this prompt's owner, which is the exact + // corruption this frame was introduced to make impossible. `Ok(None)` + // is not that — it is an empty stack, i.e. no owner to leave stale, + // which is what a game saved before this frame existed restores as. + state + .take_active_cipher_encode_frame() + .map_err(|error| EngineError::InvalidAction(error.to_string()))?; match crate::game::cipher::handle_encode_choice(state, card_id, creature, events) { crate::game::zone_pipeline::ZoneMoveResult::Done => { ResolutionChoiceOutcome::WaitingFor(WaitingFor::Priority { diff --git a/crates/engine/src/game/stack.rs b/crates/engine/src/game/stack.rs index 72fadc9137..0fba5db95a 100644 --- a/crates/engine/src/game/stack.rs +++ b/crates/engine/src/game/stack.rs @@ -1525,7 +1525,7 @@ pub fn resolve_top(state: &mut GameState, events: &mut Vec) { // exiles+encodes on accept, or routes the card to its graveyard on decline. // Skipped (resolution proceeds to graveyard normally) when there is no legal // host. `is_spell` gates out triggered/activated stack entries. - if is_spell && super::cipher::begin_encode_choice(state, entry.id, entry.controller) { + if is_spell && super::cipher::begin_encode_choice(state, entry.id, entry.controller, events) { events.push(GameEvent::StackResolved { object_id: entry.id, }); diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index edcce33cbb..af563e46e3 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -19476,6 +19476,44 @@ impl GameState { self.resolution_stack.push_mutate_merge(pending); } + /// CR 702.99a: Park a Cipher encode offer as the active prompt owner. + pub fn push_cipher_encode_frame(&mut self, pending: super::resolution::PendingCipherEncode) { + self.resolution_stack.push_cipher_encode(pending); + } + + /// CR 702.99a: Park a Cipher encode offer beneath the frame that owns the + /// spell's own prompt, so the encode arms only after that owner is + /// consumed. + /// + /// The position is the stack's decision, not this caller's: a parked offer + /// owns no prompt while `Parked`, and where such a frame may sit is a + /// property of the stack's current shape — see + /// [`ParkedFramePlacement`](super::resolution::ParkedFramePlacement). This + /// deliberately does NOT go through `InsertParentOfActive`: inserting below + /// the top is a structural guess that lands inside a paused + /// post-replacement/draw pair, and by the time the resulting `Err` came + /// back the caller had already retained its card off the normal resolution + /// route. + pub fn park_cipher_encode_beneath_live_prompt( + &mut self, + pending: super::resolution::PendingCipherEncode, + ) -> Result<(), ResolutionStackError> { + self.resolve_and_apply_frame_transition(ResolvedFrameTransition::ParkBeneathLivePrompt { + frame: super::resolution::ResolutionFrame::CipherEncode(pending), + }) + .map(|_| ()) + .map_err(|error| match error { + ResolvedFrameTransitionReplayInvariantError::Stack(error) => error, + }) + } + + /// CR 702.99a: Consume the active Cipher encode offer once answered. + pub fn take_active_cipher_encode_frame( + &mut self, + ) -> Result, ResolutionStackError> { + self.resolution_stack.take_active_cipher_encode() + } + /// Re-parks the active mutate-merge owner without exposing an empty-stack /// interval. pub fn replace_active_mutate_merge_frame( @@ -20322,6 +20360,9 @@ impl GameState { ResolvedFrameTransition::InsertParentOfActive { frame } => { resolution_stack.insert_parent_of_active(frame.clone())?; } + ResolvedFrameTransition::ParkBeneathLivePrompt { frame } => { + let _ = resolution_stack.park_beneath_live_prompt(frame.clone()); + } ResolvedFrameTransition::PopExpected { kind } => { let _ = resolution_stack.pop_expected(*kind)?; } diff --git a/crates/engine/src/types/resolution.rs b/crates/engine/src/types/resolution.rs index 583e4fdbaf..aefd7eb407 100644 --- a/crates/engine/src/types/resolution.rs +++ b/crates/engine/src/types/resolution.rs @@ -155,6 +155,50 @@ pub struct PendingMutateMerge { pub controller: PlayerId, } +/// CR 702.99a: Context stored when a Cipher spell has finished its own effects +/// and owes its controller the "you may exile this card encoded on a creature +/// you control" offer. +/// +/// The frame exists so the offer OWNS its prompt like every other direct +/// choice. Before it existed, `begin_encode_choice` set `WaitingFor` with no +/// frame behind it; when the spell's own resolution was still paused on a +/// player answer (Hidden Strings: "You may tap or untap ..."), that overwrote +/// the live prompt, stranded its frame, and left the stack permanently invalid +/// — the next prompt of any kind then failed `validate` (issue #7470). +/// +/// Ordering is the other half: the encode is the spell's LAST instruction, so +/// when a direct-choice owner is already active this frame is inserted as its +/// PARENT and arms only once that owner is consumed. +/// Whether a parked Cipher offer is already asking its question. +/// +/// CR 702.99a + the single-prompt-owner invariant: only ONE frame may own the +/// live prompt, so an offer parked beneath the spell's own still-open choice +/// must not claim ownership yet. It becomes [`Self::Armed`] when +/// `resume_resolution_frames` reaches it, i.e. once the frames above it are +/// gone. Mirrors `RepeatedOptionalPaymentFrame`, whose gate is likewise a +/// direct choice only while it actually holds a pending offer. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum CipherEncodeStage { + /// Waiting for the spell's own effects to finish. Owns no prompt. + Parked, + /// Asking its controller which creature hosts the card. + Armed, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct PendingCipherEncode { + /// Whether the offer currently owns the prompt. + pub stage: CipherEncodeStage, + /// The resolved Cipher card, held off the stack until the offer settles. + pub card_id: ObjectId, + /// The spell's controller — the player who chooses the host (CR 702.99a). + pub controller: PlayerId, + /// Legal hosts captured when the offer was parked. Re-validated against the + /// live board by `handle_encode_choice`, which already re-checks the chosen + /// creature, so a host that left the battlefield meanwhile simply declines. + pub creatures: Vec, +} + /// The ChangeZone owner plus the only sidecar that is not already embedded in /// `PendingChangeZoneIteration`. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -219,6 +263,7 @@ pub enum ResolutionFrame { LifeTotalAssignment(PendingLifeTotalAssignment), SpellResolution(PendingSpellResolution), MutateMerge(PendingMutateMerge), + CipherEncode(PendingCipherEncode), PostReplacement(PostReplacementDrainStack), } @@ -251,6 +296,7 @@ pub enum FrameKind { LifeTotalAssignment, SpellResolution, MutateMerge, + CipherEncode, PostReplacement, } @@ -282,6 +328,7 @@ impl ResolutionFrame { Self::LifeTotalAssignment(_) => FrameKind::LifeTotalAssignment, Self::SpellResolution(_) => FrameKind::SpellResolution, Self::MutateMerge(_) => FrameKind::MutateMerge, + Self::CipherEncode(_) => FrameKind::CipherEncode, Self::PostReplacement(_) => FrameKind::PostReplacement, } } @@ -318,6 +365,7 @@ impl ResolutionFrame { | Self::ConniveReentry(_) | Self::LifeTotalAssignment(_) | Self::SpellResolution(_) + | Self::CipherEncode(_) | Self::PostReplacement(_) => true, } } @@ -335,7 +383,15 @@ impl ResolutionFrame { Self::CoinFlip(_) => FrameGate::DirectChoice(DirectChoiceGate::CoinFlipKeep), Self::Proliferate(_) => FrameGate::DirectChoice(DirectChoiceGate::Proliferate), Self::MutateMerge(_) => FrameGate::DirectChoice(DirectChoiceGate::MutateMerge), - Self::AbilityContinuation(_) + Self::CipherEncode(PendingCipherEncode { + stage: CipherEncodeStage::Armed, + .. + }) => FrameGate::DirectChoice(DirectChoiceGate::CipherEncode), + Self::CipherEncode(PendingCipherEncode { + stage: CipherEncodeStage::Parked, + .. + }) + | Self::AbilityContinuation(_) | Self::RepeatFor(_) | Self::RepeatUntil(_) | Self::RepeatedOptionalPayment(RepeatedOptionalPaymentFrame { @@ -378,6 +434,7 @@ pub enum DirectChoiceGate { CoinFlipKeep, Proliferate, MutateMerge, + CipherEncode, } impl DirectChoiceGate { @@ -391,6 +448,7 @@ impl DirectChoiceGate { | (Self::CoinFlipKeep, WaitingFor::CoinFlipKeepChoice { .. }) | (Self::Proliferate, WaitingFor::ProliferateChoice { .. }) | (Self::MutateMerge, WaitingFor::MutateMergeChoice { .. }) + | (Self::CipherEncode, WaitingFor::CipherEncodeChoice { .. }) ) } } @@ -437,6 +495,34 @@ pub enum ResolutionStackError { InvalidPayload { frame: FrameKind, message: String }, } +/// Where a frame that owns no prompt goes while another frame owns the live one. +/// +/// Parking is not "insert below the top". Which position keeps the stack valid +/// depends on what is currently on it, and one shape answers differently: +/// `validate` requires a paused post-replacement/draw pair to stay immediately +/// adjacent (CR 614.11a + CR 121.6b — every action a replacement requires is +/// completed before the draw sequence resumes), and admits exactly one frame +/// above such a pair, the direct-choice owner holding the live prompt. Inserting +/// below the top lands INSIDE the pair in both of those shapes. +/// +/// Naming the position makes the placement a decision the stack takes from its +/// own shape. A caller that instead guesses "below the top" and inspects an +/// `Err` afterwards has no way to recover: by then it has already retained its +/// card off the normal resolution route (issue #7496 review). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ParkedFramePlacement { + /// Nothing is on the stack, so the parked frame is the only frame. This is + /// reachable while a question is open: a live prompt need not have a frame + /// behind it — a discard choice does not. + OnlyFrame, + /// Immediately below the active child — the ordinary case. + BelowActiveChild, + /// Immediately below a complete paused post-replacement/draw pair, i.e. + /// outside it, whether that pair is the active operation itself or sits + /// under the direct-choice owner of the choice it paused for. + OutsidePausedDrawPair, +} + /// An ordered, LIFO stack of suspended resolution work. /// /// Its backing storage is intentionally private, and the privacy is enforced by @@ -1950,6 +2036,50 @@ impl ResolutionStack { self.push_inner(ResolutionFrame::MutateMerge(frame)); } + /// CR 702.99a: Consumes exactly the active Cipher encode offer once its + /// controller has named a host (or declined). + pub fn take_active_cipher_encode( + &mut self, + ) -> Result, ResolutionStackError> { + match self.last() { + None => Ok(None), + Some(ResolutionFrame::CipherEncode(_)) => { + let ResolutionFrame::CipherEncode(frame) = + self.pop_expected(FrameKind::CipherEncode)? + else { + unreachable!("checked cipher-encode frame kind must match") + }; + Ok(Some(frame)) + } + Some(frame) => Err(ResolutionStackError::UnexpectedTop { + expected: FrameKind::CipherEncode, + actual: frame.kind(), + }), + } + } + + /// Reads the active Cipher encode offer without consuming it. + pub fn active_cipher_encode(&self) -> Option<&PendingCipherEncode> { + match self.last() { + Some(ResolutionFrame::CipherEncode(frame)) => Some(frame), + Some(_) | None => None, + } + } + + /// Mutable access to the active Cipher encode offer, for the parked → armed + /// transition. Mirrors `active_mutate_merge_mut`. + pub fn active_cipher_encode_mut(&mut self) -> Option<&mut PendingCipherEncode> { + match self.frames.last_mut() { + Some(ResolutionFrame::CipherEncode(frame)) => Some(frame), + Some(_) | None => None, + } + } + + /// Parks one Cipher encode offer. + pub fn push_cipher_encode(&mut self, frame: PendingCipherEncode) { + self.push_inner(ResolutionFrame::CipherEncode(frame)); + } + /// Re-parks the active mutate-merge owner without exposing an empty-stack /// interval. pub fn replace_active_mutate_merge( @@ -2824,6 +2954,77 @@ impl ResolutionStack { Ok(()) } + /// The post-replacement parent of a complete paused draw pair `active` + /// either belongs to or rests on, if there is one. + /// + /// Adjacent access only: the pair is reached by stepping down from the + /// located `active` slot, never by searching for a frame kind. + fn paused_draw_pair_parent(&self, active: FrameSlot) -> Option { + // The pair IS the active operation: the draw child is on top and its + // post-replacement parent immediately beneath it. + if self.has_active_post_replacement_draw_pair() { + return self.frames.below(active); + } + // Otherwise the pair may be paused for a player's choice, in which case + // the frame owning that choice sits above it — the one frame `validate` + // admits there. Anything else on top means there is no pair to protect. + if !matches!( + self.frames.get(active).map(|frame| frame.gate()), + Some(FrameGate::DirectChoice(_)) + ) { + return None; + } + let child = self.frames.below(active)?; + let parent = self.frames.below(child)?; + match (self.frames.get(parent), self.frames.get(child)) { + ( + Some(ResolutionFrame::PostReplacement(drains)), + Some(ResolutionFrame::MultiDraw(_)), + ) if matches!( + drains.resident().map(|drain| &drain.status), + Some(DrainStatus::Paused | DrainStatus::Dispatching) + ) => + { + Some(parent) + } + _ => None, + } + } + + /// The placement and the slot to insert below, if any. + /// + /// There is deliberately no public "where would this go?" companion: no + /// caller needs to know the position without taking it, and a reader that + /// could ask separately would invite deciding on the answer elsewhere — + /// which is the split this whole authority exists to close. + fn park_target(&self) -> (ParkedFramePlacement, Option) { + let Some(active) = self.frames.top() else { + return (ParkedFramePlacement::OnlyFrame, None); + }; + match self.paused_draw_pair_parent(active) { + Some(parent) => (ParkedFramePlacement::OutsidePausedDrawPair, Some(parent)), + None => (ParkedFramePlacement::BelowActiveChild, Some(active)), + } + } + + /// Park a frame that owns no prompt beneath the frame that owns the live + /// one, and report where it went. + /// + /// Infallible by construction, which is the point: the stack chooses the + /// position from its own shape rather than accepting one from a caller, so + /// there is no structural guess left for the caller to recover from. It is + /// still the caller's job to bring a frame whose gate is not + /// [`FrameGate::DirectChoice`] — parking a second prompt owner under a live + /// prompt is rejected by `validate`, and rightly so. + pub fn park_beneath_live_prompt(&mut self, frame: ResolutionFrame) -> ParkedFramePlacement { + let (placement, slot) = self.park_target(); + match slot { + None => self.push_inner(frame), + Some(slot) => self.frames.insert_below(slot, frame), + } + placement + } + /// Install an outer frame immediately below the child stack a producer /// created after recording its pre-resolution boundary. /// @@ -4210,6 +4411,9 @@ fn project_frames_into_legacy_state( ResolutionFrame::MutateMerge(pending) => { projected.push_mutate_merge_frame(pending.clone()) } + ResolutionFrame::CipherEncode(pending) => { + projected.push_cipher_encode_frame(pending.clone()) + } ResolutionFrame::MultiDraw(frame) => { projected.resolution_stack.push_multi_draw(frame.clone()) } @@ -5265,6 +5469,153 @@ mod tests { ); } + fn parked_cipher_encode_frame() -> ResolutionFrame { + ResolutionFrame::CipherEncode(PendingCipherEncode { + stage: CipherEncodeStage::Parked, + card_id: ObjectId(41), + controller: PlayerId(0), + creatures: vec![ObjectId(42)], + }) + } + + fn opponent_may_owner_frame() -> ResolutionFrame { + ResolutionFrame::OptionalEffect(OptionalEffectFrame { + ability: Box::new(resolved_draw(7)), + trigger_event: None, + trigger_events: Vec::new(), + trigger_match_count: None, + }) + } + + fn opponent_may_prompt() -> WaitingFor { + WaitingFor::OpponentMayChoice { + player: PlayerId(1), + source_id: ObjectId(7), + description: None, + remaining: Vec::new(), + } + } + + /// Parking answers from the stack's shape, and every shape has an answer. + /// + /// The three placements are exhaustive over what can be beneath a live + /// prompt: no frame at all (a discard prompt owns none), an ordinary active + /// child, or a paused post-replacement/draw pair whose adjacency + /// `validate` protects (CR 614.11a + CR 121.6b). + #[test] + fn parking_beneath_a_live_prompt_places_a_frame_by_stack_shape() { + let mut empty = ResolutionStack::default(); + assert_eq!( + empty.park_beneath_live_prompt(parked_cipher_encode_frame()), + ParkedFramePlacement::OnlyFrame + ); + assert_eq!( + empty.iter().map(ResolutionFrame::kind).collect::>(), + vec![FrameKind::CipherEncode] + ); + + let mut ordinary = ResolutionStack::default(); + ordinary.push_inner(opponent_may_owner_frame()); + assert_eq!( + ordinary.park_beneath_live_prompt(parked_cipher_encode_frame()), + ParkedFramePlacement::BelowActiveChild + ); + assert_eq!( + ordinary + .iter() + .map(ResolutionFrame::kind) + .collect::>(), + vec![FrameKind::CipherEncode, FrameKind::OptionalEffect], + "the ordinary case still parks immediately below the active child" + ); + ordinary + .validate(&opponent_may_prompt()) + .expect("a parked frame owns no prompt, so the live one keeps its owner"); + } + + /// The shape the #7496 review named: the pair must not be split. + /// + /// Both admitted forms are covered — the pair as the active operation, and + /// the pair beneath the single direct-choice owner `validate` allows above + /// it. The second is the one a "below the top" insert gets wrong, and the + /// last assertion measures exactly that rather than asserting it. + #[test] + fn parking_stays_outside_a_paused_post_replacement_draw_pair() { + let mut pair_active = ResolutionStack::default(); + pair_active + .install_adjacent_post_replacement_draw( + paused_post_replacement_frame(), + active_multi_draw_frame(), + ) + .expect("the fixture is the shipped adjacent pair"); + assert_eq!( + pair_active.park_beneath_live_prompt(parked_cipher_encode_frame()), + ParkedFramePlacement::OutsidePausedDrawPair + ); + assert_eq!( + pair_active + .iter() + .map(ResolutionFrame::kind) + .collect::>(), + vec![ + FrameKind::CipherEncode, + FrameKind::PostReplacement, + FrameKind::MultiDraw + ], + "the parked frame goes beneath the pair, never between its halves" + ); + + let mut pair_under_owner = ResolutionStack::default(); + pair_under_owner + .install_adjacent_post_replacement_draw( + paused_post_replacement_frame(), + active_multi_draw_frame(), + ) + .expect("the fixture is the shipped adjacent pair"); + pair_under_owner.push_inner(opponent_may_owner_frame()); + assert_eq!( + pair_under_owner.park_beneath_live_prompt(parked_cipher_encode_frame()), + ParkedFramePlacement::OutsidePausedDrawPair + ); + assert_eq!( + pair_under_owner + .iter() + .map(ResolutionFrame::kind) + .collect::>(), + vec![ + FrameKind::CipherEncode, + FrameKind::PostReplacement, + FrameKind::MultiDraw, + FrameKind::OptionalEffect + ] + ); + pair_under_owner + .validate(&opponent_may_prompt()) + .expect("parking outside the pair leaves both the pair and the prompt intact"); + + // What the placement is FOR: the same frame inserted below the top + // instead lands between the pair's halves, and the stack rejects it. + let mut naive = ResolutionStack::default(); + naive + .install_adjacent_post_replacement_draw( + paused_post_replacement_frame(), + active_multi_draw_frame(), + ) + .expect("the fixture is the shipped adjacent pair"); + naive.push_inner(opponent_may_owner_frame()); + naive + .insert_parent_of_active(parked_cipher_encode_frame()) + .expect("the structural insert itself succeeds — it is validation that refuses"); + assert!( + matches!( + naive.validate(&opponent_may_prompt()), + Err(ResolutionStackError::InvalidAdjacentPair(_)) + ), + "inserting below the top splits the pair, which is why placement is the \ + stack's decision and not the caller's" + ); + } + #[test] fn adjacent_pair_operations_never_search_for_a_non_top_parent() { let mut stack = ResolutionStack::default(); diff --git a/crates/engine/src/types/resolved_commands.rs b/crates/engine/src/types/resolved_commands.rs index 20198c9397..32d2c81988 100644 --- a/crates/engine/src/types/resolved_commands.rs +++ b/crates/engine/src/types/resolved_commands.rs @@ -921,10 +921,30 @@ pub enum ResolvedZoneChangeReplayInvariantError { /// records stack positions, frame identities, or displaced frame payloads. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub enum ResolvedFrameTransition { - Push { frame: ResolutionFrame }, - InsertParentOfActive { frame: ResolutionFrame }, - PopExpected { kind: FrameKind }, - ReplaceActive { frame: ResolutionFrame }, + Push { + frame: ResolutionFrame, + }, + InsertParentOfActive { + frame: ResolutionFrame, + }, + /// Park a prompt-less frame beneath the frame owning the live prompt. + /// + /// The operand is still native and no position is recorded: the applier + /// asks the stack where a parked frame belongs, and the stack answers from + /// its own shape. That keeps replay exact — the same frames plus the same + /// operand yield the same placement — while leaving the caller no position + /// to guess at. See [`ParkedFramePlacement`]. + /// + /// [`ParkedFramePlacement`]: crate::types::resolution::ParkedFramePlacement + ParkBeneathLivePrompt { + frame: ResolutionFrame, + }, + PopExpected { + kind: FrameKind, + }, + ReplaceActive { + frame: ResolutionFrame, + }, } /// One exact resolution-frame transition under its causal rules-execution node. diff --git a/crates/engine/tests/integration/cr733_resolved_frame_transition.rs b/crates/engine/tests/integration/cr733_resolved_frame_transition.rs index 8cfd768cd5..6c05e14d76 100644 --- a/crates/engine/tests/integration/cr733_resolved_frame_transition.rs +++ b/crates/engine/tests/integration/cr733_resolved_frame_transition.rs @@ -11,8 +11,9 @@ use engine::types::game_state::{ use engine::types::identifiers::ObjectId; use engine::types::player::PlayerId; use engine::types::resolution::{ - FrameKind, MultiDrawFrame, OptionalEffectFrame, PendingCoinFlip, PendingCoinFlipKind, - ResolutionFrame, ResolutionStackError, ResolutionStateWire, + CipherEncodeStage, FrameKind, MultiDrawFrame, OptionalEffectFrame, PendingCipherEncode, + PendingCoinFlip, PendingCoinFlipKind, ResolutionFrame, ResolutionStackError, + ResolutionStateWire, }; use engine::types::resolved_commands::{ ResolvedCommandOrdinal, ResolvedFrameTransition, ResolvedFrameTransitionCommand, @@ -433,3 +434,122 @@ fn resolution_state_wire_v2_round_trip_preserves_frame_transition_journal_and_st assert_eq!(restored.resolved_rules_journal, expected_journal); assert_eq!(frames(&restored), expected_stack); } + +/// The parking transition records an operand and no position: replay reaches +/// the same stack by asking the stack again, not by reading back a slot. +/// +/// The fixture is the shape that makes the distinction observable — a paused +/// post-replacement/draw pair, where "below the top" and "outside the pair" are +/// different positions (CR 614.11a + CR 121.6b keep the pair adjacent). +#[test] +fn parking_beneath_a_live_prompt_journals_its_operand_and_replays_to_the_same_stack() { + let mut state = GameState::new_two_player(103); + state + .resolution_stack + .push_inner(paused_post_replacement_frame()); + state.resolution_stack.push_inner(active_multi_draw_frame()); + let prompt_owner = optional_effect_frame(&state); + state.resolution_stack.push_inner(prompt_owner.clone()); + state.waiting_for = WaitingFor::OpponentMayChoice { + player: PlayerId(1), + source_id: ObjectId(7), + description: None, + remaining: Vec::new(), + }; + + // The parked frame must own no prompt — parking a direct-choice owner + // beneath another frame buries it, which `validate` rejects on its own + // terms. A `Parked` Cipher offer is exactly such a frame. + let parked = ResolutionFrame::CipherEncode(PendingCipherEncode { + stage: CipherEncodeStage::Parked, + card_id: ObjectId(41), + controller: PlayerId(0), + creatures: vec![ObjectId(42)], + }); + state + .resolve_and_apply_frame_transition(ResolvedFrameTransition::ParkBeneathLivePrompt { + frame: parked.clone(), + }) + .expect("a prompt-less frame always has a place beneath the live prompt"); + + assert!( + matches!( + frames(&state).as_slice(), + [ + ResolutionFrame::CipherEncode(_), + ResolutionFrame::PostReplacement(_), + ResolutionFrame::MultiDraw(_), + ResolutionFrame::OptionalEffect(_) + ] + ), + "the parked frame sits outside the pair, not between its halves: {:?}", + frames(&state) + .iter() + .map(ResolutionFrame::kind) + .collect::>() + ); + + let recorded = state + .resolved_rules_journal + .entries() + .iter() + .filter_map(|entry| match &entry.command { + Some(ResolvedRulesCommand::FrameTransition(command)) => Some(command), + _ => None, + }) + .find(|command| { + matches!( + &command.transition, + ResolvedFrameTransition::ParkBeneathLivePrompt { .. } + ) + }) + .expect("the parking transition is journaled") + .clone(); + assert_command_round_trip(&recorded); + + // Replay: the same operand against the same prior stack reaches the same + // placement, which is what makes a position-free record sufficient. + let mut replayed = GameState::new_two_player(103); + replayed + .resolution_stack + .push_inner(paused_post_replacement_frame()); + replayed + .resolution_stack + .push_inner(active_multi_draw_frame()); + replayed.resolution_stack.push_inner(prompt_owner); + replayed.waiting_for = WaitingFor::OpponentMayChoice { + player: PlayerId(1), + source_id: ObjectId(7), + description: None, + remaining: Vec::new(), + }; + replayed + .apply_resolved_frame_transition(&command(ResolvedFrameTransition::ParkBeneathLivePrompt { + frame: parked, + })) + .expect("replaying the recorded transition applies"); + assert_eq!(frames(&replayed), frames(&state)); +} + +/// A post-replacement frame whose resident drain is PAUSED — the only status +/// under which `validate` protects the pair's adjacency (CR 614.11a). +fn paused_post_replacement_frame() -> ResolutionFrame { + let mut drains = PostReplacementDrainStack::default(); + assert!(drains.install(post_replacement_drain(), ResidentDrainPolicy::KeepResident)); + let (_, dispatch) = drains + .begin_dispatch() + .expect("a ready drain begins dispatching"); + assert!(drains.pause_dispatch(dispatch)); + ResolutionFrame::PostReplacement(drains) +} + +/// A multi-draw child with an ACTIVE draw sequence, which the paired-adjacency +/// check requires (CR 121.2: a multi-card draw is that many individual draws). +fn active_multi_draw_frame() -> ResolutionFrame { + let mut draw_sequences = DrawSequenceStack::default(); + draw_sequences.push(PlayerId(0), 1); + ResolutionFrame::MultiDraw(MultiDrawFrame { + draw_sequences, + connive_reentry: None, + }) +} diff --git a/crates/engine/tests/integration/issue_7470_hidden_strings_optional_frame_leak.rs b/crates/engine/tests/integration/issue_7470_hidden_strings_optional_frame_leak.rs new file mode 100644 index 0000000000..e132f85ac4 --- /dev/null +++ b/crates/engine/tests/integration/issue_7470_hidden_strings_optional_frame_leak.rs @@ -0,0 +1,492 @@ +//! Issue #7470 — Hidden Strings leaves its `OptionalEffect` frame on the +//! resolution stack, so the next prompt panics. +//! +//! Oracle (Hidden Strings): +//! You may tap or untap target permanent, then you may tap or untap another +//! target permanent. +//! +//! Reported as an engine panic in `prepend_to_pending_continuation`: +//! +//! ```text +//! paused child operation must retain its continuation as an immediate parent: +//! PromptMismatch { frame: OptionalEffect, waiting_for: "ScryChoice" } +//! ``` +//! +//! `ResolutionStack::validate` rejects a direct-choice owner at the stack top +//! whose gate does not match the live prompt. Once this spell's `OptionalEffect` +//! frame survives its own resolution, ANY later prompt trips that check — the +//! report reached it by activating Thrasios, Triton Hero (Scry 1), but the scry +//! is incidental. The stale frame is the defect. +//! +//! Evidence from the attached save (turn 2, `waiting_for: Priority`): the +//! resolution stack still held one `OptionalEffect` frame carrying this spell's +//! own ability, tagged `cast_from_zone: "Hand"`, `cast_phase: "PreCombatMain"` — +//! i.e. left over from the ORIGINAL cast, not from the Cipher recast that later +//! ran into it. That recast trigger was stuck mid-resolution and never asked its +//! "you may cast a copy" question, which is why the copy was never offered. +//! +//! This file drives only the original cast: at priority, the resolution stack +//! must be empty. + +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::types::actions::GameAction; +use engine::types::game_state::{ExileLinkKind, WaitingFor}; +use engine::types::keywords::Keyword; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +const HIDDEN_STRINGS_ORACLE: &str = "You may tap or untap target permanent, then \ + you may tap or untap another target permanent."; + +/// Answer whatever the resolution asks until the engine hands back priority. +/// +/// Every branch takes the FIRST offered option; which branch is taken does not +/// matter to this test, only that resolution runs to completion. +fn settle(runner: &mut GameRunner, host: engine::types::identifiers::ObjectId) { + for _ in 0..60 { + match &runner.state().waiting_for { + WaitingFor::Priority { .. } => return, + WaitingFor::ChooseOneOfBranch { .. } => { + if runner.act(GameAction::ChooseBranch { index: 0 }).is_err() { + return; + } + } + WaitingFor::OptionalEffectChoice { .. } => { + if runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .is_err() + { + return; + } + } + WaitingFor::CipherEncodeChoice { .. } => { + if runner + .act(GameAction::CipherEncode { + creature: Some(host), + }) + .is_err() + { + return; + } + } + _ => return, + } + } +} + +#[test] +fn hidden_strings_leaves_no_optional_effect_frame_behind() { + let mut scenario = GameScenario::new_n_player(2, 42); + scenario.at_phase(Phase::PreCombatMain); + for pid in [P0, P1] { + scenario.with_library_top(pid, &["Lib A", "Lib B", "Lib C", "Lib D"]); + } + + let first = scenario.add_creature(P0, "First Permanent", 2, 2).id(); + let second = scenario.add_creature(P1, "Second Permanent", 2, 2).id(); + // CR 702.99a: Cipher is load-bearing here. It pauses the spell's own + // resolution for the encode offer, which is the window the frame survives. + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Hidden Strings", true, HIDDEN_STRINGS_ORACLE) + .with_keyword(Keyword::Cipher) + .id(); + + let mut runner = scenario.build(); + runner + .cast(spell) + .target_objects(&[first, second]) + .resolve(); + settle(&mut runner, first); + + assert!( + matches!(runner.state().waiting_for, WaitingFor::Priority { .. }), + "resolution must finish and hand back priority, got {:?}", + runner.state().waiting_for + ); + assert!( + runner.state().resolution_stack.is_empty(), + "a finished resolution must leave no frame behind — found {:?}", + runner + .state() + .resolution_stack + .iter() + .map(|frame| frame.kind()) + .collect::>() + ); +} + +/// Ground truth for the diagnosis: which prompts does the engine actually ask? +/// +/// `SpellCast::resolve()` auto-answers optional prompts (default `Decline`), so +/// a sequence measured through it says nothing about what a PLAYER would see. +/// This drives the cast by hand instead and records every prompt, once with +/// Cipher and once without. The two lists are the measurement. +fn prompt_sequence(with_cipher: bool) -> Vec { + let mut scenario = GameScenario::new_n_player(2, 42); + scenario.at_phase(Phase::PreCombatMain); + for pid in [P0, P1] { + scenario.with_library_top(pid, &["Lib A", "Lib B", "Lib C", "Lib D"]); + } + let first = scenario.add_creature(P0, "First Permanent", 2, 2).id(); + let second = scenario.add_creature(P1, "Second Permanent", 2, 2).id(); + let mut builder = + scenario.add_spell_to_hand_from_oracle(P0, "Hidden Strings", true, HIDDEN_STRINGS_ORACLE); + if with_cipher { + builder.with_keyword(Keyword::Cipher); + } + let spell = builder.id(); + + let mut runner = scenario.build(); + runner.cast(spell).target_objects(&[first, second]).commit(); + + let mut seen = Vec::new(); + for _ in 0..40 { + let (label, action) = match &runner.state().waiting_for { + WaitingFor::Priority { .. } => (None, GameAction::PassPriority), + WaitingFor::OptionalEffectChoice { .. } => ( + Some("OptionalEffectChoice"), + GameAction::DecideOptionalEffect { accept: true }, + ), + WaitingFor::ChooseOneOfBranch { .. } => ( + Some("ChooseOneOfBranch"), + GameAction::ChooseBranch { index: 0 }, + ), + WaitingFor::CipherEncodeChoice { .. } => ( + Some("CipherEncodeChoice"), + GameAction::CipherEncode { + creature: Some(first), + }, + ), + other => { + // Anything the loop is not taught to answer ends the sequence; + // the variant name alone keeps the row stable across unrelated + // changes to the prompt payloads. + seen.push(format!("UNBEKANNT: {}", other.variant_name())); + break; + } + }; + if let Some(label) = label { + seen.push(label.to_string()); + } + if runner.act(action).is_err() { + break; + } + if runner.state().stack.is_empty() && seen.iter().any(|s| s == "CipherEncodeChoice") { + break; + } + } + seen +} + +/// Without Cipher the same spell asks all four of its questions, in order. +/// +/// This is the control row: it proves the pause machinery is sound, so the +/// failure above cannot be blamed on the optional effect itself. Pinning the +/// exact sequence also makes the Cipher row's silence measurable — with Cipher +/// the player is asked NONE of these. +#[test] +fn without_cipher_the_spell_asks_both_optional_questions() { + assert_eq!( + prompt_sequence(false), + vec![ + "OptionalEffectChoice", + "ChooseOneOfBranch", + "OptionalEffectChoice", + "ChooseOneOfBranch", + "UNBEKANNT: DeclareAttackers", + ], + "the un-ciphered spell must ask both \"you may\" questions and both tap/untap branches" + ); +} + +/// The behavioural fix: with Cipher the player is asked the SAME questions, in +/// the same order, and the encode offer comes last. +/// +/// CR 702.99a — "then you may exile this card encoded on a creature you +/// control" is the spell's final instruction, so it must follow the tap/untap +/// choices rather than replace them. Before the fix this row saw only +/// `CipherEncodeChoice`: the offer overwrote the live prompt and stranded its +/// frame, which is what made every later prompt panic. +#[test] +fn with_cipher_the_encode_offer_comes_after_the_spells_own_questions() { + assert_eq!( + prompt_sequence(true), + vec![ + "OptionalEffectChoice", + "ChooseOneOfBranch", + "OptionalEffectChoice", + "ChooseOneOfBranch", + "CipherEncodeChoice", + ], + "the encode offer must come last, after both \"you may\" questions" + ); +} + +/// Second card shape in the class: the pause need not be a "you may". +/// +/// Mental Vapors ("Target player discards a card.") pauses on a DiscardChoice, +/// which owns the prompt exactly like the OptionalEffect frame above. Measuring +/// a second, structurally different pause is what turns "Hidden Strings works +/// now" into a statement about the class — the fix is keyed to prompt +/// ownership, not to any card's text. +fn discard_shaped_prompt_sequence(with_cipher: bool) -> Vec { + let mut scenario = GameScenario::new_n_player(2, 42); + scenario.at_phase(Phase::PreCombatMain); + for pid in [P0, P1] { + scenario.with_library_top(pid, &["Lib A", "Lib B", "Lib C", "Lib D"]); + } + let host = scenario.add_creature(P0, "Host Creature", 2, 2).id(); + scenario.add_card_to_hand(P1, "Victim Card A"); + scenario.add_card_to_hand(P1, "Victim Card B"); + let mut builder = scenario.add_spell_to_hand_from_oracle( + P0, + "Mental Vapors", + true, + "Target player discards a card.", + ); + if with_cipher { + builder.with_keyword(Keyword::Cipher); + } + let spell = builder.id(); + + let mut runner = scenario.build(); + runner.cast(spell).target_player(P1).commit(); + + let mut seen = Vec::new(); + for _ in 0..40 { + let (label, action) = match &runner.state().waiting_for { + WaitingFor::Priority { .. } => (None, GameAction::PassPriority), + WaitingFor::DiscardChoice { cards, .. } => ( + Some("DiscardChoice"), + GameAction::SelectCards { + cards: vec![cards[0]], + }, + ), + WaitingFor::CipherEncodeChoice { .. } => ( + Some("CipherEncodeChoice"), + GameAction::CipherEncode { + creature: Some(host), + }, + ), + other => { + seen.push(format!("UNBEKANNT: {}", other.variant_name())); + break; + } + }; + if let Some(label) = label { + seen.push(label.to_string()); + } + if runner.act(action).is_err() { + break; + } + if seen.iter().any(|s| s == "CipherEncodeChoice") { + break; + } + } + seen +} + +#[test] +fn a_discard_pause_also_keeps_the_encode_offer_last() { + let without = discard_shaped_prompt_sequence(false); + assert!( + without.contains(&"DiscardChoice".to_string()), + "control: without Cipher the discard is asked — {without:?}" + ); + let with = discard_shaped_prompt_sequence(true); + assert_eq!( + with.iter().take(2).collect::>(), + vec!["DiscardChoice", "CipherEncodeChoice"], + "the discard must be asked before the encode offer — {with:?}" + ); +} + +/// Declining must consume the offer's frame exactly as accepting does. +/// +/// `CipherEncode { creature: None }` ends the offer without encoding anything +/// (CR 608.2n: the card goes to its owner's graveyard). It reaches the same +/// handler as an acceptance, so it must leave the same empty stack behind — an +/// unconsumed owner is precisely the #7470 corruption, and a decline that +/// skipped the consumption would rebuild it from the other side. The offer is +/// declined here AFTER the spell's own prompts, i.e. from the parked-then-armed +/// path rather than the immediately-armed one. +#[test] +fn declining_the_encode_offer_consumes_its_frame_too() { + let mut scenario = GameScenario::new_n_player(2, 42); + scenario.at_phase(Phase::PreCombatMain); + for pid in [P0, P1] { + scenario.with_library_top(pid, &["Lib A", "Lib B", "Lib C", "Lib D"]); + } + let first = scenario.add_creature(P0, "First Permanent", 2, 2).id(); + let second = scenario.add_creature(P1, "Second Permanent", 2, 2).id(); + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Hidden Strings", true, HIDDEN_STRINGS_ORACLE) + .with_keyword(Keyword::Cipher) + .id(); + + let mut runner = scenario.build(); + runner.cast(spell).target_objects(&[first, second]).commit(); + + let mut declined = false; + for _ in 0..40 { + match &runner.state().waiting_for { + // The spell still has to be let through before it resolves; once + // the offer has been declined, priority is the resting state this + // test is measuring. + WaitingFor::Priority { .. } => { + if declined || runner.act(GameAction::PassPriority).is_err() { + break; + } + } + WaitingFor::OptionalEffectChoice { .. } => { + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("the spell's own optional must be answerable"); + } + WaitingFor::ChooseOneOfBranch { .. } => { + runner + .act(GameAction::ChooseBranch { index: 0 }) + .expect("the tap/untap branch must be answerable"); + } + WaitingFor::CipherEncodeChoice { .. } => { + runner + .act(GameAction::CipherEncode { creature: None }) + .expect("declining the encode offer must be a legal answer"); + declined = true; + } + other => panic!("unexpected prompt {:?}", other.variant_name()), + } + } + + assert!(declined, "the encode offer must have been reached at all"); + assert!( + matches!(runner.state().waiting_for, WaitingFor::Priority { .. }), + "a declined offer must hand back priority, got {:?}", + runner.state().waiting_for + ); + assert!( + runner.state().resolution_stack.is_empty(), + "a declined offer must consume its own frame — found {:?}", + runner + .state() + .resolution_stack + .iter() + .map(|frame| frame.kind()) + .collect::>() + ); + // CR 608.2n: the declined card is in its owner's graveyard, not stranded + // off the stack — the outcome the dropped-offer path could not produce. + assert!( + runner.state().players[0] + .graveyard + .iter() + .filter_map(|id| runner.state().objects.get(id)) + .any(|object| object.name == "Hidden Strings"), + "the declined cipher card belongs in its owner's graveyard" + ); +} + +/// The stack shape the #7496 review named: a paused post-replacement/draw pair. +/// +/// Zur's Weirding replaces Last Thoughts' own draw, so the spell's resolution +/// rests on that replacement's "may pay 2 life" offer with the +/// `PostReplacement` → `MultiDraw` pair parked beneath it. `validate` requires +/// that pair to stay immediately adjacent (CR 614.11a + CR 121.6b), so parking +/// the encode offer "below the top" would land INSIDE it and be rejected. +/// +/// This is the production pipeline, not a hand-built stack: a real cipher spell +/// (Last Thoughts, "Draw a card.") whose real draw is really replaced. What it +/// pins is that the offer survives such a shape at all — the earlier revision +/// dropped it there, leaving the card with neither an encode prompt nor its +/// ordinary graveyard route. +#[test] +fn the_encode_offer_survives_a_paused_post_replacement_draw_pair() { + const ZURS_WEIRDING_ORACLE: &str = "If a player would draw a card, they reveal it instead. \ + Then any other player may pay 2 life. If a player does, put that card into its owner's \ + graveyard. Otherwise, that player draws a card."; + + let mut scenario = GameScenario::new_n_player(2, 42); + scenario.at_phase(Phase::PreCombatMain); + for pid in [P0, P1] { + scenario.with_library_top(pid, &["Lib A", "Lib B", "Lib C", "Lib D"]); + } + // P1 controls the replacement, so P1 is the "any other player" who is asked + // to pay while P0's own draw is what pauses. + scenario + .add_creature_from_oracle(P1, "Zur's Weirding", 0, 1, ZURS_WEIRDING_ORACLE) + .as_enchantment(); + let host = scenario.add_creature(P0, "Host Creature", 2, 2).id(); + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Last Thoughts", false, "Draw a card.") + .with_keyword(Keyword::Cipher) + .id(); + + let mut runner = scenario.build(); + runner.cast(spell).commit(); + + let mut seen = Vec::new(); + for _ in 0..60 { + match runner.state().waiting_for.clone() { + // Let the spell through to resolution; after the offer is answered + // priority is the resting state. + WaitingFor::Priority { .. } => { + if seen.iter().any(|prompt| prompt == "CipherEncodeChoice") + || runner.act(GameAction::PassPriority).is_err() + { + break; + } + } + WaitingFor::OpponentMayChoice { .. } => { + seen.push("OpponentMayChoice".to_string()); + runner + .act(GameAction::DecideOptionalEffect { accept: false }) + .expect("the draw replacement offer must be answerable"); + } + WaitingFor::CipherEncodeChoice { .. } => { + seen.push("CipherEncodeChoice".to_string()); + runner + .act(GameAction::CipherEncode { + creature: Some(host), + }) + .expect("accepting the encode offer must be a legal answer"); + break; + } + other => panic!( + "unexpected prompt {:?}; prompts={seen:?}", + other.variant_name() + ), + } + } + + assert!( + seen.contains(&"OpponentMayChoice".to_string()), + "the draw replacement must actually pause this resolution — otherwise this \ + test does not stand on the stack shape it claims; prompts={seen:?}" + ); + assert!( + seen.contains(&"CipherEncodeChoice".to_string()), + "the encode offer must survive the paused draw pair and still be asked; \ + prompts={seen:?}" + ); + assert!( + runner.state().resolution_stack.is_empty(), + "the answered offer must leave no frame behind — found {:?}", + runner + .state() + .resolution_stack + .iter() + .map(|frame| frame.kind()) + .collect::>() + ); + assert_eq!( + runner.state().objects[&spell].zone, + Zone::Exile, + "accepting the offer must exile the cipher card" + ); + assert!( + runner.state().exile_links.iter().any(|link| { + link.exiled_id == spell && link.source_id == host && link.kind == ExileLinkKind::Cipher + }), + "accepting the offer must encode Last Thoughts on the selected host" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 96e8eccd00..ba3d64aa1b 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -744,6 +744,7 @@ mod issue_735_cost_paid_object_non_regression; mod issue_735_lily_bowen_power_double; mod issue_7384_proliferate_counter_replacement_frame; mod issue_7386_ozolith_combat_counter_move; +mod issue_7470_hidden_strings_optional_frame_leak; mod issue_787_once_upon_a_time; mod issue_788_unexpectedly_absent; mod issue_822_erode_path_to_exile_search_controller;