From 9ec5050e4327063d22c83f0373078c814ea029f8 Mon Sep 17 00:00:00 2001 From: glorydavid03023 Date: Fri, 3 Jul 2026 17:57:56 +0900 Subject: [PATCH 1/3] fix(parser+engine): parse "after blockers are declared" casting window (closes #4996) Spells reading "Cast this spell only during combat after blockers are declared." (Aleatory, Chaotic Strike, Curtain of Light, Flash Foliage) lost the "after blockers are declared" qualifier: the timing scanner dispatched "during"/"before"/"on" but had only a lone "after combat" leaf, so the phrase fell through and the spell became castable during all of combat (CR 601.3 timing violation). Add an "after" prefix dispatch (parse_after_phrase) mirroring parse_before_phrase, a CastingRestriction::AfterBlockersDeclared variant, and its enforcement window Phase::DeclareBlockers | CombatDamage | EndCombat -- the exact complement of BeforeBlockersDeclared within combat (CR 509.1 + CR 506.1). Building-block parser test plus a runtime enforcement test covering the full combat-step partition. --- crates/engine/src/game/restrictions.rs | 54 ++++++++++++++++++++++ crates/engine/src/parser/oracle_casting.rs | 36 ++++++++++++++- crates/engine/src/types/ability.rs | 1 + 3 files changed, 90 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/game/restrictions.rs b/crates/engine/src/game/restrictions.rs index b2262575a5..a112a77b33 100644 --- a/crates/engine/src/game/restrictions.rs +++ b/crates/engine/src/game/restrictions.rs @@ -975,6 +975,14 @@ fn casting_restriction_applies( matches!(state.phase, Phase::BeginCombat | Phase::DeclareAttackers) } CastingRestriction::BeforeCombatDamage => is_before_combat_damage(state.phase), + // CR 509.1 + CR 506.1: "after blockers are declared" opens once the + // declare-blockers step's turn-based action has put blockers in place + // and stays open through combat damage (CR 510) and end of combat + // (CR 511). Exact complement of `BeforeBlockersDeclared` within combat. + CastingRestriction::AfterBlockersDeclared => matches!( + state.phase, + Phase::DeclareBlockers | Phase::CombatDamage | Phase::EndCombat + ), CastingRestriction::AfterCombat => matches!( state.phase, Phase::EndCombat | Phase::PostCombatMain | Phase::End | Phase::Cleanup @@ -2693,6 +2701,52 @@ mod tests { )); } + #[test] + fn after_blockers_declared_window_spans_declare_blockers_through_end_of_combat() { + // CR 509.1 + CR 506.1: the window opens once blockers are declared and + // stays open through the rest of combat. Exact complement of + // BeforeBlockersDeclared, so together they partition the five combat + // steps. + let mut state = crate::types::game_state::GameState::new_two_player(42); + + // Before blockers are declared — excluded. + for phase in [Phase::BeginCombat, Phase::DeclareAttackers] { + state.phase = phase; + assert!( + !casting_restriction_applies( + &state, + PlayerId(0), + ObjectId(1), + &CastingRestriction::AfterBlockersDeclared + ), + "{phase:?} precedes blocker declaration and must be excluded" + ); + } + + // After blockers are declared — included. + for phase in [Phase::DeclareBlockers, Phase::CombatDamage, Phase::EndCombat] { + state.phase = phase; + assert!( + casting_restriction_applies( + &state, + PlayerId(0), + ObjectId(1), + &CastingRestriction::AfterBlockersDeclared + ), + "{phase:?} follows blocker declaration and must be included" + ); + } + + // Outside combat — excluded. + state.phase = Phase::PostCombatMain; + assert!(!casting_restriction_applies( + &state, + PlayerId(0), + ObjectId(1), + &CastingRestriction::AfterBlockersDeclared + )); + } + #[test] fn evaluates_opponent_searched_library_this_turn_condition() { let mut state = crate::types::game_state::GameState::new_two_player(42); diff --git a/crates/engine/src/parser/oracle_casting.rs b/crates/engine/src/parser/oracle_casting.rs index 9da2d6e6c1..1bf8c3af16 100644 --- a/crates/engine/src/parser/oracle_casting.rs +++ b/crates/engine/src/parser/oracle_casting.rs @@ -611,6 +611,7 @@ fn parse_timing_restriction( alt(( preceded(tag("during "), parse_during_phrase), preceded(tag("before "), parse_before_phrase), + preceded(tag("after "), parse_after_phrase), preceded( tag("on "), alt(( @@ -618,7 +619,6 @@ fn parse_timing_restriction( value(CastingRestriction::DuringYourTurn, tag("your turn")), )), ), - value(CastingRestriction::AfterCombat, tag("after combat")), value(CastingRestriction::AsSorcery, tag("as a sorcery")), )) .parse(input) @@ -712,6 +712,21 @@ fn parse_before_phrase(input: &str) -> nom::IResult<&str, CastingRestriction, Or .parse(input) } +/// Sub-dispatch for "after [rest]" — blockers declared, combat. Mirror of +/// `parse_before_phrase`: "after blockers are declared" is the complement of +/// "before blockers are declared" within the combat phase (Aleatory, Chaotic +/// Strike, Curtain of Light, Flash Foliage). +fn parse_after_phrase(input: &str) -> nom::IResult<&str, CastingRestriction, OracleError<'_>> { + alt(( + value( + CastingRestriction::AfterBlockersDeclared, + tag("blockers are declared"), + ), + value(CastingRestriction::AfterCombat, tag("combat")), + )) + .parse(input) +} + /// Walk `text` word-by-word, collecting all timing restrictions found via nom combinators. /// Tries `parse_timing_restriction` at each word boundary — on match, consumes the phrase /// and advances; on miss, skips to the next word. @@ -940,6 +955,25 @@ mod tests { assert!(restrictions.contains(&CastingRestriction::BeforeBlockersDeclared)); } + #[test] + fn spell_cast_restriction_handles_combat_after_blockers() { + // Aleatory, Chaotic Strike, Curtain of Light, Flash Foliage all print + // this exact line. The "after blockers are declared" qualifier must be + // captured, not silently dropped, or the spell becomes castable during + // all of combat (CR 601.3 timing violation). + let restrictions = parse_casting_restriction_line( + "Cast this spell only during combat after blockers are declared.", + ) + .expect("restrictions should parse"); + assert!(restrictions.contains(&CastingRestriction::DuringCombat)); + assert!(restrictions.contains(&CastingRestriction::AfterBlockersDeclared)); + // "after combat" must still parse to the distinct AfterCombat window. + let after_combat = parse_casting_restriction_line("Cast this spell only after combat.") + .expect("restrictions should parse"); + assert!(after_combat.contains(&CastingRestriction::AfterCombat)); + assert!(!after_combat.contains(&CastingRestriction::AfterBlockersDeclared)); + } + #[test] fn parse_additional_cost_optional_blight() { let lower = "as an additional cost to cast this spell, you may blight 1."; diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index e59d220d6b..7584a67a36 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -13803,6 +13803,7 @@ pub enum CastingRestriction { BeforeAttackersDeclared, BeforeBlockersDeclared, BeforeCombatDamage, + AfterBlockersDeclared, AfterCombat, RequiresCondition { condition: Option }, } From 625ea621210c64966999baa6a1638304045f56a4 Mon Sep 17 00:00:00 2001 From: glorydavid03023 Date: Fri, 3 Jul 2026 18:11:10 +0900 Subject: [PATCH 2/3] style: rustfmt the after-blockers enforcement test array --- crates/engine/src/game/restrictions.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/game/restrictions.rs b/crates/engine/src/game/restrictions.rs index a112a77b33..ad07dbf527 100644 --- a/crates/engine/src/game/restrictions.rs +++ b/crates/engine/src/game/restrictions.rs @@ -2724,7 +2724,11 @@ mod tests { } // After blockers are declared — included. - for phase in [Phase::DeclareBlockers, Phase::CombatDamage, Phase::EndCombat] { + for phase in [ + Phase::DeclareBlockers, + Phase::CombatDamage, + Phase::EndCombat, + ] { state.phase = phase; assert!( casting_restriction_applies( From bbbb228130c9a7e274dddeb0e53a262d996acc2d Mon Sep 17 00:00:00 2001 From: glorydavid03023 Date: Sat, 4 Jul 2026 14:04:48 +0900 Subject: [PATCH 3/3] fix(engine): gate after-blockers-declared on the CR 509.1 turn-based action Address review: the runtime gate was phase-only (matches!(state.phase, DeclareBlockers | CombatDamage | EndCombat)), which admitted the pre-declaration declare-blockers priority window and combats where the declare-blockers step was skipped (CR 506.7f). Gate instead on CombatState::blockers_declared_by, the engine record of the CR 509.1 declare-blockers turn-based action. It is populated when a defending player declares blockers -- including a declaration of zero blockers, which distinguishes the post-declaration empty-blockers state from the pre-declaration one -- and is never cleared mid-combat, so the window stays open through combat damage and end of combat. Require is_combat() as well so the window closes outside combat. Extract is_after_blockers_declared(state) mirroring is_before_*; rewrite the runtime test to cover pre-vs-post declaration, persistence through combat damage / end of combat, the skipped-declare-blockers combat, the before-blockers phases, and outside-combat. --- crates/engine/src/game/restrictions.rs | 118 ++++++++++++++++--------- 1 file changed, 77 insertions(+), 41 deletions(-) diff --git a/crates/engine/src/game/restrictions.rs b/crates/engine/src/game/restrictions.rs index b9263a45fd..2467128d5f 100644 --- a/crates/engine/src/game/restrictions.rs +++ b/crates/engine/src/game/restrictions.rs @@ -971,14 +971,7 @@ fn casting_restriction_applies( matches!(state.phase, Phase::BeginCombat | Phase::DeclareAttackers) } CastingRestriction::BeforeCombatDamage => is_before_combat_damage(state.phase), - // CR 509.1 + CR 506.1: "after blockers are declared" opens once the - // declare-blockers step's turn-based action has put blockers in place - // and stays open through combat damage (CR 510) and end of combat - // (CR 511). Exact complement of `BeforeBlockersDeclared` within combat. - CastingRestriction::AfterBlockersDeclared => matches!( - state.phase, - Phase::DeclareBlockers | Phase::CombatDamage | Phase::EndCombat - ), + CastingRestriction::AfterBlockersDeclared => is_after_blockers_declared(state), CastingRestriction::AfterCombat => matches!( state.phase, Phase::EndCombat | Phase::PostCombatMain | Phase::End | Phase::Cleanup @@ -1794,6 +1787,24 @@ fn is_before_combat_damage(phase: Phase) -> bool { ) } +/// CR 509.1 + CR 506.7f: "after blockers are declared" is true only once the +/// declare-blockers turn-based action (CR 509.1) has actually recorded a +/// declaration this combat — not merely that the phase label reads +/// DeclareBlockers. `CombatState::blockers_declared_by` is populated when a +/// defending player declares blockers (including a declaration of zero blockers) +/// and is never cleared mid-combat, so the window stays open through combat +/// damage (CR 510) and end of combat (CR 511). Gating on the phase label alone +/// would wrongly admit the pre-declaration declare-blockers priority window and +/// combats where the declare-blockers step was skipped (CR 506.7f: the spell +/// can't be cast in that combat at all). +fn is_after_blockers_declared(state: &crate::types::game_state::GameState) -> bool { + state.phase.is_combat() + && state + .combat + .as_ref() + .is_some_and(|combat| !combat.blockers_declared_by.is_empty()) +} + fn you_control_creature_with_keyword( state: &crate::types::game_state::GameState, player: PlayerId, @@ -2698,53 +2709,78 @@ mod tests { } #[test] - fn after_blockers_declared_window_spans_declare_blockers_through_end_of_combat() { - // CR 509.1 + CR 506.1: the window opens once blockers are declared and - // stays open through the rest of combat. Exact complement of - // BeforeBlockersDeclared, so together they partition the five combat - // steps. + fn after_blockers_declared_requires_the_declare_blockers_turn_based_action() { + // CR 509.1 + CR 506.7f: the window opens only once the declare-blockers + // turn-based action has actually run this combat (tracked by + // `CombatState::blockers_declared_by`) — the phase label alone is not + // enough. It must exclude the pre-declaration declare-blockers priority + // window and any combat where the declare-blockers step was skipped. + use crate::game::combat::CombatState; + let restriction = CastingRestriction::AfterBlockersDeclared; + let applies = |state: &crate::types::game_state::GameState| { + casting_restriction_applies(state, PlayerId(0), ObjectId(1), &restriction) + }; + let declared = || CombatState { + blockers_declared_by: vec![PlayerId(1)], + ..CombatState::default() + }; let mut state = crate::types::game_state::GameState::new_two_player(42); - // Before blockers are declared — excluded. - for phase in [Phase::BeginCombat, Phase::DeclareAttackers] { - state.phase = phase; - assert!( - !casting_restriction_applies( - &state, - PlayerId(0), - ObjectId(1), - &CastingRestriction::AfterBlockersDeclared - ), - "{phase:?} precedes blocker declaration and must be excluded" - ); - } + // Pre-declaration: in the declare-blockers step but the turn-based + // action hasn't recorded a declaration yet — excluded. + state.phase = Phase::DeclareBlockers; + state.combat = Some(CombatState::default()); + assert!( + !applies(&state), + "the pre-declaration declare-blockers priority window must be excluded" + ); - // After blockers are declared — included. + // Post-declaration in the same step, and persisting through combat + // damage and end of combat — included. (blockers_declared_by is never + // cleared mid-combat.) for phase in [ Phase::DeclareBlockers, Phase::CombatDamage, Phase::EndCombat, ] { state.phase = phase; + state.combat = Some(declared()); assert!( - casting_restriction_applies( - &state, - PlayerId(0), - ObjectId(1), - &CastingRestriction::AfterBlockersDeclared - ), - "{phase:?} follows blocker declaration and must be included" + applies(&state), + "{phase:?} after the declare-blockers action must be included" ); } - // Outside combat — excluded. + // CR 506.7f: a combat where the declare-blockers step was skipped never + // records a declaration, so even combat damage / end of combat are + // excluded. + for phase in [Phase::CombatDamage, Phase::EndCombat] { + state.phase = phase; + state.combat = Some(CombatState::default()); + assert!( + !applies(&state), + "{phase:?} in a skipped-declare-blockers combat must be excluded" + ); + } + + // Before blockers are declared: begin-combat and declare-attackers have + // no recorded declaration — excluded. + for phase in [Phase::BeginCombat, Phase::DeclareAttackers] { + state.phase = phase; + state.combat = Some(CombatState::default()); + assert!( + !applies(&state), + "{phase:?} precedes blocker declaration and must be excluded" + ); + } + + // Outside combat entirely — excluded even if a stale declaration flag + // somehow survived (the phase guard closes the window). state.phase = Phase::PostCombatMain; - assert!(!casting_restriction_applies( - &state, - PlayerId(0), - ObjectId(1), - &CastingRestriction::AfterBlockersDeclared - )); + state.combat = Some(declared()); + assert!(!applies(&state), "post-combat main must be excluded"); + state.combat = None; + assert!(!applies(&state), "no combat state must be excluded"); } #[test]