diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index 41376951c8..5ec96bf62d 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -1910,6 +1910,14 @@ fn legacy_ability_condition(x: &AbilityCondition) -> bool { | AbilityCondition::ControllerControlledMatchingAsCast { .. } | AbilityCondition::SourceLacksKeyword { .. } | AbilityCondition::WasStartingPlayer { .. } + // CR 903.3d: `false` here is about the D5 LEGACY-BATCH-PROMPT axis only — + // "does this leaf carry one of the 12 retained event-context refs" — not + // about reads. It carries none, exactly like the board-reading + // `ControllerControlsMatching` above and the `TriggerCondition` / + // `StaticCondition` mirrors below. The commander gate's actual read + // (a live battlefield census, CR 903.3d) is classified in + // `rw_ability_condition` via `commander_control_read`. + | AbilityCondition::ControlsCommander { .. } | AbilityCondition::AdditionalCostPaidInstead | AbilityCondition::AlternativeManaCostPaid | AbilityCondition::EffectOutcome { .. } @@ -3621,6 +3629,46 @@ fn board_membership_read(filter: &TargetFilter) -> RwProfile { p } +/// CR 903.3d + CR 603.3b: the single read profile for a commander-control gate, +/// shared by ALL THREE condition-vocabulary mirrors (`AbilityCondition`, +/// `TriggerCondition`, `StaticCondition`). +/// +/// CR 903.3d: "If an effect refers to controlling a commander, it refers to a +/// permanent on the battlefield that is a commander." That is a LIVE BOARD +/// CENSUS, not a static card attribute: `game::commander::controls_own_commander` +/// / `controls_any_commander` scan `state.battlefield` for +/// `is_commander && controller == you [&& owner == you] && is_phased_in`. Only +/// the `is_commander` and `owner` conjuncts are frozen card attributes (CR 903.3); +/// battlefield MEMBERSHIP, the CONTROLLER field and CR 702.26b phased-in status +/// are all sibling-mutable, and `StateKind::SetMembership` is exactly "which +/// objects are where / whose" plus the kind a `PhaseOut` write records. +/// +/// So this must NOT be `RwProfile::empty()`: a sibling copy whose effect moves a +/// commander onto or off the battlefield, steals it, or phases it out FEEDS this +/// gate's truth, and `group_is_order_independent` would otherwise auto-order the +/// pair against a gate that write flips (CR 603.3b). Modeled exactly like the +/// direct analogue `AbilityCondition::ControllerControlsMatching` — +/// `board_membership_read` over the population CR 903.3d names, expressed in the +/// engine's own filter vocabulary so the census/zone/controller spans are derived +/// by the one authority rather than hand-assembled. +/// +/// `ControllerRef::You` for both ownership arms: CR 109.5 "you" is the evaluating +/// player, and `Own` merely adds the frozen owner conjunct on top of the same +/// controller-keyed census (a strictly narrower population, so the `You` span +/// stays sound). +fn commander_control_read() -> RwProfile { + board_membership_read(&TargetFilter::Typed( + TypedFilter::permanent() + .controller(ControllerRef::You) + .properties(vec![ + FilterProp::IsCommander, + FilterProp::InZone { + zone: Zone::Battlefield, + }, + ]), + )) +} + /// §L2 (CR 400.7 + CR 400.1): a zone-change per-turn journal read, keyed to its /// DESTINATION zone when known. A `None` destination stays fail-closed (`Any` /// zones, via `board_membership_read`). Never overrides a self-scoped read (whose @@ -6199,6 +6247,15 @@ fn rw_ability_condition(x: &AbilityCondition) -> RwProfile { } p } + // CR 903.3d: a LIVE battlefield census — see `commander_control_read`. + // The three condition-vocabulary mirrors of this ONE printed clause share + // that helper, so none of them can drift from the others. + // M3 binding mandate: `commander_control_read()` is a PRECISE profile, not + // `RwProfile::conservative()`, so this arm binds every payload field. + // `ownership` is deliberately not read: CR 903.3d makes both arms the same + // live battlefield census, and the Own/Any distinction narrows WHICH + // commanders qualify, not which state the read touches. + AbilityCondition::ControlsCommander { ownership: _ } => commander_control_read(), AbilityCondition::AdditionalCostPaidInstead | AbilityCondition::AlternativeManaCostPaid | AbilityCondition::EffectOutcome { .. } @@ -6339,10 +6396,14 @@ fn rw_trigger_condition(x: &TriggerCondition) -> RwProfile { | TriggerCondition::TributeNotPaid | TriggerCondition::CastDuringPhase { .. } | TriggerCondition::CastTimingPermission { .. } - | TriggerCondition::ControlsCommander { .. } | TriggerCondition::ChosenLabelIs { .. } | TriggerCondition::ExceptFirstDrawInDrawStep | TriggerCondition::PlacedByAbilitySource => RwProfile::empty(), + // CR 903.3d: a LIVE battlefield census — see `commander_control_read`. + // Shared with the `AbilityCondition` / `StaticCondition` mirrors of the + // same printed clause. + // M3 binding mandate: precise RHS, so bind every payload field. + TriggerCondition::ControlsCommander { ownership: _ } => commander_control_read(), } } @@ -6439,12 +6500,16 @@ fn rw_static_condition(x: &StaticCondition) -> RwProfile { | StaticCondition::WasCast { .. } | StaticCondition::IsRingBearer | StaticCondition::RingLevelAtLeast { .. } - | StaticCondition::ControlsCommander { .. } | StaticCondition::SourceControllerEquals { .. } | StaticCondition::EnchantedIsFaceDown | StaticCondition::AdditionalCostPaid | StaticCondition::CastingAsVariant { .. } | StaticCondition::None => RwProfile::empty(), + // CR 903.3d: a LIVE battlefield census — see `commander_control_read`. + // Shared with the `AbilityCondition` / `TriggerCondition` mirrors of the + // same printed clause. + // M3 binding mandate: precise RHS, so bind every payload field. + StaticCondition::ControlsCommander { ownership: _ } => commander_control_read(), } } diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index d37fe1366d..6818479450 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -2705,6 +2705,21 @@ fn scan_ability_condition(x: &AbilityCondition, mode: ScanMode) -> Axes { } AbilityCondition::HasMaxSpeed => Axes::NONE, AbilityCondition::IsMonarch => Axes::NONE, + // CR 903.3d: "controlling a commander" is a permanent ON THE BATTLEFIELD + // that is a commander — a live board census (`game::commander` scans + // `state.battlefield` for `is_commander && controller == you [&& owner == + // you] && is_phased_in`), so a sibling copy that moves, steals or phases a + // commander can flip this gate (CR 603.3b ordering-relevance). Self-asserts + // its own `sibling: true` literal, as the ⛔ INVARIANT on + // `scan_target_filter`'s `Typed` arm requires of every board-aggregate + // caller. `event` stays false: the census reads no triggering-event + // characteristic. `ownership: _` is destructured explicitly (as the + // `TriggerCondition` mirror does) so a future field forces a re-audit here. + AbilityCondition::ControlsCommander { ownership: _ } => Axes { + event: false, + sibling: true, + projected: false, + }, // CR 309.7: controller-state predicate — touches no scan axis. AbilityCondition::CompletedDungeon { .. } => Axes::NONE, AbilityCondition::IsInitiative => Axes::NONE, @@ -3393,7 +3408,13 @@ fn scan_trigger_condition(x: &TriggerCondition, mode: ScanMode) -> Axes { sibling: true, projected: false, }, - TriggerCondition::ControlsCommander { ownership: _ } => Axes::NONE, + // CR 903.3d: live battlefield census — same self-asserted board read as the + // `AbilityCondition` / `StaticCondition` mirrors of this printed clause. + TriggerCondition::ControlsCommander { ownership: _ } => Axes { + event: false, + sibling: true, + projected: false, + }, TriggerCondition::IsRenowned { subject: _ } => Axes::NONE, TriggerCondition::HasCounters { .. } => Axes { event: false, @@ -3667,7 +3688,13 @@ fn scan_static_condition(x: &StaticCondition, mode: ScanMode) -> Axes { StaticCondition::WasCast { zone: _ } => Axes::NONE, StaticCondition::IsRingBearer => Axes::NONE, StaticCondition::RingLevelAtLeast { level: _ } => Axes::NONE, - StaticCondition::ControlsCommander { ownership: _ } => Axes::NONE, + // CR 903.3d: live battlefield census — same self-asserted board read as the + // `AbilityCondition` / `TriggerCondition` mirrors of this printed clause. + StaticCondition::ControlsCommander { ownership: _ } => Axes { + event: false, + sibling: true, + projected: false, + }, StaticCondition::SourceIsTapped => Axes::NONE, StaticCondition::IsTapped { scope, .. } => { let mut acc = Axes::NONE; diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 7fc8d4c867..f92c052a25 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -18,9 +18,9 @@ use crate::parser::oracle_util::SELF_REF_TYPE_PHRASES; use crate::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, ActivationRestriction, AdditionalCost, AggregateFunction, AttackScope, AttackSubject, CardTypeSetSource, ChoiceType, - CoinFlipResult, Comparator, ContinuousModification, ControllerRef, CountScope, - CounterSourceRider, DelayedTriggerCondition, DieRollModifier, DoublePTMode, Duration, - EachDamageRecipient, Effect, EffectOutcomeSignal, EffectScope, FilterProp, + CoinFlipResult, CommanderOwnership, Comparator, ContinuousModification, ControllerRef, + CountScope, CounterSourceRider, DelayedTriggerCondition, DieRollModifier, DoublePTMode, + Duration, EachDamageRecipient, Effect, EffectOutcomeSignal, EffectScope, FilterProp, ForEachCategoryAction, GameRestriction, LibraryPosition, ManaProduction, ObjectProperty, ObjectScope, PerpetualModification, PlayerFilter, PlayerScope, PtStat, PtValue, PtValueScope, QuantityExpr, QuantityRef, ReplacementCondition, ReplacementDefinition, ReplacementMode, @@ -3958,6 +3958,23 @@ fn fmt_comparator(c: &Comparator) -> &'static str { } } +/// CR 903.3 vs CR 903.3d: the single label authority for a commander-control +/// gate, shared by ALL FOUR condition-vocabulary formatters +/// (`AbilityCondition`, `TriggerCondition`, `StaticCondition`, and any future +/// mirror). +/// +/// The two arms are DIFFERENT predicates — CR 903.3 + CR 109.5 "your commander" +/// is owned AND controlled, CR 903.3d "a commander" is controlled by any owner — +/// so they must never share a label. Collapsing them prints a strictly weaker +/// predicate than the card, and the parse-details / Alt-hover overlay is what +/// bug triage reads. Centralized here so one mirror cannot drift from another. +fn fmt_commander_ownership(ownership: &CommanderOwnership) -> &'static str { + match ownership { + CommanderOwnership::Own => "you control your commander", + CommanderOwnership::Any => "you control a commander", + } +} + /// Format an `AbilityCondition` as a human-readable string for the parse-details overlay. fn fmt_ability_condition(cond: &AbilityCondition) -> String { match cond { @@ -4022,6 +4039,9 @@ fn fmt_ability_condition(cond: &AbilityCondition) -> String { ), AbilityCondition::HasMaxSpeed => "has max speed".into(), AbilityCondition::IsMonarch => "is monarch".into(), + AbilityCondition::ControlsCommander { ownership } => { + fmt_commander_ownership(ownership).into() + } AbilityCondition::CompletedDungeon { specific } => match specific { None => "you've completed a dungeon".into(), Some(dungeon) => format!("you've completed {dungeon}"), @@ -4201,7 +4221,7 @@ fn fmt_trigger_condition(cond: &crate::types::ability::TriggerCondition) -> Stri } TC::ManaSpentCondition { .. } => "mana spent condition".into(), TC::HadCounters { .. } => "had counters".into(), - TC::ControlsCommander { .. } => "you control a commander".into(), + TC::ControlsCommander { ownership } => fmt_commander_ownership(ownership).into(), TC::IsRenowned { .. } => "is renowned".into(), TC::HasCounters { minimum, maximum, .. @@ -4388,7 +4408,7 @@ fn fmt_static_condition(cond: &StaticCondition) -> String { }, SC::IsRingBearer => "is the ring-bearer".into(), SC::RingLevelAtLeast { level } => format!("ring level ≥ {level}"), - SC::ControlsCommander { .. } => "you control a commander".into(), + SC::ControlsCommander { ownership } => fmt_commander_ownership(ownership).into(), SC::SourceIsTapped => "source is tapped".into(), SC::IsTapped { .. } => "is tapped".into(), SC::SourceIsSaddled => "source is saddled".into(), @@ -7899,6 +7919,8 @@ fn condition_feature(cond: &AbilityCondition) -> (&'static str, FeatureSupport) AbilityCondition::ManaColorSpent { .. } => ("ManaColorSpent", Handled), AbilityCondition::HasMaxSpeed => ("HasMaxSpeed", Handled), AbilityCondition::IsMonarch => ("IsMonarch", Handled), + // CR 903.3d: evaluated at resolution via `game::commander`. + AbilityCondition::ControlsCommander { .. } => ("ControlsCommander", Handled), // CR 309.7: evaluated at resolution via `dungeon::has_completed_dungeon`. AbilityCondition::CompletedDungeon { .. } => ("CompletedDungeon", Handled), AbilityCondition::IsInitiative => ("IsInitiative", Handled), @@ -8337,6 +8359,27 @@ fn static_condition_feature(cond: &StaticCondition) -> (&'static str, FeatureSup StaticCondition::AnyPlayerAttackedYouLastTurn => ("AnyPlayerAttackedYouLastTurn", Handled), StaticCondition::OpponentPoisonAtLeast { .. } => ("OpponentPoisonAtLeast", Unhandled), StaticCondition::UnlessPay { .. } => ("UnlessPay", Handled), + // CR 903.3d: the RUNTIME does evaluate this static + // (`layers::evaluate_static_condition`, layers.rs:1875, delegating both + // ownership arms to the single `game::commander` authority), so the + // `Unhandled` tag below understates the resolver. + // + // It stays `Unhandled` DELIBERATELY, and must not be flipped as a rider on + // an unrelated change: the tag is currently the only thing holding two + // demonstrably MISPARSED Lieutenant cards out of the supported set. Of the + // seven `ControlsCommander` statics in the pool, Convergence of Dominion + // parses to a static with `modifications: []` (a no-op continuous effect) + // and Thunderfoot Baloth collapses "this creature gets +2/+2 and other + // creatures you control get +2/+2 and have trample" into ONE `SelfRef` + // static, dropping the "other creatures you control" clause and granting + // trample to the Baloth itself. Flipping the tag alone would advertise both + // as `supported: true, gap_count: 0`. + // + // Land the flip in its own change, AFTER an empty-`modifications` static + // and a dropped continuous-modification clause each register as real gaps. + // Nothing in the commander-gate work depends on this tag — Fight for the + // Throne's intervening-`if` is an `AbilityCondition`, classified `Handled` + // in `condition_feature` above. StaticCondition::ControlsCommander { .. } => ("ControlsCommander", Unhandled), // SourceIsEquipped resolved by layers::evaluate_condition (layers.rs:1057) StaticCondition::SourceIsEquipped => ("SourceIsEquipped", Handled), @@ -12287,6 +12330,128 @@ mod tests { assert!(missing.is_empty()); } + /// CR 903.3d: the Lieutenant STATIC's `Unhandled` coverage tag is a + /// deliberate mask, not an oversight — this pins both halves so the flip + /// cannot be smuggled in as a rider on an unrelated change. + /// + /// The runtime DOES evaluate `StaticCondition::ControlsCommander` + /// (`layers::evaluate_static_condition`, layers.rs:1875), so on the + /// resolver axis alone the tag understates the engine. But the tag is also + /// the only thing keeping two demonstrably misparsed Lieutenant cards out + /// of the supported set, so it must stay until those misparses register as + /// real gaps. + /// + /// The second assertion is the evidence, on verbatim Oracle text: Thunderfoot + /// Baloth's "…this creature gets +2/+2 AND OTHER CREATURES YOU CONTROL get + /// +2/+2 and have trample" collapses into a single `SelfRef` static, so the + /// other-creatures clause is silently dropped and trample lands on the Baloth + /// itself — with no gap recorded anywhere. + /// + /// FLIP PROTOCOL: when the dropped-clause misparse (and Convergence of + /// Dominion's empty-`modifications` no-op static) each register as a gap, the + /// second assertion here goes red. THAT is the signal to flip the tag to + /// `Handled` and delete this test — not before. + #[test] + fn lieutenant_commander_static_tag_is_a_deliberate_mask() { + let (label, support) = static_condition_feature(&StaticCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }); + assert_eq!(label, "ControlsCommander"); + assert!( + matches!(support, FeatureSupport::Unhandled), + "the mask must stay until the two misparses register as gaps; see the \ + FLIP PROTOCOL on this test" + ); + + let parsed = crate::parser::parse_oracle_text( + "Trample\nLieutenant — As long as you control your commander, this creature gets \ + +2/+2 and other creatures you control get +2/+2 and have trample.", + "Thunderfoot Baloth", + &[], + &["Creature".to_string()], + &["Beast".to_string()], + ); + // Reach-guard: the fixture only exercises the misparse if the parser + // really produced the OWNER-scoped commander gate. + let commander_statics: Vec<_> = parsed + .statics + .iter() + .filter(|s| { + matches!( + &s.condition, + Some(StaticCondition::ControlsCommander { + ownership: CommanderOwnership::Own + }) + ) + }) + .collect(); + assert!( + !commander_statics.is_empty(), + "the Lieutenant line must parse to an Own-scoped ControlsCommander static: {:#?}", + parsed.statics + ); + assert!( + commander_statics + .iter() + .all(|s| matches!(s.affected, Some(TargetFilter::SelfRef))), + "MISPARSE STILL PRESENT (expected): the \"other creatures you control\" clause \ + is dropped and the whole Lieutenant grant lands on SelfRef. When this goes \ + red the parser was fixed — flip `static_condition_feature` to Handled and \ + delete this test. Got {commander_statics:#?}" + ); + } + + /// CR 903.3 vs CR 903.3d: the parse-details label is what bug triage reads, + /// so the two ownership arms must never print the same string — in ANY of + /// the condition-vocabulary formatters. + #[test] + fn commander_ownership_labels_differ_in_every_formatter() { + for (ability_label, trigger_label, static_label) in [ + ( + fmt_ability_condition(&AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }), + fmt_trigger_condition( + &crate::types::ability::TriggerCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }, + ), + fmt_static_condition(&StaticCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }), + ), + ( + fmt_ability_condition(&AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Any, + }), + fmt_trigger_condition( + &crate::types::ability::TriggerCondition::ControlsCommander { + ownership: CommanderOwnership::Any, + }, + ), + fmt_static_condition(&StaticCondition::ControlsCommander { + ownership: CommanderOwnership::Any, + }), + ), + ] { + assert_eq!( + ability_label, trigger_label, + "the three mirrors of ONE printed clause must render identically" + ); + assert_eq!(ability_label, static_label); + } + assert_ne!( + fmt_static_condition(&StaticCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }), + fmt_static_condition(&StaticCondition::ControlsCommander { + ownership: CommanderOwnership::Any, + }), + "CR 903.3 \"your commander\" is strictly narrower than CR 903.3d \"a \ + commander\"; collapsing them prints a weaker predicate than the card" + ); + } + #[test] fn vanilla_object_has_no_unimplemented_mechanics() { let obj = make_obj(); diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index 20521d54e3..85ff03e2a8 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -10,11 +10,11 @@ use crate::game::filter; use crate::game::speed::has_max_speed; use crate::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, CardPlayMode, CardTypeSetSource, - ChosenAttribute, ControllerRef, CopyRetargetPermission, CostPaidObjectSnapshot, - EachDamageRecipient, Effect, EffectError, EffectKind, EffectOutcomeSignal, - EffectResolutionResult, EffectScope, FilterProp, ManaProduction, OpponentMayScope, - PlayerFilter, PlayerScope, QuantityExpr, QuantityRef, RepeatContinuation, ResolvedAbility, - RevealUntilDisposition, SacrificeCost, SacrificeRequirement, SharedQuality, + ChosenAttribute, CommanderOwnership, ControllerRef, CopyRetargetPermission, + CostPaidObjectSnapshot, EachDamageRecipient, Effect, EffectError, EffectKind, + EffectOutcomeSignal, EffectResolutionResult, EffectScope, FilterProp, ManaProduction, + OpponentMayScope, PlayerFilter, PlayerScope, QuantityExpr, QuantityRef, RepeatContinuation, + ResolvedAbility, RevealUntilDisposition, SacrificeCost, SacrificeRequirement, SharedQuality, SharedQualityRelation, SiblingCondition, SubAbilityLink, TapStateChange, TargetChoiceTiming, TargetFilter, TargetRef, ThisWayCause, }; @@ -2942,6 +2942,81 @@ fn condition_depends_on_effect_performed(condition: &AbilityCondition) -> bool { } } +/// CR 603.12 + CR 615.5: Whether a SUB-ability's own gate makes it resolve even +/// though its parent's condition was FALSE — the single authority for that +/// question, with two consumers. +/// +/// * `resolve_ability_chain` (below) uses it on the condition-false path to +/// decide which sub-abilities still run. +/// * `triggers::delayed_body_outlives_a_false_gate` uses it so the CR 603.4 +/// fire-time hoist of a DELAYED body's intervening-`if` declines on exactly the +/// sub shapes this resolution path would still have run. If the two predicates +/// drifted, the hoist would either delete printed work (hoisting a body whose +/// sub survives) or leave CR 603.4 unenforced (declining a body whose sub does +/// not). +/// +/// The two classes: a CR 603.12 performed/reflexive gate, whose truth is only +/// knowable at resolution and is re-evaluated on its own; and a CR 615.5 +/// INDEPENDENT per-event gate (Comeuppance's mutually-exclusive +/// creature/noncreature reflection riders), which references the event rather +/// than the parent's effect. +/// +/// NOT included, deliberately: an unconditional `SequentialSibling` (it also +/// runs on the false path, but as the next clause of the SAME gated sentence — +/// CR 603.4 says it must not happen either, which is precisely why the delayed +/// hoist must still apply to it) and `SiblingCondition::ReplicatedOrBranch` +/// (a structural marker on the sub, not a condition, so it is tested against +/// the sub itself by [`sub_outlives_false_parent_gate`], which wraps this). +pub(crate) fn condition_survives_false_parent_gate(condition: &AbilityCondition) -> bool { + condition_depends_on_effect_performed(condition) + || matches!( + condition, + AbilityCondition::PostReplacementDamageSourceMatchesFilter { .. } + ) +} + +/// CR 603.12 + CR 615.5 + CR 702.1c: whether a SUB-ability still resolves even +/// though its parent's condition was FALSE, for a reason INDEPENDENT of the +/// parent's gate — the single authority for that whole question. +/// +/// Two components, and both must stay together: +/// +/// * the sub's own gate is one of the classes +/// [`condition_survives_false_parent_gate`] names (a CR 603.12 performed / +/// reflexive gate, or a CR 615.5 independent per-event gate); +/// * the sub is a per-item keyword-list REPLICATION branch (CR 702.1c "the same +/// is true" — Kathril / Mutable Pupa). That is a structural marker on the sub +/// (`SiblingCondition::ReplicatedOrBranch`) rather than a condition, and it +/// only means "independent OR-branch" on a `SequentialSibling` link, so BOTH +/// conjuncts are part of the test. +/// +/// Two consumers, which is why this is factored out rather than spelled twice: +/// `resolve_ability_chain` (below) on its condition-false path, and +/// `triggers::delayed_body_outlives_a_false_gate`, so the CR 603.4 fire-time +/// hoist of a DELAYED body's intervening-`if` declines on EXACTLY the sub shapes +/// this resolution path still runs. When the two spellings drifted, the hoist +/// could either delete printed work (hoisting a body whose sub survives) or +/// leave CR 603.4 unenforced (declining a body whose sub does not). +/// +/// Both consumers ask the question of the DIRECT sub only. This call site never +/// looks deeper: when the direct sub does not qualify, `resolve_chain_body` +/// returns and nothing further down the chain resolves, so a grandchild's +/// `else_ability` or reflexive gate is reachable only THROUGH a qualifying direct +/// sub. `delayed_body_outlives_a_false_gate` matches that exactly (it used to +/// recurse, which declined the hoist for chains whose resolution does nothing). +/// +/// NOT included: an UNCONDITIONAL `SequentialSibling`. It also runs on the false +/// path, but as the next clause of the SAME gated sentence, so CR 603.4 says it +/// must not happen either — the delayed hoist must still apply to it. That +/// disjunct therefore stays at the `resolve_ability_chain` call site alone. +pub(crate) fn sub_outlives_false_parent_gate(sub: &ResolvedAbility) -> bool { + sub.condition + .as_ref() + .is_some_and(condition_survives_false_parent_gate) + || (sub.sibling_condition == SiblingCondition::ReplicatedOrBranch + && sub.sub_link == SubAbilityLink::SequentialSibling) +} + /// CR 603.12 + CR 608.2c: Whether a reflexive condition reads the per-resolution /// `last_zone_changed_ids` ledger ("if a [noun] was [verb]ed this way"). Unlike /// `condition_depends_on_effect_performed` (which gates on the @@ -3249,6 +3324,9 @@ fn condition_reads_filter_population( | AbilityCondition::IsInitiative | AbilityCondition::HasCityBlessing | AbilityCondition::HasEnduringStory + // CR 903.3d: a commander-designation predicate — carries neither a + // `TargetFilter` nor a `QuantityExpr`, so it reads no filter population. + | AbilityCondition::ControlsCommander { .. } | AbilityCondition::IsRingBearer | AbilityCondition::CompletedDungeon { .. } | AbilityCondition::TargetHasKeywordInstead { .. } @@ -3650,6 +3728,10 @@ fn should_resolve_subability_on_optional_decline(ability: &ResolvedAbility) -> b | AbilityCondition::IsInitiative | AbilityCondition::HasCityBlessing | AbilityCondition::HasEnduringStory + // CR 903.3d: a live game-state gate, not a decline-alternative + // selector — declining the optional effect does not pick a + // commander-control branch. Same reading as `IsMonarch` above. + | AbilityCondition::ControlsCommander { .. } | AbilityCondition::DiscardedCardMatchesFilter { .. } | AbilityCondition::IsRingBearer | AbilityCondition::TargetHasKeywordInstead { .. } @@ -9728,10 +9810,6 @@ fn resolve_chain_body( // must be evaluated on its own regardless of whether this node's // gate held. Without this, the noncreature rider never fires when // the creature rider's gate is false (and vice-versa). - let sub_has_independent_event_gate = matches!( - sub.condition.as_ref(), - Some(AbilityCondition::PostReplacementDamageSourceMatchesFilter { .. }) - ); // CR 702.1c ("the same is true") + CR 608.2c (written order): A // sub produced by per-item keyword-list replication // (`SiblingCondition::ReplicatedOrBranch`) is an INDEPENDENT @@ -9747,15 +9825,16 @@ 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 - && sub.sub_link == SubAbilityLink::SequentialSibling; - if sub - .condition - .as_ref() - .is_some_and(condition_depends_on_effect_performed) - || sub_has_independent_event_gate - || sub_is_replicated_or_branch + // + // All THREE of the above independent-sub classes are the single + // authority `sub_outlives_false_parent_gate`, which + // `triggers::delayed_body_outlives_a_false_gate` also consults so + // the CR 603.4 fire-time hoist declines on EXACTLY the sub shapes + // this resolution path still runs. Only the unconditional + // `SequentialSibling` escape below is local to this call site — + // CR 603.4 says that clause must NOT survive a false gate on a + // delayed body, so the hoist deliberately does not mirror it. + if sub_outlives_false_parent_gate(sub) || (sub.sub_link == SubAbilityLink::SequentialSibling && sub.condition.is_none()) { @@ -12622,6 +12701,29 @@ pub(crate) fn evaluate_condition( crate::game::restrictions::spell_cast_with_variant_this_turn(state, variant) } AbilityCondition::IsMonarch => eval_is_monarch(state, ability.controller), + // CR 903.3d: "If an effect refers to controlling a commander, it refers to + // a permanent on the battlefield that is a commander." CR 903.3 + CR 109.5 + // narrow the Lieutenant reading ("your commander") to a commander the + // player also OWNS, because the designation is an attribute of the card + // itself — a stolen commander is still its owner's. + // + // Delegates to the single `game::commander` authority — the same helpers + // `layers::evaluate_static_condition`, `triggers` and `restrictions` use + // for the three sibling mirrors — so `Own` cannot be silently widened to + // "any commander you control", and the CR 702.26b phased-out exclusion is + // applied once, in one place. + // + // CR 109.5 + CR 603.7d: "you" is the resolving ability's controller; for a + // delayed triggered ability that is the player who controlled the creating + // spell as it resolved, which is already stamped onto the delayed ability. + AbilityCondition::ControlsCommander { ownership } => match ownership { + CommanderOwnership::Own => { + crate::game::commander::controls_own_commander(state, ability.controller) + } + CommanderOwnership::Any => { + crate::game::commander::controls_any_commander(state, ability.controller) + } + }, // CR 309.7: dungeon completion is a controller-state predicate, evaluated // as the ability resolves. Delegates to the single truth function shared // with `TriggerCondition::CompletedDungeon` so the intervening-if reading @@ -13504,6 +13606,85 @@ mod tests { // that much damage. Before the fix the returned card was not bound as the // earlier-instruction referent (only PUBLIC-zone moves were), so the damage // resolved to 0. + /// CR 903.3 vs CR 903.3d: the `evaluate_condition` commander arm's TWO + /// ownership readings must stay distinct, and BOTH must be driven. + /// + /// `Own` (CR 903.3 + CR 109.5, the Lieutenant reading) is owned AND + /// controlled; `Any` (CR 903.3d) is controlled by any owner. The single + /// discriminating fixture is a STOLEN commander: P1 owns it, P0 controls it. + /// `Any` must be true and `Own` must be false against the same state — an + /// `Any` arm that collapsed to the `Own` helper (or vice versa) fails here + /// even though every card shipping today produces only `Own`. + /// + /// This is deliberately a unit assertion on the production resolver arm + /// rather than a card fixture: a card-data census shows Fight for the Throne + /// is the sole producer of `AbilityCondition::ControlsCommander` and it is + /// `Own`, so the `Any` arm has no reachable card to drive it yet. + #[test] + fn evaluate_condition_commander_ownership_arms_are_distinct() { + let mut state = GameState::new_two_player(42); + // CR 903.3: the designation is an attribute of the CARD, so this + // commander stays P1's however control moves. + let stolen = create_object( + &mut state, + CardId(1), + PlayerId(1), + "Opposing Commander".to_string(), + Zone::Battlefield, + ); + { + let obj = state.objects.get_mut(&stolen).expect("staged commander"); + obj.is_commander = true; + // CR 613.1b (Layer 2): control moved to P0, ownership unchanged. + obj.base_controller = Some(PlayerId(0)); + obj.controller = PlayerId(0); + } + // Non-vacuity guard: the owner/controller divergence must really exist, + // or neither assertion below discriminates anything. + let staged = state.objects.get(&stolen).expect("staged commander"); + assert_eq!( + staged.owner, + PlayerId(1), + "the fixture only isolates the owner conjunct if P1 still OWNS it" + ); + assert_eq!( + staged.controller, + PlayerId(0), + "the fixture only isolates the controller conjunct if P0 CONTROLS it" + ); + + let ability = ResolvedAbility::new( + Effect::BecomeMonarch, + Vec::new(), + ObjectId(999), + PlayerId(0), + ); + + assert!( + evaluate_condition( + &AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Any, + }, + &state, + &ability, + ), + "CR 903.3d: \"a commander\" is controller-only, so a stolen opponent's \ + commander satisfies it" + ); + assert!( + !evaluate_condition( + &AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }, + &state, + &ability, + ), + "CR 903.3 + CR 109.5: \"your commander\" also requires ownership, so the \ + stolen commander must NOT satisfy it. A true here means the Own arm was \ + widened to controls_any_commander" + ); + } + #[test] fn continuation_resume_preserves_live_delayed_trigger_firing() { let mut state = GameState::new_two_player(42); diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 6c1e9575f9..f35bdfb4c2 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -18217,6 +18217,127 @@ mod stage2_injector_tests { // shifts combine with #6958's paid-cast outcome exclusion and // #6976's conditional-branch exclusions. None creates an // `OptionalEffect` prompt. Re-pinned against the merged source. + // Current-main port: #7221's typed player-action completion seam and the + // contemporaneous upstream changes moved these three producers. Re-derived + // in the merged source, still in their named production functions. + // + // Fight for the Throne commander-gate unit (base 8035813e6): + // `:6640/:6717/:9922 ⇒ :6647/:6724/:9929`, uniform +7. LOCAL, not upstream, + // so the CI-vs-local diagnosis in the header does not apply. `git diff -U0` + // on effects/mod.rs has exactly four hunks: `@@ -12,7 +12,7 @@` (net 0 — the + // `CommanderOwnership` import reflow), `@@ -3238,0 +3239,3 @@` (+3, the + // `AbilityCondition::ControlsCommander` leaf arm in + // `condition_reads_filter_population`), `@@ -3639,0 +3643,4 @@` (+4, the same + // variant joining `should_resolve_subability_on_optional_decline`'s live-gate + // list), and `@@ -12474,0 +12482,23 @@` (+23, the `evaluate_condition` + // delegation to `game::commander`) which sits BELOW all three producers and + // therefore moves none of them. 3 + 4 = the whole +7, and predicted + // `6640+7`/`6717+7`/`9922+7` equal the observed coordinates exactly. None of + // the three arms mints a prompt — they are condition CLASSIFICATION arms plus + // one boolean predicate delegation. Identity re-established, not assumed: each + // producer at its new coordinate is sha256-identical to + // `8035813e6:effects/mod.rs` at its old one (`a8512b402f8675b7`, + // `82c6c569182ae4ed`, `c9d8e7ba3b9e29e2`) and still sits inside the enclosing + // function this row NAMES (`drive_sequential_repeated_optional_payment` ×2, + // `resolve_chain_body`). The diff instrument discriminates: the three OLD + // coordinates now hold a `PendingRepeatedOptionalPayment` field init, a + // `payment_unit` argument, and a `trigger_events` clone — none of which mints + // anything. Set preservation: the two asserts above this one ran FIRST and + // both fired GREEN on the run that caught this (total still 37, partition + // still 5/7/25), and the other two entries did not move. + // + // Fight for the Throne review-fix round (same base 8035813e6): + // `:6647/:6724/:9929 ⇒ :6680/:6757/:9964`, i.e. `+40/+40/+42` measured + // from BASE (`:6640/:6717/:9922`), not from the row above. LOCAL, not + // upstream, so the CI-vs-local diagnosis in the header does not apply. + // `git diff -U0 8035813e6` on effects/mod.rs now has eight hunks; the + // ones that move a producer are, in order: `@@ -2935,0 +2936,33 @@` + // (+33, `condition_survives_false_parent_gate` — the single authority + // the CR 603.4 delayed-hoist carve-out now shares with + // `resolve_chain_body`), `@@ -3238,0 +3272,3 @@` (+3) and + // `@@ -3639,0 +3676,4 @@` (+4) — the two condition-classification arms + // already logged above. 33 + 3 + 4 = the `+40` on the first two. The + // third takes a further `+2` from the two hunks INSIDE + // `resolve_chain_body` and above its own gate: + // `@@ -9679,4 +9719,10 @@` (+6, the twin sub-gate clauses collapsed + // into the shared `condition_survives_false_parent_gate` call) and + // `@@ -9701,5 +9747 @@` (−4, the corresponding conjunct removal); the + // remaining hunks (`@@ -12,7 +12,7 @@` net 0, the `evaluate_condition` + // delegation, and `mod tests`) are net-zero or BELOW all three. + // Predicted `6640+40`/`6717+40`/`9922+42` equal the observed + // coordinates exactly. None of the moved code mints a prompt: it is one + // boolean condition classifier plus the call sites that consume it. + // Identity re-established, not assumed: each producer at its new + // coordinate is sha256-identical to `8035813e6:effects/mod.rs` at its + // old one (`9869a19f28c791ee`, `2bc316e3aa0297f8`, `8df98486627bfe15`) + // and still sits inside the enclosing production function it always + // did — `drive_sequential_repeated_optional_payment` (6653-6687), + // `resolve_repeated_optional_payment_choice` (6695-6779) and + // `resolve_chain_body`. (The row above names the first two as + // `drive_sequential_repeated_optional_payment` ×2; re-derived here, the + // second is the `resolve_repeated_optional_payment_choice` resume arm, + // which is what the older entries called it.) The diff instrument + // discriminates: the three OLD coordinates now hold a blank line, an + // `.active_repeated_optional_payment_frame_mut()` call and a `//` + // comment, none of which mints anything. Set preservation: the two + // asserts above this one ran FIRST and both fired GREEN on the run that + // caught this (total still 37, partition still 5/7/25), and the other + // two entries did not move. + // + // Fight for the Throne review-fix round 2 (same base 8035813e6): + // `:6715/:6792/:9994`, i.e. `+75/+75/+72` measured from BASE + // (`:6640/:6717/:9922`), not from the row above. LOCAL, not upstream, so + // the CI-vs-local diagnosis in the header does not apply. `git diff -U0 + // 8035813e6` on effects/mod.rs has eight hunks; above the first two + // producers are `@@ -2935,0 +2936,68 @@` (+68 — the condition classifier + // of the row above, now also carrying the shared + // `sub_outlives_false_parent_gate` authority the CR 603.4 delayed-hoist + // carve-out and `resolve_chain_body` both call), `@@ -3238,0 +3307,3 @@` + // (+3) and `@@ -3639,0 +3711,4 @@` (+4): 68 + 3 + 4 = the `+75`. The third + // producer nets `−3` more from the two hunks INSIDE `resolve_chain_body` + // and above its own gate — `@@ -9679,4 +9753,0 @@` (−4, the twin + // `sub_survives_false_parent_gate` / `sub_is_replicated_or_branch` locals + // collapsed into the one shared call) and `@@ -9698,9 +9769,10 @@` (+1) — + // giving `+72`; the remaining hunks (`@@ -12,7 +12,7 @@` net 0, the + // `evaluate_condition` delegation, and `mod tests`) are net-zero or BELOW + // all three. Predicted `6640+75`/`6717+75`/`9922+72` equal the observed + // coordinates exactly. None of the moved code mints a prompt: it is one + // boolean sub-classification predicate plus the call site that consumes it. + // Identity re-established, not assumed: each producer at its new coordinate + // is sha256-identical to `8035813e6:effects/mod.rs` at its old one + // (`9869a19f28c791ee`, `2bc316e3aa0297f8`, `8df98486627bfe15`) and still + // sits inside the enclosing production function this row NAMES — + // `drive_sequential_repeated_optional_payment` (opens 6702), + // `resolve_repeated_optional_payment_choice` (opens 6730) and + // `resolve_chain_body`. The diff instrument discriminates: the three OLD + // coordinates now hold a `return Ok(());`, an + // `optional_cost_payments_this_resolution` binding and a + // `resolve_optional_effect_decision(` call, none of which mints anything. + // Set preservation: the two asserts above this one ran FIRST and both + // fired GREEN on the run that caught this (total still 37, partition still + // 5/7/25), and the other two entries did not move. + // Fight for the Throne, same branch, same base `8035813e6`: + // `:6715/:6792/:9994 ⇒ :6722/:6799/:10001`, a UNIFORM `+7` on all three, + // i.e. `+82/+82/+79` measured from the base. The row above measured + // `+75/+75/+72` from that same base, so the delta is `+7` and its sole + // cause is that the FIRST hunk GREW: `@@ -2935,0 +2936,68 @@ ⇒ + // @@ -2935,0 +2936,75 @@`, the `condition_survives_false_parent_gate` + // doc/authority block picking up seven more lines. No hunk was added or + // removed and none changed size: the other four are byte-for-byte the + // sizes the row above names (`+3` at `:3307 ⇒ :3314`, `+4` at + // `:3711 ⇒ :3718`, `−4` at `:9753 ⇒ :9760`, `+1` at `:9769 ⇒ :9776`) and + // merely shifted by the same `+7`, which is what makes the shift uniform + // even for the third producer (its `−4/+1` pair still nets the same `−3` + // below the other two). Predicted `6640+82`/`6717+82`/`9922+79` against + // `8035813e6` equal the observed coordinates exactly. Identity + // re-established, not assumed: each producer at its new coordinate is + // sha256-identical to `8035813e6:effects/mod.rs` at its old one + // (`7067db50922da31f`, `975791a569b1f587`, `967e35eb66a5780b` over the + // 15-line mint expression at each site). This card's own effects/mod.rs + // edits — the two `AbilityCondition::ControlsCommander` registrations at + // `:3314`/`:3718` and the `evaluate_condition` arm below all three — mint + // nothing: they are classifier list entries and one condition evaluator. + // // Wheel of Misfortune (#7266), MEASURED ON THE MERGE TREE. This row's // own header warns that a fork branch's pins are correct for the branch // and wrong for `refs/pull//merge`; both sides of this conflict were @@ -18279,9 +18400,36 @@ mod stage2_injector_tests { // added to `compute_options`' sibling classifier in this file, // which sits above all three producers. Nothing added raises a // `WaitingFor`; the census set is still exactly 5. - "game/effects/mod.rs:6656".to_string(), - "game/effects/mod.rs:6733".to_string(), - "game/effects/mod.rs:9974".to_string(), + // THIRD merge with main (this branch × `origin/main` @ 59f5a51e, which + // by now carries Wheel of Misfortune's unbounded-number round). Same rule + // as the two merges logged above, applied a third time: each side's pins + // were local-correct and BOTH are wrong for the merged tree, so the merged + // file was re-measured rather than either side taken. `origin/main` + // carried `:6656/:6733/:9974`; this branch carried `:6722/:6799/:10001`; + // the merged file measures `:6738/:6815/:10053`. + // + // The merged coordinates are PREDICTED, not merely observed, and the + // prediction is what makes this a measurement rather than a fixup: + // `main`'s pins plus this branch's own base-relative offsets — `+82/+82/+79`, + // the figure the row immediately above derives from base `8035813e6` and + // re-derives twice — give `6656+82`/`6733+82`/`9974+79` = + // `:6738`/`:6815`/`:10053`, equal to the observed coordinates exactly. + // That the branch's offsets compose additively onto main's is the evidence + // the merge introduced no new producer and displaced none: a merge that had + // gained or lost one would break the additivity, not just shift a pin. + // + // Set preservation: the assembled needle finds exactly five hits in the + // merged effects/mod.rs (`:6738`, `:6815`, `:10053`, `:14805`, `:15290`); + // the last two fall inside the `#[cfg(test)]` span opening at `:13563` and + // so are the partition's test half, leaving the same three production + // producers this row has always pinned. Total still 37, partition still + // 5/7/25. The merge added no `WaitingFor` producer on either side — main's + // contribution here is the unbounded-range arm in `compute_options`' sibling + // classifier and this branch's is the CR 603.4 delayed-hoist carve-out, both + // pure classification code. + "game/effects/mod.rs:6738".to_string(), + "game/effects/mod.rs:6815".to_string(), + "game/effects/mod.rs:10053".to_string(), // UNMOVED across the rebase, and that is itself evidence the SET did not // move: a census that had gained or lost a producer would not leave this // entry both byte-identical AND at the same coordinate. diff --git a/crates/engine/src/game/scenario.rs b/crates/engine/src/game/scenario.rs index 8a533ee379..bf85063a23 100644 --- a/crates/engine/src/game/scenario.rs +++ b/crates/engine/src/game/scenario.rs @@ -1210,6 +1210,57 @@ impl<'a> CardBuilder<'a> { // --- Special modifiers --- + /// CR 903.3: Mark this object as its owner's commander IN PLACE, without + /// moving it to the command zone (unlike `GameScenario::with_commander`, + /// which forces `Zone::Command`). + /// + /// CR 903.3d resolves "controlling a commander" against a permanent ON THE + /// BATTLEFIELD, so every "you control your/a commander" gate — Lieutenant + /// statics, Lieutenant triggers, activation restrictions, and + /// resolution-time gates alike — needs a battlefield commander to be + /// reachable at all. This is that building block. + pub fn commander(&mut self) -> &mut Self { + self.obj().is_commander = true; + self + } + + /// CR 613.1b (Layer 2: control-changing effects) + CR 109.5: put this object + /// under `player`'s control while leaving `owner` unchanged — the + /// owner/controller divergence a stolen permanent has. + /// + /// This is the only way to exercise the two conjuncts of + /// `game::commander::controls_own_commander` (owner, then controller) + /// independently, which CR 903.3 makes observable: the commander designation + /// is an attribute of the card, so a stolen commander is still its owner's. + /// + /// Sets BOTH fields on purpose: Layer 2 recomputes `obj.controller` from + /// `base_controller.unwrap_or(owner)` on every `evaluate_layers` pass, so + /// setting `controller` alone is silently reverted by the first layer pass a + /// cast pipeline runs. `controller` is set too so the state is coherent + /// before any layer pass. + /// + /// Battlefield-only: `state.battlefield` is a flat list with no per-player + /// split, so no zone-list bookkeeping is needed. Do NOT use this for + /// hand/library/graveyard objects, whose zone lists are keyed by owner — + /// the `debug_assert_eq!` below ENFORCES that precondition rather than + /// merely documenting it, so a misapplied call fails loudly at the fixture + /// that wrote it instead of desynchronizing an owner-keyed zone list and + /// surfacing somewhere unrelated. + pub fn controlled_by(&mut self, player: PlayerId) -> &mut Self { + let obj = self.obj(); + debug_assert_eq!( + obj.zone, + Zone::Battlefield, + "CardBuilder::controlled_by is battlefield-only: object {:?} is in {:?}, whose \ + zone list is keyed by OWNER, so diverging the controller would corrupt the fixture", + obj.id, + obj.zone, + ); + obj.base_controller = Some(player); + obj.controller = player; + self + } + /// Mark this creature as having summoning sickness (entered this turn). pub fn with_summoning_sickness(&mut self) -> &mut Self { let turn = self.state.turn_number; diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index be2cb143fd..a89196f8d6 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -7,10 +7,11 @@ use crate::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, AdditionalCostOrigin, BounceSelection, CardTypeSetSource, CastManaSpentMetric, ChosenAttribute, CommanderOwnership, ControllerRef, CopyRetargetPermission, DamageKindFilter, DelayedTriggerCondition, Effect, - ModalChoice, ObjectScope, OriginConstraint, PlayerFilter, PtValue, QuantityExpr, QuantityRef, - RenownSubject, ResolvedAbility, SacrificeCost, TargetFilter, TargetRef, TributeOutcome, - TriggerCondition, TriggerConstraint, TriggerDefinition, TriggerDefinitionOccurrenceRef, - TriggerDefinitionRef, TriggerEntry, TriggerGrantProducerKey, TypeFilter, TypedFilter, + FilterProp, ModalChoice, ObjectScope, OriginConstraint, PlayerFilter, PlayerScope, PtValue, + QuantityExpr, QuantityRef, RenownSubject, ResolvedAbility, SacrificeCost, StaticCondition, + TargetFilter, TargetRef, TributeOutcome, TriggerCondition, TriggerConstraint, + TriggerDefinition, TriggerDefinitionOccurrenceRef, TriggerDefinitionRef, TriggerEntry, + TriggerGrantProducerKey, TypeFilter, TypedFilter, }; #[cfg(test)] use crate::types::ability::{EffectScope, TapStateChange}; @@ -8866,16 +8867,630 @@ fn expand_multi_fire_damage_occurrences( } } +/// CR 603.4 + CR 608.2c: does this delayed body still carry work that the +/// resolution-time reading performs when the gate is FALSE — work the fire-time +/// hoist would silently DELETE? +/// +/// This is the carve-out for [`delayed_intervening_if`], and it is deliberately +/// narrower than the "any `sub_ability`" test it replaces. The two cases the old +/// blanket conflated are NOT the same: +/// +/// * `else_ability` ("…, if X, A. Otherwise, B.") is not an intervening-`if` in +/// the CR 603.4 sense at all — the ability has printed work on the false path, +/// so it MUST still trigger and resolve its else branch. Always declines. +/// * An UNCONDITIONAL `SequentialSibling` sub is the second clause of the ONE +/// gated sentence ("…, if you control your commander, draw a card and create a +/// token"). CR 603.4 gates the whole ability, so that clause must not happen +/// either — yet `resolve_ability_chain` (`effects/mod.rs`, the +/// `sub_link == SequentialSibling && condition.is_none()` escape) resolves it +/// even when the parent gate is false. Declining the hoist here is what LEAVES +/// that violation in place, so this class must hoist. +/// +/// The three sub shapes that genuinely survive a false parent gate mirror +/// `resolve_ability_chain`'s own escape hatches one-for-one — literally, by +/// calling the SAME predicate (`effects::sub_outlives_false_parent_gate`) rather +/// than restating it, so the pair cannot drift: +/// +/// 1. a reflexive / performed gate (`condition_depends_on_effect_performed`, +/// CR 603.12 — Council's Deliberation's `OptionalEffectPerformed` rider, the +/// only conditioned delayed body with a sub in the card pool today). Its truth +/// is only knowable at resolution; hoisting the parent gate is CONSERVATIVELY +/// declined so that in-pool shape keeps byte-identical behaviour; +/// 2. an INDEPENDENT per-event gate (CR 615.5 +/// `PostReplacementDamageSourceMatchesFilter`, Comeuppance's mutually-exclusive +/// riders), which resolves on its own predicate regardless of the parent's; +/// 3. a `SiblingCondition::ReplicatedOrBranch` on a `SequentialSibling` link +/// (CR 702.1c keyword-list replication, Kathril / Mutable Pupa), an +/// independent OR-branch gated on its own keyword. +/// +/// Inspects the DIRECT sub only, because that is the whole of what +/// `resolve_chain_body`'s condition-false path inspects: it resolves +/// `else_ability` if present, else the direct `sub_ability` when +/// `sub_outlives_false_parent_gate` (or the unconditional-`SequentialSibling` +/// escape) accepts it, and otherwise RETURNS — nothing deeper in the chain runs. +/// A grandchild's `else_ability` or CR 603.12 reflexive gate is therefore reached +/// only THROUGH a direct sub that already qualified, so recursing past the direct +/// sub would decline the hoist for chains whose resolution does nothing at all, +/// leaving CR 603.4's fire-time half unenforced for that shape. Pinned by +/// `two_deep_chain_mirrors_the_resolvers_direct_sub_test`. +fn delayed_body_outlives_a_false_gate(ability: &ResolvedAbility) -> bool { + if ability.else_ability.is_some() { + return true; + } + let Some(sub) = ability.sub_ability.as_deref() else { + return false; + }; + crate::game::effects::sub_outlives_false_parent_gate(sub) +} + +/// CR 603.4: does this gate read a binding that the FIRE-TIME leg of the hoist +/// resolves DIFFERENTLY from the resolution-time leg it is supposed to mirror? +/// +/// The two legs of the CR 603.4 pair must be the same predicate over the same +/// values. They are not the same *evaluator*: the fire-time leg runs +/// `check_trigger_condition_with_source`, whose `QuantityContext` +/// (`quantity::resolve_quantity_for_trigger_check`) is built from the delayed +/// ability's controller, its CR 400.7 source context and the matched event — it +/// has NO access to the delayed ability's snapshotted `targets` and no +/// resolution-scoped player. So a gate that reads one of those resolves against +/// the wrong object or player at fire time and would gate the ability off the +/// stack (and, for a consumed one-shot, delete it) on a value the resolution-time +/// reader would never have computed. +/// +/// Rather than let the two legs disagree, such a gate DECLINES the hoist and +/// keeps today's resolution-only reading — the same conservative treatment the +/// two bridges already give every other resolution-context predicate. +/// +/// Only `QuantityCheck` carries a `QuantityExpr` (and hence a scope or filter) +/// among the arms `ability_condition_to_static_condition` can bridge — the +/// others (`IsYourTurn`, `CompletedDungeon`, `SourceAttachedToCreature`, +/// `ControlsCommander`) are payload-free. `And`/`Or` do not bridge today; they +/// recurse here anyway so adding them to the bridge cannot silently bypass this. +fn gate_binding_diverges_at_fire_time(condition: &AbilityCondition) -> bool { + match condition { + AbilityCondition::QuantityCheck { lhs, rhs, .. } => { + quantity_expr_binding_diverges(lhs) || quantity_expr_binding_diverges(rhs) + } + AbilityCondition::Not { condition } => gate_binding_diverges_at_fire_time(condition), + AbilityCondition::And { conditions } | AbilityCondition::Or { conditions } => { + conditions.iter().any(gate_binding_diverges_at_fire_time) + } + _ => false, + } +} + +/// CR 603.4: the `QuantityExpr` half of [`gate_binding_diverges_at_fire_time`]. +/// Exhaustive over `QuantityExpr` so a new arithmetic wrapper cannot hide a +/// divergent leaf; the leaf test is [`quantity_ref_binding_diverges`]. +fn quantity_expr_binding_diverges(expr: &QuantityExpr) -> bool { + match expr { + QuantityExpr::Ref { qty } => quantity_ref_binding_diverges(qty), + QuantityExpr::Offset { inner, .. } + | QuantityExpr::ClampMin { inner, .. } + | QuantityExpr::Multiply { inner, .. } + | QuantityExpr::DivideRounded { inner, .. } + | QuantityExpr::UpTo { max: inner } + | QuantityExpr::Power { + exponent: inner, .. + } => quantity_expr_binding_diverges(inner), + QuantityExpr::Sum { exprs } | QuantityExpr::Max { exprs } => { + exprs.iter().any(quantity_expr_binding_diverges) + } + QuantityExpr::Difference { left, right } => { + quantity_expr_binding_diverges(left) || quantity_expr_binding_diverges(right) + } + QuantityExpr::Fixed { .. } => false, + } +} + +/// CR 115.1 + CR 608.2c: an object-axis scope the fire-time `QuantityContext` +/// cannot bind the way the resolving ability does. +/// +/// FAIL-CLOSED, and exhaustive so a new scope must be adjudicated: only the +/// three referents `quantity::resolve_quantity_for_trigger_check` is actually +/// handed — the CR 400.7 source context and the matched event's source/target — +/// are known to read the same on both legs. Everything else is a +/// RESOLUTION-scoped referent: `Target` reads `ability.targets`, `Recipient` is +/// passed as `None`, and the `CostPaidObject` / anaphor / per-resolution-local +/// family resolves through `ResolvedAbility` fields and `effect_context_object`, +/// none of which exist at detection time. Declining costs nothing but the +/// fire-time half of CR 603.4 for shapes no card in the pool has; guessing wrong +/// deletes a real ability. +fn object_scope_unbound_at_fire_time(scope: ObjectScope) -> bool { + match scope { + ObjectScope::Source | ObjectScope::EventSource | ObjectScope::EventTarget => false, + ObjectScope::Target + | ObjectScope::Recipient + | ObjectScope::CostPaidObject + | ObjectScope::Anaphoric + | ObjectScope::Demonstrative + | ObjectScope::OtherRevealedCard + | ObjectScope::AmassedArmy + | ObjectScope::OwnedLinkedExileCard + | ObjectScope::BatchSource => true, + } +} + +/// CR 115.10 + CR 608.2c: the player-axis counterpart, same fail-closed rule. +/// +/// `ScopedPlayer` is the per-iteration player of the RESOLVING ability, but the +/// fire-time context derives its `scoped_player` from the triggering event +/// (`extract_player_from_event`); `Target`, `RecipientController` and +/// `ParentObjectTargetController` all read `ability.targets` or the layer +/// recipient. The rest are derived from the controller (CR 109.5), the attacking +/// source (CR 508.5) or the source's own persisted choice (CR 613.1), all of +/// which the fire-time check has. `AnyTurn` is duration-timing-only and never +/// reaches a quantity, but is adjudicated here rather than wildcarded. +fn player_scope_unbound_at_fire_time(scope: &PlayerScope) -> bool { + match scope { + PlayerScope::ScopedPlayer + | PlayerScope::Target + | PlayerScope::RecipientController + | PlayerScope::ParentObjectTargetController => true, + PlayerScope::AllPlayers { exclude, .. } => exclude + .as_deref() + .is_some_and(player_scope_unbound_at_fire_time), + PlayerScope::Controller + | PlayerScope::Opponent { .. } + | PlayerScope::DefendingPlayer + | PlayerScope::SourceChosenPlayer + // CR 109.4: a concrete `PlayerId` already SNAPSHOTTED at resolution — a + // literal, so there is nothing left to bind and both legs read the same + // value. Same argument as `WhenLeavesPlay`'s already-resolved `ObjectId`. + // Like `AnyTurn` it is duration-timing-only and never reaches a quantity, + // but is adjudicated here rather than wildcarded. + | PlayerScope::SpecificPlayer { .. } + | PlayerScope::AnyTurn => false, + } +} + +/// CR 603.4: the leaf test of [`gate_binding_diverges_at_fire_time`]. +/// +/// Two independent reasons a leaf diverges: +/// +/// * it is scoped to an object or player the fire-time context cannot bind +/// (`object_scope_unbound_at_fire_time` / `player_scope_unbound_at_fire_time`); +/// * it counts a POPULATION whose filter the trigger-side bridge REWRITES. +/// `oracle_trigger::static_condition_to_trigger_condition` substitutes +/// `FilterProp::Another` → `FilterProp::OtherThanTriggerObject` on the +/// fire-time leg only (CR 603.4, Valakut's ruling), while +/// `effects::evaluate_condition` keeps the source-exclusion reading — so the +/// same printed "two or more OTHER creatures" counts a different population on +/// each leg. Declining is the conservative half of that pair; the alternative +/// (substituting on both legs) would change the resolution-time reading of +/// every non-delayed consumer of the same condition. +/// +/// * it reads a RESOLUTION-SCOPED tally that only exists while the ability is +/// resolving. The fire-time leg calls `resolve_quantity_for_trigger_check`, +/// which resolves with `targets = &[]`, `chosen_x = None`, `ability = None` +/// and no resolution-local ledger, so a payload-free leaf can diverge just as +/// badly as a scoped one (CR 608.2c: the chain tallies are established BY the +/// resolution). `TrackedSetSize` and friends read the most recent tracked set +/// at READ time — at fire time that is whatever an unrelated earlier +/// resolution left behind. +/// +/// EXHAUSTIVE and wildcard-free (matching `object_scope_unbound_at_fire_time` / +/// `quantity_expr_binding_diverges`), so a new `QuantityRef` must be adjudicated +/// here rather than silently defaulting to "cannot diverge" — the earlier +/// `_ => false` tail rested on exactly that claim and it was false for the +/// resolution-scoped payload-free family below. When in doubt the answer is +/// `true`: declining costs only the fire-time half of CR 603.4 for that shape, +/// while a wrong `false` deletes a real ability off the stack. +fn quantity_ref_binding_diverges(qty: &QuantityRef) -> bool { + match qty { + QuantityRef::CountersOn { scope, .. } + | QuantityRef::Power { scope } + | QuantityRef::Intensity { scope } + | QuantityRef::Toughness { scope } + | QuantityRef::ObjectManaValue { scope } + | QuantityRef::ObjectColorCount { scope } + | QuantityRef::ObjectNameWordCount { scope } + | QuantityRef::ObjectTypelineComponentCount { scope } + | QuantityRef::ManaSymbolsInManaCost { scope, .. } => { + object_scope_unbound_at_fire_time(*scope) + } + QuantityRef::HandSize { player, .. } + | QuantityRef::LifeTotal { player } + | QuantityRef::GraveyardSize { player, .. } + | QuantityRef::LifeLostThisTurn { player } + | QuantityRef::LifeGainedThisTurn { player } + | QuantityRef::PartySize { player } + | QuantityRef::Speed { player } + | QuantityRef::CardsDrawnThisTurn { player } + | QuantityRef::CardsDiscardedThisTurn { player } + | QuantityRef::LoyaltyAbilitiesActivatedThisTurn { player } => { + player_scope_unbound_at_fire_time(player) + } + QuantityRef::SacrificedThisTurn { player, filter } + | QuantityRef::TokensCreatedThisTurn { player, filter } + | QuantityRef::BattlefieldEntriesThisTurn { player, filter } => { + player_scope_unbound_at_fire_time(player) || filter_binding_diverges(filter) + } + QuantityRef::LandsPlayedThisTurn { player, .. } + | QuantityRef::PlayerActionsThisTurn { player, .. } => { + player_scope_unbound_at_fire_time(player) + } + QuantityRef::ObjectCount { filter } + | QuantityRef::ObjectCountDistinct { filter, .. } + | QuantityRef::ObjectCountBySharedQuality { filter, .. } + | QuantityRef::CountersOnObjects { filter, .. } + | QuantityRef::Aggregate { filter, .. } + | QuantityRef::EnteredThisTurn { filter } + | QuantityRef::DistinctColorsAmongPermanents { filter } + | QuantityRef::DistinctCounterKindsAmong { filter } + | QuantityRef::ControlledByEachPlayer { filter, .. } + | QuantityRef::ZoneChangeCountThisTurn { filter, .. } + | QuantityRef::ZoneChangeAggregateThisTurn { filter, .. } + | QuantityRef::CounterAddedThisTurn { + target: filter, + .. + } => filter_binding_diverges(filter), + QuantityRef::DamageDealtThisTurn { source, target, .. } => { + filter_binding_diverges(source) || filter_binding_diverges(target) + } + // Same filter axis, optional: a `None` filter names no population to + // re-scope, so only the `Some` arm can diverge. + QuantityRef::ZoneCardCount { filter, .. } + | QuantityRef::SpellsCastThisTurn { filter, .. } + | QuantityRef::SpellsCastThisGame { filter, .. } + | QuantityRef::AttackedThisTurn { filter, .. } => { + filter.as_ref().is_some_and(filter_binding_diverges) + } + // CR 205.2a + CR 205.3: the distinct-type family carries its population + // as a `CardTypeSetSource` rather than a bare filter. + QuantityRef::DistinctCardTypes { source } + | QuantityRef::DistinctSubtypes { source, .. } => { + card_type_set_source_binding_diverges(source) + } + // CR 601.2h: `AbilityTarget` is a target-slot read that + // `quantity::resolve_event_scoped_ref` explicitly answers `None` for at + // fire time; `SelfObject` reads `ctx.source` and `TriggeringSpell` is + // resolved from the matched event itself, so both bind identically on + // both legs — except through a `FromSource` metric, which carries its own + // population filter. + QuantityRef::ManaSpentToCast { scope, metric } => { + *scope == crate::types::ability::CastManaObjectScope::AbilityTarget + || match metric { + CastManaSpentMetric::FromSource { source_filter } => { + filter_binding_diverges(source_filter) + } + CastManaSpentMetric::Total + | CastManaSpentMetric::DistinctColors + | CastManaSpentMetric::OfColor { .. } => false, + } + } + // ---- RESOLUTION-SCOPED, payload-free or target-bound: always diverges ---- + // + // CR 115.1: reads the resolving ability's declared targets, which the + // fire-time resolver is handed as `&[]`. + QuantityRef::TargetControllerCounter { .. } + | QuantityRef::TargetObjectManaValue { .. } + | QuantityRef::TargetZoneCardCount { .. } + // CR 107.3a: `X` comes from the resolving ability's `chosen_x`, which is + // `None` at fire time. + | QuantityRef::Variable { .. } + // CR 608.2c: chain-local tracked sets and per-resolution tallies. Each is + // established BY a resolution; at fire time these read whatever an + // unrelated earlier resolution left in the ledger (or nothing at all). + | QuantityRef::TrackedSetSize + | QuantityRef::FilteredTrackedSetSize { .. } + | QuantityRef::TrackedSetAggregate { .. } + | QuantityRef::ExiledFromHandThisResolution + | QuantityRef::PreviousEffectAmount { .. } + | QuantityRef::TimesCostPaidThisResolution + // CR 608.2c: the secret-number ledger is populated BY the + // resolution that ran the choice (Wheel of Misfortune, Menacing Ogre) and + // is cleared per resolution — players who chose no number THIS resolution + // are excluded from the aggregate entirely. At fire time the ledger holds + // an unrelated resolution's numbers or none, so the two legs cannot agree. + // The `player` payload is irrelevant to that verdict: the ledger itself is + // resolution-scoped whichever player the scope selects. + | QuantityRef::PlayerChosenNumber { .. } + // CR 701.38: the vote tally is published by the resolution that ran the + // vote block. + | QuantityRef::VoteCount { .. } + // CR 106.4: a mana pool the resolution's own costs and mana abilities + // fill and empty; the fire-time reading is a different moment's pool. + | QuantityRef::UnspentMana { .. } + // CR 607.2a + CR 406.6: the source's linked-exile set, read through the + // resolving ability's materialized candidate set + // (`quantity::materialized_linked_exile_candidates`, which reads + // `ability.targets`). + | QuantityRef::CardsExiledBySource + | QuantityRef::ExiledCardPower { .. } + // CR 603.7c + CR 608.2c: the event-context family resolves through + // `state.current_trigger_event(s)` and the resolution-local + // amount/die/substitution cascade (`resolve_ref`'s `EventContextAmount` + // arm), none of which is populated at DETECTION time — the detection-time + // event override is consumed only by `ObjectCount`'s + // `OtherThanTriggerObject` exclusion. + | QuantityRef::EventContextAmount + | QuantityRef::EventContextPlayerCount { .. } + | QuantityRef::EventContextSourceCostX + | QuantityRef::EventContextSourceModesChosen + | QuantityRef::AttachmentsOnLeavingObject { .. } + | QuantityRef::SpellsCastBeforeTriggeringSpell { .. } + | QuantityRef::TriggeringScryLookCount + | QuantityRef::TriggeringScryBottomCount => true, + // ---- Reads that bind IDENTICALLY on both legs ---- + // + // Global or format-level state (CR 103.4, CR 500, CR 117.1), a + // controller-keyed per-turn/per-game accumulator (CR 109.5 — the + // fire-time leg is handed the delayed ability's own controller), or a + // read of the CR 400.7 source object the fire-time context carries + // (`ctx.source`, the same object `ObjectScope::Source` is adjudicated + // non-divergent for above). + QuantityRef::LifeAboveStarting + | QuantityRef::StartingLifeTotal + | QuantityRef::TriggeringDiscoverValue + | QuantityRef::PlayerCount { .. } + | QuantityRef::PlayerCounter { .. } + | QuantityRef::SelfManaValue + | QuantityRef::Devotion { .. } + | QuantityRef::BasicLandTypeCount { .. } + | QuantityRef::CrimesCommittedThisTurn + | QuantityRef::BendTypesThisTurn + | QuantityRef::TurnsTaken + | QuantityRef::ChosenNumber + | QuantityRef::DescendedThisTurn + | QuantityRef::SpellsCastLastTurn + | QuantityRef::DungeonsCompleted + | QuantityRef::CostXPaid + | QuantityRef::KickerCount + | QuantityRef::AdditionalCostPaymentCount + | QuantityRef::AdditionalCostPaymentCountFor { .. } + | QuantityRef::ConvokedCreatureCount + | QuantityRef::ColorsInCommandersColorIdentity + | QuantityRef::CommanderCastFromCommandZoneCount + | QuantityRef::CommanderManaValue { .. } => false, + } +} + +/// CR 603.4: the `CardTypeSetSource` half of [`quantity_ref_binding_diverges`]. +/// Exhaustive for the same reason: each source names a different population, and +/// two of them are resolution-scoped. +fn card_type_set_source_binding_diverges(source: &CardTypeSetSource) -> bool { + match source { + // CR 400.1: a zone census keyed by `CountScope` (controller / opponents / + // all) — the fire-time leg has the controller and reads the same zones. + CardTypeSetSource::Zone { .. } => false, + CardTypeSetSource::Objects { filter } => filter_binding_diverges(filter), + // CR 607.2a + CR 608.2c: the same two resolution-scoped populations + // `CardsExiledBySource` / `TrackedSetSize` are declined for. + CardTypeSetSource::ExiledBySource | CardTypeSetSource::TrackedSet { .. } => true, + } +} + +/// CR 603.4: the filter half of [`quantity_ref_binding_diverges`]. Recurses +/// through exactly the shapes `oracle_trigger::substitute_another_in_filter` +/// rewrites — `Typed` property lists plus the `And`/`Or`/`Not` combinators — so +/// the decline covers precisely the population the fire-time leg would have +/// re-scoped, no more and no less. +fn filter_binding_diverges(filter: &TargetFilter) -> bool { + match filter { + TargetFilter::Typed(tf) => tf + .properties + .iter() + .any(|prop| matches!(prop, FilterProp::Another)), + TargetFilter::Not { filter } => filter_binding_diverges(filter), + TargetFilter::And { filters } | TargetFilter::Or { filters } => { + filters.iter().any(filter_binding_diverges) + } + // CR 115.1 + CR 115.10: resolution-scoped anaphora read `ability.targets` + // / the per-iteration player, neither of which the fire-time context has + // — the population-level counterpart of `ObjectScope::Target`. + TargetFilter::ParentTarget + | TargetFilter::ParentTargetSlot { .. } + | TargetFilter::ParentTargetController + | TargetFilter::ParentTargetOwner + | TargetFilter::ScopedPlayer => true, + _ => false, + } +} + +/// CR 400.1 + CR 603.4: does the STATIC intermediate of the hoist LOSE the zone +/// axis on its way to the fire-time leg? +/// +/// `conditions::ability_condition_to_static_condition` folds the +/// `ObjectCount{filter} >= 1` shape into `StaticCondition::IsPresent`, and +/// `oracle_trigger::static_condition_to_trigger_condition` lowers the AFFIRMATIVE +/// `IsPresent` to `TriggerCondition::ControlsType`, whose evaluator +/// (`check_trigger_condition_with_source`) scans `state.battlefield` and nothing +/// else. The resolution-time leg counts in the FILTER'S OWN zone +/// (`quantity::object_count_matching_ids`, via `TargetFilter::extract_in_zone`). +/// For a filter that names a non-battlefield zone ("if you have a creature card +/// in your graveyard") those are two DIFFERENT predicates — TRUE at resolution, +/// FALSE at fire time — so the hoist would gate the ability off the stack and, +/// for a consumed one-shot, delete it outright. The zone axis is invisible to +/// [`gate_binding_diverges_at_fire_time`], which screens the object, player and +/// `FilterProp::Another` axes of the SAME leaf but not this one, so it is +/// adjudicated here, on the intermediate that actually loses the information. +/// +/// Only the affirmative arm is affected: the NEGATED `IsPresent` bridge already +/// lowers to `QuantityComparison { ObjectCount(f) EQ 0 }`, which resolves through +/// the same zone-aware `QuantityRef::ObjectCount` reader on both legs. +/// `And`/`Or` recurse because the trigger-side bridge maps them member-wise; no +/// `AbilityCondition` reaches them today (`ability_condition_to_static_condition` +/// declines `And`/`Or`), so the recursion is future-proofing, not live behaviour. +/// +/// Declining is the conservative half of the pair, exactly like +/// `gate_binding_diverges_at_fire_time`: the gate keeps today's resolution-only +/// reading rather than being evaluated as a different predicate. +fn static_gate_bridge_loses_zone(condition: &StaticCondition) -> bool { + match condition { + StaticCondition::IsPresent { filter: Some(f) } => f + .extract_in_zone() + .is_some_and(|zone| zone != crate::types::zones::Zone::Battlefield), + StaticCondition::And { conditions } | StaticCondition::Or { conditions } => { + conditions.iter().any(static_gate_bridge_loses_zone) + } + _ => false, + } +} + +/// CR 603.4: recover the intervening-`if` of a DELAYED triggered ability as a +/// trigger-level `TriggerCondition`. +/// +/// A printed "When [event], if [condition], [effect]" trigger carries its gate +/// on `TriggerDefinition.condition`, so both halves of CR 603.4 apply to it: the +/// collection gate refuses to put the ability on the stack, and +/// `stack.rs`'s recheck removes it if the gate flipped. A DELAYED triggered +/// ability parses the very same sentence shape into +/// `Effect::CreateDelayedTrigger`, but the gate lands on the delayed BODY as an +/// `AbilityCondition`, which only the resolution-time reader +/// (`effects::evaluate_condition`) ever consults. That is half of CR 603.4: the +/// ability would still be put onto the stack, be respondable, and count as a +/// triggered ability put onto the stack, when per CR 603.4 it must never have +/// triggered at all. +/// +/// Rather than add a fifth condition-vocabulary mirror, this composes the two +/// EXISTING single-authority bridges — +/// `conditions::ability_condition_to_static_condition` then +/// `oracle_trigger::static_condition_to_trigger_condition`. Both legs decline +/// every resolution-context predicate (`WhenYouDo`, `EffectOutcome`, +/// `HasObjectTarget`, `CoinFlipOutcome`, the casting-context family, …), so only +/// gates that are genuine game-state intervening-`if`s reach the fire-time +/// check; anything else keeps today's resolution-only behaviour unchanged. +/// +/// CR 603.4 governs only an `if` that IMMEDIATELY FOLLOWS the trigger event and +/// gates the WHOLE ability, so a body that still has work to do on the +/// condition-false path is NOT hoisted — see +/// [`delayed_body_outlives_a_false_gate`], which is deliberately NARROWER than +/// "has any sub-ability". +/// +/// The hoisted gate must also be one the two legs read IDENTICALLY, or the pair +/// CR 603.4 requires would be two different predicates — see +/// [`gate_binding_diverges_at_fire_time`] for the object/player/population axes +/// of the leaf, and [`static_gate_bridge_loses_zone`] for the zone axis the +/// `IsPresent` intermediate drops. +/// +/// CR 400.7: there is deliberately NO guard here for a delayed ability carrying +/// no `trigger_source` context, and the absence is load-bearing rather than an +/// oversight. The worry it would answer is real in shape — a source-relative gate +/// with no context would read nothing, evaluate false for want of a reading +/// rather than on the game state, and (via [`false_gate_consumes_one_shot`]) +/// DELETE a one-shot outright. It cannot happen, because matching runs BEFORE +/// gating and already requires that context: `delayed_trigger_event_with_index` +/// opens its `WhenNextEvent` arm with `let source_context = source_context?;`, so +/// a contextless one-shot never matches an event, never reaches this gate, and is +/// never discarded. The condition is checked at most where a reading exists. +/// +/// A guard here would therefore be unreachable code that also declines the hoist +/// for gates whose fire-time reading is perfectly well-defined without a source +/// (the controller-scoped `QuantityCheck` populations pinned by +/// `non_battlefield_presence_gate_declines_the_fire_time_hoist` and its +/// siblings) — buying nothing and costing CR 603.4's fire-time half. +fn delayed_intervening_if(ability: &ResolvedAbility) -> Option { + if delayed_body_outlives_a_false_gate(ability) { + return None; + } + let condition = ability.condition.as_ref()?; + if gate_binding_diverges_at_fire_time(condition) { + return None; + } + let static_condition = + crate::parser::oracle_effect::conditions::ability_condition_to_static_condition(condition)?; + if static_gate_bridge_loses_zone(&static_condition) { + return None; + } + crate::parser::oracle_trigger::static_condition_to_trigger_condition(&static_condition) +} + +/// CR 603.4 + CR 603.7b: when the hoisted intervening-`if` was FALSE, is this +/// ONE-SHOT delayed ability CONSUMED by the occurrence it just declined? +/// +/// CR 603.4 says a false gate means the ability "does nothing" — it never +/// triggered. CR 603.7b says a delayed triggered ability "will trigger only +/// once — the next time its trigger event occurs — unless it has a stated +/// duration, such as 'this turn.'" The two only agree on DROPPING the ability +/// when its stated trigger event can occur at most once: +/// +/// * a PHASE-NAMED time ("at the beginning of the next end step"). That named +/// time has now come and gone; a false gate there must not let the ability lie +/// in wait for the FOLLOWING end step; +/// * a zone change of ONE ALREADY-BOUND object ("when THAT creature dies" — +/// `TargetFilter::names_bound_single_object`). The bound object departs once +/// per incarnation, and an object that returns is a NEW object (CR 400.7 / +/// CR 603.7c), so a retained ability could never match it anyway; +/// * EVERY `WhenNextEvent`, whatever its `DelayedTriggerLifetime` — because this +/// variant IS CR 603.7b's once-only half. The rule's two outcomes are modelled +/// as two SIBLING conditions, not as a lifetime: `WheneverEvent` is the +/// "unless it has a stated duration" carve-out (multi-fire, purged by +/// `WheneverEventExpiry`), and `WhenNextEvent` is documented on its own +/// declaration as the "one-shot variant of `WheneverEvent`". A stated-duration +/// ability that must keep watching is therefore a `WheneverEvent` and CANNOT +/// reach this function: `effects::delayed_trigger` computes +/// `one_shot = !matches!(condition, WheneverEvent { .. })` and the only caller +/// gates on `delayed.one_shot`. The lifetime then bounds only how long the +/// single shot WAITS — `ThisTurn` to cleanup, `Persistent` open-ended +/// (The Pandorica), `Reflexive` to its creation batch (CR 603.12). +/// +/// Do NOT re-derive that from the wording "next": the discriminator is WotC's +/// "When" / "Whenever" templating, and the parser already keys on exactly that. +/// The distinction is load-bearing and easy to get backwards, so it is worth +/// naming the case that proves it. `parse_dealt_damage_this_way_dies_trigger` +/// (`oracle_effect/mod.rs`) parses "[subject] dealt damage this way dies +/// [this turn]" — a BROAD filter with no "next" anywhere — and is reached from +/// two call sites that lower it differently, on the templating alone: +/// - the `"whenever "` site → `WheneverEvent` (multi-fire). Ghired's +/// Belligerence and Reckless Blaze, both of which spread damage over many +/// creatures, so many deaths can qualify; +/// - the `"when "` site → `WhenNextEvent { ThisTurn }` (one-shot). Skeletonize, +/// whose damage goes to a SINGLE target creature, so at most one death can +/// ever qualify — one occurrence, correctly consumed. +/// +/// Those are the only three cards in the pool with that wording, and the split is +/// exact for all three. So "broad filter" alone never implies multi-fire here, +/// and this arm does not need to inspect the filter: the routing decision was +/// already made, correctly, one layer up. +/// +/// Exhaustive on purpose: a new `DelayedTriggerCondition` must decide whether its +/// stated event is a single occurrence before it can be discarded on a false gate. +fn false_gate_consumes_one_shot(condition: &DelayedTriggerCondition) -> bool { + match condition { + DelayedTriggerCondition::AtNextPhase { .. } + | DelayedTriggerCondition::AtNextPhaseForPlayer { .. } + // A concrete `ObjectId`: the same single-occurrence argument as a + // bound-single-object filter, already resolved to one object. + | DelayedTriggerCondition::WhenLeavesPlay { .. } => true, + DelayedTriggerCondition::WhenDies { filter } + | DelayedTriggerCondition::WhenLeavesPlayFiltered { filter } + | DelayedTriggerCondition::WhenEntersBattlefield { filter } + | DelayedTriggerCondition::WhenDiesOrExiled { filter } => { + filter.names_bound_single_object() + } + // CR 603.4 + CR 603.7b (+ CR 603.12 for `Reflexive`): this variant IS the + // one-shot half of the CR 603.7b split — its own doc calls it the + // "one-shot variant of `WheneverEvent`" — so the matched event has spent + // its single occurrence whatever lifetime it carries. Retaining it would + // silently rewrite "when X, if C" into "when X for which C holds", letting + // a later X fire an ability CR 603.4 already resolved as doing nothing. + // A trigger that must keep watching is a `WheneverEvent` and cannot reach + // here at all — see the doc above. + DelayedTriggerCondition::WhenNextEvent { .. } => true, + // Never one-shot (`effects::delayed_trigger` computes `one_shot` as + // "not `WheneverEvent`"), so this arm is unreachable from the caller; + // spelled out rather than wildcarded to keep the match exhaustive. + DelayedTriggerCondition::WheneverEvent { .. } => false, + } +} + fn delayed_trigger_to_context( state: &GameState, trigger: DelayedTrigger, trigger_event: GameEvent, ) -> PendingTriggerContext { + // CR 603.4 (second half): carry the hoisted intervening-`if` onto the stack + // entry so `stack.rs`'s resolution recheck applies to a delayed triggered + // ability exactly as it does to a printed one. `delayed_intervening_if` is + // the SAME authority the collection gate below used, so the two halves of + // the CR 603.4 pair cannot read different predicates. + let condition = delayed_intervening_if(&trigger.ability); PendingTriggerContext::delayed( PendingTrigger { source_id: trigger.source_id, controller: trigger.controller, - condition: None, + condition, ability: trigger.ability, timestamp: state.turn_number, target_constraints: Vec::new(), @@ -8919,7 +9534,11 @@ fn collect_matching_delayed_triggers( // `WhenNextEvent` would do). The phase-only path keeps the historical // narrower coin-flip discard gate because it filters non-phase matches out of // the candidate batch. - let mut to_discard: Vec = Vec::new(); + // + // Each entry pairs the index with the terminal disposition to record for it, + // so a discarded reflexive (CR 603.12) and a CR 603.4 intervening-`if` + // failure stay distinguishable in the lifecycle ledger. + let mut to_discard: Vec<(usize, super::lifecycle::DelayedTerminalDisposition)> = Vec::new(); for (idx, delayed) in state.delayed_triggers.iter().enumerate() { if let Some((event_index, trigger_event)) = delayed_trigger_event_with_index( @@ -8933,6 +9552,44 @@ fn collect_matching_delayed_triggers( if !scope.accepts(&trigger_event) { continue; } + // CR 603.4 (first half): "When the trigger event occurs, the ability + // checks whether the stated condition is true. The ability triggers + // only if it is; otherwise it does nothing." The delayed body's + // intervening-`if` was previously consulted ONLY at resolution, so a + // failing gate still put a respondable ability on the stack — and a + // player could then make the gate true in response (getting a + // commander onto the battlefield for Fight for the Throne), which + // CR 603.4 forbids outright. + // + // `check_trigger_condition_with_source` is the same fire-time + // evaluator printed triggers use, given the delayed ability's own + // CR 400.7 source context and the matched event. + if let Some(condition) = delayed_intervening_if(&delayed.ability) { + if !check_trigger_condition_with_source( + state, + &condition, + delayed.controller, + delayed.ability.trigger_source.as_ref(), + Some(&trigger_event), + ) { + // CR 603.4 + CR 603.7b: the ability did not trigger. It is + // removed without firing, tagged `InterveningIfFalse`, ONLY + // when its stated event was a single occurrence that this + // check has now consumed — see + // `false_gate_consumes_one_shot`. Everything else (a + // multi-fire "whenever … this turn", and a one-shot watching + // a BROAD event filter) stays installed and gets its gate + // re-checked on the next occurrence, per CR 603.7b's + // stated-duration clause. + if delayed.one_shot && false_gate_consumes_one_shot(&delayed.condition) { + to_discard.push(( + idx, + super::lifecycle::DelayedTerminalDisposition::InterveningIfFalse, + )); + } + continue; + } + } if delayed.one_shot { to_remove.push((idx, event_index, trigger_event)); } else { @@ -8972,7 +9629,10 @@ fn collect_matching_delayed_triggers( ) } } { - to_discard.push(idx); + to_discard.push(( + idx, + super::lifecycle::DelayedTerminalDisposition::ReflexiveUnmatched, + )); } } @@ -8996,31 +9656,34 @@ fn collect_matching_delayed_triggers( } } - // Remove fired one-shot triggers AND discarded non-matching reflexive triggers + // Remove fired one-shot triggers AND every one-shot dropped without firing // from `delayed_triggers` in a single descending-index pass so every index - // stays valid. Fired one-shots are collected into `to_fire`; discarded - // reflexives (CR 603.12) are dropped without firing. The two index sets are - // disjoint — a trigger either matched (fire) or resolved-without-match - // (discard), never both. + // stays valid. Fired one-shots are collected into `to_fire`; the unfired set + // is dropped with its own recorded disposition — `ReflexiveUnmatched` for a + // CR 603.12 reflexive that never matched, `InterveningIfFalse` for a + // CR 603.4 gate that was false when the trigger event occurred. The two + // index sets are disjoint — a trigger either fired, or was dropped + // unfired, never both. let mut fired_events: std::collections::HashMap = to_remove .iter() .map(|(idx, event_index, event)| (*idx, (*event_index, event.clone()))) .collect(); + let mut unfired_dispositions: std::collections::HashMap< + usize, + super::lifecycle::DelayedTerminalDisposition, + > = to_discard.iter().copied().collect(); let mut combined: Vec = to_remove .iter() .map(|(idx, _, _)| *idx) - .chain(to_discard.iter().copied()) + .chain(to_discard.iter().map(|(idx, _)| *idx)) .collect(); combined.sort_unstable(); for idx in combined.into_iter().rev() { let trigger = state.delayed_triggers.remove(idx); if let Some((event_index, trigger_event)) = fired_events.remove(&idx) { to_fire.push((trigger, event_index, trigger_event, true)); - } else { - super::lifecycle::record_delayed_terminal( - trigger.provenance.firing(), - super::lifecycle::DelayedTerminalDisposition::ReflexiveUnmatched, - ); + } else if let Some(disposition) = unfired_dispositions.remove(&idx) { + super::lifecycle::record_delayed_terminal(trigger.provenance.firing(), disposition); } } @@ -18201,6 +18864,1514 @@ pub mod tests { ); } + /// CR 603.4 (fire-time half) for a DELAYED triggered ability. + /// + /// Stages Fight for the Throne's shape — a one-shot `WhenDies` delayed + /// ability whose body carries the intervening-`if` "if you control your + /// commander" — and drives the production collection entry point + /// (`check_delayed_triggers`) on the death event. + /// + /// REVERT-TO-RED: with the fire-time hoist removed, `state.stack.len()` is + /// `1` in the no-commander half, because the ability was put onto the stack + /// and only skipped later, at resolution. That is precisely the CR 603.4 + /// violation — the ability is respondable, counts as a triggered ability put + /// onto the stack, and a player could make the gate TRUE in response. + /// + /// The commander half is the reachability proof: the very same fixture with + /// a commander staged DOES reach the stack, so the empty-stack assertion + /// below cannot be passing because the delayed trigger never matched. + /// + /// The two `DelayedSubject` halves pin the CR 603.7b survival split that + /// `false_gate_consumes_one_shot` decides — see the assertions below. + #[test] + fn delayed_intervening_if_gates_the_ability_off_the_stack_at_fire_time() { + /// Which shape of "when [it] dies" the fixture installs. + #[derive(Clone, Copy)] + enum DelayedSubject { + /// "when A creature dies this turn" — a CLASS filter that can match + /// again later in the turn. + BroadFilter, + /// "when THAT creature dies" — one already-bound object. + BoundObject, + } + + fn run(stage_commander: bool, subject: DelayedSubject) -> (usize, usize) { + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + + let source = create_object( + &mut state, + CardId(0x0603_0401), + controller, + "Fight for the Throne".to_string(), + Zone::Battlefield, + ); + if stage_commander { + // CR 903.3 + CR 903.3d: owned AND controlled, on the battlefield. + let commander = make_creature(&mut state, controller, "Your Commander", 2, 2); + state + .objects + .get_mut(&commander) + .expect("staged commander") + .is_commander = true; + } + let victim = make_creature(&mut state, PlayerId(1), "Doomed Squire", 1, 1); + + let mut ability = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + // The intervening-`if` as the parser lowers it onto the delayed BODY. + ability.condition = Some(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }); + let filter = match subject { + DelayedSubject::BroadFilter => TargetFilter::Any, + DelayedSubject::BoundObject => TargetFilter::SpecificObject { id: victim }, + }; + assert_eq!( + filter.names_bound_single_object(), + matches!(subject, DelayedSubject::BoundObject), + "reach-guard: the fixture's filter must land on the side of \ + `names_bound_single_object` the case under test intends, or both halves \ + take the same retention branch" + ); + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WhenDies { filter }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + let death = zone_changed_event( + victim, + Zone::Battlefield, + Zone::Graveyard, + vec![CoreType::Creature], + Vec::new(), + ); + check_delayed_triggers(&mut state, &[death]); + (state.stack.len(), state.delayed_triggers.len()) + } + + let (gated_stack, gated_remaining) = run(false, DelayedSubject::BroadFilter); + assert_eq!( + gated_stack, 0, + "CR 603.4: with the intervening-if FALSE when the trigger event occurs, the \ + delayed ability must never be put onto the stack. A 1 here is the shipped \ + defect — a respondable ability that CR 603.4 says never triggered" + ); + assert_eq!( + gated_remaining, 1, + "CR 603.4 + CR 603.7b: the ability DID NOT TRIGGER, and a broad-filter form \ + has a stated duration, so it must keep watching for a later qualifying \ + death. A 0 here is the defect where the first non-qualifying death silently \ + destroys the ability for the rest of the turn" + ); + + let (fired_stack, fired_remaining) = run(true, DelayedSubject::BroadFilter); + assert_eq!( + fired_stack, 1, + "reachability proof: with an owned-and-controlled commander the SAME fixture \ + puts the delayed ability on the stack, so the gated assertion above is not \ + passing because the trigger never matched" + ); + assert_eq!( + fired_remaining, 0, + "the fired one-shot is removed from delayed_triggers as before" + ); + + let (bound_gated_stack, bound_gated_remaining) = run(false, DelayedSubject::BoundObject); + assert_eq!( + bound_gated_stack, 0, + "CR 603.4: the bound-object form is gated off the stack on a false gate too" + ); + assert_eq!( + bound_gated_remaining, 0, + "CR 603.7b + CR 400.7: the ONE bound object has now died, and an object that \ + returns is a new object, so nothing is left for this ability to watch — it \ + is consumed. A 1 here would leak a permanently inert delayed trigger" + ); + + let (bound_fired_stack, bound_fired_remaining) = run(true, DelayedSubject::BoundObject); + assert_eq!( + bound_fired_stack, 1, + "reachability proof for the bound-object half: the same fixture with the gate \ + TRUE reaches the stack, so the retention assertion above is about the gate" + ); + assert_eq!( + bound_fired_remaining, 0, + "the fired one-shot is removed from delayed_triggers as before" + ); + } + + /// CR 603.7b for the PHASE-NAMED one-shot class — the case the discard was + /// always sound for. "At the beginning of the next end step, if [gate], …" + /// names ONE specific later time; when that time arrives with the gate false + /// the ability did nothing (CR 603.4) AND has nothing left to wait for, so it + /// must not lie in wait for the FOLLOWING end step. + /// + /// REVERT-TO-RED: drop the `AtNextPhase` arm from + /// `false_gate_consumes_one_shot` and `remaining` below is `1` — a delayed + /// ability that outlives its own named deadline. + /// + /// The gate-true half is the reachability proof that this fixture's delayed + /// trigger genuinely matches the end-step event. + #[test] + fn phase_named_one_shot_is_consumed_by_a_false_intervening_if() { + fn run(stage_commander: bool) -> (usize, usize) { + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + + let source = create_object( + &mut state, + CardId(0x0603_0406), + controller, + "End Step Rider".to_string(), + Zone::Battlefield, + ); + if stage_commander { + let commander = make_creature(&mut state, controller, "Your Commander", 2, 2); + state + .objects + .get_mut(&commander) + .expect("staged commander") + .is_commander = true; + } + + let mut ability = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + ability.condition = Some(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }); + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::AtNextPhase { phase: Phase::End }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + check_delayed_triggers(&mut state, &[GameEvent::PhaseChanged { phase: Phase::End }]); + (state.stack.len(), state.delayed_triggers.len()) + } + + let (gated_stack, gated_remaining) = run(false); + assert_eq!( + gated_stack, 0, + "CR 603.4: the ability must never be put onto the stack with the gate false" + ); + assert_eq!( + gated_remaining, 0, + "CR 603.7b: \"at the beginning of the NEXT end step\" names one specific later \ + time, which has now passed — the ability is consumed, not left waiting" + ); + + let (fired_stack, fired_remaining) = run(true); + assert_eq!( + fired_stack, 1, + "reachability proof: with the gate TRUE the same fixture reaches the stack, so \ + the consumption above is not a never-matched trigger" + ); + assert_eq!( + fired_remaining, 0, + "the fired one-shot is removed as before" + ); + } + + /// CR 603.4 + CR 603.7b for the `WhenNextEvent` one-shot class, driven through + /// the production `check_delayed_triggers` path over TWO separate event + /// batches. + /// + /// "When you next [event] this turn, if [gate], …" names ONE occurrence. When + /// that occurrence arrives with the gate false the ability "does nothing" + /// (CR 603.4) and its single shot is spent (CR 603.7b) — making the gate true + /// afterwards must NOT let a second matching event resurrect it. Retaining it + /// would silently rewrite the ability into "when you next [event] for which + /// [gate] holds". + /// + /// REVERT-TO-RED: restore `is_reflexive_lifetime(condition)` on the + /// `WhenNextEvent` arm of `false_gate_consumes_one_shot` and this + /// non-reflexive `ThisTurn` trigger survives batch one, so `after_first` + /// is `1` and the second batch puts the ability on the stack. + #[test] + fn when_next_event_one_shot_is_consumed_by_a_false_intervening_if() { + use crate::types::ability::DelayedTriggerLifetime; + use crate::types::triggers::TriggerMode; + + /// Installs "when you next play a land this turn, if you control your + /// commander, you become the monarch" and returns the land whose play is + /// the matching event. + fn install(state: &mut GameState) -> ObjectId { + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + + let source = create_object( + state, + CardId(0x0603_0407), + controller, + "Next Land Rider".to_string(), + Zone::Battlefield, + ); + let land = create_object( + state, + CardId(0x0603_0408), + controller, + "Played Land".to_string(), + Zone::Battlefield, + ); + + // CR 305.1 + CR 603.2: scope the delayed event to the controller's + // own land drop, exactly as the parser's `WhenNextEvent` builders do. + let mut trigger_def = TriggerDefinition::new(TriggerMode::LandPlayed); + trigger_def.valid_target = Some(TargetFilter::Controller); + + let mut ability = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + ability.condition = Some(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }); + // `WhenNextEvent` matching requires the delayed ability's CR 400.7 + // source context; without it `delayed_trigger_event_with_index` + // returns `None` and the fixture would prove nothing. + let source_object = state.objects.get(&source).expect("installed source"); + ability.trigger_source = Some(trigger_source_context_for_latch(state, source_object)); + + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WhenNextEvent { + trigger: Box::new(trigger_def), + or_trigger: None, + lifetime: DelayedTriggerLifetime::ThisTurn, + }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + land + } + + fn stage_commander(state: &mut GameState) { + // CR 903.3 + CR 903.3d: owned AND controlled, on the battlefield. + let commander = make_creature(state, PlayerId(0), "Your Commander", 2, 2); + state + .objects + .get_mut(&commander) + .expect("staged commander") + .is_commander = true; + } + + fn land_played(land: ObjectId) -> GameEvent { + GameEvent::LandPlayed { + object_id: land, + player_id: PlayerId(0), + from_zone: Zone::Hand, + } + } + + // Reachability proof: the same fixture with the gate TRUE on the first + // matching event does reach the stack, so the assertions below are about + // the gate and not about a trigger that never matched `LandPlayed`. + let mut reachable = setup(); + let reachable_land = install(&mut reachable); + stage_commander(&mut reachable); + check_delayed_triggers(&mut reachable, &[land_played(reachable_land)]); + assert_eq!( + reachable.stack.len(), + 1, + "reachability proof: with the gate TRUE the first land drop fires the delayed ability" + ); + + let mut state = setup(); + let land = install(&mut state); + + // Batch one: the ability's single named occurrence, gate FALSE. + check_delayed_triggers(&mut state, &[land_played(land)]); + assert_eq!( + state.stack.len(), + 0, + "CR 603.4: a false intervening-`if` means the ability never triggers, so it must \ + not be put onto the stack" + ); + let after_first = state.delayed_triggers.len(); + assert_eq!( + after_first, 0, + "CR 603.7b: \"when you NEXT play a land this turn\" names ONE occurrence, and that \ + occurrence has now happened — the one-shot is consumed, not left installed" + ); + + // Batch two: the gate is now TRUE and another land is played. A consumed + // one-shot must stay consumed; a retained one would fire here. + stage_commander(&mut state); + check_delayed_triggers(&mut state, &[land_played(land)]); + assert_eq!( + state.stack.len(), + 0, + "CR 603.4 + CR 603.7b: the ability already spent its single occurrence with the \ + gate false; making the gate true afterwards must not let a LATER land drop fire it" + ); + } + + /// The OTHER half of CR 603.7b, and the discriminating counterpart to the + /// test above: a STATED-DURATION delayed ability whose first matching event + /// fails the intervening-`if` must SURVIVE and still fire on a later matching + /// event in the same turn. + /// + /// CR 603.7b caps a delayed ability at one trigger "unless it has a stated + /// duration, such as 'this turn.'" The engine models that carve-out as + /// `DelayedTriggerCondition::WheneverEvent` (multi-fire), the sibling of the + /// one-shot `WhenNextEvent` — so the protection is structural, and this test + /// pins the structure rather than trusting it: `effects::delayed_trigger` + /// computes `one_shot = !matches!(condition, WheneverEvent { .. })`, and the + /// discard in `check_delayed_triggers` is gated on `delayed.one_shot`, so a + /// false gate here must decline the fire WITHOUT consuming the ability. + /// + /// Two matching events, gate false then true, is what makes this + /// discriminating: a one-event fixture would pass even if the ability were + /// wrongly discarded, because both readings put nothing on the stack the + /// first time. + /// + /// REVERT-TO-RED, and the exact recipe matters here: `WheneverEvent` is + /// protected TWICE over, so neither half alone reds this test. Flipping only + /// `false_gate_consumes_one_shot`'s `WheneverEvent` arm to `true` changes + /// nothing, because the caller never consults it for a multi-fire trigger; + /// dropping only the caller's `delayed.one_shot &&` conjunct changes nothing, + /// because the arm still answers `false`. Do BOTH — flip the arm to `true` + /// AND drop the conjunct — and `after_first` becomes `0` and the second event + /// fires nothing (measured, not asserted from reading). + /// + /// That redundancy is the finding, not a weakness of the fixture: this test + /// pins the CONJUNCTION that is the actual guarantee, so it stays red for any + /// change that removes the protection outright rather than merely moving it + /// between the two layers. + #[test] + fn stated_duration_multi_fire_survives_a_false_intervening_if() { + use crate::types::triggers::TriggerMode; + + /// Installs "whenever you play a land this turn, if you control your + /// commander, you become the monarch" — the stated-duration shape. + /// Deliberately the same fixture as the one-shot test above, differing + /// ONLY in the condition sibling, so the two tests isolate exactly the + /// `WhenNextEvent` / `WheneverEvent` distinction and nothing else. + fn install(state: &mut GameState) -> ObjectId { + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + + let source = create_object( + state, + CardId(0x0603_0409), + controller, + "Stated Duration Rider".to_string(), + Zone::Battlefield, + ); + let land = create_object( + state, + CardId(0x0603_040a), + controller, + "Played Land".to_string(), + Zone::Battlefield, + ); + + // CR 305.1 + CR 603.2: scope the delayed event to the controller's + // own land drop, exactly as the one-shot fixture above does. + let mut trigger_def = TriggerDefinition::new(TriggerMode::LandPlayed); + trigger_def.valid_target = Some(TargetFilter::Controller); + + let mut ability = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + ability.condition = Some(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }); + let source_object = state.objects.get(&source).expect("installed source"); + ability.trigger_source = Some(trigger_source_context_for_latch(state, source_object)); + + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WheneverEvent { + trigger: Box::new(trigger_def), + expiry: crate::types::ability::WheneverEventExpiry::EndOfTurn, + }, + ability: Box::new(ability), + controller, + source_id: source, + // CR 603.7b: the stated duration is what makes this multi-fire. + // Mirrors `effects::delayed_trigger`'s own computation + // (`one_shot = !matches!(condition, WheneverEvent { .. })`). + one_shot: false, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + land + } + + fn stage_commander(state: &mut GameState) { + // CR 903.3 + CR 903.3d: owned AND controlled, on the battlefield. + let commander = make_creature(state, PlayerId(0), "Your Commander", 2, 2); + state + .objects + .get_mut(&commander) + .expect("staged commander") + .is_commander = true; + } + + fn land_played(land: ObjectId) -> GameEvent { + GameEvent::LandPlayed { + object_id: land, + player_id: PlayerId(0), + from_zone: Zone::Hand, + } + } + + let mut state = setup(); + let land = install(&mut state); + + // Batch one: a matching event with the gate FALSE (no commander staged). + check_delayed_triggers(&mut state, &[land_played(land)]); + assert_eq!( + state.stack.len(), + 0, + "CR 603.4: a false intervening-`if` means the ability does not trigger on this \ + occurrence" + ); + let after_first = state.delayed_triggers.len(); + assert_eq!( + after_first, 1, + "CR 603.7b: a STATED-DURATION delayed ability is not capped at one trigger, so a \ + false gate must decline the occurrence WITHOUT consuming the ability" + ); + + // Batch two: make the gate TRUE, then a second matching event. The + // ability must still be installed and must now fire. + stage_commander(&mut state); + check_delayed_triggers(&mut state, &[land_played(land)]); + assert_eq!( + state.stack.len(), + 1, + "CR 603.7b + CR 603.4: the ability survived the declined occurrence and its gate is \ + re-checked on the next one, which now passes" + ); + + // CR 603.4 (second half) + CR 608.2a: passing the FIRE-TIME gate must not + // disarm the RESOLUTION-TIME one. The hoist is a pair, not a move: the + // same gate has to ride onto the stack entry so `stack.rs`'s + // `bind_resolution_scope` re-checks it, which is what denies the ability + // if the condition stops holding between triggering and resolution (a + // commander leaving the battlefield in response). An entry carrying + // `condition: None` would resolve unconditionally — the very bug this PR + // fixes, reintroduced one layer later. + let entry = state.stack.last().expect("the fired delayed ability"); + assert!( + matches!( + &entry.kind, + StackEntryKind::TriggeredAbility { + condition: Some(_), + .. + } + ), + "CR 603.4: the hoisted intervening-`if` must also be carried onto the stack entry \ + for the resolution-time recheck, got {:?}", + entry.kind + ); + } + + /// HOSTILE fixture for the CR 603.4 hoist's carve-out. + /// + /// "…, if X, A. Otherwise, B." is NOT an intervening-`if` in the CR 603.4 + /// sense — the ability has work to do when the gate is false, so it MUST + /// still be put onto the stack and resolve its `else_ability`. + /// `delayed_body_outlives_a_false_gate` declines every body carrying an + /// `else_ability` for exactly this reason. + /// + /// REVERT-TO-RED: drop the `else_ability` arm from + /// `delayed_body_outlives_a_false_gate` and the gate hoists, the ability + /// never reaches the stack, and the Otherwise branch is silently deleted — + /// life stays 20 and the stack stays empty. + #[test] + fn delayed_body_with_an_otherwise_branch_is_never_gated_off_the_stack() { + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + state.players[0].life = 20; + + let source = create_object( + &mut state, + CardId(0x0603_0402), + controller, + "Otherwise Rider".to_string(), + Zone::Battlefield, + ); + let victim = make_creature(&mut state, PlayerId(1), "Doomed Squire", 1, 1); + + // Gate is FALSE (no commander anywhere), so the Otherwise branch is the + // one that must run. + let mut ability = ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + ability.condition = Some(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }); + ability.else_ability = Some(Box::new(ResolvedAbility::new( + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + vec![], + source, + controller, + ))); + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WhenDies { + filter: TargetFilter::Any, + }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + let death = zone_changed_event( + victim, + Zone::Battlefield, + Zone::Graveyard, + vec![CoreType::Creature], + Vec::new(), + ); + check_delayed_triggers(&mut state, &[death]); + assert_eq!( + state.stack.len(), + 1, + "a body with an Otherwise branch must still reach the stack even though its \ + own condition is false" + ); + + let mut events = Vec::new(); + crate::game::stack::resolve_top(&mut state, &mut events); + assert_eq!( + state.players[0].life, 21, + "CR 608.2c: the Otherwise branch must resolve. 20 here means the CR 603.4 \ + hoist swallowed a non-intervening-if gate and deleted the else branch" + ); + assert_eq!( + state.monarch, None, + "the gated primary effect must still not happen" + ); + } + + /// CR 603.4 for a CHAINED delayed body — the case the old blanket + /// `sub_ability.is_some()` guard silently got wrong. + /// + /// "When [event], if [gate], A and B" lowers B as an UNCONDITIONAL + /// `SequentialSibling` sub of the gated body. CR 603.4's `if` gates the WHOLE + /// ability, so with the gate false NEITHER clause may happen. Under the old + /// guard the hoist was skipped AND `resolve_ability_chain`'s + /// `SequentialSibling && condition.is_none()` escape resolved B anyway — the + /// ability went on the stack and B happened with the gate false. + /// + /// REVERT-TO-RED: restore `|| ability.sub_ability.is_some()` in + /// `delayed_body_outlives_a_false_gate` and the gated half below reports + /// `stack == 1` and life 21 — B resolved through a false CR 603.4 gate. + /// + /// The commander half is the reachability proof AND the non-deletion proof: + /// the same fixture with the gate TRUE runs both clauses. + #[test] + fn delayed_body_with_an_unconditional_sequential_sub_is_gated_as_one_ability() { + fn run(stage_commander: bool) -> (usize, i32, Option) { + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + state.players[0].life = 20; + + let source = create_object( + &mut state, + CardId(0x0603_0403), + controller, + "Chained Rider".to_string(), + Zone::Battlefield, + ); + if stage_commander { + let commander = make_creature(&mut state, controller, "Your Commander", 2, 2); + state + .objects + .get_mut(&commander) + .expect("staged commander") + .is_commander = true; + } + let victim = make_creature(&mut state, PlayerId(1), "Doomed Squire", 1, 1); + + let mut ability = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + ability.condition = Some(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }); + // "…and gain 1 life" — the second clause of the ONE gated sentence. + let mut sub = ResolvedAbility::new( + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + vec![], + source, + controller, + ); + sub.sub_link = crate::types::ability::SubAbilityLink::SequentialSibling; + assert!( + sub.condition.is_none(), + "reach-guard: the sub must be UNCONDITIONAL, or it takes a different \ + resolve_ability_chain branch than the one under test" + ); + ability.sub_ability = Some(Box::new(sub)); + + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WhenDies { + filter: TargetFilter::Any, + }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + let death = zone_changed_event( + victim, + Zone::Battlefield, + Zone::Graveyard, + vec![CoreType::Creature], + Vec::new(), + ); + check_delayed_triggers(&mut state, &[death]); + let stack_len = state.stack.len(); + if stack_len == 1 { + let mut events = Vec::new(); + crate::game::stack::resolve_top(&mut state, &mut events); + } + (stack_len, state.players[0].life, state.monarch) + } + + let (gated_stack, gated_life, gated_monarch) = run(false); + assert_eq!( + gated_stack, 0, + "CR 603.4: the intervening-if gates the WHOLE ability, chain and all" + ); + assert_eq!( + gated_life, 20, + "CR 603.4: the chained second clause must NOT happen through a false gate. \ + 21 means the unconditional SequentialSibling escaped the gate" + ); + assert_eq!( + gated_monarch, None, + "the gated primary effect must not happen" + ); + + let (fired_stack, fired_life, fired_monarch) = run(true); + assert_eq!( + fired_stack, 1, + "reachability proof: with the gate TRUE the same fixture reaches the stack" + ); + assert_eq!( + fired_monarch, + Some(PlayerId(0)), + "the gated primary effect resolves when the gate is true" + ); + assert_eq!( + fired_life, 21, + "non-deletion proof: the chained clause still resolves when the gate is true, \ + so the gated assertion above is not passing because the chain was dropped" + ); + } + + /// CR 603.4: the two legs of the hoist must be the SAME predicate over the + /// SAME values. A gate whose quantity reads a binding only the + /// resolution-time leg has — the delayed ability's snapshotted `targets` + /// (`ObjectScope::Target`), the per-iteration player + /// (`PlayerScope::ScopedPlayer`), or a population the trigger bridge + /// re-scopes (`FilterProp::Another` → `OtherThanTriggerObject`) — must + /// DECLINE the hoist rather than gate the ability off the stack on a value + /// the resolver would never have computed. + /// + /// Each case is a MINIMAL PAIR: the two halves differ only in the scope or + /// the one filter property. The `hoisted` half proves a `QuantityCheck` gate + /// really does bridge and really is evaluated at fire time (so the declined + /// half's `stack == 1` is the decline, not a non-bridging condition); the + /// `declined` half proves the divergent reading never reaches the gate. + /// + /// REVERT-TO-RED: drop the `gate_binding_diverges_at_fire_time` call from + /// `delayed_intervening_if` and every `declined` half below reports + /// `stack == 0` — the ability deleted off the stack on a fire-time reading + /// (empty targets / event-derived player / trigger-object exclusion) that the + /// resolution-time reader does not share. + #[test] + fn divergent_gate_bindings_decline_the_fire_time_hoist() { + fn run(condition: AbilityCondition) -> usize { + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + + let source = create_object( + &mut state, + CardId(0x0603_0407), + controller, + "Divergent Rider".to_string(), + Zone::Battlefield, + ); + let victim = make_creature(&mut state, PlayerId(1), "Doomed Squire", 1, 1); + + let mut ability = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + ability.condition = Some(condition); + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WhenDies { + filter: TargetFilter::Any, + }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + let death = zone_changed_event( + victim, + Zone::Battlefield, + Zone::Graveyard, + vec![CoreType::Creature], + Vec::new(), + ); + check_delayed_triggers(&mut state, &[death]); + state.stack.len() + } + + let counters_on = |scope| AbilityCondition::QuantityCheck { + lhs: QuantityExpr::Ref { + qty: QuantityRef::CountersOn { + scope, + counter_type: Some(crate::types::counter::CounterType::Plus1Plus1), + }, + }, + comparator: crate::types::ability::Comparator::GE, + rhs: QuantityExpr::Fixed { value: 3 }, + }; + // No object in the fixture carries a +1/+1 counter, so BOTH readings are + // false — the halves differ only in whether the gate is consulted at all. + assert_eq!( + run(counters_on(ObjectScope::Source)), + 0, + "CR 603.4: a Source-scoped counter gate binds identically on both legs, so it \ + hoists and gates the ability off the stack" + ); + assert_eq!( + run(counters_on(ObjectScope::Target)), + 1, + "CR 115.1: `ObjectScope::Target` reads the delayed ability's snapshotted \ + targets, which the fire-time context does not carry. 0 here means the hoist \ + gated the ability off the stack on an empty-targets reading" + ); + + let life_total = |player| AbilityCondition::QuantityCheck { + lhs: QuantityExpr::Ref { + qty: QuantityRef::LifeTotal { player }, + }, + comparator: crate::types::ability::Comparator::GE, + rhs: QuantityExpr::Fixed { value: 100 }, + }; + assert_eq!( + run(life_total(PlayerScope::Controller)), + 0, + "reach-guard: the same comparison scoped to the controller DOES hoist (nobody \ + has 100 life), so the declined half below is the scope and nothing else" + ); + assert_eq!( + run(life_total(PlayerScope::ScopedPlayer)), + 1, + "CR 115.10: `ScopedPlayer` is the RESOLVING ability's per-iteration player; \ + the fire-time context derives its scoped player from the event instead" + ); + + let controls_creature = |props: Vec| AbilityCondition::QuantityCheck { + lhs: QuantityExpr::Ref { + qty: QuantityRef::ObjectCount { + filter: TargetFilter::Typed( + TypedFilter::creature() + .controller(ControllerRef::You) + .properties(props), + ), + }, + }, + comparator: crate::types::ability::Comparator::GE, + rhs: QuantityExpr::Fixed { value: 1 }, + }; + assert_eq!( + run(controls_creature(vec![])), + 0, + "reach-guard: the unqualified population gate hoists (the controller controls \ + no creature), so the declined half below is the `Another` prop alone" + ); + assert_eq!( + run(controls_creature(vec![FilterProp::Another])), + 1, + "CR 603.4: `oracle_trigger` rewrites `Another` to `OtherThanTriggerObject` on \ + the fire-time leg only, so the two legs would count DIFFERENT populations" + ); + } + + /// CR 400.1 + CR 603.4: the ZONE axis of the same two-legs-must-agree rule. + /// + /// `ObjectCount{filter} >= 1` folds into `StaticCondition::IsPresent`, whose + /// affirmative trigger-side bridge is `TriggerCondition::ControlsType` — a + /// BATTLEFIELD-ONLY scan. The resolution leg counts in the FILTER'S OWN zone. + /// A gate on a graveyard population therefore reads TRUE at resolution and + /// FALSE at fire time, and hoisting it would gate the ability off the stack + /// (deleting a consumed one-shot) on a predicate the resolver never computes. + /// + /// MINIMAL PAIR: the two halves differ only in the `InZone { Graveyard }` + /// property. The battlefield half is the reach-guard — it proves an + /// `ObjectCount >= 1` gate really does bridge and really is evaluated at fire + /// time, so the graveyard half's `stack == 1` is the decline and nothing else. + /// + /// REVERT-TO-RED: drop the `static_gate_bridge_loses_zone` call from + /// `delayed_intervening_if` and the graveyard half reports `stack == 0` (and + /// `monarch == None`) — the ability deleted at fire time on a battlefield scan + /// of a graveyard population. + #[test] + fn non_battlefield_presence_gate_declines_the_fire_time_hoist() { + fn run(props: Vec) -> (usize, Option) { + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + + let source = create_object( + &mut state, + CardId(0x0603_0409), + controller, + "Graveyard Rider".to_string(), + Zone::Battlefield, + ); + // The counted population lives in the GRAVEYARD, never on the + // battlefield: the controller controls no creature there, so a + // battlefield scan of this filter is false either way. + let buried = create_object( + &mut state, + CardId(0x0603_040A), + controller, + "Buried Squire".to_string(), + Zone::Graveyard, + ); + { + let obj = state.objects.get_mut(&buried).expect("staged card"); + obj.card_types.core_types.push(CoreType::Creature); + obj.base_card_types = obj.card_types.clone(); + } + let victim = make_creature(&mut state, PlayerId(1), "Doomed Squire", 1, 1); + + let mut ability = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + ability.condition = Some(AbilityCondition::QuantityCheck { + lhs: QuantityExpr::Ref { + qty: QuantityRef::ObjectCount { + filter: TargetFilter::Typed( + TypedFilter::creature() + .controller(ControllerRef::You) + .properties(props), + ), + }, + }, + comparator: crate::types::ability::Comparator::GE, + rhs: QuantityExpr::Fixed { value: 1 }, + }); + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WhenDies { + filter: TargetFilter::SpecificObject { id: victim }, + }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + let death = zone_changed_event( + victim, + Zone::Battlefield, + Zone::Graveyard, + vec![CoreType::Creature], + Vec::new(), + ); + check_delayed_triggers(&mut state, &[death]); + let stack_len = state.stack.len(); + if stack_len == 1 { + let mut events = Vec::new(); + crate::game::stack::resolve_top(&mut state, &mut events); + } + (stack_len, state.monarch) + } + + let (battlefield_stack, battlefield_monarch) = run(vec![]); + assert_eq!( + battlefield_stack, 0, + "reach-guard: a battlefield-scoped `ObjectCount >= 1` gate bridges and IS \ + evaluated at fire time — the controller controls no creature, so CR 603.4 \ + keeps the ability off the stack" + ); + assert_eq!( + battlefield_monarch, None, + "nothing resolved on the battlefield-scoped half" + ); + + let (graveyard_stack, graveyard_monarch) = run(vec![FilterProp::InZone { + zone: Zone::Graveyard, + }]); + assert_eq!( + graveyard_stack, 1, + "CR 400.1 + CR 603.4: the affirmative `IsPresent` bridge is battlefield-only, \ + so a graveyard population would read FALSE at fire time while the resolver \ + reads TRUE. The hoist must decline and the ability must reach the stack" + ); + assert_eq!( + graveyard_monarch, + Some(PlayerId(0)), + "divergence proof: the RESOLUTION-time leg reads the same gate as TRUE (one \ + creature card in the controller's graveyard), so a fire-time deletion would \ + have destroyed an ability that was supposed to resolve" + ); + } + + /// CR 608.2c + CR 603.4: a RESOLUTION-SCOPED quantity leaf carries no scope, + /// player or filter payload, yet the fire-time leg cannot reproduce it — the + /// resolver is called with no resolving ability, no targets and no chain + /// ledger, so `TrackedSetSize` reads whatever an unrelated earlier resolution + /// left behind (here: nothing). + /// + /// MINIMAL PAIR against another PAYLOAD-FREE leaf whose reading is identical + /// on both legs (`TurnsTaken`), so the decline below cannot be explained by + /// "payload-free gates never hoist". + /// + /// REVERT-TO-RED: restore the `_ => false` tail of + /// `quantity_ref_binding_diverges` and the tracked-set half reports + /// `stack == 0` with `delayed_triggers` emptied — the one-shot deleted by + /// `false_gate_consumes_one_shot` on a value the resolver never computed. + #[test] + fn resolution_scoped_quantity_gate_declines_the_fire_time_hoist() { + // Unit pins for the two adjudications the production pair below drives. + assert!( + quantity_ref_binding_diverges(&QuantityRef::TrackedSetSize), + "CR 608.2c: the chain tracked set exists only during a resolution" + ); + assert!( + !quantity_ref_binding_diverges(&QuantityRef::TurnsTaken), + "CR 500: turns taken is global state both legs read identically" + ); + + fn run(qty: QuantityRef, threshold: i32) -> (usize, usize) { + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + + let source = create_object( + &mut state, + CardId(0x0603_040B), + controller, + "Tracked Rider".to_string(), + Zone::Battlefield, + ); + let victim = make_creature(&mut state, PlayerId(1), "Doomed Squire", 1, 1); + + let mut ability = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + ability.condition = Some(AbilityCondition::QuantityCheck { + lhs: QuantityExpr::Ref { qty }, + comparator: crate::types::ability::Comparator::GE, + rhs: QuantityExpr::Fixed { value: threshold }, + }); + state.delayed_triggers.push(DelayedTrigger { + // A bound single object, so a false gate would CONSUME the + // one-shot (`false_gate_consumes_one_shot`) rather than leave it + // installed — the deletion half of the defect. + condition: DelayedTriggerCondition::WhenDies { + filter: TargetFilter::SpecificObject { id: victim }, + }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + let death = zone_changed_event( + victim, + Zone::Battlefield, + Zone::Graveyard, + vec![CoreType::Creature], + Vec::new(), + ); + check_delayed_triggers(&mut state, &[death]); + (state.stack.len(), state.delayed_triggers.len()) + } + + // `TurnsTaken >= 100` is false on both legs — the reach-guard that a + // payload-free `QuantityCheck` gate does hoist and does gate at fire time. + let (turns_stack, turns_remaining) = run(QuantityRef::TurnsTaken, 100); + assert_eq!( + turns_stack, 0, + "reach-guard: a payload-free gate whose reading is leg-independent hoists, so \ + the declined half below is the RESOLUTION scope and nothing else" + ); + assert_eq!( + turns_remaining, 0, + "CR 603.7b: the bound-object one-shot is consumed by its own event" + ); + + let (tracked_stack, tracked_remaining) = run(QuantityRef::TrackedSetSize, 1); + assert_eq!( + tracked_stack, 1, + "CR 608.2c: `TrackedSetSize` is published BY a resolution; the fire-time leg \ + reads a foreign (here empty) ledger, so the gate must keep its \ + resolution-only reading and the ability must reach the stack" + ); + assert_eq!( + tracked_remaining, 0, + "the ability FIRED (it is off the delayed list because it went on the stack), \ + not because a false gate deleted it — see the stack assertion above" + ); + } + + /// CR 603.4 + CR 608.2c: `delayed_body_outlives_a_false_gate` inspects the + /// DIRECT sub only, because that is all `resolve_chain_body`'s + /// condition-false path inspects. A 2-deep chain whose direct sub does NOT + /// qualify resolves nothing at all, so the hoist must still apply even though + /// a GRANDCHILD carries an `else_ability`. + /// + /// REVERT-TO-RED: restore the `|| delayed_body_outlives_a_false_gate(sub)` + /// recursion and the gated half below reports `stack == 1` — an ability put + /// onto the stack (respondable, and CR 603.4 says it never triggered) to + /// resolve exactly nothing. + /// + /// The resolver-mirror assertion drives `effects::resolve_ability_chain` + /// directly on the SAME body with the SAME false gate and proves it performs + /// no work, so the hoist deletes nothing. + #[test] + fn two_deep_chain_mirrors_the_resolvers_direct_sub_test() { + /// "…, if [gate], become the monarch" whose direct sub is an ORDINARY + /// conditional continuation and whose GRANDCHILD carries an + /// `else_ability` (life gain) that only runs if the chain gets that far. + fn body(source: ObjectId, controller: PlayerId) -> ResolvedAbility { + let mut grandchild = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + grandchild.condition = Some(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }); + grandchild.else_ability = Some(Box::new(ResolvedAbility::new( + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + vec![], + source, + controller, + ))); + + let mut sub = ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + // NOT one of the surviving classes: an ordinary game-state gate on a + // continuation link, so `resolve_chain_body` stops here when the + // parent gate is false. + sub.condition = Some(AbilityCondition::IsYourTurn); + sub.sub_link = crate::types::ability::SubAbilityLink::ContinuationStep; + sub.sub_ability = Some(Box::new(grandchild)); + + let mut ability = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + ability.condition = Some(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }); + ability.sub_ability = Some(Box::new(sub)); + ability + } + + fn run(stage_commander: bool) -> (usize, i32, Option) { + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + state.players[0].life = 20; + + let source = create_object( + &mut state, + CardId(0x0603_040C), + controller, + "Two Deep Rider".to_string(), + Zone::Battlefield, + ); + if stage_commander { + let commander = make_creature(&mut state, controller, "Your Commander", 2, 2); + state + .objects + .get_mut(&commander) + .expect("staged commander") + .is_commander = true; + } + let victim = make_creature(&mut state, PlayerId(1), "Doomed Squire", 1, 1); + + let ability = body(source, controller); + // Reach-guard: the shape under test is a 2-deep chain whose DIRECT sub + // is not one of the surviving classes but whose grandchild is. + let direct = ability.sub_ability.as_deref().expect("staged direct sub"); + assert!( + !crate::game::effects::sub_outlives_false_parent_gate(direct), + "the direct sub must NOT qualify, or the hoist declines for the shallow \ + reason and the 2-deep case is never exercised" + ); + assert!( + direct + .sub_ability + .as_deref() + .is_some_and(|grandchild| grandchild.else_ability.is_some()), + "the GRANDCHILD must carry the surviving shape the old recursion tripped on" + ); + + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WhenDies { + filter: TargetFilter::Any, + }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + let death = zone_changed_event( + victim, + Zone::Battlefield, + Zone::Graveyard, + vec![CoreType::Creature], + Vec::new(), + ); + check_delayed_triggers(&mut state, &[death]); + let stack_len = state.stack.len(); + if stack_len == 1 { + let mut events = Vec::new(); + crate::game::stack::resolve_top(&mut state, &mut events); + } + (stack_len, state.players[0].life, state.monarch) + } + + let (gated_stack, gated_life, gated_monarch) = run(false); + assert_eq!( + gated_stack, 0, + "CR 603.4: with the gate false the resolver would run NOTHING (the direct sub \ + does not survive), so the fire-time half must gate the ability off the stack" + ); + assert_eq!(gated_life, 20, "no chain work happened"); + assert_eq!(gated_monarch, None, "no chain work happened"); + + // Resolver mirror: the same body, the same false gate, resolved directly. + // Nothing runs — which is exactly why hoisting deletes nothing. + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + state.players[0].life = 20; + let source = create_object( + &mut state, + CardId(0x0603_040D), + controller, + "Two Deep Rider".to_string(), + Zone::Battlefield, + ); + let mut events = Vec::new(); + crate::game::effects::resolve_ability_chain( + &mut state, + &body(source, controller), + &mut events, + 0, + ) + .expect("chain resolution"); + assert_eq!( + state.players[0].life, 20, + "CR 608.2c: `resolve_chain_body` stops at the non-surviving DIRECT sub, so the \ + grandchild's else branch never runs — 21 here would mean the hoist above \ + deleted printed work" + ); + assert_eq!( + state.monarch, None, + "the gated primary effect and the gated grandchild both stay unperformed" + ); + + let (fired_stack, _fired_life, fired_monarch) = run(true); + assert_eq!( + fired_stack, 1, + "reachability proof: the SAME fixture with the gate TRUE reaches the stack" + ); + assert_eq!( + fired_monarch, + Some(PlayerId(0)), + "and resolves its gated effect" + ); + } + + /// CR 603.4 + CR 702.1c: `delayed_body_outlives_a_false_gate` and + /// `resolve_chain_body` must decline / run on EXACTLY the same sub shapes. + /// The `ReplicatedOrBranch` marker only means "independent OR-branch" on a + /// `SequentialSibling` link — that is the conjunct + /// `effects::sub_outlives_false_parent_gate` now carries for BOTH consumers. + /// + /// REVERT-TO-RED: drop the `sub_link == SequentialSibling` conjunct from + /// `sub_outlives_false_parent_gate` and the `ContinuationStep` half below + /// reports `stack == 1` — the hoist declined for a sub the resolver would + /// never have run through a false gate, leaving CR 603.4 unenforced. + #[test] + fn replicated_or_branch_carve_out_requires_a_sequential_sibling_link() { + fn run(sub_link: crate::types::ability::SubAbilityLink) -> usize { + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + + let source = create_object( + &mut state, + CardId(0x0603_0408), + controller, + "Replicated Rider".to_string(), + Zone::Battlefield, + ); + let victim = make_creature(&mut state, PlayerId(1), "Doomed Squire", 1, 1); + + let mut ability = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + // Gate FALSE: no commander is staged anywhere. + ability.condition = Some(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }); + let mut sub = ResolvedAbility::new( + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + vec![], + source, + controller, + ); + sub.sibling_condition = crate::types::ability::SiblingCondition::ReplicatedOrBranch; + sub.sub_link = sub_link; + // Reach-guard: the sub must carry the replication marker, or both + // halves take the plain no-sub branch and the pair proves nothing. + assert_eq!( + sub.sibling_condition, + crate::types::ability::SiblingCondition::ReplicatedOrBranch + ); + ability.sub_ability = Some(Box::new(sub)); + + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WhenDies { + filter: TargetFilter::Any, + }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + let death = zone_changed_event( + victim, + Zone::Battlefield, + Zone::Graveyard, + vec![CoreType::Creature], + Vec::new(), + ); + check_delayed_triggers(&mut state, &[death]); + state.stack.len() + } + + assert_eq!( + run(crate::types::ability::SubAbilityLink::SequentialSibling), + 1, + "CR 702.1c: a replicated OR-branch on a SequentialSibling link is exactly what \ + `resolve_chain_body` still runs through a false parent gate, so the hoist \ + must decline and the ability must still reach the stack" + ); + assert_eq!( + run(crate::types::ability::SubAbilityLink::ContinuationStep), + 0, + "CR 603.4: on a ContinuationStep link the resolver would NOT have run the sub \ + either, so nothing survives the false gate and the whole ability must be \ + gated off the stack" + ); + } + + /// CR 603.4 + CR 903.3d for the NEGATED commander gate ("if you don't + /// control your commander"). + /// + /// The hoist composes two bridges, and only the FIRST had a negated + /// commander arm: `ability_condition_to_static_condition` produced + /// `Not{ControlsCommander}` while + /// `static_condition_to_trigger_condition`'s `Not` sub-match fell through to + /// `_ => None`, so `delayed_intervening_if` returned `None` and the negated + /// gate kept only the resolution-time half of CR 603.4. + /// + /// REVERT-TO-RED: remove the `StaticCondition::ControlsCommander` arm from + /// that `Not` sub-match (`oracle_trigger.rs`) and the no-commander half below + /// reports `stack == 1` — the ability was put onto the stack instead of + /// having never triggered. + #[test] + fn negated_delayed_intervening_if_gates_at_fire_time_too() { + fn run(stage_commander: bool) -> usize { + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + + let source = create_object( + &mut state, + CardId(0x0603_0405), + controller, + "Negated Rider".to_string(), + Zone::Battlefield, + ); + if stage_commander { + let commander = make_creature(&mut state, controller, "Your Commander", 2, 2); + state + .objects + .get_mut(&commander) + .expect("staged commander") + .is_commander = true; + } + let victim = make_creature(&mut state, PlayerId(1), "Doomed Squire", 1, 1); + + let mut ability = + ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + ability.condition = Some(AbilityCondition::Not { + condition: Box::new(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }), + }); + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WhenDies { + filter: TargetFilter::Any, + }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + let death = zone_changed_event( + victim, + Zone::Battlefield, + Zone::Graveyard, + vec![CoreType::Creature], + Vec::new(), + ); + check_delayed_triggers(&mut state, &[death]); + state.stack.len() + } + + assert_eq!( + run(true), + 0, + "CR 603.4: with a commander on the battlefield the NEGATED gate is false at \ + fire time, so the ability must never be put onto the stack" + ); + assert_eq!( + run(false), + 1, + "reachability proof: with no commander the negated gate is TRUE and the same \ + fixture reaches the stack" + ); + } + + /// HOSTILE fixture for the narrowed carve-out's OTHER arm (CR 603.12). + /// + /// A sub whose own gate is a performed/reflexive one ("…, if you do, …" — + /// Council's Deliberation's shape, the only conditioned delayed body with a + /// sub in the pool today) is re-evaluated on its own at resolution, so + /// `delayed_body_outlives_a_false_gate` conservatively declines the CR 603.4 + /// hoist and this class keeps byte-identical behaviour: the ability still + /// reaches the stack even with the parent gate false. + /// + /// REVERT-TO-RED: drop the `condition_survives_false_parent_gate` arm and the + /// hoist fires, the stack stays empty, and the in-pool reflexive shape changes + /// behaviour as a side effect of a commander-gate fix. + #[test] + fn delayed_body_with_a_reflexive_sub_is_never_gated_off_the_stack() { + let mut state = setup(); + let controller = PlayerId(0); + state.active_player = controller; + state.priority_player = controller; + + let source = create_object( + &mut state, + CardId(0x0603_0404), + controller, + "Reflexive Rider".to_string(), + Zone::Battlefield, + ); + let victim = make_creature(&mut state, PlayerId(1), "Doomed Squire", 1, 1); + + let mut ability = ResolvedAbility::new(Effect::BecomeMonarch, vec![], source, controller); + ability.condition = Some(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }); + let mut sub = ResolvedAbility::new( + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + vec![], + source, + controller, + ); + sub.condition = Some(AbilityCondition::effect_performed()); + // Reach-guard: the carve-out is keyed on this classification, not on the + // mere presence of a sub. + assert!( + crate::game::effects::condition_survives_false_parent_gate( + sub.condition.as_ref().expect("staged reflexive gate") + ), + "the fixture must carry a gate the shared authority classifies as \ + surviving a false parent gate" + ); + ability.sub_ability = Some(Box::new(sub)); + + state.delayed_triggers.push(DelayedTrigger { + condition: DelayedTriggerCondition::WhenDies { + filter: TargetFilter::Any, + }, + ability: Box::new(ability), + controller, + source_id: source, + one_shot: true, + provenance: DelayedInstallIdentity::LegacyDelayed, + }); + + let death = zone_changed_event( + victim, + Zone::Battlefield, + Zone::Graveyard, + vec![CoreType::Creature], + Vec::new(), + ); + check_delayed_triggers(&mut state, &[death]); + assert_eq!( + state.stack.len(), + 1, + "a body whose sub carries a CR 603.12 performed gate keeps its resolution-time \ + reading and must still reach the stack" + ); + } + #[test] fn delayed_phase_trigger_batches_with_normal_phase_trigger_before_priority() { let mut state = setup(); diff --git a/crates/engine/src/game/triggers_ordering_parity_tests.rs b/crates/engine/src/game/triggers_ordering_parity_tests.rs index 9a0a1c2923..86bdabf98d 100644 --- a/crates/engine/src/game/triggers_ordering_parity_tests.rs +++ b/crates/engine/src/game/triggers_ordering_parity_tests.rs @@ -232,6 +232,48 @@ const DOCUMENTED_OVER_PROMPT: &[&str] = &[ // Exploration misparse-fix fixture regen surfaced this card in the sweep // corpus; same adjudication as the #7031 branch). "arashin sovereign", + // ---- commander-census read × token-creation write (CR 903.3d) ---- + // "At the beginning of your upkeep, if you control a commander, create a token + // that's a copy of this creature." The CR 603.4 intervening-if is a LIVE + // battlefield census (`TriggerCondition::ControlsCommander` ⇒ + // `commander_control_read`, ability_rw.rs), and each member WRITES battlefield + // membership by minting a token — a structural `SetMembership` read × creation + // feed, so `profiles_conflict` prompts. + // + // CONSERVATIVE: the created object is a token copy of Biowaste Blob, which is + // never a commander — CR 903.3's own example is exactly this ("A permanent + // that's copying a commander … is not a commander"), because the designation + // is an attribute of the CARD rather than a characteristic, and CR 707.2's + // copiable values are only the printed characteristics. So no member's write + // can flip another member's gate, and the two creations commute up to + // relabeling. Proving + // that needs "this creation cannot produce a commander", which no context-free + // recognizer in scope can see: the read's census is `Census::Any` (a commander + // may be any permanent type, CR 903.3), so the census/zone/controller + // disjointness rows all overlap. Fail-closed prompt, never an under-prompt. + // + // The alternative — declaring the commander gate read-free — is exactly the + // unsound classification this ledger row exists to avoid: a sibling that + // MOVES, steals or phases out a real commander genuinely flips the gate. + // + // CONSUMPTION: this row is gated like every other one — the exact-set + // `DOCUMENTED_OVER_PROMPT` assertion runs BEFORE the STRICT PROOF-GATE (see + // `ordering_parity_sweep`), so an unrelated red gate can no longer leave a + // newly-added row unverified. That ordering was introduced with this row for + // exactly that reason: at the time it landed the sweep was red on 18 + // unexplained same-event over-prompts (`hidden *` / `opal *` / `veiled *` / + // `lurking skirge` / `impending disaster` / `veil of birds`) and the + // completeness assertion downstream of the gate never executed. Those 18 are + // CORPUS DRIFT, not this change: every one carries + // `TriggerCondition::SourceMatchesFilter` or no trigger condition, ZERO carry + // `ControlsCommander` (checked against the full export), and this change's + // classification edits are confined to the `ControlsCommander` arms of + // `rw_trigger_condition` / `rw_static_condition` / `rw_ability_condition` and + // `scan_*_condition`. They must land as their own corpus-drift rows, NOT + // folded in here. The `over_prompt_hit members` line printed above the + // assertion names the consumed set directly, so the next maintainer need not + // re-derive it. + "biowaste blob", ]; /// Batch-depth GENUINE order-dependence (kept SEPARATE from the same-event @@ -785,6 +827,46 @@ fn ordering_parity_sweep() { se_event_object_class.len(), se_event_object_class ); + // CONSUMED `DOCUMENTED_OVER_PROMPT` membership, emitted as evidence for the same + // reason the two class memberships above are: it names exactly which ledger rows + // the corpus hit on this run. `over_prompt_hit` is a subset of + // `DOCUMENTED_OVER_PROMPT` by construction (both insertion sites are inside a + // `DOCUMENTED_OVER_PROMPT.contains(..)` arm), so this line plus the const is a + // complete consumption proof on its own. Evidence only — the exact-set assertion + // immediately below is the gate. + eprintln!( + "over_prompt_hit members ({}/{}): {:?}", + over_prompt_hit.len(), + DOCUMENTED_OVER_PROMPT.len(), + over_prompt_hit + ); + + // LEDGER COMPLETENESS, adjudicated BEFORE the STRICT PROOF-GATE below (full-DB + // only — the committed fixture lacks these cards). Every allowlist entry MUST be + // consumed: an unhit entry is stale bookkeeping (the card cleared — remove it — + // or its name drifted). + // + // Ordering is load-bearing, not cosmetic. The strict gate panics on ANY + // unexplained decision diff, including pure corpus drift that has nothing to do + // with the ledger; while it is red, a `DOCUMENTED_OVER_PROMPT` row added by + // ordinary card work would never be checked at all, so a stale or mis-keyed row + // could ride along unverified and only surface later, attributed to whoever + // cleared the unrelated drift. Checking consumption first makes every ledger row + // gated on every full-DB run. The gate's own diff count is carried in this + // message so a simultaneous drift is never hidden by an earlier panic. + if full_db { + let expected_over_prompt: BTreeSet = DOCUMENTED_OVER_PROMPT + .iter() + .map(|s| s.to_string()) + .collect(); + assert_eq!( + over_prompt_hit, + expected_over_prompt, + "DOCUMENTED_OVER_PROMPT stale/unhit entries ({} unexplained decision diff(s) \ + also pending at the STRICT PROOF-GATE below)", + unexplained.len() + ); + } assert!( unexplained.is_empty(), @@ -847,14 +929,9 @@ fn ordering_parity_sweep() { // Ledger completeness (full-DB only — the committed fixture lacks these // cards). Every allowlist entry MUST be consumed: an unhit entry is stale // bookkeeping (the card cleared — remove it — or its name drifted). - let expected_over_prompt: BTreeSet = DOCUMENTED_OVER_PROMPT - .iter() - .map(|s| s.to_string()) - .collect(); - assert_eq!( - over_prompt_hit, expected_over_prompt, - "DOCUMENTED_OVER_PROMPT stale/unhit entries" - ); + // `DOCUMENTED_OVER_PROMPT`'s exact-set assertion is deliberately NOT here: + // it runs ABOVE the STRICT PROOF-GATE (see the comment there) so unrelated + // corpus drift cannot leave a newly-added over-prompt row unverified. let expected_batch_genuine: BTreeSet = BATCH_GENUINE_ROWS.iter().map(|s| s.to_string()).collect(); assert_eq!( @@ -1261,6 +1338,79 @@ fn n_d_intervening_if_case_a_prompts() { ); } +/// N-D2: the commander intervening-`if` is a LIVE BOARD CENSUS (CR 903.3d), so a +/// sibling copy whose resolution writes battlefield membership FEEDS it and the +/// group must PROMPT (CR 603.3b). Biowaste Blob's exact shape ("At the beginning of +/// your upkeep, if you control a commander, create a token that's a copy of this +/// creature") — the card the full-DB sweep's `DOCUMENTED_OVER_PROMPT` row names. +/// +/// `game::commander::controls_any_commander` / `controls_own_commander` scan +/// `state.battlefield` for `is_commander && controller [&& owner] && +/// is_phased_in`. Only the designation and owner conjuncts are frozen card +/// attributes (CR 903.3); membership, control and CR 702.26b phased-in status are +/// all sibling-mutable, so the gate is NOT read-free. +/// +/// BOTH `CommanderOwnership` arms are driven. Biowaste Blob's printed gate is +/// "if you control **a** commander" ⇒ `Any` (verified against +/// `data/mtgjson/AtomicCards.json`), which is the arm the paired +/// `DOCUMENTED_OVER_PROMPT` ledger row actually exercises; `Own` is the arm every +/// card in the pool that says "your commander" (Fight for the Throne) produces. +/// `commander_control_read` is shared by both, so a regression on either arm +/// alone must red here. +/// +/// REVERT-TO-RED: classify `TriggerCondition::ControlsCommander` as +/// `RwProfile::empty()` again (its pre-fix profile) and BOTH gated groups +/// auto-order — CR 603.3b sibling-ordering silently reopened. +/// +/// The second group per arm is the discrimination proof and pins that the prompt +/// is the GATE's read, not the ability: the byte-identical ability with NO +/// condition still auto-orders. (Both groups are source-DEPENDENT — +/// `CopyTokenOf{SelfRef}` — so neither takes the T1 `source_independent` fast +/// path, which is what makes the feed row observable at all.) +#[test] +fn n_d2_commander_intervening_if_is_fed_by_a_sibling_membership_write() { + let state = empty_state(); + let copy_self = || Effect::CopyTokenOf { + target: TargetFilter::SelfRef, + owner: TargetFilter::Controller, + source_filter: None, + enters_attacking: false, + tapped: false, + count: qfix(1), + extra_keywords: vec![], + additional_modifications: vec![], + }; + let ev = shared_etb_event(); + + for ownership in [ + // CR 903.3d "a commander" — Biowaste Blob, the `DOCUMENTED_OVER_PROMPT` row. + crate::types::ability::CommanderOwnership::Any, + // CR 903.3 + CR 109.5 "your commander" — Fight for the Throne. + crate::types::ability::CommanderOwnership::Own, + ] { + let gate = TriggerCondition::ControlsCommander { ownership }; + let gated = vec![ + ctx(10, ra(copy_self()), Some(gate.clone()), ev.clone(), None), + ctx(11, ra(copy_self()), Some(gate), ev.clone(), None), + ]; + assert!( + !group_is_order_independent(&state, &gated), + "CR 903.3d + CR 603.3b: the commander census the CR 603.4 gate reads is fed by \ + a sibling's battlefield-membership write ⇒ prompt (ownership = {ownership:?})" + ); + } + + let ungated = vec![ + ctx(10, ra(copy_self()), None, ev.clone(), None), + ctx(11, ra(copy_self()), None, ev, None), + ]; + assert!( + group_is_order_independent(&state, &ungated), + "discrimination: the SAME ability with no intervening-if auto-orders, so the \ + prompts above are the commander gate's board read and nothing else" + ); +} + /// N-F: die_result conjunct unit pin (CR 706.2 + CR 603.12). Two otherwise- /// identical no-input source-independent triggers off one event with DIFFERENT /// stamped die results are NOT the same state transformation ⇒ not order- diff --git a/crates/engine/src/parser/oracle_effect/conditions.rs b/crates/engine/src/parser/oracle_effect/conditions.rs index 3d16bc3f1c..072ac7b7eb 100644 --- a/crates/engine/src/parser/oracle_effect/conditions.rs +++ b/crates/engine/src/parser/oracle_effect/conditions.rs @@ -4570,6 +4570,25 @@ pub(crate) fn static_condition_to_ability_condition( StaticCondition::HasCityBlessing => Some(AbilityCondition::HasCityBlessing), // CR 702.195b: The enduring story designation is available to effects. StaticCondition::HasEnduringStory => Some(AbilityCondition::HasEnduringStory), + // CR 903.3d ("If an effect refers to controlling a commander, it refers to + // a permanent on the battlefield that is a commander") + CR 608.2c: + // commander control is a plain game-state predicate about the resolving + // ability's CONTROLLER, evaluated as the ability resolves — it is not + // source-bound, layer-bound, or cost-bound like the unbridgeable statics + // below. `AbilityCondition::ControlsCommander` is its exact + // effect-resolution equivalent and carries the same `ownership` axis, so + // CR 903.3's owner-scoped "your commander" reading survives lowering. + // + // Listing it among the "no equivalent -> None" arms was a vocabulary + // asymmetry — the same defect the `CompletedADungeon` arm below fixed — + // and it silently dropped Fight for the Throne's intervening-if "if you + // control your commander", making the delayed trigger grant the monarch + // unconditionally. + StaticCondition::ControlsCommander { ownership } => { + Some(AbilityCondition::ControlsCommander { + ownership: *ownership, + }) + } StaticCondition::IsRingBearer => Some(AbilityCondition::IsRingBearer), StaticCondition::OpponentPoisonAtLeast { count } => { Some(opponent_poison_at_least_as_quantity_check(*count)) @@ -4828,7 +4847,6 @@ pub(crate) fn static_condition_to_ability_condition( | StaticCondition::UnlessPay { .. } | StaticCondition::Unrecognized { .. } | StaticCondition::RingLevelAtLeast { .. } - | StaticCondition::ControlsCommander { .. } | StaticCondition::EnchantedIsFaceDown // CR 311.2 / CR 901.7: plane face-up status is a duration-only continuous- // effect condition (evaluated in the layer system), never an @@ -4903,6 +4921,21 @@ pub(crate) fn ability_condition_to_static_condition( AbilityCondition::SourceAttachedToCreature => { Some(StaticCondition::SourceAttachedToCreature) } + // CR 903.3d: round-trips `static_condition_to_ability_condition`'s + // commander arm, carrying the CR 903.3 `ownership` axis unchanged so + // "your commander" can never widen to "a commander" in either direction. + // + // This inverse has TWO consumers, not just the per-`StaticDefinition` + // keyword-grant push-down: `triggers::delayed_intervening_if` composes it + // with `static_condition_to_trigger_condition` to recover the CR 603.4 + // fire-time reading of a delayed triggered ability's intervening-`if` + // (Fight for the Throne). Declining here would leave that half of CR 603.4 + // unenforceable. + AbilityCondition::ControlsCommander { ownership } => { + Some(StaticCondition::ControlsCommander { + ownership: *ownership, + }) + } AbilityCondition::QuantityCheck { lhs, comparator, @@ -7279,9 +7312,95 @@ mod tests { use super::*; use crate::parser::oracle_nom::condition::parse_inner_condition; use crate::parser::parse_oracle_text; - use crate::types::ability::{AggregateFunction, PlayerFilter, SharedQuality}; + use crate::types::ability::{ + AggregateFunction, CommanderOwnership, PlayerFilter, SharedQuality, + }; use crate::types::counter::{CounterMatch, CounterType}; + /// CR 903.3d + CR 603.4: the `StaticCondition` -> `AbilityCondition` bridge + /// must lower a commander-control gate, and must keep the two `ownership` + /// arms DISTINCT — CR 903.3 + CR 109.5 "your commander" (owned and + /// controlled) is a strictly narrower predicate than CR 903.3d "a commander" + /// (controlled, any owner). A bridge that collapsed the arms would pass a + /// single-arm test and fail this one. + /// + /// The inverse assertions pin the ROUND TRIP. The inverse is not merely the + /// keyword-grant push-down any more: `triggers::delayed_intervening_if` + /// composes `ability_condition_to_static_condition` with + /// `static_condition_to_trigger_condition` to recover the CR 603.4 fire-time + /// reading of a delayed triggered ability's intervening-`if`. A declined + /// inverse silently disables that half of CR 603.4, so the round trip is + /// load-bearing and asserted in both directions, per `ownership` arm. + #[test] + fn commander_control_bridges_round_trip_preserving_ownership() { + for ownership in [CommanderOwnership::Own, CommanderOwnership::Any] { + assert_eq!( + static_condition_to_ability_condition( + &StaticCondition::ControlsCommander { ownership }, + &mut ParseContext::default(), + ), + Some(AbilityCondition::ControlsCommander { ownership }), + "CR 903.3d: the {ownership:?} commander gate must lower to its exact \ + effect-resolution equivalent, preserving the ownership axis" + ); + assert_eq!( + ability_condition_to_static_condition(&AbilityCondition::ControlsCommander { + ownership + }), + Some(StaticCondition::ControlsCommander { ownership }), + "CR 903.3: the {ownership:?} gate must invert back to its exact static \ + counterpart — the CR 603.4 fire-time hoist composes this inverse with \ + static_condition_to_trigger_condition" + ); + assert_eq!( + crate::parser::oracle_trigger::static_condition_to_trigger_condition( + &StaticCondition::ControlsCommander { ownership } + ), + Some(crate::types::ability::TriggerCondition::ControlsCommander { ownership }), + "CR 603.4: the second leg of the hoist must also preserve {ownership:?}" + ); + + // CR 603.4 + CR 903.3d, NEGATED form ("if you don't control your + // commander"). Both legs must carry the negation, or a negated gate + // round-trips through the first leg and dies at the second — leaving + // `delayed_intervening_if` with only the RESOLUTION-time half of + // CR 603.4 for that card, silently. + let negated_static = StaticCondition::Not { + condition: Box::new(StaticCondition::ControlsCommander { ownership }), + }; + let negated_ability = AbilityCondition::Not { + condition: Box::new(AbilityCondition::ControlsCommander { ownership }), + }; + assert_eq!( + static_condition_to_ability_condition( + &negated_static, + &mut ParseContext::default() + ), + Some(negated_ability.clone()), + "the negated {ownership:?} gate must lower with its negation intact" + ); + assert_eq!( + ability_condition_to_static_condition(&negated_ability), + Some(negated_static.clone()), + "the negated {ownership:?} gate must invert back to its exact static \ + counterpart" + ); + assert_eq!( + crate::parser::oracle_trigger::static_condition_to_trigger_condition( + &negated_static + ), + Some(crate::types::ability::TriggerCondition::Not { + condition: Box::new( + crate::types::ability::TriggerCondition::ControlsCommander { ownership } + ), + }), + "CR 603.4: the second leg must bridge the NEGATED {ownership:?} gate too — \ + a `None` here is the asymmetry that makes a negated commander \ + intervening-if resolution-only" + ); + } + } + /// CR 608.2c: Aven Courier's chosen-counter predicate depends on a value /// selected by the immediately preceding instruction. The generic suffix /// condition parser has no such binding and must leave the whole clause for diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index 9193d96e8a..e33ab0e900 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -10,8 +10,9 @@ use crate::parser::parse_oracle_text; use crate::types::ability::CardPlayMode::{Cast, Play}; use crate::types::ability::CastFromZoneDriver::{DuringResolution, LingeringPermission}; use crate::types::ability::{ - AttachmentKind, CardSelectionMode, CastManaObjectScope, CastManaSpentMetric, DigRestOrder, - ExcessRecipient, ForEachCategoryAction, ModalChoice, PerpetualModification, SeatDirection, + AttachmentKind, CardSelectionMode, CastManaObjectScope, CastManaSpentMetric, + CommanderOwnership, DigRestOrder, ExcessRecipient, ForEachCategoryAction, ModalChoice, + PerpetualModification, SeatDirection, }; use crate::types::card_type::CoreType; use crate::types::mana::{ManaCost, ManaCostShard}; @@ -22130,6 +22131,56 @@ fn inline_delayed_trigger_when_damage_this_way_dies_uses_damage_condition() { } } +/// CR 603.7b: the SIBLING of the test above, and the reason that one is allowed +/// to lower onto the one-shot `WhenNextEvent`. +/// +/// CR 603.7b caps a delayed ability at one trigger "unless it has a stated +/// duration, such as 'this turn.'" Both cards here carry the same stated +/// duration and the same broad "a creature dealt damage this way" filter, so the +/// filter cannot be the discriminator — the WotC "When" / "Whenever" templating +/// is, and `parse_dealt_damage_this_way_dies_trigger` is reached from two call +/// sites that lower it accordingly: +/// - "When …" (Skeletonize, damage to a SINGLE target creature, so at most one +/// death can qualify) → one-shot `WhenNextEvent`, pinned above; +/// - "Whenever …" (Ghired's Belligerence, damage DIVIDED among any number of +/// creatures, so many deaths can qualify) → multi-fire `WheneverEvent`, +/// pinned here. +/// +/// This split is what makes `game::triggers::false_gate_consumes_one_shot` safe +/// to consume EVERY `WhenNextEvent` on a false intervening-`if` (CR 603.4): an +/// ability that must keep watching for the rest of the turn is a `WheneverEvent`, +/// whose `one_shot` is false, so it never reaches that discard. Re-route this +/// form onto `WhenNextEvent` and that guarantee is silently lost — which is why +/// the routing is pinned here rather than left to the call sites. +#[test] +fn inline_delayed_trigger_whenever_damage_this_way_dies_is_multi_fire() { + let e = parse_effect( + "Whenever a creature dealt damage this way dies this turn, create a 1/1 black Skeleton creature token", + ); + match e { + Effect::CreateDelayedTrigger { + condition: DelayedTriggerCondition::WheneverEvent { trigger, .. }, + .. + } => { + // Same event shape as the "When" form — only the multiplicity differs. + assert_eq!(trigger.mode, TriggerMode::ChangesZone); + assert_eq!(trigger.origin, Some(Zone::Battlefield)); + assert_eq!(trigger.destination, Some(Zone::Graveyard)); + assert_eq!( + trigger.condition, + Some(TriggerCondition::DealtDamageBySourceThisTurn), + "the 'whenever' form must reuse the same damage-ledger condition as the \ + 'when' form; only its multiplicity differs" + ); + } + other => panic!( + "CR 603.7b: a stated-duration 'whenever … dealt damage this way dies this turn' \ + must lower to the multi-fire WheneverEvent, not the one-shot WhenNextEvent, \ + got {other:?}" + ), + } +} + #[test] fn inline_delayed_trigger_when_leaves() { let e = parse_effect("When that creature leaves the battlefield, return it to the battlefield under its owner's control"); @@ -52297,6 +52348,86 @@ fn instead_override_lowers_the_typed_completed_dungeon_condition() { ); } +/// CR 603.4 + CR 903.3: Fight for the Throne's delayed triggered +/// ability carries an intervening-`if` — "When the creature an opponent +/// controls dies this turn, *if you control your commander*, you become the +/// monarch." The gate must survive lowering as a TYPED `AbilityCondition`. +/// +/// This is the same vocabulary asymmetry the `CompletedADungeon` test above +/// pins, one variant over: `parse_inner_condition` produced +/// `StaticCondition::ControlsCommander` all along, but the StaticCondition -> +/// AbilityCondition bridge declared it had "no effect-resolution equivalent", +/// so `strip_leading_general_conditional` dropped it on the floor and the +/// delayed trigger made you the monarch UNCONDITIONALLY. +/// +/// SHAPE test — it pins the parsed AST only. The runtime semantics, including +/// the `Own`-vs-`Any` stolen-commander discrimination, are covered by +/// `tests/integration/fight_for_the_throne_monarch_gated_on_commander.rs`, +/// which drives the real cast pipeline. +#[test] +fn delayed_trigger_intervening_if_retains_the_commander_control_gate() { + let parsed = parse_oracle_text( + "Put a +1/+1 counter on target creature you control. Then it fights target \ + creature an opponent controls. When the creature an opponent controls dies \ + this turn, if you control your commander, you become the monarch.", + "Fight for the Throne", + &[], + &["Instant".to_string()], + &[], + ); + let ability = parsed.abilities.first().expect("expected a spell ability"); + + // Reach-guard: every clause must lower for real, so the assertions below are + // read off a fully-parsed chain rather than off an honest-failure stub. + let _ = crate::types::ability_visit::visit_ability_def(ability, &mut |effect| { + assert!( + !matches!(effect, Effect::Unimplemented { .. }), + "no clause of Fight for the Throne may lower to Unimplemented: {effect:#?}" + ); + std::ops::ControlFlow::Continue(()) + }); + + let mut delayed = None; + let _ = crate::types::ability_visit::visit_ability_def(ability, &mut |effect| { + if let Effect::CreateDelayedTrigger { + condition, effect, .. + } = effect + { + delayed = Some((condition.clone(), (**effect).clone())); + return std::ops::ControlFlow::Break(()); + } + std::ops::ControlFlow::Continue(()) + }); + let (delayed_condition, delayed_body) = + delayed.expect("the third sentence must lower to a CreateDelayedTrigger"); + + // CR 603.7c unchanged-behaviour guard: the fought creature stays the trigger's + // referent. Restoring the gate must not disturb the already-correct binding. + assert_eq!( + delayed_condition, + DelayedTriggerCondition::WhenDies { + filter: TargetFilter::ParentTarget + }, + "the delayed trigger must still watch the fought creature" + ); + assert_eq!( + *delayed_body.effect, + Effect::BecomeMonarch, + "the delayed body must still be the monarch effect" + ); + // CR 903.3 + CR 109.5: the regression assertion. Reverting the bridge arm in + // `static_condition_to_ability_condition` makes this `None`, which is exactly + // the shipped misparse — an unconditional monarch grant. + assert_eq!( + delayed_body.condition, + Some(AbilityCondition::ControlsCommander { + ownership: CommanderOwnership::Own, + }), + "the intervening-if must ride the delayed body as a typed OWNER-scoped \ + commander gate" + ); +} + /// CR 608.2c + CR 614.6: the honest-failure path is CLAUSE-scoped — an "instead" /// override must never absorb the sentence BEFORE it, and must never be emitted /// as an unconditional sequel to the effect it is supposed to REPLACE. diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 7bd5b6a774..5b1d1a7cc1 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -554,6 +554,9 @@ fn rewrite_cost_x_in_condition(cond: &mut crate::types::ability::AbilityConditio | AbilityCondition::IsInitiative | AbilityCondition::HasCityBlessing | AbilityCondition::HasEnduringStory + // CR 903.3d: carries no `QuantityExpr` and nests no condition — nothing + // for the cost-X rewrite to bind. + | AbilityCondition::ControlsCommander { .. } | AbilityCondition::DiscardedCardMatchesFilter { .. } | AbilityCondition::IsRingBearer | AbilityCondition::CompletedDungeon { .. } @@ -4623,6 +4626,18 @@ pub(crate) fn static_condition_to_trigger_condition( variant: *variant, }), }), + // CR 903.3d + CR 603.4: "if you don't control your/a commander". The + // affirmative arm bridges below; without this one the negated form + // round-trips through `ability_condition_to_static_condition` and then + // dies here, so `triggers::delayed_intervening_if` would silently keep + // only the RESOLUTION-time half of CR 603.4 for a negated commander + // gate. `ownership` rides through unchanged — CR 903.3 "your commander" + // must not widen to CR 903.3d "a commander" under negation either. + StaticCondition::ControlsCommander { ownership } => Some(TriggerCondition::Not { + condition: Box::new(TriggerCondition::ControlsCommander { + ownership: *ownership, + }), + }), _ => None, }, diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 17b72b0729..77b7535603 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -15346,6 +15346,33 @@ impl TargetFilter { } } + /// CR 603.7c + CR 400.7: Returns true when this filter names ONE object that + /// is already BOUND — a specific permanent or an anaphor to one — rather than + /// a CLASS of objects re-matched against the live board on every check. + /// + /// Used where "how many times can an event matching this filter occur?" is + /// the question, e.g. CR 603.7b's one-shot delayed triggers: a bound object + /// dies, leaves, or enters exactly once per incarnation (a returning object + /// is a NEW object, CR 400.7 / CR 603.7c), whereas a class filter ("a + /// creature") can match an unbounded number of later occurrences. + /// + /// Deliberately NARROWER than the parser's `is_single_object_ref`: that + /// helper also admits `TriggeringSource`, which is re-resolved from whichever + /// event is being examined and is therefore NOT bound in advance, and it has + /// no reason to admit `SpecificObject` / `ParentTargetSlot`. `TrackedSet` + /// and `LastCreated` are excluded for the same reason as a class filter — + /// they can name several objects, so several occurrences. + pub fn names_bound_single_object(&self) -> bool { + matches!( + self, + TargetFilter::SelfRef + | TargetFilter::SpecificObject { .. } + | TargetFilter::AttachedTo + | TargetFilter::ParentTarget + | TargetFilter::ParentTargetSlot { .. } + ) + } + /// CR 115.1: Returns true for filters that are NOT player-chosen targets — /// context references (triggering event participants per CR 603.7c), /// parent target anaphora, and self-references resolve automatically @@ -20397,6 +20424,34 @@ pub enum AbilityCondition { HasCityBlessing, /// CR 702.195b: True when the ability controller has the enduring story designation. HasEnduringStory, + /// CR 903.3d + CR 608.2a: Resolution-time commander-control gate — "if you + /// control your commander" / "if you control a commander". CR 903.3d is the + /// authorizing rule: "If an effect refers to controlling a commander, it + /// refers to a permanent on the battlefield that is a commander." + /// CR 608.2a is the resolution-time half: an intervening-`if` condition is + /// re-checked as the ability resolves and the ability does nothing if it is + /// then false. + /// + /// The effect-resolution mirror of `StaticCondition::ControlsCommander` + /// (layers), `TriggerCondition::ControlsCommander` (intervening-if on a + /// printed trigger) and `ParsedCondition::ControlsCommander` + /// (activation/casting restrictions), carrying the same `ownership` axis. + /// + /// CR 903.3 + CR 109.5: `Own` ("your commander") requires the evaluating + /// player to both OWN and control it — CR 903.3 makes the designation an + /// attribute of the *card*, so a stolen commander remains its owner's, not + /// yours. `Any` ("a commander") is controller-only, any owner. + /// + /// CR 109.5: "you" is the resolving ability's controller; for a delayed + /// triggered ability that is the player who controlled the creating spell as + /// it resolved (CR 603.7d), which `Effect::CreateDelayedTrigger` already + /// stamps onto the delayed `ResolvedAbility`. + /// + /// Evaluated live at resolution against the single `game::commander` + /// authority — the same helpers the three sibling mirrors use — so the four + /// readings of one printed clause cannot drift, and the CR 702.26b + /// phased-out exclusion applies uniformly. + ControlsCommander { ownership: CommanderOwnership }, /// CR 701.9a + CR 608.2c: True when the card discarded by the directly /// preceding discard instruction matches `filter`. The resolver reads the /// discard operation's captured hand-time result, never current-zone state @@ -20693,6 +20748,14 @@ impl AbilityCondition { | AbilityCondition::IsInitiative | AbilityCondition::HasCityBlessing | AbilityCondition::HasEnduringStory + // CR 903.3d: a commander-control gate is a plain game-state predicate + // about the resolving ability's controller. It says nothing about + // whether an antecedent optional effect was performed, so a token + // minted under this gate is unconditionally live and the referent + // re-link must NOT fold the gate away as a reflexive "if you do". + // Same classification, for the same reason, as `IsMonarch` and + // `CompletedDungeon` below. + | AbilityCondition::ControlsCommander { .. } | AbilityCondition::DiscardedCardMatchesFilter { .. } | AbilityCondition::IsRingBearer | AbilityCondition::HasObjectTarget diff --git a/crates/engine/tests/integration/fight_for_the_throne_monarch_gated_on_commander.rs b/crates/engine/tests/integration/fight_for_the_throne_monarch_gated_on_commander.rs new file mode 100644 index 0000000000..2daef745ca --- /dev/null +++ b/crates/engine/tests/integration/fight_for_the_throne_monarch_gated_on_commander.rs @@ -0,0 +1,241 @@ +//! Fight for the Throne — the delayed `BecomeMonarch` trigger must honour its +//! intervening-`if` "if you control your commander". +//! +//! Oracle (MSC, verbatim): "Put a +1/+1 counter on target creature you control. +//! Then it fights target creature an opponent controls. When the creature an +//! opponent controls dies this turn, if you control your commander, you become +//! the monarch." +//! +//! The bug: `parse_inner_condition` produced +//! `StaticCondition::ControlsCommander { ownership: Own }` correctly, but the +//! `StaticCondition` -> `AbilityCondition` bridge +//! (`parser::oracle_effect::conditions::static_condition_to_ability_condition`) +//! listed that variant among its "no effect-resolution equivalent -> None" arms, +//! because `AbilityCondition` was the only one of the four condition +//! vocabularies missing a `ControlsCommander` mirror. So +//! `strip_leading_general_conditional` silently discarded the gate and the +//! delayed trigger made you the monarch UNCONDITIONALLY when the fought creature +//! died. +//! +//! These tests parse the verbatim Oracle text through +//! `add_spell_to_hand_from_oracle` (the production parser path the fix modifies) +//! and drive the full cast pipeline, so they exercise parser + runtime +//! end-to-end. They need no `integration_cards.json` regeneration — the card is +//! not in that fixture. +//! +//! Every negative fixture is paired with a positive reach-guard: the fought +//! creature must actually be in the graveyard. Without it a fixture would pass +//! vacuously if the fight failed to kill, the spell fizzled on targeting, or the +//! delayed trigger never fired at all. + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; +use engine::types::zones::Zone; + +const ORACLE: &str = "Put a +1/+1 counter on target creature you control. \ +Then it fights target creature an opponent controls. When the creature an opponent \ +controls dies this turn, if you control your commander, you become the monarch."; + +/// The post-resolution facts each fixture asserts on. +/// +/// The two commander predicates are non-vacuity guards, not incidental extras: +/// the stolen-commander and owned-but-not-controlled fixtures below are only +/// meaningful if the owner/controller divergence actually materialized. Without +/// them, a `controlled_by` that silently failed (Layer 2 recomputes `controller` +/// from `base_controller` on every pass) would leave P0 controlling no commander +/// at all, and both fixtures would pass for entirely the wrong reason. +struct FightResult { + monarch: Option, + fought_creature_zone: Zone, + /// CR 903.3d: does P0 control a commander on the battlefield, any owner? + p0_controls_any_commander: bool, + /// CR 903.3: does a commander P0 OWNS sit on the battlefield, any controller? + p0_owns_battlefield_commander: bool, +} + +/// Cast and fully resolve Fight for the Throne, with `stage_commander` given the +/// chance to place a commander object first. +fn resolve_fight_for_the_throne(stage_commander: impl FnOnce(&mut GameScenario)) -> FightResult { + let mut scenario = GameScenario::new_n_player(2, 42); + scenario.at_phase(Phase::PreCombatMain); + + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Fight for the Throne", true, ORACLE) + .id(); + // CR 613.4c + CR 701.14a: the +1/+1 counter makes the 5/5 a 6/6, then each + // creature deals damage equal to its power to the other — 6 kills the 1/1, + // and the 1 dealt back leaves the 6/6 alive. Only the opponent's creature + // dies, which is exactly the event the delayed trigger watches. + let mine = scenario.add_creature(P0, "Throne Claimant", 5, 5).id(); + let theirs = scenario.add_creature(P1, "Doomed Squire", 1, 1).id(); + + // The commander is ALWAYS a distinct object from both fighters. The gate is + // evaluated live as the delayed ability resolves (CR 603.4 + CR 608.2a), so a commander + // that were itself a fight participant could change zones mid-resolution and + // make the expected value depend on the fight outcome rather than on the gate. + stage_commander(&mut scenario); + + let mut runner = scenario.build(); + // CR 601.2c: slots answered in written order — the PutCounter target (a + // creature you control), then the Fight target (a creature an opponent + // controls). + let outcome = runner.cast(spell).target_objects(&[mine, theirs]).resolve(); + let state = outcome.state(); + FightResult { + monarch: state.monarch, + fought_creature_zone: outcome.zone_of(theirs), + // The sibling authority the `Own` arm must NOT be widened to. + p0_controls_any_commander: engine::game::commander::controls_any_commander(state, P0), + p0_owns_battlefield_commander: state.battlefield.iter().any(|id| { + state + .objects + .get(id) + .is_some_and(|obj| obj.is_commander && obj.owner == P0) + }), + } +} + +/// POSITIVE — CR 903.3 + CR 903.3d: P0 owns and controls a commander on the +/// battlefield, so the intervening-`if` holds and the delayed trigger grants the +/// monarch designation (CR 725.1). +/// +/// This is the reachability proof for the whole file: it shows the delayed +/// trigger fires, its gate is evaluated, and a passing gate still produces the +/// monarch. Without it the three negative fixtures below could all be passing +/// because the trigger never runs. +#[test] +fn own_commander_on_battlefield_grants_the_monarch() { + let result = resolve_fight_for_the_throne(|scenario| { + scenario + .add_creature(P0, "Your Commander", 2, 2) + .commander(); + }); + + assert_eq!( + result.fought_creature_zone, + Zone::Graveyard, + "reach-guard: the fought creature must actually have died for the delayed \ + trigger to fire" + ); + assert_eq!( + result.monarch, + Some(P0), + "CR 903.3d + CR 725.1: with an owned-and-controlled commander on the \ + battlefield the intervening-if holds, so P0 becomes the monarch" + ); +} + +/// NEGATIVE + reach-guard — CR 603.4 + CR 608.2a: with no commander anywhere, +/// the intervening-`if` fails as the delayed ability resolves and nobody becomes +/// the monarch. +/// +/// THE REGRESSION ASSERTION. Before the fix the gate was dropped during lowering +/// and this read `Some(P0)` — an unconditional monarch grant. Reverting the +/// bridge arm in `static_condition_to_ability_condition` flips it back. +#[test] +fn no_commander_means_no_monarch() { + let result = resolve_fight_for_the_throne(|_scenario| {}); + + assert_eq!( + result.fought_creature_zone, + Zone::Graveyard, + "reach-guard: the fought creature must actually have died, so the delayed \ + trigger genuinely fired and was GATED rather than skipped upstream" + ); + assert_eq!( + result.monarch, None, + "CR 603.4 + CR 608.2a: the intervening-if \"if you control your commander\" fails, so \ + the delayed BecomeMonarch must do nothing. Some(P0) here is the shipped \ + misparse — a dropped gate granting the monarch unconditionally" + ); +} + +/// NEGATIVE + reach-guard — the `Own`-vs-`Any` discriminator. P0 has gained +/// control of P1's commander and owns none of their own. +/// +/// CR 903.3: the commander designation "is not a characteristic of the object +/// represented by the card; rather, it is an attribute of the card itself", so a +/// stolen commander remains its OWNER's commander. Combined with CR 109.5's +/// possessive "your", `controls_own_commander` is false here even though +/// `controls_any_commander` is true. +/// +/// An implementation that delegated the `Own` arm to `controls_any_commander` +/// flips this to `Some(P0)`. +#[test] +fn stolen_opponent_commander_does_not_satisfy_your_commander() { + let result = resolve_fight_for_the_throne(|scenario| { + scenario + .add_creature(P1, "Opposing Commander", 3, 3) + .commander() + .controlled_by(P0); + }); + + assert_eq!( + result.fought_creature_zone, + Zone::Graveyard, + "reach-guard: the fought creature must actually have died for the gate to \ + be reached" + ); + // NON-VACUITY GUARD: the divergence must really exist. If `controlled_by` + // had silently failed, P0 would control no commander at all and the negative + // below would pass without discriminating anything. + assert!( + result.p0_controls_any_commander, + "CR 903.3d: the stolen commander must genuinely be under P0's control, so \ + the Any predicate is TRUE while the Own predicate must be FALSE — that \ + gap is the whole point of this fixture" + ); + assert!( + !result.p0_owns_battlefield_commander, + "P0 must own no commander here, or the fixture would not isolate the \ + owner conjunct" + ); + assert_eq!( + result.monarch, None, + "CR 903.3 + CR 109.5: a commander P0 merely CONTROLS is still its owner's \ + commander, so \"your commander\" is not satisfied. Some(P0) means the Own \ + arm was widened to controls_any_commander" + ); +} + +/// NEGATIVE + reach-guard — the other conjunct. P0 OWNS a commander but P1 +/// controls it, and P0 controls no other commander. +/// +/// CR 903.3d resolves "controlling a commander" against a permanent on the +/// battlefield, and CR 613.1b (Layer 2) is what moved control. Ownership alone +/// must not satisfy the gate. +#[test] +fn owning_a_commander_you_do_not_control_does_not_satisfy_the_gate() { + let result = resolve_fight_for_the_throne(|scenario| { + scenario + .add_creature(P0, "Your Commander", 3, 3) + .commander() + .controlled_by(P1); + }); + + assert_eq!( + result.fought_creature_zone, + Zone::Graveyard, + "reach-guard: the fought creature must actually have died for the gate to \ + be reached" + ); + // NON-VACUITY GUARD: P0's commander must really be on the battlefield (just + // under P1's control). Otherwise this fixture degenerates into "no commander + // exists", which the `no_commander_means_no_monarch` case already covers. + assert!( + result.p0_owns_battlefield_commander, + "CR 903.3: a commander P0 OWNS must be on the battlefield, so only the \ + controller conjunct is doing the work here" + ); + assert!( + !result.p0_controls_any_commander, + "CR 613.1b: control must genuinely have moved to P1, or the controller \ + conjunct is not being isolated" + ); + assert_eq!( + result.monarch, None, + "CR 903.3d: P0 owns but does not CONTROL the commander, so the gate fails. \ + Some(P0) means the Own arm dropped its controller conjunct" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index e59c831416..628dcd7602 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -237,6 +237,7 @@ mod fevered_visions; mod fewer_than_existential_threshold; mod field_marshal_soldier_anthem_first_strike; mod field_of_ruin_search; +mod fight_for_the_throne_monarch_gated_on_commander; mod finality_counter_death_to_exile; mod fireball_x_cost_surcharge_timing; mod flashback_nonmana_payability;