diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index abcc1a83dd..31fa662086 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -28,11 +28,11 @@ use crate::parser::oracle_ir::diagnostic::OracleDiagnostic; use crate::parser::oracle_ir::effect_chain::{ClauseIr, EffectChainIr}; use crate::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, AggregateFunction, AttackScope, - AttackSubject, CastingPermission, Comparator, ConjureSource, ContinuousModification, - ControllerRef, DamageChannel, DamageSource, DelayedTriggerCondition, Duration, Effect, - EffectScope, FilterProp, GameRestriction, LibraryPosition, ManaSpendPermission, - MultiTargetSpec, ObjectScope, PlayerFilter, PreventionAmount, PreventionScope, PtValue, - QuantityExpr, QuantityRef, RestrictionPlayerScope, RoundingMode, + AttackSubject, CastPermissionConstraint, CastingPermission, Comparator, ConjureSource, + ContinuousModification, ControllerRef, DamageChannel, DamageSource, DelayedTriggerCondition, + Duration, Effect, EffectScope, FilterProp, GameRestriction, LibraryPosition, + ManaSpendPermission, MultiTargetSpec, ObjectScope, PlayerFilter, PreventionAmount, + PreventionScope, PtValue, QuantityExpr, QuantityRef, RestrictionPlayerScope, RoundingMode, SpellStackToGraveyardReplacement, StaticCondition, StaticDefinition, SubAbilityLink, TargetChoiceTiming, TargetFilter, TypeFilter, TypedFilter, }; @@ -7397,6 +7397,20 @@ fn apply_where_x_expression(value: PtValue, where_x_expression: Option<&str>) -> }) }) } + // CR 107.3i: an X-bearing P/T slot does not always reach here as + // `PtValue::Variable("X")`. When the clause grammar has already lowered the slot to + // a quantity, the unbound placeholder survives one level down, inside the + // expression tree — Tivash, Gloom Summoner's "create an X/X black Demon creature + // token with flying, where X is the amount of life you gained this turn" lowers to + // `PtValue::Quantity(Ref { Variable("X") })`, not `PtValue::Variable("X")`. + // Matching only the bare-placeholder shape left that X unbound, and the token + // entered as an 0/0 (dying immediately to CR 704.5f) while the face still rendered + // as supported. Recurse so the where-clause owns every X in the slot, at whatever + // depth it sits; `apply_where_x_quantity_expression` is a no-op on a slot that + // holds no X, so a concrete P/T is left untouched. + (PtValue::Quantity(quantity), Some(_)) => { + apply_where_x_quantity_expression(quantity, where_x_expression).map(PtValue::Quantity) + } (value, _) => Some(value), } } @@ -7845,6 +7859,51 @@ fn bind_where_x_quantity( } } +/// An absent slot has no X to bind, so `None` is left alone; a present one routes +/// through the single authority above. +fn bind_where_x_optional_quantity( + slot: Option<&mut QuantityExpr>, + where_x_expression: Option<&str>, + unbound: &mut Option, +) { + if let Some(slot) = slot { + bind_where_x_quantity(slot, where_x_expression, unbound); + } +} + +/// CR 122.1 + CR 107.3i: "enters with X +1/+1 counters on it, where X is …" — the counter +/// COUNT of an enters-with rider is an ordinary where-X quantity slot. Shared by every +/// carrier of the `(CounterType, QuantityExpr)` rider shape (`Token`, `ChangeZone`, +/// `ChangeZoneAll`) so the rider binds identically wherever it appears; G'raha Tia, Scion +/// Reborn ("create a 1/1 … Hero … and put X +1/+1 counters on it") rides this slot, and it +/// sits BESIDE the token's own `count`/`power`/`toughness` rather than inside them. +fn bind_where_x_enter_with_counters( + entries: &mut [(CounterType, QuantityExpr)], + where_x_expression: Option<&str>, + unbound: &mut Option, +) { + for (_, count) in entries.iter_mut() { + bind_where_x_quantity(count, where_x_expression, unbound); + } +} + +/// CR 613.4b: an absent P/T override has no X to bind. A present one routes through the +/// same `PtValue` rewriter the `Token`/`Pump` arms use, so a failed bind is reported as +/// `unbound` exactly as it is for a quantity slot. +fn bind_where_x_optional_pt( + slot: &mut Option, + where_x_expression: Option<&str>, + unbound: &mut Option, +) { + let Some(value) = slot.as_ref() else { + return; + }; + match apply_where_x_expression(value.clone(), where_x_expression) { + Some(bound) => *slot = Some(bound), + None => *unbound = where_x_expression.map(str::to_string), + } +} + pub(super) fn apply_where_x_effect_expression( effect: &mut Effect, where_x_expression: Option<&str>, @@ -7870,23 +7929,131 @@ pub(super) fn apply_where_x_effect_expression( .. } // CR 701.47a: "amass Orcs X, where X is …" (Fall of Cair Andros) — "put N - // +1/+1 counters on that creature", so N is the bound quantity. Without - // this arm the where-X binding was never attempted and the bare - // `Variable("X")` SURVIVED — amassing 0 counters at runtime while the face - // still rendered as fully supported. This match is NOT exhaustive over the - // 64 `QuantityExpr`-carrying variants, and the `_ => {}` below turns every - // unenumerated one into that same silent fabrication rather than a red. + // +1/+1 counters on that creature", so N is the bound quantity. | Effect::Amass { count: amount, .. } - | Effect::Incubate { count: amount } => { + | Effect::Incubate { count: amount } + // The rest of the single-quantity carriers. Each of these owns exactly one + // `QuantityExpr` slot that a "where X is …" clause can define, and each was + // previously falling through the `_ => {}` below — which did not bind, so the bare + // `QuantityRef::Variable("X")` survived and resolved to 0 at runtime (amass 0 / + // surveil 0 / discard 0 / monstrosity 0) while the face still rendered as fully + // supported. The totality guard converted that fabrication into an honest red + // (#5753); binding them here is what turns the representable ones back to green. + | Effect::Adapt { count: amount, .. } + | Effect::AddPendingETBCounters { count: amount, .. } + | Effect::AdditionalPhase { count: amount, .. } + | Effect::AssembleContraptions { count: amount, .. } + | Effect::Bolster { count: amount, .. } + | Effect::ChooseCounterAdjustment { count: amount, .. } + | Effect::Cloak { count: amount, .. } + | Effect::Connive { count: amount, .. } + | Effect::CopyTokenOf { count: amount, .. } + | Effect::Discard { count: amount, .. } + | Effect::EachSourceDealsDamage { amount, .. } + | Effect::Endure { amount, .. } + | Effect::FlipCoins { count: amount, .. } + | Effect::GainEnergy { amount, .. } + | Effect::GivePlayerCounter { count: amount, .. } + | Effect::GrantExtraLoyaltyActivations { amount, .. } + | Effect::Intensify { amount, .. } + | Effect::Manifest { count: amount, .. } + | Effect::Monstrosity { count: amount, .. } + | Effect::PutAtLibraryPosition { count: amount, .. } + | Effect::PutChosenCounter { count: amount, .. } + | Effect::RemoveCounter { count: amount, .. } + | Effect::Renown { count: amount, .. } + | Effect::RevealUntil { count: amount, .. } + | Effect::RollDie { count: amount, .. } + | Effect::Sacrifice { count: amount, .. } + | Effect::SearchOutsideGame { count: amount, .. } + | Effect::SetLifeTotal { amount, .. } + | Effect::SkipNextStep { count: amount, .. } + | Effect::SkipNextTurn { count: amount, .. } + | Effect::Surveil { count: amount, .. } => { bind_where_x_quantity(amount, where_x_expression, &mut unbound_where_x); } + // Multi-slot carriers: a where-X clause defines ONE X, and every slot that + // references it must bind to the same expression (CR 107.3i: X has a single value + // for the whole ability). Binding only the "main" count would leave the siblings + // as bare placeholders resolving to 0. + Effect::ChooseDrawnThisTurnPayOrTopdeck { + count, + life_payment, + .. + } => { + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(life_payment, where_x_expression, &mut unbound_where_x); + } + Effect::CreateTokenCopyFromPool { + mv_bound, count, .. + } => { + bind_where_x_quantity(mv_bound, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + } + Effect::PutSticker { + count, + max_ticket_cost, + .. + } => { + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + bind_where_x_optional_quantity( + max_ticket_cost.as_mut(), + where_x_expression, + &mut unbound_where_x, + ); + } + // The optional-count carriers: same single where-X slot, but absent by default + // (`RevealHand` with no count reveals the whole hand; `MoveCounters` with no count + // moves all of them). `None` has no X to bind and is left alone. + Effect::MoveCounters { count, .. } + | Effect::CastCopyOfCard { count, .. } + | Effect::RevealHand { count, .. } => { + bind_where_x_optional_quantity( + count.as_mut(), + where_x_expression, + &mut unbound_where_x, + ); + } + Effect::ChooseAndSacrificeRest { + total_power_cap, .. + } => { + bind_where_x_optional_quantity( + total_power_cap.as_mut(), + where_x_expression, + &mut unbound_where_x, + ); + } + // CR 122.1: the mass-move counterpart of `ChangeZone`'s enters-with rider. + Effect::ChangeZoneAll { + target, + enter_with_counters, + .. + } => { + bind_where_x_filter(target, where_x_expression, &mut unbound_where_x); + bind_where_x_enter_with_counters( + enter_with_counters, + where_x_expression, + &mut unbound_where_x, + ); + } Effect::Token { count, power, toughness, + enter_with_counters, .. } => { bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + // CR 122.1: the enters-with rider is a THIRD X site on a token, beside the + // token count and its P/T. G'raha Tia, Scion Reborn creates a fixed 1/1 Hero + // and puts X +1/+1 counters on it, so `count`/`power`/`toughness` are all + // concrete and X lives only here — binding the other three left this slot a + // bare placeholder and the Hero entered with 0 counters. + bind_where_x_enter_with_counters( + enter_with_counters, + where_x_expression, + &mut unbound_where_x, + ); match ( apply_where_x_expression(power.clone(), where_x_expression), apply_where_x_expression(toughness.clone(), where_x_expression), @@ -7898,6 +8065,15 @@ pub(super) fn apply_where_x_effect_expression( _ => unbound_where_x = where_x_expression.map(str::to_string), } } + // CR 613.4b (layer 7b): "becomes an X/X creature, where X is …" — an animated + // permanent's base P/T is the same where-X quantity site as a token's, and + // `Animate` is the fourth `PtValue` carrier alongside `Token`/`Pump`/`PumpAll`. + Effect::Animate { + power, toughness, .. + } => { + bind_where_x_optional_pt(power, where_x_expression, &mut unbound_where_x); + bind_where_x_optional_pt(toughness, where_x_expression, &mut unbound_where_x); + } // CR 107.3i + CR 109.4 + CR 109.5: "search/seek for up to X …, where X // is …" binds the search count (Oreskos Explorer). Eldritch Evolution // binds the filter's `Cmc` bound when X appears in the card filter. @@ -7913,22 +8089,65 @@ pub(super) fn apply_where_x_effect_expression( // makes the reanimation target only mana value 0 or less — silently // breaking the trigger's intended behavior. Mirrors the // `SearchLibrary`/`Seek` filter rewrite above. - Effect::ChangeZone { target, .. } => { + Effect::ChangeZone { + target, + enter_with_counters, + conditional_enter_with_counters, + .. + } => { bind_where_x_filter(target, where_x_expression, &mut unbound_where_x); + // CR 122.1: same enters-with rider as `Token`/`ChangeZoneAll` — the moved + // permanent's counter count is a where-X site of its own. + bind_where_x_enter_with_counters( + enter_with_counters, + where_x_expression, + &mut unbound_where_x, + ); + for (_, _, count) in conditional_enter_with_counters.iter_mut() { + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + } } - Effect::Destroy { target, .. } - | Effect::Bounce { target, .. } - | Effect::BounceAll { target, .. } - | Effect::CastFromZone { target, .. } => { + Effect::Destroy { target, .. } | Effect::Bounce { target, .. } => { bind_where_x_filter(target, where_x_expression, &mut unbound_where_x); } + // `BounceAll` carries an optional count ("return X target creatures …") beside its + // filter; the filter-only arm left that count a bare placeholder. + Effect::BounceAll { target, count, .. } => { + bind_where_x_filter(target, where_x_expression, &mut unbound_where_x); + bind_where_x_optional_quantity( + count.as_mut(), + where_x_expression, + &mut unbound_where_x, + ); + } + // CR 601.2e: a cast permission may be BOUNDED by X ("you may cast a spell with mana + // value less than X from among them, where X is that spell's mana value" — Kiora, + // Sovereign of the Deep). The bound lives in the permission constraint, not in the + // target filter, so the filter-only arm never reached it and the constraint kept a + // bare `Variable("X")` — permitting only mana value < 0, i.e. nothing at all. + Effect::CastFromZone { + target, constraint, .. + } => { + bind_where_x_filter(target, where_x_expression, &mut unbound_where_x); + if let Some(CastPermissionConstraint::ManaValue { value, .. }) = constraint.as_mut() { + bind_where_x_quantity(value, where_x_expression, &mut unbound_where_x); + } + } Effect::Dig { count, + keep_count_expr, player, filter, .. } => { bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + // "look at the top N …, keep X of them" — the KEPT count is a second, distinct + // quantity slot that the original arm did not bind. + bind_where_x_optional_quantity( + keep_count_expr.as_mut(), + where_x_expression, + &mut unbound_where_x, + ); bind_where_x_filter(player, where_x_expression, &mut unbound_where_x); bind_where_x_filter(filter, where_x_expression, &mut unbound_where_x); } @@ -8038,13 +8257,20 @@ pub(super) fn apply_where_x_effect_expression( } } } + // Every remaining variant carries no `QuantityExpr` and no `PtValue`, so it has no + // X slot to bind. The arms above now cover all 62 `QuantityExpr` carriers and all 4 + // `PtValue` carriers; this wildcard is reached only by variants with nothing to + // rewrite. It is NOT an escape hatch — the totality guard below still asserts the + // post-condition, so a FUTURE variant that adds a quantity slot without an arm here + // reds honestly instead of silently fabricating. _ => {} } // CR 107.3c — TOTALITY GUARD. The match above rewrites the quantity slots of the - // `Effect` variants it enumerates. It is NOT exhaustive: of the 64 variants that - // carry a `QuantityExpr`, 42 fall through the `_ => {}` above (task #95 binds the - // representable ones). Without this guard the wildcard's failure mode is a - // FABRICATION rather than a red — the effect keeps its bare `Variable("X")`, which + // `Effect` variants it enumerates. Enumeration is necessary but not sufficient: a + // variant can be enumerated and still leave an X unbound (a slot the arm forgets, or an + // expression `parse_where_x_quantity_expression` cannot represent). Without this guard + // the failure mode is a FABRICATION rather than a red — the effect keeps its bare + // `Variable("X")`, which // resolves to 0 at runtime (amass 0 / surveil 0 / discard 0) while the face still // renders as fully supported. That lie is invisible BOTH to a red-count ledger // (there is no `Unimplemented` node to count) and to the zero-raw-text invariant diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 34454ed531..18a6f58e76 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -431,12 +431,28 @@ fn rewrite_cost_x_in_effect(effect: &mut crate::types::ability::Effect) { | Effect::Mill { count: amount, .. } | Effect::PutCounter { count: amount, .. } | Effect::PutCounterAll { count: amount, .. } - | Effect::Token { count: amount, .. } | Effect::Dig { count: amount, .. } | Effect::DamageAll { amount, .. } | Effect::DamageEachPlayer { amount, .. } => { rewrite_variable_x_to_cost_x_paid(amount); } + // CR 107.3c: a token's P/T is an X site just like `Pump`'s. `Token` was rewritten + // for its `count` only, so an X/X token minted by an X-cost ability kept a bare + // `Variable("X")` in its power/toughness — which resolves to 0, so Shark Typhoon's + // cycling trigger created an **0/0 Shark** that died immediately to state-based + // actions (CR 704.5f) while the card rendered as fully supported. The P/T rewriter + // already handled every shape needed here (including a `PtValue::Quantity` wrapping + // the placeholder); `Token` simply never reached it. + Effect::Token { + count, + power, + toughness, + .. + } => { + rewrite_variable_x_to_cost_x_paid(count); + rewrite_trigger_pt_variable_x(power); + rewrite_trigger_pt_variable_x(toughness); + } Effect::Pump { power, toughness, .. } diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 37ea7ea188..247640f7fb 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -948,6 +948,7 @@ mod vanille_meld_optional_cost; mod vannifar_cloak_from_hand; mod veteran_bodyguard_tap_redirect; mod weeping_angel_combat_prevention; +mod where_x_coverage_runtime; mod where_x_quantity_channel_binds; mod where_x_totality_guard; mod winding_way_reveal_partition_2931; diff --git a/crates/engine/tests/integration/where_x_coverage_runtime.rs b/crates/engine/tests/integration/where_x_coverage_runtime.rs new file mode 100644 index 0000000000..c2dfd7f1cc --- /dev/null +++ b/crates/engine/tests/integration/where_x_coverage_runtime.rs @@ -0,0 +1,240 @@ +//! CR 107.3c — RUNTIME witnesses for the where-X coverage enumeration (task #95). +//! +//! The parse-level pins in `where_x_totality_guard` prove the gap node is gone. They do +//! NOT prove the bound quantity resolves to the right NUMBER — a slot could bind to a +//! plausible-looking `QuantityRef` that reads the wrong state and still show up green on +//! every ledger. These witnesses close that hole: each parses the card's real Oracle text +//! through the production parser and hands the resulting quantity to the LIVE resolver +//! against a state where the referenced fact is actually true. +//! +//! The failure they discriminate is specifically ZERO. Before #95 these slots kept a bare +//! `QuantityRef::Variable { name: "X" }`, which resolves to 0 — an 0/0 token, a +//! monstrosity that adds no counters. So each assertion is written against a state whose +//! correct answer is a NON-zero number that is also not the fixed value of any sibling +//! slot, so neither "still unbound" nor "bound to the wrong reference" can pass. +//! +//! Oracle text is verbatim from MTGJSON. + +use engine::game::quantity::resolve_quantity; +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::parser::parse_oracle_text; +use engine::types::ability::{Effect, PtValue, QuantityExpr}; +use engine::types::phase::Phase; + +/// CR 613.4b — Tivash, Gloom Summoner. "create an X/X black Demon creature token with +/// flying, where X is the amount of life you gained this turn." +/// +/// The token's P/T reached the where-X rewriter as `PtValue::Quantity(Ref { Variable("X") })` +/// rather than `PtValue::Variable("X")`, so the old matcher passed it through untouched and +/// the Demon entered as an **0/0** — dead on arrival to state-based actions (CR 704.5f) +/// while the card rendered as fully supported. +/// +/// The witness gains 5 life and asserts the token's power resolves to 5. A still-unbound +/// slot resolves to 0; the token's `count` slot is a fixed 1, so a mis-bind onto the wrong +/// slot would read 1. Only a correct bind reads 5. +#[test] +fn tivash_x_x_demon_token_reads_life_gained_not_zero() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let source = scenario + .add_creature(P0, "Tivash, Gloom Summoner", 3, 3) + .id(); + let mut runner = scenario.build(); + + // The trigger's intervening-if is "if you gained life this turn"; the quantity reads + // the same tracked total. + runner.state_mut().players[0].life_gained_this_turn = 5; + + let power = tivash_token_power(); + let resolved = resolve_quantity(runner.state(), &power, P0, source); + + assert_eq!( + resolved, 5, + "CR 613.4b: the Demon's X/X must read the life gained this turn (5). 0 means the P/T slot \ + is still an unbound Variable(\"X\") and the token enters 0/0, dying immediately to SBAs \ + (CR 704.5f); 1 means it bound to the token's `count` slot instead. Got {resolved} from \ + {power:?}" + ); +} + +/// Pull the X/X token's power out of Tivash's real Oracle text. +/// +/// Reaching through the sub-ability ("If you do, create …") is deliberate: that is exactly +/// where the unbound placeholder used to survive, so a helper that could not find the token +/// there would make the witness vacuous. +fn tivash_token_power() -> QuantityExpr { + let parsed = parse_oracle_text( + "Lifelink\nAt the beginning of your end step, if you gained life this turn, you may pay X \ + life, where X is the amount of life you gained this turn. If you do, create an X/X black \ + Demon creature token with flying.", + "Tivash, Gloom Summoner", + &[], + &["Creature".to_string()], + &[], + ); + let trigger = parsed + .triggers + .first() + .expect("Tivash's end-step trigger must parse"); + let sub = trigger + .execute + .as_ref() + .expect("the trigger must lower an execute body") + .sub_ability + .as_ref() + .expect("the 'If you do, create …' continuation must lower to a sub-ability"); + match &*sub.effect { + Effect::Token { power, .. } => match power { + PtValue::Quantity(quantity) => quantity.clone(), + other => panic!( + "the Demon's power must lower to a dynamic quantity, not {other:?} — if this is \ + still PtValue::Variable(\"X\") the where-X bind never ran" + ), + }, + other => panic!("expected the sub-ability to create a Token, got {other:?}"), + } +} + +/// CR 122.1 — Maester Seymour. "Monstrosity X, where X is the number of counters among +/// creatures you control." +/// +/// `Effect::Monstrosity` was one of the unenumerated `QuantityExpr` carriers, so its count +/// kept a bare `Variable("X")` and the ability became monstrous while adding **zero** +/// +1/+1 counters — a full no-op under a green badge. +/// +/// The board is stacked so the right answer (7) is distinguishable from every wrong one: +/// counters are split 4 + 3 across two of the controller's creatures (so a bind that reads +/// only one object would read 4 or 3), and an opponent's creature carries 2 more (so a bind +/// that ignores the "you control" filter would read 9). +#[test] +fn maester_seymour_monstrosity_counts_counters_among_your_creatures_only() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let source = scenario + .add_creature(P0, "Maester Seymour", 4, 4) + .with_plus_counters(4) + .id(); + scenario + .add_creature(P0, "Bear", 2, 2) + .with_plus_counters(3); + // Opponent's counters must NOT be counted — this is the filter's discriminator. + scenario + .add_creature(P1, "Rival Bear", 2, 2) + .with_plus_counters(2); + let runner = scenario.build(); + + let count = monstrosity_count(); + let resolved = resolve_quantity(runner.state(), &count, P0, source); + + assert_eq!( + resolved, 7, + "CR 122.1: X must count the counters among creatures YOU control (4 + 3 = 7). 0 means the \ + Monstrosity count is still an unbound Variable(\"X\") and the creature becomes monstrous \ + with no counters at all; 9 means the 'you control' filter was dropped and the opponent's \ + 2 were counted. Got {resolved} from {count:?}" + ); +} + +/// Pull Monstrosity's bound count out of Maester Seymour's real activated ability. +fn monstrosity_count() -> QuantityExpr { + let parsed = parse_oracle_text( + "{3}{G}{G}: Monstrosity X, where X is the number of counters among creatures you control.", + "Maester Seymour", + &[], + &["Creature".to_string()], + &[], + ); + let def = parsed + .abilities + .first() + .expect("Maester Seymour's activated ability must parse"); + match &*def.effect { + Effect::Monstrosity { count, .. } => count.clone(), + other => panic!( + "expected Monstrosity, got {other:?} — a where_x_binding gap here means the count \ + never bound and this witness would be testing nothing" + ), + } +} + +// --------------------------------------------------------------------------- +// THE COST-X SIBLING (#96) — the X/X token minted by an X-cost spell's own ETB. +// +// This is NOT the where-X walk. These cards carry no "where X is …" tail at all; their X +// is the X PAID TO CAST THE SPELL, and the rewrite that owns it is +// `rewrite_cost_x_in_effect` (`Variable("X")` -> `CostXPaid`). +// +// That walk rewrote `Effect::Token`'s `count` but never its `power`/`toughness`, so an X/X +// token kept a bare `Variable("X")` in its P/T — which resolves to 0. Arboreal Alliance's +// Treefolk entered as an **0/0** and died instantly to state-based actions (CR 704.5f) +// while the card rendered as fully supported. +// +// SCOPE, stated so this is not over-read: this closes the Token P/T slot, NOT #96's class. +// `rewrite_cost_x_in_effect` still has a `_ => {}` wildcard, and its CALLER is gated by +// `trigger_should_rewrite_cost_x` to ChangesZone->Battlefield self-ETB triggers only — so a +// cost-X trigger that is not a self-ETB (Shark Typhoon's CYCLING trigger) never reaches +// this walk at all and is still an 0/0 today. #96 stays open. See the report for why that +// one is a two-part engine change rather than a parser arm. +// --------------------------------------------------------------------------- + +/// CR 107.3m + CR 704.5f — Arboreal Alliance. "When this enchantment enters, create an X/X +/// green Treefolk creature token." +/// +/// The witness casts for X=4 and asserts the token's power resolves to 4. `CostXPaid` reads +/// `cost_x_paid` off the source object (stamped by `finalize_cast`, CR 107.3m); an +/// unrewritten `Variable("X")` reads nothing and falls through to **0**. So 0 is precisely +/// the pre-fix bug, and the assertion cannot pass while the slot is a bare placeholder. +#[test] +fn cost_x_token_pt_is_x_by_x_not_zero_by_zero() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let source = scenario.add_creature(P0, "Arboreal Alliance", 1, 1).id(); + let mut runner = scenario.build(); + + // CR 107.3m: the {X} paid to cast the spell, stashed on the permanent. + runner + .state_mut() + .objects + .get_mut(&source) + .expect("source object") + .cost_x_paid = Some(4); + + let power = cost_x_token_power(); + let resolved = resolve_quantity(runner.state(), &power, P0, source); + + assert_eq!( + resolved, 4, + "CR 107.3m: the Treefolk must be a 4/4 — X is the X paid to cast the spell. 0 means the \ + token's P/T is still a bare Variable(\"X\") that rewrite_cost_x_in_effect never rewrote \ + (it rewrote only the token's `count`), so the Treefolk enters 0/0 and dies instantly to \ + SBAs (CR 704.5f) while the card reads as fully supported. Got {resolved} from {power:?}" + ); +} + +/// Pull the ETB token's power out of Arboreal Alliance's real Oracle text. +fn cost_x_token_power() -> QuantityExpr { + let parsed = parse_oracle_text( + "When this enchantment enters, create an X/X green Treefolk creature token.\nWhenever you \ + attack with one or more Elves, populate.", + "Arboreal Alliance", + &[], + &["Enchantment".to_string()], + &[], + ); + let power = parsed + .triggers + .iter() + .find_map(|trigger| match &*trigger.execute.as_ref()?.effect { + Effect::Token { power, .. } => Some(power.clone()), + _ => None, + }) + .expect("Arboreal Alliance must lower a token-creating ETB trigger"); + + match power { + PtValue::Quantity(quantity) => quantity, + other => panic!( + "the Treefolk's power must lower to a dynamic quantity, not {other:?} — a bare \ + PtValue::Variable(\"X\") here means the cost-X P/T rewrite never ran" + ), + } +} diff --git a/crates/engine/tests/integration/where_x_totality_guard.rs b/crates/engine/tests/integration/where_x_totality_guard.rs index 18e68cd68d..cc1ec3bd75 100644 --- a/crates/engine/tests/integration/where_x_totality_guard.rs +++ b/crates/engine/tests/integration/where_x_totality_guard.rs @@ -1,9 +1,15 @@ //! CR 107.3c — the where-X lowering pass must assert its OWN post-condition. //! //! `apply_where_x_effect_expression` rewrites the quantity slots of the `Effect` -//! variants it enumerates. That match is NOT exhaustive: of the 64 variants carrying a -//! `QuantityExpr`, only 22 are enumerated — the other 42 fall through a `_ => {}` -//! (task #95 binds the representable ones). +//! variants it enumerates. It originally enumerated 23 of the 62 `QuantityExpr` carriers; +//! the other 39 fell through a `_ => {}`. Task #95 enumerated them all (plus the 4 +//! `PtValue` carriers), so today the wildcard is reached only by variants that carry no X +//! slot at all. +//! +//! Enumeration is necessary but NOT sufficient for a green face: a variant can be +//! enumerated and still fail to bind, because `parse_where_x_quantity_expression` cannot +//! represent every "where X is …" definition (see the unrepresentable-expression test). +//! The guard is what keeps that failure honest. //! //! Before the totality guard, the wildcard's failure mode was a FABRICATION, not a red: //! the effect kept its bare `QuantityRef::Variable { name: "X" }`, which resolves to 0 @@ -150,18 +156,24 @@ fn constraint_tail_placeholder_bind_survives_the_totality_guard() { } // --------------------------------------------------------------------------- -// THE GUARD ITSELF — an unenumerated variant must RED, never fabricate. +// THE GUARD ITSELF — an X that cannot be bound must RED, never fabricate. // --------------------------------------------------------------------------- -/// CR 107.3c — `Effect::Surveil` is one of the 42 `QuantityExpr` carriers the pass does -/// NOT enumerate. Its where-X expression is perfectly representable, but until #95 binds -/// it the pass cannot rewrite the slot — so the totality guard must turn it into an -/// honest gap rather than leave `surveil Variable("X")`, which surveils 0 while reading -/// as fully supported. +/// CR 107.3c — the guard's positive witness: a where-X definition the parser cannot +/// REPRESENT must red, never fabricate. +/// +/// HISTORY, so this pin is not misread: it originally asserted that `Effect::Surveil` was +/// one of the unenumerated `QuantityExpr` carriers, and that the red came from the +/// `_ => {}` wildcard. Task #95 enumerated Surveil, so that premise is now false — yet the +/// face still reds, for a DIFFERENT reason: "the number of opponents being attacked" has no +/// `QuantityRef`, so `parse_where_x_quantity_expression` returns `None` and the bind fails. /// -/// This is the positive witness for the guard: without it, this face is a lying green. +/// The invariant being pinned is therefore "an unrepresentable where-X reds", NOT "an +/// unenumerated variant reds" — enumeration is necessary but not sufficient for green, and +/// pinning the transient marker rather than the invariant would have let this test keep +/// passing while quietly meaning something else. #[test] -fn unenumerated_effect_variant_reds_instead_of_fabricating() { +fn unrepresentable_where_x_expression_reds_instead_of_fabricating() { let parsed = parse( "Flying\nWhenever you attack, surveil X, where X is the number of opponents being \ attacked.", @@ -171,11 +183,11 @@ fn unenumerated_effect_variant_reds_instead_of_fabricating() { assert!( parsed.has_where_x_gap, - "CR 107.3c: Surveil is NOT among the effect variants apply_where_x_effect_expression \ - enumerates, so the where-X slot was never rewritten. The totality guard must report \ - the gap. Leaving Variable(\"X\") here surveils 0 at runtime while rendering as fully \ - supported — a fabrication invisible to both the red-count ledger and the \ - zero-raw-text invariant. Tree: {:?}", + "CR 107.3c: 'the number of opponents being attacked' is not a representable quantity, \ + so the where-X slot cannot be bound and the totality guard must report the gap. \ + Leaving Variable(\"X\") here surveils 0 at runtime while rendering as fully supported \ + — a fabrication invisible to both the red-count ledger and the zero-raw-text \ + invariant. Tree: {:?}", parsed.tree ); @@ -185,6 +197,29 @@ fn unenumerated_effect_variant_reds_instead_of_fabricating() { "the bare Variable(\"X\") must be REPLACED by the gap node, not left beside it. Tree: {:?}", parsed.tree ); + + // NON-VACUITY: the red must be EXPRESSION-specific, not the guard redding every where-X + // face it sees. The same effect with a representable expression must come out green. + // + // Scope of what this proves, stated precisely so it is not over-read: it discriminates a + // blanket-red guard, and nothing more. It does NOT prove the effect-level enumeration is + // load-bearing — the clause-level quantity parser already resolves this simple shape + // before the where-X rewriter ever runs, so this assertion holds with or without + // Surveil's arm. (Measured: on the pre-#95 parser this same text already bound to + // ObjectCount.) The proof that enumeration is load-bearing is the COVERAGE section + // below, whose faces were each watched failing on the pre-#95 parser. + let representable = parse( + "Whenever you attack, surveil X, where X is the number of creatures you control.", + "~", + &["Creature"], + ); + assert!( + !representable.has_where_x_gap, + "control: a REPRESENTABLE where-X expression on the same effect must bind green. A gap \ + here means the guard is redding every where-X face wholesale, so the red above proves \ + nothing about expression coverage. Tree: {:?}", + representable.tree + ); } /// NEGATIVE CONTROL for the guard — an ENUMERATED variant whose expression IS @@ -222,3 +257,160 @@ fn enumerated_bindable_where_x_still_binds_green() { abilities.abilities ); } + +// --------------------------------------------------------------------------- +// COVERAGE (task #95) — the representable where-X definitions must BIND to green. +// +// The guard (above) proves an unbindable X reds. These prove the complement: that the +// pass actually binds what it CAN represent. Without them, a pass that redded every +// where-X face in the pool would satisfy every assertion in this file. +// +// Each face below was a LYING GREEN before #5753 (bare Variable("X") resolving to 0 under +// a green badge), an honest red after it, and is bound for real here. Oracle text is +// verbatim from MTGJSON. +// --------------------------------------------------------------------------- + +/// The newly-enumerated single-quantity carriers. Each owns one `QuantityExpr` slot that +/// previously fell through the `_ => {}` wildcard unbound. +/// +/// Table-driven because the point is the CLASS (every quantity-carrying effect binds its +/// slot), not any one card — a per-card test here would pass while the next card with the +/// same shape silently fabricated. +#[test] +fn newly_enumerated_quantity_carriers_bind_their_where_x_slot() { + // (card, types, oracle) — all verbatim MTGJSON text. + let cases: [(&str, &[&str], &str); 5] = [ + ( + "Mind Burst", + &["Sorcery"], + "Target player discards X cards, where X is one plus the number of cards named \ + Mind Burst in all graveyards.", + ), + ( + "Arcane Omens", + &["Instant"], + "Converge — Target player discards X cards, where X is the number of colors of \ + mana spent to cast this spell.", + ), + ( + "Maester Seymour", + &["Creature"], + "{3}{G}{G}: Monstrosity X, where X is the number of counters among creatures you \ + control.", + ), + ( + "Clay Golem", + &["Artifact", "Creature"], + "{6}, Roll a d8: Monstrosity X, where X is the result.", + ), + ( + "Mycelic Ballad", + &["Enchantment"], + "Each player sacrifices X creatures and you gain X life, where X is this spell's \ + intensity.", + ), + ]; + + for (name, types, oracle) in cases { + let parsed = parse(oracle, name, types); + assert!( + !parsed.has_where_x_gap, + "{name}: this where-X expression IS representable, so the effect's quantity slot must \ + BIND. A where_x_binding gap means its Effect variant is still falling through the \ + wildcard unbound (discard 0 / monstrosity 0 / sacrifice 0 at runtime). Tree: {:?}", + parsed.tree + ); + assert!( + !retains_variable_x(&parsed.tree), + "{name}: the where-X clause was consumed but a bare Variable(\"X\") SURVIVED in the \ + tree — that placeholder resolves to 0 at runtime while the face renders as fully \ + supported. Binding must replace it, not sit beside it. Tree: {:?}", + parsed.tree + ); + } +} + +/// CR 122.1 — the enters-with-counters rider is its own where-X slot, BESIDE the token's +/// count and P/T. G'raha Tia creates a *fixed* 1/1 Hero, so `count`, `power` and +/// `toughness` are all concrete and X lives only in the rider: an arm that bound the other +/// three still left the Hero entering with 0 counters. +#[test] +fn token_enter_with_counters_rider_binds_where_x() { + let parsed = parse( + "Lifelink\nThrow Wide the Gates — Whenever you cast a noncreature spell, you may pay X \ + life, where X is that spell's mana value. If you do, create a 1/1 colorless Hero \ + creature token and put X +1/+1 counters on it. Do this only once each turn.", + "G'raha Tia, Scion Reborn", + &["Creature"], + ); + + assert!( + !parsed.has_where_x_gap, + "CR 122.1: 'that spell's mana value' is representable, so the enters-with rider's counter \ + COUNT must bind. Tree: {:?}", + parsed.tree + ); + assert!( + !retains_variable_x(&parsed.tree), + "the rider's count kept a bare Variable(\"X\") — the Hero enters with 0 counters while the \ + face reads as supported. Tree: {:?}", + parsed.tree + ); +} + +/// CR 601.2e — a cast permission can be BOUNDED by X. Kiora's "mana value less than X" lives +/// in the `CastFromZone` permission constraint, not in the target filter, so a filter-only +/// bind left it a bare placeholder: mana value < 0, which permits nothing at all. +#[test] +fn cast_permission_constraint_binds_where_x() { + let parsed = parse( + "Vigilance, ward {3}\nWhenever you cast a Kraken, Leviathan, Octopus, or Serpent spell \ + from your hand, look at the top X cards of your library, where X is that spell's mana \ + value. You may cast a spell with mana value less than X from among them without paying \ + its mana cost. Put the rest on the bottom of your library in a random order.", + "Kiora, Sovereign of the Deep", + &["Creature"], + ); + + assert!( + !parsed.has_where_x_gap, + "CR 601.2e: the cast-permission mana-value bound is a where-X slot and must bind. \ + Tree: {:?}", + parsed.tree + ); + assert!( + !retains_variable_x(&parsed.tree), + "the permission constraint kept a bare Variable(\"X\") — it would permit only mana value \ + < 0, i.e. nothing, while the face reads as supported. Tree: {:?}", + parsed.tree + ); +} + +/// CR 613.4b — an X-bearing P/T does not always reach the rewriter as `PtValue::Variable("X")`. +/// When the clause grammar has already lowered the slot to a quantity, X survives one level +/// down inside `PtValue::Quantity(...)`. Tivash's X/X Demon rides exactly that shape, and +/// matching only the bare-placeholder form left it entering as an 0/0 — dead on arrival to +/// state-based actions (CR 704.5f) while the face rendered as supported. +#[test] +fn pt_value_quantity_slot_binds_where_x() { + let parsed = parse( + "Lifelink\nAt the beginning of your end step, if you gained life this turn, you may pay \ + X life, where X is the amount of life you gained this turn. If you do, create an X/X \ + black Demon creature token with flying.", + "Tivash, Gloom Summoner", + &["Creature"], + ); + + assert!( + !parsed.has_where_x_gap, + "CR 613.4: 'the amount of life you gained this turn' is representable, so the token's \ + X/X must bind. Tree: {:?}", + parsed.tree + ); + assert!( + !retains_variable_x(&parsed.tree), + "the token's power/toughness kept a bare Variable(\"X\") — it enters 0/0 and dies to SBAs \ + (CR 704.5f) while the face reads as supported. Tree: {:?}", + parsed.tree + ); +}