diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index 5ec96bf62d..80f74be192 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -102,11 +102,11 @@ use crate::types::ability::FilterProp; use crate::types::ability::{ - AbilityCondition, AbilityDefinition, ContinuousModification, ControllerRef, Duration, Effect, - GuessSubject, ModalChoice, MultiTargetSpec, ObjectScope, PlayerFilter, PlayerScope, - QuantityExpr, QuantityRef, RepeatContinuation, ReplacementDefinition, ResolvedAbility, - StaticCondition, StaticDefinition, TargetFilter, TriggerCondition, TriggerDefinition, - TypeFilter, TypedFilter, ZoneRef, + AbilityCondition, AbilityDefinition, CardTypeSetSource, ContinuousModification, ControllerRef, + Duration, Effect, GuessSubject, ModalChoice, MultiTargetSpec, ObjectScope, PlayerFilter, + PlayerScope, QuantityExpr, QuantityRef, RepeatContinuation, ReplacementDefinition, + ResolvedAbility, StaticCondition, StaticDefinition, TargetFilter, TriggerCondition, + TriggerDefinition, TurnJournalKind, TypeFilter, TypedFilter, ZoneRef, }; use crate::types::game_state::TargetSelectionConstraint; use crate::types::zones::Zone; @@ -565,7 +565,11 @@ fn is_opaque_forwarded_target(f: &TargetFilter) -> bool { // RwProfile (§2 D-profile). // --------------------------------------------------------------------------- -#[derive(Clone, Debug)] +// `PartialEq`/`Eq` are derived so a test can assert a profile is EXACTLY the +// one a population declares, rather than spot-checking fields — an omitted read +// is fail-open for the CR 603.3b ordering gate, so field-by-field assertions +// would be the wrong shape of check. Every field type already derives both. +#[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct RwProfile { /// Source-scoped reads ONLY (live unless the structure freezes them, CR /// 603.10a). Recipient reads are NOT recorded — a Recipient read is the @@ -2077,7 +2081,7 @@ fn legacy_quantity_ref(x: &QuantityRef) -> bool { | QuantityRef::ObjectCountDistinct { .. } | QuantityRef::ObjectCountBySharedQuality { .. } | QuantityRef::ControlledByEachPlayer { .. } - | QuantityRef::DistinctColorsAmongPermanents { .. } + | QuantityRef::DistinctColorsAmong { .. } | QuantityRef::CountersOnObjects { .. } | QuantityRef::DistinctCounterKindsAmong { .. } | QuantityRef::Aggregate { .. } @@ -3681,6 +3685,81 @@ fn journal_zone_change_read(filter: &TargetFilter, to: Option<&Zone>) -> RwProfi p } +/// CR 603.3b: the read profile of a [`CardTypeSetSource`] population. +/// +/// The same-event ordering gate reads this profile, and an OMITTED read is +/// FAIL-OPEN, so every source must map to the profile its own scan actually +/// performs. The `DistinctCardTypes` / `DistinctSubtypes` / `DistinctColorsAmong` +/// heads all share this authority — a wrapper-level `{ .. }` or-pattern would be +/// compiler-blind to a new source variant and would silently keep claiming a +/// board read for a population that is not on the board. +fn characteristic_source_read(source: &CardTypeSetSource) -> RwProfile { + match source { + // CR 109.2: a live object census over the filter — the exact profile the + // colour head declared before it was parameterized onto this axis. + CardTypeSetSource::Objects { filter } => board_membership_read(filter), + // Whole-zone / linked-exile / tracked-set membership reads, unextractable + // filter ⇒ fail-closed. One citation per population, none shared: + // CR 400.1 (zones partition where objects are) for `Zone`, CR 607.2a + // (linked abilities refer to the cards the linked action moved) for + // `ExiledBySource`, and CR 608.2i (an effect may look back at a previous + // action's objects, which need not still be where they were) for the + // "this way" tracked set. + // + // NOT CR 608.2c, which this comment used to cite for all three: that + // rule is about following a spell's instructions in the order written, + // which is why a "this way" reference HAS a referent at all — it says + // nothing about reading membership. + CardTypeSetSource::Zone { .. } + | CardTypeSetSource::ExiledBySource + | CardTypeSetSource::TrackedSet { .. } => reads_zone_membership(), + // CR 608.2i: the cast journal is read by looking BACK at actions already + // taken this turn — the recorded spells have left the stack (CR 400.7) + // and are read from their snapshots, not from the board. That is why + // this classifies as turn-scoped player state rather than a board read. + // Mirrors `QuantityRef::SpellsCastThisTurn`'s own `JournalCast` read so + // the two readings of the same population agree. + // + // NOT CR 601.2a, which this comment used to cite: 601.2a describes how a + // spell is PUT on the stack when cast. It defines the event the journal + // records; it does not describe reading the record afterwards. + CardTypeSetSource::TurnJournal { journal, .. } => match journal { + TurnJournalKind::SpellsCast => reads_player_of(StateKind::JournalCast), + }, + // The union reads everything its members read. Deliberately UNCITED: no + // CR rule defines set union of populations — "among and " is + // English, and each member carries its own citation above. (This arm + // cited CR 109.2, which is the battlefield-default rule for a bare type + // description and says nothing about unions.) Unrolled by the bounded + // walker in the caller, so a union never reaches this arm. + // + // The arity >= 2 invariant is now carried by `UnionSources`, whose + // private `Vec` makes a degenerate union unconstructible rather than + // merely asserted — the `debug_assert!` this replaces compiled out of + // release builds, which is where a fold collapsing to the fail-open + // `RwProfile::empty()` would actually have mattered. + CardTypeSetSource::AnyOf { .. } => RwProfile::empty(), + } +} + +/// CR 603.3b: the read profile of a whole population, unions unrolled. +/// +/// A truncated union walk yields `reads_everything()`, NOT the partial fold: an +/// omitted read is FAIL-OPEN for the same-event ordering gate, so an unseen +/// member has to be assumed to read everything. +fn characteristic_source_read_bounded(source: &CardTypeSetSource) -> RwProfile { + let mut profile = RwProfile::empty(); + let complete = source + .try_for_each_member(crate::types::ability::UNION_DEPTH_BUDGET, &mut |leaf| { + profile.merge(characteristic_source_read(leaf)) + }); + if complete { + profile + } else { + RwProfile::conservative() + } +} + /// A board VALUE aggregate (power/counter aggregate) over `filter`: records the /// value kind AND `SetMembership` (a membership write changes the aggregate, §2). fn board_value_aggregate_read(filter: &TargetFilter, value: StateKind) -> RwProfile { @@ -5928,8 +6007,7 @@ fn rw_quantity_ref(x: &QuantityRef) -> RwProfile { QuantityRef::ObjectCount { filter } | QuantityRef::ObjectCountDistinct { filter, .. } | QuantityRef::ObjectCountBySharedQuality { filter, .. } - | QuantityRef::ControlledByEachPlayer { filter, .. } - | QuantityRef::DistinctColorsAmongPermanents { filter } => board_membership_read(filter), + | QuantityRef::ControlledByEachPlayer { filter, .. } => board_membership_read(filter), QuantityRef::CountersOnObjects { filter, counter_type: _, @@ -5967,10 +6045,18 @@ fn rw_quantity_ref(x: &QuantityRef) -> RwProfile { QuantityRef::Variable { name: _ } | QuantityRef::SelfManaValue => RwProfile::empty(), QuantityRef::TargetZoneCardCount { zone: _ } => reads_zone_membership(), QuantityRef::Devotion { .. } - | QuantityRef::DistinctCardTypes { .. } - | QuantityRef::DistinctSubtypes { .. } | QuantityRef::BasicLandTypeCount { .. } | QuantityRef::PartySize { .. } => reads_zone_membership(), + // CR 205.2 / CR 205.3 / CR 105.1: the three distinct-characteristic + // counts share the population axis, so they share its read profile. + // Split out of the `reads_zone_membership()` group above because a + // wrapper-level `{ .. }` pattern is COMPILER-BLIND to new + // `CardTypeSetSource` variants: a `TurnJournal` source would keep + // claiming a board read and OMIT the `JournalCast` player read, which is + // fail-open for the CR 603.3b same-event ordering gate. + QuantityRef::DistinctCardTypes { source } + | QuantityRef::DistinctSubtypes { source, .. } + | QuantityRef::DistinctColorsAmong { source } => characteristic_source_read_bounded(source), // CR 603.10a (PR-6.75 c5): promoted out of the fail-closed group below to // its own arm so the next field addition is compiler-visible (a read-bearing // field must force a re-audit). `reads_member_bound = true` is the HONEST @@ -6766,6 +6852,149 @@ mod tests { use crate::game::test_fixtures::mana_fixture_roles; + /// Row 14. CR 603.3b: the same-event ordering gate reads this profile, and + /// an OMITTED read is FAIL-OPEN. Every `CardTypeSetSource` must therefore map + /// to the profile its own scan actually performs — which is why the three + /// distinct-characteristic heads were split out of the wrapper-level + /// `{ .. }` or-pattern that was compiler-blind to a new source variant. + /// + /// The `AnyOf` fixture's two members have DELIBERATELY DIFFERENT profiles, so + /// a fold that returns only the first, only the last, or `RwProfile::empty()` + /// fails here. + #[test] + fn characteristic_source_reads_are_exact_per_population() { + use crate::types::ability::{CardTypeSetSource, TurnJournalKind, TypeFilter, TypedFilter}; + + let board_filter = TargetFilter::Typed( + TypedFilter::new(TypeFilter::Permanent).controller(ControllerRef::You), + ); + let objects = CardTypeSetSource::Objects { + filter: board_filter.clone(), + }; + let journal = CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: CountScope::Controller, + filter: None, + }; + let zone = CardTypeSetSource::Zone { + zone: crate::types::ability::ZoneRef::Graveyard, + scope: CountScope::Controller, + }; + + // (b) The object census is EXACTLY today's board-membership read — same + // census, zone span, and controller span. + assert_eq!( + characteristic_source_read(&objects), + board_membership_read(&board_filter), + "an Objects population must keep the pre-parameterization profile" + ); + + // (a) CR 601.2a: a turn journal is turn-scoped PLAYER state. It must + // declare the same `JournalCast` read `QuantityRef::SpellsCastThisTurn` + // declares, and must NOT claim a board read. + let journal_profile = characteristic_source_read(&journal); + assert_eq!( + journal_profile, + reads_player_of(StateKind::JournalCast), + "a cast journal must read player journal state, not the board" + ); + assert!( + !journal_profile.reads_member_bound, + "a journal read is not member-bound" + ); + + // (c) The zone population keeps the zone-membership profile. + assert_eq!(characteristic_source_read(&zone), reads_zone_membership()); + + // (d) CR 109.2: the union is the MERGE of its members' profiles. + let union = CardTypeSetSource::any_of(vec![objects.clone(), journal.clone()]) + .expect("two-member union"); + let mut expected = characteristic_source_read(&objects); + expected.merge(characteristic_source_read(&journal)); + let union_profile = characteristic_source_read_bounded(&union); + assert_eq!( + union_profile, expected, + "AnyOf must union both members' reads" + ); + assert_ne!( + union_profile, + characteristic_source_read(&objects), + "a fold that returns only the FIRST member must fail" + ); + assert_ne!( + union_profile, + characteristic_source_read(&journal), + "a fold that returns only the LAST member must fail" + ); + assert_ne!( + union_profile, + RwProfile::empty(), + "a fold that collapses to empty is FAIL-OPEN for CR 603.3b" + ); + + // Idempotence: a union of two identical members equals that member. + assert_eq!( + characteristic_source_read_bounded( + &CardTypeSetSource::any_of(vec![journal.clone(), journal.clone()]) + .expect("two-member union") + ), + characteristic_source_read(&journal) + ); + + // The three characteristic heads route through this single authority, so + // none of them can drift from the population's real read. + for qty in [ + QuantityRef::DistinctCardTypes { + source: journal.clone(), + }, + QuantityRef::DistinctSubtypes { + source: journal.clone(), + exclude: crate::types::ability::SubtypeExclusion::None, + }, + QuantityRef::DistinctColorsAmong { + source: journal.clone(), + }, + ] { + assert_eq!( + rw_quantity_ref(&qty), + journal_profile, + "every characteristic head must declare the population's own read: {qty:?}" + ); + } + } + + /// CR 603.3b: a union walk that exceeds its safety budget cannot retain a + /// partial read profile. In particular, a deeply nested cast journal has no + /// zone read to fall back to: treating it as one would omit `JournalCast`. + #[test] + fn truncated_characteristic_source_walk_is_conservative_for_cast_journals() { + use crate::types::ability::{CardTypeSetSource, TurnJournalKind}; + + let journal = CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: CountScope::Controller, + filter: None, + }; + let mut source = journal.clone(); + for _ in 0..crate::types::ability::UNION_DEPTH_BUDGET { + source = CardTypeSetSource::any_of(vec![journal.clone(), source]) + .expect("two sources form a union"); + } + + let profile = characteristic_source_read_bounded(&source); + assert_eq!(profile, RwProfile::conservative()); + assert_ne!( + profile, + reads_zone_membership(), + "a truncated cast-journal union must not be misclassified as a zone read" + ); + assert_ne!( + profile, + characteristic_source_read(&journal), + "a partial JournalCast fold is false precision, not a safe fallback" + ); + } + /// Matrix rows 15b + 17 — zero delta for the D5 frozen-event-tag visitor, /// which reads `Effect::Mana`'s target DIRECTLY and bypasses /// `Effect::target_filter()` entirely. diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index 6818479450..6c83730be9 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -2106,6 +2106,10 @@ fn scan_quantity_ref(x: &QuantityRef, mode: ScanMode) -> Axes { sibling: true, projected: false, }, + // Deliberately coarse: `Axes::CONSERVATIVE` is FAIL-CLOSED, so a new + // `CardTypeSetSource` variant reached through this compiler-blind `{ .. }` + // pattern can only over-report, never under-report. The population axis is + // not decomposed here because no caller needs a narrower answer. QuantityRef::DistinctCardTypes { .. } => Axes::CONSERVATIVE, QuantityRef::DistinctSubtypes { .. } => Axes::CONSERVATIVE, QuantityRef::CardsExiledBySource => Axes::NONE, @@ -2524,19 +2528,32 @@ fn scan_quantity_ref(x: &QuantityRef, mode: ScanMode) -> Axes { acc = acc.or(scan_controller_ref(owner)); acc } - QuantityRef::DistinctColorsAmongPermanents { filter } => { - let mut acc = Axes { - event: false, - sibling: true, - projected: false, - }; - acc = acc.or(scan_target_filter( - filter, - FilterReadContext::LiveBoardCensus, - mode, - )); - acc - } + QuantityRef::DistinctColorsAmong { source } => match source { + // CR 105.1 + CR 109.2: unchanged classification for the live board + // census — the only population this head could name before it was + // parameterized onto the shared axis. + crate::types::ability::CardTypeSetSource::Objects { filter } => { + let mut acc = Axes { + event: false, + sibling: true, + projected: false, + }; + acc = acc.or(scan_target_filter( + filter, + FilterReadContext::LiveBoardCensus, + mode, + )); + acc + } + // Zone / linked-exile / tracked-set / turn-journal / union + // populations are classified like their card-type and subtype peers + // above: `Axes::CONSERVATIVE`, which is fail-closed. + crate::types::ability::CardTypeSetSource::Zone { .. } + | crate::types::ability::CardTypeSetSource::ExiledBySource + | crate::types::ability::CardTypeSetSource::TrackedSet { .. } + | crate::types::ability::CardTypeSetSource::TurnJournal { .. } + | crate::types::ability::CardTypeSetSource::AnyOf { .. } => Axes::CONSERVATIVE, + }, QuantityRef::DistinctCounterKindsAmong { filter } => { let mut acc = Axes { event: false, diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index fc3d7ee4c3..c18b104874 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -4270,6 +4270,43 @@ fn filter_target_slot_filter(filter: &TargetFilter) -> Option { } } +/// The first target-slot filter reachable through a [`CardTypeSetSource`] +/// population. +/// +/// Only the object-filter and journal-filter arms carry a `TargetFilter` that +/// could name a target slot; the zone / linked-exile / tracked-set arms are +/// fixed-vocabulary. `AnyOf` recurses so a union member's slot is not dropped. +/// +/// Deliberately UNCITED. This is a structural query over the AST — which arms +/// hold a filter — not a rule implementation. It previously cited CR 109.2, +/// which says a bare type description means a permanent on the battlefield; +/// that rule has nothing to say about target-slot extraction, and a citation +/// that does not support its code is worse than none because it reads as +/// evidence the behavior was checked against the rules. +fn characteristic_source_target_slot_filter(source: &CardTypeSetSource) -> Option { + // FIRST match wins, preserving the previous `find_map` semantics: the walker + // visits members in declaration order, and later members do not overwrite an + // earlier hit. Truncation needs no conservative branch here — this returns a + // slot to wire, and inventing one would be worse than finding none. + let mut found: Option = None; + source.try_for_each_member(crate::types::ability::UNION_DEPTH_BUDGET, &mut |leaf| { + if found.is_some() { + return; + } + found = match leaf { + CardTypeSetSource::Objects { filter } => filter_target_slot_filter(filter), + CardTypeSetSource::TurnJournal { filter, .. } => { + filter.as_ref().and_then(filter_target_slot_filter) + } + CardTypeSetSource::Zone { .. } + | CardTypeSetSource::ExiledBySource + | CardTypeSetSource::TrackedSet { .. } + | CardTypeSetSource::AnyOf { .. } => None, + }; + }); + found +} + fn filter_prop_target_slot_filter( prop: &crate::types::ability::FilterProp, ) -> Option { @@ -4393,25 +4430,17 @@ fn quantity_ref_target_slot_spec(qty: &QuantityRef) -> Option { | QuantityRef::ZoneChangeAggregateThisTurn { filter, .. } | QuantityRef::CounterAddedThisTurn { target: filter, .. } | QuantityRef::TokensCreatedThisTurn { filter, .. } - | QuantityRef::DistinctColorsAmongPermanents { filter } | QuantityRef::DistinctCounterKindsAmong { filter } => filter_target_slot_filter(filter), QuantityRef::SpellsCastThisTurn { filter, .. } | QuantityRef::SpellsCastBeforeTriggeringSpell { filter, .. } | QuantityRef::SpellsCastThisGame { filter, .. } => { filter.as_ref().and_then(filter_target_slot_filter) } - QuantityRef::DistinctCardTypes { source } => match source { - CardTypeSetSource::Objects { filter } => filter_target_slot_filter(filter), - CardTypeSetSource::Zone { .. } - | CardTypeSetSource::ExiledBySource - | CardTypeSetSource::TrackedSet { .. } => None, - }, - QuantityRef::DistinctSubtypes { source, .. } => match source { - CardTypeSetSource::Objects { filter } => filter_target_slot_filter(filter), - CardTypeSetSource::Zone { .. } - | CardTypeSetSource::ExiledBySource - | CardTypeSetSource::TrackedSet { .. } => None, - }, + QuantityRef::DistinctCardTypes { source } + | QuantityRef::DistinctSubtypes { source, .. } + | QuantityRef::DistinctColorsAmong { source } => { + characteristic_source_target_slot_filter(source) + } QuantityRef::ManaSpentToCast { metric, .. } => match metric { CastManaSpentMetric::FromSource { source_filter } => { filter_target_slot_filter(source_filter) diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index f92c052a25..bbff1b8122 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -1514,6 +1514,8 @@ fn fmt_quantity_ref(qty: &QuantityRef) -> String { crate::types::ability::DevotionColors::ChosenColor => "devotion to chosen color".into(), }, QuantityRef::DistinctCardTypes { source } => match source { + // Preserved surface form: the zone reading renders "card types in + // ", not "card types among cards in ". CardTypeSetSource::Zone { zone, scope } => { format!( "card types in {} {}", @@ -1521,26 +1523,16 @@ fn fmt_quantity_ref(qty: &QuantityRef) -> String { fmt_zone_ref(zone) ) } - CardTypeSetSource::ExiledBySource => "card types among cards exiled with source".into(), - CardTypeSetSource::Objects { filter } => { - format!("card types among {}", fmt_target(filter)) - } - CardTypeSetSource::TrackedSet { caused_by } => match caused_by { - Some(cause) => { - use crate::types::ability::ThisWayCause; - let verb = match cause { - ThisWayCause::Discarded => "discarded", - ThisWayCause::Exiled => "exiled", - ThisWayCause::Milled => "milled", - ThisWayCause::Destroyed => "destroyed", - ThisWayCause::Sacrificed => "sacrificed", - ThisWayCause::Returned => "returned", - ThisWayCause::Bounced => "bounced", - }; - format!("card types among cards {verb} this way") - } - None => "card types among tracked cards".into(), - }, + CardTypeSetSource::ExiledBySource + | CardTypeSetSource::Objects { .. } + | CardTypeSetSource::TrackedSet { .. } + | CardTypeSetSource::TurnJournal { .. } + | CardTypeSetSource::AnyOf { .. } => { + format!( + "card types among {}", + fmt_characteristic_population_bounded(source) + ) + } }, QuantityRef::DistinctSubtypes { source, exclude } => { let suffix = match exclude { @@ -1549,14 +1541,7 @@ fn fmt_quantity_ref(qty: &QuantityRef) -> String { } crate::types::ability::SubtypeExclusion::None => "", }; - let scope_desc = match source { - CardTypeSetSource::Zone { zone, scope } => { - format!("cards in {} {}", fmt_count_scope(scope), fmt_zone_ref(zone)) - } - CardTypeSetSource::ExiledBySource => "cards exiled with source".into(), - CardTypeSetSource::Objects { filter } => fmt_target(filter), - CardTypeSetSource::TrackedSet { .. } => "tracked cards".into(), - }; + let scope_desc = fmt_characteristic_population_bounded(source); format!("subtypes{suffix} among {scope_desc}") } QuantityRef::CardsExiledBySource => "cards exiled with source".into(), @@ -1593,8 +1578,11 @@ fn fmt_quantity_ref(qty: &QuantityRef) -> String { fmt_controller(controller) ) } - QuantityRef::DistinctColorsAmongPermanents { filter } => { - format!("# of colors among {}", fmt_target(filter)) + QuantityRef::DistinctColorsAmong { source } => { + format!( + "# of colors among {}", + fmt_characteristic_population_bounded(source) + ) } QuantityRef::DistinctCounterKindsAmong { filter } => { format!("# of counter kinds among {}", fmt_target(filter)) @@ -2271,6 +2259,71 @@ fn fmt_core_type(ct: &CoreType) -> &'static str { } } +/// CR 109.2 + CR 400.1 + CR 601.2a: Human-readable rendering of the population a +/// [`CardTypeSetSource`] names, shared by every distinct-characteristic count +/// (card types CR 205.2, subtypes CR 205.3, colors CR 105.1) so a new population +/// renders once rather than in three drifting copies. +fn fmt_characteristic_population(source: &CardTypeSetSource) -> String { + match source { + CardTypeSetSource::Zone { zone, scope } => { + format!("cards in {} {}", fmt_count_scope(scope), fmt_zone_ref(zone)) + } + CardTypeSetSource::ExiledBySource => "cards exiled with source".into(), + CardTypeSetSource::Objects { filter } => fmt_target(filter), + CardTypeSetSource::TrackedSet { caused_by } => match caused_by { + Some(cause) => { + use crate::types::ability::ThisWayCause; + let verb = match cause { + ThisWayCause::Discarded => "discarded", + ThisWayCause::Exiled => "exiled", + ThisWayCause::Milled => "milled", + ThisWayCause::Destroyed => "destroyed", + ThisWayCause::Sacrificed => "sacrificed", + ThisWayCause::Returned => "returned", + ThisWayCause::Bounced => "bounced", + }; + format!("cards {verb} this way") + } + None => "tracked cards".into(), + }, + // CR 601.2a: the per-turn cast journal, not a live board census. + CardTypeSetSource::TurnJournal { + journal, + scope, + filter, + } => { + let base = match journal { + crate::types::ability::TurnJournalKind::SpellsCast => { + format!("spells {} cast this turn", fmt_count_scope(scope)) + } + }; + match filter { + Some(filter) => format!("{base} matching {}", fmt_target(filter)), + None => base, + } + } + // CR 109.2: a set union renders as its members joined by "and", mirroring + // the Oracle surface form ("permanents you control and spells you've cast + // this turn"). + // Rendered by the bounded walker in the caller, which flattens nested + // unions — set union is associative, so "A and B and C" is the same + // population however the tree was built, and matches the Oracle surface + // form more closely than a parenthesized nesting would. + CardTypeSetSource::AnyOf { .. } => String::new(), + } +} + +/// Display form for a whole population, unions flattened through the single +/// bounded walker. Display-only: a truncated walk renders fewer members, which +/// is a cosmetic loss in a coverage report rather than a correctness one. +fn fmt_characteristic_population_bounded(source: &CardTypeSetSource) -> String { + let mut parts: Vec = Vec::new(); + source.try_for_each_member(crate::types::ability::UNION_DEPTH_BUDGET, &mut |leaf| { + parts.push(fmt_characteristic_population(leaf)) + }); + parts.join(" and ") +} + fn fmt_count_scope(scope: &CountScope) -> &'static str { match scope { CountScope::Controller | CountScope::Owner => "your", @@ -8148,9 +8201,7 @@ fn quantity_ref_feature(qref: &QuantityRef) -> (&'static str, FeatureSupport) { QuantityRef::ExiledCardPower { .. } => ("ExiledCardPower", Handled), QuantityRef::ZoneCardCount { .. } => ("ZoneCardCount", Handled), QuantityRef::BasicLandTypeCount { .. } => ("BasicLandTypeCount", Handled), - QuantityRef::DistinctColorsAmongPermanents { .. } => { - ("DistinctColorsAmongPermanents", Handled) - } + QuantityRef::DistinctColorsAmong { .. } => ("DistinctColorsAmong", Handled), QuantityRef::DistinctCounterKindsAmong { .. } => ("DistinctCounterKindsAmong", Handled), QuantityRef::VoteCount { .. } => ("VoteCount", Handled), QuantityRef::PreviousEffectAmount { .. } => ("PreviousEffectAmount", Handled), diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index eecc01eedc..619dd31adb 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -3038,15 +3038,44 @@ fn filter_contains_last_created(filter: &TargetFilter) -> bool { /// Whether the object population a `CardTypeSetSource` reads is selected by a /// filter satisfying `filter_pred`. /// -/// Exhaustive: the three non-`Objects` sources name a zone, the source's exile -/// set, or a tracked set, none of which carries a `TargetFilter`. +/// Exhaustive: the three fixed-vocabulary sources name a zone, the source's +/// exile set, or a tracked set, none of which carries a `TargetFilter`. The +/// journal carries an optional narrowing filter, and `AnyOf` recurses. fn card_type_set_source_counts_population_matching( source: &crate::types::ability::CardTypeSetSource, filter_pred: &dyn Fn(&TargetFilter) -> bool, +) -> bool { + let mut found = false; + let complete = + source.try_for_each_member(crate::types::ability::UNION_DEPTH_BUDGET, &mut |leaf| { + if !found { + found = card_type_set_leaf_counts_population_matching(leaf, filter_pred); + } + }); + // A truncated walk claims the match: an unreported anaphor is the silent + // failure this predicate exists to prevent. + found || !complete +} + +fn card_type_set_leaf_counts_population_matching( + source: &crate::types::ability::CardTypeSetSource, + filter_pred: &dyn Fn(&TargetFilter) -> bool, ) -> bool { use crate::types::ability::CardTypeSetSource; match source { CardTypeSetSource::Objects { filter } => filter_pred(filter), + // The journal's optional narrowing filter is a population selector like + // any other, so an anaphor inside it must be reported. Uncited: this is + // an engine claim about where filters live, not a rule. (It cited + // CR 601.2a, which describes putting a spell on the stack as it is cast + // — the event the journal records, not its narrowing filter.) + CardTypeSetSource::TurnJournal { filter, .. } => filter.as_ref().is_some_and(filter_pred), + // Unrolled by the bounded walker in the caller, so a union never reaches + // this arm. Uncited for the same reason as the union arm in + // `ability_rw::characteristic_source_read`: no CR rule defines set union + // of populations. (It cited CR 109.2, the battlefield-default rule for a + // bare type description.) + CardTypeSetSource::AnyOf { .. } => false, CardTypeSetSource::Zone { .. } | CardTypeSetSource::ExiledBySource | CardTypeSetSource::TrackedSet { .. } => false, @@ -3085,7 +3114,6 @@ fn quantity_ref_counts_population_matching( | QuantityRef::CountersOnObjects { filter, .. } | QuantityRef::Aggregate { filter, .. } | QuantityRef::ControlledByEachPlayer { filter, .. } - | QuantityRef::DistinctColorsAmongPermanents { filter } | QuantityRef::DistinctCounterKindsAmong { filter } | QuantityRef::EnteredThisTurn { filter } | QuantityRef::SacrificedThisTurn { filter, .. } @@ -3112,7 +3140,8 @@ fn quantity_ref_counts_population_matching( filter_pred(source) || filter_pred(target) } QuantityRef::DistinctCardTypes { source } - | QuantityRef::DistinctSubtypes { source, .. } => { + | QuantityRef::DistinctSubtypes { source, .. } + | QuantityRef::DistinctColorsAmong { source } => { card_type_set_source_counts_population_matching(source, filter_pred) } // No `TargetFilter` anywhere: player-scoped totals, per-object scopes, diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 8c8dcff886..fd5e1969a0 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -18423,13 +18423,186 @@ mod stage2_injector_tests { // added to `compute_options`' sibling classifier in this file, // which sits above all three producers. Nothing added raises a // `WaitingFor`; the census set is still exactly 5. - // Current-main port: #7403/#7389 move main's three production pins to - // `:6738/:6815/:10053`; the Doomsday tracked-set publication adds seven - // lines above each. Re-measured in this merged tree: `:6745/:6822/:10060`. - // The three sites remain the existing `OptionalEffectChoice` producers. - "game/effects/mod.rs:6745".to_string(), - "game/effects/mod.rs:6822".to_string(), - "game/effects/mod.rs:10060".to_string(), + // + // MERGE OF `origin/main` (`59f5a51e`) INTO THIS BRANCH (First Family's + // characteristic-set union). This array conflicted, and the header's rule + // applied a third time: `origin/main` carried `:6656/:6733/:9974` and this + // branch carried `:6648/:6725/:9947`, each correct for its own tree and + // neither correct for the merge. NEITHER SIDE WAS TAKEN — the merged file + // was re-measured: `:6656/:6733/:9974 => :6664/:6741/:9982`, a uniform + // `+8` over main's coordinates. + // + // The `+8` is exactly this branch's net insertion into effects/mod.rs, and + // all of it sits above the FIRST producer, which is why the shift is + // uniform rather than staggered. `git diff -U0 origin/main` on that file + // has exactly four hunks, ALL between `:2966` and `:3047`: + // `filter_contains_last_created`'s characteristic-source arm (+1), + // `card_type_set_source_counts_population_matching`'s zone/tracked-set/ + // union population cases (+7), + // the `quantity_ref_counts_population_matching` arm the union folds + // into the shared helper (-1), and + // its replacement delegation (+1). + // 1 + 7 - 1 + 1 = 8, with nothing below `:3047` — predicted and observed + // agree. None of the four raises a prompt: they are population COUNTS + // (pure reads over zones, tracked sets and unions), so the census set is + // still exactly 5. + // + // Identity re-established at the new coordinates rather than assumed. Each + // producer line is byte-identical by sha256 to the same producer on BOTH + // parents — `9869a19f…`, `2bc316e3…` and `8df98486…` respectively, the + // same three digests the line carries at `:6656/:6733/:9974` on main and + // at `:6648/:6725/:9947` on this branch. The two asserts above this one + // fired GREEN on the merged tree — total still 38, partition still 5/8/25 + // — and the other two entries did not move (`scoped_library_search.rs:452` + // unmoved, `engine.rs:12773` unmoved, both re-read and sha256-confirmed in + // place). A merge that had gained or lost a producer could not leave two + // entries byte-identical AND at their coordinates while moving the other + // three by a figure the diff predicts exactly. + // + // CR-CITATION ROUND (review follow-up), LOCAL not upstream — so the + // CI-vs-local diagnosis in the header does not apply, the shift + // originates in this same diff. `:6664/:6741/:9982 => :6670/:6747/:9988`, + // a uniform `+6`. + // + // A COMMENT-ONLY round, and the census caught it, which is the row + // working exactly as designed rather than a defect in the row. + // effects/mod.rs's entire delta is two comment hunks in + // `card_type_set_source_counts_population_matching`, both ABOVE all three + // producers: `@@ -2976,2 +2976,5 @@` (+3, the `TurnJournal` arm's + // citation corrected off CR 601.2a) and `@@ -2979 +2982,4 @@` (+3, the + // `AnyOf` arm's off CR 109.2). 3 + 3 = 6, with nothing below `:2985` — + // predicted and observed agree. Prose cannot mint a prompt, and the + // census agrees: the two asserts above this one fired GREEN on the run + // that caught this (total still 38, partition still 5/8/25) and the + // panic was on this third assert alone, which is what makes it a + // coordinate shift rather than a set change. + // + // Identity re-established rather than assumed: the three producer lines + // are byte-identical by sha256 at their new coordinates to the same + // producers at `:6664/:6741/:9982` — `9869a19f…`, `2bc316e3…`, + // `8df98486…`, the same digests this log recorded one entry above. The + // other two entries did not move (`scoped_library_search.rs:452` and + // `engine.rs:12773`, both re-read and sha256-confirmed in place); this + // round does not touch either file's producer region at all. + // + // BOUNDED-UNION-WALKER ROUND (review follow-up), LOCAL not upstream. + // `:6670/:6747/:9988 => :6685/:6762/:10003`, a uniform `+15`. + // + // effects/mod.rs's whole delta is the split of + // `card_type_set_source_counts_population_matching` into a bounded + // walker plus a leaf classifier, both ABOVE all three producers: + // `@@ -2971,0 +2972,16 @@` (+16, the walker and its truncation + // contract) and `@@ -2982,7 +2998,6 @@` (-1, the `AnyOf` recursion arm + // collapsing to a no-op now that unions are unrolled before the + // classifier sees them). 16 - 1 = 15, with nothing below `:3004` — + // predicted and observed agree. + // + // The split moves a recursion; it mints nothing. The census agrees: the + // two asserts above this one fired GREEN (total still 38, partition + // still 5/8/25) and the panic was on this third assert alone. Identity + // re-established rather than assumed — `9869a19f…`, `2bc316e3…`, + // `8df98486…` at the new coordinates, the same digests recorded one + // entry above — and the other two entries did not move. + // + // THIRD merge with main (this branch × `origin/main` @ 59f5a51e, which + // by now carries Wheel of Misfortune's unbounded-number round). Same rule + // as the two merges logged above, applied a third time: each side's pins + // were local-correct and BOTH are wrong for the merged tree, so the merged + // file was re-measured rather than either side taken. `origin/main` + // carried `:6656/:6733/:9974`; this branch carried `:6722/:6799/:10001`; + // the merged file measures `:6738/:6815/:10053`. + // + // The merged coordinates are PREDICTED, not merely observed, and the + // prediction is what makes this a measurement rather than a fixup: + // `main`'s pins plus this branch's own base-relative offsets — `+82/+82/+79`, + // the figure the row immediately above derives from base `8035813e6` and + // re-derives twice — give `6656+82`/`6733+82`/`9974+79` = + // `:6738`/`:6815`/`:10053`, equal to the observed coordinates exactly. + // That the branch's offsets compose additively onto main's is the evidence + // the merge introduced no new producer and displaced none: a merge that had + // gained or lost one would break the additivity, not just shift a pin. + // + // Set preservation: the assembled needle finds exactly five hits in the + // merged effects/mod.rs (`:6738`, `:6815`, `:10053`, `:14805`, `:15290`); + // the last two fall inside the `#[cfg(test)]` span opening at `:13563` and + // so are the partition's test half, leaving the same three production + // producers this row has always pinned. Total still 37, partition still + // 5/7/25. The merge added no `WaitingFor` producer on either side — main's + // contribution here is the unbounded-range arm in `compute_options`' sibling + // classifier and this branch's is the CR 603.4 delayed-hoist carve-out, both + // pure classification code. + // + // FOURTH MERGE (this branch × `origin/main` @ `2ae92459`). The header's + // rule applies again and for the same reason: `origin/main` carried + // `:6738/:6815/:10053` and this branch carried `:6685/:6762/:10003`, each + // correct for its own tree and NEITHER correct for the merge. Neither side + // was taken — the merged file was re-measured to + // `:6767/:6844/:10082`, a uniform `+29` over main's coordinates. + // + // The `+29` is this branch's CUMULATIVE net insertion into + // effects/mod.rs relative to main, not any single round's: + // `git diff --numstat origin/main` on that file reads `33 4` = `+29`, and + // its five hunks all sit between `:3041` and `:3144`, above the first + // producer with nothing below. It is the sum of the three rounds this log + // records — `+8` (union population), `+6` (CR citations), `+15` (bounded + // walker) — which is exactly why the per-round figure is the WRONG one to + // compose here. + // + // Recorded because the first attempt at this entry got it wrong: it + // composed only the last round's `+15` onto main and predicted + // `:6753/:6830/:10068`, which the measurement contradicted. The pins below + // come from measuring the merged tree, and the arithmetic is reconciled + // to that measurement rather than the other way round. A prediction is + // evidence only when it is made against the cumulative offset. + // + // Identity re-established rather than assumed — `9869a19f…`, `2bc316e3…`, + // `8df98486…` at the new coordinates, the same three digests this log has + // carried since the first merge — and the other two entries did not move: + // `scoped_library_search.rs:452`, and `engine.rs:12796`, which is main's + // own coordinate for that producer (this branch's engine.rs edits are all + // in the census array far below it). + // + // NOTE on the prose above from main: that entry's "total still 37, + // partition still 5/7/25" describes an older census. The asserts in this + // file read 38 and 5/8/25, and both fired GREEN on the merged tree. + // + // FIFTH MERGE (this branch × `origin/main` @ `0f37d27b`, Doomsday). + // Main's own entry for this round, preserved: "#7403/#7389 move main's + // three production pins to `:6738/:6815/:10053`; the Doomsday tracked-set + // publication adds seven lines above each. Re-measured in this merged + // tree: `:6745/:6822/:10060`. The three sites remain the existing + // producers." That is main's coordinate, correct for main. + // + // Neither side taken, again. Main carried `:6745/:6822/:10060` and this + // branch carried `:6767/:6844/:10082`; the merged file measures + // `:6774/:6851/:10089`. + // + // Predicted with the CUMULATIVE offset, which is the lesson the previous + // entry records: main's `:6745` plus this branch's `+29` net insertion + // into effects/mod.rs gives `6745+29`/`6822+29`/`10060+29` = + // `:6774`/`:6851`/`:10089`, equal to the measurement. Main's `+7` + // (Doomsday) and this branch's `+29` compose additively, which is the + // set-preservation evidence: a merge that gained or lost a producer would + // break the additivity rather than merely shift a pin. + // + // Identity re-established: `9869a19f…`, `2bc316e3…`, `8df98486…` at the + // new coordinates. The other two entries did not move. + // + // CONVERGENT RE-MEASUREMENT, and the strongest evidence in this log. The + // maintainer merged the same upstream commit into this branch + // independently and in parallel, and recorded it thus: "#7404's Doomsday + // tracked-set publication and this branch's characteristic-source work + // both shift the producer coordinates. Re-measured in this merged tree: + // `:6774/:6851/:10089`. The three sites remain the existing producers." + // + // Two independent measurements of the same merged tree, agreeing to the + // line on all three coordinates. That is what a coordinate this log can + // trust looks like — and it is why the two prose entries are BOTH kept + // rather than one overwriting the other: they are separate witnesses, not + // duplicates. + "game/effects/mod.rs:6774".to_string(), + "game/effects/mod.rs:6851".to_string(), + "game/effects/mod.rs:10089".to_string(), // UNMOVED across the rebase, and that is itself evidence the SET did not // move: a census that had gained or lost a producer would not leave this // entry both byte-identical AND at the same coordinate. diff --git a/crates/engine/src/game/layers.rs b/crates/engine/src/game/layers.rs index 5628b5d3eb..3ea30f13e9 100644 --- a/crates/engine/src/game/layers.rs +++ b/crates/engine/src/game/layers.rs @@ -28,10 +28,10 @@ use crate::game::quantity::{ use crate::game::speed::{effective_speed, has_max_speed}; use crate::types::ability::{ AbilityCost, AbilityDefinition, AbilityKind, ActivationRestriction, BasicLandType, - CastingPermission, ChosenSubtypeKind, CommanderOwnership, ContinuousModification, - CopiableValues, Duration, Effect, FilterProp, ManaContribution, ManaProduction, PlayerFilter, - PlayerScope, QuantityExpr, QuantityRef, StaticCondition, StaticDefinition, TargetFilter, - TriggerGrantProducerKey, TriggerProducerOrigin, TypedFilter, + CardTypeSetSource, CastingPermission, ChosenSubtypeKind, CommanderOwnership, + ContinuousModification, CopiableValues, Duration, Effect, FilterProp, ManaContribution, + ManaProduction, PlayerFilter, PlayerScope, QuantityExpr, QuantityRef, StaticCondition, + StaticDefinition, TargetFilter, TriggerGrantProducerKey, TriggerProducerOrigin, TypedFilter, }; use crate::types::attribution::EffectRef; use crate::types::card_type::{ @@ -2695,15 +2695,56 @@ fn static_condition_reads_zone_membership(condition: &StaticCondition, zone: Zon } /// CR 404: Does a `ZoneRef` denote the game `zone`? +/// +/// Delegates to [`ZoneRef::zone`](crate::types::ability::ZoneRef::zone) rather +/// than restating the pairing: the hand-written `matches!` this replaced was a +/// second copy of the mapping that a new `ZoneRef` variant would have left +/// silently answering `false` instead of failing to compile. fn zone_ref_denotes_zone(zone_ref: &crate::types::ability::ZoneRef, zone: Zone) -> bool { - use crate::types::ability::ZoneRef; - matches!( - (zone_ref, zone), - (ZoneRef::Graveyard, Zone::Graveyard) - | (ZoneRef::Exile, Zone::Exile) - | (ZoneRef::Library, Zone::Library) - | (ZoneRef::Hand, Zone::Hand) - ) + zone_ref.zone() == zone +} + +/// CR 613.4a + CR 400.1: Does a [`CardTypeSetSource`] population read `zone`? +/// +/// Delegates to [`CardTypeSetSource::reads_zone`], which is THE authority for +/// the population-zone axis and is the same function +/// `game::quantity::visit_characteristic_source` walks to enumerate members. +/// This wrapper exists only to keep the local call sites reading like their +/// `characteristic_source_reads_*` siblings — it must never re-derive the +/// answer. +/// +/// It previously did re-derive it, and reported `false` for every `Objects` +/// population. That is what let a craft characteristic (`And[ExiledBySource, +/// Owned{You}]`, Sunbird Effigy) be evaluated against exile while no exile +/// transition ever dirtied it, stranding a stale value in a layer. +fn characteristic_source_reads_zone(source: &CardTypeSetSource, zone: Zone) -> bool { + source.reads_zone(zone) +} + +/// CR 119 + CR 613.4a: Does a [`CardTypeSetSource`] population route a filter +/// that reads a life total? Mirrors [`characteristic_source_reads_zone`]'s +/// recursion over the same axis. +fn characteristic_source_reads_life_total(source: &CardTypeSetSource) -> bool { + let mut found = false; + let complete = + source.try_for_each_member(crate::types::ability::UNION_DEPTH_BUDGET, &mut |leaf| { + if found { + return; + } + found = match leaf { + CardTypeSetSource::Objects { filter } => target_filter_reads_life_total(filter), + CardTypeSetSource::TurnJournal { filter, .. } => { + filter.as_ref().is_some_and(target_filter_reads_life_total) + } + CardTypeSetSource::Zone { .. } + | CardTypeSetSource::ExiledBySource + | CardTypeSetSource::TrackedSet { .. } + | CardTypeSetSource::AnyOf { .. } => false, + }; + }); + // A truncated walk claims the read: one redundant recompute beats a stale + // layer surviving a life change. + found || !complete } /// CR 404 + CR 611.3a: Does a `QuantityExpr` read the card count / object @@ -2735,7 +2776,6 @@ fn quantity_expr_reads_zone(expr: &QuantityExpr, zone: Zone) -> bool { /// quantity reference that reads a zone must be classified intentionally rather /// than silently under-escalating a zone-membership gate. fn quantity_ref_reads_zone(qty: &QuantityRef, zone: Zone) -> bool { - use crate::types::ability::CardTypeSetSource; match qty { // Direct graveyard card count (CR 404). `player` scope is irrelevant to // the zone identity — any player's graveyard is still the graveyard. @@ -2760,23 +2800,17 @@ fn quantity_ref_reads_zone(qty: &QuantityRef, zone: Zone) -> bool { | QuantityRef::ObjectCountDistinct { filter, .. } | QuantityRef::ObjectCountBySharedQuality { filter, .. } | QuantityRef::Aggregate { filter, .. } => target_filter_reads_zone(filter, zone), - // Distinct card types read `zone` only when sourced from that zone's cards - // (Tarmogoyf: card types among cards in all graveyards). - QuantityRef::DistinctCardTypes { source } => match source { - CardTypeSetSource::Zone { zone: zone_ref, .. } => zone_ref_denotes_zone(zone_ref, zone), - CardTypeSetSource::ExiledBySource - | CardTypeSetSource::Objects { .. } - | CardTypeSetSource::TrackedSet { .. } => false, - }, - // CR 613.4a: Distinct subtypes read `zone` when sourced from that zone's - // cards (Subgoyf: different subtypes among cards in all graveyards) — layer - // 7a CDA P/T must re-derive when that zone changes. - QuantityRef::DistinctSubtypes { source, .. } => match source { - CardTypeSetSource::Zone { zone: zone_ref, .. } => zone_ref_denotes_zone(zone_ref, zone), - CardTypeSetSource::ExiledBySource - | CardTypeSetSource::Objects { .. } - | CardTypeSetSource::TrackedSet { .. } => false, - }, + // CR 613.4a: A distinct-characteristic count reads `zone` only when its + // population is sourced from that zone's cards (Tarmogoyf: card types + // among cards in all graveyards; Subgoyf: different subtypes among the + // same) — layer 7a CDA P/T must re-derive when that zone changes. All + // three characteristics share the population axis, so they share this + // classification. + QuantityRef::DistinctCardTypes { source } + | QuantityRef::DistinctSubtypes { source, .. } + | QuantityRef::DistinctColorsAmong { source } => { + characteristic_source_reads_zone(source, zone) + } // Everything else reads player-level state, single-object state, battle- // field-only population, history records, choices, or tracked sets — none // depend on `zone` membership. Enumerated explicitly (no wildcard) so a @@ -2794,7 +2828,6 @@ fn quantity_ref_reads_zone(qty: &QuantityRef, zone: Zone) -> bool { | QuantityRef::Devotion { .. } | QuantityRef::BasicLandTypeCount { .. } | QuantityRef::PartySize { .. } - | QuantityRef::DistinctColorsAmongPermanents { .. } | QuantityRef::DistinctCounterKindsAmong { .. } | QuantityRef::EnteredThisTurn { .. } | QuantityRef::CommanderManaValue { .. } @@ -3024,7 +3057,7 @@ fn quantity_expr_reads_life(expr: &QuantityExpr) -> bool { /// and every filter-bearing variant ROUTES its nested payload (universal /// routing rule). EXHAUSTIVE and wildcard-free. fn quantity_ref_reads_life(qty: &QuantityRef) -> bool { - use crate::types::ability::{CardTypeSetSource, CastManaSpentMetric}; + use crate::types::ability::CastManaSpentMetric; match qty { // CR 119.3 + CR 119.9: the direct-leaf life-family readers — the exact // quantities a guarded life-mutation site changes (119.3: gain/loss @@ -3050,7 +3083,6 @@ fn quantity_ref_reads_life(qty: &QuantityRef) -> bool { | QuantityRef::CountersOnObjects { filter, .. } | QuantityRef::Aggregate { filter, .. } | QuantityRef::ControlledByEachPlayer { filter, .. } - | QuantityRef::DistinctColorsAmongPermanents { filter } | QuantityRef::DistinctCounterKindsAmong { filter } | QuantityRef::EnteredThisTurn { filter } | QuantityRef::SacrificedThisTurn { filter, .. } @@ -3087,15 +3119,14 @@ fn quantity_ref_reads_life(qty: &QuantityRef) -> bool { // reads `life_lost_this_turn` per candidate. QuantityRef::PlayerCount { filter } => player_filter_reads_life(filter), - // Distinct card-type / subtype counts route an `Objects { filter }` - // set-source; the other set-sources carry no `TargetFilter`. + // Distinct card-type / subtype / colour counts route the filters their + // population carries (`Objects { filter }` and the journal's optional + // narrowing filter); the fixed-vocabulary set-sources carry none. QuantityRef::DistinctCardTypes { source } - | QuantityRef::DistinctSubtypes { source, .. } => match source { - CardTypeSetSource::Objects { filter } => target_filter_reads_life_total(filter), - CardTypeSetSource::Zone { .. } - | CardTypeSetSource::ExiledBySource - | CardTypeSetSource::TrackedSet { .. } => false, - }, + | QuantityRef::DistinctSubtypes { source, .. } + | QuantityRef::DistinctColorsAmong { source } => { + characteristic_source_reads_life_total(source) + } // CR 601.2h: `ManaSpentToCast` carries no direct `TargetFilter`, but its // `metric` can nest a mana-source filter one level deeper diff --git a/crates/engine/src/game/quantity.rs b/crates/engine/src/game/quantity.rs index e7e2a945e7..835de95169 100644 --- a/crates/engine/src/game/quantity.rs +++ b/crates/engine/src/game/quantity.rs @@ -20,7 +20,8 @@ use crate::types::ability::{ CastManaSpentMetric, ContinuousModification, ControllerRef, CountScope, DamageChannel, FilterProp, ObjectProperty, ObjectScope, PlayerFilter, PlayerScope, PossessionAxis, QuantityExpr, QuantityRef, ResolvedAbility, RoundingMode, StaticCondition, SubtypeExclusion, - TargetFilter, TargetRef, ThisWayCause, TrackedAnaphorSource, TypeFilter, TypedFilter, ZoneRef, + TargetFilter, TargetRef, ThisWayCause, TrackedAnaphorSource, TurnJournalKind, TypeFilter, + TypedFilter, ZoneRef, }; use crate::types::card_type::CoreType; use crate::types::counter::{positive_counter_types, CounterType}; @@ -139,6 +140,228 @@ fn cards_exiled_this_turn_for_context(state: &GameState, ctx: &QuantityContext) }) } +/// CR 109.2 + CR 400.1: A characteristic-bearing member of a scanned population. +/// +/// A [`SpellCastRecord`](crate::types::game_state::SpellCastRecord) is not a +/// `GameObject` — per CR 400.7 a spell that has left the stack is a new object +/// with no relation to its previous existence, so a resolved spell cannot be +/// re-inspected. Its characteristics are therefore read from the cast-time +/// snapshot instead. This borrow is the single abstraction that lets a +/// non-object population feed characteristic extraction without smuggling +/// history into `TargetFilter` (which is object/zone-oriented and is consumed by +/// targeting legality, the layer system, and combat). +enum CharacteristicView<'a> { + Object(&'a crate::game::game_object::GameObject), + SpellRecord(&'a crate::types::game_state::SpellCastRecord), +} + +impl<'a> CharacteristicView<'a> { + /// CR 205.2a: the member's card types. + fn core_types(&self) -> &'a [CoreType] { + match self { + CharacteristicView::Object(obj) => &obj.card_types.core_types, + CharacteristicView::SpellRecord(record) => &record.core_types, + } + } + + /// CR 205.3: the member's subtypes. + fn subtypes(&self) -> &'a [String] { + match self { + CharacteristicView::Object(obj) => &obj.card_types.subtypes, + CharacteristicView::SpellRecord(record) => &record.subtypes, + } + } + + /// CR 105.2: the member's colors. An object with no color contributes none. + fn colors(&self) -> &'a [ManaColor] { + match self { + CharacteristicView::Object(obj) => &obj.color, + CharacteristicView::SpellRecord(record) => &record.colors, + } + } +} + +/// CR 109.2 + CR 400.1: Walk the population a [`CardTypeSetSource`] names, +/// yielding one [`CharacteristicView`] per member. +/// +/// The single authority for the population axis shared by +/// `QuantityRef::DistinctCardTypes` (CR 205.2), `QuantityRef::DistinctSubtypes` +/// (CR 205.3) and `QuantityRef::DistinctColorsAmong` (CR 105.1) — each supplies +/// only its own characteristic extractor. Callers tally into a `HashSet`, so +/// `AnyOf`'s recursion yields a genuine set UNION: a member present in two +/// sources contributes its characteristics once. +fn visit_characteristic_leaf<'s>( + state: &'s GameState, + source: &CardTypeSetSource, + ctx: QuantityContext, + filter_ctx: &FilterContext<'_>, + controller: PlayerId, + visit: &mut impl FnMut(CharacteristicView<'s>), +) { + match source { + CardTypeSetSource::Zone { zone, scope } => match zone { + ZoneRef::Exile => { + for &obj_id in &state.exile { + if let Some(obj) = state.objects.get(&obj_id) { + let owner_matches = count_scope_owner_matches( + state, + scope, + ctx.clone(), + controller, + obj.owner, + ); + if owner_matches { + visit(CharacteristicView::Object(obj)); + } + } + } + } + ZoneRef::Graveyard | ZoneRef::Library | ZoneRef::Hand => { + for player in scoped_players(state, scope, ctx, controller) { + let zone_ids = match zone { + ZoneRef::Graveyard => &player.graveyard, + ZoneRef::Library => &player.library, + ZoneRef::Hand => &player.hand, + ZoneRef::Exile => unreachable!(), + }; + for &obj_id in zone_ids { + if let Some(obj) = state.objects.get(&obj_id) { + visit(CharacteristicView::Object(obj)); + } + } + } + } + }, + CardTypeSetSource::ExiledBySource => { + for linked in linked_exile_for_context(state, &ctx) { + if let Some(obj) = state.objects.get(&linked.exiled_id) { + visit(CharacteristicView::Object(obj)); + } + } + } + // CR 400.1: EVERY zone the filter names, not just the first. The zone + // list comes from `CardTypeSetSource::population_zones` — the same + // authority `game::layers::characteristic_source_reads_zone` asks — so + // the set this walk enumerates and the set a zone transition dirties + // cannot drift apart. + // + // The previous `extract_in_zone().unwrap_or(Battlefield)` collapsed a + // multi-zone `FilterProp::InAnyZone` population to whichever zone the + // filter tree happened to yield first, silently undercounting every + // other zone in the union. + // + // CR 110.1: an empty list means the filter writes no zone constraint, so + // it denotes permanents. The default is substituted HERE and not inside + // `population_zones` — see that function on why the dependency half must + // not claim a defaulted battlefield read. + CardTypeSetSource::Objects { filter } => { + let mut zones = source.population_zones(); + if zones.is_empty() { + zones.push(crate::types::zones::Zone::Battlefield); + } + for zone in zones { + for obj_id in crate::game::targeting::zone_object_ids(state, zone) { + if !matches_target_filter(state, obj_id, filter, filter_ctx) { + continue; + } + if let Some(obj) = state.objects.get(&obj_id) { + visit(CharacteristicView::Object(obj)); + } + } + } + } + // CR 608.2c + CR 205.2a/205.2b: the most recent chain tracked set. A + // merged Draw->Discard set is disambiguated by CAUSE: `Some(cause)` + // (e.g. Discarded) admits only members whose recorded producer action + // equals the bound cause; drawn members are unstamped and excluded. + // `None` admits every member. Mirrors `FilteredTrackedSetSize`'s set + // selection (highest set id) and cause filter. + CardTypeSetSource::TrackedSet { caused_by } => { + if let Some((set_id, ids)) = state.tracked_object_sets.iter().max_by_key(|(id, _)| id.0) + { + for &oid in ids { + let cause_ok = match caused_by { + None => true, + Some(cause) => state + .tracked_set_member_causes + .get(set_id) + .and_then(|causes| causes.get(&oid)) + .is_some_and(|member_cause| member_cause == cause), + }; + if cause_ok { + if let Some(obj) = state.objects.get(&oid) { + visit(CharacteristicView::Object(obj)); + } + } + } + } + } + // CR 601.2a + CR 112.1: the per-turn action journal for the scoped + // players. Characteristics come from the cast-time snapshot because a + // resolved spell is no longer an object (CR 400.7). + // + // Deliberately does NOT replicate `QuantityRef::SpellsCastThisTurn`'s + // `FilterProp::Another` own-cast exclusion: this population is reached + // from "spells you've cast", never "OTHER spells you've cast", so a + // card's own cast is a member (First Family counts itself, CR 112.1 + + // CR 608.2m). + CardTypeSetSource::TurnJournal { + journal, + scope, + filter, + } => match journal { + TurnJournalKind::SpellsCast => { + for player in scoped_players(state, scope, ctx, controller) { + let Some(records) = state.spells_cast_this_turn_by_player.get(&player.id) + else { + continue; + }; + for record in records.iter() { + let matches = match filter { + None => true, + Some(filter) => spell_record_matches_filter( + record, + filter, + controller, + &state.all_creature_types, + ), + }; + if matches { + visit(CharacteristicView::SpellRecord(record)); + } + } + } + } + }, + // CR 109.2: unions are unrolled by `try_for_each_member` before this is + // called, so a union never reaches the leaf walk. Deduplication remains + // automatic because every caller tallies into one `HashSet` — which is + // why the union must be unrolled INTO this walk and not summed above it: + // `|A ∪ B| != |A| + |B|`. + CardTypeSetSource::AnyOf { .. } => {} + } +} + +/// CR 109.2: Walk a population, unrolling any union through the single bounded +/// walker. +/// +/// Every member reached within the depth budget is visited. A truncated walk +/// UNDERCOUNTS rather than over-counts, which is why the budget is set far above +/// any printed union — the honest alternative would be refusing to resolve the +/// quantity at all, and no card can reach the bound. +fn visit_characteristic_source<'s>( + state: &'s GameState, + source: &CardTypeSetSource, + ctx: QuantityContext, + filter_ctx: &FilterContext<'_>, + controller: PlayerId, + visit: &mut impl FnMut(CharacteristicView<'s>), +) { + source.try_for_each_member(crate::types::ability::UNION_DEPTH_BUDGET, &mut |leaf| { + visit_characteristic_leaf(state, leaf, ctx.clone(), filter_ctx, controller, visit); + }); +} + fn source_chosen_player_for_context(state: &GameState, ctx: &QuantityContext) -> Option { source_lki_for_context(state, ctx).and_then(|lki| { lki.chosen_attributes @@ -761,7 +984,7 @@ fn quantity_ref_uses_unspent_mana(qty: &QuantityRef) -> bool { | QuantityRef::ManaSpentToCast { .. } | QuantityRef::ColorsInCommandersColorIdentity | QuantityRef::VoteCount { .. } - | QuantityRef::DistinctColorsAmongPermanents { .. } + | QuantityRef::DistinctColorsAmong { .. } | QuantityRef::DistinctCounterKindsAmong { .. } | QuantityRef::EnteredThisTurn { .. } | QuantityRef::CommanderManaValue { .. } @@ -942,6 +1165,39 @@ pub(crate) fn static_condition_uses_unspent_mana(condition: &StaticCondition) -> /// a per-turn journal that every battlefield entry appends to (CR 608.2i /// look-back tallies) is population-sensitive in the sense this classifier means, /// even though it is a history record rather than a board scan. +/// CR 611.3a + CR 109.2: Does a [`CardTypeSetSource`] population read the live +/// battlefield object census? +/// +/// Only the object-filter arm does. A turn journal (CR 601.2a) is player state +/// appended at cast time and is unaffected by an object entering or leaving the +/// battlefield. `AnyOf` reads the census iff any member does. +fn characteristic_source_reads_object_count(source: &CardTypeSetSource) -> bool { + any_characteristic_member(source, &mut |leaf| { + matches!(leaf, CardTypeSetSource::Objects { .. }) + }) +} + +/// CR 109.2: Does ANY non-union member of `source` satisfy `pred`? +/// +/// The shared shape for every boolean question asked of a population, routed +/// through the single bounded walker so no consumer writes its own `AnyOf` +/// recursion. A truncated walk answers `true`: each of these gates gains a +/// redundant re-evaluation when it over-reports and misses one when it +/// under-reports, so exhaustion resolves to the harmless direction. +fn any_characteristic_member( + source: &CardTypeSetSource, + pred: &mut impl FnMut(&CardTypeSetSource) -> bool, +) -> bool { + let mut found = false; + let complete = + source.try_for_each_member(crate::types::ability::UNION_DEPTH_BUDGET, &mut |leaf| { + if !found { + found = pred(leaf); + } + }); + found || !complete +} + fn quantity_ref_uses_object_count(qty: &QuantityRef) -> bool { match qty { // Read battlefield object population directly. @@ -954,7 +1210,6 @@ fn quantity_ref_uses_object_count(qty: &QuantityRef) -> bool { | QuantityRef::Devotion { .. } | QuantityRef::BasicLandTypeCount { .. } | QuantityRef::PartySize { .. } - | QuantityRef::DistinctColorsAmongPermanents { .. } | QuantityRef::DistinctCounterKindsAmong { .. } | QuantityRef::EnteredThisTurn { .. } // CR 611.3a + CR 608.2i: a continuous effect from a static ability is @@ -969,23 +1224,18 @@ fn quantity_ref_uses_object_count(qty: &QuantityRef) -> bool { // leaves PRE-EXISTING recipients stale. | QuantityRef::BattlefieldEntriesThisTurn { .. } | QuantityRef::CommanderManaValue { .. } => true, - // Distinct card types reads battlefield population ONLY when its source - // is the object-filter variant; zone / linked-exile sources do not. - QuantityRef::DistinctCardTypes { source } => match source { - CardTypeSetSource::Objects { .. } => true, - CardTypeSetSource::Zone { .. } - | CardTypeSetSource::ExiledBySource - | CardTypeSetSource::TrackedSet { .. } => false, - }, - // Distinct subtypes mirrors distinct card types: only the object-filter - // source reads battlefield population; zone / linked-exile / tracked-set - // sources do not. - QuantityRef::DistinctSubtypes { source, .. } => match source { - CardTypeSetSource::Objects { .. } => true, - CardTypeSetSource::Zone { .. } - | CardTypeSetSource::ExiledBySource - | CardTypeSetSource::TrackedSet { .. } => false, - }, + // A distinct-characteristic count reads battlefield population ONLY when + // its source names a live object census; zone / linked-exile / + // tracked-set / turn-journal sources do not. All three characteristics + // share the population axis, so they share this classification — and + // `entered_object_perturbs_quantity_ref` narrows the SAME predicate to + // "does this entered object join the population?", which is what keeps + // the two functions' `false` arms aligned. + QuantityRef::DistinctCardTypes { source } + | QuantityRef::DistinctSubtypes { source, .. } + | QuantityRef::DistinctColorsAmong { source } => { + characteristic_source_reads_object_count(source) + } // Player-level, single-object, history-record, payment, and choice // references: unaffected by another object's battlefield entry/exit. QuantityRef::HandSize { .. } @@ -1123,6 +1373,65 @@ pub(crate) fn quantity_expr_characteristic_reads_at( /// those records store the object's characteristics as of the recorded event, so /// no later layer write can change the tally. They classify EMPTY, and their /// embedded filters are deliberately NOT recursed. +/// CR 109.2 + CR 601.2a: Which live characteristics a [`CardTypeSetSource`] +/// population reads through its own filters. +/// +/// Only the object filter and the journal's optional narrowing filter are live +/// filter reads; the zone / linked-exile / tracked-set arms select by membership +/// alone. `AnyOf` unions its members. +/// +/// DEPTH-BOUNDED, arm-for-arm with [`target_filter_characteristic_reads_at`]: +/// the budget is consumed at entry and exhaustion classifies +/// [`CharacteristicKinds::ALL`]. `AnyOf` nests, and its arity invariant bounds +/// WIDTH rather than DEPTH, so this walk needs the same budget its sibling +/// carries — it previously passed `depth` through untouched while every filter +/// walk it calls decremented, which made the nesting free. +/// +/// `ALL` is the fail-SAFE exhaustion answer: it over-reports reads and forces +/// conservative re-evaluation, where `EMPTY` would silently skip one. +/// +/// The bound is defence in depth rather than the only guard — `serde_json` +/// already caps deserialization nesting well below any plausible budget — but +/// a walk in a bounded chain that does not itself decrement is the kind of +/// inconsistency that stops being harmless the moment a caller passes a +/// hand-built source. +fn characteristic_source_reads_at(source: &CardTypeSetSource, depth: u32) -> CharacteristicKinds { + let mut kinds = CharacteristicKinds::EMPTY; + let complete = source.try_for_each_member(depth, &mut |leaf| { + kinds = kinds.union(characteristic_leaf_reads_at(leaf, depth)); + }); + if complete { + kinds + } else { + // Fail-SAFE: an unseen member may read anything, so over-report and + // force conservative re-evaluation rather than skip one. + CharacteristicKinds::ALL + } +} + +fn characteristic_leaf_reads_at(source: &CardTypeSetSource, depth: u32) -> CharacteristicKinds { + let Some(depth) = depth.checked_sub(1) else { + return CharacteristicKinds::ALL; + }; + match source { + CardTypeSetSource::Objects { filter } => { + target_filter_characteristic_reads_at(filter, depth) + } + CardTypeSetSource::TurnJournal { filter, .. } => filter + .as_ref() + .map_or(CharacteristicKinds::EMPTY, |filter| { + target_filter_characteristic_reads_at(filter, depth) + }), + // Unions are unrolled by `try_for_each_member` above, so a union reaching + // this arm has already been walked; contributing EMPTY here keeps the + // fold identity correct rather than double-counting. + CardTypeSetSource::AnyOf { .. } => CharacteristicKinds::EMPTY, + CardTypeSetSource::Zone { .. } + | CardTypeSetSource::ExiledBySource + | CardTypeSetSource::TrackedSet { .. } => CharacteristicKinds::EMPTY, + } +} + fn quantity_ref_characteristic_reads(qty: &QuantityRef, depth: u32) -> CharacteristicKinds { match qty { // ---- Live object censuses: recurse the counted filter. ---- @@ -1160,21 +1469,14 @@ fn quantity_ref_characteristic_reads(qty: &QuantityRef, depth: u32) -> Character // CR 109.5 + CR 613.1b: per-player partition of a live census. QuantityRef::ControlledByEachPlayer { filter, .. } => CharacteristicKinds::CONTROLLER .union(target_filter_characteristic_reads_at(filter, depth)), - // CR 106.1 + CR 109.1: distinct colors over a live census. - QuantityRef::DistinctColorsAmongPermanents { filter } => CharacteristicKinds::COLOR - .union(target_filter_characteristic_reads_at(filter, depth)), - // CR 205.2a / CR 205.3: the object-filter source scans live objects; the - // zone / linked-exile / tracked-set sources do not read a live filter. + // CR 105.1 + CR 105.2: distinct colors over the source population. + QuantityRef::DistinctColorsAmong { source } => CharacteristicKinds::COLOR + .union(characteristic_source_reads_at(source, depth)), + // CR 205.2a / CR 205.3: the object-filter and journal-filter sources read + // a live filter; the zone / linked-exile / tracked-set sources do not. QuantityRef::DistinctCardTypes { source } | QuantityRef::DistinctSubtypes { source, .. } => { - CharacteristicKinds::CARD_TYPES.union(match source { - CardTypeSetSource::Objects { filter, .. } => { - target_filter_characteristic_reads_at(filter, depth) - } - CardTypeSetSource::Zone { .. } - | CardTypeSetSource::ExiledBySource - | CardTypeSetSource::TrackedSet { .. } => CharacteristicKinds::EMPTY, - }) + CharacteristicKinds::CARD_TYPES.union(characteristic_source_reads_at(source, depth)) } // CR 604.3: a zone census, filtered by typeline and by an optional // filter, scoped by controller. @@ -1376,6 +1678,46 @@ pub(crate) fn entered_object_perturbs_quantity_expr( } } +/// CR 611.3a + CR 109.2: Would `entered`'s battlefield entry join the population +/// a [`CardTypeSetSource`] names? +/// +/// Only a live object census can gain a member from a battlefield entry. The +/// zone / linked-exile / tracked-set arms are not battlefield populations, and a +/// turn journal (CR 601.2a) records CASTS, which a battlefield entry is not — +/// the entry of a permanent that was cast was already journaled at cast time +/// (`finalize_cast`, CR 601.2a), so its entry adds nothing, and a permanent put +/// onto the battlefield without being cast is never journaled at all. `AnyOf` +/// is perturbed iff any member is. +fn characteristic_source_perturbed_by_entry( + state: &GameState, + entered: &crate::game::game_object::GameObject, + ctx: &FilterContext<'_>, + source: &CardTypeSetSource, +) -> bool { + any_characteristic_member(source, &mut |leaf| { + characteristic_leaf_perturbed_by_entry(state, entered, ctx, leaf) + }) +} + +fn characteristic_leaf_perturbed_by_entry( + state: &GameState, + entered: &crate::game::game_object::GameObject, + ctx: &FilterContext<'_>, + source: &CardTypeSetSource, +) -> bool { + match source { + CardTypeSetSource::Objects { filter } => { + matches_target_filter(state, entered.id, filter, ctx) + } + // Unrolled by the bounded walker below; a union never reaches this arm. + CardTypeSetSource::AnyOf { .. } => false, + CardTypeSetSource::Zone { .. } + | CardTypeSetSource::ExiledBySource + | CardTypeSetSource::TrackedSet { .. } + | CardTypeSetSource::TurnJournal { .. } => false, + } +} + /// CR 611.3a + CR 700.5: entry-membership leaf for /// `entered_object_perturbs_quantity_expr`. EXHAUSTIVE and wildcard-free — the /// classification mirrors `quantity_ref_uses_object_count`: every `false` arm @@ -1397,7 +1739,6 @@ fn entered_object_perturbs_quantity_ref( | QuantityRef::CountersOnObjects { filter, .. } | QuantityRef::Aggregate { filter, .. } | QuantityRef::ControlledByEachPlayer { filter, .. } - | QuantityRef::DistinctColorsAmongPermanents { filter } | QuantityRef::DistinctCounterKindsAmong { filter } | QuantityRef::EnteredThisTurn { filter } // CR 611.3a + CR 608.2i: narrowed to "would THIS object's entry join the @@ -1421,25 +1762,11 @@ fn entered_object_perturbs_quantity_ref( | QuantityRef::BattlefieldEntriesThisTurn { filter, .. } => { matches_target_filter(state, entered.id, filter, ctx) } - QuantityRef::DistinctCardTypes { source } => match source { - CardTypeSetSource::Objects { filter } => { - matches_target_filter(state, entered.id, filter, ctx) - } - // Zone / linked-exile / tracked-set sources are not battlefield - // population — the classifier returns false for them, so they cannot - // be perturbed. - CardTypeSetSource::Zone { .. } - | CardTypeSetSource::ExiledBySource - | CardTypeSetSource::TrackedSet { .. } => false, - }, - QuantityRef::DistinctSubtypes { source, .. } => match source { - CardTypeSetSource::Objects { filter } => { - matches_target_filter(state, entered.id, filter, ctx) - } - CardTypeSetSource::Zone { .. } - | CardTypeSetSource::ExiledBySource - | CardTypeSetSource::TrackedSet { .. } => false, - }, + QuantityRef::DistinctCardTypes { source } + | QuantityRef::DistinctSubtypes { source, .. } + | QuantityRef::DistinctColorsAmong { source } => { + characteristic_source_perturbed_by_entry(state, entered, ctx, source) + } // CR 700.5: devotion is perturbed iff the entered object's mana cost // contributes a symbol for one of the fixed colors. `ChosenColor`'s // color isn't statically known, so conservatively perturb (over- @@ -3386,99 +3713,18 @@ fn resolve_ref( // CR 205.2a: Count distinct card types (CoreType) across a source set. QuantityRef::DistinctCardTypes { source } => { let mut seen = HashSet::new(); - match source { - CardTypeSetSource::Zone { zone, scope } => match zone { - ZoneRef::Exile => { - for &obj_id in &state.exile { - if let Some(obj) = state.objects.get(&obj_id) { - let owner_matches = count_scope_owner_matches( - state, - scope, - ctx.clone(), - controller, - obj.owner, - ); - if owner_matches { - for ct in &obj.card_types.core_types { - seen.insert(*ct); - } - } - } - } - } - ZoneRef::Graveyard | ZoneRef::Library | ZoneRef::Hand => { - for player in scoped_players(state, scope, ctx, controller) { - let zone_ids = match zone { - ZoneRef::Graveyard => &player.graveyard, - ZoneRef::Library => &player.library, - ZoneRef::Hand => &player.hand, - ZoneRef::Exile => unreachable!(), - }; - for &obj_id in zone_ids { - if let Some(obj) = state.objects.get(&obj_id) { - for ct in &obj.card_types.core_types { - seen.insert(*ct); - } - } - } - } + visit_characteristic_source( + state, + source, + ctx.clone(), + &filter_ctx, + controller, + &mut |view| { + for ct in view.core_types() { + seen.insert(*ct); } }, - CardTypeSetSource::ExiledBySource => { - for linked in linked_exile_for_context(state, &ctx) { - if let Some(obj) = state.objects.get(&linked.exiled_id) { - for ct in &obj.card_types.core_types { - seen.insert(*ct); - } - } - } - } - CardTypeSetSource::Objects { filter } => { - let zone = filter - .extract_in_zone() - .unwrap_or(crate::types::zones::Zone::Battlefield); - for obj_id in crate::game::targeting::zone_object_ids(state, zone) { - if !matches_target_filter(state, obj_id, filter, &filter_ctx) { - continue; - } - if let Some(obj) = state.objects.get(&obj_id) { - for ct in &obj.card_types.core_types { - seen.insert(*ct); - } - } - } - } - // CR 608.2c + CR 205.2a/205.2b: distinct card types among the most - // recent chain tracked set. A merged Draw->Discard set is - // disambiguated by CAUSE: `Some(cause)` (e.g. Discarded) tallies - // only members whose recorded producer action equals the bound - // cause; drawn members are unstamped and excluded. `None` counts - // every member. Mirrors `FilteredTrackedSetSize`'s set selection - // (highest set id) and cause filter (`tracked_set_member_causes`). - CardTypeSetSource::TrackedSet { caused_by } => { - if let Some((set_id, ids)) = - state.tracked_object_sets.iter().max_by_key(|(id, _)| id.0) - { - for &oid in ids { - let cause_ok = match caused_by { - None => true, - Some(cause) => state - .tracked_set_member_causes - .get(set_id) - .and_then(|causes| causes.get(&oid)) - .is_some_and(|member_cause| member_cause == cause), - }; - if cause_ok { - if let Some(obj) = state.objects.get(&oid) { - for ct in &obj.card_types.core_types { - seen.insert(*ct); - } - } - } - } - } - } - } + ); usize_to_i32_saturating(seen.len()) } // CR 205.3 + CR 604.3: Count distinct subtype VALUES across the same @@ -3491,75 +3737,6 @@ fn resolve_ref( // other than creature types"). Subtype values are stored capitalized // (e.g. "Goblin"); inserted as-is to preserve consistent casing. QuantityRef::DistinctSubtypes { source, exclude } => { - // Gather the source object set first (same scan axis as - // `DistinctCardTypes`), then tally distinct subtype values across it. - // Collecting ids up front keeps the `&str` subtype borrows tied to - // `state.objects` for the lifetime of `seen`. - let mut obj_ids: Vec = Vec::new(); - match source { - CardTypeSetSource::Zone { zone, scope } => match zone { - ZoneRef::Exile => { - for &obj_id in &state.exile { - if let Some(obj) = state.objects.get(&obj_id) { - if count_scope_owner_matches( - state, - scope, - ctx.clone(), - controller, - obj.owner, - ) { - obj_ids.push(obj_id); - } - } - } - } - ZoneRef::Graveyard | ZoneRef::Library | ZoneRef::Hand => { - for player in scoped_players(state, scope, ctx, controller) { - let zone_ids = match zone { - ZoneRef::Graveyard => &player.graveyard, - ZoneRef::Library => &player.library, - ZoneRef::Hand => &player.hand, - ZoneRef::Exile => unreachable!(), - }; - obj_ids.extend(zone_ids.iter().copied()); - } - } - }, - CardTypeSetSource::ExiledBySource => { - for linked in linked_exile_for_context(state, &ctx) { - obj_ids.push(linked.exiled_id); - } - } - CardTypeSetSource::Objects { filter } => { - let zone = filter - .extract_in_zone() - .unwrap_or(crate::types::zones::Zone::Battlefield); - for obj_id in crate::game::targeting::zone_object_ids(state, zone) { - if matches_target_filter(state, obj_id, filter, &filter_ctx) { - obj_ids.push(obj_id); - } - } - } - CardTypeSetSource::TrackedSet { caused_by } => { - if let Some((set_id, ids)) = - state.tracked_object_sets.iter().max_by_key(|(id, _)| id.0) - { - for &oid in ids { - let cause_ok = match caused_by { - None => true, - Some(cause) => state - .tracked_set_member_causes - .get(set_id) - .and_then(|causes| causes.get(&oid)) - .is_some_and(|member_cause| member_cause == cause), - }; - if cause_ok { - obj_ids.push(oid); - } - } - } - } - } // CR 205.3m: skip subtypes that are creature types when excluding // creature types. Subtype values are stored capitalized ("Goblin") // and inserted as-is; a card with three subtypes contributes three. @@ -3577,17 +3754,25 @@ fn resolve_ref( } else { HashSet::new() }; + // The `&str` subtype borrows stay tied to `state` for the lifetime of + // `seen` because `visit_characteristic_source` yields views borrowed + // from `state`, not from a transient buffer. let mut seen: HashSet<&str> = HashSet::new(); - for obj_id in &obj_ids { - if let Some(obj) = state.objects.get(obj_id) { - for sub in &obj.card_types.subtypes { + visit_characteristic_source( + state, + source, + ctx.clone(), + &filter_ctx, + controller, + &mut |view| { + for sub in view.subtypes() { if exclude_creature && creature_types.contains(sub.as_str()) { continue; } seen.insert(sub.as_str()); } - } - } + }, + ); usize_to_i32_saturating(seen.len()) } // CR 603.10a + CR 607.2a: Count cards linked as "exiled with" the @@ -3988,29 +4173,31 @@ fn resolve_ref( .map(|obj| u32_to_i32_saturating(obj.effective_mana_value())) .unwrap_or(0) } - // CR 106.1 + CR 109.1: Count distinct colors (W/U/B/R/G) among permanents - // matching the filter. "Gold"/"multicolor"/"colorless" are not colors, so - // each ManaColor contributes at most once per colored permanent. - QuantityRef::DistinctColorsAmongPermanents { filter } => { - let zone = filter - .extract_in_zone() - .unwrap_or(crate::types::zones::Zone::Battlefield); + // CR 105.1 + CR 105.2: Count distinct colors (W/U/B/R/G) among the members + // of the source population. "Gold"/"multicolor"/"colorless" are not colors + // (CR 105.2), so each `ManaColor` contributes at most once per colored + // member and a colorless member contributes nothing. Because every member + // tallies into one `HashSet`, an `AnyOf` union counts a color shared by two + // populations once — `|A ∪ B| != |A| + |B|`. + QuantityRef::DistinctColorsAmong { source } => { let mut seen: HashSet = HashSet::new(); - for &id in crate::game::targeting::zone_object_ids(state, zone).iter() { - if !matches_target_filter(state, id, filter, &filter_ctx) { - continue; - } - if let Some(obj) = state.objects.get(&id) { - for color in &obj.color { + visit_characteristic_source( + state, + source, + ctx.clone(), + &filter_ctx, + controller, + &mut |view| { + for color in view.colors() { seen.insert(*color); } - } - } + }, + ); usize_to_i32_saturating(seen.len()) } // CR 122.1: Count distinct counter kinds among permanents matching the // filter (controller-relative, CR 109.4). Counter-side dual of - // `DistinctColorsAmongPermanents`. Each `CounterType` present on at + // `DistinctColorsAmong`. Each `CounterType` present on at // least one matching permanent contributes once. QuantityRef::DistinctCounterKindsAmong { filter } => { usize_to_i32_saturating(distinct_counter_kinds_among(state, filter, &filter_ctx).len()) @@ -5270,7 +5457,7 @@ fn object_id_for_scope( } /// CR 122.1: Distinct counter kinds present on objects matching `filter` -/// (controller-relative, CR 109.4). Mirrors `DistinctColorsAmongPermanents`'s +/// (controller-relative, CR 109.4). Mirrors `DistinctColorsAmong`'s /// resolver (zones from `filter.extract_zones()`, `zone_object_ids`, /// `matches_target_filter`), enumerating only positive-count counter kinds the /// same way proliferate does. Returns a `Vec` SORTED by @@ -7290,6 +7477,214 @@ mod tests { .push(ManaSpentSourceSnapshot { source_id, lki }); } + /// Row 18, resolver half. CR 400.1 + CR 109.2: `visit_characteristic_source`'s + /// `Objects` arm derives ONE zone via `TargetFilter::extract_in_zone`, which + /// for a composite returns the FIRST member's zone. A cross-zone `Or` is + /// therefore scanned in one zone and the other leg is DROPPED with no + /// diagnostic — which is why the parser refuses to emit one + /// (`objects_filter_zone_is_unambiguous`). + /// + /// The same cross-zone meaning IS expressible, as `AnyOf`, where each member + /// carries its own zone. The two halves together are the justification for + /// the refusal: the wrong shape silently under-counts; the right shape is + /// available. + #[test] + fn a_cross_zone_or_drops_a_leg_while_any_of_reads_both_zones() { + let mut state = GameState::new_two_player(7); + let controller = PlayerId(0); + + // A green creature on the battlefield. + let bear = create_object( + &mut state, + CardId(900), + controller, + "Bear".to_string(), + Zone::Battlefield, + ); + { + let obj = state.objects.get_mut(&bear).unwrap(); + obj.card_types.core_types = vec![CoreType::Creature]; + obj.color = vec![ManaColor::Green]; + } + // A blue card in the graveyard. + let ghost = create_object( + &mut state, + CardId(901), + controller, + "Ghost".to_string(), + Zone::Graveyard, + ); + { + let obj = state.objects.get_mut(&ghost).unwrap(); + obj.card_types.core_types = vec![CoreType::Instant]; + obj.color = vec![ManaColor::Blue]; + } + state + .players + .iter_mut() + .find(|p| p.id == controller) + .unwrap() + .graveyard + .push_back(ghost); + + let battlefield_creatures = TargetFilter::Typed( + TypedFilter::new(TypeFilter::Creature).controller(ControllerRef::You), + ); + let graveyard_cards = TargetFilter::Typed( + TypedFilter::new(TypeFilter::Card) + .controller(ControllerRef::You) + .properties(vec![FilterProp::InZone { + zone: Zone::Graveyard, + }]), + ); + + // SUPPORTED MULTI-ZONE SHAPE: one `Objects` population whose filter names + // BOTH zones. `population_zones` enumerates the whole `InAnyZone` union, + // so the walk visits both and both colours are counted. + // + // This is the assertion the old version of this test lacked. It pinned + // only the unsupported fold below and described it as "extract_in_zone + // collapses to one zone", which stopped being true when the walk started + // enumerating every zone — the number it asserted was still right, for a + // reason the comment no longer named. + let multi_zone = QuantityExpr::Ref { + qty: QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { + filter: TargetFilter::Typed( + TypedFilter::default() + .controller(ControllerRef::You) + .properties(vec![FilterProp::InAnyZone { + zones: vec![Zone::Battlefield, Zone::Graveyard], + }]), + ), + }, + }, + }; + assert_eq!( + resolve_quantity(&state, &multi_zone, controller, bear), + 2, + "an InAnyZone population counts EVERY named zone: green (battlefield) \ + + blue (graveyard). A walk that kept only the first zone reads 1" + ); + + // UNSUPPORTED SHAPE, and the boundary is asserted rather than assumed: a + // PARTIALLY zone-constrained `Or`. The first disjunct names no zone (so + // it means the battlefield, CR 110.1) while the second names the + // graveyard, and `population_zones` returns ONE FLAT LIST that cannot say + // "battlefield for that branch only". The list is `[Graveyard]`, which is + // non-empty, so the battlefield default never applies and the first + // disjunct's permanents are never scanned. + // + // The count is 1 — blue, from the graveyard — NOT the green the old + // comment here predicted. Both readings happen to be 1, which is exactly + // why the stale explanation survived: the number could not distinguish + // them. Asserting the identity below can. + let folded = QuantityExpr::Ref { + qty: QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { + filter: TargetFilter::Or { + filters: vec![battlefield_creatures.clone(), graveyard_cards.clone()], + }, + }, + }, + }; + assert_eq!( + resolve_quantity(&state, &folded, controller, bear), + 1, + "a partially zone-constrained Or scans only the zone it names" + ); + // The parser refuses to emit this shape, which is what keeps the + // undercount above unreachable from real Oracle text. If this guard ever + // goes green, the shape becomes expressible and the undercount becomes a + // live defect. + assert!( + !crate::parser::oracle_nom::quantity::objects_filter_zone_is_unambiguous( + &TargetFilter::Or { + filters: vec![battlefield_creatures.clone(), graveyard_cards.clone()], + } + ), + "the parser guard must refuse the partially zone-constrained Or that \ + the resolver cannot represent" + ); + + // RIGHT SHAPE for genuinely distinct populations: a union whose members + // each carry their own zone. + let union = QuantityExpr::Ref { + qty: QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::any_of(vec![ + CardTypeSetSource::Objects { + filter: battlefield_creatures, + }, + CardTypeSetSource::Objects { + filter: graveyard_cards, + }, + ]) + .expect("two-member union"), + }, + }; + assert_eq!( + resolve_quantity(&state, &union, controller, bear), + 2, + "AnyOf reads both zones: green (battlefield) + blue (graveyard)" + ); + } + + /// CR 109.2: the union is a SET union, not an arithmetic sum. A colour + /// present in both populations contributes once — the property that makes + /// `Sum { [DistinctColors(A), DistinctColors(B)] }` an incorrect + /// decomposition and forces the union into the population layer. + #[test] + fn any_of_deduplicates_a_characteristic_shared_by_two_populations() { + let mut state = GameState::new_two_player(7); + let controller = PlayerId(0); + + let bear = create_object( + &mut state, + CardId(910), + controller, + "Green Bear".to_string(), + Zone::Battlefield, + ); + { + let obj = state.objects.get_mut(&bear).unwrap(); + obj.card_types.core_types = vec![CoreType::Creature]; + obj.color = vec![ManaColor::Green]; + } + + // A GREEN cast record — the overlap with the battlefield population. + let record = SpellCastRecord { + colors: vec![ManaColor::Green], + core_types: vec![CoreType::Instant], + ..Default::default() + }; + state + .spells_cast_this_turn_by_player + .insert(controller, im::Vector::from(vec![record])); + + let union = QuantityExpr::Ref { + qty: QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::any_of(vec![ + CardTypeSetSource::Objects { + filter: TargetFilter::Typed( + TypedFilter::new(TypeFilter::Permanent).controller(ControllerRef::You), + ), + }, + CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: CountScope::Controller, + filter: None, + }, + ]) + .expect("two-member union"), + }, + }; + assert_eq!( + resolve_quantity(&state, &union, controller, bear), + 1, + "green is in BOTH populations and must contribute once (|A ∪ B| != |A| + |B|)" + ); + } + #[test] fn resolve_source_qualified_mana_spent_counts_matching_snapshots() { let mut state = GameState::new_two_player(42); diff --git a/crates/engine/src/game/triggers.rs b/crates/engine/src/game/triggers.rs index a89196f8d6..8e6fe1523a 100644 --- a/crates/engine/src/game/triggers.rs +++ b/crates/engine/src/game/triggers.rs @@ -9116,7 +9116,6 @@ fn quantity_ref_binding_diverges(qty: &QuantityRef) -> bool { | QuantityRef::CountersOnObjects { filter, .. } | QuantityRef::Aggregate { filter, .. } | QuantityRef::EnteredThisTurn { filter } - | QuantityRef::DistinctColorsAmongPermanents { filter } | QuantityRef::DistinctCounterKindsAmong { filter } | QuantityRef::ControlledByEachPlayer { filter, .. } | QuantityRef::ZoneChangeCountThisTurn { filter, .. } @@ -9136,10 +9135,15 @@ fn quantity_ref_binding_diverges(qty: &QuantityRef) -> bool { | QuantityRef::AttackedThisTurn { filter, .. } => { filter.as_ref().is_some_and(filter_binding_diverges) } - // CR 205.2a + CR 205.3: the distinct-type family carries its population - // as a `CardTypeSetSource` rather than a bare filter. + // CR 205.2a + CR 205.3 + CR 105.1: the distinct-characteristic family + // carries its population as a `CardTypeSetSource` rather than a bare + // filter. The COLOURS head joins its two siblings here rather than the + // bare-filter arm above: it was lifted onto the shared population axis in + // this change, so `DistinctColorsAmongPermanents { filter }` no longer + // exists to sit alongside `ObjectCount`. QuantityRef::DistinctCardTypes { source } - | QuantityRef::DistinctSubtypes { source, .. } => { + | QuantityRef::DistinctSubtypes { source, .. } + | QuantityRef::DistinctColorsAmong { source } => { card_type_set_source_binding_diverges(source) } // CR 601.2h: `AbilityTarget` is a target-slot read that @@ -9250,15 +9254,36 @@ fn quantity_ref_binding_diverges(qty: &QuantityRef) -> bool { /// Exhaustive for the same reason: each source names a different population, and /// two of them are resolution-scoped. fn card_type_set_source_binding_diverges(source: &CardTypeSetSource) -> bool { - match source { - // CR 400.1: a zone census keyed by `CountScope` (controller / opponents / - // all) — the fire-time leg has the controller and reads the same zones. - CardTypeSetSource::Zone { .. } => false, - CardTypeSetSource::Objects { filter } => filter_binding_diverges(filter), - // CR 607.2a + CR 608.2c: the same two resolution-scoped populations - // `CardsExiledBySource` / `TrackedSetSize` are declined for. - CardTypeSetSource::ExiledBySource | CardTypeSetSource::TrackedSet { .. } => true, - } + let mut diverges = false; + let complete = + source.try_for_each_member(crate::types::ability::UNION_DEPTH_BUDGET, &mut |leaf| { + if diverges { + return; + } + diverges = match leaf { + // CR 400.1: a zone census keyed by `CountScope` (controller / + // opponents / all) — the fire-time leg has the controller and + // reads the same zones. + CardTypeSetSource::Zone { .. } => false, + CardTypeSetSource::Objects { filter } => filter_binding_diverges(filter), + // CR 601.2a: the journal's optional narrowing filter is the only + // re-scopable part; the journal itself is per-player history that + // binds the same on both legs. + CardTypeSetSource::TurnJournal { filter, .. } => { + filter.as_ref().is_some_and(filter_binding_diverges) + } + // CR 607.2a + CR 608.2i: the same two resolution-scoped + // populations `CardsExiledBySource` / `TrackedSetSize` are + // declined for. + CardTypeSetSource::ExiledBySource | CardTypeSetSource::TrackedSet { .. } => true, + // Unrolled by the walker; never reaches this arm. + CardTypeSetSource::AnyOf { .. } => false, + }; + }); + // CR 603.4: a truncated walk DECLINES the hoist. Declining costs a delayed + // trigger its fire-time shortcut; wrongly allowing it re-scopes a population + // against the wrong binding, which is a rules error. + diverges || !complete } /// CR 603.4: the filter half of [`quantity_ref_binding_diverges`]. Recurses @@ -12252,6 +12277,37 @@ fn quantity_expr_refs_cost_paid_object(expr: &QuantityExpr) -> bool { } } +/// Does a [`CardTypeSetSource`] population route a filter that references the +/// cost-paid object? Only the object filter and the journal's optional +/// narrowing filter can; `AnyOf` recurses so a union member's reference is not +/// dropped. +/// +/// Uncited: a structural query over which arms hold a `TargetFilter`, not a rule +/// implementation. (It cited CR 109.2, the battlefield-default rule for a bare +/// type description, which does not speak to filter routing.) +fn characteristic_source_references_cost_paid_object(source: &CardTypeSetSource) -> bool { + let mut found = false; + let complete = + source.try_for_each_member(crate::types::ability::UNION_DEPTH_BUDGET, &mut |leaf| { + if found { + return; + } + found = match leaf { + CardTypeSetSource::Objects { filter } => filter.references_cost_paid_object(), + CardTypeSetSource::TurnJournal { filter, .. } => filter + .as_ref() + .is_some_and(TargetFilter::references_cost_paid_object), + CardTypeSetSource::Zone { .. } + | CardTypeSetSource::ExiledBySource + | CardTypeSetSource::TrackedSet { .. } + | CardTypeSetSource::AnyOf { .. } => false, + }; + }); + // A truncated walk claims the reference: this gate exists to stop a + // cost-paid-object read from escaping, so exhaustion must not let one past. + found || !complete +} + /// CR 400.7d + CR 608.2k: True when this `QuantityRef` reads the cost-paid /// object, by either of the two structural axes a ref can carry it on: /// @@ -12292,7 +12348,6 @@ fn quantity_ref_refs_cost_paid_object(qty: &QuantityRef) -> bool { | QuantityRef::ZoneChangeAggregateThisTurn { filter, .. } | QuantityRef::CounterAddedThisTurn { target: filter, .. } | QuantityRef::TokensCreatedThisTurn { filter, .. } - | QuantityRef::DistinctColorsAmongPermanents { filter } | QuantityRef::DistinctCounterKindsAmong { filter } => filter.references_cost_paid_object(), // Filter-bearing refs (boxed `TargetFilter`): recurse (auto-deref). @@ -12315,21 +12370,13 @@ fn quantity_ref_refs_cost_paid_object(qty: &QuantityRef) -> bool { source.references_cost_paid_object() || target.references_cost_paid_object() } - // Card-type counting embeds a `TargetFilter` through its source enum. - QuantityRef::DistinctCardTypes { source } => match source { - CardTypeSetSource::Objects { filter } => filter.references_cost_paid_object(), - CardTypeSetSource::Zone { .. } - | CardTypeSetSource::ExiledBySource - | CardTypeSetSource::TrackedSet { .. } => false, - }, - - // Subtype counting embeds a `TargetFilter` through its source enum too. - QuantityRef::DistinctSubtypes { source, .. } => match source { - CardTypeSetSource::Objects { filter } => filter.references_cost_paid_object(), - CardTypeSetSource::Zone { .. } - | CardTypeSetSource::ExiledBySource - | CardTypeSetSource::TrackedSet { .. } => false, - }, + // Card-type / subtype / colour counting all embed their `TargetFilter`s + // through the shared population enum. + QuantityRef::DistinctCardTypes { source } + | QuantityRef::DistinctSubtypes { source, .. } + | QuantityRef::DistinctColorsAmong { source } => { + characteristic_source_references_cost_paid_object(source) + } // Mana-spent metering embeds a `TargetFilter` through its metric enum. QuantityRef::ManaSpentToCast { metric, .. } => match metric { @@ -18274,6 +18321,92 @@ pub mod tests { ); } + /// The shared characteristic-source branch, which the three distinct-count + /// heads all route through. Sibling to the two tests above, which cover the + /// per-`QuantityRef` arms but never reach this population axis. + /// + /// Each filter-BEARING arm is exercised, plus the recursion and the + /// fixed-vocabulary arms that must stay false — a gate that answered `true` + /// for everything would pass a positive-only test. + #[test] + fn cost_paid_object_gate_covers_every_characteristic_source_arm() { + use crate::types::ability::{CardTypeSetSource, CountScope, TurnJournalKind, ZoneRef}; + + let objects = |filter| CardTypeSetSource::Objects { filter }; + let journal = |filter| CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: CountScope::Controller, + filter, + }; + + // Every head shares one population axis, so detection must not depend on + // which characteristic is being counted. + for qty in [ + QuantityRef::DistinctColorsAmong { + source: objects(TargetFilter::CostPaidObject), + }, + QuantityRef::DistinctCardTypes { + source: objects(TargetFilter::CostPaidObject), + }, + ] { + assert!( + quantity_ref_refs_cost_paid_object(&qty), + "an Objects population over the cost-paid object must be detected: {qty:?}" + ); + } + + // The journal's optional narrowing filter is the second filter-bearing + // arm, and `None` there must not be mistaken for a reference. + assert!( + quantity_ref_refs_cost_paid_object(&QuantityRef::DistinctCardTypes { + source: journal(Some(TargetFilter::CostPaidObject)), + }), + "a cost-paid-object reference in the journal's narrowing filter must be detected" + ); + assert!( + !quantity_ref_refs_cost_paid_object(&QuantityRef::DistinctCardTypes { + source: journal(None), + }), + "an unfiltered journal references nothing" + ); + + // `AnyOf` recursion: a reference in ANY member is a reference, including + // one nested a union deep, and a union of clean members stays false. + let clean = CardTypeSetSource::Zone { + zone: ZoneRef::Graveyard, + scope: CountScope::Controller, + }; + let nested = CardTypeSetSource::any_of(vec![ + clean.clone(), + CardTypeSetSource::any_of(vec![ + CardTypeSetSource::ExiledBySource, + objects(TargetFilter::CostPaidObject), + ]) + .expect("two-member union"), + ]) + .expect("two-member union"); + assert!( + quantity_ref_refs_cost_paid_object(&QuantityRef::DistinctSubtypes { + source: nested, + exclude: Default::default(), + }), + "a reference nested inside a union of unions must be detected" + ); + + // Fixed-vocabulary arms carry no filter and must stay false — this is + // what stops the gate from degenerating into "always true". + for source in [ + clean, + CardTypeSetSource::ExiledBySource, + CardTypeSetSource::TrackedSet { caused_by: None }, + ] { + assert!( + !characteristic_source_references_cost_paid_object(&source), + "a fixed-vocabulary population routes no filter: {source:?}" + ); + } + } + /// CR 400.7d end-to-end: `build_triggered_ability` propagates the emerge /// `cast_cost_paid_object` snapshot onto a sub-ability whose magnitude reads /// "the number of counters on the sacrificed creature" diff --git a/crates/engine/src/parser/oracle.rs b/crates/engine/src/parser/oracle.rs index 8edb802c04..5c1dccafbd 100644 --- a/crates/engine/src/parser/oracle.rs +++ b/crates/engine/src/parser/oracle.rs @@ -1550,7 +1550,6 @@ fn quantity_ref_uses_filter_prop(qty: &QuantityRef, pred: &impl Fn(&FilterProp) | QuantityRef::CountersOnObjects { filter, .. } | QuantityRef::Aggregate { filter, .. } | QuantityRef::ControlledByEachPlayer { filter, .. } - | QuantityRef::DistinctColorsAmongPermanents { filter } | QuantityRef::DistinctCounterKindsAmong { filter } | QuantityRef::EnteredThisTurn { filter } // CR 608.2i: the look-back sibling carries a `TargetFilter` too, and this @@ -1560,17 +1559,50 @@ fn quantity_ref_uses_filter_prop(qty: &QuantityRef, pred: &impl Fn(&FilterProp) | QuantityRef::BattlefieldEntriesThisTurn { filter, .. } => { target_filter_uses_filter_prop(filter, pred) } - QuantityRef::DistinctCardTypes { - source: crate::types::ability::CardTypeSetSource::Objects { filter }, + // CR 109.2: the three distinct-characteristic counts embed their filters + // through the shared population enum; recurse over it so a union member + // or a journal's narrowing filter is not dropped. + QuantityRef::DistinctCardTypes { source } + | QuantityRef::DistinctSubtypes { source, .. } + | QuantityRef::DistinctColorsAmong { source } => { + characteristic_source_uses_filter_prop(source, pred) } - | QuantityRef::DistinctSubtypes { - source: crate::types::ability::CardTypeSetSource::Objects { filter }, - .. - } => target_filter_uses_filter_prop(filter, pred), _ => false, } } +/// CR 109.2: Does any `TargetFilter` reachable through a `CardTypeSetSource` +/// population use `pred`? The fixed-vocabulary zone / linked-exile / tracked-set +/// arms carry none. +fn characteristic_source_uses_filter_prop( + source: &crate::types::ability::CardTypeSetSource, + pred: &impl Fn(&FilterProp) -> bool, +) -> bool { + use crate::types::ability::CardTypeSetSource; + let mut found = false; + let complete = + source.try_for_each_member(crate::types::ability::UNION_DEPTH_BUDGET, &mut |leaf| { + if found { + return; + } + found = match leaf { + CardTypeSetSource::Objects { filter } => { + target_filter_uses_filter_prop(filter, pred) + } + CardTypeSetSource::TurnJournal { filter, .. } => filter + .as_ref() + .is_some_and(|filter| target_filter_uses_filter_prop(filter, pred)), + CardTypeSetSource::Zone { .. } + | CardTypeSetSource::ExiledBySource + | CardTypeSetSource::TrackedSet { .. } + | CardTypeSetSource::AnyOf { .. } => false, + }; + }); + // A truncated walk claims the prop: this feeds parse-time capability + // reporting, where over-reporting a dependency is the harmless direction. + found || !complete +} + fn target_filter_uses_filter_prop( filter: &TargetFilter, pred: &impl Fn(&FilterProp) -> bool, diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index 0fd8850fb9..68f35e8a79 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -3854,13 +3854,14 @@ pub(crate) fn strip_for_each_prefix(text: &str) -> (Option, String terminated(take_until(", "), tag::<_, _, OracleError<'_>>(", ")).parse(rest_lower) { if let Some(qty) = parse_for_each_clause(clause) { - // CR 106.1: "for each color among [X], add one mana of that color" + // CR 105.1: "for each color among [X], add one mana of that color" // must NOT be split into (repeat_for, "add one mana of that color"). // The "that color" anaphors the per-iteration color, not the // source's `ChosenAttribute::Color`. Let the full text flow - // through to `try_parse_for_each_color_mana_public` which emits - // a single `DistinctColorsAmongPermanents` mana ability. - if matches!(qty, QuantityRef::DistinctColorsAmongPermanents { .. }) + // through to `try_parse_for_each_color_mana_public` which emits a + // single `ManaProduction::DistinctColorsAmongPermanents` mana + // ability (a DIFFERENT enum from the `QuantityRef` matched here). + if matches!(qty, QuantityRef::DistinctColorsAmong { .. }) && remainder .trim_end_matches('.') .trim() diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index 1e8a04e682..f658de4b6a 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -37549,10 +37549,10 @@ fn reveal_until_x_permanent_cards_choose_any_number_aurora() { matches!( count, QuantityExpr::Ref { - qty: QuantityRef::DistinctColorsAmongPermanents { .. } + qty: QuantityRef::DistinctColorsAmong { .. } } ), - "count must bind to DistinctColorsAmongPermanents via the where-X clause, got {count:?}" + "count must bind to DistinctColorsAmong via the where-X clause, got {count:?}" ); assert_eq!( *matched_disposition, @@ -37632,10 +37632,10 @@ fn reveal_until_x_nonland_cards_binds_dynamic_count_sanar_core() { matches!( count, QuantityExpr::Ref { - qty: QuantityRef::DistinctColorsAmongPermanents { .. } + qty: QuantityRef::DistinctColorsAmong { .. } } ), - "Sanar's count must bind to DistinctColorsAmongPermanents, got {count:?}" + "Sanar's count must bind to DistinctColorsAmong, got {count:?}" ); assert!( matches!(&type_filters[..], [TypeFilter::Non(inner)] if matches!(**inner, TypeFilter::Land)), diff --git a/crates/engine/src/parser/oracle_nom/quantity.rs b/crates/engine/src/parser/oracle_nom/quantity.rs index af1a55c764..c1d6de9e94 100644 --- a/crates/engine/src/parser/oracle_nom/quantity.rs +++ b/crates/engine/src/parser/oracle_nom/quantity.rs @@ -29,7 +29,8 @@ use crate::types::ability::{ AggregateFunction, CardTypeSetSource, CastManaObjectScope, CastManaSpentMetric, ControllerRef, CountScope, DamageChannel, DamageKindFilter, DevotionColors, FilterProp, ObjectProperty, ObjectScope, PlayerFilter, PlayerScope, PtStat, QuantityExpr, QuantityRef, RoundingMode, - SharedQuality, SubtypeExclusion, TargetFilter, ThisWayCause, TypeFilter, TypedFilter, ZoneRef, + SharedQuality, SubtypeExclusion, TargetFilter, ThisWayCause, TurnJournalKind, TypeFilter, + TypedFilter, ZoneRef, }; use crate::types::counter::{CounterMatch, CounterType}; use crate::types::keywords::Keyword; @@ -931,25 +932,21 @@ pub fn parse_quantity_ref(input: &str) -> OracleResult<'_, QuantityRef> { parse_scry_look_count_ref, parse_the_number_of, parse_object_property_aggregate_ref, - parse_distinct_card_types_exiled_with_source, // Group mana-value aggregate parsers to reduce alt arity alt(( parse_linked_exile_mana_value_ref, parse_greatest_commander_mana_value_ref, parse_commander_mana_value_ref, )), + // CR 110.4: "permanent type[s] among cards in " is a distinct head + // (it lowers to `ObjectCountDistinct`, not `DistinctCardTypes`) and must + // precede the card-type head so its leading token is not mis-committed. + parse_distinct_permanent_types_in_zone, + // CR 205.2a: one population grammar for every "card type[s] among …" + // reading. Nested with the distinct-by-quality head to keep the outer + // `alt` within nom's tuple arity (nom 8.0 max: 21 items). alt(( - parse_distinct_card_types_in_zone, - parse_distinct_permanent_types_in_zone, - )), - // CR 608.2c + CR 205.2a: "card type[s] among cards this way" must - // precede the generic `among ` arm so the chain-tracked-set, - // cause-filtered count wins on the "card type among cards" prefix. Nested - // with `parse_distinct_card_types_among_objects` to keep the outer `alt` - // within nom's tuple arity (nom 8.0 max: 21 items). - alt(( - parse_distinct_card_types_among_tracked_set, - parse_distinct_card_types_among_objects, + parse_distinct_card_types_among, // CR 201.2 + CR 603.4: "different among " // distinct-by-quality count (nested here to stay within nom's // tuple arity). @@ -1025,7 +1022,7 @@ pub fn parse_quantity_ref(input: &str) -> OracleResult<'_, QuantityRef> { // among ") + parse_type_phrase`) is shared with the "the number of // colors among ..." path; registering it here makes it reachable in the // bare-suffix context too. - parse_number_of_distinct_colors_among_permanents_tail, + parse_distinct_colors_among_tail, // CR 402.1: "the player with the {most|fewest} cards in hand" — the // cross-player hand-size extremum, the hand-zone peer of the life // extremum. Distinctive "the player with the " prefix; no ordering @@ -1676,19 +1673,12 @@ fn parse_object_property_aggregate_ref(input: &str) -> OracleResult<'_, Quantity /// Parse the inner part after "the number of". fn parse_number_of_inner(input: &str) -> OracleResult<'_, QuantityRef> { alt(( - parse_distinct_card_types_exiled_with_source, - alt(( - parse_distinct_card_types_in_zone, - parse_distinct_permanent_types_in_zone, - )), - // CR 608.2c + CR 205.2a: "card type[s] among cards this way" must - // precede the generic `among ` arm (same ordering as - // `parse_quantity_ref`). Nested with `parse_distinct_card_types_among_objects` - // to stay within nom's top-level `alt` arity (nom 8.0 max: 21 items). - alt(( - parse_distinct_card_types_among_tracked_set, - parse_distinct_card_types_among_objects, - )), + // CR 110.4: the permanent-type head lowers to `ObjectCountDistinct`, not + // `DistinctCardTypes`, so it must precede the card-type head. + parse_distinct_permanent_types_in_zone, + // CR 205.2a: one population grammar for every "card type[s] among …" + // reading (same ordering as `parse_quantity_ref`). + parse_distinct_card_types_among, // CR 205.3 + CR 500 + CR 604.3: counted CDA quantities that read live game // state — "different subtypes … among " (Subgoyf) and "turns // you've taken this game" (Control Win Condition). Both must precede the @@ -1760,7 +1750,7 @@ fn parse_number_of_inner(input: &str) -> OracleResult<'_, QuantityRef> { parse_number_of_times_you_chose_a_mode, )), parse_tokens_created_this_turn_tail, - parse_number_of_distinct_colors_among_permanents_tail, + parse_distinct_colors_among_tail, // CR 107.1 + CR 700.1: "[type] controlled by the player who controls // the fewest/most" — must precede `parse_number_of_controlled_type`, // whose " you control" suffix would otherwise not match but whose @@ -1830,35 +1820,49 @@ fn parse_number_of_inner(input: &str) -> OracleResult<'_, QuantityRef> { .parse(input) } -/// Parse "colors among [filter]" after "the number of". -fn parse_number_of_distinct_colors_among_permanents_tail( - input: &str, -) -> OracleResult<'_, QuantityRef> { +/// CR 105.1 + CR 105.2: "colors among \" → +/// [`QuantityRef::DistinctColorsAmong`]. +/// +/// Reached both from "the number of colors among …" and from the bare-suffix +/// context a parent has already stripped "there are N " from (Puca's Eye). +/// Parameterized onto the shared population grammar so First Family's union +/// ("permanents you control and spells you've cast this turn") is expressible; +/// `|A ∪ B| != |A| + |B|`, so the union must be inside the population, not +/// above it. +fn parse_distinct_colors_among_tail(input: &str) -> OracleResult<'_, QuantityRef> { let (rest, _) = tag("colors among ").parse(input)?; // CR 702.167c + CR 105.1: "the number of colors among the exiled cards used // to craft it" — distinct colors over the craft-material linked-exile pool - // (Sunbird Effigy P/T). Tried before the generic type-phrase filter so the + // (Sunbird Effigy P/T). Tried before the generic population grammar so the // craft noun phrase wins. if let Ok((craft_rest, filter)) = parse_craft_materials_filter(rest) { if matches!(craft_rest.trim(), "" | "." | ",") { - return Ok(("", QuantityRef::DistinctColorsAmongPermanents { filter })); + return Ok(( + "", + QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { filter }, + }, + )); } } - let (remainder, filter) = super::target::parse_type_phrase(rest)?; - if !matches!(remainder.trim(), "" | "." | ",") - || !quantity_filter_has_meaningful_content(&filter) - { - return Err(nom::Err::Error(nom::error::Error::new( - input, - nom::error::ErrorKind::Fail, - ))); + // CR 105.1: STRICT grammar. This head reads with + // `oracle_nom::target::parse_type_phrase` and must keep doing so. Switching + // to Legacy would silently accept anaphors ("those creatures"), turning + // General Tazri's honest `Unimplemented{where_x_binding}` into a confident + // count over a `TrackedSet(0)` sentinel that has no published set in an + // activated-ability context — an honest gap traded for a silent misparse. + let (remainder, source) = + parse_characteristic_set_source_list(rest, TypePhraseGrammar::Strict)?; + // UNCHANGED head guard: this head owns the whole clause. + if !matches!(remainder.trim(), "" | "." | ",") { + return Err(oracle_err(input)); } - Ok(("", QuantityRef::DistinctColorsAmongPermanents { filter })) + Ok(("", QuantityRef::DistinctColorsAmong { source })) } /// CR 122.1: Parse the iteration source "kind of counter on/among " → /// `QuantityRef::DistinctCounterKindsAmong { filter }`. Counter-side analogue of -/// `parse_number_of_distinct_colors_among_permanents_tail`. Used by Bribe +/// `parse_distinct_colors_among_tail`. Used by Bribe /// Taker's "for each kind of counter on permanents you control" — the filter is /// any controlled-permanent type phrase, so the combinator covers the whole /// class, not one card. Both "on" and "among" surface forms are accepted. @@ -2331,19 +2335,511 @@ fn parse_cards_in_zone_ref(input: &str) -> OracleResult<'_, QuantityRef> { parse_zone_card_count(input) } -fn parse_distinct_card_types_in_zone(input: &str) -> OracleResult<'_, QuantityRef> { - let (rest, _) = tag("card type").parse(input)?; - let (rest, _) = opt(tag("s")).parse(rest)?; - let (rest, _) = tag(" among cards in ").parse(rest)?; - let (rest, (zone, scope)) = parse_scoped_zone_ref(rest)?; +/// CR 109.2: Which of the two production type-phrase grammars an `Objects` +/// source reads with. +/// +/// NOT a stylistic choice, and NOT interchangeable. Measured differences: +/// +/// | phrase | Legacy | Strict | +/// |---|---|---| +/// | `creatures and planeswalkers they control` | FOLDED into one `Or[..]`, consumed whole | `Typed{Creature}`, remainder `" and planeswalkers …"` | +/// | `permanents you control and spells …` | not folded (the controller suffix intervenes) | same | +/// | `those creatures` / `them` | EMPTY `TypedFilter` + the whole input (its infallible failure shape) | `Err` | +/// +/// A characteristic head that switches grammars therefore changes which cards it +/// accepts. Each head keeps the grammar it is wired to, expressed as a typed +/// parameter rather than left to whichever import happened to be in scope. +#[derive(Clone, Copy, PartialEq, Eq)] +enum TypePhraseGrammar { + /// [`crate::parser::oracle_nom::target::parse_type_phrase`] — `" or "`-only + /// type lists (`parse_type_list`), no ownership / token / combat-relation + /// grammar, fails with `Err`. The colours head reads with this. + Strict, + /// [`crate::parser::oracle_target::parse_type_phrase`] — folds + /// `" and "` / `" and/or "` into type unions (`TYPE_SEPARATORS`) and carries + /// ownership / token / combat-relation grammar. INFALLIBLE: on failure it + /// yields an EMPTY `TypedFilter` plus the whole input — NOT + /// `TargetFilter::Any`, which is why the emptiness guard in + /// [`parse_objects_source`], not the `Any` guard, is what declines an + /// unrecognized phrase. The card-type and subtype heads read with this. + Legacy, +} + +/// CR 109.2 + CR 400.1: How far an `Objects` source must reach. +#[derive(Clone, Copy, PartialEq, Eq)] +enum ObjectsSourceExtent { + /// Single-source reading: the type phrase must consume the whole "among …" + /// clause (modulo the grandfathered trailing `.`/`,` trim). + WholeClause, + /// Union member: the type phrase stops at the population conjunction; the + /// terminal anchor is supplied by the LIST, not by this arm. + UnionMember, +} + +/// CR 109.2 + CR 400.1: Is this type phrase a POPULATION rather than a bare type? +/// +/// A population is anchored to a controller ("permanents you control") or to a +/// zone ("cards in your graveyard"). A bare type word ("creatures") names a +/// TYPE, not a population. +/// +/// Applied ONLY under [`TypePhraseGrammar::Strict`]. Measured: under `Legacy`, +/// `TYPE_SEPARATORS` folds `" and "` into the type union before the controller +/// suffix is read ("creatures and planeswalkers they control" → +/// `Or[Typed{Creature,You}, Typed{Planeswalker,You}]`, consumed whole), so a +/// bare-type-word conjunction never forms a list and the arity check is what +/// declines it — this predicate has no reachable Legacy input. Under `Strict`, +/// `parse_type_list` joins on `" or "` ONLY, so the same phrase WOULD split into +/// two bogus sources; this is the guard that stops it. No current card exercises +/// it, so it is a grammar-reachability guard, not a card-driven one. +fn filter_is_population_anchored(filter: &TargetFilter) -> bool { + if filter.extract_in_zone().is_some() { + return true; + } + match filter { + TargetFilter::Typed(typed) => typed.controller.is_some(), + TargetFilter::And { filters } | TargetFilter::Or { filters } => { + !filters.is_empty() && filters.iter().all(filter_is_population_anchored) + } + TargetFilter::Not { filter } => filter_is_population_anchored(filter), + // Every remaining variant is a LEAF that is neither controller-anchored + // nor zone-anchored (the zone case already returned above). Enumerated + // explicitly rather than defaulted, so a future variant that IS a + // population anchor has to be classified here instead of being silently + // declined. Not merged with the zone-bearing leaves above: those exit + // through `extract_in_zone` and never reach this match. + TargetFilter::None + | TargetFilter::Any + | TargetFilter::Player + | TargetFilter::Controller + | TargetFilter::SourceController + | TargetFilter::ControllerAndControlledPermanents { .. } + | TargetFilter::Opponent + | TargetFilter::SelfRef + | TargetFilter::GrantingObject + | TargetFilter::SourceOrPaired + | TargetFilter::StackAbility { .. } + | TargetFilter::StackSpell + | TargetFilter::SpecificObject { .. } + | TargetFilter::SpecificPlayer { .. } + | TargetFilter::PlayerWhoChoseLabel { .. } + | TargetFilter::Neighbor { .. } + | TargetFilter::ScopedPlayer + | TargetFilter::AttachedTo + | TargetFilter::LastCreated + | TargetFilter::LastRevealed + | TargetFilter::LastZoneChanged + | TargetFilter::CostPaidObject + | TargetFilter::ChosenCard + | TargetFilter::TrackedSet { .. } + | TargetFilter::TrackedSetFiltered { .. } + | TargetFilter::ExiledBySource + | TargetFilter::ExiledCardByIndex { .. } + | TargetFilter::TriggeringSpellController + | TargetFilter::TriggeringSpellOwner + | TargetFilter::TriggeringPlayer + | TargetFilter::TriggeringSource + | TargetFilter::EventTarget + | TargetFilter::TriggeringSourceController + | TargetFilter::ParentTarget + | TargetFilter::ParentTargetSlot { .. } + | TargetFilter::ParentTargetController + | TargetFilter::ParentTargetOwner + | TargetFilter::SourceChosenPlayer + | TargetFilter::OriginalController + | TargetFilter::OriginalSource + | TargetFilter::PostReplacementSourceController + | TargetFilter::PostReplacementDamageSource + | TargetFilter::PostReplacementDamageTarget + | TargetFilter::PostReplacementDamageTargetOwner + | TargetFilter::DefendingPlayer + | TargetFilter::HasChosenName + | TargetFilter::ChosenDamageSource { .. } + | TargetFilter::Named { .. } + | TargetFilter::Owner + | TargetFilter::AllPlayers => false, + } +} + +/// CR 400.1 + CR 109.2: Does this `Objects` filter denote ONE population domain? +/// +/// REASON UPDATED — the original one is obsolete. This guard was written because +/// `visit_characteristic_source` derived a single zone via `extract_in_zone` and +/// would silently drop the other leg of a cross-zone `Or`. That collapse is +/// gone: the walk now enumerates every zone in +/// [`CardTypeSetSource::population_zones`]. +/// +/// What remains, and what this still guards, is narrower and lives one level +/// down. `population_zones` returns a FLAT zone list for the whole filter, so it +/// cannot express "battlefield for this branch, graveyard for that one". A +/// PARTIALLY zone-constrained `Or` — `Or[Typed{Creature}, Typed{Card, +/// InZone(Graveyard)}]`, "creatures and cards in your graveyard" — yields +/// `[Graveyard]`, which is non-empty, so the battlefield default never applies +/// and the unconstrained disjunct's permanents are dropped. Refused, so the card +/// surfaces as an honest gap instead of a confident undercount. Cross-zone +/// populations ARE expressible as [`CardTypeSetSource::AnyOf`], where each +/// member carries its own zone. +/// +/// (`game::quantity::filter_candidate_universe` solves the same problem the +/// other way, by recursing per branch so an unconstrained branch keeps its +/// battlefield domain. Teaching `population_zones` that shape would retire this +/// guard and widen coverage; it is deliberately NOT done here, because it +/// changes which cards parse and belongs in its own change.) +/// +/// A GRAMMAR-REACHABILITY guard, not a card-driven one, and deliberately not +/// claimed to be more: measured, Legacy's `TYPE_SEPARATORS` fold of "creatures +/// and cards in your graveyard" distributes the zone across BOTH members, so +/// that particular phrase is zone-unambiguous by the time it reaches here. The +/// guard exists because nothing in the type-phrase grammar GUARANTEES that +/// distribution, and the failure it would cause is silent. +pub(crate) fn objects_filter_zone_is_unambiguous(filter: &TargetFilter) -> bool { + match filter { + // CR 601.2b: each disjunct is its OWN domain, so a zone-free disjunct + // means the battlefield (CR 110.1) and genuinely conflicts with a + // zone-bearing sibling. `None` participates in the comparison. + TargetFilter::Or { filters } => { + if !filters.iter().all(objects_filter_zone_is_unambiguous) { + return false; + } + let mut zones = filters.iter().map(TargetFilter::extract_in_zone); + match zones.next() { + None => true, + Some(first) => zones.all(|zone| zone == first), + } + } + // An `And` is ONE domain intersected, not two: a zone-free conjunct adds + // a constraint ("creature") to whatever zone its sibling names, rather + // than contributing a second population. So `None` members are IGNORED + // and only two DISTINCT named zones conflict — and such a conjunction is + // empty anyway, since an object occupies one zone (CR 400.1). + // + // Comparing `None` here (as this arm used to, sharing the `Or` path) + // rejected EVERY conjunction that pairs a zone-bearing member with a + // zone-free constraint — the `And[, Typed{…}]` shape, of + // which `linked_exile_owned_filter`'s `And[ExiledBySource, + // Typed{Owned{You}}]` is the built example. That particular filter is + // reached through the craft head, which returns before this guard runs, + // so the false-reject is latent rather than card-visible today; it would + // bite the first such conjunction that arrives via the generic + // population grammar. + TargetFilter::And { filters } => { + if !filters.iter().all(objects_filter_zone_is_unambiguous) { + return false; + } + let mut named = filters.iter().filter_map(TargetFilter::extract_in_zone); + match named.next() { + None => true, + Some(first) => named.all(|zone| zone == first), + } + } + TargetFilter::Not { filter } => objects_filter_zone_is_unambiguous(filter), + // A `Typed` leaf carries at most one `InZone`, so it names one domain. + TargetFilter::Typed(_) => true, + // Every remaining variant is a LEAF: it denotes at most one zone by + // construction, so it cannot be INTERNALLY ambiguous — ambiguity is a + // property of composites. Enumerated rather than defaulted so that a + // future variant denoting MULTIPLE zones has to be classified here; the + // old `_ => true` would have called it unambiguous with no compile + // error, which is the fail-open direction this guard exists to close. + TargetFilter::None + | TargetFilter::Any + | TargetFilter::Player + | TargetFilter::Controller + | TargetFilter::SourceController + | TargetFilter::ControllerAndControlledPermanents { .. } + | TargetFilter::Opponent + | TargetFilter::SelfRef + | TargetFilter::GrantingObject + | TargetFilter::SourceOrPaired + | TargetFilter::StackAbility { .. } + | TargetFilter::StackSpell + | TargetFilter::SpecificObject { .. } + | TargetFilter::SpecificPlayer { .. } + | TargetFilter::PlayerWhoChoseLabel { .. } + | TargetFilter::Neighbor { .. } + | TargetFilter::ScopedPlayer + | TargetFilter::AttachedTo + | TargetFilter::LastCreated + | TargetFilter::LastRevealed + | TargetFilter::LastZoneChanged + | TargetFilter::CostPaidObject + | TargetFilter::ChosenCard + | TargetFilter::TrackedSet { .. } + | TargetFilter::TrackedSetFiltered { .. } + | TargetFilter::ExiledBySource + | TargetFilter::ExiledCardByIndex { .. } + | TargetFilter::TriggeringSpellController + | TargetFilter::TriggeringSpellOwner + | TargetFilter::TriggeringPlayer + | TargetFilter::TriggeringSource + | TargetFilter::EventTarget + | TargetFilter::TriggeringSourceController + | TargetFilter::ParentTarget + | TargetFilter::ParentTargetSlot { .. } + | TargetFilter::ParentTargetController + | TargetFilter::ParentTargetOwner + | TargetFilter::SourceChosenPlayer + | TargetFilter::OriginalController + | TargetFilter::OriginalSource + | TargetFilter::PostReplacementSourceController + | TargetFilter::PostReplacementDamageSource + | TargetFilter::PostReplacementDamageTarget + | TargetFilter::PostReplacementDamageTargetOwner + | TargetFilter::DefendingPlayer + | TargetFilter::HasChosenName + | TargetFilter::ChosenDamageSource { .. } + | TargetFilter::Named { .. } + | TargetFilter::Owner + | TargetFilter::AllPlayers => true, + } +} + +/// CR 601.2a + CR 112.1: the per-turn cast journal as a population — +/// "\[\ \]spell\[s\] you\['ve\] cast this turn". +/// +/// The noun phrase is located by its VERB phrase (a typed separator, not a +/// verbatim whole-clause match), then the qualifier is read by the shared +/// spell-history filter grammar, so this arm and `QuantityRef::SpellsCastThisTurn` +/// name the same population by the same rules rather than by two drifting +/// readings. A qualifier that grammar rejects makes the arm DECLINE, which is +/// what stops "permanents you control and spells" from being swallowed as a +/// journal noun. +fn parse_turn_journal_source(input: &str) -> OracleResult<'_, CardTypeSetSource> { + let (rest, noun) = alt(( + terminated( + take_until::<_, _, OracleError<'_>>(" you've cast this turn"), + tag(" you've cast this turn"), + ), + terminated( + take_until::<_, _, OracleError<'_>>(" you cast this turn"), + tag(" you cast this turn"), + ), + )) + .parse(input)?; + // A bare spell noun is the unfiltered journal, mirroring + // `parse_spell_history_clause`'s bare-noun contract. + let filter = match noun.trim() { + "spell" | "spells" => None, + qualified => Some( + super::condition::parse_spell_history_filter(qualified) + .ok_or_else(|| oracle_err(input))?, + ), + }; + Ok(( + rest, + CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + // CR 109.4: "you've cast" is the ability controller's journal. + scope: CountScope::Controller, + filter, + }, + )) +} + +/// CR 400.1 + CR 607.2a + CR 608.2c: the three `cards …`-prefixed populations, +/// nested under their shared prefix so it is matched once. +fn parse_cards_prefixed_source(input: &str) -> OracleResult<'_, CardTypeSetSource> { + preceded( + tag("cards "), + alt(( + map( + preceded(tag("in "), parse_scoped_zone_ref), + |(zone, scope)| CardTypeSetSource::Zone { zone, scope }, + ), + value( + CardTypeSetSource::ExiledBySource, + preceded(tag("exiled with "), parse_exile_link_self_ref), + ), + parse_tracked_set_this_way_source, + )), + ) + .parse(input) +} + +/// CR 607.2a: the self-reference naming the exile link ("~", "it", "this X"). +fn parse_exile_link_self_ref(input: &str) -> OracleResult<'_, &str> { + alt(( + tag("~"), + tag("it"), + preceded( + tag("this "), + take_while1(|c: char| c.is_ascii_alphabetic() || c == '-'), + ), + )) + .parse(input) +} + +/// CR 608.2c + CR 205.2a: "\ this way" → the cause-filtered chain tracked +/// set. Called with the shared `cards ` prefix already consumed. +fn parse_tracked_set_this_way_source(input: &str) -> OracleResult<'_, CardTypeSetSource> { + let (rest, cause) = alt(( + value(ThisWayCause::Discarded, tag("discarded")), + value(ThisWayCause::Exiled, tag("exiled")), + value(ThisWayCause::Milled, tag("milled")), + value(ThisWayCause::Destroyed, tag("destroyed")), + value(ThisWayCause::Sacrificed, tag("sacrificed")), + )) + .parse(input)?; + let (rest, _) = tag(" this way").parse(rest)?; Ok(( rest, - QuantityRef::DistinctCardTypes { - source: CardTypeSetSource::Zone { zone, scope }, + CardTypeSetSource::TrackedSet { + caused_by: Some(cause), }, )) } +/// CR 109.2: the `Objects` population arm — a type phrase read with the head's +/// own grammar, to the head's own extent. +fn parse_objects_source( + input: &str, + grammar: TypePhraseGrammar, + extent: ObjectsSourceExtent, +) -> OracleResult<'_, CardTypeSetSource> { + // Grandfathered structural punctuation cleanup (not dispatch), preserved from + // the per-head combinators this arm replaces. + let type_text = input.trim_end_matches('.').trim_end_matches(','); + let (filter, remainder) = match grammar { + // `(filter, remainder)`, INFALLIBLE — never transpose with the Strict arm. + TypePhraseGrammar::Legacy => parse_type_phrase(type_text), + // `OracleResult` = `(remainder, filter)`. + TypePhraseGrammar::Strict => { + let (rem, filter) = super::target::parse_type_phrase(type_text)?; + (filter, rem) + } + }; + // Retained from the per-head combinators this arm replaces. + if matches!(filter, TargetFilter::Any) { + return Err(oracle_err(input)); + } + // BOTH grammars. The colours head already carried this guard; the card-type + // and subtype heads relied on their whole-clause remainder check instead, + // which is not available in `UnionMember` extent. It is load-bearing there: + // Legacy's infallible failure shape is an EMPTY `TypedFilter` plus the WHOLE + // input, so without this a union member would "match" while consuming + // nothing and contributing an empty population. + if !quantity_filter_has_meaningful_content(&filter) { + return Err(oracle_err(input)); + } + // CR 400.1 + CR 109.2: both grammars, both extents — a partially + // zone-constrained fold has no single correct zone list, so it would drop + // its unconstrained branch. See `objects_filter_zone_is_unambiguous`. + if !objects_filter_zone_is_unambiguous(&filter) { + return Err(oracle_err(input)); + } + match extent { + ObjectsSourceExtent::WholeClause => { + if !remainder.trim().is_empty() { + return Err(oracle_err(input)); + } + } + ObjectsSourceExtent::UnionMember => { + if grammar == TypePhraseGrammar::Strict && !filter_is_population_anchored(&filter) { + return Err(oracle_err(input)); + } + } + } + // `type_text` is a leading slice of `input` (only trailing `.`/`,` trimmed). + // The consumed prefix is whatever `type_text` has in front of `remainder` — + // derived by STRIPPING the remainder rather than by subtracting lengths. + // + // The two grammars establish that relationship differently, and only one of + // them guarantees it. `Strict` returns a nom remainder, which is a genuine + // byte suffix. `Legacy` hand-builds its `(filter, remainder)` pair, and no + // signature or contract says the remainder is a suffix of what it was given. + // Under length subtraction a re-derived or trimmed `Legacy` remainder either + // panics on underflow or, worse, silently yields a wrong offset that + // over-consumes the population. `strip_suffix` fails CLOSED instead: no + // suffix relationship, no source. + // Nothing here consumes input or decides a branch: both grammars have + // already run, and this only measures how much of `type_text` they took. A + // combinator cannot express the question, because the text was read by a + // foreign (Legacy) reader whose returned remainder is the only evidence of + // its own consumption. + // allow-noncombinator: structural offset derivation from an already-parsed remainder, not parsing dispatch. + let Some(consumed) = type_text.strip_suffix(remainder) else { + return Err(oracle_err(input)); + }; + Ok(( + &input[consumed.len()..], + CardTypeSetSource::Objects { filter }, + )) +} + +/// CR 109.2 + CR 400.1 + CR 601.2a: the single-source population grammar — one +/// arm per population, nested by prefix. +/// +/// The journal arm is ordered BEFORE the objects arm so "noncreature spells +/// you've cast this turn" is not mis-consumed as a type phrase. +fn parse_source_arms( + input: &str, + grammar: TypePhraseGrammar, + extent: ObjectsSourceExtent, +) -> OracleResult<'_, CardTypeSetSource> { + alt(( + parse_cards_prefixed_source, + parse_turn_journal_source, + |i| parse_objects_source(i, grammar, extent), + )) + .parse(input) +} + +/// CR 109.2: the population conjunction. Longest-first so `" and/or "` is not +/// mis-split by `" and "`. +fn parse_population_conjunction(input: &str) -> OracleResult<'_, ()> { + value((), alt((tag(" and/or "), tag(" and ")))).parse(input) +} + +/// CR 608.2c: the end of an "among …" clause. A `peek`, so the remainder is left +/// for the caller — a sentence-continuation `" and "` must stay parseable. +fn parse_clause_terminal(input: &str) -> OracleResult<'_, ()> { + value((), peek(alt((eof, tag("."), tag(","))))).parse(input) +} + +/// CR 109.2: the population grammar, in two tiers. +/// +/// The UNION tier is tried first (longest match): two or more population members +/// joined by `" and "` / `" and/or "`, anchored by a clause terminal. If it does +/// not form, the single-source tier is byte-for-byte the grammar each head had +/// before, including its partial-consumption behavior — which is what keeps a +/// sentence-continuation `" and "` (the goyf family's "… and its toughness is +/// equal to that number plus 1") returned to the caller instead of eaten. +fn parse_characteristic_set_source_list( + input: &str, + grammar: TypePhraseGrammar, +) -> OracleResult<'_, CardTypeSetSource> { + alt(( + map_res( + terminated( + nom::combinator::verify( + separated_list1(parse_population_conjunction, move |i| { + parse_source_arms(i, grammar, ObjectsSourceExtent::UnionMember) + }), + |members: &Vec| members.len() >= 2, + ), + parse_clause_terminal, + ), + |members| CardTypeSetSource::any_of(members).ok_or(()), + ), + move |i| parse_source_arms(i, grammar, ObjectsSourceExtent::WholeClause), + )) + .parse(input) +} + +/// CR 205.2a: "card type\[s\] among \" → +/// [`QuantityRef::DistinctCardTypes`]. +/// +/// One combinator over the shared population grammar, replacing the three +/// per-population heads (`… among cards in `, `… among cards exiled with +/// ~`, `… among `) that had drifted into a product form. Reads with +/// [`TypePhraseGrammar::Legacy`], the grammar this head has always used. +fn parse_distinct_card_types_among(input: &str) -> OracleResult<'_, QuantityRef> { + let (rest, _) = tag("card type").parse(input)?; + let (rest, _) = opt(tag("s")).parse(rest)?; + let (rest, _) = tag(" among ").parse(rest)?; + let (rest, source) = parse_characteristic_set_source_list(rest, TypePhraseGrammar::Legacy)?; + Ok((rest, QuantityRef::DistinctCardTypes { source })) +} + fn zone_ref_to_zone(zone: ZoneRef) -> Zone { match zone { ZoneRef::Graveyard => Zone::Graveyard, @@ -2382,73 +2878,26 @@ fn parse_distinct_permanent_types_in_zone(input: &str) -> OracleResult<'_, Quant )) } -fn parse_distinct_card_types_exiled_with_source(input: &str) -> OracleResult<'_, QuantityRef> { - let (rest, _) = tag("card type").parse(input)?; - let (rest, _) = opt(tag("s")).parse(rest)?; - let (rest, _) = tag(" among cards exiled with ").parse(rest)?; - let (rest, _) = alt(( - tag("~"), - tag("it"), - preceded( - tag("this "), - take_while1(|c: char| c.is_ascii_alphabetic() || c == '-'), - ), - )) - .parse(rest)?; - Ok(( - rest, - QuantityRef::DistinctCardTypes { - source: CardTypeSetSource::ExiledBySource, - }, - )) -} - -fn parse_distinct_card_types_among_objects(input: &str) -> OracleResult<'_, QuantityRef> { - let (rest, _) = tag("card type").parse(input)?; - let (rest, _) = opt(tag("s")).parse(rest)?; - let (rest, _) = tag(" among ").parse(rest)?; - let type_text = rest.trim_end_matches('.').trim_end_matches(','); - let (filter, remainder) = parse_type_phrase(type_text); - if matches!(filter, TargetFilter::Any) || !remainder.trim().is_empty() { - return Err(nom::Err::Error(nom::error::Error::new( - input, - nom::error::ErrorKind::Fail, - ))); - } - let consumed = remainder.as_ptr() as usize - input.as_ptr() as usize; - Ok(( - &input[consumed..], - QuantityRef::DistinctCardTypes { - source: CardTypeSetSource::Objects { filter }, - }, - )) -} - -/// CR 608.2c + CR 205.2a: "card type[s] among cards this way" -> distinct -/// card types among the chain tracked set, cause-filtered to (Occult Epiphany #3307). +/// CR 608.2c + CR 205.2a: "card type[s] among cards \ this way" → distinct +/// card types among the chain tracked set, cause-filtered to \ (Occult +/// Epiphany #3307). +/// +/// DELIBERATELY NOT merged into [`parse_distinct_card_types_among`]. Its two +/// external callers — `oracle_effect::token`'s "for each … this way" token +/// context and `oracle_quantity`'s `TrackedSetSize` fallback chain — both +/// deliberately restrict the source axis to the tracked set and both gate on +/// whole consumption. Repointing this symbol at the merged combinator would +/// silently give both call sites `Zone` / `Objects` / `TurnJournal` / `AnyOf` +/// sources inside a "this way" context, changing what a token count means. +/// Preserving the NAME is insufficient; the narrow CONTRACT is the point. pub(crate) fn parse_distinct_card_types_among_tracked_set( input: &str, ) -> OracleResult<'_, QuantityRef> { let (rest, _) = tag("card type").parse(input)?; let (rest, _) = opt(tag("s")).parse(rest)?; let (rest, _) = tag(" among cards ").parse(rest)?; - let (rest, cause) = alt(( - value(ThisWayCause::Discarded, tag("discarded")), - value(ThisWayCause::Exiled, tag("exiled")), - value(ThisWayCause::Milled, tag("milled")), - value(ThisWayCause::Destroyed, tag("destroyed")), - value(ThisWayCause::Sacrificed, tag("sacrificed")), - )) - .parse(rest)?; - let (rest, _) = tag(" this way").parse(rest)?; - Ok(( - rest, - QuantityRef::DistinctCardTypes { - source: CardTypeSetSource::TrackedSet { - caused_by: Some(cause), - }, - }, - )) + let (rest, source) = parse_tracked_set_this_way_source(rest)?; + Ok((rest, QuantityRef::DistinctCardTypes { source })) } /// CR 205.3 + CR 604.3: "different subtype[s] [other than creature types] among @@ -2471,39 +2920,12 @@ fn parse_distinct_subtypes_among(input: &str) -> OracleResult<'_, QuantityRef> { }) .parse(rest)?; let (rest, _) = tag(" among ").parse(rest)?; - // CR 400.1: zone form ("cards in ") vs CR 109.2: object form - // (""). Zone form is tried first so "cards in …" is not - // mis-consumed by the generic type-phrase reader. - let (rest, source) = alt(( - map( - preceded(tag("cards in "), parse_scoped_zone_ref), - |(zone, scope)| CardTypeSetSource::Zone { zone, scope }, - ), - parse_distinct_subtypes_objects_source, - )) - .parse(rest)?; + // CR 400.1 / CR 109.2 / CR 601.2a: the shared population grammar. Reads with + // `TypePhraseGrammar::Legacy`, the grammar this head has always used. + let (rest, source) = parse_characteristic_set_source_list(rest, TypePhraseGrammar::Legacy)?; Ok((rest, QuantityRef::DistinctSubtypes { source, exclude })) } -/// CR 109.2: object-set source for [`parse_distinct_subtypes_among`] — mirrors -/// [`parse_distinct_card_types_among_objects`]'s type-phrase consumption so -/// "different subtypes among " shares one `Objects { filter }` reading. -fn parse_distinct_subtypes_objects_source(input: &str) -> OracleResult<'_, CardTypeSetSource> { - let type_text = input.trim_end_matches('.').trim_end_matches(','); - let (filter, remainder) = parse_type_phrase(type_text); - if matches!(filter, TargetFilter::Any) || !remainder.trim().is_empty() { - return Err(nom::Err::Error(nom::error::Error::new( - input, - nom::error::ErrorKind::Fail, - ))); - } - // `type_text` is a leading slice of `input` (only trailing `.`/`,` trimmed) and - // `remainder` is a tail of `type_text`, so the consumed prefix length is the - // difference of their lengths — no pointer arithmetic needed. - let consumed = type_text.len() - remainder.len(); - Ok((&input[consumed..], CardTypeSetSource::Objects { filter })) -} - /// CR 122.1: Parse "different kind[s] of counters {on|among} " after /// "the number of" → [`QuantityRef::DistinctCounterKindsAmong`]. /// @@ -3935,8 +4357,15 @@ fn parse_distinct_quality_among_objects(input: &str) -> OracleResult<'_, Quantit )) } -// CR 105.1 + CR 109.1: "color among [object filter]" counts distinct colors +// CR 105.1 + CR 105.2: "color among [object filter]" counts distinct colors // among matching objects, not the number of matching objects. +// +// DELIBERATELY still reads with the LEGACY type-phrase grammar, which is what +// this for-each head has always used (Faeburrow Elder, Chromatic Orrery, Soul of +// Ravnica, Sisay, Conqueror's Flail, …). It is a separate head from +// `parse_distinct_colors_among_tail` and is not migrated onto the shared +// population grammar here: no card spells a union or a journal after "for each +// color among", so widening it would be an untested grammar change. fn parse_for_each_distinct_colors_among_permanents(input: &str) -> OracleResult<'_, QuantityRef> { let (rest, _) = tag("color among ").parse(input)?; let (filter, remainder) = parse_type_phrase(rest); @@ -3949,7 +4378,12 @@ fn parse_for_each_distinct_colors_among_permanents(input: &str) -> OracleResult< nom::error::ErrorKind::Fail, ))); } - Ok(("", QuantityRef::DistinctColorsAmongPermanents { filter })) + Ok(( + "", + QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { filter }, + }, + )) } pub(crate) fn parse_for_each_clause_ref_with_context<'a>( @@ -4076,7 +4510,9 @@ fn parse_for_each_clause_ref_with_they_controller( parse_object_name_word_count_for_each, parse_object_typeline_component_count_for_each, parse_mana_symbols_in_object_mana_cost_for_each, - parse_distinct_card_types_in_zone, + // CR 205.2a: "for each card type among " — the same + // population grammar the "the number of …" head uses. + parse_distinct_card_types_among, parse_foretold_cards_owned_in_exile, parse_zone_card_count, parse_for_each_attached_to_source, @@ -5814,6 +6250,318 @@ mod tests { ); } + // ----------------------------------------------------------------------- + // CR 109.2 population grammar — union tier, per-head grammar pinning, and + // the guards that keep each hazard from becoming a silent misparse. + // ----------------------------------------------------------------------- + + /// Row 1/6 (parse shape). First Family: "the number of colors among + /// permanents you control and spells you've cast this turn" must be a set + /// UNION over a live census and the cast journal — the exact misparse this + /// change fixes (both slots used to bind `SpellsCastThisTurn`, a count of + /// SPELLS, dropping the colour aggregation and the permanent population). + #[test] + fn first_family_colors_among_permanents_and_cast_journal_is_a_union() { + let (rest, qty) = parse_quantity_ref( + "the number of colors among permanents you control and spells you've cast this turn", + ) + .expect("First Family's where-X clause must parse"); + assert_eq!(rest, ""); + let QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::AnyOf { sources }, + } = qty + else { + panic!("expected DistinctColorsAmong{{AnyOf}}, got {qty:?}"); + }; + assert_eq!(sources.len(), 2, "exactly two populations: {sources:?}"); + match &sources[0] { + CardTypeSetSource::Objects { + filter: TargetFilter::Typed(tf), + } => { + assert_eq!(tf.type_filters, vec![TypeFilter::Permanent]); + assert_eq!(tf.controller, Some(ControllerRef::You)); + } + other => panic!("member 0 must be permanents you control, got {other:?}"), + } + assert_eq!( + sources[1], + CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: CountScope::Controller, + filter: None, + }, + "member 1 must be the unfiltered controller cast journal" + ); + } + + /// Row 6. Happily Ever After's conjunct-2 FRAGMENT (the card itself stays + /// `Unimplemented` — its serial-comma intervening-if is a separate, deferred + /// gap). Exercises the `and/or` separator and a battlefield-object ∪ + /// graveyard-card member mix, neither of which First Family covers. + #[test] + fn card_types_among_permanents_and_or_graveyard_cards_forms_a_union() { + for phrase in [ + "card types among permanents you control and/or cards in your graveyard", + "card types among permanents you control and cards in your graveyard", + ] { + let (rest, qty) = + parse_distinct_card_types_among(phrase).unwrap_or_else(|e| panic!("{phrase}: {e}")); + assert_eq!(rest, "", "{phrase}"); + let QuantityRef::DistinctCardTypes { + source: CardTypeSetSource::AnyOf { sources }, + } = qty + else { + panic!("{phrase}: expected AnyOf, got {qty:?}"); + }; + assert_eq!(sources.len(), 2, "{phrase}: {sources:?}"); + assert!( + matches!( + &sources[0], + CardTypeSetSource::Objects { + filter: TargetFilter::Typed(tf) + } if tf.controller == Some(ControllerRef::You) + ), + "{phrase}: member 0 must be permanents you control, got {:?}", + sources[0] + ); + assert_eq!( + sources[1], + CardTypeSetSource::Zone { + zone: ZoneRef::Graveyard, + scope: CountScope::Controller, + }, + "{phrase}: member 1 must be your graveyard" + ); + } + } + + /// Row 15. The intra-type-phrase `" and "` must never be mis-split — and the + /// guard that declines it is DIFFERENT under each grammar, so both halves are + /// asserted and each fails only if ITS OWN guard is removed. + /// + /// Legacy: `TYPE_SEPARATORS` FOLDS the phrase into one `Or[..]` and consumes + /// it whole, so arity is 1 and `verify(len >= 2)` declines — population + /// anchoring never runs. Strict: `parse_type_list` joins on `" or "` only, so + /// member 0 would be a bare unanchored `Creature` and + /// `filter_is_population_anchored` is what declines. + #[test] + fn union_tier_never_splits_an_intra_type_phrase_and() { + const PHRASE: &str = "creatures and planeswalkers they control"; + + let legacy = parse_characteristic_set_source_list(PHRASE, TypePhraseGrammar::Legacy); + assert!( + !matches!(legacy, Ok((_, CardTypeSetSource::AnyOf { .. }))), + "Legacy folds the conjunction into one type union (arity 1), got {legacy:?}" + ); + + let strict = parse_characteristic_set_source_list(PHRASE, TypePhraseGrammar::Strict); + assert!( + !matches!(strict, Ok((_, CardTypeSetSource::AnyOf { .. }))), + "Strict must refuse an unanchored bare-type-word member, got {strict:?}" + ); + } + + /// Row 15, mechanism pin for the Strict half: a bare type word is NOT a + /// population, an anchored one is. Removing `filter_is_population_anchored` + /// flips the first assertion. + #[test] + fn population_anchoring_distinguishes_a_type_from_a_population() { + let bare = super::super::target::parse_type_phrase("creatures") + .expect("strict grammar parses a bare type word") + .1; + assert!( + !filter_is_population_anchored(&bare), + "a bare type word names a TYPE, not a population: {bare:?}" + ); + let anchored = super::super::target::parse_type_phrase("creatures you control") + .expect("strict grammar parses a controller-anchored phrase") + .1; + assert!( + filter_is_population_anchored(&anchored), + "a controller suffix anchors the population: {anchored:?}" + ); + } + + /// Row 17. An anaphoric population ("colors among those creatures" — General + /// Tazri) must stay an HONEST GAP, never a confident count over an + /// unrebindable sentinel. + /// + /// MEASURED CORRECTION to the plan: neither `parse_type_phrase` carries the + /// anaphor grammar — that lives in `parse_target`, not in either type-phrase + /// reader. Strict `Err`s on "those creatures"; Legacy returns an EMPTY + /// `TypedFilter` plus the whole input. So both refuse, but by different + /// mechanisms, and Legacy's refusal is the weaker one (a silent empty filter + /// that only a downstream remainder or emptiness check catches). The second + /// half pins that measured asymmetry so a future "let's unify the grammars" + /// change has to confront it rather than assume equivalence. + #[test] + fn an_anaphoric_population_is_refused_by_every_characteristic_head() { + assert!( + parse_distinct_colors_among_tail("colors among those creatures").is_err(), + "the colours head must decline an anaphoric population (General Tazri)" + ); + assert!( + parse_distinct_card_types_among("card types among those cards").is_err(), + "the card-type head must decline an anaphoric population too" + ); + + // Measured grammar asymmetry: Strict fails; Legacy silently yields an + // empty filter and consumes nothing. + assert!( + super::super::target::parse_type_phrase("those creatures").is_err(), + "Strict rejects an anaphor outright" + ); + let (legacy_filter, legacy_rest) = parse_type_phrase("those creatures"); + assert_eq!( + legacy_rest, "those creatures", + "Legacy's infallible failure consumes nothing" + ); + assert!( + !quantity_filter_has_meaningful_content(&legacy_filter), + "Legacy's failure shape is an EMPTY TypedFilter, not TargetFilter::Any: {legacy_filter:?}" + ); + assert!( + !matches!(legacy_filter, TargetFilter::Any), + "the historical `Any` guard does NOT catch Legacy's failure shape: {legacy_filter:?}" + ); + } + + /// Row 18. A folded CROSS-ZONE type union is refused rather than silently + /// single-zoned: `TargetFilter::extract_in_zone` returns the FIRST member's + /// zone for an `Or`, so the other leg would be scanned in the wrong zone and + /// dropped with no diagnostic. + /// + /// The sibling assertion is what keeps this from being over-broad: a + /// same-zone (here, zone-free) fold is unambiguous and still accepted, so + /// every current card is unaffected. + #[test] + fn objects_source_refuses_an_ambiguous_cross_zone_fold() { + let cross_zone = TargetFilter::Or { + filters: vec![ + TargetFilter::Typed(TypedFilter::new(TypeFilter::Creature)), + TargetFilter::Typed(TypedFilter::new(TypeFilter::Card).properties(vec![ + FilterProp::InZone { + zone: Zone::Graveyard, + }, + ])), + ], + }; + assert!( + !objects_filter_zone_is_unambiguous(&cross_zone), + "a battlefield ∪ graveyard fold has no single zone: {cross_zone:?}" + ); + + let same_zone = TargetFilter::Or { + filters: vec![ + TargetFilter::Typed( + TypedFilter::new(TypeFilter::Creature).controller(ControllerRef::TargetPlayer), + ), + TargetFilter::Typed( + TypedFilter::new(TypeFilter::Planeswalker) + .controller(ControllerRef::TargetPlayer), + ), + ], + }; + assert!( + objects_filter_zone_is_unambiguous(&same_zone), + "the Blot Out family's fold is zone-unambiguous and must stay accepted" + ); + } + + /// Row 16. A trailing `" and "` that continues the SENTENCE is not a + /// population conjunction: the goyf family's toughness rider and the + /// delirium activation restriction must both still be returned to the caller. + #[test] + fn sentence_continuation_and_is_returned_to_the_caller() { + for (phrase, expected_rest) in [ + ( + "card types among cards in all graveyards and its toughness is equal to that number plus 1", + " and its toughness is equal to that number plus 1", + ), + ( + "card types among cards in your graveyard and only as a sorcery", + " and only as a sorcery", + ), + ] { + let (rest, qty) = + parse_distinct_card_types_among(phrase).unwrap_or_else(|e| panic!("{phrase}: {e}")); + assert_eq!(rest, expected_rest, "{phrase}"); + assert!( + matches!( + qty, + QuantityRef::DistinctCardTypes { + source: CardTypeSetSource::Zone { .. } + } + ), + "{phrase}: expected a single zone source, got {qty:?}" + ); + } + } + + /// Row 16, second half. An object population that does not consume its whole + /// clause is an ERROR, never a truncated source. + #[test] + fn card_type_head_refuses_a_truncated_object_population() { + assert!( + parse_distinct_card_types_among("card types among creatures you control blah").is_err(), + "an unconsumed tail must fail the head, not truncate the population" + ); + } + + /// Rows 4/5 (parse shape). The cast journal as a population, unfiltered + /// (April O'Neil) and narrowed (Hurkyl). + #[test] + fn card_types_among_the_cast_journal_parses_filtered_and_unfiltered() { + let (rest, qty) = + parse_distinct_card_types_among("card type among spells you've cast this turn") + .expect("April O'Neil's for-each source must parse"); + assert_eq!(rest, ""); + assert_eq!( + qty, + QuantityRef::DistinctCardTypes { + source: CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: CountScope::Controller, + filter: None, + }, + } + ); + + let (rest, qty) = parse_distinct_card_types_among( + "card type among noncreature spells you've cast this turn", + ) + .expect("Hurkyl's narrowed journal source must parse"); + assert_eq!(rest, ""); + let QuantityRef::DistinctCardTypes { + source: + CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: CountScope::Controller, + filter: Some(filter), + }, + } = qty + else { + panic!("expected a FILTERED cast journal, got {qty:?}"); + }; + assert!( + !matches!(filter, TargetFilter::Any), + "the noncreature qualifier must survive as a real filter: {filter:?}" + ); + } + + /// The journal arm must DECLINE a noun its qualifier grammar does not + /// recognize, rather than swallowing the aggregation head that precedes it. + /// This is what stops the union tier's member-1 attempt from consuming + /// "permanents you control and spells" as a journal noun. + #[test] + fn turn_journal_arm_declines_an_unrecognized_qualifier() { + assert!( + parse_turn_journal_source("permanents you control and spells you've cast this turn") + .is_err(), + "an aggregation head is not a spell-history qualifier" + ); + } + #[test] fn type_count_on_battlefield_accepts_eof_tail() { let (rest, parsed) = parse_type_count_on_battlefield("other creatures on the battlefield") @@ -6244,10 +6992,12 @@ mod tests { .unwrap(); assert_eq!(rest, ""); match q { - QuantityRef::DistinctColorsAmongPermanents { filter } => { + QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { filter }, + } => { assert_eq!(filter, linked_exile_owned_filter()) } - other => panic!("expected DistinctColorsAmongPermanents, got {other:?}"), + other => panic!("expected DistinctColorsAmong(Objects), got {other:?}"), } } @@ -7775,14 +8525,16 @@ mod tests { parse_quantity_ref("the number of colors among permanents you control").unwrap(); assert_eq!(rest, ""); match q { - QuantityRef::DistinctColorsAmongPermanents { filter } => match filter { + QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { filter }, + } => match filter { TargetFilter::Typed(tf) => { assert_eq!(tf.type_filters, vec![TypeFilter::Permanent]); assert_eq!(tf.controller, Some(ControllerRef::You)); } other => panic!("expected typed permanent filter, got {other:?}"), }, - other => panic!("expected DistinctColorsAmongPermanents, got {other:?}"), + other => panic!("expected DistinctColorsAmong(Objects), got {other:?}"), } } @@ -7791,14 +8543,16 @@ mod tests { let (rest, q) = parse_for_each_clause_ref("color among permanents you control").unwrap(); assert_eq!(rest, ""); match q { - QuantityRef::DistinctColorsAmongPermanents { filter } => match filter { + QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { filter }, + } => match filter { TargetFilter::Typed(tf) => { assert_eq!(tf.type_filters, vec![TypeFilter::Permanent]); assert_eq!(tf.controller, Some(ControllerRef::You)); } other => panic!("expected typed permanent filter, got {other:?}"), }, - other => panic!("expected DistinctColorsAmongPermanents, got {other:?}"), + other => panic!("expected DistinctColorsAmong(Objects), got {other:?}"), } assert!( diff --git a/crates/engine/src/parser/oracle_quantity.rs b/crates/engine/src/parser/oracle_quantity.rs index d31ebe7396..57bcded329 100644 --- a/crates/engine/src/parser/oracle_quantity.rs +++ b/crates/engine/src/parser/oracle_quantity.rs @@ -20,7 +20,7 @@ use std::str::FromStr; use crate::parser::oracle_nom::error::{OracleError, OracleResult}; use nom::branch::alt; use nom::bytes::complete::{tag, take_till1, take_until}; -use nom::combinator::{all_consuming, eof, map, map_opt, opt, peek, rest, value}; +use nom::combinator::{all_consuming, eof, map, map_opt, opt, peek, recognize, rest, value}; use nom::multi::separated_list1; use nom::sequence::{pair, preceded, terminated}; use nom::Parser; @@ -2906,6 +2906,68 @@ fn parse_suspended_card_clause(clause: &str) -> Option { }) } +/// CR 122.1: the counter-kind noun, spelled to match the CANONICAL head in +/// `oracle_nom::quantity::parse_distinct_counter_kinds_among_tail`. +/// +/// The plural sits on KIND, not only on COUNTER: the printed form is "different +/// kinds of counters among …" (Perrie, the Pulverizer — Oracle text verified +/// against Scryfall, not paraphrased). Composing `tag("kind of counter")` with a +/// trailing `opt(tag("s"))`, as this guard used to, recognizes "kind of +/// counters" — a form no card prints — and fails to recognize the one that is +/// printed. +/// +/// The trailing "s" of "counters" is left to the caller's shared `opt(tag("s"))` +/// so every characteristic arm pluralizes through one place. +fn parse_counter_kind_noun(input: &str) -> OracleResult<'_, &str> { + recognize((tag("kind"), opt(tag("s")), tag(" of counter"))).parse(input) +} + +/// CR 105.1 + CR 205.2 + CR 205.3 + CR 122.1: the distinct-characteristic +/// aggregation heads ("colors among …", "card types among …", …). +/// +/// A typed combinator over the already-enumerated characteristic vocabulary, not +/// a string blocklist: each axis is one `alt` arm and the "different" determiner, +/// plural, rider and "among" separators are composed, so a new characteristic is +/// one arm rather than a table of full phrases. +/// +/// DUPLICATION, ACKNOWLEDGED: the canonical heads live in +/// `oracle_nom::quantity`. This is a DETECTION-only restatement — it answers +/// "does a head start here", where the canonical combinators also consume the +/// population — and it has already drifted once, which is how the counter-kind +/// plural came to be wrong. Extracting a shared head-only combinator is the real +/// fix and is deliberately left as follow-up rather than done mid review cycle; +/// until then, a vocabulary change here must be mirrored there. +/// +/// Anchored at position 0 by design. Both production callers hand this a clause +/// whose determiner is already gone — the "the number of " arm strips that +/// prefix before calling, and the "for each " arm never carries one. +fn parse_characteristic_head(input: &str) -> OracleResult<'_, ()> { + value( + (), + ( + // Shared determiner: "different subtypes among", "different kinds of + // counters among". Hoisted out of the arms so it cannot be baked into + // one noun and forgotten on the next. + opt(tag("different ")), + alt(( + // Longest-first: "colors" must win over "color". + tag("colors"), + tag("color"), + tag("card type"), + tag("permanent type"), + tag("subtype"), + parse_counter_kind_noun, + )), + opt(tag("s")), + // CR 205.3m: the subtype head's exclusion rider sits between the noun + // and "among". + opt(tag(" other than creature types")), + tag(" among "), + ), + ) + .parse(input) +} + /// CR 400.1 + CR 601.2a: Parse a spell-history count clause into its /// controller scope and an optional characteristic/cast-origin filter. /// @@ -3008,6 +3070,17 @@ fn parse_spell_history_clause( return Some((scope, Some(filter))); } + // CR 608.2c: a noun that still carries an unconsumed aggregation head + // ("colors among …", "card types among …") is NOT a spell-history noun — + // the head belongs to a characteristic-count grammar that owns the whole + // clause. Returning a bare spell count here would silently discard it and + // report a confident count of SPELLS where the card asks for a count of + // COLORS or CARD TYPES (First Family, April O'Neil, Hurkyl). Decline so + // later arms can try. + if parse_characteristic_head(noun).is_ok() { + return None; + } + // Suffix-anchored noun with no recognized qualifier (e.g. an unknown // spell noun that ended the clause): mirror the original arms' contract // of returning the spell-history scope with no filter. @@ -3619,6 +3692,121 @@ mod tests { }; use crate::types::mana::ManaColor; + // ----------------------------------------------------------------------- + // CR 608.2c — the spell-history swallow guard, and the narrow contract of + // the retained tracked-set entry. + // ----------------------------------------------------------------------- + + /// Row 9. `parse_spell_history_clause`'s terminal fallback used to claim ANY + /// clause containing a cast verb phrase, discarding whatever aggregation + /// head preceded it and returning a confident bare spell count. That is the + /// exact mechanism that made First Family, April O'Neil and Hurkyl silently + /// wrong. This test drives the helper DIRECTLY, so it verifies the GUARD and + /// not merely the combinator ordering that now claims those phrases earlier. + #[test] + fn spell_history_clause_declines_an_unconsumed_characteristic_head() { + for clause in [ + "colors among permanents you control and spells you've cast this turn", + "card type among spells you've cast this turn", + "card types among noncreature spells you've cast this turn", + "different subtypes among spells you've cast this turn", + "different subtypes other than creature types among spells you've cast this turn", + // The counter-kind head in its PRINTED form. This row read "kind of + // counter among …" — a spelling no card uses, which the old + // `tag("kind of counter") + opt("s")` composition happened to accept + // while rejecting the real one. The test agreed with the bug and so + // could not catch it; both are now spelled the way Perrie, the + // Pulverizer prints it (verified against Scryfall). + "different kinds of counters among spells you've cast this turn", + // Plural-tolerant siblings of the same head, so the arm cannot + // regress to matching exactly one hard-coded spelling. + "different kind of counter among spells you've cast this turn", + "kinds of counters among spells you've cast this turn", + ] { + assert_eq!( + parse_spell_history_clause(clause, CountScope::Controller), + None, + "an aggregation head must NOT be swallowed as a bare spell count: {clause:?}" + ); + } + } + + /// Row 10. The guard is narrow: every bare and qualified spell-history form + /// the 20+ cards in this class rely on keeps its pre-change reading. + #[test] + fn spell_history_clause_keeps_its_bare_and_qualified_readings() { + for clause in [ + "spells you've cast this turn", + "spell you've cast this turn", + "spells you cast this turn", + ] { + assert_eq!( + parse_spell_history_clause(clause, CountScope::Controller), + Some((CountScope::Controller, None)), + "bare spell-history forms must be unchanged: {clause:?}" + ); + } + + for clause in [ + "instant and sorcery spell you've cast this turn", + "noncreature spell you've cast this turn", + "spells you've cast this turn from anywhere other than your hand", + ] { + let parsed = parse_spell_history_clause(clause, CountScope::Controller); + let Some((scope, Some(filter))) = parsed else { + panic!( + "qualified spell-history form must keep its filter: {clause:?} -> {parsed:?}" + ) + }; + assert_eq!(scope, CountScope::Controller, "{clause:?}"); + assert!( + !matches!(filter, TargetFilter::Any), + "{clause:?} must keep a real filter, got {filter:?}" + ); + } + } + + /// Row 19, call site 1 of 2 (`oracle_quantity`'s "this way" `TrackedSetSize` + /// fallback chain). Consolidating the three card-type combinators must NOT + /// widen this site's source axis: only a tracked set may reach it. + /// + /// Preserving the SYMBOL `parse_distinct_card_types_among_tracked_set` is + /// insufficient — the CONTRACT must stay narrow, which is what this asserts + /// by feeding non-tracked-set populations directly to the entry the site + /// calls. + #[test] + fn the_this_way_tracked_set_entry_stays_narrow() { + use crate::parser::oracle_nom::quantity::parse_distinct_card_types_among_tracked_set as narrow; + + // Positive control (Occult Epiphany): the tracked set still routes. + let (rest, qty) = narrow("card type among cards discarded this way") + .expect("the tracked-set reading must survive consolidation"); + assert_eq!(rest, ""); + assert!( + matches!( + qty, + QuantityRef::DistinctCardTypes { + source: CardTypeSetSource::TrackedSet { .. } + } + ), + "expected a TrackedSet source, got {qty:?}" + ); + + // Negatives: every other population must be refused HERE, even though + // the merged head accepts them. + for clause in [ + "card type among cards in your graveyard", + "card type among permanents you control", + "card type among spells you've cast this turn", + "card type among permanents you control and spells you've cast this turn", + ] { + assert!( + narrow(clause).is_err(), + "a '{clause}' population must not reach the 'this way' token context" + ); + } + } + /// DynQty subgroup D / Matrix #1 — the comparative hand-size producer builds the /// exact `PlayerAttribute` AST (Wojek Investigator). Fails iff EDIT 2 is reverted; /// independent of EDIT 1. The full `assert_eq` pins operand scope (Controller, diff --git a/crates/engine/src/parser/oracle_static/tests.rs b/crates/engine/src/parser/oracle_static/tests.rs index 21e159e072..42c28bb5b4 100644 --- a/crates/engine/src/parser/oracle_static/tests.rs +++ b/crates/engine/src/parser/oracle_static/tests.rs @@ -15810,12 +15810,14 @@ fn equipped_creature_gets_dynamic_pt_for_each_color_among_permanents() { assert_eq!(def.mode, StaticMode::Continuous); let expected = QuantityExpr::Ref { - qty: QuantityRef::DistinctColorsAmongPermanents { - filter: TargetFilter::Typed(TypedFilter { - type_filters: vec![TypeFilter::Permanent], - controller: Some(ControllerRef::You), - properties: Vec::new(), - }), + qty: QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { + filter: TargetFilter::Typed(TypedFilter { + type_filters: vec![TypeFilter::Permanent], + controller: Some(ControllerRef::You), + properties: Vec::new(), + }), + }, }, }; assert!( diff --git a/crates/engine/src/parser/oracle_tests.rs b/crates/engine/src/parser/oracle_tests.rs index e83c4c068c..fe9d696cbd 100644 --- a/crates/engine/src/parser/oracle_tests.rs +++ b/crates/engine/src/parser/oracle_tests.rs @@ -18125,8 +18125,11 @@ fn vivid_spell_cost_reduction_uses_distinct_colors_quantity() { mode: CostModifyMode::Reduce, amount: ManaCost::Cost { generic: 1, .. }, dynamic_count: - Some(QuantityRef::DistinctColorsAmongPermanents { - filter: TargetFilter::Typed(tf), + Some(QuantityRef::DistinctColorsAmong { + source: + crate::types::ability::CardTypeSetSource::Objects { + filter: TargetFilter::Typed(tf), + }, }), .. } = &r.statics[0].mode @@ -21532,8 +21535,11 @@ fn activated_draw_for_each_color_among_permanents_uses_distinct_colors_quantity( count: QuantityExpr::Ref { qty: - QuantityRef::DistinctColorsAmongPermanents { - filter: TargetFilter::Typed(tf), + QuantityRef::DistinctColorsAmong { + source: + crate::types::ability::CardTypeSetSource::Objects { + filter: TargetFilter::Typed(tf), + }, }, }, .. diff --git a/crates/engine/src/parser/swallow_check.rs b/crates/engine/src/parser/swallow_check.rs index 805a2f831d..787e4e5b32 100644 --- a/crates/engine/src/parser/swallow_check.rs +++ b/crates/engine/src/parser/swallow_check.rs @@ -2258,10 +2258,15 @@ fn detect_dynamic_qty( // "For each color among permanents you control, add one mana of that color." // // This leg exists BECAUSE the probes above are anchored. Unanchored, they "saw" this - // carrier only by ACCIDENT: `DistinctColorsAmongPermanents` is also a `QuantityRef` - // variant name, so the `ManaProduction` node deserialized as a `QuantityRef` by cross-enum - // collision. Right answer, wrong reason — and the same collision suppressed Boing! and - // Siren's Call. Anchoring removed the accident; this restores the fact, typed. + // carrier only by ACCIDENT: `DistinctColorsAmongPermanents` USED TO BE a `QuantityRef` + // variant name too, so the `ManaProduction` node deserialized as a `QuantityRef` by + // cross-enum collision. Right answer, wrong reason — and the same collision suppressed + // Boing! and Siren's Call. Anchoring removed the accident; this restores the fact, typed. + // + // The collision itself is now GONE: the `QuantityRef` side was renamed to + // `DistinctColorsAmong` when it was parameterized onto `CardTypeSetSource`. That makes + // this leg strictly load-bearing rather than belt-and-braces — an unanchored probe could + // no longer reach this carrier even by accident. // // ONE variant, and that is a MEASURED bound, not a guess: over the full 35,396-face pool, // `DistinctColorsAmongPermanents` is the only `ManaProduction` on a face where the marker diff --git a/crates/engine/src/parser/swallow_evidence.rs b/crates/engine/src/parser/swallow_evidence.rs index 487de62d26..61b260247c 100644 --- a/crates/engine/src/parser/swallow_evidence.rs +++ b/crates/engine/src/parser/swallow_evidence.rs @@ -80,7 +80,10 @@ //! QuantityRef ∩ ParsedCondition = {BattlefieldEntriesThisTurn} //! QuantityRef ∩ ChooseFromZoneConstraint = {DistinctCardTypes} //! QuantityRef ∩ ManaCost = {SelfManaValue} -//! QuantityRef ∩ ManaProduction = {DistinctColorsAmongPermanents} +//! QuantityRef ∩ ManaProduction = {} (was {DistinctColorsAmongPermanents} +//! until the QuantityRef side was renamed +//! to DistinctColorsAmong; the two enums no +//! longer share a variant name) //! QuantityRef ∩ SolveCondition = {ObjectCount} //! QuantityRef ∩ QuantityExpr = {Power} //! ``` @@ -304,7 +307,9 @@ const STATIC_MODE_KEYS: &[&str] = &["mode"]; /// QuantityRef ∩ ParsedCondition BattlefieldEntriesThisTurn /// QuantityRef ∩ ChooseFromZoneConstraint DistinctCardTypes /// QuantityRef ∩ ManaCost SelfManaValue -/// QuantityRef ∩ ManaProduction DistinctColorsAmongPermanents +/// QuantityRef ∩ ManaProduction (was DistinctColorsAmongPermanents; +/// now empty — the QuantityRef side is +/// DistinctColorsAmong) /// QuantityRef ∩ SolveCondition ObjectCount /// QuantityRef ∩ QuantityExpr Power /// ``` diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 77b7535603..00c0572e3f 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -1846,6 +1846,24 @@ pub enum ZoneRef { Hand, } +impl ZoneRef { + /// CR 400.1: The game zone this reference denotes. + /// + /// The single authority for the `ZoneRef` → [`Zone`](crate::types::zones::Zone) + /// mapping. Exhaustive by construction: a new `ZoneRef` variant fails to + /// compile here rather than silently reading as "some other zone" at a call + /// site that pattern-matched only the four it knew about. + pub fn zone(&self) -> crate::types::zones::Zone { + use crate::types::zones::Zone; + match self { + ZoneRef::Graveyard => Zone::Graveyard, + ZoneRef::Exile => Zone::Exile, + ZoneRef::Library => Zone::Library, + ZoneRef::Hand => Zone::Hand, + } + } +} + /// CR 701.10d-f: What aspect to double (counters, life total, or mana pool). /// Used by `Effect::Double` per locked decision D-05. /// DoublePT/DoublePTAll handle CR 701.10a-c (power/toughness) separately. @@ -2286,7 +2304,10 @@ pub enum ManaProduction { /// not colors (CR 105.1), so each of W/U/B/R/G contributes at most once. /// Used by Faeburrow Elder's "{T}: For each color among permanents you /// control, add one mana of that color." Mirrors the structure of - /// `QuantityRef::DistinctColorsAmongPermanents`. + /// [`QuantityRef::DistinctColorsAmong`], which is a DIFFERENT enum and + /// which is parameterized on [`CardTypeSetSource`] because a colour COUNT + /// can read a union or a non-object population (First Family). This mana + /// variant is deliberately NOT parameterized: no mana ability reads either. DistinctColorsAmongPermanents { filter: TargetFilter }, /// CR 106.1 + CR 109.1: Produce N mana of one chosen color from the distinct /// colors present among permanents matching `filter`. Mox Amber class: @@ -5970,7 +5991,36 @@ pub enum ObjectScope { BatchSource, } -/// Source set for counting distinct card types. +/// CR 601.2a: A per-turn action journal — a chronological record of a kind of +/// action taken this turn, cleared at the turn boundary. +/// +/// The parameterization axis for [`CardTypeSetSource::TurnJournal`]. Introduced +/// already parameterized rather than as a bare `SpellsCastThisTurn` leaf so the +/// journal axis cannot grow an X / X′ sibling cluster on +/// `CardTypeSetSource` itself. +/// +/// NEXT MEMBER, ALREADY IDENTIFIED: `PermanentsSacrificed` (Korvold, Gleeful +/// Glutton — "for each card type among permanents you've sacrificed this turn"). +/// BLOCKER: `GameState` has no sacrifice journal. Verified absent — the only +/// per-turn journals today are `spells_cast_this_turn_by_player` and its +/// game-scoped mirror. Adding Korvold costs one variant here plus its state and +/// write site; it costs NOTHING in `CardTypeSetSource`, whose shape absorbs it +/// unchanged. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(tag = "type")] +pub enum TurnJournalKind { + /// CR 601.2a + CR 112.1: spells cast this turn, as recorded in + /// `GameState::spells_cast_this_turn_by_player` at `finalize_cast`. + SpellsCast, +} + +/// Source set (population) whose members' characteristics are counted. +/// +/// CR 109.2 + CR 400.1 + CR 601.2a: the population axis shared by every +/// distinct-characteristic count — card types (CR 205.2), subtypes (CR 205.3), +/// and colors (CR 105.1). The CHARACTERISTIC axis stays partitioned by CR +/// section in `QuantityRef`; this axis names only "the set whose members are +/// read". #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "type")] pub enum CardTypeSetSource { @@ -5989,8 +6039,253 @@ pub enum CardTypeSetSource { #[serde(default, skip_serializing_if = "Option::is_none")] caused_by: Option, }, + /// CR 601.2a + CR 112.1: The members of a per-turn action journal for the + /// scoped players ("spells you've cast this turn"). + /// + /// A resolved spell is no longer an object (CR 400.7), so characteristics + /// come from the snapshot captured when the action was journaled — not from + /// a live object scan. `scope` / `filter` mirror + /// `QuantityRef::SpellsCastThisTurn` so the two name the same population; + /// `filter: None` admits every member. + TurnJournal { + journal: TurnJournalKind, + scope: CountScope, + #[serde(default, skip_serializing_if = "Option::is_none")] + filter: Option, + }, + /// CR 109.2: The union of two or more populations — "among \ and \" / + /// "among \ and/or \". + /// + /// Set union, not arithmetic sum: a member appearing in both contributes its + /// characteristics once (First Family). Contrast + /// `parse_greatest_among_conjunction`, which is allowed to decompose the same + /// surface form into `Max{[Aggregate, Aggregate]}` ONLY because max + /// distributes over union and a distinct-count does not. Named `AnyOf` to + /// match `FilterProp::AnyOf` / `TypeFilter::AnyOf` (set-union-of-alternatives), + /// not `Or` (reserved for boolean condition enums). + /// + /// INVARIANT: at least two members, carried by [`UnionSources`] rather than + /// asserted. A 0- or 1-member union is not a union, and an empty one is + /// actively unsound: `characteristic_source_read`'s fold would return + /// `RwProfile::empty()`, which is FAIL-OPEN for the CR 603.3b ordering gate, + /// and the resolver would return 0 with no diagnostic. + AnyOf { sources: UnionSources }, +} + +/// CR 109.2: the members of a [`CardTypeSetSource::AnyOf`], carrying the +/// at-least-two invariant IN THE TYPE. +/// +/// The `Vec` is private, so the only ways in are [`UnionSources::new`] and +/// `Deserialize` — both of which reject a 0- or 1-member list. This replaces a +/// public `Vec` field guarded by a `debug_assert!`: that assertion compiles out +/// of release builds, and the field let any caller in the crate write +/// `AnyOf { sources: vec![] }` directly, bypassing both the constructor and the +/// serde check. Several already did. An invariant that arbitrary callers can +/// step around is documentation, not an invariant. +/// +/// Derefs to `[CardTypeSetSource]`, so every existing `sources.iter()` / +/// `sources.len()` read is unchanged — the type is a construction gate, not a +/// new collection API. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(transparent)] +pub struct UnionSources(Vec); + +impl UnionSources { + /// The ONLY in-crate constructor. `None` when the arity invariant fails, so + /// a caller cannot get a degenerate union by ignoring an error. + /// + /// Callers that want the collapse-to-single behavior want + /// [`CardTypeSetSource::any_of`], which is written on top of this. + pub fn new(sources: Vec) -> Option { + (sources.len() >= 2).then_some(Self(sources)) + } + + pub fn into_vec(self) -> Vec { + self.0 + } +} + +impl std::ops::Deref for UnionSources { + type Target = [CardTypeSetSource]; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl<'a> IntoIterator for &'a UnionSources { + type Item = &'a CardTypeSetSource; + type IntoIter = std::slice::Iter<'a, CardTypeSetSource>; + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } } +/// CR 109.2: Enforce the arity invariant on load, so a saved game or +/// hand-authored payload cannot smuggle a degenerate union past the constructor. +impl<'de> Deserialize<'de> for UnionSources { + fn deserialize>(deserializer: D) -> Result { + let sources = Vec::::deserialize(deserializer)?; + let len = sources.len(); + UnionSources::new(sources).ok_or_else(|| { + serde::de::Error::custom(format!( + "CardTypeSetSource::AnyOf requires at least 2 sources, got {len}" + )) + }) + } +} + +impl CardTypeSetSource { + /// CR 109.2: Arity-checked [`CardTypeSetSource::AnyOf`] constructor — the + /// only way a union is built. + /// + /// A single member collapses to itself rather than forming a degenerate + /// union; an empty list has no population and yields `None`. + pub fn any_of(mut sources: Vec) -> Option { + match sources.len() { + 0 => None, + 1 => sources.pop(), + _ => UnionSources::new(sources).map(|sources| CardTypeSetSource::AnyOf { sources }), + } + } + + /// CR 400.1 + CR 613.4a: Every zone this population reads. + /// + /// THE single authority for the population-zone axis. Both consumers ask + /// this and nothing else: + /// + /// * **Evaluation** — `game::quantity::visit_characteristic_source` walks + /// exactly these zones to enumerate members. + /// * **Dependency tracking** — [`reads_zone`](Self::reads_zone), which + /// `game::layers::characteristic_source_reads_zone` delegates to, dirties + /// a dependent characteristic when an object crosses one of them. + /// + /// They MUST agree. When they did not, a layer or CDA could retain a stale + /// distinct-characteristic value across a zone transition: the evaluator + /// scanned exile for a craft population (`And[ExiledBySource, …]`, Sunbird + /// Effigy) while the classifier reported that same population as reading no + /// zone at all, so nothing ever re-evaluated it. Splitting the two answers + /// across two functions is what made that divergence possible; one function + /// with two callers is what prevents it. + /// + /// EMPTY means "no zone is explicitly named", which each consumer resolves + /// per [`TargetFilter::population_zones`]: the walk substitutes the + /// battlefield default (CR 110.1), the dependency check does not. Two + /// populations are empty as a POSITIVE claim rather than a fallback, and for + /// them the walk substitutes nothing because it never scans a zone at all: + /// + /// * `TurnJournal` — characteristics come from the snapshot taken when the + /// action was journaled, because a resolved spell is no longer an object + /// (CR 400.7). Nothing re-reads a zone, so no transition can stale it. + /// * `TrackedSet` — membership is by object id and is fixed when the set is + /// published (CR 608.2i — an effect may look back at a previous action's + /// objects, which need not still be where they were); a member moving + /// zones changes neither the set + /// nor the card types its members have (CR 205.2a). + pub fn population_zones(&self) -> Vec { + self.population_zones_checked().0 + } + + /// [`population_zones`](Self::population_zones) plus whether the union walk + /// completed within its depth budget. Split out because the two consumers + /// need OPPOSITE things from a truncated walk: the evaluation walk can only + /// scan what it found, while [`reads_zone`](Self::reads_zone) must answer + /// `true` rather than miss an invalidation. + fn population_zones_checked(&self) -> (Vec, bool) { + let mut out: Vec = Vec::new(); + let complete = self.try_for_each_member(UNION_DEPTH_BUDGET, &mut |leaf| { + for zone in leaf.leaf_population_zones() { + if !out.contains(&zone) { + out.push(zone); + } + } + }); + (out, complete) + } + + /// The zones ONE non-union population reads. `AnyOf` is handled by + /// [`try_for_each_member`](Self::try_for_each_member), which is the only + /// caller and never hands a union here. + fn leaf_population_zones(&self) -> Vec { + match self { + CardTypeSetSource::Zone { zone, .. } => vec![zone.zone()], + // CR 607.2a + CR 406.6: a linked-exile pool lives in exile. + CardTypeSetSource::ExiledBySource => vec![crate::types::zones::Zone::Exile], + CardTypeSetSource::Objects { filter } => filter.population_zones(), + // Not zone reads — see the doc comment on `population_zones`. + CardTypeSetSource::TrackedSet { .. } | CardTypeSetSource::TurnJournal { .. } => { + Vec::new() + } + // Unreachable through the walker; empty rather than a panic so a + // future direct caller degrades instead of crashing a game. + CardTypeSetSource::AnyOf { .. } => Vec::new(), + } + } + + /// CR 613.4a: Does this population read `zone`? + /// + /// The dependency-tracking half of [`population_zones`](Self::population_zones), + /// kept as one call so a caller cannot accidentally ask a narrower question. + /// + /// A truncated union walk answers `true`: an over-report costs one redundant + /// layer recompute, an under-report strands a stale characteristic, and only + /// one of those is a correctness bug. + pub fn reads_zone(&self, zone: crate::types::zones::Zone) -> bool { + let (zones, complete) = self.population_zones_checked(); + !complete || zones.contains(&zone) + } + + /// CR 109.2: Visit every NON-union member of this population, depth-bounded. + /// + /// THE single bounded walker for the `AnyOf` axis. Every consumer that + /// recurses a `CardTypeSetSource` routes through this instead of writing its + /// own `AnyOf` arm, so the recursion is written once and bounded once. + /// + /// `AnyOf` nests without limit — its arity invariant bounds WIDTH, not + /// DEPTH — and a persisted or hand-authored payload can carry whatever + /// nesting it likes. Every consumer recursing independently meant every + /// consumer was a separate unbounded traversal. + /// + /// Returns `false` when the budget is exhausted before the walk completed, + /// and callers MUST treat that as "I did not see everything" and answer + /// conservatively — the same fail-safe contract + /// `target_filter_characteristic_reads_at` uses when it returns + /// `CharacteristicKinds::ALL`. The visitor still runs for everything reached + /// within budget, so a `false` return means incomplete, not empty. + /// + /// [`UNION_DEPTH_BUDGET`] is the depth every in-engine caller passes. + pub fn try_for_each_member( + &self, + depth: u32, + visit: &mut impl FnMut(&CardTypeSetSource), + ) -> bool { + let Some(depth) = depth.checked_sub(1) else { + return false; + }; + match self { + CardTypeSetSource::AnyOf { sources } => { + let mut complete = true; + for member in sources { + // Not short-circuited: a truncated branch must not stop the + // siblings a caller can still legitimately see. + complete &= member.try_for_each_member(depth, visit); + } + complete + } + leaf => { + visit(leaf); + true + } + } + } +} + +/// CR 109.2: Depth budget for [`CardTypeSetSource::try_for_each_member`]. +/// +/// Sized so no real card comes close — printed unions are two or three members +/// deep — while still bounding a hostile or corrupt payload. Mirrors the filter +/// walkers' budgets rather than inventing a second convention. +pub const UNION_DEPTH_BUDGET: u32 = 64; + /// CR 205.3: Which subtypes are excluded when counting distinct subtypes. /// /// A typed qualifier (not a `bool`) so the exclusion axis stays composable and @@ -6808,21 +7103,56 @@ pub enum QuantityRef { /// or in the command zone" pattern. The resolver selects the first matching commander /// (any one if multiple exist) and returns its mana value. CommanderManaValue { owner: ControllerRef }, - /// CR 106.1 + CR 109.1: Number of distinct colors among permanents matching - /// a filter. "Gold", "multicolor", and "colorless" are not colors (CR 105.1), - /// so each of W/U/B/R/G is counted at most once. Used by Faeburrow Elder's - /// "+1/+1 for each color among permanents you control" CDA and its companion - /// mana ability. Composes with `ObjectCount`-style filter predicates and is - /// the dual to `ManaProduction::DistinctColorsAmongPermanents`. - DistinctColorsAmongPermanents { filter: TargetFilter }, + /// CR 105.1 + CR 105.2: Number of distinct colors among the members of a + /// [`CardTypeSetSource`] population. + /// + /// There are exactly five colors (CR 105.1) and an object can be one or more + /// of them or none at all (CR 105.2) — "gold", "multicolor", and "colorless" + /// are not colors — so each of W/U/B/R/G is counted at most once and a + /// colorless member contributes nothing. Parameterized on the shared + /// population axis (rather than carrying a bare `TargetFilter`) so a union or + /// a non-object population is expressible: First Family's "the number of + /// colors among permanents you control **and spells you've cast this turn**" + /// is a set union over a live census and a cast journal, and `|A ∪ B|` is not + /// `|A| + |B|`. Faeburrow Elder's "+1/+1 for each color among permanents you + /// control" CDA is the single-source reading (`Objects { filter }`). + /// + /// Dual to `ManaProduction::DistinctColorsAmongPermanents`, which is a + /// DIFFERENT enum that happens to share the old variant name and is + /// deliberately left un-parameterized (no mana ability reads a union). + /// + /// SAVED-GAME MIGRATION: this variant was `DistinctColorsAmongPermanents + /// { filter: TargetFilter }`. Those nodes live in PERSISTED GAME STATE, not + /// only in regenerated card data — a battlefield token's continuous + /// modification, a mid-resolution stack object, an in-flight reconnect + /// payload, or an out-of-repo community scenario can all carry one. Both the + /// legacy tag (`#[serde(alias)]` on the variant, same precedent as + /// [`QuantityRef::ObjectCountDistinct`]) and the legacy payload key + /// (`#[serde(alias = "filter")]` + [`deserialize_distinct_colors_population`], + /// which lifts it to `Objects { filter }`) are accepted on load, so an old + /// snapshot rehydrates instead of failing with unknown-variant / missing-field. + /// Serialization is unmigrated-only: output always uses the new tag and key. + #[serde(alias = "DistinctColorsAmongPermanents")] + DistinctColorsAmong { + #[serde( + alias = "filter", + deserialize_with = "deserialize_distinct_colors_population" + )] + source: CardTypeSetSource, + }, /// CR 122.1: distinct counter kinds among filter-matched permanents /// (controller-relative, CR 109.4). Counter-side dual of - /// `DistinctColorsAmongPermanents` — counts each distinct `CounterType` + /// [`QuantityRef::DistinctColorsAmong`] — counts each distinct `CounterType` /// appearing on at least one permanent matching `filter` exactly once. /// Used by Bribe Taker's "for each kind of counter on permanents you /// control" iteration source. Kept a separate variant from the color - /// dual because counters (CR 122.1) and colors (CR 105/106) are distinct + /// dual because counters (CR 122.1) and colors (CR 105) are distinct /// rule sections the engine resolves independently. + /// + /// ASYMMETRY, deliberate: unlike its colour dual this variant still carries a + /// bare `TargetFilter` rather than a `CardTypeSetSource`. No card demands a + /// non-object counter population, so folding it onto the shared axis is + /// deferred rather than speculative. DistinctCounterKindsAmong { filter: TargetFilter }, /// CR 701.38 + CR 608.2c: Number of votes tallied for this choice index, /// summed from `state.last_vote_ballots`. Counts votes, not voters — a @@ -14595,6 +14925,50 @@ fn default_distinct_names() -> Vec { vec![SharedQuality::Name] } +/// Backward-compat loader for the legacy +/// `QuantityRef::DistinctColorsAmongPermanents { filter }` payload, reached via +/// the `#[serde(alias = "filter")]` on +/// [`QuantityRef::DistinctColorsAmong`]'s `source` field. +/// +/// The legacy shape named exactly one population — the objects matching +/// `filter` — so it lifts to `CardTypeSetSource::Objects { filter }` with no +/// semantic change (this is a serialization shim, not rules logic; the rule +/// citations live on the variant it feeds). A saved game, an in-flight reconnect payload, or a +/// community scenario captured before the population axis was lifted therefore +/// still deserializes (Faeburrow Elder / Conqueror's Flail / Sunbird Effigy / +/// Aurora Awakener / Puca's Eye / Elemental Spectacle class). +/// +/// ORDERING, load-bearing: the current [`CardTypeSetSource`] reading is tried +/// FIRST, so a current payload is never re-read as a legacy one. Both types are +/// internally tagged on `"type"` and share exactly two tag names +/// (`ExiledBySource`, `TrackedSet` — verified against the two enum +/// declarations), which is the only place the two readings could collide; with +/// the current reading first, that collision can only ever mis-read a LEGACY +/// payload, and no legacy writer emitted either shape here. The two legacy +/// producers were `parse_number_of_distinct_colors_among_permanents_tail` +/// (craft materials → `And { [ExiledBySource, Typed] }`, or a `parse_type_phrase` +/// object filter) and `parse_for_each_distinct_colors_among_permanents` +/// (`parse_type_phrase` only), plus the mtgish-import converter (`Typed`) — +/// none of which can yield a BARE `ExiledBySource` / `TrackedSet` filter. +fn deserialize_distinct_colors_population<'de, D>( + deserializer: D, +) -> Result +where + D: serde::Deserializer<'de>, +{ + #[derive(Deserialize)] + #[serde(untagged)] + enum Repr { + Current(CardTypeSetSource), + LegacyObjects(TargetFilter), + } + + Ok(match Repr::deserialize(deserializer)? { + Repr::Current(source) => source, + Repr::LegacyObjects(filter) => CardTypeSetSource::Objects { filter }, + }) +} + /// Backward-compat default for the legacy /// `FilterProp::MostPrevalentCreatureTypeInLibrary` shape. Old saves had no /// `scope` field; it always meant `your` library. @@ -15560,6 +15934,43 @@ impl TargetFilter { _ => {} } } + + /// CR 400.1: Every zone this filter EXPLICITLY constrains its population to. + /// + /// The union of both zone readers, and never narrower than either. They + /// disagree on `StackSpell` / `StackAbility`: [`extract_in_zone`](Self::extract_in_zone) + /// reports `Stack`, while `collect_zones` has no arm for them and reports + /// nothing. A population walk that switched from the former to the latter + /// would stop scanning the stack, so the single-zone answer is unioned in + /// rather than assumed redundant. Deliberately fixed HERE rather than by + /// adding the arm to `collect_zones`: that function has ~15 callers asking + /// the narrower "what is written here" question, and widening it under them + /// is a change none of them requested. + /// + /// EMPTY IS MEANINGFUL, and is why no battlefield default is applied here. + /// A filter with no written zone constraint denotes permanents (CR 110.1), + /// but the two consumers of this list need opposite things from that fact: + /// + /// * a population WALK must scan the battlefield, so it substitutes the + /// default itself (`game::quantity::visit_characteristic_source`); + /// * a zone-transition DEPENDENCY must not claim to read the battlefield, + /// because battlefield moves are already escalated unconditionally by + /// `mark_layers_full` — reporting it here would add a redundant full + /// recompute to every battlefield move, and would break this function's + /// agreement with its `target_filter_reads_zone` siblings, none of which + /// report a defaulted zone. + /// + /// Order is deterministic (`extract_zones` order, then the single-zone + /// answer) so a walk's yield order does not depend on traversal incidentals. + pub fn population_zones(&self) -> Vec { + let mut zones = self.extract_zones(); + if let Some(single) = self.extract_in_zone() { + if !zones.contains(&single) { + zones.push(single); + } + } + zones + } } impl fmt::Debug for Effect { @@ -26224,6 +26635,413 @@ mod tests { use crate::types::mana::ZoneSpendPolarity; use crate::types::zones::Zone; + /// Row 14, degenerate `AnyOf` cases. CR 109.2: a 0- or 1-member union is not + /// a union, and an EMPTY one is actively unsound — `characteristic_source_read` + /// would fold it to `RwProfile::empty()`, which is FAIL-OPEN for the CR 603.3b + /// same-event ordering gate, and the resolver would return 0 with no + /// diagnostic. Both boundaries are closed: construction and deserialization. + #[test] + fn any_of_arity_invariant_is_enforced_at_both_boundaries() { + let member = || CardTypeSetSource::Zone { + zone: ZoneRef::Graveyard, + scope: CountScope::Controller, + }; + + // Construction: empty yields nothing; a single member COLLAPSES to + // itself rather than forming a degenerate union. + assert_eq!(CardTypeSetSource::any_of(vec![]), None); + assert_eq!(CardTypeSetSource::any_of(vec![member()]), Some(member())); + assert!(matches!( + CardTypeSetSource::any_of(vec![member(), CardTypeSetSource::ExiledBySource]), + Some(CardTypeSetSource::AnyOf { ref sources }) if sources.len() == 2 + )); + + // Deserialization: a hand-authored or saved-game payload cannot smuggle + // a degenerate union past the constructor. + for payload in [ + r#"{"type":"AnyOf","sources":[]}"#, + r#"{"type":"AnyOf","sources":[{"type":"ExiledBySource"}]}"#, + ] { + assert!( + serde_json::from_str::(payload).is_err(), + "a degenerate union must be rejected on load: {payload}" + ); + } + assert!( + serde_json::from_str::( + r#"{"type":"AnyOf","sources":[{"type":"ExiledBySource"},{"type":"ExiledBySource"}]}"# + ) + .is_ok(), + "a two-member union must still load" + ); + } + + /// CR 109.2: the arity invariant is carried by the TYPE, so it holds in + /// release builds and against callers that never touch `any_of`. + /// + /// The previous form — a public `Vec` field plus a `debug_assert!` — was + /// unenforceable twice over: the assert compiled out of release, and any + /// in-crate caller could write the struct literal directly. Several did. + #[test] + fn union_sources_arity_is_unconstructible_below_two() { + let member = || CardTypeSetSource::ExiledBySource; + + // Construction: the ONLY in-crate way in rejects both degenerate arities. + assert!(UnionSources::new(vec![]).is_none()); + assert!(UnionSources::new(vec![member()]).is_none()); + assert!(UnionSources::new(vec![member(), member()]).is_some()); + + // `any_of` keeps its collapse-to-single behavior on top of that gate. + assert_eq!(CardTypeSetSource::any_of(vec![]), None); + assert_eq!(CardTypeSetSource::any_of(vec![member()]), Some(member())); + + // Deserialization: a saved game or hand-authored payload is rejected + // with a message naming the arity, not a generic length error. + for payload in [ + r#"{"type":"AnyOf","sources":[]}"#, + r#"{"type":"AnyOf","sources":[{"type":"ExiledBySource"}]}"#, + ] { + let err = serde_json::from_str::(payload) + .expect_err("a degenerate union must be rejected on load"); + assert!( + err.to_string().contains("at least 2"), + "the error should name the invariant, got {err}" + ); + } + + // Round-trip: the wire shape is unchanged by the newtype, so saved games + // written before it still load. + let json = + r#"{"type":"AnyOf","sources":[{"type":"ExiledBySource"},{"type":"ExiledBySource"}]}"#; + let loaded: CardTypeSetSource = + serde_json::from_str(json).expect("a two-member union must still load"); + assert_eq!( + serde_json::to_string(&loaded).expect("serialize"), + json, + "the newtype must be serde-transparent" + ); + } + + /// CR 109.2: the single bounded walker unrolls unions, visits every leaf + /// once, and reports truncation instead of recursing without limit. + #[test] + fn try_for_each_member_unrolls_unions_and_bounds_depth() { + let leaf = |n: u32| CardTypeSetSource::TrackedSet { + caused_by: (n > 0).then_some(ThisWayCause::Discarded), + }; + let union = CardTypeSetSource::any_of(vec![ + leaf(0), + CardTypeSetSource::any_of(vec![leaf(1), CardTypeSetSource::ExiledBySource]) + .expect("two-member union"), + ]) + .expect("two-member union"); + + // Every leaf, exactly once, with nested unions flattened. + let mut seen = Vec::new(); + assert!(union.try_for_each_member(UNION_DEPTH_BUDGET, &mut |m| seen.push(m.clone()))); + assert_eq!( + seen, + vec![leaf(0), leaf(1), CardTypeSetSource::ExiledBySource], + "unions unroll in declaration order and yield only non-union members" + ); + + // A non-union source is its own single member. + let mut single = Vec::new(); + assert!(CardTypeSetSource::ExiledBySource + .try_for_each_member(UNION_DEPTH_BUDGET, &mut |m| single.push(m.clone()))); + assert_eq!(single, vec![CardTypeSetSource::ExiledBySource]); + + // Exhaustion reports FALSE rather than recursing, and still visits what + // it reached — "incomplete", not "empty". + let mut partial = Vec::new(); + assert!( + !union.try_for_each_member(1, &mut |m| partial.push(m.clone())), + "a budget too small for the nesting must report truncation" + ); + assert!( + partial.len() < seen.len(), + "a truncated walk sees strictly fewer members" + ); + // Depth 0 cannot even visit a leaf. + assert!(!CardTypeSetSource::ExiledBySource.try_for_each_member(0, &mut |_| {})); + } + + /// CR 400.1: the population-zone authority reports EVERY zone a population + /// reads, per variant. The craft row is the one that was silently wrong: + /// evaluation scanned exile for `And[ExiledBySource, Owned{You}]` while the + /// dependency classifier reported that population as reading no zone, so no + /// exile transition ever dirtied a characteristic derived from it. + #[test] + fn population_zones_reports_every_zone_each_population_reads() { + // Craft linked-exile (Sunbird Effigy), in both the shapes that reach it: + // the dedicated variant and the filter the craft parser actually builds. + assert_eq!( + CardTypeSetSource::ExiledBySource.population_zones(), + vec![Zone::Exile] + ); + let craft = CardTypeSetSource::Objects { + filter: TargetFilter::And { + filters: vec![ + TargetFilter::ExiledBySource, + TargetFilter::Typed(TypedFilter::default().properties(vec![ + FilterProp::Owned { + controller: ControllerRef::You, + }, + ])), + ], + }, + }; + assert_eq!(craft.population_zones(), vec![Zone::Exile]); + assert!(craft.reads_zone(Zone::Exile), "the craft regression"); + assert!(!craft.reads_zone(Zone::Graveyard)); + + // An explicit single-zone constraint, and the ZoneRef mapping. + for (zone_ref, zone) in [ + (ZoneRef::Graveyard, Zone::Graveyard), + (ZoneRef::Exile, Zone::Exile), + (ZoneRef::Library, Zone::Library), + (ZoneRef::Hand, Zone::Hand), + ] { + let source = CardTypeSetSource::Zone { + zone: zone_ref, + scope: CountScope::Controller, + }; + assert_eq!(source.population_zones(), vec![zone]); + assert!(source.reads_zone(zone)); + assert!(!source.reads_zone(Zone::Stack)); + } + + // A snapshot population is NOT a zone read (CR 400.7 / CR 608.2c), and + // that is asserted rather than left implicit. + for snapshot in [ + CardTypeSetSource::TrackedSet { caused_by: None }, + CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: CountScope::Controller, + filter: None, + }, + ] { + assert!(snapshot.population_zones().is_empty()); + for zone in [Zone::Battlefield, Zone::Exile, Zone::Graveyard] { + assert!(!snapshot.reads_zone(zone)); + } + } + } + + /// CR 400.1: a multi-zone `InAnyZone` population enumerates EVERY zone. + /// `extract_in_zone` collapses it to one, which is what made the evaluator + /// undercount every zone after the first. + #[test] + fn population_zones_preserves_multi_zone_unions_that_extract_in_zone_collapses() { + let multi = + TargetFilter::Typed( + TypedFilter::default().properties(vec![FilterProp::InAnyZone { + zones: vec![Zone::Graveyard, Zone::Hand, Zone::Library], + }]), + ); + // The collapse this replaces: one zone out of three. + assert_eq!(multi.extract_in_zone(), None); + let source = CardTypeSetSource::Objects { + filter: multi.clone(), + }; + assert_eq!( + source.population_zones(), + vec![Zone::Graveyard, Zone::Hand, Zone::Library] + ); + for zone in [Zone::Graveyard, Zone::Hand, Zone::Library] { + assert!(source.reads_zone(zone), "{zone:?} is in the union"); + } + for zone in [Zone::Battlefield, Zone::Exile, Zone::Stack] { + assert!(!source.reads_zone(zone), "{zone:?} is not in the union"); + } + } + + /// The population-zone list must never be NARROWER than `extract_in_zone`. + /// `collect_zones` has no `StackSpell` arm, so a walk that read only + /// `extract_zones` would stop scanning the stack — the union is what keeps + /// Secret Arcade's "permanent spells you control" shape reachable. + #[test] + fn population_zones_is_never_narrower_than_either_zone_reader() { + for filter in [ + TargetFilter::StackSpell, + TargetFilter::And { + filters: vec![ + TargetFilter::StackSpell, + TargetFilter::Typed(TypedFilter::permanent().controller(ControllerRef::You)), + ], + }, + ] { + assert!( + filter.extract_zones().is_empty(), + "precondition: collect_zones has no stack arm" + ); + assert_eq!(filter.extract_in_zone(), Some(Zone::Stack)); + assert_eq!(filter.population_zones(), vec![Zone::Stack]); + } + } + + /// CR 110.1 + CR 611.3a: an unconstrained filter denotes permanents, but the + /// battlefield default is deliberately NOT reported here. Battlefield moves + /// are escalated unconditionally by `mark_layers_full`, so claiming the read + /// would add a redundant full recompute to every one of them; the population + /// WALK substitutes the default itself. This asymmetry is the whole reason + /// `population_zones` returns an empty vec instead of `[Battlefield]`. + #[test] + fn population_zones_leaves_the_battlefield_default_to_the_walk() { + let source = CardTypeSetSource::Objects { + filter: TargetFilter::Typed(TypedFilter::permanent().controller(ControllerRef::You)), + }; + assert!(source.population_zones().is_empty()); + assert!(!source.reads_zone(Zone::Battlefield)); + } + + /// CR 109.2: a union reads the union of its members' zones, deduplicated — + /// First Family's shape, plus a nested union to pin the recursion. + #[test] + fn population_zones_unions_member_zones_without_duplicates() { + let graveyard = CardTypeSetSource::Zone { + zone: ZoneRef::Graveyard, + scope: CountScope::Controller, + }; + let union = CardTypeSetSource::any_of(vec![ + graveyard.clone(), + CardTypeSetSource::ExiledBySource, + // Same zone twice — the dedup is what makes this a set union. + graveyard.clone(), + CardTypeSetSource::any_of(vec![ + CardTypeSetSource::Zone { + zone: ZoneRef::Hand, + scope: CountScope::Controller, + }, + CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: CountScope::Controller, + filter: None, + }, + ]) + .expect("two-member union"), + ]) + .expect("multi-member union"); + assert_eq!( + union.population_zones(), + vec![Zone::Graveyard, Zone::Exile, Zone::Hand] + ); + } + + /// SAVED-GAME MIGRATION, unit arm. The pre-lift shape + /// `{"type":"DistinctColorsAmongPermanents","filter":…}` must rehydrate as + /// the single-population reading. Both halves of the rename are load-bearing: + /// without the variant alias the tag is an unknown variant, and even with it + /// the renamed-and-retyped key would fail as a missing `source` field. + /// + /// The input is a VERBATIM node lifted out of the persisted 4p board in + /// `crates/engine/tests/fixtures/dina_noff_turn5_4p.json.gz`, so it is the + /// exact byte shape live snapshots carry rather than a hand-written + /// paraphrase. + #[test] + fn legacy_distinct_colors_among_permanents_payload_lifts_to_an_object_population() { + let expected = QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { + filter: TargetFilter::Typed( + TypedFilter::permanent().controller(ControllerRef::You), + ), + }, + }; + + let legacy = r#"{"filter":{"controller":"You","properties":[],"type":"Typed","type_filters":["Permanent"]},"type":"DistinctColorsAmongPermanents"}"#; + assert_eq!( + serde_json::from_str::(legacy) + .expect("a pre-lift persisted node must still deserialize"), + expected, + "the legacy tag + `filter` key must lift to `Objects {{ filter }}`", + ); + + // Reach-guard: the same payload under the CURRENT tag is still legacy on + // the key axis, and the current tag + key is unaffected. + let legacy_tag_only = r#"{"filter":{"controller":"You","properties":[],"type":"Typed","type_filters":["Permanent"]},"type":"DistinctColorsAmong"}"#; + assert_eq!( + serde_json::from_str::(legacy_tag_only) + .expect("the legacy key must be accepted under the current tag too"), + expected, + ); + + // Migration is load-only: a rehydrated node re-serializes in the CURRENT + // shape, so a save written by this build never re-emits the old names. + let round = serde_json::to_string(&expected).expect("serializes"); + assert!( + round.contains(r#""type":"DistinctColorsAmong""#) && round.contains(r#""source""#), + "serialization must emit the current tag and key: {round}", + ); + assert!( + !round.contains("DistinctColorsAmongPermanents"), + "serialization must not re-emit the legacy tag: {round}", + ); + assert_eq!( + serde_json::from_str::(&round).expect("round-trips"), + expected, + ); + + // NON-VACUITY: neither acceptance above comes from a tolerant decoder. + // `QuantityRef` has no `#[serde(other)]` fallback, so a near-miss tag is + // still an unknown-variant error — the alias is what admits the legacy + // one. And the population key stays REQUIRED: aliasing it did not turn + // it into a silent default, so a payload carrying neither key errors + // instead of decoding as an empty population. + assert!( + serde_json::from_str::( + r#"{"filter":{"type":"Typed"},"type":"DistinctColorsAmongPermanentsX"}"# + ) + .is_err(), + "an unaliased tag must still be rejected, or the row measures nothing", + ); + assert!( + serde_json::from_str::(r#"{"type":"DistinctColorsAmong"}"#).is_err(), + "the population key must stay required under both tags", + ); + assert!( + serde_json::from_str::(r#"{"type":"DistinctColorsAmongPermanents"}"#) + .is_err(), + "the population key must stay required under both tags", + ); + } + + /// The legacy lift must never capture a CURRENT payload. `TargetFilter` and + /// `CardTypeSetSource` are both internally tagged on `"type"` and share + /// exactly two tag names (`ExiledBySource`, `TrackedSet`), so those are the + /// only shapes where the two readings could collide — the shim tries the + /// current reading first, and this row pins that ordering. `Objects` and the + /// `AnyOf` union (First Family) are covered as the non-colliding controls. + #[test] + fn current_distinct_colors_population_payloads_are_never_read_as_legacy() { + let objects = CardTypeSetSource::Objects { + filter: TargetFilter::Typed(TypedFilter::permanent().controller(ControllerRef::You)), + }; + for source in [ + CardTypeSetSource::ExiledBySource, + CardTypeSetSource::TrackedSet { caused_by: None }, + objects.clone(), + CardTypeSetSource::any_of(vec![ + objects, + CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: CountScope::Controller, + filter: None, + }, + ]) + .expect("two-member union"), + ] { + let node = QuantityRef::DistinctColorsAmong { + source: source.clone(), + }; + let json = serde_json::to_string(&node).expect("serializes"); + assert_eq!( + serde_json::from_str::(&json).expect("round-trips"), + node, + "the current reading must win for {json}", + ); + } + } + #[test] fn first_optional_effect_gate_does_not_latch_a_later_independent_gate() { let mut first = ResolvedAbility::new(Effect::NoOp, vec![], ObjectId(1), PlayerId(0)); diff --git a/crates/engine/tests/fixtures/integration_cards.json.gz b/crates/engine/tests/fixtures/integration_cards.json.gz index b447bdcf50..257bc7a67d 100644 Binary files a/crates/engine/tests/fixtures/integration_cards.json.gz and b/crates/engine/tests/fixtures/integration_cards.json.gz differ diff --git a/crates/engine/tests/integration/april_oneil_card_types_among_spells_cast.rs b/crates/engine/tests/integration/april_oneil_card_types_among_spells_cast.rs new file mode 100644 index 0000000000..8a143a298e --- /dev/null +++ b/crates/engine/tests/integration/april_oneil_card_types_among_spells_cast.rs @@ -0,0 +1,302 @@ +//! April O'Neil, Hacktivist — "draw a card for each **card type among spells +//! you've cast this turn**" counts DISTINCT CARD TYPES over the per-turn cast +//! journal (CR 205.2a over CR 601.2a), not the number of spells cast. +//! +//! Pre-fix this bound `QuantityRef::SpellsCastThisTurn`, a raw count of cast +//! records, because `parse_spell_history_clause`'s terminal fallback claimed +//! any clause containing a cast verb phrase and silently discarded the +//! unconsumed "card type among" aggregation head. + +use engine::game::scenario::{GameScenario, P0}; +use engine::types::ability::{CardTypeSetSource, QuantityExpr, QuantityRef, TurnJournalKind}; +use engine::types::identifiers::ObjectId; +use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; +use engine::types::phase::Phase; + +/// Verbatim Scryfall Oracle text. +const APRIL_ONEIL: &str = + "At the beginning of your end step, draw a card for each card type among spells \ +you've cast this turn."; + +const INSTANT_FILLER: &str = "Target player gains 1 life."; +const SORCERY_FILLER: &str = "Target player gains 2 life."; + +fn mono(shard: ManaCostShard) -> ManaCost { + ManaCost::Cost { + shards: vec![shard], + generic: 0, + } +} + +fn pool(colored: &[(ManaType, usize)]) -> Vec { + colored + .iter() + .flat_map(|(kind, n)| vec![ManaUnit::new(*kind, ObjectId(0), false, vec![]); *n]) + .collect() +} + +/// SHAPE half of the pair, and the reach guard for the runtime half below: the +/// trigger must parse to a distinct-card-type count over the CAST JOURNAL with +/// zero `Effect::Unimplemented`, so a "draws 2, not 3" assertion cannot pass +/// vacuously through an unimplemented early return. +#[test] +fn april_oneil_binds_card_types_over_the_cast_journal() { + let parsed = engine::parser::parse_oracle_text( + APRIL_ONEIL, + "April O'Neil, Hacktivist", + &[], + &["Creature".to_string()], + &[], + ); + assert!( + parsed.parse_warnings.is_empty(), + "April O'Neil must parse cleanly, got {:?}", + parsed.parse_warnings + ); + let trigger = parsed + .triggers + .first() + .expect("April O'Neil must parse an end-step trigger"); + let execute = trigger + .execute + .as_deref() + .expect("the end-step trigger must carry an executed ability"); + let engine::types::ability::Effect::Draw { count, .. } = execute.effect.as_ref() else { + panic!("expected a Draw effect, got {:?}", execute.effect); + }; + assert_eq!( + *count, + QuantityExpr::Ref { + qty: QuantityRef::DistinctCardTypes { + source: CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: engine::types::ability::CountScope::Controller, + filter: None, + }, + }, + }, + "the draw count must be distinct CARD TYPES over the cast journal, \ + not a count of spells" + ); +} + +/// RUNTIME half, driven through the REAL end-step trigger. +/// +/// Cast THREE spells spanning TWO card types, then advance to the end step and +/// let April O'Neil's own triggered ability resolve. The 3-casts / 2-types split +/// is the discriminator: the pre-fix `SpellsCastThisTurn` reading draws 3. +/// +/// The direct `resolve_quantity` probe is kept as a second, sharper assertion — +/// it pins the quantity in isolation — but it is NOT the primary check. On its +/// own it proves only that the resolver can count a hand-built AST; the drawn-card +/// assertion is what proves the parsed trigger actually reaches that resolver +/// with the source bound, so a break anywhere in trigger wiring is caught here +/// rather than passing green against a quantity nothing dispatches. +#[test] +fn april_oneil_counts_card_types_not_spells() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + // Deeper than the trigger can draw, so a miscount reads as a wrong DRAW + // COUNT rather than as an empty library. + scenario.with_library_top(P0, &["Draw A", "Draw B", "Draw C", "Draw D"]); + + let april = scenario + .add_creature_from_oracle(P0, "April O'Neil, Hacktivist", 2, 2, APRIL_ONEIL) + .id(); + + // Three casts, two card types: two instants and one sorcery. + let instant_a = scenario + .add_spell_to_hand_from_oracle(P0, "Instant A", true, INSTANT_FILLER) + .with_mana_cost(mono(ManaCostShard::Blue)) + .id(); + let instant_b = scenario + .add_spell_to_hand_from_oracle(P0, "Instant B", true, INSTANT_FILLER) + .with_mana_cost(mono(ManaCostShard::Blue)) + .id(); + let sorcery = scenario + .add_spell_to_hand_from_oracle(P0, "Sorcery A", false, SORCERY_FILLER) + .with_mana_cost(mono(ManaCostShard::Red)) + .id(); + + scenario.with_mana_pool(P0, pool(&[(ManaType::Blue, 2), (ManaType::Red, 1)])); + let mut runner = scenario.build(); + + for spell in [instant_a, instant_b, sorcery] { + runner.cast(spell).target_player(P0).resolve(); + } + + // Reach guard: three cast records exist, so "2" below is a real + // distinct-type answer and not an artifact of an empty journal. + assert_eq!( + runner + .state() + .spells_cast_this_turn_by_player + .get(&P0) + .map_or(0, |records| records.len()), + 3, + "reach guard: all three casts must be journaled" + ); + + let count = QuantityExpr::Ref { + qty: QuantityRef::DistinctCardTypes { + source: CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: engine::types::ability::CountScope::Controller, + filter: None, + }, + }, + }; + assert_eq!( + engine::game::quantity::resolve_quantity(runner.state(), &count, P0, april), + 2, + "three spells spanning two card types must count 2 (CR 205.2a), not 3" + ); + + // THE PRIMARY ASSERTION: April O'Neil's own trigger, through the production + // pipeline. CR 513.1 — "at the beginning of your end step" triggers when the + // end step begins; the trigger goes on the stack and resolves from there. + let hand_before = runner.state().players[P0.0 as usize].hand.len(); + // CR 508.1: April O'Neil is a 2/2 and could attack, so the declare-attackers + // turn-based action surfaces a prompt `advance_to_phase` cannot auto-pass — + // it would stop in combat and leave the end step unreached. Cross it + // explicitly rather than letting the phase helper stall. + runner.advance_to_combat(); + runner + .declare_attackers(&[]) + .expect("declare no attackers to cross combat"); + runner.advance_to_end_step(); + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().phase, + Phase::End, + "reach guard: the scenario must actually reach the end step, or the \ + draw assertion below passes vacuously by never triggering" + ); + assert_eq!( + runner.state().players[P0.0 as usize].hand.len() - hand_before, + 2, + "April O'Neil must draw one card per CARD TYPE among the three spells \ + cast this turn — 2 (instant, sorcery), not 3 (the spell count)" + ); +} + +/// Sibling: the journal's optional narrowing filter (Hurkyl's "noncreature +/// spells you've cast this turn"). `filter: None` counts every member; the +/// empty journal counts 0. +#[test] +fn a_filtered_cast_journal_narrows_the_type_tally() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + let source = scenario.add_creature(P0, "Journal Reader", 1, 1).id(); + let instant = scenario + .add_spell_to_hand_from_oracle(P0, "Instant A", true, INSTANT_FILLER) + .with_mana_cost(mono(ManaCostShard::Blue)) + .id(); + // The EXCLUDED member for the filtered arm below — a second card type in the + // journal that a narrowing filter must reject. + let sorcery = scenario + .add_spell_to_hand_from_oracle(P0, "Sorcery A", false, SORCERY_FILLER) + .with_mana_cost(mono(ManaCostShard::Red)) + .id(); + + scenario.with_mana_pool(P0, pool(&[(ManaType::Blue, 1), (ManaType::Red, 1)])); + let mut runner = scenario.build(); + + let unfiltered = |scope| QuantityExpr::Ref { + qty: QuantityRef::DistinctCardTypes { + source: CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope, + filter: None, + }, + }, + }; + // CR 601.2a: the journal's optional narrowing filter, matched against each + // record's cast-time snapshot (a resolved spell is no longer an object, + // CR 400.7). + let narrowed_to = |type_filter| QuantityExpr::Ref { + qty: QuantityRef::DistinctCardTypes { + source: CardTypeSetSource::TurnJournal { + journal: TurnJournalKind::SpellsCast, + scope: engine::types::ability::CountScope::Controller, + filter: Some(engine::types::ability::TargetFilter::Typed( + engine::types::ability::TypedFilter::new(type_filter), + )), + }, + }, + }; + + // Empty journal → 0. + assert_eq!( + engine::game::quantity::resolve_quantity( + runner.state(), + &unfiltered(engine::types::ability::CountScope::Controller), + P0, + source, + ), + 0, + "an empty journal contributes no card types" + ); + + runner.cast(instant).target_player(P0).resolve(); + + assert_eq!( + engine::game::quantity::resolve_quantity( + runner.state(), + &unfiltered(engine::types::ability::CountScope::Controller), + P0, + source, + ), + 1, + "one instant cast → one card type" + ); + + // CR 109.5: "you"/"your" on an object refers to that object's controller, + // so the same journal read at `Opponents` scope sees the OTHER player's + // journal, which is empty. (NOT CR 109.4, which says only stack/battlefield + // objects HAVE a controller; that rule does not define the possessive.) + assert_eq!( + engine::game::quantity::resolve_quantity( + runner.state(), + &unfiltered(engine::types::ability::CountScope::Opponents), + P0, + source, + ), + 0, + "the opponents' journal is empty, so the scope axis is live" + ); + + // FILTERED ARM — what this test is named for. Put a SECOND card type in the + // journal, then narrow to each one in turn. Without the second cast the + // filter would be indistinguishable from `None`, which is why the exclusion + // half is asserted alongside the inclusion half. + runner.cast(sorcery).target_player(P0).resolve(); + assert_eq!( + engine::game::quantity::resolve_quantity( + runner.state(), + &unfiltered(engine::types::ability::CountScope::Controller), + P0, + source, + ), + 2, + "reach guard: both casts are journaled, so the filter below has \ + something to exclude" + ); + for (type_filter, label) in [ + (engine::types::ability::TypeFilter::Instant, "instant"), + (engine::types::ability::TypeFilter::Sorcery, "sorcery"), + ] { + assert_eq!( + engine::game::quantity::resolve_quantity( + runner.state(), + &narrowed_to(type_filter), + P0, + source, + ), + 1, + "narrowing to {label} must admit exactly that one record, not both" + ); + } +} diff --git a/crates/engine/tests/integration/aurora_awakener_reveal_until_n_permanents.rs b/crates/engine/tests/integration/aurora_awakener_reveal_until_n_permanents.rs index e4a8f894ce..6850380565 100644 --- a/crates/engine/tests/integration/aurora_awakener_reveal_until_n_permanents.rs +++ b/crates/engine/tests/integration/aurora_awakener_reveal_until_n_permanents.rs @@ -29,8 +29,8 @@ use engine::game::effects::reveal_until; use engine::game::scenario::{GameScenario, P0}; use engine::types::ability::{ - ControllerRef, Effect, QuantityExpr, QuantityRef, ResolvedAbility, RevealUntilDisposition, - TargetFilter, TypedFilter, + CardTypeSetSource, ControllerRef, Effect, QuantityExpr, QuantityRef, ResolvedAbility, + RevealUntilDisposition, TargetFilter, TypedFilter, }; use engine::types::actions::GameAction; use engine::types::card_type::CoreType; @@ -43,8 +43,12 @@ use engine::types::zones::{EtbTapState, Zone}; /// "the number of colors among permanents you control" — the Aurora/Sanar count. fn distinct_colors_count() -> QuantityExpr { QuantityExpr::Ref { - qty: QuantityRef::DistinctColorsAmongPermanents { - filter: TargetFilter::Typed(TypedFilter::permanent().controller(ControllerRef::You)), + qty: QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { + filter: TargetFilter::Typed( + TypedFilter::permanent().controller(ControllerRef::You), + ), + }, }, } } diff --git a/crates/engine/tests/integration/craft_material_references.rs b/crates/engine/tests/integration/craft_material_references.rs index d138d8367e..1736c48d69 100644 --- a/crates/engine/tests/integration/craft_material_references.rs +++ b/crates/engine/tests/integration/craft_material_references.rs @@ -22,8 +22,8 @@ use engine::game::quantity::resolve_quantity; use engine::game::zones::create_object; use engine::parser::oracle::parse_oracle_text; use engine::types::ability::{ - AggregateFunction, ContinuousModification, Effect, ManaProduction, ObjectProperty, - QuantityExpr, QuantityRef, StaticDefinition, TargetFilter, + AggregateFunction, CardTypeSetSource, ContinuousModification, Effect, ManaProduction, + ObjectProperty, QuantityExpr, QuantityRef, StaticDefinition, TargetFilter, }; use engine::types::card_type::CoreType; use engine::types::game_state::{ExileLink, ExileLinkKind, GameState}; @@ -198,12 +198,15 @@ fn sunbird_effigy_pt_is_distinct_colors_of_craft_materials() { for (label, qty) in [("power", &power), ("toughness", &toughness)] { match qty { QuantityExpr::Ref { - qty: QuantityRef::DistinctColorsAmongPermanents { filter }, + qty: + QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { filter }, + }, } => assert!( filter_reads_exiled_by_source(filter), "{label} colors must read ExiledBySource, got {filter:?}" ), - other => panic!("{label}: expected DistinctColorsAmongPermanents, got {other:?}"), + other => panic!("{label}: expected DistinctColorsAmong(Objects), got {other:?}"), } } diff --git a/crates/engine/tests/integration/elemental_spectacle_regression.rs b/crates/engine/tests/integration/elemental_spectacle_regression.rs index 643e5a4a5b..462a55976b 100644 --- a/crates/engine/tests/integration/elemental_spectacle_regression.rs +++ b/crates/engine/tests/integration/elemental_spectacle_regression.rs @@ -2,7 +2,9 @@ use engine::game::ability_utils::build_resolved_from_def; use engine::game::effects::resolve_ability_chain; use engine::game::zones::create_object; use engine::parser::parse_oracle_text; -use engine::types::ability::{Effect, QuantityExpr, QuantityRef, TargetFilter, TypeFilter}; +use engine::types::ability::{ + CardTypeSetSource, Effect, QuantityExpr, QuantityRef, TargetFilter, TypeFilter, +}; use engine::types::card_type::CoreType; use engine::types::events::GameEvent; use engine::types::game_state::GameState; @@ -52,7 +54,10 @@ fn elemental_spectacle_counts_distinct_controlled_permanent_colors_for_tokens() match definition.effect.as_ref() { Effect::Token { count, .. } => match count { QuantityExpr::Ref { - qty: QuantityRef::DistinctColorsAmongPermanents { filter }, + qty: + QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { filter }, + }, } => match filter { TargetFilter::Typed(typed) => { assert_eq!(typed.type_filters, vec![TypeFilter::Permanent]); diff --git a/crates/engine/tests/integration/first_family_union_color_count.rs b/crates/engine/tests/integration/first_family_union_color_count.rs new file mode 100644 index 0000000000..908b4904d6 --- /dev/null +++ b/crates/engine/tests/integration/first_family_union_color_count.rs @@ -0,0 +1,253 @@ +//! First Family — "the number of colors among permanents you control **and** +//! spells you've cast this turn" is a SET UNION over two populations, resolved +//! through the real cast pipeline. +//! +//! Pre-fix, both quantity slots bound `QuantityRef::SpellsCastThisTurn` — a +//! COUNT OF SPELLS — silently dropping both the colour aggregation (CR 105.1) +//! and the "permanents you control" population, with no parse warning. Every +//! assertion below flips if that binding is restored: +//! +//! * the self-inclusion case returns 1 (one cast record) instead of 2; +//! * the disjoint case does not move when a permanent of a NEW colour is +//! added, because the permanent population is not read at all; +//! * a `Sum`-shaped implementation (`|A| + |B|`) over-counts the overlap, +//! which is why the union must live inside the `HashSet` — `max` +//! distributes over set union but a distinct-count does not. + +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; + +/// Verbatim Scryfall Oracle text — a paraphrase can take a different parser +/// branch and go green while the real card stays broken. +const FIRST_FAMILY: &str = "You draw X cards and gain X life, where X is the number of colors \ +among permanents you control and spells you've cast this turn."; + +/// A vanilla filler instant used only to put a cast record of a known colour +/// into the per-turn journal (CR 601.2a: casting moves the card to the stack, +/// which is where `finalize_cast` records it). +const FILLER: &str = "Target player gains 1 life."; + +fn mono(shard: ManaCostShard) -> ManaCost { + ManaCost::Cost { + shards: vec![shard], + generic: 0, + } +} + +/// First Family's printed cost: {2}{G}{U}. CR 105.2 — the spell is green AND +/// blue, so its own cast record contributes two colours. +fn first_family_cost() -> ManaCost { + ManaCost::Cost { + shards: vec![ManaCostShard::Green, ManaCostShard::Blue], + generic: 2, + } +} + +fn pool(colored: &[(ManaType, usize)]) -> Vec { + colored + .iter() + .flat_map(|(kind, n)| vec![ManaUnit::new(*kind, ObjectId(0), false, vec![]); *n]) + .collect() +} + +/// Enough mana for the filler plus First Family, with the exact colours both +/// need. Colour identity of the SPELL comes from its printed cost, not from the +/// mana spent (CR 105.2), so the pool composition is payment only. +fn full_pool() -> Vec { + pool(&[ + (ManaType::Green, 3), + (ManaType::Blue, 1), + (ManaType::Colorless, 2), + ]) +} + +/// CR 117.1: hand priority to `player` so their cast runs through the SAME +/// production `GameAction::CastSpell` path every other cast in this file uses, +/// rather than being injected into the journal by hand. +fn give_priority(runner: &mut GameRunner, player: PlayerId) { + let state = runner.state_mut(); + state.priority_player = player; + state.waiting_for = WaitingFor::Priority { player }; +} + +/// CR 105.1 + CR 112.1 + CR 601.2a. The full multi-authority overlap fixture: +/// a green permanent, a green spell cast EARLIER this turn, and First Family +/// itself ({G}{U}). +/// +/// colours = { G (permanent), G (earlier spell), G+U (First Family) } = {G, U} +/// +/// So X = 2 — NOT 4, which is what a `Sum` of the two populations' independent +/// colour counts (|{G}| + |{G,U}|) would give. The overlap is the whole point: +/// green is in BOTH populations and must contribute once. +#[test] +fn first_family_counts_the_union_not_the_sum_on_overlap() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_library_top(P0, &["Draw A", "Draw B", "Draw C", "Draw D"]); + + // Population A: one GREEN permanent. + scenario + .add_creature(P0, "Green Bear", 2, 2) + .with_mana_cost(mono(ManaCostShard::Green)); + + let filler = scenario + .add_spell_to_hand_from_oracle(P0, "Green Filler", true, FILLER) + .with_mana_cost(mono(ManaCostShard::Green)) + .id(); + let first_family = scenario + .add_spell_to_hand_from_oracle(P0, "First Family", true, FIRST_FAMILY) + .with_mana_cost(first_family_cost()) + .id(); + + scenario.with_mana_pool(P0, full_pool()); + let mut runner = scenario.build(); + + // Population B, member 1: a GREEN spell cast earlier this turn. + runner.cast(filler).target_player(P0).resolve(); + // Reach guard: the earlier cast really is journaled, so the union below is + // exercised over TWO non-empty populations rather than degenerating to one. + assert_eq!( + runner + .state() + .spells_cast_this_turn_by_player + .get(&P0) + .map_or(0, |records| records.len()), + 1, + "reach guard: the earlier green cast must be in the journal" + ); + + give_priority(&mut runner, P0); + let outcome = runner.cast(first_family).resolve(); + + outcome.assert_hand_drawn(P0, 2); + // The filler gained 1 life before First Family resolved; `life_delta` is + // measured from just before THIS cast, so it isolates First Family's gain. + outcome.assert_life_delta(P0, 2); +} + +/// CR 105.1. Adding a permanent of a colour that is in NEITHER population moves +/// X from 2 to 3. This is the assertion a spell-COUNT implementation cannot +/// pass: the number of spells cast is unchanged by a permanent being on the +/// battlefield. +#[test] +fn first_family_reads_the_permanent_population() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_library_top(P0, &["Draw A", "Draw B", "Draw C", "Draw D"]); + + scenario + .add_creature(P0, "Green Bear", 2, 2) + .with_mana_cost(mono(ManaCostShard::Green)); + // The disjoint third colour — present only on the battlefield. + scenario + .add_creature(P0, "Red Bear", 2, 2) + .with_mana_cost(mono(ManaCostShard::Red)); + + let filler = scenario + .add_spell_to_hand_from_oracle(P0, "Green Filler", true, FILLER) + .with_mana_cost(mono(ManaCostShard::Green)) + .id(); + let first_family = scenario + .add_spell_to_hand_from_oracle(P0, "First Family", true, FIRST_FAMILY) + .with_mana_cost(first_family_cost()) + .id(); + + scenario.with_mana_pool(P0, full_pool()); + let mut runner = scenario.build(); + + runner.cast(filler).target_player(P0).resolve(); + give_priority(&mut runner, P0); + let outcome = runner.cast(first_family).resolve(); + + // {G (bear + filler + FF), R (bear), U (FF)} = 3. The pre-fix spell-count + // reading is 2 here (two cast records), so this row discriminates. + outcome.assert_hand_drawn(P0, 3); + outcome.assert_life_delta(P0, 3); +} + +/// CR 112.1 + CR 608.2m: First Family counts ITS OWN cast. Its record is pushed +/// at `finalize_cast` and, per CR 112.1, it is still on the stack while its own +/// effect resolves — so with an empty board and no prior spells X is 2 (green +/// and blue), never 0 or 1. +/// +/// This also pins the ABSENCE of the `FilterProp::Another` own-cast exclusion: +/// the card says "spells you've cast", not "OTHER spells you've cast". Applying +/// that exclusion here would give 0. +#[test] +fn first_family_counts_its_own_cast_on_an_empty_board() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_library_top(P0, &["Draw A", "Draw B", "Draw C"]); + + let first_family = scenario + .add_spell_to_hand_from_oracle(P0, "First Family", true, FIRST_FAMILY) + .with_mana_cost(first_family_cost()) + .id(); + + scenario.with_mana_pool(P0, full_pool()); + let mut runner = scenario.build(); + + let outcome = runner.cast(first_family).resolve(); + + // REVERT GUARD, the sharpest one: the pre-fix spell-count reading is 1 here + // (exactly one cast record — First Family's own). + outcome.assert_hand_drawn(P0, 2); + outcome.assert_life_delta(P0, 2); +} + +/// CR 109.5: "spells **you've** cast" is the controller's journal +/// (`CountScope::Controller`) — "you"/"your" on an object refers to that +/// object's controller. An opponent's cast of a brand-new colour must not move +/// X. (NOT CR 109.4, which says only stack/battlefield objects HAVE a +/// controller; that rule does not define the possessive.) +#[test] +fn first_family_ignores_an_opponents_cast() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_library_top(P0, &["Draw A", "Draw B", "Draw C"]); + + scenario + .add_creature(P0, "Green Bear", 2, 2) + .with_mana_cost(mono(ManaCostShard::Green)); + + let opponent_spell = scenario + .add_spell_to_hand_from_oracle(P1, "Black Filler", true, FILLER) + .with_mana_cost(mono(ManaCostShard::Black)) + .id(); + let first_family = scenario + .add_spell_to_hand_from_oracle(P0, "First Family", true, FIRST_FAMILY) + .with_mana_cost(first_family_cost()) + .id(); + + scenario.with_mana_pool(P0, full_pool()); + scenario.with_mana_pool(P1, pool(&[(ManaType::Black, 1)])); + let mut runner = scenario.build(); + + give_priority(&mut runner, P1); + runner.cast(opponent_spell).target_player(P1).resolve(); + + // Reach guard: the opponent's BLACK cast really did happen and really is + // journaled, so the negative below is not vacuous — the record exists and + // is simply out of scope. + assert_eq!( + runner + .state() + .spells_cast_this_turn_by_player + .get(&P1) + .map_or(0, |records| records.len()), + 1, + "reach guard: the opponent's cast must be journaled before we assert it is ignored" + ); + + give_priority(&mut runner, P0); + let outcome = runner.cast(first_family).resolve(); + + // {G (bear + FF), U (FF)} = 2. Black is in the OPPONENT's journal only; a + // `CountScope::All` reading would give 3. + outcome.assert_hand_drawn(P0, 2); + outcome.assert_life_delta(P0, 2); +} diff --git a/crates/engine/tests/integration/issue_4253_sanar_vivid.rs b/crates/engine/tests/integration/issue_4253_sanar_vivid.rs index ba3343347f..06383951a5 100644 --- a/crates/engine/tests/integration/issue_4253_sanar_vivid.rs +++ b/crates/engine/tests/integration/issue_4253_sanar_vivid.rs @@ -26,9 +26,9 @@ use engine::game::effects::resolve_ability_chain; use engine::game::scenario::{GameScenario, P0, P1}; use engine::parser::oracle_effect::parse_effect_chain; use engine::types::ability::{ - AbilityKind, CardPlayMode, CastFromZoneDriver, CastingPermission, Chooser, ControllerRef, - Effect, ForEachCategoryAction, IterationCategory, QuantityExpr, QuantityRef, ResolvedAbility, - RevealUntilDisposition, TargetFilter, ThisWayCause, TypeFilter, TypedFilter, + AbilityKind, CardPlayMode, CardTypeSetSource, CastFromZoneDriver, CastingPermission, Chooser, + ControllerRef, Effect, ForEachCategoryAction, IterationCategory, QuantityExpr, QuantityRef, + ResolvedAbility, RevealUntilDisposition, TargetFilter, ThisWayCause, TypeFilter, TypedFilter, }; use engine::types::actions::GameAction; use engine::types::card_type::CoreType; @@ -45,8 +45,12 @@ cast the exiled cards this turn."; fn distinct_colors_count() -> QuantityExpr { QuantityExpr::Ref { - qty: QuantityRef::DistinctColorsAmongPermanents { - filter: TargetFilter::Typed(TypedFilter::permanent().controller(ControllerRef::You)), + qty: QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { + filter: TargetFilter::Typed( + TypedFilter::permanent().controller(ControllerRef::You), + ), + }, }, } } diff --git a/crates/engine/tests/integration/l02_bb1_activation_conditions.rs b/crates/engine/tests/integration/l02_bb1_activation_conditions.rs index ffe470c74b..1aa5c046ab 100644 --- a/crates/engine/tests/integration/l02_bb1_activation_conditions.rs +++ b/crates/engine/tests/integration/l02_bb1_activation_conditions.rs @@ -15,10 +15,10 @@ use engine::game::restrictions::{check_activation_restrictions, record_battlefie use engine::game::scenario::{GameScenario, P0, P1}; use engine::parser::parse_oracle_text; use engine::types::ability::{ - AbilityCondition, AbilityDefinition, ActivationRestriction, AggregateFunction, Comparator, - ControllerRef, CountScope, DamageChannel, DamageKindFilter, FilterProp, ObjectScope, - ParsedCondition, PlayerFilter, PlayerRelation, PlayerScope, QuantityExpr, QuantityRef, - TargetFilter, TargetRef, TriggerCondition, TypeFilter, ZoneRef, + AbilityCondition, AbilityDefinition, ActivationRestriction, AggregateFunction, + CardTypeSetSource, Comparator, ControllerRef, CountScope, DamageChannel, DamageKindFilter, + FilterProp, ObjectScope, ParsedCondition, PlayerFilter, PlayerRelation, PlayerScope, + QuantityExpr, QuantityRef, TargetFilter, TargetRef, TriggerCondition, TypeFilter, ZoneRef, }; use engine::types::card_type::CoreType; use engine::types::counter::CounterType; @@ -318,7 +318,10 @@ fn s3_pucas_eye_parse_distinct_colors_eq_five() { ParsedCondition::QuantityComparison { lhs: QuantityExpr::Ref { - qty: QuantityRef::DistinctColorsAmongPermanents { filter }, + qty: + QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { filter }, + }, }, comparator: Comparator::EQ, rhs: QuantityExpr::Fixed { value: 5 }, @@ -332,7 +335,7 @@ fn s3_pucas_eye_parse_distinct_colors_eq_five() { ), other => panic!("expected Typed filter with controller You, got {other:?}"), }, - other => panic!("expected DistinctColorsAmongPermanents EQ 5, got {other:?}"), + other => panic!("expected DistinctColorsAmong(Objects) EQ 5, got {other:?}"), } } diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 64c8103ed2..3805946155 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -30,6 +30,7 @@ mod angels_grace_2hg; mod announce_locked_x_runtime; mod another_round_repeat; mod anya_merciless_angel_5920; +mod april_oneil_card_types_among_spells_cast; mod arashin_sovereign_self_tuck; mod archmage_ascension_gated_draw_replacement; mod archnemesis_you_attack_enchanted_player; @@ -241,6 +242,7 @@ mod field_of_ruin_search; mod fight_for_the_throne_monarch_gated_on_commander; mod finality_counter_death_to_exile; mod fireball_x_cost_surcharge_timing; +mod first_family_union_color_count; mod flashback_nonmana_payability; mod flickerwisp_delayed_return; mod floodpits_drowner; diff --git a/crates/mtgish-import/src/convert/quantity.rs b/crates/mtgish-import/src/convert/quantity.rs index 248fb18a43..068225fdff 100644 --- a/crates/mtgish-import/src/convert/quantity.rs +++ b/crates/mtgish-import/src/convert/quantity.rs @@ -804,23 +804,29 @@ pub fn convert(g: &GameNumber) -> ConvResult { qty: QuantityRef::ChosenNumber, }, - // CR 105 + CR 109.1: "the number of colors among [permanents]" — - // distinct colors across the matching permanent set. Composes with - // the permanents-filter converter; mirrors the parser's CDA mapping - // (oracle_quantity.rs DistinctColorsAmongPermanents). + // CR 105.1 + CR 105.2: "the number of colors among [permanents]" — + // distinct colors across the matching permanent set. Composes with the + // permanents-filter converter; mirrors the parser's CDA mapping. The + // engine slot is parameterized on `CardTypeSetSource` (the shared + // population axis), and a live object census is the `Objects` arm. GameNumber::NumColorsAmongPermanents(filter) => QuantityExpr::Ref { - qty: QuantityRef::DistinctColorsAmongPermanents { - filter: convert_permanents(filter)?, + qty: QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { + filter: convert_permanents(filter)?, + }, }, }, - // CR 105 + CR 109.1: "the number of colors of [permanent]" — the + // CR 105.1 + CR 105.2: "the number of colors of [permanent]" — the // single-permanent specialization. Engine slot is the same - // `DistinctColorsAmongPermanents { filter }` taking a one-permanent - // TargetFilter; the resolver counts distinct W/U/B/R/G across the - // resolved set (CR 105.1 — gold/multicolor/colorless are not colors). + // `DistinctColorsAmong { source: Objects { filter } }` taking a + // one-permanent TargetFilter; the resolver counts distinct W/U/B/R/G + // across the resolved set (CR 105.2 — gold/multicolor/colorless are not + // colors). GameNumber::NumColorsOfPermanent(perm) => QuantityExpr::Ref { - qty: QuantityRef::DistinctColorsAmongPermanents { - filter: convert_permanent(perm)?, + qty: QuantityRef::DistinctColorsAmong { + source: CardTypeSetSource::Objects { + filter: convert_permanent(perm)?, + }, }, },