From 860ba1ad07b7943226ef22f039539e44621816d6 Mon Sep 17 00:00:00 2001 From: mike-theDude Date: Wed, 12 Aug 2026 22:58:54 -0500 Subject: [PATCH 1/7] fix(ai): require a trampling gang to absorb enough to avert lethal (#7183) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. The `gang_stabilize` gate claimed guard-for-guard parity with the single-blocker chump pass, but was missing that pass's trample term. The single pass computes `damage_prevented = blocker_toughness` for a trampler; the gang gate used raw `attacker_power` pressure with no absorption check. CR 702.19b: a trampling attacker assigns only lethal damage — the blockers' toughness — to the creatures blocking it and assigns the excess to the player. So a floor-sized gang that leaves a still-lethal residual spends every creature in it and the player dies anyway. `block_is_futile` catches this for a lone attacker only: its absorption bound is optimistic and board-wide, while this gang sees only the blockers the earlier passes left behind. Non-tramplers need no such term — once blocked they assign nothing to the player whatever happens to the blockers (CR 510.1c), which is exactly what makes the doomed-block route in the previous commit a full save. Adds a two-attacker regression: a 6/6 is consumed blocking a 5/5, leaving the menace trampler a gang of two 1/1s that absorbs 2 of 11 at 6 life. Reverting the term reproduces the bad declaration (both 1/1s assigned, player still takes 9 and dies), so the test is discriminating. Co-Authored-By: Claude Opus 5 (1M context) --- crates/phase-ai/src/combat_ai.rs | 54 ++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/crates/phase-ai/src/combat_ai.rs b/crates/phase-ai/src/combat_ai.rs index 4798c7022a..a1518307bb 100644 --- a/crates/phase-ai/src/combat_ai.rs +++ b/crates/phase-ai/src/combat_ai.rs @@ -1157,7 +1157,24 @@ pub fn choose_blockers_with_profile( .get(bid) .is_some_and(has_damage_reflection_to_controller) }); + // CR 702.19b: the trample term the single-blocker pass carries as + // `damage_prevented = blocker_toughness`, without which the "guard-for-guard" + // parity claimed above is false for tramplers. A trampling attacker assigns + // only lethal damage (the blockers' toughness) to the gang and tramples the + // excess to the player, so the gang has to absorb enough to actually avert + // the kill — a floor-sized gang that leaves a still-lethal residual spends + // every creature in it and the player dies anyway. `block_is_futile` catches + // this for a lone attacker only: its absorption bound is optimistic and + // board-wide, while this gang sees just the blockers earlier passes left. + // + // Non-tramplers need no such term: once blocked they assign nothing to the + // player whatever happens to the blockers (CR 510.1c), which is what makes + // the doomed block above a full save. + let trample_gang_averts_lethal = !attacker_has_trample + || (gang_toughness >= 2 + && attacker_power.saturating_sub(gang_toughness) < effective_life); let gang_stabilize = floor_stabilize_route + && trample_gang_averts_lethal && !gang_reflects_damage && !commander_chump_unsafe(state, player, attacker_id, gang_toughness); @@ -4656,4 +4673,41 @@ mod tests { CR 702.2c), so the block saves nothing. Got {assignments:?}" ); } + + /// CR 702.19b: the same boundary without deathtouch, which reaches the + /// `gang_stabilize` gate rather than exiting at the deathtouch skip. Needs two + /// attackers to be reachable: `block_is_futile` bounds absorption optimistically + /// over the whole board, so a lone trampler whose gang cannot absorb is caught + /// there — but a gang built from what earlier passes left over is not. + /// + /// The 6/6 is consumed blocking the 5/5, leaving the menace trampler a + /// floor-sized gang of two 1/1s. That gang absorbs 2 of 11, so the player takes + /// 9 at 6 life and dies either way: the block is a pure loss of both creatures. + #[test] + fn floored_trampler_is_not_chump_ganged_when_the_gang_cannot_absorb_lethal() { + let mut state = setup(); + state.players[1].life = 6; + let plain = add_creature(&mut state, PlayerId(0), "Plain", 5, 5, vec![]); + let trampler = add_creature( + &mut state, + PlayerId(0), + "Trampler", + 11, + 11, + vec![Keyword::Menace, Keyword::Trample], + ); + add_creature(&mut state, PlayerId(1), "Big", 6, 6, vec![]); + add_creature(&mut state, PlayerId(1), "Small 0", 1, 1, vec![]); + add_creature(&mut state, PlayerId(1), "Small 1", 1, 1, vec![]); + + let assignments = choose_blockers(&state, PlayerId(1), &[plain, trampler]); + + let on_trampler = assignments.iter().filter(|&&(_, a)| a == trampler).count(); + assert_eq!( + on_trampler, 0, + "CR 702.19b: a floor-sized gang absorbing 2 of 11 leaves a lethal residual \ + at 6 life, so the trampler must not be chump-ganged — the creatures are \ + spent and the player dies anyway. Got {assignments:?}" + ); + } } From e86496d4cd84dc5672e81cdfa7021fe8c091e440 Mon Sep 17 00:00:00 2001 From: mike-theDude Date: Thu, 13 Aug 2026 02:24:38 -0500 Subject: [PATCH 2/7] fix(ai): size the survival gang by real trample absorption (#7183) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up from @matthewevans and CodeRabbit on the current head. Two independent defects in the trample-absorption gate, both now covered. **1. The survival gang drew from the wrong pool.** It reused `effective_candidates`, which removes blockers the attacker kills in the first-strike step (CR 702.7b). That filter is right for estimating a *kill* — a blocker dead before its damage step deals none — and wrong for absorption: CR 702.19b requires a trampling attacker to assign each blocker its lethal damage before any excess is assigned to the player, whether or not that blocker survives. An 11/11 menace first-strike trampler against two 4/4s at 4 life assigns 4+4 and tramples 3, leaving the player alive at 1 — but the filter emptied the pool and no legal gang could be formed at all. The survival gang now draws from every legal blocker (`gang_candidates`) and grows cheapest-first only until the residual is survivable, so it spends the fewest creatures that actually save the game. `effective_candidates` still backs the kill claim, unchanged. **2. Absorption summed raw toughness.** CR 702.19b says to "take into account damage already marked on the creature", and CR 702.2c makes 1 damage lethal from a deathtouch source. Raw toughness therefore over-counts absorption and could approve a gang that leaves lethal trample damage. Absorption now goes through `combat_damage::lethal_damage_needed` — the resolver's own authority, which subtracts marked damage and returns 1 under deathtouch — exposed as `pub` for this purpose rather than re-derived. **Consequence worth calling out:** the previous revision refused every deathtouch trampler outright via `!attacker_has_trample`. That is wrong — it declines blocks that save the game. With absorption computed properly, a deathtouch trampler is gated by the arithmetic instead: ten 4/4s against an 11/11 deathtouch trampler at 6 life now block with exactly six (absorbing 6, surviving at 1) rather than declining and dying. The earlier `menace_deathtouch_trampler_is_not_chump_ganged` test asserted that wrong behavior and has been replaced by the two cases that bracket the real boundary. Tests (5, each verified discriminating): - `menace_first_strike_trampler_is_gang_blocked_by_doomed_absorbers` — the case from the review; reverting the pool to `effective_candidates` fails it - `marked_damage_lowers_absorption_below_the_survival_threshold` — 4/4s with 3 marked absorb 1 each; reverting to raw toughness fails it - `menace_deathtouch_trampler_is_ganged_only_as_far_as_survival_needs` — blocks with exactly 6, not the whole board - `menace_deathtouch_trampler_is_declined_when_absorption_cannot_save` — 3 blockers absorb 3 of 11 at 6 life, declined. `block_is_futile` does not catch this: it bounds absorption by raw toughness and believes the board survives - `floored_trampler_is_not_chump_ganged_when_the_gang_cannot_absorb_lethal` — retained from the previous head Reverting the candidate-pool fix fails 3 tests; reverting the absorption fix fails 3 others; the two are independent. Verification: `cargo fmt --all` clean, `cargo clippy -p phase-ai -p phase-engine --all-targets` clean, `cargo test -p phase-ai` 2180 passed / 0 failed, `cargo test -p phase-engine --lib combat_damage` 165 passed / 0 failed. Co-Authored-By: Claude Opus 5 (1M context) --- crates/engine/src/game/combat_damage.rs | 8 +- crates/phase-ai/src/combat_ai.rs | 233 ++++++++++++++++++------ 2 files changed, 189 insertions(+), 52 deletions(-) diff --git a/crates/engine/src/game/combat_damage.rs b/crates/engine/src/game/combat_damage.rs index bb19f9b76d..3519815ef6 100644 --- a/crates/engine/src/game/combat_damage.rs +++ b/crates/engine/src/game/combat_damage.rs @@ -1089,7 +1089,13 @@ fn assign_attacker_damage( /// How much damage is needed to kill this creature. /// CR 702.2c: Deathtouch — any amount of damage from a deathtouch source is lethal. -fn lethal_damage_needed( +/// +/// Public because this is the single authority for "how much does this blocker +/// absorb": CR 702.19b requires a trampling attacker to assign this much to each +/// blocker before any excess is assigned to the player, so a consumer estimating +/// trample absorption must read the same number this resolver assigns rather than +/// re-deriving it from raw toughness and silently ignoring damage already marked. +pub fn lethal_damage_needed( state: &GameState, object_id: ObjectId, source_has_deathtouch: bool, diff --git a/crates/phase-ai/src/combat_ai.rs b/crates/phase-ai/src/combat_ai.rs index a1518307bb..f62a884c96 100644 --- a/crates/phase-ai/src/combat_ai.rs +++ b/crates/phase-ai/src/combat_ai.rs @@ -1045,7 +1045,7 @@ pub fn choose_blockers_with_profile( // is "is this trade worth it", wrong when the question is "do I survive". // They may only decline a block that is not the difference between living // and losing (issue #7183). - let doomed_block_still_saves = floor_stabilize_route && !attacker_has_trample; + let survival_route_is_live = floor_stabilize_route; // CR 702.7b: If attacker has first strike and blocker doesn't, the blocker // dies before dealing damage. Skip blockers that would die to first strike — @@ -1075,7 +1075,7 @@ pub fn choose_blockers_with_profile( // gang-blocking just loses more creatures — unless the block is the // player's only route to surviving the turn, where CR 510.1c makes the // doomed block a full save anyway. - if attacker_has_deathtouch && !doomed_block_still_saves { + if attacker_has_deathtouch && !survival_route_is_live { continue; } @@ -1109,7 +1109,7 @@ pub fn choose_blockers_with_profile( .find(|(bid, _, _)| !gang_set.contains(bid)) .copied() .or_else(|| { - if !doomed_block_still_saves { + if !survival_route_is_live { return None; } // Contributes no damage, so it is added with zero power: it pads @@ -1127,6 +1127,61 @@ pub fn choose_blockers_with_profile( gang_value += value; } + // CR 702.19b: how much a gang actually absorbs. A trampling attacker must + // assign each blocker its lethal damage before any excess is assigned to the + // player, so absorption is the sum of those per-blocker minimums — NOT raw + // toughness. `lethal_damage_needed` is the resolver's own authority: it + // subtracts damage already marked, and returns 1 under deathtouch (CR + // 702.2c), which is why a deathtouch trampler absorbs almost nothing however + // large the gang. Summing raw toughness here approved gangs that left lethal + // trample damage against already-damaged blockers. + let gang_absorption = |set: &[ObjectId]| -> i32 { + set.iter() + .map(|&bid| { + engine::game::combat_damage::lethal_damage_needed( + state, + bid, + attacker_has_deathtouch, + ) as i32 + }) + .sum() + }; + // CR 510.1c + CR 702.19b: only trample can put damage past a legal block, so + // a nontrampler is unconditionally survivable once blocked. The `>= 2` floor + // mirrors the single-blocker pass's `damage_prevented >= 2`. + let averts_lethal = |set: &[ObjectId]| -> bool { + if !attacker_has_trample { + return true; + } + let absorbed = gang_absorption(set); + absorbed >= 2 && attacker_power.saturating_sub(absorbed) < effective_life + }; + + // CR 509.1b + CR 510.1c + CR 702.7b + CR 702.19b: the survival gang answers a + // different question from the kill gang, so it draws from a different pool. + // EVERY legal blocker absorbs, including one the attacker kills in the + // first-strike step — trample still has to assign that blocker its lethal + // damage before excess reaches the player, and a nontrampler assigns nothing + // to the player once blocked whatever becomes of its blockers. Filtering the + // first-strike casualties out is right for a kill estimate and wrong here: it + // left an 11/11 menace first-strike trampler unblockable by two 4/4s at 4 + // life, though that block absorbs 4+4 and tramples only 3. + // + // Grows cheapest-first past the floor until the residual is survivable, so it + // spends the fewest creatures that actually save the game. + let survival_gang: Option> = if floor_stabilize_route { + let mut set: Vec = Vec::new(); + for &(bid, _, _) in &gang_candidates { + if set.len() >= needed_blockers && averts_lethal(&set) { + break; + } + set.push(bid); + } + (set.len() >= needed_blockers && averts_lethal(&set)).then_some(set) + } else { + None + }; + // CR 509.1b + CR 704.5a: Survival override, restricted to attackers that a // minimum-blocker floor routes here. Such an attacker is skipped by BOTH // single-blocker chump passes, which are the only places carrying a @@ -1143,53 +1198,44 @@ pub fn choose_blockers_with_profile( // Mirrors the single-blocker pass guard-for-guard: the same // `effective_life <= attacker_power * 3` threshold with the CR 903.10a // commander-damage tightening, the same `commander_chump_unsafe` rejection - // (evaluated against the gang's combined toughness, since trample assigns - // lethal damage to every blocker before any tramples through, CR 702.19b), - // and the same exclusion of damage-reflection blockers, which hand the - // attacker's power straight back to the player and so save nothing. - let gang_toughness: i32 = gang_set - .iter() - .filter_map(|bid| state.objects.get(bid).and_then(|b| b.toughness)) - .sum(); - let gang_reflects_damage = gang_set.iter().any(|bid| { - state - .objects - .get(bid) - .is_some_and(has_damage_reflection_to_controller) - }); - // CR 702.19b: the trample term the single-blocker pass carries as - // `damage_prevented = blocker_toughness`, without which the "guard-for-guard" - // parity claimed above is false for tramplers. A trampling attacker assigns - // only lethal damage (the blockers' toughness) to the gang and tramples the - // excess to the player, so the gang has to absorb enough to actually avert - // the kill — a floor-sized gang that leaves a still-lethal residual spends - // every creature in it and the player dies anyway. `block_is_futile` catches - // this for a lone attacker only: its absorption bound is optimistic and - // board-wide, while this gang sees just the blockers earlier passes left. - // - // Non-tramplers need no such term: once blocked they assign nothing to the - // player whatever happens to the blockers (CR 510.1c), which is what makes - // the doomed block above a full save. - let trample_gang_averts_lethal = !attacker_has_trample - || (gang_toughness >= 2 - && attacker_power.saturating_sub(gang_toughness) < effective_life); - let gang_stabilize = floor_stabilize_route - && trample_gang_averts_lethal - && !gang_reflects_damage - && !commander_chump_unsafe(state, player, attacker_id, gang_toughness); + // (evaluated against the gang's absorption, since trample assigns lethal + // damage to every blocker before any tramples through, CR 702.19b), and the + // same exclusion of damage-reflection blockers, which hand the attacker's + // power straight back to the player and so save nothing. + let reflects_damage = |set: &[ObjectId]| -> bool { + set.iter().any(|bid| { + state + .objects + .get(bid) + .is_some_and(has_damage_reflection_to_controller) + }) + }; // CR 702.2c: never gang a deathtouch attacker for *value* — every blocker // assigned any damage dies, so the kill is paid for with the whole gang. // Preserves the pre-existing skip for deathtouch attackers now that the - // survival route above no longer short-circuits them out of the pass. + // survival route no longer short-circuits them out of the pass. let gang_kills_for_value = !attacker_has_deathtouch && combined_power >= attacker_toughness - && gang_value <= attacker_value; + && gang_value <= attacker_value + && gang_set.len() >= needed_blockers + && !reflects_damage(&gang_set); + + // The survival route already proved its own set legal and lethal-averting; + // it only remains to reject the two shapes that save nothing. + let stabilizing_gang = survival_gang.filter(|set| { + !reflects_damage(set) + && !commander_chump_unsafe(state, player, attacker_id, gang_absorption(set)) + }); - // Only gang-block if combined power can kill AND total value risked <= - // attacker value — or if declining loses the game outright. Never below the - // CR 509.1b floor, which would make the declaration illegal. - if gang_set.len() >= needed_blockers && (gang_kills_for_value || gang_stabilize) { + // Gang-block to kill when the trade is worth it, else to survive. Never below + // the CR 509.1b floor, which would make the declaration illegal. + let declared_gang = if gang_kills_for_value { + Some(gang_set) + } else { + stabilizing_gang + }; + if let Some(gang_set) = declared_gang { for bid in gang_set { assignments.push((bid, attacker_id)); used_blockers.insert(bid); @@ -4651,13 +4697,17 @@ mod tests { ); } - /// CR 702.19b + CR 702.2c: trample is the boundary of the CR 510.1c argument. - /// A deathtouch trampler assigns only 1 damage per blocker as "lethal" and - /// tramples the remaining 8 through to a player at 6 life, so the gang block - /// saves nothing and must not be thrown away. Guards the `!attacker_has_trample` - /// term — dropping it makes the AI chump three creatures and still lose. + /// CR 702.19b + CR 702.2c: a deathtouch trampler assigns only 1 damage per + /// blocker as "lethal", so each blocker absorbs 1 and the rest tramples through. + /// With ten 4/4s available against an 11/11 at 6 life the AI must still block — + /// six blockers absorb 6 and leave 5, surviving at 1 — and must spend exactly + /// the six that achieve it rather than the whole board. + /// + /// The absorption is what decides, not the trample keyword: an earlier revision + /// refused every deathtouch trampler outright, which declines a block that + /// saves the game. #[test] - fn menace_deathtouch_trampler_is_not_chump_ganged_at_lethal() { + fn menace_deathtouch_trampler_is_ganged_only_as_far_as_survival_needs() { let (state, attacker, _) = lethal_attacker_board_with( 6, 10, @@ -4666,11 +4716,92 @@ mod tests { let assignments = choose_blockers(&state, PlayerId(1), &[attacker]); + let on_attacker = assignments.iter().filter(|&&(_, a)| a == attacker).count(); + assert_eq!( + on_attacker, 6, + "CR 702.2c: deathtouch makes 1 damage lethal, so N blockers absorb N. \ + At 6 life facing 11 power the AI needs 6 (residual 5) and must not \ + spend more. Got {assignments:?}" + ); + } + + /// The other side of that boundary: when absorption cannot get under the life + /// total the block is a pure loss and must be declined. Three 4/4s absorb only + /// 3 against a deathtouch trampler, leaving 8 against 6 life. + /// + /// Note `block_is_futile` does not catch this — it bounds absorption by raw + /// toughness (12 here) and so believes the board survives. The gang gate's + /// `lethal_damage_needed` accounting is what declines it. + #[test] + fn menace_deathtouch_trampler_is_declined_when_absorption_cannot_save() { + let (state, attacker, _) = lethal_attacker_board_with( + 6, + 3, + vec![Keyword::Menace, Keyword::Deathtouch, Keyword::Trample], + ); + + let assignments = choose_blockers(&state, PlayerId(1), &[attacker]); + + let on_attacker = assignments.iter().filter(|&&(_, a)| a == attacker).count(); + assert_eq!( + on_attacker, 0, + "three blockers absorb 3 under deathtouch, leaving 8 against 6 life — \ + the gang dies and the player still dies. Got {assignments:?}" + ); + } + + /// CR 702.7b + CR 702.19b: a first-striker kills these blockers before their + /// damage step, so they are excluded from the *kill* estimate — but trample must + /// still assign each of them its lethal damage before any excess reaches the + /// player. An 11/11 menace first-strike trampler against two 4/4s at 4 life + /// assigns 4+4 and tramples 3, leaving the player alive at 1. + /// + /// Regression for deriving the survival gang from `effective_candidates`: that + /// filter emptied the pool, so this legal, life-saving block could not be formed + /// at all. + #[test] + fn menace_first_strike_trampler_is_gang_blocked_by_doomed_absorbers() { + let (state, attacker, _) = lethal_attacker_board_with( + 4, + 2, + vec![Keyword::Menace, Keyword::FirstStrike, Keyword::Trample], + ); + + let assignments = choose_blockers(&state, PlayerId(1), &[attacker]); + + let on_attacker = assignments.iter().filter(|&&(_, a)| a == attacker).count(); + assert_eq!( + on_attacker, 2, + "CR 702.19b: blockers that die in the first-strike step still absorb \ + their lethal damage — 4+4 of 11 leaves 3 against 4 life. Got \ + {assignments:?}" + ); + } + + /// CR 702.19b: "take into account damage already marked on the creature" — a + /// 4/4 with 3 damage marked needs only 1 more to be lethal, so it absorbs 1, not + /// 4. Two of them absorb 2 against an 11/11 menace trampler, leaving 9 against + /// 4 life, and the block must be declined. + /// + /// Summing raw toughness reads this board as absorbing 8 and approves a gang + /// that leaves the player dead — the reason absorption goes through the + /// resolver's own `lethal_damage_needed` rather than the toughness field. + #[test] + fn marked_damage_lowers_absorption_below_the_survival_threshold() { + let (mut state, attacker, blockers) = + lethal_attacker_board_with(4, 2, vec![Keyword::Menace, Keyword::Trample]); + for &bid in &blockers { + state.objects.get_mut(&bid).unwrap().damage_marked = 3; + } + + let assignments = choose_blockers(&state, PlayerId(1), &[attacker]); + let on_attacker = assignments.iter().filter(|&&(_, a)| a == attacker).count(); assert_eq!( on_attacker, 0, - "a deathtouch trampler tramples past a chump gang (1 lethal per blocker, \ - CR 702.2c), so the block saves nothing. Got {assignments:?}" + "CR 702.19b: damage already marked lowers each blocker's lethal minimum \ + to 1, so the gang absorbs 2 of 11 and the player still dies. Got \ + {assignments:?}" ); } From ce6c81e4ae8f22b8bb7fd8c120956684b666ce9f Mon Sep 17 00:00:00 2001 From: mike-theDude Date: Thu, 13 Aug 2026 14:36:52 -0500 Subject: [PATCH 3/7] refactor(engine, ai): one authority for combat damage reaching the defender (#7183) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third review round on this PR surfaced three more findings. All three were symptoms of the same thing: the AI carried its own models of combat damage assignment, and each had a different subset of the rules. block_is_futile raw toughness, single damage step commander_chump_unsafe own trample math, single blocker, deathtouch -> 1 averts_lethal (previous) lethal_damage_needed, single damage step Adds `combat_damage::combat_damage_to_defender(state, attacker, blockers)` — how much combat damage that attacker assigns to the defending player across the whole phase, given that block. One place, all the rules, because they interact: - CR 510.1a 0-or-less power assigns nothing - CR 510.1b empty block = unblocked, full power through - CR 510.1c a blocked creature assigns to its blockers, none to the player - CR 702.19b lethal to every blocker first, then excess — and nothing to the player if lethal is not assigned to all of them; counts marked damage - CR 702.19d a blocked trampler with NO blockers left assigns everything to the player, as though all had been assigned lethal damage - CR 702.4b first/double strike split combat into two damage steps - CR 702.7b blockers killed in the first step are gone for the second, and a first-striking blocker can kill the attacker before its regular step - CR 702.2c deathtouch makes 1 lethal Within a step, assignment resolves as the worst case for the defender, since the attacking player chooses (CR 702.19b): lethal goes to the cheapest blockers first, maximising deaths and so minimising absorption surviving into the second step. The three review findings, all now structural rather than patched: 1. **Double strike was counted as one step.** An 11/11 double-strike trampler against two 4/4s assigns 4+4 and tramples 3, emptying the block — then strikes again into nothing and sends all 11 through (CR 702.19d). 14 total. The old model said 3 and approved a block that loses the game at 4 life. 2. **`commander_chump_unsafe` double-applied deathtouch.** It replaced its toughness argument with 1 under deathtouch — correct for the single chump blocker it was written for, and it discarded a whole gang's absorption when handed one, rejecting legal blocks that do prevent commander lethality. It now takes the blocker set and delegates. 3. **Survival candidates were value-ordered.** Against a trampler a blocker contributes absorption, not cheapness, so the walk spent several 1/1s where one 6/6 closes the gap. Trample survival now ranks by resolver-derived absorption descending, value ascending for ties; the non-trample route keeps value order, where CR 510.1c makes any legal block a full save. Tests: 11 on the building block itself, over the rule interactions rather than through any consumer, plus 2 AI regressions — `menace_double_strike_trampler_is_declined_when_the_second_strike_still_kills` and `trample_survival_gang_prefers_absorbers_over_cheap_bodies`. Verification: cargo fmt --all clean; cargo clippy -p phase-ai -p phase-engine --all-targets clean; cargo test -p phase-ai 2182 passed / 0 failed; cargo test -p phase-engine 23827 passed / 0 failed. Every CR above was checked against docs/MagicCompRules.txt before annotation. Co-Authored-By: Claude Opus 5 (1M context) --- crates/engine/src/game/combat_damage.rs | 262 ++++++++++++++++++++++++ crates/phase-ai/src/combat_ai.rs | 177 ++++++++++------ 2 files changed, 382 insertions(+), 57 deletions(-) diff --git a/crates/engine/src/game/combat_damage.rs b/crates/engine/src/game/combat_damage.rs index 3519815ef6..02fd9ebfe4 100644 --- a/crates/engine/src/game/combat_damage.rs +++ b/crates/engine/src/game/combat_damage.rs @@ -1087,6 +1087,140 @@ fn assign_attacker_damage( } } +/// How much combat damage `attacker_id` assigns to the defending player over the +/// whole combat damage phase if exactly `blockers` block it. +/// +/// The single authority for "does this block save me". Consumers must not +/// re-derive it: every hand-rolled version of this calculation has gotten one of +/// the rules below wrong, because they interact. +/// +/// - CR 510.1a: a creature assigns damage equal to its power; 0 or less assigns none. +/// - CR 510.1b: an empty `blockers` slice means unblocked — all power to the player. +/// - CR 510.1c: a blocked creature assigns to its blockers, and none to the player. +/// - CR 702.19b: trample assigns lethal to every blocker first, then the excess to +/// the player — and *no* damage to the player if lethal is not assigned to all of +/// them. "Lethal" counts damage already marked (via `lethal_damage_needed`). +/// - CR 702.19d: a blocked trampler with no blockers left when damage is assigned +/// assigns everything to the player, as though all blockers had been assigned +/// lethal damage. This is what makes the second strike of a double striker so +/// dangerous: the first strike step usually empties the block. +/// - CR 702.4b + CR 702.7b: first and double strike split combat into two damage +/// steps. Blockers killed in the first step are gone for the second, and a +/// first-striking blocker can kill the attacker before its regular-step damage. +/// - CR 702.2c: deathtouch makes 1 damage lethal, so a deathtouch trampler is +/// absorbed by almost nothing. +/// +/// Assignment within a step is resolved as the worst case for the defending +/// player, since the attacking player chooses (CR 702.19b): lethal is assigned to +/// the cheapest blockers first, maximising how many die and therefore how little +/// absorption survives into the second step. +pub fn combat_damage_to_defender( + state: &GameState, + attacker_id: ObjectId, + blockers: &[ObjectId], +) -> i32 { + let Some(attacker) = state.objects.get(&attacker_id) else { + return 0; + }; + // CR 510.1a: a creature assigning 0 or less assigns no combat damage at all. + let power = attacker.power.unwrap_or(0).max(0); + if power == 0 { + return 0; + } + // CR 510.1b: unblocked — the whole power reaches the player. + if blockers.is_empty() { + return power; + } + + let has_trample = attacker.has_keyword(&Keyword::Trample); + let has_deathtouch = attacker.has_keyword(&Keyword::Deathtouch); + let has_double_strike = attacker.has_keyword(&Keyword::DoubleStrike); + let attacker_strikes_first = has_double_strike || attacker.has_keyword(&Keyword::FirstStrike); + + // Remaining lethal per blocker, in the order the attacker would exhaust them. + let mut remaining: Vec = blockers + .iter() + .map(|&bid| lethal_damage_needed(state, bid, has_deathtouch) as i32) + .collect(); + remaining.sort_unstable(); + + // CR 702.7b: which blockers assign their damage in the first-strike step. + let first_strike_blockers: Vec<&GameObject> = blockers + .iter() + .filter_map(|bid| state.objects.get(bid)) + .filter(|b| b.has_keyword(&Keyword::FirstStrike) || b.has_keyword(&Keyword::DoubleStrike)) + .collect(); + + let mut to_player = 0; + let mut attacker_alive = true; + + // CR 702.4b: the first combat damage step exists at all only if some + // participant has first or double strike. + if attacker_strikes_first || !first_strike_blockers.is_empty() { + if attacker_strikes_first { + to_player += assign_step_to_defender(&mut remaining, power, has_trample); + } + // CR 702.7b: first-striking blockers assign now, and can kill the attacker + // before it ever reaches the regular step. + let attacker_lethal = lethal_damage_needed( + state, + attacker_id, + first_strike_blockers + .iter() + .any(|b| b.has_keyword(&Keyword::Deathtouch)), + ) as i32; + let first_strike_damage: i32 = first_strike_blockers + .iter() + .map(|b| b.power.unwrap_or(0).max(0)) + .sum(); + if first_strike_damage >= attacker_lethal { + attacker_alive = false; + } + } + + // CR 702.4b: the attacker assigns in the regular step if it has double strike, + // or if it never had first strike and so has not assigned at all yet. + if attacker_alive && (has_double_strike || !attacker_strikes_first) { + to_player += assign_step_to_defender(&mut remaining, power, has_trample); + } + + to_player +} + +/// One combat damage step's worth of assignment from a single attacker, returning +/// the damage that reaches the defending player and consuming the blockers it +/// kills. See `combat_damage_to_defender` for the governing rules. +fn assign_step_to_defender(remaining: &mut Vec, power: i32, has_trample: bool) -> i32 { + if remaining.is_empty() { + // CR 702.19d: blocked, but nothing is blocking it now — a trampler assigns + // everything to the player. CR 510.1c: without trample it assigns nothing. + return if has_trample { power } else { 0 }; + } + + let total_lethal: i32 = remaining.iter().sum(); + + // CR 702.19b: excess reaches the player only once EVERY blocker has been + // assigned lethal damage. + if has_trample && power > total_lethal { + remaining.clear(); + return power - total_lethal; + } + + // Otherwise every point is spent on blockers. Kill the cheapest first — the + // attacking player chooses, and fewer survivors means less absorption in any + // later step. + let mut budget = power; + remaining.retain(|&lethal| { + if budget >= lethal { + budget -= lethal; + false + } else { + true + } + }); + 0 +} + /// How much damage is needed to kill this creature. /// CR 702.2c: Deathtouch — any amount of damage from a deathtouch source is lethal. /// @@ -4319,4 +4453,132 @@ mod tests { ); assert_eq!(state.objects[&attacker].damage_marked, 0); } + + // ── `combat_damage_to_defender` (CR 510.1 / 702.4b / 702.7b / 702.19b/d) ── + // + // The building block itself, exercised over the rule interactions rather than + // through any one consumer. Each case is a different pair of rules meeting. + + fn kw(state: &mut GameState, id: ObjectId, keywords: Vec) { + state.objects.get_mut(&id).unwrap().keywords = keywords; + } + + #[test] + fn unblocked_attacker_assigns_its_whole_power() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 7, 7); + // CR 510.1b + assert_eq!(combat_damage_to_defender(&state, a, &[]), 7); + } + + #[test] + fn blocked_nontrampler_assigns_nothing_to_the_player() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 11, 11); + let b = create_creature(&mut state, PlayerId(1), "B", 1, 1); + // CR 510.1c: even a 1/1 chump stops all 11. + assert_eq!(combat_damage_to_defender(&state, a, &[b]), 0); + } + + #[test] + fn trampler_assigns_the_excess_over_lethal_to_blockers() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 11, 11); + kw(&mut state, a, vec![Keyword::Trample]); + let b1 = create_creature(&mut state, PlayerId(1), "B1", 1, 4); + let b2 = create_creature(&mut state, PlayerId(1), "B2", 1, 4); + // CR 702.19b: 4 + 4 lethal, 3 excess. + assert_eq!(combat_damage_to_defender(&state, a, &[b1, b2]), 3); + } + + #[test] + fn trampler_assigns_nothing_when_it_cannot_cover_every_blocker() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 5, 5); + kw(&mut state, a, vec![Keyword::Trample]); + let b1 = create_creature(&mut state, PlayerId(1), "B1", 1, 4); + let b2 = create_creature(&mut state, PlayerId(1), "B2", 1, 4); + // CR 702.19b: "need not assign lethal damage to all those blocking creatures + // but in that case can't assign any damage to the player". + assert_eq!(combat_damage_to_defender(&state, a, &[b1, b2]), 0); + } + + #[test] + fn marked_damage_lowers_the_lethal_each_blocker_absorbs() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 11, 11); + kw(&mut state, a, vec![Keyword::Trample]); + let b1 = create_creature(&mut state, PlayerId(1), "B1", 1, 4); + let b2 = create_creature(&mut state, PlayerId(1), "B2", 1, 4); + state.objects.get_mut(&b1).unwrap().damage_marked = 3; + state.objects.get_mut(&b2).unwrap().damage_marked = 3; + // CR 702.19b: "take into account damage already marked" — 1 + 1, not 4 + 4. + assert_eq!(combat_damage_to_defender(&state, a, &[b1, b2]), 9); + } + + #[test] + fn deathtouch_trampler_needs_only_one_damage_per_blocker() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 11, 11); + kw(&mut state, a, vec![Keyword::Trample, Keyword::Deathtouch]); + let b1 = create_creature(&mut state, PlayerId(1), "B1", 1, 6); + let b2 = create_creature(&mut state, PlayerId(1), "B2", 1, 6); + // CR 702.2c: 1 each is lethal, so 9 tramples through despite 12 toughness. + assert_eq!(combat_damage_to_defender(&state, a, &[b1, b2]), 9); + } + + #[test] + fn double_strike_trampler_strikes_the_empty_block_again() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 11, 11); + kw(&mut state, a, vec![Keyword::DoubleStrike, Keyword::Trample]); + let b1 = create_creature(&mut state, PlayerId(1), "B1", 1, 4); + let b2 = create_creature(&mut state, PlayerId(1), "B2", 1, 4); + // CR 702.4b: first step assigns 4 + 4 lethal and tramples 3, emptying the + // block; CR 702.19d: the regular step then assigns all 11 to the player. + assert_eq!(combat_damage_to_defender(&state, a, &[b1, b2]), 14); + } + + #[test] + fn double_strike_without_trample_still_assigns_nothing_to_the_player() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 11, 11); + kw(&mut state, a, vec![Keyword::DoubleStrike]); + let b = create_creature(&mut state, PlayerId(1), "B", 1, 1); + // CR 510.1c: the second strike has no blockers and no trample, so no damage + // is assigned at all — the distinction CR 702.19d turns on. + assert_eq!(combat_damage_to_defender(&state, a, &[b]), 0); + } + + #[test] + fn first_striking_blocker_that_kills_the_attacker_stops_the_regular_step() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 11, 2); + kw(&mut state, a, vec![Keyword::Trample]); + let b = create_creature(&mut state, PlayerId(1), "B", 3, 1); + kw(&mut state, b, vec![Keyword::FirstStrike]); + // CR 702.7b: the blocker's 3 kills the 11/2 in the first step, so the + // attacker never assigns its own damage at all. + assert_eq!(combat_damage_to_defender(&state, a, &[b]), 0); + } + + #[test] + fn first_strike_attacker_kills_blockers_before_they_can_trade() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 11, 11); + kw(&mut state, a, vec![Keyword::FirstStrike, Keyword::Trample]); + let b = create_creature(&mut state, PlayerId(1), "B", 1, 4); + // CR 702.7b: a lone first-strike step — 4 lethal, 7 through. It does NOT + // strike again (that would need double strike). + assert_eq!(combat_damage_to_defender(&state, a, &[b]), 7); + } + + #[test] + fn zero_power_attacker_assigns_no_combat_damage() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 0, 3); + kw(&mut state, a, vec![Keyword::Trample]); + // CR 510.1a + assert_eq!(combat_damage_to_defender(&state, a, &[]), 0); + } } diff --git a/crates/phase-ai/src/combat_ai.rs b/crates/phase-ai/src/combat_ai.rs index f62a884c96..58358af3e5 100644 --- a/crates/phase-ai/src/combat_ai.rs +++ b/crates/phase-ai/src/combat_ai.rs @@ -948,8 +948,8 @@ pub fn choose_blockers_with_profile( priority == 0 && attacker_power >= 2 && matches!(objective, CombatObjective::Race); // CR 903.10a: Skip chumps that don't actually save under commander damage // (e.g. 1/1 in front of a 12/12 trample commander with 3 cmd-damage headroom). - let chump_unsafe = priority == 0 - && commander_chump_unsafe(state, player, attacker_id, blocker_toughness); + let chump_unsafe = + priority == 0 && commander_chump_unsafe(state, player, attacker_id, &[blocker_id]); let favorable_trade = priority != 1 || selected_blocker_value <= attacker_value + damage_prevented as f64; if !chump_unsafe @@ -1127,34 +1127,21 @@ pub fn choose_blockers_with_profile( gang_value += value; } - // CR 702.19b: how much a gang actually absorbs. A trampling attacker must - // assign each blocker its lethal damage before any excess is assigned to the - // player, so absorption is the sum of those per-blocker minimums — NOT raw - // toughness. `lethal_damage_needed` is the resolver's own authority: it - // subtracts damage already marked, and returns 1 under deathtouch (CR - // 702.2c), which is why a deathtouch trampler absorbs almost nothing however - // large the gang. Summing raw toughness here approved gangs that left lethal - // trample damage against already-damaged blockers. - let gang_absorption = |set: &[ObjectId]| -> i32 { - set.iter() - .map(|&bid| { - engine::game::combat_damage::lethal_damage_needed( - state, - bid, - attacker_has_deathtouch, - ) as i32 - }) - .sum() + // The engine owns this calculation. `combat_damage_to_defender` models both + // damage steps (CR 702.4b), the blockers a first striker removes between them + // (CR 702.7b), per-blocker lethal minimums including damage already marked + // (CR 702.19b), a blocked trampler left with no blockers (CR 702.19d), and + // deathtouch (CR 702.2c). Every one of those interacts, and the AI's previous + // hand-rolled single-step estimate got the double-strike case wrong by a + // whole second strike. + let damage_through = |set: &[ObjectId]| -> i32 { + engine::game::combat_damage::combat_damage_to_defender(state, attacker_id, set) }; - // CR 510.1c + CR 702.19b: only trample can put damage past a legal block, so - // a nontrampler is unconditionally survivable once blocked. The `>= 2` floor - // mirrors the single-blocker pass's `damage_prevented >= 2`. + // CR 510.1c: a block that lets nothing through is a full save whatever + // becomes of the blockers. Otherwise it has to get the player under lethal. let averts_lethal = |set: &[ObjectId]| -> bool { - if !attacker_has_trample { - return true; - } - let absorbed = gang_absorption(set); - absorbed >= 2 && attacker_power.saturating_sub(absorbed) < effective_life + let through = damage_through(set); + through < effective_life && (through == 0 || attacker_power - through >= 2) }; // CR 509.1b + CR 510.1c + CR 702.7b + CR 702.19b: the survival gang answers a @@ -1167,11 +1154,32 @@ pub fn choose_blockers_with_profile( // left an 11/11 menace first-strike trampler unblockable by two 4/4s at 4 // life, though that block absorbs 4+4 and tramples only 3. // - // Grows cheapest-first past the floor until the residual is survivable, so it - // spends the fewest creatures that actually save the game. + // Ordering: against a trampler what a blocker contributes is absorption, not + // cheapness, so take the biggest absorbers first and break ties on value — + // a value-ordered walk spends several 1/1s where one 6/6 would do. Against a + // nontrampler any legal block already prevents everything (CR 510.1c), so the + // cheapest bodies are correct there and the existing value order stands. let survival_gang: Option> = if floor_stabilize_route { + let mut pool: Vec<(ObjectId, f64)> = gang_candidates + .iter() + .map(|&(bid, _, value)| (bid, value)) + .collect(); + if attacker_has_trample { + pool.sort_by(|a, b| { + let absorb = |bid: ObjectId| { + engine::game::combat_damage::lethal_damage_needed( + state, + bid, + attacker_has_deathtouch, + ) + }; + absorb(b.0) + .cmp(&absorb(a.0)) + .then(a.1.partial_cmp(&b.1).unwrap_or(std::cmp::Ordering::Equal)) + }); + } let mut set: Vec = Vec::new(); - for &(bid, _, _) in &gang_candidates { + for &(bid, _) in &pool { if set.len() >= needed_blockers && averts_lethal(&set) { break; } @@ -1224,8 +1232,7 @@ pub fn choose_blockers_with_profile( // The survival route already proved its own set legal and lethal-averting; // it only remains to reject the two shapes that save nothing. let stabilizing_gang = survival_gang.filter(|set| { - !reflects_damage(set) - && !commander_chump_unsafe(state, player, attacker_id, gang_absorption(set)) + !reflects_damage(set) && !commander_chump_unsafe(state, player, attacker_id, set) }); // Gang-block to kill when the trade is worth it, else to survive. Never below @@ -1353,12 +1360,7 @@ pub fn choose_blockers_with_profile( if !can_block_with_engine_map(state, bid, attacker_id, valid_block_targets) { return false; } - let blocker_toughness = state - .objects - .get(&bid) - .and_then(|b| b.toughness) - .unwrap_or(1); - !commander_chump_unsafe(state, player, attacker_id, blocker_toughness) + !commander_chump_unsafe(state, player, attacker_id, &[bid]) }); if let Some(&blocker_id) = safe_blocker { assignments.push((blocker_id, attacker_id)); @@ -1391,28 +1393,19 @@ fn commander_chump_unsafe( state: &GameState, defender: PlayerId, attacker_id: ObjectId, - chump_toughness: i32, + blockers: &[ObjectId], ) -> bool { let Some(headroom) = commander_lethal_headroom(state, defender, attacker_id) else { return false; }; - let Some(attacker) = state.objects.get(&attacker_id) else { - return false; - }; - let power = attacker.power.unwrap_or(0).max(0); - let trample_through = if attacker.has_keyword(&Keyword::Trample) { - // CR 702.2c: Deathtouch makes any nonzero damage lethal, so the trampler need only - // assign 1 to the blocker before sending excess to the player. - let lethal_to_blocker = if attacker.has_keyword(&Keyword::Deathtouch) { - 1 - } else { - chump_toughness - }; - (power - lethal_to_blocker).max(0) - } else { - 0 - }; - trample_through as u32 >= headroom + // CR 903.10a: unsafe when what still gets through would cross the + // commander-damage threshold. Delegates to the engine's damage authority rather + // than re-deriving trample and deathtouch here — the previous local version took + // a single blocker's toughness and replaced it with 1 under deathtouch (CR + // 702.2c), which is right for one chump blocker and throws away a whole gang's + // absorption when handed one, rejecting legal blocks that do prevent lethality. + engine::game::combat_damage::combat_damage_to_defender(state, attacker_id, blockers) as u32 + >= headroom } fn determine_attack_objective( @@ -4778,6 +4771,76 @@ mod tests { ); } + /// CR 702.4b + CR 702.19d: a double striker assigns damage in BOTH steps. The + /// first step kills these blockers and tramples 3; the second finds no blockers + /// and, per CR 702.19d, assigns its full 11 to the player "as though all + /// blocking creatures have been assigned lethal damage". 14 total against 4 + /// life, so the block does not save and must be declined. + /// + /// Discriminating for the single-step model: counting one step reads this as + /// 3 through and approves a block that loses the game. + #[test] + fn menace_double_strike_trampler_is_declined_when_the_second_strike_still_kills() { + let (state, attacker, _) = lethal_attacker_board_with( + 4, + 2, + vec![Keyword::Menace, Keyword::DoubleStrike, Keyword::Trample], + ); + + let assignments = choose_blockers(&state, PlayerId(1), &[attacker]); + + let on_attacker = assignments.iter().filter(|&&(_, a)| a == attacker).count(); + assert_eq!( + on_attacker, 0, + "CR 702.4b + CR 702.19d: the second strike finds an empty block and \ + tramples 11 more into a player already down to 1. Got {assignments:?}" + ); + } + + /// Ordering: against a trampler a blocker contributes absorption, not cheapness. + /// One 6/6 absorbs the whole gap that three 1/1s cannot, so the AI must reach for + /// it rather than walking the value order and spending bodies that do not save. + /// + /// 8/8 menace trampler at 3 life: 1/1s absorb 1 each (three of them leave 5), the + /// 6/6 absorbs 6 and leaves 2. The floor is 2, so the answer is the 6/6 plus one + /// 1/1 — absorbing 7, leaving 1. + #[test] + fn trample_survival_gang_prefers_absorbers_over_cheap_bodies() { + let mut state = setup(); + state.players[1].life = 3; + let attacker = add_creature( + &mut state, + PlayerId(0), + "Attacker", + 8, + 8, + vec![Keyword::Menace, Keyword::Trample], + ); + let big = add_creature(&mut state, PlayerId(1), "Big", 1, 6, vec![]); + for i in 0..3 { + add_creature(&mut state, PlayerId(1), &format!("Small {i}"), 1, 1, vec![]); + } + + let assignments = choose_blockers(&state, PlayerId(1), &[attacker]); + + let on_attacker: Vec<_> = assignments + .iter() + .filter(|&&(_, a)| a == attacker) + .map(|&(b, _)| b) + .collect(); + assert!( + on_attacker.contains(&big), + "CR 702.19b: the 6-toughness blocker absorbs 6 of the 8 — a value-ordered \ + walk spends 1/1s that cannot close the gap. Got {assignments:?}" + ); + assert_eq!( + on_attacker.len(), + 2, + "the floor is 2 and the 6/6 plus one 1/1 already leaves 1 damage — no \ + further creatures should be spent. Got {assignments:?}" + ); + } + /// CR 702.19b: "take into account damage already marked on the creature" — a /// 4/4 with 3 damage marked needs only 1 more to be lethal, so it absorbs 1, not /// 4. Two of them absorb 2 against an 11/11 menace trampler, leaving 9 against From fdbe94be3066e39481ed7e901f0ca8ced2809c07 Mon Sep 17 00:00:00 2001 From: mike-theDude Date: Thu, 13 Aug 2026 15:11:40 -0500 Subject: [PATCH 4/7] fix(engine): count both damage steps and marked damage in the defender estimate (#7183) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on the new authority, both confirmed against the resolver. 1. **An unblocked double striker deals twice its power.** The empty-`blockers` early return handed back `power` before the keyword reads, so CR 702.4b's second damage step was dropped. `resolve_combat_damage` already models this — `double_strike_deals_damage_twice` has an unblocked 3/3 double striker deal 6 — so the helper claiming to be the authority for that question disagreed with it. Plain first strike still assigns once; only double strike doubles. 2. **Damage assigned to a surviving blocker was forgotten between steps.** When the attacker could not afford a blocker's lethal minimum the leftover budget was dropped, so the regular step saw the blocker's full toughness again. CR 702.19b counts damage already marked, and first-strike-step damage is marked for the regular step. A 5-power double-strike trampler into one 0/6 read as 0 through, when the first step marks 5, the regular step needs only 1 more, and 4 tramples to the player. Both understated the defender's exposure, which is the failure direction that matters: the AI reads this for survival and commander-lethality decisions, so it could accept a gang block that still loses the game. Three regressions, each verified discriminating: reverting the doubling fails `unblocked_double_striker_assigns_in_both_steps`, reverting the carryover fails `damage_marked_in_the_first_step_lowers_lethal_for_the_second`, and `unblocked_first_striker_assigns_once` guards against conflating "strikes first" with "strikes twice". Found by CodeRabbit on the current head. Co-Authored-By: Claude Opus 5 (1M context) --- crates/engine/src/game/combat_damage.rs | 65 +++++++++++++++++++++---- 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/game/combat_damage.rs b/crates/engine/src/game/combat_damage.rs index 02fd9ebfe4..56c7bbfbb0 100644 --- a/crates/engine/src/game/combat_damage.rs +++ b/crates/engine/src/game/combat_damage.rs @@ -1127,16 +1127,19 @@ pub fn combat_damage_to_defender( if power == 0 { return 0; } - // CR 510.1b: unblocked — the whole power reaches the player. - if blockers.is_empty() { - return power; - } - let has_trample = attacker.has_keyword(&Keyword::Trample); let has_deathtouch = attacker.has_keyword(&Keyword::Deathtouch); let has_double_strike = attacker.has_keyword(&Keyword::DoubleStrike); let attacker_strikes_first = has_double_strike || attacker.has_keyword(&Keyword::FirstStrike); + // CR 510.1b + CR 702.4b: unblocked — the whole power reaches the player, once + // per damage step it assigns in. A double striker assigns in both, which is what + // `resolve_combat_damage` does (`double_strike_deals_damage_twice`: an unblocked + // 3/3 double striker deals 6). Plain first strike assigns in one step only. + if blockers.is_empty() { + return if has_double_strike { power * 2 } else { power }; + } + // Remaining lethal per blocker, in the order the attacker would exhaust them. let mut remaining: Vec = blockers .iter() @@ -1209,15 +1212,23 @@ fn assign_step_to_defender(remaining: &mut Vec, power: i32, has_trample: bo // Otherwise every point is spent on blockers. Kill the cheapest first — the // attacking player chooses, and fewer survivors means less absorption in any // later step. + // + // CR 702.19b: damage assigned to a blocker that survives stays MARKED on it, so + // its lethal minimum in a later damage step is reduced by that much. Dropping + // the leftover budget instead understated the defender's exposure: a 5-power + // double-strike trampler against one 0/6 was read as 0 through, when the first + // step marks 5 and the regular step then needs only 1 more before trampling 4. + // `remaining` is sorted ascending, so the front is always the cheapest kill. let mut budget = power; - remaining.retain(|&lethal| { + while let Some(&lethal) = remaining.first() { if budget >= lethal { budget -= lethal; - false + remaining.remove(0); } else { - true + remaining[0] -= budget; + break; } - }); + } 0 } @@ -4573,6 +4584,42 @@ mod tests { assert_eq!(combat_damage_to_defender(&state, a, &[b]), 7); } + /// CR 702.4b: an unblocked double striker assigns in both damage steps. Pinned + /// against `resolve_combat_damage`'s own answer — `double_strike_deals_damage_twice` + /// has an unblocked 3/3 double striker deal 6 — because this helper claims to be + /// the authority for the same question and must not disagree with the resolver. + #[test] + fn unblocked_double_striker_assigns_in_both_steps() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 3, 3); + kw(&mut state, a, vec![Keyword::DoubleStrike]); + assert_eq!(combat_damage_to_defender(&state, a, &[]), 6); + } + + /// Plain first strike is one step, not two — the distinction double strike turns + /// on. Guards against "strikes first" being conflated with "strikes twice". + #[test] + fn unblocked_first_striker_assigns_once() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 3, 3); + kw(&mut state, a, vec![Keyword::FirstStrike]); + assert_eq!(combat_damage_to_defender(&state, a, &[]), 3); + } + + /// CR 702.19b: damage assigned to a surviving blocker stays marked on it, so the + /// regular step sees a reduced lethal minimum. A 5-power double-strike trampler + /// into one 0/6 marks 5 in the first step (0 through, since lethal was not + /// assigned to every blocker), then needs only 1 more in the regular step and + /// tramples the other 4. + #[test] + fn damage_marked_in_the_first_step_lowers_lethal_for_the_second() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 5, 5); + kw(&mut state, a, vec![Keyword::DoubleStrike, Keyword::Trample]); + let b = create_creature(&mut state, PlayerId(1), "B", 0, 6); + assert_eq!(combat_damage_to_defender(&state, a, &[b]), 4); + } + #[test] fn zero_power_attacker_assigns_no_combat_damage() { let mut state = setup(); From ab82fcd73c53f9dd6b298158a8418ee8391f7ae3 Mon Sep 17 00:00:00 2001 From: mike-theDude Date: Thu, 13 Aug 2026 16:11:52 -0500 Subject: [PATCH 5/7] fix(ai): compare the survival test against the whole-combat baseline (#7183) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. `combat_damage_to_defender` reports damage over the whole combat phase, so once `averts_lethal` started reading it, the prevented-damage floor was comparing that against `attacker_power` — one damage step's worth. For anything without double strike the two agree and nothing changed. For a double striker they differ by an entire strike (CR 702.4b), and they differ in the direction that rejects a block which saves the game: a 5/5 double-strike trampler is 10 unblocked, and a gang that cuts it to 6 gives `attacker_power - through = -1`, failing the floor. Takes the baseline from the same authority — `damage_through(&[])`, the unblocked whole-combat damage — so the subtraction is like for like and the floor keeps meaning "this block prevents at least 2 damage". Regression: `double_strike_trample_gang_is_taken_when_it_is_the_difference_between_living_and_dying`. A 5/5 menace double-strike trampler at 7 life against two 2/2s — 10 unblocked is lethal, the legal pair absorbs 2+2 in the first step (1 tramples) and takes 5 in the regular step for 6 total, and the player lives at 1. Reverting the baseline to `attacker_power` declines the block entirely and the player dies. This is the first AI-caller test for the double-strike path; the engine helper's own tests cover the arithmetic but do not exercise this consumer. Verification: cargo fmt --all clean, cargo clippy -p phase-ai --all-targets clean, cargo test -p phase-ai 2183 passed / 0 failed. Co-Authored-By: Claude Opus 5 (1M context) --- crates/phase-ai/src/combat_ai.rs | 58 ++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 2 deletions(-) diff --git a/crates/phase-ai/src/combat_ai.rs b/crates/phase-ai/src/combat_ai.rs index 58358af3e5..2ccfae4ae8 100644 --- a/crates/phase-ai/src/combat_ai.rs +++ b/crates/phase-ai/src/combat_ai.rs @@ -1137,11 +1137,19 @@ pub fn choose_blockers_with_profile( let damage_through = |set: &[ObjectId]| -> i32 { engine::game::combat_damage::combat_damage_to_defender(state, attacker_id, set) }; + // The no-block baseline, read from the same authority so the comparison below + // is like for like. NOT `attacker_power`: that is one damage step's worth, + // while `damage_through` reports the whole combat phase, so a double striker + // makes the two disagree by an entire strike (CR 702.4b) — and disagree in + // the direction that rejects a block which does save the game. + let unblocked_damage = damage_through(&[]); // CR 510.1c: a block that lets nothing through is a full save whatever - // becomes of the blockers. Otherwise it has to get the player under lethal. + // becomes of the blockers. Otherwise it has to get the player under lethal + // AND actually prevent something worth the creatures — the same + // `damage_prevented >= 2` floor the single-blocker pass applies. let averts_lethal = |set: &[ObjectId]| -> bool { let through = damage_through(set); - through < effective_life && (through == 0 || attacker_power - through >= 2) + through < effective_life && (through == 0 || unblocked_damage - through >= 2) }; // CR 509.1b + CR 510.1c + CR 702.7b + CR 702.19b: the survival gang answers a @@ -4797,6 +4805,52 @@ mod tests { ); } + /// The survival test must compare like with like. `combat_damage_to_defender` + /// reports the WHOLE combat phase, so for a double striker it counts both steps + /// (CR 702.4b) while `attacker_power` is one step's worth — and the block that + /// saves the game is exactly where the two disagree. + /// + /// A 5/5 menace double-strike trampler is 10 damage unblocked (5 through the + /// first step's excess plus 5 more into the empty block, CR 702.19d), lethal at + /// 7 life. Two 2/2s absorb 2+2 in the first step, so 1 tramples then, and 5 in + /// the regular step: 6 total, and the player lives at 1. + /// + /// Comparing against raw `attacker_power` gives `5 - 6 = -1`, fails the + /// prevented-damage floor, and declines a block that saves the game — so + /// reverting the baseline to `attacker_power` fails this test. + #[test] + fn double_strike_trample_gang_is_taken_when_it_is_the_difference_between_living_and_dying() { + let mut state = setup(); + state.players[1].life = 7; + let attacker = add_creature( + &mut state, + PlayerId(0), + "Attacker", + 5, + 5, + vec![Keyword::Menace, Keyword::DoubleStrike, Keyword::Trample], + ); + for i in 0..2 { + add_creature( + &mut state, + PlayerId(1), + &format!("Blocker {i}"), + 1, + 2, + vec![], + ); + } + + let assignments = choose_blockers(&state, PlayerId(1), &[attacker]); + + let on_attacker = assignments.iter().filter(|&&(_, a)| a == attacker).count(); + assert_eq!( + on_attacker, 2, + "10 unblocked at 7 life is lethal; the legal pair cuts it to 6 and the \ + player lives. Got {assignments:?}" + ); + } + /// Ordering: against a trampler a blocker contributes absorption, not cheapness. /// One 6/6 absorbs the whole gap that three 1/1s cannot, so the AI must reach for /// it rather than walking the value order and spending bodies that do not save. From 40e0441b6e3da18fbe8d036abb1405432ca0dd71 Mon Sep 17 00:00:00 2001 From: mike-theDude Date: Thu, 13 Aug 2026 16:31:05 -0500 Subject: [PATCH 6/7] fix(ai): drop survival blockers the rest of the gang makes redundant (#7183) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. The survival walk adds candidates by *static* absorption, which cannot express what a blocker contributes beyond soaking damage: a first-striking deathtouch body kills the attacker in the first damage step (CR 702.7b + CR 702.2c) and cancels its regular-step damage outright. Measured on the reported board — 10/10 menace double-strike trampler at 4 life, two 0/6 walls and a 1/1 first-strike deathtoucher: unblocked 20 both walls 8 (survives the first step, 8 tramples in the second) one wall + the 1/1 3 (attacker dies in the first step, no second) all three 0 Absorption order takes the walls first, still lets 8 through, and then adds the 1/1 — three creatures where two suffice. Adds a bounded shrink after the walk: once the set survives, try removing each member and keep the removal when the remainder still averts lethal, never going below the CR 509.1b floor. Judged through `combat_damage_to_defender`, the same whole-combat authority the gate uses, so it accounts for every interaction the ordering heuristic cannot. Deliberately NOT an exhaustive minimum-set search. This runs per attacker inside block selection, and a subset search is exponential in the candidate count; the shrink is one pass, at most `set.len()` extra evaluations, and finds the minimum whenever redundancy is per-blocker — which is the whole class the ordering heuristic can get wrong. Decision-cost perf gate covers the hot path. Regression: `survival_gang_drops_blockers_the_rest_of_the_gang_makes_redundant`, which also asserts the shrunk declaration still passes `complete_blocker_proposal` so the CR 509.1b floor is not undercut. Verified discriminating: removing the shrink pass fails it with all three blockers assigned. Verification: cargo fmt --all clean, cargo clippy -p phase-ai --all-targets clean, cargo test -p phase-ai 2184 passed / 0 failed. Co-Authored-By: Claude Opus 5 (1M context) --- crates/phase-ai/src/combat_ai.rs | 85 ++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/crates/phase-ai/src/combat_ai.rs b/crates/phase-ai/src/combat_ai.rs index 2ccfae4ae8..d096544642 100644 --- a/crates/phase-ai/src/combat_ai.rs +++ b/crates/phase-ai/src/combat_ai.rs @@ -1193,6 +1193,32 @@ pub fn choose_blockers_with_profile( } set.push(bid); } + + // The walk above adds by *static* absorption, which cannot see what a + // blocker contributes beyond soaking damage — a first-striking + // deathtouch blocker can kill the attacker outright in the first step + // (CR 702.7b + CR 702.2c) and cancel its regular-step damage entirely. + // So once the set survives, drop any member the rest has made redundant, + // judged through the same whole-combat authority rather than the + // ordering heuristic. One bounded pass over the gang, never below the + // CR 509.1b floor. + // + // Concretely: a 10/10 menace double-strike trampler against two 0/6 + // walls and a 1/1 first-strike deathtouch blocker. Absorption order + // takes both walls (8 still gets through) and then the 1/1 to survive — + // but one wall plus the 1/1 holds it to 3, because the attacker dies + // before the regular step. Three creatures spent where two suffice. + let mut index = 0; + while index < set.len() && set.len() > needed_blockers { + let mut trial = set.clone(); + trial.remove(index); + if averts_lethal(&trial) { + set = trial; + } else { + index += 1; + } + } + (set.len() >= needed_blockers && averts_lethal(&set)).then_some(set) } else { None @@ -4851,6 +4877,65 @@ mod tests { ); } + /// A blocker can contribute something absorption cannot express: a first-strike + /// deathtouch body kills the attacker in the first damage step (CR 702.7b + + /// CR 702.2c), cancelling its regular-step damage outright. + /// + /// 10/10 menace double-strike trampler at 4 life, against two 0/6 walls and a + /// 1/1 first-strike deathtoucher. By absorption the walls come first, and they + /// still let 8 through, so the walk then adds the 1/1 — three creatures. But one + /// wall plus the 1/1 holds it to 3: the attacker takes lethal deathtouch damage + /// in the first step and never reaches the second. Two creatures, same result. + /// + /// Guards the shrink pass; without it the AI sacrifices a blocker it did not need. + #[test] + fn survival_gang_drops_blockers_the_rest_of_the_gang_makes_redundant() { + let mut state = setup(); + state.players[1].life = 4; + let attacker = add_creature( + &mut state, + PlayerId(0), + "Attacker", + 10, + 10, + vec![Keyword::Menace, Keyword::DoubleStrike, Keyword::Trample], + ); + add_creature(&mut state, PlayerId(1), "Wall 1", 0, 6, vec![]); + add_creature(&mut state, PlayerId(1), "Wall 2", 0, 6, vec![]); + add_creature( + &mut state, + PlayerId(1), + "Deathtouch Skirmisher", + 1, + 1, + vec![Keyword::FirstStrike, Keyword::Deathtouch], + ); + + let assignments = choose_blockers(&state, PlayerId(1), &[attacker]); + + let on_attacker: Vec<_> = assignments + .iter() + .filter(|&&(_, a)| a == attacker) + .map(|&(b, _)| b) + .collect(); + assert_eq!( + on_attacker.len(), + 2, + "one wall plus the first-strike deathtoucher already holds this to 3 \ + against 4 life — the third blocker is spent for nothing. Got \ + {assignments:?}" + ); + // Still a legal declaration against the CR 702.111b menace floor. + assert_eq!( + engine::game::combat::complete_blocker_proposal(&state, PlayerId(1), &assignments), + engine::types::actions::GameAction::DeclareBlockers { + assignments: assignments.clone() + }, + "the shrunk declaration must still survive the CR 509.1c completion \ + authority" + ); + } + /// Ordering: against a trampler a blocker contributes absorption, not cheapness. /// One 6/6 absorbs the whole gap that three 1/1s cannot, so the AI must reach for /// it rather than walking the value order and spending bodies that do not save. From 86771734eb4b5c23e6861b934c24f7081c080317 Mon Sep 17 00:00:00 2001 From: mike-theDude Date: Thu, 13 Aug 2026 20:04:56 -0500 Subject: [PATCH 7/7] fix(engine): read both damage sides through the combat damage authority (#7183) `combat_damage_to_defender` derived the attacker's assignment from raw `attacker.power` and the first-strike blockers' from raw `.power`, while the resolver it claims to predict assigns through `combat_damage_amount`. That helper implements two rules the raw reads drop: CR 613.11's Doran-style "assigns damage equal to its toughness" and CR 510.1a's "assigns no combat damage". A helper documented as the single authority for AI survival and commander-lethality decisions cannot disagree with the resolver about how much a creature assigns, so both reads now go through it. Four regressions, each chosen so a raw-power read gives a different answer: a Doran attacker tramples 3 rather than being read as unable to cover its blocker at all; an "assigns no combat damage" attacker connects for 0 rather than 7; a first-striking Doran blocker kills the attacker before the regular step rather than assigning 0; and a first-striking blocker that assigns no combat damage fails to kill it, letting 10 trample rather than 0. Co-Authored-By: Claude Opus 5 (1M context) --- crates/engine/src/game/combat_damage.rs | 78 ++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/game/combat_damage.rs b/crates/engine/src/game/combat_damage.rs index 56c7bbfbb0..089b1a3ca1 100644 --- a/crates/engine/src/game/combat_damage.rs +++ b/crates/engine/src/game/combat_damage.rs @@ -1122,8 +1122,12 @@ pub fn combat_damage_to_defender( let Some(attacker) = state.objects.get(&attacker_id) else { return 0; }; + // CR 510.1a + CR 613.11: `combat_damage_amount` is the resolver's authority for + // how much a creature assigns — power normally, toughness under a Doran-style + // rule-modifying effect, and 0 under "assigns no combat damage". Reading raw + // power here would make this helper contradict the combat it exists to predict. // CR 510.1a: a creature assigning 0 or less assigns no combat damage at all. - let power = attacker.power.unwrap_or(0).max(0); + let power = combat_damage_amount(attacker) as i32; if power == 0 { return 0; } @@ -1172,9 +1176,12 @@ pub fn combat_damage_to_defender( .iter() .any(|b| b.has_keyword(&Keyword::Deathtouch)), ) as i32; + // CR 510.1a + CR 613.11: same authority for the blockers — a first-striking + // Doran-style blocker kills the attacker with its toughness, and one that + // assigns no combat damage cannot kill it at all. let first_strike_damage: i32 = first_strike_blockers .iter() - .map(|b| b.power.unwrap_or(0).max(0)) + .map(|b| combat_damage_amount(b) as i32) .sum(); if first_strike_damage >= attacker_lethal { attacker_alive = false; @@ -4628,4 +4635,71 @@ mod tests { // CR 510.1a assert_eq!(combat_damage_to_defender(&state, a, &[]), 0); } + + // ── Modified damage basis (CR 613.11) ── + // + // `combat_damage_amount` is what `resolve_combat_damage` assigns from, so this + // helper must read the same number. Every case below is chosen to give a + // different answer if either side is re-derived from raw power. + + /// CR 613.11 + CR 702.19b: a Doran-style attacker assigns its toughness, so a + /// 1/5 trampler covers a 0/2 blocker's lethal and tramples 3. Read from raw + /// power it assigns 1, fails to cover the blocker, and gets 0 through. + #[test] + fn attacker_assigning_from_toughness_tramples_on_its_toughness() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 1, 5); + kw(&mut state, a, vec![Keyword::Trample]); + state + .objects + .get_mut(&a) + .unwrap() + .assigns_damage_from_toughness = true; + let b = create_creature(&mut state, PlayerId(1), "B", 0, 2); + assert_eq!(combat_damage_to_defender(&state, a, &[b]), 3); + } + + /// CR 510.1a: "assigns no combat damage" zeroes an unblocked attacker that would + /// otherwise connect for its full power. + #[test] + fn attacker_assigning_no_combat_damage_reaches_the_player_for_nothing() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 7, 7); + kw(&mut state, a, vec![Keyword::Trample]); + state.objects.get_mut(&a).unwrap().assigns_no_combat_damage = true; + assert_eq!(combat_damage_to_defender(&state, a, &[]), 0); + } + + /// CR 613.11 + CR 702.7b: a first-striking blocker assigning from toughness kills + /// the 11/2 attacker with its 3 toughness before the regular step, so nothing + /// tramples through. Read from raw power it assigns 0 and 8 gets through. + #[test] + fn first_strike_blocker_assigning_from_toughness_kills_the_attacker() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 11, 2); + kw(&mut state, a, vec![Keyword::Trample]); + let b = create_creature(&mut state, PlayerId(1), "B", 0, 3); + kw(&mut state, b, vec![Keyword::FirstStrike]); + state + .objects + .get_mut(&b) + .unwrap() + .assigns_damage_from_toughness = true; + assert_eq!(combat_damage_to_defender(&state, a, &[b]), 0); + } + + /// CR 510.1a + CR 702.7b: the mirror case — a first-striking blocker that assigns + /// no combat damage cannot kill the attacker, so the regular step still happens + /// and 10 tramples past its 1 toughness. Read from raw power its 5 would be + /// lethal to the 11/2 and this would be 0. + #[test] + fn first_strike_blocker_assigning_no_combat_damage_cannot_kill_the_attacker() { + let mut state = setup(); + let a = create_creature(&mut state, PlayerId(0), "A", 11, 2); + kw(&mut state, a, vec![Keyword::Trample]); + let b = create_creature(&mut state, PlayerId(1), "B", 5, 1); + kw(&mut state, b, vec![Keyword::FirstStrike]); + state.objects.get_mut(&b).unwrap().assigns_no_combat_damage = true; + assert_eq!(combat_damage_to_defender(&state, a, &[b]), 10); + } }