From 1b3864ad7f7c788d3e92345f116473f505c326b2 Mon Sep 17 00:00:00 2001 From: cuinhellcat <217210902+cuinhellcat@users.noreply.github.com> Date: Fri, 21 Aug 2026 02:03:39 +0200 Subject: [PATCH 1/4] fix(engine): a mandatory parent that did nothing creates no "when you do" reflexive (#7511) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CR 603.12: a reflexive triggered ability 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 (#7414) and the failed-payment class, but returned true for every MANDATORY parent — Cemetery Desecrator with every graveyard empty still offered its mode choice, and Vhal, Scholar of Mortality reanimated for free off zero removed study counters. The sub-walk call site is the one place that holds the parent's own event slice, so the mandatory question is answered there: `when_you_do_mandatory_parent_did_nothing` suppresses the reflexive when the parent is mandatory, carries no performed-record, does not own its outcome (`effect_manages_own_outcome_flag`: coin flip, clash, dig, behold), and its event witness (`mandatory_parent_effect_performed`) saw nothing. Suppression routes through the ordinary condition-false path, so else branches and surviving sequential siblings keep their printed semantics. Effect kinds without an event witness stay "mandatory means yes" (the witness fn's default arm), which keeps RollDie/BecomeCopy reflexives unconditional. Class (card-data.json, reminder text stripped): 81 cards carry a mandatory instruction before "When you do" (all three Vhals, Minsc & Boo, Tip the Scales, Yannik, Venom, Cemetery Desecrator, ...). The d20 dragons are outcome-owning and exempt by design. Known remainder: a WhenYouDo carrier resumed from `pending_continuation` (gate on the carrier itself, no parent frame) is not covered — its parent paused for an interactive choice, so an action did occur; and the `evaluate_condition` arm itself still answers "mandatory means yes" (pinned by test), because it has no event slice — the gate lives at the call site that does. Co-Authored-By: Claude Fable 5 --- crates/engine/src/game/effects/mod.rs | 139 +++++++++++++++++- crates/engine/src/game/engine.rs | 6 +- .../mandatory_reflexive_modal_parent.rs | 28 ++-- 3 files changed, 156 insertions(+), 17 deletions(-) diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index 67614a7d94..80306d50d2 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -6376,6 +6376,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| { @@ -12597,7 +12621,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 { @@ -20549,6 +20582,110 @@ 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" + ); + } + #[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 fab9faad3f..2a3a205c06 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:7344".to_string(), - "game/effects/mod.rs:7421".to_string(), - "game/effects/mod.rs:11248".to_string(), + "game/effects/mod.rs:7368".to_string(), + "game/effects/mod.rs:7445".to_string(), + "game/effects/mod.rs:11272".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..f4ba37bc9f 100644 --- a/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs +++ b/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs @@ -179,23 +179,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!( From a7e5ba9f0035c866ec91545f0f112bfb1bcaffa8 Mon Sep 17 00:00:00 2001 From: cuinhellcat <217210902+cuinhellcat@users.noreply.github.com> Date: Fri, 21 Aug 2026 02:17:35 +0200 Subject: [PATCH 2/4] docs(test): the module doc now matches the flipped third test (#7511) The header still claimed the CR 603.12 mandatory gate was out of scope; the previous commit made the file measure exactly that gate. Co-Authored-By: Claude Fable 5 --- .../integration/mandatory_reflexive_modal_parent.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs b/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs index f4ba37bc9f..e1d1af6956 100644 --- a/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs +++ b/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs @@ -24,11 +24,11 @@ //! - 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`). use engine::game::scenario::{GameScenario, P0, P1}; use engine::types::actions::GameAction; From 5075bc2b0ab492814f5f734e2ea011ab18c39e11 Mon Sep 17 00:00:00 2001 From: cuinhellcat <217210902+cuinhellcat@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:28:41 +0200 Subject: [PATCH 3/4] test(engine): pin the RemoveCounter event-witness gate through the production resolver (#7511) Review round 1: the mandatory-RemoveCounter pair now runs the full resolver -> event slice -> sub-walk path with a self-referential "remove a +1/+1 counter from it. When you do, draw a card." chain -- zero counters suppress the reflexive, an available counter preserves it. Probes, both directions: reverting the gate fails the suppression half (left: (1, Some(0))); cutting the parent slice to &[] fails the witness half. The Vhal cards themselves hang on the unsupported "specializes" trigger, so the pair pins the class through oracle-built cards. Plus the review-suggested guard row: a non-WhenYouDo condition is never this stage's business. Co-Authored-By: Claude Fable 5 --- crates/engine/src/game/effects/mod.rs | 16 +++ .../mandatory_reflexive_modal_parent.rs | 126 ++++++++++++++++++ 2 files changed, 142 insertions(+) diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index 80306d50d2..28423a0064 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -20684,6 +20684,22 @@ mod tests { !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] diff --git a/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs b/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs index e1d1af6956..3f52faeaf8 100644 --- a/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs +++ b/crates/engine/tests/integration/mandatory_reflexive_modal_parent.rs @@ -29,6 +29,11 @@ //! 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; @@ -207,3 +212,124 @@ fn an_impossible_exile_creates_no_reflexive() { 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 + ); +} From 3ca1b4e9b61c0d6ea09b72c4268ec0607d4f5f94 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 21 Aug 2026 09:03:17 -0700 Subject: [PATCH 4/4] fix(PR-7576): refresh prompt census coordinates Co-authored-by: cuinhellcat <217210902+cuinhellcat@users.noreply.github.com> --- crates/engine/src/game/engine.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 2a3a205c06..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:7368".to_string(), - "game/effects/mod.rs:7445".to_string(), - "game/effects/mod.rs:11272".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.