From 41fb44b1ed83b507eca919b966600fdc9508da32 Mon Sep 17 00:00:00 2001 From: Yaroslav98214 Date: Wed, 15 Jul 2026 21:27:52 +0000 Subject: [PATCH] fix(parser): accept literal thresholds in infix power/toughness comparisons An infix P/T comparison with a literal threshold -- "power less than 3", "toughness greater than 4" -- failed to parse. parse_pt_infix_tail read the threshold with parse_quantity_ref (dynamic game-quantity refs only), while the postfix ("3 or less") and exact tails use parse_quantity_expr_number (literals + X). So a bare-number infix threshold couldn't parse and the whole comparison -- and any count built on it -- was silently dropped. Wasp, Shrinking Savior ("draw a card for each creature with power less than 0") collapsed to a flat Draw { count: Fixed(1) }, always drawing one. Factor the threshold grammar into a shared parse_pt_threshold combinator (dynamic ref, else literal/X) and use it in the infix tail; the dynamic ref is tried first so prior behaviour is preserved. Strict "< N" / "> N" still lower to LE/GE with a -+1 offset (CR 107.1: P/T are integers). The threshold is now an axis independent of the comparison form -- general across power/toughness, less-than/greater-than, strict/or-equal-to. Closes #5874 Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/engine/src/game/filter.rs | 106 +++++++++++++++++ crates/engine/src/parser/oracle_nom/filter.rs | 110 +++++++++++++++++- 2 files changed, 214 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/game/filter.rs b/crates/engine/src/game/filter.rs index d3f914bfe5..ec59ed04c5 100644 --- a/crates/engine/src/game/filter.rs +++ b/crates/engine/src/game/filter.rs @@ -5823,6 +5823,112 @@ mod tests { id } + /// CR 208.1 + CR 107.1 + CR 122.1 — production-path RESOLUTION proof for the + /// infix literal-threshold parser fix (#5874), across the full grammar axis. + /// Each case parses a real `... draw a card for each creature with + /// on the battlefield` clause through `parse_oracle_text`, extracts the Draw's + /// `ObjectCount`, and resolves it through `resolve_quantity` — the same shared + /// authority the Draw effect consults in the production resolution pipeline — + /// against one fixed battlefield. It spans the axis the leaf grammar tests + /// cannot reach on their own: power AND toughness, `less than` AND `greater + /// than`, strict AND `or equal to`. Case 1 is the reported card, Wasp, + /// Shrinking Savior, through its exact printed text. Remove the infix-literal + /// fallback and every ` less/greater than ` filter is dropped, so each + /// count collapses to `Fixed(1)` (no `ObjectCount` to extract) and the + /// `expect` below fails — this regression is tied to the parser change. + #[test] + fn pt_infix_literal_threshold_resolves_through_production_path_full_axis() { + use crate::game::quantity::resolve_quantity; + use crate::types::ability::{AbilityDefinition, Effect, QuantityExpr, QuantityRef}; + + fn draw_object_count(def: &AbilityDefinition) -> Option { + if let Effect::Draw { count, .. } = &*def.effect { + if matches!( + count, + QuantityExpr::Ref { + qty: QuantityRef::ObjectCount { .. } + } + ) { + return Some(count.clone()); + } + } + def.sub_ability + .as_deref() + .and_then(draw_object_count) + .or_else(|| def.else_ability.as_deref().and_then(draw_object_count)) + } + + // Fixed battlefield: (power, toughness) chosen so every axis case has a + // discriminating boundary. Object 0 doubles as the resolution `source` + // (an `ObjectCount` over "creature with …" references no source). + let mut state = setup(); + let specs = [ + ("A", -1, 3), + ("B", 0, 5), + ("C", 1, 4), + ("D", 2, 5), + ("E", 3, 2), + ]; + let mut ids = Vec::new(); + for (name, power, toughness) in specs { + let id = add_creature(&mut state, PlayerId(0), name); + let obj = state.objects.get_mut(&id).unwrap(); + obj.power = Some(power); + obj.toughness = Some(toughness); + ids.push(id); + } + let source = ids[0]; + + let resolve = |oracle: &str, name: &str, state: &GameState| -> i32 { + let parsed = + crate::parser::parse_oracle_text(oracle, name, &[], &["Creature".to_string()], &[]); + let count = parsed + .triggers + .iter() + .filter_map(|t| t.execute.as_deref()) + .find_map(draw_object_count) + .unwrap_or_else(|| { + panic!("no Draw ObjectCount for {name:?} — filter dropped?\n {oracle}") + }); + resolve_quantity(state, &count, PlayerId(0), source) + }; + + // (oracle text, card name, expected count) across the full grammar axis. + let cases: &[(&str, &str, i32)] = &[ + // Case 1 — the reported card, exact printed text: strict `<` power, only A(-1). + ( + "Whenever Wasp attacks, up to one other target creature gets -3/-0 until your next turn. Then draw a card for each creature with power less than 0 on the battlefield.", + "Wasp, Shrinking Savior", + 1, + ), + // `<=` power: A(-1), B(0), C(1). + ( + "Whenever Weaver attacks, draw a card for each creature with power less than or equal to 1 on the battlefield.", + "Weaver", + 3, + ), + // strict `>` toughness: B(5), D(5). + ( + "Whenever Warden attacks, draw a card for each creature with toughness greater than 4 on the battlefield.", + "Warden", + 2, + ), + // `>=` toughness: B(5), C(4), D(5). + ( + "Whenever Watcher attacks, draw a card for each creature with toughness greater than or equal to 4 on the battlefield.", + "Watcher", + 3, + ), + ]; + for (oracle, name, expected) in cases { + assert_eq!( + resolve(oracle, name, &state), + *expected, + "production resolver miscounted `{name}`" + ); + } + } + #[test] fn cmc_filter_treats_retained_x_as_zero_off_stack() { let mut state = setup(); diff --git a/crates/engine/src/parser/oracle_nom/filter.rs b/crates/engine/src/parser/oracle_nom/filter.rs index 783a3dcf6d..2474c00293 100644 --- a/crates/engine/src/parser/oracle_nom/filter.rs +++ b/crates/engine/src/parser/oracle_nom/filter.rs @@ -338,6 +338,23 @@ fn parse_pt_comparison_tail(input: &str) -> OracleResult<'_, (Comparator, Quanti .parse(input) } +/// The threshold value on the right of a P/T comparison — the axis orthogonal to +/// the comparison *form* (infix / postfix / exact). It is a dynamic game quantity +/// ("the number of cards in your hand") or a literal / X ("3", "X"). The postfix +/// and exact tails accept the literal / X grammar directly via +/// `parse_quantity_expr_number`; this combinator unions that with the dynamic ref +/// so the infix tail accepts either, letting "power less than 3" parse like the +/// postfix "power 3 or less". The +/// dynamic ref is tried first to preserve the infix tail's prior behaviour on a +/// dynamic threshold ("less than the number of …"). +fn parse_pt_threshold(input: &str) -> OracleResult<'_, QuantityExpr> { + alt(( + map(parse_quantity_ref, |qty| QuantityExpr::Ref { qty }), + parse_quantity_expr_number, + )) + .parse(input) +} + /// Infix form: "less than [or equal to] " / "greater than [or equal to] ". fn parse_pt_infix_tail(input: &str) -> OracleResult<'_, (Comparator, QuantityExpr)> { let (rest, base_cmp) = alt(( @@ -348,8 +365,12 @@ fn parse_pt_infix_tail(input: &str) -> OracleResult<'_, (Comparator, QuantityExp let rest = rest.trim_start(); let (rest, includes_equal) = map(opt(tag("or equal to")), |e| e.is_some()).parse(rest)?; let rest = rest.trim_start(); - let (rest, qty) = parse_quantity_ref(rest)?; - let value = QuantityExpr::Ref { qty }; + // The threshold is an axis independent of the comparison form: it may be a + // dynamic game quantity ("less than the number of cards in your hand") OR a + // literal / X ("power less than 3", Wasp, Shrinking Savior). Previously the + // infix tail read only `parse_quantity_ref`, so a bare-number threshold + // failed and the whole comparison (and any count built on it) was dropped. + let (rest, value) = parse_pt_threshold(rest)?; // Strict `<`/`>` lower to LE/GE by shifting the threshold by ∓1 (CR 107.1: // integers only, so "less than N" ≡ "≤ N-1"). let (comparator, value) = match (base_cmp, includes_equal) { @@ -722,6 +743,91 @@ mod tests { ); } + #[test] + fn test_pt_infix_literal_threshold_full_grammar_axis() { + // #5874: the INFIX P/T comparison with a LITERAL threshold must parse + // across the full grammar axis -- {power, toughness} x {less than, + // greater than} x {strict, or equal to} -- not just the postfix + // "N or less" form. Strict "< N" / "> N" lower to LE/GE via a -+1 offset + // (CR 107.1: P/T are integers, so "less than N" == "<= N-1"); the + // "or equal to" forms keep N. Wasp, Shrinking Savior's "power less than 0" + // is one point on this axis. + let fixed = |v| QuantityExpr::Fixed { value: v }; + let off = |v, o| QuantityExpr::Offset { + inner: Box::new(fixed(v)), + offset: o, + }; + let cases: &[(&str, PtStat, Comparator, QuantityExpr)] = &[ + ( + "power less than 3", + PtStat::Power, + Comparator::LE, + off(3, -1), + ), + ( + "power greater than 4", + PtStat::Power, + Comparator::GE, + off(4, 1), + ), + ( + "toughness less than or equal to 2", + PtStat::Toughness, + Comparator::LE, + fixed(2), + ), + ( + "toughness greater than or equal to 5", + PtStat::Toughness, + Comparator::GE, + fixed(5), + ), + // The exact Wasp, Shrinking Savior threshold. + ( + "power less than 0", + PtStat::Power, + Comparator::LE, + off(0, -1), + ), + ]; + for (text, stat, comparator, value) in cases { + let (rest, p) = parse_pt_comparison(text).unwrap_or_else(|e| panic!("{text:?}: {e:?}")); + assert_eq!(rest, "", "leftover input for {text:?}"); + assert_eq!( + p, + FilterProp::PtComparison { + stat: *stat, + scope: PtValueScope::Current, + comparator: *comparator, + value: value.clone(), + }, + "text: {text}" + ); + } + } + + #[test] + fn test_pt_infix_dynamic_threshold_still_parses() { + // The literal fallback must not regress the pre-existing infix behaviour: + // a dynamic (game-quantity) threshold still parses -- the dynamic ref is + // tried first. Strict "less than " still lowers to LE + Offset(-1). + let (rest, p) = + parse_pt_comparison("power less than the number of cards in your hand").unwrap(); + assert_eq!(rest, ""); + assert!( + matches!( + p, + FilterProp::PtComparison { + stat: PtStat::Power, + comparator: Comparator::LE, + value: QuantityExpr::Offset { .. }, + .. + } + ), + "dynamic infix threshold must still parse to LE + Offset, got {p:?}" + ); + } + #[test] fn test_parse_with_counter() { let (rest, p) = parse_with_property("with a +1/+1 counter on it").unwrap();