diff --git a/crates/engine/src/game/public_state.rs b/crates/engine/src/game/public_state.rs index ae79e59185..27cc59babc 100644 --- a/crates/engine/src/game/public_state.rs +++ b/crates/engine/src/game/public_state.rs @@ -3,6 +3,7 @@ use crate::types::card_type::CoreType; use crate::types::events::GameEvent; use crate::types::game_state::{GameState, PublicStateDirty, WaitingFor}; use crate::types::identifiers::ObjectId; +use crate::types::phase::Phase; use crate::types::player::PlayerId; use crate::types::zones::Zone; @@ -37,8 +38,77 @@ pub fn finalize_public_state(state: &mut GameState) { finalize_display_state(state); } +/// CR 703.1: the combat step whose TURN-BASED ACTION this prompt is the player's +/// answer to, or `None` when the prompt answers to some other authority. +/// ("Turn-based actions are game actions that happen automatically when certain +/// steps or phases begin, or when each step and phase ends." Each bound step's +/// own rule is cited on its arm below.) +/// +/// The bound set is exactly the combat turn-based actions of CR 508.1, CR 509.1, +/// CR 510.1c and CR 510.1d. It is NOT a closed enumeration of step-associated +/// `WaitingFor` variants, and the wildcard is not a shortcut — several prompts +/// are live during a combat step under a DIFFERENT authority and must stay +/// unbound, or a legal state would trip the assertion below: +/// +/// * `CombatTaxPayment { context: Attacking }`, `ExertChoice`, `EnlistChoice` — +/// CR 508.1g optional attack costs (CR 701.43d exert, CR 702.154b enlist). +/// These are a sub-step of the declaration with their own re-entry +/// (`engine_combat::handle_pay_combat_tax`), not the declaration itself. +/// * `MeldAttackTargetChoice`, `EntryAttackTargetChoice` — CR 508.4 "put onto +/// the battlefield attacking". This is a RESOLUTION-time choice: CR 508.4d +/// explicitly contemplates it during the declare blockers, combat damage, and +/// end of combat steps, and `combat::choose_entry_attack_target_or_enter` is +/// reached from effect resolution in any phase. +/// +/// Adding a variant to this match is a rules claim: it asserts the prompt can +/// only ever be installed during that one step. Verify against the CR before +/// binding anything new. +fn step_bound_phase(waiting_for: &WaitingFor) -> Option { + match waiting_for { + // CR 508.1: the active player declares attackers as a turn-based action. + WaitingFor::DeclareAttackers { .. } => Some(Phase::DeclareAttackers), + // CR 509.1: the defending player declares blockers as a turn-based action. + WaitingFor::DeclareBlockers { .. } => Some(Phase::DeclareBlockers), + // CR 510.1c: an attacker blocked by two or more creatures divides its + // combat damage as its controller chooses, during the combat damage step. + WaitingFor::AssignCombatDamage { .. } => Some(Phase::CombatDamage), + // CR 510.1d (+ CR 702.22k banding): a blocker blocking two or more + // creatures divides its combat damage among them. Same step, same + // turn-based action pair as CR 510.1c above. + WaitingFor::AssignBlockerDamage { .. } => Some(Phase::CombatDamage), + _ => None, + } +} + pub fn sync_waiting_for(state: &mut GameState, waiting_for: &WaitingFor) { state.waiting_for = waiting_for.clone(); + // CR 703.1: a turn-based action happens automatically when its own step + // begins or ends, so a prompt that is a player's answer to one is answerable + // only during that step. Installing one outside that step produces a + // decision no seat can legally submit and the game cannot leave. + // + // `debug_assert!` rather than production validation because the CR 603.3 + // drain in `turns::process_phase_triggers` is meant to have settled the + // queue before any boundary is reached, so a hit here is an engine bug to be + // fixed at its producer, not a runtime condition to handle. Read that as an + // intent this check enforces, NOT as a proof that no producer can construct + // the pairing — the paragraph below says outright that the producer + // population was never enumerated, which is precisely why the check has to + // be executable rather than argued. + // + // This is an ACTION-BOUNDARY check, not a producer check: a large number of + // sites in `crates/engine/src/` assign `state.waiting_for` directly, and this + // function is a boundary re-synchronizer with a small number of call sites, + // not the single install authority. A pairing written and then overwritten + // before the next boundary is invisible here, and a panic's backtrace names + // the boundary, not the producer. It is still the cheapest place to catch a + // pairing that actually reaches a client. + debug_assert!( + step_bound_phase(&state.waiting_for).is_none_or(|required| required == state.phase), + "waiting_for {:?} is bound to a combat step but the game is in {:?}", + state.waiting_for, + state.phase, + ); normalize_legacy_attach_waiting_for(state); sync_priority_player_from_waiting_for(state); } @@ -508,8 +578,159 @@ pub fn clear_public_state_dirty(state: &mut GameState) { #[cfg(test)] mod tests { use super::*; - use crate::types::game_state::{CastOfferKind, PendingContinuation}; + use crate::game::combat::AttackTarget; + use crate::types::game_state::{ + CastOfferKind, CombatTaxContext, CombatTaxPending, MeldSelection, PendingContinuation, + }; use crate::types::identifiers::ObjectId; + use crate::types::mana::ManaCost; + + /// CR 703.1: every combat turn-based-action prompt maps to the one step it + /// can legally be answered in, and every prompt that answers to a DIFFERENT + /// authority maps to `None`. + /// + /// The `None` rows are the load-bearing half. Each is a prompt that can + /// legitimately be live during (or adjacent to) a combat step under an + /// authority other than the declaration itself, so binding it would make + /// `sync_waiting_for`'s `debug_assert!` fire on a legal state: + /// + /// * `CombatTaxPayment { context: Attacking }`, `ExertChoice`, `EnlistChoice` + /// — CR 508.1g optional attack costs (CR 701.43d exert, CR 702.154b + /// enlist): a sub-step of the declaration, not the declaration. + /// * `MeldAttackTargetChoice`, `EntryAttackTargetChoice` — CR 508.4 "put + /// onto the battlefield attacking", a resolution-time choice that CR 508.4d + /// explicitly contemplates during declare blockers, combat damage, and end + /// of combat. + /// * `Priority` (CR 117.3a) and `OrderTriggers` (CR 603.3b) are not + /// step-bound at all. + #[test] + fn step_bound_phase_maps_combat_turn_based_prompts() { + // CR 508.1: the active player declares attackers as a turn-based action. + assert_eq!( + step_bound_phase(&WaitingFor::DeclareAttackers { + player: PlayerId(0), + valid_attacker_ids: Vec::new(), + valid_attack_targets: Vec::new(), + valid_attack_targets_by_attacker: None, + attacker_constraints: Default::default(), + }), + Some(Phase::DeclareAttackers), + ); + // CR 509.1: the defending player declares blockers as a turn-based action. + assert_eq!( + step_bound_phase(&WaitingFor::DeclareBlockers { + player: PlayerId(1), + valid_blocker_ids: Vec::new(), + valid_block_targets: Default::default(), + block_requirements: Default::default(), + blocker_constraints: Default::default(), + }), + Some(Phase::DeclareBlockers), + ); + // CR 510.1c: an attacker blocked by 2+ creatures divides its damage. + assert_eq!( + step_bound_phase(&WaitingFor::AssignCombatDamage { + player: PlayerId(0), + attacker_id: ObjectId(1), + total_damage: 2, + blockers: Vec::new(), + assignment_modes: Vec::new(), + trample: None, + defending_player: PlayerId(1), + attack_target: AttackTarget::Player(PlayerId(1)), + pw_loyalty: None, + pw_controller: None, + }), + Some(Phase::CombatDamage), + ); + // CR 510.1d (+ CR 702.22k banding): a blocker blocking 2+ creatures. + assert_eq!( + step_bound_phase(&WaitingFor::AssignBlockerDamage { + player: PlayerId(1), + blocker_id: ObjectId(2), + total_damage: 2, + attackers: Vec::new(), + }), + Some(Phase::CombatDamage), + ); + + // --- Deliberately unbound siblings, each with its CR reason above. --- + + // CR 117.3a: priority is not bound to any step. + assert_eq!( + step_bound_phase(&WaitingFor::Priority { + player: PlayerId(0) + }), + None, + ); + // CR 603.3b: trigger ordering is not bound to any step. + assert_eq!( + step_bound_phase(&WaitingFor::OrderTriggers { + player: PlayerId(0), + triggers: Vec::new(), + }), + None, + ); + // CR 508.1g: an optional attack cost is a sub-step with its own re-entry. + assert_eq!( + step_bound_phase(&WaitingFor::CombatTaxPayment { + player: PlayerId(0), + context: CombatTaxContext::Attacking, + total_cost: ManaCost::NoCost, + per_creature: Vec::new(), + pending: CombatTaxPending::Attack { + attacks: Vec::new(), + bands: Vec::new(), + }, + }), + None, + ); + // CR 701.43d: "you may exert as it attacks" is an optional attack cost. + assert_eq!( + step_bound_phase(&WaitingFor::ExertChoice { + player: PlayerId(0), + attacker: ObjectId(1), + remaining: Vec::new(), + }), + None, + ); + // CR 702.154b: enlist's static ability is an optional cost to attack. + assert_eq!( + step_bound_phase(&WaitingFor::EnlistChoice { + player: PlayerId(0), + attacker: ObjectId(1), + eligible: Vec::new(), + remaining: Vec::new(), + }), + None, + ); + // CR 508.4 / CR 508.4d: resolution-time "enters attacking" choices are + // legal during declare blockers, combat damage, and end of combat. + assert_eq!( + step_bound_phase(&WaitingFor::MeldAttackTargetChoice { + player: PlayerId(0), + context: MeldSelection { + source_id: ObjectId(1), + partner_id: ObjectId(2), + controller: PlayerId(0), + expected_source: String::new(), + expected_partner: String::new(), + result: String::new(), + entry: Default::default(), + }, + valid_targets: Vec::new(), + }), + None, + ); + assert_eq!( + step_bound_phase(&WaitingFor::EntryAttackTargetChoice { + player: PlayerId(0), + object_id: ObjectId(1), + valid_targets: Vec::new(), + }), + None, + ); + } #[test] fn sync_waiting_for_updates_priority_player_for_resolution_choices() { diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index a7ba7da9dd..09bd51b6b4 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -10883,10 +10883,28 @@ fn collect_pending_and_delayed_triggers_for_batch( } } +/// CR 603.3d: the prompt currently OWED by the trigger machinery, or `None`. +/// +/// `active_trigger_prompt` echoes `state.waiting_for` only while a trigger is +/// genuinely mid-construction (`pending_trigger`), because in that case the +/// live prompt IS that trigger's own target/mode/distribute choice. A merely +/// PARKED `deferred_triggers` batch owns no prompt: CR 603.3 says such +/// abilities are put on the stack at the next priority point, which +/// `turns::process_phase_triggers` now does via `drain_deferred_trigger_queue`. +/// Echoing on a parked queue instead re-asserted an unrelated prompt — the seam +/// that pinned a CR 508.1 `DeclareAttackers` prompt across the CR 508.8 advance +/// to `Phase::EndCombat`, and made the state absorbing (a non-`Priority` return +/// skips both `apply_action`'s post-action pipeline gate and that pipeline's own +/// `Priority`-gated deferred drain, so the queue could never leave). +/// +/// A queue that legitimately cannot drain is still reported through `fired`, +/// which keeps its own `!deferred_triggers.is_empty()` disjunct at both call +/// sites, and — where the prompt genuinely changed — through +/// `inline_resolution_prompt` below. fn current_trigger_prompt(state: &GameState, waiting_before: &WaitingFor) -> Option { let order_triggers_prompt = build_next_order_triggers_prompt_public(state); let active_trigger_prompt = (order_triggers_prompt.is_none() - && (state.pending_trigger.is_some() || !state.deferred_triggers.is_empty())) + && state.pending_trigger.is_some()) .then(|| state.waiting_for.clone()); let inline_resolution_prompt = (order_triggers_prompt.is_none() && active_trigger_prompt.is_none() diff --git a/crates/engine/src/game/turns.rs b/crates/engine/src/game/turns.rs index e4dd2aed51..8ed8f04d1e 100644 --- a/crates/engine/src/game/turns.rs +++ b/crates/engine/src/game/turns.rs @@ -2190,6 +2190,11 @@ fn clear_cleanup_damage(state: &mut GameState, events: &mut Vec) { /// Execute the cleanup step. Returns `Some(WaitingFor)` if the player must /// choose which cards to discard down to maximum hand size, or `None` if /// cleanup completes immediately. +/// +/// CR 514.3a: a `Some` return additionally means "triggered abilities were put +/// on the stack and the active player must receive priority before another +/// cleanup step begins" — either the control-reversion delayed triggers below, +/// or a parked `deferred_triggers` batch settled at the tail of this function. pub fn execute_cleanup(state: &mut GameState, events: &mut Vec) -> Option { // CR 508.6 + CR 514.2: Snapshot this turn's attacks so "attacked you during // their last turn" (Avenge / O-Kagachi / Weathered Sentinels) can query each @@ -2428,6 +2433,114 @@ pub fn execute_cleanup(state: &mut GameState, events: &mut Vec) -> Op } } + // CR 514.3a: "At this point, the game checks to see if any state-based + // actions would be performed and/or ANY TRIGGERED ABILITIES ARE WAITING TO + // BE PUT ONTO THE STACK (including those that trigger 'at the beginning of + // the next cleanup step'). If so, those state-based actions are performed, + // THEN those triggered abilities are put on the stack, then the active + // player gets priority. ... Once the stack is empty and all players pass in + // succession, another cleanup step begins." + // + // SCOPE — this block implements ONLY the triggered-ability half of CR 514.3a. + // The rule orders an SBA pass FIRST ("those state-based actions are + // performed, then those triggered abilities are put on the stack"); this + // block performs no SBA pass. SBAs are instead performed at the priority + // boundary this block routes to, by `sba::check_state_based_actions` inside + // `engine_priority::run_post_action_pipeline` (`engine_priority.rs:177`), + // i.e. AFTER the abilities are stacked rather than before. Whether cleanup + // should perform a full CR 704 pass at CR 514.3a's exact instant is a + // separate question, deliberately not answered here. + // + // REACHABILITY — read this block as defence in depth, NOT as documentation + // of a live path. No production route through the public API was FOUND that + // reaches it with a non-empty queue. The two structural reasons, which need + // no census: the drain at the end of `process_phase_triggers` sits in that + // function's shared body rather than in any one arm, so it runs for whatever + // phase/step arm reaches it; and the pipeline's drain likewise sits in + // `engine_priority::run_post_action_pipeline_from`'s body, so it runs for + // settlements routed through it. The one cleanup path that pauses and + // resumes — discard to maximum hand size — never re-enters this function + // (its resume runs `finish_cleanup_discard` and then the pipeline), so its + // CR 514.3a settlement comes from the pipeline, not from here. + // + // The remaining input, "who parks a batch during cleanup", rests on an + // identifier search for `collect_triggers_into_deferred` that returns no hit + // in this file. That instrument cannot see a macro-generated or + // trait-dispatched call, so treat it as "none found", not "none exists" — + // which is the second reason the block stays. + // + // It is kept regardless: CR 514.3a is a real obligation at this exact + // instant, the block is inert when the queue is empty (the gate refuses, it + // returns `None`, and cleanup advances unchanged), and it is the local + // guarantee that a future producer which parks a batch during cleanup + // cannot carry it across the Cleanup -> Untap wrap. If you are looking for + // the code that settles a parked batch in practice, it is the step-boundary + // drain in `process_phase_triggers` or the post-action pipeline. + // + // A parked `deferred_triggers` batch IS "a triggered ability waiting to be + // put onto the stack", so it must settle HERE, during cleanup — not survive + // `advance_phase_once`'s Cleanup -> Untap wrap (which runs `start_next_turn`) + // and land on the next turn's upkeep. CR 514.3 (the parent rule) says no + // player normally receives priority during cleanup and then states that this + // is the exception; returning `Some(Priority { .. })` is that exception, not + // a violation of it. + // + // POPULATION — this checks `deferred_triggers` only, unlike the CR 603.3b + + // issue #1350 guard in `auto_advance_once`'s `Phase::CombatDamage` arm + // (grep `issue #1350` in this file), which checks + // `!deferred_triggers.is_empty() || pending_trigger.is_some()`. The + // `pending_trigger` disjunct is deliberately excluded, not overlooked: a live + // `pending_trigger` means a trigger is mid-construction and owns the open + // prompt, so `current_trigger_prompt` still echoes for it (that disjunct is + // retained by the CR 603.3d narrowing) and the game would be sitting at that + // trigger's own target/mode choice rather than passing priority into cleanup. + // If that assumption is ever falsified, the fix is to widen this condition to + // match that `Phase::CombatDamage` guard, not to add a second block. + // + // The second half of CR 514.3a — "another cleanup step begins" — is ALREADY + // implemented: `priority.rs:79-90` re-enters `auto_advance` when all players + // pass with an empty stack at `Phase::Cleanup`, which re-runs this function. + // On that repeat the queue is empty, `can_drain_deferred_triggers` refuses, + // the stack does not grow, and cleanup advances normally. + // + // TERMINATION, in two parts. (1) No in-process loop is possible: this block + // returns only `Some(..)` — which the `Phase::Cleanup` arm converts to + // `AutoAdvanceStep::Waiting`, exiting `auto_advance`'s UNBOUNDED loop (no + // iteration cap) — or `None`, which advances the phase. It can never yield + // `Continue`, so it cannot spin that loop. Part (1) is what carries + // termination; it is sufficient on its own. (2) An unbounded REPEAT would + // still not be a freeze: each repeat cleanup step costs a real + // `PassPriority` from every living seat before `priority.rs:79-90` re-enters, + // so the game remains answerable throughout and any seat may act instead of + // passing. No CR draw rule is claimed for that case — CR 104.4b's draw is + // for loops of MANDATORY actions and expressly excludes loops containing an + // optional action, so it is not the authority here. + // NOTE: this is NOT the argument `priority.rs:86-89` makes for the + // control-reversion case — that one is monotone-decreasing (its one-shot TCE + // is already pruned, so no new event re-fires). Nothing analogous holds for + // `deferred_triggers`, which can in principle be re-parked by a resolving + // ability; the two parts above are why termination holds anyway. + // + // Structurally identical to the CR 514.3a control-reversion block above + // (`check_delayed_triggers` + stack-growth pause); same authority, same + // shape, different waiting population. + // + // `drain_deferred_trigger_queue` (the restrictive member, gate + // `can_drain_deferred_triggers(state, /*allow_spell_on_stack=*/false)`) is + // the right authority: cleanup is reached only after the end step's stack + // emptied (CR 500.2), so the guard passes on every normal entry, and a + // refusal is inert — the stack does not grow, this returns `None`, and + // cleanup advances exactly as it does today. + let stack_before = state.stack.len(); + if let Some(prompt) = super::triggers::drain_deferred_trigger_queue(state, events) { + return Some(prompt); + } + if state.stack.len() > stack_before { + return Some(WaitingFor::Priority { + player: state.active_player, + }); + } + None } @@ -2672,6 +2785,20 @@ fn add_lore_counters_to_sagas(state: &mut GameState, events: &mut Vec /// - an active trigger prompt (`TriggerTargetSelection`, etc.) when /// `pending_trigger` / `deferred_triggers` still hold unresolved work (CR /// 603.3). The caller MUST surface this prompt instead of granting priority. +/// +/// CR 117.5 + CR 603.3: this function is a stack-placement point. Abilities +/// waiting in `deferred_triggers` are put on the stack here, not merely +/// reported, so a parked batch cannot survive a phase or step boundary. +/// Individual arms must NOT re-derive their own deferred-queue guards; the +/// `Phase::CombatDamage` guard in `auto_advance_once` (issue #1350) predates +/// this and is retained because it also covers `pending_trigger`. +/// +/// The CLEANUP step does not reach this function. On the non-discard path, +/// CR 514.3a is handled by the deferred-trigger block in `execute_cleanup`. When +/// cleanup instead pauses for discard to maximum hand size, the resume runs +/// `finish_cleanup_discard` and then the post-action pipeline — it does not +/// re-enter `execute_cleanup` — so on that path CR 514.3a settles at the +/// pipeline's drain rather than in `execute_cleanup`. fn process_phase_triggers( state: &mut GameState, events: &[GameEvent], @@ -2694,7 +2821,62 @@ fn process_phase_triggers( &delayed_events, events_out, ); - (outcome.fired, outcome.prompt) + // CR 117.3a + CR 117.5 + CR 603.3: reaching this point in a phase/step arm + // IS the moment "a player would receive priority", so abilities already + // waiting in `deferred_triggers` must be PUT ON THE STACK here, not merely + // reported. This mirrors what `engine_priority::run_post_action_pipeline` + // already does at the sibling (`WaitingFor::Priority`) boundary, using the + // same authority and the same gate. + // + // This is load-bearing, not belt-and-braces: `engine::start_game` and + // `engine::start_game_skip_mulligan` drive `turns::auto_advance` and return + // an `ActionResult` without going through the post-action pipeline, so on + // those paths the pipeline's drain does not run and a parked batch would + // survive the boundary if this drain were removed. (Stated structurally + // rather than as "the only drain on those paths": that would be a universal + // over drain sites, and no search performed here could bound them.) + // Regression: `parked_queue_drains_at_first_upkeep_from_start_game`. + // + // `drain_deferred_trigger_queue` (not the post-announcement variant) is + // deliberate: its gate is `can_drain_deferred_triggers(state, /*allow_ + // spell_on_stack=*/false)`. CR 500.2 means a step in which players receive + // priority ends only with an empty stack, so on every normal step entry the + // guard passes; when it does not, refusing is the CR 601.2h + CR 602.2b + // conservative choice (issue #1793) and CANNOT re-wedge the game — a `None` + // prompt makes every calling arm fall through to `WaitingFor::Priority`, + // which is answerable and re-drains through the post-action pipeline. + // (Note: the entry gate is restrictive, but `dispatch_deferred_triggers_in_order` + // in `triggers.rs` ends in a tail call to + // `drain_deferred_triggers_after_trigger_construction`, whose `else` arm is + // permissive — identical to the deferred-drain branch of + // `engine_priority::run_post_action_pipeline_from`, and behaviourally the + // same here because the stack is empty at a step boundary.) + // + // NOTE: unlike the post-action pipeline's deferred-drain branch, this drain + // is not gated on `skip_deferred_trigger_drain` — and does not need to be. + // + // Do NOT defend that with a census of the flag's call sites. The opt-out can + // be passed positionally (as it is by the trailing `true` in + // `engine_resolution_choices::park_cast_during_resolution_cast_observers`), + // and a positional literal carries no identifier, so no search for the + // flag's name can enumerate the sites that set it. + // + // The gate is the protection instead. The flag exists to hold a drain back + // while a parent resolution continuation is still open (CR 608.2e, issue + // #1793). That condition is a property of state, and this drain already + // tests it directly: `drain_deferred_trigger_queue` is gated by + // `can_drain_deferred_triggers`, which refuses unless + // `triggers::resolution_completion_can_settle` — the same predicate that + // guards the pipeline's sibling branch, and the one that returns `false` + // while `resolving_stack_entry` is live under a resolution-choice prompt. + // So the opt-out's condition is enforced here from state rather than + // inherited through a parameter, and a caller that sets the flag cannot + // lose its effect by reaching this path. + let prompt = match outcome.prompt { + Some(prompt) => Some(prompt), + None => super::triggers::drain_deferred_trigger_queue(state, events_out), + }; + (outcome.fired, prompt) } /// CR 800.4: Skip an eliminated active player's remaining turn through the @@ -9535,3 +9717,7 @@ mod tests { ); } } + +#[cfg(test)] +#[path = "turns_declare_attackers_wedge_tests.rs"] +mod declare_attackers_wedge_tests; diff --git a/crates/engine/src/game/turns_declare_attackers_wedge_tests.rs b/crates/engine/src/game/turns_declare_attackers_wedge_tests.rs new file mode 100644 index 0000000000..1957e94654 --- /dev/null +++ b/crates/engine/src/game/turns_declare_attackers_wedge_tests.rs @@ -0,0 +1,607 @@ +//! Discriminating tests for the `(EndCombat, DeclareAttackers)` wedge and the +//! un-drained `deferred_triggers` queue behind it. +//! +//! REACHABILITY NOTE (non-negotiable — read before editing any fixture here). +//! Every test in this module seeds its parked batch by calling the engine's own +//! collector, `triggers::collect_triggers_into_deferred(state, &[real +//! ZoneChanged event])`. Nothing here hand-constructs a `PendingTriggerContext` +//! and nothing here hand-assembles a corrupt `GameState`: the trigger contexts +//! are built by the real collector, from a real event whose `ZoneChangeRecord` +//! comes from the authoritative production constructor +//! (`GameObject::snapshot_for_zone_change`), against a real `GameObject` created +//! by `zones::create_object`. **Everything downstream of seeding is the +//! unmodified production pipeline** — `apply()` / `start_game_skip_mulligan()` +//! → reducer → `handle_declare_attackers` / `handle_priority_pass` → +//! `advance_after_empty_attackers` / `advance_phase_once` → `auto_advance` → +//! `execute_cleanup` → `sync_waiting_for`. +//! +//! Why a fully-public seeding path is not used: `run_post_action_pipeline` +//! drains at every `WaitingFor::Priority` boundary, so **on a fixed engine no +//! public action sequence can leave a queue parked across a boundary — the +//! reachable park IS the bug.** `collect_triggers_into_deferred` is +//! `pub(crate)`, which is why these live in an in-crate module rather than in +//! `tests/integration/`. The public-API positive controls that prove these +//! assertions are non-vacuous live in +//! `tests/integration/declare_attackers_end_combat_pairing.rs`. + +use super::*; +use crate::game::scenario::GameScenario; +use crate::game::{engine, triggers, zones}; +use crate::types::actions::GameAction; +use crate::types::card_type::CoreType; +use crate::types::game_state::{StackEntryKind, ZoneChangeRecord}; +use crate::types::identifiers::CardId; +use crate::types::zones::Zone; + +/// Altar of the Brood's verbatim Oracle text (MTGJSON, via the repo's own +/// `card-data.json`). Per `/card-test`, fixtures are built from the real card's +/// exact text — a paraphrase can take a different parser branch and go green +/// while the real card stays broken. +/// +/// The real card is a 1-mana **Artifact**. These fixtures build it as a +/// noncreature permanent, which is what both fixture constraints require: it is +/// SBA-safe (CR 704.5f cannot move a noncreature permanent for 0 toughness) and +/// it is not a legal attacker (so `advance_to_end_step` cannot park at +/// `Phase::DeclareAttackers`). The trigger condition names "another permanent +/// you control", never the source's own card type, so the source's type is not +/// load-bearing for what these tests measure. +const ALTAR_OF_THE_BROOD: &str = + "Whenever another permanent you control enters, each opponent mills a card."; + +/// Impact Tremors' verbatim Oracle text (MTGJSON, via `card-data.json`). +/// +/// A **second, semantically distinct** observer of the same entry event, needed +/// by the CR 603.3b ordering rows. Two copies of one card will NOT raise an +/// ordering prompt: `strip_trigger_instance_identity` deliberately strips +/// per-instance object identity so genuinely indistinguishable triggers take +/// `TriggerOrderingDisposition::NoChoiceNeeded` — there is no choice to make +/// between two identical abilities. A real CR 603.3b choice needs two triggers +/// that actually differ. +/// +/// Impact Tremors is an **Enchantment**, so it preserves the noncreature +/// fixture constraints (SBA-safe; not a legal attacker). Its condition names a +/// *creature* entering, which is why `battlefield_entry_event` builds a creature. +const IMPACT_TREMORS: &str = + "Whenever a creature you control enters, this enchantment deals 1 damage to each opponent."; + +/// Put a noncreature permanent carrying the Altar observer onto the battlefield +/// under `player`, and return its `ObjectId`. +fn add_altar(scenario: &mut GameScenario, player: PlayerId) -> ObjectId { + let builder = + scenario.add_enchantment_from_oracle(player, "Altar of the Brood", ALTAR_OF_THE_BROOD); + builder.id() +} + +/// Put the second, distinct observer onto the battlefield under `player`. +fn add_impact_tremors(scenario: &mut GameScenario, player: PlayerId) -> ObjectId { + let builder = scenario.add_enchantment_from_oracle(player, "Impact Tremors", IMPACT_TREMORS); + builder.id() +} + +/// Build a **real** `GameEvent::ZoneChanged` for a permanent entering the +/// battlefield under `controller`, using the production record constructor. +/// +/// This is the event an ordinary ETB emits; feeding it to the engine's own +/// collector is what parks a genuine observer context. +/// +/// The entering permanent is a **2/2 creature**: that satisfies BOTH observers' +/// conditions (Altar of the Brood's "another permanent you control" and Impact +/// Tremors' "a creature you control"), and a positive toughness keeps it +/// SBA-safe under CR 704.5f so no fresh `ZoneChanged` is emitted behind the +/// test's back. +fn battlefield_entry_event(state: &mut GameState, controller: PlayerId) -> GameEvent { + let card_id = CardId(state.next_object_id); + let id = zones::create_object( + state, + card_id, + controller, + "Entering Creature".to_string(), + Zone::Battlefield, + ); + let obj = state.objects.get_mut(&id).expect("entering object exists"); + obj.card_types.core_types.push(CoreType::Creature); + obj.base_card_types = obj.card_types.clone(); + obj.power = Some(2); + obj.toughness = Some(2); + obj.base_power = Some(2); + obj.base_toughness = Some(2); + let obj = state.objects.get(&id).expect("entering object exists"); + let record: ZoneChangeRecord = + obj.snapshot_for_zone_change(id, Some(Zone::Hand), Zone::Battlefield); + GameEvent::ZoneChanged { + object_id: id, + from: Some(Zone::Hand), + to: Zone::Battlefield, + record: Box::new(record), + } +} + +/// Seed one parked observer context via the engine's own collector. +/// Returns the `ObjectId` of the permanent whose entry produced the event. +fn seed_parked_trigger(state: &mut GameState, controller: PlayerId) -> ObjectId { + let event = battlefield_entry_event(state, controller); + let entering_id = match &event { + GameEvent::ZoneChanged { object_id, .. } => *object_id, + _ => unreachable!("battlefield_entry_event returns ZoneChanged"), + }; + triggers::collect_triggers_into_deferred(state, std::slice::from_ref(&event)); + entering_id +} + +/// The `source_id` of the single stack entry, for the "drained, not cleared" +/// assertions. Panics with the stack contents if the shape is wrong. +fn sole_stack_trigger_source(state: &GameState) -> ObjectId { + assert_eq!( + state.stack.len(), + 1, + "expected exactly one stack entry, got {:?}", + state.stack + ); + let entry = state.stack.last().expect("stack has one entry"); + assert!( + matches!(entry.kind, StackEntryKind::TriggeredAbility { .. }), + "top of stack must be a triggered ability, got {:?}", + entry.kind + ); + entry.source_id +} + +/// Rows A1 / A2 / A3. +/// +/// A1 (CR 603.3 + CR 117.5): a parked `deferred_triggers` batch is put on the +/// stack when the phase interpreter crosses a phase boundary. +/// A2: the queue is **drained, not cleared** — a `deferred_triggers.clear()` +/// band-aid leaves the stack empty and fails here. +/// A3 (CR 508.8 + CR 511.1): the stale `DeclareAttackers` prompt does not +/// survive the advance past `Phase::DeclareAttackers`. +/// +/// DISCRIMINATION BOUNDARY (mandatory — do not relabel this test). +/// This test reds when **A-2** (the `current_trigger_prompt` narrowing) is +/// reverted. It is **green** with A-2 applied and A-1 reverted: with the echo +/// gone the arm falls through to `WaitingFor::Priority`, `apply_action`'s +/// post-action gate admits the pipeline, and `engine_priority.rs`'s +/// `Priority`-gated drain empties the queue one step later. It is therefore +/// **not** A-1's discriminating test — +/// `parked_queue_drains_at_first_upkeep_from_start_game` is. +#[test] +fn declare_no_attackers_with_parked_triggers_drains_and_leaves_declare_attackers() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::DeclareAttackers); + let altar = add_altar(&mut scenario, PlayerId(0)); + // A real attacker, so `valid_attacker_ids` is non-empty and the prompt is a + // genuine choice rather than a forced no-op. + scenario.add_vanilla(PlayerId(0), 2, 2); + let mut runner = scenario.build(); + let state = runner.state_mut(); + state.active_player = PlayerId(0); + state.priority_player = PlayerId(0); + + // Install the CR 508.1 prompt with the production builder. + state.waiting_for = combat::build_declare_attackers_waiting_for(state); + + let entering = seed_parked_trigger(state, PlayerId(0)); + let graveyard_before = state.players[1].graveyard.len(); + + // --- Pre-action reach-guards: the negative assertions below cannot pass + // --- vacuously via a rejected action or a phase that never advanced. + assert_eq!(state.phase, Phase::DeclareAttackers); + assert!( + matches!(state.waiting_for, WaitingFor::DeclareAttackers { .. }), + "fixture must start at the CR 508.1 declaration prompt, got {:?}", + state.waiting_for + ); + assert_eq!( + state.deferred_triggers.len(), + 1, + "exactly one observer context must be parked" + ); + assert!( + state.stack.is_empty(), + "fixture must start with an empty stack" + ); + + let result = engine::apply( + runner.state_mut(), + PlayerId(0), + GameAction::DeclareAttackers { + attacks: vec![], + bands: vec![], + }, + ); + assert!( + result.is_ok(), + "declaring no attackers must succeed: {result:?}" + ); + + let state = runner.state(); + // CR 508.8: with no attackers, declare blockers and combat damage are + // skipped and the game advances to the end of combat step. + assert_eq!( + state.phase, + Phase::EndCombat, + "CR 508.8: an empty declaration advances past combat" + ); + // A3 — CR 511.1: end of combat has no turn-based actions; the active player + // gets priority. The stale declaration prompt must be gone. + assert!( + !matches!(state.waiting_for, WaitingFor::DeclareAttackers { .. }), + "the CR 508.1 declaration prompt must not survive the CR 508.8 advance, got {:?}", + state.waiting_for + ); + assert!( + matches!(state.waiting_for, WaitingFor::Priority { player } if player == state.active_player), + "CR 511.1: expected Priority for the active player, got {:?}", + state.waiting_for + ); + // A1 — CR 603.3: the parked batch was put on the stack. + assert!( + state.deferred_triggers.is_empty(), + "CR 603.3: the parked queue must be drained, still holds {:?}", + state.deferred_triggers + ); + // A2 — drained, NOT cleared: the ability is really on the stack, bound to + // the permanent whose entry triggered it. + assert_eq!( + sole_stack_trigger_source(state), + altar, + "the stacked ability's source must be the observer, not {entering:?}" + ); + // ...and it is UNRESOLVED: the arm returned priority without resolving it. + assert_eq!( + state.players[1].graveyard.len(), + graveyard_before, + "the ability is on the stack, not resolved — no mill has happened yet" + ); +} + +/// Row A5 — hostile, non-empty declaration. +/// +/// A parked queue plus a **real** attack declaration (`attacks_empty == false`) +/// also drains and reaches `Priority` at `Phase::DeclareAttackers` (CR 508.2). +/// This exercises `finish_declare_attackers`'s `else` arm, which never reaches +/// `advance_after_empty_attackers`. +/// +/// **Non-regression row, not revert-failing.** This path returns `Priority` +/// directly, so the drain it exercises is `run_post_action_pipeline`'s — +/// pre-existing and expected green on main. +#[test] +fn declare_real_attacker_with_parked_triggers_drains_at_priority() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::DeclareAttackers); + add_altar(&mut scenario, PlayerId(0)); + let attacker = scenario.add_vanilla(PlayerId(0), 2, 2); + let mut runner = scenario.build(); + let state = runner.state_mut(); + state.active_player = PlayerId(0); + state.priority_player = PlayerId(0); + state.waiting_for = combat::build_declare_attackers_waiting_for(state); + seed_parked_trigger(state, PlayerId(0)); + + assert_eq!(state.phase, Phase::DeclareAttackers); + assert_eq!(state.deferred_triggers.len(), 1); + + let result = engine::apply( + runner.state_mut(), + PlayerId(0), + GameAction::DeclareAttackers { + attacks: vec![(attacker, combat::AttackTarget::Player(PlayerId(1)))], + bands: vec![], + }, + ); + assert!( + result.is_ok(), + "declaring a real attacker must succeed: {result:?}" + ); + + let state = runner.state(); + // CR 508.2: the active player gets priority after the declaration; the + // phase does NOT advance past declare attackers. + assert_eq!(state.phase, Phase::DeclareAttackers); + assert!( + state.deferred_triggers.is_empty(), + "CR 603.3: the parked queue must be drained, still holds {:?}", + state.deferred_triggers + ); +} + +/// Row A6 — hostile, multi-authority (CR 603.3b). +/// +/// **Two** parked triggers under the **same controller** must raise a genuine +/// `OrderTriggers` prompt rather than being auto-ordered or dropped. This also +/// proves the prompt the arm returns is a **real** prompt produced by the +/// drain, not the stale echo. +/// +/// The two observers must be **distinct cards**. Two copies of one card produce +/// byte-identical triggers, which `strip_trigger_instance_identity` recognises +/// as genuinely indistinguishable, so the engine correctly takes +/// `NoChoiceNeeded` and returns `Priority` — there is no ordering choice to +/// make between two identical abilities. +#[test] +fn two_parked_triggers_surface_cr_603_3b_ordering() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::DeclareAttackers); + add_altar(&mut scenario, PlayerId(0)); + add_impact_tremors(&mut scenario, PlayerId(0)); + scenario.add_vanilla(PlayerId(0), 2, 2); + let mut runner = scenario.build(); + let state = runner.state_mut(); + state.active_player = PlayerId(0); + state.priority_player = PlayerId(0); + state.waiting_for = combat::build_declare_attackers_waiting_for(state); + seed_parked_trigger(state, PlayerId(0)); + + assert_eq!( + state.deferred_triggers.len(), + 2, + "two observers must each park a context" + ); + + let result = engine::apply( + runner.state_mut(), + PlayerId(0), + GameAction::DeclareAttackers { + attacks: vec![], + bands: vec![], + }, + ); + assert!( + result.is_ok(), + "declaring no attackers must succeed: {result:?}" + ); + + let state = runner.state(); + assert_eq!(state.phase, Phase::EndCombat, "CR 508.8"); + // CR 603.3b: two simultaneous triggers under one controller — that player + // chooses the order they go on the stack. + match &state.waiting_for { + WaitingFor::OrderTriggers { player, triggers } => { + assert_eq!( + *player, + PlayerId(0), + "CR 603.3b: the controller of the triggers chooses their order" + ); + assert_eq!(triggers.len(), 2, "both parked contexts must be offered"); + } + other => panic!("CR 603.3b: expected an ordering prompt, got {other:?}"), + } +} + +/// **Row A11 — Unit A's discriminating test. This is the test the A-1 drain +/// requires; it must exist.** +/// +/// `start_game_skip_mulligan` contains **no** `run_post_action_pipeline` call — +/// it calls `turns::auto_advance`, assigns `state.waiting_for` from the result, +/// finalizes public state, and returns an `ActionResult`. This is therefore the +/// discriminating test for the drain in `turns::process_phase_triggers`: +/// reverting that drain alone (leaving the `current_trigger_prompt` narrowing in +/// place) makes the `Upkeep` arm fall through to `Priority` with the queue still +/// parked, and **nothing downstream drains it**. Do not delete this test when +/// refactoring the declare-attackers tests. +/// +/// CR 502.4: no player receives priority during the untap step, and any ability +/// that triggers then is held until the next time a player would receive +/// priority — usually upkeep. So `Upkeep` (CR 503.1) is the first arm on a new +/// turn that can settle the queue. +#[test] +fn parked_queue_drains_at_first_upkeep_from_start_game() { + let mut scenario = GameScenario::new(); + let altar = add_altar(&mut scenario, PlayerId(0)); + let mut runner = scenario.build(); + let state = runner.state_mut(); + seed_parked_trigger(state, PlayerId(0)); + + // --- Pre-call reach-guards. + assert_eq!( + state.deferred_triggers.len(), + 1, + "exactly one observer context must be parked before the walk" + ); + assert!( + state.stack.is_empty(), + "fixture must start with an empty stack" + ); + + // MANDATORY — arm the `waiting_for` reach-guard. `GameState::new` + // initialises `waiting_for` to `Priority { PlayerId(0) }` and + // `start_game_skip_mulligan` sets `active_player` to `PlayerId(0)`, so + // WITHOUT this line the stale echo on main is byte-identical to the value + // the fix produces and the post-call `player == active_player` assertion + // passes on main — i.e. it would be vacuous. With this line it fails on + // main, making that assertion a genuine reach-guard and a second + // independent discriminator. Do not "clean this up". + state.waiting_for = WaitingFor::Priority { + player: PlayerId(1), + }; + + let result = engine::start_game_skip_mulligan(runner.state_mut()); + + let state = runner.state(); + // CR 501.1 + CR 502.4: the walk crossed Untap (no priority, no triggers + // processed) and stopped at Upkeep. + assert_eq!( + state.phase, + Phase::Upkeep, + "the walk must reach the upkeep step" + ); + // Genuine reach-guard, given the pre-seed above: on main the stale echo + // returns `Priority { PlayerId(1) }` and this fails. + assert!( + matches!(result.waiting_for, WaitingFor::Priority { player } if player == state.active_player), + "CR 503.1: expected Priority for the active player, got {:?}", + result.waiting_for + ); + // --- The two assertions carrying the discrimination: positive facts only + // --- the drain can satisfy. + assert!( + state.deferred_triggers.is_empty(), + "CR 603.3: the parked queue must be drained at the first upkeep, still holds {:?}", + state.deferred_triggers + ); + assert_eq!( + sole_stack_trigger_source(state), + altar, + "CR 603.3 + CR 503.1a: the observer's ability must be on the stack" + ); +} + +/// Row A11's multi-authority case (CR 603.3b). +/// +/// Proves the pipeline-free consumer surfaces a **real** drain prompt rather +/// than a fallback: two parked contexts under one controller must produce an +/// `OrderTriggers` prompt out of `start_game_skip_mulligan` itself. +/// +/// Two DISTINCT observers, for the reason given on +/// `two_parked_triggers_surface_cr_603_3b_ordering`. +#[test] +fn two_parked_triggers_from_start_game_surface_cr_603_3b_ordering() { + let mut scenario = GameScenario::new(); + add_altar(&mut scenario, PlayerId(0)); + add_impact_tremors(&mut scenario, PlayerId(0)); + let mut runner = scenario.build(); + let state = runner.state_mut(); + seed_parked_trigger(state, PlayerId(0)); + + assert_eq!( + state.deferred_triggers.len(), + 2, + "two observers must each park a context" + ); + + let result = engine::start_game_skip_mulligan(runner.state_mut()); + + match &result.waiting_for { + WaitingFor::OrderTriggers { player, triggers } => { + assert_eq!( + *player, + PlayerId(0), + "CR 603.3b: the controller of the triggers chooses their order" + ); + assert_eq!( + triggers.len(), + 2, + "the pipeline-free consumer must offer both parked contexts" + ); + } + other => panic!( + "CR 603.3b: the pipeline-free consumer must surface a real ordering prompt, got {other:?}" + ), + } +} + +/// **Row A12 — Unit A2's discriminating test. The brief's second required +/// invariant.** +/// +/// CR 514.3a: a `deferred_triggers` batch live at the cleanup step is put on the +/// stack **during cleanup**, the active player gets priority, and the turn does +/// **not** advance. +/// +/// It reds on unmodified main (the queue crosses the boundary and the turn +/// advances) **and** reds with Units A and C applied but A2 reverted (the queue +/// crosses the boundary and drains at the *next* turn's `Upkeep`, so +/// `turn_number` increments and `phase == Upkeep`). It therefore discriminates +/// Unit A2 specifically. +/// +/// WHICH ASSERTIONS CARRY THE DISCRIMINATION — read before attributing a red. +/// Only `turn_number == recorded` and `phase == Phase::Cleanup` discriminate. +/// The other three (`deferred_triggers.is_empty()`, the stack shape, and the +/// `Priority` pairing) are **green on main**, because on main the queue does +/// drain inside the same `apply` call — one step later, at the next turn's +/// upkeep. Those three are anti-band-aid guards (they fail a +/// `deferred_triggers.clear()` shortcut and a drain that loses the source +/// identity), not discriminators. +/// +/// SEEDING ORDER IS LOAD-BEARING: the park is seeded **after** every earlier +/// priority pass, immediately before the final one. Seeding earlier is wrong — +/// an earlier pass returns `Priority` and the post-action pipeline's drain would +/// empty the queue before it could ever reach cleanup. +/// +/// FIXTURE TRAP 1: the seeded permanent must be attack-incapable, or a walk +/// through combat parks at `Phase::DeclareAttackers` and the test never reaches +/// this seam. `ALTAR_OF_THE_BROOD` is built as a noncreature permanent for +/// exactly this reason; the `phase == Phase::End` reach-guard below catches a +/// regression loudly rather than vacuously. +/// +/// FIXTURE TRAP 2: the fixture is placed **directly** at the end step with +/// `at_phase(Phase::End)` rather than walked there with `advance_to_end_step()`. +/// A `GameScenario` library is empty, so a walk that crosses the draw step kills +/// the active player (CR 704.5b) and the game ends — the observed symptom was +/// `InvalidAction("apply_as_current: no authorized submitter (game over?)")` on +/// the first pass. `at_phase` also sets `waiting_for`, `priority_player`, +/// `active_player`, and `turn_number` consistently, which is exactly the +/// pre-state this row's reach-guards assert. +#[test] +fn parked_queue_settles_during_cleanup_not_next_turn() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::End); + let altar = add_altar(&mut scenario, PlayerId(0)); + let mut runner = scenario.build(); + + // Pass with every player EXCEPT the last, so the next pass is the one that + // ends the end step and enters cleanup. + let active = runner.state().active_player; + runner + .act(GameAction::PassPriority) + .expect("first end-step pass must succeed"); + + // Seed the park only now — see the doc comment. + let state = runner.state_mut(); + seed_parked_trigger(state, PlayerId(0)); + + // --- Pre-action reach-guards. + assert_eq!( + state.phase, + Phase::End, + "fixture must be at the end step before the final pass" + ); + assert!( + matches!(state.waiting_for, WaitingFor::Priority { .. }), + "fixture must hold priority before the final pass, got {:?}", + state.waiting_for + ); + assert!(state.stack.is_empty(), "fixture must have an empty stack"); + assert_eq!(state.deferred_triggers.len(), 1); + let turn_before = state.turn_number; + let passer = match state.waiting_for { + WaitingFor::Priority { player } => player, + ref other => unreachable!("guarded above, got {other:?}"), + }; + + let result = engine::apply(runner.state_mut(), passer, GameAction::PassPriority); + assert!( + result.is_ok(), + "the final end-step pass must succeed: {result:?}" + ); + + let state = runner.state(); + // --- The two discriminating assertions (CR 514.3a). + assert_eq!( + state.turn_number, turn_before, + "CR 514.3a: the queue must settle DURING cleanup — the turn must not roll" + ); + assert_eq!( + state.phase, + Phase::Cleanup, + "CR 514.3a: the game must still be in the cleanup step, got {:?}", + state.phase + ); + // --- Anti-band-aid guards (green on main; they fail a `clear()` shortcut). + assert!( + state.deferred_triggers.is_empty(), + "CR 603.3: the parked queue must be drained, still holds {:?}", + state.deferred_triggers + ); + assert_eq!( + sole_stack_trigger_source(state), + altar, + "CR 514.3a: the observer's ability must be on the stack" + ); + assert!( + matches!(state.waiting_for, WaitingFor::Priority { player } if player == state.active_player), + "CR 514.3a: the active player gets priority during cleanup, got {:?}", + state.waiting_for + ); + assert_eq!( + active, state.active_player, + "the active player must not change" + ); +} diff --git a/crates/engine/tests/integration/declare_attackers_end_combat_pairing.rs b/crates/engine/tests/integration/declare_attackers_end_combat_pairing.rs new file mode 100644 index 0000000000..308087aea6 --- /dev/null +++ b/crates/engine/tests/integration/declare_attackers_end_combat_pairing.rs @@ -0,0 +1,151 @@ +//! Public-API positive controls for the `(EndCombat, DeclareAttackers)` wedge +//! fix (CR 508.8 / CR 511.1 / CR 514.3a). +//! +//! **All three tests here pass BOTH before and after the fix.** That is their +//! entire purpose: they prove the discriminating assertions in +//! `crates/engine/src/game/turns_declare_attackers_wedge_tests.rs` are not +//! trivially true, and that the healthy paths are unchanged by the fix. +//! +//! * `declare_no_attackers_reaches_end_combat_priority` — non-vacuity for the +//! drain rows: with an EMPTY deferred queue the empty declaration already +//! reaches `Priority` at `Phase::EndCombat`. +//! * `start_game_skip_mulligan_with_empty_queue_reaches_upkeep_priority` — +//! non-vacuity for row A11: the pipeline-free game-start walk reaches +//! `Upkeep` with an empty stack when nothing is parked. +//! * `cleanup_with_empty_queue_advances_the_turn` — non-vacuity for row A12's +//! `turn_number == recorded` assertion: an ordinary end-of-turn pass still +//! advances the turn, so the CR 514.3a pause does not stall normal turns. + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::actions::GameAction; +use engine::types::game_state::WaitingFor; +use engine::types::phase::Phase; + +/// Row A4 — positive control / non-vacuity. +/// +/// CR 508.8: declaring no attackers skips the declare blockers and combat +/// damage steps. CR 511.1: end of combat has no turn-based actions and the +/// active player gets priority. +/// +/// FIXTURE NOTE: the declaration prompt is installed by walking the real turn +/// machinery from `BeginCombat`, not by `at_phase(Phase::DeclareAttackers)` — +/// `at_phase` sets `waiting_for` to `Priority`, so submitting `DeclareAttackers` +/// there is rejected with `ActionNotAllowed`. Starting the walk at `BeginCombat` +/// also keeps it clear of the draw step, whose empty scenario library would end +/// the game (CR 704.5b). +#[test] +fn declare_no_attackers_reaches_end_combat_priority() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::BeginCombat); + scenario.add_vanilla(P0, 2, 2); + let mut runner = scenario.build(); + runner.advance_to_phase(Phase::DeclareAttackers); + + assert_eq!( + runner.state().phase, + Phase::DeclareAttackers, + "the walk must reach the declaration step" + ); + assert!( + matches!( + runner.state().waiting_for, + WaitingFor::DeclareAttackers { .. } + ), + "CR 508.1: the declaration prompt must be live, got {:?}", + runner.state().waiting_for + ); + + runner + .act(GameAction::DeclareAttackers { + attacks: vec![], + bands: vec![], + }) + .expect("declaring no attackers must succeed"); + + let state = runner.state(); + assert_eq!(state.phase, Phase::EndCombat, "CR 508.8"); + assert!( + matches!(state.waiting_for, WaitingFor::Priority { player } if player == state.active_player), + "CR 511.1: expected Priority for the active player, got {:?}", + state.waiting_for + ); + assert!( + state.deferred_triggers.is_empty(), + "nothing was parked, so nothing may be queued" + ); +} + +/// Row A11's negative sibling — non-vacuity for the game-start walk. +/// +/// With no parked queue, `start_game_skip_mulligan` reaches the first upkeep +/// with `Priority` and an EMPTY stack (CR 501.1, CR 502.4, CR 503.1). +#[test] +fn start_game_skip_mulligan_with_empty_queue_reaches_upkeep_priority() { + let scenario = GameScenario::new(); + let mut runner = scenario.build(); + + let result = engine::game::start_game_skip_mulligan(runner.state_mut()); + + let state = runner.state(); + assert_eq!(state.phase, Phase::Upkeep, "CR 501.1 + CR 502.4"); + assert!( + matches!(result.waiting_for, WaitingFor::Priority { player } if player == state.active_player), + "CR 503.1: expected Priority for the active player, got {:?}", + result.waiting_for + ); + assert!( + state.stack.is_empty(), + "nothing was parked, so the stack must stay empty, got {:?}", + state.stack + ); + assert!(state.deferred_triggers.is_empty()); +} + +/// Row A13 — A12's negative sibling. +/// +/// An end-of-turn pass with an EMPTY deferred queue advances the turn exactly as +/// it does today. This proves A12's `turn_number == recorded` assertion is not +/// trivially satisfiable, and that the CR 514.3a cleanup pause does not stall +/// ordinary turns. +/// +/// FIXTURE NOTE: placed directly at the end step with `at_phase(Phase::End)` +/// rather than walked there — a `GameScenario` library is empty, so a walk that +/// crosses the draw step ends the game (CR 704.5b) and no seat can submit. +#[test] +fn cleanup_with_empty_queue_advances_the_turn() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::End); + // A noncreature permanent only: a legal attacker could park a combat walk + // at `Phase::DeclareAttackers`. + scenario.add_basic_land(P0, engine::types::mana::ManaColor::White); + let mut runner = scenario.build(); + + let turn_before = runner.state().turn_number; + assert_eq!(runner.state().phase, Phase::End); + assert!(runner.state().deferred_triggers.is_empty()); + + // Pass until the turn rolls over. + for _ in 0..8 { + if runner.state().turn_number > turn_before { + break; + } + if !matches!(runner.state().waiting_for, WaitingFor::Priority { .. }) { + break; + } + runner + .act(GameAction::PassPriority) + .expect("passing priority must succeed"); + } + + let state = runner.state(); + assert_eq!( + state.turn_number, + turn_before + 1, + "an ordinary end-of-turn pass must advance the turn" + ); + assert_eq!( + state.active_player, P1, + "the turn must pass to the other seat" + ); + assert!(state.deferred_triggers.is_empty()); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 026fc41ac2..561f89bb5e 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -173,6 +173,7 @@ mod daretti_emblem_simultaneous_death; mod dark_confidant_upkeep; mod dark_depths_thespian_stage; mod death_priest_myrkul_oxford_anthem; +mod declare_attackers_end_combat_pairing; mod delayed_parent_target_incarnation; mod delayed_trigger_continuation; mod demilich_helbrute_graveyard_exile_cost;