From 156ba34e081057cc5be85ce066b5bf96371ef558 Mon Sep 17 00:00:00 2001 From: invalidCards <842080+invalidCards@users.noreply.github.com> Date: Wed, 9 Sep 2026 19:40:20 +0200 Subject: [PATCH 1/5] Fix formula-based damage prevention parsing --- crates/engine/src/analysis/resource.rs | 11 +- crates/engine/src/database/synthesis.rs | 12 +- crates/engine/src/game/elimination.rs | 6 + crates/engine/src/game/engine_replacement.rs | 2 + crates/engine/src/game/mana_abilities.rs | 1 + crates/engine/src/game/replacement.rs | 199 ++++++- crates/engine/src/game/sba.rs | 2 + .../src/parser/oracle_effect/imperative.rs | 38 ++ .../engine/src/parser/oracle_effect/lower.rs | 14 +- crates/engine/src/parser/oracle_nom/filter.rs | 99 +++- crates/engine/src/parser/oracle_nom/mod.rs | 1 + .../src/parser/oracle_nom/prevention.rs | 103 ++++ .../engine/src/parser/oracle_replacement.rs | 245 ++++++--- crates/engine/src/parser/oracle_tests.rs | 66 ++- crates/engine/src/types/ability.rs | 80 ++- crates/engine/src/types/game_state.rs | 5 + crates/engine/src/types/resolution.rs | 1 + .../tests/integration/cost_zone_pipeline.rs | 2 + .../integration/damage_prevention_formula.rs | 504 ++++++++++++++++++ .../issue_5902_heart_shaped_herb.rs | 41 +- crates/engine/tests/integration/main.rs | 1 + .../integration/spelunking_shockland_order.rs | 1 + docs/parser-misparse-backlog.md | 17 +- 23 files changed, 1283 insertions(+), 168 deletions(-) create mode 100644 crates/engine/src/parser/oracle_nom/prevention.rs create mode 100644 crates/engine/tests/integration/damage_prevention_formula.rs diff --git a/crates/engine/src/analysis/resource.rs b/crates/engine/src/analysis/resource.rs index 1bdb897c8e..8ba8ecf7b3 100644 --- a/crates/engine/src/analysis/resource.rs +++ b/crates/engine/src/analysis/resource.rs @@ -7018,7 +7018,8 @@ pub(crate) fn token_growth_is_observed(state: &GameState) -> bool { /// CR 614.1a: a replacement's BODY (not its `condition`) can read a projected /// player resource. `QuantityModification` variants are all fixed constants (no /// read). `DamageModification::LifeFloor` caps against a player's live life total -/// (CR 119, projected); `Plus { value }` carries a `QuantityExpr` that MAY read one +/// (CR 119, projected); `Plus { value }` and `PreventionMinus`'s live `Quantity` +/// formula carry a `QuantityExpr` that MAY read one /// — treated fail-closed. `execute` is an `AbilityDefinition` with no C0-walker /// predicate ⇒ fail-closed when present. The un-flagged `DamageModification` / /// `QuantityModification` variants are safe to omit because their outputs land in @@ -7032,7 +7033,13 @@ fn replacement_body_may_read_projected(def: &crate::types::ability::ReplacementD } matches!( def.damage_modification, - Some(DamageModification::LifeFloor { .. } | DamageModification::Plus { .. }) + Some( + DamageModification::LifeFloor { .. } + | DamageModification::Plus { .. } + | DamageModification::PreventionMinus { + value: crate::types::ability::PreventionFormula::Quantity { .. }, + } + ) ) } diff --git a/crates/engine/src/database/synthesis.rs b/crates/engine/src/database/synthesis.rs index 992adad0de..4407bb496f 100644 --- a/crates/engine/src/database/synthesis.rs +++ b/crates/engine/src/database/synthesis.rs @@ -3864,7 +3864,9 @@ pub(crate) fn entry_replacement_for_grant_static( fn build_absorb_replacement(n: u32) -> ReplacementDefinition { ReplacementDefinition::new(ReplacementEvent::DamageDone) .valid_card(TargetFilter::SelfRef) - .damage_modification(DamageModification::PreventionMinus { value: n }) + .damage_modification(DamageModification::PreventionMinus { + value: crate::types::ability::PreventionFormula::fixed(n), + }) .description(format!( "CR 702.64a: Absorb {n} — if a source would deal damage to this creature, \ prevent {n} of that damage." @@ -3880,7 +3882,9 @@ fn is_absorb_replacement(r: &ReplacementDefinition, n: u32) -> bool { && matches!(r.valid_card, Some(TargetFilter::SelfRef)) && matches!( r.damage_modification, - Some(DamageModification::PreventionMinus { value }) if value == n + Some(DamageModification::PreventionMinus { + value: crate::types::ability::PreventionFormula::Fixed(value), + }) if value == n ) } @@ -26050,7 +26054,9 @@ mod absorb_synthesis_tests { assert!( matches!( r.damage_modification, - Some(DamageModification::PreventionMinus { value: 2 }) + Some(DamageModification::PreventionMinus { + value: crate::types::ability::PreventionFormula::Fixed(2), + }) ), "CR 702.64a: prevent N (=2) of the damage (prevention provenance)" ); diff --git a/crates/engine/src/game/elimination.rs b/crates/engine/src/game/elimination.rs index f39c816d20..83fe507a67 100644 --- a/crates/engine/src/game/elimination.rs +++ b/crates/engine/src/game/elimination.rs @@ -1910,6 +1910,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, @@ -3103,6 +3104,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, @@ -3243,6 +3245,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, @@ -3296,6 +3299,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, @@ -3340,6 +3344,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, @@ -3414,6 +3419,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, diff --git a/crates/engine/src/game/engine_replacement.rs b/crates/engine/src/game/engine_replacement.rs index 74a158dc00..6b70bca151 100644 --- a/crates/engine/src/game/engine_replacement.rs +++ b/crates/engine/src/game/engine_replacement.rs @@ -3760,6 +3760,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: true, + choice_player: Some(PlayerId(0)), library_placement: None, exile_controller: None, exile_duration: None, @@ -4435,6 +4436,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, diff --git a/crates/engine/src/game/mana_abilities.rs b/crates/engine/src/game/mana_abilities.rs index a31061ce99..3429ef95ba 100644 --- a/crates/engine/src/game/mana_abilities.rs +++ b/crates/engine/src/game/mana_abilities.rs @@ -5258,6 +5258,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, diff --git a/crates/engine/src/game/replacement.rs b/crates/engine/src/game/replacement.rs index d54ff17afb..f49893ce8e 100644 --- a/crates/engine/src/game/replacement.rs +++ b/crates/engine/src/game/replacement.rs @@ -6,9 +6,10 @@ use crate::types::ability::{ AbilityCost, AbilityDefinition, CastingPermission, CombatDamageScope, ControllerRef, DamageModification, DamageRedirectTarget, DamageTargetFilter, DamageTargetPlayerScope, Duration, Effect, EffectScope, ManaSpendPermission, PermissionGrantee, - PostReplacementContinuation, PreventionAmount, QuantityExpr, QuantityModification, - RedirectionLifetime, ReplacementCondition, ReplacementDefinition, ReplacementMode, - ResolvedAbility, ShieldKind, TapStateChange, TargetFilter, TargetRef, + PostReplacementContinuation, PreventionAmount, PreventionFormula, QuantityExpr, + QuantityModification, RedirectionLifetime, ReplacementChoiceAuthority, ReplacementCondition, + ReplacementDefinition, ReplacementMode, ResolvedAbility, RoundingMode, ShieldKind, + TapStateChange, TargetFilter, TargetRef, }; use crate::types::card_type::CoreType; use crate::types::counter::CounterType; @@ -1528,13 +1529,25 @@ fn replacement_choice_player( state: &GameState, proposed: &ProposedEvent, rid: ReplacementId, -) -> PlayerId { +) -> Option { if is_commander_hand_or_library_return_replacement(rid) { - return commander_hand_or_library_return_object(state, rid.source) - .map(|obj| obj.owner) - .unwrap_or_else(|| proposed.affected_player(state)); + return Some( + commander_hand_or_library_return_object(state, rid.source) + .map(|obj| obj.owner) + .unwrap_or_else(|| proposed.affected_player(state)), + ); + } + let definition = replacement_definition_for_id(state, rid)?; + match definition.choice_authority { + ReplacementChoiceAuthority::AffectedPlayer => Some(proposed.affected_player(state)), + ReplacementChoiceAuthority::SourceController => state + .liminal_entries + .get(&rid.source) + .map(|entry| entry.object.projected()) + .or_else(|| state.objects.get(&rid.source)) + .map(replacement_source_player) + .or(definition.source_controller), } - proposed.affected_player(state) } fn replacement_mode_decline(mode: &ReplacementMode) -> Option<&AbilityDefinition> { @@ -1920,6 +1933,51 @@ fn damage_modification_for_rid( .clone() } +/// CR 615.1a + CR 107.1a: resolve a prevention formula at the moment its +/// replacement applies. A live quantity needs a replacement-controller anchor; +/// a missing anchor is deliberately a failed application rather than silently +/// reading player zero's board. +fn resolve_prevention_formula( + state: &GameState, + rid: ReplacementId, + formula: &PreventionFormula, + damage_amount: u32, +) -> Option { + match formula { + PreventionFormula::Fixed(value) => Some(*value), + PreventionFormula::Quantity { quantity } => { + let controller = if rid.source == ObjectId(0) { + state + .pending_damage_replacements + .get(rid.index) + .and_then(|replacement| replacement.source_controller) + } else { + state + .objects + .get(&rid.source) + .map(replacement_source_player) + }?; + Some( + crate::game::quantity::resolve_quantity(state, quantity, controller, rid.source) + .max(0) as u32, + ) + } + PreventionFormula::Fraction { + numerator, + denominator, + rounding, + } => { + let product = u64::from(damage_amount).saturating_mul(u64::from(*numerator)); + let denominator = u64::from(denominator.get()); + let quotient = match rounding { + RoundingMode::Down => product / denominator, + RoundingMode::Up => product.saturating_add(denominator - 1) / denominator, + }; + Some(quotient.min(u64::from(u32::MAX)) as u32) + } + } +} + /// Look up the `ShieldKind` of the matched replacement (object-hosted or pending /// registry), using the same `rid.source == ObjectId(0)` sentinel discriminator /// as `damage_modification_for_rid`. @@ -2385,12 +2443,28 @@ fn damage_done_applier( // subtraction authority for both provenances. `Minus` is plain // arithmetic (CR 614.1a); `PreventionMinus` is CR 615 prevention // provenance over the identical formula - // (`PreventionMinus { value: u32::MAX }` is the continuous + // (`PreventionMinus { value: PreventionFormula::Fixed(u32::MAX) }` is the continuous // prevent-all sentinel — yields 0 for any amount and is not // consumed; continuous, not shield-style). Only the prevention // provenance does the `DamagePrevented` bookkeeping below. - DamageModification::Minus { value } - | DamageModification::PreventionMinus { value } => amount.saturating_sub(value), + DamageModification::Minus { value } => amount.saturating_sub(value), + DamageModification::PreventionMinus { value } => { + let prevented = match resolve_prevention_formula(state, rid, &value, amount) { + Some(prevented) => prevented, + // A formula that needs an unavailable replacement authority + // must not turn into an arbitrary default amount. + None => { + return ApplyResult::Modified(ProposedEvent::Damage { + source_id, + target, + amount, + is_combat, + applied, + }) + } + }; + amount.saturating_sub(prevented) + } // CR 614.1a: Conditional — if amount < source's power, set to power. // References the replacement source's (rid.source) post-layer power. DamageModification::SetToSourcePower => { @@ -2442,7 +2516,7 @@ fn damage_done_applier( // prevention provenance of the shared `Minus` subtraction — CR 702.64 // Absorb, the bare "prevent N of that damage" statics (Heart-Shaped // Herb #5902, Sphere of Purity, Orbs of Warding, ...), and the - // `PreventionMinus { value: u32::MAX }` prevent-all sentinel. When it + // `PreventionFormula::Fixed(u32::MAX)` prevent-all sentinel. When it // actually reduces the event it prevents damage, so it performs the // same bookkeeping the `ShieldKind::Prevention` shields do (Branch 2), // with the same per-event vs post-batch binding semantics: @@ -9118,14 +9192,19 @@ fn apply_single_replacement( .then(|| proposed.clone()); // CR 614.6 + CR 614.12a: Optional `Prevent` replacements (Obstinate Familiar, - // Island Sanctuary — "you may skip that draw") suppress the event only on - // the accept (Execute) branch. Declining leaves the original event intact - // so it proceeds unmodified; `draw_applier` reads `quantity_modification` - // from the definition regardless of branch, so short-circuit here. + // Island Sanctuary — "you may skip that draw") and optional damage-prevention + // formulas (Battletide Alchemist) modify the event only on the accept + // (Execute) branch. Declining leaves the original event intact, even though + // the generic Draw/Damage appliers read their modifier from the definition + // rather than the selected branch. if matches!(branch, ReplacementBranch::Decline) { if let Some(repl_def) = repl_def_ref { if replacement_mode_is_optional(&repl_def.mode) - && repl_def.quantity_modification == Some(QuantityModification::Prevent) + && (repl_def.quantity_modification == Some(QuantityModification::Prevent) + || matches!( + repl_def.damage_modification, + Some(DamageModification::PreventionMinus { .. }) + )) { return Ok(proposed); } @@ -9556,10 +9635,15 @@ fn damage_commute_class(modification: &DamageModification) -> CommuteClass { match modification { DamageModification::Double | DamageModification::Triple => CommuteClass::Multiplicative, DamageModification::Plus { .. } => CommuteClass::Additive, - // CR 616.1: both provenances of the shared subtraction commute alike. - DamageModification::Minus { .. } | DamageModification::PreventionMinus { .. } => { - CommuteClass::Subtractive - } + DamageModification::Minus { .. } => CommuteClass::Subtractive, + // Formula evaluation may be live or rounded from the event, so it must + // keep the affected player's CR 616.1 ordering choice. + DamageModification::PreventionMinus { value } => match value { + PreventionFormula::Fixed(_) => CommuteClass::Subtractive, + PreventionFormula::Quantity { .. } | PreventionFormula::Fraction { .. } => { + CommuteClass::NonCommuting + } + }, DamageModification::SetToSourcePower | DamageModification::SetTo { .. } | DamageModification::LifeFloor { .. } => CommuteClass::NonCommuting, @@ -10108,6 +10192,7 @@ fn park_entry_controller_choice( search_found_candidates: Vec::new(), depth, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, @@ -10162,7 +10247,13 @@ fn pipeline_loop( let is_optional = replacement_is_optional(state, rid); if is_optional { - let affected = replacement_choice_player(state, &proposed, rid); + let Some(affected) = replacement_choice_player(state, &proposed, rid) else { + // An optional replacement with no authorized chooser is + // treated as declined; never default it to an unrelated player. + proposed.mark_applied(rid); + depth += 1; + continue; + }; let search_found_candidates = snapshot_search_found_candidates(state, &proposed, &candidates); state.pending_replacement = Some(PendingReplacement { @@ -10172,6 +10263,7 @@ fn pipeline_loop( search_found_candidates, depth, is_optional: true, + choice_player: Some(affected), // CR 701.24a: set by the W3 library-placement arm after parking // (the pipeline doesn't know the caller's placement here). library_placement: None, @@ -10229,6 +10321,7 @@ fn pipeline_loop( search_found_candidates, depth, is_optional: false, + choice_player: None, // CR 701.24a: set by the W3 library-placement arm after parking. library_placement: None, exile_controller: None, @@ -10477,7 +10570,16 @@ fn continue_replacement_impl( } return continue_search_found_after_decline(state, pending, rid, events); } - let payer = replacement_choice_player(state, &pending.proposed, rid); + let Some(payer) = pending + .choice_player + .or_else(|| replacement_choice_player(state, &pending.proposed, rid)) + else { + // No live or latched authority can make an optional choice. Mark it + // applied as declined and continue the ordinary replacement loop. + let mut proposed = pending.proposed; + proposed.mark_applied(rid); + return pipeline_loop(state, proposed, pending.depth + 1, registry, events); + }; // CR 614.12a: a `true` flag means this is the post-choice resume of an // accept whose `MayCost` payment paused for an interactive sub-choice // (e.g. a `DiscardChoice`). Re-park fields are captured up front so a @@ -10488,6 +10590,7 @@ fn continue_replacement_impl( let reparked_depth = pending.depth; let reparked_library_placement = pending.library_placement.clone(); let reparked_sacrifice_provenance = pending.sacrifice_provenance; + let reparked_choice_player = pending.choice_player; let mut proposed = pending.proposed.clone(); if chosen_index == 0 { if let Some((player, entry_candidates)) = entry_controller_choice(state, &proposed, rid) @@ -10573,6 +10676,7 @@ fn continue_replacement_impl( search_found_candidates: Vec::new(), depth: reparked_depth, is_optional: true, + choice_player: reparked_choice_player, library_placement: reparked_library_placement, exile_controller: None, exile_duration: None, @@ -10773,6 +10877,7 @@ fn continue_replacement_impl( pending.search_found_candidates.insert(0, selected); pending.candidates = vec![rid]; pending.is_optional = true; + pending.choice_player = Some(affected); state.pending_replacement = Some(pending); return ReplacementResult::NeedsChoice(affected); } @@ -10785,9 +10890,14 @@ fn continue_replacement_impl( // Re-park it through the same optional seam used for a lone candidate, then // re-scan the modified event so the other candidates remain available. if replacement_is_optional(state, rid) { - let affected = replacement_choice_player(state, &pending.proposed, rid); + let Some(affected) = replacement_choice_player(state, &pending.proposed, rid) else { + let mut proposed = pending.proposed; + proposed.mark_applied(rid); + return pipeline_loop(state, proposed, pending.depth + 1, registry, events); + }; pending.candidates = vec![rid]; pending.is_optional = true; + pending.choice_player = Some(affected); state.pending_replacement = Some(pending); return ReplacementResult::NeedsChoice(affected); } @@ -13485,6 +13595,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: true, + choice_player: Some(PlayerId(0)), library_placement: None, exile_controller: None, exile_duration: None, @@ -13547,6 +13658,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: true, + choice_player: Some(PlayerId(0)), library_placement: None, exile_controller: None, exile_duration: None, @@ -13630,6 +13742,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, @@ -16432,7 +16545,9 @@ mod tests { /// the stale 999. #[test] fn damage_applier_prevention_minus_stamps_per_event_amount_for_continuations() { - let repl = damage_repl(DamageModification::PreventionMinus { value: 2 }); + let repl = damage_repl(DamageModification::PreventionMinus { + value: PreventionFormula::fixed(2), + }); let mut state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); state.last_effect_count = Some(999); let mut events = Vec::new(); @@ -16476,6 +16591,35 @@ mod tests { ); } + #[test] + fn fractional_prevention_rounds_the_in_flight_damage_event() { + let repl = damage_repl(DamageModification::PreventionMinus { + value: PreventionFormula::Fraction { + numerator: 1, + denominator: std::num::NonZeroU32::new(2).expect("two is nonzero"), + rounding: RoundingMode::Up, + }, + }); + let mut state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); + let mut events = Vec::new(); + let result = damage_done_applier( + damage_event(5), + ReplacementId { + source: ObjectId(10), + index: 0, + }, + &mut state, + &mut events, + ); + assert!(matches!( + result, + ApplyResult::Modified(ProposedEvent::Damage { amount: 2, .. }) + )); + assert!(events + .iter() + .any(|event| matches!(event, GameEvent::DamagePrevented { amount: 3, .. }))); + } + /// CR 510.2 + CR 615.13: inside a combat-damage batch, `PreventionMinus` /// must defer BOTH the `DamagePrevented` emission and the /// `last_effect_count` stamp to the post-batch aggregate — it accumulates @@ -16484,7 +16628,9 @@ mod tests { /// mirroring the `Prevention::All` shield batching. #[test] fn damage_applier_prevention_minus_in_batch_defers_to_post_batch_aggregate() { - let repl = damage_repl(DamageModification::PreventionMinus { value: 2 }); + let repl = damage_repl(DamageModification::PreventionMinus { + value: PreventionFormula::fixed(2), + }); let mut state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); state.combat_prevention_tally = Some(HashMap::new()); let mut events = Vec::new(); @@ -17130,6 +17276,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, diff --git a/crates/engine/src/game/sba.rs b/crates/engine/src/game/sba.rs index f1a2fb93a8..15a33c50d5 100644 --- a/crates/engine/src/game/sba.rs +++ b/crates/engine/src/game/sba.rs @@ -4236,6 +4236,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, @@ -4409,6 +4410,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index f011c98f82..5a7e22b399 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -29,6 +29,7 @@ use crate::parser::oracle_nom::bridge::{nom_on_lower, nom_parse_lower, split_onc 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::prevention::has_event_relative_prevention_amount; 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; @@ -7007,6 +7008,20 @@ fn parse_prevent_effect(text: &str, parent_target_available: bool) -> Effect { .map(|(r, _)| r) .unwrap_or(&lower); + // CR 615.1a + CR 107.1a: an activated/spell prevention clause cannot use + // the generic one-damage fallback for an event-relative fraction. Static + // replacements lower this grammar through `PreventionFormula`; imperative + // routes that lack the required event-relative representation stay visible + // as an explicit gap. Leave `X of that damage` to the existing chain-level + // fold (Errant Minion / Power Leak), which has the preceding damage event. + if has_event_relative_prevention_amount(rest) + && nom_primitives::scan_at_word_boundaries(rest, |input| { + tag::<_, _, OracleError<'_>>("half that damage").parse(input) + }) + .is_some() + { + return Effect::unimplemented("prevent", text); + } // Determine scope: combat damage only vs all damage let scope = if nom_primitives::scan_contains(rest, "combat damage") { PreventionScope::CombatDamage @@ -11071,6 +11086,21 @@ pub(super) fn parse_imperative_family_ast( return Some(ImperativeFamilyAst::GainKeyword(effect)); } + // A delayed "each time damage is dealt" prevention formula needs both a + // repeatable event watcher and a random amount. Neither is carried by the + // ordinary one-shot `PreventDamage` effect, so fail at the outer clause + // rather than lowering its inner `prevent X` to the Next(1) fallback. + if has_event_relative_prevention_amount(lower) + && nom_primitives::scan_at_word_boundaries(lower, |input| { + tag::<_, _, OracleError<'_>>("each time ").parse(input) + }) + .is_some() + { + return Some(ImperativeFamilyAst::GainKeyword(Effect::unimplemented( + "prevent", text, + ))); + } + if all_consuming(terminated(parse_note_mana_spent_clause, opt(tag(".")))) .parse(lower.trim()) .is_ok() @@ -24621,4 +24651,12 @@ mod tests { Some(TargetFilter::Controller) ); } + + #[test] + fn unsupported_event_relative_prevention_never_becomes_next_one_damage() { + assert!(matches!( + parse_prevent_effect("Prevent half that damage, rounded down.", false), + Effect::Unimplemented { .. } + )); + } } diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index 0577885417..13c3e00e2e 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -10523,6 +10523,7 @@ pub(super) fn apply_where_x_effect_expression( // representable. Recorded here and converted to a gap node after the match // (the arms hold a mutable borrow of `effect`'s fields). let mut unbound_where_x: Option = None; + let mut unbound_prevention_where_x: Option = None; match effect { Effect::DealDamage { amount, .. } | Effect::DamageAll { amount, .. } @@ -10804,7 +10805,10 @@ pub(super) fn apply_where_x_effect_expression( crate::types::ability::PreventionAmount::All | crate::types::ability::PreventionAmount::AllBut(_) ) { - *amount_dynamic = parse_where_x_quantity_expression(expr); + match parse_where_x_quantity_expression(expr) { + Some(quantity) => *amount_dynamic = Some(quantity), + None => unbound_prevention_where_x = Some(expr.to_string()), + } } } } @@ -10897,6 +10901,14 @@ pub(super) fn apply_where_x_effect_expression( // clause DEFINED X and an unbound X survived the rewrite, report the gap. A control // with an escape hatch is not a control. // + if let Some(expression) = unbound_prevention_where_x { + *effect = Effect::unimplemented( + "prevent", + format!("prevent X of that damage, where X is {expression}"), + ); + return; + } + // The guard is keyed on the EXPRESSION, never on tree-presence of `Variable("X")`. // Some expressions legitimately bind TO the placeholder, and for those a surviving // `Variable("X")` is the CORRECT binding, not a fabrication: diff --git a/crates/engine/src/parser/oracle_nom/filter.rs b/crates/engine/src/parser/oracle_nom/filter.rs index 8545899685..9b0d43fb5d 100644 --- a/crates/engine/src/parser/oracle_nom/filter.rs +++ b/crates/engine/src/parser/oracle_nom/filter.rs @@ -642,9 +642,8 @@ pub struct ControlledPermanentsConjunct { pub source_scope: SourceExclusion, } -/// 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. +/// CR 614.1a + CR 109.1: SINGLE AUTHORITY for the controlled-permanent 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: @@ -658,21 +657,59 @@ pub struct ControlledPermanentsConjunct { /// /// 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()`. +/// but only three nouns. One combinator with composable cardinality and article +/// axes keeps those noun forms in one authority. /// -/// Composed one axis per combinator: the optional CR 109.1 "other" article, the -/// plural type noun, and the fixed " you control" suffix. +/// Composed one axis per combinator: plural cardinality (including "other" and +/// "one or more") or singular article ("a"/"another"), the corresponding type +/// noun, and the fixed " you control" suffix. Singular nouns must retain their +/// article; accepting bare "creature you control" here would make this shared +/// authority claim ungrammatical recipient text. 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")), + let (input, (permanent_type, source_scope)) = alt(( + map( + ( + opt(tag("one or more ")), + opt(tag("other ")), + 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")), + )), + ), + |(_, other, permanent_type)| { + ( + permanent_type, + if other.is_some() { + SourceExclusion::Exclude + } else { + SourceExclusion::Include + }, + ) + }, + ), + map( + ( + alt(( + value(SourceExclusion::Include, tag("a ")), + value(SourceExclusion::Exclude, tag("another ")), + )), + alt(( + value(Some(CoreType::Planeswalker), tag("planeswalker")), + value(Some(CoreType::Creature), tag("creature")), + value(Some(CoreType::Artifact), tag("artifact")), + value(Some(CoreType::Enchantment), tag("enchantment")), + value(Some(CoreType::Land), tag("land")), + value(None, tag("permanent")), + )), + ), + |(source_scope, permanent_type)| (permanent_type, source_scope), + ), )) .parse(input)?; let (input, _) = tag(" you control").parse(input)?; @@ -680,10 +717,7 @@ pub fn parse_controlled_permanents_conjunct( input, ControlledPermanentsConjunct { permanent_type, - source_scope: match other { - Some(_) => SourceExclusion::Exclude, - None => SourceExclusion::Include, - }, + source_scope, }, )) } @@ -726,6 +760,31 @@ mod tests { "the \"other\" article must reach the caller, not be opt()-discarded" ); } + + for (phrase, expected_type, expected_scope) in [ + ("a permanent you control", None, SourceExclusion::Include), + ( + "another permanent you control", + None, + SourceExclusion::Exclude, + ), + ( + "one or more creatures you control", + Some(CoreType::Creature), + SourceExclusion::Include, + ), + ( + "a creature you control", + Some(CoreType::Creature), + SourceExclusion::Include, + ), + ] { + let (rest, parsed) = parse_controlled_permanents_conjunct(phrase) + .unwrap_or_else(|_| panic!("{phrase} must parse")); + assert!(rest.is_empty(), "{phrase} must be fully consumed"); + assert_eq!(parsed.permanent_type, expected_type); + assert_eq!(parsed.source_scope, expected_scope); + } } /// Hostile: the combinator must not claim a phrase whose controller clause is @@ -737,6 +796,10 @@ mod tests { "creatures", "other stuff you control", "creature you control", + "other creature you control", + "a creatures you control", + "another creatures you control", + "one or more creature you control", ] { assert!( parse_controlled_permanents_conjunct(phrase).is_err(), diff --git a/crates/engine/src/parser/oracle_nom/mod.rs b/crates/engine/src/parser/oracle_nom/mod.rs index 05a086cda1..ad0e98a788 100644 --- a/crates/engine/src/parser/oracle_nom/mod.rs +++ b/crates/engine/src/parser/oracle_nom/mod.rs @@ -16,6 +16,7 @@ pub mod enters_under; pub mod error; pub mod filter; pub mod player_counter_difference; +pub mod prevention; pub mod primitives; pub mod quantity; pub mod return_as_aura; diff --git a/crates/engine/src/parser/oracle_nom/prevention.rs b/crates/engine/src/parser/oracle_nom/prevention.rs new file mode 100644 index 0000000000..d09f9ff771 --- /dev/null +++ b/crates/engine/src/parser/oracle_nom/prevention.rs @@ -0,0 +1,103 @@ +//! Shared grammar for prevention amounts that are relative to an in-flight +//! damage event. These forms must never fall through to a one-damage shield. + +use std::num::NonZeroU32; + +use nom::branch::alt; +use nom::bytes::complete::tag; +use nom::combinator::{map, map_opt, rest, value}; +use nom::sequence::{preceded, terminated}; +use nom::Parser; + +use crate::parser::oracle_nom::error::OracleResult; +use crate::parser::oracle_nom::primitives::parse_number; +use crate::types::ability::{PreventionFormula, RoundingMode}; + +/// CR 615.1a + CR 107.1a: parse the amount after `prevent ` when it is a +/// portion of the same damage event. The quantity-binding form deliberately +/// requires its `where X is` tail; an unbound X is not a zero or one fallback. +pub fn parse_damage_prevention_formula(input: &str) -> OracleResult<'_, PreventionFormula> { + alt(( + map( + terminated(parse_number, tag(" of that damage")), + PreventionFormula::fixed, + ), + map( + terminated(parse_number, tag(" damage that")), + PreventionFormula::fixed, + ), + value( + PreventionFormula::Fraction { + numerator: 1, + denominator: NonZeroU32::new(2).expect("2 is nonzero"), + rounding: RoundingMode::Up, + }, + tag("half that damage, rounded up"), + ), + value( + PreventionFormula::Fraction { + numerator: 1, + denominator: NonZeroU32::new(2).expect("2 is nonzero"), + rounding: RoundingMode::Down, + }, + tag("half that damage, rounded down"), + ), + map_opt( + preceded(tag("x of that damage, where x is "), rest), + |quantity| { + crate::parser::oracle_quantity::parse_cda_quantity(quantity) + .map(|quantity| PreventionFormula::Quantity { quantity }) + }, + ), + )) + .parse(input) +} + +/// Classify a prevention clause that names an event-relative amount, including +/// unsupported unbound forms. Consumers use this to produce an honest parser +/// gap instead of `PreventDamage { amount: Next(1), .. }`. +pub fn has_event_relative_prevention_amount(input: &str) -> bool { + // These are parser inputs already normalized to lowercase. `tag` keeps the + // recognition at a word boundary rather than treating an arbitrary substring + // as Oracle grammar. + crate::parser::oracle_nom::primitives::scan_at_word_boundaries(input, |candidate| { + alt(( + tag::<_, _, crate::parser::oracle_nom::error::OracleError<'_>>("x of that damage"), + tag("half that damage"), + )) + .parse(candidate) + }) + .is_some() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_fixed_fractional_and_bound_quantity_forms() { + assert!(matches!( + parse_damage_prevention_formula("3 of that damage") + .unwrap() + .1, + PreventionFormula::Fixed(3) + )); + assert!(matches!( + parse_damage_prevention_formula("half that damage, rounded up") + .unwrap() + .1, + PreventionFormula::Fraction { + rounding: RoundingMode::Up, + .. + } + )); + assert!(matches!( + parse_damage_prevention_formula( + "x of that damage, where x is the number of Clerics you control" + ) + .unwrap() + .1, + PreventionFormula::Quantity { .. } + )); + } +} diff --git a/crates/engine/src/parser/oracle_replacement.rs b/crates/engine/src/parser/oracle_replacement.rs index 338e2dd15f..5cfb0506eb 100644 --- a/crates/engine/src/parser/oracle_replacement.rs +++ b/crates/engine/src/parser/oracle_replacement.rs @@ -25,6 +25,9 @@ use super::oracle_nom::condition::{ }; use super::oracle_nom::duration::parse_duration; use super::oracle_nom::filter as nom_filter; +use super::oracle_nom::prevention::{ + has_event_relative_prevention_amount, parse_damage_prevention_formula, +}; 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; @@ -40,11 +43,11 @@ use crate::types::ability::{ CounterReplacementSubject, DamageModification, DamageRedirectTarget, DamageTargetFilter, DamageTargetPlayerScope, DieRollIgnoreRule, 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, + ManaSpendPermission, PermissionGrantee, PlayerFilter, PreventionAmount, PreventionFormula, + QuantityExpr, QuantityModification, QuantityRef, RedirectionLifetime, + ReplacementChoiceAuthority, ReplacementCondition, ReplacementDefinition, ReplacementMode, + ReplacementPlayerScope, SourceExclusion, StaticCondition, StaticDefinition, TapStateChange, + TargetFilter, TriggerDefinition, TypeFilter, TypedFilter, }; use crate::types::ability::{CardPlayMode, CastingPermission}; use crate::types::card_type::Supertype; @@ -7152,6 +7155,10 @@ pub(crate) fn parse_oneshot_damage_replacement( // `PreventDamage` resolver builds a one-shot `ShieldKind::Prevention` shield; // route the source-scoped one-shot prevention through it rather than // duplicating the shield-creation flow. + if has_event_relative_prevention_amount(result_clause) { + return Some(Effect::unimplemented("prevent", result_clause)); + } + if nom_primitives::scan_contains(result_clause, "prevent that damage") || nom_primitives::scan_contains(result_clause, "prevent the damage") { @@ -8110,10 +8117,11 @@ fn finish_damage_source_subject(subject: &str) -> Option { .map_or(subject, |(rest, _)| rest) .trim(); - // "a spell" — any spell is the source; no typed filter (Benevolent Unicorn). - // Must precede `parse_type_phrase_folding`, which maps bare "spell" to Card. + // "a spell" is a source-category restriction, not an untyped source. Must + // precede `parse_type_phrase_folding`, which maps bare "spell" to Card. + // `StackSpell` excludes permanent and activated/triggered-ability damage. if subject == "spell" { - return None; + return Some(TargetFilter::StackSpell); } // "a source" / "sources" with no qualifier — no filter needed (matches any source). @@ -8487,8 +8495,9 @@ fn parse_damage_target_phrase( // 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 + // The connector is semantically a union for a damage event: "and", + // "or", and "and/or" all introduce the permanent leg. The noun + // phrase is NOT re-spelled here; 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` → @@ -8496,19 +8505,9 @@ fn parse_damage_target_phrase( // 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 "), + alt((tag("to you and "), tag("to you or "), tag("to you and/or "))), nom_filter::parse_controlled_permanents_conjunct, ), |conjunct| DamageTargetFilter::PlayerOrPermanentsControlledBy { @@ -11889,7 +11888,7 @@ fn parse_damage_prevention_replacement( // outside the prevention bookkeeping. enum PreventionRepr { Shield(PreventionAmount), - Reduce(u32), + Reduce(PreventionFormula), } let repr = if let Some((after_all_but, _)) = after_prevent.and_then(|s| tag::<_, _, OracleError<'_>>("all but ").parse(s).ok()) @@ -11918,12 +11917,9 @@ fn parse_damage_prevention_replacement( // stays with the chunk-level where-X machinery. Any miss (no number, or // no adjacent " of that damage" anchor) means this is not a recognized // prevention pattern, so `?` bails the whole parse. - let n = after_prevent.and_then(|s| { - nom_parse_lower(s, |i| { - terminated(nom_primitives::parse_number, tag(" of that damage")).parse(i) - }) - })?; - PreventionRepr::Reduce(n) + let formula = + after_prevent.and_then(|s| nom_parse_lower(s, parse_damage_prevention_formula))?; + PreventionRepr::Reduce(formula) }; // --- 2. Extract combat scope --- @@ -11944,7 +11940,14 @@ fn parse_damage_prevention_replacement( // controller or a spell target slot) — that signal gates the follow-up // object/owner-anaphor rewrite in step 5 below. let (damage_target_filter, recipient_from_event): (Option, bool) = - if nom_primitives::scan_contains(working_lower, "dealt to you") + if let Some(tf @ DamageTargetFilter::PlayerOrPermanentsControlledBy { .. }) = + parse_damage_recipient_scope(working_lower) + { + // Keep compound player/permanent recipients ahead of the bare + // controller scan: "to you or another permanent you control" is + // one recipient domain, not a player-only shield. + (Some(tf), false) + } else if nom_primitives::scan_contains(working_lower, "dealt to you") || nom_primitives::scan_contains(working_lower, "deal to you") { // CR 615.1a: Recipient is the shield controller; not an event anaphor. @@ -12059,11 +12062,20 @@ fn parse_damage_prevention_replacement( // every qualifying event, and emitting `DamagePrevented` bookkeeping // (which plain-arithmetic `Minus`, e.g. Benevolent Unicorn's "minus 1", // must not). - PreventionRepr::Reduce(n) => { - def.damage_modification(DamageModification::PreventionMinus { value: n }) + PreventionRepr::Reduce(value) => { + def.damage_modification(DamageModification::PreventionMinus { value }) } }; + // CR 615.1a: "you may prevent" is an optional prevention replacement; + // the modal choice belongs to the ability's controller, while the separate + // CR 616.1 ordering choice remains with the affected player. + if nom_primitives::scan_contains(working_lower, "you may prevent ") { + def = def + .mode(ReplacementMode::Optional { decline: None }) + .choice_authority(ReplacementChoiceAuthority::SourceController); + } + if let Some(cs) = combat_scope { def = def.combat_scope(cs); } @@ -15218,7 +15230,9 @@ mod tests { assert_eq!( def.damage_modification, - Some(DamageModification::PreventionMinus { value: 1 }), + Some(DamageModification::PreventionMinus { + value: PreventionFormula::Fixed(1), + }), "bare 'prevent 1 of that damage' must install a continuous \ PreventionMinus(1) modification (prevention provenance of the \ shared Minus subtraction), not fall through unparsed" @@ -15538,7 +15552,9 @@ mod tests { assert_eq!( def.damage_modification, - Some(DamageModification::PreventionMinus { value: 2 }) + Some(DamageModification::PreventionMinus { + value: PreventionFormula::Fixed(2), + }) ); assert_eq!(def.shield_kind, ShieldKind::None); assert!( @@ -20723,7 +20739,7 @@ mod tests { def.damage_modification, Some(DamageModification::Minus { value: 1 }) ); - assert_eq!(def.damage_source_filter, None); // "a spell" → no source filter + assert_eq!(def.damage_source_filter, Some(TargetFilter::StackSpell)); assert_eq!(def.damage_target_filter, None); // "permanent or player" = any } @@ -22482,38 +22498,27 @@ mod tests { } #[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", - ), + fn damage_target_phrase_composes_player_and_permanent_connectors() { + // Each damage event has exactly one recipient, so "you and/or one or + // more creatures you control" has the same per-event recipient domain + // as the existing player-or-controlled-permanents representation. + for phrase in [ + "to you and/or permanents you control", + "to you and/or creatures you control", ] { let (rest, filter) = - parse_damage_target_phrase(phrase).expect("the bare \"to you\" arm still matches"); - assert_eq!( + parse_damage_target_phrase(phrase).expect("the and/or conjunct must parse"); + assert!(rest.is_empty(), "{phrase} must be fully consumed"); + assert!(matches!( 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" - ); + DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + .. + } + )); } - // Paired positive: the space-separated spelling DOES reach the new arm. + // The space-separated spelling reaches the same shared authority. 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!( @@ -26917,6 +26922,7 @@ mod snapshot_tests { mod opposition_agent_parser_tests { use super::*; use crate::types::ability::{CastingPermission, ManaSpendPermission, PermissionGrantee}; + use crate::types::card_type::CoreType; use crate::types::statics::{CastFrequency, ProhibitionScope, StaticMode}; const REPLACEMENT_TEXT: &str = "While an opponent is searching their library, they exile each card they find. You may play those cards for as long as they remain exiled, and you may spend mana as though it were mana of any color to cast them."; @@ -27070,4 +27076,121 @@ mod opposition_agent_parser_tests { .iter() .any(|ability| matches!(ability.effect.as_ref(), Effect::Unimplemented { .. }))); } + + #[test] + fn event_relative_prevention_cards_keep_their_formula_and_scope() { + let gisela = parse_replacement_line( + "If a source would deal damage to you or a permanent you control, prevent half that damage, rounded up.", + "Gisela, Blade of Goldnight", + ) + .expect("Gisela prevention replacement"); + assert!(matches!( + gisela.damage_modification, + Some(DamageModification::PreventionMinus { + value: PreventionFormula::Fraction { + rounding: crate::types::ability::RoundingMode::Up, + .. + } + }) + )); + assert!(matches!( + gisela.damage_target_filter, + Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + source_scope: SourceExclusion::Include, + .. + }) + )); + + let battletide = parse_replacement_line( + "If a source would deal damage to a player, you may prevent X of that damage, where X is the number of Clerics you control.", + "Battletide Alchemist", + ) + .expect("Battletide prevention replacement"); + assert!(matches!( + battletide.damage_modification, + Some(DamageModification::PreventionMinus { + value: PreventionFormula::Quantity { .. } + }) + )); + assert!(matches!(battletide.mode, ReplacementMode::Optional { .. })); + assert_eq!( + battletide.choice_authority, + ReplacementChoiceAuthority::SourceController + ); + } + + #[test] + fn spell_source_and_complete_recipient_domains_do_not_widen() { + let rem = parse_replacement_line( + "If a spell would deal damage to you or another permanent you control, prevent that damage.", + "Rem Karolus, Stalwart Slayer", + ) + .expect("Rem Karolus prevention replacement"); + assert_eq!(rem.damage_source_filter, Some(TargetFilter::StackSpell)); + assert!(matches!( + rem.damage_target_filter, + Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + source_scope: SourceExclusion::Exclude, + .. + }) + )); + + let rem_bonus = parse_replacement_line( + "If a spell would deal damage to an opponent or a permanent an opponent controls, it deals that much damage plus 1 instead.", + "Rem Karolus, Stalwart Slayer", + ) + .expect("Rem Karolus damage bonus replacement"); + assert_eq!( + rem_bonus.damage_source_filter, + Some(TargetFilter::StackSpell) + ); + assert_eq!( + rem_bonus.damage_target_filter, + Some(damage_target_opponent_or_permanents()) + ); + + let plated = parse_replacement_line( + "If a spell would deal damage to a permanent or player, prevent 1 damage that spell would deal to that permanent or player.", + "Plated Pegasus", + ) + .expect("Plated Pegasus prevention replacement"); + assert_eq!(plated.damage_source_filter, Some(TargetFilter::StackSpell)); + assert_eq!( + plated.damage_modification, + Some(DamageModification::PreventionMinus { + value: PreventionFormula::Fixed(1), + }) + ); + assert_eq!(plated.damage_target_filter, None); + } + + #[test] + fn cardinality_recipient_syntax_uses_the_static_replacement_path() { + let def = parse_replacement_line( + "If a creature would deal combat damage to you and/or one or more creatures you control, prevent X of that damage, where X is the number of age counters on this enchantment.", + "Cover of Winter", + ) + .expect("Cover of Winter prevention replacement"); + assert_eq!(def.combat_scope, Some(CombatDamageScope::CombatOnly)); + assert_eq!( + def.damage_target_filter, + Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: Some(CoreType::Creature), + source_scope: SourceExclusion::Include, + }) + ); + assert_eq!( + def.damage_source_filter, + Some(TargetFilter::Typed(TypedFilter::creature())) + ); + assert!(matches!( + def.damage_modification, + Some(DamageModification::PreventionMinus { + value: PreventionFormula::Quantity { .. } + }) + )); + } } diff --git a/crates/engine/src/parser/oracle_tests.rs b/crates/engine/src/parser/oracle_tests.rs index 01a7de58c5..c2df1faf94 100644 --- a/crates/engine/src/parser/oracle_tests.rs +++ b/crates/engine/src/parser/oracle_tests.rs @@ -13713,14 +13713,14 @@ fn prevent_all_combat_damage() { #[test] fn prevent_dynamic_amount_where_x_is_counters() { - use crate::types::ability::{ObjectScope, PreventionAmount, QuantityExpr, QuantityRef}; + use crate::types::ability::{ + CombatDamageScope, DamageModification, DamageTargetFilter, DamageTargetPlayerScope, + PreventionFormula, SourceExclusion, TypedFilter, + }; use crate::types::counter::CounterType; - // Cover of Winter class: "prevent X … where X is the number of age - // counters on this enchantment". The chunk machinery strips the - // trailing "where x is …" binding and `apply_where_x_effect_expression` - // re-applies it onto `Effect::PreventDamage::amount_dynamic`. Driven - // through the full `parse` path because the chunk-level where-X - // mechanism does not run inside the single-clause `parse_effect`. + // Cover of Winter's static damage prevention installs its own dynamic + // `PreventionFormula`; the full parser must retain the creature source, + // combat-only scope, and every per-event recipient category. let parsed = parse( "If a creature would deal combat damage to you and/or one or more creatures \ you control, prevent X of that damage, where X is the number of age counters \ @@ -13730,26 +13730,38 @@ fn prevent_dynamic_amount_where_x_is_counters() { &["Snow", "Enchantment"], &[], ); - let prevent = parsed - .abilities - .iter() - .find(|a| matches!(&*a.effect, Effect::PreventDamage { .. })) - .expect("expected a PreventDamage ability"); - match &*prevent.effect { - Effect::PreventDamage { - amount: PreventionAmount::Next(1), - amount_dynamic: - Some(QuantityExpr::Ref { - qty: - QuantityRef::CountersOn { - scope: ObjectScope::Source, - counter_type: Some(ct), - }, - }), - .. - } => assert_eq!(*ct, CounterType::Age), - other => panic!("expected PreventDamage with dynamic age counters, got {other:?}"), - } + let [replacement] = parsed.replacements.as_slice() else { + panic!("expected one Cover of Winter replacement, got {parsed:#?}"); + }; + assert_eq!( + replacement.combat_scope, + Some(CombatDamageScope::CombatOnly) + ); + assert_eq!( + replacement.damage_target_filter, + Some(DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: Some(CoreType::Creature), + source_scope: SourceExclusion::Include, + }) + ); + assert_eq!( + replacement.damage_source_filter, + Some(TargetFilter::Typed(TypedFilter::creature())) + ); + assert!(matches!( + &replacement.damage_modification, + Some(DamageModification::PreventionMinus { + value: PreventionFormula::Quantity { + quantity: QuantityExpr::Ref { + qty: QuantityRef::CountersOn { + scope: ObjectScope::Source, + counter_type: Some(CounterType::Age), + }, + }, + }, + }) + )); assert!( parsed .parse_warnings diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 6614ea7a9b..88481b02b3 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -1,5 +1,6 @@ use std::collections::HashSet; use std::fmt; +use std::num::NonZeroU32; use std::ops::ControlFlow; use std::sync::Arc; @@ -28028,14 +28029,14 @@ pub enum DamageModification { /// shared `Minus` applier arm: subtraction is applied by the SAME match arm /// as `Minus`, and only this provenance additionally emits `DamagePrevented` /// bookkeeping plus the CR 615.5 prevented-amount handoff for - /// "damage prevented this way" continuations. A `value` of `u32::MAX` is + /// "damage prevented this way" continuations. A fixed value of `u32::MAX` is /// the continuous prevent-all sentinel (saturating-subtraction yields 0 for /// any amount; the replacement is not consumed — continuous, not /// shield-style, distinct from `ShieldKind::Prevention { All }`). /// /// Provenance is a sibling variant rather than a field on `Minus` to /// preserve the established `Minus { value }` construction shape. - PreventionMinus { value: u32 }, + PreventionMinus { value: PreventionFormula }, /// CR 614.1a: Conditional — if amount < source's power, set amount = source's power. /// References the replacement source's (not the damage source's) current post-layer power. /// Used by Ojer Axonil: "deals damage equal to ~'s power instead." @@ -28054,6 +28055,33 @@ pub enum DamageModification { LifeFloor { minimum: i32 }, } +/// CR 615.1a + CR 107.1a: amount removed from each matching damage event. +/// +/// The numeric `Fixed` form serializes as the legacy bare value inside +/// `DamageModification::PreventionMinus`, so existing card data continues to +/// load and round-trip unchanged. `Quantity` is evaluated when the replacement +/// applies; `Fraction` is evaluated from the in-flight damage event, not from a +/// game-state quantity. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(untagged)] +pub enum PreventionFormula { + Fixed(u32), + Quantity { + quantity: QuantityExpr, + }, + Fraction { + numerator: u32, + denominator: NonZeroU32, + rounding: RoundingMode, + }, +} + +impl PreventionFormula { + pub const fn fixed(value: u32) -> Self { + Self::Fixed(value) + } +} + /// CR 614.1a: Quantity modification for replacement effects (tokens, counters). /// Modeled after DamageModification but for non-damage quantities. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -28312,6 +28340,26 @@ pub enum ReplacementMode { }, } +/// Authority for an optional replacement's accept/decline prompt. +/// CR 109.5 assigns a source's "you may" choice to that source's controller; +/// CR 616.1 separately assigns the affected player the ordering of multiple +/// applicable replacement or prevention effects. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +pub enum ReplacementChoiceAuthority { + /// The affected player, used by the default optional-replacement prompt. + #[default] + AffectedPlayer, + /// CR 109.5: "you" in a source's optional replacement text means that + /// source's controller (for example, "you may prevent"). + SourceController, +} + +impl ReplacementChoiceAuthority { + pub const fn is_affected_player(&self) -> bool { + matches!(self, Self::AffectedPlayer) + } +} + /// CR 614.6 + CR 615.5: Continuation effect that runs after a replacement /// effect's modifications complete. Stashed by the replacement pipeline, /// drained by callers (`engine_replacement`, `stack`, `deal_damage`, @@ -28378,6 +28426,15 @@ pub struct ReplacementDefinition { pub runtime_execute: Option>, #[serde(default)] pub mode: ReplacementMode, + /// CR 109.5: an optional "you may prevent" choice belongs to the + /// replacement source's controller. Defaults to the affected player for + /// compatibility; CR 616.1 still governs ordering multiple applicable + /// replacement or prevention effects. + #[serde( + default, + skip_serializing_if = "ReplacementChoiceAuthority::is_affected_player" + )] + pub choice_authority: ReplacementChoiceAuthority, #[serde(default)] pub valid_card: Option, #[serde(default)] @@ -28719,6 +28776,7 @@ impl ReplacementDefinition { execute: None, runtime_execute: None, mode: ReplacementMode::Mandatory, + choice_authority: ReplacementChoiceAuthority::AffectedPlayer, valid_card: None, description: None, condition: None, @@ -28770,6 +28828,11 @@ impl ReplacementDefinition { self } + pub fn choice_authority(mut self, authority: ReplacementChoiceAuthority) -> Self { + self.choice_authority = authority; + self + } + pub fn valid_card(mut self, filter: TargetFilter) -> Self { self.valid_card = Some(filter); self @@ -33945,6 +34008,19 @@ mod tests { assert_eq!(replacement, deserialized); } + #[test] + fn prevention_formula_keeps_legacy_fixed_json_shape() { + let legacy = r#"{"type":"PreventionMinus","value":2}"#; + let formula: DamageModification = serde_json::from_str(legacy).unwrap(); + assert_eq!( + formula, + DamageModification::PreventionMinus { + value: PreventionFormula::Fixed(2), + } + ); + assert_eq!(serde_json::to_string(&formula).unwrap(), legacy); + } + #[test] fn target_filter_nested_roundtrip() { let filter = TargetFilter::And { diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 8d441fe4a2..280b300c95 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -21064,6 +21064,11 @@ pub struct PendingReplacement { /// `candidates` has exactly one entry (the real replacement); decline is synthetic. #[serde(default)] pub is_optional: bool, + /// Choice authority captured when an optional replacement is offered. This + /// is deliberately separate from CR 616 ordering, whose chooser remains + /// the affected player. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub choice_player: Option, /// CR 701.24a: the library placement requested by the original `move_object` /// call whose replacement consult parked here (W3 library-placement arm only). /// `Some` solely for a parked Library-targeting `ZoneChange`; the resume path diff --git a/crates/engine/src/types/resolution.rs b/crates/engine/src/types/resolution.rs index 7d5fe73354..2bacb98c1c 100644 --- a/crates/engine/src/types/resolution.rs +++ b/crates/engine/src/types/resolution.rs @@ -8087,6 +8087,7 @@ mod tests { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, diff --git a/crates/engine/tests/integration/cost_zone_pipeline.rs b/crates/engine/tests/integration/cost_zone_pipeline.rs index ee9aa34c73..8651c5619c 100644 --- a/crates/engine/tests/integration/cost_zone_pipeline.rs +++ b/crates/engine/tests/integration/cost_zone_pipeline.rs @@ -973,6 +973,7 @@ fn stage_prevented_cost_move(state: &mut GameState, source: engine::types::ident search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, @@ -4730,6 +4731,7 @@ fn effect_pay_cost_composite_mana_life_prevention_serializes_and_rides_once() { search_found_candidates: Vec::new(), depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, diff --git a/crates/engine/tests/integration/damage_prevention_formula.rs b/crates/engine/tests/integration/damage_prevention_formula.rs new file mode 100644 index 0000000000..17756f23f6 --- /dev/null +++ b/crates/engine/tests/integration/damage_prevention_formula.rs @@ -0,0 +1,504 @@ +//! End-to-end regressions for event-relative damage prevention formulas. +//! +//! These tests seed the printed Oracle text, then drive the normal damage or +//! cast pipeline. They deliberately distinguish replacement choice authority +//! from the affected player's replacement ordering authority. + +use engine::game::combat::AttackTarget; +use engine::game::effects::deal_damage; +use engine::game::game_object::AttachTarget; +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::parser::oracle::parse_oracle_text; +use engine::types::ability::{ + DamageModification, Effect, QuantityExpr, ReplacementDefinition, ResolvedAbility, TargetFilter, + TargetRef, +}; +use engine::types::actions::GameAction; +use engine::types::card_type::CoreType; +use engine::types::counter::CounterType; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::mana::ManaCost; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; +use engine::types::replacements::ReplacementEvent; + +const GISELA: &str = + "If a source would deal damage to you or a permanent you control, prevent half that damage, rounded up."; +const BATTLETIDE: &str = + "If a source would deal damage to a player, you may prevent X of that damage, where X is the number of Clerics you control."; +const REM: &str = + "If a spell would deal damage to you or another permanent you control, prevent that damage."; +const PLATED_PEGASUS: &str = + "If a spell would deal damage to a permanent or player, prevent 1 damage that spell would deal to that permanent or player."; +const SHIELD_OF_THE_RIGHTEOUS: &str = + "If a source would deal damage to equipped creature, prevent X of that damage, where X is the number of creatures you control."; +const COVER_OF_WINTER: &str = "Cumulative upkeep {S} (At the beginning of your upkeep, put an age counter on this permanent, then sacrifice it unless you pay its upkeep cost for each age counter on it. {S} can be paid with one mana from a snow source.)\nIf a creature would deal combat damage to you and/or one or more creatures you control, prevent X of that damage, where X is the number of age counters on this enchantment.\n{S}: Put an age counter on this enchantment."; +const BENEVOLENT_UNICORN: &str = + "If a spell would deal damage to a permanent or player, it deals that much damage minus 1 to that permanent or player instead."; +const DAMAGE_SPELL: &str = "This spell deals 3 damage to target creature or player."; + +fn damage_ability( + source_id: ObjectId, + controller: PlayerId, + 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, + controller, + ) +} + +fn set_priority(runner: &mut GameRunner, player: PlayerId) { + let state = runner.state_mut(); + state.active_player = player; + state.priority_player = player; + state.waiting_for = WaitingFor::Priority { player }; +} + +fn choose_source_candidate(runner: &mut GameRunner, source: ObjectId) { + let index = runner + .state() + .pending_replacement + .as_ref() + .expect("damage replacement choice must be parked") + .candidates + .iter() + .position(|candidate| candidate.source == source) + .expect("the requested replacement source must be offered"); + runner + .act(GameAction::ChooseReplacement { index }) + .expect("choosing the requested replacement must succeed"); +} + +#[test] +fn gisela_rounds_up_and_affected_player_orders_against_a_doubler() { + let mut scenario = GameScenario::new(); + let gisela = scenario + .add_creature_from_oracle(P0, "Gisela, Blade of Goldnight", 5, 5, GISELA) + .id(); + let doubler = scenario.add_creature(P1, "Damage Doubler", 2, 2).id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + runner + .state_mut() + .objects + .get_mut(&doubler) + .unwrap() + .replacement_definitions + .push( + ReplacementDefinition::new(ReplacementEvent::DamageDone) + .damage_modification(DamageModification::Double), + ); + + let before = runner.life(P0); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, P1, TargetRef::Player(P0), 5), + &mut events, + ) + .expect("damage must reach replacement processing"); + + match runner.state().waiting_for { + WaitingFor::ReplacementChoice { player, .. } => assert_eq!( + player, P0, + "the damaged player, rather than a replacement controller, orders noncommuting replacements" + ), + ref other => panic!("expected a material replacement ordering choice, got {other:?}"), + } + choose_source_candidate(&mut runner, gisela); + assert_eq!( + runner.life(P0), + before - 4, + "preventing ceil(5 / 2) first leaves 2 damage, then the doubler makes 4" + ); + + let mut scenario = GameScenario::new(); + let gisela = scenario + .add_creature_from_oracle(P0, "Gisela, Blade of Goldnight", 5, 5, GISELA) + .id(); + let doubler = scenario.add_creature(P1, "Damage Doubler", 2, 2).id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + runner + .state_mut() + .objects + .get_mut(&doubler) + .unwrap() + .replacement_definitions + .push( + ReplacementDefinition::new(ReplacementEvent::DamageDone) + .damage_modification(DamageModification::Double), + ); + let before = runner.life(P0); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, P1, TargetRef::Player(P0), 5), + &mut events, + ) + .expect("damage must reach replacement processing"); + choose_source_candidate(&mut runner, doubler); + assert_eq!( + runner.life(P0), + before - 5, + "doubling first makes 10 damage, then Gisela prevents 5 rounded up" + ); + assert!( + runner.state().objects[&gisela] + .replacement_definitions + .len() + == 1, + "reach guard: Gisela's printed static replacement must be present" + ); +} + +#[test] +fn battletide_controller_chooses_optional_prevention_after_affected_player_orders() { + let mut scenario = GameScenario::new(); + let battletide = scenario + .add_creature_from_oracle(P0, "Battletide Alchemist", 3, 4, BATTLETIDE) + .with_subtypes(vec!["Cleric"]) + .id(); + scenario + .add_creature(P0, "Supporting Cleric", 1, 3) + .with_subtypes(vec!["Cleric"]); + let doubler = scenario.add_creature(P1, "Damage Doubler", 2, 2).id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + runner + .state_mut() + .objects + .get_mut(&doubler) + .unwrap() + .replacement_definitions + .push( + ReplacementDefinition::new(ReplacementEvent::DamageDone) + .damage_modification(DamageModification::Double), + ); + set_priority(&mut runner, P0); + + let before = runner.life(P1); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, P1, TargetRef::Player(P1), 5), + &mut events, + ) + .expect("damage must reach replacement processing"); + match runner.state().waiting_for { + WaitingFor::ReplacementChoice { player, .. } => assert_eq!( + player, P1, + "the affected player orders Battletide and the doubler under CR 616" + ), + ref other => panic!("expected ordering prompt, got {other:?}"), + } + choose_source_candidate(&mut runner, doubler); + match runner.state().waiting_for { + WaitingFor::ReplacementChoice { player, .. } => assert_eq!( + player, P0, + "Battletide's optional accept/decline belongs to its controller, not the damaged player" + ), + ref other => panic!("expected Battletide optional prompt, got {other:?}"), + } + runner + .act(GameAction::ChooseReplacement { index: 0 }) + .expect("Battletide controller can accept prevention"); + assert_eq!( + runner.life(P1), + before - 8, + "two live Clerics prevent 2 from the doubled 10-damage event" + ); + assert!( + runner.state().objects[&battletide] + .replacement_definitions + .len() + == 1, + "reach guard: Battletide's printed static replacement must be present" + ); +} + +#[test] +fn battletide_decline_leaves_the_original_damage_untouched() { + let mut scenario = GameScenario::new(); + scenario + .add_creature_from_oracle(P0, "Battletide Alchemist", 3, 4, BATTLETIDE) + .with_subtypes(vec!["Cleric"]); + scenario + .add_creature(P0, "Supporting Cleric", 1, 3) + .with_subtypes(vec!["Cleric"]); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + set_priority(&mut runner, P0); + let before = runner.life(P1); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, P1, TargetRef::Player(P1), 5), + &mut events, + ) + .expect("damage must reach the optional replacement"); + match runner.state().waiting_for { + WaitingFor::ReplacementChoice { player, .. } => assert_eq!(player, P0), + ref other => panic!("expected Battletide controller prompt, got {other:?}"), + } + runner + .act(GameAction::ChooseReplacement { index: 1 }) + .expect("Battletide controller can decline prevention"); + assert_eq!(runner.life(P1), before - 5); +} + +#[test] +fn rem_and_plated_apply_only_to_spells_and_keep_their_recipient_scopes() { + let mut scenario = GameScenario::new(); + let rem = scenario + .add_creature_from_oracle(P0, "Rem Karolus, Stalwart Slayer", 3, 4, REM) + .id(); + let ally = scenario.add_creature(P0, "Protected Ally", 1, 5).id(); + let spell_to_rem = scenario + .add_spell_to_hand_from_oracle(P1, "Spell to Rem", true, DAMAGE_SPELL) + .with_mana_cost(ManaCost::generic(0)) + .id(); + let spell_to_ally = scenario + .add_spell_to_hand_from_oracle(P1, "Spell to Ally", true, DAMAGE_SPELL) + .with_mana_cost(ManaCost::generic(0)) + .id(); + let permanent_source = scenario.add_creature(P1, "Permanent Source", 3, 3).id(); + let mut runner = scenario.build(); + set_priority(&mut runner, P1); + runner.cast(spell_to_rem).target_object(rem).resolve(); + assert_eq!( + runner.state().objects[&rem].damage_marked, + 3, + "Rem's 'another permanent' clause must exclude Rem itself" + ); + set_priority(&mut runner, P1); + runner.cast(spell_to_ally).target_object(ally).resolve(); + assert_eq!( + runner.state().objects[&ally].damage_marked, + 0, + "a spell's damage to another permanent the controller owns is prevented" + ); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(permanent_source, P1, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("permanent-source damage must resolve"); + assert_eq!( + runner.life(P0), + 17, + "Rem must not prevent damage from a permanent or ability source" + ); + + let mut scenario = GameScenario::new(); + scenario.add_creature_from_oracle(P0, "Plated Pegasus", 1, 1, PLATED_PEGASUS); + let spell = scenario + .add_spell_to_hand_from_oracle(P1, "Spell", true, DAMAGE_SPELL) + .with_mana_cost(ManaCost::generic(0)) + .id(); + let permanent_source = scenario.add_creature(P1, "Permanent Source", 3, 3).id(); + let mut runner = scenario.build(); + set_priority(&mut runner, P1); + let before = runner.life(P0); + let outcome = runner.cast(spell).target_player(P0).resolve(); + assert_eq!( + outcome.life_delta(P0), + -2, + "Plated Pegasus prevents one spell damage" + ); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(permanent_source, P1, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("permanent-source damage must resolve"); + assert_eq!( + runner.life(P0), + before - 5, + "Plated Pegasus must not affect nonspells" + ); +} + +#[test] +fn shield_formula_uses_the_equipped_recipient_and_live_creature_count() { + let mut scenario = GameScenario::new(); + let shield = scenario + .add_creature_from_oracle(P0, "Shield", 0, 1, SHIELD_OF_THE_RIGHTEOUS) + .id(); + let equipped = scenario.add_creature(P0, "Equipped", 2, 7).id(); + let unrelated = scenario.add_creature(P0, "Unrelated", 2, 7).id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + { + let object = runner.state_mut().objects.get_mut(&shield).unwrap(); + object.card_types.core_types = vec![CoreType::Artifact]; + object.card_types.subtypes = vec!["Equipment".to_string()]; + object.base_card_types = object.card_types.clone(); + object.power = None; + object.toughness = None; + object.base_power = None; + object.base_toughness = None; + object.attached_to = Some(AttachTarget::Object(equipped)); + } + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, P1, TargetRef::Object(equipped), 5), + &mut events, + ) + .expect("damage to equipped creature must resolve"); + assert_eq!(runner.state().objects[&equipped].damage_marked, 3); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, P1, TargetRef::Object(unrelated), 5), + &mut events, + ) + .expect("damage to unrelated creature must resolve"); + assert_eq!(runner.state().objects[&unrelated].damage_marked, 5); +} + +/// CR 120.2a + CR 615.1a: Cover of Winter's continuous prevention formula +/// applies to combat damage dealt to a creature its controller controls, but +/// not to otherwise-identical noncombat damage. +#[test] +fn cover_of_winter_prevents_live_age_counter_formula_only_for_combat_damage() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let cover = scenario + .add_enchantment_from_oracle(P0, "Cover of Winter", COVER_OF_WINTER) + .id(); + scenario.with_counter(cover, CounterType::Age, 2); + let protected_creature = scenario.add_creature(P0, "Protected Creature", 1, 10).id(); + let attacker = scenario.add_creature(P1, "Hostile Attacker", 4, 6).id(); + let mut runner = scenario.build(); + set_priority(&mut runner, P1); + + runner.advance_to_combat(); + runner + .declare_attackers(&[(attacker, AttackTarget::Player(P0))]) + .expect("attacker declaration must succeed"); + if matches!(runner.state().waiting_for, WaitingFor::Priority { .. }) { + runner.pass_both_players(); + } + runner + .declare_blockers(&[(protected_creature, attacker)]) + .expect("blocker declaration must succeed"); + runner.combat_damage(); + + assert_eq!( + runner.state().objects[&protected_creature].damage_marked, + 2, + "two age counters must prevent two of the attacker's four combat damage" + ); + assert!( + runner.state().objects[&cover].replacement_definitions.len() == 1, + "reach guard: Cover of Winter's printed static replacement must be present" + ); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(attacker, P1, TargetRef::Object(protected_creature), 4), + &mut events, + ) + .expect("noncombat damage must reach the production damage pipeline"); + assert_eq!( + runner.state().objects[&protected_creature].damage_marked, + 6, + "the same creature's noncombat damage must not match Cover of Winter's combat-only shield" + ); +} + +#[test] +fn benevolent_unicorn_minus_one_stays_spell_only_and_is_not_prevention() { + let mut scenario = GameScenario::new(); + scenario.add_creature_from_oracle(P0, "Benevolent Unicorn", 1, 2, BENEVOLENT_UNICORN); + let spell = scenario + .add_spell_to_hand_from_oracle(P1, "Spell", true, DAMAGE_SPELL) + .with_mana_cost(ManaCost::generic(0)) + .id(); + let permanent_source = scenario.add_creature(P1, "Permanent Source", 3, 3).id(); + let mut runner = scenario.build(); + set_priority(&mut runner, P1); + let outcome = runner.cast(spell).target_player(P0).resolve(); + assert_eq!(outcome.life_delta(P0), -2, "the spell is reduced by one"); + assert!( + !outcome.events().iter().any(|event| matches!( + event, + engine::types::events::GameEvent::DamagePrevented { .. } + )), + "arithmetic minus one must not emit prevention bookkeeping" + ); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(permanent_source, P1, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("permanent-source damage must resolve"); + assert_eq!( + runner.life(P0), + 15, + "Benevolent Unicorn must not reduce nonspell damage" + ); +} + +#[test] +fn unsupported_event_relative_prevention_cards_remain_named_prevent_gaps() { + for (name, oracle, types) in [ + ( + "Dark Sphere", + "{T}, Sacrifice ~: The next time a source of your choice would deal damage to you this turn, prevent half that damage, rounded down.", + &["Artifact"][..], + ), + ( + "Tornellan Protector", + "{T}: Until end of turn, each time damage is dealt to target creature or player, prevent X of that damage, where X is a number from 1 to 3 chosen at random each time.", + &["Creature"][..], + ), + ] { + let types: Vec = types.iter().map(|ty| (*ty).to_string()).collect(); + let parsed = parse_oracle_text(oracle, name, &[], &types, &[]); + let effects = parsed + .abilities + .iter() + .map(|ability| ability.effect.as_ref()) + .chain( + parsed + .triggers + .iter() + .filter_map(|trigger| trigger.execute.as_ref().map(|ability| ability.effect.as_ref())), + ) + .collect::>(); + assert!( + effects + .iter() + .copied() + .any(|effect| matches!(effect, Effect::Unimplemented { name: gap, .. } if gap == "prevent")), + "{name} must preserve its unsupported prevention clause as an honest named gap: {parsed:#?}" + ); + assert!( + effects.iter().copied().all(|effect| !matches!( + effect, + Effect::PreventDamage { + amount: engine::types::ability::PreventionAmount::Next(1), + .. + } + )), + "{name} must not retain a fallback one-damage prevention effect: {parsed:#?}" + ); + } +} diff --git a/crates/engine/tests/integration/issue_5902_heart_shaped_herb.rs b/crates/engine/tests/integration/issue_5902_heart_shaped_herb.rs index 5b30946899..f8240813f8 100644 --- a/crates/engine/tests/integration/issue_5902_heart_shaped_herb.rs +++ b/crates/engine/tests/integration/issue_5902_heart_shaped_herb.rs @@ -26,11 +26,13 @@ use engine::game::effects::deal_damage; use engine::game::scenario::{GameScenario, P0, P1}; use engine::types::ability::{ - DamageModification, Effect, GameRestriction, QuantityExpr, ResolvedAbility, RestrictionExpiry, - ShieldKind, TargetFilter, TargetRef, + DamageModification, Effect, GameRestriction, PreventionFormula, QuantityExpr, ResolvedAbility, + RestrictionExpiry, ShieldKind, TargetFilter, TargetRef, }; use engine::types::card_type::CoreType; use engine::types::events::GameEvent; +use engine::types::mana::ManaCost; +use engine::types::phase::Phase; use engine::types::player::PlayerId; const HEART_SHAPED_HERB: &str = @@ -88,7 +90,9 @@ fn heart_shaped_herb_prevents_one_from_opponent_sources_not_own_and_never_deplet let repl = &runner.state().objects[&herb].replacement_definitions[0]; assert_eq!( repl.damage_modification, - Some(DamageModification::PreventionMinus { value: 1 }), + Some(DamageModification::PreventionMinus { + value: PreventionFormula::fixed(1), + }), "Heart-Shaped Herb must install a continuous PreventionMinus(1) damage replacement, got {:?}", repl.damage_modification ); @@ -208,6 +212,7 @@ fn heart_shaped_herb_prevents_one_from_opponent_sources_not_own_and_never_deplet #[test] fn benevolent_unicorn_arithmetic_minus_reduces_without_prevention_bookkeeping() { let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); let unicorn = scenario .add_creature_from_oracle( P0, @@ -217,7 +222,15 @@ fn benevolent_unicorn_arithmetic_minus_reduces_without_prevention_bookkeeping() "If a spell would deal damage to a permanent or player, it deals that much damage minus 1 to that permanent or player instead.", ) .id(); - let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let spell = scenario + .add_spell_to_hand_from_oracle( + P0, + "Arithmetic Damage Spell", + true, + "This spell deals 3 damage to target creature or player.", + ) + .with_mana_cost(ManaCost::zero()) + .id(); let mut runner = scenario.build(); // The static parses to the ARITHMETIC provenance of the shared subtraction: @@ -231,28 +244,22 @@ fn benevolent_unicorn_arithmetic_minus_reduces_without_prevention_bookkeeping() ); assert_eq!(repl.shield_kind, ShieldKind::None); - let p0_life_before = runner.life(P0); - let mut events = Vec::new(); - deal_damage::resolve( - runner.state_mut(), - &damage_to_player_ability(source, P1, TargetRef::Player(P0), 3), - &mut events, - ) - .expect("damage to P0 resolves"); + let outcome = runner.cast(spell).target_player(P1).resolve(); assert_eq!( - runner.life(P0), - p0_life_before - 2, - "the arithmetic replacement must still reduce 3 damage to 2" + outcome.life_delta(P1), + -2, + "the arithmetic replacement must reduce a spell's 3 damage to 2" ); assert!( - !events + !outcome + .events() .iter() .any(|e| matches!(e, GameEvent::DamagePrevented { .. })), "arithmetic 'minus 1' prevents nothing — it must emit NO DamagePrevented \ and satisfy no prevention-triggered ability" ); assert_eq!( - runner.state().last_effect_count, + outcome.state().last_effect_count, None, "arithmetic 'minus 1' must not stamp the CR 615.5 prevented-amount handoff" ); diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index b41207ff8d..c8cff203dd 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -1330,6 +1330,7 @@ mod crowd_control_warden_dual_counters; mod cruel_revival_destroy_and_return_5281; mod cryptex_collect_evidence_mana_ability; mod cumulative_upkeep_discard; +mod damage_prevention_formula; mod dazzling_beauty_become_blocked; mod detectives_phoenix_bestow_graveyard; mod dreadhorde_invasion_amass; diff --git a/crates/engine/tests/integration/spelunking_shockland_order.rs b/crates/engine/tests/integration/spelunking_shockland_order.rs index 1ce3c6a3f7..c00ae26a37 100644 --- a/crates/engine/tests/integration/spelunking_shockland_order.rs +++ b/crates/engine/tests/integration/spelunking_shockland_order.rs @@ -392,6 +392,7 @@ fn legacy_save_restores_a_search_found_prompt_as_search_found_not_ordering() { search_found_candidates: vec![candidate], depth: 0, is_optional: false, + choice_player: None, library_placement: None, exile_controller: None, exile_duration: None, diff --git a/docs/parser-misparse-backlog.md b/docs/parser-misparse-backlog.md index c04926be01..35acc5bbf1 100644 --- a/docs/parser-misparse-backlog.md +++ b/docs/parser-misparse-backlog.md @@ -3,8 +3,8 @@ Consolidated from 50 per-batch clustering passes over the whole card database. Synonymous per-batch clusters were merged into canonical root causes, their card lists unioned and deduped, and ranked by total card appearances (largest first). - **Canonical root causes:** 30 -- **Distinct cards implicated:** 4698 -- **Total card appearances across root causes:** 4731 (a card may appear under more than one root cause when it exhibits multiple distinct misparses) +- **Distinct cards implicated:** 4693 +- **Total card appearances across root causes:** 4726 (a card may appear under more than one root cause when it exhibits multiple distinct misparses) This is the prioritized "fix N root causes → unlock M cards" backlog: the top handful of root causes account for the majority of broken cards. @@ -12,7 +12,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top | # | Root cause | # cards | Fix hint (where it likely lives) | |---|------------|--------:|----------------------------------| -| 1 | Relative-clause / filter restriction on target dropped | 745 | oracle_target.rs / game/filter.rs — extend TargetFilter property extraction for trailing relative clauses | +| 1 | Relative-clause / filter restriction on target dropped | 744 | oracle_target.rs / game/filter.rs — extend TargetFilter property extraction for trailing relative clauses | | 2 | Dropped intervening-if / gating condition (condition: null) | 583 | oracle_nom/condition.rs parse_inner_condition — trigger/static parsers must delegate condition extraction here | | 3 | Anaphor bound to wrong referent | 404 | oracle_quantity.rs context-ref resolution + game/ability_utils.rs forward_result wiring | | 4 | Conjoined / chained second effect clause dropped | 387 | oracle.rs effect-chain composition — split on 'and'/'then'/sentence boundaries and build sub_ability chain | @@ -22,7 +22,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top | 8 | Additional / alternative casting cost dropped | 210 | oracle_cost.rs — parse additional/alternative cost clauses into Spell.cost / AdditionalCost | | 9 | Wrong player/controller scope (You where Opponent/Scoped/Target/Defending needed) | 182 | oracle parser ControllerRef binding — resolve scoped/defending/iterated player refs instead of defaulting to You | | 10 | Trigger event/mode unrecognized → Unknown | 167 | oracle_trigger.rs — add typed TriggerMode variants for the unrecognized event classes | -| 11 | Replacement / prevention / 'instead' effect mis-modeled | 157 | add-replacement-effect: route 'would … instead' into replacements[]; preserve damage_source/target filters | +| 11 | Replacement / prevention / 'instead' effect mis-modeled | 153 | add-replacement-effect: route 'would … instead' into replacements[]; preserve damage_source/target filters | | 12 | Modal 'choose one/N' parsed as independent abilities | 138 | oracle.rs modal dispatch — detect 'Choose one —' header, wrap modes in Effect::ChooseOneOf | | 13 | State/game-state condition → StaticCondition::Unrecognized | 132 | oracle_nom/condition.rs parse_inner_condition — add typed variant for the predicate class | | 14 | Granted/quoted ability or continuous modification dropped | 96 | oracle_static.rs continuous-modification extraction — emit all conjuncts incl. GrantAbility/GrantKeyword | @@ -47,7 +47,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top ## Full card lists per root cause -### 1. Relative-clause / filter restriction on target dropped (745 cards) +### 1. Relative-clause / filter restriction on target dropped (744 cards) **Signature.** TargetFilter/affected emitted with empty or missing properties; a trailing restrictive clause (type, subtype, color, mana value, zone, combat/temporal/control predicate, exclusion) is silently dropped, over-broadening the filter. @@ -180,7 +180,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Coralhelm Chronicler - Corpse Dance - Corrosive Ooze -- Cover of Winter - Crimson Roc - Cromat - Crowd of True Believers @@ -3602,7 +3601,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top -### 11. Replacement / prevention / 'instead' effect mis-modeled (157 cards) +### 11. Replacement / prevention / 'instead' effect mis-modeled (153 cards) **Signature.** A continuous replacement / damage-prevention / redirection clause (CR 614/615) is emitted as a one-shot Spell or unconditional sequential sibling, dropping the 'instead'/replacement semantics or the source/recipient filter. @@ -3629,7 +3628,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Awe Strike - Azorius Ploy - Beamtown Beatstick -- Benevolent Unicorn - Betrayal at the Vault - Bloatfly Swarm - Chains of Mephistopheles @@ -3659,7 +3657,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Genesis Wave - Ghosts of the Innocent - Gift of Growth -- Gisela, Blade of Goldnight - Gleemax - Glimpse the Cosmos - Gluttonous Hellkite @@ -3718,7 +3715,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Phyrexian Vindicator - Pilgrim of Justice - Pilgrim of Virtue -- Plated Pegasus - Power Leak - Power Level Analyzer - Prairie Dog @@ -3743,7 +3739,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Sekki, Seasons' Guide - Shadow the Hedgehog - Shelter -- Shield of the Avatar - Shield of the Realm - Shieldmage Advocate - Shimatsu the Bloodcloaked From ef7799152f634d567de829dd2c56335540f45e6d Mon Sep 17 00:00:00 2001 From: invalidCards <842080+invalidCards@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:22:20 +0200 Subject: [PATCH 2/5] fix(engine): honor optional damage replacement decline --- crates/engine/src/game/replacement.rs | 33 ++- .../src/parser/oracle_effect/imperative.rs | 18 ++ crates/engine/src/parser/oracle_tests.rs | 32 +++ .../integration/damage_prevention_formula.rs | 230 ++++++++++++++---- 4 files changed, 251 insertions(+), 62 deletions(-) diff --git a/crates/engine/src/game/replacement.rs b/crates/engine/src/game/replacement.rs index f49893ce8e..e8d6250d33 100644 --- a/crates/engine/src/game/replacement.rs +++ b/crates/engine/src/game/replacement.rs @@ -9191,24 +9191,23 @@ fn apply_single_replacement( )) .then(|| proposed.clone()); - // CR 614.6 + CR 614.12a: Optional `Prevent` replacements (Obstinate Familiar, - // Island Sanctuary — "you may skip that draw") and optional damage-prevention - // formulas (Battletide Alchemist) modify the event only on the accept - // (Execute) branch. Declining leaves the original event intact, even though - // the generic Draw/Damage appliers read their modifier from the definition - // rather than the selected branch. - if matches!(branch, ReplacementBranch::Decline) { - if let Some(repl_def) = repl_def_ref { - if replacement_mode_is_optional(&repl_def.mode) + // CR 614.6 + CR 615.1 + CR 615.1a: An optional replacement modifies a + // damage event only on its accepted branch. The shared damage applier reads + // a definition's direct outcome (amount modification, prevention shield, or + // redirection shield), so a decline must bypass every such outcome before + // that applier runs. `QuantityModification::Prevent` deliberately remains + // here too: the Draw applier has the same definition-driven shape, and an + // optional draw-skip decline must still deliver the original draw. + if matches!(branch, ReplacementBranch::Decline) + && repl_def_ref.is_some_and(|repl_def| { + replacement_mode_is_optional(&repl_def.mode) && (repl_def.quantity_modification == Some(QuantityModification::Prevent) - || matches!( - repl_def.damage_modification, - Some(DamageModification::PreventionMinus { .. }) - )) - { - return Ok(proposed); - } - } + || (matches!(proposed, ProposedEvent::Damage { .. }) + && (repl_def.damage_modification.is_some() + || repl_def.shield_kind.is_shield()))) + }) + { + return Ok(proposed); } if let Some(handler) = registry.get(&event_key) { diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index 5a7e22b399..cf250f75b8 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -24659,4 +24659,22 @@ mod tests { Effect::Unimplemented { .. } )); } + + /// CR 615.1a: Event-relative formulas need a delayed prevention-event + /// representation that this imperative parser does not yet model. Refuse the + /// complete Tornellan Protector clause rather than falling through to the + /// ordinary `PreventDamage::Next(1)` convenience default. + #[test] + fn event_relative_prevention_formula_is_unimplemented_in_imperative_dispatch() { + let text = "Until end of turn, each time damage is dealt to target creature or player, \ + prevent X of that damage, where X is a number from 1 to 3 chosen at random \ + each time."; + let lower = text.to_lowercase(); + let parsed = parse_imperative_family_ast(text, &lower, &mut ParseContext::default()); + assert!(matches!( + parsed, + Some(ImperativeFamilyAst::GainKeyword(Effect::Unimplemented { name, .. })) + if name == "prevent" + )); + } } diff --git a/crates/engine/src/parser/oracle_tests.rs b/crates/engine/src/parser/oracle_tests.rs index c2df1faf94..b2e00c93e2 100644 --- a/crates/engine/src/parser/oracle_tests.rs +++ b/crates/engine/src/parser/oracle_tests.rs @@ -26913,6 +26913,38 @@ fn bound_delayed_recalls_are_not_demoted() { } } +/// CR 615.1a: Tornellan Protector's full activated-ability line reaches the +/// document parser's imperative dispatch and must report its unsupported +/// event-relative formula as a named `prevent` gap. This complements the +/// imperative-level regression by proving the router does not substitute the +/// historical one-damage fallback on the production Oracle-text path. +#[test] +fn tornellan_protector_event_relative_formula_is_an_honest_prevent_gap() { + let parsed = parse_oracle_text( + "{T}: Until end of turn, each time damage is dealt to target creature or player, \ + prevent X of that damage, where X is a number from 1 to 3 chosen at random each time.", + "Tornellan Protector", + &[], + &["Creature".to_string()], + &[], + ); + let ability = parsed + .abilities + .first() + .expect("the activated ability must reach the production parser"); + assert!(matches!( + ability.effect.as_ref(), + Effect::Unimplemented { name, .. } if name == "prevent" + )); + assert!(!matches!( + ability.effect.as_ref(), + Effect::PreventDamage { + amount: PreventionAmount::Next(1), + .. + } + )); +} + // --------------------------------------------------------------------------- // Namor, Atlantean King — the attacked-player predicate (CR 603.2) and the // "attacking that player" defending-player anaphor (CR 508.5). diff --git a/crates/engine/tests/integration/damage_prevention_formula.rs b/crates/engine/tests/integration/damage_prevention_formula.rs index 17756f23f6..3c6e90088a 100644 --- a/crates/engine/tests/integration/damage_prevention_formula.rs +++ b/crates/engine/tests/integration/damage_prevention_formula.rs @@ -5,34 +5,33 @@ //! from the affected player's replacement ordering authority. use engine::game::combat::AttackTarget; -use engine::game::effects::deal_damage; +use engine::game::effects::{attach::attach_to, deal_damage}; use engine::game::game_object::AttachTarget; use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; use engine::parser::oracle::parse_oracle_text; use engine::types::ability::{ - DamageModification, Effect, QuantityExpr, ReplacementDefinition, ResolvedAbility, TargetFilter, - TargetRef, + DamageModification, DamageRedirectTarget, Effect, PreventionAmount, QuantityExpr, + RedirectionLifetime, ReplacementChoiceAuthority, ReplacementDefinition, ReplacementMode, + ResolvedAbility, TargetFilter, TargetRef, }; use engine::types::actions::GameAction; -use engine::types::card_type::CoreType; use engine::types::counter::CounterType; use engine::types::game_state::WaitingFor; use engine::types::identifiers::ObjectId; +use engine::types::keywords::Keyword; use engine::types::mana::ManaCost; use engine::types::phase::Phase; use engine::types::player::PlayerId; use engine::types::replacements::ReplacementEvent; +use engine::types::zones::Zone; -const GISELA: &str = - "If a source would deal damage to you or a permanent you control, prevent half that damage, rounded up."; +const GISELA: &str = "Flying, first strike\nIf a source would deal damage to an opponent or a permanent an opponent controls, that source deals double that damage to that player or permanent instead.\nIf a source would deal damage to you or a permanent you control, prevent half that damage, rounded up."; const BATTLETIDE: &str = "If a source would deal damage to a player, you may prevent X of that damage, where X is the number of Clerics you control."; -const REM: &str = - "If a spell would deal damage to you or another permanent you control, prevent that damage."; +const REM: &str = "Flying, haste\nIf a spell would deal damage to you or another permanent you control, prevent that damage.\nIf a spell would deal damage to an opponent or another permanent an opponent controls, it deals that much damage plus 1 instead."; const PLATED_PEGASUS: &str = "If a spell would deal damage to a permanent or player, prevent 1 damage that spell would deal to that permanent or player."; -const SHIELD_OF_THE_RIGHTEOUS: &str = - "If a source would deal damage to equipped creature, prevent X of that damage, where X is the number of creatures you control."; +const SHIELD_OF_THE_AVATAR: &str = "If a source would deal damage to equipped creature, prevent X of that damage, where X is the number of creatures you control.\nEquip {2} ({2}: Attach to target creature you control. Equip only as a sorcery.)"; const COVER_OF_WINTER: &str = "Cumulative upkeep {S} (At the beginning of your upkeep, put an age counter on this permanent, then sacrifice it unless you pay its upkeep cost for each age counter on it. {S} can be paid with one mana from a snow source.)\nIf a creature would deal combat damage to you and/or one or more creatures you control, prevent X of that damage, where X is the number of age counters on this enchantment.\n{S}: Put an age counter on this enchantment."; const BENEVOLENT_UNICORN: &str = "If a spell would deal damage to a permanent or player, it deals that much damage minus 1 to that permanent or player instead."; @@ -83,7 +82,8 @@ fn choose_source_candidate(runner: &mut GameRunner, source: ObjectId) { fn gisela_rounds_up_and_affected_player_orders_against_a_doubler() { let mut scenario = GameScenario::new(); let gisela = scenario - .add_creature_from_oracle(P0, "Gisela, Blade of Goldnight", 5, 5, GISELA) + .add_creature(P0, "Gisela, Blade of Goldnight", 5, 5) + .from_oracle_text_with_keywords(&["Flying", "First strike"], GISELA) .id(); let doubler = scenario.add_creature(P1, "Damage Doubler", 2, 2).id(); let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); @@ -124,7 +124,8 @@ fn gisela_rounds_up_and_affected_player_orders_against_a_doubler() { let mut scenario = GameScenario::new(); let gisela = scenario - .add_creature_from_oracle(P0, "Gisela, Blade of Goldnight", 5, 5, GISELA) + .add_creature(P0, "Gisela, Blade of Goldnight", 5, 5) + .from_oracle_text_with_keywords(&["Flying", "First strike"], GISELA) .id(); let doubler = scenario.add_creature(P1, "Damage Doubler", 2, 2).id(); let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); @@ -153,12 +154,16 @@ fn gisela_rounds_up_and_affected_player_orders_against_a_doubler() { before - 5, "doubling first makes 10 damage, then Gisela prevents 5 rounded up" ); + let gisela_object = &runner.state().objects[&gisela]; + assert_eq!( + gisela_object.replacement_definitions.len(), + 2, + "reach guard: both of Gisela's printed damage replacements must be present" + ); assert!( - runner.state().objects[&gisela] - .replacement_definitions - .len() - == 1, - "reach guard: Gisela's printed static replacement must be present" + gisela_object.keywords.contains(&Keyword::Flying) + && gisela_object.keywords.contains(&Keyword::FirstStrike), + "Gisela's keyword-aware Oracle fixture must retain Flying and first strike" ); } @@ -257,30 +262,121 @@ fn battletide_decline_leaves_the_original_damage_untouched() { assert_eq!(runner.life(P1), before - 5); } +/// CR 614.6 + CR 615.1a: declining an optional prevention or redirection +/// replacement leaves the original damage event unchanged. These use the live +/// `GameAction::ChooseReplacement` path because the appliers read their direct +/// outcome from the stored definition, not from an AST-only test fixture. +#[test] +fn optional_shield_and_redirect_declines_leave_original_damage_untouched() { + let mut scenario = GameScenario::new(); + let shield = scenario.add_creature(P0, "Optional Shield", 1, 1).id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + runner + .state_mut() + .objects + .get_mut(&shield) + .expect("optional shield source must exist") + .replacement_definitions + .push( + ReplacementDefinition::new(ReplacementEvent::DamageDone) + .prevention_shield(PreventionAmount::All) + .mode(ReplacementMode::Optional { decline: None }) + .choice_authority(ReplacementChoiceAuthority::SourceController), + ); + set_priority(&mut runner, P0); + let before = runner.life(P1); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, P1, TargetRef::Player(P1), 5), + &mut events, + ) + .expect("damage must reach the optional shield replacement"); + assert!(matches!( + runner.state().waiting_for, + WaitingFor::ReplacementChoice { player: P0, .. } + )); + runner + .act(GameAction::ChooseReplacement { index: 1 }) + .expect("the shield controller can decline prevention"); + assert_eq!(runner.life(P1), before - 5); + + let mut scenario = GameScenario::new(); + let redirect = scenario.add_creature(P0, "Optional Redirect", 1, 1).id(); + let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); + let mut runner = scenario.build(); + runner + .state_mut() + .objects + .get_mut(&redirect) + .expect("optional redirect source must exist") + .replacement_definitions + .push( + ReplacementDefinition::new(ReplacementEvent::DamageDone) + .redirection_shield( + DamageRedirectTarget::Controller, + PreventionAmount::All, + RedirectionLifetime::OneOpportunity, + ) + .mode(ReplacementMode::Optional { decline: None }) + .choice_authority(ReplacementChoiceAuthority::SourceController), + ); + set_priority(&mut runner, P0); + let p0_before = runner.life(P0); + let p1_before = runner.life(P1); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(source, P1, TargetRef::Player(P1), 5), + &mut events, + ) + .expect("damage must reach the optional redirect replacement"); + assert!(matches!( + runner.state().waiting_for, + WaitingFor::ReplacementChoice { player: P0, .. } + )); + runner + .act(GameAction::ChooseReplacement { index: 1 }) + .expect("the redirect controller can decline redirection"); + assert_eq!( + runner.life(P0), + p0_before, + "declining must not redirect damage" + ); + assert_eq!( + runner.life(P1), + p1_before - 5, + "the original damage must be dealt" + ); +} + #[test] fn rem_and_plated_apply_only_to_spells_and_keep_their_recipient_scopes() { let mut scenario = GameScenario::new(); let rem = scenario - .add_creature_from_oracle(P0, "Rem Karolus, Stalwart Slayer", 3, 4, REM) + .add_creature(P0, "Rem Karolus, Stalwart Slayer", 2, 3) + .from_oracle_text_with_keywords(&["Flying", "Haste"], REM) .id(); let ally = scenario.add_creature(P0, "Protected Ally", 1, 5).id(); - let spell_to_rem = scenario - .add_spell_to_hand_from_oracle(P1, "Spell to Rem", true, DAMAGE_SPELL) - .with_mana_cost(ManaCost::generic(0)) - .id(); let spell_to_ally = scenario .add_spell_to_hand_from_oracle(P1, "Spell to Ally", true, DAMAGE_SPELL) .with_mana_cost(ManaCost::generic(0)) .id(); - let permanent_source = scenario.add_creature(P1, "Permanent Source", 3, 3).id(); let mut runner = scenario.build(); - set_priority(&mut runner, P1); - runner.cast(spell_to_rem).target_object(rem).resolve(); + + let rem_object = &runner.state().objects[&rem]; assert_eq!( - runner.state().objects[&rem].damage_marked, - 3, - "Rem's 'another permanent' clause must exclude Rem itself" + rem_object.replacement_definitions.len(), + 2, + "reach guard: both of Rem's printed spell-damage replacements must be present" + ); + assert!( + rem_object.keywords.contains(&Keyword::Flying) + && rem_object.keywords.contains(&Keyword::Haste), + "Rem's keyword-aware Oracle fixture must retain Flying and haste" ); + set_priority(&mut runner, P1); runner.cast(spell_to_ally).target_object(ally).resolve(); assert_eq!( @@ -288,6 +384,48 @@ fn rem_and_plated_apply_only_to_spells_and_keep_their_recipient_scopes() { 0, "a spell's damage to another permanent the controller owns is prevented" ); + + let mut scenario = GameScenario::new(); + scenario + .add_creature(P0, "Rem Karolus, Stalwart Slayer", 2, 3) + .from_oracle_text_with_keywords(&["Flying", "Haste"], REM); + let spell_to_opponent = scenario + .add_spell_to_hand_from_oracle(P0, "Spell to Opponent", true, DAMAGE_SPELL) + .with_mana_cost(ManaCost::generic(0)) + .id(); + let mut runner = scenario.build(); + set_priority(&mut runner, P0); + let opponent_damage = runner.cast(spell_to_opponent).target_player(P1).resolve(); + assert_eq!( + opponent_damage.life_delta(P1), + -4, + "Rem adds one to spell damage dealt to an opponent" + ); + + let mut scenario = GameScenario::new(); + let rem = scenario + .add_creature(P0, "Rem Karolus, Stalwart Slayer", 2, 3) + .from_oracle_text_with_keywords(&["Flying", "Haste"], REM) + .id(); + let spell_to_rem = scenario + .add_spell_to_hand_from_oracle(P1, "Spell to Rem", true, DAMAGE_SPELL) + .with_mana_cost(ManaCost::generic(0)) + .id(); + let mut runner = scenario.build(); + set_priority(&mut runner, P1); + runner.cast(spell_to_rem).target_object(rem).resolve(); + assert_eq!( + runner.state().objects[&rem].zone, + Zone::Graveyard, + "Rem's 'another permanent' clause must exclude Rem itself, so lethal spell damage kills its 2/3 body" + ); + + let mut scenario = GameScenario::new(); + scenario + .add_creature(P0, "Rem Karolus, Stalwart Slayer", 2, 3) + .from_oracle_text_with_keywords(&["Flying", "Haste"], REM); + let permanent_source = scenario.add_creature(P1, "Permanent Source", 3, 3).id(); + let mut runner = scenario.build(); let mut events = Vec::new(); deal_damage::resolve( runner.state_mut(), @@ -332,26 +470,28 @@ fn rem_and_plated_apply_only_to_spells_and_keep_their_recipient_scopes() { } #[test] -fn shield_formula_uses_the_equipped_recipient_and_live_creature_count() { +fn shield_of_the_avatar_uses_the_equipped_recipient_and_live_creature_count() { let mut scenario = GameScenario::new(); let shield = scenario - .add_creature_from_oracle(P0, "Shield", 0, 1, SHIELD_OF_THE_RIGHTEOUS) + .add_artifact_from_oracle(P0, "Shield of the Avatar", SHIELD_OF_THE_AVATAR) + .with_subtypes(vec!["Equipment"]) .id(); let equipped = scenario.add_creature(P0, "Equipped", 2, 7).id(); let unrelated = scenario.add_creature(P0, "Unrelated", 2, 7).id(); let source = scenario.add_creature(P1, "Damage Source", 3, 3).id(); let mut runner = scenario.build(); - { - let object = runner.state_mut().objects.get_mut(&shield).unwrap(); - object.card_types.core_types = vec![CoreType::Artifact]; - object.card_types.subtypes = vec!["Equipment".to_string()]; - object.base_card_types = object.card_types.clone(); - object.power = None; - object.toughness = None; - object.base_power = None; - object.base_toughness = None; - object.attached_to = Some(AttachTarget::Object(equipped)); - } + assert_eq!(attach_to(runner.state_mut(), shield, equipped), None); + assert_eq!( + runner.state().objects[&shield].attached_to, + Some(AttachTarget::Object(equipped)), + "the real Equipment must be attached through the production attachment helper" + ); + assert!( + runner.state().objects[&equipped] + .attachments + .contains(&shield), + "the equipped creature must reciprocally record Shield of the Avatar" + ); let mut events = Vec::new(); deal_damage::resolve( runner.state_mut(), @@ -472,7 +612,7 @@ fn unsupported_event_relative_prevention_cards_remain_named_prevent_gaps() { ] { let types: Vec = types.iter().map(|ty| (*ty).to_string()).collect(); let parsed = parse_oracle_text(oracle, name, &[], &types, &[]); - let effects = parsed + let direct_effects = parsed .abilities .iter() .map(|ability| ability.effect.as_ref()) @@ -484,14 +624,14 @@ fn unsupported_event_relative_prevention_cards_remain_named_prevent_gaps() { ) .collect::>(); assert!( - effects + direct_effects .iter() .copied() .any(|effect| matches!(effect, Effect::Unimplemented { name: gap, .. } if gap == "prevent")), - "{name} must preserve its unsupported prevention clause as an honest named gap: {parsed:#?}" + "{name} must preserve its unsupported prevention clause as an honest named gap through the full Oracle parser: {parsed:#?}" ); assert!( - effects.iter().copied().all(|effect| !matches!( + direct_effects.iter().copied().all(|effect| !matches!( effect, Effect::PreventDamage { amount: engine::types::ability::PreventionAmount::Next(1), From 3f6f2a7a0bc1150aa7f4cfc32bb9800febce94a5 Mon Sep 17 00:00:00 2001 From: invalidCards <842080+invalidCards@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:50:09 +0200 Subject: [PATCH 3/5] fix(parser): preserve event-relative prevention gaps --- crates/engine/src/game/replacement.rs | 53 ++++++++++++++++--- crates/engine/src/parser/oracle.rs | 18 +++++++ .../src/parser/oracle_effect/imperative.rs | 41 +++++++++----- .../src/parser/oracle_nom/prevention.rs | 21 ++++++++ crates/engine/src/parser/oracle_tests.rs | 34 ++++++++++++ 5 files changed, 146 insertions(+), 21 deletions(-) diff --git a/crates/engine/src/game/replacement.rs b/crates/engine/src/game/replacement.rs index e8d6250d33..92e29aa808 100644 --- a/crates/engine/src/game/replacement.rs +++ b/crates/engine/src/game/replacement.rs @@ -9195,16 +9195,21 @@ fn apply_single_replacement( // damage event only on its accepted branch. The shared damage applier reads // a definition's direct outcome (amount modification, prevention shield, or // redirection shield), so a decline must bypass every such outcome before - // that applier runs. `QuantityModification::Prevent` deliberately remains - // here too: the Draw applier has the same definition-driven shape, and an - // optional draw-skip decline must still deliver the original draw. + // that applier runs. The draw-skip shape is similarly direct, but + // `QuantityModification::Prevent` is only that shape for a Draw event; + // counter prevention must still reach its own applier after a decline. if matches!(branch, ReplacementBranch::Decline) && repl_def_ref.is_some_and(|repl_def| { replacement_mode_is_optional(&repl_def.mode) - && (repl_def.quantity_modification == Some(QuantityModification::Prevent) - || (matches!(proposed, ProposedEvent::Damage { .. }) - && (repl_def.damage_modification.is_some() - || repl_def.shield_kind.is_shield()))) + && match &proposed { + ProposedEvent::Draw { .. } => { + repl_def.quantity_modification == Some(QuantityModification::Prevent) + } + ProposedEvent::Damage { .. } => { + repl_def.damage_modification.is_some() || repl_def.shield_kind.is_shield() + } + _ => false, + } }) { return Ok(proposed); @@ -20876,6 +20881,40 @@ mod tests { } } + /// The decline bypass is event-specific: `QuantityModification::Prevent` + /// means a skipped original draw only for `ProposedEvent::Draw`. An optional + /// counter-prevention replacement still uses its AddCounter applier when the + /// player takes its decline branch. + #[test] + fn optional_counter_prevention_decline_reaches_counter_applier() { + let source = ObjectId(90); + let mut repl = ReplacementDefinition::new(ReplacementEvent::AddCounter) + .quantity_modification(QuantityModification::Prevent); + repl.mode = ReplacementMode::Optional { decline: None }; + let mut state = test_state_with_object(source, Zone::Battlefield, vec![repl]); + let mut events = Vec::new(); + let registry = build_replacement_registry(); + let event = ProposedEvent::AddCounter { + placement: CounterPlacement::Player { + actor: PlayerId(0), + player_id: PlayerId(0), + counter_kind: crate::types::player::PlayerCounterKind::Poison, + }, + count: 1, + applied: HashSet::new(), + }; + + let result = apply_single_replacement( + &mut state, + event, + ReplacementId { source, index: 0 }, + ReplacementBranch::Decline, + ®istry, + &mut events, + ); + assert!(matches!(result, Err(ApplyResult::Prevented))); + } + #[test] fn player_counter_prohibition_does_not_match_object_counter_placement() { let source = ObjectId(90); diff --git a/crates/engine/src/parser/oracle.rs b/crates/engine/src/parser/oracle.rs index 5738441ca7..2298cda21d 100644 --- a/crates/engine/src/parser/oracle.rs +++ b/crates/engine/src/parser/oracle.rs @@ -33,6 +33,7 @@ use crate::types::zones::Zone; use super::oracle_nom::bridge::{nom_on_lower, split_once_on_lower}; use super::oracle_nom::condition::parse_graveyard_keyword_grant_sentence; +use super::oracle_nom::prevention::has_each_time_event_relative_prevention; use super::oracle_nom::primitives::{ parse_number as nom_parse_number, parse_object_recipient_pronoun, parse_period_sentences, scan_at_word_boundaries, scan_contains, scan_preceded, @@ -6590,6 +6591,23 @@ fn parse_normalized_oracle_ir( // Priority 8: Replacement patterns if is_replacement_pattern(&lower) { + // The replacement classifier correctly recognizes this prevention + // wording, but the engine cannot yet model its continuous, + // repeatable damage-event watcher. Preserve the precise `prevent` + // gap rather than degrading it to the generic replacement-structure + // fallback below. + if has_each_time_event_relative_prevention(&lower) { + emitter.ability_at( + item_line, + AbilityDefinition::new( + AbilityKind::Spell, + Effect::unimplemented("prevent", &line), + ) + .description(line.clone()), + ); + i += 1; + continue; + } // CR 208.2b + CR 614.1c + CR 614.12a: modal "As ~ enters, it becomes // your choice of [P/T profiles]" as-enters replacement (Primal Plasma, // Primal Clay, Corrupted Shapeshifter, Aquamorph Entity). This is a diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index cf250f75b8..9ee1149840 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -29,7 +29,9 @@ use crate::parser::oracle_nom::bridge::{nom_on_lower, nom_parse_lower, split_onc 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::prevention::has_event_relative_prevention_amount; +use crate::parser::oracle_nom::prevention::{ + has_each_time_event_relative_prevention, has_event_relative_prevention_amount, +}; 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; @@ -11086,16 +11088,11 @@ pub(super) fn parse_imperative_family_ast( return Some(ImperativeFamilyAst::GainKeyword(effect)); } - // A delayed "each time damage is dealt" prevention formula needs both a - // repeatable event watcher and a random amount. Neither is carried by the - // ordinary one-shot `PreventDamage` effect, so fail at the outer clause + // CR 615.1 + CR 615.1a: An each-time prevention formula is a continuous, + // repeatable watcher of damage events. The imperative AST cannot represent + // that watcher or its event-relative amount, so fail at the outer clause // rather than lowering its inner `prevent X` to the Next(1) fallback. - if has_event_relative_prevention_amount(lower) - && nom_primitives::scan_at_word_boundaries(lower, |input| { - tag::<_, _, OracleError<'_>>("each time ").parse(input) - }) - .is_some() - { + if has_each_time_event_relative_prevention(lower) { return Some(ImperativeFamilyAst::GainKeyword(Effect::unimplemented( "prevent", text, ))); @@ -24660,10 +24657,11 @@ mod tests { )); } - /// CR 615.1a: Event-relative formulas need a delayed prevention-event - /// representation that this imperative parser does not yet model. Refuse the - /// complete Tornellan Protector clause rather than falling through to the - /// ordinary `PreventDamage::Next(1)` convenience default. + /// CR 615.1 + CR 615.1a: Event-relative formulas need a continuous, + /// repeatable prevention-event watcher that this imperative parser does not + /// yet model. Refuse the complete Tornellan Protector clause rather than + /// falling through to the ordinary `PreventDamage::Next(1)` convenience + /// default. #[test] fn event_relative_prevention_formula_is_unimplemented_in_imperative_dispatch() { let text = "Until end of turn, each time damage is dealt to target creature or player, \ @@ -24677,4 +24675,19 @@ mod tests { if name == "prevent" )); } + + /// The simple half-damage wording takes the same imperative route as the + /// fuller Tornellan formula. Keep it as an explicit `prevent` gap until the + /// AST can represent the continuous event watcher. + #[test] + fn each_time_half_damage_formula_is_unimplemented_in_imperative_dispatch() { + let text = "Each time a source would deal damage to you, prevent half that damage."; + let lower = text.to_lowercase(); + let parsed = parse_imperative_family_ast(text, &lower, &mut ParseContext::default()); + assert!(matches!( + parsed, + Some(ImperativeFamilyAst::GainKeyword(Effect::Unimplemented { name, .. })) + if name == "prevent" + )); + } } diff --git a/crates/engine/src/parser/oracle_nom/prevention.rs b/crates/engine/src/parser/oracle_nom/prevention.rs index d09f9ff771..0ea2ef2486 100644 --- a/crates/engine/src/parser/oracle_nom/prevention.rs +++ b/crates/engine/src/parser/oracle_nom/prevention.rs @@ -70,6 +70,17 @@ pub fn has_event_relative_prevention_amount(input: &str) -> bool { .is_some() } +/// Classify an each-time prevention formula that needs a continuous, repeatable +/// damage-event watcher in addition to its event-relative amount. +pub fn has_each_time_event_relative_prevention(input: &str) -> bool { + has_event_relative_prevention_amount(input) + && crate::parser::oracle_nom::primitives::scan_at_word_boundaries(input, |candidate| { + tag::<_, _, crate::parser::oracle_nom::error::OracleError<'_>>("each time ") + .parse(candidate) + }) + .is_some() +} + #[cfg(test)] mod tests { use super::*; @@ -100,4 +111,14 @@ mod tests { PreventionFormula::Quantity { .. } )); } + + #[test] + fn classifies_each_time_event_relative_prevention() { + assert!(has_each_time_event_relative_prevention( + "each time a source would deal damage to you, prevent half that damage." + )); + assert!(!has_each_time_event_relative_prevention( + "prevent half that damage, rounded up." + )); + } } diff --git a/crates/engine/src/parser/oracle_tests.rs b/crates/engine/src/parser/oracle_tests.rs index b2e00c93e2..0a4727f262 100644 --- a/crates/engine/src/parser/oracle_tests.rs +++ b/crates/engine/src/parser/oracle_tests.rs @@ -26945,6 +26945,40 @@ fn tornellan_protector_event_relative_formula_is_an_honest_prevent_gap() { )); } +/// CR 615.1 + CR 615.1a: The production document parser must report the +/// simple each-time half-damage wording as the same named `prevent` gap. Walk +/// every nested ability effect so a future partial lowering cannot conceal the +/// historical `PreventDamage::Next(1)` fallback below a wrapper. +#[test] +fn each_time_half_damage_formula_is_an_honest_prevent_gap() { + let parsed = parse_oracle_text( + "Each time a source would deal damage to you, prevent half that damage.", + "Event-Relative Prevention Test", + &[], + &[], + &[], + ); + assert!( + unimplemented_keys(&parsed) + .iter() + .any(|key| key == "prevent"), + "the event-relative formula must remain a named prevent gap; keys={:?}", + unimplemented_keys(&parsed), + ); + assert!( + !collect_all_effects(&parsed.abilities) + .iter() + .any(|effect| matches!( + effect, + Effect::PreventDamage { + amount: PreventionAmount::Next(1), + .. + } + )), + "the production parser must not hide the unsupported formula as PreventDamage::Next(1)" + ); +} + // --------------------------------------------------------------------------- // Namor, Atlantean King — the attacked-player predicate (CR 603.2) and the // "attacking that player" defending-player anaphor (CR 508.5). From 6f700b7ffc41ae4d1b24f1eced8bf9c72daf44be Mon Sep 17 00:00:00 2001 From: invalidCards <842080+invalidCards@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:26:58 +0200 Subject: [PATCH 4/5] fix(engine): honor declined counter prevention --- crates/engine/src/game/replacement.rs | 37 ++++++++++++++------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/crates/engine/src/game/replacement.rs b/crates/engine/src/game/replacement.rs index 92e29aa808..e4f9d07fb9 100644 --- a/crates/engine/src/game/replacement.rs +++ b/crates/engine/src/game/replacement.rs @@ -9195,21 +9195,17 @@ fn apply_single_replacement( // damage event only on its accepted branch. The shared damage applier reads // a definition's direct outcome (amount modification, prevention shield, or // redirection shield), so a decline must bypass every such outcome before - // that applier runs. The draw-skip shape is similarly direct, but - // `QuantityModification::Prevent` is only that shape for a Draw event; - // counter prevention must still reach its own applier after a decline. + // that applier runs. `QuantityModification::Prevent` is likewise a direct + // event outcome, regardless of which event-specific applier owns it: a + // declined optional draw-skip, counter-prevention, or future quantity + // prevention replacement must leave its original event unchanged. if matches!(branch, ReplacementBranch::Decline) && repl_def_ref.is_some_and(|repl_def| { replacement_mode_is_optional(&repl_def.mode) - && match &proposed { - ProposedEvent::Draw { .. } => { - repl_def.quantity_modification == Some(QuantityModification::Prevent) - } - ProposedEvent::Damage { .. } => { - repl_def.damage_modification.is_some() || repl_def.shield_kind.is_shield() - } - _ => false, - } + && (repl_def.quantity_modification == Some(QuantityModification::Prevent) + || (matches!(proposed, ProposedEvent::Damage { .. }) + && (repl_def.damage_modification.is_some() + || repl_def.shield_kind.is_shield()))) }) { return Ok(proposed); @@ -20881,12 +20877,12 @@ mod tests { } } - /// The decline bypass is event-specific: `QuantityModification::Prevent` - /// means a skipped original draw only for `ProposedEvent::Draw`. An optional - /// counter-prevention replacement still uses its AddCounter applier when the - /// player takes its decline branch. + /// `QuantityModification::Prevent` is a definition-driven replacement + /// outcome for every event applier that recognizes it. Declining an optional + /// counter-prevention replacement must therefore preserve the original + /// counter event, just as an optional draw-skip decline preserves its draw. #[test] - fn optional_counter_prevention_decline_reaches_counter_applier() { + fn optional_counter_prevention_decline_leaves_counter_event_unchanged() { let source = ObjectId(90); let mut repl = ReplacementDefinition::new(ReplacementEvent::AddCounter) .quantity_modification(QuantityModification::Prevent); @@ -20904,6 +20900,7 @@ mod tests { applied: HashSet::new(), }; + let expected = event.clone(); let result = apply_single_replacement( &mut state, event, @@ -20912,7 +20909,11 @@ mod tests { ®istry, &mut events, ); - assert!(matches!(result, Err(ApplyResult::Prevented))); + assert_eq!( + result, + Ok(expected), + "declining optional counter prevention must preserve the original counter event" + ); } #[test] From a867c2b9e7a9d35df792523abebc2885d7752995 Mon Sep 17 00:00:00 2001 From: invalidCards <842080+invalidCards@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:03:37 +0200 Subject: [PATCH 5/5] fix(parser): harden damage prevention review paths --- .../src/parser/oracle_nom/prevention.rs | 58 +++- .../engine/src/parser/oracle_replacement.rs | 30 +- crates/engine/src/parser/oracle_tests.rs | 264 ++++++++++++++---- .../integration/damage_prevention_formula.rs | 64 +++++ 4 files changed, 336 insertions(+), 80 deletions(-) diff --git a/crates/engine/src/parser/oracle_nom/prevention.rs b/crates/engine/src/parser/oracle_nom/prevention.rs index 0ea2ef2486..50213295f0 100644 --- a/crates/engine/src/parser/oracle_nom/prevention.rs +++ b/crates/engine/src/parser/oracle_nom/prevention.rs @@ -4,7 +4,7 @@ use std::num::NonZeroU32; use nom::branch::alt; -use nom::bytes::complete::tag; +use nom::bytes::complete::{tag, take_till1}; use nom::combinator::{map, map_opt, rest, value}; use nom::sequence::{preceded, terminated}; use nom::Parser; @@ -73,12 +73,35 @@ pub fn has_event_relative_prevention_amount(input: &str) -> bool { /// Classify an each-time prevention formula that needs a continuous, repeatable /// damage-event watcher in addition to its event-relative amount. pub fn has_each_time_event_relative_prevention(input: &str) -> bool { - has_event_relative_prevention_amount(input) - && crate::parser::oracle_nom::primitives::scan_at_word_boundaries(input, |candidate| { - tag::<_, _, crate::parser::oracle_nom::error::OracleError<'_>>("each time ") - .parse(candidate) - }) - .is_some() + crate::parser::oracle_nom::primitives::scan_at_word_boundaries( + input, + parse_each_time_event_relative_prevention, + ) + .is_some() +} + +/// Recognize one complete event-relative prevention watcher. The event clause +/// and its prevention formula must share the same sentence, rather than two +/// independent scans accidentally binding unrelated phrases on a card. +fn parse_each_time_event_relative_prevention(input: &str) -> OracleResult<'_, ()> { + preceded( + tag("each time "), + preceded( + terminated( + take_till1(|character| matches!(character, ',' | '.' | '\n' | '\r')), + tag(", prevent "), + ), + // `parse_damage_prevention_formula` accepts the fully understood + // forms. The bare heads remain deliberately unsupported, but this + // classifier must recognize them so the caller emits an honest gap. + alt(( + value((), parse_damage_prevention_formula), + value((), tag("x of that damage")), + value((), tag("half that damage")), + )), + ), + ) + .parse(input) } #[cfg(test)] @@ -114,11 +137,32 @@ mod tests { #[test] fn classifies_each_time_event_relative_prevention() { + assert!(has_each_time_event_relative_prevention( + "until end of turn, each time damage is dealt to target creature or player, prevent x of that damage, where x is a number from 1 to 3 chosen at random each time." + )); assert!(has_each_time_event_relative_prevention( "each time a source would deal damage to you, prevent half that damage." )); + assert!(has_each_time_event_relative_prevention( + "each time a source would deal damage to you, prevent half that damage, rounded up." + )); assert!(!has_each_time_event_relative_prevention( "prevent half that damage, rounded up." )); + assert!(!has_each_time_event_relative_prevention( + "each time a source would deal damage to you. Prevent half that damage." + )); + assert!(!has_each_time_event_relative_prevention( + "each time a source would deal damage to you\nprevent half that damage." + )); + assert!(!has_each_time_event_relative_prevention( + "each time a source would deal damage to you\r\nprevent half that damage." + )); + assert!(!has_each_time_event_relative_prevention( + "each time a source would deal damage to you\rprevent half that damage." + )); + assert!(!has_each_time_event_relative_prevention( + "each time a player draws a card, they gain 1 life. Prevent half that damage." + )); } } diff --git a/crates/engine/src/parser/oracle_replacement.rs b/crates/engine/src/parser/oracle_replacement.rs index 5cfb0506eb..9e2d10e3b6 100644 --- a/crates/engine/src/parser/oracle_replacement.rs +++ b/crates/engine/src/parser/oracle_replacement.rs @@ -11945,8 +11945,10 @@ fn parse_damage_prevention_replacement( { // Keep compound player/permanent recipients ahead of the bare // controller scan: "to you or another permanent you control" is - // one recipient domain, not a player-only shield. - (Some(tf), false) + // one recipient domain, not a player-only shield. Its rider's + // anaphor refers to the actual damage recipient for every player + // scope (controller, opponent, or source-chosen player). + (Some(tf), true) } else if nom_primitives::scan_contains(working_lower, "dealt to you") || nom_primitives::scan_contains(working_lower, "deal to you") { @@ -12114,13 +12116,15 @@ fn parse_damage_prevention_replacement( // the prevented event's damage recipient, exactly like a typed `valid_card` // does — so the cohort-2 anaphor rewrite must fire for it too. let recipient_is_event_filter = valid_card_filter.is_some() || recipient_from_event; - // CR 301.5f/303.4b: an OBJECT-recipient shield (typed `valid_card`, e.g. - // Panther Habit's equipped creature) rebinds a bare "it" rider to the damage - // recipient. Compute by borrow BEFORE the move below; the self-scoped cohort - // (`valid_card == SelfRef` — Anti-Venom, Unbreathing Horde) is excluded so it - // keeps its source-referring rider. - let recipient_is_object = - matches!(&valid_card_filter, Some(f) if !matches!(f, TargetFilter::SelfRef)); + // CR 615.5: an object-recipient shield (typed `valid_card`, e.g. Panther + // Habit's equipped creature) or a compound player/permanent scope rebinds a + // bare "it" rider to the actual damage recipient. The compound scope's + // recipient is event-derived even though it does not use `valid_card`. + // Compute by borrow BEFORE the move below; the self-scoped cohort + // (`valid_card == SelfRef` — Anti-Venom, Unbreathing Horde) is excluded so + // it keeps its source-referring rider. + let recipient_is_object = recipient_from_event + || matches!(&valid_card_filter, Some(f) if !matches!(f, TargetFilter::SelfRef)); // CR 608.2k: A self-scoped shield ("dealt to ~") rebinds the rider's dangling // anaphor to the SOURCE, not the event recipient — see the follow-up rewrite // branch below. Kept as its own predicate (rather than `!recipient_is_object`) @@ -12211,10 +12215,10 @@ fn parse_damage_prevention_replacement( if recipient_is_event_filter { rewrite_parent_target_to_post_replacement_damage_target(&mut followup_def); } - // CR 615.5 + CR 301.5f/303.4b: in an object-recipient shield a bare - // "it" in the prevented-amount rider (Panther Habit "put that many - // +1/+1 counters on it") lowers to SelfRef but means the damage - // recipient. + // CR 615.5: in an object-recipient or compound player/permanent + // shield, a bare "it" in the prevented-amount rider (Panther Habit + // "put that many +1/+1 counters on it") lowers to SelfRef but means + // the event's actual damage recipient. if recipient_is_object { rewrite_self_ref_to_post_replacement_damage_target(&mut followup_def); } diff --git a/crates/engine/src/parser/oracle_tests.rs b/crates/engine/src/parser/oracle_tests.rs index 0a4727f262..f54c96207c 100644 --- a/crates/engine/src/parser/oracle_tests.rs +++ b/crates/engine/src/parser/oracle_tests.rs @@ -13772,6 +13772,124 @@ fn prevent_dynamic_amount_where_x_is_counters() { ); } +/// Compound damage-recipient filters choose which events a replacement applies +/// to, while a rider's "that permanent" refers to the particular event that was +/// prevented. Keep those two semantic roles distinct for every player scope. +#[test] +fn compound_damage_recipient_riders_bind_to_the_prevented_event_target() { + use crate::types::ability::{DamageTargetFilter, DamageTargetPlayerScope, SourceExclusion}; + + let cases = [ + ( + "controller", + "If a source would deal damage to you and/or one or more creatures you control, prevent that damage. Put a +1/+1 counter on that creature for each 1 damage prevented this way.", + DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Controller, + permanent_type: Some(CoreType::Creature), + source_scope: SourceExclusion::Include, + }, + ), + ( + "opponent", + "If a source would deal damage to an opponent or a permanent an opponent controls, prevent that damage. Put a +1/+1 counter on that permanent for each 1 damage prevented this way.", + DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::Opponent, + permanent_type: None, + source_scope: SourceExclusion::Include, + }, + ), + ( + "source-chosen player", + "If a source would deal damage to the chosen player or a permanent they control, prevent that damage. Put a +1/+1 counter on that permanent for each 1 damage prevented this way.", + DamageTargetFilter::PlayerOrPermanentsControlledBy { + player: DamageTargetPlayerScope::SourceChosenPlayer, + permanent_type: None, + source_scope: SourceExclusion::Include, + }, + ), + ]; + + for (label, oracle, expected_filter) in cases { + let def = crate::parser::oracle_replacement::parse_replacement_line( + oracle, + "Compound Recipient Fixture", + ) + .unwrap_or_else(|| panic!("{label}: replacement must parse")); + assert_eq!( + def.damage_target_filter, + Some(expected_filter), + "{label}: recipient filter must remain unchanged" + ); + let execute = def + .execute + .as_ref() + .unwrap_or_else(|| panic!("{label}: prevention rider must parse")); + assert!( + matches!( + execute.effect.as_ref(), + Effect::PutCounter { + target: TargetFilter::PostReplacementDamageTarget, + .. + } + ), + "{label}: rider must bind to the prevented event target, got {execute:?}" + ); + } + + let bare_pronoun = crate::parser::oracle_replacement::parse_replacement_line( + "If a source would deal damage to an opponent or a permanent an opponent controls, prevent that damage. Put a +1/+1 counter on it for each 1 damage prevented this way.", + "Compound Recipient Bare Pronoun Fixture", + ) + .expect("opponent compound recipient with a bare-pronoun rider must parse"); + assert!(matches!( + bare_pronoun + .execute + .as_deref() + .map(|execute| execute.effect.as_ref()), + Some(Effect::PutCounter { + target: TargetFilter::PostReplacementDamageTarget, + .. + }) + )); +} + +/// Spell-target and self-scoped prevention are different anaphor cohorts from +/// event-recipient prevention and must retain their existing bindings. +#[test] +fn prevention_rider_binding_keeps_spell_target_and_self_scoped_controls() { + let spell_target = crate::parser::oracle_replacement::parse_replacement_line( + "Prevent the next 3 damage that would be dealt to target creature this turn. For each 1 damage prevented this way, put a +1/+1 counter on that creature.", + "Test of Faith", + ) + .expect("spell-target prevention must parse"); + assert!(matches!( + spell_target + .execute + .as_deref() + .map(|execute| execute.effect.as_ref()), + Some(Effect::PutCounter { + target: TargetFilter::ParentTarget, + .. + }) + )); + + let self_scoped = crate::parser::oracle_replacement::parse_replacement_line( + "If damage would be dealt to ~, prevent that damage and put that many +1/+1 counters on it.", + "Self-Scoped Prevention Fixture", + ) + .expect("self-scoped prevention must parse"); + assert!(matches!( + self_scoped + .execute + .as_deref() + .map(|execute| execute.effect.as_ref()), + Some(Effect::PutCounter { + target: TargetFilter::SelfRef, + .. + }) + )); +} + #[test] fn prevent_all_damage_has_no_dynamic_amount() { use crate::parser::oracle_effect::parse_effect; @@ -13882,26 +14000,32 @@ fn collect_prevent_nodes( out } -/// Pre-order (self, then sub_ability, then else_ability) flat list of every -/// effect in the ability/sub-ability tree. Used to assert that non-PreventDamage -/// sibling clauses on the same card survive the bidirectional split intact and -/// in the correct chain position — the family assertion only inspects the two -/// PreventDamage nodes in isolation and cannot detect a dropped/overwritten -/// sibling rider. -fn collect_all_effects(abilities: &[crate::types::ability::AbilityDefinition]) -> Vec { - fn walk(def: &crate::types::ability::AbilityDefinition, out: &mut Vec) { - out.push((*def.effect).clone()); - if let Some(sub) = def.sub_ability.as_deref() { - walk(sub, out); - } - if let Some(el) = def.else_ability.as_deref() { - walk(el, out); - } +/// Visit every ordinary ability root and every trigger execution root in a +/// parsed card. The production visitor owns nested ability traversal; these +/// test collectors only decide which parsed roots to start from. +fn visit_parsed_effects(parsed: &ParsedAbilities, visit: &mut F) +where + F: FnMut(&Effect) -> std::ops::ControlFlow<()>, +{ + for ability in &parsed.abilities { + let _ = crate::types::ability_visit::visit_ability_def(ability, visit); } - let mut out = Vec::new(); - for a in abilities { - walk(a, &mut out); + for trigger in &parsed.triggers { + let _ = crate::types::ability_visit::visit_trigger(trigger, visit); } +} + +/// Flat list of every effect reachable from ordinary ability and trigger roots. +/// Used to assert that non-PreventDamage sibling clauses on the same card +/// survive the bidirectional split intact and in the correct chain position — +/// the family assertion only inspects the two PreventDamage nodes in isolation +/// and cannot detect a dropped/overwritten sibling rider. +fn collect_all_effects(parsed: &ParsedAbilities) -> Vec { + let mut out = Vec::new(); + visit_parsed_effects(parsed, &mut |effect| { + out.push(effect.clone()); + std::ops::ControlFlow::Continue(()) + }); out } @@ -14032,7 +14156,7 @@ fn foxfire_bidirectional_prevent_the_creature() { // (Untap -> to-Prevent -> by-Prevent -> Draw-delayed-trigger), never // overwriting/displacing the "by" shield or being dropped. let parsed = parse(FOXFIRE, "Foxfire", &[], &["Instant"], &[]); - let effects = collect_all_effects(&parsed.abilities); + let effects = collect_all_effects(&parsed); let prevent_positions: Vec = effects .iter() .enumerate() @@ -14074,7 +14198,7 @@ fn delirium_bidirectional_prevent_the_creature() { // damage equal to its power to the player") must still parse and chain ahead // of the now-2-node Prevent split, not be dropped or reordered by it. let parsed = parse(DELIRIUM, "Delirium", &[], &["Instant"], &[]); - let effects = collect_all_effects(&parsed.abilities); + let effects = collect_all_effects(&parsed); let tap_pos = effects .iter() .position(|e| matches!(e, Effect::SetTapState { .. })) @@ -26560,27 +26684,52 @@ fn throne_of_eldraine_parses_all_chosen_color_mana_riders() { /// parsed card, so the honesty tests below assert on the pattern-class key /// rather than on a Debug substring. fn unimplemented_keys(parsed: &ParsedAbilities) -> Vec { - fn walk(def: &AbilityDefinition, out: &mut Vec) { - if let Effect::Unimplemented { name, .. } = &*def.effect { - out.push(name.to_string()); - } - if let Some(sub) = def.sub_ability.as_deref() { - walk(sub, out); - } - if let Some(els) = def.else_ability.as_deref() { - walk(els, out); - } - } - let mut out = Vec::new(); - for def in &parsed.abilities { - walk(def, &mut out); - } - for trig in &parsed.triggers { - if let Some(exec) = trig.execute.as_deref() { - walk(exec, &mut out); - } - } - out + collect_all_effects(parsed) + .into_iter() + .filter_map(|effect| match effect { + Effect::Unimplemented { name, .. } => Some(name), + _ => None, + }) + .collect() +} + +/// Both test collectors must include trigger execution roots, otherwise an +/// unsupported fallback can hide under a parsed trigger while direct ability +/// roots make the same assertion appear green. +#[test] +fn parsed_effect_collectors_include_trigger_execution_trees() { + let mut parsed = parse_oracle_text("", "Trigger Collector Fixture", &[], &[], &[]); + let nested_prevention = AbilityDefinition::new( + AbilityKind::Spell, + crate::parser::oracle_effect::parse_effect( + "prevent the next 1 damage that would be dealt this turn", + ), + ); + let mut execute = AbilityDefinition::new( + AbilityKind::Spell, + Effect::unimplemented("prevent", "trigger execute gap"), + ); + execute.sub_ability = Some(Box::new(nested_prevention)); + let mut trigger = TriggerDefinition::new(TriggerMode::Attacks); + trigger.execute = Some(Box::new(execute)); + parsed.triggers.push(trigger); + + assert!( + unimplemented_keys(&parsed) + .iter() + .any(|key| key == "prevent"), + "the named gap in trigger.execute must be collected" + ); + assert!( + collect_all_effects(&parsed).iter().any(|effect| matches!( + effect, + Effect::PreventDamage { + amount: PreventionAmount::Next(1), + .. + } + )), + "the nested trigger.execute prevention must be collected" + ); } /// CR 603.7a + CR 603.7c + CR 400.7: The impulse-cleanup sweep must stay HONESTLY @@ -26928,21 +27077,18 @@ fn tornellan_protector_event_relative_formula_is_an_honest_prevent_gap() { &["Creature".to_string()], &[], ); - let ability = parsed - .abilities - .first() - .expect("the activated ability must reach the production parser"); - assert!(matches!( - ability.effect.as_ref(), + let effects = collect_all_effects(&parsed); + assert!(effects.iter().any(|effect| matches!( + effect, Effect::Unimplemented { name, .. } if name == "prevent" - )); - assert!(!matches!( - ability.effect.as_ref(), + ))); + assert!(!effects.iter().any(|effect| matches!( + effect, Effect::PreventDamage { amount: PreventionAmount::Next(1), .. } - )); + ))); } /// CR 615.1 + CR 615.1a: The production document parser must report the @@ -26966,15 +27112,13 @@ fn each_time_half_damage_formula_is_an_honest_prevent_gap() { unimplemented_keys(&parsed), ); assert!( - !collect_all_effects(&parsed.abilities) - .iter() - .any(|effect| matches!( - effect, - Effect::PreventDamage { - amount: PreventionAmount::Next(1), - .. - } - )), + !collect_all_effects(&parsed).iter().any(|effect| matches!( + effect, + Effect::PreventDamage { + amount: PreventionAmount::Next(1), + .. + } + )), "the production parser must not hide the unsupported formula as PreventDamage::Next(1)" ); } diff --git a/crates/engine/tests/integration/damage_prevention_formula.rs b/crates/engine/tests/integration/damage_prevention_formula.rs index 3c6e90088a..440031f107 100644 --- a/crates/engine/tests/integration/damage_prevention_formula.rs +++ b/crates/engine/tests/integration/damage_prevention_formula.rs @@ -35,6 +35,7 @@ const SHIELD_OF_THE_AVATAR: &str = "If a source would deal damage to equipped cr const COVER_OF_WINTER: &str = "Cumulative upkeep {S} (At the beginning of your upkeep, put an age counter on this permanent, then sacrifice it unless you pay its upkeep cost for each age counter on it. {S} can be paid with one mana from a snow source.)\nIf a creature would deal combat damage to you and/or one or more creatures you control, prevent X of that damage, where X is the number of age counters on this enchantment.\n{S}: Put an age counter on this enchantment."; const BENEVOLENT_UNICORN: &str = "If a spell would deal damage to a permanent or player, it deals that much damage minus 1 to that permanent or player instead."; +const COMPOUND_OPPONENT_PREVENTION_RIDER: &str = "If a source would deal damage to an opponent or a permanent an opponent controls, prevent that damage. Put a +1/+1 counter on it for each 1 damage prevented this way."; const DAMAGE_SPELL: &str = "This spell deals 3 damage to target creature or player."; fn damage_ability( @@ -167,6 +168,69 @@ fn gisela_rounds_up_and_affected_player_orders_against_a_doubler() { ); } +/// CR 615.5: a compound opponent-recipient prevention rider must follow the +/// actual damaged permanent, rather than the prevention source or damage source. +#[test] +fn compound_opponent_prevention_rider_targets_the_damaged_permanent() { + let mut scenario = GameScenario::new(); + let prevention_source = scenario + .add_enchantment_from_oracle( + P0, + "Compound Opponent Prevention", + COMPOUND_OPPONENT_PREVENTION_RIDER, + ) + .id(); + let damage_source = scenario.add_creature(P0, "Damage Source", 3, 3).id(); + let damaged_permanent = scenario.add_creature(P1, "Damaged Permanent", 1, 5).id(); + let mut runner = scenario.build(); + + assert_eq!( + runner.state().objects[&prevention_source] + .replacement_definitions + .len(), + 1, + "reach guard: the compound prevention fixture must install its replacement" + ); + + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_ability(damage_source, P0, TargetRef::Object(damaged_permanent), 3), + &mut events, + ) + .expect("damage must run through the normal replacement pipeline"); + + assert!( + events.iter().any(|event| matches!( + event, + engine::types::events::GameEvent::DamagePrevented { amount: 3, .. } + )), + "reach guard: the compound prevention replacement must actually prevent damage; events={events:?}" + ); + assert_eq!( + runner.state().objects[&damaged_permanent].damage_marked, + 0, + "the prevented event must not mark damage on its recipient" + ); + assert_eq!( + runner.state().objects[&damaged_permanent].counters[&CounterType::Plus1Plus1], + 3, + "the rider's counters must land on P1's actual damaged permanent" + ); + assert!( + !runner.state().objects[&prevention_source] + .counters + .contains_key(&CounterType::Plus1Plus1), + "the prevention source is not the damage recipient" + ); + assert!( + !runner.state().objects[&damage_source] + .counters + .contains_key(&CounterType::Plus1Plus1), + "the damage source is not the damage recipient" + ); +} + #[test] fn battletide_controller_chooses_optional_prevention_after_affected_player_orders() { let mut scenario = GameScenario::new();