From 199721a6a954ea5dc72e4b8fcd4bd1a4f75093bb Mon Sep 17 00:00:00 2001 From: matthewevans Date: Wed, 19 Aug 2026 13:07:05 -0700 Subject: [PATCH 1/2] fix(engine): skip the legality clone for self-sacrifice mana costs The mana-availability display sweep cloned the whole GameState once per mana source whose cost the cheap gate could not conclusively decide. Treasure's "{T}, Sacrifice this: Add one mana of any color" is exactly that shape: AbilityCost::Sacrifice is unconditionally uncovered by all_components_cheap_gate_covered, so every Treasure fell through to can_activate_mana_ability_by_simulation and a full state clone. On a real 4-player Commander board (676 objects, 193 Treasure tokens) derive_display_state took 2233 ms per resolution, with layers_full growing exactly as N(N+1)/2 -- O(N^2 x battlefield), because each resolution adds a mana source that the next sweep re-clones. Widen the skip to whole-tree choice-free costs that sacrifice exactly the ability's own source, reusing the existing classifier mana_sources::has_unambiguous_self_sacrifice_component. Aggregate requirements, Count{n>1} and every non-SelfRef target are excluded by construction by that predicate's SelfRef / Count{1} match -- a non-self sacrifice may have no legal victim, so its simulation stays load-bearing. Two conservative guards keep the answer identical: * CR 601.2g -- a source already committed to a pending spell's deferred additional sacrifice cost is reserved, and paying this ability's cost would then error. Reuses the payment path's own authority, cost_sacrifices_reserved_source. * CR 118.3 + CR 601.2h -- the readiness gate evaluates player_cant_sacrifice_as_cost on the pre-payment state, but payment re-evaluates it after this tree's {T} component has already tapped the source, and a prohibition's object filter can read that tapped bit. An O(1) static-presence read declines the fast path whenever any CantPayCost static is in play. Both guards decline into today's exact simulation, so a spurious guard costs performance and never correctness. CR 616.1 needs no guard: a replacement on the sacrifice's battlefield to graveyard move makes sacrifice_permanent return NeedsReplacementChoice, which the self-sacrifice payment arm maps to Ok(Paused), so the simulation returns Ok and reports the same answer the fast path does. --- crates/engine/src/game/mana_abilities.rs | 693 +++++++++++++++++- crates/engine/src/game/mana_sources.rs | 91 +++ crates/engine/tests/integration/main.rs | 1 + .../mana_display_self_sacrifice_clone_gate.rs | 208 ++++++ 4 files changed, 982 insertions(+), 11 deletions(-) create mode 100644 crates/engine/tests/integration/mana_display_self_sacrifice_clone_gate.rs diff --git a/crates/engine/src/game/mana_abilities.rs b/crates/engine/src/game/mana_abilities.rs index 92fe535cb9..8c5fb4be70 100644 --- a/crates/engine/src/game/mana_abilities.rs +++ b/crates/engine/src/game/mana_abilities.rs @@ -1800,6 +1800,64 @@ fn mana_ability_ready_without_simulation_gated( true } +/// CR 605.3a + CR 106.12 + CR 107.6 + CR 701.21a: True when the full-state +/// legality clone in [`can_activate_mana_ability_now_gated`] would only +/// re-derive an answer the non-simulating readiness gate has already settled. +/// +/// Two disjoint shapes qualify: +/// * [`mana_sources::cost_conclusively_payable_by_cheap_gate`] — no cost, or a +/// cost built solely from the `{T}`/`{Q}` symbols. Unchanged. +/// * A whole-tree choice-free cost that sacrifices exactly the ability's own +/// source ([`mana_sources::has_unambiguous_self_sacrifice_component`]) — +/// Treasure's `{T}, Sacrifice this token` (CR 111.10a) and Gold's tapless +/// `Sacrifice this token` (CR 111.10c). `SacrificeRequirement::Aggregate`, +/// `Count { count: n > 1 }` and every non-`SelfRef` target are excluded BY +/// CONSTRUCTION by that predicate's `Count { count: 1 }` / `SelfRef` match — +/// a non-self sacrifice may have no legal victim, so its simulation is +/// load-bearing and must not be skipped. +/// +/// The second shape is the first MULTI-component cost the engine answers +/// without simulating, so the two divergences an earlier component can +/// introduce are guarded explicitly. Both guards are conservative: a `true` +/// declines the fast path and falls through to the unchanged simulation, so a +/// spurious guard costs performance, never correctness. +/// +/// Two divergences deliberately need NO guard: +/// * CR 616.1 — a replacement on the sacrifice's battlefield -> graveyard move +/// makes `sacrifice_permanent` return `NeedsReplacementChoice`, which the +/// self-sacrifice payment arm maps to `Ok(ManaAbilityPaymentProgress::Paused)`, +/// so the simulation returns `Ok` and reports the same `true` this path does. +/// * CR 608.2h — the production tail runs after the source has left the +/// battlefield, so a source-referential produced amount (Lotus Blossom's +/// `CountersOn { scope: Source }`) reads last known information. That changes +/// the amount of mana, never the legality answer: the tail's only two `?` +/// operators are `mana_ability_definition` (rescued by the activation's +/// `ability_snapshot`) and `resume_mana_ability_root` (infallible for the +/// `Priority` resume the legality simulation uses). +fn legality_simulation_is_redundant( + state: &GameState, + source_id: ObjectId, + cost: &Option, +) -> bool { + if mana_sources::cost_conclusively_payable_by_cheap_gate(cost) { + return true; + } + mana_sources::has_unambiguous_self_sacrifice_component(cost) + // CR 601.2g: a permanent already committed to a pending spell's + // additional sacrifice cost is reserved; paying this ability's cost + // then errors at `continue_mana_ability_cost_payment_in_node`. Reuses + // the payment path's own authority rather than re-deriving it. + && !cost_sacrifices_reserved_source(state, source_id, cost) + // CR 118.3 + CR 601.2h: the readiness gate evaluated + // `player_cant_sacrifice_as_cost` on the PRE-payment state, but the + // payment re-evaluates it after this tree's `{T}` component has + // already tapped the source, and a prohibition's object filter can + // read that tapped bit (`FilterProp::Tapped`). O(1) presence read + // (CR 604.1): a `false` here is precise post-flush, so the two + // evaluations are provably identical; a `true` declines and simulates. + && !static_kind_present(state, StaticModeKind::CantPayCost) +} + pub fn can_activate_mana_ability_now( state: &GameState, player: PlayerId, @@ -1850,8 +1908,12 @@ pub fn can_activate_mana_ability_now_gated( // simulation. Eliminates the mana-display board-sweep clone-storm (Cryptolith // Rite granting bare `{T}: Add` to ~700 tokens => ~700 clones/sweep). Mana/ // resource/composite costs still simulate — the auto-tap affordability - // witness (CR 601.2g) must not flip UNAVAILABLE->AVAILABLE. - if mana_sources::cost_conclusively_payable_by_cheap_gate(&ability_def.cost) { + // witness (CR 601.2g) must not flip UNAVAILABLE->AVAILABLE. CR 111.10a + + // CR 701.21a: a whole-tree choice-free cost that sacrifices the ability's + // OWN source (Treasure, Gold, Lotus Petal) is conclusively decided the same + // way, behind two state-aware guards — see + // [`legality_simulation_is_redundant`]. + if legality_simulation_is_redundant(state, source_id, &ability_def.cost) { return true; } can_activate_mana_ability_by_simulation(state, player, source_id, ability_index, ability_def) @@ -8440,29 +8502,638 @@ mod tests { ); } - /// CR 601.2g: A `Composite{{Tap, Sacrifice}}` mana cost (Treasure) is NOT - /// conclusively decided by the cheap gate, so it must still simulate — the - /// must-simulate path is preserved (clone >= 1) even though it is activatable. - /// A(b). The self-sacrifice is always a legal target, so this does NOT build a - /// cost that passes `is_payable` yet fails simulation. + /// CR 604.1: Make the O(1) `StaticModePresence` index precise, then zero the + /// perf counters. **Every self-sacrifice cheap-gate assertion below must go + /// through this.** A fresh `GameState` seeds + /// `StaticModePresence::all_present()`, so `legality_simulation_is_redundant`'s + /// `CantPayCost` presence guard declines the fast path until the layers + /// pipeline has flushed — a test that skips the flush measures an inert fast + /// path and its `== 0` clone assertion fails. Production always flushes first + /// (`public_state::finalize_rules_state` -> `finalize_display_state`), so this + /// mirrors production rather than papering over it. + fn flush_and_reset(state: &mut GameState) { + crate::game::layers::flush_layers(state); + crate::game::perf_counters::reset(); + } + + /// CR 701.21a: the bare self-sacrifice cost component — "Sacrifice this". + fn self_sacrifice_cost() -> AbilityCost { + AbilityCost::Sacrifice(SacrificeCost::count(TargetFilter::SelfRef, 1)) + } + + /// CR 111.10a: Treasure's `{T}, Sacrifice this token` cost tree. + fn tap_and_self_sacrifice_cost() -> AbilityCost { + AbilityCost::Composite { + costs: vec![AbilityCost::Tap, self_sacrifice_cost()], + } + } + + fn any_one_color(count: QuantityExpr) -> ManaProduction { + ManaProduction::AnyOneColor { + count, + color_options: ManaColor::ALL.to_vec(), + contribution: ManaContribution::Base, + } + } + + /// Attach one activated mana ability (`cost` -> `produced`) to a fresh + /// battlefield object at ability index 0. Single builder for the + /// self-sacrifice cheap-gate fixtures, so each test below states only the + /// axis it actually varies. + fn spawn_mana_source( + state: &mut GameState, + card: u64, + player: PlayerId, + name: &str, + cost: AbilityCost, + produced: ManaProduction, + ) -> ObjectId { + let id = create_object( + state, + CardId(card), + player, + name.to_string(), + Zone::Battlefield, + ); + let def = AbilityDefinition::new( + AbilityKind::Activated, + Effect::Mana { + produced, + restrictions: vec![], + grants: vec![], + expiry: None, + target: None, + }, + ) + .cost(cost); + Arc::make_mut(&mut state.objects.get_mut(&id).unwrap().abilities).push(def); + id + } + + /// CR 118.3: install a global "players can't sacrifice a creature to pay a + /// cost" static (Yasharn class) on its own battlefield permanent. Fixture + /// shape mirrors `sacrifice_mana_cost_rejects_prohibited_selected_permanent`. + fn install_cant_sacrifice_creature_static(state: &mut GameState, card: u64, player: PlayerId) { + let lock = create_object( + state, + CardId(card), + player, + "Cost Lock".to_string(), + Zone::Battlefield, + ); + state + .objects + .get_mut(&lock) + .unwrap() + .static_definitions + .push(StaticDefinition::new(StaticMode::CantPayCost { + who: ProhibitionScope::AllPlayers, + cost: CostPaymentProhibition::Sacrifice { + filter: TargetFilter::Typed(TypedFilter::creature()), + }, + })); + } + + /// CR 111.10a + CR 701.21a: A Treasure's `Composite{{Tap, Sacrifice this}}` + /// mana cost IS conclusively decided without simulating. The sacrifice target + /// is the ability's own source, so — behind the two state-aware guards in + /// `legality_simulation_is_redundant` — it is exactly as deterministic as the + /// bare `{T}` cost the cheap gate already skips. + /// + /// A(b), **rewritten**: this test previously pinned the pre-fix `clone >= 1` + /// behavior, which is the behavior being changed. + /// + /// REVERT-PROBE: drop the `has_unambiguous_self_sacrifice_component` disjunct + /// from `legality_simulation_is_redundant` and `state_clone_for_legality` + /// returns to 1 while `activatable` stays true. #[test] - fn composite_tap_sacrifice_still_simulates() { + fn composite_tap_self_sacrifice_skips_legality_clone() { let mut state = GameState::new_two_player(42); let treasure = make_any_color_treasure(&mut state, 9301, PlayerId(0), ManaColor::ALL.to_vec()); let def = state.objects.get(&treasure).unwrap().abilities[0].clone(); + flush_and_reset(&mut state); + let activatable = can_activate_mana_ability_now(&state, PlayerId(0), treasure, 0, &def); + let snap = crate::game::perf_counters::snapshot(); + + assert!( + activatable, + "an untapped Treasure with a legal self-sacrifice is activatable \ + (positive reach-guard: a 0-clone count is meaningless if the source \ + was rejected upstream by the readiness gate)" + ); + assert_eq!( + snap.state_clone_for_legality, 0, + "a whole-tree choice-free self-sacrifice cost is conclusively payable \ + (CR 111.10a + CR 701.21a) — no legality clone" + ); + } + + /// CR 111.10c: Gold's tapless `Sacrifice this token: Add one mana of any + /// color` also skips the clone. The design deliberately composes + /// `has_unambiguous_self_sacrifice_component` (which requires a self-sacrifice + /// component to be present) rather than the `{T}`/`{Q}` anchor, so the tapless + /// half of the class is in — a *tapped* Gold token genuinely can still be + /// sacrificed for mana. + /// + /// REVERT-PROBE: re-anchor the fast path on `has_tap_component` and this goes + /// red while `composite_tap_self_sacrifice_skips_legality_clone` stays green. + #[test] + fn tapless_self_sacrifice_skips_legality_clone() { + let mut state = GameState::new_two_player(42); + let gold = spawn_mana_source( + &mut state, + 9310, + PlayerId(0), + "Gold", + self_sacrifice_cost(), + any_one_color(QuantityExpr::Fixed { value: 1 }), + ); + // CR 106.12: a tapped source with no {T} component is still payable — + // the readiness gate's tapped check is gated on `has_tap_component`. + state.objects.get_mut(&gold).unwrap().tapped = true; + let def = state.objects.get(&gold).unwrap().abilities[0].clone(); + + flush_and_reset(&mut state); + let activatable = can_activate_mana_ability_now(&state, PlayerId(0), gold, 0, &def); + let snap = crate::game::perf_counters::snapshot(); + + assert!( + activatable, + "a tapped Gold token can still pay its tapless self-sacrifice cost \ + (CR 111.10c) — positive reach-guard for the clone count below" + ); + assert_eq!( + snap.state_clone_for_legality, 0, + "a tapless self-sacrifice cost is conclusively payable — no legality clone" + ); + } + + /// **The direct discharge of "the fast path must not change the ANSWER."** + /// + /// For every shape the fast path now skips, the skipped simulation is run + /// explicitly and asserted to return the same `true` — so the design rests on + /// a measurement rather than on the assumption that the simulation would have + /// agreed. This is an equivalence test: it passes before and after the change + /// by construction, and it is the anti-vacuity backstop for U1/U2. + /// + /// Shapes (d) and (e) are the **Lotus Blossom class** (`lotus blossom`, + /// `glittering stockpile`, `shrine of boundless growth`): the produced amount + /// is `CountersOn { scope: Source }`, so the production tail reads the source + /// **after** `sacrifice_permanent` has already moved it to the graveyard — + /// last known information per CR 608.2h. (e) is the boundary where that read + /// yields zero. Either way the tail is infallible, so the legality answer + /// stays `true`; only the *amount* of mana can differ. + #[test] + fn self_sacrifice_fast_path_answer_matches_simulation() { + let assert_agrees = + |state: &GameState, id: ObjectId, def: &AbilityDefinition, label: &str| { + assert!( + can_activate_mana_ability_by_simulation(state, PlayerId(0), id, 0, def), + "{label}: the simulation the fast path skips must itself answer true" + ); + assert!( + can_activate_mana_ability_now(state, PlayerId(0), id, 0, def), + "{label}: the fast path must report the simulation's answer" + ); + }; + + // (a) Treasure — `{T}, Sacrifice this` -> one mana of any color. + let mut state = GameState::new_two_player(42); + let treasure = + make_any_color_treasure(&mut state, 9320, PlayerId(0), ManaColor::ALL.to_vec()); + let def = state.objects.get(&treasure).unwrap().abilities[0].clone(); + crate::game::layers::flush_layers(&mut state); + assert_agrees(&state, treasure, &def, "Treasure {T} + self-sacrifice"); + + // (b) Gold — tapless `Sacrifice this`. + let mut state = GameState::new_two_player(42); + let gold = spawn_mana_source( + &mut state, + 9321, + PlayerId(0), + "Gold", + self_sacrifice_cost(), + any_one_color(QuantityExpr::Fixed { value: 1 }), + ); + let def = state.objects.get(&gold).unwrap().abilities[0].clone(); + crate::game::layers::flush_layers(&mut state); + assert_agrees(&state, gold, &def, "Gold tapless self-sacrifice"); + + // (c) Colorless production — no color prompt, so the simulation runs the + // whole post-sacrifice production tail instead of parking on a choice. + let mut state = GameState::new_two_player(42); + let scion = spawn_mana_source( + &mut state, + 9322, + PlayerId(0), + "Eldrazi Scion", + self_sacrifice_cost(), + ManaProduction::Colorless { + count: QuantityExpr::Fixed { value: 1 }, + }, + ); + let def = state.objects.get(&scion).unwrap().abilities[0].clone(); + crate::game::layers::flush_layers(&mut state); + assert_agrees(&state, scion, &def, "colorless self-sacrifice production"); + + // (d) CR 608.2h — source-referential produced amount, counters PRESENT. + for (counters, label) in [ + (3u32, "Lotus Blossom class, 3 petal counters"), + (0u32, "Lotus Blossom class, ZERO petal counters"), + ] { + let mut state = GameState::new_two_player(42); + let blossom = spawn_mana_source( + &mut state, + 9323, + PlayerId(0), + "Lotus Blossom", + tap_and_self_sacrifice_cost(), + any_one_color(QuantityExpr::Ref { + qty: QuantityRef::CountersOn { + scope: ObjectScope::Source, + counter_type: Some(CounterType::Generic("petal".to_string())), + }, + }), + ); + if counters > 0 { + state + .objects + .get_mut(&blossom) + .unwrap() + .counters + .insert(CounterType::Generic("petal".to_string()), counters); + } + let def = state.objects.get(&blossom).unwrap().abilities[0].clone(); + crate::game::layers::flush_layers(&mut state); + assert_agrees(&state, blossom, &def, label); + } + } + + /// CR 701.21a: a **non-self** `Sacrifice` target stays OUT of the fast path — + /// a legal victim may not exist, so its simulation is load-bearing. Phyrexian + /// Altar shape, with a legal victim on the board so the readiness gate passes + /// and the decision seam is genuinely reached. + /// + /// The `activatable == true` assertion is the paired positive control: it + /// proves readiness passed, so `>= 1` measures the fast path declining rather + /// than an upstream rejection. + #[test] + fn non_self_sacrifice_mana_cost_still_simulates() { + let mut state = GameState::new_two_player(42); + let altar = spawn_mana_source( + &mut state, + 9330, + PlayerId(0), + "Phyrexian Altar", + AbilityCost::Composite { + costs: vec![ + AbilityCost::Tap, + AbilityCost::Sacrifice(SacrificeCost::count( + TargetFilter::Typed(TypedFilter::creature()), + 1, + )), + ], + }, + any_one_color(QuantityExpr::Fixed { value: 1 }), + ); + let victim = create_object( + &mut state, + CardId(9331), + PlayerId(0), + "Grizzly Bears".to_string(), + Zone::Battlefield, + ); + state + .objects + .get_mut(&victim) + .unwrap() + .card_types + .core_types + .push(CoreType::Creature); + let def = state.objects.get(&altar).unwrap().abilities[0].clone(); + + flush_and_reset(&mut state); + let activatable = can_activate_mana_ability_now(&state, PlayerId(0), altar, 0, &def); + let snap = crate::game::perf_counters::snapshot(); + + assert!( + activatable, + "a legal victim is on the board, so readiness passes and the decision \ + seam is really reached (positive control for the clone count)" + ); + assert!( + snap.state_clone_for_legality >= 1, + "a non-self Sacrifice target must still simulate — the victim's \ + existence is not settled by the cost's AST shape" + ); + } + + /// **Guard 1 — CR 601.2g.** A permanent already committed to a pending + /// spell's additional sacrifice cost is reserved: paying this ability's cost + /// would error at `continue_mana_ability_cost_payment_in_node`, so the fast + /// path must decline and let the simulation report `false`. + /// + /// Multi-authority fixture: two definition-identical Treasures on one board, + /// exactly one reserved. A hoisted or board-global guard fails this test, + /// because the guard is keyed on `source_id`. + /// + /// **Non-vacuity:** the unreserved sibling reporting `true` is what proves the + /// reservation was really installed — `PendingCast::new` seeds + /// `deferred_sacrificed_permanents` **empty**, so a fixture that only installs + /// a `PendingCast` reserves nothing and both sources would report `true`. + /// + /// REVERT-PROBE: delete the `cost_sacrifices_reserved_source` term and the + /// reserved source flips to `true` with 0 clones. + #[test] + fn self_sacrifice_reserved_for_pending_cast_still_simulates() { + use crate::types::game_state::{DeferredSacrificeSelection, PendingCast}; + + let mut state = GameState::new_two_player(42); + let reserved = + make_any_color_treasure(&mut state, 9340, PlayerId(0), ManaColor::ALL.to_vec()); + let sibling = + make_any_color_treasure(&mut state, 9341, PlayerId(0), ManaColor::ALL.to_vec()); + let spell = create_object( + &mut state, + CardId(9342), + PlayerId(0), + "Some Spell".to_string(), + Zone::Stack, + ); + let mut pending = PendingCast::new( + spell, + CardId(9342), + ResolvedAbility::new( + Effect::unimplemented("Some Spell", "test fixture"), + Vec::new(), + spell, + PlayerId(0), + ), + ManaCost::generic(1), + ); + // CR 601.2g: `deferred_spell_sacrifice_reserved` matches on `object_id` + // alone, and `PendingCast::new` seeds this vector empty — the reservation + // must be pushed explicitly or the test passes vacuously. + pending + .deferred_sacrificed_permanents + .push(DeferredSacrificeSelection { + object_id: reserved, + filter: TargetFilter::Typed(TypedFilter::permanent()), + }); + state.pending_cast = Some(Box::new(pending)); + + let def = state.objects.get(&reserved).unwrap().abilities[0].clone(); + + flush_and_reset(&mut state); + let reserved_activatable = + can_activate_mana_ability_now(&state, PlayerId(0), reserved, 0, &def); + let reserved_snap = crate::game::perf_counters::snapshot(); + crate::game::perf_counters::reset(); + let sibling_activatable = + can_activate_mana_ability_now(&state, PlayerId(0), sibling, 0, &def); + let sibling_snap = crate::game::perf_counters::snapshot(); + + assert!( + !reserved_activatable, + "a Treasure reserved for a pending spell's additional sacrifice cost \ + can't also pay this mana ability's cost (CR 601.2g)" + ); + assert!( + reserved_snap.state_clone_for_legality >= 1, + "the reserved source declines the fast path and simulates" + ); + assert!( + sibling_activatable, + "the definition-identical UNRESERVED sibling on the same board is \ + still activatable — this is the non-vacuity guard proving the \ + reservation was really installed" + ); + assert_eq!( + sibling_snap.state_clone_for_legality, 0, + "the guard is per-source_id, not board-global: the sibling still \ + takes the fast path" + ); + } + + /// **Guard 2 — CR 118.3 + CR 601.2h, non-vacuously.** The readiness gate + /// evaluates `player_cant_sacrifice_as_cost` on the PRE-payment state, but the + /// payment re-evaluates it after this cost tree's `{T}` component has already + /// tapped the source, and a prohibition's object filter can read that tapped + /// bit (`FilterProp::Tapped`). So whenever any `CantPayCost` static is + /// functioning, the fast path declines and simulates. + /// + /// The static here filters **creatures**, which does not match the artifact + /// Treasure, so readiness still passes and the decision seam is genuinely + /// reached — the guard is measured, not inferred from an upstream rejection. + /// + /// REVERT-PROBE: delete the `static_kind_present` term and the main arm's + /// clone count drops to 0. + #[test] + fn cant_pay_cost_static_presence_declines_self_sacrifice_fast_path() { + // Main arm: the static IS present. + let mut state = GameState::new_two_player(42); + let treasure = + make_any_color_treasure(&mut state, 9350, PlayerId(0), ManaColor::ALL.to_vec()); + install_cant_sacrifice_creature_static(&mut state, 9351, PlayerId(0)); + let def = state.objects.get(&treasure).unwrap().abilities[0].clone(); + + flush_and_reset(&mut state); + // Non-vacuity FIRST: `static_kind_present` is an O(1) absence + // short-circuit, so an unflushed or mis-built board would let the + // assertions below pass for free. + assert!( + static_kind_present(&state, StaticModeKind::CantPayCost), + "the CantPayCost static must be functioning, or Guard 2 is untested" + ); let activatable = can_activate_mana_ability_now(&state, PlayerId(0), treasure, 0, &def); let snap = crate::game::perf_counters::snapshot(); assert!( activatable, - "an untapped Treasure with a legal self-sacrifice is activatable" + "the prohibition filters creatures, so an artifact Treasure is still \ + activatable — readiness passed and the decision seam was reached" ); assert!( snap.state_clone_for_legality >= 1, - "a Composite with a Sacrifice component must still simulate (CR 601.2g)" + "Guard 2 declines the fast path while any CantPayCost static is present" + ); + + // Paired positive control: the identical board WITHOUT the static. + let mut control = GameState::new_two_player(42); + let treasure = + make_any_color_treasure(&mut control, 9350, PlayerId(0), ManaColor::ALL.to_vec()); + let def = control.objects.get(&treasure).unwrap().abilities[0].clone(); + + flush_and_reset(&mut control); + assert!( + !static_kind_present(&control, StaticModeKind::CantPayCost), + "control board carries no CantPayCost static" + ); + let activatable = can_activate_mana_ability_now(&control, PlayerId(0), treasure, 0, &def); + let snap = crate::game::perf_counters::snapshot(); + + assert!(activatable, "control Treasure is activatable"); + assert_eq!( + snap.state_clone_for_legality, 0, + "without the static the same board takes the fast path — this is what \ + makes the `>= 1` above a measurement rather than a broken board" + ); + } + + /// **The correctness guard: a genuinely prohibited self-sacrifice source must + /// report `false`.** The opposite fixture from Guard 2's test — here the + /// `CantPayCost { Sacrifice { creature } }` filter **matches** the source, so + /// the source really cannot pay. + /// + /// The answer must arrive from the **readiness gate** (`is_payable_for_mana_ability` + /// -> the `SelfRef` sacrifice arm's `!player_cant_sacrifice_as_cost` check), + /// BEFORE the cheap-gate decision is consulted. That is why + /// `state_clone_for_legality == 0` is load-bearing here: it pins that the + /// `false` came from readiness and not from a simulation. It goes red if + /// anyone reorders the decision seam ahead of the readiness gate. + /// + /// Both sub-cases carry a paired no-static control asserting `true`, so the + /// negative cannot pass because of summoning sickness, a tapped bit, or a + /// missing core type. **Deliberate asymmetry:** the control also reports 0 + /// clones (it takes the fast path), so the discriminator between the arms is + /// `activatable`, never the counter. + #[test] + fn self_sacrifice_under_cant_pay_cost_static_reports_unactivatable() { + for (label, cost) in [ + ("tapless self-sacrifice creature", self_sacrifice_cost()), + ( + "tap-anchored self-sacrifice creature", + tap_and_self_sacrifice_cost(), + ), + ] { + let build = |with_static: bool| { + let mut state = GameState::new_two_player(42); + let source = spawn_mana_source( + &mut state, + 9360, + PlayerId(0), + "Wild Cantor", + cost.clone(), + any_one_color(QuantityExpr::Fixed { value: 1 }), + ); + { + let obj = state.objects.get_mut(&source).unwrap(); + // CR 118.3: the prohibition's filter is applied to the + // sacrificed object itself, so the source must be a creature + // for the static to match it. + obj.card_types.core_types.push(CoreType::Creature); + // CR 302.6: keep the {T} sub-case out of the summoning-sickness + // gate, so the only reason for a `false` is the prohibition. + obj.summoning_sick = false; + } + if with_static { + install_cant_sacrifice_creature_static(&mut state, 9361, PlayerId(0)); + } + let def = state.objects.get(&source).unwrap().abilities[0].clone(); + (state, source, def) + }; + + let (mut state, source, def) = build(true); + flush_and_reset(&mut state); + assert!( + static_kind_present(&state, StaticModeKind::CantPayCost), + "{label}: the CantPayCost static must be functioning, or this \ + negative assertion is vacuous" + ); + let activatable = can_activate_mana_ability_now(&state, PlayerId(0), source, 0, &def); + let snap = crate::game::perf_counters::snapshot(); + + assert!( + !activatable, + "{label}: a creature under a `can't sacrifice a creature to pay a \ + cost` static can't pay its own self-sacrifice mana cost (CR 118.3)" + ); + assert_eq!( + snap.state_clone_for_legality, 0, + "{label}: the `false` must come from the readiness gate, BEFORE \ + the cheap-gate decision — not from a legality simulation" + ); + + let (mut control, source, def) = build(false); + flush_and_reset(&mut control); + assert!( + !static_kind_present(&control, StaticModeKind::CantPayCost), + "{label}: control board carries no CantPayCost static" + ); + let activatable = can_activate_mana_ability_now(&control, PlayerId(0), source, 0, &def); + assert!( + activatable, + "{label}: without the static the SAME source is activatable — this \ + is what proves the `false` above is caused by the prohibition and \ + not by sickness, a tapped bit, or a missing core type" + ); + } + } + + /// **CR 616.1 — the replacement disposition, tested rather than asserted.** + /// A "would be put into a graveyard" `Moved` replacement applies to the + /// sacrifice's inner battlefield -> graveyard move, so `sacrifice_permanent` + /// returns `NeedsReplacementChoice`. The self-sacrifice payment arm maps that + /// to `Ok(ManaAbilityPaymentProgress::Paused)`, which the payment loop returns + /// as `Ok` — so the simulation reports `true`, the same answer the fast path + /// reports. A replacement makes the payment **pause**, never **fail**. + /// + /// **Reach-guard first, and it is mandatory:** without it the test would pass + /// on a replacement definition that never matched, which is the exact failure + /// mode this row exists to rule out. + #[test] + fn self_sacrifice_with_graveyard_replacement_matches_simulation() { + use crate::types::ability::{ReplacementDefinition, ReplacementMode}; + use crate::types::replacements::ReplacementEvent; + + let mut state = GameState::new_two_player(42); + let treasure = + make_any_color_treasure(&mut state, 9370, PlayerId(0), ManaColor::ALL.to_vec()); + let leyline = create_object( + &mut state, + CardId(9371), + PlayerId(0), + "Leyline of the Void".to_string(), + Zone::Battlefield, + ); + state + .objects + .get_mut(&leyline) + .unwrap() + .replacement_definitions = vec![ReplacementDefinition::new(ReplacementEvent::Moved) + .destination_zone(Zone::Graveyard) + .mode(ReplacementMode::Optional { decline: None })] + .into(); + let def = state.objects.get(&treasure).unwrap().abilities[0].clone(); + crate::game::layers::flush_layers(&mut state); + + // Reach-guard: prove on a throwaway clone that the definition genuinely + // intercepts this sacrifice's inner graveyard move. + let mut probe = state.clone(); + let outcome = + sacrifice::sacrifice_permanent(&mut probe, treasure, PlayerId(0), &mut Vec::new()) + .expect("sacrificing an on-battlefield permanent must not error"); + assert!( + matches!( + outcome, + sacrifice::SacrificeOutcome::NeedsReplacementChoice(_) + ), + "the graveyard-move replacement must really apply (CR 616.1), or the \ + equivalence below would be asserted on a replacement that never fired" + ); + + assert!( + can_activate_mana_ability_by_simulation(&state, PlayerId(0), treasure, 0, &def), + "a CR 616.1 replacement makes the simulated payment PAUSE, not fail — \ + `activate_mana_ability` still returns Ok" + ); + assert!( + can_activate_mana_ability_now(&state, PlayerId(0), treasure, 0, &def), + "the fast path reports the same answer, so no guard is needed for the \ + replacement axis" ); } @@ -8588,7 +9259,7 @@ mod tests { /// and EffectCost an arbitrary effect, so these are asserted at the /// classifier/wrapper level rather than as full runtime cards; the runtime /// "falls through to simulate" path itself is exercised by - /// `composite_tap_sacrifice_still_simulates` (A(b)) and + /// `non_self_sacrifice_mana_cost_still_simulates` (A(b)) and /// `filter_land_composite_still_activatable_via_simulation` (C). #[test] fn cheap_gate_hostile_costs_must_simulate() { diff --git a/crates/engine/src/game/mana_sources.rs b/crates/engine/src/game/mana_sources.rs index 405053282c..fd03b32e7c 100644 --- a/crates/engine/src/game/mana_sources.rs +++ b/crates/engine/src/game/mana_sources.rs @@ -4249,6 +4249,97 @@ mod tests { ); } + /// Adjacent-shape hostiles for `has_unambiguous_self_sacrifice_component`, + /// which is also the classifier the mana-display legality fast path + /// (`mana_abilities::legality_simulation_is_redundant`) composes. Covers only + /// what `led_shaped_discard_sacrifice_stays_off_auto_tap` above does not — + /// that test already pins Gold, Treasure, LED and LED-reordered. + /// + /// The point of the negative rows is that **none of them needs a guard**: the + /// literal `SelfRef` / `Count { count: 1 }` struct pattern excludes every one + /// of them by construction. In particular `SacrificeRequirement::Aggregate` + /// (Phyrexian Dreadnought class) cannot match `Count { count: 1 }`, so it can + /// never reach a fast path — pinned here so a later "simplification" of the + /// pattern into a wildcard is caught. + #[test] + fn self_sacrifice_classifier_excludes_hostile_shapes() { + use crate::types::ability::{ + Comparator, SacrificeAggregateStat, SacrificeRequirement, TargetFilter, TypedFilter, + }; + + // Positive controls first: without these, every `!` below could pass on a + // classifier that returns `false` unconditionally. + assert!( + has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Composite { + costs: vec![ + AbilityCost::Tap, + AbilityCost::Sacrifice(SacrificeCost::count(TargetFilter::SelfRef, 1)), + ], + })), + "positive control: Treasure's {{T}} + single self-sacrifice is accepted" + ); + assert!( + has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Sacrifice( + SacrificeCost::count(TargetFilter::SelfRef, 1) + ))), + "positive control: Gold's bare single self-sacrifice is accepted" + ); + + // CR 701.21: sacrificing TWO of this permanent is not the deterministic + // single-self shape — `Count { count: 2 }` does not match `Count { count: 1 }`. + assert!( + !has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Sacrifice( + SacrificeCost::count(TargetFilter::SelfRef, 2) + ))), + "Count {{ count: 2 }} is excluded by construction" + ); + + // CR 701.21: an aggregate requirement (Phyrexian Dreadnought class) needs a + // player-chosen SET of permanents — excluded by construction, no guard. + assert!( + !has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Sacrifice( + SacrificeCost::new( + TargetFilter::SelfRef, + SacrificeRequirement::Aggregate { + stat: SacrificeAggregateStat::TotalPower, + comparator: Comparator::GE, + value: 12, + }, + ) + ))), + "SacrificeRequirement::Aggregate is excluded by construction" + ); + + // A non-self target (Phyrexian Altar class) may have no legal victim, so + // its payability is NOT decided by the cost's shape. + assert!( + !has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Sacrifice( + SacrificeCost::count(TargetFilter::Typed(TypedFilter::creature()), 1) + ))), + "a non-self Sacrifice target is not an unambiguous self-sacrifice" + ); + + // A pure {T} cost is choice-free but carries NO self-sacrifice component, + // so this predicate must not claim it. + assert!( + !has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Composite { + costs: vec![AbilityCost::Tap], + })), + "a {{T}}-only cost has no self-sacrifice component to classify" + ); + + // The degenerate empty Composite is vacuously choice-free, but requiring a + // self-sacrifice component to be PRESENT rejects it — which is why this + // predicate needs no `{T}`/`{Q}` anchor of its own. + assert!( + !has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Composite { + costs: vec![], + })), + "the degenerate empty Composite is rejected because a self-sacrifice \ + component is required to be present" + ); + } + #[test] fn life_payment_mana_source_marks_controller_harm() { let mut state = GameState::new_two_player(42); diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index b7540c667d..b863523eba 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -837,6 +837,7 @@ mod magus_of_the_abyss_scoped_chooser; mod make_an_example_pile_separation; mod mana_autotap_preference; mod mana_cost_reducers_issue_141; +mod mana_display_self_sacrifice_clone_gate; mod mana_drain_refund; mod mana_payment_preview; mod mana_role_fixture_migration; diff --git a/crates/engine/tests/integration/mana_display_self_sacrifice_clone_gate.rs b/crates/engine/tests/integration/mana_display_self_sacrifice_clone_gate.rs new file mode 100644 index 0000000000..73337bb478 --- /dev/null +++ b/crates/engine/tests/integration/mana_display_self_sacrifice_clone_gate.rs @@ -0,0 +1,208 @@ +//! Mana-display sweep clone gate for **self-sacrifice** mana sources (engine tier). +//! +//! Permanent regression guard proving that a board of Treasure-class sources — +//! `{T}, Sacrifice this token: Add one mana of any color` (CR 111.10a) — answers +//! the per-source mana-display legality question **without cloning `GameState`**. +//! Before the fix each such source fell through +//! `can_activate_mana_ability_now_gated`'s cheap gate into +//! `can_activate_mana_ability_by_simulation`, which clones the whole state, so a +//! 193-Treasure board took 193 full-state clones per display sweep. +//! +//! Both tests run at **N and 2N** so the gate proves *non-scaling* rather than one +//! lucky value, and each carries a `has_mana_ability` reach-guard: a `0` clone +//! count is vacuous on a board that was never swept. +//! +//! `mana_display_sweep_still_clones_for_non_self_sacrifice_sources` is the +//! positive control — it holds every other axis fixed and changes only the +//! `Sacrifice` target, which is the discriminator the fix keys on. +//! +//! DB-free by construction: `GameState::new_two_player` + `create_object` only, +//! never loading `client/public/card-data.json` (mirrors `token_storm_scaling_gate.rs`; +//! `scripts/check-test-card-data-load.sh` guards this). Under `cargo nextest` each +//! test runs in its own process, so the `thread_local!` perf counters cannot bleed +//! across tests and the exact `== 0` assertion is sound. + +use engine::game::derived::derive_display_state; +use engine::game::layers::flush_layers; +use engine::game::perf_counters; +use engine::game::public_state::mark_mana_display_dirty; +use engine::game::zones::create_object; +use engine::types::ability::{ + AbilityCost, AbilityDefinition, AbilityKind, Effect, ManaContribution, ManaProduction, + QuantityExpr, SacrificeCost, TargetFilter, TypedFilter, +}; +use engine::types::card_type::CoreType; +use engine::types::game_state::GameState; +use engine::types::identifiers::{CardId, ObjectId}; +use engine::types::mana::ManaColor; +use engine::types::player::PlayerId; +use engine::types::zones::Zone; + +/// Board size. `2 * N` is the paired size that turns "0 clones" from a lucky +/// value into a non-scaling claim. +const N: usize = 16; + +/// `make_any_color_treasure` in `mana_abilities.rs` is `#[cfg(test)]` +/// crate-private, so an integration test must define its own equivalent — the +/// same idiom `token_storm_scaling_gate.rs` uses for a private targeting helper. +/// +/// `sacrifice_target` is the only axis that varies between the two tests: +/// `TargetFilter::SelfRef` is the shape the fast path decides, and +/// `TargetFilter::Typed(permanent)` is the shape it must decline. +fn spawn_sacrifice_mana_source( + state: &mut GameState, + card: u64, + player: PlayerId, + sacrifice_target: TargetFilter, +) -> ObjectId { + let id = create_object( + state, + CardId(card), + player, + "Treasure".to_string(), + Zone::Battlefield, + ); + // CR 111.10a: a Treasure token is an *artifact*. Both boards carry the core + // type so that a `TargetFilter::Typed(permanent)` sacrifice target finds the + // source itself as a legal victim — without it the readiness gate rejects the + // source and the sweep never reaches the decision seam at all. Setting it on + // BOTH boards keeps the two tests differing only by the sacrifice target, + // which is the discriminator the fix keys on. + state + .objects + .get_mut(&id) + .unwrap() + .card_types + .core_types + .push(CoreType::Artifact); + let def = AbilityDefinition::new( + AbilityKind::Activated, + Effect::Mana { + produced: ManaProduction::AnyOneColor { + count: QuantityExpr::Fixed { value: 1 }, + color_options: ManaColor::ALL.to_vec(), + contribution: ManaContribution::Base, + }, + restrictions: vec![], + grants: vec![], + expiry: None, + target: None, + }, + ) + .cost(AbilityCost::Composite { + costs: vec![ + AbilityCost::Tap, + AbilityCost::Sacrifice(SacrificeCost::count(sacrifice_target, 1)), + ], + }); + std::sync::Arc::make_mut(&mut state.objects.get_mut(&id).unwrap().abilities).push(def); + id +} + +/// `count` sources sharing one `sacrifice_target`, already flushed so the O(1) +/// `StaticModePresence` index is precise. +/// +/// **The flush is load-bearing.** A fresh `GameState` seeds +/// `StaticModePresence::all_present()`, so the fast path's `CantPayCost` presence +/// guard declines everything until the layers pipeline has run — an unflushed +/// board measures an inert fast path. Production always flushes first +/// (`public_state::finalize_rules_state` -> `finalize_display_state`), so this +/// mirrors production rather than working around the guard. +fn sacrifice_source_board( + count: usize, + sacrifice_target: TargetFilter, +) -> (GameState, Vec) { + let mut state = GameState::new_two_player(42); + let ids = (0..count) + .map(|i| { + spawn_sacrifice_mana_source( + &mut state, + 9500 + i as u64, + PlayerId(0), + sacrifice_target.clone(), + ) + }) + .collect(); + flush_layers(&mut state); + (state, ids) +} + +/// Run one board-wide mana-display sweep and return its counters. +fn sweep(state: &mut GameState) -> perf_counters::PerfCounterSnapshot { + mark_mana_display_dirty(state); + perf_counters::reset(); + derive_display_state(state); + perf_counters::snapshot() +} + +/// CR 111.10a + CR 701.21a: the board-wide mana-display sweep takes **zero** +/// legality clones over self-sacrifice sources, at both N and 2N. +/// +/// REVERT-PROBE: drop the `has_unambiguous_self_sacrifice_component` disjunct +/// from `mana_abilities::legality_simulation_is_redundant` and this reports N +/// (then 2N) clones — the pre-fix clone storm. +#[test] +fn mana_display_sweep_is_clone_free_for_self_sacrifice_sources() { + for count in [N, 2 * N] { + let (mut state, ids) = sacrifice_source_board(count, TargetFilter::SelfRef); + let snap = sweep(&mut state); + + assert_eq!( + snap.mana_display_sweeps, 1, + "count={count}: exactly one board-wide mana sweep" + ); + assert_eq!( + snap.mana_display_swept_objects, count as u64, + "count={count}: the sweep visited every battlefield object" + ); + // Reach-guard: `0` clones is vacuous unless the sources were really + // classified as activatable mana sources by that sweep. + for id in &ids { + assert!( + state.objects.get(id).unwrap().has_mana_ability, + "count={count}: every self-sacrifice source must report an \ + available mana ability, or the 0-clone count below is vacuous" + ); + } + assert_eq!( + snap.state_clone_for_legality, 0, + "count={count}: a self-sacrifice mana cost is conclusively payable \ + without simulating (revert-failing: pre-fix = {count} clones)" + ); + } +} + +/// The positive control for the gate above, and the class boundary at sweep +/// scale. Only the `Sacrifice` target changes: a `Typed(permanent)` target may +/// have no legal victim in general, so its simulation is load-bearing and the +/// fast path must decline. Each source is its own legal victim here, so +/// readiness still passes and the decision seam is genuinely reached. +/// +/// Without this test, `mana_display_sweep_is_clone_free_for_self_sacrifice_sources` +/// could pass on a sweep that never takes clones at all. +#[test] +fn mana_display_sweep_still_clones_for_non_self_sacrifice_sources() { + for count in [N, 2 * N] { + let (mut state, ids) = + sacrifice_source_board(count, TargetFilter::Typed(TypedFilter::permanent())); + let snap = sweep(&mut state); + + assert_eq!( + snap.mana_display_sweeps, 1, + "count={count}: exactly one board-wide mana sweep" + ); + for id in &ids { + assert!( + state.objects.get(id).unwrap().has_mana_ability, + "count={count}: each source is its own legal victim, so it is \ + activatable — this is the reach-guard for the clone count below" + ); + } + assert!( + snap.state_clone_for_legality >= count as u64, + "count={count}: a non-self Sacrifice target must still simulate per \ + source (got {} clones)", + snap.state_clone_for_legality + ); + } +} From fcaeaeddf6389f26bad5c89c5cb28a78b5f2e7db Mon Sep 17 00:00:00 2001 From: matthewevans Date: Wed, 19 Aug 2026 15:40:19 -0700 Subject: [PATCH 2/2] fix(engine): bound self-sacrifice mana costs to a flat single-leaf tree The self-sacrifice cheap-gate widening admitted cost trees carrying more than one consuming leaf. has_unambiguous_self_sacrifice_component was whole-tree cost_component_choice_free AND cost_has_component, and cost_has_component requires only that at least one self-sacrifice leaf be present -- it imposes no arity bound. Because the payer flattens the tree and pays one leaf at a time, a second consuming leaf re-enters its arm after the first has already mutated the source: Composite[Sacrifice(SelfRef,1), Sacrifice(SelfRef,1)] -> the second leaf hits sacrifice_permanent's not-on-battlefield Err Composite[Tap, Tap, Sacrifice(SelfRef,1)] -> the second Tap hits tap_source's already-tapped Err In both, the simulation answers false while the fast path answered true -- a change of ANSWER, not merely of cost, in the unsafe direction. Bound the classifier to a flat tree carrying exactly one Sacrifice(SelfRef, Count{1}) leaf and at most one Tap leaf. The census recurses so it counts the same multiset append_mana_ability_cost_components builds; a census modelled on cost_has_component's one-level shape would report 1 where the payer pays 2. The flatness clause is required for soundness, not tidiness. A flattened census alone WIDENS the classifier onto nested Composite[Composite[Tap, Sacrifice(SelfRef,1)]], which today is rejected only by cost_has_component's one-level blindness. That same blindness sits upstream in the readiness gate, which keys its tapped and summoning-sick checks on the one-level has_tap_component -- so a tapped source with a nested tree clears readiness, and removing the blindness on the classifier side alone would let it fast-path to true while the payer errors on the flattened {T}. Requiring flatness keeps the acceptance set a strict subset of the previous one, so every consumer can only move toward pre-fix behavior. SacrificeRequirement::Aggregate and Count{n>1} remain excluded by construction through the literal Count{count:1} pattern; no guard was added for them. Also document that the CantPayCost guard keys on board-global static presence rather than on the source, so a single such permanent returns every mana source on the board to the simulation path. --- crates/engine/src/game/mana_abilities.rs | 7 +- crates/engine/src/game/mana_sources.rs | 282 +++++++++++++++++- .../mana_display_self_sacrifice_clone_gate.rs | 112 +++++-- 3 files changed, 361 insertions(+), 40 deletions(-) diff --git a/crates/engine/src/game/mana_abilities.rs b/crates/engine/src/game/mana_abilities.rs index 8c5fb4be70..a92b7f5587 100644 --- a/crates/engine/src/game/mana_abilities.rs +++ b/crates/engine/src/game/mana_abilities.rs @@ -1820,7 +1820,12 @@ fn mana_ability_ready_without_simulation_gated( /// without simulating, so the two divergences an earlier component can /// introduce are guarded explicitly. Both guards are conservative: a `true` /// declines the fast path and falls through to the unchanged simulation, so a -/// spurious guard costs performance, never correctness. +/// spurious guard costs performance, never correctness. Note that Guard 2's +/// granularity is the whole board rather than this source: `static_kind_present` +/// is a board-global `StaticModeKind` presence read (CR 604.1), so a single +/// `CantPayCost` permanent anywhere on the battlefield (Yasharn, Impeccable Sire) +/// returns EVERY mana source to the clone path, not only the sources a +/// prohibition could actually name. /// /// Two divergences deliberately need NO guard: /// * CR 616.1 — a replacement on the sacrifice's battlefield -> graveyard move diff --git a/crates/engine/src/game/mana_sources.rs b/crates/engine/src/game/mana_sources.rs index fd03b32e7c..b03485aefa 100644 --- a/crates/engine/src/game/mana_sources.rs +++ b/crates/engine/src/game/mana_sources.rs @@ -375,6 +375,12 @@ impl ManaSourceOption { /// cost and every component of a `Composite`. Single walker behind all /// component-presence predicates (`has_tap_component`, `has_untap_component`, /// `cost_includes_sacrifice`, `cost_includes_loyalty`). +/// +/// Walks **one `Composite` level only**: a component nested inside a child +/// `Composite` is not seen. Sound for presence checks, whose `false` merely +/// declines a fast path, but never sound for an arity bound — for that use +/// [`count_leaf_components`], which recurses to the same flattened leaf +/// multiset the payment path actually pays. pub(crate) fn cost_has_component( cost: &Option, pred: impl Fn(&AbilityCost) -> bool, @@ -1036,22 +1042,85 @@ pub(crate) fn has_untap_component(cost: &Option) -> bool { /// prompt is never bypassed. It stays off the auto-tap path and remains /// reachable only through /// `has_activatable_non_tap_mana_ability_for_payment`'s manual-payment flow. +/// +/// The tree is additionally ARITY-BOUNDED: exactly one self-sacrifice leaf and +/// at most one `{T}` leaf, counted over the FLATTENED tree, with no nested +/// `Composite`. A multi-consuming-leaf tree is choice-free yet unpayable, +/// because the payment path pays leaves one at a time against the already +/// mutated source: a second self-sacrifice leaf finds the source no longer on +/// the battlefield (CR 701.21a) and a second `{T}` leaf finds it already tapped +/// (CR 118.3), so each errors. Without the bound this predicate would answer +/// `true` where the legality simulation answers `false` — an unsafe flip, since +/// callers use it to skip that simulation. pub(crate) fn has_unambiguous_self_sacrifice_component(cost: &Option) -> bool { - // Whole-tree invariant first (shared authority): rejects any interactive - // sibling such as LED's `Discard`. Then confirm a self-sacrifice component - // is actually present, so a pure `{T}` cost is not misclassified as - // self-sacrifice by this predicate. - cost.as_ref() - .is_some_and(mana_abilities::cost_component_choice_free) - && cost_has_component(cost, |c| { - matches!( - c, - AbilityCost::Sacrifice(SacrificeCost { - target: TargetFilter::SelfRef, - requirement: SacrificeRequirement::Count { count: 1 }, - }) - ) - }) + cost.as_ref().is_some_and(|inner| { + // Whole-tree invariant first (shared authority): rejects any interactive + // sibling such as LED's `Discard`, so only `Tap` and single-token + // self-sacrifice leaves survive to be counted below. + mana_abilities::cost_component_choice_free(inner) + // Keep the leaf multiset visible to the one-level walkers that gate + // this activation (see `cost_tree_is_flat`). + && cost_tree_is_flat(inner) + // CR 701.21a: sacrificing moves the permanent from the battlefield, + // so a second self-sacrifice leaf has nothing left to move. Requiring + // a component to be PRESENT also keeps a pure `{T}` cost from being + // misclassified as self-sacrifice. + && count_leaf_components(inner, &|leaf| { + matches!( + leaf, + AbilityCost::Sacrifice(SacrificeCost { + target: TargetFilter::SelfRef, + requirement: SacrificeRequirement::Count { count: 1 }, + }) + ) + }) == 1 + // CR 118.3: a permanent that's already tapped can't be tapped to pay + // a cost, so a second `{T}` leaf errors the same way. + && count_leaf_components(inner, &|leaf| matches!(leaf, AbilityCost::Tap)) <= 1 + }) +} + +/// Number of FLATTENED LEAF components of `cost` satisfying `pred`. +/// +/// Unlike [`cost_has_component`] — presence, one `Composite` level — this walker +/// recurses to the leaves, so it counts the same multiset the payment path pays: +/// `mana_abilities::append_mana_ability_cost_components` flattens nested +/// `Composite`s identically, then pays one leaf at a time. Every arity bound must +/// use this walker; a one-level count reports `1` for +/// `Composite[Composite[Tap, Sacrifice], Sacrifice]`, where the payer pays two +/// sacrifices. `OneOf` stays opaque — only one of its branches is ever paid, so +/// its contents are not part of the flattened multiset. +fn count_leaf_components(cost: &AbilityCost, pred: &impl Fn(&AbilityCost) -> bool) -> usize { + match cost { + AbilityCost::Composite { costs } => costs + .iter() + .map(|cost| count_leaf_components(cost, pred)) + .sum(), + leaf => usize::from(pred(leaf)), + } +} + +/// True when every leaf of `cost` is visible at the top level: a non-`Composite` +/// cost, or a `Composite` with no `Composite` child. +/// +/// The gates that surround an unambiguous self-sacrifice activation read the cost +/// tree with the ONE-LEVEL [`cost_has_component`] walker: +/// `mana_abilities::mana_ability_ready_without_simulation_gated` gates its tapped, +/// `object_cant_tap` and summoning-sickness checks on [`has_tap_component`] +/// (CR 106.12 + CR 302.6), and [`object_has_tapless_self_sacrifice_mana_ability`] +/// reads `!has_tap_component`. A nested +/// `Composite[Composite[Tap, Sacrifice(SelfRef, 1)]]` hides its `{T}` from both, +/// so an already-tapped source would clear readiness while the payment path — +/// which does flatten — still errors on that `Tap` leaf (CR 118.3). Requiring a +/// flat tree keeps this predicate aligned with every walker that gates it, rather +/// than depending on those walkers to be changed in lockstep. +fn cost_tree_is_flat(cost: &AbilityCost) -> bool { + match cost { + AbilityCost::Composite { costs } => !costs + .iter() + .any(|cost| matches!(cost, AbilityCost::Composite { .. })), + _ => true, + } } /// CR 605.3a + CR 106.12 + CR 302.6: True when `obj` has an activated mana @@ -4340,6 +4409,189 @@ mod tests { ); } + /// CR 118.3 + CR 701.21a: the classifier's LEAF-ARITY bound. A cost tree can + /// be choice-free and still unpayable, because the payment path pays leaves + /// one at a time against the already-mutated source: a second + /// `Sacrifice(SelfRef, 1)` leaf finds the source no longer on the battlefield + /// and a second `{T}` leaf finds it already tapped, so `activate_mana_ability` + /// errors and the legality simulation answers `false`. Unbounded, the + /// classifier answers `true` for these shapes and + /// `mana_abilities::legality_simulation_is_redundant` would skip the + /// simulation and FLIP the answer — the one direction the fast path may never + /// take. + /// + /// The census runs over the FLATTENED tree, but the nested rows below do NOT + /// prove that: `cost_tree_is_flat` rejects them first, so they would still + /// pass on a one-level census. The flattening itself is pinned directly on the + /// walker, in `leaf_component_census_counts_the_flattened_tree`. + #[test] + fn self_sacrifice_classifier_bounds_leaf_arity() { + use crate::types::ability::TargetFilter; + + let self_sac = || AbilityCost::Sacrifice(SacrificeCost::count(TargetFilter::SelfRef, 1)); + + // Positive controls first: without them every `!` below would pass on a + // classifier that returns `false` unconditionally, and these two shapes + // are precisely what the fast path exists to keep. + assert!( + has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Composite { + costs: vec![AbilityCost::Tap, self_sac()], + })), + "positive control: Treasure's single {{T}} + single self-sacrifice stays eligible" + ); + assert!( + has_unambiguous_self_sacrifice_component(&Some(self_sac())), + "positive control: Gold's bare single self-sacrifice stays eligible" + ); + + // CR 701.21a: the second self-sacrifice leaf has nothing left to move from + // the battlefield, so `sacrifice_permanent` errors and the simulation says + // `false`. + assert!( + !has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Composite { + costs: vec![self_sac(), self_sac()], + })), + "two self-sacrifice leaves are not unambiguously payable" + ); + + // CR 118.3: the second {T} leaf finds the source already tapped, so + // `tap_source` errors and the simulation says `false`. + assert!( + !has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Composite { + costs: vec![AbilityCost::Tap, AbilityCost::Tap, self_sac()], + })), + "two {{T}} leaves are not unambiguously payable" + ); + + // Both hostile axes in one tree. + assert!( + !has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Composite { + costs: vec![AbilityCost::Tap, self_sac(), self_sac()], + })), + "a {{T}} beside two self-sacrifice leaves is not unambiguously payable" + ); + + // Two self-sacrifice leaves again, this time split across a nested + // `Composite`: the payer flattens this to [Tap, Sac, Sac] and pays three + // leaves. Rejected by `cost_tree_is_flat` before the census runs — the + // census's own flattening is pinned directly in + // `leaf_component_census_counts_the_flattened_tree`, since this clause + // would mask an undercount here. + assert!( + !has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Composite { + costs: vec![ + AbilityCost::Composite { + costs: vec![AbilityCost::Tap, self_sac()], + }, + self_sac(), + ], + })), + "a nested Composite's self-sacrifice leaf must be counted: the arity \ + census walks the flattened tree, not one Composite level" + ); + + // NESTING GUARD: flattened this is the payable [Tap, Sac], but the {T} is + // invisible to the one-level `has_tap_component` that gates readiness's + // tapped / summoning-sickness checks (CR 106.12 + CR 302.6), so a TAPPED + // source would clear readiness while the payment path still errors on the + // Tap leaf (CR 118.3). Rejected so this predicate never outruns the + // walkers that gate it. + assert!( + !has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Composite { + costs: vec![AbilityCost::Composite { + costs: vec![AbilityCost::Tap, self_sac()], + }], + })), + "a nested cost tree is rejected: its leaves are invisible to the \ + one-level walkers that gate this activation" + ); + + // Order is NOT an axis of this bound, and the arity census did not make it + // one: sacrificing before tapping still classifies. The payment path + // tolerates it — after the sacrifice the object stays in `state.objects` + // with `zone = Graveyard`, and neither `tap_source` (which reads only + // `tapped`) nor `tap_permanent_for_cost` has a zone guard — so the fast + // path and the simulation agree. Pinned as a no-regression row, not as an + // endorsement of the shape. + assert!( + has_unambiguous_self_sacrifice_component(&Some(AbilityCost::Composite { + costs: vec![self_sac(), AbilityCost::Tap], + })), + "the leaf-arity bound is order-blind: one leaf of each still classifies" + ); + } + + /// `count_leaf_components` must report the multiset the PAYMENT path actually + /// pays: `mana_abilities::append_mana_ability_cost_components` flattens nested + /// `Composite`s before paying one leaf at a time, so a census that walked a + /// single level (as [`cost_has_component`] does) reports `1` where the payer + /// pays `2` — and an arity bound built on it would re-admit the very shapes + /// `self_sacrifice_classifier_bounds_leaf_arity` rejects. + /// + /// Pinned on the building block rather than through the classifier because the + /// classifier rejects nested trees on a separate clause (`cost_tree_is_flat`), + /// which would mask an undercount here. + #[test] + fn leaf_component_census_counts_the_flattened_tree() { + use crate::types::ability::{SacrificeRequirement, TargetFilter}; + + let self_sac = || AbilityCost::Sacrifice(SacrificeCost::count(TargetFilter::SelfRef, 1)); + let is_self_sac = |cost: &AbilityCost| { + matches!( + cost, + AbilityCost::Sacrifice(SacrificeCost { + target: TargetFilter::SelfRef, + requirement: SacrificeRequirement::Count { count: 1 }, + }) + ) + }; + let is_tap = |cost: &AbilityCost| matches!(cost, AbilityCost::Tap); + + // A bare leaf is its own multiset. + assert_eq!( + count_leaf_components(&self_sac(), &is_self_sac), + 1, + "a bare self-sacrifice cost is one self-sacrifice leaf" + ); + + // Flat control: here a one-level census and a flattened census agree, so + // the nested rows below are what discriminate between them. + let flat = AbilityCost::Composite { + costs: vec![AbilityCost::Tap, self_sac()], + }; + assert_eq!( + count_leaf_components(&flat, &is_self_sac), + 1, + "control: Treasure's flat tree has one self-sacrifice leaf" + ); + assert_eq!( + count_leaf_components(&flat, &is_tap), + 1, + "control: Treasure's flat tree has one {{T}} leaf" + ); + + // The discriminating rows: the payer flattens this to [Tap, Sac, Sac]. + let nested = AbilityCost::Composite { + costs: vec![ + AbilityCost::Composite { + costs: vec![AbilityCost::Tap, self_sac()], + }, + self_sac(), + ], + }; + assert_eq!( + count_leaf_components(&nested, &is_self_sac), + 2, + "a nested self-sacrifice leaf must be counted: a one-level census \ + reports 1 here, which is the undercount that re-admits the defect" + ); + assert_eq!( + count_leaf_components(&nested, &is_tap), + 1, + "a nested {{T}} leaf must be counted: a one-level census reports 0 here" + ); + } + #[test] fn life_payment_mana_source_marks_controller_harm() { let mut state = GameState::new_two_player(42); diff --git a/crates/engine/tests/integration/mana_display_self_sacrifice_clone_gate.rs b/crates/engine/tests/integration/mana_display_self_sacrifice_clone_gate.rs index 73337bb478..32fda8a648 100644 --- a/crates/engine/tests/integration/mana_display_self_sacrifice_clone_gate.rs +++ b/crates/engine/tests/integration/mana_display_self_sacrifice_clone_gate.rs @@ -8,13 +8,16 @@ //! `can_activate_mana_ability_by_simulation`, which clones the whole state, so a //! 193-Treasure board took 193 full-state clones per display sweep. //! -//! Both tests run at **N and 2N** so the gate proves *non-scaling* rather than one +//! Every test runs at **N and 2N** so the gate proves *non-scaling* rather than one //! lucky value, and each carries a `has_mana_ability` reach-guard: a `0` clone //! count is vacuous on a board that was never swept. //! //! `mana_display_sweep_still_clones_for_non_self_sacrifice_sources` is the //! positive control — it holds every other axis fixed and changes only the //! `Sacrifice` target, which is the discriminator the fix keys on. +//! `mana_display_sweep_declines_multi_leaf_self_sacrifice_costs` is the ANSWER +//! guard: it varies the cost's leaf ARITY instead, and pins that a shape the fast +//! path cannot decide is still handed to the simulation. //! //! DB-free by construction: `GameState::new_two_player` + `create_object` only, //! never loading `client/public/card-data.json` (mirrors `token_storm_scaling_gate.rs`; @@ -46,14 +49,15 @@ const N: usize = 16; /// crate-private, so an integration test must define its own equivalent — the /// same idiom `token_storm_scaling_gate.rs` uses for a private targeting helper. /// -/// `sacrifice_target` is the only axis that varies between the two tests: -/// `TargetFilter::SelfRef` is the shape the fast path decides, and -/// `TargetFilter::Typed(permanent)` is the shape it must decline. -fn spawn_sacrifice_mana_source( +/// `cost` is the only axis that varies across the tests: `{T}` + a `SelfRef` +/// sacrifice is the shape the fast path decides, `{T}` + a `Typed(permanent)` +/// sacrifice is the shape it must decline on its target, and a multi-leaf tree is +/// the shape it must decline on its arity. +fn spawn_mana_source( state: &mut GameState, card: u64, player: PlayerId, - sacrifice_target: TargetFilter, + cost: AbilityCost, ) -> ObjectId { let id = create_object( state, @@ -89,12 +93,7 @@ fn spawn_sacrifice_mana_source( target: None, }, ) - .cost(AbilityCost::Composite { - costs: vec![ - AbilityCost::Tap, - AbilityCost::Sacrifice(SacrificeCost::count(sacrifice_target, 1)), - ], - }); + .cost(cost); std::sync::Arc::make_mut(&mut state.objects.get_mut(&id).unwrap().abilities).push(def); id } @@ -108,25 +107,32 @@ fn spawn_sacrifice_mana_source( /// board measures an inert fast path. Production always flushes first /// (`public_state::finalize_rules_state` -> `finalize_display_state`), so this /// mirrors production rather than working around the guard. -fn sacrifice_source_board( - count: usize, - sacrifice_target: TargetFilter, -) -> (GameState, Vec) { +fn mana_source_board(count: usize, cost: AbilityCost) -> (GameState, Vec) { let mut state = GameState::new_two_player(42); let ids = (0..count) - .map(|i| { - spawn_sacrifice_mana_source( - &mut state, - 9500 + i as u64, - PlayerId(0), - sacrifice_target.clone(), - ) - }) + .map(|i| spawn_mana_source(&mut state, 9500 + i as u64, PlayerId(0), cost.clone())) .collect(); flush_layers(&mut state); (state, ids) } +/// The Treasure-shaped board (`{T}`, `Sacrifice `) both original tests +/// use, differing only in the sacrifice target. +fn sacrifice_source_board( + count: usize, + sacrifice_target: TargetFilter, +) -> (GameState, Vec) { + mana_source_board( + count, + AbilityCost::Composite { + costs: vec![ + AbilityCost::Tap, + AbilityCost::Sacrifice(SacrificeCost::count(sacrifice_target, 1)), + ], + }, + ) +} + /// Run one board-wide mana-display sweep and return its counters. fn sweep(state: &mut GameState) -> perf_counters::PerfCounterSnapshot { mark_mana_display_dirty(state); @@ -206,3 +212,61 @@ fn mana_display_sweep_still_clones_for_non_self_sacrifice_sources() { ); } } + +/// CR 118.3: the ANSWER guard for the classifier's leaf-arity bound, on the +/// production path and at sweep scale. +/// +/// `Composite[{T}, {T}, Sacrifice this]` is whole-tree choice-free and does carry +/// a single self-sacrifice leaf, so an unbounded classifier accepts it and the +/// fast path answers `true`. The payment path disagrees: it flattens the tree and +/// pays one leaf at a time, so the second `{T}` finds the source already tapped, +/// `tap_source` errors, and the legality simulation answers `false`. Skipping a +/// simulation may only ever save work — never change the answer — so this shape +/// must stay on the simulating path. +/// +/// REVERT-PROBE: drop the `<= 1` tap term from +/// `mana_sources::has_unambiguous_self_sacrifice_component` and every source here +/// flips to `has_mana_ability == true` with `0` legality clones, failing both +/// assertions below. Dropping the `== 1` self-sacrifice term instead is caught by +/// the same shape with two `Sacrifice` leaves, pinned as a unit row in +/// `mana_sources::tests::self_sacrifice_classifier_bounds_leaf_arity`. +#[test] +fn mana_display_sweep_declines_multi_leaf_self_sacrifice_costs() { + for count in [N, 2 * N] { + let (mut state, ids) = mana_source_board( + count, + AbilityCost::Composite { + costs: vec![ + AbilityCost::Tap, + AbilityCost::Tap, + AbilityCost::Sacrifice(SacrificeCost::count(TargetFilter::SelfRef, 1)), + ], + }, + ); + let snap = sweep(&mut state); + + assert_eq!( + snap.mana_display_sweeps, 1, + "count={count}: exactly one board-wide mana sweep" + ); + // The ANSWER: the second {T} leaf cannot be paid (CR 118.3), so the + // ability is not activatable and the sweep must report it as such. + for id in &ids { + assert!( + !state.objects.get(id).unwrap().has_mana_ability, + "count={count}: a two-{{T}} cost is unpayable, so the sweep must \ + answer `false` exactly as the legality simulation does" + ); + } + // Non-vacuity pair for the negative above: one clone per source proves + // readiness PASSED and each source really reached the legality seam, so + // the `false` came from the simulation rather than from an upstream + // short-circuit that would make the assertion meaningless. + assert!( + snap.state_clone_for_legality >= count as u64, + "count={count}: each source must reach the legality simulation \ + (got {} clones) — otherwise the negative assertion above is vacuous", + snap.state_clone_for_legality + ); + } +}