Use better event in SkinFixer and some cleanup.

Penumbra/Api/PenumbraApi.cs

@@ -85,26 +85,7 @@ public class PenumbraApi : IDisposable, IPenumbraApi
         }
     }
 
-    public event CreatedCharacterBaseDelegate? CreatedCharacterBase
+    public event CreatedCharacterBaseDelegate? CreatedCharacterBase;
-    {
-        add
-        {
-            if (value == null)
-                return;
-
-            CheckInitialized();
-            _communicator.CreatedCharacterBase.Subscribe(new Action<nint, string, nint>(value),
-                Communication.CreatedCharacterBase.Priority.Api);
-        }
-        remove
-        {
-            if (value == null)
-                return;
-
-            CheckInitialized();
-            _communicator.CreatedCharacterBase.Unsubscribe(new Action<nint, string, nint>(value));
-        }
-    }
 
     public bool Valid
         => _lumina != null;
@@ -157,6 +138,7 @@ public class PenumbraApi : IDisposable, IPenumbraApi
         _resourceLoader.ResourceLoaded += OnResourceLoaded;
         _communicator.ModPathChanged.Subscribe(ModPathChangeSubscriber, ModPathChanged.Priority.Api);
         _communicator.ModSettingChanged.Subscribe(OnModSettingChange, Communication.ModSettingChanged.Priority.Api);
+        _communicator.CreatedCharacterBase.Subscribe(OnCreatedCharacterBase, Communication.CreatedCharacterBase.Priority.Api);
     }
 
     public unsafe void Dispose()
@@ -167,6 +149,7 @@ public class PenumbraApi : IDisposable, IPenumbraApi
         _resourceLoader.ResourceLoaded -= OnResourceLoaded;
         _communicator.ModPathChanged.Unsubscribe(ModPathChangeSubscriber);
         _communicator.ModSettingChanged.Unsubscribe(OnModSettingChange);
+        _communicator.CreatedCharacterBase.Unsubscribe(OnCreatedCharacterBase);
         _lumina             = null;
         _communicator       = null!;
         _modManager         = null!;
@@ -1189,4 +1172,7 @@ public class PenumbraApi : IDisposable, IPenumbraApi
 
     private void OnModSettingChange(ModCollection collection, ModSettingChange type, Mod? mod, int _1, int _2, bool inherited)
         => ModSettingChanged?.Invoke(type, collection.Name, mod?.ModPath.Name ?? string.Empty, inherited);
+
+    private void OnCreatedCharacterBase(nint gameObject, ModCollection collection, nint drawObject)
+        => CreatedCharacterBase?.Invoke(gameObject, collection.Name, drawObject);
 }

Penumbra/Communication/CreatedCharacterBase.cs

@@ -1,26 +1,30 @@
 using System;
 using OtterGui.Classes;
 using Penumbra.Api;
+using Penumbra.Collections;
 
 namespace Penumbra.Communication;
 
 /// <summary> <list type="number">
 ///     <item>Parameter is the game object for which a draw object is created. </item>
-///     <item>Parameter is the name of the applied collection. </item>
+///     <item>Parameter is the applied collection. </item>
 ///     <item>Parameter is the created draw object. </item>
 /// </list> </summary>
-public sealed class CreatedCharacterBase : EventWrapper<Action<nint, string, nint>, CreatedCharacterBase.Priority>
+public sealed class CreatedCharacterBase : EventWrapper<Action<nint, ModCollection, nint>, CreatedCharacterBase.Priority>
 {
     public enum Priority
     {
         /// <seealso cref="PenumbraApi.CreatedCharacterBase"/>
-        Api = 0,
+        Api = int.MinValue,
+
+        /// <seealso cref="Interop.Services.SkinFixer.OnCharacterBaseCreated"/>
+        SkinFixer = 0,
     }
 
     public CreatedCharacterBase()
         : base(nameof(CreatedCharacterBase))
     { }
 
-    public void Invoke(nint gameObject, string appliedCollectionName, nint drawObject)
+    public void Invoke(nint gameObject, ModCollection appliedCollection, nint drawObject)
-        => Invoke(this, gameObject, appliedCollectionName, drawObject);
+        => Invoke(this, gameObject, appliedCollection, drawObject);
 }

Penumbra/Interop/PathResolving/MetaState.cs

@@ -142,7 +142,7 @@ public unsafe class MetaState : IDisposable
         _characterBaseCreateMetaChanges = DisposableContainer.Empty;
         if (_lastCreatedCollection.Valid && _lastCreatedCollection.AssociatedGameObject != nint.Zero)
             _communicator.CreatedCharacterBase.Invoke(_lastCreatedCollection.AssociatedGameObject,
-                _lastCreatedCollection.ModCollection.Name, drawObject);
+                _lastCreatedCollection.ModCollection, drawObject);
         _lastCreatedCollection = ResolveData.Invalid;
     }
 

Penumbra/Interop/Services/SkinFixer.cs

