Unify event dispatch in Permanent#receive_event

docs/prs/pr-008-unify-event-dispatch.md

@@ -0,0 +1,41 @@
+# PR 8: Unify event dispatch in Permanent
+
+## Summary
+
+Replaces three separate event dispatch patterns in `Permanent#receive_event` with a single, consistent mechanism.
+
+## Why
+
+`Permanent#receive_event` contained two independent dispatch paths that served different purposes but had no clear boundary:
+
+1. A hardcoded `case` statement routing `EnteredTheBattlefield`, `LeftTheBattlefield`, and `CreatureDied` to `entered_the_battlefield!`, `left_the_battlefield!`, and `died!` helper methods
+2. A hash lookup in `card.event_handlers` for all other events
+
+The `case` path called `.perform` directly on trigger instances, bypassing `should_perform?`. The hash path called `.perform!`, which correctly gates on `should_perform?`. These two paths were also inconsistent with each other in naming (`def perform` vs `def call`) and in how much responsibility the trigger class held.
+
+Additionally, `add_event_handler` in `Cards::Shared::Events` had a bug: it set up an array with `||= []` then immediately overwrote it with `= klass`, preventing multiple handlers from accumulating for the same event type.
+
+## What Changed
+
+**`lib/magic/permanent.rb`**
+- Replaced the `case` block and duplicated handler dispatch in `receive_event` with two private methods: `dispatch_lifecycle_triggers` and `dispatch_event_handlers`
+- `dispatch_lifecycle_triggers` checks `event.permanent == self` and handles attachment cleanup on LTB — both things previously spread across three separate methods
+- `dispatch_event_handlers` wraps the hash lookup in `Array()` to support single-class or array values uniformly
+- Removed `left_the_battlefield!` and `died!` (internal-only, now inlined)
+- Kept `entered_the_battlefield!` public (used directly in some specs) delegating to the same private logic
+
+**`lib/magic/card.rb`**
+- DSL `enters_the_battlefield` now creates triggers with `def call` instead of `def perform`, consistent with how event_handlers triggers work
+
+**18 card files** (`academy_elite`, `annex_sentry`, `aven_gagglemaster`, `barrin_tolarian_archmage`, `basris_acolyte`, `bog_badger`, `carrion_grub`, `cloudkin_seer`, `conclave_mentor`, `elderfang_ritualist`, `gale_swooper`, `geist_honored_monk`, `hill_giant_herdgorger`, `idol_of_endurance`, `nine_lives`, `shalais_acolyte`, `storm_caller`, `valorous_steed`)
+- Renamed `def perform` → `def call` in ETB, LTB, and death trigger inner classes so they all go through `perform!` → `should_perform?` → `call`
+
+**`lib/magic/cards/shared/events.rb`**
+- Fixed `add_event_handler` to accumulate an array rather than overwrite; `dispatch_event_handlers` uses `Array()` to handle both single-class and array values
+
+## Invariants Preserved
+
+- ETB triggers still only fire when `event.permanent == self` (checked in `dispatch_lifecycle_triggers`)
+- Attachment cleanup on LTB still runs before LTB triggers
+- Cards using `event_handlers` for ETB/death events (e.g. `EssenceWarden`, `ArchonOfSunsGrace`) continue to gate on their own `should_perform?` as before
+- All 534 tests pass

lib/magic/card.rb

@@ -65,7 +65,7 @@ def modes(*modes)
 
       def enters_the_battlefield(&block)
         etb = Class.new(TriggeredAbility::EnterTheBattlefield)
-        etb.define_method(:perform, &block)
+        etb.define_method(:call, &block)
 
         define_method(:etb_triggers) do
           [etb]

lib/magic/cards/academy_elite.rb

@@ -9,7 +9,7 @@ module Cards
 
     class AcademyElite < Creature
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           counters = game.graveyard_cards.by_any_type(T::Instant, T::Sorcery).count
           actor.trigger_effect(:add_counter, target: actor, counter_type: Counters::Plus1Plus1, amount: counters)
         end

