refactor(engine): unify tap effects into Effect::SetTapState (#2777)

crates/engine/src/database/forge/effect.rs

@@ -1,5 +1,6 @@
 use crate::types::ability::{
-    ControllerRef, Effect, ManaProduction, PtValue, QuantityExpr, TargetFilter, TypedFilter,
+    ControllerRef, Effect, EffectScope, ManaProduction, PtValue, QuantityExpr, TapStateChange,
+    TargetFilter, TypedFilter,
 };
 use crate::types::mana::ManaColor;
 use crate::types::Zone;
@@ -341,13 +342,21 @@ fn translate_destroy_all(params: &ForgeParams) -> Result<Effect, ForgeTranslateE
 // CR 701.26a: Tap target permanent.
 fn translate_tap(params: &ForgeParams) -> Result<Effect, ForgeTranslateError> {
     let target = resolve_target(params, "ValidTgts");
-    Ok(Effect::Tap { target })
+    Ok(Effect::SetTapState {
+        target,
+        scope: EffectScope::Single,
+        state: TapStateChange::Tap,
+    })
 }
 
-// CR 701.26a: Untap target permanent.
+// CR 701.26b: Untap target permanent.
 fn translate_untap(params: &ForgeParams) -> Result<Effect, ForgeTranslateError> {
     let target = resolve_target(params, "ValidTgts");
-    Ok(Effect::Untap { target })
+    Ok(Effect::SetTapState {
+        target,
+        scope: EffectScope::Single,
+        state: TapStateChange::Untap,
+    })
 }
 
 // CR 400.7: Move objects between zones.

crates/engine/src/database/synthesis.rs

@@ -13,13 +13,14 @@ use crate::types::ability::{
     AttackScope, AttackSubject, CardPlayMode, CastFromZoneDriver, CastManaObjectScope,
     CastManaSpentMetric, CastVariantPaid, ChoiceType, Comparator, ContinuousModification,
     ControllerRef, CopyRetargetPermission, CounterTriggerFilter, DamageKindFilter,
-    DamageModification, DelayedTriggerCondition, Duration, Effect, FilterProp, KickerVariant,
-    ManaContribution, ManaProduction, ModalSelectionCondition, ModalSelectionConstraint,
-    NinjutsuVariant, ObjectScope, ParsedCondition, PaymentCost, PlayerFilter, PlayerScope, PtStat,
-    PtValue, PtValueScope, QuantityExpr, QuantityRef, RenownSubject, ReplacementCondition,
-    ReplacementDefinition, RuntimeHandler, SearchSelectionConstraint, StaticCondition,
-    StaticDefinition, TargetChoiceTiming, TargetFilter, TriggerCondition, TriggerDefinition,
-    TypeFilter, TypedFilter, UnlessPayModifier,
+    DamageModification, DelayedTriggerCondition, Duration, Effect, EffectScope, FilterProp,
+    KickerVariant, ManaContribution, ManaProduction, ModalSelectionCondition,
+    ModalSelectionConstraint, NinjutsuVariant, ObjectScope, ParsedCondition, PaymentCost,
+    PlayerFilter, PlayerScope, PtStat, PtValue, PtValueScope, QuantityExpr, QuantityRef,
+    RenownSubject, ReplacementCondition, ReplacementDefinition, RuntimeHandler,
+    SearchSelectionConstraint, StaticCondition, StaticDefinition, TapStateChange,
+    TargetChoiceTiming, TargetFilter, TriggerCondition, TriggerDefinition, TypeFilter, TypedFilter,
+    UnlessPayModifier,
 };
 use crate::types::card::{CardFace, CardLayout, CleaveVariant};
 use crate::types::card_type::{CardType, CoreType, Supertype};
@@ -4772,8 +4773,10 @@ fn is_provoke_attack_trigger(t: &TriggerDefinition) -> bool {
     }
     // CR 702.39a + CR 701.26b: the parent body untaps a creature the defending
     // player controls.
-    let Effect::Untap {
+    let Effect::SetTapState {
         target: TargetFilter::Typed(tf),
+        scope: EffectScope::Single,
+        state: TapStateChange::Untap,
     } = &*execute.effect
     else {
         return false;
@@ -4829,14 +4832,21 @@ fn build_enlist_trigger() -> TriggerDefinition {
 
     // CR 702.154a: "you may tap … when you do, [pump]." The optional parent taps
     // the eligible creature; the reflexive pump rides as its sub-ability.
-    let execute = AbilityDefinition::new(AbilityKind::Spell, Effect::Tap { target: tap_target })
-        .optional()
-        .sub_ability(pump)
-        .description(
-            "Enlist — you may tap an untapped creature you control; if you do, this \
+    let execute = AbilityDefinition::new(
+        AbilityKind::Spell,
+        Effect::SetTapState {
+            target: tap_target,
+            scope: EffectScope::Single,
+            state: TapStateChange::Tap,
+        },
+    )
+    .optional()
+    .sub_ability(pump)
+    .description(
+        "Enlist — you may tap an untapped creature you control; if you do, this \
              creature gets +X/+0 where X is that creature's power"
-                .to_string(),
-        );
+            .to_string(),
+    );
 
     TriggerDefinition::new(TriggerMode::Attacks)
         .valid_card(TargetFilter::SelfRef)
@@ -4886,7 +4896,14 @@ fn is_enlist_trigger(t: &TriggerDefinition) -> bool {
         return false;
     };
     execute.optional
-        && matches!(&*execute.effect, Effect::Tap { .. })
+        && matches!(
+            &*execute.effect,
+            Effect::SetTapState {
+                scope: EffectScope::Single,
+                state: TapStateChange::Tap,
+                ..
+            }
+        )
         && execute
             .sub_ability
             .as_deref()
@@ -4936,8 +4953,10 @@ fn build_provoke_trigger() -> TriggerDefinition {
     // optional parent body untaps the chosen defender, then force-blocks it.
     let execute = AbilityDefinition::new(
         AbilityKind::Spell,
-        Effect::Untap {
+        Effect::SetTapState {
             target: untap_target,
+            scope: EffectScope::Single,
+            state: TapStateChange::Untap,
         },
     )
     .optional()
@@ -12823,8 +12842,10 @@ mod provoke_synthesis_tests {
         );
 
         // CR 702.39a + CR 701.26b: parent body untaps the defending player's creature.
-        let Effect::Untap {
+        let Effect::SetTapState {
             target: TargetFilter::Typed(tf),
+            scope: EffectScope::Single,
+            state: TapStateChange::Untap,
         } = &*execute.effect
         else {
             panic!("execute body must be Effect::Untap over a TypedFilter");
@@ -12892,10 +12913,12 @@ mod provoke_synthesis_tests {
             .valid_card(TargetFilter::SelfRef)
             .execute(AbilityDefinition::new(
                 AbilityKind::Spell,
-                Effect::Untap {
+                Effect::SetTapState {
                     target: TargetFilter::Typed(
                         TypedFilter::creature().controller(ControllerRef::DefendingPlayer),
                     ),
+                    scope: EffectScope::Single,
+                    state: TapStateChange::Untap,
                 },
             ));
         assert!(
@@ -12955,7 +12978,12 @@ mod provoke_synthesis_tests {
         );
 
         // Parent body taps an eligible Enlist creature.
-        let Effect::Tap { target } = &*execute.effect else {
+        let Effect::SetTapState {
+            target,
+            scope: EffectScope::Single,
+            state: TapStateChange::Tap,
+        } = &*execute.effect
+        else {
             panic!("execute body must be Effect::Tap");
         };
         let TargetFilter::And { filters } = target else {

crates/engine/src/game/ability_utils.rs

@@ -1,6 +1,8 @@
+#[cfg(test)]
+use crate::types::ability::TapStateChange;
 use crate::types::ability::{
     AbilityCondition, AbilityDefinition, CardTypeSetSource, CastManaSpentMetric,
-    CombatRelationSubject, ControllerRef, CounterMoveSelection, Effect, FilterProp,
+    CombatRelationSubject, ControllerRef, CounterMoveSelection, Effect, EffectScope, FilterProp,
     GameRestriction, ModalChoice, ModalSelectionCondition, ModalSelectionConstraint,
     MultiTargetSpec, ObjectScope, PlayerFilter, QuantityExpr, QuantityRef, ResolvedAbility,
     RestrictionPlayerScope, SpellContext, SubAbilityLink, TargetChoiceTiming, TargetFilter,
@@ -1734,8 +1736,11 @@ fn effect_references_target_player(effect: &Effect) -> bool {
         | Effect::DestroyAll { target, .. }
         | Effect::PumpAll { target, .. }
         | Effect::DamageAll { target, .. }
-        | Effect::TapAll { target, .. }
-        | Effect::UntapAll { target, .. }
+        | Effect::SetTapState {
+            scope: EffectScope::All,
+            target,
+            ..
+        }
         | Effect::BounceAll { target, .. }
         | Effect::CounterAll { target, .. }
         | Effect::ChangeZoneAll { target, .. }
@@ -1939,8 +1944,11 @@ fn effect_references_parent_target_combat_relation(effect: &Effect) -> bool {
     match effect {
         Effect::DestroyAll { target, .. }
         | Effect::PumpAll { target, .. }
-        | Effect::TapAll { target, .. }
-        | Effect::UntapAll { target, .. }
+        | Effect::SetTapState {
+            scope: EffectScope::All,
+            target,
+            ..
+        }
         | Effect::BounceAll { target, .. }
         | Effect::CounterAll { target, .. }
         | Effect::ChangeZoneAll { target, .. }
@@ -2013,8 +2021,11 @@ fn effect_references_target_creature_quantity(effect: &Effect) -> bool {
         }
         Effect::DestroyAll { target, .. }
         | Effect::PumpAll { target, .. }
-        | Effect::TapAll { target, .. }
-        | Effect::UntapAll { target, .. }
+        | Effect::SetTapState {
+            scope: EffectScope::All,
+            target,
+            ..
+        }
         | Effect::BounceAll { target, .. }
         | Effect::CounterAll { target, .. }
         | Effect::ChangeZoneAll { target, .. }
@@ -4858,8 +4869,10 @@ mod tests {
         };
         let def = AbilityDefinition::new(
             AbilityKind::Activated,
-            Effect::Tap {
+            Effect::SetTapState {
                 target: TargetFilter::ParentTarget,
+                scope: EffectScope::Single,
+                state: TapStateChange::Tap,
             },
         )
         .unless_pay(modifier.clone());
@@ -6638,10 +6651,12 @@ mod tests {
             .tapped = true;
         let mut ability = per_opponent_gain_control_ability().sub_ability(
             ResolvedAbility::new(
-                Effect::Untap {
+                Effect::SetTapState {
                     target: TargetFilter::TrackedSet {
                         id: TrackedSetId(0),
                     },
+                    scope: EffectScope::Single,
+                    state: TapStateChange::Untap,
                 },
                 vec![],
                 ObjectId(900),
@@ -7267,8 +7282,10 @@ mod tests {
             TargetFilter::Typed(TypedFilter::creature().controller(ControllerRef::TargetPlayer));
 
         let ability = ResolvedAbility::new(
-            Effect::Tap {
+            Effect::SetTapState {
                 target: creature_filter.clone(),
+                scope: EffectScope::Single,
+                state: TapStateChange::Tap,
             },
             vec![],
             ObjectId(900),
@@ -7607,8 +7624,10 @@ mod tests {
         }
 
         let mut ability = ResolvedAbility::new(
-            Effect::Tap {
+            Effect::SetTapState {
                 target: TargetFilter::Typed(TypedFilter::creature()),
+                scope: EffectScope::Single,
+                state: TapStateChange::Tap,
             },
             vec![],
             ObjectId(10),
@@ -7649,8 +7668,10 @@ mod tests {
         }
 
         let mut ability = ResolvedAbility::new(
-            Effect::Tap {
+            Effect::SetTapState {
                 target: TargetFilter::Typed(TypedFilter::creature()),
+                scope: EffectScope::Single,
+                state: TapStateChange::Tap,
             },
             vec![],
             ObjectId(10),
@@ -7683,8 +7704,10 @@ mod tests {
             .push(crate::types::card_type::CoreType::Creature);
 
         let mut ability = ResolvedAbility::new(
-            Effect::Tap {
+            Effect::SetTapState {
                 target: TargetFilter::Typed(TypedFilter::creature()),
+                scope: EffectScope::Single,
+                state: TapStateChange::Tap,
             },
             vec![],
             ObjectId(10),
@@ -7726,8 +7749,10 @@ mod tests {
         }
 
         let mut ability = ResolvedAbility::new(
-            Effect::Tap {
+            Effect::SetTapState {
                 target: TargetFilter::Typed(TypedFilter::creature()),
+                scope: EffectScope::Single,
+                state: TapStateChange::Tap,
             },
             vec![],
             ObjectId(10),
@@ -7772,8 +7797,10 @@ mod tests {
         }
 
         let mut ability = ResolvedAbility::new(
-            Effect::Untap {
+            Effect::SetTapState {
                 target: TargetFilter::Typed(TypedFilter::land()),
+                scope: EffectScope::Single,
+                state: TapStateChange::Untap,
             },
             vec![],
             ObjectId(10),
@@ -7812,8 +7839,10 @@ mod tests {
         }
 
         let mut ability = ResolvedAbility::new(
-            Effect::Untap {
+            Effect::SetTapState {
                 target: TargetFilter::Typed(TypedFilter::land()),
+                scope: EffectScope::Single,
+                state: TapStateChange::Untap,
             },
             vec![],
             ObjectId(10),
@@ -8312,8 +8341,10 @@ mod tests {
                 target: TargetFilter::Typed(TypedFilter::creature()),
                 cant_regenerate: false,
             }),
-            single_target_mode(Effect::Tap {
+            single_target_mode(Effect::SetTapState {
                 target: TargetFilter::Typed(TypedFilter::creature()),
+                scope: EffectScope::Single,
+                state: TapStateChange::Tap,
             }),
         ];
         let descriptions = vec![
@@ -8351,8 +8382,10 @@ mod tests {
             target: TargetFilter::Typed(TypedFilter::creature()),
             cant_regenerate: false,
         });
-        mode.sub_ability = Some(Box::new(single_target_mode(Effect::Tap {
+        mode.sub_ability = Some(Box::new(single_target_mode(Effect::SetTapState {
             target: TargetFilter::Typed(TypedFilter::creature()),
+            scope: EffectScope::Single,
+            state: TapStateChange::Tap,
         })));
         let abilities = vec![mode];
         let descriptions = vec!["Destroy then tap.".to_string()];
@@ -8386,8 +8419,10 @@ mod tests {
         let mut state = crate::types::game_state::GameState::new_two_player(42);
         spawn_creatures(&mut state, PlayerId(1), 1);
 
-        let mode = single_target_mode(Effect::Tap {
+        let mode = single_target_mode(Effect::SetTapState {
             target: TargetFilter::Typed(TypedFilter::creature()),
+            scope: EffectScope::Single,
+            state: TapStateChange::Tap,
         });
         let abilities = vec![mode];
         let descriptions = vec!["Tap a creature.".to_string()];
@@ -8420,8 +8455,10 @@ mod tests {
         let mut state = crate::types::game_state::GameState::new_two_player(42);
         spawn_creatures(&mut state, PlayerId(0), 1);
 
-        let abilities = vec![single_target_mode(Effect::Tap {
+        let abilities = vec![single_target_mode(Effect::SetTapState {
             target: TargetFilter::Typed(TypedFilter::creature()),
+            scope: EffectScope::Single,
+            state: TapStateChange::Tap,
         })];
 
         let (slots, labels) = build_target_slots_labelled(
@@ -8456,8 +8493,10 @@ mod tests {
         max: usize,
     ) -> ResolvedAbility {
         let mut ability = ResolvedAbility::new(
-            Effect::Tap {
+            Effect::SetTapState {
                 target: TargetFilter::Typed(TypedFilter::creature()),
+                scope: EffectScope::Single,
+                state: TapStateChange::Tap,
             },
             vec![],
             source,

crates/engine/src/game/attractions.rs

@@ -272,8 +272,10 @@ mod tests {
             src.replacement_definitions = vec![ReplacementDefinition::new(ReplacementEvent::Moved)
                 .execute(AbilityDefinition::new(
                     AbilityKind::Spell,
-                    Effect::Tap {
+                    Effect::SetTapState {
                         target: TargetFilter::SelfRef,
+                        scope: crate::types::ability::EffectScope::Single,
+                        state: crate::types::ability::TapStateChange::Tap,
                     },
                 ))
                 .destination_zone(Zone::Battlefield)