Add ClientStatePluginScoped (#1384)

* Add ClientStatePluginScoped

* Restore InvokeSafely

* Add InvokeSafely for basic Action types.

* Set delegates to null to prevent leaking memory

* Resolve Merge

Dalamud/Game/ClientState/ClientState.cs

@@ -8,24 +8,25 @@ using Dalamud.Game.Network.Internal;
 using Dalamud.Hooking;
 using Dalamud.IoC;
 using Dalamud.IoC.Internal;
+using Dalamud.Logging.Internal;
 using Dalamud.Plugin.Services;
 using Dalamud.Utility;
 using FFXIVClientStructs.FFXIV.Client.Game;
-using Serilog;
+using Lumina.Excel.GeneratedSheets;
+
+using Action = System.Action;
 
 namespace Dalamud.Game.ClientState;
 
 /// <summary>
 /// This class represents the state of the game client at the time of access.
 /// </summary>
-[PluginInterface]
 [InterfaceVersion("1.0")]
 [ServiceManager.BlockingEarlyLoadedService]
-#pragma warning disable SA1015
-[ResolveVia<IClientState>]
-#pragma warning restore SA1015
 internal sealed class ClientState : IDisposable, IServiceType, IClientState
 {
+    private static readonly ModuleLog Log = new("ClientState");
+    
     private readonly GameLifecycle lifecycle;
     private readonly ClientStateAddressResolver address;
     private readonly Hook<SetupTerritoryTypeDelegate> setupTerritoryTypeHook;
@@ -37,7 +38,7 @@ internal sealed class ClientState : IDisposable, IServiceType, IClientState
     private readonly NetworkHandlers networkHandlers = Service<NetworkHandlers>.Get();
 
     private bool lastConditionNone = true;
-    private bool lastFramePvP = false;
+    private bool lastFramePvP;
 
     [ServiceManager.ServiceConstructor]
     private ClientState(SigScanner sigScanner, DalamudStartInfo startInfo, GameLifecycle lifecycle)
@@ -63,22 +64,22 @@ internal sealed class ClientState : IDisposable, IServiceType, IClientState
     private delegate IntPtr SetupTerritoryTypeDelegate(IntPtr manager, ushort terriType);
 
     /// <inheritdoc/>
-    public event EventHandler<ushort> TerritoryChanged;
+    public event Action<ushort>? TerritoryChanged;
 
     /// <inheritdoc/>
-    public event EventHandler Login;
+    public event Action? Login;
 
     /// <inheritdoc/>
-    public event EventHandler Logout;
+    public event Action? Logout;
 
     /// <inheritdoc/>
-    public event Action EnterPvP;
+    public event Action? EnterPvP;
 
     /// <inheritdoc/>
-    public event Action LeavePvP;
+    public event Action? LeavePvP;
 
     /// <inheritdoc/>
-    public event EventHandler<Lumina.Excel.GeneratedSheets.ContentFinderCondition> CfPop;
+    public event Action<ContentFinderCondition>? CfPop;
 
     /// <inheritdoc/>
     public ClientLanguage ClientLanguage { get; }
@@ -128,16 +129,16 @@ internal sealed class ClientState : IDisposable, IServiceType, IClientState
     private IntPtr SetupTerritoryTypeDetour(IntPtr manager, ushort terriType)
     {
         this.TerritoryType = terriType;
-        this.TerritoryChanged?.InvokeSafely(this, terriType);
+        this.TerritoryChanged?.InvokeSafely(terriType);
 
         Log.Debug("TerritoryType changed: {0}", terriType);
 
         return this.setupTerritoryTypeHook.Original(manager, terriType);
     }
 
-    private void NetworkHandlersOnCfPop(object sender, Lumina.Excel.GeneratedSheets.ContentFinderCondition e)
+    private void NetworkHandlersOnCfPop(ContentFinderCondition e)
     {
-        this.CfPop?.InvokeSafely(this, e);
+        this.CfPop?.InvokeSafely(e);
     }
 
     private void FrameworkOnOnUpdateEvent(IFramework framework1)
@@ -149,12 +150,12 @@ internal sealed class ClientState : IDisposable, IServiceType, IClientState
         if (condition == null || gameGui == null || data == null)
             return;
 
-        if (condition.Any() && this.lastConditionNone == true && this.LocalPlayer != null)
+        if (condition.Any() && this.lastConditionNone && this.LocalPlayer != null)
         {
             Log.Debug("Is login");
             this.lastConditionNone = false;
             this.IsLoggedIn = true;
-            this.Login?.InvokeSafely(this, null);
+            this.Login?.InvokeSafely();
             gameGui.ResetUiHideState();
 
             this.lifecycle.ResetLogout();
@@ -165,7 +166,7 @@ internal sealed class ClientState : IDisposable, IServiceType, IClientState
             Log.Debug("Is logout");
             this.lastConditionNone = true;
             this.IsLoggedIn = false;
-            this.Logout?.InvokeSafely(this, null);
+            this.Logout?.InvokeSafely();
             gameGui.ResetUiHideState();
 
             this.lifecycle.SetLogout();
@@ -189,3 +190,103 @@ internal sealed class ClientState : IDisposable, IServiceType, IClientState
         }
     }
 }
