refactor(engine): one permanent filter for the one-shot classes - #834
Draft
ninthworld wants to merge 1 commit into
Draft
ninthworld wants to merge 1 commit into
ninthworld wants to merge 1 commit into
Conversation
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft — this is the first half of #824, not the whole issue. Opening it now because it is coherent, green, and reviewable on its own; see What is left below for what #824 still needs before it can close.
Part of #819.
What landed
MassAffectsandDestroyAffectsare gone. A sharedPermanentFilterand one predicate (src/filter.rs) 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 nearly all were a product of axes the engine already modelled: controller relation, card type, subtype, colour, a keyword, a power bound, attacking, token-ness.
PermanentCountwas already that product, and its own doc said why:This is that struct, generalised until the other selectors fit in it. The catalog reads better for it:
DestroyAffectsexisted only becauseMassAffectswas creature-only — its own doc said so — and it disappears entirely: a sweeper's class is a filter like any other.Reading— the one thing that really does differA class named by a resolution is asked from outside the CR 613 layer system, so every question is answered from the computed characteristics. A class named by a static ability's own selector is asked from inside the walk that produces them, so it must read the printed face or it would ask the computation for the answer it is producing.
That was stated in prose on two different selectors. It is a type now, named once, and the predicate branches on it in one place.
Rules text
The noun phrase is generated from the filter in both positions (subject and distributive recipient) instead of held as a phrase per named class — "other attacking blue Dragons you control with power 4 or greater" composes rather than being written out.
One generated text changes. A card printing "each creature and planeswalker they control" after naming its opponents now reads "…your opponents control". The pronoun is a discourse feature of the preceding sentence, not a property of the class, and two M19 cards in the same shape print it differently (Sarkhan, Dragonsoul says "your opponents control"; Sarkhan's Unsealing says "they control"), so it isn't derivable from the class at all. This formatter states behaviour rather than reproducing Oracle text (
AGENTS.md, Legal Considerations), so the explicit form is the right default. Flagging it because it is user-visible and I could not verify it reads well in play.What is left on #824
Named here rather than left implicit — this PR does not meet the issue's acceptance criteria on its own:
StaticAffects→{ Source, AttachedTo, Class(PermanentFilter) }. The open design question is the wire format:SourceandAttachedToare classes-of-one rather than filters, and they currently share thescopekey with the filter's own controller relation.PermanentCountreconciled with the filter (distinct_namesis a counting concept and stays on the count).Reading::Printed—min_power/max_toughness/below_source_powercannot be answered from inside the layer walk.PermanentFilter::reads_computed_power()exists for this and has no caller yet.exclusions.jsonentries those unlock: static abilities that affect a class of the controller's own noncreature permanents, and selectors that filter by toughness. The filter can already express both; nothing authors them untilStaticAffectsmigrates.docs/card-schema.md.Verification
Behaviour-preserving: the existing
m19_*suites pass over the re-authored catalog unchanged in intent.make verifygreen —e2e-smokeandcargo denyincluded.make compatshows no drift.The catalog re-authoring was the risky half, not the Rust: a filter with a field forgotten silently widens a class. It was done by script from an explicit old→new map, and the 26 rewritten classes are visible in the diff.
🤖 Generated with Claude Code