From 2ff39f9263f02b11543a8747605e2d87a7a2e232 Mon Sep 17 00:00:00 2001 From: cuinhellcat <217210902+cuinhellcat@users.noreply.github.com> Date: Tue, 18 Aug 2026 14:06:04 +0200 Subject: [PATCH 1/3] fix(parser): keep the source exclusion in "tap another untapped" costs (#7522) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CR 109.4 + CR 601.2b + CR 701.21a. `parse_oracle_cost`'s tap-cost branch consumed "another untapped " as a bare count of 1, so the phrase handed to `parse_target` no longer carried the qualifier and the cost filter came out without `FilterProp::Another`. The ability source was therefore an eligible payment for its own cost — Spire Mechcycle is a Vehicle paying "Tap another untapped Mount or Vehicle you control". The leading-quantifier `alt` now reports the same typed `CountWord` the sacrifice imperative already uses for this exact failure (#4513), and the exclusion is re-applied to every leg of the built filter. The numeric branch ("tap two other untapped artifacts you control") is unchanged: it never consumes "other", so the phrase reaches `parse_type_phrase` intact and that supplies the property itself. The runtime is not touched. `has_enough_tap_creatures` (`game/cost_payability.rs`) evaluates the cost filter against a `FilterContext::from_source`, so `FilterProp::Another` takes effect as soon as the parser emits it; the separate `exclude_source` flag (composite `{T}` costs, CR 601.2b) stays as it is. Class, measured over the 96 distinct printed tap-cost phrases in `client/public/card-data.json` (35,795 cards): 94 parse to `TapCreatures`, 12 say "another"/"other", and phrases carrying `FilterProp::Another` go 4 -> 11. The 7 newly fixed phrases cover 11 cards / 11 activated abilities: Black Oak of Odunos, Kumena Tyrant of Orazca, Network Terminal, Radiant Serra Archangel, Ranger's Hawk, Shadow Stinger, Spire Mechcycle, Sure-Footed Infiltrator, Tyvar the Pummeler, Veteran Warleader, Wanderbrine Trapper. Meanders Guide's triggered "you may tap another untapped Merfolk you control" runs through the same branch and is fixed too, measured separately through `parse_oracle_text` (properties `[]` before, `[Another]` after) — 12 cards in total. The issue reported 8; the measured class is 12. Counter-probe: with `CountWord::SourceExclusion => filter` the two new parser tests and `the_source_alone_cannot_pay_its_own_tap_another_cost` fail ("exclusion mismatch, got []"); the two counter-direction tests stay green, as does the Meanders Guide measurement's `[]`. Not covered: the "other than this creature" tail forms reach the exclusion through a different grammar and were already correct (Impelled Giant), and Mossbridge Troll's "tap any number of untapped creatures you control other than this creature with total power 10 or greater" does not lower to a `TapCreatures` cost at all — it becomes an `EffectCost`, which this change does not address. Co-Authored-By: Claude Opus 5 --- crates/engine/src/parser/oracle_cost.rs | 141 +++++++++++++++--- crates/engine/src/parser/oracle_util.rs | 7 +- crates/engine/tests/integration/main.rs | 1 + .../tap_cost_another_self_exclusion.rs | 117 +++++++++++++++ 4 files changed, 248 insertions(+), 18 deletions(-) create mode 100644 crates/engine/tests/integration/tap_cost_another_self_exclusion.rs diff --git a/crates/engine/src/parser/oracle_cost.rs b/crates/engine/src/parser/oracle_cost.rs index 9a23ee7b84..e9d4716182 100644 --- a/crates/engine/src/parser/oracle_cost.rs +++ b/crates/engine/src/parser/oracle_cost.rs @@ -19,6 +19,7 @@ use super::oracle_util::parse_count_expr; use super::oracle_util::parse_creature_subtype; use super::oracle_util::parse_mana_symbols; use super::oracle_util::parse_number; +use super::oracle_util::CountWord; use super::oracle_util::TextPair; use crate::types::ability::{ AbilityCost, AggregateFunction, BeholdCostAction, ChoiceType, Comparator, ControllerRef, @@ -1031,19 +1032,32 @@ pub fn parse_single_cost(text: &str) -> AbilityCost { value((), alt((tag("tap "), tag("tapped ")))).parse(i) }) { let tap_lower = tap_rest.to_lowercase(); - let (count, filter_text) = if let Some(((), r)) = nom_on_lower(tap_rest, &tap_lower, |i| { - value( - (), - alt((tag("another untapped "), tag("an untapped "), tag("an "))), - ) - .parse(i) - }) { - (1u32, r.to_lowercase()) - } else if let Some(((), r)) = nom_on_lower(tap_rest, &tap_lower, |i| { + // The leading quantifier reports a typed `CountWord` alongside the + // count. "Another"/"other" is not merely a quantity of one: it is the + // source-exclusion qualifier, and this branch CONSUMES it, so the + // remainder handed to `parse_target` no longer carries it. Without the + // signal the exclusion is lost and the source pays its own cost + // (Spire Mechcycle, #7522). Same failure and same typed remedy as the + // sacrifice imperative's `parse_count_expr_with_exclusion` (#4513). + let (count, filter_text, count_word) = if let Some((word, r)) = + nom_on_lower(tap_rest, &tap_lower, |i| { + alt(( + value(CountWord::SourceExclusion, tag("another untapped ")), + value(CountWord::Plain, tag("an untapped ")), + value(CountWord::Plain, tag("an ")), + )) + .parse(i) + }) { + (1u32, r.to_lowercase(), word) + } else if let Some((word, r)) = nom_on_lower(tap_rest, &tap_lower, |i| { // "X untapped [type]" — variable count, use u32::MAX as sentinel. - value((), alt((tag("x untapped "), tag("x other untapped ")))).parse(i) + alt(( + value(CountWord::Plain, tag("x untapped ")), + value(CountWord::SourceExclusion, tag("x other untapped ")), + )) + .parse(i) }) { - (u32::MAX, r.to_lowercase()) + (u32::MAX, r.to_lowercase(), word) } else if let Some((n, r)) = super::oracle_util::parse_number(&tap_lower) { let r = nom_on_lower( &tap_rest[tap_lower.len() - r.len()..], @@ -1052,15 +1066,24 @@ pub fn parse_single_cost(text: &str) -> AbilityCost { ) .map(|((), rest)| rest.to_lowercase()) .unwrap_or_else(|| r.trim_start().to_string()); - (n, r) + // The numeric branch does NOT consume "other" ("tap two other + // untapped artifacts you control"): the `untapped ` tag fails on + // the "other " lead, so the phrase reaches `parse_target` intact + // and `parse_type_phrase` supplies `FilterProp::Another` itself. + // Nothing to re-apply here. + (n, r, CountWord::Plain) } else { - (0, String::new()) + (0, String::new(), CountWord::Plain) }; if count > 0 { let target_text = format!("target {filter_text}"); let (filter, remainder) = parse_target(&target_text); if remainder.trim().is_empty() { + let filter = match count_word { + CountWord::SourceExclusion => apply_source_exclusion(filter), + CountWord::Plain => filter, + }; return AbilityCost::TapCreatures { requirement: TapCreaturesRequirement::count(count), filter, @@ -1535,6 +1558,26 @@ fn ensure_another_sacrifice_filter(filter: TargetFilter, phrase: &str) -> Target if !has_another_prefix { return filter; } + apply_source_exclusion(filter) +} + +/// CR 109.4 + CR 701.21a: Put the source exclusion (`FilterProp::Another`) on a +/// cost filter, distributing it over every leg of an `Or` disjunction. +/// +/// "Another" means "not this source permanent", so it belongs on each leg the +/// source's type could match; it is vacuous, never wrong, on a leg the source +/// cannot match. Marking only the first leg leaves a hole for a source that +/// matches a later one — Spire Mechcycle is a Vehicle paying "tap another +/// untapped Mount or Vehicle you control" (#7522), the tap-cost twin of the +/// sacrifice case in #4513. +/// +/// The catch-all arm is a mapping, not a classification: the exclusion is a +/// `TypedFilter` property, and the remaining `TargetFilter` variants +/// (`SelfRef`, `Any`, zone/player filters, …) carry no property list to put it +/// on. Callers detect the exclusion word themselves — `parse_oracle_cost`'s tap +/// branch via a typed `CountWord`, the sacrifice branch via +/// `ensure_another_sacrifice_filter`'s phrase prefix. +fn apply_source_exclusion(filter: TargetFilter) -> TargetFilter { match filter { TargetFilter::Typed(mut typed) => { if !typed.properties.contains(&FilterProp::Another) { @@ -1543,10 +1586,7 @@ fn ensure_another_sacrifice_filter(filter: TargetFilter, phrase: &str) -> Target TargetFilter::Typed(typed) } TargetFilter::Or { filters } => TargetFilter::Or { - filters: filters - .into_iter() - .map(|f| ensure_another_sacrifice_filter(f, phrase)) - .collect(), + filters: filters.into_iter().map(apply_source_exclusion).collect(), }, other => other, } @@ -2346,6 +2386,73 @@ mod tests { ); } + /// CR 109.4 + CR 701.21a: the source-exclusion "another" in a `TapCreatures` + /// activation cost must survive into the cost filter — a permanent may not + /// pay its own "tap another untapped …" cost (Spire Mechcycle, #7522). + /// + /// Table-driven over the printed shapes the tap-cost grammar distinguishes, + /// with both counter-directions: an ordinary article and a plain numeric + /// count must NOT gain the exclusion (CR 601.2b — a standalone tap cost + /// with no "another" does include the source). + /// + /// The "two other untapped" row was already green before this fix: the + /// numeric branch never consumes "other", so the phrase reaches + /// `parse_type_phrase` intact and it supplies the property. That row pins + /// the path; it is not evidence for the fix. + #[test] + fn tap_cost_another_carries_the_source_exclusion() { + let cases: &[(&str, bool)] = &[ + ("Tap another untapped Merfolk you control", true), + ( + "Tap another untapped creature you control with flying", + true, + ), + ("Tap two other untapped artifacts you control", true), + ("Tap an untapped Merfolk you control", false), + ("Tap three untapped Merfolk you control", false), + ]; + for (text, excluded) in cases { + let AbilityCost::TapCreatures { filter, .. } = parse_oracle_cost(text) else { + panic!("{text:?} must parse to a TapCreatures cost"); + }; + let TargetFilter::Typed(typed) = &filter else { + panic!("{text:?} must parse to a typed filter, got {filter:?}"); + }; + assert_eq!( + typed.properties.contains(&FilterProp::Another), + *excluded, + "{text:?} exclusion mismatch, got {:?}", + typed.properties + ); + } + } + + /// Spire Mechcycle (#7522): "Tap another untapped Mount or Vehicle you + /// control" — the exclusion must land on EVERY leg of the disjunction. The + /// Mechcycle is itself a Vehicle, so it matches the SECOND leg; marking + /// only the first would still let it pay its own cost. + #[test] + fn tap_cost_another_marks_every_leg_of_a_disjunction() { + let AbilityCost::TapCreatures { filter, .. } = + parse_oracle_cost("Tap another untapped Mount or Vehicle you control") + else { + panic!("expected a TapCreatures cost"); + }; + let TargetFilter::Or { filters } = &filter else { + panic!("expected an Or filter for 'Mount or Vehicle', got {filter:?}"); + }; + assert_eq!(filters.len(), 2, "expected two legs, got {filters:?}"); + for leg in filters { + let TargetFilter::Typed(typed) = leg else { + panic!("expected typed legs, got {leg:?}"); + }; + assert!( + typed.properties.contains(&FilterProp::Another), + "every leg must carry the exclusion, got {typed:?}" + ); + } + } + #[test] fn cost_unattach_this_equipment() { assert_eq!( diff --git a/crates/engine/src/parser/oracle_util.rs b/crates/engine/src/parser/oracle_util.rs index 1ec8ad617b..814e32db8e 100644 --- a/crates/engine/src/parser/oracle_util.rs +++ b/crates/engine/src/parser/oracle_util.rs @@ -708,7 +708,7 @@ pub(crate) fn rewrite_quantity_expr_rounding(expr: &mut QuantityExpr, mode: Roun } } -/// Typed signal distinguishing which count-word `parse_count_expr` consumed. +/// Typed signal distinguishing which count-word a quantifier grammar consumed. /// /// The numeric value of a count is the same whether the text said "a", "an", /// "1", "any", or "another" — all yield `QuantityExpr::Fixed { value: 1 }`. But @@ -718,6 +718,11 @@ pub(crate) fn rewrite_quantity_expr_rounding(expr: &mut QuantityExpr, mode: Roun /// distinguish the exclusion word from an ordinary article without re-matching /// the raw string at the call site (CLAUDE.md forbids stringly-typed dispatch). /// This enum is that typed signal. +/// +/// Every grammar that consumes the qualifier reports it, not just +/// [`parse_count_expr_with_exclusion`]: `parse_oracle_cost`'s tap-cost branch +/// reports it from its own leading-quantifier `alt` ("tap another untapped +/// Merfolk you control", #7522). #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum CountWord { /// The count word was the source-exclusion "another" — the consuming caller diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index a688071dd7..03a2090598 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -1021,6 +1021,7 @@ mod swans_prevention_followup; mod swarm_combat_witness; mod tales_of_the_ancestors_catch_up_draw; mod talon_gates_from_hand_activation; +mod tap_cost_another_self_exclusion; mod targeted_exchange_preview_budget; mod tchaka_venerable_king; mod teamwork_aggregate_legal_actions; diff --git a/crates/engine/tests/integration/tap_cost_another_self_exclusion.rs b/crates/engine/tests/integration/tap_cost_another_self_exclusion.rs new file mode 100644 index 0000000000..f625e805e5 --- /dev/null +++ b/crates/engine/tests/integration/tap_cost_another_self_exclusion.rs @@ -0,0 +1,117 @@ +//! CR 109.4 + CR 601.2b + CR 701.21a: a `"Tap another untapped … you control"` +//! activation cost excludes the ability's own source (#7522). +//! +//! Spire Mechcycle's exhaust cost reads "Tap another untapped Mount or Vehicle +//! you control", and the Mechcycle is itself a Vehicle — before the fix the +//! parsed cost filter carried no `FilterProp::Another`, so the source was an +//! eligible payment for its own ability. +//! +//! The runtime was never the defect. `has_enough_tap_creatures` +//! (`game/cost_payability.rs`) evaluates the cost filter against a +//! `FilterContext::from_source`, so `FilterProp::Another` is honoured the +//! moment the parser emits it; its separate `exclude_source` flag belongs to +//! composite `{T}` costs and is untouched here. These tests therefore drive the +//! real payability gate (`ai_support::legal_actions`), not the parser. +//! +//! Card text is built from Oracle text rather than named cards, so the tests +//! run in CI (which has no card database). +//! +//! Not covered: "other than this creature" tail forms (Impelled Giant, +//! Mossbridge Troll) reach the exclusion through a different grammar and were +//! already correct; the subtype disjunction (Spire Mechcycle's "Mount or +//! Vehicle") is pinned at the parser layer in +//! `parser::oracle_cost::tests::tap_cost_another_marks_every_leg_of_a_disjunction`, +//! because `GameScenario` has no helper that stamps a Vehicle subtype. + +use engine::game::scenario::{GameRunner, GameScenario, P0}; +use engine::types::actions::GameAction; +use engine::types::identifiers::ObjectId; +use engine::types::phase::Phase; + +/// The reported shape: the cost excludes the source. +const ANOTHER: &str = + "Tap another untapped creature you control: This creature gains indestructible until end of turn."; + +/// CR 601.2b counter-direction: no "another", so the source IS eligible. +const PLAIN: &str = + "Tap an untapped creature you control: This creature gains indestructible until end of turn."; + +/// One creature carrying `oracle` plus `helpers` vanilla untapped creatures, +/// all controlled by P0, with P0 holding priority in its precombat main phase. +fn board(oracle: &str, helpers: usize) -> (GameRunner, ObjectId) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let source = scenario + .add_creature_from_oracle(P0, "Tapper", 2, 2, oracle) + .id(); + for i in 0..helpers { + scenario.add_creature(P0, &format!("Helper {i}"), 1, 1); + } + (scenario.build(), source) +} + +/// Does the engine offer `source`'s first activated ability right now? +fn offers_activation(runner: &GameRunner, source: ObjectId) -> bool { + engine::ai_support::legal_actions(runner.state()) + .iter() + .any(|action| { + matches!( + action, + GameAction::ActivateAbility { + source_id, + ability_index: 0, + } if *source_id == source + ) + }) +} + +/// The defect: alone on the battlefield, the source matched its own cost filter +/// and the ability was offered — it would have paid by tapping itself. +/// +/// The negative assertion is not vacuous: `a_plain_tap_cost_still_includes_the_source` +/// runs the IDENTICAL board with the article form and finds the ability offered, +/// so an unreachable-ability setup would fail there. +/// +/// Reverting the fix flips this test to red: `assert!(!offers_activation(…))` +/// fails, and the `Err` expectation below becomes `Ok`. +#[test] +fn the_source_alone_cannot_pay_its_own_tap_another_cost() { + let (mut runner, source) = board(ANOTHER, 0); + assert!( + !offers_activation(&runner, source), + "a lone source must not be offered its own \"tap another untapped creature\" ability" + ); + assert!( + runner + .act(GameAction::ActivateAbility { + source_id: source, + ability_index: 0, + }) + .is_err(), + "activating anyway must be rejected — the source may not tap itself for the cost" + ); +} + +/// Positive counter-direction: with a second untapped creature the cost is +/// payable and the ability is offered. Guards against over-suppression, the +/// expensive collateral of a self-exclusion fix. +#[test] +fn a_second_untapped_creature_pays_the_tap_another_cost() { + let (runner, source) = board(ANOTHER, 1); + assert!( + offers_activation(&runner, source), + "another untapped creature makes the cost payable" + ); +} + +/// CR 601.2b: a standalone `TapCreatures` cost with no "another" DOES include +/// the source. This is the behaviour the fix must not break — and it is the +/// reach guard for the negative assertion in the first test. +#[test] +fn a_plain_tap_cost_still_includes_the_source() { + let (runner, source) = board(PLAIN, 0); + assert!( + offers_activation(&runner, source), + "\"tap an untapped creature you control\" is payable by the source itself (CR 601.2b)" + ); +} From 279c2beb05f6333b5c329c39a3c264b162d64ccf Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 18 Aug 2026 06:10:49 -0700 Subject: [PATCH 2/3] fix(PR-7530): resolve source-exclusion review findings --- crates/engine/src/parser/oracle_cost.rs | 74 +++++++++---------- crates/engine/src/parser/oracle_target.rs | 5 +- .../tap_cost_another_self_exclusion.rs | 30 +++++--- 3 files changed, 61 insertions(+), 48 deletions(-) diff --git a/crates/engine/src/parser/oracle_cost.rs b/crates/engine/src/parser/oracle_cost.rs index e9d4716182..b7937666a4 100644 --- a/crates/engine/src/parser/oracle_cost.rs +++ b/crates/engine/src/parser/oracle_cost.rs @@ -14,7 +14,7 @@ use super::oracle_nom::primitives::{scan_contains, split_once_on}; use super::oracle_nom::quantity as nom_quantity; use super::oracle_nom::target::parse_cost_self_reference; use super::oracle_static::parse_dynamic_x_clause; -use super::oracle_target::{parse_target, parse_type_phrase}; +use super::oracle_target::{distribute_shared_properties, parse_target, parse_type_phrase}; use super::oracle_util::parse_count_expr; use super::oracle_util::parse_creature_subtype; use super::oracle_util::parse_mana_symbols; @@ -1081,7 +1081,9 @@ pub fn parse_single_cost(text: &str) -> AbilityCost { let (filter, remainder) = parse_target(&target_text); if remainder.trim().is_empty() { let filter = match count_word { - CountWord::SourceExclusion => apply_source_exclusion(filter), + CountWord::SourceExclusion => { + distribute_shared_properties(filter, &[FilterProp::Another]) + } CountWord::Plain => filter, }; return AbilityCost::TapCreatures { @@ -1558,38 +1560,7 @@ fn ensure_another_sacrifice_filter(filter: TargetFilter, phrase: &str) -> Target if !has_another_prefix { return filter; } - apply_source_exclusion(filter) -} - -/// CR 109.4 + CR 701.21a: Put the source exclusion (`FilterProp::Another`) on a -/// cost filter, distributing it over every leg of an `Or` disjunction. -/// -/// "Another" means "not this source permanent", so it belongs on each leg the -/// source's type could match; it is vacuous, never wrong, on a leg the source -/// cannot match. Marking only the first leg leaves a hole for a source that -/// matches a later one — Spire Mechcycle is a Vehicle paying "tap another -/// untapped Mount or Vehicle you control" (#7522), the tap-cost twin of the -/// sacrifice case in #4513. -/// -/// The catch-all arm is a mapping, not a classification: the exclusion is a -/// `TypedFilter` property, and the remaining `TargetFilter` variants -/// (`SelfRef`, `Any`, zone/player filters, …) carry no property list to put it -/// on. Callers detect the exclusion word themselves — `parse_oracle_cost`'s tap -/// branch via a typed `CountWord`, the sacrifice branch via -/// `ensure_another_sacrifice_filter`'s phrase prefix. -fn apply_source_exclusion(filter: TargetFilter) -> TargetFilter { - match filter { - TargetFilter::Typed(mut typed) => { - if !typed.properties.contains(&FilterProp::Another) { - typed.properties.push(FilterProp::Another); - } - TargetFilter::Typed(typed) - } - TargetFilter::Or { filters } => TargetFilter::Or { - filters: filters.into_iter().map(apply_source_exclusion).collect(), - }, - other => other, - } + distribute_shared_properties(filter, &[FilterProp::Another]) } /// CR 117.1 + CR 601.2b + CR 107.4a/107.4e/202.1: Parse Baron Helmut Zemo's @@ -2386,9 +2357,10 @@ mod tests { ); } - /// CR 109.4 + CR 701.21a: the source-exclusion "another" in a `TapCreatures` - /// activation cost must survive into the cost filter — a permanent may not - /// pay its own "tap another untapped …" cost (Spire Mechcycle, #7522). + /// CR 602.2b + CR 118.3: the source-exclusion "another" in a + /// `TapCreatures` activation cost must survive into the cost filter, so the + /// ability's source cannot pay an activation cost that requires another + /// untapped creature (Spire Mechcycle, #7522). /// /// Table-driven over the printed shapes the tap-cost grammar distinguishes, /// with both counter-directions: an ordinary article and a plain numeric @@ -2453,6 +2425,34 @@ mod tests { } } + /// CR 602.2b + CR 118.3: shared source exclusion must flow through the + /// `And` shape for "creature you control but don't own" without changing + /// the negated ownership leg. + #[test] + fn tap_cost_another_preserves_exclusion_in_conjunctive_filter() { + let AbilityCost::TapCreatures { filter, .. } = + parse_oracle_cost("Tap another untapped creature you control but don't own") + else { + panic!("expected a TapCreatures cost"); + }; + let TargetFilter::And { filters } = filter else { + panic!("expected an And filter, got {filter:?}"); + }; + assert!(matches!( + filters.first(), + Some(TargetFilter::Typed(TypedFilter { properties, .. })) + if properties.contains(&FilterProp::Another) + )); + assert!(matches!( + filters.get(1), + Some(TargetFilter::Not { filter }) if matches!( + filter.as_ref(), + TargetFilter::Typed(TypedFilter { properties, .. }) + if !properties.contains(&FilterProp::Another) + ) + )); + } + #[test] fn cost_unattach_this_equipment() { assert_eq!( diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index a8af054692..58839ab654 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -4274,7 +4274,10 @@ fn stack_spell_filter(mut typed: TypedFilter) -> TargetFilter { } } -fn distribute_shared_properties(filter: TargetFilter, shared_props: &[FilterProp]) -> TargetFilter { +pub(super) fn distribute_shared_properties( + filter: TargetFilter, + shared_props: &[FilterProp], +) -> TargetFilter { match filter { TargetFilter::Typed(mut typed) => { for prop in shared_props { diff --git a/crates/engine/tests/integration/tap_cost_another_self_exclusion.rs b/crates/engine/tests/integration/tap_cost_another_self_exclusion.rs index f625e805e5..e1ea199fb6 100644 --- a/crates/engine/tests/integration/tap_cost_another_self_exclusion.rs +++ b/crates/engine/tests/integration/tap_cost_another_self_exclusion.rs @@ -1,5 +1,5 @@ -//! CR 109.4 + CR 601.2b + CR 701.21a: a `"Tap another untapped … you control"` -//! activation cost excludes the ability's own source (#7522). +//! CR 602.2b + CR 118.3: a `"Tap another untapped … you control"` activation +//! cost excludes the ability's own source (#7522). //! //! Spire Mechcycle's exhaust cost reads "Tap another untapped Mount or Vehicle //! you control", and the Mechcycle is itself a Vehicle — before the fix the @@ -38,16 +38,16 @@ const PLAIN: &str = /// One creature carrying `oracle` plus `helpers` vanilla untapped creatures, /// all controlled by P0, with P0 holding priority in its precombat main phase. -fn board(oracle: &str, helpers: usize) -> (GameRunner, ObjectId) { +fn board(oracle: &str, helper_count: usize) -> (GameRunner, ObjectId, Vec) { let mut scenario = GameScenario::new(); scenario.at_phase(Phase::PreCombatMain); let source = scenario .add_creature_from_oracle(P0, "Tapper", 2, 2, oracle) .id(); - for i in 0..helpers { - scenario.add_creature(P0, &format!("Helper {i}"), 1, 1); - } - (scenario.build(), source) + let helpers = (0..helper_count) + .map(|i| scenario.add_creature(P0, &format!("Helper {i}"), 1, 1).id()) + .collect(); + (scenario.build(), source, helpers) } /// Does the engine offer `source`'s first activated ability right now? @@ -76,7 +76,7 @@ fn offers_activation(runner: &GameRunner, source: ObjectId) -> bool { /// fails, and the `Err` expectation below becomes `Ok`. #[test] fn the_source_alone_cannot_pay_its_own_tap_another_cost() { - let (mut runner, source) = board(ANOTHER, 0); + let (mut runner, source, _) = board(ANOTHER, 0); assert!( !offers_activation(&runner, source), "a lone source must not be offered its own \"tap another untapped creature\" ability" @@ -97,11 +97,21 @@ fn the_source_alone_cannot_pay_its_own_tap_another_cost() { /// expensive collateral of a self-exclusion fix. #[test] fn a_second_untapped_creature_pays_the_tap_another_cost() { - let (runner, source) = board(ANOTHER, 1); + let (mut runner, source, helpers) = board(ANOTHER, 1); assert!( offers_activation(&runner, source), "another untapped creature makes the cost payable" ); + let helper = helpers[0]; + runner.activate(source, 0).pay_with(&[helper]).resolve(); + assert!( + runner.state().objects[&helper].tapped, + "the selected helper must be tapped by the real activation cost payment" + ); + assert!( + !runner.state().objects[&source].tapped, + "the source must remain untapped; only another creature pays this cost" + ); } /// CR 601.2b: a standalone `TapCreatures` cost with no "another" DOES include @@ -109,7 +119,7 @@ fn a_second_untapped_creature_pays_the_tap_another_cost() { /// reach guard for the negative assertion in the first test. #[test] fn a_plain_tap_cost_still_includes_the_source() { - let (runner, source) = board(PLAIN, 0); + let (runner, source, _) = board(PLAIN, 0); assert!( offers_activation(&runner, source), "\"tap an untapped creature you control\" is payable by the source itself (CR 601.2b)" From 2cc86406c7521efabdce42459760cdff5ca45d04 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 18 Aug 2026 07:08:45 -0700 Subject: [PATCH 3/3] fix(PR-7530): harden tap-cost regression evidence --- crates/engine/src/parser/oracle_cost.rs | 6 ++-- .../tap_cost_another_self_exclusion.rs | 32 +++++++++++++------ 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/crates/engine/src/parser/oracle_cost.rs b/crates/engine/src/parser/oracle_cost.rs index b7937666a4..0ac10f3b6c 100644 --- a/crates/engine/src/parser/oracle_cost.rs +++ b/crates/engine/src/parser/oracle_cost.rs @@ -2357,15 +2357,15 @@ mod tests { ); } - /// CR 602.2b + CR 118.3: the source-exclusion "another" in a + /// CR 602.2b + CR 601.2h + CR 118.3: the source-exclusion "another" in a /// `TapCreatures` activation cost must survive into the cost filter, so the /// ability's source cannot pay an activation cost that requires another /// untapped creature (Spire Mechcycle, #7522). /// /// Table-driven over the printed shapes the tap-cost grammar distinguishes, /// with both counter-directions: an ordinary article and a plain numeric - /// count must NOT gain the exclusion (CR 601.2b — a standalone tap cost - /// with no "another" does include the source). + /// count must NOT gain the exclusion (a standalone tap cost with no + /// "another" does include the source). /// /// The "two other untapped" row was already green before this fix: the /// numeric branch never consumes "other", so the phrase reaches diff --git a/crates/engine/tests/integration/tap_cost_another_self_exclusion.rs b/crates/engine/tests/integration/tap_cost_another_self_exclusion.rs index e1ea199fb6..a2c9c0f354 100644 --- a/crates/engine/tests/integration/tap_cost_another_self_exclusion.rs +++ b/crates/engine/tests/integration/tap_cost_another_self_exclusion.rs @@ -1,4 +1,4 @@ -//! CR 602.2b + CR 118.3: a `"Tap another untapped … you control"` activation +//! CR 602.2b + CR 601.2h + CR 118.3: a `"Tap another untapped … you control"` activation //! cost excludes the ability's own source (#7522). //! //! Spire Mechcycle's exhaust cost reads "Tap another untapped Mount or Vehicle @@ -24,6 +24,7 @@ //! because `GameScenario` has no helper that stamps a Vehicle subtype. use engine::game::scenario::{GameRunner, GameScenario, P0}; +use engine::types::ability::{AbilityCost, Effect}; use engine::types::actions::GameAction; use engine::types::identifiers::ObjectId; use engine::types::phase::Phase; @@ -32,7 +33,7 @@ use engine::types::phase::Phase; const ANOTHER: &str = "Tap another untapped creature you control: This creature gains indestructible until end of turn."; -/// CR 601.2b counter-direction: no "another", so the source IS eligible. +/// Counter-direction: no "another", so the source remains eligible. const PLAIN: &str = "Tap an untapped creature you control: This creature gains indestructible until end of turn."; @@ -68,15 +69,29 @@ fn offers_activation(runner: &GameRunner, source: ObjectId) -> bool { /// The defect: alone on the battlefield, the source matched its own cost filter /// and the ability was offered — it would have paid by tapping itself. /// -/// The negative assertion is not vacuous: `a_plain_tap_cost_still_includes_the_source` -/// runs the IDENTICAL board with the article form and finds the ability offered, -/// so an unreachable-ability setup would fail there. +/// The reach guards below prove that ability 0 was published as a concrete +/// `TapCreatures` cost. Without them, a parser failure could make both negative +/// assertions pass without exercising the source-exclusion behavior. /// /// Reverting the fix flips this test to red: `assert!(!offers_activation(…))` /// fails, and the `Err` expectation below becomes `Ok`. #[test] fn the_source_alone_cannot_pay_its_own_tap_another_cost() { let (mut runner, source, _) = board(ANOTHER, 0); + let ability = runner.state().objects[&source] + .abilities + .first() + .expect("the source must publish ability 0"); + assert!( + matches!(&ability.cost, Some(AbilityCost::TapCreatures { .. })), + "ability 0 must publish a TapCreatures cost, got {:?}", + ability.cost + ); + assert!( + !matches!(ability.effect.as_ref(), Effect::Unimplemented { .. }), + "ability 0 must not lower to Effect::Unimplemented, got {:?}", + ability.effect + ); assert!( !offers_activation(&runner, source), "a lone source must not be offered its own \"tap another untapped creature\" ability" @@ -114,14 +129,13 @@ fn a_second_untapped_creature_pays_the_tap_another_cost() { ); } -/// CR 601.2b: a standalone `TapCreatures` cost with no "another" DOES include -/// the source. This is the behaviour the fix must not break — and it is the -/// reach guard for the negative assertion in the first test. +/// A standalone `TapCreatures` cost with no "another" includes the source. +/// This is the behaviour the fix must not break. #[test] fn a_plain_tap_cost_still_includes_the_source() { let (runner, source, _) = board(PLAIN, 0); assert!( offers_activation(&runner, source), - "\"tap an untapped creature you control\" is payable by the source itself (CR 601.2b)" + "\"tap an untapped creature you control\" is payable by the source itself" ); }