diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index 2b80fb3d8f..88715b9a29 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -3696,6 +3696,7 @@ fn walk_ability( target_selection_mode: _, chosen_players: _, sub_link: _, + sibling_condition: _, // replication marker, no read/write effect replacement_applied: _, parent_target_missing_reason: _, } = a; @@ -3816,6 +3817,7 @@ fn walk_definition( target_selection_mode: _, sub_link: _, iteration_kind_binding: _, + sibling_condition: _, } = a; // §4.3.2: own `player_scope` overrides the inherited scope (Brink's Discard diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index f60544b1b9..3ff5bb6ffa 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -258,6 +258,7 @@ fn resolved_ability_axes(a: &ResolvedAbility, mode: ScanMode) -> Axes { 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; @@ -4240,6 +4241,7 @@ fn ability_definition_axes(def: &AbilityDefinition, mode: ScanMode) -> Axes { target_selection_mode: _, sub_link: _, iteration_kind_binding: _, + sibling_condition: _, } = def; let mut acc = scan_effect(effect, mode); @@ -6451,6 +6453,7 @@ pub(crate) fn ability_resolution_choice_freedom(a: &ResolvedAbility) -> Resoluti chosen_players: _, // concrete chosen player ids (already selected) replacement_applied: _, // replacement provenance set, no prompt sub_link: _, // SubAbilityLink kind tag + sibling_condition: _, // SiblingCondition replication marker, no resolution-time choice parent_target_missing_reason: _, // seam flag } = a; diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index 803f41ca9e..691957cd34 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -157,6 +157,13 @@ pub fn build_resolved_from_def_with_targets( // CR 608.2c: Carry the parent-link kind through so the decline classifier can // distinguish a separate-sentence sibling from a within-clause continuation. resolved.sub_link = def.sub_link; + // CR 702.1c ("the same is true") + CR 608.2c (written order): Carry the + // replication marker through so `resolve_chain_body` evaluates a + // `ReplicatedOrBranch` per-item OR-branch (Mutable Pupa, Kathril) + // independently of a preceding sibling's failed gate. Without this copy the + // parser-stamped `SiblingCondition` never reaches the resolved sub and the + // keyword list collapses after the first false gate. + resolved.sibling_condition = def.sibling_condition; // CR 700.2b + CR 603.3c: Carry the reflexive modal choice + per-mode abilities // through so try_begin_reflexive_target_selection can route a gated modal // trigger (Caesar) to AbilityModeChoice instead of resolving the modes diff --git a/crates/engine/src/game/effects/additional_phase.rs b/crates/engine/src/game/effects/additional_phase.rs index c8fca38d9c..ebc1811c7e 100644 --- a/crates/engine/src/game/effects/additional_phase.rs +++ b/crates/engine/src/game/effects/additional_phase.rs @@ -307,6 +307,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, diff --git a/crates/engine/src/game/effects/double.rs b/crates/engine/src/game/effects/double.rs index d1e61cc247..2e79d947cf 100644 --- a/crates/engine/src/game/effects/double.rs +++ b/crates/engine/src/game/effects/double.rs @@ -359,6 +359,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, diff --git a/crates/engine/src/game/effects/extra_turn.rs b/crates/engine/src/game/effects/extra_turn.rs index a3a38b41a4..a319d7f88d 100644 --- a/crates/engine/src/game/effects/extra_turn.rs +++ b/crates/engine/src/game/effects/extra_turn.rs @@ -104,6 +104,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, 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 0b7baed07d..7ff475faec 100644 --- a/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs +++ b/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs @@ -73,7 +73,9 @@ pub fn resolve( #[cfg(test)] mod tests { use super::*; - use crate::types::ability::{AbilityKind, QuantityExpr, SpellContext, SubAbilityLink}; + use crate::types::ability::{ + AbilityKind, QuantityExpr, SiblingCondition, SpellContext, SubAbilityLink, + }; use crate::types::identifiers::ObjectId; use crate::types::player::PlayerId; @@ -128,6 +130,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: SubAbilityLink::ContinuationStep, + sibling_condition: SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index df0396d9f0..6c45909d2c 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -14,7 +14,8 @@ use crate::types::ability::{ EffectKind, EffectOutcomeSignal, EffectScope, FilterProp, OpponentMayScope, PlayerFilter, PlayerScope, PtValue, QuantityExpr, QuantityRef, RepeatContinuation, ResolvedAbility, RevealUntilDisposition, SacrificeCost, SacrificeRequirement, SharedQuality, - SharedQualityRelation, SubAbilityLink, TapStateChange, TargetFilter, TargetRef, ThisWayCause, + SharedQualityRelation, SiblingCondition, SubAbilityLink, TapStateChange, TargetChoiceTiming, + TargetFilter, TargetRef, ThisWayCause, }; #[cfg(test)] use crate::types::ability::{AttackScope, AttackSubject}; @@ -1644,7 +1645,7 @@ pub(crate) fn resolve_effect_pay_cost_rider( return Ok(()); }; let mut rider = sub.as_ref().clone(); - if rider.targets.is_empty() && !ability.targets.is_empty() { + if should_propagate_parent_targets(ability, &rider) { rider.targets = ability.targets.clone(); } apply_parent_chain_context(&mut rider, ability, None, state); @@ -1670,7 +1671,7 @@ pub(crate) fn prepend_remaining_pay_cost_continuation( if let Some(sub) = ability.sub_ability.as_ref() { let mut sub_clone = sub.as_ref().clone(); - if sub_clone.targets.is_empty() && !ability.targets.is_empty() { + if should_propagate_parent_targets(ability, &sub_clone) { sub_clone.targets = ability.targets.clone(); } apply_parent_chain_context(&mut sub_clone, ability, None, state); @@ -2418,6 +2419,22 @@ fn apply_parent_chain_context( } } +/// CR 608.2d: whether `ability`'s already-chosen targets should propagate into +/// an empty-targeted `sub`. Single authority for every `sub.targets = +/// ability.targets.clone()` propagation site in this file — a `Resolution`- +/// timed sub makes its OWN untargeted choice at its own resolution (see the +/// interactive `PutCounter` recipient prompt in `resolve_chain_body`) and must +/// NOT inherit an earlier instruction's already-chosen recipient, or every +/// replicated instruction in a chain collapses onto whichever single object +/// the first one picked (Kathril, Aspect Warper, issue #6321 / PR #6533). +/// Every other sub keeps today's behavior: parent targets propagate when the +/// sub declares none of its own. +fn should_propagate_parent_targets(ability: &ResolvedAbility, sub: &ResolvedAbility) -> bool { + sub.targets.is_empty() + && !ability.targets.is_empty() + && sub.target_choice_timing != TargetChoiceTiming::Resolution +} + fn waits_for_resolution_choice(waiting_for: &WaitingFor) -> bool { matches!( waiting_for, @@ -2618,8 +2635,7 @@ pub(super) fn resolve_optional_effect_decision( // carries its OWN target filter (e.g. a Chaos-Wand cleanup that // returns `ExiledBySource` cards) from being clobbered with the // parent's targets. - if resolved.targets.is_empty() - && !ability.targets.is_empty() + if should_propagate_parent_targets(&ability, &resolved) && effect_refs_parent_target(&resolved.effect) { resolved.targets = ability.targets.clone(); @@ -8211,7 +8227,7 @@ fn resolve_chain_body( if !evaluate_condition(condition, state, ability) { if let Some(ref else_branch) = ability.else_ability { let mut else_resolved = else_branch.as_ref().clone(); - if else_resolved.targets.is_empty() && !ability.targets.is_empty() { + if should_propagate_parent_targets(ability, &else_resolved) { else_resolved.targets = ability.targets.clone(); } else_resolved.context = ability.context.clone(); @@ -8263,16 +8279,44 @@ fn resolve_chain_body( sub.condition.as_ref(), Some(AbilityCondition::PostReplacementDamageSourceMatchesFilter { .. }) ); + // CR 702.1c ("the same is true") + CR 608.2c (written order): A + // sub produced by per-item keyword-list replication + // (`SiblingCondition::ReplicatedOrBranch`) is an INDEPENDENT + // OR-branch gated on its OWN keyword — Mutable Pupa's "perpetually + // gains if that creature has " and Kathril's "put a + // counter if a creature card in your graveyard has ". + // Its gate references neither this node's effect nor this node's + // keyword, so it must be evaluated regardless of whether this + // node's own gate (K_j's keyword check) held. Without this, once + // any earlier sibling's gate is false the rest of the keyword list + // never resolves ("list collapse"). Same shape of independent + // per-branch gate as `PostReplacementDamageSourceMatchesFilter` + // above, keyed on the replication marker rather than the condition + // variant (the gate here is a plain `ZoneChangeObjectMatchesFilter` + // / `QuantityCheck` that would otherwise look dependent). + let sub_is_replicated_or_branch = sub.sibling_condition + == SiblingCondition::ReplicatedOrBranch + && sub.sub_link == SubAbilityLink::SequentialSibling; if sub .condition .as_ref() .is_some_and(condition_depends_on_effect_performed) || sub_has_independent_event_gate + || sub_is_replicated_or_branch || (sub.sub_link == SubAbilityLink::SequentialSibling && sub.condition.is_none()) { let mut sub_resolved = sub.as_ref().clone(); - if sub_resolved.targets.is_empty() && !ability.targets.is_empty() { + // CR 608.2d: a `Resolution`-timed sub makes its OWN + // untargeted choice at its own resolution (see the + // interactive `PutCounter` recipient prompt in this + // function) — inheriting the parent's already-chosen + // target here would force every replicated instruction + // onto whichever single recipient the parent picked, + // instead of letting each independently-gated sibling + // offer its own choice (Kathril, Aspect Warper, issue + // #6321 / PR #6533). + if should_propagate_parent_targets(ability, &sub_resolved) { sub_resolved.targets = ability.targets.clone(); } sub_resolved.context = ability.context.clone(); @@ -8645,6 +8689,84 @@ fn resolve_chain_body( } } + // CR 608.2d + CR 115.10a: An untargeted `PutCounter` recipient ("any + // creature you control", no literal "target") is chosen while APPLYING + // the effect — at THIS instruction's OWN resolution, independently of any + // sibling instruction's own choice — not once, when the whole ability + // went on the stack. `target_choice_timing_for_clause` (parser) marks + // such clauses `Resolution`; the `ReplicatedOrBranch` propagation-skip + // above (via `should_propagate_parent_targets`) ensures such a node + // reaches here with `ability.targets` still empty rather than inheriting + // a parent's already-made choice. Mirrors the existing + // `filter_chosen_player_index` 0/1/N pattern above (a single legal + // recipient auto-binds with no prompt; zero legal recipients is a silent + // no-op — CR 608.2d: "The player can't choose an option that's illegal or + // impossible") and reuses the SAME `ChooseFromZoneChoice` + + // parked-continuation machinery already proven for `PutCounter` + // continuations — `engine_resolution_choices.rs`'s `is_partition` gate on + // this exact drain path names `Effect::PutCounter` explicitly (the + // Bolster keyword action). Kathril, Aspect Warper — issue #6321 / PR + // #6533. + // + // Candidates are enumerated via a PLAIN filter scan (`matches_target_filter` + // over the battlefield), NOT the targeting-legality path + // (`find_legal_targets`/`can_target`) — per CR 115.10a this is a CHOICE, not + // a target, so hexproof/shroud/protection/"can't be the target of" must NOT + // restrict candidacy. Mirrors `MultiplyCounter`'s existing untargeted + // enumeration a few lines below and `bolster.rs`'s own doc comment + // ("Bolster is a keyword action that 'chooses' (not 'targets') — hexproof + // and shroud do not prevent bolster"). + // + // EXCLUDES `contains_source_attachment_host()` (Equipped/Enchanted) + // targets — those have no real CHOICE (the host is uniquely determined by + // the attachment relationship) and already resolve deterministically via + // `resolve_defined_or_targets`'s existing `resolved_object_ids_for_filter` + // fallback in `counters.rs`; routing them through an interactive prompt + // here would be wrong (and untested against that resolver's semantics). + if ability.target_choice_timing == TargetChoiceTiming::Resolution + && ability.targets.is_empty() + && ability.distribution.is_none() + { + if let Effect::PutCounter { target, .. } = &ability.effect { + if !target.contains_source_attachment_host() { + let effective_filter = resolved_object_filter(ability, target); + let filter_ctx = filter::FilterContext::from_ability(ability); + let legal: Vec = state + .battlefield_phased_in_ids() + .into_iter() + .filter(|id| { + filter::matches_target_filter(state, *id, &effective_filter, &filter_ctx) + }) + .collect(); + match legal.len() { + 0 => {} + 1 => { + let mut bound = ability.clone(); + bound.targets = vec![TargetRef::Object(legal[0])]; + return resolve_ability_chain(state, &bound, events, depth); + } + _ => { + let mut cont = ability.clone(); + cont.targets.clear(); + state.park_ability_continuation(PendingContinuation::new( + Box::new(cont), + state, + )); + state.waiting_for = WaitingFor::ChooseFromZoneChoice { + player: ability.controller, + cards: legal, + count: 1, + up_to: false, + constraint: None, + source_id: ability.source_id, + }; + return Ok(()); + } + } + } + } + } + // CR 603.7: Snapshot event count so we can detect objects moved by this effect. let events_before = events.len(); @@ -9288,7 +9410,7 @@ fn resolve_chain_body( ) { if let Some(ref base_chain) = sub.else_ability { let mut resolved = base_chain.as_ref().clone(); - if resolved.targets.is_empty() && !ability.targets.is_empty() { + if should_propagate_parent_targets(ability, &resolved) { resolved.targets = ability.targets.clone(); } apply_parent_chain_context( @@ -9353,7 +9475,7 @@ fn resolve_chain_body( resolved.targets.insert(0, TargetRef::Object(source)); } } - } else if resolved.targets.is_empty() && !ability.targets.is_empty() { + } else if should_propagate_parent_targets(ability, &resolved) { resolved.targets = ability.targets.clone(); } apply_parent_chain_context( @@ -9424,7 +9546,7 @@ fn resolve_chain_body( || condition_awaits_resolution_only_referent(condition, state, ability)) { let mut sub_clone = sub.as_ref().clone(); - if sub_clone.targets.is_empty() && !ability.targets.is_empty() { + if should_propagate_parent_targets(ability, &sub_clone) { sub_clone.targets = ability.targets.clone(); } apply_parent_chain_context( @@ -9503,7 +9625,7 @@ fn resolve_chain_body( .iter() .map(|&id| TargetRef::Object(id)) .collect(); - } else if else_resolved.targets.is_empty() && !ability.targets.is_empty() { + } else if should_propagate_parent_targets(ability, &else_resolved) { else_resolved.targets = ability.targets.clone(); } apply_parent_chain_context( @@ -9539,7 +9661,7 @@ fn resolve_chain_body( while let Some(ref sibling) = current { if sibling.sub_link == SubAbilityLink::SequentialSibling { let mut sibling_resolved = sibling.as_ref().clone(); - if sibling_resolved.targets.is_empty() && !ability.targets.is_empty() { + if should_propagate_parent_targets(ability, &sibling_resolved) { sibling_resolved.targets = ability.targets.clone(); } apply_parent_chain_context( @@ -9668,7 +9790,7 @@ fn resolve_chain_body( && waits_for_resolution_choice(&state.waiting_for) { let mut sub_clone = sub.as_ref().clone(); - if sub_clone.targets.is_empty() && !ability.targets.is_empty() { + if should_propagate_parent_targets(ability, &sub_clone) { sub_clone.targets = ability.targets.clone(); } apply_parent_chain_context( @@ -9701,7 +9823,7 @@ fn resolve_chain_body( // rather than immediately processing it (which would bypass the UI). if waits_for_resolution_choice(&state.waiting_for) { let mut sub_clone = sub.as_ref().clone(); - if sub_clone.targets.is_empty() && !ability.targets.is_empty() { + if should_propagate_parent_targets(ability, &sub_clone) { sub_clone.targets = ability.targets.clone(); } apply_parent_chain_context( @@ -9848,6 +9970,14 @@ fn resolve_chain_body( } } else if sub_with_context.targets.is_empty() && !effect_uses_implicit_tracked_set_targets(&sub.effect) + // CR 608.2d: a `Resolution`-timed sub makes its OWN untargeted + // choice at its own resolution — neither inheriting the + // parent's target NOR force-binding the just-moved + // forward_result object is correct for it; leave `targets` + // empty so the interactive `PutCounter` recipient prompt + // handles it when this sub's own turn to resolve comes + // (Kathril, Aspect Warper, issue #6321 / PR #6533). + && sub_with_context.target_choice_timing != TargetChoiceTiming::Resolution { // CR 608.2c: ParentTarget consumers in a forward_result sub-chain // need the moved object's id in `targets`, not just a rebound diff --git a/crates/engine/src/game/effects/player_counter.rs b/crates/engine/src/game/effects/player_counter.rs index ad72642be6..7b6674444a 100644 --- a/crates/engine/src/game/effects/player_counter.rs +++ b/crates/engine/src/game/effects/player_counter.rs @@ -373,6 +373,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, @@ -565,6 +566,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, diff --git a/crates/engine/src/game/effects/reverse_turn_order.rs b/crates/engine/src/game/effects/reverse_turn_order.rs index 9315c07194..267d1491c7 100644 --- a/crates/engine/src/game/effects/reverse_turn_order.rs +++ b/crates/engine/src/game/effects/reverse_turn_order.rs @@ -85,6 +85,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, diff --git a/crates/engine/src/game/effects/skip_next_step.rs b/crates/engine/src/game/effects/skip_next_step.rs index 6f8e81459a..c357f10349 100644 --- a/crates/engine/src/game/effects/skip_next_step.rs +++ b/crates/engine/src/game/effects/skip_next_step.rs @@ -148,6 +148,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, diff --git a/crates/engine/src/game/effects/skip_next_turn.rs b/crates/engine/src/game/effects/skip_next_turn.rs index 8f9378f2b2..66715c058b 100644 --- a/crates/engine/src/game/effects/skip_next_turn.rs +++ b/crates/engine/src/game/effects/skip_next_turn.rs @@ -126,6 +126,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, diff --git a/crates/engine/src/game/effects/vote.rs b/crates/engine/src/game/effects/vote.rs index 4051653eed..5602ab2948 100644 --- a/crates/engine/src/game/effects/vote.rs +++ b/crates/engine/src/game/effects/vote.rs @@ -398,6 +398,7 @@ pub fn resolve_tally( repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, @@ -463,6 +464,7 @@ pub fn resolve_tally( repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, @@ -706,6 +708,8 @@ fn resolved_from_def( replacement_applied: Default::default(), // CR 608.2c: Carry the parent-link kind through to the resolved ability. sub_link: def.sub_link, + // CR 608.2c: Carry the replication marker through (Dependent for vote sub-effects). + sibling_condition: def.sibling_condition, // CR 700.2b + CR 603.3c: Carry the reflexive modal choice + per-mode // abilities through (None for vote sub-effects). modal: def.modal.clone(), @@ -958,6 +962,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, @@ -1064,6 +1069,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, @@ -1495,6 +1501,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, @@ -1658,6 +1665,7 @@ mod tests { repeat_until: None, replacement_applied: Default::default(), sub_link: crate::types::ability::SubAbilityLink::ContinuationStep, + sibling_condition: crate::types::ability::SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None, diff --git a/crates/engine/src/game/stack.rs b/crates/engine/src/game/stack.rs index 791718071d..bd8aeb191e 100644 --- a/crates/engine/src/game/stack.rs +++ b/crates/engine/src/game/stack.rs @@ -1,8 +1,8 @@ use crate::types::ability::{ AbilityKind, ContinuousModification, CopyCountStatus, Duration, Effect, EffectKind, FilterProp, KeywordAction, ObjectScope, PlayerFilter, QuantityExpr, QuantityRef, ResolvedAbility, - SpellContext, SubAbilityLink, TargetChoiceTiming, TargetFilter, TargetRef, TargetSelectionMode, - TriggerCondition, + SiblingCondition, SpellContext, SubAbilityLink, TargetChoiceTiming, TargetFilter, TargetRef, + TargetSelectionMode, TriggerCondition, }; use crate::types::card_type::CoreType; use crate::types::counter::CounterType; @@ -2297,6 +2297,7 @@ fn self_counter_ability_is_batch_candidate(ability: &ResolvedAbility) -> bool { repeat_until, replacement_applied: _, sub_link, + sibling_condition, modal, mode_abilities, parent_target_missing_reason, @@ -2362,6 +2363,11 @@ fn self_counter_ability_is_batch_candidate(ability: &ResolvedAbility) -> bool { && chosen_players.is_empty() && repeat_until.is_none() && *sub_link == SubAbilityLink::ContinuationStep + // CR 702.1c ("the same is true") + CR 608.2c (written order): a + // `ReplicatedOrBranch` per-item keyword-list sibling (Mutable Pupa, + // Kathril) is not the vanilla batchable shape this proof + // covers — its independent OR-branch gate must be evaluated per entry. + && *sibling_condition == SiblingCondition::Dependent && modal.is_none() && mode_abilities.is_empty() && parent_target_missing_reason.is_none() @@ -2492,6 +2498,7 @@ fn fixed_controller_gain_life_ability_is_batch_candidate(ability: &ResolvedAbili repeat_until, replacement_applied: _, sub_link, + sibling_condition, modal, mode_abilities, parent_target_missing_reason, @@ -2540,6 +2547,11 @@ fn fixed_controller_gain_life_ability_is_batch_candidate(ability: &ResolvedAbili && chosen_players.is_empty() && repeat_until.is_none() && *sub_link == SubAbilityLink::ContinuationStep + // CR 702.1c ("the same is true") + CR 608.2c (written order): a + // `ReplicatedOrBranch` per-item keyword-list sibling (Mutable Pupa, + // Kathril) is not the vanilla batchable shape this proof + // covers — its independent OR-branch gate must be evaluated per entry. + && *sibling_condition == SiblingCondition::Dependent && modal.is_none() && mode_abilities.is_empty() && parent_target_missing_reason.is_none() @@ -2672,6 +2684,7 @@ fn fixed_opponent_lose_life_ability_is_batch_candidate(ability: &ResolvedAbility repeat_until, replacement_applied: _, sub_link, + sibling_condition, modal, mode_abilities, parent_target_missing_reason, @@ -2720,6 +2733,11 @@ fn fixed_opponent_lose_life_ability_is_batch_candidate(ability: &ResolvedAbility && chosen_players.is_empty() && repeat_until.is_none() && *sub_link == SubAbilityLink::ContinuationStep + // CR 702.1c ("the same is true") + CR 608.2c (written order): a + // `ReplicatedOrBranch` per-item keyword-list sibling (Mutable Pupa, + // Kathril) is not the vanilla batchable shape this proof + // covers — its independent OR-branch gate must be evaluated per entry. + && *sibling_condition == SiblingCondition::Dependent && modal.is_none() && mode_abilities.is_empty() && parent_target_missing_reason.is_none() @@ -3277,6 +3295,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( repeat_until: a_repeat_until, replacement_applied: a_replacement_applied, sub_link: a_sub_link, + sibling_condition: a_sibling_condition, modal: a_modal, mode_abilities: a_mode_abilities, parent_target_missing_reason: a_parent_target_missing_reason, @@ -3328,6 +3347,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( repeat_until: b_repeat_until, replacement_applied: b_replacement_applied, sub_link: b_sub_link, + sibling_condition: b_sibling_condition, modal: b_modal, mode_abilities: b_mode_abilities, parent_target_missing_reason: b_parent_target_missing_reason, @@ -3383,6 +3403,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( && a_repeat_until == b_repeat_until && a_replacement_applied == b_replacement_applied && a_sub_link == b_sub_link + && a_sibling_condition == b_sibling_condition && a_modal == b_modal && a_mode_abilities == b_mode_abilities && a_parent_target_missing_reason == b_parent_target_missing_reason diff --git a/crates/engine/src/parser/oracle_effect/assembly.rs b/crates/engine/src/parser/oracle_effect/assembly.rs index b1b9bf5028..1286b62186 100644 --- a/crates/engine/src/parser/oracle_effect/assembly.rs +++ b/crates/engine/src/parser/oracle_effect/assembly.rs @@ -65,18 +65,18 @@ use super::sequence::{apply_clause_continuation, def_bears_retargetable_copy}; use super::{ append_to_deepest_sub_ability, apply_player_scope_rewrites, attach_alt_cost_to_prior_cast_from_zone, attach_mana_retention_to_prior_mana, - attach_repeat_process_keywords, attach_same_is_true_keywords, + attach_perpetual_keyword_grants, attach_repeat_process_keywords, attach_same_is_true_keywords, bind_anaphoric_damage_subject_keep_recipient, collapse_ephemeral_color_choice_mana, contains_explicit_tracked_set_pronoun, contains_implicit_tracked_set_pronoun, def_is_damage_dealer, def_is_dig_look, def_is_dig_or_mill, def_is_generic_effect_head, - def_is_keyword_counter_placement, demote_unbindable_batch_aggregate, draw_object_count_filter, - fold_cast_copy_of_card_defs, has_explicit_player_target, inject_chosen_color_choice_grant, - mark_uses_tracked_set, parse_spell_graveyard_replacement_rider, - publishes_aggregate_set_from_resolution, publishes_tracked_set_from_resolution, - rebind_tracked_aggregate_to_chain_set, retarget_counter_additional_cost_to_target, - rewrite_grant_parent_to_filter, rewrite_parent_targets_to_tracked_set, rewrite_rounding_mode, - rewrite_that_type_mana_instead, stamp_delayed_returns, try_fold_token_repeat_into_count, - wire_optional_cast_decline_fallback, + def_is_keyword_counter_placement, def_is_perpetual_keyword_grant, + demote_unbindable_batch_aggregate, draw_object_count_filter, fold_cast_copy_of_card_defs, + has_explicit_player_target, inject_chosen_color_choice_grant, mark_uses_tracked_set, + parse_spell_graveyard_replacement_rider, publishes_aggregate_set_from_resolution, + publishes_tracked_set_from_resolution, rebind_tracked_aggregate_to_chain_set, + retarget_counter_additional_cost_to_target, rewrite_grant_parent_to_filter, + rewrite_parent_targets_to_tracked_set, rewrite_rounding_mode, rewrite_that_type_mana_instead, + stamp_delayed_returns, try_fold_token_repeat_into_count, wire_optional_cast_decline_fallback, }; /// CR 601.2c: True when the assembled head chose one or more players at @@ -696,6 +696,15 @@ pub(super) enum AntecedentRole { /// the sibling template a "Repeat this process for " continuation /// clones (Kathril, Aspect Warper). KeywordCounterPlacement, + /// CR 702.1c + CR 608.2c: A perpetual keyword grant + /// (`ApplyPerpetual { GrantKeywords }`) — the sibling + /// template a "The same is true for " continuation clones when the + /// antecedent is a PERPETUAL grant rather than Odric's static `GenericEffect` + /// grant (Mutable Pupa). Membership is the EFFECT VARIANT ALONE, mirroring + /// `def_is_perpetual_keyword_grant`; the gating condition is the mutator's + /// business, not the role's filter. "Perpetually" is a digital-only extension + /// outside the Comprehensive Rules. + PerpetualKeywordGrantHead, /// A `DealDamage` — the antecedent an "excess damage" rider redirects from /// (CR 120.4a). The rider need not be adjacent to the damage clause, which is /// why this is a role and not `LastEmitted`. @@ -767,6 +776,11 @@ fn live_role_predicate(role: AntecedentRole) -> Option match role { AntecedentRole::GenericEffectHead => Some(def_is_generic_effect_head), AntecedentRole::KeywordCounterPlacement => Some(def_is_keyword_counter_placement), + // LIVE — mirrors `KeywordCounterPlacement`. The mutator + // (`attach_perpetual_keyword_grants`) appends siblings (length-changing), + // but staying live keeps it consistent with its sibling role and immune to + // any future in-place effect rewrite. + AntecedentRole::PerpetualKeywordGrantHead => Some(def_is_perpetual_keyword_grant), // LIVE, not cached. The scan this role replaces (`sequence.rs`, the // `DigFromAmong` fallthrough) re-derived its antecedent from `defs` on every // call, so it saw the CURRENT effect of every def. A cached registry is @@ -1109,7 +1123,8 @@ impl AssemblyEnv { | AntecedentRole::DigOrMill | AntecedentRole::DigLook | AntecedentRole::DamageDealer - | AntecedentRole::CopySpellBearer => Vec::new(), + | AntecedentRole::CopySpellBearer + | AntecedentRole::PerpetualKeywordGrantHead => Vec::new(), }, }; members @@ -1372,6 +1387,19 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition { attach_repeat_process_keywords(&mut defs, bound_index, keywords); } } + ReplicateKind::PerpetualKeywordGrant => { + let bound = env.resolve( + &defs, + AntecedentSelector::LastWithRole( + AntecedentRole::PerpetualKeywordGrantHead, + ), + None, + OnMiss::Ignore, + ); + if let Some(bound_index) = bound { + attach_perpetual_keyword_grants(&mut defs, bound_index, keywords); + } + } } env.observe(&defs, Some(clause_ir.id), NodeRole::HandlerProduct); true diff --git a/crates/engine/src/parser/oracle_effect/conditions.rs b/crates/engine/src/parser/oracle_effect/conditions.rs index 2b1abef02f..5311435dbd 100644 --- a/crates/engine/src/parser/oracle_effect/conditions.rs +++ b/crates/engine/src/parser/oracle_effect/conditions.rs @@ -3121,6 +3121,32 @@ pub(super) fn strip_suffix_conditional( if let Some(cond) = parse_source_pt_comparison_condition_text(condition_text) { return (Some(cond), text[..if_pos].trim().to_string()); } + // CR 608.2c: "that creature has " / "that permanent has " + // are in NON_REHOMEABLE_CONDITION_PREFIXES, so — like the "it has " colored- + // mana and source-P/T gates above — this TRAILING zone-change object gate must + // run BEFORE the rehomeable bail or it would never reach the condition parser. + // It binds the TRIGGER's event-bound entering object + // (`AbilityCondition::ZoneChangeObjectMatchesFilter`, evaluated against + // `state.current_trigger_event`), which is DISJOINT from the leading-only + // `strip_target_keyword_instead` path (`AbilityCondition::TargetHasKeywordInstead`, + // evaluated against `ability.targets` — a spell/ability TARGET): a genuinely + // different anaphor source (event object vs. chosen target), not a duplicate + // of the same concept. Mutable Pupa's "…if that creature has " riders. + // + // This is gated on trigger context, mirroring `strip_counter_conditional`'s + // identical demonstrative-subject handling ("that creature has … counter" is + // offered `if !in_trigger` there). `ZoneChangeObjectMatchesFilter` reads + // `state.current_trigger_event`, which is only meaningful inside a trigger's + // resolution; outside a trigger the demonstrative "that creature" is the + // spell's target, NOT an entering object, so this branch must decline and + // leave the non-trigger form to whatever else handles it (nothing currently + // emits `ZoneChangeObjectMatchesFilter` for a non-trigger keyword predicate) + // rather than misfire an event-bound gate against a spell target. + if ctx.in_trigger { + if let Some(cond) = parse_zone_change_object_has_keyword_condition(condition_text) { + return (Some(cond), text[..if_pos].trim().to_string()); + } + } if !condition_text_is_rehomeable(condition_text) { return (None, text.to_string()); } @@ -6636,13 +6662,91 @@ fn parse_entered_or_cast_from_zone_ability_condition(lower: &str) -> Option "` prefix +/// in a zone-change object gate. Typed (not a bool) per the typed-enum mandate, +/// so the caller routes each form to its own `TargetFilter` construction. The +/// copula form (`is/isn't [a/an] `) carries a type phrase; the keyword +/// form (`has/doesn't have `) carries a keyword name. +enum ZoneChangeObjectPredicate<'a> { + /// Remaining text is a type phrase (parsed via `parse_type_phrase`). + Type(&'a str), + /// Remaining text is a keyword name (parsed via `Keyword::from_str`). + Keyword(&'a str), +} + +/// Build the `TargetFilter` for a parsed zone-change object predicate. Copula → +/// type filter (rejecting `Any`/leftover, as before); keyword → a single +/// `FilterProp::WithKeyword` typed filter, mirroring the "it has [keyword]" arm. +fn zone_change_object_predicate_filter( + predicate: ZoneChangeObjectPredicate<'_>, +) -> Option { + match predicate { + ZoneChangeObjectPredicate::Type(type_text) => { + let (filter, leftover) = parse_type_phrase(type_text); + if matches!(filter, TargetFilter::Any) || !leftover.trim().is_empty() { + return None; + } + Some(filter) + } + ZoneChangeObjectPredicate::Keyword(keyword_text) => { + let keyword: Keyword = keyword_text + .trim() + .parse() + .unwrap_or(Keyword::Unknown(String::new())); + if matches!(keyword, Keyword::Unknown(_)) { + return None; + } + Some(TargetFilter::Typed(TypedFilter { + properties: vec![FilterProp::WithKeyword { value: keyword }], + ..Default::default() + })) + } + } +} + fn parse_zone_change_object_matches_filter_condition(lower: &str) -> Option { - let (type_text, negated) = parse_zone_change_object_type_text(lower).ok()?.1; - let (filter, leftover) = parse_type_phrase(type_text); - if matches!(filter, TargetFilter::Any) || !leftover.trim().is_empty() { + let (predicate, negated) = parse_zone_change_object_type_text(lower).ok()?.1; + // Copula-form only. The keyword form (`has/doesn't have `) is + // reachable through this ungated leading-conditional route + // (`strip_leading_general_conditional` → `try_nom_condition_as_ability_condition`), + // which runs BEFORE the dedicated `strip_target_keyword_instead` stripper. + // Accepting `Keyword` here would hijack CR 608.2c "If that creature has + // , [effect] instead" cards (Porcelain Zealot, Cut Propulsion, + // Burn the Impure, Compleat Devotion, Hexgold Slash) into a + // `ZoneChangeObjectMatchesFilter` that only reads `current_trigger_event` + // (always `None` off-trigger), permanently killing their "instead" branch. + // The keyword form must flow ONLY through the `ctx.in_trigger`-gated + // `parse_zone_change_object_has_keyword_condition` (Mutable Pupa's rider). + if matches!(predicate, ZoneChangeObjectPredicate::Keyword(_)) { return None; } + let filter = zone_change_object_predicate_filter(predicate)?; + + Some(maybe_negate( + AbilityCondition::ZoneChangeObjectMatchesFilter { + origin: None, + destination: Zone::Battlefield, + filter, + }, + negated, + )) +} +/// CR 608.2c: the KEYWORD-form-only slice of the trailing zone-change object gate +/// — "that creature has " / "that permanent has " (Mutable +/// Pupa's per-keyword riders). Split out from +/// `parse_zone_change_object_matches_filter_condition` so `strip_suffix_conditional` +/// can early-except ONLY this form (its `"that has "` prefixes live in +/// `NON_REHOMEABLE_CONDITION_PREFIXES`), while the copula form keeps flowing +/// through its existing downstream `try_nom_condition_as_ability_condition` route. +pub(super) fn parse_zone_change_object_has_keyword_condition( + lower: &str, +) -> Option { + let (predicate, negated) = parse_zone_change_object_type_text(lower).ok()?.1; + if !matches!(predicate, ZoneChangeObjectPredicate::Keyword(_)) { + return None; + } + let filter = zone_change_object_predicate_filter(predicate)?; Some(maybe_negate( AbilityCondition::ZoneChangeObjectMatchesFilter { origin: None, @@ -6693,9 +6797,20 @@ fn parse_outcome_this_way_condition(lower: &str) -> Option { )) } +/// Predicate-head discriminant for `parse_zone_change_object_type_text`: whether +/// the matched head was the copula (type phrase follows) or the "has" form +/// (keyword follows), plus the negation flag. Local selector so the remainder +/// `&str` (only known after the `alt` matches) maps into the typed +/// `ZoneChangeObjectPredicate` payload. +#[derive(Clone, Copy)] +enum PredicateHead { + Type, + Keyword, +} + fn parse_zone_change_object_type_text( input: &str, -) -> nom::IResult<&str, (&str, bool), OracleError<'_>> { +) -> nom::IResult<&str, (ZoneChangeObjectPredicate<'_>, bool), OracleError<'_>> { let (input, _) = tag("that ").parse(input)?; let (input, _) = alt(( tag("permanent"), @@ -6709,9 +6824,13 @@ fn parse_zone_change_object_type_text( tag("card"), )) .parse(input)?; - let (input, negated) = alt(( + // Two predicate forms share the `"that "` prefix: the copula + // (`is/isn't [a/an] `) and the keyword form (`has / doesn't have + // `). Composed as one `alt()` over the predicate heads; each arm + // yields `(negated, head)` and the remainder becomes the head's payload. + let (input, (negated, head)) = alt(( value( - true, + (true, PredicateHead::Type), alt(( tag(" is not an "), tag(" is not a "), @@ -6721,10 +6840,22 @@ fn parse_zone_change_object_type_text( tag(" isn't "), )), ), - value(false, alt((tag(" is an "), tag(" is a "), tag(" is ")))), + value( + (false, PredicateHead::Type), + alt((tag(" is an "), tag(" is a "), tag(" is "))), + ), + value( + (true, PredicateHead::Keyword), + alt((tag(" doesn't have "), tag(" does not have "))), + ), + value((false, PredicateHead::Keyword), tag(" has ")), )) .parse(input)?; - Ok(("", (input, negated))) + let predicate = match head { + PredicateHead::Type => ZoneChangeObjectPredicate::Type(input), + PredicateHead::Keyword => ZoneChangeObjectPredicate::Keyword(input), + }; + Ok(("", (predicate, negated))) } fn parse_target_supertype_condition_text(lower: &str) -> Option { diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index ce956a53b7..1efea91ae8 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -1635,9 +1635,22 @@ pub(super) fn target_choice_timing_for_clause(clause_ir: &ClauseIr) -> TargetCho .fragment() .unwrap_or_default() .to_ascii_lowercase(); - if !nom_primitives::scan_contains(&lower, "target ") - && target.contains_source_attachment_host() - { + // CR 115.10a: an object is a target only if the text uses the literal + // word "target"; CR 608.2d: an untargeted choice is made "while + // applying the effect" (at resolution), not at announcement. Was + // previously scoped to `contains_source_attachment_host()` alone + // (Equipped/Enchanted-host counters, e.g. "put a loyalty counter on + // the equipped creature" — deterministic, no player choice). Widened + // to every untargeted `PutCounter` recipient that isn't already a + // deterministic `is_context_ref()` shape (SelfRef/ParentTarget/None/…, + // which resolve automatically regardless of timing) — this is the + // same generalization `MultiplyCounter` below already applies. Covers + // "put a keyword counter on any creature you control" (Kathril, + // Aspect Warper, issue #6321/#6533): each independent instruction in a + // replicated keyword-counter chain must offer its own untargeted + // choice at ITS OWN resolution (CR 608.2d), not inherit one shared + // choice made once when the whole ability went on the stack. + if !nom_primitives::scan_contains(&lower, "target ") && !target.is_context_ref() { return TargetChoiceTiming::Resolution; } } diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index a5523736d1..bd99b7ecd4 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -103,11 +103,11 @@ use crate::types::ability::{ DelayedTriggerLifetime, DoubleTarget, Duration, Effect, EffectOutcomeSignal, EffectScope, FilterProp, GameRestriction, GuessSubject, IntensityScope, IterationKindBinding, KeeperConstraint, LibraryPosition, ManaProduction, ManaSpendPermission, MultiTargetSpec, - NumberDistinctness, ObjectProperty, ObjectScope, OriginConstraint, PlayPermissionInvalidation, - PlayerFilter, PlayerRelation, PlayerScope, PreventionAmount, PreventionScope, - ProhibitedActivity, PtValue, QuantityExpr, QuantityRef, ReplacementCondition, + NumberDistinctness, ObjectProperty, ObjectScope, OriginConstraint, PerpetualModification, + PlayPermissionInvalidation, PlayerFilter, PlayerRelation, PlayerScope, PreventionAmount, + PreventionScope, ProhibitedActivity, PtValue, QuantityExpr, QuantityRef, ReplacementCondition, ReplacementDefinition, RestrictionExpiry, RestrictionPlayerScope, RevealUntilDisposition, - RoundingMode, SharedQuality, SharedQualityRelation, SkipScope, + RoundingMode, SharedQuality, SharedQualityRelation, SiblingCondition, SkipScope, SpellStackToGraveyardReplacement, StaticCondition, StaticDefinition, StepSkipTarget, SubAbilityLink, TapStateChange, TargetFilter, TargetSelectionMode, ThisWayCause, TrackedAnaphorSource, TriggerCondition, TriggerDefinition, TypeFilter, TypedFilter, @@ -8759,7 +8759,14 @@ fn parse_effect_clause_inner(text: &str, ctx: &mut ParseContext) -> ParsedEffect } // Digital-only Alchemy: "[~/that X] perpetually gains [keyword(s)]" — persistent - // keyword grant (Monoist Gravliner station trigger). + // keyword grant (Monoist Gravliner station trigger). Mutable Pupa's + // keyword-MIRROR antecedent ("… perpetually gains if that creature has + // ") also lands here: the trailing "if that creature has " gate is + // peeled UPSTREAM by `strip_suffix_conditional` (its trigger-gated + // `ZoneChangeObjectMatchesFilter` branch) in the effect-chain chunk loop — + // the ONLY production path that reaches this dispatch — so the text arriving + // here is already the short bare-keyword form and the peeled condition is + // reattached at the chunk level. if let Some(effect) = try_parse_perpetual_grant_keywords(tp) { return parsed_clause(effect); } @@ -22207,6 +22214,12 @@ fn attach_repeat_process_keywords( } // Each replicated counter placement is its own sequential instruction. new_def.sub_link = SubAbilityLink::SequentialSibling; + // CR 608.2c: each per-keyword sibling is an INDEPENDENT OR-branch gated on + // its own keyword, so it must resolve even when a preceding sibling's + // condition (K0's graveyard-keyword gate) was false. Without this, K1..Kn + // never place their counters once K0's gate fails (the same "list + // collapse" bug this marker fixes for Mutable Pupa's perpetual grants). + new_def.sibling_condition = SiblingCondition::ReplicatedOrBranch; new_def.sub_ability = None; defs.push(new_def); } @@ -22224,6 +22237,58 @@ pub(super) fn def_is_keyword_counter_placement(def: &AbilityDefinition) -> bool ) } +/// Membership mirror for `AntecedentRole::PerpetualKeywordGrantHead` — the shape +/// `attach_perpetual_keyword_grants` clones its sibling template from (Mutable +/// Pupa's "perpetually gains if that creature has " antecedent). +pub(super) fn def_is_perpetual_keyword_grant(def: &AbilityDefinition) -> bool { + matches!( + &*def.effect, + Effect::ApplyPerpetual { + modification: PerpetualModification::GrantKeywords { .. }, + .. + } + ) +} + +/// CR 702.1c + CR 608.2c: Apply a "The same is true for " continuation whose +/// antecedent is a PERPETUAL keyword grant (Mutable Pupa). The counters-class +/// `attach_repeat_process_keywords` analogue: walk `defs` back to the most +/// recent conditional perpetual keyword-grant (`ApplyPerpetual { GrantKeywords }` +/// gated by a zone-change keyword `condition`) and append one cloned sibling per +/// listed keyword — swapping both the granted keyword and the gating condition's +/// keyword. Each clone is an independent sequential sibling +/// (`SiblingCondition::ReplicatedOrBranch`) so the engine grants every keyword +/// the entering object actually has during the trigger's one resolution, rather +/// than collapsing to K0's gate. Digital-only Alchemy (no CR entry for +/// "perpetually"). +fn attach_perpetual_keyword_grants( + defs: &mut Vec, + template_index: usize, + keywords: &[Keyword], +) { + let template = defs[template_index].clone(); + for keyword in keywords { + let mut new_def = template.clone(); + if let Effect::ApplyPerpetual { + modification: PerpetualModification::GrantKeywords { keywords: kws }, + .. + } = &mut *new_def.effect + { + *kws = vec![keyword.clone()]; + } + if let Some(condition) = &mut new_def.condition { + rewrite_ability_condition_keyword(condition, keyword); + } + // Each replicated perpetual grant is its own sequential instruction. + new_def.sub_link = SubAbilityLink::SequentialSibling; + // CR 702.1c + CR 608.2c: independent OR-branch — resolves regardless of any other + // sibling's keyword gate (see `SiblingCondition::ReplicatedOrBranch`). + new_def.sibling_condition = SiblingCondition::ReplicatedOrBranch; + new_def.sub_ability = None; + defs.push(new_def); + } +} + /// Swap the gating keyword inside an `AbilityCondition` to `new_keyword`. Used /// by `attach_repeat_process_keywords` to rewrite each replicated counter /// clause's keyword gate. Covers every keyword-gate shape the antecedent @@ -22245,6 +22310,13 @@ fn rewrite_ability_condition_keyword(condition: &mut AbilityCondition, new_keywo | AbilityCondition::SourceLacksKeyword { keyword } => { *keyword = new_keyword.clone(); } + // CR 702.1c + CR 608.2c: Mutable Pupa's per-keyword gate — "if that creature has + // " — carries the keyword inside a `ZoneChangeObjectMatchesFilter` + // typed filter (`FilterProp::WithKeyword`), swapped via the shared + // `rewrite_filter_keyword` walker. + AbilityCondition::ZoneChangeObjectMatchesFilter { filter, .. } => { + rewrite_filter_keyword(filter, new_keyword); + } AbilityCondition::And { conditions } | AbilityCondition::Or { conditions } => { for inner in conditions { rewrite_ability_condition_keyword(inner, new_keyword); @@ -27482,15 +27554,40 @@ pub(crate) fn parse_effect_chain_ir( // keyword. Requires a prior clause to attach to. if !builder.is_empty() { if let Some(keywords) = try_parse_same_is_true_continuation(normalized_text) { + // CR 702.1c ("the same is true") + CR 608.2c (written order): + // select the replication template shape from the antecedent clause's + // parsed effect. A PERPETUAL keyword grant + // (Mutable Pupa, `ApplyPerpetual { GrantKeywords }`) replicates via + // `attach_perpetual_keyword_grants`; every other "same is true for" + // antecedent (Odric's `GenericEffect` static grant) keeps the + // default `StaticGrant`. Mirrors `def_is_perpetual_keyword_grant`, + // applied to the clause's `.effect` (both expose `Effect`). + let kind = if builder + .clauses() + .iter() + .rev() + .find(|clause| { + !matches!(clause.disposition, ClauseDisposition::Continue { .. }) + }) + .is_some_and(|clause| { + matches!( + &clause.parsed.effect, + Effect::ApplyPerpetual { + modification: PerpetualModification::GrantKeywords { .. }, + .. + } + ) + }) { + ReplicateKind::PerpetualKeywordGrant + } else { + ReplicateKind::StaticGrant + }; builder .clause( normalized_text, placeholder_parsed_clause("same_is_true_for_placeholder"), chunk.boundary_after, - ClauseDisposition::ReplicatePerKeyword { - keywords, - kind: ReplicateKind::StaticGrant, - }, + ClauseDisposition::ReplicatePerKeyword { keywords, kind }, ) .push(); continue; diff --git a/crates/engine/src/parser/oracle_ir/effect_chain.rs b/crates/engine/src/parser/oracle_ir/effect_chain.rs index e12a6868db..c9d2017045 100644 --- a/crates/engine/src/parser/oracle_ir/effect_chain.rs +++ b/crates/engine/src/parser/oracle_ir/effect_chain.rs @@ -406,6 +406,13 @@ pub(crate) enum ReplicateKind { /// CR 608.2c: "Repeat this process for ." — replicate the antecedent /// conditional keyword-COUNTER clause per keyword (Kathril, Aspect Warper). CounterPlacement, + /// CR 702.1c + CR 608.2c: "The same is true for ." — replicate the antecedent + /// conditional PERPETUAL keyword-GRANT clause per keyword (Mutable Pupa). Each + /// replicated grant is gated on the entering object having THAT keyword, an + /// independent OR-branch (unlike `StaticGrant`, whose Odric antecedent carries + /// no per-keyword condition). Digital-only Alchemy (no CR entry for + /// "perpetually"). + PerpetualKeywordGrant, } /// Per-clause IR: captures everything about a single parsed chunk before chain assembly. diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kathril_aspect_warper_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kathril_aspect_warper_ir.snap index 664acb2dcc..a10829e3c9 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kathril_aspect_warper_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kathril_aspect_warper_ir.snap @@ -34,7 +34,12 @@ expression: "&ir" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -48,7 +53,12 @@ expression: "&ir" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -62,7 +72,12 @@ expression: "&ir" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -76,7 +91,12 @@ expression: "&ir" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -90,7 +110,12 @@ expression: "&ir" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -104,7 +129,12 @@ expression: "&ir" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -118,7 +148,12 @@ expression: "&ir" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -132,7 +167,12 @@ expression: "&ir" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -146,7 +186,12 @@ expression: "&ir" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -160,7 +205,12 @@ expression: "&ir" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -174,7 +224,12 @@ expression: "&ir" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -240,8 +295,10 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -279,8 +336,10 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -318,8 +377,10 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -357,8 +418,10 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -396,8 +459,10 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -435,8 +500,10 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -474,8 +541,10 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -513,8 +582,10 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -552,8 +623,10 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -591,8 +664,10 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -630,6 +705,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false }, "valid_card": { @@ -655,13 +731,5 @@ expression: "&ir" } ], "source_text": "When Kathril enters, put a flying counter on any creature you control if a creature card in your graveyard has flying. Repeat this process for first strike, double strike, deathtouch, hexproof, indestructible, lifelink, menace, reach, trample, and vigilance. Then put a +1/+1 counter on Kathril for each counter put on a creature this way.", - "card_name": "Kathril, Aspect Warper", - "diagnostics": [ - { - "type": "TargetFallback", - "context": "parse_target could not classify", - "text": "any creature you control", - "line_index": 0 - } - ] + "card_name": "Kathril, Aspect Warper" } diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kathril_aspect_warper_lowered.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kathril_aspect_warper_lowered.snap index 4adeb1ffca..c7ac016c31 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kathril_aspect_warper_lowered.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kathril_aspect_warper_lowered.snap @@ -17,7 +17,12 @@ expression: "&lowered" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -31,7 +36,12 @@ expression: "&lowered" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -45,7 +55,12 @@ expression: "&lowered" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -59,7 +74,12 @@ expression: "&lowered" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -73,7 +93,12 @@ expression: "&lowered" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -87,7 +112,12 @@ expression: "&lowered" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -101,7 +131,12 @@ expression: "&lowered" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -115,7 +150,12 @@ expression: "&lowered" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -129,7 +169,12 @@ expression: "&lowered" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -143,7 +188,12 @@ expression: "&lowered" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -157,7 +207,12 @@ expression: "&lowered" "value": 1 }, "target": { - "type": "Any" + "type": "Typed", + "type_filters": [ + "Creature" + ], + "controller": "You", + "properties": [] } }, "cost": null, @@ -223,8 +278,10 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -262,8 +319,10 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -301,8 +360,10 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -340,8 +401,10 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -379,8 +442,10 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -418,8 +483,10 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -457,8 +524,10 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -496,8 +565,10 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -535,8 +606,10 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -574,8 +647,10 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -613,6 +688,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false }, "valid_card": { @@ -637,13 +713,5 @@ expression: "&lowered" ], "statics": [], "replacements": [], - "extractedKeywords": [], - "parseWarnings": [ - { - "type": "TargetFallback", - "context": "parse_target could not classify", - "text": "any creature you control", - "line_index": 0 - } - ] + "extractedKeywords": [] } diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index 68d08aa176..2d404c106b 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -2075,21 +2075,47 @@ pub fn parse_type_phrase_with_ctx<'a>( let offset = lower.len() - lower_trimmed.len(); pos += offset; - // Strip leading article ("a "/"an ") when followed by a recognized type word - // or the "commander" class. Guard: "an opponent" → "opponent" fails type word - // check → no stripping. CR 903.3: "commander" is recognized by the commander - // atom below (it pushes `IsCommander`), not by `starts_with_type_phrase_lead`, - // so the article guard must also accept it — otherwise "a commander you own" - // (Hellkite Courser, #5256) keeps its article and never reaches the atom, - // collapsing to a match-anything filter. "commander you own" / "target - // commander" already work; this makes the indefinite article compose too. - if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("a ").parse(&lower[pos..]) { - if starts_with_type_phrase_lead(rest) || starts_with_commander_word(rest) { - pos += "a ".len(); - } - } else if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("an ").parse(&lower[pos..]) { - if starts_with_type_phrase_lead(rest) || starts_with_commander_word(rest) { - pos += "an ".len(); + // Strip a leading indefinite quantifier ("a "/"an "/"any ") when followed by + // a recognized type word or the "commander" class. Guard: "an opponent" → + // "opponent" fails the type-word check → no stripping. CR 903.3: "commander" + // is recognized by the commander atom below (it pushes `IsCommander`), not by + // `starts_with_type_phrase_lead`, so the guard must also accept it — + // otherwise "a commander you own" (Hellkite Courser, #5256) keeps its article + // and never reaches the atom, collapsing to a match-anything filter. + // "commander you own" / "target commander" already work; this makes the + // indefinite article/quantifier compose too. + // + // CR 115.10a (+ CR 115.1d for the triggered-ability case): an object/player + // is a target ONLY if the text uses the literal word "target" — "any + // creature you control" (no "target") is an untargeted controller choice, + // distinct from "any target" (a fixed keyword phrase matched earlier in + // `parse_target_with_syntax`, which requires "target" as the very next word + // and so never reaches here). "any " strips exactly like "a "/"an " above: a + // plain quantifier over the following type word, adding no extra + // `FilterProp` (unlike "other"/"another" below). Without this the type word + // is never reached and the phrase falls through every arm to the + // `TargetFilter::Any` fallback at the bottom of this function's caller + // (Kathril, Aspect Warper's "put a flying counter on any creature you + // control", issue #6321). + // + // Composed through "other"/"another" — mirroring the "all"/"each"/"every" + // block's own `after_other` composition just below — so "any other + // creature you control" (gain-control / sacrifice effects) also reaches + // the type word instead of leaking "other" into the subtype string. Only + // the quantifier is consumed here; the "other"/"another" handler below + // still runs on the remainder and adds `FilterProp::Another`. + if let Ok((rest, matched)) = + alt((tag::<_, _, OracleError<'_>>("a "), tag("an "), tag("any "))).parse(&lower[pos..]) + { + let after_other = alt((tag::<_, _, OracleError<'_>>("other "), tag("another "))) + .parse(rest) + .map(|(r, _)| r) + .ok(); + if starts_with_type_phrase_lead(rest) + || starts_with_commander_word(rest) + || after_other.is_some_and(starts_with_type_phrase_lead) + { + pos += matched.len(); } } @@ -2507,6 +2533,72 @@ pub fn parse_type_phrase_with_ctx<'a>( _ => unreachable!(), }; (Some(tf), Some(sub_name)) + } else if let TypeFilter::Subtype(second) = tf { + // CR 205.3b + CR 205.3m: on a PRINTED type line, subtypes + // of every card type except creature (and plane) are + // always single words — each dash-separated word is its + // own subtype. Creature subtypes are the one category the + // rules let run one OR two words (the sole two-word + // creature type is "Time Lord"; every other type in the + // 205.3m list — "Elder"/"Dragon"/"Elf"/"Warrior"/"Human"/ + // "Wizard" included — is one word). So when ORACLE TEXT + // names two consecutive creature-subtype words, that is + // ambiguous ONLY for creatures — the same word-boundary + // question ("one two-word type, or two one-word types + // stacked?") never arises for other categories, where + // CR 205.3b already guarantees each word is separate. + // This generic phrase-chaining rule exists to resolve + // exactly that creature-only ambiguity, so it is scoped + // to fire ONLY when NEITHER matched word is a registered + // NONCREATURE subtype (`fixed_noncreature_subtypes` — + // land/artifact/enchantment/spell/battle/planeswalker). + // "Urza's" (a real land type per CR 205.3i, LAND_SUBTYPES + // in card_type.rs) is noncreature — land subtypes CAN + // co-occur on one permanent (Urza's Mine genuinely has + // BOTH the "Urza's" and "Mine" land subtypes), but the + // dedicated Urza-lands condition parser already owns that + // Oracle-text pattern and deliberately extracts only the + // discriminating second word ("Mine"/"Power-Plant"/ + // "Tower" — "Urza's" is common to all three lands in the + // cycle, so checking for it adds no discriminating + // power). Chaining here instead fully consumed "an urza's + // mine" into one filter with an empty remainder, which + // changed which downstream condition-builder claimed the + // clause and regressed that specialized parser (issue + // #6321 / PR #6533 review — + // urzas_lands_share_delta_shape / + // legacy_misparses_are_now_honest_gaps). Staying out of + // every noncreature category's way, not just this one + // land cycle, is why the check is by vocabulary + // membership rather than an Urza's-specific special case. + let first_name = match &card_type { + Some(TypeFilter::Subtype(s)) => s.as_str(), + _ => unreachable!(), + }; + let is_noncreature_subtype = |name: &str| { + crate::types::card_type::fixed_noncreature_subtypes() + .any(|s| s.eq_ignore_ascii_case(name)) + }; + if is_noncreature_subtype(first_name) || is_noncreature_subtype(&second) { + // Decline — this generic creature-stack rule doesn't + // own noncreature subtype pairs. Whichever specialized + // handler owns this category still gets the untouched + // trailing text. + (card_type, subtype) + } else { + // Both words are creature-only: chain the second as + // an additional AND-combined type filter instead of + // silently dropping it. Reuses the existing `subtype` + // slot (already flows into `base_type_filters` + // below), so `card_type` keeps the first subtype and + // this fills the second (Fate Reforged chapter II — + // "a copy of any Elder Dragon…", issue #6321 / PR + // #6533: without this, "any " strips down to + // `Subtype("Elder")` alone, dropping "Dragon"). + let ct_len = rest_after.len() - ct_rest.len(); + pos += ws + ct_len; + (card_type, Some(second)) + } } else { (card_type, subtype) } @@ -15293,6 +15385,196 @@ mod tests { } } + /// CR 115.10a + CR 608.2d: "any other you control" — the indefinite + /// quantifier "any" must compose through "other"/"another" the same way + /// "all"/"each"/"every" already do above, or the type word is never + /// reached and the phrase collapses to the degenerate `TargetFilter::Any` + /// fallback (gain-control / sacrifice effects — "gain control of any + /// other creature", "sacrifice any other creature you control"). + #[test] + fn parse_type_phrase_any_other_creature_you_control() { + let (filter, rest) = parse_type_phrase("any other creature you control"); + assert!(rest.trim().is_empty(), "remainder: '{rest}'"); + let TargetFilter::Typed(tf) = &filter else { + panic!("Expected Typed filter, got {filter:?}"); + }; + assert!( + tf.type_filters.contains(&TypeFilter::Creature), + "expected Creature, got {:?}", + tf.type_filters + ); + assert!( + !tf.type_filters + .iter() + .any(|t| matches!(t, TypeFilter::Subtype(s) if s.contains(' '))), + "quantifier/other leaked into subtype: {:?}", + tf.type_filters + ); + // "other" excludes the source → Another IS present. + assert!( + tf.properties.contains(&FilterProp::Another), + "expected Another: {:?}", + tf.properties + ); + assert_eq!(tf.controller, Some(ControllerRef::You)); + } + + /// CR 205.3b + CR 205.3m: creature subtypes are the one category the + /// rules let run one OR two words on a type line (the sole two-word + /// creature type is "Time Lord"; every other listed creature type — + /// "Elder"/"Dragon"/"Elf"/"Warrior"/"Human"/"Wizard" included — is one + /// word), so two of them printed back to back are two SEPARATE stacked + /// subtypes, not a compound word (`oracle-subtypes.json` lists "Elder" + /// and "Dragon" as separate entries). Not a `[Subtype] [CoreType]` + /// promotion either (that existing arm only fires when the SECOND word is + /// a concrete core type like "creature"). Before this fix the second + /// subtype word was silently dropped (Fate Reforged chapter II — "a copy + /// of any Elder Dragon from the Legends expansion" — collapsed to bare + /// `Subtype("Elder")`, an over-broad filter matching any "Elder"-subtype + /// creature, not just Elder Dragons; issue #6321 / PR #6533 review). + #[test] + fn parse_type_phrase_two_word_subtype_chain() { + for (text, first, second) in [ + ("Elder Dragon", "Elder", "Dragon"), + ("Elf Warrior", "Elf", "Warrior"), + ("Human Wizard", "Human", "Wizard"), + ] { + let (filter, rest) = parse_type_phrase(text); + assert!(rest.trim().is_empty(), "remainder for '{text}': '{rest}'"); + let TargetFilter::Typed(tf) = &filter else { + panic!("Expected Typed filter for '{text}', got {filter:?}"); + }; + assert!( + tf.type_filters + .contains(&TypeFilter::Subtype(first.to_string())), + "expected Subtype(\"{first}\") for '{text}', got {:?}", + tf.type_filters + ); + assert!( + tf.type_filters + .contains(&TypeFilter::Subtype(second.to_string())), + "expected Subtype(\"{second}\") for '{text}' — the second subtype word must \ + not be silently dropped, got {:?}", + tf.type_filters + ); + } + } + + /// CR 205.3b + CR 205.3i: "Urza's" is a real land type (LAND_SUBTYPES, + /// `card_type.rs`), and land subtypes CAN co-occur on one permanent — + /// Urza's Mine genuinely has both the "Urza's" and "Mine" land subtypes. + /// But the two-consecutive-subtype-word chain above is scoped to resolve + /// a CREATURE-only word-boundary ambiguity (CR 205.3b/205.3m) and must + /// stay out of every noncreature category's way — including this one. + /// Chaining here would fully consume "urza's mine" into one + /// `Typed{Subtype("Urza's"), Subtype("Mine")}` filter with an empty + /// remainder, which changes which downstream condition-builder claims the + /// clause and regresses the dedicated Urza-lands + /// `ControllerControlsMatching` parser (`urzas_lands_share_delta_shape` / + /// `legacy_misparses_are_now_honest_gaps` in oracle_tests.rs / + /// oracle_condition.rs — issue #6321 / PR #6533 review), which + /// deliberately extracts only the discriminating second word ("Mine" — + /// "Urza's" is common to all three cycle members and adds no + /// discriminating power). "mine" must stay unconsumed in the remainder so + /// that specialized handler still sees it. + #[test] + fn parse_type_phrase_urzas_possessive_prefix_does_not_chain() { + let (filter, rest) = parse_type_phrase("urza's mine"); + assert_eq!( + rest.trim(), + "mine", + "\"mine\" must stay unconsumed, not chained into the type filter" + ); + let TargetFilter::Typed(tf) = &filter else { + panic!("Expected Typed filter, got {filter:?}"); + }; + assert_eq!( + tf.type_filters, + vec![TypeFilter::Subtype("Urza's".to_string())], + "only the possessive fragment may be consumed here, got {:?}", + tf.type_filters + ); + } + + /// CR 201.2 + CR 115.10a: Naming Screen — "Each creature you control that + /// doesn't share a name with any other creature you control gets +1/+1." + /// `parse_shared_quality_reference` (the reference-population parser for + /// "that doesn't share a name with X") explicitly REJECTS a `TargetFilter + /// ::Any` result from `parse_target` as a parse failure (it cannot build a + /// meaningful name comparison against "anything"). Before the "any"/ + /// "other" composition fix, "any other creature you control" collapsed to + /// `Any`, so this whole relative clause failed to parse and the static + /// ability fell through to an unstructured fallback — after the fix it + /// builds a real `Typed{Creature, Another, You}` reference and the clause + /// parses (issue #6321 / PR #6533 review). + #[test] + fn parse_shared_quality_clause_naming_screen_reference() { + let ctx = ParseContext::default(); + let (rest, prop) = + parse_shared_quality_clause("that doesn't share a name with any other creature you control", &ctx) + .expect("the reference population must parse now that \"any other ...\" is a real Typed filter, not Any"); + assert!(rest.trim().is_empty(), "remainder: '{rest}'"); + let FilterProp::SharesQuality { + quality, + reference, + relation, + } = prop + else { + panic!("expected SharesQuality, got {prop:?}"); + }; + assert_eq!(quality, SharedQuality::Name); + assert_eq!(relation, SharedQualityRelation::DoesNotShare); + let reference = reference.expect("reference population must be present"); + let TargetFilter::Typed(tf) = *reference else { + panic!("expected Typed reference filter, got {reference:?}"); + }; + assert!( + tf.type_filters.contains(&TypeFilter::Creature), + "expected Creature in the reference filter, got {:?}", + tf.type_filters + ); + assert!( + tf.properties.contains(&FilterProp::Another), + "\"other\" must exclude the compared creature itself, got {:?}", + tf.properties + ); + assert_eq!(tf.controller, Some(ControllerRef::You)); + } + + /// CR 707.2 + CR 115.10a: Duplication Device — "target creature becomes a + /// copy of any creature on the battlefield". "any creature on the + /// battlefield" carries no controller restriction (any player's + /// creatures) — before the "any" widening this collapsed to `Any` + /// (matching literally anything, including non-creatures/players); + /// afterward it correctly reaches the pre-existing, unmodified zone- + /// suffix machinery that already handles "creature on the battlefield" + /// for non-"any" phrasing (issue #6321 / PR #6533 review). + #[test] + fn parse_type_phrase_any_creature_on_the_battlefield() { + let (filter, rest) = parse_type_phrase("any creature on the battlefield"); + assert!(rest.trim().is_empty(), "remainder: '{rest}'"); + let TargetFilter::Typed(tf) = &filter else { + panic!("Expected Typed filter, got {filter:?}"); + }; + assert!( + tf.type_filters.contains(&TypeFilter::Creature), + "expected Creature, got {:?}", + tf.type_filters + ); + assert!( + tf.properties + .iter() + .any(|p| matches!(p, FilterProp::InZone { zone } if *zone == Zone::Battlefield)), + "expected an InZone(Battlefield) property, got {:?}", + tf.properties + ); + // "on the battlefield" (not "you control") — no controller restriction. + assert_eq!( + tf.controller, None, + "\"on the battlefield\" must not add a controller restriction" + ); + } + /// CR 700.9 + CR 109.4: "modified creatures you control other than ~" /// (Thundering Raiju). The "modified" adjective adds `FilterProp::Modified` /// and the trailing "other than ~" adds `FilterProp::Another` so the count diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 16d36c21ee..a2c78fd3bc 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -12,8 +12,9 @@ use crate::types::ability::{ CountScope, DamageChannel, DamageModification, DamageSource, DelayedTriggerCondition, DiscardSelfScope, Duration, Effect, EffectScope, FilterProp, ManaContribution, ManaProduction, ManaSpendPermission, ObjectScope, PerpetualModification, PlayerFilter, PlayerScope, PtStat, - PtValue, PtValueScope, QuantityExpr, QuantityRef, SeatDirection, SharedQuality, TapStateChange, - TargetFilter, TriggerCondition, TypeFilter, TypedFilter, ZoneRef, + PtValue, PtValueScope, QuantityExpr, QuantityRef, SeatDirection, SharedQuality, + SiblingCondition, SubAbilityLink, TapStateChange, TargetFilter, TriggerCondition, TypeFilter, + TypedFilter, ZoneRef, }; use crate::types::card_type::Supertype; use crate::types::counter::{CounterMatch, CounterType}; @@ -25174,3 +25175,225 @@ fn parse_sigil_of_sleep_bounce_targets_triggering_player_controlled_creature() { other => panic!("Sigil of Sleep effect must be Bounce, got {other:?}"), } } + +// ----------------------------------------------------------------------- +// Mutable Pupa — perpetual keyword-mirror ETB trigger (issue #6321). +// Digital-only Alchemy (no CR entry for "perpetually"); CR 702.1c + CR 608.2c govern the +// per-branch resolution order the `SiblingCondition::ReplicatedOrBranch` marker +// restores. Oracle text verified verbatim against data/card-data.json. +// ----------------------------------------------------------------------- + +// The antecedent on its own — the SAME production entry point the real pipeline +// uses (`parse_trigger_line`), but with a SINGLE-sentence body (no "The same is +// true for …" tail), so no keyword replication runs. This isolates the +// antecedent build: the trigger body reaches `parse_effect_chain_ir`'s chunk +// loop with `in_trigger == true`, where `strip_suffix_conditional`'s +// trigger-gated `ZoneChangeObjectMatchesFilter` branch peels the trailing "if +// that creature has flying" gate BEFORE `parse_effect_clause` sees the chunk; +// the short bare-keyword form then lands `try_parse_perpetual_grant_keywords` +// (`ApplyPerpetual { GrantKeywords[Flying] }`) and the peeled gate is reattached +// at the chunk level. The root node must carry BOTH — proving the antecedent +// builds correctly through the real suffix-strip path without depending on the +// replication machinery. +#[test] +fn mutable_pupa_antecedent_clause_grants_and_gates_the_same_keyword() { + let def = parse_trigger_line( + "Whenever another creature you control enters, this creature perpetually gains flying if that creature has flying.", + "Mutable Pupa", + ); + let root = def + .execute + .as_deref() + .expect("trigger has an ability chain"); + // Single-sentence antecedent: exactly one node, no replicated siblings. + assert!( + root.sub_ability.is_none(), + "single-sentence antecedent must not build a sibling chain", + ); + match &*root.effect { + Effect::ApplyPerpetual { + modification: PerpetualModification::GrantKeywords { keywords }, + .. + } => assert_eq!( + keywords, + &vec![Keyword::Flying], + "antecedent must grant exactly Flying" + ), + other => panic!("expected ApplyPerpetual GrantKeywords, got {other:?}"), + } + assert_eq!( + root.condition, + Some(AbilityCondition::ZoneChangeObjectMatchesFilter { + origin: None, + destination: crate::types::zones::Zone::Battlefield, + filter: TargetFilter::Typed(TypedFilter { + properties: vec![FilterProp::WithKeyword { + value: Keyword::Flying + }], + ..Default::default() + }), + }), + "antecedent must be gated on the entering object having Flying", + ); +} + +// The whole two-sentence trigger builds EXACTLY 12 independent keyword-mirror +// nodes. Each node grants ONLY its own keyword and is gated on THAT SAME keyword +// (the positional correspondence is the "list collapse" regression guard: a +// bug that reused keyword[0] for every gate would fail the per-node condition +// assertion). Nodes 1..11 are `SequentialSibling` + `ReplicatedOrBranch`; the +// root is the unmarked `ContinuationStep` antecedent. +#[test] +fn mutable_pupa_full_trigger_builds_twelve_independent_keyword_mirrors() { + let def = parse_trigger_line( + "Whenever another creature you control enters, this creature perpetually gains flying if that creature has flying. The same is true for first strike, double strike, deathtouch, haste, hexproof, indestructible, lifelink, menace, reach, trample, and vigilance.", + "Mutable Pupa", + ); + let root = def + .execute + .as_deref() + .expect("trigger has an ability chain"); + let mut nodes: Vec<&AbilityDefinition> = Vec::new(); + let mut cur = Some(root); + while let Some(n) = cur { + nodes.push(n); + cur = n.sub_ability.as_deref(); + } + let expected = [ + Keyword::Flying, + Keyword::FirstStrike, + Keyword::DoubleStrike, + Keyword::Deathtouch, + Keyword::Haste, + Keyword::Hexproof, + Keyword::Indestructible, + Keyword::Lifelink, + Keyword::Menace, + Keyword::Reach, + Keyword::Trample, + Keyword::Vigilance, + ]; + assert_eq!( + nodes.len(), + expected.len(), + "expected exactly 12 keyword-mirror nodes, got {}", + nodes.len() + ); + for (i, (node, kw)) in nodes.iter().zip(expected.iter()).enumerate() { + match &*node.effect { + Effect::ApplyPerpetual { + modification: PerpetualModification::GrantKeywords { keywords }, + .. + } => assert_eq!( + keywords, + &vec![kw.clone()], + "node {i} must grant only {kw:?}" + ), + other => panic!("node {i}: expected ApplyPerpetual GrantKeywords, got {other:?}"), + } + assert_eq!( + node.condition, + Some(AbilityCondition::ZoneChangeObjectMatchesFilter { + origin: None, + destination: crate::types::zones::Zone::Battlefield, + filter: TargetFilter::Typed(TypedFilter { + properties: vec![FilterProp::WithKeyword { value: kw.clone() }], + ..Default::default() + }), + }), + "node {i} must be gated on {kw:?} (not keyword[0])", + ); + if i == 0 { + assert_eq!( + node.sub_link, + SubAbilityLink::ContinuationStep, + "root antecedent is a continuation step", + ); + assert_eq!( + node.sibling_condition, + SiblingCondition::Dependent, + "root antecedent keeps the default sibling condition", + ); + } else { + assert_eq!( + node.sub_link, + SubAbilityLink::SequentialSibling, + "node {i} must be a sequential sibling", + ); + assert_eq!( + node.sibling_condition, + SiblingCondition::ReplicatedOrBranch, + "node {i} must be an independent OR-branch", + ); + } + } +} + +// Non-regression: Odric's "the same is true for" antecedent is a static +// keyword grant (`GenericEffect`, replicated in-place into `static_abilities`), +// NOT a perpetual grant — the new shape-based `ReplicateKind` selection must +// keep routing it through `StaticGrant`, so no node becomes `ApplyPerpetual` and +// no `SequentialSibling` sibling chain is built. +#[test] +fn odric_same_is_true_stays_generic_effect_not_perpetual_chain() { + let def = parse_trigger_line( + "At the beginning of each combat, creatures you control gain first strike until end of turn if a creature you control has first strike. The same is true for flying, deathtouch, double strike, haste, hexproof, indestructible, lifelink, menace, reach, skulk, trample, and vigilance.", + "Odric, Lunarch Marshal", + ); + let root = def + .execute + .as_deref() + .expect("trigger has an ability chain"); + assert!( + matches!(&*root.effect, Effect::GenericEffect { .. }), + "Odric's antecedent must stay a GenericEffect keyword grant, got {:?}", + root.effect, + ); + let mut cur = Some(root); + while let Some(n) = cur { + assert!( + !matches!(&*n.effect, Effect::ApplyPerpetual { .. }), + "Odric must never route through the perpetual keyword-grant path", + ); + assert_eq!( + n.sibling_condition, + SiblingCondition::Dependent, + "Odric nodes must not be stamped ReplicatedOrBranch", + ); + cur = n.sub_ability.as_deref(); + } +} + +// Field-level non-regression: an ordinary DEPENDENT continuation (Thieving +// Skydiver's "If that artifact is an Equipment, attach it") must keep the +// default `SiblingCondition::Dependent` — the `ReplicatedOrBranch` marker is +// stamped ONLY by the two replication helpers, never by sentence-boundary +// sibling stamping. The `node_count >= 2` reach guard proves the multi-node +// continuation chain actually built (so the all-`Dependent` assertion is not +// vacuous on a single node). +#[test] +fn thieving_skydiver_dependent_continuation_is_never_replicated_or_branch() { + let def = parse_trigger_line( + "When this creature enters, if it was kicked, gain control of target artifact with mana value X or less. If that artifact is an Equipment, attach it to this creature.", + "Thieving Skydiver", + ); + let root = def + .execute + .as_deref() + .expect("trigger has an ability chain"); + let mut cur = Some(root); + let mut node_count = 0usize; + while let Some(n) = cur { + node_count += 1; + assert_eq!( + n.sibling_condition, + SiblingCondition::Dependent, + "no Thieving Skydiver node may be stamped ReplicatedOrBranch", + ); + cur = n.sub_ability.as_deref(); + } + assert!( + node_count >= 2, + "reach guard: Thieving Skydiver must build a multi-node chain (GainControl + Attach continuation), got {node_count}", + ); +} diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index aa4471679b..e7821757ca 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -16455,6 +16455,12 @@ pub struct AbilityDefinition { /// counter type must be rewritten to the current iteration's counter kind /// before resolution. `None` (default) = branch is fixed (e.g. "+1/+1"). pub iteration_kind_binding: Option, + /// CR 702.1c ("the same is true") + CR 608.2c (written order): whether a + /// `SequentialSibling` continuation with its OWN gating condition must still be + /// checked when a PRECEDING sibling's condition was + /// false. See `SiblingCondition`. `Dependent` (default) preserves today's + /// behavior; `ReplicatedOrBranch` marks per-item keyword-list replication. + pub sibling_condition: SiblingCondition, } /// Private serialization mirror for `AbilityDefinition`. Holds a borrowed view @@ -16530,6 +16536,8 @@ struct AbilityDefinitionRepr<'a> { sub_link: SubAbilityLink, #[serde(skip_serializing_if = "Option::is_none")] iteration_kind_binding: &'a Option, + #[serde(skip_serializing_if = "SiblingCondition::is_default")] + sibling_condition: SiblingCondition, } impl Serialize for AbilityDefinition { @@ -16574,6 +16582,7 @@ impl Serialize for AbilityDefinition { repeat_until, sub_link, iteration_kind_binding, + sibling_condition, } = self; let repr = AbilityDefinitionRepr { kind, @@ -16613,6 +16622,7 @@ impl Serialize for AbilityDefinition { repeat_until, sub_link: *sub_link, iteration_kind_binding, + sibling_condition: *sibling_condition, }; /// Flatten wrapper: the mirror carries the real field set; /// `consumes_source` (#506) and `is_mana_ability` (CR 605.1a) are @@ -16721,6 +16731,8 @@ struct AbilityDefinitionDe { sub_link: SubAbilityLink, #[serde(default)] iteration_kind_binding: Option, + #[serde(default)] + sibling_condition: SiblingCondition, } impl<'de> Deserialize<'de> for AbilityDefinition { @@ -16770,6 +16782,7 @@ impl<'de> Deserialize<'de> for AbilityDefinition { repeat_until: de.repeat_until, sub_link: de.sub_link, iteration_kind_binding: de.iteration_kind_binding, + sibling_condition: de.sibling_condition, }) } } @@ -16803,6 +16816,36 @@ impl SubAbilityLink { } } +/// CR 702.1c ("the same is true") + CR 608.2c (written order): whether a +/// `SequentialSibling` continuation with its OWN gating condition must still be +/// checked when a PRECEDING sibling's condition was +/// false. `Dependent` (default) is today's behavior — the continuation's own +/// condition/effect may presuppose the preceding sibling's effect actually ran +/// (Thieving Skydiver's "If that artifact is an Equipment" presupposes +/// `GainControl` produced a target), so it is skipped alongside a failed +/// predecessor. `ReplicatedOrBranch` marks a sibling produced by per-item +/// keyword-list replication ("The same is true for…" is CR 702.1c; "Repeat +/// this process for…" follows CR 608.2c) — each item is an INDEPENDENT OR-branch checked on its own +/// keyword, so it must be evaluated regardless of any other branch's outcome. +/// Stamped ONLY by the `ReplicatePerKeyword` lowering helpers +/// (`attach_repeat_process_keywords`, `attach_perpetual_keyword_grants`) — +/// never by ordinary sentence-boundary `SequentialSibling` stamping — so it +/// cannot leak into a Thieving-Skydiver-shaped dependent continuation that +/// also happens to carry `SequentialSibling`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +pub enum SiblingCondition { + #[default] + Dependent, + ReplicatedOrBranch, +} + +impl SiblingCondition { + /// `skip_serializing_if` predicate — the default needs no JSON byte. + pub fn is_default(cond: &Self) -> bool { + matches!(cond, Self::Dependent) + } +} + /// CR 608.2c + CR 107.1c: how a "repeat this process" loop decides whether to /// run another iteration. The non-count companion to `AbilityDefinition`'s /// `repeat_for` (a fixed `QuantityExpr` count) — this predicate decides @@ -16924,6 +16967,7 @@ impl AbilityDefinition { repeat_until: None, sub_link: SubAbilityLink::ContinuationStep, iteration_kind_binding: None, + sibling_condition: SiblingCondition::Dependent, } } @@ -21468,6 +21512,15 @@ pub struct ResolvedAbility { /// `SequentialSibling` subs resolve even when an optional parent is declined. #[serde(default, skip_serializing_if = "SubAbilityLink::is_continuation")] pub sub_link: SubAbilityLink, + /// CR 702.1c ("the same is true") + CR 608.2c (written order): Copied through + /// from the originating `AbilityDefinition`. When `ReplicatedOrBranch`, this + /// `SequentialSibling` is an INDEPENDENT + /// per-item OR-branch produced by keyword-list replication (Mutable Pupa, + /// Kathril) and must be evaluated by `resolve_chain_body` regardless of a + /// preceding sibling's failed gate. `Dependent` (default) preserves the + /// prior skip-with-failed-predecessor behavior. See [`SiblingCondition`]. + #[serde(default, skip_serializing_if = "SiblingCondition::is_default")] + pub sibling_condition: SiblingCondition, /// CR 700.2b + CR 603.3c: Modal choice for a reflexive modal trigger whose modes /// are gated behind an optional cost (Caesar). Carried from the def so /// try_begin_reflexive_target_selection can hand it to the PendingTrigger and @@ -21542,6 +21595,7 @@ impl ResolvedAbility { repeat_until: None, replacement_applied: HashSet::new(), sub_link: SubAbilityLink::ContinuationStep, + sibling_condition: SiblingCondition::Dependent, source_incarnation: None, trigger_source: None, trigger_definition_ref: None, diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index fac0eefaf6..a8eb15f66b 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -709,6 +709,7 @@ mod msh_wave5a_group2_conditions; mod multi_layer_continuous_effect; mod multi_upkeep_triggers_suspend; mod must_attack_player_attribution; +mod mutable_pupa_perpetual_keyword_mirror; mod mycoloth_upkeep_trigger; mod myrkul_crew_phase1_incarnation; mod mystic_forge_regression; diff --git a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs new file mode 100644 index 0000000000..f69bde6775 --- /dev/null +++ b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs @@ -0,0 +1,360 @@ +//! Issue #6321 — Mutable Pupa's perpetual keyword-mirror ETB trigger. +//! +//! "Whenever another creature you control enters, this creature perpetually +//! gains flying if that creature has flying. The same is true for first strike, +//! double strike, deathtouch, haste, hexproof, indestructible, lifelink, menace, +//! reach, trample, and vigilance." +//! +//! Digital-only Alchemy (no CR entry for "perpetually"); CR 702.1c + CR 608.2c govern the +//! per-branch resolution order that the `SiblingCondition::ReplicatedOrBranch` +//! marker restores. Each of the 12 keyword nodes is an INDEPENDENT OR-branch +//! gated on the entering object having THAT keyword — so the grant list must not +//! collapse to keyword[0]'s gate, and a keyword deep in the list (vigilance, +//! node 11) must still resolve after the earlier gates (flying, etc.) are false. +//! +//! These drive the REAL cast pipeline (`GameRunner::cast(..).resolve()`); the +//! Mutable Pupa trigger grants to its SOURCE (no target selection), so the +//! entering creature is cast and the trigger auto-resolves. Oracle text is +//! verbatim from data/card-data.json. + +use engine::game::layers::evaluate_layers; +use engine::game::scenario::{GameScenario, P0}; +use engine::types::actions::GameAction; +use engine::types::counter::CounterType; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::keywords::Keyword; +use engine::types::mana::{ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +const MUTABLE_PUPA: &str = "Whenever another creature you control enters, this creature perpetually gains flying if that creature has flying. The same is true for first strike, double strike, deathtouch, haste, hexproof, indestructible, lifelink, menace, reach, trample, and vigilance."; + +/// Every keyword the mirror can grant EXCEPT the ones a given entering creature +/// carries — used for the "no collapsed/leaked grant" negative sweep. +const ALL_MIRROR_KEYWORDS: &[Keyword] = &[ + Keyword::Flying, + Keyword::FirstStrike, + Keyword::DoubleStrike, + Keyword::Deathtouch, + Keyword::Haste, + Keyword::Hexproof, + Keyword::Indestructible, + Keyword::Lifelink, + Keyword::Menace, + Keyword::Reach, + Keyword::Trample, + Keyword::Vigilance, +]; + +/// Fund a pool with `n` white mana (white pays generic too), so a small creature +/// cast auto-pays without surfacing a mana window. The exact cost is not the +/// subject under test. +fn white_pool(n: usize) -> Vec { + vec![ManaUnit::new(ManaType::White, ObjectId(9_999), false, vec![]); n] +} + +// Affa Protector ({2}{W}, Human Soldier Ally, 1/4) has exactly one of the listed +// keywords — Vigilance. It enters under Mutable Pupa's controller: the mirror +// must grant Vigilance (node 11, reached ONLY because the resolve_chain_body +// ReplicatedOrBranch disjunct carries the chain past the false flying/first +// strike/... gates) and grant NOTHING else. +#[test] +fn mutable_pupa_gains_only_the_entering_creatures_vigilance() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let pupa = scenario + .add_creature_from_oracle(P0, "Mutable Pupa", 1, 1, MUTABLE_PUPA) + .id(); + // Affa Protector — keyword built via the builder (not inline reminder text), + // per the card-test foot-gun on inline keyword lines. + let affa = scenario + .add_creature_to_hand(P0, "Affa Protector", 1, 4) + .vigilance() + .id(); + scenario.with_mana_pool(P0, white_pool(3)); + let mut runner = scenario.build(); + + // Baseline reach-guard (positive, not vacuous): Mutable Pupa starts with + // neither Vigilance nor Flying. + assert!( + !runner.state().objects[&pupa].has_keyword(&Keyword::Vigilance), + "baseline: Mutable Pupa has no vigilance", + ); + assert!( + !runner.state().objects[&pupa].has_keyword(&Keyword::Flying), + "baseline: Mutable Pupa has no flying", + ); + + let outcome = runner.cast(affa).resolve(); + // Reach-guard: Affa Protector actually entered (so the trigger really fired). + outcome.assert_zone(&[affa], Zone::Battlefield); + + // Re-run layers so the perpetual base_keywords grant is reflected live. + let mut state = outcome.state().clone(); + state.layers_dirty.mark_full(); + evaluate_layers(&mut state); + let pupa_obj = &state.objects[&pupa]; + + // Vigilance IS granted — FALSE without the resolve_chain_body fix (the chain + // would collapse at flying's false gate and never reach node 11). + assert!( + pupa_obj.has_keyword(&Keyword::Vigilance), + "Affa Protector has vigilance ⇒ Mutable Pupa perpetually gains vigilance", + ); + // Nothing else the entering creature lacks is granted (no collapse/leak). + for kw in ALL_MIRROR_KEYWORDS { + if *kw == Keyword::Vigilance { + continue; + } + assert!( + !pupa_obj.has_keyword(kw), + "Mutable Pupa must not gain {kw:?} (Affa Protector lacks it)", + ); + } +} + +// Accumulation: an entering creature carrying TWO listed keywords (vigilance AND +// trample) makes the mirror grant BOTH — independent `ApplyPerpetual` +// resolutions accumulate (GrantKeywords pushes to base_keywords, never +// overwrites), and neither is at keyword[0]'s position. +#[test] +fn mutable_pupa_accumulates_every_matching_keyword() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let pupa = scenario + .add_creature_from_oracle(P0, "Mutable Pupa", 1, 1, MUTABLE_PUPA) + .id(); + let twin = scenario + .add_creature_to_hand(P0, "Twin Keyworder", 3, 3) + .vigilance() + .trample() + .id(); + scenario.with_mana_pool(P0, white_pool(4)); + let mut runner = scenario.build(); + + assert!( + !runner.state().objects[&pupa].has_keyword(&Keyword::Vigilance) + && !runner.state().objects[&pupa].has_keyword(&Keyword::Trample), + "baseline: Mutable Pupa has neither vigilance nor trample", + ); + + let outcome = runner.cast(twin).resolve(); + outcome.assert_zone(&[twin], Zone::Battlefield); + + let mut state = outcome.state().clone(); + state.layers_dirty.mark_full(); + evaluate_layers(&mut state); + let pupa_obj = &state.objects[&pupa]; + + // BOTH matching keywords accumulate (trample is node 10, vigilance node 11 — + // both past the earlier false gates, and both independently granted). + assert!( + pupa_obj.has_keyword(&Keyword::Vigilance), + "Mutable Pupa gains vigilance", + ); + assert!( + pupa_obj.has_keyword(&Keyword::Trample), + "Mutable Pupa gains trample (accumulates alongside vigilance, not overwritten)", + ); + for kw in ALL_MIRROR_KEYWORDS { + if matches!(kw, Keyword::Vigilance | Keyword::Trample) { + continue; + } + assert!( + !pupa_obj.has_keyword(kw), + "Mutable Pupa must not gain {kw:?} (Twin Keyworder lacks it)", + ); + } +} + +// ----------------------------------------------------------------------- +// Kathril, Aspect Warper — the SAME list-collapse bug in the counters class +// (`ReplicateKind::CounterPlacement` via `attach_repeat_process_keywords`), +// fixed by the same `SiblingCondition::ReplicatedOrBranch` marker + the shared +// `resolve_chain_body` disjunct. CR 608.2c. Oracle text verbatim from +// data/card-data.json. The counter recipient, "any creature you control", now +// parses to a real `TargetFilter::Typed{Creature, controller: You}` (see the +// `parse_type_phrase_with_ctx` "any " quantifier fix) instead of falling back +// to the degenerate `TargetFilter::Any`. +// +// CR 608.2d: an untargeted "any creature you control" choice (no literal +// "target") is made independently at EACH instruction's own resolution, not +// once when the whole ability goes on the stack. `target_choice_timing_for_ +// clause` (`oracle_effect/lower.rs`) marks every untargeted, non-context-ref +// `PutCounter` recipient `Resolution`-timed (widened from the narrower +// Equipped/Enchanted-only case, matching `MultiplyCounter`'s existing +// pattern); `resolve_chain_body` skips copying a parent's already-chosen +// target into a `Resolution`-timed sub; and each such instruction that +// reaches resolution with an empty, multi-candidate recipient opens an +// interactive `WaitingFor::ChooseFromZoneChoice` prompt (a single legal +// candidate auto-binds with no prompt; zero is a silent no-op — CR 608.2d: +// "The player can't choose an option that's illegal or impossible") — +// reusing the SAME parked-continuation machinery already proven +// for `PutCounter` (the Bolster keyword action). The two tests below cover +// both the single-creature case (no observable choice, matches the original +// #6321 regression exactly) and the multi-creature case (proves the choice +// is genuinely independent per instruction, not one shared pick). +// ----------------------------------------------------------------------- + +const KATHRIL: &str = "When Kathril enters, put a flying counter on any creature you control if a creature card in your graveyard has flying. Repeat this process for first strike, double strike, deathtouch, hexproof, indestructible, lifelink, menace, reach, trample, and vigilance. Then put a +1/+1 counter on Kathril for each counter put on a creature this way."; + +// Only trample is in the graveyard — flying (K0) and every gate before trample +// are FALSE. The trample counter must still be placed (the chain reaches node 9 +// past the false earlier gates), the flying counter must NOT, and the +// unconditional +1/+1 tail must land (the chain reaches the tail past the last +// false gate). Reverting the `attach_repeat_process_keywords` marker OR the +// `resolve_chain_body` disjunct collapses the chain at flying's false gate, and +// both the trample and the +1/+1 assertions flip. +#[test] +fn kathril_reaches_matching_counter_and_tail_past_false_earlier_gates() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + // A creature card with ONLY trample (explicitly NOT flying) in P0's graveyard. + scenario + .add_creature_to_graveyard(P0, "Trampling Remains", 2, 2) + .trample(); + let kathril = scenario + .add_creature_to_hand_from_oracle(P0, "Kathril, Aspect Warper", 3, 3, KATHRIL) + .id(); + // Kathril costs {2}{W}{B}{G}; use exact colored and generic mana so the + // cast reaches the ETB trigger under test. + scenario.with_mana_pool( + P0, + vec![ + ManaUnit::new(ManaType::White, ObjectId(9_996), false, vec![]), + ManaUnit::new(ManaType::Black, ObjectId(9_997), false, vec![]), + ManaUnit::new(ManaType::Green, ObjectId(9_998), false, vec![]), + ManaUnit::new(ManaType::Colorless, ObjectId(9_999), false, vec![]), + ManaUnit::new(ManaType::Colorless, ObjectId(10_000), false, vec![]), + ], + ); + let mut runner = scenario.build(); + + let outcome = runner.cast(kathril).target_object(kathril).resolve(); + outcome.assert_zone(&[kathril], Zone::Battlefield); + + let kathril_obj = &outcome.state().objects[&kathril]; + let count = |ct: CounterType| kathril_obj.counters.get(&ct).copied().unwrap_or(0); + + // The trample gate is true → a trample counter is placed (chain reached it). + assert_eq!( + count(CounterType::Keyword(Keyword::Trample.kind())), + 1, + "trample is in the graveyard ⇒ exactly one trample counter is placed", + ); + // The flying gate is false → NO flying counter (per-item independence, not a + // shared/collapsed gate). + assert_eq!( + count(CounterType::Keyword(Keyword::Flying.kind())), + 0, + "no flying card in the graveyard ⇒ no flying counter", + ); + // The unconditional tail fires: a +1/+1 counter on Kathril (proves the chain + // reached the end past vigilance's false gate). + assert_eq!( + count(CounterType::Plus1Plus1), + 1, + "exactly one +1/+1 counter is placed for the one counter put on a creature this way", + ); +} + +// Discriminating case for CR 608.2d: the graveyard has BOTH flying and +// trample, so TWO independent PutCounter instructions fire (flying is node 0, +// the head; trample is node 9, reached only via the SAME per-item independent +// gate the test above proves). P0 controls TWO creatures — Kathril plus +// "Second Recipient", already on the battlefield — so each instruction's "any +// creature you control" choice has a genuine, non-trivial answer. Declaring +// [second_recipient, kathril] in that order pins the flying prompt (which +// fires first, since flying resolves before trample in Oracle-text order) to +// second_recipient and leaves kathril for the trample prompt. If the two +// instructions wrongly shared one upfront choice (the bug this test guards +// against), both counters would land on whichever object was bound first and +// second_recipient would have NEITHER counter. +#[test] +fn kathril_offers_each_matching_counter_its_own_independent_recipient() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + // A creature card with BOTH flying and trample in P0's graveyard, so both + // the head (flying) and the trample sibling fire their own instruction. + scenario + .add_creature_to_graveyard(P0, "Skybound Charger", 3, 3) + .flying() + .trample(); + // Already on the battlefield when Kathril enters — the second legal + // recipient for each "any creature you control" choice. + let second_recipient = scenario.add_creature(P0, "Second Recipient", 1, 1).id(); + let kathril = scenario + .add_creature_to_hand_from_oracle(P0, "Kathril, Aspect Warper", 3, 3, KATHRIL) + .id(); + scenario.with_mana_pool( + P0, + vec![ + ManaUnit::new(ManaType::White, ObjectId(9_996), false, vec![]), + ManaUnit::new(ManaType::Black, ObjectId(9_997), false, vec![]), + ManaUnit::new(ManaType::Green, ObjectId(9_998), false, vec![]), + ManaUnit::new(ManaType::Colorless, ObjectId(9_999), false, vec![]), + ManaUnit::new(ManaType::Colorless, ObjectId(10_000), false, vec![]), + ], + ); + let mut runner = scenario.build(); + + // `.resolve()` stops at the first `WaitingFor::ChooseFromZoneChoice` and + // hands control back here — the shared test driver deliberately does NOT + // auto-drive this variant (it also carries the pre-existing CR 608.2d + // tracked-set choice, e.g. Portent of Calamity's per-type exile picks, + // whose own tests rely on manually driving one prompt at a time). Declare + // the two independent recipients explicitly, in the order the two + // instructions actually resolve: flying (the head) first, trample + // (reached past the intervening false gates) second. + let _outcome = runner.cast(kathril).resolve(); + let declared_recipients = [second_recipient, kathril]; + let mut next_recipient = declared_recipients.iter(); + while let WaitingFor::ChooseFromZoneChoice { .. } = &runner.state().waiting_for { + let pick = *next_recipient + .next() + .expect("exactly two independent recipient prompts (flying, trample)"); + runner + .act(GameAction::SelectCards { cards: vec![pick] }) + .expect("per-instruction recipient choice"); + } + + assert_eq!( + runner.state().objects[&kathril].zone, + Zone::Battlefield, + "Kathril must resolve onto the battlefield" + ); + + let state = runner.state(); + let count_on = + |id: ObjectId, ct: CounterType| state.objects[&id].counters.get(&ct).copied().unwrap_or(0); + let flying_ct = CounterType::Keyword(Keyword::Flying.kind()); + let trample_ct = CounterType::Keyword(Keyword::Trample.kind()); + + // The flying instruction (resolved first) independently chose + // second_recipient — the first declared object still legal at that point. + assert_eq!( + count_on(second_recipient, flying_ct.clone()), + 1, + "second_recipient receives the flying counter (first instruction's own choice)", + ); + assert_eq!( + count_on(second_recipient, trample_ct.clone()), + 0, + "second_recipient must not also receive the trample counter", + ); + // The trample instruction (resolved later, past flying/first_strike/…'s + // now-satisfied-then-irrelevant gates — trample's OWN gate is what matters + // here) independently chose kathril — the only object left declared. + assert_eq!( + count_on(kathril, trample_ct), + 1, + "kathril receives the trample counter (second instruction's OWN, independent choice)", + ); + assert_eq!( + count_on(kathril, flying_ct), + 0, + "kathril must not also receive the flying counter — the two choices are independent, \ + not one shared pick forced onto a single recipient", + ); +} 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 4eb75157be..fa4ddab48b 100644 --- a/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs +++ b/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs @@ -27,8 +27,8 @@ use engine::game::planeswalker; use engine::game::zones::create_object; use engine::types::ability::{ AbilityCost, AbilityDefinition, AbilityKind, CopyCountStatus, Effect, QuantityExpr, - QuantityModification, ReplacementDefinition, ResolvedAbility, SubAbilityLink, TargetFilter, - TargetRef, TargetSelectionMode, + QuantityModification, ReplacementDefinition, ResolvedAbility, SiblingCondition, SubAbilityLink, + TargetFilter, TargetRef, TargetSelectionMode, }; use engine::types::actions::GameAction; use engine::types::card_type::CoreType; @@ -192,6 +192,7 @@ fn make_grant_ability(controller: PlayerId, source: ObjectId) -> ResolvedAbility chosen_players: Vec::new(), repeat_until: None, sub_link: SubAbilityLink::ContinuationStep, + sibling_condition: SiblingCondition::Dependent, modal: None, mode_abilities: vec![], parent_target_missing_reason: None,