From 9fcab573fdd55911220af84ec1ef6e683c56bae6 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 10:38:14 -0700 Subject: [PATCH 01/12] fix(engine): route they-may ETB choices to entrant controller --- .../src/ai_support/shortcut_efficacy.rs | 2 + crates/engine/src/game/ability_rw.rs | 8 ++ crates/engine/src/game/ability_scan.rs | 16 +++ crates/engine/src/game/ability_utils.rs | 1 + crates/engine/src/game/effects/mod.rs | 12 ++ crates/engine/src/game/resolution_prompt.rs | 1 + crates/engine/src/game/stack.rs | 9 ++ crates/engine/src/parser/oracle_ir/trigger.rs | 3 + crates/engine/src/parser/oracle_trigger.rs | 50 +++++-- .../engine/src/parser/oracle_trigger_tests.rs | 92 +++++++++++++ crates/engine/src/types/ability.rs | 15 +++ .../issue_4963_charismatic_conqueror.rs | 126 ++++++++++++++++++ crates/engine/tests/integration/main.rs | 1 + 13 files changed, 328 insertions(+), 8 deletions(-) create mode 100644 crates/engine/tests/integration/issue_4963_charismatic_conqueror.rs diff --git a/crates/engine/src/ai_support/shortcut_efficacy.rs b/crates/engine/src/ai_support/shortcut_efficacy.rs index f7a6caff94..40faa2a91b 100644 --- a/crates/engine/src/ai_support/shortcut_efficacy.rs +++ b/crates/engine/src/ai_support/shortcut_efficacy.rs @@ -625,6 +625,7 @@ fn ability_window_reach(def: &AbilityDefinition) -> WindowReach { repeat_for, announced_x, repeat_until, + optional_player, optional_for, iteration_kind_binding, // ---- read-free ---- @@ -691,6 +692,7 @@ fn ability_window_reach(def: &AbilityDefinition) -> WindowReach { || repeat_for.is_some() || announced_x.is_some() || repeat_until.is_some() + || optional_player.is_some() || optional_for.is_some() || iteration_kind_binding.is_some(); acc.or(WindowReach::of(!conservative_when_present)) diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index 4bc7653a5d..20e03d1221 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -3753,6 +3753,7 @@ fn walk_ability( context: _, optional_targeting: _, optional: _, + optional_player, optional_for: _, target_choice_timing: _, description: _, @@ -3832,6 +3833,9 @@ fn walk_ability( if let Some(tc) = target_chooser { acc.merge(rw_target_filter(tc)); } + if let Some(player) = optional_player { + acc.merge(rw_target_filter(player)); + } if let Some(ru) = repeat_until { acc.merge(rw_repeat_continuation(ru)); } @@ -3886,6 +3890,7 @@ fn walk_definition( ability_tag: _, optional_targeting: _, optional: _, + optional_player, optional_for: _, target_choice_timing: _, distribute: _, @@ -3951,6 +3956,9 @@ fn walk_definition( if let Some(tc) = target_chooser { acc.merge(rw_target_filter(tc)); } + if let Some(player) = optional_player { + acc.merge(rw_target_filter(player)); + } if let Some(ru) = repeat_until { acc.merge(rw_repeat_continuation(ru)); } diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index 3a32bab3d8..d325891add 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -242,6 +242,7 @@ fn resolved_ability_axes(a: &ResolvedAbility, mode: ScanMode) -> Axes { context: _, // SpellContext: cast-time fact snapshot, not a live read optional_targeting: _, // bool optional: _, // bool + optional_player, optional_for: _, // OpponentMayScope: AnyOpponent/AnyPlayer, no read target_choice_timing: _, // Stack/Resolution tag description: _, // display string @@ -324,6 +325,13 @@ fn resolved_ability_axes(a: &ResolvedAbility, mode: ScanMode) -> Axes { mode, )); } + if let Some(player) = optional_player { + acc = acc.or(scan_target_filter( + player, + FilterReadContext::SnapshotOrEvent, + mode, + )); + } // CR 608.2c / CR 107.1c: a "repeat this process while " predicate is // re-evaluated against freshly-resolved state each iteration — a resolution read. if let Some(repeat_until) = repeat_until { @@ -4353,6 +4361,7 @@ fn ability_definition_axes(def: &AbilityDefinition, mode: ScanMode) -> Axes { ability_tag: _, optional_targeting: _, optional: _, + optional_player, optional_for: _, target_choice_timing: _, min_x_value: _, @@ -4413,6 +4422,13 @@ fn ability_definition_axes(def: &AbilityDefinition, mode: ScanMode) -> Axes { mode, )); } + if let Some(player) = optional_player { + acc = acc.or(scan_target_filter( + player, + FilterReadContext::SnapshotOrEvent, + mode, + )); + } if let Some(ru) = repeat_until { acc = acc.or(scan_repeat_continuation(ru, mode)); } diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index 8c686b9d40..89f272b282 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -119,6 +119,7 @@ pub fn build_resolved_from_def_with_targets( } resolved.optional_targeting = def.optional_targeting; resolved.optional = def.optional; + resolved.optional_player = def.optional_player.clone(); resolved.optional_for = def.optional_for; resolved.multi_target = def.multi_target.clone(); // CR 115.1 + CR 601.2c: Carry the target-set constraints (e.g. combined diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index 82b2d0d8a1..b7c47102bc 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -7123,6 +7123,18 @@ pub(crate) fn optional_prompt_player(state: &GameState, ability: &ResolvedAbilit return player; } } + // CR 608.2d: a parser-stamped subject such as "they may" names the player + // who receives this choice. The reference resolves from the trigger event, + // preserving the event-time controller rather than inferring from effect shape. + if let Some(optional_player) = &ability.optional_player { + if let Some(player) = crate::game::targeting::resolve_effect_player_ref( + state, + ability, + optional_player, + ) { + return player; + } + } if let Effect::Sacrifice { target, .. } = &ability.effect { if target_filter_controller_scope(target) == Some(ControllerRef::ParentTargetController) { if let Some(player) = crate::game::targeting::resolve_effect_player_ref( diff --git a/crates/engine/src/game/resolution_prompt.rs b/crates/engine/src/game/resolution_prompt.rs index a995e32e86..96449f475c 100644 --- a/crates/engine/src/game/resolution_prompt.rs +++ b/crates/engine/src/game/resolution_prompt.rs @@ -529,6 +529,7 @@ pub(crate) fn chain_offers_choice(a: &ResolvedAbility) -> bool { sub_ability, else_ability, optional, + optional_player: _, // selects the optional actor; `optional` already records the choice optional_for, optional_targeting, unless_pay, diff --git a/crates/engine/src/game/stack.rs b/crates/engine/src/game/stack.rs index ef74710c04..2cb2a5136a 100644 --- a/crates/engine/src/game/stack.rs +++ b/crates/engine/src/game/stack.rs @@ -3084,6 +3084,7 @@ fn self_counter_ability_is_batch_candidate(ability: &ResolvedAbility) -> bool { context, optional_targeting, optional, + optional_player, optional_for, multi_target, target_constraints, @@ -3144,6 +3145,7 @@ fn self_counter_ability_is_batch_candidate(ability: &ResolvedAbility) -> bool { && *context == SpellContext::default() && !*optional_targeting && !*optional + && optional_player.is_none() && optional_for.is_none() && multi_target.is_none() && target_constraints.is_empty() @@ -3297,6 +3299,7 @@ fn fixed_controller_gain_life_ability_is_batch_candidate(ability: &ResolvedAbili context, optional_targeting, optional, + optional_player, optional_for, multi_target, target_constraints, @@ -3352,6 +3355,7 @@ fn fixed_controller_gain_life_ability_is_batch_candidate(ability: &ResolvedAbili && *context == SpellContext::default() && !*optional_targeting && !*optional + && optional_player.is_none() && optional_for.is_none() && multi_target.is_none() && target_constraints.is_empty() @@ -3490,6 +3494,7 @@ fn fixed_opponent_lose_life_ability_is_batch_candidate(ability: &ResolvedAbility context, optional_targeting, optional, + optional_player, optional_for, multi_target, target_constraints, @@ -3545,6 +3550,7 @@ fn fixed_opponent_lose_life_ability_is_batch_candidate(ability: &ResolvedAbility && *context == SpellContext::default() && !*optional_targeting && !*optional + && optional_player.is_none() && optional_for.is_none() && multi_target.is_none() && target_constraints.is_empty() @@ -4130,6 +4136,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( context: a_context, optional_targeting: a_optional_targeting, optional: a_optional, + optional_player: a_optional_player, optional_for: a_optional_for, multi_target: a_multi_target, target_constraints: a_target_constraints, @@ -4186,6 +4193,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( context: b_context, optional_targeting: b_optional_targeting, optional: b_optional, + optional_player: b_optional_player, optional_for: b_optional_for, multi_target: b_multi_target, target_constraints: b_target_constraints, @@ -4254,6 +4262,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( && a_context == b_context && a_optional_targeting == b_optional_targeting && a_optional == b_optional + && a_optional_player == b_optional_player && a_optional_for == b_optional_for && a_multi_target == b_multi_target && a_target_constraints == b_target_constraints diff --git a/crates/engine/src/parser/oracle_ir/trigger.rs b/crates/engine/src/parser/oracle_ir/trigger.rs index 1dadec2617..a2e97315aa 100644 --- a/crates/engine/src/parser/oracle_ir/trigger.rs +++ b/crates/engine/src/parser/oracle_ir/trigger.rs @@ -291,6 +291,9 @@ pub(crate) struct TriggerModifiers { /// CR 603.5: Some triggered abilities' effects are optional (they contain /// "may"). They go on the stack regardless; the choice is made on resolution. pub(crate) optional: bool, + /// CR 608.2d: Event-relative player explicitly named by the root optional + /// subject, after any intervening-if wrapper has been removed. + pub(crate) optional_player: Option, /// CR 118.12: "unless [player] pays {cost}" tax modifier. pub(crate) unless_pay: Option, /// Intervening-if condition extracted from effect text. diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 22c4509637..bbd2c14d28 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -51,12 +51,13 @@ use crate::types::ability::{ AdditionalCostOrigin, AdditionalCostPaymentSource, AggregateFunction, AttachmentKind, AttackersDeclaredCountSubject, CastManaObjectScope, CastManaSpentMetric, CastVariantPaid, CoinFlipResult, Comparator, ControllerRef, CountScope, CounterTriggerFilter, DamageKindFilter, - DestinationConstraint, DieResultFilter, Effect, FilterProp, ManaAbilityProducedFilter, - ObjectScope, OriginConstraint, ParsedCondition, PlayerFilter, PlayerScope, PtStat, + DestinationConstraint, DieResultFilter, Effect, EffectScope, FilterProp, + ManaAbilityProducedFilter, ObjectScope, OriginConstraint, ParsedCondition, PlayerFilter, + PlayerScope, PtStat, PtValueScope, QuantityExpr, QuantityRef, RenownSubject, SacrificeAggregateStat, SacrificeCost, SacrificeRequirement, SharedQuality, StaticCondition, SubAbilityLink, TapCreaturesRequirement, - TargetFilter, TriggerCondition, TriggerConstraint, TriggerDefinition, TypeFilter, TypedFilter, - UnlessPayModifier, ZoneChangeClause, + TapStateChange, TargetFilter, TriggerCondition, TriggerConstraint, TriggerDefinition, + TypeFilter, TypedFilter, UnlessPayModifier, ZoneChangeClause, }; use crate::types::card_type::{is_land_subtype, CoreType}; use crate::types::counter::CounterType; @@ -296,6 +297,18 @@ fn effect_adds_mana_to_triggering_player(effect_lower: &str) -> bool { .is_ok() } +/// CR 608.2d + CR 603.2: A leading "they may" in a normalized trigger body +/// names the player recorded by that trigger event, rather than the ability's +/// controller. Call after stripping an intervening-if wrapper so the actor is +/// retained for both direct and conditional root modals. +fn optional_player_from_effect_body(effect_text: &str) -> Option { + let lower = effect_text.to_lowercase(); + tag::<_, _, OracleError<'_>>("they may ") + .parse(lower.trim_start()) + .ok() + .map(|_| TargetFilter::TriggeringPlayer) +} + /// CR 113.6 + CR 113.6b: Collect every zone the trigger's /// source must occupy for the condition to be satisfiable. Returns the /// deduplicated union of `SourceInZone { zone }` references across @@ -1376,6 +1389,7 @@ pub(crate) fn parse_trigger_line_with_index_ir( (without_if, cond, None) } }; + let optional_player = optional_player_from_effect_body(&effect_without_if); // CR 608.2c (resolution-order instructions): "You may" at the start of // the effect text makes the triggered effect optional at resolution. @@ -1606,6 +1620,7 @@ pub(crate) fn parse_trigger_line_with_index_ir( body, modifiers: TriggerModifiers { optional, + optional_player, unless_pay, intervening_if: if_condition, trigger_subject, @@ -1805,6 +1820,11 @@ pub(crate) fn lower_trigger_ir(ir: &TriggerIr) -> TriggerDefinition { // quantities to `PlayerScope::ScopedPlayer` so they resolve against the // damaged/attacked player rather than an absent chosen target. let mut execute = execute; + if let Some(optional_player) = &modifiers.optional_player { + if let Some(ability) = execute.as_deref_mut() { + ability.optional_player = Some(optional_player.clone()); + } + } // CR 603.2c: A `TrackedSetAggregate { source: TriggeringBatch }` reduces the // objects of THIS trigger's event, read back through // `extract_sources_from_event`. That only yields anything for the events that @@ -2123,7 +2143,7 @@ pub(crate) fn lower_trigger_ir(ir: &TriggerIr) -> TriggerDefinition { } } - // CR 608.2k + CR 603.7c: For event-source-bearing trigger modes, the "that + // CR 603.2 + CR 603.6 + CR 608.2k: For event-source-bearing trigger modes, the "that // card / that creature / that permanent" anaphor in the effect body // refers to the *triggering object* carried by the event (the just- // discarded card, sacrificed permanent, drawn card, etc.) — not a chosen @@ -2225,7 +2245,7 @@ fn valid_target_blocks_event_source_lift( /// TargetFilter` and whose runtime semantics make sense against the event /// object (e.g. `ChangeZone` operating on the just-discarded card). Other /// effect variants are left untouched. -fn lift_parent_target_to_triggering_source(effect: &mut Effect) { +fn lift_parent_target_to_triggering_source(effect: &mut Effect, allow_set_tap_lift: bool) { // CR 608.2k: each variant carries a top-level `target` that, when the // surface anaphor was "that ", refers to the event object. let target = match effect { @@ -2234,6 +2254,15 @@ fn lift_parent_target_to_triggering_source(effect: &mut Effect) { // "create a token that's a copy of that creature" (Necroduality) — the // copy source is the entering object, not the trigger's own source. Effect::CopyTokenOf { target, .. } => target, + // CR 608.2k + CR 701.26a: on a single-object zone-change trigger, + // "they may tap that permanent" refers to the entering object. The + // caller limits this to the trigger's top-level, untargeted Tap effect; + // a reflexive or targeted tap has its own chosen referent instead. + Effect::SetTapState { + target, + scope: EffectScope::Single, + state: TapStateChange::Tap, + } if allow_set_tap_lift => target, _ => return, }; if matches!(target, TargetFilter::ParentTarget) { @@ -2260,7 +2289,8 @@ fn first_independent_sibling_after_search( None } -/// CR 608.2k + CR 603.7c: Recurse `lift_parent_target_to_triggering_source` +/// CR 603.2 + CR 603.6 + CR 608.2k: Recurse +/// `lift_parent_target_to_triggering_source` /// through an ability's effect AND every chained `sub_ability`. Required /// for the punisher-trigger class: a chained Tergrid-shape ability like /// "...exile that card, then create a token" carries the "that card" @@ -2276,6 +2306,7 @@ fn lift_parent_target_to_triggering_source_in_ability(ability: &mut AbilityDefin // Necroduality (top-level `CopyTokenOf` with no prior choice) and Tergrid // ("put that card …, then create a token") still lift correctly. let mut node = Some(ability); + let mut is_top_level = true; while let Some(link) = node { // CR 701.23a: A library search's continuation receives the found cards // as its parent targets. In an event-source-bearing trigger, the @@ -2294,8 +2325,11 @@ fn lift_parent_target_to_triggering_source_in_ability(ability: &mut AbilityDefin if introduces_chosen_object_target(link.effect.as_ref()) { break; } - lift_parent_target_to_triggering_source(link.effect.as_mut()); + let allow_set_tap_lift = + is_top_level && link.multi_target.is_none() && !link.optional_targeting; + lift_parent_target_to_triggering_source(link.effect.as_mut(), allow_set_tap_lift); node = link.sub_ability.as_deref_mut(); + is_top_level = false; } } diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 1e48219cf4..0c0bf44d49 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -2472,6 +2472,98 @@ fn trigger_etb_subject_enters_untapped_attaches_negated_condition() { condition: Box::new(TriggerCondition::ZoneChangeObjectIsTapped) }) ); + let execute = def.execute.as_deref().expect("Charismatic execute ability"); + assert!(execute.optional, "they may tap must remain optional"); + assert_eq!( + execute.optional_player, + Some(TargetFilter::TriggeringPlayer), + "the parsed `they` subject, not the tap shape, names the optional actor" + ); + assert!(matches!( + execute.effect.as_ref(), + Effect::SetTapState { + target: TargetFilter::TriggeringSource, + scope: EffectScope::Single, + state: TapStateChange::Tap, + } + )); + let decline = execute + .sub_ability + .as_deref() + .expect("decline token continuation"); + assert_eq!( + decline.condition, + Some(AbilityCondition::EffectOutcome { + signal: crate::types::ability::EffectOutcomeSignal::OptionalEffectPerformed, + }) + .negate(), + "the Vampire token must remain the optional tap's decline branch" + ); +} + +/// CR 608.2d: The controller's "you may" modal must not acquire the +/// event-relative actor provenance reserved for an explicit "they may" subject. +#[test] +fn trigger_you_may_tap_does_not_stamp_triggering_player_as_optional_actor() { + let def = parse_trigger_line( + "Whenever a creature enters, you may tap that permanent.", + "Controller's Tap", + ); + let execute = def.execute.as_deref().expect("execute ability"); + assert!(execute.optional); + assert_eq!(execute.optional_player, None); +} + +/// CR 603.4 + CR 608.2d: Actor provenance survives a supported intervening-if +/// wrapper, so its `they may` body still prompts the player from the event. +#[test] +fn conditional_they_may_tap_stamps_triggering_player_as_optional_actor() { + let def = parse_trigger_line( + "Whenever a creature enters, if that creature is white, they may tap that permanent.", + "Conditional Tap", + ); + let execute = def.execute.as_deref().expect("execute ability"); + assert!(execute.optional); + assert_eq!(execute.optional_player, Some(TargetFilter::TriggeringPlayer)); +} + +/// CR 603.2 + CR 603.6 + CR 608.2k: Only a trigger's direct, untargeted +/// "tap that permanent" instruction is rebound to the zone-change object. +/// A reflexive selected tap and an untap anaphor retain their own referents. +#[test] +fn event_source_tap_lift_preserves_reflexive_and_untap_referents() { + fn first_tap(ability: &AbilityDefinition) -> Option<&Effect> { + if matches!(ability.effect.as_ref(), Effect::SetTapState { .. }) { + return Some(ability.effect.as_ref()); + } + ability.sub_ability.as_deref().and_then(first_tap) + } + + let snare = parse_trigger_line( + "When Snaremaster Sprite enters, you may pay {2}. When you do, tap target creature an opponent controls and put a stun counter on it.", + "Snaremaster Sprite", + ); + assert!(matches!( + snare.execute.as_deref().and_then(first_tap), + Some(Effect::SetTapState { + target: TargetFilter::ParentTarget, + state: TapStateChange::Tap, + .. + }) + )); + + let howl = parse_trigger_line( + "When Howl of the Hunt enters, if enchanted creature is a Wolf or Werewolf, untap that creature.", + "Howl of the Hunt", + ); + assert!(matches!( + howl.execute.as_deref().and_then(first_tap), + Some(Effect::SetTapState { + target: TargetFilter::ParentTarget, + state: TapStateChange::Untap, + .. + }) + )); } // Guard: a bare "enters" (no tapped-state rider) must NOT attach a diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 44a5493940..709392c3f1 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -18940,6 +18940,9 @@ pub struct AbilityDefinition { pub optional_targeting: bool, /// CR 608.2d: When true, the controller chooses whether to perform this effect ("You may X"). pub optional: bool, + /// CR 608.2d: The event-relative player named by an optional subject (for example, + /// "they may"). `None` keeps the default of this ability's controller. + pub optional_player: Option, /// CR 608.2d: When set, an opponent (not the controller) chooses whether to perform this /// optional effect. Requires `optional: true`. Opponents are prompted in APNAP order. pub optional_for: Option, @@ -19084,6 +19087,8 @@ struct AbilityDefinitionRepr<'a> { optional_targeting: bool, optional: bool, #[serde(skip_serializing_if = "Option::is_none")] + optional_player: &'a Option, + #[serde(skip_serializing_if = "Option::is_none")] optional_for: &'a Option, #[serde(skip_serializing_if = "Option::is_none")] multi_target: &'a Option, @@ -19150,6 +19155,7 @@ impl Serialize for AbilityDefinition { condition, optional_targeting, optional, + optional_player, optional_for, multi_target, target_constraints, @@ -19191,6 +19197,7 @@ impl Serialize for AbilityDefinition { condition, optional_targeting: *optional_targeting, optional: *optional, + optional_player, optional_for, multi_target, target_constraints, @@ -19282,6 +19289,8 @@ struct AbilityDefinitionDe { #[serde(default)] optional: bool, #[serde(default)] + optional_player: Option, + #[serde(default)] optional_for: Option, #[serde(default)] multi_target: Option, @@ -19354,6 +19363,7 @@ impl<'de> Deserialize<'de> for AbilityDefinition { condition: de.condition, optional_targeting: de.optional_targeting, optional: de.optional, + optional_player: de.optional_player, optional_for: de.optional_for, multi_target: de.multi_target, target_constraints: de.target_constraints, @@ -19550,6 +19560,7 @@ impl AbilityDefinition { condition: None, optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), @@ -24402,6 +24413,9 @@ pub struct ResolvedAbility { /// CR 608.2d: Optional effect — controller prompted before execution. #[serde(default)] pub optional: bool, + /// CR 608.2d: Event-relative player explicitly named by an optional subject. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub optional_player: Option, /// CR 608.2d: When set, an opponent chooses whether to perform this optional effect. #[serde(default, skip_serializing_if = "Option::is_none")] pub optional_for: Option, @@ -24642,6 +24656,7 @@ impl ResolvedAbility { context: SpellContext::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), diff --git a/crates/engine/tests/integration/issue_4963_charismatic_conqueror.rs b/crates/engine/tests/integration/issue_4963_charismatic_conqueror.rs new file mode 100644 index 0000000000..f1cc690623 --- /dev/null +++ b/crates/engine/tests/integration/issue_4963_charismatic_conqueror.rs @@ -0,0 +1,126 @@ +//! Regression for issue #4963: Charismatic Conqueror's optional tap belongs to +//! the player who controlled the permanent as it entered. + +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::types::actions::GameAction; +use engine::types::game_state::{CastPaymentMode, WaitingFor}; +use engine::types::identifiers::ObjectId; +use engine::types::keywords::Keyword; +use engine::types::mana::ManaCost; +use engine::types::phase::Phase; + +const CHARISMATIC_CONQUEROR_ORACLE: &str = "Whenever an artifact or creature an opponent controls enters untapped, they may tap that permanent. If they don't, you create a 1/1 white Vampire creature token with lifelink."; +const CONTROLLER_MAY_TAP_ORACLE: &str = "Whenever a creature enters, you may tap that permanent."; + +fn scenario_with_optional_tapper(oracle: &str) -> (GameRunner, ObjectId) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario + .add_creature_from_oracle( + P0, + "Charismatic Conqueror", + 2, + 2, + oracle, + ) + .id(); + let entrant = scenario + .add_creature_to_hand_from_oracle(P1, "Untapped Entrant", 1, 1, "") + .with_mana_cost(ManaCost::zero()) + .id(); + let mut runner = scenario.build(); + { + let state = runner.state_mut(); + state.active_player = P1; + state.priority_player = P1; + state.waiting_for = WaitingFor::Priority { player: P1 }; + } + (runner, entrant) +} + +fn resolve_entrant_to_optional(runner: &mut GameRunner, entrant: ObjectId) { + let card_id = runner.state().objects[&entrant].card_id; + runner + .act(GameAction::CastSpell { + object_id: entrant, + card_id, + targets: vec![], + payment_mode: CastPaymentMode::Auto, + }) + .expect("cast the zero-cost entrant through the production pipeline"); + runner.advance_until_stack_empty(); +} + +/// CR 603.2 + CR 603.6a + CR 608.2d: P1's untapped creature ETB triggers +/// Conqueror, but P1—not the Conqueror controller—makes the optional choice. +/// Accepting taps the entrant and suppresses the decline token branch. +#[test] +fn charismatic_conqueror_accept_prompts_entering_controller_and_taps_entrant() { + let (mut runner, entrant) = scenario_with_optional_tapper(CHARISMATIC_CONQUEROR_ORACLE); + resolve_entrant_to_optional(&mut runner, entrant); + + assert!(matches!( + runner.state().waiting_for, + WaitingFor::OptionalEffectChoice { player: P1, .. } + )); + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("P1 accepts the optional tap"); + runner.advance_until_stack_empty(); + + assert!( + runner.state().objects[&entrant].tapped, + "P1 accepted the tap" + ); + assert!( + !runner.state().objects.values().any(|object| { + object.is_token && object.name == "Vampire" && object.controller == P0 + }), + "accepting must not execute the 'If they don't' Vampire branch" + ); +} + +/// CR 608.2c + CR 109.5: declining leaves P1's entrant untapped and creates +/// the Vampire for P0, the controller of Conqueror when its trigger fired. +#[test] +fn charismatic_conqueror_decline_keeps_entrant_untapped_and_creates_p0_vampire() { + let (mut runner, entrant) = scenario_with_optional_tapper(CHARISMATIC_CONQUEROR_ORACLE); + resolve_entrant_to_optional(&mut runner, entrant); + + assert!(matches!( + runner.state().waiting_for, + WaitingFor::OptionalEffectChoice { player: P1, .. } + )); + runner + .act(GameAction::DecideOptionalEffect { accept: false }) + .expect("P1 declines the optional tap"); + runner.advance_until_stack_empty(); + + assert!( + !runner.state().objects[&entrant].tapped, + "declining must leave P1's entrant untapped" + ); + let vampires: Vec<_> = runner + .state() + .objects + .values() + .filter(|object| object.is_token && object.name == "Vampire") + .collect(); + assert_eq!(vampires.len(), 1, "declining creates one Vampire token"); + assert_eq!(vampires[0].controller, P0); + assert!(vampires[0].keywords.contains(&Keyword::Lifelink)); +} + +/// CR 608.2d: A controller's "you may tap that permanent" uses the same +/// event-object referent but must prompt P0, proving the `they may` actor stamp +/// is not inferred from the lowered tap effect. +#[test] +fn controller_may_tap_that_permanent_prompts_ability_controller() { + let (mut runner, entrant) = scenario_with_optional_tapper(CONTROLLER_MAY_TAP_ORACLE); + resolve_entrant_to_optional(&mut runner, entrant); + + assert!(matches!( + runner.state().waiting_for, + WaitingFor::OptionalEffectChoice { player: P0, .. } + )); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 24257508a7..07bb4c7e6a 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -590,6 +590,7 @@ mod issue_4955_greenbelt_rampager; mod issue_4956_gift_of_immortality_reattach; mod issue_4960_nova_flame; mod issue_4962_volo_guide_to_monsters; +mod issue_4963_charismatic_conqueror; mod issue_4966_waterbenders_ascension; mod issue_4991_vigorous_farming; mod issue_4999_treasure_cruise_delve_tokens; From 197c4da30685a497ea92e239e47542e8a6d3e78a Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 10:42:01 -0700 Subject: [PATCH 02/12] fix(engine): preserve optional actor through vote chains --- crates/engine/src/game/effects/vote.rs | 3 +++ crates/engine/src/parser/oracle_trigger.rs | 6 ++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/game/effects/vote.rs b/crates/engine/src/game/effects/vote.rs index 78d00004d8..bc0cc7e63f 100644 --- a/crates/engine/src/game/effects/vote.rs +++ b/crates/engine/src/game/effects/vote.rs @@ -373,6 +373,7 @@ pub fn resolve_tally( context: Default::default(), optional_targeting: per_choice_effect[idx].optional_targeting, optional: per_choice_effect[idx].optional, + optional_player: per_choice_effect[idx].optional_player.clone(), optional_for: None, multi_target: None, target_constraints: Vec::new(), @@ -443,6 +444,7 @@ pub fn resolve_tally( context: Default::default(), optional_targeting: per_choice_effect[idx].optional_targeting, optional: per_choice_effect[idx].optional, + optional_player: per_choice_effect[idx].optional_player.clone(), optional_for: None, multi_target: None, target_constraints: Vec::new(), @@ -687,6 +689,7 @@ fn resolved_from_def( context: Default::default(), optional_targeting: def.optional_targeting, optional: def.optional, + optional_player: def.optional_player.clone(), optional_for: None, multi_target: None, target_constraints: Vec::new(), diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index bbd2c14d28..6533254d7c 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -303,10 +303,8 @@ fn effect_adds_mana_to_triggering_player(effect_lower: &str) -> bool { /// retained for both direct and conditional root modals. fn optional_player_from_effect_body(effect_text: &str) -> Option { let lower = effect_text.to_lowercase(); - tag::<_, _, OracleError<'_>>("they may ") - .parse(lower.trim_start()) - .ok() - .map(|_| TargetFilter::TriggeringPlayer) + let parsed = tag::<_, _, OracleError<'_>>("they may ").parse(lower.trim_start()); + parsed.ok().map(|_| TargetFilter::TriggeringPlayer) } /// CR 113.6 + CR 113.6b: Collect every zone the trigger's From 8443c02dc132c412ec8fc125e1ab444755e17360 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 10:46:02 -0700 Subject: [PATCH 03/12] style(engine): format optional actor routing --- crates/engine/src/game/ability_scan.rs | 42 +++++++++---------- crates/engine/src/game/effects/mod.rs | 8 ++-- crates/engine/src/parser/oracle_trigger.rs | 10 ++--- .../engine/src/parser/oracle_trigger_tests.rs | 5 ++- .../issue_4963_charismatic_conqueror.rs | 8 +--- 5 files changed, 34 insertions(+), 39 deletions(-) diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index d325891add..fbff5eccad 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -243,27 +243,27 @@ fn resolved_ability_axes(a: &ResolvedAbility, mode: ScanMode) -> Axes { optional_targeting: _, // bool optional: _, // bool optional_player, - optional_for: _, // OpponentMayScope: AnyOpponent/AnyPlayer, no read - target_choice_timing: _, // Stack/Resolution tag - description: _, // display string - selected_mode_labels: _, // display strings, no dynamic read - min_x_value: _, // u32 - cant_be_copied: _, // bool - copy_count_status: _, // status tag - forward_result: _, // bool - distribution: _, // concrete pre-assigned (TargetRef, u32) portions - chosen_x: _, // concrete cast-time X - cost_paid_object: _, // concrete captured-object snapshot - cost_paid_object_ids: _, // concrete captured-object ids (issue #4948) - effect_context_object: _, // concrete captured-object snapshot - amassed_army_object: _, // concrete captured-object snapshot - ability_index: _, // usize provenance - may_trigger_origin: _, // provenance tag - target_selection_mode: _, // Chosen/Random tag - chosen_players: _, // concrete chosen player ids - replacement_applied: _, // replacement provenance set, no dynamic read - sub_link: _, // SubAbilityLink kind tag - sibling_condition: _, // SiblingCondition replication marker, no dynamic read + optional_for: _, // OpponentMayScope: AnyOpponent/AnyPlayer, no read + target_choice_timing: _, // Stack/Resolution tag + description: _, // display string + selected_mode_labels: _, // display strings, no dynamic read + min_x_value: _, // u32 + cant_be_copied: _, // bool + copy_count_status: _, // status tag + forward_result: _, // bool + distribution: _, // concrete pre-assigned (TargetRef, u32) portions + chosen_x: _, // concrete cast-time X + cost_paid_object: _, // concrete captured-object snapshot + cost_paid_object_ids: _, // concrete captured-object ids (issue #4948) + effect_context_object: _, // concrete captured-object snapshot + amassed_army_object: _, // concrete captured-object snapshot + ability_index: _, // usize provenance + may_trigger_origin: _, // provenance tag + target_selection_mode: _, // Chosen/Random tag + chosen_players: _, // concrete chosen player ids + replacement_applied: _, // replacement provenance set, no dynamic read + sub_link: _, // SubAbilityLink kind tag + sibling_condition: _, // SiblingCondition replication marker, no dynamic read parent_target_missing_reason: _, // seam flag } = a; diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index b7c47102bc..82d464bedf 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -7127,11 +7127,9 @@ pub(crate) fn optional_prompt_player(state: &GameState, ability: &ResolvedAbilit // who receives this choice. The reference resolves from the trigger event, // preserving the event-time controller rather than inferring from effect shape. if let Some(optional_player) = &ability.optional_player { - if let Some(player) = crate::game::targeting::resolve_effect_player_ref( - state, - ability, - optional_player, - ) { + if let Some(player) = + crate::game::targeting::resolve_effect_player_ref(state, ability, optional_player) + { return player; } } diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 6533254d7c..dd946095ab 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -53,11 +53,11 @@ use crate::types::ability::{ CoinFlipResult, Comparator, ControllerRef, CountScope, CounterTriggerFilter, DamageKindFilter, DestinationConstraint, DieResultFilter, Effect, EffectScope, FilterProp, ManaAbilityProducedFilter, ObjectScope, OriginConstraint, ParsedCondition, PlayerFilter, - PlayerScope, PtStat, - PtValueScope, QuantityExpr, QuantityRef, RenownSubject, SacrificeAggregateStat, SacrificeCost, - SacrificeRequirement, SharedQuality, StaticCondition, SubAbilityLink, TapCreaturesRequirement, - TapStateChange, TargetFilter, TriggerCondition, TriggerConstraint, TriggerDefinition, - TypeFilter, TypedFilter, UnlessPayModifier, ZoneChangeClause, + PlayerScope, PtStat, PtValueScope, QuantityExpr, QuantityRef, RenownSubject, + SacrificeAggregateStat, SacrificeCost, SacrificeRequirement, SharedQuality, StaticCondition, + SubAbilityLink, TapCreaturesRequirement, TapStateChange, TargetFilter, TriggerCondition, + TriggerConstraint, TriggerDefinition, TypeFilter, TypedFilter, UnlessPayModifier, + ZoneChangeClause, }; use crate::types::card_type::{is_land_subtype, CoreType}; use crate::types::counter::CounterType; diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 0c0bf44d49..450fc7e38c 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -2524,7 +2524,10 @@ fn conditional_they_may_tap_stamps_triggering_player_as_optional_actor() { ); let execute = def.execute.as_deref().expect("execute ability"); assert!(execute.optional); - assert_eq!(execute.optional_player, Some(TargetFilter::TriggeringPlayer)); + assert_eq!( + execute.optional_player, + Some(TargetFilter::TriggeringPlayer) + ); } /// CR 603.2 + CR 603.6 + CR 608.2k: Only a trigger's direct, untargeted diff --git a/crates/engine/tests/integration/issue_4963_charismatic_conqueror.rs b/crates/engine/tests/integration/issue_4963_charismatic_conqueror.rs index f1cc690623..d08b6d2403 100644 --- a/crates/engine/tests/integration/issue_4963_charismatic_conqueror.rs +++ b/crates/engine/tests/integration/issue_4963_charismatic_conqueror.rs @@ -16,13 +16,7 @@ fn scenario_with_optional_tapper(oracle: &str) -> (GameRunner, ObjectId) { let mut scenario = GameScenario::new(); scenario.at_phase(Phase::PreCombatMain); scenario - .add_creature_from_oracle( - P0, - "Charismatic Conqueror", - 2, - 2, - oracle, - ) + .add_creature_from_oracle(P0, "Charismatic Conqueror", 2, 2, oracle) .id(); let entrant = scenario .add_creature_to_hand_from_oracle(P1, "Untapped Entrant", 1, 1, "") From e4e1d1842df10bb6e29e1b1a313713ed216e6c90 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 10:54:40 -0700 Subject: [PATCH 04/12] test(engine): clarify optional actor contracts --- crates/engine/src/parser/oracle_trigger_tests.rs | 7 +++---- crates/engine/src/types/ability.rs | 12 +++++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 450fc7e38c..c23ff54411 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -2493,10 +2493,9 @@ fn trigger_etb_subject_enters_untapped_attaches_negated_condition() { .expect("decline token continuation"); assert_eq!( decline.condition, - Some(AbilityCondition::EffectOutcome { - signal: crate::types::ability::EffectOutcomeSignal::OptionalEffectPerformed, - }) - .negate(), + Some(AbilityCondition::Not { + condition: Box::new(AbilityCondition::effect_performed()), + }), "the Vampire token must remain the optional tap's decline branch" ); } diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 709392c3f1..cce1415deb 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -18940,11 +18940,13 @@ pub struct AbilityDefinition { pub optional_targeting: bool, /// CR 608.2d: When true, the controller chooses whether to perform this effect ("You may X"). pub optional: bool, - /// CR 608.2d: The event-relative player named by an optional subject (for example, - /// "they may"). `None` keeps the default of this ability's controller. + /// CR 608.2d: Event-relative player named by an optional subject (for example, + /// "they may"). Unlike `optional_for` and `target_chooser`, this selects the + /// resolution-time optional actor; `None` uses this ability's controller. pub optional_player: Option, /// CR 608.2d: When set, an opponent (not the controller) chooses whether to perform this - /// optional effect. Requires `optional: true`. Opponents are prompted in APNAP order. + /// optional effect. Unlike `optional_player` and `target_chooser`, this is an + /// any-opponent permission. Requires `optional: true`; prompts use APNAP order. pub optional_for: Option, /// Variable-count targeting: min/max targets the player can choose. /// When present, resolution enters MultiTargetSelection instead of immediate resolve. @@ -19024,8 +19026,8 @@ pub struct AbilityDefinition { /// CR 601.2c + CR 603.3d: When set, this player (not the controller) announces /// this ability's target(s) at stack placement. `None` = controller chooses /// (default). Mirrors `target_selection_mode` (the same "by-whom are targets - /// selected" axis). Distinct from CR 608.2d resolution-time "of their choice" - /// sacrifices. + /// selected" axis). Unlike `optional_player` and `optional_for`, this is a + /// stack-placement target choice, not a resolution-time optional actor. pub target_chooser: Option, /// CR 608.2c + CR 107.1c: per-iteration loop-continuation predicate, the /// non-count companion to `repeat_for`. When `Some`, the resolution chain From 83c79aa810bd8c881412458176d3f7019e8a3620 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 10:56:53 -0700 Subject: [PATCH 05/12] fix(engine): initialize optional actor in fixtures --- crates/engine/src/game/effects/additional_phase.rs | 1 + crates/engine/src/game/effects/double.rs | 1 + crates/engine/src/game/effects/extra_turn.rs | 1 + .../src/game/effects/grant_extra_loyalty_activations.rs | 1 + crates/engine/src/game/effects/player_counter.rs | 2 ++ crates/engine/src/game/effects/reverse_turn_order.rs | 1 + crates/engine/src/game/effects/skip_next_step.rs | 1 + crates/engine/src/game/effects/skip_next_turn.rs | 1 + crates/engine/src/game/effects/vote.rs | 4 ++++ .../engine/tests/integration/the_chain_veil_loyalty_grants.rs | 1 + 10 files changed, 14 insertions(+) diff --git a/crates/engine/src/game/effects/additional_phase.rs b/crates/engine/src/game/effects/additional_phase.rs index db6c404301..b5a2554305 100644 --- a/crates/engine/src/game/effects/additional_phase.rs +++ b/crates/engine/src/game/effects/additional_phase.rs @@ -282,6 +282,7 @@ mod tests { context: SpellContext::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), diff --git a/crates/engine/src/game/effects/double.rs b/crates/engine/src/game/effects/double.rs index 149061ef06..0c7d856f1c 100644 --- a/crates/engine/src/game/effects/double.rs +++ b/crates/engine/src/game/effects/double.rs @@ -352,6 +352,7 @@ mod tests { may_trigger_origin: None, optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), diff --git a/crates/engine/src/game/effects/extra_turn.rs b/crates/engine/src/game/effects/extra_turn.rs index be995ef991..2de6a6c7d8 100644 --- a/crates/engine/src/game/effects/extra_turn.rs +++ b/crates/engine/src/game/effects/extra_turn.rs @@ -87,6 +87,7 @@ mod tests { context: SpellContext::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), diff --git a/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs b/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs index 1dc37343fd..7f8542b864 100644 --- a/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs +++ b/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs @@ -105,6 +105,7 @@ mod tests { context: SpellContext::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), diff --git a/crates/engine/src/game/effects/player_counter.rs b/crates/engine/src/game/effects/player_counter.rs index b564934717..26af405939 100644 --- a/crates/engine/src/game/effects/player_counter.rs +++ b/crates/engine/src/game/effects/player_counter.rs @@ -455,6 +455,7 @@ mod tests { context: SpellContext::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), @@ -652,6 +653,7 @@ mod tests { context: SpellContext::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), diff --git a/crates/engine/src/game/effects/reverse_turn_order.rs b/crates/engine/src/game/effects/reverse_turn_order.rs index b851afa6d9..48e429ce4c 100644 --- a/crates/engine/src/game/effects/reverse_turn_order.rs +++ b/crates/engine/src/game/effects/reverse_turn_order.rs @@ -60,6 +60,7 @@ mod tests { context: SpellContext::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), diff --git a/crates/engine/src/game/effects/skip_next_step.rs b/crates/engine/src/game/effects/skip_next_step.rs index 37bfb8c4ac..33549edca1 100644 --- a/crates/engine/src/game/effects/skip_next_step.rs +++ b/crates/engine/src/game/effects/skip_next_step.rs @@ -123,6 +123,7 @@ mod tests { context: SpellContext::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), diff --git a/crates/engine/src/game/effects/skip_next_turn.rs b/crates/engine/src/game/effects/skip_next_turn.rs index 0f6d74987a..608b7c2caf 100644 --- a/crates/engine/src/game/effects/skip_next_turn.rs +++ b/crates/engine/src/game/effects/skip_next_turn.rs @@ -101,6 +101,7 @@ mod tests { context: SpellContext::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), diff --git a/crates/engine/src/game/effects/vote.rs b/crates/engine/src/game/effects/vote.rs index bc0cc7e63f..a5457009f7 100644 --- a/crates/engine/src/game/effects/vote.rs +++ b/crates/engine/src/game/effects/vote.rs @@ -952,6 +952,7 @@ mod tests { context: Default::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), @@ -1063,6 +1064,7 @@ mod tests { context: Default::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), @@ -1499,6 +1501,7 @@ mod tests { context: Default::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), @@ -1667,6 +1670,7 @@ mod tests { context: Default::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), diff --git a/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs b/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs index 5f118cea0d..d5355c4176 100644 --- a/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs +++ b/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs @@ -168,6 +168,7 @@ fn make_grant_ability(controller: PlayerId, source: ObjectId) -> ResolvedAbility replacement_applied: Default::default(), optional_targeting: false, optional: false, + optional_player: None, optional_for: None, multi_target: None, target_constraints: Vec::new(), From df1486ea449a800b68aabaf577b63a7f78042b73 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 11:11:59 -0700 Subject: [PATCH 06/12] test(parser): update optional actor snapshots --- ...parser__oracle_ir__snapshot_tests__bonecrusher_giant_ir.snap | 1 + ...cle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snap | 1 + ...ine__parser__oracle_ir__snapshot_tests__goblin_guide_ir.snap | 1 + ...ser__oracle_ir__snapshot_tests__karn_legacy_reforged_ir.snap | 1 + ...er__oracle_ir__snapshot_tests__liliana_the_repentant_ir.snap | 1 + ...arser__oracle_ir__snapshot_tests__luminarch_aspirant_ir.snap | 1 + ...er__oracle_ir__snapshot_tests__odric_lunarch_marshal_ir.snap | 1 + ..._parser__oracle_ir__snapshot_tests__smugglers_copter_ir.snap | 1 + ...rser__oracle_ir__snapshot_tests__valakut_exploration_ir.snap | 2 ++ 9 files changed, 10 insertions(+) diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__bonecrusher_giant_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__bonecrusher_giant_ir.snap index d342e0155b..9e2219bcfc 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__bonecrusher_giant_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__bonecrusher_giant_ir.snap @@ -119,6 +119,7 @@ expression: "&ir" }, "modifiers": { "optional": false, + "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snap index 4977b7c740..32dd3e7323 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snap @@ -136,6 +136,7 @@ expression: "&ir" }, "modifiers": { "optional": false, + "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__goblin_guide_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__goblin_guide_ir.snap index 707aa83ea1..53b088c6d9 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__goblin_guide_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__goblin_guide_ir.snap @@ -204,6 +204,7 @@ expression: "&ir" }, "modifiers": { "optional": false, + "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__karn_legacy_reforged_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__karn_legacy_reforged_ir.snap index 775c9f7cbe..4b58b2ffe9 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__karn_legacy_reforged_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__karn_legacy_reforged_ir.snap @@ -315,6 +315,7 @@ expression: "&ir" }, "modifiers": { "optional": false, + "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_the_repentant_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_the_repentant_ir.snap index f8ae6edf02..0e48077bf2 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_the_repentant_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_the_repentant_ir.snap @@ -144,6 +144,7 @@ expression: "&ir" }, "modifiers": { "optional": false, + "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__luminarch_aspirant_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__luminarch_aspirant_ir.snap index ba86aca22e..416daba635 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__luminarch_aspirant_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__luminarch_aspirant_ir.snap @@ -123,6 +123,7 @@ expression: "&ir" }, "modifiers": { "optional": false, + "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__odric_lunarch_marshal_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__odric_lunarch_marshal_ir.snap index 811eca9597..62850ea562 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__odric_lunarch_marshal_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__odric_lunarch_marshal_ir.snap @@ -224,6 +224,7 @@ expression: "&ir" }, "modifiers": { "optional": false, + "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__smugglers_copter_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__smugglers_copter_ir.snap index 58b10c8b0e..b6cfc6c0ea 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__smugglers_copter_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__smugglers_copter_ir.snap @@ -203,6 +203,7 @@ expression: "&ir" }, "modifiers": { "optional": true, + "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__valakut_exploration_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__valakut_exploration_ir.snap index 5d03043e54..45aa719532 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__valakut_exploration_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__valakut_exploration_ir.snap @@ -185,6 +185,7 @@ expression: "&ir" }, "modifiers": { "optional": false, + "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { @@ -375,6 +376,7 @@ expression: "&ir" }, "modifiers": { "optional": false, + "optional_player": null, "unless_pay": null, "intervening_if": { "type": "QuantityComparison", From 97bbd29ec6771f950a9a0b9c65e38cbba45822ca Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 11:17:08 -0700 Subject: [PATCH 07/12] fix(parser): omit absent optional actor from IR --- ...parser__oracle_ir__snapshot_tests__bonecrusher_giant_ir.snap | 1 - ...cle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snap | 1 - ...ine__parser__oracle_ir__snapshot_tests__goblin_guide_ir.snap | 1 - ...ser__oracle_ir__snapshot_tests__karn_legacy_reforged_ir.snap | 1 - ...er__oracle_ir__snapshot_tests__liliana_the_repentant_ir.snap | 1 - ...arser__oracle_ir__snapshot_tests__luminarch_aspirant_ir.snap | 1 - ...er__oracle_ir__snapshot_tests__odric_lunarch_marshal_ir.snap | 1 - ..._parser__oracle_ir__snapshot_tests__smugglers_copter_ir.snap | 1 - ...rser__oracle_ir__snapshot_tests__valakut_exploration_ir.snap | 2 -- crates/engine/src/parser/oracle_ir/trigger.rs | 1 + 10 files changed, 1 insertion(+), 10 deletions(-) diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__bonecrusher_giant_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__bonecrusher_giant_ir.snap index 9e2219bcfc..d342e0155b 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__bonecrusher_giant_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__bonecrusher_giant_ir.snap @@ -119,7 +119,6 @@ expression: "&ir" }, "modifiers": { "optional": false, - "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snap index 32dd3e7323..4977b7c740 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snap @@ -136,7 +136,6 @@ expression: "&ir" }, "modifiers": { "optional": false, - "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__goblin_guide_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__goblin_guide_ir.snap index 53b088c6d9..707aa83ea1 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__goblin_guide_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__goblin_guide_ir.snap @@ -204,7 +204,6 @@ expression: "&ir" }, "modifiers": { "optional": false, - "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__karn_legacy_reforged_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__karn_legacy_reforged_ir.snap index 4b58b2ffe9..775c9f7cbe 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__karn_legacy_reforged_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__karn_legacy_reforged_ir.snap @@ -315,7 +315,6 @@ expression: "&ir" }, "modifiers": { "optional": false, - "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_the_repentant_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_the_repentant_ir.snap index 0e48077bf2..f8ae6edf02 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_the_repentant_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_the_repentant_ir.snap @@ -144,7 +144,6 @@ expression: "&ir" }, "modifiers": { "optional": false, - "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__luminarch_aspirant_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__luminarch_aspirant_ir.snap index 416daba635..ba86aca22e 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__luminarch_aspirant_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__luminarch_aspirant_ir.snap @@ -123,7 +123,6 @@ expression: "&ir" }, "modifiers": { "optional": false, - "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__odric_lunarch_marshal_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__odric_lunarch_marshal_ir.snap index 62850ea562..811eca9597 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__odric_lunarch_marshal_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__odric_lunarch_marshal_ir.snap @@ -224,7 +224,6 @@ expression: "&ir" }, "modifiers": { "optional": false, - "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__smugglers_copter_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__smugglers_copter_ir.snap index b6cfc6c0ea..58b10c8b0e 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__smugglers_copter_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__smugglers_copter_ir.snap @@ -203,7 +203,6 @@ expression: "&ir" }, "modifiers": { "optional": true, - "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__valakut_exploration_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__valakut_exploration_ir.snap index 45aa719532..5d03043e54 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__valakut_exploration_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__valakut_exploration_ir.snap @@ -185,7 +185,6 @@ expression: "&ir" }, "modifiers": { "optional": false, - "optional_player": null, "unless_pay": null, "intervening_if": null, "trigger_subject": { @@ -376,7 +375,6 @@ expression: "&ir" }, "modifiers": { "optional": false, - "optional_player": null, "unless_pay": null, "intervening_if": { "type": "QuantityComparison", diff --git a/crates/engine/src/parser/oracle_ir/trigger.rs b/crates/engine/src/parser/oracle_ir/trigger.rs index a2e97315aa..cd1d8ce3e6 100644 --- a/crates/engine/src/parser/oracle_ir/trigger.rs +++ b/crates/engine/src/parser/oracle_ir/trigger.rs @@ -293,6 +293,7 @@ pub(crate) struct TriggerModifiers { pub(crate) optional: bool, /// CR 608.2d: Event-relative player explicitly named by the root optional /// subject, after any intervening-if wrapper has been removed. + #[serde(skip_serializing_if = "Option::is_none")] pub(crate) optional_player: Option, /// CR 118.12: "unless [player] pays {cost}" tax modifier. pub(crate) unless_pay: Option, From 70eca9c12a12a8f76fe7830e2dac0dbf34504889 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 11:35:06 -0700 Subject: [PATCH 08/12] test(engine): refresh optional prompt census pin --- crates/engine/src/game/engine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 765dcc7a9d..056adb4ca2 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -16115,7 +16115,7 @@ mod stage2_injector_tests { // in the merged source, still in their named production functions. "game/effects/mod.rs:6640".to_string(), "game/effects/mod.rs:6717".to_string(), - "game/effects/mod.rs:9922".to_string(), + "game/effects/mod.rs:9932".to_string(), // UNMOVED across the rebase, and that is itself evidence the SET did not // move: a census that had gained or lost a producer would not leave this // entry both byte-identical AND at the same coordinate. From 2e16fb6491fe513c167a9d3d8bed61ffbef012e4 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 11:42:52 -0700 Subject: [PATCH 09/12] fix(parser): mark they-may triggers optional --- crates/engine/src/parser/oracle_trigger.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index dd946095ab..42d37a3fe3 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -1410,7 +1410,8 @@ pub(crate) fn parse_trigger_line_with_index_ir( .strip_prefix("if ") // allow-noncombinator: structural if-clause skip when condition is unrecognized .and_then(|rest| rest.split_once(", ")) .map(|(_cond, body)| body); - let mut optional = starts_with_you_may(effect_lower.as_str()) + let mut optional = optional_player.is_some() + || starts_with_you_may(effect_lower.as_str()) || starts_with_you_may(effect_without_if.trim_start()) || after_structural_if.is_some_and(starts_with_you_may); From 4e125a438ca294fae04a77842ddd1f88525b8dd0 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 12:02:00 -0700 Subject: [PATCH 10/12] fix(parser): retain structural they-may actor --- crates/engine/src/parser/oracle_trigger.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 42d37a3fe3..47e5a8024a 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -1364,6 +1364,10 @@ pub(crate) fn parse_trigger_line_with_index_ir( let cond_lower = condition_text.to_lowercase(); let effect_lower = effect_text.to_lowercase(); + let after_structural_if = effect_lower + .strip_prefix("if ") // allow-noncombinator: structural if-clause skip when condition is unrecognized + .and_then(|rest| rest.split_once(", ")) + .map(|(_cond, body)| body); // CR 701.42b: A meld instigator's effect text opens with the own/control // gate ("if you both own and control ~ and a [type] named [partner], exile // them, then meld them into [result]"). Recognize it as a unit: the gate @@ -1387,7 +1391,8 @@ pub(crate) fn parse_trigger_line_with_index_ir( (without_if, cond, None) } }; - let optional_player = optional_player_from_effect_body(&effect_without_if); + let optional_player = optional_player_from_effect_body(&effect_without_if) + .or_else(|| after_structural_if.and_then(optional_player_from_effect_body)); // CR 608.2c (resolution-order instructions): "You may" at the start of // the effect text makes the triggered effect optional at resolution. @@ -1406,10 +1411,6 @@ pub(crate) fn parse_trigger_line_with_index_ir( // The detection below only fires when the `you may` is the FIRST token // (modulo an intervening-if), which excludes the multi-sentence case. let starts_with_you_may = |s: &str| tag::<_, _, OracleError<'_>>("you may ").parse(s).is_ok(); - let after_structural_if = effect_lower - .strip_prefix("if ") // allow-noncombinator: structural if-clause skip when condition is unrecognized - .and_then(|rest| rest.split_once(", ")) - .map(|(_cond, body)| body); let mut optional = optional_player.is_some() || starts_with_you_may(effect_lower.as_str()) || starts_with_you_may(effect_without_if.trim_start()) From da5dca973f9fe0147be8dfb6135c043377c08041 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 12:35:03 -0700 Subject: [PATCH 11/12] fix(parser): bind inline modal tap modes to event source --- crates/engine/src/parser/oracle_trigger.rs | 8 ++++ .../engine/src/parser/oracle_trigger_tests.rs | 46 ++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 47e5a8024a..04e4d5e9b2 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -2298,6 +2298,14 @@ fn first_independent_sibling_after_search( /// Without the descent, the second link would silently bind to the trigger /// source object instead of the just-acted-on event object. fn lift_parent_target_to_triggering_source_in_ability(ability: &mut AbilityDefinition) { + // CR 608.2c + CR 608.2k: An inline modal stores each mode outside the + // ordinary sub-ability chain. Each mode is nevertheless a root instruction + // of this event-source trigger, so it needs the same narrow rewrite before + // the modal choice selects one; a chosen target inside a mode remains + // protected by this walk's existing boundary. + for mode in &mut ability.mode_abilities { + lift_parent_target_to_triggering_source_in_ability(mode); + } // CR 608.2c + CR 608.2k: Stop the descent as soon as a link introduces a // player-*chosen* object target. A later `ParentTarget` then refers to // *that* choice, not the trigger event — the enters-flicker class diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index c23ff54411..728460ba50 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -11,8 +11,9 @@ use crate::types::ability::{ Comparator, ContinuousModification, ControllerRef, CopyChooseScope, CopyRetargetPermission, CountScope, DamageChannel, DamageModification, DamageSource, DelayedTriggerCondition, DiscardSelfScope, Duration, Effect, EffectScope, FilterProp, ManaContribution, ManaProduction, - ManaSpendPermission, ObjectScope, PerpetualModification, PlayerFilter, PlayerScope, PtStat, - PtValue, PtValueScope, QuantityExpr, QuantityRef, SeatDirection, SharedQuality, + ManaSpendPermission, ModalChoice, ObjectScope, PerpetualModification, PlayerFilter, + PlayerScope, PtStat, PtValue, PtValueScope, QuantityExpr, QuantityRef, SeatDirection, + SharedQuality, SiblingCondition, SubAbilityLink, TapStateChange, TargetFilter, TriggerCondition, TypeFilter, TypedFilter, ZoneRef, }; @@ -28966,3 +28967,44 @@ fn synthetic_sentence_separated_mass_move_damage_keeps_event_context_amount() { other => panic!("expected DamageEachPlayer(EventContextAmount, Opponent), got {other:?}"), } } + +/// SHAPE — inline modal roots are independent of `sub_ability`, but a +/// targetless top-level tap in each mode still refers to the zone-change event +/// source. This pins the parser's event-source rewrite without widening it +/// through an explicitly chosen target. +#[test] +fn event_source_lift_rewrites_inline_modal_tap_mode_roots() { + let mode = AbilityDefinition::new( + AbilityKind::Spell, + Effect::SetTapState { + target: TargetFilter::ParentTarget, + scope: EffectScope::Single, + state: TapStateChange::Tap, + }, + ); + let mut root = AbilityDefinition::new( + AbilityKind::Spell, + Effect::unimplemented("modal marker", "Choose one"), + ) + .with_modal( + ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 1, + mode_descriptions: vec!["Tap that permanent.".to_string()], + ..Default::default() + }, + vec![mode], + ); + + lift_parent_target_to_triggering_source_in_ability(&mut root); + + assert!(matches!( + root.mode_abilities[0].effect.as_ref(), + Effect::SetTapState { + target: TargetFilter::TriggeringSource, + scope: EffectScope::Single, + state: TapStateChange::Tap, + } + )); +} From 2036780059aa9ae75b6899d56fc34dfd98902174 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 12:35:25 -0700 Subject: [PATCH 12/12] fix(parser): bind inline modal tap modes to event source --- crates/engine/src/parser/oracle_trigger_tests.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 728460ba50..e391b32196 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -13,9 +13,8 @@ use crate::types::ability::{ DiscardSelfScope, Duration, Effect, EffectScope, FilterProp, ManaContribution, ManaProduction, ManaSpendPermission, ModalChoice, ObjectScope, PerpetualModification, PlayerFilter, PlayerScope, PtStat, PtValue, PtValueScope, QuantityExpr, QuantityRef, SeatDirection, - SharedQuality, - SiblingCondition, SubAbilityLink, TapStateChange, TargetFilter, TriggerCondition, TypeFilter, - TypedFilter, ZoneRef, + SharedQuality, SiblingCondition, SubAbilityLink, TapStateChange, TargetFilter, + TriggerCondition, TypeFilter, TypedFilter, ZoneRef, }; use crate::types::card_type::Supertype; use crate::types::counter::{CounterMatch, CounterType};