Remove unnecessary stuff from ObjectTable (#1713)

* Remove unnecessary stuff from ObjectTable

* Remove unused

* fix

Dalamud/Game/ClientState/Objects/ObjectTable.cs

@@ -15,6 +15,8 @@ using Microsoft.Extensions.ObjectPool;
 
 using Serilog;
 
+using CSGameObject = FFXIVClientStructs.FFXIV.Client.Game.Object.GameObject;
+
 namespace Dalamud.Game.ClientState.Objects;
 
 /// <summary>
@@ -36,16 +38,19 @@ internal sealed partial class ObjectTable : IServiceType, IObjectTable
     private readonly ObjectPool<Enumerator> multiThreadedEnumerators =
         new DefaultObjectPoolProvider().Create<Enumerator>();
 
-    private readonly Enumerator?[] frameworkThreadEnumerators = new Enumerator?[64];
+    private readonly Enumerator?[] frameworkThreadEnumerators = new Enumerator?[4];
 
     private long nextMultithreadedUsageWarnTime;
 
     [ServiceManager.ServiceConstructor]
-    private ObjectTable(ClientState clientState)
+    private unsafe ObjectTable(ClientState clientState)
     {
         this.clientState = clientState;
-        foreach (ref var e in this.cachedObjectTable.AsSpan())
+
-            e = CachedEntry.CreateNew();
+        var nativeObjectTableAddress = (CSGameObject**)this.clientState.AddressResolver.ObjectTable;
+        for (var i = 0; i < this.cachedObjectTable.Length; i++)
+            this.cachedObjectTable[i] = new(nativeObjectTableAddress, i);
+
         for (var i = 0; i < this.frameworkThreadEnumerators.Length; i++)
             this.frameworkThreadEnumerators[i] = new(this, i);
 
@@ -73,9 +78,7 @@ internal sealed partial class ObjectTable : IServiceType, IObjectTable
         {
             _ = this.WarnMultithreadedUsage();
 
-            if (index is >= ObjectTableLength or < 0) return null;
+            return index is >= ObjectTableLength or < 0 ? null : this.cachedObjectTable[index].Update();
-            this.cachedObjectTable[index].Update(this.GetObjectAddressUnsafe(index));
-            return this.cachedObjectTable[index].ActiveObject;
         }
     }
 
@@ -87,24 +90,21 @@ internal sealed partial class ObjectTable : IServiceType, IObjectTable
         if (objectId is GameObject.InvalidGameObjectId or 0)
             return null;
 
-        foreach (var obj in this)
+        foreach (var e in this.cachedObjectTable)
         {
-            if (obj == null)
+            if (e.Update() is { } o && o.ObjectId == objectId)
-                continue;
+                return o;
-
-            if (obj.ObjectId == objectId)
-                return obj;
         }
 
         return null;
     }
 
     /// <inheritdoc/>
-    public nint GetObjectAddress(int index)
+    public unsafe nint GetObjectAddress(int index)
     {
         _ = this.WarnMultithreadedUsage();
 
-        return index is < 0 or >= ObjectTableLength ? nint.Zero : this.GetObjectAddressUnsafe(index);
+        return index is < 0 or >= ObjectTableLength ? nint.Zero : (nint)this.cachedObjectTable[index].Address;
     }
 
     /// <inheritdoc/>
@@ -118,7 +118,7 @@ internal sealed partial class ObjectTable : IServiceType, IObjectTable
         if (address == nint.Zero)
             return null;
 
-        var obj = (FFXIVClientStructs.FFXIV.Client.Game.Object.GameObject*)address;
+        var obj = (CSGameObject*)address;
         var objKind = (ObjectKind)obj->ObjectKind;
         return objKind switch
         {
@@ -134,6 +134,7 @@ internal sealed partial class ObjectTable : IServiceType, IObjectTable
         };
     }
 
