AddonLifecycle ReceiveEvent improvements (#1511)

* Prototype

* Add hook null safety

Add a check to make sure addons that invoke virtual functions for other addons don't trigger lifecycle messages multiple times.

* Expose event listeners for AddonLifecycleWidget.cs

Disable hook when all listeners for an addon unregister

* Add AddonLifecycleWidget.cs

* Remove excess logging

Dalamud/Game/Addon/Lifecycle/AddonLifecycle.cs

@@ -21,6 +21,16 @@ namespace Dalamud.Game.Addon.Lifecycle;
 [ServiceManager.EarlyLoadedService]
 internal unsafe class AddonLifecycle : IDisposable, IServiceType
 {
+    /// <summary>
+    /// List of all AddonLifecycle ReceiveEvent Listener Hooks.
+    /// </summary>
+    internal readonly List<AddonLifecycleReceiveEventListener> ReceiveEventListeners = new();
+    
+    /// <summary>
+    /// List of all AddonLifecycle Event Listeners.
+    /// </summary>
+    internal readonly List<AddonLifecycleEventListener> EventListeners = new();
+    
     private static readonly ModuleLog Log = new("AddonLifecycle");
 
     [ServiceManager.ServiceDependency]
@@ -39,9 +49,6 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
 
     private readonly ConcurrentBag<AddonLifecycleEventListener> newEventListeners = new();
     private readonly ConcurrentBag<AddonLifecycleEventListener> removeEventListeners = new();
-    private readonly List<AddonLifecycleEventListener> eventListeners = new();
-
-    private readonly Dictionary<string, Hook<AddonReceiveEventDelegate>> receiveEventHooks = new();
 
     [ServiceManager.ServiceConstructor]
     private AddonLifecycle(TargetSigScanner sigScanner)
@@ -75,8 +82,6 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
 
     private delegate byte AddonOnRefreshDelegate(AtkUnitManager* unitManager, AtkUnitBase* addon, uint valueCount, AtkValue* values);
 
-    private delegate void AddonReceiveEventDelegate(AtkUnitBase* addon, AtkEventType eventType, int eventParam, AtkEvent* atkEvent, nint a5);
-
     /// <inheritdoc/>
     public void Dispose()
     {
@@ -90,9 +95,9 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
         this.onAddonRefreshHook.Dispose();
         this.onAddonRequestedUpdateHook.Dispose();
 
-        foreach (var (_, hook) in this.receiveEventHooks)
+        foreach (var receiveEventListener in this.ReceiveEventListeners)
         {
-            hook.Dispose();
+            receiveEventListener.Dispose();
         }
     }
 
@@ -114,6 +119,20 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
         this.removeEventListeners.Add(listener);
     }
 
+    /// <summary>
+    /// Invoke listeners for the specified event type.
+    /// </summary>
+    /// <param name="eventType">Event Type.</param>
+    /// <param name="args">AddonArgs.</param>
+    internal void InvokeListeners(AddonEvent eventType, AddonArgs args)
+    {
+        // Match on string.empty for listeners that want events for all addons.
+        foreach (var listener in this.EventListeners.Where(listener => listener.EventType == eventType && (listener.AddonName == args.AddonName || listener.AddonName == string.Empty)))
+        {
+            listener.FunctionDelegate.Invoke(eventType, args);
+        }
+    }
+
     // Used to prevent concurrency issues if plugins try to register during iteration of listeners.
     private void OnFrameworkUpdate(IFramework unused)
     {
@@ -121,15 +140,15 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
         {
             foreach (var toAddListener in this.newEventListeners)
             {
-                this.eventListeners.Add(toAddListener);
+                this.EventListeners.Add(toAddListener);
 
                 // If we want receive event messages have an already active addon, enable the receive event hook.
                 // If the addon isn't active yet, we'll grab the hook when it sets up.
                 if (toAddListener is { EventType: AddonEvent.PreReceiveEvent or AddonEvent.PostReceiveEvent })
                 {
-                    if (this.receiveEventHooks.TryGetValue(toAddListener.AddonName, out var hook))
+                    if (this.ReceiveEventListeners.FirstOrDefault(listener => listener.AddonNames.Contains(toAddListener.AddonName)) is { } receiveEventListener)
                     {
-                        hook.Enable();
+                        receiveEventListener.Hook?.Enable();
                     }
                 }
             }
@@ -141,7 +160,21 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
         {
             foreach (var toRemoveListener in this.removeEventListeners)
             {
-                this.eventListeners.Remove(toRemoveListener);
+                this.EventListeners.Remove(toRemoveListener);
+                
+                // If we are disabling an ReceiveEvent listener, check if we should disable the hook.
+                if (toRemoveListener is { EventType: AddonEvent.PreReceiveEvent or AddonEvent.PostReceiveEvent })
+                {
+                    // Get the ReceiveEvent Listener for this addon
+                    if (this.ReceiveEventListeners.FirstOrDefault(listener => listener.AddonNames.Contains(toRemoveListener.AddonName)) is { } receiveEventListener)
+                    {
+                        // If there are no other listeners listening for this event, disable the hook.
+                        if (!this.EventListeners.Any(listener => listener.AddonName.Contains(toRemoveListener.AddonName) && listener.EventType is AddonEvent.PreReceiveEvent or AddonEvent.PostReceiveEvent))
+                        {
+                            receiveEventListener.Hook?.Disable();
+                        }
+                    }
+                }
             }
 
             this.removeEventListeners.Clear();
@@ -160,12 +193,53 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
         this.onAddonRequestedUpdateHook.Enable();
     }
 
-    private void InvokeListeners(AddonEvent eventType, AddonArgs args)
+    private void RegisterReceiveEventHook(AtkUnitBase* addon)
     {
-        // Match on string.empty for listeners that want events for all addons.
-        foreach (var listener in this.eventListeners.Where(listener => listener.EventType == eventType && (listener.AddonName == args.AddonName || listener.AddonName == string.Empty)))
+        // Hook the addon's ReceiveEvent function here, but only enable the hook if we have an active listener.
+        // Disallows hooking the core internal event handler.
+        var addonName = MemoryHelper.ReadStringNullTerminated((nint)addon->Name);
+        var receiveEventAddress = (nint)addon->VTable->ReceiveEvent;
+        if (receiveEventAddress != this.disallowedReceiveEventAddress)
         {
-            listener.FunctionDelegate.Invoke(eventType, args);
+            // If we have a ReceiveEvent listener already made for this hook address, add this addon's name to that handler.
+            if (this.ReceiveEventListeners.FirstOrDefault(listener => listener.HookAddress == receiveEventAddress) is { } existingListener)
+            {
+                if (!existingListener.AddonNames.Contains(addonName))
+                {
+                    existingListener.AddonNames.Add(addonName);
+                }
+            }
+
+            // Else, we have an addon that we don't have the ReceiveEvent for yet, make it.
+            else
+            {
+                this.ReceiveEventListeners.Add(new AddonLifecycleReceiveEventListener(this, addonName, receiveEventAddress));
+            }
+
+            // If we have an active listener for this addon already, we need to activate this hook.
+            if (this.EventListeners.Any(listener => (listener.EventType is AddonEvent.PostReceiveEvent or AddonEvent.PreReceiveEvent) && listener.AddonName == addonName))
+            {
+                if (this.ReceiveEventListeners.FirstOrDefault(listener => listener.AddonNames.Contains(addonName)) is { } receiveEventListener)
+                {
+                    receiveEventListener.Hook?.Enable();
+                }
+            }
+        }
+    }
+
+    private void UnregisterReceiveEventHook(string addonName)
+    {
+        // Remove this addons ReceiveEvent Registration
+        if (this.ReceiveEventListeners.FirstOrDefault(listener => listener.AddonNames.Contains(addonName)) is { } eventListener)
+        {
+            eventListener.AddonNames.Remove(addonName);
+
+            // If there are no more listeners let's remove and dispose.
+            if (eventListener.AddonNames.Count is 0)
+            {
+                this.ReceiveEventListeners.Remove(eventListener);
+                eventListener.Dispose();
+            }
         }
     }
 
@@ -173,20 +247,7 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
     {
         try
         {
-            // Hook the addon's ReceiveEvent function here, but only enable the hook if we have an active listener.
-            // Disallows hooking the core internal event handler.
-            var addonName = MemoryHelper.ReadStringNullTerminated((nint)addon->Name);
-            var receiveEventAddress = (nint)addon->VTable->ReceiveEvent;
-            if (receiveEventAddress != this.disallowedReceiveEventAddress)
-            {
-                var receiveEventHook = Hook<AddonReceiveEventDelegate>.FromAddress(receiveEventAddress, this.OnReceiveEvent);
-                this.receiveEventHooks.TryAdd(addonName, receiveEventHook);
-
-                if (this.eventListeners.Any(listener => (listener.EventType is AddonEvent.PostReceiveEvent or AddonEvent.PreReceiveEvent) && listener.AddonName == addonName))
-                {
-                    receiveEventHook.Enable();
-                }
-            }
+            this.RegisterReceiveEventHook(addon);
         }
         catch (Exception e)
         {
@@ -235,13 +296,8 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
     {
         try
         {
-            // Remove this addons ReceiveEvent Registration
             var addonName = MemoryHelper.ReadStringNullTerminated((nint)atkUnitBase[0]->Name);
-            if (this.receiveEventHooks.TryGetValue(addonName, out var hook))
-            {
-                hook.Dispose();
-                this.receiveEventHooks.Remove(addonName);
-            }
+            this.UnregisterReceiveEventHook(addonName);
         }
         catch (Exception e)
         {
@@ -410,51 +466,6 @@ internal unsafe class AddonLifecycle : IDisposable, IServiceType
             Log.Error(e, "Exception in OnRequestedUpdate post-requestedUpdate invoke.");
         }
     }
-
-    private void OnReceiveEvent(AtkUnitBase* addon, AtkEventType eventType, int eventParam, AtkEvent* atkEvent, nint data)
-    {
-        try
-        {
-            this.InvokeListeners(AddonEvent.PreReceiveEvent, new AddonReceiveEventArgs
-            {
-                Addon = (nint)addon, 
-                AtkEventType = (byte)eventType,
-                EventParam = eventParam,
-                AtkEvent = (nint)atkEvent,
-                Data = data,
-            });
-        }
-        catch (Exception e)
-        {
-            Log.Error(e, "Exception in OnReceiveEvent pre-receiveEvent invoke.");
-        }
-
-        try
-        {
-            var addonName = MemoryHelper.ReadStringNullTerminated((nint)addon->Name);
-            this.receiveEventHooks[addonName].Original(addon, eventType, eventParam, atkEvent, data);
-        }
-        catch (Exception e)
-        {
-            Log.Error(e, "Caught exception when calling original AddonReceiveEvent. This may be a bug in the game or another plugin hooking this method.");
-        }
-        
-        try
-        {
-            this.InvokeListeners(AddonEvent.PostReceiveEvent, new AddonReceiveEventArgs
-            {
-                Addon = (nint)addon, 
-                AtkEventType = (byte)eventType,
-                EventParam = eventParam,
-                AtkEvent = (nint)atkEvent,
-                Data = data,
-            });
-        }
-        catch (Exception e)
-        {
-            Log.Error(e, "Exception in OnAddonRefresh post-receiveEvent invoke.");
-        }
-    }
 }
 
 /// <summary>

Dalamud/Game/Addon/Lifecycle/AddonLifecycleReceiveEventListener.cs

@@ -0,0 +1,119 @@
+using System.Collections.Generic;
+
+using Dalamud.Game.Addon.Lifecycle.AddonArgTypes;
+using Dalamud.Hooking;
+using Dalamud.Logging.Internal;
+using Dalamud.Memory;
+using FFXIVClientStructs.FFXIV.Component.GUI;
+
+namespace Dalamud.Game.Addon.Lifecycle;
+
+/// <summary>
+/// This class is a helper for tracking and invoking listener delegates for Addon_OnReceiveEvent.
+/// Multiple addons may use the same ReceiveEvent function, this helper makes sure that those addon events are handled properly.
+/// </summary>
+internal unsafe class AddonLifecycleReceiveEventListener : IDisposable
+{
+    private static readonly ModuleLog Log = new("AddonLifecycle");
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="AddonLifecycleReceiveEventListener"/> class.
+    /// </summary>
+    /// <param name="service">AddonLifecycle service instance.</param>
+    /// <param name="addonName">Initial Addon Requesting this listener.</param>
+    /// <param name="receiveEventAddress">Address of Addon's ReceiveEvent function.</param>
+    internal AddonLifecycleReceiveEventListener(AddonLifecycle service, string addonName, nint receiveEventAddress)
+    {
+        this.AddonLifecycle = service;
+        this.AddonNames = new List<string> { addonName };
+        this.Hook = Hook<AddonReceiveEventDelegate>.FromAddress(receiveEventAddress, this.OnReceiveEvent);
+    }
+
+    /// <summary>
+    /// Addon Receive Event Function delegate.
+    /// </summary>
+    /// <param name="addon">Addon Pointer.</param>
+    /// <param name="eventType">Event Type.</param>
+    /// <param name="eventParam">Unique Event ID.</param>
+    /// <param name="atkEvent">Event Data.</param>
+    /// <param name="a5">Unknown.</param>
+    public delegate void AddonReceiveEventDelegate(AtkUnitBase* addon, AtkEventType eventType, int eventParam, AtkEvent* atkEvent, nint a5);
+
+    /// <summary>
+    /// Gets the list of addons that use this receive event hook.
+    /// </summary>
+    public List<string> AddonNames { get; init; }
+    
+    /// <summary>
+    /// Gets the address of the registered hook.
+    /// </summary>
+    public nint HookAddress => this.Hook?.Address ?? nint.Zero;
+    
+    /// <summary>
+    /// Gets the contained hook for these addons.
+    /// </summary>
+    public Hook<AddonReceiveEventDelegate>? Hook { get; init; }
+    
+    /// <summary>
+    /// Gets or sets the Reference to AddonLifecycle service instance.
+    /// </summary>
+    private AddonLifecycle AddonLifecycle { get; set; }
+
+    /// <inheritdoc/>
+    public void Dispose()
+    {
+        this.Hook?.Dispose();
+    }
+
+    private void OnReceiveEvent(AtkUnitBase* addon, AtkEventType eventType, int eventParam, AtkEvent* atkEvent, nint data)
+    {
+        // Check that we didn't get here through a call to another addons handler.
+        var addonName = MemoryHelper.ReadString((nint)addon->Name, 0x20);
+        if (!this.AddonNames.Contains(addonName))
+        {
+            this.Hook!.Original(addon, eventType, eventParam, atkEvent, data);
+            return;
+        }
+        
+        try
+        {
+            this.AddonLifecycle.InvokeListeners(AddonEvent.PreReceiveEvent, new AddonReceiveEventArgs
+            {
+                Addon = (nint)addon, 
+                AtkEventType = (byte)eventType,
+                EventParam = eventParam,
+                AtkEvent = (nint)atkEvent,
+                Data = data,
+            });
+        }
+        catch (Exception e)
+        {
+            Log.Error(e, "Exception in OnReceiveEvent pre-receiveEvent invoke.");
+        }
+        
+        try
+        {
+            this.Hook!.Original(addon, eventType, eventParam, atkEvent, data);
+        }
+        catch (Exception e)
+        {
+            Log.Error(e, "Caught exception when calling original AddonReceiveEvent. This may be a bug in the game or another plugin hooking this method.");
+        }
+
+        try
+        {
+            this.AddonLifecycle.InvokeListeners(AddonEvent.PostReceiveEvent, new AddonReceiveEventArgs
+            {
+                Addon = (nint)addon, 
+                AtkEventType = (byte)eventType,
+                EventParam = eventParam,
+                AtkEvent = (nint)atkEvent,
+                Data = data,
+            });
+        }
+        catch (Exception e)
+        {
+            Log.Error(e, "Exception in OnAddonRefresh post-receiveEvent invoke.");
+        }
+    }
+}

Dalamud/Interface/Internal/Windows/Data/DataWindow.cs

@@ -50,6 +50,7 @@ internal class DataWindow : Window
         new DataShareWidget(),
         new NetworkMonitorWidget(),
         new IconBrowserWidget(),
+        new AddonLifecycleWidget(),
     };
 
     private readonly IOrderedEnumerable<IDataWindowWidget> orderedModules;

Dalamud/Interface/Internal/Windows/Data/Widgets/AddonLifecycleWidget.cs

@@ -0,0 +1,135 @@
+using System.Diagnostics.CodeAnalysis;
+using System.Drawing;
+using System.Linq;
+using Dalamud.Game.Addon.Lifecycle;
+using Dalamud.Interface.Utility;
+using ImGuiNET;
+
+namespace Dalamud.Interface.Internal.Windows.Data.Widgets;
+
+/// <summary>
+/// Debug widget for displaying AddonLifecycle data.
+/// </summary>
+public class AddonLifecycleWidget : IDataWindowWidget
+{
+    /// <inheritdoc/>
+    public string[]? CommandShortcuts { get; init; } = { "AddonLifecycle" };
+
+    /// <inheritdoc/>
+    public string DisplayName { get; init; } = "Addon Lifecycle";
+    
+    /// <inheritdoc/>
+    [MemberNotNullWhen(true, "AddonLifecycle")]
+    public bool Ready { get; set; }
+    
+    private AddonLifecycle? AddonLifecycle { get; set; }
+
+    /// <inheritdoc/>
+    public void Load()
+    {
+        this.AddonLifecycle = Service<AddonLifecycle>.GetNullable();
+        if (this.AddonLifecycle is not null) this.Ready = true;
+    }
+    
+    /// <inheritdoc/>
+    public void Draw()
+    {
+        if (!this.Ready)
+        {
+            ImGui.Text("AddonLifecycle Reference is null, reload module.");
+            return;
+        }
+
+        if (ImGui.CollapsingHeader("Listeners"))
+        {
+            ImGui.Indent();
+            this.DrawEventListeners();
+            ImGui.Unindent();
+        }
+
+        if (ImGui.CollapsingHeader("ReceiveEvent Hooks"))
+        {
+            ImGui.Indent();
+            this.DrawReceiveEventHooks();
+            ImGui.Unindent();
+        }
+    }
+    
+    private void DrawEventListeners()
+    {
+        if (!this.Ready) return;
+        
+        foreach (var eventType in Enum.GetValues<AddonEvent>())
+        {
+            if (ImGui.CollapsingHeader(eventType.ToString()))
+            {
+                ImGui.Indent();
+                var listeners = this.AddonLifecycle.EventListeners.Where(listener => listener.EventType == eventType).ToList();
+
+                if (!listeners.Any())
+                {
+                    ImGui.Text("No Listeners Registered for Event");
+                }
+                
+                if (ImGui.BeginTable("AddonLifecycleListenersTable", 2))
+                {
+                    ImGui.TableSetupColumn("##AddonName", ImGuiTableColumnFlags.WidthFixed, 100.0f * ImGuiHelpers.GlobalScale);
+                    ImGui.TableSetupColumn("##MethodInvoke", ImGuiTableColumnFlags.WidthStretch);
+
+                    foreach (var listener in listeners)
+                    {
+                        ImGui.TableNextColumn();
+                        ImGui.Text(listener.AddonName is "" ? "GLOBAL" : listener.AddonName);
+
+                        ImGui.TableNextColumn();
+                        ImGui.Text($"{listener.FunctionDelegate.Target}::{listener.FunctionDelegate.Method.Name}");
+                    }
+                    
+                    ImGui.EndTable();
+                }
+                
+                ImGui.Unindent();
+            }
+        }
+    }
+
+    private void DrawReceiveEventHooks()
+    {
+        if (!this.Ready) return;
+
+        var listeners = this.AddonLifecycle.ReceiveEventListeners;
+
+        if (!listeners.Any())
+        {
+            ImGui.Text("No ReceiveEvent Hooks are Registered");
+        }
+        
+        foreach (var receiveEventListener in this.AddonLifecycle.ReceiveEventListeners)
+        {
+            if (ImGui.CollapsingHeader(string.Join(", ", receiveEventListener.AddonNames)))
+            {
+                ImGui.Columns(2);
+
+                ImGui.Text("Hook Address");
+                ImGui.NextColumn();
+                ImGui.Text(receiveEventListener.HookAddress.ToString("X"));
+
+                ImGui.NextColumn();
+                ImGui.Text("Hook Status");
+                ImGui.NextColumn();
+                if (receiveEventListener.Hook is null)
+                {
+                    ImGui.Text("Hook is null");
+                }
+                else
+                {
+                    var color = receiveEventListener.Hook.IsEnabled ? KnownColor.Green.Vector() : KnownColor.OrangeRed.Vector();
+                    var text = receiveEventListener.Hook.IsEnabled ? "Enabled" : "Disabled";
+                    ImGui.TextColored(color, text);
+                }
+                
+                ImGui.Columns(1);
+            }
+        }
+    }
+}