+
+/// <summary>
+/// Plugin-scoped version of a GameConfig service.
+/// </summary>
+[PluginInterface]
+[InterfaceVersion("1.0")]
+[ServiceManager.ScopedService]
+#pragma warning disable SA1015
+[ResolveVia<IClientState>]
+#pragma warning restore SA1015
+internal class ClientStatePluginScoped : IDisposable, IServiceType, IClientState
+{
+    [ServiceManager.ServiceDependency]
+    private readonly ClientState clientStateService = Service<ClientState>.Get();
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="ClientStatePluginScoped"/> class.
+    /// </summary>
+    internal ClientStatePluginScoped()
+    {
+        this.clientStateService.TerritoryChanged += this.TerritoryChangedForward;
+        this.clientStateService.Login += this.LoginForward;
+        this.clientStateService.Logout += this.LogoutForward;
+        this.clientStateService.EnterPvP += this.EnterPvPForward;
+        this.clientStateService.LeavePvP += this.ExitPvPForward;
+        this.clientStateService.CfPop += this.ContentFinderPopForward;
+    }
+    
+    /// <inheritdoc/>
+    public event Action<ushort>? TerritoryChanged;
+    
+    /// <inheritdoc/>
+    public event Action? Login;
+    
+    /// <inheritdoc/>
+    public event Action? Logout;
+    
+    /// <inheritdoc/>
+    public event Action? EnterPvP;
+    
+    /// <inheritdoc/>
+    public event Action? LeavePvP;
+    
+    /// <inheritdoc/>
+    public event Action<ContentFinderCondition>? CfPop;
+
+    /// <inheritdoc/>
+    public ClientLanguage ClientLanguage => this.clientStateService.ClientLanguage;
+
+    /// <inheritdoc/>
+    public ushort TerritoryType => this.clientStateService.TerritoryType;
+
+    /// <inheritdoc/>
+    public PlayerCharacter? LocalPlayer => this.clientStateService.LocalPlayer;
+
+    /// <inheritdoc/>
+    public ulong LocalContentId => this.clientStateService.LocalContentId;
+
+    /// <inheritdoc/>
+    public bool IsLoggedIn => this.clientStateService.IsLoggedIn;
+
+    /// <inheritdoc/>
+    public bool IsPvP => this.clientStateService.IsPvP;
+
+    /// <inheritdoc/>
+    public bool IsPvPExcludingDen => this.clientStateService.IsPvPExcludingDen;
+
+    /// <inheritdoc/>
+    public bool IsGPosing => this.clientStateService.IsGPosing;
+
+    /// <inheritdoc/>
+    public void Dispose()
+    {
+        this.clientStateService.TerritoryChanged -= this.TerritoryChangedForward;
+        this.clientStateService.Login -= this.LoginForward;
+        this.clientStateService.Logout -= this.LogoutForward;
+        this.clientStateService.EnterPvP -= this.EnterPvPForward;
+        this.clientStateService.LeavePvP -= this.ExitPvPForward;
+        this.clientStateService.CfPop -= this.ContentFinderPopForward;
+
+        this.TerritoryChanged = null;
+        this.Login = null;
+        this.Logout = null;
+        this.EnterPvP = null;
+        this.LeavePvP = null;
+        this.CfPop = null;
+    }
+
+    private void TerritoryChangedForward(ushort territoryId) => this.TerritoryChanged?.Invoke(territoryId);
+    
+    private void LoginForward() => this.Login?.Invoke();
+    
+    private void LogoutForward() => this.Logout?.Invoke();
+    
+    private void EnterPvPForward() => this.EnterPvP?.Invoke();
+    
+    private void ExitPvPForward() => this.LeavePvP?.Invoke();
+
+    private void ContentFinderPopForward(ContentFinderCondition cfc) => this.CfPop?.Invoke(cfc);
+}

