fix: Improved error checking for ClientScene.RegisterPrefab with handler (#1841)

* test for RegisterPrefab with handler

* comments

* finishing tests and ignore some cases

* updating checks and log messages

* more tests

* removing test cases

Assets/Mirror/Runtime/ClientScene.cs

@@ -330,6 +330,13 @@ public static void RegisterPrefab(GameObject prefab)
         /// <param name="unspawnHandler">A method to use as a custom un-spawnhandler on clients.</param>
         public static void RegisterPrefab(GameObject prefab, Guid newAssetId, SpawnDelegate spawnHandler, UnSpawnDelegate unspawnHandler)
         {
+            // We need this check here because we don't want a null handler in the lambda expression below
+            if (spawnHandler == null)
+            {
+                logger.LogError($"Can not Register null SpawnHandler for {newAssetId}");
+                return;
+            }
+
             RegisterPrefab(prefab, newAssetId, msg => spawnHandler(msg.position, msg.assetId), unspawnHandler);
         }
 
@@ -344,6 +351,40 @@ public static void RegisterPrefab(GameObject prefab, Guid newAssetId, SpawnDeleg
         /// <param name="unspawnHandler">A method to use as a custom un-spawnhandler on clients.</param>
         public static void RegisterPrefab(GameObject prefab, SpawnDelegate spawnHandler, UnSpawnDelegate unspawnHandler)
         {
+            if (prefab == null)
+            {
+                logger.LogError("Could not register handler for prefab because the prefab was null");
+                return;
+            }
+
+            NetworkIdentity identity = prefab.GetComponent<NetworkIdentity>();
+            if (identity == null)
+            {
+                logger.LogError("Could not register handler for '" + prefab.name + "' since it contains no NetworkIdentity component");
+                return;
+            }
+
+            if (identity.sceneId != 0)
+            {
+                logger.LogError($"Can not Register '{prefab.name}' because it has a sceneId, make sure you are passing in the original prefab and not an instance in the scene.");
+                return;
+            }
+
+            Guid assetId = identity.assetId;
+
+            if (assetId == Guid.Empty)
+            {
+                logger.LogError($"Can not Register handler for '{prefab.name}' because it had empty assetid. If this is a scene Object use RegisterSpawnHandler instead");
+                return;
+            }
+
+            // We need this check here because we don't want a null handler in the lambda expression below
+            if (spawnHandler == null)
+            {
+                logger.LogError($"Can not Register null SpawnHandler for {assetId}");
+                return;
+            }
+
             RegisterPrefab(prefab, msg => spawnHandler(msg.position, msg.assetId), unspawnHandler);
         }
 
@@ -359,31 +400,73 @@ public static void RegisterPrefab(GameObject prefab, SpawnDelegate spawnHandler,
         /// <param name="unspawnHandler">A method to use as a custom un-spawnhandler on clients.</param>
         public static void RegisterPrefab(GameObject prefab, Guid newAssetId, SpawnHandlerDelegate spawnHandler, UnSpawnDelegate unspawnHandler)
         {
+            if (newAssetId == Guid.Empty)
+            {
+                logger.LogError($"Could not register handler for '{prefab.name}' with new assetId because the new assetId was empty");
+                return;
+            }
+
+            if (prefab == null)
+            {
+                logger.LogError("Could not register handler for prefab because the prefab was null");
+                return;
+            }
+
             NetworkIdentity identity = prefab.GetComponent<NetworkIdentity>();
             if (identity == null)
             {
-                logger.LogError("Could not register '" + prefab.name + "' since it contains no NetworkIdentity component");
+                logger.LogError("Could not register handler for '" + prefab.name + "' since it contains no NetworkIdentity component");
+                return;
+            }
+
+            if (identity.sceneId != 0)
+            {
+                logger.LogError($"Can not Register '{prefab.name}' because it has a sceneId, make sure you are passing in the original prefab and not an instance in the scene.");
                 return;
             }
 
             identity.assetId = newAssetId;
+            Guid assetId = identity.assetId;
 
-            if (spawnHandler == null || unspawnHandler == null)
+            if (spawnHandler == null)
             {
-                logger.LogError("RegisterPrefab custom spawn function null for " + identity.assetId);
+                logger.LogError($"Can not Register null SpawnHandler for {assetId}");
                 return;
             }
 
-            if (identity.assetId == Guid.Empty)
+            if (unspawnHandler == null)
             {
-                logger.LogError("RegisterPrefab game object " + prefab.name + " has no prefab. Use RegisterSpawnHandler() instead?");
+                logger.LogError($"Can not Register null UnSpawnHandler for {assetId}");
                 return;
             }
 
-            if (logger.LogEnabled()) logger.Log("Registering custom prefab '" + prefab.name + "' as asset:" + identity.assetId + " " + spawnHandler.GetMethodName() + "/" + unspawnHandler.GetMethodName());
+            if (assetId == Guid.Empty)
+            {
+                logger.LogError($"Can not Register handler for '{prefab.name}' because it had empty assetid. If this is a scene Object use RegisterSpawnHandler instead");
+                return;
+            }
 
-            spawnHandlers[identity.assetId] = spawnHandler;
+            if (spawnHandlers.ContainsKey(assetId) || unspawnHandlers.ContainsKey(assetId))
-            unspawnHandlers[identity.assetId] = unspawnHandler;
+            {
+                logger.LogWarning($"Replacing existing spawnHandlers for prefab '{prefab.name}' with assetId '{assetId}'");
+            }
+
+            if (prefabs.ContainsKey(assetId))
+            {
+                // this is error because SpawnPrefab checks prefabs before handler
+                logger.LogError($"assetId '{assetId}' is already used by prefab '{prefabs[assetId].name}', unregister the prefab first before trying to add handler");
+            }
+
+            NetworkIdentity[] identities = prefab.GetComponentsInChildren<NetworkIdentity>();
+            if (identities.Length > 1)
+            {
+                logger.LogWarning($"Prefab '{prefab.name}' has multiple NetworkIdentity components. There should only be one NetworkIdentity on a prefab, and it must be on the root object.");
+            }
+
+            if (logger.LogEnabled()) logger.Log("Registering custom prefab '" + prefab.name + "' as asset:" + assetId + " " + spawnHandler.GetMethodName() + "/" + unspawnHandler.GetMethodName());
+
+            spawnHandlers[assetId] = spawnHandler;
+            unspawnHandlers[assetId] = unspawnHandler;
         }
 
         /// <summary>
@@ -397,29 +480,66 @@ public static void RegisterPrefab(GameObject prefab, Guid newAssetId, SpawnHandl
         /// <param name="unspawnHandler">A method to use as a custom un-spawnhandler on clients.</param>
         public static void RegisterPrefab(GameObject prefab, SpawnHandlerDelegate spawnHandler, UnSpawnDelegate unspawnHandler)
         {
+            if (prefab == null)
+            {
+                logger.LogError("Could not register handler for prefab because the prefab was null");
+                return;
+            }
+
             NetworkIdentity identity = prefab.GetComponent<NetworkIdentity>();
             if (identity == null)
             {
-                logger.LogError("Could not register '" + prefab.name + "' since it contains no NetworkIdentity component");
+                logger.LogError("Could not register handler for '" + prefab.name + "' since it contains no NetworkIdentity component");
                 return;
             }
 
-            if (spawnHandler == null || unspawnHandler == null)
+            if (identity.sceneId != 0)
             {
-                logger.LogError("RegisterPrefab custom spawn function null for " + identity.assetId);
+                logger.LogError($"Can not Register '{prefab.name}' because it has a sceneId, make sure you are passing in the original prefab and not an instance in the scene.");
                 return;
             }
 
-            if (identity.assetId == Guid.Empty)
+            Guid assetId = identity.assetId;
+
+            if (assetId == Guid.Empty)
             {
-                logger.LogError("RegisterPrefab game object " + prefab.name + " has no prefab. Use RegisterSpawnHandler() instead?");
+                logger.LogError($"Can not Register handler for '{prefab.name}' because it had empty assetid. If this is a scene Object use RegisterSpawnHandler instead");
                 return;
             }
 
-            if (logger.LogEnabled()) logger.Log("Registering custom prefab '" + prefab.name + "' as asset:" + identity.assetId + " " + spawnHandler.GetMethodName() + "/" + unspawnHandler.GetMethodName());
+            if (spawnHandler == null)
+            {
+                logger.LogError($"Can not Register null SpawnHandler for {assetId}");
+                return;
+            }
 
-            spawnHandlers[identity.assetId] = spawnHandler;
+            if (unspawnHandler == null)
-            unspawnHandlers[identity.assetId] = unspawnHandler;
+            {
+                logger.LogError($"Can not Register null UnSpawnHandler for {assetId}");
+                return;
+            }
+
+            if (spawnHandlers.ContainsKey(assetId) || unspawnHandlers.ContainsKey(assetId))
+            {
+                logger.LogWarning($"Replacing existing spawnHandlers for prefab '{prefab.name}' with assetId '{assetId}'");
+            }
+
+            if (prefabs.ContainsKey(assetId))
+            {
+                // this is error because SpawnPrefab checks prefabs before handler
+                logger.LogError($"assetId '{assetId}' is already used by prefab '{prefabs[assetId].name}', unregister the prefab first before trying to add handler");
+            }
+
+            NetworkIdentity[] identities = prefab.GetComponentsInChildren<NetworkIdentity>();
+            if (identities.Length > 1)
+            {
+                logger.LogWarning($"Prefab '{prefab.name}' has multiple NetworkIdentity components. There should only be one NetworkIdentity on a prefab, and it must be on the root object.");
+            }
+
+            if (logger.LogEnabled()) logger.Log("Registering custom prefab '" + prefab.name + "' as asset:" + assetId + " " + spawnHandler.GetMethodName() + "/" + unspawnHandler.GetMethodName());
+
+            spawnHandlers[assetId] = spawnHandler;
+            unspawnHandlers[assetId] = unspawnHandler;
         }
 
         /// <summary>
@@ -494,8 +614,6 @@ public static void RegisterSpawnHandler(Guid assetId, SpawnHandlerDelegate spawn
                 return;
             }
 
-            if (logger.LogEnabled()) logger.Log("RegisterSpawnHandler asset '" + assetId + "' " + spawnHandler.GetMethodName() + "/" + unspawnHandler.GetMethodName());
-
             if (spawnHandlers.ContainsKey(assetId) || unspawnHandlers.ContainsKey(assetId))
             {
                 logger.LogWarning($"Replacing existing spawnHandlers for {assetId}");
@@ -507,6 +625,8 @@ public static void RegisterSpawnHandler(Guid assetId, SpawnHandlerDelegate spawn
                 logger.LogError($"assetId '{assetId}' is already used by prefab '{prefabs[assetId].name}'");
             }
 
+            if (logger.LogEnabled()) logger.Log("RegisterSpawnHandler asset '" + assetId + "' " + spawnHandler.GetMethodName() + "/" + unspawnHandler.GetMethodName());
+
             spawnHandlers[assetId] = spawnHandler;
             unspawnHandlers[assetId] = unspawnHandler;
         }

Assets/Mirror/Tests/Editor/ClientSceneTests.cs

@@ -10,6 +10,8 @@ namespace Mirror.Tests
     [TestFixture]
     public class ClientSceneTests
     {
+        const string NewAssetIdIgnoreMessage = "Ignoring this test till we know how to fix it, see https://github.com/vis2k/Mirror/issues/1831";
+
         // use guid to find asset so that the path does not matter
         const string ValidPrefabAssetGuid = "33169286da0313d45ab5bfccc6cf3775";
         const string PrefabWithChildrenAssetGuid = "a78e009e3f2dee44e8859516974ede43";
@@ -128,6 +130,19 @@ public void CheckOverloadWithAssetId(RegisterPrefabOverload overload, bool expec
             Assert.That(OverloadWithAssetId(overload), Is.EqualTo(expected));
         }
 
+        [Test]
+        [TestCase(RegisterPrefabOverload.Prefab, false)]
+        [TestCase(RegisterPrefabOverload.Prefab_NewAssetId, false)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate, true)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId, true)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate, true)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId, true)]
+        public void CheckOverloadWithHandler(RegisterPrefabOverload overload, bool expected)
+        {
+            // test to make sure OverloadWithAssetId correctly works with flags
+            Assert.That(OverloadWithHandler(overload), Is.EqualTo(expected));
+        }
+
         /// <summary>
         /// Allows TestCases to call different overloads for RegisterPrefab.
         /// Without this we would need duplicate tests for each overload
@@ -142,7 +157,8 @@ public enum RegisterPrefabOverload
             Prefab_SpawnHandlerDelegate = 16,
             Prefab_SpawnHandlerDelegate_NewAssetId = 32,
 
-            WithAssetId = Prefab_NewAssetId | Prefab_SpawnDelegate_NewAssetId | Prefab_SpawnHandlerDelegate_NewAssetId
+            WithAssetId = Prefab_NewAssetId | Prefab_SpawnDelegate_NewAssetId | Prefab_SpawnHandlerDelegate_NewAssetId,
+            WithHandler = Prefab_SpawnDelegate | Prefab_SpawnDelegate_NewAssetId | Prefab_SpawnHandlerDelegate | Prefab_SpawnHandlerDelegate_NewAssetId
         }
 
         static bool OverloadWithAssetId(RegisterPrefabOverload overload)