+    [Api10ToDo("Use ThreadSafety.AssertMainThread() instead of this.")]
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     private bool WarnMultithreadedUsage()
     {
@@ -154,56 +155,58 @@ internal sealed partial class ObjectTable : IServiceType, IObjectTable
         return true;
     }
 
-    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    /// <summary>Stores an object table entry, with preallocated concrete types.</summary>
-    private unsafe nint GetObjectAddressUnsafe(int index) =>
+    internal readonly unsafe struct CachedEntry
-        *(nint*)(this.clientState.AddressResolver.ObjectTable + (8 * index));
-
-    private struct CachedEntry
     {
-        public GameObject? ActiveObject;
+        private readonly CSGameObject** gameObjectPtrPtr;
-        public PlayerCharacter PlayerCharacter;
+        private readonly PlayerCharacter playerCharacter;
-        public BattleNpc BattleNpc;
+        private readonly BattleNpc battleNpc;
-        public Npc Npc;
+        private readonly Npc npc;
-        public EventObj EventObj;
+        private readonly EventObj eventObj;
-        public GameObject GameObject;
+        private readonly GameObject gameObject;
 
-        public static CachedEntry CreateNew() =>
+        /// <summary>Initializes a new instance of the <see cref="CachedEntry"/> struct.</summary>
-            new()
+        /// <param name="ownerTable">The object table that this entry should be pointing to.</param>
-            {
+        /// <param name="slot">The slot index inside the table.</param>
-                PlayerCharacter = new(nint.Zero),
+        public CachedEntry(CSGameObject** ownerTable, int slot)
-                BattleNpc = new(nint.Zero),
-                Npc = new(nint.Zero),
-                EventObj = new(nint.Zero),
-                GameObject = new(nint.Zero),
-            };
-
-        public unsafe void Update(nint address)
         {
-            if (this.ActiveObject != null && address == this.ActiveObject.Address)
+            this.gameObjectPtrPtr = ownerTable + slot;
-                return;
+            this.playerCharacter = new(nint.Zero);
+            this.battleNpc = new(nint.Zero);
+            this.npc = new(nint.Zero);
+            this.eventObj = new(nint.Zero);
+            this.gameObject = new(nint.Zero);
+        }
 
-            if (address == nint.Zero)
+        /// <summary>Gets the address of the underlying native object. May be null.</summary>
-            {
+        public CSGameObject* Address
-                this.ActiveObject = null;
+        {
-                return;
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            }
+            get => *this.gameObjectPtrPtr;
+        }
 
-            var obj = (FFXIVClientStructs.FFXIV.Client.Game.Object.GameObject*)address;
+        /// <summary>Updates and gets the wrapped game object pointed by this struct.</summary>
-            var objKind = (ObjectKind)obj->ObjectKind;
+        /// <returns>The pointed object, or <c>null</c> if no object exists at that slot.</returns>
-            var activeObject = objKind switch
+        public GameObject? Update()
+        {
+            var address = this.Address;
+            if (address is null)
+                return null;
+
+            var activeObject = (ObjectKind)address->ObjectKind switch
             {
-                ObjectKind.Player => this.PlayerCharacter,
+                ObjectKind.Player => this.playerCharacter,
-                ObjectKind.BattleNpc => this.BattleNpc,
+                ObjectKind.BattleNpc => this.battleNpc,
-                ObjectKind.EventNpc => this.Npc,
+                ObjectKind.EventNpc => this.npc,
-                ObjectKind.Retainer => this.Npc,
+                ObjectKind.Retainer => this.npc,
-                ObjectKind.EventObj => this.EventObj,
+                ObjectKind.EventObj => this.eventObj,
-                ObjectKind.Companion => this.Npc,
+                ObjectKind.Companion => this.npc,
-                ObjectKind.MountType => this.Npc,
+                ObjectKind.MountType => this.npc,
-                ObjectKind.Ornament => this.Npc,
+                ObjectKind.Ornament => this.npc,
-                _ => this.GameObject,
+                _ => this.gameObject,
             };
-            activeObject.Address = address;
+            activeObject.Address = (nint)address;
-            this.ActiveObject = activeObject;
+            return activeObject;
         }
     }
 }