Dalamud/Game/DutyState/DutyState.cs

@@ -120,7 +120,7 @@ internal unsafe class DutyState : IDisposable, IServiceType, IDutyState
         return this.contentDirectorNetworkMessageHook.Original(a1, a2, a3);
     }
 
-    private void TerritoryOnChangedEvent(object? sender, ushort e)
+    private void TerritoryOnChangedEvent(ushort territoryId)
     {
         if (this.IsDutyStarted)
         {

Dalamud/Game/Network/Internal/NetworkHandlers.cs

@@ -44,7 +44,7 @@ internal class NetworkHandlers : IDisposable, IServiceType
     private NetworkHandlers(GameNetwork gameNetwork)
     {
         this.uploader = new UniversalisMarketBoardUploader();
-        this.CfPop = (_, _) => { };
+        this.CfPop = _ => { };
 
         this.messages = Observable.Create<NetworkMessage>(observer =>
         {
@@ -75,7 +75,7 @@ internal class NetworkHandlers : IDisposable, IServiceType
     /// <summary>
     /// Event which gets fired when a duty is ready.
     /// </summary>
-    public event EventHandler<ContentFinderCondition> CfPop;
+    public event Action<ContentFinderCondition> CfPop;
 
     /// <summary>
     /// Disposes of managed and unmanaged resources.
@@ -430,7 +430,7 @@ internal class NetworkHandlers : IDisposable, IServiceType
                                    Service<ChatGui>.GetNullable()?.Print($"Duty pop: {cfcName}");
                                }
 
-                               this.CfPop.InvokeSafely(this, cfCondition);
+                               this.CfPop.InvokeSafely(cfCondition);
                            }).ContinueWith(
                                task => Log.Error(task.Exception, "CfPop.Invoke failed"),
                                TaskContinuationOptions.OnlyOnFaulted);

Dalamud/Interface/Internal/Windows/SelfTest/AgingSteps/EnterTerritoryAgingStep.cs

@@ -59,9 +59,9 @@ internal class EnterTerritoryAgingStep : IAgingStep
         this.subscribed = false;
     }
 
-    private void ClientStateOnTerritoryChanged(object sender, ushort e)
+    private void ClientStateOnTerritoryChanged(ushort territoryId)
     {
-        if (e == this.territory)
+        if (territoryId == this.territory)
         {
             this.hasPassed = true;
         }

Dalamud/Interface/Internal/Windows/SelfTest/AgingSteps/LoginEventAgingStep.cs

@@ -51,7 +51,7 @@ internal class LoginEventAgingStep : IAgingStep
         }
     }
 
-    private void ClientStateOnOnLogin(object sender, EventArgs e)
+    private void ClientStateOnOnLogin()
     {
         this.hasPassed = true;
     }

Dalamud/Interface/Internal/Windows/SelfTest/AgingSteps/LogoutEventAgingStep.cs

@@ -51,7 +51,7 @@ internal class LogoutEventAgingStep : IAgingStep
         }
     }
 
-    private void ClientStateOnOnLogout(object sender, EventArgs e)
+    private void ClientStateOnOnLogout()
     {
         this.hasPassed = true;
     }

Dalamud/Plugin/Services/IClientState.cs

@@ -12,17 +12,17 @@ public interface IClientState
     /// <summary>
     /// Event that gets fired when the current Territory changes.
     /// </summary>
-    public event EventHandler<ushort> TerritoryChanged;
+    public event Action<ushort> TerritoryChanged;
 
     /// <summary>
     /// Event that fires when a character is logging in, and the local character object is available.
     /// </summary>
