From 62e465b9f49b7fb97c5ae7e4b9224586b3ab3a7e Mon Sep 17 00:00:00 2001 From: minion1227 Date: Sun, 26 Jul 2026 05:09:10 -0700 Subject: [PATCH 1/4] fix(phase-ai): rank damage-removal targets by whether the damage is lethal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #6582. EvasionRemovalPriorityPolicy ranked removal targets purely by threat value, so a fixed-damage burn spell was pointed at the biggest creature on the board — a 7/7 — even when the spell deals 3 and cannot destroy it, wasting the card. "Kills it" and "tickles it" scored identically. New building block `policies/removal_lethality.rs` (pure functions over the pending spell's own DealDamage effects and the target's runtime toughness / marked damage / indestructibility): - pending_damage_to_object: total damage the pending cast will deal to a candidate, resolved against live state (X/dynamic amounts concrete); None when it deals no damage, so the term stays inert for non-damage removal (-X/-X, destroy, exile). - damage_is_lethal: CR 120.6 / CR 704.5g marked-damage-vs-toughness, with CR 702.12b indestructible never lethal and a 0-toughness body not "killed" by the spell (its own SBA is). - lethality_bonus: +clean-kill for a lethal target; a waste penalty scaled by the surviving body (indestructible counts full toughness), capped — so a killable small target outranks a survivable large one. EvasionRemovalPriorityPolicy folds the term into its target score. This covers every direct-damage removal spell, not one card. Tests: 9 new removal-lethality tests (pure CR arithmetic + composed over a real pending-cast PolicyContext) + full 1514-test phase-ai lib suite pass; clippy -p phase-ai --all-targets -D warnings clean. Co-Authored-By: Claude Opus 4.8 --- .../src/policies/evasion_removal_priority.rs | 6 +- crates/phase-ai/src/policies/mod.rs | 1 + .../src/policies/removal_lethality.rs | 116 ++++++++++ crates/phase-ai/src/policies/tests/mod.rs | 1 + .../src/policies/tests/removal_lethality.rs | 205 ++++++++++++++++++ 5 files changed, 328 insertions(+), 1 deletion(-) create mode 100644 crates/phase-ai/src/policies/removal_lethality.rs create mode 100644 crates/phase-ai/src/policies/tests/removal_lethality.rs diff --git a/crates/phase-ai/src/policies/evasion_removal_priority.rs b/crates/phase-ai/src/policies/evasion_removal_priority.rs index dbb0a94532..13c11bbea4 100644 --- a/crates/phase-ai/src/policies/evasion_removal_priority.rs +++ b/crates/phase-ai/src/policies/evasion_removal_priority.rs @@ -58,8 +58,12 @@ impl EvasionRemovalPriorityPolicy { let target_quality_bonus = removal_target_quality_score(target_value); let evasion_bonus = evasion_score(ctx, target, *target_id); let velocity_bonus = velocity_score(ctx, target, *target_id); + // #6582: threat value alone rewards the biggest body, so a damage spell + // gets pointed at a creature it can't kill. Fold in whether the pending + // damage is actually lethal (CR 704.5g) so a clean kill outranks a whiff. + let lethality_bonus = super::removal_lethality::lethality_bonus(ctx, *target_id, target); - target_quality_bonus + evasion_bonus + velocity_bonus + target_quality_bonus + evasion_bonus + velocity_bonus + lethality_bonus } } diff --git a/crates/phase-ai/src/policies/mod.rs b/crates/phase-ai/src/policies/mod.rs index 5b139cb415..ce325c6d8e 100644 --- a/crates/phase-ai/src/policies/mod.rs +++ b/crates/phase-ai/src/policies/mod.rs @@ -47,6 +47,7 @@ mod reactive_self_protection; mod recursion_awareness; mod redundancy_avoidance; pub mod registry; +mod removal_lethality; mod sacrifice_land_protection; mod sacrifice_value; mod self_cost; diff --git a/crates/phase-ai/src/policies/removal_lethality.rs b/crates/phase-ai/src/policies/removal_lethality.rs new file mode 100644 index 0000000000..4c7d7fca15 --- /dev/null +++ b/crates/phase-ai/src/policies/removal_lethality.rs @@ -0,0 +1,116 @@ +//! Removal-lethality assessment — makes a direct-damage removal spell prefer a +//! target its damage can actually KILL over the biggest body on the board. +//! +//! ## The defect this closes (#6582) +//! +//! [`EvasionRemovalPriorityPolicy`](super::evasion_removal_priority) ranks +//! removal targets by threat value, so it points a fixed-damage burn spell at +//! the biggest creature — a 7/7 — even when the spell deals 3 and cannot +//! destroy it, wasting the card. CR 120.6 / CR 704.5g: a creature is destroyed +//! only once the damage marked on it reaches its toughness; CR 702.12b: an +//! indestructible creature is never destroyed by lethal damage. The AI modelled +//! neither, so "kills it" and "tickles it" scored the same and the biggest +//! threat always won. +//! +//! ## Building block, not a card fix +//! +//! These are pure functions over the pending spell's own `DealDamage` effects +//! and the target's runtime toughness / marked damage / indestructibility — a +//! reusable primitive any removal-targeting policy can consult, covering every +//! direct-damage removal spell rather than one card. The term is inert (`0.0`) +//! whenever the pending effect deals no damage to the target, so `-X/-X`, +//! destroy, and exile removal are untouched. + +use engine::game::game_object::GameObject; +use engine::game::quantity::resolve_quantity; +use engine::types::ability::Effect; +use engine::types::identifiers::ObjectId; +use engine::types::keywords::Keyword; + +use super::context::PolicyContext; +use super::effect_classify::effect_targets_object; + +/// Reward for a target the removal spell actually destroys — a clean kill is +/// worth more than the marginal threat-value ranking that lured the AI to an +/// un-killable body. Sized to clear `removal_target_quality_score`'s `2.0` cap +/// so a lethal small target outranks a survivable large one. +pub(crate) const LETHAL_BONUS: f64 = 2.5; +/// Per-point-of-survived-toughness penalty for a damage spell that leaves the +/// creature alive — the classic "3 damage on a 7/7" waste. Scaling by the body +/// it failed to kill punishes the biggest whiffs hardest. +pub(crate) const WASTE_PENALTY_MULT: f64 = 0.45; +/// Cap on the waste penalty so a single non-lethal target can dampen but not +/// completely dominate the overall target ranking. +pub(crate) const WASTE_PENALTY_MAX: f64 = 3.0; + +/// Total damage the pending spell/ability will deal to `target_id`, resolved +/// against live game state (so `X` and dynamic amounts are concrete). `None` +/// when the pending effect deals no damage to that object at all — the signal +/// the caller uses to stay inert for non-damage removal. +/// +/// CR 120.3e: damage from a source without wither/infect is marked on the +/// creature; multiple `DealDamage` effects on one spell stack additively. +pub(crate) fn pending_damage_to_object( + ctx: &PolicyContext<'_>, + target_id: ObjectId, +) -> Option { + let source_id = ctx.source_object()?.id; + let mut total: i64 = 0; + let mut found = false; + for effect in ctx.effects() { + if let Effect::DealDamage { amount, .. } = effect { + if effect_targets_object(ctx, effect, target_id) { + found = true; + total += + i64::from(resolve_quantity(ctx.state, amount, ctx.ai_player, source_id).max(0)); + } + } + } + found.then_some(total.clamp(0, i64::from(u32::MAX)) as u32) +} + +/// CR 120.6 + CR 704.5g: `damage` is lethal to `target` when the target has +/// toughness greater than 0 and the damage added to what is already marked +/// reaches that toughness — UNLESS the creature is indestructible (CR 702.12b), +/// which ignores the lethal-damage state-based action. A creature already at 0 +/// toughness is dying to its own SBA, not to this spell, so it is not "killed" +/// by the damage here. +pub(crate) fn damage_is_lethal(target: &GameObject, damage: u32) -> bool { + if target.has_keyword(&Keyword::Indestructible) { + return false; + } + let toughness = target.toughness.unwrap_or(0); + if toughness <= 0 { + return false; + } + target.damage_marked.saturating_add(damage) >= toughness as u32 +} + +/// Lethality contribution for pointing a damage removal spell at `target`. +/// +/// * Lethal (CR 704.5g destroy) → `+LETHAL_BONUS`: a clean kill. +/// * Survives (ordinary high toughness OR indestructible, CR 702.12b) → a +/// penalty scaled by the body it failed to kill, so a 3-damage spell on a 7/7 +/// ranks well below a smaller target the same spell destroys. +/// * Pending spell deals no damage to the target → `0.0`, leaving non-damage +/// removal targeting (`-X/-X`, destroy, exile) exactly as it was. +pub(crate) fn lethality_bonus( + ctx: &PolicyContext<'_>, + target_id: ObjectId, + target: &GameObject, +) -> f64 { + let Some(damage) = pending_damage_to_object(ctx, target_id) else { + return 0.0; + }; + if damage == 0 { + return 0.0; + } + if damage_is_lethal(target, damage) { + return LETHAL_BONUS; + } + // Survives: the removal is wasted. Penalty grows with the toughness of the + // creature that lived through it (indestructible bodies count their full + // toughness), capped so one whiff can't swamp the ranking. + let survived_toughness = f64::from(target.toughness.unwrap_or(0).max(0)); + -(survived_toughness * WASTE_PENALTY_MULT).min(WASTE_PENALTY_MAX) +} diff --git a/crates/phase-ai/src/policies/tests/mod.rs b/crates/phase-ai/src/policies/tests/mod.rs index ecaaa98afb..77a313b10a 100644 --- a/crates/phase-ai/src/policies/tests/mod.rs +++ b/crates/phase-ai/src/policies/tests/mod.rs @@ -13,4 +13,5 @@ pub mod mill_payoff; pub mod mulligan_input_lint; pub mod poison; pub mod reanimator_payoff; +pub mod removal_lethality; pub mod score_contract_lint; diff --git a/crates/phase-ai/src/policies/tests/removal_lethality.rs b/crates/phase-ai/src/policies/tests/removal_lethality.rs new file mode 100644 index 0000000000..c2afddd890 --- /dev/null +++ b/crates/phase-ai/src/policies/tests/removal_lethality.rs @@ -0,0 +1,205 @@ +//! Unit tests for `policies::removal_lethality` — CR 704.5g removal-target +//! lethality. No `#[cfg(test)]` in SOURCE files; tests live here. +//! +//! The pure `damage_is_lethal` arithmetic is checked directly; the composed +//! `lethality_bonus` runs against a real `PolicyContext` built over a pending +//! damage cast in `TargetSelection`, mirroring the engine's own +//! `effects_returns_pending_cast_during_target_selection` fixture. + +use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, TacticalClass}; +use engine::game::zones::create_object; +use engine::types::ability::{Effect, QuantityExpr, ResolvedAbility, TargetFilter, TargetRef}; +use engine::types::actions::GameAction; +use engine::types::card_type::{CardType, CoreType}; +use engine::types::format::FormatConfig; +use engine::types::game_state::{GameState, PendingCast, TargetSelectionSlot, WaitingFor}; +use engine::types::identifiers::CardId; +use engine::types::keywords::Keyword; +use engine::types::mana::ManaCost; +use engine::types::player::PlayerId; +use engine::types::zones::Zone; + +use crate::config::AiConfig; +use crate::context::AiContext; +use crate::policies::context::{PolicyContext, SearchDepth}; +use crate::policies::removal_lethality::*; + +const AI: PlayerId = PlayerId(0); +const OPP: PlayerId = PlayerId(1); + +// ─── damage_is_lethal (pure CR arithmetic) ────────────────────────────────── + +/// A tiny stand-in creature carrying only the fields lethality reads. +fn creature( + toughness: i32, + damage_marked: u32, + indestructible: bool, +) -> engine::game::game_object::GameObject { + let mut state = GameState::new(FormatConfig::standard(), 2, 42); + let id = create_object(&mut state, CardId(9), OPP, "Body".into(), Zone::Battlefield); + let obj = state.objects.get_mut(&id).unwrap(); + obj.card_types = CardType { + supertypes: Vec::new(), + core_types: vec![CoreType::Creature], + subtypes: Vec::new(), + }; + obj.power = Some(1); + obj.toughness = Some(toughness); + obj.damage_marked = damage_marked; + if indestructible { + obj.keywords.push(Keyword::Indestructible); + } + state.objects.remove(&id).unwrap() +} + +#[test] +fn exact_toughness_is_lethal() { + // CR 704.5g: 3 damage on an undamaged 3-toughness body destroys it. + assert!(damage_is_lethal(&creature(3, 0, false), 3)); +} + +#[test] +fn short_of_toughness_is_not_lethal() { + // The #6582 misplay: 3 damage on a 7-toughness body. + assert!(!damage_is_lethal(&creature(7, 0, false), 3)); +} + +#[test] +fn prior_marked_damage_lowers_the_bar() { + // CR 120.6: marked damage accumulates — 1 already + 3 new ≥ 4 toughness. + assert!(damage_is_lethal(&creature(4, 1, false), 3)); +} + +#[test] +fn indestructible_is_never_lethal() { + // CR 702.12b: indestructible ignores the lethal-damage SBA. + assert!(!damage_is_lethal(&creature(1, 0, true), 99)); +} + +#[test] +fn zero_toughness_is_not_killed_by_the_spell() { + // Already dying to its own 0-toughness SBA (CR 704.5f), not to this damage. + assert!(!damage_is_lethal(&creature(0, 0, false), 5)); +} + +// ─── lethality_bonus (composed over a pending cast) ───────────────────────── + +/// Build a pending `DealDamage` (or non-damage) cast in `TargetSelection` +/// aimed at one opponent creature and return the lethality term for choosing it. +fn bonus_for( + target_toughness: i32, + target_damage_marked: u32, + indestructible: bool, + effect: Effect, +) -> f64 { + let mut state = GameState::new(FormatConfig::standard(), 2, 42); + let spell = create_object(&mut state, CardId(1), AI, "Removal".into(), Zone::Stack); + let target = create_object(&mut state, CardId(2), OPP, "Body".into(), Zone::Battlefield); + { + let obj = state.objects.get_mut(&target).unwrap(); + obj.card_types = CardType { + supertypes: Vec::new(), + core_types: vec![CoreType::Creature], + subtypes: Vec::new(), + }; + obj.power = Some(1); + obj.toughness = Some(target_toughness); + obj.damage_marked = target_damage_marked; + if indestructible { + obj.keywords.push(Keyword::Indestructible); + } + } + + let ability = ResolvedAbility::new(effect, Vec::new(), spell, AI); + let pending = PendingCast::new(spell, CardId(1), ability, ManaCost::zero()); + let decision = AiDecisionContext { + waiting_for: WaitingFor::TargetSelection { + player: AI, + pending_cast: Box::new(pending), + target_slots: vec![TargetSelectionSlot { + legal_targets: Vec::new(), + optional: false, + chooser: None, + }], + mode_labels: Vec::new(), + selection: Default::default(), + }, + candidates: Vec::new(), + }; + let candidate = CandidateAction { + action: GameAction::ChooseTarget { + target: Some(TargetRef::Object(target)), + }, + metadata: ActionMetadata::for_actor(Some(AI), TacticalClass::Target), + }; + let config = AiConfig::default(); + let aicontext = AiContext::empty(&config.weights); + let ctx = PolicyContext { + state: &state, + decision: &decision, + candidate: &candidate, + ai_player: AI, + config: &config, + context: &aicontext, + cast_facts: None, + search_depth: SearchDepth::Root, + }; + let target_obj = state.objects.get(&target).unwrap(); + lethality_bonus(&ctx, target, target_obj) +} + +fn burn(damage: i32) -> Effect { + Effect::DealDamage { + amount: QuantityExpr::Fixed { value: damage }, + target: TargetFilter::Any, + damage_source: None, + excess: None, + } +} + +#[test] +fn lethal_target_is_rewarded() { + // 3 damage kills a 3/3 → the clean-kill bonus. + let b = bonus_for(3, 0, false, burn(3)); + assert!( + (b - LETHAL_BONUS).abs() < 1e-9, + "expected +{LETHAL_BONUS} for a clean kill, got {b}" + ); +} + +#[test] +fn nonlethal_big_target_is_penalized() { + // The #6582 misplay: 3 damage on a 7/7. Must score net-negative so a + // killable smaller target outranks it. + let b = bonus_for(7, 0, false, burn(3)); + assert!( + b < 0.0, + "expected a waste penalty for a survivable target, got {b}" + ); + // And the penalty must exceed the +2.0 target-quality lure it counteracts. + assert!( + b <= -2.0, + "penalty must overcome the threat-value bonus, got {b}" + ); +} + +#[test] +fn indestructible_target_is_penalized_even_when_damage_exceeds_toughness() { + // CR 702.12b: 5 damage on an indestructible 1/1 still whiffs. + let b = bonus_for(1, 0, true, burn(5)); + assert!( + b < 0.0, + "indestructible target must read as wasted, got {b}" + ); +} + +#[test] +fn non_damage_removal_is_inert() { + // A Destroy spell carries no DealDamage → the term must not perturb its + // targeting at all. + let destroy = Effect::Destroy { + target: TargetFilter::Any, + cant_regenerate: false, + }; + assert_eq!(bonus_for(7, 0, false, destroy), 0.0); +} From 62b1943d80c661b86351016ba8c1b4623e69d9ff Mon Sep 17 00:00:00 2001 From: minion1227 Date: Sun, 26 Jul 2026 06:25:07 -0700 Subject: [PATCH 2/4] fix(phase-ai): model source-dependent damage results in removal lethality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1 found the lethality term dropped `Effect::DealDamage`'s `damage_source` and modelled a kill as marked-damage-vs-toughness with an indestructible exception. That is not the engine's damage model, so the policy could mis-rank three whole classes of removal. Damage results depend on the SOURCE, not just the amount (CR 120.3), so the per-effect amount is now reduced to a typed `DamageOutcome` (marked damage + -1/-1 counters + deathtouch) resolved against that effect's own source, and only then judged by `outcome_is_lethal`, which follows the same precedence as `engine::game::sba`: - CR 120.3d + CR 702.80a/702.90c: a wither/infect source marks no damage; it puts -1/-1 counters on the creature, lowering toughness (CR 122.1a). Reaching 0 toughness is CR 704.5f, which is not a destruction, so CR 702.12b indestructible does NOT prevent it — previously scored as a waste even when it cleanly killed. - CR 702.2b + CR 704.5h: a deathtouch source makes any marked damage lethal, however large the body — previously missed entirely. - CR 704.5g: the marked-damage threshold now drops with any -1/-1 counters the same spell adds. Where the damage source is not knowable while a target is still being chosen, the term reports the new `PendingDamage::Unresolved` and the policy stays neutral instead of scoring a guess: - `DamageSource::Target` — the first object target IS the source and is excluded from the recipient slice, so the object being scored may not be a recipient at all (the review's source-scored-as-recipient bug). - `DamageSource::EachTarget` — every leading target is an independent source with its own keywords and its own re-resolved amount. - `DamageSource::TriggeringSource` — bound to the triggering event's object; the engine's `extract_source_from_event` authority is crate-private and re-deriving it here would duplicate engine logic. - `EachDealsDamageEqualToPower` / `EachSourceDealsDamage` / `DamageAll` / `ApplyPostReplacementDamage` — per-source batches this layer does not model, so they bail rather than silently under-count to zero. Tests: 22 removal-lethality tests (up from 9). New discriminating cases for default-source deathtouch on an oversized body, wither and infect driving an indestructible creature to 0 toughness, counter-reduced marked-damage thresholds, waste scaled by surviving toughness, and `Target` / `EachTarget` / `TriggeringSource` / multi-source neutrality — each paired with a same-amount control that IS scored, so the assertion discriminates the source semantics rather than the target filter. Full 1527-test phase-ai lib suite passes; clippy -p phase-ai --all-targets -D warnings clean. Co-Authored-By: Claude Opus 4.8 --- .../src/policies/removal_lethality.rs | 290 ++++++++++--- .../src/policies/tests/removal_lethality.rs | 391 +++++++++++++++--- 2 files changed, 574 insertions(+), 107 deletions(-) diff --git a/crates/phase-ai/src/policies/removal_lethality.rs b/crates/phase-ai/src/policies/removal_lethality.rs index 4c7d7fca15..1a0667d016 100644 --- a/crates/phase-ai/src/policies/removal_lethality.rs +++ b/crates/phase-ai/src/policies/removal_lethality.rs @@ -6,32 +6,57 @@ //! [`EvasionRemovalPriorityPolicy`](super::evasion_removal_priority) ranks //! removal targets by threat value, so it points a fixed-damage burn spell at //! the biggest creature — a 7/7 — even when the spell deals 3 and cannot -//! destroy it, wasting the card. CR 120.6 / CR 704.5g: a creature is destroyed -//! only once the damage marked on it reaches its toughness; CR 702.12b: an -//! indestructible creature is never destroyed by lethal damage. The AI modelled -//! neither, so "kills it" and "tickles it" scored the same and the biggest -//! threat always won. +//! destroy it, wasting the card. The AI modelled no lethality at all, so +//! "kills it" and "tickles it" scored the same and the biggest threat always +//! won. +//! +//! ## Model the engine's damage RESULTS, not a damage integer +//! +//! Whether damage kills depends on the damage SOURCE, not only on the amount +//! (CR 120.3), so collapsing a spell's damage into one number gets three whole +//! classes of removal wrong: +//! +//! * CR 120.3d + CR 702.80a: a source with wither/infect marks no damage at +//! all — it puts that many -1/-1 counters on the creature, which lower its +//! toughness (CR 122.1a). Reaching 0 toughness is CR 704.5f (put into the +//! graveyard), which is *not* a destruction, so indestructible (CR 702.12b) +//! does not save the creature. +//! * CR 702.2b + CR 704.5h: a source with deathtouch makes *any* marked damage +//! lethal, however large the body. +//! * CR 120.3: for `DamageSource::Target` the first object target IS the source +//! and is excluded from the recipients, so the object being scored may not be +//! a recipient at all. +//! +//! The pending spell's damage is therefore reduced to a typed [`DamageOutcome`] +//! (marked damage + -1/-1 counters + deathtouch) resolved per damage source, +//! and only then judged against the target in [`outcome_is_lethal`], which +//! mirrors the state-based-action precedence in `engine::game::sba`. Where the +//! source is not knowable while a target is still being chosen, the term +//! reports [`PendingDamage::Unresolved`] and the policy stays neutral rather +//! than scoring a guess. //! //! ## Building block, not a card fix //! -//! These are pure functions over the pending spell's own `DealDamage` effects -//! and the target's runtime toughness / marked damage / indestructibility — a -//! reusable primitive any removal-targeting policy can consult, covering every -//! direct-damage removal spell rather than one card. The term is inert (`0.0`) -//! whenever the pending effect deals no damage to the target, so `-X/-X`, -//! destroy, and exile removal are untouched. +//! These are pure functions over the pending spell's own damage effects and the +//! target's runtime state — a reusable primitive any removal-targeting policy +//! can consult, covering every direct-damage removal spell rather than one +//! card. The term is inert (`0.0`) whenever the pending effect deals no +//! modelled damage to the target, so `-X/-X`, destroy, and exile removal are +//! untouched. use engine::game::game_object::GameObject; +use engine::game::keywords::object_has_effective_keyword_kind; use engine::game::quantity::resolve_quantity; -use engine::types::ability::Effect; +use engine::types::ability::{DamageSource, Effect}; +use engine::types::card_type::CoreType; use engine::types::identifiers::ObjectId; -use engine::types::keywords::Keyword; +use engine::types::keywords::{Keyword, KeywordKind}; use super::context::PolicyContext; use super::effect_classify::effect_targets_object; -/// Reward for a target the removal spell actually destroys — a clean kill is -/// worth more than the marginal threat-value ranking that lured the AI to an +/// Reward for a target the removal spell actually kills — a clean kill is worth +/// more than the marginal threat-value ranking that lured the AI to an /// un-killable body. Sized to clear `removal_target_quality_score`'s `2.0` cap /// so a lethal small target outranks a survivable large one. pub(crate) const LETHAL_BONUS: f64 = 2.5; @@ -43,74 +68,231 @@ pub(crate) const WASTE_PENALTY_MULT: f64 = 0.45; /// completely dominate the overall target ranking. pub(crate) const WASTE_PENALTY_MAX: f64 = 3.0; -/// Total damage the pending spell/ability will deal to `target_id`, resolved -/// against live game state (so `X` and dynamic amounts are concrete). `None` -/// when the pending effect deals no damage to that object at all — the signal -/// the caller uses to stay inert for non-damage removal. +/// CR 120.3: the object whose characteristics govern one damage effect's +/// results. Deathtouch (CR 702.2b) and wither/infect (CR 120.3d) are read from +/// the SOURCE, never from the spell that created the damage, so the source must +/// be resolved before an amount can be turned into an outcome. +enum EffectDamageSource { + /// A concrete object, resolvable while targets are still being chosen. + Object(ObjectId), + /// The source depends on information this policy does not have yet: + /// + /// * [`DamageSource::Target`] — the first object target *is* the source and + /// is excluded from the recipient slice + /// (`effects::deal_damage::resolve_effect_recipients`), so the object + /// being scored may be the source rather than a recipient. + /// * [`DamageSource::EachTarget`] — every leading target is an independent + /// source with its own keywords and its own re-resolved amount. + /// * [`DamageSource::TriggeringSource`] — bound to the triggering event's + /// object; the engine's `targeting::extract_source_from_event` authority + /// is crate-private, and re-deriving that mapping in the AI layer would + /// duplicate engine logic. + Unresolved, +} + +/// CR 120.3: resolve which object deals one `DealDamage` effect's damage. +fn effect_damage_source( + ctx: &PolicyContext<'_>, + damage_source: Option<&DamageSource>, +) -> EffectDamageSource { + match damage_source { + // CR 120.3: default — the spell or ability's own source deals the damage. + None => ctx + .source_object() + .map_or(EffectDamageSource::Unresolved, |object| { + EffectDamageSource::Object(object.id) + }), + Some(DamageSource::Target | DamageSource::EachTarget | DamageSource::TriggeringSource) => { + EffectDamageSource::Unresolved + } + } +} + +/// CR 120.3: how one modelled batch of damage lands on a single creature. Kept +/// as a typed per-source outcome so the results stay distinguishable through +/// aggregation instead of collapsing into a single "damage" integer that +/// silently loses wither/infect and deathtouch. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub(crate) struct DamageOutcome { + /// CR 120.3e: damage from sources with neither wither nor infect, marked on + /// the creature. + pub(crate) marked: u32, + /// CR 120.3d + CR 702.80a: damage from a wither/infect source, dealt as + /// -1/-1 counters instead of being marked. + pub(crate) minus_counters: u32, + /// CR 702.2b: at least one source contributing this damage has deathtouch. + pub(crate) deathtouch: bool, +} + +/// What the pending spell or ability does to one candidate object, resolved +/// against live game state (so `X` and dynamic amounts are concrete). +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum PendingDamage { + /// No damage effect on the pending spell reaches this object — the signal + /// the caller uses to stay inert for non-damage removal. + None, + /// A damage effect reaches (or may reach) this object, but its source — and + /// therefore its result — is not modellable during target selection. The + /// caller stays neutral instead of scoring a guess. + Unresolved, + /// Fully modelled damage results. + Dealt(DamageOutcome), +} + +/// Reduce every damage effect on the pending spell that reaches `target` into a +/// single typed [`PendingDamage`]. /// -/// CR 120.3e: damage from a source without wither/infect is marked on the -/// creature; multiple `DealDamage` effects on one spell stack additively. +/// CR 120.3d / CR 120.3e: each effect's amount is routed to -1/-1 counters or +/// to marked damage according to ITS OWN source's wither/infect, so a spell +/// mixing sources aggregates correctly. pub(crate) fn pending_damage_to_object( ctx: &PolicyContext<'_>, target_id: ObjectId, -) -> Option { - let source_id = ctx.source_object()?.id; - let mut total: i64 = 0; + target: &GameObject, +) -> PendingDamage { + // CR 120.3d: only a creature converts wither/infect damage into -1/-1 + // counters; other permanents take the damage by their own rules. + let is_creature = target.card_types.core_types.contains(&CoreType::Creature); + let mut outcome = DamageOutcome::default(); let mut found = false; + for effect in ctx.effects() { - if let Effect::DealDamage { amount, .. } = effect { - if effect_targets_object(ctx, effect, target_id) { + match effect { + Effect::DealDamage { + amount, + damage_source, + .. + } => { + if !effect_targets_object(ctx, effect, target_id) { + continue; + } + let EffectDamageSource::Object(source_id) = + effect_damage_source(ctx, damage_source.as_ref()) + else { + return PendingDamage::Unresolved; + }; found = true; - total += - i64::from(resolve_quantity(ctx.state, amount, ctx.ai_player, source_id).max(0)); + let dealt = u32::try_from( + resolve_quantity(ctx.state, amount, ctx.ai_player, source_id).max(0), + ) + .unwrap_or(u32::MAX); + // CR 120.3d + CR 702.80a + CR 702.90c: wither/infect damage to a + // creature is dealt as -1/-1 counters and is never marked. + if is_creature + && (object_has_effective_keyword_kind( + ctx.state, + source_id, + KeywordKind::Wither, + ) || object_has_effective_keyword_kind( + ctx.state, + source_id, + KeywordKind::Infect, + )) + { + outcome.minus_counters = outcome.minus_counters.saturating_add(dealt); + } else { + // CR 120.3e: otherwise the damage is marked on the creature. + outcome.marked = outcome.marked.saturating_add(dealt); + } + // CR 702.2b: the deathtouch flag comes from the source that + // actually dealt damage, mirroring `dealt_deathtouch_damage`. + outcome.deathtouch |= dealt > 0 + && object_has_effective_keyword_kind( + ctx.state, + source_id, + KeywordKind::Deathtouch, + ); } + // CR 120.1: multi-source batches and mass damage put damage on this + // object from sources this policy does not model per-source. Bail + // rather than under-count and mis-report a lethal spell as a whiff. + Effect::EachDealsDamageEqualToPower { .. } + | Effect::EachSourceDealsDamage { .. } + | Effect::DamageAll { .. } + | Effect::ApplyPostReplacementDamage { .. } => return PendingDamage::Unresolved, + _ => {} } } - found.then_some(total.clamp(0, i64::from(u32::MAX)) as u32) -} -/// CR 120.6 + CR 704.5g: `damage` is lethal to `target` when the target has -/// toughness greater than 0 and the damage added to what is already marked -/// reaches that toughness — UNLESS the creature is indestructible (CR 702.12b), -/// which ignores the lethal-damage state-based action. A creature already at 0 -/// toughness is dying to its own SBA, not to this spell, so it is not "killed" -/// by the damage here. -pub(crate) fn damage_is_lethal(target: &GameObject, damage: u32) -> bool { - if target.has_keyword(&Keyword::Indestructible) { - return false; + if found { + PendingDamage::Dealt(outcome) + } else { + PendingDamage::None } +} + +/// Does `outcome` kill `target` at the next state-based action check? +/// +/// Ordered to match the precedence in `engine::game::sba`: +/// +/// 1. CR 704.5f: -1/-1 counters (CR 120.3d) lower toughness (CR 122.1a); at 0 +/// or less the creature is put into its owner's graveyard. That is not a +/// destruction, so CR 702.12b indestructible does NOT prevent it. +/// 2. CR 702.12b: otherwise an indestructible creature ignores both +/// lethal-damage state-based actions. +/// 3. CR 704.5h + CR 702.2b: any marked damage from a deathtouch source is +/// lethal to a creature with toughness greater than 0. +/// 4. CR 704.5g: marked damage reaching the counter-reduced toughness. +/// +/// A creature already at 0 or less toughness is dying to its own CR 704.5f +/// state-based action, not to this spell, so it does not count as killed here. +pub(crate) fn outcome_is_lethal(target: &GameObject, outcome: &DamageOutcome) -> bool { let toughness = target.toughness.unwrap_or(0); if toughness <= 0 { return false; } - target.damage_marked.saturating_add(damage) >= toughness as u32 + let counters = i32::try_from(outcome.minus_counters).unwrap_or(i32::MAX); + let reduced_toughness = toughness.saturating_sub(counters); + // CR 704.5f: 0 toughness kills through indestructible. + if reduced_toughness <= 0 { + return true; + } + // CR 702.12b: indestructible ignores the lethal-damage state-based actions. + if target.has_keyword(&Keyword::Indestructible) { + return false; + } + let marked = target.damage_marked.saturating_add(outcome.marked); + // CR 704.5h + CR 702.2b: deathtouch damage is lethal regardless of amount. + if outcome.deathtouch && marked > 0 { + return true; + } + // CR 704.5g: lethal marked damage, measured against the reduced toughness. + u32::try_from(reduced_toughness).is_ok_and(|threshold| marked >= threshold) +} + +/// Toughness the creature keeps after this spell's -1/-1 counters — the size of +/// the body the removal failed to kill, used to scale the waste penalty. +fn surviving_toughness(target: &GameObject, outcome: &DamageOutcome) -> i32 { + let counters = i32::try_from(outcome.minus_counters).unwrap_or(i32::MAX); + target + .toughness + .unwrap_or(0) + .saturating_sub(counters) + .max(0) } /// Lethality contribution for pointing a damage removal spell at `target`. /// -/// * Lethal (CR 704.5g destroy) → `+LETHAL_BONUS`: a clean kill. -/// * Survives (ordinary high toughness OR indestructible, CR 702.12b) → a -/// penalty scaled by the body it failed to kill, so a 3-damage spell on a 7/7 -/// ranks well below a smaller target the same spell destroys. -/// * Pending spell deals no damage to the target → `0.0`, leaving non-damage -/// removal targeting (`-X/-X`, destroy, exile) exactly as it was. +/// * Kills it (CR 704.5f / CR 704.5g / CR 704.5h) → `+LETHAL_BONUS`. +/// * Survives (high toughness, or indestructible per CR 702.12b) → a penalty +/// scaled by the body it failed to kill, so a 3-damage spell on a 7/7 ranks +/// well below a smaller target the same spell destroys. +/// * No modelled damage reaches the target, or the damage source is not +/// resolvable during target selection → `0.0`, leaving that targeting +/// decision exactly as it was. pub(crate) fn lethality_bonus( ctx: &PolicyContext<'_>, target_id: ObjectId, target: &GameObject, ) -> f64 { - let Some(damage) = pending_damage_to_object(ctx, target_id) else { + let PendingDamage::Dealt(outcome) = pending_damage_to_object(ctx, target_id, target) else { return 0.0; }; - if damage == 0 { + if outcome.marked == 0 && outcome.minus_counters == 0 { return 0.0; } - if damage_is_lethal(target, damage) { + if outcome_is_lethal(target, &outcome) { return LETHAL_BONUS; } - // Survives: the removal is wasted. Penalty grows with the toughness of the - // creature that lived through it (indestructible bodies count their full - // toughness), capped so one whiff can't swamp the ranking. - let survived_toughness = f64::from(target.toughness.unwrap_or(0).max(0)); - -(survived_toughness * WASTE_PENALTY_MULT).min(WASTE_PENALTY_MAX) + -(f64::from(surviving_toughness(target, &outcome)) * WASTE_PENALTY_MULT).min(WASTE_PENALTY_MAX) } diff --git a/crates/phase-ai/src/policies/tests/removal_lethality.rs b/crates/phase-ai/src/policies/tests/removal_lethality.rs index c2afddd890..b86f58fc8a 100644 --- a/crates/phase-ai/src/policies/tests/removal_lethality.rs +++ b/crates/phase-ai/src/policies/tests/removal_lethality.rs @@ -1,19 +1,28 @@ -//! Unit tests for `policies::removal_lethality` — CR 704.5g removal-target +//! Unit tests for `policies::removal_lethality` — CR 704.5f-h removal-target //! lethality. No `#[cfg(test)]` in SOURCE files; tests live here. //! -//! The pure `damage_is_lethal` arithmetic is checked directly; the composed -//! `lethality_bonus` runs against a real `PolicyContext` built over a pending -//! damage cast in `TargetSelection`, mirroring the engine's own -//! `effects_returns_pending_cast_during_target_selection` fixture. +//! The pure `outcome_is_lethal` arithmetic is checked directly; the composed +//! `pending_damage_to_object` / `lethality_bonus` pair runs against a real +//! `PolicyContext` built over a pending damage cast in `TargetSelection`, +//! mirroring the engine's own +//! `effects_returns_pending_cast_during_target_selection` fixture. The spell +//! object carries real keywords, so the source-dependent damage results +//! (CR 120.3d wither/infect counters, CR 702.2b deathtouch) are exercised +//! through the same `object_has_effective_keyword_kind` authority the engine's +//! `DamageContext::from_source` reads. use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, TacticalClass}; +use engine::game::game_object::GameObject; use engine::game::zones::create_object; -use engine::types::ability::{Effect, QuantityExpr, ResolvedAbility, TargetFilter, TargetRef}; +use engine::types::ability::{ + DamageSource, EachDamageRecipient, Effect, QuantityExpr, ResolvedAbility, TargetFilter, + TargetRef, +}; use engine::types::actions::GameAction; use engine::types::card_type::{CardType, CoreType}; use engine::types::format::FormatConfig; use engine::types::game_state::{GameState, PendingCast, TargetSelectionSlot, WaitingFor}; -use engine::types::identifiers::CardId; +use engine::types::identifiers::{CardId, ObjectId}; use engine::types::keywords::Keyword; use engine::types::mana::ManaCost; use engine::types::player::PlayerId; @@ -27,87 +36,195 @@ use crate::policies::removal_lethality::*; const AI: PlayerId = PlayerId(0); const OPP: PlayerId = PlayerId(1); -// ─── damage_is_lethal (pure CR arithmetic) ────────────────────────────────── - -/// A tiny stand-in creature carrying only the fields lethality reads. -fn creature( +/// The body a removal spell is pointed at. +#[derive(Clone, Copy)] +struct Body { toughness: i32, damage_marked: u32, indestructible: bool, -) -> engine::game::game_object::GameObject { - let mut state = GameState::new(FormatConfig::standard(), 2, 42); - let id = create_object(&mut state, CardId(9), OPP, "Body".into(), Zone::Battlefield); - let obj = state.objects.get_mut(&id).unwrap(); +} + +impl Body { + const fn new(toughness: i32) -> Self { + Self { + toughness, + damage_marked: 0, + indestructible: false, + } + } + + const fn marked(mut self, damage_marked: u32) -> Self { + self.damage_marked = damage_marked; + self + } + + const fn indestructible(mut self) -> Self { + self.indestructible = true; + self + } +} + +/// Shape `object_id` into `body` in place so the pure-arithmetic helper and the +/// composed fixture build the identical creature. +fn shape_body(state: &mut GameState, object_id: ObjectId, body: Body) { + let obj = state.objects.get_mut(&object_id).unwrap(); obj.card_types = CardType { supertypes: Vec::new(), core_types: vec![CoreType::Creature], subtypes: Vec::new(), }; obj.power = Some(1); - obj.toughness = Some(toughness); - obj.damage_marked = damage_marked; - if indestructible { + obj.toughness = Some(body.toughness); + obj.damage_marked = body.damage_marked; + if body.indestructible { obj.keywords.push(Keyword::Indestructible); } +} + +// ─── outcome_is_lethal (pure CR 704.5f-h arithmetic) ──────────────────────── + +/// A detached stand-in creature carrying only the fields lethality reads. +fn creature(body: Body) -> GameObject { + let mut state = GameState::new(FormatConfig::standard(), 2, 42); + let id = create_object(&mut state, CardId(9), OPP, "Body".into(), Zone::Battlefield); + shape_body(&mut state, id, body); state.objects.remove(&id).unwrap() } +/// Ordinary marked damage from a source with no relevant keywords (CR 120.3e). +fn marked(amount: u32) -> DamageOutcome { + DamageOutcome { + marked: amount, + minus_counters: 0, + deathtouch: false, + } +} + #[test] fn exact_toughness_is_lethal() { // CR 704.5g: 3 damage on an undamaged 3-toughness body destroys it. - assert!(damage_is_lethal(&creature(3, 0, false), 3)); + assert!(outcome_is_lethal(&creature(Body::new(3)), &marked(3))); } #[test] fn short_of_toughness_is_not_lethal() { // The #6582 misplay: 3 damage on a 7-toughness body. - assert!(!damage_is_lethal(&creature(7, 0, false), 3)); + assert!(!outcome_is_lethal(&creature(Body::new(7)), &marked(3))); } #[test] fn prior_marked_damage_lowers_the_bar() { - // CR 120.6: marked damage accumulates — 1 already + 3 new ≥ 4 toughness. - assert!(damage_is_lethal(&creature(4, 1, false), 3)); + // CR 704.5g: marked damage accumulates — 1 already + 3 new ≥ 4 toughness. + assert!(outcome_is_lethal( + &creature(Body::new(4).marked(1)), + &marked(3) + )); } #[test] -fn indestructible_is_never_lethal() { - // CR 702.12b: indestructible ignores the lethal-damage SBA. - assert!(!damage_is_lethal(&creature(1, 0, true), 99)); +fn indestructible_survives_lethal_marked_damage() { + // CR 702.12b: indestructible ignores the lethal-damage state-based action. + assert!(!outcome_is_lethal( + &creature(Body::new(1).indestructible()), + &marked(99) + )); } #[test] fn zero_toughness_is_not_killed_by_the_spell() { // Already dying to its own 0-toughness SBA (CR 704.5f), not to this damage. - assert!(!damage_is_lethal(&creature(0, 0, false), 5)); + assert!(!outcome_is_lethal(&creature(Body::new(0)), &marked(5))); +} + +#[test] +fn deathtouch_marked_damage_kills_any_size_body() { + // CR 704.5h + CR 702.2b: 1 deathtouch damage destroys a 7/7 that the same + // amount of ordinary marked damage leaves untouched. + let outcome = DamageOutcome { + marked: 1, + minus_counters: 0, + deathtouch: true, + }; + assert!(outcome_is_lethal(&creature(Body::new(7)), &outcome)); + assert!(!outcome_is_lethal(&creature(Body::new(7)), &marked(1))); } -// ─── lethality_bonus (composed over a pending cast) ───────────────────────── +#[test] +fn deathtouch_without_marked_damage_is_not_lethal() { + // CR 704.5h keys on damage having been MARKED; a wither/infect deathtouch + // source marks none (CR 120.3d), so only the counters can kill. + let outcome = DamageOutcome { + marked: 0, + minus_counters: 1, + deathtouch: true, + }; + assert!(!outcome_is_lethal(&creature(Body::new(7)), &outcome)); +} -/// Build a pending `DealDamage` (or non-damage) cast in `TargetSelection` -/// aimed at one opponent creature and return the lethality term for choosing it. -fn bonus_for( - target_toughness: i32, - target_damage_marked: u32, - indestructible: bool, +#[test] +fn deathtouch_does_not_beat_indestructible() { + // CR 702.12b: indestructible ignores CR 704.5h as well as CR 704.5g. + let outcome = DamageOutcome { + marked: 3, + minus_counters: 0, + deathtouch: true, + }; + assert!(!outcome_is_lethal( + &creature(Body::new(7).indestructible()), + &outcome + )); +} + +#[test] +fn minus_counters_kill_through_indestructible() { + // CR 120.3d + CR 122.1a + CR 704.5f: -1/-1 counters drive a 3/3 to 0 + // toughness, and CR 704.5f is not a destruction, so CR 702.12b does not + // save it — the case a marked-damage-only model wrongly calls a waste. + let outcome = DamageOutcome { + marked: 0, + minus_counters: 3, + deathtouch: false, + }; + assert!(outcome_is_lethal( + &creature(Body::new(3).indestructible()), + &outcome + )); +} + +#[test] +fn minus_counters_lower_the_marked_damage_bar() { + // CR 122.1a + CR 704.5g: counters reduce toughness, so the marked-damage + // threshold drops with it — 2 counters + 2 marked kills a 4/4, not a 5/5. + let outcome = DamageOutcome { + marked: 2, + minus_counters: 2, + deathtouch: false, + }; + assert!(outcome_is_lethal(&creature(Body::new(4)), &outcome)); + assert!(!outcome_is_lethal(&creature(Body::new(5)), &outcome)); +} + +// ─── pending_damage_to_object / lethality_bonus (composed) ────────────────── + +/// Build a pending cast of `effect` in `TargetSelection`, aimed at one opponent +/// creature shaped by `body`, and run `probe` against the resulting context. +/// `spell_keywords` land on the spell object, which is the default damage source +/// (CR 120.3) — on `base_keywords` too, since off-battlefield keyword reads go +/// through `off_zone_characteristics` rather than `keywords`. +fn with_pending( + spell_keywords: &[Keyword], + body: Body, effect: Effect, -) -> f64 { + probe: impl FnOnce(&PolicyContext<'_>, ObjectId, &GameObject) -> R, +) -> R { let mut state = GameState::new(FormatConfig::standard(), 2, 42); let spell = create_object(&mut state, CardId(1), AI, "Removal".into(), Zone::Stack); let target = create_object(&mut state, CardId(2), OPP, "Body".into(), Zone::Battlefield); + shape_body(&mut state, target, body); { - let obj = state.objects.get_mut(&target).unwrap(); - obj.card_types = CardType { - supertypes: Vec::new(), - core_types: vec![CoreType::Creature], - subtypes: Vec::new(), - }; - obj.power = Some(1); - obj.toughness = Some(target_toughness); - obj.damage_marked = target_damage_marked; - if indestructible { - obj.keywords.push(Keyword::Indestructible); - } + let obj = state.objects.get_mut(&spell).unwrap(); + obj.keywords.extend(spell_keywords.iter().cloned()); + obj.base_keywords.extend(spell_keywords.iter().cloned()); } let ability = ResolvedAbility::new(effect, Vec::new(), spell, AI); @@ -145,14 +262,30 @@ fn bonus_for( search_depth: SearchDepth::Root, }; let target_obj = state.objects.get(&target).unwrap(); - lethality_bonus(&ctx, target, target_obj) + probe(&ctx, target, target_obj) +} + +fn bonus_for(spell_keywords: &[Keyword], body: Body, effect: Effect) -> f64 { + with_pending(spell_keywords, body, effect, |ctx, id, target| { + lethality_bonus(ctx, id, target) + }) +} + +fn pending_for(spell_keywords: &[Keyword], body: Body, effect: Effect) -> PendingDamage { + with_pending(spell_keywords, body, effect, |ctx, id, target| { + pending_damage_to_object(ctx, id, target) + }) } fn burn(damage: i32) -> Effect { + burn_from(damage, None) +} + +fn burn_from(damage: i32, damage_source: Option) -> Effect { Effect::DealDamage { amount: QuantityExpr::Fixed { value: damage }, target: TargetFilter::Any, - damage_source: None, + damage_source, excess: None, } } @@ -160,7 +293,7 @@ fn burn(damage: i32) -> Effect { #[test] fn lethal_target_is_rewarded() { // 3 damage kills a 3/3 → the clean-kill bonus. - let b = bonus_for(3, 0, false, burn(3)); + let b = bonus_for(&[], Body::new(3), burn(3)); assert!( (b - LETHAL_BONUS).abs() < 1e-9, "expected +{LETHAL_BONUS} for a clean kill, got {b}" @@ -171,7 +304,7 @@ fn lethal_target_is_rewarded() { fn nonlethal_big_target_is_penalized() { // The #6582 misplay: 3 damage on a 7/7. Must score net-negative so a // killable smaller target outranks it. - let b = bonus_for(7, 0, false, burn(3)); + let b = bonus_for(&[], Body::new(7), burn(3)); assert!( b < 0.0, "expected a waste penalty for a survivable target, got {b}" @@ -186,7 +319,7 @@ fn nonlethal_big_target_is_penalized() { #[test] fn indestructible_target_is_penalized_even_when_damage_exceeds_toughness() { // CR 702.12b: 5 damage on an indestructible 1/1 still whiffs. - let b = bonus_for(1, 0, true, burn(5)); + let b = bonus_for(&[], Body::new(1).indestructible(), burn(5)); assert!( b < 0.0, "indestructible target must read as wasted, got {b}" @@ -195,11 +328,163 @@ fn indestructible_target_is_penalized_even_when_damage_exceeds_toughness() { #[test] fn non_damage_removal_is_inert() { - // A Destroy spell carries no DealDamage → the term must not perturb its + // A Destroy spell carries no damage effect → the term must not perturb its // targeting at all. let destroy = Effect::Destroy { target: TargetFilter::Any, cant_regenerate: false, }; - assert_eq!(bonus_for(7, 0, false, destroy), 0.0); + assert_eq!( + pending_for(&[], Body::new(7), destroy.clone()), + PendingDamage::None + ); + assert_eq!(bonus_for(&[], Body::new(7), destroy), 0.0); +} + +// ─── source-dependent damage results ──────────────────────────────────────── + +#[test] +fn default_source_deathtouch_reads_as_a_clean_kill_on_an_oversized_body() { + // CR 120.3 + CR 702.2b + CR 704.5h: a 1-damage deathtouch source (a spell + // granted deathtouch) kills a 7/7 that ordinary burn only tickles. + assert_eq!( + pending_for(&[Keyword::Deathtouch], Body::new(7), burn(1)), + PendingDamage::Dealt(DamageOutcome { + marked: 1, + minus_counters: 0, + deathtouch: true, + }), + "deathtouch must be read from the resolved damage source" + ); + let b = bonus_for(&[Keyword::Deathtouch], Body::new(7), burn(1)); + assert!( + (b - LETHAL_BONUS).abs() < 1e-9, + "deathtouch removal on a 7/7 is a clean kill, got {b}" + ); + // Discriminating control: the same spell without deathtouch is the whiff. + assert!(bonus_for(&[], Body::new(7), burn(1)) < 0.0); +} + +#[test] +fn wither_kills_an_indestructible_creature_it_drives_to_zero_toughness() { + // CR 120.3d + CR 702.80a + CR 704.5f: 3 wither damage puts three -1/-1 + // counters on an indestructible 3/3, and 0 toughness is not a destruction. + assert_eq!( + pending_for(&[Keyword::Wither], Body::new(3).indestructible(), burn(3)), + PendingDamage::Dealt(DamageOutcome { + marked: 0, + minus_counters: 3, + deathtouch: false, + }), + "wither damage must become -1/-1 counters, not marked damage" + ); + let b = bonus_for(&[Keyword::Wither], Body::new(3).indestructible(), burn(3)); + assert!( + (b - LETHAL_BONUS).abs() < 1e-9, + "wither to 0 toughness kills through indestructible, got {b}" + ); + // Discriminating control: without wither the same damage is a pure waste. + assert!(bonus_for(&[], Body::new(3).indestructible(), burn(3)) < 0.0); +} + +#[test] +fn infect_kills_an_indestructible_creature_it_drives_to_zero_toughness() { + // CR 702.90c: infect routes damage to -1/-1 counters exactly as wither does. + assert_eq!( + pending_for(&[Keyword::Infect], Body::new(2).indestructible(), burn(2)), + PendingDamage::Dealt(DamageOutcome { + marked: 0, + minus_counters: 2, + deathtouch: false, + }) + ); + let b = bonus_for(&[Keyword::Infect], Body::new(2).indestructible(), burn(2)); + assert!( + (b - LETHAL_BONUS).abs() < 1e-9, + "infect to 0 toughness kills through indestructible, got {b}" + ); +} + +#[test] +fn wither_short_of_toughness_scales_the_waste_by_the_surviving_body() { + // CR 122.1a: two counters on a 7/7 leave a 7/5 alive, so the waste penalty + // must be measured against the toughness it kept, not the printed one. + let b = bonus_for(&[Keyword::Wither], Body::new(7), burn(2)); + let expected = -(5.0_f64 * WASTE_PENALTY_MULT).min(WASTE_PENALTY_MAX); + assert!( + (b - expected).abs() < 1e-9, + "expected {expected} for a 7/7 reduced to 7/5, got {b}" + ); +} + +#[test] +fn target_sourced_damage_stays_neutral() { + // CR 120.3: with `DamageSource::Target` the first object target IS the + // damage source and is excluded from the recipients, so this object may not + // be dealt damage at all. Its deathtouch/wither are likewise unknown while + // targets are still being chosen → stay out of the ranking entirely. + assert_eq!( + pending_for(&[], Body::new(3), burn_from(3, Some(DamageSource::Target))), + PendingDamage::Unresolved, + "a target-sourced damage effect must not be modelled as a recipient hit" + ); + assert_eq!( + bonus_for(&[], Body::new(3), burn_from(3, Some(DamageSource::Target))), + 0.0 + ); + // Discriminating control: the identical amount with the default source IS + // scored, so the neutrality above comes from the source, not the filter. + assert!(bonus_for(&[], Body::new(3), burn(3)) > 0.0); +} + +#[test] +fn each_target_sourced_damage_stays_neutral() { + // CR 120.1: every leading target is an independent source with its own + // keywords and its own re-resolved amount; only `targets.last()` is the + // recipient. + assert_eq!( + pending_for( + &[], + Body::new(3), + burn_from(3, Some(DamageSource::EachTarget)) + ), + PendingDamage::Unresolved + ); + assert_eq!( + bonus_for( + &[], + Body::new(3), + burn_from(3, Some(DamageSource::EachTarget)) + ), + 0.0 + ); +} + +#[test] +fn triggering_source_damage_stays_neutral() { + // CR 120.3: the source is the triggering event's object, whose keywords this + // layer cannot resolve — stay neutral rather than assume a vanilla source. + assert_eq!( + pending_for( + &[], + Body::new(3), + burn_from(3, Some(DamageSource::TriggeringSource)) + ), + PendingDamage::Unresolved + ); +} + +#[test] +fn multi_source_damage_effects_stay_neutral() { + // CR 120.1: `EachSourceDealsDamage` is a per-source batch this layer does + // not model, so it must not be silently under-counted to zero damage. + let each_source = Effect::EachSourceDealsDamage { + sources: TargetFilter::Any, + amount: QuantityExpr::Fixed { value: 3 }, + recipient: EachDamageRecipient::EachController, + }; + assert_eq!( + pending_for(&[], Body::new(3), each_source), + PendingDamage::Unresolved + ); } From d086a68293c098cf2fbe640a4a3af2b26c7bdc30 Mon Sep 17 00:00:00 2001 From: minion1227 Date: Sun, 26 Jul 2026 06:30:34 -0700 Subject: [PATCH 3/4] refactor(phase-ai): single authority for the counter-reduced toughness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `outcome_is_lethal` and the waste-penalty scale both needed toughness after this spell's -1/-1 counters (CR 120.3d + CR 122.1a) and each did the saturating subtraction itself. Extract `reduced_toughness` as the one authority: it is the CR 704.5g lethal-damage threshold, and clamped at 0 it is the surviving body the penalty scales by. No behaviour change — full 1527-test phase-ai lib suite passes, clippy -p phase-ai --all-targets -D warnings clean. Co-Authored-By: Claude Opus 4.8 --- .../src/policies/removal_lethality.rs | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/crates/phase-ai/src/policies/removal_lethality.rs b/crates/phase-ai/src/policies/removal_lethality.rs index 1a0667d016..ea6aec8553 100644 --- a/crates/phase-ai/src/policies/removal_lethality.rs +++ b/crates/phase-ai/src/policies/removal_lethality.rs @@ -237,14 +237,12 @@ pub(crate) fn pending_damage_to_object( /// A creature already at 0 or less toughness is dying to its own CR 704.5f /// state-based action, not to this spell, so it does not count as killed here. pub(crate) fn outcome_is_lethal(target: &GameObject, outcome: &DamageOutcome) -> bool { - let toughness = target.toughness.unwrap_or(0); - if toughness <= 0 { + if target.toughness.unwrap_or(0) <= 0 { return false; } - let counters = i32::try_from(outcome.minus_counters).unwrap_or(i32::MAX); - let reduced_toughness = toughness.saturating_sub(counters); + let reduced = reduced_toughness(target, outcome); // CR 704.5f: 0 toughness kills through indestructible. - if reduced_toughness <= 0 { + if reduced <= 0 { return true; } // CR 702.12b: indestructible ignores the lethal-damage state-based actions. @@ -257,18 +255,16 @@ pub(crate) fn outcome_is_lethal(target: &GameObject, outcome: &DamageOutcome) -> return true; } // CR 704.5g: lethal marked damage, measured against the reduced toughness. - u32::try_from(reduced_toughness).is_ok_and(|threshold| marked >= threshold) + u32::try_from(reduced).is_ok_and(|threshold| marked >= threshold) } -/// Toughness the creature keeps after this spell's -1/-1 counters — the size of -/// the body the removal failed to kill, used to scale the waste penalty. -fn surviving_toughness(target: &GameObject, outcome: &DamageOutcome) -> i32 { +/// CR 122.1a: `target`'s toughness once this spell's -1/-1 counters (CR 120.3d) +/// are on it. The single authority for the counter-reduced toughness — it is +/// both the CR 704.5g lethal-damage threshold and, clamped at 0, the size of the +/// body a non-lethal spell failed to kill. +fn reduced_toughness(target: &GameObject, outcome: &DamageOutcome) -> i32 { let counters = i32::try_from(outcome.minus_counters).unwrap_or(i32::MAX); - target - .toughness - .unwrap_or(0) - .saturating_sub(counters) - .max(0) + target.toughness.unwrap_or(0).saturating_sub(counters) } /// Lethality contribution for pointing a damage removal spell at `target`. @@ -294,5 +290,6 @@ pub(crate) fn lethality_bonus( if outcome_is_lethal(target, &outcome) { return LETHAL_BONUS; } - -(f64::from(surviving_toughness(target, &outcome)) * WASTE_PENALTY_MULT).min(WASTE_PENALTY_MAX) + let survived = reduced_toughness(target, &outcome).max(0); + -(f64::from(survived) * WASTE_PENALTY_MULT).min(WASTE_PENALTY_MAX) } From 058baba67cd8eb922af7ac2e665f21547e5a6cab Mon Sep 17 00:00:00 2001 From: minion1227 Date: Sun, 26 Jul 2026 10:53:56 -0700 Subject: [PATCH 4/4] test(phase-ai): production ranking regression + full batch-damage coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2 flagged two test-evidence gaps. Both closed. **[MED] production-path ranking regression.** The unit probes called `lethality_bonus` / `pending_damage_to_object` directly and never proved the bonus survives registry wiring. `burn_prefers_the_killable_body_over_ the_bigger_unkillable_threat` now drives the real path: a real Lightning Bolt ("Lightning Bolt deals 3 damage to any target.", Oracle text verified against Scryfall) is cast to `TargetSelection` over an opponent 2/2 and 7/7, with reach guards that the spell parses as 3 default-sourced damage and drops no clause. It then asserts at all three production layers — the registered `PolicyRegistry::verdicts` delta, the full `search::score_candidates` ranking, and `choose_action` not burning the 7/7. The killable body is deliberately the LOWER threat, so threat value alone picks the wrong target. Fail-on-revert verified: with the `evasion_removal_priority.rs:64` wiring neutralized, the registered policy scores the unkillable 7/7 at 2.0 and the killable 2/2 at 1.095 and the test fails — the exact #6582 misplay. **[LOW] batch-damage coverage matched to the claim.** The grouped match arm covers four variants but only `EachSourceDealsDamage` was exercised. `batch_damage_effects_stay_neutral` is now table-driven over all four — `EachSourceDealsDamage`, `EachDealsDamageEqualToPower`, `DamageAll`, and `ApplyPostReplacementDamage` — asserting both `PendingDamage::Unresolved` and a neutral bonus, so a later change can't make an untested grouped variant look lethal. The `ApplyPostReplacementDamage` snapshot sets every source characteristic off, so if it were ever modelled instead of bailing it would read as plain marked damage and the assertion would fail loudly. Tests: 23 removal-lethality tests + the production regression; full 1528-test phase-ai lib suite passes; clippy -p phase-ai --all-targets -D warnings clean. Co-Authored-By: Claude Opus 4.8 --- .../src/policies/evasion_removal_priority.rs | 111 +++++++++++++++++- .../src/policies/tests/removal_lethality.rs | 91 +++++++++++--- 2 files changed, 186 insertions(+), 16 deletions(-) diff --git a/crates/phase-ai/src/policies/evasion_removal_priority.rs b/crates/phase-ai/src/policies/evasion_removal_priority.rs index 13c11bbea4..8c685045e6 100644 --- a/crates/phase-ai/src/policies/evasion_removal_priority.rs +++ b/crates/phase-ai/src/policies/evasion_removal_priority.rs @@ -215,8 +215,8 @@ mod tests { use engine::game::scenario::{GameScenario, P0}; use engine::game::zones::create_object; use engine::types::ability::{ - AbilityDefinition, AbilityKind, Effect, EffectKind, PtValue, ResolvedAbility, TargetFilter, - TargetRef, TypedFilter, + AbilityDefinition, AbilityKind, Effect, EffectKind, PtValue, QuantityExpr, ResolvedAbility, + TargetFilter, TargetRef, TypedFilter, }; use engine::types::format::FormatConfig; use engine::types::game_state::{ @@ -234,6 +234,7 @@ mod tests { const BEAST_WITHIN_ORACLE: &str = "Destroy target permanent. Its controller creates a 3/3 green Beast creature token."; + const LIGHTNING_BOLT_ORACLE: &str = "Lightning Bolt deals 3 damage to any target."; fn add_creature( state: &mut GameState, @@ -466,6 +467,112 @@ mod tests { ); } + /// #6582 end-to-end: a real 3-damage burn spell must be aimed at the body it + /// can actually kill, not at the biggest threat on the board. This drives the + /// PRODUCTION path — a cast that reaches `TargetSelection`, the registered + /// `EvasionRemovalPriorityPolicy` verdict, and the full `score_candidates` + /// ranking — so the lethality term is proven to survive registry wiring, not + /// just to compute the right number in isolation. + #[test] + fn burn_prefers_the_killable_body_over_the_bigger_unkillable_threat() { + let mut scenario = GameScenario::new_n_player(2, 42); + scenario.at_phase(Phase::PreCombatMain); + let bolt = scenario + .add_spell_to_hand_from_oracle(P0, "Lightning Bolt", true, LIGHTNING_BOLT_ORACLE) + .with_mana_cost(ManaCost::Cost { + shards: vec![ManaCostShard::Red], + generic: 0, + }) + .id(); + // The killable body is deliberately the LOWER threat, so threat value + // alone (the pre-#6582 ranking) would pick the 7/7 the Bolt can't kill. + let killable = scenario + .add_creature(PlayerId(1), "Scrappy Skirmisher", 2, 2) + .id(); + let unkillable = scenario + .add_creature(PlayerId(1), "Looming Colossus", 7, 7) + .id(); + scenario.with_mana_pool( + P0, + vec![ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![])], + ); + + let mut runner = scenario.build(); + let card_id = runner.state().objects[&bolt].card_id; + runner + .act(GameAction::CastSpell { + object_id: bolt, + card_id, + targets: Vec::new(), + payment_mode: CastPaymentMode::Auto, + }) + .expect("the real Lightning Bolt fixture should reach target selection"); + + let (pending_cast, target_slots) = match &runner.state().waiting_for { + WaitingFor::TargetSelection { + pending_cast, + target_slots, + .. + } => (pending_cast, target_slots), + other => panic!("expected Lightning Bolt target selection, got {other:?}"), + }; + let effects = crate::policies::context::collect_ability_effects(&pending_cast.ability); + assert!( + effects.iter().any(|effect| matches!( + effect, + Effect::DealDamage { + amount: QuantityExpr::Fixed { value: 3 }, + damage_source: None, + .. + } + )), + "reach guard: Lightning Bolt must parse as 3 default-sourced damage" + ); + assert!( + effects + .iter() + .all(|effect| !matches!(effect, Effect::Unimplemented { .. })), + "the regression fixture must not silently drop an unsupported clause" + ); + assert!(target_slots[0] + .legal_targets + .contains(&TargetRef::Object(killable))); + assert!(target_slots[0] + .legal_targets + .contains(&TargetRef::Object(unkillable))); + + let state = runner.state(); + let decision = build_decision_context(state); + let config = create_config(AiDifficulty::VeryHard, Platform::Native).into_measurement(42); + + let killable_delta = registry_delta(state, &decision, killable, &config); + let unkillable_delta = registry_delta(state, &decision, unkillable, &config); + assert!( + killable_delta > unkillable_delta, + "the registered removal policy must prefer the body the Bolt kills: \ + killable 2/2={killable_delta}, unkillable 7/7={unkillable_delta}" + ); + + let scores = crate::search::score_candidates(state, P0, &config); + assert!( + full_score_for_target(&scores, killable) > full_score_for_target(&scores, unkillable), + "the complete Very Hard scorer must carry the lethality preference through to the \ + final target ranking" + ); + + // The #6582 misplay itself: whatever else Very Hard does with a Bolt + // ("any target" keeps the opponent's face on the table), it must not + // burn it on a 7/7 that survives. + let mut rng = SmallRng::seed_from_u64(42); + assert_ne!( + crate::choose_action(state, P0, &config, &mut rng), + Some(GameAction::ChooseTarget { + target: Some(TargetRef::Object(unkillable)), + }), + "Very Hard must not spend a 3-damage burn spell on a 7/7 it cannot kill" + ); + } + #[test] fn activated_removal_weights_controller_threat_but_beneficial_activation_is_neutral() { let destroy = Effect::Destroy { diff --git a/crates/phase-ai/src/policies/tests/removal_lethality.rs b/crates/phase-ai/src/policies/tests/removal_lethality.rs index b86f58fc8a..b9ef7638af 100644 --- a/crates/phase-ai/src/policies/tests/removal_lethality.rs +++ b/crates/phase-ai/src/policies/tests/removal_lethality.rs @@ -15,8 +15,8 @@ use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, Tac use engine::game::game_object::GameObject; use engine::game::zones::create_object; use engine::types::ability::{ - DamageSource, EachDamageRecipient, Effect, QuantityExpr, ResolvedAbility, TargetFilter, - TargetRef, + DamageContextSnapshot, DamageSource, EachDamageRecipient, Effect, QuantityExpr, + ResolvedAbility, TargetFilter, TargetRef, }; use engine::types::actions::GameAction; use engine::types::card_type::{CardType, CoreType}; @@ -474,17 +474,80 @@ fn triggering_source_damage_stays_neutral() { ); } +/// A snapshot standing in for an already-replaced damage event (CR 120.3), with +/// every source characteristic off — so if `ApplyPostReplacementDamage` were +/// ever modelled instead of bailing, it would look like plain marked damage and +/// the `Unresolved` assertion below would fail loudly. +fn vanilla_damage_snapshot() -> DamageContextSnapshot { + DamageContextSnapshot { + source_id: ObjectId(1), + controller: AI, + source_is_creature: false, + has_deathtouch: false, + has_lifelink: false, + has_wither: false, + has_infect: false, + combat_damage_poison: 0, + excess_recipient: None, + lifelink_bonus: 0, + } +} + #[test] -fn multi_source_damage_effects_stay_neutral() { - // CR 120.1: `EachSourceDealsDamage` is a per-source batch this layer does - // not model, so it must not be silently under-counted to zero damage. - let each_source = Effect::EachSourceDealsDamage { - sources: TargetFilter::Any, - amount: QuantityExpr::Fixed { value: 3 }, - recipient: EachDamageRecipient::EachController, - }; - assert_eq!( - pending_for(&[], Body::new(3), each_source), - PendingDamage::Unresolved - ); +fn batch_damage_effects_stay_neutral() { + // CR 120.1: every member of the batch-damage group puts damage on this object + // from sources this layer does not model per-source. Each must report + // `Unresolved` — a variant that silently under-counted to zero would look + // like "no damage reaches this target" and leave the ranking unguarded. + // Covered as a table so the assertion set can never drift behind the match + // arm it guards in `pending_damage_to_object`. + let batch = [ + ( + "EachSourceDealsDamage", + Effect::EachSourceDealsDamage { + sources: TargetFilter::Any, + amount: QuantityExpr::Fixed { value: 3 }, + recipient: EachDamageRecipient::EachController, + }, + ), + ( + "EachDealsDamageEqualToPower", + Effect::EachDealsDamageEqualToPower { + sources: TargetFilter::Any, + recipient: TargetFilter::Any, + extra_source: None, + }, + ), + ( + "DamageAll", + Effect::DamageAll { + amount: QuantityExpr::Fixed { value: 3 }, + target: TargetFilter::Any, + player_filter: None, + damage_source: None, + }, + ), + ( + "ApplyPostReplacementDamage", + Effect::ApplyPostReplacementDamage { + context: vanilla_damage_snapshot(), + target: TargetRef::Object(ObjectId(1)), + amount: 3, + is_combat: false, + }, + ), + ]; + + for (name, effect) in batch { + assert_eq!( + pending_for(&[], Body::new(3), effect.clone()), + PendingDamage::Unresolved, + "{name} must report Unresolved rather than under-counting to zero" + ); + assert_eq!( + bonus_for(&[], Body::new(3), effect), + 0.0, + "{name} must leave the target ranking untouched" + ); + } }