@@ -219,6 +222,7 @@ internal sealed partial class ObjectTable
     /// <inheritdoc/>
     public IEnumerator<GameObject> GetEnumerator()
     {
+        // If something's trying to enumerate outside the framework thread, we use the ObjectPool.
         if (this.WarnMultithreadedUsage())
         {
             // let's not
@@ -227,6 +231,7 @@ internal sealed partial class ObjectTable
             return e;
         }
 
+        // If we're on the framework thread, see if there's an already allocated enumerator available for use.
         foreach (ref var x in this.frameworkThreadEnumerators.AsSpan())
         {
             if (x is not null)
@@ -238,6 +243,7 @@ internal sealed partial class ObjectTable
             }
         }
 
+        // No reusable enumerator is available; allocate a new temporary one.
         return new Enumerator(this, -1);
     }
 
@@ -271,9 +277,7 @@ internal sealed partial class ObjectTable
             var cache = this.owner!.cachedObjectTable.AsSpan();
             for (this.index++; this.index < ObjectTableLength; this.index++)
             {
-                this.owner!.cachedObjectTable[this.index].Update(this.owner!.GetObjectAddressUnsafe(this.index));
+                if (cache[this.index].Update() is { } ao)
-                
-                if (cache[this.index].ActiveObject is { } ao)
                 {
                     this.Current = ao;
                     return true;
@@ -292,7 +296,7 @@ internal sealed partial class ObjectTable
             if (this.owner is not { } o)
                 return;
 
-            if (this.index == -1)
+            if (this.slotId == -1)
                 o.multiThreadedEnumerators.Return(this);
             else
                 o.frameworkThreadEnumerators[this.slotId] = this;

Dalamud/Game/Framework.cs

@@ -498,6 +498,9 @@ internal class FrameworkPluginScoped : IDisposable, IServiceType, IFramework
     /// <inheritdoc/>
     public DateTime LastUpdateUTC => this.frameworkService.LastUpdateUTC;
 
+    /// <inheritdoc/>
+    public TaskFactory FrameworkThreadTaskFactory => this.frameworkService.FrameworkThreadTaskFactory;
+
     /// <inheritdoc/>
     public TimeSpan UpdateDelta => this.frameworkService.UpdateDelta;
 

Dalamud/Plugin/Services/IObjectTable.cs

@@ -1,24 +1,27 @@
 using System.Collections.Generic;
 
 using Dalamud.Game.ClientState.Objects.Types;
+using Dalamud.Utility;
 
 namespace Dalamud.Plugin.Services;
 
 /// <summary>
 /// This collection represents the currently spawned FFXIV game objects.
 /// </summary>
+[Api10ToDo(
+    "Make it an IEnumerable<GameObject> instead. Skipping null objects make IReadOnlyCollection<T>.Count yield incorrect values.")]
 public interface IObjectTable : IReadOnlyCollection<GameObject>
 {
     /// <summary>
     /// Gets the address of the object table.
     /// </summary>
     public nint Address { get; }
-    
+
     /// <summary>
     /// Gets the length of the object table.
     /// </summary>
     public int Length { get; }
-    
+
     /// <summary>
     /// Get an object at the specified spawn index.
     /// </summary>
@@ -32,14 +35,14 @@ public interface IObjectTable : IReadOnlyCollection<GameObject>
     /// <param name="objectId">Object ID to find.</param>
     /// <returns>A game object or null.</returns>
     public GameObject? SearchById(ulong objectId);
-    
+
     /// <summary>
     /// Gets the address of the game object at the specified index of the object table.
     /// </summary>
     /// <param name="index">The index of the object.</param>
     /// <returns>The memory address of the object.</returns>
     public nint GetObjectAddress(int index);
-    
+
     /// <summary>
     /// Create a reference to an FFXIV game object.
     /// </summary>