diff --git a/crates/engine/src/game/casting_tests.rs b/crates/engine/src/game/casting_tests.rs index 325b829eeb..93962714c8 100644 --- a/crates/engine/src/game/casting_tests.rs +++ b/crates/engine/src/game/casting_tests.rs @@ -9027,6 +9027,267 @@ fn jhoira_exile_cost_activation_suspends_the_exiled_card() { ); } +/// Negative sibling of `jhoira_exile_cost_activation_suspends_the_exiled_card` +/// and the runtime proof for the COST-PAID binding class of the keyword anaphor. +/// +/// CR 608.2k: "if it doesn't have suspend" back-references the object introduced +/// by the ability's COST, so the parser lowers Jhoira's gate to +/// `AbilityCondition::CostPaidObjectMatchesFilter` (clause-context re-anchoring +/// in `rewrite_keyword_anaphor_for_cost_paid_parent`) — `TargetFilter:: +/// CostPaidObject` is never written into `ResolvedAbility.targets`, and an +/// activated ability has no trigger event, so the target-scoped reading would +/// find no subject at all. +/// +/// When the exiled card ALREADY has printed `Suspend 4—{U}` the gate must be +/// FALSE and no grant may fire. Revert-fail: with the old `SourceLacksKeyword` +/// lowering the gate reads Jhoira (never suspended), the redundant grant fires, +/// and `off_zone_characteristics::upsert_keyword_contribution` replaces the +/// printed contribution with `Suspend { count: 0, cost: {} }` — so the effective +/// suspend cost collapses from `{U}` to `{0}`. +#[test] +fn jhoira_does_not_regrant_suspend_to_a_natively_suspended_card() { + use super::super::engine::apply_as_current; + use crate::types::counter::CounterType; + use crate::types::keywords::Keyword; + use crate::types::mana::ManaCostShard; + + let blue = ManaCost::Cost { + shards: vec![ManaCostShard::Blue], + generic: 0, + }; + + let mut state = setup_game_at_main_phase(); + let jhoira = create_object( + &mut state, + CardId(983), + PlayerId(0), + "Jhoira of the Ghitu".to_string(), + Zone::Battlefield, + ); + // The eligible exile-cost card, printed with `Suspend 4—{U}`. + let nonland = create_object( + &mut state, + CardId(984), + PlayerId(0), + "Already Suspended Sorcery".to_string(), + Zone::Hand, + ); + { + let obj = state.objects.get_mut(&jhoira).unwrap(); + obj.card_types.core_types.push(CoreType::Creature); + let parsed = crate::parser::oracle::parse_oracle_text( + "{2}, Exile a nonland card from your hand: Put four time \ + counters on the exiled card. If it doesn't have suspend, it \ + gains suspend.", + "Jhoira of the Ghitu", + &[], + &[String::from("Creature")], + &[], + ); + Arc::make_mut(&mut obj.abilities).extend(parsed.abilities); + } + { + let nl = state.objects.get_mut(&nonland).unwrap(); + nl.card_types.core_types.push(CoreType::Sorcery); + nl.base_card_types = nl.card_types.clone(); + let printed = Keyword::Suspend { + count: 4, + cost: blue.clone(), + }; + // Both lists: the cost-payment LKI snapshot reads `keywords`, and the + // post-exile off-zone read starts from `base_keywords`. + nl.keywords.push(printed.clone()); + nl.base_keywords.push(printed); + } + add_mana(&mut state, PlayerId(0), ManaType::Colorless, 2); + + apply_as_current( + &mut state, + GameAction::ActivateAbility { + source_id: jhoira, + ability_index: 0, + }, + ) + .expect("Jhoira's exile-cost ability must enter the activation pipeline"); + apply_as_current( + &mut state, + GameAction::SelectCards { + cards: vec![nonland], + }, + ) + .expect("paying the exile-from-hand cost must succeed"); + + let mut events = Vec::new(); + stack::resolve_top(&mut state, &mut events); + + // Reach-guard: the chain really ran — CR 122.1, four time counters landed. + assert_eq!( + state.objects[&nonland] + .counters + .get(&CounterType::Time) + .copied(), + Some(4), + "the exiled card must still carry four time counters" + ); + // CR 702.62a: "Suspend N—[cost]". The printed cost must survive untouched. + assert_eq!( + crate::game::keywords::effective_suspend_cost(&state, nonland), + Some(blue), + "the cost-paid anaphor must read the EXILED CARD: it already has suspend, \ + so no redundant grant may clobber its printed Suspend 4—{{U}} to {{0}}" + ); +} + +/// CR 608.2h + CR 608.2k + CR 613.1f: the cost-paid referent must be read at +/// RESOLUTION, not from the payment-time snapshot. +/// +/// The card enters the cost with NO suspend at all, so the payment-time +/// `LKISnapshot` honestly records "no suspend". Only after the cost is paid — +/// while the card is sitting in EXILE, and before the ability resolves — does a +/// Layer-6 continuous effect grant it `Suspend 4—{U}`. CR 608.2k keeps the +/// ability pointing at that object, and CR 608.2h says a reference to an object +/// still in the public zone it was expected to be in reads that object's CURRENT +/// information. So by the time the gate is evaluated the card DOES have suspend +/// and no grant may fire. +/// +/// This is the case a snapshot read cannot get right in principle: the kind-level +/// keyword props exist to consult the off-zone keyword ledger, and an off-zone +/// grant is applied to the live object, never captured in an LKI snapshot. +/// +/// Revert-fail: evaluate the filter through `matches_target_filter_on_lki_snapshot` +/// instead of `matches_target_filter_on_cost_paid_reference` and the gate reads +/// the payment-time keyword list (empty), fires the redundant grant, and +/// `upsert_keyword_contribution` replaces the granted `Suspend 4—{U}` contribution +/// with `Suspend { count: 0, cost: {} }` — so this reads `Some({0})`. +#[test] +fn jhoira_reads_a_suspend_granted_in_exile_after_the_cost_was_paid() { + use super::super::engine::apply_as_current; + use crate::types::ability::{ContinuousModification, Duration, TargetFilter}; + use crate::types::counter::CounterType; + use crate::types::keywords::Keyword; + use crate::types::mana::ManaCostShard; + + let blue = ManaCost::Cost { + shards: vec![ManaCostShard::Blue], + generic: 0, + }; + + let mut state = setup_game_at_main_phase(); + let jhoira = create_object( + &mut state, + CardId(985), + PlayerId(0), + "Jhoira of the Ghitu".to_string(), + Zone::Battlefield, + ); + // Deliberately NO printed suspend, on either list: the payment-time snapshot + // must honestly say "no suspend" so the grant below is the only source. + let nonland = create_object( + &mut state, + CardId(986), + PlayerId(0), + "Plain Sorcery".to_string(), + Zone::Hand, + ); + { + let obj = state.objects.get_mut(&jhoira).unwrap(); + obj.card_types.core_types.push(CoreType::Creature); + let parsed = crate::parser::oracle::parse_oracle_text( + "{2}, Exile a nonland card from your hand: Put four time \ + counters on the exiled card. If it doesn't have suspend, it \ + gains suspend.", + "Jhoira of the Ghitu", + &[], + &[String::from("Creature")], + &[], + ); + Arc::make_mut(&mut obj.abilities).extend(parsed.abilities); + } + { + let nl = state.objects.get_mut(&nonland).unwrap(); + nl.card_types.core_types.push(CoreType::Sorcery); + nl.base_card_types = nl.card_types.clone(); + } + add_mana(&mut state, PlayerId(0), ManaType::Colorless, 2); + + apply_as_current( + &mut state, + GameAction::ActivateAbility { + source_id: jhoira, + ability_index: 0, + }, + ) + .expect("Jhoira's exile-cost ability must enter the activation pipeline"); + apply_as_current( + &mut state, + GameAction::SelectCards { + cards: vec![nonland], + }, + ) + .expect("paying the exile-from-hand cost must succeed"); + + // Reach-guard: the cost really moved the card to exile, so the grant below is + // an EXILE-zone characteristic change and the snapshot is already stale. + assert_eq!( + state.objects[&nonland].zone, + Zone::Exile, + "reach-guard: paying the cost must have exiled the card" + ); + assert!( + !crate::game::keywords::object_has_effective_keyword_kind( + &state, + nonland, + crate::types::keywords::KeywordKind::Suspend, + ), + "reach-guard: the card must have no suspend at payment time, or the \ + grant below proves nothing" + ); + + // The characteristic change, AFTER payment and BEFORE resolution. + state.add_transient_continuous_effect( + jhoira, + PlayerId(0), + Duration::Permanent, + TargetFilter::SpecificObject { id: nonland }, + vec![ContinuousModification::AddKeyword { + keyword: Keyword::Suspend { + count: 4, + cost: blue.clone(), + }, + }], + None, + ); + assert!( + crate::game::keywords::object_has_effective_keyword_kind( + &state, + nonland, + crate::types::keywords::KeywordKind::Suspend, + ), + "reach-guard: the off-zone grant must be visible before resolution, or \ + the gate below is not being asked the question this test intends" + ); + + let mut events = Vec::new(); + stack::resolve_top(&mut state, &mut events); + + // Reach-guard: the chain really ran — CR 122.1, four time counters landed. + assert_eq!( + state.objects[&nonland] + .counters + .get(&CounterType::Time) + .copied(), + Some(4), + "the exiled card must carry four time counters" + ); + assert_eq!( + crate::game::keywords::effective_suspend_cost(&state, nonland), + Some(blue), + "CR 608.2h: the gate must read the card's state AT RESOLUTION, where the \ + exile-zone grant already gave it Suspend 4—{{U}} — no redundant grant may \ + clobber that to {{0}}" + ); +} + /// The Wedding of River Song (WHO) — runtime regression for the core chain. /// Drives the real cast pipeline (CastSpell → resolution) and asserts: /// (a) the controller draws two cards; (b) the controller's nonland card is @@ -9034,9 +9295,11 @@ fn jhoira_exile_cost_activation_suspends_the_exiled_card() { /// /// The "Cards exiled this way that don't have suspend gain suspend" clause /// (Defect C) is a *documented strict-failure* — the "that don't have " -/// restrictive clause strict-fails to `Unimplemented` because the correct -/// per-card object-scoped condition (applying `SourceLacksKeyword` per exiled -/// card, not per spell source) does not yet exist in the engine. The exiled +/// restrictive clause strict-fails to `Unimplemented` because it needs a +/// PER-MEMBER predicate over the exiled tracked set. The singular anaphor's two +/// lowerings (`TargetMatchesFilter` / `CostPaidObjectMatchesFilter`) each test +/// exactly one subject, and `ZoneChangedThisWay` is a set existential, so none +/// of them expresses "exclude each member that already has suspend". The exiled /// card therefore does NOT gain suspend at runtime — this is expected, not a /// regression. See `try_parse_exiled_this_way_keyword_grant` for details. /// diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index a3b006412d..7773fe4c62 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -13175,9 +13175,14 @@ pub(crate) fn evaluate_condition( .iter() .any(|&id| crate::game::filter::matches_target_filter(state, id, filter, &ctx)) } + // CR 608.2k + CR 608.2h: the cost-paid object is a persistent untargeted + // reference, so it reads CURRENT information while it is still in a + // public zone — not the payment-time snapshot. See + // `matches_target_filter_on_cost_paid_reference` for why only the + // keyword set is refreshed. AbilityCondition::CostPaidObjectMatchesFilter { filter } => { if let Some(snapshot) = &ability.cost_paid_object { - crate::game::filter::matches_target_filter_on_lki_snapshot( + crate::game::filter::matches_target_filter_on_cost_paid_reference( state, snapshot.object_id, &snapshot.lki, diff --git a/crates/engine/src/game/filter.rs b/crates/engine/src/game/filter.rs index c24418c1c1..93c09c59ec 100644 --- a/crates/engine/src/game/filter.rs +++ b/crates/engine/src/game/filter.rs @@ -2239,6 +2239,16 @@ pub fn context_free_prop_matches_face(face: &CardFace, prop: &FilterProp) -> Opt FilterProp::WithKeyword { value } => Some(face.keywords.contains(value)), // allow-raw-authority: bare CardFace has no object, so no keyword grant can exist to miss FilterProp::WithoutKeyword { value } => Some(!face.keywords.contains(value)), + // The kind-level siblings (`HasKeywordKind` / `WithoutKeywordKind`) are + // intentionally ABSENT and fall to the `None` arm below. They exist to + // consult off-zone Layer-6 grants, which a bare face by definition cannot + // have, so a face reading would answer a strictly narrower question than + // the prop asks. Every production caller of this function evaluates an + // effect target filter or a static's `spell_filter` — never an + // `AbilityCondition` filter, which is the only place those props appear + // today — so the `None` default is unreachable rather than lossy. A future + // caller that needs them must add explicit arms here instead of relying on + // the fail-closed default. // CR 111.1 + CR 108.2: a bare face is a card definition, never a token. FilterProp::Token => Some(false), FilterProp::NonToken | FilterProp::RepresentedByCard => Some(true), @@ -2759,6 +2769,63 @@ pub fn matches_target_filter_on_lki_snapshot( matches_target_filter_on_zone_change_record(state, &record, filter, ctx) } +/// CR 608.2k + CR 608.2h: Evaluate a target filter against an ability's +/// PERSISTENT untargeted reference — today, its cost-paid object. +/// +/// This is NOT the same question as [`matches_target_filter_on_lki_snapshot`]. +/// A zone-change subject is gone, so its snapshot IS the answer. A cost-paid +/// referent is a live reference the ability keeps pointing at (CR 608.2k), and +/// CR 608.2h says such a reference reads the object's CURRENT information while +/// it is in the public zone it was expected to be in — only a departed or +/// hidden-zone object falls back to last known information. +/// +/// The refresh is deliberately scoped to `keywords`. That is the one field the +/// payment-time snapshot cannot answer honestly: the kind-level keyword props +/// exist to consult Layer-6 grants recorded in the off-zone ledger +/// (CR 613.1f), which by construction are applied to the LIVE object and are +/// absent from any snapshot. A card discarded to Jhoira of the Ghitu's cost and +/// then granted (or stripped of) suspend in the graveyard or in exile before the +/// ability resolves must be read as it is at resolution, or the gate answers a +/// question about a game state that no longer exists. Every other LKI field stays +/// on the snapshot: type, name, P/T, colors and controller are look-back facts +/// about the payment itself. (A filter that pairs a kind-level prop with an +/// object-level `WithKeyword`/`WithoutKeyword` would see the refreshed list for +/// both, since they read the same field — no card does that today, and CR 608.2h +/// makes the live reading the correct one either way.) +/// +/// Guarded by `TargetFilter::queries_keyword_kind` so the common cost-paid filter +/// — a plain type/name look-back with no keyword question — costs one recursive +/// predicate walk and skips both the ledger recomputation +/// (`effective_off_zone_keywords` collects every applicable continuous effect) +/// and the snapshot clone. +pub fn matches_target_filter_on_cost_paid_reference( + state: &GameState, + object_id: ObjectId, + lki: &LKISnapshot, + filter: &TargetFilter, + ctx: &FilterContext<'_>, +) -> bool { + let refreshed = filter + .queries_keyword_kind() + .then(|| state.objects.get(&object_id)) + .flatten() + .filter(|object| object.zone.is_public()) + .map(|_| { + crate::game::off_zone_characteristics::effective_off_zone_keywords(state, object_id) + }); + + match refreshed { + Some(keywords) => { + let mut lki = lki.clone(); + lki.keywords = keywords; + matches_target_filter_on_lki_snapshot(state, object_id, &lki, filter, ctx) + } + // Gone, or moved to a hidden zone: CR 608.2h mandates last known + // information, which is exactly what the payment snapshot holds. + None => matches_target_filter_on_lki_snapshot(state, object_id, lki, filter, ctx), + } +} + /// CR 400.7 + CR 603.10a: Match an event subject from its captured facts, /// never by re-reading the live object at the same storage id. Connive uses /// this for its exact completion snapshot after a replacement-ordering pause. diff --git a/crates/engine/src/game/meld.rs b/crates/engine/src/game/meld.rs index 8e30a3c250..c550bf2c8f 100644 --- a/crates/engine/src/game/meld.rs +++ b/crates/engine/src/game/meld.rs @@ -208,10 +208,7 @@ pub fn is_canonical_physical_meld_pair(state: &GameState, context: &MeldSelectio /// CR 400.2: graveyard, battlefield, stack, exile, and command are public /// zones. Hand and library remain hidden even when their cards are revealed. fn is_public_zone(zone: Zone) -> bool { - match zone { - Zone::Battlefield | Zone::Graveyard | Zone::Stack | Zone::Exile | Zone::Command => true, - Zone::Library | Zone::Hand => false, - } + zone.is_public() } /// Commit the projected result entry, then atomically make the second card a diff --git a/crates/engine/src/parser/oracle_effect/conditions.rs b/crates/engine/src/parser/oracle_effect/conditions.rs index f244ad491f..3d7f5070ee 100644 --- a/crates/engine/src/parser/oracle_effect/conditions.rs +++ b/crates/engine/src/parser/oracle_effect/conditions.rs @@ -5387,6 +5387,31 @@ fn parse_or_if_disjunction(text: &str, ctx: &mut ParseContext) -> Option" / "if it doesn't have ") onto a +/// `KeywordKind`-level `FilterProp`. +/// +/// Both polarities must answer the same question at the same seam, and both +/// must survive the subject being OFF the battlefield — the whole class exiles +/// its subject first (suspend, foretell, time counters). Only the kind-level +/// props (`HasKeywordKind` / `WithoutKeywordKind`) do that: they route through +/// `object_has_effective_keyword_kind`, which consults the off-zone Layer-6 +/// ledger, while the object-level props read `obj.keywords`, which the layer +/// system never refreshes in exile. +/// +/// Returns `None` — a deliberate strict failure, leaving the clause to the +/// swallowed-clause / `Unimplemented` path so coverage stays honest — when +/// `Keyword::kind()` does NOT identify the parsed ability +/// ([`Keyword::kind_identifies_ability`] documents both families). Falling back +/// to `FilterProp::WithKeyword`/`WithoutKeyword` there is NOT an option: those +/// are discriminant-matched on the live path, so they cannot separate +/// protection from red from protection from blue, and they read the stale +/// off-zone keyword vec besides — i.e. they would reintroduce the +/// always-wrong-guard failure mode this lowering exists to remove. +fn keyword_presence_kind(keyword: &Keyword) -> Option { + keyword.kind_identifies_ability().then(|| keyword.kind()) +} + pub(super) fn try_nom_condition_as_ability_condition( text: &str, ctx: &mut ParseContext, @@ -5557,9 +5582,40 @@ pub(super) fn try_nom_condition_as_ability_condition( return Some(condition); } - // CR 702.62a: "it doesn't have [keyword]" / "it does not have [keyword]" — pronoun - // subject lacks-keyword check (e.g., "If it doesn't have suspend, it gains suspend"). - // Mirrors the "~ doesn't have" / "this creature doesn't have" handler in oracle_condition.rs. + // CR 702.62a + CR 608.2c + CR 608.2k + CR 400.7: "it doesn't have [keyword]" / + // "it does not have [keyword]" — the negative-polarity twin of the "it has + // [keyword]" arm later in this same function. + // + // `it` is an ANAPHOR to the object introduced by the preceding instruction, + // by the ability's COST, or by the trigger condition — CR 608.2k enumerates + // all three — and NOT to the ability's source. This arm emits the + // CONTEXT-FREE (target / trigger-subject) reading; + // `rewrite_keyword_anaphor_for_cost_paid_parent` (oracle_effect/mod.rs) + // re-anchors it to `CostPaidObjectMatchesFilter` when the preceding clause + // binds its subject through the cost. That split mirrors + // `rewrite_cost_paid_exiled_reflexive_for_effect_exile_parent`, whose doc + // states the governing principle: only the clause context can disambiguate + // these subject classes. + // + // This arm previously emitted `SourceLacksKeyword`, whose evaluator reads + // `ability.source_id` — making the guard unconditionally true for every card + // whose `it` is not the source (Kang Prime, Jhoira of the Ghitu, Suspend, + // Delay, …), so an exiled card that already had the keyword was re-granted + // and its printed parameters clobbered. + // + // CR 613.1f + CR 702.62b: the prop is the KIND-level, off-zone-aware + // `WithoutKeywordKind`, not `WithoutKeyword`. `WithoutKeyword` reads + // `obj.keywords`, which the layer system refreshes only for battlefield, + // hand, and stack objects — never exile, where most of this class evaluates. + // `WithoutKeywordKind` routes through `object_has_effective_keyword_kind` on + // the live path and through the zone-change record's keywords on the snapshot + // path, so BOTH lowering targets read it correctly. It is also the CR-correct + // reading: CR 702.62a defines suspend as "Suspend N—[cost]", so "it doesn't + // have suspend" is a keyword-ABILITY presence test, parameters aside. + // + // The kind-level prop is only sound where `Keyword::kind()` IDENTIFIES the + // ability, which is NOT every keyword — `keyword_presence_kind` below is the + // single authority for that test and strict-fails the rest. if let Ok((keyword_text, _)) = alt(( tag::<_, _, OracleError<'_>>("it doesn't have "), tag("it does not have "), @@ -5570,8 +5626,15 @@ pub(super) fn try_nom_condition_as_ability_condition( .trim() .parse() .unwrap_or(Keyword::Unknown(String::new())); - if !matches!(keyword, Keyword::Unknown(_)) { - return Some(AbilityCondition::SourceLacksKeyword { keyword }); + if let Some(value) = keyword_presence_kind(&keyword) { + return Some(AbilityCondition::TargetMatchesFilter { + filter: TargetFilter::Typed(TypedFilter { + properties: vec![FilterProp::WithoutKeywordKind { value }], + ..Default::default() + }), + use_lki: false, + subject_slot: None, + }); } } @@ -5972,19 +6035,32 @@ pub(super) fn try_nom_condition_as_ability_condition( } // CR 608.2c + CR 702.1: "it has [keyword]" — affirmative pronoun keyword check - // (e.g. "If it has flying, ..."). Routed through TargetMatchesFilter + - // FilterProp::WithKeyword, the same abstraction the "it's a [type]" arm uses - // (no SourceHasKeyword sibling to SourceLacksKeyword). Disjoint prefix from the - // "it doesn't have" arm above, so ordering is irrelevant. + // (e.g. "If it has flying, ..."). Routed through TargetMatchesFilter, the + // same abstraction the "it's a [type]" arm uses. + // + // CR 613.1f: shares `keyword_presence_kind` with its negative twin — the + // "it doesn't have [keyword]" arm above — so both polarities emit the same + // kind-level, off-zone-aware prop and strict-fail on the same keywords. The + // affirmative arm previously emitted the object-level + // `FilterProp::WithKeyword`, a shape `filter_is_bare_keyword_kind_predicate` + // does not accept, so `rewrite_keyword_anaphor_for_cost_paid_parent` could + // never re-anchor this polarity: an "if it has " clause after a + // cost-paid parent found neither an object target nor (for an activated + // ability) a trigger event and failed closed. Both polarities now reach that + // rewrite, and `rewrite_filter_keyword` (oracle_effect/mod.rs) already swaps + // `HasKeywordKind` for "the same is true for " replication. + // + // Disjoint prefix from the negative arm, so ordering between them is + // irrelevant. if let Ok((keyword_text, _)) = tag::<_, _, OracleError<'_>>("it has ").parse(lower.as_str()) { let keyword: Keyword = keyword_text .trim() .parse() .unwrap_or(Keyword::Unknown(String::new())); - if !matches!(keyword, Keyword::Unknown(_)) { + if let Some(value) = keyword_presence_kind(&keyword) { return Some(AbilityCondition::TargetMatchesFilter { filter: TargetFilter::Typed(TypedFilter { - properties: vec![FilterProp::WithKeyword { value: keyword }], + properties: vec![FilterProp::HasKeywordKind { value }], ..Default::default() }), use_lki: false, @@ -9866,7 +9942,9 @@ mod tests { ); } - /// CR 608.2c + CR 702.1: "If it has [keyword]" gates on FilterProp::WithKeyword. + /// CR 608.2c + CR 702.1 + CR 613.1f: "If it has [keyword]" gates on the + /// kind-level `FilterProp::HasKeywordKind` — the same off-zone-aware prop its + /// negative twin uses (`keyword_presence_kind` is the shared authority). /// Pre-fix this dropped to `None` (only the negative "it doesn't have" arm /// existed), dropping the else-branch. #[test] @@ -9887,10 +9965,10 @@ mod tests { panic!("expected Typed filter for keyword"); }; assert!( - tf.properties.contains(&FilterProp::WithKeyword { - value: Keyword::Flying + tf.properties.contains(&FilterProp::HasKeywordKind { + value: crate::types::keywords::KeywordKind::Flying }), - "expected WithKeyword(Flying) property, got {:?}", + "expected HasKeywordKind(Flying) property, got {:?}", tf.properties ); } diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 4ce7eaa061..c55f864caa 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -796,6 +796,174 @@ fn rewrite_cost_paid_exiled_reflexive_for_effect_exile_parent( condition } +/// True for exactly the filter shape the keyword anaphor's context-free lowering +/// emits: a bare, controller-agnostic, type-agnostic typed filter carrying one +/// kind-level keyword predicate. Both polarities are in scope on purpose — +/// `conditions::keyword_presence_kind` lowers "it has " and "it doesn't have +/// " to `HasKeywordKind` / `WithoutKeywordKind` respectively, and both need +/// the same clause-context re-anchoring. Still narrow: it is the guard that +/// keeps `rewrite_keyword_anaphor_for_cost_paid_parent` and +/// `keyword_anaphor_referent_is_unpublished_resolution_pick` off every other +/// `TargetMatchesFilter` (the "it's a [type]" arm, the anaphoric-status arm, and +/// any object-level `WithKeyword` gate parsed elsewhere). +fn filter_is_bare_keyword_kind_predicate(filter: &TargetFilter) -> bool { + matches!( + filter, + TargetFilter::Typed(TypedFilter { + type_filters, + controller: None, + properties, + }) if type_filters.is_empty() + && matches!( + properties.as_slice(), + [FilterProp::HasKeywordKind { .. } | FilterProp::WithoutKeywordKind { .. }] + ) + ) +} + +/// CR 608.2k + CR 608.2c + CR 702.62a: re-anchor a keyword-presence anaphor +/// ("if it doesn't have suspend") to the COST-PAID object when the +/// immediately-preceding non-continuation clause binds its own subject through +/// the ability's cost (Jhoira of the Ghitu: "{2}, Exile a nonland card from your +/// hand: Put four time counters on the exiled card. If it doesn't have suspend, +/// it gains suspend."). +/// +/// CR 608.2k names three sources for an untargeted back-reference — the effect's +/// own earlier instruction, the ability's COST, and the trigger condition — and +/// the engine reads each from a DIFFERENT runtime slot: +/// +/// * effect instruction / declared target → `ResolvedAbility.targets` +/// (`AbilityCondition::TargetMatchesFilter`) +/// * ability cost → `ResolvedAbility.cost_paid_object` +/// (`AbilityCondition::CostPaidObjectMatchesFilter`) +/// * trigger condition → `GameState.current_trigger_event` +/// (`TargetMatchesFilter`'s `TriggeringSource` fallback) +/// +/// `TargetFilter::CostPaidObject` is resolved at effect-apply time out of the +/// documented `cost_paid_object → effect_context_object` ladder +/// (`game::targeting`, CR 608.2k) and is NEVER written into `targets`, so a +/// cost-paid parent leaves `targets` EMPTY. For an ACTIVATED ability there is +/// also no `current_trigger_event` (the stack lifts one only for a triggered +/// ability), so the context-free `TargetMatchesFilter` reading would fail closed +/// and the grant would never fire. +/// +/// Only the clause context can disambiguate, exactly as +/// `rewrite_cost_paid_exiled_reflexive_for_effect_exile_parent` above states for +/// its own pair. Fire ONLY when the previous non-continuation clause's effect +/// filter references the cost-paid object. Three of the four other parent shapes +/// keep the context-free target-scoped reading, which binds correctly for each: +/// injected target (Kang Prime), declared stack target (Suspend, Delay), and +/// trigger source (Momentum Rumbler). The fourth — the resolution-time pick +/// (The Eleventh Doctor, Amy's Home) — binds to NOTHING, and is strict-failed by +/// `keyword_anaphor_referent_is_unpublished_resolution_pick` below rather than +/// left to the misleading `TriggeringSource` fallback. +/// +/// Both readings are LIVE for the keyword-kind props, and deliberately so. +/// CR 608.2k keeps a cost-introduced reference pointing at its object "even if +/// the object has changed characteristics" — it does NOT freeze the object's +/// characteristics at payment time. CR 608.2h then supplies the timing: a +/// reference to an object still in the public zone it was expected to be in +/// reads that object's CURRENT information. `CostPaidObjectMatchesFilter` is +/// evaluated through `filter::matches_target_filter_on_cost_paid_reference`, +/// which preserves the payment snapshot's look-back facts while reading the +/// keyword set off the live object, so an off-zone Layer-6 grant applied after +/// payment (CR 613.1f) is visible to the gate. +fn rewrite_keyword_anaphor_for_cost_paid_parent( + condition: Option, + clauses: &[ClauseIr], +) -> Option { + let Some(AbilityCondition::TargetMatchesFilter { + filter, + use_lki: false, + subject_slot: None, + }) = &condition + else { + return condition; + }; + if !filter_is_bare_keyword_kind_predicate(filter) { + return condition; + } + let prev_binds_cost_paid_object = clauses + .iter() + .rev() + .find(|clause| !matches!(clause.disposition, ClauseDisposition::Continue { .. })) + .and_then(|clause| clause.parsed.effect.target_filter()) + .is_some_and(TargetFilter::references_cost_paid_object); + if prev_binds_cost_paid_object { + return Some(AbilityCondition::CostPaidObjectMatchesFilter { + filter: filter.clone(), + }); + } + condition +} + +/// CR 608.2k + CR 608.2d + CR 115.10a: True when the keyword-presence anaphor +/// ("if it doesn't have suspend") has NO runtime slot to bind to, because the +/// preceding clause introduces its subject through a RESOLUTION-TIME PICK +/// (The Eleventh Doctor: "you may exile a card from your hand …"; Amy's Home). +/// +/// CR 608.2d makes that pick an untargeted choice made while the ability +/// resolves, so — unlike a declared target (CR 115.10a) — it is never written +/// into `ResolvedAbility.targets`, which is what +/// `AbilityCondition::TargetMatchesFilter` reads. The condition therefore finds +/// no object target and silently falls through to its `TriggeringSource` +/// fallback: for a combat-damage trigger that is the ability's own source, i.e. +/// exactly the always-wrong-guard reading the kind-level lowering exists to +/// remove. The grant's RECIPIENT still binds (`TargetFilter::ParentTarget` is +/// resolved at effect-apply time), so the misread is invisible at runtime — a +/// card that already has the keyword is re-granted and its printed parameters +/// are clobbered, while coverage reports the card fully supported. +/// +/// Rather than ship a knowingly-misbinding gate, strict-fail the clause to +/// `Effect::Unimplemented` so `cargo coverage` reports the gap (the same +/// discipline `try_parse_exiled_this_way_keyword_grant` applies to the plural +/// "cards exiled this way that don't have " form). Repairing it means +/// publishing the resolution-time pick into the sub-chain's `targets`; this +/// predicate is where that fix removes the strict failure. +/// +/// Deliberately narrow, in three independent ways: +/// * the condition must be the bare keyword-kind anaphor shape +/// (`filter_is_bare_keyword_kind_predicate`); +/// * the parent must actually be resolution-timed +/// (`lower::target_choice_timing_for_clause`); +/// * the parent's own target filter must be a real player pick, not a +/// context reference. Delay's "exile it with three time counters" is +/// `TargetChoiceTiming::Resolution` too (off-battlefield origin, no printed +/// "target"), but its filter is `TargetFilter::ParentTarget`, which the +/// resolver binds deterministically from the countered spell — nothing is +/// picked, and the anaphor binds through the propagated target. `Suspend` +/// (declared stack target) and Kang Prime / Jhoira of the Ghitu (context +/// refs) are excluded by the same test. +fn keyword_anaphor_referent_is_unpublished_resolution_pick( + condition: Option<&AbilityCondition>, + clauses: &[ClauseIr], +) -> bool { + let Some(AbilityCondition::TargetMatchesFilter { + filter, + use_lki: false, + subject_slot: None, + }) = condition + else { + return false; + }; + if !filter_is_bare_keyword_kind_predicate(filter) { + return false; + } + clauses + .iter() + .rev() + .find(|clause| !matches!(clause.disposition, ClauseDisposition::Continue { .. })) + .is_some_and(|clause| { + lower::target_choice_timing_for_clause(clause) + == crate::types::ability::TargetChoiceTiming::Resolution + && clause + .parsed + .effect + .target_filter() + .is_some_and(|target| !target.is_context_ref()) + }) +} + fn merge_clause_conditions( outer: AbilityCondition, inner: Option, @@ -24774,15 +24942,35 @@ fn attach_mana_retention_to_prior_mana(defs: &mut [AbilityDefinition], expiry: M false } -/// Swap every `Keyword` inside a `TargetFilter`'s `WithKeyword` properties to +/// Swap every `Keyword` inside a `TargetFilter`'s keyword-presence properties to /// `new_keyword`. Recurses through `Or`/`And` filter trees so a compound /// affected filter is handled uniformly. fn rewrite_filter_keyword(filter: &mut TargetFilter, new_keyword: &Keyword) { match filter { TargetFilter::Typed(typed) => { for prop in &mut typed.properties { - if let FilterProp::WithKeyword { value } = prop { - *value = new_keyword.clone(); + match prop { + FilterProp::WithKeyword { value } => { + *value = new_keyword.clone(); + } + // CR 702.1c: the kind-level siblings carry a `KeywordKind`, so + // a replicated "the same is true for " gate swaps to + // the new keyword's kind. Added for the subject-scoped keyword + // anaphor, whose two polarities lower to `WithoutKeywordKind` + // and `HasKeywordKind` (`conditions::keyword_presence_kind`). + // + // `FilterProp::WithoutKeyword` is deliberately NOT handled + // here: no card routes it through this walker today, and + // adding it would change behavior for pre-existing conditions + // reachable via the `ZoneChangeObjectMatchesFilter` arm in + // `rewrite_ability_condition_keyword` — outside this change's + // scope. `rewrite_filter_keyword_leaves_object_level_without_keyword_alone` + // pins the omission so a future widening is a conscious act. + FilterProp::HasKeywordKind { value } + | FilterProp::WithoutKeywordKind { value } => { + *value = new_keyword.kind(); + } + _ => {} } } } @@ -25080,6 +25268,9 @@ fn attach_perpetual_keyword_grants( /// your graveyard has " (Kathril, Aspect Warper). /// - `TargetHasKeywordInstead` / `SourceLacksKeyword` — "if that creature has /// and ~ doesn't" (Super-Adaptoid). +/// - `TargetMatchesFilter` / `CostPaidObjectMatchesFilter` — the subject-scoped +/// keyword anaphor, "if it doesn't have " (Kang Prime, Jhoira of the +/// Ghitu), whose keyword lives in a typed filter prop. /// - `And`/`Or`/`Not` — recurse into each compound conjunct so the /// Super-Adaptoid conjunction has BOTH the target-has and the source-lacks /// keyword swapped together. @@ -25100,6 +25291,18 @@ fn rewrite_ability_condition_keyword(condition: &mut AbilityCondition, new_keywo AbilityCondition::ZoneChangeObjectMatchesFilter { filter, .. } => { rewrite_filter_keyword(filter, new_keyword); } + // CR 702.1c + CR 608.2c: the subject-scoped keyword anaphor gates ("if it + // doesn't have ") carry their keyword inside a typed filter — the same + // shape the Mutable Pupa `ZoneChangeObjectMatchesFilter` arm above handles. + // Both subject seams the anaphor can lower to are covered, so a replicated + // "the same is true for " continuation swaps the gate regardless + // of which seam clause context selected. Without these arms the class would + // fall into `_ => {}` — a silent regression from the `SourceLacksKeyword` + // handling above, which this change moves the class out of. + AbilityCondition::TargetMatchesFilter { filter, .. } + | AbilityCondition::CostPaidObjectMatchesFilter { filter } => { + rewrite_filter_keyword(filter, new_keyword); + } AbilityCondition::And { conditions } | AbilityCondition::Or { conditions } => { for inner in conditions { rewrite_ability_condition_keyword(inner, new_keyword); @@ -31855,6 +32058,31 @@ pub(crate) fn parse_effect_chain_ir( condition, builder.clauses(), ); + // CR 608.2k: "if it doesn't have " after a cost-paid parent reads the + // cost-paid object, not the (empty) target slot — see the helper for the + // three-slot rationale (Jhoira of the Ghitu). Input shapes are disjoint + // from the two rewrites above, so the order within this family is only a + // reading convention. + let condition = rewrite_keyword_anaphor_for_cost_paid_parent(condition, builder.clauses()); + // CR 608.2k + CR 608.2d: the same anaphor after a RESOLUTION-TIME PICK + // parent has no slot to bind to at all — the pick never reaches + // `targets`, so the gate would silently read the trigger source. Runs + // after the cost-paid rewrite so a re-anchored (cost-paid) condition is + // already out of this shape. Strict-fail to `Unimplemented` instead of + // shipping a misbinding gate, so coverage reports the gap (The Eleventh + // Doctor, Amy's Home) — see the predicate for the full rationale. + if keyword_anaphor_referent_is_unpublished_resolution_pick( + condition.as_ref(), + builder.clauses(), + ) { + unimplemented_clause( + &mut builder, + "keyword_anaphor_resolution_time_pick", + normalized_text, + chunk.boundary_after, + ); + continue; + } // CR 608.2c: "[effect] a number of times equal to the difference" — when // a leading comparison condition was just stripped, a trailing // difference-repeat suffix repeats the effect by the unsigned magnitude diff --git a/crates/engine/src/parser/oracle_effect/subject.rs b/crates/engine/src/parser/oracle_effect/subject.rs index db06e0c023..7151f675bb 100644 --- a/crates/engine/src/parser/oracle_effect/subject.rs +++ b/crates/engine/src/parser/oracle_effect/subject.rs @@ -4262,15 +4262,23 @@ fn build_continuous_clause( /// byte-for-byte the Jhoira/Tenth suspend-grant shape. /// /// The optional "that don't have " restrictive clause (CR 702.62a) is -/// recognised by the parser but results in a strict-failure (`None`) because -/// `evaluate_condition` resolves `SourceLacksKeyword` against the ability's -/// `source_id` (the spell, which never carries the keyword), not each individual -/// exiled card. Attaching the condition therefore produces an unconditional -/// overgrant — already- cards would still receive a redundant grant. A -/// correct per-card exclusion requires an object-scoped condition variant (e.g. -/// `CostPaidObjectLacksKeyword`) that does not yet exist in the engine. Until -/// that building block is added, "cards exiled this way that don't have -/// gain " is a documented strict-failure deferred to `Unimplemented`. +/// recognised by the parser but results in a strict-failure (`None`), because it +/// is a PER-MEMBER predicate over a whole tracked set and no existing condition +/// variant expresses that. The SINGULAR anaphor ("if it doesn't have ") is +/// covered — it lowers to `AbilityCondition::TargetMatchesFilter` with +/// `FilterProp::WithoutKeywordKind`, re-anchored to +/// `CostPaidObjectMatchesFilter` by clause context (see +/// `rewrite_keyword_anaphor_for_cost_paid_parent`) — but both of those test ONE +/// subject: the ability's first object target, or the single cost-paid snapshot. +/// `AbilityCondition::ZoneChangedThisWay` covers the set, yet only as an +/// EXISTENTIAL ("some card exiled this way matches"), which answers a different +/// question than "exclude each member that already has the keyword". +/// +/// Attaching any of the three therefore produces an unconditional overgrant for +/// the plural form — already- cards would still receive a redundant grant, +/// clobbering their printed parameters. Until a per-member predicate over a +/// tracked set exists, "cards exiled this way that don't have gain " +/// stays a documented strict-failure deferred to `Unimplemented`. /// /// Returns `None` (strict-failure to `Unimplemented`) when the restrictive /// clause is present or when the predicate is not a recognised "gain " @@ -4295,10 +4303,11 @@ pub(super) fn try_parse_exiled_this_way_keyword_grant( })?; // Detect the restrictive "that don't have " clause (CR 702.62a). - // When present, strict-fail: the correct object-scoped condition - // (`evaluate_condition` per exiled card, not per spell source) is not - // yet implemented. Attaching `SourceLacksKeyword` here would silently - // overgrant — see the fn doc for the full explanation. + // When present, strict-fail: a PER-MEMBER predicate over the exiled tracked + // set is not yet expressible. The singular anaphor's two lowerings each test + // one subject and `ZoneChangedThisWay` is a set existential, so attaching any + // of them here would silently overgrant — see the fn doc for the full + // explanation. let after_head_lower = after_head.to_lowercase(); let has_restrictive = nom_on_lower(after_head, &after_head_lower, |i| { let (i, _) = tag(" that do").parse(i)?; diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index f2785c46e9..3b2946b7ce 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -14706,12 +14706,11 @@ fn shared_target_untap_or_tap_reversed_ordering_parses() { /// the grant has no turn-scoped expiry. Keyed on the typed `Keyword::Suspend` /// variant in `build_continuous_clause`. /// -/// NOTE: The parsed `SourceLacksKeyword { Suspend }` condition on this clause -/// resolves against the activating object (Jhoira / The Tenth Doctor), not -/// the exiled card. For these two cards this is inert — neither granter ever -/// has Suspend, so the grant fires unconditionally. A future "if it doesn't -/// have X" pattern whose subject is the cost-paid object will need a typed -/// `CostPaidObjectLacksKeyword` variant; flag and skip here. +/// This test asserts DURATION only. The clause's subject-binding condition is +/// covered separately: `it_doesnt_have_keyword_lowers_to_target_matches_filter` +/// pins the context-free lowering and +/// `keyword_anaphor_rebinds_to_cost_paid_object_after_cost_paid_parent` pins the +/// clause-context re-anchoring to the cost-paid object. #[test] fn suspend_keyword_grant_carries_permanent_duration() { use crate::types::keywords::Keyword; @@ -14751,6 +14750,672 @@ fn suspend_keyword_grant_carries_permanent_duration() { ); } +/// Expected shape of the keyword anaphor's context-free lowering: a +/// controller-agnostic, type-agnostic typed filter carrying exactly one +/// kind-level negated keyword predicate. +#[cfg(test)] +fn without_keyword_kind_filter(kind: crate::types::keywords::KeywordKind) -> TargetFilter { + TargetFilter::Typed(TypedFilter { + properties: vec![FilterProp::WithoutKeywordKind { value: kind }], + ..Default::default() + }) +} + +/// CR 702.62a + CR 608.2c + CR 608.2k: "if it doesn't have " is an +/// ANAPHOR to the object the preceding instruction / cost / trigger condition +/// introduced — never to the ability's source. Its context-free lowering must be +/// the target-scoped, KIND-level, off-zone-aware `TargetMatchesFilter`. +/// +/// Revert-fail: before this fix the arm emitted +/// `AbilityCondition::SourceLacksKeyword { Suspend }`, whose evaluator reads +/// `ability.source_id`, so every `assert_eq!` below fails on revert. +#[test] +fn it_doesnt_have_keyword_lowers_to_target_matches_filter() { + use crate::types::keywords::{Keyword, KeywordKind}; + + let def = parse_effect_chain( + "If it doesn't have suspend, it gains suspend", + AbilityKind::Spell, + ); + assert_eq!( + def.condition, + Some(AbilityCondition::TargetMatchesFilter { + filter: without_keyword_kind_filter(KeywordKind::Suspend), + use_lki: false, + subject_slot: None, + }), + "the anaphor must bind the ability's object subject, not its source" + ); + // Reach-guard: the gated body is still the real suspend grant, so the + // condition assertion above cannot pass vacuously against an + // `Effect::Unimplemented` short-circuit. + match &*def.effect { + Effect::GenericEffect { + static_abilities, + duration, + .. + } => { + assert_eq!(*duration, Some(Duration::Permanent)); + assert!( + static_abilities + .iter() + .any(|s| s.modifications.iter().any(|m| matches!( + m, + ContinuousModification::AddKeyword { + keyword: Keyword::Suspend { .. } + } + ))), + "the gated body must still be the suspend grant" + ); + } + other => panic!("expected the suspend grant body, got {other:?}"), + } + + // The "does not" spelling is the same arm. + let spelled_out = parse_effect_chain( + "If it does not have suspend, it gains suspend", + AbilityKind::Spell, + ); + assert_eq!( + spelled_out.condition, + Some(AbilityCondition::TargetMatchesFilter { + filter: without_keyword_kind_filter(KeywordKind::Suspend), + use_lki: false, + subject_slot: None, + }), + "\"does not have\" must lower identically to \"doesn't have\"" + ); + + // Parameterized on the keyword, not Suspend-specific (Momentum Rumbler's + // grammar). + let first_strike = parse_effect_chain( + "If it doesn't have first strike, put a first strike counter on it", + AbilityKind::Spell, + ); + assert_eq!( + first_strike.condition, + Some(AbilityCondition::TargetMatchesFilter { + filter: without_keyword_kind_filter(KeywordKind::FirstStrike), + use_lki: false, + subject_slot: None, + }), + "the arm must cover the whole keyword class, not just Suspend" + ); + + // Hostile fixture (Aven Courier's grammar): "doesn't have" followed by a + // COUNTER predicate is not a keyword at all — it must reach the + // `Keyword::Unknown` reject guard and never produce a keyword condition. + let counter_predicate = parse_effect_chain( + "Put a counter of that kind on target permanent you control if it doesn't \ + have a counter of that kind on it", + AbilityKind::Spell, + ); + // Reach-guard for the negative below. An absent keyword predicate is also + // what an upstream short-circuit produces, so first prove the clause was + // actually CONSUMED — by the counter path, as the `target_condition` rider on + // `PutChosenCounter`. CR 122.1 + CR 608.2c: "if it doesn't have a counter of + // that kind on it" is a count-of-the-chosen-kind == 0 eligibility test, not a + // keyword test. If this shape ever changes, the negative assertion below + // stops discriminating and must be re-derived alongside it. + match &*counter_predicate.effect { + Effect::PutChosenCounter { + target_condition, .. + } => assert_eq!( + target_condition.as_ref(), + Some(&crate::types::ability::ChosenCounterCountCondition { + comparator: Comparator::EQ, + rhs: QuantityExpr::Fixed { value: 0 }, + }), + "the counter clause must survive as the chosen-counter eligibility rider" + ), + other => panic!("the counter clause must reach the counter path, got {other:?}"), + } + assert!( + !matches!( + counter_predicate.condition, + Some(AbilityCondition::TargetMatchesFilter { + filter: TargetFilter::Typed(TypedFilter { ref properties, .. }), + .. + }) if properties + .iter() + .any(|p| matches!(p, FilterProp::WithoutKeywordKind { .. })) + ), + "a counter predicate must not be mistaken for a keyword predicate, got {:?}", + counter_predicate.condition + ); +} + +/// Expected shape of the affirmative twin's lowering — the kind-level mirror of +/// [`without_keyword_kind_filter`]. +#[cfg(test)] +fn has_keyword_kind_filter(kind: crate::types::keywords::KeywordKind) -> TargetFilter { + TargetFilter::Typed(TypedFilter { + properties: vec![FilterProp::HasKeywordKind { value: kind }], + ..Default::default() + }) +} + +/// CR 608.2c + CR 702.1 + CR 613.1f: polarity symmetry — the affirmative twin +/// lowers into the same `TargetMatchesFilter` family AND the same kind-level, +/// off-zone-aware prop, so both halves of a card like Momentum Rumbler read the +/// same subject through the same evaluator. +/// +/// Revert-fail: with the object-level `FilterProp::WithKeyword`, this assertion +/// fails — and, more importantly, the shape stops satisfying +/// `filter_is_bare_keyword_kind_predicate`, so the cost-paid re-anchoring can +/// never fire for the affirmative polarity (pinned below). +#[test] +fn it_has_keyword_lowers_to_the_same_kind_level_prop_as_its_negative_twin() { + use crate::types::keywords::KeywordKind; + let def = parse_effect_chain( + "If it has first strike, it gains double strike until end of turn", + AbilityKind::Spell, + ); + assert_eq!( + def.condition, + Some(AbilityCondition::TargetMatchesFilter { + filter: has_keyword_kind_filter(KeywordKind::FirstStrike), + use_lki: false, + subject_slot: None, + }), + "the affirmative arm must emit the kind-level HasKeywordKind prop" + ); +} + +/// CR 608.2k + CR 702.62a: the affirmative polarity reaches the cost-paid +/// re-anchoring too. An "if it has " gate after a cost-paid parent has an +/// EMPTY `targets` slot and (for an activated ability) no trigger event, so the +/// context-free target-scoped reading would fail closed and the gated body would +/// silently never run. +/// +/// Revert-fail: with the affirmative arm emitting `FilterProp::WithKeyword`, +/// `filter_is_bare_keyword_kind_predicate` rejects the shape and the condition +/// stays `TargetMatchesFilter`. +#[test] +fn affirmative_keyword_anaphor_rebinds_to_cost_paid_object() { + use crate::types::keywords::KeywordKind; + + // Jhoira's grammar with the affirmative polarity substituted, so the only + // difference from the pinned negative case is the polarity itself. + let def = parse_oracle_text( + "{2}, Exile a nonland card from your hand: Put four time counters on the \ + exiled card. If it has suspend, draw a card.", + "Affirmative Jhoira", + &[], + &["Creature".to_string()], + &[], + ); + let gated = def.abilities[0] + .sub_ability + .as_deref() + .expect("the gated draw is a sub-ability"); + assert_eq!( + gated.condition, + Some(AbilityCondition::CostPaidObjectMatchesFilter { + filter: has_keyword_kind_filter(KeywordKind::Suspend), + }), + "a cost-paid parent must re-anchor BOTH polarities to the cost-paid snapshot" + ); +} + +/// CR 702.11d + CR 702.14a + CR 702.16a + CR 702.124a: the keyword-presence +/// anaphor must strict-fail whenever `Keyword::kind()` does not identify the +/// parsed ability. Emitting the kind-level prop there would collapse ~60 +/// keywords into `KeywordKind::Unknown` ("it doesn't have storm" reading FALSE +/// for anything that happens to have banding), and the object-level props are +/// discriminant-matched, so they cannot express the parameterized families +/// either. Both polarities share the guard. +/// +/// Revert-fail: guarding only on the `Keyword::Unknown(_)` VARIANT lets every +/// row below through and produces a keyword predicate. +#[test] +fn keyword_anaphor_strict_fails_when_kind_does_not_identify_the_ability() { + fn keyword_predicate(def: &AbilityDefinition) -> Option<&FilterProp> { + let Some(AbilityCondition::TargetMatchesFilter { + filter: TargetFilter::Typed(TypedFilter { properties, .. }), + .. + }) = def.condition.as_ref() + else { + return None; + }; + properties.iter().find(|prop| { + matches!( + prop, + FilterProp::WithKeyword { .. } + | FilterProp::WithoutKeyword { .. } + | FilterProp::HasKeywordKind { .. } + | FilterProp::WithoutKeywordKind { .. } + ) + }) + } + + // (kind() == KeywordKind::Unknown) — parses from bare text, so the variant + // guard alone would pass it through. + for text in [ + "If it doesn't have storm, draw a card", + "If it doesn't have banding, draw a card", + "If it doesn't have melee, draw a card", + "If it has storm, draw a card", + // Parameter renames the printed keyword: one shared kind each. + "If it doesn't have hexproof, draw a card", + "If it doesn't have hexproof from black, draw a card", + "If it doesn't have islandwalk, draw a card", + "If it doesn't have partner, draw a card", + "If it has hexproof, draw a card", + ] { + let def = parse_effect_chain(text, AbilityKind::Spell); + // Per-row reach-guard: the strict failure must drop ONLY the gate. A row + // that never reached the keyword-presence arm — swallowed upstream, or + // collapsed to `Unimplemented` — would satisfy the negatives below for + // the wrong reason, so pin the surviving "draw a card" body first. + assert!( + matches!( + &*def.effect, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + } + ), + "{text:?} must keep its draw body — only the gate may be dropped, got {:?}", + def.effect + ); + assert_eq!( + keyword_predicate(&def), + None, + "{text:?} must not lower to a keyword predicate, got {:?}", + def.condition + ); + assert_eq!( + def.condition, None, + "{text:?} must drop the gate outright rather than lower it to some other condition" + ); + } + + // Positive control at the same seam: an identifying kind still lowers. + let suspend = parse_effect_chain( + "If it doesn't have suspend, draw a card", + AbilityKind::Spell, + ); + assert_eq!( + suspend.condition, + Some(AbilityCondition::TargetMatchesFilter { + filter: without_keyword_kind_filter(crate::types::keywords::KeywordKind::Suspend), + use_lki: false, + subject_slot: None, + }), + "the guard must narrow the arm, not disable it" + ); +} + +/// CR 608.2k: the anaphor's referent lives in a DIFFERENT runtime slot depending +/// on the clause that introduced it. When the preceding clause binds its subject +/// through the ability's COST (Jhoira of the Ghitu), `targets` is empty and there +/// is no trigger event, so the context-free target-scoped reading would fail +/// closed — the clause-context rewrite must re-anchor it to the cost-paid slot. +/// +/// Revert-fail: without `rewrite_keyword_anaphor_for_cost_paid_parent`, Jhoira's +/// gated sub-ability carries `TargetMatchesFilter` instead of +/// `CostPaidObjectMatchesFilter`. +#[test] +fn keyword_anaphor_rebinds_to_cost_paid_object_after_cost_paid_parent() { + use crate::types::keywords::KeywordKind; + + // Verbatim Jhoira of the Ghitu Oracle text (reminder text elided; the + // reminder is stripped before dispatch). + let jhoira = parse_oracle_text( + "{2}, Exile a nonland card from your hand: Put four time counters on the \ + exiled card. If it doesn't have suspend, it gains suspend.", + "Jhoira of the Ghitu", + &[], + &["Legendary".to_string(), "Creature".to_string()], + &["Human".to_string(), "Wizard".to_string()], + ); + let gated = jhoira.abilities[0] + .sub_ability + .as_deref() + .expect("Jhoira's suspend grant is a gated sub-ability"); + assert_eq!( + gated.condition, + Some(AbilityCondition::CostPaidObjectMatchesFilter { + filter: without_keyword_kind_filter(KeywordKind::Suspend), + }), + "a cost-paid parent must re-anchor the anaphor to the cost-paid snapshot" + ); + + // Negative sibling: an EFFECT-introduced referent (Kang Prime) keeps the + // context-free target-scoped reading — proving the rewrite is context-gated + // and does not fire on a `ParentTarget` parent. + let kang = parse_oracle_text( + "Flying\nWhenever Kang Prime enters or attacks, exile cards from the top of \ + your library until you exile a nonland card. Put two time counters on that \ + card. If it doesn't have suspend, it gains suspend.", + "Kang Prime", + &["Flying".to_string()], + &["Legendary".to_string(), "Creature".to_string()], + &["Human".to_string(), "Villain".to_string()], + ); + let kang_gated = kang.triggers[0] + .execute + .as_deref() + .and_then(|execute| execute.sub_ability.as_deref()) + .and_then(|put_counter| put_counter.sub_ability.as_deref()) + .expect("Kang Prime's suspend grant is the second gated sub-ability"); + assert_eq!( + kang_gated.condition, + Some(AbilityCondition::TargetMatchesFilter { + filter: without_keyword_kind_filter(KeywordKind::Suspend), + use_lki: false, + subject_slot: None, + }), + "an effect-introduced referent must keep the target-scoped reading" + ); + + // Hostile fixture: the sibling rewrite at the same hook (Ardyn's + // "If you exiled a card this way") must be unaffected — the two rewrites + // key on disjoint input shapes and must not interfere. + let ardyn = parse_effect_chain( + "Exile up to one target creature card from a graveyard. If you exiled a card \ + this way, draw a card.", + AbilityKind::Spell, + ); + let ardyn_gated = ardyn + .sub_ability + .as_deref() + .expect("the reflexive draw is a gated sub-ability"); + assert!( + matches!( + ardyn_gated.condition, + Some(AbilityCondition::ZoneChangedThisWay { .. }) + ), + "the effect-exile reflexive rewrite must still win its own shape, got {:?}", + ardyn_gated.condition + ); +} + +/// CR 608.2d + CR 115.10a: when the anaphor's referent is a RESOLUTION-TIME PICK +/// (The Eleventh Doctor, Amy's Home), it never reaches `ResolvedAbility.targets` +/// — the gate would silently fall back to the trigger source and re-grant the +/// keyword onto a card that already has it, clobbering its printed parameters, +/// while `cargo coverage` reported the card fully supported. The clause +/// strict-fails to `Effect::Unimplemented` instead, so the gap is visible. +/// +/// Revert-fail: without +/// `keyword_anaphor_referent_is_unpublished_resolution_pick` the gated grant +/// parses as a supported `GenericEffect` carrying `WithoutKeywordKind`. +#[test] +fn keyword_anaphor_after_resolution_time_pick_strict_fails_to_unimplemented() { + // Verbatim The Eleventh Doctor Oracle text (ability-word prefix elided; it + // is stripped before dispatch). + let doctor = parse_oracle_text( + "Whenever The Eleventh Doctor deals combat damage to a player, you may exile \ + a card from your hand with a number of time counters on it equal to its \ + mana value. If it doesn't have suspend, it gains suspend.", + "The Eleventh Doctor", + &[], + &["Legendary".to_string(), "Creature".to_string()], + &["Time Lord".to_string(), "Doctor".to_string()], + ); + let execute = doctor.triggers[0] + .execute + .as_deref() + .expect("the combat-damage trigger has an execute chain"); + assert!( + matches!(&*execute.effect, Effect::ChangeZone { .. }), + "the hand exile itself must still parse, got {:?}", + execute.effect + ); + let gated = execute + .sub_ability + .as_deref() + .expect("the suspend grant is the gated sub-ability"); + assert_eq!( + gated.effect.unimplemented_description(), + Some("If it doesn't have suspend, it gains suspend"), + "the unbindable gate must surface as a coverage gap, got {:?}", + gated.effect + ); + + // The class, not the card: Amy's Home is the second corpus member and words + // its pick differently ("a nonland card from your hand"). Parsed as a bare + // chain so the assertion is about the pick→anaphor pair, not about the + // planeswalk/upkeep trigger shell. + let amys_home = parse_effect_chain( + "you may exile a nonland card from your hand with a number of time counters \ + on it equal to its mana value. If it doesn't have suspend, it gains suspend.", + AbilityKind::Spell, + ); + assert_eq!( + amys_home + .sub_ability + .as_deref() + .and_then(|sub| sub.effect.unimplemented_description()), + Some("If it doesn't have suspend, it gains suspend"), + "the same pick→anaphor pair must strict-fail for Amy's Home, got {:?}", + amys_home.sub_ability + ); + + // Hostile fixture #1 — Delay. Its exile clause is `TargetChoiceTiming:: + // Resolution` too (off-battlefield origin, no printed "target"), but its + // filter is `TargetFilter::ParentTarget`: the resolver binds the countered + // spell deterministically, nothing is picked, and the anaphor binds. It must + // keep its condition. + let delay = parse_oracle_text( + "Counter target spell. If the spell is countered this way, exile it with \ + three time counters on it instead of putting it into its owner's graveyard. \ + If it doesn't have suspend, it gains suspend.", + "Delay", + &[], + &["Instant".to_string()], + &[], + ); + let delay_gated = delay.abilities[0] + .sub_ability + .as_deref() + .and_then(|exile| exile.sub_ability.as_deref()) + .expect("Delay's suspend grant is the second gated sub-ability"); + assert_eq!( + delay_gated.condition, + Some(AbilityCondition::TargetMatchesFilter { + filter: without_keyword_kind_filter(crate::types::keywords::KeywordKind::Suspend), + use_lki: false, + subject_slot: None, + }), + "a deterministic context-ref parent must not be mistaken for a player pick" + ); + + // Hostile fixture #2 — Kang Prime. An injected `ParentTarget` referent after + // a stack-timed parent is likewise untouched (the pinned baseline above + // asserts the same shape from the other direction). + let kang = parse_oracle_text( + "Flying\nWhenever Kang Prime enters or attacks, exile cards from the top of \ + your library until you exile a nonland card. Put two time counters on that \ + card. If it doesn't have suspend, it gains suspend.", + "Kang Prime", + &["Flying".to_string()], + &["Legendary".to_string(), "Creature".to_string()], + &["Human".to_string(), "Villain".to_string()], + ); + let kang_gated = kang.triggers[0] + .execute + .as_deref() + .and_then(|execute| execute.sub_ability.as_deref()) + .and_then(|put_counter| put_counter.sub_ability.as_deref()) + .expect("Kang Prime's suspend grant is the second gated sub-ability"); + assert!( + kang_gated.condition.is_some(), + "Kang Prime's target-scoped reading must survive" + ); + assert_eq!( + kang_gated.effect.unimplemented_description(), + None, + "Kang Prime must stay supported, got {:?}", + kang_gated.effect + ); +} + +/// CR 702.1c: a replicated "the same is true for " continuation +/// must swap the gating keyword on BOTH subject seams the anaphor can lower to. +/// Before this change the class lived in `SourceLacksKeyword`, which +/// `rewrite_ability_condition_keyword` handles explicitly; moving it into the +/// `*MatchesFilter` family without these arms would silently drop the swap into +/// the walker's `_ => {}` fallback. +/// +/// Revert-fail: removing either the condition arms or the `rewrite_filter_keyword` +/// kind-prop arm leaves the `KeywordKind::Suspend` value in place. +#[test] +fn rewrite_ability_condition_keyword_swaps_subject_filter_keyword_props() { + use crate::types::keywords::{Keyword, KeywordKind}; + + // (a) `TargetMatchesFilter` — the effect/trigger-subject seam. + let mut target_seam = AbilityCondition::TargetMatchesFilter { + filter: without_keyword_kind_filter(KeywordKind::Suspend), + use_lki: false, + subject_slot: None, + }; + rewrite_ability_condition_keyword(&mut target_seam, &Keyword::Flying); + assert_eq!( + target_seam, + AbilityCondition::TargetMatchesFilter { + filter: without_keyword_kind_filter(KeywordKind::Flying), + use_lki: false, + subject_slot: None, + } + ); + + // (b) `CostPaidObjectMatchesFilter` — the cost-paid seam. + let mut cost_seam = AbilityCondition::CostPaidObjectMatchesFilter { + filter: without_keyword_kind_filter(KeywordKind::Suspend), + }; + rewrite_ability_condition_keyword(&mut cost_seam, &Keyword::Flying); + assert_eq!( + cost_seam, + AbilityCondition::CostPaidObjectMatchesFilter { + filter: without_keyword_kind_filter(KeywordKind::Flying), + } + ); + + // (c) The affirmative kind-level prop swaps too, asserted separately so each + // kind prop carries its own proof. + let mut has_kind = AbilityCondition::TargetMatchesFilter { + filter: TargetFilter::Typed(TypedFilter { + properties: vec![FilterProp::HasKeywordKind { + value: KeywordKind::Suspend, + }], + ..Default::default() + }), + use_lki: false, + subject_slot: None, + }; + rewrite_ability_condition_keyword(&mut has_kind, &Keyword::Flying); + assert_eq!( + has_kind, + AbilityCondition::TargetMatchesFilter { + filter: TargetFilter::Typed(TypedFilter { + properties: vec![FilterProp::HasKeywordKind { + value: KeywordKind::Flying + }], + ..Default::default() + }), + use_lki: false, + subject_slot: None, + } + ); + + // (d) Pre-existing behavior must not move: the object-level `WithKeyword` + // prop inside a `ZoneChangeObjectMatchesFilter` (Mutable Pupa) still swaps. + let mut pupa = AbilityCondition::ZoneChangeObjectMatchesFilter { + origin: None, + destination: Zone::Battlefield, + filter: TargetFilter::Typed(TypedFilter { + properties: vec![FilterProp::WithKeyword { + value: Keyword::Suspend { + count: 0, + cost: ManaCost::default(), + }, + }], + ..Default::default() + }), + }; + rewrite_ability_condition_keyword(&mut pupa, &Keyword::Flying); + match &pupa { + AbilityCondition::ZoneChangeObjectMatchesFilter { + filter: TargetFilter::Typed(TypedFilter { properties, .. }), + .. + } => assert_eq!( + properties.as_slice(), + [FilterProp::WithKeyword { + value: Keyword::Flying + }] + ), + other => panic!("the Mutable Pupa shape must be preserved, got {other:?}"), + } + + // (e) Super-Adaptoid's conjunction still rewrites both conjuncts. + let mut conjunction = AbilityCondition::And { + conditions: vec![ + AbilityCondition::TargetHasKeywordInstead { + keyword: Keyword::Suspend { + count: 0, + cost: ManaCost::default(), + }, + }, + AbilityCondition::SourceLacksKeyword { + keyword: Keyword::Suspend { + count: 0, + cost: ManaCost::default(), + }, + }, + ], + }; + rewrite_ability_condition_keyword(&mut conjunction, &Keyword::Flying); + assert_eq!( + conjunction, + AbilityCondition::And { + conditions: vec![ + AbilityCondition::TargetHasKeywordInstead { + keyword: Keyword::Flying + }, + AbilityCondition::SourceLacksKeyword { + keyword: Keyword::Flying + }, + ], + } + ); +} + +/// Scope-boundary pin for the `rewrite_filter_keyword` extension: the +/// OBJECT-level `FilterProp::WithoutKeyword` is deliberately left unhandled. No +/// card routes it through this walker today, and adding it would change behavior +/// for pre-existing conditions reachable via the `ZoneChangeObjectMatchesFilter` +/// arm — outside the keyword-anaphor class. A future widening must flip this +/// assertion consciously rather than land silently. +#[test] +fn rewrite_filter_keyword_leaves_object_level_without_keyword_alone() { + use crate::types::keywords::Keyword; + let mut filter = TargetFilter::Typed(TypedFilter { + properties: vec![FilterProp::WithoutKeyword { + value: Keyword::Suspend { + count: 0, + cost: ManaCost::default(), + }, + }], + ..Default::default() + }); + rewrite_filter_keyword(&mut filter, &Keyword::Flying); + match &filter { + TargetFilter::Typed(TypedFilter { properties, .. }) => assert!( + matches!( + properties.as_slice(), + [FilterProp::WithoutKeyword { + value: Keyword::Suspend { .. } + }] + ), + "object-level WithoutKeyword must stay unchanged, got {properties:?}" + ), + other => panic!("expected a typed filter, got {other:?}"), + } +} + /// Issue #501 FOLLOW-UP — negative sibling for Root Cause A. The /// `Duration::Permanent` override is keyed strictly on `Keyword::Suspend`: /// an ordinary "gains flying" combat trick still parses to @@ -25987,8 +26652,10 @@ fn if_its_a_subtype_otherwise_attaches_else_branch() { ); } -/// CR 608.2c + CR 702.1: "If it has [keyword], A. Otherwise, B." — affirmative -/// keyword guard parses to FilterProp::WithKeyword and attaches the else-branch. +/// CR 608.2c + CR 702.1 + CR 613.1f: "If it has [keyword], A. Otherwise, B." — +/// affirmative keyword guard parses to the kind-level FilterProp::HasKeywordKind +/// (shared with its negative twin via `keyword_presence_kind`) and attaches the +/// else-branch. #[test] fn if_it_has_keyword_otherwise_attaches_else_branch() { let def = parse_effect_chain( @@ -26009,10 +26676,10 @@ fn if_it_has_keyword_otherwise_attaches_else_branch() { panic!("expected Typed keyword filter"); }; assert!( - tf.properties.contains(&FilterProp::WithKeyword { - value: Keyword::Flying + tf.properties.contains(&FilterProp::HasKeywordKind { + value: crate::types::keywords::KeywordKind::Flying }), - "expected WithKeyword(Flying), got {:?}", + "expected HasKeywordKind(Flying), got {:?}", tf.properties ); assert!(matches!(*sub.effect, Effect::Destroy { .. })); diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index d7cb3c6ec4..c3a364072f 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -16140,6 +16140,32 @@ impl TargetFilter { } } + /// CR 613.1f + CR 702.1: True when this filter tree asks a KIND-level keyword + /// question (`HasKeywordKind` / `WithoutKeywordKind`) at any structural + /// position. Mirrors [`Self::references_cost_paid_object`]'s recursion. + /// + /// Those two props are the only ones that must be answered from an object's + /// LIVE effective keyword set rather than a snapshot, because they route + /// through the off-zone Layer-6 ledger. Evaluators that would otherwise pay + /// to recompute that ledger use this as a cheap early-out — see + /// `game::filter::matches_target_filter_on_cost_paid_reference`. + pub fn queries_keyword_kind(&self) -> bool { + match self { + TargetFilter::Typed(TypedFilter { properties, .. }) => properties.iter().any(|prop| { + matches!( + prop, + FilterProp::HasKeywordKind { .. } | FilterProp::WithoutKeywordKind { .. } + ) + }), + TargetFilter::And { filters } | TargetFilter::Or { filters } => { + filters.iter().any(TargetFilter::queries_keyword_kind) + } + TargetFilter::Not { filter } => filter.queries_keyword_kind(), + TargetFilter::TrackedSetFiltered { filter, .. } => filter.queries_keyword_kind(), + _ => false, + } + } + pub fn contains_source_attachment_host(&self) -> bool { match self { TargetFilter::Typed(TypedFilter { properties, .. }) => properties diff --git a/crates/engine/src/types/keywords.rs b/crates/engine/src/types/keywords.rs index 446d169468..c6f1496a67 100644 --- a/crates/engine/src/types/keywords.rs +++ b/crates/engine/src/types/keywords.rs @@ -1673,6 +1673,287 @@ impl Keyword { } } + /// True when [`Keyword::kind`] IDENTIFIES this keyword ability — the + /// returned `KeywordKind` names this ability and no other, so a kind-level + /// presence test ("does it have suspend?", CR 702.62a) asks exactly the + /// question the Oracle text asks. + /// + /// False in exactly two situations, each of which makes a kind-level test + /// answer a DIFFERENT question than the printed text: + /// + /// * `KeywordKind::Unknown` — the catch-all bucket the `kind()` match + /// above assigns to ~60 unrelated keywords (`Banding`, `Melee`, + /// `Storm`, `Toxic`, `Echo`, `StartingIntensity`, …). "Has an + /// Unknown-kind keyword" is TRUE for a creature with banding when the + /// text asked about storm. The off-zone keyword ledger + /// (`game::off_zone_characteristics`) is kind-indexed as well, so these + /// keywords have no per-ability presence answer off the battlefield at + /// all. + /// * the families whose PRINTED keyword name varies with the parameter: + /// "protection from red" (CR 702.16a), "hexproof from black" + /// (CR 702.11d — which shares `KeywordKind::Hexproof` with plain + /// hexproof), "islandwalk" (CR 702.14a), "landcycling" (CR 702.29e), + /// and the partner family (CR 702.124a). Each parameter value is its + /// own keyword ability, and they all share one kind. + /// + /// A `false` answer means "no exact presence test exists", NOT "use + /// `FilterProp::WithKeyword`/`WithoutKeyword` instead": those props are + /// discriminant-matched on the live-object path + /// (`game::keywords::has_keyword`) and value-matched on the snapshot paths + /// (`game::filter::spell_record_matches_property`), so neither is a + /// per-ability test either — the first cannot separate protection from red + /// from protection from blue, and the second cannot separate `Suspend 4—{U}` + /// from `Suspend 0—{}`. Callers should strict-fail so coverage stays honest. + /// + /// Deliberately conservative: `Partner(DoctorsCompanion)` and + /// `Partner(ChooseABackground)` do get their own kinds, but the whole + /// partner family answers `false` because under-reporting injectivity only + /// costs a strict failure, while over-reporting it ships a wrong guard. + /// + /// Maintenance: this is a hand-derived property of the `kind()` match above, + /// so the census below is exhaustive for the same reason that one is — adding + /// a `Keyword` variant fails compilation here until the author makes the + /// injectivity call. + /// + /// Do NOT collapse the `true` arm back into an `other => other.kind() != + /// KeywordKind::Unknown` fallback. That derivation is only sound while every + /// non-`Unknown` kind has exactly one `Keyword` variant producing it, which is + /// a property of the *current* `kind()` match rather than a guarantee: a new + /// variant that aliases an existing non-`Unknown` kind (the + /// `Hexproof`/`HexproofFrom` shape) would silently answer `true` and ship the + /// wrong guard. A new parameterized family whose parameter renames the printed + /// keyword belongs in the first `false` arm. + pub fn kind_identifies_ability(&self) -> bool { + match self { + // CR 702.11d + CR 702.14a + CR 702.16a + CR 702.29e + CR 702.124a: + // the PRINTED keyword ability name varies with the parameter, so a + // single kind spans several distinct abilities and no kind-level test + // can separate them. + Keyword::Hexproof + | Keyword::HexproofFrom(_) + | Keyword::Landwalk(_) + | Keyword::Partner(_) + | Keyword::Protection(_) + | Keyword::Typecycling { .. } => false, + + // The `KeywordKind::Unknown` catch-all bucket — "has an Unknown-kind + // keyword" is true for a creature with banding when the text asked + // about storm, and the kind-indexed off-zone ledger has no per-ability + // answer for these at all. + Keyword::Affinity(_) + | Keyword::Amplify(_) + | Keyword::Backup(_) + | Keyword::Banding + | Keyword::Bloodthirst(_) + | Keyword::Buyback(_) + | Keyword::Casualty(_) + | Keyword::Compleated + | Keyword::Conspire + | Keyword::CumulativeUpkeep(_) + | Keyword::Daybound + | Keyword::Demonstrate + | Keyword::Dethrone + | Keyword::Discover(_) + | Keyword::DoubleTeam + | Keyword::Echo(_) + | Keyword::Emerge(_) + | Keyword::Encore(_) + | Keyword::Enlist + | Keyword::Entwine(_) + | Keyword::Epic + | Keyword::Evoke(_) + | Keyword::Fortify(_) + | Keyword::Gravestorm + | Keyword::Haunt + | Keyword::Hideaway(_) + | Keyword::Impending { .. } + | Keyword::Improvise + | Keyword::Ingest + | Keyword::LevelUp(_) + | Keyword::LivingMetal + | Keyword::Melee + | Keyword::Mentor + | Keyword::Mobilize(_) + | Keyword::Myriad + | Keyword::Nightbound + | Keyword::Overload(_) + | Keyword::Poisonous(_) + | Keyword::Prototype { .. } + | Keyword::Provoke + | Keyword::Prowl(_) + | Keyword::Ravenous + | Keyword::ReadAhead + | Keyword::Rebound + | Keyword::Reinforce { .. } + | Keyword::Ripple(_) + | Keyword::Saddle(_) + | Keyword::Scavenge(_) + | Keyword::Soulshift(_) + | Keyword::Spectacle(_) + | Keyword::SplitSecond + | Keyword::Spree + | Keyword::Squad(_) + | Keyword::StartingIntensity(_) + | Keyword::Storm + | Keyword::Surge(_) + | Keyword::Teamwork(_) + | Keyword::Totem + | Keyword::Toxic(_) + | Keyword::Unknown(_) + | Keyword::WebSlinging(_) => false, + + // 1:1 with their kind: the kind names this ability and no other, so a + // kind-level presence test asks exactly what the Oracle text asks. + Keyword::Absorb(_) + | Keyword::Afflict(_) + | Keyword::Afterlife(_) + | Keyword::Aftermath + | Keyword::Annihilator(_) + | Keyword::Ascend + | Keyword::Assist + | Keyword::Augment + | Keyword::Awaken { .. } + | Keyword::BandsWithOther(_) + | Keyword::Bargain + | Keyword::Battlecry + | Keyword::Bestow(_) + | Keyword::Blitz(_) + | Keyword::Bushido(_) + | Keyword::Cascade + | Keyword::Champion(_) + | Keyword::Changeling + | Keyword::Cipher + | Keyword::Cleave(_) + | Keyword::CommanderNinjutsu(_) + | Keyword::Companion(_) + | Keyword::Convoke + | Keyword::Craft { .. } + | Keyword::Crew { .. } + | Keyword::Cycling(_) + | Keyword::Dash(_) + | Keyword::Deathtouch + | Keyword::Decayed + | Keyword::Defender + | Keyword::Delve + | Keyword::Devoid + | Keyword::Devour { .. } + | Keyword::Disguise(_) + | Keyword::Disturb(_) + | Keyword::DoubleStrike + | Keyword::Dredge(_) + | Keyword::Embalm(_) + | Keyword::Enchant(_) + | Keyword::Equip(_) + | Keyword::Escalate(_) + | Keyword::Escape(_) + | Keyword::EtbCounter { .. } + | Keyword::Eternalize(_) + | Keyword::Evolve + | Keyword::Exalted + | Keyword::Exploit + | Keyword::Explore + | Keyword::Extort + | Keyword::Fabricate(_) + | Keyword::Fading(_) + | Keyword::Fear + | Keyword::Firebending(_) + | Keyword::FirstStrike + | Keyword::Flanking + | Keyword::Flash + | Keyword::Flashback(_) + | Keyword::Flying + | Keyword::ForMirrodin + | Keyword::Foretell(_) + | Keyword::Freerunning(_) + | Keyword::Frenzy(_) + | Keyword::Fuse + | Keyword::Gift(_) + | Keyword::Graft(_) + | Keyword::Harmonize(_) + | Keyword::Haste + | Keyword::Horsemanship + | Keyword::Increment + | Keyword::Indestructible + | Keyword::Infect + | Keyword::Intimidate + | Keyword::JobSelect + | Keyword::JumpStart + | Keyword::Kicker(_) + | Keyword::Lifelink + | Keyword::LivingWeapon + | Keyword::Madness(_) + | Keyword::Mayhem(_) + | Keyword::Megamorph(_) + | Keyword::Menace + | Keyword::Miracle(_) + | Keyword::Modular(_) + | Keyword::MoreThanMeetsTheEye(_) + | Keyword::Morph(_) + | Keyword::Mutate(_) + | Keyword::Ninjutsu(_) + | Keyword::Offering(_) + | Keyword::Offspring(_) + | Keyword::Outlast(_) + | Keyword::Paradigm + | Keyword::Persist + | Keyword::Phasing + | Keyword::Plot(_) + | Keyword::Prowess + | Keyword::Rampage(_) + | Keyword::Reach + | Keyword::Reconfigure(_) + | Keyword::Recover(_) + | Keyword::Renown(_) + | Keyword::Replicate(_) + | Keyword::Retrace + | Keyword::Riot + | Keyword::Shadow + | Keyword::Shroud + | Keyword::Skulk + | Keyword::Sneak(_) + | Keyword::Soulbond + | Keyword::Specialize(_) + | Keyword::Splice { .. } + | Keyword::StartYourEngines + | Keyword::Station + | Keyword::Storied + | Keyword::Sunburst + | Keyword::Suspend { .. } + | Keyword::TotemArmor + | Keyword::Training + | Keyword::Trample + | Keyword::TrampleOverPlaneswalkers + | Keyword::Transfigure(_) + | Keyword::Transmute(_) + | Keyword::Tribute(_) + | Keyword::Undaunted + | Keyword::Undying + | Keyword::Unearth(_) + | Keyword::Unleash + | Keyword::Vanishing(_) + | Keyword::Vigilance + | Keyword::Ward(_) + | Keyword::Warp(_) + | Keyword::Waterbend + | Keyword::Wither => { + // The census above is hand-derived, so it can disagree with + // `kind()` in a way the compiler cannot see: a variant listed + // here but mapped to the catch-all would hand a kind-level + // presence test the shared `Unknown` bucket — the exact + // over-report this predicate exists to prevent. Only this arm + // can be wrong that way (the other two answer `false`, which is + // always safe), and the predicate runs at parse time, so pin the + // invariant rather than deriving it. + debug_assert_ne!( + self.kind(), + KeywordKind::Unknown, + "{self:?} is censused as kind-identifying but maps to the Unknown bucket", + ); + true + } + } + } + /// CR 601.2f + CR 707.2: Keywords that only function while a player is /// casting a spell. A token created by `CopyTokenOf` was not cast, so these /// keywords are inert on the copy and are stripped at creation time so the @@ -3665,6 +3946,78 @@ mod tests { use super::*; use crate::types::ability::Effect; + /// CR 702.62a + CR 702.7a: the ordinary case — a keyword whose `kind()` names + /// it and nothing else supports a kind-level presence test. + #[test] + fn kind_identifies_ability_accepts_one_to_one_keywords() { + for keyword in [ + Keyword::Suspend { + count: 4, + cost: crate::types::mana::ManaCost::generic(1), + }, + Keyword::FirstStrike, + Keyword::Flying, + Keyword::Ward(WardCost::Mana(crate::types::mana::ManaCost::generic(2))), + Keyword::Foretell(crate::types::mana::ManaCost::generic(2)), + ] { + assert!( + keyword.kind_identifies_ability(), + "{keyword:?} maps 1:1 onto {:?}", + keyword.kind() + ); + } + } + + /// The catch-all bucket: ~60 unrelated keywords share `KeywordKind::Unknown`, + /// so a kind-level presence test on any of them answers "does it have ANY + /// Unknown-kind keyword" — TRUE for a creature with banding when the text + /// asked about storm. + #[test] + fn kind_identifies_ability_rejects_the_unknown_bucket() { + for keyword in [ + Keyword::Storm, + Keyword::Banding, + Keyword::Melee, + Keyword::Mentor, + Keyword::Toxic(1), + Keyword::StartingIntensity(3), + Keyword::Unknown("rapid fire".to_string()), + ] { + assert_eq!(keyword.kind(), KeywordKind::Unknown); + assert!( + !keyword.kind_identifies_ability(), + "{keyword:?} shares the catch-all kind" + ); + } + } + + /// CR 702.11d + CR 702.14a + CR 702.16a + CR 702.29e + CR 702.124a: families + /// whose PRINTED keyword name varies with the parameter. Each parameter value + /// is its own keyword ability, and they all collapse into one kind — + /// `KeywordKind::Hexproof` even absorbs plain hexproof. + #[test] + fn kind_identifies_ability_rejects_parameter_renamed_families() { + assert_eq!(Keyword::Hexproof.kind(), KeywordKind::Hexproof); + assert_eq!( + Keyword::HexproofFrom(HexproofFilter::Color(crate::types::mana::ManaColor::Black)) + .kind(), + KeywordKind::Hexproof, + ); + for keyword in [ + Keyword::Hexproof, + Keyword::HexproofFrom(HexproofFilter::Color(crate::types::mana::ManaColor::Black)), + Keyword::Protection(ProtectionTarget::Color(crate::types::mana::ManaColor::Red)), + Keyword::Landwalk("Island".to_string()), + Keyword::Partner(PartnerType::Generic), + ] { + assert!( + !keyword.kind_identifies_ability(), + "{keyword:?} shares {:?} with a differently-named keyword ability", + keyword.kind() + ); + } + } + /// CR 702.143d + CR 702 (alt-cost family): `with_cost` maps each variant to /// its `Keyword::X(ManaCost)`, and `matches_keyword`/`from_name` round-trip. #[test] diff --git a/crates/engine/src/types/zones.rs b/crates/engine/src/types/zones.rs index 692dbe7467..2645bf46ba 100644 --- a/crates/engine/src/types/zones.rs +++ b/crates/engine/src/types/zones.rs @@ -16,6 +16,23 @@ pub enum Zone { Command, } +impl Zone { + /// CR 400.2: the battlefield, graveyard, stack, exile, and command zones are + /// public; the library and hand are hidden even when their cards are + /// momentarily revealed. + /// + /// The distinction is load-bearing for CR 608.2h: an effect that needs + /// information from a specific object reads that object's CURRENT + /// characteristics while it is in the public zone it was expected to be in, + /// and falls back to last known information once it is not. + pub fn is_public(self) -> bool { + match self { + Zone::Battlefield | Zone::Graveyard | Zone::Stack | Zone::Exile | Zone::Command => true, + Zone::Library | Zone::Hand => false, + } + } +} + /// CR 118.9a + CR 601.2b + CR 601.2h: Source zone for an `AbilityCost::Exile` /// payment. Only `Hand` (pitch spells, CR 118.9a) and `Graveyard` (escape, /// CR 702.138a) are valid; any other zone is rejected at cost-resolution time diff --git a/crates/engine/tests/integration/keyword_anaphor_subject_binding.rs b/crates/engine/tests/integration/keyword_anaphor_subject_binding.rs new file mode 100644 index 0000000000..134891c956 --- /dev/null +++ b/crates/engine/tests/integration/keyword_anaphor_subject_binding.rs @@ -0,0 +1,715 @@ +//! Subject binding for the keyword-presence anaphor — "If it doesn't have +//! , …" (Kang Prime, Jhoira of the Ghitu, Suspend, Delay, Momentum +//! Rumbler, …). +//! +//! `it` is an ANAPHOR to the object introduced by the preceding instruction, by +//! the ability's cost, or by the trigger condition — never to the ability's +//! source. Which rule supplies the referent depends on the binding class: +//! +//! * CR 608.2c — a referent introduced by a PRECEDING INSTRUCTION of the same +//! effect. "Read the whole text and apply the rules of English to the text": +//! `it` in "Put two time counters on that card. If it doesn't have suspend" +//! is the nonland card the earlier sentence exiled (Kang Prime, Suspend, +//! Delay, Doom's Time Platform). +//! * CR 608.2k — a referent previously referred to by the ability's COST or +//! TRIGGER CONDITION, which keeps pointing at that object even after its +//! characteristics change (Jhoira of the Ghitu's cost-paid card; Momentum +//! Rumbler's attacking creature). +//! +//! The parser used to lower it to +//! `AbilityCondition::SourceLacksKeyword`, whose evaluator reads +//! `ability.source_id`, so the gate was unconditionally TRUE for every card +//! whose `it` is not the source. The observable symptom is a redundant grant +//! onto a card that already has the keyword, which clobbers the card's PRINTED +//! keyword parameters: `off_zone_characteristics::upsert_keyword_contribution` +//! replaces a same-kind contribution unless the keyword is a summing keyword, +//! and `Keyword::instances_must_coexist` does not list Suspend. A card exiled +//! with printed `Suspend 4—{U}` came back as `Suspend 0—{}`. +//! +//! One module per RUNTIME BINDING CLASS, because the referent lives in a +//! different slot per clause shape. Note the two distinct condition-evaluation +//! seams in `game::effects::resolve_chain_body`: +//! +//! * the SUB-ABILITY gate, which passes the PARENT node as the condition +//! ability — used by `injected_target` and `declared_stack_target` below; +//! * the TOP-LEVEL gate, which passes the RESOLVING node itself — used by +//! `top_level_trigger_source` below, and by no other module here. That is +//! where an intervening-"if" condition is rechecked on resolution +//! (CR 603.4 + CR 608.2a). +//! +//! The cost-paid binding class (Jhoira of the Ghitu) is covered in +//! `crates/engine/src/game/casting_tests.rs`, next to the pre-existing +//! activation-pipeline test it must not regress. + +use engine::game::combat::AttackTarget; +use engine::game::keywords::{effective_suspend_cost, object_has_effective_keyword_kind}; +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::types::actions::GameAction; +use engine::types::counter::CounterType; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::keywords::{Keyword, KeywordKind}; +use engine::types::mana::{ManaCost, ManaCostShard}; +use engine::types::zones::Zone; +use engine::types::Phase; + +/// Verbatim Oracle text (reminder text elided — it is stripped before dispatch). +const KANG_PRIME: &str = "Flying\nWhenever Kang Prime enters or attacks, exile cards \ + from the top of your library until you exile a nonland card. Put two time counters \ + on that card. If it doesn't have suspend, it gains suspend."; + +const MOMENTUM_RUMBLER: &str = "Whenever this creature attacks, if it doesn't have \ + first strike, put a first strike counter on it.\nWhenever this creature attacks, \ + if it has first strike, it gains double strike until end of turn."; + +const SUSPEND_CARD: &str = "Exile target creature and put two time counters on it. \ + If it doesn't have suspend, it gains suspend."; + +/// `{U}` — the printed suspend cost the redundant grant used to clobber to `{0}`. +fn blue_mana_cost() -> ManaCost { + ManaCost::Cost { + shards: vec![ManaCostShard::Blue], + generic: 0, + } +} + +/// Printed `Suspend 4—{U}` — the parameters the redundant grant used to clobber. +fn printed_suspend_four_blue() -> Keyword { + Keyword::Suspend { + count: 4, + cost: blue_mana_cost(), + } +} + +/// Drive the pipeline to the ONE terminal state these scenarios may end in: an +/// empty stack at a priority window. +/// +/// Every other exit is a test failure, not a stopping condition. An action +/// error, an unanticipated prompt, or running out of steps all mean the chain +/// under test never resolved — and the assertions downstream are written so a +/// stalled game passes them for the wrong reason: a card still sitting in the +/// library trivially has no suspend, no time counters, and is not the source. +/// Returning quietly from here would turn every one of them into coverage +/// theatre, so each non-terminal exit panics with the state that caused it. +fn settle(runner: &mut GameRunner) { + for _ in 0..60 { + match runner.state().waiting_for.clone() { + WaitingFor::OrderTriggers { .. } => { + engine::game::triggers::drain_order_triggers_with_identity(runner.state_mut()); + } + WaitingFor::TriggerTargetSelection { .. } | WaitingFor::TargetSelection { .. } => { + runner + .choose_first_legal_target() + .expect("a pending target selection must offer a legal target"); + } + WaitingFor::Priority { .. } => { + if runner.state().stack.is_empty() { + return; + } + runner + .act(GameAction::PassPriority) + .expect("passing priority on a non-empty stack must be legal"); + } + other => panic!("unexpected prompt while settling the stack: {other:?}"), + } + } + panic!( + "the stack never emptied within 60 steps (waiting_for = {:?}, stack depth = {})", + runner.state().waiting_for, + runner.state().stack.len(), + ); +} + +/// Binding class 1 — the referent is INJECTED into the parent's `targets` by the +/// producing instruction (`ExileFromTopUntil` stamps the hit onto the sub-chain). +/// Evaluated at the sub-ability gate with the `PutCounter` parent as the +/// condition ability. Kang Prime and The Tenth Doctor share this shape. +mod injected_target { + use super::*; + + fn kang_scenario(library_card_has_printed_suspend: bool) -> (GameRunner, ObjectIdPair) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + let kang = scenario + .add_creature_from_oracle(P0, "Kang Prime", 3, 5, KANG_PRIME) + .id(); + + let mut builder = scenario.add_spell_to_library_top(P0, "Exiled Sorcery", false); + if library_card_has_printed_suspend { + builder.with_keyword(printed_suspend_four_blue()); + } + let exiled = builder.id(); + + (scenario.build(), ObjectIdPair { kang, exiled }) + } + + struct ObjectIdPair { + kang: ObjectId, + exiled: ObjectId, + } + + /// THE FIX. CR 608.2c + CR 702.62a: `it` is the card the PRECEDING + /// instruction exiled, and that card already has printed `Suspend 4—{U}`, so + /// the gate must be FALSE and no grant may fire. + /// + /// Revert-fail: with `SourceLacksKeyword`, the gate reads Kang Prime (which + /// never has suspend), fires the grant, and + /// `upsert_keyword_contribution` overwrites the printed contribution with + /// the granted `Suspend { count: 0, cost: {} }` — so + /// `effective_off_zone_keyword` returns `Suspend 0—{}` and this assertion + /// fails. + #[test] + fn natively_suspended_exiled_card_keeps_its_printed_parameters() { + let (mut runner, ids) = kang_scenario(true); + + runner.advance_to_combat(); + runner + .declare_attackers(&[(ids.kang, AttackTarget::Player(P1))]) + .expect("Kang Prime must be able to attack"); + settle(&mut runner); + + // Reach-guard: the chain actually ran — the card left the library for + // exile and took its two time counters (CR 122.1). + assert_eq!( + runner.state().objects[&ids.exiled].zone, + Zone::Exile, + "the nonland card must be exiled by ExileFromTopUntil" + ); + assert_eq!( + runner.state().objects[&ids.exiled] + .counters + .get(&CounterType::Time) + .copied(), + Some(2), + "CR 122.1: the exiled card must carry two time counters" + ); + + // CR 702.62a: "Suspend N—[cost]" — the printed parameters must survive. + assert_eq!( + effective_suspend_cost(runner.state(), ids.exiled), + Some(blue_mana_cost()), + "the anaphor must read the EXILED CARD: it already has suspend, so no \ + redundant grant may clobber its printed Suspend 4—{{U}} down to {{0}}" + ); + } + + /// Positive sibling — the gate is not vacuously false. A card WITHOUT + /// printed suspend still gains it (CR 702.62a), so the fix narrows the gate + /// rather than disabling it. + #[test] + fn exiled_card_without_suspend_still_gains_it() { + let (mut runner, ids) = kang_scenario(false); + + runner.advance_to_combat(); + runner + .declare_attackers(&[(ids.kang, AttackTarget::Player(P1))]) + .expect("Kang Prime must be able to attack"); + settle(&mut runner); + + assert_eq!( + runner.state().objects[&ids.exiled].zone, + Zone::Exile, + "the nonland card must be exiled by ExileFromTopUntil" + ); + assert_eq!( + runner.state().objects[&ids.exiled] + .counters + .get(&CounterType::Time) + .copied(), + Some(2), + "CR 122.1: the exiled card must carry two time counters" + ); + assert!( + object_has_effective_keyword_kind(runner.state(), ids.exiled, KeywordKind::Suspend), + "a card with no printed suspend must still gain it" + ); + } + + /// Hostile fixture — no legal referent. With an empty library the + /// `ExileFromTopUntil` finds no nonland hit, so the gated sub-chain never + /// runs: no panic, no grant, and Kang Prime itself must not be touched. + #[test] + fn no_nonland_hit_grants_nothing_and_does_not_touch_the_source() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let kang = scenario + .add_creature_from_oracle(P0, "Kang Prime", 3, 5, KANG_PRIME) + .id(); + let mut runner = scenario.build(); + + runner.advance_to_combat(); + runner + .declare_attackers(&[(kang, AttackTarget::Player(P1))]) + .expect("Kang Prime must be able to attack"); + settle(&mut runner); + + assert!( + !object_has_effective_keyword_kind(runner.state(), kang, KeywordKind::Suspend), + "the ability's SOURCE must never be the anaphor's referent" + ); + assert_eq!( + runner.state().objects[&kang] + .counters + .get(&CounterType::Time) + .copied() + .unwrap_or(0), + 0, + "no time counters may land on the source" + ); + } +} + +/// Binding class 2 — the condition sits on the TOP-LEVEL `execute` node, so the +/// resolving node itself is the condition ability. Its `targets` are empty (a +/// `SelfRef`-slotted `PutCounter` declares no choosable slot), so the subject +/// resolves through `TargetMatchesFilter`'s `TriggeringSource` fallback against +/// the singleton `AttackersDeclared` event. +/// +/// Momentum Rumbler is the only corpus card on this seam, and it is the +/// REGRESSION TRIPWIRE: its `it` really is the trigger source, so behavior must +/// be identical before and after the lowering change. +mod top_level_trigger_source { + use super::*; + + fn first_strike_counters(runner: &GameRunner, id: ObjectId) -> u32 { + runner.state().objects[&id] + .counters + .get(&CounterType::Keyword(KeywordKind::FirstStrike)) + .copied() + .unwrap_or(0) + } + + /// CR 603.4 + CR 608.2a: the "if it doesn't have first strike" clause is an + /// intervening "if", rechecked as the ability resolves. With no first + /// strike the gate is true and the counter is placed. + #[test] + fn attacker_without_first_strike_gets_the_counter() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let rumbler = scenario + .add_creature_from_oracle(P0, "Momentum Rumbler", 4, 4, MOMENTUM_RUMBLER) + .id(); + let mut runner = scenario.build(); + + runner.advance_to_combat(); + runner + .declare_attackers(&[(rumbler, AttackTarget::Player(P1))]) + .expect("Momentum Rumbler must be able to attack"); + settle(&mut runner); + + assert_eq!( + first_strike_counters(&runner, rumbler), + 1, + "CR 122.1: the attacker must receive exactly one first strike counter" + ); + // Deliberately NOT asserted here: whether the card's second trigger also + // grants double strike. Both attack triggers go on the stack together and + // CR 603.3b lets their controller order them, so the affirmative twin + // sees first strike only if it resolves after this one. That ordering is + // untouched by this change; the affirmative twin's own gate is pinned in + // `attacker_with_first_strike_gets_no_counter`, where first strike is + // present before either trigger resolves. + } + + /// Negative branch — a printed first strike makes the gate false, so no + /// counter is placed. Paired with the positive case above, so neither + /// assertion is vacuous. + #[test] + fn attacker_with_first_strike_gets_no_counter() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let rumbler = { + let mut builder = + scenario.add_creature_from_oracle(P0, "Momentum Rumbler", 4, 4, MOMENTUM_RUMBLER); + builder.first_strike(); + builder.id() + }; + let mut runner = scenario.build(); + + runner.advance_to_combat(); + runner + .declare_attackers(&[(rumbler, AttackTarget::Player(P1))]) + .expect("Momentum Rumbler must be able to attack"); + settle(&mut runner); + + assert_eq!( + first_strike_counters(&runner, rumbler), + 0, + "an attacker that already has first strike must not receive the counter" + ); + assert!( + object_has_effective_keyword_kind(runner.state(), rumbler, KeywordKind::DoubleStrike), + "reach-guard: the affirmative twin still fires, proving the trigger ran" + ); + } + + /// Hostile fixture, seam-critical. `TriggeringSource` resolves only because + /// `matching_attack_events` narrows every non-batched attack trigger to a + /// SINGLETON `AttackersDeclared`, satisfying `extract_source_from_event`'s + /// one-attacker guard. Attacking with a second creature must not collapse + /// that narrowing — if it ever does, the condition fails closed and the + /// counter silently stops being placed. + #[test] + fn multi_attacker_declaration_still_binds_the_triggering_source() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let rumbler = scenario + .add_creature_from_oracle(P0, "Momentum Rumbler", 4, 4, MOMENTUM_RUMBLER) + .id(); + let ally = scenario.add_creature(P0, "Runeclaw Bear", 2, 2).id(); + let mut runner = scenario.build(); + + runner.advance_to_combat(); + runner + .declare_attackers(&[ + (rumbler, AttackTarget::Player(P1)), + (ally, AttackTarget::Player(P1)), + ]) + .expect("both creatures must be able to attack"); + settle(&mut runner); + + assert_eq!( + first_strike_counters(&runner, rumbler), + 1, + "the per-attacker event narrowing must survive a multi-attacker declaration" + ); + assert_eq!( + first_strike_counters(&runner, ally), + 0, + "the counter must land on the trigger's own source, not on a co-attacker" + ); + } +} + +/// Binding class 3 — the referent is a DECLARED stack target, propagated into +/// the sub-chain's `targets`. Evaluated at the sub-ability gate. The card +/// "Suspend" shares this shape with Delay, Doom's Time Platform and Soovril. +mod declared_stack_target { + use super::*; + + fn cast_suspend_at(printed_suspend: bool) -> (GameRunner, ObjectId, ObjectId) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let victim = { + let mut builder = scenario.add_creature(P1, "Grizzly Bear", 2, 2); + if printed_suspend { + builder.with_keyword(printed_suspend_four_blue()); + } + builder.id() + }; + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Suspend", true, SUSPEND_CARD) + .id(); + let mut runner = scenario.build(); + let outcome = runner.cast(spell).target_objects(&[victim]).resolve(); + drop(outcome); + (runner, spell, victim) + } + + /// Positive branch — a plain creature card exiled by "Suspend" gains + /// suspend (CR 702.62a) and carries two time counters (CR 122.1). + #[test] + fn exiled_creature_without_suspend_gains_it() { + let (runner, _spell, victim) = cast_suspend_at(false); + + assert_eq!( + runner.state().objects[&victim].zone, + Zone::Exile, + "the targeted creature must be exiled" + ); + assert_eq!( + runner.state().objects[&victim] + .counters + .get(&CounterType::Time) + .copied(), + Some(2), + "CR 122.1: the exiled card must carry two time counters" + ); + assert!( + object_has_effective_keyword_kind(runner.state(), victim, KeywordKind::Suspend), + "the exiled card must gain suspend" + ); + } + + /// Negative branch — the declared target already has printed + /// `Suspend 4—{U}`, so the gate is false and the printed parameters survive. + /// + /// Revert-fail: with `SourceLacksKeyword` the gate reads the Suspend SPELL, + /// which never has suspend, so the grant fires and clobbers the parameters. + #[test] + fn exiled_creature_with_printed_suspend_keeps_its_parameters() { + let (runner, _spell, victim) = cast_suspend_at(true); + + assert_eq!( + runner.state().objects[&victim].zone, + Zone::Exile, + "reach-guard: the targeted creature must actually be exiled" + ); + assert_eq!( + effective_suspend_cost(runner.state(), victim), + Some(blue_mana_cost()), + "CR 702.62a: the declared target's printed Suspend 4—{{U}} must survive, \ + not be clobbered to {{0}} by a redundant grant" + ); + } +} + +/// Binding class 4 — the referent is picked DURING RESOLUTION (Amy's Home, The +/// Eleventh Doctor). The keyword gate is UNBINDABLE for this shape, so the +/// parser strict-fails it to `Effect::Unimplemented` and coverage reports the +/// gap instead of reporting the card supported. +/// +/// Why unbindable: the pick reaches the grant's RECIPIENT correctly +/// (`TargetFilter::ParentTarget` resolves to the chosen card at effect-apply +/// time), but CR 608.2d makes it an untargeted choice made while the ability +/// resolves, so it is never written into `ResolvedAbility.targets` — which is +/// what the condition reads. `TargetMatchesFilter { subject_slot: None }` would +/// therefore find no object target and fall through to its `TriggeringSource` +/// fallback: for a combat-damage trigger that is The Eleventh Doctor itself, the +/// very object the old `SourceLacksKeyword` lowering read. Shipping that gate +/// would re-grant suspend onto a card that already has it and clobber its +/// printed `Suspend 4—{U}` down to `{0}`, while `cargo coverage` reported the +/// card fully supported — the coverage-honesty contract the plural form +/// (`try_parse_exiled_this_way_keyword_grant`) already respects. +/// +/// Two separate upstream defects remain, each its own unit of work: +/// 1. **The resolution-time pick is not published into the sub-chain's +/// `targets`.** Repairing that is what lifts the strict failure: the +/// predicate to relax is +/// `keyword_anaphor_referent_is_unpublished_resolution_pick` +/// (`parser/oracle_effect/mod.rs`). +/// 2. **`change_zone.rs` resolves `enter_with_counters` EAGERLY.** The parser +/// is NOT at fault here: "with a number of time counters on it equal to its +/// mana value" lowers correctly to +/// `enter_with_counters: [(Time, Ref(ObjectManaValue { scope: Recipient }))]` +/// — pinned by `parse_exile_from_hand_with_dynamic_counter_suffix` in +/// `parser/oracle_effect/imperative.rs`. +/// `game/effects/change_zone.rs` (`resolve_quantity_with_targets` at +/// resolver entry) resolves it BEFORE the interactive `EffectZoneChoice` +/// pick binds the recipient, and `resolve_quantity_with_targets` passes +/// `recipient: None`. `ObjectScope::Recipient`'s fallback ladder +/// (`game/quantity.rs`, `object_for_scope`) then walks recipient → first +/// object target (empty for a resolution pick) → `ctx.entering` (unset +/// outside ETB replacement) → the ability SOURCE, so the count becomes the +/// SOURCE's mana value, never the chosen card's. In the fixture below the +/// source is a scenario-built creature with no mana cost, so that is 0. +/// +/// This module pins both so a future fix has a baseline and so the class cannot +/// silently look repaired. +mod resolution_time_choice_disclosed_gap { + use super::*; + use crate::rules::run_combat; + use engine::parser::parse_oracle_text; + use engine::types::ability::Effect; + + const ELEVENTH_DOCTOR: &str = "Whenever The Eleventh Doctor deals combat damage to a \ + player, you may exile a card from your hand with a number of time counters on it \ + equal to its mana value. If it doesn't have suspend, it gains suspend."; + + fn drive(printed_suspend: bool) -> (GameRunner, ObjectId) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let doctor = scenario + .add_creature_from_oracle(P0, "The Eleventh Doctor", 3, 3, ELEVENTH_DOCTOR) + .id(); + let hand_card = { + let mut builder = scenario.add_spell_to_hand(P0, "Chosen Sorcery", false); + builder.with_mana_cost(ManaCost::generic(3)); + if printed_suspend { + builder.with_keyword(printed_suspend_four_blue()); + } + builder.id() + }; + // A second, plain hand card so the resolution-time pick is a REAL choice + // and not a degenerate single-candidate auto-resolve. + { + let mut decoy = scenario.add_spell_to_hand(P0, "Decoy Sorcery", false); + decoy.with_mana_cost(ManaCost::generic(1)); + } + let mut runner = scenario.build(); + run_combat(&mut runner, vec![doctor], vec![]); + + // Answer the optional trigger and its resolution-time card pick. Same + // terminal-state contract as `settle`: only an empty stack at a priority + // window is a legal exit, because both pins below (a zero time-counter + // count, an absent suspend grant) are exactly what a card that never + // left the hand would also show. + let mut saw_optional = false; + let mut saw_pick = false; + let mut settled = false; + for _ in 0..40 { + match runner.state().waiting_for.clone() { + WaitingFor::OrderTriggers { .. } => { + engine::game::triggers::drain_order_triggers_with_identity(runner.state_mut()); + } + // Accept the trigger's "you may" so the exile actually happens. + WaitingFor::OptionalEffectChoice { .. } => { + saw_optional = true; + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("the optional combat-damage trigger must accept"); + } + // The resolution-time pick of which hand card to exile. + WaitingFor::EffectZoneChoice { .. } => { + saw_pick = true; + runner + .act(GameAction::SelectCards { + cards: vec![hand_card], + }) + .expect("the chosen hand card must be a legal pick"); + } + WaitingFor::TriggerTargetSelection { .. } | WaitingFor::TargetSelection { .. } => { + runner + .choose_first_legal_target() + .expect("a pending target selection must offer a legal target"); + } + WaitingFor::Priority { .. } => { + if runner.state().stack.is_empty() { + settled = true; + break; + } + runner + .act(GameAction::PassPriority) + .expect("passing priority on a non-empty stack must be legal"); + } + other => panic!("unexpected prompt while driving the trigger: {other:?}"), + } + } + assert!( + settled, + "the stack never emptied within 40 steps (waiting_for = {:?}, stack depth = {})", + runner.state().waiting_for, + runner.state().stack.len(), + ); + assert!( + saw_optional, + "reach-guard: the optional combat-damage trigger must be offered" + ); + assert!( + saw_pick, + "reach-guard: the resolution-time hand pick must be offered" + ); + (runner, hand_card) + } + + /// DISCLOSED GAP #1, pinned at the PARSE seam: the unbindable gate must be + /// visible to coverage as `Effect::Unimplemented`, not hidden behind a green + /// card whose only warning lives in a test comment. + /// + /// Revert-fail: without + /// `keyword_anaphor_referent_is_unpublished_resolution_pick`, the gated + /// clause parses as a supported `GenericEffect` suspend grant and + /// `is_ability_supported` reports the card fully supported. + #[test] + fn the_unbindable_gate_is_disclosed_as_a_coverage_gap() { + let parsed = parse_oracle_text( + ELEVENTH_DOCTOR, + "The Eleventh Doctor", + &[], + &["Legendary".to_string(), "Creature".to_string()], + &["Time Lord".to_string(), "Doctor".to_string()], + ); + let execute = parsed.triggers[0] + .execute + .as_deref() + .expect("the combat-damage trigger has an effect chain"); + // Reach-guard: the exile half still parses, so the gap is scoped to the + // gate and did not swallow the whole trigger. + assert!( + matches!(&*execute.effect, Effect::ChangeZone { .. }), + "the hand exile must still parse, got {:?}", + execute.effect + ); + let gated = execute + .sub_ability + .as_deref() + .expect("the suspend grant is the gated sub-ability"); + assert_eq!( + gated.effect.unimplemented_description(), + Some("If it doesn't have suspend, it gains suspend"), + "the unbindable gate must surface as a coverage gap, got {:?}", + gated.effect + ); + } + + /// The half that DOES work, and the reach-guard for the two runtime pins + /// below: the optional trigger, the resolution-time pick and the exile all + /// happen. Only the gated grant is deferred. + #[test] + fn chosen_card_is_exiled_and_the_deferred_grant_does_not_fire() { + let (runner, card) = drive(false); + + assert_eq!( + runner.state().objects[&card].zone, + Zone::Exile, + "the chosen hand card must be exiled" + ); + assert!( + !object_has_effective_keyword_kind(runner.state(), card, KeywordKind::Suspend), + "KNOWN GAP #1: the grant is deferred to `Unimplemented` until the \ + resolution-time pick is published into the sub-chain's `targets`" + ); + } + + /// DISCLOSED GAP #2, pinned: the chosen MV-3 card enters exile with ZERO time + /// counters instead of three (CR 122.1). + /// + /// The defect is in the RUNTIME, not the parser: the AST carries + /// `ObjectManaValue { scope: Recipient }`, but `change_zone.rs` resolves + /// `enter_with_counters` at resolver entry — before the `EffectZoneChoice` + /// pick binds the recipient — with `recipient: None`, so `object_for_scope` + /// walks its fallback ladder down to the ability SOURCE and reports the + /// SOURCE's mana value. This scenario's Doctor is built with no mana cost, so + /// that is 0; on a real board it would be the Doctor's mana value (3), which + /// is just as wrong. Flip this to 3 when `change_zone.rs` resolves the count + /// AFTER the pick binds the recipient. + #[test] + fn mana_value_time_counters_read_the_source_not_the_chosen_card() { + let (runner, card) = drive(false); + + // Reach-guard: a card still in hand also has zero time counters, so the + // pin below only means anything once the exile has actually happened. + assert_eq!( + runner.state().objects[&card].zone, + Zone::Exile, + "reach-guard: the chosen hand card must actually be exiled" + ); + assert_eq!( + runner.state().objects[&card] + .counters + .get(&CounterType::Time) + .copied() + .unwrap_or(0), + 0, + "KNOWN GAP #2: the eager `enter_with_counters` resolution reads the \ + SOURCE's mana value (0 here), not the chosen card's 3" + ); + } + + /// The observable payoff of disclosing gap #1 instead of shipping the + /// misbinding gate: a card that already has printed `Suspend 4—{U}` keeps its + /// parameters. The old lowering (and the unbindable `TriggeringSource` + /// fallback) re-granted suspend here and `upsert_keyword_contribution` + /// clobbered the printed contribution down to `Suspend 0—{}`. + /// + /// Revert-fail: with the strict failure removed, the gate reads the trigger + /// source (which never has suspend), the grant fires, and this reads + /// `Some({0})`. + #[test] + fn printed_suspend_parameters_survive_the_deferred_grant() { + let (runner, card) = drive(true); + + assert_eq!( + runner.state().objects[&card].zone, + Zone::Exile, + "reach-guard: the chosen hand card must actually be exiled" + ); + assert_eq!( + effective_suspend_cost(runner.state(), card), + Some(blue_mana_cost()), + "CR 702.62a: the printed Suspend 4—{{U}} must survive — no redundant \ + grant may clobber it to {{0}}" + ); + } +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 76e6d57cf7..c926ead335 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -1203,6 +1203,7 @@ mod karplusan_yeti_fight_back; mod kav_landseeker_delayed_sacrifice; mod kellan_daring_traveler_mana_value_gate; mod ketramose_exile_trigger_gate_4952; +mod keyword_anaphor_subject_binding; mod kid_loki_counter_hexproof_static; mod klaw_play_from_exile_trigger_msh_wave1; mod koh_face_stealer_grants;