@@ -150,6 +166,11 @@ static bool OverloadWithAssetId(RegisterPrefabOverload overload)
             return (overload & RegisterPrefabOverload.WithAssetId) != 0;
         }
 
+        static bool OverloadWithHandler(RegisterPrefabOverload overload)
+        {
+            return (overload & RegisterPrefabOverload.WithHandler) != 0;
+        }
+
         void CallRegisterPrefab(GameObject prefab, RegisterPrefabOverload overload)
         {
             SpawnDelegate spawnHandler = new SpawnDelegate((x, y) => null);
@@ -176,52 +197,136 @@ void CallRegisterPrefab(GameObject prefab, RegisterPrefabOverload overload)
                 case RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId:
                     ClientScene.RegisterPrefab(prefab, anotherGuid, spawnHandlerDelegate, unspawnHandler);
                     break;
+
+                default:
+                    Debug.LogError("Overload not found");
+                    break;
+            }
+        }
+        void CallRegisterPrefab(GameObject prefab, RegisterPrefabOverload overload, Guid guid)
+        {
+            SpawnDelegate spawnHandler = new SpawnDelegate((x, y) => null);
+            SpawnHandlerDelegate spawnHandlerDelegate = new SpawnHandlerDelegate(x => null);
+            UnSpawnDelegate unspawnHandler = new UnSpawnDelegate(x => { });
+
+            switch (overload)
+            {
+                case RegisterPrefabOverload.Prefab_NewAssetId:
+                    ClientScene.RegisterPrefab(prefab, guid);
+                    break;
+                case RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId:
+                    ClientScene.RegisterPrefab(prefab, guid, spawnHandler, unspawnHandler);
+                    break;
+                case RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId:
+                    ClientScene.RegisterPrefab(prefab, guid, spawnHandlerDelegate, unspawnHandler);
+                    break;
+
+                case RegisterPrefabOverload.Prefab:
+                case RegisterPrefabOverload.Prefab_SpawnDelegate:
+                case RegisterPrefabOverload.Prefab_SpawnHandlerDelegate:
+                    Debug.LogError("Overload did not have guid parameter");
+                    break;
+                default:
+                    Debug.LogError("Overload not found");
+                    break;
+            }
+        }
+        void CallRegisterPrefab(GameObject prefab, RegisterPrefabOverload overload, SpawnDelegate spawnHandler)
+        {
+            UnSpawnDelegate unspawnHandler = new UnSpawnDelegate(x => { });
+
+            switch (overload)
+            {
+                case RegisterPrefabOverload.Prefab_SpawnDelegate:
+                    ClientScene.RegisterPrefab(prefab, spawnHandler, unspawnHandler);
+                    break;
+                case RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId:
+                    ClientScene.RegisterPrefab(prefab, anotherGuid, spawnHandler, unspawnHandler);
+                    break;
+
+                case RegisterPrefabOverload.Prefab:
+                case RegisterPrefabOverload.Prefab_NewAssetId:
+                case RegisterPrefabOverload.Prefab_SpawnHandlerDelegate:
+                case RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId:
+                    Debug.LogError("Overload did not have SpawnDelegate parameter");
+                    break;
+                default:
+                    Debug.LogError("Overload not found");
+                    break;
+            }
+        }
+        void CallRegisterPrefab(GameObject prefab, RegisterPrefabOverload overload, SpawnHandlerDelegate spawnHandlerDelegate)
+        {
+            UnSpawnDelegate unspawnHandler = new UnSpawnDelegate(x => { });
+
+            switch (overload)
+            {
+                case RegisterPrefabOverload.Prefab_SpawnHandlerDelegate:
+                    ClientScene.RegisterPrefab(prefab, spawnHandlerDelegate, unspawnHandler);
+                    break;
+                case RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId:
+                    ClientScene.RegisterPrefab(prefab, anotherGuid, spawnHandlerDelegate, unspawnHandler);
+                    break;
+
+                case RegisterPrefabOverload.Prefab:
+                case RegisterPrefabOverload.Prefab_NewAssetId:
+                case RegisterPrefabOverload.Prefab_SpawnDelegate:
+                case RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId:
+                    Debug.LogError("Overload did not have SpawnDelegate parameter");
+                    break;
+                default:
+                    Debug.LogError("Overload not found");
+                    break;
+            }
+        }
+        void CallRegisterPrefab(GameObject prefab, RegisterPrefabOverload overload, UnSpawnDelegate unspawnHandler)
+        {
+            SpawnDelegate spawnHandler = new SpawnDelegate((x, y) => null);
+            SpawnHandlerDelegate spawnHandlerDelegate = new SpawnHandlerDelegate(x => null);
+
+            switch (overload)
+            {
+
+                case RegisterPrefabOverload.Prefab_SpawnDelegate:
+                    ClientScene.RegisterPrefab(prefab, spawnHandler, unspawnHandler);
+                    break;
+                case RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId:
+                    ClientScene.RegisterPrefab(prefab, anotherGuid, spawnHandler, unspawnHandler);
+                    break;
+                case RegisterPrefabOverload.Prefab_SpawnHandlerDelegate:
+                    ClientScene.RegisterPrefab(prefab, spawnHandlerDelegate, unspawnHandler);
+                    break;
+                case RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId:
+                    ClientScene.RegisterPrefab(prefab, anotherGuid, spawnHandlerDelegate, unspawnHandler);
+                    break;
+
+                case RegisterPrefabOverload.Prefab:
+                case RegisterPrefabOverload.Prefab_NewAssetId:
+                    Debug.LogError("Overload did not have UnSpawnDelegate parameter");
+                    break;
                 default:
                     Debug.LogError("Overload not found");
                     break;
             }
         }
 
