Ensure objects are cleaned up on a failed spawn

Author: EmandM

com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs

@@ -972,27 +972,39 @@ internal void HandleConnectionApproval(ulong ownerClientId, bool createPlayerObj
             }
 
             // Server-side spawning (only if there is a prefab hash or player prefab provided)
-            if (!NetworkManager.DistributedAuthorityMode && createPlayerObject && (playerPrefabHash.HasValue || NetworkManager.NetworkConfig.PlayerPrefab != null))
+            var idHashToSpawn = playerPrefabHash ?? NetworkManager.NetworkConfig.PlayerPrefab?.GetComponent<NetworkObject>()?.GlobalObjectIdHash;
+            if (!NetworkManager.DistributedAuthorityMode && createPlayerObject && idHashToSpawn.HasValue)
             {
-                var playerObject = playerPrefabHash.HasValue ? NetworkManager.SpawnManager.GetNetworkObjectToSpawn(playerPrefabHash.Value, ownerClientId, playerPosition, playerRotation)
-                : NetworkManager.SpawnManager.GetNetworkObjectToSpawn(NetworkManager.NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash, ownerClientId, playerPosition, playerRotation);
+                var playerObject = NetworkManager.SpawnManager.GetNetworkObjectToSpawn(idHashToSpawn.Value, ownerClientId, playerPosition, playerRotation);
 
                 if (playerObject == null)
                 {
-                    Debug.LogError($"[{nameof(NetworkObject)}] Player prefab is null! Cannot spawn player object!");
+                    if (NetworkManager.LogLevel <= LogLevel.Error)
+                    {
+                        NetworkLog.LogError($"[{nameof(NetworkObject)}] Player prefab is null! Cannot spawn player object!");
+                    }
                 }
                 else
                 {
                     // Spawn the player NetworkObject locally
-                    NetworkManager.SpawnManager.AuthorityLocalSpawn(
+                    if (NetworkManager.SpawnManager.AuthorityLocalSpawn(
                         playerObject,
                         NetworkManager.SpawnManager.GetNetworkObjectId(),
                         sceneObject: false,
                         playerObject: true,
                         ownerClientId,
-                        destroyWithScene: false);
+                        destroyWithScene: false))
+                    {
+                        client.AssignPlayerObject(ref playerObject);
+                    }
+                    else
+                    {
+                        if (NetworkManager.LogLevel <= LogLevel.Developer)
+                        {
+                            NetworkLog.LogError($"[{nameof(NetworkObject)}] Player prefab failed to spawn!");
+                        }
+                    }
 
-                    client.AssignPlayerObject(ref playerObject);
                 }
             }
 
@@ -1122,8 +1134,25 @@ internal void CreateAndSpawnPlayer(ulong ownerId)
                 }
                 return;
             }
-            var globalObjectIdHash = playerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash;
-            var networkObject = NetworkManager.SpawnManager.GetNetworkObjectToSpawn(globalObjectIdHash, ownerId, playerPrefab.transform.position, playerPrefab.transform.rotation);
+            var prefabObject = playerPrefab.GetComponent<NetworkObject>();
+            if (prefabObject == null)
+            {
+                if (NetworkManager.LogLevel <= LogLevel.Normal)
+                {
+                    NetworkLog.LogError("Failed to fetch valid player prefab. Ensure PlayerPrefab that is set in NetcodeConfig contains a NetworkObject component.");
+                }
+                return;
+            }
+            var networkObject = NetworkManager.SpawnManager.GetNetworkObjectToSpawn(prefabObject.GlobalObjectIdHash, ownerId, playerPrefab.transform.position, playerPrefab.transform.rotation);
+            if (networkObject == null)
+            {
+                if (NetworkManager.LogLevel <= LogLevel.Normal)
+                {
+                    NetworkLog.LogError("Failed to spawn player prefab!");
+                }
+                return;
+            }
+
             networkObject.IsSceneObject = false;
             networkObject.NetworkManagerOwner = NetworkManager;
             networkObject.SpawnAsPlayerObject(ownerId, networkObject.DestroyWithScene);

com.unity.netcode.gameobjects/Runtime/Core/FindObjects.cs

