From e22c02e1ebd7b11144c023e089f11aba2c4d294b Mon Sep 17 00:00:00 2001 From: Chris Swenson <1323803+ninthworld@users.noreply.github.com> Date: Fri, 7 Aug 2026 19:09:53 -0700 Subject: [PATCH] refactor(engine): one permanent filter for the one-shot classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First half of #824. `MassAffects` and `DestroyAffects` are gone; a shared `PermanentFilter` and one predicate answer "which permanents?" for every mass effect, every class-damage effect, and the sweeper. Of the classes the catalog actually authored, nine were used by exactly one card, and each was a *product* of axes the engine already modelled: controller relation, card type, subtype, colour, a keyword, a power bound, whether it is attacking, whether it is a token. `PermanentCount` was already that product and its own doc said why — "a count is asked about an open-ended variety of things, and enumerating each as its own variant would grow the vocabulary once per card". This is that struct, generalised until the other selectors fit in it. `DestroyAffects` existed only because `MassAffects` was creature-only, and it disappears: a sweeper's class is a filter like any other, and `destroy all artifacts and enchantments` is one card type list rather than a named variant. The one thing that genuinely differs between callers is now named rather than duplicated. `Reading` says whether the questions are answered from the computed characteristics or from the printed face, because a class named by a resolution is asked from outside the CR 613 layer system and a class named by a static ability's own selector is asked from inside the walk that produces it. That asymmetry is the recursion, not a difference of opinion — it was stated in prose on two selectors and is stated once in a type now. The rules-text side generates the noun phrase from the filter instead of holding a phrase per named class, in both the subject and the recipient position. One text changes: a card that prints "each creature and planeswalker they control" after naming its opponents now reads "…your opponents control". The pronoun is a discourse feature of the sentence before it rather than a property of the class, and this formatter states behaviour rather than reproducing Oracle text. Still to come on this issue: `StaticAffects` and `PermanentCount`, the validator refusals that go with `Reading::Printed`, and the two `exclusions.json` entries those unlock. Part of #824, part of #819. Co-Authored-By: Claude Opus 5 (1M context) --- .../data/catalog/ajani_wise_counselor.json | 2 +- .../data/catalog/angel_of_the_dawn.json | 8 +- .../data/catalog/cleansing_nova.json | 9 +- .../data/catalog/crash_through.json | 2 +- .../catalog/goreclaw_terror_of_qal_sisma.json | 4 +- .../data/catalog/heroic_reinforcements.json | 4 +- .../data/catalog/inspired_charge.json | 2 +- .../data/catalog/lathliss_dragon_queen.json | 2 +- .../data/catalog/lena_selfless_champion.json | 2 +- .../data/catalog/make_a_stand.json | 4 +- .../sage-engine/data/catalog/plague_mare.json | 5 +- .../data/catalog/radiating_lightning.json | 3 +- .../data/catalog/sarkhan_dragonsoul.json | 3 +- .../data/catalog/sarkhan_s_unsealing.json | 6 +- .../data/catalog/tectonic_rift.json | 4 +- .../data/catalog/tezzeret_s_gatebreaker.json | 2 +- .../data/catalog/trumpet_blast.json | 4 +- .../data/catalog/uncomfortable_chill.json | 3 +- .../data/catalog/valiant_knight.json | 2 +- .../data/catalog/vivien_of_the_arkbow.json | 4 +- crates/sage-engine/src/ability.rs | 2 + crates/sage-engine/src/ability/effect.rs | 10 +- crates/sage-engine/src/ability/filter.rs | 254 ++++++++++++++++++ crates/sage-engine/src/ability/selector.rs | 128 +-------- .../sage-engine/src/ability/static_ability.rs | 2 +- crates/sage-engine/src/ability/tests.rs | 72 +++-- crates/sage-engine/src/apply/cast.rs | 6 +- crates/sage-engine/src/apply/cast/effects.rs | 202 +++----------- .../src/apply/cast/effects/tests.rs | 26 +- crates/sage-engine/src/filter.rs | 202 ++++++++++++++ crates/sage-engine/src/lib.rs | 13 +- crates/sage-engine/src/resolve.rs | 2 +- crates/sage-server/src/rules_text.rs | 16 +- crates/sage-server/src/rules_text/effects.rs | 196 +++++++------- crates/sage-server/src/rules_text/tests.rs | 6 +- 35 files changed, 738 insertions(+), 474 deletions(-) create mode 100644 crates/sage-engine/src/ability/filter.rs create mode 100644 crates/sage-engine/src/filter.rs diff --git a/crates/sage-engine/data/catalog/ajani_wise_counselor.json b/crates/sage-engine/data/catalog/ajani_wise_counselor.json index 4773abb6..8cd7d1aa 100644 --- a/crates/sage-engine/data/catalog/ajani_wise_counselor.json +++ b/crates/sage-engine/data/catalog/ajani_wise_counselor.json @@ -48,7 +48,7 @@ { "kind": "pump_all", "affects": { - "scope": "creatures_you_control" + "card_type": "creature" }, "power": 2, "toughness": 2 diff --git a/crates/sage-engine/data/catalog/angel_of_the_dawn.json b/crates/sage-engine/data/catalog/angel_of_the_dawn.json index 887e6a2b..6514d1e3 100644 --- a/crates/sage-engine/data/catalog/angel_of_the_dawn.json +++ b/crates/sage-engine/data/catalog/angel_of_the_dawn.json @@ -25,16 +25,16 @@ { "kind": "pump_all", "affects": { - "scope": "creatures_you_control" - }, + "card_type": "creature" + }, "power": 1, "toughness": 1 }, { "kind": "grant_keyword_all", "affects": { - "scope": "creatures_you_control" - }, + "card_type": "creature" + }, "keyword": "vigilance" } ] diff --git a/crates/sage-engine/data/catalog/cleansing_nova.json b/crates/sage-engine/data/catalog/cleansing_nova.json index fcd2da8c..625a24da 100644 --- a/crates/sage-engine/data/catalog/cleansing_nova.json +++ b/crates/sage-engine/data/catalog/cleansing_nova.json @@ -15,7 +15,8 @@ { "kind": "destroy_all", "affects": { - "scope": "each_creature" + "scope": "any", + "card_type": "creature" } } ] @@ -25,7 +26,11 @@ { "kind": "destroy_all", "affects": { - "scope": "each_artifact_or_enchantment" + "scope": "any", + "card_type": [ + "artifact", + "enchantment" + ] } } ] diff --git a/crates/sage-engine/data/catalog/crash_through.json b/crates/sage-engine/data/catalog/crash_through.json index d83b5872..3f323ded 100644 --- a/crates/sage-engine/data/catalog/crash_through.json +++ b/crates/sage-engine/data/catalog/crash_through.json @@ -13,7 +13,7 @@ { "kind": "grant_keyword_all", "affects": { - "scope": "creatures_you_control" + "card_type": "creature" }, "keyword": "trample" }, diff --git a/crates/sage-engine/data/catalog/goreclaw_terror_of_qal_sisma.json b/crates/sage-engine/data/catalog/goreclaw_terror_of_qal_sisma.json index 6aacc85f..1203b512 100644 --- a/crates/sage-engine/data/catalog/goreclaw_terror_of_qal_sisma.json +++ b/crates/sage-engine/data/catalog/goreclaw_terror_of_qal_sisma.json @@ -37,7 +37,7 @@ { "kind": "pump_all", "affects": { - "scope": "creatures_you_control", + "card_type": "creature", "min_power": 4 }, "power": 1, @@ -46,7 +46,7 @@ { "kind": "grant_keyword_all", "affects": { - "scope": "creatures_you_control", + "card_type": "creature", "min_power": 4 }, "keyword": "trample" diff --git a/crates/sage-engine/data/catalog/heroic_reinforcements.json b/crates/sage-engine/data/catalog/heroic_reinforcements.json index f69abbd4..c867f272 100644 --- a/crates/sage-engine/data/catalog/heroic_reinforcements.json +++ b/crates/sage-engine/data/catalog/heroic_reinforcements.json @@ -32,7 +32,7 @@ { "kind": "pump_all", "affects": { - "scope": "creatures_you_control" + "card_type": "creature" }, "power": 1, "toughness": 1 @@ -40,7 +40,7 @@ { "kind": "grant_keyword_all", "affects": { - "scope": "creatures_you_control" + "card_type": "creature" }, "keyword": "haste" } diff --git a/crates/sage-engine/data/catalog/inspired_charge.json b/crates/sage-engine/data/catalog/inspired_charge.json index ed5fefee..6fdbd3ac 100644 --- a/crates/sage-engine/data/catalog/inspired_charge.json +++ b/crates/sage-engine/data/catalog/inspired_charge.json @@ -13,7 +13,7 @@ { "kind": "pump_all", "affects": { - "scope": "creatures_you_control" + "card_type": "creature" }, "power": 2, "toughness": 1 diff --git a/crates/sage-engine/data/catalog/lathliss_dragon_queen.json b/crates/sage-engine/data/catalog/lathliss_dragon_queen.json index b4d584ac..4e83de84 100644 --- a/crates/sage-engine/data/catalog/lathliss_dragon_queen.json +++ b/crates/sage-engine/data/catalog/lathliss_dragon_queen.json @@ -66,7 +66,7 @@ { "kind": "pump_all", "affects": { - "scope": "creatures_you_control", + "card_type": "creature", "subtype": "Dragon" }, "power": 1, diff --git a/crates/sage-engine/data/catalog/lena_selfless_champion.json b/crates/sage-engine/data/catalog/lena_selfless_champion.json index c3ac23ec..a5cca0ae 100644 --- a/crates/sage-engine/data/catalog/lena_selfless_champion.json +++ b/crates/sage-engine/data/catalog/lena_selfless_champion.json @@ -59,7 +59,7 @@ { "kind": "grant_keyword_all", "affects": { - "scope": "creatures_you_control", + "card_type": "creature", "below_source_power": true }, "keyword": "indestructible" diff --git a/crates/sage-engine/data/catalog/make_a_stand.json b/crates/sage-engine/data/catalog/make_a_stand.json index 16e1db69..31b54e3d 100644 --- a/crates/sage-engine/data/catalog/make_a_stand.json +++ b/crates/sage-engine/data/catalog/make_a_stand.json @@ -13,7 +13,7 @@ { "kind": "pump_all", "affects": { - "scope": "creatures_you_control" + "card_type": "creature" }, "power": 1, "toughness": 0 @@ -21,7 +21,7 @@ { "kind": "grant_keyword_all", "affects": { - "scope": "creatures_you_control" + "card_type": "creature" }, "keyword": "indestructible" } diff --git a/crates/sage-engine/data/catalog/plague_mare.json b/crates/sage-engine/data/catalog/plague_mare.json index a6fe87fd..e8f4bcaf 100644 --- a/crates/sage-engine/data/catalog/plague_mare.json +++ b/crates/sage-engine/data/catalog/plague_mare.json @@ -28,8 +28,9 @@ { "kind": "pump_all", "affects": { - "scope": "creatures_your_opponents_control" - }, + "scope": "opponents_control", + "card_type": "creature" + }, "power": -1, "toughness": -1 } diff --git a/crates/sage-engine/data/catalog/radiating_lightning.json b/crates/sage-engine/data/catalog/radiating_lightning.json index 8e3d46ca..d3fb4c9d 100644 --- a/crates/sage-engine/data/catalog/radiating_lightning.json +++ b/crates/sage-engine/data/catalog/radiating_lightning.json @@ -18,7 +18,8 @@ { "kind": "deal_damage", "affects": { - "scope": "creatures_that_player_controls" + "scope": "that_player", + "card_type": "creature" }, "amount": 1 } diff --git a/crates/sage-engine/data/catalog/sarkhan_dragonsoul.json b/crates/sage-engine/data/catalog/sarkhan_dragonsoul.json index b4370f98..f2a0f986 100644 --- a/crates/sage-engine/data/catalog/sarkhan_dragonsoul.json +++ b/crates/sage-engine/data/catalog/sarkhan_dragonsoul.json @@ -34,7 +34,8 @@ { "kind": "deal_damage", "affects": { - "scope": "creatures_your_opponents_control" + "scope": "opponents_control", + "card_type": "creature" }, "amount": 1 } diff --git a/crates/sage-engine/data/catalog/sarkhan_s_unsealing.json b/crates/sage-engine/data/catalog/sarkhan_s_unsealing.json index 975550e5..b1a810cf 100644 --- a/crates/sage-engine/data/catalog/sarkhan_s_unsealing.json +++ b/crates/sage-engine/data/catalog/sarkhan_s_unsealing.json @@ -46,7 +46,11 @@ { "kind": "deal_damage", "affects": { - "scope": "creatures_and_planeswalkers_your_opponents_control" + "scope": "opponents_control", + "card_type": [ + "creature", + "planeswalker" + ] }, "amount": 4 } diff --git a/crates/sage-engine/data/catalog/tectonic_rift.json b/crates/sage-engine/data/catalog/tectonic_rift.json index 8a7c9593..051d569b 100644 --- a/crates/sage-engine/data/catalog/tectonic_rift.json +++ b/crates/sage-engine/data/catalog/tectonic_rift.json @@ -17,7 +17,9 @@ { "kind": "restrict_all", "affects": { - "scope": "creatures_without_flying" + "scope": "any", + "card_type": "creature", + "without_keyword": "flying" }, "restriction": "cant_block" } diff --git a/crates/sage-engine/data/catalog/tezzeret_s_gatebreaker.json b/crates/sage-engine/data/catalog/tezzeret_s_gatebreaker.json index 2de8ab37..5d58c50f 100644 --- a/crates/sage-engine/data/catalog/tezzeret_s_gatebreaker.json +++ b/crates/sage-engine/data/catalog/tezzeret_s_gatebreaker.json @@ -42,7 +42,7 @@ { "kind": "restrict_all", "affects": { - "scope": "creatures_you_control" + "card_type": "creature" }, "restriction": "cant_be_blocked" } diff --git a/crates/sage-engine/data/catalog/trumpet_blast.json b/crates/sage-engine/data/catalog/trumpet_blast.json index f6a27979..1e1b79f4 100644 --- a/crates/sage-engine/data/catalog/trumpet_blast.json +++ b/crates/sage-engine/data/catalog/trumpet_blast.json @@ -13,7 +13,9 @@ { "kind": "pump_all", "affects": { - "scope": "attacking_creatures" + "scope": "any", + "card_type": "creature", + "attacking": true }, "power": 2, "toughness": 0 diff --git a/crates/sage-engine/data/catalog/uncomfortable_chill.json b/crates/sage-engine/data/catalog/uncomfortable_chill.json index b61ecc02..adec56df 100644 --- a/crates/sage-engine/data/catalog/uncomfortable_chill.json +++ b/crates/sage-engine/data/catalog/uncomfortable_chill.json @@ -13,7 +13,8 @@ { "kind": "pump_all", "affects": { - "scope": "creatures_your_opponents_control" + "scope": "opponents_control", + "card_type": "creature" }, "power": -2, "toughness": 0 diff --git a/crates/sage-engine/data/catalog/valiant_knight.json b/crates/sage-engine/data/catalog/valiant_knight.json index 288e7b2b..a050e773 100644 --- a/crates/sage-engine/data/catalog/valiant_knight.json +++ b/crates/sage-engine/data/catalog/valiant_knight.json @@ -41,7 +41,7 @@ { "kind": "grant_keyword_all", "affects": { - "scope": "creatures_you_control", + "card_type": "creature", "subtype": "Knight" }, "keyword": "double_strike" diff --git a/crates/sage-engine/data/catalog/vivien_of_the_arkbow.json b/crates/sage-engine/data/catalog/vivien_of_the_arkbow.json index 69ef3b6a..00673455 100644 --- a/crates/sage-engine/data/catalog/vivien_of_the_arkbow.json +++ b/crates/sage-engine/data/catalog/vivien_of_the_arkbow.json @@ -65,7 +65,7 @@ { "kind": "pump_all", "affects": { - "scope": "creatures_you_control" + "card_type": "creature" }, "power": 4, "toughness": 4 @@ -73,7 +73,7 @@ { "kind": "grant_keyword_all", "affects": { - "scope": "creatures_you_control" + "card_type": "creature" }, "keyword": "trample" } diff --git a/crates/sage-engine/src/ability.rs b/crates/sage-engine/src/ability.rs index 6c892ee3..24ab8364 100644 --- a/crates/sage-engine/src/ability.rs +++ b/crates/sage-engine/src/ability.rs @@ -22,6 +22,7 @@ use crate::token::TokenData; mod cost; mod effect; +mod filter; mod selector; mod static_ability; mod target; @@ -29,6 +30,7 @@ mod trigger; pub use cost::*; pub use effect::*; +pub use filter::*; pub use selector::*; pub use static_ability::*; pub use target::*; diff --git a/crates/sage-engine/src/ability/effect.rs b/crates/sage-engine/src/ability/effect.rs index 7bfb320e..987dd905 100644 --- a/crates/sage-engine/src/ability/effect.rs +++ b/crates/sage-engine/src/ability/effect.rs @@ -90,7 +90,7 @@ pub enum Effect { /// The subject is a [`PlayerRef`] exactly as [`Effect::Mill`]'s is, and decides on /// its own whether a target is chosen: `target_player` fills a slot and can fizzle, /// `each_opponent` fills none and cannot. It is deliberately **not** a - /// [`MassAffects`] class: every one of those is read relative to the effect's + /// [`PermanentFilter`] class: every one of those is read relative to the effect's /// controller and none of them targets, so "creatures *that player* controls" is /// unsayable in that vocabulary and sayable in this one without inventing anything. /// @@ -174,7 +174,7 @@ pub enum Effect { /// Indestructible and regeneration are unmodeled, so nothing survives this. DestroyAll { /// The class of permanents destroyed. - affects: DestroyAffects, + affects: PermanentFilter, }, /// Exile the single permanent this effect targets (CR 406.2 / CR 701.19): it is /// moved from the battlefield to its owner's exile zone through the one @@ -472,7 +472,7 @@ pub enum Effect { /// difference between a one-shot pump and an anthem. PumpAll { /// The class of permanents modified. - affects: MassAffects, + affects: PermanentFilter, /// The signed amount added to each affected permanent's power. power: i32, /// The signed amount added to each affected permanent's toughness. @@ -484,7 +484,7 @@ pub enum Effect { /// affected set in on resolution exactly as [`Effect::PumpAll`] does. GrantKeywordAll { /// The class of permanents granted the keyword. - affects: MassAffects, + affects: PermanentFilter, /// The keyword ability granted until end of turn. keyword: Keyword, }, @@ -532,7 +532,7 @@ pub enum Effect { /// on resolution exactly as [`Effect::PumpAll`] does (CR 611.2c). RestrictAll { /// The class of permanents restricted. - affects: MassAffects, + affects: PermanentFilter, /// The restriction imposed until end of turn. restriction: CombatRestriction, }, diff --git a/crates/sage-engine/src/ability/filter.rs b/crates/sage-engine/src/ability/filter.rs new file mode 100644 index 00000000..cd69f366 --- /dev/null +++ b/crates/sage-engine/src/ability/filter.rs @@ -0,0 +1,254 @@ +//! One class of permanents, asked by everything that names one. +//! +//! Four selectors used to answer the same question — *which permanents?* — and none of +//! them composed: `MassAffects` for a one-shot class, +//! `StaticAffects` for a continuous one, `DestroyAffects` for a sweeper, and +//! [`PermanentCount`](crate::PermanentCount) for a count. Between them, nine of the +//! classes actually authored in the catalog were used by exactly **one** card, and each +//! new phrasing cost a variant plus an arm in every consumer across two crates +//! (issue #824). +//! +//! Nearly every one of them was a *product* of axes the engine already modelled — +//! controller relation, card type, subtype, colour, a keyword, a power bound, whether it +//! is attacking, whether it is a token. [`PermanentCount`] was already that product, and +//! its own doc said why: +//! +//! > Deliberately a small product of independent filters rather than a closed list of +//! > named classes: a count is asked about an open-ended variety of things, and +//! > enumerating each as its own variant would grow the vocabulary once per card. +//! +//! This is that struct, generalised until the other three are expressible in it. + +use serde::{Deserialize, Deserializer}; + +use crate::card::Keyword; +use crate::card_type::CardType; +use crate::mana::Color; +use crate::state::CounterKind; + +/// Whose permanents a selector names, relative to the **reading object's controller** — +/// the controller of the spell, ability, or static ability doing the asking. +/// +/// Controller-relative rather than by seat, which is what lets one authored card mean +/// "you" from either side of the table. A seat that has lost the game is no longer an +/// opponent (CR 102.1), and [`Self::OpponentsControl`] excludes it everywhere. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ControllerScope { + /// The reading object's controller — "you control". The default, because it is what + /// most printed classes say. + #[default] + YouControl, + /// Every opponent of the reading object's controller, and none of their own. + OpponentsControl, + /// The player this resolution's most recent targeting effect **named** — the `each + /// creature **that player** controls` of a spell that hits a player and their board + /// in one sentence. + /// + /// The class counterpart of [`PlayerRef`](crate::PlayerRef)'s `ThatPlayer`, reading the same fact for the + /// same reason: the choice belongs to the sentence before it. Names nobody in a + /// resolution that aimed at nothing, which is a card that could not have been + /// written. + ThatPlayer, + /// Every permanent on the battlefield, whoever controls it — the symmetric class a + /// sweeper names. + Any, +} + +/// How a filter reads the characteristics it asks about. +/// +/// The one thing about this predicate that is **not** the same everywhere it is used, and +/// it is a rules fact rather than an inconsistency. A class named by a resolution, a +/// count, or a trigger is asked from outside the CR 613 layer system, so every question +/// is answered from the permanent's *current* characteristics. A class named by a +/// **static ability's own selector** is asked from inside the layer-6/7 walk that +/// produces those characteristics, so it has to read the printed face or it would ask +/// the computation for the answer it is in the middle of producing. +/// +/// The asymmetry is the recursion, not a difference of opinion — the same sentence the +/// old `StaticAffects` carried, now stated once instead of per selector. The catalog +/// validator refuses the fields that cannot be answered under [`Self::Printed`] rather +/// than letting them silently read the wrong thing. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Reading { + /// From **outside** the layer system: a resolution (CR 611.2c), a count, a + /// condition, a trigger diff. Types, keywords, power and toughness are the computed + /// ones (CR 613.1f), so an animated artifact is a creature and a pumped 3/3 has + /// power 4. + Computed, + /// From **inside** the layer walk: a printed static ability selecting the permanents + /// it modifies. Everything is read off the printed face. + Printed, +} + +/// What a filter needs to know about the object doing the asking. +/// +/// Small and `Copy`, because every field is either a handle or a number the caller +/// already has: nothing here is derived, and the predicate derives nothing that outlives +/// one call. +#[derive(Clone, Copy, Debug)] +pub struct FilterContext { + /// The controller of the object naming the class — what every + /// [`ControllerScope`] is relative to. + pub controller: crate::id::PlayerId, + /// The permanent the class is named *by*, when there is one. `None` for a spell's + /// own effects and for an emblem, neither of which is a permanent — which is exactly + /// why [`PermanentFilter::except_this`] and + /// [`PermanentFilter::with_the_named_card`] then match nothing rather than + /// everything. + pub source: Option, + /// The source's power, read by the caller **before** any cost was paid, for + /// [`PermanentFilter::below_source_power`]. A source that is already gone took its + /// power with it, and the class is then empty rather than universal. + pub source_power: Option, + /// The player a targeting effect earlier in this same resolution named, for + /// [`ControllerScope::ThatPlayer`]. + pub chosen_player: Option, + /// Which characteristics the questions are answered from. + pub reading: Reading, +} + +/// One or more printed card types, satisfied by **any** of them. +/// +/// Authored as a bare string for the ordinary one-type class and as an array for a +/// disjunction — `"creature"` and `["artifact", "enchantment"]`. One key rather than two +/// spellings of two keys, because a card prints one class either way: *destroy all +/// artifacts and enchantments* is one destruction, and a permanent that is both is +/// destroyed once. +fn card_types<'de, D: Deserializer<'de>>(deserializer: D) -> Result, D::Error> { + #[derive(Deserialize)] + #[serde(untagged)] + enum OneOrMany { + One(CardType), + Many(Vec), + } + Ok(match OneOrMany::deserialize(deserializer)? { + OneOrMany::One(card_type) => vec![card_type], + OneOrMany::Many(types) => types, + }) +} + +/// A class of permanents, as a product of the axes a printed card actually narrows by. +/// +/// Every field absent is "every permanent on the battlefield the scope allows"; each one +/// present narrows further, and they are conjunctive. Growing the vocabulary is adding a +/// field here — once — rather than a variant in four enums and an arm in six exhaustive +/// matches. +#[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct PermanentFilter { + /// Whose permanents. Defaults to the reading object's controller's own. + #[serde(default)] + pub scope: ControllerScope, + /// Printed card types, satisfied by any one of them. Empty names every type. + #[serde(default, deserialize_with = "card_types")] + pub card_type: Vec, + /// A printed subtype the permanent must have — the `Elves` of a lord, the `Dragons` + /// of a mass pump. Absent names every subtype. + #[serde(default)] + pub subtype: Option, + /// A printed colour (CR 105.2) the permanent must be. Absent names every colour, + /// colourless included. + /// + /// Printed rather than computed even under [`Reading::Computed`], because CR 613 + /// layer 5 is not modelled for permanents on the battlefield: printed colour *is* + /// current colour here, exactly as it is in the blocking restrictions that name one. + #[serde(default)] + pub color: Option, + /// A keyword the permanent must have — the `with defender` of a class that names + /// one. + /// + /// Read through the computed keyword set under [`Reading::Computed`], so a *granted* + /// defender is in the class; off the printed face under [`Reading::Printed`], where + /// asking for the computed set would not terminate. + #[serde(default)] + pub keyword: Option, + /// A keyword the permanent must **not** have — the `without flying` of an effect + /// that clears the ground. Read exactly as [`Self::keyword`] is. + #[serde(default)] + pub without_keyword: Option, + /// The least power the permanent may have — `with power 4 or greater`. + /// + /// Read through the **computed** characteristics, which is the only reading a printed + /// card means, and therefore refusable under [`Reading::Printed`]: the catalog + /// validator rejects it in a static ability's selector and in an attachment's count + /// rather than letting it recurse + /// ([`Violation::PowerInStaticCondition`](crate::Violation)). + #[serde(default)] + pub min_power: Option, + /// The greatest toughness the permanent may have — the bound the old selectors could + /// not express at all. Read and restricted exactly as [`Self::min_power`] is. + #[serde(default)] + pub max_toughness: Option, + /// Power strictly less than the **source's** — the `creatures you control with power + /// less than Lena's power` of a sacrifice that protects the small. + /// + /// A flag rather than a value, because the number it compares against is not knowable + /// when the card is authored. A source that has left — sacrificed to its own cost, + /// which is exactly what Lena does — took its power with it, and the class is then + /// empty rather than everything. + #[serde(default)] + pub below_source_power: bool, + /// Only permanents currently **attacking** (CR 508.1a) — the class a combat pump + /// names. Empty outside combat, which makes such a spell a legal but pointless + /// main-phase cast rather than an uncastable one. + #[serde(default)] + pub attacking: bool, + /// Whether the permanent must be a token (`true`) or must not be (`false`); absent + /// takes both. + /// + /// Read off what the permanent *is* rather than inferred: a token has no card at all + /// (CR 111), which the state already records. + #[serde(default)] + pub token: Option, + /// At least one counter of this kind on the permanent — the `permanents with + /// **phylactery counters** on them` a Lich's life depends on. + /// + /// The one field here that is not a characteristic: a counter is not produced by any + /// layer, so unlike [`Self::min_power`] it can be asked from anywhere, static + /// selectors included. + #[serde(default)] + pub with_counter: Option, + /// Only the permanent whose printed card the **source named as it entered** + /// (CR 614.12) — the `with the chosen name` of a card that asks for one. + /// + /// Compares card identity, not a string: two printings of one functional card share a + /// [`CardId`](crate::CardId) and nothing else does, and a token has no card to bear a + /// name anyone named. A source that named nothing matches nothing. + #[serde(default)] + pub with_the_named_card: bool, + /// Exclude the source itself — the `other` in "other Elves you control". + /// + /// Compares the specific object: a [`PermanentId`](crate::PermanentId) is minted + /// fresh on every battlefield entry, so two copies of one lord do pump each other. An + /// object with no source permanent excludes nothing. + #[serde(default)] + pub except_this: bool, +} + +impl PermanentFilter { + /// The everything-the-controller-controls filter, for a caller building one in code. + #[must_use] + pub fn you_control() -> Self { + Self::default() + } + + /// Narrow to one printed card type. + #[must_use] + pub fn of_type(mut self, card_type: CardType) -> Self { + self.card_type = vec![card_type]; + self + } + + /// Whether this filter asks a question that cannot be answered from inside the layer + /// system — the fields the catalog validator refuses in a static ability's selector + /// and in an attachment's count. + /// + /// Stated here, beside the fields, so the rule and the reason live together: a bound + /// on a computed power or toughness asked from within the computation of a + /// permanent's characteristics would not terminate. + #[must_use] + pub fn reads_computed_power(&self) -> bool { + self.min_power.is_some() || self.max_toughness.is_some() || self.below_source_power + } +} diff --git a/crates/sage-engine/src/ability/selector.rs b/crates/sage-engine/src/ability/selector.rs index e7ddeadb..5201a27b 100644 --- a/crates/sage-engine/src/ability/selector.rs +++ b/crates/sage-engine/src/ability/selector.rs @@ -254,31 +254,6 @@ pub enum DerivedAmount { of: HalvedTotal, }, } - -/// The class of permanents a **mass destruction** puts into their owners' graveyards -/// (CR 701.7) — the `all creatures` and the `all artifacts and enchantments` of a -/// sweeper's two modes. -/// -/// Its own vocabulary rather than a widening of [`MassAffects`], which every existing -/// member of is a class of *creatures* feeding a pump or a keyword grant: a -/// non-creature scope there would make "artifacts you control get +1/+1" an authorable -/// sentence that means nothing. Closed and named, for [`MassAffects`]'s reason — a -/// disjunction of two card types is not a product of independent filters and -/// [`PermanentCount`] could not say it — and it grows by adding variants. -/// -/// The affected set is enumerated **on resolution** (CR 611.2c), so a permanent that -/// arrives afterwards survives. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize)] -#[serde(tag = "scope", rename_all = "snake_case")] -pub enum DestroyAffects { - /// Every creature on the battlefield, whoever controls it. - EachCreature, - /// Every artifact and every enchantment on the battlefield, whoever controls it. One - /// class rather than two, because the printed sentence is one destruction and a - /// permanent that is both is destroyed once. - EachArtifactOrEnchantment, -} - /// A total a [`DerivedAmount::HalfRoundedUp`] takes half of, asked about the player the /// effect names. /// @@ -346,7 +321,7 @@ pub struct GraveyardCount { /// A class of permanents to **count**, relative to an effect's controller. /// /// Deliberately a small product of three independent filters rather than a closed list -/// of named classes like [`MassAffects`]: a count is asked about an open-ended variety +/// of named classes: a count is asked about an open-ended variety /// of things ("artifacts you control", "Zombies you control"), and enumerating each as /// its own variant would grow the vocabulary once per card. Nothing here selects /// permanents to *modify*, so the `except_this` and creature-only assumptions the @@ -602,105 +577,6 @@ pub enum CardFilter { color: Color, }, } - -/// The class of permanents a **mass, non-targeting** effect ([`Effect::PumpAll`], -/// [`Effect::GrantKeywordAll`]) applies to. -/// -/// Deliberately separate from [`StaticAffects`], which selects for a *continuous* -/// ability and carries an `except_this` that a one-shot spell has no "this" for — but -/// authored in the same internally tagged shape, so the two selectors read alike: -/// `{"kind":"pump_all","affects":{"scope":"creatures_you_control"},"power":2,"toughness":1}`. -/// It grows by adding variants (attacking creatures, tapped creatures, …) as cards -/// need them. -#[derive(Clone, Debug, PartialEq, Eq, Deserialize)] -#[serde(tag = "scope", rename_all = "snake_case")] -pub enum MassAffects { - /// Every creature the effect's controller controls at the moment it resolves. - CreaturesYouControl { - /// Restrict to creatures whose subtypes include this one — the `Dragons` of - /// "Dragons you control get +1/+0 until end of turn". Absent means every - /// creature its controller controls. - #[serde(default)] - subtype: Option, - /// Restrict to creatures whose power is at least this — the "each creature you - /// control **with power 4 or greater**" of an attack trigger. Absent means - /// every power, including a creature that has none. - /// - /// Read through the **computed** characteristics (CR 613.1f), like the - /// identically-named field on [`PermanentCount`] and for the same reason: that - /// is the only reading a printed card means, so a creature pumped to 4 is in the - /// class and one shrunk out of it is not. Asking for a computed power here is - /// safe where it is not in a static ability's condition — a mass effect is - /// enumerated **during a resolution** (CR 611.2c), from outside the layer - /// system, so there is no computation to recurse into. - #[serde(default)] - min_power: Option, - /// Restrict to creatures whose power is **strictly less than the source's** — the - /// "creatures you control with power less than Lena's power" of a sacrifice that - /// protects the small. - /// - /// A bound relative to another permanent rather than to a printed number, which is - /// why it is its own flag beside [`min_power`](Self::CreaturesYouControl::min_power) - /// rather than a value: the number it compares against is not knowable when the card - /// is authored, and it changes with the source. - /// - /// Both sides are read through the **computed** characteristics at the moment of - /// resolution (CR 613.1f / CR 611.2c), so a source pumped before the ability resolves - /// protects more, and a creature pumped past it drops out. Safe for the same reason - /// `min_power` is: a mass effect is enumerated from inside a resolution, outside the - /// layer system, so there is no computation to recurse into. - /// - /// A source that has **left** — sacrificed to its own cost, which is exactly what - /// Lena does — takes its power with it, and the class is then empty rather than - /// everything: "less than Lena's power" with no Lena is not a bound that lets every - /// creature in. The caller reads the source's power *before* paying the cost and - /// passes it in. - #[serde(default)] - below_source_power: bool, - }, - /// Every creature on the battlefield at the moment the effect resolves, - /// whoever controls it — the symmetric class a sweeper names. - EachCreature, - /// Every creature controlled by an opponent of the effect's controller, at the - /// moment it resolves. The mirror of [`Self::CreaturesYouControl`], and the - /// reason both are relative to the controller rather than to a seat: one - /// authored card must mean "you" from either side of the table. - CreaturesYourOpponentsControl, - /// Every **creature and planeswalker** an opponent of the effect's controller - /// controls — the wider class a sweeper that also burns walkers names. - /// - /// A class rather than two, because the card prints it as one breath: `deals 4 damage - /// to each opponent and each creature and planeswalker they control`. Damage to a - /// planeswalker removes loyalty (CR 120.3c) at the same seam damage to a creature is - /// marked, so nothing about the verb has to know which it hit. - CreaturesAndPlaneswalkersYourOpponentsControl, - /// Every creature controlled by the player this resolution's most recent targeted - /// effect named — the `each creature **that player** controls` of a spell that hits a - /// player and their board in one sentence. - /// - /// The class counterpart of [`PlayerRef::ThatPlayer`], reading the same fact for the - /// same reason: the choice belongs to the sentence before it. - CreaturesThatPlayerControls, - /// Every creature on the battlefield that does not currently have flying, whoever - /// controls it — the scope of an effect that clears the ground. - /// - /// Flying is read through the computed keywords (CR 613.1f), so a creature that was - /// *granted* flying is outside the class exactly as a printed flyer is. The class is - /// still evaluated once, on resolution (CR 611.2c), like every other mass effect: a - /// creature that loses flying later in the turn does not retroactively join it. - CreaturesWithoutFlying, - /// Every creature currently **attacking**, whoever controls it — the class a combat - /// pump names (`Attacking creatures get +2/+0 until end of turn.`). - /// - /// Read off [`Permanent::attacking`](crate::Permanent), so it is exactly the set the - /// declare-attackers step produced, and it is locked in on resolution like every - /// other mass class (CR 611.2c): a creature removed from combat afterwards keeps the - /// pump, and one that was never in it never had one. The class is empty outside - /// combat, which makes such a spell a legal but pointless main-phase cast rather - /// than an uncastable one. - AttackingCreatures, -} - /// **Who or what** an [`Effect::DealDamage`] deals its damage to (CR 120.3). /// /// The same design [`PlayerRef`] states for life change: the *subject* declares @@ -736,7 +612,7 @@ pub enum DamageSubject { /// A class of **permanents**, named by the same selector mass pump takes. Never /// a target, and so never a fizzle. #[serde(rename = "affects")] - Permanents(MassAffects), + Permanents(PermanentFilter), } impl DamageSubject { diff --git a/crates/sage-engine/src/ability/static_ability.rs b/crates/sage-engine/src/ability/static_ability.rs index 3e0035ca..95960968 100644 --- a/crates/sage-engine/src/ability/static_ability.rs +++ b/crates/sage-engine/src/ability/static_ability.rs @@ -77,7 +77,7 @@ pub enum StaticAffects { /// /// Like every other selector it is **re-derived on every read**, which is the entire /// distinction between a static ability and the resolution-time - /// [`MassAffects`](crate::MassAffects) class a sweeper names: a land that arrives + /// [`PermanentFilter`](crate::PermanentFilter) class a sweeper names: a land that arrives /// under an opponent after the source did is affected the instant it arrives, one /// that changes hands leaves the class at CR 613 layer 2, and everything stops the /// instant the source leaves the battlefield. diff --git a/crates/sage-engine/src/ability/tests.rs b/crates/sage-engine/src/ability/tests.rs index 18251b42..22225a28 100644 --- a/crates/sage-engine/src/ability/tests.rs +++ b/crates/sage-engine/src/ability/tests.rs @@ -417,13 +417,17 @@ fn issue_611_deal_damage_round_trips_with_a_class_of_players_or_permanents() { } ); let permanents: Effect = serde_json::from_str( - r#"{"kind":"deal_damage","affects":{"scope":"each_creature"},"amount":1}"#, + r#"{"kind":"deal_damage","affects":{"scope":"any","card_type":"creature"},"amount":1}"#, ) .unwrap(); assert_eq!( permanents, Effect::DealDamage { - subject: DamageSubject::Permanents(MassAffects::EachCreature), + subject: DamageSubject::Permanents(PermanentFilter { + scope: ControllerScope::Any, + card_type: vec![CardType::Creature], + ..Default::default() + }), amount: 1, } ); @@ -437,9 +441,15 @@ fn issue_611_deal_damage_round_trips_with_a_class_of_players_or_permanents() { Some(TargetSpec::AnyOpponent) ); assert_eq!( - serde_json::from_str::(r#"{"scope":"creatures_your_opponents_control"}"#) - .unwrap(), - MassAffects::CreaturesYourOpponentsControl + serde_json::from_str::( + r#"{"scope":"opponents_control","card_type":"creature"}"# + ) + .unwrap(), + PermanentFilter { + scope: ControllerScope::OpponentsControl, + card_type: vec![CardType::Creature], + ..Default::default() + } ); } @@ -655,15 +665,14 @@ fn the_new_effect_verbs_round_trip_with_their_target_or_class() { ); // A mass modification names a class, which is not a target (CR 115.1). - let pump = r#"{"kind":"pump_all","affects":{"scope":"creatures_you_control"},"power":2,"toughness":1}"#; + let pump = r#"{"kind":"pump_all","affects":{"card_type":"creature"},"power":2,"toughness":1}"#; let pump: Effect = serde_json::from_str(pump).unwrap(); assert_eq!( pump, Effect::PumpAll { - affects: MassAffects::CreaturesYouControl { - subtype: None, - min_power: None, - below_source_power: false, + affects: PermanentFilter { + card_type: vec![CardType::Creature], + ..Default::default() }, power: 2, toughness: 1, @@ -671,16 +680,15 @@ fn the_new_effect_verbs_round_trip_with_their_target_or_class() { ); assert_eq!(pump.target_spec(2), None); - let grant = r#"{"kind":"grant_keyword_all","affects":{"scope":"creatures_you_control"}, + let grant = r#"{"kind":"grant_keyword_all","affects":{"card_type":"creature"}, "keyword":"trample"}"#; let grant: Effect = serde_json::from_str(grant).unwrap(); assert_eq!( grant, Effect::GrantKeywordAll { - affects: MassAffects::CreaturesYouControl { - subtype: None, - min_power: None, - below_source_power: false, + affects: PermanentFilter { + card_type: vec![CardType::Creature], + ..Default::default() }, keyword: Keyword::Trample, } @@ -1158,23 +1166,35 @@ fn issue_735_cost_modification_round_trips_with_its_selectors() { ObservedSpell::InstantOrSorcery ); - // The mass class's bound is optional and defaults to absent, so every card authored - // before it existed parses unchanged. + // Every field of the shared filter is optional and defaults to absent, so the empty + // object is "every permanent you control" and each field narrows from there. assert_eq!( - serde_json::from_str::(r#"{"scope":"creatures_you_control","min_power":4}"#) + serde_json::from_str::(r#"{"card_type":"creature","min_power":4}"#) .unwrap(), - MassAffects::CreaturesYouControl { - subtype: None, + PermanentFilter { + card_type: vec![CardType::Creature], min_power: Some(4), - below_source_power: false, + ..Default::default() } ); assert_eq!( - serde_json::from_str::(r#"{"scope":"creatures_you_control"}"#).unwrap(), - MassAffects::CreaturesYouControl { - subtype: None, - min_power: None, - below_source_power: false, + serde_json::from_str::(r#"{"card_type":"creature"}"#).unwrap(), + PermanentFilter { + card_type: vec![CardType::Creature], + ..Default::default() + } + ); + // One key, both spellings: a card names one class whether it prints one type or a + // disjunction of two. + assert_eq!( + serde_json::from_str::( + r#"{"scope":"any","card_type":["artifact","enchantment"]}"# + ) + .unwrap(), + PermanentFilter { + scope: ControllerScope::Any, + card_type: vec![CardType::Artifact, CardType::Enchantment], + ..Default::default() } ); } diff --git a/crates/sage-engine/src/apply/cast.rs b/crates/sage-engine/src/apply/cast.rs index 90556245..e418b48b 100644 --- a/crates/sage-engine/src/apply/cast.rs +++ b/crates/sage-engine/src/apply/cast.rs @@ -15,15 +15,15 @@ use super::*; use crate::ability::{ - is_mana_ability, Ability, Cost, DamageSubject, Effect, MassAffects, PermanentAmount, PlayerRef, - Target, + is_mana_ability, Ability, Cost, DamageSubject, Effect, PermanentAmount, PermanentFilter, + PlayerRef, Target, }; use crate::card::abilities_of_permanent; use crate::id::{CardInstance, PermanentId, PlayerId}; use crate::mana::parse_mana_cost; use crate::replacement::PendingDamage; use crate::stack::AbilityOrigin; -use crate::state::{Duration, EffectAffects, Modification, Permanent, StaticEffect}; +use crate::state::{Duration, EffectAffects, Modification, StaticEffect}; mod announce; mod effects; diff --git a/crates/sage-engine/src/apply/cast/effects.rs b/crates/sage-engine/src/apply/cast/effects.rs index 76cfb596..8430036e 100644 --- a/crates/sage-engine/src/apply/cast/effects.rs +++ b/crates/sage-engine/src/apply/cast/effects.rs @@ -440,7 +440,12 @@ pub(crate) fn apply_effect( // trigger sees every one of them. Collected before any of it happens, because // destroying the first member moves the battlefield out from under the scan. Effect::DestroyAll { affects } => { - for id in permanents_to_destroy(state, *affects, db) { + for id in crate::filter::permanents_matching( + state, + db, + affects, + mass_context(controller, permanent_source, resolution), + ) { state.destroy_permanent(id, db); } } @@ -457,9 +462,7 @@ pub(crate) fn apply_effect( apply_mass_modification( state, affects, - controller, - resolution.paid.source_power, - resolution.chosen_player, + mass_context(controller, permanent_source, resolution), Modification::PowerToughness { power: *power, toughness: *toughness, @@ -471,9 +474,7 @@ pub(crate) fn apply_effect( apply_mass_modification( state, affects, - controller, - resolution.paid.source_power, - resolution.chosen_player, + mass_context(controller, permanent_source, resolution), Modification::GrantKeyword(*keyword), db, ); @@ -485,9 +486,7 @@ pub(crate) fn apply_effect( apply_mass_modification( state, affects, - controller, - resolution.paid.source_power, - resolution.chosen_player, + mass_context(controller, permanent_source, resolution), Modification::GrantRestriction(restriction.clone()), db, ); @@ -829,6 +828,26 @@ pub(super) fn win_the_game(state: &mut GameState, winner: PlayerId) { } } +/// What a resolution knows about the object naming a class: who controls it, which +/// permanent it is, the power it had before its cost was paid, and the seat an earlier +/// sentence named. Built in one place so the four facts cannot be passed in a different +/// order at one call site than at another. +fn mass_context( + controller: PlayerId, + source: Option, + resolution: crate::resolve::Resolution, +) -> crate::ability::FilterContext { + crate::ability::FilterContext { + controller, + source, + source_power: resolution.paid.source_power, + chosen_player: resolution.chosen_player, + // A resolution is outside the CR 613 layer system, so every question is answered + // from the permanent's current characteristics. + reading: crate::ability::Reading::Computed, + } +} + /// Add `modification` to every permanent in `affects` until end of turn, on behalf of /// `controller` (CR 611.2c). /// @@ -840,14 +859,12 @@ pub(super) fn win_the_game(state: &mut GameState, winner: PlayerId) { /// duration or timestamp ordering is special-cased for the mass case. fn apply_mass_modification( state: &mut GameState, - affects: &MassAffects, - controller: PlayerId, - source_power: Option, - chosen_player: Option, + affects: &PermanentFilter, + ctx: crate::ability::FilterContext, modification: Modification, db: &CardDatabase, ) { - for id in permanents_in(state, affects, controller, source_power, chosen_player, db) { + for id in crate::filter::permanents_matching(state, db, affects, ctx) { let source = state.mint_id(); state.static_effects.push(StaticEffect { source, @@ -962,125 +979,6 @@ pub(super) fn tokens_created( } } -/// The permanents a [`MassAffects`] class names, in battlefield order, for an object -/// controlled by `controller`. -/// -/// The permanent-side counterpart of [`non_targeting_subjects`], and the one place a class is -/// turned into a concrete set: a mass modification and a mass damage effect must agree -/// on what "each creature" means, and the set is enumerated **at the moment of -/// resolution** (CR 611.2c) so a permanent that arrived after announcement is included -/// and one that has left is not. Every class is read relative to `controller`, which is -/// what lets one authored card mean "you" from either seat. -fn permanents_in( - state: &GameState, - affects: &MassAffects, - controller: PlayerId, - source_power: Option, - chosen_player: Option, - db: &CardDatabase, -) -> Vec { - // CR 613 layer 4 has already run by the time a resolution asks: an artifact animated - // into a creature is in every class of creatures, which is what makes it die to a - // sweeper. Safe to read the computed types here, outside the layer system. - let is_creature = |perm: &Permanent| { - crate::characteristics::characteristics(state, perm.id, db) - .types - .contains(&crate::card_type::CardType::Creature) - }; - // Every class but one is a class of creatures, so the type test is applied once here - // rather than restated in each arm. The exception names planeswalkers outright and - // says so by answering the question itself. - let type_ok = |perm: &Permanent| match affects { - MassAffects::CreaturesAndPlaneswalkersYourOpponentsControl => { - is_creature(perm) - || perm - .printed - .face(db) - .is_some_and(|face| face.has_type(crate::card_type::CardType::Planeswalker)) - } - _ => is_creature(perm), - }; - state - .battlefield - .iter() - .filter(|p| { - type_ok(p) - && match affects { - // A subtype narrows the class to a lord's tribe ("Dragons you - // control"), read off the printed face — the same place every other - // subtype question is answered. - MassAffects::CreaturesYouControl { - subtype, - min_power, - below_source_power, - } => { - crate::characteristics::controller_of(state, p) == controller - && subtype.as_deref().is_none_or(|wanted| { - p.printed - .face(db) - .is_some_and(|face| face.has_subtype(wanted)) - }) - // A power bound is the one field here read through the - // **computed** characteristics (CR 613.1f): "each creature - // you control with power 4 or greater" means the power the - // creature has now, so one pumped up to 4 is in the class - // and one shrunk below it is out. Safe from inside a - // resolution, which is outside the layer system. - && min_power.is_none_or(|min| { - crate::characteristics::characteristics(state, p.id, db) - .power - .is_some_and(|power| power >= min) - }) - // And the same reading against the *source's* power rather - // than a printed number. A source that is gone has no power to - // compare against, and the honest answer to "less than its - // power" is then nobody — never everybody. - && (!below_source_power - || source_power.is_some_and(|source| { - crate::characteristics::characteristics(state, p.id, db) - .power - .is_some_and(|power| power < source) - })) - } - MassAffects::EachCreature => true, - // Exactly the set declare-attackers produced (CR 508.1a); empty - // outside combat, which is what a combat pump cast in a main phase - // means. - MassAffects::AttackingCreatures => p.attacking.is_some(), - // A seat that has lost is no longer an opponent (CR 102.1); its - // permanents are on their way off the battlefield in the same SBA - // loop, and this is the same exclusion `non_targeting_subjects` makes. - // The class counterpart of `PlayerRef::ThatPlayer`, reading the same - // fact: whose creatures were named by the sentence before this one. - MassAffects::CreaturesThatPlayerControls => chosen_player.is_some_and(|seat| { - crate::characteristics::controller_of(state, p) == seat - }), - MassAffects::CreaturesYourOpponentsControl - | MassAffects::CreaturesAndPlaneswalkersYourOpponentsControl => { - let seat = crate::characteristics::controller_of(state, p); - seat != controller - && state - .players - .get(seat.0) - .is_some_and(|player| !player.has_lost) - } - // Flying is read through the computed keywords (CR 613.1f), so a - // creature that was *granted* flying is outside the class exactly - // as a printed flyer is. - MassAffects::CreaturesWithoutFlying => { - !crate::characteristics::permanent_has_keyword( - state, - p.id, - crate::card::Keyword::Flying, - db, - ) - } - } - }) - .map(|p| p.id) - .collect() -} - /// The seats a **non-targeting** [`PlayerRef`] names, in seat order, for an object /// controlled by `controller` (CR 115.1 — no target is chosen, so this list is /// derived fresh at resolution and never fizzles). @@ -1156,13 +1054,11 @@ fn apply_class_damage( } } DamageSubject::Permanents(affects) => { - for id in permanents_in( + for id in crate::filter::permanents_matching( state, - affects, - controller, - resolution.paid.source_power, - resolution.chosen_player, db, + affects, + mass_context(controller, source, resolution), ) { dealt |= state.deal_damage( resolution.damage(PendingDamage::to_permanent(id, amount).from(source)), @@ -1179,31 +1075,3 @@ fn apply_class_damage( } } } - -/// The permanents a [`DestroyAffects`] class names, in battlefield order. -/// -/// The mass-destruction counterpart of [`permanents_in`], and separate from it for the -/// reason [`DestroyAffects`] is separate from [`MassAffects`]: nothing here is -/// controller-relative and nothing here is limited to creatures, so sharing the scan -/// would mean a filter with two halves that never both apply. -fn permanents_to_destroy( - state: &GameState, - affects: crate::ability::DestroyAffects, - db: &CardDatabase, -) -> Vec { - use crate::ability::DestroyAffects; - use crate::card_type::CardType; - state - .battlefield - .iter() - .filter(|p| { - p.printed.face(db).is_some_and(|face| match affects { - DestroyAffects::EachCreature => face.has_type(CardType::Creature), - DestroyAffects::EachArtifactOrEnchantment => { - face.has_type(CardType::Artifact) || face.has_type(CardType::Enchantment) - } - }) - }) - .map(|p| p.id) - .collect() -} diff --git a/crates/sage-engine/src/apply/cast/effects/tests.rs b/crates/sage-engine/src/apply/cast/effects/tests.rs index a4cd6333..faeb281c 100644 --- a/crates/sage-engine/src/apply/cast/effects/tests.rs +++ b/crates/sage-engine/src/apply/cast/effects/tests.rs @@ -3,7 +3,9 @@ #![allow(clippy::unwrap_used)] use super::*; +use crate::ability::ControllerScope; use crate::apply::test_support::*; +use crate::card_type::CardType; #[test] fn issue_card_effects_etb_draw_end_to_end() { @@ -185,7 +187,11 @@ fn issue_611_class_damage_to_creatures_is_marked_and_drives_the_lethal_sba() { let wurm = place_permanent(&mut state, fixture("pelakka_wurm"), PlayerId(0), false, 0); push_class_damage( &mut state, - DamageSubject::Permanents(MassAffects::EachCreature), + DamageSubject::Permanents(PermanentFilter { + scope: ControllerScope::Any, + card_type: vec![CardType::Creature], + ..Default::default() + }), 2, ); @@ -217,7 +223,11 @@ fn issue_611_a_one_sided_sweeper_spares_the_creatures_you_control() { let yours = place_permanent(&mut state, fixture("pelakka_wurm"), PlayerId(0), false, 0); push_class_damage( &mut state, - DamageSubject::Permanents(MassAffects::CreaturesYourOpponentsControl), + DamageSubject::Permanents(PermanentFilter { + scope: ControllerScope::OpponentsControl, + card_type: vec![CardType::Creature], + ..Default::default() + }), 3, ); @@ -237,7 +247,11 @@ fn issue_611_the_class_is_enumerated_on_resolution_not_on_announcement() { let mut state = main_phase_p0(); push_class_damage( &mut state, - DamageSubject::Permanents(MassAffects::EachCreature), + DamageSubject::Permanents(PermanentFilter { + scope: ControllerScope::Any, + card_type: vec![CardType::Creature], + ..Default::default() + }), 1, ); let late = place_permanent(&mut state, fixture("pelakka_wurm"), PlayerId(1), false, 0); @@ -266,7 +280,11 @@ fn issue_611_the_targeted_damage_form_is_unchanged() { for subject in [ DamageSubject::Players(PlayerRef::EachOpponent), DamageSubject::Players(PlayerRef::Controller), - DamageSubject::Permanents(MassAffects::EachCreature), + DamageSubject::Permanents(PermanentFilter { + scope: ControllerScope::Any, + card_type: vec![CardType::Creature], + ..Default::default() + }), ] { assert_eq!( Effect::DealDamage { subject, amount: 2 }.target_spec(2), diff --git a/crates/sage-engine/src/filter.rs b/crates/sage-engine/src/filter.rs new file mode 100644 index 00000000..0091469b --- /dev/null +++ b/crates/sage-engine/src/filter.rs @@ -0,0 +1,202 @@ +//! The one predicate that decides whether a permanent is in a class (issue #824). +//! +//! Four selectors used to answer this question and each had its own matcher, so "creatures +//! you control" could mean one set when a spell pumped them, another when a static ability +//! did, and a third when something counted them. There is one body now, and every caller +//! reaches it: a mass effect, a sweeper, a static ability's own selector, and a count. +//! +//! What still differs between callers is [`Reading`], and that difference is a rules fact +//! rather than a leftover — see its documentation. + +use crate::ability::{ControllerScope, FilterContext, PermanentFilter, Reading}; +use crate::card::CardDatabase; +use crate::id::PermanentId; +use crate::state::{GameState, Permanent}; + +/// Whether `perm` is in the class `filter` names, for an object described by `ctx`. +/// +/// Every field of the filter is conjunctive and an absent field asks nothing, so the +/// empty filter is "every permanent the scope allows". The order below is cheapest-first +/// only where that changes nothing: the computed reads are last because they are the ones +/// that walk the layer system. +#[must_use] +pub(crate) fn permanent_matches( + state: &GameState, + db: &CardDatabase, + filter: &PermanentFilter, + perm: &Permanent, + ctx: FilterContext, +) -> bool { + // CR 613 layer 2, read through the one control path — applied before every layer this + // predicate reads, and itself unable to recurse, so it is answered the same way from + // inside the layer walk as from outside it. An anthem stops pumping a creature the + // moment someone else gains control of it, and starts pumping one it just stole. + let seat = crate::characteristics::controller_of(state, perm); + let scope_ok = match filter.scope { + ControllerScope::YouControl => seat == ctx.controller, + // A seat that has lost is no longer an opponent (CR 102.1); its permanents are on + // their way off the battlefield in the same state-based-action pass. + ControllerScope::OpponentsControl => { + seat != ctx.controller + && state + .players + .get(seat.0) + .is_some_and(|player| !player.has_lost) + } + ControllerScope::ThatPlayer => ctx.chosen_player == Some(seat), + ControllerScope::Any => true, + }; + if !scope_ok { + return false; + } + + // "Other …". An object with no source permanent — a spell's own effects, an emblem — + // excludes nothing, which is the honest reading of a sentence that has no "this". + if filter.except_this && ctx.source == Some(perm.id) { + return false; + } + // CR 111: a token is not a card. Read off what the permanent is, never inferred from a + // missing card handle. + if let Some(wanted) = filter.token { + if perm.printed.is_token() != wanted { + return false; + } + } + if filter.attacking && perm.attacking.is_none() { + return false; + } + // A counter is not a characteristic and no layer produces one, so this is the one + // narrowing that reads the same under both readings and can be asked from anywhere. + if let Some(kind) = filter.with_counter { + if perm.counter_count(kind) == 0 { + return false; + } + } + if filter.with_the_named_card && !bears_the_named_card(state, ctx.source, perm) { + return false; + } + + let Some(face) = perm.printed.face(db) else { + return false; + }; + // Subtype and colour are printed under both readings. Subtype because CR 613 layer 4 + // is only modelled as *adding* types to one named permanent and a class read is not + // where that arrives; colour because layer 5 is not modelled on the battlefield at + // all, so printed colour is current colour here exactly as it is in the blocking + // restrictions that name one. + if let Some(wanted) = &filter.subtype { + if !face.has_subtype(wanted) { + return false; + } + } + if let Some(wanted) = filter.color { + if !face.colors().contains(&wanted) { + return false; + } + } + + match ctx.reading { + // Outside the layer system: every remaining question is answered from the + // permanent's current characteristics. An artifact animated into a creature is in + // every class of creatures, which is what makes it die to a sweeper, and a + // creature *granted* defender is in the class that names one. + Reading::Computed => { + let current = crate::characteristics::characteristics(state, perm.id, db); + if !filter.card_type.is_empty() + && !filter + .card_type + .iter() + .any(|wanted| current.types.contains(wanted)) + { + return false; + } + if let Some(wanted) = filter.keyword { + if !current.keywords.contains(&wanted) { + return false; + } + } + if let Some(wanted) = filter.without_keyword { + if current.keywords.contains(&wanted) { + return false; + } + } + if let Some(min) = filter.min_power { + if !current.power.is_some_and(|power| power >= min) { + return false; + } + } + if let Some(max) = filter.max_toughness { + if !current.toughness.is_some_and(|toughness| toughness <= max) { + return false; + } + } + // "Less than its power", against a source that may already be gone: the + // caller read the number before the cost was paid, and no number means an + // empty class rather than a universal one. + if filter.below_source_power { + let below = ctx + .source_power + .is_some_and(|source| current.power.is_some_and(|power| power < source)); + if !below { + return false; + } + } + true + } + // Inside the layer walk. Types and keywords come off the printed face, because the + // computed sets are what the walk is producing. The power and toughness bounds + // cannot be answered at all here, and the catalog validator refuses them rather + // than letting this arm quietly say "no" to a card that looked authored. + Reading::Printed => { + if !filter.card_type.is_empty() + && !filter.card_type.iter().any(|&wanted| face.has_type(wanted)) + { + return false; + } + if let Some(wanted) = filter.keyword { + if !face.keywords().contains(&wanted) { + return false; + } + } + if let Some(wanted) = filter.without_keyword { + if face.keywords().contains(&wanted) { + return false; + } + } + true + } + } +} + +/// Every permanent in the class, in battlefield order. +/// +/// The set is whatever the caller's moment says it is: a resolution enumerates once +/// (CR 611.2c) and keeps what it found, a static ability re-derives on every read. This +/// function does not know the difference and does not need to — it is the *caller* that +/// calls it once or calls it again. +#[must_use] +pub(crate) fn permanents_matching( + state: &GameState, + db: &CardDatabase, + filter: &PermanentFilter, + ctx: FilterContext, +) -> Vec { + state + .battlefield + .iter() + .filter(|perm| permanent_matches(state, db, filter, perm, ctx)) + .map(|perm| perm.id) + .collect() +} + +/// Whether `perm`'s printed card is the one `source` named as it entered (CR 614.12). +/// +/// Card identity, not a string: two printings of one functional card share a `CardId` and +/// nothing else does. A source that named nothing, or that is not a permanent at all, +/// matches nothing — there is no chosen name, so no permanent has it. +fn bears_the_named_card(state: &GameState, source: Option, perm: &Permanent) -> bool { + let named = source + .and_then(|id| state.battlefield.iter().find(|p| p.id == id)) + .and_then(|source| source.named_card); + named.is_some() && named == perm.printed.card() +} diff --git a/crates/sage-engine/src/lib.rs b/crates/sage-engine/src/lib.rs index 5adf91d7..6fc8d15d 100644 --- a/crates/sage-engine/src/lib.rs +++ b/crates/sage-engine/src/lib.rs @@ -28,6 +28,7 @@ mod copy; mod cost_modification; pub mod coverage; mod delayed; +mod filter; #[cfg(test)] mod fixtures; mod id; @@ -52,13 +53,13 @@ pub use ability::{ activation_condition_holds, activation_taps, group_target_counts, is_emblem_ability, is_equip_ability, is_graveyard_ability, is_hand_ability, is_loyalty_ability, is_mana_ability, is_sorcery_speed_ability, maximum_targets, minimum_targets, target_counts, Ability, - ActivationTiming, ActivatorScope, BottomOrder, CardFilter, Chooser, Condition, Cost, - CostModification, CountScope, DamageSubject, DerivedAmount, DestroyAffects, Effect, + ActivationTiming, ActivatorScope, BottomOrder, CardFilter, Chooser, Condition, ControllerScope, + Cost, CostModification, CountScope, DamageSubject, DerivedAmount, Effect, FilterContext, FoundDestination, GraveyardCardClass, GraveyardCount, GraveyardScope, HalvedTotal, - ManaRestriction, MassAffects, ObservedActivation, ObservedPermanent, ObservedSpell, - OptionalCost, PermanentAmount, PermanentCount, PlayerModification, PlayerRef, SacrificeCount, - StaticAffects, StaticCondition, StaticModification, Target, TargetCount, TargetGroup, - TargetSpec, TriggerCondition, TriggerStep, TurnScope, + ManaRestriction, ObservedActivation, ObservedPermanent, ObservedSpell, OptionalCost, + PermanentAmount, PermanentCount, PermanentFilter, PlayerModification, PlayerRef, Reading, + SacrificeCount, StaticAffects, StaticCondition, StaticModification, Target, TargetCount, + TargetGroup, TargetSpec, TriggerCondition, TriggerStep, TurnScope, }; pub use actions::{ activation_discard_cost, activation_exile_cost, activation_sacrifice_cost, diff --git a/crates/sage-engine/src/resolve.rs b/crates/sage-engine/src/resolve.rs index 134602bc..0a21cf51 100644 --- a/crates/sage-engine/src/resolve.rs +++ b/crates/sage-engine/src/resolve.rs @@ -90,7 +90,7 @@ pub struct Resolution { /// The player this resolution's most recent **targeted** effect named — a player it /// aimed at, or the controller of a permanent it aimed at — read by /// [`PlayerRef::ThatPlayer`](crate::PlayerRef) and - /// [`MassAffects::CreaturesThatPlayerControls`](crate::MassAffects). + /// [`ControllerScope::ThatPlayer`](crate::ControllerScope). /// /// The `its controller` of `Destroy target creature. **Its controller** creates a 2/4 /// white Ox`, and the `that player` of `deals 3 damage to target player and 1 damage diff --git a/crates/sage-server/src/rules_text.rs b/crates/sage-server/src/rules_text.rs index 33aaa4a0..10998405 100644 --- a/crates/sage-server/src/rules_text.rs +++ b/crates/sage-server/src/rules_text.rs @@ -24,14 +24,14 @@ use sage_engine::{ equip_ability, Ability, ActivationTiming, ActivatorScope, AdditionalCost, Attachment, AttachmentKind, BackFace, BottomOrder, CardData, CardFilter, CardType, Chooser, Color, - CombatRestriction, Condition, CopyClass, CopySubject, Cost, CostModification, CountScope, - CounterKind, DamageCharacteristic, DamageSubject, DelayedCondition, DerivedAmount, - DestroyAffects, Effect, EnteringFilter, FoundDestination, GraveyardCardClass, GraveyardCount, - GraveyardScope, HalvedTotal, Keyword, ManaRestriction, MassAffects, NamedCardClass, - ObservedPermanent, ObservedSpell, OptionalCost, PermanentAmount, PermanentCount, - PlayerModification, PlayerRef, ReplacementEffect, SacrificeCount, SpellMode, SpellTrait, - StaticAffects, StaticCondition, StaticModification, TargetCount, TargetSpec, TokenData, - TriggerCondition, TriggerStep, TurnScope, + CombatRestriction, Condition, ControllerScope, CopyClass, CopySubject, Cost, CostModification, + CountScope, CounterKind, DamageCharacteristic, DamageSubject, DelayedCondition, DerivedAmount, + Effect, EnteringFilter, FoundDestination, GraveyardCardClass, GraveyardCount, GraveyardScope, + HalvedTotal, Keyword, ManaRestriction, NamedCardClass, ObservedPermanent, ObservedSpell, + OptionalCost, PermanentAmount, PermanentCount, PermanentFilter, PlayerModification, PlayerRef, + ReplacementEffect, SacrificeCount, SpellMode, SpellTrait, StaticAffects, StaticCondition, + StaticModification, TargetCount, TargetSpec, TokenData, TriggerCondition, TriggerStep, + TurnScope, }; mod effects; diff --git a/crates/sage-server/src/rules_text/effects.rs b/crates/sage-server/src/rules_text/effects.rs index aba66980..a0b001db 100644 --- a/crates/sage-server/src/rules_text/effects.rs +++ b/crates/sage-server/src/rules_text/effects.rs @@ -69,7 +69,9 @@ pub(super) fn effect_clause(source: &str, effect: &Effect) -> String { // the creature it is already on, and the card prints a pronoun rather than a // second choice. Effect::DestroyAttached => "destroy it".to_string(), - Effect::DestroyAll { affects } => format!("destroy all {}", destroy_class(*affects)), + Effect::DestroyAll { affects } => { + format!("destroy all {}", mass_subject(source, affects)) + } // The derived-amount damage verb, in the two shapes English gives it. An // announced X reads the way a printed card writes it: the letter itself, in // quantity position, not the number it turned out to be — what a *particular* @@ -1251,14 +1253,6 @@ fn amount_noun(amount: &DerivedAmount, subject: PlayerRef) -> String { } } -/// The class a mass destruction names, as the plural noun after "destroy all". -fn destroy_class(affects: DestroyAffects) -> &'static str { - match affects { - DestroyAffects::EachCreature => "creatures", - DestroyAffects::EachArtifactOrEnchantment => "artifacts and enchantments", - } -} - /// The subject of a relative clause hanging off a noun — "the creatures **they** /// control". The third of the player-reference renderings beside [`subject_pronoun`] and /// [`possessive_pronoun`], and separate for the same reason those two are: English wants @@ -1544,100 +1538,112 @@ fn possessive_subject(player_ref: PlayerRef) -> &'static str { } } -/// The class a mass, non-targeting effect names, as the subject of its sentence. A -/// subtype replaces the noun outright — "Dragons you control", never "Dragon creatures -/// you control", which is not how a card is written. -fn mass_subject(source: &str, affects: &MassAffects) -> String { - match affects { - MassAffects::CreaturesYouControl { - subtype, - min_power, - below_source_power, - } => { - let noun = match subtype { - Some(subtype) => format!("{subtype}s"), - None => "creatures".to_string(), - }; - format!( - "{noun} you control{}{}", - mass_power_clause(*min_power), - relative_power_clause(source, *below_source_power) - ) - } - MassAffects::EachCreature => "creatures".to_string(), - MassAffects::CreaturesYourOpponentsControl => { - "creatures your opponents control".to_string() - } - MassAffects::CreaturesThatPlayerControls => "creatures that player controls".to_string(), - MassAffects::CreaturesAndPlaneswalkersYourOpponentsControl => { - "creatures and planeswalkers your opponents control".to_string() +/// The class a [`PermanentFilter`] names, as a noun phrase. +/// +/// One generator for a vocabulary that used to be four enums of named classes (issue +/// #824), and the reason it is one: a class is a product of axes, and English composes +/// those axes in a fixed order — `other attacking blue Dragons you control with power 4 or +/// greater`. A phrase per pairing would have to be written once per card. +/// +/// `plural` picks the position. A class is a bare plural when it *acts* ("creatures you +/// control get +2/+1") and a distributive singular when it is *acted on* ("deals 2 damage +/// to each creature you control"), which is a fact about English rather than about the +/// class, so both come out of one body. +/// +/// It does **not** try to reproduce printed wording. A card that prints "each creature and +/// planeswalker they control" after naming its opponents gets "…your opponents control" +/// here: the pronoun is a discourse feature of the sentence before it, not a property of +/// the class, and this formatter states behaviour rather than reproducing Oracle text +/// (`AGENTS.md`, Legal Considerations). +fn class_noun(source: &str, filter: &PermanentFilter, plural: bool) -> String { + let mut words: Vec = Vec::new(); + // "Other …" leads, because that is where a card puts it. + if filter.except_this { + words.push("other".to_string()); + } + if filter.attacking { + words.push("attacking".to_string()); + } + if let Some(color) = filter.color { + words.push(color.word().to_string()); + } + // A subtype **replaces** the type noun — "Dragons you control", never "Dragon + // creatures you control", which is not how a card is written. + let mut nouns: Vec = match (&filter.subtype, filter.card_type.as_slice()) { + (Some(subtype), _) => vec![subtype.clone()], + (None, []) => vec!["permanent".to_string()], + (None, types) => types + .iter() + .map(|&kind| card_type_word(kind).to_string()) + .collect(), + }; + // "Nontoken" is an adjective on the class; "token" is a noun the class qualifies — + // "creature tokens" — so the two halves of one field land in different places. + match filter.token { + Some(false) => words.push("nontoken".to_string()), + Some(true) => nouns = vec![format!("{} token", list_words_owned(&nouns))], + None => {} + } + // Every noun in a disjunction agrees with the number, not just the last: "artifacts + // and enchantments", never "artifact and enchantments". + if plural { + for noun in &mut nouns { + noun.push('s'); } - MassAffects::CreaturesWithoutFlying => "creatures without flying".to_string(), - MassAffects::AttackingCreatures => "attacking creatures".to_string(), } + words.push(list_words_owned(&nouns)); + let mut phrase = words.join(" "); + phrase.push_str(match filter.scope { + ControllerScope::YouControl => " you control", + ControllerScope::OpponentsControl => " your opponents control", + ControllerScope::ThatPlayer => " that player controls", + ControllerScope::Any => "", + }); + // The qualifiers a card prints after the class, in the order it prints them. + if let Some(keyword) = filter.keyword { + phrase.push_str(&format!(" with {}", keyword_word(keyword))); + } + if let Some(keyword) = filter.without_keyword { + phrase.push_str(&format!(" without {}", keyword_word(keyword))); + } + if let Some(min) = filter.min_power { + phrase.push_str(&format!(" with power {min} or greater")); + } + if let Some(max) = filter.max_toughness { + phrase.push_str(&format!(" with toughness {max} or less")); + } + // The source is named, because a card naming itself in its own text uses its name + // (CR 201.4) — and the reader needs to know which creature the comparison is against. + if filter.below_source_power { + phrase.push_str(&format!(" with power less than {source}'s power")); + } + if let Some(counter) = filter.with_counter { + phrase.push_str(&format!( + " with a {} counter on {}", + crate::rules_text::counter_symbol(counter), + if plural { "them" } else { "it" } + )); + } + if filter.with_the_named_card { + phrase.push_str(" with the chosen name"); + } + phrase } -/// The same class as the **object** of a sentence — what damage is dealt *to*. -/// -/// Separate from [`mass_subject`] because English is: a class is a bare plural when it -/// acts ("creatures you control get +2/+1") and a distributive "each" when it is acted -/// on ("deals 2 damage to each creature you control"). One function per position keeps -/// both exhaustive, so a new [`MassAffects`] variant must be given words for each. -fn mass_recipient(source: &str, affects: &MassAffects) -> String { - match affects { - MassAffects::CreaturesYouControl { - subtype, - min_power, - below_source_power, - } => { - let noun = match subtype { - Some(subtype) => subtype.clone(), - None => "creature".to_string(), - }; - format!( - "each {noun} you control{}{}", - mass_power_clause(*min_power), - relative_power_clause(source, *below_source_power) - ) - } - MassAffects::EachCreature => "each creature".to_string(), - MassAffects::CreaturesYourOpponentsControl => { - "each creature your opponents control".to_string() - } - MassAffects::CreaturesThatPlayerControls => { - "each creature that player controls".to_string() - } - // The card prints this one as a single breath about the seat and its board — - // "each opponent and each creature and planeswalker they control" — so the - // possessive is *they*, not *your opponents*, wherever it follows that seat. - MassAffects::CreaturesAndPlaneswalkersYourOpponentsControl => { - "each creature and planeswalker they control".to_string() - } - MassAffects::CreaturesWithoutFlying => "each creature without flying".to_string(), - MassAffects::AttackingCreatures => "each attacking creature".to_string(), - } +/// [`list_words`] over owned strings — the class nouns are built rather than borrowed. +fn list_words_owned(words: &[String]) -> String { + list_words(&words.iter().map(String::as_str).collect::>()) } -/// The " with power less than Lena's power" that trails a mass class when the bound is the -/// source's own power rather than a printed number. The source is named, because a card -/// naming itself in its own text uses its name (CR 201.4) — and the reader needs to know -/// which creature the comparison is against. -fn relative_power_clause(source: &str, below_source_power: bool) -> String { - if below_source_power { - format!(" with power less than {source}'s power") - } else { - String::new() - } +/// The class as the **subject** of its sentence — a bare plural. +fn mass_subject(source: &str, affects: &PermanentFilter) -> String { + class_noun(source, affects, true) } -/// The " with power 4 or greater" that trails a mass class, where a card prints it, or -/// nothing when the class names no bound. Written once so the subject and the recipient -/// phrasings cannot drift. -fn mass_power_clause(min_power: Option) -> String { - match min_power { - None => String::new(), - Some(min) => format!(" with power {min} or greater"), - } +/// The same class as the **object** of a sentence — what damage is dealt *to*, or what a +/// sweeper destroys. Distributive: "each creature you control". +fn mass_recipient(source: &str, affects: &PermanentFilter) -> String { + format!("each {}", class_noun(source, affects, false)) } /// Who or what damage is dealt to (CR 120.3), as a noun phrase. diff --git a/crates/sage-server/src/rules_text/tests.rs b/crates/sage-server/src/rules_text/tests.rs index af109d7d..919568c2 100644 --- a/crates/sage-server/src/rules_text/tests.rs +++ b/crates/sage-server/src/rules_text/tests.rs @@ -270,16 +270,16 @@ fn issue_611_damage_dealt_to_a_class_reads_as_a_sentence() { r#"[ {"schema_version":1,"functional_id":"test_pyroclasm","name":"Test Pyroclasm", "types":["sorcery"],"mana_cost":"{1}{R}","colors":["red"], - "spell_effects":[{"kind":"deal_damage","affects":{"scope":"each_creature"},"amount":2}]}, + "spell_effects":[{"kind":"deal_damage","affects":{"scope":"any","card_type":"creature"},"amount":2}]}, {"schema_version":1,"functional_id":"test_slagstorm","name":"Test Slagstorm", "types":["sorcery"],"mana_cost":"{2}{R}","colors":["red"], - "spell_effects":[{"kind":"deal_damage","affects":{"scope":"creatures_your_opponents_control"},"amount":3}]}, + "spell_effects":[{"kind":"deal_damage","affects":{"scope":"opponents_control","card_type":"creature"},"amount":3}]}, {"schema_version":1,"functional_id":"test_recoil","name":"Test Recoil", "types":["sorcery"],"mana_cost":"{R}","colors":["red"], "spell_effects":[{"kind":"deal_damage","player_ref":"controller","amount":1}]}, {"schema_version":1,"functional_id":"test_rally","name":"Test Rally", "types":["sorcery"],"mana_cost":"{1}{G}","colors":["green"], - "spell_effects":[{"kind":"pump_all","affects":{"scope":"each_creature"},"power":1,"toughness":1}]} + "spell_effects":[{"kind":"pump_all","affects":{"scope":"any","card_type":"creature"},"power":1,"toughness":1}]} ]"#, ) .unwrap();