Skip to content

Commit b302136

Browse files
committed
fix(ai): rank DamageSource::Target removal by the source's lethality (phase-rs#6582 class)
The phase-rs#6582 burn-lethality term in removal_lethality stopped the AI wasting default-sourced burn on bodies it cannot kill, but it returned Unresolved for DamageSource::Target damage — Self-Destruct and every "Target creature deals X damage to ..." card. During the recipient slot of a multi-slot selection the damage source is already committed to selected_slots, so its power and wither/infect/deathtouch keywords are knowable. Resolving it there lets the recipient be ranked by whether the damage actually destroys the body instead of by raw threat value. Changes (all in phase-ai): - policies/context.rs: PolicyContext::first_selected_object_target() reads the already-chosen first object target (the damage source) from the in-flight TargetSelectionProgress (CR 120.1 + CR 120.3). - policies/removal_lethality.rs: DamageSource::Target now resolves its source from that leading slot (EachTarget / TriggeringSource stay Unresolved — genuinely not resolvable from one recipient slot), and the amount is resolved against a [source] target slice so a Power{Target} amount reads the source's current power instead of silently 0 (CR 608.2h + CR 208.1). Scope boundary is explicit: a DamageSource::Target reached from a triggered ability or the bulk MultiTargetSelection flow stays Unresolved (source not resolvable from a single recipient slot). - policies/tests/removal_lethality.rs: unit tests resolving a pre-chosen power source and a pre-chosen deathtouch source. - policies/evasion_removal_priority.rs: an end-to-end regression driving real Self-Destruct through cast -> TargetSelection -> the registered EvasionRemovalPriorityPolicy verdict, asserting the killable recipient outranks the unkillable one and pinning the lethality arithmetic. - search.rs: a confirmation test showing today's engine targeted_exchange_verdict already rejects the turn-16 Clash of the Eikons fight (AI 3/2 vs opponent 3/5), documenting that the fight-class "bad play" is owned by that separate authority rather than this term. Verification: cargo fmt clean; cargo clippy --all-targets -- -D warnings clean; full phase-ai lib suite 1983 passed / 0 failed / 8 ignored. The Self-Destruct e2e test was confirmed discriminating: reverting the production change flips it to fail with the exact phase-rs#6582 misplay (killable 2/2=0.644 < unkillable 3/3=1.38). An independent fresh-context review-impl pass returned no HIGH/MED findings and two LOW findings, both addressed with code: the damage-amount CR citation corrected from 608.2c to CR 608.2h, and the coverable surface made explicit for triggered-ability / MultiTargetSelection DamageSource::Target cases. Comment-only, re-verified fmt/clippy/tests clean. Note: this is scoped to recipient ranking for DamageSource::Target spells (the precise phase-rs#6582 scope). It does not address the separate question of whether the AI should cast a self-harming spell at all, which is handled by engine::ai_support::targeted_exchange_verdict (phase-rs#6826) at the root-cast gate.
1 parent ed0a8e5 commit b302136

5 files changed

Lines changed: 489 additions & 16 deletions

File tree

crates/phase-ai/src/policies/context.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,38 @@ impl<'a> PolicyContext<'a> {
132132
}
133133
}
134134