@@ -21,17 +21,19 @@ internal static class FindObjects
         /// <typeparam name="T"></typeparam>
         /// <param name="includeInactive">When true, inactive objects will be included.</param>
         /// <param name="orderByIdentifier">When true, the array returned will be sorted by identifier.</param>
-        /// <returns>Resulst as an <see cref="Array"/> of type T</returns>
+        /// <returns>Results as an <see cref="Array"/> of type T</returns>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static T[] ByType<T>(bool includeInactive = false, bool orderByIdentifier = false) where T : Object
         {
             var inactive = includeInactive ? UnityEngine.FindObjectsInactive.Include : UnityEngine.FindObjectsInactive.Exclude;
 #if NGO_FINDOBJECTS_NOSORTING
             var results = Object.FindObjectsByType<T>(inactive);
+#if !NGO_FINDOBJECTS_UNORDERED_IDS
             if (orderByIdentifier)
             {
                 Array.Sort(results, (a, b) => a.GetEntityId().CompareTo(b.GetEntityId()));
             }
+#endif
 #else
             var results = Object.FindObjectsByType<T>(inactive, orderByIdentifier ? UnityEngine.FindObjectsSortMode.InstanceID : UnityEngine.FindObjectsSortMode.None);
 #endif

com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

@@ -1807,7 +1807,15 @@ internal void SpawnInternal(bool destroyWithScene, ulong ownerClientId, bool pla
                 }
             }
 
