diff --git a/crates/engine/src/parser/oracle.rs b/crates/engine/src/parser/oracle.rs index fc56b42ae1..c3d5ab9f00 100644 --- a/crates/engine/src/parser/oracle.rs +++ b/crates/engine/src/parser/oracle.rs @@ -4598,6 +4598,7 @@ pub(crate) fn parse_oracle_ir( // Must run before keyword extraction so "Spree" header + follow-on `+` lines // are consumed as a modal block, not swallowed as a keyword-only line. if let Some((block, next_i)) = parse_oracle_block(&lines, i) { + let mut next_i = next_i; match lower_oracle_block_ir(block, card_name, ctx.host_self_reference.clone(), &mut ctx) { OracleBlockIr::Activated(ability) => { @@ -4613,7 +4614,12 @@ pub(crate) fn parse_oracle_ir( } emitter.modal_at(item_line, choice); } - OracleBlockIr::Triggered(triggers) => { + OracleBlockIr::Triggered(mut triggers) => { + // CR 706.3b: a triggered modal consumes its bullet modes + // before this boundary, so table rows follow `next_i`, not + // the trigger header. Retain them on the trigger IR until + // lowering can attach them to the chain that owns the roll. + next_i = attach_trigger_die_result_branches(&mut triggers, &lines, next_i); for trigger in triggers { emitter.trigger_ir_at(item_line, TriggerNodeIr::Parsed(Box::new(trigger))); } diff --git a/crates/engine/src/parser/oracle_ir/ast.rs b/crates/engine/src/parser/oracle_ir/ast.rs index 0153e39645..de77f5b30c 100644 --- a/crates/engine/src/parser/oracle_ir/ast.rs +++ b/crates/engine/src/parser/oracle_ir/ast.rs @@ -1992,6 +1992,28 @@ fn normalize_play_from_exile_duration(duration: Duration) -> Duration { // --- Modal types (moved from oracle_modal.rs) --- +/// CR 603.12: The printed instruction a triggered modal's reflexive +/// connector rides on — `", . When you do, choose …"`. +/// +/// The `"When you do"` connector creates the reflexive triggered ability. The +/// `"you may "` marker only makes its parent instruction optional, so it cannot +/// decide whether a reflexive exists. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub(crate) enum ReflexiveModalParent { + /// `"…, you may . When you do, choose …"` — a declinable + /// resolution-time instruction (Caesar, Legion's Emperor). Carries the + /// printed instruction text with the `"you may "` marker and connector stripped; + /// `trigger_line` is reduced to the bare trigger condition alongside it. + MayPay(String), + /// `"…, . When you do, choose …"` — a mandatory instruction + /// (Cemetery Desecrator). No text is carried: the + /// instruction stays in `trigger_line`, where the ordinary trigger parser + /// lowers it as it already does for every non-modal reflexive (Bone + /// Rattler, Diregraf Horde). Lowering then attaches the modal as that + /// chain's `WhenYouDo` sub instead of replacing it. + Mandatory, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub(crate) enum OracleBlockAst { ActivatedModal { @@ -2008,15 +2030,14 @@ pub(crate) enum OracleBlockAst { trigger_line: String, header: ModalHeaderAst, modes: Vec, - /// CR 603.12 + CR 700.2b: When the trigger gates its modal choice behind - /// an optional reflexive cost ("Whenever you attack, you may sacrifice - /// another creature. When you do, choose ..."), this holds the cost - /// effect text (e.g. "Sacrifice another creature"). The lowering builds - /// an `Effect::Sacrifice { optional }` whose `WhenYouDo` sub_ability - /// carries the modal, so the modes fire only after the cost is paid. - /// `None` for a plain triggered modal (Pip-Boy), where the modal attaches - /// directly as the trigger's execute. - optional_cost: Option, + /// CR 603.12 + CR 700.2b: How the modal choice is introduced. + /// + /// `None` is a plain triggered modal (Pip-Boy 3000), where the modal + /// attaches directly as the trigger's execute. Anything else means a + /// reflexive connector stands between the trigger and the mode list, + /// and the modal must ride on the printed instruction before it — + /// see `ReflexiveModalParent`. + reflexive_parent: Option, }, /// CR 614.12c + CR 607.2d: "As [this permanent] enters, choose or /// . \n • — . \n • — ." The diff --git a/crates/engine/src/parser/oracle_ir/trigger.rs b/crates/engine/src/parser/oracle_ir/trigger.rs index cd1d8ce3e6..27e6032362 100644 --- a/crates/engine/src/parser/oracle_ir/trigger.rs +++ b/crates/engine/src/parser/oracle_ir/trigger.rs @@ -95,30 +95,55 @@ pub(crate) struct TriggerIr { impl TriggerIr { /// Whether the body ends in the typed die-roll node that owns a result table. pub(crate) fn has_terminal_roll_die(&self) -> bool { - let chain = match &self.body { - Some(TriggerBody::EffectChain(chain)) => chain, - Some(TriggerBody::ReflexivePayment(reflexive)) => &reflexive.effect_chain, + match &self.body { + Some(TriggerBody::EffectChain(chain)) => effect_chain_has_terminal_roll_die(chain), + // CR 706.3b: the table belongs to the printed die-roll instruction, + // not the reflexive `WhenYouDo` body it creates. A modal reflexive + // body contains only the mode marker, so looking there drops rows + // for a parent such as "roll a d20. When you do, choose one". + Some(TriggerBody::Reflexive(reflexive)) => { + effect_chain_has_terminal_roll_die(&reflexive.effect_chain) + || match &reflexive.parent { + ReflexiveParent::MayPay { + payment_chain: Some(chain), + .. + } => effect_chain_has_terminal_roll_die(chain), + ReflexiveParent::MayPay { + payment_chain: None, + .. + } => false, + ReflexiveParent::Mandatory { instruction } => { + effect_chain_has_terminal_roll_die(instruction) + } + } + } Some(TriggerBody::Modal(_)) | Some(TriggerBody::Vote(_)) | Some(TriggerBody::Pile(_)) - | None => return false, - }; - let Some(clause) = chain.clauses.last() else { - return false; - }; - matches!(clause.parsed.effect, Effect::RollDie { .. }) + | None => false, + } } } +/// Whether this exact chain ends at the typed die roll that owns its following +/// result table. Parent and reflexive chains are distinct printed instructions, +/// so callers use this to preserve the table on whichever one owns the roll. +pub(crate) fn effect_chain_has_terminal_roll_die(chain: &EffectChainIr) -> bool { + chain + .clauses + .last() + .is_some_and(|clause| matches!(clause.parsed.effect, Effect::RollDie { .. })) +} + /// The body of a trigger. Whole-body recognizers retain their typed payloads /// here so trigger lowering owns all root-level transforms. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub(crate) enum TriggerBody { /// Normal effect chain — lowering calls `lower_effect_chain_ir`. EffectChain(EffectChainIr), - /// CR 118.12 + CR 603.12: A resolution-time optional cost and the - /// reflexive effect that follows when the player pays it. - ReflexivePayment(Box), + /// CR 603.12: A printed parent instruction and the reflexive + /// effect that follows when its event occurred. + Reflexive(Box), /// CR 700.2: An inline modal's marker clause and its already-lowered mode /// bodies. The marker still flows through ordinary trigger-chain lowering; /// this payload carries the modal metadata no clause can represent. @@ -130,14 +155,42 @@ pub(crate) enum TriggerBody { Pile(Box), } +/// CR 603.12: A reflexive "when you do" body together with the printed parent +/// instruction it rides on. +/// +/// `parent` is the axis that used to be assumed rather than represented: this +/// node only existed for the `"you may . When you do"` surface, so +/// a mandatory parent had nowhere to live. Keeping the parent as a parameterized +/// field rather than a sibling node means the reflexive lowering has exactly one +/// shape. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] -pub(crate) struct ReflexivePaymentIr { - pub(crate) cost: AbilityCost, +pub(crate) struct ReflexiveParentIr { + /// How the parent instruction is printed and offered. + pub(crate) parent: ReflexiveParent, + /// The reflexive body — what `"When you do, …"` introduces. pub(crate) effect_chain: EffectChainIr, - pub(crate) payment_chain: Option, + /// CR 700.2b: modal metadata when the reflexive body is a mode choice. pub(crate) modal: Option, } +/// CR 603.12: the two printed forms a reflexive parent can take. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub(crate) enum ReflexiveParent { + /// `"you may . When you do, …"` — a resolution-time offer the + /// controller may decline (Caesar, Legion's Emperor). `payment_chain` + /// carries the printed instruction as an effect chain when one parsed; otherwise + /// lowering synthesizes an `Effect::PayCost` from `cost`. + MayPay { + cost: AbilityCost, + payment_chain: Option, + }, + /// `". When you do, …"` — the instruction is not an offer, it + /// simply happens (Cemetery Desecrator). The trigger parser + /// already lowered the printed instruction into this chain, so lowering + /// reuses it instead of re-parsing the same words a second time. + Mandatory { instruction: EffectChainIr }, +} + /// CR 700.2: Typed inline-modal trigger body. /// /// The root marker is an ordinary effect chain so trigger lowering applies the diff --git a/crates/engine/src/parser/oracle_modal.rs b/crates/engine/src/parser/oracle_modal.rs index b49eb0513d..b4ed5e1e05 100644 --- a/crates/engine/src/parser/oracle_modal.rs +++ b/crates/engine/src/parser/oracle_modal.rs @@ -17,6 +17,7 @@ use crate::types::replacements::ReplacementEvent; use crate::types::triggers::TriggerMode; use super::oracle::{find_activated_colon, strip_activated_constraints}; +use super::oracle_classifier::has_trigger_prefix; use super::oracle_cost::parse_oracle_cost; #[cfg(test)] use super::oracle_effect::lower_ability_ir; @@ -28,7 +29,9 @@ use super::oracle_ir::effect_chain::{ }; use super::oracle_ir::replacement::ReplacementIr; use super::oracle_ir::static_ir::StaticIr; -use super::oracle_ir::trigger::{ModalIr, ReflexivePaymentIr, TriggerBody, TriggerIr}; +use super::oracle_ir::trigger::{ + ModalIr, ReflexiveParent, ReflexiveParentIr, TriggerBody, TriggerIr, +}; use super::oracle_nom::bridge::nom_on_lower; use super::oracle_nom::condition as nom_condition; use super::oracle_nom::primitives::{self as nom_primitives, scan_preceded}; @@ -41,7 +44,7 @@ use super::oracle_trigger::parse_trigger_lines; use super::oracle_trigger::parse_trigger_lines_at_index_ir; use super::oracle_util::{parse_mana_symbols, strip_reminder_text, TextPair}; use crate::parser::oracle_ir::ast::{ - parsed_clause, ModalHeaderAst, ModalOptionality, ModeAst, OracleBlockAst, + parsed_clause, ModalHeaderAst, ModalOptionality, ModeAst, OracleBlockAst, ReflexiveModalParent, }; #[cfg(test)] use crate::types::ability::AbilityCondition; @@ -143,16 +146,13 @@ pub(crate) fn parse_oracle_block(lines: &[&str], start: usize) -> Option<(Oracle // out the cost so the lowering builds an `Effect::Sacrifice` whose // `WhenYouDo` sub carries the modal, instead of firing the modes // unconditionally on the trigger. - let (trigger_line, optional_cost) = match split_reflexive_optional_cost(&trigger_line) { - Some((trigger, cost)) => (trigger, Some(cost)), - None => (trigger_line, None), - }; + let (trigger_line, reflexive_parent) = classify_reflexive_modal_parent(trigger_line); return Some(( OracleBlockAst::TriggeredModal { trigger_line, header, modes, - optional_cost, + reflexive_parent, }, next, )); @@ -975,17 +975,75 @@ fn split_triggered_modal_header(line: &str) -> Option<(String, String)> { None } -/// CR 603.12 + CR 700.2b: Recognize a reflexive optional-cost trigger header of -/// the shape `", you may . When you do"` (Caesar, Legion's +/// CR 603.12: Classify how a triggered modal's mode list is +/// introduced, and reduce `trigger_line` to what the trigger parser should see. +/// +/// This is the single authority for that question. The reflexive CONNECTOR +/// decides whether a reflexive exists at all; the `"you may "` marker only says +/// whether the parent instruction is optional. Keying the whole decision on the +/// marker — as this dispatch did — silently answered +/// "no reflexive here" for every mandatory parent, and the mode list then +/// attached straight to the trigger with the printed instruction discarded. +/// A mandatory parent must still retain the shared trigger-prefix shape; +/// non-triggered effects keep their original line and existing route. +/// +/// * `MayPay` — `trigger_line` is reduced to the bare trigger condition and the +/// instruction text travels separately, because that optional instruction is +/// lowered by the resolution-time path. +/// * `Mandatory` — the terminal connector is removed and the printed +/// instruction remains in `trigger_line`. The trigger parser lowers that +/// instruction as the parent chain; lowering then hangs the modal off it. +/// * `None` — no connector: a plain triggered modal (Pip-Boy 3000). +fn classify_reflexive_modal_parent(trigger_line: String) -> (String, Option) { + if let Some((trigger, cost)) = split_reflexive_optional_cost(&trigger_line) { + return (trigger, Some(ReflexiveModalParent::MayPay(cost))); + } + if let Some(instruction) = strip_terminal_reflexive_connector(&trigger_line) + .filter(|instruction| has_trigger_prefix(&instruction.to_lowercase())) + { + return ( + instruction.to_string(), + Some(ReflexiveModalParent::Mandatory), + ); + } + (trigger_line, None) +} + +/// CR 603.12: remove a bare reflexive connector after `trigger_line`'s final +/// sentence break, leaving the parent instruction for ordinary trigger parsing. +/// +/// `split_triggered_modal_header` has already taken the mode list away, so the +/// connector is the entire tail — there is no reflexive body left here to +/// consume. The classifier separately validates that the remaining prefix is +/// a trigger before accepting this terminal connector as a mandatory parent. +fn strip_terminal_reflexive_connector(trigger_line: &str) -> Option<&str> { + let lower = trigger_line.to_lowercase(); + let mut tail = lower.as_str(); + let mut connector_start = None; + while let Ok((after, _)) = + terminated(take_until::<_, _, OracleError<'_>>(". "), tag(". ")).parse(tail) + { + connector_start = Some(lower.len() - after.len() - 2); + tail = after; + } + let tail = tail.trim().trim_end_matches(',').trim(); + if !nom_condition::match_when_you_do(tail).is_ok_and(|(rest, ())| rest.is_empty()) { + return None; + } + trigger_line.get(..connector_start?).map(str::trim_end) +} + +/// CR 603.12 + CR 700.2b: Recognize a reflexive optional-instruction trigger +/// header of the shape `", you may . When you do"` (Caesar, Legion's /// Emperor) and split it into the bare trigger condition (`"Whenever you -/// attack"`) and the cost effect text (`"Sacrifice another creature"`, with the +/// attack"`) and the instruction text (`"Sacrifice another creature"`, with the /// `"you may "` optional marker and the trailing `". When you do"` reflexive /// connector stripped). Returns `None` for a plain triggered modal (Pip-Boy /// 3000's `"Whenever equipped creature attacks ..."`), which has neither a -/// `"you may "` optional cost nor a `"when you do"` reflexive connector — that +/// `"you may "` optional instruction nor a `"when you do"` reflexive connector — that /// modal attaches directly to the trigger's execute. /// -/// The cost text is returned with an uppercased leading letter so it parses as +/// The instruction text is returned with an uppercased leading letter so it parses as /// an imperative effect clause (`parse_effect_chain` expects sentence case). fn split_reflexive_optional_cost(trigger_line: &str) -> Option<(String, String)> { // Combinator (run on lowercase, slice original by equal ASCII byte offset): @@ -1113,7 +1171,7 @@ pub(crate) fn lower_oracle_block_ir( trigger_line, header, modes, - optional_cost, + reflexive_parent, } => { let mut trigger_ctx = ctx.clone(); trigger_ctx.host_self_reference = host_self_reference; @@ -1146,10 +1204,28 @@ pub(crate) fn lower_oracle_block_ir( modes: parse_modal_mode_irs(&modes, AbilityKind::Spell, &mut mode_ctx), }; ctx.diagnostics.extend(mode_ctx.diagnostics); - trigger.body = Some(match &optional_cost { - Some(cost_text) => { - TriggerBody::ReflexivePayment(Box::new(ReflexivePaymentIr { - cost: parse_oracle_cost(cost_text), + // CR 603.12: the printed instruction the mode list rides on is + // whatever `classify_reflexive_modal_parent` found. Compute the + // body before assigning it — the mandatory arm reads the chain + // the trigger parser already produced. + let body = match &reflexive_parent { + Some(ReflexiveModalParent::MayPay(cost_text)) => { + TriggerBody::Reflexive(Box::new(ReflexiveParentIr { + parent: ReflexiveParent::MayPay { + cost: parse_oracle_cost(cost_text), + payment_chain: Some( + parse_ability_ir_with_context( + cost_text, + AbilityKind::Spell, + &mut ParseContext { + actor: ctx.actor.clone(), + in_trigger: true, + ..Default::default() + }, + ) + .body, + ), + }, effect_chain: EffectChainIr::single_clause( cost_text, AbilityKind::Spell, @@ -1162,23 +1238,39 @@ pub(crate) fn lower_oracle_block_ir( None, true, ), - payment_chain: Some( - parse_ability_ir_with_context( - cost_text, - AbilityKind::Spell, - &mut ParseContext { - actor: ctx.actor.clone(), - in_trigger: true, - ..Default::default() - }, - ) - .body, - ), modal: Some(payload.clone()), })) } + // CR 603.12 + CR 608.2c: a mandatory parent. The trigger + // parser already lowered the printed instruction — the same + // path that handles every non-modal reflexive — so take + // that chain as the parent and hang the mode list off it as + // the CR 603.12 reflexive body. Replacing it (as this arm + // did) dropped the instruction from the card entirely. + Some(ReflexiveModalParent::Mandatory) => match trigger.body.take() { + Some(TriggerBody::EffectChain(instruction)) => { + TriggerBody::Reflexive(Box::new(ReflexiveParentIr { + parent: ReflexiveParent::Mandatory { instruction }, + effect_chain: payload.marker.clone(), + modal: Some(payload.clone()), + })) + } + // The connector is there but the instruction did not + // lower to a plain chain. These shapes carry their own + // root transforms and nesting them here would misreport + // what the card does, so keep the pre-existing plain + // modal rather than invent a parent. + Some( + TriggerBody::Reflexive(_) + | TriggerBody::Modal(_) + | TriggerBody::Vote(_) + | TriggerBody::Pile(_), + ) + | None => TriggerBody::Modal(Box::new(payload.clone())), + }, None => TriggerBody::Modal(Box::new(payload.clone())), - }); + }; + trigger.body = Some(body); if matches!(header.optionality, ModalOptionality::MayDecline) { trigger.modifiers.optional = true; } @@ -1391,7 +1483,7 @@ pub(crate) fn lower_oracle_block( trigger_line, header, modes, - optional_cost, + reflexive_parent, } => { let mut triggers = parse_trigger_lines(&trigger_line, card_name); // CR 608.2k + CR 301.5a: Derive the trigger subject from the parsed @@ -1433,33 +1525,50 @@ pub(crate) fn lower_oracle_block( modal_ability.optional = true; } - let execute = match optional_cost { + // CR 603.12: `None` means the modal attaches directly; either + // reflexive arm makes it the `WhenYouDo` body of the printed parent. + // `Mandatory` needs the per-trigger execute the trigger parser + // produced, so it is resolved inside the loop below. + let execute = match &reflexive_parent { // CR 603.12 + CR 700.2b: The modal is gated behind a reflexive - // optional cost. Build `Effect::Sacrifice { optional }` whose + // optional instruction. Build `Effect::Sacrifice { optional }` whose // `WhenYouDo` sub_ability carries the modal, so the modes are - // chosen and resolved only after the controller pays the cost + // chosen and resolved only after the controller performs that instruction // (Caesar, Legion's Emperor). The decline path is handled by // `should_resolve_subability_on_optional_decline` (WhenYouDo → // false), so declining the sacrifice resolves no modes. - Some(cost_text) => { + Some(ReflexiveModalParent::MayPay(cost_text)) => { modal_ability.condition = Some(AbilityCondition::WhenYouDo); let mut cost_ability = crate::parser::oracle_effect::parse_effect_chain( - &cost_text, + cost_text, AbilityKind::Spell, ); - // CR 118.12 + CR 701.21: "you may sacrifice" makes the - // sacrifice cost optional during resolution; the controller - // is prompted before paying it. + // CR 603.12 + CR 701.21: "you may sacrifice" makes the + // sacrifice instruction optional during resolution; the + // controller chooses whether to perform it. cost_ability.optional = true; cost_ability.sub_ability = Some(Box::new(modal_ability)); Box::new(cost_ability) } + // CR 603.12 + CR 608.2c: a mandatory parent stays in the trigger + // line, so the parent is the trigger's own execute and the modal + // becomes its reflexive body. + Some(ReflexiveModalParent::Mandatory) => { + modal_ability.condition = Some(AbilityCondition::WhenYouDo); + Box::new(modal_ability) + } // Plain triggered modal (Pip-Boy): the modal attaches directly. None => Box::new(modal_ability), }; for trigger in &mut triggers { - trigger.execute = Some(execute.clone()); + match (&reflexive_parent, trigger.execute.take()) { + (Some(ReflexiveModalParent::Mandatory), Some(mut instruction)) => { + instruction.sub_ability = Some(execute.clone()); + trigger.execute = Some(instruction); + } + _ => trigger.execute = Some(execute.clone()), + } if matches!(header.optionality, ModalOptionality::MayDecline) { trigger.optional = true; } @@ -4212,6 +4321,180 @@ When The Ruinous Wrecking Crew enters, choose up to X —\n\ ); } + /// CR 603.12: the classifier answers on the reflexive CONNECTOR, so the + /// `"you may "` marker only chooses between the two parent shapes. + /// + /// Table-driven on purpose: the marker and the connector are independent + /// axes, and the shipped defect was exactly one cell of this table — a + /// connector with no marker — being read as "no reflexive at all". + #[test] + fn classify_reflexive_modal_parent_keys_on_the_connector_not_the_marker() { + let rows: &[(&str, Option, &str)] = &[ + // Marker + connector: Caesar. The instruction leaves `trigger_line` + // because a resolution payment is not something the trigger parser + // can lower on its own. + ( + "Whenever you attack, you may sacrifice another creature. When you do", + Some(ReflexiveModalParent::MayPay( + "Sacrifice another creature".to_string(), + )), + "Whenever you attack", + ), + // Connector, NO marker: Cemetery Desecrator. The instruction stays + // in `trigger_line`, but its connector is removed before the + // ordinary trigger parser lowers the mandatory parent chain. + ( + "When this creature enters or dies, exile another card from a graveyard. When you do", + Some(ReflexiveModalParent::Mandatory), + "When this creature enters or dies, exile another card from a graveyard", + ), + // Dialogue Tree is a sorcery, not a triggered parent. Its terminal + // connector must stay intact so this targeted reflexive repair does + // not alter the non-triggered modal route. + ( + "Scry 1. When you do", + None, + "Scry 1. When you do", + ), + // Neither: Pip-Boy 3000 stays a plain triggered modal. + ( + "Whenever equipped creature attacks", + None, + "Whenever equipped creature attacks", + ), + // Marker, NO connector: a plain optional effect, not a reflexive. + ( + "Whenever you attack, you may draw a card", + None, + "Whenever you attack, you may draw a card", + ), + ]; + for (line, expected_parent, expected_trigger) in rows { + let (trigger, parent) = classify_reflexive_modal_parent(line.to_string()); + assert_eq!(&parent, expected_parent, "parent for {line:?}"); + assert_eq!(&trigger.as_str(), expected_trigger, "trigger for {line:?}"); + } + } + + /// CR 603.12 + CR 608.2c: a mandatory instruction printed before the mode + /// list is kept and the modal becomes its reflexive body — the + /// same shape Caesar's optional parent produces, minus the decline. + /// + /// Guards the class, not the card: `Mandatory` carries no card-specific + /// text, so any printed instruction the trigger parser can lower reaches + /// this shape. + #[test] + fn a_mandatory_parent_keeps_its_instruction_under_the_mode_list() { + let parsed = parse_oracle_text( + "When this creature enters, exile another card from a graveyard. When you do, choose one —\n• Draw a card.\n• You gain 2 life.", + "Class Probe", + &[], + &["Creature".to_string()], + &[], + ); + let execute = parsed + .triggers + .first() + .and_then(|t| t.execute.as_ref()) + .expect("the enters trigger must carry an execute"); + assert!( + matches!(*execute.effect, Effect::ChangeZone { .. }), + "the printed instruction must survive as the parent effect, got {:?}", + execute.effect + ); + assert!( + !execute.optional, + "a mandatory instruction is not an offer and must not be marked optional" + ); + let sub = execute + .sub_ability + .as_ref() + .expect("the mode list must hang off the instruction as its reflexive body"); + assert_eq!(sub.condition, Some(AbilityCondition::WhenYouDo)); + assert_eq!(sub.mode_abilities.len(), 2, "both modes must survive"); + } + + /// CR 603.12: the connector may be followed by an intervening condition + /// before the mode list ("When you do, if you control five or more …, + /// choose one —"). The modal header split then leaves the trigger line + /// ending on the bare connector, which is the second surface this class + /// takes — The Cobra King, whose Cobra Coil token was dropped the same way + /// Cemetery Desecrator's exile was. + /// + /// Does NOT assert the "five or more" gate: that condition lands in the + /// modal header and is unrepresented both before and after this change. + #[test] + fn a_mandatory_parent_survives_a_condition_between_connector_and_modes() { + let parsed = parse_oracle_text( + "At the beginning of each player's upkeep, create a 1/1 blue Serpent creature token named Cobra Coil. When you do, if you control five or more Snakes and/or Serpents, choose one —\n• Strike first — Target Snake or Serpent you control fights target creature an opponent controls.\n• Strike hard — Put a +1/+1 counter on each Snake and Serpent you control.", + "The Cobra King", + &[], + &["Legendary".to_string(), "Creature".to_string()], + &["Snake".to_string()], + ); + let execute = parsed + .triggers + .first() + .and_then(|t| t.execute.as_ref()) + .expect("the upkeep trigger must carry an execute"); + assert!( + matches!(*execute.effect, Effect::Token { .. }), + "the printed token instruction must survive as the parent effect, got {:?}", + execute.effect + ); + let sub = execute + .sub_ability + .as_ref() + .expect("the mode list must hang off the instruction as its reflexive body"); + assert_eq!(sub.condition, Some(AbilityCondition::WhenYouDo)); + assert_eq!(sub.mode_abilities.len(), 2, "both modes must survive"); + } + + /// CR 706.3b: result-table rows belong to the mandatory printed die-roll + /// parent, even when its CR 603.12 reflexive body is a modal marker. + #[test] + fn mandatory_roll_die_parent_retains_its_result_table() { + let parsed = parse_oracle_text( + "When this creature enters, roll a d20. When you do, choose one —\n• Draw a card.\n• You gain 2 life.\n1—10 | Draw a card.\n11—20 | You gain 2 life.", + "Roll Parent Probe", + &[], + &["Creature".to_string()], + &[], + ); + let execute = parsed + .triggers + .first() + .and_then(|trigger| trigger.execute.as_ref()) + .expect("the enters trigger must carry its mandatory parent"); + let Effect::RollDie { results, .. } = execute.effect.as_ref() else { + panic!( + "the printed roll must remain the parent effect, got {:?}", + execute.effect + ); + }; + assert_eq!( + results.len(), + 2, + "the parent roll must retain both immediately following result rows" + ); + assert_eq!( + results + .iter() + .map(|branch| (branch.min, branch.max)) + .collect::>(), + vec![(1, 10), (11, 20)], + "the result ranges must remain attached to the printed parent roll" + ); + assert_eq!( + execute + .sub_ability + .as_ref() + .and_then(|sub| sub.condition.clone()), + Some(AbilityCondition::WhenYouDo), + "the mode list remains the parent roll's reflexive body" + ); + } + #[test] fn caesar_lowers_to_reflexive_gated_modal() { // CR 603.12 + CR 700.2b: Caesar's attack trigger must lower to an @@ -4287,7 +4570,7 @@ When The Ruinous Wrecking Crew enters, choose up to X —\n\ item.source.fragment(), Some("Whenever you attack, you may sacrifice another creature. When you do, choose two —") ); - let Some(TriggerBody::ReflexivePayment(reflexive)) = trigger.body.as_ref() else { + let Some(TriggerBody::Reflexive(reflexive)) = trigger.body.as_ref() else { panic!("Caesar must retain its native reflexive-payment trigger body"); }; let modal = reflexive diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 11ccbafb0f..c4ea8b9985 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -18,7 +18,8 @@ use super::oracle_ir::context::{ParseContext, TriggerConditionScope}; use super::oracle_ir::doc::PrintedTriggerIndex; use super::oracle_ir::effect_chain::{DieResultBranchIr, EffectChainIr}; use super::oracle_ir::trigger::{ - FirstTimeLimit, ReflexivePaymentIr, TriggerBody, TriggerIr, TriggerModifiers, TriggerNodeIr, + effect_chain_has_terminal_roll_die, FirstTimeLimit, ReflexiveParent, ReflexiveParentIr, + TriggerBody, TriggerIr, TriggerModifiers, TriggerNodeIr, }; use super::oracle_modal::try_parse_inline_modal_ir; use super::oracle_nom::condition::parse_elided_subject_state_condition; @@ -1506,14 +1507,14 @@ pub(crate) fn parse_trigger_line_with_index_ir( optional = false; let effect_chain = parse_effect_chain_ir(&reflexive_effect_text, AbilityKind::Spell, &mut effect_ctx); - Some(TriggerBody::ReflexivePayment(Box::new( - ReflexivePaymentIr { + Some(TriggerBody::Reflexive(Box::new(ReflexiveParentIr { + parent: ReflexiveParent::MayPay { cost, - effect_chain, payment_chain: None, - modal: None, }, - ))) + effect_chain, + modal: None, + }))) } else if is_unsupported_disjunctive_reflexive_optional_payment(&effect_for_parse) { Some(TriggerBody::EffectChain(EffectChainIr::single_clause( &effect_for_parse, @@ -1764,9 +1765,22 @@ pub(crate) fn lower_trigger_ir(ir: &TriggerIr) -> TriggerDefinition { modifiers, &ir.die_results, ))), - Some(TriggerBody::ReflexivePayment(reflexive)) => { - let mut reflexive_ability = - lower_trigger_effect_chain(&reflexive.effect_chain, modifiers, &ir.die_results); + Some(TriggerBody::Reflexive(reflexive)) => { + let reflexive_owns_die_results = + effect_chain_has_terminal_roll_die(&reflexive.effect_chain); + let (reflexive_die_results, parent_die_results): ( + &[DieResultBranchIr], + &[DieResultBranchIr], + ) = if reflexive_owns_die_results { + (ir.die_results.as_slice(), &[]) + } else { + (&[], ir.die_results.as_slice()) + }; + let mut reflexive_ability = lower_trigger_effect_chain( + &reflexive.effect_chain, + modifiers, + reflexive_die_results, + ); reflexive_ability.condition = Some(AbilityCondition::WhenYouDo); if let Some(modal) = &reflexive.modal { @@ -1780,20 +1794,39 @@ pub(crate) fn lower_trigger_ir(ir: &TriggerIr) -> TriggerDefinition { ); } - let mut pay_ability = match &reflexive.payment_chain { - Some(chain) => lower_trigger_effect_chain(chain, modifiers, &[]), - None => AbilityDefinition::new( - AbilityKind::Spell, - Effect::PayCost { - cost: reflexive.cost.clone(), - scale: None, - payer: TargetFilter::Controller, - }, - ), + // CR 603.12: the parent is either an optional instruction the + // controller may decline or a mandatory instruction. Both build the + // same parent → `WhenYouDo` sub shape. + let mut parent_ability = match &reflexive.parent { + ReflexiveParent::MayPay { + cost, + payment_chain, + } => { + let mut pay_ability = match payment_chain { + Some(chain) => { + lower_trigger_effect_chain(chain, modifiers, parent_die_results) + } + None => AbilityDefinition::new( + AbilityKind::Spell, + Effect::PayCost { + cost: cost.clone(), + scale: None, + payer: TargetFilter::Controller, + }, + ), + }; + pay_ability.optional = true; + pay_ability + } + // CR 603.12 + CR 608.2c: a mandatory instruction is the next + // printed instruction, not an offer — it carries no `optional` + // flag and no decline prompt. + ReflexiveParent::Mandatory { instruction } => { + lower_trigger_effect_chain(instruction, modifiers, parent_die_results) + } }; - pay_ability.optional = true; - pay_ability.sub_ability = Some(Box::new(reflexive_ability)); - Some(Box::new(pay_ability)) + parent_ability.sub_ability = Some(Box::new(reflexive_ability)); + Some(Box::new(parent_ability)) } Some(TriggerBody::Modal(modal)) => Some(Box::new( lower_trigger_effect_chain(&modal.marker, modifiers, &[]).with_modal( diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index a688071dd7..6115b35bb1 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -1242,6 +1242,7 @@ mod love_on_the_battlefield_combat_counters; mod loyalty_ability_activated_trigger; mod loyalty_replacement_order_resume; mod mad_mage_lost_level_scry_runtime; +mod mandatory_reflexive_modal_parent; mod mass_unsuspect_701_60a; mod memory_vessel_std_s25; mod menace_requires_two_blockers; diff --git a/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs b/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs new file mode 100644 index 0000000000..1474757f1d --- /dev/null +++ b/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs @@ -0,0 +1,207 @@ +//! CR 603.12: a mandatory instruction printed before a reflexive +//! "When you do" whose body is a mode list. +//! +//! Cemetery Desecrator — "When this creature enters or dies, exile another card +//! from a graveyard. When you do, choose one — • Remove X counters from target +//! permanent, where X is the mana value of the exiled card. • Target creature an +//! opponent controls gets -X/-X until end of turn, where X is the mana value of +//! the exiled card." +//! +//! Reported from a real game with every graveyard empty: the enters trigger +//! asked for a mode although there was no card anywhere to exile. Reading the +//! parse showed the cause is upstream of that symptom — the exile instruction +//! was not merely ungated, it was absent. The triggered-modal dispatch keyed the +//! whole reflexive decision on the printed `"you may "` marker, so a mandatory +//! parent classified as "no reflexive here" and the mode list replaced the +//! instruction outright. The card never exiled anything, in any game state. +//! +//! Oracle text verified against `client/public/card-data.json`. +//! +//! CR references (verified against docs/MagicCompRules.txt): +//! - CR 603.12: a reflexive triggered ability triggers "based on whether the +//! trigger event or events occurred earlier during the resolution of the +//! spell or ability that created them". +//! - CR 700.2b: a modal triggered ability chooses its mode(s) as it is put on +//! the stack. +//! +//! What this file does NOT prove: it does not measure the CR 603.12 gate. With +//! every graveyard empty the instruction now runs and exiles nothing, but the +//! reflexive is still created and still asks for a mode. Suppressing it needs +//! the engine to record that a mandatory instruction did nothing — issue #7511's +//! remaining half, deliberately out of scope here. + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::actions::GameAction; +use engine::types::game_state::WaitingFor; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +/// Copied verbatim from `client/public/card-data.json`, newlines included — +/// the mode list is line-separated there, and joining it onto one line is not +/// the printed text the parser is given at runtime. +const DESECRATOR: &str = "When this creature enters or dies, exile another card from a graveyard. When you do, choose one —\n• Remove X counters from target permanent, where X is the mana value of the exiled card.\n• Target creature an opponent controls gets -X/-X until end of turn, where X is the mana value of the exiled card."; + +struct Resolution { + prompts: Vec, + exiled: usize, + graveyard: usize, +} + +/// Cast Cemetery Desecrator and record what its enters trigger did. +/// +/// `graveyard_fodder` is the whole variable: one card sitting in the opponent's +/// graveyard is the only thing "exile another card from a graveyard" can reach. +fn resolve_enters(graveyard_fodder: bool) -> Resolution { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + // The opponent's creature is the legal target for the second mode. Without + // it the mode could be skipped for want of a target, which would hide the + // behavior rather than measure it. + scenario.add_creature_from_oracle(P1, "Warded Bear", 2, 2, "Ward {2}"); + if graveyard_fodder { + scenario.with_graveyard(P1, &["Fodder Card"]); + } + let desecrator = scenario + .add_spell_to_hand_from_oracle(P0, "Cemetery Desecrator", false, DESECRATOR) + .as_creature() + .id(); + for _ in 0..10 { + scenario.add_card_to_library_top(P0, "Plains"); + } + + let mut runner = scenario.build(); + runner.cast(desecrator).commit(); + + let mut prompts = Vec::new(); + let mut settled = false; + for _ in 0..40 { + let (label, action) = match runner.state().waiting_for.clone() { + // Priority with an empty stack is the resting state: the spell, its + // enters trigger and the reflexive it created have all finished. + WaitingFor::Priority { .. } if runner.state().stack.is_empty() => { + settled = true; + break; + } + WaitingFor::Priority { .. } => (None, GameAction::PassPriority), + WaitingFor::AbilityModeChoice { .. } => ( + Some("AbilityModeChoice".to_string()), + GameAction::SelectModes { indices: vec![1] }, + ), + // Answer every slot with its first legal target so the chosen mode + // actually resolves. Recording the prompt and stopping here would + // leave the reflexive half-resolved and prove only that a question + // was asked. + WaitingFor::TriggerTargetSelection { target_slots, .. } + | WaitingFor::TargetSelection { target_slots, .. } => { + let targets: Vec<_> = target_slots + .iter() + .filter_map(|slot| slot.legal_targets.first().cloned()) + .collect(); + ( + Some("TargetSelection".to_string()), + GameAction::SelectTargets { targets }, + ) + } + // CR 702.21a: the chosen mode targets the opponent's warded + // creature. Declining the ward cost counters that mode and lets the + // resolution finish, which is what makes the settled state + // reachable without turning this file into a ward test. + WaitingFor::UnlessPayment { .. } => ( + Some("UnlessPayment".to_string()), + GameAction::PayUnlessCost { pay: false }, + ), + other => { + prompts.push(format!("PROMPT: {}", other.variant_name())); + break; + } + }; + if let Some(label) = label { + prompts.push(label); + } + if runner.act(action).is_err() { + break; + } + } + assert!( + settled, + "the resolution must reach an empty stack — prompts seen: {prompts:?}" + ); + + // Count only the fodder card: the Desecrator itself is on the battlefield + // and the library cards never moved, so a zone census over the named card + // cannot be satisfied by anything else the resolution touched. + let count_in = |zone: Zone| { + runner + .state() + .objects + .values() + .filter(|o| o.name == "Fodder Card" && o.zone == zone) + .count() + }; + Resolution { + prompts, + exiled: count_in(Zone::Exile), + graveyard: count_in(Zone::Graveyard), + } +} + +/// CR 603.12: the printed instruction before the reflexive connector is a real +/// instruction and must be performed. +/// +/// Counter-probe: with the `Mandatory` arm removed from +/// `classify_reflexive_modal_parent` this line reads `left: (0, 1)` against +/// `right: (1, 0)` — the card exiles nothing and the fodder stays in the +/// graveyard, which is exactly the shipped behavior this fixes. +#[test] +fn the_mandatory_instruction_before_a_mode_list_is_performed() { + let resolved = resolve_enters(true); + assert_eq!( + (resolved.exiled, resolved.graveyard), + (1, 0), + "\"exile another card from a graveyard\" must move the one reachable card \ + out of the graveyard and into exile — prompts seen: {:?}", + resolved.prompts + ); +} + +/// The mode list must still be offered once the instruction has run — the fix +/// re-parents the modal, it does not remove it. +/// +/// This is the positive counter-direction: a fix that suppressed the modal +/// altogether would satisfy the test above and break the card a second way. +#[test] +fn the_mode_list_still_resolves_after_the_instruction() { + let resolved = resolve_enters(true); + assert!( + resolved.prompts.iter().any(|p| p == "AbilityModeChoice"), + "the reflexive mode choice must still be offered — {:?}", + resolved.prompts + ); +} + +/// With nothing to exile, the instruction runs and moves no card. +/// +/// This row does NOT assert that the resolution asks nothing. It still asks for +/// a mode: CR 603.12 says the reflexive should never have been created, but the +/// engine has no record that a mandatory instruction did nothing. That gap is +/// issue #7511's remaining half and is not addressed here. +#[test] +fn an_impossible_exile_moves_no_card() { + let resolved = resolve_enters(false); + // Reach-guard: no object named "Fodder Card" exists in this game, so the + // census below would read (0, 0) even if the card failed to parse or the + // trigger never fired. The mode choice proves the enters trigger resolved + // its instruction and created the reflexive. + assert!( + resolved.prompts.iter().any(|p| p == "AbilityModeChoice"), + "the enters trigger must have run its instruction and created the \ + reflexive mode choice — {:?}", + resolved.prompts + ); + assert_eq!( + (resolved.exiled, resolved.graveyard), + (0, 0), + "no graveyard held a card, so nothing may be exiled — prompts seen: {:?}", + resolved.prompts + ); +}