-        void CallRegisterPrefab_Handler(GameObject prefab, SpawnDelegate spawn, UnSpawnDelegate unspawn, RegisterPrefabOverload overload)
+        Guid GuidForOverload(RegisterPrefabOverload overload) => OverloadWithAssetId(overload) ? anotherGuid : validPrefabGuid;
-        {
-            if (overload == RegisterPrefabOverload.Prefab_SpawnDelegate)
-            {
-                ClientScene.RegisterPrefab(prefab, spawn, unspawn);
-            }
-            else if (overload == RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId)
-            {
-                ClientScene.RegisterPrefab(prefab, anotherGuid, spawn, unspawn);
-            }
-            else
-            {
-                Debug.LogError("Overload did not have SpawnDelegate");
-            }
-        }
 
-        void CallRegisterPrefab_Handler(GameObject prefab, SpawnHandlerDelegate spawn, UnSpawnDelegate unspawn, RegisterPrefabOverload overload)
+        static void CreateSceneObject(out GameObject runtimeObject, out NetworkIdentity networkIdentity)
         {
-            if (overload == RegisterPrefabOverload.Prefab_SpawnHandlerDelegate)
+            runtimeObject = new GameObject("Runtime GameObject");
-            {
+            networkIdentity = runtimeObject.AddComponent<NetworkIdentity>();
-                ClientScene.RegisterPrefab(prefab, spawn, unspawn);
+            // set sceneId to zero as it is set in onvalidate (does not set id at runtime)
-            }
+            networkIdentity.sceneId = 0;
-            else if (overload == RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId)
-            {
-                ClientScene.RegisterPrefab(prefab, anotherGuid, spawn, unspawn);
-            }
-            else
-            {
-                Debug.LogError("Overload did not have SpawnHandlerDelegate");
-            }
         }
 
 
         [Test]
         [TestCase(RegisterPrefabOverload.Prefab)]