-            NetworkManagerOwner.SpawnManager.AuthorityLocalSpawn(this, NetworkManagerOwner.SpawnManager.GetNetworkObjectId(), IsSceneObject.HasValue && IsSceneObject.Value, playerObject, ownerClientId, destroyWithScene);
+            if (!NetworkManagerOwner.SpawnManager.AuthorityLocalSpawn(this, NetworkManagerOwner.SpawnManager.GetNetworkObjectId(), IsSceneObject.HasValue && IsSceneObject.Value, playerObject, ownerClientId, destroyWithScene))
+            {
+                if (NetworkManagerOwner.LogLevel <= LogLevel.Normal)
+                {
+                    NetworkLog.LogWarning($"[{name}] Failed to finish spawning!");
+                }
+                ResetOnDespawn();
+                return;
+            }
 
             if ((NetworkManagerOwner.DistributedAuthorityMode && NetworkManagerOwner.DAHost) || (!NetworkManagerOwner.DistributedAuthorityMode && NetworkManagerOwner.IsServer))
             {
@@ -1865,7 +1873,7 @@ public static NetworkObject InstantiateAndSpawn(GameObject networkPrefab, Networ
         /// <summary>
         /// This invokes <see cref="NetworkSpawnManager.InstantiateAndSpawn(NetworkObject, ulong, bool, bool, bool, Vector3, Quaternion)"/>.
         /// </summary>
-        /// <param name="networkManager">The local instance of the NetworkManager connected to an session in progress.</param>
+        /// <param name="networkManager">The local instance of the NetworkManager connected to a session in progress.</param>
         /// <param name="ownerClientId">The owner of the <see cref="NetworkObject"/> instance (defaults to server).</param>
         /// <param name="destroyWithScene">Whether the <see cref="NetworkObject"/> instance will be destroyed when the scene it is located within is unloaded (default is false).</param>
         /// <param name="isPlayerObject">Whether the <see cref="NetworkObject"/> instance is a player object or not (default is false).</param>
@@ -2161,6 +2169,11 @@ internal void SetNetworkParenting(ulong? latestParent, bool worldPositionStays)
             m_CachedWorldPositionStays = worldPositionStays;
         }
 
+        internal void ClearNetworkParenting()
+        {
+            m_LatestParent = null;
+        }
+
         /// <summary>
         /// Set the parent of the NetworkObject transform.
         /// </summary>
@@ -3304,90 +3317,35 @@ internal static NetworkObject Deserialize(in SerializedObject serializedObject,
         {
             var endOfSynchronizationData = reader.Position + serializedObject.SynchronizationDataSize;
 
-            byte[] instantiationData = null;
-            if (serializedObject.HasInstantiationData)
-            {
-                reader.ReadValueSafe(out instantiationData);
-            }
-
-
-            // Attempt to create a local NetworkObject
-            var networkObject = networkManager.SpawnManager.CreateLocalNetworkObject(serializedObject, instantiationData);
-
-
-            if (networkObject == null)
-            {
-                // Log the error that the NetworkObject failed to construct
-                if (networkManager.LogLevel <= LogLevel.Normal)
-                {
-                    NetworkLog.LogError($"Failed to spawn {nameof(NetworkObject)} for Hash {serializedObject.Hash}.");
-                }
-
-                try
-                {
-                    // If we failed to load this NetworkObject, then skip past the Network Variable and (if any) synchronization data
-                    reader.Seek(endOfSynchronizationData);
-                }
-                catch (Exception ex)
-                {
-                    Debug.LogException(ex);
-                }
+            // Do the SpawnManager parts of the object spawn
+            var succeeded = networkManager.SpawnManager.NonAuthorityLocalSpawn(in serializedObject, out var networkObject, reader, serializedObject.DestroyWithScene);
 
-                // We have nothing left to do here.
-                return null;
-            }
-
-            networkObject.NetworkManagerOwner = networkManager;
-
-            // This will get set again when the NetworkObject is spawned locally, but we set it here ahead of spawning
-            // in order to be able to determine which NetworkVariables the client will be allowed to read.
-            networkObject.OwnerClientId = serializedObject.OwnerClientId;
+            // Process any deferred messages once the object is 100% finished spawning
+            // Ensure this is done whether the spawn succeeds or fails
+            networkManager.DeferredMessageManager.ProcessTriggers(IDeferredNetworkMessageManager.TriggerType.OnSpawn, networkObject.NetworkObjectId);
 
-            // Special Case: Invoke NetworkBehaviour.OnPreSpawn methods here before SynchronizeNetworkBehaviours
-            networkObject.InvokeBehaviourNetworkPreSpawn();
-
-            // Process the remaining synchronization data from the buffer
-            try
+            // If the SpawnManager spawn doesn't succeed, be sure to clean up
+            if (!succeeded)
             {
-                // Synchronize NetworkBehaviours
-                var bufferSerializer = new BufferSerializer<BufferSerializerReader>(new BufferSerializerReader(reader));
-                networkObject.SynchronizeNetworkBehaviours(ref bufferSerializer, networkManager.LocalClientId);
-
                 // Ensure that the buffer is completely reset
                 if (reader.Position != endOfSynchronizationData)
                 {
-                    Debug.LogWarning($"[Size mismatch] Expected: {endOfSynchronizationData} Currently At: {reader.Position}!");
+                    if (networkManager.LogLevel <= LogLevel.Normal)
+                    {
+                        NetworkLog.LogWarning($"[{networkObject.name}][Deserialize][Size mismatch] Expected: {endOfSynchronizationData} Currently At: {reader.Position}!");
+                    }
                     reader.Seek(endOfSynchronizationData);
                 }
-            }
-            catch
-            {
-                reader.Seek(endOfSynchronizationData);
-            }
 
-            // If we are an in-scene placed NetworkObject and we originally had a parent but when synchronized we are
-            // being told we do not have a parent, then we want to clear the latest parent so it is not automatically
-            // "re-parented" to the original parent. This can happen if not unloading the scene and the parenting of
-            // the in-scene placed Networkobject changes several times over different sessions.
-            if (serializedObject.IsSceneObject && !serializedObject.HasParent && networkObject.m_LatestParent.HasValue)
-            {
-                networkObject.m_LatestParent = null;
-            }
-
-            // Spawn the NetworkObject
-            if (networkObject.IsSpawned)
-            {
-                if (NetworkManager.Singleton.LogLevel <= LogLevel.Error)
+                // If the networkObject was created but the spawn failed, the created object needs to be destroyed
+                if (networkObject != null)
                 {
-                    NetworkLog.LogErrorServer($"[{networkObject.name}] Object-{networkObject.NetworkObjectId} is already spawned!");
+                    Destroy(networkObject.gameObject);
                 }
+
                 return null;
             }
 
-            // Invoke the non-authority local spawn method
-            // (It also invokes post spawn and handles processing derferred messages)
-            networkManager.SpawnManager.NonAuthorityLocalSpawn(networkObject, serializedObject, serializedObject.DestroyWithScene);
-
             if (serializedObject.SyncObservers)
             {
                 foreach (var observer in serializedObject.Observers)