diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index cb56d0c19c..59246c7382 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -10848,6 +10848,29 @@ fn resolve_chain_body( // hit and exclude it — treating it as independent here stranded // that exclusion, sweeping the hit itself to the library bottom // alongside the misses. + // CR 115.1 + Digital-only Alchemy (root cause #19): a zone-pinned + // mass `ApplyPerpetual` ("creature cards in your hand perpetually + // get +1/+1") is ALSO independent — the same species as the + // `ExiledBySource` arm above. Its targeting authority + // (`extract_target_filter_from_effect`) deliberately suppresses the + // slot precisely so `ability.targets` stays EMPTY, which is what + // lets `perpetual_target_object_ids` reach its mass zone branch. + // Inheriting the parent's object target refills that vector, and the + // resolver then short-circuits and pumps the parent's battlefield + // target instead of the zone population. + // + // Carries the same `!effect_refs_parent_target` guard as the two + // disjuncts above, for the same reason: a BARE `ParentTarget` filter + // does have `extract_in_zone() == None`, but a COMPOSED one does not. + // `extract_in_zone` recurses into `And`/`Or`, so `And { ParentTarget, + // Typed { InZone { Graveyard } } }` reports `Some(Graveyard)` and the + // population predicate fires; and `filter_refs_parent_target` also + // fires for a plain `Typed` carrying `FilterProp::DistinctFrom { + // reference: ParentTarget }` (the Jodah cleanup-rider shape). In both + // of those the sub still needs the propagated parent object target so + // its `ParentTarget` anaphor can resolve — denying it would strand the + // reference. No card in the corpus has that shape today, but the guard + // is correctness, not decoration. let has_independent_target_slot = (crate::game::triggers::extract_target_filter_from_effect(&sub.effect).is_some() && !effect_refs_parent_target(&sub.effect) @@ -10856,6 +10879,9 @@ fn resolve_chain_body( .effect .target_filter() .is_some_and(TargetFilter::references_exiled_by_source) + && !effect_refs_parent_target(&sub.effect)) + || (matches!(&sub.effect, Effect::ApplyPerpetual { target, .. } + if crate::game::triggers::apply_perpetual_targets_zone_population(target)) && !effect_refs_parent_target(&sub.effect)); sub_with_targets.targets = ability .targets diff --git a/crates/engine/src/game/effects/perpetual.rs b/crates/engine/src/game/effects/perpetual.rs index 7a103a4608..4df4994484 100644 --- a/crates/engine/src/game/effects/perpetual.rs +++ b/crates/engine/src/game/effects/perpetual.rs @@ -104,12 +104,31 @@ fn perpetual_target_object_ids( // keep every object matching the filter — NOT a single declared target and // NOT the source fallback (the grant edits a set of cards, possibly empty, in // a hidden zone). Mirrors `layers::apply_continuous_effect_filtered`. + // + // CR 109.4 + CR 109.5 + CR 400.3: in the owner-scoped zones an object has no + // controller (CR 109.4) and always sits in its OWNER's copy of the zone + // (CR 400.3); CR 109.5 is what then makes "you"/"your" on a controllerless + // object refer to its OWNER. So the filter's "your" + // predicate must be answered against ownership — otherwise a card you own + // that an opponent last controlled is excluded from your own graveyard by + // stale control LKI, while a card an opponent owns that you last controlled + // is wrongly swept in. `matches_target_filter_in_owner_zone` is the existing + // authority for exactly that (game/filter.rs); the dispatch mirrors + // `off_zone_characteristics::matches_off_zone_keyword_recipient`. if let Some(zone) = target.extract_in_zone() { if zone != crate::types::zones::Zone::Battlefield { let ctx = crate::game::filter::FilterContext::from_source(state, ability.source_id); return crate::game::targeting::zone_object_ids(state, zone) .into_iter() - .filter(|&id| crate::game::filter::matches_target_filter(state, id, target, &ctx)) + .filter(|&id| { + if crate::game::filter::is_owner_scoped_zone(zone) { + crate::game::filter::matches_target_filter_in_owner_zone( + state, id, target, &ctx, + ) + } else { + crate::game::filter::matches_target_filter(state, id, target, &ctx) + } + }) .collect(); } } @@ -527,6 +546,370 @@ mod tests { assert_eq!(state.objects.get(&source).unwrap().base_power, None); } + /// The mass non-battlefield-zone branch of `perpetual_target_object_ids` + /// keyed on a zone OTHER than Hand. `zone_object_ids` (game/targeting.rs) is + /// exhaustive over every `Zone` with the identical `players.iter().flat_map` + /// shape, so Graveyard must behave exactly like the already-covered Hand + /// case (`casting_tests.rs perpetual_mass_hand_cost_reduces_only_matching_cards`). + /// + /// CR 109.4 + CR 109.5 + CR 108.3: a card in a hidden zone has no controller + /// (CR 109.4), and CR 109.5 makes "you"/"your" on a controllerless object + /// refer to its OWNER — an ownership that CR 108.3 fixes for the whole game. + /// `effective_controller` (game/filter.rs) falls back to `obj.controller`, + /// which `GameObject::new` seeds from the owner — so `ControllerRef::You` + /// scopes to the OWNER's copy of the zone. Hostile siblings on the type axis + /// (a noncreature card in the same graveyard) and the controller axis (an + /// opponent's creature card in THEIR graveyard) must both be untouched. + #[test] + fn perpetual_mass_graveyard_pt_modifies_only_matching_cards() { + use crate::game::scenario::{GameScenario, P0, P1}; + use crate::types::ability::{ControllerRef, FilterProp, TypeFilter, TypedFilter}; + + let mut scenario = GameScenario::new(); + // Battlefield source so `FilterContext::from_source` can anchor `You`. + let source = scenario.add_creature(P0, "Feed the Bog Source", 1, 1).id(); + let mine = scenario + .add_creature_to_graveyard(P0, "Grizzly Bears", 2, 2) + .id(); + // Second positive with DISTINCT base P/T. With only one match the test + // could not tell "the whole zone population was enumerated" from "the + // first match was taken"; distinct P/T additionally proves each card is + // modified from its OWN base rather than a shared/latched value. + let mine_second = scenario + .add_creature_to_graveyard(P0, "Runeclaw Bear", 3, 1) + .id(); + let mine_noncreature = scenario.add_spell_to_graveyard(P0, "Shock", true).id(); + let theirs = scenario + .add_creature_to_graveyard(P1, "Opposing Bears", 2, 2) + .id(); + let mut runner = scenario.build(); + let state = runner.state_mut(); + + // Revert baseline, asserted BEFORE resolution. + for id in [mine, theirs] { + assert_eq!(state.objects.get(&id).unwrap().base_power, Some(2)); + assert_eq!(state.objects.get(&id).unwrap().base_toughness, Some(2)); + } + assert_eq!( + ( + state.objects.get(&mine_second).unwrap().base_power, + state.objects.get(&mine_second).unwrap().base_toughness + ), + (Some(3), Some(1)) + ); + + let ability = ResolvedAbility::new( + Effect::ApplyPerpetual { + target: TargetFilter::Typed( + TypedFilter::default() + .with_type(TypeFilter::Creature) + .controller(ControllerRef::You) + .properties(vec![FilterProp::InZone { + zone: Zone::Graveyard, + }]), + ), + modification: PerpetualModification::ModifyPowerToughness { + power_delta: 1, + toughness_delta: 1, + }, + }, + vec![], + source, + PlayerId(0), + ); + let mut events = Vec::new(); + super::resolve(state, &ability, &mut events).unwrap(); + + for (id, expected, why) in [ + ( + mine, + (Some(3), Some(3)), + "first matching creature card in the controller's graveyard", + ), + ( + mine_second, + (Some(4), Some(2)), + "SECOND matching card — proves the whole zone population is \ + enumerated, not just the first match, and that each card is \ + modified from its own distinct base P/T", + ), + ] { + let hit = state.objects.get(&id).unwrap(); + assert_eq!((hit.base_power, hit.base_toughness), expected, "{why}"); + assert!(!hit.perpetual_mods.is_empty(), "{why}"); + } + + for (id, why) in [ + ( + mine_noncreature, + "type axis: a noncreature card in the same graveyard", + ), + (theirs, "controller axis: an opponent's creature card"), + ( + source, + "source fallback must NOT fire on the mass zone path", + ), + ] { + assert!( + state.objects.get(&id).unwrap().perpetual_mods.is_empty(), + "{why} must be untouched" + ); + } + assert_eq!( + state.objects.get(&theirs).unwrap().base_power, + Some(2), + "controller axis" + ); + } + + /// Library sibling of `perpetual_mass_graveyard_pt_modifies_only_matching_cards` + /// (Bramblearmor Brawler's zone). No scenario helper places a creature CARD + /// with base P/T into a library, so this one keeps the module's raw + /// `create_object` form — `create_object` routes through `add_to_zone`, so + /// the object really joins `player.library`, which is what + /// `zone_object_ids(state, Zone::Library)` enumerates. + #[test] + fn perpetual_mass_library_pt_modifies_only_matching_cards() { + use crate::types::ability::{ControllerRef, FilterProp, TypeFilter, TypedFilter}; + use crate::types::card_type::CoreType; + + let mut state = GameState::new_two_player(7); + + fn seed_creature_card( + state: &mut GameState, + card: u64, + owner: PlayerId, + name: &str, + power: i32, + toughness: i32, + ) -> ObjectId { + let id = create_object(state, CardId(card), owner, name.to_string(), Zone::Library); + let obj = state.objects.get_mut(&id).unwrap(); + obj.card_types.core_types.push(CoreType::Creature); + obj.base_card_types = obj.card_types.clone(); + obj.base_power = Some(power); + obj.base_toughness = Some(toughness); + id + } + + let source = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Bramblearmor Brawler".to_string(), + Zone::Battlefield, + ); + let mine = seed_creature_card(&mut state, 2, PlayerId(0), "Grizzly Bears", 2, 2); + // Second positive with DISTINCT base P/T — same discrimination as the + // Graveyard sibling: one match cannot separate "enumerated the whole + // zone" from "took the first match". + let mine_second = seed_creature_card(&mut state, 5, PlayerId(0), "Runeclaw Bear", 3, 1); + let theirs = seed_creature_card(&mut state, 3, PlayerId(1), "Opposing Bears", 2, 2); + // Type axis: a card with NO creature core type in the same library. + let mine_noncreature = create_object( + &mut state, + CardId(4), + PlayerId(0), + "Shock".to_string(), + Zone::Library, + ); + + assert_eq!(state.objects.get(&mine).unwrap().base_power, Some(2)); + assert_eq!(state.objects.get(&theirs).unwrap().base_power, Some(2)); + assert_eq!( + ( + state.objects.get(&mine_second).unwrap().base_power, + state.objects.get(&mine_second).unwrap().base_toughness + ), + (Some(3), Some(1)) + ); + + let ability = ResolvedAbility::new( + Effect::ApplyPerpetual { + target: TargetFilter::Typed( + TypedFilter::default() + .with_type(TypeFilter::Creature) + .controller(ControllerRef::You) + .properties(vec![FilterProp::InZone { + zone: Zone::Library, + }]), + ), + modification: PerpetualModification::ModifyPowerToughness { + power_delta: 1, + toughness_delta: 1, + }, + }, + vec![], + source, + PlayerId(0), + ); + let mut events = Vec::new(); + super::resolve(&mut state, &ability, &mut events).unwrap(); + + for (id, expected, why) in [ + ( + mine, + (Some(3), Some(3)), + "the controller's creature card in their library must be modified", + ), + ( + mine_second, + (Some(4), Some(2)), + "SECOND matching library card — proves the whole zone population \ + is enumerated, each from its own distinct base P/T", + ), + ] { + let hit = state.objects.get(&id).unwrap(); + assert_eq!((hit.base_power, hit.base_toughness), expected, "{why}"); + assert!(!hit.perpetual_mods.is_empty(), "{why}"); + } + for (id, why) in [ + (mine_noncreature, "type axis"), + (theirs, "controller axis"), + ( + source, + "source fallback must NOT fire on the mass zone path", + ), + ] { + assert!( + state.objects.get(&id).unwrap().perpetual_mods.is_empty(), + "{why} must be untouched" + ); + } + assert_eq!(state.objects.get(&theirs).unwrap().base_power, Some(2)); + } + + /// CR 115.6 + CR 608.2c — chain-inheritance guard for the zone-population + /// `ApplyPerpetual`. + /// + /// Suppressing the stack-time slot for a zone-pinned `ApplyPerpetual` is + /// exactly what leaves `ability.targets` empty, which is the PRECONDITION + /// for `perpetual_target_object_ids` reaching its mass zone branch — with a + /// non-empty `targets` the function short-circuits and returns the single + /// propagated object instead. + /// + /// That makes the sub-ability inheritance path load-bearing: when such an + /// `ApplyPerpetual` is the SUB-ABILITY of a TARGETED parent, + /// `resolve_ability_chain` must NOT hand it the parent's object target, or + /// the grant lands on the parent's battlefield creature instead of the hand + /// population. `has_independent_target_slot` (game/effects/mod.rs) carries a + /// disjunct for exactly this, alongside the `ExiledBySource` one added for + /// the Fight Rigging / Collector's Cage class (issue #6437) — the same + /// species of bug. + /// + /// The parent is `Effect::SetTapState { scope: Single }` deliberately: its + /// target must SURVIVE resolution so its `perpetual_mods` can be inspected, + /// and its own effect stays observable as a reach-guard. + #[test] + fn perpetual_zone_population_sub_ability_ignores_parent_object_target() { + use crate::game::ability_utils::build_resolved_from_def_with_targets; + use crate::types::ability::{ + AbilityDefinition, AbilityKind, ControllerRef, EffectScope, FilterProp, TapStateChange, + TypeFilter, TypedFilter, + }; + use crate::types::card_type::CoreType; + + let mut state = GameState::new_two_player(7); + let source = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Chain Source".to_string(), + Zone::Battlefield, + ); + // The parent's declared object target: a battlefield creature that must + // stay a 2/2 with NO perpetual modification. + let board = create_object( + &mut state, + CardId(2), + PlayerId(0), + "Board Bear".to_string(), + Zone::Battlefield, + ); + // The population the sub-ability is written for. + let hand_card = create_object( + &mut state, + CardId(3), + PlayerId(0), + "Hand Bear".to_string(), + Zone::Hand, + ); + for id in [board, hand_card] { + let obj = state.objects.get_mut(&id).unwrap(); + obj.card_types.core_types.push(CoreType::Creature); + obj.base_card_types = obj.card_types.clone(); + obj.base_power = Some(2); + obj.base_toughness = Some(2); + } + + let def = AbilityDefinition::new( + AbilityKind::Spell, + Effect::SetTapState { + target: TargetFilter::Typed(TypedFilter::creature()), + scope: EffectScope::Single, + state: TapStateChange::Tap, + }, + ) + .sub_ability(AbilityDefinition::new( + AbilityKind::Spell, + Effect::ApplyPerpetual { + target: TargetFilter::Typed( + TypedFilter::default() + .with_type(TypeFilter::Creature) + .controller(ControllerRef::You) + .properties(vec![FilterProp::InZone { zone: Zone::Hand }]), + ), + modification: PerpetualModification::ModifyPowerToughness { + power_delta: 1, + toughness_delta: 1, + }, + }, + )); + + let ability = build_resolved_from_def_with_targets( + &def, + source, + PlayerId(0), + vec![TargetRef::Object(board)], + ); + let mut events = Vec::new(); + resolve_ability_chain(&mut state, &ability, &mut events, 0).unwrap(); + + // Reach-guard: the TARGETED parent really applied to its declared target, + // so the chain genuinely carried an object target into the sub. + assert!( + state.objects.get(&board).unwrap().tapped, + "reach-guard: the parent's declared object target must actually be tapped" + ); + + let hand = state.objects.get(&hand_card).unwrap(); + assert_eq!( + (hand.base_power, hand.base_toughness), + (Some(3), Some(3)), + "the hand population must receive the perpetual grant" + ); + + let target = state.objects.get(&board).unwrap(); + assert_eq!( + (target.base_power, target.base_toughness), + (Some(2), Some(2)), + "the parent's battlefield target must NOT inherit the perpetual grant" + ); + assert!( + target.perpetual_mods.is_empty(), + "the parent's battlefield target must record no perpetual modification" + ); + assert!( + state + .objects + .get(&source) + .unwrap() + .perpetual_mods + .is_empty(), + "the source fallback must not fire on the mass zone path" + ); + } + #[test] fn empty_choose_from_zone_parent_target_perpetual_noops() { let mut state = GameState::new_two_player(7); diff --git a/crates/engine/src/game/filter.rs b/crates/engine/src/game/filter.rs index 49a00a4768..10545d9e29 100644 --- a/crates/engine/src/game/filter.rs +++ b/crates/engine/src/game/filter.rs @@ -2051,6 +2051,22 @@ pub fn matches_type_filter_against_face(face: &CardFace, filter: &TypeFilter) -> } } +/// CR 109.4 + CR 109.5 + CR 400.3: hand, library and graveyard are the zones +/// whose contents have no controller at all (CR 109.4) and to which an object +/// always goes to its OWNER's copy (CR 400.3); CR 109.5 is what then makes +/// "you"/"your" on such an object refer to its OWNER. A "your card" predicate +/// over one of them is therefore an OWNERSHIP scope, and must be evaluated with +/// [`matches_target_filter_in_owner_zone`] rather than plain +/// [`matches_target_filter`]. +/// +/// Single authority for that zone set: `off_zone_characteristics`'s keyword +/// recipient dispatch, `triggers::apply_perpetual_targets_zone_population`, and +/// `effects::perpetual`'s mass zone branch all key on this predicate so they +/// cannot drift apart. +pub(crate) fn is_owner_scoped_zone(zone: Zone) -> bool { + matches!(zone, Zone::Hand | Zone::Library | Zone::Graveyard) +} + /// CR 109.5 + CR 400.3: In owner-scoped zones (hand, library, graveyard), /// Oracle text still says "your card" even though cards are owned rather than /// controlled there. Evaluate the same typed filter with ownership standing in diff --git a/crates/engine/src/game/off_zone_characteristics.rs b/crates/engine/src/game/off_zone_characteristics.rs index 77d47ec710..df3ba67038 100644 --- a/crates/engine/src/game/off_zone_characteristics.rs +++ b/crates/engine/src/game/off_zone_characteristics.rs @@ -1,5 +1,5 @@ use crate::game::filter::{ - matches_target_filter, matches_target_filter_in_owner_zone, FilterContext, + is_owner_scoped_zone, matches_target_filter, matches_target_filter_in_owner_zone, FilterContext, }; use crate::game::layers::{ active_continuous_effects_from_base_static_source, active_effect_condition_controller, @@ -212,12 +212,6 @@ fn matches_off_zone_keyword_recipient( } } -fn is_owner_scoped_zone(zone: Zone) -> bool { - // CR 109.5 + CR 400.3: "your" cards in hand/library/graveyard are scoped - // by owner, not stale object controller/LKI. - matches!(zone, Zone::Hand | Zone::Library | Zone::Graveyard) -} - fn supports_off_zone_keyword_query(modification: &ContinuousModification) -> bool { matches!( modification, diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index e5a2a52461..af9f5336cf 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -11308,6 +11308,36 @@ fn phase_out_or_in_filter_is_mass(filter: &TargetFilter) -> bool { } } +/// Digital-only Alchemy (no CR entry for "perpetually") + CR 115.1 + CR 400.1: +/// True when an [`Effect::ApplyPerpetual`] target denotes the POPULATION of a +/// private per-player CARD zone rather than a declared target. +/// +/// The zone set is ENUMERATED rather than expressed as `!= Battlefield` on +/// purpose: `TargetFilter::extract_in_zone` also reports `Zone::Stack` for +/// `StackSpell`/`StackAbility` and `Zone::Exile` for `ExiledBySource`, and +/// `StackSpell`/`StackAbility` are NOT context refs — they surface a genuine +/// declared target today ("Choose target spell. If it's a creature spell, it +/// perpetually gets -2/-0"). Mirrors `phase_out_or_in_filter_is_mass` above: one +/// typed predicate distinguishing a mass population filter from a declared +/// target. The parser arm that emits these effects +/// (`oracle_effect::try_parse_zone_scoped_cards_perpetual_modify_pt`) gates on +/// the same three zones, so every effect it produces is classified here as a +/// population. +/// +/// The zone set itself is not restated here: it is +/// [`crate::game::filter::is_owner_scoped_zone`], the single authority shared +/// with `off_zone_characteristics`'s keyword-recipient dispatch and with +/// `effects::perpetual`'s mass zone branch — the same three +/// CR 109.4 + CR 109.5 + CR 400.3 owner-scoped zones (no controller per +/// CR 109.4, always the OWNER's copy per CR 400.3, and CR 109.5 is what makes +/// "your" on such a controllerless object read as its OWNER), which is exactly +/// the set whose "your card" population the resolver enumerates. +pub(crate) fn apply_perpetual_targets_zone_population(filter: &TargetFilter) -> bool { + filter + .extract_in_zone() + .is_some_and(crate::game::filter::is_owner_scoped_zone) +} + pub(crate) fn extract_target_filter_from_effect(effect: &Effect) -> Option<&TargetFilter> { // CR 701.21a: Sacrifice does not target — the controller chooses permanents // at resolution time via EffectZoneChoice. Returning a filter here would @@ -11406,6 +11436,32 @@ pub(crate) fn extract_target_filter_from_effect(effect: &Effect) -> Option<&Targ } } } + // CR 115.1 + CR 400.1 — Digital-only Alchemy (no CR entry for "perpetually"): + // an `ApplyPerpetual` whose target pins a private card-set zone is a mass + // POPULATION filter, not a declared target — `perpetual_target_object_ids` + // (game/effects/perpetual.rs) enumerates the whole zone. Suppressing the slot + // here is what leaves `ability.targets` EMPTY, which is the precondition for + // that branch: with a slot filled, `perpetual_target_object_ids` short-circuits + // on the propagated target and the grant lands on the ONE chosen card instead + // of the population ("creature cards in your hand/graveyard/library perpetually + // get +1/+1" — Begin Anew, Feed the Bog, Bramblearmor Brawler; "[type] cards in + // your hand perpetually cost {1} less" — Blooming Cactusfolk, Fearsome Whelp). + // It also keeps the empty-zone case away from `legal_targets.is_empty() && + // !optional_targeting`, which otherwise makes the spell uncastable and drops a + // trigger-hosted copy as `DroppedTargetUnresolved`. The is-pinned-zone test + // mirrors the `ChangeZone` / `PutAtLibraryPosition` / `CastFromZone` arms above. + // + // The zone set is ENUMERATED (see `apply_perpetual_targets_zone_population`) + // rather than written as `!= Battlefield` on purpose: `extract_in_zone` also + // reports Stack (`StackSpell`/`StackAbility`) and Exile (`ExiledBySource`), + // which ARE genuine declared targets. Keeping their slot leaves + // `ability.targets` non-empty, so the resolver honours the declared target — + // the targeting and resolution authorities still agree on the outcome. + if let Effect::ApplyPerpetual { target, .. } = effect { + if apply_perpetual_targets_zone_population(target) { + return None; + } + } // CR 115.1 / CR 115.1d: Only effects that use the word "target" require stack-time target // selection. `TargetFilter::Any` is a sentinel value meaning "broadcast to all // matching permanents at resolution time" — it is never a declared target on any @@ -22909,6 +22965,148 @@ pub mod tests { assert_eq!(state.stack.len(), 1, "Ygra trigger should be on the stack"); } + // --- Trigger-hosted zone-population ApplyPerpetual (root cause #19) --- + + /// Klement, Novice Acolyte — the verbatim ETB LINE (verified against + /// `client/public/card-data.json`). The printed card has a second line, + /// `Specialize {2}`, which is deliberately omitted: it is an unrelated + /// activated ability and this fixture exercises only the ETB trigger. + const KLEMENT_ETB: &str = + "When Klement, Novice Acolyte enters, creature cards in your hand perpetually get +1/+1."; + + /// Shared fixture: Klement on the battlefield with its parsed ETB trigger, + /// plus the ETB `ZoneChanged` event to feed `process_triggers`. Follows the + /// Ygra precedent above — the event is CONSTRUCTED here rather than assumed + /// to be emitted by the scenario helper, which places the creature directly + /// on the battlefield without running the ETB pipeline. + fn klement_etb_fixture() -> (crate::game::scenario::GameRunner, ObjectId, Vec) { + use crate::game::scenario::{GameScenario, P0}; + use crate::types::game_state::TriggerIndex; + + let mut scenario = GameScenario::new(); + let klement = scenario + .add_creature_from_oracle(P0, "Klement, Novice Acolyte", 2, 2, KLEMENT_ETB) + .id(); + let mut runner = scenario.build(); + + // Reach-guard on the FIXTURE itself: the parsed trigger really is the + // zone-pinned mass `ApplyPerpetual` this test is about. Without this the + // stack-depth assertions below could be satisfied by any other trigger. + let effect = runner + .state() + .objects + .get(&klement) + .expect("Klement is on the battlefield") + .trigger_definitions + .first() + .and_then(|t| t.definition.execute.as_ref()) + .map(|a| (*a.effect).clone()) + .expect("Klement's ETB trigger must parse"); + let is_population = match &effect { + Effect::ApplyPerpetual { target, .. } => { + apply_perpetual_targets_zone_population(target) + } + _ => false, + }; + assert!( + is_population, + "fixture guard: Klement's ETB must lower to a zone-pinned mass ApplyPerpetual, got {effect:?}" + ); + + TriggerIndex::rebuild_from_battlefield(runner.state_mut()); + let events = vec![zone_changed_event( + klement, + Zone::Hand, + Zone::Battlefield, + vec![CoreType::Creature], + vec![], + )]; + (runner, klement, events) + } + + /// CR 115.1 + CR 603.3: a trigger whose effect is a zone-pinned mass + /// `ApplyPerpetual` declares NO target, so it must reach the stack even when + /// the pinned zone holds no matching card. Without the carve-out, + /// `build_target_slots` returns `Err(no_legal_target_slots())` and the + /// trigger is discarded as + /// `TriggerDispatchDisposition::DroppedTargetUnresolved`. + #[test] + fn trigger_hosted_zone_perpetual_is_not_dropped_when_hand_is_empty() { + let (mut runner, _klement, events) = klement_etb_fixture(); + assert!( + runner.state().players.iter().all(|p| p.hand.is_empty()), + "fixture guard: no player may hold a card for the empty-hand case" + ); + + process_triggers(runner.state_mut(), &events); + assert_eq!( + runner.state().stack.len(), + 1, + "a zone-pinned mass ApplyPerpetual trigger must reach the stack with an empty hand" + ); + } + + /// Paired POSITIVE reach-guard for the test above: with matching cards in + /// hand the same trigger must also reach the stack, must NOT stall on a + /// hidden-hand target prompt, and must actually RESOLVE onto the whole + /// population. Two matching cards, deliberately: with one, a declared target + /// slot and the population coincide and the fixture proves nothing. + #[test] + fn trigger_hosted_zone_perpetual_resolves_over_the_whole_hand_population() { + use crate::game::scenario::{GameScenario, P0}; + use crate::types::game_state::TriggerIndex; + + let mut scenario = GameScenario::new(); + let klement = scenario + .add_creature_from_oracle(P0, "Klement, Novice Acolyte", 2, 2, KLEMENT_ETB) + .id(); + let bear = scenario.add_creature_to_hand(P0, "Bear", 2, 2).id(); + let ogre = scenario.add_creature_to_hand(P0, "Ogre", 4, 1).id(); + let mut runner = scenario.build(); + TriggerIndex::rebuild_from_battlefield(runner.state_mut()); + + let events = vec![zone_changed_event( + klement, + Zone::Hand, + Zone::Battlefield, + vec![CoreType::Creature], + vec![], + )]; + process_triggers(runner.state_mut(), &events); + + assert_eq!(runner.state().stack.len(), 1); + assert!( + !matches!( + runner.state().waiting_for, + WaitingFor::TriggerTargetSelection { .. } + ), + "the perpetual population must not surface a hidden-hand target prompt: {:?}", + runner.state().waiting_for + ); + + let mut resolution_events = Vec::new(); + crate::game::stack::resolve_top(runner.state_mut(), &mut resolution_events); + + for (id, expected) in [(bear, (Some(3), Some(3))), (ogre, (Some(5), Some(2)))] { + let obj = runner.state().objects.get(&id).expect("card still in hand"); + assert_eq!( + (obj.base_power, obj.base_toughness), + expected, + "every matching card in the hand population must be modified, not just one" + ); + } + assert!( + runner + .state() + .objects + .get(&klement) + .unwrap() + .perpetual_mods + .is_empty(), + "the trigger source must not receive the grant via the source fallback" + ); + } + // === extract_target_filter_from_effect private zone tests === #[test] @@ -22966,6 +23164,163 @@ pub mod tests { ); } + // --- ApplyPerpetual zone-population carve-out (root cause #19) --- + + /// Shared fixture for the `ApplyPerpetual` carve-out tests: a + /// `ModifyPowerToughness` grant over an arbitrary target filter. + fn perpetual_over(target: TargetFilter) -> Effect { + Effect::ApplyPerpetual { + target, + modification: crate::types::ability::PerpetualModification::ModifyPowerToughness { + power_delta: 1, + toughness_delta: 1, + }, + } + } + + /// A `Typed[Creature]` filter with the given controller scope + properties. + fn perpetual_typed(properties: Vec, controller: Option) -> Effect { + let mut typed = TypedFilter::default() + .with_type(TypeFilter::Creature) + .properties(properties); + if let Some(c) = controller { + typed = typed.controller(c); + } + perpetual_over(TargetFilter::Typed(typed)) + } + + /// H12 — CR 115.1 + CR 400.1: "Creature cards in your hand perpetually get + /// +1/+1" (Begin Anew, Klement, Thoughtweft's Call) names no "target". The + /// resolver mass-scans the hand (`perpetual_target_object_ids`' zone branch), + /// and it can only reach that branch while `ability.targets` is EMPTY — + /// suppressing the slot here is what keeps it empty. + #[test] + fn extract_target_skips_apply_perpetual_over_hand_population() { + let effect = perpetual_typed( + vec![FilterProp::InZone { zone: Zone::Hand }], + Some(ControllerRef::You), + ); + assert!( + extract_target_filter_from_effect(&effect).is_none(), + "a hand-pinned ApplyPerpetual population must not surface a target slot, got {:?}", + extract_target_filter_from_effect(&effect) + ); + } + + /// H12b — Bramblearmor Brawler's zone. Same branch; pinned separately so the + /// enumerated zone set cannot silently shrink to Hand-only. + #[test] + fn extract_target_skips_apply_perpetual_over_library_population() { + let effect = perpetual_typed( + vec![FilterProp::InZone { + zone: Zone::Library, + }], + Some(ControllerRef::You), + ); + assert!( + extract_target_filter_from_effect(&effect).is_none(), + "a library-pinned ApplyPerpetual population must not surface a target slot, got {:?}", + extract_target_filter_from_effect(&effect) + ); + } + + /// H13 — Feed the Bog / Elvish Elegy / Blighted Nightmare. This is the test + /// that fails if the carve-out copies the sibling arms' `Hand | Library` set + /// verbatim instead of the three private CARD zones. + #[test] + fn extract_target_skips_apply_perpetual_over_graveyard_population() { + let effect = perpetual_typed( + vec![FilterProp::InZone { + zone: Zone::Graveyard, + }], + Some(ControllerRef::You), + ); + assert!( + extract_target_filter_from_effect(&effect).is_none(), + "a graveyard-pinned ApplyPerpetual population must not surface a target slot, got {:?}", + extract_target_filter_from_effect(&effect) + ); + } + + /// H14 — CR 115.1 boundary. Davriel's Withering's exact shape ("Target + /// creature an opponent controls perpetually gets -1/-2"): `properties` is + /// empty, so `extract_in_zone()` is `None` and the carve-out never fires. + /// Reverting the discriminator to a blanket `Effect::ApplyPerpetual => + /// return None` flips this assertion. + #[test] + fn extract_target_keeps_apply_perpetual_single_object_target() { + let effect = perpetual_typed(vec![], Some(ControllerRef::Opponent)); + assert!( + extract_target_filter_from_effect(&effect).is_some(), + "a genuinely targeted ApplyPerpetual must keep its declared target slot" + ); + } + + /// H15 — mirrors `extract_target_keeps_change_zone_from_battlefield`. The + /// resolver does NOT take the mass branch for `Zone::Battlefield`, so the + /// targeting authority must not either. + #[test] + fn extract_target_keeps_apply_perpetual_pinned_to_battlefield() { + let effect = perpetual_typed( + vec![FilterProp::InZone { + zone: Zone::Battlefield, + }], + Some(ControllerRef::You), + ); + assert!( + extract_target_filter_from_effect(&effect).is_some(), + "a battlefield-pinned ApplyPerpetual must keep its target slot" + ); + } + + /// H16 — CR 109.2 + CR 115.1: `TargetFilter::StackSpell` reports + /// `Zone::Stack` from `extract_in_zone()` and is NOT an `is_context_ref`, so + /// it surfaces a genuine declared target today ("Choose target spell. If + /// it's a creature spell, it perpetually gets -2/-0"). The carve-out's zone + /// set is enumerated precisely so this keeps its slot. + #[test] + fn extract_target_keeps_apply_perpetual_over_stack_spell() { + let effect = perpetual_over(TargetFilter::StackSpell); + assert!( + extract_target_filter_from_effect(&effect).is_some(), + "a stack-spell ApplyPerpetual is a declared target and must keep its slot" + ); + } + + /// H17 — the Exile sibling of H16. `extract_in_zone()` reports + /// `Zone::Exile`, which is deliberately OUTSIDE the enumerated population + /// set, so an exile-pinned filter keeps whatever semantics it has today. + #[test] + fn extract_target_keeps_apply_perpetual_pinned_to_exile() { + let effect = perpetual_typed( + vec![FilterProp::InZone { zone: Zone::Exile }], + Some(ControllerRef::You), + ); + assert!( + extract_target_filter_from_effect(&effect).is_some(), + "an exile-pinned ApplyPerpetual must keep today's declared-target semantics" + ); + } + + /// H17b — CR 406.6 + CR 607.2a: a bare `ExiledBySource` perpetual ("the + /// exiled card perpetually gets +1/+1") also reports `Zone::Exile`, but it + /// already returns `None` through the final `is_context_ref` guard. Pinned so + /// the carve-out is provably NOT what suppresses it — the exile-link class is + /// untouched by this change. + #[test] + fn extract_target_skips_apply_perpetual_over_exiled_by_source() { + let effect = perpetual_over(TargetFilter::ExiledBySource); + assert!( + extract_target_filter_from_effect(&effect).is_none(), + "ExiledBySource stays a context ref, not a declared target" + ); + assert!( + !apply_perpetual_targets_zone_population(&TargetFilter::ExiledBySource), + "the population predicate must NOT claim ExiledBySource — its None comes \ + from the is_context_ref guard, which this change does not touch" + ); + } + /// CR 701.21a: Sacrifice does not target — the sacrifice effect handler /// uses EffectZoneChoice for controller-scoped selection at resolution time. #[test] diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index bd316881c1..7aa3e45f25 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -9198,6 +9198,17 @@ fn parse_effect_clause_inner(text: &str, ctx: &mut ParseContext) -> ParsedEffect return parsed_clause(effect); } + // Digital-only Alchemy: "[each|all] card(s) in your perpetually + // get(s) +N/+M" — the ZONE-SCOPED CARD-SET sibling of the single-object arm + // above (Begin Anew, Feed the Bog, Bramblearmor Brawler, …). Disjoint from + // it: that arm requires a "target "/"that "/"it "/self-ref prefix, none of + // which `parse_type_phrase` can consume, so each would leave a non-empty + // subject remainder here. Ordered AFTER it so a single-object subject can + // never reach the filter grammar. + if let Some(effect) = try_parse_zone_scoped_cards_perpetual_modify_pt(tp) { + return parsed_clause(effect); + } + // Digital-only Alchemy: "[~/that X] perpetually gains \"This spell costs {N} // less/more to cast\"" — persistent self-spell cost modifier (CR 601.2f). // Tried before the keyword grant: disjoint (this requires a quoted body, the @@ -9580,6 +9591,27 @@ fn try_parse_perpetual_base_pt(tp: TextPair) -> Option { }) } +/// Digital-only Alchemy: `perpetually get(s) ±N/±M` — the shared PREDICATE tail +/// of every perpetual power/toughness grant, independent of the subject. +/// +/// Number agreement ("gets" for a singular subject, "get" for a plural or +/// distributive one) is a single `alt()`; the delta itself delegates to the +/// shared [`nom_primitives::parse_pt_modifier`] primitive, which accepts fixed +/// digits only (so "+X/+X" is rejected here rather than at each call site). +/// +/// SINGLE AUTHORITY for this predicate. Composed by every subject form in +/// [`try_parse_perpetual_modify_pt`] (explicit target / anaphoric "that " / +/// anaphoric "it" / self-reference) and by +/// [`try_parse_zone_scoped_cards_perpetual_modify_pt`], so the grammar lives in +/// exactly one place and the five arms cannot drift apart. +fn parse_perpetual_pt_predicate(input: &str) -> OracleResult<'_, (i32, i32)> { + preceded( + pair(tag("perpetually "), alt((tag("gets "), tag("get ")))), + nom_primitives::parse_pt_modifier, + ) + .parse(input) +} + /// Digital-only Alchemy: parse the "perpetually gets +N/+M" modifier form — /// "[~ / this creature / …] perpetually gets +N/+M" or /// "that [type] perpetually gets +N/+M" → [`Effect::ApplyPerpetual`] with @@ -9606,17 +9638,7 @@ fn try_parse_perpetual_modify_pt(tp: TextPair) -> Option { let (target, after_target) = parse_target(lower); if !matches!(target, TargetFilter::Any) { let (rest, _) = space1::<_, OracleError<'_>>(after_target).ok()?; - let (rest, _) = tag::<_, _, OracleError<'_>>("perpetually ") - .parse(rest) - .ok()?; - let (rest, _) = alt(( - tag::<_, _, OracleError<'_>>("gets "), - tag::<_, _, OracleError<'_>>("get "), - )) - .parse(rest) - .ok()?; - let (rest, (power_delta, toughness_delta)) = - nom_primitives::parse_pt_modifier(rest).ok()?; + let (rest, (power_delta, toughness_delta)) = parse_perpetual_pt_predicate(rest).ok()?; return tail_done(rest).then_some(Effect::ApplyPerpetual { target, modification: crate::types::ability::PerpetualModification::ModifyPowerToughness { @@ -9632,17 +9654,7 @@ fn try_parse_perpetual_modify_pt(tp: TextPair) -> Option { let (rest, _) = take_until::<_, _, OracleError<'_>>("perpetually ") .parse(after_that) .ok()?; - let (rest, _) = tag::<_, _, OracleError<'_>>("perpetually ") - .parse(rest) - .ok()?; - let (rest, _) = alt(( - tag::<_, _, OracleError<'_>>("gets "), - tag::<_, _, OracleError<'_>>("get "), - )) - .parse(rest) - .ok()?; - let (rest, (power_delta, toughness_delta)) = - nom_primitives::parse_pt_modifier(rest).ok()?; + let (rest, (power_delta, toughness_delta)) = parse_perpetual_pt_predicate(rest).ok()?; return tail_done(rest).then_some(Effect::ApplyPerpetual { target: TargetFilter::ParentTarget, modification: crate::types::ability::PerpetualModification::ModifyPowerToughness { @@ -9653,15 +9665,8 @@ fn try_parse_perpetual_modify_pt(tp: TextPair) -> Option { } // Anaphoric back-reference: "it perpetually gets +1/+0". - if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("it perpetually ").parse(lower) { - let (rest, _) = alt(( - tag::<_, _, OracleError<'_>>("gets "), - tag::<_, _, OracleError<'_>>("get "), - )) - .parse(rest) - .ok()?; - let (rest, (power_delta, toughness_delta)) = - nom_primitives::parse_pt_modifier(rest).ok()?; + if let Ok((rest, _)) = tag::<_, _, OracleError<'_>>("it ").parse(lower) { + let (rest, (power_delta, toughness_delta)) = parse_perpetual_pt_predicate(rest).ok()?; return tail_done(rest).then_some(Effect::ApplyPerpetual { target: TargetFilter::ParentTarget, modification: crate::types::ability::PerpetualModification::ModifyPowerToughness { @@ -9687,16 +9692,8 @@ fn try_parse_perpetual_modify_pt(tp: TextPair) -> Option { .ok() .map(|(rest, _)| rest) })?; - let (rest, _) = tag::<_, _, OracleError<'_>>("perpetually ") - .parse(after_subject) - .ok()?; - let (rest, _) = alt(( - tag::<_, _, OracleError<'_>>("gets "), - tag::<_, _, OracleError<'_>>("get "), - )) - .parse(rest) - .ok()?; - let (rest, (power_delta, toughness_delta)) = nom_primitives::parse_pt_modifier(rest).ok()?; + let (rest, (power_delta, toughness_delta)) = + parse_perpetual_pt_predicate(after_subject).ok()?; tail_done(rest).then_some(Effect::ApplyPerpetual { target: TargetFilter::Any, modification: crate::types::ability::PerpetualModification::ModifyPowerToughness { @@ -9706,6 +9703,145 @@ fn try_parse_perpetual_modify_pt(tp: TextPair) -> Option { }) } +/// Digital-only Alchemy (no CR entry for "perpetually"; the delta itself applies +/// as a CR 613.4c layer-7c power/toughness modification): parse the ZONE-SCOPED +/// CARD-SET form of the perpetual P/T modifier — +/// "[each|all|every] card(s) in your perpetually get(s) +N/+M" → +/// [`Effect::ApplyPerpetual`] with [`PerpetualModification::ModifyPowerToughness`] +/// over the parsed subject filter (Begin Anew, Grow Old Together, Klement Novice +/// Acolyte, Thoughtweft's Call, Kithkin Brinefarer, Blighted Nightmare, Feed the +/// Bog, Elvish Elegy, Advanced Floral Invocations, Bramblearmor Brawler). +/// +/// Sibling of [`try_parse_typed_cards_in_hand_perpetual_gain_cost`] for the +/// `ModifyCost` axis, and of [`try_parse_perpetual_modify_pt`], which owns every +/// SINGLE-OBJECT subject (target / "that " / "it" / self-ref). The two are +/// disjoint: this arm requires a type-phrase subject pinning a private per-player +/// card zone, which none of those prefixes can produce, and it is ordered AFTER +/// them. +/// +/// CR 400.1 + CR 400.3 + CR 109.4 + CR 109.5: hand, library and graveyard are +/// per-player zones (CR 400.1) whose contents have no controller (CR 109.4) and +/// always sit in their OWNER's copy (CR 400.3); CR 109.5 is what then makes +/// "you"/"your" on such a controllerless object refer to its OWNER, so +/// "your " is an OWNERSHIP scope. The +/// filter this arm emits is the plain `parse_type_phrase` shape +/// (`controller: Some(ControllerRef::You)` + `FilterProp::InZone`); the +/// owner-vs-controller reading is applied at the CONSUMING seam, where +/// `game/effects/perpetual.rs`'s mass zone branch answers the predicate through +/// `filter::matches_target_filter_in_owner_zone` for every +/// `filter::is_owner_scoped_zone`. That is the same dispatch +/// `off_zone_characteristics` already uses, so the parser does not fork a +/// second, arm-local encoding of the scope. +/// CR 115.1: this clause names no "target" — the affected set is a population +/// enumerated at resolution, which is why +/// `triggers::extract_target_filter_from_effect` carries a matching +/// `ApplyPerpetual` carve-out keyed on the SAME zone set +/// (`triggers::apply_perpetual_targets_zone_population`). Emitting a zone this +/// arm accepts but that predicate rejects would leave the targeting authority +/// demanding a pick the clause never declared, so the two zone sets are kept +/// identical by construction. +/// CR 611.2c: the affected set is determined when the effect begins — the +/// resolver (`game/effects/perpetual.rs`, the mass non-battlefield zone branch) +/// enumerates the zone once, at resolution. +/// +/// Fail-closed guards — every one returns `None` rather than installing a +/// subtly-wrong effect. +/// +/// "Fail-closed" describes THIS ARM only, not the pipeline. A reject here falls +/// through to the pre-existing dispatch, which for MOST of these shapes lands on +/// `Effect::Pump { target: Any }` — today's misparse. A clause with an unmodeled +/// compound rider ("…and gain flying") falls further, to the static-ability +/// reading `Effect::GenericEffect` (pinned as H11 in +/// `perpetual_zone_scoped_card_set_arm_honest_red`). Either way coverage still +/// reports `supported: true`. That differs from the cost sibling +/// [`try_parse_typed_cards_in_hand_perpetual_gain_cost`], whose rejects genuinely +/// reach `Effect::unimplemented`. So a reject here is "no worse than today", NOT +/// honest red; making it honest means fixing `oracle_effect/imperative.rs`'s +/// position-agnostic `get +` acceptance, which is a separate follow-up. The +/// guards: +/// * an unconsumed subject remainder (a rider, a multi-subject conjunction, or a +/// comma-separated multi-zone list — Arming Gala's "creatures you control and +/// creature cards in your hand, library, and graveyard"); +/// * a subject pinning zero zones, more than one zone, or a zone outside the +/// three private card zones (`perpetual_target_object_ids` has no mass +/// battlefield path: it falls back to `ids.push(ability.source_id)`, silently +/// pumping the source); +/// * a controller scope other than `You` (mirrors the deliberate "your hand"-only +/// restriction on the cost sibling, `parse_in_whose_hand_perpetually_gain_body`: +/// a `None` controller would make the resolver sweep EVERY player's zone); +/// * a delta the fixed-number grammar rejects ("+X/+X" — Golden Sidekick, Mycoid +/// Resurrection); `parse_pt_modifier` accepts digits only; +/// * any unconsumed clause tail (a compound "…and gain flying" rider). +fn try_parse_zone_scoped_cards_perpetual_modify_pt(tp: TextPair) -> Option { + fn tail_done(tail: &str) -> bool { + tail.is_empty() || tail == "." + } + + let ((filter, power_delta, toughness_delta), rest) = + nom_on_lower(tp.original, tp.lower, |input| { + // Bound the subject at the perpetual adverb so the type-phrase + // authority can never over-consume into the predicate. + let (after_subject, subject) = take_until("perpetually ").parse(input)?; + + // Predicate: "perpetually get(s) +N/+M" — the shared authority. + let (after_delta, (power_delta, toughness_delta)) = + parse_perpetual_pt_predicate(after_subject)?; + + // Subject -> TargetFilter through the SINGLE type-phrase authority. + // `parse_type_phrase` lowercases internally and `parse_subtype` + // returns canonical registry casing case-insensitively, so the + // already-lowered slice is the correct input (pinned by + // `parse_type_phrase_zone_scoped_card_set_subjects_bind_fully` in + // `oracle_target.rs`). The remainder must be empty, or the subject + // carries something this arm does not model. + // + // The leading universal quantifier ("each"/"all"/"every") is NOT + // stripped here: `parse_type_phrase` already owns that axis — see its + // CR 109.2 annotation in `oracle_target.rs`, which covers all three + // words. A local strip would duplicate that authority and lag it. + let (filter, subject_rest) = parse_type_phrase(subject.trim_end()); + if !subject_rest.trim().is_empty() { + return Err(oracle_err(subject)); + } + + Ok((after_delta, (filter, power_delta, toughness_delta))) + })?; + + // Honest-red: any unconsumed tail (a compound "…and gain flying") means this + // leaf cannot faithfully model the clause. + if !tail_done(rest) { + return None; + } + + // Zone guard: exactly one pinned zone, and one of the three private + // per-player CARD zones. An exhaustive slice match rejects the zero-zone + // ("creatures you control") and multi-zone (`InAnyZone`) cases in the same + // expression, and the enumerated set is identical to + // `triggers::apply_perpetual_targets_zone_population`, so every effect this + // arm emits is classified as a population — never a declared target — by the + // targeting authority. + match filter.extract_zones().as_slice() { + [Zone::Hand | Zone::Library | Zone::Graveyard] => {} + _ => return None, + } + + // Controller guard: only "your " is modeled. + let TargetFilter::Typed(typed) = filter else { + return None; + }; + if typed.controller != Some(ControllerRef::You) { + return None; + } + + Some(Effect::ApplyPerpetual { + target: TargetFilter::Typed(typed), + modification: PerpetualModification::ModifyPowerToughness { + power_delta, + toughness_delta, + }, + }) +} + /// Shared self-subject prefix for the perpetual self-grant arms /// (`try_parse_perpetual_grant_keywords`, `try_parse_perpetual_modify_cost`). /// diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index 086e14d473..88e94b4e34 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -36405,6 +36405,283 @@ fn perpetual_parser_maps_modify_pt() { )); } +/// SHAPE — zone-scoped card-set arm, positives. Every clause is the VERBATIM +/// text `card-data.json` hands the parser for a real card, one per grammar axis +/// (zone × number agreement × subtype head). Root cause #19: before this arm +/// each of these lowered to `Effect::Pump { target: Any }` with a `null` +/// duration, i.e. an UNTIL-END-OF-TURN pump over every permanent. +#[test] +fn perpetual_zone_scoped_card_set_arm_binds_zone_and_controller() { + use crate::types::ability::{ControllerRef, FilterProp, PerpetualModification, TypeFilter}; + use crate::types::zones::Zone; + + fn zone_scoped( + text: &str, + ) -> ( + Vec, + Option, + Vec, + i32, + i32, + ) { + let e = parse_effect(text); + let Effect::ApplyPerpetual { + target: TargetFilter::Typed(typed), + modification: + PerpetualModification::ModifyPowerToughness { + power_delta, + toughness_delta, + }, + } = e + else { + panic!("{text:?} must lower to a Typed ApplyPerpetual/ModifyPowerToughness, got {e:?}"); + }; + ( + typed.type_filters, + typed.controller, + typed.properties, + power_delta, + toughness_delta, + ) + } + + for (text, zone) in [ + // Begin Anew / Grow Old Together / Klement / Thoughtweft's Call. + ( + "Creature cards in your hand perpetually get +1/+1.", + Zone::Hand, + ), + // Blighted Nightmare / Feed the Bog / Advanced Floral Invocations. + ( + "creature cards in your graveyard perpetually get +1/+1", + Zone::Graveyard, + ), + // Bramblearmor Brawler. + ( + "creature cards in your library perpetually get +1/+1", + Zone::Library, + ), + // Elvish Elegy — the `each` quantifier plus singular verb agreement. + ( + "each creature card in your graveyard perpetually gets +1/+1", + Zone::Graveyard, + ), + // Universal-quantifier axis is owned by `parse_type_phrase` (CR 109.2), + // not by an arm-local strip — so all three of its words work, including + // "every", which no arm-local strip ever covered. + ( + "all creature cards in your hand perpetually get +1/+1", + Zone::Hand, + ), + ( + "every creature card in your graveyard perpetually gets +1/+1", + Zone::Graveyard, + ), + ] { + let (types, controller, props, power_delta, toughness_delta) = zone_scoped(text); + assert!( + types.contains(&TypeFilter::Creature), + "{text:?} must keep the Creature type axis, got {types:?}" + ); + // The arm emits the PLAIN `parse_type_phrase` shape — no arm-local + // rebinding. CR 400.3 + CR 109.4's owner-vs-controller reading of "your + // " is applied at the consuming seam + // (`game/effects/perpetual.rs`'s mass zone branch, through + // `filter::matches_target_filter_in_owner_zone`), which is where the + // engine already answers that question for off-zone keyword grants. + assert_eq!( + controller, + Some(ControllerRef::You), + "{text:?} must keep `parse_type_phrase`'s natural You scope, got {controller:?}" + ); + assert!( + !props.iter().any(|p| matches!(p, FilterProp::Owned { .. })), + "{text:?} must NOT carry an arm-local `Owned` rebind — the owner scoping \ + lives at the resolver seam, got {props:?}" + ); + assert!( + props + .iter() + .any(|p| matches!(p, FilterProp::InZone { zone: z } if *z == zone)), + "{text:?} must pin InZone{{{zone:?}}}, got {props:?}" + ); + assert_eq!((power_delta, toughness_delta), (1, 1), "{text:?}"); + } + + // Kithkin Brinefarer — subtype head in front of the core type. + let (types, _, props, _, _) = + zone_scoped("Kithkin creature cards in your hand perpetually get +1/+1"); + assert!( + types.contains(&TypeFilter::Subtype("Kithkin".to_string())) + && types.contains(&TypeFilter::Creature), + "Kithkin Brinefarer must keep BOTH the subtype and the core type, got {types:?}" + ); + assert!(props + .iter() + .any(|p| matches!(p, FilterProp::InZone { zone: Zone::Hand }))); + + // Building block, not the card: a type/zone/delta combination no printed + // card uses. The arm is parameterized on all three axes, so this must work + // without any new `tag()`. + let (types, controller, props, power_delta, toughness_delta) = + zone_scoped("artifact cards in your graveyard perpetually get -2/-0"); + assert!(types.contains(&TypeFilter::Artifact), "got {types:?}"); + assert_eq!(controller, Some(ControllerRef::You)); + assert!(!props.iter().any(|p| matches!(p, FilterProp::Owned { .. }))); + assert!(props.iter().any(|p| matches!( + p, + FilterProp::InZone { + zone: Zone::Graveyard + } + ))); + assert_eq!((power_delta, toughness_delta), (-2, 0)); +} + +/// SHAPE — zone-scoped card-set arm, hostile inputs. Each negative is paired +/// with a positive reach-guard proving the clause still reaches a real parse +/// (H5) or is left EXACTLY as broken as it is today (H7-H13), so a deferral is +/// honest rather than a silent new `Unimplemented`. +#[test] +fn perpetual_zone_scoped_card_set_arm_honest_red() { + fn is_zone_scoped_perpetual(e: &Effect) -> bool { + matches!( + e, + Effect::ApplyPerpetual { + target: TargetFilter::Typed(_), + .. + } + ) + } + + // H5 — no "perpetually" adverb: `take_until("perpetually ")` fails, and the + // clause keeps today's ordinary pump parse. Paired positive reach-guard: it + // really does still parse to a pump, and NOT to Unimplemented. + let e = parse_effect("Creature cards in your hand get +1/+1."); + assert!( + !is_zone_scoped_perpetual(&e), + "a non-perpetual pump must not reach the perpetual arm, got {e:?}" + ); + assert!( + matches!(e, Effect::Pump { .. } | Effect::PumpAll { .. }), + "reach-guard: the non-perpetual clause must still parse to a pump, got {e:?}" + ); + + // H6 — the single-object arm above still owns "target …". Davriel's + // Withering's real clause; §6.5's H14 pins that it also keeps its slot. + let e = parse_effect("Target creature an opponent controls perpetually gets -1/-2."); + assert!( + matches!( + &e, + Effect::ApplyPerpetual { + target: TargetFilter::Typed(f), + modification: crate::types::ability::PerpetualModification::ModifyPowerToughness { + power_delta: -1, + toughness_delta: -2, + }, + } if f.properties.is_empty() + && f.controller == Some(crate::types::ability::ControllerRef::Opponent) + ), + "the single-object arm must still own the targeted form, got {e:?}" + ); + + // Each negative carries its OWN expected fallthrough shape, not just + // "didn't match". A bare `!is_zone_scoped_perpetual` cannot tell "the arm + // was reached and this guard inside it rejected the clause" from "the arm + // was never reached at all" — a dispatch reorder or a `take_until` change + // would keep all seven green for the wrong reason. Pinning the exact + // fallthrough is what makes each one prove which guard fired. + let pump: fn(&Effect) -> bool = |e| matches!(e, Effect::Pump { .. }); + let generic: fn(&Effect) -> bool = |e| matches!(e, Effect::GenericEffect { .. }); + + for (label, text, expected_fallthrough, expected_desc) in [ + // H7 — X-valued delta: `parse_pt_modifier` is digits-only, so the arm is + // reached and rejected there; the clause keeps today's `Pump` with a + // `Variable("X")` delta. + ( + "H7 X-delta", + "creature cards in your hand perpetually get +X/+X, where X is the amount of life you gained", + pump, + "Effect::Pump", + ), + // H8 — multi-subject conjunction + multi-zone list (Arming Gala). + ( + "H8 multi-subject", + "creatures you control and creature cards in your hand, library, and graveyard perpetually get +1/+1", + pump, + "Effect::Pump", + ), + // H9 — battlefield/zoneless mass (Legion of Clay). `extract_zones()` is + // empty, so the slice match falls to `_`. The resolver has no mass + // battlefield path — it would pump the SOURCE only. + ( + "H9 zoneless", + "creatures you control perpetually get +1/+1", + pump, + "Effect::Pump", + ), + // H10 — controller scope other than `You`: the resolver would sweep + // EVERY player's copy of the zone. + ( + "H10 other controller", + "creature cards in that player's hand perpetually get +1/+1", + pump, + "Effect::Pump", + ), + // H11 — compound rider the leaf cannot model (`tail_done`). Falls all + // the way through to the pre-existing static-ability reading (a + // `SelfRef` continuous grant), which is exactly as wrong as it is today + // — the arm changes nothing about it. + ( + "H11 compound tail", + "Creature cards in your hand perpetually get +1/+1 and gain flying.", + generic, + "Effect::GenericEffect", + ), + // H12 — SINGULAR-SUBJECT near-miss (Dalkovan Outrider, verbatim). The + // clause reaches the arm — `take_until("perpetually ")` and + // `parse_perpetual_pt_predicate` both succeed, the zone is `Library`, + // the delta is a plain +1/+1 — and is rejected ONLY by the + // subject-remainder guard, because `parse_type_phrase` cannot consume a + // leading "the " head (`oracle_target.rs` strips "a "/"an "/"any " only, + // and only when a type word follows). That is a POPULATION-vs-ONE + // distinction, not a formatting one: the clause edits the single topmost + // library card, while this arm would hand the resolver a filter matching + // EVERY creature card in the library. Pinned so the guard cannot become + // incidental if that shared head-stripping authority ever learns "the ". + ( + "H12 singular 'the topmost' head", + "the topmost creature card in your library perpetually gets +1/+1", + pump, + "Effect::Pump", + ), + // H13 — the same singular-subject class through the OTHER head that + // shares this grammar: "a random card in your " (Golden + // Sidekick, Freyalise Skyshroud Partisan). Golden Sidekick's printed + // clause also carries an X-delta, which H7 already covers; the delta is + // fixed here so the "a random " head is the ONLY reason for the reject + // and the row stays a clean isolation of it. `parse_type_phrase` strips + // "a " only when a type word follows it, so "random" blocks the strip + // today — again incidentally. + ( + "H13 singular 'a random' head", + "a random creature card in your hand perpetually gets +1/+1", + pump, + "Effect::Pump", + ), + ] { + let e = parse_effect(text); + assert!( + !is_zone_scoped_perpetual(&e), + "{label}: {text:?} must stay deferred, got {e:?}" + ); + assert!( + expected_fallthrough(&e), + "{label}: reach-guard — {text:?} must fall through to \ + {expected_desc} (today's unchanged reading), got {e:?}" + ); + } +} + #[test] fn perpetual_parser_maps_grant_keywords() { use crate::types::ability::PerpetualModification; diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index e5c170dada..1f1a36f9db 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -10368,6 +10368,77 @@ mod tests { ); } + /// CR 400.1 — Zone-scoped CARD-SET subject pin. `parse_type_phrase` is the + /// single subject-filter authority the perpetual zone-population arm + /// (`oracle_effect::try_parse_zone_scoped_cards_perpetual_modify_pt`) + /// delegates to, so the exact contract that arm depends on is pinned here: + /// for `" card(s) in your "` the phrase must consume + /// COMPLETELY, yield a `Typed` filter, scope to `ControllerRef::You`, and + /// carry `FilterProp::InZone { zone }`. Both number forms (plural `cards` + /// and the singular `each … card` form) and both casings must agree — the + /// arm passes the pre-lowered slice through `nom_on_lower`, so a + /// casing-sensitive filter would silently diverge from the mixed-case + /// Oracle text. + #[test] + fn parse_type_phrase_zone_scoped_card_set_subjects_bind_fully() { + fn probe(text: &str) -> (Vec, Option, Vec) { + let (filter, rest) = parse_type_phrase(text); + assert!(rest.trim().is_empty(), "{text:?} left remainder {rest:?}"); + let TargetFilter::Typed(typed) = filter else { + panic!("{text:?} must bind a Typed filter, got {filter:?}"); + }; + (typed.type_filters, typed.controller, typed.properties) + } + + for (text, zone) in [ + ("Creature cards in your hand", Zone::Hand), + ("creature cards in your hand", Zone::Hand), + ("creature cards in your graveyard", Zone::Graveyard), + ("creature cards in your library", Zone::Library), + // The singular agreement form produced by "each card in your …". + ("creature card in your graveyard", Zone::Graveyard), + ] { + let (types, controller, props) = probe(text); + assert!( + types.contains(&TypeFilter::Creature), + "{text:?} must carry TypeFilter::Creature, got {types:?}" + ); + assert_eq!( + controller, + Some(ControllerRef::You), + "{text:?} must scope to the controller's own zone" + ); + assert!( + props + .iter() + .any(|p| matches!(p, FilterProp::InZone { zone: z } if *z == zone)), + "{text:?} must carry InZone{{{zone:?}}}, got {props:?}" + ); + } + + // Kithkin Brinefarer's subtype + core-type head. `parse_subtype` returns + // the canonical registry spelling case-insensitively, so the lowered + // slice the arm passes must still yield `Subtype("Kithkin")`. + for text in [ + "Kithkin creature cards in your hand", + "kithkin creature cards in your hand", + ] { + let (types, controller, props) = probe(text); + assert!( + types.contains(&TypeFilter::Subtype("Kithkin".to_string())), + "{text:?} must carry the canonical Subtype(\"Kithkin\"), got {types:?}" + ); + assert!( + types.contains(&TypeFilter::Creature), + "{text:?} must keep the Creature core type, got {types:?}" + ); + assert_eq!(controller, Some(ControllerRef::You)); + assert!(props + .iter() + .any(|p| matches!(p, FilterProp::InZone { zone: Zone::Hand }))); + } + } + #[test] fn card_with_mana_value_equal_to_offset_event_source() { let (f, rest) = parse_type_phrase( diff --git a/crates/engine/tests/integration/begin_anew_perpetual_hand_pump.rs b/crates/engine/tests/integration/begin_anew_perpetual_hand_pump.rs new file mode 100644 index 0000000000..20e5ec3c87 --- /dev/null +++ b/crates/engine/tests/integration/begin_anew_perpetual_hand_pump.rs @@ -0,0 +1,253 @@ +//! Begin Anew ({G}{G}{W}{W} Sorcery, digital-only Alchemy) — root cause #19 +//! (`docs/parser-misparse-backlog.md`) regression suite. +//! +//! Oracle (VERBATIM, verified against Scryfall and `client/public/card-data.json`): +//! "Destroy all creatures. Creature cards in your hand perpetually get +1/+1." +//! +//! Before this fix the second sentence lowered to +//! `Effect::Pump { power: Fixed(1), toughness: Fixed(1), target: Any }` with a +//! `null` duration — the perpetual routing AND the entire subject filter were +//! dropped, while coverage still reported `supported: true`. +//! +//! It must lower to `Effect::ApplyPerpetual { Typed[Creature] + controller You + +//! InZone{Hand}, ModifyPowerToughness{1,1} }` — AND (CR 115.1) it must declare +//! NO target, so casting surfaces zero prompts and the spell stays castable when +//! the hand holds no creature card. + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::ability::PerpetualModification; +use engine::types::game_state::{GameState, WaitingFor}; +use engine::types::identifiers::ObjectId; +use engine::types::mana::{ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +const BEGIN_ANEW: &str = + "Destroy all creatures. Creature cards in your hand perpetually get +1/+1."; + +/// `(base_power, base_toughness)` — the fields the perpetual edit writes +/// (`game/game_object.rs`, `ModifyPowerToughness` arm). +fn base_pt(state: &GameState, id: ObjectId) -> (Option, Option) { + let obj = state.objects.get(&id).expect("object must still exist"); + (obj.base_power, obj.base_toughness) +} + +/// The recorded perpetual modifications (`GameObject::perpetual_mods`). +fn perpetual_mods(state: &GameState, id: ObjectId) -> &[PerpetualModification] { + &state + .objects + .get(&id) + .expect("object must still exist") + .perpetual_mods +} + +fn pumped_by_one(mods: &[PerpetualModification]) -> bool { + mods.iter().any(|m| { + matches!( + m, + PerpetualModification::ModifyPowerToughness { + power_delta: 1, + toughness_delta: 1, + } + ) + }) +} + +/// {G}{G}{W}{W} plus slack (Test 2 casts a second spell afterwards). +fn mana() -> Vec { + let mut pool: Vec = (0..4) + .map(|_| ManaUnit::new(ManaType::Green, ObjectId(0), false, vec![])) + .collect(); + pool.extend((0..4).map(|_| ManaUnit::new(ManaType::White, ObjectId(0), false, vec![]))); + pool +} + +/// Test 1 — the multi-axis hostile fixture (claims C2 and C11). +/// +/// Every axis of the subject filter is given an independent counterexample that +/// differs from the positive subject on exactly ONE property. +#[test] +fn begin_anew_perpetually_buffs_only_your_hand_creature_cards() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool(P0, mana()); + + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Begin Anew", false, BEGIN_ANEW) + .id(); + + // Positive subject: TWO Creature cards in P0's hand. Two, deliberately: with + // exactly one matching card the population and a single declared target + // coincide, so the fixture could not tell "the whole hand population was + // enumerated" from "one card was picked out of a hidden hand" — the very + // defect this suite pins. Distinct P/T so each assertion is independent. + let hand_bear = scenario.add_creature_to_hand(P0, "Bear", 2, 2).id(); + let hand_ogre = scenario.add_creature_to_hand(P0, "Ogre", 4, 1).id(); + + // H2 — TYPE axis: a noncreature card in the SAME hand and the SAME zone. + let hand_land = scenario.add_land_to_hand(P0, "Forest").id(); + + // H1 — CONTROLLER axis: a Creature card in the OPPONENT's hand. + let opp_bear = scenario.add_creature_to_hand(P1, "Opp Bear", 2, 2).id(); + + // H3 — ZONE axis, ISOLATED. A P0-controlled CREATURE on the BATTLEFIELD that + // SURVIVES the first sentence. CR 702.12b: a permanent with indestructible + // can't be destroyed — enforced for the `DestroyAll` path by the battlefield + // filter inside `destroy::resolve_all`. It matches the type axis and the + // controller axis, so the ONLY thing that can exclude it is that + // `zone_object_ids(state, Zone::Hand)` never enumerates the battlefield — + // which is exactly the claim. (A vanilla creature here would be in the + // GRAVEYARD by the time ApplyPerpetual resolves and would not isolate the + // zone axis.) + let board_indestructible = scenario + .add_creature(P0, "Indestructible Bear", 3, 3) + .indestructible() + .id(); + + // CR 701.8a reach-guard + graveyard negative: a plain vanilla that DOES die. + let board_vanilla = scenario.add_vanilla(P0, 3, 3); + + let mut runner = scenario.build(); + + // Revert baseline, asserted BEFORE the cast. + assert_eq!(base_pt(runner.state(), hand_bear), (Some(2), Some(2))); + assert_eq!(base_pt(runner.state(), hand_ogre), (Some(4), Some(1))); + assert!(perpetual_mods(runner.state(), hand_bear).is_empty()); + assert!(perpetual_mods(runner.state(), hand_ogre).is_empty()); + + // C11: NO `.targeting(..)` is declared. If the `ApplyPerpetual` sub-ability + // surfaces a required target slot (the pre-carve-out behaviour) the harness + // panics in `pick_slot_target` before reaching any assertion below — + // reaching the next line IS the no-spurious-prompt assertion. + let outcome = runner.cast(spell).resolve(); + + // C11, explicit form: the pipeline halted at a clean priority window, not at + // a target/trigger prompt. + assert!( + matches!(outcome.final_waiting_for(), WaitingFor::Priority { .. }), + "Begin Anew must resolve to a clean Priority window, not a target prompt: {:?}", + outcome.final_waiting_for() + ); + + // CR 701.8a: the DestroyAll half really ran — the positive reach-guard that + // every negative assertion below is paired with. + outcome.assert_zone(&[board_vanilla], Zone::Graveyard); + outcome.assert_zone(&[board_indestructible], Zone::Battlefield); + + // Digital-only Alchemy (no CR entry for "perpetually"); the delta itself is a + // CR 613.4c layer-7c power/toughness modification recorded on the card. + assert_eq!( + base_pt(outcome.state(), hand_bear), + (Some(3), Some(3)), + "the creature card in the controller's hand must take a PERMANENT base P/T edit" + ); + assert!(pumped_by_one(perpetual_mods(outcome.state(), hand_bear))); + // The POPULATION claim: EVERY matching card in the zone, not one chosen card. + assert_eq!( + base_pt(outcome.state(), hand_ogre), + (Some(5), Some(2)), + "the perpetual grant is a zone POPULATION, so the second matching hand card \ + must be modified too — with a declared target slot only one would be" + ); + assert!(pumped_by_one(perpetual_mods(outcome.state(), hand_ogre))); + + // H2 — type axis. + assert!(perpetual_mods(outcome.state(), hand_land).is_empty()); + // H1 — controller axis. + assert_eq!(base_pt(outcome.state(), opp_bear), (Some(2), Some(2))); + assert!(perpetual_mods(outcome.state(), opp_bear).is_empty()); + // H3 — zone axis, isolated (same controller, same type, battlefield). + assert_eq!( + base_pt(outcome.state(), board_indestructible), + (Some(3), Some(3)), + "the surviving battlefield creature must keep its printed 3/3 base" + ); + assert!(perpetual_mods(outcome.state(), board_indestructible).is_empty()); + // Graveyard negative (the dead vanilla is not in Hand either). + assert!(perpetual_mods(outcome.state(), board_vanilla).is_empty()); + // H-source: the spell object must NOT be the source-fallback target. + assert!(perpetual_mods(outcome.state(), spell).is_empty()); +} + +/// Test 2 — the PERMANENCE discriminator (claim C3). This is what separates +/// `ApplyPerpetual` from *any* `Pump`, however well targeted: a `Pump` is a +/// battlefield-scoped transient continuous effect swept by +/// `prune_end_of_turn_effects` and cannot touch a card sitting in hand at all, +/// so the buffed card could never ENTER the battlefield already enlarged. +#[test] +fn begin_anew_perpetual_buff_survives_the_card_entering_the_battlefield() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool(P0, mana()); + + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Begin Anew", false, BEGIN_ANEW) + .id(); + let hand_bear = scenario.add_creature_to_hand(P0, "Bear", 2, 2).id(); + // Second matching card so the population is not a one-element set (see + // Test 1) — it stays in hand and is never cast. + let hand_ogre = scenario.add_creature_to_hand(P0, "Ogre", 4, 1).id(); + let board_vanilla = scenario.add_vanilla(P0, 3, 3); + + let mut runner = scenario.build(); + assert_eq!(base_pt(runner.state(), hand_bear), (Some(2), Some(2))); + + let outcome = runner.cast(spell).resolve(); + // Reach-guard: the spell really resolved. + outcome.assert_zone(&[board_vanilla], Zone::Graveyard); + assert_eq!(base_pt(outcome.state(), hand_bear), (Some(3), Some(3))); + assert_eq!(base_pt(outcome.state(), hand_ogre), (Some(5), Some(2))); + + // Now cast the buffed card. CR 613.4c: the layer pass derives live P/T from + // the edited base, so it must ENTER as a 3/3. + let outcome = runner.cast(hand_bear).resolve(); + outcome.assert_zone(&[hand_bear], Zone::Battlefield); + let entered = outcome + .state() + .objects + .get(&hand_bear) + .expect("the creature is on the battlefield"); + assert_eq!( + (entered.power, entered.toughness), + (Some(3), Some(3)), + "a perpetual +1/+1 granted while the card was in hand must still be live \ + after it enters the battlefield (a Pump could never do this)" + ); + assert!(pumped_by_one(&entered.perpetual_mods)); +} + +/// Test 3 — the empty-set reach-guard and the castability claim (C5, C12, H4). +/// +/// CR 115.1: the perpetual clause declares no target, so the spell must be +/// castable with no creature card anywhere in hand. Pre-carve-out this returns +/// `Err(EngineError::ActionNotAllowed("No legal targets available"))` from +/// `no_legal_target_slots()`, because the `Typed[Creature] + InZone{Hand} + You` +/// filter matches nothing and the ability is not `optional_targeting`. +#[test] +fn begin_anew_with_no_matching_hand_card_is_castable_and_does_not_hit_the_source() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool(P0, mana()); + + // P0's hand holds ONLY Begin Anew — no creature cards in ANY hand. + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Begin Anew", false, BEGIN_ANEW) + .id(); + let board_vanilla = scenario.add_vanilla(P0, 3, 3); + + let mut runner = scenario.build(); + let outcome = runner.cast(spell).try_resolve().expect( + "Begin Anew must be castable with no creature card in hand \ + (CR 115.1: the perpetual clause declares no target)", + ); + + // Reach-guard: the spell actually resolved. + outcome.assert_zone(&[board_vanilla], Zone::Graveyard); + // C5: the empty matching set must NOT fall back to the source + // (`ids.push(ability.source_id)` is unreachable on the mass zone path). + assert!( + perpetual_mods(outcome.state(), spell).is_empty(), + "an empty hand population must not fall back to the spell source" + ); + assert!(perpetual_mods(outcome.state(), board_vanilla).is_empty()); +} diff --git a/crates/engine/tests/integration/feed_the_bog_perpetual_graveyard_parent.rs b/crates/engine/tests/integration/feed_the_bog_perpetual_graveyard_parent.rs new file mode 100644 index 0000000000..04ec7e0adc --- /dev/null +++ b/crates/engine/tests/integration/feed_the_bog_perpetual_graveyard_parent.rs @@ -0,0 +1,341 @@ +//! Feed the Bog ({1}{B} Sorcery, digital-only Alchemy) — root cause #19 +//! (`docs/parser-misparse-backlog.md`), the SECOND structural shape of the class. +//! +//! Oracle (VERBATIM, verified against `client/public/card-data.json`): +//! "Replicate {1}{B}\n +//! Creature cards in your graveyard perpetually get +1/+1. Then return target +//! creature card with mana value 3 or less from your graveyard to the +//! battlefield." +//! +//! `begin_anew_perpetual_hand_pump.rs` covers the perpetual as a SUB of an +//! untargeted parent, and `game/effects/perpetual.rs`'s chain-inheritance test +//! covers the perpetual as a SUB of a TARGETED parent. This file covers the +//! remaining shape: `ApplyPerpetual` as the **PARENT**, with a targeted +//! `ChangeZone` sub — and it is the shape where the targeting carve-out changes a +//! SHIPPED card's declared target-slot COUNT. Before the carve-out, Feed the Bog +//! demanded TWO graveyard picks (one for the suppressed `ApplyPerpetual` slot, +//! one for the `ChangeZone`); after it, exactly one. +//! +//! `assign_targets_recursive` (`game/ability_utils.rs`) gates each node's slot on +//! the same `extract_target_filter_from_effect` authority, so suppressing the +//! parent's filter is what makes the parent take no target and hands the single +//! declared target to the sub — and is also what leaves the parent's +//! `ability.targets` EMPTY, the precondition for `perpetual_target_object_ids` +//! reaching its mass zone branch instead of short-circuiting on one chosen card. + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::game::zones::move_to_zone; +use engine::types::ability::PerpetualModification; +use engine::types::game_state::{GameState, WaitingFor}; +use engine::types::identifiers::ObjectId; +use engine::types::mana::{ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +const FEED_THE_BOG: &str = "Replicate {1}{B}\nCreature cards in your graveyard perpetually get \ + +1/+1. Then return target creature card with mana value 3 or less \ + from your graveyard to the battlefield."; + +fn base_pt(state: &GameState, id: ObjectId) -> (Option, Option) { + let obj = state.objects.get(&id).expect("object must still exist"); + (obj.base_power, obj.base_toughness) +} + +fn perpetual_mods(state: &GameState, id: ObjectId) -> &[PerpetualModification] { + &state + .objects + .get(&id) + .expect("object must still exist") + .perpetual_mods +} + +fn pumped_by_one(mods: &[PerpetualModification]) -> bool { + mods.iter().any(|m| { + matches!( + m, + PerpetualModification::ModifyPowerToughness { + power_delta: 1, + toughness_delta: 1, + } + ) + }) +} + +/// Which player's graveyard holds `id`. CR 400.3 makes this the OWNER's, which +/// is the whole point of the owner-scoping test below — so the index is derived +/// from the actual per-player zone lists rather than read off `obj.owner`. +fn graveyard_index(state: &GameState, id: ObjectId) -> usize { + state + .players + .iter() + .position(|p| p.graveyard.contains(&id)) + .expect("card must be in some player's graveyard") +} + +/// {1}{B} with slack. +fn mana() -> Vec { + (0..4) + .map(|_| ManaUnit::new(ManaType::Black, ObjectId(0), false, vec![])) + .collect() +} + +/// The `ApplyPerpetual`-as-PARENT witness. +/// +/// Two independent claims, both of which flip if the targeting carve-out is +/// reverted: +/// +/// 1. **Exactly ONE object target slot** — the `ChangeZone`'s. Only ONE object +/// intent is declared. The driver answers one slot per declared object, in +/// written order (CR 601.2c); a second required slot makes `pick_slot_target` +/// panic with "could not satisfy required target slot 1". Reaching the +/// assertions below therefore IS the "exactly one prompt" assertion, and with +/// the carve-out reverted the parent's `ApplyPerpetual` claims slot 0 and the +/// `ChangeZone` slot 1 has nothing left to consume. +/// 2. **Both matching graveyard cards take the perpetual grant** — the mass zone +/// branch really enumerated the POPULATION. With a slot filled, +/// `perpetual_target_object_ids` short-circuits on `ability.targets` and only +/// the one chosen card would be modified. +#[test] +fn feed_the_bog_declares_one_target_and_perpetually_buffs_the_whole_graveyard() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool(P0, mana()); + + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Feed the Bog", false, FEED_THE_BOG) + .id(); + + // Positive subject: TWO creature cards in P0's graveyard, DISTINCT base P/T + // so each assertion is independent. Two, deliberately — with one matching + // card the population and a single declared target coincide and the fixture + // could not discriminate. Both have mana value 0, so both are also legal + // `ChangeZone` targets ("mana value 3 or less"). + let gy_bear = scenario.add_creature_to_graveyard(P0, "Bear", 2, 2).id(); + let gy_ogre = scenario.add_creature_to_graveyard(P0, "Ogre", 4, 1).id(); + + // TYPE axis: a noncreature card in the SAME graveyard. + let gy_shock = scenario.add_spell_to_graveyard(P0, "Shock", true).id(); + // CONTROLLER axis: a creature card in the OPPONENT's graveyard. + let opp_bear = scenario + .add_creature_to_graveyard(P1, "Opp Bear", 2, 2) + .id(); + // ZONE axis: a P0 creature already on the battlefield. + let board_bear = scenario.add_creature(P0, "Board Bear", 3, 3).id(); + + let mut runner = scenario.build(); + + // Revert baseline, asserted BEFORE the cast. + assert_eq!(base_pt(runner.state(), gy_bear), (Some(2), Some(2))); + assert_eq!(base_pt(runner.state(), gy_ogre), (Some(4), Some(1))); + assert!(perpetual_mods(runner.state(), gy_bear).is_empty()); + assert!(perpetual_mods(runner.state(), gy_ogre).is_empty()); + + // Claim 1: exactly ONE object intent for the whole spell. + let outcome = runner.cast(spell).target_objects(&[gy_bear]).resolve(); + + assert!( + matches!(outcome.final_waiting_for(), WaitingFor::Priority { .. }), + "Feed the Bog must resolve to a clean Priority window, not a further prompt: {:?}", + outcome.final_waiting_for() + ); + + // Reach-guard AND slot-identity proof: the single declared target was + // consumed by the `ChangeZone` sub, which really ran. + outcome.assert_zone(&[gy_bear], Zone::Battlefield); + + // The perpetual ran BEFORE the return ("Then return ..."), so the returned + // card was still in the graveyard when the population was enumerated and + // enters the battlefield already enlarged (CR 613.4c derives live P/T from + // the edited base). + assert_eq!( + base_pt(outcome.state(), gy_bear), + (Some(3), Some(3)), + "the returned card must carry the perpetual edit onto the battlefield" + ); + assert!(pumped_by_one(perpetual_mods(outcome.state(), gy_bear))); + let entered = outcome + .state() + .objects + .get(&gy_bear) + .expect("the returned creature is on the battlefield"); + assert_eq!((entered.power, entered.toughness), (Some(3), Some(3))); + + // Claim 2, the POPULATION claim: the SECOND matching graveyard card was + // never a declared target, stayed in the graveyard, and still took the + // grant. This is the assertion that fails if the carve-out is reverted and + // the parent claims a target slot of its own. + outcome.assert_zone(&[gy_ogre], Zone::Graveyard); + assert_eq!( + base_pt(outcome.state(), gy_ogre), + (Some(5), Some(2)), + "the perpetual grant is a zone POPULATION, so the second matching graveyard \ + card must be modified too — with a declared target slot only one would be" + ); + assert!(pumped_by_one(perpetual_mods(outcome.state(), gy_ogre))); + + // Type axis. + assert!(perpetual_mods(outcome.state(), gy_shock).is_empty()); + // Controller axis. + assert_eq!(base_pt(outcome.state(), opp_bear), (Some(2), Some(2))); + assert!(perpetual_mods(outcome.state(), opp_bear).is_empty()); + // Zone axis. + assert_eq!(base_pt(outcome.state(), board_bear), (Some(3), Some(3))); + assert!(perpetual_mods(outcome.state(), board_bear).is_empty()); + // The spell source must not be the source-fallback recipient. + assert!(perpetual_mods(outcome.state(), spell).is_empty()); +} + +/// "Your graveyard" is scoped by OWNERSHIP, not by last-known control. +/// +/// CR 400.3: an object that would go to any library, graveyard or hand other +/// than its owner's goes to its OWNER's corresponding zone. CR 108.3: ownership +/// is fixed for the whole game. CR 109.4: an object outside the battlefield and +/// stack has no controller at all — so the only player-scoping a graveyard +/// population can legitimately use is ownership. +/// +/// The engine's `effective_controller` (`game/filter.rs`) answers a control +/// predicate for a non-battlefield object out of `state.lki_cache`, i.e. with +/// the LAST KNOWN controller. This test builds exactly the state where that +/// diverges from ownership — two creatures that changed hands and then died in +/// the same step — and pins that the perpetual grant follows the OWNER: +/// +/// * `stolen` is OWNED by P0, was CONTROLLED by P1, and died into P0's +/// graveyard. It IS in "your graveyard" and MUST take the grant. This +/// assertion fails under plain `matches_target_filter`, whose control +/// predicate reads the LKI thief (P1). +/// * `loaned` is OWNED by P1, was CONTROLLED by P0, and died into P1's +/// graveyard. It is NOT in "your graveyard" and MUST NOT take the grant. +/// This assertion fails under plain `matches_target_filter`, which would +/// match `ControllerRef::You` against the stale LKI controller and hand the +/// caster's grant to a card in the opponent's graveyard — nothing else on +/// this path is player-scoped, since `zone_object_ids` sweeps EVERY player's +/// graveyard (`game/targeting.rs`) and `FilterProp::InZone` only compares +/// `obj.zone` (`game/filter.rs`). +/// +/// The shipped scoping lives at the RESOLVER seam, not in the parsed filter: +/// `game/effects/perpetual.rs`'s mass zone branch answers the predicate through +/// `filter::matches_target_filter_in_owner_zone` for every +/// `filter::is_owner_scoped_zone`. The subject filter keeps `parse_type_phrase`'s +/// plain `controller: Some(ControllerRef::You)` shape. +/// +/// `opp_bear` (owned AND last controlled by P1) is the plain controller-axis +/// negative; it is already covered above but repeated here so this fixture is +/// self-contained even if the player scoping is dropped entirely. +#[test] +fn feed_the_bog_scopes_the_graveyard_population_by_owner_not_last_known_controller() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool(P0, mana()); + + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Feed the Bog", false, FEED_THE_BOG) + .id(); + + // The declared `ChangeZone` target: an ordinary P0-owned graveyard creature + // with no LKI at all. + let gy_bear = scenario.add_creature_to_graveyard(P0, "Bear", 2, 2).id(); + // Plain controller-axis negative: owned and controlled by P1 throughout. + let opp_bear = scenario + .add_creature_to_graveyard(P1, "Opp Bear", 2, 2) + .id(); + + // Both start on the battlefield so that the move to the graveyard records an + // LKI snapshot (`apply_zone_exit_cleanup` snapshots on a battlefield exit). + let stolen = scenario.add_creature(P0, "Stolen Ogre", 4, 1).id(); + let loaned = scenario.add_creature(P1, "Loaned Golem", 3, 3).id(); + + let mut runner = scenario.build(); + + // The theft, and its mirror image. + runner + .state_mut() + .objects + .get_mut(&stolen) + .expect("stolen creature exists") + .controller = P1; + runner + .state_mut() + .objects + .get_mut(&loaned) + .expect("loaned creature exists") + .controller = P0; + + // Both die THIS step. `state.lki_cache` is step-scoped, so the divergence is + // still live when the spell resolves below. + let mut events = Vec::new(); + move_to_zone(runner.state_mut(), stolen, Zone::Graveyard, &mut events); + move_to_zone(runner.state_mut(), loaned, Zone::Graveyard, &mut events); + + // Reach-guards: the fixture really is the divergent state this test is about. + // Without these the owner assertion below could pass for the trivial reason + // that no LKI was ever recorded. + { + let state = runner.state(); + assert_eq!( + graveyard_index(state, stolen), + 0, + "CR 400.3: the stolen card must be in its OWNER's (P0's) graveyard" + ); + assert_eq!( + graveyard_index(state, loaned), + 1, + "CR 400.3: the loaned card must be in its OWNER's (P1's) graveyard" + ); + assert_eq!( + state + .lki_cache + .get(&stolen) + .expect("a battlefield -> graveyard move records LKI") + .controller, + P1, + "reach-guard: the stolen card's LAST KNOWN controller must be the thief" + ); + assert_eq!( + state + .lki_cache + .get(&loaned) + .expect("a battlefield -> graveyard move records LKI") + .controller, + P0, + "reach-guard: the loaned card's LAST KNOWN controller must be the caster" + ); + assert_eq!(base_pt(state, stolen), (Some(4), Some(1))); + assert_eq!(base_pt(state, loaned), (Some(3), Some(3))); + } + + let outcome = runner.cast(spell).target_objects(&[gy_bear]).resolve(); + + // Positive reach-guard: the spell really resolved and the `ChangeZone` sub + // consumed the single declared target. + outcome.assert_zone(&[gy_bear], Zone::Battlefield); + assert_eq!(base_pt(outcome.state(), gy_bear), (Some(3), Some(3))); + + // OWNER-SCOPED POSITIVE. Fails under plain `matches_target_filter`, whose + // control predicate reads the LKI thief. + outcome.assert_zone(&[stolen], Zone::Graveyard); + assert_eq!( + base_pt(outcome.state(), stolen), + (Some(5), Some(2)), + "CR 400.3 + CR 108.3: a card you OWN that an opponent last controlled is \ + still in YOUR graveyard and must take the grant" + ); + assert!(pumped_by_one(perpetual_mods(outcome.state(), stolen))); + + // OWNER-SCOPED NEGATIVE. Fails if the mass branch stops dispatching through + // `matches_target_filter_in_owner_zone`. + outcome.assert_zone(&[loaned], Zone::Graveyard); + assert_eq!( + base_pt(outcome.state(), loaned), + (Some(3), Some(3)), + "CR 400.3: a card an OPPONENT owns is in THEIR graveyard even though you \ + controlled it last — it must NOT take the grant" + ); + assert!(perpetual_mods(outcome.state(), loaned).is_empty()); + + // Plain controller-axis negative, restated for self-containment. + assert_eq!(base_pt(outcome.state(), opp_bear), (Some(2), Some(2))); + assert!(perpetual_mods(outcome.state(), opp_bear).is_empty()); + + // The spell must still not be the source-fallback recipient. + assert!(perpetual_mods(outcome.state(), spell).is_empty()); +} diff --git a/crates/engine/tests/integration/fountainport_charmer_perpetual_hand_cost.rs b/crates/engine/tests/integration/fountainport_charmer_perpetual_hand_cost.rs new file mode 100644 index 0000000000..562210cabf --- /dev/null +++ b/crates/engine/tests/integration/fountainport_charmer_perpetual_hand_cost.rs @@ -0,0 +1,254 @@ +//! Fountainport Charmer ({1}{G} Creature — Frog Bard, digital-only Alchemy) — +//! the `ModifyCost` sibling of root cause #19's zone-population `ApplyPerpetual`. +//! +//! Oracle (VERBATIM, verified against `client/public/card-data.json`): +//! "Offspring {2}\n +//! When Fountainport Charmer enters, creature cards in your hand perpetually +//! gain \"This spell costs {1} less to cast.\"" +//! +//! This card ships today. Its ETB lowers (via +//! `oracle_effect::try_parse_typed_cards_in_hand_perpetual_gain_cost`) to +//! `ApplyPerpetual { Typed[Card, Creature] + controller You + InZone{Hand}, +//! ModifyCost{Reduce, {1}} }` — the SAME filter shape as the `+N/+M` arm, so it +//! rides both engine seams this change touches: +//! +//! * `triggers::extract_target_filter_from_effect`'s zone-population carve-out, +//! which suppresses the stack-time slot. Before it, the controller was forced +//! to pick ONE card out of their own hidden hand and only that card got the +//! reduction — and with no matching card the trigger was DROPPED entirely +//! (`TriggerDispatchDisposition::DroppedTargetUnresolved`). +//! * `effects::perpetual`'s mass zone branch, which now answers the filter's +//! "your" predicate through `filter::matches_target_filter_in_owner_zone` for +//! every `filter::is_owner_scoped_zone` (CR 109.4 + CR 400.3). +//! +//! `crates/engine/src/game/casting_tests.rs`'s +//! `perpetual_mass_hand_cost_reduces_only_matching_cards` hand-builds a +//! `ResolvedAbility` with empty `targets`, so it bypasses +//! `extract_target_filter_from_effect` entirely and cannot see either seam. This +//! file drives the REAL cast pipeline instead. +//! +//! Scope note: the EMPTY-population case (the trigger must still reach the stack +//! when no card matches) is deliberately NOT tested here. Through the cast +//! pipeline a dropped trigger and a resolved no-op trigger are observationally +//! identical — no event, no state delta — so such a test would be vacuous. That +//! claim is pinned at the trigger-dispatch seam instead, by +//! `game::triggers::tests::trigger_hosted_zone_perpetual_is_not_dropped_when_hand_is_empty`, +//! which asserts `stack.len() == 1` and keys on the SAME +//! `apply_perpetual_targets_zone_population` predicate over the SAME filter +//! shape (the `PerpetualModification` variant is irrelevant to that predicate). + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::game::zones::move_to_zone; +use engine::types::ability::PerpetualModification; +use engine::types::game_state::{GameState, WaitingFor}; +use engine::types::identifiers::ObjectId; +use engine::types::mana::{ManaCost, ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::statics::CostModifyMode; +use engine::types::zones::Zone; + +const FOUNTAINPORT_CHARMER: &str = "Offspring {2}\nWhen Fountainport Charmer enters, creature \ + cards in your hand perpetually gain \"This spell costs {1} \ + less to cast.\""; + +/// True when the object carries the {1} self-spell cost reduction this card +/// grants (`game/game_object.rs`, the `ModifyCost` arm of +/// `apply_perpetual_modification`). +fn reduced_by_one(state: &GameState, id: ObjectId) -> bool { + state + .objects + .get(&id) + .expect("object must still exist") + .perpetual_mods + .iter() + .any(|m| { + matches!( + m, + PerpetualModification::ModifyCost { + mode: CostModifyMode::Reduce, + amount, + } if *amount == ManaCost::generic(1) + ) + }) +} + +/// Which player's hand holds `id`. CR 400.3 makes this the OWNER's, which is the +/// point of the owner-scoping axes below — so it is derived from the actual +/// per-player zone lists rather than read off `obj.owner`. +fn hand_index(state: &GameState, id: ObjectId) -> usize { + state + .players + .iter() + .position(|p| p.hand.contains(&id)) + .expect("card must be in some player's hand") +} + +/// {1}{G} with slack. +fn mana() -> Vec { + let mut pool: Vec = (0..2) + .map(|_| ManaUnit::new(ManaType::Green, ObjectId(0), false, vec![])) + .collect(); + pool.extend((0..2).map(|_| ManaUnit::new(ManaType::Colorless, ObjectId(0), false, vec![]))); + pool +} + +/// The cast-pipeline witness for the `ModifyCost` sibling. +/// +/// Three claims, each of which flips if part of this change is reverted: +/// +/// 1. **Zero target prompts.** No `.target_objects(..)` is declared. If the ETB +/// trigger surfaces a required target slot (the pre-carve-out behaviour) the +/// shared resolution driver panics before any assertion below, so reaching +/// them IS the no-prompt assertion; `final_waiting_for()` restates it. +/// 2. **The whole hand POPULATION is reduced**, not one chosen card — pinned by +/// a SECOND matching hand card that was never a declared target. +/// 3. **The population is scoped by OWNERSHIP, not last-known control** +/// (CR 109.4 + CR 400.3). `stolen` and `loaned` are the two directions; they +/// fail if the mass branch uses plain `matches_target_filter`. +#[test] +fn fountainport_charmer_reduces_the_whole_hand_population_with_no_target_prompt() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool(P0, mana()); + + let charmer = scenario + .add_creature_to_hand_from_oracle(P0, "Fountainport Charmer", 2, 3, FOUNTAINPORT_CHARMER) + .id(); + + // Positive subject: TWO creature cards in P0's hand. Two, deliberately — + // with exactly one matching card the population and a single declared + // target coincide and the fixture could not discriminate them. + let hand_bear = scenario.add_creature_to_hand(P0, "Bear", 2, 2).id(); + let hand_ogre = scenario.add_creature_to_hand(P0, "Ogre", 4, 1).id(); + // TYPE axis: a noncreature card in the SAME hand. + let hand_land = scenario.add_land_to_hand(P0, "Forest").id(); + // PLAIN controller/owner axis: a creature card owned AND last controlled by + // the opponent, in the opponent's hand. + let opp_bear = scenario.add_creature_to_hand(P1, "Opp Bear", 2, 2).id(); + + // The two LKI-divergent cards. Both start on the battlefield so that leaving + // it records an LKI snapshot (`apply_zone_exit_cleanup`). + let stolen = scenario.add_creature(P0, "Stolen Ogre", 4, 1).id(); + let loaned = scenario.add_creature(P1, "Loaned Golem", 3, 3).id(); + + let mut runner = scenario.build(); + + // The theft, and its mirror image. + runner + .state_mut() + .objects + .get_mut(&stolen) + .expect("stolen creature exists") + .controller = P1; + runner + .state_mut() + .objects + .get_mut(&loaned) + .expect("loaned creature exists") + .controller = P0; + + // Both bounce THIS step. `state.lki_cache` is step-scoped, so the divergence + // is still live when the ETB trigger resolves below. + let mut events = Vec::new(); + move_to_zone(runner.state_mut(), stolen, Zone::Hand, &mut events); + move_to_zone(runner.state_mut(), loaned, Zone::Hand, &mut events); + + // Reach-guards on the FIXTURE: without these the owner assertions could pass + // for the trivial reason that no LKI was ever recorded. + { + let state = runner.state(); + assert_eq!( + hand_index(state, stolen), + 0, + "CR 400.3: the stolen card must be in its OWNER's (P0's) hand" + ); + assert_eq!( + hand_index(state, loaned), + 1, + "CR 400.3: the loaned card must be in its OWNER's (P1's) hand" + ); + assert_eq!( + state + .lki_cache + .get(&stolen) + .expect("a battlefield -> hand move records LKI") + .controller, + P1, + "reach-guard: the stolen card's LAST KNOWN controller must be the thief" + ); + assert_eq!( + state + .lki_cache + .get(&loaned) + .expect("a battlefield -> hand move records LKI") + .controller, + P0, + "reach-guard: the loaned card's LAST KNOWN controller must be the caster" + ); + } + + // Revert baseline, asserted BEFORE the cast. + for id in [hand_bear, hand_ogre, hand_land, opp_bear, stolen, loaned] { + assert!( + !reduced_by_one(runner.state(), id), + "no card may carry the reduction before the trigger resolves" + ); + } + + // Claim 1: cast with NO declared target intent at all. + let outcome = runner.cast(charmer).resolve(); + + assert!( + matches!(outcome.final_waiting_for(), WaitingFor::Priority { .. }), + "Fountainport Charmer's ETB declares no target, so the cast must end at a \ + clean Priority window rather than a hidden-hand pick: {:?}", + outcome.final_waiting_for() + ); + + // Reach-guard: the creature really resolved and its ETB really fired. + outcome.assert_zone(&[charmer], Zone::Battlefield); + + // Claim 2: BOTH matching hand cards took the grant. + assert!( + reduced_by_one(outcome.state(), hand_bear), + "the first matching creature card in the caster's hand must be reduced" + ); + assert!( + reduced_by_one(outcome.state(), hand_ogre), + "the SECOND matching hand card must be reduced too — the grant is a zone \ + POPULATION; with a declared target slot only one card would be" + ); + + // Claim 3, POSITIVE direction: a card you OWN that an opponent last + // controlled is still in YOUR hand and must be reduced. Fails under plain + // `matches_target_filter`, whose control predicate reads the LKI thief. + outcome.assert_zone(&[stolen], Zone::Hand); + assert!( + reduced_by_one(outcome.state(), stolen), + "CR 400.3 + CR 108.3: a card you own that an opponent last controlled is in \ + YOUR hand and must take the grant" + ); + + // Claim 3, NEGATIVE direction: a card an OPPONENT owns is in THEIR hand even + // though you controlled it last. Fails under plain `matches_target_filter`, + // which would match `ControllerRef::You` against the stale LKI controller and + // hand the caster's cost reduction to a card in the opponent's hand. + outcome.assert_zone(&[loaned], Zone::Hand); + assert!( + !reduced_by_one(outcome.state(), loaned), + "CR 400.3: a card an opponent OWNS is in THEIR hand even though you \ + controlled it last — it must NOT take the grant" + ); + + // Type axis, and the plain controller/owner axis. + assert!(!reduced_by_one(outcome.state(), hand_land), "type axis"); + assert!( + !reduced_by_one(outcome.state(), opp_bear), + "controller/owner axis" + ); + // The source must not be the source-fallback recipient of its own grant. + assert!( + !reduced_by_one(outcome.state(), charmer), + "the mass zone path must never fall back to the trigger source" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 34a971b839..f3f3c32979 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -44,6 +44,7 @@ mod batched_trigger_subject_count; mod battle_of_wits; mod bbfu10_entered_this_turn_snapshot; mod bbfu7_attacks_if_able_not_goad; +mod begin_anew_perpetual_hand_pump; mod belbe_thornbow_life_loss; mod betor_lifelink_counters_repro; mod birgi; @@ -189,6 +190,7 @@ mod eyetwitch_learn_decline_lesson; mod fact_or_fiction_pile_separation; mod fateful_handoff_target_mana_value_draw; mod favor_of_the_mighty_greatest_mana_value_protection; +mod feed_the_bog_perpetual_graveyard_parent; mod felisa_fang_of_silverquill; mod festival_of_embers_graveyard_additional_cost; mod fevered_visions; @@ -203,6 +205,7 @@ mod floodpits_drowner; mod flowstone_surge_mixed_anthem; mod forced_retarget_multi_role_mana_6056; mod foretell_pipeline; +mod fountainport_charmer_perpetual_hand_cost; mod frenzy_attacker_unblocked_pump; mod frodo_ringbearer_must_be_blocked_gate; mod frostcliff_siege_anchor_word_modes; diff --git a/docs/parser-misparse-backlog.md b/docs/parser-misparse-backlog.md index 81445a33cc..0c10338397 100644 --- a/docs/parser-misparse-backlog.md +++ b/docs/parser-misparse-backlog.md @@ -12,7 +12,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top | # | Root cause | # cards | Fix hint (where it likely lives) | |---|------------|--------:|----------------------------------| -| 1 | Relative-clause / filter restriction on target dropped | 746 | oracle_target.rs / game/filter.rs — extend TargetFilter property extraction for trailing relative clauses | +| 1 | Relative-clause / filter restriction on target dropped | 743 | oracle_target.rs / game/filter.rs — extend TargetFilter property extraction for trailing relative clauses | | 2 | Dropped intervening-if / gating condition (condition: null) | 590 | oracle_nom/condition.rs parse_inner_condition — trigger/static parsers must delegate condition extraction here | | 3 | Anaphor bound to wrong referent | 404 | oracle_quantity.rs context-ref resolution + game/ability_utils.rs forward_result wiring | | 4 | Conjoined / chained second effect clause dropped | 387 | oracle.rs effect-chain composition — split on 'and'/'then'/sentence boundaries and build sub_ability chain | @@ -30,7 +30,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top | 16 | Keyword payload / multiplicity / mis-tokenization | 84 | game/keywords.rs + oracle keyword parsing — use typed discriminants and guard ability-word labels | | 17 | Copy 'except' / additional-modification clause dropped | 81 | oracle parser copy handling — populate BecomeCopy/CopyTokenOf additional_modifications from the except-list (CR 707.2) | | 18 | Subtype / type-change modification malformed or dropped | 79 | oracle_util.rs SUBTYPES + parse_enchanted_is_type — register subtypes and emit full type-change set | -| 19 | Perpetual (Alchemy) duration mis-mapped to UntilEndOfTurn | 67 | oracle_nom/duration.rs — add Perpetual duration combinator branch | +| 19 | Perpetual (Alchemy) duration mis-mapped to UntilEndOfTurn | 65 | oracle_nom/duration.rs — add Perpetual duration combinator branch | | 20 | Damage subject/recipient set incomplete | 70 | Effect::DealDamage handling — capture all damage subjects/recipients per CR 120 | | 21 | Token entry flags / keyword / attachment clause dropped | 52 | oracle parser token-description handling — preserve attacking/tapped flags, keyword grants, attach target | | 22 | Attacks-alone / while-saddled combat constraint dropped | 43 | oracle_trigger.rs scan_for_phase / attacks-trigger constraint parsing; add SourceAttackingAlone/MinCoAttackers (attacks-alone remainder); "while saddled" folds into the attack trigger's valid_card at declaration (And { filters: [subject, Typed([IsSaddled])] }, CR 508.1m) — no stored TriggerCondition, no LKI (done) | @@ -47,7 +47,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top ## Full card lists per root cause -### 1. Relative-clause / filter restriction on target dropped (746 cards) +### 1. Relative-clause / filter restriction on target dropped (743 cards) **Signature.** TargetFilter/affected emitted with empty or missing properties; a trailing restrictive clause (type, subtype, color, mana value, zone, combat/temporal/control predicate, exclusion) is silently dropped, over-broadening the filter. @@ -112,7 +112,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Battlefront Krushok - Battlegate Mimic - Beetle-Headed Merchants -- Begin Anew - Behold the Sinister Six! - Benalish Missionary - Bitter Work duplicate? @@ -132,7 +131,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Boxing Ring - Brace for Impact - Brainspoil -- Bramblearmor Brawler - Brassclaw Orcs - Break Out - Brine Seer @@ -412,7 +410,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Kitsune Palliator - Kjeldoran Frostbeast - Klement, Knowledge Acolyte -- Klement, Novice Acolyte - Knight of Dusk - Knight of Valor - Knight of the Mists @@ -4570,7 +4567,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top -### 19. Perpetual (Alchemy) duration mis-mapped to UntilEndOfTurn (55 cards) +### 19. Perpetual (Alchemy) duration mis-mapped to UntilEndOfTurn (53 cards) **Signature.** 'perpetually' grant emitted with UntilEndOfTurn/null instead of a Perpetual duration; modification expires too soon. @@ -4590,7 +4587,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Garruk, Wrath of the Wilds - Gitrog, Horror of Zhava - Goblin Trapfinder -- Grow Old Together - Hardened Bonds - Homarid Warrior - Incessant Provocation @@ -4624,7 +4620,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Teyo, Aegis Adept - The Five Stages of Grief - Thought Rattle -- Thoughtweft's Call - Thrill-Kill Disciple - Timeline Culler - Traumatic Prank