From 40d0269ab6151e26a022390cc4519085a1a50bd5 Mon Sep 17 00:00:00 2001 From: Nicholas Tindle Date: Mon, 29 Jun 2026 00:39:55 -0500 Subject: [PATCH 1/3] fix(parser): UNSUPPORTED one-off: Ace's Baseball Bat --- crates/engine/src/ai_support/filter.rs | 2 +- crates/engine/src/game/combat.rs | 227 +++++++++++++---- crates/engine/src/game/coverage.rs | 11 +- crates/engine/src/game/static_abilities.rs | 7 +- .../src/parser/oracle_effect/imperative.rs | 21 +- crates/engine/src/parser/oracle_effect/mod.rs | 13 +- .../src/parser/oracle_effect/subject.rs | 12 +- .../src/parser/oracle_static/dispatch.rs | 34 ++- .../src/parser/oracle_static/evasion.rs | 2 +- .../src/parser/oracle_static/grammar.rs | 185 ++++++++++---- .../engine/src/parser/oracle_static/shared.rs | 65 ++--- .../engine/src/parser/oracle_static/tests.rs | 233 ++++++++++++++---- crates/engine/src/parser/swallow_check.rs | 57 ++++- crates/engine/src/types/statics.rs | 35 ++- crates/engine/tests/squirrel_perf_probe.rs | 2 +- .../src/convert/static_effect.rs | 2 +- 16 files changed, 688 insertions(+), 220 deletions(-) diff --git a/crates/engine/src/ai_support/filter.rs b/crates/engine/src/ai_support/filter.rs index 9649f93cf6..9be1757768 100644 --- a/crates/engine/src/ai_support/filter.rs +++ b/crates/engine/src/ai_support/filter.rs @@ -863,7 +863,7 @@ impl LegalityPoisonGates { | StaticMode::BlockRestriction { .. } | StaticMode::MustBlock | StaticMode::MustBlockAttacker { .. } - | StaticMode::MustBeBlocked + | StaticMode::MustBeBlocked { .. } | StaticMode::MustBeBlockedByAll | StaticMode::MaxBlockersEachCombat { .. } | StaticMode::ExtraBlockers { .. } diff --git a/crates/engine/src/game/combat.rs b/crates/engine/src/game/combat.rs index 1ae658c8c9..20740bd73d 100644 --- a/crates/engine/src/game/combat.rs +++ b/crates/engine/src/game/combat.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use super::game_object::GameObject; use super::players; use crate::game::filter::{matches_target_filter, FilterContext}; -use crate::types::ability::{StaticDefinition, TargetRef}; +use crate::types::ability::{StaticDefinition, TargetFilter, TargetRef}; use crate::types::card_type::{CoreType, Supertype}; use crate::types::events::GameEvent; use crate::types::game_state::GameState; @@ -717,7 +717,7 @@ pub fn collect_must_be_blocked_statics(state: &GameState) -> Vec<(ObjectId, Stat .filter(|(_, def)| { matches!( def.mode, - StaticMode::MustBeBlocked | StaticMode::MustBeBlockedByAll + StaticMode::MustBeBlocked { .. } | StaticMode::MustBeBlockedByAll ) }) .map(|(src, def)| (src.id, def.clone())) @@ -877,14 +877,23 @@ fn blocker_has_cant_block_static_from_precomputed( .is_some() } -/// CR 509.1c: precomputed-slice variant of `attacker_has_must_be_blocked`. -fn attacker_has_must_be_blocked_from_precomputed( - state: &GameState, +/// CR 509.1c: each `MustBeBlocked` requirement functioning on `attacker_id`, +/// paired with its optional blocker filter (`None` = any blocker satisfies the +/// requirement; `Some(filter)` = only a blocker matching `filter` does) and the +/// source id (re-resolves the controller for `FilterContext`). The bare and +/// filtered forms are the same CR 509.1c blocking requirement parameterized on +/// the blocker-set axis; `MustBeBlockedByAll` is a distinct requirement handled +/// by its own loop. +fn must_be_blocked_requirements_for_attacker<'a>( + state: &'a GameState, attacker_id: ObjectId, - precomputed: &[(ObjectId, StaticDefinition)], -) -> bool { + precomputed: &'a [(ObjectId, StaticDefinition)], +) -> impl Iterator, ObjectId)> + 'a { must_be_blocked_statics_for_attacker_from_precomputed(state, attacker_id, precomputed) - .any(|(def, _)| def.mode == StaticMode::MustBeBlocked) + .filter_map(|(def, src_id)| match &def.mode { + StaticMode::MustBeBlocked { by } => Some((by.as_ref(), src_id)), + _ => None, // MustBeBlockedByAll handled by its own loop + }) } /// CR 509.1c: precomputed-slice variant of `attacker_has_must_be_blocked_by_all`. @@ -1343,9 +1352,15 @@ pub fn validate_blockers_for_player( } } - // CR 509.1c: MustBeBlocked — if a creature with "must be blocked if able" is attacking, - // the defending player must assign at least one blocker to it, provided a legal blocker - // exists that isn't already required elsewhere. + // CR 509.1c: MustBeBlocked — if a creature with a "must be blocked if able" + // requirement is attacking, the defending player must obey it by assigning a + // qualifying blocker whenever one is able. Each requirement on the attacker + // is enforced independently (an attacker may carry both a bare and a filtered + // requirement): `by == None` ⇒ any assigned blocker satisfies it; `by == + // Some(filter)` ⇒ only an assigned blocker matching `filter` does (Ace's + // Baseball Bat: a Dalek; Slayer's Cleaver: an Eldrazi). Uses the engine's + // existing per-attacker greedy approximation of the CR 509.1c requirement- + // maximization rule, applied uniformly to the bare and filtered forms. if let Some(combat) = &state.combat { // Collect all assigned blocker IDs for quick lookup let assigned_blockers: std::collections::HashSet = assignments @@ -1359,44 +1374,74 @@ pub fn validate_blockers_for_player( } let attacker_id = attacker_info.object_id; - if !attacker_has_must_be_blocked_from_precomputed(state, attacker_id, &must_be_blocked) - { - continue; - } - - // Already has at least one blocker assigned — constraint satisfied - if blockers_per_attacker.contains_key(&attacker_id) { - continue; - } + let requirements: Vec<(Option<&TargetFilter>, ObjectId)> = + must_be_blocked_requirements_for_attacker(state, attacker_id, &must_be_blocked) + .collect(); - // Check if any unassigned defending creature could legally block this attacker. - // If so, the assignment is invalid because that creature should have been assigned. - let has_available_blocker = state.battlefield.iter().any(|id| { - if assigned_blockers.contains(id) { - return false; + for (by, src_id) in requirements { + // CR 509.1c: the requirement is obeyed if a qualifying blocker is + // already assigned to this attacker — `None` ⇒ any assigned + // blocker; `Some(filter)` ⇒ an assigned blocker matching `filter`. + let satisfied = blockers_per_attacker + .get(&attacker_id) + .is_some_and(|blockers| { + blockers.iter().any(|blocker_id| match by { + None => true, + Some(filter) => matches_target_filter( + state, + *blocker_id, + filter, + &FilterContext::from_source(state, src_id), + ), + }) + }); + if satisfied { + continue; } - let Some(obj) = state.objects.get(id) else { - return false; - }; - obj.controller == player - && obj.card_types.core_types.contains(&CoreType::Creature) - && !obj.tapped - && can_block_pair_with_precomputed( - state, - *id, - attacker_id, - &blocker_restriction, - &block_restriction, - &blocker_allowed, - can_block_shadow_exists, - ) - }); - if has_available_blocker { - return Err(format!( - "{:?} must be blocked if able (CR 509.1c)", - attacker_id - )); + // Check if any unassigned defending creature could legally block + // this attacker AND (for the filtered form) match the filter. If + // so, the declaration is illegal because that creature should + // have been assigned. CR 509.1b: a creature that can't legally + // block doesn't make the requirement obey-able. + let has_available_blocker = state.battlefield.iter().any(|id| { + if assigned_blockers.contains(id) { + return false; + } + let Some(obj) = state.objects.get(id) else { + return false; + }; + obj.controller == player + && obj.card_types.core_types.contains(&CoreType::Creature) + && !obj.tapped + && can_block_pair_with_precomputed( + state, + *id, + attacker_id, + &blocker_restriction, + &block_restriction, + &blocker_allowed, + can_block_shadow_exists, + ) + && match by { + None => true, + Some(filter) => matches_target_filter( + state, + *id, + filter, + &FilterContext::from_source(state, src_id), + ), + } + }); + + if has_available_blocker { + return Err(match by { + None => format!("{attacker_id:?} must be blocked if able (CR 509.1c)"), + Some(_) => format!( + "{attacker_id:?} must be blocked by a qualifying creature if able (CR 509.1c)" + ), + }); + } } } @@ -6306,9 +6351,11 @@ mod tests { aura_obj.card_types.core_types.push(CoreType::Enchantment); aura_obj.attached_to = Some(attacker.into()); aura_obj.static_definitions.push( - StaticDefinition::new(StaticMode::MustBeBlocked).affected(TargetFilter::Typed( - TypedFilter::creature().properties(vec![FilterProp::EnchantedBy]), - )), + StaticDefinition::new(StaticMode::MustBeBlocked { by: None }).affected( + TargetFilter::Typed( + TypedFilter::creature().properties(vec![FilterProp::EnchantedBy]), + ), + ), ); state.combat = Some(CombatState { @@ -6983,7 +7030,9 @@ mod tests { .get_mut(&id) .unwrap() .static_definitions - .push(StaticDefinition::new(StaticMode::MustBeBlocked)); + .push(StaticDefinition::new(StaticMode::MustBeBlocked { + by: None, + })); } #[test] @@ -7026,6 +7075,80 @@ mod tests { assert!(validate_blockers(&state, &[]).is_ok()); } + /// Helper: a `MustBeBlocked { by: Some() }` requirement on `id` + /// (Ace's Baseball Bat: "must be blocked by a Dalek if able"). + fn add_must_be_blocked_by_dalek(state: &mut GameState, id: ObjectId) { + let dalek = TargetFilter::Typed( + crate::types::ability::TypedFilter::default().subtype("Dalek".to_string()), + ); + state + .objects + .get_mut(&id) + .unwrap() + .static_definitions + .push(StaticDefinition::new(StaticMode::MustBeBlocked { + by: Some(dalek), + })); + } + + /// CR 509.1c: the FILTERED "must be blocked by a Dalek if able" requirement + /// (Ace's Baseball Bat) reaches and is enforced by the declare-blockers + /// validator: when the defender controls an untapped Dalek able to block, the + /// declaration is illegal unless a Dalek is assigned — a non-Dalek block does + /// not satisfy the requirement. This is the runtime proof that the parsed + /// `MustBeBlocked { by: Some(Dalek) }` static reaches `validate_blockers`. + #[test] + fn must_be_blocked_by_subtype_requires_matching_blocker() { + let mut state = setup(); + let attacker = create_creature(&mut state, PlayerId(0), "Ace", 3, 3); + add_must_be_blocked_by_dalek(&mut state, attacker); + let dalek = create_creature(&mut state, PlayerId(1), "Dalek Drone", 2, 2); + state + .objects + .get_mut(&dalek) + .unwrap() + .card_types + .subtypes + .push("Dalek".to_string()); + let non_dalek = create_creature(&mut state, PlayerId(1), "Bear", 2, 2); + + state.combat = Some(CombatState { + attackers: vec![AttackerInfo::attacking_player(attacker, PlayerId(1))], + ..Default::default() + }); + + // No blockers: illegal — an able Dalek is left idle. + assert!(validate_blockers(&state, &[]).is_err()); + // Only the non-Dalek blocks: still illegal — the requirement is unobeyed + // while a Dalek is able to block. + assert!(validate_blockers(&state, &[(non_dalek, attacker)]).is_err()); + // The Dalek blocks: legal — the requirement is obeyed. + assert!(validate_blockers(&state, &[(dalek, attacker)]).is_ok()); + // Both blockers assigned (Dalek satisfies it): legal. + assert!(validate_blockers(&state, &[(dalek, attacker), (non_dalek, attacker)]).is_ok()); + } + + /// CR 509.1c "if able": with NO able Dalek, the filtered requirement is + /// satisfied vacuously — the defender may block with anything or not block. + #[test] + fn must_be_blocked_by_subtype_vacuous_when_no_dalek_able() { + let mut state = setup(); + let attacker = create_creature(&mut state, PlayerId(0), "Ace", 3, 3); + add_must_be_blocked_by_dalek(&mut state, attacker); + // Defender controls only a non-Dalek creature. + let non_dalek = create_creature(&mut state, PlayerId(1), "Bear", 2, 2); + + state.combat = Some(CombatState { + attackers: vec![AttackerInfo::attacking_player(attacker, PlayerId(1))], + ..Default::default() + }); + + // No Dalek able → requirement vacuously satisfied: an empty block is + // legal, and a non-Dalek block is also legal. + assert!(validate_blockers(&state, &[]).is_ok()); + assert!(validate_blockers(&state, &[(non_dalek, attacker)]).is_ok()); + } + fn add_must_be_blocked_by_all(state: &mut GameState, id: ObjectId) { state .objects @@ -7226,10 +7349,10 @@ mod tests { let attacker = create_creature(&mut state, PlayerId(0), "Lure Beast", 3, 3); let blocker = create_creature(&mut state, PlayerId(1), "Bear", 2, 2); - let static_def = StaticDefinition::new(StaticMode::MustBeBlocked) + let static_def = StaticDefinition::new(StaticMode::MustBeBlocked { by: None }) .affected(TargetFilter::SpecificObject { id: attacker }) .modifications(vec![ContinuousModification::AddStaticMode { - mode: StaticMode::MustBeBlocked, + mode: StaticMode::MustBeBlocked { by: None }, }]); let ability = ResolvedAbility::new( Effect::GenericEffect { diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 2c5060ce0f..32fb13572b 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -107,6 +107,13 @@ fn is_data_carrying_static(mode: &StaticMode) -> bool { | StaticMode::MaximumHandSize { .. } | StaticMode::StepEndUnspentMana { .. } | StaticMode::CantBeBlockedBy { .. } + // CR 509.1c: MustBeBlocked carries an optional blocker `TargetFilter` + // (None = any blocker; Some = "must be blocked by a "). The + // None shape is no longer registry-keyed (the variant is now + // parameterized with a non-Hash TargetFilter); runtime enforcement is + // direct-match in combat.rs declare-blockers validation (mirrors + // CantBeBlockedBy). + | StaticMode::MustBeBlocked { .. } // CR 509.1b: CantBeBlockedExceptBy carries `kind`. | StaticMode::CantBeBlockedExceptBy { .. } // CR 702.39a + CR 509.1c: MustBlockAttacker carries the `ObjectId` of @@ -10362,10 +10369,10 @@ mod tests { AbilityKind::Spell, Effect::GenericEffect { static_abilities: vec![StaticDefinition { - mode: StaticMode::MustBeBlocked, + mode: StaticMode::MustBeBlocked { by: None }, affected: None, modifications: vec![ContinuousModification::AddStaticMode { - mode: StaticMode::MustBeBlocked, + mode: StaticMode::MustBeBlocked { by: None }, }], condition: None, per_player_condition: None, diff --git a/crates/engine/src/game/static_abilities.rs b/crates/engine/src/game/static_abilities.rs index 8329c98b27..0082e32dd6 100644 --- a/crates/engine/src/game/static_abilities.rs +++ b/crates/engine/src/game/static_abilities.rs @@ -165,8 +165,11 @@ pub fn build_static_registry() -> HashMap { registry.insert(StaticMode::Lifelink, handle_static_lifelink); registry.insert(StaticMode::CantTap, handle_rule_mod); registry.insert(StaticMode::CantUntap, handle_rule_mod); - // CR 509.1c: MustBeBlocked — this creature must be blocked if able. - registry.insert(StaticMode::MustBeBlocked, handle_rule_mod); + // CR 509.1c: MustBeBlocked is now a parameterized, data-carrying variant + // (`by: Option`) — it cannot be an exact HashMap key, so it is + // NOT registry-keyed (mirrors CantBeBlockedBy). Coverage support is via + // coverage::is_data_carrying_static; runtime enforcement is direct-match in + // combat.rs declare-blockers validation. // CR 509.1c: MustBeBlockedByAll — every creature able to block this creature // must do so ("All creatures able to block ~ do so"; enforced in combat.rs). registry.insert(StaticMode::MustBeBlockedByAll, handle_rule_mod); diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index 847d2801ae..b186c0adf0 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -9596,16 +9596,17 @@ fn lower_imperative_family_effect(ast: ImperativeFamilyAst) -> Effect { // CR 509.1c: Must be blocked — grant transient MustBeBlocked static via GenericEffect. // Uses AddStaticMode so the mode propagates through the layer system to // static_definitions, where combat.rs checks it. - ImperativeFamilyAst::MustBeBlocked => { - Effect::GenericEffect { - static_abilities: vec![StaticDefinition::new(StaticMode::MustBeBlocked) - .modifications(vec![ContinuousModification::AddStaticMode { - mode: StaticMode::MustBeBlocked, - }])], - duration: Some(Duration::UntilEndOfTurn), - target: None, - } - } + ImperativeFamilyAst::MustBeBlocked => Effect::GenericEffect { + static_abilities: vec![ + StaticDefinition::new(StaticMode::MustBeBlocked { by: None }).modifications(vec![ + ContinuousModification::AddStaticMode { + mode: StaticMode::MustBeBlocked { by: None }, + }, + ]), + ], + duration: Some(Duration::UntilEndOfTurn), + target: None, + }, ImperativeFamilyAst::Investigate => Effect::Investigate, ImperativeFamilyAst::Learn => Effect::Learn, // CR 701.40a: Default subject is the controller ("you manifest..."). Subject diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 72f955f3e1..cddb999248 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -33805,7 +33805,10 @@ mod tests { duration, .. } => { - assert_eq!(static_abilities[0].mode, StaticMode::MustBeBlocked); + assert_eq!( + static_abilities[0].mode, + StaticMode::MustBeBlocked { by: None } + ); assert_eq!(*duration, Some(Duration::UntilEndOfTurn)); assert_eq!( static_abilities[0].affected, @@ -38896,7 +38899,7 @@ mod tests { assert!( matches!(&e, Effect::GenericEffect { static_abilities, .. } if static_abilities.iter().any(|sd| - sd.mode == crate::types::statics::StaticMode::MustBeBlocked + matches!(sd.mode, crate::types::statics::StaticMode::MustBeBlocked { by: None }) ) ), "Expected GenericEffect with MustBeBlocked, got {:?}", @@ -38911,7 +38914,7 @@ mod tests { assert!( matches!(&e, Effect::GenericEffect { static_abilities, .. } if static_abilities.iter().any(|sd| - sd.mode == crate::types::statics::StaticMode::MustBeBlocked + matches!(sd.mode, crate::types::statics::StaticMode::MustBeBlocked { by: None }) ) ), "Expected GenericEffect with MustBeBlocked, got {:?}", @@ -38940,7 +38943,7 @@ mod tests { assert!( matches!(&*sub.effect, Effect::GenericEffect { static_abilities, .. } if static_abilities.iter().any(|sd| - sd.mode == crate::types::statics::StaticMode::MustBeBlocked + matches!(sd.mode, crate::types::statics::StaticMode::MustBeBlocked { by: None }) ) ), "Expected sub_ability GenericEffect with MustBeBlocked, got {:?}", @@ -48316,7 +48319,7 @@ mod tests { assert!( matches!(&*sub.effect, Effect::GenericEffect { static_abilities, .. } if static_abilities.iter().any(|sd| - sd.mode == crate::types::statics::StaticMode::MustBeBlocked + matches!(sd.mode, crate::types::statics::StaticMode::MustBeBlocked { by: None }) ) ), "expected sub_ability GenericEffect with MustBeBlocked, got {:?}", diff --git a/crates/engine/src/parser/oracle_effect/subject.rs b/crates/engine/src/parser/oracle_effect/subject.rs index 20ec2e0f3b..53bc3099a8 100644 --- a/crates/engine/src/parser/oracle_effect/subject.rs +++ b/crates/engine/src/parser/oracle_effect/subject.rs @@ -982,11 +982,13 @@ fn try_parse_subject_restriction_clause( let affected = static_affected_for_application(&application); return Some(ParsedEffectClause { effect: Effect::GenericEffect { - static_abilities: vec![StaticDefinition::new(StaticMode::MustBeBlocked) - .affected(affected) - .modifications(vec![ContinuousModification::AddStaticMode { - mode: StaticMode::MustBeBlocked, - }])], + static_abilities: vec![StaticDefinition::new(StaticMode::MustBeBlocked { + by: None, + }) + .affected(affected) + .modifications(vec![ContinuousModification::AddStaticMode { + mode: StaticMode::MustBeBlocked { by: None }, + }])], duration: Some(Duration::UntilEndOfTurn), target: application.target, }, diff --git a/crates/engine/src/parser/oracle_static/dispatch.rs b/crates/engine/src/parser/oracle_static/dispatch.rs index 1d03b5ad87..ee0688bd60 100644 --- a/crates/engine/src/parser/oracle_static/dispatch.rs +++ b/crates/engine/src/parser/oracle_static/dispatch.rs @@ -2676,13 +2676,35 @@ pub(crate) fn parse_static_line_inner( } } - // --- "must be blocked if able" (CR 509.1b) --- + // --- "must be blocked [by ] if able" (CR 509.1c) --- if nom_primitives::scan_contains(tp.lower, "must be blocked") { - return Some( - StaticDefinition::new(StaticMode::MustBeBlocked) - .affected(TargetFilter::SelfRef) - .description(text.to_string()), - ); + // CR 509.1c: classify the OPTIONAL "by " conjunct so a present + // quality is never silently weakened to the bare "any blocker" (None) + // requirement. Mirrors the attached-grant paths (grammar.rs / shared.rs) + // which distinguish the same three cases via the shared conjunct helper: + // * Recognized quality → typed `MustBeBlocked { by: Some(filter) }`. + // * Unrecognized quality → leave the line Unimplemented (`return None`); + // emitting `by: None` here would force a block by ANY creature and + // drop the quality restriction. Falling through surfaces the gap to + // coverage instead of weakening the requirement. + // * No quality (bare "must be blocked if able") → `by: None`. + match extract_must_be_blocked_by_conjunct(tp.lower) { + Some(MustBeBlockedByConjunct::Recognized(filter)) => { + return Some( + StaticDefinition::new(StaticMode::MustBeBlocked { by: Some(filter) }) + .affected(TargetFilter::SelfRef) + .description(text.to_string()), + ); + } + Some(MustBeBlockedByConjunct::Unrecognized(_)) => return None, + None => { + return Some( + StaticDefinition::new(StaticMode::MustBeBlocked { by: None }) + .affected(TargetFilter::SelfRef) + .description(text.to_string()), + ); + } + } } // --- "can't gain life" (CR 119.7) --- diff --git a/crates/engine/src/parser/oracle_static/evasion.rs b/crates/engine/src/parser/oracle_static/evasion.rs index b9205a5205..f12fdb2a7e 100644 --- a/crates/engine/src/parser/oracle_static/evasion.rs +++ b/crates/engine/src/parser/oracle_static/evasion.rs @@ -619,7 +619,7 @@ pub(crate) fn try_split_and_must_attack_block(text: &str) -> Option("must be blocked each combat if able"), tag("must be blocked if able"), diff --git a/crates/engine/src/parser/oracle_static/grammar.rs b/crates/engine/src/parser/oracle_static/grammar.rs index 2cdf67208b..73f14ece39 100644 --- a/crates/engine/src/parser/oracle_static/grammar.rs +++ b/crates/engine/src/parser/oracle_static/grammar.rs @@ -54,9 +54,11 @@ pub(crate) fn lower_rule_static( RuleStaticPredicate::MustBlock => StaticDefinition::new(StaticMode::MustBlock) .affected(affected) .description(description.to_string()), - RuleStaticPredicate::MustBeBlocked => StaticDefinition::new(StaticMode::MustBeBlocked) - .affected(affected) - .description(description.to_string()), + RuleStaticPredicate::MustBeBlocked => { + StaticDefinition::new(StaticMode::MustBeBlocked { by: None }) + .affected(affected) + .description(description.to_string()) + } RuleStaticPredicate::Goaded => StaticDefinition::new(StaticMode::Goaded) .affected(affected) .description(description.to_string()), @@ -462,46 +464,119 @@ pub(crate) fn parse_attached_condition_run(input: &str) -> OracleResult<'_, Stat /// `StaticMode`). Simple lines return a length-1 vec; unparsed lines an empty /// vec. /// -/// CR 509.1c: Recognize a "must be blocked by if able" lure conjunct. +/// CR 509.1c: Capture the inner "" of a filtered "must be blocked by +/// if able" lure conjunct. /// -/// The BARE form ("must be blocked if able" → `StaticMode::MustBeBlocked`) is -/// already modeled by `try_split_and_must_attack_block`. The FILTERED form -/// ("must be blocked by a Dalek if able", "must be blocked by an Eldrazi if -/// able") requires the typed `MustBeBlocked { by: }` requirement that -/// has not yet been parameterized (/add-engine-variant Stage-2 -/// REFUSE_WITH_REFACTOR, ~80 sites). This combinator detects ONLY the filtered -/// form — the leading `tag("by ")` after "must be blocked " excludes the bare -/// form — so it can be surfaced as an `Effect::Unimplemented` residual rather -/// than silently dropped. -pub(crate) fn parse_must_be_blocked_by_filter_lure(input: &str) -> OracleResult<'_, &str> { - recognize(( - tag("must be blocked by "), - take_until(" if able"), - tag(" if able"), - )) - .parse(input) +/// The BARE form ("must be blocked if able" → `StaticMode::MustBeBlocked { by: +/// None }`) is modeled by `try_split_and_must_attack_block` / +/// `RuleStaticPredicate::MustBeBlocked`. The FILTERED form ("must be blocked by +/// a Dalek if able", "must be blocked by an Eldrazi if able") lowers to the +/// parameterized `StaticMode::MustBeBlocked { by: Some(filter) }`. This +/// combinator captures ONLY the filtered form — the `tag("must be blocked by ")` +/// requires the "by " that the bare form lacks — and returns the inner quality +/// span; the caller parses it into a `TargetFilter`. The successful combinator +/// parse IS the detector (no `contains`/`find` dispatch). +pub(crate) fn parse_must_be_blocked_by_quality(input: &str) -> OracleResult<'_, &str> { + let (rest, _) = tag("must be blocked by ").parse(input)?; + let (after, inner) = take_until(" if able").parse(rest)?; + let (after, _) = tag(" if able").parse(after)?; + Ok((after, inner)) +} + +/// CR 509.1c + CR 105.4: Lower a captured "" span (e.g. +/// "a Dalek", "an Eldrazi", "a creature of the chosen color") to the blocker +/// `TargetFilter`. Composes the SAME quality combinators `CantBeBlockedBy` uses +/// (`parse_chosen_qualifier_subject`, then `parse_type_phrase`). Returns `None` +/// when the quality fails to constrain the blocker at all — either +/// `TargetFilter::Any` or the empty `Typed` filter `parse_type_phrase` yields for +/// an UNRECOGNIZED noun — so an unparseable requirement is never silently +/// weakened to "any blocker satisfies". +fn must_be_blocked_quality_to_filter(quality: &str) -> Option { + // Operate on the lowercase quality (mirrors the `CantBeBlockedBy` path, whose + // `filter_text` is a slice of the already-lowercased predicate). `TextPair` + // requires `lower` to be the lowercase of `original`, so pair them honestly + // even when the caller passes mixed-case input (e.g. a direct unit test). + let quality_lower = quality.to_lowercase(); + let quality_tp = TextPair::new(quality, &quality_lower); + let filter = parse_chosen_qualifier_subject(&quality_tp).unwrap_or_else(|| { + let (f, _) = parse_type_phrase(&quality_lower); + f + }); + filter_constrains_blocker(&filter).then_some(filter) } -/// Scan the lowercase predicate for a filtered "must be blocked by … if able" -/// lure conjunct at any word boundary and, when present, return the matched -/// conjunct span (from "must" through "if able"). The successful combinator parse -/// IS the detector — `scan_at_word_boundaries` tries the combinator at each word -/// start, so there is no `contains`/`find` dispatch. Returns `None` when only the -/// bare (already-modeled) form or no lure is present. -fn extract_must_be_blocked_by_filter_lure(predicate: &str) -> Option { +/// CR 509.1c: Does `filter` actually narrow the set of legal blockers? An +/// unconstrained filter — `TargetFilter::Any`, or an empty `Typed` carrying no +/// type, property, or controller constraint (what `parse_type_phrase` returns for +/// an unrecognized noun like "a splorf") — matches every blocker and therefore +/// expresses no quality requirement. Lowering such a filter into a +/// `MustBeBlocked { by }` would silently degrade "must be blocked by " to +/// "must be blocked by anything"; rejecting it lets callers surface the gap. +fn filter_constrains_blocker(filter: &TargetFilter) -> bool { + match filter { + TargetFilter::Any => false, + TargetFilter::Typed(typed) => { + !typed.type_filters.is_empty() + || !typed.properties.is_empty() + || typed.controller.is_some() + } + _ => true, + } +} + +/// CR 509.1c: Parse a filtered "must be blocked by if able" span +/// anchored at the start of `input` → the blocker `TargetFilter`. Used by the +/// conditional attached-grant path (Ace's Baseball Bat) where the residual +/// conjunct is already isolated. Returns `None` for the bare form or an +/// unrecognized quality. +pub(crate) fn parse_must_be_blocked_by_filter(input: &str) -> Option { + let (_, quality) = parse_must_be_blocked_by_quality(input).ok()?; + must_be_blocked_quality_to_filter(quality) +} + +/// CR 509.1c: Classification of a scanned "must be blocked by if able" +/// conjunct. Distinguishes an ABSENT conjunct (no `Some`) from a PRESENT one, +/// and within present, whether the quality is recognized vs. unrecognized — so +/// the un-gated attached-grant path can surface an `Unimplemented` residual for +/// an unrecognized quality instead of silently dropping the block requirement. +pub(crate) enum MustBeBlockedByConjunct { + /// The quality lowered to a recognized blocker `TargetFilter`. + Recognized(TargetFilter), + /// The conjunct is present but its quality is unrecognized (would weaken to + /// `TargetFilter::Any`). Carries the reconstructed conjunct text so the + /// requirement can be surfaced as an `Unimplemented` residual diagnostic — + /// never silently dropped. + Unrecognized(String), +} + +/// Scan the predicate for a filtered "must be blocked by if able" +/// conjunct at any word boundary and classify it. The successful combinator +/// parse IS the detector — `scan_at_word_boundaries` tries +/// `parse_must_be_blocked_by_quality` at each word start, so there is no +/// `contains`/`find` dispatch. Returns `None` when only the bare form or no lure +/// is present. +pub(crate) fn extract_must_be_blocked_by_conjunct( + predicate: &str, +) -> Option { let lower = predicate.to_lowercase(); - nom_primitives::scan_at_word_boundaries(&lower, parse_must_be_blocked_by_filter_lure) - .map(|span| span.trim().to_string()) -} - -/// CR 509.1c: Build an `Effect::Unimplemented` residual static for an unmodeled -/// effect-conjunct inside an attached-subject grant (the filtered "must be -/// blocked by … if able" lure). The residual rides in a `GrantAbility` -/// modification so coverage flags the card (`is_static_supported`) and the -/// swallow check defers (`any_ability_has_unimplemented`) — the single honest -/// signal that the conjunct is a known gap. See -/// `try_parse_inverted_attached_combat_grant` for the full deferral rationale. -pub(crate) fn unimplemented_conjunct_residual( + let quality = + nom_primitives::scan_at_word_boundaries(&lower, parse_must_be_blocked_by_quality)?; + Some(match must_be_blocked_quality_to_filter(quality) { + Some(filter) => MustBeBlockedByConjunct::Recognized(filter), + None => { + MustBeBlockedByConjunct::Unrecognized(format!("must be blocked by {quality} if able")) + } + }) +} + +/// CR 509.1c: Build the sibling `Effect::Unimplemented` residual for an attached +/// grant conjunct the typed static modes can't model. Carried inside a +/// `GrantAbility` continuous modification so coverage flags the gap (stable +/// category key `"attached_grant_unmodeled_conjunct"`) and the swallow check +/// defers, rather than silently dropping the requirement. Shared by the gated +/// (`try_parse_inverted_attached_combat_grant`) and un-gated attached-grant +/// paths so both surface unrecognized conjuncts identically. +pub(crate) fn attached_grant_unmodeled_conjunct_residual( affected: TargetFilter, residual_text: &str, ) -> StaticDefinition { @@ -793,15 +868,29 @@ pub(crate) fn parse_enchanted_equipped_predicate( // CR 509.1c: " and must be blocked by if able" // (Slayer's Cleaver: "Equipped creature gets +3/+1 and must be blocked // by an Eldrazi if able."). `parse_continuous_modifications` models the - // P/T/keyword grant but silently drops the filtered lure conjunct (the - // bare "must be blocked if able" form is handled by - // `try_split_and_must_attack_block`; the typed by-filter requirement is - // the deferred /add-engine-variant Stage-2 work). Surface the dropped - // conjunct as an `Effect::Unimplemented` residual so it is a visible - // coverage gap, not a silent drop, even when the predicate has no - // continuous grant sibling. - if let Some(residual_text) = extract_must_be_blocked_by_filter_lure(predicate) { - defs.push(unimplemented_conjunct_residual(affected, &residual_text)); + // P/T/keyword grant; this branch models the filtered blocking + // requirement as the typed `MustBeBlocked { by: Some(filter) }` static + // (unconditional — this non-conditional path has no "as long as" gate). + match extract_must_be_blocked_by_conjunct(predicate) { + Some(MustBeBlockedByConjunct::Recognized(filter)) => { + defs.push( + StaticDefinition::new(StaticMode::MustBeBlocked { by: Some(filter) }) + .affected(affected.clone()) + .description(description.to_string()), + ); + } + // CR 509.1c: the lure conjunct is present but its quality is + // unrecognized (would weaken to `TargetFilter::Any`). Surface an + // `Unimplemented` residual so coverage flags the gap — mirroring the + // gated path (`try_parse_inverted_attached_combat_grant`) — instead + // of silently dropping the blocking requirement. + Some(MustBeBlockedByConjunct::Unrecognized(residual)) => { + defs.push(attached_grant_unmodeled_conjunct_residual( + affected.clone(), + &residual, + )); + } + None => {} } defs } diff --git a/crates/engine/src/parser/oracle_static/shared.rs b/crates/engine/src/parser/oracle_static/shared.rs index ccf0a2e930..c82ce9fca6 100644 --- a/crates/engine/src/parser/oracle_static/shared.rs +++ b/crates/engine/src/parser/oracle_static/shared.rs @@ -371,21 +371,19 @@ pub(crate) fn try_parse_inverted_attached_subject_grant( /// Equipment is never an attacker, so the static never fires, and the keyword /// would land on the Equipment rather than the host. /// -/// Returns a `Vec` so that any conjunct of the effect predicate which -/// `push_grant_clause_modifications` cannot model (e.g. "must be blocked by a -/// Dalek if able") is surfaced as a sibling `Effect::Unimplemented` residual, -/// rather than being silently dropped. The residual makes coverage mark the card -/// partially unsupported (`is_static_supported`) and the swallow check defer -/// (`any_ability_has_unimplemented`) — an honest signal independent of the -/// whole-card `"condition":{` suppression that the supported static's gate would -/// otherwise trip in `detect_condition_if`. -/// -/// DEFER: typed "must be blocked by if able" requires parameterizing -/// the `MustBeBlocked`/`MustBeBlockedByAll` requirement family with a blocker -/// filter — /add-engine-variant Stage-2 REFUSE_WITH_REFACTOR (~80 sites). Until -/// then the dropped conjunct rides as an `Effect::Unimplemented` residual. An -/// `Unrecognized`-condition companion is NOT used: it would suppress -/// `detect_condition_if` (cond_markers include `"condition":{`) AND be +/// Returns a `Vec` so that each conjunct of the effect predicate is modeled +/// independently: the P/T + keyword grants merge into one gated `Continuous` +/// static, recognized combat requirements ("must be blocked if able", "is +/// goaded") become gated rule-statics, and the FILTERED "must be blocked by a +/// Dalek if able" conjunct lowers to the typed `MustBeBlocked { by: Some(filter) +/// }` requirement gated on the same combat condition (CR 509.1c). Only a +/// conjunct that none of these recognize is surfaced as a sibling +/// `Effect::Unimplemented` residual rather than being silently dropped — an +/// honest coverage signal (`is_static_supported` / `any_ability_has_unimplemented`) +/// independent of the whole-card `"condition":{` suppression that the supported +/// static's gate would otherwise trip in `detect_condition_if`. An +/// `Unrecognized`-condition companion is NOT used for residuals: it would +/// suppress `detect_condition_if` (cond_markers include `"condition":{`) AND be /// runtime-active (`layers.rs` evaluates `Unrecognized => true`). CR 509.1c. pub(crate) fn try_parse_inverted_attached_combat_grant( split: &InvertedSplit, @@ -469,8 +467,8 @@ pub(crate) fn try_parse_inverted_attached_combat_grant( // if able", "attacks each combat if able", "is goaded"). Recover it via the // rule-static predicate combinator and emit a sibling rule-static gated on the // same combat condition — modeled, not an `Unimplemented` residual. (The - // FILTERED "must be blocked by a Dalek if able" form is NOT recognized by the - // combinator, so it correctly falls through to the residual below.) + // FILTERED "must be blocked by a Dalek if able" form is handled by the typed + // `MustBeBlocked { by }` branch below, not this bare-form combinator.) let residual_lower = residual_text.to_lowercase(); if let Ok((rest, predicate)) = all_consuming(parse_rule_static_predicate_nom).parse(residual_lower.trim()) @@ -482,24 +480,29 @@ pub(crate) fn try_parse_inverted_attached_combat_grant( continue; } + // CR 509.1c: the FILTERED "must be blocked by if able" conjunct + // (Ace's Baseball Bat: "must be blocked by a Dalek if able") lowers to the + // typed `MustBeBlocked { by: Some(filter) }` requirement, gated on the same + // combat condition as the grant (so it inherits the "as long as ~ is + // attacking" gate). Modeled, not an `Unimplemented` residual. + if let Some(filter) = parse_must_be_blocked_by_filter(&residual_lower) { + defs.push( + StaticDefinition::new(StaticMode::MustBeBlocked { by: Some(filter) }) + .affected(affected.clone()) + .condition(gate.clone()) + .description(residual_text.clone()), + ); + continue; + } + // CR 509.1c: surface the still-unmodeled conjunct as an `Effect::Unimplemented` // residual carried in a `GrantAbility` modification so coverage flags it and // the swallow check defers (see fn-level note). The stable category key // groups the gap in coverage; the raw conjunct text is the diagnostic. - defs.push( - StaticDefinition::continuous() - .affected(affected.clone()) - .modifications(vec![ContinuousModification::GrantAbility { - definition: Box::new(AbilityDefinition::new( - AbilityKind::Spell, - crate::types::ability::Effect::unimplemented( - "attached_grant_unmodeled_conjunct", - residual_text.clone(), - ), - )), - }]) - .description(residual_text), - ); + defs.push(attached_grant_unmodeled_conjunct_residual( + affected.clone(), + &residual_text, + )); } defs diff --git a/crates/engine/src/parser/oracle_static/tests.rs b/crates/engine/src/parser/oracle_static/tests.rs index de0d3b79b8..ed297ef430 100644 --- a/crates/engine/src/parser/oracle_static/tests.rs +++ b/crates/engine/src/parser/oracle_static/tests.rs @@ -5657,7 +5657,7 @@ fn static_pump_and_must_be_blocked_if_able_emits_both_defs() { assert!(defs[0] .modifications .contains(&ContinuousModification::AddToughness { value: 3 })); - assert_eq!(defs[1].mode, StaticMode::MustBeBlocked); + assert_eq!(defs[1].mode, StaticMode::MustBeBlocked { by: None }); assert_eq!(defs[1].affected, defs[0].affected); } @@ -6202,7 +6202,7 @@ fn static_pump_must_be_blocked_and_goaded_emits_all_defs() { assert!(defs[0] .modifications .contains(&ContinuousModification::AddToughness { value: 3 })); - assert_eq!(defs[1].mode, StaticMode::MustBeBlocked); + assert_eq!(defs[1].mode, StaticMode::MustBeBlocked { by: None }); assert_eq!(defs[2].mode, StaticMode::Goaded); assert_eq!(defs[1].affected, defs[0].affected); assert_eq!(defs[2].affected, defs[0].affected); @@ -6230,18 +6230,29 @@ fn is_unimplemented_residual(def: &StaticDefinition, needle: &str) -> bool { }) } -/// CR 508.1a + CR 611.3a + CR 613.1f: "As long as equipped creature is +/// CR 508.1a + CR 611.3a + CR 509.1c: "As long as equipped creature is /// attacking, it has first strike and must be blocked by a Dalek if able." -/// (Ace's Baseball Bat). The first-strike grant must land on the EquippedBy -/// creature gated on the recipient being attacking — NOT on SelfRef with -/// SourceIsAttacking — and the unmodeled "must be blocked by a Dalek if able" -/// lure must surface as an `Effect::Unimplemented` residual, not be dropped. +/// (Ace's Baseball Bat). The first-strike grant lands on the EquippedBy creature +/// gated on the recipient being attacking — NOT on SelfRef with +/// SourceIsAttacking — and the "must be blocked by a Dalek if able" lure lowers +/// to the typed `MustBeBlocked { by: Some(Dalek) }` requirement gated on the +/// same combat condition (no Unimplemented residual). #[test] fn static_as_long_as_equipped_creature_is_attacking_grants_first_strike_to_host() { let defs = parse_static_line_multi( "As long as equipped creature is attacking, it has first strike and must be blocked by a Dalek if able.", ); - assert_eq!(defs.len(), 2, "expected supported + residual, got {defs:?}"); + assert_eq!( + defs.len(), + 2, + "expected first-strike grant + typed MustBeBlocked, got {defs:?}" + ); + + let attacking_gate = StaticCondition::RecipientMatchesFilter { + filter: TargetFilter::Typed( + TypedFilter::creature().properties(vec![FilterProp::Attacking { defender: None }]), + ), + }; // Supported static: first strike on the host, gated on the host attacking. assert_eq!(defs[0].mode, StaticMode::Continuous); @@ -6251,27 +6262,24 @@ fn static_as_long_as_equipped_creature_is_attacking_grants_first_strike_to_host( .contains(&ContinuousModification::AddKeyword { keyword: Keyword::FirstStrike, })); - assert_eq!( - defs[0].condition, - Some(StaticCondition::RecipientMatchesFilter { - filter: TargetFilter::Typed( - TypedFilter::creature().properties(vec![FilterProp::Attacking { defender: None }]) - ), - }), - ); + assert_eq!(defs[0].condition, Some(attacking_gate.clone())); // Must NOT regress to the wrong subject/condition. assert_ne!(defs[0].affected, Some(TargetFilter::SelfRef)); assert_ne!(defs[0].condition, Some(StaticCondition::SourceIsAttacking)); - // Residual static surfaces the dropped lure. + // CR 509.1c: the lure is the typed MustBeBlocked { by: Some(Dalek) }, gated + // on the same attacking condition, affecting the equipped creature. + let dalek = crate::parser::oracle_target::parse_type_phrase("a dalek").0; + assert_ne!(dalek, TargetFilter::Any, "Dalek subtype must be recognized"); + assert_eq!(defs[1].mode, StaticMode::MustBeBlocked { by: Some(dalek) }); assert_eq!(defs[1].affected, Some(equipped_creature_filter())); + assert_eq!(defs[1].condition, Some(attacking_gate)); + // The lure is modeled, NOT an Unimplemented residual. assert!( - is_unimplemented_residual(&defs[1], "must be blocked by a"), - "lure must surface as an Unimplemented residual, got {:?}", + !is_unimplemented_residual(&defs[1], "must be blocked by a"), + "lure must be modeled, got Unimplemented residual {:?}", defs[1] ); - // The lure must NOT be modeled as a (bare) MustBeBlocked mode. - assert_ne!(defs[1].mode, StaticMode::MustBeBlocked); } /// CR 611.3a: the Aura analog binds the gate to the EnchantedBy host. @@ -6343,7 +6351,7 @@ fn static_as_long_as_attacking_bare_must_be_blocked_models_requirement() { .contains(&ContinuousModification::AddKeyword { keyword: Keyword::FirstStrike, })); - assert_eq!(defs[1].mode, StaticMode::MustBeBlocked); + assert_eq!(defs[1].mode, StaticMode::MustBeBlocked { by: None }); assert_eq!(defs[1].affected, Some(equipped_creature_filter())); // The requirement is also gated on the combat condition (CR 611.3a). assert_eq!( @@ -6369,7 +6377,7 @@ fn static_as_long_as_attacking_pure_must_be_blocked_models_requirement() { "As long as equipped creature is attacking, it must be blocked if able.", ); assert_eq!(defs.len(), 1, "pure requirement, got {defs:?}"); - assert_eq!(defs[0].mode, StaticMode::MustBeBlocked); + assert_eq!(defs[0].mode, StaticMode::MustBeBlocked { by: None }); assert_eq!(defs[0].affected, Some(equipped_creature_filter())); assert_eq!( defs[0].condition, @@ -6407,14 +6415,19 @@ fn static_as_long_as_attacking_gets_and_has_keyword_has_no_false_residual() { /// CR 509.1c: the un-gated direct attached-subject grant lure (Slayer's Cleaver: /// "Equipped creature gets +3/+1 and must be blocked by an Eldrazi if able.") -/// surfaces the filtered lure as an Unimplemented residual. The first def carries -/// the P/T grant and no condition; the residual carries no condition either. +/// models the filtered lure as the typed `MustBeBlocked { by: Some(Eldrazi) }`. +/// The first def carries the P/T grant and no condition; the requirement carries +/// no condition either (this is the unconditional, non-"as long as" form). #[test] -fn slayers_cleaver_lure_conjunct_surfaces_as_unimplemented_residual() { +fn slayers_cleaver_lure_conjunct_models_typed_must_be_blocked() { let defs = parse_static_line_multi( "Equipped creature gets +3/+1 and must be blocked by an Eldrazi if able.", ); - assert_eq!(defs.len(), 2, "P/T grant + residual, got {defs:?}"); + assert_eq!( + defs.len(), + 2, + "P/T grant + typed MustBeBlocked, got {defs:?}" + ); assert_eq!(defs[0].mode, StaticMode::Continuous); assert!(defs[0] .modifications @@ -6423,37 +6436,146 @@ fn slayers_cleaver_lure_conjunct_surfaces_as_unimplemented_residual() { .modifications .contains(&ContinuousModification::AddToughness { value: 1 })); assert_eq!(defs[0].condition, None); - assert!( - is_unimplemented_residual(&defs[1], "must be blocked by an"), - "lure must surface as an Unimplemented residual, got {:?}", - defs[1] + + let eldrazi = crate::parser::oracle_target::parse_type_phrase("an eldrazi").0; + assert_ne!( + eldrazi, + TargetFilter::Any, + "Eldrazi subtype must be recognized" + ); + assert_eq!( + defs[1].mode, + StaticMode::MustBeBlocked { by: Some(eldrazi) } ); assert_eq!(defs[1].condition, None); assert_eq!(defs[1].affected, Some(equipped_creature_filter())); + assert!( + !is_unimplemented_residual(&defs[1], "must be blocked by an"), + "lure must be modeled, got Unimplemented residual {:?}", + defs[1] + ); } /// CR 509.1c: a direct attached-subject filtered lure with no continuous grant -/// sibling is still an explicit unsupported residual, not a silent drop. +/// sibling is fully modeled as the typed `MustBeBlocked { by: Some(Eldrazi) }`. #[test] -fn attached_subject_pure_filtered_lure_surfaces_as_unimplemented_residual() { +fn attached_subject_pure_filtered_lure_models_typed_must_be_blocked() { let defs = parse_static_line_multi("Equipped creature must be blocked by an Eldrazi if able."); - assert_eq!(defs.len(), 1, "pure filtered lure residual, got {defs:?}"); + assert_eq!( + defs.len(), + 1, + "pure filtered lure → one static, got {defs:?}" + ); + let eldrazi = crate::parser::oracle_target::parse_type_phrase("an eldrazi").0; + assert_eq!( + defs[0].mode, + StaticMode::MustBeBlocked { by: Some(eldrazi) } + ); + assert_eq!(defs[0].affected, Some(equipped_creature_filter())); assert!( - is_unimplemented_residual(&defs[0], "must be blocked by an"), - "filtered lure must surface as an Unimplemented residual, got {:?}", + !is_unimplemented_residual(&defs[0], "must be blocked by an"), + "filtered lure must be modeled, got Unimplemented residual {:?}", defs[0] ); - assert_eq!(defs[0].affected, Some(equipped_creature_filter())); } -/// Building-block: the filtered-lure detector recognizes ONLY the by-filter form, -/// never the bare form (which is a modeled requirement). +/// Building-block: `parse_must_be_blocked_by_quality` captures ONLY the filtered +/// "must be blocked by if able" form (returning the inner quality +/// span); the bare "must be blocked if able" form has no "by " and must NOT +/// match. `parse_must_be_blocked_by_filter` lowers the captured quality into the +/// blocker `TargetFilter` for the two known subtypes. +#[test] +fn parse_must_be_blocked_by_filter_building_block() { + // Detector: filtered form matches, bare form does not. + assert!(parse_must_be_blocked_by_quality("must be blocked by a Dalek if able").is_ok()); + assert!(parse_must_be_blocked_by_quality("must be blocked by an Eldrazi if able").is_ok()); + assert!(parse_must_be_blocked_by_quality("must be blocked if able").is_err()); + + // Filter lowering for both known subtypes (case-insensitive canonicalization). + assert_eq!( + parse_must_be_blocked_by_filter("must be blocked by a Dalek if able"), + Some(crate::parser::oracle_target::parse_type_phrase("a dalek").0), + ); + assert_eq!( + parse_must_be_blocked_by_filter("must be blocked by an Eldrazi if able"), + Some(crate::parser::oracle_target::parse_type_phrase("an eldrazi").0), + ); + // Bare form yields no filter. + assert_eq!( + parse_must_be_blocked_by_filter("must be blocked if able"), + None, + ); +} + +/// Building-block (CR 509.1c): `extract_must_be_blocked_by_conjunct` distinguishes +/// an ABSENT filtered lure (`None`) from a PRESENT one, classifying whether its +/// quality is recognized vs. unrecognized. The Unrecognized arm carries the +/// reconstructed conjunct so the un-gated attached-grant path surfaces a residual +/// diagnostic instead of silently dropping the requirement. #[test] -fn must_be_blocked_by_filter_lure_detector_excludes_bare_form() { - assert!(parse_must_be_blocked_by_filter_lure("must be blocked by a Dalek if able").is_ok()); - assert!(parse_must_be_blocked_by_filter_lure("must be blocked by an Eldrazi if able").is_ok()); - // Bare form has no "by " — must NOT match. - assert!(parse_must_be_blocked_by_filter_lure("must be blocked if able").is_err()); +fn extract_must_be_blocked_by_conjunct_classifies_quality() { + // Absent / bare form → None (no filtered lure present). + assert!(extract_must_be_blocked_by_conjunct("equipped creature gets +1/+1").is_none()); + assert!(extract_must_be_blocked_by_conjunct("must be blocked if able").is_none()); + + // Recognized quality → Recognized(filter). + assert!(matches!( + extract_must_be_blocked_by_conjunct("must be blocked by an Eldrazi if able"), + Some(MustBeBlockedByConjunct::Recognized(_)), + )); + + // Guard: the fixture quality is genuinely unrecognized — it constrains no + // blocker, so the lowering helper rejects it (returns no filter). NOTE: + // `parse_type_phrase` yields an empty `Typed` (not `TargetFilter::Any`) here. + assert!(parse_must_be_blocked_by_filter("must be blocked by a splorf if able").is_none()); + // Unrecognized quality → Unrecognized(diagnostic), NOT collapsed to None. + let conjunct = extract_must_be_blocked_by_conjunct("must be blocked by a Splorf if able"); + assert!( + matches!( + &conjunct, + Some(MustBeBlockedByConjunct::Unrecognized(text)) + if text.contains("must be blocked by a splorf if able") + ), + "expected an Unrecognized diagnostic conjunct, never None", + ); +} + +/// CR 509.1c (regression): the un-gated direct attached-subject path must NOT +/// silently drop a filtered "must be blocked by if able" lure whose +/// quality is UNRECOGNIZED. The P/T grant is modeled and the unrecognized lure +/// surfaces as a sibling `Unimplemented` residual (coverage-visible), mirroring +/// the gated path — the requirement never vanishes, and it is never weakened to a +/// typed `MustBeBlocked { by: Some() }`. +#[test] +fn unrecognized_lure_conjunct_surfaces_unimplemented_residual() { + // Guard: the fixture quality is genuinely unrecognized — it constrains no + // blocker, so the lowering helper rejects it (returns no filter). + assert!(parse_must_be_blocked_by_filter("must be blocked by a splorf if able").is_none()); + let defs = parse_static_line_multi( + "Equipped creature gets +3/+1 and must be blocked by a Splorf if able.", + ); + // The P/T grant is still modeled. + assert!( + defs.iter().any(|d| d + .modifications + .contains(&ContinuousModification::AddPower { value: 3 })), + "P/T grant must be modeled, got {defs:?}" + ); + // The lure is surfaced as an Unimplemented residual, not dropped. + assert!( + defs.iter() + .any(|d| is_unimplemented_residual(d, "must be blocked by a")), + "unrecognized lure must surface an Unimplemented residual, got {defs:?}" + ); + // It must NOT be modeled as a typed MustBeBlocked — an unrecognized quality + // would carry only an unconstrained filter ("any blocker"), so no typed + // MustBeBlocked { by: Some(..) } may appear. + assert!( + !defs + .iter() + .any(|d| matches!(&d.mode, StaticMode::MustBeBlocked { by: Some(_) })), + "unrecognized quality must not be modeled as typed MustBeBlocked, got {defs:?}" + ); } /// Building-block (Step 3 backstop): `parse_static_condition` for combat state @@ -7253,10 +7375,31 @@ fn parse_compound_static_kaito_animation() { fn static_must_be_blocked_if_able() { // CR 509.1b: "must be blocked if able" let def = parse_static_line("Darksteel Myr must be blocked if able.").unwrap(); - assert_eq!(def.mode, StaticMode::MustBeBlocked); + assert_eq!(def.mode, StaticMode::MustBeBlocked { by: None }); assert_eq!(def.affected, Some(TargetFilter::SelfRef)); } +/// CR 509.1c (regression): the STANDALONE self-ref "must be blocked [by +/// ] if able" dispatch must classify the optional blocker quality +/// exactly like the attached-grant paths — the requirement is never silently +/// weakened to the bare "any blocker" (`by: None`) form when a quality is +/// present but unrecognized. +#[test] +fn static_must_be_blocked_standalone_quality_classification() { + // Recognized quality → typed `MustBeBlocked { by: Some(filter) }`. + let recognized = + parse_static_line("This creature must be blocked by an Eldrazi if able.").unwrap(); + assert!(matches!( + recognized.mode, + StaticMode::MustBeBlocked { by: Some(_) } + )); + assert_eq!(recognized.affected, Some(TargetFilter::SelfRef)); + + // Unrecognized quality → line left Unimplemented (`None`), NOT weakened to a + // bare `MustBeBlocked { by: None }` that would force a block by ANY creature. + assert!(parse_static_line("This creature must be blocked by a Splorf if able.").is_none()); +} + #[test] fn static_legend_rule_global_exemption() { // CR 704.5j: Mirror Gallery — "The legend rule doesn't apply." (global). diff --git a/crates/engine/src/parser/swallow_check.rs b/crates/engine/src/parser/swallow_check.rs index 94d96a8fdb..806b6c2b35 100644 --- a/crates/engine/src/parser/swallow_check.rs +++ b/crates/engine/src/parser/swallow_check.rs @@ -2055,7 +2055,14 @@ fn detect_condition_if( // `ForceBlock`/`ForceAttack` effects, not conditional gates. "\"mode\":\"MustAttack\"", "\"mode\":\"MustBlock\"", - "\"mode\":\"MustBeBlocked\"", + // `MustBeBlocked` is data-carrying (`MustBeBlocked { by: Option<_> }`). + // The `by` field has no `skip_serializing_if`, so serde emits the object + // form `{"MustBeBlocked":{"by":...}}` for BOTH shapes: `{"by":null}` for + // the bare "must be blocked if able" rider (`by: None`) and + // `{"by":{...}}` for typed riders (Ace's Baseball Bat, Slayer's Cleaver). + // Match the quoted variant key `"MustBeBlocked"`, which is common to both + // serializations, so both suppress the "if able" Condition_If marker. + "\"MustBeBlocked\"", "\"type\":\"ForceBlock\"", "\"type\":\"ForceAttack\"", // CR 305.9: "as ~ enters, you may pay X. If you don't, it enters @@ -3753,6 +3760,54 @@ mod tests { assert!(!has_swallowed_detector(&parsed, "Condition_If")); } + /// CR 509.1c: "must be blocked if able" riders are represented by + /// `StaticMode::MustBeBlocked { by }`, so the trailing "if able" must not be + /// reported as a swallowed `Condition_If`. The `by` field has no + /// `skip_serializing_if`, so serde emits the object form for BOTH shapes: + /// `{"MustBeBlocked":{"by":null}}` for the bare rider (`by: None`, Gaea's + /// Protector) and `{"MustBeBlocked":{"by":{...}}}` for the typed form + /// (`by: Some(filter)`, Slayer's Cleaver). The suppression marker matches the + /// quoted variant key `"MustBeBlocked"` common to both serializations. + #[test] + fn condition_if_accepts_must_be_blocked_rider() { + let bare = parse_named( + "This creature must be blocked if able.", + "Gaea's Protector", + &["Creature"], + ); + assert!( + !has_swallowed_detector(&bare, "Condition_If"), + "bare must-be-blocked static must not report Condition_If: {:?}", + bare.parse_warnings + ); + assert!( + bare.statics + .iter() + .any(|s| matches!(s.mode, StaticMode::MustBeBlocked { by: None })), + "expected bare MustBeBlocked static, got {:?}", + bare.statics + ); + + let typed = parse_named( + "Equipped creature gets +3/+1 and must be blocked by an Eldrazi if able.\nEquip {4}", + "Slayer's Cleaver", + &["Artifact"], + ); + assert!( + !has_swallowed_detector(&typed, "Condition_If"), + "typed must-be-blocked static must not report Condition_If: {:?}", + typed.parse_warnings + ); + assert!( + typed + .statics + .iter() + .any(|s| matches!(s.mode, StaticMode::MustBeBlocked { by: Some(_) })), + "expected typed MustBeBlocked static, got {:?}", + typed.statics + ); + } + #[test] fn condition_if_accepts_tiered_enters_with_counter_static() { let parsed = parse_named( diff --git a/crates/engine/src/types/statics.rs b/crates/engine/src/types/statics.rs index 8349810f10..6745d644c0 100644 --- a/crates/engine/src/types/statics.rs +++ b/crates/engine/src/types/statics.rs @@ -1456,9 +1456,19 @@ pub enum StaticMode { // -- Tier 2: Rule-modification statics -- CantTap, CantUntap, - /// CR 509.1c: This creature must be blocked if able — the defending player - /// must assign at least one legal blocker to it. - MustBeBlocked, + /// CR 509.1c: This creature must be blocked if able. `by == None` ⇒ any + /// blocker satisfies the requirement (the bare "must be blocked if able" + /// form — the defending player must assign at least one legal blocker to + /// it). `by == Some(filter)` ⇒ the requirement is obeyed only by assigning + /// a blocker matching `filter` (Ace's Baseball Bat: "must be blocked by a + /// Dalek if able"; Slayer's Cleaver: "…by an Eldrazi…"). Data-carrying + /// (holds `TargetFilter`) — not registry-keyed (see + /// `coverage::is_data_carrying_static`); enforced by direct pattern-match in + /// combat.rs declare-blockers validation. Positive mirror of + /// `CantBeBlockedBy { filter }`. + MustBeBlocked { + by: Option, + }, /// CR 509.1c: "All creatures able to block this creature do so" (Lure, /// Prized Unicorn, Breaker of Armies, …). Unlike [`MustBeBlocked`], this /// places a block requirement on *every* creature able to block it: each @@ -1786,6 +1796,7 @@ impl Hash for StaticMode { BlockExceptionKind::MinBlockers { min } => min.hash(state), }, StaticMode::CantBeBlockedBy { .. } => {} // TargetFilter does not implement Hash; discriminant only + StaticMode::MustBeBlocked { .. } => {} // optional TargetFilter does not implement Hash; discriminant only StaticMode::BlockRestriction { .. } => {} // TargetFilter does not implement Hash; discriminant only StaticMode::AttachmentRestriction { .. } => {} // TargetFilter does not implement Hash; discriminant only StaticMode::CantBeBlockedByMoreThan { max } => max.hash(state), @@ -1973,7 +1984,7 @@ impl StaticMode { | StaticMode::FlashBack | StaticMode::CantTap | StaticMode::CantUntap - | StaticMode::MustBeBlocked + | StaticMode::MustBeBlocked { .. } | StaticMode::MustBeBlockedByAll | StaticMode::Goaded | StaticMode::CombatAlone { .. } @@ -2295,7 +2306,13 @@ impl fmt::Display for StaticMode { // Tier 2 StaticMode::CantTap => write!(f, "CantTap"), StaticMode::CantUntap => write!(f, "CantUntap"), - StaticMode::MustBeBlocked => write!(f, "MustBeBlocked"), + StaticMode::MustBeBlocked { by: None } => write!(f, "MustBeBlocked"), + // CR 509.1c: TargetFilter has no parseable string form — Debug + // format, one-way (mirrors CantBeBlockedBy). No from_str + // reconstruction; the Some form is parse-time only. + StaticMode::MustBeBlocked { by: Some(filter) } => { + write!(f, "MustBeBlocked:By({filter:?})") + } StaticMode::MustBeBlockedByAll => write!(f, "MustBeBlockedByAll"), StaticMode::Goaded => write!(f, "Goaded"), StaticMode::CombatAlone { @@ -2743,7 +2760,7 @@ impl FromStr for StaticMode { // Tier 2 "CantTap" => StaticMode::CantTap, "CantUntap" => StaticMode::CantUntap, - "MustBeBlocked" => StaticMode::MustBeBlocked, + "MustBeBlocked" => StaticMode::MustBeBlocked { by: None }, "MustBeBlockedByAll" => StaticMode::MustBeBlockedByAll, "Goaded" => StaticMode::Goaded, "CombatAlone(Attack,NeedsCompanion)" => StaticMode::CombatAlone { @@ -3219,7 +3236,7 @@ mod tests { assert_eq!(StaticMode::from_str("Flying").unwrap(), StaticMode::Flying); assert_eq!( StaticMode::from_str("MustBeBlocked").unwrap(), - StaticMode::MustBeBlocked + StaticMode::MustBeBlocked { by: None } ); assert_eq!( StaticMode::from_str("NoMaximumHandSize").unwrap(), @@ -3303,7 +3320,7 @@ mod tests { // Tier 2: rule-mod statics StaticMode::CantTap, StaticMode::CantUntap, - StaticMode::MustBeBlocked, + StaticMode::MustBeBlocked { by: None }, StaticMode::CombatAlone { action: CombatAloneAction::Attack, requirement: CombatAloneRequirement::NeedsCompanion, @@ -3465,7 +3482,7 @@ mod tests { StaticMode::CantBeTargeted, StaticMode::CantBeBlocked, StaticMode::Flying, - StaticMode::MustBeBlocked, + StaticMode::MustBeBlocked { by: None }, StaticMode::GrantsExtraVote, // CR 118.9: data-carrying ManaCost — serde must preserve {0} and {WUBRG}. StaticMode::CastWithAlternativeCost { diff --git a/crates/engine/tests/squirrel_perf_probe.rs b/crates/engine/tests/squirrel_perf_probe.rs index 8c4cb100c6..59a428c36e 100644 --- a/crates/engine/tests/squirrel_perf_probe.rs +++ b/crates/engine/tests/squirrel_perf_probe.rs @@ -184,7 +184,7 @@ fn probe_declare_blockers() { | StaticMode::BlockRestriction { .. } | StaticMode::MustBlock | StaticMode::MustBlockAttacker { .. } - | StaticMode::MustBeBlocked + | StaticMode::MustBeBlocked { .. } | StaticMode::MustBeBlockedByAll | StaticMode::MaxBlockersEachCombat { .. } | StaticMode::ExtraBlockers { .. } diff --git a/crates/mtgish-import/src/convert/static_effect.rs b/crates/mtgish-import/src/convert/static_effect.rs index 32768fa8b0..679c8711e9 100644 --- a/crates/mtgish-import/src/convert/static_effect.rs +++ b/crates/mtgish-import/src/convert/static_effect.rs @@ -308,7 +308,7 @@ pub fn convert_permanent_rule( }, P::MustAttack => StaticMode::MustAttack, P::MustBlock => StaticMode::MustBlock, - P::MustBeBlocked => StaticMode::MustBeBlocked, + P::MustBeBlocked => StaticMode::MustBeBlocked { by: None }, P::CanBlockOnly(filter) if is_creature_with_flying_filter(filter) => { StaticMode::BlockRestriction { filter: engine::types::statics::block_only_creatures_with_flying_filter(), From 6efa7a34ada87b2a6c09c10cd2840b1934f592a6 Mon Sep 17 00:00:00 2001 From: Nicholas Tindle Date: Mon, 29 Jun 2026 00:42:13 -0500 Subject: [PATCH 2/3] test: annotate non-combinator test assertion in must-be-blocked diagnostic check --- crates/engine/src/parser/oracle_static/tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/engine/src/parser/oracle_static/tests.rs b/crates/engine/src/parser/oracle_static/tests.rs index ed297ef430..3b29afb828 100644 --- a/crates/engine/src/parser/oracle_static/tests.rs +++ b/crates/engine/src/parser/oracle_static/tests.rs @@ -6534,6 +6534,7 @@ fn extract_must_be_blocked_by_conjunct_classifies_quality() { matches!( &conjunct, Some(MustBeBlockedByConjunct::Unrecognized(text)) + // allow-noncombinator: test assertion verifying diagnostic text content, not parsing dispatch if text.contains("must be blocked by a splorf if able") ), "expected an Unrecognized diagnostic conjunct, never None", From f534720e8704b18348446610204e79f5815fcf8e Mon Sep 17 00:00:00 2001 From: Nicholas Tindle Date: Mon, 29 Jun 2026 14:28:21 -0500 Subject: [PATCH 3/3] =?UTF-8?q?fix(PR-4591):=20address=20matthewevans=20re?= =?UTF-8?q?view=20=E2=80=94=20spare=20capacity=20+=20mtgish=20import?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves both CHANGES_REQUESTED items from @matthewevans: [MED] combat.rs — filtered MustBeBlocked spare-capacity correctness Previously, has_available_blocker for MustBeBlocked { by: Some(filter) } rejected any creature in assigned_blockers regardless of its ExtraBlockers capacity, mirroring a bug that MustBeBlockedByAll already handled correctly. Fix: switch the early-exit from assigned_blockers.contains(id) (all-attacker set) to blockers_per_attacker.get(&attacker_id).is_some_and(|bl| bl.contains(id)) (this-attacker set), and add attackers_per_blocker.get(id) < extra_block_limit() to allow multi-block-capable creatures to count as available — mirroring the MustBeBlockedByAll path at combat.rs:1496. Test: must_be_blocked_filtered_counts_multi_blocker_spare_capacity (Dalek with ExtraBlockers{count:Some(1)} blocking another attacker must still cover the MustBeBlocked attacker; assigned to both is legal). [MED] static_effect.rs — wire MustBeBlockedByADefender in mtgish converter P::MustBeBlockedByADefender(filter) previously fell through to UnknownVariant. Now maps to StaticMode::MustBeBlocked { by: Some(filter::convert(filter)?) }, matching the engine type this PR introduced. Covers Dalek (Ace's Baseball Bat) and Eldrazi (Slayer's Cleaver) cases in the mtgish corpus. CR 509.1c annotation added. Test: must_be_blocked_by_a_defender_lowers_to_filtered_must_be_blocked verifies Dalek and Eldrazi cases convert to MustBeBlocked{by:Some(_)}. Verification: - cargo fmt --all — clean - cargo clippy -p engine --all-targets — exit 0 - cargo clippy -p mtgish-import --all-targets — exit 0 - cargo test -p engine — 14139 passed / 0 failed - cargo test -p mtgish-import (unit tests) — 1 new test passed - check-parser-combinators.sh — no new forbidden patterns Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_019aQYsGCjiRn71Z4vQDo9QR --- crates/engine/src/game/combat.rs | 122 ++++++++++++++---- .../src/convert/static_effect.rs | 52 +++++++- 2 files changed, 148 insertions(+), 26 deletions(-) diff --git a/crates/engine/src/game/combat.rs b/crates/engine/src/game/combat.rs index 20740bd73d..e0ba51265e 100644 --- a/crates/engine/src/game/combat.rs +++ b/crates/engine/src/game/combat.rs @@ -1399,39 +1399,60 @@ pub fn validate_blockers_for_player( continue; } - // Check if any unassigned defending creature could legally block - // this attacker AND (for the filtered form) match the filter. If - // so, the declaration is illegal because that creature should - // have been assigned. CR 509.1b: a creature that can't legally - // block doesn't make the requirement obey-able. + // Check if any defending creature not yet assigned to THIS + // attacker could legally block it AND (for the filtered form) + // match the filter. If so, the declaration is illegal because + // that creature should have been assigned. CR 509.1b: a + // creature that can't legally block doesn't make the + // requirement obey-able. + // + // CR 509.1c: a creature already blocking another attacker is + // still "able" to block this one if it has spare block + // capacity granted by ExtraBlockers — mirror of the + // MustBeBlockedByAll path at line ~1496 above. let has_available_blocker = state.battlefield.iter().any(|id| { - if assigned_blockers.contains(id) { + // Skip creatures already assigned to this specific attacker + // — they're already counted in the `satisfied` check above. + if blockers_per_attacker + .get(&attacker_id) + .is_some_and(|blockers| blockers.contains(id)) + { return false; } let Some(obj) = state.objects.get(id) else { return false; }; - obj.controller == player - && obj.card_types.core_types.contains(&CoreType::Creature) - && !obj.tapped - && can_block_pair_with_precomputed( + if obj.controller != player + || !obj.card_types.core_types.contains(&CoreType::Creature) + || obj.tapped + { + return false; + } + // A creature blocking other attacker(s) is only "able" to + // also block this one if it has spare block capacity. + // CR 509.1c: a blocker at its per-creature limit cannot + // take on an additional block. + let assigned_count = attackers_per_blocker.get(id).copied().unwrap_or(0); + if assigned_count >= extra_block_limit(state, obj) { + return false; + } + can_block_pair_with_precomputed( + state, + *id, + attacker_id, + &blocker_restriction, + &block_restriction, + &blocker_allowed, + can_block_shadow_exists, + ) && match by { + None => true, + Some(filter) => matches_target_filter( state, *id, - attacker_id, - &blocker_restriction, - &block_restriction, - &blocker_allowed, - can_block_shadow_exists, - ) - && match by { - None => true, - Some(filter) => matches_target_filter( - state, - *id, - filter, - &FilterContext::from_source(state, src_id), - ), - } + filter, + &FilterContext::from_source(state, src_id), + ), + } }); if has_available_blocker { @@ -7128,6 +7149,57 @@ mod tests { assert!(validate_blockers(&state, &[(dalek, attacker), (non_dalek, attacker)]).is_ok()); } + /// CR 509.1c: a Dalek already blocking another attacker but with spare + /// block capacity (ExtraBlockers) is still "able" to satisfy the filtered + /// `MustBeBlocked { by: Some(Dalek) }` requirement. Not assigning it is + /// illegal; assigning it to both attackers is legal. + #[test] + fn must_be_blocked_filtered_counts_multi_blocker_spare_capacity() { + let mut state = setup(); + let ace = create_creature(&mut state, PlayerId(0), "Ace's Bat", 3, 3); + add_must_be_blocked_by_dalek(&mut state, ace); + let other_attacker = create_creature(&mut state, PlayerId(0), "Bear", 2, 2); + + // A Dalek with ExtraBlockers { count: Some(1) } — can block 2 creatures. + let dalek = create_creature(&mut state, PlayerId(1), "Dalek Drone", 2, 2); + state + .objects + .get_mut(&dalek) + .unwrap() + .card_types + .subtypes + .push("Dalek".to_string()); + state + .objects + .get_mut(&dalek) + .unwrap() + .static_definitions + .push(StaticDefinition::new(StaticMode::ExtraBlockers { + count: Some(1), + })); + + state.combat = Some(CombatState { + attackers: vec![ + AttackerInfo::attacking_player(ace, PlayerId(1)), + AttackerInfo::attacking_player(other_attacker, PlayerId(1)), + ], + ..Default::default() + }); + + // Dalek blocks the other attacker but not Ace — illegal: the Dalek has + // spare capacity and could also block Ace. + assert!( + validate_blockers(&state, &[(dalek, other_attacker)]).is_err(), + "Dalek with spare capacity blocking elsewhere must still cover Ace" + ); + // Dalek blocks both — legal: Dalek satisfies the filtered requirement, + // and its spare-capacity slot is used for the other attacker. + assert!( + validate_blockers(&state, &[(dalek, ace), (dalek, other_attacker)]).is_ok(), + "Dalek assigned to both attackers should be legal" + ); + } + /// CR 509.1c "if able": with NO able Dalek, the filtered requirement is /// satisfied vacuously — the defender may block with anything or not block. #[test] diff --git a/crates/mtgish-import/src/convert/static_effect.rs b/crates/mtgish-import/src/convert/static_effect.rs index 679c8711e9..62ee2bb940 100644 --- a/crates/mtgish-import/src/convert/static_effect.rs +++ b/crates/mtgish-import/src/convert/static_effect.rs @@ -308,7 +308,16 @@ pub fn convert_permanent_rule( }, P::MustAttack => StaticMode::MustAttack, P::MustBlock => StaticMode::MustBlock, + // CR 509.1c: bare "must be blocked if able" — no filter, any assigned + // blocker satisfies the requirement. P::MustBeBlocked => StaticMode::MustBeBlocked { by: None }, + // CR 509.1c: filtered "must be blocked by a if able" — + // only an assigned blocker matching `filter` satisfies the requirement. + // Covers Ace's Baseball Bat ("must be blocked by a Dalek if able") and + // Slayer's Cleaver ("must be blocked by an Eldrazi if able"). + P::MustBeBlockedByADefender(filter) => StaticMode::MustBeBlocked { + by: Some(filter::convert(filter)?), + }, P::CanBlockOnly(filter) if is_creature_with_flying_filter(filter) => { StaticMode::BlockRestriction { filter: engine::types::statics::block_only_creatures_with_flying_filter(), @@ -1005,9 +1014,50 @@ fn check_hasable_to_keyword(c: &CheckHasable) -> ConvResult