@@ -10,16 +10,18 @@ using FFXIVClientStructs.FFXIV.Client.Graphics.Render;
 using FFXIVClientStructs.FFXIV.Client.Graphics.Scene;
 using FFXIVClientStructs.FFXIV.Client.System.Resource;
 using FFXIVClientStructs.FFXIV.Client.System.Resource.Handle;
+using Penumbra.Collections;
+using Penumbra.Communication;
 using Penumbra.GameData;
 using Penumbra.GameData.Enums;
-using Penumbra.Interop.PathResolving;
 using Penumbra.Interop.ResourceLoading;
 using Penumbra.Interop.SafeHandles;
+using Penumbra.Services;
 using Penumbra.String.Classes;
 
 namespace Penumbra.Interop.Services;
 
-public unsafe class SkinFixer : IDisposable
+public sealed unsafe class SkinFixer : IDisposable
 {
     public static readonly Utf8GamePath SkinShpkPath =
         Utf8GamePath.FromSpan("shader/sm5/shpk/skin.shpk"u8, out var p) ? p : Utf8GamePath.Empty;
@@ -34,60 +36,48 @@ public unsafe class SkinFixer : IDisposable
     {
         [FieldOffset(0x0)]
         public Model* Model;
+
         [FieldOffset(0x8)]
         public uint MaterialIndex;
     }
 
     private readonly Hook<OnRenderMaterialDelegate> _onRenderMaterialHook;
 
-    private readonly CollectionResolver _collectionResolver;
+    private readonly GameEventManager    _gameEvents;
-    private readonly GameEventManager   _gameEvents;
+    private readonly CommunicatorService _communicator;
-    private readonly ResourceLoader     _resources;
+    private readonly ResourceLoader      _resources;
-    private readonly CharacterUtility   _utility;
+    private readonly CharacterUtility    _utility;
-
+
-    private readonly ConcurrentDictionary<nint /* CharacterBase* */, SafeResourceHandle> _skinShpks = new();
+    // CharacterBase to ShpkHandle
+    private readonly ConcurrentDictionary<nint, SafeResourceHandle> _skinShpks = new();
 
     private readonly object _lock = new();
-    
+
-    private bool  _enabled = true;
     private int   _moddedSkinShpkCount = 0;
-    private ulong _slowPathCallDelta = 0;
+    private ulong _slowPathCallDelta   = 0;
-    public bool Enabled
+
-    {
+    public bool Enabled { get; internal set; } = true;
-        get => _enabled;
-        set => _enabled = value;
-    }
 
     public int ModdedSkinShpkCount
         => _moddedSkinShpkCount;
 
-    public SkinFixer(CollectionResolver collectionResolver, GameEventManager gameEvents, ResourceLoader resources, CharacterUtility utility, DrawObjectState _)
+    public SkinFixer(GameEventManager gameEvents, ResourceLoader resources, CharacterUtility utility, CommunicatorService communicator)
     {
         SignatureHelper.Initialise(this);
-        _collectionResolver   = collectionResolver;
         _gameEvents           = gameEvents;
         _resources            = resources;
         _utility              = utility;
+        _communicator    = communicator;
         _onRenderMaterialHook = Hook<OnRenderMaterialDelegate>.FromAddress(_humanVTable[62], OnRenderHumanMaterial);
-        _gameEvents.CharacterBaseCreated    += OnCharacterBaseCreated; // The dependency on DrawObjectState shall ensure that this handler is registered after its one.
+        _communicator.CreatedCharacterBase.Subscribe(OnCharacterBaseCreated, CreatedCharacterBase.Priority.SkinFixer);
         _gameEvents.CharacterBaseDestructor += OnCharacterBaseDestructor;
         _onRenderMaterialHook.Enable();
     }
 
-    ~SkinFixer()
-    {
-        Dispose(false);
-    }
-
     public void Dispose()
-    {
-        Dispose(true);
-        GC.SuppressFinalize(this);
-    }
-    protected virtual void Dispose(bool disposing)
     {
         _onRenderMaterialHook.Dispose();
-        _gameEvents.CharacterBaseCreated    -= OnCharacterBaseCreated;
+        _communicator.CreatedCharacterBase.Unsubscribe(OnCharacterBaseCreated);
         _gameEvents.CharacterBaseDestructor -= OnCharacterBaseDestructor;
         foreach (var skinShpk in _skinShpks.Values)
             skinShpk.Dispose();
@@ -98,21 +88,21 @@ public unsafe class SkinFixer : IDisposable
     public ulong GetAndResetSlowPathCallDelta()
         => Interlocked.Exchange(ref _slowPathCallDelta, 0);
 
-    private void OnCharacterBaseCreated(uint modelCharaId, nint customize, nint equipment, nint drawObject)
+    private void OnCharacterBaseCreated(nint gameObject, ModCollection collection, nint drawObject)
     {
         if (((CharacterBase*)drawObject)->GetModelType() != CharacterBase.ModelType.Human)
             return;
 
-        Task.Run(delegate
+        Task.Run(() =>
         {
             var skinShpk = SafeResourceHandle.CreateInvalid();
             try
             {
-                var data = _collectionResolver.IdentifyCollection((DrawObject*)drawObject, true);
+                var data = collection.ToResolveData(gameObject);
                 if (data.Valid)
                 {
                     var loadedShpk = _resources.LoadResolvedResource(ResourceCategory.Shader, ResourceType.Shpk, SkinShpkPath.Path, data);
-                    skinShpk = new SafeResourceHandle((ResourceHandle*)loadedShpk, incRef: false);
+                    skinShpk = new SafeResourceHandle((ResourceHandle*)loadedShpk, false);
                 }
             }
             catch (Exception e)
@@ -128,30 +118,31 @@ public unsafe class SkinFixer : IDisposable
                         Interlocked.Increment(ref _moddedSkinShpkCount);
                 }
                 else
+                {
                     skinShpk.Dispose();
+                }
             }
         });
     }
 
     private void OnCharacterBaseDestructor(nint characterBase)
     {
-        if (_skinShpks.Remove(characterBase, out var skinShpk))
+        if (!_skinShpks.Remove(characterBase, out var skinShpk))
-        {
+            return;
-            var handle = skinShpk.ResourceHandle;
+
-            skinShpk.Dispose();
+        var handle = skinShpk.ResourceHandle;
-            if ((nint)handle != _utility.DefaultSkinShpkResource)
+        skinShpk.Dispose();
-                Interlocked.Decrement(ref _moddedSkinShpkCount);
+        if ((nint)handle != _utility.DefaultSkinShpkResource)
-        }
+            Interlocked.Decrement(ref _moddedSkinShpkCount);
     }
 
     private nint OnRenderHumanMaterial(nint human, OnRenderMaterialParams* param)
     {
-        if (!_enabled ||                 // Can be toggled on the debug tab.
+        // If we don't have any on-screen instances of modded skin.shpk, we don't need the slow path at all.
-            _moddedSkinShpkCount == 0 || // If we don't have any on-screen instances of modded skin.shpk, we don't need the slow path at all.
+        if (!Enabled || _moddedSkinShpkCount == 0 || !_skinShpks.TryGetValue(human, out var skinShpk) || skinShpk.IsInvalid)
-            !_skinShpks.TryGetValue(human, out var skinShpk))
             return _onRenderMaterialHook!.Original(human, param);
 
-        var material = param->Model->Materials[param->MaterialIndex];
+        var material     = param->Model->Materials[param->MaterialIndex];
         var shpkResource = ((Structs.MtrlResource*)material->MaterialResourceHandle)->ShpkResourceHandle;
         if ((nint)shpkResource != (nint)skinShpk.ResourceHandle)
             return _onRenderMaterialHook!.Original(human, param);
@@ -164,6 +155,7 @@ public unsafe class SkinFixer : IDisposable
         // - Swapping path is taken up to hundreds of times a frame.
         // At the time of writing, the lock doesn't seem to have a noticeable impact in either framerate or CPU usage, but the swapping path shall still be avoided as much as possible.
         lock (_lock)
+        {
             try
             {
                 _utility.Address->SkinShpkResource = (Structs.ResourceHandle*)skinShpk.ResourceHandle;
@@ -173,5 +165,6 @@ public unsafe class SkinFixer : IDisposable
             {
                 _utility.Address->SkinShpkResource = (Structs.ResourceHandle*)_utility.DefaultSkinShpkResource;
             }
+        }
     }
 }

Penumbra/UI/Tabs/DebugTab.cs

@@ -36,7 +36,6 @@ using ObjectKind = Dalamud.Game.ClientState.Objects.Enums.ObjectKind;
 using ResidentResourceManager = Penumbra.Interop.Services.ResidentResourceManager;
 using Penumbra.Interop.Services;
 using FFXIVClientStructs.FFXIV.Client.Graphics.Kernel;
-using static Lumina.Data.Parsing.Layer.LayerCommon;
 
 namespace Penumbra.UI.Tabs;
 
@@ -623,18 +622,14 @@ public class DebugTab : Window, ITab
             ImGui.TableNextColumn();
             if (resource == null)
             {
-                ImGui.TableNextColumn();
+                ImGui.TableNextRow();
-                ImGui.TableNextColumn();
-                ImGui.TableNextColumn();
-                ImGui.TableNextColumn();
                 continue;
             }
             UiHelpers.Text(resource);
             ImGui.TableNextColumn();
             var data = (nint)ResourceHandle.GetData(resource);
             var length = ResourceHandle.GetLength(resource);
-            ImGui.Selectable($"0x{data:X}");
+            if (ImGui.Selectable($"0x{data:X}"))
-            if (ImGui.IsItemClicked())
             {
                 if (data != nint.Zero && length > 0)
                     ImGui.SetClipboardText(string.Join("\n",
@@ -643,7 +638,7 @@ public class DebugTab : Window, ITab
 
             ImGuiUtil.HoverTooltip("Click to copy bytes to clipboard.");
             ImGui.TableNextColumn();
-            ImGui.TextUnformatted($"{length}");
+            ImGui.TextUnformatted(length.ToString());
 
             ImGui.TableNextColumn();
             if (intern.Value != -1)