From d4ddd2042cf92642e1c5b4fb181a224b8985d226 Mon Sep 17 00:00:00 2001 From: Christopher Galpin <52485+CodeOptimist@users.noreply.github.com> Date: Fri, 7 Aug 2026 08:59:19 -0600 Subject: [PATCH] Fix AI target selection for DamageSource::Target to prefer lethal recipients ### The Bug * The AI opponent misplayed self-destructive exchange spells (like Self-Destruct) by targeting large bodies it could not kill (e.g., a 3/3) instead of available lethal kills (e.g., 0/1s). ### Root Cause * The `removal_lethality` policy unconditionally mapped `DamageSource::Target` to `EffectDamageSource::Unresolved`. * This blinded the AI's lethality scoring to the source's power and keywords, forcing target selection to rely purely on base threat value, which mathematically favors the largest bodies on the board. ### The Fix * Updated `effect_damage_source` in `removal_lethality.rs` to read the bound source object directly from `TargetSelectionProgress.selected_slots` during interactive selection. * Added a `bound_target_slice` helper to properly resolve `QuantityRef::Power { scope: Target }` ("X is its power") against the bound source using the existing `resolve_quantity_with_targets_slice` mechanic. * The implementation strictly adheres to CR 120.3 (establishing the first object target as the damage source) and CR 601.2c (pulling target binding from selected slots). ### Testing and Verification * Added building-block unit tests to verify that `bound_target_sourced_damage` accurately rewards lethal recipients and penalizes non-lethal recipients with surviving toughness. * Ensured that unbound (first-slot) decisions, `EachTarget`, and `TriggeringSource` effects correctly fall back to neutral (`Unresolved`) to preserve existing reach-guards. * Validated through an integration test guard that the AI now correctly chooses the lethal 0/1 Wizard over the 3/3 Cloud of Darkness. --- .../src/policies/removal_lethality.rs | 90 +++++++++- .../src/policies/tests/removal_lethality.rs | 169 +++++++++++++++++- crates/phase-ai/src/search.rs | 57 ++++++ 3 files changed, 300 insertions(+), 16 deletions(-) diff --git a/crates/phase-ai/src/policies/removal_lethality.rs b/crates/phase-ai/src/policies/removal_lethality.rs index ea6aec8553..4404502959 100644 --- a/crates/phase-ai/src/policies/removal_lethality.rs +++ b/crates/phase-ai/src/policies/removal_lethality.rs @@ -46,9 +46,10 @@ 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::{DamageSource, Effect}; +use engine::game::quantity::{resolve_quantity, resolve_quantity_with_targets_slice}; +use engine::types::ability::{DamageSource, Effect, TargetRef}; use engine::types::card_type::CoreType; +use engine::types::game_state::WaitingFor; use engine::types::identifiers::ObjectId; use engine::types::keywords::{Keyword, KeywordKind}; @@ -102,12 +103,63 @@ fn effect_damage_source( .map_or(EffectDamageSource::Unresolved, |object| { EffectDamageSource::Object(object.id) }), - Some(DamageSource::Target | DamageSource::EachTarget | DamageSource::TriggeringSource) => { + // CR 120.3: for `DamageSource::Target` the first object target IS the + // source. During interactive target selection the engine binds already- + // declared targets in `TargetSelectionProgress.selected_slots` (NOT + // `ability.targets`, which stays empty until + // `assign_selected_slots_in_chain` welds the final selection after the + // last slot commits — CR 601.2c / CR 608.2c). Once a later slot's + // selection is being made the source is already bound there and is + // knowable, so lethality against it can be modelled. + Some(DamageSource::Target) => match bound_target_source_id(ctx) { + Some(source_id) => EffectDamageSource::Object(source_id), + // Before the source's own slot is declared (`selected_slots` empty) + // there is no bound source to model — stay neutral rather than + // guessing (first-slot / empty-selection case). + None => EffectDamageSource::Unresolved, + }, + // CR 120.1 (EachTarget) / triggering-event source: the source is not + // resolvable from interactive target selection — see the enum doc. + Some(DamageSource::EachTarget | DamageSource::TriggeringSource) => { EffectDamageSource::Unresolved } } } +/// CR 120.3: the first already-declared OBJECT target of a `DamageSource::Target` +/// effect, as bound in `TargetSelectionProgress.selected_slots` while targets +/// are still being chosen (CR 601.2c). Before that slot is declared the source +/// is not yet bound, so the caller stays `Unresolved` (neutral), not a guess. +fn bound_target_source_id(ctx: &PolicyContext<'_>) -> Option { + match &ctx.decision.waiting_for { + WaitingFor::TargetSelection { selection, .. } + | WaitingFor::TriggerTargetSelection { selection, .. } => { + selection.selected_slots.iter().find_map(|slot| match slot { + Some(TargetRef::Object(id)) => Some(*id), + _ => None, + }) + } + _ => None, + } +} + +/// CR 601.2c: the declared-object target slice for the current interactive +/// selection — the engine buffers already-chosen targets in +/// `TargetSelectionProgress.selected_slots`. Used to resolve a +/// `DamageSource::Target` amount that references the first object target +/// (`QuantityRef::Power { scope: Target }`), so "X, where X is its power" +/// reads the already-bound source's power. A `Some(TargetRef::Object(id))` +/// slot is unwrapped into the slice; `None`/`Player` slots are skipped. +fn bound_target_slice(ctx: &PolicyContext<'_>) -> Vec { + match &ctx.decision.waiting_for { + WaitingFor::TargetSelection { selection, .. } + | WaitingFor::TriggerTargetSelection { selection, .. } => { + selection.selected_slots.iter().flatten().cloned().collect() + } + _ => Vec::new(), + } +} + /// 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 @@ -166,16 +218,40 @@ pub(crate) fn pending_damage_to_object( if !effect_targets_object(ctx, effect, target_id) { continue; } + // CR 120.3: resolve the damage source. let EffectDamageSource::Object(source_id) = effect_damage_source(ctx, damage_source.as_ref()) else { return PendingDamage::Unresolved; }; found = true; - let dealt = u32::try_from( - resolve_quantity(ctx.state, amount, ctx.ai_player, source_id).max(0), - ) - .unwrap_or(u32::MAX); + // CR 120.3 + CR 208.1 + CR 601.2c: for a `DamageSource::Target` + // effect whose amount is "X, where X is its power", the amount is + // the FIRST object target's power — the same bound source object + // resolved above. `resolve_quantity_with_targets_slice` resolves + // `QuantityRef::Power { scope: Target }` against the first entry + // of the passed slice, which is the declared source already bound + // in `selection.selected_slots[0]`. All other sources resolve the + // amount against the source object (CR 120.3 default) or a fixed + // value, unchanged. + let dealt = if matches!(damage_source, Some(DamageSource::Target)) { + u32::try_from( + resolve_quantity_with_targets_slice( + ctx.state, + amount, + ctx.ai_player, + source_id, + &bound_target_slice(ctx), + ) + .max(0), + ) + .unwrap_or(u32::MAX) + } else { + 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 diff --git a/crates/phase-ai/src/policies/tests/removal_lethality.rs b/crates/phase-ai/src/policies/tests/removal_lethality.rs index acb46cec6d..60cc49629a 100644 --- a/crates/phase-ai/src/policies/tests/removal_lethality.rs +++ b/crates/phase-ai/src/policies/tests/removal_lethality.rs @@ -15,14 +15,15 @@ use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, Tac use engine::game::game_object::GameObject; use engine::game::zones::create_object; use engine::types::ability::{ - DamageContextSnapshot, DamageSource, EachDamageRecipient, Effect, EffectKind, QuantityExpr, - ResolvedAbility, TargetFilter, TargetRef, + DamageContextSnapshot, DamageSource, EachDamageRecipient, Effect, EffectKind, ObjectScope, + QuantityExpr, QuantityRef, 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, TargetEffectDetail, TargetSelectionSlot, WaitingFor, + GameState, PendingCast, TargetEffectDetail, TargetSelectionProgress, TargetSelectionSlot, + WaitingFor, }; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::keywords::Keyword; @@ -44,6 +45,7 @@ struct Body { toughness: i32, damage_marked: u32, indestructible: bool, + power: i32, } impl Body { @@ -52,6 +54,7 @@ impl Body { toughness, damage_marked: 0, indestructible: false, + power: 1, } } @@ -64,6 +67,11 @@ impl Body { self.indestructible = true; self } + + const fn power(mut self, power: i32) -> Self { + self.power = power; + self + } } /// Shape `object_id` into `body` in place so the pure-arithmetic helper and the @@ -75,7 +83,7 @@ fn shape_body(state: &mut GameState, object_id: ObjectId, body: Body) { core_types: vec![CoreType::Creature], subtypes: Vec::new(), }; - obj.power = Some(1); + obj.power = Some(body.power); obj.toughness = Some(body.toughness); obj.damage_marked = body.damage_marked; if body.indestructible { @@ -218,11 +226,42 @@ fn with_pending( body: Body, effect: Effect, probe: impl FnOnce(&PolicyContext<'_>, ObjectId, &GameObject) -> R, +) -> R { + with_pending_and_source(spell_keywords, body, effect, None, probe) +} + +/// Like [`with_pending`], but optionally pre-binds a `DamageSource::Target` +/// source object in `selection.selected_slots[0]` — mirroring a real later-slot +/// decision for a Self-Destruct-class spell. The FIRST object target is the +/// damage source (CR 120.3) and, once it is declared during interative target +/// selection, lives in `TargetSelectionProgress.selected_slots` (CR 601.2c) even +/// though `ability.targets` stays empty until `assign_selected_slots_in_chain` +/// welds the final selection. `source` (an AI-controlled creature) is created in +/// the state and bound as slot 0. +fn with_pending_and_source( + spell_keywords: &[Keyword], + body: Body, + effect: Effect, + source: Option, + 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 selected_slots = if let Some(source_body) = source { + let source_id = create_object( + &mut state, + CardId(3), + AI, + "Source".into(), + Zone::Battlefield, + ); + shape_body(&mut state, source_id, source_body); + vec![Some(TargetRef::Object(source_id))] + } else { + Vec::new() + }; { let obj = state.objects.get_mut(&spell).unwrap(); obj.keywords.extend(spell_keywords.iter().cloned()); @@ -243,7 +282,10 @@ fn with_pending( effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), - selection: Default::default(), + selection: TargetSelectionProgress { + selected_slots, + ..Default::default() + }, }, candidates: Vec::new(), }; @@ -281,6 +323,39 @@ fn pending_for(spell_keywords: &[Keyword], body: Body, effect: Effect) -> Pendin }) } +/// Same as [`with_pending`] but with a pre-bound `DamageSource::Target` source +/// of `source_power` in `selected_slots[0]` (Self-Destruct class). `X` in the +/// effect resolves to `its power` (CR 120.3). +fn pending_for_with_source( + spell_keywords: &[Keyword], + body: Body, + source_power: i32, + effect: Effect, +) -> PendingDamage { + with_pending_and_source( + spell_keywords, + body, + effect, + Some(Body::new(1).power(source_power)), + pending_damage_to_object, + ) +} + +fn bonus_for_with_source( + spell_keywords: &[Keyword], + body: Body, + source_power: i32, + effect: Effect, +) -> f64 { + with_pending_and_source( + spell_keywords, + body, + effect, + Some(Body::new(1).power(source_power)), + lethality_bonus, + ) +} + fn burn(damage: i32) -> Effect { burn_from(damage, None) } @@ -424,13 +499,14 @@ fn wither_short_of_toughness_scales_the_waste_by_the_surviving_body() { #[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. + // damage source and is excluded from the recipients. With NO source slot + // bound yet (`selected_slots` empty — the first-slot / source-declaration + // case, CR 601.2c), the source is not knowable while targets are still + // being chosen → stay out of the ranking entirely rather than guess. 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" + "an unbound 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))), @@ -441,6 +517,81 @@ fn target_sourced_damage_stays_neutral() { assert!(bonus_for(&[], Body::new(3), burn(3)) > 0.0); } +// ─── DamageSource::Target with a pre-bound source (Self-Destruct class) ────── +// CR 120.3: the first object target of a `DamageSource::Target` spell IS the +// damage source. Once it is declared during a later slot's interactive target +// selection it sits in `TargetSelectionProgress.selected_slots[0]` (CR 601.2c), +// so its power (the `X` of "deals X damage" — CR 208.1, CR 120.3) and its +// keywords (CR 120.3d wither/infect, CR 702.2b deathtouch) become knowable and +// lethality against the recipient can be modelled. + +/// A `DealDamage` whose amount is `its power` (Self-Destruct's `X = power`) and +/// whose source is the first object target — the faithful production shape. The +/// amount is `Power { scope: Target }` exactly as the parser emits it, and +/// resolves against the first object target (the bound source) via the +/// targets-aware resolver. +fn burn_source_power() -> Effect { + Effect::DealDamage { + amount: QuantityExpr::Ref { + qty: QuantityRef::Power { + scope: ObjectScope::Target, + }, + }, + target: TargetFilter::Any, + damage_source: Some(DamageSource::Target), + excess: None, + } +} + +#[test] +fn bound_target_sourced_damage_to_lethal_recipient_is_rewarded() { + // CR 120.3: with the 2/2 source bound as slot 0, Self-Destruct deals 2 + // damage; a 2/2 recipient is destroyed (CR 704.5g) → the clean-kill bonus. + // The amount is resolved against the SOURCE's power (CR 208.1), so the + // recipient's own 1/1 body is irrelevant to the amount. + let b = bonus_for_with_source(&[], Body::new(2), 2, burn_source_power()); + assert!( + (b - LETHAL_BONUS).abs() < 1e-9, + "a 2-power source must score a clean kill on a 2/2, got {b}" + ); + // Positive reach-guard: resolve the real pipeline, not just the score. + assert_eq!( + pending_for_with_source(&[], Body::new(2), 2, burn_source_power()), + PendingDamage::Dealt(DamageOutcome { + marked: 2, + minus_counters: 0, + deathtouch: false, + }) + ); +} + +#[test] +fn bound_target_sourced_damage_to_nonlethal_recipient_is_penalized() { + // CR 120.3: the same 2/2 source deals 2 damage into a 3/3, which it cannot + // kill (CR 704.5g). The waste penalty scales by the body it failed to kill. + let b = bonus_for_with_source(&[], Body::new(3), 2, burn_source_power()); + let expected = -(3.0_f64 * WASTE_PENALTY_MULT).min(WASTE_PENALTY_MAX); + assert!( + (b - expected).abs() < 1e-9, + "2 damage on a 3/3 must be penalized by the surviving toughness, got {b}" + ); + // Discriminating control — identical source & amount, only the recipient's + // body differs: the same spell must rank the lethal small body above this. + assert!(b < bonus_for_with_source(&[], Body::new(2), 2, burn_source_power())); +} + +#[test] +fn bound_target_sourced_damage_has_no_bound_source_stays_neutral() { + // CR 601.2c: before the source slot is declared (`selected_slots` empty) + // there is no source to resolve `its power` against → stay neutral rather + // than guess. This is the first-slot / source-declaration guard. + assert_eq!( + pending_for(&[], Body::new(3), burn_source_power()), + PendingDamage::Unresolved + ); + assert_eq!(bonus_for(&[], Body::new(3), burn_source_power()), 0.0); +} + #[test] fn each_target_sourced_damage_stays_neutral() { // CR 120.1: every leading target is an independent source with its own diff --git a/crates/phase-ai/src/search.rs b/crates/phase-ai/src/search.rs index c495af6055..e07c111b54 100644 --- a/crates/phase-ai/src/search.rs +++ b/crates/phase-ai/src/search.rs @@ -5399,6 +5399,63 @@ mod tests { .expect("the reducer must issue the test spell root cast") } + /// Drive the AI through a real cast of Self-Destruct where the opponent has + /// BOTH a big body the 2/2 source cannot kill (a non-lethal waste) and a + /// small body it CAN kill (a clean lethal kill). The tactical target + /// selection must pick the lethal small body over the survivable big one. + #[test] + fn self_destruct_target_selection_prefers_lethal_over_nonlethal_body() { + use engine::parser::oracle::parse_oracle_text; + + let mut state = make_state(); + add_mana(&mut state, P0, ManaType::Red, 1); + let spell = add_spell_to_hand(&mut state, P0, "Self-Destruct", 1); + let parsed = parse_oracle_text( + SELF_DESTRUCT_ORACLE, + "Self-Destruct", + &[], + &["Instant".to_string()], + &[], + ); + *Arc::make_mut(&mut state.objects.get_mut(&spell).unwrap().abilities) = parsed.abilities; + + // The AI's damage source: a 2/2 Bird token (deals X = power = 2). + let bird = add_creature(&mut state, P0, 2, 2); + // Opponent's board: + // a 3/3 Cloud of Darkness the 2 damage cannot kill ... + let cloud = add_creature(&mut state, P1, 3, 3); + // ... and lethal 0/1 Wizards the 2 damage destroys outright. + let wizard_a = add_creature(&mut state, P1, 0, 1); + let wizard_b = add_creature(&mut state, P1, 0, 1); + + let config = create_config(AiDifficulty::VeryHard, Platform::Native); + let mut rng = SmallRng::seed_from_u64(7); + + // Drive the AI through the full decision sequence (cast → source target + // → recipient target), exactly as the game loop replays candidate + // actions, and record every object it picks as a ChooseTarget target. + let mut picked: Vec = Vec::new(); + for _ in 0..20 { + let Some(action) = choose_action(&state, P0, &config, &mut rng) else { + break; + }; + if let GameAction::ChooseTarget { + target: Some(TargetRef::Object(id)), + } = &action + { + picked.push(*id); + } + if engine::game::engine::apply_as_current(&mut state, action).is_err() { + break; + } + } + + assert!( + picked.contains(&wizard_a) || picked.contains(&wizard_b), + "the AI must pick a lethal 0/1 Wizard as the Self-Destruct recipient (got picked targets {picked:?}, bird={bird:?} cloud={cloud:?})" + ); + } + #[test] fn choose_action_rejects_bad_self_destruct_before_cast_and_keeps_source_in_hand() { let (state, spell) = self_destruct_state(2, 3);