refactor: Simplify spawn message (#1195)

* refactor: Simplify spawn message

* Update Assets/Mirror/Runtime/Messages.cs

* fix brainfart

* consolidate spawning logic

* simpler find

* remove unnecesary private keyword

* Remove redundant else

Assets/Mirror/Runtime/ClientScene.cs

@@ -438,28 +438,32 @@ public static GameObject FindLocalObject(uint netId)
             return null;
         }
 
-        static void ApplySpawnPayload(NetworkIdentity identity, Vector3 position, Quaternion rotation, Vector3 scale, ArraySegment<byte> payload, uint netId)
+        static void ApplySpawnPayload(NetworkIdentity identity, SpawnMessage msg)
         {
+            identity.Reset();
+            identity.pendingLocalPlayer = msg.isLocalPlayer;
+            identity.assetId = msg.assetId;
+
             if (!identity.gameObject.activeSelf)
             {
                 identity.gameObject.SetActive(true);
             }
 
             // apply local values for VR support
-            identity.transform.localPosition = position;
-            identity.transform.localRotation = rotation;
-            identity.transform.localScale = scale;
+            identity.transform.localPosition = msg.position;
+            identity.transform.localRotation = msg.rotation;
+            identity.transform.localScale = msg.scale;
 
             // deserialize components if any payload
             // (Count is 0 if there were no components)
-            if (payload.Count > 0)
+            if (msg.payload.Count > 0)
             {
-                NetworkReader payloadReader = new NetworkReader(payload);
+                NetworkReader payloadReader = new NetworkReader(msg.payload);
                 identity.OnUpdateVars(payloadReader, true);
             }
 
-            identity.netId = netId;
-            NetworkIdentity.spawned[netId] = identity;
+            identity.netId = msg.netId;
+            NetworkIdentity.spawned[msg.netId] = identity;
 
             // objects spawned as part of initial state are started on a second pass
             if (isSpawnFinished)
@@ -469,14 +473,14 @@ static void ApplySpawnPayload(NetworkIdentity identity, Vector3 position, Quater
             }
         }
 
-        internal static void OnSpawnPrefab(NetworkConnection _, SpawnPrefabMessage msg)
+        internal static void OnSpawn(NetworkConnection _, SpawnMessage msg)
         {
-            if (msg.assetId == Guid.Empty)
+            if (msg.assetId == Guid.Empty && msg.sceneId == 0)
             {
                 Debug.LogError("OnObjSpawn netId: " + msg.netId + " has invalid asset Id");
                 return;
             }
-            if (LogFilter.Debug) Debug.Log("Client spawn handler instantiating [netId:" + msg.netId + " asset ID:" + msg.assetId + " pos:" + msg.position + "]");
+            if (LogFilter.Debug) Debug.Log($"Client spawn handler instantiating netId={msg.netId} assetID={msg.assetId} sceneId={msg.sceneId} pos={msg.position}");
 
             // is this supposed to be the local player?
             if (msg.isLocalPlayer)
@@ -484,14 +488,31 @@ internal static void OnSpawnPrefab(NetworkConnection _, SpawnPrefabMessage msg)
                 OnSpawnMessageForLocalPlayer(msg.netId);
             }
 
-            if (NetworkIdentity.spawned.TryGetValue(msg.netId, out NetworkIdentity localObject) && localObject != null)
+            // was the object already spawned?
+            NetworkIdentity identity = GetExistingObject(msg.netId);
+
+            if (identity == null)
             {
-                // this object already exists (was in the scene), just apply the update to existing object
-                localObject.Reset();
-                ApplySpawnPayload(localObject, msg.position, msg.rotation, msg.scale, msg.payload, msg.netId);
+                identity = msg.sceneId == 0 ? SpawnPrefab(msg) : SpawnSceneObject(msg);
+            }
+
+            if (identity == null)
+            {
+                Debug.LogError($"Could not spawn assetId={msg.assetId} scene={msg.sceneId} netId={msg.netId}");
                 return;
             }
 
