feat(engine): the self row of a keyword grant, and one target for one sentence - #831
Merged
Merged
Conversation
… sentence Two M19 cards the ability IR could not say (#821). Resplendent Angel's `{3}{W}{W}{W}: Until end of turn, this creature gets +2/+2 and gains lifelink` was missing from the catalog entirely, and the shape of the vocabulary is why. The subject × payload matrix has a hole on the self row: the only self-directed keyword *addition* was `alter_abilities_self`'s `gain`, whose sibling field is `lose_all`. Reaching for the lose-all-abilities verb to say "gains lifelink" reads as a card doing something it does not do, so `pump_self` carries its own keywords now — one printed sentence, one effect, one CR 613.7 timestamp, and the printed flying untouched. Skyrider Patrol prints one target and asked for two. `put a +1/+1 counter on another target creature you control, **and that creature** gains flying` was authored as a counter beside a standalone grant, and `Effect::target_groups` is a `flat_map` with no dedup — so the ability advertised two slots and a player could counter one creature while a different one flew. `put_counters` carries `keywords` now, for the reason `pump` already did. The counter stays and the keyword is gone at cleanup: one sentence, two durations, one target. The test that covered the Patrol had been filling both slots with the same creature, which is how it stayed green while the card was wrong. It now places a second eligible creature, asserts a two-target answer is refused, and asserts the bystander took nothing. A validator refusing two sibling effects of one class was written and then backed out: two same-class slots are a shape the engine handles correctly and `view::stack`'s two-target projection test depends on. Whether a card names one target or two is knowable only from the printed card, so it stays a behavioural claim rather than a build-time one. Making it structural is #827's job, where the subject becomes one field. Closes #821. Part of #819. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6 tasks
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.
Closes #821. Part of #819. Independent of #829/#830 — branches off
main.The two M19 cards the IR could not say. Both are the same hole seen from two sides: one printed sentence has to be one effect.
Resplendent Angel — an ability missing from the catalog entirely
Printed:
{3}{W}{W}{W}: Until end of turn, this creature gets +2/+2 and gains lifelink.There was no such ability in the definition.The subject × payload matrix has a hole on the self row:
PumpSelfRestrictSelfPumpPump.keywords/GrantKeywordPump.restrictions/RestrictPumpAllGrantKeywordAllRestrictAllThe only self-directed keyword addition was
AlterAbilitiesSelf.gain, whose sibling field islose_all— reaching for the lose-all-abilities verb to say "gains lifelink".PumpSelfcarrieskeywordsnow, so the pump and the grant share one subject, one duration, and one CR 613.7 timestamp. Tests assert the printed flying survives it: this grants, it does not replace.Skyrider Patrol — one printed target, two slots
Printed:
put a +1/+1 counter on another target creature you control, **and that creature** gains flying until end of turn.Authored as
put_countersbeside a standalonegrant_keyword, both naminganother_creature_you_control.Effect::target_groupsis aflat_mapover the effect list with no dedup, so the ability advertised two independent slots — counter one creature, fly a different one.PutCounterscarrieskeywordsnow, for the reasonPumpalready did. The counter stays on the permanent and the keyword is gone at cleanup: one sentence, two durations, one target.The existing test was filling both slots with the same creature, which is how it stayed green while the card was wrong. It now places a second eligible creature, asserts a two-target answer is refused, and asserts the bystander took no counter and no flying.
Rules text
Both new shapes render, and the counter case points back with a pronoun rather than repeating the class —
…on another target creature you control, and that creature gains flying until end of turn. Writing the noun twice is what reads as a second target.One acceptance criterion moved, and why
#821 asked for the double-slot shape to be refused "by the type or by a validator with a named
Violation, not by review."I implemented that —
Violation::TwoTargetsOfOneClass— and it failed the build oncrates/sage-server/src/view/stack.rs'stest_twin_boltfixture: a two-target damage spell that exists specifically to prove the projection pairs two same-class slots positionally, in order. That shape is supported and correct.So the rule is wrong as a build-time check. Whether a card names one target or two is knowable only from the printed card, and two same-class slots are legitimately representable. I backed the validator out rather than ship one that refuses a legal shape, and the guarantee is a behavioural claim instead: the Skyrider test above fails if the slot count regresses.
This becomes structural in #827, where
Subjectis a single field on one effect and the two-slot shape stops being representable for a card that declares one. I've noted this on #821.Docs
docs/card-schema.md— the "Granting keywords" section gains the self row and the counter's target, beside the target row it already had, with thealter_abilities_selfdistinction stated.make verifygreen —e2e-smokeandcargo denyincluded.make compatshows no drift.🤖 Generated with Claude Code