From 9304d40ad6ad316921918c9c5969ba88634a5d37 Mon Sep 17 00:00:00 2001 From: Lourince Daging Date: Tue, 14 Jul 2026 03:43:50 +0200 Subject: [PATCH 1/3] fix(parser): return Auras via "do the same for cards" (Estrid) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "..., then do the same for cards" replicates the immediately-preceding mass zone-change for a sibling card type. Estrid, the Masked's ult — "Return all non-Aura enchantment cards from your graveyard to the battlefield, then do the same for Aura cards." — dropped the Aura return entirely (#4779): the comma-"then do the same" tail was glued into the first clause, and the "do the same for " verb was never recognized. Parser-only, no new engine variant: - split_comma_clause_boundary: treat ", then do the same for " as a Then boundary. The "do the same" verb is not in the imperative-verb table, so — mirroring the villainous-choice guard directly above — the continuation was glued into the prior clause and dropped. - new try_parse_do_the_same_for_type recognizer + chunk-loop dispatch that clones the antecedent sibling effect and swaps its type filter. This is the same antecedent-clone mechanic try_parse_scoped_does_the_same uses for the player-scoped fan-out, so it emits an ordinary sibling Effect (no disposition, resolver, or scope added). Estrid now emits both returns: non-Aura enchantments, then Auras — zones and controller preserved (CR 608.2c: the antecedent action is replicated modulo the stated type substitution). Building-block level: covers the "do the same for " clause class, not Estrid alone. Closes #4779. --- crates/engine/src/parser/oracle_effect/mod.rs | 34 ++++++ .../src/parser/oracle_effect/sequence.rs | 62 ++++++++++- .../engine/src/parser/oracle_effect/tests.rs | 101 ++++++++++++++++++ 3 files changed, 196 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 830f0af767..5d0bce4999 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -29533,6 +29533,40 @@ pub(crate) fn parse_effect_chain_ir( .push(); continue; } + + // CR 608.2c: "[then] do the same for cards/permanents." — + // Estrid, the Masked: "Return all non-Aura enchantment cards … to + // the battlefield, then do the same for Aura cards." Clone the + // antecedent sibling effect and swap its type filter for the stated + // type (the same clone mechanic as the scoped fan-out above), so the + // repeated action lands on the sibling type without a new disposition + // or engine variant. Placed after the scoped/before the targeted + // forms; the three subjects are disjoint ("each …" / "for " / + // "target opponent …"). + if let Some(new_filter) = sequence::try_parse_do_the_same_for_type(normalized_text) { + let new_type_filters = match new_filter { + TargetFilter::Typed(t) => t.type_filters, + _ => Vec::new(), + }; + let mut cloned = prev_effect; + each_target_filter_mut(&mut cloned, &mut |tf| { + if let TargetFilter::Typed(existing) = tf { + existing.type_filters = new_type_filters.clone(); + } + }); + builder + .clause( + normalized_text, + parsed_clause(cloned), + chunk.boundary_after, + ClauseDisposition::Emit { + followup: None, + intrinsic: None, + }, + ) + .push(); + continue; + } } // CR 608.2c + CR 601.2c: "[then] target opponent does the same / does diff --git a/crates/engine/src/parser/oracle_effect/sequence.rs b/crates/engine/src/parser/oracle_effect/sequence.rs index b77d96243a..71bac9f7c9 100644 --- a/crates/engine/src/parser/oracle_effect/sequence.rs +++ b/crates/engine/src/parser/oracle_effect/sequence.rs @@ -12,7 +12,7 @@ use super::super::oracle_nom::enters_under::{ }; use super::super::oracle_nom::primitives as nom_primitives; use super::super::oracle_nom::primitives::parse_keyword_name; -use super::super::oracle_target::{parse_target, parse_target_with_ctx}; +use super::super::oracle_target::{parse_target, parse_target_with_ctx, parse_type_phrase}; use super::super::oracle_util::{contains_possessive, parse_count_expr, TextPair}; use super::{apply_where_x_to_filter, strip_trailing_where_x}; use crate::parser::oracle_ir::ast::*; @@ -1734,6 +1734,21 @@ fn split_comma_clause_boundary(current: &str, remainder: &str) -> Option<(Clause { return Some((ClauseBoundary::Then, whitespace_len + "then ".len())); } + // CR 608.2c: "..., then do the same for cards" continues the + // antecedent action for a sibling card type (Estrid, the Masked: "Return + // all non-Aura enchantment cards ... to the battlefield, then do the same + // for Aura cards."). The "do the same" verb is not in the imperative-verb + // table `starts_clause_text_or_conjugated` checks, so — exactly like the + // villainous-choice guard above — without this the continuation is glued + // into the prior clause and silently dropped (#4779: Auras never return). + // The split-off chunk is consumed by `try_parse_do_the_same_for_type`, + // which clones the antecedent effect with the swapped type. + if tag::<_, _, OracleError<'_>>("do the same for ") + .parse(after_then_lower) + .is_ok() + { + return Some((ClauseBoundary::Then, whitespace_len + "then ".len())); + } if starts_clause_text_or_conjugated(after_then) || starts_you_control_subject_predicate(after_then_lower) || starts_with_damage_clause(after_then_lower) @@ -7933,6 +7948,51 @@ pub(super) fn try_parse_repeat_process_for_keywords(text: &str) -> Option cards/permanents." — an +/// effect-replication directive that repeats the immediately-preceding sibling +/// action for a DIFFERENT card type (Estrid, the Masked: "Return all non-Aura +/// enchantment cards from your graveyard to the battlefield, then do the same +/// for Aura cards."). Returns the new type filter; the chunk loop clones the +/// antecedent effect and swaps its `type_filters` for these — the same +/// antecedent-clone mechanic `try_parse_scoped_does_the_same` uses for the +/// player-scoped fanout, so no new disposition, effect variant, or resolver is +/// needed (both produce an ordinary sibling `Effect`). +/// +/// Distinct from the two sibling forms: the keyword-list form +/// (`try_parse_repeat_process_for_keywords`, tried first, replicates per +/// keyword) and the target-opponent form (`try_parse_does_the_same_clause`, +/// deferred — the opponent acts on their OWN objects via a mid-chain target +/// slot). This form is the SAME action on the SAME zones for a sibling type, so +/// a straight clone-and-retype is rules-correct (CR 608.2c: the antecedent +/// action is replicated verbatim modulo the stated substitution). Covers the +/// class ("do the same for "), not Estrid alone. +/// +/// Combinators only: `opt`/`tag`/`alt` for the prefix, then the shared +/// `parse_type_phrase` for the filter. Requires the phrase to be fully consumed +/// (modulo a trailing period) by a non-empty typed filter, so unrelated +/// "do/repeat …" tails fall through to normal dispatch rather than being +/// swallowed. +pub(super) fn try_parse_do_the_same_for_type(text: &str) -> Option { + let lower = text.to_lowercase(); + let ((), rest) = nom_on_lower(text, &lower, |i| { + let (i, _) = opt(tag("then ")).parse(i)?; + let (i, _) = alt(( + tag::<_, _, OracleError<'_>>("do the same for "), + tag("repeat this process for "), + )) + .parse(i)?; + Ok((i, ())) + })?; + let (filter, remainder) = parse_type_phrase(rest.trim()); + if !remainder.trim().trim_end_matches('.').trim().is_empty() { + return None; + } + match &filter { + TargetFilter::Typed(t) if !t.type_filters.is_empty() => Some(filter), + _ => None, + } +} + /// CR 608.2c + CR 601.2c: Parse "[then] target opponent does the same / does so." /// — an effect-replication directive (The Wedding of River Song). The clause has /// no effect of its own; it *would* replicate the immediately-preceding sibling diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index 9df10f6084..d0ed34181c 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -46627,6 +46627,107 @@ fn repeat_process_directive_ignores_non_repeat_flip_branch() { ); } +/// CR 608.2c: "..., then do the same for cards" replicates the antecedent +/// mass zone-change for a sibling card type — the antecedent action is repeated +/// verbatim modulo the stated type substitution, preserving zones and controller. +/// Estrid, the Masked's ult ("Return all non-Aura enchantment cards from your +/// graveyard to the battlefield, then do the same for Aura cards.") must emit TWO +/// returns: the non-Aura enchantments, then the Auras. +/// +/// Building-block level: exercises the "do the same for " clause class, not +/// Estrid alone. Regression guard for #4779 (Auras never returned because the +/// comma-"then do the same" tail was glued into the first clause and dropped). +#[test] +fn do_the_same_for_type_replicates_mass_zone_change_with_swapped_type() { + use crate::types::zones::Zone; + let parsed = parse_oracle_text( + "Return all non-Aura enchantment cards from your graveyard to the battlefield, then do the same for Aura cards.", + "Estrid, the Masked", + &[], + &["Planeswalker".to_string()], + &[], + ); + assert_eq!( + parsed.abilities.len(), + 1, + "expected one ability, got {:?}", + parsed.abilities + ); + let primary = &parsed.abilities[0]; + + // Primary: return all NON-Aura enchantment cards, graveyard -> battlefield. + let Effect::ChangeZoneAll { + origin, + destination, + target, + .. + } = &*primary.effect + else { + panic!("primary must be ChangeZoneAll, got {:?}", primary.effect); + }; + assert_eq!(*origin, Some(Zone::Graveyard)); + assert_eq!(*destination, Zone::Battlefield); + let TargetFilter::Typed(pt) = target else { + panic!("primary target must be Typed, got {target:?}"); + }; + assert!( + pt.type_filters.iter().any(|t| matches!( + t, + TypeFilter::Non(inner) if matches!(&**inner, TypeFilter::Subtype(s) if s == "Aura") + )), + "primary must exclude Auras (Non(Aura)), got {:?}", + pt.type_filters + ); + + // Sub-ability: the "do the same for Aura cards" clone — same zones and + // controller as the antecedent, with the type swapped to Aura and the + // Non(Aura) exclusion dropped. + let sub = primary + .sub_ability + .as_ref() + .expect("expected a 'do the same for Aura cards' sub-ability"); + let Effect::ChangeZoneAll { + origin: sub_origin, + destination: sub_dest, + target: sub_target, + .. + } = &*sub.effect + else { + panic!("sub-ability must be ChangeZoneAll, got {:?}", sub.effect); + }; + assert_eq!( + *sub_origin, + Some(Zone::Graveyard), + "Aura return must keep the graveyard origin" + ); + assert_eq!( + *sub_dest, + Zone::Battlefield, + "Aura return must keep the battlefield destination" + ); + let TargetFilter::Typed(st) = sub_target else { + panic!("sub target must be Typed, got {sub_target:?}"); + }; + assert!( + st.type_filters + .iter() + .any(|t| matches!(t, TypeFilter::Subtype(s) if s == "Aura")), + "sub-ability must return Aura cards, got {:?}", + st.type_filters + ); + assert!( + !st.type_filters + .iter() + .any(|t| matches!(t, TypeFilter::Non(_))), + "sub-ability must NOT inherit the antecedent's Non(Aura) exclusion, got {:?}", + st.type_filters + ); + assert_eq!( + st.controller, pt.controller, + "Aura return must keep the antecedent's controller (You)" + ); +} + /// Unleash the Flux (Phenomenon) — full-card parse drops zero `Unimplemented` /// nodes; the each-player-sacrifice root carries the unbounded `WhileCondition` /// gated on losing the flip, and the flip sub-ability has no leftover branch. From dd05342b71c3df5a7a7863743b821232476f924e Mon Sep 17 00:00:00 2001 From: Lourince Daging Date: Tue, 14 Jul 2026 07:03:14 +0200 Subject: [PATCH 2/3] fix(parser): narrow "do the same for " to a clean type substitution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review on #5774: the general path must not touch continuations it cannot model. Three narrowings keep the blast radius to exactly the handled class (Estrid); every richer sibling stays exactly as before and strict-fails until its full grammar lands (CR #1: a flagged gap beats a silent misparse). - Recognizer (try_parse_do_the_same_for_type): drop the broader "repeat this process for" family; accept ONLY a pure card-type substitution — reject any filter carrying a FilterProp predicate (Gruesome Menageries --- crates/engine/src/parser/oracle_effect/mod.rs | 33 ++++--- .../src/parser/oracle_effect/sequence.rs | 90 ++++++++++++++++--- 2 files changed, 98 insertions(+), 25 deletions(-) diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 5d0bce4999..e8f79d15c8 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -29549,23 +29549,32 @@ pub(crate) fn parse_effect_chain_ir( _ => Vec::new(), }; let mut cloned = prev_effect; + // Retype every `Typed` target the antecedent exposes (Estrid's + // mass `ChangeZoneAll`). Only emit the clone when a substitution + // actually happened: an antecedent with no `Typed` target (an + // `Unimplemented` head, a bare player effect) must NOT be cloned + // verbatim — fall through to a documented strict-failure instead. + let mut swapped = false; each_target_filter_mut(&mut cloned, &mut |tf| { if let TargetFilter::Typed(existing) = tf { existing.type_filters = new_type_filters.clone(); + swapped = true; } }); - builder - .clause( - normalized_text, - parsed_clause(cloned), - chunk.boundary_after, - ClauseDisposition::Emit { - followup: None, - intrinsic: None, - }, - ) - .push(); - continue; + if swapped { + builder + .clause( + normalized_text, + parsed_clause(cloned), + chunk.boundary_after, + ClauseDisposition::Emit { + followup: None, + intrinsic: None, + }, + ) + .push(); + continue; + } } } diff --git a/crates/engine/src/parser/oracle_effect/sequence.rs b/crates/engine/src/parser/oracle_effect/sequence.rs index 71bac9f7c9..05bf70dca4 100644 --- a/crates/engine/src/parser/oracle_effect/sequence.rs +++ b/crates/engine/src/parser/oracle_effect/sequence.rs @@ -1741,12 +1741,18 @@ fn split_comma_clause_boundary(current: &str, remainder: &str) -> Option<(Clause // table `starts_clause_text_or_conjugated` checks, so — exactly like the // villainous-choice guard above — without this the continuation is glued // into the prior clause and silently dropped (#4779: Auras never return). - // The split-off chunk is consumed by `try_parse_do_the_same_for_type`, - // which clones the antecedent effect with the swapped type. - if tag::<_, _, OracleError<'_>>("do the same for ") - .parse(after_then_lower) - .is_ok() - { + // + // Gate the split on the SAME recognizer the chunk loop uses + // (`try_parse_do_the_same_for_type`), so ONLY a clean pure-type + // substitution is split off. Richer forms this PR does not model — + // Gruesome Menagerie's "creature cards with mana value 2 and 3" + // (`FilterProp` predicate) and Grim Captain's Call's "Vampire, Dinosaur, + // and Merfolk" (type list) — fail the recognizer and stay glued exactly + // as before, keeping this change's blast radius to the handled class. + // The first-sentence slice bounds the recognizer's whole-consumption + // check to this clause (later sentences are chunked separately). + let do_the_same_head = trimmed.split('.').next().unwrap_or(trimmed); + if try_parse_do_the_same_for_type(do_the_same_head).is_some() { return Some((ClauseBoundary::Then, whitespace_len + "then ".len())); } if starts_clause_text_or_conjugated(after_then) @@ -7976,19 +7982,27 @@ pub(super) fn try_parse_do_the_same_for_type(text: &str) -> Option let lower = text.to_lowercase(); let ((), rest) = nom_on_lower(text, &lower, |i| { let (i, _) = opt(tag("then ")).parse(i)?; - let (i, _) = alt(( - tag::<_, _, OracleError<'_>>("do the same for "), - tag("repeat this process for "), - )) - .parse(i)?; + let (i, _) = tag::<_, _, OracleError<'_>>("do the same for ").parse(i)?; Ok((i, ())) })?; - let (filter, remainder) = parse_type_phrase(rest.trim()); + let (filter, remainder) = parse_type_phrase(rest.trim().trim_end_matches('.').trim()); if !remainder.trim().trim_end_matches('.').trim().is_empty() { return None; } + // Only a PURE card-type substitution is modeled: the continuation swaps the + // antecedent's `type_filters` wholesale (Estrid: non-Aura enchantment → Aura). + // Reject any filter that also carries `FilterProp` predicates (Gruesome + // Menagerie's "creature cards with mana value 2 and 3") or a `controller` + // scope — those need a full replacement-filter/cardinality grammar and must + // stay strict-failing until it lands (CR #1: a flagged gap beats a misparse). + // The multi-type list form (Grim Captain's Call's "Vampire, Dinosaur, and + // Merfolk") is already rejected by the non-empty `remainder` guard above. match &filter { - TargetFilter::Typed(t) if !t.type_filters.is_empty() => Some(filter), + TargetFilter::Typed(t) + if !t.type_filters.is_empty() && t.properties.is_empty() && t.controller.is_none() => + { + Some(filter) + } _ => None, } } @@ -8278,6 +8292,56 @@ mod tests { } } + // CR 608.2c: the "do the same for " continuation recognizer accepts + // ONLY a clean, whole card-type substitution (Estrid's "Aura cards") — the + // chunk loop clones the antecedent effect and swaps just its `type_filters`. + #[test] + fn do_the_same_for_type_accepts_clean_type_substitution() { + match try_parse_do_the_same_for_type("then do the same for Aura cards.") { + Some(TargetFilter::Typed(t)) => { + assert!( + t.type_filters + .iter() + .any(|f| matches!(f, TypeFilter::Subtype(s) if s == "Aura")), + "expected an Aura type substitution, got {:?}", + t.type_filters + ); + assert!( + t.properties.is_empty(), + "must carry no FilterProp predicate" + ); + assert!(t.controller.is_none(), "must carry no controller scope"); + } + other => panic!("expected a clean Aura type substitution, got {other:?}"), + } + assert!( + try_parse_do_the_same_for_type("do the same for creature cards").is_some(), + "a bare creature-card substitution is also clean" + ); + } + + // Guard: continuations carrying a `FilterProp` predicate, a multi-type list, + // or the broader "repeat this process for" family are NOT modeled by the + // type-substitution path and must be rejected, so they stay strict-failing + // until the full replacement-filter/cardinality grammar lands (Gruesome + // Menagerie, Grim Captain's Call, Firemind's Foresight) — CR #1: a flagged + // gap beats a silent misparse. + #[test] + fn do_the_same_for_type_rejects_unmodeled_continuations() { + for phrasing in [ + "do the same for creature cards with mana value 2 and 3", + "do the same for Vampire, Dinosaur, and Merfolk", + "do the same for creature cards with flying", + "repeat this process for instant cards", + ] { + assert_eq!( + try_parse_do_the_same_for_type(phrasing), + None, + "must reject the unmodeled continuation {phrasing:?}" + ); + } + } + // Guard: the recognizer must NOT swallow unrelated "same" phrases or a // "does the same for " player-set/category fanout (deferred), so // they fall through to normal dispatch instead of being silently dropped. From 34bc8e23efb8ac7244fc6069a882644072563277 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Sat, 1 Aug 2026 17:50:05 -0700 Subject: [PATCH 3/3] fix(parser): preserve filtered partitions in do-the-same clauses --- crates/engine/src/parser/oracle_effect/mod.rs | 37 ++++- .../src/parser/oracle_effect/sequence.rs | 83 +++++++--- .../engine/src/parser/oracle_effect/tests.rs | 154 ++++++++++++++++++ .../issue_4779_do_same_for_type.rs | 72 ++++++++ crates/engine/tests/integration/main.rs | 1 + 5 files changed, 313 insertions(+), 34 deletions(-) create mode 100644 crates/engine/tests/integration/issue_4779_do_same_for_type.rs diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index e8f79d15c8..44507b31a3 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -26049,6 +26049,30 @@ pub(crate) fn each_target_filter_mut(effect: &mut Effect, f: &mut impl FnMut(&mu } } +/// Replace the card-type predicate in a direct card selector while preserving +/// a tracked-set provenance wrapper when it is the direct selector. +/// +/// CR 608.2c: a "do the same for " continuation repeats the antecedent +/// instruction with a different card-type restriction. `TrackedSetFiltered` +/// records a prior action's selected/revealed set, so only its nested predicate +/// changes; replacing the wrapper would discard the "this way" provenance. +fn replace_type_filters(filter: &mut TargetFilter, replacement: &[TypeFilter]) -> bool { + match filter { + TargetFilter::Typed(typed) => { + typed.type_filters = replacement.to_vec(); + true + } + TargetFilter::TrackedSetFiltered { filter, .. } => { + let TargetFilter::Typed(typed) = filter.as_mut() else { + return false; + }; + typed.type_filters = replacement.to_vec(); + true + } + _ => false, + } +} + /// CR 608.2 + CR 107.2: Rewrite target-scoped `QuantityRef` variants to their /// controller-scoped equivalents across an ability tree. Under /// `player_scope: All` / `Opponent` / etc., the resolver rebinds @@ -29543,11 +29567,9 @@ pub(crate) fn parse_effect_chain_ir( // or engine variant. Placed after the scoped/before the targeted // forms; the three subjects are disjoint ("each …" / "for " / // "target opponent …"). - if let Some(new_filter) = sequence::try_parse_do_the_same_for_type(normalized_text) { - let new_type_filters = match new_filter { - TargetFilter::Typed(t) => t.type_filters, - _ => Vec::new(), - }; + if let Some(new_type_filters) = + sequence::try_parse_do_the_same_for_type(normalized_text) + { let mut cloned = prev_effect; // Retype every `Typed` target the antecedent exposes (Estrid's // mass `ChangeZoneAll`). Only emit the clone when a substitution @@ -29556,10 +29578,7 @@ pub(crate) fn parse_effect_chain_ir( // verbatim — fall through to a documented strict-failure instead. let mut swapped = false; each_target_filter_mut(&mut cloned, &mut |tf| { - if let TargetFilter::Typed(existing) = tf { - existing.type_filters = new_type_filters.clone(); - swapped = true; - } + swapped |= replace_type_filters(tf, &new_type_filters); }); if swapped { builder diff --git a/crates/engine/src/parser/oracle_effect/sequence.rs b/crates/engine/src/parser/oracle_effect/sequence.rs index 05bf70dca4..090aa21fb7 100644 --- a/crates/engine/src/parser/oracle_effect/sequence.rs +++ b/crates/engine/src/parser/oracle_effect/sequence.rs @@ -1749,10 +1749,12 @@ fn split_comma_clause_boundary(current: &str, remainder: &str) -> Option<(Clause // (`FilterProp` predicate) and Grim Captain's Call's "Vampire, Dinosaur, // and Merfolk" (type list) — fail the recognizer and stay glued exactly // as before, keeping this change's blast radius to the handled class. - // The first-sentence slice bounds the recognizer's whole-consumption - // check to this clause (later sentences are chunked separately). - let do_the_same_head = trimmed.split('.').next().unwrap_or(trimmed); - if try_parse_do_the_same_for_type(do_the_same_head).is_some() { + // The complete recognizer covers a terminal continuation. A following + // comma-"then" clause (Glimpse of Tomorrow) is segmented by the same + // grammar, so we do not weaken the pure-type whole-consumption rule. + if try_parse_do_the_same_for_type(trimmed).is_some() + || starts_do_the_same_for_type_before_then(after_then) + { return Some((ClauseBoundary::Then, whitespace_len + "then ".len())); } if starts_clause_text_or_conjugated(after_then) @@ -7958,7 +7960,7 @@ pub(super) fn try_parse_repeat_process_for_keywords(text: &str) -> Option Option Option { +pub(super) fn try_parse_do_the_same_for_type(text: &str) -> Option> { let lower = text.to_lowercase(); let ((), rest) = nom_on_lower(text, &lower, |i| { let (i, _) = opt(tag("then ")).parse(i)?; @@ -7997,11 +7999,35 @@ pub(super) fn try_parse_do_the_same_for_type(text: &str) -> Option // stay strict-failing until it lands (CR #1: a flagged gap beats a misparse). // The multi-type list form (Grim Captain's Call's "Vampire, Dinosaur, and // Merfolk") is already rejected by the non-empty `remainder` guard above. - match &filter { + pure_type_substitution(filter) +} + +/// Recognize a pure type-substitution segment when it is immediately followed +/// by another comma-"then" clause in the same sentence. The separator is parsed +/// as grammar, rather than manually slicing the sentence, so the terminal +/// recognizer remains strict about its complete input. +fn starts_do_the_same_for_type_before_then(text: &str) -> bool { + let lower = text.to_lowercase(); + let Ok((_, type_text)) = preceded( + tag::<_, _, OracleError<'_>>("do the same for "), + terminated(take_until(", then "), tag(", then ")), + ) + .parse(lower.as_str()) else { + return false; + }; + let (filter, remainder) = parse_type_phrase(type_text.trim()); + remainder.trim().is_empty() && pure_type_substitution(filter).is_some() +} + +/// The modeled continuation replaces exactly one card-type predicate. Richer +/// target predicates remain strict failures until their replacement grammar is +/// modeled end-to-end. +fn pure_type_substitution(filter: TargetFilter) -> Option> { + match filter { TargetFilter::Typed(t) if !t.type_filters.is_empty() && t.properties.is_empty() && t.controller.is_none() => { - Some(filter) + Some(t.type_filters) } _ => None, } @@ -8297,23 +8323,14 @@ mod tests { // chunk loop clones the antecedent effect and swaps just its `type_filters`. #[test] fn do_the_same_for_type_accepts_clean_type_substitution() { - match try_parse_do_the_same_for_type("then do the same for Aura cards.") { - Some(TargetFilter::Typed(t)) => { - assert!( - t.type_filters - .iter() - .any(|f| matches!(f, TypeFilter::Subtype(s) if s == "Aura")), - "expected an Aura type substitution, got {:?}", - t.type_filters - ); - assert!( - t.properties.is_empty(), - "must carry no FilterProp predicate" - ); - assert!(t.controller.is_none(), "must carry no controller scope"); - } - other => panic!("expected a clean Aura type substitution, got {other:?}"), - } + let type_filters = try_parse_do_the_same_for_type("then do the same for Aura cards.") + .expect("expected a clean Aura type substitution"); + assert!( + type_filters + .iter() + .any(|f| matches!(f, TypeFilter::Subtype(s) if s == "Aura")), + "expected an Aura type substitution, got {type_filters:?}" + ); assert!( try_parse_do_the_same_for_type("do the same for creature cards").is_some(), "a bare creature-card substitution is also clean" @@ -8342,6 +8359,22 @@ mod tests { } } + #[test] + fn do_the_same_for_type_segment_before_following_then_is_strict() { + assert!( + starts_do_the_same_for_type_before_then( + "do the same for Aura cards, then put the rest on the bottom of your library" + ), + "a pure Aura continuation may be followed by another then-clause" + ); + assert!( + !starts_do_the_same_for_type_before_then( + "do the same for creature cards with mana value 2 and 3, then shuffle" + ), + "richer continuation must not gain support merely because another clause follows" + ); + } + // Guard: the recognizer must NOT swallow unrelated "same" phrases or a // "does the same for " player-set/category fanout (deferred), so // they fall through to normal dispatch instead of being silently dropped. diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index d0ed34181c..2d7225d9af 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -44,6 +44,22 @@ fn each_target_filter_mut_does_not_visit_shuffle() { ); } +#[test] +fn do_the_same_type_rewrite_does_not_overwrite_logical_target_leaves() { + let original = TargetFilter::And { + filters: vec![TargetFilter::Typed(TypedFilter::creature())], + }; + let mut filter = original.clone(); + assert!( + !replace_type_filters(&mut filter, &[TypeFilter::Subtype("Aura".to_string())]), + "a logical target composition is not a direct card selector" + ); + assert_eq!( + filter, original, + "the type-substitution path must not rewrite unrelated logical leaves" + ); +} + // ── MSH-F Sub-Plan A: Cosmic Cube — dynamic mana-value cast permission ── /// Recursively find the first `CastFromZone` effect's constraint in an @@ -46728,6 +46744,144 @@ fn do_the_same_for_type_replicates_mass_zone_change_with_swapped_type() { ); } +/// CR 608.2c: the type substitution must reach a filtered tracked +/// set without replacing its provenance wrapper. Glimpse of Tomorrow's second +/// instruction acts only on Aura permanents revealed by its preceding reveal; +/// changing the outer target to Typed(Aura) would instead discard "this way". +#[test] +fn do_the_same_for_type_retypes_nested_tracked_set_filter() { + let primary = parse_effect_chain( + "Put all non-Aura permanent cards revealed this way onto the battlefield, then do the same for Aura cards.", + AbilityKind::Spell, + ); + + let Effect::ChangeZoneAll { + target: primary_target, + .. + } = &*primary.effect + else { + panic!("primary must be ChangeZoneAll, got {:?}", primary.effect); + }; + let TargetFilter::TrackedSetFiltered { + id: primary_id, + filter: primary_filter, + .. + } = primary_target + else { + panic!("primary must preserve the revealed-card tracked set, got {primary_target:?}"); + }; + assert!( + matches!( + primary_filter.as_ref(), + TargetFilter::Typed(typed) + if typed.type_filters.iter().any(|filter| matches!( + filter, + TypeFilter::Non(inner) + if matches!(&**inner, TypeFilter::Subtype(subtype) if subtype == "Aura") + )) + ), + "primary must retain the non-Aura restriction, got {primary_filter:?}" + ); + + let sub = primary + .sub_ability + .as_ref() + .expect("expected the Aura continuation"); + let Effect::ChangeZoneAll { + target: continuation_target, + .. + } = &*sub.effect + else { + panic!("continuation must be ChangeZoneAll, got {:?}", sub.effect); + }; + let TargetFilter::TrackedSetFiltered { + id: continuation_id, + filter: continuation_filter, + .. + } = continuation_target + else { + panic!( + "continuation must retain the revealed-card tracked set, got {continuation_target:?}" + ); + }; + assert_eq!( + continuation_id, primary_id, + "both instructions must refer to the same revealed-card set" + ); + assert!( + matches!( + continuation_filter.as_ref(), + TargetFilter::Typed(typed) + if typed.type_filters.iter().any( + |filter| matches!(filter, TypeFilter::Subtype(subtype) if subtype == "Aura") + ) + ), + "continuation must select Auras inside the tracked set, got {continuation_filter:?}" + ); +} + +/// CR 608.2c: Glimpse of Tomorrow keeps its Aura partition and its later +/// rest-placement instruction in one sentence. This guards the comma-then +/// boundary between the continuation and the following cleanup clause. +#[test] +fn glimpse_of_tomorrow_keeps_aura_partition_and_rest_placement() { + fn contains_aura_tracked_set(def: &AbilityDefinition) -> bool { + let current = matches!( + def.effect.as_ref(), + Effect::ChangeZoneAll { + target: + TargetFilter::TrackedSetFiltered { + filter, + .. + }, + .. + } if matches!( + filter.as_ref(), + TargetFilter::Typed(typed) + if typed.type_filters.iter().any( + |type_filter| matches!( + type_filter, + TypeFilter::Subtype(subtype) if subtype == "Aura" + ) + ) + ) + ); + current + || def + .sub_ability + .as_deref() + .is_some_and(contains_aura_tracked_set) + || def + .else_ability + .as_deref() + .is_some_and(contains_aura_tracked_set) + } + + let parsed = parse_oracle_text( + "Suspend 3—{R}{R}\nShuffle all permanents you own into your library, then reveal that many cards from the top of your library. Put all non-Aura permanent cards revealed this way onto the battlefield, then do the same for Aura cards, then put the rest on the bottom of your library in a random order.", + "Glimpse of Tomorrow", + &[], + &["Sorcery".to_string()], + &[], + ); + assert!( + parsed.abilities.iter().any(contains_aura_tracked_set), + "Glimpse must retain the Aura continuation inside its revealed-card tracked set" + ); + let json = serde_json::to_string(&parsed).expect("serialize Glimpse parse"); + assert!( + // allow-noncombinator: test assertion checks the serialized AST, not parser dispatch + !json.contains("\"Unimplemented\""), + "Glimpse's handled sentence must not leave an unimplemented clause: {json}" + ); + assert!( + // allow-noncombinator: test assertion checks the serialized AST, not parser dispatch + json.contains("\"type\":\"PutAtLibraryPosition\"") + && json.contains("\"position\":{\"type\":\"Bottom\"}"), + "Glimpse must retain the later rest-on-bottom placement: {json}" + ); +} + /// Unleash the Flux (Phenomenon) — full-card parse drops zero `Unimplemented` /// nodes; the each-player-sacrifice root carries the unbounded `WhileCondition` /// gated on losing the flip, and the flip sub-ability has no leftover branch. diff --git a/crates/engine/tests/integration/issue_4779_do_same_for_type.rs b/crates/engine/tests/integration/issue_4779_do_same_for_type.rs new file mode 100644 index 0000000000..6cd8c78b99 --- /dev/null +++ b/crates/engine/tests/integration/issue_4779_do_same_for_type.rs @@ -0,0 +1,72 @@ +//! Runtime regression for issue #4779: a pure "do the same for " clause +//! must repeat the preceding mass zone-change for the sibling card type. + +use engine::game::scenario::{GameScenario, P0}; +use engine::types::actions::GameAction; +use engine::types::game_state::CastPaymentMode; +use engine::types::mana::ManaCost; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +const RETURN_CREATURE_PARTITIONS: &str = + "Return all non-Human creature cards from your graveyard to the battlefield, then do the same for Human cards."; + +/// CR 608.2c: the Human continuation repeats the return instruction after the +/// non-Human creatures. Both partitions use attachment-free creature cards so +/// the test isolates repetition rather than Aura attachment legality. +#[test] +fn do_the_same_for_type_returns_both_enchantment_partitions() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Partition Return", false, RETURN_CREATURE_PARTITIONS) + .with_mana_cost(ManaCost::generic(0)) + .id(); + let non_human = scenario + .add_creature_from_oracle(P0, "Non-Human Creature", 0, 1, "") + .id(); + let human = scenario + .add_creature_from_oracle(P0, "Human Creature", 0, 1, "") + .with_subtypes(vec!["Human"]) + .id(); + + let mut runner = scenario.build(); + let mut setup_events = Vec::new(); + for object_id in [non_human, human] { + engine::game::zones::move_to_zone( + runner.state_mut(), + object_id, + Zone::Graveyard, + &mut setup_events, + ); + } + + let card_id = runner.state().objects[&spell].card_id; + runner + .act(GameAction::CastSpell { + object_id: spell, + card_id, + targets: vec![], + payment_mode: CastPaymentMode::Auto, + }) + .expect("cast the parsed return spell"); + for _ in 0..16 { + if runner.state().stack.is_empty() { + break; + } + runner + .act(GameAction::PassPriority) + .expect("advance the spell through normal resolution"); + } + + assert!( + runner.state().stack.is_empty(), + "return spell must finish resolving" + ); + assert_eq!(runner.state().objects[&non_human].zone, Zone::Battlefield); + assert_eq!( + runner.state().objects[&human].zone, + Zone::Battlefield, + "the Human must be returned by the repeated type-substitution instruction" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index cafb13816f..18d62e8b0f 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -535,6 +535,7 @@ mod issue_4663_chosen_x_ptcomparison_targets; mod issue_4752_blood_artist_each_player_sacrifice; mod issue_4759_chandras_ignition_single_target; mod issue_4772_too_evil_to_stay_dead; +mod issue_4779_do_same_for_type; mod issue_4786_wrenn_realmbreaker; mod issue_4792_isochron_scepter; mod issue_4824_light_paws_aura_attach;