lib/magic/cards/annex_sentry.rb

@@ -33,15 +33,15 @@ def resolve!(target:)
 
 
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           game.choices.add(Choice.new(actor: actor))
         end
       end
 
       def etb_triggers = [ETB]
 
       class LTB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           actor.exiled_cards.each { _1.resolve! }
         end
       end

lib/magic/cards/aven_gagglemaster.rb

@@ -10,7 +10,7 @@ module Cards
 
     class AvenGagglemaster < Creature
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           flying_creatures = controller.creatures.count(&:flying?)
           actor.trigger_effect(:gain_life, life: 2 * flying_creatures)
         end

lib/magic/cards/barrin_tolarian_archmage.rb

@@ -26,7 +26,7 @@ def resolve!(target:)
       end
 
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           game.choices.add(Choice.new(actor: actor))
         end
       end

lib/magic/cards/basris_acolyte.rb

@@ -35,7 +35,7 @@ def resolve!(target:)
       end
 
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           choices = other_creatures_you_control
           effect = FirstChoice.new(
             actor: actor,

lib/magic/cards/bog_badger.rb

@@ -12,7 +12,7 @@ module Cards
 
     class BogBadger < Creature
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           if actor.kicked?
             controller.creatures.each do |creature|
               creature.grant_keyword(Keywords::MENACE, until_eot: true)

lib/magic/cards/carrion_grub.rb

@@ -8,7 +8,7 @@ module Cards
 
     class CarrionGrub < Creature
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           controller.mill(4)
         end
       end

lib/magic/cards/cloudkin_seer.rb

@@ -10,7 +10,7 @@ module Cards
 
     class CloudkinSeer < Creature
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           actor.trigger_effect(:draw_cards)
         end
       end

lib/magic/cards/conclave_mentor.rb

@@ -31,7 +31,7 @@ def replacement_effects
       end
 
       class Death < TriggeredAbility::Death
-        def perform
+        def call
           actor.trigger_effect(:gain_life, life: actor.power)
         end
       end

lib/magic/cards/elderfang_ritualist.rb

@@ -23,7 +23,7 @@ def resolve!(target:)
       end
 
       class Death < TriggeredAbility::Death
-        def perform
+        def call
           game.add_choice(ElderfangRitualist::Choice.new(actor: actor))
         end
       end

lib/magic/cards/gale_swooper.rb

@@ -15,7 +15,7 @@ def resolve!(target:)
       end
 
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           game.choices.add(Choice.new(actor: actor))
         end
       end

lib/magic/cards/geist_honored_monk.rb

@@ -34,7 +34,7 @@ def applicable_targets
       end
 
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           actor.trigger_effect(:create_token, token_class: SpiritToken, amount: 2)
         end
       end

lib/magic/cards/hill_giant_herdgorger.rb

@@ -9,7 +9,7 @@ module Cards
 
     class HillGiantHerdgorger < Creature
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           controller.gain_life(3)
         end
       end

lib/magic/cards/idol_of_endurance.rb

@@ -6,7 +6,7 @@ module Cards
 
     class IdolOfEndurance < Artifact
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           cards_to_exile = controller.graveyard.cmc_lte(3)
           cards_to_exile.each do |card|
             actor.trigger_effect(:exile, target: card)
@@ -18,7 +18,7 @@ def perform
       def etb_triggers = [ETB]
 
       class LTB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           # TODO: Should this take into account the graveyard the cards came _from_?
           # If this Idol changes controllers before the LTB is triggered, those cards would go to that new controller's graveyard.
           actor.exiled_cards.each { _1.move_to_graveyard!(controller) }

lib/magic/cards/nine_lives.rb

@@ -21,7 +21,7 @@ def call(effect)
       end
 
       class LTB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           actor.trigger_effect(:lose_game, player: controller)
         end
       end

lib/magic/cards/shalais_acolyte.rb

@@ -12,7 +12,7 @@ module Cards
 
     class ShalaisAcolyte < Creature
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           if actor.kicked?
             actor.trigger_effect(
               :add_counter,

lib/magic/cards/shared/events.rb

@@ -7,8 +7,7 @@ def event_handlers
         end
 
         def add_event_handler(event, klass)
-          event_handlers[event] ||= []
-          event_handlers[event] = klass
+          event_handlers[event] = Array(event_handlers[event]) + [klass]
         end
 
         def trigger_effect(effect, source: self, **args)

lib/magic/cards/storm_caller.rb

@@ -9,7 +9,7 @@ module Cards
 
     class StormCaller < Creature
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           actor.opponents.each do |opponent|
             actor.trigger_effect(:deal_damage, damage: 2, target: opponent)
           end

lib/magic/cards/valorous_steed.rb

@@ -16,7 +16,7 @@ module Cards
       end
 
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           actor.trigger_effect(:create_token, token_class: KnightToken)
         end
       end

lib/magic/permanent.rb

@@ -182,52 +182,12 @@ def replacement_matcher_applies?(matcher, effect)
 
     def receive_event(event)
       trigger_delayed_response(event)
-      case event
-      when Events::EnteredTheBattlefield
-        entered_the_battlefield!(event)
-      when Events::LeftTheBattlefield
-        left_the_battlefield!(event)
-      when Events::CreatureDied
-        died!(event)
-      end
-
-      handler_class = card.event_handlers[event.class]
-      if handler_class
-        # TODO: deprecate procs, move to classes
-        if handler_class.is_a?(Proc)
-          raise "Proc-based class for #{card}"
-          handler_class.call(self, event)
-        else
-          logger.debug "EVENT HANDLER: #{self} handling #{event}"
-          handler = handler_class.new(actor: self, event: event)
-
-          handler.perform!
-        end
-      end
-    end
-
-    def died!(event)
-      return unless event.permanent == self
-      card.death_triggers.each do |trigger|
-        trigger.new(actor: self, event: event).perform
-      end
-    end
-
-    def left_the_battlefield!(event)
-      return unless event.permanent == self
-      @attachments.each(&:destroy!)
-
-      card.ltb_triggers.each do |trigger|
-        trigger.new(actor: self, event: event).perform
-      end
+      dispatch_lifecycle_triggers(event)
+      dispatch_event_handlers(event)
     end
 
     def entered_the_battlefield!(event)
-      return unless event.permanent == self
-
-      card.etb_triggers.each do |trigger|
-        trigger.new(actor: self, event: event).perform
-      end
+      dispatch_lifecycle_triggers(event)
     end
 
     def protected_from?(card)
@@ -407,6 +367,32 @@ def devotion(color)
 
     private
 
+    def dispatch_lifecycle_triggers(event)
+      return unless event.respond_to?(:permanent) && event.permanent == self
+
+      @attachments.each(&:destroy!) if event.is_a?(Events::LeftTheBattlefield)
+
+      lifecycle_triggers_for(event).each do |trigger_class|
+        trigger_class.new(actor: self, event: event).perform!
+      end
+    end
+
+    def lifecycle_triggers_for(event)
+      case event
+      when Events::EnteredTheBattlefield then card.etb_triggers
+      when Events::LeftTheBattlefield     then card.ltb_triggers
+      when Events::CreatureDied           then card.death_triggers
+      else []
+      end
+    end
+
+    def dispatch_event_handlers(event)
+      Array(card.event_handlers[event.class]).each do |handler_class|
+        logger.debug "EVENT HANDLER: #{self} handling #{event}"
+        handler_class.new(actor: self, event: event).perform!
+      end
+    end
+
     def remove_until_eot_keyword_grants!
       until_eot_grants = keyword_grants.select(&:until_eot?)
       until_eot_grants.each do |grant|