From 641bbf10d1395a04e8963cba40c51b4060af1fbf Mon Sep 17 00:00:00 2001 From: Jacob Woodson Date: Sun, 2 Aug 2026 05:45:10 -0500 Subject: [PATCH 01/11] Partial: Add Delayed/created "deals combat damage to a player/opponent" trigger (CreateDelayedTrigger + WheneverEvent) with tracked-set/self source binding mechanic --- ...c24-a7a6-d1f9f7693f55scratchpadantman.json | 169 ++++++++ crates/engine/src/game/coverage.rs | 1 + .../src/game/effects/delayed_trigger.rs | 212 ++++++++-- crates/engine/src/game/triggers.rs | 100 ++++- crates/engine/src/game/turns.rs | 49 ++- .../engine/src/parser/oracle_effect/lower.rs | 1 + crates/engine/src/parser/oracle_effect/mod.rs | 70 +++- .../engine/src/parser/oracle_effect/tests.rs | 2 +- crates/engine/src/parser/oracle_ir/context.rs | 9 + crates/engine/src/parser/oracle_tests.rs | 6 +- crates/engine/src/parser/oracle_trigger.rs | 115 +++++- .../engine/src/parser/oracle_trigger_tests.rs | 8 +- crates/engine/src/parser/swallow_check.rs | 11 + crates/engine/src/parser/swallow_evidence.rs | 18 + crates/engine/src/types/ability.rs | 42 +- .../human_torch_combat_damage_redirect.rs | 155 ++++++++ .../hunters_insight_combat_draw.rs | 2 +- .../issue_3294_good_king_mog_chapter_ii.rs | 2 +- .../kang_dynasty_until_next_turn_rider.rs | 220 +++++++++++ ...love_on_the_battlefield_combat_counters.rs | 367 ++++++++++++++++++ crates/engine/tests/integration/main.rs | 3 + .../stensian_sanguinist_prepare.rs | 2 +- crates/mtgish-import/src/convert/action.rs | 5 + 23 files changed, 1513 insertions(+), 56 deletions(-) create mode 100644 UsersjacobAppDataLocalTempclaudeC--Users-jacob-source-repos-phase-card-runs!a248f4-b4f3-4c24-a7a6-d1f9f7693f55scratchpadantman.json create mode 100644 crates/engine/tests/integration/human_torch_combat_damage_redirect.rs create mode 100644 crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs create mode 100644 crates/engine/tests/integration/love_on_the_battlefield_combat_counters.rs diff --git a/UsersjacobAppDataLocalTempclaudeC--Users-jacob-source-repos-phase-card-runs!a248f4-b4f3-4c24-a7a6-d1f9f7693f55scratchpadantman.json b/UsersjacobAppDataLocalTempclaudeC--Users-jacob-source-repos-phase-card-runs!a248f4-b4f3-4c24-a7a6-d1f9f7693f55scratchpadantman.json new file mode 100644 index 0000000000..4ead0e4533 --- /dev/null +++ b/UsersjacobAppDataLocalTempclaudeC--Users-jacob-source-repos-phase-card-runs!a248f4-b4f3-4c24-a7a6-d1f9f7693f55scratchpadantman.json @@ -0,0 +1,169 @@ +{ + "name": "Ant-Man, Elusive Avenger", + "mana_cost": { + "type": "Cost", + "shards": [ + "Blue", + "Red" + ], + "generic": 1 + }, + "card_type": { + "supertypes": [ + "Legendary" + ], + "core_types": [ + "Creature" + ], + "subtypes": [ + "Human", + "Rogue", + "Hero" + ] + }, + "power": { + "type": "Fixed", + "value": 1 + }, + "toughness": { + "type": "Fixed", + "value": 2 + }, + "loyalty": null, + "defense": null, + "oracle_text": "Ant-Man can't be blocked by creatures with greater power.\nWhenever Ant-Man deals combat damage to a player, create that many Treasure tokens.", + "non_ability_text": null, + "flavor_name": null, + "keywords": [], + "abilities": [], + "triggers": [ + { + "mode": "DamageDone", + "execute": { + "kind": "Spell", + "effect": { + "type": "Token", + "name": "Treasure", + "power": { + "type": "Fixed", + "value": 0 + }, + "toughness": { + "type": "Fixed", + "value": 0 + }, + "types": [ + "Artifact", + "Treasure" + ], + "colors": [], + "keywords": [], + "tapped": false, + "count": { + "type": "Ref", + "qty": { + "type": "EventContextAmount" + } + }, + "owner": { + "type": "Controller" + }, + "enters_attacking": false + }, + "cost": null, + "sub_ability": null, + "duration": null, + "description": null, + "target_prompt": null, + "condition": null, + "optional_targeting": false, + "optional": false, + "forward_result": false + }, + "valid_card": null, + "origin": null, + "destination": null, + "trigger_zones": [ + "Battlefield" + ], + "phase": null, + "optional": false, + "damage_kind": "CombatOnly", + "secondary": false, + "valid_target": { + "type": "Player" + }, + "valid_source": { + "type": "SelfRef" + }, + "description": "Whenever ~ deals combat damage to a player, create that many Treasure tokens.", + "constraint": null, + "condition": null, + "batched": false + } + ], + "static_abilities": [ + { + "mode": { + "CantBeBlockedBy": { + "filter": { + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": null, + "properties": [ + { + "type": "PowerGTSource" + } + ] + } + } + }, + "affected": { + "type": "SelfRef" + }, + "modifications": [], + "condition": null, + "affected_zone": null, + "effect_zone": null, + "active_zones": [], + "characteristic_defining": false, + "description": "~ can't be blocked by creatures with greater power." + } + ], + "replacements": [], + "color_override": [ + "Red", + "Blue" + ], + "color_identity": [ + "Red", + "Blue" + ], + "scryfall_oracle_id": "f334087a-deda-4e9e-ab58-68f7e226d7ce", + "brawl_commander": true, + "is_commander": true, + "metadata": { + "related_token_ids": [ + "50aae3ed-fa82-5258-9a21-31f5988f6b8f" + ], + "source_printing_ids": [ + "18e2b0a4-0ed3-48e6-8334-198a19e54bff", + "3a38b622-4129-4d84-908f-35c223c7c61b" + ] + }, + "legalities": { + "commander": "legal", + "duel": "legal", + "legacy": "legal", + "oathbreaker": "legal", + "vintage": "legal" + }, + "printings": [ + "MSC" + ], + "rarities": [ + "rare" + ] +} \ No newline at end of file diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 8fe3233585..4a4027b10d 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -12632,6 +12632,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 5083bbabdf..d24dec45ac 100644 --- a/crates/engine/src/game/effects/delayed_trigger.rs +++ b/crates/engine/src/game/effects/delayed_trigger.rs @@ -64,8 +64,46 @@ 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 source + // ("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 + // source 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 `parent_targets_filter(&[]) => TargetFilter::Any` (below) would + // OVER-FIRE on every creature's combat damage. Scoped to a pre-bind + // `ParentTarget` source only, so a `SelfRef` source (Human Torch's "he", whose + // empty `ability.targets` is normal) still installs. + if let DelayedTriggerCondition::WheneverEvent { trigger, .. } = &condition { + if matches!(trigger.valid_source, Some(TargetFilter::ParentTarget)) + && ability.targets.is_empty() + { + 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,31 +164,47 @@ 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.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, - // which is correct for ParentTarget (Flickerwisp) but wrong here. - crate::game::targeting::resolve_event_context_target( - state, - &crate::types::ability::TargetFilter::TriggeringSource, - ability.source_id, - ) - .map(|t| vec![t]) - .unwrap_or_default() - } else if super::ability_refs_parent_target(&delayed_ability) { - parent_target_snapshot(state, ability) - } else if effect_references_last_created(&delayed_ability.effect) - && !state.last_created_token_ids.is_empty() - { - state - .last_created_token_ids - .iter() - .map(|&id| TargetRef::Object(id)) - .collect() - } else { - vec![] - }; + // + // 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_ref` (a WheneverEvent) forces per-firing event-context + // resolution instead. + let one_shot_ref = !matches!( + condition, + crate::types::ability::DelayedTriggerCondition::WheneverEvent { .. } + ); + let snapshot_targets = + if one_shot_ref && 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, + // which is correct for ParentTarget (Flickerwisp) but wrong here. + crate::game::targeting::resolve_event_context_target( + state, + &crate::types::ability::TargetFilter::TriggeringSource, + ability.source_id, + ) + .map(|t| vec![t]) + .unwrap_or_default() + } else if super::ability_refs_parent_target(&delayed_ability) { + parent_target_snapshot(state, ability) + } else if effect_references_last_created(&delayed_ability.effect) + && !state.last_created_token_ids.is_empty() + { + state + .last_created_token_ids + .iter() + .map(|&id| TargetRef::Object(id)) + .collect() + } else { + vec![] + }; if super::ability_refs_triggering_source(&delayed_ability) { if let Some(zone) = triggering_source_destination_zone(state) { @@ -433,7 +487,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, @@ -1390,6 +1444,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, @@ -1402,7 +1457,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!( @@ -1416,6 +1471,109 @@ 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)" + ); + } + + /// 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 e64cecc417..2228d73081 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -6692,8 +6692,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() @@ -8050,6 +8063,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, @@ -8120,7 +8191,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)); + // 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)); + } } } else if match scope { DelayedTriggerEventScope::Any => is_reflexive_lifetime(&delayed.condition), @@ -8544,7 +8638,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 diff --git a/crates/engine/src/game/turns.rs b/crates/engine/src/game/turns.rs index 3bfa98b926..7a32b4e75b 100644 --- a/crates/engine/src/game/turns.rs +++ b/crates/engine/src/game/turns.rs @@ -1322,6 +1322,30 @@ 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; + state.delayed_triggers.retain(|dt| { + !matches!( + &dt.condition, + Cond::WheneverEvent { + expiry: WheneverEventExpiry::UntilControllersNextTurn { + after: TurnGate::After(floor), + }, + .. + } if dt.controller == active && turn_number > *floor + ) + }); + } // 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"). @@ -2119,15 +2143,32 @@ pub fn execute_cleanup(state: &mut GameState, events: &mut Vec) -> Op // a "this turn" trigger: the end step "doesn't back up", so it legitimately // persists to the next turn's end step — it must survive this retain. state.delayed_triggers.retain(|dt| { - dt.one_shot + 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. + (dt.one_shot && !matches!( dt.condition, - crate::types::ability::DelayedTriggerCondition::WhenNextEvent { + 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!( + dt.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 9ffb5254b2..5e63e2ee29 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}; @@ -938,7 +939,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(); @@ -1004,7 +1044,13 @@ fn try_parse_whenever_this_turn(tp: TextPair) -> Option { let effect_text = after.original; // Parse the condition as a trigger using the trigger parser. - let mut inner_ctx = ParseContext::default(); + // CR 603.7c: this is a DELAYED trigger condition — `in_delayed_trigger` enables + // anaphoric subject resolution ("he"/"she" → SelfRef, "those creatures" → + // ParentTarget) that is valid only as a back-reference to the creating ability. + let mut inner_ctx = ParseContext { + in_delayed_trigger: true, + ..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 +1071,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 206ddbd901..1dc931aba1 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -1510,7 +1510,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..cc02bf48ef 100644 --- a/crates/engine/src/parser/oracle_ir/context.rs +++ b/crates/engine/src/parser/oracle_ir/context.rs @@ -44,6 +44,15 @@ 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, + /// CR 603.7c + CR 201.5: Whether we are parsing the CONDITION of a DELAYED + /// triggered ability created from an effect chain (set by + /// `try_parse_whenever_this_turn`). Anaphoric trigger 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 when this is set, so a + /// standalone printed trigger that happens to contain those words stays + /// coverage-honest (`Unknown`) instead of binding its source to `Any`. + pub in_delayed_trigger: bool, /// 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 ee92d09bdb..25d9c6d4bd 100644 --- a/crates/engine/src/parser/oracle_tests.rs +++ b/crates/engine/src/parser/oracle_tests.rs @@ -10539,7 +10539,7 @@ 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, .. } = condition else { panic!("expected WheneverEvent, got {condition:?}"); }; @@ -10952,7 +10952,7 @@ 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, .. } = condition else { panic!("expected WheneverEvent, got {condition:?}"); }; @@ -10977,7 +10977,7 @@ 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, .. } = condition else { panic!("expected WheneverEvent, got {condition:?}"); }; diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 5823eacede..6fc9d68be4 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -9012,6 +9012,70 @@ fn parse_single_subject<'a>(text: &'a str, ctx: &mut ParseContext) -> (TargetFil } } + // CR 109.4 + CR 603.7c + CR 608.2c: 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.in_delayed_trigger`, 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.in_delayed_trigger { + // CR 201.5: text referring to the object it's on (here the trigger + // source, via a gendered pronoun) means just that object → `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); + } + } + + // CR 608.2c + CR 603.7c: plural-set anaphora → `ParentTarget`. 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 +12821,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 +12903,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 +12933,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 dfa298d0c5..7ffba431f3 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 @@ -10921,7 +10921,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 a09c79c42c..8df2d08f6a 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -3413,6 +3413,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")] @@ -3451,7 +3482,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..6712722e1c --- /dev/null +++ b/crates/engine/tests/integration/human_torch_combat_damage_redirect.rs @@ -0,0 +1,155 @@ +//! 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 + ); + }; + + // The delayed subject arm evaluated "his commander" and must NOT have bound + // it to the source permanent — possessive case is excluded from the gendered + // SelfRef arm. This assertion flips (fails) if "his " ever binds SelfRef. + assert_ne!( + trigger.valid_source, + Some(TargetFilter::SelfRef), + "possessive 'his ' must not bind SelfRef" + ); +} 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..4fc9db2696 --- /dev/null +++ b/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs @@ -0,0 +1,220 @@ +//! 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, 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" + ); +} + +/// 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)" + ); +} 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..005d58096c --- /dev/null +++ b/crates/engine/tests/integration/love_on_the_battlefield_combat_counters.rs @@ -0,0 +1,367 @@ +//! 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); + run_combat(&mut runner, vec![a, b, c], vec![]); + runner.advance_until_stack_empty(); + + 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:?}"), + } +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 70234a00c7..d05555458d 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -1051,6 +1051,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; @@ -1061,6 +1062,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; @@ -1083,6 +1085,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 edec390cce..d3d9d7834f 100644 --- a/crates/mtgish-import/src/convert/action.rs +++ b/crates/mtgish-import/src/convert/action.rs @@ -5472,6 +5472,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, From bcdb70248e56d8c9c7ca10ffefaea4668e361fd3 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 11:40:57 -0500 Subject: [PATCH 02/11] fix(engine): address delayed combat-damage trigger review blockers (#6884) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the two runtime over-fire paths and test-evidence gaps raised in review of the delayed "deals combat damage" WheneverEvent mechanic. Blocker 1 — double-fire (triggers.rs): collect_matching_delayed_triggers recorded the synthetic per-source DamageDealt event as the consumed identity while computing its occurrence from the raw aggregate CombatDamageDealtToPlayer at event_index. Because filter_consumed_trigger_events_from compares both event equality and occurrence, the aggregate never matched the consumed set and survived into a later priority scan, re-firing the trigger. Record events[event_index] (the raw aggregate) as the consumed identity and keep the synthetic event as per-firing trigger context only. New building-block regression multi_fire_combat_damage_consumes_raw_aggregate_not_synthetic fails on the old code and passes on the fix. Blocker 2 — empty ParentTarget over-fire (delayed_trigger.rs): bind_contextual_filter_to_condition rewrites all three WheneverEvent filter slots, so an "up to N target" parent that chose zero could turn a bare ParentTarget in valid_card/valid_target (not just valid_source) into TargetFilter::Any and install an over-firing trigger. The install guard now inspects all three slots; added sibling fixtures for valid_card and valid_target. CodeRabbit maintainability + test evidence: - Replace the in_delayed_trigger bool with a typed TriggerConditionScope enum (Printed/Delayed) per the codebase's typed-enum convention. - Compute the delayed-trigger one_shot boolean once and reuse it; gate the TriggeringSource origin-stamp to one-shot triggers (a multi-fire WheneverEvent re-resolves TriggeringSource per firing, so freezing the creation-event zone would skip a later firing's zone move). - Assert the WheneverEventExpiry fields in the three spell-temporal parser tests instead of discarding them. - Strengthen negative tests with positive reach guards: human_torch now pins the coverage-honest Unknown parse shape (the possessive subject is not a recognized anaphor) and love_on_the_battlefield's three-attacker test asserts the defending player's life dropped (attack actually resolved). Verification: cargo fmt; cargo clippy -p phase-engine --all-targets (clean); cargo test -p phase-engine --lib (18272 passed); combat/damage/trigger/delayed integration sweep (798 passed). Co-Authored-By: Claude Opus 4.8 --- .../src/game/effects/delayed_trigger.rs | 168 +++++++++++++----- crates/engine/src/game/triggers.rs | 137 +++++++++++++- crates/engine/src/parser/oracle_effect/mod.rs | 8 +- crates/engine/src/parser/oracle_ir/context.rs | 32 +++- crates/engine/src/parser/oracle_tests.rs | 27 ++- crates/engine/src/parser/oracle_trigger.rs | 12 +- .../human_torch_combat_damage_redirect.rs | 29 ++- ...love_on_the_battlefield_combat_counters.rs | 12 ++ 8 files changed, 357 insertions(+), 68 deletions(-) diff --git a/crates/engine/src/game/effects/delayed_trigger.rs b/crates/engine/src/game/effects/delayed_trigger.rs index d24dec45ac..db8db65098 100644 --- a/crates/engine/src/game/effects/delayed_trigger.rs +++ b/crates/engine/src/game/effects/delayed_trigger.rs @@ -64,21 +64,32 @@ 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 source + // 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 - // source 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 `parent_targets_filter(&[]) => TargetFilter::Any` (below) would - // OVER-FIRE on every creature's combat damage. Scoped to a pre-bind - // `ParentTarget` source only, so a `SelfRef` source (Human Torch's "he", whose - // empty `ability.targets` is normal) still installs. + // 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 { - if matches!(trigger.valid_source, Some(TargetFilter::ParentTarget)) - && ability.targets.is_empty() - { + 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, @@ -173,40 +184,51 @@ pub fn resolve( // (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_ref` (a WheneverEvent) forces per-firing event-context + // it. `!one_shot` (a WheneverEvent) forces per-firing event-context // resolution instead. - let one_shot_ref = !matches!( + // + // 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_ref && 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, - // which is correct for ParentTarget (Flickerwisp) but wrong here. - crate::game::targeting::resolve_event_context_target( - state, - &crate::types::ability::TargetFilter::TriggeringSource, - ability.source_id, - ) - .map(|t| vec![t]) - .unwrap_or_default() - } else if super::ability_refs_parent_target(&delayed_ability) { - parent_target_snapshot(state, ability) - } else if effect_references_last_created(&delayed_ability.effect) - && !state.last_created_token_ids.is_empty() - { - state - .last_created_token_ids - .iter() - .map(|&id| TargetRef::Object(id)) - .collect() - } else { - vec![] - }; + 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, + // which is correct for ParentTarget (Flickerwisp) but wrong here. + crate::game::targeting::resolve_event_context_target( + state, + &crate::types::ability::TargetFilter::TriggeringSource, + ability.source_id, + ) + .map(|t| vec![t]) + .unwrap_or_default() + } else if super::ability_refs_parent_target(&delayed_ability) { + parent_target_snapshot(state, ability) + } else if effect_references_last_created(&delayed_ability.effect) + && !state.last_created_token_ids.is_empty() + { + state + .last_created_token_ids + .iter() + .map(|&id| TargetRef::Object(id)) + .collect() + } else { + 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); } @@ -275,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 { @@ -1517,6 +1536,69 @@ mod tests { ); } + /// 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 diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index 2228d73081..43f14044bf 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -8278,9 +8278,21 @@ fn collect_matching_delayed_triggers( let mut pending: Vec = to_fire .into_iter() .map(|(trigger, event_index, trigger_event)| { + // 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(), }); delayed_trigger_to_context(state, trigger, trigger_event) }) @@ -17471,6 +17483,129 @@ 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, + provenance: None, + }); + + // 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/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 939fa22cdb..92ce255f6d 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -157,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, @@ -1044,11 +1046,11 @@ fn try_parse_whenever_this_turn(tp: TextPair) -> Option { let effect_text = after.original; // Parse the condition as a trigger using the trigger parser. - // CR 603.7c: this is a DELAYED trigger condition — `in_delayed_trigger` enables + // CR 603.7c: this is a DELAYED trigger condition — 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. let mut inner_ctx = ParseContext { - in_delayed_trigger: true, + trigger_condition_scope: TriggerConditionScope::Delayed, ..ParseContext::default() }; let mut trigger_def = parse_dealt_damage_this_way_dies_trigger(condition_text, &mut inner_ctx) diff --git a/crates/engine/src/parser/oracle_ir/context.rs b/crates/engine/src/parser/oracle_ir/context.rs index cc02bf48ef..99de13e572 100644 --- a/crates/engine/src/parser/oracle_ir/context.rs +++ b/crates/engine/src/parser/oracle_ir/context.rs @@ -16,6 +16,25 @@ pub(crate) enum TokenPtFollowup { PowerToughness { power: PtValue, toughness: PtValue }, } +/// CR 603.7c + CR 201.5: Whether the trigger CONDITION currently being parsed is +/// a printed (card-text) trigger or a DELAYED trigger created from a resolving +/// effect chain. 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,15 +63,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, - /// CR 603.7c + CR 201.5: Whether we are parsing the CONDITION of a DELAYED - /// triggered ability created from an effect chain (set by - /// `try_parse_whenever_this_turn`). Anaphoric trigger 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 when this is set, so a - /// standalone printed trigger that happens to contain those words stays - /// coverage-honest (`Unknown`) instead of binding its source to `Any`. - pub in_delayed_trigger: bool, + /// CR 603.7c + CR 201.5: 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 e50093023b..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 6fc9d68be4..3d968ca77f 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::{ @@ -9015,11 +9015,11 @@ fn parse_single_subject<'a>(text: &'a str, ctx: &mut ParseContext) -> (TargetFil // CR 109.4 + CR 603.7c + CR 608.2c: 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.in_delayed_trigger`, 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.in_delayed_trigger { + // 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 { // CR 201.5: text referring to the object it's on (here the trigger // source, via a gendered pronoun) means just that object → `SelfRef`. // Nominative-only diff --git a/crates/engine/tests/integration/human_torch_combat_damage_redirect.rs b/crates/engine/tests/integration/human_torch_combat_damage_redirect.rs index 6712722e1c..050da4a87b 100644 --- a/crates/engine/tests/integration/human_torch_combat_damage_redirect.rs +++ b/crates/engine/tests/integration/human_torch_combat_damage_redirect.rs @@ -144,12 +144,35 @@ fn possessive_gendered_subject_does_not_become_selfref() { ); }; - // The delayed subject arm evaluated "his commander" and must NOT have bound - // it to the source permanent — possessive case is excluded from the gendered - // SelfRef arm. This assertion flips (fails) if "his " ever binds SelfRef. + // 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/love_on_the_battlefield_combat_counters.rs b/crates/engine/tests/integration/love_on_the_battlefield_combat_counters.rs index 005d58096c..73385acc12 100644 --- a/crates/engine/tests/integration/love_on_the_battlefield_combat_counters.rs +++ b/crates/engine/tests/integration/love_on_the_battlefield_combat_counters.rs @@ -265,9 +265,21 @@ fn three_attackers_do_not_satisfy_exactly_two() { 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, From 554aee781e74a62a21477eb6a3ad391f43c5ed4a Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 11:54:14 -0500 Subject: [PATCH 03/11] test(engine): scope-evidence class guards for delayed-trigger duration relocation (#6884) Reconcile the parse blast radius the maintainer flagged (14 cards / 4 signatures) with permanent class-level regression coverage rather than one-off card tests. Signatures 1-2 (12 cards, "until your next turn, whenever ..." delayed triggers): the CreateDelayedTrigger duration field change is the intended Gap-C fix. "until your next turn" relocates from the (previously misattached) creator-ability duration to the WheneverEvent expiry (UntilControllersNextTurn). This is a correctness win: before, these triggers had no expiry, defaulted to EndOfTurn, and were purged at the creating turn's cleanup, so they never fired on opponents' turns (CR 603.7b). Sig 1 (plain inner effect) leaves the creator ability with no residual duration; Sig 2 (inner "... until end of turn" buff, e.g. Jace/Tamiyo) surfaces the residual UntilEndOfTurn on the creator ability. until_next_turn_delayed_trigger_relocates_duration_to_expiry_across_class pins both. None of these 12 cards is claimed as newly-supported; they remain coverage-red multi-ability cards (strict failure retained). Signatures 3-4 (Alluring Suitor + Love on the Battlefield): the "attack with exactly two creatures" recognition (AttackersDeclaredCount EQ 2 + you target) is a class fix. Love has full runtime coverage; added parse_shape_alluring_suitor_exactly_two_attack_constraint for the sibling so the class cannot regress independently. Co-Authored-By: Claude Opus 4.8 --- .../kang_dynasty_until_next_turn_rider.rs | 81 +++++++++++++++++++ ...love_on_the_battlefield_combat_counters.rs | 39 +++++++++ 2 files changed, 120 insertions(+) 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 index 4fc9db2696..4003ebf429 100644 --- a/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs +++ b/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs @@ -85,6 +85,87 @@ fn chapter_rider_parses_until_next_turn_expiry_and_parent_target() { ); } +/// 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 { 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 index 73385acc12..31ec494b78 100644 --- a/crates/engine/tests/integration/love_on_the_battlefield_combat_counters.rs +++ b/crates/engine/tests/integration/love_on_the_battlefield_combat_counters.rs @@ -377,3 +377,42 @@ fn parse_shape_exactly_two_and_delayed_damage_rider() { 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:?}"), + } +} From c7d5b6a48a4ff73bb6ec2623c2a1889c717d5cab Mon Sep 17 00:00:00 2001 From: matthewevans Date: Sun, 2 Aug 2026 10:04:40 -0700 Subject: [PATCH 04/11] fix(PR-6884): remove accidental scratch artifact --- ...c24-a7a6-d1f9f7693f55scratchpadantman.json | 169 ------------------ 1 file changed, 169 deletions(-) delete mode 100644 UsersjacobAppDataLocalTempclaudeC--Users-jacob-source-repos-phase-card-runs!a248f4-b4f3-4c24-a7a6-d1f9f7693f55scratchpadantman.json diff --git a/UsersjacobAppDataLocalTempclaudeC--Users-jacob-source-repos-phase-card-runs!a248f4-b4f3-4c24-a7a6-d1f9f7693f55scratchpadantman.json b/UsersjacobAppDataLocalTempclaudeC--Users-jacob-source-repos-phase-card-runs!a248f4-b4f3-4c24-a7a6-d1f9f7693f55scratchpadantman.json deleted file mode 100644 index 4ead0e4533..0000000000 --- a/UsersjacobAppDataLocalTempclaudeC--Users-jacob-source-repos-phase-card-runs!a248f4-b4f3-4c24-a7a6-d1f9f7693f55scratchpadantman.json +++ /dev/null @@ -1,169 +0,0 @@ -{ - "name": "Ant-Man, Elusive Avenger", - "mana_cost": { - "type": "Cost", - "shards": [ - "Blue", - "Red" - ], - "generic": 1 - }, - "card_type": { - "supertypes": [ - "Legendary" - ], - "core_types": [ - "Creature" - ], - "subtypes": [ - "Human", - "Rogue", - "Hero" - ] - }, - "power": { - "type": "Fixed", - "value": 1 - }, - "toughness": { - "type": "Fixed", - "value": 2 - }, - "loyalty": null, - "defense": null, - "oracle_text": "Ant-Man can't be blocked by creatures with greater power.\nWhenever Ant-Man deals combat damage to a player, create that many Treasure tokens.", - "non_ability_text": null, - "flavor_name": null, - "keywords": [], - "abilities": [], - "triggers": [ - { - "mode": "DamageDone", - "execute": { - "kind": "Spell", - "effect": { - "type": "Token", - "name": "Treasure", - "power": { - "type": "Fixed", - "value": 0 - }, - "toughness": { - "type": "Fixed", - "value": 0 - }, - "types": [ - "Artifact", - "Treasure" - ], - "colors": [], - "keywords": [], - "tapped": false, - "count": { - "type": "Ref", - "qty": { - "type": "EventContextAmount" - } - }, - "owner": { - "type": "Controller" - }, - "enters_attacking": false - }, - "cost": null, - "sub_ability": null, - "duration": null, - "description": null, - "target_prompt": null, - "condition": null, - "optional_targeting": false, - "optional": false, - "forward_result": false - }, - "valid_card": null, - "origin": null, - "destination": null, - "trigger_zones": [ - "Battlefield" - ], - "phase": null, - "optional": false, - "damage_kind": "CombatOnly", - "secondary": false, - "valid_target": { - "type": "Player" - }, - "valid_source": { - "type": "SelfRef" - }, - "description": "Whenever ~ deals combat damage to a player, create that many Treasure tokens.", - "constraint": null, - "condition": null, - "batched": false - } - ], - "static_abilities": [ - { - "mode": { - "CantBeBlockedBy": { - "filter": { - "type": "Typed", - "type_filters": [ - "Creature" - ], - "controller": null, - "properties": [ - { - "type": "PowerGTSource" - } - ] - } - } - }, - "affected": { - "type": "SelfRef" - }, - "modifications": [], - "condition": null, - "affected_zone": null, - "effect_zone": null, - "active_zones": [], - "characteristic_defining": false, - "description": "~ can't be blocked by creatures with greater power." - } - ], - "replacements": [], - "color_override": [ - "Red", - "Blue" - ], - "color_identity": [ - "Red", - "Blue" - ], - "scryfall_oracle_id": "f334087a-deda-4e9e-ab58-68f7e226d7ce", - "brawl_commander": true, - "is_commander": true, - "metadata": { - "related_token_ids": [ - "50aae3ed-fa82-5258-9a21-31f5988f6b8f" - ], - "source_printing_ids": [ - "18e2b0a4-0ed3-48e6-8334-198a19e54bff", - "3a38b622-4129-4d84-908f-35c223c7c61b" - ] - }, - "legalities": { - "commander": "legal", - "duel": "legal", - "legacy": "legal", - "oathbreaker": "legal", - "vintage": "legal" - }, - "printings": [ - "MSC" - ], - "rarities": [ - "rare" - ] -} \ No newline at end of file From dd07cbafbd4fbc2a43e702171b1c66e692bcd06f Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:44:19 -0500 Subject: [PATCH 05/11] style(engine): fix clippy doc_lazy_continuation in scope-evidence test (#6884) The scope-evidence class-guard test's doc comment had a bulleted list immediately followed by a paragraph; add the blank /// separator so clippy::doc_lazy_continuation (workspace -D warnings) is satisfied. Co-Authored-By: Claude Opus 4.8 --- .../tests/integration/kang_dynasty_until_next_turn_rider.rs | 1 + 1 file changed, 1 insertion(+) 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 index 4003ebf429..700e3aac7f 100644 --- a/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs +++ b/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs @@ -95,6 +95,7 @@ fn chapter_rider_parses_until_next_turn_expiry_and_parent_target() { /// - 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 From c82717b10265eabc55c1c1bb36487c183ab89a6b Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:07:17 -0500 Subject: [PATCH 06/11] test(engine)+docs: address second-round review on delayed-trigger PR (#6884) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [MED] The "until your next turn" duration test only proved persistence (delayed_triggers.len) and never exercised firing. Added until_next_turn_parent_target_rider_fires_on_intervening_turn_then_expires: installs a ParentTarget rider bound to a real chosen creature (Kang's "any of those creatures"), drives that creature through unblocked combat against the controller on the intervening (opponent's) turn via the production pipeline, asserts the controller drew and took the combat damage, then crosses into the controller's next turn and asserts the rider was purged at untap. 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). This firing path also exercises the aggregate->per-source expansion + consumed-identity fix (blocker 1) at runtime. The existing survives-then-purges persistence test is retained. [LOW] Removed the false-provenance CR annotations (CR 603.7c / CR 201.5) from the parser-scope TriggerConditionScope enum and its ParseContext field. CR 603.7c governs a delayed ability's object reference across zone/characteristic changes and CR 201.5 governs name-based self-reference — neither describes parser scope or pronoun classification. Per the annotation convention this is parser scaffolding, not a rule implementation, so it carries no CR citation. Co-Authored-By: Claude Opus 4.8 --- crates/engine/src/parser/oracle_ir/context.rs | 21 +-- .../kang_dynasty_until_next_turn_rider.rs | 168 +++++++++++++++++- 2 files changed, 178 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/parser/oracle_ir/context.rs b/crates/engine/src/parser/oracle_ir/context.rs index 99de13e572..dab5fa4ea1 100644 --- a/crates/engine/src/parser/oracle_ir/context.rs +++ b/crates/engine/src/parser/oracle_ir/context.rs @@ -16,15 +16,16 @@ pub(crate) enum TokenPtFollowup { PowerToughness { power: PtValue, toughness: PtValue }, } -/// CR 603.7c + CR 201.5: Whether the trigger CONDITION currently being parsed is -/// a printed (card-text) trigger or a DELAYED trigger created from a resolving -/// effect chain. 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. +/// 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`. @@ -63,7 +64,7 @@ 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, - /// CR 603.7c + CR 201.5: Whether the trigger CONDITION being parsed is printed + /// 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, 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 index 700e3aac7f..2136ac0df5 100644 --- a/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs +++ b/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs @@ -21,7 +21,7 @@ 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, TriggerDefinition, TurnGate, WheneverEventExpiry, + QuantityExpr, TargetFilter, TargetRef, TriggerDefinition, TurnGate, WheneverEventExpiry, }; use engine::types::game_state::GameState; use engine::types::phase::Phase; @@ -300,3 +300,169 @@ fn until_next_turn_rider_survives_intervening_turn_then_purges() { 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) +} + +/// Install the Kang rider bound to a REAL chosen creature via `ParentTarget` +/// ("any of THOSE creatures" — the goaded creatures). `resolved.targets` carries +/// the chosen object so `bind_contextual_filter_to_condition` resolves the +/// `ParentTarget` source to it, mirroring production resolution of Kang's tap/goad +/// clause. Installed through `resolve_ability_chain`, the same path production uses. +fn install_kang_parent_target_rider( + state: &mut GameState, + source: engine::types::identifiers::ObjectId, + chosen: engine::types::identifiers::ObjectId, +) { + 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 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 mut resolved = build_resolved_from_def(&def, source, P0); + // The chosen goaded creature: `ParentTarget` binds to it at install. + resolved.targets = vec![TargetRef::Object(chosen)]; + let mut events = Vec::new(); + resolve_ability_chain(state, &resolved, &mut events, 0) + .expect("Kang ParentTarget rider installs via resolve_ability_chain"); +} + +/// 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 (MED review follow-up): the load-bearing behavioral claim is that +/// an "until your next turn" rider actually FIRES on the opponent's turn, bound to +/// the goaded creature via `ParentTarget`. This drives the full production pipeline: +/// install a `ParentTarget` rider on a real chosen creature, run that creature +/// through unblocked combat against the controller on the intervening turn, and +/// assert the controller DREW — then cross into the controller's next turn and +/// prove the rider expired. A revert of the `UntilControllersNextTurn` expiry +/// purges the rider at the creating turn's cleanup, so it never fires here (draw +/// assertion fails); a revert of the untap purge leaks it (expiry assertion fails). +#[test] +fn until_next_turn_parent_target_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 goaded "any of those creatures" — the rider's + // ParentTarget source. It attacks P0 (its controller) 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(); + + install_kang_parent_target_rider(runner.state_mut(), source, goaded); + assert_eq!(runner.state().delayed_triggers.len(), 1, "rider installed"); + + // 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" + ); +} From 382b98174aff31d43c4fc2a511d70b92bbcddc50 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:08:12 -0500 Subject: [PATCH 07/11] test(engine): drive Kang firing test through the parsed chapter, not a synthetic def (#6884) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [HIGH] The intervening-turn firing test constructed a fresh CreateDelayedTrigger AbilityDefinition and resolved that, so it could stay green even if the parsed chain lost its UntilControllersNextTurn expiry, failed to carry the parent target through the preceding tap/goad clauses, or attached the delayed sub-ability at the wrong point. Replace it with kang_parsed_chapter_rider_fires_on_intervening_turn_ then_expires, which resolves Kang's ACTUAL parsed chapter via parse_effect_chain(KANG_CHAPTER) + build_resolved_from_def, supplying a real chosen creature as the tap target. This exercises the full production seam the PR changes: parse -> SetTapState/Goad clauses -> parent-target propagation of the chosen creature into the rider's ParentTarget source (asserted: valid_source == SpecificObject{chosen}, tapped == true) -> the delayed-trigger resolver's expiry stamping. It then drives that goaded creature through unblocked combat against the controller on the intervening turn (asserts the controller drew and took combat damage) and crosses into the controller's next turn (asserts the rider was purged). The synthetic install helper is removed. Second signature (Jace/Tamiyo class): its delayed-trigger install/expiry/purge seam is identical (pinned by the parse-shape class-guard test). A runtime firing drive was probed and intentionally NOT added: its inner "it gets -X/-0 until end of turn" parses "it" to Pump{target: SelfRef} (the source), a pre-existing inner-effect misparse on that unclaimed card that this PR neither introduces nor touches — so a Sig-2 firing assertion would test pre-existing behavior, not the Gap-C expiry seam. Tracked as a separate follow-up. Verification: cargo fmt --all; cargo clippy --workspace --exclude phase-tauri --all-targets --features engine/proptest -- -D warnings (clean); Kang test file passes (4/4). Co-Authored-By: Claude Opus 4.8 --- .../kang_dynasty_until_next_turn_rider.rs | 109 +++++++++--------- 1 file changed, 53 insertions(+), 56 deletions(-) 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 index 2136ac0df5..cdf4d0265e 100644 --- a/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs +++ b/crates/engine/tests/integration/kang_dynasty_until_next_turn_rider.rs @@ -311,48 +311,6 @@ fn hand_len(runner: &GameRunner, player: engine::types::player::PlayerId) -> usi .unwrap_or(0) } -/// Install the Kang rider bound to a REAL chosen creature via `ParentTarget` -/// ("any of THOSE creatures" — the goaded creatures). `resolved.targets` carries -/// the chosen object so `bind_contextual_filter_to_condition` resolves the -/// `ParentTarget` source to it, mirroring production resolution of Kang's tap/goad -/// clause. Installed through `resolve_ability_chain`, the same path production uses. -fn install_kang_parent_target_rider( - state: &mut GameState, - source: engine::types::identifiers::ObjectId, - chosen: engine::types::identifiers::ObjectId, -) { - 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 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 mut resolved = build_resolved_from_def(&def, source, P0); - // The chosen goaded creature: `ParentTarget` binds to it at install. - resolved.targets = vec![TargetRef::Object(chosen)]; - let mut events = Vec::new(); - resolve_ability_chain(state, &resolved, &mut events, 0) - .expect("Kang ParentTarget rider installs via resolve_ability_chain"); -} - /// 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. @@ -380,17 +338,25 @@ fn advance_to_declare_attackers(runner: &mut GameRunner, player: engine::types:: ); } -/// Gap C FIRING (MED review follow-up): the load-bearing behavioral claim is that -/// an "until your next turn" rider actually FIRES on the opponent's turn, bound to -/// the goaded creature via `ParentTarget`. This drives the full production pipeline: -/// install a `ParentTarget` rider on a real chosen creature, run that creature -/// through unblocked combat against the controller on the intervening turn, and -/// assert the controller DREW — then cross into the controller's next turn and -/// prove the rider expired. A revert of the `UntilControllersNextTurn` expiry -/// purges the rider at the creating turn's cleanup, so it never fires here (draw -/// assertion fails); a revert of the untap purge leaks it (expiry assertion fails). +/// 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 until_next_turn_parent_target_rider_fires_on_intervening_turn_then_expires() { +fn kang_parsed_chapter_rider_fires_on_intervening_turn_then_expires() { use super::rules::AttackTarget; use engine::types::GameAction; @@ -399,8 +365,9 @@ fn until_next_turn_parent_target_rider_fires_on_intervening_turn_then_expires() let source = scenario .add_enchantment_from_oracle(P0, "Kang Dynasty", "Enchantment.") .id(); - // P1's creature is the goaded "any of those creatures" — the rider's - // ParentTarget source. It attacks P0 (its controller) on P1's turn. + // 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}")); @@ -408,8 +375,38 @@ fn until_next_turn_parent_target_rider_fires_on_intervening_turn_then_expires() } let mut runner = scenario.build(); - install_kang_parent_target_rider(runner.state_mut(), source, goaded); - assert_eq!(runner.state().delayed_triggers.len(), 1, "rider installed"); + // 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); From ea81a78332c2a649df61f843093517d0925e76ad Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 19:43:30 -0500 Subject: [PATCH 08/11] docs(parser): remove unsupported CR citations from delayed-anaphora comments (#6884) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [MED] Follow-through on the earlier context.rs annotation fix: the same false-provenance CR citations remained on the sibling parser-scope/anaphora comments this PR added. Removed CR 603.7c / CR 201.5 (and the CR 109.4 + 608.2c lead-in) from the four PR-introduced comments: - oracle_effect/mod.rs: the `Delayed` scope set site. - oracle_trigger.rs: the anaphoric-subjects gate, the gendered-pronoun→SelfRef arm, and the plural-set→ParentTarget arm. CR 603.7c governs a delayed ability's object identity across characteristic/zone changes and CR 201.5 governs name-based self-reference; neither describes these Oracle-text parsing heuristics (mode selection, pronoun/anaphora recognition). Per the reviewer's instruction the citations are removed rather than swapped for a nearby delayed-trigger rule that does not demonstrably support the behavior; the explanatory prose is retained and each is labeled parser scaffolding. Scope confirmed via `git diff ...HEAD`: these four are the only CR 603.7c/201.5 lines this PR added to the parser; all other such citations are pre-existing and left untouched. Comment-only change. Verified: cargo fmt --all clean. Co-Authored-By: Claude Opus 4.8 --- crates/engine/src/parser/oracle_effect/mod.rs | 9 ++++---- crates/engine/src/parser/oracle_trigger.rs | 23 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 92ce255f6d..2eeac7cf22 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -1045,10 +1045,11 @@ 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. - // CR 603.7c: this is a DELAYED trigger condition — 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. + // 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() diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 3d968ca77f..994d2f2cb3 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -9012,17 +9012,16 @@ fn parse_single_subject<'a>(text: &'a str, ctx: &mut ParseContext) -> (TargetFil } } - // CR 109.4 + CR 603.7c + CR 608.2c: 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`. + // 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 { - // CR 201.5: text referring to the object it's on (here the trigger - // source, via a gendered pronoun) means just that object → `SelfRef`. - // Nominative-only + // 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 @@ -9044,8 +9043,8 @@ fn parse_single_subject<'a>(text: &'a str, ctx: &mut ParseContext) -> (TargetFil } } - // CR 608.2c + CR 603.7c: plural-set anaphora → `ParentTarget`. Compose the - // quantifier axis (one `alt`) with the noun axis (one `alt`), per the + // 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 From 2b622272cdeef43bda103208bdc74173839619fc Mon Sep 17 00:00:00 2001 From: matthewevans Date: Sun, 2 Aug 2026 18:12:59 -0700 Subject: [PATCH 09/11] fix: retain delayed triggers without moving condition --- crates/engine/src/game/turns.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/game/turns.rs b/crates/engine/src/game/turns.rs index e16f4daa04..9e60f02b9c 100644 --- a/crates/engine/src/game/turns.rs +++ b/crates/engine/src/game/turns.rs @@ -2276,7 +2276,7 @@ pub fn execute_cleanup(state: &mut GameState, events: &mut Vec) -> Op // `matches!` respectively. let retain = trigger.one_shot && !matches!( - trigger.condition, + &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 @@ -2290,7 +2290,7 @@ pub fn execute_cleanup(state: &mut GameState, events: &mut Vec) -> Op // intervening (opponents') turns and is instead purged at the // controller's next turn start (see `execute_untap_with_choices`). || matches!( - trigger.condition, + &trigger.condition, Cond::WheneverEvent { expiry: WheneverEventExpiry::UntilControllersNextTurn { .. }, .. From ec3b6bde657741a4bdd3e4ff99ce23bfcbbcec6f Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 20:51:51 -0500 Subject: [PATCH 10/11] test(engine): fix delayed-trigger test construction for #6933 provenance type (#6884) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lifecycle-port merge (#6933) canonicalized DelayedTrigger.provenance from Option<..> to the DelayedInstallIdentity enum. The maintainer's port updated every production construction but not the building-block regression test added by this PR, so `provenance: None` in `multi_fire_combat_damage_consumes_raw_aggregate_not_synthetic` failed to compile (E0308), which cascaded to red Rust lint + both test shards. Set it to `DelayedInstallIdentity::LegacyDelayed` (a normal, non-command delayed trigger — the same value the production install path uses in effects/delayed_trigger.rs). Test-only change. Verified against the ported head: engine tests compile clean; the PR's added tests pass at runtime under the new canonical lifecycle (no behavioral reconciliation needed) — full lib suite 18323 passed, combat/trigger/delayed integration sweep 800 passed; cargo fmt + engine clippy (-D warnings) clean. Co-Authored-By: Claude Opus 4.8 --- crates/engine/src/game/triggers.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index 31dee748a6..3b988a3c85 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -17752,7 +17752,9 @@ pub mod tests { controller, source_id: rider_source, one_shot: false, - provenance: None, + // CR 603.7c: a normal (legacy) delayed trigger — no command receipt. + // (#6933 canonicalized `provenance` from Option to DelayedInstallIdentity.) + provenance: crate::types::identifiers::DelayedInstallIdentity::LegacyDelayed, }); // One combat-damage step: two creatures deal combat damage to one player. From c44a4451e8555ab8bc27464023391ddb557a0cc8 Mon Sep 17 00:00:00 2001 From: Jacob Woodson <38709105+JacobWoodson@users.noreply.github.com> Date: Sun, 2 Aug 2026 21:08:19 -0500 Subject: [PATCH 11/11] docs(engine): drop stray CR citation from provenance test fixture (#6884) Follow-up to the provenance test fix: the comment I added cited CR 603.7c on a test-only DelayedInstallIdentity::LegacyDelayed assignment. 603.7c governs a delayed ability's object reference across characteristic/zone changes, not this internal lifecycle/test provenance representation. Removed the citation (engine test scaffolding is not a rule implementation); kept the #6933 context note. Comment-only change. Co-Authored-By: Claude Opus 4.8 --- crates/engine/src/game/triggers.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index 3b988a3c85..2a9293c2db 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -17752,8 +17752,9 @@ pub mod tests { controller, source_id: rider_source, one_shot: false, - // CR 603.7c: a normal (legacy) delayed trigger — no command receipt. - // (#6933 canonicalized `provenance` from Option to DelayedInstallIdentity.) + // 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, });