+            ApplySpawnPayload(identity, msg);
+        }
+
+        static NetworkIdentity GetExistingObject(uint netid)
+        {
+            NetworkIdentity.spawned.TryGetValue(netid, out NetworkIdentity localObject);
+            return localObject;
+        }
+
+        static NetworkIdentity SpawnPrefab(SpawnMessage msg)
+        {
             if (GetPrefab(msg.assetId, out GameObject prefab))
             {
                 GameObject obj = Object.Instantiate(prefab, msg.position, msg.rotation);
@@ -500,60 +521,24 @@ internal static void OnSpawnPrefab(NetworkConnection _, SpawnPrefabMessage msg)
                     Debug.Log("Client spawn handler instantiating [netId:" + msg.netId + " asset ID:" + msg.assetId + " pos:" + msg.position + " rotation: " + msg.rotation + "]");
                 }
 
-                localObject = obj.GetComponent<NetworkIdentity>();
-                if (localObject == null)
-                {
-                    Debug.LogError("Client object spawned for " + msg.assetId + " does not have a NetworkIdentity");
-                    return;
+                return obj.GetComponent<NetworkIdentity>();
             }
-                localObject.Reset();
-                localObject.pendingLocalPlayer = msg.isLocalPlayer;
-                ApplySpawnPayload(localObject, msg.position, msg.rotation, msg.scale, msg.payload, msg.netId);
-            }
-            // lookup registered factory for type:
-            else if (spawnHandlers.TryGetValue(msg.assetId, out SpawnDelegate handler))
+            if (spawnHandlers.TryGetValue(msg.assetId, out SpawnDelegate handler))
             {
                 GameObject obj = handler(msg.position, msg.assetId);
                 if (obj == null)
                 {
                     Debug.LogWarning("Client spawn handler for " + msg.assetId + " returned null");
-                    return;
+                    return null;
                 }
-                localObject = obj.GetComponent<NetworkIdentity>();
-                if (localObject == null)
-                {
-                    Debug.LogError("Client object spawned for " + msg.assetId + " does not have a network identity");
-                    return;
+                return obj.GetComponent<NetworkIdentity>();
             }
-                localObject.Reset();
-                localObject.pendingLocalPlayer = msg.isLocalPlayer;
-                localObject.assetId = msg.assetId;
-                ApplySpawnPayload(localObject, msg.position, msg.rotation, msg.scale, msg.payload, msg.netId);
-            }
-            else
-            {
             Debug.LogError("Failed to spawn server object, did you forget to add it to the NetworkManager? assetId=" + msg.assetId + " netId=" + msg.netId);
-            }
+            return null;
         }
 
-        internal static void OnSpawnSceneObject(NetworkConnection _, SpawnSceneObjectMessage msg)
+        static NetworkIdentity SpawnSceneObject(SpawnMessage msg)
         {
-            if (LogFilter.Debug) Debug.Log("Client spawn scene handler instantiating [netId:" + msg.netId + " sceneId:" + msg.sceneId + " pos:" + msg.position);
-
-            // is this supposed to be the local player?
-            if (msg.isLocalPlayer)
-            {
-                OnSpawnMessageForLocalPlayer(msg.netId);
-            }
-
-            if (NetworkIdentity.spawned.TryGetValue(msg.netId, out NetworkIdentity localObject) && localObject != null)
-            {
-                // this object already exists (was in the scene)
-                localObject.Reset();
-                ApplySpawnPayload(localObject, msg.position, msg.rotation, msg.scale, msg.payload, msg.netId);
-                return;
-            }
-
             NetworkIdentity spawnedId = SpawnSceneObject(msg.sceneId);
             if (spawnedId == null)
             {
@@ -565,14 +550,10 @@ internal static void OnSpawnSceneObject(NetworkConnection _, SpawnSceneObjectMes
                     foreach (KeyValuePair<ulong, NetworkIdentity> kvp in spawnableObjects)
                         Debug.Log("Spawnable: SceneId=" + kvp.Key + " name=" + kvp.Value.name);
                 }
-
-                return;
             }
 
             if (LogFilter.Debug) Debug.Log("Client spawn for [netId:" + msg.netId + "] [sceneId:" + msg.sceneId + "] obj:" + spawnedId.gameObject.name);
-            spawnedId.Reset();
-            spawnedId.pendingLocalPlayer = msg.isLocalPlayer;
-            ApplySpawnPayload(spawnedId, msg.position, msg.rotation, msg.scale, msg.payload, msg.netId);
+            return spawnedId;
         }
 
         internal static void OnObjectSpawnStarted(NetworkConnection _, ObjectSpawnStartedMessage msg)
@@ -659,15 +640,7 @@ internal static void OnLocalClientObjectHide(NetworkConnection _, ObjectHideMess
             }
         }
 