-        [TestCase(RegisterPrefabOverload.Prefab_NewAssetId)]
+        [TestCase(RegisterPrefabOverload.Prefab_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
-        [Ignore("Ignoring this test till we know how to fix it, see https://github.com/vis2k/Mirror/issues/1831")]
         public void RegisterPrefab_Prefab_AddsPrefabToDictionary(RegisterPrefabOverload overload)
         {
-            Guid guid = OverloadWithAssetId(overload) ? anotherGuid : validPrefabGuid;
+            Guid guid = GuidForOverload(overload);
 
             CallRegisterPrefab(validPrefab, overload);
 
@@ -230,11 +335,13 @@ public void RegisterPrefab_Prefab_AddsPrefabToDictionary(RegisterPrefabOverload
         }
 
         [Test]
-        [Ignore("Ignoring this test till we know how to fix it, see https://github.com/vis2k/Mirror/issues/1831")]
+        [TestCase(RegisterPrefabOverload.Prefab_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
-        public void RegisterPrefab_PrefabNewGuid_ChangePrefabsAssetId()
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        public void RegisterPrefab_NewGuid_ChangePrefabsAssetId(RegisterPrefabOverload overload)
         {
             Guid guid = anotherGuid;
-            ClientScene.RegisterPrefab(validPrefab, guid);
+            CallRegisterPrefab(validPrefab, overload);
 
             Assert.IsTrue(prefabs.ContainsKey(guid));
             Assert.AreEqual(prefabs[guid], validPrefab);
@@ -247,70 +354,132 @@ public void RegisterPrefab_PrefabNewGuid_ChangePrefabsAssetId()
         [Test]
         [TestCase(RegisterPrefabOverload.Prefab)]
         [TestCase(RegisterPrefabOverload.Prefab_NewAssetId)]
-        public void RegisterPrefab_Prefab_ErrorForNullPrefab(RegisterPrefabOverload overload)
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId)]
+        public void RegisterPrefab_ErrorForNullPrefab(RegisterPrefabOverload overload)
         {
-            LogAssert.Expect(LogType.Error, "Could not register prefab because it was null");
+            string msg = OverloadWithHandler(overload)
+                ? "Could not register handler for prefab because the prefab was null"
+                : "Could not register prefab because it was null";
+
+            LogAssert.Expect(LogType.Error, msg);
             CallRegisterPrefab(null, overload);
         }
 
         [Test]
         [TestCase(RegisterPrefabOverload.Prefab)]
         [TestCase(RegisterPrefabOverload.Prefab_NewAssetId)]
-        public void RegisterPrefab_Prefab_ErrorForPrefabWithoutNetworkIdentity(RegisterPrefabOverload overload)
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId)]
+        public void RegisterPrefab_ErrorForPrefabWithoutNetworkIdentity(RegisterPrefabOverload overload)
         {
-            LogAssert.Expect(LogType.Error, $"Could not register '{invalidPrefab.name}' since it contains no NetworkIdentity component");
+            string msg = OverloadWithHandler(overload)
+                ? $"Could not register handler for '{invalidPrefab.name}' since it contains no NetworkIdentity component"
+                : $"Could not register '{invalidPrefab.name}' since it contains no NetworkIdentity component";
+
+            LogAssert.Expect(LogType.Error, msg);
             CallRegisterPrefab(invalidPrefab, overload);
         }
 
-        static void CreateSceneObject(out GameObject runtimeObject, out NetworkIdentity networkIdentity)
-        {
-            runtimeObject = new GameObject("Runtime GameObject");
-            networkIdentity = runtimeObject.AddComponent<NetworkIdentity>();
-            // set sceneId to zero as it is set in onvalidate (does not set id at runtime)
-            networkIdentity.sceneId = 0;
-        }
-
         [Test]
-        public void RegisterPrefab_Prefab_ErrorForEmptyGuid()
+        [TestCase(RegisterPrefabOverload.Prefab)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate)]
+        public void RegisterPrefab_ErrorForEmptyGuid(RegisterPrefabOverload overload)
         {
+            // setup
             CreateSceneObject(out GameObject runtimeObject, out NetworkIdentity networkIdentity);
 
             //test
-            LogAssert.Expect(LogType.Error, $"Can not Register '{runtimeObject.name}' because it had empty assetid. If this is a scene Object use RegisterSpawnHandler instead");
+            string msg = OverloadWithHandler(overload)
-            ClientScene.RegisterPrefab(runtimeObject);
+               ? $"Can not Register handler for '{runtimeObject.name}' because it had empty assetid. If this is a scene Object use RegisterSpawnHandler instead"
+               : $"Can not Register '{runtimeObject.name}' because it had empty assetid. If this is a scene Object use RegisterSpawnHandler instead";
+
+            LogAssert.Expect(LogType.Error, msg);
+            CallRegisterPrefab(runtimeObject, overload);
 
             // teardown
             GameObject.DestroyImmediate(runtimeObject);
         }
 
         [Test]
-        public void RegisterPrefab_PrefabNewGuid_AddsRuntimeObjectToDictionary()
+        [TestCase(RegisterPrefabOverload.Prefab_NewAssetId)]
+        public void RegisterPrefab_PrefabNewGuid_AddsRuntimeObjectToDictionary(RegisterPrefabOverload overload)
         {
+            // setup
             CreateSceneObject(out GameObject runtimeObject, out NetworkIdentity networkIdentity);
 
-            Guid guid = anotherGuid;
+            //test
-            ClientScene.RegisterPrefab(runtimeObject, guid);
+            CallRegisterPrefab(runtimeObject, overload);
 
-            Assert.IsTrue(prefabs.ContainsKey(guid));
+            Assert.IsTrue(prefabs.ContainsKey(anotherGuid));
-            Assert.AreEqual(prefabs[guid], runtimeObject);
+            Assert.AreEqual(prefabs[anotherGuid], runtimeObject);
 
-            Assert.AreEqual(networkIdentity.assetId, guid);
+            Assert.AreEqual(networkIdentity.assetId, anotherGuid);
 
             // teardown
             GameObject.DestroyImmediate(runtimeObject);
         }
 
         [Test]
-        public void RegisterPrefab_PrefabNewGuid_ErrorForEmptyGuid()
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId)]
+        public void RegisterPrefab_Handler_AddsSpawnHandlerToDictionaryForRuntimeObject(RegisterPrefabOverload overload)
         {
-            LogAssert.Expect(LogType.Error, $"Could not register '{validPrefab.name}' with new assetId because the new assetId was empty");
+            // setup
-            ClientScene.RegisterPrefab(validPrefab, new Guid());
+            CreateSceneObject(out GameObject runtimeObject, out NetworkIdentity networkIdentity);
+
+            //test
+            CallRegisterPrefab(runtimeObject, overload);
+
+            Assert.IsTrue(spawnHandlers.ContainsKey(anotherGuid));
+
+            // teardown
+            GameObject.DestroyImmediate(runtimeObject);
+        }
+
+        [Test]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId)]
+        public void RegisterPrefab_Handler_AddsUnSpawnHandlerToDictionaryForRuntimeObject(RegisterPrefabOverload overload)
+        {
+            // setup
+            CreateSceneObject(out GameObject runtimeObject, out NetworkIdentity networkIdentity);
+
+            //test
+            CallRegisterPrefab(runtimeObject, overload);
+
+            Assert.IsTrue(unspawnHandlers.ContainsKey(anotherGuid));
+
+            // teardown
+            GameObject.DestroyImmediate(runtimeObject);
+        }
+
+        [Test]
+        [TestCase(RegisterPrefabOverload.Prefab_NewAssetId)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId)]
+        public void RegisterPrefab_NewGuid_ErrorForEmptyGuid(RegisterPrefabOverload overload)
+        {
+            string msg = OverloadWithHandler(overload)
+                ? $"Could not register handler for '{validPrefab.name}' with new assetId because the new assetId was empty"
+                : $"Could not register '{validPrefab.name}' with new assetId because the new assetId was empty";
+            LogAssert.Expect(LogType.Error, msg);
+            CallRegisterPrefab(validPrefab, overload, guid: new Guid());
         }
 
         [Test]
         [TestCase(RegisterPrefabOverload.Prefab)]
         [TestCase(RegisterPrefabOverload.Prefab_NewAssetId)]
