From 0e42577edf15853c80a3363ff554936c0f828ac6 Mon Sep 17 00:00:00 2001 From: Lindsey Gray Date: Fri, 17 Jul 2026 10:13:23 -0500 Subject: [PATCH 1/4] =?UTF-8?q?feat(parser):=20lift=20"[once]=20for=20each?= =?UTF-8?q?=20=E2=9F=A8player-set=E2=9F=A9"=20onto=20fieldless=20Investiga?= =?UTF-8?q?te?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lift a trailing "[once] for each " clause on a fieldless Effect::Investigate into repeat_for = PlayerCount{filter}: - Teysa, Opulent Oligarch: "investigate for each opponent who lost life this turn" → PlayerCount{OpponentLostLife} - Wojek Investigator: "investigate once for each opponent who has more cards in hand than you" → PlayerCount{PlayerAttribute{Opponent, HandSize{ScopedPlayer}, GT, HandSize{Controller}}} EDIT 1 extracts a shared split_for_each_suffix primitive (byte-identical strip refactor) plus a PlayerCount-gated for_each_player_set_repeat_for lift, consumed at the chunk-loop seam. EDIT 2 adds the comparative hand-size who-clause combinator and widens the player-attribute operand to QuantityExpr. Parser-only: runtime resolution rides the existing filter-agnostic repeat_for driver (CR 608.2c). Object-ranged "investigate for each" (Serene Sleuth, Sophina) stays on the unchanged path. Assisted-by: ClaudeCode:claude-opus-4.8 --- .../engine/src/parser/oracle_effect/lower.rs | 198 +++++++- crates/engine/src/parser/oracle_effect/mod.rs | 19 + crates/engine/src/parser/oracle_quantity.rs | 139 +++++- crates/engine/tests/integration/main.rs | 1 + .../teysa_wojek_investigate_per_opponent.rs | 450 ++++++++++++++++++ 5 files changed, 780 insertions(+), 27 deletions(-) create mode 100644 crates/engine/tests/integration/teysa_wojek_investigate_per_opponent.rs diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index f180dd53b7..e01c406471 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -3530,32 +3530,64 @@ pub(super) fn zada_repeat_for_implies_distinct_copy_targets(qty: &QuantityExpr) filter_has_could_be_targeted_by_triggering_spell(filter) } +/// Split a clause at the first " for each " boundary. Returns the base byte-length +/// (an offset into the ORIGINAL text — lowercasing is byte-length-preserving for the +/// ASCII Oracle corpus) and the lowercase tail after " for each ". The single split +/// authority shared by `strip_for_each_repeat_suffix` and `for_each_player_set_repeat_for`. +fn split_for_each_suffix(text: &str) -> Option<(usize, String)> { + let lower = text.to_lowercase(); + let (rest, base) = take_until::<_, _, OracleError<'_>>(" for each ") + .parse(lower.as_str()) + .ok()?; + let (tail, _) = tag::<_, _, OracleError<'_>>(" for each ") + .parse(rest) + .ok()?; + Some((base.len(), tail.to_string())) +} + pub(super) fn strip_for_each_repeat_suffix(text: &str) -> (Option, String) { let (_, text) = strip_each_copy_targets_distinct_member_suffix(text); - let lower = text.to_lowercase(); - let parsed = nom_on_lower(&text, &lower, |input| { - let (rest, base) = take_until::<_, _, OracleError<'_>>(" for each ").parse(input)?; - let (rest, _) = tag(" for each ").parse(rest)?; - let (rest, qty) = nom_quantity::parse_for_each_clause_ref(rest)?; - let (rest, _) = nom::combinator::opt(tag(".")).parse(rest)?; - let (rest, _) = nom::combinator::eof::<_, OracleError<'_>>(rest)?; - Ok((rest, (base.len(), qty))) - }); - if let Some(((base_len, qty), _)) = parsed { - if matches!(&qty, QuantityRef::CommanderCastFromCommandZoneCount) - || zada_repeat_for_implies_distinct_copy_targets(&QuantityExpr::Ref { - qty: qty.clone(), - }) + if let Some((base_len, tail)) = split_for_each_suffix(&text) { + if let Ok((_, qty)) = all_consuming(terminated( + nom_quantity::parse_for_each_clause_ref, + opt(tag::<_, _, OracleError<'_>>(".")), + )) + .parse(tail.as_str()) { - return ( - Some(QuantityExpr::Ref { qty }), - text[..base_len].trim_end().to_string(), - ); + // Unchanged gate: the repeat-suffix lift is restricted to CommanderCast + // and Zada distinct-copy today. A player-set `PlayerCount` is deliberately + // NOT admitted here — that class routes through the fieldless-Investigate + // seam via `for_each_player_set_repeat_for`. + if matches!(&qty, QuantityRef::CommanderCastFromCommandZoneCount) + || zada_repeat_for_implies_distinct_copy_targets(&QuantityExpr::Ref { + qty: qty.clone(), + }) + { + return ( + Some(QuantityExpr::Ref { qty }), + text[..base_len].trim_end().to_string(), + ); + } } } (None, text) } +/// CR 701.16a + CR 608.2c: Lift a trailing "[once] for each ⟨player-set⟩" multiplier +/// off a fieldless keyword-action effect (Investigate has no count slot) into a +/// `repeat_for`. Uses the `parse_for_each_clause` WRAPPER — NOT +/// `parse_for_each_clause_ref` — because the `PlayerAttribute` producer (Wojek's +/// "opponent who has more cards in hand than you") is reachable only via the +/// wrapper's `oracle_quantity` fallback. Gated on `PlayerCount` so object for-each +/// is left to the count-bearing effect path. +pub(super) fn for_each_player_set_repeat_for(text: &str) -> Option { + let (_, tail) = split_for_each_suffix(text)?; + match parse_for_each_clause(&tail) { + Some(qty @ QuantityRef::PlayerCount { .. }) => Some(QuantityExpr::Ref { qty }), + _ => None, + } +} + /// CR 107.1: Strip "twice" / "three times" / "N times" suffix to produce a /// `repeat_for` count — an integer repeat multiplier (count templating), not the /// CR 609.3 "do as much as possible" rule. Unified with `strip_for_each_prefix` @@ -11553,3 +11585,133 @@ mod strip_optional_effect_prefix_tests { assert_eq!(rest, "cast the exiled card without paying its mana cost"); } } + +/// DynQty subgroup D — "[once] for each ⟨player-set⟩" lift for fieldless Investigate. +/// Building-block tests for the shared split refactor (byte-identity), the player-set +/// lift helper, and the wrapper-vs-`_ref` non-domination guard. +#[cfg(test)] +mod dq_d_player_set_lift_tests { + use super::{for_each_player_set_repeat_for, strip_for_each_repeat_suffix}; + use crate::parser::oracle_nom::quantity::parse_for_each_clause_ref; + use crate::types::ability::{PlayerFilter, QuantityExpr, QuantityRef}; + + // Matrix #3 — the shared `split_for_each_suffix` refactor is byte-identical: + // each input yields the SAME `(Option, String)` as pre-refactor. + // Reverting to a byte-changing split (or admitting `PlayerCount` into the gate) + // flips one of these assertions. + #[test] + fn strip_for_each_repeat_suffix_byte_identity_corpus() { + // (a) CommanderCast "for each" lift is preserved. + let (qty, base) = strip_for_each_repeat_suffix( + "copy it for each time you've cast your commander from the command zone this game", + ); + assert!( + matches!( + qty, + Some(QuantityExpr::Ref { + qty: QuantityRef::CommanderCastFromCommandZoneCount + }) + ), + "CommanderCast lift must survive the refactor: {qty:?}" + ); + assert_eq!(base, "copy it"); + + // (b) a player-set for-each is REJECTED by this gate (routes through the + // fieldless-Investigate seam instead) → `(None, )`. + let input = "investigate for each opponent who lost life this turn"; + let (qty, base) = strip_for_each_repeat_suffix(input); + assert!( + qty.is_none(), + "PlayerCount must not be lifted here: {qty:?}" + ); + assert_eq!(base, input); + + // (c) the Zada distinct-copy ObjectCount lift is preserved: strip lifts + // "other creature you control that the spell could target" to an + // `ObjectCount{CouldBeTargetedByTriggeringSpell}` and returns the base "copy + // that spell". Byte-identical to the pre-refactor `_ref + eof` path, and proves + // the new player-set routing did not disturb the CopySpell/Zada gate. + let (qty, base) = strip_for_each_repeat_suffix( + "copy that spell for each other creature you control that the spell could target", + ); + assert!( + matches!( + qty, + Some(QuantityExpr::Ref { + qty: QuantityRef::ObjectCount { .. } + }) + ), + "Zada ObjectCount lift must survive the refactor: {qty:?}" + ); + assert_eq!(base, "copy that spell"); + + // (d) no "for each" suffix at all → unchanged passthrough. + let (qty, base) = strip_for_each_repeat_suffix("draw a card"); + assert!(qty.is_none()); + assert_eq!(base, "draw a card"); + } + + // Matrix #4 — the player-set lift helper. + #[test] + fn for_each_player_set_repeat_for_lifts_player_count_only() { + // Teysa: OpponentLostLife → PlayerCount. + let teysa = + for_each_player_set_repeat_for("investigate for each opponent who lost life this turn"); + assert!( + matches!( + teysa, + Some(QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { + filter: PlayerFilter::OpponentLostLife + } + }) + ), + "Teysa must lift OpponentLostLife: {teysa:?}" + ); + + // Wojek: PlayerAttribute (comparative hand size). REVERT PROBE: switching the + // helper body from the `parse_for_each_clause` wrapper to `parse_for_each_clause_ref` + // makes THIS case return `None` (the `_ref` alt has no PlayerAttribute arm) — + // that is the wrapper-vs-`_ref` guard. + let wojek = for_each_player_set_repeat_for( + "investigate once for each opponent who has more cards in hand than you", + ); + assert!( + matches!( + wojek, + Some(QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { + filter: PlayerFilter::PlayerAttribute { .. } + } + }) + ), + "Wojek must lift PlayerAttribute via the wrapper: {wojek:?}" + ); + + // Object for-each is left to the count-bearing path (ObjectCount ≠ PlayerCount). + assert!( + for_each_player_set_repeat_for("investigate for each artifact you control").is_none(), + "object for-each must NOT lift here" + ); + + // No "for each" suffix → None. + assert!(for_each_player_set_repeat_for("investigate").is_none()); + } + + // Matrix #2 — non-domination: the bare `_ref` combinator does NOT consume Wojek's + // comparative tail. This is why the helper MUST use the wrapper (which reaches the + // `oracle_quantity` PlayerAttribute producer). If `_ref` DID consume this to empty, + // matrix #4/#6's discriminator would be vacuous. + #[test] + fn parse_for_each_clause_ref_does_not_dominate_comparative_hand_size() { + let tail = "opponent who has more cards in hand than you"; + match parse_for_each_clause_ref(tail) { + Err(_) => {} // rejected outright — non-dominating + Ok((rest, _)) => assert!( + !rest.is_empty(), + "_ref must NOT consume the comparative tail to empty (would make the \ + wrapper's `rest.is_empty()` gate fire): rest={rest:?}" + ), + } + } +} diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 3e20a36d65..e87a51543d 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -27922,6 +27922,19 @@ pub(crate) fn parse_effect_chain_ir( let (suffix_repeat_for, stripped_text_no_qty) = strip_for_each_repeat_suffix(&text_no_qty); let mut stripped_clause = parse_effect_clause(&stripped_text_no_qty, ctx); + // CR 701.16a + CR 608.2c: a fieldless Investigate (no count slot) drops a + // trailing "[once] for each " multiplier. Precompute the lift, + // gated to Investigate + no prior repeat_for, so ONLY the player-set-for-each + // Investigate class enters the branch below; every other Investigate chunk + // (plain "Investigate.", object for-each) falls through to the unchanged final + // else. `for_each_player_set_repeat_for` returns None without a real + // "for each " suffix. + let investigate_player_set_lift = + if repeat_for.is_none() && matches!(stripped_clause.effect, Effect::Investigate) { + lower::for_each_player_set_repeat_for(&text_no_qty) + } else { + None + }; if suffix_repeat_for.is_some() && matches!(stripped_clause.effect, Effect::CopySpell { .. }) { @@ -27952,6 +27965,12 @@ pub(crate) fn parse_effect_chain_ir( *ctx = fanout_ctx; multi_target = Some(fanout_spec); (fanout_clause, None) + } else if let Some(player_qty) = investigate_player_set_lift { + // CR 608.2c: re-parse `text_no_qty` EXACTLY as the final else does + // (clause + ctx byte-identical), attaching ONLY the lifted repeat_for. + // The gate guaranteed `repeat_for.is_none()`, so `Some(player_qty)` + // cannot clobber a prior count. + (parse_effect_clause(&text_no_qty, ctx), Some(player_qty)) } else { (parse_effect_clause(&text_no_qty, ctx), repeat_for) } diff --git a/crates/engine/src/parser/oracle_quantity.rs b/crates/engine/src/parser/oracle_quantity.rs index 4e93f77454..cf9246c1c9 100644 --- a/crates/engine/src/parser/oracle_quantity.rs +++ b/crates/engine/src/parser/oracle_quantity.rs @@ -1462,19 +1462,22 @@ fn parse_opponents_attacked_clause(input: &str) -> nom::IResult<&str, (), Oracle /// (GE); the count-style predicates (cards drawn, battlefield entries) are always /// "N or more" (GE), so a `map` adapter tags them with `Comparator::GE`. fn parse_for_each_opponent_player_attribute_clause(clause: &str) -> Option { - let ((relation, attr, comparator, count), rest) = nom_on_lower(clause, clause, |input| { + let ((relation, attr, comparator, value_expr), rest) = nom_on_lower(clause, clause, |input| { let (input, relation) = parse_player_population(input)?; - let (input, (attr, comparator, count)) = alt(( - parse_hand_size_who_attr_clause, - map(parse_cards_drawn_attr_clause, |(attr, n)| { - (attr, Comparator::GE, n) + let (input, (attr, comparator, value_expr)) = alt(( + map(parse_hand_size_who_attr_clause, |(a, c, n)| { + (a, c, QuantityExpr::Fixed { value: n }) }), - map(parse_battlefield_entries_attr_clause, |(attr, n)| { - (attr, Comparator::GE, n) + parse_comparative_hand_size_who_clause, + map(parse_cards_drawn_attr_clause, |(a, n)| { + (a, Comparator::GE, QuantityExpr::Fixed { value: n }) + }), + map(parse_battlefield_entries_attr_clause, |(a, n)| { + (a, Comparator::GE, QuantityExpr::Fixed { value: n }) }), )) .parse(input)?; - Ok((input, (relation, attr, comparator, count))) + Ok((input, (relation, attr, comparator, value_expr))) })?; if !rest.is_empty() || relation != PlayerRelation::Opponent { return None; @@ -1484,7 +1487,7 @@ fn parse_for_each_opponent_player_attribute_clause(clause: &str) -> Option OracleResult<'_, (QuantityRef, Comparator, QuantityExpr)> { + let (input, _) = alt((tag("who has "), tag("who have "))).parse(input)?; + let (input, (comparator, connector)) = alt(( + value((Comparator::GT, "than "), tag("more ")), + value((Comparator::LT, "than "), tag("fewer ")), + value((Comparator::EQ, "as "), tag("as many ")), + )) + .parse(input)?; + let (input, _) = tag("cards in hand ").parse(input)?; + let (input, _) = tag(connector).parse(input)?; + // Reference-operand axis; today only "you" → controller. `value`-ready for future refs. + let (input, ref_scope) = value(PlayerScope::Controller, tag("you")).parse(input)?; + Ok(( + input, + ( + QuantityRef::HandSize { + player: PlayerScope::ScopedPlayer, + }, // per-candidate (inert scope) + comparator, + QuantityExpr::Ref { + qty: QuantityRef::HandSize { player: ref_scope }, + }, // CR 109.5 operand + ), + )) +} + /// CR 402.1 / 119.1 / 122.1f / 404.1: Parse a player population whose scalar /// attribute crosses a threshold, into `PlayerFilter::PlayerAttribute`. Reached /// after `"the number of "` has been stripped. @@ -3384,6 +3422,89 @@ mod tests { }; use crate::types::mana::ManaColor; + /// DynQty subgroup D / Matrix #1 — the comparative hand-size producer builds the + /// exact `PlayerAttribute` AST (Wojek Investigator). Fails iff EDIT 2 is reverted; + /// independent of EDIT 1. The full `assert_eq` pins operand scope (Controller, + /// CR 109.5) ≠ per-candidate attr scope (ScopedPlayer) — swapping them flips it. + /// Sibling cells (fewer→LT, as many→EQ) and the fixed-arm reach-guard prove the + /// alt is axis-composed and the numeric backtrack is intact. + #[test] + fn comparative_hand_size_producer_builds_player_attribute() { + let gt = parse_for_each_clause("opponent who has more cards in hand than you"); + assert_eq!( + gt, + Some(QuantityRef::PlayerCount { + filter: PlayerFilter::PlayerAttribute { + relation: PlayerRelation::Opponent, + attr: Box::new(QuantityRef::HandSize { + player: PlayerScope::ScopedPlayer + }), + comparator: Comparator::GT, + value: Box::new(QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::Controller + } + }), + } + }), + "Wojek exact AST" + ); + + let lt = parse_for_each_clause("opponent who has fewer cards in hand than you"); + assert!( + matches!( + lt, + Some(QuantityRef::PlayerCount { + filter: PlayerFilter::PlayerAttribute { + comparator: Comparator::LT, + .. + } + }) + ), + "fewer → LT: {lt:?}" + ); + + let eq = parse_for_each_clause("opponent who has as many cards in hand as you"); + assert!( + matches!( + eq, + Some(QuantityRef::PlayerCount { + filter: PlayerFilter::PlayerAttribute { + comparator: Comparator::EQ, + .. + } + }) + ), + "as many → EQ: {eq:?}" + ); + + // Reach-guard: the fixed-threshold arm is untouched — a numeric "N or more" + // still lowers to a `Fixed` operand with `GE` (backtrack intact). + let fixed = parse_for_each_clause("opponent who has 3 or more cards in hand"); + assert!( + matches!( + fixed, + Some(QuantityRef::PlayerCount { + filter: PlayerFilter::PlayerAttribute { + comparator: Comparator::GE, + .. + } + }) + ), + "fixed 'N or more' must still parse to GE: {fixed:?}" + ); + if let Some(QuantityRef::PlayerCount { + filter: PlayerFilter::PlayerAttribute { value, .. }, + }) = fixed + { + assert_eq!( + *value, + QuantityExpr::Fixed { value: 3 }, + "fixed operand = 3" + ); + } + } + /// The expected `QuantityExpr::Difference` for "power and toughness" order: /// `Difference { Ref(Power{Recipient}), Ref(Toughness{Recipient}) }`. /// Operand order is irrelevant at resolution (`Difference` resolves to an diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 6d7b8c16b6..7f49f73e74 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -737,6 +737,7 @@ mod tempt_with_discovery; mod terra_herald_optional_prompt; mod terra_magical_adept_milled_enchantment; mod terror_of_the_peaks_issue_2911; +mod teysa_wojek_investigate_per_opponent; mod the_chain_veil_loyalty_grants; mod the_fourteenth_doctor_graveyard_copy; mod the_kingpin_of_crime_combat_damage; diff --git a/crates/engine/tests/integration/teysa_wojek_investigate_per_opponent.rs b/crates/engine/tests/integration/teysa_wojek_investigate_per_opponent.rs new file mode 100644 index 0000000000..2f6903b6c2 --- /dev/null +++ b/crates/engine/tests/integration/teysa_wojek_investigate_per_opponent.rs @@ -0,0 +1,450 @@ +//! DynQty subgroup D — "[once] for each ⟨player-set⟩" lift on a fieldless +//! `Effect::Investigate` (parser-only). +//! +//! - **Teysa, Opulent Oligarch**: "At the beginning of your end step, investigate +//! for each opponent who lost life this turn." → repeat_for +//! `PlayerCount { OpponentLostLife }`. +//! - **Wojek Investigator**: "At the beginning of your upkeep, investigate once for +//! each opponent who has more cards in hand than you." → repeat_for +//! `PlayerCount { PlayerAttribute { Opponent, HandSize{ScopedPlayer}, GT, +//! Ref(HandSize{Controller}) } }`. +//! +//! Matrix #5/#6 drive the real parse pipeline (`parse_oracle_text`) on verbatim +//! Oracle text; matrix #7 drives the real runtime through `apply()` (a 4-player +//! upkeep trigger → Clue tokens), including the hostile tie fixture that proves +//! the comparative operand binds the controller (CR 109.5) and the attr binds +//! per-candidate. + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::parser::oracle::parse_oracle_text; +use engine::types::ability::{ + Comparator, Effect, PlayerFilter, PlayerRelation, PlayerScope, QuantityExpr, QuantityRef, +}; +use engine::types::game_state::GameState; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; + +const P2: PlayerId = PlayerId(2); +const P3: PlayerId = PlayerId(3); + +const TEYSA_ORACLE: &str = "Deathtouch\n\ + At the beginning of your end step, investigate for each opponent who lost life this turn.\n\ + Whenever a Clue you control is put into a graveyard from the battlefield, create a 1/1 white \ + and black Spirit creature token with flying. This ability triggers only once each turn."; + +const WOJEK_ORACLE: &str = "Flying, vigilance\n\ + At the beginning of your upkeep, investigate once for each opponent who has more cards in \ + hand than you. (To investigate, create a Clue token. It's an artifact with \"{2}, Sacrifice \ + this token: Draw a card.\")"; + +// The upkeep trigger sentence in isolation (verbatim, reminder retained) — used to +// seed the runtime creature so the Flying/vigilance keyword line does not interfere +// with the trigger under test. +const WOJEK_UPKEEP_TRIGGER: &str = "At the beginning of your upkeep, investigate once for each \ + opponent who has more cards in hand than you. (To investigate, create a Clue token. It's an \ + artifact with \"{2}, Sacrifice this token: Draw a card.\")"; + +// Teysa's end-step trigger sentence in isolation — the runtime creature carries ONLY +// the Investigate trigger (no Deathtouch, no Clue-death Spirit trigger) so the Clue +// delta measures the for-each investigate alone. +const TEYSA_END_STEP_TRIGGER: &str = + "At the beginning of your end step, investigate for each opponent who lost life this turn."; + +/// Matrix #5 — Teysa's end-step trigger carries `repeat_for = PlayerCount{OpponentLostLife}`. +/// Reach-guard: the trigger parsed to a real `Investigate` (not `Unimplemented`), so +/// the swallow assertion is not vacuous. Fails iff EDIT 1 is reverted; passes with EDIT 2 +/// reverted (Teysa does not exercise the comparative arm). +#[test] +fn teysa_end_step_investigate_lifts_opponent_lost_life() { + let parsed = parse_oracle_text(TEYSA_ORACLE, "Teysa, Opulent Oligarch", &[], &[], &[]); + + let end_step = parsed + .triggers + .iter() + .find(|t| t.phase == Some(Phase::End)) + .expect("Teysa has an end-step trigger"); + let execute = end_step.execute.as_ref().expect("end-step execute"); + + // Reach-guard: the effect really is Investigate (not Unimplemented) — the + // positive branch the swallow check would early-return past. + assert!( + matches!(execute.effect.as_ref(), Effect::Investigate), + "end-step effect must be Investigate, got {:?}", + execute.effect + ); + assert_eq!( + execute.repeat_for, + Some(QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { + filter: PlayerFilter::OpponentLostLife, + }, + }), + "Teysa must investigate once per opponent who lost life this turn" + ); + + // The DynamicQty / Duration_ThisTurn swallow warnings must clear (the for-each + // clause now owns the "this turn" duration and the player-set predicate). + assert!( + !parsed + .parse_warnings + .iter() + .any(|w| format!("{w:?}").contains("SwallowedClause")), + "no clause may remain swallowed: {:?}", + parsed.parse_warnings + ); +} + +/// Matrix #6 — Wojek's upkeep trigger carries the comparative `PlayerAttribute` +/// repeat_for. Verbatim input (reminder retained) self-guards reminder stripping. +/// Fails iff EDIT 1 OR EDIT 2 is reverted. +#[test] +fn wojek_upkeep_investigate_lifts_comparative_hand_size() { + let parsed = parse_oracle_text( + WOJEK_ORACLE, + "Wojek Investigator", + &["Flying".to_string(), "Vigilance".to_string()], + &[], + &[], + ); + + let upkeep = parsed + .triggers + .iter() + .find(|t| t.phase == Some(Phase::Upkeep)) + .expect("Wojek has an upkeep trigger"); + let execute = upkeep.execute.as_ref().expect("upkeep execute"); + + assert!( + matches!(execute.effect.as_ref(), Effect::Investigate), + "upkeep effect must be Investigate, got {:?}", + execute.effect + ); + assert_eq!( + execute.repeat_for, + Some(QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { + filter: PlayerFilter::PlayerAttribute { + relation: PlayerRelation::Opponent, + attr: Box::new(QuantityRef::HandSize { + player: PlayerScope::ScopedPlayer, + }), + comparator: Comparator::GT, + value: Box::new(QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::Controller, + }, + }), + }, + }, + }), + "Wojek must investigate once per opponent with strictly more cards in hand than the controller" + ); + + assert!( + !parsed + .parse_warnings + .iter() + .any(|w| format!("{w:?}").contains("SwallowedClause")), + "no clause may remain swallowed: {:?}", + parsed.parse_warnings + ); +} + +/// Count battlefield Clue tokens controlled by `player` (CR 111.10f — Clue subtype). +fn count_clues(state: &GameState, player: PlayerId) -> usize { + state + .battlefield + .iter() + .filter_map(|id| state.objects.get(id)) + .filter(|obj| obj.controller == player) + .filter(|obj| { + obj.card_types + .subtypes + .iter() + .any(|s| s.eq_ignore_ascii_case("Clue")) + }) + .count() +} + +fn hand_len(state: &GameState, player: PlayerId) -> usize { + state + .players + .iter() + .find(|p| p.id == player) + .map(|p| p.hand.len()) + .unwrap_or(0) +} + +/// Build a 4-player game (P0 controls Wojek) at P0's Untap step with the given +/// per-player hand sizes, then return the runner ready to advance into upkeep. +fn wojek_runner(hands: [(PlayerId, usize); 4]) -> engine::game::scenario::GameRunner { + let mut scenario = GameScenario::new_n_player(4, 20); + scenario.at_phase(Phase::Untap); + scenario + .add_creature(P0, "Wojek Investigator", 2, 2) + .from_oracle_text(WOJEK_UPKEEP_TRIGGER); + for (pid, n) in hands { + for _ in 0..n { + scenario.add_card_to_hand(pid, "Filler"); + } + } + let mut runner = scenario.build(); + runner.state_mut().turn_number = 2; + runner.state_mut().active_player = P0; + runner.state_mut().priority_player = P0; + runner +} + +/// Matrix #7 — end-to-end runtime binding through `apply()`. Controller hand = 2; +/// opponents A=4 (qualifies), B=1 (no), C=3 (qualifies) → exactly 2 Clues. +/// +/// Reach-guard (non-vacuous): the parsed upkeep trigger MUST carry the comparative +/// `PlayerAttribute` repeat_for before we cast — with EDIT 1/2 reverted the trigger +/// investigates once (1 Clue ≠ 2) and this precondition also fails first. +#[test] +fn wojek_runtime_makes_one_clue_per_opponent_with_more_cards() { + // Reach-guard: the fix is active (repeat_for is the comparative PlayerAttribute). + let parsed = parse_oracle_text( + WOJEK_ORACLE, + "Wojek Investigator", + &["Flying".to_string(), "Vigilance".to_string()], + &[], + &[], + ); + let repeat_for = parsed + .triggers + .iter() + .find(|t| t.phase == Some(Phase::Upkeep)) + .and_then(|t| t.execute.as_ref()) + .and_then(|e| e.repeat_for.clone()); + assert!( + matches!( + repeat_for, + Some(QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { + filter: PlayerFilter::PlayerAttribute { + comparator: Comparator::GT, + .. + } + } + }) + ), + "reach-guard: Wojek's trigger must carry the comparative repeat_for, got {repeat_for:?}" + ); + + // Controller P0 = 2; A(P1)=4 qualifies, B(P2)=1 no, C(P3)=3 qualifies. + let mut runner = wojek_runner([(P0, 2), (P1, 4), (P2, 1), (P3, 3)]); + assert_eq!( + hand_len(runner.state(), P0), + 2, + "precondition: controller hand = 2" + ); + assert_eq!( + hand_len(runner.state(), P1), + 4, + "precondition: opp A hand = 4" + ); + assert_eq!( + hand_len(runner.state(), P2), + 1, + "precondition: opp B hand = 1" + ); + assert_eq!( + hand_len(runner.state(), P3), + 3, + "precondition: opp C hand = 3" + ); + let clues_before = count_clues(runner.state(), P0); + + runner.advance_to_upkeep(); + runner.advance_until_stack_empty(); + + assert_eq!( + count_clues(runner.state(), P0) - clues_before, + 2, + "Wojek investigates once per opponent with strictly more cards than P0 (A and C) → 2 Clues" + ); +} + +/// Matrix #7 (hostile flip) — opp A ties the controller's hand size (2 == 2). GT +/// (CR 109.5 "more … than you") excludes the tie, so only C (3 > 2) qualifies → 1 +/// Clue. Proves `value` binds the controller and the attr binds per-candidate: if +/// the operand were per-candidate (or GE), the tie would count and the total would +/// be 2. +#[test] +fn wojek_runtime_excludes_opponent_tied_with_controller() { + let mut runner = wojek_runner([(P0, 2), (P1, 2), (P2, 1), (P3, 3)]); + assert_eq!( + hand_len(runner.state(), P1), + 2, + "precondition: opp A ties controller at 2" + ); + let clues_before = count_clues(runner.state(), P0); + + runner.advance_to_upkeep(); + runner.advance_until_stack_empty(); + + assert_eq!( + count_clues(runner.state(), P0) - clues_before, + 1, + "a tied opponent (A: 2 == controller 2) is excluded by GT → only C qualifies → 1 Clue" + ); +} + +/// Build a 4-player game (P0 controls Teysa) after combat (P0's post-combat main +/// phase) with the given per-player life-loss totals, then return the runner ready to +/// advance into the end step. Starting after combat means `advance_to_end_step` neither +/// halts at DeclareAttackers nor wraps a turn boundary, so `life_lost_this_turn` +/// (CR 119.3 — losing life; reset only at turn start by `start_next_turn`) is seeded at +/// build and survives to the end-step trigger's resolution. +fn teysa_runner(losses: [(PlayerId, u32); 4]) -> engine::game::scenario::GameRunner { + let mut scenario = GameScenario::new_n_player(4, 20); + scenario.at_phase(Phase::PostCombatMain); + scenario + .add_creature(P0, "Teysa, Opulent Oligarch", 2, 3) + .from_oracle_text(TEYSA_END_STEP_TRIGGER); + let mut runner = scenario.build(); + runner.state_mut().active_player = P0; + runner.state_mut().priority_player = P0; + for (pid, n) in losses { + if let Some(p) = runner.state_mut().players.iter_mut().find(|p| p.id == pid) { + p.life_lost_this_turn = n; + } + } + runner +} + +/// Matrix #7 (Teysa) — end-to-end runtime binding through `apply()`. Controller P0 +/// lost 5 (self-excluded), A(P1) lost 3, B(P2) lost 0 (zero-loss excluded), C(P3) lost +/// 1 → exactly 2 Clues. The `OpponentLostLife` predicate is +/// `p.id != controller && p.life_lost_this_turn > 0` (CR 119.3): P0 is the controller +/// so its own loss never counts, and P2's zero loss fails `> 0`. +/// +/// Reach-guard (non-vacuous): the parsed end-step trigger MUST carry the +/// `PlayerCount { OpponentLostLife }` repeat_for before we drive — with the lift +/// reverted the trigger is a bare Investigate (repeat_for == None → 1 Clue) and this +/// precondition also fails first. +/// +/// Discrimination: a wrong filter that counted the controller or a zero-loss player +/// would make 3; an absent wire (bare Investigate) would make 1; only the correct +/// `p.id != controller && life > 0` predicate makes 2. +#[test] +fn teysa_runtime_makes_one_clue_per_opponent_who_lost_life() { + // Reach-guard: the lift is active (repeat_for is PlayerCount{OpponentLostLife}). + let parsed = parse_oracle_text(TEYSA_ORACLE, "Teysa, Opulent Oligarch", &[], &[], &[]); + let repeat_for = parsed + .triggers + .iter() + .find(|t| t.phase == Some(Phase::End)) + .and_then(|t| t.execute.as_ref()) + .and_then(|e| e.repeat_for.clone()); + assert_eq!( + repeat_for, + Some(QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { + filter: PlayerFilter::OpponentLostLife, + }, + }), + "reach-guard: Teysa's end-step trigger must carry the OpponentLostLife repeat_for, got {repeat_for:?}" + ); + + // Controller P0 lost 5 (self-excluded); A(P1)=3 qualifies, B(P2)=0 excluded, + // C(P3)=1 qualifies. + let mut runner = teysa_runner([(P0, 5), (P1, 3), (P2, 0), (P3, 1)]); + let clues_before = count_clues(runner.state(), P0); + + runner.advance_to_end_step(); + runner.advance_until_stack_empty(); + + assert_eq!( + count_clues(runner.state(), P0) - clues_before, + 2, + "Teysa investigates once per opponent who lost life this turn (A and C) → 2 Clues" + ); +} + +/// Matrix #7 (Teysa, zero flip) — no opponent lost life this turn (P0 lost 4 but is the +/// controller; A/B/C lost 0). The for-each ranges over an empty player set, so the +/// repeat_for driver runs 0 iterations → 0 Clues (CR 513.1 — the end-step trigger still +/// fires and resolves; it simply investigates zero times). A non-lifted bare Investigate +/// would wrongly make 1 Clue, so the 0 delta is a crisp wire discriminator: the +/// revert-probe flips it 0 → 1. +#[test] +fn teysa_runtime_no_clue_when_no_opponent_lost_life() { + let mut runner = teysa_runner([(P0, 4), (P1, 0), (P2, 0), (P3, 0)]); + let clues_before = count_clues(runner.state(), P0); + + runner.advance_to_end_step(); + runner.advance_until_stack_empty(); + + assert_eq!( + count_clues(runner.state(), P0) - clues_before, + 0, + "no opponent lost life → empty for-each → 0 iterations → 0 Clues (a bare Investigate would make 1)" + ); +} + +// Verbatim (reminder retained). The only two "investigate ... for each" cards in the +// std corpus whose for-each ranges over OBJECTS (not a player set): Serene Sleuth +// ("for each goaded creature you control") and Sophina ("for each nontoken attacking +// creature"). The seam's `PlayerCount` gate must leave these on the unchanged final +// else — no spurious `repeat_for`. This is the class-wide zero-regression guard: it +// drives the real seam (not just the helper) and pairs the negative `repeat_for` +// assertion with a positive `Effect::Investigate` reach-guard so it is not vacuous. +const SERENE_SLEUTH: &str = "When this creature enters, investigate. (Create a Clue token. It's \ + an artifact with \"{2}, Sacrifice this token: Draw a card.\")\n\ + At the beginning of combat on your turn, investigate for each goaded creature you control. \ + Then each creature you control is no longer goaded."; + +const SOPHINA: &str = "Menace\n\ + Whenever Sophina, Spearsage Deserter attacks, investigate once for each nontoken attacking \ + creature. (To investigate, create a Clue token. It's an artifact with \"{2}, Sacrifice this \ + artifact: Draw a card.\")"; + +#[test] +fn object_for_each_investigate_is_not_spuriously_lifted() { + // Serene Sleuth's combat trigger: object for-each (goaded creatures). + let sleuth = parse_oracle_text(SERENE_SLEUTH, "Serene Sleuth", &[], &[], &[]); + // The combat trigger (a Phase trigger → `phase.is_some()`) is the object + // for-each; the ETB Investigate is a ChangesZone trigger (`phase.is_none()`). + let combat = sleuth + .triggers + .iter() + .filter(|t| t.phase.is_some()) + .filter_map(|t| t.execute.as_ref()) + .find(|e| matches!(e.effect.as_ref(), Effect::Investigate)) + .expect("Serene Sleuth has an Investigate combat trigger"); + assert!( + matches!(combat.effect.as_ref(), Effect::Investigate), + "reach-guard: Serene Sleuth's clause must parse to Investigate" + ); + assert!( + !matches!( + combat.repeat_for, + Some(QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { .. } + }) + ), + "object for-each (goaded creatures) must NOT be lifted to a PlayerCount repeat_for: {:?}", + combat.repeat_for + ); + + // Sophina's attack trigger: object for-each (nontoken attacking creatures). + let sophina = parse_oracle_text(SOPHINA, "Sophina, Spearsage Deserter", &[], &[], &[]); + let attack = sophina + .triggers + .iter() + .filter_map(|t| t.execute.as_ref()) + .find(|e| matches!(e.effect.as_ref(), Effect::Investigate)) + .expect("Sophina has an Investigate attack trigger"); + assert!( + !matches!( + attack.repeat_for, + Some(QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { .. } + }) + ), + "object for-each (nontoken attackers) must NOT be lifted to a PlayerCount repeat_for: {:?}", + attack.repeat_for + ); +} From 29e9ad9673625750ed396366d488d975923cc6c0 Mon Sep 17 00:00:00 2001 From: Lindsey Gray Date: Fri, 17 Jul 2026 16:05:47 -0500 Subject: [PATCH 2/4] feat(parser): generalize fieldless-Investigate for-each lift to member-count class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Responds to the #6110 review: the fieldless-Investigate "for each ⟨set⟩" lift was gated to QuantityRef::PlayerCount only, so object-ranged cards (Serene Sleuth: "investigate for each goaded creature you control") dropped to a bare 1-Clue Investigate (rules-wrong, CR 701.16a/608.2c). - Gate-widen: rename for_each_player_set_repeat_for -> for_each_repeatable_repeat_for and widen the single-seam eligibility gate to the member-count class matches!(qty, PlayerCount{..} | ObjectCount{..}) with a fail-closed _ => None. Preserves Teysa/Wojek (PlayerCount) + Serene Sleuth (ObjectCount); a contextual amount-ref (e.g. Tamiyo's "investigate twice for each card discarded", FilteredTrackedSetSize) is deliberately NOT lifted. Locked by a revert-probed negative-boundary test. - Gap A: add FilterProp::Goaded (candidate-local read of GameObject.goaded_by, CR 701.15b/c) across all 15 registration sites, mirroring FilterProp::Renowned. - ASK 2: project repeat_for into coverage.rs ability_details (None -> byte-identical). - Gap B (deferred): Sophina "nontoken attacking creature" strict-failure tag + flipping tripwire. Collateral (measured via regen-both at merge-base 931c2dc2f, ZERO regressions): FilterProp::Goaded also correctly parses the "goaded creature" filter on 4 previously-Unknown/degraded cards (Bothersome Quasit CantBlock static; Puppet Master / The Rani / Vengeful Ancestor triggers). A revert-probed runtime drive (vengeful_ancestor_goaded_attack_trigger.rs) proves the trigger-subject Goaded eval resolves against the LIVE attacker's goaded_by (CR 508.2a/603.2), not the fieldless EventObjectSnapshot -- so these are genuinely supported, not false-supported. Assisted-by: ClaudeCode:claude-opus-4.8 --- crates/engine/src/ai_support/filter.rs | 2 + crates/engine/src/game/ability_rw.rs | 6 + crates/engine/src/game/ability_scan.rs | 3 + crates/engine/src/game/coverage.rs | 41 ++++ crates/engine/src/game/filter.rs | 13 ++ .../engine/src/parser/oracle_effect/lower.rs | 71 ++++-- crates/engine/src/parser/oracle_effect/mod.rs | 23 +- crates/engine/src/parser/oracle_nom/filter.rs | 10 + crates/engine/src/parser/oracle_target.rs | 83 +++++++ crates/engine/src/types/ability.rs | 3 + crates/engine/src/types/events.rs | 2 + crates/engine/tests/integration/main.rs | 1 + .../teysa_wojek_investigate_per_opponent.rs | 179 +++++++++++++-- ...vengeful_ancestor_goaded_attack_trigger.rs | 207 ++++++++++++++++++ 14 files changed, 590 insertions(+), 54 deletions(-) create mode 100644 crates/engine/tests/integration/vengeful_ancestor_goaded_attack_trigger.rs diff --git a/crates/engine/src/ai_support/filter.rs b/crates/engine/src/ai_support/filter.rs index 30c128a40b..de352a35ff 100644 --- a/crates/engine/src/ai_support/filter.rs +++ b/crates/engine/src/ai_support/filter.rs @@ -728,6 +728,8 @@ fn filterprop_reads_only_candidate_fp(p: &FilterProp) -> bool { | FilterProp::PowerExceedsBase | FilterProp::Suspected | FilterProp::Renowned + // CR 701.15b/c: reads only the candidate's own `goaded_by` fingerprint field. + | FilterProp::Goaded | FilterProp::Modified | FilterProp::Historic | FilterProp::NotHistoric diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index c97d163191..62e184a3b3 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -2363,6 +2363,9 @@ fn legacy_filter_prop(p: &FilterProp) -> bool { | FilterProp::NotSupertype { .. } | FilterProp::Suspected | FilterProp::Renowned + // CR 701.15b/c: goad is a candidate-local designation, not a legacy + // event-context or per-source member-bound referent. + | FilterProp::Goaded | FilterProp::ToughnessGTPower | FilterProp::PowerExceedsBase | FilterProp::InAnyZone { .. } @@ -2624,6 +2627,9 @@ fn member_bound_filter_prop(p: &FilterProp) -> bool { | FilterProp::NotSupertype { .. } | FilterProp::Suspected | FilterProp::Renowned + // CR 701.15b/c: goad is a candidate-local designation, not a legacy + // event-context or per-source member-bound referent. + | FilterProp::Goaded | FilterProp::ToughnessGTPower | FilterProp::PowerExceedsBase | FilterProp::InAnyZone { .. } diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index ccd8eae787..d5d8349d07 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -3164,6 +3164,9 @@ fn scan_filter_prop(x: &FilterProp) -> Axes { | FilterProp::NotSupertype { .. } | FilterProp::Suspected | FilterProp::Renowned + // CR 701.15b/c: goad is a candidate-local designation read; it scans no + // board/object axis. + | FilterProp::Goaded | FilterProp::ToughnessGTPower | FilterProp::PowerExceedsBase | FilterProp::InTrackedSet { .. } diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 6b05b974ff..0388ec5cd8 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -896,6 +896,8 @@ fn fmt_typed_filter(tf: &TypedFilter) -> String { } FilterProp::Suspected => parts.push("suspected".into()), FilterProp::Renowned => parts.push("renowned".into()), + // CR 701.15b/c + FilterProp::Goaded => parts.push("goaded".into()), // CR 700.9 FilterProp::Modified => parts.push("modified".into()), // CR 700.6 @@ -3600,6 +3602,13 @@ fn ability_details(def: &AbilityDefinition) -> Vec<(String, String)> { if let Some(dur) = &def.duration { d.push(("duration".into(), fmt_duration(dur))); } + // CR 608.2c: a lifted "[once] for each ⟨set⟩" repeat multiplier (e.g. the + // fieldless-Investigate lift) is an `AbilityDefinition` field — surface it so + // the per-card signature reflects the member-count. `None` pushes nothing, so + // cards without a `repeat_for` keep a byte-identical signature. + if let Some(rf) = &def.repeat_for { + d.push(("repeat_for".into(), fmt_quantity(rf))); + } if def.optional_targeting { d.push(("targeting".into(), "optional (up to)".into())); } @@ -10674,6 +10683,38 @@ mod tests { ); } + #[test] + fn investigate_signature_exposes_repeat_for() { + // ASK 2: a lifted "[once] for each ⟨set⟩" multiplier (e.g. the fieldless + // Investigate lift → `def.repeat_for = Some(ObjectCount/PlayerCount)`) must be + // visible in the per-card parse-diff signature. `None` adds no row so unrelated + // cards keep a byte-identical signature. Reverting the `ability_details` + // projection flips the `Some` assertion to fail. + use crate::types::ability::{ + AbilityDefinition, AbilityKind, QuantityExpr, QuantityRef, TargetFilter, TypedFilter, + }; + let detail_keys = |repeat: Option| -> Vec { + let mut def = AbilityDefinition::new(AbilityKind::Spell, Effect::Investigate); + def.repeat_for = repeat; + ability_details(&def).into_iter().map(|(k, _)| k).collect() + }; + let object_count = QuantityExpr::Ref { + qty: QuantityRef::ObjectCount { + filter: TargetFilter::Typed(TypedFilter::creature()), + }, + }; + assert!( + detail_keys(Some(object_count)) + .iter() + .any(|k| k == "repeat_for"), + "a lifted repeat_for must appear in the per-card signature", + ); + assert!( + !detail_keys(None).iter().any(|k| k == "repeat_for"), + "an ability with no repeat_for must not add the row (byte-identical signature)", + ); + } + #[test] fn prevent_damage_signature_exposes_damage_source_filter() { // #5492: a change to `damage_source_filter` (e.g. unqualified diff --git a/crates/engine/src/game/filter.rs b/crates/engine/src/game/filter.rs index 04f6d5bd75..cacf185121 100644 --- a/crates/engine/src/game/filter.rs +++ b/crates/engine/src/game/filter.rs @@ -218,6 +218,9 @@ fn filter_prop_uses_object_population(prop: &FilterProp) -> bool { | FilterProp::NotSupertype { .. } | FilterProp::Suspected | FilterProp::Renowned + // CR 701.15b/c: goad is a candidate-local designation (reads only the + // object's own `goaded_by` set), so the board population is irrelevant. + | FilterProp::Goaded | FilterProp::ToughnessGTPower | FilterProp::PowerExceedsBase | FilterProp::Modified @@ -460,6 +463,9 @@ fn entered_object_perturbs_filter_prop( | FilterProp::NotSupertype { .. } | FilterProp::Suspected | FilterProp::Renowned + // CR 701.15b/c: an entering object cannot perturb a candidate-local goad + // designation (reads only the object's own `goaded_by` set). + | FilterProp::Goaded | FilterProp::ToughnessGTPower | FilterProp::PowerExceedsBase | FilterProp::Modified @@ -3350,6 +3356,8 @@ fn spell_record_matches_property(record: &SpellCastRecord, prop: &FilterProp) -> | FilterProp::HasSingleTarget | FilterProp::Suspected | FilterProp::Renowned + // CR 701.15b/c: a spell on the stack carries no goad designation. Fail closed. + | FilterProp::Goaded // CR 700.9: Modified requires on-battlefield attachments/counters, // unavailable from a stack-snapshot record. | FilterProp::Modified @@ -4295,6 +4303,8 @@ fn matches_filter_prop( FilterProp::Suspected => obj.is_suspected, // CR 702.112b: Match permanents with the renowned designation. FilterProp::Renowned => obj.is_renowned, + // CR 701.15b/c: a creature is goaded iff at least one player has goaded it. + FilterProp::Goaded => !obj.goaded_by.is_empty(), // CR 700.9: A permanent is modified if it has one or more counters on // it (CR 122), is equipped (CR 301.5), or is enchanted by an Aura // controlled by its controller (CR 303.4). @@ -5036,6 +5046,9 @@ fn zone_change_record_matches_property( // evaluated on the live stack object, not the snapshot). | FilterProp::Modal | FilterProp::Renowned + // CR 701.15b/c: goad is not snapshotted onto the zone-change record + // (unlike Suspected's `record.is_suspected`). Fail closed. + | FilterProp::Goaded // CR 700.9: Modified is a live-battlefield predicate (counters + // attachments) — a zone-change snapshot cannot represent it. | FilterProp::Modified diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index e01c406471..1278d71494 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -3533,7 +3533,7 @@ pub(super) fn zada_repeat_for_implies_distinct_copy_targets(qty: &QuantityExpr) /// Split a clause at the first " for each " boundary. Returns the base byte-length /// (an offset into the ORIGINAL text — lowercasing is byte-length-preserving for the /// ASCII Oracle corpus) and the lowercase tail after " for each ". The single split -/// authority shared by `strip_for_each_repeat_suffix` and `for_each_player_set_repeat_for`. +/// authority shared by `strip_for_each_repeat_suffix` and `for_each_repeatable_repeat_for`. fn split_for_each_suffix(text: &str) -> Option<(usize, String)> { let lower = text.to_lowercase(); let (rest, base) = take_until::<_, _, OracleError<'_>>(" for each ") @@ -3557,7 +3557,7 @@ pub(super) fn strip_for_each_repeat_suffix(text: &str) -> (Option, // Unchanged gate: the repeat-suffix lift is restricted to CommanderCast // and Zada distinct-copy today. A player-set `PlayerCount` is deliberately // NOT admitted here — that class routes through the fieldless-Investigate - // seam via `for_each_player_set_repeat_for`. + // seam via `for_each_repeatable_repeat_for`. if matches!(&qty, QuantityRef::CommanderCastFromCommandZoneCount) || zada_repeat_for_implies_distinct_copy_targets(&QuantityExpr::Ref { qty: qty.clone(), @@ -3573,17 +3573,26 @@ pub(super) fn strip_for_each_repeat_suffix(text: &str) -> (Option, (None, text) } -/// CR 701.16a + CR 608.2c: Lift a trailing "[once] for each ⟨player-set⟩" multiplier -/// off a fieldless keyword-action effect (Investigate has no count slot) into a -/// `repeat_for`. Uses the `parse_for_each_clause` WRAPPER — NOT -/// `parse_for_each_clause_ref` — because the `PlayerAttribute` producer (Wojek's -/// "opponent who has more cards in hand than you") is reachable only via the -/// wrapper's `oracle_quantity` fallback. Gated on `PlayerCount` so object for-each -/// is left to the count-bearing effect path. -pub(super) fn for_each_player_set_repeat_for(text: &str) -> Option { +/// CR 701.16a + CR 608.2c: Lift a trailing "[once] for each ⟨set⟩" multiplier off a +/// fieldless keyword-action effect (Investigate has no count slot) into a `repeat_for`. +/// Restricted to the per-each MEMBER-COUNT class — a count of the players or objects the +/// "for each" ranges over: `PlayerCount` (including its nested `PlayerAttribute` filter, +/// e.g. Wojek's comparative hand size) and `ObjectCount` (e.g. Serene Sleuth's goaded +/// creatures). Contextual amount-refs (`FilteredTrackedSetSize` / `TrackedSetSize` / +/// `PreviousEffectAmount` / `EventContextAmount`) are deliberately NOT lifted, and the +/// match is fail-closed (an unrecognized ref leaves the Investigate bare). This matters +/// because such refs co-occur with a leading Fixed multiplier the single `repeat_for` +/// slot cannot represent: Tamiyo Meets the Story Circle's "investigate TWICE for each +/// card discarded this way" would otherwise lift the per-each `FilteredTrackedSetSize` +/// and silently DROP the "twice" (N Clues instead of 2×N). The runtime repeat_for driver +/// resolves either admitted member-count generically (one Clue per member). One shape — +/// a class-membership guard, not per-family handling. +pub(super) fn for_each_repeatable_repeat_for(text: &str) -> Option { let (_, tail) = split_for_each_suffix(text)?; match parse_for_each_clause(&tail) { - Some(qty @ QuantityRef::PlayerCount { .. }) => Some(QuantityExpr::Ref { qty }), + Some(qty @ (QuantityRef::PlayerCount { .. } | QuantityRef::ObjectCount { .. })) => { + Some(QuantityExpr::Ref { qty }) + } _ => None, } } @@ -11591,7 +11600,7 @@ mod strip_optional_effect_prefix_tests { /// lift helper, and the wrapper-vs-`_ref` non-domination guard. #[cfg(test)] mod dq_d_player_set_lift_tests { - use super::{for_each_player_set_repeat_for, strip_for_each_repeat_suffix}; + use super::{for_each_repeatable_repeat_for, strip_for_each_repeat_suffix}; use crate::parser::oracle_nom::quantity::parse_for_each_clause_ref; use crate::types::ability::{PlayerFilter, QuantityExpr, QuantityRef}; @@ -11651,12 +11660,12 @@ mod dq_d_player_set_lift_tests { assert_eq!(base, "draw a card"); } - // Matrix #4 — the player-set lift helper. + // Matrix #4 — the repeatable member-count lift helper (widened: player-set OR object-set). #[test] - fn for_each_player_set_repeat_for_lifts_player_count_only() { + fn for_each_repeatable_repeat_for_lifts_any_repeatable_count() { // Teysa: OpponentLostLife → PlayerCount. let teysa = - for_each_player_set_repeat_for("investigate for each opponent who lost life this turn"); + for_each_repeatable_repeat_for("investigate for each opponent who lost life this turn"); assert!( matches!( teysa, @@ -11673,7 +11682,7 @@ mod dq_d_player_set_lift_tests { // helper body from the `parse_for_each_clause` wrapper to `parse_for_each_clause_ref` // makes THIS case return `None` (the `_ref` alt has no PlayerAttribute arm) — // that is the wrapper-vs-`_ref` guard. - let wojek = for_each_player_set_repeat_for( + let wojek = for_each_repeatable_repeat_for( "investigate once for each opponent who has more cards in hand than you", ); assert!( @@ -11688,14 +11697,36 @@ mod dq_d_player_set_lift_tests { "Wojek must lift PlayerAttribute via the wrapper: {wojek:?}" ); - // Object for-each is left to the count-bearing path (ObjectCount ≠ PlayerCount). + // Object for-each now DOES lift (parameterized gate-widen). "attacking creature + // you control" is an already-supported typed filter (needs no Gap A / FilterProp:: + // Goaded), so the widened helper lifts it to `ObjectCount`. REVERT PROBE: narrowing + // the gate back to `PlayerCount`-only flips this assertion to None. + let object_lift = + for_each_repeatable_repeat_for("investigate for each attacking creature you control"); + assert!( + matches!( + object_lift, + Some(QuantityExpr::Ref { + qty: QuantityRef::ObjectCount { .. } + }) + ), + "object for-each must now lift to ObjectCount: {object_lift:?}" + ); + + // Amount-ref for-each is NOT lifted (fail-closed member-count restriction). + // Tamiyo Meets the Story Circle's "investigate twice for each card discarded this + // way" parses the tail to a contextual `FilteredTrackedSetSize`, NOT a member + // count. Lifting it would silently drop the leading "twice" Fixed multiplier + // (N Clues instead of 2×N — CR 701.16a). REVERT PROBE: broadening the body back to + // `parse_for_each_clause(&tail).map(...)` makes this return `Some` and FAILS. assert!( - for_each_player_set_repeat_for("investigate for each artifact you control").is_none(), - "object for-each must NOT lift here" + for_each_repeatable_repeat_for("investigate twice for each card discarded this way") + .is_none(), + "a contextual amount-ref (FilteredTrackedSetSize) must NOT be lifted" ); // No "for each" suffix → None. - assert!(for_each_player_set_repeat_for("investigate").is_none()); + assert!(for_each_repeatable_repeat_for("investigate").is_none()); } // Matrix #2 — non-domination: the bare `_ref` combinator does NOT consume Wojek's diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index e87a51543d..401b73f409 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -27923,15 +27923,16 @@ pub(crate) fn parse_effect_chain_ir( strip_for_each_repeat_suffix(&text_no_qty); let mut stripped_clause = parse_effect_clause(&stripped_text_no_qty, ctx); // CR 701.16a + CR 608.2c: a fieldless Investigate (no count slot) drops a - // trailing "[once] for each " multiplier. Precompute the lift, - // gated to Investigate + no prior repeat_for, so ONLY the player-set-for-each - // Investigate class enters the branch below; every other Investigate chunk - // (plain "Investigate.", object for-each) falls through to the unchanged final - // else. `for_each_player_set_repeat_for` returns None without a real - // "for each " suffix. - let investigate_player_set_lift = + // trailing "[once] for each " multiplier. Precompute the lift, + // gated to Investigate + no prior repeat_for, so ONLY the repeatable-for-each + // Investigate class (player-set OR object-set) enters the branch below; every + // other Investigate chunk (plain "Investigate.", non-Investigate for-each) + // falls through to the unchanged final else. + // `for_each_repeatable_repeat_for` returns None without a real + // "for each " suffix. + let investigate_repeatable_lift = if repeat_for.is_none() && matches!(stripped_clause.effect, Effect::Investigate) { - lower::for_each_player_set_repeat_for(&text_no_qty) + lower::for_each_repeatable_repeat_for(&text_no_qty) } else { None }; @@ -27965,12 +27966,12 @@ pub(crate) fn parse_effect_chain_ir( *ctx = fanout_ctx; multi_target = Some(fanout_spec); (fanout_clause, None) - } else if let Some(player_qty) = investigate_player_set_lift { + } else if let Some(lifted_qty) = investigate_repeatable_lift { // CR 608.2c: re-parse `text_no_qty` EXACTLY as the final else does // (clause + ctx byte-identical), attaching ONLY the lifted repeat_for. - // The gate guaranteed `repeat_for.is_none()`, so `Some(player_qty)` + // The gate guaranteed `repeat_for.is_none()`, so `Some(lifted_qty)` // cannot clobber a prior count. - (parse_effect_clause(&text_no_qty, ctx), Some(player_qty)) + (parse_effect_clause(&text_no_qty, ctx), Some(lifted_qty)) } else { (parse_effect_clause(&text_no_qty, ctx), repeat_for) } diff --git a/crates/engine/src/parser/oracle_nom/filter.rs b/crates/engine/src/parser/oracle_nom/filter.rs index 8e59df8050..086ad1e75a 100644 --- a/crates/engine/src/parser/oracle_nom/filter.rs +++ b/crates/engine/src/parser/oracle_nom/filter.rs @@ -164,6 +164,8 @@ pub fn parse_property_filter(input: &str) -> OracleResult<'_, FilterProp> { value(FilterProp::Unblocked, tag("unblocked")), value(FilterProp::Suspected, tag("suspected")), value(FilterProp::Renowned, tag("renowned")), + // CR 701.15b/c: standalone "goaded" designation property token. + value(FilterProp::Goaded, tag("goaded")), value(FilterProp::EnchantedBy, tag("enchanted")), value(FilterProp::EquippedBy, tag("equipped")), parse_color_property, @@ -565,6 +567,14 @@ mod tests { assert_eq!(rest, " creature"); } + #[test] + fn test_parse_property_filter_goaded() { + // CR 701.15b/c: standalone "goaded" designation property token (Gap A, site 14). + let (rest, p) = parse_property_filter("goaded creature").unwrap(); + assert_eq!(p, FilterProp::Goaded); + assert_eq!(rest, " creature"); + } + #[test] fn test_parse_property_filter_failure() { assert!(parse_property_filter("flying").is_err()); diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index f0ee871e88..2f9eee675f 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -2130,6 +2130,21 @@ pub fn parse_type_phrase_with_ctx<'a>( } } + // GAP B (DEFERRED — strict-failure tag, DynQty subgroup D follow-up): the leading + // adjective handlers here run as a fixed positional cascade (combat-status → + // enchanted/equipped → modified → renowned → goaded → historic → … → nontoken at + // ~:2310). A phrase whose adjectives appear in a different order — notably "nontoken + // attacking creature" (Sophina, Spearsage Deserter) — is only partly stripped: + // "nontoken" leads, so THIS combat-status loop never sees "attacking"; by the time + // "nontoken " is consumed further down, the combat-status loop has already passed, so + // "attacking creature" fails the type parse and `parse_for_each_clause` returns None + // (NO false lift — Sophina's Investigate stays bare and coverage stays honestly RED). + // The fix is to collapse this cascade into a single order-free many0-style property + // loop, but that is the hottest shared parser path (high CI-regression blast radius) + // and is out of scope here. Tripwire: the Sophina branch of + // `object_for_each_investigate_is_lifted` asserts the bare-Investigate state and + // FLIPS to fail when this gap is closed. + // // CR 509.1h: Consume combat status prefixes (unblocked, attacking, blocking). // Handles "or" compound as a property disjunction: "attacking or blocking // creature" means attacking creature OR blocking creature, not both. @@ -2203,6 +2218,17 @@ pub fn parse_type_phrase_with_ctx<'a>( } } + // CR 701.15b/c: "goaded" is a permanent designation used as an adjective in + // filters like "goaded creature you control". Mirrors the "renowned" strip: + // only consume when a type word follows, so the "goad target creature" verb + // path is untouched. + if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("goaded ").parse(&lower[pos..]) { + if starts_with_type_phrase_lead(rest) { + properties.push(FilterProp::Goaded); + pos += lower[pos..].len() - rest.len(); + } + } + // CR 700.6: "historic" adjective prefix. An object is historic if it has // the legendary supertype, the artifact card type, or the Saga subtype. // Emits FilterProp::Historic (a first-class typed predicate — see @@ -3922,6 +3948,8 @@ pub(crate) fn is_adjective_prefix_prop(prop: &FilterProp) -> bool { FilterProp::Modified // CR 702.112b: "renowned [type]" adjective prefix. | FilterProp::Renowned + // CR 701.15b/c: "goaded [type]" adjective prefix. + | FilterProp::Goaded // CR 700.6: "historic [type]" adjective prefix. | FilterProp::Historic | FilterProp::NotHistoric @@ -12125,6 +12153,61 @@ mod tests { assert_eq!(rest.trim(), ""); } + #[test] + fn goaded_adjective_creates_filter_prop() { + // CR 701.15b/c: "goaded creature" is a designation adjective (Gap A, site 15). + // This is the exact path Serene Sleuth's "goaded creature you control" takes. + let (f, rest) = parse_type_phrase("goaded creature you control"); + assert_eq!( + f, + TargetFilter::Typed( + TypedFilter::creature() + .controller(ControllerRef::You) + .properties(vec![FilterProp::Goaded]) + ) + ); + assert_eq!(rest.trim(), ""); + } + + #[test] + fn goad_verb_is_not_a_goaded_filter_prop() { + // Negative sibling: the Goad verb ("goad target creature") must NOT be + // misread as the `FilterProp::Goaded` designation. The adjective strip is + // `tag("goaded ")` guarded on a trailing type word, so the bare verb "goad " + // never fires it. + let (f, _rest) = parse_type_phrase("goad target creature"); + let has_goaded = match &f { + TargetFilter::Typed(t) => t.properties.contains(&FilterProp::Goaded), + _ => false, + }; + assert!( + !has_goaded, + "the Goad verb must not produce a FilterProp::Goaded designation: {f:?}" + ); + } + + #[test] + fn goaded_is_registered_as_leg_local_adjective_prefix() { + // Site 13 (`is_adjective_prefix_prop`) — the silent-break registration and the + // review's headline miss. This predicate is the single leg-locality registry for + // both disjunctive grammars; an unregistered adjective prop is wrongly + // distributed across earlier `Or` legs (the #2892 class bug). + // + // This is a DIRECT unit guard rather than a behavioral multi-leg parse: I + // measured that the natural "goaded X or Y" disjunction does not route through + // `parse_type_phrase`'s Or distributor — `parse_type_phrase("goaded creature or + // an artifact")` leaves " or an artifact" unconsumed (no in-repo grammar emits a + // goaded disjunction), which the plan anticipated as the fallback case. The + // direct guard is nonetheless a genuine revert-probe: dropping the + // `| FilterProp::Goaded` arm from `is_adjective_prefix_prop` flips this to false + // and FAILS, so the silent class bug cannot ship undetected. + assert!( + is_adjective_prefix_prop(&FilterProp::Goaded), + "FilterProp::Goaded must register as a leg-local adjective prefix, or it \ + distributes across earlier Or legs and silently breaks 'goaded X or Y' filters" + ); + } + #[test] fn modified_adjective_in_comma_list_silkguard() { // CR 700.4 + CR 700.9: Silkguard — "Auras, Equipment, and modified diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 8655649770..87f9324f26 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -3757,6 +3757,9 @@ pub enum FilterProp { Suspected, /// CR 702.112b: Matches permanents with the renowned designation. Renowned, + /// CR 701.15b/c: Matches creatures with the goaded designation (at least one + /// player has goaded it). + Goaded, /// CR 510.1c: Matches creatures whose toughness is greater than their power. ToughnessGTPower, /// CR 208.1 + CR 613.4a + CR 613.4b: Matches a creature whose current diff --git a/crates/engine/src/types/events.rs b/crates/engine/src/types/events.rs index 9d50f3a8cf..cd53439950 100644 --- a/crates/engine/src/types/events.rs +++ b/crates/engine/src/types/events.rs @@ -569,6 +569,8 @@ impl EventObjectSnapshot { | FilterProp::Transformed | FilterProp::Suspected | FilterProp::Renowned + // CR 701.15b/c: goad is an embedded candidate-local designation. + | FilterProp::Goaded | FilterProp::IsSaddled => Supported, // ---- embedded characteristics ---- diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 7f49f73e74..d57f408163 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -768,6 +768,7 @@ mod urge_to_feed_regression; mod urza_lord_high_artificer_shuffle_exile_free_cast; mod urzas_saga_chapter_two; mod urzas_tower_conditional_mana; +mod vengeful_ancestor_goaded_attack_trigger; mod veteran_armorsmith_soldier_anthem; mod vigor_regression; mod vincents_limit_break_tiered; diff --git a/crates/engine/tests/integration/teysa_wojek_investigate_per_opponent.rs b/crates/engine/tests/integration/teysa_wojek_investigate_per_opponent.rs index 2f6903b6c2..caad19dc1d 100644 --- a/crates/engine/tests/integration/teysa_wojek_investigate_per_opponent.rs +++ b/crates/engine/tests/integration/teysa_wojek_investigate_per_opponent.rs @@ -18,7 +18,8 @@ use engine::game::scenario::{GameScenario, P0, P1}; use engine::parser::oracle::parse_oracle_text; use engine::types::ability::{ - Comparator, Effect, PlayerFilter, PlayerRelation, PlayerScope, QuantityExpr, QuantityRef, + Comparator, ControllerRef, Effect, FilterProp, PlayerFilter, PlayerRelation, PlayerScope, + QuantityExpr, QuantityRef, TargetFilter, TypeFilter, }; use engine::types::game_state::GameState; use engine::types::phase::Phase; @@ -387,10 +388,11 @@ fn teysa_runtime_no_clue_when_no_opponent_lost_life() { // Verbatim (reminder retained). The only two "investigate ... for each" cards in the // std corpus whose for-each ranges over OBJECTS (not a player set): Serene Sleuth // ("for each goaded creature you control") and Sophina ("for each nontoken attacking -// creature"). The seam's `PlayerCount` gate must leave these on the unchanged final -// else — no spurious `repeat_for`. This is the class-wide zero-regression guard: it -// drives the real seam (not just the helper) and pairs the negative `repeat_for` -// assertion with a positive `Effect::Investigate` reach-guard so it is not vacuous. +// creature"). The parameterized gate-widen + Gap A (`FilterProp::Goaded`) now lift +// Serene Sleuth to an `ObjectCount` repeat_for; Sophina stays a bare Investigate +// (Gap B deferred — parse_type_phrase leading-adjective order-dependence). Both +// branches drive the real seam and pair their `repeat_for` assertion with a positive +// `Effect::Investigate` reach-guard so neither is vacuous. const SERENE_SLEUTH: &str = "When this creature enters, investigate. (Create a Clue token. It's \ an artifact with \"{2}, Sacrifice this token: Draw a card.\")\n\ At the beginning of combat on your turn, investigate for each goaded creature you control. \ @@ -401,9 +403,107 @@ const SOPHINA: &str = "Menace\n\ creature. (To investigate, create a Clue token. It's an artifact with \"{2}, Sacrifice this \ artifact: Draw a card.\")"; +// Serene Sleuth's combat-trigger sentence in isolation — the runtime creature carries +// ONLY the object for-each Investigate (not the ETB Investigate, not the un-goad sibling) +// so the Clue delta measures the goaded-creature count alone. +const SERENE_COMBAT_TRIGGER: &str = + "At the beginning of combat on your turn, investigate for each goaded creature you control."; + +/// Build a 4-player game (P0 controls Serene Sleuth) in P0's precombat main with +/// `n_goaded` P0 creatures each goaded by P1 and `n_plain` ungoaded P0 creatures, ready +/// to advance into the beginning-of-combat step. Serene Sleuth itself is an ungoaded P0 +/// creature, so a filter that ignored the goad designation (CR 701.15b/c) would +/// over-count (it would include Sleuth and the plain creatures). +fn serene_runner(n_goaded: usize, n_plain: usize) -> engine::game::scenario::GameRunner { + let mut scenario = GameScenario::new_n_player(4, 20); + scenario.at_phase(Phase::PreCombatMain); + scenario + .add_creature(P0, "Serene Sleuth", 2, 2) + .from_oracle_text(SERENE_COMBAT_TRIGGER); + let mut goaded_ids = Vec::new(); + for i in 0..n_goaded { + goaded_ids.push( + scenario + .add_creature(P0, &format!("Goaded Ox {i}"), 2, 2) + .id(), + ); + } + for i in 0..n_plain { + scenario.add_creature(P0, &format!("Calm Bear {i}"), 2, 2); + } + let mut runner = scenario.build(); + runner.state_mut().active_player = P0; + runner.state_mut().priority_player = P0; + // CR 701.15b/c: designate each Ox as goaded by P1 — a nonempty `goaded_by` set is + // exactly what `FilterProp::Goaded` reads. + for id in goaded_ids { + if let Some(obj) = runner.state_mut().objects.get_mut(&id) { + obj.goaded_by.insert(P1); + } + } + runner +} + +/// Matrix #7 (Serene Sleuth) — RUNTIME discriminator, end-to-end binding through +/// `apply()`. P0 controls Serene Sleuth (ungoaded) + 3 creatures goaded by P1 + 1 +/// ungoaded plain creature. The beginning-of-combat trigger investigates once per goaded +/// creature P0 controls (CR 701.16a + CR 701.15b/c) → exactly 3 Clues. +/// +/// Reach-guard (non-vacuous): the parsed combat trigger MUST carry the +/// `ObjectCount { Typed(.., [Goaded]) }` repeat_for before we drive — with the gate +/// narrowed (revert-probe a) or Gap A reverted (revert-probe b) the trigger is a bare +/// Investigate (repeat_for None → 1 Clue) and this precondition also fails first. +/// +/// Discrimination (three distinct outcomes): the correct goaded filter → 3; a filter +/// that ignored `FilterProp::Goaded` would count all 5 P0 creatures (Sleuth + 3 Ox + 1 +/// Bear) → 5; a non-lifted bare Investigate → 1. Only the correct wire makes 3. +/// (The "no longer goaded" sibling sentence is not part of the isolated trigger text, so +/// nothing un-goads the Oxen mid-resolution.) #[test] -fn object_for_each_investigate_is_not_spuriously_lifted() { - // Serene Sleuth's combat trigger: object for-each (goaded creatures). +fn serene_sleuth_runtime_makes_one_clue_per_goaded_creature() { + // Reach-guard: the lift is active (repeat_for is ObjectCount carrying Goaded). + let parsed = parse_oracle_text(SERENE_COMBAT_TRIGGER, "Serene Sleuth", &[], &[], &[]); + let repeat_for = parsed + .triggers + .iter() + .find(|t| t.phase == Some(Phase::BeginCombat)) + .and_then(|t| t.execute.as_ref()) + .and_then(|e| e.repeat_for.clone()); + match &repeat_for { + Some(QuantityExpr::Ref { + qty: + QuantityRef::ObjectCount { + filter: TargetFilter::Typed(t), + }, + }) => assert!( + t.properties.contains(&FilterProp::Goaded), + "reach-guard: repeat_for filter must carry FilterProp::Goaded, got {:?}", + t.properties + ), + other => { + panic!( + "reach-guard: combat trigger must carry an ObjectCount repeat_for, got {other:?}" + ) + } + } + + let mut runner = serene_runner(3, 1); + let clues_before = count_clues(runner.state(), P0); + + runner.advance_to_phase(Phase::BeginCombat); + runner.advance_until_stack_empty(); + + assert_eq!( + count_clues(runner.state(), P0) - clues_before, + 3, + "Serene Sleuth investigates once per goaded creature P0 controls (3 Ox) → 3 Clues \ + (a Goaded-blind filter would make 5; a bare Investigate would make 1)" + ); +} + +#[test] +fn object_for_each_investigate_is_lifted() { + // Serene Sleuth's combat trigger: object for-each (goaded creatures you control). let sleuth = parse_oracle_text(SERENE_SLEUTH, "Serene Sleuth", &[], &[], &[]); // The combat trigger (a Phase trigger → `phase.is_some()`) is the object // for-each; the ETB Investigate is a ChangesZone trigger (`phase.is_none()`). @@ -414,22 +514,55 @@ fn object_for_each_investigate_is_not_spuriously_lifted() { .filter_map(|t| t.execute.as_ref()) .find(|e| matches!(e.effect.as_ref(), Effect::Investigate)) .expect("Serene Sleuth has an Investigate combat trigger"); + // Reach-guard: the effect really is Investigate (not Unimplemented) — the + // positive branch the seam gate reads before the lift. assert!( matches!(combat.effect.as_ref(), Effect::Investigate), "reach-guard: Serene Sleuth's clause must parse to Investigate" ); - assert!( - !matches!( - combat.repeat_for, - Some(QuantityExpr::Ref { - qty: QuantityRef::PlayerCount { .. } - }) + // The parameterized gate-widen lifts the object for-each to an `ObjectCount` + // repeat_for whose filter is `Typed(Creature, You, [Goaded])`. + // Revert-probe (a) — narrow the gate back to `PlayerCount`-only → the + // `ObjectCount` is rejected → repeat_for None → the `ObjectCount` match FAILS. + // Revert-probe (b) — drop Gap A parser sites 14/15 → "goaded creature you + // control" no longer parses to a typed filter → `parse_for_each_clause` + // returns None → the seam finds no count → repeat_for None → FAILS. + let filter = match &combat.repeat_for { + Some(QuantityExpr::Ref { + qty: QuantityRef::ObjectCount { filter }, + }) => filter, + other => panic!( + "Serene Sleuth combat trigger must lift to an ObjectCount repeat_for, got {other:?}" ), - "object for-each (goaded creatures) must NOT be lifted to a PlayerCount repeat_for: {:?}", - combat.repeat_for + }; + let typed = match filter { + TargetFilter::Typed(t) => t, + other => panic!("the lifted ObjectCount filter must be Typed, got {other:?}"), + }; + // Gap A is load-bearing for THIS card: the filter must carry `FilterProp::Goaded`. + assert!( + typed.properties.contains(&FilterProp::Goaded), + "the lifted filter must carry FilterProp::Goaded (Gap A), got {:?}", + typed.properties + ); + assert!( + typed.type_filters.contains(&TypeFilter::Creature), + "the lifted filter must be a creature filter, got {:?}", + typed.type_filters + ); + assert_eq!( + typed.controller, + Some(ControllerRef::You), + "the lifted filter must be scoped to 'you control', got {:?}", + typed.controller ); - // Sophina's attack trigger: object for-each (nontoken attacking creatures). + // Sophina's attack trigger: object for-each ("nontoken attacking creature") — + // Gap B (DEFERRED). parse_type_phrase's leading-adjective order-dependence means + // this for-each does NOT yet parse to a member-count, so the seam leaves it a bare + // Investigate. Deferred-gap tripwire: paired with a positive `Effect::Investigate` + // reach-guard (non-vacuous), it asserts the CURRENT bare-Investigate state and + // FLIPS to fail when Gap B lands — the signal to update this expectation. let sophina = parse_oracle_text(SOPHINA, "Sophina, Spearsage Deserter", &[], &[], &[]); let attack = sophina .triggers @@ -438,13 +571,13 @@ fn object_for_each_investigate_is_not_spuriously_lifted() { .find(|e| matches!(e.effect.as_ref(), Effect::Investigate)) .expect("Sophina has an Investigate attack trigger"); assert!( - !matches!( - attack.repeat_for, - Some(QuantityExpr::Ref { - qty: QuantityRef::PlayerCount { .. } - }) - ), - "object for-each (nontoken attackers) must NOT be lifted to a PlayerCount repeat_for: {:?}", + matches!(attack.effect.as_ref(), Effect::Investigate), + "reach-guard: Sophina's clause must parse to Investigate" + ); + assert!( + attack.repeat_for.is_none(), + "Gap B deferred (parse_type_phrase leading-adjective order-dependence): \ + 'nontoken attacking creature' does not yet lift — flip this guard when Gap B lands: {:?}", attack.repeat_for ); } diff --git a/crates/engine/tests/integration/vengeful_ancestor_goaded_attack_trigger.rs b/crates/engine/tests/integration/vengeful_ancestor_goaded_attack_trigger.rs new file mode 100644 index 0000000000..b3bb7d495f --- /dev/null +++ b/crates/engine/tests/integration/vengeful_ancestor_goaded_attack_trigger.rs @@ -0,0 +1,207 @@ +//! DynQty subgroup D collateral — `FilterProp::Goaded` as an **Attacks-trigger +//! subject** filter, driven end-to-end through `apply()`. +//! +//! PR #6110 (dq-d) adds `FilterProp::Goaded`. Besides the intended Serene Sleuth +//! `ObjectCount` (battlefield-scan) lift, it now also parses the "goaded creature" +//! subject of TRIGGERS — e.g. Vengeful Ancestor's "Whenever a goaded creature +//! attacks, it deals 1 damage to its controller." (verified verbatim against +//! card-data). That is a DISTINCT new wire from the ObjectCount scan: the trigger's +//! `valid_card` Goaded filter is evaluated against the attacking object. +//! +//! The risk the driver flagged: `EventObjectSnapshot` (types/events.rs) carries no +//! goaded field. If the trigger's `valid_card` were evaluated against the fieldless +//! snapshot instead of the LIVE attacker, `Goaded` would always read false, the +//! trigger would NEVER fire, and the card would be FALSE-SUPPORTED (strictly worse +//! than the pre-PR Unknown). This runtime pair proves the eval resolves against the +//! live attacker (which carries `goaded_by`): +//! - goaded leg → the goaded attacker's controller loses exactly 1 life. +//! - ungoaded leg → identical setup minus the goad → controller loses 0 life. +//! +//! The two legs differ ONLY in the Ox's goaded designation, so the delta isolates +//! the `FilterProp::Goaded` evaluation on the trigger subject. +//! +//! CR references: +//! - CR 701.15b/c: a creature is goaded iff at least one player has goaded it; +//! it must attack and attack a player other than its goader if able. +//! - CR 508.1a: a creature attacks only on its controller's turn. +//! - CR 508.2a + CR 603.2: an attacks-triggered ability triggers at the point a +//! creature is declared as an attacker; its trigger event is that attacking +//! creature, so the `valid_card` filter is evaluated against the live attacker, +//! not the trigger's own source. + +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::parser::oracle::parse_oracle_text; +use engine::types::ability::{FilterProp, TargetFilter}; +use engine::types::actions::GameAction; +use engine::types::game_state::{GameState, WaitingFor}; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; +use engine::types::triggers::TriggerMode; + +use super::rules::AttackTarget; + +const P2: PlayerId = PlayerId(2); + +// Vengeful Ancestor's punisher trigger sentence in isolation (verbatim). The +// runtime creature carries ONLY this trigger — not the Flying keyword and not the +// "enters or attacks, goad target creature" sibling — so the life delta measures the +// goaded-attack punish alone. +const VENGEFUL_GOADED_ATTACK_TRIGGER: &str = + "Whenever a goaded creature attacks, it deals 1 damage to its controller."; + +fn life_of(state: &GameState, player: PlayerId) -> i32 { + state + .players + .iter() + .find(|p| p.id == player) + .map(|p| p.life) + .expect("player exists") +} + +/// Build a 3-player game (P0 controls Vengeful Ancestor, P1 controls the Ox that +/// will attack, P2 is the goader) in P1's precombat main, ready to advance into +/// P1's declare-attackers step. When `goaded`, the Ox is designated goaded by P2. +/// +/// Goader = P2 ≠ the attacked player (P0), so goad's "attack a player other than the +/// goader if able" (CR 701.15b) is satisfied by attacking P0 — the manual attack +/// declaration is legal in both legs. +fn vengeful_runner(goaded: bool) -> (GameRunner, engine::types::identifiers::ObjectId) { + let mut scenario = GameScenario::new_n_player(3, 20); + scenario.at_phase(Phase::PreCombatMain); + scenario + .add_creature(P0, "Vengeful Ancestor", 3, 2) + .from_oracle_text(VENGEFUL_GOADED_ATTACK_TRIGGER); + let ox = scenario.add_creature(P1, "Ornery Ox", 2, 2).id(); + let mut runner = scenario.build(); + if goaded { + // CR 701.15b/c: designate the Ox as goaded by P2. A nonempty `goaded_by` set is + // exactly what `FilterProp::Goaded` reads on the LIVE object. + runner + .state_mut() + .objects + .get_mut(&ox) + .expect("Ox exists") + .goaded_by + .insert(P2); + } + // CR 508.1a: a creature attacks only on its controller's turn — hand the turn to + // P1 and advance to P1's declare-attackers step. + hand_turn_to(&mut runner, P1); + (runner, ox) +} + +/// Move the turn to `attacker` and advance to the declare-attackers step, mirroring +/// `total_war_attacking_player_scope::hand_turn_to`. Sets `active_player`, +/// `priority_player`, and `waiting_for` consistently, then passes priority until the +/// engine surfaces the declare-attackers turn-based action (CR 508.1). +fn hand_turn_to(runner: &mut GameRunner, attacker: PlayerId) { + runner.state_mut().active_player = attacker; + runner.state_mut().priority_player = attacker; + runner.state_mut().waiting_for = WaitingFor::Priority { player: attacker }; + for _ in 0..16 { + if runner.waiting_for_kind() == "DeclareAttackers" { + return; + } + runner + .act(GameAction::PassPriority) + .expect("priority pass should advance toward declare attackers"); + } + panic!("expected DeclareAttackers"); +} + +/// Reach-guard: the isolated sentence must parse to an `Attacks` trigger whose +/// `valid_card` is a `Typed` filter carrying `FilterProp::Goaded`. If the dq-d parse +/// regressed, this fails first and the runtime legs below are not vacuous. +fn assert_goaded_attacks_trigger_parses() { + let parsed = parse_oracle_text( + VENGEFUL_GOADED_ATTACK_TRIGGER, + "Vengeful Ancestor", + &[], + &[], + &[], + ); + let attacks = parsed + .triggers + .iter() + .find(|t| t.mode == TriggerMode::Attacks) + .expect("the sentence parses to an Attacks trigger"); + match attacks.valid_card.as_ref() { + Some(TargetFilter::Typed(t)) => assert!( + t.properties.contains(&FilterProp::Goaded), + "reach-guard: the Attacks trigger's valid_card must carry FilterProp::Goaded, got {:?}", + t.properties + ), + other => panic!("reach-guard: valid_card must be a Typed Goaded filter, got {other:?}"), + } +} + +/// Positive leg — a GOADED opponent creature attacks. Vengeful Ancestor's trigger +/// matches the live attacker's `goaded_by` (CR 701.15b/c), fires, and the attacker +/// deals 1 damage to its controller (P1). P1 loses exactly 1 life. +/// +/// Revert-probe: this assertion (P1: 20 → 19) FLIPS to fail if the trigger's Goaded +/// filter is evaluated against the fieldless `EventObjectSnapshot` instead of the +/// live attacker — the trigger would not fire and P1 would stay at 20. It also flips +/// if the whole `FilterProp::Goaded` parse addition is reverted (the reach-guard +/// fails first). +#[test] +fn vengeful_ancestor_goaded_attacker_loses_one_life() { + assert_goaded_attacks_trigger_parses(); + + let (mut runner, ox) = vengeful_runner(true); + let p1_before = life_of(runner.state(), P1); + assert_eq!(p1_before, 20, "precondition: P1 starts at 20 life"); + + runner + .declare_attackers(&[(ox, AttackTarget::Player(P0))]) + .expect("declaring the goaded Ox attacking P0 is legal"); + runner.advance_until_stack_empty(); + + assert_eq!( + life_of(runner.state(), P1), + 19, + "the goaded attacker's controller (P1) must lose exactly 1 life — the trigger fired \ + against the LIVE attacker's goaded_by, not a fieldless snapshot" + ); +} + +/// Negative leg (revert-probe pair) — IDENTICAL setup with the goad removed. The +/// trigger's `valid_card` Goaded filter no longer matches the (ungoaded) attacker, +/// so it does NOT fire and P1's life is unchanged. Differs from the positive leg +/// ONLY in the Ox's goaded designation, isolating the `FilterProp::Goaded` eval. +/// +/// Reach-guard (non-vacuous): the Ox still attacks and the trigger source (Vengeful +/// Ancestor) is still present — the trigger is genuinely offered and declines only +/// because the subject is not goaded, not because the attack never happened. +#[test] +fn vengeful_ancestor_ungoaded_attacker_loses_no_life() { + assert_goaded_attacks_trigger_parses(); + + let (mut runner, ox) = vengeful_runner(false); + assert!( + runner + .state() + .objects + .get(&ox) + .expect("Ox exists") + .goaded_by + .is_empty(), + "reach-guard: the Ox is genuinely ungoaded in the negative leg" + ); + assert_eq!( + life_of(runner.state(), P1), + 20, + "precondition: P1 starts at 20 life" + ); + + runner + .declare_attackers(&[(ox, AttackTarget::Player(P0))]) + .expect("declaring the ungoaded Ox attacking P0 is legal"); + runner.advance_until_stack_empty(); + + assert_eq!( + life_of(runner.state(), P1), + 20, + "an ungoaded attacker must not trigger the goaded-attack punisher — P1 loses no life" + ); +} From e3448a3c356e9ec11a134589e714b602daeeefa8 Mon Sep 17 00:00:00 2001 From: Lindsey Gray Date: Fri, 17 Jul 2026 19:42:37 -0500 Subject: [PATCH 3/4] fix(coverage): scope repeat_for parse-diff projection to the Investigate lift class (#6110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global repeat_for projection in `ability_details` surfaced a new signature key on every card carrying repeat_for, migrating ~250 parse-identical cards' coverage signatures at once. Scope it to the lift's own eligibility set — a fieldless `Effect::Investigate` with a member-count `QuantityRef` (`PlayerCount`/`ObjectCount`), mirroring `for_each_repeatable_repeat_for` — so only the intended lift class surfaces. The real `coverage-parse-diff` vs the merge-base now reports 8 cards / 10 signatures (7 real AST changes + Ethereal Investigator, a pre-existing Investigate+PlayerCount that is byte-identical), down from 250. A revert-probed `investigate_signature_exposes_repeat_for` regression (2 positive + 4 negative cases) locks the scope. Assisted-by: ClaudeCode:claude-opus-4.8 --- crates/engine/src/game/coverage.rs | 110 +++++++++++++++++++++++------ 1 file changed, 89 insertions(+), 21 deletions(-) diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 0388ec5cd8..b445992255 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -3602,12 +3602,33 @@ fn ability_details(def: &AbilityDefinition) -> Vec<(String, String)> { if let Some(dur) = &def.duration { d.push(("duration".into(), fmt_duration(dur))); } - // CR 608.2c: a lifted "[once] for each ⟨set⟩" repeat multiplier (e.g. the - // fieldless-Investigate lift) is an `AbilityDefinition` field — surface it so - // the per-card signature reflects the member-count. `None` pushes nothing, so - // cards without a `repeat_for` keep a byte-identical signature. + // CR 608.2c: a lifted "[once] for each ⟨set⟩" repeat multiplier is an + // `AbilityDefinition` field. Surface it in the per-card parse-diff signature ONLY + // for the shapes THIS PR's lift produces — a fieldless `Effect::Investigate` whose + // `repeat_for` is a member-count `QuantityRef` (`PlayerCount`/`ObjectCount`), i.e. + // exactly the eligibility set of `for_each_repeatable_repeat_for` + // (parser/oracle_effect/mod.rs). Projecting the *whole* repeat_for surface + // (CopySpell/Token/Proliferate/… and pre-existing `Fixed`/`Variable`/tracked-set + // Investigate forms) would migrate ~250 unrelated, parse-identical cards' coverage + // signatures in one shot — a deliberate global coverage-schema migration, deferred + // out of this focused feature. `None`, or any out-of-scope shape, pushes nothing, + // so those cards keep a byte-identical signature. + // COUPLING: if the lift's eligible quantity set ever widens (e.g. the Gap B + // leading-adjective fix), this scope MUST widen in lockstep, or the new lift class + // becomes false-green in the parse-diff. if let Some(rf) = &def.repeat_for { - d.push(("repeat_for".into(), fmt_quantity(rf))); + let is_lift_shape = matches!(&*def.effect, Effect::Investigate) + && matches!( + rf, + QuantityExpr::Ref { qty } + if matches!( + qty, + QuantityRef::PlayerCount { .. } | QuantityRef::ObjectCount { .. } + ) + ); + if is_lift_shape { + d.push(("repeat_for".into(), fmt_quantity(rf))); + } } if def.optional_targeting { d.push(("targeting".into(), "optional (up to)".into())); @@ -10685,33 +10706,80 @@ mod tests { #[test] fn investigate_signature_exposes_repeat_for() { - // ASK 2: a lifted "[once] for each ⟨set⟩" multiplier (e.g. the fieldless - // Investigate lift → `def.repeat_for = Some(ObjectCount/PlayerCount)`) must be - // visible in the per-card parse-diff signature. `None` adds no row so unrelated - // cards keep a byte-identical signature. Reverting the `ability_details` - // projection flips the `Some` assertion to fail. + // ASK 2 + #6110 3rd review: a lifted "[once] for each ⟨set⟩" multiplier + // (`def.repeat_for = Some(PlayerCount/ObjectCount)`) must be visible in the + // per-card parse-diff signature — but ONLY for the shapes this PR's lift + // produces (fieldless `Effect::Investigate` + a member-count `QuantityRef`). + // The projection must NOT fire for the whole pre-existing repeat_for surface + // (CopySpell/Token/Proliferate, or pre-existing `Fixed`/`Variable` Investigate + // forms), which would migrate ~250 parse-identical cards' signatures at once. use crate::types::ability::{ - AbilityDefinition, AbilityKind, QuantityExpr, QuantityRef, TargetFilter, TypedFilter, + AbilityDefinition, AbilityKind, PlayerFilter, QuantityExpr, QuantityRef, TargetFilter, + TypedFilter, }; - let detail_keys = |repeat: Option| -> Vec { - let mut def = AbilityDefinition::new(AbilityKind::Spell, Effect::Investigate); + let projects = |effect: Effect, repeat: Option| -> bool { + let mut def = AbilityDefinition::new(AbilityKind::Spell, effect); def.repeat_for = repeat; - ability_details(&def).into_iter().map(|(k, _)| k).collect() + ability_details(&def) + .into_iter() + .any(|(k, _)| k == "repeat_for") }; - let object_count = QuantityExpr::Ref { + let object_count = || QuantityExpr::Ref { qty: QuantityRef::ObjectCount { filter: TargetFilter::Typed(TypedFilter::creature()), }, }; + let player_count = || QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { + filter: PlayerFilter::OpponentLostLife, + }, + }; + + // Positive — both member-count lift shapes surface (Serene = ObjectCount, + // Teysa/Wojek = PlayerCount). Revert-probe: reverting the `ability_details` + // projection drops the row and flips both. assert!( - detail_keys(Some(object_count)) - .iter() - .any(|k| k == "repeat_for"), - "a lifted repeat_for must appear in the per-card signature", + projects(Effect::Investigate, Some(object_count())), + "Investigate + ObjectCount lift must appear in the signature", + ); + assert!( + projects(Effect::Investigate, Some(player_count())), + "Investigate + PlayerCount lift must appear in the signature", + ); + + // Negative — no repeat_for → byte-identical signature (unchanged cards). + assert!( + !projects(Effect::Investigate, None), + "an Investigate with no repeat_for must not add the row", + ); + // Negative — a `Fixed` multiplier ("investigate twice", Confirm Suspicions et + // al.) is not a member-count lift. Revert-probe: dropping the + // `QuantityExpr::Ref` guard flips this. + assert!( + !projects(Effect::Investigate, Some(QuantityExpr::Fixed { value: 2 })), + "a Fixed repeat_for must not project (not a member-count lift)", + ); + // Negative — a non-member-count `Ref` (pre-existing `Variable`/tracked-set + // Investigate forms: Disorder in the Court, Declaration in Stone) must not + // project. Revert-probe: dropping the inner `PlayerCount|ObjectCount` guard + // flips this. + assert!( + !projects( + Effect::Investigate, + Some(QuantityExpr::Ref { + qty: QuantityRef::Variable { name: "x".into() }, + }), + ), + "a non-member-count Ref repeat_for must not project", ); + // Negative (team-lead required) — the SAME member-count lift on a + // NON-Investigate effect (stand-in for the CopySpell/Token/Proliferate + // repeat_for surface) must not project. Revert-probe: dropping the + // `Effect::Investigate` guard widens the scope to the whole surface and flips + // this — this case is what locks a1. assert!( - !detail_keys(None).iter().any(|k| k == "repeat_for"), - "an ability with no repeat_for must not add the row (byte-identical signature)", + !projects(Effect::Populate, Some(object_count())), + "a non-Investigate repeat_for must not project (scope is the Investigate lift class)", ); } From 4b7c8cc3f0fe4d06fc1b153034fd35049497f4a0 Mon Sep 17 00:00:00 2001 From: Lindsey Gray Date: Fri, 17 Jul 2026 21:29:39 -0500 Subject: [PATCH 4/4] fix(engine): reclassify FilterProp::Goaded fail-closed in the event-subject reach gate The event-subject reach-gate classifier (classify_prop in types/events.rs) marked FilterProp::Goaded as Supported, asserting a goaded event-subject filter is answerable from an EventObjectSnapshot. It is not: the snapshot carries no goaded field, and the runtime zone-change-record matcher already fail-closes Goaded (game/filter.rs). The certificate was fail-open. Reclassify FilterProp::Goaded as Unsupported, aligning the reach-gate certificate with the snapshot's actual fields and the runtime's existing fail-closed behavior, so a future goaded event-subject filter fails the reach gate loudly rather than silently reading an ungoaded snapshot. Pure contract + test change with zero behavior/coverage delta: the reach gate is test-only latent scaffolding (classify_prop/classify_filter_shape have no production caller), coverage is supplied independently by coverage.rs, and the live goaded predicate (filter.rs FilterProp::Goaded => !obj.goaded_by.is_empty()) is untouched. card-data.json byte-identical before/after (4-card + full-DB). Adds a revert-probed test goaded_subject_filter_is_unsupported. Deferred follow-up (option a): snapshot goad onto EventObjectSnapshot + ZoneChangeRecord and reclassify back to Supported. CR 701.15b/c: goad is a designation on the live permanent (its goaded_by set). Assisted-by: ClaudeCode:claude-opus-4.8 --- crates/engine/src/types/events.rs | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/types/events.rs b/crates/engine/src/types/events.rs index cd53439950..a0c3b3178b 100644 --- a/crates/engine/src/types/events.rs +++ b/crates/engine/src/types/events.rs @@ -569,8 +569,6 @@ impl EventObjectSnapshot { | FilterProp::Transformed | FilterProp::Suspected | FilterProp::Renowned - // CR 701.15b/c: goad is an embedded candidate-local designation. - | FilterProp::Goaded | FilterProp::IsSaddled => Supported, // ---- embedded characteristics ---- @@ -665,7 +663,15 @@ impl EventObjectSnapshot { // ---- unsupported: needs a live candidate lookup or an unmodeled field ---- // Not reachable from the subject grammar today. Reaching one fails the gate, // which is the designed signal to extend the snapshot + evaluator together. - FilterProp::WasPlayed + // CR 701.15b/c: goad is a designation on the LIVE permanent (its `goaded_by` + // set, read by game/filter.rs `FilterProp::Goaded => !obj.goaded_by.is_empty()`). + // Neither EventObjectSnapshot nor ZoneChangeRecord carries a goaded field, and the + // runtime already fail-closes it (game/filter.rs zone-change-record matcher). + // Classify Unsupported so a future goaded event-subject filter fails the reach gate + // LOUDLY rather than silently reading an ungoaded snapshot. Deferred follow-up + // (option a): snapshot goaded onto EventObjectSnapshot + ZoneChangeRecord. + FilterProp::Goaded + | FilterProp::WasPlayed // CR 108.2 + CR 108.2b: event snapshots retain token status but not whether // a nontoken object is a copy, so card representation cannot be reconstructed. | FilterProp::RepresentedByCard @@ -1799,6 +1805,22 @@ mod tests { assert_eq!(classify(&needs_live), Unsupported); } + /// CR 701.15b/c: goad is a designation on the LIVE permanent, not a fact the event + /// snapshot / zone-change record carries — the runtime fail-closes it. The reach-gate + /// classifier must AGREE: a goaded event-subject filter is `Unsupported`, so a future + /// card that reaches it fails the gate loudly instead of silently certifying ungoaded. + /// Revert-probe: returning Goaded to the Supported group (its state on head e3448a3c3) + /// makes classify yield Supported, flipping this assertion. + #[test] + fn goaded_subject_filter_is_unsupported() { + let goaded = TargetFilter::Typed(TypedFilter { + type_filters: vec![TypeFilter::Creature], + controller: None, + properties: vec![FilterProp::Goaded], + }); + assert_eq!(classify(&goaded), Unsupported); + } + /// `Unsupported` dominates a composite: if one branch cannot be answered, the whole /// filter cannot be. Getting this backwards would let an unanswerable filter through /// the gate and be silently evaluated as `false` at runtime.