From e35395e2aa88d5df8695899fb77ef677ed8e8dc5 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 7 Aug 2026 18:33:56 -0700 Subject: [PATCH 1/3] fix: dispatch search landfall before priority --- crates/engine/src/game/effects/pay.rs | 4 - crates/engine/src/game/engine.rs | 9 +- .../src/game/engine_resolution_choices.rs | 23 +- crates/engine/src/game/triggers.rs | 2 +- .../integration/escape_tunnel_landfall.rs | 33 +++ .../issue_5336_kodama_mana_value_filter.rs | 7 +- crates/engine/tests/integration/main.rs | 1 + .../search_delivery_observer_dedup.rs | 214 ++++++++---------- 8 files changed, 145 insertions(+), 148 deletions(-) create mode 100644 crates/engine/tests/integration/escape_tunnel_landfall.rs diff --git a/crates/engine/src/game/effects/pay.rs b/crates/engine/src/game/effects/pay.rs index 6b075021d1..1c32af9fc9 100644 --- a/crates/engine/src/game/effects/pay.rs +++ b/crates/engine/src/game/effects/pay.rs @@ -1383,7 +1383,6 @@ mod tests { outcome, ResolutionChoiceOutcome::WaitingFor(_) | ResolutionChoiceOutcome::WaitingForWithInlineTriggers(_) - | ResolutionChoiceOutcome::WaitingForWithParkedObservers(_) | ResolutionChoiceOutcome::ActionResult(_) )); assert_eq!(state.players[0].life, 23); @@ -1541,7 +1540,6 @@ mod tests { match outcome { ResolutionChoiceOutcome::WaitingFor(_) => {} ResolutionChoiceOutcome::WaitingForWithInlineTriggers(_) => {} - ResolutionChoiceOutcome::WaitingForWithParkedObservers(_) => {} ResolutionChoiceOutcome::ActionResult(_) => {} } @@ -1660,7 +1658,6 @@ mod tests { outcome, ResolutionChoiceOutcome::WaitingFor(_) | ResolutionChoiceOutcome::WaitingForWithInlineTriggers(_) - | ResolutionChoiceOutcome::WaitingForWithParkedObservers(_) | ResolutionChoiceOutcome::ActionResult(_) )); assert_eq!(state.players[0].hand.len(), 2); @@ -1807,7 +1804,6 @@ mod tests { outcome, ResolutionChoiceOutcome::WaitingFor(_) | ResolutionChoiceOutcome::WaitingForWithInlineTriggers(_) - | ResolutionChoiceOutcome::WaitingForWithParkedObservers(_) | ResolutionChoiceOutcome::ActionResult(_) )); // All 7 mana units (4 colorless for X + W + U + B) must be spent — diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 54df0eb669..532a16a29a 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -6766,7 +6766,7 @@ fn apply_action( let mut events = Vec::new(); let mut triggers_processed_inline = false; - let mut skip_deferred_trigger_drain = false; + let skip_deferred_trigger_drain = false; // CancelAutoPass works from any WaitingFor state (player may cancel during // interactive choices). Routed by `actor` — previously used @@ -10402,13 +10402,6 @@ fn apply_action( triggers_processed_inline = true; waiting_for } - engine_resolution_choices::ResolutionChoiceOutcome::WaitingForWithParkedObservers( - waiting_for, - ) => { - triggers_processed_inline = true; - skip_deferred_trigger_drain = true; - waiting_for - } engine_resolution_choices::ResolutionChoiceOutcome::ActionResult(result) => { return Ok(result); } diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index 95dad02826..c63a800b84 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -496,8 +496,7 @@ fn finish_search_found_batch( events, ) { ResolutionChoiceOutcome::WaitingFor(waiting) - | ResolutionChoiceOutcome::WaitingForWithInlineTriggers(waiting) - | ResolutionChoiceOutcome::WaitingForWithParkedObservers(waiting) => waiting, + | ResolutionChoiceOutcome::WaitingForWithInlineTriggers(waiting) => waiting, ResolutionChoiceOutcome::ActionResult(result) => result.waiting_for, }, ) @@ -506,10 +505,6 @@ fn finish_search_found_batch( pub(super) enum ResolutionChoiceOutcome { WaitingFor(WaitingFor), WaitingForWithInlineTriggers(WaitingFor), - /// CR 603.3b: observer triggers from a completed search put/shuffle were - /// collected into `deferred_triggers` but must not drain until the caller - /// receives priority again (issue #5336: Kodama + Nature's Lore). - WaitingForWithParkedObservers(WaitingFor), ActionResult(ActionResult), } @@ -616,9 +611,9 @@ fn batch_or_drain_observer_triggers( } /// CR 603.2 + CR 603.3b + CR 701.23: after a search tutor's put/shuffle -/// continuation drains, park ETB/dies/discards observers for the next priority -/// checkpoint instead of dispatching them while the test harness (or UI) may -/// still be inside the same `SelectCards` action (issue #5336). +/// continuation drains, collect ETB/dies/discards observers before this +/// `SelectCards` action reaches its priority checkpoint. The ordinary +/// post-action drain then puts them on the stack before priority is returned. /// /// CR 603.2c: this slice spans the whole continuation drain, so it holds both /// the delivery's logical zone-change owner's occurrences (already collected by @@ -629,7 +624,7 @@ fn batch_or_drain_observer_triggers( /// authority instead. That authority's ledger half applies to every event kind, /// matching the generic priority scan. Without it a fetched land's landfall/ETB /// observers fire twice. -fn park_search_observer_triggers( +fn collect_search_observer_triggers( state: &mut GameState, events: &[GameEvent], events_before_drain: usize, @@ -651,7 +646,7 @@ fn park_search_observer_triggers( // shared carrier authority prove that every such frame has drained before // retiring the parent and releasing its CR 400.7j self-move link. super::engine::settle_resolving_stack_entry_after_continuation_resume(state); - ResolutionChoiceOutcome::WaitingForWithParkedObservers(state.waiting_for.clone()) + ResolutionChoiceOutcome::WaitingForWithInlineTriggers(state.waiting_for.clone()) } pub(super) fn handles(waiting_for: &WaitingFor) -> bool { @@ -995,7 +990,7 @@ fn finalize_standard_search_selection( // before (and instead of stranding) the ordinary rider. super::engine::resume_pending_continuation_if_priority(state, events) .expect("a settled search choice must resume its continuation"); - park_search_observer_triggers(state, events, events_before_drain) + collect_search_observer_triggers(state, events, events_before_drain) } /// CR 800.4a + CR 701.23a: If the exact hidden zone backing an ordinary @@ -3902,7 +3897,7 @@ pub(super) fn handle_resolution_choice( set_priority(state, player); super::engine::resume_pending_continuation_if_priority(state, events) .expect("a settled search choice must resume its continuation"); - return Ok(park_search_observer_triggers( + return Ok(collect_search_observer_triggers( state, events, events_before_drain, @@ -3973,7 +3968,7 @@ pub(super) fn handle_resolution_choice( set_priority(state, player); super::engine::resume_pending_continuation_if_priority(state, events) .expect("a settled search choice must resume its continuation"); - park_search_observer_triggers(state, events, events_before_partition) + collect_search_observer_triggers(state, events, events_before_partition) } ( WaitingFor::OutsideGameChoice { diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index 88a4d90d9a..04f9bbe0c0 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -8505,7 +8505,7 @@ pub(crate) fn filter_consumed_trigger_events( /// NOT need this — a blanket `ZoneChanged` drop is equivalent there, and that is /// what `engine_resolution_choices::batch_or_drain_observer_triggers` /// (owner-bounded slice + `zone_changes_are_logically_owned`) and the resumed -/// `ChangeZone` drain in `effects/mod.rs` do. `park_search_observer_triggers`' +/// `ChangeZone` drain in `effects/mod.rs` do. `collect_search_observer_triggers`' /// slice spans a whole continuation drain and can hold zone changes no owner /// allocated a group for, so it must consult this instead. /// diff --git a/crates/engine/tests/integration/escape_tunnel_landfall.rs b/crates/engine/tests/integration/escape_tunnel_landfall.rs new file mode 100644 index 0000000000..437fdae227 --- /dev/null +++ b/crates/engine/tests/integration/escape_tunnel_landfall.rs @@ -0,0 +1,33 @@ +use engine::game::scenario::{GameScenario, P0}; +use engine::types::card_type::{CoreType, Supertype}; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +const ESCAPE_TUNNEL_ORACLE: &str = "{T}, Sacrifice this land: Search your library for a basic land card, put it onto the battlefield tapped, then shuffle.\n{T}, Sacrifice this land: Target creature with power 2 or less can't be blocked this turn."; +const KAZANDU_NECTARPOT_ORACLE: &str = + "Landfall — Whenever a land you control enters, you gain 1 life."; + +#[test] +fn escape_tunnel_search_land_enters_and_triggers_landfall() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let tunnel = scenario + .add_land_from_oracle(P0, "Escape Tunnel", ESCAPE_TUNNEL_ORACLE) + .id(); + scenario.add_creature_from_oracle(P0, "Kazandu Nectarpot", 1, 2, KAZANDU_NECTARPOT_ORACLE); + let forest = scenario.add_card_to_library_top(P0, "Forest"); + let mut runner = scenario.build(); + let forest_object = runner.state_mut().objects.get_mut(&forest).unwrap(); + forest_object.card_types.core_types.push(CoreType::Land); + forest_object.card_types.supertypes.push(Supertype::Basic); + forest_object.base_card_types = forest_object.card_types.clone(); + + // CR 603.2 + CR 603.3: the fetched basic land's entry triggers landfall, + // which goes on the stack before priority and resolves after both players pass. + let outcome = runner.activate(tunnel, 0).search_first_legal().resolve(); + + outcome.assert_zone(&[tunnel], Zone::Graveyard); + outcome.assert_zone(&[forest], Zone::Battlefield); + assert!(outcome.state().objects[&forest].tapped); + outcome.assert_life_delta(P0, 1); +} diff --git a/crates/engine/tests/integration/issue_5336_kodama_mana_value_filter.rs b/crates/engine/tests/integration/issue_5336_kodama_mana_value_filter.rs index 8e75aa4349..df29337df6 100644 --- a/crates/engine/tests/integration/issue_5336_kodama_mana_value_filter.rs +++ b/crates/engine/tests/integration/issue_5336_kodama_mana_value_filter.rs @@ -276,7 +276,12 @@ fn kodama_natures_lore_forest_limits_hand_to_zero_mana_value() { let mut runner = scenario.build(); let forest = seed_forest_on_library_top(&mut runner); - runner.cast(natures_lore).search_first_legal().resolve(); + runner.cast(natures_lore).resolve(); + runner + .act(GameAction::SelectCards { + cards: vec![forest], + }) + .expect("selecting Nature's Lore's Forest must succeed"); assert_eq!( runner.state().objects[&forest].zone, diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 7f261d4b48..46d28d5185 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -181,6 +181,7 @@ mod engine_invariants; mod enlightened_tutor_regression; mod equipment_etb_attach_parent_target; mod ertai_trickery_counter_kicked; +mod escape_tunnel_landfall; mod etali_primal_sickness_poison; mod etrata_cloak_enters_under_cloaker_5944; mod evelyn_regression; diff --git a/crates/engine/tests/integration/search_delivery_observer_dedup.rs b/crates/engine/tests/integration/search_delivery_observer_dedup.rs index 9e61d48980..158a7e3bce 100644 --- a/crates/engine/tests/integration/search_delivery_observer_dedup.rs +++ b/crates/engine/tests/integration/search_delivery_observer_dedup.rs @@ -10,7 +10,7 @@ //! 1. the logical zone-change owner — `change_zone::resolve` / //! `zone_pipeline::move_objects_simultaneously_then` → //! `triggers::complete_logical_zone_trigger_collection`; -//! 2. `engine_resolution_choices::park_search_observer_triggers`, which +//! 2. `engine_resolution_choices::collect_search_observer_triggers`, which //! re-scanned the raw action slice and collected again with no filter but //! `PhaseChanged`. //! @@ -23,22 +23,11 @@ //! occurrences." Row `two_land_search_delivery_fires_landfall_twice` pins the //! second sentence so the fix is not over-applied into a blanket suppression. //! -//! HARNESS NOTE — every park-path row that asserts what the parked observers -//! DID passes priority first (H1, H2, H5, H6, N1, N2). Two park-path rows -//! deliberately assert PRE-drain and must NOT pass, because a pass runs the -//! drain, empties the queue and makes the assertion vacuous: N5 -//! (`parked_delivery_records_carry_distinct_occurrence_indices`) asserts on the -//! parked queue's CONTENTS, behind the `assert_observers_were_parked` -//! reach-guard; and N4 (`fetch_with_no_legal_target_parks_nothing`) asserts the -//! queue is EMPTY on a fail-to-find, which is also what licenses its no-firing -//! assertion without a pass. H3, H4 and N3 are NOT park paths; they reach the -//! stack in-action or through `advance_until_stack_empty`. -//! `park_search_observer_triggers` deliberately defers its observers to the NEXT -//! priority checkpoint (issue #5336): the parked action returns -//! `ResolutionChoiceOutcome::WaitingForWithParkedObservers`, which sets -//! `skip_deferred_trigger_drain`, and both `drive_resolution` and -//! `advance_until_stack_empty` break immediately on an empty stack. Asserting at -//! the end of the parked action therefore measures nothing at all. +//! HARNESS NOTE — after a search choice settles, its observer triggers must be +//! dispatched before either player receives priority (CR 603.3). H1, H2, H5, +//! H6, N2, and N5 inspect that immediate post-choice state; H3, H4, and N3 +//! reach the same result through other zone-change paths. N1 and N4 assert the +//! corresponding no-trigger cases. use engine::ai_support::validated_candidate_actions_for_semantic_owner; use engine::game::scenario::{GameRunner, GameScenario, P0}; @@ -97,39 +86,29 @@ fn misty_ability_index(state: &GameState, misty: ObjectId) -> usize { .expect("Misty Rainforest's printed activated ability must be a validated root candidate") } -/// The positive reach-guard every park-path row shares: the action really did -/// settle back to `Priority` with an EMPTY stack, which is what proves the -/// observers were parked (issue #5336) rather than dispatched inline. Without -/// this, a row that never reached the deferred drain would look identical to a -/// row that reached it and found one trigger. -fn assert_observers_were_parked(runner: &GameRunner) { +/// CR 603.3: search-delivery observers must be dispatched before priority. +fn assert_observers_were_dispatched(runner: &GameRunner) { assert!( - matches!(runner.state().waiting_for, WaitingFor::Priority { .. }), - "the parked action must settle back to Priority, got {:?}", + matches!( + runner.state().waiting_for, + WaitingFor::Priority { .. } | WaitingFor::OrderTriggers { .. } + ), + "the search choice must settle into trigger dispatch, got {:?}", runner.state().waiting_for ); assert!( - runner.state().stack.is_empty(), - "issue #5336: park defers observers to the NEXT priority checkpoint, \ - so nothing may be on the stack yet" + runner.state().deferred_triggers.is_empty(), + "all search-delivery observers must leave the deferred queue before priority" ); assert!( - !runner.state().deferred_triggers.is_empty(), - "the delivery's observers must actually be sitting in the parked queue" + !runner.state().stack.is_empty() + || matches!(runner.state().waiting_for, WaitingFor::OrderTriggers { .. }), + "the delivery's observers must be on the stack or awaiting their mandatory order" ); } -/// Reach the priority checkpoint park exists to defer to. The parked action set -/// `skip_deferred_trigger_drain`; the NEXT action runs the post-action pipeline -/// without it and hits the deferred drain. -fn pass_priority_to_reach_the_drain(runner: &mut GameRunner) { - runner - .act(GameAction::PassPriority) - .expect("a priority pass must reach the deferred-trigger drain"); -} - // --------------------------------------------------------------------------- -// H1 — reported symptom 1: landfall fires ONCE on a cracked fetch (park site A) +// H1 — reported symptom 1: landfall fires ONCE on a cracked fetch // --------------------------------------------------------------------------- #[test] @@ -140,7 +119,7 @@ fn fetchland_crack_fires_landfall_observer_once() { let misty = scenario.add_real_card(P0, "Misty Rainforest", Zone::Battlefield, db); let mammoth = scenario.add_real_card(P0, "Kazandu Mammoth", Zone::Battlefield, db); // A basic Forest is the ONLY card Misty's filter can find, and it has no ETB - // trigger — so exactly ONE observer is parked and no `OrderTriggers` or + // trigger — so exactly ONE observer is dispatched and no `OrderTriggers` or // surveil prompt entangles the assertion. let forest = scenario.add_real_card(P0, "Forest", Zone::Library, db); @@ -154,19 +133,19 @@ fn fetchland_crack_fires_landfall_observer_once() { "Kazandu Mammoth's printed body is 3/3 before any landfall" ); + runner.activate(misty, ability_index).resolve(); runner - .activate(misty, ability_index) - .search_first_legal() - .resolve(); + .act(GameAction::SelectCards { + cards: vec![forest], + }) + .expect("selecting the searched Forest must succeed"); assert_eq!( runner.state().objects[&forest].zone, Zone::Battlefield, "the fetch must actually have delivered the Forest" ); - assert_observers_were_parked(&runner); - - pass_priority_to_reach_the_drain(&mut runner); + assert_observers_were_dispatched(&runner); runner.advance_until_stack_empty(); // 3/3 base, +2/+2 exactly once. 7/7 is the double collection. @@ -178,7 +157,7 @@ fn fetchland_crack_fires_landfall_observer_once() { } // --------------------------------------------------------------------------- -// H2 — reported symptom 2: the fetched land's own ETB fires ONCE (park site A) +// H2 — reported symptom 2: the fetched land's own ETB fires ONCE // --------------------------------------------------------------------------- #[test] @@ -188,32 +167,32 @@ fn fetchland_fetched_land_etb_trigger_fires_once() { scenario.at_phase(Phase::PreCombatMain); let misty = scenario.add_real_card(P0, "Misty Rainforest", Zone::Battlefield, db); // No Kazandu Mammoth here: Undercity Sewers' own "When this land enters, - // surveil 1" is the single observer, so the drain parks exactly one trigger. + // surveil 1" is the single observer, so exactly one trigger is dispatched. let sewers = scenario.add_real_card(P0, "Undercity Sewers", Zone::Library, db); let mut runner = scenario.build(); engine::game::rehydrate_game_from_card_db(runner.state_mut(), db); let ability_index = misty_ability_index(runner.state(), misty); + runner.activate(misty, ability_index).resolve(); runner - .activate(misty, ability_index) - .search_first_legal() - .resolve(); + .act(GameAction::SelectCards { + cards: vec![sewers], + }) + .expect("selecting the searched land must succeed"); assert_eq!( runner.state().objects[&sewers].zone, Zone::Battlefield, "the fetch must actually have delivered Undercity Sewers" ); - assert_observers_were_parked(&runner); - - pass_priority_to_reach_the_drain(&mut runner); + assert_observers_were_dispatched(&runner); - // Count STACK OBJECTS, not prompts, and stop here — deliberately before the - // surveil prompt, which `advance_until_stack_empty` does not model. + // Count stack objects directly after the choice, before the surveil trigger + // resolves into its prompt. assert!( !runner.state().stack.is_empty(), - "the priority pass must have run the deferred drain" + "the search choice must have dispatched the fetched land's ETB" ); let surveil_copies = runner .state() @@ -344,7 +323,7 @@ fn fetch_pauses_on_optional_replacement_then_fires_landfall_once() { } // --------------------------------------------------------------------------- -// H5 — park site B: the single-basic partition fast path +// H5 — the single-basic partition fast path // --------------------------------------------------------------------------- #[test] @@ -362,27 +341,30 @@ fn cultivate_fast_path_fires_landfall_once() { engine::game::rehydrate_game_from_card_db(runner.state_mut(), db); add_mana(&mut runner, 1, 0, 2); - runner.cast(cultivate).search_first_legal().resolve(); + runner.cast(cultivate).resolve(); + runner + .act(GameAction::SelectCards { + cards: vec![forest], + }) + .expect("selecting Cultivate's battlefield basic must succeed"); assert_eq!( runner.state().objects[&forest].zone, Zone::Battlefield, "the fast path must have delivered the single basic" ); - assert_observers_were_parked(&runner); - - pass_priority_to_reach_the_drain(&mut runner); + assert_observers_were_dispatched(&runner); runner.advance_until_stack_empty(); assert_eq!( power_toughness(&runner, mammoth), (5, 5), - "park site B must fire the landfall observer exactly once" + "the single-basic partition path must fire landfall exactly once" ); } // --------------------------------------------------------------------------- -// H6 — park site C: the explicit `SearchPartitionChoice` route +// H6 — the explicit `SearchPartitionChoice` route // --------------------------------------------------------------------------- #[test] @@ -407,7 +389,7 @@ fn cultivate_partition_fires_landfall_once() { runner.state().waiting_for, WaitingFor::SearchPartitionChoice { .. } ), - "two findable basics must park a SearchPartitionChoice, got {:?}", + "two findable basics must reach a SearchPartitionChoice, got {:?}", runner.state().waiting_for ); runner @@ -426,15 +408,13 @@ fn cultivate_partition_fires_landfall_once() { Zone::Hand, "the rest basic must reach the hand — exactly ONE land entered" ); - assert_observers_were_parked(&runner); - - pass_priority_to_reach_the_drain(&mut runner); + assert_observers_were_dispatched(&runner); runner.advance_until_stack_empty(); assert_eq!( power_toughness(&runner, mammoth), (5, 5), - "park site C must fire the landfall observer exactly once" + "the explicit partition path must fire landfall exactly once" ); } @@ -458,11 +438,12 @@ fn search_to_hand_delivers_and_fires_no_landfall() { // Journey of Discovery is modal + entwine; mode 0 is the // `ChangeZone { Library -> Hand }` half. + runner.cast(journey).modes(&[0]).resolve(); runner - .cast(journey) - .modes(&[0]) - .search_first_legal() - .resolve(); + .act(GameAction::SelectCards { + cards: vec![forest, mountain], + }) + .expect("selecting Journey of Discovery's basics must succeed"); // The positive reach-guard: the search really delivered. Without it the // (3,3) assertion below could pass on a fail-to-find. @@ -477,11 +458,6 @@ fn search_to_hand_delivers_and_fires_no_landfall() { "mode 0 must put the found basics into HAND" ); - // The priority pass is mandatory here: without it (3,3) would be satisfied - // by mere deferral rather than by there being no landfall at all. - pass_priority_to_reach_the_drain(&mut runner); - runner.advance_until_stack_empty(); - assert_eq!( power_toughness(&runner, mammoth), (3, 3), @@ -509,11 +485,12 @@ fn two_land_search_delivery_fires_landfall_twice() { engine::game::rehydrate_game_from_card_db(runner.state_mut(), db); add_mana(&mut runner, 1, 0, 2); + runner.cast(harrow).sacrifice_with(&[spare]).resolve(); runner - .cast(harrow) - .sacrifice_with(&[spare]) - .search_first_legal() - .resolve(); + .act(GameAction::SelectCards { + cards: vec![forest, island], + }) + .expect("selecting Harrow's basics must succeed"); assert_eq!( runner.state().objects[&forest].zone, @@ -525,11 +502,9 @@ fn two_land_search_delivery_fires_landfall_twice() { Zone::Battlefield, "both basics must have been delivered" ); - assert_observers_were_parked(&runner); - - pass_priority_to_reach_the_drain(&mut runner); + assert_observers_were_dispatched(&runner); // `advance_until_stack_empty` drains the CR 603.3b `OrderTriggers` prompt - // internally; both parked triggers are pumps with no further prompt. + // internally; both dispatched triggers are pumps with no further prompt. runner.advance_until_stack_empty(); assert_eq!( @@ -596,7 +571,7 @@ fn aura_exiled_via_targeted_change_zone_fires_delayed_sacrifice() { } // --------------------------------------------------------------------------- -// N4 — fail-to-find: an empty park slice still short-circuits cleanly +// N4 — fail-to-find: an empty delivery slice still short-circuits cleanly // --------------------------------------------------------------------------- #[test] @@ -636,7 +611,7 @@ fn fetch_with_no_legal_target_parks_nothing() { assert!( runner.state().deferred_triggers.is_empty(), - "a fail-to-find delivers nothing, so nothing may be parked" + "a fail-to-find delivers nothing, so no trigger may be deferred" ); assert_eq!( power_toughness(&runner, mammoth), @@ -646,7 +621,7 @@ fn fetch_with_no_legal_target_parks_nothing() { } // --------------------------------------------------------------------------- -// N5 — CR 400.7 + CR 603.2c: the two occurrences parked by ONE search delivery +// N5 — CR 400.7 + CR 603.2c: the two occurrences dispatched by ONE search delivery // carry DISTINCT `turn_zone_change_index` values, each agreeing with the // ledger row `restrictions::record_zone_change` wrote for it. // @@ -677,18 +652,14 @@ fn fetch_with_no_legal_target_parks_nothing() { // `PartialEq`. That link is pinned at the authority layer by // `occurrence_exact_witness_consumes_the_occurrence_its_witness_names`. // -// NO PRIORITY PASS — DELIBERATE, AND THE EXCEPTION THE HARNESS NOTE ABOVE -// NAMES. This row asserts on the PRE-DRAIN parked queue, not on a -// post-drain effect, so `assert_observers_were_parked` (which requires a -// NON-EMPTY `deferred_triggers`) is its positive reach-guard. Passing -// priority first would run the drain, empty the queue, and make every -// assertion below vacuous — the exact opposite of what the harness note -// protects against for post-drain rows. `fetch_with_no_legal_target_parks_nothing` -// already asserts on `deferred_triggers` the same way. +// NO PRIORITY PASS — DELIBERATE. This row reads the trigger events after +// the mandatory CR 603.3 dispatch but before the triggered abilities +// resolve, so each event's occurrence index remains observable on its +// stack entry. // --------------------------------------------------------------------------- #[test] -fn parked_delivery_records_carry_distinct_occurrence_indices() { +fn dispatched_delivery_records_carry_distinct_occurrence_indices() { let db = shared_card_db().expect("integration card fixture must load"); let mut scenario = GameScenario::new(); scenario.at_phase(Phase::PreCombatMain); @@ -703,14 +674,15 @@ fn parked_delivery_records_carry_distinct_occurrence_indices() { engine::game::rehydrate_game_from_card_db(runner.state_mut(), db); add_mana(&mut runner, 1, 0, 2); + runner.cast(harrow).sacrifice_with(&[spare]).resolve(); runner - .cast(harrow) - .sacrifice_with(&[spare]) - .search_first_legal() - .resolve(); + .act(GameAction::SelectCards { + cards: vec![forest, island], + }) + .expect("selecting Harrow's basics must succeed"); - // Reach-guards: the delivery really happened, and the observers really are - // sitting in the PRE-drain parked queue. + // Reach-guards: the delivery really happened and its observers have been + // dispatched before priority. assert_eq!( runner.state().objects[&forest].zone, Zone::Battlefield, @@ -721,26 +693,28 @@ fn parked_delivery_records_carry_distinct_occurrence_indices() { Zone::Battlefield, "both basics must have been delivered" ); - assert_observers_were_parked(&runner); + assert_observers_were_dispatched(&runner); - // Every `ZoneChanged` the parked contexts carry, paired with the occurrence - // index its record was stamped with. `GameEvent` is spelled out rather than - // imported so this row adds no import to the module. - let parked: Vec<(ObjectId, usize)> = runner + // Every `ZoneChanged` carried by the dispatched trigger stack entries, + // paired with the occurrence index its record was stamped with. + let dispatched: Vec<(ObjectId, usize)> = runner .state() - .deferred_triggers + .stack .iter() - .flat_map(|context| context.trigger_events.iter()) - .filter_map(|event| match event { - engine::types::events::GameEvent::ZoneChanged { - object_id, record, .. + .filter_map(|entry| match &entry.kind { + engine::types::game_state::StackEntryKind::TriggeredAbility { + trigger_event: + Some(engine::types::events::GameEvent::ZoneChanged { + object_id, record, .. + }), + .. } => Some((*object_id, record.turn_zone_change_index)), _ => None, }) .collect(); let indices_for = |land: ObjectId| -> Vec { - parked + dispatched .iter() .filter(|(id, _)| *id == land) .map(|(_, index)| *index) @@ -753,11 +727,11 @@ fn parked_delivery_records_carry_distinct_occurrence_indices() { // below cannot pass vacuously on an empty iterator. assert!( !forest_indices.is_empty(), - "the parked queue must carry a ZoneChanged for the delivered Forest" + "the dispatched trigger must carry a ZoneChanged for the delivered Forest" ); assert!( !island_indices.is_empty(), - "the parked queue must carry a ZoneChanged for the delivered Island" + "the dispatched trigger must carry a ZoneChanged for the delivered Island" ); assert!( forest_indices @@ -774,8 +748,8 @@ fn parked_delivery_records_carry_distinct_occurrence_indices() { // `restrictions::record_zone_change` allocates a distinct index for each. assert_ne!( forest_indices[0], island_indices[0], - "CR 400.7 + CR 603.2c: two distinct occurrences in ONE park slice must \ - carry DISTINCT turn_zone_change_index values, or the queued-context \ + "CR 400.7 + CR 603.2c: two distinct occurrences in ONE delivery must \ + carry DISTINCT turn_zone_change_index values, or the trigger-context \ witness could cross-consume them" ); From 5d4114ac3474171292b1455c11d4afbf3e970583 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 7 Aug 2026 18:58:36 -0700 Subject: [PATCH 2/3] test: preserve batched search trigger context --- crates/engine/src/game/engine.rs | 5 ++- .../search_delivery_observer_dedup.rs | 36 ++++++++++++------- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 532a16a29a..9507241224 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -15880,7 +15880,10 @@ mod stage2_injector_tests { // `scoped_library_search.rs`, neither of which this change touches, and the // test module it adds contains no line matching the needle — total still 37, // partition still 5/7/25. - "game/engine.rs:11828".to_string(), + // Search-observer dispatch: `:11828 ⇒ :11821`, −7. Removing the retired + // `WaitingForWithParkedObservers` match arm is the only hunk above this + // producer; it changes trigger-drain timing but does not add a prompt. + "game/engine.rs:11821".to_string(), ], "the five production producers, NAMED: the CR 603.5 gate in `resolve_chain_body` \ plus the two repeated-optional-payment drivers, the per-player acceptance cursor \ diff --git a/crates/engine/tests/integration/search_delivery_observer_dedup.rs b/crates/engine/tests/integration/search_delivery_observer_dedup.rs index 158a7e3bce..d39b685a09 100644 --- a/crates/engine/tests/integration/search_delivery_observer_dedup.rs +++ b/crates/engine/tests/integration/search_delivery_observer_dedup.rs @@ -696,20 +696,32 @@ fn dispatched_delivery_records_carry_distinct_occurrence_indices() { assert_observers_were_dispatched(&runner); // Every `ZoneChanged` carried by the dispatched trigger stack entries, - // paired with the occurrence index its record was stamped with. - let dispatched: Vec<(ObjectId, usize)> = runner - .state() + // paired with the occurrence index its record was stamped with. A trigger + // for simultaneous entries stores its complete event batch in the side + // table keyed by stack-entry id; the stack payload itself holds only its + // representative event. + let state = runner.state(); + let dispatched: Vec<(ObjectId, usize)> = state .stack .iter() - .filter_map(|entry| match &entry.kind { - engine::types::game_state::StackEntryKind::TriggeredAbility { - trigger_event: - Some(engine::types::events::GameEvent::ZoneChanged { - object_id, record, .. - }), - .. - } => Some((*object_id, record.turn_zone_change_index)), - _ => None, + .flat_map(|entry| { + let trigger_events = state + .stack_trigger_event_batches + .get(&entry.id) + .map(Vec::as_slice) + .unwrap_or_else(|| match &entry.kind { + engine::types::game_state::StackEntryKind::TriggeredAbility { + trigger_event, + .. + } => trigger_event.as_slice(), + _ => &[], + }); + trigger_events.iter().filter_map(|event| match event { + engine::types::events::GameEvent::ZoneChanged { + object_id, record, .. + } => Some((*object_id, record.turn_zone_change_index)), + _ => None, + }) }) .collect(); From cb1cb3115d50a3d0d62afee64e9e95f2b4baa205 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 7 Aug 2026 19:22:59 -0700 Subject: [PATCH 3/3] test: order simultaneous search triggers before inspection --- .../search_delivery_observer_dedup.rs | 47 +++++++++---------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/crates/engine/tests/integration/search_delivery_observer_dedup.rs b/crates/engine/tests/integration/search_delivery_observer_dedup.rs index d39b685a09..160e55f192 100644 --- a/crates/engine/tests/integration/search_delivery_observer_dedup.rs +++ b/crates/engine/tests/integration/search_delivery_observer_dedup.rs @@ -695,33 +695,32 @@ fn dispatched_delivery_records_carry_distinct_occurrence_indices() { ); assert_observers_were_dispatched(&runner); + // CR 603.3b requires the controller to order the two simultaneous landfall + // triggers before either is put on the stack. This is not a priority pass + // and does not resolve either trigger, so their event contexts remain + // observable below. + if let WaitingFor::OrderTriggers { triggers, .. } = runner.state().waiting_for.clone() { + let order = (0..triggers.len()).collect(); + runner + .act(GameAction::OrderTriggers { order }) + .expect("ordering the simultaneous landfall triggers must succeed"); + } + // Every `ZoneChanged` carried by the dispatched trigger stack entries, - // paired with the occurrence index its record was stamped with. A trigger - // for simultaneous entries stores its complete event batch in the side - // table keyed by stack-entry id; the stack payload itself holds only its - // representative event. - let state = runner.state(); - let dispatched: Vec<(ObjectId, usize)> = state + // paired with the occurrence index its record was stamped with. + let dispatched: Vec<(ObjectId, usize)> = runner + .state() .stack .iter() - .flat_map(|entry| { - let trigger_events = state - .stack_trigger_event_batches - .get(&entry.id) - .map(Vec::as_slice) - .unwrap_or_else(|| match &entry.kind { - engine::types::game_state::StackEntryKind::TriggeredAbility { - trigger_event, - .. - } => trigger_event.as_slice(), - _ => &[], - }); - trigger_events.iter().filter_map(|event| match event { - engine::types::events::GameEvent::ZoneChanged { - object_id, record, .. - } => Some((*object_id, record.turn_zone_change_index)), - _ => None, - }) + .filter_map(|entry| match &entry.kind { + engine::types::game_state::StackEntryKind::TriggeredAbility { + trigger_event: + Some(engine::types::events::GameEvent::ZoneChanged { + object_id, record, .. + }), + .. + } => Some((*object_id, record.turn_zone_change_index)), + _ => None, }) .collect();