From cd599b37acc40d782167293f047b3a44851168ac Mon Sep 17 00:00:00 2001 From: hurryup52 Date: Sun, 26 Jul 2026 12:50:18 +0200 Subject: [PATCH 01/14] fix(engine): add player-counter Ward cost for "Get five poison counters" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Serpent Society's Ward reads "Ward—Get five poison counters." No WardCost variant existed for "give yourself N counters", so the parser fell through to mana-cost parsing, found no mana symbols, and silently produced WardCost::Mana(generic: 0) — a free, always-paid Ward that never gave the targeting opponent any poison counters (issue #6640). Add WardCost::GetPlayerCounters { counter_kind, count } (parameterized over PlayerCounterKind rather than poison-only, so a future Ward cost of a different counter kind reuses the same shape) and a matching AbilityCost::GetPlayerCounters variant. Wire it through: - ward_cost_to_ability_cost (triggers.rs) - supported_at_resolution + can_pay_resolution + pay_ability_cost_inner (costs.rs) — the resolution-time payment authority; pay_ability_cost_inner routes through the existing add_player_counter_with_replacement so "can't get counters" replacement effects still apply - handle_unless_payment (engine_payment_choices.rs) — the unless-pay round trip, unchanged in shape otherwise - parse_ward_cost_single (oracle_keyword.rs) — new nom-combinator branch for "get N counter(s)", reusing the same parse_number/ parse_player_counter_kind combinators the GivePlayerCounter effect parser already uses, inserted before the mana-cost fallback that caused the silent misparse Every other exhaustive AbilityCost match in the crate (ability_graph.rs, ability_scan.rs, cost_payability.rs, printed_cards.rs, replacement.rs, oracle_effect/lower.rs, plus four helper methods on AbilityCost itself) now has an explicit arm, found and fixed via the compiler's own exhaustiveness checking rather than manual auditing. Also fixes the frontend Ward-payment prompt, which would otherwise show a useless generic "Pay a cost" label for this new cost shape. Closes #6640 --- client/src/i18n/locales/en/game.json | 2 + client/src/pages/GamePage.tsx | 15 +- crates/engine/src/analysis/ability_graph.rs | 7 +- crates/engine/src/game/ability_scan.rs | 1 + crates/engine/src/game/cost_payability.rs | 4 + crates/engine/src/game/costs.rs | 28 ++++ .../engine/src/game/engine_payment_choices.rs | 24 +++ crates/engine/src/game/printed_cards.rs | 1 + crates/engine/src/game/replacement.rs | 12 ++ crates/engine/src/game/triggers.rs | 21 +++ .../engine/src/parser/oracle_effect/lower.rs | 3 + crates/engine/src/parser/oracle_keyword.rs | 71 ++++++++ crates/engine/src/types/ability.rs | 17 ++ crates/engine/src/types/keywords.rs | 9 + crates/engine/tests/integration/main.rs | 1 + .../serpent_society_ward_poison_cost.rs | 156 ++++++++++++++++++ 16 files changed, 370 insertions(+), 2 deletions(-) create mode 100644 crates/engine/tests/integration/serpent_society_ward_poison_cost.rs diff --git a/client/src/i18n/locales/en/game.json b/client/src/i18n/locales/en/game.json index d22455282f..03f404b7c2 100644 --- a/client/src/i18n/locales/en/game.json +++ b/client/src/i18n/locales/en/game.json @@ -1121,6 +1121,8 @@ "returnToHand_one": "return a permanent to hand", "returnToHand_other": "return {{count}} permanents to hand", "energy": "{{amount}} energy", + "playerCounters_one": "get {{count}} {{kind}} counter", + "playerCounters_other": "get {{count}} {{kind}} counters", "generic": "a cost", "pay": "Pay {{cost}}" }, diff --git a/client/src/pages/GamePage.tsx b/client/src/pages/GamePage.tsx index 37c50208e2..9ca163895c 100644 --- a/client/src/pages/GamePage.tsx +++ b/client/src/pages/GamePage.tsx @@ -3439,7 +3439,13 @@ function formatManaCost(cost: { type: string; shards?: string[]; generic?: numbe } function formatUnlessCost( - cost: { type: string; cost?: { type: string; shards?: string[]; generic?: number }; amount?: number; count?: number }, + cost: { + type: string; + cost?: { type: string; shards?: string[]; generic?: number }; + amount?: number; + count?: number; + counter_kind?: string; + }, t: TFunction<"game">, ): string { switch (cost.type) { @@ -3471,6 +3477,13 @@ function formatUnlessCost( } case "PayEnergy": return t("gamePage.cost.energy", { amount: cost.amount ?? 0 }); + // CR 702.21a + CR 122.1 + CR 104.3d: Ward's player-counter cost + // (The Serpent Society: "Ward—Get five poison counters."). + case "GetPlayerCounters": { + const count = cost.count ?? 1; + const kind = (cost.counter_kind ?? "").toLowerCase(); + return t("gamePage.cost.playerCounters", { count, kind }); + } default: return t("gamePage.cost.generic"); } diff --git a/crates/engine/src/analysis/ability_graph.rs b/crates/engine/src/analysis/ability_graph.rs index bbf63ae952..30eb9b8fb1 100644 --- a/crates/engine/src/analysis/ability_graph.rs +++ b/crates/engine/src/analysis/ability_graph.rs @@ -1455,7 +1455,7 @@ fn sink_mana_cost(acc: &mut NodeAcc, cost: &ManaCost) { } /// CR 118 cost fold: the fourth compile-time drift gate — an exhaustive -/// **no-wildcard** match over all 29 [`AbilityCost`] variants. Polarity/sign +/// **no-wildcard** match over all 30 [`AbilityCost`] variants. Polarity/sign /// aware: a cost consumes a resource (negative `net`, ⇒ `requires`) or, in cost /// position, *produces* one (positive `net`, ⇒ `produces`). Field-less axes /// (`Tap`, `AnyCounter`) are injected directly. @@ -1569,6 +1569,11 @@ fn fold_cost(acc: &mut NodeAcc, cost: &AbilityCost) { // cast (the spell being cast), never an activation cost of this ability, // so it carries no modeled axis for the loop detector. | AbilityCost::KeywordCostOfCastSpell { .. } + // CR 702.21a: a Ward player-counter cost, like the effect-side + // `Effect::GivePlayerCounter` above, carries no modeled axis here — + // poison accumulation is a loss condition, not a combo resource this + // loop detector tracks. + | AbilityCost::GetPlayerCounters { .. } | AbilityCost::Unimplemented { .. } => {} } } diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index deb416008d..b13b0068fd 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -4436,6 +4436,7 @@ fn scan_ability_cost(cost: &AbilityCost, mode: ScanMode) -> Axes { | AbilityCost::Waterbend { .. } | AbilityCost::NinjutsuFamily { .. } | AbilityCost::KeywordCostOfCastSpell { .. } + | AbilityCost::GetPlayerCounters { .. } | AbilityCost::Unimplemented { .. } => Axes::NONE, } } diff --git a/crates/engine/src/game/cost_payability.rs b/crates/engine/src/game/cost_payability.rs index e41c23c1e6..2acfe89580 100644 --- a/crates/engine/src/game/cost_payability.rs +++ b/crates/engine/src/game/cost_payability.rs @@ -724,6 +724,10 @@ impl AbilityCost { // affordability is decided by the separate mana-payment step, not this // choice-of-object gate. AbilityCost::KeywordCostOfCastSpell { .. } => true, + // CR 702.21a + CR 122.1: Ward's player-counter cost is never paid + // as an activation cost (only at resolution, via the unless-pay + // round trip), and it has no affordability limit — always payable. + AbilityCost::GetPlayerCounters { .. } => true, } } } diff --git a/crates/engine/src/game/costs.rs b/crates/engine/src/game/costs.rs index 48dfdb0649..ca6cec0a61 100644 --- a/crates/engine/src/game/costs.rs +++ b/crates/engine/src/game/costs.rs @@ -1382,6 +1382,27 @@ fn pay_ability_cost_inner( delta: -(amount as i32), }); } + // CR 702.21a + CR 122.1 + CR 104.3d: Ward cost paid by giving the + // paying player counters of a kind (The Serpent Society). No + // affordability check (see `can_pay_resolution`) — a player may + // always choose to accept more counters. Routes through + // `add_player_counter_with_replacement` — not a raw + // `resolve_and_apply_player_edit` call — so "players can't get + // counters" replacement effects still apply, mirroring the + // `EffectCost`/`PutCounter` arm's use of the sibling + // `effects::counters::add_counter_with_replacement` above. + AbilityCost::GetPlayerCounters { + counter_kind, + count, + } => { + if !super::effects::player_counter::add_player_counter_with_replacement( + state, player, player, *counter_kind, *count, events, + ) { + return Ok(PaymentOutcome::Paused { + remaining_cost: None, + }); + } + } AbilityCost::PaySpeed { amount } => { let amount = resolve_cost_quantity(state, amount, player, source_id, scope); let amount = u8::try_from(amount.max(0)).unwrap_or(u8::MAX); @@ -1772,6 +1793,9 @@ pub(crate) fn supported_at_resolution(cost: &AbilityCost) -> bool { | AbilityCost::PaySpeed { .. } | AbilityCost::TapCreatures { .. } | AbilityCost::Composite { .. } + // CR 702.21a + CR 122.1: Ward's unless-pay always resolves at + // resolution time (never activation), so this must be true here. + | AbilityCost::GetPlayerCounters { .. } | AbilityCost::OneOf { .. } => true, // Only the chosen-from-hand discard has a resolution arm (the // `WaitingFor::DiscardChoice` / forced-choice fast path). The source-card @@ -1943,6 +1967,10 @@ fn can_pay_resolution( AbilityCost::OneOf { costs } => costs .iter() .any(|cost| can_pay_resolution(state, payer, cost, ability)), + // CR 702.21a + CR 122.1: Always payable — no resource/eligibility + // limit on giving yourself more counters (poison's ten-or-more loss + // condition is a separate SBA, not a payment-time affordability gate). + AbilityCost::GetPlayerCounters { .. } => true, // Variants below have no resolution-time payment arm // (`supported_at_resolution` is the shared membership authority). // Refusing here is the conservative affordability answer (treat as diff --git a/crates/engine/src/game/engine_payment_choices.rs b/crates/engine/src/game/engine_payment_choices.rs index 149ba172cd..88a6520dfd 100644 --- a/crates/engine/src/game/engine_payment_choices.rs +++ b/crates/engine/src/game/engine_payment_choices.rs @@ -730,6 +730,30 @@ pub(super) fn handle_unless_payment( } } } + // CR 702.21a + CR 122.1 + CR 104.3d: Unless-cost of giving + // yourself N counters (Ward's player-counter form). No + // affordability gate exists — route through the single payment + // authority exactly like PayLife/PayEnergy above. Unlike those + // two, a real `Paused` path exists here (a "can't get counters" + // replacement effect may need a live choice), so it is preserved + // rather than lumped with `Failed`. + AbilityCost::GetPlayerCounters { .. } => { + match costs::pay_ability_cost_for_resolution( + state, + player, + &cost, + pending_effect.as_ref(), + events, + )? { + PaymentOutcome::Paid => {} + PaymentOutcome::Failed { .. } => { + payment_failed = true; + } + PaymentOutcome::Paused { .. } => { + return Ok(action_result(events, state.waiting_for.clone())); + } + } + } // CR 118.12a + CR 701.9 + CR 702.24a: Unless-discard. Resolve the // per-counter-scaled count, gate on eligible hand size, and seed the // `remaining` re-prompt loop (one card per round-trip). Defers to the diff --git a/crates/engine/src/game/printed_cards.rs b/crates/engine/src/game/printed_cards.rs index 378621c6b0..324674bbde 100644 --- a/crates/engine/src/game/printed_cards.rs +++ b/crates/engine/src/game/printed_cards.rs @@ -1004,6 +1004,7 @@ fn walk_cost(cost: &AbilityCost, out: &mut Vec) { | AbilityCost::NinjutsuFamily { .. } // CR 118.9: a borrowed keyword cost carries no nested effect/cost carrier. | AbilityCost::KeywordCostOfCastSpell { .. } + | AbilityCost::GetPlayerCounters { .. } | AbilityCost::Unimplemented { .. } => {} } } diff --git a/crates/engine/src/game/replacement.rs b/crates/engine/src/game/replacement.rs index c931c0403d..807808b159 100644 --- a/crates/engine/src/game/replacement.rs +++ b/crates/engine/src/game/replacement.rs @@ -1170,6 +1170,18 @@ fn replacement_cost_description(cost: &AbilityCost) -> String { // multiplier itself doesn't change the *kind* of cost the prompt // describes; the resolved scaled amount is decided in Task 6. AbilityCost::PerCounter { base, .. } => replacement_cost_description(base), + // CR 702.21a + CR 122.1 + CR 104.3d: Ward's player-counter cost. + AbilityCost::GetPlayerCounters { + counter_kind, + count, + } => { + let kind = format!("{counter_kind:?}").to_lowercase(); + if *count == 1 { + format!("Get a {kind} counter") + } else { + format!("Get {count} {kind} counters") + } + } AbilityCost::ManaDynamic { .. } | AbilityCost::Tap | AbilityCost::Untap diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index 44d1f58fd9..4c43ab76a3 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -230,6 +230,13 @@ fn ward_cost_to_ability_cost(ward_cost: &WardCost) -> AbilityCost { } } } + WardCost::GetPlayerCounters { + counter_kind, + count, + } => AbilityCost::GetPlayerCounters { + counter_kind: *counter_kind, + count: *count, + }, } } @@ -19843,6 +19850,20 @@ pub mod tests { let waterbend = WardCost::Waterbend(ManaCost::generic(4)); let result = ward_cost_to_ability_cost(&waterbend); assert!(matches!(result, AbilityCost::Mana { cost } if cost == ManaCost::generic(4))); + + // Get player counters (The Serpent Society: "Ward—Get five poison counters.") + let poison = WardCost::GetPlayerCounters { + counter_kind: crate::types::player::PlayerCounterKind::Poison, + count: 5, + }; + let result = ward_cost_to_ability_cost(&poison); + assert!(matches!( + result, + AbilityCost::GetPlayerCounters { + counter_kind: crate::types::player::PlayerCounterKind::Poison, + count: 5, + } + )); } #[test] diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index ed6bc7b445..f6f198a37d 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -9884,6 +9884,9 @@ fn apply_where_x_to_ability_cost( // CR 118.9: the borrowed keyword cost is read at runtime from the cast // spell's keyword — it carries no where-X `QuantityExpr` amount to bind. | AbilityCost::KeywordCostOfCastSpell { .. } + // CR 702.21a: `count` is a fixed `u32`, not a `QuantityExpr` — no + // where-X amount to bind. + | AbilityCost::GetPlayerCounters { .. } | AbilityCost::Unimplemented { .. } => {} } } diff --git a/crates/engine/src/parser/oracle_keyword.rs b/crates/engine/src/parser/oracle_keyword.rs index e6bd07bf32..d247ae4f92 100644 --- a/crates/engine/src/parser/oracle_keyword.rs +++ b/crates/engine/src/parser/oracle_keyword.rs @@ -750,6 +750,37 @@ fn parse_ward_cost_single(lower: &str) -> Option { return Some(WardCost::Waterbend(cost)); } + // CR 702.21a + CR 122.1 + CR 104.3d: "get N counter(s)" — a + // player-counter ward cost (The Serpent Society: "Ward—Get five poison + // counters."). MUST run before the mana-cost fallback below, which + // otherwise silently parses unrecognized cost text with no mana + // symbols/braces as a free, always-paid Ward (phase-rs/phase#6640). + if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("get ").parse(lower) { + if let Some(before_counter) = rest + .strip_suffix(" counters") + .or_else(|| rest.strip_suffix(" counter")) + { + let parsed = nom_primitives::parse_number + .parse(before_counter) + .map(|(kind_word, n)| (n, kind_word.trim())) + .or_else(|_: nom::Err>| { + nom_primitives::parse_article + .parse(before_counter) + .map(|(kind_word, ())| (1, kind_word.trim())) + }); + if let Ok((count, kind_word)) = parsed { + if let Ok((_, counter_kind)) = + all_consuming(nom_primitives::parse_player_counter_kind).parse(kind_word) + { + return Some(WardCost::GetPlayerCounters { + counter_kind, + count, + }); + } + } + } + } + // Fall back to mana cost parsing let cost = crate::database::mtgjson::parse_mtgjson_mana_cost(lower.trim()); Some(WardCost::Mana(cost)) @@ -2829,6 +2860,46 @@ mod tests { use super::*; use crate::types::ability::{AbilityCost, SacrificeCost}; use crate::types::mana::ManaCost; + use crate::types::player::PlayerCounterKind; + + #[test] + fn ward_get_poison_counters_parses_as_player_counter_cost() { + // Issue #6640 (The Serpent Society): "Ward—Get five poison counters." + // must not silently fall through to the mana-cost fallback. + let result = parse_ward_cost("Get five poison counters."); + assert_eq!( + result, + Some(Keyword::Ward(WardCost::GetPlayerCounters { + counter_kind: PlayerCounterKind::Poison, + count: 5, + })) + ); + } + + #[test] + fn ward_get_player_counters_accepts_digit_count_and_other_kinds() { + // Class-level coverage: digit form, and a non-poison counter kind. + let result = parse_ward_cost("Get 3 experience counters."); + assert_eq!( + result, + Some(Keyword::Ward(WardCost::GetPlayerCounters { + counter_kind: PlayerCounterKind::Experience, + count: 3, + })) + ); + } + + #[test] + fn ward_get_a_poison_counter_singular_defaults_to_count_one() { + let result = parse_ward_cost("Get a poison counter."); + assert_eq!( + result, + Some(Keyword::Ward(WardCost::GetPlayerCounters { + counter_kind: PlayerCounterKind::Poison, + count: 1, + })) + ); + } #[test] fn parse_granted_keyword_fragment_cascade() { diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index dcc1bedae3..ec0d8df00b 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -8634,6 +8634,16 @@ pub enum AbilityCost { KeywordCostOfCastSpell { keyword: crate::types::keywords::KeywordKind, }, + /// CR 702.21a + CR 122.1 + CR 104.3d: Give the paying player counters of + /// a kind, as a Ward unless-cost (The Serpent Society's "Ward—Get five + /// poison counters."). Unlike `PayLife`/`PayEnergy`, there is no + /// affordability gate — a player may always choose to accept more + /// counters; poison's ten-or-more loss condition is a separate, + /// already-implemented SBA, not a payment-time limit. + GetPlayerCounters { + counter_kind: PlayerCounterKind, + count: u32, + }, Unimplemented { description: String, }, @@ -8731,6 +8741,7 @@ impl AbilityCost { | AbilityCost::Waterbend { .. } | AbilityCost::NinjutsuFamily { .. } | AbilityCost::KeywordCostOfCastSpell { .. } + | AbilityCost::GetPlayerCounters { .. } | AbilityCost::Unimplemented { .. } => {} } } @@ -8787,6 +8798,7 @@ impl AbilityCost { | AbilityCost::EffectCost { .. } | AbilityCost::PerCounter { .. } | AbilityCost::KeywordCostOfCastSpell { .. } + | AbilityCost::GetPlayerCounters { .. } | AbilityCost::Unimplemented { .. } => false, } } @@ -8864,6 +8876,9 @@ impl AbilityCost { AbilityCost::Mill { .. } => vec![CostCategory::Mills], AbilityCost::Exert => vec![CostCategory::Exerts], AbilityCost::Blight { .. } => vec![CostCategory::PutsCounters], + // CR 702.21a + CR 122.1: Ward's player-counter cost puts counters + // on the payer, same category as Blight's self-counter cost. + AbilityCost::GetPlayerCounters { .. } => vec![CostCategory::PutsCounters], AbilityCost::Reveal { .. } => vec![CostCategory::Reveals], AbilityCost::Behold { action, .. } => { if *action == BeholdCostAction::ExileChosen { @@ -8976,6 +8991,8 @@ impl AbilityCost { | AbilityCost::EffectCost { .. } // CR 118.9: borrowed mana cost — pays mana, never destroys the source. | AbilityCost::KeywordCostOfCastSpell { .. } + // CR 702.21a: gives the payer counters — never destroys the source. + | AbilityCost::GetPlayerCounters { .. } | AbilityCost::Unimplemented { .. } => false, } } diff --git a/crates/engine/src/types/keywords.rs b/crates/engine/src/types/keywords.rs index d792ff332c..bb7ebd068f 100644 --- a/crates/engine/src/types/keywords.rs +++ b/crates/engine/src/types/keywords.rs @@ -543,6 +543,15 @@ pub enum WardCost { /// CR 702.21a: Compound ward cost — multiple costs that must all be paid. /// Used for "Ward—{2}, Pay 2 life" where comma-separated sub-costs are conjoined. Compound(Vec), + /// CR 702.21a + CR 122.1 + CR 104.3d: Ward cost paid by giving the paying + /// player counters of a kind (The Serpent Society: "Ward—Get five poison + /// counters."). Parameterized over `PlayerCounterKind` rather than a + /// poison-only variant so a future Ward cost of a different + /// player-counter kind reuses this shape instead of adding a sibling. + GetPlayerCounters { + counter_kind: crate::types::player::PlayerCounterKind, + count: u32, + }, } /// CR 702.54a + CR 702.54b: Bloodthirst has fixed-N and X-count forms. diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 3de438afaf..864f6f8be0 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -840,6 +840,7 @@ mod selenia_vigilance_grant; mod self_destruct_target_power; mod sensei_golden_tail_5950; mod sentinel_sliver_vigilance_grant; +mod serpent_society_ward_poison_cost; mod serras_emissary_chosen_card_type_protection; mod sin_spiras_punishment_repeat; mod skullwinder_chosen_opponent; diff --git a/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs b/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs new file mode 100644 index 0000000000..cd358d2f56 --- /dev/null +++ b/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs @@ -0,0 +1,156 @@ +//! Regression for issue #6640: The Serpent Society's Ward—Get five poison +//! counters never gave the targeting opponent poison counters, because the +//! Oracle parser had no `WardCost` variant for "give yourself N counters" and +//! silently fell back to `WardCost::Mana(generic: 0)` — a free, always-paid +//! Ward that does nothing. +//! +//! https://github.com/phase-rs/phase/issues/6640 +//! +//! CR references: +//! - CR 702.21a: Ward — counter the targeting spell/ability unless the +//! targeting player pays the stated cost. +//! - CR 122.1 + CR 104.3d: giving a player poison counters; a player with +//! ten or more poison counters loses the game (a separate SBA, not +//! exercised by this test). + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::actions::GameAction; +use engine::types::game_state::WaitingFor; +use engine::types::phase::Phase; +use engine::types::player::PlayerCounterKind; + +const SERPENT_SOCIETY: &str = "Deathtouch\n\ +Ward—Get five poison counters. (A player with ten or more poison counters loses the game.)\n\ +Whenever another creature you control with deathtouch dies, each opponent sacrifices a nontoken creature of their choice."; + +#[test] +fn serpent_society_ward_prompts_the_targeting_opponent_for_poison_counters() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let serpent_society = scenario + .add_creature_from_oracle(P0, "The Serpent Society", 3, 4, SERPENT_SOCIETY) + .id(); + let destroy = scenario + .add_spell_to_hand_from_oracle(P1, "Destroy Spell", true, "Destroy target creature.") + .id(); + let mut runner = scenario.build(); + { + let state = runner.state_mut(); + state.active_player = P1; + state.priority_player = P1; + state.waiting_for = WaitingFor::Priority { player: P1 }; + } + + runner + .cast(destroy) + .target_objects(&[serpent_society]) + .commit(); + runner.advance_until_stack_empty(); + + let WaitingFor::UnlessPayment { player, cost, .. } = &runner.state().waiting_for else { + panic!( + "Ward must prompt the targeting opponent to pay the poison-counter cost, got {:?}", + runner.state().waiting_for + ); + }; + assert_eq!(*player, P1); + assert!(matches!( + cost, + engine::types::ability::AbilityCost::GetPlayerCounters { + counter_kind: PlayerCounterKind::Poison, + count: 5, + } + )); +} + +#[test] +fn serpent_society_ward_declined_counters_the_spell_and_gives_no_poison() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let serpent_society = scenario + .add_creature_from_oracle(P0, "The Serpent Society", 3, 4, SERPENT_SOCIETY) + .id(); + let destroy = scenario + .add_spell_to_hand_from_oracle(P1, "Destroy Spell", true, "Destroy target creature.") + .id(); + let mut runner = scenario.build(); + { + let state = runner.state_mut(); + state.active_player = P1; + state.priority_player = P1; + state.waiting_for = WaitingFor::Priority { player: P1 }; + } + + runner + .cast(destroy) + .target_objects(&[serpent_society]) + .commit(); + runner.advance_until_stack_empty(); + + runner + .act(GameAction::PayUnlessCost { pay: false }) + .expect("declining Ward must be a legal action"); + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().players[P1.0 as usize].poison_counters, + 0, + "declining Ward's cost must not give the opponent any poison counters" + ); + assert!( + runner + .state() + .objects + .get(&serpent_society) + .is_some_and(|obj| obj.zone == engine::types::zones::Zone::Battlefield), + "declining Ward's cost must counter the targeting spell, leaving Serpent Society alive" + ); + assert!( + !runner.state().stack.iter().any(|entry| entry.id == destroy), + "the countered spell must be removed from the stack" + ); +} + +#[test] +fn serpent_society_ward_paid_gives_five_poison_and_the_spell_resolves() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let serpent_society = scenario + .add_creature_from_oracle(P0, "The Serpent Society", 3, 4, SERPENT_SOCIETY) + .id(); + let destroy = scenario + .add_spell_to_hand_from_oracle(P1, "Destroy Spell", true, "Destroy target creature.") + .id(); + let mut runner = scenario.build(); + { + let state = runner.state_mut(); + state.active_player = P1; + state.priority_player = P1; + state.waiting_for = WaitingFor::Priority { player: P1 }; + } + + runner + .cast(destroy) + .target_objects(&[serpent_society]) + .commit(); + runner.advance_until_stack_empty(); + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("the opponent pays Ward's poison-counter cost"); + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().players[P1.0 as usize].poison_counters, + 5, + "paying Ward's cost must give the targeting opponent five poison counters" + ); + assert!( + runner + .state() + .objects + .get(&serpent_society) + .is_none_or(|obj| obj.zone != engine::types::zones::Zone::Battlefield), + "paying Ward's cost must let the targeted destroy spell resolve, removing Serpent Society from the battlefield" + ); +} From b4ab22336488acbc56469afd33e8e93ffdeb582f Mon Sep 17 00:00:00 2001 From: hurryup52 Date: Sun, 26 Jul 2026 13:10:14 +0200 Subject: [PATCH 02/14] fix(ci): cover WardCost::GetPlayerCounters in phase-ai and locale files CI on #6662 caught two gaps my local `cargo check -p engine --lib` never touched: - crates/phase-ai has its own exhaustive matches directly on WardCost (not AbilityCost) for AI tactical scoring: the Ward-cost "severity" pricing in anti_self_harm.rs (both the top-level match and its duplicated inline copy inside the Compound arm) and can_pay_ward_cost's affordability check in strategy_helpers.rs. Add GetPlayerCounters arms to all three: severity scales uncapped with count (voluntarily taking poison counters is a real, severe cost, unlike the capped mana/life costs), and affordability is unconditionally true (no resource limit on giving yourself more counters, mirroring the engine's own can_pay_resolution). - The i18n "locale key parity" test requires es/fr/de/it/pt/pl to have the exact same key set as en, not just en as I'd assumed. Add cost.playerCounters_one/_other and a cost.playerCounterKind noun map to all six locales with real translations (not English placeholders), matching the badges section's existing poison/experience/rad counter naming conventions. Also redesigned formatUnlessCost's GetPlayerCounters case to look the counter-kind noun up through i18n rather than interpolating the raw English enum name, so non-English locales render correctly. --- client/src/i18n/locales/de/game.json | 8 ++++++++ client/src/i18n/locales/en/game.json | 6 ++++++ client/src/i18n/locales/es/game.json | 8 ++++++++ client/src/i18n/locales/fr/game.json | 8 ++++++++ client/src/i18n/locales/it/game.json | 8 ++++++++ client/src/i18n/locales/pl/game.json | 8 ++++++++ client/src/i18n/locales/pt/game.json | 8 ++++++++ client/src/pages/GamePage.tsx | 12 ++++++++++-- crates/phase-ai/src/policies/anti_self_harm.rs | 6 ++++++ crates/phase-ai/src/policies/strategy_helpers.rs | 4 ++++ 10 files changed, 74 insertions(+), 2 deletions(-) diff --git a/client/src/i18n/locales/de/game.json b/client/src/i18n/locales/de/game.json index fdba287aa7..1139dc2d1a 100644 --- a/client/src/i18n/locales/de/game.json +++ b/client/src/i18n/locales/de/game.json @@ -1083,6 +1083,14 @@ "returnToHand_one": "eine bleibende Karte auf die Hand zurückgeben", "returnToHand_other": "{{count}} bleibende Karten auf die Hand zurückgeben", "energy": "{{amount}} Energie", + "playerCounters_one": "{{count}} {{kind}}marke erhalten", + "playerCounters_other": "{{count}} {{kind}}marken erhalten", + "playerCounterKind": { + "poison": "Gift", + "experience": "Erfahrung", + "rad": "Strahlung", + "ticket": "Ticket" + }, "generic": "Kosten", "pay": "{{cost}} bezahlen" }, diff --git a/client/src/i18n/locales/en/game.json b/client/src/i18n/locales/en/game.json index 03f404b7c2..1ad2c35a3f 100644 --- a/client/src/i18n/locales/en/game.json +++ b/client/src/i18n/locales/en/game.json @@ -1123,6 +1123,12 @@ "energy": "{{amount}} energy", "playerCounters_one": "get {{count}} {{kind}} counter", "playerCounters_other": "get {{count}} {{kind}} counters", + "playerCounterKind": { + "poison": "poison", + "experience": "experience", + "rad": "rad", + "ticket": "ticket" + }, "generic": "a cost", "pay": "Pay {{cost}}" }, diff --git a/client/src/i18n/locales/es/game.json b/client/src/i18n/locales/es/game.json index 26e0897409..5897130350 100644 --- a/client/src/i18n/locales/es/game.json +++ b/client/src/i18n/locales/es/game.json @@ -1083,6 +1083,14 @@ "returnToHand_one": "devuelve un permanente a la mano", "returnToHand_other": "devuelve {{count}} permanentes a la mano", "energy": "{{amount}} de energía", + "playerCounters_one": "consigue {{count}} contador de {{kind}}", + "playerCounters_other": "consigue {{count}} contadores de {{kind}}", + "playerCounterKind": { + "poison": "veneno", + "experience": "experiencia", + "rad": "rad", + "ticket": "ticket" + }, "generic": "un coste", "pay": "Pagar {{cost}}" }, diff --git a/client/src/i18n/locales/fr/game.json b/client/src/i18n/locales/fr/game.json index af9fcad115..8fbda29e3c 100644 --- a/client/src/i18n/locales/fr/game.json +++ b/client/src/i18n/locales/fr/game.json @@ -1083,6 +1083,14 @@ "returnToHand_one": "renvoyer un permanent en main", "returnToHand_other": "renvoyer {{count}} permanents en main", "energy": "{{amount}} énergie", + "playerCounters_one": "obtenir {{count}} marqueur {{kind}}", + "playerCounters_other": "obtenir {{count}} marqueurs {{kind}}", + "playerCounterKind": { + "poison": "poison", + "experience": "expérience", + "rad": "rad", + "ticket": "ticket" + }, "generic": "un coût", "pay": "Payer {{cost}}" }, diff --git a/client/src/i18n/locales/it/game.json b/client/src/i18n/locales/it/game.json index 24aff1b0f9..8475d97ed8 100644 --- a/client/src/i18n/locales/it/game.json +++ b/client/src/i18n/locales/it/game.json @@ -1083,6 +1083,14 @@ "returnToHand_one": "restituisci un permanente alla mano", "returnToHand_other": "restituisci {{count}} permanenti alla mano", "energy": "{{amount}} energia", + "playerCounters_one": "ottieni {{count}} segnalino {{kind}}", + "playerCounters_other": "ottieni {{count}} segnalini {{kind}}", + "playerCounterKind": { + "poison": "veleno", + "experience": "esperienza", + "rad": "radiazione", + "ticket": "ticket" + }, "generic": "un costo", "pay": "Paga {{cost}}" }, diff --git a/client/src/i18n/locales/pl/game.json b/client/src/i18n/locales/pl/game.json index 1b28510c77..f9604f1478 100644 --- a/client/src/i18n/locales/pl/game.json +++ b/client/src/i18n/locales/pl/game.json @@ -1083,6 +1083,14 @@ "returnToHand_one": "zwróć trwałego do ręki", "returnToHand_other": "zwróć {{count}} trwałych do ręki", "energy": "{{amount}} energii", + "playerCounters_one": "zdobądź {{count}} znacznik {{kind}}", + "playerCounters_other": "zdobądź {{count}} znaczników {{kind}}", + "playerCounterKind": { + "poison": "trucizny", + "experience": "doświadczenia", + "rad": "promieniowania", + "ticket": "biletu" + }, "generic": "koszt", "pay": "Zapłać {{cost}}" }, diff --git a/client/src/i18n/locales/pt/game.json b/client/src/i18n/locales/pt/game.json index 71fab73662..6450cbd259 100644 --- a/client/src/i18n/locales/pt/game.json +++ b/client/src/i18n/locales/pt/game.json @@ -1083,6 +1083,14 @@ "returnToHand_one": "devolva um permanente à mão", "returnToHand_other": "devolva {{count}} permanentes à mão", "energy": "{{amount}} de energia", + "playerCounters_one": "obtenha {{count}} marcador de {{kind}}", + "playerCounters_other": "obtenha {{count}} marcadores de {{kind}}", + "playerCounterKind": { + "poison": "veneno", + "experience": "experiência", + "rad": "radiação", + "ticket": "ticket" + }, "generic": "um custo", "pay": "Pagar {{cost}}" }, diff --git a/client/src/pages/GamePage.tsx b/client/src/pages/GamePage.tsx index 9ca163895c..ea8cbd3cce 100644 --- a/client/src/pages/GamePage.tsx +++ b/client/src/pages/GamePage.tsx @@ -3478,10 +3478,18 @@ function formatUnlessCost( case "PayEnergy": return t("gamePage.cost.energy", { amount: cost.amount ?? 0 }); // CR 702.21a + CR 122.1 + CR 104.3d: Ward's player-counter cost - // (The Serpent Society: "Ward—Get five poison counters."). + // (The Serpent Society: "Ward—Get five poison counters."). The counter + // kind is looked up through i18n (not interpolated as raw English) so + // non-English locales get a real translated noun, matching how the + // badges section already localizes poison/experience/rad counter names. case "GetPlayerCounters": { const count = cost.count ?? 1; - const kind = (cost.counter_kind ?? "").toLowerCase(); + const kindKey = ["Poison", "Experience", "Rad", "Ticket"].includes( + cost.counter_kind ?? "", + ) + ? (cost.counter_kind as string).toLowerCase() + : "poison"; + const kind = t(`gamePage.cost.playerCounterKind.${kindKey}`); return t("gamePage.cost.playerCounters", { count, kind }); } default: diff --git a/crates/phase-ai/src/policies/anti_self_harm.rs b/crates/phase-ai/src/policies/anti_self_harm.rs index dad9d1ed09..ebd488f35d 100644 --- a/crates/phase-ai/src/policies/anti_self_harm.rs +++ b/crates/phase-ai/src/policies/anti_self_harm.rs @@ -790,6 +790,11 @@ fn score_target_object(ctx: &PolicyContext<'_>, object_id: ObjectId, beneficial: WardCost::DiscardCard => 1.5, WardCost::Sacrifice { count, .. } => *count as f64 * 2.0, WardCost::Waterbend(cost) => (cost.mana_value() as f64 / 2.0).min(2.0), + // CR 702.21a + CR 122.1 + CR 104.3d: voluntarily taking + // poison (or other player) counters is a severe cost — + // uncapped, scaled by count, since poison specifically + // progresses toward an unconditional loss at 10. + WardCost::GetPlayerCounters { count, .. } => *count as f64 * 3.0, // CR 702.21a: Compound costs sum severity of components. WardCost::Compound(costs) => costs .iter() @@ -804,6 +809,7 @@ fn score_target_object(ctx: &PolicyContext<'_>, object_id: ObjectId, beneficial: WardCost::Waterbend(cost) => { (cost.mana_value() as f64 / 2.0).min(2.0) } + WardCost::GetPlayerCounters { count, .. } => *count as f64 * 3.0, WardCost::Compound(_) => 2.0, }) .sum::() diff --git a/crates/phase-ai/src/policies/strategy_helpers.rs b/crates/phase-ai/src/policies/strategy_helpers.rs index d73c3e8faa..e2ab1a5d66 100644 --- a/crates/phase-ai/src/policies/strategy_helpers.rs +++ b/crates/phase-ai/src/policies/strategy_helpers.rs @@ -780,6 +780,10 @@ pub(crate) fn can_pay_ward_cost( .count(); matching as u32 >= *count } + // CR 702.21a + CR 122.1: no affordability limit — a player can always + // choose to accept more counters (mirrors the engine's own + // `can_pay_resolution` for this cost). + WardCost::GetPlayerCounters { .. } => true, // CR 702.21a: every conjoined sub-cost must be payable. Mana contention // between multiple mana sub-costs is approximated (each checked against // the full post-spell pool) — rare enough not to warrant exact tracking. From 2452d41844a1517a2966996a13ac9f530658973e Mon Sep 17 00:00:00 2001 From: hurryup52 Date: Sun, 26 Jul 2026 14:41:16 +0200 Subject: [PATCH 03/14] fix(review): lethality-aware AI severity, fail-closed parser, strict frontend type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses matthewevans's review on #6662: - Third crate with an exhaustive AbilityCost match my earlier per-crate checks missed: crates/mtgish-import/src/convert/action.rs's rewrite_bound_x_in_ability_cost (no-op arm — count is a fixed u32, not an X-bindable QuantityExpr). - crates/engine/src/game/costs.rs's exhaustive "lockstep gate" test (sample_for + all_variants) only exercised under --tests, which this sandbox can't compile, so cargo check --lib alone never caught it. Added a real arm so GetPlayerCounters is actually exercised by every_ability_cost_variant_has_resolution_support_answer, not just made to compile. - AI severity scoring (anti_self_harm.rs) is now lethality- and kind-aware instead of a flat count*3.0: a poison payment that would push the AI to LETHAL_POISON (10, per features::poison) scores prohibitively (100.0, matching this file's own "never do this" sentinel) rather than an ordinary linear severity; non-poison player counters (experience/rad/ticket) carry no loss-condition risk and score 0. Fixed the Compound arm's sum-then-min(4.0) cap, which would otherwise silently clamp a lethal poison sub-cost back down to an ordinary-looking severity. - Parser (oracle_keyword.rs): counter-shaped "get ... counter(s)" text that fails to parse (unparseable count, unknown counter kind) now fails closed (returns None) instead of falling through to the mana-cost fallback — the same silent-free-Ward bug class #6640 was about, for different malformed input. Added negative tests for both failure shapes. - Frontend (GamePage.tsx): formatUnlessCost's GetPlayerCounters case is now a real discriminated union member with required, exact-cased fields (count: number, counter_kind: "Poison"|"Experience"|"Rad"| "Ticket") — no more toLowerCase()/fallback-default reinterpretation of engine data in the display layer. Renamed the playerCounterKind i18n keys in all 7 locales to match the engine's exact PascalCase strings. - New integration test: a Ward payment that pushes the payer to ten poison counters triggers the CR 104.3d loss SBA immediately (before the targeted destroy spell can resolve), mirroring sba.rs's own sba_poison_10_player_loses unit test shape. Verified with cargo check --lib on all three affected crates (engine, phase-ai, mtgish-import) — all clean. Full cargo test still blocked by this sandbox's memory ceiling (disclosed previously on this PR); the new/changed test code was hand-verified against the real APIs it calls. --- client/src/i18n/locales/de/game.json | 8 +- client/src/i18n/locales/en/game.json | 8 +- client/src/i18n/locales/es/game.json | 8 +- client/src/i18n/locales/fr/game.json | 8 +- client/src/i18n/locales/it/game.json | 8 +- client/src/i18n/locales/pl/game.json | 8 +- client/src/i18n/locales/pt/game.json | 8 +- client/src/pages/GamePage.tsx | 43 ++++---- crates/engine/src/game/costs.rs | 8 ++ crates/engine/src/parser/oracle_keyword.rs | 23 ++++ .../serpent_society_ward_poison_cost.rs | 59 ++++++++++ crates/mtgish-import/src/convert/action.rs | 3 + .../phase-ai/src/policies/anti_self_harm.rs | 104 ++++++++++++++---- 13 files changed, 225 insertions(+), 71 deletions(-) diff --git a/client/src/i18n/locales/de/game.json b/client/src/i18n/locales/de/game.json index 1139dc2d1a..813ece1d56 100644 --- a/client/src/i18n/locales/de/game.json +++ b/client/src/i18n/locales/de/game.json @@ -1086,10 +1086,10 @@ "playerCounters_one": "{{count}} {{kind}}marke erhalten", "playerCounters_other": "{{count}} {{kind}}marken erhalten", "playerCounterKind": { - "poison": "Gift", - "experience": "Erfahrung", - "rad": "Strahlung", - "ticket": "Ticket" + "Poison": "Gift", + "Experience": "Erfahrung", + "Rad": "Strahlung", + "Ticket": "Ticket" }, "generic": "Kosten", "pay": "{{cost}} bezahlen" diff --git a/client/src/i18n/locales/en/game.json b/client/src/i18n/locales/en/game.json index 1ad2c35a3f..085163d207 100644 --- a/client/src/i18n/locales/en/game.json +++ b/client/src/i18n/locales/en/game.json @@ -1124,10 +1124,10 @@ "playerCounters_one": "get {{count}} {{kind}} counter", "playerCounters_other": "get {{count}} {{kind}} counters", "playerCounterKind": { - "poison": "poison", - "experience": "experience", - "rad": "rad", - "ticket": "ticket" + "Poison": "poison", + "Experience": "experience", + "Rad": "rad", + "Ticket": "ticket" }, "generic": "a cost", "pay": "Pay {{cost}}" diff --git a/client/src/i18n/locales/es/game.json b/client/src/i18n/locales/es/game.json index 5897130350..618f738f5d 100644 --- a/client/src/i18n/locales/es/game.json +++ b/client/src/i18n/locales/es/game.json @@ -1086,10 +1086,10 @@ "playerCounters_one": "consigue {{count}} contador de {{kind}}", "playerCounters_other": "consigue {{count}} contadores de {{kind}}", "playerCounterKind": { - "poison": "veneno", - "experience": "experiencia", - "rad": "rad", - "ticket": "ticket" + "Poison": "veneno", + "Experience": "experiencia", + "Rad": "rad", + "Ticket": "ticket" }, "generic": "un coste", "pay": "Pagar {{cost}}" diff --git a/client/src/i18n/locales/fr/game.json b/client/src/i18n/locales/fr/game.json index 8fbda29e3c..e0e079d665 100644 --- a/client/src/i18n/locales/fr/game.json +++ b/client/src/i18n/locales/fr/game.json @@ -1086,10 +1086,10 @@ "playerCounters_one": "obtenir {{count}} marqueur {{kind}}", "playerCounters_other": "obtenir {{count}} marqueurs {{kind}}", "playerCounterKind": { - "poison": "poison", - "experience": "expérience", - "rad": "rad", - "ticket": "ticket" + "Poison": "poison", + "Experience": "expérience", + "Rad": "rad", + "Ticket": "ticket" }, "generic": "un coût", "pay": "Payer {{cost}}" diff --git a/client/src/i18n/locales/it/game.json b/client/src/i18n/locales/it/game.json index 8475d97ed8..5327b4019b 100644 --- a/client/src/i18n/locales/it/game.json +++ b/client/src/i18n/locales/it/game.json @@ -1086,10 +1086,10 @@ "playerCounters_one": "ottieni {{count}} segnalino {{kind}}", "playerCounters_other": "ottieni {{count}} segnalini {{kind}}", "playerCounterKind": { - "poison": "veleno", - "experience": "esperienza", - "rad": "radiazione", - "ticket": "ticket" + "Poison": "veleno", + "Experience": "esperienza", + "Rad": "radiazione", + "Ticket": "ticket" }, "generic": "un costo", "pay": "Paga {{cost}}" diff --git a/client/src/i18n/locales/pl/game.json b/client/src/i18n/locales/pl/game.json index f9604f1478..0453e6c252 100644 --- a/client/src/i18n/locales/pl/game.json +++ b/client/src/i18n/locales/pl/game.json @@ -1086,10 +1086,10 @@ "playerCounters_one": "zdobądź {{count}} znacznik {{kind}}", "playerCounters_other": "zdobądź {{count}} znaczników {{kind}}", "playerCounterKind": { - "poison": "trucizny", - "experience": "doświadczenia", - "rad": "promieniowania", - "ticket": "biletu" + "Poison": "trucizny", + "Experience": "doświadczenia", + "Rad": "promieniowania", + "Ticket": "biletu" }, "generic": "koszt", "pay": "Zapłać {{cost}}" diff --git a/client/src/i18n/locales/pt/game.json b/client/src/i18n/locales/pt/game.json index 6450cbd259..87224b5f85 100644 --- a/client/src/i18n/locales/pt/game.json +++ b/client/src/i18n/locales/pt/game.json @@ -1086,10 +1086,10 @@ "playerCounters_one": "obtenha {{count}} marcador de {{kind}}", "playerCounters_other": "obtenha {{count}} marcadores de {{kind}}", "playerCounterKind": { - "poison": "veneno", - "experience": "experiência", - "rad": "radiação", - "ticket": "ticket" + "Poison": "veneno", + "Experience": "experiência", + "Rad": "radiação", + "Ticket": "ticket" }, "generic": "um custo", "pay": "Pagar {{cost}}" diff --git a/client/src/pages/GamePage.tsx b/client/src/pages/GamePage.tsx index ea8cbd3cce..5281090263 100644 --- a/client/src/pages/GamePage.tsx +++ b/client/src/pages/GamePage.tsx @@ -3439,13 +3439,22 @@ function formatManaCost(cost: { type: string; shards?: string[]; generic?: numbe } function formatUnlessCost( - cost: { - type: string; - cost?: { type: string; shards?: string[]; generic?: number }; - amount?: number; - count?: number; - counter_kind?: string; - }, + cost: + // CR 702.21a + CR 122.1 + CR 104.3d: Ward's player-counter cost is a real + // discriminated variant with required fields (the engine's + // `AbilityCost::GetPlayerCounters` always sends both) — rendered + // unchanged, not reinterpreted (no lowercasing, no fallback defaults). + | { + type: "GetPlayerCounters"; + count: number; + counter_kind: "Poison" | "Experience" | "Rad" | "Ticket"; + } + | { + type: string; + cost?: { type: string; shards?: string[]; generic?: number }; + amount?: number; + count?: number; + }, t: TFunction<"game">, ): string { switch (cost.type) { @@ -3477,20 +3486,14 @@ function formatUnlessCost( } case "PayEnergy": return t("gamePage.cost.energy", { amount: cost.amount ?? 0 }); - // CR 702.21a + CR 122.1 + CR 104.3d: Ward's player-counter cost - // (The Serpent Society: "Ward—Get five poison counters."). The counter - // kind is looked up through i18n (not interpolated as raw English) so - // non-English locales get a real translated noun, matching how the - // badges section already localizes poison/experience/rad counter names. + // The counter kind is looked up through i18n (not interpolated as raw + // English) so non-English locales get a real translated noun, matching + // how the badges section already localizes poison/experience/rad counter + // names. `cost.counter_kind` is rendered exactly as the engine sent it — + // no lowercasing, no fallback default. case "GetPlayerCounters": { - const count = cost.count ?? 1; - const kindKey = ["Poison", "Experience", "Rad", "Ticket"].includes( - cost.counter_kind ?? "", - ) - ? (cost.counter_kind as string).toLowerCase() - : "poison"; - const kind = t(`gamePage.cost.playerCounterKind.${kindKey}`); - return t("gamePage.cost.playerCounters", { count, kind }); + const kind = t(`gamePage.cost.playerCounterKind.${cost.counter_kind}`); + return t("gamePage.cost.playerCounters", { count: cost.count, kind }); } default: return t("gamePage.cost.generic"); diff --git a/crates/engine/src/game/costs.rs b/crates/engine/src/game/costs.rs index ca6cec0a61..4fcc77ae31 100644 --- a/crates/engine/src/game/costs.rs +++ b/crates/engine/src/game/costs.rs @@ -2151,6 +2151,10 @@ mod tests { AbilityCost::KeywordCostOfCastSpell { .. } => AbilityCost::KeywordCostOfCastSpell { keyword: crate::types::keywords::KeywordKind::Suspend, }, + AbilityCost::GetPlayerCounters { .. } => AbilityCost::GetPlayerCounters { + counter_kind: crate::types::player::PlayerCounterKind::Poison, + count: 1, + }, AbilityCost::Unimplemented { .. } => AbilityCost::Unimplemented { description: "test".to_string(), }, @@ -2264,6 +2268,10 @@ mod tests { AbilityCost::KeywordCostOfCastSpell { keyword: crate::types::keywords::KeywordKind::Suspend, }, + AbilityCost::GetPlayerCounters { + counter_kind: crate::types::player::PlayerCounterKind::Poison, + count: 1, + }, AbilityCost::Unimplemented { description: String::new(), }, diff --git a/crates/engine/src/parser/oracle_keyword.rs b/crates/engine/src/parser/oracle_keyword.rs index d247ae4f92..48ec6dde95 100644 --- a/crates/engine/src/parser/oracle_keyword.rs +++ b/crates/engine/src/parser/oracle_keyword.rs @@ -778,6 +778,13 @@ fn parse_ward_cost_single(lower: &str) -> Option { }); } } + // CR 702.21a: recognized as counter-shaped ("get ... counter(s)") + // but the count or counter kind didn't parse — fail closed rather + // than falling through to the mana-cost fallback below, which + // would otherwise silently produce a free, always-paid Ward for + // unsupported/malformed counter text (phase-rs/phase#6640's exact + // bug class, for different malformed input). + return None; } } @@ -2901,6 +2908,22 @@ mod tests { ); } + // Issue #6640 follow-up: counter-shaped "get ... counter(s)" text that + // fails to parse (malformed count, unknown kind) must fail closed + // (`None`) rather than silently falling through to the mana-cost + // fallback and becoming a free, always-paid Ward. + #[test] + fn ward_get_counters_with_unparseable_count_fails_closed() { + let result = parse_ward_cost("Get many poison counters."); + assert_eq!(result, None); + } + + #[test] + fn ward_get_counters_with_unknown_kind_fails_closed() { + let result = parse_ward_cost("Get five sprocket counters."); + assert_eq!(result, None); + } + #[test] fn parse_granted_keyword_fragment_cascade() { // CR 702.85a: Cascade is a no-parameter keyword. diff --git a/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs b/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs index cd358d2f56..6bed48e249 100644 --- a/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs +++ b/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs @@ -154,3 +154,62 @@ fn serpent_society_ward_paid_gives_five_poison_and_the_spell_resolves() { "paying Ward's cost must let the targeted destroy spell resolve, removing Serpent Society from the battlefield" ); } + +/// CR 104.3d + CR 704.5c: a payment that pushes the payer to ten or more +/// poison counters must trigger the loss state-based action immediately — +/// before the targeted destroy spell gets a chance to continue resolving. +/// Mirrors `crates/engine/src/game/sba.rs`'s own `sba_poison_10_player_loses` +/// unit test's expected shape. +#[test] +fn serpent_society_ward_payment_that_reaches_ten_poison_loses_the_game() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let serpent_society = scenario + .add_creature_from_oracle(P0, "The Serpent Society", 3, 4, SERPENT_SOCIETY) + .id(); + let destroy = scenario + .add_spell_to_hand_from_oracle(P1, "Destroy Spell", true, "Destroy target creature.") + .id(); + let mut runner = scenario.build(); + { + let state = runner.state_mut(); + state.active_player = P1; + state.priority_player = P1; + state.waiting_for = WaitingFor::Priority { player: P1 }; + state.players[P1.0 as usize].poison_counters = 5; + } + + runner + .cast(destroy) + .target_objects(&[serpent_society]) + .commit(); + runner.advance_until_stack_empty(); + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("the opponent pays Ward's poison-counter cost"); + + assert_eq!( + runner.state().players[P1.0 as usize].poison_counters, + 10, + "5 existing + 5 from Ward's cost must reach the ten-poison threshold" + ); + assert!( + matches!( + runner.state().waiting_for, + WaitingFor::GameOver { + winner: Some(p) if p == P0 + } + ), + "reaching ten poison must trigger the CR 104.3d loss SBA immediately, got {:?}", + runner.state().waiting_for + ); + assert!( + runner + .state() + .objects + .get(&serpent_society) + .is_some_and(|obj| obj.zone == engine::types::zones::Zone::Battlefield), + "the game must end (P1 loses) before the destroy spell gets a chance to resolve, so Serpent Society must still be on the battlefield" + ); +} diff --git a/crates/mtgish-import/src/convert/action.rs b/crates/mtgish-import/src/convert/action.rs index edec390cce..f6ad849b76 100644 --- a/crates/mtgish-import/src/convert/action.rs +++ b/crates/mtgish-import/src/convert/action.rs @@ -403,6 +403,9 @@ fn rewrite_bound_x_in_ability_cost(cost: &mut AbilityCost, binding: &QuantityExp // CR 118.9: the borrowed keyword cost is read at runtime from the cast // spell's keyword — it carries no X-bound `QuantityExpr` to rewrite. | AbilityCost::KeywordCostOfCastSpell { .. } + // CR 702.21a: `count` is a fixed `u32`, not a `QuantityExpr` — no + // X-bound amount to rewrite. + | AbilityCost::GetPlayerCounters { .. } | AbilityCost::Unimplemented { .. } => 0, } } diff --git a/crates/phase-ai/src/policies/anti_self_harm.rs b/crates/phase-ai/src/policies/anti_self_harm.rs index ebd488f35d..b541da727f 100644 --- a/crates/phase-ai/src/policies/anti_self_harm.rs +++ b/crates/phase-ai/src/policies/anti_self_harm.rs @@ -790,30 +790,88 @@ fn score_target_object(ctx: &PolicyContext<'_>, object_id: ObjectId, beneficial: WardCost::DiscardCard => 1.5, WardCost::Sacrifice { count, .. } => *count as f64 * 2.0, WardCost::Waterbend(cost) => (cost.mana_value() as f64 / 2.0).min(2.0), - // CR 702.21a + CR 122.1 + CR 104.3d: voluntarily taking - // poison (or other player) counters is a severe cost — - // uncapped, scaled by count, since poison specifically - // progresses toward an unconditional loss at 10. - WardCost::GetPlayerCounters { count, .. } => *count as f64 * 3.0, + // CR 702.21a + CR 122.1 + CR 104.3d: giving yourself + // player counters is only a real cost for poison, + // which progresses toward an unconditional loss at 10 + // (`LETHAL_POISON`) — a payment that would reach or + // cross that threshold must never be scored as an + // ordinary affordable, low-severity ward cost, so it + // gets the same prohibitive magnitude as the + // protection-prevents-targeting case above. Below the + // threshold it scales linearly like Sacrifice. Other + // counter kinds (experience/rad/ticket) carry no loss + // condition, so receiving them is not a self-harm + // signal. + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count, + } => { + let current = + ctx.state.players[ctx.ai_player.0 as usize].poison_counters; + if current.saturating_add(*count) + >= crate::features::poison::LETHAL_POISON + { + 100.0 + } else { + *count as f64 * 3.0 + } + } + WardCost::GetPlayerCounters { .. } => 0.0, // CR 702.21a: Compound costs sum severity of components. - WardCost::Compound(costs) => costs - .iter() - .map(|c| match c { - WardCost::Mana(cost) => (cost.mana_value() as f64 / 2.0).min(2.0), - WardCost::PayLife(amount) => (*amount as f64 / 3.0).min(2.0), - WardCost::PayLifeEqualToPower => { - (object.power.unwrap_or(0).max(0) as f64 / 3.0).min(2.0) - } - WardCost::DiscardCard => 1.5, - WardCost::Sacrifice { count, .. } => *count as f64 * 2.0, - WardCost::Waterbend(cost) => { - (cost.mana_value() as f64 / 2.0).min(2.0) - } - WardCost::GetPlayerCounters { count, .. } => *count as f64 * 3.0, - WardCost::Compound(_) => 2.0, - }) - .sum::() - .min(4.0), + // A lethal poison sub-cost must bypass the ordinary + // sum's `.min(4.0)` cap — otherwise it would be + // clamped down to an ordinary-looking severity, + // exactly the outcome the top-level arm above exists + // to prevent. + WardCost::Compound(costs) => { + let has_lethal_poison = costs.iter().any(|c| { + matches!( + c, + WardCost::GetPlayerCounters { + counter_kind: + engine::types::player::PlayerCounterKind::Poison, + count, + } if ctx.state.players[ctx.ai_player.0 as usize] + .poison_counters + .saturating_add(*count) + >= crate::features::poison::LETHAL_POISON + ) + }); + if has_lethal_poison { + 100.0 + } else { + costs + .iter() + .map(|c| match c { + WardCost::Mana(cost) => { + (cost.mana_value() as f64 / 2.0).min(2.0) + } + WardCost::PayLife(amount) => { + (*amount as f64 / 3.0).min(2.0) + } + WardCost::PayLifeEqualToPower => { + (object.power.unwrap_or(0).max(0) as f64 / 3.0).min(2.0) + } + WardCost::DiscardCard => 1.5, + WardCost::Sacrifice { count, .. } => *count as f64 * 2.0, + WardCost::Waterbend(cost) => { + (cost.mana_value() as f64 / 2.0).min(2.0) + } + // Non-lethal poison payments scale like + // the top-level arm; non-poison kinds + // carry no self-harm signal. + WardCost::GetPlayerCounters { + counter_kind: + engine::types::player::PlayerCounterKind::Poison, + count, + } => *count as f64 * 3.0, + WardCost::GetPlayerCounters { .. } => 0.0, + WardCost::Compound(_) => 2.0, + }) + .sum::() + .min(4.0) + } + } }; score += ctx.penalties().ward_cost_penalty_base * severity; break; From 601052337e093c536af744066f3e60f14d4737d8 Mon Sep 17 00:00:00 2001 From: hurryup52 Date: Sun, 26 Jul 2026 14:49:05 +0200 Subject: [PATCH 04/14] fix: nested guard pattern (unstable) and TS discriminated-union narrowing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real compile/type errors caught by CI on the previous push: - crates/engine/tests/integration/serpent_society_ward_poison_cost.rs: `WaitingFor::GameOver { winner: Some(p) if p == P0 }` nests the guard inside the `Some(p)` sub-pattern, which is the unstable `guard_patterns` feature (rust-lang/rust#129967), not a stable match-arm guard. Moved the guard to the whole-pattern position: `WaitingFor::GameOver { winner: Some(p) } if p == P0` — standard, stable Rust. Verified in isolation with a standalone rustc invocation (this sandbox can't compile the full integration test binary). - client/src/pages/GamePage.tsx: `switch (cost.type)` / a `cost.type === "GetPlayerCounters"` check can't actually narrow the discriminated union to the `GetPlayerCounters` member, because the sibling member's `type: string` is too wide for literal-comparison narrowing to exclude it — `cost.counter_kind` doesn't type-check inside that branch. Switched to a `"counter_kind" in cost` property-presence check before the switch, which narrows correctly since the sibling member doesn't declare that property at all. Verified locally this time (pnpm/tsc/vitest are available in this sandbox, unlike full Rust test codegen): `tsc -b --noEmit` clean, `eslint` clean (pre-existing warning only, unrelated line), and the full `vitest run` suite green (270 files / 2366 tests passed). --- client/src/pages/GamePage.tsx | 22 +++++++++++-------- .../serpent_society_ward_poison_cost.rs | 4 +--- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/client/src/pages/GamePage.tsx b/client/src/pages/GamePage.tsx index 5281090263..b8a3f87813 100644 --- a/client/src/pages/GamePage.tsx +++ b/client/src/pages/GamePage.tsx @@ -3457,6 +3457,19 @@ function formatUnlessCost( }, t: TFunction<"game">, ): string { + // `"counter_kind" in cost` narrows via property presence rather than a + // `cost.type` literal comparison — the sibling union member's `type: string` + // is too wide for a `switch (cost.type)`/`cost.type === "GetPlayerCounters"` + // check to exclude it, so `cost.counter_kind` would otherwise fail to + // type-check inside that branch. The counter kind is looked up through i18n + // (not interpolated as raw English) so non-English locales get a real + // translated noun, matching how the badges section already localizes + // poison/experience/rad counter names. `cost.counter_kind` is rendered + // exactly as the engine sent it — no lowercasing, no fallback default. + if ("counter_kind" in cost) { + const kind = t(`gamePage.cost.playerCounterKind.${cost.counter_kind}`); + return t("gamePage.cost.playerCounters", { count: cost.count, kind }); + } switch (cost.type) { // Legacy `UnlessCost` JSON (pre-2026-05-09 fold) — preserved for // saved-game compat. @@ -3486,15 +3499,6 @@ function formatUnlessCost( } case "PayEnergy": return t("gamePage.cost.energy", { amount: cost.amount ?? 0 }); - // The counter kind is looked up through i18n (not interpolated as raw - // English) so non-English locales get a real translated noun, matching - // how the badges section already localizes poison/experience/rad counter - // names. `cost.counter_kind` is rendered exactly as the engine sent it — - // no lowercasing, no fallback default. - case "GetPlayerCounters": { - const kind = t(`gamePage.cost.playerCounterKind.${cost.counter_kind}`); - return t("gamePage.cost.playerCounters", { count: cost.count, kind }); - } default: return t("gamePage.cost.generic"); } diff --git a/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs b/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs index 6bed48e249..51b801d76f 100644 --- a/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs +++ b/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs @@ -197,9 +197,7 @@ fn serpent_society_ward_payment_that_reaches_ten_poison_loses_the_game() { assert!( matches!( runner.state().waiting_for, - WaitingFor::GameOver { - winner: Some(p) if p == P0 - } + WaitingFor::GameOver { winner: Some(p) } if p == P0 ), "reaching ten poison must trigger the CR 104.3d loss SBA immediately, got {:?}", runner.state().waiting_for From ca0c22ffd1500fba16e5bf64a1a81331c9ad3915 Mon Sep 17 00:00:00 2001 From: hurryup52 Date: Sun, 26 Jul 2026 21:41:56 +0200 Subject: [PATCH 05/14] fix(ai): reject lethal poison Ward payments and value Rad/noncreature Ward correctly - can_pay_ward_cost now aggregates poison across the whole Compound cost tree before checking lethality once, instead of checking each sub-cost against the same starting total (missed jointly-lethal combinations). - anti_self_harm.rs gives Rad, Experience, and Ticket counters explicit typed severity valuations instead of a zero-cost wildcard fallback. - The Ward severity-pricing block now runs for every permanent type, not just creatures, so an affordable Ward tax on a noncreature target is priced by the AI's judgment layer too. - oracle_keyword.rs's "get N counter(s)" ward-cost parsing is now fully composed from nom combinators instead of ad hoc strip_suffix dispatch, and fails closed (no fallback to a free Mana Ward) on malformed count/kind text. - GamePage.tsx maps the engine's player-counter kind to its i18n key via an exhaustive switch instead of interpolating the raw engine value into the key template, so a future counter kind fails to compile here rather than silently missing a translation. Addresses matthewevans's review on #6640. --- client/src/pages/GamePage.tsx | 26 ++- crates/engine/src/parser/oracle_keyword.rs | 78 ++++--- .../phase-ai/src/policies/anti_self_harm.rs | 202 +++++++++--------- .../phase-ai/src/policies/strategy_helpers.rs | 38 +++- crates/phase-ai/src/tactical_gate.rs | 138 ++++++++++++ 5 files changed, 335 insertions(+), 147 deletions(-) diff --git a/client/src/pages/GamePage.tsx b/client/src/pages/GamePage.tsx index b8a3f87813..9ac4159d9b 100644 --- a/client/src/pages/GamePage.tsx +++ b/client/src/pages/GamePage.tsx @@ -3461,13 +3461,27 @@ function formatUnlessCost( // `cost.type` literal comparison — the sibling union member's `type: string` // is too wide for a `switch (cost.type)`/`cost.type === "GetPlayerCounters"` // check to exclude it, so `cost.counter_kind` would otherwise fail to - // type-check inside that branch. The counter kind is looked up through i18n - // (not interpolated as raw English) so non-English locales get a real - // translated noun, matching how the badges section already localizes - // poison/experience/rad counter names. `cost.counter_kind` is rendered - // exactly as the engine sent it — no lowercasing, no fallback default. + // type-check inside that branch. The exhaustive switch below maps each + // engine value to its i18n key explicitly rather than interpolating + // `cost.counter_kind` directly into the key template — that would make the + // display layer depend on the engine's serde string matching the locale + // JSON's key names, an implicit coupling the compiler can't check. This way + // a future `PlayerCounterKind` variant fails to compile here instead of + // silently rendering a missing translation. if ("counter_kind" in cost) { - const kind = t(`gamePage.cost.playerCounterKind.${cost.counter_kind}`); + const kindKey: "Poison" | "Experience" | "Rad" | "Ticket" = (() => { + switch (cost.counter_kind) { + case "Poison": + return "Poison"; + case "Experience": + return "Experience"; + case "Rad": + return "Rad"; + case "Ticket": + return "Ticket"; + } + })(); + const kind = t(`gamePage.cost.playerCounterKind.${kindKey}`); return t("gamePage.cost.playerCounters", { count: cost.count, kind }); } switch (cost.type) { diff --git a/crates/engine/src/parser/oracle_keyword.rs b/crates/engine/src/parser/oracle_keyword.rs index 48ec6dde95..cc73c8b806 100644 --- a/crates/engine/src/parser/oracle_keyword.rs +++ b/crates/engine/src/parser/oracle_keyword.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; -use crate::parser::oracle_nom::error::OracleError; +use crate::parser::oracle_nom::error::{OracleError, OracleResult}; use nom::branch::alt; use nom::bytes::complete::{tag, take_until}; use nom::character::complete::{alpha1, space0, space1}; @@ -700,6 +700,34 @@ fn parse_ward_cost(cost_text: &str) -> Option { Some(Keyword::Ward(cost)) } +/// CR 702.21a + CR 122.1 + CR 104.3d: "get N counter(s)" / "get a/an +/// counter" — the single grammatical authority for this ward-cost +/// family. Composes the count/article, kind, and singular/plural axes as +/// independent nom combinators (this repo's mandated style) rather than +/// enumerating their product as ad-hoc string dispatch. +fn parse_get_player_counters_ward_cost(input: &str) -> OracleResult<'_, WardCost> { + all_consuming(|i| { + let (rest, _) = tag::<_, _, OracleError<'_>>("get ").parse(i)?; + let (rest, count) = alt(( + nom_primitives::parse_number, + value(1u32, nom_primitives::parse_article), + )) + .parse(rest)?; + let (rest, _) = space0.parse(rest)?; + let (rest, counter_kind) = nom_primitives::parse_player_counter_kind.parse(rest)?; + let (rest, _) = tag(" counter").parse(rest)?; + let (rest, _) = opt(tag("s")).parse(rest)?; + Ok(( + rest, + WardCost::GetPlayerCounters { + counter_kind, + count, + }, + )) + }) + .parse(input) +} + /// Parse a single ward cost component (not compound). fn parse_ward_cost_single(lower: &str) -> Option { // CR 702.21a + CR 608.2h + CR 113.7a: Ward's life cost reads the source's @@ -755,37 +783,23 @@ fn parse_ward_cost_single(lower: &str) -> Option { // counters."). MUST run before the mana-cost fallback below, which // otherwise silently parses unrecognized cost text with no mana // symbols/braces as a free, always-paid Ward (phase-rs/phase#6640). - if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("get ").parse(lower) { - if let Some(before_counter) = rest - .strip_suffix(" counters") - .or_else(|| rest.strip_suffix(" counter")) - { - let parsed = nom_primitives::parse_number - .parse(before_counter) - .map(|(kind_word, n)| (n, kind_word.trim())) - .or_else(|_: nom::Err>| { - nom_primitives::parse_article - .parse(before_counter) - .map(|(kind_word, ())| (1, kind_word.trim())) - }); - if let Ok((count, kind_word)) = parsed { - if let Ok((_, counter_kind)) = - all_consuming(nom_primitives::parse_player_counter_kind).parse(kind_word) - { - return Some(WardCost::GetPlayerCounters { - counter_kind, - count, - }); - } - } - // CR 702.21a: recognized as counter-shaped ("get ... counter(s)") - // but the count or counter kind didn't parse — fail closed rather - // than falling through to the mana-cost fallback below, which - // would otherwise silently produce a free, always-paid Ward for - // unsupported/malformed counter text (phase-rs/phase#6640's exact - // bug class, for different malformed input). - return None; - } + // + // One grammatical authority over the count/article, kind, and + // singular/plural axes — composed nom combinators, not string-suffix + // dispatch — so this parser family has a single production to extend + // rather than ad-hoc per-branch string handling. + if tag::<_, _, OracleError<'_>>("get ").parse(lower).is_ok() { + return match parse_get_player_counters_ward_cost(lower) { + Ok((_, cost)) => Some(cost), + // CR 702.21a: recognized as counter-shaped ("get ...") but the + // count, kind, or "counter(s)" tail didn't parse in full — fail + // closed rather than falling through to the mana-cost fallback + // below, which would otherwise silently produce a free, + // always-paid Ward for unsupported/malformed counter text + // (phase-rs/phase#6640's exact bug class, for different + // malformed input). + Err(_) => None, + }; } // Fall back to mana cost parsing diff --git a/crates/phase-ai/src/policies/anti_self_harm.rs b/crates/phase-ai/src/policies/anti_self_harm.rs index b541da727f..a7a9cd19c6 100644 --- a/crates/phase-ai/src/policies/anti_self_harm.rs +++ b/crates/phase-ai/src/policies/anti_self_harm.rs @@ -772,112 +772,6 @@ fn score_target_object(ctx: &PolicyContext<'_>, object_id: ObjectId, beneficial: } } - // Price the cost of an *affordable* ward (must pay an extra cost). - // An unaffordable ward is hard-rejected upstream by `tactical_gate` - // (CR 702.21a — the spell would just be countered), so this judgment - // layer never double-scores that case. - for keyword in &object.keywords { - if let Keyword::Ward(ward_cost) = keyword { - if !can_pay_ward_cost(ctx, ward_cost, object) { - break; - } - let severity = match ward_cost { - WardCost::Mana(cost) => (cost.mana_value() as f64 / 2.0).min(2.0), - WardCost::PayLife(amount) => (*amount as f64 / 3.0).min(2.0), - WardCost::PayLifeEqualToPower => { - (object.power.unwrap_or(0).max(0) as f64 / 3.0).min(2.0) - } - WardCost::DiscardCard => 1.5, - WardCost::Sacrifice { count, .. } => *count as f64 * 2.0, - WardCost::Waterbend(cost) => (cost.mana_value() as f64 / 2.0).min(2.0), - // CR 702.21a + CR 122.1 + CR 104.3d: giving yourself - // player counters is only a real cost for poison, - // which progresses toward an unconditional loss at 10 - // (`LETHAL_POISON`) — a payment that would reach or - // cross that threshold must never be scored as an - // ordinary affordable, low-severity ward cost, so it - // gets the same prohibitive magnitude as the - // protection-prevents-targeting case above. Below the - // threshold it scales linearly like Sacrifice. Other - // counter kinds (experience/rad/ticket) carry no loss - // condition, so receiving them is not a self-harm - // signal. - WardCost::GetPlayerCounters { - counter_kind: engine::types::player::PlayerCounterKind::Poison, - count, - } => { - let current = - ctx.state.players[ctx.ai_player.0 as usize].poison_counters; - if current.saturating_add(*count) - >= crate::features::poison::LETHAL_POISON - { - 100.0 - } else { - *count as f64 * 3.0 - } - } - WardCost::GetPlayerCounters { .. } => 0.0, - // CR 702.21a: Compound costs sum severity of components. - // A lethal poison sub-cost must bypass the ordinary - // sum's `.min(4.0)` cap — otherwise it would be - // clamped down to an ordinary-looking severity, - // exactly the outcome the top-level arm above exists - // to prevent. - WardCost::Compound(costs) => { - let has_lethal_poison = costs.iter().any(|c| { - matches!( - c, - WardCost::GetPlayerCounters { - counter_kind: - engine::types::player::PlayerCounterKind::Poison, - count, - } if ctx.state.players[ctx.ai_player.0 as usize] - .poison_counters - .saturating_add(*count) - >= crate::features::poison::LETHAL_POISON - ) - }); - if has_lethal_poison { - 100.0 - } else { - costs - .iter() - .map(|c| match c { - WardCost::Mana(cost) => { - (cost.mana_value() as f64 / 2.0).min(2.0) - } - WardCost::PayLife(amount) => { - (*amount as f64 / 3.0).min(2.0) - } - WardCost::PayLifeEqualToPower => { - (object.power.unwrap_or(0).max(0) as f64 / 3.0).min(2.0) - } - WardCost::DiscardCard => 1.5, - WardCost::Sacrifice { count, .. } => *count as f64 * 2.0, - WardCost::Waterbend(cost) => { - (cost.mana_value() as f64 / 2.0).min(2.0) - } - // Non-lethal poison payments scale like - // the top-level arm; non-poison kinds - // carry no self-harm signal. - WardCost::GetPlayerCounters { - counter_kind: - engine::types::player::PlayerCounterKind::Poison, - count, - } => *count as f64 * 3.0, - WardCost::GetPlayerCounters { .. } => 0.0, - WardCost::Compound(_) => 2.0, - }) - .sum::() - .min(4.0) - } - } - }; - score += ctx.penalties().ward_cost_penalty_base * severity; - break; - } - } - // Removal quality mismatch: penalize premium removal on cheap targets if let Some(source) = ctx.source_object() { let spell_mv = source.mana_cost.mana_value(); @@ -948,6 +842,102 @@ fn score_target_object(ctx: &PolicyContext<'_>, object_id: ObjectId, beneficial: score += controller_delta * noncreature_value; } + // Price the cost of an *affordable* ward (must pay an extra cost). Applies to every + // permanent type, not just creatures — a Ward-bearing artifact/enchantment/planeswalker + // is exactly as real a target-choice cost as a Ward-bearing creature. An unaffordable ward + // is hard-rejected upstream by `tactical_gate` (CR 702.21a — the spell would just be + // countered), so this judgment layer never double-scores that case. + if !beneficial { + for keyword in &object.keywords { + if let Keyword::Ward(ward_cost) = keyword { + if !can_pay_ward_cost(ctx, ward_cost, object) { + break; + } + let severity = match ward_cost { + WardCost::Mana(cost) => (cost.mana_value() as f64 / 2.0).min(2.0), + WardCost::PayLife(amount) => (*amount as f64 / 3.0).min(2.0), + WardCost::PayLifeEqualToPower => { + (object.power.unwrap_or(0).max(0) as f64 / 3.0).min(2.0) + } + WardCost::DiscardCard => 1.5, + WardCost::Sacrifice { count, .. } => *count as f64 * 2.0, + WardCost::Waterbend(cost) => (cost.mana_value() as f64 / 2.0).min(2.0), + // CR 702.21a + CR 122.1 + CR 728.1: giving yourself + // player counters has an explicit, kind-specific + // valuation — no wildcard fallback, so a future + // supported counter kind forces a deliberate + // decision here. A lethal poison payment never + // reaches this scoring at all — `can_pay_ward_cost` + // above already rejects it (reframed as "can't + // rationally pay"), so the Poison arm only ever sees + // sub-lethal, ordinary-severity payments. + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count, + } => *count as f64 * 3.0, + // CR 728.1: each rad counter mills a card and, if that + // card is nonland, costs 1 life. Real cost, not + // harmless — approximated as PayLife's per-life + // severity (amount/3.0) scaled by ~0.6 (typical + // nonland fraction of a deck), i.e. ~0.2 per counter. + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Rad, + count, + } => (*count as f64 * 0.2).min(2.0), + // Experience/ticket counters carry no loss-condition + // or resource-drain risk — purely beneficial or + // neutral to receive. + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Experience, + .. + } => 0.0, + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Ticket, + .. + } => 0.0, + // CR 702.21a: Compound costs sum severity of components. + // A Compound containing a lethal poison sub-cost is + // also already rejected upstream by `can_pay_ward_cost` + // (which requires every sub-cost payable), so this + // fold only ever sees payable compounds. + WardCost::Compound(costs) => costs + .iter() + .map(|c| match c { + WardCost::Mana(cost) => (cost.mana_value() as f64 / 2.0).min(2.0), + WardCost::PayLife(amount) => (*amount as f64 / 3.0).min(2.0), + WardCost::PayLifeEqualToPower => { + (object.power.unwrap_or(0).max(0) as f64 / 3.0).min(2.0) + } + WardCost::DiscardCard => 1.5, + WardCost::Sacrifice { count, .. } => *count as f64 * 2.0, + WardCost::Waterbend(cost) => (cost.mana_value() as f64 / 2.0).min(2.0), + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count, + } => *count as f64 * 3.0, + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Rad, + count, + } => (*count as f64 * 0.2).min(2.0), + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Experience, + .. + } => 0.0, + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Ticket, + .. + } => 0.0, + WardCost::Compound(_) => 2.0, + }) + .sum::() + .min(4.0), + }; + score += ctx.penalties().ward_cost_penalty_base * severity; + break; + } + } + } + score } diff --git a/crates/phase-ai/src/policies/strategy_helpers.rs b/crates/phase-ai/src/policies/strategy_helpers.rs index e2ab1a5d66..e899c2b16e 100644 --- a/crates/phase-ai/src/policies/strategy_helpers.rs +++ b/crates/phase-ai/src/policies/strategy_helpers.rs @@ -733,6 +733,24 @@ pub(crate) fn available_mana_after_spell(ctx: &PolicyContext<'_>) -> u32 { sources.saturating_sub(spell_cost) } +/// CR 104.3d: total poison counters `ward` would give the payer, summed +/// across every `GetPlayerCounters { Poison, .. }` sub-cost in the whole +/// tree — a `Compound` cost's sub-costs are all paid together (CR 702.21a: +/// "every conjoined sub-cost must be payable"), not independently, so two +/// individually-nonlethal poison sub-costs can be jointly lethal and must be +/// checked against their COMBINED total, not each against the same +/// unchanged starting count. +fn total_poison_from_ward_cost(ward: &WardCost) -> u32 { + match ward { + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count, + } => *count, + WardCost::Compound(costs) => costs.iter().map(total_poison_from_ward_cost).sum(), + _ => 0, + } +} + /// CR 702.21a: Whether the AI can pay `ward` after committing to the spell it is /// casting. Mana / Waterbend costs use the post-spell mana estimate; non-mana /// costs check the corresponding resource (life, a spare card, sacrificeable @@ -743,6 +761,19 @@ pub(crate) fn can_pay_ward_cost( ward: &WardCost, warded: &GameObject, ) -> bool { + // CR 104.3d: reject up front if the AGGREGATE poison this cost would + // give (direct or across every Compound sub-cost) reaches or crosses + // `LETHAL_POISON` — checked once, against the combined total, before any + // per-variant mechanical-affordability logic below. The AI must never + // treat ending its own game as an ordinary payable cost, for direct or + // compound Ward alike. + let total_poison = total_poison_from_ward_cost(ward); + if total_poison > 0 { + let current = ctx.state.players[ctx.ai_player.0 as usize].poison_counters; + if current.saturating_add(total_poison) >= crate::features::poison::LETHAL_POISON { + return false; + } + } match ward { WardCost::Mana(cost) | WardCost::Waterbend(cost) => { available_mana_after_spell(ctx) >= cost.mana_value() @@ -780,9 +811,10 @@ pub(crate) fn can_pay_ward_cost( .count(); matching as u32 >= *count } - // CR 702.21a + CR 122.1: no affordability limit — a player can always - // choose to accept more counters (mirrors the engine's own - // `can_pay_resolution` for this cost). + // CR 702.21a + CR 122.1: mechanically always payable — no resource + // limit on giving yourself more counters (mirrors the engine's own + // `can_pay_resolution`). Lethal poison is already rejected by the + // aggregate check above, for direct and compound costs alike. WardCost::GetPlayerCounters { .. } => true, // CR 702.21a: every conjoined sub-cost must be payable. Mana contention // between multiple mana sub-costs is approximated (each checked against diff --git a/crates/phase-ai/src/tactical_gate.rs b/crates/phase-ai/src/tactical_gate.rs index c7cd16897c..8d9634c13c 100644 --- a/crates/phase-ai/src/tactical_gate.rs +++ b/crates/phase-ai/src/tactical_gate.rs @@ -1158,4 +1158,142 @@ mod tests { assert_ne!(gate_for(8, 7), GateDecision::Reject); assert_ne!(gate_for(4, 3), GateDecision::Reject); } + + /// CR 702.21a + CR 104.3d: a Ward payment that would push the AI to ten + /// or more poison counters must reject the target — the AI must never + /// treat ending its own game as an ordinary payable ward cost. + #[test] + fn rejects_targeting_ward_that_would_be_lethal_poison() { + let mut scenario = GameScenario::new(); + let creature = scenario + .add_creature(P1, "Warded", 2, 2) + .with_keyword(Keyword::Ward(WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count: 5, + })) + .id(); + let mut runner = scenario.build(); + let state = runner.state_mut(); + state.players[P0.0 as usize].poison_counters = 5; + let decision = damage_target_decision(creature, 3); + let candidate = choose_target_candidate(creature); + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let ctx = PolicyContext { + state, + decision: &decision, + candidate: &candidate, + ai_player: P0, + config: &config, + context: &AiContext::empty(&config.weights), + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_eq!(assess_candidate(&ctx), GateDecision::Reject); + } + + /// A poison Ward payment that stays below the ten-poison threshold does + /// not gate the target out — mirrors `allows_targeting_payable_ward`. + #[test] + fn allows_targeting_ward_with_nonlethal_poison_payment() { + let mut scenario = GameScenario::new(); + let creature = scenario + .add_creature(P1, "Warded", 2, 2) + .with_keyword(Keyword::Ward(WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count: 5, + })) + .id(); + let mut runner = scenario.build(); + let state = runner.state_mut(); + // P0 starts at 0 poison — 0 + 5 = 5, well below the 10-poison SBA. + let decision = damage_target_decision(creature, 3); + let candidate = choose_target_candidate(creature); + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let ctx = PolicyContext { + state, + decision: &decision, + candidate: &candidate, + ai_player: P0, + config: &config, + context: &AiContext::empty(&config.weights), + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_ne!(assess_candidate(&ctx), GateDecision::Reject); + } + + /// CR 702.21a + CR 104.3d: two individually-nonlethal poison sub-costs in + /// a `Compound` Ward can be jointly lethal — the aggregate across every + /// sub-cost must be checked, not each sub-cost against the same + /// unchanged starting total. + #[test] + fn rejects_targeting_ward_with_jointly_lethal_compound_poison() { + let mut scenario = GameScenario::new(); + let creature = scenario + .add_creature(P1, "Warded", 2, 2) + .with_keyword(Keyword::Ward(WardCost::Compound(vec![ + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count: 3, + }, + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count: 3, + }, + ]))) + .id(); + let mut runner = scenario.build(); + let state = runner.state_mut(); + // 4 existing + 3 + 3 = 10 (lethal), but 4 + 3 = 7 alone is not — a + // per-sub-cost check against the same starting total would wrongly + // allow this. + state.players[P0.0 as usize].poison_counters = 4; + let decision = damage_target_decision(creature, 3); + let candidate = choose_target_candidate(creature); + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let ctx = PolicyContext { + state, + decision: &decision, + candidate: &candidate, + ai_player: P0, + config: &config, + context: &AiContext::empty(&config.weights), + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_eq!(assess_candidate(&ctx), GateDecision::Reject); + } + + /// CR 702.21a: the ward-affordability gate applies to any targetable + /// permanent, not just creatures — a lethal poison Ward on a noncreature + /// permanent must be rejected identically. + #[test] + fn rejects_targeting_noncreature_ward_that_would_be_lethal_poison() { + let mut scenario = GameScenario::new(); + let artifact = scenario + .add_creature(P1, "Warded Artifact", 0, 0) + .as_artifact() + .with_keyword(Keyword::Ward(WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count: 5, + })) + .id(); + let mut runner = scenario.build(); + let state = runner.state_mut(); + state.players[P0.0 as usize].poison_counters = 5; + let decision = damage_target_decision(artifact, 3); + let candidate = choose_target_candidate(artifact); + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let ctx = PolicyContext { + state, + decision: &decision, + candidate: &candidate, + ai_player: P0, + config: &config, + context: &AiContext::empty(&config.weights), + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_eq!(assess_candidate(&ctx), GateDecision::Reject); + } } From 92557e4d332c11235362bc727ef9c27089519063 Mon Sep 17 00:00:00 2001 From: hurryup52 Date: Sun, 26 Jul 2026 23:54:48 +0200 Subject: [PATCH 06/14] test(ai): cover noncreature Ward severity scoring in AntiSelfHarmPolicy The prior fix moved Ward severity pricing out of the creature-only branch so it runs for every permanent type, but nothing exercised the production target scorer directly for a noncreature case -- only tactical_gate's accept/reject GateDecision tests covered it. Adds a regression asserting an unwarded artifact scores higher (a more attractive target) than an otherwise-identical artifact with a small, payable, nonlethal poison-counter Ward. --- .../phase-ai/src/policies/anti_self_harm.rs | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/crates/phase-ai/src/policies/anti_self_harm.rs b/crates/phase-ai/src/policies/anti_self_harm.rs index a7a9cd19c6..c51cdd05c6 100644 --- a/crates/phase-ai/src/policies/anti_self_harm.rs +++ b/crates/phase-ai/src/policies/anti_self_harm.rs @@ -3601,6 +3601,156 @@ mod tests { ); } + #[test] + fn noncreature_ward_target_scores_lower_than_unwarded_equivalent() { + let mut state = make_state(); + + // Two identical artifacts (non-creature): one bare, one with a small, payable, + // nonlethal poison-counter Ward. Both owned by the opponent (PlayerId(1)) so removal + // targeting them is non-beneficial from the AI's (PlayerId(0)) perspective. + let bare_card_id = CardId(state.next_object_id); + let bare_artifact = create_object( + &mut state, + bare_card_id, + PlayerId(1), + "Prophetic Prism".to_string(), + Zone::Battlefield, + ); + state + .objects + .get_mut(&bare_artifact) + .unwrap() + .card_types + .core_types + .push(engine::types::card_type::CoreType::Artifact); + + let warded_card_id = CardId(state.next_object_id); + let warded_artifact = create_object( + &mut state, + warded_card_id, + PlayerId(1), + "Warded Relic".to_string(), + Zone::Battlefield, + ); + let warded_obj = state.objects.get_mut(&warded_artifact).unwrap(); + warded_obj + .card_types + .core_types + .push(engine::types::card_type::CoreType::Artifact); + warded_obj + .keywords + .push(Keyword::Ward(WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count: 2, + })); + + // Set up pending trigger with a removal (exile) effect, matching + // `trigger_target_prefers_creature_over_token` above. + state.pending_trigger = Some(engine::game::triggers::PendingTrigger { + source_id: ObjectId(200), + controller: PlayerId(0), + condition: None, + ability: ResolvedAbility::new( + Effect::ChangeZone { + origin: None, + destination: Zone::Exile, + target: TargetFilter::Any, + 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![], + face_down_profile: None, + enters_modified_if: None, + }, + Vec::new(), + ObjectId(200), + PlayerId(0), + ), + timestamp: 1, + target_constraints: Vec::new(), + distribute: None, + trigger_event: None, + modal: None, + mode_abilities: vec![], + description: None, + may_trigger_origin: None, + subject_match_count: None, + die_result: None, + }); + + let config = AiConfig::default(); + let legal_targets = vec![ + TargetRef::Object(bare_artifact), + TargetRef::Object(warded_artifact), + ]; + let decision = AiDecisionContext { + waiting_for: WaitingFor::TriggerTargetSelection { + player: PlayerId(0), + trigger_controller: None, + trigger_event: None, + trigger_events: Vec::new(), + target_slots: vec![TargetSelectionSlot { + legal_targets: legal_targets.clone(), + optional: false, + chooser: None, + }], + mode_labels: Vec::new(), + target_constraints: Vec::new(), + selection: Default::default(), + source_id: Some(ObjectId(200)), + description: None, + }, + candidates: Vec::new(), + }; + + // Score targeting the bare artifact + let bare_candidate = CandidateAction { + action: GameAction::ChooseTarget { + target: Some(TargetRef::Object(bare_artifact)), + }, + metadata: ActionMetadata::for_actor(Some(PlayerId(0)), TacticalClass::Target), + }; + let bare_ctx = PolicyContext { + state: &state, + decision: &decision, + candidate: &bare_candidate, + ai_player: PlayerId(0), + config: &config, + context: &crate::context::AiContext::empty(&config.weights), + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + let bare_score = AntiSelfHarmPolicy.score(&bare_ctx); + + // Score targeting the warded artifact + let warded_candidate = CandidateAction { + action: GameAction::ChooseTarget { + target: Some(TargetRef::Object(warded_artifact)), + }, + metadata: ActionMetadata::for_actor(Some(PlayerId(0)), TacticalClass::Target), + }; + let warded_ctx = PolicyContext { + state: &state, + decision: &decision, + candidate: &warded_candidate, + ai_player: PlayerId(0), + config: &config, + context: &crate::context::AiContext::empty(&config.weights), + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + let warded_score = AntiSelfHarmPolicy.score(&warded_ctx); + + assert!( + bare_score > warded_score, + "Should prefer targeting the unwarded artifact ({bare_score}) over the poison-Ward artifact ({warded_score})" + ); + } + #[test] fn trigger_target_effects_are_extracted() { let mut state = make_state(); From c40a6402391e14c29e3c23d4360da2e06e7eceb8 Mon Sep 17 00:00:00 2001 From: hurryup52 Date: Mon, 27 Jul 2026 00:46:09 +0200 Subject: [PATCH 07/14] fix(ai): box PendingTrigger/ResolvedAbility construction after upstream rebase Rebased onto upstream/main, which boxed GameState.pending_trigger and PendingTrigger.ability since this branch was created. The new noncreature Ward scoring test still constructed both unboxed. --- crates/phase-ai/src/policies/anti_self_harm.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/phase-ai/src/policies/anti_self_harm.rs b/crates/phase-ai/src/policies/anti_self_harm.rs index c51cdd05c6..96a08f932b 100644 --- a/crates/phase-ai/src/policies/anti_self_harm.rs +++ b/crates/phase-ai/src/policies/anti_self_harm.rs @@ -3646,11 +3646,11 @@ mod tests { // Set up pending trigger with a removal (exile) effect, matching // `trigger_target_prefers_creature_over_token` above. - state.pending_trigger = Some(engine::game::triggers::PendingTrigger { + state.pending_trigger = Some(Box::new(engine::game::triggers::PendingTrigger { source_id: ObjectId(200), controller: PlayerId(0), condition: None, - ability: ResolvedAbility::new( + ability: Box::new(ResolvedAbility::new( Effect::ChangeZone { origin: None, destination: Zone::Exile, @@ -3669,7 +3669,7 @@ mod tests { Vec::new(), ObjectId(200), PlayerId(0), - ), + )), timestamp: 1, target_constraints: Vec::new(), distribute: None, @@ -3680,7 +3680,7 @@ mod tests { may_trigger_origin: None, subject_match_count: None, die_result: None, - }); + })); let config = AiConfig::default(); let legal_targets = vec![ From 0acc31d47aec5f876915adc372f9b425679169ff Mon Sep 17 00:00:00 2001 From: hurryup52 Date: Mon, 27 Jul 2026 01:42:35 +0200 Subject: [PATCH 08/14] fix(engine): treat a prevented player-counter payment as failed, not paid Two rules-correctness bugs in the Ward player-counter cost path: - can_pay_ward_cost trusted WardCost::GetPlayerCounters's printed count when checking poison lethality, while actual payment routes counter additions through the replacement pipeline. A doubler or +N effect on the payer could make a lethal payment look safe. Added preview_player_counter_addition (mirrors the existing object-counter preview_counter_addition) to project the real, replacement-adjusted result side-effect-free; total_poison_from_ward_cost now uses it and conservatively declines when the outcome can't be cleanly projected. - add_player_counter_with_replacement returned true for a replacement- prevented addition (e.g. Solemnity's "players can't get counters"), identical to a genuinely applied one. Effect-resolution callers correctly treat prevented and applied the same way (nothing more to wait for), but the GetPlayerCounters cost arm in costs.rs used that same true to mean "paid" -- so a Ward's poison-counter cost could be paid for free under a counter-prevention effect, bypassing Ward entirely. The function now returns a PlayerCounterAdditionOutcome (Applied/Prevented/NeedsChoice); the cost arm maps Prevented to a failed payment so Ward correctly counters the spell instead. Adds a doubling-replacement boundary test (tactical_gate.rs) and a Solemnity + Serpent Society integration test asserting the payment fails and the targeting spell is countered. --- crates/engine/src/game/costs.rs | 24 +++- crates/engine/src/game/effects/counters.rs | 18 +-- crates/engine/src/game/effects/deal_damage.rs | 10 +- .../engine/src/game/effects/player_counter.rs | 133 ++++++++++++++++-- crates/engine/src/game/effects/proliferate.rs | 18 +-- .../serpent_society_ward_poison_cost.rs | 60 ++++++++ .../phase-ai/src/policies/strategy_helpers.rs | 85 ++++++++--- crates/phase-ai/src/tactical_gate.rs | 55 ++++++++ 8 files changed, 344 insertions(+), 59 deletions(-) diff --git a/crates/engine/src/game/costs.rs b/crates/engine/src/game/costs.rs index 4fcc77ae31..e8d727a20d 100644 --- a/crates/engine/src/game/costs.rs +++ b/crates/engine/src/game/costs.rs @@ -1390,17 +1390,31 @@ fn pay_ability_cost_inner( // `resolve_and_apply_player_edit` call — so "players can't get // counters" replacement effects still apply, mirroring the // `EffectCost`/`PutCounter` arm's use of the sibling - // `effects::counters::add_counter_with_replacement` above. + // `effects::counters::add_counter_with_replacement` above. A + // replacement that PREVENTS the addition (Solemnity) is a genuinely + // FAILED payment here, not a paused one: unlike effect resolution + // (where "prevented" and "applied" both just mean the pending item is + // resolved), a cost that silently gives zero counters must not be + // mistaken for having actually been paid, or Ward's deterrent is + // bypassed for free. AbilityCost::GetPlayerCounters { counter_kind, count, } => { - if !super::effects::player_counter::add_player_counter_with_replacement( + match super::effects::player_counter::add_player_counter_with_replacement( state, player, player, *counter_kind, *count, events, ) { - return Ok(PaymentOutcome::Paused { - remaining_cost: None, - }); + super::effects::player_counter::PlayerCounterAdditionOutcome::Applied => {} + super::effects::player_counter::PlayerCounterAdditionOutcome::Prevented => { + return Ok(payment_failed( + "Player-counter cost prevented by a replacement effect", + )); + } + super::effects::player_counter::PlayerCounterAdditionOutcome::NeedsChoice => { + return Ok(PaymentOutcome::Paused { + remaining_cost: None, + }); + } } } AbilityCost::PaySpeed { amount } => { diff --git a/crates/engine/src/game/effects/counters.rs b/crates/engine/src/game/effects/counters.rs index 61eb335bf0..3797ccfab4 100644 --- a/crates/engine/src/game/effects/counters.rs +++ b/crates/engine/src/game/effects/counters.rs @@ -436,14 +436,16 @@ pub(crate) fn drain_pending_counter_additions(state: &mut GameState, events: &mu player_id, counter_kind, count, - } => super::player_counter::add_player_counter_with_replacement( - state, - actor, - player_id, - counter_kind, - count, - events, - ), + } => { + super::player_counter::add_player_counter_with_replacement( + state, + actor, + player_id, + counter_kind, + count, + events, + ) != super::player_counter::PlayerCounterAdditionOutcome::NeedsChoice + } PendingCounterAddition::Energy { actor, player_id, diff --git a/crates/engine/src/game/effects/deal_damage.rs b/crates/engine/src/game/effects/deal_damage.rs index fced3100bf..7f6559a92f 100644 --- a/crates/engine/src/game/effects/deal_damage.rs +++ b/crates/engine/src/game/effects/deal_damage.rs @@ -568,14 +568,15 @@ pub(crate) fn apply_damage_after_replacement( // counters. Route through the player-counter replacement pipeline // so "players can't get poison counters" / poison-doublers apply; // the actor is the source's controller. - if !player_counter::add_player_counter_with_replacement( + if player_counter::add_player_counter_with_replacement( state, ctx.controller, *player_id, PlayerCounterKind::Poison, actual_amount, events, - ) { + ) == player_counter::PlayerCounterAdditionOutcome::NeedsChoice + { return DamageResult::NeedsChoice; } } else { @@ -596,14 +597,15 @@ pub(crate) fn apply_damage_after_replacement( // when a creature deals combat damage to a player. Route through // the player-counter replacement pipeline (prevention/doublers); // the actor is the source's controller. - if !player_counter::add_player_counter_with_replacement( + if player_counter::add_player_counter_with_replacement( state, ctx.controller, *player_id, PlayerCounterKind::Poison, ctx.combat_damage_poison, events, - ) { + ) == player_counter::PlayerCounterAdditionOutcome::NeedsChoice + { return DamageResult::NeedsChoice; } } diff --git a/crates/engine/src/game/effects/player_counter.rs b/crates/engine/src/game/effects/player_counter.rs index bd7130cacc..1c947c4715 100644 --- a/crates/engine/src/game/effects/player_counter.rs +++ b/crates/engine/src/game/effects/player_counter.rs @@ -8,6 +8,30 @@ use crate::types::player::{PlayerCounterKind, PlayerId}; use crate::types::proposed_event::{CounterPlacement, ProposedEvent}; use crate::types::resolved_commands::ResolvedPlayerEdit; +/// The replacement-aware outcome of attempting to add player counters. +/// +/// Distinguished from a plain `bool` because callers fall into two families +/// that need `Prevented` handled differently: +/// - Effect resolution (`resolve` below, `deal_damage`'s infect/toxic poison, +/// `proliferate`, the pending-counter-addition drain in `counters.rs`) +/// treats `Applied` and `Prevented` identically — the pending item is fully +/// resolved either way, whether or not any counters actually landed. +/// - Cost payment (`costs.rs`'s `GetPlayerCounters` ability-cost arm) must +/// treat `Prevented` as a FAILED payment: a "players can't get counters" +/// replacement (Solemnity) silently zeroing out a Ward's player-counter +/// cost must not be mistaken for having actually paid it, or Ward's whole +/// deterrent is bypassed for free (CR 702.21a). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PlayerCounterAdditionOutcome { + /// The counters were added (possibly replacement-adjusted in count). + Applied, + /// A replacement effect prevented the counter addition outright. + Prevented, + /// Replacement ordering or an optional replacement needs this player's + /// choice; `state.waiting_for` has already been set. + NeedsChoice, +} + pub fn add_player_counter_with_replacement( state: &mut GameState, actor: PlayerId, @@ -15,9 +39,9 @@ pub fn add_player_counter_with_replacement( counter_kind: PlayerCounterKind, count: u32, events: &mut Vec, -) -> bool { +) -> PlayerCounterAdditionOutcome { if count == 0 { - return true; + return PlayerCounterAdditionOutcome::Applied; } // CR 122.1 + CR 614.17: Player-counter additions pass through the @@ -48,12 +72,100 @@ pub fn add_player_counter_with_replacement( { apply_player_counter_addition(state, player_id, counter_kind, count, events); } - true + PlayerCounterAdditionOutcome::Applied } - replacement::ReplacementResult::Prevented => true, + replacement::ReplacementResult::Prevented => PlayerCounterAdditionOutcome::Prevented, replacement::ReplacementResult::NeedsChoice(player) => { state.waiting_for = replacement::replacement_choice_waiting_for(player, state); - false + PlayerCounterAdditionOutcome::NeedsChoice + } + } +} + +/// The replacement-aware result of previewing a player-counter addition. +/// +/// Mirrors `counters::CounterAdditionPreview` (the object-counter sibling), +/// parameterized for players instead of an object incarnation — players have +/// no incarnation-staleness concept, so there is no `None` "target no longer +/// matches" case here. +/// +/// This is intentionally an engine-internal decision fact rather than wire +/// state: callers use it while evaluating a currently-bound action, so it +/// must not be serialized or retained across turns. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PlayerCounterAdditionPreview { + /// The proposed count reaches the player unchanged. + Applied { count: u32 }, + /// A replacement effect prevents the counter addition. + Prevented, + /// Replacement ordering or an optional replacement needs this player's choice. + ChoiceRequired { player: PlayerId }, + /// Replacement effects change the proposed counter count (e.g. a doubler). + Transformed { count: u32 }, + /// A replacement rewrites the counter event into a different event class. + /// + /// The preview cannot claim that the requested counter was added, so + /// consumers must handle this explicitly rather than treating it as an + /// absent preview. + Unsupported, +} + +/// Preview a player-counter addition through the real replacement pipeline, +/// without mutating live game state. +/// +/// Runs on an isolated clone of `state`, so a tactical caller cannot add +/// pending choices, events, or counters to the live game. Used by +/// `phase-ai`'s Ward-lethality check (`can_pay_ward_cost`) to project the +/// REPLACEMENT-ADJUSTED poison total a payment would actually give — a +/// doubler or +N effect can make a printed count understate the real result, +/// and trusting the printed count alone can let the AI accept a payment that +/// is actually lethal (CR 104.3d). +/// +/// CR 122.1 + CR 614.1: Counter placement is subject to applicable +/// replacement effects before the event happens. +pub fn preview_player_counter_addition( + state: &GameState, + actor: PlayerId, + player_id: PlayerId, + counter_kind: PlayerCounterKind, + count: u32, +) -> PlayerCounterAdditionPreview { + if count == 0 { + return PlayerCounterAdditionPreview::Applied { count }; + } + + let proposed = ProposedEvent::AddCounter { + placement: CounterPlacement::Player { + actor, + player_id, + counter_kind, + }, + count, + applied: HashSet::new(), + }; + let mut preview_state = state.clone(); + let mut events = Vec::new(); + + match replacement::replace_event(&mut preview_state, proposed, &mut events) { + replacement::ReplacementResult::Execute(ProposedEvent::AddCounter { + count: resulting_count, + .. + }) if resulting_count == count => PlayerCounterAdditionPreview::Applied { + count: resulting_count, + }, + replacement::ReplacementResult::Execute(ProposedEvent::AddCounter { + count: resulting_count, + .. + }) => PlayerCounterAdditionPreview::Transformed { + count: resulting_count, + }, + // A replacement may redirect the event into a different event class. + // The counter-placement fact is explicitly unsupported rather than + // absent, so conservative callers cannot mistake it for "no counters". + replacement::ReplacementResult::Execute(_) => PlayerCounterAdditionPreview::Unsupported, + replacement::ReplacementResult::Prevented => PlayerCounterAdditionPreview::Prevented, + replacement::ReplacementResult::NeedsChoice(player) => { + PlayerCounterAdditionPreview::ChoiceRequired { player } } } } @@ -158,14 +270,9 @@ pub fn resolve( else { continue; }; - if !add_player_counter_with_replacement( - state, - actor, - player_id, - counter_kind, - count, - events, - ) { + if add_player_counter_with_replacement(state, actor, player_id, counter_kind, count, events) + == PlayerCounterAdditionOutcome::NeedsChoice + { super::counters::stash_pending_counter_additions( state, additions[index + 1..].to_vec(), diff --git a/crates/engine/src/game/effects/proliferate.rs b/crates/engine/src/game/effects/proliferate.rs index a5503083d7..275a484d91 100644 --- a/crates/engine/src/game/effects/proliferate.rs +++ b/crates/engine/src/game/effects/proliferate.rs @@ -370,14 +370,16 @@ fn apply_counter_addition_plan_item( player_id, counter_kind, count, - } => super::player_counter::add_player_counter_with_replacement( - state, - actor, - player_id, - counter_kind, - count, - events, - ), + } => { + super::player_counter::add_player_counter_with_replacement( + state, + actor, + player_id, + counter_kind, + count, + events, + ) != super::player_counter::PlayerCounterAdditionOutcome::NeedsChoice + } PendingCounterAddition::Energy { actor, player_id, diff --git a/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs b/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs index 51b801d76f..6a0c822f5d 100644 --- a/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs +++ b/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs @@ -211,3 +211,63 @@ fn serpent_society_ward_payment_that_reaches_ten_poison_loses_the_game() { "the game must end (P1 loses) before the destroy spell gets a chance to resolve, so Serpent Society must still be on the battlefield" ); } + +/// CR 122.1 + CR 614.17 + CR 702.21a: Solemnity's "Players can't get +/// counters" replacement must make Ward's poison-counter cost a FAILED +/// payment, not a free bypass. Before this fix, `add_player_counter_with_ +/// replacement` reported `Prevented` as if it were a paid cost, so the +/// targeting opponent's spell would incorrectly continue resolving even +/// though no poison was actually given — nullifying Ward's entire deterrent +/// for free. Solemnity's real Oracle text is "Players can't get counters. +/// Prevent all damage that would be dealt to permanents by sources with +/// counters on them." — only the first (relevant) sentence is used here. +#[test] +fn serpent_society_ward_payment_prevented_by_solemnity_counters_the_spell() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario + .add_creature_from_oracle(P0, "Solemnity", 0, 0, "Players can't get counters.") + .as_enchantment(); + let serpent_society = scenario + .add_creature_from_oracle(P0, "The Serpent Society", 3, 4, SERPENT_SOCIETY) + .id(); + let destroy = scenario + .add_spell_to_hand_from_oracle(P1, "Destroy Spell", true, "Destroy target creature.") + .id(); + let mut runner = scenario.build(); + { + let state = runner.state_mut(); + state.active_player = P1; + state.priority_player = P1; + state.waiting_for = WaitingFor::Priority { player: P1 }; + } + + runner + .cast(destroy) + .target_objects(&[serpent_society]) + .commit(); + runner.advance_until_stack_empty(); + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("attempting to pay Ward's poison-counter cost must be a legal action even when Solemnity prevents the actual counter gain"); + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().players[P1.0 as usize].poison_counters, + 0, + "Solemnity must prevent the poison counters from actually being given" + ); + assert!( + runner + .state() + .objects + .get(&serpent_society) + .is_some_and(|obj| obj.zone == engine::types::zones::Zone::Battlefield), + "a prevented player-counter payment must be treated as a FAILED cost, countering the targeting spell exactly like a declined payment — Serpent Society must survive" + ); + assert!( + !runner.state().stack.iter().any(|entry| entry.id == destroy), + "the countered spell must be removed from the stack" + ); +} diff --git a/crates/phase-ai/src/policies/strategy_helpers.rs b/crates/phase-ai/src/policies/strategy_helpers.rs index e899c2b16e..2b3140ad0e 100644 --- a/crates/phase-ai/src/policies/strategy_helpers.rs +++ b/crates/phase-ai/src/policies/strategy_helpers.rs @@ -733,21 +733,58 @@ pub(crate) fn available_mana_after_spell(ctx: &PolicyContext<'_>) -> u32 { sources.saturating_sub(spell_cost) } -/// CR 104.3d: total poison counters `ward` would give the payer, summed -/// across every `GetPlayerCounters { Poison, .. }` sub-cost in the whole -/// tree — a `Compound` cost's sub-costs are all paid together (CR 702.21a: -/// "every conjoined sub-cost must be payable"), not independently, so two -/// individually-nonlethal poison sub-costs can be jointly lethal and must be -/// checked against their COMBINED total, not each against the same +/// CR 104.3d: total poison counters `ward` would ACTUALLY give the payer, +/// summed across every `GetPlayerCounters { Poison, .. }` sub-cost in the +/// whole tree — a `Compound` cost's sub-costs are all paid together (CR +/// 702.21a: "every conjoined sub-cost must be payable"), not independently, +/// so two individually-nonlethal poison sub-costs can be jointly lethal and +/// must be checked against their COMBINED total, not each against the same /// unchanged starting count. -fn total_poison_from_ward_cost(ward: &WardCost) -> u32 { +/// +/// Projects each sub-cost through `preview_player_counter_addition` — the +/// real replacement pipeline, side-effect-free — rather than trusting the +/// printed count: a doubler or +N effect on the payer can make the printed +/// count understate what actually happens, letting a lethal payment look +/// safe. Returns `None` when any sub-cost's replacement outcome can't be +/// cleanly projected (`ChoiceRequired`/`Unsupported`) — callers must treat +/// `None` as "can't prove this is safe", never as zero poison. +fn total_poison_from_ward_cost(ctx: &PolicyContext<'_>, ward: &WardCost) -> Option { match ward { WardCost::GetPlayerCounters { - counter_kind: engine::types::player::PlayerCounterKind::Poison, + counter_kind: kind @ engine::types::player::PlayerCounterKind::Poison, count, - } => *count, - WardCost::Compound(costs) => costs.iter().map(total_poison_from_ward_cost).sum(), - _ => 0, + } => { + match engine::game::effects::player_counter::preview_player_counter_addition( + ctx.state, + ctx.ai_player, + ctx.ai_player, + *kind, + *count, + ) { + engine::game::effects::player_counter::PlayerCounterAdditionPreview::Applied { + count, + } + | engine::game::effects::player_counter::PlayerCounterAdditionPreview::Transformed { + count, + } => Some(count), + // A "players can't get counters" replacement (Solemnity) means + // this sub-cost actually gives zero poison — genuinely safe, + // not merely unproven. + engine::game::effects::player_counter::PlayerCounterAdditionPreview::Prevented => { + Some(0) + } + engine::game::effects::player_counter::PlayerCounterAdditionPreview::ChoiceRequired { + .. + } + | engine::game::effects::player_counter::PlayerCounterAdditionPreview::Unsupported => { + None + } + } + } + WardCost::Compound(costs) => costs.iter().try_fold(0u32, |total, cost| { + Some(total.saturating_add(total_poison_from_ward_cost(ctx, cost)?)) + }), + _ => Some(0), } } @@ -762,17 +799,23 @@ pub(crate) fn can_pay_ward_cost( warded: &GameObject, ) -> bool { // CR 104.3d: reject up front if the AGGREGATE poison this cost would - // give (direct or across every Compound sub-cost) reaches or crosses - // `LETHAL_POISON` — checked once, against the combined total, before any - // per-variant mechanical-affordability logic below. The AI must never - // treat ending its own game as an ordinary payable cost, for direct or - // compound Ward alike. - let total_poison = total_poison_from_ward_cost(ward); - if total_poison > 0 { - let current = ctx.state.players[ctx.ai_player.0 as usize].poison_counters; - if current.saturating_add(total_poison) >= crate::features::poison::LETHAL_POISON { - return false; + // ACTUALLY give (direct or across every Compound sub-cost, replacement- + // adjusted) reaches or crosses `LETHAL_POISON` — checked once, against + // the combined total, before any per-variant mechanical-affordability + // logic below. `None` means the replacement outcome couldn't be cleanly + // projected (a live choice or an unmodeled event rewrite) — conservatively + // decline rather than assume it's safe. The AI must never treat ending + // its own game as an ordinary payable cost, for direct or compound Ward + // alike. + match total_poison_from_ward_cost(ctx, ward) { + None => return false, + Some(total_poison) if total_poison > 0 => { + let current = ctx.state.players[ctx.ai_player.0 as usize].poison_counters; + if current.saturating_add(total_poison) >= crate::features::poison::LETHAL_POISON { + return false; + } } + Some(_) => {} } match ward { WardCost::Mana(cost) | WardCost::Waterbend(cost) => { diff --git a/crates/phase-ai/src/tactical_gate.rs b/crates/phase-ai/src/tactical_gate.rs index 8d9634c13c..4a8737ee40 100644 --- a/crates/phase-ai/src/tactical_gate.rs +++ b/crates/phase-ai/src/tactical_gate.rs @@ -706,6 +706,9 @@ mod tests { use engine::game::combat::{AttackerInfo, CombatState}; use engine::game::scenario::{GameScenario, P0, P1}; use engine::types::ability::{BounceSelection, EffectKind, ResolvedAbility, TargetFilter}; + use engine::types::ability::{ + QuantityModification, ReplacementDefinition, ReplacementPlayerScope, + }; use engine::types::game_state::{ PendingCast, StackEntry, StackEntryKind, TargetEffectDetail, TargetSelectionProgress, TargetSelectionSlot, WaitingFor, @@ -713,6 +716,7 @@ mod tests { use engine::types::identifiers::CardId; use engine::types::keywords::WardCost; use engine::types::mana::ManaCost; + use engine::types::replacements::ReplacementEvent; #[test] fn rejects_pump_after_combat_without_live_threat() { @@ -1296,4 +1300,55 @@ mod tests { }; assert_eq!(assess_candidate(&ctx), GateDecision::Reject); } + + /// CR 104.3d + CR 614.1a: a doubler on the poison counters the AI itself + /// would receive can make an individually-nonlethal PRINTED count + /// actually lethal once replacement-adjusted. The AI must project the + /// real, replacement-adjusted result (`preview_player_counter_addition`) + /// rather than trusting the printed count — the naive printed-count math + /// (4 existing + 3 printed = 7) would wrongly call this safe, but the + /// doubled result (4 + 6 = 10) is lethal. + #[test] + fn rejects_targeting_ward_with_lethal_poison_after_doubling_replacement() { + let mut scenario = GameScenario::new(); + // A permanent the AI (P0) controls that doubles poison counters P0 + // would receive. `valid_player: Some(You)` + the default recipient + // scope means this applies whenever P0 is the one gaining counters, + // mirroring how `player_counter.rs`'s own Solemnity test constructs a + // global player-counter replacement, parameterized to double instead + // of prevent. + let doubler_id = scenario.add_creature(P0, "Poison Doubler", 0, 0).id(); + let mut doubler_def = ReplacementDefinition::new(ReplacementEvent::AddCounter) + .quantity_modification(QuantityModification::DOUBLE); + doubler_def.valid_player = Some(ReplacementPlayerScope::You); + let creature = scenario + .add_creature(P1, "Warded", 2, 2) + .with_keyword(Keyword::Ward(WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count: 3, + })) + .id(); + let mut runner = scenario.build(); + let state = runner.state_mut(); + state + .objects + .get_mut(&doubler_id) + .unwrap() + .replacement_definitions = vec![doubler_def].into(); + state.players[P0.0 as usize].poison_counters = 4; + let decision = damage_target_decision(creature, 3); + let candidate = choose_target_candidate(creature); + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let ctx = PolicyContext { + state, + decision: &decision, + candidate: &candidate, + ai_player: P0, + config: &config, + context: &AiContext::empty(&config.weights), + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_eq!(assess_candidate(&ctx), GateDecision::Reject); + } } From eac7caf9c55a47466ed130d86dc53931d4ffd6e2 Mon Sep 17 00:00:00 2001 From: hurryup52 Date: Mon, 27 Jul 2026 23:04:23 +0200 Subject: [PATCH 09/14] fix(ai): reject player-counter Ward payments the engine would actually fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit can_pay_ward_cost's GetPlayerCounters arm returned true unconditionally, even when the specific counter addition would be prevented (Solemnity), need a live choice, or hit an unmodeled replacement rewrite. The payment path (AbilityCost::GetPlayerCounters in costs.rs) already treats a Prevented outcome as a failed payment, not a zero-cost one, so the AI could target into Solemnity, believe the Ward was safely payable, and have its spell countered when payment actually failed. Preview the specific counter addition and only report payable for Applied/Transformed outcomes. Compound costs reject recursively through the existing .all(|cost| can_pay_ward_cost(...)) — no separate recursion needed. total_poison_from_ward_cost's Prevented => Some(0) arm is unchanged (still correct for poison-quantity projection), but its comment is reworded to stop implying the whole cost is safe merely because it gives zero poison; payability is now checked separately. Adds two tactical-gate regressions: a direct prevented GetPlayerCounters Ward, and a Compound Ward with one payable ordinary leaf plus one prevented GetPlayerCounters leaf, proving the recursive rejection. --- .../phase-ai/src/policies/strategy_helpers.rs | 34 +++++-- crates/phase-ai/src/tactical_gate.rs | 88 +++++++++++++++++++ 2 files changed, 114 insertions(+), 8 deletions(-) diff --git a/crates/phase-ai/src/policies/strategy_helpers.rs b/crates/phase-ai/src/policies/strategy_helpers.rs index 2b3140ad0e..765dbe0413 100644 --- a/crates/phase-ai/src/policies/strategy_helpers.rs +++ b/crates/phase-ai/src/policies/strategy_helpers.rs @@ -767,9 +767,10 @@ fn total_poison_from_ward_cost(ctx: &PolicyContext<'_>, ward: &WardCost) -> Opti | engine::game::effects::player_counter::PlayerCounterAdditionPreview::Transformed { count, } => Some(count), - // A "players can't get counters" replacement (Solemnity) means - // this sub-cost actually gives zero poison — genuinely safe, - // not merely unproven. + // For poison-quantity projection only, prevention contributes zero counters. + // This does not mean the Ward cost is payable: `can_pay_ward_cost` separately + // rejects Prevented, ChoiceRequired, and Unsupported previews because the + // engine cannot successfully complete or safely project that payment. engine::game::effects::player_counter::PlayerCounterAdditionPreview::Prevented => { Some(0) } @@ -854,11 +855,28 @@ pub(crate) fn can_pay_ward_cost( .count(); matching as u32 >= *count } - // CR 702.21a + CR 122.1: mechanically always payable — no resource - // limit on giving yourself more counters (mirrors the engine's own - // `can_pay_resolution`). Lethal poison is already rejected by the - // aggregate check above, for direct and compound costs alike. - WardCost::GetPlayerCounters { .. } => true, + // CR 702.21a + CR 122.1 + CR 104.3d: mechanically payable in the ordinary + // case — no resource limit on giving yourself more counters — UNLESS a + // replacement effect actually prevents the addition (Solemnity) or its + // outcome can't be cleanly projected (a live choice or an unmodeled event + // rewrite). `costs.rs`'s `AbilityCost::GetPlayerCounters` payment path + // treats `Prevented` as a genuinely FAILED payment, not a paused or + // zero-cost one — so the AI must decline here too, or it will target into + // Solemnity, believe the Ward is safely payable, and have its spell + // countered when payment actually fails. Lethal (but payable) poison is + // already rejected by the aggregate check above, for direct and compound + // costs alike. + WardCost::GetPlayerCounters { counter_kind, count } => matches!( + engine::game::effects::player_counter::preview_player_counter_addition( + ctx.state, + ctx.ai_player, + ctx.ai_player, + *counter_kind, + *count, + ), + engine::game::effects::player_counter::PlayerCounterAdditionPreview::Applied { .. } + | engine::game::effects::player_counter::PlayerCounterAdditionPreview::Transformed { .. } + ), // CR 702.21a: every conjoined sub-cost must be payable. Mana contention // between multiple mana sub-costs is approximated (each checked against // the full post-spell pool) — rare enough not to warrant exact tracking. diff --git a/crates/phase-ai/src/tactical_gate.rs b/crates/phase-ai/src/tactical_gate.rs index 4a8737ee40..52e4aba99b 100644 --- a/crates/phase-ai/src/tactical_gate.rs +++ b/crates/phase-ai/src/tactical_gate.rs @@ -1351,4 +1351,92 @@ mod tests { }; assert_eq!(assess_candidate(&ctx), GateDecision::Reject); } + + /// CR 702.21a: a "players can't get counters" replacement (Solemnity) means + /// the AI's Ward payment will actually FAIL — `costs.rs`'s + /// `AbilityCost::GetPlayerCounters` treats `Prevented` as a failed payment, + /// not a zero-cost one — so the AI must not target into this believing the + /// Ward is safely (and freely) payable. + #[test] + fn rejects_targeting_ward_with_prevented_player_counter_payment() { + let mut scenario = GameScenario::new(); + let solemnity_id = scenario.add_creature(P0, "Solemnity", 0, 0).id(); + let mut prevent_def = ReplacementDefinition::new(ReplacementEvent::AddCounter) + .quantity_modification(QuantityModification::Prevent); + prevent_def.valid_player = Some(ReplacementPlayerScope::AnyPlayer); + let creature = scenario + .add_creature(P1, "Warded", 2, 2) + .with_keyword(Keyword::Ward(WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count: 3, + })) + .id(); + let mut runner = scenario.build(); + let state = runner.state_mut(); + state + .objects + .get_mut(&solemnity_id) + .unwrap() + .replacement_definitions = vec![prevent_def].into(); + let decision = damage_target_decision(creature, 3); + let candidate = choose_target_candidate(creature); + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let ctx = PolicyContext { + state, + decision: &decision, + candidate: &candidate, + ai_player: P0, + config: &config, + context: &AiContext::empty(&config.weights), + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_eq!(assess_candidate(&ctx), GateDecision::Reject); + } + + /// CR 702.21a: a `Compound` Ward's sub-costs are conjoined — ALL must be + /// payable, so a prevented `GetPlayerCounters` sub-cost must reject the + /// whole cost even when its sibling sub-cost (here, a small life payment) + /// is perfectly payable on its own. Proves the recursion through + /// `Compound`'s `.all(|cost| can_pay_ward_cost(...))`, not just the direct + /// leaf case covered by `rejects_targeting_ward_with_prevented_player_counter_payment`. + #[test] + fn rejects_compound_ward_with_prevented_player_counter_leaf() { + let mut scenario = GameScenario::new(); + let solemnity_id = scenario.add_creature(P0, "Solemnity", 0, 0).id(); + let mut prevent_def = ReplacementDefinition::new(ReplacementEvent::AddCounter) + .quantity_modification(QuantityModification::Prevent); + prevent_def.valid_player = Some(ReplacementPlayerScope::AnyPlayer); + let creature = scenario + .add_creature(P1, "Warded", 2, 2) + .with_keyword(Keyword::Ward(WardCost::Compound(vec![ + WardCost::PayLife(2), // trivially payable on its own (P0 starts at 20 life) + WardCost::GetPlayerCounters { + counter_kind: engine::types::player::PlayerCounterKind::Poison, + count: 3, + }, + ]))) + .id(); + let mut runner = scenario.build(); + let state = runner.state_mut(); + state + .objects + .get_mut(&solemnity_id) + .unwrap() + .replacement_definitions = vec![prevent_def].into(); + let decision = damage_target_decision(creature, 3); + let candidate = choose_target_candidate(creature); + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let ctx = PolicyContext { + state, + decision: &decision, + candidate: &candidate, + ai_player: P0, + config: &config, + context: &AiContext::empty(&config.weights), + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_eq!(assess_candidate(&ctx), GateDecision::Reject); + } } From 1db69a775cf6ad39c5e6c1bece150a50b6fea064 Mon Sep 17 00:00:00 2001 From: hurryup52 Date: Tue, 28 Jul 2026 03:19:21 +0200 Subject: [PATCH 10/14] fix(engine): preserve the Ward unless-payment continuation across a player-counter replacement choice add_player_counter_with_replacement's NeedsChoice arm only overwrote state.waiting_for with the ReplacementChoice prompt, never preserving a payment root. handle_unless_payment's GetPlayerCounters arm returned immediately on PaymentOutcome::Paused, discarding pending_effect, trigger_event, effect_description, and remaining as local stack variables. Once the player answered the replacement choice, handle_replacement_choice applied (or failed to apply) the counters and reset straight to WaitingFor::Priority, leaving a Ward with an optional/orderable AddCounter replacement neither settled as paid nor countered. Extracts handle_unless_payment's shared paid/failed epilogue into finish_unless_payment, adds PendingCostMoveResume::GetPlayerCountersUnlessPayment to carry the full unless-payment continuation across the replacement choice, and wires resume_get_player_counters_unless_payment into the drain_pending_cost_move_resume dispatcher at both the ReplacementDelivered and ReplacementPrevented boundaries (mirroring PlayerCounterAdditionOutcome's established Applied/Prevented <-> Paid/Failed mapping for the immediate- payment case). Adds two end-to-end regressions in serpent_society_ward_poison_cost.rs covering a synthetic optional player-counter-prevention replacement, accepted (countered) and declined (paid) branches. --- crates/engine/src/game/engine.rs | 15 +- .../engine/src/game/engine_payment_choices.rs | 99 +++++++++- crates/engine/src/types/game_state.rs | 22 +++ .../serpent_society_ward_poison_cost.rs | 184 ++++++++++++++++++ 4 files changed, 317 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index bbcfd9ef4e..d0c7ecfa51 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -3173,12 +3173,15 @@ pub(crate) fn drain_pending_cost_move_resume( | PendingCostMoveResume::ManaAbilityPayment { .. } | PendingCostMoveResume::ActivationMillPayment { .. } | PendingCostMoveResume::LoyaltyActivation { .. } + | PendingCostMoveResume::GetPlayerCountersUnlessPayment { .. } ) ), // CR 606.4 + CR 616.1: a fully-prevented loyalty counter add (e.g. an // opponent's Solemnity would prevent the counters) must still complete the // parked activation instead of wedging, so `LoyaltyActivation` is eligible - // at the Prevented boundary as well. + // at the Prevented boundary as well. `GetPlayerCountersUnlessPayment` is + // eligible here too: a prevented Ward player-counter payment is a FAILED + // cost (CR 702.21a) that must counter the guarded ability, not wedge. CostMoveDrainBoundary::ReplacementPrevented { .. } => matches!( state.pending_cost_move_resume, Some( @@ -3193,6 +3196,7 @@ pub(crate) fn drain_pending_cost_move_resume( | PendingCostMoveResume::ManaAbilityPayment { .. } | PendingCostMoveResume::ActivationMillPayment { .. } | PendingCostMoveResume::LoyaltyActivation { .. } + | PendingCostMoveResume::GetPlayerCountersUnlessPayment { .. } ) ), CostMoveDrainBoundary::PriorityBoundary => matches!( @@ -3264,6 +3268,15 @@ pub(crate) fn drain_pending_cost_move_resume( Some(PendingCostMoveResume::LoyaltyActivation { .. }) ) { super::planeswalker::resume_loyalty_activation(state, events)? + } else if matches!( + state.pending_cost_move_resume, + Some(PendingCostMoveResume::GetPlayerCountersUnlessPayment { .. }) + ) { + engine_payment_choices::resume_get_player_counters_unless_payment( + state, + events, + matches!(boundary, CostMoveDrainBoundary::ReplacementDelivered { .. }), + )? } 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 88a6520dfd..52bf4936e2 100644 --- a/crates/engine/src/game/engine_payment_choices.rs +++ b/crates/engine/src/game/engine_payment_choices.rs @@ -735,8 +735,9 @@ pub(super) fn handle_unless_payment( // affordability gate exists — route through the single payment // authority exactly like PayLife/PayEnergy above. Unlike those // two, a real `Paused` path exists here (a "can't get counters" - // replacement effect may need a live choice), so it is preserved - // rather than lumped with `Failed`. + // replacement effect, or a CR 616.1 replacement-ordering choice, + // may need a live choice), so it is preserved rather than lumped + // with `Failed`. AbilityCost::GetPlayerCounters { .. } => { match costs::pay_ability_cost_for_resolution( state, @@ -749,7 +750,25 @@ pub(super) fn handle_unless_payment( PaymentOutcome::Failed { .. } => { payment_failed = true; } + // CR 702.21a + CR 122.1 + CR 616.1: the counter-placement + // replacement needs the player's choice. Stash the FULL + // unless-payment continuation here — nothing else records + // `pending_effect`/`trigger_event`/`effect_description`/ + // `remaining` once `add_player_counter_with_replacement` + // overwrites `state.waiting_for` with the ReplacementChoice + // prompt below — so the choice's resolution can settle + // this exact Ward payment (via `finish_unless_payment`, + // resumed by `resume_get_player_counters_unless_payment`) + // instead of leaving it orphaned at bare Priority. PaymentOutcome::Paused { .. } => { + state.pending_cost_move_resume = + Some(PendingCostMoveResume::GetPlayerCountersUnlessPayment { + cost: poll_cost.clone(), + pending_effect: pending_effect.clone(), + trigger_event: trigger_event.clone(), + effect_description: effect_description.clone(), + remaining: remaining.clone(), + }); return Ok(action_result(events, state.waiting_for.clone())); } } @@ -1228,6 +1247,42 @@ pub(super) fn handle_unless_payment( } } + finish_unless_payment( + state, + pay, + payment_failed, + poll_cost, + pending_effect, + trigger_event, + effect_description, + remaining, + post_action_event_start, + events, + ) +} + +/// CR 118.12 + CR 118.12a: The shared paid/failed epilogue for every +/// unless-cost shape — poll re-emit for "unless any player pays", resolve the +/// guarded ability's chain when the cost is unpaid/failed, and settle +/// priority/continuations either way. Extracted from `handle_unless_payment`'s +/// own tail so a cost shape that pauses on a nested replacement choice mid-payment +/// (`AbilityCost::GetPlayerCounters`, via +/// `PendingCostMoveResume::GetPlayerCountersUnlessPayment`) can resume through +/// EXACTLY this same logic once the choice resolves, instead of duplicating it +/// and risking drift between the immediate and deferred paths. +#[allow(clippy::too_many_arguments)] +fn finish_unless_payment( + state: &mut GameState, + pay: bool, + payment_failed: bool, + poll_cost: AbilityCost, + pending_effect: Box, + trigger_event: Option, + effect_description: Option, + remaining: Vec, + mut post_action_event_start: Option, + events: &mut Vec, +) -> Result { if !pay || payment_failed { // CR 118.12a: "[Effect] unless any player pays ..." poll — when the // current player declines (or cannot pay) and more players remain, @@ -1780,6 +1835,46 @@ pub(super) fn resume_ward_sacrifice_payment( } } +/// CR 702.21a + CR 122.1 + CR 616.1: Resume a Ward player-counter unless-payment +/// after its `AddCounter` replacement choice settled. `payment_succeeded` comes +/// from the exact boundary the replacement pipeline resolved to +/// (`CostMoveDrainBoundary::ReplacementDelivered` = the counters were actually +/// added = paid; `ReplacementPrevented` = a replacement fully suppressed the +/// placement = failed — mirroring `PlayerCounterAdditionOutcome`'s established +/// Applied/Prevented ↔ Paid/Failed mapping for the immediate-payment case). +/// Delegates to the same `finish_unless_payment` tail every other unless-cost +/// shape uses, so the Ward-guarded ability is settled exactly once, either way, +/// instead of the game resetting to bare priority with its fate undetermined. +pub(super) fn resume_get_player_counters_unless_payment( + state: &mut GameState, + events: &mut Vec, + payment_succeeded: bool, +) -> Result { + let Some(PendingCostMoveResume::GetPlayerCountersUnlessPayment { + cost, + pending_effect, + trigger_event, + effect_description, + remaining, + }) = state.pending_cost_move_resume.take() + else { + unreachable!("GetPlayerCounters unless-payment resume requires its typed continuation") + }; + 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/types/game_state.rs b/crates/engine/src/types/game_state.rs index e562d0b16e..a7d94578dd 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -5952,6 +5952,28 @@ pub enum PendingCostMoveResume { resolved: Box, ability_index: usize, }, + /// CR 702.21a + CR 122.1 + CR 616.1: Ward's player-counter unless-cost + /// (`AbilityCost::GetPlayerCounters`) paused on a replacement choice for + /// the `AddCounter` event it attempted (e.g. an optional "you may + /// prevent a player from getting counters" replacement, or a CR 616.1 + /// ordering choice among several applicable replacements). Retains the + /// full `WaitingFor::UnlessPayment` payload so the choice's resolution — + /// Applied (paid) via the `ReplacementDelivered` boundary, or Prevented + /// (failed) via the `ReplacementPrevented` boundary — can drive the same + /// paid/failed tail `handle_unless_payment` uses for every other cost + /// shape, instead of resetting to bare priority with the Ward-guarded + /// ability's fate undetermined. + GetPlayerCountersUnlessPayment { + #[serde(deserialize_with = "crate::types::ability::deserialize_ability_cost_compat")] + cost: AbilityCost, + pending_effect: Box, + #[serde(default, skip_serializing_if = "Option::is_none")] + trigger_event: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + effect_description: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + remaining: Vec, + }, } /// CR 601.2h + CR 616.1: Resume paying a sequential cost after a replacement diff --git a/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs b/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs index 6a0c822f5d..fd4f11e83f 100644 --- a/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs +++ b/crates/engine/tests/integration/serpent_society_ward_poison_cost.rs @@ -14,10 +14,18 @@ //! exercised by this test). use engine::game::scenario::{GameScenario, P0, P1}; +use engine::game::zones::create_object; +use engine::types::ability::{ + QuantityModification, ReplacementDefinition, ReplacementMode, ReplacementPlayerScope, +}; use engine::types::actions::GameAction; use engine::types::game_state::WaitingFor; +use engine::types::identifiers::CardId; use engine::types::phase::Phase; use engine::types::player::PlayerCounterKind; +use engine::types::replacements::ReplacementEvent; +use engine::types::zones::Zone; +use std::sync::Arc; const SERPENT_SOCIETY: &str = "Deathtouch\n\ Ward—Get five poison counters. (A player with ten or more poison counters loses the game.)\n\ @@ -271,3 +279,179 @@ fn serpent_society_ward_payment_prevented_by_solemnity_counters_the_spell() { "the countered spell must be removed from the stack" ); } + +/// Installs a synthetic OPTIONAL "you may prevent a player from getting +/// counters" replacement on a fresh P0 permanent. No real card has exactly +/// this wording, so — mirroring this file's own Solemnity test (which uses a +/// real, if partial, MANDATORY prevention) and the engine's established +/// pattern for exercising an optional replacement choice with no real-card +/// precedent — the definition is installed directly, after `scenario.build()`, +/// so the real Ward -> `GetPlayerCounters` -> `add_player_counter_with_ +/// replacement` -> `replace_event` path discovers it naturally (a production +/// setup, not a hand-constructed `WaitingFor`). +fn install_optional_player_counter_prevention(state: &mut engine::types::game_state::GameState) { + let source = create_object( + state, + CardId(9101), + P0, + "Optional Poison Warden".to_string(), + Zone::Battlefield, + ); + let mut def = ReplacementDefinition::new(ReplacementEvent::AddCounter); + def.mode = ReplacementMode::Optional { decline: None }; + def.quantity_modification = Some(QuantityModification::Prevent); + def.valid_player = Some(ReplacementPlayerScope::AnyPlayer); + let reps = vec![def]; + let obj = state.objects.get_mut(&source).unwrap(); + obj.replacement_definitions = reps.clone().into(); + obj.base_replacement_definitions = Arc::new(reps); +} + +/// Regression for reviewer matthewevans's finding on PR #6662: a Ward +/// player-counter cost whose `AddCounter` event needs a CR 616.1 replacement +/// choice (as opposed to Solemnity's unconditional, mandatory prevention +/// above) must not orphan the unless-payment continuation. Before this fix, +/// `add_player_counter_with_replacement`'s `NeedsChoice` arm replaced +/// `waiting_for` with the bare `ReplacementChoice` prompt and nothing +/// preserved `pending_effect`/`trigger_event` — once the player answered the +/// prompt, `handle_replacement_choice` applied (or failed to apply) the +/// counters and reset straight to `WaitingFor::Priority`, leaving Ward's +/// guarded "counter the spell" outcome permanently undetermined: the +/// targeting spell was neither countered nor allowed to resolve. +/// +/// Accept branch: the optional replacement prevents the counter placement +/// (`PlayerCounterAdditionOutcome::Prevented`) — a FAILED Ward payment, so the +/// targeting spell must be countered, exactly like the Solemnity test above. +#[test] +fn serpent_society_ward_optional_counter_prevention_accepted_counters_the_spell() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let serpent_society = scenario + .add_creature_from_oracle(P0, "The Serpent Society", 3, 4, SERPENT_SOCIETY) + .id(); + let destroy = scenario + .add_spell_to_hand_from_oracle(P1, "Destroy Spell", true, "Destroy target creature.") + .id(); + let mut runner = scenario.build(); + { + let state = runner.state_mut(); + state.active_player = P1; + state.priority_player = P1; + state.waiting_for = WaitingFor::Priority { player: P1 }; + install_optional_player_counter_prevention(state); + } + + runner + .cast(destroy) + .target_objects(&[serpent_society]) + .commit(); + runner.advance_until_stack_empty(); + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("attempting to pay Ward's poison-counter cost must be legal even when an optional replacement can prevent it"); + + // Reaching a REPLACEMENT CHOICE (not an orphaned bare Priority) is the + // regression's core assertion. + let WaitingFor::ReplacementChoice { + player, + candidate_count, + .. + } = runner.state().waiting_for + else { + panic!( + "optional player-counter prevention must surface a real replacement choice, got {:?}", + runner.state().waiting_for + ); + }; + assert_eq!( + player, P1, + "the payer (Ward's targeting opponent) makes the replacement choice" + ); + assert_eq!( + candidate_count, 2, + "an Optional replacement offers accept (0) and decline (1)" + ); + + runner + .act(GameAction::ChooseReplacement { index: 0 }) + .expect("accepting the optional prevention must be a legal replacement choice"); + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().players[P1.0 as usize].poison_counters, + 0, + "the accepted prevention must stop the poison counters from being given" + ); + assert!( + runner + .state() + .objects + .get(&serpent_society) + .is_some_and(|obj| obj.zone == Zone::Battlefield), + "a prevented player-counter payment must be treated as a FAILED cost, countering the targeting spell — Serpent Society must survive" + ); + assert!( + !runner.state().stack.iter().any(|entry| entry.id == destroy), + "the countered spell must be removed from the stack, not left stranded" + ); +} + +/// Decline branch: the optional replacement does not apply, so the original +/// `AddCounter` proceeds unmodified (`PlayerCounterAdditionOutcome::Applied`) +/// — a PAID Ward payment, so the targeting spell must resolve normally. +#[test] +fn serpent_society_ward_optional_counter_prevention_declined_pays_the_cost_and_resolves_the_spell() +{ + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let serpent_society = scenario + .add_creature_from_oracle(P0, "The Serpent Society", 3, 4, SERPENT_SOCIETY) + .id(); + let destroy = scenario + .add_spell_to_hand_from_oracle(P1, "Destroy Spell", true, "Destroy target creature.") + .id(); + let mut runner = scenario.build(); + { + let state = runner.state_mut(); + state.active_player = P1; + state.priority_player = P1; + state.waiting_for = WaitingFor::Priority { player: P1 }; + install_optional_player_counter_prevention(state); + } + + runner + .cast(destroy) + .target_objects(&[serpent_society]) + .commit(); + runner.advance_until_stack_empty(); + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("attempting to pay must be legal"); + let WaitingFor::ReplacementChoice { .. } = runner.state().waiting_for else { + panic!( + "expected a replacement choice, got {:?}", + runner.state().waiting_for + ); + }; + + runner + .act(GameAction::ChooseReplacement { index: 1 }) + .expect("declining the optional prevention must be a legal replacement choice"); + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().players[P1.0 as usize].poison_counters, + 5, + "declining the optional prevention must let Ward's cost actually give five poison counters" + ); + assert!( + runner + .state() + .objects + .get(&serpent_society) + .is_none_or(|obj| obj.zone != Zone::Battlefield), + "a successfully paid Ward cost must let the targeted destroy spell resolve" + ); +} From e4b646d956549429d7ea1e35175352eaf071488a Mon Sep 17 00:00:00 2001 From: hurryup52 Date: Fri, 31 Jul 2026 19:23:11 +0200 Subject: [PATCH 11/14] fix(engine,ai): adapt to upstream/main's payment_continuation refactor Rebase-adaptation fixup, no new behavior beyond matching main's shape: - crates/engine/src/ai_support/payment_continuation.rs: main split the parked-cost-move classification logic out of engine_payment_choices.rs into this new module since this branch was last rebased. Add the GetPlayerCountersUnlessPayment arm to both match statements, joining the same NotAffiliated/false group as its sibling unless-payment continuations (WardSacrificePayment, ReplacementMayCost, UnlessBouncePayment, etc.) -- it's the same class of continuation, just for a player-counter Ward cost. - crates/engine/src/game/engine_payment_choices.rs: drop a now-unused `mut` (pre-existing on main, unrelated to this PR, surfaced by clippy -D warnings once the crate compiled again). - crates/phase-ai/src/policies/anti_self_harm.rs: add the effect_kind/ effect_detail fields main's TargetSelectionSlot gained to the one literal in this PR's own test suite that predates them, matching every other TargetSelectionSlot construction already in this file. --- crates/engine/src/ai_support/payment_continuation.rs | 2 ++ crates/engine/src/game/engine_payment_choices.rs | 2 +- crates/phase-ai/src/policies/anti_self_harm.rs | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/ai_support/payment_continuation.rs b/crates/engine/src/ai_support/payment_continuation.rs index 60cb96d141..c94c5a1201 100644 --- a/crates/engine/src/ai_support/payment_continuation.rs +++ b/crates/engine/src/ai_support/payment_continuation.rs @@ -413,6 +413,7 @@ fn classify_parked_cost_move_root(state: &GameState) -> PaymentContinuationState | PendingCostMoveResume::ReplacementMayCost { .. } | PendingCostMoveResume::Foretell { .. } | PendingCostMoveResume::UnlessBouncePayment { .. } + | PendingCostMoveResume::GetPlayerCountersUnlessPayment { .. } | PendingCostMoveResume::LoyaltyActivation { .. } => { PaymentContinuationState::NotAffiliated } @@ -657,6 +658,7 @@ fn pending_cost_move_contains_root( | Some(PendingCostMoveResume::Foretell { .. }) | Some(PendingCostMoveResume::DelveManaPayment { .. }) | Some(PendingCostMoveResume::UnlessBouncePayment { .. }) + | Some(PendingCostMoveResume::GetPlayerCountersUnlessPayment { .. }) | Some(PendingCostMoveResume::LoyaltyActivation { .. }) | None => false, } diff --git a/crates/engine/src/game/engine_payment_choices.rs b/crates/engine/src/game/engine_payment_choices.rs index 52bf4936e2..9f474127ae 100644 --- a/crates/engine/src/game/engine_payment_choices.rs +++ b/crates/engine/src/game/engine_payment_choices.rs @@ -636,7 +636,7 @@ pub(super) fn handle_unless_payment( let poll_cost = cost.clone(); let mut payment_failed = !pay; - let mut post_action_event_start = None; + let post_action_event_start = None; if pay { match cost { // CR 118.12: Pay the static mana component of the unless cost diff --git a/crates/phase-ai/src/policies/anti_self_harm.rs b/crates/phase-ai/src/policies/anti_self_harm.rs index 96a08f932b..c90c6657c9 100644 --- a/crates/phase-ai/src/policies/anti_self_harm.rs +++ b/crates/phase-ai/src/policies/anti_self_harm.rs @@ -3697,6 +3697,8 @@ mod tests { legal_targets: legal_targets.clone(), optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: Vec::new(), From 18b0f153ebff5f7321c883a1ea46bd06679c82ca Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 31 Jul 2026 17:06:48 -0700 Subject: [PATCH 12/14] chore(PR-6662): preserve current main tactical gate tests --- crates/phase-ai/src/tactical_gate.rs | 445 ++++++++++++++++++++++++++- 1 file changed, 444 insertions(+), 1 deletion(-) diff --git a/crates/phase-ai/src/tactical_gate.rs b/crates/phase-ai/src/tactical_gate.rs index 52e4aba99b..0d6d6aa918 100644 --- a/crates/phase-ai/src/tactical_gate.rs +++ b/crates/phase-ai/src/tactical_gate.rs @@ -5,9 +5,10 @@ use engine::game::combat::AttackTarget; use engine::types::ability::{AbilityCondition, Effect, PtValue, TargetFilter, TargetRef}; use engine::types::actions::GameAction; use engine::types::card_type::CoreType; -use engine::types::game_state::GameState; +use engine::types::game_state::{GameState, WaitingFor}; use engine::types::identifiers::ObjectId; use engine::types::keywords::Keyword; +use engine::types::mana::ManaType; use engine::types::phase::Phase; use engine::types::player::PlayerId; @@ -1439,4 +1440,446 @@ mod tests { }; assert_eq!(assess_candidate(&ctx), GateDecision::Reject); } + /// Build an Improvise `ManaPayment` decision context: a `TapForConvoke` + /// Colorless candidate for `object_id`, plus whatever sibling candidates + /// the caller supplies for that same dual-purpose permanent. + fn improvise_mana_payment_decision( + sibling_candidates: Vec, + object_id: ObjectId, + ) -> AiDecisionContext { + let mut candidates = vec![CandidateAction { + action: GameAction::TapForConvoke { + object_id, + mana_type: ManaType::Colorless, + }, + metadata: ActionMetadata::for_actor(Some(P0), TacticalClass::Mana), + }]; + candidates.extend(sibling_candidates); + AiDecisionContext { + waiting_for: WaitingFor::ManaPayment { + player: P0, + convoke_mode: Some(engine::types::game_state::ConvokeMode::Improvise), + }, + candidates, + } + } + + fn native_blue_tap_candidate(object_id: ObjectId) -> CandidateAction { + CandidateAction { + action: GameAction::TapLandForMana { + selection: engine::types::mana::ManaSourceSelection { + source: engine::types::identifiers::ObjectIncarnationRef { + object_id, + incarnation: 0, + }, + ability_index: None, + mana_type: ManaType::Blue, + output: engine::types::mana::ManaSourceOutput::Concrete(ManaType::Blue), + atomic_combination: None, + restrictions: Vec::new(), + penalty: engine::types::mana::ManaSourcePenalty::None, + taps_for_mana: Vec::new(), + }, + }, + metadata: ActionMetadata::for_actor(Some(P0), TacticalClass::Mana), + } + } + + fn convoke_candidate_ctx<'a>( + state: &'a GameState, + decision: &'a AiDecisionContext, + config: &'a crate::config::AiConfig, + context: &'a AiContext, + ) -> PolicyContext<'a> { + PolicyContext { + state, + decision, + candidate: &decision.candidates[0], + ai_player: P0, + config, + context, + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + } + } + + /// CR 702.51a + CR 702.126a: a dual-purpose permanent (an artifact land + /// producing {U} natively) must not be tapped for its Colorless Improvise + /// marker while the pending cast's {U} pip is still outstanding — that + /// would strand the pip and dead-end `ManaPayment` (the Metallic Rebuke + /// bug: crates/phase-ai/src/search.rs's `fallback_action` panic). + #[test] + fn rejects_convoke_colorless_tap_when_native_ability_still_covers_colored_demand() { + let mut state = GameState::new_two_player(42); + state.pending_cast = Some(Box::new(PendingCast::new( + ObjectId(900), + CardId(900), + ResolvedAbility::new( + Effect::Draw { + count: engine::types::ability::QuantityExpr::Fixed { value: 0 }, + target: TargetFilter::Controller, + }, + Vec::new(), + ObjectId(900), + P0, + ), + ManaCost::Cost { + shards: vec![engine::types::mana::ManaCostShard::Blue], + generic: 2, + }, + ))); + let object_id = ObjectId(901); + let decision = + improvise_mana_payment_decision(vec![native_blue_tap_candidate(object_id)], object_id); + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let context = AiContext::empty(&config.weights); + let ctx = convoke_candidate_ctx(&state, &decision, &config, &context); + assert_eq!(assess_candidate(&ctx), GateDecision::Reject); + } + + /// The sibling native-ability tap for the same permanent must stay + /// `Allow` — the fix removes only the redundant Colorless path, not the + /// source's usability. + #[test] + fn allows_native_tap_when_colorless_marker_is_gated() { + let mut state = GameState::new_two_player(42); + state.pending_cast = Some(Box::new(PendingCast::new( + ObjectId(900), + CardId(900), + ResolvedAbility::new( + Effect::Draw { + count: engine::types::ability::QuantityExpr::Fixed { value: 0 }, + target: TargetFilter::Controller, + }, + Vec::new(), + ObjectId(900), + P0, + ), + ManaCost::Cost { + shards: vec![engine::types::mana::ManaCostShard::Blue], + generic: 2, + }, + ))); + let object_id = ObjectId(901); + let decision = + improvise_mana_payment_decision(vec![native_blue_tap_candidate(object_id)], object_id); + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let context = AiContext::empty(&config.weights); + let ctx = PolicyContext { + state: &state, + decision: &decision, + candidate: &decision.candidates[1], + ai_player: P0, + config: &config, + context: &context, + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_eq!(assess_candidate(&ctx), GateDecision::Allow); + } + + /// Once colored demand is satisfied (a generic-only remaining cost), the + /// Colorless marker is fine again — this isn't a blanket ban on + /// Improvise/Convoke for dual-purpose permanents. + #[test] + fn allows_convoke_colorless_tap_once_colored_demand_is_satisfied() { + let mut state = GameState::new_two_player(42); + state.pending_cast = Some(Box::new(PendingCast::new( + ObjectId(900), + CardId(900), + ResolvedAbility::new( + Effect::Draw { + count: engine::types::ability::QuantityExpr::Fixed { value: 0 }, + target: TargetFilter::Controller, + }, + Vec::new(), + ObjectId(900), + P0, + ), + ManaCost::Cost { + shards: Vec::new(), + generic: 3, + }, + ))); + let object_id = ObjectId(901); + let decision = + improvise_mana_payment_decision(vec![native_blue_tap_candidate(object_id)], object_id); + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let context = AiContext::empty(&config.weights); + let ctx = convoke_candidate_ctx(&state, &decision, &config, &context); + assert_eq!(assess_candidate(&ctx), GateDecision::Allow); + } + + /// A permanent with no sibling native colored option (a plain + /// non-mana-producing artifact) is unaffected by the gate — it's scoped + /// to true tap-channel dominance, not all Colorless taps. + #[test] + fn allows_convoke_colorless_tap_on_permanent_with_no_native_colored_option() { + let mut state = GameState::new_two_player(42); + state.pending_cast = Some(Box::new(PendingCast::new( + ObjectId(900), + CardId(900), + ResolvedAbility::new( + Effect::Draw { + count: engine::types::ability::QuantityExpr::Fixed { value: 0 }, + target: TargetFilter::Controller, + }, + Vec::new(), + ObjectId(900), + P0, + ), + ManaCost::Cost { + shards: vec![engine::types::mana::ManaCostShard::Blue], + generic: 2, + }, + ))); + let object_id = ObjectId(901); + let decision = improvise_mana_payment_decision(Vec::new(), object_id); + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let context = AiContext::empty(&config.weights); + let ctx = convoke_candidate_ctx(&state, &decision, &config, &context); + assert_eq!(assess_candidate(&ctx), GateDecision::Allow); + } + + /// CR 702.51a: the production Convoke candidate path (`mana_payment_actions` + /// via `candidate_actions_broad`) offers a Colorless marker AND a matching + /// colored marker for the same creature when its color is in the cost. The + /// Improvise-only tests above build a synthetic native-mana sibling and + /// never exercise this real Convoke-generated pair -- confirmed missing by + /// review on #6840: `sibling_native_tap_pays_demand` didn't recognize a + /// colored `TapForConvoke` on the same object as a dominating sibling, so a + /// real Convoke spell with a colored pip could still dead-end. + #[test] + fn rejects_convoke_colorless_tap_when_real_convoke_colored_sibling_covers_demand() { + let mut scenario = GameScenario::new(); + let creature = scenario.add_creature(P0, "Convoke Creature", 2, 2).id(); + let mut runner = scenario.build(); + { + let state = runner.state_mut(); + state.objects.get_mut(&creature).unwrap().color = + vec![engine::types::mana::ManaColor::Blue]; + state.pending_cast = Some(Box::new(PendingCast::new( + ObjectId(900), + CardId(900), + ResolvedAbility::new( + Effect::Draw { + count: engine::types::ability::QuantityExpr::Fixed { value: 0 }, + target: TargetFilter::Controller, + }, + Vec::new(), + ObjectId(900), + P0, + ), + ManaCost::Cost { + shards: vec![engine::types::mana::ManaCostShard::Blue], + generic: 1, + }, + ))); + state.waiting_for = WaitingFor::ManaPayment { + player: P0, + convoke_mode: Some(engine::types::game_state::ConvokeMode::Convoke), + }; + } + let state = runner.state(); + let candidates = engine::ai_support::candidate_actions_broad(state); + let colorless = candidates + .iter() + .find(|c| { + matches!( + c.action, + GameAction::TapForConvoke { + object_id, + mana_type: ManaType::Colorless, + } if object_id == creature + ) + }) + .expect("production candidate path must offer the Colorless convoke tap") + .clone(); + let colored = candidates + .iter() + .find(|c| { + matches!( + c.action, + GameAction::TapForConvoke { + object_id, + mana_type: ManaType::Blue, + } if object_id == creature + ) + }) + .expect("production candidate path must offer the matching colored convoke tap") + .clone(); + + let decision = AiDecisionContext { + waiting_for: state.waiting_for.clone(), + candidates: candidates.clone(), + }; + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let context = AiContext::empty(&config.weights); + + let colorless_ctx = PolicyContext { + state, + decision: &decision, + candidate: &colorless, + ai_player: P0, + config: &config, + context: &context, + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_eq!(assess_candidate(&colorless_ctx), GateDecision::Reject); + + let colored_ctx = PolicyContext { + state, + decision: &decision, + candidate: &colored, + ai_player: P0, + config: &config, + context: &context, + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_eq!(assess_candidate(&colored_ctx), GateDecision::Allow); + } + + /// Builds an Improvise-eligible artifact with one `Activated` mana ability + /// producing Blue under the given `cost`, plus a `{1}{U}` pending cast, and + /// returns the production `ManaPayment` candidates + /// (`candidate_actions_broad` -> `mana_payment_actions`) for it. + fn improvise_artifact_with_mana_ability_candidates( + cost: Option, + ) -> ( + engine::game::scenario::GameRunner, + ObjectId, + Vec, + ) { + let mut scenario = GameScenario::new(); + let artifact = scenario.add_creature(P0, "Improvise Artifact", 0, 0).id(); + let mut runner = scenario.build(); + { + let state = runner.state_mut(); + let obj = state.objects.get_mut(&artifact).unwrap(); + obj.card_types.core_types.push(CoreType::Artifact); + let mut mana_ability = engine::types::ability::AbilityDefinition::new( + engine::types::ability::AbilityKind::Activated, + Effect::Mana { + produced: engine::types::ability::ManaProduction::Fixed { + colors: vec![engine::types::mana::ManaColor::Blue], + contribution: engine::types::ability::ManaContribution::Base, + }, + restrictions: vec![], + grants: vec![], + expiry: None, + target: None, + }, + ); + mana_ability.cost = cost; + std::sync::Arc::make_mut(&mut obj.abilities).push(mana_ability); + + state.pending_cast = Some(Box::new(PendingCast::new( + ObjectId(900), + CardId(900), + ResolvedAbility::new( + Effect::Draw { + count: engine::types::ability::QuantityExpr::Fixed { value: 0 }, + target: TargetFilter::Controller, + }, + Vec::new(), + ObjectId(900), + P0, + ), + ManaCost::Cost { + shards: vec![engine::types::mana::ManaCostShard::Blue], + generic: 1, + }, + ))); + state.waiting_for = WaitingFor::ManaPayment { + player: P0, + convoke_mode: Some(engine::types::game_state::ConvokeMode::Improvise), + }; + } + let candidates = engine::ai_support::candidate_actions_broad(runner.state()); + (runner, artifact, candidates) + } + + fn find_colorless_convoke_candidate( + candidates: &[CandidateAction], + object_id: ObjectId, + ) -> CandidateAction { + candidates + .iter() + .find(|c| { + matches!( + c.action, + GameAction::TapForConvoke { + object_id: o, + mana_type: ManaType::Colorless, + } if o == object_id + ) + }) + .expect("production candidate path must offer the Colorless convoke tap") + .clone() + } + + /// Review finding on #6840: a tapless mana ability (e.g. a + /// sacrifice-based one) on the SAME permanent as the Colorless Improvise + /// marker does not compete for the tap -- both can legally be used in the + /// same payment (Colorless first, then sacrifice the permanent for its + /// ability), so it must not gate the Colorless action. Drives the real + /// production `ManaPayment` candidate set, not a synthetic sibling. + #[test] + fn allows_colorless_improvise_tap_when_sibling_mana_ability_is_tapless() { + let (runner, artifact, candidates) = improvise_artifact_with_mana_ability_candidates(Some( + engine::types::ability::AbilityCost::Sacrifice( + engine::types::ability::SacrificeCost::count(TargetFilter::Any, 1), + ), + )); + let state = runner.state(); + let colorless = find_colorless_convoke_candidate(&candidates, artifact); + let decision = AiDecisionContext { + waiting_for: state.waiting_for.clone(), + candidates: candidates.clone(), + }; + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let context = AiContext::empty(&config.weights); + let ctx = PolicyContext { + state, + decision: &decision, + candidate: &colorless, + ai_player: P0, + config: &config, + context: &context, + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_eq!(assess_candidate(&ctx), GateDecision::Allow); + } + + /// Regression guard for the fix above: a genuine tap-cost native mana + /// ability on the same permanent still gates the Colorless marker, via + /// the real production candidate path. + #[test] + fn rejects_colorless_improvise_tap_when_sibling_mana_ability_taps() { + let (runner, artifact, candidates) = improvise_artifact_with_mana_ability_candidates(Some( + engine::types::ability::AbilityCost::Tap, + )); + let state = runner.state(); + let colorless = find_colorless_convoke_candidate(&candidates, artifact); + let decision = AiDecisionContext { + waiting_for: state.waiting_for.clone(), + candidates: candidates.clone(), + }; + let config = create_config(AiDifficulty::VeryHard, Platform::Wasm); + let context = AiContext::empty(&config.weights); + let ctx = PolicyContext { + state, + decision: &decision, + candidate: &colorless, + ai_player: P0, + config: &config, + context: &context, + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + }; + assert_eq!(assess_candidate(&ctx), GateDecision::Reject); + } } From 774a5dc6b1c12aee59478e9ae60733e309bfa8a5 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 7 Aug 2026 02:05:31 -0700 Subject: [PATCH 13/14] fix(ai): update pending trigger fixture provenance --- crates/phase-ai/src/policies/anti_self_harm.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/phase-ai/src/policies/anti_self_harm.rs b/crates/phase-ai/src/policies/anti_self_harm.rs index c85303f525..de887d9bb7 100644 --- a/crates/phase-ai/src/policies/anti_self_harm.rs +++ b/crates/phase-ai/src/policies/anti_self_harm.rs @@ -4088,6 +4088,7 @@ mod tests { may_trigger_origin: None, subject_match_count: None, die_result: None, + provenance: None, })); let config = AiConfig::default(); From 8d47671edb6f5e26740a73312e76838413871167 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 7 Aug 2026 03:03:36 -0700 Subject: [PATCH 14/14] test(engine): update prompt census source coordinate --- crates/engine/src/game/engine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 5f2138a547..fd01786865 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -15898,7 +15898,7 @@ mod stage2_injector_tests { // `scoped_library_search.rs`, neither of which this change touches, and the // test module it adds contains no line matching the needle — total still 37, // partition still 5/7/25. - "game/engine.rs:11828".to_string(), + "game/engine.rs:11841".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 \