135+
/// First *already-chosen* object target of an in-flight target selection.
136+
///
137+
/// CR 120.1 + CR 120.3: for a `DamageSource::Target` effect ("Target creature
138+
/// deals X damage to ..."), the first object target IS the damage source. In
139+
/// a cast/activation multi-slot selection that source is committed to
140+
/// `selected_slots` before the later recipient slots are offered, so it is
141+
/// resolvable here while the AI is still choosing the recipient — which is
142+
/// what lets the removal-lethality term use the source's power/keywords
143+
/// instead of bailing to `Unresolved`. Returns `None` when the leading slot
144+
/// has not been picked yet or is not an object.
145+
///
146+
/// SCOPE: this reads only the ordinary `WaitingFor::TargetSelection` path
147+
/// (spells/activated abilities). It deliberately returns `None` for
148+
/// `TriggerTargetSelection` (event-bound source, CR 120.7) and for the bulk
149+
/// `MultiTargetSelection` path, because the source there is not resolvable
150+
/// from a single recipient slot — so `DamageSource::Target` in those flow
151+
/// contexts stays `Unresolved` in the removal-lethality term rather than
152+
/// being silently ranked on a guess.
153+
pub fn first_selected_object_target(&self) -> Option<ObjectId> {
154+
let selected_slots = match &self.decision.waiting_for {
155+
WaitingFor::TargetSelection { selection, .. } => Some(&selection.selected_slots),
156+
WaitingFor::TriggerTargetSelection { .. } => None,
157+
_ => None,
158+
};
159+
selected_slots.and_then(|slots| {
160+
slots.iter().find_map(|slot| match slot {
161+
Some(TargetRef::Object(id)) => Some(*id),
162+
_ => None,
163+
})
164+
})
165+
}
166+
135167
pub fn effects(&self) -> Vec<&'a Effect> {
136168
// If we're casting/activating, get effects from the source object
137169
match &self.candidate.action {

crates/phase-ai/src/policies/evasion_removal_priority.rs

Lines changed: 190 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,8 @@ mod tests {
215215
use engine::game::scenario::{GameScenario, P0};
216216
use engine::game::zones::create_object;
217217
use engine::types::ability::{
218-
AbilityDefinition, AbilityKind, Effect, EffectKind, PtValue, QuantityExpr, ResolvedAbility,
219-
TargetFilter, TargetRef, TypedFilter,
218+
AbilityDefinition, AbilityKind, DamageSource, Effect, EffectKind, PtValue, QuantityExpr,
219+
ResolvedAbility, TargetFilter, TargetRef, TypedFilter,
220220
};
221221
use engine::types::format::FormatConfig;
222222
use engine::types::game_state::{
@@ -573,6 +573,194 @@ mod tests {
573573
);
574574
}
575575

576+
/// Self-Destruct-style `DamageSource::Target` regression guard for #6582.
577+
///
578+
/// `Self-Destruct` ("Target creature you control deals X damage to any other
579+
/// target and X damage to itself, where X is its power") parses its two
580+
/// damage effects with `DamageSource::Target` — the *targeted creature* is
581+
/// the damage source, not the spell. The removal-lethality term must resolve
582+
/// that source (from the already-chosen first target) so the recipient slot
583+
/// is scored by whether the damage actually destroys the body, exactly as the
584+
/// #6582 fix does for default-sourced burn.
585+
///
586+
/// Before the fix, `removal_lethality` returned `Unresolved` for
587+
/// `DamageSource::Target`, so lethality was inert and the AI ranked the
588+
/// recipient purely by threat value — repeating the #6582 misplay (pointing
589+
/// non-lethal damage at the biggest body it cannot kill) for
590+
/// `Self-Destruct`-style spells. This test drives the PRODUCTION path — a
591+
/// real Self-Destruct cast through `TargetSelection`, then the registered
592+
/// `EvasionRemovalPriorityPolicy` verdict and the `lethality_bonus` it feeds
593+
/// — and asserts the corrected preference for the killable body, pinning the
594+
/// fix to observable behaviour.
595+
#[test]
596+
fn target_sourced_damage_prefers_the_killable_body() {
597+
const SELF_DESTRUCT_ORACLE: &str =
598+
"Target creature you control deals X damage to any other target and X damage to itself, where X is its power.";
599+
600+
let mut scenario = GameScenario::new_n_player(2, 42);
601+
scenario.at_phase(Phase::PreCombatMain);
602+
let self_destruct = scenario
603+
.add_spell_to_hand_from_oracle(P0, "Self-Destruct", true, SELF_DESTRUCT_ORACLE)
604+
.with_mana_cost(ManaCost::Cost {
605+
shards: vec![ManaCostShard::Red],
606+
generic: 1,
607+
})
608+
.id();
609+
// The damage source: the AI's own 2/2, the only "creature you control"
610+
// (and therefore the forced slot-1 target). Its power (2) is the damage
611+
// amount, so 2 damage reaches the recipient.
612+
let bird = scenario.add_creature(P0, "Bird", 2, 2).id();
613+
// The killable recipient — 2 damage destroys a 2/2. Low threat.
614+
let killable = scenario
615+
.add_creature(PlayerId(1), "Scrappy Skirmisher", 1, 2)
616+
.id();
617+
// The unkillable high-threat recipient — 2 damage cannot destroy a 3/3.
618+
let unkillable = scenario
619+
.add_creature(PlayerId(1), "Cloud of Darkness", 3, 3)
620+
.id();
621+
scenario.with_mana_pool(
622+
P0,
623+
vec![ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![])],
624+
);
625+
626+
let mut runner = scenario.build();
627+
let card_id = runner.state().objects[&self_destruct].card_id;
628+
runner
629+
.act(GameAction::CastSpell {
630+
object_id: self_destruct,
631+
card_id,
632+
targets: Vec::new(),
633+
payment_mode: CastPaymentMode::Auto,
634+
})
635+
.expect("the real Self-Destruct fixture should reach target selection");
636+
637+
// Self-Destruct has two target slots: slot 1 is "creature you control"
638+
// (the source; here the forced Bird), slot 2 is "any other target" (the
639+
// recipient — where the non-lethal-vs-lethal decision actually lives).
640+
// Drive the runner through slot 1 so `ResolvedAbility.targets` carries the
641+
// already-chosen source and the decision context is at the recipient slot.
642+
let first_slot = match &runner.state().waiting_for {
643+
WaitingFor::TargetSelection { target_slots, .. } => &target_slots[0],
644+
other => panic!("expected Self-Destruct target selection, got {other:?}"),
645+
};
646+
assert!(
647+
first_slot.legal_targets.contains(&TargetRef::Object(bird)),
648+
"slot 1 (creature you control) must legally be the Bird source"
649+
);
650+
runner
651+
.act(GameAction::ChooseTarget {
652+
target: Some(TargetRef::Object(bird)),
653+
})
654+
.expect("choosing the Bird for slot 1 should advance to the recipient slot");
655+
656+
let (pending_cast, target_slots, selection) = match &runner.state().waiting_for {
657+
WaitingFor::TargetSelection {
658+
pending_cast,
659+
target_slots,
660+
selection,
661+
..
662+
} => (pending_cast, target_slots, selection),
663+
other => panic!("expected Self-Destruct recipient slot, got {other:?}"),
664+
};
665+
let effects = crate::policies::context::collect_ability_effects(&pending_cast.ability);
666+
assert!(
667+
effects.iter().any(|effect| matches!(
668+
effect,
669+
Effect::DealDamage {
670+
damage_source: Some(DamageSource::Target),
671+
..
672+
}
673+
)),
674+
"reach guard: Self-Destruct must parse as DamageSource::Target damage"
675+
);
676+
assert!(
677+
effects
678+
.iter()
679+
.all(|effect| !matches!(effect, Effect::Unimplemented { .. })),
680+
"the regression fixture must not silently drop an unsupported clause"
681+
);
682+
// The source (Bird, power 2) is ALREADY chosen in slot 1 and locked into
683+
// the selection progress before the recipient slot is presented — so its
684+
// power, and hence the damage amount, is knowable during recipient choice.
685+
assert!(
686+
selection
687+
.selected_slots
688+
.first()
689+
.is_some_and(|slot| *slot == Some(TargetRef::Object(bird))),
690+
"the Bird source must already be chosen (selected_slots[0]) before the \
691+
recipient slot, so its power is knowable"
692+
);
693+
// The recipient slot (slot 2, "any other target") offers both the killable
694+
// 2/2 and the unkillable 3/3.
695+
assert!(target_slots
696+
.iter()
697+
.any(|slot| slot.legal_targets.contains(&TargetRef::Object(killable))));
698+
assert!(target_slots
699+
.iter()
700+
.any(|slot| slot.legal_targets.contains(&TargetRef::Object(unkillable))));
701+
702+
let state = runner.state();
703+
let decision = build_decision_context(state);
704+
let config = create_config(AiDifficulty::VeryHard, Platform::Native).into_measurement(42);
705+
706+
// The #6582 fix now covers `DamageSource::Target`: the lethality term
707+
// resolves the source (the already-chosen Bird, power 2) and scores a
708+
// recipient by whether that damage destroys it. So the registered removal
709+
// policy must rank the KILLABLE 2/2 above the unkillable 3/3 the 2 damage
710+
// cannot destroy — the exact #6582 preference, now extended to
711+
// Self-Destruct-style spells.
712+
let killable_delta = registry_delta(state, &decision, killable, &config);
713+
let unkillable_delta = registry_delta(state, &decision, unkillable, &config);
714+
assert!(
715+
killable_delta > unkillable_delta,
716+
"Self-Destruct recipient ranking must prefer the body the 2 damage kills \
717+
(killable 2/2) over the 3/3 it only tickles: \
718+
killable 2/2={killable_delta}, unkillable 3/3={unkillable_delta}"
719+
);
720+
721+
// And pin the underlying signal directly: the 2-damage source is provably
722+
// lethal to the 2/2 (+LETHAL_BONUS) and non-lethal to the 3/3 (a negative
723+
// waste penalty). This is the exact arithmetic the #6582 fix added for
724+
// default-sourced burn, now extended to the resolved `DamageSource::Target`
725+
// source.
726+
let state_ref = runner.state();
727+
let kil_obj = state_ref.objects.get(&killable).unwrap();
728+
let unk_obj = state_ref.objects.get(&unkillable).unwrap();
729+
let aicontext = crate::context::AiContext::empty(&config.weights);
730+
731+
let lethal_bonus_for =
732+
|target: ObjectId, target_obj: &engine::game::game_object::GameObject| {
733+
let candidate = CandidateAction {
734+
action: GameAction::ChooseTarget {
735+
target: Some(TargetRef::Object(target)),
736+
},
737+
metadata: ActionMetadata::for_actor(Some(P0), TacticalClass::Target),
738+
};
739+
let ctx = crate::policies::context::PolicyContext {
740+
state: state_ref,
741+
decision: &decision,
742+
candidate: &candidate,
743+
ai_player: P0,
744+
config: &config,
745+
context: &aicontext,
746+
cast_facts: None,
747+
search_depth: crate::policies::context::SearchDepth::Root,
748+
};
749+
crate::policies::removal_lethality::lethality_bonus(&ctx, target, target_obj)
750+
};
751+
752+
let killable_bonus = lethal_bonus_for(killable, kil_obj);
753+
let unkillable_bonus = lethal_bonus_for(unkillable, unk_obj);
754+
assert!(
755+
(killable_bonus - crate::policies::removal_lethality::LETHAL_BONUS).abs() < 1e-9,
756+
"the 2-damage source must read as a clean kill on the 2/2, got {killable_bonus}"
757+
);
758+
assert!(
759+
unkillable_bonus < 0.0,
760+
"the 2-damage source must read as a wasted non-lethal on the 3/3, got {unkillable_bonus}"
761+
);
762+
}
763+
576764
#[test]
577765
fn activated_removal_weights_controller_threat_but_beneficial_activation_is_neutral() {
578766
let destroy = Effect::Destroy {

crates/phase-ai/src/policies/removal_lethality.rs

Lines changed: 69 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@
4646
4747
use engine::game::game_object::GameObject;
4848
use engine::game::keywords::object_has_effective_keyword_kind;
49-
use engine::game::quantity::resolve_quantity;
50-
use engine::types::ability::{DamageSource, Effect};
49+
use engine::game::quantity::{resolve_quantity, resolve_quantity_with_targets_slice};
50+
use engine::types::ability::{DamageSource, Effect, TargetRef};
5151
use engine::types::card_type::CoreType;
5252
use engine::types::identifiers::ObjectId;
5353
use engine::types::keywords::{Keyword, KeywordKind};
@@ -77,20 +77,44 @@ enum EffectDamageSource {
7777
Object(ObjectId),
7878
/// The source depends on information this policy does not have yet:
7979
///
80-
/// * [`DamageSource::Target`] — the first object target *is* the source and
81-
/// is excluded from the recipient slice
82-
/// (`effects::deal_damage::resolve_effect_recipients`), so the object
83-
/// being scored may be the source rather than a recipient.
8480
/// * [`DamageSource::EachTarget`] — every leading target is an independent
8581
/// source with its own keywords and its own re-resolved amount.
8682
/// * [`DamageSource::TriggeringSource`] — bound to the triggering event's
8783
/// object; the engine's `targeting::extract_source_from_event` authority
8884
/// is crate-private, and re-deriving that mapping in the AI layer would
8985
/// duplicate engine logic.
86+
///
87+
/// [`DamageSource::Target`] is NOT here: its source is the first *already
88+
/// chosen* object target, which the policy can resolve from the in-flight
89+
/// selection, so it resolves to `Object`.
9090
Unresolved,
9191
}
9292

9393
/// CR 120.3: resolve which object deals one `DealDamage` effect's damage.
94+
///
95+
/// `Target` (CR 120.1 + CR 120.3: "that creature deals damage...") has its
96+
/// source bound to the FIRST object target of the ability — the creature chosen
97+
/// in the leading slot, not the spell. That selection is committed to the
98+
/// ongoing `TargetSelectionProgress` *before* the later (recipient) slots are
99+
/// offered, so while a recipient is being chosen the source is already knowable
100+
/// and the lethality of the damage it will deal can be computed (its power for
101+
/// the amount, plus wither/infect/deathtouch from its keywords). Resolving it
102+
/// here is what lets the #6582 lethality term cover `Self-Destruct`-style
103+
/// spells.
104+
///
105+
/// When the first target has not been chosen yet (e.g. the very first slot of a
106+
/// `DamageSource::Target` spell, or a target the engine has not exposed), the
107+
/// result is `Unresolved` and the caller stays neutral rather than guessing.
108+
///
109+
/// SCOPE: this resolves the source only on the ordinary cast/activation
110+
/// `TargetSelection` path. A `DamageSource::Target` effect reached from a
111+
/// triggered ability (`TriggerTargetSelection`, event-bound source per CR 120.7)
112+
/// or from the bulk `MultiTargetSelection` flow stays `Unresolved` — that source
113+
/// is not resolvable from a single recipient slot — so those card classes are
114+
/// not ranked by this term (not a regression; `Target` was always `Unresolved`
115+
/// before). The boundary is stated here (and on
116+
/// [`PolicyContext::first_selected_object_target`]) so the coverable surface is
117+
/// explicit rather than implied.
94118
fn effect_damage_source(
95119
ctx: &PolicyContext<'_>,
96120
damage_source: Option<&DamageSource>,
@@ -102,7 +126,17 @@ fn effect_damage_source(
102126
.map_or(EffectDamageSource::Unresolved, |object| {
103127
EffectDamageSource::Object(object.id)
104128
}),
105-
Some(DamageSource::Target | DamageSource::EachTarget | DamageSource::TriggeringSource) => {
129+
// CR 120.1 + CR 120.3: "Target creature deals X damage to ..." — the first
130+
// resolved object target is the damage source (see deal_damage.rs, which
131+
// binds `targets[0]` as the source and damages `targets[1..]`). Resolve it
132+
// from the already-chosen leading slot so its power/keywords are known.
133+
Some(DamageSource::Target) => ctx
134+
.first_selected_object_target()
135+
.map_or(EffectDamageSource::Unresolved, EffectDamageSource::Object),
136+
// CR 120.1: multi-source batches (EachTarget: every leading target is an
137+
// independent source) and event-bound sources are not resolvable from the
138+
// recipient slot alone.
139+
Some(DamageSource::EachTarget | DamageSource::TriggeringSource) => {
106140
EffectDamageSource::Unresolved
107141
}
108142
}
@@ -172,10 +206,34 @@ pub(crate) fn pending_damage_to_object(
172206
return PendingDamage::Unresolved;
173207
};
174208
found = true;
175-
let dealt = u32::try_from(
176-
resolve_quantity(ctx.state, amount, ctx.ai_player, source_id).max(0),
177-
)
178-
.unwrap_or(u32::MAX);
209+
// CR 608.2h + CR 208.1: the damage amount for a
210+
// `DamageSource::Target` effect ("target creature deals X damage,
211+
// where X is its power") reads the SOURCE creature's current power
212+
// when the effect resolves. That is `targets[0]` at resolution
213+
// (`deal_damage.rs` binds the first object target as the source and
214+
// damages `targets[1..]`), so mirror the engine by resolving the
215+
// amount against a targets slice whose first entry is the source.
216+
// Without this, a `Power { scope: Target }` amount reads an empty
217+
// targets list and resolves to 0 — silently scoring a Self-Destruct
218+
// as dealing no damage at all.
219+
let dealt = if matches!(damage_source, Some(DamageSource::Target)) {
220+
u32::try_from(
221+
resolve_quantity_with_targets_slice(
222+
ctx.state,
223+
amount,
224+
ctx.ai_player,
225+
source_id,
226+
&[TargetRef::Object(source_id)],
227+
)
228+
.max(0),
229+
)
230+
.unwrap_or(u32::MAX)
231+
} else {
232+
u32::try_from(
233+
resolve_quantity(ctx.state, amount, ctx.ai_player, source_id).max(0),
234+
)
235+
.unwrap_or(u32::MAX)
236+
};
179237
// CR 120.3d + CR 702.80a + CR 702.90c: wither/infect damage to a
180238
// creature is dealt as -1/-1 counters and is never marked.
181239
if is_creature

0 commit comments

Comments
 (0)