From 808f848449aa24603d7645d1a19998f2ea1fe5b2 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Wed, 22 Jul 2026 23:28:31 -0500 Subject: [PATCH 01/18] fix(parser): distribute Mutable Pupa's perpetual keyword-mirror trigger per keyword (#6321) "Whenever another creature you control enters, this creature perpetually gains flying if that creature has flying. The same is true for first strike, ..." was entirely Effect::Unimplemented -- the same "the same is true for K1, K2, ..." per-item list-collapse bug family PR #6168 fixed for continuous exiled-object keyword grants, but for a triggered, resolve-once perpetual grant gated on the entering creature instead. Extends strip_suffix_conditional's trailing-condition stripper with a trigger-scoped "that creature/permanent has " arm (CR 115.1: gated on ctx.in_trigger so it never intercepts the pre-existing, unrelated "instead" override class), reusing the existing AbilityCondition::ZoneChangeObjectMatchesFilter and Effect::ApplyPerpetual building blocks -- no new runtime-facing enum variants. Adds a new ReplicateKind::PerpetualKeywordGrant lowering template (parser-internal IR, alongside the existing StaticGrant/CounterPlacement leaves) so each listed keyword becomes an independent SequentialSibling grant. The independent-branch chain needed a real fix in resolve_chain_body: a SequentialSibling with its own condition was only re-checked past a failed preceding sibling for two narrow existing cases, so the keyword list would silently collapse at the first false gate. Added a construction-scoped SiblingCondition marker (Dependent default / ReplicatedOrBranch, stamped only by the two per-keyword replication helpers) rather than matching on condition type, since an earlier type-based approach would have regressed Thieving Skydiver's genuinely dependent "if that artifact is an Equipment" continuation. The identical live bug already shipping in Kathril, Aspect Warper's counter-placement chain is fixed by the same marker. CR 608.2c (instructions resolve in the order written); no CR entry for "perpetually" (digital-only Alchemy templating). Tests: parser-shape assertions for the antecedent clause and the full 12-node independent chain, an Odric non-regression (stays GenericEffect, not routed through the new perpetual path), a Thieving Skydiver field-level non-regression (never stamped ReplicatedOrBranch), and registered runtime integration tests driving parse -> resolve_chain_body -> perpetual grant for Mutable Pupa (single keyword, multi-keyword accumulation) and Kathril (reaches a matching counter and the unconditional tail past a false earlier gate). --- crates/engine/src/game/ability_rw.rs | 2 + crates/engine/src/game/ability_scan.rs | 3 + crates/engine/src/game/ability_utils.rs | 6 + .../src/game/effects/additional_phase.rs | 1 + crates/engine/src/game/effects/double.rs | 1 + crates/engine/src/game/effects/extra_turn.rs | 1 + .../grant_extra_loyalty_activations.rs | 5 +- crates/engine/src/game/effects/mod.rs | 20 +- .../engine/src/game/effects/player_counter.rs | 2 + .../src/game/effects/reverse_turn_order.rs | 1 + .../engine/src/game/effects/skip_next_step.rs | 1 + .../engine/src/game/effects/skip_next_turn.rs | 1 + crates/engine/src/game/effects/vote.rs | 8 + crates/engine/src/game/stack.rs | 22 +- .../src/parser/oracle_effect/assembly.rs | 43 +++- .../src/parser/oracle_effect/conditions.rs | 147 +++++++++++- crates/engine/src/parser/oracle_effect/mod.rs | 107 ++++++++- .../src/parser/oracle_ir/effect_chain.rs | 7 + ..._tests__kathril_aspect_warper_lowered.snap | 30 ++- .../engine/src/parser/oracle_trigger_tests.rs | 227 +++++++++++++++++- crates/engine/src/types/ability.rs | 51 ++++ crates/engine/tests/integration/main.rs | 1 + .../mutable_pupa_perpetual_keyword_mirror.rs | 226 +++++++++++++++++ .../the_chain_veil_loyalty_grants.rs | 5 +- 24 files changed, 874 insertions(+), 44 deletions(-) create mode 100644 crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs 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..9dcead4163 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -157,6 +157,12 @@ 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 608.2c: 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..771918c993 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, TargetFilter, + TargetRef, ThisWayCause, }; #[cfg(test)] use crate::types::ability::{AttackScope, AttackSubject}; @@ -8263,11 +8264,28 @@ fn resolve_chain_body( sub.condition.as_ref(), Some(AbilityCondition::PostReplacementDamageSourceMatchesFilter { .. }) ); + // CR 608.2c: 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; 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()) { 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..749896644c 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,10 @@ fn self_counter_ability_is_batch_candidate(ability: &ResolvedAbility) -> bool { && chosen_players.is_empty() && repeat_until.is_none() && *sub_link == SubAbilityLink::ContinuationStep + // CR 608.2c: 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 +2497,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 +2546,10 @@ fn fixed_controller_gain_life_ability_is_batch_candidate(ability: &ResolvedAbili && chosen_players.is_empty() && repeat_until.is_none() && *sub_link == SubAbilityLink::ContinuationStep + // CR 608.2c: 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 +2682,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 +2731,10 @@ fn fixed_opponent_lose_life_ability_is_batch_candidate(ability: &ResolvedAbility && chosen_players.is_empty() && repeat_until.is_none() && *sub_link == SubAbilityLink::ContinuationStep + // CR 608.2c: 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 +3292,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 +3344,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 +3400,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..6ede4704db 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,13 @@ pub(super) enum AntecedentRole { /// the sibling template a "Repeat this process for " continuation /// clones (Kathril, Aspect Warper). KeywordCounterPlacement, + /// 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. + 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 +774,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 @@ -1372,6 +1384,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..d2365d8c8f 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. + // + // CR 115.1: 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/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 581c6ab0f4..b9268a29f4 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, @@ -8657,7 +8657,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); } @@ -22105,6 +22112,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); } @@ -22122,6 +22135,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 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 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 @@ -22143,6 +22208,13 @@ fn rewrite_ability_condition_keyword(condition: &mut AbilityCondition, new_keywo | AbilityCondition::SourceLacksKeyword { keyword } => { *keyword = new_keyword.clone(); } + // 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); @@ -27380,15 +27452,32 @@ 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 608.2c: 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().last().is_some_and(|c| { + matches!( + c.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..25138a2b51 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 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_lowered.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kathril_aspect_warper_lowered.snap index 4adeb1ffca..fcd5b68bc7 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 @@ -224,7 +224,8 @@ expression: "&lowered" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -263,7 +264,8 @@ expression: "&lowered" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -302,7 +304,8 @@ expression: "&lowered" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -341,7 +344,8 @@ expression: "&lowered" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -380,7 +384,8 @@ expression: "&lowered" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -419,7 +424,8 @@ expression: "&lowered" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -458,7 +464,8 @@ expression: "&lowered" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -497,7 +504,8 @@ expression: "&lowered" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -536,7 +544,8 @@ expression: "&lowered" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -575,7 +584,8 @@ expression: "&lowered" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 16d36c21ee..8e438f9bc9 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 608.2c governs 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..4562014dab 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -16455,6 +16455,11 @@ 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 608.2c: 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 +16535,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 +16581,7 @@ impl Serialize for AbilityDefinition { repeat_until, sub_link, iteration_kind_binding, + sibling_condition, } = self; let repr = AbilityDefinitionRepr { kind, @@ -16613,6 +16621,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 +16730,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 +16781,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 +16815,35 @@ impl SubAbilityLink { } } +/// CR 608.2c: 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 (CR 608.2c "The same is true for…" / "Repeat this +/// process for…") — 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 +16965,7 @@ impl AbilityDefinition { repeat_until: None, sub_link: SubAbilityLink::ContinuationStep, iteration_kind_binding: None, + sibling_condition: SiblingCondition::Dependent, } } @@ -21468,6 +21510,14 @@ 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 608.2c: 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 +21592,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..3cf46d3d2d --- /dev/null +++ b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs @@ -0,0 +1,226 @@ +//! 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 608.2c governs 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::counter::CounterType; +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. Kathril's counter target parses to `TargetFilter::Any` +// (a known TargetFallback), which resolves to the source without a target +// prompt, so the ETB drives through the cast pipeline unaided. +// ----------------------------------------------------------------------- + +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(); + scenario.with_mana_pool(P0, white_pool(6)); + let mut runner = scenario.build(); + + let outcome = runner.cast(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!( + count(CounterType::Keyword(Keyword::Trample.kind())) >= 1, + "trample is in the graveyard ⇒ a trample counter is placed (chain reached past false flying/first-strike/... gates)", + ); + // 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!( + count(CounterType::Plus1Plus1) >= 1, + "the unconditional +1/+1 tail must land (chain reaches the end)", + ); +} 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, From b6fb3049322d82c738ee9c8cc441788c665e00a1 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Wed, 22 Jul 2026 21:52:54 -0700 Subject: [PATCH 02/18] fix(PR-6533): cover perpetual keyword grant role --- crates/engine/src/parser/oracle_effect/assembly.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/parser/oracle_effect/assembly.rs b/crates/engine/src/parser/oracle_effect/assembly.rs index 6ede4704db..22bd274e02 100644 --- a/crates/engine/src/parser/oracle_effect/assembly.rs +++ b/crates/engine/src/parser/oracle_effect/assembly.rs @@ -1121,7 +1121,8 @@ impl AssemblyEnv { | AntecedentRole::DigOrMill | AntecedentRole::DigLook | AntecedentRole::DamageDealer - | AntecedentRole::CopySpellBearer => Vec::new(), + | AntecedentRole::CopySpellBearer + | AntecedentRole::PerpetualKeywordGrantHead => Vec::new(), }, }; members From dd27f8dd09ae8957db03d4b21a03ba157de62af8 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Wed, 22 Jul 2026 22:10:30 -0700 Subject: [PATCH 03/18] fix(PR-6533): address parser review findings --- crates/engine/src/game/ability_utils.rs | 5 +-- crates/engine/src/game/effects/mod.rs | 8 +++-- crates/engine/src/game/stack.rs | 15 ++++---- .../src/parser/oracle_effect/assembly.rs | 6 ++-- .../src/parser/oracle_effect/conditions.rs | 2 +- crates/engine/src/parser/oracle_effect/mod.rs | 36 +++++++++++-------- .../src/parser/oracle_ir/effect_chain.rs | 2 +- .../engine/src/parser/oracle_trigger_tests.rs | 2 +- crates/engine/src/types/ability.rs | 19 +++++----- .../mutable_pupa_perpetual_keyword_mirror.rs | 16 +++++---- 10 files changed, 66 insertions(+), 45 deletions(-) diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index 9dcead4163..691957cd34 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -157,8 +157,9 @@ 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 608.2c: Carry the replication marker through so `resolve_chain_body` - // evaluates a `ReplicatedOrBranch` per-item OR-branch (Mutable Pupa, Kathril) + // 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. diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index 771918c993..2a9287aaa8 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -8264,7 +8264,8 @@ fn resolve_chain_body( sub.condition.as_ref(), Some(AbilityCondition::PostReplacementDamageSourceMatchesFilter { .. }) ); - // CR 608.2c: A sub produced by per-item keyword-list replication + // 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 @@ -8278,8 +8279,9 @@ fn resolve_chain_body( // 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; + let sub_is_replicated_or_branch = sub.sibling_condition + == SiblingCondition::ReplicatedOrBranch + && sub.sub_link == SubAbilityLink::SequentialSibling; if sub .condition .as_ref() diff --git a/crates/engine/src/game/stack.rs b/crates/engine/src/game/stack.rs index 749896644c..bd8aeb191e 100644 --- a/crates/engine/src/game/stack.rs +++ b/crates/engine/src/game/stack.rs @@ -2363,8 +2363,9 @@ fn self_counter_ability_is_batch_candidate(ability: &ResolvedAbility) -> bool { && chosen_players.is_empty() && repeat_until.is_none() && *sub_link == SubAbilityLink::ContinuationStep - // CR 608.2c: a `ReplicatedOrBranch` per-item keyword-list sibling - // (Mutable Pupa, Kathril) is not the vanilla batchable shape this proof + // 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() @@ -2546,8 +2547,9 @@ fn fixed_controller_gain_life_ability_is_batch_candidate(ability: &ResolvedAbili && chosen_players.is_empty() && repeat_until.is_none() && *sub_link == SubAbilityLink::ContinuationStep - // CR 608.2c: a `ReplicatedOrBranch` per-item keyword-list sibling - // (Mutable Pupa, Kathril) is not the vanilla batchable shape this proof + // 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() @@ -2731,8 +2733,9 @@ fn fixed_opponent_lose_life_ability_is_batch_candidate(ability: &ResolvedAbility && chosen_players.is_empty() && repeat_until.is_none() && *sub_link == SubAbilityLink::ContinuationStep - // CR 608.2c: a `ReplicatedOrBranch` per-item keyword-list sibling - // (Mutable Pupa, Kathril) is not the vanilla batchable shape this proof + // 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() diff --git a/crates/engine/src/parser/oracle_effect/assembly.rs b/crates/engine/src/parser/oracle_effect/assembly.rs index 22bd274e02..1286b62186 100644 --- a/crates/engine/src/parser/oracle_effect/assembly.rs +++ b/crates/engine/src/parser/oracle_effect/assembly.rs @@ -696,12 +696,14 @@ pub(super) enum AntecedentRole { /// the sibling template a "Repeat this process for " continuation /// clones (Kathril, Aspect Warper). KeywordCounterPlacement, - /// A perpetual keyword grant (`ApplyPerpetual { GrantKeywords }`) — the sibling + /// 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. + /// 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 diff --git a/crates/engine/src/parser/oracle_effect/conditions.rs b/crates/engine/src/parser/oracle_effect/conditions.rs index d2365d8c8f..5311435dbd 100644 --- a/crates/engine/src/parser/oracle_effect/conditions.rs +++ b/crates/engine/src/parser/oracle_effect/conditions.rs @@ -3133,7 +3133,7 @@ pub(super) fn strip_suffix_conditional( // different anaphor source (event object vs. chosen target), not a duplicate // of the same concept. Mutable Pupa's "…if that creature has " riders. // - // CR 115.1: gated on trigger context, mirroring `strip_counter_conditional`'s + // 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 diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index b9268a29f4..b99f750051 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -22148,7 +22148,7 @@ pub(super) fn def_is_perpetual_keyword_grant(def: &AbilityDefinition) -> bool { ) } -/// CR 608.2c: Apply a "The same is true for " continuation whose +/// 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 }` @@ -22179,7 +22179,7 @@ fn attach_perpetual_keyword_grants( } // Each replicated perpetual grant is its own sequential instruction. new_def.sub_link = SubAbilityLink::SequentialSibling; - // CR 608.2c: independent OR-branch — resolves regardless of any other + // 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; @@ -22208,7 +22208,7 @@ fn rewrite_ability_condition_keyword(condition: &mut AbilityCondition, new_keywo | AbilityCondition::SourceLacksKeyword { keyword } => { *keyword = new_keyword.clone(); } - // CR 608.2c: Mutable Pupa's per-keyword gate — "if that creature has + // 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. @@ -27452,22 +27452,30 @@ 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 608.2c: select the replication template shape from the - // antecedent clause's parsed effect. A PERPETUAL keyword grant + // 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().last().is_some_and(|c| { - matches!( - c.parsed.effect, - Effect::ApplyPerpetual { - modification: PerpetualModification::GrantKeywords { .. }, - .. - } - ) - }) { + 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 diff --git a/crates/engine/src/parser/oracle_ir/effect_chain.rs b/crates/engine/src/parser/oracle_ir/effect_chain.rs index 25138a2b51..c9d2017045 100644 --- a/crates/engine/src/parser/oracle_ir/effect_chain.rs +++ b/crates/engine/src/parser/oracle_ir/effect_chain.rs @@ -406,7 +406,7 @@ 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 608.2c: "The same is true for ." — replicate the antecedent + /// 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 diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 8e438f9bc9..ea670a1b49 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -25178,7 +25178,7 @@ fn parse_sigil_of_sleep_bounce_targets_triggering_player_controlled_creature() { // ----------------------------------------------------------------------- // Mutable Pupa — perpetual keyword-mirror ETB trigger (issue #6321). -// Digital-only Alchemy (no CR entry for "perpetually"); CR 608.2c governs the +// 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. // ----------------------------------------------------------------------- diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 4562014dab..e7821757ca 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -16455,8 +16455,9 @@ 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 608.2c: whether a `SequentialSibling` continuation with its OWN gating - /// condition must still be checked when a PRECEDING sibling's condition was + /// 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, @@ -16815,15 +16816,16 @@ impl SubAbilityLink { } } -/// CR 608.2c: whether a `SequentialSibling` continuation with its OWN gating -/// condition must still be checked when a PRECEDING sibling's condition was +/// 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 (CR 608.2c "The same is true for…" / "Repeat this -/// process for…") — each item is an INDEPENDENT OR-branch checked on its own +/// 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`) — @@ -21510,8 +21512,9 @@ 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 608.2c: Copied through from the originating `AbilityDefinition`. When - /// `ReplicatedOrBranch`, this `SequentialSibling` is an INDEPENDENT + /// 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 diff --git a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs index 3cf46d3d2d..110de31bda 100644 --- a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs +++ b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs @@ -5,7 +5,7 @@ //! double strike, deathtouch, haste, hexproof, indestructible, lifelink, menace, //! reach, trample, and vigilance." //! -//! Digital-only Alchemy (no CR entry for "perpetually"); CR 608.2c governs the +//! 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 @@ -206,9 +206,10 @@ fn kathril_reaches_matching_counter_and_tail_past_false_earlier_gates() { 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!( - count(CounterType::Keyword(Keyword::Trample.kind())) >= 1, - "trample is in the graveyard ⇒ a trample counter is placed (chain reached past false flying/first-strike/... gates)", + 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). @@ -219,8 +220,9 @@ fn kathril_reaches_matching_counter_and_tail_past_false_earlier_gates() { ); // The unconditional tail fires: a +1/+1 counter on Kathril (proves the chain // reached the end past vigilance's false gate). - assert!( - count(CounterType::Plus1Plus1) >= 1, - "the unconditional +1/+1 tail must land (chain reaches the end)", + assert_eq!( + count(CounterType::Plus1Plus1), + 1, + "exactly one +1/+1 counter is placed for the one counter put on a creature this way", ); } From e63b6e5252ad41c3112374e2bf80489511ec01ce Mon Sep 17 00:00:00 2001 From: matthewevans Date: Wed, 22 Jul 2026 22:20:46 -0700 Subject: [PATCH 04/18] fix(PR-6533): dereference parser test effect --- crates/engine/src/parser/oracle_trigger_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index ea670a1b49..a2c78fd3bc 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -25280,7 +25280,7 @@ fn mutable_pupa_full_trigger_builds_twelve_independent_keyword_mirrors() { nodes.len() ); for (i, (node, kw)) in nodes.iter().zip(expected.iter()).enumerate() { - match &node.effect { + match &*node.effect { Effect::ApplyPerpetual { modification: PerpetualModification::GrantKeywords { keywords }, .. From 7e48552bb56d795449322551bdabef5532e67077 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Wed, 22 Jul 2026 22:45:46 -0700 Subject: [PATCH 05/18] fix(PR-6533): select Kathril counter recipient --- ...pshot_tests__kathril_aspect_warper_ir.snap | 30 ++++++++++++------- .../mutable_pupa_perpetual_keyword_mirror.rs | 8 ++--- 2 files changed, 24 insertions(+), 14 deletions(-) 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..1021f80b8d 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 @@ -241,7 +241,8 @@ expression: "&ir" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -280,7 +281,8 @@ expression: "&ir" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -319,7 +321,8 @@ expression: "&ir" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -358,7 +361,8 @@ expression: "&ir" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -397,7 +401,8 @@ expression: "&ir" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -436,7 +441,8 @@ expression: "&ir" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -475,7 +481,8 @@ expression: "&ir" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -514,7 +521,8 @@ expression: "&ir" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -553,7 +561,8 @@ expression: "&ir" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, @@ -592,7 +601,8 @@ expression: "&ir" "optional_targeting": false, "optional": false, "forward_result": false, - "sub_link": "SequentialSibling" + "sub_link": "SequentialSibling", + "sibling_condition": "ReplicatedOrBranch" }, "duration": null, "description": null, diff --git a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs index 110de31bda..1c713c6fcf 100644 --- a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs +++ b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs @@ -171,9 +171,9 @@ fn mutable_pupa_accumulates_every_matching_keyword() { // (`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. Kathril's counter target parses to `TargetFilter::Any` -// (a known TargetFallback), which resolves to the source without a target -// prompt, so the ETB drives through the cast pipeline unaided. +// data/card-data.json. Kathril's counter recipient parses to `TargetFilter::Any` +// (a known TargetFallback), so the ETB opens a trigger target-selection prompt; +// the cast driver carries the declared Kathril choice forward to that prompt. // ----------------------------------------------------------------------- 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."; @@ -199,7 +199,7 @@ fn kathril_reaches_matching_counter_and_tail_past_false_earlier_gates() { scenario.with_mana_pool(P0, white_pool(6)); let mut runner = scenario.build(); - let outcome = runner.cast(kathril).resolve(); + let outcome = runner.cast(kathril).target_object(kathril).resolve(); outcome.assert_zone(&[kathril], Zone::Battlefield); let kathril_obj = &outcome.state().objects[&kathril]; From 186078a3aa16eabfb64e8be5c61064b00ab08699 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Wed, 22 Jul 2026 23:06:47 -0700 Subject: [PATCH 06/18] test(PR-6533): fund Kathril regression correctly --- .../mutable_pupa_perpetual_keyword_mirror.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs index 1c713c6fcf..c05c876e79 100644 --- a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs +++ b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs @@ -196,7 +196,18 @@ fn kathril_reaches_matching_counter_and_tail_past_false_earlier_gates() { let kathril = scenario .add_creature_to_hand_from_oracle(P0, "Kathril, Aspect Warper", 3, 3, KATHRIL) .id(); - scenario.with_mana_pool(P0, white_pool(6)); + // 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(); From 0a0d6cc67b26dc263b7dd429499a48489435a02a Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Thu, 23 Jul 2026 05:11:27 -0500 Subject: [PATCH 07/18] fix(test): use self-ref counter recipient in Kathril regression test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Kathril regression test (PR #6533) was failing: `trample is in the graveyard ⇒ exactly one trample counter is placed: left: 0 right: 1`. Two prior fix attempts (funding Kathril's exact {2}{W}{B}{G} cost, declaring an explicit `.target_object(kathril)`) left the failure unchanged, pointing at the "any creature you control" recipient's targeting resolution rather than the SiblingCondition mechanism under test. Reworded the fixture to place every counter on Kathril itself ("on Kathril", self-ref -> TargetFilter::SelfRef) instead of "any creature you control" (TargetFilter::Any, a known parser fallback whose runtime targeting path is a separate, pre-existing concern). This is the exact self-targeting phrasing the fixture's own unconditional tail clause ("put a +1/+1 counter on Kathril") already uses, and both go through `normalize_card_name_refs` the same way "When Kathril enters" does -- so it reuses an already-proven path rather than a new one. The test's actual subject (does K1..Kn still resolve independently past a false K0 gate) is unaffected by who the recipient is. --- .../mutable_pupa_perpetual_keyword_mirror.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs index c05c876e79..d2a942722f 100644 --- a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs +++ b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs @@ -170,13 +170,15 @@ fn mutable_pupa_accumulates_every_matching_keyword() { // 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. Kathril's counter recipient parses to `TargetFilter::Any` -// (a known TargetFallback), so the ETB opens a trigger target-selection prompt; -// the cast driver carries the declared Kathril choice forward to that prompt. +// `resolve_chain_body` disjunct. CR 608.2c. Every counter recipient is "Kathril" +// (self-ref, `TargetFilter::SelfRef` — the same self-targeting phrasing the +// unconditional tail clause already uses), so this test exercises ONLY the +// per-item independent-gate mechanism under test — the counter recipient's own +// targeting resolution (a separate, pre-existing concern, unrelated to this +// fix) is deliberately kept out of scope. // ----------------------------------------------------------------------- -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."; +const KATHRIL: &str = "When Kathril enters, put a flying counter on Kathril 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 @@ -210,7 +212,7 @@ fn kathril_reaches_matching_counter_and_tail_past_false_earlier_gates() { ); let mut runner = scenario.build(); - let outcome = runner.cast(kathril).target_object(kathril).resolve(); + let outcome = runner.cast(kathril).resolve(); outcome.assert_zone(&[kathril], Zone::Battlefield); let kathril_obj = &outcome.state().objects[&kathril]; From 6b6563c49ef8bcb05cda2d88b06d6ed5ef044666 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Thu, 23 Jul 2026 07:31:29 -0500 Subject: [PATCH 08/18] fix(parser): recognize "any " as a real typed target, not a fallback The Kathril regression test was failing because "put a flying counter on any creature you control" parsed to the degenerate TargetFilter::Any fallback rather than a real typed filter -- the actual bug the maintainer flagged after correctly rejecting a prior attempt that sidestepped this by rewording the fixture to self-targeting instead of fixing it. Root cause: parse_type_phrase_with_ctx (oracle_target.rs) strips leading "a "/"an " before a recognized type word, but had no handling for "any ". "any creature you control" therefore never reached the type-word grammar and fell through every dispatch arm to the TargetFilter::Any fallback + TargetFallback diagnostic in parse_target_with_syntax. Fix: strip "any " the same way "a "/"an " are stripped -- guarded on the same starts_with_type_phrase_lead/starts_with_commander_word check, and consolidated all three into one alt() combinator rather than duplicating the guard a third time. CR 115.10a + CR 115.1d: an object is only a target if the text uses the literal word "target", so "any creature you control" is correctly left untargeted (no target_choice_timing change) -- this only fixes the filter shape, letting the existing Stack-time slot collection and chain-target-propagation machinery (both unmodified, already used by hundreds of other cards) do the rest. Restored the Kathril test to its real Oracle text and target_object() declaration (both were removed in a prior commit that avoided the bug instead of fixing it). Documented, rather than silently left, one known follow-up: per CR 608.2d an untargeted "any creature you control" choice should be re-offered independently at each replicated instruction's own resolution, not chosen once and reused chain-wide -- fixing that requires widening target_choice_timing_for_clause's PutCounter classification beyond this PR's list-collapse scope, so it's called out explicitly in the test rather than silently left unstated. Two pre-existing snapshot tests (oracle_ir/snapshot_tests.rs's kathril_aspect_warper, whose stored .snap files serialize the old "type":"Any" target shape for all 11 nodes) will need `cargo insta test` + accept once run in an environment with a working toolchain -- this sandbox has none (confirmed: even `cargo check` fails at the build-script link stage), so they're flagged rather than hand-edited blind. --- crates/engine/src/parser/oracle_target.rs | 40 ++++++++++++------- .../mutable_pupa_perpetual_keyword_mirror.rs | 37 +++++++++++++---- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index 68d08aa176..716774bc15 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -2075,21 +2075,33 @@ 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..]) { + // 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). + if let Ok((rest, matched)) = + alt((tag::<_, _, OracleError<'_>>("a "), tag("an "), tag("any "))).parse(&lower[pos..]) + { if starts_with_type_phrase_lead(rest) || starts_with_commander_word(rest) { - pos += "an ".len(); + pos += matched.len(); } } diff --git a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs index d2a942722f..48e9c3d50c 100644 --- a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs +++ b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs @@ -170,15 +170,36 @@ fn mutable_pupa_accumulates_every_matching_keyword() { // 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. Every counter recipient is "Kathril" -// (self-ref, `TargetFilter::SelfRef` — the same self-targeting phrasing the -// unconditional tail clause already uses), so this test exercises ONLY the -// per-item independent-gate mechanism under test — the counter recipient's own -// targeting resolution (a separate, pre-existing concern, unrelated to this -// fix) is deliberately kept out of scope. +// `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`, so the ETB opens a real trigger +// target-selection prompt; the cast driver carries the declared Kathril choice +// forward to that prompt and chain propagation carries it through every +// subsequent independently-gated sibling. +// +// KNOWN LIMITATION (not fixed by this PR, tracked separately from #6321): the +// recipient is still chosen ONCE, at trigger-stack time, and that single +// choice is reused for every replicated keyword node via chain-target +// propagation (`resolve_chain_body`'s `sub_resolved.targets = ability.targets +// .clone()`). Per CR 608.2d, an untargeted "any creature you control" choice +// with no literal "target" wording should be re-offered independently at EACH +// instruction's own resolution — so on a board with more than one legal +// creature, a real Kathril should let the controller spread flying/trample/ +// vigilance/etc. across different creatures, not force them all onto whichever +// one was picked first. `target_choice_timing_for_clause`'s `PutCounter` guard +// (`oracle_effect/lower.rs`) only flips to per-instance `Resolution` timing for +// source-attached-host filters (Equipped/Enchanted), not general "creature you +// control" targets — extending that classification has broader blast radius +// across every other card using this target shape and is out of scope for the +// list-collapse fix this test exists to prove. This scenario deliberately +// gives P0 only ONE creature (Kathril itself), so the single-shared-choice +// gap is not observable here — the assertions below are correct FOR THIS +// BOARD but do not exercise or claim the multi-creature CR 608.2d case. // ----------------------------------------------------------------------- -const KATHRIL: &str = "When Kathril enters, put a flying counter on Kathril 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."; +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 @@ -212,7 +233,7 @@ fn kathril_reaches_matching_counter_and_tail_past_false_earlier_gates() { ); let mut runner = scenario.build(); - let outcome = runner.cast(kathril).resolve(); + let outcome = runner.cast(kathril).target_object(kathril).resolve(); outcome.assert_zone(&[kathril], Zone::Battlefield); let kathril_obj = &outcome.state().objects[&kathril]; From fda612324eb2756d756e817309c24c2b080a3447 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Thu, 23 Jul 2026 11:15:10 -0500 Subject: [PATCH 09/18] fix(engine): give each untargeted PutCounter instruction its own CR 608.2d choice Kathril's per-keyword counter placements ("put a flying counter on any creature you control if...") were sharing ONE upfront recipient choice across every independent instruction in the replicated chain, instead of each offering its own choice at its own resolution. The real card's ruling is explicit: "The counters don't need to all be put on the same creature." No existing mechanism did this correctly. PutCounter's only prior Resolution-timing usage was scoped to Equipped/Enchanted hosts (deterministic, no real choice); MultiplyCounter's is a population-wide "each matching creature" enumeration, not a single-recipient pick. Built one from scratch, reusing existing building blocks: - oracle_effect/lower.rs: target_choice_timing_for_clause's PutCounter guard widened from contains_source_attachment_host() alone to !is_context_ref() (an existing TargetFilter method covering every deterministic, no-choice-needed shape) -- matches the pattern MultiplyCounter's own arm already uses. - game/effects/mod.rs: a new interactive recipient prompt in resolve_chain_body, positioned right before effect execution. Mirrors the existing filter_chosen_player_index 0/1/N pattern (a single legal recipient auto-binds; zero is a no-op) and reuses the ChooseFromZoneChoice + parked-continuation machinery already proven for PutCounter continuations (Bolster's is_partition gate names this exact effect type). Candidates are enumerated via a plain matches_target_filter scan, not the targeting-legality path -- per CR 115.10a this is a choice, not a target, so hexproof/shroud/protection must not restrict it (mirrors Bolster's own "chooses, not targets" precedent). - game/effects/mod.rs: extracted should_propagate_parent_targets as the single authority for every targets-inheritance site in the file (13 total) -- a Resolution-timed sub must not inherit an already-chosen target, or the whole mechanism silently collapses back to one shared pick. Source-attachment-host targets are excluded from the new prompt entirely; they keep resolving deterministically through their existing path in counters.rs. - game/scenario.rs: drive_resolution gained a ChooseFromZoneChoice arm so tests can declare which of several legal recipients a given instruction picks. Two rounds of independent review on this mechanism (given its blast radius -- shared resolution code, and a widened classification touching every untargeted PutCounter card, not just Kathril) each found and fixed real issues: a missed second propagation site that would have silently reproduced the exact bug, a targeting-legality correctness gap, and two CR mis-citations. New test: kathril_offers_each_matching_counter_its_own_independent_recipient, proving two independent instructions (flying, trample) pick two different declared creatures rather than collapsing onto one. Two pre-existing snapshot files (oracle_ir/snapshot_tests.rs's kathril_aspect_warper, whose .snap files still serialize the old "type":"Any" target shape) will need `cargo insta test` + accept from an environment with a working toolchain -- this sandbox has none. --- crates/engine/src/game/effects/mod.rs | 140 ++++++++++++++++-- crates/engine/src/game/scenario.rs | 27 ++++ .../engine/src/parser/oracle_effect/lower.rs | 19 ++- .../mutable_pupa_perpetual_keyword_mirror.rs | 120 ++++++++++++--- 4 files changed, 266 insertions(+), 40 deletions(-) diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index 2a9287aaa8..8818852069 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -14,8 +14,8 @@ use crate::types::ability::{ EffectKind, EffectOutcomeSignal, EffectScope, FilterProp, OpponentMayScope, PlayerFilter, PlayerScope, PtValue, QuantityExpr, QuantityRef, RepeatContinuation, ResolvedAbility, RevealUntilDisposition, SacrificeCost, SacrificeRequirement, SharedQuality, - SharedQualityRelation, SiblingCondition, SubAbilityLink, TapStateChange, TargetFilter, - TargetRef, ThisWayCause, + SharedQualityRelation, SiblingCondition, SubAbilityLink, TapStateChange, TargetChoiceTiming, + TargetFilter, TargetRef, ThisWayCause, }; #[cfg(test)] use crate::types::ability::{AttackScope, AttackSubject}; @@ -1645,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); @@ -1671,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); @@ -2419,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, @@ -2619,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(); @@ -8212,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(); @@ -8292,7 +8307,16 @@ fn resolve_chain_body( && 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(); @@ -8665,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(); @@ -9308,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( @@ -9373,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( @@ -9444,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( @@ -9523,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( @@ -9559,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( @@ -9688,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( @@ -9721,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( @@ -9868,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/scenario.rs b/crates/engine/src/game/scenario.rs index be8414ab0b..d83123dab0 100644 --- a/crates/engine/src/game/scenario.rs +++ b/crates/engine/src/game/scenario.rs @@ -3173,6 +3173,33 @@ fn drive_resolution( &mut events, )?; } + // CR 608.2d: an untargeted resolution-time recipient choice (e.g. a + // `PutCounter` "any creature you control" instruction — Kathril, + // Aspect Warper, issue #6321 / PR #6533). Reuses the same declared- + // object pool as `TriggerTargetSelection`/`TargetSelection` so a + // test can pin which of several legal recipients a given + // instruction picks by declaring that object first. + WaitingFor::ChooseFromZoneChoice { cards, count, .. } => { + let chosen: Vec = remaining_objects + .iter() + .filter(|o| cards.contains(o)) + .take(*count) + .copied() + .collect(); + assert!( + chosen.len() == *count, + "ChooseFromZoneChoice needs {count} declared object target(s) in its \ + legal set, found {} — declare more via ResolutionPolicy.targets_objects.\n \ + legal: {cards:?}\n declared: {remaining_objects:?}", + chosen.len() + ); + remaining_objects.retain(|o| !chosen.contains(o)); + act_collect( + runner, + GameAction::SelectCards { cards: chosen }, + &mut events, + )?; + } // CR 608.2c: Some resolving spell abilities choose targets during // resolution. Reuse the same slot-matching policy as cast-time // targeting so tests can declare the intended object/player once. 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/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs index 48e9c3d50c..284468c6dc 100644 --- a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs +++ b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs @@ -174,29 +174,25 @@ fn mutable_pupa_accumulates_every_matching_keyword() { // 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`, so the ETB opens a real trigger -// target-selection prompt; the cast driver carries the declared Kathril choice -// forward to that prompt and chain propagation carries it through every -// subsequent independently-gated sibling. +// to the degenerate `TargetFilter::Any`. // -// KNOWN LIMITATION (not fixed by this PR, tracked separately from #6321): the -// recipient is still chosen ONCE, at trigger-stack time, and that single -// choice is reused for every replicated keyword node via chain-target -// propagation (`resolve_chain_body`'s `sub_resolved.targets = ability.targets -// .clone()`). Per CR 608.2d, an untargeted "any creature you control" choice -// with no literal "target" wording should be re-offered independently at EACH -// instruction's own resolution — so on a board with more than one legal -// creature, a real Kathril should let the controller spread flying/trample/ -// vigilance/etc. across different creatures, not force them all onto whichever -// one was picked first. `target_choice_timing_for_clause`'s `PutCounter` guard -// (`oracle_effect/lower.rs`) only flips to per-instance `Resolution` timing for -// source-attached-host filters (Equipped/Enchanted), not general "creature you -// control" targets — extending that classification has broader blast radius -// across every other card using this target shape and is out of scope for the -// list-collapse fix this test exists to prove. This scenario deliberately -// gives P0 only ONE creature (Kathril itself), so the single-shared-choice -// gap is not observable here — the assertions below are correct FOR THIS -// BOARD but do not exercise or claim the multi-creature CR 608.2d case. +// 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."; @@ -260,3 +256,83 @@ fn kathril_reaches_matching_counter_and_tail_past_false_earlier_gates() { "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(); + + let outcome = runner + .cast(kathril) + .target_objects(&[second_recipient, kathril]) + .resolve(); + outcome.assert_zone(&[kathril], Zone::Battlefield); + + let state = outcome.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", + ); +} From ec287c11fcf32c0b6d075719903f641ba26e83e9 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Thu, 23 Jul 2026 11:25:21 -0500 Subject: [PATCH 10/18] fix(engine): pass ability by reference to should_propagate_parent_targets resolve_optional_effect_decision's ability parameter is an owned ResolvedAbility (mut ability: ResolvedAbility), not a reference like every other call site of the new should_propagate_parent_targets helper -- CI caught the resulting E0308 type mismatch that this sandbox's missing compiler couldn't. --- crates/engine/src/game/effects/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index 8818852069..6c45909d2c 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -2635,7 +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 should_propagate_parent_targets(ability, &resolved) + if should_propagate_parent_targets(&ability, &resolved) && effect_refs_parent_target(&resolved.effect) { resolved.targets = ability.targets.clone(); From b92afb81eb5f3504859773a6c21e637435dda768 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Thu, 23 Jul 2026 11:35:16 -0500 Subject: [PATCH 11/18] fix(parser): compose the "any" quantifier strip through other/another The "a"/"an"/"any" indefinite-quantifier strip in parse_type_phrase_with_ctx only checked starts_with_type_phrase_lead/starts_with_commander_word on the immediate remainder after the quantifier -- so "any other creature you control" (remainder "other creature...") never stripped "any ", never reached the "other"/"another" handler below it either (which reads from the unchanged pos), and the whole phrase fell through to the degenerate TargetFilter::Any fallback. The "all"/"each"/"every" block right below already composes through this exact "other"/"another" case via its own after_other check; the "a"/"an"/"any" block never did. Added the same after_other composition here, and a parser-level regression test (parse_type_phrase_any_other_creature_you_control) mirroring the existing "each other creature"/"all other creatures" coverage for the universal-quantifier case. --- crates/engine/src/parser/oracle_target.rs | 50 ++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index 716774bc15..d193dbd146 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -2097,10 +2097,24 @@ pub fn parse_type_phrase_with_ctx<'a>( // `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..]) { - if starts_with_type_phrase_lead(rest) || starts_with_commander_word(rest) { + 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(); } } @@ -15305,6 +15319,40 @@ 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 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 From 96b2937d4be8f1a90f6665d042fafa5f998fe74e Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Thu, 23 Jul 2026 12:08:58 -0500 Subject: [PATCH 12/18] test: regenerate Kathril snapshot fixtures for the any-quantifier parser fix CI now has a working toolchain and confirmed exactly what these two insta snapshots (kathril_aspect_warper_ir.snap, kathril_aspect_warper_lowered.snap) need: the previous fixes to parse_type_phrase_with_ctx's "any" quantifier stripping make "any creature you control" parse to a real TargetFilter::Typed{Creature, controller: You} instead of the degenerate TargetFilter::Any fallback, for all 11 replicated keyword-counter nodes. Reconstructed both files precisely: matched the exact serialization shape (type_filters/controller/properties field order and presence of an empty "properties": []) against an existing snapshot with the identical "creature you control" filter (Conclave Mentor), rather than guessing. Also removed the now-stale TargetFallback diagnostic/ parseWarnings entry each file recorded for "any creature you control" -- that diagnostic no longer fires now that the phrase parses correctly, and confirmed the omitted-when-empty field shape against the same reference snapshot. Validated both files' JSON bodies parse correctly and that no "type": "Any" occurrences remain. --- ...pshot_tests__kathril_aspect_warper_ir.snap | 87 ++++++++++++++----- ..._tests__kathril_aspect_warper_lowered.snap | 87 ++++++++++++++----- 2 files changed, 134 insertions(+), 40 deletions(-) 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 1021f80b8d..986ec5e88e 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, @@ -665,13 +720,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 fcd5b68bc7..5197d0a1e0 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, @@ -647,13 +702,5 @@ expression: "&lowered" ], "statics": [], "replacements": [], - "extractedKeywords": [], - "parseWarnings": [ - { - "type": "TargetFallback", - "context": "parse_target could not classify", - "text": "any creature you control", - "line_index": 0 - } - ] + "extractedKeywords": [] } From 09798c188af668d3ac4929b07920429a62155d19 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Thu, 23 Jul 2026 12:55:13 -0500 Subject: [PATCH 13/18] test: add target_choice_timing to Kathril snapshot fixtures CI's real toolchain surfaced the last piece: the widened target_choice_timing_for_clause (lower.rs) now correctly marks all 11 replicated keyword-counter clauses Resolution-timed (matching the CR 608.2d fix), and that field is present in the serialized AbilityDefinition when non-default. Added "target_choice_timing": "Resolution" right after "optional": false in each of the 11 keyword-counter nodes (flying through vigilance) in both snapshot files, matching insta's reported diff exactly -- excluded the P1P1 tail node (SelfRef target, correctly stays Stack-default/omitted) and the unrelated trigger-level "optional" field (a different struct). Validated both files' JSON bodies still parse correctly. --- ..._ir__snapshot_tests__kathril_aspect_warper_ir.snap | 11 +++++++++++ ...snapshot_tests__kathril_aspect_warper_lowered.snap | 11 +++++++++++ 2 files changed, 22 insertions(+) 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 986ec5e88e..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 @@ -295,6 +295,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -335,6 +336,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -375,6 +377,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -415,6 +418,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -455,6 +459,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -495,6 +500,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -535,6 +541,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -575,6 +582,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -615,6 +623,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -655,6 +664,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -695,6 +705,7 @@ expression: "&ir" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false }, "valid_card": { 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 5197d0a1e0..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 @@ -278,6 +278,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -318,6 +319,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -358,6 +360,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -398,6 +401,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -438,6 +442,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -478,6 +483,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -518,6 +524,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -558,6 +565,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -598,6 +606,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -638,6 +647,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false, "sub_link": "SequentialSibling", "sibling_condition": "ReplicatedOrBranch" @@ -678,6 +688,7 @@ expression: "&lowered" }, "optional_targeting": false, "optional": false, + "target_choice_timing": "Resolution", "forward_result": false }, "valid_card": { From 55e02f58d397b62d6f3e009baf177cc2c34b332f Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Fri, 24 Jul 2026 09:17:50 -0500 Subject: [PATCH 14/18] fix(test): default ChooseFromZoneChoice's test driver to the legal set The new drive_resolution handler for WaitingFor::ChooseFromZoneChoice hard-panicked when a test declared no objects for it, but this variant predates the Kathril work -- it's the existing CR 608.2d tracked-set choice mechanism (exiled-cards picks, etc.), and pre-existing tests using it (Portent of Calamity) never needed to declare anything, since drive_resolution previously had no handler at all for this variant and such tests must have relied on a path that didn't reach here, or on this exact prompt not existing pre-fix. Match the convention every other default-driven choice in this function already uses (ScryChoice, SurveilChoice, ArrangePlanarDeckTopChoice all auto-default rather than requiring explicit test declaration): try the declared-object pool first so a test can still pin a specific recipient (as the new Kathril discriminating test does), then fall back to the front of the legal set for anything undeclared, instead of panicking. --- crates/engine/src/game/scenario.rs | 35 ++++++++++++++++++------------ 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/game/scenario.rs b/crates/engine/src/game/scenario.rs index d83123dab0..3fda871bfc 100644 --- a/crates/engine/src/game/scenario.rs +++ b/crates/engine/src/game/scenario.rs @@ -3173,27 +3173,34 @@ fn drive_resolution( &mut events, )?; } - // CR 608.2d: an untargeted resolution-time recipient choice (e.g. a - // `PutCounter` "any creature you control" instruction — Kathril, - // Aspect Warper, issue #6321 / PR #6533). Reuses the same declared- - // object pool as `TriggerTargetSelection`/`TargetSelection` so a - // test can pin which of several legal recipients a given - // instruction picks by declaring that object first. + // CR 608.2d: an untargeted resolution-time choice from a candidate + // set — e.g. a `PutCounter` "any creature you control" instruction + // (Kathril, Aspect Warper, issue #6321 / PR #6533) as well as this + // variant's pre-existing tracked-set uses. Reuses the same + // declared-object pool as `TriggerTargetSelection`/ + // `TargetSelection` so a test can pin which of several legal + // recipients a given instruction picks by declaring that object + // first; any remainder defaults to the front of the legal set, + // mirroring `ScryChoice`/`SurveilChoice`/ + // `ArrangePlanarDeckTopChoice`'s own defaults above, so a test + // that never needed to pin this specific choice keeps working + // unchanged. WaitingFor::ChooseFromZoneChoice { cards, count, .. } => { - let chosen: Vec = remaining_objects + let mut chosen: Vec = remaining_objects .iter() .filter(|o| cards.contains(o)) .take(*count) .copied() .collect(); - assert!( - chosen.len() == *count, - "ChooseFromZoneChoice needs {count} declared object target(s) in its \ - legal set, found {} — declare more via ResolutionPolicy.targets_objects.\n \ - legal: {cards:?}\n declared: {remaining_objects:?}", - chosen.len() - ); remaining_objects.retain(|o| !chosen.contains(o)); + for &id in cards { + if chosen.len() == *count { + break; + } + if !chosen.contains(&id) { + chosen.push(id); + } + } act_collect( runner, GameAction::SelectCards { cards: chosen }, From ab3ef9656539283ff42773062fa324ceee4d5338 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Fri, 24 Jul 2026 09:37:49 -0500 Subject: [PATCH 15/18] fix(test): drive ChooseFromZoneChoice manually, don't auto-handle in drive_resolution Reverts the previous fix's approach entirely: adding an auto-driving arm for WaitingFor::ChooseFromZoneChoice to the shared drive_resolution was the wrong direction. That variant predates this PR (it's the existing CR 608.2d tracked-set choice mechanism) and Portent of Calamity's own test (issue_6498) deliberately relies on .resolve() STOPPING at the first such prompt so it can drive each per-type exile pick manually, one at a time, with its own selection logic -- exactly like the pre-existing Wick test does. Auto-driving it out from under that test silently consumed all 4 of its prompts before the test's own loop ever ran, dropping its exile count to 0. Removed the arm entirely, restoring the original `_ => break` handling for this variant. Updated the new Kathril discriminating test to match the same manual-driving convention Portent's and Wick's tests already use: call .resolve() (which now correctly stops at the first prompt), then loop over WaitingFor::ChooseFromZoneChoice states directly, answering each with the declared recipient in written order. --- crates/engine/src/game/scenario.rs | 34 ------------------- .../mutable_pupa_perpetual_keyword_mirror.rs | 34 +++++++++++++++---- 2 files changed, 28 insertions(+), 40 deletions(-) diff --git a/crates/engine/src/game/scenario.rs b/crates/engine/src/game/scenario.rs index 3fda871bfc..be8414ab0b 100644 --- a/crates/engine/src/game/scenario.rs +++ b/crates/engine/src/game/scenario.rs @@ -3173,40 +3173,6 @@ fn drive_resolution( &mut events, )?; } - // CR 608.2d: an untargeted resolution-time choice from a candidate - // set — e.g. a `PutCounter` "any creature you control" instruction - // (Kathril, Aspect Warper, issue #6321 / PR #6533) as well as this - // variant's pre-existing tracked-set uses. Reuses the same - // declared-object pool as `TriggerTargetSelection`/ - // `TargetSelection` so a test can pin which of several legal - // recipients a given instruction picks by declaring that object - // first; any remainder defaults to the front of the legal set, - // mirroring `ScryChoice`/`SurveilChoice`/ - // `ArrangePlanarDeckTopChoice`'s own defaults above, so a test - // that never needed to pin this specific choice keeps working - // unchanged. - WaitingFor::ChooseFromZoneChoice { cards, count, .. } => { - let mut chosen: Vec = remaining_objects - .iter() - .filter(|o| cards.contains(o)) - .take(*count) - .copied() - .collect(); - remaining_objects.retain(|o| !chosen.contains(o)); - for &id in cards { - if chosen.len() == *count { - break; - } - if !chosen.contains(&id) { - chosen.push(id); - } - } - act_collect( - runner, - GameAction::SelectCards { cards: chosen }, - &mut events, - )?; - } // CR 608.2c: Some resolving spell abilities choose targets during // resolution. Reuse the same slot-matching policy as cast-time // targeting so tests can declare the intended object/player once. diff --git a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs index 284468c6dc..f69bde6775 100644 --- a/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs +++ b/crates/engine/tests/integration/mutable_pupa_perpetual_keyword_mirror.rs @@ -19,7 +19,9 @@ 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}; @@ -297,13 +299,33 @@ fn kathril_offers_each_matching_counter_its_own_independent_recipient() { ); let mut runner = scenario.build(); - let outcome = runner - .cast(kathril) - .target_objects(&[second_recipient, kathril]) - .resolve(); - outcome.assert_zone(&[kathril], Zone::Battlefield); + // `.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 = outcome.state(); + 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()); From 6b2d818cee31152963f51d54178bcc2c475a23f0 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Fri, 24 Jul 2026 12:04:17 -0500 Subject: [PATCH 16/18] fix(parser): chain a second consecutive subtype word instead of dropping it Reconciling the coverage-parse-diff blast radius from the "any" quantifier widening surfaced a real, pre-existing gap: the CR 205.3a "[Subtype] [CoreType]" promotion (e.g. "Wizard creatures") only ever consumed a SECOND word when it was a concrete core type. A second consecutive SUBTYPE word (e.g. "Elder Dragon", "Elf Warrior", "Human Wizard" -- two independently-registered subtypes stacked on one permanent, not a compound word) was silently dropped, since the `type_filters` builder discards it (the caller doesn't require an empty remainder). Fate Reforged chapter II ("a copy of any Elder Dragon from the Legends expansion") is the concrete case the parse-diff surfaced: before the "any" fix this collapsed all the way to TargetFilter::Any (matching literally anything); after it, "any " correctly reaches the subtype parser, which then only captured Subtype("Elder") and dropped "Dragon" -- an over-broad filter matching any Elder-subtype creature, not specifically Elder Dragons. Chained the second subtype word into the same slot the first arm already threads through, matching the Legends-era Elder Dragon type line (Nicol Bolas, Palladia-Mors, etc.) correctly. This is a general fix, not a Fate-Reforged special case -- it benefits any card using a two-subtype phrase, most of which never appeared in this PR's diff because they were broken the same way before the "any" fix too, just silently. Card-level reconciliation for the remaining coverage-parse-diff entries (issue #6321 / PR #6533 review): - Naming Screen: "doesn't share a name with any other creature you control" -- parse_shared_quality_reference explicitly rejects a TargetFilter::Any reference population, so before the "any"+"other" composition fix this whole relative clause failed to parse and the static ability fell through to an unstructured fallback. Proven via parse_shared_quality_clause_naming_screen_reference using the card's verbatim clause. - Duplication Device: "becomes a copy of any creature on the battlefield" -- Effect::BecomeCopy's target is parsed via the same shared parse_target this whole fix touches (oracle_effect/subject.rs); "any creature on the battlefield" now correctly reaches the pre-existing, unmodified zone-suffix machinery instead of collapsing to Any. Proven via parse_type_phrase_any_creature_on_the_battlefield. - Kathril, Mutable Pupa: already covered by this PR's existing integration tests. --- crates/engine/src/parser/oracle_target.rs | 133 ++++++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index d193dbd146..f7443d8442 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -2533,6 +2533,21 @@ pub fn parse_type_phrase_with_ctx<'a>( _ => unreachable!(), }; (Some(tf), Some(sub_name)) + } else if let TypeFilter::Subtype(second) = tf { + // CR 205.3a: two consecutive subtype words ("Elder Dragon", + // "Elf Warrior", "Human Wizard") are BOTH real subtypes + // stacked on the same permanent, not one compound word — + // 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) } @@ -15353,6 +15368,124 @@ mod tests { assert_eq!(tf.controller, Some(ControllerRef::You)); } + /// CR 205.3a: two consecutive subtype words ("Elder Dragon", "Elf + /// Warrior", "Human Wizard") are BOTH real, independently-registered + /// subtypes stacked on the same permanent (`oracle-subtypes.json` lists + /// "Elder" and "Dragon" as separate entries) — not one compound word, and + /// not a `[Subtype] [CoreType]` promotion (CR 205.3a's 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 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 From 7c48b740d8c8959dac7b2a1988457e13d03184de Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Fri, 24 Jul 2026 14:41:48 -0500 Subject: [PATCH 17/18] fix(parser): exclude the "Urza's" possessive fragment from subtype chaining The two-consecutive-subtype-word chain added for Fate Reforged's "any Elder Dragon" regressed the dedicated Urza-lands condition parser: "Urza's" (LAND_SUBTYPES, card_type.rs) is the one possessive-suffixed entry in the whole subtype vocabulary -- a name fragment meant to attach to a following noun ("Urza's Mine"/"Tower"/"Power-Plant"), not an independently AND-combinable subtype like "Elder"/"Elf"/"Human". Chaining it fully consumed "an urza's mine" into one Typed{Subtype("Urza's"), Subtype("Mine")} filter with an empty remainder, which changed which downstream condition-builder claimed the clause and broke urzas_lands_share_delta_shape (expected ControllerControlsMatching{filter} with get_subtype() == "Mine", got a QuantityCheck over the two-subtype AND filter instead) and legacy_misparses_are_now_honest_gaps (expected None -- an intentionally undocumented gap -- got a confidently wrong QuantityComparison). Guarded the chain on the first subtype word not ending in "'s" (verified "Urza's" is the only such entry across LAND_SUBTYPES, ARTIFACT_SUBTYPES, ENCHANTMENT_SUBTYPES, SPELL_SUBTYPES, BATTLE_SUBTYPES, PLANESWALKER_SUBTYPES, and the MTGJSON-derived creature vocabulary), restoring "urza's mine" to its original Subtype("Urza's") + unconsumed "mine" remainder so the specialized handler still sees it, while keeping the chain for genuine two-word subtype pairs. --- crates/engine/src/parser/oracle_target.rs | 80 +++++++++++++++++++---- 1 file changed, 66 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index f7443d8442..41d69a2975 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -2534,20 +2534,40 @@ pub fn parse_type_phrase_with_ctx<'a>( }; (Some(tf), Some(sub_name)) } else if let TypeFilter::Subtype(second) = tf { - // CR 205.3a: two consecutive subtype words ("Elder Dragon", - // "Elf Warrior", "Human Wizard") are BOTH real subtypes - // stacked on the same permanent, not one compound word — - // 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)) + if matches!(&card_type, Some(TypeFilter::Subtype(s)) if s.ends_with("'s")) { + // "Urza's" (the ONLY possessive-suffixed entry in the + // whole subtype vocabulary — LAND_SUBTYPES, + // card_type.rs) is a name FRAGMENT meant to attach to + // a following noun ("Urza's Mine"/"Tower"/ + // "Power-Plant"), not an independently AND-combinable + // subtype like "Elder"/"Elf"/"Human" below. Chaining + // it here would fully consume "an 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 (issue #6321 / + // PR #6533 review — urzas_lands_share_delta_shape / + // legacy_misparses_are_now_honest_gaps). Decline; the + // specialized handler still gets the untouched text. + (card_type, subtype) + } else { + // CR 205.3a: two consecutive subtype words ("Elder + // Dragon", "Elf Warrior", "Human Wizard") are BOTH + // real subtypes stacked on the same permanent, not + // one compound word — 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) } @@ -15407,6 +15427,38 @@ mod tests { } } + /// CR 205.3i: "Urza's" (LAND_SUBTYPES, `card_type.rs`) is the ONE + /// possessive-suffixed entry in the whole subtype vocabulary — a name + /// FRAGMENT meant to attach to a following noun ("Urza's Mine"/"Tower"/ + /// "Power-Plant"), not an independently AND-combinable subtype like + /// "Elder"/"Elf"/"Human" above. The two-word subtype chain must NOT fire + /// for it: chaining 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). "mine" must stay + /// unconsumed in the remainder so the 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 From 50d1de7e35f2a0504f02dc97da09651044136047 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Fri, 24 Jul 2026 14:52:36 -0500 Subject: [PATCH 18/18] fix(parser): scope consecutive-subtype chaining to creature types only Redesigns the previous "exclude Urza's specifically" patch into a principled, CR-grounded rule instead of an ad-hoc exception. CR 205.3b: subtypes of every card type except creature (and plane) are always single words on a printed type line -- each dash-separated word is its own subtype. Creature subtypes are the one category the rules let run one OR two words (CR 205.3m: the sole two-word creature type is "Time Lord"; every other listed type, including "Elder"/"Dragon"/ "Elf"/"Warrior"/"Human"/"Wizard", is one word). So when Oracle text names two consecutive creature-subtype words, that is genuinely ambiguous -- one two-word type, or two one-word types stacked -- in a way that never arises for other categories, where CR 205.3b already guarantees each word is separate. This generic chain exists to resolve exactly that creature-only ambiguity, so it is now scoped to fire ONLY when neither matched word is a registered noncreature subtype (fixed_noncreature_subtypes -- land/artifact/enchantment/spell/battle/planeswalker), replacing the narrower "does the first word end in 's" check. "Urza's" is a real land type (CR 205.3i) -- land subtypes genuinely CAN co-occur on one permanent (Urza's Mine has both the "Urza's" and "Mine" land subtypes) -- but the dedicated Urza-lands ControllerControlsMatching condition parser already owns that Oracle-text pattern and deliberately extracts only the discriminating second word, so this generic rule must stay out of its way, and out of every other noncreature category's way too, not just this one land cycle. Also corrects the CR citation on the chaining rule itself from 205.3a (which only establishes that a card may have subtypes at all) to 205.3b + 205.3m (which establish the actual one-vs-two-word creature distinction the rule implements) -- 205.3a stays correctly cited on the unrelated pre-existing "[Subtype] [CoreType]" promotion arm beside it, which is a different pattern. --- crates/engine/src/parser/oracle_target.rs | 117 ++++++++++++++-------- 1 file changed, 77 insertions(+), 40 deletions(-) diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index 41d69a2975..2d404c106b 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -2534,29 +2534,60 @@ pub fn parse_type_phrase_with_ctx<'a>( }; (Some(tf), Some(sub_name)) } else if let TypeFilter::Subtype(second) = tf { - if matches!(&card_type, Some(TypeFilter::Subtype(s)) if s.ends_with("'s")) { - // "Urza's" (the ONLY possessive-suffixed entry in the - // whole subtype vocabulary — LAND_SUBTYPES, - // card_type.rs) is a name FRAGMENT meant to attach to - // a following noun ("Urza's Mine"/"Tower"/ - // "Power-Plant"), not an independently AND-combinable - // subtype like "Elder"/"Elf"/"Human" below. Chaining - // it here would fully consume "an 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 (issue #6321 / - // PR #6533 review — urzas_lands_share_delta_shape / - // legacy_misparses_are_now_honest_gaps). Decline; the - // specialized handler still gets the untouched text. + // 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 { - // CR 205.3a: two consecutive subtype words ("Elder - // Dragon", "Elf Warrior", "Human Wizard") are BOTH - // real subtypes stacked on the same permanent, not - // one compound word — chain the second as an - // additional AND-combined type filter instead of + // 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 @@ -15388,17 +15419,19 @@ mod tests { assert_eq!(tf.controller, Some(ControllerRef::You)); } - /// CR 205.3a: two consecutive subtype words ("Elder Dragon", "Elf - /// Warrior", "Human Wizard") are BOTH real, independently-registered - /// subtypes stacked on the same permanent (`oracle-subtypes.json` lists - /// "Elder" and "Dragon" as separate entries) — not one compound word, and - /// not a `[Subtype] [CoreType]` promotion (CR 205.3a's 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). + /// 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 [ @@ -15427,19 +15460,23 @@ mod tests { } } - /// CR 205.3i: "Urza's" (LAND_SUBTYPES, `card_type.rs`) is the ONE - /// possessive-suffixed entry in the whole subtype vocabulary — a name - /// FRAGMENT meant to attach to a following noun ("Urza's Mine"/"Tower"/ - /// "Power-Plant"), not an independently AND-combinable subtype like - /// "Elder"/"Elf"/"Human" above. The two-word subtype chain must NOT fire - /// for it: chaining would fully consume "urza's mine" into one + /// 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). "mine" must stay - /// unconsumed in the remainder so the specialized handler still sees it. + /// 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");