fix: NullReferenceExceptions in spawn path (#4067)

* fix: NullReferenceExceptions in spawn path

* Update NetcodeIntegrationTest

* Update CHANGELOG.md

* put back serialization code

Author: EmandM

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -22,6 +22,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Issue where a NullReferenceException was thrown when a non-authority failed to spawn a NetworkObject. (#4067)
 - Issue when FastBufferReader is attempting to read a string and all or a portion of the character count has already been read by user script, it could read a character length that results in a negative byte length which could result in an editor crash. (#4052)
 - Issue where NetworkRigidbodyBase was not applying rotation correctly when using Rigidbody2D. (#4012)
 - Issue where NetworkRigidbodyBase was always checking the 3D rigid body's interpolation mode when determining if it is kinematic and needs to put the rigid body to sleep and then switch to interpolation. (#4012)

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

@@ -1218,7 +1218,7 @@ internal void ApprovedPlayerSpawn(ulong clientId, uint playerPrefabHash)
 
                 var message = new CreateObjectMessage
                 {
-                    ObjectInfo = ConnectedClients[clientId].PlayerObject.Serialize(clientPair.Key),
+                    ObjectInfo = ConnectedClients[clientId].PlayerObject.SerializeSpawnedObject(clientPair.Key),
                     IncludesSerializedObject = true,
                 };
 

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

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Runtime.CompilerServices;
 using System.Text;
@@ -3369,7 +3370,12 @@ internal void SynchronizeNetworkBehaviours<T>(ref BufferSerializer<T> serializer
             }
         }
 
-        internal SerializedObject Serialize(ulong targetClientId = NetworkManager.ServerClientId, bool syncObservers = false)
+        /// <summary>
+        /// Creates a <see cref="SerializedObject"/> on an authority client.
+        /// Used to synchronize <see cref="NetworkObject"/> state to a non-authority client.
+        /// </summary>
+        /// <remarks>This function is the authority mirror of <see cref="DeserializeAndSpawnObject"/></remarks>
+        internal SerializedObject SerializeSpawnedObject(ulong targetClientId = NetworkManager.ServerClientId, bool syncObservers = false)
         {
             var obj = new SerializedObject
             {
@@ -3444,15 +3450,17 @@ internal SerializedObject Serialize(ulong targetClientId = NetworkManager.Server
         }
 
         /// <summary>
-        /// Used to deserialize a serialized <see cref="SerializedObject"/> which occurs
-        /// when the client is approved or during a scene transition
+        /// Does a non-authority local spawn of a given <see cref="SerializedObject"/>.
+        /// This occurs when the client is approved, a new object is spawned by an authority, or during a scene transition.
         /// </summary>
-        /// <param name="serializedObject">Deserialized scene object data</param>
-        /// <param name="reader">FastBufferReader for the NetworkVariable data</param>
-        /// <param name="networkManager">NetworkManager instance</param>
+        /// <remarks>This function is the non-authority mirror of <see cref="SerializeSpawnedObject"/></remarks>
+        /// <param name="serializedObject">Deserialized data received from the authority for this <see cref="NetworkObject"/></param>
+        /// <param name="reader">FastBufferReader for any additional data sent with this object on spawn.</param>
+        /// <param name="networkManager">NetworkManager instance.</param>
         /// <param name="invokedByMessage">will be true if invoked by CreateObjectMessage</param>
         /// <returns>The deserialized NetworkObject or null if deserialization failed</returns>
-        internal static NetworkObject Deserialize(in SerializedObject serializedObject, FastBufferReader reader, NetworkManager networkManager, bool invokedByMessage = false)
+        [return: MaybeNull]
+        internal static NetworkObject DeserializeAndSpawnObject(in SerializedObject serializedObject, FastBufferReader reader, NetworkManager networkManager, bool invokedByMessage = false)
         {
             var endOfSynchronizationData = reader.Position + serializedObject.SynchronizationDataSize;
 
@@ -3479,7 +3487,8 @@ internal static NetworkObject Deserialize(in SerializedObject serializedObject,
             {
                 if (networkManager.LogLevel <= LogLevel.Normal)
                 {
-                    NetworkLog.LogWarning($"[{networkObject.name}][Deserialize][{nameof(NetworkBehaviour)}Synchronization][Size mismatch] Expected: {endOfSynchronizationData} Currently At: {reader.Position}!");
+                    var networkObjectName = networkObject != null ? networkObject.name : "null";
+                    NetworkLog.LogWarning($"[{networkObjectName}][Deserialize][{nameof(NetworkBehaviour)}Synchronization][Size mismatch] Expected: {endOfSynchronizationData} Currently At: {reader.Position}!");
                 }
                 reader.Seek(endOfSynchronizationData);
             }

com.unity.netcode.gameobjects/Runtime/Messaging/ILPPMessageProvider.cs

@@ -148,6 +148,7 @@ internal static Dictionary<Type, NetworkMessageTypes> GetMessageTypesMap()
         [InitializeOnLoadMethod]
         public static void NotifyOnPlayStateChange()
         {
+            EditorApplication.playModeStateChanged -= OnPlayModeStateChanged;
             EditorApplication.playModeStateChanged += OnPlayModeStateChanged;
         }
 

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs

@@ -160,7 +160,7 @@ public void Serialize(FastBufferWriter writer, int targetVersion)
                     {
                         sobj.AddObserver(OwnerClientId);
                         // In distributed authority mode, we send the currently known observers of each NetworkObject to the client being synchronized.
-                        var serializedObject = sobj.Serialize(OwnerClientId, IsDistributedAuthority);
+                        var serializedObject = sobj.SerializeSpawnedObject(OwnerClientId, IsDistributedAuthority);
                         serializedObject.Serialize(writer);
                         ++sceneObjectCount;
                     }
@@ -344,7 +344,7 @@ public void Handle(ref NetworkContext context)
                 {
                     var serializedObject = new NetworkObject.SerializedObject();
                     serializedObject.Deserialize(m_ReceivedSceneObjectData);
-                    NetworkObject.Deserialize(serializedObject, m_ReceivedSceneObjectData, networkManager);
+                    NetworkObject.DeserializeAndSpawnObject(serializedObject, m_ReceivedSceneObjectData, networkManager);
                 }
 
                 if (networkManager.DistributedAuthorityMode && networkManager.AutoSpawnPlayerPrefabClientSide)

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/CreateObjectMessage.cs

@@ -1,5 +1,6 @@
 using System.Linq;
 using System.Runtime.CompilerServices;
+using Unity.Netcode.Logging;
 
 namespace Unity.Netcode
 {
@@ -171,33 +172,40 @@ internal static void CreateObject(ref NetworkManager networkManager, ulong sende
             {
                 if (!networkManager.DistributedAuthorityMode)
                 {
-                    networkObject = NetworkObject.Deserialize(serializedObject, networkVariableData, networkManager);
+                    networkObject = NetworkObject.DeserializeAndSpawnObject(serializedObject, networkVariableData, networkManager);
+                    if (networkObject == null)
+                    {
+                        networkManager.Log.ErrorServer(new Context(LogLevel.Developer, $"Failed to deserialize {nameof(NetworkObject)}.").AddInfo(nameof(NetworkObject.GlobalObjectIdHash), serializedObject.Hash).AddInfo(nameof(NetworkObject.NetworkObjectId), serializedObject.NetworkObjectId));
+                        return;
+                    }
                 }
                 else
                 {
                     var hasObserverIdList = observerIds != null && observerIds.Length > 0;
                     var hasNewObserverIdList = newObserverIds != null && newObserverIds.Length > 0;
                     // Depending upon visibility of the NetworkObject and the client in question, it could be that
                     // this client already has visibility of this NetworkObject
-                    if (networkManager.SpawnManager.SpawnedObjects.ContainsKey(serializedObject.NetworkObjectId))
+                    if (networkManager.SpawnManager.SpawnedObjects.TryGetValue(serializedObject.NetworkObjectId, out networkObject))
                     {
-                        // If so, then just get the local instance
-                        networkObject = networkManager.SpawnManager.SpawnedObjects[serializedObject.NetworkObjectId];
-
                         // This should not happen, logging error just in case
                         if (hasNewObserverIdList && newObserverIds.Contains(networkManager.LocalClientId))
                         {
                             NetworkLog.LogErrorServer($"[{nameof(CreateObjectMessage)}][Duplicate-Broadcast] Detected duplicated object creation for {serializedObject.NetworkObjectId}!");
                         }
-                        else // Trap to make sure the owner is not receiving any messages it sent
-                            if (networkManager.CMBServiceConnection && networkManager.LocalClientId == networkObject.OwnerClientId)
-                            {
-                                NetworkLog.LogWarning($"[{nameof(CreateObjectMessage)}][Client-{networkManager.LocalClientId}][Duplicate-CreateObjectMessage][Client Is Owner] Detected duplicated object creation for {networkObject.name}-{serializedObject.NetworkObjectId}!");
-                            }
+                        // Trap to make sure the owner is not receiving any messages it sent
+                        else if (networkManager.CMBServiceConnection && networkManager.LocalClientId == networkObject.OwnerClientId)
+                        {
+                            NetworkLog.LogWarning($"[{nameof(CreateObjectMessage)}][Client-{networkManager.LocalClientId}][Duplicate-CreateObjectMessage][Client Is Owner] Detected duplicated object creation for {networkObject.name}-{serializedObject.NetworkObjectId}!");
+                        }
                     }
                     else
                     {
-                        networkObject = NetworkObject.Deserialize(serializedObject, networkVariableData, networkManager, true);
+                        networkObject = NetworkObject.DeserializeAndSpawnObject(serializedObject, networkVariableData, networkManager, true);
+                        if (networkObject == null)
+                        {
+                            networkManager.Log.ErrorServer(new Context(LogLevel.Developer, $"Failed to deserialize {nameof(NetworkObject)}.").AddInfo(nameof(NetworkObject.GlobalObjectIdHash), serializedObject.Hash).AddInfo(nameof(NetworkObject.NetworkObjectId), serializedObject.NetworkObjectId));
+                            return;
+                        }
                     }
 
                     // DA - NGO CMB SERVICE NOTES:
@@ -214,27 +222,6 @@ internal static void CreateObject(ref NetworkManager networkManager, ulong sende
                     // Mock CMB Service and forward to all clients
                     if (networkManager.DAHost)
                     {
-                        // DA - NGO CMB SERVICE NOTES:
-                        // (*** See above notes fist ***)
-                        // If it is a player object freshly spawning and one or more clients all connect at the exact same time (i.e. received on effectively
-                        // the same frame), then we need to check the observers list to make sure all players are visible upon first spawning. At a later date,
-                        // for area of interest we will need to have some form of follow up "observer update" message to cull out players not within each
-                        // player's AOI.
-                        if (networkObject.IsPlayerObject && hasNewObserverIdList && clientList.Count != observerIds.Length)
-                        {
-                            // For same-frame newly spawned players that might not be aware of all other players, update the player's observer
-                            // list.
-                            observerIds = clientList.ToArray();
-                        }
-
-                        var createObjectMessage = new CreateObjectMessage()
-                        {
-                            ObjectInfo = serializedObject,
-                            m_ReceivedNetworkVariableData = networkVariableData,
-                            ObserverIds = hasObserverIdList ? observerIds : null,
-                            NetworkObjectId = networkObject.NetworkObjectId,
-                            IncludesSerializedObject = true,
-                        };
                         foreach (var clientId in clientList)
                         {
                             // DA - NGO CMB SERVICE NOTES:
@@ -250,16 +237,12 @@ internal static void CreateObject(ref NetworkManager networkManager, ulong sende
                             // If this included a list of new observers and the targeted clientId is one of the observers, then send the serialized data.
                             // Otherwise, the targeted clientId has already has visibility (i.e. it is already spawned) and so just send the updated
                             // observers list to that client's instance.
-                            createObjectMessage.IncludesSerializedObject = hasNewObserverIdList && newObserverIds.Contains(clientId);
-
                             networkManager.SpawnManager.SendSpawnCallForObject(clientId, networkObject);
                         }
                     }
                 }
-                if (networkObject != null)
-                {
-                    networkManager.NetworkMetrics.TrackObjectSpawnReceived(senderId, networkObject, messageSize);
-                }
+
+                networkManager.NetworkMetrics.TrackObjectSpawnReceived(senderId, networkObject, messageSize);
             }
             catch (System.Exception ex)
             {

com.unity.netcode.gameobjects/Runtime/SceneManagement/SceneEventData.cs

@@ -622,7 +622,7 @@ private void WriteSceneSynchronizationData(FastBufferWriter writer)
             {
                 var noStart = writer.Position;
                 // In distributed authority mode, we send the currently known observers of each NetworkObject to the client being synchronized.
-                var serializedObject = networkObject.Serialize(TargetClientId, distributedAuthority);
+                var serializedObject = networkObject.SerializeSpawnedObject(TargetClientId, distributedAuthority);
 
                 serializedObject.Serialize(writer);
                 var noStop = writer.Position;
@@ -698,7 +698,7 @@ private void SerializeScenePlacedObjects(FastBufferWriter writer)
             foreach (var objectToSync in m_NetworkObjectsSync)
             {
                 // Serialize the NetworkObject
-                var serializedObject = objectToSync.Serialize(TargetClientId, distributedAuthority);
+                var serializedObject = objectToSync.SerializeSpawnedObject(TargetClientId, distributedAuthority);
                 serializedObject.Serialize(writer);
                 numberOfObjects++;
             }
@@ -875,9 +875,9 @@ internal void DeserializeScenePlacedObjects()
                         m_NetworkManager.SceneManager.SetTheSceneBeingSynchronized(serializedObject.NetworkSceneHandle);
                     }
 
-                    var networkObject = NetworkObject.Deserialize(serializedObject, m_InternalBuffer, m_NetworkManager);
+                    var networkObject = NetworkObject.DeserializeAndSpawnObject(serializedObject, m_InternalBuffer, m_NetworkManager);
 
-                    if (serializedObject.IsSceneObject)
+                    if (serializedObject.IsSceneObject && networkObject != null)
                     {
                         sceneObjects.Add(networkObject);
                     }
@@ -1101,7 +1101,11 @@ internal void SynchronizeSceneNetworkObjects(NetworkManager networkManager)
                     {
                         m_NetworkManager.SceneManager.SetTheSceneBeingSynchronized(serializedObject.NetworkSceneHandle);
                     }
-                    var spawnedNetworkObject = NetworkObject.Deserialize(serializedObject, m_InternalBuffer, networkManager);
+                    var spawnedNetworkObject = NetworkObject.DeserializeAndSpawnObject(serializedObject, m_InternalBuffer, networkManager);
+                    if (spawnedNetworkObject == null)
+                    {
+                        continue;
+                    }
 
                     var noStop = m_InternalBuffer.Position;
                     if (EnableSerializationLogs)
@@ -1110,12 +1114,9 @@ internal void SynchronizeSceneNetworkObjects(NetworkManager networkManager)
                         LogArray(m_InternalBuffer.ToArray(), noStart, noStop, builder);
                     }
                     // If we failed to deserialize the NetworkObject then don't add null to the list
-                    if (spawnedNetworkObject != null)
+                    if (!m_NetworkObjectsSync.Contains(spawnedNetworkObject))
                     {
-                        if (!m_NetworkObjectsSync.Contains(spawnedNetworkObject))
-                        {
-                            m_NetworkObjectsSync.Add(spawnedNetworkObject);
-                        }
+                        m_NetworkObjectsSync.Add(spawnedNetworkObject);
                     }
                 }
                 if (EnableSerializationLogs)

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

@@ -1165,13 +1165,13 @@ internal bool AuthorityLocalSpawn([NotNull] NetworkObject networkObject, ulong n
         /// <summary>
         /// Only spawn non-authority <see cref="NetworkManager"/> instances should invoke this.
         /// This is invoked to instantiate an authority spawned <see cref="NetworkObject"/>, and
-        /// is only invoked by: <see cref="NetworkObject.AddSceneObject(in NetworkObject.SceneObject, FastBufferReader, NetworkManager, bool)"/>
+        /// is only invoked by: <see cref="NetworkObject.DeserializeAndSpawnObject"/>
         /// </summary>
         /// <remarks>
-        /// IMPORTANT: Pre spawn methods need to be invoked from within <see cref="NetworkObject.Deserialize"/>.
+        /// IMPORTANT: Pre spawn methods need to be invoked from within <see cref="NetworkObject.DeserializeAndSpawnObject"/>.
         /// </remarks>
         /// <returns>boolean indicating whether the spawn succeeded</returns>
-        internal bool NonAuthorityLocalSpawn(in NetworkObject.SerializedObject serializedObject, out NetworkObject networkObject, FastBufferReader reader, bool destroyWithScene)
+        internal bool NonAuthorityLocalSpawn(in NetworkObject.SerializedObject serializedObject, [MaybeNullWhen(false)] out NetworkObject networkObject, FastBufferReader reader, bool destroyWithScene)
         {
             if (SpawnedObjects.ContainsKey(serializedObject.NetworkObjectId))
             {
@@ -1368,7 +1368,7 @@ internal void SendSpawnCallForObject(ulong clientId, NetworkObject networkObject
             }
             var message = new CreateObjectMessage
             {
-                ObjectInfo = networkObject.Serialize(clientId, NetworkManager.DistributedAuthorityMode),
+                ObjectInfo = networkObject.SerializeSpawnedObject(clientId, NetworkManager.DistributedAuthorityMode),
                 IncludesSerializedObject = true,
                 UpdateObservers = NetworkManager.DistributedAuthorityMode,
                 ObserverIds = NetworkManager.DistributedAuthorityMode ? networkObject.Observers.ToArray() : null,
@@ -1394,7 +1394,7 @@ internal void SendSpawnCallForObserverUpdate(ulong[] newObservers, NetworkObject
 
             var message = new CreateObjectMessage
             {
-                ObjectInfo = networkObject.Serialize(),
+                ObjectInfo = networkObject.SerializeSpawnedObject(),
                 ObserverIds = networkObject.Observers.ToArray(),
                 NewObserverIds = newObservers.ToArray(),
                 IncludesSerializedObject = true,