diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index 39fc9309f5..e6d6a72aa2 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -2,7 +2,7 @@ use nom::branch::alt; use nom::bytes::complete::{tag, take_till1, take_until}; use nom::character::complete::{multispace0, multispace1, satisfy}; use nom::combinator::{all_consuming, eof, map, not, opt, peek, rest, value, verify}; -use nom::multi::separated_list1; +use nom::multi::many0; use nom::sequence::{preceded, terminated}; use nom::Parser; @@ -10868,28 +10868,112 @@ pub(crate) fn parse_with_counters_suffix_spanned( /// Those two are the cards this combinator actually reaches, confirmed against /// the CI parse diff. The self-referential "…enters with two +1/+1 counters and /// a lifelink counter on it" shape (Dust Animus, Voidpouncer) prints the SAME -/// conjoined grammar but is parsed at a different seam that calls -/// `parse_counter_suffix_body_combinator` directly rather than through this -/// list, so it still lifts only the first conjunct. Routing that seam through -/// here is the follow-up; do not assume this function already covers it. -/// -/// `separated_list1` is the right combinator rather than a hand-rolled loop -/// because nom backtracks the separator when the following element fails, which -/// is what stops the list from swallowing a non-counter conjunct: on "…counters -/// on it and you become the monarch" the `" and "` separator matches but -/// `parse_counter_suffix_body_combinator` rejects "you" at its leading -/// `parse_number`, so the list ends with the separator unconsumed and the -/// monarch instruction stays in the remainder. Same for "and draw X cards" -/// (Cosima) and "and with haste" (Voidpouncer) — every non-counter conjunct is -/// rejected by the element's mandatory leading count/article. +/// conjoined grammar but never reaches this combinator: a CR 614.1c +/// "[permanent] enters with …" line is an object-hosted REPLACEMENT, parsed by +/// `oracle_replacement::parse_enters_with_counters`, which carries its own +/// conjoined-list reader (`parse_enters_counter_entries`). That reader already +/// lifts every conjunct — pinned by `gated_self_enters_with_conjoined_counters` +/// there — so there is no missing routing to add. Do NOT "unify" the two by +/// pointing the replacement seam at this list: the two count axes are not the +/// same. The replacement reader opens each element with +/// `oracle_util::parse_count_expr` (X, `twice X`, `half X, rounded up`, +/// `N plus/minus X`) and rewrites X to the entering object's `CostXPaid`, while +/// this list opens on `nom_primitives::parse_number`, which takes digits and +/// English number words only. Routing the replacement path through here would +/// REGRESS the X-counted enters-with cards (Astral Cornucopia, Sin, Unending +/// Cataclysm), not extend them. +/// +/// What the two readers DO share is the ELEMENT grammar, and both take the +/// elided-count conjunct through the same combinator +/// ([`parse_countless_counter_element`]) so they cannot drift on it. +/// +/// `many0` over a `preceded(separator, element)` — rather than a hand-rolled +/// loop — is what stops the list from swallowing a non-counter conjunct: nom +/// backtracks the whole `preceded` when the element fails, so on "…counters on +/// it and you become the monarch" the `" and "` separator matches, both element +/// arms reject "you" (no leading count; "you become the monarch" is not a +/// recognized counter type), and the list ends with the separator unconsumed so +/// the monarch instruction stays in the remainder. Same for "and draw X cards" +/// (Cosima) and "and with haste" (Voidpouncer). +/// +/// It is `many0` over an explicit first element rather than `separated_list1` +/// because the two positions no longer take the same parser: only a NON-LEADING +/// element may elide its count. fn parse_enter_counters_clause_body( input: &str, ) -> OracleResult<'_, Vec<(CounterType, QuantityExpr)>> { - preceded( - tag("with "), - separated_list1(tag(" and "), parse_counter_suffix_body_combinator), - ) - .parse(input) + let (rest, _) = tag("with ").parse(input)?; + // CR 122.1: the LEADING element must carry its own count — that mandatory + // number is what anchors the list and stops it claiming arbitrary prose. + // Later elements may elide it (see `parse_countless_counter_element`), so + // the tail tries the counted form first and falls back to the elided one. + let (rest, first) = parse_counter_suffix_body_combinator(rest)?; + let (rest, tail) = many0(preceded( + tag(" and "), + alt(( + parse_counter_suffix_body_combinator, + parse_countless_counter_element, + )), + )) + .parse(rest)?; + // "on it" terminates the LIST. A counted final element consumes it itself + // (the `opt` inside `parse_counter_suffix_body_combinator`), an elided one + // leaves it — strip it here either way so the consumed span is the same + // shape for both, which is what `parse_with_counters_suffix_spanned`'s + // callers slice on. + let (rest, _) = opt(tag::<_, _, OracleError<'_>>(" on it")).parse(rest)?; + + let mut counters = Vec::with_capacity(1 + tail.len()); + counters.push(first); + counters.extend(tail); + Ok((rest, counters)) +} + +/// CR 122.1: ONE element of a conjoined counter list whose count is ELIDED — +/// "…an additional +1/+1 counter and deathtouch counter on it" (March Toward +/// Perfection), "…an additional +1/+1 counter, reach counter, and trample +/// counter on it" (Arcane Archery), "…an additional +1/+1 counter, trample +/// counter, and vigilance counter on it" (Tenacious Pup). A corpus sweep over +/// every printed "enters/enter with … counter" and battlefield-rider line found +/// those three and no others, so this is the whole class, not a sample of it. +/// +/// English coordination lets the leading element's determiner distribute across +/// the later conjuncts — "an additional [+1/+1 counter] and [deathtouch +/// counter]" — so a later element can carry no count of its own. Each such +/// element is a SINGULAR counter noun, and CR 122.1 places counters +/// individually, so the elided count is exactly one. That is a fact about +/// English determiner scope, not a rules inference: no CR section governs the +/// elision, which is why none beyond CR 122.1 (what a counter is) is cited. +/// +/// Two guards keep this from over-claiming. `parse_counter_suffix_body_combinator` +/// is anchored by its mandatory leading number, and slices its counter type with +/// an unbounded `take_until(" counter")`; with the number gone that slice would +/// happily swallow any prose sitting in front of the word "counter". So instead: +/// +/// * the type must be a RECOGNIZED counter — `parse_strict_counter_type`, i.e. +/// the P/T-modifier, keyword-counter and named-counter arms WITHOUT the +/// open-ended `take_till1 → Generic` fallback. An unrecognized token fails +/// the element rather than becoming a bogus `Generic`. +/// * the noun must be SINGULAR. A plural elided element ("two +1/+1 counters +/// and trample counters") is genuinely ambiguous about whether the head +/// count distributes across the conjunction; no card prints one, so it fails +/// closed instead of guessing. +/// +/// Valid only in NON-LEADING position — the first element must still carry its +/// own count, which is what keeps the anchor on the list as a whole. Callers +/// enforce that by reaching for this only after a separator has matched. +/// +/// Deliberately does NOT consume a trailing " on it": that filler terminates the +/// LIST, not the element, so both callers strip it once after their loop ends. +pub(crate) fn parse_countless_counter_element( + input: &str, +) -> nom::IResult<&str, (CounterType, QuantityExpr), OracleError<'_>> { + let (rest, counter_type) = nom_primitives::parse_strict_counter_type(input)?; + let (rest, _) = tag(" counter").parse(rest)?; + // Singular-only guard — see the plural note above. `not` does not consume, + // so the terminator is left intact for the caller. + not(tag::<_, _, OracleError<'_>>("s")).parse(rest)?; + Ok((rest, (counter_type, QuantityExpr::Fixed { value: 1 }))) } /// CR 122.6: the enter-with-counters rider in LEADING position, tolerating the @@ -11954,6 +12038,73 @@ mod tests { } } + /// CR 122.1: a NON-LEADING conjunct may elide its count, and the elided + /// count is one. Building-block coverage for the battlefield-rider list — + /// the printed cards for this shape (March Toward Perfection, Arcane + /// Archery, Tenacious Pup) all reach the sibling reader in + /// `oracle_replacement`, so without this the element grammar would only ever + /// be exercised from one of its two callers. + #[test] + fn counter_clause_list_accepts_elided_count_conjunct() { + use crate::types::keywords::KeywordKind; + + let (rest, counters) = parse_enter_counters_clause_body( + "with an additional +1/+1 counter and deathtouch counter on it", + ) + .expect("elided-count conjunct must parse"); + assert_eq!(rest, "", "the list-level \" on it\" must be consumed"); + assert_eq!( + counters, + vec![ + (CounterType::Plus1Plus1, QuantityExpr::Fixed { value: 1 }), + ( + CounterType::Keyword(KeywordKind::Deathtouch), + QuantityExpr::Fixed { value: 1 } + ), + ] + ); + } + + /// The elided element has no leading number to anchor it, so its two guards + /// carry the whole burden of not over-claiming. Each input must yield a + /// ONE-element list, with the unclaimed text left on the remainder: + /// + /// * an unrecognized type is rejected by `parse_strict_counter_type` + /// rather than becoming a `CounterType::Generic`. Note the conjunct here + /// carries NO article — with one it would take the COUNTED arm, whose + /// open-ended `take_until(" counter")` maps any token to `Generic`; that + /// arm is anchored by its number and is out of scope for this guard. + /// * a PLURAL elided conjunct is ambiguous about whether the head count + /// distributes, so it fails closed; + /// * the leading element still REQUIRES its count — an elided head would + /// let the list start anywhere. + #[test] + fn elided_count_conjunct_guards() { + for (input, expected_rest) in [ + // Not a counter type — must not become Generic("fresh idea"). + ( + "with a +1/+1 counter and fresh idea counter on it", + " and fresh idea counter on it", + ), + // Plural elided conjunct — fails closed. + ( + "with two +1/+1 counters and trample counters on it", + " and trample counters on it", + ), + ] { + let (rest, counters) = + parse_enter_counters_clause_body(input).expect("leading element must still parse"); + assert_eq!(counters.len(), 1, "over-claimed on {input:?}: {counters:?}"); + assert_eq!(rest, expected_rest, "wrong stop point for {input:?}"); + } + + // An elided LEADING element is not a list at all. + assert!( + parse_enter_counters_clause_body("with trample counter on it").is_err(), + "the leading element must carry its own count" + ); + } + fn variable_x() -> QuantityExpr { QuantityExpr::Ref { qty: QuantityRef::Variable { diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 4ce7eaa061..035f44acc6 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -16,9 +16,9 @@ pub(crate) use search::parse_search_name_reference_suffix; pub(crate) use lower::{ capitalize, lower_effect_chain_ir, parse_controls_permanent_object, - parse_counter_suffix_body_combinator, parse_with_counters_suffix, - parse_with_counters_suffix_spanned, player_lookback_relative_clause_owns_suffix, - strip_trailing_duration, strip_trailing_where_x, + parse_counter_suffix_body_combinator, parse_countless_counter_element, + parse_with_counters_suffix, parse_with_counters_suffix_spanned, + player_lookback_relative_clause_owns_suffix, strip_trailing_duration, strip_trailing_where_x, }; // pub(super) re-exports used by sibling submodules via `super::fn_name()`. pub(super) use lower::{ diff --git a/crates/engine/src/parser/oracle_replacement.rs b/crates/engine/src/parser/oracle_replacement.rs index 65726da8b0..fc0fb9bba5 100644 --- a/crates/engine/src/parser/oracle_replacement.rs +++ b/crates/engine/src/parser/oracle_replacement.rs @@ -4058,7 +4058,18 @@ fn parse_enters_with_counters( } } - let counter_entries = parse_enters_counter_entries(after_additional); + // CR 122.1: the conjoined-list reader gets `after_with`, NOT + // `after_additional`. The caller-level "an additional " strip above exists + // for the single-counter path, and it eats the very article that anchors the + // list's leading element: "an additional +1/+1 counter and deathtouch + // counter on it" (March Toward Perfection) would arrive as "+1/+1 counter + // and …", whose head carries no count, so the whole list was rejected and + // every conjunct past the first silently dropped. The element grammar + // consumes "[an] additional" itself (`strip_additional_counter_qualifier`), + // so handing it the unstripped text is strictly more permissive — it also + // picks up "an additional +1/+1 counter and a lifelink counter on it", which + // the strip likewise used to break. + let counter_entries = parse_enters_counter_entries(after_with); // Detect dynamic count: "a number of [type] counters ... equal to [qty]" let after_prefix = tag::<_, _, OracleError<'_>>("a number of ") .parse(after_additional) @@ -4729,27 +4740,11 @@ fn parse_enters_counter_entries(after_with: &str) -> Option>(" counter") - .parse(rest) - .ok()?; - if counter_type_raw.trim().is_empty() { - return None; - } - let counter_type = - crate::parser::oracle_effect::counter::normalize_counter_type(counter_type_raw); - let (after_space, _) = tag::<_, _, OracleError<'_>>(" ").parse(at_counter).ok()?; - let (after_counter_word, _) = - alt((tag::<_, _, OracleError<'_>>("counters"), tag("counter"))) - .parse(after_space) - .ok()?; - - entries.push((counter_type, count_expr)); + // CR 122.1: only a NON-LEADING conjunct may elide its count — the first + // element's mandatory count is what anchors the list. + let (entry, after_counter_word) = + parse_enters_counter_entry(remaining, !entries.is_empty())?; + entries.push(entry); if let Some(next) = parse_enters_counter_separator(after_counter_word) { remaining = next; @@ -4765,27 +4760,80 @@ fn parse_enters_counter_entries(after_with: &str) -> Option= 2).then_some(entries) } -fn parse_enters_counter_separator(input: &str) -> Option<&str> { - let (after_sep, _) = alt(( - tag::<_, _, OracleError<'_>>(", and "), - tag(" and "), - tag(", "), - )) - .parse(input) - .ok()?; +/// CR 122.1: parse ONE element of an "enters with" counter list, returning the +/// entry and the remainder after its `counter`/`counters` noun. +/// +/// Two element shapes, tried in that order: +/// * COUNTED — "two +1/+1 counters", "an additional lifelink counter", +/// "X charge counters". The count opens with `oracle_util::parse_count_expr`, +/// so the whole arithmetic grammar (X, `twice X`, `half X, rounded up`, +/// `N plus/minus X`) is available and any surviving `X` is rewritten to the +/// entering object's `CostXPaid` (CR 614.12). +/// * ELIDED — "…and deathtouch counter on it", where the leading element's +/// determiner distributes across the conjunction. Delegated to the shared +/// `oracle_effect::parse_countless_counter_element` so this reader and the +/// battlefield-rider list in `oracle_effect::lower` cannot drift on the +/// element grammar; see that function for the recognized-type and +/// singular-noun guards that keep an unanchored element from over-claiming. +/// +/// `allow_elided_count` is false at the head of the list and true after any +/// separator — the count is what anchors the leading element. +fn parse_enters_counter_entry( + input: &str, + allow_elided_count: bool, +) -> Option<((CounterType, QuantityExpr), &str)> { + if let Some(parsed) = parse_counted_enters_counter_entry(input) { + return Some(parsed); + } + if !allow_elided_count { + return None; + } + let (rest, entry) = + crate::parser::oracle_effect::parse_countless_counter_element(input).ok()?; + Some((entry, rest)) +} + +/// The COUNTED element shape of [`parse_enters_counter_entry`] — see there. +fn parse_counted_enters_counter_entry(input: &str) -> Option<((CounterType, QuantityExpr), &str)> { + let (mut count_expr, rest) = parse_count_expr(input)?; + rewrite_variable_x_to_cost_x_paid(&mut count_expr); + // CR 122.1: strip the "additional" qualifier that follows the count word + // ("two additional +1/+1 counters") so it doesn't leak into the type. + let rest = strip_additional_counter_qualifier(rest); - let (_, rest) = parse_count_expr(after_sep)?; let (at_counter, counter_type_raw) = take_until::<_, _, OracleError<'_>>(" counter") .parse(rest) .ok()?; if counter_type_raw.trim().is_empty() { return None; } + let counter_type = + crate::parser::oracle_effect::counter::normalize_counter_type(counter_type_raw); let (after_space, _) = tag::<_, _, OracleError<'_>>(" ").parse(at_counter).ok()?; - alt((tag::<_, _, OracleError<'_>>("counters"), tag("counter"))) + let (after_counter_word, _) = alt((tag::<_, _, OracleError<'_>>("counters"), tag("counter"))) .parse(after_space) .ok()?; + Some(((counter_type, count_expr), after_counter_word)) +} + +/// Match a list separator AND verify the element after it parses, so a +/// separator that is really a sentence connective ("…on it, then draw a card") +/// leaves the list intact. This is the hand-rolled equivalent of the backtracking +/// nom gets for free from `many0(preceded(sep, element))`; it lives here because +/// this reader's counted element is not a pure combinator. +fn parse_enters_counter_separator(input: &str) -> Option<&str> { + let (after_sep, _) = alt(( + tag::<_, _, OracleError<'_>>(", and "), + tag(" and "), + tag(", "), + )) + .parse(input) + .ok()?; + + // Post-separator position, so the elided-count form is in scope here too. + parse_enters_counter_entry(after_sep, true)?; + Some(after_sep) } @@ -15591,6 +15639,203 @@ mod tests { } } + /// CR 614.1c: a conjoined "enters with" counter list must survive a GATE. + /// `self_enters_with_multiple_counter_types` pins the ungated, all-singular + /// list (Agent's Toolkit); this pins the two axes that card does not cover, + /// because both are places the list could be truncated to its first element: + /// + /// * a gate is peeled BEFORE the payload is read — sentence-initial + /// "If , " (CR 614.1c, Dust Animus) and kicker-conditional + /// "If ~ was kicked, " (CR 702.33d, Voidpouncer). The gate must land on + /// the definition's single condition slot AND leave the whole list. + /// * a non-counter conjunct printed AFTER the list ("… on it and with + /// haste", Voidpouncer) must stop the list rather than be consumed as a + /// third counter — and must not take the second counter down with it. + /// + /// A multi-count leading element ("two +1/+1 counters") is used deliberately: + /// Agent's Toolkit's elements are all bare articles, so the count axis and + /// the conjunct axis have never been exercised together. + /// + /// This is the direct-evidence pin for the claim in + /// `oracle_effect::lower::parse_enter_counters_clause_body`'s doc comment + /// that the replacement seam needs no routing through that list. + #[test] + fn gated_self_enters_with_conjoined_counters() { + let assert_chain = |text: &str, card: &str, expected: &[(CounterType, i32)]| { + let def = parse_replacement_line(text, card) + .unwrap_or_else(|| panic!("{card}: gated conjoined enters-with must parse")); + assert_eq!(def.event, ReplacementEvent::Moved, "{card}: self-ETB event"); + assert_eq!( + def.valid_card, + Some(TargetFilter::SelfRef), + "{card}: self-referential subject" + ); + assert!( + def.condition.is_some(), + "{card}: the gate must reach the condition slot, not be dropped" + ); + + let mut cursor = Some(def.execute.as_deref().expect("execute ability")); + for (index, (counter, count)) in expected.iter().enumerate() { + let ability = cursor.unwrap_or_else(|| { + panic!("{card}: conjunct {index} ({counter:?}) was dropped from the chain") + }); + assert!( + matches!( + &*ability.effect, + Effect::PutCounter { + counter_type, + count: QuantityExpr::Fixed { value }, + target: TargetFilter::SelfRef, + } if counter_type == counter && value == count + ), + "{card}: conjunct {index} expected {counter:?} x{count}, got {:?}", + ability.effect + ); + cursor = ability.sub_ability.as_deref(); + } + assert!( + cursor.is_none(), + "{card}: a non-counter conjunct was swallowed as an extra PutCounter" + ); + }; + + // Dust Animus — leading game-state gate. + assert_chain( + "If you control five or more untapped lands, this creature enters with two +1/+1 \ + counters and a lifelink counter on it.", + "Dust Animus", + &[ + (CounterType::Plus1Plus1, 2), + ( + CounterType::Keyword(crate::types::keywords::KeywordKind::Lifelink), + 1, + ), + ], + ); + + // Voidpouncer — kicker gate, plus a trailing "and with haste" rider that + // must terminate the list without truncating it. + assert_chain( + "If this creature was kicked, it enters with two +1/+1 counters and a trample \ + counter on it and with haste.", + "Voidpouncer", + &[ + (CounterType::Plus1Plus1, 2), + ( + CounterType::Keyword(crate::types::keywords::KeywordKind::Trample), + 1, + ), + ], + ); + } + + /// CR 122.1: a conjunct whose count is ELIDED must still place its counter. + /// English coordination lets the leading determiner distribute — "an + /// additional [+1/+1 counter] and [deathtouch counter]" — and every conjunct + /// past the first used to be dropped on the floor, so these cards entered + /// with only their +1/+1. + /// + /// These three are the entire printed class (corpus sweep over every + /// "enters/enter with … counter" and battlefield-rider line), and they cover + /// both separator shapes: bare `" and "` and the Oxford `", …, and "` list. + /// The last one also pins that the longer "enters the battlefield with" + /// spelling takes the same path as the short "enters with". + #[test] + fn enters_with_elided_count_conjuncts() { + use crate::types::keywords::KeywordKind; + + let assert_chain = |text: &str, card: &str, expected: &[CounterType]| { + let def = parse_replacement_line(text, card) + .unwrap_or_else(|| panic!("{card}: elided-count conjunct list must parse")); + let mut cursor = Some(def.execute.as_deref().expect("execute ability")); + for (index, counter) in expected.iter().enumerate() { + let ability = cursor.unwrap_or_else(|| { + panic!("{card}: conjunct {index} ({counter:?}) was dropped from the chain") + }); + assert!( + matches!( + &*ability.effect, + // CR 122.1: an elided count is exactly one counter. + Effect::PutCounter { + counter_type, + count: QuantityExpr::Fixed { value: 1 }, + .. + } if counter_type == counter + ), + "{card}: conjunct {index} expected {counter:?} x1, got {:?}", + ability.effect + ); + cursor = ability.sub_ability.as_deref(); + } + assert!( + cursor.is_none(), + "{card}: trailing text was swallowed as an extra PutCounter" + ); + }; + + assert_chain( + "When you cast a Phyrexian creature spell, that creature enters with an additional \ + +1/+1 counter and deathtouch counter on it.", + "March Toward Perfection", + &[ + CounterType::Plus1Plus1, + CounterType::Keyword(KeywordKind::Deathtouch), + ], + ); + + assert_chain( + "When you cast a creature spell, that creature enters with an additional +1/+1 \ + counter, reach counter, and trample counter on it.", + "Arcane Archery", + &[ + CounterType::Plus1Plus1, + CounterType::Keyword(KeywordKind::Reach), + CounterType::Keyword(KeywordKind::Trample), + ], + ); + + assert_chain( + "When you cast a creature spell, that creature enters the battlefield with an \ + additional +1/+1 counter, trample counter, and vigilance counter on it.", + "Tenacious Pup", + &[ + CounterType::Plus1Plus1, + CounterType::Keyword(KeywordKind::Trample), + CounterType::Keyword(KeywordKind::Vigilance), + ], + ); + } + + /// The elided-count element is unanchored (no leading number), so it must not + /// widen what the list as a whole claims. Each case below stays on the + /// single-counter path — one `PutCounter`, no chain — rather than growing a + /// bogus second entry: + /// + /// * a non-counter conjunct after the list ("and you draw a card") + /// * a conjunct naming something that is not a counter type — the guard + /// against `parse_strict_counter_type`'s absent `Generic` fallback + /// * a PLURAL elided conjunct, which is ambiguous about whether the head + /// count distributes and so fails closed rather than guessing + #[test] + fn elided_count_conjunct_does_not_over_claim() { + for text in [ + "This creature enters with an additional +1/+1 counter and you draw a card.", + "This creature enters with an additional +1/+1 counter and haste on it.", + "This creature enters with two +1/+1 counters and trample counters on it.", + ] { + let Some(def) = parse_replacement_line(text, "Test Enterer") else { + continue; + }; + let execute = def.execute.as_deref().expect("execute ability"); + assert!( + execute.sub_ability.is_none(), + "elided-count element over-claimed on {text:?}: {:?}", + execute.sub_ability + ); + } + } + #[test] fn enters_with_x_counters_uses_cost_x_paid() { // CR 107.3m: "This artifact enters with X charge counters on it" — X is the