feat(engine): the self row of a keyword grant, and one target for one sentence

crates/sage-engine/data/catalog/resplendent_angel.json

@@ -58,6 +58,25 @@
           ]
         }
       ]
+    },
+    {
+      "type": "activated",
+      "cost": [
+        {
+          "kind": "mana",
+          "mana": "{3}{W}{W}{W}"
+        }
+      ],
+      "effects": [
+        {
+          "kind": "pump_self",
+          "power": 2,
+          "toughness": 2,
+          "keywords": [
+            "lifelink"
+          ]
+        }
+      ]
     }
   ]
 }

crates/sage-engine/data/catalog/skyrider_patrol.json

@@ -40,12 +40,10 @@
               "kind": "put_counters",
               "target": "another_creature_you_control",
               "counter": "plus_one_plus_one",
-              "count": 1
-            },
-            {
-              "kind": "grant_keyword",
-              "target": "another_creature_you_control",
-              "keyword": "flying"
+              "count": 1,
+              "keywords": [
+                "flying"
+              ]
             }
           ]
         }

crates/sage-engine/src/ability/effect.rs

@@ -307,6 +307,25 @@ pub enum Effect {
         /// card that names two creatures gives each of them the same X.
         #[serde(default)]
         count_amount: Option<DerivedAmount>,
+        /// Keyword abilities granted to the **same** target(s) until end of turn, applied
+        /// at CR 613 layer 6 exactly as [`Effect::Pump`]'s are. Empty for the ordinary
+        /// counter placement, which is nearly every card.
+        ///
+        /// Here for the reason they are on [`Effect::Pump`], and it is the reason rather
+        /// than an analogy: one effect declares one target group, so a card printed as
+        /// `put a +1/+1 counter on another target creature you control, **and that
+        /// creature** gains flying until end of turn` has to be one effect. Authored as a
+        /// counter beside a standalone [`Effect::GrantKeyword`] it would advertise two
+        /// independent slots and let a player counter one creature while a different one
+        /// gained flying — which is exactly what Skyrider Patrol did before this field
+        /// existed (issue #821). The catalog validator now refuses that shape outright
+        /// ([`Violation::TwoTargetsOfOneClass`](crate::Violation)).
+        ///
+        /// The counter and the grant have different lifetimes and are meant to: a counter
+        /// stays on the permanent, and the keyword is gone at cleanup (CR 514.2). One
+        /// printed sentence, two durations, one target.
+        #[serde(default)]
+        keywords: Vec<Keyword>,
     },
     /// Give the single creature this effect targets `+power`/`+toughness`
     /// **until end of turn** — the pump-spell verb (e.g. `Target creature gets
