From 70e76d42e5d1afe0410c94a28919cfbe767d2c31 Mon Sep 17 00:00:00 2001 From: Clayton Date: Fri, 31 Jul 2026 12:41:20 -0500 Subject: [PATCH 1/4] fix(engine): skip declare-attackers prompt when no legal attackers exist The DeclareAttackers arm of run_auto_pass_loop only auto-collapsed the interactive prompt during an UntilTurnBoundary auto-pass session, unlike its DeclareBlockers sibling which already auto-submits whenever valid_blocker_ids is empty. Any begin-of-combat trigger (unrelated to combat) bypasses the has_potential_attackers short-circuit in Phase::BeginCombat, so a player whose only creature is tapped could still land on the interactive "declare attackers" prompt with zero legal attackers. Mirror the DeclareBlockers pattern: auto-submit an empty attacker declaration whenever valid_attacker_ids is empty (CR 508.1a), regardless of auto-pass state, unless the player has an explicit phase stop set. Fixes #6463. --- crates/engine/src/game/engine.rs | 17 ++- ..._6463_no_eligible_attackers_skip_prompt.rs | 109 ++++++++++++++++++ crates/engine/tests/integration/main.rs | 1 + 3 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index bbcfd9ef4e..0dde0e43d8 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -3907,10 +3907,19 @@ fn run_auto_pass_loop(state: &mut GameState, result: &mut ActionResult) -> bool } } - // UntilTurnBoundary: auto-submit empty attackers unless the user - // flagged this phase as a stop. - WaitingFor::DeclareAttackers { player, .. } - if end_of_turn_active(state, *player) && !state.phase_stop_hit(*player) => + // Auto-submit empty attackers when there's nothing to choose + // (CR 508.1a: 0 attackers is always a legal declaration), mirroring + // the DeclareBlockers arm below — the turn-based action and its + // triggers still run via handle_empty_attackers, only the + // interactive prompt is elided. Also auto-submit during an + // UntilTurnBoundary auto-pass session even when legal attackers + // exist. A phase stop overrides both cases. + WaitingFor::DeclareAttackers { + player, + valid_attacker_ids, + .. + } if !state.phase_stop_hit(*player) + && (valid_attacker_ids.is_empty() || end_of_turn_active(state, *player)) => { let mut events = Vec::new(); match engine_combat::handle_empty_attackers(state, &mut events) { diff --git a/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs b/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs new file mode 100644 index 0000000000..8f8513a852 --- /dev/null +++ b/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs @@ -0,0 +1,109 @@ +//! Issue #6463: the declare-attackers step must not surface the interactive +//! `WaitingFor::DeclareAttackers` prompt when the active player has zero +//! legal attackers (e.g. their only creature is tapped). +//! +//! CR 508.1a: 0 attackers is always a legal declaration, and the turn-based +//! action still runs even when nothing can be declared — only the +//! interactive prompt should be elided (mirroring how `DeclareBlockers` +//! already collapses when `valid_blocker_ids` is empty). +//! +//! The reproduction needs a begin-of-combat trigger, not just a tapped +//! creature: `Phase::BeginCombat`'s `has_potential_attackers` short-circuit +//! (which does correctly skip the whole combat phase when there's nothing to +//! do AND no begin-combat triggers) is bypassed whenever any begin-of-combat +//! trigger fires — once one exists, the engine unconditionally continues +//! into `Phase::DeclareAttackers` after the trigger resolves, regardless of +//! whether any creature can actually attack. +use engine::game::scenario::{GameScenario, P0}; +use engine::types::ability::{ + AbilityDefinition, AbilityKind, Effect, QuantityExpr, TargetFilter, TriggerConstraint, + TriggerDefinition, +}; +use engine::types::actions::{DebugAction, GameAction}; +use engine::types::game_state::WaitingFor; +use engine::types::phase::Phase; +use engine::types::triggers::TriggerMode; +use engine::types::zones::Zone; + +#[test] +fn declare_attackers_prompt_skipped_when_no_legal_attackers_exist() { + let mut scenario = GameScenario::new_n_player(2, 42); + scenario.at_phase(Phase::PreCombatMain); + + // An unrelated "at the beginning of combat on your turn" trigger — this + // is what causes the engine to enter Phase::DeclareAttackers at all + // (see module doc). `GainLife` targets the controller, so it resolves + // without any further player choice. + let begin_combat_trigger = TriggerDefinition::new(TriggerMode::Phase) + .phase(Phase::BeginCombat) + .trigger_zones(vec![Zone::Battlefield]) + .constraint(TriggerConstraint::OnlyDuringYourTurn) + .execute(AbilityDefinition::new( + AbilityKind::Spell, + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + )); + + // The player's only creature — tapped, so it cannot legally attack. + let elves = scenario + .add_creature(P0, "Fyndhorn Elves", 1, 1) + .with_trigger_definition(begin_combat_trigger) + .id(); + + let mut runner = scenario.build(); + runner.state_mut().debug_mode = true; + runner + .act(GameAction::Debug(DebugAction::SetTapped { + object_id: elves, + tapped: true, + })) + .expect("tapping the only creature should succeed"); + + // Drive priority forward: PreCombatMain -> BeginCombat (trigger goes on + // the stack) -> trigger resolves -> BeginCombat's empty stack advances + // the phase. At every step, the engine must never stop on the + // interactive DeclareAttackers prompt, since valid_attacker_ids is empty + // throughout. + for _ in 0..8 { + if !matches!(runner.state().waiting_for, WaitingFor::Priority { .. }) + || runner.state().phase == Phase::PostCombatMain + { + break; + } + let result = runner + .act(GameAction::PassPriority) + .expect("passing priority should always succeed here"); + assert!( + !matches!(result.waiting_for, WaitingFor::DeclareAttackers { .. }), + "declare-attackers prompt must be skipped when there are no legal \ + attackers, got {:?}", + result.waiting_for + ); + } + + assert_eq!( + runner.state().phase, + Phase::PostCombatMain, + "with zero legal attackers the turn should sail through combat to \ + postcombat main without ever pausing on an attacker prompt" + ); + assert!( + runner.state().combat.is_none(), + "combat should be cleared once the (empty) attacker declaration is \ + auto-submitted" + ); + let p0_life = runner + .state() + .players + .iter() + .find(|p| p.id == P0) + .expect("P0 exists") + .life; + assert_eq!( + p0_life, 21, + "the begin-of-combat trigger must still resolve even though the \ + prompt itself is skipped" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 3de438afaf..86e3b944a7 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -625,6 +625,7 @@ mod issue_6435_mosswort_bridge_hideaway_play; mod issue_6437_fight_rigging_exiled_card_target; mod issue_6440_mockingbird_uncast_copy_ceiling; mod issue_6459_scheming_symmetry; +mod issue_6463_no_eligible_attackers_skip_prompt; mod issue_6477_wandering_archaic_optional_payment; mod issue_6498_portent_of_calamity; mod issue_6499_flickering_ward_protection_exemption; From e1ce77b80cdf0105e362fa4ac9180ce9ccfa9e15 Mon Sep 17 00:00:00 2001 From: Clayton Date: Fri, 31 Jul 2026 13:47:23 -0500 Subject: [PATCH 2/4] fix(engine): check aggregate target legality, not just candidate emptiness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on #6845: valid_attacker_ids only reflects the creature-level candidate set (untapped, not summoning-sick, etc.). An untapped candidate can still have zero legal defenders — e.g. every opponent is protected by a scoped CantAttack, or a defender-scoped restriction in multiplayer — leaving an interactive no-choice DeclareAttackers prompt even though valid_attacker_ids was non-empty. Check the aggregate valid_attack_targets instead, which is empty exactly when no non-empty attack declaration is possible (it's built solely from candidates' per-attacker legal-target lists, so it's empty whenever valid_attacker_ids is, and also catches the additional no-legal-target case). Also: - Add a second regression test covering the target-legality path directly (untapped candidate, sole opponent protected), independent of the begin-of-combat-trigger mechanism used by the first test. - Fix combat.rs's debug_set_summoning_sickness_removes_from_valid_attackers unit test, which put the engine's only creature to sleep and expected the prompt to stay interactive — under the fixed behavior that now correctly auto-collapses. Added a second, always-healthy creature so the test keeps isolating its actual concern (the debug refresh drops the now-sick creature from the live snapshot). --- crates/engine/src/game/combat.rs | 21 ++- crates/engine/src/game/engine.rs | 18 ++- ..._6463_no_eligible_attackers_skip_prompt.rs | 149 +++++++++++++----- 3 files changed, 140 insertions(+), 48 deletions(-) diff --git a/crates/engine/src/game/combat.rs b/crates/engine/src/game/combat.rs index 3c263b363e..939246a79e 100644 --- a/crates/engine/src/game/combat.rs +++ b/crates/engine/src/game/combat.rs @@ -7352,6 +7352,13 @@ mod tests { // Non-sick, eligible creature (create_creature leaves summoning_sick false). let id = create_creature(&mut state, PlayerId(0), "Bear", 2, 2); + // A second eligible attacker that stays healthy — keeps the prompt + // interactive after `id` goes sick below, isolating this test's + // concern (the refresh drops the now-sick creature from the live + // snapshot) from the separate empty-declaration auto-collapse + // behavior (issue #6463; that path is covered by + // `issue_6463_no_eligible_attackers_skip_prompt`). + let healthy = create_creature(&mut state, PlayerId(0), "Other Bear", 2, 2); state.waiting_for = WaitingFor::DeclareAttackers { player: PlayerId(0), @@ -7383,10 +7390,16 @@ mod tests { match &result.waiting_for { WaitingFor::DeclareAttackers { valid_attacker_ids, .. - } => assert!( - !valid_attacker_ids.contains(&id), - "refreshed snapshot must drop the now-sick creature" - ), + } => { + assert!( + !valid_attacker_ids.contains(&id), + "refreshed snapshot must drop the now-sick creature" + ); + assert!( + valid_attacker_ids.contains(&healthy), + "the still-healthy creature must remain a valid attacker" + ); + } other => panic!("expected refreshed DeclareAttackers, got {other:?}"), } } diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 0dde0e43d8..94b39c5055 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -3911,15 +3911,23 @@ fn run_auto_pass_loop(state: &mut GameState, result: &mut ActionResult) -> bool // (CR 508.1a: 0 attackers is always a legal declaration), mirroring // the DeclareBlockers arm below — the turn-based action and its // triggers still run via handle_empty_attackers, only the - // interactive prompt is elided. Also auto-submit during an - // UntilTurnBoundary auto-pass session even when legal attackers - // exist. A phase stop overrides both cases. + // interactive prompt is elided. `valid_attack_targets` is the + // aggregate of every candidate's per-attacker legal target list + // (combat.rs `AttackDeclarationConstraints::build`), so checking + // it rather than `valid_attacker_ids` also catches an untapped + // candidate that has no legal defender (e.g. every opponent is + // protected by a scoped CantAttack, or a defender-scoped + // restriction in multiplayer) — `valid_attacker_ids` alone can be + // non-empty in that case even though no non-empty declaration is + // actually possible. Also auto-submit during an UntilTurnBoundary + // auto-pass session even when legal attackers exist. A phase stop + // overrides both cases. WaitingFor::DeclareAttackers { player, - valid_attacker_ids, + valid_attack_targets, .. } if !state.phase_stop_hit(*player) - && (valid_attacker_ids.is_empty() || end_of_turn_active(state, *player)) => + && (valid_attack_targets.is_empty() || end_of_turn_active(state, *player)) => { let mut events = Vec::new(); match engine_combat::handle_empty_attackers(state, &mut events) { diff --git a/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs b/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs index 8f8513a852..3f4b3c69bb 100644 --- a/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs +++ b/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs @@ -1,30 +1,81 @@ //! Issue #6463: the declare-attackers step must not surface the interactive -//! `WaitingFor::DeclareAttackers` prompt when the active player has zero -//! legal attackers (e.g. their only creature is tapped). +//! `WaitingFor::DeclareAttackers` prompt when the active player has no legal +//! attack declaration available. //! //! CR 508.1a: 0 attackers is always a legal declaration, and the turn-based //! action still runs even when nothing can be declared — only the //! interactive prompt should be elided (mirroring how `DeclareBlockers` //! already collapses when `valid_blocker_ids` is empty). //! -//! The reproduction needs a begin-of-combat trigger, not just a tapped -//! creature: `Phase::BeginCombat`'s `has_potential_attackers` short-circuit -//! (which does correctly skip the whole combat phase when there's nothing to -//! do AND no begin-combat triggers) is bypassed whenever any begin-of-combat -//! trigger fires — once one exists, the engine unconditionally continues -//! into `Phase::DeclareAttackers` after the trigger resolves, regardless of -//! whether any creature can actually attack. -use engine::game::scenario::{GameScenario, P0}; +//! There are two independent ways to end up with no legal declaration, and +//! both are covered here: +//! +//! - `valid_attacker_ids` (the creature-level candidate set) is empty — e.g. +//! the only creature is tapped. Reaching `Phase::DeclareAttackers` at all +//! in that case needs a begin-of-combat trigger: `Phase::BeginCombat`'s +//! `has_potential_attackers` short-circuit (which does correctly skip the +//! whole combat phase when there's nothing to do AND no begin-combat +//! triggers) is bypassed whenever any begin-of-combat trigger fires — once +//! one exists, the engine unconditionally continues into +//! `Phase::DeclareAttackers` after the trigger resolves, regardless of +//! whether any creature can actually attack. +//! - `valid_attacker_ids` is non-empty (an untapped, non-sick creature is a +//! valid *candidate*) but every candidate's per-attacker legal-target list +//! is empty — e.g. the only opponent has protection from everything. The +//! aggregate `valid_attack_targets` is empty even though +//! `valid_attacker_ids` is not, and `has_potential_attackers` never checks +//! target legality, so this reaches `Phase::DeclareAttackers` through the +//! ordinary (trigger-free) path. +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::game::zones::create_object; use engine::types::ability::{ - AbilityDefinition, AbilityKind, Effect, QuantityExpr, TargetFilter, TriggerConstraint, - TriggerDefinition, + AbilityDefinition, AbilityKind, ContinuousModification, Duration, Effect, QuantityExpr, + TargetFilter, TriggerConstraint, TriggerDefinition, }; use engine::types::actions::{DebugAction, GameAction}; use engine::types::game_state::WaitingFor; +use engine::types::identifiers::CardId; +use engine::types::keywords::{Keyword, ProtectionTarget}; use engine::types::phase::Phase; use engine::types::triggers::TriggerMode; use engine::types::zones::Zone; +/// Drive priority forward from precombat main until either the interactive +/// declare-attackers prompt would appear (asserted against on every step) or +/// the game reaches postcombat main. Shared by both reproductions below. +fn assert_never_prompts_and_reaches_postcombat_main( + runner: &mut engine::game::scenario::GameRunner, +) { + for _ in 0..8 { + if !matches!(runner.state().waiting_for, WaitingFor::Priority { .. }) + || runner.state().phase == Phase::PostCombatMain + { + break; + } + let result = runner + .act(GameAction::PassPriority) + .expect("passing priority should always succeed here"); + assert!( + !matches!(result.waiting_for, WaitingFor::DeclareAttackers { .. }), + "declare-attackers prompt must be skipped when there is no legal \ + attack declaration, got {:?}", + result.waiting_for + ); + } + + assert_eq!( + runner.state().phase, + Phase::PostCombatMain, + "with no legal attack declaration the turn should sail through combat \ + to postcombat main without ever pausing on an attacker prompt" + ); + assert!( + runner.state().combat.is_none(), + "combat should be cleared once the (empty) attacker declaration is \ + auto-submitted" + ); +} + #[test] fn declare_attackers_prompt_skipped_when_no_legal_attackers_exist() { let mut scenario = GameScenario::new_n_player(2, 42); @@ -66,34 +117,8 @@ fn declare_attackers_prompt_skipped_when_no_legal_attackers_exist() { // the phase. At every step, the engine must never stop on the // interactive DeclareAttackers prompt, since valid_attacker_ids is empty // throughout. - for _ in 0..8 { - if !matches!(runner.state().waiting_for, WaitingFor::Priority { .. }) - || runner.state().phase == Phase::PostCombatMain - { - break; - } - let result = runner - .act(GameAction::PassPriority) - .expect("passing priority should always succeed here"); - assert!( - !matches!(result.waiting_for, WaitingFor::DeclareAttackers { .. }), - "declare-attackers prompt must be skipped when there are no legal \ - attackers, got {:?}", - result.waiting_for - ); - } + assert_never_prompts_and_reaches_postcombat_main(&mut runner); - assert_eq!( - runner.state().phase, - Phase::PostCombatMain, - "with zero legal attackers the turn should sail through combat to \ - postcombat main without ever pausing on an attacker prompt" - ); - assert!( - runner.state().combat.is_none(), - "combat should be cleared once the (empty) attacker declaration is \ - auto-submitted" - ); let p0_life = runner .state() .players @@ -107,3 +132,49 @@ fn declare_attackers_prompt_skipped_when_no_legal_attackers_exist() { prompt itself is skipped" ); } + +/// The candidate set can be non-empty (an untapped, non-sick creature) while +/// every candidate's legal-target list is still empty — e.g. the sole +/// opponent has protection from everything. Checking only +/// `valid_attacker_ids` misses this: it stays non-empty even though no +/// non-empty attack declaration is actually possible. The engine must check +/// the aggregate `valid_attack_targets` instead. +#[test] +fn declare_attackers_prompt_skipped_when_every_candidate_has_no_legal_target() { + let mut scenario = GameScenario::new_n_player(2, 7); + scenario.at_phase(Phase::PreCombatMain); + + // Untapped, non-summoning-sick creature: a normal candidate attacker. + // `has_potential_attackers` (the coarse BeginCombat gate) never checks + // per-target legality, so this reaches Phase::DeclareAttackers through + // the ordinary trigger-free path — no begin-of-combat trigger needed. + scenario.add_creature(P0, "Fyndhorn Elves", 1, 1); + + let mut runner = scenario.build(); + + // CR 702.16e-style protection from everything on the only opponent + // (Teferi's Protection), for the rest of the turn. `get_valid_attack_targets` + // already excludes protected players (see + // `get_valid_attack_targets_excludes_protected_player` in combat.rs), so + // with a single opponent this empties the aggregate `valid_attack_targets` + // even though `valid_attacker_ids` stays non-empty. + let source = create_object( + runner.state_mut(), + CardId(999), + P1, + "Teferi's Protection".to_string(), + Zone::Battlefield, + ); + runner.state_mut().add_transient_continuous_effect( + source, + P1, + Duration::UntilEndOfTurn, + TargetFilter::SpecificPlayer { id: P1 }, + vec![ContinuousModification::AddKeyword { + keyword: Keyword::Protection(ProtectionTarget::Everything), + }], + None, + ); + + assert_never_prompts_and_reaches_postcombat_main(&mut runner); +} From fc1ed98f965315d2c836b55f14b1d056ff0e1240 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 31 Jul 2026 11:54:22 -0700 Subject: [PATCH 3/4] fix(PR-6845): correct combat rules annotations --- crates/engine/src/game/engine.rs | 5 +++-- .../issue_6463_no_eligible_attackers_skip_prompt.rs | 9 +++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 94b39c5055..d41934e5c0 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -3907,8 +3907,9 @@ fn run_auto_pass_loop(state: &mut GameState, result: &mut ActionResult) -> bool } } - // Auto-submit empty attackers when there's nothing to choose - // (CR 508.1a: 0 attackers is always a legal declaration), mirroring + // Auto-submit empty attackers when there's nothing to choose. When no + // nonempty declaration is legal, declaring zero attackers satisfies + // CR 508.1a/d, mirroring // the DeclareBlockers arm below — the turn-based action and its // triggers still run via handle_empty_attackers, only the // interactive prompt is elided. `valid_attack_targets` is the diff --git a/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs b/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs index 3f4b3c69bb..9edbe95a38 100644 --- a/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs +++ b/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs @@ -2,8 +2,8 @@ //! `WaitingFor::DeclareAttackers` prompt when the active player has no legal //! attack declaration available. //! -//! CR 508.1a: 0 attackers is always a legal declaration, and the turn-based -//! action still runs even when nothing can be declared — only the +//! CR 508.1a/d: When no nonempty declaration is legal, declaring zero attackers +//! is legal and the turn-based action still runs — only the //! interactive prompt should be elided (mirroring how `DeclareBlockers` //! already collapses when `valid_blocker_ids` is empty). //! @@ -152,8 +152,9 @@ fn declare_attackers_prompt_skipped_when_every_candidate_has_no_legal_target() { let mut runner = scenario.build(); - // CR 702.16e-style protection from everything on the only opponent - // (Teferi's Protection), for the rest of the turn. `get_valid_attack_targets` + // CR 702.16j protection from everything on the only opponent + // (Teferi's Protection) prevents the opponent from being an attack target + // for the rest of the turn. `get_valid_attack_targets` // already excludes protected players (see // `get_valid_attack_targets_excludes_protected_player` in combat.rs), so // with a single opponent this empties the aggregate `valid_attack_targets` From c3427726cde56516bb6b00c852dcc28c58dd7eec Mon Sep 17 00:00:00 2001 From: Clayton Date: Fri, 31 Jul 2026 15:51:33 -0500 Subject: [PATCH 4/4] fix(PR-6845): replace rules-incorrect fixture in target-legality regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified CR 702.16j: protection from everything (targeting, attachment, blocking, damage) does not prohibit declaring attackers against that player. The target-legality regression test was relying on a pre-existing exclusion of protected players in get_valid_attack_targets, which codified that gap rather than exercising the intended "candidate exists, every target prohibited" path. Replace it with a real CR 508.1c temporary attack prohibition (GameRestriction::ProhibitActivity { activity: ProhibitedActivity::Attack { .. } }) barring P0 from attacking its only opponent — the same restriction already covered by temporary_attack_prohibition_bars_only_the_protected_player in rules/combat.rs, applied here through the DeclareAttackers prompt path. This is a genuine per-target hard restriction consulted inside attacker_can_attack_target, not a candidate-level gate, so it leaves valid_attacker_ids non-empty while emptying valid_attack_targets — the exact split the fix's predicate must detect. Added an explicit precondition assertion on the built DeclareAttackers payload (nonempty candidates, empty aggregate targets) before driving prompt elision. --- ..._6463_no_eligible_attackers_skip_prompt.rs | 96 +++++++++++-------- 1 file changed, 58 insertions(+), 38 deletions(-) diff --git a/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs b/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs index 9edbe95a38..69c557ee24 100644 --- a/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs +++ b/crates/engine/tests/integration/issue_6463_no_eligible_attackers_skip_prompt.rs @@ -21,23 +21,21 @@ //! whether any creature can actually attack. //! - `valid_attacker_ids` is non-empty (an untapped, non-sick creature is a //! valid *candidate*) but every candidate's per-attacker legal-target list -//! is empty — e.g. the only opponent has protection from everything. The -//! aggregate `valid_attack_targets` is empty even though -//! `valid_attacker_ids` is not, and `has_potential_attackers` never checks -//! target legality, so this reaches `Phase::DeclareAttackers` through the -//! ordinary (trigger-free) path. +//! is empty — e.g. a CR 508.1c temporary attack prohibition bars attacking +//! the only opponent. The aggregate `valid_attack_targets` is empty even +//! though `valid_attacker_ids` is not, and `has_potential_attackers` never +//! checks target legality, so this reaches `Phase::DeclareAttackers` +//! through the ordinary (trigger-free) path. +use engine::game::combat::build_declare_attackers_waiting_for; use engine::game::scenario::{GameScenario, P0, P1}; -use engine::game::zones::create_object; use engine::types::ability::{ - AbilityDefinition, AbilityKind, ContinuousModification, Duration, Effect, QuantityExpr, - TargetFilter, TriggerConstraint, TriggerDefinition, + AbilityDefinition, AbilityKind, Effect, GameRestriction, ProhibitedActivity, QuantityExpr, + RestrictionExpiry, RestrictionPlayerScope, TargetFilter, TriggerConstraint, TriggerDefinition, }; use engine::types::actions::{DebugAction, GameAction}; use engine::types::game_state::WaitingFor; -use engine::types::identifiers::CardId; -use engine::types::keywords::{Keyword, ProtectionTarget}; use engine::types::phase::Phase; -use engine::types::triggers::TriggerMode; +use engine::types::triggers::{AttackTargetFilter, TriggerMode}; use engine::types::zones::Zone; /// Drive priority forward from precombat main until either the interactive @@ -134,8 +132,8 @@ fn declare_attackers_prompt_skipped_when_no_legal_attackers_exist() { } /// The candidate set can be non-empty (an untapped, non-sick creature) while -/// every candidate's legal-target list is still empty — e.g. the sole -/// opponent has protection from everything. Checking only +/// every candidate's legal-target list is still empty — e.g. a temporary +/// attack prohibition bars attacking the sole opponent. Checking only /// `valid_attacker_ids` misses this: it stays non-empty even though no /// non-empty attack declaration is actually possible. The engine must check /// the aggregate `valid_attack_targets` instead. @@ -148,34 +146,56 @@ fn declare_attackers_prompt_skipped_when_every_candidate_has_no_legal_target() { // `has_potential_attackers` (the coarse BeginCombat gate) never checks // per-target legality, so this reaches Phase::DeclareAttackers through // the ordinary trigger-free path — no begin-of-combat trigger needed. - scenario.add_creature(P0, "Fyndhorn Elves", 1, 1); + let source = scenario.add_creature(P0, "Fyndhorn Elves", 1, 1).id(); let mut runner = scenario.build(); - // CR 702.16j protection from everything on the only opponent - // (Teferi's Protection) prevents the opponent from being an attack target - // for the rest of the turn. `get_valid_attack_targets` - // already excludes protected players (see - // `get_valid_attack_targets_excludes_protected_player` in combat.rs), so - // with a single opponent this empties the aggregate `valid_attack_targets` - // even though `valid_attacker_ids` stays non-empty. - let source = create_object( - runner.state_mut(), - CardId(999), - P1, - "Teferi's Protection".to_string(), - Zone::Battlefield, - ); - runner.state_mut().add_transient_continuous_effect( - source, - P1, - Duration::UntilEndOfTurn, - TargetFilter::SpecificPlayer { id: P1 }, - vec![ContinuousModification::AddKeyword { - keyword: Keyword::Protection(ProtectionTarget::Everything), - }], - None, - ); + // CR 508.1c + CR 109.5: a temporary attack prohibition ("players can't + // attack P1 this turn", a `ProhibitActivity::Attack` restriction) bars + // declaring an attack against the only opponent. This is a per-target + // hard restriction consulted inside `attacker_can_attack_target` + // (`attack_passes_temporary_prohibition`), NOT a candidate-level gate — + // `creature_cant_attack_gated` (which builds `valid_attacker_ids`) never + // consults `state.restrictions` — so the creature remains a valid + // candidate while its legal-target list becomes empty. With only one + // opponent, that empties the aggregate `valid_attack_targets` too. See + // `temporary_attack_prohibition_bars_only_the_protected_player` in + // rules/combat.rs for the same restriction exercised directly against + // `DeclareAttackers` validation. + runner + .state_mut() + .restrictions + .push(GameRestriction::ProhibitActivity { + source, + affected_players: RestrictionPlayerScope::AllPlayers, + expiry: RestrictionExpiry::EndOfTurn, + activity: ProhibitedActivity::Attack { + defended: AttackTargetFilter::PlayerOrPlaneswalker, + protected_player: Some(P1), + }, + }); + + // Precondition: the candidate set is non-empty but the aggregate legal- + // target set is empty — the exact split the fix must detect. + match build_declare_attackers_waiting_for(runner.state()) { + WaitingFor::DeclareAttackers { + valid_attacker_ids, + valid_attack_targets, + .. + } => { + assert!( + !valid_attacker_ids.is_empty(), + "precondition: the creature must remain a valid candidate \ + despite the attack prohibition" + ); + assert!( + valid_attack_targets.is_empty(), + "precondition: the only opponent is barred, so no legal \ + attack target remains" + ); + } + other => panic!("expected DeclareAttackers, got {other:?}"), + } assert_never_prompts_and_reaches_postcombat_main(&mut runner); }