From 9f3e615b7941bdcb1486a39c0f587d9ad6dc8f1c Mon Sep 17 00:00:00 2001 From: Jacob Woodson Date: Sat, 15 Aug 2026 02:25:08 -0500 Subject: [PATCH 1/3] Fix Heroic Sacrifice --- crates/engine/src/game/ability_utils.rs | 3 + crates/engine/src/game/coverage.rs | 27 +- .../game/effects/add_target_replacement.rs | 15 +- .../game/effects/create_damage_replacement.rs | 313 ++++- .../engine/src/game/effects/prevent_damage.rs | 18 +- crates/engine/src/game/replacement.rs | 199 ++- .../src/parser/oracle_effect/imperative.rs | 104 +- crates/engine/src/parser/oracle_nom/filter.rs | 117 +- .../engine/src/parser/oracle_replacement.rs | 1104 +++++++++++++++-- crates/engine/src/types/ability.rs | 138 ++- .../integration/heroic_sacrifice_redirect.rs | 243 ++++ crates/engine/tests/integration/main.rs | 2 + .../engine/tests/integration/oracle_parser.rs | 7 +- .../integration/palisade_giant_redirect.rs | 201 ++- .../integration/pariah_attached_redirect.rs | 477 +++++++ 15 files changed, 2743 insertions(+), 225 deletions(-) create mode 100644 crates/engine/tests/integration/heroic_sacrifice_redirect.rs create mode 100644 crates/engine/tests/integration/pariah_attached_redirect.rs diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index fc3d7ee4c3..7090274b91 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -14652,6 +14652,7 @@ mod tests { let ability = ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: crate::types::ability::RedirectionLifetime::OneOpportunity, source_filter: Some(TargetFilter::SelfRef), combat_scope: None, target_filter: None, @@ -14710,6 +14711,7 @@ mod tests { let ability = ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: crate::types::ability::RedirectionLifetime::OneOpportunity, source_filter: Some(TargetFilter::ChosenDamageSource { filter: None }), combat_scope: None, target_filter: None, @@ -14762,6 +14764,7 @@ mod tests { let ability = ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: crate::types::ability::RedirectionLifetime::OneOpportunity, source_filter: Some(TargetFilter::SelfRef), combat_scope: None, target_filter: None, diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 7fc8d4c867..74693cc92b 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -624,10 +624,21 @@ fn fmt_target(filter: &TargetFilter) -> String { TargetFilter::PostReplacementDamageSource => "prevented event's damage source".into(), TargetFilter::PostReplacementDamageTarget => "prevented damage target".into(), TargetFilter::PostReplacementDamageTargetOwner => "prevented damage target's owner".into(), - TargetFilter::ControllerAndControlledPermanents { permanent_type } => { + // CR 109.1: the "other" article is part of the human-readable scope — a + // change between "you and permanents you control" and "you and OTHER + // permanents you control" must be visible in the coverage/parse diff. + TargetFilter::ControllerAndControlledPermanents { + permanent_type, + source_scope, + } => { + let other = if source_scope.excludes_source() { + "other " + } else { + "" + }; match permanent_type { - Some(ct) => format!("you and {ct:?}s you control"), - None => "you and permanents you control".into(), + Some(ct) => format!("you and {other}{ct:?}s you control"), + None => format!("you and {other}permanents you control"), } } TargetFilter::SpecificObject { id } => format!("object #{}", id.0), @@ -3327,6 +3338,7 @@ fn effect_details(effect: &Effect) -> Vec<(String, String)> { target_filter, redirect_object_filter, recipient_object_filter, + redirect_lifetime, .. } => { if let Some(m) = modification { @@ -3335,6 +3347,15 @@ fn effect_details(effect: &Effect) -> Vec<(String, String)> { if let Some(r) = redirect_to { d.push(("redirect_to".into(), format!("{r:?}"))); } + // CR 614.5 vs CR 611.2a: parser-alterable, and the difference between + // "protects one damage event" and "protects the rest of the turn" — + // omitting it would make that flip invisible to the parse diff. + if !redirect_lifetime.is_one_opportunity() { + d.push(( + "redirect_lifetime".into(), + format!("{redirect_lifetime:?}"), + )); + } if let Some(a) = redirect_amount { d.push(("redirect_amount".into(), format!("{a:?}"))); } diff --git a/crates/engine/src/game/effects/add_target_replacement.rs b/crates/engine/src/game/effects/add_target_replacement.rs index bee07c2fbd..29a948bac8 100644 --- a/crates/engine/src/game/effects/add_target_replacement.rs +++ b/crates/engine/src/game/effects/add_target_replacement.rs @@ -1,8 +1,8 @@ use crate::game::targeting::{extract_source_from_event, resolve_event_context_target}; use crate::types::ability::{ - AbilityDefinition, DamageTargetFilter, DamageTargetPlayerScope, Duration, Effect, EffectError, - EffectKind, ReplacementCondition, ReplacementDefinition, ResolvedAbility, RestrictionExpiry, - TargetFilter, TargetRef, + AbilityDefinition, ControlledPermanentsScope, DamageTargetFilter, DamageTargetPlayerScope, + Duration, Effect, EffectError, EffectKind, ReplacementCondition, ReplacementDefinition, + ResolvedAbility, RestrictionExpiry, TargetFilter, TargetRef, }; use crate::types::events::GameEvent; use crate::types::game_state::GameState; @@ -385,6 +385,10 @@ pub fn resolve( Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Specific(player), permanent_type: None, + // CR 109.1: no "other" article in this class — + // the granted shield covers every permanent the + // targeted player controls. + source_scope: ControlledPermanentsScope::IncludingSource, }); } state.pending_damage_replacements.push(replacement); @@ -411,8 +415,8 @@ mod tests { use crate::game::replacement::{replace_event, ReplacementResult}; use crate::game::zones::create_object; use crate::types::ability::{ - AbilityDefinition, DamageModification, DamageTargetPlayerScope, Duration, - ReplacementDefinition, RestrictionExpiry, TargetFilter, TypeFilter, TypedFilter, + AbilityDefinition, ControlledPermanentsScope, DamageModification, DamageTargetPlayerScope, + Duration, ReplacementDefinition, RestrictionExpiry, TargetFilter, TypeFilter, TypedFilter, }; use crate::types::identifiers::{CardId, ObjectId}; use crate::types::player::PlayerId; @@ -662,6 +666,7 @@ mod tests { Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Specific(PlayerId(1)), permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, }) ); assert_eq!( diff --git a/crates/engine/src/game/effects/create_damage_replacement.rs b/crates/engine/src/game/effects/create_damage_replacement.rs index 963a65fc8d..ec3b71a29a 100644 --- a/crates/engine/src/game/effects/create_damage_replacement.rs +++ b/crates/engine/src/game/effects/create_damage_replacement.rs @@ -1,5 +1,6 @@ use crate::game::effects::choose_damage_source; use crate::game::effects::prevent_damage::resolve_source_filter; +use crate::game::game_object::AttachTarget; use crate::types::ability::{ DamageRedirectTarget, Effect, EffectError, EffectKind, PreventionAmount, ReplacementDefinition, ResolvedAbility, TargetFilter, TargetRef, @@ -31,6 +32,7 @@ pub fn resolve( ability: &ResolvedAbility, events: &mut Vec, ) -> Result<(), EffectError> { + #[allow(clippy::type_complexity)] let ( source_filter, combat_scope, @@ -39,6 +41,7 @@ pub fn resolve( redirect_to, redirect_amount, recipient_object_filter, + redirect_lifetime, ) = match &ability.effect { Effect::CreateDamageReplacement { source_filter, @@ -52,6 +55,7 @@ pub fn resolve( // resolved object arrives via `ability.targets`. redirect_object_filter: _, recipient_object_filter, + redirect_lifetime, } => ( source_filter.clone(), combat_scope.clone(), @@ -60,6 +64,7 @@ pub fn resolve( *redirect_to, *redirect_amount, recipient_object_filter.clone(), + *redirect_lifetime, ), _ => { return Err(EffectError::InvalidParam( @@ -187,18 +192,32 @@ pub fn resolve( .damage_replacement_oneshot_shield(); } (None, Some(recipient)) => { - // CR 614.9: redirection one-shot (Soltari Guerrillas, Beacon of - // Destiny, Jade Monolith, Goblin Psychopath). `Controller` and - // `SourceObject` resolve from the shield host at damage-apply time; - // `ChosenObjectTarget` ("to target creature instead") captures the - // chosen creature now into the shield's `redirect_target` field for - // the applier to read back. - shield = shield - .redirection_shield(recipient, redirect_amount.unwrap_or(PreventionAmount::All)); + // CR 614.9: redirection shield (Soltari Guerrillas, Beacon of + // Destiny, Jade Monolith, Goblin Psychopath, and the CR 611.2a + // duration-bound class — Heroic Sacrifice, Gideon's Sacrifice). + // `Controller` and `SourceObject` resolve from the shield host at + // damage-apply time; `ChosenObjectTarget` ("to target creature + // instead", "…to the chosen creature instead") captures the chosen + // creature now into the shield's `redirect_target` field for the + // applier to read back; `AttachedToSource` reads the host's live + // `attached_to` on every event. + // + // CR 614.5 vs CR 611.2a: `redirect_lifetime` rides onto the shield so + // `damage_done_applier` knows whether this shield is spent by its + // first event or keeps applying until cleanup. + shield = shield.redirection_shield( + recipient, + redirect_amount.unwrap_or(PreventionAmount::All), + redirect_lifetime, + ); if recipient == DamageRedirectTarget::ChosenObjectTarget { // The redirect target is the LAST declared object slot — the // original-recipient slot (Jade Monolith) is declared first when - // both are present, though no single card has both today. + // both are present, though no single card has both today. The + // CR 611.2a class declares NO slot of its own: its recipient is + // the target its parent instruction already chose ("Choose target + // creature you control. …to the chosen creature instead"), which + // reaches this resolver through the propagated parent targets. if let Some(id) = chosen_redirect_object(ability, recipient_consumes_slot) { shield = shield.redirect_target(TargetFilter::SpecificObject { id }); } @@ -287,7 +306,8 @@ fn chosen_redirect_object( /// source's controller; `SourceObject` → the source object itself; /// `ChosenObjectTarget` → `chosen_object`, captured at resolution time into the /// shield's `redirect_target` field (the shield host does not retain the -/// creating ability's targets, so the applier reads them back from there). +/// creating ability's targets, so the applier reads them back from there); +/// `AttachedToSource` → the permanent the source is attached to. /// /// Used by `replacement::damage_done_applier` to rewrite the damage event's /// recipient. Returns `None` when no concrete recipient can be resolved. @@ -304,6 +324,30 @@ pub(crate) fn resolve_redirect_recipient( .map(|obj| TargetRef::Player(obj.controller)), DamageRedirectTarget::SourceObject => Some(TargetRef::Object(source_id)), DamageRedirectTarget::ChosenObjectTarget => chosen_object.map(TargetRef::Object), + // CR 303.4b + CR 301.5a: the Aura's/Equipment's own host, read LIVE from + // `attached_to` on every damage event rather than latched at install, so + // moving the attachment moves the redirect (Pariah, Pariah's Shield, With + // Great Power . . .). `redirect_damage_event` passes `rid.source` — the + // shield host — as `source_id`, so this is that permanent's own host. + // + // `AttachTarget::as_object` yields `None` for a PLAYER host (the Curse + // cycle) and for an unattached source, so both produce no recipient here + // — which the caller treats as "the redirection does nothing". + // + // The player case is a CORPUS boundary, not a rules one: CR 614.9 + // explicitly permits a player recipient ("…with the same damage dealt to + // another battle, creature, planeswalker, or PLAYER"). No corpus Curse + // names its enchanted *player* as a redirect recipient, so nothing binds + // that shape today; if such a card appears, this arm must grow a + // `TargetRef::Player` branch rather than keep returning `None`. + // (`redirect_recipient_is_legal` already accepts player recipients, and + // the CR 614.9 "left the game" clause is checked there.) + DamageRedirectTarget::AttachedToSource => state + .objects + .get(&source_id) + .and_then(|obj| obj.attached_to.as_ref()) + .and_then(AttachTarget::as_object) + .map(TargetRef::Object), } } @@ -330,7 +374,10 @@ mod tests { use super::*; use crate::game::effects::deal_damage; use crate::game::zones::create_object; - use crate::types::ability::{DamageModification, ShieldKind, TargetFilter}; + use crate::types::ability::{ + ControlledPermanentsScope, DamageModification, DamageTargetFilter, DamageTargetPlayerScope, + RedirectionLifetime, ShieldKind, TargetFilter, + }; use crate::types::card_type::CoreType; use crate::types::identifiers::CardId; use crate::types::player::PlayerId; @@ -344,6 +391,7 @@ mod tests { fn amount_oneshot_ability(source: ObjectId, controller: PlayerId) -> ResolvedAbility { ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::OneOpportunity, // SelfRef source filter: the shield fires on damage dealt *by* // the shield host (Desperate Gambit's chosen source ≡ host here). source_filter: Some(TargetFilter::SelfRef), @@ -435,6 +483,7 @@ mod tests { let ability = ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::OneOpportunity, source_filter: Some(TargetFilter::SelfRef), combat_scope: None, target_filter: None, @@ -454,7 +503,8 @@ mod tests { state.objects.get(&source).unwrap().replacement_definitions[0].shield_kind, ShieldKind::Redirection { recipient: DamageRedirectTarget::Controller, - amount: PreventionAmount::All + amount: PreventionAmount::All, + lifetime: RedirectionLifetime::OneOpportunity } )); @@ -500,6 +550,7 @@ mod tests { let ability = ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::OneOpportunity, source_filter: None, combat_scope: None, target_filter: None, @@ -531,7 +582,8 @@ mod tests { shield.shield_kind, ShieldKind::Redirection { recipient: DamageRedirectTarget::ChosenObjectTarget, - amount: PreventionAmount::Next(1) + amount: PreventionAmount::Next(1), + lifetime: RedirectionLifetime::OneOpportunity } )); assert_eq!(shield.valid_card, Some(TargetFilter::SelfRef)); @@ -579,6 +631,231 @@ mod tests { ); } + /// CR 611.2a + CR 614.9 + CR 614.1a: HEROIC SACRIFICE, end to end at the + /// resolver/runtime seam. "Choose target creature you control. Until end of + /// turn, all damage that would be dealt to you and creatures you control is + /// dealt to the chosen creature instead." + /// + /// The spell is an Instant, so its shield lands in the pending registry under + /// the sentinel host; the recipient is the target its parent instruction + /// already bound (propagated into `ability.targets`). + /// + /// REVERT GUARDS — each assertion below names the axis it pins: + /// * `RedirectionLifetime::Continuous` → without it the shield is consumed by + /// the FIRST damage event and the second one lands unredirected; + /// * the `PlayerOrPermanentsControlledBy` victim conjunct → without it only + /// the controller is protected and damage to the bystander is untouched; + /// * `DamageRedirectTarget::ChosenObjectTarget` reading the propagated parent + /// target → without it there is no recipient and the redirect does nothing. + #[test] + fn heroic_sacrifice_continuous_redirect_moves_every_event_to_the_chosen_creature() { + let mut state = GameState::new_two_player(42); + // The resolving Instant itself — deliberately NOT on the battlefield, so + // the shield must survive in `pending_damage_replacements`. + let spell = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Heroic Sacrifice".to_string(), + Zone::Stack, + ); + let chosen = create_creature(&mut state, PlayerId(0), "Chosen Creature"); + let bystander = create_creature(&mut state, PlayerId(0), "Bystander"); + let enemy = create_creature(&mut state, PlayerId(1), "Enemy Creature"); + let attacker = create_creature(&mut state, PlayerId(1), "Damage Source"); + + let ability = ResolvedAbility::new( + Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::Continuous, + source_filter: None, + combat_scope: None, + target_filter: Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: Some(CoreType::Creature), + source_scope: ControlledPermanentsScope::IncludingSource, + }), + modification: None, + redirect_to: Some(DamageRedirectTarget::ChosenObjectTarget), + redirect_amount: None, + redirect_object_filter: None, + recipient_object_filter: None, + }, + // The parent "Choose target creature you control" instruction's bound + // target, propagated into this sub-ability. + vec![TargetRef::Object(chosen)], + spell, + PlayerId(0), + ); + let mut events = Vec::new(); + resolve(&mut state, &ability, &mut events).unwrap(); + + assert_eq!( + state.pending_damage_replacements.len(), + 1, + "an instant-sourced shield lives in the pending registry" + ); + let shield = &state.pending_damage_replacements[0]; + assert!(matches!( + shield.shield_kind, + ShieldKind::Redirection { + recipient: DamageRedirectTarget::ChosenObjectTarget, + amount: PreventionAmount::All, + lifetime: RedirectionLifetime::Continuous + } + )); + assert_eq!( + shield.redirect_target, + Some(TargetFilter::SpecificObject { id: chosen }), + "the parent's chosen creature must be captured as the recipient" + ); + assert_eq!(shield.source_controller, Some(PlayerId(0))); + + let ctx = deal_damage::DamageContext::from_source(&state, attacker).unwrap(); + let life_before = state.players[0].life; + + // Event 1: damage aimed at the controller ("to you"). + let mut events = Vec::new(); + deal_damage::apply_damage_to_target( + &mut state, + &ctx, + TargetRef::Player(PlayerId(0)), + 3, + false, + &mut events, + ) + .unwrap(); + assert_eq!( + state.players[0].life, life_before, + "damage to the controller must move, not land" + ); + assert_eq!(state.objects.get(&chosen).unwrap().damage_marked, 3); + + // Event 2: damage aimed at ANOTHER creature you control. This is the + // conjunct's permanent leg AND the second use of a shield that a + // one-opportunity lifetime would already have consumed. + let mut events = Vec::new(); + deal_damage::apply_damage_to_target( + &mut state, + &ctx, + TargetRef::Object(bystander), + 4, + false, + &mut events, + ) + .unwrap(); + assert_eq!( + state.objects.get(&bystander).unwrap().damage_marked, + 0, + "the \"creatures you control\" victim leg must be protected too" + ); + assert_eq!( + state.objects.get(&chosen).unwrap().damage_marked, + 7, + "a CR 611.2a continuous redirection re-fires for every event in its window" + ); + assert!( + !state.pending_damage_replacements[0].is_consumed, + "a continuous shield is never consumed by use" + ); + + // Negative: a permanent you do NOT control is outside the victim scope. + let mut events = Vec::new(); + deal_damage::apply_damage_to_target( + &mut state, + &ctx, + TargetRef::Object(enemy), + 5, + false, + &mut events, + ) + .unwrap(); + assert_eq!(state.objects.get(&enemy).unwrap().damage_marked, 5); + assert_eq!( + state.objects.get(&chosen).unwrap().damage_marked, + 7, + "an opponent's creature must not be redirected onto the chosen creature" + ); + + // CR 614.5: damage aimed at the CHOSEN creature is inside the victim + // scope, but the shield gets one opportunity per event — it must not + // re-enter itself and double the damage, nor delete it. + let mut events = Vec::new(); + deal_damage::apply_damage_to_target( + &mut state, + &ctx, + TargetRef::Object(chosen), + 2, + false, + &mut events, + ) + .unwrap(); + assert_eq!( + state.objects.get(&chosen).unwrap().damage_marked, + 9, + "self-directed damage is marked exactly once — not 11 (re-entry), not 7 (deleted)" + ); + } + + /// CR 614.9: A `Continuous` redirection whose recipient never bound (no + /// parent target reached the resolver) must make the redirection DO NOTHING — + /// the damage stays on its original recipient. It must never degrade into a + /// CR 615 prevention that deletes the damage. + #[test] + fn continuous_redirect_without_a_bound_recipient_does_nothing() { + let mut state = GameState::new_two_player(42); + let spell = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Heroic Sacrifice".to_string(), + Zone::Stack, + ); + let attacker = create_creature(&mut state, PlayerId(1), "Damage Source"); + + let ability = ResolvedAbility::new( + Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::Continuous, + source_filter: None, + combat_scope: None, + target_filter: Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: Some(CoreType::Creature), + source_scope: ControlledPermanentsScope::IncludingSource, + }), + modification: None, + redirect_to: Some(DamageRedirectTarget::ChosenObjectTarget), + redirect_amount: None, + redirect_object_filter: None, + recipient_object_filter: None, + }, + // No target bound — the degenerate case. + vec![], + spell, + PlayerId(0), + ); + let mut events = Vec::new(); + resolve(&mut state, &ability, &mut events).unwrap(); + assert_eq!(state.pending_damage_replacements[0].redirect_target, None); + + let ctx = deal_damage::DamageContext::from_source(&state, attacker).unwrap(); + let life_before = state.players[0].life; + let mut events = Vec::new(); + deal_damage::apply_damage_to_target( + &mut state, + &ctx, + TargetRef::Player(PlayerId(0)), + 3, + false, + &mut events, + ) + .unwrap(); + assert_eq!( + state.players[0].life, + life_before - 3, + "with no recipient the redirection does nothing — the damage is NOT prevented" + ); + } + /// CR 614.7a: A source dealing 0 damage has no event to replace — the /// redirection does nothing and the shield is NOT consumed. #[test] @@ -589,6 +866,7 @@ mod tests { let ability = ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::OneOpportunity, source_filter: Some(TargetFilter::SelfRef), combat_scope: None, target_filter: None, @@ -635,6 +913,7 @@ mod tests { let ability = ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::OneOpportunity, source_filter: Some(TargetFilter::SelfRef), combat_scope: None, target_filter: None, @@ -695,6 +974,7 @@ mod tests { let ability = ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::OneOpportunity, source_filter: None, combat_scope: None, target_filter: None, @@ -782,6 +1062,7 @@ mod tests { fn chosen_source_redirect_ability(host: ObjectId, controller: PlayerId) -> ResolvedAbility { ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::OneOpportunity, // "a source of your choice" → ChosenDamageSource. source_filter: Some(TargetFilter::ChosenDamageSource { filter: None }), combat_scope: None, @@ -902,6 +1183,7 @@ mod tests { let ability = ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::OneOpportunity, source_filter: Some(TargetFilter::ChosenDamageSource { filter: None }), combat_scope: None, target_filter: None, @@ -980,6 +1262,7 @@ mod tests { let ability = ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::OneOpportunity, source_filter: Some(TargetFilter::SelfRef), combat_scope: Some(crate::types::ability::CombatDamageScope::CombatOnly), target_filter: None, @@ -1117,6 +1400,7 @@ mod tests { }); let ability = ResolvedAbility::new( Effect::CreateDamageReplacement { + redirect_lifetime: RedirectionLifetime::OneOpportunity, source_filter: Some(TargetFilter::ChosenDamageSource { filter: None }), combat_scope: None, target_filter: None, @@ -1159,7 +1443,8 @@ mod tests { shield.shield_kind, ShieldKind::Redirection { recipient: DamageRedirectTarget::Controller, - amount: PreventionAmount::All + amount: PreventionAmount::All, + lifetime: RedirectionLifetime::OneOpportunity } )); diff --git a/crates/engine/src/game/effects/prevent_damage.rs b/crates/engine/src/game/effects/prevent_damage.rs index f452c772d3..a3776f80a6 100644 --- a/crates/engine/src/game/effects/prevent_damage.rs +++ b/crates/engine/src/game/effects/prevent_damage.rs @@ -184,12 +184,18 @@ fn untargeted_damage_filter( // object-only `valid_card` slot would silently drop the player ("you") // leg, so it must yield `Some` here (and `typed_recipient_valid_card_filter` // returns `None` for it) — the shield's controller is the recipient player. - TargetFilter::ControllerAndControlledPermanents { permanent_type } => { - Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { - player: DamageTargetPlayerScope::Controller, - permanent_type: *permanent_type, - }) - } + // + // CR 109.1: the "other" article is carried straight through (The + // Wanderer's "you and OTHER permanents you control" must not prevent + // damage dealt to The Wanderer itself). + TargetFilter::ControllerAndControlledPermanents { + permanent_type, + source_scope, + } => Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: *permanent_type, + source_scope: *source_scope, + }), // CR 608.2c + CR 611.2c + CR 615.11 (issue #6682): a tracked-set // recipient ("those permanents"/"those creatures" — Mutational // Advantage's clause-derived population, Energy Arc's target-derived diff --git a/crates/engine/src/game/replacement.rs b/crates/engine/src/game/replacement.rs index b8d41f1855..4a2073db68 100644 --- a/crates/engine/src/game/replacement.rs +++ b/crates/engine/src/game/replacement.rs @@ -7,8 +7,8 @@ use crate::types::ability::{ DamageModification, DamageRedirectTarget, DamageTargetFilter, DamageTargetPlayerScope, Duration, Effect, EffectScope, ManaSpendPermission, PermissionGrantee, PostReplacementContinuation, PreventionAmount, QuantityExpr, QuantityModification, - ReplacementCondition, ReplacementDefinition, ReplacementMode, ResolvedAbility, ShieldKind, - TapStateChange, TargetFilter, TargetRef, + RedirectionLifetime, ReplacementCondition, ReplacementDefinition, ReplacementMode, + ResolvedAbility, ShieldKind, TapStateChange, TargetFilter, TargetRef, }; use crate::types::card_type::CoreType; use crate::types::counter::CounterType; @@ -1841,16 +1841,55 @@ fn redirect_target_for_rid(state: &GameState, rid: ReplacementId) -> Option Option { + match filter { + // "...is dealt to ~ instead" — the shield host itself. + TargetFilter::SelfRef => Some(DamageRedirectTarget::SourceObject), + // CR 303.4b + CR 301.5a: "...is dealt to enchanted/equipped creature + // instead" — the host the shield's source is attached to. + TargetFilter::AttachedTo => Some(DamageRedirectTarget::AttachedToSource), + // CR 614.9: a concrete object recipient belongs exclusively to the + // EFFECT-CREATED path — `create_damage_replacement::resolve` writes + // `SpecificObject { id }` alongside a `ShieldKind::Redirection` (of + // either `RedirectionLifetime`), and `redirect_chosen_object_for_rid` is + // its reader. Such a shield is claimed by Branch 1b and never reaches + // this Prevention-shield gate; mapping it here would resurrect a consumed + // one-shot as a durable shield. `None` by intent. + TargetFilter::SpecificObject { .. } => None, + other => { + debug_assert!( + false, + "CR 614.9: unmapped durable redirect recipient {other:?} — a recipient was added \ + to parse_durable_redirect_recipient_filter without a mapping here; it would \ + silently degrade to a CR 615 prevention" + ); + None + } + } +} + +/// CR 614.9: Resolve and apply a damage redirection. Shared by the +/// `ShieldKind::Redirection` path (whose own `lifetime` decides consumption) and +/// the durable `ShieldKind::Prevention` + `redirect_target` path (a printed, +/// object-hosted static, whose re-firing lifecycle is governed by the host +/// permanent's presence rather than by depletion, so it always passes +/// [`RedirectionLifetime::Continuous`]). +/// +/// `lifetime` is the CR 614.5-vs-CR 611.2a axis, carried as the typed enum rather +/// than a bool because it is no longer a hard-coded literal at every call site: +/// the `ShieldKind::Redirection` call site reads it off the shield, which the +/// parser stamped from the Oracle grammar. #[allow(clippy::too_many_arguments)] fn redirect_damage_event( state: &mut GameState, @@ -1862,9 +1901,11 @@ fn redirect_damage_event( damage_amount: u32, is_combat: bool, applied: HashSet, - consume_after_redirect: bool, + lifetime: RedirectionLifetime, events: &mut Vec, ) -> ApplyResult { + // CR 614.5: only a single-opportunity shield spends itself on this event. + let consume_after_redirect = lifetime.is_one_opportunity(); // CR 614.7a: A source that would deal 0 damage deals no damage at all — // there is no damage event to redirect. Pass through and do not consume the // shield (no opportunity was spent). @@ -1891,9 +1932,10 @@ fn redirect_damage_event( // CR 614.5: The one-shot opportunity is spent on this event whether // or not the redirection succeeds — consume the shield in both the // success and the "does nothing" (illegal recipient per CR 614.9) - // outcomes. Continuous `ShieldKind::Prevention` + `redirect_target` - // shields pass `consume_after_redirect: false` and re-fire for every - // damage event within their lifetime. + // outcomes. `RedirectionLifetime::Continuous` shields (the durable + // `ShieldKind::Prevention` + `redirect_target` statics and the + // duration-bound Heroic Sacrifice class) are never consumed and + // re-fire for every damage event within their lifetime. if consume_after_redirect { consume_prevention_shield(state, rid, None); } @@ -1935,6 +1977,7 @@ fn redirect_damage_event( rid, recipient, PreventionAmount::Next(n - redirected_amount), + lifetime, ); } } @@ -2180,7 +2223,7 @@ fn damage_done_applier( return ApplyResult::Modified(event); } - // Branch 1b: CR 614.9 — one-shot redirection shield. Whole-event + // Branch 1b: CR 614.9 — effect-created redirection shield. Whole-event // redirections replace the damage event's recipient; amount-capped // redirections split the event, route the redirected portion through the // same replacement/damage application path, and leave any remainder on the @@ -2188,6 +2231,7 @@ fn damage_done_applier( if let Some(ShieldKind::Redirection { recipient, amount: redirect_amount, + lifetime, }) = shield_kind_for_rid(state, rid) { if let ProposedEvent::Damage { @@ -2198,8 +2242,12 @@ fn damage_done_applier( applied, } = event { - // CR 614.9: one-shot redirection shields always consume their single - // opportunity after the redirect resolves (or does nothing). + // CR 614.5 vs CR 611.2a: the shield's OWN stamped lifetime decides + // consumption. "The next time…"/"the next N damage…" shields spend + // their single opportunity here (whether or not the redirect did + // anything); a `Continuous` shield created by "until end of turn, all + // damage … is dealt to instead" (Heroic Sacrifice) keeps + // applying until cleanup prunes it. return redirect_damage_event( state, rid, @@ -2210,7 +2258,7 @@ fn damage_done_applier( damage_amount, is_combat, applied, - true, + lifetime, events, ); } @@ -2247,12 +2295,15 @@ fn damage_done_applier( } = event { // CR 614.9: Continuous "all damage that would be dealt to you ... is - // dealt to this creature instead" statics (Palisade Giant, Veteran - // Bodyguard, Weathered Bodyguards) parse to a `ShieldKind::Prevention` - // shield carrying `redirect_target: Some(SelfRef)`. This is a - // *redirection* (CR 614.9), not a prevention (CR 615) — route it - // through the shared redirection mechanics with - // `consume_after_redirect: false` so the continuous shield re-fires + // dealt to instead" statics parse to a + // `ShieldKind::Prevention` shield carrying a `redirect_target` filter — + // `SelfRef` for the self-recipient class (Palisade Giant, Ancient + // Adamantoise, Empyrial Archangel, Protector of the Crown, Veteran + // Bodyguard, Weathered Bodyguards, Martyrs of Korlis) and `AttachedTo` + // for the attachment-host class (Pariah, Pariah's Shield, With Great + // Power . . .). This is a *redirection* (CR 614.9), not a prevention + // (CR 615) — route it through the shared redirection mechanics with + // `RedirectionLifetime::Continuous` so the durable shield re-fires // for every damage event within its lifetime, and skip the // DamagePrevented / `combat_prevention_tally` bookkeeping entirely // (no damage is prevented — it is dealt to a new recipient). The @@ -2264,20 +2315,22 @@ fn damage_done_applier( // hypothetical future one carrying a `redirect_target` — falls // through to the ordinary prevention arms below rather than reaching // that dead code. - if redirect_target_for_rid(state, rid) == Some(TargetFilter::SelfRef) - && matches!(amount, PreventionAmount::All) + if let Some(recipient) = redirect_target_for_rid(state, rid) + .as_ref() + .and_then(durable_redirect_recipient) + .filter(|_| matches!(amount, PreventionAmount::All)) { return redirect_damage_event( state, rid, - DamageRedirectTarget::SourceObject, + recipient, PreventionAmount::All, source_id, target, dmg, is_combat, applied, - false, + RedirectionLifetime::Continuous, events, ); } @@ -2459,11 +2512,16 @@ fn consume_prevention_shield( } } +/// CR 615.7: Deplete a `Next(n)` redirection shield's remaining amount in place. +/// `lifetime` is carried through unchanged — depletion is an amount edit, never a +/// lifetime change, so a shield can never silently switch between the CR 614.5 +/// one-opportunity and CR 611.2a continuous classes here. fn update_redirection_shield( state: &mut GameState, rid: ReplacementId, recipient: crate::types::ability::DamageRedirectTarget, amount: PreventionAmount, + lifetime: RedirectionLifetime, ) { let repl = if rid.source == ObjectId(0) { state.pending_damage_replacements.get_mut(rid.index) @@ -2475,7 +2533,11 @@ fn update_redirection_shield( }; if let Some(repl) = repl { - repl.shield_kind = ShieldKind::Redirection { recipient, amount }; + repl.shield_kind = ShieldKind::Redirection { + recipient, + amount, + lifetime, + }; } } @@ -5400,6 +5462,7 @@ fn matches_damage_target_filter( DamageTargetFilter::PlayerOrPermanentsControlledBy { player, permanent_type, + source_scope, } => match target { TargetRef::Player(pid) => { player_scope_matches(player, *pid, repl_controller, repl_source, state) @@ -5408,12 +5471,30 @@ fn matches_damage_target_filter( // the scoped player AND, when `permanent_type` is set, of that card // type (Comeuppance protects "planeswalkers you control", not every // permanent you control). - TargetRef::Object(oid) => state.objects.get(oid).is_some_and(|obj| { - player_scope_matches(player, obj.controller, repl_controller, repl_source, state) - && permanent_type - .as_ref() - .is_none_or(|ct| obj.card_types.core_types.contains(ct)) - }), + // + // CR 109.1: the "OTHER" article (Palisade Giant, Ancient Adamantoise, + // The Wanderer) excludes the replacement's own source object from the + // permanent leg. CR 614.5 already stops a self-recipient shield from + // re-entering itself, but this exclusion is what keeps the shield out + // of the CR 616.1 candidate list in the first place — so a second + // applicable replacement is not made to compete with a self-no-op. + // For a shield installed by an instant/sorcery, `repl_source` is the + // sentinel `ObjectId(0)`, which matches no permanent, so the + // exclusion is correctly inert there. + TargetRef::Object(oid) => { + (!source_scope.excludes_source() || *oid != repl_source) + && state.objects.get(oid).is_some_and(|obj| { + player_scope_matches( + player, + obj.controller, + repl_controller, + repl_source, + state, + ) && permanent_type + .as_ref() + .is_none_or(|ct| obj.card_types.core_types.contains(ct)) + }) + } }, DamageTargetFilter::CreatureOnly => match target { TargetRef::Player(_) => false, @@ -9998,10 +10079,10 @@ mod tests { use crate::game::game_object::{AttachTarget, GameObject}; use crate::types::ability::{ AbilityCost, AbilityDefinition, AbilityKind, CastManaObjectScope, CastManaSpentMetric, - ChosenAttribute, Comparator, ControllerRef, Effect, EffectScope, FilterProp, - OriginConstraint, PlayerFilter, PtValue, QuantityExpr, QuantityModification, QuantityRef, - ReplacementDefinition, ReplacementMode, ReplacementPlayerScope, TapStateChange, - TargetFilter, TargetRef, TypeFilter, TypedFilter, + ChosenAttribute, Comparator, ControlledPermanentsScope, ControllerRef, Effect, EffectScope, + FilterProp, OriginConstraint, PlayerFilter, PtValue, QuantityExpr, QuantityModification, + QuantityRef, ReplacementDefinition, ReplacementMode, ReplacementPlayerScope, + TapStateChange, TargetFilter, TargetRef, TypeFilter, TypedFilter, }; use crate::types::actions::GameAction; use crate::types::card_type::CoreType; @@ -10020,6 +10101,36 @@ mod tests { ReplacementDefinition::new(event) } + /// CR 614.9: the durable-redirection recipient mapping is TOTAL over every + /// filter the parser's `parse_durable_redirect_recipient_filter` can emit. An + /// unmapped recipient would fall through to the ordinary CR 615 prevention + /// arms and DELETE the damage instead of moving it. + /// + /// The residual arm's `debug_assert!` is the loud-failure guard for a future + /// recipient added to the parser without a mapping here; it is deliberately + /// NOT exercised by a test (tripping it would abort the test binary) — the + /// assert itself is the assertion. + #[test] + fn durable_redirect_recipient_maps_every_parser_producible_filter() { + assert_eq!( + durable_redirect_recipient(&TargetFilter::SelfRef), + Some(DamageRedirectTarget::SourceObject), + "\"...is dealt to ~ instead\" redirects onto the shield host" + ); + assert_eq!( + durable_redirect_recipient(&TargetFilter::AttachedTo), + Some(DamageRedirectTarget::AttachedToSource), + "\"...is dealt to enchanted/equipped creature instead\" redirects onto the host" + ); + // Owned by the ONE-SHOT path (`redirect_chosen_object_for_rid`), which + // reads it off a `ShieldKind::Redirection` shield — never this gate. + assert_eq!( + durable_redirect_recipient(&TargetFilter::SpecificObject { id: ObjectId(7) }), + None, + "a captured chosen object belongs to the one-shot redirection shield" + ); + } + fn search_found_execute(destination: Zone) -> AbilityDefinition { AbilityDefinition::new( AbilityKind::Spell, @@ -14947,6 +15058,7 @@ mod tests { .damage_target_filter(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Opponent, permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, }); // Hawkeye = ObjectId(10), controlled by P0, power 2. @@ -15366,6 +15478,7 @@ mod tests { .damage_target_filter(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Opponent, permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, }); // Replacement on P0's object let state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); @@ -15391,6 +15504,7 @@ mod tests { .damage_target_filter(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Opponent, permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, }); let state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); @@ -15416,6 +15530,7 @@ mod tests { .damage_target_filter(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Opponent, permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, }); let mut state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); @@ -15452,6 +15567,7 @@ mod tests { DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::SourceChosenPlayer, permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, }, ); let mut state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); @@ -15494,6 +15610,7 @@ mod tests { DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::SourceChosenPlayer, permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, }, ); let mut state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index c288c87d94..5b94760d79 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -24,6 +24,8 @@ use crate::parser::oracle_ir::ast::*; use crate::parser::oracle_ir::diagnostic::OracleDiagnostic; use crate::parser::oracle_nom::bridge::{nom_on_lower, nom_parse_lower, split_once_on_lower}; use crate::parser::oracle_nom::enters_under::{bind_control_clause, name_entry_control_antecedent}; +use crate::parser::oracle_nom::filter as nom_filter; +use crate::parser::oracle_nom::filter::ControlledPermanentsConjunct; use crate::parser::oracle_nom::primitives as nom_primitives; use crate::parser::oracle_nom::quantity as nom_quantity; use crate::parser::oracle_nom::target as nom_target; @@ -6357,15 +6359,22 @@ fn parse_prevent_effect(text: &str, parent_target_available: bool) -> Effect { } else { TargetFilter::Any } - } else if let Some(permanent_type) = parse_compound_you_and_permanents(text, &lower) { - // CR 615 + CR 614.1a: "to you and [] permanents you control" — a - // compound player+permanent recipient (Comeuppance's "you and - // planeswalkers you control"; Channel Harm's "you and permanents you - // control"). Checked BEFORE the bare "to you" scan (which this phrase - // also contains) so the permanent leg is not dropped. Lowered to the - // dedicated `DamageTargetFilter::PlayerOrPermanentsControlledBy` at - // shield creation — never to a bare `Or` that would leak to `valid_card`. - TargetFilter::ControllerAndControlledPermanents { permanent_type } + } else if let Some(conjunct) = parse_compound_you_and_permanents(text, &lower) { + // CR 615 + CR 614.1a: "to you and [other] [] permanents you + // control" — a compound player+permanent recipient (Comeuppance's "you + // and planeswalkers you control"; Channel Harm's "you and permanents you + // control"; The Wanderer's "you and OTHER permanents you control"). + // Checked BEFORE the bare "to you" scan (which this phrase also contains) + // so the permanent leg is not dropped. Lowered to the dedicated + // `DamageTargetFilter::PlayerOrPermanentsControlledBy` at shield creation + // — never to a bare `Or` that would leak to `valid_card`. + // + // CR 109.1: `source_scope` carries the "other" article through so the + // shield does not claim damage dealt to its own source. + TargetFilter::ControllerAndControlledPermanents { + permanent_type: conjunct.permanent_type, + source_scope: conjunct.source_scope, + } } else if nom_primitives::scan_contains(rest, "to you") || nom_primitives::scan_contains(rest, "to its controller") { @@ -6425,33 +6434,23 @@ fn parse_prevent_that_would_deal_source_filter(text: &str, lower: &str) -> Optio } } -/// CR 615 + CR 614.1a: Recognize the compound damage recipient "you and -/// [``] permanents you control". Returns the permanent-leg type -/// restriction — `Some(Some(Planeswalker))` for Comeuppance's "you and -/// planeswalkers you control", `Some(None)` for Channel Harm's bare "you and -/// permanents you control", and `None` when the phrase is absent. Nom -/// combinators only — the plural type word is a single `alt()` axis. -fn parse_compound_you_and_permanents(text: &str, lower: &str) -> Option> { +/// CR 615 + CR 614.1a + CR 109.1: Recognize the compound damage recipient "you +/// and \[other\] [``] permanents you control". Returns the parsed conjunct +/// — the permanent-leg type restriction plus the CR 109.1 self-exclusion article +/// — or `None` when the phrase is absent. +/// +/// The noun phrase itself is NOT re-spelled here: it delegates to +/// [`nom_filter::parse_controlled_permanents_conjunct`], the single authority +/// shared with the replacement surface's `parse_damage_target_phrase`. That is +/// what keeps "artifacts/enchantments/lands you control" and the "other" article +/// available on both surfaces instead of drifting apart. +fn parse_compound_you_and_permanents( + text: &str, + lower: &str, +) -> Option { let region = TextPair::new(text, lower).strip_after("to you and ")?; - let (_, permanent_type) = you_and_controlled_permanent_type(region.lower).ok()?; - Some(permanent_type) -} - -/// CR 614.1a: "`` you control" tail of the compound recipient. Maps -/// the plural permanent-type word to its `CoreType` restriction; bare -/// "permanents" carries no restriction (`None`). -fn you_and_controlled_permanent_type(input: &str) -> OracleResult<'_, Option> { - let (input, permanent_type) = alt(( - value(Some(CoreType::Planeswalker), tag("planeswalkers")), - value(Some(CoreType::Creature), tag("creatures")), - value(Some(CoreType::Artifact), tag("artifacts")), - value(Some(CoreType::Enchantment), tag("enchantments")), - value(Some(CoreType::Land), tag("lands")), - value(None, tag("permanents")), - )) - .parse(input)?; - let (input, _) = tag(" you control").parse(input)?; - Ok((input, permanent_type)) + let (_, conjunct) = nom_filter::parse_controlled_permanents_conjunct(region.lower).ok()?; + Some(conjunct) } /// CR 615.1 + CR 609.7b: Optional trailing "by [source-filter]" on @@ -21441,6 +21440,7 @@ mod tests { /// restriction (Channel Harm). #[test] fn prevent_compound_recipient_you_and_controlled_permanents() { + use crate::types::ability::ControlledPermanentsScope; use crate::types::card_type::CoreType; let planeswalker_text = @@ -21452,7 +21452,8 @@ mod tests { assert_eq!( target, TargetFilter::ControllerAndControlledPermanents { - permanent_type: Some(CoreType::Planeswalker) + permanent_type: Some(CoreType::Planeswalker), + source_scope: ControlledPermanentsScope::IncludingSource, } ); @@ -21465,9 +21466,38 @@ mod tests { assert_eq!( target, TargetFilter::ControllerAndControlledPermanents { - permanent_type: None + permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, } ); + + // CR 109.1: the "other" article must reach the recipient rather than being + // dropped, and the permanent leg must not collapse to a bare `Controller`. + // Before the shared conjunct authority this phrase matched NO compound arm + // at all on this surface (the old copy had no "other" prefix), so the whole + // permanent leg was silently lost. + // + // HONESTY NOTE: the text is The Wanderer's VERBATIM first line, but the + // CARD does not reach this parser — a printed static "Prevent all …" line + // is claimed earlier by `oracle_replacement::parse_damage_prevention_ + // replacement`, which still collapses the victim to `Player { Controller }` + // and drops the permanent leg. That is a separate, pre-existing gap on the + // static-replacement surface and is NOT fixed here; this assertion pins the + // `Effect::PreventDamage` surface's own behavior for the same phrase. + let wanderer_text = + "Prevent all noncombat damage that would be dealt to you and other permanents you control."; + let Effect::PreventDamage { target, .. } = parse_prevent_effect(wanderer_text, false) + else { + panic!("expected PreventDamage"); + }; + assert_eq!( + target, + TargetFilter::ControllerAndControlledPermanents { + permanent_type: None, + source_scope: ControlledPermanentsScope::ExcludingSource, + }, + "The Wanderer must exclude itself from its own noncombat-damage shield" + ); } /// CR 615: the plain "to you" recipient must remain `Controller` — the diff --git a/crates/engine/src/parser/oracle_nom/filter.rs b/crates/engine/src/parser/oracle_nom/filter.rs index de5898e7e9..cf31d90bcf 100644 --- a/crates/engine/src/parser/oracle_nom/filter.rs +++ b/crates/engine/src/parser/oracle_nom/filter.rs @@ -17,9 +17,10 @@ use super::primitives::{ }; use super::quantity::{parse_quantity_expr_number, parse_quantity_ref}; use crate::types::ability::{ - AggregateFunction, Comparator, ControllerRef, FilterProp, ObjectProperty, PtStat, PtValueScope, - QuantityExpr, + AggregateFunction, Comparator, ControlledPermanentsScope, ControllerRef, FilterProp, + ObjectProperty, PtStat, PtValueScope, QuantityExpr, }; +use crate::types::card_type::CoreType; #[cfg(test)] use crate::types::counter::CounterType; use crate::types::counter::{parse_counter_type, CounterMatch}; @@ -501,10 +502,122 @@ pub fn parse_color_property(input: &str) -> OracleResult<'_, FilterProp> { .parse(input) } +/// CR 614.1a + CR 109.1: the parsed "\[other\] `` you control" tail +/// of a compound damage recipient. A named struct rather than a tuple so both +/// axes are explicit at every call site. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ControlledPermanentsConjunct { + /// CR 614.1a: restriction on the permanent leg — `None` for bare + /// "permanents", `Some(ct)` for a plural type word. + pub permanent_type: Option, + /// CR 109.1: whether the leading "other" article excluded the ability's own + /// source object. + pub source_scope: ControlledPermanentsScope, +} + +/// CR 614.1a + CR 109.1: SINGLE AUTHORITY for the "\[other\] `` you +/// control" noun phrase that follows "…to you and " in a compound damage +/// recipient. +/// +/// Both damage surfaces compose this one combinator rather than re-spelling the +/// noun list: +/// * `oracle_effect::imperative::parse_compound_you_and_permanents` → +/// `TargetFilter::ControllerAndControlledPermanents` (the `Effect::PreventDamage` +/// half: Comeuppance, Channel Harm, Blessed Sanctuary, Safe Passage, The +/// Wanderer). +/// * `oracle_replacement::parse_damage_target_phrase` → +/// `DamageTargetFilter::PlayerOrPermanentsControlledBy` (the replacement half: +/// Palisade Giant, Ancient Adamantoise, Heroic Sacrifice, Gideon's Sacrifice). +/// +/// They previously kept two hand-rolled copies that had already drifted apart in +/// both directions — one knew six nouns but not "other", the other knew "other" +/// but only three nouns. One combinator, one noun `alt()`, one article `opt()`. +/// +/// Composed one axis per combinator: the optional CR 109.1 "other" article, the +/// plural type noun, and the fixed " you control" suffix. +pub fn parse_controlled_permanents_conjunct( + input: &str, +) -> OracleResult<'_, ControlledPermanentsConjunct> { + let (input, other) = opt(tag("other ")).parse(input)?; + let (input, permanent_type) = alt(( + value(Some(CoreType::Planeswalker), tag("planeswalkers")), + value(Some(CoreType::Creature), tag("creatures")), + value(Some(CoreType::Artifact), tag("artifacts")), + value(Some(CoreType::Enchantment), tag("enchantments")), + value(Some(CoreType::Land), tag("lands")), + value(None, tag("permanents")), + )) + .parse(input)?; + let (input, _) = tag(" you control").parse(input)?; + Ok(( + input, + ControlledPermanentsConjunct { + permanent_type, + source_scope: match other { + Some(_) => ControlledPermanentsScope::ExcludingSource, + None => ControlledPermanentsScope::IncludingSource, + }, + }, + )) +} + #[cfg(test)] mod tests { use super::*; + /// CR 614.1a + CR 109.1: the single authority must cover every plural noun + /// BOTH former copies knew, and must carry the "other" article rather than + /// discarding it. + #[test] + fn controlled_permanents_conjunct_covers_every_noun_and_the_other_article() { + for (phrase, expected_type) in [ + ("permanents you control", None), + ("creatures you control", Some(CoreType::Creature)), + ("planeswalkers you control", Some(CoreType::Planeswalker)), + ("artifacts you control", Some(CoreType::Artifact)), + ("enchantments you control", Some(CoreType::Enchantment)), + ("lands you control", Some(CoreType::Land)), + ] { + let (rest, plain) = parse_controlled_permanents_conjunct(phrase) + .unwrap_or_else(|_| panic!("{phrase} must parse")); + assert!(rest.is_empty(), "{phrase} must be fully consumed"); + assert_eq!(plain.permanent_type, expected_type); + assert_eq!( + plain.source_scope, + ControlledPermanentsScope::IncludingSource, + "no \"other\" article means the source is included" + ); + + let othered = format!("other {phrase}"); + let (rest, excluded) = parse_controlled_permanents_conjunct(&othered) + .unwrap_or_else(|_| panic!("{othered} must parse")); + assert!(rest.is_empty()); + assert_eq!(excluded.permanent_type, expected_type); + assert_eq!( + excluded.source_scope, + ControlledPermanentsScope::ExcludingSource, + "the \"other\" article must reach the caller, not be opt()-discarded" + ); + } + } + + /// Hostile: the combinator must not claim a phrase whose controller clause is + /// absent or inverted, and must leave the remainder untouched on failure. + #[test] + fn controlled_permanents_conjunct_fails_closed_off_grammar() { + for phrase in [ + "permanents an opponent controls", + "creatures", + "other stuff you control", + "creature you control", + ] { + assert!( + parse_controlled_permanents_conjunct(phrase).is_err(), + "{phrase} must not be claimed by the conjunct authority" + ); + } + } + #[test] fn test_parse_zone_filter_battlefield() { let (rest, z) = parse_zone_filter("on the battlefield this turn").unwrap(); diff --git a/crates/engine/src/parser/oracle_replacement.rs b/crates/engine/src/parser/oracle_replacement.rs index b6d0d0603a..5e94c40d85 100644 --- a/crates/engine/src/parser/oracle_replacement.rs +++ b/crates/engine/src/parser/oracle_replacement.rs @@ -24,6 +24,7 @@ use super::oracle_nom::condition::{ parse_opponent_who_controls_at_least_as_many, parse_you_cast_another_spell_filter_this_turn, }; use super::oracle_nom::duration::parse_duration; +use super::oracle_nom::filter as nom_filter; use super::oracle_nom::primitives as nom_primitives; use super::oracle_nom::quantity as nom_quantity; use super::oracle_nom::target::parse_type_filter_word; @@ -36,14 +37,14 @@ use super::oracle_util::{ use crate::types::ability::CastingPermission; use crate::types::ability::{ AbilityCost, AbilityDefinition, AbilityKind, CastVariantPaid, ChoiceType, CombatDamageScope, - Comparator, ContinuousModification, ControllerRef, CopyManaValueLimit, CountScope, - CounterReplacementSubject, DamageModification, DamageRedirectTarget, DamageTargetFilter, - DamageTargetPlayerScope, DrawReplacementScope, Duration, Effect, EffectScope, FilterProp, - LibraryPosition, ManaModification, ManaReplacementScope, ManaSpendPermission, - PermissionGrantee, PlayerFilter, PreventionAmount, QuantityExpr, QuantityModification, - QuantityRef, ReplacementCondition, ReplacementDefinition, ReplacementMode, - ReplacementPlayerScope, StaticCondition, StaticDefinition, TapStateChange, TargetFilter, - TriggerDefinition, TypeFilter, TypedFilter, + Comparator, ContinuousModification, ControlledPermanentsScope, ControllerRef, + CopyManaValueLimit, CountScope, CounterReplacementSubject, DamageModification, + DamageRedirectTarget, DamageTargetFilter, DamageTargetPlayerScope, DrawReplacementScope, + Duration, Effect, EffectScope, FilterProp, LibraryPosition, ManaModification, + ManaReplacementScope, ManaSpendPermission, PermissionGrantee, PlayerFilter, PreventionAmount, + QuantityExpr, QuantityModification, QuantityRef, RedirectionLifetime, ReplacementCondition, + ReplacementDefinition, ReplacementMode, ReplacementPlayerScope, StaticCondition, + StaticDefinition, TapStateChange, TargetFilter, TriggerDefinition, TypeFilter, TypedFilter, }; use crate::types::card_type::Supertype; use crate::types::counter::{CounterMatch, CounterType}; @@ -6367,6 +6368,16 @@ pub(crate) fn parse_oneshot_damage_replacement(norm_lower: &str) -> Option is dealt to + // instead" (Heroic Sacrifice, Gideon's Sacrifice, Saving Grace). Its required + // leading "all " is disjoint from both "the next N damage" forms above and + // from the "the next time" spine below, so ordering here is for readability, + // not for disambiguation. + if let Some(effect) = parse_continuous_all_damage_redirect(norm_lower) { + return Some(effect); + } + // CR 614.1a + CR 514.2: "the next time ... this turn" — a replacement effect // ("instead", CR 614.1a) with a "this turn" duration that ends at cleanup // (CR 514.2). The one-opportunity consumption is CR 614.5 (see resolver). @@ -6424,6 +6435,8 @@ pub(crate) fn parse_oneshot_damage_replacement(norm_lower: &str) -> Option Option { parse_damage_to_target_filter(result_clause) } - DamageRedirectTarget::Controller | DamageRedirectTarget::SourceObject => None, + // `redirect_object_filter` carries the filter for a CHOSEN object slot + // the player must select. `AttachedToSource` joins the `None` arm + // deliberately, not by default: like `SourceObject`, its recipient is + // resolved from game state at apply time (the source's `attached_to`) + // and surfaces no target slot. The "the next time …" spine never + // produces `AttachedToSource` (its recipient grammar has no + // "enchanted/equipped creature" arm); the CR 611.2a sibling + // `parse_continuous_all_damage_redirect` is where that recipient + // lives, and it likewise declares no slot. + DamageRedirectTarget::Controller + | DamageRedirectTarget::SourceObject + | DamageRedirectTarget::AttachedToSource => None, }; return Some(Effect::CreateDamageReplacement { source_filter, @@ -6444,6 +6468,8 @@ pub(crate) fn parse_oneshot_damage_replacement(norm_lower: &str) -> Option Option Option` instead" tail of +/// [`parse_continuous_all_damage_redirect`]. +/// +/// Two recipient identities, and only two: +/// * a CHOSEN-PERMANENT anaphor ("the chosen creature" / "the chosen permanent") +/// naming the permanent the SAME spell's preceding instruction already chose +/// (Heroic Sacrifice's "Choose target creature you control.", Gideon's +/// Sacrifice's "Choose a creature or planeswalker you control." — both of which +/// the chunker lifts into a `TargetOnly` parent whose target propagates into +/// this sub-ability). It surfaces NO target slot of its own; the resolver reads +/// the already-bound parent target. +/// * the ATTACHMENT HOST ("enchanted creature" / "equipped creature" — Saving +/// Grace), delegated to `parse_attached_host_subject`, the module's single +/// authority for that noun phrase. +/// +/// DELIBERATELY NOT HERE: "that creature" (Ascent of the Worthy). Its victim +/// scope is the bare "creatures you control" with no player leg, which has no +/// `DamageTargetFilter` representation, so the card fails closed at the victim +/// slot regardless; adding the anaphor without the victim would be an +/// unreachable arm kept alive by nothing. +fn parse_continuous_redirect_recipient(input: &str) -> OracleResult<'_, DamageRedirectTarget> { + alt(( + value( + DamageRedirectTarget::ChosenObjectTarget, + alt(( + tag::<_, _, OracleError<'_>>("the chosen creature"), + tag("the chosen permanent"), + )), + ), + value( + DamageRedirectTarget::AttachedToSource, + parse_attached_host_subject, + ), + )) + .parse(input) +} + +/// CR 611.2a + CR 614.9: the CONTINUOUS damage redirection created by a +/// RESOLVING spell or ability — the line-anchored production +/// +/// ```text +/// "all " " that would be dealt " ["this turn "] +/// " is dealt to " " instead" ["."] EOF +/// ``` +/// +/// This is the "one-shot path" sibling of [`parse_redirection_spine`] (the +/// printed, object-hosted static). The two are distinguished by *who creates the +/// effect*, which the grammar shows in two places: this one always leads with +/// "all " and its recipient is a chosen-permanent anaphor or an attachment host +/// with a stated duration, never the printed static's "~". +/// +/// CR 611.2a is what makes the resulting shield `RedirectionLifetime::Continuous` +/// rather than a CR 614.5 one-opportunity shield: "all damage that would be dealt +/// …" for a stated window applies to EVERY matching damage event in that window, +/// unlike "the next time …"/"the next N damage …", which are spent by their first +/// event. The window itself is end-of-turn cleanup (CR 514.2), which +/// `ShieldKind::is_shield()` already enforces — the two spellings the corpus uses +/// are Heroic Sacrifice's leading "Until end of turn," (lifted to the ability's +/// own `duration` by the chunker before this parser sees the clause) and Gideon's +/// Sacrifice / Saving Grace's inline "this turn". +/// +/// FAIL-CLOSED ANCHORS (each declines the whole line rather than emitting a +/// partially-understood shield): +/// * the required leading "all " keeps every "the next N damage …" depletion +/// redirection on its own parser; +/// * an unsupported victim (Ascent of the Worthy's bare "to creatures you +/// control") fails inside `parse_damage_target_phrase`; +/// * an unsupported recipient fails in `parse_continuous_redirect_recipient` — +/// emitting a recipient-less shield would be a CR 615 prevention that DELETES +/// the damage (CR 615.1a: prevention effects "use the word 'prevent'", and this +/// grammar never says it); +/// * a "by ``" scope clause, a trailing sentence, or any other residue +/// breaks the required `tag(" is dealt to ")` / end-of-input anchor. No corpus +/// card in this class carries one, so no speculative slot is parsed for it. +fn parse_continuous_all_damage_redirect(norm_lower: &str) -> Option { + let (rest, _) = tag::<_, _, OracleError<'_>>("all ") + .parse(norm_lower) + .ok()?; + let (rest, combat_scope) = parse_damage_noun_with_scope(rest).ok()?; + let (rest, _) = tag::<_, _, OracleError<'_>>(" that would be dealt ") + .parse(rest) + .ok()?; + // CR 514.2: the inline duration spelling (Gideon's Sacrifice, Saving Grace). + // Heroic Sacrifice's leading "Until end of turn," was already lifted onto the + // ability by the chunker, so this slot is optional. + let (rest, _) = opt(tag::<_, _, OracleError<'_>>("this turn ")) + .parse(rest) + .ok()?; + let (rest, victim) = parse_damage_target_phrase(rest).ok()?; + let (rest, _) = tag::<_, _, OracleError<'_>>(" is dealt to ") + .parse(rest) + .ok()?; + let (rest, redirect_to) = parse_continuous_redirect_recipient(rest).ok()?; + let (rest, _) = tag::<_, _, OracleError<'_>>(" instead").parse(rest).ok()?; + // `strip_reminder_text` can leave " ." where the "(if it's still on the + // battlefield)" parenthetical stood, so the final period may be detached. + let (rest, _) = opt(preceded(multispace0, char::<_, OracleError<'_>>('.'))) + .parse(rest) + .ok()?; + if !rest.trim().is_empty() { + return None; + } + + Some(Effect::CreateDamageReplacement { + source_filter: None, + combat_scope, + target_filter: Some(victim), + modification: None, + redirect_to: Some(redirect_to), + // `None` → `PreventionAmount::All`: the whole event moves, every time. + redirect_amount: None, + // CR 115.1: no NEW target slot. A chosen-permanent recipient reuses the + // slot the parent "Choose …" instruction already declared; an attachment + // host is read from the shield host's live `attached_to`. + redirect_object_filter: None, + recipient_object_filter: None, + redirect_lifetime: RedirectionLifetime::Continuous, }) } @@ -7017,6 +7167,12 @@ fn parse_qualified_chosen_damage_source(subject: &str) -> Option { /// "that source deals that damage to ") collapse to two `to`-anchors; the /// recipient is "you" (Controller), "~" (the source object), or "target /// creature"/"target permanent" (a chosen object target). +/// +/// This serves the ONE-SHOT `Effect::CreateDamageReplacement` path and returns +/// `DamageRedirectTarget`. Its durable counterpart is +/// [`parse_durable_redirect_recipient_filter`], which is positional (not +/// scanning) and returns the `TargetFilter` stored in +/// `ReplacementDefinition::redirect_target`. fn parse_redirect_recipient(body: &str) -> Option { nom_primitives::scan_at_word_boundaries(body, parse_redirect_recipient_phrase) } @@ -7350,17 +7506,25 @@ fn damage_target_opponent() -> DamageTargetFilter { } } +/// CR 109.1: "to an opponent or a permanent an opponent controls" states no +/// "other" article, so the permanent leg includes every permanent that opponent +/// controls (the shield's own source is never one of them anyway — it is +/// controlled by the other player). fn damage_target_opponent_or_permanents() -> DamageTargetFilter { DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Opponent, permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, } } +/// CR 109.1: "to the chosen player or a permanent they control" — likewise no +/// "other" article. fn damage_target_source_chosen_player_or_permanents() -> DamageTargetFilter { DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::SourceChosenPlayer, permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, } } @@ -7391,6 +7555,42 @@ fn parse_damage_target_phrase( alt((tag("to a creature"), tag("to that creature"))), ), value(damage_target_opponent(), tag("to an opponent")), + // CR 614.1a + CR 614.9: the CONJUNCT victim scope — "to you and [other] + // you control" (Palisade Giant, Ancient Adamantoise, Heroic + // Sacrifice, Gideon's Sacrifice). Ordered BEFORE the bare `tag("to you")` + // arm so the longer production wins; without it the conjunct's permanent + // leg is silently dropped and only the controller is protected. + // + // The noun phrase is NOT re-spelled here: `"to you and "` is the only tag + // this arm owns, and everything after it delegates to + // `nom_filter::parse_controlled_permanents_conjunct` — the single + // authority shared with the `Effect::PreventDamage` surface in + // `oracle_effect/imperative.rs` (`parse_compound_you_and_permanents` → + // `TargetFilter::ControllerAndControlledPermanents`). Both surfaces + // therefore agree on the six plural nouns AND on the CR 109.1 "other" + // article, which is carried into `source_scope` rather than discarded. + // + // BOUNDARY — the `"and/or"` spelling is deliberately out of scope. The + // prefix `tag("to you and ")` carries a trailing space, so it cannot match + // "to you and/or ...". Five corpus cards use that spelling — Divine + // Deflection, Refraction Trap, Shadowbane (`Effect::PreventDamage`) and + // Harm's Way, Shining Shoal (the one-shot "next N damage" family) — and + // all five collapse their victim to the controller today on OTHER parsers. + // Widening this to `alt((tag("to you and "), tag("to you and/or ")))` + // reclassifies all five across two other effect paths and must not be done + // without re-running the card-data corpus diff; see the negative guard + // `damage_target_phrase_does_not_claim_and_or_conjunct`. + nom::combinator::map( + preceded( + tag("to you and "), + nom_filter::parse_controlled_permanents_conjunct, + ), + |conjunct| DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: conjunct.permanent_type, + source_scope: conjunct.source_scope, + }, + ), // CR 602.2a: "to you" — the recipient is the ability's controller. For an // activated ability that is the activator (Mercenaries: "Any player may // activate this ability"), so the scope must be carried through rather @@ -9684,18 +9884,181 @@ fn parse_cant_become_untapped_replacement( /// `parse_type_phrase_unblocked_attacking_creatures_you_control` test. This /// function adds NO new unblocked-detection — only the "by ... is dealt to" /// boundary extraction that the redirection grammar does not yet have. -/// Returns `None` when no "by " clause is present, so the unrestricted-source -/// class (Pariah / Palisade Giant) is unaffected. -fn parse_damage_redirection_source_clause(working_lower: &str) -> Option { - let (_, (_, after_by)) = nom_primitives::split_once_on(working_lower, " by ").ok()?; - let (_, (subject, _)) = nom_primitives::split_once_on(after_by, " is dealt to").ok()?; - parse_damage_source_subject_filter(subject.trim()) +/// Fails (rather than yielding `None`) when no " by " clause is present, so the +/// unrestricted-source class (Pariah / Palisade Giant) simply skips the +/// `opt(...)`-wrapped slot in `parse_redirection_spine`. +/// +/// POSITIONAL FORM: `parse_damage_source_subject_filter` is an `Option`-returning +/// WHOLE-STRING consumer (it requires the subject to be fully consumed), not a +/// `nom::Parser`, so it cannot occupy a bare `preceded()` slot and has no way to +/// stop at " is dealt to". The `" is dealt to"` delimiter is what bounds the +/// subject; `take_until` locates it without consuming it (the spine's following +/// `tag(" is dealt to ")` still sees it), `peek` makes that boundary contract +/// explicit, and `map_opt` bridges the `Option` return. +/// +/// FAIL-CLOSED: the spine wraps this in `opt(...)`. If " by " is present but the +/// subject does not type, `map_opt` errors, `opt` swallows it WITHOUT consuming, +/// and the spine's next required `tag(" is dealt to ")` then fails on the +/// leftover " by ..." — declining the whole line. That is the +/// correct outcome: silently dropping an unrecognized source restriction would +/// widen the shield from "damage from these sources" to "all damage". +fn parse_damage_redirection_source_slot(input: &str) -> OracleResult<'_, TargetFilter> { + preceded( + tag::<_, _, OracleError<'_>>(" by "), + map_opt( + terminated(take_until(" is dealt to"), peek(tag(" is dealt to"))), + |subject: &str| parse_damage_source_subject_filter(subject.trim()), + ), + ) + .parse(input) +} + +/// CR 120.2a: the anchored damage noun of the redirection spine, carrying its +/// optional combat scope positionally — `["noncombat"|"combat"] "damage"`. The +/// "noncombat" and "combat" arms precede the bare arm because "combat damage" +/// contains "damage" and "noncombat damage" contains "combat damage". +/// +/// This is the spine's ANCHOR: its failure at position 0 is what declines every +/// multi-sentence line (Heroic Sacrifice — "Choose target creature you +/// control. ..."), every "the next N damage ... this turn" one-shot (Captain's +/// Maneuver, Carom — CR 615.7 depletion redirections that belong on the +/// `Effect::CreateDamageReplacement` path), and every granted-ability quotation +/// (Martyrdom). The pre-existing scanning sibling `scan_combat_scope` cannot +/// serve here: it matches at ANY word boundary, so it can bind a scope token +/// from a different sentence. +fn parse_damage_noun_with_scope(input: &str) -> OracleResult<'_, Option> { + alt(( + value( + Some(CombatDamageScope::NoncombatOnly), + tag::<_, _, OracleError<'_>>("noncombat damage"), + ), + value(Some(CombatDamageScope::CombatOnly), tag("combat damage")), + value(None, tag("damage")), + )) + .parse(input) +} + +/// CR 614.9 + CR 301.5a + CR 303.4b: the recipient slot of a DURABLE (printed, +/// object-hosted) redirection — the "... is dealt to instead" tail of +/// [`parse_redirection_spine`]. Returns the `TargetFilter` stored in +/// `ReplacementDefinition::redirect_target`, which `game/replacement.rs`'s +/// `durable_redirect_recipient` maps to a `DamageRedirectTarget` at apply time. +/// +/// NOT [`parse_redirect_recipient`] / `parse_redirect_recipient_phrase`: those +/// serve the ONE-SHOT `Effect::CreateDamageReplacement` path ("... prevent that +/// damage. It deals that damage to "), scan at word boundaries, and +/// return `DamageRedirectTarget` directly. This one is positional and returns +/// `TargetFilter`. The two representations are known debt (the durability axis is +/// encoded by which of the two you pick); unifying them is a whole-shield- +/// lifecycle refactor, not something to fold in here. +/// +/// An unrecognized recipient MUST `Err` so the caller declines the line rather +/// than degrading a CR 614.9 redirection into a CR 615 prevention — the exact +/// defect this spine exists to remove: CR 615.1a prevention effects "use the word +/// 'prevent'", and this grammar never says "prevent". A prevention shield with no +/// recipient DELETES the damage instead of moving it. +fn parse_durable_redirect_recipient_filter(input: &str) -> OracleResult<'_, TargetFilter> { + alt(( + value(TargetFilter::SelfRef, tag::<_, _, OracleError<'_>>("~")), + // CR 301.5a ("equipped creature") + CR 303.4b ("enchanted") — Pariah, + // Pariah's Shield, With Great Power . . . . Composed from the module's + // existing single authority for that noun phrase rather than re-spelling + // its two tags; `parse_damage_source_subject_filter` already uses the same + // helper for the SOURCE side of this grammar, so both sides agree. + parse_attached_host_subject, + )) + .parse(input) +} + +/// The parsed shape of a durable damage-redirection line. A typed struct rather +/// than a tuple of `Option`s so each axis is named at every call site. +struct RedirectionSpine { + /// CR 120.2a: `None` = all damage; `Some(_)` = combat/noncombat only. + combat_scope: Option, + /// CR 614.9: whose damage is replaced ("to you", "to you and creatures you + /// control", ...). + victim: DamageTargetFilter, + /// CR 509.1h: optional " by " scope restriction. + source_filter: Option, + /// CR 614.9: the new recipient. REQUIRED — never `Option`. + recipient: TargetFilter, +} + +/// CR 614.9 + CR 614.1a: the line-anchored durable-redirection production +/// +/// ```text +/// ["all "] " that would be dealt " [" by " ] +/// " is dealt to " " instead" ["."] EOF +/// ``` +/// +/// Anchored and fully consuming, replacing two whole-text `scan_contains` probes +/// that could match tokens in DIFFERENT sentences of the same line — the defect +/// that let Heroic Sacrifice's three-sentence body be claimed as a single +/// replacement (its "Choose target creature you control" target clause and its +/// CR 603.7a delayed trigger were both swallowed into the description). +/// +/// CR 611.2a: there is deliberately NO `"this turn"` / `"until end of turn"` +/// duration slot. A stated duration means a continuous effect generated by the +/// RESOLUTION of a spell or ability, which belongs on that ability's effect chain +/// (`Effect::CreateDamageReplacement`), not on a printed object-hosted static +/// replacement. Only Turn the Tables and Mirror Strike actually decline on that +/// token (their victim parses, then the required `tag(" is dealt to ")` meets +/// " this turn"); the other duration-bearing cards decline earlier — Heroic +/// Sacrifice / Captain's Maneuver / Carom / Martyrdom at the damage-noun anchor, +/// Sivvi's Valor ("to target creature") and Treacherous Link ("to enchanted +/// creature") inside `parse_damage_target_phrase`. No card-type flag is plumbed +/// into the parser; the grammar alone does the work. +fn parse_redirection_spine(input: &str) -> OracleResult<'_, RedirectionSpine> { + let (input, _) = opt(tag::<_, _, OracleError<'_>>("all ")).parse(input)?; + let (input, combat_scope) = parse_damage_noun_with_scope(input)?; + let (input, _) = tag(" that would be dealt ").parse(input)?; + let (input, victim) = parse_damage_target_phrase(input)?; + let (input, source_filter) = opt(parse_damage_redirection_source_slot).parse(input)?; + let (input, _) = tag(" is dealt to ").parse(input)?; + let (input, recipient) = parse_durable_redirect_recipient_filter(input)?; + let (input, _) = tag(" instead").parse(input)?; + // `strip_reminder_text` can leave " ." where a parenthetical stood, so the + // sentence-final period may be detached from the preceding word. + let (input, _) = opt(preceded(multispace0, char('.'))).parse(input)?; + if !input.trim().is_empty() { + return Err(oracle_err(input)); + } + Ok(( + input, + RedirectionSpine { + combat_scope, + victim, + source_filter, + recipient, + }, + )) } /// CR 614.1a + CR 604.2: Parse damage redirection replacement effects. -/// Handles "all damage that would be dealt to [target] is dealt to ~ instead" (Pariah, Palisade Giant) -/// and "if a source would deal damage to you, prevent that damage. ~ deals that much damage to -/// any target" (Pariah's Shield). +/// +/// Patterns 1/2 are the DURABLE (printed, object-hosted) redirection — +/// "[all] [combat] damage that would be dealt to [by ] is dealt +/// to instead" (Pariah, Pariah's Shield, With Great Power . . ., +/// Palisade Giant, Ancient Adamantoise, Empyrial Archangel, Protector of the +/// Crown, and the three "by " cards: Veteran Bodyguard, Weathered +/// Bodyguards, Martyrs of Korlis). Recognized by [`parse_redirection_spine`], +/// which consumes the WHOLE line: an unrecognized victim, source, or recipient +/// declines the line entirely rather than emitting a partially-understood shield. +/// +/// There is deliberately no second "if a source would deal damage to you, +/// prevent that damage. ~ deals that much damage to any target" pattern here. It +/// existed until this change with ZERO live corpus members — every "would deal +/// damage to you" + "prevent that damage" card (the Circles/Runes of Protection, +/// Deflecting Palm, Nine Lives, Bone Mask, Rem Karolus, …) is claimed by +/// `parse_damage_prevention_replacement`, which `parse_replacement_line` consults +/// several hundred lines EARLIER — and its only test fed it a FABRICATED Oracle +/// line attributed to Pariah's Shield. Pariah's Shield actually reads "All damage +/// that would be dealt to you is dealt to equipped creature instead.", which is +/// the spine above. Dead code kept alive by invented text is exactly the defect +/// class this file's `damage_redirection_to_self_instead` re-pointing fixed, so +/// the pattern and its fixture were removed rather than relabeled; the guard +/// `prevent_and_redirect_class_is_owned_by_the_prevention_handler` pins the real +/// corpus line to the real handler. /// /// CR 604.2: an optional leading "as long as , " gate (a printed /// static ability's own continuous-effect activation condition) is stripped and @@ -9722,73 +10085,32 @@ fn parse_damage_redirection_replacement( // Pattern 1: "all damage that would be dealt to [X] is dealt to ~ instead" (Pariah) // Pattern 2: "damage that would be dealt to [X] is dealt to ~ instead" (Palisade Giant) - // CR 615.1a: Redirect = prevent original + deal to new target - if nom_primitives::scan_contains(working_lower, "would be dealt to") - && nom_primitives::scan_contains(working_lower, "is dealt to") - { - let target_filter = if nom_primitives::scan_contains(working_lower, "would be dealt to you") - { - Some(damage_target_controller()) - } else { - // "would be dealt to ~" or other targets — no specific filter - None - }; - - // Determine redirect destination - let redirect = if nom_primitives::scan_contains(working_lower, "is dealt to ~ instead") { - // Redirect to self (the permanent with this ability) - Some(TargetFilter::SelfRef) - } else { - None - }; - - // CR 614.9 + CR 509.1h: optional "by " scope-restriction. - let source_filter = parse_damage_redirection_source_clause(working_lower); - // CR 120.2a: optional "combat damage" qualifier scopes to combat damage only. - let combat_scope = scan_combat_scope(working_lower); - + if let Ok((_, spine)) = parse_redirection_spine(working_lower) { + // CR 614.9: This is a REDIRECTION, not a CR 615 prevention — the grammar + // never says "prevent" (CR 615.1a). The `PreventionAmount::All` shield is + // only the durable-redirection ENCODING the runtime's `damage_done_applier` + // reads (its Branch-2 gate maps `redirect_target` through + // `durable_redirect_recipient` and routes to `redirect_damage_event`); it + // is always paired with a recipient here, never emitted bare. A bare + // prevention shield would DELETE the damage instead of moving it. let mut def = ReplacementDefinition::new(ReplacementEvent::DamageDone) .prevention_shield(PreventionAmount::All) + .damage_target_filter(spine.victim) + .redirect_target(spine.recipient) .description(original_text.to_string()); - if let Some(tf) = target_filter { - def = def.damage_target_filter(tf); - } - if let Some(rt) = redirect { - def = def.redirect_target(rt); - } // CR 604.2: attach the leading "as long as " gate. if let Some(cond) = prefix_condition { def = def.condition(cond); } - if let Some(sf) = source_filter { + if let Some(sf) = spine.source_filter { def = def.damage_source_filter(sf); } - if let Some(cs) = combat_scope { + if let Some(cs) = spine.combat_scope { def = def.combat_scope(cs); } return Some(def); } - // Pattern 3: "if a source would deal damage to you, prevent that damage" - // followed by "~ deals that much damage to any target" (Pariah's Shield) - // CR 615.1a: Prevention + redirect combination - if nom_primitives::scan_contains(working_lower, "would deal damage to you") - && nom_primitives::scan_contains(working_lower, "prevent that damage") - { - let mut def = ReplacementDefinition::new(ReplacementEvent::DamageDone) - .prevention_shield(PreventionAmount::All) - .damage_target_filter(damage_target_controller()) - .redirect_target(TargetFilter::SelfRef) - .description(original_text.to_string()); - // CR 604.2: attach the leading "as long as " gate, same as - // Pattern 1/2 above — no current card matches Pattern 3 with this - // prefix, but silently dropping it would be wrong if one existed. - if let Some(cond) = prefix_condition { - def = def.condition(cond); - } - return Some(def); - } - None } @@ -13718,7 +14040,8 @@ mod tests { assert_eq!( *target, TargetFilter::ControllerAndControlledPermanents { - permanent_type: Some(CoreType::Planeswalker) + permanent_type: Some(CoreType::Planeswalker), + source_scope: ControlledPermanentsScope::IncludingSource, } ); assert!(matches!( @@ -13794,7 +14117,8 @@ mod tests { assert_eq!( *target, TargetFilter::ControllerAndControlledPermanents { - permanent_type: None + permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, }, "Channel Harm's \"you and permanents you control\" is unrestricted" ); @@ -19048,12 +19372,30 @@ mod tests { )); } + /// Heroic Sacrifice's VERBATIM Scryfall Oracle text — one line, three + /// sentences: a CR 115 target clause, a CR 611.2a "until end of turn" + /// redirection, and a CR 603.7a delayed trigger. + const HEROIC_SACRIFICE_TEXT: &str = "Choose target creature you control. Until end of turn, all damage that would be dealt to you and creatures you control is dealt to the chosen creature instead (if it's still on the battlefield). When that creature dies this turn, put its counters on up to one target creature you control and draw a card."; + + /// Palisade Giant's VERBATIM Scryfall redirection line (byte-identical to + /// Ancient Adamantoise's). + const PALISADE_GIANT_TEXT: &str = "All damage that would be dealt to you and other permanents you control is dealt to this creature instead."; + #[test] fn damage_redirection_to_self_instead() { - // CR 614.1a: "All damage that would be dealt to you is dealt to ~ instead" + // CR 614.1a: "All damage that would be dealt to you is dealt to ~ instead". + // + // Re-pointed at Empyrial Archangel: this test previously used the + // FABRICATED line "...is dealt to Pariah instead." with card_name "Pariah" + // (which `replace_self_refs` normalizes to `~`). Pariah's real Oracle text + // is "...is dealt to enchanted creature instead." — an attachment-host + // recipient, not a self recipient — so the fabricated fixture is exactly + // why Pariah's misparse survived undetected. Empyrial Archangel's line is + // verbatim Scryfall text for the `~` recipient this test is about; Pariah + // is covered by `pariah_redirects_to_attachment_host` below. let def = parse_replacement_line( - "All damage that would be dealt to you is dealt to Pariah instead.", - "Pariah", + "All damage that would be dealt to you is dealt to this creature instead.", + "Empyrial Archangel", ) .unwrap(); assert_eq!(def.event, ReplacementEvent::DamageDone); @@ -19068,6 +19410,542 @@ mod tests { assert_eq!(def.redirect_target, Some(TargetFilter::SelfRef)); } + #[test] + fn pariah_redirects_to_attachment_host() { + // CR 614.9 + CR 303.4b: Pariah's VERBATIM Scryfall redirection line. The + // recipient is the enchanted creature, not the Aura itself — before the + // anchored spine this produced a bare `Prevention{All}` shield with NO + // redirect destination, i.e. a CR 615 prevention that deleted the damage. + for (name, text) in [ + ( + "Pariah", + "All damage that would be dealt to you is dealt to enchanted creature instead.", + ), + ( + "Pariah's Shield", + "All damage that would be dealt to you is dealt to equipped creature instead.", + ), + ] { + let def = parse_replacement_line(text, name) + .unwrap_or_else(|| panic!("{name}'s redirect must parse")); + assert_eq!( + def.redirect_target, + Some(TargetFilter::AttachedTo), + "{name} must redirect to its attachment host" + ); + assert_eq!(def.damage_target_filter, Some(damage_target_controller())); + assert_eq!(def.combat_scope, None); + assert_eq!(def.damage_source_filter, None); + } + } + + #[test] + fn durable_redirect_recipient_filter_delegates_to_attached_host_authority() { + // The durable recipient combinator must COMPOSE the module's existing + // single authority for the attachment-host noun phrase rather than + // re-spelling its two tags: same value, same remainder, same (absent) + // widening. + for phrase in ["equipped creature instead", "enchanted creature instead"] { + let (rest, filter) = parse_durable_redirect_recipient_filter(phrase) + .unwrap_or_else(|_| panic!("{phrase} must parse as an attachment host")); + assert_eq!(filter, TargetFilter::AttachedTo); + assert_eq!( + rest, " instead", + "the recipient slot must stop before \" instead\"" + ); + + let shared = parse_attached_host_subject(phrase).expect("shared authority must agree"); + assert_eq!((shared.0, shared.1), (rest, filter)); + } + + // `~` is the other (and only other) recognized durable recipient. + assert_eq!( + parse_durable_redirect_recipient_filter("~ instead").unwrap(), + (" instead", TargetFilter::SelfRef) + ); + + // Hostile: the shared authority has no "enchanted " arm, so + // the recipient slot must NOT have accidentally widened. + assert!( + parse_durable_redirect_recipient_filter("enchanted artifact instead").is_err(), + "an unrecognized attachment-host noun must fail closed" + ); + } + + #[test] + fn heroic_sacrifice_multi_sentence_line_is_not_claimed_as_replacement() { + // CR 614.9 vs CR 615.1a: Heroic Sacrifice's VERBATIM Scryfall text is one + // three-sentence line. Two whole-text `scan_contains` probes matched tokens + // in different sentences and produced a `Prevention{All}` shield with no + // recipient — deleting the damage, dropping the "creatures you control" + // victim leg, and swallowing both the target clause and the CR 603.7a + // delayed trigger into the description. The anchored spine declines at the + // damage-noun anchor (position 0 is "choose target creature you control"). + assert!( + parse_replacement_line(HEROIC_SACRIFICE_TEXT, "Heroic Sacrifice").is_none(), + "a multi-sentence line must not be claimed by the durable redirection handler" + ); + + // Positive reach-guard: the handler still claims a real single-sentence + // durable redirection with the same conjunct victim scope. + let giant = parse_replacement_line(PALISADE_GIANT_TEXT, "Palisade Giant") + .expect("Palisade Giant's redirect must still parse"); + assert_eq!(giant.redirect_target, Some(TargetFilter::SelfRef)); + assert_eq!( + giant.damage_target_filter, + Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: None, + source_scope: ControlledPermanentsScope::ExcludingSource, + }), + "the \"and other permanents you control\" conjunct must be carried, not dropped" + ); + } + + /// Walk an ability's `sub_ability` chain the way `oracle::has_unimplemented` + /// does, collecting every `Effect::Unimplemented` description. + fn collect_chain_gaps(def: &AbilityDefinition, out: &mut Vec) { + if let Effect::Unimplemented { description, .. } = &*def.effect { + out.push(description.clone().unwrap_or_default()); + } + if let Some(sub) = def.sub_ability.as_deref() { + collect_chain_gaps(sub, out); + } + } + + /// Find the first `CreateDamageReplacement` in an ability's `sub_ability` + /// chain. + fn find_chain_damage_replacement(def: &AbilityDefinition) -> Option<&Effect> { + let mut cursor = Some(def); + while let Some(node) = cursor { + if matches!(&*node.effect, Effect::CreateDamageReplacement { .. }) { + return Some(&node.effect); + } + cursor = node.sub_ability.as_deref(); + } + None + } + + #[test] + fn heroic_sacrifice_installs_a_continuous_redirect_onto_the_chosen_creature() { + // CR 611.2a + CR 614.9: Heroic Sacrifice's VERBATIM three-sentence line. + // + // Two prior states were both wrong: a bogus card-level `Prevention{All}` + // shield with no recipient (damage DELETED, victim leg dropped, target + // clause and CR 603.7a delayed trigger swallowed into the description), + // and then an honest-but-inert `Effect::Unimplemented` gap. Neither + // protected anything. The clause must now lower to the real effect. + let parsed = parse_oracle_text( + HEROIC_SACRIFICE_TEXT, + "Heroic Sacrifice", + &[], + &["Instant".to_string()], + &[], + ); + + // Still no CARD-LEVEL replacement: this is an Instant, which never + // reaches the battlefield, so a card-hosted replacement could never be a + // candidate. The shield is created by the resolving spell instead. + assert!( + parsed.replacements.is_empty(), + "no card-level replacement may be synthesized from this line, got {:?}", + parsed.replacements + ); + + let mut unimplemented = Vec::new(); + for ability in &parsed.abilities { + collect_chain_gaps(ability, &mut unimplemented); + } + assert!( + unimplemented.is_empty(), + "the redirection clause must no longer be an honest gap, got {unimplemented:?}" + ); + + let effect = parsed + .abilities + .iter() + .find_map(|a| find_chain_damage_replacement(a)) + .unwrap_or_else(|| { + panic!( + "the redirection clause must lower to CreateDamageReplacement, got {:?}", + parsed.abilities + ) + }); + let Effect::CreateDamageReplacement { + target_filter, + redirect_to, + redirect_amount, + redirect_lifetime, + redirect_object_filter, + recipient_object_filter, + combat_scope, + source_filter, + modification, + } = effect + else { + unreachable!("matched above") + }; + // CR 614.1a: BOTH victim legs — the controller AND the creatures they + // control. Reverting the conjunct arm flips this to `Player{Controller}`. + assert_eq!( + *target_filter, + Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: Some(CoreType::Creature), + // No "other" article on this card. + source_scope: ControlledPermanentsScope::IncludingSource, + }) + ); + // CR 614.9: the recipient is the permanent the parent "Choose target + // creature you control" instruction already bound — no NEW target slot. + assert_eq!( + *redirect_to, + Some(DamageRedirectTarget::ChosenObjectTarget), + "\"the chosen creature\" must resolve to the parent's chosen object" + ); + assert_eq!( + *redirect_object_filter, None, + "the recipient reuses the parent target slot; declaring a second slot would demand a second target" + ); + assert_eq!(*recipient_object_filter, None); + // CR 611.2a: continuous, not a CR 614.5 single opportunity. Reverting the + // lifetime axis flips this and the card protects only the first hit. + assert_eq!(*redirect_lifetime, RedirectionLifetime::Continuous); + assert_eq!( + *redirect_amount, None, + "the WHOLE event moves, so no depletion cap" + ); + assert_eq!(*combat_scope, None); + assert_eq!(*source_filter, None); + assert_eq!(*modification, None); + + // The CR 603.7a delayed trigger that follows must still be its own node — + // the redirection clause must not have swallowed it. + let has_delayed_trigger = parsed.abilities.iter().any(|a| { + let mut cursor = Some(a); + while let Some(node) = cursor { + if matches!(&*node.effect, Effect::CreateDelayedTrigger { .. }) { + return true; + } + cursor = node.sub_ability.as_deref(); + } + false + }); + assert!( + has_delayed_trigger, + "the \"When that creature dies this turn\" clause must remain its own delayed trigger" + ); + } + + #[test] + fn gideons_sacrifice_inline_this_turn_reaches_the_same_continuous_class() { + // CR 611.2a: the sibling spelling — the duration is INLINE ("dealt this + // turn to …") instead of a leading "Until end of turn,", and the victim + // leg is untyped ("permanents"). Same class, same effect shape. + // VERBATIM Scryfall text. + let parsed = parse_oracle_text( + "Choose a creature or planeswalker you control. All damage that would be dealt this turn to you and permanents you control is dealt to the chosen permanent instead (if it's still on the battlefield).", + "Gideon's Sacrifice", + &[], + &["Instant".to_string()], + &[], + ); + let mut unimplemented = Vec::new(); + for ability in &parsed.abilities { + collect_chain_gaps(ability, &mut unimplemented); + } + assert!( + unimplemented.is_empty(), + "Gideon's Sacrifice must have no remaining gap, got {unimplemented:?}" + ); + let effect = parsed + .abilities + .iter() + .find_map(|a| find_chain_damage_replacement(a)) + .expect("Gideon's Sacrifice must lower to CreateDamageReplacement"); + let Effect::CreateDamageReplacement { + target_filter, + redirect_to, + redirect_lifetime, + .. + } = effect + else { + unreachable!("matched above") + }; + assert_eq!( + *target_filter, + Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, + }) + ); + assert_eq!(*redirect_to, Some(DamageRedirectTarget::ChosenObjectTarget)); + assert_eq!(*redirect_lifetime, RedirectionLifetime::Continuous); + } + + #[test] + fn continuous_all_damage_redirect_fails_closed_off_class() { + // Hostile fixtures for every anchor of the CR 611.2a production. Each must + // decline outright — a partially-understood shield here is either a CR 615 + // prevention that DELETES damage or a shield with the wrong victim scope. + for (label, clause) in [ + // Missing the required leading "all " — this is the CR 615.7 depletion + // family, owned by `parse_oneshot_next_n_damage_to_*`. + ( + "next-N depletion form", + "the next 3 damage that would be dealt to you and creatures you control is dealt to the chosen creature instead", + ), + // VERBATIM Ascent of the Worthy chapter text (post-normalization): the + // bare "to creatures you control" victim has no DamageTargetFilter + // representation, so the whole line must fail closed rather than + // silently widen to "you and creatures you control". + ( + "Ascent of the Worthy victim", + "all damage that would be dealt to creatures you control is dealt to that creature instead", + ), + // Unsupported recipient — must not degrade into a recipient-less shield. + ( + "unsupported recipient", + "all damage that would be dealt to you and creatures you control is dealt to any target instead", + ), + // Trailing text after the clause. + ( + "trailing sentence", + "all damage that would be dealt to you is dealt to the chosen creature instead. draw a card.", + ), + // A "by " scope clause: no corpus card in this class has one, + // and silently dropping it would widen the shield to all sources. + ( + "unparsed by-source clause", + "all damage that would be dealt to you by unblocked creatures is dealt to the chosen creature instead", + ), + ] { + assert!( + parse_oneshot_damage_replacement(clause).is_none(), + "{label} must not be claimed by the continuous redirection class" + ); + } + + // Paired positive reach-guard: the minimally-different in-class clause IS + // claimed, proving each rejection above is caused by its own anchor and + // not by the whole family being unreachable. + let effect = parse_oneshot_damage_replacement( + "all damage that would be dealt to you is dealt to the chosen creature instead", + ) + .expect("the in-class control clause must parse"); + assert!(matches!( + effect, + Effect::CreateDamageReplacement { + redirect_to: Some(DamageRedirectTarget::ChosenObjectTarget), + redirect_lifetime: RedirectionLifetime::Continuous, + .. + } + )); + } + + #[test] + fn saving_grace_clause_redirects_onto_the_attachment_host_continuously() { + // CR 303.4b + CR 611.2a: the attachment-host recipient of the same class. + // Saving Grace's trigger BODY, verbatim (the "When this Aura enters," + // trigger head is a separate, still-unsupported gap — see the report). + let effect = parse_oneshot_damage_replacement( + "all damage that would be dealt this turn to you and permanents you control is dealt to enchanted creature instead", + ) + .expect("Saving Grace's redirection clause must parse"); + assert!(matches!( + effect, + Effect::CreateDamageReplacement { + redirect_to: Some(DamageRedirectTarget::AttachedToSource), + redirect_lifetime: RedirectionLifetime::Continuous, + redirect_object_filter: None, + .. + } + )); + + // BOUNDARY: the same recipient WITHOUT the "all …" continuous spine is + // Pariah's printed static, which belongs to `parse_replacement_line`'s + // durable spine and must never be claimed as an effect-created shield. + assert!( + parse_oneshot_damage_replacement("enchanted creature gets +2/+2").is_none(), + "an unrelated attachment-host line must not be claimed" + ); + } + + #[test] + fn redirection_spine_rejects_trailing_text_after_the_clause() { + // The end-of-input guard. SYNTHETIC fixture: no corpus card reaches this + // guard (every real declining line fails earlier), so a constructed one is + // the only way to exercise it. The clause is Pariah's real line plus a + // trailing sentence. + assert!( + parse_replacement_line( + "All damage that would be dealt to you is dealt to enchanted creature instead. Draw a card.", + "Pariah", + ) + .is_none(), + "a redirection clause with trailing text must not be claimed" + ); + // Positive reach-guard: the identical clause WITHOUT the trailing sentence + // parses, proving the rejection is caused by the trailing text alone. + let def = parse_replacement_line( + "All damage that would be dealt to you is dealt to enchanted creature instead.", + "Pariah", + ) + .expect("the same clause without trailing text must parse"); + assert_eq!(def.redirect_target, Some(TargetFilter::AttachedTo)); + } + + #[test] + fn unrecognized_redirect_recipient_never_yields_a_bare_prevention_shield() { + // CR 615.1a: prevention effects "use the word 'prevent'". None of these + // lines does, so none may produce a prevention shield. Each must decline + // outright rather than degrade into one. + // + // The first entry is the genuine recipient-slot reach fixture: Heroic + // Sacrifice's redirection clause in isolation. Its victim parses (the new + // conjunct arm), `" is dealt to "` matches, and the recipient "the chosen + // creature" is what fails — the only corpus-derived input that reaches + // `parse_durable_redirect_recipient_filter`'s error branch. + // + // The remaining entries are verbatim corpus lines that decline EARLIER + // (Sivvi's Valor / Treacherous Link inside `parse_damage_target_phrase`; + // Turn the Tables / Mirror Strike at the required `tag(" is dealt to ")`, + // meeting " this turn"), plus one SYNTHETIC adjacent-grammar line. The + // assertion is deliberately about the OUTCOME, so the test stays honest + // regardless of which anchor each line dies at. + for (name, text) in [ + ( + "Heroic Sacrifice", + "All damage that would be dealt to you and creatures you control is dealt to the chosen creature instead.", + ), + ( + "Sivvi's Valor", + "All damage that would be dealt to target creature this turn is dealt to you instead.", + ), + ( + "Treacherous Link", + "All damage that would be dealt to enchanted creature is dealt to its controller instead.", + ), + ( + "Turn the Tables", + "All combat damage that would be dealt to you this turn is dealt to target attacking creature instead.", + ), + ( + "Mirror Strike", + "All combat damage that would be dealt to you this turn by target unblocked creature is dealt to its controller instead.", + ), + ( + "Captain's Maneuver", + "The next X damage that would be dealt to target creature, planeswalker, or player this turn is dealt to another target creature, planeswalker, or player instead.", + ), + // SYNTHETIC: adjacent grammar with an unsupported "any target" + // recipient — must decline, not guess a recipient. + ( + "Synthetic Any Target", + "All damage that would be dealt to you is dealt to any target instead.", + ), + ] { + match parse_replacement_line(text, name) { + None => {} + Some(def) => panic!( + "{name} must decline; got shield_kind={:?} redirect_target={:?}", + def.shield_kind, def.redirect_target + ), + } + } + } + + #[test] + fn damage_target_phrase_does_not_claim_and_or_conjunct() { + // BOUNDARY guard for the shared `parse_damage_target_phrase` edit. The new + // conjunct arm leads with `tag("to you and ")` (trailing space), so the + // "and/or" spelling falls through to the pre-existing bare `tag("to you")` + // arm — it does NOT error. Five corpus cards use that spelling (Divine + // Deflection, Refraction Trap, Shadowbane on `Effect::PreventDamage`; + // Harm's Way, Shining Shoal on the one-shot path) and must stay on their + // current parsers. Widening the tag would silently reclassify all five. + for (phrase, unconsumed) in [ + ( + "to you and/or permanents you control", + " and/or permanents you control", + ), + ( + "to you and/or creatures you control", + " and/or creatures you control", + ), + ] { + let (rest, filter) = + parse_damage_target_phrase(phrase).expect("the bare \"to you\" arm still matches"); + assert_eq!( + filter, + damage_target_controller(), + "the and/or spelling must not reach PlayerOrPermanentsControlledBy" + ); + assert_eq!( + rest, unconsumed, + "the and/or conjunct must be left entirely unconsumed" + ); + } + + // Paired positive: the space-separated spelling DOES reach the new arm. + let (rest, filter) = parse_damage_target_phrase("to you and other permanents you control") + .expect("the conjunct arm must match the space-separated spelling"); + assert_eq!( + filter, + DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: None, + // CR 109.1: the "other" article must reach the filter, not be + // opt()-discarded. + source_scope: ControlledPermanentsScope::ExcludingSource, + } + ); + assert!(rest.is_empty()); + + // The typed legs of the same conjunct. + assert_eq!( + parse_damage_target_phrase("to you and creatures you control") + .unwrap() + .1, + DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: Some(CoreType::Creature), + source_scope: ControlledPermanentsScope::IncludingSource, + } + ); + assert_eq!( + parse_damage_target_phrase("to you and planeswalkers you control") + .unwrap() + .1, + DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: Some(CoreType::Planeswalker), + source_scope: ControlledPermanentsScope::IncludingSource, + } + ); + + // CR 614.1a: the three nouns the replacement surface previously did NOT + // know, now available for free because both surfaces share one authority. + // Latent today (no corpus card), so this is the guard that keeps the two + // surfaces from silently drifting apart again. + for (phrase, expected) in [ + ("to you and artifacts you control", CoreType::Artifact), + ("to you and enchantments you control", CoreType::Enchantment), + ("to you and lands you control", CoreType::Land), + ] { + assert_eq!( + parse_damage_target_phrase(phrase).unwrap().1, + DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: Some(expected), + source_scope: ControlledPermanentsScope::IncludingSource, + }, + "{phrase} must reach the shared conjunct authority" + ); + } + } + #[test] fn veteran_bodyguard_redirect_source_restricted_no_combat_scope() { // CR 604.2 (static "as long as" gate) + CR 614.9 (redirection) + CR 509.1h @@ -19130,20 +20008,28 @@ mod tests { #[test] fn palisade_giant_current_oracle_no_unblocked_source_restriction() { // Palisade Giant's REAL current Oracle text (Scryfall-verified) has NO - // tap-condition and NO combat/unblocked qualifier. The recipient-list half - // ("and other permanents you control") has its own separate, pre-existing - // bug in this same conditional — deferred as a separate follow-up, NOT - // fixed by this PR. This test only guards the redirection-scope - // regression this PR touches. - let def = parse_replacement_line( - "All damage that would be dealt to you and other permanents you control is dealt to this creature instead.", - "Palisade Giant", - ) - .expect("Palisade Giant's redirect must still parse after the Bodyguard fix"); + // tap-condition and NO combat/unblocked qualifier. The conjunct victim + // half ("and other permanents you control") IS handled here now — both + // its permanent leg and its CR 109.1 self-exclusion — so this test + // asserts the whole victim filter rather than only the redirect scope. + let def = parse_replacement_line(PALISADE_GIANT_TEXT, "Palisade Giant") + .expect("Palisade Giant's redirect must still parse after the Bodyguard fix"); // Positive: the redirect itself still applies (reach guard). assert_eq!(def.redirect_target, Some(TargetFilter::SelfRef)); + // CR 614.1a + CR 109.1: the conjunct victim, carried in full. Reverting + // either the conjunct arm or the "other" axis flips this assertion. + assert_eq!( + def.damage_target_filter, + Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: None, + source_scope: ControlledPermanentsScope::ExcludingSource, + }), + "\"you and OTHER permanents you control\" must carry both the permanent leg and the self-exclusion" + ); + // Negative: no unblocked/combat restriction was spuriously attached. assert_eq!(def.combat_scope, None); assert_eq!(def.damage_source_filter, None); @@ -19151,23 +20037,34 @@ mod tests { } #[test] - fn damage_redirection_prevent_and_redirect() { - // CR 614.1a: "If a source would deal damage to you, prevent that damage. - // ~ deals that much damage to any target." + fn prevent_and_redirect_class_is_owned_by_the_prevention_handler() { + // Replaces `damage_redirection_prevent_and_redirect`, which fed a + // FABRICATED line ("If a source would deal damage to you, prevent that + // damage. Pariah's Shield deals that much damage to any target.") under a + // real card name to keep a redirection-handler pattern alive that no + // corpus card ever reached. Pariah's Shield's real Oracle text is "All + // damage that would be dealt to you is dealt to equipped creature + // instead." — the anchored spine, covered by + // `pariah_redirects_to_attachment_host`. + // + // This is the honest replacement: VERBATIM Nine Lives, the real + // "if a source would deal damage to you, prevent that damage" card, is + // still claimed — by `parse_damage_prevention_replacement`, which + // `parse_replacement_line` consults long before the redirection handler. let def = parse_replacement_line( - "If a source would deal damage to you, prevent that damage. Pariah's Shield deals that much damage to any target.", - "Pariah's Shield", + "If a source would deal damage to you, prevent that damage and put an incarnation counter on this enchantment.", + "Nine Lives", ) - .unwrap(); + .expect("Nine Lives' prevention replacement must still be claimed"); assert_eq!(def.event, ReplacementEvent::DamageDone); - assert_eq!(def.damage_target_filter, Some(damage_target_controller())); - assert!(matches!( - def.shield_kind, - ShieldKind::Prevention { - amount: PreventionAmount::All - } - )); - assert_eq!(def.redirect_target, Some(TargetFilter::SelfRef)); + // Revert guard for the deletion: the removed pattern's signature output + // was a `SelfRef` redirect destination on a prevention shield. A + // prevention is NOT a redirection (CR 615.1a vs CR 614.9), so this card + // must carry no redirect destination at all. + assert_eq!( + def.redirect_target, None, + "a CR 615 prevention must not acquire a CR 614.9 redirect destination" + ); } #[test] @@ -21740,6 +22637,8 @@ mod snapshot_tests { // offers the slot (Defect 1). redirect_object_filter: Some(_), recipient_object_filter: None, + // CR 614.5: "the next time …" is spent by one event. + redirect_lifetime: RedirectionLifetime::OneOpportunity, } => {} other => panic!("expected redirect-to-target-creature, got {other:?}"), } @@ -21768,6 +22667,8 @@ mod snapshot_tests { source_filter: None, combat_scope: None, target_filter: None, + // CR 615.7: a depleting "next N damage" shield is one-opportunity. + redirect_lifetime: RedirectionLifetime::OneOpportunity, } => {} other => panic!("expected en-Kor redirect-to-target, got {other:?}"), } @@ -22230,6 +23131,7 @@ mod snapshot_tests { source_filter: None, combat_scope: None, target_filter: None, + redirect_lifetime: RedirectionLifetime::OneOpportunity, } => {} other => panic!("expected redirect-target->source, got {other:?}"), } @@ -22254,6 +23156,7 @@ mod snapshot_tests { source_filter: None, combat_scope: None, target_filter: None, + redirect_lifetime: RedirectionLifetime::OneOpportunity, } => {} other => panic!("expected redirect-target->controller, got {other:?}"), } @@ -22303,6 +23206,7 @@ mod snapshot_tests { source_filter: None, combat_scope: None, target_filter: None, + redirect_lifetime: RedirectionLifetime::OneOpportunity, } => {} other => panic!("expected recipient+redirect both chosen targets, got {other:?}"), } diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 17b72b0729..8de806c329 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -1339,10 +1339,11 @@ pub enum PreventionAmount { AllBut(u32), } -/// CR 614.9: Recipient of a one-shot damage-redirection effect — the +/// CR 614.9: Recipient of a damage-redirection effect — the /// battle/creature/planeswalker/player the replaced damage is dealt to instead. -/// Resolved to a concrete object/player at effect-resolution time (see -/// `effects::create_damage_replacement::resolve`). +/// Each variant is a distinct IDENTITY SOURCE for that recipient, resolved +/// against live game state at damage-apply time by +/// `effects::create_damage_replacement::resolve_redirect_recipient`. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(tag = "type")] pub enum DamageRedirectTarget { @@ -1355,6 +1356,15 @@ pub enum DamageRedirectTarget { /// "...to target creature instead" — an object chosen as a target of the /// creating ability (Soltari Guerrillas). ChosenObjectTarget, + /// CR 303.4b + CR 301.5a: "...to enchanted creature instead" / "...to + /// equipped creature instead" — the permanent this replacement's source is + /// attached to (Pariah, Pariah's Shield, With Great Power . . .). + /// + /// Resolved LIVE from the source's `attached_to` at apply time, never latched + /// at install: if the attachment moves, the redirect follows it; if it falls + /// off, CR 614.9 makes the redirection do nothing and the damage stays on its + /// original recipient. + AttachedToSource, } /// Shield type for one-shot replacement effects that expire at cleanup. @@ -1373,16 +1383,57 @@ pub enum ShieldKind { /// a continuous static `damage_modification` (Furnace of Rath), which keeps /// `ShieldKind::None` and re-applies to every damage event. DamageReplacementOneShot, - /// CR 614.9: One-shot redirection shield — replaces all or part of a damage - /// event's recipient with `recipient`. `All` covers "the next time ... would - /// deal damage"; `Next(n)` covers "the next N damage ... is dealt to ..." - /// redirections. Consumed on use, expires at cleanup. + /// CR 614.9: Redirection shield — replaces all or part of a damage event's + /// recipient with `recipient`. `All` covers "the next time ... would deal + /// damage"; `Next(n)` covers "the next N damage ... is dealt to ..." + /// redirections. Expires at cleanup. + /// + /// `lifetime` is the CR 614.5-vs-CR 611.2a axis: whether the shield spends a + /// single opportunity (Desperate Gambit, the en-Kor cycle) or applies + /// continuously to every matching event in its window (Heroic Sacrifice, + /// Gideon's Sacrifice). See [`RedirectionLifetime`]. Redirection { recipient: DamageRedirectTarget, amount: PreventionAmount, + #[serde(default)] + lifetime: RedirectionLifetime, }, } +/// CR 614.5 vs CR 611.2a: how many damage events one `ShieldKind::Redirection` +/// applies to. +/// +/// This is a genuine grammar-borne axis, NOT derivable from the shield's other +/// fields: "the next time … would deal damage … instead" and "until end of turn, +/// all damage … is dealt to X instead" both carry `PreventionAmount::All`, yet +/// the first is spent by one event and the second is not. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(tag = "type")] +pub enum RedirectionLifetime { + /// CR 614.5: "a replacement effect gets only one opportunity to affect an + /// event" — this shield is consumed after the single event it applies to + /// (whether or not the redirection itself did anything, per CR 614.9). + /// "The next time …" / "the next N damage …" (Desperate Gambit, Soltari + /// Guerrillas, Jade Monolith, the en-Kor cycle). + #[default] + OneOpportunity, + /// CR 611.2a: a continuous effect created by a resolving spell or ability + /// with a stated duration — it re-applies to EVERY matching damage event + /// until the shield is pruned at cleanup (CR 514.2). "Until end of turn, all + /// damage that would be dealt to you and creatures you control is dealt to + /// the chosen creature instead" (Heroic Sacrifice, Gideon's Sacrifice, + /// Saving Grace). + Continuous, +} + +impl RedirectionLifetime { + /// True for the CR 614.5 single-opportunity lifetime (the serde default, so + /// this doubles as the `skip_serializing_if` predicate). + pub fn is_one_opportunity(&self) -> bool { + matches!(self, RedirectionLifetime::OneOpportunity) + } +} + impl ShieldKind { pub fn is_none(&self) -> bool { matches!(self, ShieldKind::None) @@ -5317,9 +5368,13 @@ pub enum TargetFilter { /// dropped) — `typed_recipient_valid_card_filter` returns `None` for it and /// `untargeted_damage_filter` always yields `Some`. `permanent_type` restricts /// the permanent leg (`Some(Planeswalker)` for Comeuppance; `None` = any - /// permanent you control). + /// permanent you control), and `source_scope` carries the "OTHER" article + /// (The Wanderer's "you and other permanents you control") so the permanent + /// leg can exclude the ability's own source. ControllerAndControlledPermanents { permanent_type: Option, + #[serde(default)] + source_scope: ControlledPermanentsScope, }, /// CR 102.2 + CR 102.3 + CR 601.2c: A player reference to an opponent of the /// ability's controller, used as the announcing player (`target_chooser`) for @@ -13263,6 +13318,18 @@ pub enum Effect { /// Soltari) or implicit ("you" — Beacon). #[serde(default, skip_serializing_if = "Option::is_none")] recipient_object_filter: Option, + /// CR 614.5 vs CR 611.2a: whether the created redirection shield spends a + /// single opportunity or applies continuously until cleanup. Carried from + /// the Oracle grammar — "the next time / the next N damage" is + /// `OneOpportunity`, "\[until end of turn,\] all damage that would be + /// dealt … is dealt to `` instead" is `Continuous` — because + /// the two forms are indistinguishable from the shield's other fields. + /// Meaningless for the `modification` form, which is always one-shot. + #[serde( + default, + skip_serializing_if = "RedirectionLifetime::is_one_opportunity" + )] + redirect_lifetime: RedirectionLifetime, }, /// CR 614.1a + CR 614.6 + CR 514.2 + CR 121.1: install a one-shot, this-turn /// "the next time you would draw a card this turn, [effect] instead" draw @@ -23428,6 +23495,42 @@ pub enum DamageTargetPlayerScope { Specific(PlayerId), } +/// CR 109.1 (cited as identity foundation — the CR has no dedicated "another" +/// entry; mirrors the citation on [`FilterProp::Another`]): whether the +/// controlled-permanents leg of a compound "`` and \[other\] `` you control" recipient includes the object the ability is printed on. +/// +/// This is the typed home of the "OTHER" article that the phrase grammar reads. +/// It is a genuine rules axis, not a cosmetic one: Palisade Giant, Ancient +/// Adamantoise and The Wanderer say "other permanents you control", so their own +/// shield must NOT claim damage dealt to the shield host, while Comeuppance, +/// Channel Harm, Blessed Sanctuary and Heroic Sacrifice say "permanents/creatures +/// you control" with no exclusion. +/// +/// CR 614.5 ("a replacement effect gets only one opportunity to affect an +/// event") makes the distinction invisible for a LONE self-recipient shield, but +/// not once a second applicable replacement exists: under CR 616.1 the affected +/// player is offered every applicable replacement, and offering a self-no-op +/// burns the CR 614.5 opportunity on the wrong shield. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(tag = "type")] +pub enum ControlledPermanentsScope { + /// "…and permanents you control" — no exclusion stated; every permanent the + /// scoped player controls, including the ability's own source. + #[default] + IncludingSource, + /// "…and OTHER permanents you control" — the object this ability is printed + /// on is excluded from the permanent leg. + ExcludingSource, +} + +impl ControlledPermanentsScope { + /// True when the permanent leg excludes the ability's own source object. + pub fn excludes_source(self) -> bool { + matches!(self, ControlledPermanentsScope::ExcludingSource) + } +} + /// CR 614.1a: Restricts which damage targets a replacement applies to. /// Dedicated enum because `TargetRef` can be `Player` (not handled by `matches_target_filter`). #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -23445,9 +23548,15 @@ pub enum DamageTargetFilter { /// planeswalkers you control"), `None` for the unrestricted "you/that player /// and permanents they control" (Channel Harm, the opponent-redirect cycle). /// The player leg is always matched regardless of this restriction. + /// + /// CR 109.1: `source_scope` carries the "OTHER" article — Palisade Giant's + /// "you and OTHER permanents you control" excludes the shield host itself + /// from the permanent leg. See [`ControlledPermanentsScope`]. PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope, permanent_type: Option, + #[serde(default)] + source_scope: ControlledPermanentsScope, }, } @@ -23958,14 +24067,21 @@ impl ReplacementDefinition { self } - /// CR 614.9: Mark this replacement as a one-shot redirection shield that - /// re-targets the damage recipient. Consumed on use, expires at cleanup. + /// CR 614.9: Mark this replacement as a redirection shield that re-targets + /// the damage recipient. Expires at cleanup; `lifetime` decides whether it is + /// also consumed by its first event (CR 614.5) or re-applies to every + /// matching event in its window (CR 611.2a). pub fn redirection_shield( mut self, recipient: DamageRedirectTarget, amount: PreventionAmount, + lifetime: RedirectionLifetime, ) -> Self { - self.shield_kind = ShieldKind::Redirection { recipient, amount }; + self.shield_kind = ShieldKind::Redirection { + recipient, + amount, + lifetime, + }; self } diff --git a/crates/engine/tests/integration/heroic_sacrifice_redirect.rs b/crates/engine/tests/integration/heroic_sacrifice_redirect.rs new file mode 100644 index 0000000000..f35ce68d25 --- /dev/null +++ b/crates/engine/tests/integration/heroic_sacrifice_redirect.rs @@ -0,0 +1,243 @@ +//! Runtime regression for the CR 611.2a CONTINUOUS chosen-permanent damage +//! redirection created by a resolving spell (Heroic Sacrifice, Gideon's +//! Sacrifice, and the Saving Grace attachment-host sibling). +//! +//! Heroic Sacrifice was, in sequence: (a) silently misparsed into a card-level +//! `ShieldKind::Prevention { All }` that DELETED the damage, dropped the +//! "creatures you control" victim leg and swallowed its CR 603.7a delayed +//! trigger; then (b) an honest `Effect::Unimplemented` gap. Both left the card a +//! complete no-op — casting it protected nothing. +//! +//! These tests drive the real cast pipeline (`GameRunner::cast(..).resolve()`) +//! followed by the real damage pipeline, so they exercise parser + targeting + +//! resolver + `damage_done_applier` end to end. Every Oracle text here is +//! verbatim Scryfall text. +//! +//! SCOPE / KNOWN GAP (pre-existing, NOT introduced or asserted here): Heroic +//! Sacrifice's third sentence — "When that creature dies this turn, put its +//! counters on up to one target creature you control and draw a card." — is +//! chunked with the "and draw a card" conjunct as a SIBLING of the +//! `CreateDelayedTrigger` rather than inside its payload, so the draw happens on +//! resolution instead of on death. That is identical before and after this +//! change (it is present in the committed `data/card-data.json` baseline) and +//! lives in the clause-assembly/delayed-payload subsystem, not the damage +//! redirection this file covers. These fixtures therefore stock a library so the +//! stray draw is inert, and deliberately assert NOTHING about the draw — pinning +//! the wrong placement would bless it. + +use engine::game::effects::deal_damage; +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::ability::{Effect, QuantityExpr, ResolvedAbility, TargetFilter, TargetRef}; +use engine::types::identifiers::ObjectId; +use engine::types::phase::Phase; + +/// Verbatim Heroic Sacrifice (Scryfall-verified). +const HEROIC_SACRIFICE_TEXT: &str = "Choose target creature you control. Until end of turn, all damage that would be dealt to you and creatures you control is dealt to the chosen creature instead (if it's still on the battlefield). When that creature dies this turn, put its counters on up to one target creature you control and draw a card."; + +/// Verbatim Gideon's Sacrifice (Scryfall-verified) — the inline "this turn" +/// duration spelling with an untyped permanent leg. +const GIDEONS_SACRIFICE_TEXT: &str = "Choose a creature or planeswalker you control. All damage that would be dealt this turn to you and permanents you control is dealt to the chosen permanent instead (if it's still on the battlefield)."; + +/// A non-combat damage source dealing `amount` to `target`, controlled by P1. +fn damage_ability(source_id: ObjectId, target: TargetRef, amount: i32) -> ResolvedAbility { + ResolvedAbility::new( + Effect::DealDamage { + amount: QuantityExpr::Fixed { value: amount }, + target: TargetFilter::Any, + damage_source: None, + excess: None, + }, + vec![target], + source_id, + P1, + ) +} + +/// CR 611.2a + CR 614.9 + CR 614.1a: the whole card, through the production +/// cast pipeline. +/// +/// REVERT GUARDS — each assertion names the axis it pins: +/// * `protector.damage_marked == 3` after event 1 → the parser production and +/// the `ChosenObjectTarget` recipient reading the parent's bound target. With +/// the clause back to `Effect::Unimplemented`, no shield exists at all and P0 +/// simply loses 3 life. +/// * `bystander.damage_marked == 0` / `protector == 7` after event 2 → BOTH the +/// `PlayerOrPermanentsControlledBy` victim conjunct (without it only "you" is +/// protected) AND `RedirectionLifetime::Continuous` (without it the shield was +/// consumed by event 1). +#[test] +fn heroic_sacrifice_redirects_every_event_to_the_chosen_creature_until_end_of_turn() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_library_top(P0, &["Filler A", "Filler B"]); + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Heroic Sacrifice", true, HEROIC_SACRIFICE_TEXT) + .id(); + let protector = scenario.add_creature(P0, "Chosen Creature", 2, 20).id(); + let bystander = scenario.add_creature(P0, "Bystander", 2, 20).id(); + let enemy = scenario.add_creature(P1, "Enemy Creature", 2, 20).id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + + let outcome = runner.cast(spell).target_objects(&[protector]).resolve(); + assert_eq!( + outcome.life_delta(P0), + 0, + "resolving the spell itself changes no life total" + ); + + let life_before = runner.life(P0); + + // Event 1 — the "to you" victim leg. + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("damage to the protected controller resolves"); + assert_eq!( + runner.life(P0), + life_before, + "damage aimed at you must be redirected, not dealt" + ); + assert_eq!( + runner.state().objects[&protector].damage_marked, + 3, + "the chosen creature takes it instead" + ); + + // Event 2 — the "creatures you control" victim leg, on a shield a + // one-opportunity lifetime would already have spent. + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Object(bystander), 4), + &mut events, + ) + .expect("damage to another creature you control resolves"); + assert_eq!( + runner.state().objects[&bystander].damage_marked, + 0, + "the \"and creatures you control\" leg must be protected too" + ); + assert_eq!( + runner.state().objects[&protector].damage_marked, + 7, + "a continuous redirection re-fires for every damage event this turn" + ); + + // Negative: a creature you do NOT control is outside the victim scope. + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Object(enemy), 5), + &mut events, + ) + .expect("damage to an opponent's creature resolves"); + assert_eq!(runner.state().objects[&enemy].damage_marked, 5); + assert_eq!( + runner.state().objects[&protector].damage_marked, + 7, + "an opponent's creature must not be redirected onto the chosen creature" + ); +} + +/// CR 514.2 + CR 611.2a: "Until end of turn" — the shield must be gone next +/// turn. A continuous redirection that outlived its window would silently +/// protect its controller forever. +/// +/// Revert guard: if the shield stopped being a `ShieldKind` (and so stopped +/// being pruned at cleanup), the next-turn event would still redirect and the +/// life total would be untouched. +#[test] +fn heroic_sacrifice_redirect_expires_at_end_of_turn() { + let mut scenario = GameScenario::new(); + // Cast in the END step (CR 513) so advancing to the next turn's upkeep never + // passes through the declare-attackers turn-based action, which would halt + // the scenario driver's priority-passing loop before cleanup. + scenario.at_phase(Phase::End); + scenario.with_library_top(P0, &["Filler A", "Filler B"]); + scenario.with_library_top(P1, &["Filler C", "Filler D"]); + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Heroic Sacrifice", true, HEROIC_SACRIFICE_TEXT) + .id(); + let protector = scenario.add_creature(P0, "Chosen Creature", 2, 20).id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + + runner.cast(spell).target_objects(&[protector]).resolve(); + + // Reach guard: it really does redirect while the window is open. + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("this-turn damage resolves"); + assert_eq!(runner.state().objects[&protector].damage_marked, 3); + + // CR 514.2: cleanup prunes the shield; the next turn is unprotected. + runner.advance_to_phase(Phase::Upkeep); + let life_before = runner.life(P0); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("next-turn damage resolves"); + assert_eq!( + runner.life(P0), + life_before - 3, + "the \"until end of turn\" redirection must not survive cleanup" + ); + // CR 514.2 also removes all damage from permanents during that cleanup step, + // so the chosen creature is back to 0 — and must stay there, proving the + // next-turn event was NOT redirected onto it. + assert_eq!( + runner.state().objects[&protector].damage_marked, + 0, + "the chosen creature takes nothing more after the window closes" + ); +} + +/// CR 611.2a: the sibling spelling. Gideon's Sacrifice carries its duration +/// INLINE ("dealt this turn to …") and its chosen permanent may be a +/// planeswalker, so its victim leg is untyped ("permanents you control"). Same +/// class, same runtime behavior — this is the "build for the class, not the +/// card" guard. +#[test] +fn gideons_sacrifice_untyped_permanent_leg_redirects_onto_the_chosen_permanent() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Gideon's Sacrifice", true, GIDEONS_SACRIFICE_TEXT) + .id(); + let protector = scenario.add_creature(P0, "Chosen Creature", 2, 20).id(); + let artifact = scenario + .add_creature(P0, "Bystanding Artifact", 0, 0) + .as_artifact() + .id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + + runner.cast(spell).target_objects(&[protector]).resolve(); + + // The UNTYPED permanent leg: a noncreature permanent you control is covered + // here, where Heroic Sacrifice's "creatures you control" would not cover it. + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Object(artifact), 3), + &mut events, + ) + .expect("damage to a noncreature permanent you control resolves"); + assert_eq!( + runner.state().objects[&artifact].damage_marked, + 0, + "an untyped permanent leg must cover noncreature permanents" + ); + assert_eq!(runner.state().objects[&protector].damage_marked, 3); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index e59c831416..13b53964dc 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -289,6 +289,7 @@ mod hawkeye_avenging_archer_dealt_damage_draw; mod heist_production_path_handoff; mod hellkite_tyrant_steal_artifacts_2906; mod heroic_defiance_recipient_color_4590; +mod heroic_sacrifice_redirect; mod hit_the_mother_lode; mod hogaak_cant_spend_mana_1095; mod hollow_one_cost_reduction; @@ -890,6 +891,7 @@ mod ozolith_leaves_battlefield_counters; mod painters_servant_multi_zone_additive_color; mod palisade_giant_redirect; mod panther_habit_equipped_prevention_scope; +mod pariah_attached_redirect; mod pass_priority_structural_legality; mod peer_into_the_abyss; mod peerless_recycling_gift_recipient; diff --git a/crates/engine/tests/integration/oracle_parser.rs b/crates/engine/tests/integration/oracle_parser.rs index d0b163c46d..9d370639b1 100644 --- a/crates/engine/tests/integration/oracle_parser.rs +++ b/crates/engine/tests/integration/oracle_parser.rs @@ -1,8 +1,8 @@ use engine::parser::oracle::{keyword_display_name, parse_oracle_text}; use engine::types::ability::{ - ChosenSubtypeKind, ContinuousModification, ControllerRef, DamageModification, - DamageTargetFilter, DamageTargetPlayerScope, Effect, FilterProp, StaticCondition, TargetFilter, - TypeFilter, + ChosenSubtypeKind, ContinuousModification, ControlledPermanentsScope, ControllerRef, + DamageModification, DamageTargetFilter, DamageTargetPlayerScope, Effect, FilterProp, + StaticCondition, TargetFilter, TypeFilter, }; use engine::types::keywords::Keyword; use engine::types::statics::StaticMode; @@ -521,6 +521,7 @@ fn sawhorn_nemesis_damage_replacement_scopes_to_source_chosen_player() { Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::SourceChosenPlayer, permanent_type: None, + source_scope: ControlledPermanentsScope::IncludingSource, }) ); } diff --git a/crates/engine/tests/integration/palisade_giant_redirect.rs b/crates/engine/tests/integration/palisade_giant_redirect.rs index 10302c8e34..1de77a7291 100644 --- a/crates/engine/tests/integration/palisade_giant_redirect.rs +++ b/crates/engine/tests/integration/palisade_giant_redirect.rs @@ -19,6 +19,7 @@ use engine::game::sba::check_state_based_actions; use engine::game::scenario::{GameScenario, P0, P1}; use engine::game::triggers::process_triggers; use engine::types::ability::{Effect, QuantityExpr, ResolvedAbility, TargetFilter, TargetRef}; +use engine::types::game_state::WaitingFor; use engine::types::identifiers::ObjectId; use engine::types::phase::Phase; use engine::types::triggers::TriggerMode; @@ -26,9 +27,11 @@ use engine::types::zones::Zone; use super::rules::run_combat; -/// Verbatim Palisade Giant redirection text (Scryfall-verified). The recipient- -/// list half ("and other permanents you control") is a separate, pre-existing -/// gap and is not exercised here. +/// Verbatim Palisade Giant redirection text (Scryfall-verified). Byte-identical +/// to Ancient Adamantoise's redirection line, so both cards are covered by the +/// same parser arm. The conjunct victim scope ("and other permanents you +/// control") is exercised by +/// `palisade_giant_redirects_damage_dealt_to_your_other_permanents`. const PALISADE_GIANT_TEXT: &str = "All damage that would be dealt to you and other permanents you control is dealt to this creature instead."; @@ -269,6 +272,198 @@ fn destroyed_palisade_giant_neither_prevents_nor_redirects() { ); } +/// CR 614.9 + CR 614.1a: the CONJUNCT victim scope. Palisade Giant's line reads +/// "to you AND OTHER PERMANENTS YOU CONTROL", so damage that would be dealt to +/// another permanent you control redirects onto the Giant too — not just damage +/// aimed at you. Before the `PlayerOrPermanentsControlledBy` victim arm the +/// parser collapsed the conjunct to `Player { Controller }` and the permanent leg +/// was silently unprotected. +/// +/// Revert guard: with the victim arm reverted, `bystander.damage_marked` is 3 and +/// the Giant's is 0 — both assertions below flip. +#[test] +fn palisade_giant_redirects_damage_dealt_to_your_other_permanents() { + let mut scenario = GameScenario::new(); + let giant = scenario + .add_creature_from_oracle(P0, "Palisade Giant", 2, 20, PALISADE_GIANT_TEXT) + .id(); + let bystander = scenario.add_creature(P0, "Grizzly Bears", 2, 2).id(); + // Negative control: an opponent's creature is NOT covered by "you control". + let enemy = scenario.add_creature(P1, "Hill Giant", 3, 3).id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Object(bystander), 3), + &mut events, + ) + .expect("damage to another permanent you control resolves"); + + assert_eq!( + runner.state().objects[&giant].damage_marked, + 3, + "damage aimed at another permanent you control must redirect onto Palisade Giant" + ); + assert_eq!( + runner.state().objects[&bystander].damage_marked, + 0, + "the protected permanent takes none of it" + ); + + // Negative: an opponent's creature is outside the victim scope entirely. + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Object(enemy), 3), + &mut events, + ) + .expect("damage to an opponent's creature resolves"); + assert_eq!( + runner.state().objects[&enemy].damage_marked, + 3, + "damage to a permanent you do NOT control is untouched by the shield" + ); + assert_eq!( + runner.state().objects[&giant].damage_marked, + 3, + "the shield must not have redirected the opponent's creature's damage onto the Giant" + ); +} + +/// CR 109.1 + CR 614.5: Damage dealt DIRECTLY to a LONE Palisade Giant is +/// outside its own victim scope — its text says "you and OTHER permanents you +/// control", so the shield host is excluded from the permanent leg. The damage +/// is marked exactly once: neither doubled by re-entry nor deleted. +/// +/// With a single shield this outcome is the same either way (CR 614.5 already +/// stops re-entry), which is exactly why the single-shield case cannot pin the +/// exclusion — `two_shields_...` below is the discriminating fixture. +#[test] +fn palisade_giant_self_damage_is_marked_once_not_doubled_or_prevented() { + let mut scenario = GameScenario::new(); + let giant = scenario + .add_creature_from_oracle(P0, "Palisade Giant", 2, 20, PALISADE_GIANT_TEXT) + .id(); + let source = scenario.add_creature(P1, "Damage Source", 5, 5).id(); + let mut runner = scenario.build(); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Object(giant), 5), + &mut events, + ) + .expect("damage dealt directly to the shield host resolves"); + + assert_eq!( + runner.state().objects[&giant].damage_marked, + 5, + "self-directed damage is marked exactly once — not 10 (re-entry) and not 0 (prevented)" + ); +} + +/// CR 109.1 + CR 614.1a + CR 616.1: THE discriminating fixture for the `"other "` +/// self-exclusion token. +/// +/// Two Palisade Giants under the same controller (Ancient Adamantoise carries a +/// byte-identical redirection line). Damage aimed at Giant A is: +/// * OUTSIDE Giant A's own victim scope — A's text protects "you and OTHER +/// permanents you control", and A is not other than A; +/// * INSIDE Giant B's victim scope — A is another permanent that player controls. +/// +/// So exactly ONE replacement is applicable to the announced event: no CR 616.1 +/// choice is offered. B's shield moves the damage to B; the modified event is +/// then inside A's scope (B is an other permanent), so A's shield — which has not +/// yet had its CR 614.5 opportunity — moves it back to A. Both shields are now +/// spent, and A takes the 3. That ping-pong is the rules-correct outcome, not an +/// engine artifact. +/// +/// REVERT GUARD: with the `"other "` token discarded (an `opt()` whose value is +/// thrown away, or a `DamageTargetFilter` with no exclusion axis), BOTH shields +/// are applicable to the ANNOUNCED event, so the affected player is handed a +/// CR 616.1 choice and the pipeline halts in `WaitingFor::ReplacementChoice` with +/// no damage marked anywhere. Every assertion below flips. +#[test] +fn two_shields_exclude_their_own_host_so_no_self_no_op_choice_is_offered() { + let mut scenario = GameScenario::new(); + let giant_a = scenario + .add_creature_from_oracle(P0, "Palisade Giant", 2, 20, PALISADE_GIANT_TEXT) + .id(); + let giant_b = scenario + .add_creature_from_oracle(P0, "Ancient Adamantoise", 2, 20, PALISADE_GIANT_TEXT) + .id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Object(giant_a), 3), + &mut events, + ) + .expect("damage dealt to one of two shield hosts resolves"); + + assert!( + !matches!( + runner.state().waiting_for, + WaitingFor::ReplacementChoice { .. } + ), + "only ONE shield may be applicable to the announced event — a self-no-op must not be \ + offered as a CR 616.1 candidate, got {:?}", + runner.state().waiting_for + ); + // Reach guard: the event really did travel through the OTHER host's shield + // rather than never being replaced at all. + let applied: Vec = events + .iter() + .filter_map(|e| match e { + engine::types::events::GameEvent::ReplacementApplied { source_id, .. } => { + Some(*source_id) + } + _ => None, + }) + .collect(); + assert_eq!( + applied, + vec![giant_b, giant_a], + "CR 616.1 + CR 614.5: the OTHER host's shield applies first, then the damaged host's \ + shield applies to the modified event" + ); + assert_eq!( + runner.state().objects[&giant_a].damage_marked, + 3, + "after both single-use opportunities the damage lands back on Giant A" + ); + assert_eq!( + runner.state().objects[&giant_b].damage_marked, + 0, + "Giant B only passed the damage along; it does not keep it" + ); + + // Positive reach-guard: the same board DOES offer a CR 616.1 choice when both + // shields are applicable to the ANNOUNCED event — damage aimed at the + // controller is inside both victim scopes (the player leg carries no + // exclusion). This proves the assertion above measures the exclusion axis and + // not a board where choices never arise. + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("damage to the shared controller resolves"); + assert!( + matches!( + runner.state().waiting_for, + WaitingFor::ReplacementChoice { .. } + ), + "two shields applicable to the PLAYER leg must still surface a CR 616.1 choice, got {:?}", + runner.state().waiting_for + ); +} + /// CR 510.2 + CR 614.9: Simultaneous multi-attacker combat damage — two unblocked /// attackers deal combat damage to the same protected controller in one combat /// damage step. Every attacker's damage must redirect onto the Bodyguard (summed diff --git a/crates/engine/tests/integration/pariah_attached_redirect.rs b/crates/engine/tests/integration/pariah_attached_redirect.rs new file mode 100644 index 0000000000..fe87db4b6c --- /dev/null +++ b/crates/engine/tests/integration/pariah_attached_redirect.rs @@ -0,0 +1,477 @@ +//! Runtime regression for the ATTACHMENT-HOST recipient of a durable damage +//! redirection (CR 614.9 + CR 303.4b + CR 301.5a): "All damage that would be +//! dealt to you is dealt to enchanted creature instead." (Pariah, With Great +//! Power . . .) and its Equipment sibling "…to equipped creature instead." +//! (Pariah's Shield). +//! +//! Before the anchored redirection spine these three cards parsed to a bare +//! `ShieldKind::Prevention { All }` with NO redirect destination — a CR 615 +//! prevention that DELETED the damage instead of moving it, while +//! `coverage-data.json` reported them fully supported. These tests drive the +//! real `deal_damage` pipeline and fail if either the parser's +//! `TargetFilter::AttachedTo` recipient or the runtime's +//! `DamageRedirectTarget::AttachedToSource` arm is reverted: the host's +//! `damage_marked` stays 0 and the controller's life is untouched (damage +//! vanishing), instead of the host taking it. +//! +//! All Oracle text is verbatim / Scryfall-verified. + +use engine::game::effects::attach::attach_to; +use engine::game::effects::deal_damage; +use engine::game::game_object::AttachTarget; +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::types::ability::{Effect, QuantityExpr, ResolvedAbility, TargetFilter, TargetRef}; +use engine::types::actions::GameAction; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; + +/// Verbatim Pariah (Aura) — the enchanted creature is the recipient. +const PARIAH_TEXT: &str = "Enchant creature\nAll damage that would be dealt to you is dealt to enchanted creature instead."; + +/// Verbatim Pariah's Shield (Equipment) — the equipped creature is the recipient. +const PARIAHS_SHIELD_TEXT: &str = + "All damage that would be dealt to you is dealt to equipped creature instead.\nEquip {3}"; + +/// Verbatim With Great Power . . . (Aura) — same recipient class as Pariah, used +/// as the second authority in the multi-Aura provenance fixture. +const WITH_GREAT_POWER_TEXT: &str = "Enchant creature you control\nEnchanted creature gets +2/+2 for each Aura and Equipment attached to it.\nAll damage that would be dealt to you is dealt to enchanted creature instead."; + +/// Verbatim Empyrial Archangel — the `~`-recipient sibling, used as the negative +/// control proving the new `AttachedToSource` arm did not capture `SelfRef`. +const EMPYRIAL_ARCHANGEL_TEXT: &str = + "All damage that would be dealt to you is dealt to this creature instead."; + +/// Verbatim Treacherous Link — "…is dealt to its controller instead", a recipient +/// the durable grammar deliberately does not support. It must now DECLINE rather +/// than install a global prevention field. +const TREACHEROUS_LINK_TEXT: &str = "Enchant creature\nAll damage that would be dealt to enchanted creature is dealt to its controller instead."; + +/// A non-combat damage source dealing `amount` to `target`, controlled by P1. +fn damage_ability(source_id: ObjectId, target: TargetRef, amount: i32) -> ResolvedAbility { + ResolvedAbility::new( + Effect::DealDamage { + amount: QuantityExpr::Fixed { value: amount }, + target: TargetFilter::Any, + damage_source: None, + excess: None, + }, + vec![target], + source_id, + P1, + ) +} + +/// CR 701.3a/b/c + CR 613.7e: Attach through the engine's SINGLE ATTACH +/// AUTHORITY, `effects::attach::attach_to`, rather than hand-wiring +/// `attached_to` / `attachments`. +/// +/// That matters here because `attach_to` does three things a hand-wired fixture +/// silently skips: it runs the CR 701.3b legality gate, it bumps the CR 613.7e +/// timestamp on a real host transition, and it detaches from any previous host. +/// A future change that makes redirection depend on attachment legality or +/// timestamp would pass a hand-wired fixture while breaking real games. +/// +/// `attach_to` returns `None` when the gate refuses, which would leave the +/// fixture silently unattached and turn every downstream assertion vacuous, so +/// the gate result is asserted rather than discarded. (Not +/// `attach_as_bestowed_aura` — Pariah is a printed Aura, not a bestowed +/// creature.) +fn attach(runner: &mut GameRunner, attachment: ObjectId, host: ObjectId) { + attach_to(runner.state_mut(), attachment, host); + assert_eq!( + runner.state().objects[&attachment].attached_to, + Some(AttachTarget::Object(host)), + "the production attach authority must have wired the host (CR 701.3b gate refused?)" + ); + assert!( + runner.state().objects[&host] + .attachments + .contains(&attachment), + "the host must record the attachment" + ); +} + +/// CR 614.9 + CR 303.4b: Pariah redirects damage that would be dealt to its +/// controller onto the ENCHANTED CREATURE. +#[test] +fn pariah_redirects_controller_damage_onto_the_enchanted_creature() { + let mut scenario = GameScenario::new(); + let host = scenario.add_creature(P0, "Wall of Blossoms", 0, 20).id(); + let aura = scenario + .add_creature(P0, "Pariah", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Aura"]) + .from_oracle_text(PARIAH_TEXT) + .id(); + let source = scenario.add_creature(P1, "Damage Source", 6, 6).id(); + let mut runner = scenario.build(); + attach(&mut runner, aura, host); + + let p0_life_before = runner.life(P0); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 6), + &mut events, + ) + .expect("damage to Pariah's controller resolves"); + + assert_eq!( + runner.state().objects[&host].damage_marked, + 6, + "the damage must be redirected onto the enchanted creature, not deleted" + ); + assert_eq!( + runner.life(P0), + p0_life_before, + "the controller takes none of it — redirected, not dealt twice" + ); + assert_eq!( + runner.state().objects[&aura].damage_marked, + 0, + "the Aura itself is not the recipient" + ); +} + +/// CR 614.9 + CR 301.5a: the Equipment sibling. Same arm, "equipped creature". +#[test] +fn pariahs_shield_redirects_controller_damage_onto_the_equipped_creature() { + let mut scenario = GameScenario::new(); + let host = scenario.add_creature(P0, "Wall of Blossoms", 0, 20).id(); + let equipment = scenario + .add_creature(P0, "Pariah's Shield", 0, 0) + .as_artifact() + .with_subtypes(vec!["Equipment"]) + .from_oracle_text(PARIAHS_SHIELD_TEXT) + .id(); + let source = scenario.add_creature(P1, "Damage Source", 4, 4).id(); + let mut runner = scenario.build(); + attach(&mut runner, equipment, host); + + let p0_life_before = runner.life(P0); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 4), + &mut events, + ) + .expect("damage to Pariah's Shield's controller resolves"); + + assert_eq!(runner.state().objects[&host].damage_marked, 4); + assert_eq!(runner.life(P0), p0_life_before); +} + +/// NEGATIVE SIBLING: the `~`-recipient class must be unaffected — the new +/// attachment arm must not have captured `TargetFilter::SelfRef`. Empyrial +/// Archangel still marks ITSELF, and a co-existing Pariah in the same fixture +/// still marks its own host. +#[test] +fn self_recipient_sibling_still_redirects_onto_itself() { + let mut scenario = GameScenario::new(); + let archangel = scenario + .add_creature_from_oracle(P0, "Empyrial Archangel", 5, 20, EMPYRIAL_ARCHANGEL_TEXT) + .id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + + let p0_life_before = runner.life(P0); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("damage to the Archangel's controller resolves"); + + assert_eq!( + runner.state().objects[&archangel].damage_marked, + 3, + "a `~` recipient still redirects onto the shield host itself" + ); + assert_eq!(runner.life(P0), p0_life_before); +} + +/// CR 614.9 IDENTITY/PROVENANCE: the recipient is LIVE, not latched at install. +/// Re-pointing the Aura from host A to host B between two damage events must move +/// the redirect with it. +/// +/// Revert guard: a latched recipient would mark host A both times. +#[test] +fn attachment_recipient_is_live_not_latched() { + let mut scenario = GameScenario::new(); + let host_a = scenario.add_creature(P0, "Host A", 0, 20).id(); + let host_b = scenario.add_creature(P0, "Host B", 0, 20).id(); + let aura = scenario + .add_creature(P0, "Pariah", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Aura"]) + .from_oracle_text(PARIAH_TEXT) + .id(); + let source = scenario.add_creature(P1, "Damage Source", 2, 2).id(); + let mut runner = scenario.build(); + attach(&mut runner, aura, host_a); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 2), + &mut events, + ) + .expect("first damage event resolves"); + assert_eq!(runner.state().objects[&host_a].damage_marked, 2); + + // Move the Aura. CR 701.3a: `attach_to` performs the detach from the old + // host itself, so the fixture no longer re-implements it inline. + attach(&mut runner, aura, host_b); + assert!( + !runner.state().objects[&host_a].attachments.contains(&aura), + "the production attach authority must have detached the old host" + ); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 5), + &mut events, + ) + .expect("second damage event resolves"); + + assert_eq!( + runner.state().objects[&host_b].damage_marked, + 5, + "the second event must follow the Aura to its new host" + ); + assert_eq!( + runner.state().objects[&host_a].damage_marked, + 2, + "the old host keeps only the first event's damage" + ); +} + +/// CR 614.9 + CR 616.1 MULTI-AUTHORITY / PROVENANCE: two Auras of the same class +/// on DIFFERENT hosts, both controlled by you, both applicable to the same +/// player-damage event. The affected player chooses which replacement to apply +/// (`WaitingFor::ReplacementChoice` → `GameAction::ChooseReplacement`), and the +/// recipient must bind to the CHOSEN replacement's own `ReplacementId` source — +/// its own host — not to a per-controller value shared by both shields. +/// +/// This is the production `GameAction` route, not a helper call: whichever Aura +/// is selected, ONLY that Aura's host is marked. A per-controller or latched +/// recipient would mark the same host for both selections, flipping one of the +/// two loop iterations. +#[test] +fn chosen_aura_binds_its_own_host_not_a_shared_controller_value() { + for choose_pariah in [true, false] { + let mut scenario = GameScenario::new(); + let host_a = scenario.add_creature(P0, "Host A", 0, 20).id(); + let host_b = scenario.add_creature(P0, "Host B", 0, 20).id(); + let pariah = scenario + .add_creature(P0, "Pariah", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Aura"]) + .from_oracle_text(PARIAH_TEXT) + .id(); + let great_power = scenario + .add_creature(P0, "With Great Power . . .", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Aura"]) + .from_oracle_text(WITH_GREAT_POWER_TEXT) + .id(); + let source = scenario.add_creature(P1, "Damage Source", 7, 7).id(); + let mut runner = scenario.build(); + attach(&mut runner, pariah, host_a); + attach(&mut runner, great_power, host_b); + + let p0_life_before = runner.life(P0); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 7), + &mut events, + ) + .expect("damage to the shared controller resolves"); + + // Reach guard: two applicable shields really do surface a choice. + let WaitingFor::ReplacementChoice { candidates, .. } = runner.state().waiting_for.clone() + else { + panic!( + "two applicable redirection shields must surface a ReplacementChoice, got {:?}", + runner.state().waiting_for + ); + }; + let wanted = if choose_pariah { pariah } else { great_power }; + let index = candidates + .iter() + .position(|c| c.source_id == wanted) + .expect("both Auras must be offered as candidates"); + + runner + .act(GameAction::ChooseReplacement { index }) + .expect("applying the chosen redirection replacement must succeed"); + + let (expected_host, other_host) = if choose_pariah { + (host_a, host_b) + } else { + (host_b, host_a) + }; + assert_eq!( + runner.state().objects[&expected_host].damage_marked, + 7, + "the CHOSEN Aura's own host must take the damage (choose_pariah={choose_pariah})" + ); + assert_eq!( + runner.state().objects[&other_host].damage_marked, + 0, + "the other Aura's host must be untouched (choose_pariah={choose_pariah})" + ); + assert_eq!( + runner.life(P0), + p0_life_before, + "the controller takes none of it" + ); + // Reach guard: neither Aura marks itself. + assert_eq!(runner.state().objects[&pariah].damage_marked, 0); + assert_eq!(runner.state().objects[&great_power].damage_marked, 0); + } +} + +/// CR 614.9: "If one of those permanents is no longer on the battlefield … the +/// effect does nothing." An UNATTACHED Aura has no host at all, so the redirect +/// does nothing and the damage stays on the original recipient — it is neither +/// prevented nor vanished. +#[test] +fn unattached_aura_redirect_does_nothing_and_damage_hits_the_controller() { + let mut scenario = GameScenario::new(); + let host = scenario.add_creature(P0, "Wall of Blossoms", 0, 20).id(); + let aura = scenario + .add_creature(P0, "Pariah", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Aura"]) + .from_oracle_text(PARIAH_TEXT) + .id(); + let source = scenario.add_creature(P1, "Damage Source", 4, 4).id(); + let mut runner = scenario.build(); + // Deliberately NOT attached. + + let p0_life_before = runner.life(P0); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 4), + &mut events, + ) + .expect("damage to the controller resolves"); + + assert_eq!( + runner.life(P0), + p0_life_before - 4, + "with no attachment host the redirection does nothing — the damage is NOT prevented" + ); + assert_eq!(runner.state().objects[&host].damage_marked, 0); + assert_eq!(runner.state().objects[&aura].damage_marked, 0); +} + +/// CR 614.9: "…or is no longer a battle, creature, or planeswalker when the +/// damage would be redirected, the effect does nothing." The host stays on the +/// battlefield but loses its creature core type — an illegal recipient. Mirrors +/// `palisade_giant_illegal_recipient_makes_redirect_do_nothing`. +#[test] +fn illegal_attachment_host_makes_redirect_do_nothing() { + let mut scenario = GameScenario::new(); + let host = scenario.add_creature(P0, "Wall of Blossoms", 0, 20).id(); + let aura = scenario + .add_creature(P0, "Pariah", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Aura"]) + .from_oracle_text(PARIAH_TEXT) + .id(); + let source = scenario.add_creature(P1, "Damage Source", 4, 4).id(); + let mut runner = scenario.build(); + attach(&mut runner, aura, host); + runner + .state_mut() + .objects + .get_mut(&host) + .unwrap() + .card_types + .core_types = vec![]; + + let p0_life_before = runner.life(P0); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 4), + &mut events, + ) + .expect("damage to the controller resolves"); + + assert_eq!( + runner.life(P0), + p0_life_before - 4, + "an illegal recipient makes the redirection do nothing — damage is not prevented" + ); + assert_eq!(runner.state().objects[&host].damage_marked, 0); +} + +/// The live-bug guard. Treacherous Link's "…is dealt to its controller instead" +/// recipient is unsupported, and the handler used to emit +/// `Prevention { All }` with `damage_target_filter: null` — a shield matching +/// EVERY damage event to every object and player in the game. With the anchored +/// spine it declines outright, so unrelated damage lands normally. +/// +/// Revert guard: with the old handler, the first assertion fails (the unrelated +/// player takes no damage at all). +#[test] +fn treacherous_link_no_longer_installs_a_global_damage_prevention_field() { + let mut scenario = GameScenario::new(); + let host = scenario.add_creature(P1, "Enchanted Bear", 2, 20).id(); + let aura = scenario + .add_creature(P0, "Treacherous Link", 0, 0) + .as_enchantment() + .with_subtypes(vec!["Aura"]) + .from_oracle_text(TREACHEROUS_LINK_TEXT) + .id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + attach(&mut runner, aura, host); + + let p0_life_before = runner.life(P0); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("unrelated damage resolves"); + + assert_eq!( + runner.life(P0), + p0_life_before - 3, + "an unrelated damage event must still be dealt — no global prevention field" + ); + + // Positive reach-guard: the same board WITHOUT Treacherous Link produces an + // identical life delta, proving the assertion is not measuring a no-op board. + let mut scenario = GameScenario::new(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut control = scenario.build(); + let control_life_before = control.life(P0); + let mut events = Vec::new(); + deal_damage::resolve( + control.state_mut(), + &damage_ability(source, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("control damage resolves"); + assert_eq!( + control_life_before - control.life(P0), + p0_life_before - runner.life(P0), + "the life delta must match the Treacherous-Link-free control board" + ); +} From 4f48e4f38f73ec45c61f7301c0dde4580495349b Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 16 Aug 2026 12:59:25 -0500 Subject: [PATCH 2/3] fix(PR-7408): address review comments on the redirection spine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blockers - CR 615.12 no longer suppresses CR 614.9 durable redirections. A Prevention-shaped shield carrying a redirect_target prevents nothing (CR 615.1a: prevention effects use the word "prevent"; this grammar never does), so Pariah, Pariah's Shield, With Great Power . . ., Palisade Giant and Ancient Adamantoise stopped redirecting under any "damage can't be prevented" effect. Introduces PreventionShieldRoute as the single authority consulted by BOTH the suppression gate (is_damage_prevention_replacement) and the apply-time route (damage_done_applier Branch 2), so the two cannot disagree — a gate keyed only on redirect_target.is_some() would exempt an AllBut/Next shield that still behaves as a prevention. - Removes ControlledPermanentsScope, which duplicated the existing SourceExclusion axis. Both source_scope fields now use SourceExclusion with skip_serializing_if, so the default is no longer emitted and no shipped card's serialized data changes. Review findings - durable_redirect_recipient returns a typed 3-way route and FAILS CLOSED for an unmapped recipient in every build profile; the old debug-only assert let release builds degrade a redirection into a prevention and delete the damage. Its totality test now drives the real parse_durable_redirect_recipient_filter grammar instead of restating the mapping's own arms back to it. - Shield description derives from redirect_lifetime, so a Continuous shield no longer renders as "One-shot damage replacement" in a CR 616.1 prompt. - Guards PreventionAmount::Next against RedirectionLifetime::Continuous (CR 614.5: the amount would never deplete). - Adds a positive reach guard to the Palisade Giant self-damage fixture, whose sole assertion also held when no shield existed. - Shares the damage_ability fixture via integration::rules instead of three byte-identical private copies. - Corrects the attach_to doc comment. The suggested assert on attach_to(..).is_some() is wrong: the function returns the PREVIOUS host, so a first attach returns None on success and the assertion failed all six attachment fixtures. The wired-state assertions are what discriminate. Kept deliberately: the CR 109.1 citation on the "other" article. No CR rule defines the other/another qualifier, but main already cites CR 109.1 as the identity foundation for "another" exclusions in eleven places, so the annotation follows the existing convention with an explicit caveat rather than fragmenting it. Flagged on the PR for a repo-wide decision. Tests - restriction_does_not_block_durable_redirect_shields — the redirect survives an active DamagePreventionDisabled and actually moves the damage. Verified discriminating: fails with the pre-fix classification restored. - unmapped_durable_redirect_recipient_fails_closed_instead_of_preventing — returns Prevented if the fail-closed branch is removed. Co-Authored-By: Claude Opus 5 --- crates/engine/src/game/coverage.rs | 2 +- .../game/effects/add_target_replacement.rs | 15 +- .../game/effects/create_damage_replacement.rs | 23 +- crates/engine/src/game/replacement.rs | 444 ++++++++++++++---- .../src/parser/oracle_effect/imperative.rs | 8 +- crates/engine/src/parser/oracle_nom/filter.rs | 14 +- .../engine/src/parser/oracle_replacement.rs | 50 +- crates/engine/src/types/ability.rs | 64 +-- .../integration/heroic_sacrifice_redirect.rs | 19 +- .../engine/tests/integration/oracle_parser.rs | 6 +- .../integration/palisade_giant_redirect.rs | 50 +- .../integration/pariah_attached_redirect.rs | 28 +- crates/engine/tests/integration/rules.rs | 23 + 13 files changed, 503 insertions(+), 243 deletions(-) diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index f6194b3cb3..d6983e94ec 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -631,7 +631,7 @@ fn fmt_target(filter: &TargetFilter) -> String { permanent_type, source_scope, } => { - let other = if source_scope.excludes_source() { + let other = if source_scope.is_exclude() { "other " } else { "" diff --git a/crates/engine/src/game/effects/add_target_replacement.rs b/crates/engine/src/game/effects/add_target_replacement.rs index 29a948bac8..d3cd9162bc 100644 --- a/crates/engine/src/game/effects/add_target_replacement.rs +++ b/crates/engine/src/game/effects/add_target_replacement.rs @@ -1,8 +1,8 @@ use crate::game::targeting::{extract_source_from_event, resolve_event_context_target}; use crate::types::ability::{ - AbilityDefinition, ControlledPermanentsScope, DamageTargetFilter, DamageTargetPlayerScope, - Duration, Effect, EffectError, EffectKind, ReplacementCondition, ReplacementDefinition, - ResolvedAbility, RestrictionExpiry, TargetFilter, TargetRef, + AbilityDefinition, DamageTargetFilter, DamageTargetPlayerScope, Duration, Effect, EffectError, + EffectKind, ReplacementCondition, ReplacementDefinition, ResolvedAbility, RestrictionExpiry, + SourceExclusion, TargetFilter, TargetRef, }; use crate::types::events::GameEvent; use crate::types::game_state::GameState; @@ -388,7 +388,7 @@ pub fn resolve( // CR 109.1: no "other" article in this class — // the granted shield covers every permanent the // targeted player controls. - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }); } state.pending_damage_replacements.push(replacement); @@ -415,8 +415,9 @@ mod tests { use crate::game::replacement::{replace_event, ReplacementResult}; use crate::game::zones::create_object; use crate::types::ability::{ - AbilityDefinition, ControlledPermanentsScope, DamageModification, DamageTargetPlayerScope, - Duration, ReplacementDefinition, RestrictionExpiry, TargetFilter, TypeFilter, TypedFilter, + AbilityDefinition, DamageModification, DamageTargetPlayerScope, Duration, + ReplacementDefinition, RestrictionExpiry, SourceExclusion, TargetFilter, TypeFilter, + TypedFilter, }; use crate::types::identifiers::{CardId, ObjectId}; use crate::types::player::PlayerId; @@ -666,7 +667,7 @@ mod tests { Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Specific(PlayerId(1)), permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }) ); assert_eq!( diff --git a/crates/engine/src/game/effects/create_damage_replacement.rs b/crates/engine/src/game/effects/create_damage_replacement.rs index ec3b71a29a..ba7782b394 100644 --- a/crates/engine/src/game/effects/create_damage_replacement.rs +++ b/crates/engine/src/game/effects/create_damage_replacement.rs @@ -2,8 +2,8 @@ use crate::game::effects::choose_damage_source; use crate::game::effects::prevent_damage::resolve_source_filter; use crate::game::game_object::AttachTarget; use crate::types::ability::{ - DamageRedirectTarget, Effect, EffectError, EffectKind, PreventionAmount, ReplacementDefinition, - ResolvedAbility, TargetFilter, TargetRef, + DamageRedirectTarget, Effect, EffectError, EffectKind, PreventionAmount, RedirectionLifetime, + ReplacementDefinition, ResolvedAbility, TargetFilter, TargetRef, }; use crate::types::card_type::CoreType; use crate::types::events::GameEvent; @@ -140,8 +140,17 @@ pub fn resolve( other => other.clone(), }; + // CR 614.5 vs CR 611.2a: label the shield by its actual lifetime. + // `replacement_choice_label` falls back to `description` for any shield whose + // `execute` shape it doesn't recognize, so a CR 616.1 ordering prompt renders + // this string verbatim — a `Continuous` shield (Heroic Sacrifice, Gideon's + // Sacrifice) must not announce itself as one-shot. + let description = match redirect_lifetime { + RedirectionLifetime::OneOpportunity => "One-shot damage replacement", + RedirectionLifetime::Continuous => "Continuous damage replacement", + }; let mut shield = ReplacementDefinition::new(ReplacementEvent::DamageDone) - .description("One-shot damage replacement".to_string()); + .description(description.to_string()); // CR 614.1a: Match filters — which damage source / recipient / kind this // one-shot replaces. SelfRef ("it"/"~"/"this creature") matches the host; @@ -375,8 +384,8 @@ mod tests { use crate::game::effects::deal_damage; use crate::game::zones::create_object; use crate::types::ability::{ - ControlledPermanentsScope, DamageModification, DamageTargetFilter, DamageTargetPlayerScope, - RedirectionLifetime, ShieldKind, TargetFilter, + DamageModification, DamageTargetFilter, DamageTargetPlayerScope, RedirectionLifetime, + ShieldKind, SourceExclusion, TargetFilter, }; use crate::types::card_type::CoreType; use crate::types::identifiers::CardId; @@ -672,7 +681,7 @@ mod tests { target_filter: Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Controller, permanent_type: Some(CoreType::Creature), - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }), modification: None, redirect_to: Some(DamageRedirectTarget::ChosenObjectTarget), @@ -820,7 +829,7 @@ mod tests { target_filter: Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Controller, permanent_type: Some(CoreType::Creature), - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }), modification: None, redirect_to: Some(DamageRedirectTarget::ChosenObjectTarget), diff --git a/crates/engine/src/game/replacement.rs b/crates/engine/src/game/replacement.rs index 4a2073db68..0a429691ec 100644 --- a/crates/engine/src/game/replacement.rs +++ b/crates/engine/src/game/replacement.rs @@ -1823,22 +1823,34 @@ fn redirect_chosen_object_for_rid(state: &GameState, rid: ReplacementId) -> Opti } } -/// CR 614.9: Read back the `redirect_target` filter stored on the matched -/// replacement. Mirrors `redirect_chosen_object_for_rid`'s repl-lookup shape, -/// but `TargetFilter` (unlike the `Copy` `ShieldKind`) is not `Copy`, so the -/// stored filter is cloned. Used by the continuous `ShieldKind::Prevention` -/// path to detect a `TargetFilter::SelfRef` redirect (Palisade Giant, Veteran -/// Bodyguard, Weathered Bodyguards) and reuse the shared redirection mechanics. -fn redirect_target_for_rid(state: &GameState, rid: ReplacementId) -> Option { - let repl = if rid.source == ObjectId(0) { - state.pending_damage_replacements.get(rid.index) - } else { - state - .objects - .get(&rid.source) - .and_then(|obj| obj.replacement_definitions.get(rid.index)) - }; - repl.and_then(|repl| repl.redirect_target.clone()) +/// CR 614.9 vs CR 615.1a: what a `ShieldKind::Prevention` shield actually does. +/// +/// The durable redirection spine stores its recipient in +/// `ReplacementDefinition::redirect_target` on a shield whose `ShieldKind` is +/// still `Prevention`, so "does this shield prevent?" is NOT answerable from the +/// shield kind alone. This enum is the answer, and [`prevention_shield_route`] +/// is its single authority — consulted by BOTH the CR 615.12 suppression gate +/// (`is_damage_prevention_replacement`) and the apply-time route +/// (`damage_done_applier`'s Branch 2), so the two can never disagree about +/// whether a given shield prevents or redirects. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum PreventionShieldRoute { + /// CR 614.9: a recognized `redirect_target` plus an amount the redirection + /// mechanics own — the damage MOVES to this recipient. CR 615.12 ("damage + /// can't be prevented") must NOT suppress this: nothing is prevented. + Redirect(DamageRedirectTarget), + /// CR 615: an ordinary prevention shield — the damage is PREVENTED, and + /// CR 615.12 suppresses it. Also the route for the two non-redirecting + /// shapes that legitimately reach this gate: a `SpecificObject` recipient + /// (owned by the effect-created one-shot path, Branch 1b) and the + /// `AllBut`/`Next` amounts the redirection mechanics do not own. + Prevent, + /// CR 614.9: a `redirect_target` with NO mapping onto a + /// [`DamageRedirectTarget`]. Fails CLOSED in every build profile — the + /// replacement does not apply at all. Neither fallback is acceptable: + /// redirecting needs a recipient we do not have, and preventing would DELETE + /// the damage this spine exists to MOVE. + Unmapped, } /// CR 614.9: Map a DURABLE redirection shield's stored `redirect_target` filter @@ -1846,39 +1858,87 @@ fn redirect_target_for_rid(state: &GameState, rid: ReplacementId) -> Option Option { +/// `parse_durable_redirect_recipient_filter`; a recipient added there without a +/// mapping here lands in the residual arm and FAILS CLOSED rather than silently +/// degrading a CR 614.9 redirection into a CR 615 prevention (damage deleted +/// rather than moved) — the exact defect the anchored redirection spine exists +/// to eliminate. `durable_redirect_route_is_total_over_parser_recipients` drives +/// this function with the parser's own accepted phrasings, so the residual arm +/// cannot become reachable without a test failure in every build profile. +fn durable_redirect_route_for_filter(filter: &TargetFilter) -> PreventionShieldRoute { match filter { // "...is dealt to ~ instead" — the shield host itself. - TargetFilter::SelfRef => Some(DamageRedirectTarget::SourceObject), + TargetFilter::SelfRef => { + PreventionShieldRoute::Redirect(DamageRedirectTarget::SourceObject) + } // CR 303.4b + CR 301.5a: "...is dealt to enchanted/equipped creature // instead" — the host the shield's source is attached to. - TargetFilter::AttachedTo => Some(DamageRedirectTarget::AttachedToSource), + TargetFilter::AttachedTo => { + PreventionShieldRoute::Redirect(DamageRedirectTarget::AttachedToSource) + } // CR 614.9: a concrete object recipient belongs exclusively to the // EFFECT-CREATED path — `create_damage_replacement::resolve` writes // `SpecificObject { id }` alongside a `ShieldKind::Redirection` (of // either `RedirectionLifetime`), and `redirect_chosen_object_for_rid` is // its reader. Such a shield is claimed by Branch 1b and never reaches - // this Prevention-shield gate; mapping it here would resurrect a consumed - // one-shot as a durable shield. `None` by intent. - TargetFilter::SpecificObject { .. } => None, - other => { - debug_assert!( - false, - "CR 614.9: unmapped durable redirect recipient {other:?} — a recipient was added \ - to parse_durable_redirect_recipient_filter without a mapping here; it would \ - silently degrade to a CR 615 prevention" - ); - None + // this Prevention-shield gate; routing it to `Redirect` here would + // resurrect a consumed one-shot as a durable shield. + TargetFilter::SpecificObject { .. } => PreventionShieldRoute::Prevent, + _ => PreventionShieldRoute::Unmapped, + } +} + +/// CR 614.9 + CR 615.1a: the SINGLE authority for whether a +/// `ShieldKind::Prevention` shield prevents, redirects, or must fail closed. +/// +/// Takes the definition rather than looking it up: `is_damage_prevention_replacement` +/// (on `find_applicable_replacements`' per-candidate loop, i.e. the damage hot +/// path) already holds it, and borrows the stored filter rather than cloning, so +/// the CR 615.12 gate adds neither a second map lookup nor an allocation. +fn prevention_shield_route_for_def( + repl: &ReplacementDefinition, + amount: PreventionAmount, +) -> PreventionShieldRoute { + // CR 615.1a: no recipient stored — an ordinary "prevent" shield. + let Some(filter) = repl.redirect_target.as_ref() else { + return PreventionShieldRoute::Prevent; + }; + match durable_redirect_route_for_filter(filter) { + // CR 615.7: `redirect_damage_event` treats `PreventionAmount::AllBut` as + // `unreachable!()` — an invariant of `ShieldKind::Redirection`, not of + // `ShieldKind::Prevention`, which legitimately uses `AllBut` for Temple + // Altisaur. Only `All` is owned by the redirection mechanics; every other + // amount stays on the CR 615 prevention arms, and therefore stays + // suppressible by CR 615.12. + PreventionShieldRoute::Redirect(_) if !matches!(amount, PreventionAmount::All) => { + PreventionShieldRoute::Prevent } + route => route, } } +/// `ReplacementId`-keyed wrapper over [`prevention_shield_route_for_def`] for the +/// apply-time call site, which has only the `Copy` `ShieldKind` in scope. A +/// missing definition cannot redirect, so it routes to `Prevent` (the pre-existing +/// behavior for an unresolvable rid). +fn prevention_shield_route( + state: &GameState, + rid: ReplacementId, + amount: PreventionAmount, +) -> PreventionShieldRoute { + let repl = if rid.source == ObjectId(0) { + state.pending_damage_replacements.get(rid.index) + } else { + state + .objects + .get(&rid.source) + .and_then(|obj| obj.replacement_definitions.get(rid.index)) + }; + repl.map_or(PreventionShieldRoute::Prevent, |repl| { + prevention_shield_route_for_def(repl, amount) + }) +} + /// CR 614.9: Resolve and apply a damage redirection. Shared by the /// `ShieldKind::Redirection` path (whose own `lifetime` decides consumption) and /// the durable `ShieldKind::Prevention` + `redirect_target` path (a printed, @@ -1966,6 +2026,18 @@ fn redirect_damage_event( unreachable!("PreventionAmount::AllBut is never assigned to a ShieldKind::Redirection") } PreventionAmount::Next(n) => { + // CR 614.5: "the next N damage" is one-opportunity grammar, and the + // depletion bookkeeping below runs only under `consume_after_redirect`. + // A `Continuous` shield would therefore never spend `n` and would + // redirect up to N damage from EVERY event in its window. No parser + // path produces that pairing (the durable Prevention gate always + // passes `All`), but the type permits it and the failure is silent — + // so it is asserted rather than assumed, mirroring the `AllBut` arm. + debug_assert!( + consume_after_redirect, + "CR 614.5: PreventionAmount::Next must not pair with \ + RedirectionLifetime::Continuous — the amount would never deplete" + ); let redirected_amount = damage_amount.min(n); let remaining_amount = damage_amount.saturating_sub(redirected_amount); if consume_after_redirect { @@ -2306,33 +2378,47 @@ fn damage_done_applier( // `RedirectionLifetime::Continuous` so the durable shield re-fires // for every damage event within its lifetime, and skip the // DamagePrevented / `combat_prevention_tally` bookkeeping entirely - // (no damage is prevented — it is dealt to a new recipient). The - // `matches!(amount, PreventionAmount::All)` conjunct is required: - // `redirect_damage_event`'s body treats `PreventionAmount::AllBut` as - // `unreachable!()` (an invariant of `ShieldKind::Redirection`, not of - // `ShieldKind::Prevention`, which legitimately uses `AllBut` for - // Temple Altisaur), so any `AllBut`/`Next` prevention shield — even a - // hypothetical future one carrying a `redirect_target` — falls - // through to the ordinary prevention arms below rather than reaching - // that dead code. - if let Some(recipient) = redirect_target_for_rid(state, rid) - .as_ref() - .and_then(durable_redirect_recipient) - .filter(|_| matches!(amount, PreventionAmount::All)) - { - return redirect_damage_event( - state, - rid, - recipient, - PreventionAmount::All, - source_id, - target, - dmg, - is_combat, - applied, - RedirectionLifetime::Continuous, - events, - ); + // (no damage is prevented — it is dealt to a new recipient). + // + // `prevention_shield_route` is the SAME authority the CR 615.12 + // suppression gate (`is_damage_prevention_replacement`) consults, so + // a shield can never be classified as prevention there and applied as + // a redirection here. It also owns the amount gate: `AllBut`/`Next` + // shields route to `Prevent` and fall through to the ordinary + // prevention arms below. + match prevention_shield_route(state, rid, amount) { + PreventionShieldRoute::Redirect(recipient) => { + return redirect_damage_event( + state, + rid, + recipient, + PreventionAmount::All, + source_id, + target, + dmg, + is_combat, + applied, + RedirectionLifetime::Continuous, + events, + ); + } + // CR 614.9: fail CLOSED. The shield carries a recipient this + // build cannot map, so it can neither redirect (no recipient) nor + // fall through to the CR 615 arms (which would DELETE the damage + // rather than move it). Return the event unmodified so the damage + // is dealt exactly as proposed — the only outcome that loses no + // damage. `mark_applied(rid)` already ran before this applier, so + // declining here cannot re-enter the same shield. + PreventionShieldRoute::Unmapped => { + return ApplyResult::Modified(ProposedEvent::Damage { + source_id, + target, + amount: dmg, + is_combat, + applied, + }); + } + PreventionShieldRoute::Prevent => {} } let prevented_amount; @@ -5412,11 +5498,28 @@ fn is_damage_prevention_replacement( return false; } - // Check for ShieldKind::Prevention or description-based prevention patterns - // CR 615: Prevention shields created by prevent_damage.rs - matches!(repl.shield_kind, ShieldKind::Prevention { .. }) + // CR 615: Prevention shields created by prevent_damage.rs. + // + // CR 614.9 + CR 615.1a: a Prevention-SHAPED shield that carries a + // `redirect_target` is a CR 614.9 REDIRECTION, not a CR 615 prevention — its + // Oracle grammar never says "prevent" (CR 615.1a), and no damage is prevented + // when it applies (the damage is dealt to a new recipient). CR 615.12 + // therefore must NOT suppress it; classifying it as prevention here made + // Pariah, Pariah's Shield, With Great Power . . ., Palisade Giant and Ancient + // Adamantoise silently stop redirecting under any "damage can't be prevented" + // effect. `prevention_shield_route` is the SAME authority + // `damage_done_applier`'s Branch 2 consults, so this gate and the apply-time + // route cannot drift apart (an `Unmapped` shield stays classified as + // prevention here and fails closed there — either way it does nothing). + if let ShieldKind::Prevention { amount } = repl.shield_kind { + return !matches!( + prevention_shield_route_for_def(repl, amount), + PreventionShieldRoute::Redirect(_) + ); + } + // Legacy: description-based prevention from parsed replacement definitions - || repl.description.as_ref().is_some_and(|d| { + repl.description.as_ref().is_some_and(|d| { let lower = d.to_lowercase(); lower.contains("prevent") && lower.contains("damage") }) @@ -5482,7 +5585,7 @@ fn matches_damage_target_filter( // sentinel `ObjectId(0)`, which matches no permanent, so the // exclusion is correctly inert there. TargetRef::Object(oid) => { - (!source_scope.excludes_source() || *oid != repl_source) + (!source_scope.is_exclude() || *oid != repl_source) && state.objects.get(oid).is_some_and(|obj| { player_scope_matches( player, @@ -10079,9 +10182,9 @@ mod tests { use crate::game::game_object::{AttachTarget, GameObject}; use crate::types::ability::{ AbilityCost, AbilityDefinition, AbilityKind, CastManaObjectScope, CastManaSpentMetric, - ChosenAttribute, Comparator, ControlledPermanentsScope, ControllerRef, Effect, EffectScope, - FilterProp, OriginConstraint, PlayerFilter, PtValue, QuantityExpr, QuantityModification, - QuantityRef, ReplacementDefinition, ReplacementMode, ReplacementPlayerScope, + ChosenAttribute, Comparator, ControllerRef, Effect, EffectScope, FilterProp, + OriginConstraint, PlayerFilter, PtValue, QuantityExpr, QuantityModification, QuantityRef, + ReplacementDefinition, ReplacementMode, ReplacementPlayerScope, SourceExclusion, TapStateChange, TargetFilter, TargetRef, TypeFilter, TypedFilter, }; use crate::types::actions::GameAction; @@ -10102,33 +10205,61 @@ mod tests { } /// CR 614.9: the durable-redirection recipient mapping is TOTAL over every - /// filter the parser's `parse_durable_redirect_recipient_filter` can emit. An - /// unmapped recipient would fall through to the ordinary CR 615 prevention - /// arms and DELETE the damage instead of moving it. + /// filter the parser's `parse_durable_redirect_recipient_filter` can emit. + /// An unmapped recipient makes the shield fail closed (the damage is dealt + /// as proposed instead of being moved), which is safe but silently drops the + /// card's whole ability — so the mapping must never have a hole. /// - /// The residual arm's `debug_assert!` is the loud-failure guard for a future - /// recipient added to the parser without a mapping here; it is deliberately - /// NOT exercised by a test (tripping it would abort the test binary) — the - /// assert itself is the assertion. + /// The cases are driven through the REAL grammar rather than a hand-written + /// list of `TargetFilter`s: a recipient phrasing added to + /// `parse_durable_redirect_recipient_filter` without a matching arm in + /// `durable_redirect_route_for_filter` fails here, in every build profile. + /// The previous hard-coded version could not catch that — it restated the + /// mapping's own arms back to it. #[test] - fn durable_redirect_recipient_maps_every_parser_producible_filter() { - assert_eq!( - durable_redirect_recipient(&TargetFilter::SelfRef), - Some(DamageRedirectTarget::SourceObject), - "\"...is dealt to ~ instead\" redirects onto the shield host" - ); - assert_eq!( - durable_redirect_recipient(&TargetFilter::AttachedTo), - Some(DamageRedirectTarget::AttachedToSource), - "\"...is dealt to enchanted/equipped creature instead\" redirects onto the host" - ); + fn durable_redirect_route_is_total_over_parser_recipients() { + use crate::parser::oracle_replacement::parse_durable_redirect_recipient_filter; + + // Every recipient phrasing the "... is dealt to instead" + // tail accepts, in the normalized (`~`-substituted, lowercased) form the + // spine sees. Each must route to `Redirect`. + let phrasings = [ + ("~", DamageRedirectTarget::SourceObject), + ("equipped creature", DamageRedirectTarget::AttachedToSource), + ("enchanted creature", DamageRedirectTarget::AttachedToSource), + ]; + for (phrase, expected) in phrasings { + let (rest, filter) = parse_durable_redirect_recipient_filter(phrase) + .unwrap_or_else(|_| panic!("the spine's recipient slot must accept {phrase:?}")); + assert!( + rest.is_empty(), + "{phrase:?} must be fully consumed by the recipient slot, left {rest:?}" + ); + assert_eq!( + durable_redirect_route_for_filter(&filter), + PreventionShieldRoute::Redirect(expected), + "{phrase:?} parsed to {filter:?}, which has no redirection mapping — it would \ + fail closed and silently drop the card's redirection" + ); + } + // Owned by the ONE-SHOT path (`redirect_chosen_object_for_rid`), which - // reads it off a `ShieldKind::Redirection` shield — never this gate. + // reads it off a `ShieldKind::Redirection` shield — never this gate. Not + // parser-producible here, so it is asserted directly. assert_eq!( - durable_redirect_recipient(&TargetFilter::SpecificObject { id: ObjectId(7) }), - None, + durable_redirect_route_for_filter(&TargetFilter::SpecificObject { id: ObjectId(7) }), + PreventionShieldRoute::Prevent, "a captured chosen object belongs to the one-shot redirection shield" ); + + // The fail-closed residual arm, asserted rather than assumed: an + // unmapped recipient must NOT reach the CR 615 prevention arms, where it + // would delete the damage instead of moving it. + assert_eq!( + durable_redirect_route_for_filter(&TargetFilter::Any), + PreventionShieldRoute::Unmapped, + "an unmapped recipient must fail closed, not degrade into a CR 615 prevention" + ); } fn search_found_execute(destination: Zone) -> AbilityDefinition { @@ -13822,6 +13953,123 @@ mod tests { ); } + /// CR 614.9 + CR 615.12: "damage can't be prevented" suppresses PREVENTION + /// effects only. A durable redirection (Palisade Giant, Ancient Adamantoise, + /// Pariah, Pariah's Shield, With Great Power . . .) is stored as a + /// `ShieldKind::Prevention` shield carrying a `redirect_target`, but it + /// prevents nothing — it moves the damage — so it must keep applying. + /// + /// Discriminating: reverting the `redirect_target` guard in + /// `is_damage_prevention_replacement` re-classifies this shield as a + /// prevention, `find_applicable_replacements` drops it, and the redirect + /// assertion below fails with an empty candidate list. + #[test] + fn restriction_does_not_block_durable_redirect_shields() { + use crate::types::ability::{ + GameRestriction, PreventionAmount, ReplacementDefinition, RestrictionExpiry, + }; + + // Palisade Giant's shape: "all damage that would be dealt to you and + // other permanents you control is dealt to ~ instead". + let giant = ObjectId(1); + let redirect_repl = ReplacementDefinition::new(ReplacementEvent::DamageDone) + .prevention_shield(PreventionAmount::All) + .redirect_target(TargetFilter::SelfRef); + let mut state = test_state_with_object(giant, Zone::Battlefield, vec![redirect_repl]); + // CR 614.9: the recipient must still be a creature on the battlefield, or + // the redirection legitimately does nothing. + state + .objects + .get_mut(&giant) + .expect("fixture object") + .card_types + .core_types = vec![crate::types::card_type::CoreType::Creature]; + state + .restrictions + .push(GameRestriction::DamagePreventionDisabled { + source: ObjectId(99), + expiry: RestrictionExpiry::EndOfTurn, + scope: None, // Global + }); + + let proposed = ProposedEvent::Damage { + source_id: ObjectId(50), + target: TargetRef::Player(PlayerId(0)), + amount: 3, + is_combat: false, + applied: HashSet::new(), + }; + + let registry = build_replacement_registry(); + let candidates = find_applicable_replacements(&state, &proposed, ®istry); + assert!( + !candidates.is_empty(), + "CR 615.12 suppresses prevention, not CR 614.9 redirection — the shield must survive" + ); + + // Reach guard + behavior: the surviving candidate must actually MOVE the + // damage onto the Giant, not merely be offered. An `is_empty()` check + // alone would still pass if the shield were applied as a prevention. + let rid = candidates[0]; + let mut events = Vec::new(); + let result = damage_done_applier(proposed, rid, &mut state, &mut events); + match result { + ApplyResult::Modified(ProposedEvent::Damage { target, amount, .. }) => { + assert_eq!( + target, + TargetRef::Object(giant), + "CR 614.9: the damage is dealt to the redirection host instead" + ); + assert_eq!(amount, 3, "CR 615.12: a redirection prevents no damage"); + } + other => panic!("expected the damage to be redirected, got {other:?}"), + } + } + + /// CR 614.9: a durable shield whose `redirect_target` has no mapping FAILS + /// CLOSED — the damage is dealt as proposed. It must never fall through to + /// the CR 615 prevention arms, which would delete the damage entirely. + /// + /// Discriminating: routing `Unmapped` to the prevention arms instead makes + /// this return `ApplyResult::Prevented`. + #[test] + fn unmapped_durable_redirect_recipient_fails_closed_instead_of_preventing() { + use crate::types::ability::{PreventionAmount, ReplacementDefinition}; + + let host = ObjectId(1); + // `TargetFilter::Any` is not a recipient the spine can produce; it stands + // in for a future parser recipient added without a mapping. + let repl = ReplacementDefinition::new(ReplacementEvent::DamageDone) + .prevention_shield(PreventionAmount::All) + .redirect_target(TargetFilter::Any); + let mut state = test_state_with_object(host, Zone::Battlefield, vec![repl]); + + let proposed = ProposedEvent::Damage { + source_id: ObjectId(50), + target: TargetRef::Player(PlayerId(0)), + amount: 3, + is_combat: false, + applied: HashSet::new(), + }; + let rid = ReplacementId { + source: host, + index: 0, + }; + let mut events = Vec::new(); + let result = damage_done_applier(proposed, rid, &mut state, &mut events); + match result { + ApplyResult::Modified(ProposedEvent::Damage { target, amount, .. }) => { + assert_eq!( + target, + TargetRef::Player(PlayerId(0)), + "the damage stays on its original recipient" + ); + assert_eq!(amount, 3, "no damage may be deleted by an unmapped shield"); + } + other => panic!("an unmapped recipient must fail closed, got {other:?}"), + } + } + // ── destination_zone filter tests (CR 614.6) ── fn rip_replacement() -> ReplacementDefinition { @@ -15058,7 +15306,7 @@ mod tests { .damage_target_filter(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Opponent, permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }); // Hawkeye = ObjectId(10), controlled by P0, power 2. @@ -15478,7 +15726,7 @@ mod tests { .damage_target_filter(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Opponent, permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }); // Replacement on P0's object let state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); @@ -15504,7 +15752,7 @@ mod tests { .damage_target_filter(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Opponent, permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }); let state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); @@ -15530,7 +15778,7 @@ mod tests { .damage_target_filter(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Opponent, permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }); let mut state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); @@ -15567,7 +15815,7 @@ mod tests { DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::SourceChosenPlayer, permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }, ); let mut state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); @@ -15610,7 +15858,7 @@ mod tests { DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::SourceChosenPlayer, permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }, ); let mut state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index a273a9b340..95e333a3e0 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -21447,7 +21447,7 @@ mod tests { /// restriction (Channel Harm). #[test] fn prevent_compound_recipient_you_and_controlled_permanents() { - use crate::types::ability::ControlledPermanentsScope; + use crate::types::ability::SourceExclusion; use crate::types::card_type::CoreType; let planeswalker_text = @@ -21460,7 +21460,7 @@ mod tests { target, TargetFilter::ControllerAndControlledPermanents { permanent_type: Some(CoreType::Planeswalker), - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, } ); @@ -21474,7 +21474,7 @@ mod tests { target, TargetFilter::ControllerAndControlledPermanents { permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, } ); @@ -21501,7 +21501,7 @@ mod tests { target, TargetFilter::ControllerAndControlledPermanents { permanent_type: None, - source_scope: ControlledPermanentsScope::ExcludingSource, + source_scope: SourceExclusion::Exclude, }, "The Wanderer must exclude itself from its own noncombat-damage shield" ); diff --git a/crates/engine/src/parser/oracle_nom/filter.rs b/crates/engine/src/parser/oracle_nom/filter.rs index cf31d90bcf..36ac8770d0 100644 --- a/crates/engine/src/parser/oracle_nom/filter.rs +++ b/crates/engine/src/parser/oracle_nom/filter.rs @@ -17,8 +17,8 @@ use super::primitives::{ }; use super::quantity::{parse_quantity_expr_number, parse_quantity_ref}; use crate::types::ability::{ - AggregateFunction, Comparator, ControlledPermanentsScope, ControllerRef, FilterProp, - ObjectProperty, PtStat, PtValueScope, QuantityExpr, + AggregateFunction, Comparator, ControllerRef, FilterProp, ObjectProperty, PtStat, PtValueScope, + QuantityExpr, SourceExclusion, }; use crate::types::card_type::CoreType; #[cfg(test)] @@ -512,7 +512,7 @@ pub struct ControlledPermanentsConjunct { pub permanent_type: Option, /// CR 109.1: whether the leading "other" article excluded the ability's own /// source object. - pub source_scope: ControlledPermanentsScope, + pub source_scope: SourceExclusion, } /// CR 614.1a + CR 109.1: SINGLE AUTHORITY for the "\[other\] `` you @@ -554,8 +554,8 @@ pub fn parse_controlled_permanents_conjunct( ControlledPermanentsConjunct { permanent_type, source_scope: match other { - Some(_) => ControlledPermanentsScope::ExcludingSource, - None => ControlledPermanentsScope::IncludingSource, + Some(_) => SourceExclusion::Exclude, + None => SourceExclusion::Include, }, }, )) @@ -584,7 +584,7 @@ mod tests { assert_eq!(plain.permanent_type, expected_type); assert_eq!( plain.source_scope, - ControlledPermanentsScope::IncludingSource, + SourceExclusion::Include, "no \"other\" article means the source is included" ); @@ -595,7 +595,7 @@ mod tests { assert_eq!(excluded.permanent_type, expected_type); assert_eq!( excluded.source_scope, - ControlledPermanentsScope::ExcludingSource, + SourceExclusion::Exclude, "the \"other\" article must reach the caller, not be opt()-discarded" ); } diff --git a/crates/engine/src/parser/oracle_replacement.rs b/crates/engine/src/parser/oracle_replacement.rs index 5e94c40d85..f3c508503e 100644 --- a/crates/engine/src/parser/oracle_replacement.rs +++ b/crates/engine/src/parser/oracle_replacement.rs @@ -37,14 +37,14 @@ use super::oracle_util::{ use crate::types::ability::CastingPermission; use crate::types::ability::{ AbilityCost, AbilityDefinition, AbilityKind, CastVariantPaid, ChoiceType, CombatDamageScope, - Comparator, ContinuousModification, ControlledPermanentsScope, ControllerRef, - CopyManaValueLimit, CountScope, CounterReplacementSubject, DamageModification, - DamageRedirectTarget, DamageTargetFilter, DamageTargetPlayerScope, DrawReplacementScope, - Duration, Effect, EffectScope, FilterProp, LibraryPosition, ManaModification, - ManaReplacementScope, ManaSpendPermission, PermissionGrantee, PlayerFilter, PreventionAmount, - QuantityExpr, QuantityModification, QuantityRef, RedirectionLifetime, ReplacementCondition, - ReplacementDefinition, ReplacementMode, ReplacementPlayerScope, StaticCondition, - StaticDefinition, TapStateChange, TargetFilter, TriggerDefinition, TypeFilter, TypedFilter, + Comparator, ContinuousModification, ControllerRef, CopyManaValueLimit, CountScope, + CounterReplacementSubject, DamageModification, DamageRedirectTarget, DamageTargetFilter, + DamageTargetPlayerScope, DrawReplacementScope, Duration, Effect, EffectScope, FilterProp, + LibraryPosition, ManaModification, ManaReplacementScope, ManaSpendPermission, + PermissionGrantee, PlayerFilter, PreventionAmount, QuantityExpr, QuantityModification, + QuantityRef, RedirectionLifetime, ReplacementCondition, ReplacementDefinition, ReplacementMode, + ReplacementPlayerScope, SourceExclusion, StaticCondition, StaticDefinition, TapStateChange, + TargetFilter, TriggerDefinition, TypeFilter, TypedFilter, }; use crate::types::card_type::Supertype; use crate::types::counter::{CounterMatch, CounterType}; @@ -7514,7 +7514,7 @@ fn damage_target_opponent_or_permanents() -> DamageTargetFilter { DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Opponent, permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, } } @@ -7524,7 +7524,7 @@ fn damage_target_source_chosen_player_or_permanents() -> DamageTargetFilter { DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::SourceChosenPlayer, permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, } } @@ -9957,7 +9957,15 @@ fn parse_damage_noun_with_scope(input: &str) -> OracleResult<'_, Option OracleResult<'_, TargetFilter> { +/// +/// `pub(crate)` solely so `game::replacement`'s +/// `durable_redirect_route_is_total_over_parser_recipients` can drive the real +/// grammar rather than restating an inevitably-drifting hand-written list of the +/// filters it emits. That test is what keeps +/// `durable_redirect_route_for_filter`'s residual (fail-closed) arm unreachable. +pub(crate) fn parse_durable_redirect_recipient_filter( + input: &str, +) -> OracleResult<'_, TargetFilter> { alt(( value(TargetFilter::SelfRef, tag::<_, _, OracleError<'_>>("~")), // CR 301.5a ("equipped creature") + CR 303.4b ("enchanted") — Pariah, @@ -14041,7 +14049,7 @@ mod tests { *target, TargetFilter::ControllerAndControlledPermanents { permanent_type: Some(CoreType::Planeswalker), - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, } ); assert!(matches!( @@ -14118,7 +14126,7 @@ mod tests { *target, TargetFilter::ControllerAndControlledPermanents { permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }, "Channel Harm's \"you and permanents you control\" is unrestricted" ); @@ -19496,7 +19504,7 @@ mod tests { Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Controller, permanent_type: None, - source_scope: ControlledPermanentsScope::ExcludingSource, + source_scope: SourceExclusion::Exclude, }), "the \"and other permanents you control\" conjunct must be carried, not dropped" ); @@ -19593,7 +19601,7 @@ mod tests { player: DamageTargetPlayerScope::Controller, permanent_type: Some(CoreType::Creature), // No "other" article on this card. - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }) ); // CR 614.9: the recipient is the permanent the parent "Choose target @@ -19677,7 +19685,7 @@ mod tests { Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Controller, permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }) ); assert_eq!(*redirect_to, Some(DamageRedirectTarget::ChosenObjectTarget)); @@ -19898,7 +19906,7 @@ mod tests { permanent_type: None, // CR 109.1: the "other" article must reach the filter, not be // opt()-discarded. - source_scope: ControlledPermanentsScope::ExcludingSource, + source_scope: SourceExclusion::Exclude, } ); assert!(rest.is_empty()); @@ -19911,7 +19919,7 @@ mod tests { DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Controller, permanent_type: Some(CoreType::Creature), - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, } ); assert_eq!( @@ -19921,7 +19929,7 @@ mod tests { DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Controller, permanent_type: Some(CoreType::Planeswalker), - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, } ); @@ -19939,7 +19947,7 @@ mod tests { DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Controller, permanent_type: Some(expected), - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }, "{phrase} must reach the shared conjunct authority" ); @@ -20025,7 +20033,7 @@ mod tests { Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::Controller, permanent_type: None, - source_scope: ControlledPermanentsScope::ExcludingSource, + source_scope: SourceExclusion::Exclude, }), "\"you and OTHER permanents you control\" must carry both the permanent leg and the self-exclusion" ); diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index b473535d1d..7a16cbde0a 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -5394,8 +5394,8 @@ pub enum TargetFilter { /// leg can exclude the ability's own source. ControllerAndControlledPermanents { permanent_type: Option, - #[serde(default)] - source_scope: ControlledPermanentsScope, + #[serde(default, skip_serializing_if = "SourceExclusion::is_include")] + source_scope: SourceExclusion, }, /// CR 102.2 + CR 102.3 + CR 601.2c: A player reference to an opponent of the /// ability's controller, used as the announcing player (`target_chooser`) for @@ -23969,42 +23969,6 @@ pub enum DamageTargetPlayerScope { Specific(PlayerId), } -/// CR 109.1 (cited as identity foundation — the CR has no dedicated "another" -/// entry; mirrors the citation on [`FilterProp::Another`]): whether the -/// controlled-permanents leg of a compound "`` and \[other\] `` you control" recipient includes the object the ability is printed on. -/// -/// This is the typed home of the "OTHER" article that the phrase grammar reads. -/// It is a genuine rules axis, not a cosmetic one: Palisade Giant, Ancient -/// Adamantoise and The Wanderer say "other permanents you control", so their own -/// shield must NOT claim damage dealt to the shield host, while Comeuppance, -/// Channel Harm, Blessed Sanctuary and Heroic Sacrifice say "permanents/creatures -/// you control" with no exclusion. -/// -/// CR 614.5 ("a replacement effect gets only one opportunity to affect an -/// event") makes the distinction invisible for a LONE self-recipient shield, but -/// not once a second applicable replacement exists: under CR 616.1 the affected -/// player is offered every applicable replacement, and offering a self-no-op -/// burns the CR 614.5 opportunity on the wrong shield. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] -#[serde(tag = "type")] -pub enum ControlledPermanentsScope { - /// "…and permanents you control" — no exclusion stated; every permanent the - /// scoped player controls, including the ability's own source. - #[default] - IncludingSource, - /// "…and OTHER permanents you control" — the object this ability is printed - /// on is excluded from the permanent leg. - ExcludingSource, -} - -impl ControlledPermanentsScope { - /// True when the permanent leg excludes the ability's own source object. - pub fn excludes_source(self) -> bool { - matches!(self, ControlledPermanentsScope::ExcludingSource) - } -} - /// CR 614.1a: Restricts which damage targets a replacement applies to. /// Dedicated enum because `TargetRef` can be `Player` (not handled by `matches_target_filter`). #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -24025,12 +23989,30 @@ pub enum DamageTargetFilter { /// /// CR 109.1: `source_scope` carries the "OTHER" article — Palisade Giant's /// "you and OTHER permanents you control" excludes the shield host itself - /// from the permanent leg. See [`ControlledPermanentsScope`]. + /// from the permanent leg, while Comeuppance, Channel Harm, Blessed Sanctuary + /// and Heroic Sacrifice state no exclusion. + /// + /// The CR has no dedicated "other"/"another" entry; CR 109.1 (object + /// identity) is cited as the foundation for the same-object comparison the + /// article implies, matching how the rest of the engine annotates `another` + /// exclusions (`FilterProp::Another`, `with_own_cast_exclusion`, + /// `restrictions.rs`, `trigger_matchers.rs`). It is a convention, not a + /// verbatim rule — do not read it as "CR 109.1 defines `other`". + /// + /// It is a genuine rules axis, not a cosmetic one. CR 614.5 ("a replacement + /// effect gets only one opportunity to affect an event") makes the + /// distinction invisible for a LONE self-recipient shield, but not once a + /// second applicable replacement exists: under CR 616.1 the affected player + /// is offered every applicable replacement, and offering a self-no-op burns + /// the CR 614.5 opportunity on the wrong shield. + /// + /// Reuses [`SourceExclusion`] — the existing "does this predicate include the + /// ability's own source object" axis — rather than a parallel vocabulary. PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope, permanent_type: Option, - #[serde(default)] - source_scope: ControlledPermanentsScope, + #[serde(default, skip_serializing_if = "SourceExclusion::is_include")] + source_scope: SourceExclusion, }, } diff --git a/crates/engine/tests/integration/heroic_sacrifice_redirect.rs b/crates/engine/tests/integration/heroic_sacrifice_redirect.rs index f35ce68d25..ce10996f62 100644 --- a/crates/engine/tests/integration/heroic_sacrifice_redirect.rs +++ b/crates/engine/tests/integration/heroic_sacrifice_redirect.rs @@ -25,10 +25,10 @@ //! stray draw is inert, and deliberately assert NOTHING about the draw — pinning //! the wrong placement would bless it. +use super::rules::damage_ability; use engine::game::effects::deal_damage; use engine::game::scenario::{GameScenario, P0, P1}; -use engine::types::ability::{Effect, QuantityExpr, ResolvedAbility, TargetFilter, TargetRef}; -use engine::types::identifiers::ObjectId; +use engine::types::ability::TargetRef; use engine::types::phase::Phase; /// Verbatim Heroic Sacrifice (Scryfall-verified). @@ -38,21 +38,6 @@ const HEROIC_SACRIFICE_TEXT: &str = "Choose target creature you control. Until e /// duration spelling with an untyped permanent leg. const GIDEONS_SACRIFICE_TEXT: &str = "Choose a creature or planeswalker you control. All damage that would be dealt this turn to you and permanents you control is dealt to the chosen permanent instead (if it's still on the battlefield)."; -/// A non-combat damage source dealing `amount` to `target`, controlled by P1. -fn damage_ability(source_id: ObjectId, target: TargetRef, amount: i32) -> ResolvedAbility { - ResolvedAbility::new( - Effect::DealDamage { - amount: QuantityExpr::Fixed { value: amount }, - target: TargetFilter::Any, - damage_source: None, - excess: None, - }, - vec![target], - source_id, - P1, - ) -} - /// CR 611.2a + CR 614.9 + CR 614.1a: the whole card, through the production /// cast pipeline. /// diff --git a/crates/engine/tests/integration/oracle_parser.rs b/crates/engine/tests/integration/oracle_parser.rs index 9d370639b1..d03395365b 100644 --- a/crates/engine/tests/integration/oracle_parser.rs +++ b/crates/engine/tests/integration/oracle_parser.rs @@ -1,7 +1,7 @@ use engine::parser::oracle::{keyword_display_name, parse_oracle_text}; use engine::types::ability::{ - ChosenSubtypeKind, ContinuousModification, ControlledPermanentsScope, ControllerRef, - DamageModification, DamageTargetFilter, DamageTargetPlayerScope, Effect, FilterProp, + ChosenSubtypeKind, ContinuousModification, ControllerRef, DamageModification, + DamageTargetFilter, DamageTargetPlayerScope, Effect, FilterProp, SourceExclusion, StaticCondition, TargetFilter, TypeFilter, }; use engine::types::keywords::Keyword; @@ -521,7 +521,7 @@ fn sawhorn_nemesis_damage_replacement_scopes_to_source_chosen_player() { Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { player: DamageTargetPlayerScope::SourceChosenPlayer, permanent_type: None, - source_scope: ControlledPermanentsScope::IncludingSource, + source_scope: SourceExclusion::Include, }) ); } diff --git a/crates/engine/tests/integration/palisade_giant_redirect.rs b/crates/engine/tests/integration/palisade_giant_redirect.rs index 1de77a7291..4cf0a6e21e 100644 --- a/crates/engine/tests/integration/palisade_giant_redirect.rs +++ b/crates/engine/tests/integration/palisade_giant_redirect.rs @@ -16,16 +16,16 @@ use engine::game::effects::deal_damage; use engine::game::sba::check_state_based_actions; -use engine::game::scenario::{GameScenario, P0, P1}; +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; use engine::game::triggers::process_triggers; -use engine::types::ability::{Effect, QuantityExpr, ResolvedAbility, TargetFilter, TargetRef}; +use engine::types::ability::{Effect, ShieldKind, TargetFilter, TargetRef}; use engine::types::game_state::WaitingFor; use engine::types::identifiers::ObjectId; use engine::types::phase::Phase; use engine::types::triggers::TriggerMode; use engine::types::zones::Zone; -use super::rules::run_combat; +use super::rules::{damage_ability, run_combat}; /// Verbatim Palisade Giant redirection text (Scryfall-verified). Byte-identical /// to Ancient Adamantoise's redirection line, so both cards are covered by the @@ -40,20 +40,32 @@ const PALISADE_GIANT_TEXT: &str = const WEATHERED_BODYGUARDS_TEXT: &str = "As long as this creature is untapped, all combat damage \ that would be dealt to you by unblocked creatures is dealt to this creature instead."; -/// A non-combat damage source dealing `amount` to `target`, controlled by P1 -/// (the opponent of the shield's controller in these fixtures). -fn damage_ability(source_id: ObjectId, target: TargetRef, amount: i32) -> ResolvedAbility { - ResolvedAbility::new( - Effect::DealDamage { - amount: QuantityExpr::Fixed { value: amount }, - target: TargetFilter::Any, - damage_source: None, - excess: None, - }, - vec![target], - source_id, - P1, - ) +/// Structural reach-guard: the redirection line really parsed and really +/// installed a CR 614.9 shield on this object. +/// +/// Required by any fixture whose only assertion is a value that would also hold +/// if the code under test were never reached — notably +/// `palisade_giant_self_damage_is_marked_once_not_doubled_or_prevented`, where +/// `damage_marked == 5` is equally true when the line regressed to +/// `Effect::Unimplemented` and no shield exists at all. +fn assert_redirect_shield_installed(runner: &GameRunner, obj: ObjectId, name: &str) { + let object = &runner.state().objects[&obj]; + assert!( + !object + .abilities + .iter() + .any(|a| matches!(&*a.effect, Effect::Unimplemented { .. })), + "{name} must parse with zero Effect::Unimplemented, got {:?}", + object.abilities + ); + assert!( + object.replacement_definitions.iter_unchecked().any(|def| { + matches!(def.shield_kind, ShieldKind::Prevention { .. }) + && def.redirect_target == Some(TargetFilter::SelfRef) + }), + "{name} must install a CR 614.9 self-recipient redirection shield, got {:?}", + object.replacement_definitions + ); } /// CR 614.9: Palisade Giant redirects damage that would be dealt to its @@ -349,6 +361,10 @@ fn palisade_giant_self_damage_is_marked_once_not_doubled_or_prevented() { let source = scenario.add_creature(P1, "Damage Source", 5, 5).id(); let mut runner = scenario.build(); + // Positive reach guard: `damage_marked == 5` below also holds when no shield + // exists at all, so prove the shield is installed before relying on it. + assert_redirect_shield_installed(&runner, giant, "Palisade Giant"); + let mut events = Vec::new(); deal_damage::resolve( runner.state_mut(), diff --git a/crates/engine/tests/integration/pariah_attached_redirect.rs b/crates/engine/tests/integration/pariah_attached_redirect.rs index fe87db4b6c..c1fb107ef3 100644 --- a/crates/engine/tests/integration/pariah_attached_redirect.rs +++ b/crates/engine/tests/integration/pariah_attached_redirect.rs @@ -16,11 +16,12 @@ //! //! All Oracle text is verbatim / Scryfall-verified. +use super::rules::damage_ability; use engine::game::effects::attach::attach_to; use engine::game::effects::deal_damage; use engine::game::game_object::AttachTarget; use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; -use engine::types::ability::{Effect, QuantityExpr, ResolvedAbility, TargetFilter, TargetRef}; +use engine::types::ability::TargetRef; use engine::types::actions::GameAction; use engine::types::game_state::WaitingFor; use engine::types::identifiers::ObjectId; @@ -46,21 +47,6 @@ const EMPYRIAL_ARCHANGEL_TEXT: &str = /// than install a global prevention field. const TREACHEROUS_LINK_TEXT: &str = "Enchant creature\nAll damage that would be dealt to enchanted creature is dealt to its controller instead."; -/// A non-combat damage source dealing `amount` to `target`, controlled by P1. -fn damage_ability(source_id: ObjectId, target: TargetRef, amount: i32) -> ResolvedAbility { - ResolvedAbility::new( - Effect::DealDamage { - amount: QuantityExpr::Fixed { value: amount }, - target: TargetFilter::Any, - damage_source: None, - excess: None, - }, - vec![target], - source_id, - P1, - ) -} - /// CR 701.3a/b/c + CR 613.7e: Attach through the engine's SINGLE ATTACH /// AUTHORITY, `effects::attach::attach_to`, rather than hand-wiring /// `attached_to` / `attachments`. @@ -71,10 +57,12 @@ fn damage_ability(source_id: ObjectId, target: TargetRef, amount: i32) -> Resolv /// A future change that makes redirection depend on attachment legality or /// timestamp would pass a hand-wired fixture while breaking real games. /// -/// `attach_to` returns `None` when the gate refuses, which would leave the -/// fixture silently unattached and turn every downstream assertion vacuous, so -/// the gate result is asserted rather than discarded. (Not -/// `attach_as_bestowed_aura` — Pariah is a printed Aura, not a bestowed +/// `attach_to`'s return value is deliberately DISCARDED: it yields the PREVIOUS +/// host (`old_target`), not a success flag, so a first attach returns `None` +/// precisely when it succeeded — asserting `.is_some()` on it would fail every +/// fixture here. A refused CR 701.3b gate is instead caught by the wired-state +/// assertions below, which is the only signal that actually discriminates. +/// (Not `attach_as_bestowed_aura` — Pariah is a printed Aura, not a bestowed /// creature.) fn attach(runner: &mut GameRunner, attachment: ObjectId, host: ObjectId) { attach_to(runner.state_mut(), attachment, host); diff --git a/crates/engine/tests/integration/rules.rs b/crates/engine/tests/integration/rules.rs index 55a333d073..44739cb387 100644 --- a/crates/engine/tests/integration/rules.rs +++ b/crates/engine/tests/integration/rules.rs @@ -16,6 +16,29 @@ pub use engine::types::phase::Phase; pub use engine::types::player::PlayerId; pub use engine::types::zones::{ExileCostSourceZone, Zone}; +/// Shared damage fixture: a non-combat source dealing `amount` to `target`, +/// controlled by P1 (the opponent of the shield controller in the CR 614.9 +/// redirection fixtures). Shared by `heroic_sacrifice_redirect`, +/// `pariah_attached_redirect`, and `palisade_giant_redirect`, which previously +/// each carried a byte-identical private copy. +pub fn damage_ability( + source_id: ObjectId, + target: engine::types::ability::TargetRef, + amount: i32, +) -> engine::types::ability::ResolvedAbility { + engine::types::ability::ResolvedAbility::new( + engine::types::ability::Effect::DealDamage { + amount: engine::types::ability::QuantityExpr::Fixed { value: amount }, + target: engine::types::ability::TargetFilter::Any, + damage_source: None, + excess: None, + }, + vec![target], + source_id, + P1, + ) +} + /// Shared combat helper: drives the engine from DeclareAttackers through damage resolution. /// /// Assumes the runner is at a phase where passing priority twice will reach DeclareAttackers From 80ab3a290869e45c41116c08bc0d20caf3cdb420 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Sun, 16 Aug 2026 12:14:28 -0700 Subject: [PATCH 3/3] fix(PR-7408): fail closed on invalid continuous shield CR 615.7 requires a finite Next(N) shield to deplete by the damage it prevents. A Continuous redirection has no such lifecycle, so reject the invalid pair without redirecting or mutating the shield. Also scope the parser-recipient regression to its hand-maintained supported phrases rather than claiming parser totality. Co-authored-by: Jacob Woodson --- crates/engine/src/game/replacement.rs | 124 +++++++++++++++++++++----- 1 file changed, 100 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/game/replacement.rs b/crates/engine/src/game/replacement.rs index 8a23070c27..eb756c20c6 100644 --- a/crates/engine/src/game/replacement.rs +++ b/crates/engine/src/game/replacement.rs @@ -1862,9 +1862,9 @@ enum PreventionShieldRoute { /// mapping here lands in the residual arm and FAILS CLOSED rather than silently /// degrading a CR 614.9 redirection into a CR 615 prevention (damage deleted /// rather than moved) — the exact defect the anchored redirection spine exists -/// to eliminate. `durable_redirect_route_is_total_over_parser_recipients` drives -/// this function with the parser's own accepted phrasings, so the residual arm -/// cannot become reachable without a test failure in every build profile. +/// to eliminate. The parser-facing regression below covers the currently +/// supported phrasings; this match remains the release-mode fail-closed boundary +/// for any future parser expansion. fn durable_redirect_route_for_filter(filter: &TargetFilter) -> PreventionShieldRoute { match filter { // "...is dealt to ~ instead" — the shield host itself. @@ -1979,6 +1979,21 @@ fn redirect_damage_event( }); } + // CR 615.7: a finite shield must deplete by each point it prevents. A + // `Continuous` redirection has no depletion lifecycle, so this otherwise + // representable pair would redirect `Next(n)` from every event forever in + // release builds. Refuse the malformed replacement before resolving a + // recipient or mutating its shield: leave the proposed damage untouched. + if matches!(redirect_amount, PreventionAmount::Next(_)) && !consume_after_redirect { + return ApplyResult::Modified(ProposedEvent::Damage { + source_id, + target, + amount: damage_amount, + is_combat, + applied, + }); + } + let chosen = redirect_chosen_object_for_rid(state, rid); let new_recipient = super::effects::create_damage_replacement::resolve_redirect_recipient( state, recipient, rid.source, chosen, @@ -2026,18 +2041,9 @@ fn redirect_damage_event( unreachable!("PreventionAmount::AllBut is never assigned to a ShieldKind::Redirection") } PreventionAmount::Next(n) => { - // CR 614.5: "the next N damage" is one-opportunity grammar, and the - // depletion bookkeeping below runs only under `consume_after_redirect`. - // A `Continuous` shield would therefore never spend `n` and would - // redirect up to N damage from EVERY event in its window. No parser - // path produces that pairing (the durable Prevention gate always - // passes `All`), but the type permits it and the failure is silent — - // so it is asserted rather than assumed, mirroring the `AllBut` arm. - debug_assert!( - consume_after_redirect, - "CR 614.5: PreventionAmount::Next must not pair with \ - RedirectionLifetime::Continuous — the amount would never deplete" - ); + // The invalid continuous pair returned above. A remaining `Next(n)` + // is therefore a one-opportunity shield and must deplete by the + // redirected amount (CR 615.7). let redirected_amount = damage_amount.min(n); let remaining_amount = damage_amount.saturating_sub(redirected_amount); if consume_after_redirect { @@ -10260,20 +10266,18 @@ mod tests { ReplacementDefinition::new(event) } - /// CR 614.9: the durable-redirection recipient mapping is TOTAL over every - /// filter the parser's `parse_durable_redirect_recipient_filter` can emit. + /// CR 614.9: the durable-redirection recipient mapping covers the supported + /// parser recipient phrasings. /// An unmapped recipient makes the shield fail closed (the damage is dealt /// as proposed instead of being moved), which is safe but silently drops the /// card's whole ability — so the mapping must never have a hole. /// - /// The cases are driven through the REAL grammar rather than a hand-written - /// list of `TargetFilter`s: a recipient phrasing added to - /// `parse_durable_redirect_recipient_filter` without a matching arm in - /// `durable_redirect_route_for_filter` fails here, in every build profile. - /// The previous hard-coded version could not catch that — it restated the - /// mapping's own arms back to it. + /// The cases exercise the real grammar rather than constructing + /// `TargetFilter`s directly. Keep this table synchronized with parser + /// additions; the runtime residual arm remains the release-mode safety + /// boundary if a new parser recipient is not yet mapped here. #[test] - fn durable_redirect_route_is_total_over_parser_recipients() { + fn durable_redirect_route_maps_supported_parser_recipients() { use crate::parser::oracle_replacement::parse_durable_redirect_recipient_filter; // Every recipient phrasing the "... is dealt to instead" @@ -10318,6 +10322,78 @@ mod tests { ); } + #[test] + fn continuous_next_redirection_fails_closed_without_spending_the_shield() { + // CR 615.7: a finite "next N damage" shield depletes by each point it + // prevents. Pairing it with a continuous redirection has no valid + // depletion lifecycle, so the runtime must leave the damage untouched + // rather than redirecting N damage from every later event. + let mut state = GameState::new_two_player(42); + let mut source = GameObject::new( + ObjectId(10), + CardId(1), + PlayerId(1), + "Damage source".to_string(), + Zone::Battlefield, + ); + source.card_types.core_types = vec![CoreType::Creature]; + let mut chosen = GameObject::new( + ObjectId(20), + CardId(2), + PlayerId(0), + "Chosen recipient".to_string(), + Zone::Battlefield, + ); + chosen.card_types.core_types = vec![CoreType::Creature]; + state.objects.insert(ObjectId(10), source); + state.objects.insert(ObjectId(20), chosen); + state.battlefield.push_back(ObjectId(10)); + state.battlefield.push_back(ObjectId(20)); + state.pending_damage_replacements.push( + ReplacementDefinition::new(ReplacementEvent::DamageDone) + .redirection_shield( + DamageRedirectTarget::ChosenObjectTarget, + PreventionAmount::Next(2), + RedirectionLifetime::Continuous, + ) + .redirect_target(TargetFilter::SpecificObject { id: ObjectId(20) }), + ); + + let mut events = Vec::new(); + let result = replace_event( + &mut state, + ProposedEvent::Damage { + source_id: ObjectId(10), + target: TargetRef::Player(PlayerId(0)), + amount: 3, + is_combat: false, + applied: HashSet::new(), + }, + &mut events, + ); + + assert!(matches!( + result, + ReplacementResult::Execute(ProposedEvent::Damage { + target: TargetRef::Player(PlayerId(0)), + amount: 3, + .. + }) + )); + assert!(matches!( + state.pending_damage_replacements[0].shield_kind, + ShieldKind::Redirection { + amount: PreventionAmount::Next(2), + lifetime: RedirectionLifetime::Continuous, + .. + } + )); + assert!( + !state.pending_damage_replacements[0].is_consumed, + "rejecting the malformed pair must not consume or mutate its shield" + ); + } + fn search_found_execute(destination: Zone) -> AbilityDefinition { AbilityDefinition::new( AbilityKind::Spell,