diff --git a/crates/engine/src/ai_support/payment_continuation.rs b/crates/engine/src/ai_support/payment_continuation.rs index cc4c9e3481..ef1e5b80c2 100644 --- a/crates/engine/src/ai_support/payment_continuation.rs +++ b/crates/engine/src/ai_support/payment_continuation.rs @@ -414,6 +414,11 @@ fn classify_parked_cost_move_root(state: &GameState) -> PaymentContinuationState | PendingCostMoveResume::Foretell { .. } | PendingCostMoveResume::UnlessBouncePayment { .. } | PendingCostMoveResume::CounterAdditionUnlessPayment { .. } + // CR 701.9b: a parked random unless-discard holds no pending cast and + // no mana-ability cursor — the game picks the cards with no player + // input — so like its counter-addition sibling it affiliates with no + // payment-continuation root. + | PendingCostMoveResume::RandomDiscardUnlessPayment(..) | PendingCostMoveResume::LoyaltyActivation { .. } => { PaymentContinuationState::NotAffiliated } @@ -659,6 +664,8 @@ fn pending_cost_move_contains_root( | Some(PendingCostMoveResume::DelveManaPayment { .. }) | Some(PendingCostMoveResume::UnlessBouncePayment { .. }) | Some(PendingCostMoveResume::CounterAdditionUnlessPayment { .. }) + // CR 701.9b: holds no pending cast, so it can contain no root. + | Some(PendingCostMoveResume::RandomDiscardUnlessPayment(..)) | Some(PendingCostMoveResume::LoyaltyActivation { .. }) | None => false, } diff --git a/crates/engine/src/game/effects/discard.rs b/crates/engine/src/game/effects/discard.rs index 92bc64cb9c..5b51987b15 100644 --- a/crates/engine/src/game/effects/discard.rs +++ b/crates/engine/src/game/effects/discard.rs @@ -439,27 +439,32 @@ pub fn resolve( // CR 608.2c: Effect resolved as no-op (empty hand) — veto downstream IfYouDo. state.cost_payment_failed_flag = true; } else if random { - let mut remaining = hand_cards; - for _ in 0..count { - if remaining.is_empty() { - break; - } - let index = state.rng.random_range(0..remaining.len()); - let obj_id = remaining.swap_remove(index); - if let DiscardOutcome::NeedsReplacementChoice(player) = - discard_caused_by_effect_with_source_and_frame( - state, - obj_id, - discard_player, - Some(ability.source_id), + // CR 701.9a: this is a resolving effect, so Library-of-Leng-class + // replacements DO apply — `DiscardCause::Effect`. + // + // PRE-EXISTING GAP (unchanged by the extraction, called out so the + // asymmetry with the cost caller below is not mistaken for an + // oversight): a replacement choice mid-batch drops the remaining + // picks, because the effect layer has no batch cursor to resume + // through. The returned cursor is therefore ignored here. The cost + // caller DOES persist it, since it additionally owes a pending + // unless-payment that would otherwise never settle. + if matches!( + discard_at_random( + state, + RandomDiscardRequest { + player: discard_player, + source_id: ability.source_id, + count, + eligible: hand_cards, + cause: DiscardCause::Effect, discard_frame, - events, - ) - { - state.waiting_for = - crate::game::replacement::replacement_choice_waiting_for(player, state); - return Ok(()); - } + }, + events, + ), + RandomDiscardOutcome::NeedsReplacementChoice { .. } + ) { + return Ok(()); } } else if hand_cards.is_empty() { // up_to=true with empty hand — choosing 0 is the only option, skip interaction. @@ -547,6 +552,160 @@ pub(crate) fn discard_caused_by_effect_with_source( } /// Resolving-effect discard with optional operation-owned provenance. +/// CR 701.9a vs CR 118.12 / CR 601.2h: WHY a card is being discarded. +/// +/// This is the `caused_by_effect` axis of `route_discard`, surfaced as a type +/// rather than a bool because it is load-bearing and silently mis-set: it gates +/// `ReplacementCondition::EffectCausedDiscard`. Library of Leng replaces a +/// discard caused by a spell or ability, and must NOT touch a discard made to +/// pay a cost — the boundary `library_of_leng_does_not_apply_to_discard_cost` +/// pins. +/// +/// Callers must state which one they are; there is deliberately no default. A +/// shared discard helper that hard-codes one of these silently launders a cost +/// payment into an effect (or vice versa), which is exactly the bug this enum +/// exists to make unrepresentable. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum DiscardCause { + /// A resolving spell or ability discards the card (CR 701.9a). + /// Library-of-Leng-class replacements gate on this. + Effect, + /// The discard IS the payment of a cost (CR 118.12 unless-cost, + /// CR 601.2h additional cost). Effect-caused replacements must not apply. + Cost, +} + +/// One game-selected discard batch: who, how many, from what pool, and why. +/// +/// Bundled rather than passed as positional arguments because the caller-supplied +/// axes are all easy to transpose — `player` vs the source's controller, +/// `count` vs pool length, and especially `cause`, which is silently wrong +/// rather than loudly wrong. Named fields make each call site state its intent. +pub(crate) struct RandomDiscardRequest { + /// The discarding player. + pub player: PlayerId, + /// Discard source, for replacement provenance. + pub source_id: ObjectId, + /// How many cards to pick. + pub count: usize, + /// The already-filtered, already-length-checked pool to pick from. + pub eligible: Vec, + /// Effect or cost — see [`DiscardCause`]. + pub cause: DiscardCause, + /// Operation-owned discard frame, when the caller has one. + pub discard_frame: Option, +} + +/// Result of a game-selected (random) discard batch. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum RandomDiscardOutcome { + /// Every requested card was discarded (or replacement-redirected). + Completed, + /// A replacement effect needs a player choice before the batch can finish. + /// `state.waiting_for` has been set; callers MUST return without treating + /// the batch as complete. + /// + /// The payload is the batch cursor: what a caller needs to finish the job + /// after the choice settles. It is returned rather than stored globally so + /// each caller can persist it in ITS own typed continuation — the cost + /// caller owns an unless-payment that must still be settled, which is not + /// the effect caller's problem. + NeedsReplacementChoice { + /// Cards still un-picked. Excludes the card whose replacement paused. + remaining_eligible: Vec, + /// Picks still owed AFTER the paused one resolves. + remaining_count: usize, + }, +} + +/// CR 701.9b: "Some effects … require a random discard." Move `count` cards +/// picked uniformly at random from `eligible` to their owner's graveyard. +/// +/// SINGLE AUTHORITY for game-selected discard. Both layers call it: +/// +/// * the EFFECT layer — `Effect::Discard { selection: Random }` (Wheel of +/// Torture class), and +/// * the COST layer — an `AbilityCost::Discard { selection: Random }` +/// unless-payment (Balduvian Horde class). +/// +/// Keeping one implementation is what stops the two from drifting on the four +/// things that are easy to get subtly wrong independently: which RNG is used, +/// how a replacement effect mid-batch is surfaced, whether a short pool +/// discards partially, and — via `cause` — whether the discard counts as +/// effect-caused. +/// +/// RNG: `state.rng` — the seeded, replay-deterministic game RNG. Never +/// `rand::thread_rng()`: a replayed game (and the CR 732.2a loop replay) must +/// reproduce the identical discards, and a thread RNG would desync them. +/// +/// `cause` is REQUIRED and has no default. Sharing one helper across an effect +/// and a cost is only safe if provenance travels with the call — hard-coding +/// `Effect` here made Balduvian Horde's *cost* payment trip +/// `ReplacementCondition::EffectCausedDiscard`, so Library of Leng put the paid +/// card on top of the library. See [`DiscardCause`]. +/// +/// Caller contract: `eligible` must already be filtered and length-checked. +/// This function discards `min(count, eligible.len())` cards — it does NOT +/// enforce CR 118.3's all-or-nothing rule, because the two layers disagree on +/// what a short pool means (an effect discards what it can; a cost is simply +/// unpayable). The cost caller performs that check before calling. +pub(crate) fn discard_at_random( + state: &mut GameState, + request: RandomDiscardRequest, + events: &mut Vec, +) -> RandomDiscardOutcome { + let RandomDiscardRequest { + player, + source_id, + count, + eligible, + cause, + discard_frame, + } = request; + let mut remaining = eligible; + for pick in 0..count { + if remaining.is_empty() { + break; + } + let index = state.rng.random_range(0..remaining.len()); + let obj_id = remaining.swap_remove(index); + // CR 701.9a + CR 614.1a: route with this call site's OWN provenance. + // `route_discard` is the shared tail; only the `caused_by_effect` flag + // differs, and it is exactly what Library-of-Leng-class replacements + // gate on. + let outcome = match cause { + DiscardCause::Effect => discard_caused_by_effect_with_source_and_frame( + state, + obj_id, + player, + Some(source_id), + discard_frame, + events, + ), + DiscardCause::Cost => route_discard( + state, + obj_id, + player, + Some(source_id), + false, + discard_frame, + events, + ), + }; + if let DiscardOutcome::NeedsReplacementChoice(chooser) = outcome { + state.waiting_for = + crate::game::replacement::replacement_choice_waiting_for(chooser, state); + return RandomDiscardOutcome::NeedsReplacementChoice { + remaining_eligible: remaining, + // The paused pick is settled by the replacement itself, so the + // resumed batch owes only the picks after it. + remaining_count: count - pick - 1, + }; + } + } + RandomDiscardOutcome::Completed +} + pub(crate) fn discard_caused_by_effect_with_source_and_frame( state: &mut GameState, object_id: ObjectId, @@ -648,6 +807,271 @@ fn route_discard( DiscardOutcome::Complete } +#[cfg(test)] +mod random_discard_authority_tests { + use super::*; + use crate::game::zones::create_object; + use crate::types::identifiers::{CardId, ObjectId}; + use crate::types::player::PlayerId; + use crate::types::zones::Zone; + + /// Stage `n` cards in P0's hand on a game seeded with `seed`. + fn hand_of(seed: u64, n: usize) -> (GameState, Vec) { + let mut state = GameState::new_two_player(seed); + let hand = (0..n) + .map(|i| { + create_object( + &mut state, + CardId(10 + i as u64), + PlayerId(0), + format!("Hand {i}"), + Zone::Hand, + ) + }) + .collect(); + (state, hand) + } + + fn discarded(state: &GameState, hand: &[ObjectId]) -> Vec { + hand.iter() + .copied() + .filter(|id| state.objects[id].zone == Zone::Graveyard) + .collect() + } + + /// A plain EFFECT-caused request. Tests that care about the provenance axis + /// build their own request so the `cause` they exercise is visible at the + /// call site rather than hidden in this default. + fn request(count: usize, eligible: Vec) -> RandomDiscardRequest { + RandomDiscardRequest { + player: PlayerId(0), + source_id: ObjectId(500), + count, + eligible, + cause: DiscardCause::Effect, + discard_frame: None, + } + } + + /// CR 701.9b: the authority moves exactly `count` cards from the eligible + /// pool to the graveyard. + #[test] + fn discard_at_random_moves_exactly_count_cards() { + let (mut state, hand) = hand_of(42, 5); + let mut events = Vec::new(); + let outcome = discard_at_random(&mut state, request(2, hand.clone()), &mut events); + assert_eq!(outcome, RandomDiscardOutcome::Completed); + assert_eq!(discarded(&state, &hand).len(), 2); + assert_eq!(state.players[0].hand.len(), 3, "the rest stay in hand"); + } + + /// The RNG must be the seeded, replay-deterministic `state.rng` — NOT a + /// thread RNG. Two games with the same seed must discard the same cards, + /// or a replayed game (and the CR 732.2a accept-time loop replay) desyncs. + /// A `thread_rng` implementation passes the count test above but fails this. + #[test] + fn discard_at_random_is_seed_deterministic() { + let pick = |seed: u64| { + let (mut state, hand) = hand_of(seed, 6); + let mut events = Vec::new(); + discard_at_random(&mut state, request(3, hand.clone()), &mut events); + discarded(&state, &hand) + }; + assert_eq!( + pick(7), + pick(7), + "same seed must reproduce the same random discards" + ); + } + + /// Reach-guard for the determinism test: the selection genuinely varies + /// with the seed, so `pick(7) == pick(7)` above is not passing merely + /// because the function always takes the same positions. + #[test] + fn discard_at_random_varies_across_seeds() { + let pick = |seed: u64| { + let (mut state, hand) = hand_of(seed, 8); + let mut events = Vec::new(); + discard_at_random(&mut state, request(3, hand.clone()), &mut events); + // Compare by hand POSITION, not ObjectId: ids are assigned in the + // same order every game, so positions are the comparable signal. + discarded(&state, &hand) + .iter() + .map(|id| hand.iter().position(|h| h == id).unwrap()) + .collect::>() + }; + let seeds: Vec> = (0u64..12).map(pick).collect(); + assert!( + seeds.windows(2).any(|w| w[0] != w[1]), + "picks must depend on the seed, got identical selections: {seeds:?}" + ); + } + + /// CR 701.9a + CR 118.12: `DiscardCause` must actually reach + /// `route_discard`'s `caused_by_effect` flag, because that is what + /// `ReplacementCondition::EffectCausedDiscard` gates on. + /// + /// Library of Leng replaces an EFFECT-caused discard (card goes to the top + /// of the library instead of the graveyard) and must not touch a COST + /// payment. The shared random helper originally hard-coded the effect + /// route, so paying Balduvian Horde's cost wrongly offered the replacement. + /// This is the random-selection twin of + /// `library_of_leng_does_not_apply_to_discard_cost`. + /// + /// Both arms run in ONE test so the pair cannot drift: the Cost arm alone + /// would still pass if `DiscardCause` were ignored and everything routed as + /// a cost. + #[test] + fn discard_at_random_honors_cost_vs_effect_provenance() { + let setup = || { + let mut state = GameState::new_two_player(42); + let leng = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Library of Leng".to_string(), + Zone::Battlefield, + ); + let card = create_object( + &mut state, + CardId(2), + PlayerId(0), + "Hand Card".to_string(), + Zone::Hand, + ); + state + .objects + .get_mut(&leng) + .unwrap() + .replacement_definitions + .push(super::tests::library_of_leng_discard_replacement()); + (state, card) + }; + + // COST: no effect-caused replacement may fire — the card hits the + // graveyard and nothing pauses for a choice. + let (mut state, card) = setup(); + let mut events = Vec::new(); + let outcome = discard_at_random( + &mut state, + RandomDiscardRequest { + player: PlayerId(0), + source_id: ObjectId(500), + count: 1, + eligible: vec![card], + cause: DiscardCause::Cost, + discard_frame: None, + }, + &mut events, + ); + assert_eq!( + outcome, + RandomDiscardOutcome::Completed, + "a cost payment must not stop for an effect-caused replacement" + ); + assert!( + state.players[0].graveyard.contains(&card), + "cost discard goes to the graveyard, not the top of the library" + ); + + // EFFECT: the same replacement IS offered, proving the flag is read and + // the Cost arm above is not passing vacuously. + let (mut state, card) = setup(); + let mut events = Vec::new(); + let outcome = discard_at_random( + &mut state, + RandomDiscardRequest { + player: PlayerId(0), + source_id: ObjectId(500), + count: 1, + eligible: vec![card], + cause: DiscardCause::Effect, + discard_frame: None, + }, + &mut events, + ); + assert!( + matches!(outcome, RandomDiscardOutcome::NeedsReplacementChoice { .. }), + "an effect-caused random discard must offer Library of Leng, got {outcome:?}" + ); + } + + /// The batch cursor returned on a pause must describe the work still owed, + /// so the cost caller's persisted continuation can finish it. The paused + /// pick is settled by the replacement itself and must NOT be re-counted. + #[test] + fn discard_at_random_pause_reports_the_remaining_batch() { + let mut state = GameState::new_two_player(42); + let leng = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Library of Leng".to_string(), + Zone::Battlefield, + ); + state + .objects + .get_mut(&leng) + .unwrap() + .replacement_definitions + .push(super::tests::library_of_leng_discard_replacement()); + let hand: Vec = (0..4) + .map(|i| { + create_object( + &mut state, + CardId(10 + i as u64), + PlayerId(0), + format!("Hand {i}"), + Zone::Hand, + ) + }) + .collect(); + + let mut events = Vec::new(); + let outcome = discard_at_random( + &mut state, + RandomDiscardRequest { + player: PlayerId(0), + source_id: ObjectId(500), + count: 3, + eligible: hand.clone(), + cause: DiscardCause::Effect, + discard_frame: None, + }, + &mut events, + ); + let RandomDiscardOutcome::NeedsReplacementChoice { + remaining_eligible, + remaining_count, + } = outcome + else { + panic!("expected a replacement pause, got {outcome:?}"); + }; + assert_eq!( + remaining_count, 2, + "3 requested, the 1st paused and is settled by the replacement, so 2 remain" + ); + assert_eq!( + remaining_eligible.len(), + 3, + "the un-picked pool excludes only the paused card" + ); + } + + /// Caller contract (documented on the authority): a pool shorter than + /// `count` discards what it can and reports `Completed`. Enforcing + /// CR 118.3's all-or-nothing rule is the COST caller's job, because the + /// effect layer legitimately discards a short hand. + #[test] + fn discard_at_random_short_pool_discards_what_it_can() { + let (mut state, hand) = hand_of(42, 2); + let mut events = Vec::new(); + let outcome = discard_at_random(&mut state, request(5, hand.clone()), &mut events); + assert_eq!(outcome, RandomDiscardOutcome::Completed); + assert_eq!(discarded(&state, &hand).len(), 2); + } +} + #[cfg(test)] mod tests { use super::*; @@ -758,7 +1182,7 @@ mod tests { ); } - fn library_of_leng_discard_replacement() -> ReplacementDefinition { + pub(super) fn library_of_leng_discard_replacement() -> ReplacementDefinition { ReplacementDefinition::new(ReplacementEvent::Discard) .mode(ReplacementMode::Optional { decline: None }) .condition(ReplacementCondition::EffectCausedDiscard) diff --git a/crates/engine/src/game/elimination.rs b/crates/engine/src/game/elimination.rs index 7adc2a83e7..9a16566c75 100644 --- a/crates/engine/src/game/elimination.rs +++ b/crates/engine/src/game/elimination.rs @@ -91,7 +91,8 @@ fn abandon_pending_spell_casts( | PendingCostMoveResume::UnlessBouncePayment { .. } | PendingCostMoveResume::ManaAbilityPayment { .. } | PendingCostMoveResume::LoyaltyActivation { .. } - | PendingCostMoveResume::CounterAdditionUnlessPayment { .. } => false, + | PendingCostMoveResume::CounterAdditionUnlessPayment { .. } + | PendingCostMoveResume::RandomDiscardUnlessPayment(..) => false, }; if !abandons_spell { state.pending_cost_move_resume = Some(resume); diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 4302cf2322..eecf7293e3 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -6448,6 +6448,7 @@ pub(crate) fn drain_pending_cost_move_resume( | PendingCostMoveResume::ActivationMillPayment { .. } | PendingCostMoveResume::LoyaltyActivation { .. } | PendingCostMoveResume::CounterAdditionUnlessPayment { .. } + | PendingCostMoveResume::RandomDiscardUnlessPayment(..) ) ), // CR 606.4 + CR 616.1: a fully-prevented loyalty counter add (e.g. an @@ -6471,6 +6472,7 @@ pub(crate) fn drain_pending_cost_move_resume( | PendingCostMoveResume::ActivationMillPayment { .. } | PendingCostMoveResume::LoyaltyActivation { .. } | PendingCostMoveResume::CounterAdditionUnlessPayment { .. } + | PendingCostMoveResume::RandomDiscardUnlessPayment(..) ) ), CostMoveDrainBoundary::PriorityBoundary => matches!( @@ -6551,6 +6553,14 @@ pub(crate) fn drain_pending_cost_move_resume( events, matches!(boundary, CostMoveDrainBoundary::ReplacementDelivered { .. }), )? + } else if matches!( + state.pending_cost_move_resume, + Some(PendingCostMoveResume::RandomDiscardUnlessPayment(..)) + ) { + // CR 118.12: random discard pauses only after its Moved replacement + // returns a replacement choice; the delivered boundary resumes the + // payment through its already-authorized paid epilogue. + engine_payment_choices::resume_random_discard_unless_payment(state, events)? } else { unreachable!("eligible cost-move root must remain parked") }; @@ -18166,6 +18176,28 @@ mod stage2_injector_tests { // with a delegation; it sits above this producer and below the first two. // The merge tree therefore retains main's first two coordinates // (`:6177`/`:6254`) and shifts this one by −16 to `:9442`. + // Random-discard-as-a-cost (#7320, review round 1): `engine.rs:12004 ⇒ + // :12019`, +15, and ONLY the engine.rs entry moved — the four + // effects/mod.rs + scoped_library_search entries did not, which is the + // set-preservation evidence. `git diff -U0` on this file has exactly three + // hunks, ALL inside `drain_pending_cost_move_resume` at `:5761`/ + // `:5865` (+1/+13 = +14, zero deletions), i.e. entirely ABOVE this + // producer; predicted `12004+14` equals the observed coordinate exactly. + // They add the `RandomDiscardUnlessPayment` delivery resume and its + // dispatch arm — a cost-payment continuation, not + // a prompt mint: it RESUMES an already-minted `UnlessPayment` rather than + // creating a recipient, so it is correctly absent from this census. + // Identity re-established, not assumed: the producer at `:12019` is the + // same announcement-time modal mint this row NAMES — an `Ok(Some(..))` of + // the optional-effect prompt over `player` / `source_id` / + // `trigger_description` / `may_trigger_key` — still inside + // `begin_pending_trigger_target_selection`. (Spelled out rather than + // quoted: the needle above is ASSEMBLED so this row cannot be counted by + // its own instrument, and a verbatim quote here re-introduces exactly the + // self-count that defends against — it inflates `in_test` and reds the + // TOTAL assert instead of this one.) The two asserts + // above this one fired GREEN on the run that caught it — total still 37, + // partition still 5/7/25 — so no producer was added or lost. // // ⚠ THIS ROW FAILS IN CI BEFORE IT FAILS LOCALLY, and that is not a bug in the // row. CI checks out `refs/pull//merge` — this branch merged with CURRENT @@ -18846,7 +18878,10 @@ mod stage2_injector_tests { // This rebase raised the literal as a CONFLICT twice and then drifted it SILENTLY a // third time at the tip; only the offset control caught the silent one. That is the // drift class FU-4 (content-hash coordinate anchor) exists to end. - "game/engine.rs:12763".to_string(), + // #7320's random-discard continuation adds ten lines above this producer in the + // merged tree. Re-derived by the exact producer text at `:12773`, not by carrying + // the prior coordinate. + "game/engine.rs:12773".to_string(), ], "the five production producers, NAMED: the CR 603.5 gate in `resolve_chain_body` \ plus the two repeated-optional-payment drivers, the per-player acceptance cursor \ diff --git a/crates/engine/src/game/engine_payment_choices.rs b/crates/engine/src/game/engine_payment_choices.rs index b510bf11da..16f05c53b6 100644 --- a/crates/engine/src/game/engine_payment_choices.rs +++ b/crates/engine/src/game/engine_payment_choices.rs @@ -789,7 +789,7 @@ pub(super) fn handle_unless_payment( AbilityCost::Discard { count, filter, - selection: _, + selection, self_scope: _, } => { let resolved = crate::game::quantity::resolve_quantity_with_targets( @@ -810,6 +810,62 @@ pub(super) fn handle_unless_payment( // the effect happens. if (hand_cards.len() as u32) < count { payment_failed = true; + } else if selection.is_random() { + // CR 701.9b: a RANDOM discard offers the payer no choice — + // the game picks. Pay it inline through the shared + // `discard_at_random` authority (same code the effect layer + // uses, same seeded `state.rng`) instead of surfacing + // `WardDiscardChoice`, which would let the payer select and + // silently turn Balduvian Horde's cost into a cheaper one. + // + // Structural precedent: the `Mill` arm below — the other + // unless-cost with no choice to offer pays inline and falls + // through to the paid path. + // + // CR 118.12 + CR 601.2h: `DiscardCause::Cost`, NOT `Effect`. + // This discard IS the payment, so an effect-caused + // replacement (Library of Leng) must not apply to it — the + // boundary `library_of_leng_does_not_apply_to_discard_cost` + // pins. + match crate::game::effects::discard::discard_at_random( + state, + crate::game::effects::discard::RandomDiscardRequest { + player, + source_id: pending_effect.source_id, + count: count as usize, + eligible: hand_cards, + cause: crate::game::effects::discard::DiscardCause::Cost, + discard_frame: None, + }, + events, + ) { + crate::game::effects::discard::RandomDiscardOutcome::Completed => {} + // CR 616.1: a replacement effect parked a choice. Unlike + // the chosen-discard sibling there is no + // `WardDiscardChoice` re-prompt loop to own the + // remainder, and unlike the effect layer this caller + // still owes an unless-payment. Persist BOTH the batch + // cursor and the full payment payload so the drain can + // settle the guarded ability, instead of returning and + // leaving it neither paid nor unpaid at bare priority. + crate::game::effects::discard::RandomDiscardOutcome::NeedsReplacementChoice { + remaining_eligible, + remaining_count, + } => { + state.pending_cost_move_resume = + Some(PendingCostMoveResume::RandomDiscardUnlessPayment(Box::new( + crate::types::game_state::RandomDiscardUnlessPaymentResume { + source_id: pending_effect.source_id, + pending_effect: pending_effect.clone(), + trigger_event: trigger_event.clone(), + payer: player, + remaining_eligible, + remaining_count: remaining_count as u32, + }, + ))); + return Ok(action_result(events, state.waiting_for.clone())); + } + } } else { state.waiting_for = WaitingFor::WardDiscardChoice { player, @@ -1910,6 +1966,90 @@ pub(super) fn resume_counter_addition_unless_payment( Ok(state.waiting_for.clone()) } +/// CR 701.9b + CR 118.12 + CR 616.1: Resume a RANDOM unless-discard after the +/// replacement choice that paused it settled. +/// +/// The replacement's outcome deliberately does NOT decide whether the cost was +/// paid, which is why this takes no boundary argument. CR 118.12: the "if they +/// do / don't" clause "checks whether the player chose to pay an optional cost +/// … **regardless of what events actually occurred**." The player already +/// elected to pay (`PayUnlessCost { pay: true }`) and the up-front eligible-hand +/// check already established the CR 118.3 resources, so the payment is +/// authorized before the replacement is ever consulted. A redirect (Library of +/// Leng) and a prevention alike leave that choice intact. +/// +/// The earlier `Delivered → Paid` / `Prevented → Failed` mapping was copied from +/// `resume_counter_addition_unless_payment` rather than derived from CR 118.12; +/// under it, an applicable replacement preventing the first move would sacrifice +/// Balduvian Horde out from under a player who had paid. +/// +/// The Moved replacement path parks only for a replacement choice; once that +/// choice resolves to delivery, this continuation settles the paid epilogue. +pub(super) fn resume_random_discard_unless_payment( + state: &mut GameState, + events: &mut Vec, +) -> Result { + let Some(PendingCostMoveResume::RandomDiscardUnlessPayment(parked)) = + state.pending_cost_move_resume.take() + else { + unreachable!("random-discard unless-payment resume requires its typed continuation") + }; + let crate::types::game_state::RandomDiscardUnlessPaymentResume { + pending_effect, + trigger_event, + payer, + source_id, + remaining_eligible, + remaining_count, + } = *parked; + + if remaining_count > 0 { + // Finish the batch. A SECOND replacement choice mid-remainder re-parks + // the same continuation with the narrowed cursor, so an N-card random + // discard can pause once per card without losing the payment. + match crate::game::effects::discard::discard_at_random( + state, + crate::game::effects::discard::RandomDiscardRequest { + player: payer, + source_id, + count: remaining_count as usize, + eligible: remaining_eligible, + cause: crate::game::effects::discard::DiscardCause::Cost, + discard_frame: None, + }, + events, + ) { + crate::game::effects::discard::RandomDiscardOutcome::Completed => {} + crate::game::effects::discard::RandomDiscardOutcome::NeedsReplacementChoice { + remaining_eligible, + remaining_count, + } => { + state.pending_cost_move_resume = + Some(PendingCostMoveResume::RandomDiscardUnlessPayment(Box::new( + crate::types::game_state::RandomDiscardUnlessPaymentResume { + pending_effect, + trigger_event, + payer, + source_id, + remaining_eligible, + remaining_count: remaining_count as u32, + }, + ))); + return Ok(state.waiting_for.clone()); + } + } + } + + // CR 118.12 + CR 118.12a: settle through the PAID epilogue — the same call + // the uninterrupted path makes at the `!payment_failed` early return above. + // `finish_unless_payment` is the DECLINE tail: its body is gated on + // `!pay || payment_failed`, so routing a successful resume through it + // silently skips `EffectResolved`, the `IfAPlayerDoes` alternative-outcome + // sub, and the `SequentialSibling` chain. Balduvian Horde has none of + // those, which is exactly why that mistake was invisible in its tests. + finish_successful_unless_payment(state, &pending_effect, &trigger_event, events) +} + pub(super) fn handle_ward_sacrifice_choice( state: &mut GameState, waiting_for: WaitingFor, @@ -2160,9 +2300,9 @@ mod tests { use super::*; use crate::game::zones::create_object; use crate::types::ability::{ - AbilityCondition, AbilityDefinition, AbilityKind, ControllerRef, ManaContribution, - ManaProduction, QuantityExpr, ResolvedAbility, SacrificeCost, SubAbilityLink, - TriggerDefinition, TypedFilter, + AbilityCondition, AbilityDefinition, AbilityKind, CardSelectionMode, ControllerRef, + ManaContribution, ManaProduction, QuantityExpr, ResolvedAbility, SacrificeCost, + SubAbilityLink, TriggerDefinition, TypedFilter, }; use crate::types::card_type::CoreType; use crate::types::game_state::{AutoMayChoice, MayTriggerAutoChoiceKey, MayTriggerOrigin}; @@ -2437,6 +2577,130 @@ mod tests { assert!(result.is_err()); } + /// Stage `hand_size` discardable cards for P0 and park an unless-payment + /// whose cost is a `count`-card discard in `selection` mode. The pending + /// effect is a marker `gain_life(5)`: it fires only if the unless-cost goes + /// UNPAID, so "life still 20" proves the cost was paid. + fn unless_discard_state( + hand_size: usize, + count: i32, + selection: CardSelectionMode, + ) -> (GameState, Vec) { + let mut state = GameState::new_two_player(42); + state.players[0].life = 20; + let hand: Vec = (0..hand_size) + .map(|i| { + create_object( + &mut state, + CardId(10 + i as u64), + PlayerId(0), + format!("Hand {i}"), + crate::types::zones::Zone::Hand, + ) + }) + .collect(); + let pending = ResolvedAbility::new(gain_life(5), vec![], ObjectId(100), PlayerId(0)); + state.waiting_for = WaitingFor::UnlessPayment { + player: PlayerId(0), + cost: AbilityCost::Discard { + count: QuantityExpr::Fixed { value: count }, + filter: None, + selection, + self_scope: crate::types::ability::DiscardSelfScope::FromHand, + }, + pending_effect: Box::new(pending), + trigger_event: None, + effect_description: None, + remaining: Vec::new(), + }; + (state, hand) + } + + fn graveyard_count(state: &GameState, hand: &[ObjectId]) -> usize { + hand.iter() + .filter(|id| state.objects[id].zone == crate::types::zones::Zone::Graveyard) + .count() + } + + /// CR 701.9b + CR 118.12a: a RANDOM unless-discard has no choice to offer, + /// so it must be paid inline by the game — never surfaced as an interactive + /// selection. Before the fix this arm ignored `selection` and raised + /// `WardDiscardChoice`, letting the payer pick which card to pitch and + /// silently making a Balduvian Horde-class cost cheaper than printed. + #[test] + fn unless_discard_random_pays_inline_without_prompting() { + let (mut state, hand) = unless_discard_state(3, 1, CardSelectionMode::Random); + let mut events = Vec::new(); + let waiting_for = state.waiting_for.clone(); + handle_unless_payment(&mut state, waiting_for, true, &mut events) + .expect("random unless-discard should resolve"); + + assert!( + !matches!(state.waiting_for, WaitingFor::WardDiscardChoice { .. }), + "a random discard must not surface an interactive selection, got {:?}", + state.waiting_for + ); + assert_eq!( + graveyard_count(&state, &hand), + 1, + "exactly one card must have been discarded by the game" + ); + assert_eq!( + state.players[0].life, 20, + "the cost was paid, so the unless-effect (gain 5) must not happen" + ); + } + + /// NO-REGRESSION twin of the test above: a player-CHOSEN unless-discard + /// still routes to the interactive prompt and moves nothing until the + /// player selects. Without this, the arm above could pass by making every + /// discard game-selected. + #[test] + fn unless_discard_chosen_still_prompts() { + let (mut state, hand) = unless_discard_state(3, 1, CardSelectionMode::Chosen); + let mut events = Vec::new(); + let waiting_for = state.waiting_for.clone(); + handle_unless_payment(&mut state, waiting_for, true, &mut events) + .expect("chosen unless-discard should resolve"); + + assert!( + matches!( + state.waiting_for, + WaitingFor::WardDiscardChoice { remaining: 1, .. } + ), + "a player-chosen discard must still prompt, got {:?}", + state.waiting_for + ); + assert_eq!( + graveyard_count(&state, &hand), + 0, + "nothing may move before the player has chosen" + ); + } + + /// CR 118.3: "A player can't pay a cost without having the necessary + /// resources to pay it fully." A random discard demanding more cards than + /// the payer holds is unpayable, so the unless-effect happens and the hand + /// is left untouched — no partial random discard. + #[test] + fn unless_discard_random_short_hand_is_unpayable() { + let (mut state, hand) = unless_discard_state(1, 2, CardSelectionMode::Random); + let mut events = Vec::new(); + let waiting_for = state.waiting_for.clone(); + handle_unless_payment(&mut state, waiting_for, true, &mut events) + .expect("unpayable unless-discard should resolve"); + + assert_eq!( + graveyard_count(&state, &hand), + 0, + "an unpayable cost must not take a partial random discard" + ); + assert_eq!( + state.players[0].life, 25, + "the cost was unpayable, so the unless-effect (gain 5) happens" + ); + } + /// CR 118.12 + CR 119.4 + CR 107.3c (M1 fold): An unless-pay-life cost /// with a `QuantityExpr` amount evaluates the quantity at unless-time. /// Pre-fold the cost was an `i32`; post-fold it carries the same widened diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 04e4d5e9b2..7bd5b6a774 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -49,9 +49,9 @@ use crate::types::ability::ManaProduction; use crate::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, AbilityTag, AdditionalCostOrigin, AdditionalCostPaymentSource, AggregateFunction, AttachmentKind, - AttackersDeclaredCountSubject, CastManaObjectScope, CastManaSpentMetric, CastVariantPaid, - CoinFlipResult, Comparator, ControllerRef, CountScope, CounterTriggerFilter, DamageKindFilter, - DestinationConstraint, DieResultFilter, Effect, EffectScope, FilterProp, + AttackersDeclaredCountSubject, CardSelectionMode, CastManaObjectScope, CastManaSpentMetric, + CastVariantPaid, CoinFlipResult, Comparator, ControllerRef, CountScope, CounterTriggerFilter, + DamageKindFilter, DestinationConstraint, DieResultFilter, Effect, EffectScope, FilterProp, ManaAbilityProducedFilter, ObjectScope, OriginConstraint, ParsedCondition, PlayerFilter, PlayerScope, PtStat, PtValueScope, QuantityExpr, QuantityRef, RenownSubject, SacrificeAggregateStat, SacrificeCost, SacrificeRequirement, SharedQuality, StaticCondition, @@ -3367,7 +3367,7 @@ fn parse_unless_life_cost(rest: &str) -> Option { /// Grammar — two independent axes over one noun: /// /// ```text -/// discard_phrase := [ | "a" | "an"] [] ("card" | "cards") +/// discard_phrase := [ | "a" | "an"] [] ("card" | "cards") ["at random"] /// ``` /// /// Both unless-payer forms route here: the controller form ("unless **you** @@ -3385,10 +3385,13 @@ fn parse_unless_life_cost(rest: &str) -> Option { /// at `unless_branch_boundary` so a chained " or …" branch survives, while the /// `you` form owns the rest of the clause. /// -/// CR 701.9b ("some effects … require a random discard") stays unsupported: -/// the resolution-time unless-payment path (`engine_payment_choices.rs`) -/// ignores `selection` and always prompts, so accepting an "at random" tail -/// would falsely lower a player-chosen discard as a random discard. +/// CR 701.9b ("some effects … require a random discard") is the third axis: an +/// "at random" tail lowers to `CardSelectionMode::Random`. That is only honest +/// because the unless-payment path now pays such a cost through +/// `effects::discard::discard_at_random` instead of prompting. Before that, the +/// only two options were both wrong — claim a player-chosen discard (making a +/// Balduvian Horde-class cost strictly cheaper than printed) or fail the clause +/// closed (dropping the whole class to `Unimplemented`). fn parse_unless_discard_cost_phrase(branch_text: &str) -> Option { let trimmed = branch_text.trim().trim_end_matches('.').trim(); if trimmed.is_empty() { @@ -3413,29 +3416,41 @@ fn parse_unless_discard_cost_phrase(branch_text: &str) -> Option { } let count = i32::try_from(count).ok()?; - let discard = |filter| AbilityCost::Discard { + let discard = |filter, selection| AbilityCost::Discard { count: QuantityExpr::Fixed { value: count }, filter, - selection: crate::types::ability::CardSelectionMode::Chosen, + selection, self_scope: crate::types::ability::DiscardSelfScope::FromHand, }; - - // Untyped noun: the count axis alone ("a card", "two cards"). The plural - // arm precedes the singular so `tag("card")` cannot leave a stray "s". + // Untyped noun: the count axis alone ("a card", "two cards"), optionally + // carrying the CR 701.9b randomness axis. The plural arm precedes the + // singular so `tag("card")` cannot leave a stray "s". if let Ok((rest, _)) = alt((tag::<_, _, OracleError<'_>>("cards"), tag("card"))).parse(after_count) { let rest = rest.trim().trim_end_matches('.').trim(); if rest.is_empty() { - return Some(discard(None)); + return Some(discard(None, CardSelectionMode::Chosen)); + } + // Full consumption is required. A bare `.is_ok()` also accepts + // "at randomly" and "at random foo", which would lower an unrecognized + // clause as a random discard instead of leaving it honestly unsupported. + if all_consuming(tag::<_, _, OracleError<'_>>("at random")) + .parse(rest) + .is_ok() + { + return Some(discard(None, CardSelectionMode::Random)); } } // Typed noun: the remainder is a type phrase plus the noun, lowered by the // shared `parse_discard_card_filter` authority (which owns the - // " card"/" cards" suffix strip and rejects anything it cannot type). + // " card"/" cards" suffix strip and rejects anything it cannot type). No + // printed card combines a type phrase with "at random" in an unless-cost, + // so the typed arm stays `Chosen`; the randomness axis lives on the + // untyped arm above until such a card ships. super::oracle_effect::imperative::parse_discard_card_filter(after_count) - .map(|filter| discard(Some(filter))) + .map(|filter| discard(Some(filter), CardSelectionMode::Chosen)) } /// CR 118.12 + CR 608.2c + CR 119.4: Recognize non-mana "unless" alternative diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index e391b32196..0afb3e407c 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -12353,27 +12353,49 @@ fn self_etb_sacrifice_it_anaphor_binds_to_self_ref() { ); } +/// CR 701.9b + CR 118.12a: Balduvian Horde — "sacrifice it unless you discard a +/// card at random". The clause is now fully supported, and this test tracks the +/// third state it has been in. +/// +/// Originally it asserted a `Chosen` discard: the clause lowered, but the payer +/// got to pick, which made the printed cost strictly cheaper. It was then +/// changed to assert `Unimplemented` — honest, but it dropped the card. Now the +/// unless-payment resolver honors `CardSelectionMode::Random` +/// (`effects::discard::discard_at_random`), so the clause lowers truthfully: +/// a real unless-cost whose selection mode is `Random`. +/// +/// `selection` is the load-bearing assertion. A `Chosen` here would be the +/// original bug back again, and the test would still otherwise pass. #[test] -fn trigger_unless_you_discard_a_card_at_random_preserves_unsupported_clause() { - // Balduvian Horde's random discard cannot be lowered as a player-chosen - // unless payment: the payment resolver currently ignores selection mode. - // Keep the entire clause visible as unsupported until it can honor random - // discard rather than silently changing the card's behavior. +fn trigger_unless_you_discard_a_card_at_random_lowers_as_random_cost() { let def = parse_trigger_line( "When ~ enters, sacrifice it unless you discard a card at random.", "Balduvian Horde", ); + + let unless_pay = def + .unless_pay + .as_ref() + .expect("the random discard must lower to a real unless cost"); + assert_eq!(unless_pay.payer, TargetFilter::Controller); assert!( - def.unless_pay.is_none(), - "random discard must not lower to a player-chosen unless payment" + matches!( + unless_pay.cost, + AbilityCost::Discard { + count: QuantityExpr::Fixed { value: 1 }, + filter: None, + selection: CardSelectionMode::Random, + self_scope: DiscardSelfScope::FromHand + } + ), + "cost must be a one-card RANDOM discard, got {:?}", + unless_pay.cost ); - let execute = def - .execute - .as_ref() - .expect("should preserve the unsupported clause"); + + let execute = def.execute.as_ref().expect("should have execute"); assert!( - matches!(*execute.effect, Effect::Unimplemented { .. }), - "random-discard unless clause must remain visible as unimplemented, got {:?}", + matches!(*execute.effect, Effect::Sacrifice { .. }), + "the unless-effect is the self-sacrifice, got {:?}", execute.effect ); } @@ -13655,18 +13677,80 @@ fn unless_discard_cost_phrase_rejects_zero_count() { ); } -/// CR 701.9b: random discard is distinct from a player-selected discard. Until -/// the unless-payment resolver preserves `CardSelectionMode::Random`, this -/// phrase must remain unsupported rather than being lowered dishonestly. +/// CR 701.9b: random discard is distinct from a player-selected discard, and +/// the phrase now lowers TRUTHFULLY as `CardSelectionMode::Random` instead of +/// having to pick between two wrong answers. This test previously asserted the +/// clause stayed unsupported — the right call only while the unless-payment +/// resolver ignored `selection`. It now honors it +/// (`effects::discard::discard_at_random`), so the honest lowering is the typed +/// one. The mode must be `Random`, not `Chosen`, on BOTH payer forms, or a +/// Balduvian Horde-class cost silently gets cheaper than printed. #[test] -fn unless_discard_cost_phrase_rejects_random_discard() { +fn unless_discard_cost_phrase_lowers_random_discard_as_random() { + let (they_cost, rest) = + parse_unless_they_discard_cost("a card at random").expect("the they form must lower"); + assert!( + rest.trim().is_empty(), + "the whole branch should be consumed, left {rest:?}" + ); + let you_cost = + parse_unless_alt_cost("you discard a card at random").expect("the you form must lower"); + assert_eq!( + they_cost, you_cost, + "both payer forms must agree on the random tail" + ); assert!( - parse_unless_they_discard_cost("a card at random").is_none(), - "the anaphoric-payer form must not lower random discard as chosen" + matches!( + they_cost, + AbilityCost::Discard { + count: QuantityExpr::Fixed { value: 1 }, + filter: None, + selection: CardSelectionMode::Random, + .. + } + ), + "expected a one-card RANDOM discard, got {they_cost:?}" ); +} + +/// The random tail must be FULLY consumed. A prefix match would swallow +/// "at randomly" and "at random foo" and lower an unrecognized clause as a +/// random discard, which is the coverage-dishonesty failure mode in the other +/// direction — claiming support for text the grammar never understood. +#[test] +fn unless_discard_cost_phrase_rejects_partial_random_suffix() { + for tail in [ + "a card at randomly", + "a card at random foo", + "a card atrandom", + ] { + assert!( + parse_unless_they_discard_cost(tail).is_none(), + "{tail:?} is not the random-discard grammar and must not lower" + ); + assert!( + parse_unless_alt_cost(&format!("you discard {tail}")).is_none(), + "{tail:?} must not lower on the controller form either" + ); + } +} + +/// NO-REGRESSION twin: without an "at random" tail the discard stays +/// player-chosen. Guards against the randomness axis leaking onto every +/// unless-discard — which would make Court of Ambition pick for the opponent +/// instead of letting them choose what to pitch. +#[test] +fn unless_discard_cost_phrase_without_random_tail_stays_chosen() { + let cost = parse_unless_alt_cost("you discard a card").expect("plain discard must lower"); assert!( - parse_unless_alt_cost("you discard a card at random").is_none(), - "the controller form must not lower random discard as chosen" + matches!( + cost, + AbilityCost::Discard { + selection: CardSelectionMode::Chosen, + .. + } + ), + "a plain discard must remain player-chosen, got {cost:?}" ); } diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 8f9dc2dae3..4ab182e6da 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -6313,6 +6313,50 @@ pub enum PendingCostMoveResume { #[serde(default, skip_serializing_if = "Vec::is_empty")] remaining: Vec, }, + /// CR 701.9b + CR 118.12 + CR 616.1: a RANDOM unless-discard that paused on + /// a replacement choice (Library of Leng, Madness) partway through its batch. + /// + /// Two things would otherwise be lost, because both live only in the paying + /// stack frame that returns when the choice is raised: + /// + /// * the unless-payment itself — nothing else records + /// `pending_effect` / `trigger_event` / `effect_description` / + /// `remaining`, so the guarded ability is left neither paid nor unpaid + /// and the game resets to bare priority with its fate undetermined; + /// * the batch cursor — the picks still owed after the paused card. + /// + /// The player-CHOSEN sibling needs no analogue: it parks in + /// `WaitingFor::WardDiscardChoice`, whose own re-prompt loop owns the + /// remainder. A random discard raises no prompt, so nothing else can own it. + /// + /// Boxed deliberately. `GameState` is moved by value through the + /// phase-server action + AI path and is guarded by a hard size budget + /// (`game_state_size.rs`); this payload is large and populated only during + /// a replacement pause, which is exactly the shape that guard says to box + /// rather than widen the budget for. + RandomDiscardUnlessPayment(Box), +} + +/// CR 701.9b + CR 118.12 + CR 616.1: payload of +/// [`PendingCostMoveResume::RandomDiscardUnlessPayment`]. Split into its own +/// boxed struct purely to keep `PendingCostMoveResume` — and therefore +/// `GameState` — inside its stack budget. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct RandomDiscardUnlessPaymentResume { + pub pending_effect: Box, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub trigger_event: Option, + /// The paying player — the unless-payer, not necessarily the ability's + /// controller. + pub payer: PlayerId, + /// Discard source, so resumed picks keep their replacement provenance. + pub source_id: ObjectId, + /// Cards still un-picked when the batch paused. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub remaining_eligible: Vec, + /// Picks still owed after the paused card settles. + #[serde(default)] + pub remaining_count: u32, } /// CR 601.2h + CR 616.1: Resume paying a sequential cost after a replacement diff --git a/crates/engine/tests/integration/balduvian_horde_random_discard.rs b/crates/engine/tests/integration/balduvian_horde_random_discard.rs new file mode 100644 index 0000000000..50d23a4669 --- /dev/null +++ b/crates/engine/tests/integration/balduvian_horde_random_discard.rs @@ -0,0 +1,160 @@ +//! Balduvian Horde — random discard as an unless-COST. +//! +//! Oracle text (verbatim, Scryfall): +//! "When this creature enters, sacrifice it unless you discard a card at +//! random." +//! +//! CR 701.9b draws a hard line between a random discard and a player-selected +//! one, and the engine only ever implemented the EFFECT side of it. As a COST +//! the mode was dropped: the unless-payment path destructured `selection: _` +//! and raised `WardDiscardChoice`, so the payer got to pick which card to +//! pitch. That is not cosmetic — on this card it converts the printed cost into +//! a strictly cheaper one, letting you keep your best card and ditch a land. +//! +//! The fix routes the cost through `effects::discard::discard_at_random`, the +//! same authority (and the same seeded `state.rng`) the effect layer uses. +//! +//! These tests drive the REAL pipeline: the creature is built from its verbatim +//! Oracle text and cast, so the ETB trigger, the parse of the "at random" tail, +//! and the cost payment all have to work together. + +use engine::game::scenario::{GameRunner, GameScenario, P0}; +use engine::types::actions::GameAction; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +const BALDUVIAN_HORDE: &str = + "When this creature enters, sacrifice it unless you discard a card at random."; + +/// P0 casts Balduvian Horde with `hand_size` other cards in hand. Returns the +/// runner, the Horde's id, and the ids of the staged hand cards. +fn cast_horde(hand_size: usize, seed: u64) -> (GameRunner, ObjectId, Vec) { + let mut scenario = GameScenario::new_n_player(2, seed); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool( + P0, + (0..4) + .map(|_| ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![])) + .collect(), + ); + + let horde = scenario + .add_creature_to_hand_from_oracle(P0, "Balduvian Horde", 5, 5, BALDUVIAN_HORDE) + // Printed cost {2}{R}{R}. + .with_mana_cost(ManaCost::Cost { + generic: 2, + shards: vec![ManaCostShard::Red, ManaCostShard::Red], + }) + .id(); + + let hand: Vec = (0..hand_size) + .map(|i| scenario.add_card_to_hand(P0, &format!("Filler Card {i}"))) + .collect(); + + let mut runner = scenario.build(); + runner.state_mut().active_player = P0; + runner.state_mut().priority_player = P0; + runner.cast(horde).resolve(); + (runner, horde, hand) +} + +fn discarded_count(runner: &GameRunner, hand: &[ObjectId]) -> usize { + hand.iter() + .filter(|id| runner.state().objects[id].zone == Zone::Graveyard) + .count() +} + +/// Drive the ETB trigger to its unless-payment prompt. +fn advance_to_unless_prompt(runner: &mut GameRunner) { + for _ in 0..20 { + if matches!(runner.state().waiting_for, WaitingFor::UnlessPayment { .. }) { + return; + } + if runner.act(GameAction::PassPriority).is_err() { + break; + } + } + panic!( + "the ETB trigger never surfaced an unless-payment prompt: {:?}", + runner.state().waiting_for + ); +} + +/// CR 701.9b: paying the cost discards a card WITHOUT asking which one. This is +/// the discriminating assertion — before the fix the engine parked on +/// `WardDiscardChoice` here and let the payer select. +#[test] +fn balduvian_horde_random_discard_is_paid_without_a_prompt() { + let (mut runner, horde, hand) = cast_horde(3, 42); + advance_to_unless_prompt(&mut runner); + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("paying the random discard must be accepted"); + + assert!( + !matches!( + runner.state().waiting_for, + WaitingFor::WardDiscardChoice { .. } + ), + "a random discard must never ask the payer to choose, got {:?}", + runner.state().waiting_for + ); + assert_eq!( + discarded_count(&runner, &hand), + 1, + "exactly one card must have been discarded by the game" + ); + runner.advance_until_stack_empty(); + assert_eq!( + runner.state().objects[&horde].zone, + Zone::Battlefield, + "paying the cost keeps the Horde on the battlefield" + ); +} + +/// CR 118.12a: declining makes the unless-effect happen — the Horde sacrifices +/// itself and the hand is untouched. +#[test] +fn balduvian_horde_declining_sacrifices_and_keeps_the_hand() { + let (mut runner, horde, hand) = cast_horde(3, 42); + advance_to_unless_prompt(&mut runner); + + runner + .act(GameAction::PayUnlessCost { pay: false }) + .expect("declining must be accepted"); + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().objects[&horde].zone, + Zone::Graveyard, + "declining the discard sacrifices the Horde" + ); + assert_eq!( + discarded_count(&runner, &hand), + 0, + "a declined cost discards nothing" + ); +} + +/// CR 118.3: an empty hand cannot pay a one-card discard, so the cost is +/// unpayable and the Horde is sacrificed even on `pay: true`. +#[test] +fn balduvian_horde_empty_hand_cannot_pay() { + let (mut runner, horde, _hand) = cast_horde(0, 42); + advance_to_unless_prompt(&mut runner); + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("attempting an unpayable cost must be accepted"); + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().objects[&horde].zone, + Zone::Graveyard, + "an unpayable random discard still sacrifices the Horde" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index fda6125963..da7d28eb01 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -50,6 +50,7 @@ mod awaken_runtime; mod azors_gateway_transform_condition; mod backup_becomes_target_trigger; mod balance_equalization; +mod balduvian_horde_random_discard; mod baleful_mastery_regression; mod banding_combat; mod bards_company_recruit; @@ -913,6 +914,7 @@ mod purged_source_intervening_if_lki; mod purged_source_matches_filter_lki; mod quirion_ranger_activation; mod rage_reflection_double_strike_grant; +mod random_discard_cost_replacement_resume; mod refurbished_familiar; mod relic_of_progenitus_6446; mod render_silent_cant_cast; diff --git a/crates/engine/tests/integration/random_discard_cost_replacement_resume.rs b/crates/engine/tests/integration/random_discard_cost_replacement_resume.rs new file mode 100644 index 0000000000..fbdb1031c8 --- /dev/null +++ b/crates/engine/tests/integration/random_discard_cost_replacement_resume.rs @@ -0,0 +1,318 @@ +//! End-to-end coverage for the RANDOM unless-discard replacement continuation +//! (`PendingCostMoveResume::RandomDiscardUnlessPayment`). +//! +//! A random unless-discard pays inline with no prompt, so unlike its +//! player-chosen sibling it has no `WardDiscardChoice` re-prompt loop to own the +//! remainder. If a replacement effect interrupts the batch, TWO things live only +//! in the paying stack frame that returns: the unless-payment itself (which must +//! still be settled, or the guarded ability is left neither paid nor unpaid at +//! bare priority) and the batch cursor (the picks still owed). +//! +//! REACHABILITY — worth stating, because it determines what a valid fixture is. +//! After the `DiscardCause` split a COST discard can no longer pause at the +//! `Discard` replacement gate: the corpus's only two `ReplacementEvent::Discard` +//! definitions are the Library of Leng class (`EffectCausedDiscard`, correctly +//! excluded for costs) and the Dodecapod class (not `Optional`, so it raises no +//! choice). The pause survives only at the SECOND gate — the hand→graveyard +//! `Moved` replacement inside `complete_discard_to_graveyard`, which is not +//! gated on `caused_by_effect`. So these fixtures use a graveyard-redirect +//! replacement (Rest in Peace class), made `Optional` so it raises the choice. +//! +//! CR ANCHORS: +//! * CR 616.1 — the affected player chooses which applicable replacement to +//! apply; that choice is what parks the batch. +//! * CR 701.9a/b — discard, and random discard specifically. +//! * CR 118.12a — the "unless" construction; declining ≡ the effect happens. +//! * CR 118.3 — a cost cannot be paid partially. + +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::types::ability::{ + AbilityDefinition, AbilityKind, Effect, ReplacementDefinition, ReplacementMode, TargetFilter, +}; +use engine::types::actions::GameAction; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::replacements::ReplacementEvent; +use engine::types::zones::Zone; + +/// Balduvian Horde's printed Oracle text — a one-card random unless-discard. +const BALDUVIAN_HORDE: &str = + "When this creature enters, sacrifice it unless you discard a card at random."; + +/// Rest in Peace class, made OPTIONAL so it surfaces an Accept/Decline choice +/// instead of applying silently. Watches other cards (`valid_card: None`) moving +/// to the graveyard from anywhere, and exiles them instead. +fn optional_graveyard_exile_replacement() -> ReplacementDefinition { + ReplacementDefinition::new(ReplacementEvent::Moved) + .destination_zone(Zone::Graveyard) + .mode(ReplacementMode::Optional { decline: None }) + .execute(AbilityDefinition::new( + AbilityKind::Spell, + Effect::ChangeZone { + origin: None, + destination: Zone::Exile, + target: TargetFilter::SelfRef, + owner_library: false, + enter_transformed: false, + enters_under: None, + enter_tapped: engine::types::zones::EtbTapState::Unspecified, + enters_attacking: false, + up_to: false, + enter_with_counters: vec![], + conditional_enter_with_counters: vec![], + enters_modified_if: None, + face_down_profile: None, + }, + )) +} + +/// P0 casts Balduvian Horde with `hand_size` other cards in hand, and a +/// battlefield permanent hosting the optional graveyard-redirect replacement. +fn setup(hand_size: usize) -> (GameRunner, ObjectId, Vec) { + let mut scenario = GameScenario::new(); + scenario.at_phase(engine::types::phase::Phase::PreCombatMain); + scenario.with_mana_pool( + P0, + (0..4) + .map(|_| { + engine::types::mana::ManaUnit::new( + engine::types::mana::ManaType::Red, + ObjectId(0), + false, + vec![], + ) + }) + .collect(), + ); + + // The replacement host. On P1 so it cannot be confused with the Horde. + scenario + .add_creature(P1, "Graveyard Warden", 1, 1) + .with_replacement_definition(optional_graveyard_exile_replacement()); + + let horde = scenario + .add_creature_to_hand_from_oracle(P0, "Balduvian Horde", 5, 5, BALDUVIAN_HORDE) + .with_mana_cost(engine::types::mana::ManaCost::Cost { + generic: 2, + shards: vec![ + engine::types::mana::ManaCostShard::Red, + engine::types::mana::ManaCostShard::Red, + ], + }) + .id(); + + let hand: Vec = (0..hand_size) + .map(|i| scenario.add_card_to_hand(P0, &format!("Filler Card {i}"))) + .collect(); + + let mut runner = scenario.build(); + runner.state_mut().active_player = P0; + runner.state_mut().priority_player = P0; + runner.cast(horde).resolve(); + (runner, horde, hand) +} + +fn advance_to_unless_prompt(runner: &mut GameRunner) { + for _ in 0..20 { + if matches!(runner.state().waiting_for, WaitingFor::UnlessPayment { .. }) { + return; + } + if runner.act(GameAction::PassPriority).is_err() { + break; + } + } + panic!( + "the ETB trigger never surfaced an unless-payment prompt: {:?}", + runner.state().waiting_for + ); +} + +fn moved_out_of_hand(runner: &GameRunner, hand: &[ObjectId]) -> usize { + hand.iter() + .filter(|id| runner.state().objects[id].zone != Zone::Hand) + .count() +} + +/// CR 616.1 + CR 118.12: the batch parks on the replacement choice, and +/// ACCEPTING it redirects the filler to exile and resumes the preserved +/// payment: the cost counts as paid, the +/// guarded unless-effect (sacrifice) does NOT happen, and no cost continuation +/// is left parked. +/// +/// This is the discriminating case for `PendingCostMoveResume:: +/// RandomDiscardUnlessPayment`. Without the persisted continuation the drain has +/// no owner able to call `finish_unless_payment`, and the Horde is left neither +/// sacrificed nor kept. +#[test] +fn random_discard_cost_resumes_its_payment_after_an_accepted_replacement() { + let (mut runner, horde, hand) = setup(3); + advance_to_unless_prompt(&mut runner); + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("paying the random discard must be accepted"); + + // The batch parked on the graveyard-redirect choice rather than completing. + let WaitingFor::ReplacementChoice { candidates, .. } = runner.state().waiting_for.clone() + else { + panic!( + "expected the random discard to park on a ReplacementChoice, got {:?}", + runner.state().waiting_for + ); + }; + assert!( + runner.state().pending_cost_move_resume.is_some(), + "the unless-payment continuation must be persisted while the choice is open" + ); + let accept_idx = candidates + .iter() + .position(|c| c.description == "Accept") + .expect("an Accept option"); + + let resumed = runner + .act(GameAction::ChooseReplacement { index: accept_idx }) + .expect("accepting the redirect must be accepted"); + + // CR 118.12: the resume must settle through the PAID epilogue, not the + // decline tail. `EffectResolved` is emitted only by + // `finish_successful_unless_payment` — which also runs the `IfAPlayerDoes` + // alternative-outcome sub and the `SequentialSibling` chain. Routing a + // successful resume through `finish_unless_payment` skips all three, and + // Balduvian Horde's body is too simple to notice, so this event is the + // discriminator that does. + assert!( + resumed + .events + .iter() + .any(|e| matches!(e, engine::types::events::GameEvent::EffectResolved { .. })), + "the paid epilogue must run on resume (EffectResolved), got {:?}", + resumed.events + ); + + runner.advance_until_stack_empty(); + + assert_eq!( + moved_out_of_hand(&runner, &hand), + 1, + "exactly one card left the hand as the payment" + ); + let moved_filler = hand + .iter() + .find(|id| runner.state().objects[id].zone != Zone::Hand) + .expect("exactly one filler must have moved"); + assert_eq!( + runner.state().objects[moved_filler].zone, + Zone::Exile, + "accepting the replacement must redirect the paid filler to exile" + ); + assert_eq!( + runner.state().objects[&horde].zone, + Zone::Battlefield, + "the resumed payment counts as paid, so the Horde is NOT sacrificed" + ); + assert!( + runner.state().pending_cost_move_resume.is_none(), + "the continuation must be drained, not left parked" + ); +} + +/// CR 616.1: DECLINING the optional replacement lets the natural hand→graveyard +/// move happen. That is still a delivered discard, so the payment resumes +/// identically — the same continuation must own both branches of the choice. +#[test] +fn random_discard_cost_resumes_its_payment_after_a_declined_replacement() { + let (mut runner, horde, hand) = setup(3); + advance_to_unless_prompt(&mut runner); + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("paying the random discard must be accepted"); + + let WaitingFor::ReplacementChoice { candidates, .. } = runner.state().waiting_for.clone() + else { + panic!( + "expected a ReplacementChoice, got {:?}", + runner.state().waiting_for + ); + }; + let decline_idx = candidates + .iter() + .position(|c| c.description == "Decline") + .expect("a Decline option"); + + runner + .act(GameAction::ChooseReplacement { index: decline_idx }) + .expect("declining the redirect must be accepted"); + runner.advance_until_stack_empty(); + + assert_eq!( + moved_out_of_hand(&runner, &hand), + 1, + "the card still leaves the hand — declining the redirect sends it to the graveyard" + ); + let moved_filler = hand + .iter() + .find(|id| runner.state().objects[id].zone != Zone::Hand) + .expect("exactly one filler must have moved"); + assert_eq!( + runner.state().objects[moved_filler].zone, + Zone::Graveyard, + "declining the replacement must send the paid filler to the graveyard" + ); + assert_eq!( + runner.state().objects[&horde].zone, + Zone::Battlefield, + "a declined redirect is still a completed discard, so the cost is paid" + ); + assert!( + runner.state().pending_cost_move_resume.is_none(), + "the continuation must be drained on the decline branch too" + ); +} + +/// CR 118.12a + CR 118.3: REACH-GUARD. With an empty hand the cost is unpayable, +/// so the guarded effect happens and the Horde is sacrificed — proving the two +/// tests above are not passing merely because the Horde survives by default. +/// +/// Note the fixture's own second-order effect: the sacrifice moves the Horde to +/// the graveyard, which trips the SAME optional redirect the payment did. That +/// choice is answered here (decline) so the sacrifice completes naturally. It is +/// a distinct choice from the payment's — the payment never started, so no +/// unless-payment continuation is ever parked, which is the other half of this +/// guard. +#[test] +fn random_discard_cost_with_no_cards_still_sacrifices() { + let (mut runner, horde, _hand) = setup(0); + advance_to_unless_prompt(&mut runner); + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("attempting an unpayable cost must be accepted"); + assert!( + runner.state().pending_cost_move_resume.is_none(), + "an unpayable cost never begins, so nothing may be parked" + ); + + // The sacrifice's own graveyard move offers the redirect; decline it so the + // Horde lands in the graveyard rather than exile. + if let WaitingFor::ReplacementChoice { candidates, .. } = runner.state().waiting_for.clone() { + let decline_idx = candidates + .iter() + .position(|c| c.description == "Decline") + .expect("a Decline option on the sacrifice's graveyard move"); + runner + .act(GameAction::ChooseReplacement { index: decline_idx }) + .expect("declining the redirect must be accepted"); + } + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().objects[&horde].zone, + Zone::Graveyard, + "an unpayable random discard sacrifices the Horde" + ); + assert!( + runner.state().pending_cost_move_resume.is_none(), + "nothing may be left parked" + ); +}