@@ -525,11 +544,29 @@ pub enum Effect {
     /// not a *target* (CR 115.1), so this chooses nothing, fills no slot, and can
     /// never fizzle. A source that has left the battlefield by the time the ability
     /// resolves is simply not there to modify, and the effect does nothing.
+    ///
+    /// The optional `keywords` are granted to that same source, at CR 613 layer 6 and for
+    /// the same until-end-of-turn duration — the self row of what [`Effect::Pump`] carries
+    /// for a target and [`Effect::GrantKeywordAll`] for a class. `{3}{W}{W}{W}: Until end
+    /// of turn, this creature gets +2/+2 and gains lifelink` is **one** printed sentence
+    /// about one permanent, so it is one effect with one CR 613.7 timestamp.
+    ///
+    /// Deliberately not spelled through [`Effect::AlterAbilitiesSelf`], which is the
+    /// verb for a clause that *subtracts* — `loses defender and gains flying`, and its
+    /// `lose_all`. Reaching for the lose-all-abilities verb to say "gains lifelink" reads
+    /// as a card doing something it does not do, and the absence of any other way to say
+    /// it is why Resplendent Angel's third ability was missing from the catalog
+    /// altogether (issue #821).
     PumpSelf {
         /// The signed amount added to the source's power until end of turn.
         power: i32,
         /// The signed amount added to the source's toughness until end of turn.
         toughness: i32,
+        /// Keyword abilities granted to the source until end of turn. Empty for the
+        /// ordinary self-pump that only changes numbers. Duplicate grants are redundant,
+        /// not additive (CR 613.1f).
+        #[serde(default)]
+        keywords: Vec<Keyword>,
     },
     /// Change what abilities **this ability's own source** has until end of turn at
     /// CR 613 **layer 6** — `loses defender and gains flying until end of turn`, or

crates/sage-engine/src/ability/tests.rs

@@ -466,6 +466,7 @@ fn issue_149_put_counters_round_trips_with_both_kinds() {
         serde_json::from_str::<Effect>(plus).unwrap(),
         Effect::PutCounters {
             count_amount: None,
+            keywords: Vec::new(),
             targets: crate::ability::TargetCount::Exactly(1),
             target: TargetSpec::AnyCreature,
             counter: CounterKind::PlusOnePlusOne,
@@ -477,6 +478,7 @@ fn issue_149_put_counters_round_trips_with_both_kinds() {
         serde_json::from_str::<Effect>(minus).unwrap(),
         Effect::PutCounters {
             count_amount: None,
+            keywords: Vec::new(),
             targets: crate::ability::TargetCount::Exactly(1),
             target: TargetSpec::AnyCreature,
             counter: CounterKind::MinusOneMinusOne,

crates/sage-engine/src/apply/cast/effects.rs

@@ -496,7 +496,11 @@ pub(crate) fn apply_effect(
         // not a target (CR 115.1) and so was never chosen. A source that has left the
         // battlefield is not there to modify, and the effect simply does nothing —
         // the same no-op a fizzled target produces, without the fizzle.
-        Effect::PumpSelf { power, toughness } => {
+        Effect::PumpSelf {
+            power,
+            toughness,
+            keywords,
+        } => {
             if let Some(id) = permanent_source {
                 if state.battlefield.iter().any(|p| p.id == id) {
                     let stamp = state.mint_id();
@@ -509,6 +513,17 @@ pub(crate) fn apply_effect(
                         },
                         duration: Duration::UntilEndOfTurn,
                     });
+                    // The same source, in the same breath, at layer 6 instead of 7c —
+                    // the self-referential half of what `Effect::Pump` does for a target.
+                    for keyword in keywords {
+                        let stamp = state.mint_id();
+                        state.static_effects.push(StaticEffect {
+                            source: stamp,
+                            affects: EffectAffects::SpecificPermanent(id),
+                            modification: Modification::GrantKeyword(*keyword),
+                            duration: Duration::UntilEndOfTurn,
+                        });
+                    }
                 }
             }
         }

crates/sage-engine/src/apply/cast/targeted.rs

@@ -183,6 +183,7 @@ pub(crate) fn apply_targeted_effect(
             counter,
             count,
             count_amount,
+            keywords,
             ..
         } => {
             // The printed number, or the one the game supplies (CR 608.2) — read here
@@ -202,6 +203,20 @@ pub(crate) fn apply_targeted_effect(
             };
             if let Target::Permanent(id) = target {
                 state.put_counters_on_permanent(id, *counter, count, db);
+                // A keyword the same sentence grants the same creature, at CR 613 layer
+                // 6 and until end of turn — the counter stays and the keyword does not,
+                // which is the two durations one printed sentence really has. The
+                // permanent has been re-checked as a legal target by the caller
+                // (CR 608.2b); one that has since left is skipped by the `if let` above.
+                for keyword in keywords {
+                    let stamp = state.mint_id();
+                    state.static_effects.push(StaticEffect {
+                        source: stamp,
+                        affects: EffectAffects::SpecificPermanent(id),
+                        modification: Modification::GrantKeyword(*keyword),
+                        duration: Duration::UntilEndOfTurn,
+                    });
+                }
             }
         }
         // Pump the targeted creature until end of turn (CR 514.2): add a

crates/sage-engine/src/apply/combat/strike/dies_tests.rs

@@ -96,6 +96,7 @@ fn issue_151_dies_trigger_fires_from_a_minus_one_counter_toughness_drop() {
         lurker,
         vec![Effect::PutCounters {
             count_amount: None,
+            keywords: Vec::new(),
             targets: crate::ability::TargetCount::Exactly(1),
             target: TargetSpec::AnyCreature,
             counter: CounterKind::MinusOneMinusOne,

crates/sage-engine/src/card/helpers.rs

@@ -531,6 +531,7 @@ mod tests {
                 event: TriggerCondition::SelfEntersBattlefield,
                 effects: vec![Effect::PutCounters {
                     count_amount: None,
+                    keywords: Vec::new(),
                     targets: crate::ability::TargetCount::Exactly(1),
                     target: TargetSpec::AnyCreature,
                     counter: CounterKind::PlusOnePlusOne,
@@ -550,6 +551,7 @@ mod tests {
             crate::card::tests::card_named(&inline, "test_wither").spell_effects,
             vec![Effect::PutCounters {
                 count_amount: None,
+                keywords: Vec::new(),
                 targets: crate::ability::TargetCount::Exactly(1),
                 target: TargetSpec::AnyCreature,
                 counter: CounterKind::MinusOneMinusOne,

crates/sage-engine/src/resolve/tests.rs

@@ -424,6 +424,7 @@ fn issue_149_put_counters_ability_lands_on_its_target_cr_122() {
             origin: AbilityOrigin::Activated,
             effects: vec![Effect::PutCounters {
                 count_amount: None,
+                keywords: Vec::new(),
                 targets: crate::ability::TargetCount::Exactly(1),
                 target: TargetSpec::AnyCreature,
                 counter: CounterKind::PlusOnePlusOne,

crates/sage-engine/tests/m19_ability_removal.rs

@@ -464,3 +464,71 @@ fn cr_613_1f_a_keyword_gained_in_the_same_clause_survives_the_loss_of_everything
         "a granted keyword is not an entry in the ability list"
     );
 }
+
+// ----- Resplendent Angel: layer 6 adding, on the same row -------------------
+
+/// The additive counterpart, and the reason it is not spelled with the verb above.
+///
+/// `{3}{W}{W}{W}: Until end of turn, this creature gets +2/+2 and gains lifelink` was
+/// **missing from the catalog entirely** (#819, #821), and the shape of the vocabulary is
+/// why: 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 a `pump_self`
+/// carries its own keywords now — one printed sentence, one effect, one CR 613.7
+/// timestamp.
+#[test]
+fn issue_821_the_angel_pumps_and_grants_in_one_breath() {
+    let db = db();
+    let mut state = main_phase(&db, "forest");
+    let angel = place(&mut state, &db, "resplendent_angel", PlayerId(0));
+
+    let before = characteristics(&state, angel, &db);
+    assert_eq!((before.power, before.toughness), (Some(3), Some(3)));
+    assert!(before.keywords.contains(&Keyword::Flying), "printed flying");
+    assert!(!before.keywords.contains(&Keyword::Lifelink));
+
+    let state = activate(&state, &db, angel, 1);
+
+    let after = characteristics(&state, angel, &db);
+    assert_eq!(
+        (after.power, after.toughness),
+        (Some(5), Some(5)),
+        "a 3/3 with +2/+2"
+    );
+    assert!(after.keywords.contains(&Keyword::Lifelink), "and lifelink");
+    assert!(
+        after.keywords.contains(&Keyword::Flying),
+        "and the flying it printed — this grants, it does not replace, which is the \
+         whole difference from the removal verb above"
+    );
+}
+
+/// Both halves are `until end of turn` and both are gone at cleanup (CR 514.2), with
+/// nothing written onto the permanent to undo (ADR 0005).
+#[test]
+fn issue_821_the_angels_pump_and_its_lifelink_wear_off_together() {
+    let db = db();
+    let mut state = main_phase(&db, "forest");
+    let angel = place(&mut state, &db, "resplendent_angel", PlayerId(0));
+
+    let state = activate(&state, &db, angel, 1);
+    assert!(characteristics(&state, angel, &db)
+        .keywords
+        .contains(&Keyword::Lifelink));
+
+    let next_turn = settle_until(&state, &db, |s| s.turn == 2 && s.step == Step::Upkeep);
+    let after = characteristics(&next_turn, angel, &db);
+    assert_eq!(
+        (after.power, after.toughness),
+        (Some(3), Some(3)),
+        "the pump is gone"
+    );
+    assert!(
+        !after.keywords.contains(&Keyword::Lifelink),
+        "and so is the keyword it came with"
+    );
+    assert!(
+        after.keywords.contains(&Keyword::Flying),
+        "the printed keyword was never touched"
+    );
+}

crates/sage-engine/tests/m19_pay_for_a_trigger.rs

@@ -183,14 +183,24 @@ fn an_unpayable_offer_is_not_posed() {
     assert!(state.stack.is_empty());
 }
 
-/// Skyrider Patrol buys a two-effect ability, and both effects land on the one creature
-/// its single slot named.
+/// Skyrider Patrol buys **one** effect for **one** creature: `put a +1/+1 counter on
+/// another target creature you control, and that creature gains flying until end of
+/// turn` names one creature and then points back at it.
+///
+/// The ability was authored as a counter beside a standalone keyword grant, which is two
+/// effects each declaring the same target class — so it advertised two slots and this
+/// test filled both of them with the same creature, which is how it stayed green while
+/// the card was wrong (issue #821). A second ally is on the board now, and the slot count
+/// is asserted rather than assumed.
 #[test]
-fn skyrider_patrol_buys_a_counter_and_a_keyword_for_one_creature() {
+fn issue_821_skyrider_patrol_buys_a_counter_and_a_keyword_for_one_creature() {
     let db = db();
     let mut state = main_phase(&db);
     let patrol = place(&mut state, &db, "skyrider_patrol", PlayerId(0));
     let ally = place(&mut state, &db, "onakke_ogre", PlayerId(0));
+    // A second creature the ability could have aimed at, so "one slot" is a claim that
+    // can fail rather than a board with only one answer available.
+    let bystander = place(&mut state, &db, "centaur_courser", PlayerId(0));
     // Two lands to pay {G}{U} with. The pool empties between steps, so the mana has to be
     // made *while the offer is owed* (CR 605.3a) — which is the only time it is legal.
     let forest = place(&mut state, &db, "forest", PlayerId(0));
@@ -225,12 +235,29 @@ fn skyrider_patrol_buys_a_counter_and_a_keyword_for_one_creature() {
     let state = apply_action(&state, &Action::AnswerConfirm { accept: true }, &db);
 
     let ability = pending_trigger_target_choice(&state).expect("the ability owes targets");
+
+    // One slot, not two. A second target would be a card asking a question it does not
+    // print, and the answer would let a player counter one creature and fly another.
+    let two_targets = apply_action(
+        &state,
+        &Action::ChooseTriggerTargets {
+            ability,
+            mode: None,
+            targets: vec![Target::Permanent(ally), Target::Permanent(bystander)],
+        },
+        &db,
+    );
+    assert_eq!(
+        two_targets, state,
+        "the ability declares one target group, so a two-target answer is refused"
+    );
+
     let state = apply_action(
         &state,
         &Action::ChooseTriggerTargets {
             ability,
             mode: None,
-            targets: vec![Target::Permanent(ally), Target::Permanent(ally)],
+            targets: vec![Target::Permanent(ally)],
         },
         &db,
     );
@@ -240,6 +267,14 @@ fn skyrider_patrol_buys_a_counter_and_a_keyword_for_one_creature() {
     let stats = characteristics(&state, ally, &db);
     assert_eq!(stats.power, Some(5), "a 4/2 with a +1/+1 counter");
     assert!(stats.keywords.contains(&Keyword::Flying), "and flying");
+    // Both halves landed on the one creature the slot named, and nothing reached the
+    // other one — which is the whole of what "and that creature" means.
+    let untouched = characteristics(&state, bystander, &db);
+    assert_eq!(untouched.power, Some(3), "the bystander took no counter");
+    assert!(
+        !untouched.keywords.contains(&Keyword::Flying),
+        "and gained no flying"
+    );
     // "Another" is the source-relative class: the Patrol is not one of its own targets.
     assert_eq!(
         characteristics(&state, patrol, &db).power,