diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index da036f68d6..fa4b18b3df 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -6378,6 +6378,30 @@ fn affected_objects_from_events( } } +/// CR 603.12 (#7511): A reflexive "when you do" triggers "based on whether the +/// trigger event or events occurred earlier during the resolution" of its +/// parent. The `WhenYouDo` arm of `evaluate_condition` covers the OPTIONAL +/// parent (declined / infeasible — #7414) and the failed-payment class; it +/// cannot see the resolution's event slice, so the MANDATORY-parent question +/// ("the instruction ran and did nothing") is answered here, at the sub-walk +/// call site that has the parent's own events in hand. Mirrors the CR 608.2c +/// mandatory-rider seed's exclusions: an outcome-owning parent (coin flip, +/// clash, dig, behold — `effect_manages_own_outcome_flag`) keeps its own +/// record, an effect kind without an event witness stays "mandatory means +/// yes" (`mandatory_parent_effect_performed`'s default arm), and any recorded +/// performance (`optional_effect_performed`) always wins. +fn when_you_do_mandatory_parent_did_nothing( + condition: &AbilityCondition, + parent: &ResolvedAbility, + parent_events: &[GameEvent], +) -> bool { + matches!(condition, AbilityCondition::WhenYouDo) + && !parent.optional + && !parent.context.optional_effect_performed + && !effect_manages_own_outcome_flag(&parent.effect) + && !mandatory_parent_effect_performed(&parent.effect, parent_events) +} + fn mandatory_parent_effect_performed(effect: &Effect, events: &[GameEvent]) -> bool { match effect { Effect::Destroy { .. } | Effect::DestroyAll { .. } => events.iter().any(|event| { @@ -12599,7 +12623,16 @@ fn resolve_chain_body( ability }; - let condition_met = evaluate_condition(condition, state, condition_ability); + // CR 603.12 (#7511): a MANDATORY parent whose witnessed action did + // nothing — "when you do" never happened. Suppression routes + // through the ordinary false path below, so an else branch and the + // surviving sequential siblings keep their printed semantics. + let condition_met = evaluate_condition(condition, state, condition_ability) + && !when_you_do_mandatory_parent_did_nothing( + condition, + ability, + &events[events_before..], + ); if !condition_met { // CR 608.2c: Execute else branch if present ("Otherwise, [effect]") if let Some(ref else_branch) = sub.else_ability { @@ -20551,6 +20584,126 @@ mod tests { ); } + /// CR 603.12 (#7511): the MANDATORY-parent stage of the "when you do" + /// gate — answered at the sub-walk call site from the parent's own event + /// slice, because the arm above has no events and (as its third row pins) + /// must keep saying "mandatory means yes". Rows vary one axis at a time + /// around the same `RemoveCounter` parent (Vhal, Scholar of Mortality's + /// shape: "remove all study counters from it. When you do, …" with zero + /// counters). + #[test] + fn a_mandatory_parent_that_did_nothing_suppresses_its_reflexive() { + let remove_counter = || Effect::RemoveCounter { + counter_type: Some(CounterType::Generic("study".to_string())), + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::SelfRef, + }; + let parent = |optional: bool, performed: bool| { + let mut ability = + ResolvedAbility::new(remove_counter(), vec![], ObjectId(100), PlayerId(0)); + ability.optional = optional; + ability.context.optional_effect_performed = performed; + ability + }; + let when_you_do = AbilityCondition::WhenYouDo; + let no_events: Vec = vec![]; + let witnessed = vec![GameEvent::CounterRemoved { + object_id: ObjectId(100), + counter_type: CounterType::Generic("study".to_string()), + count: 2, + }]; + + // The suppression case: mandatory, no record, no witness event. + assert!( + when_you_do_mandatory_parent_did_nothing( + &when_you_do, + &parent(false, false), + &no_events + ), + "a mandatory RemoveCounter that removed nothing did not happen (CR 603.12)" + ); + // The witness event clears it — the working card keeps its reflexive. + assert!( + !when_you_do_mandatory_parent_did_nothing( + &when_you_do, + &parent(false, false), + &witnessed + ), + "a CounterRemoved event is the parent's occurrence — no suppression" + ); + // An optional parent is the arm's business, never this stage's. + assert!( + !when_you_do_mandatory_parent_did_nothing( + &when_you_do, + &parent(true, false), + &no_events + ), + "the optional axis is owned by the WhenYouDo arm (#7414), not this stage" + ); + // A recorded performance always wins. + assert!( + !when_you_do_mandatory_parent_did_nothing( + &when_you_do, + &parent(false, true), + &no_events + ), + "a recorded performance must never be second-guessed" + ); + // An outcome-owning parent (RollDie) keeps its own record. + let mut roll = ResolvedAbility::new( + Effect::RollDie { + count: QuantityExpr::Fixed { value: 1 }, + sides: 6, + results: vec![], + modifier: None, + }, + vec![], + ObjectId(100), + PlayerId(0), + ); + roll.optional = false; + assert!( + !when_you_do_mandatory_parent_did_nothing(&when_you_do, &roll, &no_events), + "an outcome-owning parent (effect_manages_own_outcome_flag) is exempt" + ); + // A kind without an event witness stays "mandatory means yes" + // (`mandatory_parent_effect_performed`'s default arm) — BecomeCopy is + // the arm test's own example of a mandatory reflexive that must stay + // unconditional. + let become_copy = ResolvedAbility::new( + Effect::BecomeCopy { + recipient: TargetFilter::SelfRef, + target: TargetFilter::SelfRef, + duration: None, + mana_value_limit: None, + additional_modifications: vec![], + }, + vec![], + ObjectId(100), + PlayerId(0), + ); + assert!( + !when_you_do_mandatory_parent_did_nothing(&when_you_do, &become_copy, &no_events), + "an effect kind without an event witness must stay unconditional" + ); + // The condition guard: the call site hands EVERY sub-effect condition + // to this stage, so a non-WhenYouDo condition must pass through even + // when all the parent-side conjuncts would otherwise suppress it. + let quantity_check = AbilityCondition::QuantityCheck { + lhs: QuantityExpr::Fixed { value: 0 }, + comparator: Comparator::EQ, + rhs: QuantityExpr::Fixed { value: 0 }, + }; + assert!( + !when_you_do_mandatory_parent_did_nothing( + &quantity_check, + &parent(false, false), + &no_events + ), + "only AbilityCondition::WhenYouDo is this stage's business" + ); + } + #[test] fn chain_depth_exceeds_limit_returns_error() { let mut state = GameState::new_two_player(42); diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index aefc9d6000..fe8fb005cc 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -19352,9 +19352,9 @@ mod stage2_injector_tests { // resume/finalization helpers are above this existing producer; // they do not mint an optional-effect prompt. The census above // still finds exactly the same five production producers. - "game/effects/mod.rs:7346".to_string(), - "game/effects/mod.rs:7423".to_string(), - "game/effects/mod.rs:11250".to_string(), + "game/effects/mod.rs:7370".to_string(), + "game/effects/mod.rs:7447".to_string(), + "game/effects/mod.rs:11274".to_string(), // UNMOVED across the rebase, and that is itself evidence the SET did not // move: a census that had gained or lost a producer would not leave this // entry both byte-identical AND at the same coordinate. diff --git a/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs b/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs index 1474757f1d..3f52faeaf8 100644 --- a/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs +++ b/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs @@ -24,11 +24,16 @@ //! - 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. +//! The third test measures the CR 603.12 gate itself: with every graveyard +//! empty the mandatory instruction runs, moves nothing, and the reflexive is +//! never created — issue #7511's remaining half, answered from the parent's +//! own event slice at the sub-walk site in `resolve_ability_chain` +//! (`when_you_do_mandatory_parent_did_nothing`). +//! +//! The `RemoveCounter` pair at the end walks the same gate over the +//! event-witness branch (`GameEvent::CounterRemoved`) through the production +//! resolver — review round 1 asked for exactly this runtime pin, in both +//! directions. use engine::game::scenario::{GameScenario, P0, P1}; use engine::types::actions::GameAction; @@ -179,23 +184,25 @@ fn the_mode_list_still_resolves_after_the_instruction() { ); } -/// With nothing to exile, the instruction runs and moves no card. +/// With nothing to exile, the instruction runs, moves no card — and the +/// reflexive is never created. /// -/// 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. +/// CR 603.12: a reflexive triggered ability triggers "based on whether the +/// trigger event or events occurred earlier during the resolution". With every +/// graveyard empty the mandatory "exile another card from a graveyard" exiles +/// nothing, so "when you do" never happened: no mode choice may be offered. +/// This closes issue #7511's remaining half (the optional-parent side landed +/// in #7414). #[test] -fn an_impossible_exile_moves_no_card() { +fn an_impossible_exile_creates_no_reflexive() { 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 + .iter() + .any(|p| p == "AbilityModeChoice" || p == "TargetSelection"), + "CR 603.12: the mandatory exile did nothing, so the reflexive mode \ + choice must never be offered — prompts seen: {:?}", resolved.prompts ); assert_eq!( @@ -205,3 +212,124 @@ fn an_impossible_exile_moves_no_card() { resolved.prompts ); } + +/// Oracle text for the `RemoveCounter` pair below: a MANDATORY +/// self-referential counter removal ahead of a reflexive draw — Vhal, Scholar +/// of Mortality's "remove all study counters from it. When you do, …" shape +/// reduced to its building blocks. No mode list and no targets, so the whole +/// chain resolves without a single player choice and the only observable is +/// the outcome itself. +const COUNTER_SCHOLAR: &str = + "When this creature enters, remove a +1/+1 counter from it. When you do, draw a card."; + +/// As above, plus the counter the mandatory removal needs — the one-variable +/// positive twin. +const COUNTER_SCHOLAR_STOCKED: &str = "This creature enters with a +1/+1 counter on it.\nWhen this creature enters, remove a +1/+1 counter from it. When you do, draw a card."; + +struct CounterOutcome { + prompts: Vec, + plains_in_hand: usize, + counters_on_scholar: Option, +} + +/// Cast the scholar and let its enters chain resolve with no interaction. +/// +/// The census reads only what the chain can move: Plains stay in the library +/// unless the reflexive draws one, and the +1/+1 counter count on the scholar +/// is the mandatory instruction's own footprint. +fn resolve_counter_scholar(oracle_text: &str) -> CounterOutcome { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let scholar = scenario + .add_spell_to_hand_from_oracle(P0, "Counter Scholar", false, oracle_text) + .as_creature() + .id(); + for _ in 0..10 { + scenario.add_card_to_library_top(P0, "Plains"); + } + + let mut runner = scenario.build(); + runner.cast(scholar).commit(); + + let mut prompts = Vec::new(); + let mut settled = false; + for _ in 0..40 { + match runner.state().waiting_for.clone() { + WaitingFor::Priority { .. } if runner.state().stack.is_empty() => { + settled = true; + break; + } + WaitingFor::Priority { .. } => { + if runner.act(GameAction::PassPriority).is_err() { + break; + } + } + other => { + prompts.push(format!("PROMPT: {}", other.variant_name())); + break; + } + } + } + assert!( + settled, + "the chain must resolve without interaction — prompts seen: {prompts:?}" + ); + + let state = runner.state(); + CounterOutcome { + prompts, + plains_in_hand: state + .objects + .values() + .filter(|o| o.name == "Plains" && o.zone == Zone::Hand) + .count(), + counters_on_scholar: state + .objects + .values() + .find(|o| o.name == "Counter Scholar" && o.zone == Zone::Battlefield) + .map(|o| { + o.counters + .get(&engine::types::counter::CounterType::Plus1Plus1) + .copied() + .unwrap_or(0) + }), + } +} + +/// The witness side of the runtime pair from review round 1: the counter is +/// there, the mandatory removal happens, and the reflexive must survive the +/// event-witness gate. +/// +/// This half fails if the `GameEvent::CounterRemoved` witness is not threaded +/// into the parent's event slice at the sub-walk site — silencing the working +/// chain is the failure mode the gate must not have. +#[test] +fn a_performed_counter_removal_keeps_its_reflexive() { + let outcome = resolve_counter_scholar(COUNTER_SCHOLAR_STOCKED); + assert_eq!( + (outcome.plains_in_hand, outcome.counters_on_scholar), + (1, Some(0)), + "the counter came off, so the reflexive must draw — prompts seen: {:?}", + outcome.prompts + ); +} + +/// The suppression side: no counter to remove, the mandatory instruction runs +/// and moves nothing — CR 603.12: "when you do" never happened, so the +/// reflexive draw must not resolve. +/// +/// Counter-probe: with the sub-walk gate +/// (`when_you_do_mandatory_parent_did_nothing`) reverted, this line reads +/// `left: (1, Some(0))` against `right: (0, Some(0))` — exactly the shipped +/// behavior this PR fixes, now pinned through the production resolver. +#[test] +fn an_impossible_counter_removal_creates_no_reflexive() { + let outcome = resolve_counter_scholar(COUNTER_SCHOLAR); + assert_eq!( + (outcome.plains_in_hand, outcome.counters_on_scholar), + (0, Some(0)), + "nothing was removed, so nothing may be drawn (CR 603.12) — prompts \ + seen: {:?}", + outcome.prompts + ); +}