-        internal static void OnLocalClientSpawnPrefab(NetworkConnection _, SpawnPrefabMessage msg)
-        {
-            if (NetworkIdentity.spawned.TryGetValue(msg.netId, out NetworkIdentity localObject) && localObject != null)
-            {
-                localObject.OnSetLocalVisibility(true);
-            }
-        }
-
-        internal static void OnLocalClientSpawnSceneObject(NetworkConnection _, SpawnSceneObjectMessage msg)
+        internal static void OnLocalClientSpawn(NetworkConnection _, SpawnMessage msg)
         {
             if (NetworkIdentity.spawned.TryGetValue(msg.netId, out NetworkIdentity localObject) && localObject != null)
             {

Assets/Mirror/Runtime/Messages.cs

@@ -312,10 +312,11 @@ public void Serialize(NetworkWriter writer)
     #endregion
 
     #region Internal System Messages
-    public struct SpawnPrefabMessage : IMessageBase
+    public struct SpawnMessage : IMessageBase
     {
         public uint netId;
         public bool isLocalPlayer;
+        public ulong sceneId;
         public Guid assetId;
         public Vector3 position;
         public Quaternion rotation;
@@ -328,7 +329,11 @@ public void Deserialize(NetworkReader reader)
         {
             netId = reader.ReadPackedUInt32();
             isLocalPlayer = reader.ReadBoolean();
+            sceneId = reader.ReadPackedUInt64();
+            if (sceneId == 0)
+            {
                 assetId = reader.ReadGuid();
+            }
             position = reader.ReadVector3();
             rotation = reader.ReadQuaternion();
             scale = reader.ReadVector3();
@@ -339,42 +344,11 @@ public void Serialize(NetworkWriter writer)
         {
             writer.WritePackedUInt32(netId);
             writer.WriteBoolean(isLocalPlayer);
+            writer.WritePackedUInt64(sceneId);
+            if (sceneId == 0)
+            {
                 writer.WriteGuid(assetId);
-            writer.WriteVector3(position);
-            writer.WriteQuaternion(rotation);
-            writer.WriteVector3(scale);
-            writer.WriteBytesAndSizeSegment(payload);
             }
-    }
-
-    public struct SpawnSceneObjectMessage : IMessageBase
-    {
-        public uint netId;
-        public bool isLocalPlayer;
-        public ulong sceneId;
-        public Vector3 position;
-        public Quaternion rotation;
-        public Vector3 scale;
-        // the serialized component data
-        // -> ArraySegment to avoid unnecessary allocations
-        public ArraySegment<byte> payload;
-
-        public void Deserialize(NetworkReader reader)
-        {
-            netId = reader.ReadPackedUInt32();
-            isLocalPlayer = reader.ReadBoolean();
-            sceneId = reader.ReadUInt64();
-            position = reader.ReadVector3();
-            rotation = reader.ReadQuaternion();
-            scale = reader.ReadVector3();
-            payload = reader.ReadBytesAndSizeSegment();
-        }
-
-        public void Serialize(NetworkWriter writer)
-        {
-            writer.WritePackedUInt32(netId);
-            writer.WriteBoolean(isLocalPlayer);
-            writer.WriteUInt64(sceneId);
             writer.WriteVector3(position);
             writer.WriteQuaternion(rotation);
             writer.WriteVector3(scale);

Assets/Mirror/Runtime/NetworkClient.cs

@@ -344,8 +344,7 @@ internal static void RegisterSystemHandlers(bool localClient)
                 RegisterHandler<ObjectDestroyMessage>(ClientScene.OnLocalClientObjectDestroy);
                 RegisterHandler<ObjectHideMessage>(ClientScene.OnLocalClientObjectHide);
                 RegisterHandler<NetworkPongMessage>((conn, msg) => { }, false);
-                RegisterHandler<SpawnPrefabMessage>(ClientScene.OnLocalClientSpawnPrefab);
-                RegisterHandler<SpawnSceneObjectMessage>(ClientScene.OnLocalClientSpawnSceneObject);
+                RegisterHandler<SpawnMessage>(ClientScene.OnLocalClientSpawn);
                 RegisterHandler<ObjectSpawnStartedMessage>((conn, msg) => { }); // host mode doesn't need spawning
                 RegisterHandler<ObjectSpawnFinishedMessage>((conn, msg) => { }); // host mode doesn't need spawning
                 RegisterHandler<UpdateVarsMessage>((conn, msg) => { });
@@ -355,8 +354,7 @@ internal static void RegisterSystemHandlers(bool localClient)
                 RegisterHandler<ObjectDestroyMessage>(ClientScene.OnObjectDestroy);
                 RegisterHandler<ObjectHideMessage>(ClientScene.OnObjectHide);
                 RegisterHandler<NetworkPongMessage>(NetworkTime.OnClientPong, false);
-                RegisterHandler<SpawnPrefabMessage>(ClientScene.OnSpawnPrefab);
-                RegisterHandler<SpawnSceneObjectMessage>(ClientScene.OnSpawnSceneObject);
+                RegisterHandler<SpawnMessage>(ClientScene.OnSpawn);
                 RegisterHandler<ObjectSpawnStartedMessage>(ClientScene.OnObjectSpawnStarted);
                 RegisterHandler<ObjectSpawnFinishedMessage>(ClientScene.OnObjectSpawnFinished);
                 RegisterHandler<UpdateVarsMessage>(ClientScene.OnUpdateVarsMessage);

Assets/Mirror/Runtime/NetworkServer.cs

@@ -1077,7 +1077,7 @@ internal static void SendSpawnMessage(NetworkIdentity identity, NetworkConnectio
             // 'identity' is a prefab that should be spawned
             if (identity.sceneId == 0)
             {
-                SpawnPrefabMessage msg = new SpawnPrefabMessage
+                SpawnMessage msg = new SpawnMessage
                 {
                     netId = identity.netId,
                     isLocalPlayer = conn?.identity == identity,
@@ -1119,7 +1119,7 @@ internal static void SendSpawnMessage(NetworkIdentity identity, NetworkConnectio
             // 'identity' is a scene object that should be spawned again
             else
             {
-                SpawnSceneObjectMessage msg = new SpawnSceneObjectMessage
+                SpawnMessage msg = new SpawnMessage
                 {
                     netId = identity.netId,
                     isLocalPlayer = conn?.identity == identity,