From 76ea5c339d6e065022b8be6aa78b969138ea6c4f Mon Sep 17 00:00:00 2001 From: keloide Date: Tue, 11 Aug 2026 13:01:22 +0200 Subject: [PATCH 01/14] fix(engine): support non-first spell triggers --- crates/engine/src/game/coverage.rs | 19 +- .../engine_phase_trigger_regression_tests.rs | 5 +- crates/engine/src/game/triggers.rs | 15 +- crates/engine/src/parser/oracle_trigger.rs | 159 ++++++++++++- .../engine/src/parser/oracle_trigger_tests.rs | 215 +++++++++++++++++- crates/engine/src/types/ability.rs | 19 +- .../tests/integration/ichneumon_druid.rs | 84 +++++++ .../integration/lady_loki_agent_of_chaos.rs | 2 +- crates/engine/tests/integration/main.rs | 1 + crates/mtgish-import/src/convert/trigger.rs | 6 +- .../src/features/spellslinger_prowess.rs | 12 +- 11 files changed, 508 insertions(+), 29 deletions(-) create mode 100644 crates/engine/tests/integration/ichneumon_druid.rs diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index ee446f9006..b206c1cdce 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -4168,10 +4168,21 @@ fn fmt_trigger_constraint(c: &crate::types::ability::TriggerConstraint) -> Strin TC::OncePerTurn => "once per turn".into(), TC::OncePerGame => "once per game".into(), TC::OnlyDuringYourTurn => "only during your turn".into(), - TC::NthSpellThisTurn { n, filter } => match filter { - Some(f) => format!("on your {n}th {} spell this turn", fmt_target(f)), - None => format!("on your {n}th spell this turn"), - }, + TC::NthSpellThisTurn { + n, + comparator, + filter, + } => { + let ordinal = match comparator { + Comparator::EQ => format!("your {n}th"), + Comparator::GT if *n == 1 => "after your first".to_string(), + _ => format!("when your spell count {} {n}", fmt_comparator(comparator)), + }; + match filter { + Some(f) => format!("on {ordinal} {} spell this turn", fmt_target(f)), + None => format!("on {ordinal} spell this turn"), + } + } TC::NthDrawThisTurn { n } => format!("on your {n}th draw this turn"), TC::OnlyDuringOpponentsTurn => "only during opponent's turn".into(), TC::OnlyDuringYourMainPhase => "only during your main phase".into(), diff --git a/crates/engine/src/game/engine_phase_trigger_regression_tests.rs b/crates/engine/src/game/engine_phase_trigger_regression_tests.rs index 333241a7f8..6f3835cb45 100644 --- a/crates/engine/src/game/engine_phase_trigger_regression_tests.rs +++ b/crates/engine/src/game/engine_phase_trigger_regression_tests.rs @@ -6,8 +6,8 @@ use crate::game::combat::AttackTarget; use crate::game::zones::create_object; use crate::parser::oracle::parse_oracle_text; use crate::types::ability::{ - AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, ControllerRef, Effect, - EffectScope, FilterProp, ObjectScope, PlayerFilter, QuantityExpr, QuantityRef, + AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, Comparator, ControllerRef, + Effect, EffectScope, FilterProp, ObjectScope, PlayerFilter, QuantityExpr, QuantityRef, ReplacementDefinition, ReplacementMode, ResolvedAbility, TapStateChange, TargetFilter, TargetRef, TriggerConstraint, TriggerDefinition, TypeFilter, TypedFilter, UnlessPayModifier, }; @@ -1174,6 +1174,7 @@ fn setup_esper_sentinel_unless_payment(pay_mana: bool) -> GameState { )) .constraint(TriggerConstraint::NthSpellThisTurn { n: 1, + comparator: Comparator::EQ, filter: Some(TargetFilter::Typed( TypedFilter::default() .with_type(TypeFilter::Non(Box::new(TypeFilter::Creature))), diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index 9d4975afda..e8586a8c8b 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -9539,7 +9539,11 @@ fn check_trigger_constraint_with_ref( // CR 603.2: Per-caster spell count. The caster is extracted from the SpellCast // event; the count comes from the per-player map (not the global counter). // When `filter` contains `TypeFilter::Non(Creature)`, use the noncreature counter. - TriggerConstraint::NthSpellThisTurn { n, filter } => { + TriggerConstraint::NthSpellThisTurn { + n, + comparator, + filter, + } => { let caster = match event { Some(GameEvent::SpellCast { controller: c, .. }) => *c, _ => return false, @@ -9571,7 +9575,7 @@ fn check_trigger_constraint_with_ref( }) .count() as u32, }); - count == *n + comparator.evaluate(count as i32, *n as i32) } // CR 121.2: Use the ordinal stamped onto the individual draw event // rather than the final per-turn count after a multi-card draw batch. @@ -25606,6 +25610,7 @@ pub mod tests { let mut d = make_trigger(TriggerMode::SpellCast); d.constraint = Some(TriggerConstraint::NthSpellThisTurn { n: 1, + comparator: Comparator::EQ, filter: Some(TargetFilter::Typed( TypedFilter::default().properties(vec![FilterProp::HasXInManaCost]), )), @@ -25833,7 +25838,11 @@ pub mod tests { .valid_target(TargetFilter::Typed( TypedFilter::default().controller(ControllerRef::You), )) - .constraint(TriggerConstraint::NthSpellThisTurn { n: 2, filter: None }) + .constraint(TriggerConstraint::NthSpellThisTurn { + n: 2, + comparator: Comparator::EQ, + filter: None, + }) .execute(AbilityDefinition::new( AbilityKind::Database, Effect::Draw { diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index a8cbfdcd86..c457fcec29 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -23,7 +23,7 @@ use super::oracle_ir::trigger::{ use super::oracle_modal::try_parse_inline_modal_ir; use super::oracle_nom::condition::parse_elided_subject_state_condition; use super::oracle_nom::condition::{ - parse_inner_condition, parse_there_are_battlefield_count_clause, + parse_inner_condition, parse_spell_history_filter, parse_there_are_battlefield_count_clause, }; use super::oracle_nom::condition::{parse_source_counters_exist, parse_source_has_counters}; use super::oracle_nom::error::{oracle_err, OracleResult}; @@ -2606,7 +2606,11 @@ fn parse_nth_spell_this_turn_intervening_if(input: &str) -> OracleResult<'_, Tri { return Ok(( rest, - TriggerConstraint::NthSpellThisTurn { n, filter: None }, + TriggerConstraint::NthSpellThisTurn { + n, + comparator: Comparator::EQ, + filter: None, + }, )); } @@ -2621,6 +2625,7 @@ fn parse_nth_spell_this_turn_intervening_if(input: &str) -> OracleResult<'_, Tri rest, TriggerConstraint::NthSpellThisTurn { n, + comparator: Comparator::EQ, filter: Some(filter), }, )) @@ -15890,6 +15895,18 @@ fn parse_proliferate_player_action(input: &str) -> OracleResult<'_, PlayerAction /// "whenever an opponent casts their Nth [noncreature] spell each turn" into a SpellCast /// trigger with a NthSpellThisTurn constraint. fn try_parse_nth_spell_trigger(lower: &str) -> Option<(TriggerMode, TriggerDefinition)> { + match parse_other_than_first_spell_trigger(lower) { + OtherThanFirstSpellParse::Accepted(def) => return Some((TriggerMode::SpellCast, def)), + // The marker commits this grammar: never let a malformed qualifier + // silently broaden through the generic SpellCast parser. + OtherThanFirstSpellParse::Rejected => { + return Some(( + TriggerMode::Unknown("malformed other-than-first spell trigger".to_string()), + make_base(), + )); + } + OtherThanFirstSpellParse::NotCandidate => {} + } // Branch 1: "you cast your [qualifier] spell each turn" if let Some(result) = try_parse_nth_spell_you(lower) { return Some(result); @@ -15905,6 +15922,126 @@ fn try_parse_nth_spell_trigger(lower: &str) -> Option<(TriggerMode, TriggerDefin None } +enum OtherThanFirstSpellParse { + NotCandidate, + Rejected, + Accepted(TriggerDefinition), +} + +#[derive(Clone, Copy)] +enum OtherThanFirstSpellActor { + You, + Opponent, + AnyPlayer, + EnchantedPlayer, +} + +/// CR 603.2: this event qualifier is fire-time only, unlike a CR 603.4 +/// intervening-if condition. The recognizer owns the predicate only after it +/// reaches the marker, so a malformed repeated qualifier fails closed. +fn parse_other_than_first_spell_trigger(input: &str) -> OtherThanFirstSpellParse { + let actor_parser = alt(( + value(OtherThanFirstSpellActor::You, tag("you cast an ")), + value(OtherThanFirstSpellActor::You, tag("you cast a ")), + value( + OtherThanFirstSpellActor::Opponent, + tag("an opponent casts an "), + ), + value( + OtherThanFirstSpellActor::Opponent, + tag("an opponent casts a "), + ), + value( + OtherThanFirstSpellActor::EnchantedPlayer, + tag("enchanted player casts an "), + ), + value( + OtherThanFirstSpellActor::EnchantedPlayer, + tag("enchanted player casts a "), + ), + value( + OtherThanFirstSpellActor::AnyPlayer, + tag("a player casts an "), + ), + value( + OtherThanFirstSpellActor::AnyPlayer, + tag("a player casts a "), + ), + )); + let Ok((after_actor, actor)) = actor_parser.parse(input) else { + return OtherThanFirstSpellParse::NotCandidate; + }; + let Ok((after_marker, first_qualifier)) = terminated( + take_until(" spell other than the first "), + tag::<_, _, OracleError<'_>>(" spell other than the first "), + ) + .parse(after_actor) else { + return OtherThanFirstSpellParse::NotCandidate; + }; + let Ok((after_repeated, repeated_qualifier)) = + terminated(take_until(" spell "), tag(" spell ")).parse(after_marker) + else { + return OtherThanFirstSpellParse::Rejected; + }; + let pronoun_is_valid = match actor { + OtherThanFirstSpellActor::You => { + all_consuming(tag::<_, _, OracleError<'_>>("you cast each turn")) + .parse(after_repeated) + .is_ok() + } + OtherThanFirstSpellActor::Opponent + | OtherThanFirstSpellActor::AnyPlayer + | OtherThanFirstSpellActor::EnchantedPlayer => all_consuming(alt(( + tag::<_, _, OracleError<'_>>("that player casts each turn"), + tag("they cast each turn"), + ))) + .parse(after_repeated) + .is_ok(), + }; + if !pronoun_is_valid { + return OtherThanFirstSpellParse::Rejected; + } + let parse_qualifier = |qualifier: &str| { + let qualifier = qualifier.trim(); + if qualifier.is_empty() { + Some(None) + } else { + // Shared spell-history grammar handles the same typed/color + // qualifier axis at both occurrence sites. The marker/remainder + // structure is still parsed exclusively with nom above. + parse_spell_history_filter(qualifier).map(Some) + } + }; + let Some(first_filter) = parse_qualifier(first_qualifier) else { + return OtherThanFirstSpellParse::Rejected; + }; + let Some(repeated_filter) = parse_qualifier(repeated_qualifier) else { + return OtherThanFirstSpellParse::Rejected; + }; + if first_filter != repeated_filter { + return OtherThanFirstSpellParse::Rejected; + } + let mut def = make_base(); + def.mode = TriggerMode::SpellCast; + def.valid_target = match actor { + OtherThanFirstSpellActor::You => Some(TargetFilter::Typed( + TypedFilter::default().controller(ControllerRef::You), + )), + OtherThanFirstSpellActor::Opponent => Some(TargetFilter::Typed( + TypedFilter::default().controller(ControllerRef::Opponent), + )), + OtherThanFirstSpellActor::AnyPlayer => None, + OtherThanFirstSpellActor::EnchantedPlayer => Some(TargetFilter::AttachedTo), + }; + def.valid_card = first_filter.clone(); + def.constraint = Some(TriggerConstraint::NthSpellThisTurn { + n: 1, + comparator: Comparator::GT, + filter: first_filter, + }); + OtherThanFirstSpellParse::Accepted(def) +} + /// Timing-clause kind for nth-spell/nth-draw triggers. /// CR 601.2 + CR 603.4: The trailing "each turn" / "in a turn" (unrestricted /// timing), or "during turn" (restricted to the active player @@ -16037,7 +16174,11 @@ fn try_parse_nth_spell_you(lower: &str) -> Option<(TriggerMode, TriggerDefinitio def.valid_target = Some(TargetFilter::Typed( TypedFilter::default().controller(ControllerRef::You), )); - def.constraint = Some(TriggerConstraint::NthSpellThisTurn { n, filter }); + def.constraint = Some(TriggerConstraint::NthSpellThisTurn { + n, + comparator: Comparator::EQ, + filter, + }); def.condition = timing_condition(timing); Some((TriggerMode::SpellCast, def)) } @@ -16060,7 +16201,11 @@ fn try_parse_nth_spell_opponent(lower: &str) -> Option<(TriggerMode, TriggerDefi def.valid_target = Some(TargetFilter::Typed( TypedFilter::default().controller(ControllerRef::Opponent), )); - def.constraint = Some(TriggerConstraint::NthSpellThisTurn { n, filter }); + def.constraint = Some(TriggerConstraint::NthSpellThisTurn { + n, + comparator: Comparator::EQ, + filter, + }); def.condition = timing_condition(timing); Some((TriggerMode::SpellCast, def)) } @@ -16083,7 +16228,11 @@ fn try_parse_nth_spell_any_player(lower: &str) -> Option<(TriggerMode, TriggerDe let filter = extract_spell_type_filter(rest); let mut def = make_base(); def.mode = TriggerMode::SpellCast; - def.constraint = Some(TriggerConstraint::NthSpellThisTurn { n, filter }); + def.constraint = Some(TriggerConstraint::NthSpellThisTurn { + n, + comparator: Comparator::EQ, + filter, + }); def.condition = timing_condition(timing); Some((TriggerMode::SpellCast, def)) } diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index dbadaab783..addbedb033 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -11213,7 +11213,174 @@ fn trigger_nth_spell_second() { assert_eq!(def.mode, TriggerMode::SpellCast); assert_eq!( def.constraint, - Some(TriggerConstraint::NthSpellThisTurn { n: 2, filter: None }) + Some(TriggerConstraint::NthSpellThisTurn { + n: 2, + comparator: Comparator::EQ, + filter: None + }) + ); +} + +#[test] +fn trigger_other_than_first_instant_spell_is_a_fire_time_constraint() { + let def = parse_trigger_line( + "Whenever an opponent casts an instant spell other than the first instant spell that player casts each turn, this creature deals 4 damage to that player.", + "Ichneumon Druid", + ); + let instant = TargetFilter::Typed(TypedFilter::new(TypeFilter::Instant)); + assert_eq!(def.mode, TriggerMode::SpellCast); + assert_eq!( + def.valid_target, + Some(TargetFilter::Typed( + TypedFilter::default().controller(ControllerRef::Opponent) + )) + ); + assert_eq!(def.valid_card, Some(instant.clone())); + assert_eq!( + def.constraint, + Some(TriggerConstraint::NthSpellThisTurn { + n: 1, + comparator: Comparator::GT, + filter: Some(instant), + }) + ); + assert_eq!( + def.condition, None, + "a fire-time qualifier is not intervening-if" + ); + assert!(def.execute.is_some(), "the payoff must remain reachable"); +} + +#[test] +fn trigger_ordinary_opponent_spell_cast_remains_generic_spellcast() { + let def = parse_trigger_line( + "Whenever an opponent casts an instant spell, draw a card.", + "Ordinary opponent fixture", + ); + assert_eq!(def.mode, TriggerMode::SpellCast); + assert_eq!( + def.constraint, None, + "no nonfirst marker means no constraint" + ); + assert_eq!( + def.valid_target, + Some(TargetFilter::Typed( + TypedFilter::default().controller(ControllerRef::Opponent) + )) + ); + assert_eq!( + def.valid_card, + Some(TargetFilter::Typed(TypedFilter::new(TypeFilter::Instant))) + ); +} + +#[test] +fn other_than_first_classifier_preserves_not_candidate_and_rejected_states() { + assert!(matches!( + parse_other_than_first_spell_trigger("an opponent casts an instant spell"), + OtherThanFirstSpellParse::NotCandidate + )); + assert!(matches!( + parse_other_than_first_spell_trigger( + "enchanted player casts a spell other than the first spell they cast each turn or copies a spell" + ), + OtherThanFirstSpellParse::Rejected + )); +} + +#[test] +fn trigger_other_than_first_supports_you_and_any_player_actors() { + let you = parse_trigger_line( + "Whenever you cast an instant spell other than the first instant spell you cast each turn, draw a card.", + "Self nonfirst fixture", + ); + assert_eq!(you.mode, TriggerMode::SpellCast); + assert_eq!( + you.valid_target, + Some(TargetFilter::Typed( + TypedFilter::default().controller(ControllerRef::You) + )) + ); + assert!(matches!( + you.constraint, + Some(TriggerConstraint::NthSpellThisTurn { + n: 1, + comparator: Comparator::GT, + .. + }) + )); + + let any = parse_trigger_line( + "Whenever a player casts a creature spell other than the first creature spell that player casts each turn, draw a card.", + "Any player nonfirst fixture", + ); + assert_eq!(any.mode, TriggerMode::SpellCast); + assert_eq!(any.valid_target, None); + assert!(matches!( + any.constraint, + Some(TriggerConstraint::NthSpellThisTurn { + n: 1, + comparator: Comparator::GT, + filter: Some(TargetFilter::Typed(TypedFilter { ref type_filters, .. })), + }) if type_filters == &vec![TypeFilter::Creature] + )); +} + +#[test] +fn trigger_nonfirst_curse_with_or_copies_tail_fails_closed() { + let def = parse_trigger_line( + "Whenever enchanted player casts a spell other than the first spell they cast each turn or copies a spell, Curse of Shaken Faith deals 2 damage to that player.", + "Curse of Shaken Faith", + ); + assert!( + matches!(def.mode, TriggerMode::Unknown(_)), + "a marker-present unsupported disjunction must not become SpellCast: {def:?}" + ); +} + +#[test] +fn trigger_other_than_first_mismatched_spell_filter_is_unknown() { + let def = parse_trigger_line( + "Whenever an opponent casts an instant spell other than the first sorcery spell that player casts each turn, draw a card.", + "Malformed nonfirst fixture", + ); + assert!( + matches!(def.mode, TriggerMode::Unknown(_)), + "mismatched filters must not reach generic SpellCast parsing: {def:?}" + ); +} + +#[test] +fn nth_spell_constraint_serde_defaults_and_omits_exact_comparator() { + let old = r#"{\"type\":\"NthSpellThisTurn\",\"n\":2}"#; + let decoded: TriggerConstraint = serde_json::from_str(old).expect("legacy export loads"); + assert_eq!( + decoded, + TriggerConstraint::NthSpellThisTurn { + n: 2, + comparator: Comparator::EQ, + filter: None + } + ); + let json = serde_json::to_string(&decoded).expect("exact ordinal serializes"); + assert!( + !json.contains("comparator"), + "default must preserve export shape: {json}" + ); + + let gt = TriggerConstraint::NthSpellThisTurn { + n: 1, + comparator: Comparator::GT, + filter: Some(TargetFilter::Typed(TypedFilter::new(TypeFilter::Instant))), + }; + let gt_json = serde_json::to_string(>).expect("GT ordinal serializes"); + assert!( + gt_json.contains("\"comparator\":\"GT\""), + "non-default comparator must be explicit: {gt_json}" + ); + assert_eq!( + serde_json::from_str::(>_json).expect("GT round-trips"), + gt ); } @@ -11230,6 +11397,7 @@ fn trigger_nth_spell_with_filter_constrains_triggering_spell() { def.constraint, Some(TriggerConstraint::NthSpellThisTurn { n: 2, + comparator: Comparator::EQ, filter: Some(filter), }) ); @@ -11248,6 +11416,7 @@ fn trigger_vengevine_intervening_if_maps_to_nth_creature_spell_constraint() { def.constraint, Some(TriggerConstraint::NthSpellThisTurn { n: 2, + comparator: Comparator::EQ, filter: Some(filter), }) ); @@ -11409,7 +11578,11 @@ fn trigger_nth_spell_third() { assert_eq!(def.mode, TriggerMode::SpellCast); assert_eq!( def.constraint, - Some(TriggerConstraint::NthSpellThisTurn { n: 3, filter: None }) + Some(TriggerConstraint::NthSpellThisTurn { + n: 3, + comparator: Comparator::EQ, + filter: None + }) ); } @@ -11544,7 +11717,11 @@ fn trigger_nth_spell_any_player_during_their_turn() { assert_eq!(def.valid_target, None); assert_eq!( def.constraint, - Some(TriggerConstraint::NthSpellThisTurn { n: 2, filter: None }) + Some(TriggerConstraint::NthSpellThisTurn { + n: 2, + comparator: Comparator::EQ, + filter: None + }) ); assert_eq!( def.condition, @@ -11567,7 +11744,11 @@ fn trigger_nth_spell_any_player_each_turn_no_condition() { assert_eq!(def.mode, TriggerMode::SpellCast); assert_eq!( def.constraint, - Some(TriggerConstraint::NthSpellThisTurn { n: 2, filter: None }) + Some(TriggerConstraint::NthSpellThisTurn { + n: 2, + comparator: Comparator::EQ, + filter: None + }) ); assert_eq!(def.condition, None); } @@ -18431,7 +18612,11 @@ fn trigger_first_spell_opponents_turn() { assert_eq!(def.mode, TriggerMode::SpellCast); assert_eq!( def.constraint, - Some(TriggerConstraint::NthSpellThisTurn { n: 1, filter: None }) + Some(TriggerConstraint::NthSpellThisTurn { + n: 1, + comparator: Comparator::EQ, + filter: None + }) ); assert_eq!( def.condition, @@ -18453,7 +18638,11 @@ fn trigger_first_spell_during_each_of_your_turns() { assert_eq!(def.mode, TriggerMode::SpellCast); assert_eq!( def.constraint, - Some(TriggerConstraint::NthSpellThisTurn { n: 1, filter: None }) + Some(TriggerConstraint::NthSpellThisTurn { + n: 1, + comparator: Comparator::EQ, + filter: None + }) ); assert_eq!( def.condition, @@ -18494,6 +18683,7 @@ fn trigger_first_spell_with_x_in_cost() { def.constraint, Some(TriggerConstraint::NthSpellThisTurn { n: 1, + comparator: Comparator::EQ, filter: Some(expected_filter), }), "first-spell-with-X trigger must carry HasXInManaCost filter" @@ -18512,7 +18702,12 @@ fn trigger_first_creature_spell_with_x_in_cost() { "Hypothetical", ); assert_eq!(def.mode, TriggerMode::SpellCast); - let TriggerConstraint::NthSpellThisTurn { n, ref filter } = def.constraint.unwrap() else { + let TriggerConstraint::NthSpellThisTurn { + n, + comparator: Comparator::EQ, + ref filter, + } = def.constraint.unwrap() + else { panic!("expected NthSpellThisTurn"); }; assert_eq!(n, 1); @@ -18551,7 +18746,11 @@ fn trigger_first_spell_no_qualifier_remains_none() { assert_eq!(def.mode, TriggerMode::SpellCast); assert_eq!( def.constraint, - Some(TriggerConstraint::NthSpellThisTurn { n: 1, filter: None }) + Some(TriggerConstraint::NthSpellThisTurn { + n: 1, + comparator: Comparator::EQ, + filter: None + }) ); } diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 0f45b9afed..4472122f34 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -21465,6 +21465,16 @@ pub enum ReplacementCondition { Unrecognized { text: String }, } +/// The historical wire shape of `NthSpellThisTurn` represented an exact ordinal. +/// Keep that meaning when a persisted export has no comparator field. +fn default_nth_spell_comparator() -> Comparator { + Comparator::EQ +} + +fn is_default_nth_spell_comparator(comparator: &Comparator) -> bool { + *comparator == Comparator::EQ +} + /// Rate-limiting constraint for triggered abilities. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "type")] @@ -21476,11 +21486,18 @@ pub enum TriggerConstraint { /// "This ability triggers only during your turn." OnlyDuringYourTurn, /// "Whenever you/an opponent casts your/their Nth [qualifier] spell each turn" — - /// fires exactly when the caster's per-player spell count equals `n`. + /// fires when the caster's per-player spell count satisfies `comparator` + /// against `n`. `EQ` preserves the existing exact-ordinal behavior; `GT` + /// represents "other than the first ... spell" fire-time qualifiers. /// When `filter` is `Some`, only spells matching the filter are counted /// (e.g., `TypeFilter::Non(Creature)` for "noncreature spell"). NthSpellThisTurn { n: u32, + #[serde( + default = "default_nth_spell_comparator", + skip_serializing_if = "is_default_nth_spell_comparator" + )] + comparator: Comparator, #[serde(default, skip_serializing_if = "Option::is_none")] filter: Option, }, diff --git a/crates/engine/tests/integration/ichneumon_druid.rs b/crates/engine/tests/integration/ichneumon_druid.rs new file mode 100644 index 0000000000..938f33e49a --- /dev/null +++ b/crates/engine/tests/integration/ichneumon_druid.rs @@ -0,0 +1,84 @@ +//! Runtime regression for Ichneumon Druid's non-first instant-spell trigger. + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::mana::{ManaType, ManaUnit}; +use engine::types::phase::Phase; + +const ICHNEUMON_DRUID: &str = "Whenever an opponent casts an instant spell other than the first instant spell that player casts each turn, this creature deals 4 damage to that player."; + +/// CR 603.2: this is a fire-time event qualifier, not a CR 603.4 +/// intervening-if. A noninstant between the first and second instant must not +/// increment the instant-only history. +#[test] +fn ichneumon_druid_damages_only_after_opponents_first_instant() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.add_creature_from_oracle(P0, "Ichneumon Druid", 1, 1, ICHNEUMON_DRUID); + let own_instant = scenario.add_bolt_to_hand(P0); + let first_instant = scenario.add_bolt_to_hand(P1); + let noninstant = scenario + .add_creature_to_hand_from_oracle(P1, "Ordinary Bear", 1, 1, "") + .with_mana_cost(engine::types::mana::ManaCost::generic(0)) + .id(); + let second_instant = scenario.add_bolt_to_hand(P1); + let third_instant = scenario.add_bolt_to_hand(P1); + let target_a = scenario.add_creature(P0, "Target A", 0, 8).id(); + let target_b = scenario.add_creature(P0, "Target B", 0, 8).id(); + let target_c = scenario.add_creature(P0, "Target C", 0, 8).id(); + let own_target = scenario.add_creature(P1, "Own-Cast Target", 0, 8).id(); + let mana = ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![]); + scenario.with_mana_pool(P1, vec![mana.clone(), mana.clone(), mana]); + scenario.with_mana_pool( + P0, + vec![ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![])], + ); + let mut runner = scenario.build(); + runner.state_mut().active_player = P1; + runner.state_mut().priority_player = P1; + runner.state_mut().waiting_for = WaitingFor::Priority { player: P1 }; + + let initial_life = runner.life(P1); + let controller_life = runner.life(P0); + // Source/controller and caster deliberately diverge: the source's own + // instant is not an opponent event and must not damage its controller. + runner.cast(own_instant).target_object(own_target).resolve(); + assert_eq!( + runner.life(P0), + controller_life, + "controller's own instant must not trigger" + ); + assert_eq!( + runner.life(P1), + initial_life, + "own instant must not damage opponent either" + ); + runner.cast(first_instant).target_object(target_a).resolve(); + assert_eq!( + runner.life(P1), + initial_life, + "first instant must not trigger" + ); + runner.cast(noninstant).resolve(); + assert_eq!( + runner.life(P1), + initial_life, + "noninstant must not increment instant history" + ); + runner + .cast(second_instant) + .target_object(target_b) + .resolve(); + assert_eq!( + runner.life(P1), + initial_life - 4, + "second instant must trigger once" + ); + runner.cast(third_instant).target_object(target_c).resolve(); + assert_eq!( + runner.life(P1), + initial_life - 8, + "every later instant must trigger" + ); +} diff --git a/crates/engine/tests/integration/lady_loki_agent_of_chaos.rs b/crates/engine/tests/integration/lady_loki_agent_of_chaos.rs index 669f8ccbee..cee4d4ac48 100644 --- a/crates/engine/tests/integration/lady_loki_agent_of_chaos.rs +++ b/crates/engine/tests/integration/lady_loki_agent_of_chaos.rs @@ -277,7 +277,7 @@ fn lady_loki_parses_nested_payoff_chain() { // Trigger constraint: first instant/sorcery/Villain spell each turn. match trigger.constraint.as_ref() { - Some(TriggerConstraint::NthSpellThisTurn { n, filter }) => { + Some(TriggerConstraint::NthSpellThisTurn { n, filter, .. }) => { assert_eq!(*n, 1, "fires on the FIRST matching spell each turn"); let Some(TargetFilter::Or { filters }) = filter else { panic!("expected an Or filter, got {filter:?}"); diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 951a41254b..5fefabd63d 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -277,6 +277,7 @@ mod hogaak_cant_spend_mana_1095; mod hollow_one_cost_reduction; mod hunters_insight_combat_draw; mod inevitable_betrayal_no_mana_cost; +mod ichneumon_druid; mod infantry_shield_mobilize_grant; mod inspiring_call_indestructible_grant; mod integration_adventure; diff --git a/crates/mtgish-import/src/convert/trigger.rs b/crates/mtgish-import/src/convert/trigger.rs index ea3e5382b8..d1dc6aa01f 100644 --- a/crates/mtgish-import/src/convert/trigger.rs +++ b/crates/mtgish-import/src/convert/trigger.rs @@ -6,7 +6,9 @@ //! the highest-frequency are mapped here; the long tail fails strict and //! shows up in the report. -use engine::types::ability::{CounterTriggerFilter, DamageKindFilter, TriggerConstraint}; +use engine::types::ability::{ + Comparator, CounterTriggerFilter, DamageKindFilter, TriggerConstraint, +}; use engine::types::triggers::{AttackTargetFilter, PlaneswalkRole, TriggerMode}; use engine::types::{Phase, TargetFilter, TriggerCondition, TriggerDefinition, TypedFilter, Zone}; @@ -654,7 +656,7 @@ pub fn convert(t: &Trigger) -> ConvResult { def.valid_target = Some(TargetFilter::Typed( TypedFilter::default().controller(controller), )); - def.constraint = Some(TriggerConstraint::NthSpellThisTurn { n, filter }); + def.constraint = Some(TriggerConstraint::NthSpellThisTurn { n, comparator: Comparator::EQ, filter }); def } diff --git a/crates/phase-ai/src/features/spellslinger_prowess.rs b/crates/phase-ai/src/features/spellslinger_prowess.rs index 43dee981f2..40bce2855d 100644 --- a/crates/phase-ai/src/features/spellslinger_prowess.rs +++ b/crates/phase-ai/src/features/spellslinger_prowess.rs @@ -8,8 +8,10 @@ //! - `Keyword::Prowess` at `keywords.rs:311`. CR 702.108a: prowess triggered ability. //! - `TriggerMode::SpellCast` / `SpellCastOrCopy` / `SpellAbilityCast` / //! `SpellAbilityCopy` at `triggers.rs:50-57`. CR 601.2i (cast) + CR 707.10 (copy). -//! - `TriggerConstraint::NthSpellThisTurn { n, filter }` at `ability.rs:4484`. -//! CR 603.4: intervening-if clause. CR 603.1: triggered abilities. +//! - `TriggerConstraint::NthSpellThisTurn { n, comparator, filter }` at +//! `ability.rs`. This is a CR 603.2 fire-time trigger-event constraint; +//! `Comparator::EQ` represents an exact ordinal and `GT` represents the +//! non-first-spell class. It is not a CR 603.4 intervening-if clause. //! - `TriggerDefinition.valid_card: Option` at `ability.rs:4522`. //! - `TriggerDefinition.valid_target: Option` at `ability.rs:4539`. //! - `TriggerDefinition.constraint: Option` at `ability.rs:4545`. @@ -601,7 +603,11 @@ mod tests { // SpellCast + NthSpellThisTurn. CR 603.4. let mut c = creature_face("Spectral Sailor Shape"); let mut t = TriggerDefinition::new(TriggerMode::SpellCast); - t.constraint = Some(TriggerConstraint::NthSpellThisTurn { n: 2, filter: None }); + t.constraint = Some(TriggerConstraint::NthSpellThisTurn { + n: 2, + comparator: engine::types::ability::Comparator::EQ, + filter: None, + }); c.triggers.push(t); let f = detect(&[entry(c, 2)]); assert_eq!(f.nth_spell_payoff_count, 2); From 0e2d2deb1f0527c06609a90c6447877d83096db4 Mon Sep 17 00:00:00 2001 From: keloide Date: Tue, 11 Aug 2026 13:03:41 +0200 Subject: [PATCH 02/14] test(engine): cover non-first trigger compatibility --- crates/engine/src/parser/oracle_trigger_tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index addbedb033..278e455047 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -11352,7 +11352,7 @@ fn trigger_other_than_first_mismatched_spell_filter_is_unknown() { #[test] fn nth_spell_constraint_serde_defaults_and_omits_exact_comparator() { - let old = r#"{\"type\":\"NthSpellThisTurn\",\"n\":2}"#; + let old = r#"{"type":"NthSpellThisTurn","n":2}"#; let decoded: TriggerConstraint = serde_json::from_str(old).expect("legacy export loads"); assert_eq!( decoded, @@ -11936,6 +11936,7 @@ fn trigger_nth_spell_opponent_noncreature() { def.constraint, Some(TriggerConstraint::NthSpellThisTurn { n: 1, + comparator: Comparator::EQ, filter: Some(TargetFilter::Typed(TypedFilter { type_filters: vec![TypeFilter::Non(Box::new(TypeFilter::Creature))], controller: None, From 7be4f80788bf8f84205f5d7d772919101e62a496 Mon Sep 17 00:00:00 2001 From: keloide Date: Tue, 11 Aug 2026 13:06:56 +0200 Subject: [PATCH 03/14] fix(parser): handle untyped non-first spell triggers --- crates/engine/src/parser/oracle_trigger.rs | 8 +- .../engine/src/parser/oracle_trigger_tests.rs | 15 ++++ .../tests/integration/curse_misc_triggers.rs | 80 +------------------ 3 files changed, 21 insertions(+), 82 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index c457fcec29..c475bd8b76 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -15978,9 +15978,11 @@ fn parse_other_than_first_spell_trigger(input: &str) -> OtherThanFirstSpellParse .parse(after_actor) else { return OtherThanFirstSpellParse::NotCandidate; }; - let Ok((after_repeated, repeated_qualifier)) = - terminated(take_until(" spell "), tag(" spell ")).parse(after_marker) - else { + let Ok((after_repeated, repeated_qualifier)) = alt(( + terminated(take_until(" spell "), tag(" spell ")), + value("", tag::<_, _, OracleError<'_>>("spell ")), + )) + .parse(after_marker) else { return OtherThanFirstSpellParse::Rejected; }; let pronoun_is_valid = match actor { diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 278e455047..9b4cafb231 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -11324,6 +11324,21 @@ fn trigger_other_than_first_supports_you_and_any_player_actors() { filter: Some(TargetFilter::Typed(TypedFilter { ref type_filters, .. })), }) if type_filters == &vec![TypeFilter::Creature] )); + + let untyped = parse_trigger_line( + "Whenever a player casts a spell other than the first spell that player casts each turn, draw a card.", + "Untyped nonfirst fixture", + ); + assert_eq!(untyped.mode, TriggerMode::SpellCast); + assert_eq!(untyped.valid_target, None); + assert_eq!( + untyped.constraint, + Some(TriggerConstraint::NthSpellThisTurn { + n: 1, + comparator: Comparator::GT, + filter: None, + }) + ); } #[test] diff --git a/crates/engine/tests/integration/curse_misc_triggers.rs b/crates/engine/tests/integration/curse_misc_triggers.rs index dcbf7190d4..eea7f69a45 100644 --- a/crates/engine/tests/integration/curse_misc_triggers.rs +++ b/crates/engine/tests/integration/curse_misc_triggers.rs @@ -1,10 +1,9 @@ //! Integration tests for curse cards with miscellaneous trigger patterns. //! -//! Covers 5 curses with varied trigger conditions: +//! Covers 4 curses with varied trigger conditions: //! - Curse of Clinging Webs (creature enchanted player controls dies → exile + Spider) //! - Curse of Fool's Wisdom (enchanted player draws → deal 2 + gain 2) //! - Curse of Obsession (draw step: draw 2 extra; end step: discard hand) -//! - Curse of Shaken Faith (2nd+ spell each turn → deal 2 damage) //! - Fraying Sanity (end step: mill X = cards put into graveyard this turn) //! //! Each test verifies at minimum that the trigger fires. For simpler cards, @@ -38,9 +37,6 @@ const CURSE_OF_OBSESSION: &str = "At the beginning of enchanted player's draw step, that player draws two additional cards.\n\ At the beginning of enchanted player's end step, that player discards their hand."; -const CURSE_OF_SHAKEN_FAITH: &str = - "Whenever enchanted player casts a spell other than the first spell they cast each turn, Curse of Shaken Faith deals 2 damage to that player."; - const FRAYING_SANITY: &str = "At the beginning of each end step, enchanted player mills X cards, where X is the number of cards put into their graveyard from anywhere this turn."; @@ -264,80 +260,6 @@ fn curse_of_obsession_fires_at_draw_step() { ); } -/// Curse of Shaken Faith: trigger fires on the 2nd spell cast by enchanted player. -#[test] -fn curse_of_shaken_faith_fires_on_second_spell() { - let mut scenario = GameScenario::new(); - scenario.at_phase(Phase::PreCombatMain); - - let curse_id = { - let mut builder = scenario.add_creature_from_oracle( - P0, - "Curse of Shaken Faith", - 0, - 0, - CURSE_OF_SHAKEN_FAITH, - ); - builder.as_enchantment(); - builder.with_subtypes(vec!["Aura", "Curse"]); - builder.id() - }; - - // P1 needs two spells to cast and mana. - let spell_1 = scenario.add_bolt_to_hand(P1); - let spell_2 = scenario.add_bolt_to_hand(P1); - - // Targets for the bolts. - let dummy1 = scenario.add_creature(P0, "Memnite", 1, 1).id(); - let dummy2 = scenario.add_creature(P0, "Ornithopter", 0, 2).id(); - - // Mana for P1. - let mana_unit = engine::types::mana::ManaUnit::new( - engine::types::mana::ManaType::Red, - ObjectId(0), - false, - vec![], - ); - scenario.with_mana_pool(P1, vec![mana_unit.clone(), mana_unit]); - - // Library padding. - for _ in 0..10 { - scenario.add_card_to_library_top(P0, "Plains"); - scenario.add_card_to_library_top(P1, "Plains"); - } - - let mut runner = scenario.build(); - runner.state_mut().active_player = P1; - runner.state_mut().priority_player = P1; - runner.state_mut().waiting_for = engine::types::game_state::WaitingFor::Priority { player: P1 }; - - attach_to_player(runner.state_mut(), curse_id, P1); - evaluate_layers(runner.state_mut()); - reindex_object_triggers(runner.state_mut(), curse_id); - - let _life_before = runner.life(P1); - - // Cast first spell — should NOT trigger Curse of Shaken Faith. - runner.cast(spell_1).target_object(dummy1).resolve(); - - let life_after_first = runner.life(P1); - // First spell should not cause life loss from the curse. - // (Bolt deals 3 to a creature, not to P1.) - - // Cast second spell — SHOULD trigger Curse of Shaken Faith (2 damage to P1). - runner.cast(spell_2).target_object(dummy2).resolve(); - - let life_after_second = runner.life(P1); - - // P1 should have lost 2 life from Curse of Shaken Faith on the second cast. - assert!( - life_after_second <= life_after_first - 2, - "Curse of Shaken Faith must deal 2 damage on the second spell cast (life: {} → {})", - life_after_first, - life_after_second - ); -} - /// Fraying Sanity (issue #5947): at each end step, enchanted player mills X /// cards where X is the number of cards put into their graveyard from anywhere /// this turn. Seed N owned GY puts, advance to the end step, and assert the From 26463a76842446c2476034e8a8195e088c7a52dd Mon Sep 17 00:00:00 2001 From: keloide Date: Tue, 11 Aug 2026 13:08:41 +0200 Subject: [PATCH 04/14] fix(parser): parse bare non-first spell qualifiers --- crates/engine/src/parser/oracle_trigger.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index c475bd8b76..45fcc2a4e7 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -15971,10 +15971,13 @@ fn parse_other_than_first_spell_trigger(input: &str) -> OtherThanFirstSpellParse let Ok((after_actor, actor)) = actor_parser.parse(input) else { return OtherThanFirstSpellParse::NotCandidate; }; - let Ok((after_marker, first_qualifier)) = terminated( - take_until(" spell other than the first "), - tag::<_, _, OracleError<'_>>(" spell other than the first "), - ) + let Ok((after_marker, first_qualifier)) = alt(( + terminated( + take_until(" spell other than the first "), + tag::<_, _, OracleError<'_>>(" spell other than the first "), + ), + value("", tag("spell other than the first ")), + )) .parse(after_actor) else { return OtherThanFirstSpellParse::NotCandidate; }; From 0693935e3829a956156239e82f5d264a82cad763 Mon Sep 17 00:00:00 2001 From: keloide Date: Tue, 11 Aug 2026 13:12:04 +0200 Subject: [PATCH 05/14] test(engine): exercise non-first trigger actors --- crates/engine/src/parser/oracle_trigger.rs | 2 +- crates/engine/tests/integration/ichneumon_druid.rs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 45fcc2a4e7..0230e53044 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -15940,7 +15940,7 @@ enum OtherThanFirstSpellActor { /// intervening-if condition. The recognizer owns the predicate only after it /// reaches the marker, so a malformed repeated qualifier fails closed. fn parse_other_than_first_spell_trigger(input: &str) -> OtherThanFirstSpellParse { - let actor_parser = alt(( + let mut actor_parser = alt(( value(OtherThanFirstSpellActor::You, tag("you cast an ")), value(OtherThanFirstSpellActor::You, tag("you cast a ")), value( diff --git a/crates/engine/tests/integration/ichneumon_druid.rs b/crates/engine/tests/integration/ichneumon_druid.rs index 938f33e49a..465264d7aa 100644 --- a/crates/engine/tests/integration/ichneumon_druid.rs +++ b/crates/engine/tests/integration/ichneumon_druid.rs @@ -39,10 +39,12 @@ fn ichneumon_druid_damages_only_after_opponents_first_instant() { runner.state_mut().priority_player = P1; runner.state_mut().waiting_for = WaitingFor::Priority { player: P1 }; - let initial_life = runner.life(P1); let controller_life = runner.life(P0); + let initial_life = runner.life(P1); // Source/controller and caster deliberately diverge: the source's own // instant is not an opponent event and must not damage its controller. + runner.state_mut().priority_player = P0; + runner.state_mut().waiting_for = WaitingFor::Priority { player: P0 }; runner.cast(own_instant).target_object(own_target).resolve(); assert_eq!( runner.life(P0), @@ -54,6 +56,8 @@ fn ichneumon_druid_damages_only_after_opponents_first_instant() { initial_life, "own instant must not damage opponent either" ); + runner.state_mut().priority_player = P1; + runner.state_mut().waiting_for = WaitingFor::Priority { player: P1 }; runner.cast(first_instant).target_object(target_a).resolve(); assert_eq!( runner.life(P1), From 0e17ca27dd1d0f56f74e58b1a3ca1ed100261079 Mon Sep 17 00:00:00 2001 From: keloide Date: Tue, 11 Aug 2026 13:15:05 +0200 Subject: [PATCH 06/14] docs(ai): clarify nth-spell trigger timing --- crates/phase-ai/src/features/spellslinger_prowess.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/phase-ai/src/features/spellslinger_prowess.rs b/crates/phase-ai/src/features/spellslinger_prowess.rs index 40bce2855d..c5f9eda918 100644 --- a/crates/phase-ai/src/features/spellslinger_prowess.rs +++ b/crates/phase-ai/src/features/spellslinger_prowess.rs @@ -71,7 +71,8 @@ pub struct SpellslingerProwessFeature { /// with a valid_card filter that permits Instant/Sorcery (or unset). /// CR 601.2i + CR 603.1. Includes magecraft-shaped triggers. pub cast_payoff_count: u32, - /// Cast triggers with `TriggerConstraint::NthSpellThisTurn`. CR 603.4. + /// Cast triggers with the fire-time `TriggerConstraint::NthSpellThisTurn`. + /// CR 603.2. pub nth_spell_payoff_count: u32, /// `AbilityKind::Spell` abilities whose chain contains `Effect::CopySpell`. /// CR 707.10: to copy a spell means to put a copy onto the stack. @@ -600,7 +601,7 @@ mod tests { #[test] fn detects_nth_spell_payoff() { - // SpellCast + NthSpellThisTurn. CR 603.4. + // SpellCast + fire-time NthSpellThisTurn. CR 603.2. let mut c = creature_face("Spectral Sailor Shape"); let mut t = TriggerDefinition::new(TriggerMode::SpellCast); t.constraint = Some(TriggerConstraint::NthSpellThisTurn { From 7924729f8e7d8514deb47fe173878f886a1a79d7 Mon Sep 17 00:00:00 2001 From: keloide Date: Tue, 11 Aug 2026 13:20:22 +0200 Subject: [PATCH 07/14] fix(engine): render non-first trigger coverage --- crates/engine/src/game/coverage.rs | 36 +++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index b206c1cdce..1432ea5c8e 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -4173,14 +4173,14 @@ fn fmt_trigger_constraint(c: &crate::types::ability::TriggerConstraint) -> Strin comparator, filter, } => { - let ordinal = match comparator { - Comparator::EQ => format!("your {n}th"), + let timing = match comparator { + Comparator::EQ => format!("on your {n}th"), Comparator::GT if *n == 1 => "after your first".to_string(), _ => format!("when your spell count {} {n}", fmt_comparator(comparator)), }; match filter { - Some(f) => format!("on {ordinal} {} spell this turn", fmt_target(f)), - None => format!("on {ordinal} spell this turn"), + Some(f) => format!("{timing} {} spell this turn", fmt_target(f)), + None => format!("{timing} spell this turn"), } } TC::NthDrawThisTurn { n } => format!("on your {n}th draw this turn"), @@ -11325,10 +11325,10 @@ mod tests { use crate::database::legality::{legalities_to_export_map, LegalityStatus}; use crate::parser::oracle_ir::diagnostic::{CascadeSlot, OracleDiagnostic}; use crate::types::ability::{ - AbilityCondition, AbilityKind, ContinuousModification, ControllerRef, CounterTransferMode, - DieResultBranch, Effect, PileSource, PlayerFilter, PlayerScope, PreventionAmount, - PreventionScope, ReplacementCondition, StaticDefinition, TargetFilter, VoteTally, - VoteVisibility, VoterScope, + AbilityCondition, AbilityKind, Comparator, ContinuousModification, ControllerRef, + CounterTransferMode, DieResultBranch, Effect, PileSource, PlayerFilter, PlayerScope, + PreventionAmount, PreventionScope, ReplacementCondition, StaticDefinition, TargetFilter, + TriggerConstraint, VoteTally, VoteVisibility, VoterScope, }; use crate::types::card_type::CardType; use crate::types::identifiers::{CardId, ObjectId}; @@ -11338,6 +11338,26 @@ mod tests { use crate::types::statics::{BlockExceptionKind, ProhibitionScope}; use crate::types::zones::{EtbTapState, Zone}; + #[test] + fn nonfirst_spell_constraint_has_grammatical_coverage_detail() { + assert_eq!( + fmt_trigger_constraint(&TriggerConstraint::NthSpellThisTurn { + n: 1, + comparator: Comparator::GT, + filter: None, + }), + "after your first spell this turn" + ); + assert_eq!( + fmt_trigger_constraint(&TriggerConstraint::NthSpellThisTurn { + n: 2, + comparator: Comparator::EQ, + filter: None, + }), + "on your 2th spell this turn" + ); + } + #[test] fn change_zone_signature_exposes_enters_attacking() { // #5495: a parser change flipping `enters_attacking` (e.g. teaching From 3dd1a31bc82baaa5086e31a25733b96abcaba2b6 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 05:07:01 -0700 Subject: [PATCH 08/14] fix(PR-7233): restore parser type inference Co-authored-by: keloide <75585494+keloide@users.noreply.github.com> --- crates/engine/src/parser/oracle_trigger.rs | 5 ++++- crates/engine/tests/integration/main.rs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 0230e53044..51a375ab15 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -15941,7 +15941,10 @@ enum OtherThanFirstSpellActor { /// reaches the marker, so a malformed repeated qualifier fails closed. fn parse_other_than_first_spell_trigger(input: &str) -> OtherThanFirstSpellParse { let mut actor_parser = alt(( - value(OtherThanFirstSpellActor::You, tag("you cast an ")), + value( + OtherThanFirstSpellActor::You, + tag::<_, _, OracleError<'_>>("you cast an "), + ), value(OtherThanFirstSpellActor::You, tag("you cast a ")), value( OtherThanFirstSpellActor::Opponent, diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 5fefabd63d..70a9ab30b4 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -276,8 +276,8 @@ mod hit_the_mother_lode; mod hogaak_cant_spend_mana_1095; mod hollow_one_cost_reduction; mod hunters_insight_combat_draw; -mod inevitable_betrayal_no_mana_cost; mod ichneumon_druid; +mod inevitable_betrayal_no_mana_cost; mod infantry_shield_mobilize_grant; mod inspiring_call_indestructible_grant; mod integration_adventure; From 321370a05e3c1ccaa1cc1f0e7afcfc174e9fceba Mon Sep 17 00:00:00 2001 From: keloide <75585494+keloide@users.noreply.github.com> Date: Tue, 11 Aug 2026 15:49:04 +0200 Subject: [PATCH 09/14] fix(parser): recognize non-first spell trigger heads --- crates/engine/src/parser/oracle_trigger.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 51a375ab15..522a5b3623 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -15896,7 +15896,9 @@ fn parse_proliferate_player_action(input: &str) -> OracleResult<'_, PlayerAction /// trigger with a NthSpellThisTurn constraint. fn try_parse_nth_spell_trigger(lower: &str) -> Option<(TriggerMode, TriggerDefinition)> { match parse_other_than_first_spell_trigger(lower) { - OtherThanFirstSpellParse::Accepted(def) => return Some((TriggerMode::SpellCast, def)), + OtherThanFirstSpellParse::Accepted(def) => { + return Some((TriggerMode::SpellCast, *def)); + } // The marker commits this grammar: never let a malformed qualifier // silently broaden through the generic SpellCast parser. OtherThanFirstSpellParse::Rejected => { @@ -15925,7 +15927,7 @@ fn try_parse_nth_spell_trigger(lower: &str) -> Option<(TriggerMode, TriggerDefin enum OtherThanFirstSpellParse { NotCandidate, Rejected, - Accepted(TriggerDefinition), + Accepted(Box), } #[derive(Clone, Copy)] @@ -15940,6 +15942,10 @@ enum OtherThanFirstSpellActor { /// intervening-if condition. The recognizer owns the predicate only after it /// reaches the marker, so a malformed repeated qualifier fails closed. fn parse_other_than_first_spell_trigger(input: &str) -> OtherThanFirstSpellParse { + // `parse_trigger_condition` passes the complete trigger head (including + // "whenever"), while direct classifier tests pass its payload. Normalize + // both callers before matching the actor phrase. + let input = input.strip_prefix("whenever ").unwrap_or(input); let mut actor_parser = alt(( value( OtherThanFirstSpellActor::You, @@ -16047,7 +16053,7 @@ fn parse_other_than_first_spell_trigger(input: &str) -> OtherThanFirstSpellParse comparator: Comparator::GT, filter: first_filter, }); - OtherThanFirstSpellParse::Accepted(def) + OtherThanFirstSpellParse::Accepted(Box::new(def)) } /// Timing-clause kind for nth-spell/nth-draw triggers. From 3cad6630395e2e79c7f6cc12d3e360330eda65a8 Mon Sep 17 00:00:00 2001 From: keloide <75585494+keloide@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:15:50 +0200 Subject: [PATCH 10/14] fix(parser): use combinator for trigger prefix --- crates/engine/src/parser/oracle_trigger.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 522a5b3623..687d853d8f 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -15945,7 +15945,9 @@ fn parse_other_than_first_spell_trigger(input: &str) -> OtherThanFirstSpellParse // `parse_trigger_condition` passes the complete trigger head (including // "whenever"), while direct classifier tests pass its payload. Normalize // both callers before matching the actor phrase. - let input = input.strip_prefix("whenever ").unwrap_or(input); + let Ok((input, _)) = opt(tag::<_, _, OracleError<'_>>("whenever ")).parse(input) else { + return OtherThanFirstSpellParse::NotCandidate; + }; let mut actor_parser = alt(( value( OtherThanFirstSpellActor::You, From 45748cd3a6c0a1bf4200b81c281666dbcc25e4cf Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 08:16:48 -0700 Subject: [PATCH 11/14] fix(PR-7233): render trigger ordinals correctly --- crates/engine/src/game/coverage.rs | 55 +++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 1432ea5c8e..ea59b373f2 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -4161,6 +4161,19 @@ fn fmt_trigger_condition(cond: &crate::types::ability::TriggerCondition) -> Stri } } +fn fmt_ordinal(n: u32) -> String { + let suffix = match n % 100 { + 11..=13 => "th", + _ => match n % 10 { + 1 => "st", + 2 => "nd", + 3 => "rd", + _ => "th", + }, + }; + format!("{n}{suffix}") +} + /// Format a `TriggerConstraint` as a human-readable string for the parse-details overlay. fn fmt_trigger_constraint(c: &crate::types::ability::TriggerConstraint) -> String { use crate::types::ability::TriggerConstraint as TC; @@ -4174,16 +4187,22 @@ fn fmt_trigger_constraint(c: &crate::types::ability::TriggerConstraint) -> Strin filter, } => { let timing = match comparator { - Comparator::EQ => format!("on your {n}th"), + Comparator::EQ => format!("on your {}", fmt_ordinal(*n)), Comparator::GT if *n == 1 => "after your first".to_string(), - _ => format!("when your spell count {} {n}", fmt_comparator(comparator)), + Comparator::GT + | Comparator::LT + | Comparator::GE + | Comparator::LE + | Comparator::NE => { + format!("when your spell count {} {n}", fmt_comparator(comparator)) + } }; match filter { Some(f) => format!("{timing} {} spell this turn", fmt_target(f)), None => format!("{timing} spell this turn"), } } - TC::NthDrawThisTurn { n } => format!("on your {n}th draw this turn"), + TC::NthDrawThisTurn { n } => format!("on your {} draw this turn", fmt_ordinal(*n)), TC::OnlyDuringOpponentsTurn => "only during opponent's turn".into(), TC::OnlyDuringYourMainPhase => "only during your main phase".into(), TC::AtClassLevel { level } => format!("at class level {level}"), @@ -11354,8 +11373,36 @@ mod tests { comparator: Comparator::EQ, filter: None, }), - "on your 2th spell this turn" + "on your 2nd spell this turn" + ); + assert_eq!( + fmt_trigger_constraint(&TriggerConstraint::NthSpellThisTurn { + n: 13, + comparator: Comparator::EQ, + filter: None, + }), + "on your 13th spell this turn" ); + assert_eq!( + fmt_trigger_constraint(&TriggerConstraint::NthDrawThisTurn { n: 3 }), + "on your 3rd draw this turn" + ); + } + + #[test] + fn ordinal_formatter_handles_last_digits_and_teens() { + for (n, expected) in [ + (1, "1st"), + (2, "2nd"), + (3, "3rd"), + (4, "4th"), + (11, "11th"), + (12, "12th"), + (13, "13th"), + (21, "21st"), + ] { + assert_eq!(fmt_ordinal(n), expected); + } } #[test] From 02e63166a7eaae191c2af180ce6f0ec0b5efc948 Mon Sep 17 00:00:00 2001 From: keloide <75585494+keloide@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:35:41 +0200 Subject: [PATCH 12/14] fix(engine): respect teams in opponent triggers --- crates/engine/src/game/trigger_matchers.rs | 6 +- .../tests/integration/ichneumon_druid.rs | 69 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/game/trigger_matchers.rs b/crates/engine/src/game/trigger_matchers.rs index ec1901697c..86a3229981 100644 --- a/crates/engine/src/game/trigger_matchers.rs +++ b/crates/engine/src/game/trigger_matchers.rs @@ -683,7 +683,9 @@ fn player_matches_filter( TargetFilter::Player => true, TargetFilter::AllPlayers => true, TargetFilter::Controller => trigger_controller == player_id, - TargetFilter::Opponent => trigger_controller != player_id, + // In team games, opponents are players on other teams; + // teammates are not opponents even though their player IDs differ. + TargetFilter::Opponent => crate::game::players::is_opponent(state, trigger_controller, player_id), TargetFilter::Typed(TypedFilter { controller: Some(ControllerRef::You), .. @@ -691,7 +693,7 @@ fn player_matches_filter( TargetFilter::Typed(TypedFilter { controller: Some(ControllerRef::Opponent), .. - }) => trigger_controller != player_id, + }) => crate::game::players::is_opponent(state, trigger_controller, player_id), TargetFilter::SourceChosenPlayer => source_context .source_read(state) .lki() diff --git a/crates/engine/tests/integration/ichneumon_druid.rs b/crates/engine/tests/integration/ichneumon_druid.rs index 465264d7aa..34176a96c8 100644 --- a/crates/engine/tests/integration/ichneumon_druid.rs +++ b/crates/engine/tests/integration/ichneumon_druid.rs @@ -1,12 +1,15 @@ //! Runtime regression for Ichneumon Druid's non-first instant-spell trigger. use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::format::FormatConfig; use engine::types::game_state::WaitingFor; use engine::types::identifiers::ObjectId; use engine::types::mana::{ManaType, ManaUnit}; use engine::types::phase::Phase; +use engine::types::PlayerId; const ICHNEUMON_DRUID: &str = "Whenever an opponent casts an instant spell other than the first instant spell that player casts each turn, this creature deals 4 damage to that player."; +const P2: PlayerId = PlayerId(2); /// CR 603.2: this is a fire-time event qualifier, not a CR 603.4 /// intervening-if. A noninstant between the first and second instant must not @@ -86,3 +89,69 @@ fn ichneumon_druid_damages_only_after_opponents_first_instant() { "every later instant must trigger" ); } + +/// In Two-Headed Giant, P0/P1 are teammates and P2/P3 are the +/// opposing team. The same spell-history threshold is met for P1 and P2, so +/// team membership is the only axis that may determine whether this opponent +/// trigger fires. +#[test] +fn ichneumon_druid_excludes_teammates_in_two_headed_giant() { + let mut scenario = GameScenario::new_with_format(FormatConfig::two_headed_giant(), 4, 42); + scenario.at_phase(Phase::PreCombatMain); + scenario.add_creature_from_oracle(P0, "Ichneumon Druid", 1, 1, ICHNEUMON_DRUID); + + let teammate_first = scenario.add_bolt_to_hand(P1); + let teammate_second = scenario.add_bolt_to_hand(P1); + let opponent_first = scenario.add_bolt_to_hand(P2); + let opponent_second = scenario.add_bolt_to_hand(P2); + let target_a = scenario.add_creature(P0, "Target A", 0, 8).id(); + let target_b = scenario.add_creature(P0, "Target B", 0, 8).id(); + let target_c = scenario.add_creature(P0, "Target C", 0, 8).id(); + let target_d = scenario.add_creature(P0, "Target D", 0, 8).id(); + let mana = ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![]); + scenario.with_mana_pool(P1, vec![mana.clone(), mana.clone()]); + scenario.with_mana_pool(P2, vec![mana.clone(), mana]); + + let mut runner = scenario.build(); + let teammate_life = runner.life(P1); + let opponent_life = runner.life(P2); + + runner.state_mut().active_player = P1; + runner.state_mut().priority_player = P1; + runner.state_mut().waiting_for = WaitingFor::Priority { player: P1 }; + runner + .cast(teammate_first) + .target_object(target_a) + .resolve(); + runner + .cast(teammate_second) + .target_object(target_b) + .resolve(); + assert_eq!( + runner.life(P1), + teammate_life, + "P1 is P0's teammate in Two-Headed Giant, so neither instant may trigger Ichneumon Druid" + ); + + runner.state_mut().active_player = P2; + runner.state_mut().priority_player = P2; + runner.state_mut().waiting_for = WaitingFor::Priority { player: P2 }; + runner + .cast(opponent_first) + .target_object(target_c) + .resolve(); + assert_eq!( + runner.life(P2), + opponent_life, + "an opposing team's first instant must not trigger Ichneumon Druid" + ); + runner + .cast(opponent_second) + .target_object(target_d) + .resolve(); + assert_eq!( + runner.life(P2), + opponent_life - 4, + "an opposing team's second instant must trigger Ichneumon Druid exactly once" + ); +} From 379eeed3412aef8c379e666a01de069a026a2b4b Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 11:04:44 -0700 Subject: [PATCH 13/14] fix(PR-7233): document team-aware opponent relation --- crates/engine/src/game/trigger_matchers.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/engine/src/game/trigger_matchers.rs b/crates/engine/src/game/trigger_matchers.rs index 86a3229981..968fcc177b 100644 --- a/crates/engine/src/game/trigger_matchers.rs +++ b/crates/engine/src/game/trigger_matchers.rs @@ -679,6 +679,8 @@ fn player_matches_filter( source_context: &TriggerSourceContext, ) -> bool { let trigger_controller = source_context.source_read(state).controller(); + // CR 102.3: In games between teams, teammates are not opponents; use the + // shared team-topology authority for every opponent-scoped player filter. match filter { TargetFilter::Player => true, TargetFilter::AllPlayers => true, From 8e0efb41b225b87cb66dbdc5d8de5f7b6ff17387 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 12:05:21 -0700 Subject: [PATCH 14/14] docs(PR-7233): clarify team-aware opponent trigger scope --- crates/engine/src/parser/oracle_trigger.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 687d853d8f..854c01d1a9 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -12825,7 +12825,7 @@ fn parse_event_amount_quantifier(input: &str) -> OracleResult<'_, (Comparator, u /// in trigger_matchers.rs validates the caster against `valid_target`, so: /// - "you cast or copy" → `TargetFilter::Controller` /// - "an opponent casts or copies" → `TypedFilter` with `ControllerRef::Opponent` -/// (evaluates as `source_controller != player_id` in the current engine model) +/// (evaluates through the shared team-aware opponent relation, CR 102.3) /// - "a player casts or copies" → `TargetFilter::Player` (any player, CR 102.1) /// /// Covers Storm-Kiln Artist (you), Mage Hunter (opponent), and any future card