diff --git a/crates/engine/src/game/engine_priority.rs b/crates/engine/src/game/engine_priority.rs index f8f1736579..55f7db7a61 100644 --- a/crates/engine/src/game/engine_priority.rs +++ b/crates/engine/src/game/engine_priority.rs @@ -100,17 +100,13 @@ pub(crate) fn run_post_action_pipeline_from( } } // A completed logical owner has already collected its segment and - // settlement contexts into the existing deferred queue. The owner is - // intentionally gone before the trailing completion event, so use those - // exact queued occurrences to keep the generic scan from rediscovering - // them while still allowing every unrelated event through. - let deferred_logical_zone_events: Vec<_> = state - .deferred_triggers - .iter() - .flat_map(|context| context.trigger_events.iter()) - .filter(|event| matches!(event, GameEvent::ZoneChanged { .. })) - .collect(); - let unconsumed_events = triggers::filter_consumed_trigger_events_from( + // settlement contexts into the deferred queue, and a paused owner that + // drained may instead have claimed them in the consumed ledger. + // `filter_already_collected_trigger_events_from` is the single authority + // for both (CR 603.2c), shared with the search-delivery park family so + // the two collectors cannot drift. + let unconsumed_events = triggers::filter_already_collected_trigger_events_from( + state, events, event_start, &consumed_trigger_events, @@ -121,7 +117,6 @@ pub(crate) fn run_post_action_pipeline_from( !matches!(event, GameEvent::PhaseChanged { .. }) && !state.deferred_entry_events.contains(event) && !retained_logical_zone_events.contains(event) - && !deferred_logical_zone_events.contains(event) }) .cloned() .collect(); diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index b434a6b5eb..644b0f78f2 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -567,6 +567,13 @@ fn batch_or_drain_observer_triggers( events: &mut Vec, event_slice_start: usize, event_slice_end: usize, + // CR 603.2c: `true` declares that `events[event_slice_start..event_slice_end]` + // is exactly one completed logical zone-change owner's completion slice. + // `LogicalZoneChangeGroup::append_delivery_events` retains EVERY `ZoneChanged` + // in the slice it is handed, so within such a slice a blanket drop is + // equivalent to per-occurrence suppression. A collector whose slice is not + // owner-bounded must use + // `triggers::filter_already_collected_trigger_events_from` instead. zone_changes_are_logically_owned: bool, ) -> Option { if matches!(state.waiting_for, WaitingFor::Priority { .. }) { @@ -611,15 +618,30 @@ fn batch_or_drain_observer_triggers( /// 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). +/// +/// 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 +/// `change_zone::resolve` / `zone_pipeline::move_objects_simultaneously_then`) +/// AND zone changes no owner allocated a group for. It is therefore NOT +/// owner-bounded and cannot blanket-drop `ZoneChanged` the way +/// `batch_or_drain_observer_triggers` does; it consults the shared ownership +/// 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( state: &mut GameState, events: &[GameEvent], events_before_drain: usize, ) -> ResolutionChoiceOutcome { - let trigger_events: Vec = events[events_before_drain..] - .iter() + let uncollected_events = super::triggers::filter_already_collected_trigger_events_from( + state, + events, + events_before_drain, + &state.consumed_before_priority_trigger_events, + ); + let trigger_events: Vec = uncollected_events + .into_iter() .filter(|ev| !matches!(ev, GameEvent::PhaseChanged { .. })) - .cloned() .collect(); if !trigger_events.is_empty() { super::triggers::collect_triggers_into_deferred(state, &trigger_events); diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index 1b030177b7..43db9608b6 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -8276,6 +8276,107 @@ pub(crate) fn filter_consumed_trigger_events( filter_consumed_trigger_events_from(events, 0, consumed) } +/// CR 603.2c: Remove from `events[event_start..]` the occurrences a trigger +/// collector has already taken, so a second collector over the same raw slice +/// cannot fire the same observers twice. +/// +/// Two witnesses answer "already collected", and neither is sufficient alone: +/// +/// 1. `consumed` — occurrences explicitly claimed by +/// [`mark_logical_zone_events_consumed_before_priority`]. Required wherever an +/// intervening `drain_deferred_trigger_queue` has already emptied +/// `deferred_triggers`. Only three owners mark, and their ordinals are NOT +/// uniformly exact: `effects/mod.rs` passes the whole action buffer (exact); +/// `zone_pipeline.rs`'s synchronous-completion site passes a sub-slice, so its +/// ordinals are rebased; and `zone_pipeline.rs`'s batch-drain site passes the +/// whole buffer only after `drain_pending_batch_deliveries` has moved it out +/// and re-assembled it, so its ordinals are computed against a re-ordered +/// buffer. See the warning on [`filter_consumed_trigger_events_from`]. +/// 2. `state.deferred_triggers` — the `ZoneChanged` values carried by contexts +/// that [`complete_logical_zone_trigger_collection`] and +/// [`append_and_collect_logical_zone_trigger_segment`] already queued. This is +/// the only witness for the four owners that deliberately do NOT mark +/// (`effects/change_zone.rs` x2, `engine_resolution_choices.rs` x2). Do NOT +/// "fix" that asymmetry by adding `mark_`: claiming an occurrence also hides +/// it from `check_delayed_triggers` (`engine_priority.rs`), which would +/// silently kill the CR 603.7b leaves-the-battlefield delayed family (an +/// ability triggers only the next time its trigger event occurs; hide the +/// event and it never triggers). +/// +/// WITNESS 2 IS A BOUND, NOT AN OCCURRENCE COUNT. `deferred_triggers` holds one +/// context per matching observer, not one entry per occurrence (every zone-change +/// collection site pushes one `PendingTriggerContext::batched` per matched +/// `(object_id, trig_idx)`, and for a non-batched trigger `matched.trigger_events` +/// is the singleton `vec![event.clone()]`, so one context is one witness copy; a +/// batched trigger carries its whole matched batch, which yields more witnesses, +/// never fewer), so N observers of ONE occurrence contribute N copies of that +/// value. Consuming +/// witnesses one-for-one therefore removes at most `min(queued_copies, +/// slice_copies)` — never more than the set-membership filter this replaces at +/// the priority scan, which removed every copy. It is NOT occurrence-exact and +/// does NOT by itself discharge CR 603.2c's second sentence ("it can trigger +/// repeatedly if one event contains multiple occurrences"): if a slice holds a +/// byte-identical `ZoneChanged` that no owner collected alongside one that two +/// observers saw, both are dropped. At the priority scan that residual is no +/// larger than the filter this replaces; at the search-delivery park there is no +/// prior `ZoneChanged` filter at all, so the residual is new there and is bounded +/// by byte-identical `ZoneChanged` duplicates being unreachable inside one +/// collector slice. An occurrence-exact witness is NOT available here: the only +/// exact record is `LogicalZoneChangeGroup::all_origin_occurrences`, and a +/// completed owner's group is a caller-owned local that is gone before this runs +/// (`GameState` holds a group only inside the two *paused* frames, +/// `PendingChangeZoneIteration` and `PendingBatchDeliveries`). +/// +/// A collector whose slice is provably exactly one owner's completion slice does +/// 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`' +/// slice spans a whole continuation drain and can hold zone changes no owner +/// allocated a group for, so it must consult this instead. +/// +/// Three further raw-slice collectors exist. [`park_observer_triggers_if_paused`] +/// and [`collect_and_drain_observer_triggers_if_settled`] are not on any path that +/// follows a logical zone-change owner today. The third — `engine_priority`'s +/// exile-return pass — ALREADY follows one (`check_exile_returns` delivers through +/// `zone_pipeline::move_objects_simultaneously_then`, which completes and marks), +/// and it applies the ledger half ONLY, not the queued-context witness. It is safe +/// today solely because that owner marks; if `zone_pipeline` ever stops marking — +/// which is the right call for four of the seven owners, per witness 2 above — that +/// collector is exposed. A future caller that puts any of the three after a +/// `complete_logical_zone_trigger_collection` must route it through here. +pub(crate) fn filter_already_collected_trigger_events_from( + state: &GameState, + events: &[GameEvent], + event_start: usize, + consumed: &[ConsumedTriggerEventOccurrence], +) -> Vec { + let mut queued_zone_change_witnesses: Vec<&GameEvent> = state + .deferred_triggers + .iter() + .flat_map(|context| context.trigger_events.iter()) + .filter(|event| matches!(event, GameEvent::ZoneChanged { .. })) + .collect(); + filter_consumed_trigger_events_from(events, event_start, consumed) + .into_iter() + .filter(|event| { + if !matches!(event, GameEvent::ZoneChanged { .. }) { + return true; + } + match queued_zone_change_witnesses + .iter() + .position(|queued| *queued == event) + { + Some(index) => { + queued_zone_change_witnesses.remove(index); + false + } + None => true, + } + }) + .collect() +} + /// CR 603.2c + CR 510.2: Expand a multi-fire `WheneverEvent` `DamageDone` /// trigger's aggregate `CombatDamageDealtToPlayer` matches into one synthetic /// per-source `DamageDealt` event per matching (source, defending player) diff --git a/crates/engine/src/game/triggers_dedup_regression_tests.rs b/crates/engine/src/game/triggers_dedup_regression_tests.rs index 976fa32255..f0e112d672 100644 --- a/crates/engine/src/game/triggers_dedup_regression_tests.rs +++ b/crates/engine/src/game/triggers_dedup_regression_tests.rs @@ -3646,3 +3646,194 @@ fn order_triggers_apnap_three_players() { ); } } + +// --------------------------------------------------------------------------- +// CR 603.2c: the shared "already collected" authority +// (`filter_already_collected_trigger_events_from`). +// +// These pin the exact semantics of the queued-context witness, which is a BOUND +// and not an occurrence count: `deferred_triggers` holds one context per matching +// observer, so N observers of ONE occurrence contribute N copies of that value. +// --------------------------------------------------------------------------- + +/// A byte-identical `ZoneChanged` builder — `ZoneChangeRecord::test_minimal` is +/// fully deterministic, so two calls with the same arguments compare equal. +fn zone_change_event(object_id: ObjectId) -> GameEvent { + GameEvent::ZoneChanged { + object_id, + from: Some(Zone::Library), + to: Zone::Battlefield, + record: Box::new(ZoneChangeRecord::test_minimal( + object_id, + Some(Zone::Library), + Zone::Battlefield, + )), + } +} + +/// One queued context carrying exactly one copy of `event`, matching the +/// one-witness-copy-per-matched-observer shape +/// `collect_pending_triggers_with_collection` produces. That function builds +/// `PendingTriggerContext::batched(matched.pending, matched.trigger_events)`, but +/// for a non-batched trigger `matched.trigger_events` is the singleton +/// `vec![event.clone()]` — so a `::single` context is the same one-copy shape and +/// is used here because `::batched` is private to `triggers`. +fn queued_context_for(event: GameEvent) -> PendingTriggerContext { + PendingTriggerContext::single(PendingTrigger { + source_id: ObjectId(99), + controller: PlayerId(0), + condition: None, + ability: Box::new(ResolvedAbility::new( + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + Vec::new(), + ObjectId(99), + PlayerId(0), + )), + timestamp: 0, + target_constraints: Vec::new(), + distribute: None, + trigger_event: Some(event), + modal: None, + mode_abilities: Vec::new(), + description: None, + may_trigger_origin: None, + subject_match_count: None, + die_result: None, + provenance: None, + }) +} + +fn zone_change_count(events: &[GameEvent]) -> usize { + events + .iter() + .filter(|event| matches!(event, GameEvent::ZoneChanged { .. })) + .count() +} + +/// U1 — the queued witness is COUNT-LIMITED, not set membership. +/// +/// Two byte-identical `ZoneChanged` in the slice against ONE queued context +/// carrying that value must leave exactly one survivor. Set membership would +/// return 0, and so would a blanket `ZoneChanged` drop; both are wrong, because +/// the second occurrence belongs to no owner. +#[test] +fn owner_collected_filter_consumes_one_witness_per_queued_context() { + let mut state = setup(); + let event = zone_change_event(ObjectId(7)); + let events = vec![event.clone(), event.clone()]; + state.deferred_triggers.push(queued_context_for(event)); + + assert_eq!( + zone_change_count(&events), + 2, + "the slice must really hold two byte-identical ZoneChanged" + ); + assert_eq!( + state.deferred_triggers.len(), + 1, + "exactly one context must be queued" + ); + + let survivors = filter_already_collected_trigger_events_from(&state, &events, 0, &[]); + assert_eq!( + zone_change_count(&survivors), + 1, + "CR 603.2c: one queued witness consumes one copy, not every copy" + ); +} + +/// U2 — the consumed-occurrence ledger alone suppresses, with an empty queue. +/// +/// This is the witness that survives an intervening `drain_deferred_trigger_queue`. +/// Production isolation of this case at the search-delivery park is an open gap; +/// it is evidenced here at the authority layer. +#[test] +fn owner_collected_filter_honors_consumed_ledger_with_empty_queue() { + let state = setup(); + let claimed = zone_change_event(ObjectId(7)); + let other = zone_change_event(ObjectId(8)); + let events = vec![claimed.clone(), other.clone()]; + + assert!( + state.deferred_triggers.is_empty(), + "the queued-context witness must be absent so the ledger is isolated" + ); + + let consumed = vec![ConsumedTriggerEventOccurrence { + event: claimed.clone(), + occurrence: 0, + }]; + let survivors = filter_already_collected_trigger_events_from(&state, &events, 0, &consumed); + assert_eq!( + survivors, + vec![other], + "the ledger-claimed occurrence is removed and the unrelated one survives" + ); +} + +/// U3 — the queued witness never touches a non-`ZoneChanged` event. +#[test] +fn owner_collected_filter_never_drops_non_zone_change_events() { + let mut state = setup(); + let zone_change = zone_change_event(ObjectId(7)); + let life = GameEvent::LifeChanged { + player_id: PlayerId(0), + amount: -1, + }; + let events = vec![zone_change.clone(), life.clone()]; + state + .deferred_triggers + .push(queued_context_for(zone_change)); + + let survivors = filter_already_collected_trigger_events_from(&state, &events, 0, &[]); + assert!( + !survivors.is_empty(), + "the non-zone event must not be swept away with the zone change" + ); + assert_eq!( + survivors, + vec![life], + "only the owner-collected ZoneChanged is removed" + ); +} + +/// U4 — the witness counts CONTEXT COPIES, not occurrences. +/// +/// Two observers of ONE occurrence queue two contexts, each carrying that same +/// single value. A slice holding two byte-identical copies therefore loses BOTH. +/// This is the documented bound in +/// `filter_already_collected_trigger_events_from`'s contract, made executable so +/// no future author can re-assert occurrence-exactness without deliberately +/// updating this row. +#[test] +fn owner_collected_filter_counts_contexts_not_occurrences() { + let mut state = setup(); + let event = zone_change_event(ObjectId(7)); + let events = vec![event.clone(), event.clone()]; + state + .deferred_triggers + .push(queued_context_for(event.clone())); + state.deferred_triggers.push(queued_context_for(event)); + + assert_eq!( + zone_change_count(&events), + 2, + "the slice must really hold two byte-identical ZoneChanged" + ); + assert_eq!( + state.deferred_triggers.len(), + 2, + "two observers of one occurrence queue two contexts" + ); + + let survivors = filter_already_collected_trigger_events_from(&state, &events, 0, &[]); + assert_eq!( + zone_change_count(&survivors), + 0, + "the queued witness is a min(queued_copies, slice_copies) BOUND, and is \ + NOT occurrence-exact" + ); +} diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 85f2cbd97d..2019f172b5 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -13535,10 +13535,22 @@ declare_game_state! { #[serde(default, skip_serializing_if = "Option::is_none")] pub pending_trigger_order: Option, - /// CR 603.3b: PhaseChanged occurrences whose delayed triggers were merged - /// into a simultaneous normal-trigger ordering batch before priority. The - /// generic delayed-trigger pass filters these exact occurrences so the same - /// delayed ability is not dispatched again. Transient engine coordination, + /// CR 603.2c: Event occurrences already collected before a player would + /// receive priority — including the `PhaseChanged` delayed/normal merge, an + /// activation trigger collection (`casting_costs.rs`, + /// `engine_priority::stage_pending_activation_trigger_events`), a dispatched + /// batch's consumed events, and the `ZoneChanged` occurrences claimed by + /// `triggers::mark_logical_zone_events_consumed_before_priority`. Consumed by + /// `triggers::filter_consumed_trigger_events{,_from}` and + /// `triggers::filter_already_collected_trigger_events_from`, so the same + /// occurrence is neither re-collected nor re-dispatched. Ordinals are exact + /// only when the marking owner passed the whole, un-reordered action buffer; + /// one `zone_pipeline.rs` owner passes a sub-slice (rebased ordinals) and the + /// other marks after its buffer has been re-assembled by the batch drain. + /// NOTE: this ledger is ALSO the delayed-trigger input filter + /// (`engine_priority.rs`) — claiming an occurrence hides it from + /// `check_delayed_triggers` too, which is why only three of the seven logical + /// zone-change owners mark (CR 603.7b). Transient engine coordination, /// cleared at action/pipeline boundaries. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub consumed_before_priority_trigger_events: diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index a35edf1dc5..0e3168ed66 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -866,6 +866,7 @@ mod scarab_god_regression; mod scholarship_sponsor; mod screaming_nemesis_combat_damage_multi_trigger; mod screaming_nemesis_life_lock; +mod search_delivery_observer_dedup; mod season_points_budget_modal; mod seasoned_dungeoneer_initiative_room_trigger; mod selenia_vigilance_grant; diff --git a/crates/engine/tests/integration/search_delivery_observer_dedup.rs b/crates/engine/tests/integration/search_delivery_observer_dedup.rs new file mode 100644 index 0000000000..5fdf6ac37c --- /dev/null +++ b/crates/engine/tests/integration/search_delivery_observer_dedup.rs @@ -0,0 +1,637 @@ +//! Regression: a library-search delivery must not double-collect the +//! `GameEvent::ZoneChanged` occurrences a logical zone-change owner already +//! collected. +//! +//! Cracking a fetchland made the fetched land's ETB (Undercity Sewers' surveil) +//! and every landfall observer (Kazandu Mammoth) fire TWICE, while the same land +//! merely *played* fired once. The same occurrence reached +//! `state.deferred_triggers` from two collectors: +//! +//! 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 +//! re-scanned the raw action slice and collected again with no filter but +//! `PhaseChanged`. +//! +//! `triggers.rs`'s per-event CR 603.2 dedup (`registered_this_event`) is a +//! `HashSet` allocated *inside* the event loop, so it cannot see across two +//! collection passes. +//! +//! CR 603.2c: "An ability triggers only once each time its trigger event occurs. +//! However, it can trigger repeatedly if one event contains multiple +//! 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 here passes priority before asserting. +//! `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. + +use engine::ai_support::validated_candidate_actions_for_semantic_owner; +use engine::game::scenario::{GameRunner, GameScenario, P0}; +use engine::game::scenario_db::GameScenarioDbExt; +use engine::types::actions::GameAction; +use engine::types::game_state::{GameState, WaitingFor}; +use engine::types::identifiers::ObjectId; +use engine::types::mana::{ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +use crate::support::shared_card_db; + +/// Effective P/T (post-layers), read off the materialized `GameObject` fields +/// the layer pipeline writes during `apply`. +fn power_toughness(runner: &GameRunner, id: ObjectId) -> (i32, i32) { + let obj = runner + .state() + .objects + .get(&id) + .expect("object must still be present"); + (obj.power.unwrap_or(0), obj.toughness.unwrap_or(0)) +} + +fn add_mana(runner: &mut GameRunner, green: usize, black: usize, colorless: usize) { + let pool = &mut runner.state_mut().players[0].mana_pool; + for _ in 0..green { + pool.add(ManaUnit::new(ManaType::Green, ObjectId(0), false, vec![])); + } + for _ in 0..black { + pool.add(ManaUnit::new(ManaType::Black, ObjectId(0), false, vec![])); + } + for _ in 0..colorless { + pool.add(ManaUnit::new( + ManaType::Colorless, + ObjectId(0), + false, + vec![], + )); + } +} + +/// The validated activation index for Misty Rainforest's real printed ability, +/// derived the same way `prospective_fetchland_mana.rs` derives it. +fn misty_ability_index(state: &GameState, misty: ObjectId) -> usize { + validated_candidate_actions_for_semantic_owner(state, P0) + .into_iter() + .find_map(|candidate| match candidate.action { + GameAction::ActivateAbility { + source_id, + ability_index, + .. + } if source_id == misty => Some(ability_index), + _ => None, + }) + .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) { + assert!( + matches!(runner.state().waiting_for, WaitingFor::Priority { .. }), + "the parked action must settle back to Priority, 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" + ); + assert!( + !runner.state().deferred_triggers.is_empty(), + "the delivery's observers must actually be sitting in the parked queue" + ); +} + +/// 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) +// --------------------------------------------------------------------------- + +#[test] +fn fetchland_crack_fires_landfall_observer_once() { + let db = shared_card_db().expect("integration card fixture must load"); + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + 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 + // surveil prompt entangles the assertion. + let forest = scenario.add_real_card(P0, "Forest", 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); + + assert_eq!( + power_toughness(&runner, mammoth), + (3, 3), + "Kazandu Mammoth's printed body is 3/3 before any landfall" + ); + + runner + .activate(misty, ability_index) + .search_first_legal() + .resolve(); + + 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); + runner.advance_until_stack_empty(); + + // 3/3 base, +2/+2 exactly once. 7/7 is the double collection. + assert_eq!( + power_toughness(&runner, mammoth), + (5, 5), + "CR 603.2c: one land entering is ONE occurrence, so landfall fires once" + ); +} + +// --------------------------------------------------------------------------- +// H2 — reported symptom 2: the fetched land's own ETB fires ONCE (park site A) +// --------------------------------------------------------------------------- + +#[test] +fn fetchland_fetched_land_etb_trigger_fires_once() { + let db = shared_card_db().expect("integration card fixture must load"); + let mut scenario = GameScenario::new(); + 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. + 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) + .search_first_legal() + .resolve(); + + 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); + + // Count STACK OBJECTS, not prompts, and stop here — deliberately before the + // surveil prompt, which `advance_until_stack_empty` does not model. + assert!( + !runner.state().stack.is_empty(), + "the priority pass must have run the deferred drain" + ); + let surveil_copies = runner + .state() + .stack + .iter() + .filter(|entry| entry.source_id == sewers) + .count(); + assert_eq!( + surveil_copies, 1, + "CR 603.2c: the fetched land's ETB must reach the stack exactly once" + ); +} + +// --------------------------------------------------------------------------- +// H3 — control: a land PLAYED (one collector, never two) still fires once +// --------------------------------------------------------------------------- + +#[test] +fn played_land_fires_landfall_observer_once() { + let db = shared_card_db().expect("integration card fixture must load"); + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let mammoth = scenario.add_real_card(P0, "Kazandu Mammoth", Zone::Battlefield, db); + let forest = scenario.add_real_card(P0, "Forest", Zone::Hand, db); + + let mut runner = scenario.build(); + engine::game::rehydrate_game_from_card_db(runner.state_mut(), db); + + let card_id = runner.state().objects[&forest].card_id; + runner + .act(GameAction::PlayLand { + object_id: forest, + card_id, + }) + .expect("playing a land in the precombat main phase must succeed"); + + assert_eq!( + runner.state().objects[&forest].zone, + Zone::Battlefield, + "the played land must have entered the battlefield" + ); + // `handle_play_land` calls `zone_pipeline::deliver` directly and allocates + // no logical zone-change group, so the landfall trigger reaches the stack + // in-action — no priority pass is needed here. + runner.advance_until_stack_empty(); + + assert_eq!( + power_toughness(&runner, mammoth), + (5, 5), + "a played land has exactly one collector and must stay at one firing" + ); +} + +// --------------------------------------------------------------------------- +// H4 — pause/resume through a search delivery still fires landfall once +// --------------------------------------------------------------------------- + +#[test] +fn fetch_pauses_on_optional_replacement_then_fires_landfall_once() { + let db = shared_card_db().expect("integration card fixture must load"); + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let misty = scenario.add_real_card(P0, "Misty Rainforest", Zone::Battlefield, db); + let mammoth = scenario.add_real_card(P0, "Kazandu Mammoth", Zone::Battlefield, db); + // Breeding Pool's "As this land enters, you may pay 2 life" surfaces a real + // `ReplacementChoice` mid-delivery, so the owner pauses and resumes. + let pool = scenario.add_real_card(P0, "Breeding Pool", 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); + + let outcome = runner + .activate(misty, ability_index) + .search_first_legal() + .resolve(); + + // `AbilityActivation::resolve` hard-codes `replacement_choice: None`, so the + // driver breaks and leaves the prompt live for us to answer by hand. + assert!( + matches!( + outcome.final_waiting_for(), + WaitingFor::ReplacementChoice { .. } + ), + "Breeding Pool's MayCost must surface a real replacement pause, got {:?}", + outcome.final_waiting_for() + ); + // For an optional replacement the candidate vec is exactly + // `[accept, decline]`; index 1 declines (enters tapped, no life paid). + runner + .act(GameAction::ChooseReplacement { index: 1 }) + .expect("declining the optional replacement must be accepted"); + + assert_eq!( + runner.state().objects[&pool].zone, + Zone::Battlefield, + "the paused delivery must still have completed" + ); + // MEASURED, not assumed: this row is NOT a park path. The replacement pause + // resumes through `effects/mod.rs`'s parked-`ChangeZone` drain, which drains + // `deferred_triggers` and then collects + dispatches the resumed slice + // INLINE — so the landfall observer is already on the stack here and the + // parked queue is empty. That is the structural difference the plan's §4d + // calls out, and it is why no priority pass belongs in this sequence. + assert!( + runner.state().deferred_triggers.is_empty(), + "the resumed-ChangeZone drain dispatches inline; nothing may remain parked" + ); + let landfall_copies = runner + .state() + .stack + .iter() + .filter(|entry| entry.source_id == mammoth) + .count(); + assert_eq!( + landfall_copies, 1, + "the landfall observer must reach the stack exactly once on the \ + pause/resume route" + ); + + runner.advance_until_stack_empty(); + + assert_eq!( + power_toughness(&runner, mammoth), + (5, 5), + "a paused-then-resumed delivery must still fire landfall exactly once" + ); +} + +// --------------------------------------------------------------------------- +// H5 — park site B: the single-basic partition fast path +// --------------------------------------------------------------------------- + +#[test] +fn cultivate_fast_path_fires_landfall_once() { + let db = shared_card_db().expect("integration card fixture must load"); + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let mammoth = scenario.add_real_card(P0, "Kazandu Mammoth", Zone::Battlefield, db); + let cultivate = scenario.add_real_card(P0, "Cultivate", Zone::Hand, db); + let forest = scenario.add_real_card(P0, "Forest", Zone::Library, db); + // A nonbasic so exactly one basic is findable and the fast path is taken. + scenario.add_real_card(P0, "Mishra's Factory", Zone::Library, db); + + let mut runner = scenario.build(); + 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(); + + 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); + runner.advance_until_stack_empty(); + + assert_eq!( + power_toughness(&runner, mammoth), + (5, 5), + "park site B must fire the landfall observer exactly once" + ); +} + +// --------------------------------------------------------------------------- +// H6 — park site C: the explicit `SearchPartitionChoice` route +// --------------------------------------------------------------------------- + +#[test] +fn cultivate_partition_fires_landfall_once() { + let db = shared_card_db().expect("integration card fixture must load"); + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let mammoth = scenario.add_real_card(P0, "Kazandu Mammoth", Zone::Battlefield, db); + let cultivate = scenario.add_real_card(P0, "Cultivate", Zone::Hand, db); + let forest = scenario.add_real_card(P0, "Forest", Zone::Library, db); + let mountain = scenario.add_real_card(P0, "Mountain", Zone::Library, db); + + let mut runner = scenario.build(); + engine::game::rehydrate_game_from_card_db(runner.state_mut(), db); + add_mana(&mut runner, 1, 0, 2); + + // `search_first_legal` submits both basics at the `SearchChoice`; the + // partition prompt then parks for an explicit pick. + runner.cast(cultivate).search_first_legal().resolve(); + assert!( + matches!( + runner.state().waiting_for, + WaitingFor::SearchPartitionChoice { .. } + ), + "two findable basics must park a SearchPartitionChoice, got {:?}", + runner.state().waiting_for + ); + runner + .act(GameAction::SelectCards { + cards: vec![forest], + }) + .expect("the partition pick must resolve"); + + assert_eq!( + runner.state().objects[&forest].zone, + Zone::Battlefield, + "the primary basic must reach the battlefield" + ); + assert_eq!( + runner.state().objects[&mountain].zone, + 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); + runner.advance_until_stack_empty(); + + assert_eq!( + power_toughness(&runner, mammoth), + (5, 5), + "park site C must fire the landfall observer exactly once" + ); +} + +// --------------------------------------------------------------------------- +// N1 — a non-battlefield search destination is delivered and swallows nothing +// --------------------------------------------------------------------------- + +#[test] +fn search_to_hand_delivers_and_fires_no_landfall() { + let db = shared_card_db().expect("integration card fixture must load"); + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let mammoth = scenario.add_real_card(P0, "Kazandu Mammoth", Zone::Battlefield, db); + let journey = scenario.add_real_card(P0, "Journey of Discovery", Zone::Hand, db); + let forest = scenario.add_real_card(P0, "Forest", Zone::Library, db); + let mountain = scenario.add_real_card(P0, "Mountain", Zone::Library, db); + + let mut runner = scenario.build(); + engine::game::rehydrate_game_from_card_db(runner.state_mut(), db); + add_mana(&mut runner, 1, 0, 2); + + // Journey of Discovery is modal + entwine; mode 0 is the + // `ChangeZone { Library -> Hand }` half. + runner + .cast(journey) + .modes(&[0]) + .search_first_legal() + .resolve(); + + // The positive reach-guard: the search really delivered. Without it the + // (3,3) assertion below could pass on a fail-to-find. + assert_eq!( + runner.state().objects[&forest].zone, + Zone::Hand, + "mode 0 must put the found basics into HAND" + ); + assert_eq!( + runner.state().objects[&mountain].zone, + Zone::Hand, + "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), + "no land ENTERED, so landfall must not fire at all" + ); +} + +// --------------------------------------------------------------------------- +// N2 — CR 603.2c sentence 2: two lands in ONE logical group fire landfall TWICE +// --------------------------------------------------------------------------- + +#[test] +fn two_land_search_delivery_fires_landfall_twice() { + let db = shared_card_db().expect("integration card fixture must load"); + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let mammoth = scenario.add_real_card(P0, "Kazandu Mammoth", Zone::Battlefield, db); + // Harrow is an Instant with "sacrifice a land" as an additional cost. + let harrow = scenario.add_real_card(P0, "Harrow", Zone::Hand, db); + let spare = scenario.add_real_card(P0, "Mountain", Zone::Battlefield, db); + let forest = scenario.add_real_card(P0, "Forest", Zone::Library, db); + let island = scenario.add_real_card(P0, "Island", Zone::Library, db); + + let mut runner = scenario.build(); + engine::game::rehydrate_game_from_card_db(runner.state_mut(), db); + add_mana(&mut runner, 1, 0, 2); + + runner + .cast(harrow) + .sacrifice_with(&[spare]) + .search_first_legal() + .resolve(); + + assert_eq!( + runner.state().objects[&forest].zone, + Zone::Battlefield, + "both basics must have been delivered" + ); + assert_eq!( + runner.state().objects[&island].zone, + Zone::Battlefield, + "both basics must have been delivered" + ); + assert_observers_were_parked(&runner); + + pass_priority_to_reach_the_drain(&mut runner); + // `advance_until_stack_empty` drains the CR 603.3b `OrderTriggers` prompt + // internally; both parked triggers are pumps with no further prompt. + runner.advance_until_stack_empty(); + + assert_eq!( + power_toughness(&runner, mammoth), + (7, 7), + "CR 603.2c sentence 2: TWO lands entering is TWO occurrences, so the \ + fix must not become a blanket zone-change suppression" + ); +} + +// --------------------------------------------------------------------------- +// N3 — CR 603.7b fence: the leaves-the-battlefield delayed family must survive +// a targeted `Effect::ChangeZone` +// --------------------------------------------------------------------------- + +#[test] +fn aura_exiled_via_targeted_change_zone_fires_delayed_sacrifice() { + let db = shared_card_db().expect("integration card fixture must load"); + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let creature = scenario.add_real_card(P0, "Grizzly Bears", Zone::Graveyard, db); + let aura = scenario.add_real_card(P0, "Animate Dead", Zone::Hand, db); + let exiler = scenario.add_real_card(P0, "Introduction to Annihilation", Zone::Hand, db); + // Introduction to Annihilation's `SequentialSibling` is "Its controller + // draws a card". Without a library P0 decks out (CR 704.5b) and the game + // ends before the delayed sacrifice can be observed. + for _ in 0..5 { + scenario.add_real_card(P0, "Plains", Zone::Library, db); + } + + let mut runner = scenario.build(); + engine::game::rehydrate_game_from_card_db(runner.state_mut(), db); + add_mana(&mut runner, 0, 1, 1); + + runner.cast(aura).target_object(creature).resolve(); + runner.advance_until_stack_empty(); + assert_eq!( + runner.state().objects[&creature].zone, + Zone::Battlefield, + "Animate Dead's ETB must have reanimated the creature, which is what \ + creates the WhenLeavesPlayFiltered delayed trigger" + ); + + add_mana(&mut runner, 0, 0, 5); + // `Effect::ChangeZone { destination: Exile }` on a targeted nonland + // permanent — the targeted `change_zone::resolve` path that allocates a + // logical zone-change group and completes it without a paired `mark_`. + runner.cast(exiler).target_object(aura).resolve(); + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().objects[&aura].zone, + Zone::Exile, + "the targeted Effect::ChangeZone must actually have exiled the Aura" + ); + assert_eq!( + runner.state().objects[&creature].zone, + Zone::Graveyard, + "CR 603.7b: a delayed triggered ability triggers the next time its \ + trigger event occurs — claiming the Aura's ZoneChanged in the consumed \ + ledger would hide it from check_delayed_triggers and the reanimated \ + creature would never be sacrificed" + ); +} + +// --------------------------------------------------------------------------- +// N4 — fail-to-find: an empty park slice still short-circuits cleanly +// --------------------------------------------------------------------------- + +#[test] +fn fetch_with_no_legal_target_parks_nothing() { + let db = shared_card_db().expect("integration card fixture must load"); + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let misty = scenario.add_real_card(P0, "Misty Rainforest", Zone::Battlefield, db); + let mammoth = scenario.add_real_card(P0, "Kazandu Mammoth", Zone::Battlefield, db); + // Nothing Misty's "Forest or Island card" filter can find. + scenario.add_real_card(P0, "Grizzly Bears", 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); + let life_before = runner.state().players[0].life; + + runner + .activate(misty, ability_index) + .search_first_legal() + .resolve(); + + // Positive reach-guards: the activation cost `{T}, Pay 1 life, Sacrifice + // this land` was really paid, so the ability really ran. (A "waiting_for is + // not SearchChoice" guard would be satisfied by a never-accepted activation + // and does not discriminate.) + assert_eq!( + runner.state().objects[&misty].zone, + Zone::Graveyard, + "the sacrifice half of the activation cost must have been paid" + ); + assert_eq!( + runner.state().players[0].life, + life_before - 1, + "the pay-1-life half of the activation cost must have been paid" + ); + + assert!( + runner.state().deferred_triggers.is_empty(), + "a fail-to-find delivers nothing, so nothing may be parked" + ); + assert_eq!( + power_toughness(&runner, mammoth), + (3, 3), + "no land entered, so landfall must not fire" + ); +}