From f2736b3902f3134f620c4cd15bbf5e63b0abd5f6 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Tue, 11 Aug 2026 20:21:57 -0500 Subject: [PATCH 1/3] ENGINE: Support Court of Ambition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Court of Ambition's monarch rider silently did nothing. The first sentence ("each opponent loses 3 life unless they discard a card") already worked, but the second ("If you're the monarch, instead each opponent loses 6 life unless they discard two cards") lowered to Effect::Unimplemented { name: "Unsupported unless clause" }. Root cause: two hand-written mirrors of the same grammar drifted. parse_unless_discard_cost (the "you" payer) had grown a numeric-count axis; parse_unless_they_discard_cost (the anaphoric-player payer) had not — it hard-coded count: 1 and only accepted the singular article. Every card printing a plural discard against a "they" / "that player" payer fell out of the grammar entirely. Collapse both into one authority, parse_unless_discard_cost_phrase, with the count and type axes composed rather than enumerated: [ | "a" | "an"] [] ("card" | "cards") ["at random"] so "discard two nonland cards" needs no new arm and the two payer forms cannot diverge again. parse_number spans both the numeral and the article, so one call covers the whole count axis. Two deliberate calls, both documented at the authority: * CR 118.12a — fail closed on a zero count. parse_number folds a bare X to 0, and a zero-card unless-cost is free, so a punisher would silently never fire. An unresolvable count stays visible as an unsupported clause instead. * CR 701.9b — an "at random" tail is accepted but deliberately does NOT set CardSelectionMode::Random. The resolution-time unless-payment path (engine_payment_choices.rs) discards the selection field and always prompts, so emitting Random would claim behavior the engine does not implement. This preserves both mirrors' pre-existing mapping exactly. The runtime plumbing this card needs already existed: the per-opponent player_scope fan-out, the ScopedPlayer unless-payer arm, and apply_instead_swap preserving player_scope / unless_pay across the CR 614.15 self-replacement swap. Court of Ambition now parses to BecomeMonarch on the ETB plus an upkeep trigger whose base branch is LoseLife 3 / Discard 1 and whose ConditionInstead { IsMonarch } rider is LoseLife 6 / Discard 2, both scoped Opponent with a ScopedPlayer payer. No Unimplemented, no parse warnings. Tests: four parser building-block tests (count x type axes across both payers, the "or"-chain branch boundary, the zero-count fail-closed guard, and the full Court of Ambition AST) and seven runtime tests driving the real upkeep trigger — decline, pay, monarch decline (6, not 3 and not 9), monarch pay, unpayable with one card in hand, a three-player game where one opponent pays and another declines independently, and the ETB monarch grant via an actual cast. Every CR citation verified against docs/MagicCompRules.txt. Co-Authored-By: Claude Opus 5 --- crates/engine/src/parser/oracle_trigger.rs | 179 ++++----- .../engine/src/parser/oracle_trigger_tests.rs | 184 +++++++++ .../tests/integration/court_of_ambition.rs | 370 ++++++++++++++++++ crates/engine/tests/integration/main.rs | 1 + 4 files changed, 638 insertions(+), 96 deletions(-) create mode 100644 crates/engine/tests/integration/court_of_ambition.rs diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 854c01d1a9..c953575298 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -3294,7 +3294,7 @@ fn parse_inferred_pronoun_unless_alt_cost( let cost = if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("they discard ").parse(after_unless) { - parse_unless_discard_cost(rest)? + parse_unless_discard_cost_phrase(rest)? } else if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("they pay ").parse(after_unless) { parse_unless_life_cost(rest)? } else { @@ -3319,64 +3319,88 @@ fn parse_unless_life_cost(rest: &str) -> Option { None } -fn parse_unless_discard_cost(discard_tail: &str) -> Option { - let trailing = discard_tail.trim().trim_end_matches('.').trim(); +/// CR 701.9 + CR 118.12: SINGLE AUTHORITY for the discard alternative-cost +/// phrase that follows an unless-clause's `discard(s)` verb. +/// +/// Grammar — two independent axes over one noun: +/// +/// ```text +/// discard_phrase := [ | "a" | "an"] [] ("card" | "cards") ["at random"] +/// ``` +/// +/// Both unless-payer forms route here: the controller form ("unless **you** +/// discard two cards", from `parse_unless_alt_cost` / +/// `parse_inferred_pronoun_unless_alt_cost`) and the anaphoric-player form +/// ("unless **they** discard two cards" — Court of Ambition, +/// `parse_unless_they_discard_cost`). They were hand-written mirrors that +/// drifted — only the controller form ever grew the count axis, so every card +/// printing a plural discard against an anaphoric payer fell out of the grammar +/// and surfaced as `Unimplemented`. One authority means the count and type axes +/// cannot diverge by payer again, and it composes them: "discard two nonland +/// cards" needs no new arm. +/// +/// `branch_text` must already be bounded by the caller — the `they` form stops +/// 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") is accepted as an "at +/// random" tail but deliberately does NOT set +/// `CardSelectionMode::Random`: the resolution-time unless-payment path +/// (`engine_payment_choices.rs`) discards `selection` and always prompts, so +/// emitting `Random` would claim a behavior the engine does not implement. +/// Preserves the pre-existing mapping of both mirrors exactly. +fn parse_unless_discard_cost_phrase(branch_text: &str) -> Option { + let trimmed = branch_text.trim().trim_end_matches('.').trim(); + if trimmed.is_empty() { + return None; + } - // CR 118.12 + CR 701.9: Try numeric count first ("two cards", "three cards"), - // then fall back to article form ("a card", "an enchantment card"). - if let Some((n, after_num)) = parse_number(trailing) { - let after_num = after_num.trim(); - if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("cards").parse(after_num) { - let rest = rest.trim().trim_end_matches('.').trim(); - if rest.is_empty() - || tag::<_, _, OracleError<'_>>("at random") - .parse(rest) - .is_ok() - { - return Some(AbilityCost::Discard { - count: QuantityExpr::Fixed { value: n as i32 }, - filter: None, - selection: crate::types::ability::CardSelectionMode::Chosen, - self_scope: crate::types::ability::DiscardSelfScope::FromHand, - }); - } - } + // CR 701.9: the count axis. `parse_number` spans both the numeral ("two + // cards") and the singular article ("a card" / "an enchantment card"), so + // one call covers the whole axis; a bare noun ("card") has neither and + // counts as one. + let (count, after_count) = parse_number(trimmed).unwrap_or((1, trimmed)); + let after_count = after_count.trim(); + + // CR 118.12a: fail closed on a zero count. `parse_number` folds a bare `X` + // to 0 for the callers that want a numeric-only reading, but an unless-cost + // of "discard 0 cards" is FREE — the punisher would silently never fire. + // No printed card announces X inside an unless-discard, so an unresolvable + // count must stay visible as an unsupported clause rather than lower to a + // cost every player can always pay. + if count == 0 { + return None; } - let trailing = alt(( - tag::<_, _, OracleError<'_>>("a "), - tag::<_, _, OracleError<'_>>("an "), - )) - .parse(trailing) - .map(|(rest, _)| rest.trim()) - .unwrap_or(trailing); - if !trailing.is_empty() { - if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("card").parse(trailing) { - let rest = rest.trim().trim_end_matches('.').trim(); - if rest.is_empty() - || tag::<_, _, OracleError<'_>>("at random") - .parse(rest) - .is_ok() - { - return Some(AbilityCost::Discard { - count: QuantityExpr::Fixed { value: 1 }, - filter: None, - selection: crate::types::ability::CardSelectionMode::Chosen, - self_scope: crate::types::ability::DiscardSelfScope::FromHand, - }); - } - } - if let Some(filter) = super::oracle_effect::imperative::parse_discard_card_filter(trailing) + let discard = |filter| AbilityCost::Discard { + count: QuantityExpr::Fixed { + value: count as i32, + }, + filter, + selection: crate::types::ability::CardSelectionMode::Chosen, + self_scope: crate::types::ability::DiscardSelfScope::FromHand, + }; + + // Untyped noun: the count axis alone ("a card", "two cards"). The plural + // arm precedes the singular so `tag("card")` cannot leave a stray "s". + 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() + || tag::<_, _, OracleError<'_>>("at random") + .parse(rest) + .is_ok() { - return Some(AbilityCost::Discard { - count: QuantityExpr::Fixed { value: 1 }, - filter: Some(filter), - selection: crate::types::ability::CardSelectionMode::Chosen, - self_scope: crate::types::ability::DiscardSelfScope::FromHand, - }); + return Some(discard(None)); } } - None + + // 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). + super::oracle_effect::imperative::parse_discard_card_filter(after_count) + .map(|filter| discard(Some(filter))) } /// CR 118.12 + CR 608.2c + CR 119.4: Recognize non-mana "unless" alternative @@ -3423,7 +3447,7 @@ pub(crate) fn parse_unless_alt_cost(after_unless: &str) -> Option { // ("at random", trailing punctuation) since the caller strips the entire // unless-clause wholesale. if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("you discard ").parse(after_unless) { - return parse_unless_discard_cost(rest); + return parse_unless_discard_cost_phrase(rest); } // "you pay N life" / "you pay N life." — life amount is bare integer. @@ -3835,52 +3859,15 @@ fn parse_unless_they_sacrifice_filter(input: &str) -> Option<(AbilityCost, &str) } /// CR 701.9 + CR 118.12: Parse the tail of "they discard ..." preserving -/// the unconsumed remainder. Mirrors `parse_unless_discard_cost` but stops -/// at the branch boundary. +/// the unconsumed remainder. Bounds the branch at `unless_branch_boundary` so a +/// chained " or …" branch survives, then defers the phrase grammar itself to +/// the shared `parse_unless_discard_cost_phrase` authority — the `you` and +/// `they` payer axes share one vocabulary. fn parse_unless_they_discard_cost(input: &str) -> Option<(AbilityCost, &str)> { let boundary = unless_branch_boundary(input); let branch_text = input[..boundary].trim(); let after = &input[boundary..]; - if branch_text.is_empty() { - return None; - } - // Strip article - let stripped = alt((tag::<_, _, OracleError<'_>>("a "), tag("an "))) - .parse(branch_text) - .map(|(rest, _)| rest) - .unwrap_or(branch_text); - // Plain "card" / "card at random" - if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("card").parse(stripped) { - let rest = rest.trim(); - if rest.is_empty() - || tag::<_, _, OracleError<'_>>("at random") - .parse(rest) - .is_ok() - { - return Some(( - AbilityCost::Discard { - count: QuantityExpr::Fixed { value: 1 }, - filter: None, - selection: crate::types::ability::CardSelectionMode::Chosen, - self_scope: crate::types::ability::DiscardSelfScope::FromHand, - }, - after, - )); - } - } - // Typed filter ("nonland card", "creature card", etc.) - if let Some(filter) = super::oracle_effect::imperative::parse_discard_card_filter(stripped) { - return Some(( - AbilityCost::Discard { - count: QuantityExpr::Fixed { value: 1 }, - filter: Some(filter), - selection: crate::types::ability::CardSelectionMode::Chosen, - self_scope: crate::types::ability::DiscardSelfScope::FromHand, - }, - after, - )); - } - None + parse_unless_discard_cost_phrase(branch_text).map(|cost| (cost, after)) } /// CR 119.4 + CR 118.12: Parse the tail of "they pay N life" preserving diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 9b4cafb231..2fc9a88f1f 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -13342,6 +13342,190 @@ fn trigger_unless_they_discard_multi_sentence_branch_not_terminal_cost() { ); } +/// CR 701.9 + CR 118.12: the discard unless-cost's COUNT axis, exercised across +/// both payer forms of the shared `parse_unless_discard_cost_phrase` authority. +/// The `they` form used to lack the axis entirely, so anything but "a card" +/// failed to lower; the two forms must now accept the identical vocabulary. +#[test] +fn unless_discard_cost_phrase_spans_count_and_type_axes_for_both_payers() { + fn discard(count: i32, filter: Option) -> AbilityCost { + AbilityCost::Discard { + count: QuantityExpr::Fixed { value: count }, + filter, + selection: CardSelectionMode::Chosen, + self_scope: DiscardSelfScope::FromHand, + } + } + // The type axis is owned by the shared `parse_discard_card_filter` + // authority; this test's claim is that the COUNT axis composes with it, not + // what that authority lowers "nonland" to — so read the expected filter from + // the authority rather than restating its grammar here. + let nonland = + crate::parser::oracle_effect::imperative::parse_discard_card_filter("nonland cards") + .expect("the shared filter authority types 'nonland cards'"); + + // (phrase, expected cost) — the count axis (article / numeral) crossed with + // the type axis (bare noun / type phrase), plus the CR 701.9b random tail. + let cases: [(&str, AbilityCost); 5] = [ + ("a card", discard(1, None)), + ("two cards", discard(2, None)), + ("three cards", discard(3, None)), + ("a card at random", discard(1, None)), + ("two nonland cards", discard(2, Some(nonland))), + ]; + + for (phrase, expected) in cases { + // `they` payer form — must stop at the branch boundary and lower the phrase. + let (they_cost, rest) = parse_unless_they_discard_cost(phrase) + .unwrap_or_else(|| panic!("`they discard {phrase}` must lower")); + assert_eq!(they_cost, expected, "they-payer cost for {phrase:?}"); + assert!( + rest.trim().is_empty(), + "whole branch should be consumed for {phrase:?}, left {rest:?}" + ); + + // `you` payer form — same vocabulary, same lowering. + let you_cost = parse_unless_alt_cost(&format!("you discard {phrase}")) + .unwrap_or_else(|| panic!("`you discard {phrase}` must lower")); + assert_eq!(you_cost, expected, "you-payer cost for {phrase:?}"); + } +} + +/// CR 118.12a: an unresolvable count must fail closed. `parse_number` folds a +/// bare `X` to 0, and a zero-card discard is a cost every player can always pay +/// — the punisher would silently never fire. The clause must stay unlowered so +/// coverage reports it honestly instead. +#[test] +fn unless_discard_cost_phrase_rejects_zero_count() { + assert!( + parse_unless_they_discard_cost("x cards").is_none(), + "an X-count unless-discard must not lower to a free cost" + ); + assert!( + parse_unless_alt_cost("you discard x cards").is_none(), + "the controller form must fail closed on the same input" + ); +} + +/// CR 118.12a: a plural discard branch must still leave a chained " or …" +/// branch for the disjunction combinator — the count axis must not swallow it. +#[test] +fn unless_they_discard_plural_keeps_chained_or_branch() { + let cost = parse_unless_they_alt_cost_chain("they discard two cards or pay 5 life") + .expect("disjunctive chain should lower"); + let AbilityCost::OneOf { costs } = &cost else { + panic!("expected OneOf, got {cost:?}"); + }; + assert_eq!(costs.len(), 2, "both branches should survive: {costs:?}"); + assert!( + matches!( + costs[0], + AbilityCost::Discard { + count: QuantityExpr::Fixed { value: 2 }, + .. + } + ), + "first branch should be a two-card discard, got {:?}", + costs[0] + ); + assert!( + matches!( + costs[1], + AbilityCost::PayLife { + amount: QuantityExpr::Fixed { value: 5 } + } + ), + "second branch should survive the plural first branch, got {:?}", + costs[1] + ); +} + +/// CR 608.2c + CR 614.15 + CR 725.1: Court of Ambition's upkeep trigger is a +/// per-opponent punisher whose monarch rider replaces BOTH the life loss and its +/// unless-cost. Both branches must carry their own `unless_pay` scoped to the +/// iterating opponent, and the rider must be a `ConditionInstead` swap (an +/// additive sub would make a monarch controller drain 3 AND 6). +#[test] +fn court_of_ambition_monarch_branch_carries_its_own_scoped_unless_cost() { + fn scoped_discard(unless: &UnlessPayModifier, expected_count: i32) { + assert_eq!( + unless.payer, + TargetFilter::ScopedPlayer, + "each opponent pays for their own iteration" + ); + assert!( + matches!( + unless.cost, + AbilityCost::Discard { + count: QuantityExpr::Fixed { value } , + filter: None, + .. + } if value == expected_count + ), + "expected a {expected_count}-card discard, got {:?}", + unless.cost + ); + } + + let def = parse_trigger_line( + "At the beginning of your upkeep, each opponent loses 3 life unless they discard a card. If you're the monarch, instead each opponent loses 6 life unless they discard two cards.", + "Court of Ambition", + ); + let execute = def.execute.as_ref().expect("should have execute"); + + assert!( + matches!( + *execute.effect, + Effect::LoseLife { + amount: QuantityExpr::Fixed { value: 3 }, + .. + } + ), + "base branch should lose 3 life, got {:?}", + execute.effect + ); + assert_eq!(execute.player_scope, Some(PlayerFilter::Opponent)); + scoped_discard( + execute + .unless_pay + .as_ref() + .expect("base branch must keep its unless cost"), + 1, + ); + + let rider = execute + .sub_ability + .as_ref() + .expect("monarch rider should be chained"); + assert!( + matches!( + rider.condition, + Some(AbilityCondition::ConditionInstead { ref inner }) if matches!(**inner, AbilityCondition::IsMonarch) + ), + "rider must REPLACE the base branch, got {:?}", + rider.condition + ); + assert!( + matches!( + *rider.effect, + Effect::LoseLife { + amount: QuantityExpr::Fixed { value: 6 }, + .. + } + ), + "monarch branch should lose 6 life, got {:?}", + rider.effect + ); + assert_eq!(rider.player_scope, Some(PlayerFilter::Opponent)); + scoped_discard( + rider + .unless_pay + .as_ref() + .expect("monarch branch must carry its own unless cost"), + 2, + ); +} + #[test] fn trigger_unless_pay_for_each_uses_dynamic_generic_cost() { let def = parse_trigger_line( diff --git a/crates/engine/tests/integration/court_of_ambition.rs b/crates/engine/tests/integration/court_of_ambition.rs new file mode 100644 index 0000000000..06c3c66d7c --- /dev/null +++ b/crates/engine/tests/integration/court_of_ambition.rs @@ -0,0 +1,370 @@ +//! Court of Ambition — the monarch-gated per-opponent punisher. +//! +//! Oracle text (verbatim, Scryfall `cmr` #114): +//! "When this enchantment enters, you become the monarch. +//! At the beginning of your upkeep, each opponent loses 3 life unless they +//! discard a card. If you're the monarch, instead each opponent loses 6 life +//! unless they discard two cards." +//! +//! Two axes meet on this card and neither works without the other: +//! +//! 1. **Per-opponent unless-costs** (CR 118.12a + CR 608.2f). The upkeep +//! trigger fans out over `player_scope: Opponent`, and each iteration's +//! unless-payer is that iteration's scoped opponent — so every opponent +//! independently chooses to discard or to lose the life. A payer bound to +//! the controller (or to only the first opponent) would let one decision +//! speak for the table. +//! +//! 2. **The monarch "instead" swap** (CR 614.15 + CR 608.2c + CR 725.1). The +//! rider replaces BOTH halves of the printed instruction — the life total +//! AND the unless-cost. An additive rider would drain 3 *and* 6, and a +//! rider that swapped only the life amount would still ask for one card. +//! +//! ROOT CAUSE this file pins: `parse_unless_they_discard_cost` hard-coded a +//! count of one and only accepted the singular article, while its `you`-payer +//! mirror had grown a numeric-count axis. "unless they discard two cards" fell +//! out of the grammar entirely, so the whole monarch branch lowered to +//! `Effect::Unimplemented { name: "Unsupported unless clause" }` — the monarch +//! half of the card silently did nothing. Both payer forms now share one +//! authority (`parse_unless_discard_cost_phrase`). +//! +//! These tests drive the REAL pipeline: the card is built from its verbatim +//! Oracle text and the upkeep trigger is fired by advancing the turn, so a +//! regression in parsing, fan-out, swap, or payment all surface here. + +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::types::ability::{AbilityCost, QuantityExpr}; +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::player::PlayerId; +use engine::types::zones::Zone; + +const P2: PlayerId = PlayerId(2); + +/// Verbatim printed Oracle text — the fixture must never paraphrase, because +/// the clause boundary ("… a card. If you're the monarch, instead …") is +/// precisely what routes the second sentence to the "instead" branch builder. +const COURT_OF_AMBITION: &str = "When this enchantment enters, you become the monarch.\n\ + At the beginning of your upkeep, each opponent loses 3 life unless they discard a card. \ + If you're the monarch, instead each opponent loses 6 life unless they discard two cards."; + +fn life(runner: &GameRunner, player: PlayerId) -> i32 { + runner + .state() + .players + .iter() + .find(|p| p.id == player) + .expect("player exists") + .life +} + +fn hand_size(runner: &GameRunner, player: PlayerId) -> usize { + runner + .state() + .players + .iter() + .find(|p| p.id == player) + .expect("player exists") + .hand + .len() +} + +/// `player_count`-seat game with Court of Ambition already on P0's battlefield +/// and `opponent_hand` discardable cards in every opponent's hand. `monarch` seeds +/// the designation directly — a scenario-seeded permanent never fires its own +/// ETB, so the "you become the monarch" half is covered separately by +/// `court_of_ambition_etb_makes_its_controller_the_monarch`. +fn build_runner(player_count: u8, monarch: Option, opponent_hand: usize) -> GameRunner { + let mut scenario = GameScenario::new_n_player(player_count, 42); + scenario.at_phase(Phase::Untap); + scenario.add_enchantment_from_oracle(P0, "Court of Ambition", COURT_OF_AMBITION); + + for seat in 1..player_count { + let pid = PlayerId(seat); + let names: Vec = (0..opponent_hand) + .map(|i| format!("Filler Card {seat}-{i}")) + .collect(); + let refs: Vec<&str> = names.iter().map(String::as_str).collect(); + scenario.with_cards_in_hand(pid, &refs); + } + + let mut runner = scenario.build(); + runner.state_mut().turn_number = 2; + runner.state_mut().active_player = P0; + runner.state_mut().priority_player = P0; + runner.state_mut().monarch = monarch; + runner +} + +/// Drive to P0's upkeep and let the trigger resolve until it pauses on the +/// first opponent's unless-payment prompt (CR 118.12a). +fn fire_upkeep(runner: &mut GameRunner) { + runner.advance_to_upkeep(); + for _ in 0..20 { + if matches!(runner.state().waiting_for, WaitingFor::UnlessPayment { .. }) { + return; + } + if runner.state().stack.is_empty() && runner.state().phase != Phase::Upkeep { + break; + } + if runner.act(GameAction::PassPriority).is_err() { + break; + } + } + panic!( + "upkeep trigger never surfaced an unless-payment prompt: {:?}", + runner.state().waiting_for + ); +} + +/// Assert the pending prompt is an unless-discard of `expected_count` cards +/// owed by `expected_payer`. CR 118.12a: the payer is the scoped opponent of +/// this fan-out iteration, never the controller. +fn expect_discard_prompt(runner: &GameRunner, expected_payer: PlayerId, expected_count: i32) { + match &runner.state().waiting_for { + WaitingFor::UnlessPayment { player, cost, .. } => { + assert_eq!( + *player, expected_payer, + "the scoped opponent pays their own unless-cost, not the controller" + ); + match cost { + AbilityCost::Discard { count, filter, .. } => { + assert_eq!( + *count, + QuantityExpr::Fixed { + value: expected_count + }, + "unless-cost card count" + ); + assert!( + filter.is_none(), + "any card may be discarded, got {filter:?}" + ); + } + other => panic!("expected a Discard unless-cost, got {other:?}"), + } + } + other => panic!("expected UnlessPayment prompt, got {other:?}"), + } +} + +/// Pay the pending unless-discard by submitting one card per re-prompt round +/// trip, until the discard loop is exhausted. +fn pay_discard(runner: &mut GameRunner) { + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("choosing to pay the discard cost must be accepted"); + for _ in 0..8 { + let card = match &runner.state().waiting_for { + WaitingFor::WardDiscardChoice { cards, .. } => { + *cards.first().expect("an eligible card to discard") + } + _ => return, + }; + runner + .act(GameAction::SelectCards { cards: vec![card] }) + .expect("discard selection must be accepted"); + } + panic!("discard loop did not terminate"); +} + +/// CR 118.12a: declining the discard makes the effect happen — the opponent +/// loses 3 life and keeps their hand. +#[test] +fn court_of_ambition_non_monarch_opponent_declining_loses_three() { + let mut runner = build_runner(2, None, 3); + fire_upkeep(&mut runner); + expect_discard_prompt(&runner, P1, 1); + + runner + .act(GameAction::PayUnlessCost { pay: false }) + .expect("declining must be accepted"); + runner.advance_until_stack_empty(); + + assert_eq!(life(&runner, P1), 17, "declining costs the opponent 3 life"); + assert_eq!( + hand_size(&runner, P1), + 3, + "a declined cost discards nothing" + ); + assert_eq!(life(&runner, P0), 20, "the controller is never the subject"); +} + +/// CR 118.12a: paying the discard prevents the life loss entirely. +#[test] +fn court_of_ambition_non_monarch_opponent_paying_discards_one_and_keeps_life() { + let mut runner = build_runner(2, None, 3); + fire_upkeep(&mut runner); + expect_discard_prompt(&runner, P1, 1); + + pay_discard(&mut runner); + runner.advance_until_stack_empty(); + + assert_eq!( + life(&runner, P1), + 20, + "a paid unless-cost prevents the loss" + ); + assert_eq!(hand_size(&runner, P1), 2, "exactly one card was discarded"); +} + +/// CR 614.15 + CR 725.1: while the controller is the monarch, the rider +/// REPLACES the printed instruction — the demand becomes two cards, and +/// declining costs 6 life, not 3 and not 9 (which is what an additive rider +/// would produce). +#[test] +fn court_of_ambition_monarch_branch_demands_two_cards_and_drains_six() { + let mut runner = build_runner(2, Some(P0), 3); + fire_upkeep(&mut runner); + expect_discard_prompt(&runner, P1, 2); + + runner + .act(GameAction::PayUnlessCost { pay: false }) + .expect("declining must be accepted"); + runner.advance_until_stack_empty(); + + assert_eq!( + life(&runner, P1), + 14, + "the monarch branch REPLACES the 3-life branch: 6 total, not 3 and not 9" + ); + assert_eq!( + hand_size(&runner, P1), + 3, + "a declined cost discards nothing" + ); +} + +/// CR 614.15 + CR 701.9: paying the monarch branch costs exactly two cards. +#[test] +fn court_of_ambition_monarch_branch_paid_with_two_discards_keeps_life() { + let mut runner = build_runner(2, Some(P0), 3); + fire_upkeep(&mut runner); + expect_discard_prompt(&runner, P1, 2); + + pay_discard(&mut runner); + runner.advance_until_stack_empty(); + + assert_eq!( + life(&runner, P1), + 20, + "a paid unless-cost prevents the loss" + ); + assert_eq!( + hand_size(&runner, P1), + 1, + "exactly two cards were discarded" + ); +} + +/// CR 118.3 + CR 118.12a: "A player can't pay a cost without having the +/// necessary resources to pay it fully" — an opponent who cannot produce the +/// full count cannot partially pay, so the effect happens. With the monarch +/// branch demanding two cards and only one in hand, the life loss happens and +/// the card stays put. +#[test] +fn court_of_ambition_monarch_branch_is_unpayable_with_one_card() { + let mut runner = build_runner(2, Some(P0), 1); + fire_upkeep(&mut runner); + expect_discard_prompt(&runner, P1, 2); + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("attempting an unpayable cost must be accepted"); + runner.advance_until_stack_empty(); + + assert_eq!( + life(&runner, P1), + 14, + "an unpayable cost lets the loss happen" + ); + assert_eq!( + hand_size(&runner, P1), + 1, + "the lone card must not be taken as a partial payment" + ); +} + +/// CR 608.2f + CR 101.4: every opponent is polled independently, in APNAP +/// order, and each decision binds only to that opponent. This is the assertion +/// that a controller-bound (or first-opponent-bound) payer cannot satisfy: +/// P1 pays and keeps their life, P2 declines and loses it. +#[test] +fn court_of_ambition_polls_each_opponent_independently() { + let mut runner = build_runner(3, None, 3); + fire_upkeep(&mut runner); + + // First iteration: P1, in APNAP order after the active player P0. + expect_discard_prompt(&runner, P1, 1); + pay_discard(&mut runner); + + // The fan-out continuation must now surface P2's own prompt. + for _ in 0..10 { + if matches!(runner.state().waiting_for, WaitingFor::UnlessPayment { .. }) { + break; + } + if runner.act(GameAction::PassPriority).is_err() { + break; + } + } + expect_discard_prompt(&runner, P2, 1); + runner + .act(GameAction::PayUnlessCost { pay: false }) + .expect("declining must be accepted"); + runner.advance_until_stack_empty(); + + assert_eq!(life(&runner, P1), 20, "P1 paid, so P1 loses no life"); + assert_eq!(hand_size(&runner, P1), 2, "P1 discarded exactly one card"); + assert_eq!(life(&runner, P2), 17, "P2 declined, so P2 loses 3 life"); + assert_eq!(hand_size(&runner, P2), 3, "P2 discarded nothing"); + assert_eq!(life(&runner, P0), 20, "the controller is never the subject"); +} + +/// CR 725.1: the ETB half. Cast from hand so the enters trigger actually fires +/// (a scenario-seeded permanent does not), and confirm the controller takes the +/// monarch designation. +#[test] +fn court_of_ambition_etb_makes_its_controller_the_monarch() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let court: ObjectId = scenario + .add_spell_to_hand_from_oracle(P0, "Court of Ambition", false, COURT_OF_AMBITION) + .as_enchantment() + // Printed cost {2}{B}{B}. + .with_mana_cost(ManaCost::Cost { + generic: 2, + shards: vec![ManaCostShard::Black, ManaCostShard::Black], + }) + .id(); + scenario.with_mana_pool( + P0, + (0..4) + .map(|_| ManaUnit::new(ManaType::Black, ObjectId(0), false, vec![])) + .collect(), + ); + + let mut runner = scenario.build(); + runner.state_mut().active_player = P0; + runner.state_mut().priority_player = P0; + assert!( + runner.state().monarch.is_none(), + "fixture reach-guard: nobody is the monarch before the cast" + ); + + runner.cast(court).resolve(); + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().objects[&court].zone, + Zone::Battlefield, + "the enchantment must have resolved onto the battlefield" + ); + assert_eq!( + runner.state().monarch, + Some(P0), + "the enters trigger makes its controller the monarch" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index ef07130791..c55c086ec8 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -107,6 +107,7 @@ mod counter_anaphor_binds_to_recipient; mod counter_anaphor_created_token_binding; mod counter_double_redirect_choice; mod counter_spell_zone_redirect; +mod court_of_ambition; mod court_of_cunning_multi_target_mill; mod cr605_1a_library_criterion; mod cr733_resolved_attachment; From cdc05868ff0d7377bc62b69e8df2372f3f2fbdd8 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 19:06:27 -0700 Subject: [PATCH 2/3] fix(PR-7261): preserve unless-discard fidelity --- crates/engine/src/parser/oracle_trigger.rs | 34 +++++++------------ .../engine/src/parser/oracle_trigger_tests.rs | 25 ++++++++++++-- .../tests/integration/court_of_ambition.rs | 25 ++++++++++++++ 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index c953575298..d668470118 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") ["at random"] +/// discard_phrase := [ | "a" | "an"] [] ("card" | "cards") /// ``` /// /// Both unless-payer forms route here: the controller form ("unless **you** @@ -3343,12 +3343,10 @@ 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") is accepted as an "at -/// random" tail but deliberately does NOT set -/// `CardSelectionMode::Random`: the resolution-time unless-payment path -/// (`engine_payment_choices.rs`) discards `selection` and always prompts, so -/// emitting `Random` would claim a behavior the engine does not implement. -/// Preserves the pre-existing mapping of both mirrors exactly. +/// 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. fn parse_unless_discard_cost_phrase(branch_text: &str) -> Option { let trimmed = branch_text.trim().trim_end_matches('.').trim(); if trimmed.is_empty() { @@ -3372,10 +3370,9 @@ fn parse_unless_discard_cost_phrase(branch_text: &str) -> Option { return None; } + let count = i32::try_from(count).ok()?; let discard = |filter| AbilityCost::Discard { - count: QuantityExpr::Fixed { - value: count as i32, - }, + count: QuantityExpr::Fixed { value: count }, filter, selection: crate::types::ability::CardSelectionMode::Chosen, self_scope: crate::types::ability::DiscardSelfScope::FromHand, @@ -3387,11 +3384,7 @@ fn parse_unless_discard_cost_phrase(branch_text: &str) -> Option { alt((tag::<_, _, OracleError<'_>>("cards"), tag("card"))).parse(after_count) { let rest = rest.trim().trim_end_matches('.').trim(); - if rest.is_empty() - || tag::<_, _, OracleError<'_>>("at random") - .parse(rest) - .is_ok() - { + if rest.is_empty() { return Some(discard(None)); } } @@ -3408,7 +3401,7 @@ fn parse_unless_discard_cost_phrase(branch_text: &str) -> Option { /// text immediately after `" unless "`. /// /// Patterns recognized: -/// - `you discard [N] card(s)[ at random][.]` → `AbilityCost::Discard { count, .. }` +/// - `you discard [N] card(s)[.]` → `AbilityCost::Discard { count, .. }` /// - `you sacrifice [N] [filter][.]` → `AbilityCost::Sacrifice { count, filter }` /// - `you pay N life[.]` → `AbilityCost::PayLife { amount }` /// - `you mill [N] card(s)[.]` → `AbilityCost::Mill { count }` @@ -3417,11 +3410,10 @@ fn parse_unless_discard_cost_phrase(branch_text: &str) -> Option { /// Returns `None` for any other shape (mana costs and unknown forms fall /// through to the existing mana-cost path in `extract_unless_pay_modifier`). /// -/// FIDELITY NOTE: `UnlessCost::DiscardCard` does not currently model "at random" -/// — the engine resolves via `WaitingFor::WardDiscardChoice` (player-chosen). -/// This is a known sub-fidelity gap (Balduvian Horde class). Post the -/// 2026-05-09 fold, the `random: bool` field on `AbilityCost::Discard` is -/// the natural home for this; wiring it into the runtime is future work. +/// FIDELITY NOTE: `UnlessCost::DiscardCard` does not currently implement "at +/// random" — the engine resolves via `WaitingFor::WardDiscardChoice` +/// (player-chosen). Those phrases stay unimplemented (Balduvian Horde class) +/// until the unless-payment path preserves `CardSelectionMode::Random`. pub(crate) fn parse_unless_alt_cost(after_unless: &str) -> Option { // CR 118.12 + CR 202.1: "you pay its mana cost" / "you pay ~'s mana cost" — // the unless cost is the ability source's OWN printed mana cost, which is diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 2fc9a88f1f..63f0d0f134 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -13365,12 +13365,11 @@ fn unless_discard_cost_phrase_spans_count_and_type_axes_for_both_payers() { .expect("the shared filter authority types 'nonland cards'"); // (phrase, expected cost) — the count axis (article / numeral) crossed with - // the type axis (bare noun / type phrase), plus the CR 701.9b random tail. - let cases: [(&str, AbilityCost); 5] = [ + // the type axis (bare noun / type phrase). + let cases: [(&str, AbilityCost); 4] = [ ("a card", discard(1, None)), ("two cards", discard(2, None)), ("three cards", discard(3, None)), - ("a card at random", discard(1, None)), ("two nonland cards", discard(2, Some(nonland))), ]; @@ -13397,6 +13396,11 @@ fn unless_discard_cost_phrase_spans_count_and_type_axes_for_both_payers() { /// coverage reports it honestly instead. #[test] fn unless_discard_cost_phrase_rejects_zero_count() { + assert_eq!( + parse_number("x cards"), + Some((0, "cards")), + "the zero-count guard is reached only when parse_number folds X to zero" + ); assert!( parse_unless_they_discard_cost("x cards").is_none(), "an X-count unless-discard must not lower to a free cost" @@ -13407,6 +13411,21 @@ 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. +#[test] +fn unless_discard_cost_phrase_rejects_random_discard() { + assert!( + parse_unless_they_discard_cost("a card at random").is_none(), + "the anaphoric-payer form must not lower random discard as chosen" + ); + assert!( + parse_unless_alt_cost("you discard a card at random").is_none(), + "the controller form must not lower random discard as chosen" + ); +} + /// CR 118.12a: a plural discard branch must still leave a chained " or …" /// branch for the disjunction combinator — the count axis must not swallow it. #[test] diff --git a/crates/engine/tests/integration/court_of_ambition.rs b/crates/engine/tests/integration/court_of_ambition.rs index 06c3c66d7c..f3b86820a5 100644 --- a/crates/engine/tests/integration/court_of_ambition.rs +++ b/crates/engine/tests/integration/court_of_ambition.rs @@ -238,6 +238,31 @@ fn court_of_ambition_monarch_branch_demands_two_cards_and_drains_six() { ); } +/// CR 109.5 + CR 725.1: "you're the monarch" on this ability means its +/// controller, not merely that any player is monarch. +#[test] +fn court_of_ambition_opponent_monarch_uses_non_monarch_branch() { + let mut runner = build_runner(2, Some(P1), 3); + fire_upkeep(&mut runner); + expect_discard_prompt(&runner, P1, 1); + + runner + .act(GameAction::PayUnlessCost { pay: false }) + .expect("declining must be accepted"); + runner.advance_until_stack_empty(); + + assert_eq!( + life(&runner, P1), + 17, + "only the controller's monarch status applies" + ); + assert_eq!( + hand_size(&runner, P1), + 3, + "a declined cost discards nothing" + ); +} + /// CR 614.15 + CR 701.9: paying the monarch branch costs exactly two cards. #[test] fn court_of_ambition_monarch_branch_paid_with_two_discards_keeps_life() { From c3a6ad767ea31f2f398f7fa717ce5a2b1451aeec Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 20:04:54 -0700 Subject: [PATCH 3/3] test(PR-7261): preserve random discard coverage honesty --- .../engine/src/parser/oracle_trigger_tests.rs | 33 ++++++++----------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 63f0d0f134..b5f0627cde 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -12103,33 +12103,26 @@ fn self_etb_sacrifice_it_anaphor_binds_to_self_ref() { } #[test] -fn trigger_unless_you_discard_a_card() { - // CR 608.2c: Balduvian Horde — "sacrifice it unless you discard a card at random". - // The "at random" suffix is currently sub-fidelity (player-chosen via WardDiscardChoice); - // the cost-gate itself is captured. +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. 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("should have unless_pay"); - assert_eq!(unless_pay.payer, TargetFilter::Controller); assert!( - matches!( - unless_pay.cost, - AbilityCost::Discard { - count: QuantityExpr::Fixed { value: 1 }, - filter: None, - selection: CardSelectionMode::Chosen, - self_scope: DiscardSelfScope::FromHand - } - ), - "cost should be DiscardCard, got {:?}", - unless_pay.cost + def.unless_pay.is_none(), + "random discard must not lower to a player-chosen unless payment" ); - let execute = def.execute.as_ref().expect("should have execute"); + let execute = def + .execute + .as_ref() + .expect("should preserve the unsupported clause"); assert!( - matches!(*execute.effect, Effect::Sacrifice { .. }), - "execute should be Sacrifice, got {:?}", + matches!(*execute.effect, Effect::Unimplemented { .. }), + "random-discard unless clause must remain visible as unimplemented, got {:?}", execute.effect ); }