From c8faa0af635e65a542a18766ebe1d0cb1545d210 Mon Sep 17 00:00:00 2001 From: Daedalus-Icarus <6442298+Daedalus-Icarus@users.noreply.github.com> Date: Thu, 2 Jul 2026 23:23:38 +0200 Subject: [PATCH 1/5] fix: use per-attacker attack target legality --- client/src/adapter/types.ts | 2 +- client/src/components/board/ActionButton.tsx | 14 +- .../controls/AttackTargetPicker.tsx | 34 +- client/src/utils/__tests__/combat.test.ts | 79 +++ client/src/utils/combat.ts | 40 +- crates/engine/src/ai_support/candidates.rs | 65 +- crates/engine/src/ai_support/mod.rs | 1 + crates/engine/src/database/synthesis.rs | 1 + crates/engine/src/game/combat.rs | 619 ++++++++++++------ .../engine_phase_trigger_regression_tests.rs | 3 + crates/engine/src/game/engine_tests.rs | 3 + crates/engine/src/game/triggers.rs | 6 + crates/engine/src/game/turns.rs | 3 + crates/engine/src/types/game_state.rs | 3 + .../tests/integration/emissary_green.rs | 1 + .../giggling_skitterspike_issue_890.rs | 2 + .../issue_1297_venture_initiative_triggers.rs | 2 + .../engine/tests/integration/rules/combat.rs | 1 + crates/engine/tests/squirrel_perf_probe.rs | 1 + crates/manabrew-compat/src/lib.rs | 1 + .../phase-ai/src/bin/attack_scaling_bench.rs | 1 + .../src/bin/declare_attackers_bench.rs | 1 + crates/phase-ai/src/combat_ai.rs | 249 ++++++- crates/phase-ai/src/decision_kind.rs | 1 + .../phase-ai/src/policies/aggro_pressure.rs | 2 + crates/phase-ai/src/policies/tokens_wide.rs | 1 + crates/phase-ai/src/search.rs | 45 ++ crates/phase-ai/tests/ai_quality.rs | 2 + crates/phase-ai/tests/scenarios.rs | 1 + 29 files changed, 945 insertions(+), 239 deletions(-) create mode 100644 client/src/utils/__tests__/combat.test.ts diff --git a/client/src/adapter/types.ts b/client/src/adapter/types.ts index c35b86b31c..a69608d862 100644 --- a/client/src/adapter/types.ts +++ b/client/src/adapter/types.ts @@ -1183,7 +1183,7 @@ export type WaitingFor = } | { type: "PayAmountChoice"; data: { player: PlayerId; resource: PayableResource; min: number; max: number; accumulated?: number; source_id: ObjectId; pending_mana_ability?: unknown } } | { type: "TargetSelection"; data: { player: PlayerId; pending_cast: PendingCast; target_slots: TargetSelectionSlot[]; mode_labels?: (string | null)[]; selection: TargetSelectionProgress } } - | { type: "DeclareAttackers"; data: { player: PlayerId; valid_attacker_ids: ObjectId[]; valid_attack_targets?: AttackTarget[] } } + | { type: "DeclareAttackers"; data: { player: PlayerId; valid_attacker_ids: ObjectId[]; valid_attack_targets?: AttackTarget[]; valid_attack_targets_by_attacker?: Record } } | { type: "DeclareBlockers"; data: { player: PlayerId; valid_blocker_ids: ObjectId[]; valid_block_targets: Record; block_requirements?: Record } } | { type: "GameOver"; data: { winner: PlayerId | null } } | { type: "ReplacementChoice"; data: { player: PlayerId; candidate_count: number; candidate_descriptions?: string[] } } diff --git a/client/src/components/board/ActionButton.tsx b/client/src/components/board/ActionButton.tsx index a28bdc8d0e..4cb7d73039 100644 --- a/client/src/components/board/ActionButton.tsx +++ b/client/src/components/board/ActionButton.tsx @@ -9,7 +9,11 @@ import { usePhaseInfo } from "../../hooks/usePhaseInfo.ts"; import { useGameStore } from "../../stores/gameStore.ts"; import { useMultiplayerStore } from "../../stores/multiplayerStore.ts"; import { useUiStore } from "../../stores/uiStore.ts"; -import { buildAttacks, hasMultipleAttackTargets, getValidAttackTargets } from "../../utils/combat.ts"; +import { + buildAttacks, + getValidAttackTargets, + selectedAttackersNeedTargetPicker, +} from "../../utils/combat.ts"; import { useBlockRequirements } from "../combat/useBlockRequirements.ts"; import { gameButtonClass } from "../ui/buttonStyles.ts"; import { GameplayTooltip } from "../ui/GameplayTooltip.tsx"; @@ -97,8 +101,11 @@ export function ActionButton() { // Attack target picker visibility (multiplayer) const [showTargetPicker, setShowTargetPicker] = useState(false); - const isMultiTarget = hasMultipleAttackTargets(gameState); const validAttackTargets = getValidAttackTargets(gameState); + const validAttackTargetsByAttacker = + waitingFor?.type === "DeclareAttackers" + ? (waitingFor.data.valid_attack_targets_by_attacker ?? {}) + : {}; // Reset skip-confirm when mode changes useEffect(() => { @@ -215,7 +222,7 @@ export function ActionButton() { } function handleConfirmAttackers() { - if (isMultiTarget) { + if (selectedAttackersNeedTargetPicker(gameState, selectedAttackers)) { setShowTargetPicker(true); return; } @@ -462,6 +469,7 @@ export function ActionButton() { {showTargetPicker && ( setShowTargetPicker(false)} diff --git a/client/src/components/controls/AttackTargetPicker.tsx b/client/src/components/controls/AttackTargetPicker.tsx index b87403c684..2458e9217e 100644 --- a/client/src/components/controls/AttackTargetPicker.tsx +++ b/client/src/components/controls/AttackTargetPicker.tsx @@ -15,6 +15,7 @@ import { PeekTab } from "../modal/DialogShell.tsx"; interface AttackTargetPickerProps { validTargets: AttackTarget[]; + validTargetsByAttacker?: Record; selectedAttackers: ObjectId[]; onConfirm: (attacks: [ObjectId, AttackTarget][]) => void; onCancel: () => void; @@ -29,6 +30,7 @@ interface AttackTargetPickerProps { */ export function AttackTargetPicker({ validTargets, + validTargetsByAttacker, selectedAttackers, onConfirm, onCancel, @@ -48,6 +50,10 @@ export function AttackTargetPicker({ const teamBased = gameState?.format_config?.team_based ?? false; + function targetsForCreature(creatureId: ObjectId): AttackTarget[] { + return validTargetsByAttacker?.[String(creatureId)] ?? validTargets; + } + const sortedTargets = useMemo(() => { if (!seatOrder) return validTargets; return [...validTargets].sort((a, b) => { @@ -57,6 +63,14 @@ export function AttackTargetPicker({ }); }, [validTargets, seatOrder]); + const commonTargets = useMemo( + () => sortedTargets.filter((target) => + selectedAttackers.every((id) => + targetsForCreature(id).some((candidate) => sameAttackTarget(candidate, target))), + ), + [selectedAttackers, sortedTargets, validTargets, validTargetsByAttacker], + ); + function getTargetLabel(target: AttackTarget): string { if (target.type === "Player") { return getPlayerLabel(t, target.data, myId, teamBased); @@ -79,7 +93,7 @@ export function AttackTargetPicker({ function handleSplitConfirm() { const attacks: [ObjectId, AttackTarget][] = selectedAttackers.map((id) => { - const target = perCreatureTargets.get(id) ?? validTargets[0]; + const target = perCreatureTargets.get(id) ?? targetsForCreature(id)[0] ?? validTargets[0]; return [id, target]; }); onConfirm(attacks); @@ -97,7 +111,9 @@ export function AttackTargetPicker({ setPerCreatureTargets(() => { const next = new Map(); for (const id of selectedAttackers) { - next.set(id, target); + if (targetsForCreature(id).some((candidate) => sameAttackTarget(candidate, target))) { + next.set(id, target); + } } return next; }); @@ -149,7 +165,7 @@ export function AttackTargetPicker({ {mode === "all" ? ( /* Attack All mode: one button per target */
- {sortedTargets.map((target) => { + {commonTargets.map((target) => { const color = getTargetSeatColor(target); return (
void }) { const { t } = useTranslation("game"); return ( diff --git a/client/src/utils/__tests__/combat.test.ts b/client/src/utils/__tests__/combat.test.ts new file mode 100644 index 0000000000..cd65d3b802 --- /dev/null +++ b/client/src/utils/__tests__/combat.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from "vitest"; + +import type { AttackTarget, GameState, ObjectId } from "../../adapter/types"; +import { + buildAttacks, + getValidAttackTargetsForAttacker, + selectedAttackersNeedTargetPicker, +} from "../combat"; + +function playerTarget(playerId: number): AttackTarget { + return { type: "Player", data: playerId }; +} + +function declareAttackersState( + validAttackTargetsByAttacker: Record, +): GameState { + return { + waiting_for: { + type: "DeclareAttackers", + data: { + player: 0, + valid_attacker_ids: [11, 12], + valid_attack_targets: [playerTarget(1), playerTarget(2)], + valid_attack_targets_by_attacker: validAttackTargetsByAttacker, + }, + }, + players: [ + { id: 0, life: 20, poison_counters: 0, mana_pool: { mana: [] }, library: [], hand: [], graveyard: [], has_drawn_this_turn: false, lands_played_this_turn: 0, turns_taken: 0 }, + { id: 1, life: 20, poison_counters: 0, mana_pool: { mana: [] }, library: [], hand: [], graveyard: [], has_drawn_this_turn: false, lands_played_this_turn: 0, turns_taken: 0 }, + { id: 2, life: 20, poison_counters: 0, mana_pool: { mana: [] }, library: [], hand: [], graveyard: [], has_drawn_this_turn: false, lands_played_this_turn: 0, turns_taken: 0 }, + ], + seat_order: [0, 1, 2], + } as unknown as GameState; +} + +describe("combat utils", () => { + it("builds per-attacker defaults from engine legality instead of seat order", () => { + const state = declareAttackersState({ + "11": [playerTarget(2)], + "12": [playerTarget(1)], + }); + + expect(buildAttacks([11, 12], state, 0)).toEqual([ + [11, playerTarget(2)], + [12, playerTarget(1)], + ]); + }); + + it("does not force the target picker when each attacker has only one legal target", () => { + const attackers: ObjectId[] = [11, 12]; + const state = declareAttackersState({ + "11": [playerTarget(2)], + "12": [playerTarget(1)], + }); + + expect(selectedAttackersNeedTargetPicker(state, attackers)).toBe(false); + }); + + it("forces the target picker when an attacker has multiple legal targets", () => { + const state = declareAttackersState({ + "11": [playerTarget(1), playerTarget(2)], + "12": [playerTarget(1)], + }); + + expect(getValidAttackTargetsForAttacker(state, 11)).toEqual([ + playerTarget(1), + playerTarget(2), + ]); + expect(selectedAttackersNeedTargetPicker(state, [11, 12])).toBe(true); + }); + + it("treats an attacker missing from the per-attacker map as having no legal targets", () => { + const state = declareAttackersState({ + "12": [playerTarget(1)], + }); + + expect(getValidAttackTargetsForAttacker(state, 11)).toEqual([]); + }); +}); diff --git a/client/src/utils/combat.ts b/client/src/utils/combat.ts index 0953a2b21c..5e0f8d814a 100644 --- a/client/src/utils/combat.ts +++ b/client/src/utils/combat.ts @@ -12,7 +12,10 @@ export function buildAttacks( targetOverrides?: Map, ): [ObjectId, AttackTarget][] { const defaultTarget = getDefaultAttackTarget(state, myId); - return attackerIds.map((id) => [id, targetOverrides?.get(id) ?? defaultTarget]); + return attackerIds.map((id) => [ + id, + targetOverrides?.get(id) ?? getDefaultAttackTargetForAttacker(state, id) ?? defaultTarget, + ]); } /** Returns the default attack target: first non-eliminated opponent. */ @@ -40,6 +43,20 @@ export function hasMultipleAttackTargets( return targets != null && targets.length > 1; } +/** Check if the selected attackers need explicit target selection. */ +export function selectedAttackersNeedTargetPicker( + state: GameState | null, + attackerIds: ObjectId[], +): boolean { + if (!state) return false; + const wf = state.waiting_for; + if (wf.type !== "DeclareAttackers") return false; + if (wf.data.valid_attack_targets_by_attacker != null) { + return attackerIds.some((id) => getValidAttackTargetsForAttacker(state, id).length > 1); + } + return hasMultipleAttackTargets(state); +} + /** Get valid attack targets from the current WaitingFor state. */ export function getValidAttackTargets( state: GameState | null, @@ -49,3 +66,24 @@ export function getValidAttackTargets( if (wf.type !== "DeclareAttackers") return []; return wf.data.valid_attack_targets ?? []; } + +/** Get legal attack targets for a specific attacker from the current WaitingFor state. */ +export function getValidAttackTargetsForAttacker( + state: GameState | null, + attackerId: ObjectId, +): AttackTarget[] { + if (!state) return []; + const wf = state.waiting_for; + if (wf.type !== "DeclareAttackers") return []; + if (wf.data.valid_attack_targets_by_attacker) { + return wf.data.valid_attack_targets_by_attacker[String(attackerId)] ?? []; + } + return wf.data.valid_attack_targets ?? []; +} + +function getDefaultAttackTargetForAttacker( + state: GameState | null, + attackerId: ObjectId, +): AttackTarget | null { + return getValidAttackTargetsForAttacker(state, attackerId)[0] ?? null; +} diff --git a/crates/engine/src/ai_support/candidates.rs b/crates/engine/src/ai_support/candidates.rs index 3c31918b1d..bc2bcd507c 100644 --- a/crates/engine/src/ai_support/candidates.rs +++ b/crates/engine/src/ai_support/candidates.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use crate::game::casting; use crate::game::combat::AttackTarget; @@ -726,7 +726,14 @@ pub fn candidate_actions_broad(state: &GameState) -> Vec { player, valid_attacker_ids, valid_attack_targets, - } => attacker_actions(state, *player, valid_attacker_ids, valid_attack_targets), + valid_attack_targets_by_attacker, + } => attacker_actions( + state, + *player, + valid_attacker_ids, + valid_attack_targets, + valid_attack_targets_by_attacker, + ), WaitingFor::DeclareBlockers { player, valid_blocker_ids, @@ -3582,6 +3589,7 @@ fn attacker_actions( player: PlayerId, valid_attacker_ids: &[crate::types::identifiers::ObjectId], valid_attack_targets: &[AttackTarget], + valid_attack_targets_by_attacker: &HashMap>, ) -> Vec { // CR 508.1a: declaring no attackers is a structurally legal submission. The // engine's combat-requirement check rejects it at apply time only when a @@ -3597,7 +3605,7 @@ fn attacker_actions( Some(player), )]; - if valid_attack_targets.is_empty() { + if valid_attack_targets.is_empty() && valid_attack_targets_by_attacker.is_empty() { return actions; } @@ -3609,7 +3617,11 @@ fn attacker_actions( // target is the goader, collapsing the legal-action set to empty and // hanging the game. for &id in valid_attacker_ids { - for &target in valid_attack_targets { + let attacker_targets = valid_attack_targets_by_attacker + .get(&id) + .map(Vec::as_slice) + .unwrap_or(valid_attack_targets); + for &target in attacker_targets { actions.push(candidate( GameAction::DeclareAttackers { attacks: vec![(id, target)], @@ -3625,7 +3637,18 @@ fn attacker_actions( // Offer one per target so goad on a lone attacker doesn't make the only // all-in candidate illegal. if valid_attacker_ids.len() > 1 { - for &target in valid_attack_targets { + let common_targets: Vec = valid_attack_targets + .iter() + .copied() + .filter(|target| { + valid_attacker_ids.iter().all(|id| { + valid_attack_targets_by_attacker + .get(id) + .is_none_or(|targets| targets.contains(target)) + }) + }) + .collect(); + for &target in &common_targets { actions.push(candidate( GameAction::DeclareAttackers { attacks: valid_attacker_ids @@ -3691,7 +3714,10 @@ fn attacker_actions( let must_attack_players = crate::game::combat::must_attack_players_for_creature(state, obj); let goaders = crate::game::combat::goading_players_for_creature(state, id); - valid_attack_targets + valid_attack_targets_by_attacker + .get(&id) + .map(Vec::as_slice) + .unwrap_or(valid_attack_targets) .iter() .copied() // CR 508.1b: honor a directed MustAttackPlayer requirement first. @@ -3701,13 +3727,24 @@ fn attacker_actions( // CR 701.15b "if able": otherwise steer away from this creature's // goader. .or_else(|| { - valid_attack_targets.iter().copied().find(|target| match target { - AttackTarget::Player(pid) => !goaders.contains(pid), - _ => true, - }) + valid_attack_targets_by_attacker + .get(&id) + .map(Vec::as_slice) + .unwrap_or(valid_attack_targets) + .iter() + .copied() + .find(|target| match target { + AttackTarget::Player(pid) => !goaders.contains(pid), + _ => true, + }) }) // Fall back to any valid target when no constraint can be honored. - .or_else(|| valid_attack_targets.first().copied()) + .or_else(|| { + valid_attack_targets_by_attacker + .get(&id) + .and_then(|targets| targets.first().copied()) + .or_else(|| valid_attack_targets.first().copied()) + }) .map(|target| (id, target)) }) .collect(); @@ -4907,7 +4944,7 @@ mod tests { let targets = vec![AttackTarget::Player(PlayerId(1))]; crate::game::perf_counters::reset(); - let _ = attacker_actions(&state, PlayerId(0), &ids, &targets); + let _ = attacker_actions(&state, PlayerId(0), &ids, &targets, &HashMap::new()); let snap = crate::game::perf_counters::snapshot(); assert_eq!( @@ -5174,6 +5211,7 @@ mod tests { crate::types::identifiers::ObjectId(2), ], valid_attack_targets: vec![AttackTarget::Player(PlayerId(1))], + valid_attack_targets_by_attacker: HashMap::new(), }, ..GameState::new_two_player(42) }; @@ -5206,6 +5244,7 @@ mod tests { // The goading player is deliberately first: the pre-fix generator // would only ever offer this single (illegal-under-goad) pairing. valid_attack_targets: vec![goader, other_a, other_b], + valid_attack_targets_by_attacker: HashMap::new(), }, ..GameState::new_two_player(42) }; @@ -5274,6 +5313,7 @@ mod tests { AttackTarget::Player(PlayerId(1)), AttackTarget::Player(PlayerId(2)), ], + valid_attack_targets_by_attacker: HashMap::new(), }; let actions = candidate_actions(&state); @@ -5349,6 +5389,7 @@ mod tests { AttackTarget::Player(PlayerId(2)), AttackTarget::Player(PlayerId(1)), ], + valid_attack_targets_by_attacker: HashMap::new(), }; let actions = candidate_actions(&state); diff --git a/crates/engine/src/ai_support/mod.rs b/crates/engine/src/ai_support/mod.rs index 9359523e7e..33404c457b 100644 --- a/crates/engine/src/ai_support/mod.rs +++ b/crates/engine/src/ai_support/mod.rs @@ -1496,6 +1496,7 @@ mod tests { player: PlayerId(1), valid_attacker_ids: Vec::new(), valid_attack_targets: Vec::new(), + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; // Acting player gets the full result (matches `legal_actions_full`). let acting = legal_actions_for_viewer(&state, PlayerId(1)); diff --git a/crates/engine/src/database/synthesis.rs b/crates/engine/src/database/synthesis.rs index 7b4bd5bc0a..f637c78001 100644 --- a/crates/engine/src/database/synthesis.rs +++ b/crates/engine/src/database/synthesis.rs @@ -15531,6 +15531,7 @@ mod myriad_runtime_tests { player: PlayerId(0), valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; let card_id = CardId(state.next_object_id); diff --git a/crates/engine/src/game/combat.rs b/crates/engine/src/game/combat.rs index b88e48eaea..2a7c1d8219 100644 --- a/crates/engine/src/game/combat.rs +++ b/crates/engine/src/game/combat.rs @@ -1965,16 +1965,21 @@ fn extract_combat_tax_payload<'a>( /// override (CR 702.3b) /// - `has_summoning_sickness(obj)` (CR 302.6) pub fn creature_must_attack(state: &GameState, obj_id: ObjectId) -> bool { - let attackable_players = attackable_player_targets(state); - creature_must_attack_with_attackable_players(state, obj_id, &attackable_players) + let gates = CombatStaticGates::compute(state); + let broad_targets = get_valid_attack_targets(state); + creature_must_attack_from_broad_targets_gated(state, obj_id, &broad_targets, &gates) } pub fn attackable_player_targets(state: &GameState) -> Vec { crate::game::perf_counters::record_attackable_player_sweep(); - get_valid_attack_targets(state) - .into_iter() + attackable_players_from_targets(&get_valid_attack_targets(state)) +} + +fn attackable_players_from_targets(targets: &[AttackTarget]) -> Vec { + targets + .iter() .filter_map(|target| match target { - AttackTarget::Player(pid) => Some(pid), + AttackTarget::Player(pid) => Some(*pid), _ => None, }) .collect() @@ -2000,19 +2005,18 @@ pub(crate) fn must_attack_players_for_creature( pub fn creature_must_attack_with_attackable_players( state: &GameState, obj_id: ObjectId, - attackable_players: &[PlayerId], + _attackable_players: &[PlayerId], ) -> bool { - // Single-permanent entry: compute the loop-invariant gates once, then - // delegate. The single batch caller (`declare_attackers_with_bands`) reuses - // its already-hoisted gates via the `_gated` form below. let gates = CombatStaticGates::compute(state); - creature_must_attack_with_attackable_players_gated(state, obj_id, attackable_players, &gates) + let broad_targets = get_valid_attack_targets(state); + creature_must_attack_from_broad_targets_gated(state, obj_id, &broad_targets, &gates) } fn creature_must_attack_with_attackable_players_gated( state: &GameState, obj_id: ObjectId, attackable_players: &[PlayerId], + has_any_attack_target: bool, gates: &CombatStaticGates, ) -> bool { let Some(obj) = state.objects.get(&obj_id) else { @@ -2046,6 +2050,9 @@ fn creature_must_attack_with_attackable_players_gated( if !has_must_attack && !is_goaded && !has_attackable_must_attack_player { return false; } + if !has_any_attack_target { + return false; + } // Exemptions: tapped, defender (no override), summoning sick. // CR 508.1a: chosen attackers must be untapped. if obj.tapped { @@ -2076,6 +2083,27 @@ fn creature_must_attack_with_attackable_players_gated( true } +/// CR 508.1b + CR 508.1c + CR 508.1d + CR 701.15b: "If able" is measured +/// against this attacker's legal target set after its own scoped restrictions +/// have been applied. +fn creature_must_attack_from_broad_targets_gated( + state: &GameState, + obj_id: ObjectId, + broad_targets: &[AttackTarget], + gates: &CombatStaticGates, +) -> bool { + let base_targets = + base_valid_attack_targets_for_attacker_gated(state, obj_id, broad_targets, gates); + let attackable_players = attackable_players_from_targets(&base_targets); + creature_must_attack_with_attackable_players_gated( + state, + obj_id, + &attackable_players, + !base_targets.is_empty(), + gates, + ) +} + /// CR 702.22: Whether a creature has the banding keyword. pub fn has_banding(state: &GameState, object_id: ObjectId) -> bool { state @@ -2329,21 +2357,44 @@ pub fn declare_attackers_with_bands( if !bands.is_empty() { validate_attack_band_declarations(state, attacks, bands)?; } - let attackable_players = attackable_player_targets(state); + let broad_targets = get_valid_attack_targets(state); // CR 604.1: hoist the combat-restriction existence gates once; every // per-permanent / per-attacker static check below reuses them so each loop // stays O(N) instead of O(N^2). let gates = CombatStaticGates::compute(state); + let base_targets_by_attacker: HashMap> = state + .battlefield + .iter() + .copied() + .map(|id| { + ( + id, + base_valid_attack_targets_for_attacker_gated(state, id, &broad_targets, &gates), + ) + }) + .collect(); + let attackable_players_by_attacker: HashMap> = base_targets_by_attacker + .iter() + .map(|(id, targets)| (*id, attackable_players_from_targets(targets))) + .collect(); // CR 508.1d / CR 701.15b: Creatures that must attack each combat if able. // `creature_must_attack` is the single authority for the requirement + // exemption logic; this loop only adds the "already declared?" check and // the rejection error text. for &obj_id in &state.battlefield { + let attackable_players = attackable_players_by_attacker + .get(&obj_id) + .map(Vec::as_slice) + .unwrap_or(&[]); + let has_any_attack_target = base_targets_by_attacker + .get(&obj_id) + .is_some_and(|targets| !targets.is_empty()); if !creature_must_attack_with_attackable_players_gated( state, obj_id, - &attackable_players, + attackable_players, + has_any_attack_target, &gates, ) { continue; @@ -2435,129 +2486,44 @@ pub fn declare_attackers_with_bands( } } - // CR 508.1d: Scoped remote CantAttack (Eriette — enchanted creatures can't - // attack you or planeswalkers you control). + // CR 508.1b + CR 508.1c + CR 508.1d: Each chosen target must survive this + // creature's scoped attack restrictions and temporary "can't attack you or + // permanents you control" prohibitions. for (attacker_id, target) in attacks { - if (gates.has_cant_attack - && crate::game::static_abilities::check_static_ability( - state, - StaticMode::CantAttack, - &crate::game::static_abilities::StaticCheckContext { - target_id: Some(*attacker_id), - attack_target: Some(*target), - ..Default::default() - }, - )) - || (gates.has_cant_attack_or_block - && crate::game::static_abilities::check_static_ability( - state, - StaticMode::CantAttackOrBlock, - &crate::game::static_abilities::StaticCheckContext { - target_id: Some(*attacker_id), - attack_target: Some(*target), - ..Default::default() - }, - )) - { + if !attack_target_passes_scoped_restrictions(state, *attacker_id, *target, &gates) { return Err(format!( - "{attacker_id:?} can't attack {target:?} (CR 508.1d attack restriction)" + "{attacker_id:?} can't attack {target:?} (CR 508.1b/508.1c scoped attack restriction)" )); } } - // CR 508.1c + CR 109.5: Player-scoped temporary attack prohibitions - // (`GameRestriction::ProhibitActivity { activity: Attack { defended } }` — - // Willie Lumpkin: "that player can't attack you or permanents you control - // during their next turn"). Each restriction defends a specific player (the - // grant's controller per CR 109.5) against the affected players. Reuse the - // SAME `attack_target_matches_defended_scope` authority static `CantAttack` - // uses, so both seams share one scope matcher. + // CR 701.15b: a goaded creature must attack a player other than the goading + // player *if able*. "Able" is measured against the same per-attacker base + // target set the UI uses: global defenders filtered by this creature's own + // scoped attack restrictions. for (attacker_id, target) in attacks { - let Some(attacker_controller) = state.objects.get(attacker_id).map(|o| o.controller) else { + let goading_players = + goading_players_for_creature_gated(state, *attacker_id, gates.has_goad); + if goading_players.is_empty() { continue; - }; - for restriction in &state.restrictions { - let crate::types::ability::GameRestriction::ProhibitActivity { - source, - affected_players, - activity: crate::types::ability::ProhibitedActivity::Attack { defended }, - .. - } = restriction - else { - continue; - }; - // CR 109.5: the protected player ("you") is the grant's controller. - let Some(protected) = state.objects.get(source).map(|o| o.controller) else { - continue; - }; - // CR 101.2: only the affected players are prohibited. Targeted scopes - // are resolved to `SpecificPlayer` by `add_restriction` before this - // gate ever runs. - let attacker_is_affected = match affected_players { - crate::types::ability::RestrictionPlayerScope::AllPlayers => true, - crate::types::ability::RestrictionPlayerScope::SpecificPlayer(p) => { - *p == attacker_controller - } - crate::types::ability::RestrictionPlayerScope::OpponentsOfSourceController => { - attacker_controller != protected - } - crate::types::ability::RestrictionPlayerScope::TargetedPlayer - | crate::types::ability::RestrictionPlayerScope::ParentTargetedPlayer - | crate::types::ability::RestrictionPlayerScope::DefendingPlayer - // CR 109.5: resolved to `SpecificPlayer` by `add_restriction` at - // creation time, so an unresolved scope here restricts no one. - | crate::types::ability::RestrictionPlayerScope::ScopedPlayer => false, - }; - if !attacker_is_affected { - continue; - } - // CR 508.5: the defended planeswalker/battle compares on controller, - // so pass `protected` as both source-controller and source-owner. - if crate::game::restrictions::attack_target_matches_defended_scope( - state, - Some(target), - defended, - protected, - protected, - ) { - return Err(format!( - "{attacker_id:?} can't attack {target:?} (CR 508.1c player-scoped attack prohibition)" - )); - } } - } - - // CR 701.15b: a goaded creature must attack a player other than the goading - // player *if able*. "Able" is measured against the players this creature - // could legally be declared attacking: `get_valid_attack_targets` already - // applies CR 506.2/506.3 plus the exclusions for the active player, - // eliminated players, teammates, phased-out players, and players with - // protection from everything. A non-goading player who is not a legal attack - // target (e.g. a phased-out opponent or a teammate) does not make the - // creature able to attack elsewhere, so attacking a goading player stays - // legal. The previous check counted every non-eliminated player, wrongly - // forcing the creature off a goading player toward a target it could not - // actually attack. - for (attacker_id, target) in attacks { - if let AttackTarget::Player(defending_pid) = target { - let goading_players = - goading_players_for_creature_gated(state, *attacker_id, gates.has_goad); - if goading_players.is_empty() { - continue; - } - // Only enforce the redirect if a non-goading player is actually a - // legal attack target for this creature. - if goading_players.contains(defending_pid) { - let has_attackable_non_goading_target = attackable_players - .iter() - .any(|pid| !goading_players.contains(pid)); - if has_attackable_non_goading_target { - return Err(format!( - "{:?} is goaded by {:?} and must attack a different player if able (CR 701.15b)", - attacker_id, defending_pid - )); - } - } + let attackable_players = attackable_players_by_attacker + .get(attacker_id) + .map(Vec::as_slice) + .unwrap_or(&[]); + let has_attackable_non_goading_target = attackable_players + .iter() + .any(|pid| !goading_players.contains(pid)); + if has_attackable_non_goading_target + && !matches!( + target, + AttackTarget::Player(pid) if !goading_players.contains(pid) + ) + { + return Err(format!( + "{:?} is goaded and must attack a different player directly if able (CR 701.15b)", + attacker_id + )); } } @@ -2570,6 +2536,10 @@ pub fn declare_attackers_with_bands( let Some(obj) = state.objects.get(attacker_id) else { continue; }; + let attackable_players = attackable_players_by_attacker + .get(attacker_id) + .map(Vec::as_slice) + .unwrap_or(&[]); for req_player in must_attack_players_for_creature(state, obj) { // Only enforce when the creature can legally attack the required player. if !attackable_players.contains(&req_player) { @@ -2902,68 +2872,12 @@ pub fn has_summoning_sickness(obj: &GameObject) -> bool { /// CR 508.1a / CR 302.6: Untapped creature controlled since turn started, without Defender. /// CR 702.26b: Phased-out creatures can't attack. pub fn get_valid_attacker_ids(state: &GameState) -> Vec { - let active = state.active_player; - // CR 604.1: hoist the combat-restriction existence gates once before the - // per-permanent scan (collapses O(N^2) to O(N)). let gates = CombatStaticGates::compute(state); + let broad_targets = get_valid_attack_targets(state); - state - .battlefield_phased_in_ids() - .iter() - .filter_map(|id| { - let obj = state.objects.get(id)?; - if obj.controller == active - && obj.card_types.core_types.contains(&CoreType::Creature) - && !obj.tapped - && (!obj.has_keyword(&Keyword::Defender) - || super::functioning_abilities::active_static_definitions(state, obj) - .any(|sd| sd.mode == StaticMode::CanAttackWithDefender) - || (gates.has_can_attack_with_defender - && crate::game::static_abilities::check_static_ability( - state, - StaticMode::CanAttackWithDefender, - &crate::game::static_abilities::StaticCheckContext { - target_id: Some(*id), - ..Default::default() - }, - ))) - && !super::functioning_abilities::active_static_definitions(state, obj).any(|sd| { - matches!( - sd.mode, - StaticMode::CantAttack | StaticMode::CantAttackOrBlock - ) && sd.attack_defended.is_none() - }) - // CR 508.1 + CR 101.2 + CR 109.5: remote CantAttack statics - // (Angelic Arbiter restricting opponents' creatures) resolved via - // the shared `check_static_ability` building block. - && !(gates.has_cant_attack - && crate::game::static_abilities::check_static_ability( - state, - StaticMode::CantAttack, - &crate::game::static_abilities::StaticCheckContext { - target_id: Some(*id), - ..Default::default() - }, - )) - && !(gates.has_cant_attack_or_block - && crate::game::static_abilities::check_static_ability( - state, - StaticMode::CantAttackOrBlock, - &crate::game::static_abilities::StaticCheckContext { - target_id: Some(*id), - ..Default::default() - }, - )) - // CR 302.6: delegate to the single authority for summoning - // sickness — folds in Haste at query time without duplicating - // the flag/keyword logic here. - && !has_summoning_sickness(obj) - { - Some(*id) - } else { - None - } - }) + valid_attack_targets_by_attacker_gated(state, &broad_targets, &gates) + .into_iter() + .map(|(id, _)| id) .collect() } @@ -2979,17 +2893,21 @@ pub fn get_valid_attacker_ids(state: &GameState) -> Vec { pub fn refresh_combat_declaration_waiting_for(state: &mut GameState) { match &state.waiting_for { crate::types::game_state::WaitingFor::DeclareAttackers { .. } => { - // CR 508.1a: Mirror turns.rs:1369-1370 — rebuild both payload fields. + // CR 508.1a: Mirror turns.rs:1369-1372 — rebuild the attacker and + // target payloads from the live legality queries. let valid_attacker_ids = get_valid_attacker_ids(state); let valid_attack_targets = get_valid_attack_targets(state); + let valid_attack_targets_by_attacker = get_valid_attack_targets_by_attacker(state); if let crate::types::game_state::WaitingFor::DeclareAttackers { valid_attacker_ids: ids, valid_attack_targets: targets, + valid_attack_targets_by_attacker: targets_by_attacker, .. } = &mut state.waiting_for { *ids = valid_attacker_ids; *targets = valid_attack_targets; + *targets_by_attacker = valid_attack_targets_by_attacker; } } crate::types::game_state::WaitingFor::DeclareBlockers { player, .. } => { @@ -3679,6 +3597,246 @@ pub fn get_valid_attack_targets(state: &GameState) -> Vec { targets } +/// CR 508.1a + CR 508.1c + CR 702.26b: An attacker must be an untapped, +/// phased-in creature the active player can legally declare as an attacker. +fn attacker_is_generically_eligible_to_attack( + state: &GameState, + attacker_id: ObjectId, + gates: &CombatStaticGates, +) -> bool { + let Some(obj) = state.objects.get(&attacker_id) else { + return false; + }; + obj.controller == state.active_player + && obj.zone == Zone::Battlefield + && !obj.is_phased_out() + && obj.card_types.core_types.contains(&CoreType::Creature) + && !obj.tapped + && (!obj.has_keyword(&Keyword::Defender) + || super::functioning_abilities::active_static_definitions(state, obj) + .any(|sd| sd.mode == StaticMode::CanAttackWithDefender) + || (gates.has_can_attack_with_defender + && crate::game::static_abilities::check_static_ability( + state, + StaticMode::CanAttackWithDefender, + &crate::game::static_abilities::StaticCheckContext { + target_id: Some(attacker_id), + ..Default::default() + }, + ))) + && !super::functioning_abilities::active_static_definitions(state, obj).any(|sd| { + matches!( + sd.mode, + StaticMode::CantAttack | StaticMode::CantAttackOrBlock + ) && sd.attack_defended.is_none() + }) + && !(gates.has_cant_attack + && crate::game::static_abilities::check_static_ability( + state, + StaticMode::CantAttack, + &crate::game::static_abilities::StaticCheckContext { + target_id: Some(attacker_id), + ..Default::default() + }, + )) + && !(gates.has_cant_attack_or_block + && crate::game::static_abilities::check_static_ability( + state, + StaticMode::CantAttackOrBlock, + &crate::game::static_abilities::StaticCheckContext { + target_id: Some(attacker_id), + ..Default::default() + }, + )) + && !has_summoning_sickness(obj) +} + +/// CR 508.1b + CR 508.1c: Each declared attacker must have a legal defending +/// player or battle after applying scoped attack restrictions. +fn attack_target_passes_scoped_restrictions( + state: &GameState, + attacker_id: ObjectId, + target: AttackTarget, + gates: &CombatStaticGates, +) -> bool { + if (gates.has_cant_attack + && crate::game::static_abilities::check_static_ability( + state, + StaticMode::CantAttack, + &crate::game::static_abilities::StaticCheckContext { + target_id: Some(attacker_id), + attack_target: Some(target), + ..Default::default() + }, + )) + || (gates.has_cant_attack_or_block + && crate::game::static_abilities::check_static_ability( + state, + StaticMode::CantAttackOrBlock, + &crate::game::static_abilities::StaticCheckContext { + target_id: Some(attacker_id), + attack_target: Some(target), + ..Default::default() + }, + )) + { + return false; + } + + let Some(attacker_controller) = state.objects.get(&attacker_id).map(|o| o.controller) else { + return false; + }; + for restriction in &state.restrictions { + let crate::types::ability::GameRestriction::ProhibitActivity { + source, + affected_players, + activity: crate::types::ability::ProhibitedActivity::Attack { defended }, + .. + } = restriction + else { + continue; + }; + let Some(protected) = state.objects.get(source).map(|o| o.controller) else { + continue; + }; + let attacker_is_affected = match affected_players { + crate::types::ability::RestrictionPlayerScope::AllPlayers => true, + crate::types::ability::RestrictionPlayerScope::SpecificPlayer(p) => { + *p == attacker_controller + } + crate::types::ability::RestrictionPlayerScope::OpponentsOfSourceController => { + attacker_controller != protected + } + crate::types::ability::RestrictionPlayerScope::TargetedPlayer + | crate::types::ability::RestrictionPlayerScope::ParentTargetedPlayer + | crate::types::ability::RestrictionPlayerScope::DefendingPlayer + | crate::types::ability::RestrictionPlayerScope::ScopedPlayer => false, + }; + if !attacker_is_affected { + continue; + } + if crate::game::restrictions::attack_target_matches_defended_scope( + state, + Some(&target), + defended, + protected, + protected, + ) { + return false; + } + } + + true +} + +/// CR 508.1b + CR 508.1c: Start from globally attackable defenders, then +/// remove attack targets this creature is individually restricted from +/// attacking. +fn base_valid_attack_targets_for_attacker_gated( + state: &GameState, + attacker_id: ObjectId, + broad_targets: &[AttackTarget], + gates: &CombatStaticGates, +) -> Vec { + if !attacker_is_generically_eligible_to_attack(state, attacker_id, gates) { + return Vec::new(); + } + + broad_targets + .iter() + .copied() + .filter(|target| { + attack_target_passes_scoped_restrictions(state, attacker_id, *target, gates) + }) + .collect() +} + +/// CR 508.1d + CR 701.15b: Attack requirements and goad further narrow the +/// legal attack targets for a specific attacker. +fn valid_attack_targets_for_attacker_from_base( + state: &GameState, + attacker_id: ObjectId, + base_targets: &[AttackTarget], + gates: &CombatStaticGates, +) -> Vec { + let Some(obj) = state.objects.get(&attacker_id) else { + return Vec::new(); + }; + let attackable_players = attackable_players_from_targets(base_targets); + let goading_players = goading_players_for_creature_gated(state, attacker_id, gates.has_goad); + let has_attackable_non_goading_player = attackable_players + .iter() + .any(|pid| !goading_players.contains(pid)); + let required_players: Vec = must_attack_players_for_creature(state, obj) + .into_iter() + .filter(|player| attackable_players.contains(player)) + .collect(); + + base_targets + .iter() + .copied() + .filter(|target| { + if has_attackable_non_goading_player + && !matches!( + target, + AttackTarget::Player(pid) if !goading_players.contains(pid) + ) + { + return false; + } + if required_players.is_empty() { + return true; + } + matches!(target, AttackTarget::Player(pid) if required_players.contains(pid)) + }) + .collect() +} + +/// CR 508.1a + CR 508.1b + CR 508.1c + CR 508.1d: Enumerate the legal attack +/// targets for each attacker during declaration. +fn valid_attack_targets_by_attacker_gated( + state: &GameState, + broad_targets: &[AttackTarget], + gates: &CombatStaticGates, +) -> Vec<(ObjectId, Vec)> { + state + .battlefield_phased_in_ids() + .iter() + .filter_map(|id| { + let base_targets = + base_valid_attack_targets_for_attacker_gated(state, *id, broad_targets, gates); + let targets = + valid_attack_targets_for_attacker_from_base(state, *id, &base_targets, gates); + (!targets.is_empty()).then_some((*id, targets)) + }) + .collect() +} + +/// CR 508.1a + CR 508.1b + CR 508.1c + CR 508.1d: Legal attack targets for +/// each attacker during declaration. +pub fn get_valid_attack_targets_by_attacker( + state: &GameState, +) -> HashMap> { + let gates = CombatStaticGates::compute(state); + let broad_targets = get_valid_attack_targets(state); + valid_attack_targets_by_attacker_gated(state, &broad_targets, &gates) + .into_iter() + .collect() +} + +/// CR 508.1a + CR 508.1b + CR 508.1c + CR 508.1d: Legal attack targets for one +/// attacker during declaration. +pub fn get_valid_attack_targets_for_attacker( + state: &GameState, + attacker_id: ObjectId, +) -> Vec { + let gates = CombatStaticGates::compute(state); + let broad_targets = get_valid_attack_targets(state); + let base_targets = + base_valid_attack_targets_for_attacker_gated(state, attacker_id, &broad_targets, &gates); + valid_attack_targets_for_attacker_from_base(state, attacker_id, &base_targets, &gates) +} + /// CR 506.4: A creature stops being an attacker when it leaves the battlefield /// or phases out. Attackers that left during the declare-attackers step may /// remain listed until pruned. @@ -4365,6 +4523,7 @@ mod tests { player: PlayerId(0), valid_attacker_ids: get_valid_attacker_ids(&state), valid_attack_targets: get_valid_attack_targets(&state), + valid_attack_targets_by_attacker: get_valid_attack_targets_by_attacker(&state), }; match &state.waiting_for { WaitingFor::DeclareAttackers { @@ -4432,6 +4591,7 @@ mod tests { player: PlayerId(0), valid_attacker_ids: get_valid_attacker_ids(&state), valid_attack_targets: get_valid_attack_targets(&state), + valid_attack_targets_by_attacker: get_valid_attack_targets_by_attacker(&state), }; match &state.waiting_for { WaitingFor::DeclareAttackers { @@ -8000,6 +8160,62 @@ mod tests { assert!(attacks_non_owner.is_ok()); } + #[test] + fn valid_attack_targets_by_attacker_filters_scoped_attack_lockouts() { + let mut state = setup_multiplayer_combat(3); + let attacker = create_creature(&mut state, PlayerId(1), "Owner-Restricted Bear", 2, 2); + { + let obj = state.objects.get_mut(&attacker).unwrap(); + obj.controller = PlayerId(0); + obj.static_definitions.push( + StaticDefinition::new(StaticMode::CantAttack) + .affected(TargetFilter::SelfRef) + .attack_defended(Some(crate::types::triggers::AttackTargetFilter::Owner)), + ); + } + + let global_targets = get_valid_attack_targets(&state); + assert_eq!( + global_targets, + vec![ + AttackTarget::Player(PlayerId(1)), + AttackTarget::Player(PlayerId(2)), + ], + "global declare-attackers targets still list both opponents" + ); + + let by_attacker = get_valid_attack_targets_by_attacker(&state); + assert_eq!( + by_attacker.get(&attacker), + Some(&vec![AttackTarget::Player(PlayerId(2))]), + "per-attacker targets must exclude the owner-side target this creature can't attack" + ); + } + + #[test] + fn valid_attacker_ids_drop_creatures_with_no_legal_attack_target() { + let mut state = setup(); + let attacker = create_creature(&mut state, PlayerId(1), "Stranded Bear", 2, 2); + { + let obj = state.objects.get_mut(&attacker).unwrap(); + obj.controller = PlayerId(0); + obj.static_definitions.push( + StaticDefinition::new(StaticMode::CantAttack) + .affected(TargetFilter::SelfRef) + .attack_defended(Some(crate::types::triggers::AttackTargetFilter::Owner)), + ); + } + + assert!( + get_valid_attack_targets_for_attacker(&state, attacker).is_empty(), + "the only opponent is this creature's owner, so it has no legal attack target" + ); + assert!( + !get_valid_attacker_ids(&state).contains(&attacker), + "declare-attackers UI must not surface attackers with no legal target" + ); + } + #[test] fn cant_attack_owner_or_planeswalker_blocks_owner_side_targets() { let mut state = setup_multiplayer_combat(3); @@ -8195,6 +8411,35 @@ mod tests { ); } + #[test] + fn goad_requires_attacking_non_goading_player_not_their_planeswalker() { + // CR 701.15b: when a non-goading player is directly attackable, attacking + // that player's planeswalker does not satisfy "attack a different player + // if able." + let mut state = setup_multiplayer_combat(3); + let goaded = create_goaded_creature(&mut state, PlayerId(0), PlayerId(1)); + let p2_walker = create_planeswalker(&mut state, PlayerId(2), "P2 Walker"); + + assert_eq!( + get_valid_attack_targets_for_attacker(&state, goaded), + vec![AttackTarget::Player(PlayerId(2))], + "goad should narrow legal targets to direct non-goading players only" + ); + + let result = declare_attackers( + &mut state, + &[(goaded, AttackTarget::Planeswalker(p2_walker))], + &mut vec![], + ); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .contains("must attack a different player directly if able"), + "attacking P2's planeswalker must not satisfy goad while P2 is attackable" + ); + } + #[test] fn cant_be_blocked_except_by_enforces_filter() { use crate::parser::oracle_target::parse_target; diff --git a/crates/engine/src/game/engine_phase_trigger_regression_tests.rs b/crates/engine/src/game/engine_phase_trigger_regression_tests.rs index 59cf8a2458..ef3215cd6a 100644 --- a/crates/engine/src/game/engine_phase_trigger_regression_tests.rs +++ b/crates/engine/src/game/engine_phase_trigger_regression_tests.rs @@ -1270,6 +1270,7 @@ fn attack_trigger_resolves_before_combat_damage_and_only_once() { player: PlayerId(0), valid_attacker_ids: vec![ajani, linden], valid_attack_targets: vec![AttackTarget::Player(PlayerId(1))], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; let declare_result = apply_as_current( @@ -1462,6 +1463,7 @@ fn lifelink_replacement_does_not_double_fire_life_gain_triggers() { player: PlayerId(0), valid_attacker_ids: vec![bat], valid_attack_targets: vec![AttackTarget::Player(PlayerId(1))], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; apply_as_current( @@ -4105,6 +4107,7 @@ fn declare_blockers_grants_ap_priority_when_no_legal_blockers() { player: PlayerId(0), valid_attacker_ids: vec![attacker], valid_attack_targets: vec![AttackTarget::Player(PlayerId(1))], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; apply_as_current( diff --git a/crates/engine/src/game/engine_tests.rs b/crates/engine/src/game/engine_tests.rs index 024bc046eb..9801589830 100644 --- a/crates/engine/src/game/engine_tests.rs +++ b/crates/engine/src/game/engine_tests.rs @@ -1265,6 +1265,7 @@ fn broadside_bombardiers_boast_activates_after_attacking_and_requires_sacrifice( player: PlayerId(0), valid_attacker_ids: vec![bombardiers], valid_attack_targets: vec![AttackTarget::Player(PlayerId(1))], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; apply_as_current( &mut state, @@ -1761,6 +1762,7 @@ fn concede_owner_of_waiting_for_advances_state() { player: PlayerId(1), valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; let result = apply_as_current( @@ -5026,6 +5028,7 @@ fn setup_tempest_hawk_attack(library_hawk_ids: &[u64]) -> (GameState, ObjectId, player: PlayerId(0), valid_attacker_ids: vec![attacker], valid_attack_targets: vec![AttackTarget::Player(PlayerId(1))], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; apply_as_current( diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index feb1e695a7..becd0b1989 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -20046,6 +20046,7 @@ pub mod tests { player: PlayerId(0), valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; // Boggart Prankster trigger: Whenever you attack, target attacking @@ -20358,6 +20359,7 @@ pub mod tests { player: PlayerId(0), valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; // The Kratos-like commander carries the trigger but does not attack. @@ -20445,6 +20447,7 @@ pub mod tests { player: PlayerId(0), valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; let source = make_creature(&mut state, PlayerId(0), "Subject Source", 1, 1); @@ -20731,6 +20734,7 @@ pub mod tests { player: PlayerId(0), valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; // The Tenth Doctor — attacking creature carrying the Allons-y! trigger. @@ -20976,6 +20980,7 @@ pub mod tests { player: PlayerId(0), valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; // Raph & Mikey on P0's battlefield, carrying its real Attacks trigger. @@ -21129,6 +21134,7 @@ pub mod tests { player: PlayerId(0), valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; // CR 104.3c: stock both libraries so neither player decks out while the diff --git a/crates/engine/src/game/turns.rs b/crates/engine/src/game/turns.rs index d809f79f36..2141c34be2 100644 --- a/crates/engine/src/game/turns.rs +++ b/crates/engine/src/game/turns.rs @@ -2093,10 +2093,13 @@ pub fn auto_advance(state: &mut GameState, events: &mut Vec) -> Waiti // CR 508.1: Active player declares attackers as a turn-based action. let valid_attacker_ids = super::combat::get_valid_attacker_ids(state); let valid_attack_targets = super::combat::get_valid_attack_targets(state); + let valid_attack_targets_by_attacker = + super::combat::get_valid_attack_targets_by_attacker(state); return WaitingFor::DeclareAttackers { player: state.active_player, valid_attacker_ids, valid_attack_targets, + valid_attack_targets_by_attacker, }; } Phase::DeclareBlockers => { diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 424bac7626..79d039526f 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -3008,6 +3008,8 @@ pub enum WaitingFor { valid_attacker_ids: Vec, #[serde(default)] valid_attack_targets: Vec, + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + valid_attack_targets_by_attacker: HashMap>, }, DeclareBlockers { player: PlayerId, @@ -9108,6 +9110,7 @@ mod tests { player: PlayerId(0), valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: HashMap::new(), })); variants.push(Box::new(WaitingFor::DeclareBlockers { player: PlayerId(0), diff --git a/crates/engine/tests/integration/emissary_green.rs b/crates/engine/tests/integration/emissary_green.rs index 1b8a69a8be..590227fd1f 100644 --- a/crates/engine/tests/integration/emissary_green.rs +++ b/crates/engine/tests/integration/emissary_green.rs @@ -69,6 +69,7 @@ fn attack_with_emissary() -> (GameRunner, ObjectId, ObjectId) { player: P0, valid_attacker_ids: vec![emissary], valid_attack_targets: vec![AttackTarget::Player(P1)], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; } runner diff --git a/crates/engine/tests/integration/giggling_skitterspike_issue_890.rs b/crates/engine/tests/integration/giggling_skitterspike_issue_890.rs index 968ed122cc..b365b0b94f 100644 --- a/crates/engine/tests/integration/giggling_skitterspike_issue_890.rs +++ b/crates/engine/tests/integration/giggling_skitterspike_issue_890.rs @@ -35,6 +35,7 @@ fn issue_890_attack_trigger_deals_damage() { player: P0, valid_attacker_ids: vec![skitterspike], valid_attack_targets: vec![AttackTarget::Player(P1)], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; } @@ -69,6 +70,7 @@ fn issue_890_block_trigger_deals_damage() { player: P1, valid_attacker_ids: vec![attacker], valid_attack_targets: vec![AttackTarget::Player(P0)], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; } diff --git a/crates/engine/tests/integration/issue_1297_venture_initiative_triggers.rs b/crates/engine/tests/integration/issue_1297_venture_initiative_triggers.rs index 0c4febb077..b2b18d0bf8 100644 --- a/crates/engine/tests/integration/issue_1297_venture_initiative_triggers.rs +++ b/crates/engine/tests/integration/issue_1297_venture_initiative_triggers.rs @@ -196,6 +196,7 @@ fn initiative_attack_trigger_skips_without_initiative() { player: P0, valid_attacker_ids: vec![attacker], valid_attack_targets: vec![AttackTarget::Player(P1)], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; let hand_before = runner.state().players[0].hand.len(); @@ -231,6 +232,7 @@ fn initiative_attack_trigger_draws_with_initiative() { player: P0, valid_attacker_ids: vec![attacker], valid_attack_targets: vec![AttackTarget::Player(P1)], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; let hand_before = runner.state().players[0].hand.len(); diff --git a/crates/engine/tests/integration/rules/combat.rs b/crates/engine/tests/integration/rules/combat.rs index f16dd18352..294d2de6d3 100644 --- a/crates/engine/tests/integration/rules/combat.rs +++ b/crates/engine/tests/integration/rules/combat.rs @@ -1177,6 +1177,7 @@ fn build_3p_propaganda_scenario(propaganda_owner: PlayerId) -> (GameRunner, Obje player: P1, valid_attacker_ids: vec![attacker], valid_attack_targets: vec![AttackTarget::Player(P0), AttackTarget::Player(PlayerId(2))], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; (runner, attacker) } diff --git a/crates/engine/tests/squirrel_perf_probe.rs b/crates/engine/tests/squirrel_perf_probe.rs index 8c4cb100c6..b956d7bb35 100644 --- a/crates/engine/tests/squirrel_perf_probe.rs +++ b/crates/engine/tests/squirrel_perf_probe.rs @@ -101,6 +101,7 @@ fn probe_squirrel_board() { player: active, valid_attacker_ids, valid_attack_targets, + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; measure("declare-attackers", &da); } diff --git a/crates/manabrew-compat/src/lib.rs b/crates/manabrew-compat/src/lib.rs index b1f268faf8..aec98395fd 100644 --- a/crates/manabrew-compat/src/lib.rs +++ b/crates/manabrew-compat/src/lib.rs @@ -2183,6 +2183,7 @@ mod tests { player: PlayerId(0), valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }, ), ( diff --git a/crates/phase-ai/src/bin/attack_scaling_bench.rs b/crates/phase-ai/src/bin/attack_scaling_bench.rs index 4a71958615..c7b6e8b4d8 100644 --- a/crates/phase-ai/src/bin/attack_scaling_bench.rs +++ b/crates/phase-ai/src/bin/attack_scaling_bench.rs @@ -60,6 +60,7 @@ fn bench_n(n: usize) { player: P0, valid_attacker_ids, valid_attack_targets, + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; let at_declare = matches!( diff --git a/crates/phase-ai/src/bin/declare_attackers_bench.rs b/crates/phase-ai/src/bin/declare_attackers_bench.rs index ea70b0731f..175fb7d2c2 100644 --- a/crates/phase-ai/src/bin/declare_attackers_bench.rs +++ b/crates/phase-ai/src/bin/declare_attackers_bench.rs @@ -107,6 +107,7 @@ fn main() { player: active, valid_attacker_ids: valid_attacker_ids.clone(), valid_attack_targets: valid_attack_targets.clone(), + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; // ── 2. Generic candidate path (frontend legal-actions / SimulationFilter) ─ diff --git a/crates/phase-ai/src/combat_ai.rs b/crates/phase-ai/src/combat_ai.rs index 7dc2c43aa7..f7039ee0ef 100644 --- a/crates/phase-ai/src/combat_ai.rs +++ b/crates/phase-ai/src/combat_ai.rs @@ -135,13 +135,17 @@ pub fn choose_attackers_with_targets( state: &GameState, player: PlayerId, ) -> Vec<(ObjectId, AttackTarget)> { + let valid_attack_targets = engine::game::combat::get_valid_attack_targets(state); + let valid_attack_targets_by_attacker = + engine::game::combat::get_valid_attack_targets_by_attacker(state); choose_attackers_with_targets_with_profile( state, player, &AiProfile::default(), false, None, - None, + Some(&valid_attack_targets), + Some(&valid_attack_targets_by_attacker), ) } @@ -152,6 +156,7 @@ pub fn choose_attackers_with_targets_with_profile( combat_lookahead: bool, valid_attacker_ids: Option<&[ObjectId]>, valid_attack_targets: Option<&[AttackTarget]>, + valid_attack_targets_by_attacker: Option<&HashMap>>, ) -> Vec<(ObjectId, AttackTarget)> { let opponents = players::opponents(state, player); if opponents.is_empty() { @@ -162,6 +167,19 @@ pub fn choose_attackers_with_targets_with_profile( // local can_attack() for tests and hypothetical scenarios. let candidates: Vec = if let Some(ids) = valid_attacker_ids { ids.to_vec() + } else if let Some(targets_by_attacker) = valid_attack_targets_by_attacker { + state + .battlefield + .iter() + .copied() + .filter(|id| { + targets_by_attacker.contains_key(id) + && state + .objects + .get(id) + .is_some_and(|obj| obj.controller == player) + }) + .collect() } else { state .battlefield @@ -419,6 +437,7 @@ pub fn choose_attackers_with_targets_with_profile( state, &attacking_ids, valid_attack_targets, + valid_attack_targets_by_attacker, objective, opp, opponent_life, @@ -428,11 +447,57 @@ pub fn choose_attackers_with_targets_with_profile( } // Multi-opponent: assign attack targets (planeswalker redirect deferred). - let assignments = assign_attack_targets(state, player, &opponents, attacking_ids); + let assignments = assign_attack_targets( + state, + player, + &opponents, + attacking_ids, + valid_attack_targets, + valid_attack_targets_by_attacker, + ); emit_attack_trace(player, &candidates, &assignments); assignments } +fn legal_attack_targets_for_attacker<'a>( + attacker_id: ObjectId, + valid_attack_targets: Option<&'a [AttackTarget]>, + valid_attack_targets_by_attacker: Option<&'a HashMap>>, +) -> Option<&'a [AttackTarget]> { + valid_attack_targets_by_attacker + .and_then(|targets| targets.get(&attacker_id).map(Vec::as_slice)) + .or(valid_attack_targets) +} + +fn preferred_attack_target_for_attacker( + attacker_id: ObjectId, + preferred_players: &[PlayerId], + valid_attack_targets: Option<&[AttackTarget]>, + valid_attack_targets_by_attacker: Option<&HashMap>>, +) -> Option { + let legal_targets = legal_attack_targets_for_attacker( + attacker_id, + valid_attack_targets, + valid_attack_targets_by_attacker, + )?; + + preferred_players + .iter() + .find_map(|pid| { + legal_targets + .iter() + .copied() + .find(|target| *target == AttackTarget::Player(*pid)) + }) + .or_else(|| { + legal_targets + .iter() + .copied() + .find(|target| matches!(target, AttackTarget::Player(_))) + }) + .or_else(|| legal_targets.first().copied()) +} + /// Single-opponent planeswalker redirect (CR 508.1: legality of attacking a /// planeswalker is decided by the engine, which surfaces every legal target in /// `valid_attack_targets` — this only *chooses* among them). @@ -448,6 +513,7 @@ fn redirect_attackers_to_planeswalker( state: &GameState, attacking_ids: &[ObjectId], valid_attack_targets: Option<&[AttackTarget]>, + valid_attack_targets_by_attacker: Option<&HashMap>>, objective: CombatObjective, opponent: PlayerId, opponent_life: i32, @@ -456,7 +522,18 @@ fn redirect_attackers_to_planeswalker( let all_at_player = || -> Vec<(ObjectId, AttackTarget)> { attacking_ids .iter() - .map(|&id| (id, player_target)) + .map(|&id| { + ( + id, + preferred_attack_target_for_attacker( + id, + &[opponent], + valid_attack_targets, + valid_attack_targets_by_attacker, + ) + .unwrap_or(player_target), + ) + }) .collect() }; @@ -473,16 +550,23 @@ fn redirect_attackers_to_planeswalker( } // Highest-loyalty attackable opponent planeswalker from the engine's list. - let Some(targets) = valid_attack_targets else { - return all_at_player(); - }; - let best_pw = targets + let best_pw = attacking_ids .iter() - .filter_map(|t| match t { - AttackTarget::Planeswalker(id) => { - let loyalty = state.objects.get(id)?.loyalty.unwrap_or(0); - (loyalty > 0).then_some((*id, loyalty as i32)) - } + .filter_map(|&attacker_id| { + legal_attack_targets_for_attacker( + attacker_id, + valid_attack_targets, + valid_attack_targets_by_attacker, + ) + }) + .flatten() + .filter_map(|target| match target { + AttackTarget::Planeswalker(id) => state + .objects + .get(id) + .and_then(|obj| obj.loyalty) + .filter(|loyalty| *loyalty > 0) + .map(|loyalty| (*id, loyalty as i32)), _ => None, }) .max_by_key(|&(_, loyalty)| loyalty); @@ -493,6 +577,14 @@ fn redirect_attackers_to_planeswalker( // Largest-power-first: the fewest big attackers that sum to >= loyalty. let mut by_power: Vec<(ObjectId, i32)> = attacking_ids .iter() + .filter(|&&id| { + legal_attack_targets_for_attacker( + id, + valid_attack_targets, + valid_attack_targets_by_attacker, + ) + .is_some_and(|targets| targets.contains(&AttackTarget::Planeswalker(pw_id))) + }) .filter_map(|&id| Some((id, state.objects.get(&id)?.power.unwrap_or(0)))) .collect(); by_power.sort_by_key(|b| std::cmp::Reverse(b.1)); @@ -513,16 +605,32 @@ fn redirect_attackers_to_planeswalker( } let pw_target = AttackTarget::Planeswalker(pw_id); - attacking_ids + let assignments: Vec<_> = attacking_ids .iter() .map(|&id| { if redirected.contains(&id) { (id, pw_target) } else { - (id, player_target) + ( + id, + preferred_attack_target_for_attacker( + id, + &[opponent], + valid_attack_targets, + valid_attack_targets_by_attacker, + ) + .unwrap_or(player_target), + ) } }) - .collect() + .collect(); + if !assignments + .iter() + .any(|(_, target)| *target == player_target) + { + return all_at_player(); + } + assignments } fn preferred_attack_opponent( @@ -559,15 +667,11 @@ fn assign_attack_targets( player: PlayerId, opponents: &[PlayerId], attacking_ids: Vec, + valid_attack_targets: Option<&[AttackTarget]>, + valid_attack_targets_by_attacker: Option<&HashMap>>, ) -> Vec<(ObjectId, AttackTarget)> { let threat_ranked = threat_ranked_opponents(state, player, opponents); - let total_power: i32 = attacking_ids - .iter() - .filter_map(|&id| state.objects.get(&id)) - .map(|obj| obj.power.unwrap_or(0)) - .sum(); - // Check for alpha-strike: can we eliminate the weakest opponent? let weakest = opponents .iter() @@ -576,10 +680,21 @@ fn assign_attack_targets( if let Some(weak_opp) = weakest { let weak_life = state.players[weak_opp.0 as usize].life; - if weak_life > 0 && total_power >= weak_life { + let attack_power_vs_weakest: i32 = attacking_ids + .iter() + .filter(|&&id| { + preferred_attack_target_for_attacker( + id, + &[weak_opp], + valid_attack_targets, + valid_attack_targets_by_attacker, + ) == Some(AttackTarget::Player(weak_opp)) + }) + .filter_map(|&id| state.objects.get(&id).map(|obj| obj.power.unwrap_or(0))) + .sum(); + if weak_life > 0 && attack_power_vs_weakest >= weak_life { // Send enough to kill the weakest, rest to highest threat let target_weak = AttackTarget::Player(weak_opp); - let primary_target = AttackTarget::Player(threat_ranked[0].0); let mut result = Vec::new(); let mut allocated_power = 0; @@ -590,16 +705,40 @@ fn assign_attack_targets( .collect(); sorted_attackers.sort_by_key(|&(_, p)| p); + let secondary_players: Vec = if weak_opp == threat_ranked[0].0 { + vec![weak_opp] + } else { + let mut players = vec![threat_ranked[0].0]; + players.extend( + threat_ranked + .iter() + .map(|(pid, _)| *pid) + .filter(|pid| *pid != threat_ranked[0].0), + ); + players + }; for (id, power) in sorted_attackers { - if allocated_power < weak_life { + if allocated_power < weak_life + && preferred_attack_target_for_attacker( + id, + &[weak_opp], + valid_attack_targets, + valid_attack_targets_by_attacker, + ) == Some(target_weak) + { result.push((id, target_weak)); allocated_power += power; } else { - // If weakest IS the highest threat, keep sending there let target = if weak_opp == threat_ranked[0].0 { target_weak } else { - primary_target + preferred_attack_target_for_attacker( + id, + &secondary_players, + valid_attack_targets, + valid_attack_targets_by_attacker, + ) + .unwrap_or(target_weak) }; result.push((id, target)); } @@ -611,10 +750,28 @@ fn assign_attack_targets( // Default: pressure the next opponent in turn order unless one opponent is // a clear archenemy. This prevents every bot in a multiplayer pod from // dogpiling the same seat on small, noisy threat-score differences. - let primary = AttackTarget::Player( - multiplayer_pressure_target(state, player, opponents).unwrap_or(threat_ranked[0].0), + let primary = + multiplayer_pressure_target(state, player, opponents).unwrap_or(threat_ranked[0].0); + let mut preferred_players: Vec = vec![primary]; + preferred_players.extend( + threat_ranked + .iter() + .map(|(pid, _)| *pid) + .filter(|pid| *pid != primary), ); - attacking_ids.into_iter().map(|id| (id, primary)).collect() + attacking_ids + .into_iter() + .map(|id| { + let target = preferred_attack_target_for_attacker( + id, + &preferred_players, + valid_attack_targets, + valid_attack_targets_by_attacker, + ) + .unwrap_or(AttackTarget::Player(primary)); + (id, target) + }) + .collect() } const MULTIPLAYER_FOCUS_THREAT_MARGIN: f64 = 0.18; @@ -3441,6 +3598,7 @@ mod tests { false, None, Some(&targets), + None, ); // The lone 5/5 (>= loyalty 3) goes at the planeswalker; the 2/2s at the player. @@ -3475,6 +3633,7 @@ mod tests { false, None, Some(&targets), + None, ); assert!( attacks @@ -3500,6 +3659,7 @@ mod tests { false, None, Some(&targets), + None, ); assert!( attacks @@ -3528,6 +3688,7 @@ mod tests { false, None, Some(&targets), + None, ); assert!( attacks @@ -3557,10 +3718,40 @@ mod tests { false, None, Some(&targets), + None, ); assert_eq!( attacks.iter().find(|(id, _)| *id == bear).map(|(_, t)| *t), Some(AttackTarget::Player(PlayerId(1))), ); } + + #[test] + fn per_attacker_targets_override_global_attack_preferences() { + let mut state = setup_multiplayer(3); + let attacker = add_creature(&mut state, PlayerId(0), "Scout", 3, 3, vec![]); + state.players[1].life = 2; + state.players[2].life = 20; + let global_targets = vec![ + AttackTarget::Player(PlayerId(1)), + AttackTarget::Player(PlayerId(2)), + ]; + let scoped_targets = HashMap::from([(attacker, vec![AttackTarget::Player(PlayerId(2))])]); + + let attacks = choose_attackers_with_targets_with_profile( + &state, + PlayerId(0), + &AiProfile::default(), + false, + Some(&[attacker]), + Some(&global_targets), + Some(&scoped_targets), + ); + + assert_eq!( + attacks, + vec![(attacker, AttackTarget::Player(PlayerId(2)))], + "per-attacker legality must override the global weakest-opponent heuristic" + ); + } } diff --git a/crates/phase-ai/src/decision_kind.rs b/crates/phase-ai/src/decision_kind.rs index 4af3e83d99..2e0ae3856d 100644 --- a/crates/phase-ai/src/decision_kind.rs +++ b/crates/phase-ai/src/decision_kind.rs @@ -230,6 +230,7 @@ mod tests { player: PlayerId(0), valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }, &dummy_action ), diff --git a/crates/phase-ai/src/policies/aggro_pressure.rs b/crates/phase-ai/src/policies/aggro_pressure.rs index b6116cc327..f5ac0764b4 100644 --- a/crates/phase-ai/src/policies/aggro_pressure.rs +++ b/crates/phase-ai/src/policies/aggro_pressure.rs @@ -262,6 +262,7 @@ mod tests { player: AI, valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }, candidates: Vec::new(), } @@ -505,6 +506,7 @@ mod tests { player: AI, valid_attacker_ids: vec![oid], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }, candidates: Vec::new(), }; diff --git a/crates/phase-ai/src/policies/tokens_wide.rs b/crates/phase-ai/src/policies/tokens_wide.rs index 6c0065d2fc..7308d0d741 100644 --- a/crates/phase-ai/src/policies/tokens_wide.rs +++ b/crates/phase-ai/src/policies/tokens_wide.rs @@ -191,6 +191,7 @@ mod tests { player: AI, valid_attacker_ids: vec![], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }, candidates: Vec::new(), } diff --git a/crates/phase-ai/src/search.rs b/crates/phase-ai/src/search.rs index d8f324ed5d..5f550c7353 100644 --- a/crates/phase-ai/src/search.rs +++ b/crates/phase-ai/src/search.rs @@ -2224,6 +2224,7 @@ pub(crate) fn deterministic_choice( if let WaitingFor::DeclareAttackers { valid_attacker_ids, valid_attack_targets, + valid_attack_targets_by_attacker, .. } = &state.waiting_for { @@ -2234,6 +2235,7 @@ pub(crate) fn deterministic_choice( config.combat_lookahead, Some(valid_attacker_ids), Some(valid_attack_targets), + Some(valid_attack_targets_by_attacker), ); return Some(validated_declare_attackers(state, attacks)); } @@ -2283,6 +2285,7 @@ fn deterministic_combat_choice( if let WaitingFor::DeclareAttackers { valid_attacker_ids, valid_attack_targets, + valid_attack_targets_by_attacker, .. } = &state.waiting_for { @@ -2293,6 +2296,7 @@ fn deterministic_combat_choice( false, Some(valid_attacker_ids), Some(valid_attack_targets), + Some(valid_attack_targets_by_attacker), ); return Some(validated_declare_attackers(state, attacks)); } @@ -3658,6 +3662,7 @@ mod tests { player: PlayerId(0), valid_attacker_ids: vec![creature], valid_attack_targets: vec![], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; let config = create_config(AiDifficulty::VeryHard, Platform::Native); @@ -3690,6 +3695,7 @@ mod tests { player: PlayerId(0), valid_attacker_ids: vec![creature], valid_attack_targets: vec![target], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; let action = validated_declare_attackers(&state, vec![(creature, target)]); @@ -3703,6 +3709,45 @@ mod tests { } } + #[test] + fn declare_attackers_uses_per_attacker_target_map() { + let mut state = GameState::new(engine::types::format::FormatConfig::standard(), 3, 42); + state.turn_number = 2; + state.phase = Phase::DeclareAttackers; + state.active_player = PlayerId(0); + state.priority_player = PlayerId(0); + let attacker = add_creature(&mut state, PlayerId(0), 3, 3); + state.players[1].life = 2; + state.players[2].life = 20; + let p1 = engine::game::combat::AttackTarget::Player(PlayerId(1)); + let p2 = engine::game::combat::AttackTarget::Player(PlayerId(2)); + + state.waiting_for = WaitingFor::DeclareAttackers { + player: PlayerId(0), + valid_attacker_ids: vec![attacker], + valid_attack_targets: vec![p1, p2], + valid_attack_targets_by_attacker: std::collections::HashMap::from([( + attacker, + vec![p2], + )]), + }; + + let config = create_config(AiDifficulty::VeryHard, Platform::Native); + let mut rng = SmallRng::seed_from_u64(42); + let action = choose_action(&state, PlayerId(0), &config, &mut rng); + + match action { + Some(GameAction::DeclareAttackers { attacks, .. }) => { + assert_eq!( + attacks, + vec![(attacker, p2)], + "declare-attackers choice must respect the per-attacker legal target map" + ); + } + other => panic!("expected DeclareAttackers, got {other:?}"), + } + } + /// CR 608.2c + CR 701.23: Gifts Ungiven scaling regression — with a /// large library (80 cards), a count-4 search must complete via the /// BEAM_K-bounded path rather than the pre-fix Cartesian enumerator diff --git a/crates/phase-ai/tests/ai_quality.rs b/crates/phase-ai/tests/ai_quality.rs index b3e9d2fe8b..0fc433a84c 100644 --- a/crates/phase-ai/tests/ai_quality.rs +++ b/crates/phase-ai/tests/ai_quality.rs @@ -316,6 +316,7 @@ fn attacks_when_opponent_is_at_lethal() { player: P0, valid_attacker_ids: vec![attacker], valid_attack_targets: vec![AttackTarget::Player(P1)], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; } @@ -395,6 +396,7 @@ fn attacks_with_evasive_creatures() { player: P0, valid_attacker_ids: vec![flyer], valid_attack_targets: vec![AttackTarget::Player(P1)], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; } diff --git a/crates/phase-ai/tests/scenarios.rs b/crates/phase-ai/tests/scenarios.rs index 59f8f51b12..f0e9394886 100644 --- a/crates/phase-ai/tests/scenarios.rs +++ b/crates/phase-ai/tests/scenarios.rs @@ -136,6 +136,7 @@ fn scenario_multiplayer_attacks_to_finish_exposed_player() { player: P0, valid_attacker_ids: vec![attacker_a, attacker_b], valid_attack_targets: vec![AttackTarget::Player(P1), AttackTarget::Player(PlayerId(2))], + valid_attack_targets_by_attacker: std::collections::HashMap::new(), }; } From 104bd11d095fe0cc08c52d14a3bf142fa8989c8e Mon Sep 17 00:00:00 2001 From: Daedalus-Icarus <6442298+Daedalus-Icarus@users.noreply.github.com> Date: Fri, 3 Jul 2026 08:10:04 +0200 Subject: [PATCH 2/5] fix(ai): reduce attacker target API args for clippy --- crates/phase-ai/src/combat_ai.rs | 174 +++++++++++++++++-------------- crates/phase-ai/src/search.rs | 28 ++--- 2 files changed, 108 insertions(+), 94 deletions(-) diff --git a/crates/phase-ai/src/combat_ai.rs b/crates/phase-ai/src/combat_ai.rs index bb042e2ccd..90aadbf64c 100644 --- a/crates/phase-ai/src/combat_ai.rs +++ b/crates/phase-ai/src/combat_ai.rs @@ -143,24 +143,72 @@ pub fn choose_attackers_with_targets( choose_attackers_with_targets_with_profile( state, player, - &AiProfile::default(), - false, - None, - Some(&valid_attack_targets), - Some(&valid_attack_targets_by_attacker), - None, + ChooseAttackersWithTargetsOptions::new(&AiProfile::default()) + .with_valid_attack_targets(Some(&valid_attack_targets)) + .with_valid_attack_targets_by_attacker(Some(&valid_attack_targets_by_attacker)), ) } +pub struct ChooseAttackersWithTargetsOptions<'a> { + profile: &'a AiProfile, + combat_lookahead: bool, + valid_attacker_ids: Option<&'a [ObjectId]>, + valid_attack_targets: Option<&'a [AttackTarget]>, + valid_attack_targets_by_attacker: Option<&'a HashMap>>, + session: Option<&'a AiSession>, +} + +impl<'a> ChooseAttackersWithTargetsOptions<'a> { + pub(crate) fn new(profile: &'a AiProfile) -> Self { + Self { + profile, + combat_lookahead: false, + valid_attacker_ids: None, + valid_attack_targets: None, + valid_attack_targets_by_attacker: None, + session: None, + } + } + + pub(crate) fn with_combat_lookahead(mut self, combat_lookahead: bool) -> Self { + self.combat_lookahead = combat_lookahead; + self + } + + pub(crate) fn with_valid_attacker_ids( + mut self, + valid_attacker_ids: Option<&'a [ObjectId]>, + ) -> Self { + self.valid_attacker_ids = valid_attacker_ids; + self + } + + pub(crate) fn with_valid_attack_targets( + mut self, + valid_attack_targets: Option<&'a [AttackTarget]>, + ) -> Self { + self.valid_attack_targets = valid_attack_targets; + self + } + + pub(crate) fn with_valid_attack_targets_by_attacker( + mut self, + valid_attack_targets_by_attacker: Option<&'a HashMap>>, + ) -> Self { + self.valid_attack_targets_by_attacker = valid_attack_targets_by_attacker; + self + } + + pub(crate) fn with_session(mut self, session: Option<&'a AiSession>) -> Self { + self.session = session; + self + } +} + pub fn choose_attackers_with_targets_with_profile( state: &GameState, player: PlayerId, - profile: &AiProfile, - combat_lookahead: bool, - valid_attacker_ids: Option<&[ObjectId]>, - valid_attack_targets: Option<&[AttackTarget]>, - valid_attack_targets_by_attacker: Option<&HashMap>>, - session: Option<&AiSession>, + options: ChooseAttackersWithTargetsOptions<'_>, ) -> Vec<(ObjectId, AttackTarget)> { let opponents = players::opponents(state, player); if opponents.is_empty() { @@ -169,9 +217,9 @@ pub fn choose_attackers_with_targets_with_profile( // Use engine-provided valid attacker list when available; fall back to // local can_attack() for tests and hypothetical scenarios. - let candidates: Vec = if let Some(ids) = valid_attacker_ids { + let candidates: Vec = if let Some(ids) = options.valid_attacker_ids { ids.to_vec() - } else if let Some(targets_by_attacker) = valid_attack_targets_by_attacker { + } else if let Some(targets_by_attacker) = options.valid_attack_targets_by_attacker { state .battlefield .iter() @@ -242,7 +290,7 @@ pub fn choose_attackers_with_targets_with_profile( &opponents, &candidates, &opponent_blockers, - profile, + options.profile, ); // Hoist the block-legality static slices once for the whole candidate sweep — @@ -369,8 +417,8 @@ pub fn choose_attackers_with_targets_with_profile( // crackback_damage sees scaled creatures (Ouroboroid class) and // attack-trigger pumps (Battle Cry, Mentor). Failure to project // falls through to current state — matches pre-projection behavior. - let projection: Option> = if combat_lookahead { - match session { + let projection: Option> = if options.combat_lookahead { + match options.session { // Session present: route through the per-game projection cache // (turn-scoped key; identical result to project_to on a miss, // cached on subsequent identical combat decisions this turn). @@ -456,8 +504,8 @@ pub fn choose_attackers_with_targets_with_profile( let assignments = redirect_attackers_to_planeswalker( state, &attacking_ids, - valid_attack_targets, - valid_attack_targets_by_attacker, + options.valid_attack_targets, + options.valid_attack_targets_by_attacker, objective, opp, opponent_life, @@ -472,8 +520,8 @@ pub fn choose_attackers_with_targets_with_profile( player, &opponents, attacking_ids, - valid_attack_targets, - valid_attack_targets_by_attacker, + options.valid_attack_targets, + options.valid_attack_targets_by_attacker, ); emit_attack_trace(player, &candidates, &assignments); assignments @@ -3618,12 +3666,8 @@ mod tests { let attacks = choose_attackers_with_targets_with_profile( &state, PlayerId(0), - &AiProfile::default(), - false, - None, - Some(&targets), - None, - None, + ChooseAttackersWithTargetsOptions::new(&AiProfile::default()) + .with_valid_attack_targets(Some(&targets)), ); // The lone 5/5 (>= loyalty 3) goes at the planeswalker; the 2/2s at the player. @@ -3654,12 +3698,8 @@ mod tests { let attacks = choose_attackers_with_targets_with_profile( &state, PlayerId(0), - &AiProfile::default(), - false, - None, - Some(&targets), - None, - None, + ChooseAttackersWithTargetsOptions::new(&AiProfile::default()) + .with_valid_attack_targets(Some(&targets)), ); assert!( attacks @@ -3681,12 +3721,8 @@ mod tests { let attacks = choose_attackers_with_targets_with_profile( &state, PlayerId(0), - &AiProfile::default(), - false, - None, - Some(&targets), - None, - None, + ChooseAttackersWithTargetsOptions::new(&AiProfile::default()) + .with_valid_attack_targets(Some(&targets)), ); assert!( attacks @@ -3711,12 +3747,8 @@ mod tests { let attacks = choose_attackers_with_targets_with_profile( &state, PlayerId(0), - &AiProfile::default(), - false, - None, - Some(&targets), - None, - None, + ChooseAttackersWithTargetsOptions::new(&AiProfile::default()) + .with_valid_attack_targets(Some(&targets)), ); assert!( attacks @@ -3742,12 +3774,8 @@ mod tests { let attacks = choose_attackers_with_targets_with_profile( &state, PlayerId(0), - &AiProfile::default(), - false, - None, - Some(&targets), - None, - None, + ChooseAttackersWithTargetsOptions::new(&AiProfile::default()) + .with_valid_attack_targets(Some(&targets)), ); assert_eq!( attacks.iter().find(|(id, _)| *id == bear).map(|(_, t)| *t), @@ -3770,12 +3798,10 @@ mod tests { let attacks = choose_attackers_with_targets_with_profile( &state, PlayerId(0), - &AiProfile::default(), - false, - Some(&[attacker]), - Some(&global_targets), - Some(&scoped_targets), - None, + ChooseAttackersWithTargetsOptions::new(&AiProfile::default()) + .with_valid_attacker_ids(Some(&[attacker])) + .with_valid_attack_targets(Some(&global_targets)) + .with_valid_attack_targets_by_attacker(Some(&scoped_targets)), ); assert_eq!( @@ -3823,12 +3849,9 @@ mod tests { let _ = choose_attackers_with_targets_with_profile( &state, PlayerId(0), - &profile, - /* combat_lookahead = */ true, - None, - None, - None, - Some(&session), + ChooseAttackersWithTargetsOptions::new(&profile) + .with_combat_lookahead(true) + .with_session(Some(&session)), ); let expected = ProjectionKey { @@ -3865,12 +3888,9 @@ mod tests { let _ = choose_attackers_with_targets_with_profile( &state, PlayerId(0), - &profile, - /* combat_lookahead = */ false, - None, - None, - None, - Some(&session), + ChooseAttackersWithTargetsOptions::new(&profile) + .with_combat_lookahead(false) + .with_session(Some(&session)), ); assert!( @@ -3891,22 +3911,14 @@ mod tests { let with_session = choose_attackers_with_targets_with_profile( &state, PlayerId(0), - &profile, - /* combat_lookahead = */ true, - None, - None, - None, - Some(&AiSession::empty()), + ChooseAttackersWithTargetsOptions::new(&profile) + .with_combat_lookahead(true) + .with_session(Some(&AiSession::empty())), ); let without_session = choose_attackers_with_targets_with_profile( &state, PlayerId(0), - &profile, - /* combat_lookahead = */ true, - None, - None, - None, - None, + ChooseAttackersWithTargetsOptions::new(&profile).with_combat_lookahead(true), ); assert_eq!( diff --git a/crates/phase-ai/src/search.rs b/crates/phase-ai/src/search.rs index a4863b43b7..a4876ac97b 100644 --- a/crates/phase-ai/src/search.rs +++ b/crates/phase-ai/src/search.rs @@ -20,7 +20,10 @@ use engine::types::statics::StaticMode; use engine::types::zones::Zone; use crate::cast_facts::cast_facts_for_action; -use crate::combat_ai::{choose_attackers_with_targets_with_profile, choose_blockers_with_profile}; +use crate::combat_ai::{ + choose_attackers_with_targets_with_profile, choose_blockers_with_profile, + ChooseAttackersWithTargetsOptions, +}; use crate::config::{AiConfig, PlannerMode, ThreatAwareness}; use crate::context::AiContext; use crate::features::DeckFeatures; @@ -2392,12 +2395,12 @@ pub(crate) fn deterministic_choice( let attacks = choose_attackers_with_targets_with_profile( state, ai_player, - &config.profile, - config.combat_lookahead, - Some(valid_attacker_ids), - Some(valid_attack_targets), - Some(valid_attack_targets_by_attacker), - context.map(|c| c.session.as_ref()), + ChooseAttackersWithTargetsOptions::new(&config.profile) + .with_combat_lookahead(config.combat_lookahead) + .with_valid_attacker_ids(Some(valid_attacker_ids)) + .with_valid_attack_targets(Some(valid_attack_targets)) + .with_valid_attack_targets_by_attacker(Some(valid_attack_targets_by_attacker)) + .with_session(context.map(|c| c.session.as_ref())), ); return Some(validated_declare_attackers(state, attacks)); } @@ -2455,12 +2458,11 @@ fn deterministic_combat_choice( let attacks = choose_attackers_with_targets_with_profile( state, ai_player, - profile, - false, - Some(valid_attacker_ids), - Some(valid_attack_targets), - Some(valid_attack_targets_by_attacker), - session, + ChooseAttackersWithTargetsOptions::new(profile) + .with_valid_attacker_ids(Some(valid_attacker_ids)) + .with_valid_attack_targets(Some(valid_attack_targets)) + .with_valid_attack_targets_by_attacker(Some(valid_attack_targets_by_attacker)) + .with_session(session), ); return Some(validated_declare_attackers(state, attacks)); } From 892f97f783977e3d41fa830a94177a79ff965398 Mon Sep 17 00:00:00 2001 From: Daedalus-Icarus <6442298+Daedalus-Icarus@users.noreply.github.com> Date: Fri, 3 Jul 2026 01:45:59 -1000 Subject: [PATCH 3/5] fix(ai): scope per-attacker target map to active player and fix even-split fill order Two test regressions from the per-attacker target map threading: 1. choose_attackers_with_targets fetched engine target lists that are scoped to state.active_player, producing empty candidates when the decision is for a different player (multi-player bot tests). Only thread the engine lists when player == state.active_player; otherwise fall back to can_attack + players::opponents. 2. legal_attack_targets_for_attacker returned None when an attacker was absent from the per-attacker map, collapsing every attacker to no legal targets in scenarios that supply an empty/stale map. Fall back to valid_attack_targets instead, matching the engine's own candidate generator (attacker_actions). 3. Frontend spreadStackEvenly used a greedy least-loaded round-robin that assigned ascending-id members to alternating targets instead of filling targets in display order. Rewrite it to use the shared evenSplit building block for per-target capacities, then fill in display order with ascending-id members. --- .../controls/AttackTargetPicker.tsx | 39 +++++++++++-------- crates/phase-ai/src/combat_ai.rs | 33 ++++++++++++---- 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/client/src/components/controls/AttackTargetPicker.tsx b/client/src/components/controls/AttackTargetPicker.tsx index 9c75dc9811..20516bad71 100644 --- a/client/src/components/controls/AttackTargetPicker.tsx +++ b/client/src/components/controls/AttackTargetPicker.tsx @@ -8,7 +8,7 @@ import { usePlayerId } from "../../hooks/usePlayerId.ts"; import { getSeatColor } from "../../hooks/useSeatColor.ts"; import { useGameStore } from "../../stores/gameStore.ts"; import { getPlayerDisplayName } from "../../stores/multiplayerStore.ts"; -import { type AttackerStack, groupAttackers } from "../../utils/combat.ts"; +import { type AttackerStack, evenSplit, groupAttackers } from "../../utils/combat.ts"; import { formatCounterType } from "../../viewmodel/cardProps.ts"; import { PeekTab } from "../modal/DialogShell.tsx"; import { gameButtonClass } from "../ui/buttonStyles.ts"; @@ -727,8 +727,11 @@ function stackFullySupportsTarget( /** * Redistribute a stack as evenly as legality allows across display-order - * targets. Each member is assigned to the currently least-populated legal - * target, breaking ties by target order. + * targets. Uses the shared `evenSplit` building block to compute per-target + * capacities (remainder front-loaded to the earliest targets), then fills + * targets in display order with ascending-id members — so the lowest-id + * members claim the earliest targets deterministically, matching the per-target + * stepper's "lowest-id unassigned member" convention. */ function spreadStackEvenly( map: AssignmentMap, @@ -737,7 +740,12 @@ function spreadStackEvenly( targetsForCreature: (creatureId: ObjectId) => AttackTarget[], ): void { if (targets.length === 0) return; - const counts = new Map(); + const split = evenSplit(stack.ids.length, targets.length); + const cap = new Map(); + for (let i = 0; i < targets.length; i++) { + cap.set(attackTargetKey(targets[i]), split[i]); + } + const used = new Map(); for (const id of stack.ids) { const legalTargets = targets.filter((target) => targetsForCreature(id).some((candidate) => sameAttackTarget(candidate, target)), @@ -746,18 +754,17 @@ function spreadStackEvenly( map.set(id, null); continue; } - let bestTarget = legalTargets[0]; - let bestCount = counts.get(attackTargetKey(bestTarget)) ?? 0; - for (const target of legalTargets.slice(1)) { - const count = counts.get(attackTargetKey(target)) ?? 0; - if (count < bestCount) { - bestTarget = target; - bestCount = count; - } - } - map.set(id, bestTarget); - const key = attackTargetKey(bestTarget); - counts.set(key, (counts.get(key) ?? 0) + 1); + // Assign to the first legal target (display order) with remaining capacity; + // fall back to the first legal target when all are full (overfill rather + // than leave the member unassigned). + const target = + legalTargets.find((t) => { + const key = attackTargetKey(t); + return (used.get(key) ?? 0) < (cap.get(key) ?? 0); + }) ?? legalTargets[0]; + map.set(id, target); + const key = attackTargetKey(target); + used.set(key, (used.get(key) ?? 0) + 1); } } diff --git a/crates/phase-ai/src/combat_ai.rs b/crates/phase-ai/src/combat_ai.rs index 90aadbf64c..a14f129986 100644 --- a/crates/phase-ai/src/combat_ai.rs +++ b/crates/phase-ai/src/combat_ai.rs @@ -137,15 +137,26 @@ pub fn choose_attackers_with_targets( state: &GameState, player: PlayerId, ) -> Vec<(ObjectId, AttackTarget)> { - let valid_attack_targets = engine::game::combat::get_valid_attack_targets(state); - let valid_attack_targets_by_attacker = - engine::game::combat::get_valid_attack_targets_by_attacker(state); + // The engine's `get_valid_attack_targets` / `get_valid_attack_targets_by_attacker` + // are scoped to `state.active_player`. Only thread them through when the + // decision is for that player; otherwise (multiplayer tests, hypothetical + // scenarios where `player != active_player`) fall back to the local + // `can_attack` + `players::opponents` derivation so non-active players still + // get a rules-correct candidate set and target assignment. + let (valid_attack_targets, valid_attack_targets_by_attacker) = if player == state.active_player + { + let targets = engine::game::combat::get_valid_attack_targets(state); + let by_attacker = engine::game::combat::get_valid_attack_targets_by_attacker(state); + (Some(targets), Some(by_attacker)) + } else { + (None, None) + }; choose_attackers_with_targets_with_profile( state, player, ChooseAttackersWithTargetsOptions::new(&AiProfile::default()) - .with_valid_attack_targets(Some(&valid_attack_targets)) - .with_valid_attack_targets_by_attacker(Some(&valid_attack_targets_by_attacker)), + .with_valid_attack_targets(valid_attack_targets.as_deref()) + .with_valid_attack_targets_by_attacker(valid_attack_targets_by_attacker.as_ref()), ) } @@ -532,8 +543,16 @@ fn legal_attack_targets_for_attacker<'a>( valid_attack_targets: Option<&'a [AttackTarget]>, valid_attack_targets_by_attacker: Option<&'a HashMap>>, ) -> Option<&'a [AttackTarget]> { - if let Some(targets) = valid_attack_targets_by_attacker { - return targets.get(&attacker_id).map(Vec::as_slice); + // CR 508.1b/c/d: when the per-attacker map is present, use the attacker's + // scoped target set; if the attacker is absent from the map, fall back to + // the global `valid_attack_targets` — matching the engine's own candidate + // generator (`attacker_actions`), which does the same `unwrap_or` fallback. + // This keeps tests and scenarios that supply an empty map (or a stale + // snapshot) from collapsing every attacker to "no legal targets." + if let Some(by_attacker) = valid_attack_targets_by_attacker { + if let Some(targets) = by_attacker.get(&attacker_id) { + return Some(targets.as_slice()); + } } valid_attack_targets } From 318363e9ba05e4bc851135c3d1a4f4a10242f8f5 Mon Sep 17 00:00:00 2001 From: Daedalus-Icarus <6442298+Daedalus-Icarus@users.noreply.github.com> Date: Fri, 3 Jul 2026 02:49:25 -1000 Subject: [PATCH 4/5] fix(engine): align goad redirect test assertion with diagnostic string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The goad redirect error at combat.rs:2524 reads 'must attack a different player directly if able (CR 701.15b)', but the test still asserted the older 'must attack a different player if able' substring (missing 'directly'). The 'directly' wording is the more precise one — it matches CR 701.15b's requirement for a direct player attack, not a planeswalker/battle proxy — so update the test assertion to match. --- crates/engine/src/game/combat.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/game/combat.rs b/crates/engine/src/game/combat.rs index 463be6e3cd..868a46e998 100644 --- a/crates/engine/src/game/combat.rs +++ b/crates/engine/src/game/combat.rs @@ -8480,7 +8480,7 @@ mod tests { assert!( result .unwrap_err() - .contains("must attack a different player if able"), + .contains("must attack a different player directly if able"), "an attackable non-goading opponent (P2) must still force the redirect" ); } From 538aa2d2d1ecc429215d5087d95b5c9e5110938d Mon Sep 17 00:00:00 2001 From: Daedalus-Icarus <6442298+Daedalus-Icarus@users.noreply.github.com> Date: Fri, 3 Jul 2026 13:38:43 -1000 Subject: [PATCH 5/5] fix(engine): add valid_attack_targets_by_attacker to token_storm_scaling_gate fixture New test file from main merge constructs WaitingFor::DeclareAttackers without the per-attacker target field this PR adds to the variant. --- crates/engine/tests/token_storm_scaling_gate.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/engine/tests/token_storm_scaling_gate.rs b/crates/engine/tests/token_storm_scaling_gate.rs index 03f511086e..150634ed2e 100644 --- a/crates/engine/tests/token_storm_scaling_gate.rs +++ b/crates/engine/tests/token_storm_scaling_gate.rs @@ -18,7 +18,9 @@ //! process-per-test harness. use engine::ai_support::legal_actions_full; -use engine::game::combat::{get_valid_attack_targets, get_valid_attacker_ids}; +use engine::game::combat::{ + get_valid_attack_targets, get_valid_attack_targets_by_attacker, get_valid_attacker_ids, +}; use engine::game::layers; use engine::game::perf_counters; use engine::game::targeting::find_legal_targets; @@ -195,6 +197,7 @@ fn token_storm_declare_attackers_gate() { player: active, valid_attacker_ids, valid_attack_targets, + valid_attack_targets_by_attacker: get_valid_attack_targets_by_attacker(&restored), }; perf_counters::reset();