diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 63de7c9cfb..5189826299 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -12723,6 +12723,7 @@ mod tests { Effect::CreateDelayedTrigger { condition: DelayedTriggerCondition::WheneverEvent { trigger: Box::new(delayed_trigger), + expiry: crate::types::ability::WheneverEventExpiry::EndOfTurn, }, effect: Box::new(delayed_effect), uses_tracked_set: false, diff --git a/crates/engine/src/game/effects/delayed_trigger.rs b/crates/engine/src/game/effects/delayed_trigger.rs index 6fd77fd73d..1412b3dd3e 100644 --- a/crates/engine/src/game/effects/delayed_trigger.rs +++ b/crates/engine/src/game/effects/delayed_trigger.rs @@ -64,8 +64,57 @@ pub fn resolve( bind_tracked_set_to_condition(&mut condition, real_id); } + // CR 608.2c + CR 603.7c + CR 601.2c: An anaphoric plural-set reference + // ("those creatures" / "any of those creatures", parsed to a pre-bind + // `ParentTarget`) back-references the parent ability's chosen/declared object + // set. When that set is empty — a legal outcome for an "up to N target" + // parent that chose zero (Kang Dynasty taps no creatures, CR 601.2c) — the + // reference can match nothing, so the delayed trigger can never fire and must + // NOT be installed. Skipping here is required: letting a bare `ParentTarget` + // fall through to `bind_contextual_filter_to_condition`, whose empty-parent + // rewrite resolves `ParentTarget` → `TargetFilter::Any` + // (`parent_targets_filter(&[])`), would OVER-FIRE on every creature's combat + // damage. The contextual bind rewrites all three `WheneverEvent` filter slots + // (`valid_card`, `valid_source`, `valid_target`), so a bare `ParentTarget` in + // ANY of them is over-fire prone and must gate installation — not just + // `valid_source`. Scoped to a pre-bind `ParentTarget` only, so a `SelfRef` + // reference (Human Torch's "he", whose empty `ability.targets` is normal) still + // installs. + if let DelayedTriggerCondition::WheneverEvent { trigger, .. } = &condition { + let references_empty_parent = ability.targets.is_empty() + && [ + &trigger.valid_source, + &trigger.valid_card, + &trigger.valid_target, + ] + .iter() + .any(|filter| matches!(filter, Some(TargetFilter::ParentTarget))); + if references_empty_parent { + events.push(GameEvent::EffectResolved { + kind: EffectKind::CreateDelayedTrigger, + source_id: ability.source_id, + subject: None, + }); + return Ok(()); + } + } + bind_contextual_filter_to_condition(&mut condition, &ability.targets); + // CR 603.7b: "until your next turn" is fixed at CREATION. The parser emits the + // symbolic `AfterCreationTurn` floor (compile-time AST has no runtime turn + // number); stamp it to the actual creation turn here, mirroring the + // `AtNextPhaseForPlayer` gate rewrite below. + if let DelayedTriggerCondition::WheneverEvent { + expiry: crate::types::ability::WheneverEventExpiry::UntilControllersNextTurn { after }, + .. + } = &mut condition + { + if matches!(after, crate::types::ability::TurnGate::AfterCreationTurn) { + *after = crate::types::ability::TurnGate::After(state.turn_number); + } + } + // CR 505.1 + CR 603.7a: "your next " binds the trigger to the // ability's controller. The parser emits a placeholder `PlayerId(0)` in // `AtNextPhaseForPlayer.player` because compile-time AST has no access to @@ -126,7 +175,27 @@ pub fn resolve( // creation, and the final fallback correctly returns [dying_creature]. // // CR 603.7c: See separate branch for LastCreated snapshots. - let snapshot_targets = if super::ability_refs_triggering_source(&delayed_ability) { + // + // CR 603.7b: A MULTI-FIRE WheneverEvent must NOT snapshot TriggeringSource at + // creation — each firing has its own triggering source (Love on the + // Battlefield: "put a +1/+1 counter on it" resolves `it` = the creature that + // dealt combat damage THIS firing, re-resolved from the per-firing damage + // event). Snapshotting here would freeze it to the creation event + // (AttackersDeclared), which carries no per-firing source, dropping the + // counter. The snapshot exists for ONE-SHOT delayed triggers whose end-step + // firing event lacks the source (Grave Betrayal, Liliana emblem); those keep + // it. `!one_shot` (a WheneverEvent) forces per-firing event-context + // resolution instead. + // + // CR 603.7c: Computed ONCE here and reused for the creation-snapshot gate, the + // TriggeringSource origin-stamp gate, and the `DelayedTrigger.one_shot` field, + // so the three sites can never silently diverge. `condition`'s variant is not + // reassigned between them. + let one_shot = !matches!( + condition, + crate::types::ability::DelayedTriggerCondition::WheneverEvent { .. } + ); + let snapshot_targets = if one_shot && super::ability_refs_triggering_source(&delayed_ability) { // CR 603.7c: TriggeringSource always reads the event context (the dying // creature from the ZoneChanged event), not the parent ability's chosen // targets. Bypasses parent_target_snapshot's ability.targets early-return, @@ -152,7 +221,14 @@ pub fn resolve( vec![] }; - if super::ability_refs_triggering_source(&delayed_ability) { + // CR 603.7c: Stamp `ChangeZone.origin` from the CREATION event's + // TriggeringSource destination zone only for ONE-SHOT delayed triggers, whose + // later firing event (an end step / phase change) carries no source and so + // relies on the creation-time snapshot. A MULTI-FIRE WheneverEvent re-resolves + // TriggeringSource from EACH firing event, so freezing the origin to the + // creation event's zone would make a later firing from a different zone skip + // the zone move. Gated on the same `one_shot` flag as `snapshot_targets` above. + if one_shot && super::ability_refs_triggering_source(&delayed_ability) { if let Some(zone) = triggering_source_destination_zone(state) { stamp_triggering_source_origins_in_ability_chain(&mut delayed_ability, zone); } @@ -221,10 +297,7 @@ pub fn resolve( // CR 603.7c: Most delayed triggers fire once and are removed. // WheneverEvent triggers fire each time and persist until end-of-turn cleanup. - let one_shot = !matches!( - condition, - crate::types::ability::DelayedTriggerCondition::WheneverEvent { .. } - ); + // `one_shot` was computed once above (single source of truth) and is reused here. crate::game::triggers::install_delayed_trigger( state, DelayedTrigger { @@ -434,7 +507,7 @@ fn bind_contextual_filter_to_condition( | DelayedTriggerCondition::WhenDiesOrExiled { filter } => { bind_parent_target_filter(filter, parent_targets); } - DelayedTriggerCondition::WheneverEvent { trigger } => { + DelayedTriggerCondition::WheneverEvent { trigger, .. } => { for filter in [ &mut trigger.valid_card, &mut trigger.valid_source, @@ -1391,6 +1464,7 @@ mod tests { Effect::CreateDelayedTrigger { condition: DelayedTriggerCondition::WheneverEvent { trigger: Box::new(trigger), + expiry: crate::types::ability::WheneverEventExpiry::EndOfTurn, }, effect: Box::new(effect_def), uses_tracked_set: false, @@ -1403,7 +1477,7 @@ mod tests { resolve(&mut state, &ability, &mut events).unwrap(); - let DelayedTriggerCondition::WheneverEvent { trigger } = + let DelayedTriggerCondition::WheneverEvent { trigger, .. } = &state.delayed_triggers[0].condition else { panic!( @@ -1417,6 +1491,172 @@ mod tests { ); } + /// CR 601.2c + CR 608.2c: an anaphoric `ParentTarget` source whose parent set + /// is EMPTY (an "up to N target" parent that chose zero — Kang Dynasty tapping + /// no creatures) must NOT install the delayed trigger. Reverting the empty-set + /// guard binds `valid_source` to `TargetFilter::Any` (over-fire on every + /// source), which this test rejects. + #[test] + fn whenever_event_empty_parent_target_set_skips_install() { + let mut state = GameState::new_two_player(42); + + let mut trigger = TriggerDefinition::new(TriggerMode::DamageDone); + trigger.damage_kind = DamageKindFilter::CombatOnly; + trigger.valid_source = Some(TargetFilter::ParentTarget); + trigger.valid_target = Some(TargetFilter::Player); + + let effect_def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ); + let ability = ResolvedAbility::new( + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::WheneverEvent { + trigger: Box::new(trigger), + expiry: crate::types::ability::WheneverEventExpiry::EndOfTurn, + }, + effect: Box::new(effect_def), + uses_tracked_set: false, + }, + // Empty parent-target set — the "up to N target" parent chose zero. + vec![], + ObjectId(5), + PlayerId(0), + ); + let mut events = Vec::new(); + + resolve(&mut state, &ability, &mut events).unwrap(); + + assert!( + state.delayed_triggers.is_empty(), + "an empty anaphoric ParentTarget source must not install a delayed trigger \ + (else it would bind to Any and over-fire)" + ); + } + + /// Build a `WheneverEvent` delayed trigger whose `TriggerDefinition` is shaped + /// by `set_slot`, resolve it with an EMPTY parent-target set, and assert it did + /// NOT install. Shared by the `valid_card` / `valid_target` sibling fixtures. + fn empty_parent_target_in_slot_skips_install(set_slot: impl FnOnce(&mut TriggerDefinition)) { + let mut state = GameState::new_two_player(42); + + let mut trigger = TriggerDefinition::new(TriggerMode::DamageDone); + trigger.damage_kind = DamageKindFilter::CombatOnly; + set_slot(&mut trigger); + + let effect_def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ); + let ability = ResolvedAbility::new( + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::WheneverEvent { + trigger: Box::new(trigger), + expiry: crate::types::ability::WheneverEventExpiry::EndOfTurn, + }, + effect: Box::new(effect_def), + uses_tracked_set: false, + }, + // Empty parent-target set — the "up to N target" parent chose zero. + vec![], + ObjectId(5), + PlayerId(0), + ); + let mut events = Vec::new(); + + resolve(&mut state, &ability, &mut events).unwrap(); + + assert!( + state.delayed_triggers.is_empty(), + "an empty anaphoric ParentTarget in ANY WheneverEvent slot must not install \ + (else it binds to Any and over-fires)" + ); + } + + /// CR 601.2c + CR 608.2c (PR #6884 blocker 2): `bind_contextual_filter_to_condition` + /// rewrites all three `WheneverEvent` filter slots, so an empty parent set turns a + /// bare `ParentTarget` in `valid_card` — not only `valid_source` — into + /// `TargetFilter::Any`. The install guard must inspect `valid_card` too. + #[test] + fn whenever_event_empty_parent_target_in_valid_card_skips_install() { + empty_parent_target_in_slot_skips_install(|trigger| { + trigger.valid_card = Some(TargetFilter::ParentTarget); + trigger.valid_target = Some(TargetFilter::Player); + }); + } + + /// CR 601.2c + CR 608.2c (PR #6884 blocker 2): sibling of the `valid_card` fixture + /// — an empty bare `ParentTarget` in `valid_target` must likewise gate installation. + #[test] + fn whenever_event_empty_parent_target_in_valid_target_skips_install() { + empty_parent_target_in_slot_skips_install(|trigger| { + trigger.valid_target = Some(TargetFilter::ParentTarget); + }); + } + + /// CR 603.7b: an "until your next turn" `WheneverEvent` is a multi-fire trigger + /// (`one_shot == false`) whose symbolic `AfterCreationTurn` expiry floor is + /// stamped to the concrete creation turn at resolution (Kang Dynasty). Reverting + /// the resolve-time stamp leaves the symbolic gate, and reverting the field + /// drops the expiry entirely. + #[test] + fn whenever_event_until_controllers_next_turn_stamps_creation_turn() { + use crate::types::ability::{TurnGate, WheneverEventExpiry}; + let mut state = GameState::new_two_player(42); + state.turn_number = 7; + let target = ObjectId(10); + + let mut trigger = TriggerDefinition::new(TriggerMode::DamageDone); + trigger.damage_kind = DamageKindFilter::CombatOnly; + trigger.valid_source = Some(TargetFilter::ParentTarget); + trigger.valid_target = Some(TargetFilter::Player); + + let effect_def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ); + let ability = ResolvedAbility::new( + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::WheneverEvent { + trigger: Box::new(trigger), + expiry: WheneverEventExpiry::UntilControllersNextTurn { + after: TurnGate::AfterCreationTurn, + }, + }, + effect: Box::new(effect_def), + uses_tracked_set: false, + }, + vec![TargetRef::Object(target)], + ObjectId(5), + PlayerId(0), + ); + let mut events = Vec::new(); + + resolve(&mut state, &ability, &mut events).unwrap(); + + let installed = &state.delayed_triggers[0]; + assert!(!installed.one_shot, "WheneverEvent is multi-fire"); + let DelayedTriggerCondition::WheneverEvent { expiry, .. } = &installed.condition else { + panic!("expected WheneverEvent, got {:?}", installed.condition); + }; + assert_eq!( + *expiry, + WheneverEventExpiry::UntilControllersNextTurn { + after: TurnGate::After(7), + }, + "AfterCreationTurn must be stamped to After(creation turn = 7)" + ); + } + #[test] fn uses_tracked_set_binds_to_change_zone_all() { let mut state = GameState::new_two_player(42); diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index 4ff99d96eb..2a9293c2db 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -6711,8 +6711,21 @@ fn effect_uses_parent_target(effect: &Effect) -> bool { Effect::Pump { target, .. } | Effect::PumpAll { target, .. } => { matches!(target, TargetFilter::ParentTarget) } - Effect::GenericEffect { target, .. } => { + // CR 608.2c: "those creatures gain until end of turn" lowers to a + // `GenericEffect` whose granted static ability is `affected: ParentTarget` + // while the effect's own `target` slot stays `None` (Love on the + // Battlefield). Recognize BOTH the direct-target form and the + // static-ability-affected form so the batched-attack anaphor ("those + // creatures") seeds the declared attackers into `ability.targets`. + Effect::GenericEffect { + target, + static_abilities, + .. + } => { matches!(target, Some(TargetFilter::ParentTarget)) + || static_abilities + .iter() + .any(|s| matches!(s.affected, Some(TargetFilter::ParentTarget))) } _ => effect .target_filter() @@ -8168,6 +8181,64 @@ pub(crate) fn filter_consumed_trigger_events( filter_consumed_trigger_events_from(events, 0, consumed) } +/// CR 603.2c + CR 510.2: Expand a multi-fire `WheneverEvent` `DamageDone` +/// trigger's aggregate `CombatDamageDealtToPlayer` matches into one synthetic +/// per-source `DamageDealt` event per matching (source, defending player) +/// occurrence, so each firing binds `TriggeringSource`/`EventContextAmount` to a +/// single creature. CR 510.2 deals all combat damage in a step simultaneously, so +/// one combat-damage step can emit SEVERAL aggregate events at once — one per +/// defending player (multiplayer / batched attacks split across opponents, e.g. +/// Love on the Battlefield). Every such aggregate in the batch is expanded, not +/// just the first `.find()` match, so a rider that hits two defenders fires for +/// each (source, player) occurrence. Each returned pair carries the originating +/// aggregate's event index for consumed-occurrence tracking. Returns the matched +/// event unchanged (paired with `matched_index`) for every other case +/// (non-`WheneverEvent`, or no aggregate match — e.g. a `SelfRef` source already +/// matching the per-source `DamageDealt` event directly, or a non-damage trigger). +fn expand_multi_fire_damage_occurrences( + condition: &crate::types::ability::DelayedTriggerCondition, + events: &[GameEvent], + matched_index: usize, + matched_event: &GameEvent, + state: &GameState, + source_context: Option<&TriggerSourceContext>, +) -> Vec<(usize, GameEvent)> { + use crate::types::ability::DelayedTriggerCondition; + let DelayedTriggerCondition::WheneverEvent { trigger, .. } = condition else { + return vec![(matched_index, matched_event.clone())]; + }; + let Some(source_context) = source_context else { + return vec![(matched_index, matched_event.clone())]; + }; + // CR 603.2c: a single trigger event (the combat-damage step) can contain + // multiple occurrences. Expand EVERY matching aggregate + // `CombatDamageDealtToPlayer` in the batch — one per defending player — into + // its per-source synthetic `DamageDealt` events, tagging each with the source + // aggregate's index. `matching_damage_done_events` is empty for non-aggregate + // listeners (SelfRef) and non-`DamageDone` triggers, so this scan is inert for + // every case handled by the unchanged-fallback below. + let expanded: Vec<(usize, GameEvent)> = events + .iter() + .enumerate() + .filter(|(_, event)| matches!(event, GameEvent::CombatDamageDealtToPlayer { .. })) + .flat_map(|(idx, event)| { + super::trigger_matchers::matching_damage_done_events( + event, + trigger, + source_context, + state, + ) + .into_iter() + .map(move |synth| (idx, synth)) + }) + .collect(); + if expanded.is_empty() { + vec![(matched_index, matched_event.clone())] + } else { + expanded + } +} + fn delayed_trigger_to_context( state: &GameState, trigger: DelayedTrigger, @@ -8237,7 +8308,30 @@ fn collect_matching_delayed_triggers( if delayed.one_shot { to_remove.push((idx, event_index, trigger_event)); } else { - to_fire.push((delayed.clone(), event_index, trigger_event, false)); + // CR 603.2c + CR 510.2: A MULTI-FIRE WheneverEvent DamageDone + // trigger that matched the AGGREGATE `CombatDamageDealtToPlayer` + // event fires ONCE PER matching (source, defending player) + // occurrence — each creature dealing combat damage is a separate + // occurrence (CR 603.2c), and one simultaneous combat-damage step + // (CR 510.2) can deal to several defenders at once (multiplayer / + // batched attacks). Expand EVERY matching aggregate in the batch — + // not just the first `.find()` match — into per-source synthetic + // `DamageDealt` events so `TriggeringSource` / `EventContextAmount` + // bind to each specific source (Love on the Battlefield's + // per-creature "+1/+1 counter on it"), for every defender hit. + // Non-aggregate matches and non-DamageDone conditions fire once on + // the matched event unchanged. Each occurrence carries its own + // originating aggregate index for consumed-occurrence tracking. + for (occ_index, occurrence) in expand_multi_fire_damage_occurrences( + &delayed.condition, + events, + event_index, + &trigger_event, + state, + delayed.ability.trigger_source.as_ref(), + ) { + to_fire.push((delayed.clone(), occ_index, occurrence, false)); + } } } else if match scope { DelayedTriggerEventScope::Any => is_reflexive_lifetime(&delayed.condition), @@ -8306,9 +8400,21 @@ fn collect_matching_delayed_triggers( let mut pending: Vec = to_fire .into_iter() .map(|(trigger, event_index, trigger_event, removed_one_shot)| { + // CR 603.2c + CR 510.2: The consumed IDENTITY is the raw originating + // buffer event at `event_index`. For an expanded multi-fire combat + // trigger that is the aggregate `CombatDamageDealtToPlayer` — NOT the + // synthetic per-source `DamageDealt` in `trigger_event`, which exists + // only as per-firing context. `trigger_event_occurrence` counts + // occurrences of `events[event_index]`, so the recorded event MUST key + // off the same raw event; otherwise `filter_consumed_trigger_events_from` + // (which compares both event equality and occurrence) never matches the + // aggregate, leaving it in the buffer for a later priority scan to + // re-expand and fire the delayed trigger a second time. For every + // non-expanded case `trigger_event == events[event_index]`, so this is a + // no-op there. consumed_events.push(ConsumedTriggerEventOccurrence { occurrence: trigger_event_occurrence(events, event_index), - event: trigger_event.clone(), + event: events[event_index].clone(), }); let origin = trigger.provenance.origin(); let binding = super::lifecycle::ImmutableBinding { @@ -8677,7 +8783,7 @@ fn delayed_trigger_event_with_index( }) .map(|(idx, event)| (idx, event.clone())), // CR 603.7c: "Whenever [event] this turn" — delegate to trigger matcher registry. - DelayedTriggerCondition::WheneverEvent { trigger } => { + DelayedTriggerCondition::WheneverEvent { trigger, .. } => { let source_context = source_context?; if let Some(matcher) = super::trigger_matchers::trigger_matcher(trigger.mode.clone()) { events @@ -17562,6 +17668,132 @@ pub mod tests { ); } + /// CR 603.2c + CR 510.2 (PR #6884 blocker 1): a MULTI-FIRE combat-damage + /// `WheneverEvent` that expands one aggregate `CombatDamageDealtToPlayer` into + /// per-source synthetic `DamageDealt` firings must record the RAW AGGREGATE as + /// its consumed identity — never the synthetic per-source event. + /// `trigger_event_occurrence` keys the occurrence off the aggregate's buffer + /// slot, so a synthetic-keyed consumed entry could never be matched by + /// `filter_consumed_trigger_events`, leaving the aggregate in the buffer for a + /// later priority scan to re-expand and fire the trigger a SECOND time. This + /// proves both halves of the fix: (1) the consumed identity is the aggregate, + /// and (2) filtering the buffer by the consumed set actually removes the + /// aggregate, so a re-scan finds no combat event to re-fire on (one firing per + /// source, no double-fire). + #[test] + fn multi_fire_combat_damage_consumes_raw_aggregate_not_synthetic() { + let mut state = setup(); + let controller = PlayerId(0); + let defender = PlayerId(1); + + let rider_source = create_object( + &mut state, + CardId(0x6884_0001), + controller, + "Combat Rider".to_string(), + Zone::Battlefield, + ); + let attacker_a = create_object( + &mut state, + CardId(0x6884_0002), + controller, + "Attacker A".to_string(), + Zone::Battlefield, + ); + let attacker_b = create_object( + &mut state, + CardId(0x6884_0003), + controller, + "Attacker B".to_string(), + Zone::Battlefield, + ); + for attacker in [attacker_a, attacker_b] { + state + .objects + .get_mut(&attacker) + .expect("attacker exists") + .card_types + .core_types + .push(CoreType::Creature); + } + + // A Love-on-the-Battlefield-shaped rider: "whenever a creature you control + // deals combat damage to a player, ..." — a source-filtered (non-SelfRef) + // listener that binds on the AGGREGATE combat-damage event and expands + // per damaging source. + let mut trigger = TriggerDefinition::new(TriggerMode::DamageDone); + trigger.valid_source = Some(TargetFilter::Typed( + TypedFilter::creature().controller(ControllerRef::You), + )); + trigger.valid_target = Some(TargetFilter::Player); + trigger.damage_kind = DamageKindFilter::CombatOnly; + + let mut ability = ResolvedAbility::new( + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + vec![], + rider_source, + controller, + ); + // The expansion path requires the delayed ability to carry its + // creation-time trigger source; without it, collection returns the event + // unexpanded (see `expand_multi_fire_damage_occurrences`). + let rider_ctx = trigger_source_context_for_latch(&state, &state.objects[&rider_source]); + ability.set_trigger_source_recursive(rider_ctx); + + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WheneverEvent { + trigger: Box::new(trigger), + expiry: crate::types::ability::WheneverEventExpiry::EndOfTurn, + }, + ability: Box::new(ability), + controller, + source_id: rider_source, + one_shot: false, + // A normal (legacy) delayed trigger with no command receipt — engine + // test scaffolding, not a rule implementation, so no CR citation. (#6933 + // canonicalized `provenance` from Option to the DelayedInstallIdentity enum.) + provenance: crate::types::identifiers::DelayedInstallIdentity::LegacyDelayed, + }); + + // One combat-damage step: two creatures deal combat damage to one player. + let events = vec![GameEvent::CombatDamageDealtToPlayer { + player_id: defender, + source_amounts: vec![(attacker_a, 2), (attacker_b, 3)], + total_damage: 5, + }]; + + let (pending, consumed) = + collect_matching_delayed_triggers(&mut state, &events, DelayedTriggerEventScope::Any); + + // CR 603.2c: one firing per damaging source — two creatures → two firings. + assert_eq!(pending.len(), 2, "one firing per damaging source"); + + // The consumed identity is the RAW aggregate, not the synthetic per-source + // `DamageDealt`. Before the fix this recorded the synthetic event. + assert!( + consumed + .iter() + .all(|c| matches!(c.event, GameEvent::CombatDamageDealtToPlayer { .. })), + "consumed identity must be the raw aggregate CombatDamageDealtToPlayer, got {:?}", + consumed.iter().map(|c| &c.event).collect::>() + ); + + // Decisive anti-double-fire guard: filtering the buffer by the consumed set + // must REMOVE the aggregate, so a subsequent priority scan sees no combat + // event to re-expand. With a synthetic-keyed consumed entry the aggregate + // would survive here and the trigger could re-fire. + let remaining = filter_consumed_trigger_events(&events, &consumed); + assert!( + !remaining + .iter() + .any(|e| matches!(e, GameEvent::CombatDamageDealtToPlayer { .. })), + "the aggregate must be consumed so a re-scan cannot fire the trigger twice" + ); + } + /// Issue #1304 — RUNTIME: Keeper of the Accord's intervening-if must compare /// the active player's creatures to the source controller's at opponent end /// step, not fail closed because the condition was never hoisted/parsed. diff --git a/crates/engine/src/game/turns.rs b/crates/engine/src/game/turns.rs index 6f53ec78aa..9e60f02b9c 100644 --- a/crates/engine/src/game/turns.rs +++ b/crates/engine/src/game/turns.rs @@ -1431,6 +1431,43 @@ pub fn execute_untap_with_choices( // CR 514.2: Prune "until your next turn" transient effects for the active player. super::layers::prune_until_next_turn_effects(state, active); + // CR 603.7b: A `WheneverEvent` delayed trigger with a stated "until your next + // turn" duration ends at the START of its controller's next turn (the untap + // step, CR 502.4 — before priority), not at cleanup (CR 514.2). This boundary + // coincides with the goad window it was designed around (CR 701.15a: "until the + // next turn of the controller"). It survived the creating turn's cleanup via + // the retain disjunct in `execute_cleanup`; remove it now that the controller's + // next turn has begun (`turn_number` strictly past the stamped creation floor). + { + use crate::types::ability::{ + DelayedTriggerCondition as Cond, TurnGate, WheneverEventExpiry, + }; + let turn_number = state.turn_number; + let mut survivors = Vec::new(); + let mut expired = Vec::new(); + for trigger in std::mem::take(&mut state.delayed_triggers) { + if matches!( + &trigger.condition, + Cond::WheneverEvent { + expiry: WheneverEventExpiry::UntilControllersNextTurn { + after: TurnGate::After(floor), + }, + .. + } if trigger.controller == active && turn_number > *floor + ) { + expired.push(trigger); + } else { + survivors.push(trigger); + } + } + state.delayed_triggers = survivors; + for trigger in expired { + super::lifecycle::record_delayed_terminal( + trigger.provenance.firing(), + super::lifecycle::DelayedTerminalDisposition::CleanupExpired, + ); + } + } // CR 514.2 + CR 611.2a/b: Expire `PlayFromExile` permissions granted to // the active player with `UntilYourNextTurn` duration (impulse draws that // last "until your next turn"). @@ -2230,15 +2267,32 @@ pub fn execute_cleanup(state: &mut GameState, events: &mut Vec) -> Op let mut survivors = Vec::new(); let mut expired = Vec::new(); for trigger in std::mem::take(&mut state.delayed_triggers) { + use crate::types::ability::{ + DelayedTriggerCondition as Cond, DelayedTriggerLifetime as Life, WheneverEventExpiry, + }; + // CR 514.2: a default (`EndOfTurn`) `WheneverEvent` and a lingering + // one-shot end at this cleanup — caught below by the `one_shot == false` + // leg (a `WheneverEvent` has `one_shot == false`) and the `WhenNextEvent` + // `matches!` respectively. let retain = trigger.one_shot && !matches!( - trigger.condition, - crate::types::ability::DelayedTriggerCondition::WhenNextEvent { + &trigger.condition, + Cond::WhenNextEvent { // CR 603.7b + CR 603.12: both a stated-"this turn" one-shot and // any reflexive that (defensively) escaped its creation-batch // discard are bounded to the creating turn — prune at cleanup. - lifetime: crate::types::ability::DelayedTriggerLifetime::ThisTurn - | crate::types::ability::DelayedTriggerLifetime::Reflexive, + lifetime: Life::ThisTurn | Life::Reflexive, + .. + } + ) + // CR 603.7b: a `WheneverEvent` with a stated "until your next turn" + // duration must survive the CREATING turn's cleanup — it fires on the + // intervening (opponents') turns and is instead purged at the + // controller's next turn start (see `execute_untap_with_choices`). + || matches!( + &trigger.condition, + Cond::WheneverEvent { + expiry: WheneverEventExpiry::UntilControllersNextTurn { .. }, .. } ); diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index b95bd924ab..6ef6f37b94 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -11535,6 +11535,7 @@ mod tests { Effect::CreateDelayedTrigger { condition: DelayedTriggerCondition::WheneverEvent { trigger: Box::new(TriggerDefinition::new(TriggerMode::YouAttack)), + expiry: crate::types::ability::WheneverEventExpiry::EndOfTurn, }, effect: Box::new(token_creator), uses_tracked_set: false, diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 44507b31a3..2eeac7cf22 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -114,7 +114,8 @@ use crate::types::ability::{ SharedQualityRelation, SiblingCondition, SkipScope, SpellStackToGraveyardReplacement, StaticCondition, StaticDefinition, StepSkipTarget, SubAbilityLink, TapStateChange, TargetFilter, TargetSelectionMode, ThisWayCause, TrackedAnaphorSource, TriggerCondition, - TriggerDefinition, TypeFilter, TypedFilter, UnlessPayModifier, UntilCondition, ZoneOwner, + TriggerDefinition, TurnGate, TypeFilter, TypedFilter, UnlessPayModifier, UntilCondition, + WheneverEventExpiry, ZoneOwner, }; #[cfg(test)] use crate::types::ability::{AttackScope, AttackSubject}; @@ -156,7 +157,9 @@ use self::subject::{ try_parse_subject_predicate_ast, try_parse_targeted_controller_gain_life, }; use crate::parser::oracle_ir::ast::*; -pub(crate) use crate::parser::oracle_ir::context::{ParseContext, TokenPtFollowup}; +pub(crate) use crate::parser::oracle_ir::context::{ + ParseContext, TokenPtFollowup, TriggerConditionScope, +}; use crate::parser::oracle_ir::effect_chain::{ AbilityIr, AbilityRootTransform, AbilityShellIr, AbsorbKind, ClauseDisposition, ClauseIr, ClauseIrBuilder, DieResultBranchIr, EffectChainIr, OtherwiseKind, PlayerScopeRewrite, @@ -938,7 +941,46 @@ const DELAYED_TRIGGER_WINDOWS: [&str; 2] = [" this turn, ", " this combat, "]; /// each matching phase for the rest of the turn. Without this path it would fall /// through to printed-trigger dispatch and become a battlefield Phase trigger that /// never fires for an instant/sorcery. +/// CR 603.7b: Map a leading stated duration to a multi-fire `WheneverEvent` +/// delayed-trigger expiry. Only "until your next turn" (`UntilNextTurnOf` scoped +/// to the controller) is intercepted — it is the load-bearing case where the +/// trigger must survive intervening turns (Kang Dynasty's goaded attackers strike +/// on opponents' turns). Every other leading duration returns `None`, so the +/// caller bails and the outer `strip_leading_duration` dispatch applies it to the +/// enclosing clause exactly as before (e.g. "Until end of turn, whenever …" — the +/// `WheneverEvent` keeps the default `EndOfTurn` expiry and is purged at cleanup). +fn whenever_event_expiry_from_duration(duration: &Duration) -> Option { + match duration { + Duration::UntilNextTurnOf { + player: PlayerScope::Controller, + } => Some(WheneverEventExpiry::UntilControllersNextTurn { + after: TurnGate::AfterCreationTurn, + }), + _ => None, + } +} + fn try_parse_whenever_this_turn(tp: TextPair) -> Option { + // CR 603.7b: capture a leading stated duration as the delayed trigger's + // EXPIRY (its own lifetime), not the enclosing clause's duration. This runs + // at the head so it precedes the outer `strip_leading_duration` dispatch site, + // which would otherwise apply "until your next turn" to the surrounding clause + // (inert for the delayed trigger — it would keep the default `EndOfTurn` expiry + // and be purged at the creating turn's cleanup, so Kang would never fire). + let leading = strip_leading_duration(tp.original); + let expiry = match &leading { + Some((duration, _)) => whenever_event_expiry_from_duration(duration)?, + None => WheneverEventExpiry::EndOfTurn, + }; + // Working `TextPair`: the duration-stripped remainder when a leading duration + // was intercepted, else the original. The lowercase remainder is owned here so + // the rebuilt `TextPair` can borrow it for the rest of the function. + let remainder_lower = leading.as_ref().map(|(_, rest)| rest.to_lowercase()); + let tp = match (&leading, &remainder_lower) { + (Some((_, rest)), Some(lower)) => TextPair::new(rest, lower.as_str()), + _ => tp, + }; + let is_phase_form = tag::<_, _, OracleError<'_>>("at the beginning of ") .parse(tp.lower) .is_ok(); @@ -1003,8 +1045,15 @@ fn try_parse_whenever_this_turn(tp: TextPair) -> Option { // Effect is the remainder after the split boundary. let effect_text = after.original; - // Parse the condition as a trigger using the trigger parser. - let mut inner_ctx = ParseContext::default(); + // Parse the condition as a trigger using the trigger parser. This is a DELAYED + // trigger condition, so the `Delayed` scope enables anaphoric subject resolution + // ("he"/"she" → SelfRef, "those creatures" → ParentTarget) that is valid only as + // a back-reference to the creating ability. (Parser scaffolding — no CR citation: + // this selects a parsing mode, it does not implement a rule.) + let mut inner_ctx = ParseContext { + trigger_condition_scope: TriggerConditionScope::Delayed, + ..ParseContext::default() + }; let mut trigger_def = parse_dealt_damage_this_way_dies_trigger(condition_text, &mut inner_ctx) .unwrap_or_else(|| { let (_, trigger_def) = crate::parser::oracle_trigger::parse_trigger_condition( @@ -1025,12 +1074,32 @@ fn try_parse_whenever_this_turn(tp: TextPair) -> Option { // is already lowercase (`before.lower`). inner_ctx.relative_player_scope = crate::parser::oracle_trigger::relative_player_scope_for_condition(condition_text); + // CR 608.2k + CR 120.1: In a delayed combat/noncombat-damage trigger whose + // subject is a set/other object (not the source permanent), an untargeted + // object anaphor ("it"/"them") in the effect body names the per-firing damage + // dealer — the `TriggeringSource` — e.g. Love on the Battlefield's "put a +1/+1 + // counter on it" (the creature that dealt combat damage, not the enchantment). + // Seed the trigger subject so `resolve_it_pronoun` binds "it" → TriggeringSource + // (via the non-self-subject arm) instead of defaulting to `SelfRef`. Mirrors the + // printed-trigger effect context (`parse_trigger_line`), which likewise seeds + // `subject`. Scoped to `DamageDone`; a `SelfRef`/`Any` subject ("he", Human + // Torch) is left unset so its body keeps the source binding. + if matches!(trigger_def.mode, TriggerMode::DamageDone) { + if let Some(subject) = trigger_def + .valid_source + .clone() + .filter(|f| !matches!(f, TargetFilter::SelfRef | TargetFilter::Any)) + { + inner_ctx.subject = Some(subject); + } + } let inner = parse_effect_chain_with_context(effect_text, AbilityKind::Spell, &mut inner_ctx); Some(ParsedEffectClause { effect: Effect::CreateDelayedTrigger { condition: DelayedTriggerCondition::WheneverEvent { trigger: Box::new(trigger_def), + expiry, }, effect: Box::new(inner), uses_tracked_set: false, diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index 2d7225d9af..69be92d85e 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -1526,7 +1526,7 @@ fn stensian_class_builds_whenever_event_this_combat_delayed_trigger() { ); }; assert!(!uses_tracked_set); - let DelayedTriggerCondition::WheneverEvent { trigger } = condition else { + let DelayedTriggerCondition::WheneverEvent { trigger, .. } = condition else { panic!("expected WheneverEvent, got {condition:?}"); }; assert_eq!(trigger.mode, TriggerMode::DamageDone); diff --git a/crates/engine/src/parser/oracle_ir/context.rs b/crates/engine/src/parser/oracle_ir/context.rs index 0fc5f0d53d..dab5fa4ea1 100644 --- a/crates/engine/src/parser/oracle_ir/context.rs +++ b/crates/engine/src/parser/oracle_ir/context.rs @@ -16,6 +16,26 @@ pub(crate) enum TokenPtFollowup { PowerToughness { power: PtValue, toughness: PtValue }, } +/// Parser-internal scope flag: whether the trigger CONDITION currently being +/// parsed is a printed (card-text) trigger or a DELAYED trigger created from a +/// resolving effect chain. This is parser scaffolding, not a rule implementation, +/// so it carries no CR annotation. Anaphoric subjects that only bind as delayed +/// back-references to the creating ability — the gendered pronoun "he"/"she" (→ +/// `SelfRef`) and the plural set "those creatures"/"any of those creatures" (→ +/// `ParentTarget`) — are recognized ONLY under `Delayed`, so a standalone printed +/// trigger that happens to contain those words stays coverage-honest (`Unknown`) +/// instead of binding its source to `Any`. A typed scope rather than a bare bool +/// per the codebase's "typed enum over bool" convention. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub(crate) enum TriggerConditionScope { + /// A trigger printed on the card. Delayed-only anaphoric subjects stay `Unknown`. + #[default] + Printed, + /// A delayed trigger condition created from a resolving effect chain + /// (set by `try_parse_whenever_this_turn`). + Delayed, +} + /// Unified parsing context — threaded through all parser branches for /// pronoun/reference resolution ("it", "that creature", "that many"). /// @@ -44,6 +64,10 @@ pub(crate) struct ParseContext { /// Whether we are inside a replacement effect. #[allow(dead_code)] // Retained for future nom combinator consumers (D-02). pub in_replacement: bool, + /// Parser-internal scope: whether the trigger CONDITION being parsed is printed + /// card text or a DELAYED trigger created from a resolving effect chain. Gates + /// delayed-only anaphoric subject resolution; see [`TriggerConditionScope`]. + pub trigger_condition_scope: TriggerConditionScope, /// CR 608.2k + CR 601.2a: Event object that bare object pronouns in the /// current trigger body ("it", "them") should bind to. Spell-cast triggers /// set this to `TriggeringSource` so "Whenever you cast a spell, put it ..." diff --git a/crates/engine/src/parser/oracle_tests.rs b/crates/engine/src/parser/oracle_tests.rs index ef7fa1d524..583db699a5 100644 --- a/crates/engine/src/parser/oracle_tests.rs +++ b/crates/engine/src/parser/oracle_tests.rs @@ -10732,13 +10732,20 @@ fn spell_temporal_whenever_line_builds_delayed_trigger() { let Effect::CreateDelayedTrigger { condition, .. } = &*r.abilities[0].effect else { panic!("expected delayed trigger, got {:?}", r.abilities[0].effect); }; - let crate::types::ability::DelayedTriggerCondition::WheneverEvent { trigger } = condition + let crate::types::ability::DelayedTriggerCondition::WheneverEvent { trigger, expiry } = + condition else { panic!("expected WheneverEvent, got {condition:?}"); }; assert_eq!(trigger.mode, TriggerMode::SpellCast); assert_eq!(trigger.valid_target, Some(TargetFilter::Controller)); assert!(trigger.valid_card.is_some()); + // CR 514.2: "this turn" ends the WheneverEvent at the creating turn's cleanup. + assert_eq!( + *expiry, + crate::types::ability::WheneverEventExpiry::EndOfTurn, + "\"this turn\" must lower to an EndOfTurn expiry" + ); assert!(r.parse_warnings.is_empty()); } @@ -11145,12 +11152,19 @@ fn spell_temporal_phase_line_builds_delayed_trigger() { let Effect::CreateDelayedTrigger { condition, .. } = &*r.abilities[0].effect else { panic!("expected delayed trigger, got {:?}", r.abilities[0].effect); }; - let crate::types::ability::DelayedTriggerCondition::WheneverEvent { trigger } = condition + let crate::types::ability::DelayedTriggerCondition::WheneverEvent { trigger, expiry } = + condition else { panic!("expected WheneverEvent, got {condition:?}"); }; assert_eq!(trigger.mode, TriggerMode::Phase); assert_eq!(trigger.phase, Some(Phase::BeginCombat)); + // CR 514.2: "this turn" ends the phase-based WheneverEvent at cleanup. + assert_eq!( + *expiry, + crate::types::ability::WheneverEventExpiry::EndOfTurn, + "\"this turn\" must lower to an EndOfTurn expiry" + ); } #[test] @@ -11170,13 +11184,20 @@ fn spell_temporal_enters_line_builds_delayed_trigger() { else { panic!("expected delayed trigger, got {:?}", r.abilities[0].effect); }; - let crate::types::ability::DelayedTriggerCondition::WheneverEvent { trigger } = condition + let crate::types::ability::DelayedTriggerCondition::WheneverEvent { trigger, expiry } = + condition else { panic!("expected WheneverEvent, got {condition:?}"); }; assert_eq!(trigger.mode, TriggerMode::ChangesZone); assert_eq!(trigger.destination, Some(Zone::Battlefield)); assert!(trigger.valid_card.is_some()); + // CR 514.2: "this turn" ends the zone-change WheneverEvent at cleanup. + assert_eq!( + *expiry, + crate::types::ability::WheneverEventExpiry::EndOfTurn, + "\"this turn\" must lower to an EndOfTurn expiry" + ); assert!(effect.optional); assert!(r.parse_warnings.is_empty()); } diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 5823eacede..994d2f2cb3 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -14,7 +14,7 @@ use super::oracle_effect::{ try_parse_reanimator_aura_grant_etb_effect_ir, }; use super::oracle_ir::ast::parsed_clause; -use super::oracle_ir::context::ParseContext; +use super::oracle_ir::context::{ParseContext, TriggerConditionScope}; use super::oracle_ir::doc::PrintedTriggerIndex; use super::oracle_ir::effect_chain::{DieResultBranchIr, EffectChainIr}; use super::oracle_ir::trigger::{ @@ -9012,6 +9012,69 @@ fn parse_single_subject<'a>(text: &'a str, ctx: &mut ParseContext) -> (TargetFil } } + // Parser heuristic (no CR citation — this is Oracle-text interpretation, not a + // rule implementation): anaphoric subjects that only bind inside a DELAYED + // triggered ability created by a parent ability. These pronouns have no + // antecedent in a standalone printed trigger, so they are recognized ONLY when + // parsing a delayed-trigger condition (`ctx.trigger_condition_scope == Delayed`, + // set by `try_parse_whenever_this_turn`). This keeps a standalone printed trigger + // that happens to contain the same words honestly coverage-red rather than + // binding its source to `Any`. + if ctx.trigger_condition_scope == TriggerConditionScope::Delayed { + // A gendered pronoun naming the trigger source → `SelfRef`. Nominative-only + // ("he "/"she ") and guarded on the following damage verb, mirroring the + // "it enters" bare-pronoun precedent above: a gendered pronoun naming the + // trigger SOURCE is always nominative, and in this delayed combat-damage + // rider class it is always the damage dealer. Object/possessive case + // ("him "/"her ") is intentionally excluded — it never denotes the + // trigger source. `rest` (verb intact) is returned so `try_parse_event`'s + // subject-led damage arm consumes "deals combat damage …". + if let Ok((rest, ())) = + value((), alt((tag::<_, _, OracleError<'_>>("he "), tag("she ")))).parse(text) + { + if alt(( + value((), tag::<_, _, OracleError<'_>>("deals ")), + value((), tag("deal ")), + )) + .parse(rest) + .is_ok() + { + return (TargetFilter::SelfRef, rest); + } + } + + // Plural-set anaphora → `ParentTarget` (parser heuristic, no CR citation). + // Compose the quantifier axis (one `alt`) with the noun axis (one `alt`), per the + // "compose, don't enumerate permutations" convention. "those creatures" / + // "any of those creatures" back-references the set the parent ability + // established (Love's declared attackers, Kang's per-opponent tap + // targets). The quantifier prefix is semantically inert on the trigger's + // fire predicate (any single member firing satisfies it) and is consumed + // only so the noun matches; `bind_contextual_filter_to_condition` binds + // `ParentTarget` → `Or[SpecificObject…]` at delayed-trigger creation. + let after_quantifier = opt(alt(( + tag::<_, _, OracleError<'_>>("any of "), + tag("either of "), + tag("one of "), + tag("each of "), + ))) + .parse(text) + .map(|(rest, _)| rest) + .unwrap_or(text); + if let Ok((rest, ())) = value( + (), + alt(( + tag::<_, _, OracleError<'_>>("those creatures"), + tag("those permanents"), + tag("those cards"), + )), + ) + .parse(after_quantifier) + { + return (TargetFilter::ParentTarget, rest); + } + } + // "equipped creature" / "enchanted creature/land/permanent" / "enchanted " // → AttachedTo. The Enchant keyword already constrains the attach target's type, // so `AttachedTo` alone is sufficient here (CR 702.5a). Utopia Sprawl's @@ -12757,22 +12820,54 @@ fn try_parse_attack_with_n_creatures(lower: &str) -> Option<(TriggerMode, Trigge .parse(after_target) .ok()?; + // CR 508.1 + CR 603.2c: a leading "exactly " flags an EQ attacker-count + // constraint (Love on the Battlefield's "attack with exactly two creatures"). + // Optional; when absent, the trailing quantifier axis below decides. + let (after_exactly, exactly) = opt(tag::<_, _, OracleError<'_>>("exactly ")) + .parse(after_with) + .ok()?; + // Parse the count word/digit. `parse_number` already maps "one"→1 as well as // digits and other number-words; do NOT add a duplicate `value(1, tag("one"))`. - let (after_n, n) = nom_primitives::parse_number.parse(after_with).ok()?; - let (after_or_more, ()) = value((), tag::<_, _, OracleError<'_>>(" or more ")) - .parse(after_n) - .ok()?; + let (after_n, n) = nom_primitives::parse_number.parse(after_exactly).ok()?; + + // CR 508.1 + CR 603.2c: trailing quantifier axis — " or more " (GE, the legacy + // form) or " or fewer " (LE) — composed as one `alt`. + let (after_quantifier, trailing) = opt(alt(( + value(Comparator::GE, tag::<_, _, OracleError<'_>>(" or more ")), + value(Comparator::LE, tag(" or fewer ")), + ))) + .parse(after_n) + .ok()?; + + // CR 508.1: resolve the comparator with no silent default. Leading "exactly" + // → EQ; else a trailing quantifier decides; else a bare "attack with N + // creatures" (no quantifier) is deliberately NOT matched here — `trailing?` + // returns `None` for that bare form, preserving the pre-existing behavior + // exactly and avoiding newly defaulting it to EQ (which would over-narrow + // legacy GE cards). + let comparator = if exactly.is_some() { + Comparator::EQ + } else { + trailing? + }; if n < 1 { return None; } + // The " or more "/" or fewer " tags consumed their surrounding spaces, but the + // EQ path ("exactly two creatures") leaves a leading space before the head + // noun. Consume it uniformly (no-op for the already-stripped trailing arms). + let after_quantifier = nom::character::complete::space0::<_, OracleError<'_>>(after_quantifier) + .map(|(rest, _)| rest) + .unwrap_or(after_quantifier); + // Capture the head-noun type phrase once for both count==1 and count>1. // Count==1 needs only the matcher's valid_card gate; count>1 additionally // uses AttackersDeclaredCount when the type phrase narrows beyond bare // "creatures". - let (filter, remainder) = parse_type_phrase(after_or_more); + let (filter, remainder) = parse_type_phrase(after_quantifier); // Accept optional trailing " each turn" / " this turn" qualifier (unused here, // but keeps the matcher permissive for CR 603.4 timing qualifiers). Must end // at the condition boundary — the caller already split the effect text off, @@ -12807,9 +12902,12 @@ fn try_parse_attack_with_n_creatures(lower: &str) -> Option<(TriggerMode, Trigge if attacks_you { def.attack_target_filter = Some(AttackTargetFilter::Player); } - if n == 1 { + if n == 1 && comparator == Comparator::GE { // CR 508.1 + CR 603.2c: the matcher's "at least one attacker matching - // valid_card" gate is the whole "one or more" condition. + // valid_card" gate is the whole "one or more" condition. Restricted to GE: + // "exactly one" / "one or fewer" (EQ/LE) must enforce the count exactly and + // therefore fall through to the `AttackersDeclaredCount` path below instead + // of firing on 2+ attackers. def.valid_card = Some(filter); return Some((mode, def)); } @@ -12834,7 +12932,7 @@ fn try_parse_attack_with_n_creatures(lower: &str) -> Option<(TriggerMode, Trigge filter: count_filter, } }, - comparator: Comparator::GE, + comparator, count: n, }); diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index b971ba0d1b..c3099b43c2 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -2686,7 +2686,7 @@ fn hunters_insight_class_builds_whenever_event_delayed_trigger() { else { panic!("expected CreateDelayedTrigger, got {:?}", delayed.effect); }; - let DelayedTriggerCondition::WheneverEvent { trigger } = condition else { + let DelayedTriggerCondition::WheneverEvent { trigger, .. } = condition else { panic!("expected WheneverEvent, got {condition:?}"); }; assert_eq!(trigger.mode, TriggerMode::DamageDone); @@ -8093,7 +8093,7 @@ fn trigger_dalkovan_encampment_delayed_you_attack() { let Effect::CreateDelayedTrigger { condition, .. } = delayed_effect else { panic!("expected CreateDelayedTrigger, got {delayed_effect:?}"); }; - let DelayedTriggerCondition::WheneverEvent { trigger } = condition else { + let DelayedTriggerCondition::WheneverEvent { trigger, .. } = condition else { panic!("expected WheneverEvent, got {condition:?}"); }; assert_eq!(trigger.mode, TriggerMode::YouAttack); @@ -10815,7 +10815,7 @@ fn high_tide_delayed_trigger_taps_for_mana_mode_and_filter() { assert_eq!(ability.duration, Some(Duration::UntilEndOfTurn)); let Effect::CreateDelayedTrigger { - condition: DelayedTriggerCondition::WheneverEvent { trigger }, + condition: DelayedTriggerCondition::WheneverEvent { trigger, .. }, effect, .. } = &*ability.effect @@ -10923,7 +10923,7 @@ fn bubbling_muck_delayed_trigger_taps_for_mana_class_general() { .find(|a| matches!(*a.effect, Effect::CreateDelayedTrigger { .. })) .expect("Bubbling Muck must parse a CreateDelayedTrigger"); let Effect::CreateDelayedTrigger { - condition: DelayedTriggerCondition::WheneverEvent { trigger }, + condition: DelayedTriggerCondition::WheneverEvent { trigger, .. }, effect, .. } = &*ability.effect diff --git a/crates/engine/src/parser/swallow_check.rs b/crates/engine/src/parser/swallow_check.rs index 0d7d3b64ed..4871de316b 100644 --- a/crates/engine/src/parser/swallow_check.rs +++ b/crates/engine/src/parser/swallow_check.rs @@ -4305,6 +4305,17 @@ fn detect_duration_next_turn( }) { return; } + // CR 603.7b: "until your next turn" on a multi-fire delayed trigger is carried + // by `WheneverEventExpiry::UntilControllersNextTurn`, not a `Duration` slot + // (Kang Dynasty). Discharge the expectation off that typed carrier too. + if evidence.any_whenever_event_expiry(|e| { + matches!( + e, + crate::types::ability::WheneverEventExpiry::UntilControllersNextTurn { .. } + ) + }) { + return; + } diagnostics.push(OracleDiagnostic::swallowed_clause( OracleSemanticFeature::DurationNextTurn.detector_label(), truncate(original, 140), diff --git a/crates/engine/src/parser/swallow_evidence.rs b/crates/engine/src/parser/swallow_evidence.rs index 22518dc52b..487de62d26 100644 --- a/crates/engine/src/parser/swallow_evidence.rs +++ b/crates/engine/src/parser/swallow_evidence.rs @@ -255,6 +255,14 @@ const DESCRIPTION_KEY: &str = "description"; /// `"`), so the marker was blind to every damage-prevention shield's duration. const DURATION_KEYS: &[&str] = &["duration", "prevention_duration"]; +/// The JSON key at which a `WheneverEventExpiry`-typed field is serialized +/// (`DelayedTriggerCondition::WheneverEvent.expiry`). Externally tagged, same +/// anchoring requirement as [`DURATION_KEYS`]. The key `"expiry"` also carries +/// `RestrictionExpiry` values elsewhere, but the two cannot be confused: only the +/// `UntilControllersNextTurn` variant name is unique to `WheneverEventExpiry`, and +/// a `RestrictionExpiry` value fails `WheneverEventExpiry::deserialize` for it. +const WHENEVER_EVENT_EXPIRY_KEYS: &[&str] = &["expiry"]; + /// Every JSON key at which a `StaticMode`-typed field is serialized. Externally tagged, /// same anchoring requirement as [`DURATION_KEYS`]. /// @@ -534,6 +542,16 @@ impl UnitEvidence { self.any_at(DURATION_KEYS, pred) } + /// Does any `WheneverEventExpiry` carrier satisfy `pred`? Key-anchored per + /// [`WHENEVER_EVENT_EXPIRY_KEYS`]. A delayed `WheneverEvent`'s stated duration + /// ("until your next turn") lives here, not on a `Duration` slot. + pub(super) fn any_whenever_event_expiry( + &self, + pred: impl Fn(&crate::types::ability::WheneverEventExpiry) -> bool, + ) -> bool { + self.any_at(WHENEVER_EVENT_EXPIRY_KEYS, pred) + } + /// Does any `QuantityRef` carrier satisfy `pred`? Key-anchored per [`QUANTITY_KEYS`]. /// /// Never probe `QuantityRef` unanchored: its tag is not discriminating, because 10 of its diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 6afbc1852e..4fa9aa4b43 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -3607,6 +3607,37 @@ impl TurnGate { } } +/// CR 603.7b: The stated duration of a multi-fire `WheneverEvent` delayed +/// triggered ability. "A delayed triggered ability will trigger only once … unless +/// it has a stated duration, such as 'this turn.'" This axis records that stated +/// duration so cleanup/purge can end the trigger at the right boundary. +/// +/// WheneverEvent-specific (turn-gated expiry, not the `ThisTurn`/`Persistent`/ +/// `Reflexive` set that `DelayedTriggerLifetime` carries for `WhenNextEvent`), so +/// it is a small dedicated enum rather than an overload of `DelayedTriggerLifetime`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +pub enum WheneverEventExpiry { + /// DEFAULT — "this turn" / "this combat" / "until end of turn": the trigger + /// ends at the creating turn's cleanup step (CR 514.2). Every pre-existing + /// `WheneverEvent` card (Hunter's Insight, etc.) is this variant. + #[default] + EndOfTurn, + /// CR 603.7b: "until your next turn" — the trigger fires on intervening + /// (opponents') turns and ends at the START of the controller's next turn. + /// `after` is the symbolic `AfterCreationTurn` at parse time, stamped to + /// `After(creation_turn)` at delayed-trigger resolution (mirroring + /// `AtNextPhaseForPlayer.gate`). + UntilControllersNextTurn { after: TurnGate }, +} + +impl WheneverEventExpiry { + /// Serde skip-helper: `EndOfTurn` is the default and is omitted from JSON, so + /// all existing serialized card-data stays byte-identical. + pub fn is_end_of_turn(&self) -> bool { + matches!(self, WheneverEventExpiry::EndOfTurn) + } +} + /// When a delayed triggered ability fires (CR 603.7). #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "type")] @@ -3645,7 +3676,16 @@ pub enum DelayedTriggerCondition { /// until end of turn. Reuses existing trigger matching infrastructure via embedded /// TriggerDefinition. The embedded trigger's `execute` field should be `None` — /// the actual effect lives in `DelayedTrigger.ability`. - WheneverEvent { trigger: Box }, + WheneverEvent { + trigger: Box, + /// CR 603.7b: the trigger's stated duration. `EndOfTurn` (default) is + /// purged at the creating turn's cleanup; `UntilControllersNextTurn` + /// survives intervening turns and is purged at the controller's next turn + /// start (Kang Dynasty). Defaulted + skipped so existing card-data is + /// byte-identical. + #[serde(default, skip_serializing_if = "WheneverEventExpiry::is_end_of_turn")] + expiry: WheneverEventExpiry, + }, /// CR 603.7: "When you next [event] this turn" — fires once on the next matching /// event, then is removed. One-shot variant of `WheneverEvent`. /// Uses existing trigger matching infrastructure to detect the event. diff --git a/crates/engine/tests/integration/human_torch_combat_damage_redirect.rs b/crates/engine/tests/integration/human_torch_combat_damage_redirect.rs new file mode 100644 index 0000000000..050da4a87b --- /dev/null +++ b/crates/engine/tests/integration/human_torch_combat_damage_redirect.rs @@ -0,0 +1,178 @@ +//! Human Torch — gendered-pronoun delayed combat-damage rider. +//! +//! Verified Oracle text (`client/public/card-data.json`, +//! `jq '.["human torch"].oracle_text'`), second ability: +//! "Whenever Human Torch attacks, you may pay {R}{G}{W}{U}. If you do, until +//! end of turn, whenever he deals combat damage to an opponent, he deals that +//! much damage to each other opponent." +//! +//! Pins Gap A's gendered-pronoun arm: "he" (nominative, damage-verb-guarded) → +//! `SelfRef`, folded into a delayed `WheneverEvent` `DamageDone`/`CombatOnly` +//! trigger scoped to opponents, with an inner `DamageEachPlayer` over +//! `OpponentOtherThanTriggering`. A revert of the "he" arm returns `mode: Unknown`. +//! +//! The `SelfRef` delayed damage rider fires via the pre-existing per-source +//! `DamageDealt` match path (a `SelfRef` source does not listen on the aggregate +//! `CombatDamageDealtToPlayer` event — see +//! `trigger_matchers::listens_on_aggregate_combat_damage_done`), which the +//! established combat-damage trigger corpus already exercises at runtime; the new +//! surface here is confined to the parse. + +use engine::parser::oracle::parse_oracle_text; +use engine::types::ability::{ + DamageKindFilter, DelayedTriggerCondition, Effect, PlayerFilter, TargetFilter, + WheneverEventExpiry, +}; +use engine::types::triggers::TriggerMode; + +const HUMAN_TORCH_ORACLE: &str = "At the beginning of combat on your turn, if you've \ + cast a noncreature spell this turn, Human Torch gains flying, double strike, and \ + haste until end of turn.\nWhenever Human Torch attacks, you may pay {R}{G}{W}{U}. \ + If you do, until end of turn, whenever he deals combat damage to an opponent, he \ + deals that much damage to each other opponent."; + +fn find_delayed(ability: &engine::types::ability::AbilityDefinition) -> &Effect { + let mut cur = ability; + loop { + if matches!(&*cur.effect, Effect::CreateDelayedTrigger { .. }) { + return &cur.effect; + } + cur = cur + .sub_ability + .as_deref() + .expect("CreateDelayedTrigger must appear in the attack trigger's chain"); + } +} + +#[test] +fn he_folds_into_selfref_combat_damage_redirect_rider() { + let parsed = parse_oracle_text( + HUMAN_TORCH_ORACLE, + "Human Torch", + &[], + &["Legendary".to_string(), "Creature".to_string()], + &["Human".to_string()], + ); + + // The attack trigger (second ability). + let attack = parsed + .triggers + .iter() + .find(|t| t.mode == TriggerMode::Attacks) + .expect("Human Torch attacks trigger"); + let execute = attack.execute.as_ref().expect("attack trigger execute"); + + let Effect::CreateDelayedTrigger { + condition, effect, .. + } = find_delayed(execute) + else { + unreachable!("find_delayed returns a CreateDelayedTrigger"); + }; + let DelayedTriggerCondition::WheneverEvent { trigger, expiry } = condition else { + panic!("expected WheneverEvent, got {condition:?}"); + }; + + // Gap A: "he" → SelfRef source; combat-only; recipient is an opponent. + assert_eq!( + trigger.mode, + TriggerMode::DamageDone, + "not Unknown — 'he deals combat damage' parsed" + ); + assert_eq!(trigger.damage_kind, DamageKindFilter::CombatOnly); + assert_eq!( + trigger.valid_source, + Some(TargetFilter::SelfRef), + "'he' resolves to the source permanent (Human Torch)" + ); + assert!( + trigger.valid_target.is_some(), + "recipient 'to an opponent' populates valid_target" + ); + + // Gap C: no stated multi-turn duration → default EndOfTurn expiry (the + // "until end of turn" prefix is inert for the WheneverEvent — purged at + // cleanup by default). + assert_eq!(*expiry, WheneverEventExpiry::EndOfTurn); + + // Inner effect: "he deals that much damage to each OTHER opponent" → + // DamageEachPlayer over OpponentOtherThanTriggering. + match &*effect.effect { + Effect::DamageEachPlayer { player_filter, .. } => assert_eq!( + *player_filter, + PlayerFilter::OpponentOtherThanTriggering, + "each OTHER opponent (excludes the damaged opponent)" + ), + other => panic!("expected DamageEachPlayer, got {other:?}"), + } +} + +/// Negative parse sibling: a POSSESSIVE gendered subject ("his …") must NOT +/// resolve to `SelfRef`. The gendered subject arm is nominative-only ("he "/ +/// "she ") and damage-verb guarded, so a possessive form must decline. +/// +/// Reach-guard (fixes a prior vacuous negative): the probe is an INSTANT carrying +/// the " this turn, " delayed-trigger window, so `parse_oracle_text` routes it +/// through `try_parse_temporal_delayed_trigger_ability` (spell + trigger-prefix) +/// → the `WheneverEvent` delayed-trigger path with `in_delayed_trigger = true`. +/// That is the SAME `parse_single_subject` gendered branch the positive test +/// exercises, so "his commander" is genuinely evaluated and declined — the parse +/// no longer short-circuits to `Effect::Unimplemented` before the gendered arm is +/// ever reached. `find_delayed` panicking is itself the positive reach-guard: a +/// regression that fails to build the delayed `WheneverEvent` fails this test +/// rather than passing it silently. +#[test] +fn possessive_gendered_subject_does_not_become_selfref() { + let oracle = "Whenever his commander deals combat damage to a player this turn, draw a card."; + let parsed = parse_oracle_text(oracle, "Probe", &[], &["Instant".to_string()], &[]); + + // Positive reach-guard: the delayed-trigger path was reached and produced a + // `WheneverEvent` condition (not `Unimplemented`). `find_delayed` panics if no + // `CreateDelayedTrigger` is in the chain, so the SelfRef check below is never + // vacuous. + let ability = parsed + .abilities + .first() + .expect("instant delayed-trigger creates a spell ability"); + let Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::WheneverEvent { trigger, .. }, + .. + } = find_delayed(ability) + else { + panic!( + "expected CreateDelayedTrigger/WheneverEvent, got {:?}", + ability.effect + ); + }; + + // Positive shape pin (replaces a prior vacuous negative): the delayed subject + // arm evaluated "his commander" and DECLINED — possessive case is excluded from + // the gendered SelfRef arm, and "his commander" is not otherwise a recognized + // subject. The inner combat-damage trigger therefore stays coverage-honest: + // `mode` is `Unknown` carrying the original clause, and NO filter slot binds. + // This is the discriminating positive assertion — a SelfRef regression would + // instead RECOGNIZE the trigger (a concrete `DamageDone`/`CombatOnly` mode with + // `valid_source == Some(SelfRef)`), so pinning `Unknown` rejects both the + // SelfRef bug and a spurious recognized-but-wrong parse; it cannot be satisfied + // by an upstream `None` masquerading as success. + assert!( + matches!(&trigger.mode, TriggerMode::Unknown(text) if text == oracle_condition_clause()), + "possessive 'his ' must stay coverage-honest (Unknown), got mode={:?}", + trigger.mode + ); + assert_eq!( + trigger.valid_source, None, + "an unrecognized possessive subject must bind no source (not SelfRef, not Any)" + ); + assert_ne!( + trigger.valid_source, + Some(TargetFilter::SelfRef), + "possessive 'his ' must not bind SelfRef" + ); +} + +/// The exact combat-damage condition clause the delayed trigger carries for the +/// probe oracle — pinned so the `Unknown` assertion above proves the *specific* +/// clause reached classification and was left honestly unclassified. +fn oracle_condition_clause() -> &'static str { + "his commander deals combat damage to a player" +} diff --git a/crates/engine/tests/integration/hunters_insight_combat_draw.rs b/crates/engine/tests/integration/hunters_insight_combat_draw.rs index f6700ddaa2..34af60eb44 100644 --- a/crates/engine/tests/integration/hunters_insight_combat_draw.rs +++ b/crates/engine/tests/integration/hunters_insight_combat_draw.rs @@ -158,7 +158,7 @@ fn delayed_trigger_shape_covers_planeswalker_and_parent_target() { }; // The delayed trigger condition is a WheneverEvent over a DamageDone trigger. - let DelayedTriggerCondition::WheneverEvent { trigger } = condition else { + let DelayedTriggerCondition::WheneverEvent { trigger, .. } = condition else { panic!("delayed trigger condition must be WheneverEvent, got {condition:?}"); }; diff --git a/crates/engine/tests/integration/issue_3294_good_king_mog_chapter_ii.rs b/crates/engine/tests/integration/issue_3294_good_king_mog_chapter_ii.rs index eb5b4bb3d9..c803a7a5d6 100644 --- a/crates/engine/tests/integration/issue_3294_good_king_mog_chapter_ii.rs +++ b/crates/engine/tests/integration/issue_3294_good_king_mog_chapter_ii.rs @@ -70,7 +70,7 @@ fn chapter_ii_parses_to_delayed_spell_cast_trigger() { let Effect::CreateDelayedTrigger { condition, .. } = &*def.effect else { panic!("expected CreateDelayedTrigger, got {:?}", def.effect); }; - let DelayedTriggerCondition::WheneverEvent { trigger } = condition else { + let DelayedTriggerCondition::WheneverEvent { trigger, .. } = condition else { panic!("expected WheneverEvent, got {condition:?}"); }; assert_eq!( diff --git a/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs b/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs new file mode 100644 index 0000000000..cdf4d0265e --- /dev/null +++ b/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs @@ -0,0 +1,465 @@ +//! Kang Dynasty — "until your next turn" delayed combat-damage rider (Gap C). +//! +//! Verified Oracle text (`client/public/card-data.json`, +//! `jq '.["kang dynasty"].oracle_text'`), chapters I/II: +//! "For each opponent, tap up to one target creature that player controls. Goad +//! those creatures. Until your next turn, whenever any of those creatures deals +//! combat damage to a player, draw a card." +//! +//! Pins: +//! - Gap A: "any of those creatures" → `ParentTarget`, `DamageDone`/`CombatOnly`, +//! recipient a player; inner `Draw`. +//! - Gap C: the rider's stated "until your next turn" duration lands on the +//! `WheneverEvent`'s `expiry` (`UntilControllersNextTurn`), NOT the enclosing +//! ability's `duration`. This is load-bearing: goaded creatures attack on +//! opponents' turns AFTER the creating turn's cleanup (CR 701.15a), so a +//! default `EndOfTurn` `WheneverEvent` would be purged before it could fire. + +use engine::game::ability_utils::build_resolved_from_def; +use engine::game::effects::resolve_ability_chain; +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::parser::oracle_effect::parse_effect_chain; +use engine::types::ability::{ + AbilityDefinition, AbilityKind, DamageKindFilter, DelayedTriggerCondition, Effect, + QuantityExpr, TargetFilter, TargetRef, TriggerDefinition, TurnGate, WheneverEventExpiry, +}; +use engine::types::game_state::GameState; +use engine::types::phase::Phase; +use engine::types::triggers::TriggerMode; + +/// Chapters I/II effect chain (the tap/goad/rider clause). +const KANG_CHAPTER: &str = "For each opponent, tap up to one target creature that \ + player controls. Goad those creatures. Until your next turn, whenever any of \ + those creatures deals combat damage to a player, draw a card."; + +fn find_delayed(ability: &AbilityDefinition) -> &Effect { + let mut cur = ability; + loop { + if matches!(&*cur.effect, Effect::CreateDelayedTrigger { .. }) { + return &cur.effect; + } + cur = cur + .sub_ability + .as_deref() + .expect("CreateDelayedTrigger must appear in the chapter chain"); + } +} + +#[test] +fn chapter_rider_parses_until_next_turn_expiry_and_parent_target() { + let def = parse_effect_chain(KANG_CHAPTER, AbilityKind::Spell); + let Effect::CreateDelayedTrigger { + condition, effect, .. + } = find_delayed(&def) + else { + unreachable!("find_delayed returns a CreateDelayedTrigger"); + }; + let DelayedTriggerCondition::WheneverEvent { trigger, expiry } = condition else { + panic!("expected WheneverEvent, got {condition:?}"); + }; + + // Gap A. + assert_eq!(trigger.mode, TriggerMode::DamageDone, "not Unknown"); + assert_eq!(trigger.damage_kind, DamageKindFilter::CombatOnly); + assert_eq!( + trigger.valid_source, + Some(TargetFilter::ParentTarget), + "'any of those creatures' → ParentTarget" + ); + assert_eq!(trigger.valid_target, Some(TargetFilter::Player)); + + // Gap C: the duration is carried on the WheneverEvent expiry (symbolic at + // parse time), NOT on the ability's `duration`. + assert_eq!( + *expiry, + WheneverEventExpiry::UntilControllersNextTurn { + after: TurnGate::AfterCreationTurn, + }, + "'until your next turn' → WheneverEventExpiry::UntilControllersNextTurn" + ); + + // Inner effect: draw a card. + assert!( + matches!(&*effect.effect, Effect::Draw { .. }), + "inner effect draws" + ); +} + +/// Scope-evidence class guard (PR #6884): "until your next turn, whenever …" +/// delayed triggers on cards OTHER than Kang Dynasty. The parse-diff showed 12 +/// such cards whose CreateDelayedTrigger `duration` field changed — this pins the +/// two signatures so the class behavior can't silently regress: +/// - Sig 1 (Don't Move / A Display / Davriel class): a plain inner effect → +/// the "until your next turn" moves ENTIRELY to the WheneverEvent expiry and +/// the creator ability keeps no `duration`. +/// - Sig 2 (Jace, Architect of Thought / Tamiyo class): an inner "… until end +/// of turn" buff → the "until your next turn" still moves to the expiry, and +/// the residual `UntilEndOfTurn` surfaces on the creator ability. +/// +/// In BOTH cases the load-bearing fix is identical and correct: the expiry is +/// `UntilControllersNextTurn`, so the trigger fires on opponents' turns instead +/// of being purged at the creating turn's cleanup (the pre-fix default-EndOfTurn +/// behavior, CR 603.7b). +#[test] +fn until_next_turn_delayed_trigger_relocates_duration_to_expiry_across_class() { + fn delayed_ability(def: &AbilityDefinition) -> &AbilityDefinition { + let mut cur = def; + loop { + if matches!(&*cur.effect, Effect::CreateDelayedTrigger { .. }) { + return cur; + } + cur = cur + .sub_ability + .as_deref() + .expect("class fixture must contain a CreateDelayedTrigger"); + } + } + + // Sig 1: plain inner effect (Don't Move's "destroy it"). + let sig1 = parse_effect_chain( + "Until your next turn, whenever a creature becomes tapped, destroy it.", + AbilityKind::Spell, + ); + let sig1_ability = delayed_ability(&sig1); + let Effect::CreateDelayedTrigger { condition, .. } = &*sig1_ability.effect else { + unreachable!(); + }; + let DelayedTriggerCondition::WheneverEvent { expiry, .. } = condition else { + panic!("expected WheneverEvent, got {condition:?}"); + }; + assert_eq!( + *expiry, + WheneverEventExpiry::UntilControllersNextTurn { + after: TurnGate::AfterCreationTurn, + }, + "sig1: 'until your next turn' must land on the expiry" + ); + assert_eq!( + sig1_ability.duration, None, + "sig1: a plain inner effect leaves no residual duration on the creator ability" + ); + + // Sig 2: inner "… until end of turn" buff (Jace, Architect of Thought's +1). + let sig2 = parse_effect_chain( + "Until your next turn, whenever a creature an opponent controls attacks, \ + it gets -1/-0 until end of turn.", + AbilityKind::Spell, + ); + let sig2_ability = delayed_ability(&sig2); + let Effect::CreateDelayedTrigger { condition, .. } = &*sig2_ability.effect else { + unreachable!(); + }; + let DelayedTriggerCondition::WheneverEvent { expiry, .. } = condition else { + panic!("expected WheneverEvent, got {condition:?}"); + }; + assert_eq!( + *expiry, + WheneverEventExpiry::UntilControllersNextTurn { + after: TurnGate::AfterCreationTurn, + }, + "sig2: 'until your next turn' must STILL land on the expiry, not be shadowed \ + by the inner 'until end of turn'" + ); + assert_eq!( + sig2_ability.duration, + Some(engine::types::ability::Duration::UntilEndOfTurn), + "sig2: the inner buff's residual 'until end of turn' surfaces on the creator ability" + ); +} + +/// Turn-structure pump (auto no-attacks/blocks, drain trigger order, no-op +/// cleanup discard, pass priority). +fn pump(runner: &mut GameRunner) -> bool { + use engine::types::game_state::WaitingFor; + use engine::types::GameAction; + match runner.state().waiting_for.clone() { + WaitingFor::DeclareAttackers { .. } => runner + .act(GameAction::DeclareAttackers { + attacks: vec![], + bands: vec![], + }) + .is_ok(), + WaitingFor::DeclareBlockers { .. } => runner + .act(GameAction::DeclareBlockers { + assignments: vec![], + }) + .is_ok(), + WaitingFor::OrderTriggers { .. } => { + engine::game::triggers::drain_order_triggers_with_identity(runner.state_mut()); + true + } + WaitingFor::DiscardChoice { .. } => runner + .act(GameAction::SelectCards { cards: vec![] }) + .is_ok(), + WaitingFor::Priority { .. } => runner.act(GameAction::PassPriority).is_ok(), + _ => false, + } +} + +fn advance_past_turn(runner: &mut GameRunner, from_turn: u32) { + for _ in 0..400 { + if runner.state().turn_number > from_turn { + return; + } + if !pump(runner) { + break; + } + } + assert!( + runner.state().turn_number > from_turn, + "stalled advancing past turn {from_turn} (now turn {}, phase {:?})", + runner.state().turn_number, + runner.state().phase + ); +} + +/// Build the Kang-shaped rider programmatically (its shape is pinned by the +/// parse test above) and install it via the production `resolve_ability_chain`. +fn install_kang_rider(state: &mut GameState, source: engine::types::identifiers::ObjectId) { + let mut trigger = TriggerDefinition::new(TriggerMode::DamageDone); + trigger.damage_kind = DamageKindFilter::CombatOnly; + // SelfRef source keeps the empty-set guard out of scope (it applies only to a + // pre-bind ParentTarget source); Gap C retention/purge is independent of the + // source filter. + trigger.valid_source = Some(TargetFilter::SelfRef); + trigger.valid_target = Some(TargetFilter::Player); + let inner = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ); + let def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::WheneverEvent { + trigger: Box::new(trigger), + expiry: WheneverEventExpiry::UntilControllersNextTurn { + after: TurnGate::AfterCreationTurn, + }, + }, + effect: Box::new(inner), + uses_tracked_set: false, + }, + ); + let resolved = build_resolved_from_def(&def, source, P0); + let mut events = Vec::new(); + resolve_ability_chain(state, &resolved, &mut events, 0) + .expect("Kang rider installs via resolve_ability_chain"); +} + +/// Gap C (load-bearing): an "until your next turn" `WheneverEvent` SURVIVES the +/// creating turn's cleanup (retention disjunct) and is PURGED at the controller's +/// next turn's untap. Reverting the retention disjunct drops it at the creating +/// turn's cleanup (fails the survives-assert); reverting the untap purge leaks it +/// forever (fails the purged-assert). +#[test] +fn until_next_turn_rider_survives_intervening_turn_then_purges() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let source = scenario + .add_enchantment_from_oracle(P0, "Kang Dynasty", "Enchantment.") + .id(); + // Stock both libraries so multi-turn draw steps don't cause a draw-from-empty + // loss that would end the game before the purge boundary. + for i in 0..12 { + scenario.add_card_to_library_top(P0, &format!("P0 Lib {i}")); + scenario.add_card_to_library_top(P1, &format!("P1 Lib {i}")); + } + let mut runner = scenario.build(); + + let creation_turn = runner.state().turn_number; + install_kang_rider(runner.state_mut(), source); + assert_eq!( + runner.state().delayed_triggers.len(), + 1, + "rider installed on the creating turn" + ); + + // Cross P0's cleanup into the opponent's (intervening) turn. + advance_past_turn(&mut runner, creation_turn); + assert_eq!( + runner.state().delayed_triggers.len(), + 1, + "CR 603.7b: an until-your-next-turn WheneverEvent survives the creating \ + turn's cleanup (fires on the opponent's turn)" + ); + + // Cross the opponent's turn into P0's next turn (untap purges it). + let opponent_turn = runner.state().turn_number; + advance_past_turn(&mut runner, opponent_turn); + assert_eq!( + runner.state().active_player, + P0, + "advanced to the controller's next turn" + ); + assert!( + runner.state().delayed_triggers.is_empty(), + "CR 603.7b / CR 502.4: the rider is purged at the controller's next turn's \ + untap step (does not leak into later turns)" + ); +} + +fn hand_len(runner: &GameRunner, player: engine::types::player::PlayerId) -> usize { + runner + .state() + .players + .iter() + .find(|p| p.id == player) + .map(|p| p.hand.len()) + .unwrap_or(0) +} + +/// Advance (auto-passing/declining everything) until `player` is the active player +/// AND the engine is waiting for them to declare attackers — WITHOUT auto-declining +/// that declaration, so the caller can drive a real attack. +fn advance_to_declare_attackers(runner: &mut GameRunner, player: engine::types::player::PlayerId) { + use engine::types::game_state::WaitingFor; + for _ in 0..400 { + if runner.state().active_player == player + && matches!( + runner.state().waiting_for, + WaitingFor::DeclareAttackers { .. } + ) + { + return; + } + if !pump(runner) { + break; + } + } + panic!( + "never reached {player:?}'s declare-attackers (now turn {}, active {:?}, phase {:?}, wf {:?})", + runner.state().turn_number, + runner.state().active_player, + runner.state().phase, + runner.state().waiting_for, + ); +} + +/// Gap C FIRING through the FULL PRODUCTION PARSER→RESOLVER SEAM (HIGH review +/// follow-up): resolves Kang's ACTUAL parsed chapter — "For each opponent, tap up +/// to one target creature that player controls. Goad those creatures. Until your +/// next turn, whenever any of those creatures deals combat damage to a player, +/// draw a card." — supplying a real chosen creature as the tap target. This +/// exercises the complete pipeline the PR changes: `parse_effect_chain` → the +/// tap/goad clauses → parent-target propagation of the chosen creature into the +/// delayed trigger's `ParentTarget` source → the delayed-trigger resolver's expiry +/// stamping. Then it drives that goaded creature through unblocked combat against +/// the controller on the intervening (opponent's) turn and asserts the controller +/// DREW, then crosses into the controller's next turn and proves the rider expired. +/// +/// A revert of the `UntilControllersNextTurn` expiry purges the rider at the +/// creating turn's cleanup so it never fires (draw assertion fails); a revert of +/// the untap purge leaks it (expiry assertion fails); a break in parent-target +/// propagation leaves the rider's source unbound (`Any` → the empty-set guard +/// skips install, or it over-fires) — none of which the synthetic fixture caught. +#[test] +fn kang_parsed_chapter_rider_fires_on_intervening_turn_then_expires() { + use super::rules::AttackTarget; + use engine::types::GameAction; + + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let source = scenario + .add_enchantment_from_oracle(P0, "Kang Dynasty", "Enchantment.") + .id(); + // P1's creature is the "target creature that player controls" — tapped and + // goaded by the chapter, and the rider's `ParentTarget` source. It attacks P0 + // (its controller's opponent, per goad) on P1's turn. + let goaded = scenario.add_creature(P1, "Goaded Bear", 2, 2).id(); + for i in 0..12 { + scenario.add_card_to_library_top(P0, &format!("P0 Lib {i}")); + scenario.add_card_to_library_top(P1, &format!("P1 Lib {i}")); + } + let mut runner = scenario.build(); + + // Resolve Kang's PARSED chapter chain (not a synthetic rider): the chosen + // creature is supplied as the tap target and must propagate to the rider's + // `ParentTarget` source through the intervening tap/goad clauses. + let def = parse_effect_chain(KANG_CHAPTER, AbilityKind::Spell); + let mut resolved = build_resolved_from_def(&def, source, P0); + resolved.targets = vec![TargetRef::Object(goaded)]; + let mut events = Vec::new(); + resolve_ability_chain(runner.state_mut(), &resolved, &mut events, 0) + .expect("Kang parsed chapter resolves"); + + // The parsed chain resolved end to end: the target was tapped + goaded and + // exactly one rider installed, bound to that creature (not `Any`). + assert_eq!( + runner.state().delayed_triggers.len(), + 1, + "parsed chapter installed exactly one rider" + ); + assert!( + runner.state().objects[&goaded].tapped, + "the parsed tap/goad clause tapped the chosen creature" + ); + let DelayedTriggerCondition::WheneverEvent { trigger, .. } = + &runner.state().delayed_triggers[0].condition + else { + panic!("expected a WheneverEvent rider"); + }; + assert_eq!( + trigger.valid_source, + Some(TargetFilter::SpecificObject { id: goaded }), + "parent-target propagation bound the rider's source to the chosen creature \ + (not Any, not unbound)" + ); + + // Advance into P1's (intervening) turn, stopping at P1's declare-attackers. + advance_to_declare_attackers(&mut runner, P1); + let p0_hand_before = hand_len(&runner, P0); + let p0_life_before = runner.life(P0); + + // P1's goaded creature attacks P0, unblocked (P0 controls no creatures). + runner + .act(GameAction::DeclareAttackers { + attacks: vec![(goaded, AttackTarget::Player(P0))], + bands: vec![], + }) + .expect("P1 declares the goaded attacker against P0"); + + // Drive blockers/combat-damage/the fired rider through the production pump + // until the controller draws (the rider resolved) or the flow stalls. + let mut drew = false; + for _ in 0..200 { + if hand_len(&runner, P0) > p0_hand_before { + drew = true; + break; + } + if !pump(&mut runner) { + break; + } + } + + assert!( + drew, + "the ParentTarget rider FIRED on the opponent's turn: the goaded creature \ + dealt combat damage to a player, so the controller drew a card" + ); + assert_eq!( + hand_len(&runner, P0), + p0_hand_before + 1, + "exactly one draw from the single combat-damage occurrence" + ); + assert_eq!( + runner.life(P0), + p0_life_before - 2, + "CR 120.2a: the goaded 2/2 dealt its combat damage to P0 (attack resolved)" + ); + + // Cross into the controller's next turn: the multi-fire rider is purged at untap. + let opponent_turn = runner.state().turn_number; + advance_past_turn(&mut runner, opponent_turn); + assert_eq!( + runner.state().active_player, + P0, + "advanced to the controller's next turn" + ); + assert!( + runner.state().delayed_triggers.is_empty(), + "CR 603.7b / CR 502.4: the rider is purged at the controller's next turn's untap" + ); +} diff --git a/crates/engine/tests/integration/love_on_the_battlefield_combat_counters.rs b/crates/engine/tests/integration/love_on_the_battlefield_combat_counters.rs new file mode 100644 index 0000000000..31ec494b78 --- /dev/null +++ b/crates/engine/tests/integration/love_on_the_battlefield_combat_counters.rs @@ -0,0 +1,418 @@ +//! Love on the Battlefield — batched-attack delayed combat-damage rider. +//! +//! Verified Oracle text (`client/public/card-data.json`, +//! `jq '.["love on the battlefield"].oracle_text'`): +//! "Whenever you attack with exactly two creatures, those creatures gain first +//! strike until end of turn, then draw a card. Whenever either of those +//! creatures deals combat damage to a player this combat, put a +1/+1 counter +//! on it." +//! +//! Exercises the full mechanic: +//! - Gap B: the "attack with exactly two creatures" count constraint +//! (`AttackersDeclaredCount { comparator: EQ, count: 2 }`). +//! - Gap A: the second sentence folds into a delayed `WheneverEvent` whose +//! source anaphor "either of those creatures" → `ParentTarget` (the declared +//! attackers, seeded by `seed_batched_attack_parent_targets`). +//! - The "it" antecedent in "put a +1/+1 counter on it" → `TriggeringSource` +//! (the creature that dealt combat damage), NOT `SelfRef` (the enchantment). + +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::parser::oracle::parse_oracle_text; +use engine::types::ability::{ + Comparator, DelayedTriggerCondition, Effect, TargetFilter, TriggerCondition, +}; +use engine::types::counter::CounterType; +use engine::types::identifiers::ObjectId; +use engine::types::keywords::Keyword; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; +use engine::types::triggers::TriggerMode; + +use super::rules::{run_combat, AttackTarget, GameAction, WaitingFor}; + +const P2: PlayerId = PlayerId(2); + +const LOVE_ORACLE: &str = "Whenever you attack with exactly two creatures, those \ + creatures gain first strike until end of turn, then draw a card. Whenever \ + either of those creatures deals combat damage to a player this combat, put a \ + +1/+1 counter on it."; + +fn counters(runner: &GameRunner, id: ObjectId) -> u32 { + runner + .state() + .objects + .get(&id) + .and_then(|o| o.counters.get(&CounterType::Plus1Plus1).copied()) + .unwrap_or(0) +} + +fn hand_count(runner: &GameRunner, player: PlayerId) -> usize { + runner + .state() + .players + .iter() + .find(|p| p.id == player) + .map(|p| p.hand.len()) + .unwrap_or(0) +} + +fn stock_library(scenario: &mut GameScenario) { + for name in ["Lib 1", "Lib 2", "Lib 3", "Lib 4"] { + scenario.add_card_to_library_top(P0, name); + } +} + +/// End-to-end (A2): two attackers both deal combat damage to a player → EACH +/// gains exactly one +1/+1 counter ON ITSELF, the first-strike grant applied +/// (reach-guard proving the batched-attack seeding worked), and the controller +/// drew a card. +#[test] +fn two_attackers_each_get_one_counter_on_themselves() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.add_enchantment_from_oracle(P0, "Love on the Battlefield", LOVE_ORACLE); + let bear_a = scenario.add_creature(P0, "Bear A", 2, 2).id(); + let bear_b = scenario.add_creature(P0, "Bear B", 2, 2).id(); + stock_library(&mut scenario); + let mut runner = scenario.build(); + + let hand_before = hand_count(&runner, P0); + let life_before = runner.life(P1); + + run_combat(&mut runner, vec![bear_a, bear_b], vec![]); + runner.advance_until_stack_empty(); + + // Reach-guard: the first-strike grant landed on the attackers. Without the + // batched-attack `ParentTarget` seeding (`effect_uses_parent_target` + + // `seed_batched_attack_parent_targets`) this grant would silently drop. + assert!( + runner.state().objects[&bear_a].has_keyword(&Keyword::FirstStrike), + "Bear A must have gained first strike (proves attacker seeding)" + ); + + // CR 121.1: the "then draw a card" rider drew for the controller. + assert_eq!( + hand_count(&runner, P0), + hand_before + 1, + "controller drew a card from the first ability" + ); + + // CR 120.2a: both 2/2s dealt combat damage to P1. + assert_eq!( + runner.life(P1), + life_before - 4, + "both attackers hit P1 for 2" + ); + + // The discriminating assertion: each attacker got exactly ONE +1/+1 counter, + // on ITSELF. If "it" bound to `SelfRef` the counters would land on the + // enchantment (neither bear); if the source bound to `Any` a wrong count + // could appear; if `TriggeringSource` didn't re-resolve per firing both + // counters would land on one creature. + assert_eq!( + counters(&runner, bear_a), + 1, + "Bear A gets exactly one counter" + ); + assert_eq!( + counters(&runner, bear_b), + 1, + "Bear B gets exactly one counter" + ); + + // The enchantment itself must NOT receive a counter ("it" is the creature, + // not the source permanent). + let love = runner + .state() + .objects + .values() + .find(|o| o.name == "Love on the Battlefield") + .expect("Love present") + .id; + assert_eq!(counters(&runner, love), 0, "enchantment gets no counter"); +} + +/// Multiplayer discrimination (CR 603.2c + CR 510.2): the two attackers attack +/// DIFFERENT opponents, so one simultaneous combat-damage step emits TWO aggregate +/// `CombatDamageDealtToPlayer` events at once — one per defending player. Each +/// attacker must still get exactly one +1/+1 counter on itself. Before the +/// multi-defender expansion fix, the delayed rider only expanded the FIRST +/// aggregate (`.find()`), so the creature that hit the second defender silently +/// got no counter — this test's `counters(bear_b) == 1` assertion flips (fails) +/// when the fix is reverted. +#[test] +fn attackers_against_two_different_opponents_each_get_a_counter() { + let mut scenario = GameScenario::new_n_player(3, 71); + scenario.at_phase(Phase::PreCombatMain); + scenario.add_enchantment_from_oracle(P0, "Love on the Battlefield", LOVE_ORACLE); + let bear_a = scenario.add_creature(P0, "Bear A", 2, 2).id(); + let bear_b = scenario.add_creature(P0, "Bear B", 2, 2).id(); + stock_library(&mut scenario); + let mut runner = scenario.build(); + + let p1_life_before = runner.life(P1); + let p2_life_before = runner.life(P2); + + // bear_a attacks P1, bear_b attacks P2 — two defending players in one attack. + runner.advance_to_combat(); + runner + .declare_attackers(&[ + (bear_a, AttackTarget::Player(P1)), + (bear_b, AttackTarget::Player(P2)), + ]) + .expect("declare two attackers against different opponents"); + + // Drive the YouAttack trigger, the (empty) declare-blockers step, and the + // combat-damage step. P1/P2 control no creatures, so there are no blockers and + // no interactive damage assignment. A bounded priority loop (multiplayer-safe, + // unlike `run_combat`'s 2-player `pass_both_players`) carries the turn through + // combat; the harness may batch the combat-damage delayed-trigger firing with + // the phase advance to the postcombat main phase, so `advance_until_stack_empty` + // afterward resolves the two rider triggers the combat-damage step placed on + // the stack (each puts a +1/+1 counter on its damaging creature). + for _ in 0..80 { + if matches!( + runner.state().phase, + Phase::PostCombatMain | Phase::End | Phase::Cleanup + ) { + break; + } + match &runner.state().waiting_for { + WaitingFor::OrderTriggers { triggers, .. } => { + let order: Vec = (0..triggers.len()).collect(); + let _ = runner.act(GameAction::OrderTriggers { order }); + } + WaitingFor::DeclareBlockers { .. } => { + let _ = runner.act(GameAction::DeclareBlockers { + assignments: vec![], + }); + } + _ => { + if runner.act(GameAction::PassPriority).is_err() { + break; + } + } + } + } + runner.advance_until_stack_empty(); + + // Reach-guard: both defenders took 2 combat damage — both aggregate + // `CombatDamageDealtToPlayer` events genuinely fired, so the counters below + // cannot be attributed to a no-combat path. + assert_eq!( + runner.life(P1), + p1_life_before - 2, + "bear_a dealt 2 combat damage to P1" + ); + assert_eq!( + runner.life(P2), + p2_life_before - 2, + "bear_b dealt 2 combat damage to P2" + ); + + // The discriminating assertions: EACH attacker gets exactly one +1/+1 counter, + // even though they hit different defenders (separate aggregate events). + assert_eq!( + counters(&runner, bear_a), + 1, + "attacker vs P1 gets its counter" + ); + assert_eq!( + counters(&runner, bear_b), + 1, + "attacker vs P2 gets its counter (reverting the multi-defender fix drops this)" + ); +} + +/// Negative reach-guard: a third creature that does NOT attack receives no +/// counter (the rider fires only for members of the declared-attacker set that +/// actually dealt combat damage). +#[test] +fn non_attacking_creature_gets_no_counter() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.add_enchantment_from_oracle(P0, "Love on the Battlefield", LOVE_ORACLE); + let bear_a = scenario.add_creature(P0, "Bear A", 2, 2).id(); + let bear_b = scenario.add_creature(P0, "Bear B", 2, 2).id(); + let bench = scenario.add_creature(P0, "Bench Bear", 2, 2).id(); + stock_library(&mut scenario); + let mut runner = scenario.build(); + + run_combat(&mut runner, vec![bear_a, bear_b], vec![]); + runner.advance_until_stack_empty(); + + assert_eq!(counters(&runner, bench), 0, "non-attacker gets no counter"); + assert_eq!( + counters(&runner, bear_a), + 1, + "attacker A still gets its counter" + ); +} + +/// Gap B discrimination: attacking with THREE creatures does not satisfy the +/// "exactly two" (`Comparator::EQ`) constraint, so the whole ability does not +/// fire — no first strike, no draw, no counters. If the constraint were dropped +/// (`constraint: None`, the pre-fix behavior) or read as GE, this would fire. +#[test] +fn three_attackers_do_not_satisfy_exactly_two() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.add_enchantment_from_oracle(P0, "Love on the Battlefield", LOVE_ORACLE); + let a = scenario.add_creature(P0, "A", 2, 2).id(); + let b = scenario.add_creature(P0, "B", 2, 2).id(); + let c = scenario.add_creature(P0, "C", 2, 2).id(); + stock_library(&mut scenario); + let mut runner = scenario.build(); + + let hand_before = hand_count(&runner, P0); + let life_before = runner.life(P1); + run_combat(&mut runner, vec![a, b, c], vec![]); + runner.advance_until_stack_empty(); + + // CR 120.2a (positive reach-guard): the three 2/2s actually attacked and dealt + // combat damage to P1 (6 total). This proves the attack RESOLVED and the + // `Comparator::EQ`/`count: 2` constraint was genuinely exercised on a real + // combat — the no-draw/no-counter/no-first-strike assertions below are the + // constraint declining, not a combat that never happened. + assert_eq!( + runner.life(P1), + life_before - 6, + "all three attackers hit P1 for 2 (attack resolved)" + ); + + assert_eq!( + hand_count(&runner, P0), + hand_before, + "exactly-two constraint not met by 3 attackers → no draw" + ); + assert_eq!(counters(&runner, a), 0, "no counter — ability did not fire"); + assert!( + !runner.state().objects[&a].has_keyword(&Keyword::FirstStrike), + "no first strike — ability did not fire on a 3-creature attack" + ); +} + +/// Parse-shape pins (Gap A + Gap B): the outer trigger enforces +/// `AttackersDeclaredCount { EQ, 2 }`, and the folded delayed trigger is +/// `DamageDone`/`CombatOnly` with `valid_source: ParentTarget`, `valid_target: +/// Player`, and an inner `PutCounter` on `TriggeringSource`. Complements the +/// runtime tests: a revert of Gap A returns `mode: Unknown`, and a revert of the +/// "it" fix returns `SelfRef`. +#[test] +fn parse_shape_exactly_two_and_delayed_damage_rider() { + let parsed = parse_oracle_text( + LOVE_ORACLE, + "Love on the Battlefield", + &[], + &["Enchantment".to_string()], + &[], + ); + let trigger = parsed + .triggers + .iter() + .find(|t| t.mode == TriggerMode::YouAttack) + .expect("YouAttack trigger"); + + // Gap B: the exactly-two count constraint. + match trigger + .condition + .as_ref() + .expect("count constraint present") + { + TriggerCondition::AttackersDeclaredCount { + comparator, count, .. + } => { + assert_eq!(*comparator, Comparator::EQ, "exactly → EQ"); + assert_eq!(*count, 2, "two"); + } + other => panic!("expected AttackersDeclaredCount, got {other:?}"), + } + + // Walk the effect chain to the folded CreateDelayedTrigger. + let execute = trigger.execute.as_ref().expect("execute present"); + let mut ability = execute.as_ref(); + let delayed = loop { + if let Effect::CreateDelayedTrigger { + condition, effect, .. + } = &*ability.effect + { + break (condition, effect); + } + ability = ability + .sub_ability + .as_deref() + .expect("CreateDelayedTrigger must appear in the effect chain"); + }; + let (condition, inner) = delayed; + let DelayedTriggerCondition::WheneverEvent { + trigger: inner_trigger, + .. + } = condition + else { + panic!("delayed condition must be WheneverEvent, got {condition:?}"); + }; + assert_eq!( + inner_trigger.mode, + TriggerMode::DamageDone, + "Gap A: rider is DamageDone, not Unknown" + ); + assert_eq!( + inner_trigger.valid_source, + Some(TargetFilter::ParentTarget), + "source anaphor 'either of those creatures' → ParentTarget" + ); + assert_eq!( + inner_trigger.valid_target, + Some(TargetFilter::Player), + "recipient is a player" + ); + + // Inner "it" → TriggeringSource (the damaging creature). + match &*inner.effect { + Effect::PutCounter { target, .. } => assert_eq!( + *target, + TargetFilter::TriggeringSource, + "'it' must be TriggeringSource (the creature that dealt damage)" + ), + other => panic!("expected PutCounter, got {other:?}"), + } +} + +/// Scope-evidence class guard (PR #6884, signatures 3 & 4): the "attack with +/// exactly two creatures" recognition is a CLASS fix, not a one-off for Love on +/// the Battlefield. Alluring Suitor is the sibling card whose YouAttack trigger +/// gained the same `AttackersDeclaredCount { EQ, 2 }` condition and `you` target +/// scope. Before the fix the constraint was dropped, so the transform over-fired +/// on any attack; pinning it here proves the class improvement and guards the +/// sibling from regressing independently of Love's runtime tests. +#[test] +fn parse_shape_alluring_suitor_exactly_two_attack_constraint() { + // Verified Oracle text (Scryfall, front face): "When you attack with exactly + // two creatures, transform this creature." + let parsed = parse_oracle_text( + "When you attack with exactly two creatures, transform this creature.", + "Alluring Suitor", + &[], + &["Creature".to_string()], + &[], + ); + let trigger = parsed + .triggers + .iter() + .find(|t| t.mode == TriggerMode::YouAttack) + .expect("YouAttack trigger"); + + match trigger + .condition + .as_ref() + .expect("exactly-two count constraint present (not dropped → no over-fire)") + { + TriggerCondition::AttackersDeclaredCount { + comparator, count, .. + } => { + assert_eq!(*comparator, Comparator::EQ, "exactly → EQ"); + assert_eq!(*count, 2, "two"); + } + other => panic!("expected AttackersDeclaredCount, got {other:?}"), + } +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 18d62e8b0f..cf8c6cdd40 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -1058,6 +1058,7 @@ mod granted_ability_self_binding; mod grenzo_havoc_raiser_modal_2346; mod greymond_avacyns_stalwart; mod heartwood_storyteller_opponents_draw; +mod human_torch_combat_damage_redirect; mod i_call_for_slaughter; mod identity_crisis_multi_zone_exile; mod incredible_hulk_enrage_attacking; @@ -1068,6 +1069,7 @@ mod issue_4937_iona_chosen_color; mod issue_4945_zada_hedron_grinder; mod issue_4948_samwise_gamgee_sacrifice_target_order; mod issue_5263_chaos_warp; +mod kang_dynasty_until_next_turn_rider; mod karplusan_yeti_fight_back; mod kav_landseeker_delayed_sacrifice; mod kellan_daring_traveler_mana_value_gate; @@ -1090,6 +1092,7 @@ mod loop_shortcut_mana_engine; mod lose_control_this_turn_delayed_trigger; mod lost_mine_fungi_cavern_duration_runtime; mod lost_mine_storeroom_targeting_runtime; +mod love_on_the_battlefield_combat_counters; mod loyalty_ability_activated_trigger; mod loyalty_replacement_order_resume; mod mad_mage_lost_level_scry_runtime; diff --git a/crates/engine/tests/integration/stensian_sanguinist_prepare.rs b/crates/engine/tests/integration/stensian_sanguinist_prepare.rs index d7b6fd459f..e5a4db49eb 100644 --- a/crates/engine/tests/integration/stensian_sanguinist_prepare.rs +++ b/crates/engine/tests/integration/stensian_sanguinist_prepare.rs @@ -106,7 +106,7 @@ fn stensian_sanguinist_becomes_prepared_on_combat_damage() { // at resolution); its EFFECT prepares Stensian itself ("this creature // becomes prepared" → SelfRef, the source — NOT the targeted attacker). let installed = runner.state().delayed_triggers.iter().any(|dt| { - let DelayedTriggerCondition::WheneverEvent { trigger } = &dt.condition else { + let DelayedTriggerCondition::WheneverEvent { trigger, .. } = &dt.condition else { return false; }; trigger.valid_source == Some(TargetFilter::SpecificObject { id: attacker }) diff --git a/crates/mtgish-import/src/convert/action.rs b/crates/mtgish-import/src/convert/action.rs index 59bcacfe23..45d6cd851b 100644 --- a/crates/mtgish-import/src/convert/action.rs +++ b/crates/mtgish-import/src/convert/action.rs @@ -5474,6 +5474,11 @@ fn build_create_trigger_until( Ok(Effect::CreateDelayedTrigger { condition: DelayedTriggerCondition::WheneverEvent { trigger: Box::new(trigger_def), + // CR 603.7b: only `Expiration::UntilEndOfTurn` reaches here (others + // strict-fail above), so the trigger ends at cleanup — the default + // `EndOfTurn` expiry. Mapping other mtgish expirations onto the new + // `UntilControllersNextTurn` slot is deferred mtgish-coverage work. + expiry: engine::types::ability::WheneverEventExpiry::EndOfTurn, }, effect: Box::new(body_ability), uses_tracked_set: false,