From 3dedb2f35e6d5b7ea5c1f98a8aa79afd9b90ce30 Mon Sep 17 00:00:00 2001 From: Jacob Woodson Date: Sun, 2 Aug 2026 18:19:40 -0500 Subject: [PATCH 1/7] Partial: Ultimate Nullification --- .../src/parser/oracle_effect/imperative.rs | 181 ++++++++++++--- crates/engine/src/parser/oracle_effect/mod.rs | 23 ++ .../engine/src/parser/oracle_effect/tests.rs | 163 ++++++++++++++ crates/engine/src/parser/oracle_target.rs | 2 +- crates/engine/tests/integration/main.rs | 1 + .../integration/ultimate_nullification.rs | 212 ++++++++++++++++++ 6 files changed, 550 insertions(+), 32 deletions(-) create mode 100644 crates/engine/tests/integration/ultimate_nullification.rs diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index a40617e3ee..99648cb77b 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -49,15 +49,16 @@ use crate::types::statics::{ActivationExemption, CostModifyMode, StaticMode}; use crate::types::zones::Zone; use super::super::oracle_target::{ - parse_anaphoric_target_ref, parse_event_context_ref, parse_fight_target, parse_mass_type_union, - parse_target, parse_target_with_ctx, parse_target_with_syntax, parse_type_phrase, - parse_type_phrase_with_ctx, parse_word_bounded, resolve_pronoun_target, - resolve_singular_exiled_card_target, TargetSyntax, + match_mass_union_separator, parse_anaphoric_target_ref, parse_event_context_ref, + parse_fight_target, parse_mass_type_union, parse_target, parse_target_with_ctx, + parse_target_with_syntax, parse_type_phrase, parse_type_phrase_with_ctx, parse_word_bounded, + resolve_pronoun_target, resolve_singular_exiled_card_target, starts_with_type_word, + TargetSyntax, }; use super::super::oracle_util::{ - contains_possessive, contains_self_or_object_pronoun, parse_count_expr, parse_mana_symbols, - parse_ordinal, parse_rounding_suffix_only, rewrite_quantity_expr_rounding, split_around, - starts_with_possessive, TextPair, + contains_possessive, contains_self_or_object_pronoun, merge_or_filters, parse_count_expr, + parse_mana_symbols, parse_ordinal, parse_rounding_suffix_only, rewrite_quantity_expr_rounding, + split_around, starts_with_possessive, TextPair, }; /// CR 611.2 + CR 601.2f + CR 118.7: Parse the transient (this-turn) @@ -2625,6 +2626,32 @@ pub(super) fn try_parse_multi_zone_same_name_exile( run(lower).ok().map(|(_, result)| result) } +/// CR 400.1 + CR 401.1 + CR 402.1 + CR 404.1 + CR 406.2: map a single zone word to +/// its [`Zone`] — the one lexical zone-word→`Zone` mapping shared by every +/// zone-union recognizer in this module (`try_parse_multi_zone_player_exile`, +/// `parse_trailing_zone_union`, and the `Choose`-a-zone parsers). +/// +/// The three per-player zones (CR 401.1 library, CR 402.1 hand, CR 404.1 +/// graveyard) each also match their plural form, which — with no possessive — +/// denotes "every player's ``" (each player owns their own such zone, +/// CR 400.1) in a whole-zone union. The plural arm precedes the singular so the +/// longer form wins. Exile (CR 406.2) is a single zone shared by all players +/// (CR 400.1), so it has no per-player instance and no plural form. +fn parse_zone_word(input: &str) -> nom::IResult<&str, Zone, OracleError<'_>> { + type E<'a> = OracleError<'a>; + + alt(( + value( + Zone::Graveyard, + alt((tag::<_, _, E>("graveyards"), tag("graveyard"))), + ), + value(Zone::Hand, alt((tag("hands"), tag("hand")))), + value(Zone::Library, alt((tag("libraries"), tag("library")))), + value(Zone::Exile, tag("exile")), + )) + .parse(input) +} + /// Parse output of the multi-zone player-exile recognizer: remaining input paired /// with the owner axis and the origin-zone union. Named so the inner `nom` /// combinator signature stays under `clippy::type_complexity`. @@ -2646,14 +2673,6 @@ type MultiZonePlayerExileParse<'a> = (&'a str, (ControllerRef, Vec)); pub(super) fn try_parse_multi_zone_player_exile( rest_lower: &str, ) -> Option<(ControllerRef, Vec)> { - fn zone_word(input: &str) -> Result<(&str, Zone), nom::Err>> { - alt(( - value(Zone::Graveyard, tag::<_, _, OracleError<'_>>("graveyard")), - value(Zone::Hand, tag("hand")), - value(Zone::Library, tag("library")), - )) - .parse(input) - } fn run(input: &str) -> Result, nom::Err>> { let (input, _) = alt(( tag::<_, _, OracleError<'_>>("cards from "), @@ -2677,7 +2696,7 @@ pub(super) fn try_parse_multi_zone_player_exile( value(ControllerRef::You, tag("your ")), )) .parse(input)?; - let (mut input, first) = zone_word(input)?; + let (mut input, first) = parse_zone_word(input)?; let mut zones = vec![first]; // Additional zones joined by " and " / ", and " / ", " (oxford comma). loop { @@ -2689,7 +2708,7 @@ pub(super) fn try_parse_multi_zone_player_exile( .parse(input) else { break; }; - let Ok((after_zone, zone)) = zone_word(after_sep) else { + let Ok((after_zone, zone)) = parse_zone_word(after_sep) else { break; }; if !zones.contains(&zone) { @@ -2712,6 +2731,101 @@ pub(super) fn try_parse_multi_zone_player_exile( Some((owner, zones)) } +/// CR 406.2 + CR 404.1 + CR 108.2 + CR 608.2f: "exile all `` and +/// ``" — a heterogeneous mass exile whose operand unions a battlefield +/// permanent population with one or more whole-zone populations (every card in +/// every player's graveyard / hand / library, keyed by owner per CR 400.3). +/// Ultimate Nullification ("Exile all creatures and graveyards") is the +/// exemplar. Generalizes to any permutation ("… creatures and artifacts and +/// graveyards", "… creatures and graveyards and hands"). +/// +/// The permanent legs are parsed by [`parse_mass_type_union`] (CR 205.2a + +/// CR 205.3a — a card-type/subtype union) and scoped to the battlefield via +/// `InZone`; the trailing zone legs become an all-cards (`TypeFilter::Card`), +/// all-owners (`controller: None`) leg carrying the zone union on `InAnyZone`. +/// The two are merged into one `Or`, so the resolving `ChangeZoneAll` scans +/// `extract_zones()` = Battlefield ∪ zone-union and moves every match to exile +/// simultaneously (CR 608.2f) — one instruction, never a battlefield wipe plus +/// an orphaned zone conjunct. +/// +/// Declines (`None`) unless a permanent leg is followed by at least one zone leg +/// AND the operand is fully consumed (only trailing punctuation may remain), so +/// pure type unions ("all creatures and artifacts") keep the existing type-union +/// path and no trailing fragment is ever silently dropped. +pub(super) fn try_parse_mass_exile_permanents_and_zones( + rest: &str, + rest_lower: &str, + ctx: &mut ParseContext, +) -> Option { + // The operand must LEAD with a permanent-type leg; pure-zone and zone-first + // forms are declined so existing behavior is untouched. Uses the shared + // type-word predicate combinator, never a raw-text dispatch. + if !starts_with_type_word(rest_lower) { + return None; + } + // CR 205.2a + CR 205.3a: consume the leading permanent-type union + // ("creatures", "creatures and artifacts", …); `rem` is the untyped tail + // (e.g. " and graveyards"). + let (perm_filter, rem) = parse_mass_type_union(rest, ctx); + // CR 404.1 + CR 108.2: the trailing whole-zone union — declines if absent. + let zones = parse_trailing_zone_union(&rem.to_lowercase())?; + // CR 109.2: a bare card-type description ("creatures", no zone word / "card") + // means permanents of that type on the battlefield — so scope the permanent + // legs to the battlefield. This also makes `ChangeZoneAll::extract_zones` + // yield Battlefield ∪ zone-union; without it the zone leg would shadow the + // scan and battlefield permanents would never be collected. + let perm_scoped = super::add_filter_props( + perm_filter, + &[FilterProp::InZone { + zone: Zone::Battlefield, + }], + ); + // CR 108.2 + CR 404.1: "all cards in ``" — a bare zone word (no + // possessive) is every player's such zone, so the leg is `TypeFilter::Card` + // (CR 108.2) with `controller` left `None` (every owner). + let zone_leg = + TargetFilter::Typed(TypedFilter::card().properties(vec![FilterProp::InAnyZone { zones }])); + Some(merge_or_filters(perm_scoped, zone_leg)) +} + +/// CR 404.1 + CR 108.2: parse a trailing whole-zone union tail after the +/// permanent legs of a mass exile — a leading union separator, then one or more +/// bare zone words ("graveyards", "hands", "libraries"), each optionally +/// preceded by "all "/"each ". A bare zone word (no possessive) denotes every +/// card in that zone across all owners. Returns the deduped zone list, or `None` +/// when there is no leading separator, no zone word, or a trailing fragment that +/// would be silently dropped. Mirrors the separator/consumption discipline of +/// [`try_parse_multi_zone_player_exile`]. +fn parse_trailing_zone_union(rem_lower: &str) -> Option> { + fn strip_leg_quantifier(input: &str) -> &str { + alt((tag::<_, _, OracleError<'_>>("all "), tag("each "))) + .parse(input) + .map(|(rest, _)| rest) + .unwrap_or(input) + } + // Leading separator joining the permanent legs to the first zone leg. + let sep_len = match_mass_union_separator(rem_lower)?; + let (mut input, first) = parse_zone_word(strip_leg_quantifier(&rem_lower[sep_len..])).ok()?; + let mut zones = vec![first]; + // Additional zone legs joined by the same mass-union separators. + while let Some(sep) = match_mass_union_separator(input) { + let Ok((next, zone)) = parse_zone_word(strip_leg_quantifier(&input[sep..])) else { + break; + }; + if !zones.contains(&zone) { + zones.push(zone); + } + input = next; + } + // Full-consumption guard: nothing but sentence punctuation may remain, so no + // trailing fragment is orphaned into an unsupported child node. + let tail = input.trim_start().trim_start_matches('.').trim(); // allow-noncombinator: punctuation cleanup after typed terminator + if !tail.is_empty() { + return None; + } + Some(zones) +} + pub(super) fn parse_search_and_creation_ast( text: &str, lower: &str, @@ -4560,8 +4674,8 @@ fn parse_choose_zone_connector( fn parse_choose_zone_list(input: &str) -> nom::IResult<&str, Vec, OracleError<'_>> { type E<'a> = OracleError<'a>; - let (rest, first) = parse_choose_zone(input)?; - let (rest, second) = opt(preceded(tag::<_, _, E>(" or "), parse_choose_zone)).parse(rest)?; + let (rest, first) = parse_zone_word(input)?; + let (rest, second) = opt(preceded(tag::<_, _, E>(" or "), parse_zone_word)).parse(rest)?; let mut zones = vec![first]; if let Some(second) = second { zones.push(second); @@ -4569,18 +4683,6 @@ fn parse_choose_zone_list(input: &str) -> nom::IResult<&str, Vec, OracleEr Ok((rest, zones)) } -fn parse_choose_zone(input: &str) -> nom::IResult<&str, Zone, OracleError<'_>> { - type E<'a> = OracleError<'a>; - - alt(( - value(Zone::Graveyard, tag::<_, _, E>("graveyard")), - value(Zone::Library, tag("library")), - value(Zone::Hand, tag("hand")), - value(Zone::Exile, tag("exile")), - )) - .parse(input) -} - /// CR 115.1c + CR 601.2c + CR 608.2c: Detect "target X and target Y" wording /// after a "Choose " prefix and split it into two independent target slots. /// @@ -8465,6 +8567,23 @@ pub(super) fn parse_exile_ast( multi_target: None, }); } + // CR 406.2 + CR 404.1 + CR 108.2 + CR 608.2f: "exile all and " — a heterogeneous union of a battlefield + // permanent population and whole-zone (graveyard/hand/library) + // populations (Ultimate Nullification: "Exile all creatures and + // graveyards"). Recognized before the type-only union path below, which + // parses only the permanent leg and orphans the trailing zone leg as an + // unsupported child clause. `origin: None` defers the multi-zone scan to + // `extract_zones()` (Battlefield ∪ zone-union) carried on the filter. + if let Some(target) = try_parse_mass_exile_permanents_and_zones(rest, rest_lower, ctx) { + return Some(ZoneCounterImperativeAst::Exile { + origin: None, + target, + all: true, + enter_with_counters: vec![], + multi_target: None, + }); + } // CR 205.2a + CR 205.3a + CR 608.2c: parse the full target as a // multi-type union so "exile all A except , all B, and all C" lowers to // one `ChangeZoneAll { Or[…] }` instead of fragmenting the trailing diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 44507b31a3..8b6ad53deb 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -15790,6 +15790,29 @@ fn try_parse_verb_and_target<'a>( "", )); } + // CR 406.2 + CR 404.1 + CR 608.2f: "exile all and + // " (Ultimate Nullification) is ONE mass-exile instruction + // spanning the battlefield and whole zones, not a compound. This is the + // compound-splitter's remainder probe: claiming the whole clause (empty + // remainder) keeps it single so it is not mis-split into an orphaned + // "graveyards" conjunct. The actual `ChangeZoneAll { Or[..] }` is built + // by `parse_exile_ast`, which mirrors this recognizer. + if let Some(target) = + imperative::try_parse_mass_exile_permanents_and_zones(rest, rest_lower, ctx) + { + return Some(( + TargetedImperativeAst::ZoneCounterProxy(Box::new( + ZoneCounterImperativeAst::Exile { + origin: None, + target, + all: true, + enter_with_counters: vec![], + multi_target: None, + }, + )), + "", + )); + } let (parsed_target, rem) = parse_target_with_ctx(rest, ctx); // CR 701.5a: "exile all spells" must constrain to the stack. let target = if scan_contains_phrase(rest_lower, "spell") { diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index 2d7225d9af..eba94e1786 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -24242,6 +24242,169 @@ fn exile_all_creatures_and_spacecraft_lowers_to_mass_zone_change() { } } +/// CR 406.2 + CR 404.1 + CR 608.2f (Ultimate Nullification): "Exile all creatures +/// and graveyards" is ONE mass exile spanning the battlefield and every +/// graveyard — a single `ChangeZoneAll { Or[Creature+InZone(BF), +/// Card+InAnyZone([Graveyard])] }`, never a creature wipe plus an orphaned +/// `Unimplemented { "graveyards" }` conjunct (the pre-fix parse). The self-return +/// tail (`Put ~ on the bottom of its owner's library`) chains as a +/// `PutAtLibraryPosition { SelfRef, Bottom }`. +#[test] +fn ultimate_nullification_exiles_creatures_and_all_graveyards() { + let def = parse_effect_chain( + "Exile all creatures and graveyards. Put ~ on the bottom of its owner's library.", + AbilityKind::Spell, + ); + + let target = match &*def.effect { + Effect::ChangeZoneAll { + destination: Zone::Exile, + origin: None, + target, + .. + } => target, + other => panic!("expected ChangeZoneAll to Exile, got {other:?}"), + }; + let filters = match target { + TargetFilter::Or { filters } => filters, + other => panic!("expected an Or of a battlefield leg and a graveyard leg, got {other:?}"), + }; + // Battlefield creature leg: Typed(Creature) scoped to the battlefield so + // `extract_zones` yields Battlefield ∪ Graveyard. + assert!( + filters.iter().any(|f| matches!( + f, + TargetFilter::Typed(tf) + if tf.type_filters.contains(&TypeFilter::Creature) + && tf.properties.contains(&FilterProp::InZone { zone: Zone::Battlefield }) + )), + "missing battlefield creature leg, got {filters:?}" + ); + // Whole-graveyard leg: every card (`TypeFilter::Card`), every owner + // (`controller: None`), in the graveyard zone. + assert!( + filters.iter().any(|f| matches!( + f, + TargetFilter::Typed(tf) + if tf.type_filters.contains(&TypeFilter::Card) + && tf.controller.is_none() + && tf.properties.contains(&FilterProp::InAnyZone { zones: vec![Zone::Graveyard] }) + )), + "missing all-cards all-graveyards leg, got {filters:?}" + ); + + // No coverage-gap sentinel anywhere in the lowered chain — the pre-fix parse + // emitted `Effect::Unimplemented { name: "graveyards" }`. + fn chain_has_unimplemented(ability: &AbilityDefinition) -> bool { + matches!(*ability.effect, Effect::Unimplemented { .. }) + || ability + .sub_ability + .as_deref() + .is_some_and(chain_has_unimplemented) + || ability + .else_ability + .as_deref() + .is_some_and(chain_has_unimplemented) + } + assert!( + !chain_has_unimplemented(&def), + "chain must not retain an Unimplemented node: {def:#?}" + ); + + // The self-return tail: put the spell on the bottom of its owner's library. + fn find_put_at_library(ability: &AbilityDefinition) -> Option<&AbilityDefinition> { + if matches!(*ability.effect, Effect::PutAtLibraryPosition { .. }) { + return Some(ability); + } + ability.sub_ability.as_deref().and_then(find_put_at_library) + } + let put = find_put_at_library(&def).expect("expected a PutAtLibraryPosition tail"); + assert!( + matches!( + &*put.effect, + Effect::PutAtLibraryPosition { + target: TargetFilter::SelfRef, + position: LibraryPosition::Bottom, + .. + } + ), + "self-return must be PutAtLibraryPosition {{ SelfRef, Bottom }}, got {:?}", + put.effect + ); +} + +/// Reach-guard: a pure permanent-type union with NO zone leg ("Exile all +/// creatures and artifacts") must be DECLINED by the heterogeneous recognizer and +/// keep the existing type-union lowering — crucially with NO `InZone(Battlefield)` +/// scoping injected (that belongs only to the mixed permanent+zone form). +#[test] +fn exile_permanents_and_zones_declines_pure_type_union() { + let def = parse_effect_chain("Exile all creatures and artifacts.", AbilityKind::Spell); + let filters = match &*def.effect { + Effect::ChangeZoneAll { + destination: Zone::Exile, + target: TargetFilter::Or { filters }, + .. + } => filters, + other => panic!("expected ChangeZoneAll with an Or filter, got {other:?}"), + }; + assert_eq!( + filters.len(), + 2, + "expected exactly Creature/Artifact legs, got {filters:?}" + ); + assert!( + filters + .iter() + .all(|f| matches!(f, TargetFilter::Typed(tf) if tf.properties.is_empty())), + "pure type union must carry no InZone scoping, got {filters:?}" + ); + assert!( + filters.iter().any(|f| matches!( + f, + TargetFilter::Typed(tf) if tf.type_filters.contains(&TypeFilter::Creature) + )), + "expected a Creature leg, got {filters:?}" + ); + assert!( + filters.iter().any(|f| matches!( + f, + TargetFilter::Typed(tf) if tf.type_filters.contains(&TypeFilter::Artifact) + )), + "expected an Artifact leg, got {filters:?}" + ); +} + +/// Generalization: the zone-union tail is a building block, not a "graveyards" +/// special case — "Exile all creatures and graveyards and hands" carries both +/// zones on the whole-zone leg's `InAnyZone`. +#[test] +fn exile_permanents_and_zones_generalizes_to_multiple_zones() { + let def = parse_effect_chain( + "Exile all creatures and graveyards and hands.", + AbilityKind::Spell, + ); + let filters = match &*def.effect { + Effect::ChangeZoneAll { + destination: Zone::Exile, + target: TargetFilter::Or { filters }, + .. + } => filters, + other => panic!("expected ChangeZoneAll with an Or filter, got {other:?}"), + }; + assert!( + filters.iter().any(|f| matches!( + f, + TargetFilter::Typed(tf) + if tf.type_filters.contains(&TypeFilter::Card) + && tf.properties.contains(&FilterProp::InAnyZone { + zones: vec![Zone::Graveyard, Zone::Hand] + }) + )), + "expected a Card + InAnyZone([Graveyard, Hand]) leg, got {filters:?}" + ); +} + #[test] fn parse_put_on_top_or_bottom_possessive() { // "Target creature's owner puts it on their choice of the top or bottom of their library." diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index 54dd07f773..e5c170dada 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -2062,7 +2062,7 @@ pub fn parse_type_phrase(text: &str) -> (TargetFilter, &str) { /// ("…, all artifacts, and all enchantments"). Longest-match-first over the /// comma / "and" / "or" connectors. Returns `None` when `lower` does not start /// with a union separator. -fn match_mass_union_separator(lower: &str) -> Option { +pub(crate) fn match_mass_union_separator(lower: &str) -> Option { alt(( tag::<_, _, OracleError<'_>>(", and/or "), tag(", and "), diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 18d62e8b0f..fce0ff7d45 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -929,6 +929,7 @@ mod twice_instead_repeat_for; mod twilight_prophet_upkeep_drain_1375; mod typhoon_per_opponent_island_count; mod tyvar_activate_as_though_haste; +mod ultimate_nullification; mod unholy_citadel_legendary_color_banding_grant; mod unmaterialized_lki_serialization; mod unravel_counter_mana_value; diff --git a/crates/engine/tests/integration/ultimate_nullification.rs b/crates/engine/tests/integration/ultimate_nullification.rs new file mode 100644 index 0000000000..2be7720fe4 --- /dev/null +++ b/crates/engine/tests/integration/ultimate_nullification.rs @@ -0,0 +1,212 @@ +//! Runtime pipeline coverage — Ultimate Nullification ({4}{W} sorcery). +//! +//! Verbatim Oracle text (Scryfall oracle_id 2fe0ebf5-52ed-4e92-9b93-81e9ae564439): +//! "As an additional cost to cast this spell, sacrifice a legendary creature. +//! Exile all creatures and graveyards. Put Ultimate Nullification on the +//! bottom of its owner's library." +//! +//! Before the parser fix, "Exile all creatures and graveyards" lowered to a +//! creature-only `ChangeZoneAll` plus an orphaned `Unimplemented { "graveyards" }` +//! — the graveyard wipe was silently dropped. These tests drive the REAL +//! cast -> pay-the-sacrifice -> resolve pipeline. +//! +//! DISCRIMINATING: with the fix reverted, `Effect::Unimplemented { "graveyards" }` +//! is a no-op, so every card in every graveyard stays put — the graveyard-exile +//! assertions below flip red. The battlefield-creature assertions do NOT +//! discriminate (the creature-only `ChangeZoneAll` still exiles them), so the +//! graveyard assertions (including the just-sacrificed legendary, in the caster's +//! own graveyard, exiled by a `controller: None` leg) are the revert-failing +//! authority. Surviving noncreature permanents are the paired reach-guard proving +//! the mass exile is not a nuke of everything. + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::actions::GameAction; +use engine::types::game_state::{CastPaymentMode, PayCostKind, WaitingFor}; +use engine::types::mana::ManaColor; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +// Built from the real card's exact Oracle text (the card refers to itself by its +// printed name, exercising the `~`-normalization -> `SelfRef` path). +const ULTIMATE_NULLIFICATION: &str = "As an additional cost to cast this spell, sacrifice a legendary creature.\n\ + Exile all creatures and graveyards. Put Ultimate Nullification on the bottom of its owner's library."; + +#[test] +fn ultimate_nullification_wipes_creatures_and_all_graveyards_then_self_tucks() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + // A filler on top of the caster's library so "bottom" placement is observable + // (the spell must land BELOW it, not merely somewhere in the library). + scenario.with_library_top(P0, &["Filler Top"]); + + // Caster (P0): the spell in hand, the legendary sacrificed to pay the cost, a + // plain creature, and a noncreature permanent (land) that must survive. + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Ultimate Nullification", false, ULTIMATE_NULLIFICATION) + .id(); + let legendary = scenario + .add_creature(P0, "Legendary Bear", 2, 2) + .as_legendary() + .id(); + let own_creature = scenario.add_vanilla(P0, 1, 1); + let own_land = scenario.add_basic_land(P0, ManaColor::White); + + // Opponent (P1): a battlefield creature and a battlefield land — proving the + // creature leg spans ALL controllers and the land (noncreature) survives. + let opp_creature = scenario.add_vanilla(P1, 3, 3); + let opp_land = scenario.add_basic_land(P1, ManaColor::Green); + + // Seed both graveyards with a creature card AND a noncreature card, proving + // the graveyard leg is "all cards, every type, every owner". + let p0_gy_creature = scenario + .add_creature_to_graveyard(P0, "Dead Bear", 2, 2) + .id(); + let p0_gy_spell = scenario.add_spell_to_graveyard(P0, "Spent Bolt", true).id(); + let p1_gy_creature = scenario + .add_creature_to_graveyard(P1, "Dead Wolf", 2, 2) + .id(); + let p1_gy_spell = scenario + .add_spell_to_graveyard(P1, "Spent Counterspell", true) + .id(); + + let mut runner = scenario.build(); + + let outcome = runner.cast(spell).sacrifice_with(&[legendary]).resolve(); + + // --- Battlefield creatures (both controllers) are exiled. --- + assert_eq!( + outcome.zone_of(own_creature), + Zone::Exile, + "the caster's battlefield creature must be exiled" + ); + assert_eq!( + outcome.zone_of(opp_creature), + Zone::Exile, + "the opponent's battlefield creature must be exiled (creature leg spans all controllers)" + ); + + // --- Every graveyard card, every owner, every type, is exiled. This is the + // revert-failing authority: on the pre-fix parse these stay in graveyard. --- + for (id, label) in [ + (p0_gy_creature, "caster graveyard creature card"), + (p0_gy_spell, "caster graveyard instant card"), + (p1_gy_creature, "opponent graveyard creature card"), + (p1_gy_spell, "opponent graveyard instant card"), + ] { + assert_eq!( + outcome.zone_of(id), + Zone::Exile, + "{label} must be exiled by the whole-graveyard leg" + ); + } + // The legendary sacrificed to pay the cost lands in the caster's graveyard, + // then the same `controller: None` graveyard leg exiles it — proving the leg + // is NOT scoped to the caster and really spans all owners. + assert_eq!( + outcome.zone_of(legendary), + Zone::Exile, + "the sacrificed legendary (caster's graveyard) must also be exiled by the graveyard leg" + ); + + // --- Reach-guard: noncreature battlefield permanents survive. --- + assert_eq!( + outcome.zone_of(own_land), + Zone::Battlefield, + "the caster's land is neither a creature nor a graveyard card and must survive" + ); + assert_eq!( + outcome.zone_of(opp_land), + Zone::Battlefield, + "the opponent's land must survive" + ); + + // --- Mechanic 3: the spell tucks itself to the BOTTOM of its owner's library + // (CR 400.3 owner-keyed library routing), never the graveyard. --- + assert_eq!( + outcome.zone_of(spell), + Zone::Library, + "Ultimate Nullification must end in its owner's library, not the graveyard" + ); + let p0_library: Vec<_> = outcome + .state() + .players + .iter() + .find(|p| p.id == P0) + .expect("P0 exists") + .library + .iter() + .copied() + .collect(); + assert_eq!( + p0_library.last().copied(), + Some(spell), + "the spell must be on the BOTTOM of the caster's library, below the filler; got {p0_library:?}" + ); + assert!( + p0_library.first() != Some(&spell), + "the spell must NOT be on top (it went to the bottom)" + ); +} + +/// Control (mechanic 1): the additional cost requires a *legendary* creature. +/// With only a nonlegendary creature available, the cast cannot be paid: the +/// engine either rejects the announcement or surfaces a sacrifice prompt with no +/// legal choice — and the nonlegendary creature is never sacrificed. +#[test] +fn ultimate_nullification_requires_a_legendary_creature_to_sacrifice() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Ultimate Nullification", false, ULTIMATE_NULLIFICATION) + .id(); + // Only a NONlegendary creature — not a legal sacrifice for this cost. + let plain_creature = scenario.add_vanilla(P0, 1, 1); + + let mut runner = scenario.build(); + let card_id = runner.state().objects[&spell].card_id; + + let cast = runner.act(GameAction::CastSpell { + object_id: spell, + card_id, + targets: vec![], + payment_mode: CastPaymentMode::Auto, + }); + + match cast { + // The engine rejected the announcement outright — the sacrifice + // additional cost (CR 601.2f) cannot be paid with no legendary creature. + Err(_) => {} + Ok(_) => { + // Otherwise it must surface the mandatory sacrifice with NO legal + // legendary to choose. + match &runner.state().waiting_for { + WaitingFor::PayCost { + kind: PayCostKind::Sacrifice, + choices, + .. + } => { + assert!( + !choices.contains(&plain_creature), + "a nonlegendary creature must never be a legal sacrifice for this cost" + ); + assert!( + choices.is_empty(), + "with no legendary creature there must be no legal sacrifice, got {choices:?}" + ); + } + other => panic!( + "expected either cast rejection or an unsatisfiable Sacrifice prompt, got {other:?}" + ), + } + } + } + + // Whatever path the engine took, the nonlegendary creature is never sacrificed. + assert_eq!( + runner.state().objects[&plain_creature].zone, + Zone::Battlefield, + "the nonlegendary creature must not have been sacrificed" + ); +} From f6342d2d04337489515b08bcd6536d5220528d74 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 20:01:57 -0500 Subject: [PATCH 2/7] fix: decline owner-scoped forms in mass-exile permanent+zone recognizer The heterogeneous "exile all and " recognizer (try_parse_mass_exile_permanents_and_zones) over-matched cards whose leading leg already carries its own source-zone / owner scope, then injected InZone(Battlefield) and rebuilt the zone leg as all-owners / all-cards. This regressed two supported cards: - Thought Distortion ("exile all noncreature, nonland cards from that player's hand and graveyard") gained an impossible hand-and-battlefield leg and lost both the "that player's" owner scope and the noncreature/nonland restriction on the graveyard leg. - Worldfire ("exile all cards from all hands and graveyards") gained the same impossible battlefield injection on its hand leg. Guard the recognizer with is_bare_battlefield_permanent_leg: only fire when the leading permanent leg has no controller scope and no InZone/InAnyZone property (a true battlefield permanent-type union per CR 109.2). Ultimate Nullification ("creatures and graveyards") is unaffected; Thought Distortion / Worldfire fall back to their existing owner-scoped multi-zone parse. Add a Thought Distortion parser regression pinning the fix. Review nit cleanup: - Remove unverified CR 608.2f from the parser-test annotation (a resolution-time simultaneity rule, not a parse-shape rule). - Strengthen the multi-zone generalization test to also validate the Creature+InZone(Battlefield) leg from the same filters result. - Tighten the integration Err branch to assert the specific ActionNotAllowed("...required additional cost") rejection so an unrelated parser/mana failure cannot satisfy it. - Fix CR 601.2f -> CR 601.2h in the announcement-rejection comment. Co-Authored-By: Claude Opus 4.8 --- .../src/parser/oracle_effect/imperative.rs | 42 ++++++++ .../engine/src/parser/oracle_effect/tests.rs | 95 ++++++++++++++++++- .../integration/ultimate_nullification.rs | 15 ++- 3 files changed, 148 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index 99648cb77b..ec7e9e1164 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -2767,6 +2767,17 @@ pub(super) fn try_parse_mass_exile_permanents_and_zones( // ("creatures", "creatures and artifacts", …); `rem` is the untyped tail // (e.g. " and graveyards"). let (perm_filter, rem) = parse_mass_type_union(rest, ctx); + // The leading leg MUST be a bare battlefield permanent-type description. If + // `parse_mass_type_union` already consumed an explicit source zone or owner + // scope ("… cards from that player's hand", "… cards from all hands"), this + // form belongs to the owner-scoped multi-zone parser, not here — injecting + // `InZone(Battlefield)` below would make that zone leg unsatisfiable and the + // rebuilt zone leg would drop the parsed owner (CR 400.3) and type + // restriction (CR 205). Decline so Thought Distortion / Worldfire keep their + // existing owner-scoped parse. + if !is_bare_battlefield_permanent_leg(&perm_filter) { + return None; + } // CR 404.1 + CR 108.2: the trailing whole-zone union — declines if absent. let zones = parse_trailing_zone_union(&rem.to_lowercase())?; // CR 109.2: a bare card-type description ("creatures", no zone word / "card") @@ -2788,6 +2799,37 @@ pub(super) fn try_parse_mass_exile_permanents_and_zones( Some(merge_or_filters(perm_scoped, zone_leg)) } +/// Whether every leg of `filter` is a BARE battlefield permanent-type +/// description — no owner scope (`controller`) and no explicit source-zone +/// property (`InZone` / `InAnyZone`). This is the discriminator that keeps +/// [`try_parse_mass_exile_permanents_and_zones`] off owner-scoped / zone-scoped +/// forms: Thought Distortion ("… noncreature, nonland cards from that player's +/// hand and graveyard") and Worldfire ("… cards from all hands and graveyards") +/// both have `parse_mass_type_union` consume a leg that already carries its own +/// zone (CR 404.1 / CR 402.1) and, for Thought Distortion, owner (CR 400.3) +/// scope. Scoping such a leg to the battlefield would make it unsatisfiable and +/// the rebuilt all-owners zone leg would drop the parsed owner/type restriction, +/// exiling the wrong cards. Only a leg with no zone and no owner scope denotes +/// "permanents of that type on the battlefield" (CR 109.2), which is the sole +/// shape this recognizer may battlefield-scope and union with whole zones. +fn is_bare_battlefield_permanent_leg(filter: &TargetFilter) -> bool { + match filter { + TargetFilter::Typed(tf) => { + tf.controller.is_none() + && !tf + .properties + .iter() + .any(|p| matches!(p, FilterProp::InZone { .. } | FilterProp::InAnyZone { .. })) + } + TargetFilter::Or { filters } | TargetFilter::And { filters } => { + filters.iter().all(is_bare_battlefield_permanent_leg) + } + TargetFilter::Not { filter } => is_bare_battlefield_permanent_leg(filter), + // Any other filter shape is not a bare permanent-type union. + _ => false, + } +} + /// CR 404.1 + CR 108.2: parse a trailing whole-zone union tail after the /// permanent legs of a mass exile — a leading union separator, then one or more /// bare zone words ("graveyards", "hands", "libraries"), each optionally diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index eba94e1786..28a4ccddc1 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -24242,7 +24242,7 @@ fn exile_all_creatures_and_spacecraft_lowers_to_mass_zone_change() { } } -/// CR 406.2 + CR 404.1 + CR 608.2f (Ultimate Nullification): "Exile all creatures +/// CR 406.2 + CR 404.1 (Ultimate Nullification): "Exile all creatures /// and graveyards" is ONE mass exile spanning the battlefield and every /// graveyard — a single `ChangeZoneAll { Or[Creature+InZone(BF), /// Card+InAnyZone([Graveyard])] }`, never a creature wipe plus an orphaned @@ -24392,6 +24392,19 @@ fn exile_permanents_and_zones_generalizes_to_multiple_zones() { } => filters, other => panic!("expected ChangeZoneAll with an Or filter, got {other:?}"), }; + // Battlefield permanent leg: the "creatures" operand scoped to the + // battlefield — validated from the same `filters` result as the zone leg so + // the union really carries both. + assert!( + filters.iter().any(|f| matches!( + f, + TargetFilter::Typed(tf) + if tf.type_filters.contains(&TypeFilter::Creature) + && tf.properties.contains(&FilterProp::InZone { zone: Zone::Battlefield }) + )), + "expected a Creature + InZone(Battlefield) leg, got {filters:?}" + ); + // Whole-zone leg: every card, every owner, across both trailing zones. assert!( filters.iter().any(|f| matches!( f, @@ -24405,6 +24418,86 @@ fn exile_permanents_and_zones_generalizes_to_multiple_zones() { ); } +/// Regression (Thought Distortion / Worldfire): the heterogeneous +/// permanent+zone recognizer MUST decline forms whose leading leg already +/// carries its own source-zone / owner scope. Thought Distortion ("Exile all +/// noncreature, nonland cards from that player's hand and graveyard") is a +/// hand+graveyard exile of a *targeted opponent's* cards — it has no +/// battlefield component. A prior version of the recognizer injected +/// `InZone(Battlefield)` onto that leg (making the hand leg unsatisfiable) and +/// rebuilt the zone leg as all-owners/all-cards (dropping "that player's" and +/// the noncreature/nonland restriction). This test pins the fix: the lowered +/// chain must contain NO `InZone(Battlefield)` leg, the hand exile must survive, +/// and the noncreature/nonland restriction must be preserved. +#[test] +fn exile_permanents_and_zones_declines_owner_scoped_hand_graveyard_form() { + fn collect_typed<'a>(f: &'a TargetFilter, out: &mut Vec<&'a TypedFilter>) { + match f { + TargetFilter::Typed(tf) => out.push(tf), + TargetFilter::Or { filters } | TargetFilter::And { filters } => { + filters.iter().for_each(|c| collect_typed(c, out)) + } + TargetFilter::Not { filter } => collect_typed(filter, out), + _ => {} + } + } + + let def = parse_effect_chain( + "Exile all noncreature, nonland cards from that player's hand and graveyard.", + AbilityKind::Spell, + ); + + // Walk every effect in the chain, gathering the target filters of any + // ChangeZoneAll to Exile it lowered to. + let mut typed: Vec<&TypedFilter> = Vec::new(); + let mut ability: Option<&AbilityDefinition> = Some(&def); + while let Some(a) = ability { + if let Effect::ChangeZoneAll { + destination: Zone::Exile, + target, + .. + } = &*a.effect + { + collect_typed(target, &mut typed); + } + ability = a.sub_ability.as_deref(); + } + assert!( + !typed.is_empty(), + "expected at least one ChangeZoneAll to Exile in the chain: {def:#?}" + ); + + // The bug signature: no leg may be scoped to the battlefield — these are + // hand/graveyard cards, never permanents. + assert!( + typed + .iter() + .all(|tf| !tf.properties.contains(&FilterProp::InZone { + zone: Zone::Battlefield + })), + "no leg may carry InZone(Battlefield); the cards are in hand/graveyard, got {typed:?}" + ); + // The hand exile must survive (some leg still references the hand zone). + assert!( + typed.iter().any(|tf| tf.properties.iter().any(|p| match p { + FilterProp::InZone { zone } => *zone == Zone::Hand, + FilterProp::InAnyZone { zones } => zones.contains(&Zone::Hand), + _ => false, + })), + "the hand exile must be preserved, got {typed:?}" + ); + // The noncreature/nonland restriction must be preserved on some leg. + assert!( + typed.iter().any(|tf| tf + .type_filters + .contains(&TypeFilter::Non(Box::new(TypeFilter::Creature))) + && tf + .type_filters + .contains(&TypeFilter::Non(Box::new(TypeFilter::Land)))), + "the noncreature/nonland restriction must be preserved, got {typed:?}" + ); +} + #[test] fn parse_put_on_top_or_bottom_possessive() { // "Target creature's owner puts it on their choice of the top or bottom of their library." diff --git a/crates/engine/tests/integration/ultimate_nullification.rs b/crates/engine/tests/integration/ultimate_nullification.rs index 2be7720fe4..1b25c6f025 100644 --- a/crates/engine/tests/integration/ultimate_nullification.rs +++ b/crates/engine/tests/integration/ultimate_nullification.rs @@ -20,6 +20,7 @@ //! the mass exile is not a nuke of everything. use engine::game::scenario::{GameScenario, P0, P1}; +use engine::game::EngineError; use engine::types::actions::GameAction; use engine::types::game_state::{CastPaymentMode, PayCostKind, WaitingFor}; use engine::types::mana::ManaColor; @@ -175,9 +176,17 @@ fn ultimate_nullification_requires_a_legendary_creature_to_sacrifice() { }); match cast { - // The engine rejected the announcement outright — the sacrifice - // additional cost (CR 601.2f) cannot be paid with no legendary creature. - Err(_) => {} + // CR 601.2h: the engine rejected the announcement outright because its + // total cost cannot be paid — here the unpayable component is the + // mandatory legendary-creature sacrifice (CR 601.2f additional cost). + // Assert the SPECIFIC required-additional-cost rejection so an unrelated + // parser, mana, or implementation failure can't satisfy this branch. + Err(e) => { + assert!( + matches!(&e, EngineError::ActionNotAllowed(msg) if msg.contains("required additional cost")), + "cast must fail specifically because the required sacrifice is unpayable, got {e:?}" + ); + } Ok(_) => { // Otherwise it must surface the mandatory sacrifice with NO legal // legendary to choose. From 008c609d37a691aa22b3b2c634fcb145d79b6715 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 21:00:54 -0500 Subject: [PATCH 3/7] test: harden Ultimate Nullification / Thought Distortion regressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the second review round on PR #6940. Thought Distortion regression at the production boundary (was: isolated `parse_effect_chain` on one sentence with weak assertions). The new test runs the card's COMPLETE Oracle text through `build_oracle_face` — the same entry the card-data pipeline uses — and pins the exact baseline shape my recognizer guard restores: - `RevealHand` targeting the Opponent (the owner binding), - its `ChangeZoneAll` keeps `origin: Some(Hand)` — the ownership linkage the prior defect destroyed by rewriting it to `origin: None`, - the exile filter keeps `Non(Creature)`/`Non(Land)` + `InZone(Hand)` with NO injected `InZone(Battlefield)`, as a single `Typed` (never an `Or`). It also documents the boundary: the trailing "and graveyard" leg is a pre-existing `Unimplemented` gap on main (`Effect:graveyard`) that this PR neither closes nor worsens. Ultimate Nullification runtime fixtures now cast the printed {4}{W} spell: both fixtures set the real mana cost and seed matching mana. The positive fixture therefore pays the true total cost (mana + the sacrifice additional cost); the negative fixture has mana fully payable so the ONLY unpayable component is the legendary sacrifice, which the tightened error assertion confirms. Co-Authored-By: Claude Opus 4.8 --- crates/engine/src/database/synthesis.rs | 153 ++++++++++++++++++ .../engine/src/parser/oracle_effect/tests.rs | 85 +--------- .../integration/ultimate_nullification.rs | 33 +++- 3 files changed, 191 insertions(+), 80 deletions(-) diff --git a/crates/engine/src/database/synthesis.rs b/crates/engine/src/database/synthesis.rs index e18b8c9a9e..e8c5c74c45 100644 --- a/crates/engine/src/database/synthesis.rs +++ b/crates/engine/src/database/synthesis.rs @@ -10766,6 +10766,159 @@ mod cycling_synthesis_tests { ); } + /// Regression (Thought Distortion, PR #6940): the heterogeneous + /// permanent+zone mass-exile recognizer must DECLINE owner-scoped forms, so + /// this card's production parse stays byte-identical to its pre-PR baseline. + /// The prior defect rewrote its hand-exile `ChangeZoneAll { origin: Hand }` + /// (which ties the exile to the reveal target's hand) into an `origin: None` + /// `Or[.. + InZone(Battlefield), Card + InAnyZone(Graveyard)]`, injecting an + /// impossible hand-and-battlefield constraint and dropping the + /// noncreature/nonland restriction. This asserts the exact restored shape at + /// the production boundary (`build_oracle_face`): a `RevealHand` targeting the + /// Opponent (the owner binding); its `ChangeZoneAll` keeping `origin: + /// Some(Hand)` rather than `None`; and the exile filter keeping + /// `Non(Creature)`/`Non(Land)` + `InZone(Hand)` with NO `InZone(Battlefield)`, + /// as a single `Typed` rather than an `Or`. + /// + /// The trailing "and graveyard" leg remains an `Unimplemented` gap + /// (`Effect:graveyard`), a pre-existing main limitation this PR neither closes + /// nor worsens. + #[test] + fn thought_distortion_declines_recognizer_at_production_boundary() { + use crate::database::mtgjson::AtomicIdentifiers; + use crate::types::ability::{AbilityDefinition, TypeFilter}; + use crate::types::zones::Zone; + + let oracle = "This spell can't be countered.\n\ + Target opponent reveals their hand. Exile all noncreature, nonland cards from that player's hand and graveyard."; + let mtgjson = AtomicCard { + name: "Thought Distortion".to_string(), + mana_cost: Some("{4}{B}{B}".to_string()), + colors: vec!["B".to_string()], + color_identity: vec!["B".to_string()], + text: Some(oracle.to_string()), + power: None, + toughness: None, + loyalty: None, + defense: None, + layout: "normal".to_string(), + type_line: Some("Sorcery".to_string()), + types: vec!["Sorcery".to_string()], + subtypes: vec![], + supertypes: vec![], + keywords: None, + side: None, + face_name: None, + mana_value: 6.0, + legalities: Default::default(), + leadership_skills: None, + printings: Vec::new(), + rulings: Vec::new(), + is_game_changer: false, + identifiers: AtomicIdentifiers { + scryfall_oracle_id: Some("5f089ac6-9e92-4ec2-bf46-a0b08d1e2979".to_string()), + scryfall_id: Some("thought-distortion-face".to_string()), + }, + foreign_data: Vec::new(), + related_cards: crate::database::mtgjson::SetRelatedCards::default(), + }; + + let face = build_oracle_face(&mtgjson, None); + + // The reveal establishes the owner binding: the exile is scoped to the + // targeted OPPONENT's hand (CR 601.2c target opponent). + let reveal = face + .abilities + .iter() + .find(|a| matches!(&*a.effect, Effect::RevealHand { .. })) + .expect("Thought Distortion must parse a RevealHand ability"); + match &*reveal.effect { + Effect::RevealHand { target, .. } => assert!( + matches!( + target, + TargetFilter::Typed(tf) + if tf.controller == Some(crate::types::ability::ControllerRef::Opponent) + ), + "RevealHand must target the opponent, got {target:?}" + ), + _ => unreachable!(), + } + + // Walk the whole chain and locate the hand-exile ChangeZoneAll. + fn walk<'a>(a: &'a AbilityDefinition, out: &mut Vec<&'a AbilityDefinition>) { + out.push(a); + if let Some(sub) = a.sub_ability.as_deref() { + walk(sub, out); + } + if let Some(els) = a.else_ability.as_deref() { + walk(els, out); + } + } + let mut chain = Vec::new(); + for a in &face.abilities { + walk(a, &mut chain); + } + + let exile = chain + .iter() + .find_map(|a| match &*a.effect { + Effect::ChangeZoneAll { + destination: Zone::Exile, + origin, + target, + .. + } => Some((origin, target)), + _ => None, + }) + .expect("must retain a ChangeZoneAll to Exile for the hand leg"); + let (origin, target) = exile; + + // The owner-linkage the bug destroyed: origin stays Hand, not None. + assert_eq!( + *origin, + Some(Zone::Hand), + "hand-exile ChangeZoneAll must keep origin: Some(Hand) (bug changed it to None)" + ); + + // A single Typed leg (never collapsed into an Or), carrying the + // noncreature/nonland restriction and Hand scoping — and crucially NO + // battlefield injection. + let tf = match target { + TargetFilter::Typed(tf) => tf, + other => panic!("hand-exile target must be a single Typed leg, got {other:?}"), + }; + assert!( + tf.type_filters + .contains(&TypeFilter::Non(Box::new(TypeFilter::Creature))) + && tf + .type_filters + .contains(&TypeFilter::Non(Box::new(TypeFilter::Land))), + "noncreature/nonland restriction must survive, got {:?}", + tf.type_filters + ); + assert!( + tf.properties + .contains(&FilterProp::InZone { zone: Zone::Hand }), + "hand scoping must survive, got {:?}", + tf.properties + ); + assert!( + !tf.properties.contains(&FilterProp::InZone { + zone: Zone::Battlefield + }), + "no InZone(Battlefield) may be injected, got {:?}", + tf.properties + ); + + // Documented boundary: the graveyard leg is still an unimplemented gap on + // main; this PR neither closes nor worsens it. + assert_eq!( + crate::game::coverage::card_face_gaps(&face), + vec!["Effect:graveyard".to_string()], + "only the pre-existing graveyard gap should remain" + ); + } + /// MSH Wave 2 (Storm, Queen of Wakanda): MTGJSON phantom-tags the Storm keyword /// (CR 702.40) because the card's name embeds the word "Storm". The synthesis /// name-guard must drop the uncorroborated Storm keyword while keeping the real diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index 28a4ccddc1..eb09a67637 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -24418,85 +24418,12 @@ fn exile_permanents_and_zones_generalizes_to_multiple_zones() { ); } -/// Regression (Thought Distortion / Worldfire): the heterogeneous -/// permanent+zone recognizer MUST decline forms whose leading leg already -/// carries its own source-zone / owner scope. Thought Distortion ("Exile all -/// noncreature, nonland cards from that player's hand and graveyard") is a -/// hand+graveyard exile of a *targeted opponent's* cards — it has no -/// battlefield component. A prior version of the recognizer injected -/// `InZone(Battlefield)` onto that leg (making the hand leg unsatisfiable) and -/// rebuilt the zone leg as all-owners/all-cards (dropping "that player's" and -/// the noncreature/nonland restriction). This test pins the fix: the lowered -/// chain must contain NO `InZone(Battlefield)` leg, the hand exile must survive, -/// and the noncreature/nonland restriction must be preserved. -#[test] -fn exile_permanents_and_zones_declines_owner_scoped_hand_graveyard_form() { - fn collect_typed<'a>(f: &'a TargetFilter, out: &mut Vec<&'a TypedFilter>) { - match f { - TargetFilter::Typed(tf) => out.push(tf), - TargetFilter::Or { filters } | TargetFilter::And { filters } => { - filters.iter().for_each(|c| collect_typed(c, out)) - } - TargetFilter::Not { filter } => collect_typed(filter, out), - _ => {} - } - } - - let def = parse_effect_chain( - "Exile all noncreature, nonland cards from that player's hand and graveyard.", - AbilityKind::Spell, - ); - - // Walk every effect in the chain, gathering the target filters of any - // ChangeZoneAll to Exile it lowered to. - let mut typed: Vec<&TypedFilter> = Vec::new(); - let mut ability: Option<&AbilityDefinition> = Some(&def); - while let Some(a) = ability { - if let Effect::ChangeZoneAll { - destination: Zone::Exile, - target, - .. - } = &*a.effect - { - collect_typed(target, &mut typed); - } - ability = a.sub_ability.as_deref(); - } - assert!( - !typed.is_empty(), - "expected at least one ChangeZoneAll to Exile in the chain: {def:#?}" - ); - - // The bug signature: no leg may be scoped to the battlefield — these are - // hand/graveyard cards, never permanents. - assert!( - typed - .iter() - .all(|tf| !tf.properties.contains(&FilterProp::InZone { - zone: Zone::Battlefield - })), - "no leg may carry InZone(Battlefield); the cards are in hand/graveyard, got {typed:?}" - ); - // The hand exile must survive (some leg still references the hand zone). - assert!( - typed.iter().any(|tf| tf.properties.iter().any(|p| match p { - FilterProp::InZone { zone } => *zone == Zone::Hand, - FilterProp::InAnyZone { zones } => zones.contains(&Zone::Hand), - _ => false, - })), - "the hand exile must be preserved, got {typed:?}" - ); - // The noncreature/nonland restriction must be preserved on some leg. - assert!( - typed.iter().any(|tf| tf - .type_filters - .contains(&TypeFilter::Non(Box::new(TypeFilter::Creature))) - && tf - .type_filters - .contains(&TypeFilter::Non(Box::new(TypeFilter::Land)))), - "the noncreature/nonland restriction must be preserved, got {typed:?}" - ); -} +// NOTE: the Thought Distortion regression now lives at the PRODUCTION boundary +// (`crate::database::synthesis` tests → +// `thought_distortion_declines_recognizer_at_production_boundary`), which parses +// the card's COMPLETE Oracle text through `build_oracle_face` and asserts the +// preserved `RevealHand`→opponent binding and `ChangeZoneAll { origin: Hand }` +// shape — a stronger guard than an isolated-sentence `parse_effect_chain` call. #[test] fn parse_put_on_top_or_bottom_possessive() { diff --git a/crates/engine/tests/integration/ultimate_nullification.rs b/crates/engine/tests/integration/ultimate_nullification.rs index 1b25c6f025..6f044b9deb 100644 --- a/crates/engine/tests/integration/ultimate_nullification.rs +++ b/crates/engine/tests/integration/ultimate_nullification.rs @@ -23,7 +23,8 @@ use engine::game::scenario::{GameScenario, P0, P1}; use engine::game::EngineError; use engine::types::actions::GameAction; use engine::types::game_state::{CastPaymentMode, PayCostKind, WaitingFor}; -use engine::types::mana::ManaColor; +use engine::types::identifiers::ObjectId; +use engine::types::mana::{ManaColor, ManaCost, ManaCostShard, ManaType, ManaUnit}; use engine::types::phase::Phase; use engine::types::zones::Zone; @@ -32,6 +33,28 @@ use engine::types::zones::Zone; const ULTIMATE_NULLIFICATION: &str = "As an additional cost to cast this spell, sacrifice a legendary creature.\n\ Exile all creatures and graveyards. Put Ultimate Nullification on the bottom of its owner's library."; +// The printed cost: {4}{W}. Both fixtures set this on the spell and seed matching +// mana so the REAL total cost is paid — the sacrifice is an ADDITIONAL cost on +// top of it (CR 601.2f), never a stand-in for the mana cost. +fn ultimate_nullification_cost() -> ManaCost { + ManaCost::Cost { + shards: vec![ManaCostShard::White], + generic: 4, + } +} + +// {4}{W} worth of mana: one white plus four generic (colorless satisfies generic). +fn ultimate_nullification_mana() -> Vec { + let unit = |color| ManaUnit::new(color, ObjectId(0), false, vec![]); + vec![ + unit(ManaType::White), + unit(ManaType::Colorless), + unit(ManaType::Colorless), + unit(ManaType::Colorless), + unit(ManaType::Colorless), + ] +} + #[test] fn ultimate_nullification_wipes_creatures_and_all_graveyards_then_self_tucks() { let mut scenario = GameScenario::new(); @@ -45,7 +68,11 @@ fn ultimate_nullification_wipes_creatures_and_all_graveyards_then_self_tucks() { // plain creature, and a noncreature permanent (land) that must survive. let spell = scenario .add_spell_to_hand_from_oracle(P0, "Ultimate Nullification", false, ULTIMATE_NULLIFICATION) + .with_mana_cost(ultimate_nullification_cost()) .id(); + // Fund the {4}{W} mana cost from the pool so the cast harness auto-pays it; + // the sacrifice is the additional cost paid on top. + scenario.with_mana_pool(P0, ultimate_nullification_mana()); let legendary = scenario .add_creature(P0, "Legendary Bear", 2, 2) .as_legendary() @@ -161,9 +188,13 @@ fn ultimate_nullification_requires_a_legendary_creature_to_sacrifice() { let spell = scenario .add_spell_to_hand_from_oracle(P0, "Ultimate Nullification", false, ULTIMATE_NULLIFICATION) + .with_mana_cost(ultimate_nullification_cost()) .id(); // Only a NONlegendary creature — not a legal sacrifice for this cost. let plain_creature = scenario.add_vanilla(P0, 1, 1); + // Seed the full {4}{W} so mana is payable: the ONLY unpayable component is the + // legendary sacrifice, so the announcement can't fail for a mana reason. + scenario.with_mana_pool(P0, ultimate_nullification_mana()); let mut runner = scenario.build(); let card_id = runner.state().objects[&spell].card_id; From 9793b5feadd774df68c9ffa48ced360ae680e542 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 21:47:46 -0500 Subject: [PATCH 4/7] feat: implement owner-scoped type-restricted multi-zone exile (Thought Distortion) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the last open item on PR #6940's review: implement — not just decline — the owner-scoped, type-restricted, two-zone exile so Thought Distortion fully resolves, with a discriminating runtime regression. Previously `try_parse_multi_zone_player_exile` claimed only the BARE "cards from and " form (Identity Crisis, CR 108.2). A type-qualified variant — Thought Distortion's "exile all noncreature, nonland cards from that player's hand and graveyard" — fell through: the hand leg parsed as a single-zone exile and "and graveyard" was orphaned into an `Unimplemented { "graveyard" }` no-op (card marked unsupported, gap `Effect:graveyard`). Generalize the recognizer to carry an optional leading type restriction (CR 205.2a/205.3a): it now parses "[] cards from " and returns the card-type filters alongside the owner axis and the zone union. The bare form is tried first and keeps empty type_filters, so its representation (and Identity Crisis's parse) is byte-identical. Both dual-site call sites (parse_exile_ast + the compound-splitter probe) build a single `ChangeZoneAll { origin: None, Typed { , controller: TargetPlayer, InAnyZone([Hand, Graveyard]) } }`. Runtime resolution is inherited from the existing multi-zone owner-exile path (Identity Crisis), so `TargetPlayer` binds to the revealed opponent and the exile is confined to that player's zones. Thought Distortion is now fully supported (no coverage gap). Tests: - runtime regression (tests/integration/thought_distortion.rs): casting the real card exiles ONLY the targeted opponent's noncreature/nonland cards, from BOTH hand and graveyard — with owner-scope controls (the caster's own cards stay) and type controls (the target's creature/land cards stay). The graveyard assertions are the revert-failing authority. - production-boundary parse test (synthesis): asserts the owner-scoped, type-restricted, InAnyZone([Hand, Graveyard]) shape and zero gaps. - matcher unit test: bare (empty type_filters) and type-qualified cases. Co-Authored-By: Claude Opus 4.8 --- crates/engine/src/database/synthesis.rs | 81 ++++++------ .../src/parser/oracle_effect/imperative.rs | 99 +++++++++----- crates/engine/src/parser/oracle_effect/mod.rs | 18 +-- .../engine/src/parser/oracle_effect/tests.rs | 18 +++ crates/engine/tests/integration/main.rs | 1 + .../tests/integration/thought_distortion.rs | 122 ++++++++++++++++++ 6 files changed, 264 insertions(+), 75 deletions(-) create mode 100644 crates/engine/tests/integration/thought_distortion.rs diff --git a/crates/engine/src/database/synthesis.rs b/crates/engine/src/database/synthesis.rs index e8c5c74c45..df6afc8514 100644 --- a/crates/engine/src/database/synthesis.rs +++ b/crates/engine/src/database/synthesis.rs @@ -10766,27 +10766,25 @@ mod cycling_synthesis_tests { ); } - /// Regression (Thought Distortion, PR #6940): the heterogeneous - /// permanent+zone mass-exile recognizer must DECLINE owner-scoped forms, so - /// this card's production parse stays byte-identical to its pre-PR baseline. - /// The prior defect rewrote its hand-exile `ChangeZoneAll { origin: Hand }` - /// (which ties the exile to the reveal target's hand) into an `origin: None` - /// `Or[.. + InZone(Battlefield), Card + InAnyZone(Graveyard)]`, injecting an - /// impossible hand-and-battlefield constraint and dropping the - /// noncreature/nonland restriction. This asserts the exact restored shape at - /// the production boundary (`build_oracle_face`): a `RevealHand` targeting the - /// Opponent (the owner binding); its `ChangeZoneAll` keeping `origin: - /// Some(Hand)` rather than `None`; and the exile filter keeping - /// `Non(Creature)`/`Non(Land)` + `InZone(Hand)` with NO `InZone(Battlefield)`, - /// as a single `Typed` rather than an `Or`. - /// - /// The trailing "and graveyard" leg remains an `Unimplemented` gap - /// (`Effect:graveyard`), a pre-existing main limitation this PR neither closes - /// nor worsens. - #[test] - fn thought_distortion_declines_recognizer_at_production_boundary() { + /// Thought Distortion (PR #6940), production boundary (`build_oracle_face`): + /// "Exile all noncreature, nonland cards from that player's hand and + /// graveyard" lowers to ONE owner-scoped, type-restricted, multi-zone exile — + /// not a hand-only wipe plus an orphaned `Unimplemented { "graveyard" }` (the + /// pre-PR parse), and never the mis-parse that injected `InZone(Battlefield)` + /// and dropped the noncreature/nonland restriction. The asserted shape: + /// - `RevealHand` targeting the Opponent (the target that "that player" + /// anaphorically binds to, CR 601.2c), + /// - a single `ChangeZoneAll` to Exile with `origin: None` (the zone union + /// rides on the filter) whose `Typed` filter carries the + /// `Non(Creature)`/`Non(Land)` restriction (CR 205.2a), the + /// `ControllerRef::TargetPlayer` owner scope (CR 400.3), and + /// `InAnyZone([Hand, Graveyard])` (CR 402.1 + CR 404.1) — with NO + /// `InZone(Battlefield)`, + /// - and NO remaining coverage gap (`card_face_gaps` is empty). + #[test] + fn thought_distortion_owner_scoped_multizone_exile_at_production_boundary() { use crate::database::mtgjson::AtomicIdentifiers; - use crate::types::ability::{AbilityDefinition, TypeFilter}; + use crate::types::ability::{AbilityDefinition, ControllerRef, TypeFilter}; use crate::types::zones::Zone; let oracle = "This spell can't be countered.\n\ @@ -10870,22 +10868,21 @@ mod cycling_synthesis_tests { } => Some((origin, target)), _ => None, }) - .expect("must retain a ChangeZoneAll to Exile for the hand leg"); + .expect("must lower to a ChangeZoneAll to Exile"); let (origin, target) = exile; - // The owner-linkage the bug destroyed: origin stays Hand, not None. + // Multi-zone origin rides on the filter, so the lowering passes None. assert_eq!( - *origin, - Some(Zone::Hand), - "hand-exile ChangeZoneAll must keep origin: Some(Hand) (bug changed it to None)" + *origin, None, + "multi-zone exile carries its origin on the filter (InAnyZone), so origin is None" ); - // A single Typed leg (never collapsed into an Or), carrying the - // noncreature/nonland restriction and Hand scoping — and crucially NO - // battlefield injection. + // A single Typed leg (never an Or) carrying: the noncreature/nonland + // restriction, the target-player owner scope, and the hand+graveyard zone + // union — with NO battlefield injection. let tf = match target { TargetFilter::Typed(tf) => tf, - other => panic!("hand-exile target must be a single Typed leg, got {other:?}"), + other => panic!("exile target must be a single Typed leg, got {other:?}"), }; assert!( tf.type_filters @@ -10896,10 +10893,19 @@ mod cycling_synthesis_tests { "noncreature/nonland restriction must survive, got {:?}", tf.type_filters ); + assert_eq!( + tf.controller, + Some(ControllerRef::TargetPlayer), + "the exile must be owner-scoped to the targeted player, got {:?}", + tf.controller + ); assert!( - tf.properties - .contains(&FilterProp::InZone { zone: Zone::Hand }), - "hand scoping must survive, got {:?}", + tf.properties.iter().any(|p| matches!( + p, + FilterProp::InAnyZone { zones } + if zones.contains(&Zone::Hand) && zones.contains(&Zone::Graveyard) + )), + "the zone union must span Hand and Graveyard, got {:?}", tf.properties ); assert!( @@ -10910,12 +10916,11 @@ mod cycling_synthesis_tests { tf.properties ); - // Documented boundary: the graveyard leg is still an unimplemented gap on - // main; this PR neither closes nor worsens it. - assert_eq!( - crate::game::coverage::card_face_gaps(&face), - vec!["Effect:graveyard".to_string()], - "only the pre-existing graveyard gap should remain" + // The whole card is now supported: no coverage gap remains. + assert!( + crate::game::coverage::card_face_gaps(&face).is_empty(), + "Thought Distortion must be fully supported, gaps: {:?}", + crate::game::coverage::card_face_gaps(&face) ); } diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index ec7e9e1164..fa210834aa 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -1,4 +1,4 @@ -use crate::parser::oracle_nom::error::{OracleError, OracleResult}; +use crate::parser::oracle_nom::error::{oracle_err, OracleError, OracleResult}; use nom::branch::alt; use nom::bytes::complete::{tag, take_till, take_until}; use nom::character::complete::{one_of, space0, space1}; @@ -2653,32 +2653,73 @@ fn parse_zone_word(input: &str) -> nom::IResult<&str, Zone, OracleError<'_>> { } /// Parse output of the multi-zone player-exile recognizer: remaining input paired -/// with the owner axis and the origin-zone union. Named so the inner `nom` -/// combinator signature stays under `clippy::type_complexity`. -type MultiZonePlayerExileParse<'a> = (&'a str, (ControllerRef, Vec)); - -/// CR 400.3 + CR 404.1 + CR 406.2 + CR 108.2: "exile all cards from `` `` and -/// ``" — mass exile of every card a player owns across a *union* of zones -/// (Identity Crisis: "target player's hand and graveyard"). Mirrors the -/// multi-zone origin handling of [`try_parse_multi_zone_same_name_exile`]: the -/// zone union is encoded on the target filter via `InAnyZone`, and the -/// `ChangeZoneAll` resolver reads the multi-zone origin from the filter (so the -/// lowering passes `origin: None`). +/// with the (optional) card-type restriction, the owner axis, and the origin-zone +/// union. Named so the inner `nom` combinator signature stays under +/// `clippy::type_complexity`. +type MultiZonePlayerExileParse<'a> = (&'a str, (Vec, ControllerRef, Vec)); + +/// CR 400.3 + CR 404.1 + CR 406.2 + CR 108.2 + CR 205.2a: "exile all `[]` +/// cards from `` `` and ``" — mass exile of the cards a +/// player owns across a *union* of zones. Two forms: +/// - bare "cards"/"card" (CR 108.2 — every card, any type): Identity Crisis, +/// "exile all cards from target player's hand and graveyard". +/// - type-qualified "`` cards" (CR 205.2a): Thought +/// Distortion, "exile all noncreature, nonland cards from that player's hand +/// and graveyard". /// -/// Returns the owner axis and the origin zones (always `>= 2`). Declines -/// (`None`) on a single zone so the generic single-origin `exile all` path keeps -/// handling those, and on any trailing fragment so nothing is silently dropped. -/// The leading noun is fixed to "cards"/"card" (CR 108.2 — every card, any -/// type); a type-qualified variant ("all creature cards from …") is not claimed. +/// Mirrors the multi-zone origin handling of +/// [`try_parse_multi_zone_same_name_exile`]: the zone union is encoded on the +/// target filter via `InAnyZone`, and the `ChangeZoneAll` resolver reads the +/// multi-zone origin from the filter (so the lowering passes `origin: None`). The +/// owner possessive is parsed into a `ControllerRef` here — the owner scope the +/// filter carries, so the exile is confined to that player's zones. +/// +/// Returns the card-type restriction (empty for the bare form — a semantic no-op +/// preserving the pre-existing representation), the owner axis, and the origin +/// zones (always `>= 2`). Declines (`None`) on a single zone so the generic +/// single-origin `exile all` path keeps handling those, and on any trailing +/// fragment so nothing is silently dropped. pub(super) fn try_parse_multi_zone_player_exile( rest_lower: &str, -) -> Option<(ControllerRef, Vec)> { +) -> Option<(Vec, ControllerRef, Vec)> { fn run(input: &str) -> Result, nom::Err>> { - let (input, _) = alt(( + // CR 108.2 vs CR 205.2a: bare "card(s) from" is tried first so its filter + // stays type-restriction-free (byte-identical to before); only a genuine + // " cards from" leading phrase takes the type-phrase branch. + let (input, type_filters) = if let Ok((after, _)) = alt(( tag::<_, _, OracleError<'_>>("cards from "), tag("card from "), )) - .parse(input)?; + .parse(input) + { + (after, Vec::new()) + } else { + // Delimit the head noun phrase at the " from " that introduces the + // owner possessive, then require the head noun to be "card"/"cards" + // (CR 108.2 — we exile CARDS from a player's zones, never battlefield + // permanents like "creatures from …") with a non-empty type qualifier. + let (after_head, head) = take_until::<_, _, OracleError<'_>>(" from ").parse(input)?; + // Guard: the head noun is "card"/"cards" with a NON-empty type + // qualifier before it. The binding is discarded — the parse below + // re-reads the whole head; this only gates out non-card heads. + let _qualifier = head + .strip_suffix(" cards") + .or_else(|| head.strip_suffix(" card")) + .filter(|q| !q.is_empty()) + .ok_or_else(|| oracle_err(head))?; + // The head must be a well-formed type phrase that fully consumes it + // (CR 205.2a/205.3a card-type/subtype union). Parse the WHOLE head + // (" cards") so the "card(s)" noun folds into the filter. + let (tf, head_rem) = parse_type_phrase(head); + let TargetFilter::Typed(tf) = tf else { + return Err(oracle_err(head)); + }; + if !head_rem.trim().is_empty() { + return Err(oracle_err(head_rem)); + } + let (input, _) = tag::<_, _, OracleError<'_>>(" from ").parse(after_head)?; + (input, tf.type_filters) + }; let (input, owner) = alt(( value( ControllerRef::ParentTargetOwner, @@ -2716,9 +2757,9 @@ pub(super) fn try_parse_multi_zone_player_exile( } input = after_zone; } - Ok((input, (owner, zones))) + Ok((input, (type_filters, owner, zones))) } - let (rem, (owner, zones)) = run(rest_lower).ok()?; + let (rem, (type_filters, owner, zones)) = run(rest_lower).ok()?; if zones.len() < 2 { return None; } @@ -2728,7 +2769,7 @@ pub(super) fn try_parse_multi_zone_player_exile( if !tail.is_empty() { return None; } - Some((owner, zones)) + Some((type_filters, owner, zones)) } /// CR 406.2 + CR 404.1 + CR 108.2 + CR 608.2f: "exile all `` and @@ -8596,14 +8637,14 @@ pub(super) fn parse_exile_ast( // orphans the trailing " and " as an unsupported child clause. The // zone union rides on the target filter via `InAnyZone`; `origin: None` // defers to it, matching the `MultiZoneSameNameExile` lowering. - if let Some((owner, zones)) = try_parse_multi_zone_player_exile(rest_lower) { + if let Some((type_filters, owner, zones)) = try_parse_multi_zone_player_exile(rest_lower) { return Some(ZoneCounterImperativeAst::Exile { origin: None, - target: TargetFilter::Typed( - TypedFilter::default() - .controller(owner) - .properties(vec![crate::types::ability::FilterProp::InAnyZone { zones }]), - ), + target: TargetFilter::Typed(TypedFilter { + type_filters, + controller: Some(owner), + properties: vec![crate::types::ability::FilterProp::InAnyZone { zones }], + }), all: true, enter_with_counters: vec![], multi_target: None, diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 8b6ad53deb..7fb09a3c85 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -15768,18 +15768,20 @@ fn try_parse_verb_and_target<'a>( // "graveyard" conjunct. Claiming the whole clause here (empty remainder) // keeps it single — the actual `ChangeZoneAll { InAnyZone }` is built by // `parse_exile_ast`, which mirrors this recognizer. - if let Some((owner, zones)) = imperative::try_parse_multi_zone_player_exile(rest_lower) { + if let Some((type_filters, owner, zones)) = + imperative::try_parse_multi_zone_player_exile(rest_lower) + { return Some(( TargetedImperativeAst::ZoneCounterProxy(Box::new( ZoneCounterImperativeAst::Exile { origin: None, - target: TargetFilter::Typed( - crate::types::ability::TypedFilter::default() - .controller(owner) - .properties(vec![crate::types::ability::FilterProp::InAnyZone { - zones, - }]), - ), + target: TargetFilter::Typed(crate::types::ability::TypedFilter { + type_filters, + controller: Some(owner), + properties: vec![crate::types::ability::FilterProp::InAnyZone { + zones, + }], + }), all: true, enter_with_counters: vec![], multi_target: None, diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index eb09a67637..84a5c7910e 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -39718,11 +39718,29 @@ fn identity_crisis_parses_multi_zone_player_exile() { #[test] fn multi_zone_player_exile_matcher_recognizes_zone_union() { use crate::types::ability::ControllerRef; + // Bare "cards" form (CR 108.2): no type restriction (empty type_filters). assert_eq!( super::imperative::try_parse_multi_zone_player_exile( "cards from target player's hand and graveyard." ), Some(( + vec![], + ControllerRef::TargetPlayer, + vec![Zone::Hand, Zone::Graveyard] + )) + ); + // Type-qualified form (CR 205.2a): carries the noncreature/nonland restriction + // AND the owner scope AND the zone union (Thought Distortion). + assert_eq!( + super::imperative::try_parse_multi_zone_player_exile( + "noncreature, nonland cards from that player's hand and graveyard." + ), + Some(( + vec![ + TypeFilter::Card, + TypeFilter::Non(Box::new(TypeFilter::Creature)), + TypeFilter::Non(Box::new(TypeFilter::Land)), + ], ControllerRef::TargetPlayer, vec![Zone::Hand, Zone::Graveyard] )) diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index fce0ff7d45..8c49e7bd3d 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -910,6 +910,7 @@ mod the_immortal_sun; mod the_kingpin_of_crime_combat_damage; mod the_ur_dragon_eminence; mod the_who_opponent_guess_resolution; +mod thought_distortion; mod thoughtweft_trample_regression; mod throne_of_eldraine_mana_riders; mod throw_instead_tail_class; diff --git a/crates/engine/tests/integration/thought_distortion.rs b/crates/engine/tests/integration/thought_distortion.rs new file mode 100644 index 0000000000..cb90450e4e --- /dev/null +++ b/crates/engine/tests/integration/thought_distortion.rs @@ -0,0 +1,122 @@ +//! Runtime pipeline coverage — Thought Distortion ({4}{B}{B} sorcery). +//! +//! Verbatim Oracle text (Scryfall oracle_id 5f089ac6-9e92-4ec2-bf46-a0b08d1e2979): +//! "This spell can't be countered. +//! Target opponent reveals their hand. Exile all noncreature, nonland cards +//! from that player's hand and graveyard." +//! +//! This is the discriminating regression for PR #6940's owner-scoped, +//! type-restricted, multi-zone exile: the exile must move ONLY the targeted +//! opponent's noncreature, nonland cards, and only from that player's hand and +//! graveyard. The controls prove all three scopes at once: +//! - OWNER scope (CR 400.3): the caster's own noncreature/nonland cards, in the +//! same zones, must NOT move. +//! - TYPE restriction (CR 205.2a): the target's creature and land cards must +//! NOT move. +//! - ZONE union (CR 402.1 + CR 404.1): the target's qualifying cards move from +//! BOTH hand and graveyard. +//! +//! Before this PR the "and graveyard" leg was an `Unimplemented` no-op, so the +//! graveyard assertions are the revert-failing authority. + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::mana::{ManaColor, ManaCost, ManaCostShard}; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +const THOUGHT_DISTORTION: &str = "This spell can't be countered.\n\ + Target opponent reveals their hand. Exile all noncreature, nonland cards from that player's hand and graveyard."; + +#[test] +fn thought_distortion_exiles_only_the_targeted_opponents_noncreature_nonland_cards() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + // Caster (P0) casts, targeting opponent P1. + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Thought Distortion", false, THOUGHT_DISTORTION) + .with_mana_cost(ManaCost::Cost { + shards: vec![ManaCostShard::Black, ManaCostShard::Black], + generic: 4, + }) + .id(); + + // --- Target opponent (P1): the cards that SHOULD move, plus type controls. --- + let opp_hand_noncreature = scenario + .add_spell_to_hand(P1, "Opp Hand Instant", true) + .id(); + let opp_hand_creature = scenario + .add_creature_to_hand(P1, "Opp Hand Bear", 2, 2) + .id(); + let opp_hand_land = scenario.add_land_to_hand(P1, "Opp Hand Forest").id(); + let opp_gy_noncreature = scenario + .add_spell_to_graveyard(P1, "Opp GY Instant", true) + .id(); + let opp_gy_creature = scenario + .add_creature_to_graveyard(P1, "Opp GY Bear", 2, 2) + .id(); + + // --- Caster (P0): owner-scope controls — must NOT move. --- + let my_hand_noncreature = scenario.add_spell_to_hand(P0, "My Hand Instant", true).id(); + let my_gy_noncreature = scenario + .add_spell_to_graveyard(P0, "My GY Instant", true) + .id(); + + // Fund {4}{B}{B} so the real cost is paid from the battlefield. + for _ in 0..6 { + scenario.add_basic_land(P0, ManaColor::Black); + } + + let mut runner = scenario.build(); + + let outcome = runner.cast(spell).target_player(P1).resolve(); + + // --- The targeted opponent's noncreature, nonland cards, from BOTH zones. --- + assert_eq!( + outcome.zone_of(opp_hand_noncreature), + Zone::Exile, + "the target opponent's noncreature/nonland HAND card must be exiled" + ); + assert_eq!( + outcome.zone_of(opp_gy_noncreature), + Zone::Exile, + "the target opponent's noncreature/nonland GRAVEYARD card must be exiled \ + (the revert-failing 'and graveyard' leg)" + ); + + // --- Type controls: the target's creature/land cards stay put. --- + assert_eq!( + outcome.zone_of(opp_hand_creature), + Zone::Hand, + "a creature card is not noncreature — it must stay in the target's hand" + ); + assert_eq!( + outcome.zone_of(opp_hand_land), + Zone::Hand, + "a land card is not nonland — it must stay in the target's hand" + ); + assert_eq!( + outcome.zone_of(opp_gy_creature), + Zone::Graveyard, + "a creature card must stay in the target's graveyard" + ); + + // --- Owner-scope controls: the CASTER's own qualifying cards never move. --- + assert_eq!( + outcome.zone_of(my_hand_noncreature), + Zone::Hand, + "the caster's own hand card must not move — the exile is scoped to the target player" + ); + assert_eq!( + outcome.zone_of(my_gy_noncreature), + Zone::Graveyard, + "the caster's own graveyard card must not move — owner scope (CR 400.3)" + ); + + // The spell resolves to its owner's graveyard (an ordinary sorcery). + assert_eq!( + outcome.zone_of(spell), + Zone::Graveyard, + "Thought Distortion is an ordinary sorcery — it goes to its owner's graveyard" + ); +} From 0ace32d0782855e7c6e4c688886f95e689e8df99 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 22:15:27 -0500 Subject: [PATCH 5/7] fix: satisfy parser-combinator gate in multi-zone type-qualified exile The head-noun guard used .strip_suffix(" cards")/(" card") for parse dispatch, which the nom-combinator mandate gate (Gate A) rejects. Drop it: parse_type_phrase must FULLY consume the delimited head, and the owner-possessive + 2-zone-union structure is already the discriminator, so the manual suffix guard was redundant. Behavior and all type_filters are unchanged (parse_type_phrase output is identical). Co-Authored-By: Claude Opus 4.8 --- .../src/parser/oracle_effect/imperative.rs | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index fa210834aa..ea2f7c86be 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -2694,22 +2694,15 @@ pub(super) fn try_parse_multi_zone_player_exile( { (after, Vec::new()) } else { - // Delimit the head noun phrase at the " from " that introduces the - // owner possessive, then require the head noun to be "card"/"cards" - // (CR 108.2 — we exile CARDS from a player's zones, never battlefield - // permanents like "creatures from …") with a non-empty type qualifier. + // Type-qualified head " card(s) from …" (CR 205.2a + + // CR 205.3a). Delimit the head noun phrase at the " from " that + // introduces the owner possessive, parse it as a full type phrase, and + // require it to be FULLY consumed — a malformed or non-type head (e.g. + // "creatures except X from …") leaves a remainder and is declined, so + // only a clean " card(s)" head is claimed. The + // owner-possessive + 2-zone-union structure parsed below is the rest of + // the discriminator (CR 108.2 — cards in a player's zones). let (after_head, head) = take_until::<_, _, OracleError<'_>>(" from ").parse(input)?; - // Guard: the head noun is "card"/"cards" with a NON-empty type - // qualifier before it. The binding is discarded — the parse below - // re-reads the whole head; this only gates out non-card heads. - let _qualifier = head - .strip_suffix(" cards") - .or_else(|| head.strip_suffix(" card")) - .filter(|q| !q.is_empty()) - .ok_or_else(|| oracle_err(head))?; - // The head must be a well-formed type phrase that fully consumes it - // (CR 205.2a/205.3a card-type/subtype union). Parse the WHOLE head - // (" cards") so the "card(s)" noun folds into the filter. let (tf, head_rem) = parse_type_phrase(head); let TargetFilter::Typed(tf) = tf else { return Err(oracle_err(head)); From 25a88b9c965d3a361a159dd0dd2b1ddc232725b3 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 22:31:53 -0500 Subject: [PATCH 6/7] test: prove nonland restriction on Thought Distortion's graveyard origin Per review: the runtime regression only proved 'nonland' for the hand origin (the sole land control was in P1's hand); a regression dropping Non(Land) for just the graveyard leg would still pass. Add a P1-owned land card in the graveyard and assert it stays, so the nonland restriction is proven on BOTH origins of the single owner-scoped multi-zone filter. Adds a reusable add_land_to_graveyard scenario helper mirroring add_creature_to_graveyard. Co-Authored-By: Claude Opus 4.8 --- crates/engine/src/game/scenario.rs | 22 +++++++++++++++++++ .../tests/integration/thought_distortion.rs | 9 ++++++++ 2 files changed, 31 insertions(+) diff --git a/crates/engine/src/game/scenario.rs b/crates/engine/src/game/scenario.rs index d9df01b955..13b5708793 100644 --- a/crates/engine/src/game/scenario.rs +++ b/crates/engine/src/game/scenario.rs @@ -560,6 +560,28 @@ impl GameScenario { } } + /// Add a land card to a player's graveyard (CR 404). Returns a `CardBuilder` + /// for fluent chaining. Mirrors [`Self::add_creature_to_graveyard`] — used to + /// stage `nonland`/type-restricted graveyard-exile controls. + pub fn add_land_to_graveyard(&mut self, player: PlayerId, name: &str) -> CardBuilder<'_> { + let card_id = CardId(self.state.next_object_id); + let id = create_object( + &mut self.state, + card_id, + player, + name.to_string(), + Zone::Graveyard, + ); + let obj = self.state.objects.get_mut(&id).unwrap(); + obj.card_types.core_types.push(CoreType::Land); + obj.base_card_types = obj.card_types.clone(); + + CardBuilder { + state: &mut self.state, + id, + } + } + /// Add a creature card to a player's exile. Returns a `CardBuilder` for /// fluent chaining. Used to stage cards tracked by source-linked exile /// effects. diff --git a/crates/engine/tests/integration/thought_distortion.rs b/crates/engine/tests/integration/thought_distortion.rs index cb90450e4e..950f3b4501 100644 --- a/crates/engine/tests/integration/thought_distortion.rs +++ b/crates/engine/tests/integration/thought_distortion.rs @@ -55,6 +55,9 @@ fn thought_distortion_exiles_only_the_targeted_opponents_noncreature_nonland_car let opp_gy_creature = scenario .add_creature_to_graveyard(P1, "Opp GY Bear", 2, 2) .id(); + // A land card in the SAME graveyard: proves the `nonland` restriction is + // enforced on the graveyard origin too, not just hand (the filter spans both). + let opp_gy_land = scenario.add_land_to_graveyard(P1, "Opp GY Swamp").id(); // --- Caster (P0): owner-scope controls — must NOT move. --- let my_hand_noncreature = scenario.add_spell_to_hand(P0, "My Hand Instant", true).id(); @@ -100,6 +103,12 @@ fn thought_distortion_exiles_only_the_targeted_opponents_noncreature_nonland_car Zone::Graveyard, "a creature card must stay in the target's graveyard" ); + assert_eq!( + outcome.zone_of(opp_gy_land), + Zone::Graveyard, + "a land card is not nonland — it must stay in the target's GRAVEYARD \ + (proves the nonland restriction on the graveyard origin)" + ); // --- Owner-scope controls: the CASTER's own qualifying cards never move. --- assert_eq!( From 919094646178ec7c90e3a4d3809a3eba5d669387 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 23:28:41 -0500 Subject: [PATCH 7/7] docs: drop unverified CR 601.2c from Thought Distortion parser test Per review: CR 601.2c governs choosing targets during casting, not the subsequent "that player's" anaphoric binding the comment attached it to. Remove the citation from both the doc comment and the inline reveal-assertion comment; describe the reveal->exile owner linkage as parser-chain/anaphora resolution (template behavior, not a numbered rule). Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 --- crates/engine/src/database/synthesis.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/database/synthesis.rs b/crates/engine/src/database/synthesis.rs index df6afc8514..8cba812eb8 100644 --- a/crates/engine/src/database/synthesis.rs +++ b/crates/engine/src/database/synthesis.rs @@ -10772,8 +10772,9 @@ mod cycling_synthesis_tests { /// not a hand-only wipe plus an orphaned `Unimplemented { "graveyard" }` (the /// pre-PR parse), and never the mis-parse that injected `InZone(Battlefield)` /// and dropped the noncreature/nonland restriction. The asserted shape: - /// - `RevealHand` targeting the Opponent (the target that "that player" - /// anaphorically binds to, CR 601.2c), + /// - `RevealHand` targeting the Opponent (the parse-chain antecedent that + /// the exile's "that player" anaphor resolves to — template/anaphora + /// resolution, not a numbered rule), /// - a single `ChangeZoneAll` to Exile with `origin: None` (the zone union /// rides on the filter) whose `Typed` filter carries the /// `Non(Creature)`/`Non(Land)` restriction (CR 205.2a), the @@ -10823,8 +10824,8 @@ mod cycling_synthesis_tests { let face = build_oracle_face(&mtgjson, None); - // The reveal establishes the owner binding: the exile is scoped to the - // targeted OPPONENT's hand (CR 601.2c target opponent). + // The reveal names the target opponent; the exile's "that player" anaphor + // resolves to that same antecedent (parser-chain behavior, not a CR rule). let reveal = face .abilities .iter()