-    public event EventHandler Login;
+    public event Action Login;
 
     /// <summary>
     /// Event that fires when a character is logging out.
     /// </summary>
-    public event EventHandler Logout;
+    public event Action Logout;
 
     /// <summary>
     /// Event that fires when a character is entering PvP.
@@ -37,7 +37,7 @@ public interface IClientState
     /// <summary>
     /// Event that gets fired when a duty is ready.
     /// </summary>
-    public event EventHandler<Lumina.Excel.GeneratedSheets.ContentFinderCondition> CfPop;
+    public event Action<Lumina.Excel.GeneratedSheets.ContentFinderCondition> CfPop;
 
     /// <summary>
     /// Gets the language of the client.

Dalamud/Utility/EventHandlerExtensions.cs

@@ -1,12 +1,9 @@
-using System;
 using System.Linq;
 
 using Dalamud.Game;
 using Dalamud.Plugin.Services;
 using Serilog;
 
-using static Dalamud.Game.Framework;
-
 namespace Dalamud.Utility;
 
 /// <summary>
@@ -21,7 +18,7 @@ internal static class EventHandlerExtensions
     /// <param name="eh">The EventHandler in question.</param>
     /// <param name="sender">Default sender for Invoke equivalent.</param>
     /// <param name="e">Default EventArgs for Invoke equivalent.</param>
-    public static void InvokeSafely(this EventHandler eh, object sender, EventArgs e)
+    public static void InvokeSafely(this EventHandler? eh, object sender, EventArgs e)
     {
         if (eh == null)
             return;
@@ -40,7 +37,7 @@ internal static class EventHandlerExtensions
     /// <param name="sender">Default sender for Invoke equivalent.</param>
     /// <param name="e">Default EventArgs for Invoke equivalent.</param>
     /// <typeparam name="T">Type of EventArgs.</typeparam>
-    public static void InvokeSafely<T>(this EventHandler<T> eh, object sender, T e)
+    public static void InvokeSafely<T>(this EventHandler<T>? eh, object sender, T e)
     {
         if (eh == null)
             return;
@@ -56,7 +53,7 @@ internal static class EventHandlerExtensions
     /// of a thrown Exception inside of an invocation.
     /// </summary>
     /// <param name="act">The Action in question.</param>
-    public static void InvokeSafely(this Action act)
+    public static void InvokeSafely(this Action? act)
     {
         if (act == null)
             return;
@@ -67,13 +64,31 @@ internal static class EventHandlerExtensions
         }
     }
 
+    /// <summary>
+    /// Replacement for Invoke() on event Actions to catch exceptions that stop event propagation in case
+    /// of a thrown Exception inside of an invocation.
+    /// </summary>
+    /// <param name="act">The Action in question.</param>
+    /// <param name="argument">Templated argument for Action.</param>
+    /// <typeparam name="T">Type of Action args.</typeparam>
+    public static void InvokeSafely<T>(this Action<T>? act, T argument)
+    {
+        if (act == null)
+            return;
+
+        foreach (var action in act.GetInvocationList().Cast<Action<T>>())
+        {
+            HandleInvoke(action, argument);
+        }
+    }
+
     /// <summary>
     /// Replacement for Invoke() on OnUpdateDelegate to catch exceptions that stop event propagation in case
     /// of a thrown Exception inside of an invocation.
     /// </summary>
     /// <param name="updateDelegate">The OnUpdateDelegate in question.</param>
     /// <param name="framework">Framework to be passed on to OnUpdateDelegate.</param>
-    public static void InvokeSafely(this IFramework.OnUpdateDelegate updateDelegate, Framework framework)
+    public static void InvokeSafely(this IFramework.OnUpdateDelegate? updateDelegate, Framework framework)
     {
         if (updateDelegate == null)
             return;
@@ -95,4 +110,16 @@ internal static class EventHandlerExtensions
             Log.Error(ex, "Exception during raise of {handler}", act.Method);
         }
     }
+
+    private static void HandleInvoke<T>(Action<T> act, T argument)
+    {
+        try
+        {
+            act(argument);
+        }
+        catch (Exception ex)
+        {
+            Log.Error(ex, "Exception during raise of {handler}", act.Method);
+        }
+    }
 }