-        public void RegisterPrefab_Prefab_ErrorIfPrefabHadSceneId(RegisterPrefabOverload overload)
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId)]
+        public void RegisterPrefab_ErrorIfPrefabHadSceneId(RegisterPrefabOverload overload)
         {
             GameObject clone = GameObject.Instantiate(validPrefab);
             NetworkIdentity netId = clone.GetComponent<NetworkIdentity>();
@@ -326,7 +495,11 @@ public void RegisterPrefab_Prefab_ErrorIfPrefabHadSceneId(RegisterPrefabOverload
         [Test]
         [TestCase(RegisterPrefabOverload.Prefab)]
         [TestCase(RegisterPrefabOverload.Prefab_NewAssetId)]
-        public void RegisterPrefab_Prefab_WarningForNetworkIdentityInChildren(RegisterPrefabOverload overload)
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId)]
+        public void RegisterPrefab_WarningForNetworkIdentityInChildren(RegisterPrefabOverload overload)
         {
             LogAssert.Expect(LogType.Warning, $"Prefab '{prefabWithChildren.name}' has multiple NetworkIdentity components. There should only be one NetworkIdentity on a prefab, and it must be on the root object.");
             CallRegisterPrefab(prefabWithChildren, overload);
@@ -335,11 +508,10 @@ public void RegisterPrefab_Prefab_WarningForNetworkIdentityInChildren(RegisterPr
 
         [Test]
         [TestCase(RegisterPrefabOverload.Prefab)]
-        [TestCase(RegisterPrefabOverload.Prefab_NewAssetId)]
+        [TestCase(RegisterPrefabOverload.Prefab_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
-        [Ignore("Ignoring this test till we know how to fix it, see https://github.com/vis2k/Mirror/issues/1831")]
         public void RegisterPrefab_Prefab_WarningForAssetIdAlreadyExistingInPrefabsDictionary(RegisterPrefabOverload overload)
         {
-            Guid guid = OverloadWithAssetId(overload) ? anotherGuid : validPrefabGuid;
+            Guid guid = GuidForOverload(overload);
 
             prefabs.Add(guid, validPrefab);
 
@@ -348,21 +520,143 @@ public void RegisterPrefab_Prefab_WarningForAssetIdAlreadyExistingInPrefabsDicti
         }
 
         [Test]
-        [TestCase(RegisterPrefabOverload.Prefab)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate)]
-        [TestCase(RegisterPrefabOverload.Prefab_NewAssetId)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
-        [Ignore("Ignoring this test till we know how to fix it, see https://github.com/vis2k/Mirror/issues/1831")]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate)]
-        public void RegisterPrefab_Prefab_WarningForAssetIdAlreadyExistingInHandlersDictionary(RegisterPrefabOverload overload)
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        public void RegisterPrefab_Handler_ErrorForAssetIdAlreadyExistingInPrefabsDictionary(RegisterPrefabOverload overload)
         {
-            Guid guid = OverloadWithAssetId(overload) ? anotherGuid : validPrefabGuid;
+            Guid guid = GuidForOverload(overload);
+
+            prefabs.Add(guid, validPrefab);
+
+            LogAssert.Expect(LogType.Error, $"assetId '{guid}' is already used by prefab '{validPrefab.name}', unregister the prefab first before trying to add handler");
+            CallRegisterPrefab(validPrefab, overload);
+        }
+
+        [Test]
+        [TestCase(RegisterPrefabOverload.Prefab)]
+        [TestCase(RegisterPrefabOverload.Prefab_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        public void RegisterPrefab_WarningForAssetIdAlreadyExistingInHandlersDictionary(RegisterPrefabOverload overload)
+        {
+            Guid guid = GuidForOverload(overload);
 
             spawnHandlers.Add(guid, x => null);
             unspawnHandlers.Add(guid, x => { });
 
-            LogAssert.Expect(LogType.Warning, $"Adding prefab '{validPrefab.name}' with assetId '{guid}' when spawnHandlers with same assetId already exists.");
+            string msg = OverloadWithHandler(overload)
+                ? $"Replacing existing spawnHandlers for prefab '{validPrefab.name}' with assetId '{guid}'"
+                : $"Adding prefab '{validPrefab.name}' with assetId '{guid}' when spawnHandlers with same assetId already exists.";
+
+            LogAssert.Expect(LogType.Warning, msg);
             CallRegisterPrefab(validPrefab, overload);
         }
 
 
+        [Test]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        public void RegisterPrefab_SpawnDelegate_AddsHandlerToSpawnHandlers(RegisterPrefabOverload overload)
+        {
+            int handlerCalled = 0;
+
+            Guid guid = GuidForOverload(overload);
+            SpawnDelegate handler = new SpawnDelegate((pos, rot) =>
+            {
+                handlerCalled++;
+                return null;
+            });
+
+            CallRegisterPrefab(validPrefab, overload, spawnHandler: handler);
+
+
+            Assert.IsTrue(spawnHandlers.ContainsKey(guid));
+
+            // check spawnHandler above is called
+            SpawnHandlerDelegate handlerInDictionary = spawnHandlers[guid];
+            handlerInDictionary.Invoke(default);
+            Assert.That(handlerCalled, Is.EqualTo(1));
+        }
+
+        [Test]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        public void RegisterPrefab_SpawnDelegate_AddsHandlerToSpawnHandlersWithCorrectArguments(RegisterPrefabOverload overload)
+        {
+            int handlerCalled = 0;
+            Vector3 somePosition = new Vector3(10, 20, 3);
+
+            Guid guid = GuidForOverload(overload);
+            SpawnDelegate handler = new SpawnDelegate((pos, assetId) =>
+            {
+                handlerCalled++;
+                Assert.That(pos, Is.EqualTo(somePosition));
+                Assert.That(assetId, Is.EqualTo(guid));
+                return null;
+            });
+
+            CallRegisterPrefab(validPrefab, overload, spawnHandler: handler);
+
+            Assert.IsTrue(spawnHandlers.ContainsKey(guid));
+
+            // check spawnHandler above is called
+            SpawnHandlerDelegate handlerInDictionary = spawnHandlers[guid];
+            handlerInDictionary.Invoke(new SpawnMessage { position = somePosition, assetId = guid });
+            Assert.That(handlerCalled, Is.EqualTo(1));
+        }
+
+        [Test]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId)]
+        public void RegisterPrefab_SpawnDelegate_ErrorWhenSpawnHandlerIsNull(RegisterPrefabOverload overload)
+        {
+            Guid guid = GuidForOverload(overload);
+            LogAssert.Expect(LogType.Error, $"Can not Register null SpawnHandler for {guid}");
+            CallRegisterPrefab(validPrefab, overload, spawnHandler: null);
+        }
+
+        [Test]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        public void RegisterPrefab_SpawnHandleDelegate_AddsHandlerToSpawnHandlers(RegisterPrefabOverload overload)
+        {
+            Guid guid = GuidForOverload(overload);
+
+            SpawnHandlerDelegate handler = new SpawnHandlerDelegate(x => null);
+
+            CallRegisterPrefab(validPrefab, overload, spawnHandlerDelegate: handler);
+
+            Assert.IsTrue(spawnHandlers.ContainsKey(guid));
+            Assert.AreEqual(spawnHandlers[guid], handler);
+        }
+
+        [Test]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        public void RegisterPrefab_SpawnHandleDelegate_ErrorWhenSpawnHandlerIsNull(RegisterPrefabOverload overload)
+        {
+            Guid guid = GuidForOverload(overload);
+            LogAssert.Expect(LogType.Error, $"Can not Register null SpawnHandler for {guid}");
+            CallRegisterPrefab(validPrefab, overload, spawnHandlerDelegate: null);
+        }
+
+        [Test]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate)]
+        [TestCase(RegisterPrefabOverload.Prefab_SpawnHandlerDelegate_NewAssetId, IgnoreReason = NewAssetIdIgnoreMessage)]
+        public void RegisterPrefab_Handler_ErrorWhenUnSpawnHandlerIsNull(RegisterPrefabOverload overload)
+        {
+            Guid guid = GuidForOverload(overload);
+            LogAssert.Expect(LogType.Error, $"Can not Register null UnSpawnHandler for {guid}");
+            CallRegisterPrefab(validPrefab, overload, unspawnHandler: null);
+        }
+
+
         [Test]
         public void UnregisterPrefab_RemovesPrefabFromDictionary()
         {