diff --git a/crates/engine/src/game/sba.rs b/crates/engine/src/game/sba.rs index 6cff515227..2a6f21deb0 100644 --- a/crates/engine/src/game/sba.rs +++ b/crates/engine/src/game/sba.rs @@ -2124,8 +2124,8 @@ fn check_token_cease_to_exist(state: &mut GameState, any_performed: &mut bool) { .objects .iter() .filter(|(_, obj)| { - zones::token_is_outside_battlefield_and_stack(obj) - || zones::copy_of_card_outside_battlefield_and_stack(obj) + zones::token_is_outside_battlefield_and_stack(state, obj) + || zones::copy_of_card_outside_battlefield_and_stack(state, obj) }) .map(|(id, obj)| (*id, obj.zone, obj.owner)) .collect(); @@ -2306,6 +2306,7 @@ mod tests { }; use crate::types::actions::GameAction; use crate::types::format::FormatConfig; + use crate::types::game_state::{CastingVariant, StackEntry, StackEntryKind}; use crate::types::identifiers::{CardId, ObjectId}; use crate::types::replacements::ReplacementEvent; @@ -4910,6 +4911,66 @@ mod tests { ); } + #[test] + fn bare_same_id_spell_entry_does_not_prevent_off_zone_noncard_cleanup() { + fn add_off_zone_noncard( + state: &mut GameState, + card_id: u64, + name: &str, + is_token: bool, + is_copy: bool, + ) -> ObjectId { + let id = create_object( + state, + CardId(card_id), + PlayerId(0), + name.to_string(), + Zone::Exile, + ); + let object = state.objects.get_mut(&id).unwrap(); + object.is_token = is_token; + object.is_copy = is_copy; + id + } + + fn push_spell_placeholder(state: &mut GameState, id: ObjectId, card_id: u64) { + state.stack.push_back(StackEntry { + id, + source_id: id, + controller: PlayerId(0), + kind: StackEntryKind::Spell { + card_id: CardId(card_id), + ability: None, + casting_variant: CastingVariant::Normal, + actual_mana_spent: 0, + }, + }); + } + + let mut state = setup(); + let token = add_off_zone_noncard(&mut state, 1, "Orphan Token", true, false); + let copy = add_off_zone_noncard(&mut state, 2, "Orphan Copy", false, true); + push_spell_placeholder(&mut state, token, 1); + push_spell_placeholder(&mut state, copy, 2); + + for id in [token, copy] { + assert!(state.objects.contains_key(&id)); + assert_eq!(state.objects[&id].zone, Zone::Exile); + } + + let mut events = Vec::new(); + check_state_based_actions(&mut state, &mut events); + + // CR 704.5d + CR 704.5e: Bare same-id spell entries do not establish a + // live casting lifecycle, so both synthetic off-zone objects cease. + for id in [token, copy] { + assert!( + !state.objects.contains_key(&id), + "off-zone noncard object {id:?} must cease without its own PendingCast" + ); + } + } + // --- CR 704.5e + CR 707.10a: Copy-of-a-card cease-to-exist tests --- /// A copy of a card (is_copy = true, is_token = false) resolving to the diff --git a/crates/engine/src/game/zone_pipeline.rs b/crates/engine/src/game/zone_pipeline.rs index 2a5611424c..aa11e3530a 100644 --- a/crates/engine/src/game/zone_pipeline.rs +++ b/crates/engine/src/game/zone_pipeline.rs @@ -738,8 +738,10 @@ pub(crate) fn move_object_with_terminal( .get(&req.object_id) .expect("object exists (zone read above)"); // CR 111.8: A token that has left the battlefield can't change zones; it - // remains in place and ceases to exist at the next SBA (CR 111.7). - if zones::token_is_outside_battlefield_and_stack(obj) { + // remains in place and ceases to exist at the next SBA (CR 111.7). An + // exact CR 601.2a pending spell plus its announcement placeholder makes + // the retained-origin representation stack-resident until this delivery. + if zones::token_is_outside_battlefield_and_stack(state, obj) { return ZoneMoveTerminalResult::Completed(ZoneMoveCompletion::Remained); } // CR 603.2g + CR 603.6a: A Battlefield -> Battlefield move does not put a @@ -3081,6 +3083,62 @@ fn execute_zone_move_with_applied_terminal( } } +#[cfg(test)] +mod announced_spell_residency_tests { + use super::*; + use crate::game::zones::create_object; + use crate::types::ability::{Effect, ResolvedAbility}; + use crate::types::game_state::{StackEntry, StackEntryKind}; + use crate::types::identifiers::CardId; + + #[test] + fn casting_to_stack_rejects_same_id_activated_ability_entry() { + let mut state = GameState::new_two_player(42); + let object_id = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Activated Source".to_string(), + Zone::Exile, + ); + state.objects.get_mut(&object_id).unwrap().is_token = true; + state.stack.push_back(StackEntry { + id: object_id, + source_id: object_id, + controller: PlayerId(0), + kind: StackEntryKind::ActivatedAbility { + source_id: object_id, + ability: Box::new(ResolvedAbility::new( + Effect::NoOp, + vec![], + object_id, + PlayerId(0), + )), + }, + }); + assert_eq!(state.objects[&object_id].zone, Zone::Exile); + assert!(state.stack.iter().any(|entry| { + entry.id == object_id && matches!(entry.kind, StackEntryKind::ActivatedAbility { .. }) + })); + + // CR 109.1 / CR 602.2a: A same-id activated ability is a distinct + // noncard stack object, so it cannot satisfy the spell-residency gate. + let mut events = Vec::new(); + let result = move_object_with_terminal( + &mut state, + ZoneMoveRequest::casting_to_stack(object_id, object_id), + &mut events, + ); + + assert!(matches!( + result, + ZoneMoveTerminalResult::Completed(ZoneMoveCompletion::Remained) + )); + assert_eq!(state.objects[&object_id].zone, Zone::Exile); + assert!(events.is_empty()); + } +} + #[cfg(test)] mod w3_library_placement_tests { use super::*; diff --git a/crates/engine/src/game/zones.rs b/crates/engine/src/game/zones.rs index 932a762af4..25de9f2a0d 100644 --- a/crates/engine/src/game/zones.rs +++ b/crates/engine/src/game/zones.rs @@ -1,7 +1,7 @@ use crate::types::card_type::CoreType; use crate::types::events::GameEvent; use crate::types::game_state::{ - GameState, ResolutionSourceRelatch, StackEntry, ZoneChangeCombatStatus, + GameState, ResolutionSourceRelatch, StackEntry, StackEntryKind, ZoneChangeCombatStatus, }; use crate::types::identifiers::{CardId, ObjectId, ObjectIncarnationRef}; use crate::types::player::PlayerId; @@ -17,11 +17,38 @@ use crate::types::zones::Zone; use super::game_object::GameObject; use super::printed_cards::{apply_back_face_to_object, snapshot_object_face}; -/// CR 111.7 / CR 111.8: A token outside the battlefield ceases to exist at -/// the next SBA, and can't change zones before then. Stack tokens are excluded -/// so spell copies can finish resolving before the next SBA check. -pub(super) fn token_is_outside_battlefield_and_stack(obj: &GameObject) -> bool { - obj.is_token && obj.zone != Zone::Battlefield && obj.zone != Zone::Stack +/// CR 109.1 + CR 601.2a + CR 405.1: A spell is an object on the stack from +/// announcement, even while this engine retains its origin-zone field until +/// finalization. The retained-origin representation is stack-resident only while +/// the exact spell's `PendingCast` lifecycle and announcement placeholder both +/// remain live; a bare same-id stack entry is insufficient. +fn object_has_stack_residency(state: &GameState, obj: &GameObject) -> bool { + if obj.zone == Zone::Stack { + return true; + } + + let is_pending_spell = |pending: &crate::types::game_state::PendingCast| { + pending.object_id == obj.id && pending.activation_ability_index.is_none() + }; + let has_pending_spell = state.pending_cast.as_deref().is_some_and(is_pending_spell) + || state + .waiting_for + .pending_cast_ref() + .is_some_and(is_pending_spell); + + has_pending_spell + && state + .stack + .iter() + .any(|entry| entry.id == obj.id && matches!(entry.kind, StackEntryKind::Spell { .. })) +} + +/// CR 704.5d / CR 111.7 / CR 111.8: A token outside the battlefield ceases to +/// exist at the next SBA and can't change zones before then. Effectively +/// stack-resident tokens are excluded so announced spell copies can finish +/// casting and resolving before the next applicable SBA check. +pub(super) fn token_is_outside_battlefield_and_stack(state: &GameState, obj: &GameObject) -> bool { + obj.is_token && obj.zone != Zone::Battlefield && !object_has_stack_residency(state, obj) } /// CR 704.5e + CR 707.10a: A copy of a card in any zone other than the stack or @@ -30,8 +57,11 @@ pub(super) fn token_is_outside_battlefield_and_stack(obj: &GameObject) -> bool { /// (CR 707.10f makes a permanent copy a token there) and may change zones freely /// while alive, so this predicate is used ONLY by the cease-to-exist SBA — never /// by the CR 111.8 "can't change zones" movement guards, which apply to tokens only. -pub(super) fn copy_of_card_outside_battlefield_and_stack(obj: &GameObject) -> bool { - obj.is_copy && obj.zone != Zone::Battlefield && obj.zone != Zone::Stack +pub(super) fn copy_of_card_outside_battlefield_and_stack( + state: &GameState, + obj: &GameObject, +) -> bool { + obj.is_copy && obj.zone != Zone::Battlefield && !object_has_stack_residency(state, obj) } /// CR 122.2 + CR 113.6b: Determine whether `object_id`'s counters survive a move @@ -941,7 +971,7 @@ pub fn move_to_zone( if state .objects .get(&object_id) - .is_some_and(token_is_outside_battlefield_and_stack) + .is_some_and(|obj| token_is_outside_battlefield_and_stack(state, obj)) { return; } @@ -1707,7 +1737,7 @@ pub fn move_to_library_at_index( if state .objects .get(&object_id) - .is_some_and(token_is_outside_battlefield_and_stack) + .is_some_and(|obj| token_is_outside_battlefield_and_stack(state, obj)) { return; } diff --git a/crates/engine/tests/integration/issue_1312_prepared_spell_cast_triggers.rs b/crates/engine/tests/integration/issue_1312_prepared_spell_cast_triggers.rs index 9ffa2ed329..366cbf47d7 100644 --- a/crates/engine/tests/integration/issue_1312_prepared_spell_cast_triggers.rs +++ b/crates/engine/tests/integration/issue_1312_prepared_spell_cast_triggers.rs @@ -3,12 +3,13 @@ //! //! https://github.com/phase-rs/phase/issues/1312 -use engine::game::scenario::{GameScenario, P0}; +use engine::database::CardDatabase; +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; use engine::game::scenario_db::GameScenarioDbExt; use engine::types::ability::TargetRef; use engine::types::actions::GameAction; use engine::types::counter::CounterType; -use engine::types::game_state::WaitingFor; +use engine::types::game_state::{StackEntryKind, WaitingFor}; use engine::types::identifiers::ObjectId; use engine::types::mana::{ManaType, ManaUnit}; use engine::types::phase::Phase; @@ -16,45 +17,40 @@ use engine::types::zones::Zone; use crate::support::shared_card_db as load_db; -fn drive_cast_to_stack(runner: &mut engine::game::scenario::GameRunner, spell_target: ObjectId) { - loop { - match &runner.state().waiting_for { - WaitingFor::TargetSelection { .. } => { - runner - .act(GameAction::ChooseTarget { - target: Some(TargetRef::Object(spell_target)), - }) - .expect("spell target selection should succeed"); - } - WaitingFor::TriggerTargetSelection { .. } => { - runner - .choose_first_legal_target() - .expect("trigger target selection should succeed"); - } - WaitingFor::ManaPayment { .. } => { - runner.act(GameAction::PassPriority).expect("pay mana"); - } - WaitingFor::Priority { .. } => break, - other => panic!("unexpected waiting state during cast: {other:?}"), - } - } +struct PreparedSwordsFixture { + runner: GameRunner, + emeritus: ObjectId, + exile_target: ObjectId, + scornmage: Option, + counterspell: Option, } -#[test] -fn issue_1312_prepared_swords_to_plowshares_triggers_lecturing_scornmage() { - let Some(db) = load_db() else { - return; - }; - +fn build_prepared_swords_fixture( + db: &CardDatabase, + with_scornmage: bool, + with_counterspell: bool, +) -> PreparedSwordsFixture { let mut scenario = GameScenario::new(); scenario.at_phase(Phase::PreCombatMain); - let scornmage = scenario.add_real_card(P0, "Lecturing Scornmage", Zone::Battlefield, db); + let scornmage = with_scornmage + .then(|| scenario.add_real_card(P0, "Lecturing Scornmage", Zone::Battlefield, db)); let emeritus = scenario.add_real_card(P0, "Emeritus of Truce", Zone::Battlefield, db); let exile_target = scenario.add_creature(P0, "Exile Target", 2, 2).id(); + let counterspell = + with_counterspell.then(|| scenario.add_real_card(P1, "Counterspell", Zone::Hand, db)); scenario.with_mana_pool( P0, vec![ManaUnit::new(ManaType::White, ObjectId(0), false, vec![])], ); + if with_counterspell { + scenario.with_mana_pool( + P1, + vec![ + ManaUnit::new(ManaType::Blue, ObjectId(0), false, vec![]), + ManaUnit::new(ManaType::Blue, ObjectId(0), false, vec![]), + ], + ); + } let mut runner = scenario.build(); runner.state_mut().debug_mode = true; @@ -64,10 +60,20 @@ fn issue_1312_prepared_swords_to_plowshares_triggers_lecturing_scornmage() { .state() .objects .get(&emeritus) - .and_then(|o| o.back_face.clone()) + .and_then(|object| object.back_face.clone()) .expect("Emeritus of Truce must hydrate Swords to Plowshares prepare face"); assert_eq!(back.name, "Swords to Plowshares"); + PreparedSwordsFixture { + runner, + emeritus, + exile_target, + scornmage, + counterspell, + } +} + +fn begin_prepared_cast(runner: &mut GameRunner, emeritus: ObjectId) -> ObjectId { runner .act(GameAction::Debug( engine::types::actions::DebugAction::SetPrepared { @@ -81,7 +87,90 @@ fn issue_1312_prepared_swords_to_plowshares_triggers_lecturing_scornmage() { .act(GameAction::CastPreparedCopy { source: emeritus }) .expect("CastPreparedCopy should start the prepared spell cast"); + let copy_id = match &runner.state().waiting_for { + WaitingFor::TargetSelection { pending_cast, .. } => pending_cast.object_id, + other => panic!("prepared Swords cast must pause for a target, got {other:?}"), + }; + let placeholder = runner + .state() + .stack + .iter() + .find(|entry| entry.id == copy_id) + .expect("CR 601.2a announcement must create the exact prepared-copy stack entry"); + assert!(matches!( + &placeholder.kind, + StackEntryKind::Spell { ability: None, .. } + )); + assert!(runner.state().objects.contains_key(©_id)); + + copy_id +} + +fn drive_cast_to_stack(runner: &mut engine::game::scenario::GameRunner, spell_target: ObjectId) { + loop { + match &runner.state().waiting_for { + WaitingFor::TargetSelection { .. } => { + runner + .act(GameAction::ChooseTarget { + target: Some(TargetRef::Object(spell_target)), + }) + .expect("spell target selection should succeed"); + } + WaitingFor::TriggerTargetSelection { .. } => { + runner + .choose_first_legal_target() + .expect("trigger target selection should succeed"); + } + WaitingFor::ManaPayment { .. } => { + runner.act(GameAction::PassPriority).expect("pay mana"); + } + WaitingFor::Priority { .. } => break, + other => panic!("unexpected waiting state during cast: {other:?}"), + } + } +} + +fn assert_prepared_copy_finalized_on_stack( + runner: &GameRunner, + copy_id: ObjectId, + spell_target: ObjectId, +) { + let entry = runner + .state() + .stack + .iter() + .find(|entry| entry.id == copy_id) + .expect("prepared spell must retain its exact stack entry after targeting"); + let ability = entry + .ability() + .expect("prepared spell stack entry must carry its finalized ability"); + assert!( + engine::game::ability_utils::flatten_targets_in_chain(ability) + .contains(&TargetRef::Object(spell_target)) + ); + assert_eq!(runner.state().objects[©_id].zone, Zone::Stack); +} + +#[test] +fn issue_1312_prepared_swords_to_plowshares_triggers_lecturing_scornmage() { + let Some(db) = load_db() else { + return; + }; + + let PreparedSwordsFixture { + mut runner, + emeritus, + exile_target, + scornmage, + counterspell: _, + } = build_prepared_swords_fixture(db, true, false); + let scornmage = scornmage.expect("Scornmage fixture requested"); + let copy_id = begin_prepared_cast(&mut runner, emeritus); drive_cast_to_stack(&mut runner, exile_target); + // CR 601.2a + CR 704.3: Choosing the target completes the cast through + // `apply`, which reaches the ordinary priority-boundary SBA pipeline. The + // prepared copy must already have finalized into its real Stack zone there. + assert_prepared_copy_finalized_on_stack(&runner, copy_id, exile_target); let scornmage_triggers = runner .state() @@ -93,27 +182,14 @@ fn issue_1312_prepared_swords_to_plowshares_triggers_lecturing_scornmage() { scornmage_triggers > 0, "Lecturing Scornmage must have SpellCast trigger after rehydrate" ); - let swords_stack_entry = runner - .state() - .stack - .iter() - .find(|entry| { - matches!( - entry.kind, - engine::types::game_state::StackEntryKind::Spell { .. } - ) - }) - .expect("prepared Swords copy must be on the stack after casting"); - let stack_ability = swords_stack_entry - .ability() - .expect("prepared spell stack entry must carry finalized ability"); - assert!( - !engine::game::ability_utils::flatten_targets_in_chain(stack_ability).is_empty(), - "prepared targeting spell must have targets on stack entry before trigger scan" - ); - runner.advance_until_stack_empty(); + assert_eq!(runner.state().objects[&exile_target].zone, Zone::Exile); + assert!(runner.state().stack.is_empty()); + // CR 608.2n + CR 704.5d + CR 704.5e: Swords resolves normally, then its + // previously proven-live synthetic copy ceases through the ordinary cleanup route. + assert!(!runner.state().objects.contains_key(©_id)); + let counters = runner .state() .objects @@ -126,3 +202,39 @@ fn issue_1312_prepared_swords_to_plowshares_triggers_lecturing_scornmage() { "Lecturing Scornmage must get a +1/+1 counter when a prepared targeting spell is cast" ); } + +#[test] +fn issue_1312_countered_prepared_copy_ceases_without_resolving() { + let Some(db) = load_db() else { + return; + }; + + let PreparedSwordsFixture { + mut runner, + emeritus, + exile_target, + scornmage: _, + counterspell, + } = build_prepared_swords_fixture(db, false, true); + let counterspell = counterspell.expect("Counterspell fixture requested"); + let copy_id = begin_prepared_cast(&mut runner, emeritus); + drive_cast_to_stack(&mut runner, exile_target); + assert_prepared_copy_finalized_on_stack(&runner, copy_id, exile_target); + + runner + .act(GameAction::PassPriority) + .expect("P0 should pass priority to the Counterspell controller"); + assert!(matches!( + runner.state().waiting_for, + WaitingFor::Priority { player: P1 } + )); + + let outcome = runner.cast(counterspell).target_object(copy_id).resolve(); + + // CR 701.6a: Counterspell removes the prepared spell without resolving it. + outcome.assert_zone(&[exile_target], Zone::Battlefield); + assert!(outcome.state().stack.is_empty()); + // CR 704.5d + CR 704.5e: Once its own spell entry is gone, the synthetic + // copy's live stack-residency exemption expires and the next SBA makes it cease. + assert!(!outcome.state().objects.contains_key(©_id)); +}