fix(engine): a mandatory parent that did nothing creates no "when you do" reflexive (#7511)

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<GameEvent> = 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);

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.

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<String>,
+    plains_in_hand: usize,
+    counters_on_scholar: Option<u32>,
+}
+
+/// 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
+    );
+}