diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index 797a166e61..6a0e6e9ba1 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -5175,6 +5175,9 @@ fn rw_effect( sides: _, results, modifier: _, + // CR 706.6: keep-highest is a pure result-lookup aggregate with no + // read/write axis of its own — the branch effects already contribute. + keep: _, } => { let mut p = rw_quantity_expr(count); for r in results { diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index 12e2f36405..a1d3936830 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -4886,6 +4886,7 @@ mod tests { sides: 6, results: Vec::new(), modifier: None, + keep: crate::types::ability::DieRollAggregate::EachIndependently, })); assert!(effect_is_randomness_bearing(&Effect::FlipCoinUntilLose { win_effect: Box::new(AbilityDefinition::new(AbilityKind::Spell, Effect::NoOp)), diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index ac6d3fcf21..a6b171070c 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -19,8 +19,8 @@ use crate::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, ActivationRestriction, AdditionalCost, AggregateFunction, AttackScope, AttackSubject, CardTypeSetSource, ChoiceType, CoinFlipResult, Comparator, ContinuousModification, ControllerRef, CountScope, - CounterSourceRider, DelayedTriggerCondition, DieRollModifier, DoublePTMode, Duration, - EachDamageRecipient, Effect, EffectOutcomeSignal, EffectScope, FilterProp, + CounterSourceRider, DelayedTriggerCondition, DieRollAggregate, DieRollModifier, DoublePTMode, + Duration, EachDamageRecipient, Effect, EffectOutcomeSignal, EffectScope, FilterProp, ForEachCategoryAction, GameRestriction, LibraryPosition, ManaProduction, ObjectProperty, ObjectScope, PerpetualModification, PlayerFilter, PlayerScope, PtStat, PtValue, PtValueScope, QuantityExpr, QuantityRef, ReplacementCondition, ReplacementDefinition, ReplacementMode, @@ -3045,6 +3045,7 @@ fn effect_details(effect: &Effect) -> Vec<(String, String)> { sides, results, modifier, + keep, } => { if !matches!(count, QuantityExpr::Fixed { value: 1 }) { d.push(("count".into(), fmt_quantity(count))); @@ -3060,6 +3061,12 @@ fn effect_details(effect: &Effect) -> Vec<(String, String)> { }; d.push(("modifier".into(), label.into())); } + // CR 706.6: surface keep-highest aggregation ("ignore all but the + // highest roll"). + match keep { + DieRollAggregate::EachIndependently => {} + DieRollAggregate::Highest => d.push(("keep".into(), "highest".into())), + } } Effect::FlipCoin { win_effect, diff --git a/crates/engine/src/game/effects/effect.rs b/crates/engine/src/game/effects/effect.rs index bc34402676..dffeb8495d 100644 --- a/crates/engine/src/game/effects/effect.rs +++ b/crates/engine/src/game/effects/effect.rs @@ -2965,6 +2965,7 @@ mod tests { sides: 6, results: vec![], modifier: None, + keep: crate::types::ability::DieRollAggregate::EachIndependently, }, vec![], creature, diff --git a/crates/engine/src/game/effects/gain_control.rs b/crates/engine/src/game/effects/gain_control.rs index 15340d7d80..c2adc8f127 100644 --- a/crates/engine/src/game/effects/gain_control.rs +++ b/crates/engine/src/game/effects/gain_control.rs @@ -1109,6 +1109,7 @@ mod tests { sides: 4, results: vec![], modifier: None, + keep: crate::types::ability::DieRollAggregate::EachIndependently, }, vec![], purse, diff --git a/crates/engine/src/game/effects/roll_die.rs b/crates/engine/src/game/effects/roll_die.rs index ffd719086a..7de41492ce 100644 --- a/crates/engine/src/game/effects/roll_die.rs +++ b/crates/engine/src/game/effects/roll_die.rs @@ -1,7 +1,10 @@ use rand::Rng; use crate::game::quantity::resolve_quantity; -use crate::types::ability::{DieRollModifier, Effect, EffectError, EffectKind, ResolvedAbility}; +use crate::types::ability::{ + DieResultBranch, DieRollAggregate, DieRollModifier, Effect, EffectError, EffectKind, + ResolvedAbility, +}; use crate::types::events::GameEvent; use crate::types::game_state::GameState; @@ -38,13 +41,14 @@ pub fn resolve( ability: &ResolvedAbility, events: &mut Vec, ) -> Result<(), EffectError> { - let (count_expr, sides, results, modifier) = match &ability.effect { + let (count_expr, sides, results, modifier, keep) = match &ability.effect { Effect::RollDie { count, sides, results, modifier, - } => (count, *sides, results, modifier.as_ref()), + keep, + } => (count, *sides, results, modifier.as_ref(), *keep), _ => return Err(EffectError::MissingParam("RollDie".to_string())), }; @@ -55,6 +59,7 @@ pub fn resolve( resolve_quantity(state, count_expr, ability.controller, ability.source_id).max(0) as u32; let mut total_actual = 0_i32; + let mut highest_actual: Option = None; let mut rolled_any = false; for _ in 0..count { @@ -91,35 +96,42 @@ pub fn resolve( let actual_amount = i32::from(actual); total_actual = total_actual.saturating_add(actual_amount); + highest_actual = Some(highest_actual.map_or(actual, |h| h.max(actual))); rolled_any = true; - // CR 706.2 + CR 706.3a: The stored value is this die's actual result - // while its results-table branch resolves. - state.die_result_this_resolution = Some(actual_amount); - - // CR 706.3a: Find the matching result branch and resolve its effect. - // Each die consults the same table independently. - if let Some(branch) = results.iter().find(|b| actual >= b.min && actual <= b.max) { - // CR 608.2c: Branch bodies are full `AbilityDefinition`s (player_scope, - // sub_abilities, conditions, etc.). `ResolvedAbility::new` with only the - // effect drops `player_scope`, so "each opponent loses N life" on a d20 - // table (Herald of Hadar) incorrectly hit the controller (#2026). - let sub = build_resolved_from_def_with_targets( - &branch.effect, - ability.source_id, - ability.controller, - ability.targets.clone(), - ); - resolve_ability_chain(state, &sub, events, 0)?; + // CR 706.3a: For per-die aggregation, each die independently consults + // the results table using its own actual result, right after it is + // rolled. For keep-highest (CR 706.6) the table is deferred to a single + // lookup after every die is rolled, so skip the per-die resolution here. + if matches!(keep, DieRollAggregate::EachIndependently) { + // CR 706.2 + CR 706.3a: The stored value is this die's actual result + // while its results-table branch resolves. + state.die_result_this_resolution = Some(actual_amount); + resolve_matching_branch(state, results, actual, ability, events)?; + } + } + + // CR 706.6: "ignore all but the highest roll" — the ignored rolls are + // treated as never having happened, so the results table is consulted + // exactly once against the single highest actual result. Every die still + // emitted its own `DieRolled` event above; only the table lookup collapses. + if matches!(keep, DieRollAggregate::Highest) { + if let Some(highest) = highest_actual { + state.die_result_this_resolution = Some(i32::from(highest)); + resolve_matching_branch(state, results, highest, ability, events)?; } } if rolled_any { // CR 706.4: For no-table rolls, the outer sub_ability is resolved by - // the caller after this function returns. Leave the aggregate result - // available so "equal to the result(s)" reads all dice, not just the - // last one. - state.die_result_this_resolution = Some(total_actual); + // the caller after this function returns. Leave the result available so + // "equal to the result(s)" reads the correct value: the aggregate total + // for independent rolls, or the single kept result for keep-highest + // (CR 706.6 — the ignored rolls contribute nothing). + state.die_result_this_resolution = Some(match keep { + DieRollAggregate::EachIndependently => total_actual, + DieRollAggregate::Highest => highest_actual.map_or(0, i32::from), + }); } else { state.die_result_this_resolution = None; } @@ -133,6 +145,34 @@ pub fn resolve( Ok(()) } +/// CR 706.3a: Find the results-table branch whose `min..=max` range contains +/// `result` and resolve its effect. Shared by both aggregation modes so the +/// per-die (each independently) and single keep-highest lookups build the +/// branch ability identically — preserving branch `player_scope`/sub-abilities +/// (CR 608.2c, issue #2026). +fn resolve_matching_branch( + state: &mut GameState, + results: &[DieResultBranch], + result: u8, + ability: &ResolvedAbility, + events: &mut Vec, +) -> Result<(), EffectError> { + if let Some(branch) = results.iter().find(|b| result >= b.min && result <= b.max) { + // CR 608.2c: Branch bodies are full `AbilityDefinition`s (player_scope, + // sub_abilities, conditions, etc.). `ResolvedAbility::new` with only the + // effect drops `player_scope`, so "each opponent loses N life" on a d20 + // table (Herald of Hadar) incorrectly hit the controller (#2026). + let sub = build_resolved_from_def_with_targets( + &branch.effect, + ability.source_id, + ability.controller, + ability.targets.clone(), + ); + resolve_ability_chain(state, &sub, events, 0)?; + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -160,6 +200,7 @@ mod tests { sides: 20, results: vec![branch], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -207,6 +248,7 @@ mod tests { sides: 20, results: vec![branch], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -230,6 +272,7 @@ mod tests { sides: 6, results: vec![], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -288,6 +331,7 @@ mod tests { }, }, }), + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -334,6 +378,7 @@ mod tests { }, }, }), + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -365,6 +410,7 @@ mod tests { sides: 20, results: vec![], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -406,6 +452,7 @@ mod tests { sides, results: vec![], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -443,6 +490,7 @@ mod tests { sides: 20, results: vec![], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -539,6 +587,7 @@ mod tests { }, }, }), + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -614,6 +663,7 @@ mod tests { }, }, }), + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -644,6 +694,7 @@ mod tests { sides: 20, results: vec![], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -711,6 +762,7 @@ mod tests { sides: 20, results: vec![], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -777,6 +829,7 @@ mod tests { sides: 6, results: vec![], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -843,6 +896,7 @@ mod tests { sides: 6, results: vec![], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -892,6 +946,7 @@ mod tests { sides: 20, results: vec![branch], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -923,6 +978,7 @@ mod tests { sides: 6, results: vec![], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -982,6 +1038,7 @@ mod tests { sides: 6, results: vec![branch], modifier: None, + keep: DieRollAggregate::EachIndependently, }, vec![], ObjectId(1), @@ -996,4 +1053,119 @@ mod tests { "each of the two dice must resolve the 1..=6 branch, drawing one card per die" ); } + + /// Roll both dice for a given seed, returning the two d6 results (in roll + /// order). Shared by the keep-highest tests to locate a seed whose two dice + /// differ so "highest" is unambiguous. + fn two_d6_rolls(seed: u64) -> (u8, u8) { + let mut state = GameState::new_two_player(seed); + let ability = ResolvedAbility::new( + Effect::RollDie { + count: QuantityExpr::Fixed { value: 2 }, + sides: 6, + results: vec![], + modifier: None, + keep: DieRollAggregate::EachIndependently, + }, + vec![], + ObjectId(1), + PlayerId(0), + ); + let mut events = Vec::new(); + resolve(&mut state, &ability, &mut events).unwrap(); + let rolls: Vec = events + .iter() + .filter_map(|e| match e { + GameEvent::DieRolled { + result: Some(result), + sides: 6, + .. + } => Some(*result), + _ => None, + }) + .collect(); + assert_eq!(rolls.len(), 2, "count == 2 must emit two rolls"); + (rolls[0], rolls[1]) + } + + /// CR 706.6: With `keep: Highest`, a count-2 roll consults the results table + /// exactly ONCE, against the single highest actual result. Constructed with a + /// branch that covers ONLY the higher of the two distinct dice and draws one + /// card: the draw happens iff the table was consulted for the MAX (not the + /// min, not both, not the last die). A per-die (`EachIndependently`) + /// resolution over the same branch would draw 0 or 1 depending on which die + /// matched — this asserts exactly 1, discriminating keep-highest. + #[test] + fn roll_die_keep_highest_consults_table_once_for_max() { + // Locate a seed whose two dice differ so "highest" is a strict maximum. + let seed = (0..10_000u64) + .find(|&s| { + let (a, b) = two_d6_rolls(s); + a != b + }) + .expect("some seed in 0..10000 must roll two distinct d6 values"); + let (a, b) = two_d6_rolls(seed); + let (lo, hi) = (a.min(b), a.max(b)); + + let mut state = GameState::new_two_player(seed); + // Branch covers ONLY the maximum face value. + let branch = DieResultBranch { + min: hi, + max: hi, + effect: Box::new(AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: crate::types::ability::TargetFilter::Controller, + }, + )), + }; + for i in 0..5 { + crate::game::zones::create_object( + &mut state, + crate::types::identifiers::CardId(8000 + i as u64), + PlayerId(0), + format!("Card {i}"), + crate::types::zones::Zone::Library, + ); + } + let ability = ResolvedAbility::new( + Effect::RollDie { + count: QuantityExpr::Fixed { value: 2 }, + sides: 6, + results: vec![branch], + modifier: None, + keep: DieRollAggregate::Highest, + }, + vec![], + ObjectId(1), + PlayerId(0), + ); + let mut events = Vec::new(); + resolve(&mut state, &ability, &mut events).unwrap(); + + // Both dice were still physically rolled (CR 706.6: the ignored roll + // happened, it is only its result that is ignored for the table). + let rolls = events + .iter() + .filter(|e| matches!(e, GameEvent::DieRolled { sides: 6, .. })) + .count(); + assert_eq!(rolls, 2, "keep-highest must still roll all {} dice", 2); + + // The table fired exactly once, for the MAX: one card drawn. If the min + // die drove the table (lo != hi), zero cards would be drawn; if both dice + // consulted the table, we could not draw exactly one with a max-only + // branch — so exactly 1 discriminates keep-highest-for-max. + assert_eq!( + state.players[0].hand.len(), + 1, + "keep-highest must consult the max-only branch exactly once (dice = {lo},{hi})" + ); + // The stored resolution result is the max, not the total or the min. + assert_eq!( + state.die_result_this_resolution, + Some(i32::from(hi)), + "keep-highest must expose the max as the resolution result" + ); + } } diff --git a/crates/engine/src/game/mana_abilities.rs b/crates/engine/src/game/mana_abilities.rs index b9fdfe255c..ab432301ae 100644 --- a/crates/engine/src/game/mana_abilities.rs +++ b/crates/engine/src/game/mana_abilities.rs @@ -4014,6 +4014,7 @@ mod tests { sides: 20, results: vec![], modifier: None, + keep: crate::types::ability::DieRollAggregate::EachIndependently, }, ))); assert!(is_mana_ability(&def)); @@ -4041,6 +4042,7 @@ mod tests { sides: 20, results: vec![], modifier: None, + keep: crate::types::ability::DieRollAggregate::EachIndependently, }, ))); let mut events = Vec::new(); diff --git a/crates/engine/src/game/printed_cards.rs b/crates/engine/src/game/printed_cards.rs index 8fe5d9d215..235fb5a6a6 100644 --- a/crates/engine/src/game/printed_cards.rs +++ b/crates/engine/src/game/printed_cards.rs @@ -3408,6 +3408,7 @@ mod tests { effect: Box::new(conjure_ability("roll", Zone::Hand)), }], modifier: None, + keep: crate::types::ability::DieRollAggregate::EachIndependently, }; walk_effect(&roll, &mut names); diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index ee28df0cd0..ee170b9191 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -9973,6 +9973,11 @@ pub(super) fn parse_imperative_family_ast( .is_some() { Some(ImperativeFamilyAst::RollToVisitAttractions) + } else if let Some(ast) = try_parse_roll_keep_highest_for_each(lower) { + // CR 706.6: "roll a d{N} for each and ignore all but + // the highest roll" (Iron Mastiff) — rolls one die per subject + // and consults the result table once against the highest. + Some(ast) } else if let Some(ast) = try_parse_roll_n_dice(lower) { // CR 706.1: "roll two six-sided dice" / "roll X d12" — the // multi-dice form. Tried before the single-die path; returns @@ -9984,6 +9989,7 @@ pub(super) fn parse_imperative_family_ast( count: QuantityExpr::Fixed { value: 1 }, sides, modifier, + keep: crate::types::ability::DieRollAggregate::EachIndependently, } }) } @@ -10733,9 +10739,65 @@ fn try_parse_roll_n_dice(lower: &str) -> Option { // vanishingly rare and parsed via the table path; the bare form has // no modifier. modifier: None, + // CR 706.3a: "roll two six-sided dice" resolves each die's result + // against the table independently. + keep: crate::types::ability::DieRollAggregate::EachIndependently, + }) +} + +/// CR 706.6: Parse "roll a d{N} for each and ignore all but the +/// highest roll" (Iron Mastiff: "roll a d20 for each player being attacked and +/// ignore all but the highest roll"). One die is rolled per counted subject, +/// but the results table is consulted exactly once against the single highest +/// result (`DieRollAggregate::Highest`). +/// +/// `count` is bound to the player-count the "for each" clause names; the only +/// shape currently attested is "for each player being attacked" (the players +/// this creature is attacking this combat, CR 508.6). Any other "for each ..." +/// subject falls through so its own quantity parser can claim it later — this +/// combinator returns `None` rather than guessing. +fn try_parse_roll_keep_highest_for_each(lower: &str) -> Option { + let (sides, rest) = try_parse_roll_die_sides_with_rest(lower)?; + let rest = rest.trim_start(); + // "for each " binds how many dice to roll (one per subject). + let (after_for_each, _) = tag::<_, _, OracleError<'_>>("for each ").parse(rest).ok()?; + let (after_subject, count) = parse_keep_highest_for_each_subject(after_for_each)?; + // The clause must close with "and ignore all but the highest roll" — this is + // the marker that collapses the table to a single highest-result lookup. + let after_subject = after_subject.trim_start(); + let (tail, _) = tag::<_, _, OracleError<'_>>("and ignore all but the highest roll") + .parse(after_subject) + .ok()?; + if !tail.trim_end_matches(['.', ',', ';']).trim().is_empty() { + return None; + } + Some(ImperativeFamilyAst::RollDie { + count, + sides, + modifier: None, + keep: crate::types::ability::DieRollAggregate::Highest, }) } +/// CR 508.6: Parse the "for each " subject of a keep-highest roll, +/// returning the remainder and the `QuantityExpr` counting those players. +/// "player being attacked" → the players this creature is attacking this combat +/// (`PlayerCount { OpponentAttacked { Source, ThisCombat } }`). +fn parse_keep_highest_for_each_subject(input: &str) -> Option<(&str, QuantityExpr)> { + let (rest, qty) = value( + QuantityRef::PlayerCount { + filter: crate::types::ability::PlayerFilter::OpponentAttacked { + subject: crate::types::ability::AttackSubject::Source, + scope: crate::types::ability::AttackScope::ThisCombat, + }, + }, + tag::<_, _, OracleError<'_>>("player being attacked "), + ) + .parse(input) + .ok()?; + Some((rest, QuantityExpr::Ref { qty })) +} + /// CR 706.1a: Returns `(sides, remainder)`. The remainder is the slice immediately after /// the consumed die phrase, with whitespace untrimmed. Callers needing to /// attach trailing modifiers / clauses can branch on the remainder shape. @@ -11758,11 +11820,13 @@ fn lower_imperative_family_effect(ast: ImperativeFamilyAst) -> Effect { count, sides, modifier, + keep, } => Effect::RollDie { count, sides, results: vec![], modifier, + keep, }, // CR 705.2: the bare imperative lowers with `flipper = Controller`; a // player subject ("that player flips a coin") is stamped onto `flipper` @@ -19040,6 +19104,7 @@ mod tests { sides, modifier, results, + keep: _, } => { assert_eq!(*sides, 20); assert_eq!( diff --git a/crates/engine/src/parser/oracle_ir/ast.rs b/crates/engine/src/parser/oracle_ir/ast.rs index 3d11f03e9d..3242e40eac 100644 --- a/crates/engine/src/parser/oracle_ir/ast.rs +++ b/crates/engine/src/parser/oracle_ir/ast.rs @@ -683,6 +683,9 @@ pub(crate) enum ImperativeFamilyAst { count: crate::types::ability::QuantityExpr, sides: u8, modifier: Option, + /// CR 706.6: `Highest` for "roll a d{N} for each ... and ignore all but + /// the highest roll" (Iron Mastiff); `EachIndependently` otherwise. + keep: crate::types::ability::DieRollAggregate, }, /// CR 705: Flip a coin. FlipCoin, diff --git a/crates/engine/src/parser/oracle_special.rs b/crates/engine/src/parser/oracle_special.rs index ab357967dc..0bd36d9d84 100644 --- a/crates/engine/src/parser/oracle_special.rs +++ b/crates/engine/src/parser/oracle_special.rs @@ -310,6 +310,9 @@ pub(super) fn try_parse_die_roll_table( sides, results: branches, modifier, + // CR 706.3a: a single-die table roll consults the table for that + // one result; the keep-highest axis only matters for multi-die rolls. + keep: crate::types::ability::DieRollAggregate::EachIndependently, }, ); def.description = Some(line.to_string()); diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 7f72dcf8f1..0c30e7ee28 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -659,6 +659,7 @@ fn trigger_conjunctive_battlefield_condition_does_not_corrupt_roll_die_effect() sides: 20, results: vec![], modifier: None, + keep: crate::types::ability::DieRollAggregate::EachIndependently, }, "the conjunctive condition's residual \"and you control...\" text must not \ leak into the effect body and corrupt the RollDie parse" diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index d311e2abd5..db31b5ec58 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -1103,6 +1103,26 @@ pub enum DieRollModifier { Subtract { value: QuantityExpr }, } +/// How a multi-die `RollDie`'s result table is consulted after all `count` +/// dice are rolled. Leaf-level parameterization of `RollDie`'s result-lookup +/// axis (not a sibling effect): every die is still rolled and emits its own +/// `DieRolled` event; this only governs *which* result(s) drive the table. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "type")] +pub enum DieRollAggregate { + /// CR 706.3a: Each die independently uses its own result to determine which + /// effect on the results table happens. A count-N roll consults the table N + /// times. This is the default and preserves the single-die behavior for + /// count == 1 (max of one roll is that roll). + #[default] + EachIndependently, + /// CR 706.6: "roll a d20 for each player being attacked and ignore all but + /// the highest roll" (Iron Mastiff). Ignored rolls are treated as never + /// having happened, so the results table is consulted exactly ONCE, against + /// the single highest actual result among all `count` dice. + Highest, +} + impl std::str::FromStr for Parity { type Err = (); fn from_str(s: &str) -> Result { @@ -11848,6 +11868,16 @@ pub enum Effect { results: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] modifier: Option, + /// CR 706.6: For multi-die rolls, whether every die independently + /// consults the results table (`EachIndependently`, default) or only the + /// single highest actual result does (`Highest` — "ignore all but the + /// highest roll", Iron Mastiff). Skipped in serialization when the + /// default, so all existing single-die card-data stays byte-identical. + #[serde( + default = "default_die_roll_aggregate", + skip_serializing_if = "is_die_roll_aggregate_each_independently" + )] + keep: DieRollAggregate, }, /// CR 705: Flip a coin. Optionally execute different effects on win/lose. /// @@ -13010,6 +13040,17 @@ fn default_quantity_one() -> QuantityExpr { QuantityExpr::Fixed { value: 1 } } +/// Serde default for `Effect::RollDie::keep` — data written before the +/// keep-highest axis existed (and every single-die roll) keeps the per-die +/// table behavior (CR 706.3a). +fn default_die_roll_aggregate() -> DieRollAggregate { + DieRollAggregate::EachIndependently +} + +fn is_die_roll_aggregate_each_independently(agg: &DieRollAggregate) -> bool { + matches!(agg, DieRollAggregate::EachIndependently) +} + fn default_duration_until_end_of_turn() -> Duration { Duration::UntilEndOfTurn } diff --git a/crates/engine/tests/integration/iron_mastiff_roll_d20_attack_5928.rs b/crates/engine/tests/integration/iron_mastiff_roll_d20_attack_5928.rs new file mode 100644 index 0000000000..87d651fed3 --- /dev/null +++ b/crates/engine/tests/integration/iron_mastiff_roll_d20_attack_5928.rs @@ -0,0 +1,287 @@ +//! Issue #5928 — Iron Mastiff's "roll a d20 for each player being attacked and +//! ignore all but the highest roll" attack trigger with a d20 outcome table. +//! +//! Oracle (Iron Mastiff, Scryfall-verified): +//! > Whenever this creature attacks, roll a d20 for each player being attacked +//! > and ignore all but the highest roll. +//! > 1—9 | This creature deals damage equal to its power to you. +//! > 10—19 | This creature deals damage equal to its power to defending player. +//! > 20 | This creature deals damage equal to its power to each opponent. +//! +//! Before the fix, the attacks-trigger effect parsed to +//! `Unimplemented("roll", …)` and the three outcome rows landed as three +//! detached abilities, so the roll never happened and no damage was dealt. +//! +//! These tests drive real combat (Iron Mastiff, a 4/4, attacks P1), force a +//! roll into each outcome bucket by scanning seeds, and assert that the correct +//! player's life dropped by 4 (the mastiff's power) and the OTHER rows' targets +//! were untouched. Because "ignore all but the highest roll" collapses the +//! table to a single lookup, exactly one row fires per attack. + +use engine::game::combat::AttackTarget; +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::types::actions::GameAction; +use engine::types::events::GameEvent; +use engine::types::game_state::WaitingFor; +use engine::types::phase::Phase; + +use engine::parser::oracle::parse_oracle_text; +use engine::types::ability::{DieRollAggregate, Effect, PlayerFilter, QuantityExpr, QuantityRef}; +use engine::types::triggers::TriggerMode; + +const IRON_MASTIFF_ORACLE: &str = "Whenever this creature attacks, roll a d20 for each player \ +being attacked and ignore all but the highest roll.\n1—9 | This creature deals damage equal to \ +its power to you.\n10—19 | This creature deals damage equal to its power to defending player.\n20 \ +| This creature deals damage equal to its power to each opponent."; + +const MASTIFF_POWER: i32 = 4; + +/// Build a two-player scenario at `seed`, declare Iron Mastiff (a 4/4) attacking +/// P1, resolve the on-stack attacks trigger (roll + outcome-table damage) by +/// passing priority ourselves so we capture the emitted `DieRolled` event, and +/// STOP before combat damage so life deltas reflect the trigger alone (the 4/4's +/// own 4 combat damage to P1 would otherwise confound the "to defending player" +/// row). Returns the runner and the actual d20 result that drove the table. +fn attack_and_resolve(seed: u64) -> (GameRunner, u8) { + let mut scenario = GameScenario::new_n_player(2, seed); + scenario.at_phase(Phase::PreCombatMain); + let mastiff = scenario + .add_creature_from_oracle(P0, "Iron Mastiff", 4, 4, IRON_MASTIFF_ORACLE) + .id(); + let mut runner = scenario.build(); + + // Advance to the declare-attackers step and declare the mastiff attacking P1. + runner.pass_both_players(); + runner + .act(GameAction::DeclareAttackers { + attacks: vec![(mastiff, AttackTarget::Player(P1))], + bands: vec![], + }) + .expect("DeclareAttackers should succeed"); + + // The attacks trigger is now on the stack. Resolve it (and only it) by + // passing priority, capturing events, until the stack is empty again — which + // happens during the declare-attackers priority window, before combat damage. + let mut all_events: Vec = Vec::new(); + for _ in 0..40 { + if !runner.state().stack.is_empty() + || matches!(runner.state().waiting_for, WaitingFor::Priority { .. }) + { + match runner.act(GameAction::PassPriority) { + Ok(result) => all_events.extend(result.events), + Err(_) => break, + } + // Stop once the trigger has resolved and the stack is drained. + if runner.state().stack.is_empty() + && all_events + .iter() + .any(|e| matches!(e, GameEvent::DieRolled { sides: 20, .. })) + { + break; + } + } else { + break; + } + } + + let rolled = all_events + .iter() + .find_map(|e| match e { + GameEvent::DieRolled { + result, sides: 20, .. + } => result.map(u8::from), + _ => None, + }) + .expect("Iron Mastiff must roll a d20 when it attacks"); + assert!( + (1..=20).contains(&rolled), + "d20 result out of range: {rolled}" + ); + (runner, rolled) +} + +/// Scan seeds until the mastiff's single d20 roll lands in `[lo, hi]`, so the +/// outcome row under test deterministically fires. +fn resolve_with_roll_in(lo: u8, hi: u8) -> (GameRunner, u8) { + for seed in 0..2000u64 { + let (runner, rolled) = attack_and_resolve(seed); + if (lo..=hi).contains(&rolled) { + return (runner, rolled); + } + } + panic!("no seed in 0..2000 produced a d20 roll in {lo}..={hi}"); +} + +#[test] +fn iron_mastiff_low_roll_deals_power_to_controller() { + // 1—9: "deals damage equal to its power to you" — the controller (P0). + let (runner, rolled) = resolve_with_roll_in(1, 9); + assert!( + (1..=9).contains(&rolled), + "expected a low roll, got {rolled}" + ); + + // CR 119.3: P0 (you) took 4; P1 (defending player) untouched. + assert_eq!( + runner.state().players[P0.0 as usize].life, + 20 - MASTIFF_POWER, + "low roll must deal the mastiff's power to its controller (roll = {rolled})" + ); + assert_eq!( + runner.state().players[P1.0 as usize].life, + 20, + "low roll must NOT touch the defending player (roll = {rolled})" + ); +} + +#[test] +fn iron_mastiff_mid_roll_deals_power_to_defending_player() { + // 10—19: "deals damage equal to its power to defending player" (P1). + let (runner, rolled) = resolve_with_roll_in(10, 19); + assert!( + (10..=19).contains(&rolled), + "expected a mid roll, got {rolled}" + ); + + assert_eq!( + runner.state().players[P1.0 as usize].life, + 20 - MASTIFF_POWER, + "mid roll must deal the mastiff's power to the defending player (roll = {rolled})" + ); + assert_eq!( + runner.state().players[P0.0 as usize].life, + 20, + "mid roll must NOT touch the controller (roll = {rolled})" + ); +} + +#[test] +fn iron_mastiff_max_roll_deals_power_to_each_opponent() { + // 20: "deals damage equal to its power to each opponent" — P1 is P0's only + // opponent in a two-player game. + let (runner, rolled) = resolve_with_roll_in(20, 20); + assert_eq!(rolled, 20, "expected a natural 20"); + + assert_eq!( + runner.state().players[P1.0 as usize].life, + 20 - MASTIFF_POWER, + "natural 20 must deal the mastiff's power to each opponent (roll = {rolled})" + ); + // The controller is not an opponent, so it must not be hit. + assert_eq!( + runner.state().players[P0.0 as usize].life, + 20, + "natural 20 must NOT touch the controller (each opponent excludes you)" + ); +} + +/// Guard: the attacks trigger genuinely produces a roll (proving the fixture +/// reaches the RollDie arm and past any Unimplemented short-circuit). Without a +/// working parse the `attack_and_resolve` helper would panic on the missing +/// `DieRolled` event, so a passing outcome test above already proves reach — this +/// test makes the reach-guard explicit and independent of the outcome bucket. +#[test] +fn iron_mastiff_attack_actually_rolls_a_d20() { + let (_runner, rolled) = attack_and_resolve(7); + assert!( + (1..=20).contains(&rolled), + "attacks trigger must roll a real d20 (got {rolled})" + ); +} + +/// Parser shape: Iron Mastiff's verbatim Oracle text parses to a single +/// `TriggerMode::Attacks` trigger whose execute effect is +/// `RollDie { sides: 20, keep: Highest, count: , +/// results: [1—9, 10—19, 20] }`, each branch a real `DealDamage` (never +/// `Unimplemented`). This is the AST guard behind the runtime tests above. +#[test] +fn iron_mastiff_parses_to_keep_highest_roll_table() { + let parsed = parse_oracle_text( + IRON_MASTIFF_ORACLE, + "Iron Mastiff", + &[], + &["Artifact".to_string(), "Creature".to_string()], + &["Dog".to_string()], + ); + + // Exactly one attacks trigger, no detached outcome-row abilities and no + // Unimplemented fallthrough. + assert_eq!( + parsed.triggers.len(), + 1, + "expected one attacks trigger, got {:#?}", + parsed.triggers + ); + assert!( + parsed.abilities.is_empty(), + "outcome rows must attach to the roll, not become detached abilities: {:#?}", + parsed.abilities + ); + + let trigger = &parsed.triggers[0]; + assert!( + matches!(trigger.mode, TriggerMode::Attacks), + "expected TriggerMode::Attacks, got {:?}", + trigger.mode + ); + + let execute = trigger + .execute + .as_ref() + .expect("attacks trigger must have an execute effect"); + + match &*execute.effect { + Effect::RollDie { + count, + sides, + results, + modifier, + keep, + } => { + assert_eq!(*sides, 20, "Iron Mastiff rolls a d20"); + assert_eq!( + *keep, + DieRollAggregate::Highest, + "'ignore all but the highest roll' must set keep-highest" + ); + assert!(modifier.is_none(), "no add/subtract modifier on this roll"); + // "for each player being attacked" → players this creature is + // attacking this combat (CR 508.6). + assert!( + matches!( + count, + QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { + filter: PlayerFilter::OpponentAttacked { .. } + } + } + ), + "count must be the attacked-players count, got {count:?}" + ); + + // Three outcome rows with the printed ranges, each a DealDamage. + let ranges: Vec<(u8, u8)> = results.iter().map(|b| (b.min, b.max)).collect(); + assert_eq!( + ranges, + vec![(1, 9), (10, 19), (20, 20)], + "outcome-table ranges must be 1—9, 10—19, 20" + ); + // Each row deals damage; "to you"/"to defending player" lower to + // `DealDamage`, "to each opponent" to `DamageEachPlayer` (CR 120). + // Neither may be `Unimplemented`. + for branch in results { + assert!( + matches!( + &*branch.effect.effect, + Effect::DealDamage { .. } | Effect::DamageEachPlayer { .. } + ), + "branch {}—{} must deal damage (not Unimplemented), got {:?}", + branch.min, + branch.max, + branch.effect.effect + ); + } + } + other => panic!("expected RollDie execute effect, got {other:?}"), + } +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 2c95c800e3..919a9af587 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -198,6 +198,7 @@ mod integration_adventure; mod integration_bending; mod integration_landfall; mod invoke_calamity_free_cast; +mod iron_mastiff_roll_d20_attack_5928; mod issue_1005_suffer_the_past; mod issue_1007_fractal_harness_attach; mod issue_1008_korvold_sacrifice_triggers;