From 11034549fd4a159a123f148e3330942997debd68 Mon Sep 17 00:00:00 2001
From: Ryan Bigg <me@ryanbigg.com>
Date: Thu, 23 Apr 2026 09:36:19 +1000
Subject: [PATCH] Unify event dispatch in Permanent#receive_event

Three separate dispatch paths (case statement for lifecycle events,
event_handlers hash for others, and inconsistent perform vs call naming)
are replaced with two private methods: dispatch_lifecycle_triggers and
dispatch_event_handlers. Lifecycle trigger classes renamed from def perform
to def call so all triggers go through perform! -> should_perform? -> call.
Fixed add_event_handler accumulation bug in Cards::Shared::Events.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 docs/prs/pr-008-unify-event-dispatch.md     | 41 ++++++++++++
 lib/magic/card.rb                           |  2 +-
 lib/magic/cards/academy_elite.rb            |  2 +-
 lib/magic/cards/annex_sentry.rb             |  4 +-
 lib/magic/cards/aven_gagglemaster.rb        |  2 +-
 lib/magic/cards/barrin_tolarian_archmage.rb |  2 +-
 lib/magic/cards/basris_acolyte.rb           |  2 +-
 lib/magic/cards/bog_badger.rb               |  2 +-
 lib/magic/cards/carrion_grub.rb             |  2 +-
 lib/magic/cards/cloudkin_seer.rb            |  2 +-
 lib/magic/cards/conclave_mentor.rb          |  2 +-
 lib/magic/cards/elderfang_ritualist.rb      |  2 +-
 lib/magic/cards/gale_swooper.rb             |  2 +-
 lib/magic/cards/geist_honored_monk.rb       |  2 +-
 lib/magic/cards/hill_giant_herdgorger.rb    |  2 +-
 lib/magic/cards/idol_of_endurance.rb        |  4 +-
 lib/magic/cards/nine_lives.rb               |  2 +-
 lib/magic/cards/shalais_acolyte.rb          |  2 +-
 lib/magic/cards/shared/events.rb            |  3 +-
 lib/magic/cards/storm_caller.rb             |  2 +-
 lib/magic/cards/valorous_steed.rb           |  2 +-
 lib/magic/permanent.rb                      | 72 +++++++++------------
 22 files changed, 92 insertions(+), 66 deletions(-)
 create mode 100644 docs/prs/pr-008-unify-event-dispatch.md

diff --git a/docs/prs/pr-008-unify-event-dispatch.md b/docs/prs/pr-008-unify-event-dispatch.md
new file mode 100644
index 00000000..7786e987
--- /dev/null
+++ b/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
diff --git a/lib/magic/card.rb b/lib/magic/card.rb
index a3fb3d79..0246d09f 100644
--- a/lib/magic/card.rb
+++ b/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]
diff --git a/lib/magic/cards/academy_elite.rb b/lib/magic/cards/academy_elite.rb
index d8afc4cc..cfa3d5db 100644
--- a/lib/magic/cards/academy_elite.rb
+++ b/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
diff --git a/lib/magic/cards/annex_sentry.rb b/lib/magic/cards/annex_sentry.rb
index 9ca0692f..8335da38 100644
--- a/lib/magic/cards/annex_sentry.rb
+++ b/lib/magic/cards/annex_sentry.rb
@@ -33,7 +33,7 @@ def resolve!(target:)
 
 
       class ETB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           game.choices.add(Choice.new(actor: actor))
         end
       end
@@ -41,7 +41,7 @@ def perform
       def etb_triggers = [ETB]
 
       class LTB < TriggeredAbility::EnterTheBattlefield
-        def perform
+        def call
           actor.exiled_cards.each { _1.resolve! }
         end
       end
