diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index ceaab2f1dc..53c0d659bb 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -8230,4 +8230,54 @@ mod tests { assert_eq!(p.reads_event_live, twin.reads_event_live); assert_eq!(p.legacy_batch_prompt(), twin.legacy_batch_prompt()); } + + /// CR 400.7d: the Adamant *rider* cards (Slaying Fire, Cauldron's Gift, …) + /// re-route from `AbilityCondition::ManaColorSpent` — an inert, + /// reads-nothing arm — to the generic + /// `QuantityCheck { ManaSpentToCast { OfColor } }`, which resolves through + /// `legacy_ref()`. Pin BOTH sides so the delta is explicit rather than + /// incidental: the flip is toward the MORE conservative classification + /// (batch-prompt rather than silent auto-order), which is fail-safe, and a + /// future retirement of `ManaColorSpent` cannot silently change it. + /// + /// Companion to `adamant_rider_generic_shape_reads_all_scan_axes` in + /// `ability_scan.rs`, which pins the corresponding scan-axis delta. Without + /// this test the `ability_rw` half of that re-routing is unpinned. + #[test] + fn adamant_rider_generic_shape_is_more_conservative_than_legacy() { + use crate::types::ability::{CastManaObjectScope, CastManaSpentMetric}; + use crate::types::mana::ManaColor; + + let generic = AbilityCondition::QuantityCheck { + lhs: qref(QuantityRef::ManaSpentToCast { + scope: CastManaObjectScope::SelfObject, + metric: CastManaSpentMetric::OfColor { + color: ManaColor::Red, + }, + }), + comparator: Comparator::GE, + rhs: qfix(3), + }; + let legacy = AbilityCondition::ManaColorSpent { + color: ManaColor::Red, + minimum: 3, + }; + + // SCOPE OF THIS TEST: it pins the rw CLASSIFICATION of both shapes. It + // constructs the conditions directly and never invokes the parser, so it + // does NOT detect a revert of the `OfColor` lowering — that is guarded by + // `leading_word_mana_spent_condition_parses_adamant` in + // `parser/oracle_effect/conditions.rs`. What reverting the lowering WOULD + // do is make this test's `generic` shape unreachable in production while + // leaving it green; the pin below is what keeps the two classifications + // visibly different so such a change cannot pass unnoticed. + assert!( + rw_ability_condition(&generic).legacy_batch_prompt(), + "the generic ManaSpentToCast shape must carry the legacy_ref batch-prompt marker" + ); + assert!( + !rw_ability_condition(&legacy).legacy_batch_prompt(), + "the legacy ManaColorSpent arm reads nothing; pinned so the delta stays visible" + ); + } } diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index deb416008d..d208c23885 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -7929,6 +7929,66 @@ mod tests { ))); } + /// CR 400.7d + CR 601.2h: the Adamant ability rider ("if at least three red + /// mana was spent to cast this spell, it deals 4 damage instead") now parses + /// to the generic `QuantityCheck { ManaSpentToCast { .., OfColor } }` shape + /// instead of the legacy `AbilityCondition::ManaColorSpent`. That moves it + /// from the `Axes::NONE` arm (`:2572`) onto the `Axes::CONSERVATIVE` arm + /// (`:2452`, reached via `QuantityCheck` → `scan_quantity_expr`), flipping + /// ALL THREE scan axes false→true for the 11 affected cards. + /// + /// RULING — the flip is accepted, and is the intended direction: + /// - `event`: CORRECT, not merely conservative. CR 400.7d makes the paid-mana + /// record a cost-paid-object characteristic, which is precisely what the + /// `event` axis names; the legacy `Axes::NONE` under-read it. + /// - `sibling` / `projected`: over-inclusive but fail-SAFE. The payment record + /// is stamped once at CR 601.2h and is neither sibling-mutable nor a + /// player-level projected resource, so `true` can only make the analysis + /// reject an auto-order cover (prompt) — never auto-resolve something it + /// should have prompted on. + /// + /// Neither `ordering_parity_sweep` (which imports `ability_rw` only) nor + /// `coverage-data.json` observes this axis, hence this direct assertion. + #[test] + fn adamant_rider_generic_shape_reads_all_scan_axes() { + let generic = AbilityCondition::QuantityCheck { + lhs: QuantityExpr::Ref { + qty: QuantityRef::ManaSpentToCast { + scope: CastManaObjectScope::SelfObject, + metric: CastManaSpentMetric::OfColor { + color: ManaColor::Red, + }, + }, + }, + comparator: Comparator::GE, + rhs: QuantityExpr::Fixed { value: 3 }, + }; + // SCOPE OF THIS TEST: it pins the scan-axis CLASSIFICATION of both + // shapes. It constructs the conditions directly and never invokes the + // parser, so it does NOT detect a revert of the `OfColor` lowering — + // that is guarded by `leading_word_mana_spent_condition_parses_adamant` + // in `parser/oracle_effect/conditions.rs`. The legacy pin below is what + // keeps the delta explicit. + let generic_axes = scan_ability_condition(&generic, ScanMode::Conservative); + assert!(generic_axes.event); + assert!(generic_axes.sibling); + assert!(generic_axes.projected); + // The public projection consumed by `analysis::resource` agrees. + assert!(ability_condition_reads_projected_resource(&generic)); + + // Pin the legacy shape's classification so the delta is explicit and a + // future retirement of `ManaColorSpent` cannot silently change it. + let legacy = AbilityCondition::ManaColorSpent { + color: ManaColor::Red, + minimum: 3, + }; + let legacy_axes = scan_ability_condition(&legacy, ScanMode::Conservative); + assert!(!legacy_axes.event); + assert!(!legacy_axes.sibling); + assert!(!legacy_axes.projected); + assert!(!ability_condition_reads_projected_resource(&legacy)); + } + // ---- BLOCKER 1 regression: multi_target bounds are traversed ---- #[test] fn multi_target_bound_event_read_classifies() { diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index ed77913be4..59fbe1d1aa 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -4204,7 +4204,9 @@ fn quantity_ref_target_slot_spec(qty: &QuantityRef) -> Option { CastManaSpentMetric::FromSource { source_filter } => { filter_target_slot_filter(source_filter) } - CastManaSpentMetric::Total | CastManaSpentMetric::DistinctColors => None, + CastManaSpentMetric::Total + | CastManaSpentMetric::DistinctColors + | CastManaSpentMetric::OfColor { .. } => None, }, QuantityRef::PlayerCount { filter: crate::types::ability::PlayerFilter::ControlsCount { filter, .. }, diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index e81afd2fea..c1120fe749 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -1753,9 +1753,20 @@ fn fmt_quantity_ref(qty: &QuantityRef) -> String { QuantityRef::TimesCostPaidThisResolution => { "times the repeated optional cost was paid this resolution".into() } - QuantityRef::ManaSpentToCast { scope, metric } => { - format!("mana spent to cast ({scope:?}, {metric:?})") - } + QuantityRef::ManaSpentToCast { scope, metric } => match metric { + // CR 106.3: the per-color leaf names a concrete color, so render it + // in words. The other three metrics keep their existing `{metric:?}` + // rendering byte-identically. + crate::types::ability::CastManaSpentMetric::OfColor { color } => format!( + "mana spent to cast ({scope:?}, {} mana)", + fmt_mana_color_full(color) + ), + crate::types::ability::CastManaSpentMetric::Total + | crate::types::ability::CastManaSpentMetric::DistinctColors + | crate::types::ability::CastManaSpentMetric::FromSource { .. } => { + format!("mana spent to cast ({scope:?}, {metric:?})") + } + }, QuantityRef::EventContextSourceCostX => "X of triggering spell".into(), QuantityRef::EventContextSourceModesChosen => { "modes chosen for the triggering spell".into() diff --git a/crates/engine/src/game/layers.rs b/crates/engine/src/game/layers.rs index 05186f0bde..82ab91581b 100644 --- a/crates/engine/src/game/layers.rs +++ b/crates/engine/src/game/layers.rs @@ -2845,7 +2845,9 @@ fn quantity_ref_reads_life(qty: &QuantityRef) -> bool { CastManaSpentMetric::FromSource { source_filter } => { target_filter_reads_life_total(source_filter) } - CastManaSpentMetric::Total | CastManaSpentMetric::DistinctColors => false, + CastManaSpentMetric::Total + | CastManaSpentMetric::DistinctColors + | CastManaSpentMetric::OfColor { .. } => false, }, // CR 603.2c: the trigger-event player set filters candidates through a diff --git a/crates/engine/src/game/quantity.rs b/crates/engine/src/game/quantity.rs index 409769adf5..bcd3d74da6 100644 --- a/crates/engine/src/game/quantity.rs +++ b/crates/engine/src/game/quantity.rs @@ -1640,6 +1640,9 @@ pub(crate) fn resolve_mana_spent_to_cast_metric( CastManaSpentMetric::DistinctColors => { usize_to_i32_saturating(spent_colors.distinct_colors()) } + // CR 106.3 + CR 601.2h: how much mana of exactly this color paid the + // cost, read off the same per-color payment tally. + CastManaSpentMetric::OfColor { color } => u32_to_i32_saturating(spent_colors.get(*color)), CastManaSpentMetric::FromSource { source_filter } => usize_to_i32_saturating( source_snapshots .iter() diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index cb2fa0cb32..7faa923fa6 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -10080,7 +10080,9 @@ fn quantity_ref_refs_cost_paid_object(qty: &QuantityRef) -> bool { CastManaSpentMetric::FromSource { source_filter } => { source_filter.references_cost_paid_object() } - CastManaSpentMetric::Total | CastManaSpentMetric::DistinctColors => false, + CastManaSpentMetric::Total + | CastManaSpentMetric::DistinctColors + | CastManaSpentMetric::OfColor { .. } => false, }, // Non-object, non-filter refs never read the cost-paid object. Listed diff --git a/crates/engine/src/parser/oracle_effect/conditions.rs b/crates/engine/src/parser/oracle_effect/conditions.rs index 6775a3a3c9..8eab729f2d 100644 --- a/crates/engine/src/parser/oracle_effect/conditions.rs +++ b/crates/engine/src/parser/oracle_effect/conditions.rs @@ -3870,6 +3870,23 @@ fn parse_symbolic_mana_color_spent_condition( Ok((rest, condition)) } +/// CR 106.3 + CR 601.2h: legacy leading-word form, "at least N mana was +/// spent to cast " → `AbilityCondition::ManaColorSpent`. +/// +/// SHADOWED, kept as a fallback only. `parse_mana_color_spent_condition_text` +/// has exactly one caller (`parse_condition_text`), and every non-test caller of +/// that reaches it only through `.or_else(..)` AFTER +/// `try_nom_condition_as_ability_condition`, whose `parse_inner_condition` +/// fall-through now recognizes this exact phrase via +/// `parse_color_mana_spent_threshold` and bridges it to the canonical +/// `AbilityCondition::QuantityCheck { ManaSpentToCast { scope, OfColor } }`. +/// So on the production path this arm no longer fires for any real input; the +/// generic form carries an explicit `CastManaObjectScope` (CR 400.7d) that +/// `ManaColorSpent` cannot express. +/// +/// `parse_symbolic_mana_color_spent_condition` above is NOT shadowed — the +/// `{W}{W}` symbolic form has no `parse_inner_condition` grammar and stays live +/// for 22 cards. fn parse_word_mana_color_spent_condition( input: &str, ) -> super::super::oracle_nom::error::OracleResult<'_, AbilityCondition> { @@ -8839,6 +8856,13 @@ mod tests { ); } + /// CR 106.3 + CR 601.2h + CR 400.7d: the leading-word Adamant rider now + /// bridges through `parse_inner_condition` → + /// `parse_color_mana_spent_threshold` to the canonical generic shape, which + /// (unlike the legacy `ManaColorSpent`) records the subject anaphora as an + /// explicit `CastManaObjectScope`. Runtime reading is unchanged: + /// `QuantityCheck` resolves `ManaSpentToCast { SelfObject, .. }` against the + /// ability's own source object, exactly as the `ManaColorSpent` arm did. #[test] fn leading_word_mana_spent_condition_parses_adamant() { let (condition, body) = strip_leading_general_conditional( @@ -8848,13 +8872,84 @@ mod tests { assert_eq!(body, "it deals 4 damage instead."); assert_eq!( condition, - Some(AbilityCondition::ManaColorSpent { - color: ManaColor::Red, - minimum: 3, + Some(AbilityCondition::QuantityCheck { + lhs: QuantityExpr::Ref { + qty: QuantityRef::ManaSpentToCast { + scope: CastManaObjectScope::SelfObject, + metric: CastManaSpentMetric::OfColor { + color: ManaColor::Red + }, + }, + }, + comparator: Comparator::GE, + rhs: QuantityExpr::Fixed { value: 3 }, }) ); } + /// CR 400.7d: the "that spell" anaphora on the PRODUCTION path. + /// + /// `parse_condition_text`'s leading-word arm + /// (`parse_word_mana_color_spent_condition`) only ever accepted the + /// `{this spell, it, them, ~}` subject set and flattened the anaphora away + /// into a scope-less `ManaColorSpent`. That arm is now shadowed: every + /// non-test caller reaches `try_nom_condition_as_ability_condition` first, + /// and its `parse_inner_condition` fall-through recognizes "that spell" and + /// records it as `CastManaObjectScope::TriggeringSpell`. + /// + /// RULING — this is acceptable and strictly more faithful. CR 400.7d + /// distinguishes reading the payment record of the object the ability is on + /// from reading that of a referenced spell; the generic shape carries the + /// distinction, the legacy shape could not. Zero live cards use "that + /// spell" in this rider today (all 11 leading-word Adamant cards say "this + /// spell"), so this is a latent capability, not a behavior change. The + /// runtime `QuantityCheck` reader resolves `TriggeringSpell` through the + /// trigger event context rather than `ability.source_id`, which is the + /// correct reading for that phrasing. + #[test] + fn leading_word_mana_spent_condition_records_that_spell_anaphora() { + let condition = try_nom_condition_as_ability_condition( + "at least three white mana was spent to cast that spell", + &mut ParseContext::default(), + ) + .expect("the generic grammar must recognize the 'that spell' anaphora"); + assert_eq!( + condition, + AbilityCondition::QuantityCheck { + lhs: QuantityExpr::Ref { + qty: QuantityRef::ManaSpentToCast { + scope: CastManaObjectScope::TriggeringSpell, + metric: CastManaSpentMetric::OfColor { + color: ManaColor::White + }, + }, + }, + comparator: Comparator::GE, + rhs: QuantityExpr::Fixed { value: 3 }, + } + ); + + // Reach-guard + contrast: "this spell" stays `SelfObject`, so the scope + // axis is genuinely threaded rather than hardcoded. + let self_scoped = try_nom_condition_as_ability_condition( + "at least three white mana was spent to cast this spell", + &mut ParseContext::default(), + ) + .expect("the 'this spell' anaphora must still parse"); + assert!(matches!( + self_scoped, + AbilityCondition::QuantityCheck { + lhs: QuantityExpr::Ref { + qty: QuantityRef::ManaSpentToCast { + scope: CastManaObjectScope::SelfObject, + .. + }, + }, + .. + } + )); + } + /// CR 122.1 + CR 608.2c: "there are no counters on ~" round-trips through /// the bridge to a `QuantityCheck` against `AnyCountersOnSelf`. Previously /// the bridge returned `None` for `CounterMatch::Any`, which silently diff --git a/crates/engine/src/parser/oracle_nom/condition.rs b/crates/engine/src/parser/oracle_nom/condition.rs index a736b93b94..6a5dbc6b92 100644 --- a/crates/engine/src/parser/oracle_nom/condition.rs +++ b/crates/engine/src/parser/oracle_nom/condition.rs @@ -658,6 +658,7 @@ fn parse_resolution_context_conditions(input: &str) -> OracleResult<'_, StaticCo parse_source_qualified_mana_spent_threshold, parse_mana_spent_vs_source_pt, parse_colors_of_mana_spent_threshold, + parse_color_mana_spent_threshold, parse_mana_spent_threshold, parse_combat_context_conditions, parse_put_onto_battlefield_this_way, @@ -6625,6 +6626,39 @@ fn parse_colors_of_mana_spent_threshold(input: &str) -> OracleResult<'_, StaticC )) } +/// CR 106.3 + CR 601.2h: Parse "{at least N | N or more | N or fewer | N or +/// less} mana {was|were} spent to cast " — the Adamant threshold +/// (CR 207.2c ability word; the label itself carries no rules meaning and is +/// peeled by the caller). Measures `CastManaSpentMetric::OfColor` — how much +/// mana of exactly one color paid the cost — against a fixed threshold. +/// +/// Sibling of `parse_colors_of_mana_spent_threshold` (distinct colors) and +/// `parse_mana_spent_threshold` (total): all three share +/// `parse_amount_threshold` and `parse_mana_spent_self_subject`, differing only +/// in the aggregate leaf. CR 400.7d: the subject anaphora selects the scope. +fn parse_color_mana_spent_threshold(input: &str) -> OracleResult<'_, StaticCondition> { + let (rest, (n, comparator)) = parse_amount_threshold(input)?; + let (rest, _) = tag(" ").parse(rest)?; + let (rest, color) = parse_color(rest)?; + let (rest, _) = tag(" mana ").parse(rest)?; + let (rest, _) = alt((tag("was "), tag("were "))).parse(rest)?; + let (rest, _) = tag("spent to cast ").parse(rest)?; + let (rest, scope) = nom_quantity::parse_mana_spent_self_subject(rest)?; + Ok(( + rest, + StaticCondition::QuantityComparison { + lhs: QuantityExpr::Ref { + qty: QuantityRef::ManaSpentToCast { + scope, + metric: CastManaSpentMetric::OfColor { color }, + }, + }, + comparator, + rhs: QuantityExpr::Fixed { value: n as i32 }, + }, + )) +} + /// CR 509.1b + CR 506.5: Parse combat-context conditions. /// /// Handles "defending player controls a/an [type]" and "it's attacking alone". @@ -16735,6 +16769,235 @@ mod tests { ); } + // ----------------------------------------------------------------------- + // CR 106.3 + CR 601.2h: per-color mana-spent threshold (Adamant grammar). + // Group A — accepted forms across every axis of the composed combinator. + // ----------------------------------------------------------------------- + + fn color_mana_spent( + text: &str, + ) -> ( + CastManaObjectScope, + crate::types::mana::ManaColor, + Comparator, + i32, + ) { + let (rest, c) = parse_inner_condition(text) + .unwrap_or_else(|e| panic!("expected {text:?} to parse, got {e:?}")); + assert_eq!(rest, "", "combinator must self-delimit for {text:?}"); + match c { + StaticCondition::QuantityComparison { + lhs: + QuantityExpr::Ref { + qty: + QuantityRef::ManaSpentToCast { + scope, + metric: CastManaSpentMetric::OfColor { color }, + }, + }, + comparator, + rhs: QuantityExpr::Fixed { value }, + } => (scope, color, comparator, value), + other => panic!("expected OfColor QuantityComparison for {text:?}, got {other:?}"), + } + } + + /// All five colors route through the shared `parse_color` leaf — the arm is + /// not hardcoded to white (the color of the first Adamant card fixed). + #[test] + fn color_mana_spent_threshold_all_five_colors() { + use crate::types::mana::ManaColor; + for (text, expected) in [ + ( + "at least three white mana was spent to cast this spell", + ManaColor::White, + ), + ( + "at least three blue mana was spent to cast this spell", + ManaColor::Blue, + ), + ( + "at least three black mana was spent to cast this spell", + ManaColor::Black, + ), + ( + "at least three red mana was spent to cast this spell", + ManaColor::Red, + ), + ( + "at least three green mana was spent to cast this spell", + ManaColor::Green, + ), + ] { + let (scope, color, comparator, value) = color_mana_spent(text); + assert_eq!(scope, CastManaObjectScope::SelfObject); + assert_eq!(color, expected); + assert_eq!(comparator, Comparator::GE); + assert_eq!(value, 3); + } + } + + /// The comparator axis is threaded from the shared `parse_amount_threshold`, + /// not pinned to the single `at least` surface form Adamant happens to use. + #[test] + fn color_mana_spent_threshold_comparator_axis() { + for (text, cmp, n) in [ + ( + "at least two red mana was spent to cast this spell", + Comparator::GE, + 2, + ), + ( + "two or more red mana was spent to cast this spell", + Comparator::GE, + 2, + ), + ( + "two or fewer red mana was spent to cast this spell", + Comparator::LE, + 2, + ), + ( + "two or less red mana was spent to cast this spell", + Comparator::LE, + 2, + ), + ] { + let (_, _, comparator, value) = color_mana_spent(text); + assert_eq!(comparator, cmp, "for {text:?}"); + assert_eq!(value, n, "for {text:?}"); + } + } + + /// CR 400.7d: the subject anaphora selects the scope, and the "was"/"were" + /// number axis is accepted on both surfaces. + #[test] + fn color_mana_spent_threshold_subject_and_number_axes() { + for (text, expected) in [ + ( + "at least three red mana was spent to cast this spell", + CastManaObjectScope::SelfObject, + ), + ( + "at least three red mana was spent to cast this creature", + CastManaObjectScope::SelfObject, + ), + ( + "at least three red mana was spent to cast it", + CastManaObjectScope::SelfObject, + ), + ( + "at least three red mana were spent to cast ~", + CastManaObjectScope::SelfObject, + ), + ( + "at least three red mana was spent to cast that spell", + CastManaObjectScope::TriggeringSpell, + ), + ] { + let (scope, _, _, _) = color_mana_spent(text); + assert_eq!(scope, expected, "for {text:?}"); + } + } + + // ----------------------------------------------------------------------- + // Hostile rows U1-U3: the new arm must not STEAL its siblings' phrases. + // Reach-guard: `color_mana_spent_threshold_all_five_colors` above proves the + // arm is live, so these negatives are not vacuous. + // ----------------------------------------------------------------------- + + /// U1/U2/U3: the sibling phrases must NOT produce an `OfColor` metric. + /// Henge Walker's max-over-colors ("mana of the same color") is explicitly + /// out of scope and must fail closed; the `DistinctColors` and `Total` + /// siblings must keep their own arms. + #[test] + fn color_mana_spent_threshold_does_not_steal_sibling_phrases() { + for text in [ + // U1 — Henge Walker (out of scope, must fail closed entirely). + "at least three mana of the same color was spent to cast this spell", + // U2 — Steel Exemplar's DistinctColors form. + "two or more colors of mana were spent to cast it", + // U3 — the bare Total form, and the source-qualified form. + "at least three mana was spent to cast this spell", + "at least three mana from a treasure was spent to cast this spell", + ] { + let produced_of_color = matches!( + parse_inner_condition(text), + Ok(( + _, + StaticCondition::QuantityComparison { + lhs: QuantityExpr::Ref { + qty: QuantityRef::ManaSpentToCast { + metric: CastManaSpentMetric::OfColor { .. }, + .. + }, + }, + .. + } + )) + ); + assert!(!produced_of_color, "new arm stole sibling phrase {text:?}"); + } + // U1 specifically must not parse AT ALL — a silent success would gate + // Henge Walker on the wrong metric instead of failing closed. + assert!( + parse_inner_condition( + "at least three mana of the same color was spent to cast this spell" + ) + .is_err(), + "max-over-colors is out of scope and must fail closed" + ); + } + + /// Group B — sibling non-regression: the three incumbent metrics keep their + /// exact pre-change output now that a fourth arm sits between them in the + /// `parse_resolution_context_conditions` alt. + #[test] + fn color_mana_spent_threshold_siblings_unchanged() { + let metric_of = |text: &str| match parse_inner_condition(text) { + Ok(( + "", + StaticCondition::QuantityComparison { + lhs: + QuantityExpr::Ref { + qty: QuantityRef::ManaSpentToCast { scope, metric }, + }, + comparator, + rhs: QuantityExpr::Fixed { value }, + }, + )) => (scope, metric, comparator, value), + other => panic!("expected mana-spent comparison for {text:?}, got {other:?}"), + }; + + assert_eq!( + metric_of("two or more colors of mana were spent to cast it"), + ( + CastManaObjectScope::SelfObject, + CastManaSpentMetric::DistinctColors, + Comparator::GE, + 2 + ) + ); + assert_eq!( + metric_of("at least three mana was spent to cast this spell"), + ( + CastManaObjectScope::SelfObject, + CastManaSpentMetric::Total, + Comparator::GE, + 3 + ) + ); + assert_eq!( + metric_of("five or more mana was spent to cast that spell"), + ( + CastManaObjectScope::TriggeringSpell, + CastManaSpentMetric::Total, + Comparator::GE, + 5 + ) + ); + } + #[test] fn test_parse_condition_mana_spent_vs_self_power_only() { let (rest, c) = parse_condition( diff --git a/crates/engine/src/parser/oracle_replacement.rs b/crates/engine/src/parser/oracle_replacement.rs index a74fc1f2e6..75e2d73a50 100644 --- a/crates/engine/src/parser/oracle_replacement.rs +++ b/crates/engine/src/parser/oracle_replacement.rs @@ -3961,6 +3961,15 @@ fn parse_enters_with_counters( // the bug class being fixed (Hotheaded Giant / Steel Exemplar). let (work_text, unless_outcome) = extract_enters_with_unless_suffix(work_text); + // CR 614.1c + CR 207.2c: Peel a SENTENCE-INITIAL "[ — ]if + // , " gate (Adamant / Spell mastery / bare leading + // conditional). Before this, no leading-position gate was recognized at all + // and the whole clause was swallowed, making the replacement unconditional. + // Runs AFTER `extract_kicker_enters_condition`, which already consumes and + // strips its own "if ~ was kicked, " prefix, so kicker lines arrive here as + // "it enters with …" → `NoLeadingIf`. + let (work_text, leading_if_outcome) = extract_enters_with_leading_if_gate(work_text); + // CR 702.138c: "escapes with" / plural-subject "escape with" is // semantically "enters with" gated on escape. let is_escape = nom_primitives::scan_contains(work_text, "escapes with") @@ -4049,7 +4058,8 @@ fn parse_enters_with_counters( // one condition slot — so their co-occurrence fails closed. let other_suffix = enters_with_condition_suffix(is_escape, &kicker_condition, work_text); - match resolve_enters_with_condition(&unless_outcome, other_suffix) { + match resolve_enters_with_condition(&unless_outcome, &leading_if_outcome, other_suffix) + { None => return None, Some(Some(cond)) => def = def.condition(cond), Some(None) => {} @@ -4237,7 +4247,7 @@ fn parse_enters_with_counters( // condition slot, so their co-occurrence fails closed (→ unimplemented) // rather than silently dropping one gate. let other_suffix = enters_with_condition_suffix(is_escape, &kicker_condition, work_text); - match resolve_enters_with_condition(&unless_outcome, other_suffix) { + match resolve_enters_with_condition(&unless_outcome, &leading_if_outcome, other_suffix) { None => return None, Some(Some(cond)) => def = def.condition(cond), Some(None) => {} @@ -4279,22 +4289,51 @@ fn enters_with_condition_suffix( } } -/// CR 614.1c: Reconcile the up-front " unless " gate with the trailing -/// conditional-suffix gate. `None` = fail closed (the caller returns `None` so -/// the line falls through to `Effect::unimplemented`): either the unless clause -/// was present-but-unparsed, or both gates co-occur and there is only one -/// condition slot. `Some(None)` = no gate. `Some(Some(cond))` = the single -/// applicable gate. +/// CR 614.1c: Reconcile the three mutually exclusive gate positions an +/// "enters with" clause can carry — the up-front " unless " gate, the +/// sentence-initial "if , " gate, and the trailing conditional suffix. +/// `ReplacementDefinition` has exactly ONE condition slot, so any co-occurrence +/// fails closed rather than silently dropping a gate. +/// +/// `None` = fail closed (the caller returns `None` so the line falls through to +/// `Effect::unimplemented`): a gate was present-but-unparsed, or two gates +/// co-occur. `Some(None)` = no gate. `Some(Some(cond))` = the single applicable +/// gate. fn resolve_enters_with_condition( unless_outcome: &EntersWithUnlessOutcome, + leading_if_outcome: &EntersWithLeadingIfOutcome, other_suffix: Option, ) -> Option> { - match (unless_outcome, other_suffix) { - (EntersWithUnlessOutcome::Unparsed, _) => None, - (EntersWithUnlessOutcome::Parsed(_), Some(_)) => None, - (EntersWithUnlessOutcome::Parsed(cond), None) => Some(Some(cond.clone())), - (EntersWithUnlessOutcome::NoUnlessClause, Some(other)) => Some(Some(other)), - (EntersWithUnlessOutcome::NoUnlessClause, None) => Some(None), + match (unless_outcome, leading_if_outcome, other_suffix) { + // Present-but-unparsed on either gate axis: fail closed. + (EntersWithUnlessOutcome::Unparsed, _, _) + | (_, EntersWithLeadingIfOutcome::Unparsed, _) => None, + // The " unless " gate, alone. + (EntersWithUnlessOutcome::Parsed(cond), EntersWithLeadingIfOutcome::NoLeadingIf, None) => { + Some(Some(cond.clone())) + } + // Any two gates at once: one slot, fail closed. + (EntersWithUnlessOutcome::Parsed(_), _, Some(_)) + | (EntersWithUnlessOutcome::Parsed(_), EntersWithLeadingIfOutcome::Parsed(_), _) + | (_, EntersWithLeadingIfOutcome::Parsed(_), Some(_)) => None, + // The sentence-initial "if , " gate, alone. + ( + EntersWithUnlessOutcome::NoUnlessClause, + EntersWithLeadingIfOutcome::Parsed(cond), + None, + ) => Some(Some(cond.clone())), + // The trailing conditional suffix, alone. + ( + EntersWithUnlessOutcome::NoUnlessClause, + EntersWithLeadingIfOutcome::NoLeadingIf, + Some(other), + ) => Some(Some(other)), + // No gate at all. + ( + EntersWithUnlessOutcome::NoUnlessClause, + EntersWithLeadingIfOutcome::NoLeadingIf, + None, + ) => Some(None), } } @@ -4426,6 +4465,89 @@ fn extract_enters_with_unless_suffix(text: &str) -> (&str, EntersWithUnlessOutco (head, EntersWithUnlessOutcome::Unparsed) } +/// Outcome of scanning an "enters with [counters]" clause for a SENTENCE-INITIAL +/// "if , " gate (CR 614.1c). +/// +/// The presence axis is decided independently of the parse axis: once `"if "` +/// matches at position 0 the clause IS a gate, so an unrecognized condition +/// yields `Unparsed` (caller fails closed) and never `NoLeadingIf`. Silently +/// dropping the gate — which would make the replacement unconditional — is the +/// bug class this fixes. +#[derive(Debug, Clone)] +enum EntersWithLeadingIfOutcome { + /// No sentence-initial "if " — the payload text is returned byte-identical, + /// ability-word label included. + NoLeadingIf, + /// A recognized game-state condition; the replacement applies only while it + /// holds (only-if polarity). + Parsed(ReplacementCondition), + /// A sentence-initial "if " gate is present but unrecognized — the caller + /// MUST fail closed so the line falls through to `Effect::unimplemented`. + /// + /// Note the fall-through is a SHARED `None` signal: `parse_enters_with_counters` + /// returns `None` both for "not my shape" and for "my shape, gate unparseable", + /// so nothing structurally stops a LATER replacement parser from claiming the + /// same line as an UNCONDITIONAL replacement and reintroducing the bug class. + /// That it does not happen is a corpus fact pinned card-level by + /// `unparseable_leading_if_gate_fails_closed` (Henge Walker), not a type-level + /// guarantee — a new sibling shape with a leading gate needs its own pin. + Unparsed, +} + +/// CR 614.1c: Split a SENTENCE-INITIAL "if , " gate off an +/// "enters with [counter payload]" clause and classify the condition. Returns +/// the gate-free body plus the outcome. +/// +/// This is the leading-position mirror of `extract_enters_with_only_if_suffix` +/// (which scans for a TRAILING " if "). Adamant (Ardenvale Paladin), Spell +/// mastery (Necromantic Summons) and bare-conditional cards (Dust Animus) all +/// write the gate first: "[ — ]If , ~ enters with …". +/// +/// CR 207.2c: the optional em-dash label is an ability word, which has no rules +/// meaning and must be peeled before the `"if "` test. +/// +/// The peel uses `parse_known_ability_word_name`, i.e. the CURATED list in +/// `oracle_modal::ABILITY_WORD_NAMES` — the CR 207.2c ability words PLUS a small +/// number of curated CR 207.2d flavor markers ("protector", "proclamator hailer", +/// "increment"). It is deliberately NOT the loose 4-word `strip_ability_word` +/// heuristic, which would peel ANY short em-dash label: that would strip Scarlet +/// Spider, Ben Reilly's "Sensational Save" (a CR 207.2d flavor word, absent from +/// the curated list), exposing its `"if ~ was cast using web-slinging, …"` body +/// to the `"if "` test. `parse_inner_condition` consumes only `"~ was cast"` +/// there, the `", "` check then fails, and the card would regress from its +/// working `CastVariantPaid` routing to `Unparsed`. +/// +/// The three curated flavor markers ARE peelable, so this is a bounded widening +/// rather than a strict CR 207.2c gate. That is accepted, not overlooked: no card +/// in the corpus writes `" — If …, … enters with …"`, and a +/// future one would fail closed to `Effect::unimplemented` (honest gap), never +/// silently drop its gate. Pinned by `flavor_word_label_is_not_peelable_as_an_ability_word`. +fn extract_enters_with_leading_if_gate(text: &str) -> (&str, EntersWithLeadingIfOutcome) { + let after_label = opt(terminated( + super::oracle_modal::parse_known_ability_word_name, + alt((tag(" — "), tag(" – "), tag(" - "))), + )) + .parse(text) + .map_or(text, |(rest, _)| rest); + + let Ok((condition_text, _)) = tag::<_, _, OracleError<'_>>("if ").parse(after_label) else { + return (text, EntersWithLeadingIfOutcome::NoLeadingIf); + }; + + // `parse_inner_condition` is self-delimiting (not `all_consuming`), so it + // stops at the comma that separates the gate from the payload. + let Ok((rest, condition)) = parse_inner_condition(condition_text) else { + return (text, EntersWithLeadingIfOutcome::Unparsed); + }; + let Ok((body, _)) = tag::<_, _, OracleError<'_>>(", ").parse(rest) else { + return (text, EntersWithLeadingIfOutcome::Unparsed); + }; + match replacement_condition_from_static(condition) { + Some(cond) => (body, EntersWithLeadingIfOutcome::Parsed(cond)), + None => (text, EntersWithLeadingIfOutcome::Unparsed), + } +} + /// CR 614.1c + CR 614.1d: Map a parsed `StaticCondition` to the `unless`-polarity /// `ReplacementCondition`. Unlike `replacement_condition_from_static` (the /// only-if polarity used by "enters with ... if ..."), an "unless" clause @@ -21069,6 +21191,207 @@ mod tests { "copy-effect must not match the modal as-enters recognizer" ); } + + // ----------------------------------------------------------------------- + // CR 614.1c + CR 207.2c: the sentence-initial "if , " enters-with gate. + // ----------------------------------------------------------------------- + + /// POSITIVE reach-guard for every negative below: Ardenvale Paladin's + /// Adamant gate is peeled off the ability-word label and attached as an + /// only-if `ReplacementCondition`. Revert the peel and `condition` is + /// `None` — the counter would apply unconditionally. + #[test] + fn adamant_leading_if_gate_attaches_only_if_quantity() { + let def = parse_replacement_line( + "Adamant — If at least three white mana was spent to cast this spell, this creature \ + enters with a +1/+1 counter on it.", + "Ardenvale Paladin", + ) + .expect("Adamant enters-with must still parse to a replacement"); + + assert_eq!( + def.condition, + Some(ReplacementCondition::OnlyIfQuantity { + lhs: QuantityExpr::Ref { + qty: QuantityRef::ManaSpentToCast { + scope: crate::types::ability::CastManaObjectScope::SelfObject, + metric: crate::types::ability::CastManaSpentMetric::OfColor { + color: ManaColor::White, + }, + }, + }, + comparator: Comparator::GE, + rhs: QuantityExpr::Fixed { value: 3 }, + active_player_req: None, + }), + "the Adamant gate must be attached, not swallowed" + ); + // Payload untouched by the peel: still one +1/+1 counter on itself. + let execute = def.execute.as_deref().expect("execute present"); + assert!(matches!( + &*execute.effect, + Effect::PutCounter { + counter_type: CounterType::Plus1Plus1, + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::SelfRef, + } + )); + } + + /// U5 — hostile co-occurrence rows. `ReplacementDefinition` has exactly one + /// condition slot, so a leading "if" gate that co-occurs with either of the + /// other two gate positions MUST fail closed (`None`), never silently drop + /// one. Both rows are dead on the current corpus; this pins them. + /// Reach-guard: the two single-gate rows immediately below return `Some`. + #[test] + fn resolve_enters_with_condition_fails_closed_on_gate_co_occurrence() { + let gate = || ReplacementCondition::CastViaEscape; + let other = || ReplacementCondition::YouAttackedThisTurn; + + // (NoUnlessClause, Parsed, Some) — leading gate + trailing suffix. + assert!(resolve_enters_with_condition( + &EntersWithUnlessOutcome::NoUnlessClause, + &EntersWithLeadingIfOutcome::Parsed(gate()), + Some(other()), + ) + .is_none()); + // (Parsed, Parsed, None) — unless gate + leading gate. + assert!(resolve_enters_with_condition( + &EntersWithUnlessOutcome::Parsed(other()), + &EntersWithLeadingIfOutcome::Parsed(gate()), + None, + ) + .is_none()); + // Present-but-unparsed leading gate fails closed regardless of the rest. + assert!(resolve_enters_with_condition( + &EntersWithUnlessOutcome::NoUnlessClause, + &EntersWithLeadingIfOutcome::Unparsed, + None, + ) + .is_none()); + + // POSITIVE reach-guards: each gate ALONE still resolves. + assert_eq!( + resolve_enters_with_condition( + &EntersWithUnlessOutcome::NoUnlessClause, + &EntersWithLeadingIfOutcome::Parsed(gate()), + None, + ), + Some(Some(gate())) + ); + assert_eq!( + resolve_enters_with_condition( + &EntersWithUnlessOutcome::NoUnlessClause, + &EntersWithLeadingIfOutcome::NoLeadingIf, + Some(other()), + ), + Some(Some(other())) + ); + assert_eq!( + resolve_enters_with_condition( + &EntersWithUnlessOutcome::NoUnlessClause, + &EntersWithLeadingIfOutcome::NoLeadingIf, + None, + ), + Some(None) + ); + } + + /// U6 — LOAD-BEARING: the peel uses `parse_known_ability_word_name` (the + /// curated `ABILITY_WORD_NAMES` list — the CR 207.2c ability words plus + /// three curated CR 207.2d flavor markers; see the contract on + /// `extract_enters_with_leading_if_gate`), NOT the loose 4-word + /// `strip_ability_word` heuristic. What matters here is that + /// "Sensational Save" is absent from that curated list; it is NOT the case + /// that every CR 207.2d flavor word is absent. + /// If "Sensational Save" were peelable, Scarlet Spider's body would + /// reach `tag("if ")`, `parse_inner_condition` would consume only + /// "~ was cast", the following `tag(", ")` would fail, and the card would + /// regress from green to `Unparsed` → `Effect::unimplemented`. + #[test] + fn flavor_word_label_is_not_peelable_as_an_ability_word() { + assert!( + super::super::oracle_modal::parse_known_ability_word_name( + "sensational save — if ~ was cast using web-slinging, he enters with x +1/+1 \ + counters on him." + ) + .is_err(), + "\"Sensational Save\" is absent from the curated ABILITY_WORD_NAMES list and must \ + stay unpeelable — peeling it regresses Scarlet Spider, Ben Reilly to Unparsed. \ + (Note the list DOES contain three curated CR 207.2d markers, so this is the \ + specific case, not a universal 'no flavor word is peelable' invariant.)" + ); + // Positive reach-guard: a real CR 207.2c ability word IS peeled, so the + // negative above is not vacuous. + assert_eq!( + super::super::oracle_modal::parse_known_ability_word_name( + "adamant — if at least three white mana was spent to cast this spell, ~ enters \ + with a +1/+1 counter on it." + ) + .map(|(_, name)| name), + Ok("adamant") + ); + } + + /// U7 — positive card-level pin for F1: Scarlet Spider's gate is still the + /// web-slinging suffix condition, reached by the scan-anywhere route the + /// peel deliberately does not intercept. + #[test] + fn flavor_word_card_keeps_its_scan_anywhere_gate() { + let def = parse_replacement_line( + "Sensational Save — If Scarlet Spider was cast using web-slinging, he enters with X \ + +1/+1 counters on him, where X is the mana value of the returned creature.", + "Scarlet Spider, Ben Reilly", + ) + .expect("Scarlet Spider's enters-with must still parse"); + assert_eq!( + def.condition, + Some(ReplacementCondition::CastVariantPaid { + variant: CastVariantPaid::WebSlinging, + }) + ); + } + + /// U9 — honest failure, both polarities. Henge Walker's max-over-colors + /// ("mana of the same color") has no grammar and Red and Black Legac chains + /// three sentence-initial "if"s into one condition slot; both are explicitly + /// out of scope and MUST fail closed rather than parse to an unconditional + /// replacement. Reach-guard: the parseable sibling + /// `adamant_leading_if_gate_attaches_only_if_quantity` proves the peel is + /// live, so `None` here is not a dead-code pass. + #[test] + fn unparseable_leading_if_gate_fails_closed() { + assert!( + parse_replacement_line( + "Adamant — If at least three mana of the same color was spent to cast this spell, \ + this creature enters with a +1/+1 counter on it.", + "Henge Walker", + ) + .is_none(), + "an unrecognized leading gate must fail closed, not become unconditional" + ); + // Card level: the line falls through to `Effect::unimplemented` — the + // engine reports the gap instead of silently mis-resolving. + let card = parse_oracle_text( + "Adamant — If at least three mana of the same color was spent to cast this spell, \ + this creature enters with a +1/+1 counter on it.", + "Henge Walker", + &[], + &["Artifact".to_string(), "Creature".to_string()], + &["Golem".to_string()], + ); + assert!( + card.replacements.is_empty(), + "no replacement may be published for the fail-closed line" + ); + assert!( + card.abilities + .iter() + .any(|a| matches!(&*a.effect, Effect::Unimplemented { .. })), + "the fail-closed line must surface as Effect::Unimplemented, got {:?}", + card.abilities + ); + } } /// Snapshot tests locking current replacement parser output before/after the IR split. diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 3a8322bce2..ee1b5084a6 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -5439,6 +5439,11 @@ pub enum CastManaSpentMetric { Total, /// Number of distinct colors of mana spent. DistinctColors, + /// CR 106.3 + CR 601.2h: Amount of mana of one specific color spent to pay + /// the cost — the Adamant axis ("if at least three white mana was spent to + /// cast this spell"). A leaf parameterization of the same payment record + /// `Total` and `DistinctColors` aggregate differently. + OfColor { color: ManaColor }, /// Amount of mana whose source matched the filter at payment time. FromSource { source_filter: TargetFilter }, } @@ -18288,8 +18293,18 @@ pub enum AbilityCondition { /// CR 601.3b + CR 702.8a: The source permanent came from a spell cast using /// a specific timing permission this turn. CastTimingPermission { permission: CastTimingPermission }, - /// CR 601.2h + CR 608.2c: "if {C} was spent to cast this spell" gates - /// resolution on the source object's recorded paid-mana colors. + /// CR 601.2h: "if {C} was spent to cast this spell" gates resolution on the + /// source object's recorded paid-mana colors. + /// + /// LEGACY SHAPE. The canonical generic form is + /// `QuantityCheck { lhs: ManaSpentToCast { scope, OfColor { color } }, GE, Fixed(minimum) }`, + /// which carries the CR 400.7d subject anaphora as an explicit + /// `CastManaObjectScope` that this variant cannot express. The leading-word + /// Adamant grammar already emits the generic form; this variant survives + /// only for the symbolic `{W}{W}` phrasing, which has no + /// `parse_inner_condition` grammar and fans into `And`/`Not` compositions. + /// Retiring it is a semantic migration (per-card scope decision), not a + /// rename — see `TriggerCondition::ManaColorSpent` for the sibling case. ManaColorSpent { color: ManaColor, minimum: u32 }, /// CR 608.2c: "If it's a [type] card" — gates sub_ability on the last /// revealed card's type, or on the just-moved card when the parent effect @@ -19400,6 +19415,14 @@ pub enum TriggerCondition { /// a specific timing permission this turn. CastTimingPermission { permission: CastTimingPermission }, /// CR 207.2c: "if at least N mana of [color] was spent to cast this spell" — Adamant. + /// + /// LEGACY SHAPE, produced by the independent trigger-side grammar in + /// `parser::oracle_trigger`. The canonical generic form is + /// `QuantityCheck { lhs: ManaSpentToCast { scope, OfColor { color } }, GE, Fixed(minimum) }` + /// (see `AbilityCondition::ManaColorSpent`). Converging this one is a + /// SEMANTIC migration, not a rename: the producer accepts both "this spell" + /// and "that spell" and records neither, so lowering requires a per-card + /// CR 400.7d `CastManaObjectScope` decision. ManaColorSpent { color: ManaColor, minimum: u32 }, /// CR 601.2b: "if no mana was spent to cast it" / "if mana from a [source] was spent" ManaSpentCondition { text: String }, diff --git a/crates/engine/tests/integration/adamant_enters_with_leading_if_gate.rs b/crates/engine/tests/integration/adamant_enters_with_leading_if_gate.rs new file mode 100644 index 0000000000..3dc89acdc9 --- /dev/null +++ b/crates/engine/tests/integration/adamant_enters_with_leading_if_gate.rs @@ -0,0 +1,466 @@ +//! Runtime cast-pipeline coverage for the SENTENCE-INITIAL "if , " +//! enters-with gate (CR 614.1c), the position Adamant and Spell mastery cards +//! write their gate in. +//! +//! Before the fix, `parse_enters_with_counters` recognized a gate only in the +//! trailing " unless …" / " … if …" positions, so a leading gate was silently +//! swallowed and the replacement became UNCONDITIONAL — the Adamant Paladins +//! entered with a +1/+1 counter no matter what mana paid for them. +//! +//! Built via the `/card-test` recipe: `GameScenario` + +//! `GameRunner::cast(..).resolve()` + `CastOutcome` counter/life deltas, on +//! verbatim Oracle text. Every negative assertion is paired with a positive +//! reach-guard in the same test AND with a structural guard proving the card +//! parsed (a `Some` replacement condition, zero `Effect::Unimplemented`), so an +//! upstream parse failure cannot satisfy it vacuously. +//! +//! REVERT DISCRIMINATOR: `ardenvale_paladin_white_below_threshold_no_counter` +//! (R2). Neutralize `extract_enters_with_leading_if_gate` to always return +//! `NoLeadingIf` and the gate is dropped again — the counter applies +//! unconditionally and R2's `assert_counters(.., 0)` fails. + +use engine::game::scenario::{CastOutcome, GameRunner, GameScenario, P0, P1}; +use engine::types::ability::Effect; +use engine::types::counter::CounterType; +use engine::types::identifiers::ObjectId; +use engine::types::keywords::{Keyword, KeywordKind}; +use engine::types::mana::{ManaColor, ManaCost, ManaCostShard, ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +/// Ardenvale Paladin {3}{W} 2/3 — verbatim Oracle text (`data/card-data.json`). +const ARDENVALE_PALADIN: &str = "Adamant — If at least three white mana was spent to cast this \ + spell, this creature enters with a +1/+1 counter on it."; + +/// Embereth Paladin {3}{R} 3/1 — verbatim Oracle text, Haste line included. +const EMBERETH_PALADIN: &str = "Haste\nAdamant — If at least three red mana was spent to cast \ + this spell, this creature enters with a +1/+1 counter on it."; + +/// Dust Animus {1}{W} 1/1 — verbatim Oracle text. The Plot line is dropped +/// because plotting is irrelevant here and its reminder text would only add +/// unrelated parse surface; the leading-if line under test is verbatim. +const DUST_ANIMUS: &str = "Flying\nIf you control five or more untapped lands, this creature \ + enters with two +1/+1 counters and a lifelink counter on it."; + +/// Slaying Fire {2}{R} — verbatim Oracle text. Guards the Adamant ABILITY +/// RIDER, which converges on the same `OfColor` quantity shape as the +/// replacement gate. +const SLAYING_FIRE: &str = "Slaying Fire deals 3 damage to any target.\nAdamant — If at least \ + three red mana was spent to cast this spell, it deals 4 damage \ + instead."; + +fn mana(kind: ManaType, n: usize) -> Vec { + vec![ManaUnit::new(kind, ObjectId(0), false, vec![]); n] +} + +/// Build a pool from a colored/colorless mix, in one place so each row reads as +/// its payment record. +fn pool(colored: &[(ManaType, usize)]) -> Vec { + colored + .iter() + .flat_map(|(kind, n)| mana(*kind, *n)) + .collect() +} + +/// Cast a 4-mana Paladin ({3}{}) out of an exactly-sized pool and return +/// the outcome plus its object id. +/// +/// The pool always holds EXACTLY the four mana the cost needs, so the auto-payer +/// has no discretion: every unit in the pool is spent and `colors_spent_to_cast` +/// (CR 601.2h) is fully determined by the pool contents. That removes payment +/// nondeterminism from every assertion below. +fn cast_paladin( + name: &str, + oracle: &str, + shard: ManaCostShard, + power: i32, + toughness: i32, + payment: &[(ManaType, usize)], +) -> (CastOutcome, ObjectId) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let paladin = scenario + .add_creature_to_hand_from_oracle(P0, name, power, toughness, oracle) + .with_mana_cost(ManaCost::Cost { + generic: 3, + shards: vec![shard], + }) + .id(); + scenario.with_mana_pool(P0, pool(payment)); + let mut runner = scenario.build(); + + assert_gate_is_attached(&runner, paladin, name); + + let outcome = runner.cast(paladin).resolve(); + assert_eq!( + outcome.zone_of(paladin), + Zone::Battlefield, + "{name} must resolve onto the battlefield" + ); + (outcome, paladin) +} + +/// Structural reach-guard (the `/card-test` foot-gun #6 defence): the card +/// really parsed, the enters-with replacement really exists, and it really +/// carries a condition. Without this, a "0 counters" assertion would pass just +/// as well on a card whose replacement failed to parse at all. +fn assert_gate_is_attached(runner: &GameRunner, obj: ObjectId, name: &str) { + let object = &runner.state().objects[&obj]; + assert!( + !object + .abilities + .iter() + .any(|a| matches!(&*a.effect, Effect::Unimplemented { .. })), + "{name} must parse with zero Effect::Unimplemented, got {:?}", + object.abilities + ); + let def = object + .replacement_definitions + .first() + .unwrap_or_else(|| panic!("{name} must publish an enters-with replacement")); + assert!( + def.condition.is_some(), + "{name}'s enters-with replacement must carry the leading-if gate; \ + a None condition means the gate was swallowed and the counter is unconditional" + ); +} + +// --------------------------------------------------------------------------- +// R1-R6 — the Adamant per-color threshold, CR 106.3 + CR 601.2h. +// --------------------------------------------------------------------------- + +/// R1 — POSITIVE reach-guard for the whole white family. {W}{W}{W}{W} pays +/// {3}{W}: white spent = 4 >= 3, so the counter applies. Discriminates +/// `OfColor` from `DistinctColors` (which is 1 here, below the threshold). +#[test] +fn ardenvale_paladin_four_white_applies_counter() { + let (outcome, paladin) = cast_paladin( + "Ardenvale Paladin", + ARDENVALE_PALADIN, + ManaCostShard::White, + 2, + 3, + &[(ManaType::White, 4)], + ); + outcome.assert_counters(paladin, CounterType::Plus1Plus1, 1); +} + +/// R2 — **THE PRIMARY REVERT DISCRIMINATOR.** One white + three colorless pays +/// {3}{W}: white spent = 1 < 3, so NO counter. Total mana spent is 4 >= 3, so +/// this row also discriminates `OfColor` from `CastManaSpentMetric::Total`. +/// +/// Drop the leading-if peel and the replacement becomes unconditional → 1 +/// counter → this assertion fails. Paired reach-guard: R1 above. +#[test] +fn ardenvale_paladin_white_below_threshold_no_counter() { + let (outcome, paladin) = cast_paladin( + "Ardenvale Paladin", + ARDENVALE_PALADIN, + ManaCostShard::White, + 2, + 3, + &[(ManaType::White, 1), (ManaType::Colorless, 3)], + ); + outcome.assert_counters(paladin, CounterType::Plus1Plus1, 0); +} + +/// R6 — pins the comparator (GE, not GT) and the literal threshold 3. Exactly +/// three white → counter applies; exactly two white → it does not. +#[test] +fn ardenvale_paladin_threshold_is_greater_or_equal_three() { + let (at_threshold, paladin) = cast_paladin( + "Ardenvale Paladin", + ARDENVALE_PALADIN, + ManaCostShard::White, + 2, + 3, + &[(ManaType::White, 3), (ManaType::Colorless, 1)], + ); + at_threshold.assert_counters(paladin, CounterType::Plus1Plus1, 1); + + let (below, paladin) = cast_paladin( + "Ardenvale Paladin", + ARDENVALE_PALADIN, + ManaCostShard::White, + 2, + 3, + &[(ManaType::White, 2), (ManaType::Colorless, 2)], + ); + below.assert_counters(paladin, CounterType::Plus1Plus1, 0); +} + +/// R4 — POSITIVE reach-guard for the red family, and proof the color is read +/// per-card rather than hardcoded to the first card fixed (white). Three red + +/// one colorless pays {3}{R}: red spent = 3 >= 3 → counter. +#[test] +fn embereth_paladin_three_red_applies_counter() { + let (outcome, paladin) = cast_paladin( + "Embereth Paladin", + EMBERETH_PALADIN, + ManaCostShard::Red, + 3, + 1, + &[(ManaType::Red, 3), (ManaType::Colorless, 1)], + ); + outcome.assert_counters(paladin, CounterType::Plus1Plus1, 1); +} + +/// R3 — the gate reads the card's OWN color. {W}{W}{W}{R} pays Embereth's +/// {3}{R}: red = 1 < 3 (no counter) even though WHITE = 3 would have passed +/// Ardenvale's gate, and total = 4 would have passed a `Total` gate. +/// Paired reach-guard: R4 above. +#[test] +fn embereth_paladin_reads_red_not_white() { + let (outcome, paladin) = cast_paladin( + "Embereth Paladin", + EMBERETH_PALADIN, + ManaCostShard::Red, + 3, + 1, + &[(ManaType::White, 3), (ManaType::Red, 1)], + ); + outcome.assert_counters(paladin, CounterType::Plus1Plus1, 0); +} + +/// R5 — the row that separates `OfColor` from `DistinctColors` at a point where +/// `DistinctColors` PASSES. {W}{U}{B}{R} pays Embereth's {3}{R}: four distinct +/// colors (>= 3, so a `DistinctColors` gate would fire) but red = 1 < 3, so the +/// correct `OfColor` gate does not. Paired reach-guard: R4 above. +#[test] +fn embereth_paladin_four_distinct_colors_still_no_counter() { + let (outcome, paladin) = cast_paladin( + "Embereth Paladin", + EMBERETH_PALADIN, + ManaCostShard::Red, + 3, + 1, + &[ + (ManaType::White, 1), + (ManaType::Blue, 1), + (ManaType::Black, 1), + (ManaType::Red, 1), + ], + ); + outcome.assert_counters(paladin, CounterType::Plus1Plus1, 0); +} + +// --------------------------------------------------------------------------- +// R7 — Dust Animus: a leading-if gate that is NOT a mana-spent threshold. +// Proves the peel makes the WHOLE `parse_inner_condition` grammar reachable +// from the sentence-initial position, not just the one new arm. +// --------------------------------------------------------------------------- + +/// Cast Dust Animus ({1}{W}) out of an exact pool while controlling +/// `untapped_lands` untapped and `tapped_lands` tapped Plains. The lands are +/// never tapped for mana (the pool is pre-staged), so their tap state is +/// controlled purely by the fixture. +fn cast_dust_animus(untapped_lands: usize, tapped_lands: usize) -> (CastOutcome, ObjectId) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let animus = scenario + .add_creature_to_hand_from_oracle(P0, "Dust Animus", 1, 1, DUST_ANIMUS) + .with_mana_cost(ManaCost::Cost { + generic: 1, + shards: vec![ManaCostShard::White], + }) + .id(); + let mut to_tap = Vec::new(); + for _ in 0..untapped_lands { + scenario.add_basic_land(P0, ManaColor::White); + } + for _ in 0..tapped_lands { + to_tap.push(scenario.add_basic_land(P0, ManaColor::White)); + } + scenario.with_mana_pool(P0, pool(&[(ManaType::White, 1), (ManaType::Colorless, 1)])); + let mut runner = scenario.build(); + for land in to_tap { + runner.state_mut().objects.get_mut(&land).unwrap().tapped = true; + } + + assert_gate_is_attached(&runner, animus, "Dust Animus"); + + let outcome = runner.cast(animus).resolve(); + assert_eq!( + outcome.zone_of(animus), + Zone::Battlefield, + "Dust Animus must resolve onto the battlefield" + ); + (outcome, animus) +} + +/// R7a — POSITIVE reach-guard: five untapped lands satisfies the gate, so Dust +/// Animus keeps both counter payloads. This is the "the peel did not break the +/// already-green card" row (Dust Animus is `supported=true` today, but was +/// silently UNCONDITIONAL — the gate was swallowed). +#[test] +fn dust_animus_five_untapped_lands_applies_counters() { + let (outcome, animus) = cast_dust_animus(5, 0); + outcome.assert_counters(animus, CounterType::Plus1Plus1, 2); + assert_eq!( + outcome.counters(animus, CounterType::Keyword(KeywordKind::Lifelink)), + 1, + "the lifelink counter rides the same gated payload" + ); +} + +/// R7b — REVERT DISCRIMINATOR (second polarity). Six lands, one of them TAPPED, +/// leaves four untapped: below the "five or more untapped lands" threshold, so +/// no counters. Drop the peel and the payload applies unconditionally → 2 +1/+1 +/// counters → this fails. Also discriminates `FilterProp::Untapped` from a bare +/// land count: a count-only reading would see six lands and fire. +/// Paired reach-guard: R7a above. +#[test] +fn dust_animus_only_four_untapped_lands_no_counters() { + let (outcome, animus) = cast_dust_animus(4, 2); + outcome.assert_counters(animus, CounterType::Plus1Plus1, 0); + assert_eq!( + outcome.counters(animus, CounterType::Keyword(KeywordKind::Lifelink)), + 0, + "the whole gated payload is suppressed, not just the +1/+1 counters" + ); +} + +// --------------------------------------------------------------------------- +// R8 — the Adamant ABILITY RIDER. The same grammar change re-routes 11 riders +// from `AbilityCondition::ManaColorSpent` to the generic +// `QuantityCheck { ManaSpentToCast { .., OfColor } }`. This is the mandatory +// runtime guard on that AST churn: the observable damage must not move. +// --------------------------------------------------------------------------- + +fn cast_slaying_fire(payment: &[(ManaType, usize)]) -> CastOutcome { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let fire = scenario + .add_spell_to_hand_from_oracle(P0, "Slaying Fire", true, SLAYING_FIRE) + .with_mana_cost(ManaCost::Cost { + generic: 2, + shards: vec![ManaCostShard::Red], + }) + .id(); + scenario.with_mana_pool(P0, pool(payment)); + let mut runner = scenario.build(); + assert!( + !runner.state().objects[&fire] + .abilities + .iter() + .any(|a| matches!(&*a.effect, Effect::Unimplemented { .. })), + "Slaying Fire must parse with zero Effect::Unimplemented, got {:?}", + runner.state().objects[&fire].abilities + ); + runner.cast(fire).target_player(P1).resolve() +} + +/// R8 positive: three red mana satisfies the Adamant rider → 4 damage, not 3. +#[test] +fn slaying_fire_three_red_deals_four() { + let outcome = cast_slaying_fire(&[(ManaType::Red, 3)]); + outcome.assert_life_delta(P1, -4); +} + +/// R8 negative (paired with the positive above): one red + two colorless is +/// three TOTAL mana but only one RED, so the rider does not fire → 3 damage. +/// Discriminates `OfColor` from `Total` on the ability-rider path exactly as R2 +/// does on the replacement path. +#[test] +fn slaying_fire_one_red_deals_three() { + let outcome = cast_slaying_fire(&[(ManaType::Red, 1), (ManaType::Colorless, 2)]); + outcome.assert_life_delta(P1, -3); +} + +// --------------------------------------------------------------------------- +// R9 — the Adamant rider whose payload is a CONTINUOUS STATIC GRANT, not an +// ability-level effect. This subclass routes DIFFERENTLY from R8: the gate +// lands on `StaticDefinition.condition` (CR 613 layer 6 keyword grant) rather +// than on the ability's own `condition`, because the payload is a continuous +// effect over a set of permanents. +// +// Why this test exists: the CI parse-diff bot reports Silverflame Ritual's +// ABILITY-level `conditional` as `3+ White spent → ∅`, which reads like a +// dropped gate — the exact bug class this file guards. It is NOT a drop; the +// condition moved down one layer onto the static. The bot's signature reads the +// ability level only, so a layer move renders as `∅`. R8 cannot catch this +// because Slaying Fire's payload is ability-level. Nothing else pins it, so a +// future refactor really COULD drop the static's condition and every existing +// test here would stay green while "creatures you control gain vigilance" +// became unconditional. +// --------------------------------------------------------------------------- + +/// Silverflame Ritual {3}{W} — verbatim Oracle text (`data/card-data.json`). +const SILVERFLAME_RITUAL: &str = "Put a +1/+1 counter on each creature you control.\nAdamant — If \ + at least three white mana was spent to cast this spell, \ + creatures you control gain vigilance until end of turn."; + +/// Casts Silverflame Ritual with `payment` and returns the runner plus the +/// controller's creature, so the caller can read granted keywords off the board +/// AFTER resolution (a continuous grant is not visible in `CastOutcome` deltas). +fn cast_silverflame_ritual(payment: &[(ManaType, usize)]) -> (GameRunner, ObjectId) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let bear = scenario.add_vanilla(P0, 2, 2); + let ritual = scenario + .add_spell_to_hand_from_oracle(P0, "Silverflame Ritual", true, SILVERFLAME_RITUAL) + .with_mana_cost(ManaCost::Cost { + generic: 3, + shards: vec![ManaCostShard::White], + }) + .id(); + scenario.with_mana_pool(P0, pool(payment)); + let mut runner = scenario.build(); + assert!( + !runner.state().objects[&ritual] + .abilities + .iter() + .any(|a| matches!(&*a.effect, Effect::Unimplemented { .. })), + "Silverflame Ritual must parse with zero Effect::Unimplemented, got {:?}", + runner.state().objects[&ritual].abilities + ); + runner.cast(ritual).resolve(); + (runner, bear) +} + +/// R9 positive reach-guard: three white mana satisfies the rider → the creature +/// gains vigilance. Also proves the UNGATED half of the card resolved (the +/// +1/+1 counter), so a total failure to resolve cannot masquerade as a pass. +#[test] +fn silverflame_ritual_three_white_grants_vigilance() { + let (runner, bear) = cast_silverflame_ritual(&[(ManaType::White, 3), (ManaType::Colorless, 1)]); + let obj = &runner.state().objects[&bear]; + assert_eq!( + obj.counters + .get(&CounterType::Plus1Plus1) + .copied() + .unwrap_or(0), + 1, + "the ungated first line must always resolve — if this is 0 the spell never resolved \ + and the vigilance assertion below would be vacuous" + ); + assert!( + obj.has_keyword(&Keyword::Vigilance), + "three white mana satisfies the Adamant rider, so the static grant must apply" + ); +} + +/// R9 negative (paired with the reach-guard above): one white + three colorless +/// is four TOTAL mana but only one WHITE, so the rider must NOT fire. The +/// +1/+1 counter still lands, proving the spell resolved and the absence of +/// vigilance is a real gate decision rather than a non-resolution. +/// +/// This is the assertion that would fail if the static's condition were ever +/// dropped — i.e. if the `∅` the CI bot displays ever became literally true. +#[test] +fn silverflame_ritual_one_white_does_not_grant_vigilance() { + let (runner, bear) = cast_silverflame_ritual(&[(ManaType::White, 1), (ManaType::Colorless, 3)]); + let obj = &runner.state().objects[&bear]; + assert_eq!( + obj.counters + .get(&CounterType::Plus1Plus1) + .copied() + .unwrap_or(0), + 1, + "the ungated first line must still resolve, so the vigilance check below is not vacuous" + ); + assert!( + !obj.has_keyword(&Keyword::Vigilance), + "only one white mana was spent — the Adamant static grant must stay gated off" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 7e430108e7..647c5edb8e 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -1,6 +1,7 @@ mod abigale_integration; mod abundance_optional_draw_replacement; mod ad_nauseam_repeat; +mod adamant_enters_with_leading_if_gate; mod adapter_contract_fixtures; mod advanced_reconstruction_regression; mod ajani_nacatl_pariah_co_departure_6427; diff --git a/docs/parser-misparse-backlog.md b/docs/parser-misparse-backlog.md index b951858543..5deb64c09f 100644 --- a/docs/parser-misparse-backlog.md +++ b/docs/parser-misparse-backlog.md @@ -13,7 +13,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top | # | Root cause | # cards | Fix hint (where it likely lives) | |---|------------|--------:|----------------------------------| | 1 | Relative-clause / filter restriction on target dropped | 746 | oracle_target.rs / game/filter.rs — extend TargetFilter property extraction for trailing relative clauses | -| 2 | Dropped intervening-if / gating condition (condition: null) | 590 | oracle_nom/condition.rs parse_inner_condition — trigger/static parsers must delegate condition extraction here | +| 2 | Dropped intervening-if / gating condition (condition: null) | 586 | oracle_nom/condition.rs parse_inner_condition — trigger/static parsers must delegate condition extraction here | | 3 | Anaphor bound to wrong referent | 404 | oracle_quantity.rs context-ref resolution + game/ability_utils.rs forward_result wiring | | 4 | Conjoined / chained second effect clause dropped | 387 | oracle.rs effect-chain composition — split on 'and'/'then'/sentence boundaries and build sub_ability chain | | 5 | Dropped 'for each' / dynamic count collapsed to Fixed | 330 | oracle_quantity.rs parse_for_each_clause / parse_quantity_ref — thread ForEach/ObjectCount into the effect count field | @@ -805,12 +805,31 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top -### 2. Dropped intervening-if / gating condition (condition: null) (590 cards) +### 2. Dropped intervening-if / gating condition (condition: null) (586 cards) **Signature.** Trigger/static/replacement/spell condition left null though Oracle has an 'if/while/as long as/unless' game-state gate; the effect resolves unconditionally (CR 603.4 / 608.2c). **Fix hint.** oracle_nom/condition.rs parse_inner_condition — trigger/static parsers must delegate condition extraction here +**Status note (sentence-initial "If <condition>, ~ enters with …" sub-class, CR 614.1c).** +`parse_enters_with_counters` now peels an optional CR 207.2c ability-word label plus a +sentence-initial `if , ` gate and attaches it as the replacement's condition, +making the whole `parse_inner_condition` grammar reachable from that position (previously +NO shape was). A new `CastManaSpentMetric::OfColor` leaf covers the Adamant per-color +threshold. Removed from the list below: Ardenvale Paladin, Locthwain Paladin, +Vantress Paladin (Adamant, now gated and `supported`), and Dust Animus (its +"five or more untapped lands" gate was silently dropped; now attached). Embereth Paladin +and Garenbrig Paladin are fixed by the same change but appear nowhere in this file, so no +line was removed for them. **Deliberately KEPT:** Henge Walker — its +"at least three mana of the same color" is a max-over-colors metric with no grammar and +still fails closed; and Necromantic Summons (listed under §11, not here) — §11's root cause +remains true because its replacement carries `valid_card: SelfRef` on a sorcery, so +`object_replacement_candidate_applies` can never match it; the condition now parses but the +replacement stays dead code. +Observed, not introduced: this section's committed header count (590 before this edit) did +not match its own bullet list (597 entries). The counts here were decremented by the four +removals; the pre-existing 7-entry drift is untouched. +
Cards - A-Paragon of Modernity @@ -834,7 +853,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Antiquities on the Loose - Arcbound Tracker - Arcum's Sleigh -- Ardenvale Paladin - Arid Archway - Ascendant Packleader - Ashen Ghoul @@ -954,7 +972,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Drop of Honey - Drowner of Truth - Drownyard Behemoth -- Dust Animus - Dúnedain Rangers - Earwig Squad - Ego Drain @@ -1118,7 +1135,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Lighthouse Chronologist - Lightning Dart - Liliana's Defeat -- Locthwain Paladin - Lord Skitter's Blessing - Loxodon Surveyor - Ludevic, Necro-Alchemist @@ -1364,7 +1380,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Valiant Emberkin - Vampire Scrivener - Vampire Socialite -- Vantress Paladin - Venom's Hunger - Venom, Eddie Brock - Verity Circle