refactor: replacing custom pool implementation with ObjectPool [MTT-6263]

Assets/Scripts/Infrastructure/NetworkObjectPool.cs

@@ -4,39 +4,37 @@
 using Unity.Netcode;
 using UnityEngine;
 using UnityEngine.Assertions;
+using UnityEngine.Pool;
 
 namespace Unity.BossRoom.Infrastructure
 {
     /// <summary>
-    /// Object Pool for networked objects, used for controlling how objects are spawned by Netcode. Netcode by default will allocate new memory when spawning new
-    /// objects. With this Networked Pool, we're using custom spawning to reuse objects.
+    /// Object Pool for networked objects, used for controlling how objects are spawned by Netcode. Netcode by default
+    /// will allocate new memory when spawning new objects. With this Networked Pool, we're using the ObjectPool to
+    /// reuse objects.
     /// Boss Room uses this for projectiles. In theory it should use this for imps too, but we wanted to show vanilla spawning vs pooled spawning.
-    /// Hooks to NetworkManager's prefab handler to intercept object spawning and do custom actions
+    /// Hooks to NetworkManager's prefab handler to intercept object spawning and do custom actions.
     /// </summary>
     public class NetworkObjectPool : NetworkBehaviour
     {
-        private static NetworkObjectPool _instance;
-
-        public static NetworkObjectPool Singleton { get { return _instance; } }
+        public static NetworkObjectPool Singleton { get; private set; }
 
         [SerializeField]
         List<PoolConfigObject> PooledPrefabsList;
 
-        HashSet<GameObject> prefabs = new HashSet<GameObject>();
-
-        Dictionary<GameObject, Queue<NetworkObject>> pooledObjects = new Dictionary<GameObject, Queue<NetworkObject>>();
+        HashSet<GameObject> m_Prefabs = new HashSet<GameObject>();
 
-        private bool m_HasInitialized = false;
+        Dictionary<GameObject, ObjectPool<NetworkObject>> m_PooledObjects = new Dictionary<GameObject, ObjectPool<NetworkObject>>();
 
         public void Awake()
         {
-            if (_instance != null && _instance != this)
+            if (Singleton != null && Singleton != this)
             {
-                Destroy(this.gameObject);
+                Destroy(gameObject);
             }
             else
             {
-                _instance = this;
+                Singleton = this;
             }
         }
 
@@ -89,9 +87,7 @@ public NetworkObject GetNetworkObject(GameObject prefab, Vector3 position, Quate
         /// </summary>
         public void ReturnNetworkObject(NetworkObject networkObject, GameObject prefab)
         {
-            var go = networkObject.gameObject;
-            go.SetActive(false);
-            pooledObjects[prefab].Enqueue(networkObject);
+            m_PooledObjects[prefab].Release(networkObject);
         }
 
         /// <summary>
@@ -104,32 +100,60 @@ public void AddPrefab(GameObject prefab, int prewarmCount = 0)
             var networkObject = prefab.GetComponent<NetworkObject>();
 
             Assert.IsNotNull(networkObject, $"{nameof(prefab)} must have {nameof(networkObject)} component.");
-            Assert.IsFalse(prefabs.Contains(prefab), $"Prefab {prefab.name} is already registered in the pool.");
+            Assert.IsFalse(m_Prefabs.Contains(prefab), $"Prefab {prefab.name} is already registered in the pool.");
 
             RegisterPrefabInternal(prefab, prewarmCount);
         }
 
         /// <summary>
         /// Builds up the cache for a prefab.
         /// </summary>
-        private void RegisterPrefabInternal(GameObject prefab, int prewarmCount)
+        void RegisterPrefabInternal(GameObject prefab, int prewarmCount)
         {
-            prefabs.Add(prefab);
+            NetworkObject CreateFunc()
+            {
+                return CreateInstance(prefab).GetComponent<NetworkObject>();
+            }
+
+            void ActionOnGet(NetworkObject networkObject)
+            {
+                var go = networkObject.gameObject;
+                go.SetActive(true);
+            }
+
+            void ActionOnRelease(NetworkObject networkObject)
+            {
+                var go = networkObject.gameObject;
+                go.SetActive(false);
+            }
+
+            void ActionOnDestroy(NetworkObject networkObject)
+            {
+                Destroy(networkObject.gameObject);
+            }
+
+            m_Prefabs.Add(prefab);
+
+            // Create the pool
+            m_PooledObjects[prefab] = new ObjectPool<NetworkObject>((CreateFunc), ActionOnGet, ActionOnRelease, ActionOnDestroy, defaultCapacity: prewarmCount);
 
-            var prefabQueue = new Queue<NetworkObject>();
-            pooledObjects[prefab] = prefabQueue;
-            for (int i = 0; i < prewarmCount; i++)
+            // Populate the pool
+            var prewarmNetworkObjects = new List<NetworkObject>();
+            for (var i = 0; i < prewarmCount; i++)
             {
-                var go = CreateInstance(prefab);
-                ReturnNetworkObject(go.GetComponent<NetworkObject>(), prefab);
+                prewarmNetworkObjects.Add(m_PooledObjects[prefab].Get());
+            }
+            foreach (var networkObject in prewarmNetworkObjects)
+            {
+                m_PooledObjects[prefab].Release(networkObject);
             }
 
             // Register Netcode Spawn handlers
             NetworkManager.Singleton.PrefabHandler.AddHandler(prefab, new PooledPrefabInstanceHandler(prefab, this));
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private GameObject CreateInstance(GameObject prefab)
+        GameObject CreateInstance(GameObject prefab)
         {
             return Instantiate(prefab);
         }
@@ -141,54 +165,40 @@ private GameObject CreateInstance(GameObject prefab)
         /// <param name="position"></param>
         /// <param name="rotation"></param>
         /// <returns></returns>
-        private NetworkObject GetNetworkObjectInternal(GameObject prefab, Vector3 position, Quaternion rotation)
+        NetworkObject GetNetworkObjectInternal(GameObject prefab, Vector3 position, Quaternion rotation)
         {
-            var queue = pooledObjects[prefab];
-
-            NetworkObject networkObject;
-            if (queue.Count > 0)
-            {
-                networkObject = queue.Dequeue();
-            }
-            else
-            {
-                networkObject = CreateInstance(prefab).GetComponent<NetworkObject>();
-            }
-
-            // Here we must reverse the logic in ReturnNetworkObject.
-            var go = networkObject.gameObject;
-            go.SetActive(true);
+            var networkObject = m_PooledObjects[prefab].Get();
 
-            go.transform.position = position;
-            go.transform.rotation = rotation;
+            networkObject.transform.position = position;
+            networkObject.transform.rotation = rotation;
 
             return networkObject;
         }
 
         /// <summary>
         /// Registers all objects in <see cref="PooledPrefabsList"/> to the cache.
         /// </summary>
-        public void InitializePool()
+        void InitializePool()
         {
-            if (m_HasInitialized) return;
             foreach (var configObject in PooledPrefabsList)
             {
                 RegisterPrefabInternal(configObject.Prefab, configObject.PrewarmCount);
             }
-            m_HasInitialized = true;
         }
 
         /// <summary>
         /// Unregisters all objects in <see cref="PooledPrefabsList"/> from the cache.
         /// </summary>
-        public void ClearPool()
+        void ClearPool()
         {
-            foreach (var prefab in prefabs)
+            foreach (var prefab in m_Prefabs)
             {
                 // Unregister Netcode Spawn handlers
                 NetworkManager.Singleton.PrefabHandler.RemoveHandler(prefab);
+                m_PooledObjects[prefab].Clear();
             }
-            pooledObjects.Clear();
+            m_PooledObjects.Clear();
+            m_Prefabs.Clear();
         }
     }
 

CHANGELOG.md

@@ -10,6 +10,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Changed
 * Replaced our polling for lobby updates with a subscription to the new Websocket based LobbyEvents (#805). This saves up a significant amount of bandwidth usage to and from the service, since updates are infrequent in this game. Now clients and hosts only use up bandwidth on the Lobby service when it is needed. With polling, we used to send a GET request per client once every 2s. The responses were between ~550 bytes and 900 bytes, so if we suppose an average of 725 bytes and 100 000 concurrent users (CCU), this amounted to around 725B * 30 calls per minute * 100 000 CCU = 2.175 GB per minute. Scaling this to a month would get us 93.96 TB per month. In our case, since the only changes to the lobbies happen when a user connects or disconnects, most of that data was not necessary and can be saved to reduce bandwidth usage. Since the cost of using the Lobby service depends on bandwidth usage, this would also save money on an actual game.
+* Replaced our custom pool implementation using queues with ObjectPool (#824)
 * 
 ### Cleanup
 * Clarified a TODO comment inside ClientCharacter, detailing how anticipation should only be executed on owning client players (#786)