diff --git a/lib/magic/cards/aven_gagglemaster.rb b/lib/magic/cards/aven_gagglemaster.rb
index edfe7d25..232468e7 100644
--- a/lib/magic/cards/aven_gagglemaster.rb
+++ b/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
diff --git a/lib/magic/cards/barrin_tolarian_archmage.rb b/lib/magic/cards/barrin_tolarian_archmage.rb
index a769de33..ca14aa1e 100644
--- a/lib/magic/cards/barrin_tolarian_archmage.rb
+++ b/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
diff --git a/lib/magic/cards/basris_acolyte.rb b/lib/magic/cards/basris_acolyte.rb
index 330f372f..b4db9680 100644
--- a/lib/magic/cards/basris_acolyte.rb
+++ b/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,
diff --git a/lib/magic/cards/bog_badger.rb b/lib/magic/cards/bog_badger.rb
index f7d24250..d3842e48 100644
--- a/lib/magic/cards/bog_badger.rb
+++ b/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)
diff --git a/lib/magic/cards/carrion_grub.rb b/lib/magic/cards/carrion_grub.rb
index 28d1d76d..830b811a 100644
--- a/lib/magic/cards/carrion_grub.rb
+++ b/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
diff --git a/lib/magic/cards/cloudkin_seer.rb b/lib/magic/cards/cloudkin_seer.rb
index dd892edf..3be1793d 100644
--- a/lib/magic/cards/cloudkin_seer.rb
+++ b/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
diff --git a/lib/magic/cards/conclave_mentor.rb b/lib/magic/cards/conclave_mentor.rb
index 0c5a1ae8..37ef576d 100644
--- a/lib/magic/cards/conclave_mentor.rb
+++ b/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
diff --git a/lib/magic/cards/elderfang_ritualist.rb b/lib/magic/cards/elderfang_ritualist.rb
index b60098b0..b1971531 100644
--- a/lib/magic/cards/elderfang_ritualist.rb
+++ b/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
diff --git a/lib/magic/cards/gale_swooper.rb b/lib/magic/cards/gale_swooper.rb
index e55feefa..cc2c7900 100644
--- a/lib/magic/cards/gale_swooper.rb
+++ b/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
diff --git a/lib/magic/cards/geist_honored_monk.rb b/lib/magic/cards/geist_honored_monk.rb
index aeb0b958..65c65366 100644
--- a/lib/magic/cards/geist_honored_monk.rb
+++ b/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
diff --git a/lib/magic/cards/hill_giant_herdgorger.rb b/lib/magic/cards/hill_giant_herdgorger.rb
index c66ec81a..8df5aa85 100644
--- a/lib/magic/cards/hill_giant_herdgorger.rb
+++ b/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
diff --git a/lib/magic/cards/idol_of_endurance.rb b/lib/magic/cards/idol_of_endurance.rb
index f30042cc..3fef21ea 100644
--- a/lib/magic/cards/idol_of_endurance.rb
+++ b/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) }
diff --git a/lib/magic/cards/nine_lives.rb b/lib/magic/cards/nine_lives.rb
index 6d897127..42f18d79 100644
--- a/lib/magic/cards/nine_lives.rb
+++ b/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
diff --git a/lib/magic/cards/shalais_acolyte.rb b/lib/magic/cards/shalais_acolyte.rb
index f390d7f4..fae16f64 100644
--- a/lib/magic/cards/shalais_acolyte.rb
+++ b/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,
diff --git a/lib/magic/cards/shared/events.rb b/lib/magic/cards/shared/events.rb
index dbc38771..8a8ab978 100644
--- a/lib/magic/cards/shared/events.rb
+++ b/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)
diff --git a/lib/magic/cards/storm_caller.rb b/lib/magic/cards/storm_caller.rb
index 7e8cfa2b..894bcd36 100644
--- a/lib/magic/cards/storm_caller.rb
+++ b/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
diff --git a/lib/magic/cards/valorous_steed.rb b/lib/magic/cards/valorous_steed.rb
index 2d7deff6..3433e94b 100644
--- a/lib/magic/cards/valorous_steed.rb
+++ b/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
diff --git a/lib/magic/permanent.rb b/lib/magic/permanent.rb
index d44164d5..ef731797 100644
--- a/lib/magic/permanent.rb
+++ b/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|