diff --git a/crates/engine/src/database/synthesis.rs b/crates/engine/src/database/synthesis.rs index 14a23554d1..c84e3df8c2 100644 --- a/crates/engine/src/database/synthesis.rs +++ b/crates/engine/src/database/synthesis.rs @@ -3703,16 +3703,19 @@ pub(crate) fn entry_replacement_for_grant_static( /// CR 702.64a: Absorb N — "If a source would deal damage to this creature, /// prevent N of that damage." A continuous, self-recipient damage replacement: -/// `DamageModification::Minus { value: N }` saturating-subtracts N from each -/// damage event whose recipient is this creature (`valid_card: SelfRef`). It is -/// NOT a consumed shield, so it re-applies to every source and every event -/// independently (CR 702.64b). No new variant — mirrors the continuous -/// damage-prevention statics (Benevolent Unicorn class) and the self-scoped -/// `valid_card(SelfRef)` damage replacements (persistent prevention shields). +/// `DamageModification::PreventionMinus { value: N }` saturating-subtracts N +/// from each damage event whose recipient is this creature (`valid_card: +/// SelfRef`). `PreventionMinus` is the CR 615 prevention provenance of the +/// shared `Minus` subtraction authority — Absorb genuinely PREVENTS damage, so +/// it emits `DamagePrevented` bookkeeping, unlike the plain-arithmetic +/// `Minus` statics (Benevolent Unicorn class). It is NOT a consumed shield, so +/// it re-applies to every source and every event independently (CR 702.64b), +/// like the self-scoped `valid_card(SelfRef)` damage replacements (persistent +/// prevention shields). fn build_absorb_replacement(n: u32) -> ReplacementDefinition { ReplacementDefinition::new(ReplacementEvent::DamageDone) .valid_card(TargetFilter::SelfRef) - .damage_modification(DamageModification::Minus { value: n }) + .damage_modification(DamageModification::PreventionMinus { value: n }) .description(format!( "CR 702.64a: Absorb {n} — if a source would deal damage to this creature, \ prevent {n} of that damage." @@ -3728,7 +3731,7 @@ fn is_absorb_replacement(r: &ReplacementDefinition, n: u32) -> bool { && matches!(r.valid_card, Some(TargetFilter::SelfRef)) && matches!( r.damage_modification, - Some(DamageModification::Minus { value }) if value == n + Some(DamageModification::PreventionMinus { value }) if value == n ) } @@ -25200,9 +25203,11 @@ mod absorb_synthesis_tests { //! CR 702.64a shape tests: Absorb was parsed/typed but had no runtime. //! `synthesize_absorb` installs a continuous self-recipient `DamageDone` //! replacement that subtracts N from each incoming damage event - //! (`DamageModification::Minus { value: N }`, `valid_card: SelfRef`). The - //! continuous, non-consumed, per-source/per-event semantics (CR 702.64b) come - //! for free from `Minus`; CR 702.64c (each instance separate) is one + //! (`DamageModification::PreventionMinus { value: N }` — the CR 615 + //! prevention provenance of the shared `Minus` subtraction — + //! `valid_card: SelfRef`). The continuous, non-consumed, + //! per-source/per-event semantics (CR 702.64b) come for free from the + //! shared subtraction arm; CR 702.64c (each instance separate) is one //! replacement per instance. use super::*; use crate::game::effects::deal_damage; @@ -25287,9 +25292,9 @@ mod absorb_synthesis_tests { assert!( matches!( r.damage_modification, - Some(DamageModification::Minus { value: 2 }) + Some(DamageModification::PreventionMinus { value: 2 }) ), - "CR 702.64a: prevent N (=2) of the damage" + "CR 702.64a: prevent N (=2) of the damage (prevention provenance)" ); } diff --git a/crates/engine/src/game/replacement.rs b/crates/engine/src/game/replacement.rs index 7f53b1e119..7f3d5b1a07 100644 --- a/crates/engine/src/game/replacement.rs +++ b/crates/engine/src/game/replacement.rs @@ -1548,6 +1548,9 @@ fn damage_done_applier( ) -> ApplyResult { // Branch 1: Damage modification (Double, Triple, Plus, Minus) if let Some(modification) = damage_modification_for_rid(state, rid) { + // CR 510.2: identity for the combat-damage-batch prevention tally, taken + // before the event is destructured (mirrors the Branch 2 shield path). + let applied_key = AppliedReplacementKey::for_event(&event, rid); if let ProposedEvent::Damage { source_id, target, @@ -1556,6 +1559,15 @@ fn damage_done_applier( applied, } = event { + // CR 615.1a: typed prevention provenance, captured before the match + // consumes `modification` (the `Plus`/`SetTo` arms move their + // non-`Copy` payload out). ONLY `PreventionMinus` — the CR 615 + // prevention provenance of the shared subtraction — does prevention + // bookkeeping below; plain arithmetic `Minus` (Benevolent Unicorn's + // "that much damage minus 1") reduces the amount without preventing + // anything. + let is_minus_prevention = + matches!(modification, DamageModification::PreventionMinus { .. }); let new_amount = match modification { DamageModification::Double => amount.saturating_mul(2), DamageModification::Triple => amount.saturating_mul(3), @@ -1593,10 +1605,16 @@ fn damage_done_applier( .max(0) as u32; amount.saturating_add(added) } - // CR 615.1 + CR 614.1a: Saturating subtract. `Minus { value: u32::MAX }` - // is the continuous prevent-all sentinel — yields 0 for any amount and - // is not consumed (continuous, not shield-style). - DamageModification::Minus { value } => amount.saturating_sub(value), + // CR 615.1 + CR 614.1a: Saturating subtract — the ONE shared + // 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 + // 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), // CR 614.1a: Conditional — if amount < source's power, set to power. // References the replacement source's (rid.source) post-layer power. DamageModification::SetToSourcePower => { @@ -1644,6 +1662,57 @@ fn damage_done_applier( if let Some(ShieldKind::DamageReplacementOneShot) = shield_kind_for_rid(state, rid) { consume_prevention_shield(state, rid, None); } + // CR 615.1a + CR 702.64b + CR 510.2: `PreventionMinus` is the typed + // 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 + // 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: + // * outside a combat-damage batch, emit `DamagePrevented` per event + // and stamp the per-event prevented amount into + // `last_effect_count` so a "damage prevented this way" + // continuation resolves `QuantityRef::EventContextAmount` against + // THIS event's amount (CR 615.5; mirrors Branch 2's stamp); + // * inside a batch, accumulate into the per-replacement tally — the + // single `DamagePrevented` and the aggregate `last_effect_count` + // stamp happen post-batch in `fire_combat_prevention_riders` + // (CR 510.2 + CR 615.13), so nothing is emitted or stamped here; + // * exception (mirrors Branch 2): an `execute`-template follow-up + // drains per-event inside `replace_combat_damage_batch` and needs + // the per-event amount stamped even while the batch tally is + // active; a per-source-reflecting rider (Comeuppance class) must + // never aggregate at all. + // Plain arithmetic `Minus` and the increase/no-op modifications + // (Double, Triple, Plus, SetTo*, LifeFloor) are not prevention and + // record nothing. + if is_minus_prevention { + let prevented = amount.saturating_sub(new_amount); + if prevented > 0 { + let mut accumulated_in_batch = false; + if !shield_rider_reflects_per_event(state, rid) { + if let Some(tally) = state.combat_prevention_tally.as_mut() { + *tally.entry(applied_key).or_insert(0) += prevented as i32; + accumulated_in_batch = true; + } + } + let per_event_execute_followup = + accumulated_in_batch && shield_has_per_event_execute_followup(state, rid); + if !accumulated_in_batch || per_event_execute_followup { + if !accumulated_in_batch { + events.push(GameEvent::DamagePrevented { + source_id, + target: target.clone(), + amount: prevented, + }); + } + // CR 615.5: the prevented-amount handoff for follow-up + // continuations — identical to Branch 2's stamp. + state.last_effect_count = Some(prevented as i32); + } + } + } return ApplyResult::Modified(ProposedEvent::Damage { source_id, target, @@ -4715,7 +4784,15 @@ fn is_damage_prevention_replacement( return false; }; - // CR 614.1a: Damage boost/reduction replacements are definitively not prevention effects + // Ordinary damage modifications are not prevention, but `PreventionMinus` + // carries explicit prevention provenance and must be suppressed when damage + // can't be prevented. + if matches!( + repl.damage_modification, + Some(DamageModification::PreventionMinus { .. }) + ) { + return true; + } if repl.damage_modification.is_some() { return false; } @@ -7852,7 +7929,10 @@ fn damage_commute_class(modification: &DamageModification) -> CommuteClass { match modification { DamageModification::Double | DamageModification::Triple => CommuteClass::Multiplicative, DamageModification::Plus { .. } => CommuteClass::Additive, - DamageModification::Minus { .. } => CommuteClass::Subtractive, + // CR 616.1: both provenances of the shared subtraction commute alike. + DamageModification::Minus { .. } | DamageModification::PreventionMinus { .. } => { + CommuteClass::Subtractive + } DamageModification::SetToSourcePower | DamageModification::SetTo { .. } | DamageModification::LifeFloor { .. } => CommuteClass::NonCommuting, @@ -13881,6 +13961,135 @@ mod tests { } } + /// CR 614.1a vs CR 615: plain arithmetic `Minus` (Benevolent Unicorn's + /// "that much damage minus 1") is NOT prevention provenance — it must + /// reduce the amount WITHOUT emitting `DamagePrevented` and WITHOUT + /// stamping the CR 615.5 prevented-amount handoff. (Regression for the + /// review finding that every `Minus` was classified as prevention.) + #[test] + fn damage_applier_arithmetic_minus_is_not_prevention() { + let repl = damage_repl(DamageModification::Minus { value: 1 }); + let mut state = test_state_with_damage_repl(ObjectId(10), PlayerId(0), vec![repl]); + let mut events = Vec::new(); + let rid = ReplacementId { + source: ObjectId(10), + index: 0, + }; + let result = damage_done_applier(damage_event(3), rid, &mut state, &mut events); + match result { + ApplyResult::Modified(ProposedEvent::Damage { amount, .. }) => { + assert_eq!(amount, 2, "arithmetic Minus must still subtract"); + } + other => panic!("Expected Modified Damage, got {other:?}"), + } + assert!( + !events + .iter() + .any(|e| matches!(e, GameEvent::DamagePrevented { .. })), + "arithmetic Minus prevents nothing — no DamagePrevented may be emitted" + ); + assert_eq!( + state.last_effect_count, None, + "arithmetic Minus must not stamp the CR 615.5 prevented-amount handoff" + ); + } + + /// CR 615.1a + CR 615.5: the `PreventionMinus` provenance of the shared + /// subtraction must, OUTSIDE a combat batch, emit `DamagePrevented` for the + /// per-event prevented amount AND stamp it into `last_effect_count` so a + /// "damage prevented this way" continuation resolves + /// `QuantityRef::EventContextAmount` against THIS event's amount (mirrors + /// the Branch 2 shield stamp). Seeded with a stale count to prove the + /// binding overwrites it — without the stamp the continuation would read + /// the stale 999. + #[test] + fn damage_applier_prevention_minus_stamps_per_event_amount_for_continuations() { + let repl = damage_repl(DamageModification::PreventionMinus { value: 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(); + let rid = ReplacementId { + source: ObjectId(10), + index: 0, + }; + let result = damage_done_applier(damage_event(5), rid, &mut state, &mut events); + match result { + ApplyResult::Modified(ProposedEvent::Damage { amount, .. }) => { + assert_eq!(amount, 3, "PreventionMinus(2) must subtract from 5"); + } + other => panic!("Expected Modified Damage, got {other:?}"), + } + assert!( + events + .iter() + .any(|e| matches!(e, GameEvent::DamagePrevented { amount: 2, .. })), + "prevention provenance must emit DamagePrevented for the prevented 2" + ); + assert_eq!( + state.last_effect_count, + Some(2), + "the per-event prevented amount must be stamped for the rider handoff" + ); + // The continuation's view: resolve the prevented amount through the real + // quantity resolver, exactly as a "for each 1 damage prevented this way" + // rider would (`current_trigger_event` is None here, so the documented + // `last_effect_count` fallback is the read path). + let observed = crate::game::quantity::resolve_quantity( + &state, + &QuantityExpr::Ref { + qty: crate::types::ability::QuantityRef::EventContextAmount, + }, + PlayerId(0), + ObjectId(10), + ); + assert_eq!( + observed, 2, + "a prevented-amount continuation must observe the per-event amount" + ); + } + + /// 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 + /// into the per-replacement tally that `fire_combat_prevention_riders` + /// consumes (which emits the single event and stamps the batch total), + /// 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 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(); + let rid = ReplacementId { + source: ObjectId(10), + index: 0, + }; + let result = damage_done_applier(damage_event(5), rid, &mut state, &mut events); + match result { + ApplyResult::Modified(ProposedEvent::Damage { amount, .. }) => { + assert_eq!(amount, 3); + } + other => panic!("Expected Modified Damage, got {other:?}"), + } + assert!( + !events + .iter() + .any(|e| matches!(e, GameEvent::DamagePrevented { .. })), + "in-batch prevention must not emit per-source DamagePrevented (deferred)" + ); + assert_eq!( + state.last_effect_count, None, + "in-batch prevention must not stamp per-event — the aggregate stamp \ + happens post-batch so the rider sees the un-fragmented total" + ); + let tally = state.combat_prevention_tally.as_ref().unwrap(); + assert_eq!( + tally.values().copied().collect::>(), + vec![2], + "the prevented amount must accumulate into the per-replacement batch tally" + ); + } + #[test] fn damage_applier_life_floor_does_not_increase_damage() { let repl = damage_repl(DamageModification::LifeFloor { minimum: 1 }); diff --git a/crates/engine/src/parser/oracle_replacement.rs b/crates/engine/src/parser/oracle_replacement.rs index 34c152300e..91980c4045 100644 --- a/crates/engine/src/parser/oracle_replacement.rs +++ b/crates/engine/src/parser/oracle_replacement.rs @@ -9784,23 +9784,63 @@ fn parse_damage_prevention_replacement( // the amount. The bare "all" arm below must stay ordered after this one // because it shares the "all" prefix. let after_prevent = strip_after(working_lower, "prevent "); - let amount = if let Some((after_all_but, _)) = + // CR 615.1a: A prevention clause resolves to one of two representations on the + // resulting `DamageDone` replacement: + // * a `ShieldKind::Prevention` shield — "prevent all", "prevent all but N", + // the depleting "prevent the next N", or the redirection-context + // "prevent that damage"; or + // * for the continuous, non-depleting bare "prevent N of that damage" class + // (Heart-Shaped Herb — issue #5902, Sphere of Purity, Orbs of Warding, + // Urza's Armor, Guardian Seraph, Daunting Defender, ...), a + // `DamageModification::PreventionMinus { value: N }` that + // saturating-subtracts N from every qualifying damage event. + // + // CR 702.64: `PreventionMinus` deliberately reuses the same shared per-event + // damage-subtraction authority that Absorb already synthesizes + // (`database::synthesis::build_absorb_replacement`) rather than a new + // prevention-amount variant — it is the typed PREVENTION provenance of the + // shared `Minus` subtraction arm, so parser and resolver keep a single + // semantic representation for continuous per-event prevention (it is + // non-consumed and re-fires for every event — exactly Absorb's CR 702.64b + // semantics) while plain arithmetic `Minus` (Benevolent Unicorn) stays + // outside the prevention bookkeeping. + enum PreventionRepr { + Shield(PreventionAmount), + Reduce(u32), + } + let repr = if let Some((after_all_but, _)) = after_prevent.and_then(|s| tag::<_, _, OracleError<'_>>("all but ").parse(s).ok()) { let (n, _) = parse_number(after_all_but)?; - PreventionAmount::AllBut(n) + PreventionRepr::Shield(PreventionAmount::AllBut(n)) } else if nom_primitives::scan_contains(working_lower, "prevent all") { - PreventionAmount::All + PreventionRepr::Shield(PreventionAmount::All) } else if let Some(rest) = strip_after(working_lower, "prevent the next ") { // Uses oracle_util::parse_number (not nom directly) because it handles "X" → 0 // for cards like Temper, Acolyte's Reward, etc. let (n, _) = parse_number(rest)?; - PreventionAmount::Next(n) + PreventionRepr::Shield(PreventionAmount::Next(n)) } else if nom_primitives::scan_contains(working_lower, "prevent that damage") { // "prevent that damage" in redirection context — redirect handled separately - PreventionAmount::All + PreventionRepr::Shield(PreventionAmount::All) } else { - return None; + // CR 615.1a: bare "prevent N of that damage" — a numeric amount + // immediately following the "prevent " verb, anchored by the literal + // " of that damage" suffix DIRECTLY after the number (a composed nom + // sequence, not a scan), so a non-adjacent "of that damage" phrase + // elsewhere in the clause cannot be misbound as the anchor. + // `nom_primitives::parse_number` (unlike `parse_number_or_x`) never + // matches the bare pronoun "x", so the dynamic "prevent X ..." idiom is + // not swallowed into a static `PreventionMinus { value: 0 }` no-op and + // 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) }; // --- 2. Extract combat scope --- @@ -9916,8 +9956,22 @@ fn parse_damage_prevention_replacement( // --- 5. Build the replacement definition --- let mut def = ReplacementDefinition::new(ReplacementEvent::DamageDone) - .prevention_shield(amount) .description(original_text.to_string()); + def = match repr { + // CR 615.1a: shield-style prevention (all / all-but / depleting next / + // redirection-context "prevent that damage"). + PreventionRepr::Shield(amount) => def.prevention_shield(amount), + // CR 615.1a + CR 702.64: continuous "prevent N of that damage" reuses the + // shared `Minus` per-event subtraction authority (Branch 1 of + // `damage_done_applier`) under its typed PREVENTION provenance, + // `DamageModification::PreventionMinus` — never consumed, re-firing for + // 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 }) + } + }; if let Some(cs) = combat_scope { def = def.combat_scope(cs); @@ -12874,6 +12928,103 @@ mod tests { } } + /// CR 615.1a + CR 702.64: Heart-Shaped Herb (issue #5902) — "If a source an + /// opponent controls would deal damage to you, prevent 1 of that damage." + /// Before this fix the bare "prevent N of that damage" amount phrasing (no + /// "all" / "all but" / "the next") matched none of the amount branches, so + /// `parse_damage_prevention_replacement` returned `None` and the whole + /// static ability silently failed to install — matching the reported symptom + /// ("isn't affecting it at all"). Per the maintainer's CR review the class is + /// re-emitted onto the shared `Minus` per-event subtraction authority under + /// its typed prevention provenance, `DamageModification::PreventionMinus` + /// (the CR 702.64 Absorb representation), NOT a new prevention-amount + /// variant. This idiom is shared by many real cards + /// (Sphere of Purity, Orbs of Warding, Urza's Armor, Guardian Seraph, + /// Daunting Defender, ...), so the fix is generic. + #[test] + fn heart_shaped_herb_prevent_n_of_that_damage_is_minus_modification() { + let def = parse_replacement_line( + "If a source an opponent controls would deal damage to you, prevent 1 of that damage.", + "Heart-Shaped Herb", + ) + .expect("Heart-Shaped Herb should parse as damage prevention"); + + assert_eq!( + def.damage_modification, + Some(DamageModification::PreventionMinus { value: 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" + ); + assert_eq!( + def.shield_kind, + ShieldKind::None, + "the Minus representation must not also carry a prevention shield_kind" + ); + assert_eq!(def.event, ReplacementEvent::DamageDone); + assert_eq!( + def.damage_target_filter, + Some(damage_target_controller()), + "recipient must be the shield controller ('deal damage to you')" + ); + + let source_filter = def + .damage_source_filter + .as_ref() + .expect("'a source an opponent controls' must produce a source filter"); + match source_filter { + TargetFilter::Typed(tf) => { + assert_eq!( + tf.controller, + Some(ControllerRef::Opponent), + "source must be scoped to opponent-controlled, not any source" + ); + } + other => panic!("expected Typed opponent-controlled source filter, got {other:?}"), + } + } + + /// Sibling coverage for the same bare "prevent N of that damage" idiom with + /// N > 1 and no source-controller qualifier (Sphere of Purity-style). Pins + /// that the fix generalizes to other N and doesn't require an "an opponent + /// controls" clause to be present. + #[test] + fn bare_prevent_n_of_that_damage_generalizes_without_source_controller_clause() { + let def = parse_replacement_line( + "If a source would deal damage to equipped creature, prevent 2 of that damage.", + "Shield of the Realm", + ) + .expect("bare 'prevent N of that damage' with no controller qualifier should parse"); + + assert_eq!( + def.damage_modification, + Some(DamageModification::PreventionMinus { value: 2 }) + ); + assert_eq!(def.shield_kind, ShieldKind::None); + assert!( + def.damage_source_filter.is_none(), + "unqualified 'a source' must not synthesize a source filter" + ); + } + + /// CR 615.1a: the "prevent N of that damage" grammar is an ANCHORED nom + /// sequence — the number must be immediately followed by " of that damage". + /// A non-adjacent "of that damage" later in the clause must NOT satisfy the + /// anchor (the pre-fix scan-based check accepted it), and the clause must + /// fall through unrecognized rather than misbind the amount. + #[test] + fn prevent_n_requires_adjacent_of_that_damage_anchor() { + let def = parse_replacement_line( + "If a source would deal damage to you, prevent 2 damage this turn of that damage.", + "Anchor Probe", + ); + assert!( + def.is_none(), + "a non-adjacent 'of that damage' phrase must not satisfy the anchored \ + 'prevent N of that damage' grammar, got {def:?}" + ); + } + /// CR 614.1a + CR 615.5 + CR 608.2c: Vigor — "If damage would be dealt to /// another creature you control, prevent that damage. Put a +1/+1 counter /// on that creature for each 1 damage prevented this way." diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index b55e8cf8c5..3980901e9e 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -19739,13 +19739,32 @@ pub enum DamageModification { /// Talent +2); a `Ref` carries a live game quantity ("...plus X, where X is /// ~'s power" — Hawkeye, Young Avenger). Plus { value: QuantityExpr }, - /// amount.saturating_sub(value) (e.g. Benevolent Unicorn, -1). - /// CR 615.1 + CR 614.1a: Continuous prevention statics ("prevent that damage") - /// emit `Minus { value: u32::MAX }` — saturating-subtraction yields 0 for any - /// amount, and the replacement is not consumed (continuous, not shield-style). - /// This is distinct from `ShieldKind::Prevention { All }` (one-shot consumed - /// shield); the saturating-max sentinel covers the continuous case. + /// CR 614.1a: plain arithmetic subtraction — amount.saturating_sub(value) + /// (e.g. Benevolent Unicorn, "that much damage minus 1 instead"). This is + /// NOT prevention: no damage is "prevented" in the CR 615 sense, so the + /// applier emits no `DamagePrevented` bookkeeping and fires no prevention + /// riders for it. For the CR 615 prevention class that shares this exact + /// subtraction formula, see `PreventionMinus` — the two variants are typed + /// provenance over ONE shared applier subtraction arm, not two subtraction + /// authorities. Minus { value: u32 }, + /// CR 615.1a + CR 702.64b: continuous per-event damage PREVENTION carrying + /// the same saturating subtraction as `Minus` — "prevent N of that damage" + /// (Heart-Shaped Herb, Sphere of Purity, Orbs of Warding) and the CR 702.64 + /// Absorb synthesis. The variant tag is the typed prevention provenance + /// threaded from the producer (parser / keyword synthesis) through the + /// 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 + /// 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` because + /// the dormant, contributor-frozen `crates/mtgish-import` constructs + /// `Minus { value }` literals that a new mandatory field would break. + PreventionMinus { value: u32 }, /// 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." diff --git a/crates/engine/tests/integration/issue_5902_heart_shaped_herb.rs b/crates/engine/tests/integration/issue_5902_heart_shaped_herb.rs new file mode 100644 index 0000000000..5b30946899 --- /dev/null +++ b/crates/engine/tests/integration/issue_5902_heart_shaped_herb.rs @@ -0,0 +1,259 @@ +//! Issue #5902: Heart-Shaped Herb must prevent 1 damage from each opponent- +//! controlled source dealt to you, and must not affect damage from your own +//! sources. This is a discriminating engine scenario driving the real +//! `deal_damage::resolve` pipeline, not just parsing. +//! +//! Before this fix, "prevent N of that damage" (no "all" / "all but" / "the +//! next" qualifier) matched none of `parse_damage_prevention_replacement`'s +//! amount branches, so the whole static ability failed to parse into a +//! `ReplacementDefinition` at all — the reported symptom was that Heart- +//! Shaped Herb "isn't affecting it at all". +//! +//! Per the maintainer's CR review (add-engine-variant Stage 1 = +//! `REFUSE_WITH_EXISTING_SLOT`), the fix reuses the existing shared +//! per-event damage-subtraction authority — the `Minus` applier arm — under +//! its typed prevention provenance `DamageModification::PreventionMinus +//! { value: N }` on a `DamageDone` replacement, the same continuous, +//! non-consumed representation CR 702.64 Absorb already synthesizes +//! (`database::synthesis::build_absorb_replacement`) — instead of a new +//! `PreventionAmount` variant. Only the prevention provenance emits the +//! `DamagePrevented` bookkeeping the prevention shields emit; plain +//! arithmetic `Minus` (Benevolent Unicorn) does not. +//! +//! CR 615.1a (prevention shield) + CR 702.64b (continuous, non-depleting) + +//! CR 609.7b (controller-scoped source restriction). + +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, +}; +use engine::types::card_type::CoreType; +use engine::types::events::GameEvent; +use engine::types::player::PlayerId; + +const HEART_SHAPED_HERB: &str = + "If a source an opponent controls would deal damage to you, prevent 1 of that damage."; + +fn damage_to_player_ability( + source_id: engine::types::identifiers::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, + ) +} + +#[test] +fn heart_shaped_herb_prevents_one_from_opponent_sources_not_own_and_never_depletes() { + let mut scenario = GameScenario::new(); + // Built as a 0/1 creature so it survives build-time SBAs, then converted + // into a plain Artifact below — same convention as the Panther Habit + // (issue #5246) prevention test. + let herb = scenario + .add_creature_from_oracle(P0, "Heart-Shaped Herb", 0, 1, HEART_SHAPED_HERB) + .id(); + let opponent_source_a = scenario.add_creature(P1, "Opponent Source A", 3, 3).id(); + let opponent_source_b = scenario.add_creature(P1, "Opponent Source B", 3, 3).id(); + let own_source = scenario.add_creature(P0, "Own Source", 3, 3).id(); + let mut runner = scenario.build(); + + { + let obj = runner.state_mut().objects.get_mut(&herb).unwrap(); + obj.card_types.core_types = vec![CoreType::Artifact]; + obj.card_types.subtypes = vec![]; + obj.base_card_types = obj.card_types.clone(); + obj.power = None; + obj.toughness = None; + obj.base_power = None; + obj.base_toughness = None; + } + + // CR 615.1a + CR 702.64: the shield installs as a continuous + // `DamageModification::PreventionMinus { value: 1 }` `DamageDone` + // replacement — the shared per-event subtraction authority under its typed + // prevention provenance, NOT a bespoke prevention-amount variant — with + // `shield_kind` left as the default `None`. + let repl = &runner.state().objects[&herb].replacement_definitions[0]; + assert_eq!( + repl.damage_modification, + Some(DamageModification::PreventionMinus { value: 1 }), + "Heart-Shaped Herb must install a continuous PreventionMinus(1) damage replacement, got {:?}", + repl.damage_modification + ); + assert_eq!( + repl.shield_kind, + ShieldKind::None, + "the Minus representation must not also carry a prevention shield_kind, got {:?}", + repl.shield_kind + ); + + // Damage from an OPPONENT-controlled source is reduced by 1. + let p0_life_before = runner.life(P0); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_to_player_ability(opponent_source_a, P1, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("opponent damage to P0 resolves"); + assert_eq!( + runner.life(P0), + p0_life_before - 2, + "3 damage from an opponent-controlled source must be reduced to 2" + ); + assert!( + events + .iter() + .any(|e| matches!(e, GameEvent::DamagePrevented { amount: 1, .. })), + "the shared Minus authority must emit DamagePrevented for the 1 point it prevents" + ); + + // The shield must NOT be exhausted — a SECOND opponent-source damage + // event must also be reduced by 1 (CR 702.64b: continuous, non-consumed). + // A depleting shield would only ever prevent 1 damage total. + let p0_life_before = runner.life(P0); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_to_player_ability(opponent_source_b, P1, TargetRef::Player(P0), 4), + &mut events, + ) + .expect("second opponent damage to P0 resolves"); + assert_eq!( + runner.life(P0), + p0_life_before - 3, + "shield must re-fire on a second opponent-source event, not be exhausted by the first" + ); + assert!( + events + .iter() + .any(|e| matches!(e, GameEvent::DamagePrevented { amount: 1, .. })), + "must prevent exactly 1 of the second opponent-source damage event" + ); + + // Damage from the shield controller's OWN source is not affected at all + // (CR 609.7b: the shield is scoped to sources an opponent controls). + let p0_life_before = runner.life(P0); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_to_player_ability(own_source, P0, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("self-inflicted damage to P0 resolves"); + assert_eq!( + runner.life(P0), + p0_life_before - 3, + "damage from a source P0 controls must not be prevented by P0's own shield" + ); + assert!( + !events + .iter() + .any(|e| matches!(e, GameEvent::DamagePrevented { .. })), + "no prevention event should fire for a source the shield's controller controls" + ); + + // Damage that can't be prevented must bypass the typed + // `PreventionMinus` replacement while still allowing ordinary damage. + runner + .state_mut() + .restrictions + .push(GameRestriction::DamagePreventionDisabled { + source: herb, + expiry: RestrictionExpiry::EndOfTurn, + scope: None, + }); + let p0_life_before = runner.life(P0); + let mut events = Vec::new(); + deal_damage::resolve( + runner.state_mut(), + &damage_to_player_ability(opponent_source_a, P1, TargetRef::Player(P0), 3), + &mut events, + ) + .expect("unpreventable opponent damage resolves"); + assert_eq!( + runner.life(P0), + p0_life_before - 3, + "damage that can't be prevented must not be reduced by PreventionMinus" + ); + assert!( + !events + .iter() + .any(|e| matches!(e, GameEvent::DamagePrevented { .. })), + "unpreventable damage must emit no DamagePrevented event" + ); +} + +/// CR 614.1a vs CR 615: negative provenance regression (maintainer review on +/// PR #6307). Benevolent Unicorn's "it deals that much damage minus 1 instead" +/// is a plain ARITHMETIC damage replacement — `DamageModification::Minus`, the +/// same shared subtraction arm — but it is NOT prevention: no damage is +/// "prevented" in the CR 615 sense, so reducing an event must emit NO +/// `DamagePrevented` (which would wrongly feed prevention-triggered abilities) +/// and must NOT stamp the CR 615.5 "damage prevented this way" continuation +/// handoff. Before the typed-provenance fix, every `Minus` was classified as +/// prevention and this card emitted phantom prevention bookkeeping. +#[test] +fn benevolent_unicorn_arithmetic_minus_reduces_without_prevention_bookkeeping() { + let mut scenario = GameScenario::new(); + let unicorn = scenario + .add_creature_from_oracle( + P0, + "Benevolent Unicorn", + 1, + 2, + "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 mut runner = scenario.build(); + + // The static parses to the ARITHMETIC provenance of the shared subtraction: + // `Minus`, never `PreventionMinus`. + let repl = &runner.state().objects[&unicorn].replacement_definitions[0]; + assert_eq!( + repl.damage_modification, + Some(DamageModification::Minus { value: 1 }), + "'that much damage minus 1' must stay plain arithmetic Minus(1), got {:?}", + repl.damage_modification + ); + 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"); + assert_eq!( + runner.life(P0), + p0_life_before - 2, + "the arithmetic replacement must still reduce 3 damage to 2" + ); + assert!( + !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, + 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 3d937c8634..96bb77ac3b 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -551,6 +551,7 @@ mod issue_5820_susan_foreman; mod issue_5821_psychic_paper_attach_choice; mod issue_583_vivi_ornitier_mana_source; mod issue_5901_depthshaker_titan; +mod issue_5902_heart_shaped_herb; mod issue_5910_kitchen_finks_persist; mod issue_5945_kellan_the_kid; mod issue_5946_pest_infestation_bogwater_softlock; diff --git a/crates/mtgish-import/src/convert/mod.rs b/crates/mtgish-import/src/convert/mod.rs index e6921eddec..3e74477a84 100644 --- a/crates/mtgish-import/src/convert/mod.rs +++ b/crates/mtgish-import/src/convert/mod.rs @@ -834,7 +834,8 @@ fn convert_rule( // CR 614.2 + CR 615.1: Damage replacement effects. Maps the event // shape to `damage_*_filter` / `combat_scope` slots and the action // (PreventThatDamage / PreventSomeOfThatDamage / CancelThatDamage) - // to `damage_modification` (Minus { u32::MAX } / Minus). Other actions + // to `damage_modification` (PreventionMinus { u32::MAX } / PreventionMinus). + // Other actions // strict-fail until further engine extensions. Rule::ReplaceWouldDealDamage(event, actions) => { let mut reps = replacement::convert_replace_would_deal_damage(event, actions)?; diff --git a/crates/mtgish-import/src/convert/replacement.rs b/crates/mtgish-import/src/convert/replacement.rs index 2f00dc4f38..1b9a2f6319 100644 --- a/crates/mtgish-import/src/convert/replacement.rs +++ b/crates/mtgish-import/src/convert/replacement.rs @@ -411,27 +411,28 @@ fn damage_action_to_modification( match act { // CR 615.1: "Prevent that damage." / "If a source would deal damage // ... prevent that damage." Continuous prevent-all replacement encoded - // as `Minus { value: u32::MAX }` — saturating-subtraction yields 0 for - // any amount and the replacement is not consumed. + // as `PreventionMinus { value: u32::MAX }` — saturating-subtraction + // yields 0 for any amount and the replacement is not consumed. ReplacementActionWouldDealDamage::PreventThatDamage | ReplacementActionWouldDealDamage::CancelThatDamage => { - Ok(DamageModification::Minus { value: u32::MAX }) + Ok(DamageModification::PreventionMinus { value: u32::MAX }) } // "Prevent N of that damage." ReplacementActionWouldDealDamage::PreventSomeOfThatDamage(g) => { let qty = quantity::convert(g)?; match qty { - QuantityExpr::Fixed { value } if (0..=u32::MAX as i32).contains(&value) => { - Ok(DamageModification::Minus { - value: value as u32, - }) - } + QuantityExpr::Fixed { value } => u32::try_from(value) + .map(|value| DamageModification::PreventionMinus { value }) + .map_err(|_| ConversionGap::EnginePrerequisiteMissing { + engine_type: "DamageModification", + needed_variant: "PreventionMinus { count: QuantityExpr }".into(), + }), // CR 615.1: Dynamic prevention amount ("prevent X damage, - // where X is …") — engine `DamageModification::Minus` + // where X is …") — engine `DamageModification::PreventionMinus` // takes only `u32`, not `QuantityExpr`. _ => Err(ConversionGap::EnginePrerequisiteMissing { engine_type: "DamageModification", - needed_variant: "Minus { count: QuantityExpr }".into(), + needed_variant: "PreventionMinus { count: QuantityExpr }".into(), }), } } @@ -449,11 +450,13 @@ fn damage_action_to_modification( } let qty = quantity::convert(g)?; match qty { - QuantityExpr::Fixed { value } if (0..=u32::MAX as i32).contains(&value) => { - Ok(DamageModification::SetTo { - value: value as u32, - }) - } + QuantityExpr::Fixed { value } => u32::try_from(value) + .map(|value| DamageModification::SetTo { value }) + .map_err(|_| ConversionGap::MalformedIdiom { + idiom: "DamageAction/DealDamageInstead", + path: String::new(), + detail: "non-fixed override amount needs dynamic SetTo".into(), + }), _ => Err(ConversionGap::MalformedIdiom { idiom: "DamageAction/DealDamageInstead", path: String::new(), @@ -3284,8 +3287,8 @@ fn expiration_tag(e: &Expiration) -> String { #[cfg(test)] mod tests { use engine::types::ability::{ - AbilityCost, ContinuousModification, Duration, Effect, QuantityExpr, ReplacementMode, - TargetFilter, + AbilityCost, ContinuousModification, DamageModification, Duration, Effect, QuantityExpr, + ReplacementMode, TargetFilter, }; use engine::types::card_type::{CoreType, Supertype}; use engine::types::keywords::Keyword; @@ -3294,10 +3297,40 @@ mod tests { use crate::schema::types::{ CardInExile, CardType, Condition, CopyEffect, CopyEffects, FutureReplacableEventWouldDealDamage, GameNumber, Permanent, Permanents, Player, Players, - ReplacementActionWouldDealDamage, ReplacementActionWouldEnter, Rule, SingleDamageSource, - SuperType, + ReplacableEventWouldDealDamage, ReplacementActionWouldDealDamage, + ReplacementActionWouldEnter, Rule, SingleDamageSource, SuperType, }; + #[test] + fn would_deal_damage_fixed_actions_convert_to_typed_modifications() { + let defs = convert_replace_would_deal_damage( + &ReplacableEventWouldDealDamage::CombatDamageWouldBeDealt, + &[ + ReplacementActionWouldDealDamage::PreventThatDamage, + ReplacementActionWouldDealDamage::PreventSomeOfThatDamage(Box::new( + GameNumber::Integer(2), + )), + ReplacementActionWouldDealDamage::DealDamageInstead(Box::new(GameNumber::Integer( + 3, + ))), + ], + ) + .expect("fixed damage replacement actions should convert"); + + assert!(matches!( + defs[0].damage_modification.as_ref(), + Some(DamageModification::PreventionMinus { value: u32::MAX }) + )); + assert!(matches!( + defs[1].damage_modification.as_ref(), + Some(DamageModification::PreventionMinus { value: 2 }) + )); + assert!(matches!( + defs[2].damage_modification.as_ref(), + Some(DamageModification::SetTo { value: 3 }) + )); + } + #[test] fn as_enters_may_pay_life_unless_tapped_lowers_to_single_cost_gate() { let defs = convert_as_enters(