From 8981018d984548d7f8f52be72e67d84c8137c6c8 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:20:12 -0500 Subject: [PATCH 1/7] ENGINE: Pay random discard costs without prompting CR 701.9b distinguishes a random discard from a player-selected one, but only the EFFECT layer implemented it. As a COST, random discard was unimplemented in four places, inconsistently: * casting.rs::resolve_non_self_discard_requirement - selection-agnostic, so it prompts and the payer picks * mana_abilities::discard_cost_choice - gates on Chosen, Random leg never offered * engine_payment_choices.rs unless-payment - destructures `selection: _` and always raises WardDiscardChoice * effects/pay.rs resolution scope - deliberately fails the payment rather than faking Paid The practical consequence on the unless-payment path: a Balduvian Horde-class cost ("sacrifice it unless you discard a card at random") let the payer choose which card to pitch, silently converting the printed cost into a strictly cheaper one. Extract the effect layer's existing implementation into `effects::discard::discard_at_random` as the single authority for game-selected discard, and route the unless-payment path through it. The extraction is the point: the two layers were one copy-paste away from drifting on the three things that are easy to get independently wrong - which RNG is used, how a mid-batch replacement effect is surfaced, and whether a short pool discards partially. The doc comment pins all three. RNG is `state.rng`, the seeded replay-deterministic game RNG, never `rand::thread_rng()`: a replayed game and the CR 732.2a accept-time loop replay must reproduce identical discards. A test asserts same-seed reproducibility, with a companion reach-guard proving the picks actually vary by seed so that assertion is not vacuous. The authority deliberately does NOT enforce CR 118.3's all-or-nothing rule. The two layers genuinely disagree about a short pool - an effect discards what it can, a cost is simply unpayable - so that check stays with the cost caller, which already performs it. Scope: unless-payment only. The casting and mana-ability call sites have the same gap, but casting.rs is a shared hot path and widening the blast radius buys nothing here; they are listed above so the remaining work is visible rather than silently deferred. Tests: four at the authority level (exact count, seed determinism, the cross-seed reach-guard, short-pool contract) and three on the cost path (random pays inline with no prompt; chosen still prompts - the no-regression twin; a short hand is unpayable per CR 118.3 with no partial discard). Co-Authored-By: Claude Opus 5 --- crates/engine/src/game/effects/discard.rs | 233 ++++++++++++++++-- .../engine/src/game/engine_payment_choices.rs | 160 +++++++++++- 2 files changed, 368 insertions(+), 25 deletions(-) diff --git a/crates/engine/src/game/effects/discard.rs b/crates/engine/src/game/effects/discard.rs index 89dfabcb77..479f4b5123 100644 --- a/crates/engine/src/game/effects/discard.rs +++ b/crates/engine/src/game/effects/discard.rs @@ -438,27 +438,17 @@ 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), - discard_frame, - events, - ) - { - state.waiting_for = - crate::game::replacement::replacement_choice_waiting_for(player, state); - return Ok(()); - } + if discard_at_random( + state, + discard_player, + ability.source_id, + count, + hand_cards, + discard_frame, + 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. @@ -546,6 +536,75 @@ pub(crate) fn discard_caused_by_effect_with_source( } /// Resolving-effect discard with optional operation-owned provenance. +/// Result of a game-selected (random) discard batch. +#[derive(Debug, Clone, Copy, 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. + NeedsReplacementChoice, +} + +/// 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 three +/// things that are easy to get subtly wrong independently: which RNG is used, +/// how a replacement effect mid-batch is surfaced, and whether a short pool +/// discards partially. +/// +/// 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. +/// +/// 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, + player: PlayerId, + source_id: ObjectId, + count: usize, + eligible: Vec, + discard_frame: Option, + events: &mut Vec, +) -> RandomDiscardOutcome { + let mut remaining = eligible; + 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(chooser) = + discard_caused_by_effect_with_source_and_frame( + state, + obj_id, + player, + Some(source_id), + discard_frame, + events, + ) + { + state.waiting_for = + crate::game::replacement::replacement_choice_waiting_for(chooser, state); + return RandomDiscardOutcome::NeedsReplacementChoice; + } + } + RandomDiscardOutcome::Completed +} + pub(crate) fn discard_caused_by_effect_with_source_and_frame( state: &mut GameState, object_id: ObjectId, @@ -647,6 +706,138 @@ 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() + } + + /// 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, + PlayerId(0), + ObjectId(500), + 2, + hand.clone(), + None, + &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, + PlayerId(0), + ObjectId(500), + 3, + hand.clone(), + None, + &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, + PlayerId(0), + ObjectId(500), + 3, + hand.clone(), + None, + &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:?}" + ); + } + + /// 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, + PlayerId(0), + ObjectId(500), + 5, + hand.clone(), + None, + &mut events, + ); + assert_eq!(outcome, RandomDiscardOutcome::Completed); + assert_eq!(discarded(&state, &hand).len(), 2); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/engine/src/game/engine_payment_choices.rs b/crates/engine/src/game/engine_payment_choices.rs index b547c2736b..163abbd748 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,34 @@ 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. + match crate::game::effects::discard::discard_at_random( + state, + player, + pending_effect.source_id, + count as usize, + hand_cards, + None, + events, + ) { + crate::game::effects::discard::RandomDiscardOutcome::Completed => {} + // CR 616.1: a replacement effect parked a choice; its + // cursor owns the continuation. Do not clobber it and + // do not treat the pause as a declined payment. + crate::game::effects::discard::RandomDiscardOutcome::NeedsReplacementChoice => { + return Ok(action_result(events, state.waiting_for.clone())); + } + } } else { state.waiting_for = WaitingFor::WardDiscardChoice { player, @@ -2134,9 +2162,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}; @@ -2411,6 +2439,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 From bb88ffeb3922685042bd859129df4dbcf6b7f572 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Wed, 12 Aug 2026 20:05:06 -0500 Subject: [PATCH 2/7] ENGINE: Support Balduvian Horde (random discard as a cost) With the unless-payment resolver now honoring CardSelectionMode::Random, the parser can lower "at random" truthfully instead of choosing between two wrong answers. Before this, both options were bad: * lower as Chosen - the clause parses, but the payer picks which card to pitch, making Balduvian Horde's printed cost strictly cheaper (keep the bomb, ditch a land) * fail closed - honest, but drops the whole "unless [you] discard a card at random" class to Unimplemented parse_unless_discard_cost_phrase now carries the CR 701.9b randomness axis alongside the count and type axes it already had. The typed-noun arm stays Chosen: no printed card combines a type phrase with "at random" in an unless-cost, and the untyped arm owns the axis until one ships. Test updates - three separate tests encoded the old fail-closed contract, and only a full-suite run surfaced all of them: * unless_discard_cost_phrase_lowers_random_discard_as_random (was ..._rejects_random_discard) - both payer forms agree, and the mode is Random * unless_discard_cost_phrase_without_random_tail_stays_chosen - the no-regression twin; without this the randomness axis could leak onto every unless-discard and make Court of Ambition pick for the opponent * trigger_unless_you_discard_a_card_at_random_lowers_as_random_cost (was ..._preserves_unsupported_clause) - the Balduvian Horde trigger. Its doc comment records all three states the test has been in so the next reader does not re-litigate the history. `selection` is the load-bearing assertion: a Chosen there is the original bug back, with the test otherwise still passing. New integration coverage drives the real pipeline - Balduvian Horde built from its verbatim Scryfall Oracle text and cast, so the ETB trigger, the "at random" parse, and the cost payment all have to work together: paying discards without a prompt and the Horde survives; declining sacrifices it with the hand untouched; an empty hand cannot pay (CR 118.3) and it is sacrificed anyway. Co-Authored-By: Claude Opus 5 --- crates/engine/src/parser/oracle_trigger.rs | 38 +++-- .../engine/src/parser/oracle_trigger_tests.rs | 104 +++++++++--- .../balduvian_horde_random_discard.rs | 160 ++++++++++++++++++ crates/engine/tests/integration/main.rs | 1 + 4 files changed, 270 insertions(+), 33 deletions(-) create mode 100644 crates/engine/tests/integration/balduvian_horde_random_discard.rs diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 4d7a7702f0..0c05d7b055 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -3325,7 +3325,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** @@ -3343,10 +3343,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() { @@ -3371,29 +3374,40 @@ 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, }; + use crate::types::ability::CardSelectionMode; - // 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)); + } + if 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 3e8134ed58..e27ed4174a 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -12229,27 +12229,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 ); } @@ -13531,18 +13553,58 @@ 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!( - parse_unless_they_discard_cost("a card at random").is_none(), - "the anaphoric-payer form must not lower random discard as chosen" + 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_alt_cost("you discard a card at random").is_none(), - "the controller 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:?}" + ); +} + +/// 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!( + matches!( + cost, + AbilityCost::Discard { + selection: CardSelectionMode::Chosen, + .. + } + ), + "a plain discard must remain player-chosen, got {cost:?}" ); } 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 98b53a2153..77d11994c7 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -41,6 +41,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; From f9af1320ad3f7b59d6eeedde390470a30ac5d520 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Wed, 12 Aug 2026 22:37:52 -0500 Subject: [PATCH 3/7] fix(PR-7320): carry discard provenance and persist the unless-payment Addresses both review blockers. Each was verified against the code before changing anything; both were real. 1. RANDOM DISCARD PAYMENT WAS RECORDED AS AN EFFECT DISCARD discard_at_random hard-coded the effect route, so a cost payment reached route_discard with caused_by_effect: true and Library of Leng (ReplacementCondition::EffectCausedDiscard) wrongly applied to it. Provenance now travels with the call as a required parameter with no default: DiscardCause::{Effect, Cost}. A type rather than a bool because this axis fails SILENTLY - the wrong value yields a plausible game that is subtly wrong, not a crash. The regression runs both arms in ONE test on purpose. A Cost-only assertion would still pass if the parameter were ignored and everything routed as a cost; the Effect arm proves the flag is actually read. 2. A REPLACEMENT CHOICE LOST THE UNLESS-PAYMENT CONTINUATION PendingCostMoveResume had no discard-unless variant, so the drain had no owner able to call finish_unless_payment and the pending effect was left neither accepted nor rejected. The code comment claiming "its cursor owns the continuation" was an unverified assumption, and false. Adds PendingCostMoveResume::RandomDiscardUnlessPayment, modelled on CounterAdditionUnlessPayment: persisted at the pause with the full payment payload plus a batch cursor, drained through the same finish_unless_payment tail, with the same Delivered->Paid / Prevented->Failed mapping (a redirected discard still happened per CR 701.9a; a prevented one cannot pay per CR 118.3). A second pause mid-remainder re-parks the narrowed cursor, so an N-card random discard can pause once per card without losing the payment. RandomDiscardOutcome::NeedsReplacementChoice now carries that cursor rather than storing it globally, so each caller persists it in its own typed continuation. 3. "at random" REQUIRED FULL CONSUMPTION (CodeRabbit) A prefix match also accepted "at randomly" and "at random foo". Wrapped in all_consuming. INCIDENTAL, all guardrails that wanted the real fix rather than the easy one: * GameState stack budget - the new variant tripped the 12,800-byte guard. Boxed the payload into RandomDiscardUnlessPaymentResume per that guard's own instruction, rather than widening the constant. * clippy too_many_arguments (8/7) - bundled the caller-supplied axes into RandomDiscardRequest. Better shape regardless: player/count/cause are all easy to transpose positionally, and cause is the one that fails silently. * CR 603.5 prompt census - engine.rs:12004 => :12019, pure line movement, drift-logged in the established format. git diff -U0 has exactly three hunks, all inside drain_pending_cost_move_resume and all above the producer (+1/+1/+13 = +15, zero deletions); 12004+15 matches exactly; the producer at the new line is the same announcement-time modal mint inside begin_pending_trigger_target_selection; the other four entries did not move; total/partition asserts stayed green. The new resume RESUMES an already-minted UnlessPayment rather than creating a recipient, so it is correctly absent from that census. * The census caught its own drift-log entry: the first draft quoted the producer verbatim, and that literal IS the needle the scanner greps for (assembled via format! precisely so the row cannot count itself). Rewritten to describe the producer instead, with the reason recorded in the log. NOT FIXED, DELIBERATELY: the effect layer still drops the remainder of a random batch on a replacement pause. That predates this PR (the code was extracted verbatim) and needs its own resume plumbing; the asymmetry is documented at the effect call site so it reads as a known gap rather than an oversight. Co-Authored-By: Claude Opus 5 --- .../src/ai_support/payment_continuation.rs | 7 + crates/engine/src/game/effects/discard.rs | 359 +++++++++++++++--- crates/engine/src/game/engine.rs | 39 +- .../engine/src/game/engine_payment_choices.rs | 144 ++++++- crates/engine/src/parser/oracle_trigger.rs | 5 +- .../engine/src/parser/oracle_trigger_tests.rs | 22 ++ crates/engine/src/types/game_state.rs | 50 +++ 7 files changed, 552 insertions(+), 74 deletions(-) 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 479f4b5123..4920c4aa86 100644 --- a/crates/engine/src/game/effects/discard.rs +++ b/crates/engine/src/game/effects/discard.rs @@ -438,16 +438,31 @@ 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 { - if discard_at_random( - state, - discard_player, - ability.source_id, - count, - hand_cards, - discard_frame, - events, - ) == RandomDiscardOutcome::NeedsReplacementChoice - { + // 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, + ), + RandomDiscardOutcome::NeedsReplacementChoice { .. } + ) { return Ok(()); } } else if hand_cards.is_empty() { @@ -536,15 +551,70 @@ pub(crate) fn discard_caused_by_effect_with_source( } /// Resolving-effect discard with optional operation-owned provenance. -/// Result of a game-selected (random) discard batch. +/// 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. - NeedsReplacementChoice, + /// + /// 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 @@ -557,15 +627,22 @@ pub(crate) enum RandomDiscardOutcome { /// * 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 three +/// 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, and whether a short pool -/// discards partially. +/// 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 @@ -573,33 +650,56 @@ pub(crate) enum RandomDiscardOutcome { /// unpayable). The cost caller performs that check before calling. pub(crate) fn discard_at_random( state: &mut GameState, - player: PlayerId, - source_id: ObjectId, - count: usize, - eligible: Vec, - discard_frame: Option, + request: RandomDiscardRequest, events: &mut Vec, ) -> RandomDiscardOutcome { + let RandomDiscardRequest { + player, + source_id, + count, + eligible, + cause, + discard_frame, + } = request; let mut remaining = eligible; - for _ in 0..count { + 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); - if let DiscardOutcome::NeedsReplacementChoice(chooser) = - discard_caused_by_effect_with_source_and_frame( + // 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; + 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 @@ -738,21 +838,27 @@ mod random_discard_authority_tests { .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, - PlayerId(0), - ObjectId(500), - 2, - hand.clone(), - None, - &mut events, - ); + 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"); @@ -767,15 +873,7 @@ mod random_discard_authority_tests { let pick = |seed: u64| { let (mut state, hand) = hand_of(seed, 6); let mut events = Vec::new(); - discard_at_random( - &mut state, - PlayerId(0), - ObjectId(500), - 3, - hand.clone(), - None, - &mut events, - ); + discard_at_random(&mut state, request(3, hand.clone()), &mut events); discarded(&state, &hand) }; assert_eq!( @@ -793,15 +891,7 @@ mod random_discard_authority_tests { let pick = |seed: u64| { let (mut state, hand) = hand_of(seed, 8); let mut events = Vec::new(); - discard_at_random( - &mut state, - PlayerId(0), - ObjectId(500), - 3, - hand.clone(), - None, - &mut events, - ); + 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) @@ -816,6 +906,157 @@ mod random_discard_authority_tests { ); } + /// 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 @@ -824,15 +1065,7 @@ mod random_discard_authority_tests { 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, - PlayerId(0), - ObjectId(500), - 5, - hand.clone(), - None, - &mut events, - ); + 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); } @@ -948,7 +1181,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/engine.rs b/crates/engine/src/game/engine.rs index 87050a07e4..dcfa96782f 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -5758,6 +5758,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 @@ -5781,6 +5782,7 @@ pub(crate) fn drain_pending_cost_move_resume( | PendingCostMoveResume::ActivationMillPayment { .. } | PendingCostMoveResume::LoyaltyActivation { .. } | PendingCostMoveResume::CounterAdditionUnlessPayment { .. } + | PendingCostMoveResume::RandomDiscardUnlessPayment { .. } ) ), CostMoveDrainBoundary::PriorityBoundary => matches!( @@ -5861,6 +5863,19 @@ 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 701.9b + CR 616.1: same Delivered/Prevented -> Paid/Failed mapping + // as the counter-addition sibling directly above; a delivered (possibly + // redirected) discard counts as paid, a fully prevented one cannot pay + // a cost (CR 118.3). + engine_payment_choices::resume_random_discard_unless_payment( + state, + events, + matches!(boundary, CostMoveDrainBoundary::ReplacementDelivered { .. }), + )? } else { unreachable!("eligible cost-move root must remain parked") }; @@ -16087,6 +16102,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`/`:5785`/ + // `:5865` (+1/+1/+13 = +15, zero deletions), i.e. entirely ABOVE this + // producer; predicted `12004+15` equals the observed coordinate exactly. + // They add the `RandomDiscardUnlessPayment` resume to the two drain + // eligibility lists 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 @@ -16426,7 +16463,7 @@ mod stage2_injector_tests { // // SET PRESERVATION: unchanged. Upstream adds no line matching the needle to this file and // neither does this branch — total still 37, partition still 5/7/25. - "game/engine.rs:12004".to_string(), + "game/engine.rs:12019".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 095cc18ded..4c61fc3ef5 100644 --- a/crates/engine/src/game/engine_payment_choices.rs +++ b/crates/engine/src/game/engine_payment_choices.rs @@ -821,20 +821,51 @@ pub(super) fn handle_unless_payment( // 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, - player, - pending_effect.source_id, - count as usize, - hand_cards, - None, + 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; its - // cursor owns the continuation. Do not clobber it and - // do not treat the pause as a declined payment. - crate::game::effects::discard::RandomDiscardOutcome::NeedsReplacementChoice => { + // 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 { + cost: poll_cost.clone(), + source_id: pending_effect.source_id, + pending_effect: pending_effect.clone(), + trigger_event: trigger_event.clone(), + effect_description: effect_description.clone(), + remaining: remaining.clone(), + payer: player, + remaining_eligible, + remaining_count: remaining_count as u32, + }, + ))); return Ok(action_result(events, state.waiting_for.clone())); } } @@ -1942,6 +1973,101 @@ 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. +/// +/// `delivered` is the boundary the replacement pipeline resolved to, mapped the +/// same way `resume_counter_addition_unless_payment` maps it: +/// +/// * `ReplacementDelivered` — the card moved (possibly redirected, e.g. Library +/// of Leng putting it on top of the library instead of the graveyard). CR +/// 701.9a: it was still discarded, so that pick counts as paid and the batch +/// continues with the picks it still owes. +/// * `ReplacementPrevented` — nothing moved. CR 118.3 forbids partial payment, +/// so the cost is unpayable; abandon the rest of the batch and let the +/// unless-effect happen. +/// +/// Either way the payment is settled exactly once through the same +/// `finish_unless_payment` tail every other unless-cost shape uses. +pub(super) fn resume_random_discard_unless_payment( + state: &mut GameState, + events: &mut Vec, + delivered: bool, +) -> 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 { + cost, + pending_effect, + trigger_event, + effect_description, + remaining, + payer, + source_id, + remaining_eligible, + remaining_count, + } = *parked; + + let mut payment_succeeded = delivered; + if delivered && 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 { + cost, + pending_effect, + trigger_event, + effect_description, + remaining, + payer, + source_id, + remaining_eligible, + remaining_count: remaining_count as u32, + }, + ))); + return Ok(state.waiting_for.clone()); + } + } + payment_succeeded = true; + } + + finish_unless_payment( + state, + true, + !payment_succeeded, + cost, + pending_effect, + trigger_event, + effect_description, + remaining, + None, + events, + )?; + Ok(state.waiting_for.clone()) +} + pub(super) fn handle_ward_sacrifice_choice( state: &mut GameState, waiting_for: WaitingFor, diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 0c05d7b055..1c0f042df1 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -3392,7 +3392,10 @@ fn parse_unless_discard_cost_phrase(branch_text: &str) -> Option { if rest.is_empty() { return Some(discard(None, CardSelectionMode::Chosen)); } - if tag::<_, _, OracleError<'_>>("at random") + // 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() { diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index e27ed4174a..d56f88f8ef 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -13589,6 +13589,28 @@ fn unless_discard_cost_phrase_lowers_random_discard_as_random() { ); } +/// 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 diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 5db8d5e62a..54babb4914 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -6257,6 +6257,56 @@ 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 { + #[serde(deserialize_with = "crate::types::ability::deserialize_ability_cost_compat")] + pub cost: AbilityCost, + pub pending_effect: Box, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub trigger_event: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub effect_description: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub remaining: Vec, + /// 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 From eb4bf0081cf95c20a204a05f4573eff9dc7e2538 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:44:20 -0500 Subject: [PATCH 4/7] test(PR-7320): drive the random-discard replacement continuation end-to-end Review asked for runtime coverage proving the new PendingCostMoveResume::RandomDiscardUnlessPayment actually settles its payment across a replacement choice. The prior tests stopped at the helper's cursor shape and uninterrupted payment, so the dispatch arms were unexercised through apply(). Adds three GameRunner tests driving a real replacement choice during Balduvian Horde's random unless-discard: * accepted redirect - the card is exiled instead of hitting the graveyard (still discarded, CR 701.9c), the preserved payment resumes, and the Horde survives * declined redirect - the natural hand->graveyard move happens and the payment resumes identically; both branches of the choice must reach the same continuation * reach-guard - an empty hand makes the cost unpayable, nothing parks, and the Horde is sacrificed, so the two above cannot pass merely because the Horde survives by default The first two assert pending_cost_move_resume is POPULATED while the choice is open and DRAINED afterward, so they fail if the continuation is removed. FIXTURE CHOICE is forced by reachability. 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, 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 use an optional Rest-in-Peace class graveyard redirect. TWO BRANCHES REMAIN UNTESTED, deliberately and disclosed rather than papered over: * ReplacementPrevented - believed unreachable for a hand->graveyard move. Every ApplyResult::Prevented path in replacement.rs is damage prevention, regeneration, a destroy-shield, or counter-placement prevention; none can apply to this event, and at that gate a Prevented result returns Complete without pausing. The arm is KEPT: it is the rules-correct mapping if the shape ever becomes reachable (CR 118.3 - a prevented discard cannot pay), and dropping it from the eligibility list would strand a parked continuation instead of draining it. Writing a synthetic test that manufactures a state the game cannot produce would assert nothing. * the remaining_eligible/remaining_count re-park - needs a MULTI-card random unless-discard to express a second pause mid-remainder. Balduvian Horde discards one card, and no known printed card pairs a random discard with a count above one in an unless-cost. Co-Authored-By: Claude Opus 5 --- crates/engine/tests/integration/main.rs | 1 + .../random_discard_cost_replacement_resume.rs | 283 ++++++++++++++++++ 2 files changed, 284 insertions(+) create mode 100644 crates/engine/tests/integration/random_discard_cost_replacement_resume.rs diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 77d11994c7..703f504cc7 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -893,6 +893,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..c6a5345cfe --- /dev/null +++ b/crates/engine/tests/integration/random_discard_cost_replacement_resume.rs @@ -0,0 +1,283 @@ +//! 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.12a: the batch parks on the replacement choice, and +/// ACCEPTING it (the card is redirected to exile — still discarded per +/// CR 701.9c) 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"); + + runner + .act(GameAction::ChooseReplacement { index: accept_idx }) + .expect("accepting the redirect must be accepted"); + runner.advance_until_stack_empty(); + + assert_eq!( + moved_out_of_hand(&runner, &hand), + 1, + "exactly one card left the hand as the payment" + ); + 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" + ); + 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" + ); +} From e6cb558327dc1568aa87ca4e4cf8b27e9d98bb91 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Thu, 13 Aug 2026 22:02:26 -0500 Subject: [PATCH 5/7] fix(PR-7320): settle the resumed random-discard payment as PAID Both review blockers were verified against the code before changing anything; both were real. 1. A DELIVERED REPLACEMENT RESUMED THROUGH THE UNPAID EPILOGUE handle_unless_payment returns EARLY through finish_successful_unless_payment when the payment succeeds (engine_payment_choices.rs, the `if !payment_failed` arm); only the failed/declined path falls through to finish_unless_payment, whose entire body is gated on `!pay || payment_failed`. The resume called that decline tail with payment_failed = false, so on success it skipped everything the paid epilogue does: the EffectResolved event, the IfAPlayerDoes alternative-outcome sub, and the SequentialSibling chain. Balduvian Horde's body has none of those, which is exactly why the existing tests passed over it. Now routed through finish_successful_unless_payment, and the accepted- replacement test asserts EffectResolved is emitted on resume - that event is produced ONLY by the paid epilogue, so it is the discriminator that catches this specific regression. 2. A PREVENTED REPLACEMENT WAS TREATED AS DECLINING THE PAYMENT CR 118.12 (docs/MagicCompRules.txt:1031): 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, and the up-front eligible-hand check already established the CR 118.3 resources, before any replacement was consulted. A redirect and a prevention alike leave that choice intact. The old Delivered->Paid / Prevented->Failed mapping was copied from resume_counter_addition_unless_payment rather than derived from the rule; under it, an applicable replacement preventing the first move would sacrifice Balduvian Horde out from under a player who had paid. The `delivered` parameter is therefore REMOVED, not merely re-mapped: the boundary no longer participates in the decision. Both drain boundaries settle identically. ReplacementPrevented stays in the eligibility list purely so a parked continuation is DRAINED rather than stranded there. Consequence: RandomDiscardUnlessPaymentResume no longer needs `cost`, `effect_description` or `remaining` - the paid epilogue does not take them, and the CR 118.12a APNAP poll correctly stops once a player pays. Dropping them also shrinks the boxed payload. Verification: 18,993 lib + 4,908 integration green; `clippy -p phase-engine --all-targets -D warnings` clean. (A workspace-wide clippy run fails in crates/probe-pin on `std::os::unix` under Windows - that crate arrived with main, is unrelated to this change, and compiles on the Linux CI runners.) Co-Authored-By: Claude Opus 5 --- crates/engine/src/game/engine.rs | 18 +++--- .../engine/src/game/engine_payment_choices.rs | 61 ++++++++----------- crates/engine/src/types/game_state.rs | 6 -- .../random_discard_cost_replacement_resume.rs | 19 +++++- 4 files changed, 51 insertions(+), 53 deletions(-) diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index e9a88b7a7b..b91b7b73ee 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -5867,15 +5867,15 @@ pub(crate) fn drain_pending_cost_move_resume( state.pending_cost_move_resume, Some(PendingCostMoveResume::RandomDiscardUnlessPayment { .. }) ) { - // CR 701.9b + CR 616.1: same Delivered/Prevented -> Paid/Failed mapping - // as the counter-addition sibling directly above; a delivered (possibly - // redirected) discard counts as paid, a fully prevented one cannot pay - // a cost (CR 118.3). - engine_payment_choices::resume_random_discard_unless_payment( - state, - events, - matches!(boundary, CostMoveDrainBoundary::ReplacementDelivered { .. }), - )? + // CR 118.12: unlike the counter-addition sibling directly above, the + // boundary is deliberately NOT passed in. The "if they do / don't" + // clause checks whether the player CHOSE to pay "regardless of what + // events actually occurred", and that choice was made — with the + // CR 118.3 resources already verified — before any replacement was + // consulted. Both boundaries therefore settle identically; + // `ReplacementPrevented` stays in the eligibility list above purely so + // a parked continuation is drained rather than stranded. + engine_payment_choices::resume_random_discard_unless_payment(state, events)? } else { unreachable!("eligible cost-move root must remain parked") }; diff --git a/crates/engine/src/game/engine_payment_choices.rs b/crates/engine/src/game/engine_payment_choices.rs index 4c61fc3ef5..3395cbb502 100644 --- a/crates/engine/src/game/engine_payment_choices.rs +++ b/crates/engine/src/game/engine_payment_choices.rs @@ -855,12 +855,9 @@ pub(super) fn handle_unless_payment( state.pending_cost_move_resume = Some(PendingCostMoveResume::RandomDiscardUnlessPayment(Box::new( crate::types::game_state::RandomDiscardUnlessPaymentResume { - cost: poll_cost.clone(), source_id: pending_effect.source_id, pending_effect: pending_effect.clone(), trigger_event: trigger_event.clone(), - effect_description: effect_description.clone(), - remaining: remaining.clone(), payer: player, remaining_eligible, remaining_count: remaining_count as u32, @@ -1976,23 +1973,26 @@ pub(super) fn resume_counter_addition_unless_payment( /// CR 701.9b + CR 118.12 + CR 616.1: Resume a RANDOM unless-discard after the /// replacement choice that paused it settled. /// -/// `delivered` is the boundary the replacement pipeline resolved to, mapped the -/// same way `resume_counter_addition_unless_payment` maps it: +/// 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. /// -/// * `ReplacementDelivered` — the card moved (possibly redirected, e.g. Library -/// of Leng putting it on top of the library instead of the graveyard). CR -/// 701.9a: it was still discarded, so that pick counts as paid and the batch -/// continues with the picks it still owes. -/// * `ReplacementPrevented` — nothing moved. CR 118.3 forbids partial payment, -/// so the cost is unpayable; abandon the rest of the batch and let the -/// unless-effect happen. +/// 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. /// -/// Either way the payment is settled exactly once through the same -/// `finish_unless_payment` tail every other unless-cost shape uses. +/// Both drain boundaries therefore land here and settle identically; the +/// `ReplacementPrevented` arm remains in the eligibility list purely so a parked +/// continuation is DRAINED rather than stranded at that boundary. pub(super) fn resume_random_discard_unless_payment( state: &mut GameState, events: &mut Vec, - delivered: bool, ) -> Result { let Some(PendingCostMoveResume::RandomDiscardUnlessPayment(parked)) = state.pending_cost_move_resume.take() @@ -2000,19 +2000,15 @@ pub(super) fn resume_random_discard_unless_payment( unreachable!("random-discard unless-payment resume requires its typed continuation") }; let crate::types::game_state::RandomDiscardUnlessPaymentResume { - cost, pending_effect, trigger_event, - effect_description, - remaining, payer, source_id, remaining_eligible, remaining_count, } = *parked; - let mut payment_succeeded = delivered; - if delivered && remaining_count > 0 { + 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. @@ -2036,11 +2032,8 @@ pub(super) fn resume_random_discard_unless_payment( state.pending_cost_move_resume = Some(PendingCostMoveResume::RandomDiscardUnlessPayment(Box::new( crate::types::game_state::RandomDiscardUnlessPaymentResume { - cost, pending_effect, trigger_event, - effect_description, - remaining, payer, source_id, remaining_eligible, @@ -2050,22 +2043,16 @@ pub(super) fn resume_random_discard_unless_payment( return Ok(state.waiting_for.clone()); } } - payment_succeeded = true; } - finish_unless_payment( - state, - true, - !payment_succeeded, - cost, - pending_effect, - trigger_event, - effect_description, - remaining, - None, - events, - )?; - 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( diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 54699a7f9c..782e142f7e 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -6341,15 +6341,9 @@ pub enum PendingCostMoveResume { /// `GameState` — inside its stack budget. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct RandomDiscardUnlessPaymentResume { - #[serde(deserialize_with = "crate::types::ability::deserialize_ability_cost_compat")] - pub cost: AbilityCost, pub pending_effect: Box, #[serde(default, skip_serializing_if = "Option::is_none")] pub trigger_event: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub effect_description: Option, - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub remaining: Vec, /// The paying player — the unless-payer, not necessarily the ability's /// controller. pub payer: PlayerId, diff --git a/crates/engine/tests/integration/random_discard_cost_replacement_resume.rs b/crates/engine/tests/integration/random_discard_cost_replacement_resume.rs index c6a5345cfe..c00271c9f9 100644 --- a/crates/engine/tests/integration/random_discard_cost_replacement_resume.rs +++ b/crates/engine/tests/integration/random_discard_cost_replacement_resume.rs @@ -169,9 +169,26 @@ fn random_discard_cost_resumes_its_payment_after_an_accepted_replacement() { .position(|c| c.description == "Accept") .expect("an Accept option"); - runner + 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!( From d1a8100d2b66ebb79da8f48178b2474b2b5d6d0d Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 21:04:29 -0700 Subject: [PATCH 6/7] fix(PR-7320): retain random unless payment on elimination --- crates/engine/src/game/elimination.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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); From 238fee03ede5202aac11a417bf1bd238396d3fcf Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 22:05:41 -0700 Subject: [PATCH 7/7] fix(PR-7320): resume random discard after replacement delivery --- crates/engine/src/game/engine.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 1c160d9ae2..eefce47b75 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -5758,6 +5758,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