fix: CreateObjectMessage created NetworkObjects can get destroyed during client synchronization [MTT-7539] (#2735)

* fix

This resolves the issue where any NetworkObjects created during client synchronization when scene management is enabled. client synchronization mode is set to LoadSceneMode.Single, and the active scene (where the NetworkObjects are created) is unloaded during synchronization.

* fix

This fixes the exception that can occur when there are many (100's) NetworkObjects spawned and the application instance is exited (or you exit play mode in the editor).

* style

removing auto-added namespace

* test

Updating WhenManyObjectsAreSpawnedAtOnce_AllAreReceived test as it was not using the right logic to validate all NetworkObjects had been spawned.

Adding WhenManyObjectsAreSpawnedDuringSynchronization_AllAreReceived to validate this PR.

* update

adding changelog entry

* test

Migrating WhenManyObjectsAreSpawnedDuringSynchronization_AllAreReceived  to SpawnNetworkObjectsDuringSynchronizationTest.SpawnNetworkObjectsDuringSynchronization.

* fix

Scenes loaded in single mode, after a client is connected, will defer CreateObjectMessage driven spawning until the scene loading event is completed. Consolidated the logic into a single method.

Added two new SceneEventProgress enums to be returned when a client attempts to load or unload a scene using NetworkSceneManager or any instance type tries to use the NetworkSceneManager when scene management is disabled (as opposed to throwing an exception it just logs a warning and returns the relative SceneEventProgress value).

* test

Made modifications due to SceneEventProgress adjustments.
Renamed SpawnNetworkObjectsDuringSynchronizationTest to SpawnNetworkObjectsDuringSceneEventsTest and migrated it to the test project tests runtime NetworkSceneManager  folder.
Added an additional

* update

Updated the change log entry.

* style

removing unused namespace.

* Update CHANGELOG.md

* Update CHANGELOG.md

Fixing bad changelog merge

* Update SceneEventProgress.cs

Fixing the issue with the enum changes breaking the InternalNetcodeError value

* Update SceneEventProgress.cs

* update

Made adjustments based on PR review suggestions.
Added additional changelog entries to better reflect the PR.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -10,9 +10,13 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Added
 
+- Added `SceneEventProgress.SceneManagementNotEnabled` return status to be returned when a `NetworkSceneManager` method is invoked and scene management is not enabled. (#2735)
+- Added `SceneEventProgress.ServerOnlyAction` return status to be returned when a `NetworkSceneManager` method is invoked by a client. (#2735)
+
 ### Fixed
 
 - Fixed a bug where having a class with Rpcs that inherits from a class without Rpcs that inherits from NetworkVariable would cause a compile error. (#2751)
+- Fixed issue where during client synchronization and scene loading, when client synchronization or the scene loading mode are set to `LoadSceneMode.Single`, a `CreateObjectMessage` could be received, processed, and the resultant spawned `NetworkObject` could be instantiated in the client's currently active scene that could, towards the end of the client synchronization or loading process, be unloaded and cause the newly created `NetworkObject` to be destroyed (and throw and exception). (#2735)
 - Fixed issue where `NetworkBehaviour.Synchronize` was not truncating the write buffer if nothing was serialized during `NetworkBehaviour.OnSynchronize` causing an additional 6 bytes to be written per `NetworkBehaviour` component instance. (#2749)
 - Fixed issue where a parented in-scene placed NetworkObject would be destroyed upon a client or server exiting a network session but not unloading the original scene in which the NetworkObject was placed. (#2737)
 - Fixed issue where during client synchronization and scene loading, when client synchronization or the scene loading mode are set to `LoadSceneMode.Single`, a `CreateObjectMessage` could be received, processed, and the resultant spawned `NetworkObject` could be instantiated in the client's currently active scene that could, towards the end of the client synchronization or loading process, be unloaded and cause the newly created `NetworkObject` to be destroyed (and throw and exception). (#2735)
@@ -21,6 +25,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Changed
 - Changed `NetworkTransform` authoritative instance tick registration so a single `NetworkTransform` specific tick event update will update all authoritative instances to improve perofmance. (#2713)
 
+- Changed `NetworkSceneManager` to return a `SceneEventProgress` status and not throw exceptions for methods invoked when scene management is disabled and when a client attempts to access a `NetworkSceneManager` method by a client. (#2735)
+
 ## [1.7.0] - 2023-10-11
 
 ### Added

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

@@ -1,3 +1,4 @@
+using System.Runtime.CompilerServices;
 namespace Unity.Netcode
 {
     internal struct CreateObjectMessage : INetworkMessage
@@ -34,9 +35,29 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
         public void Handle(ref NetworkContext context)
         {
             var networkManager = (NetworkManager)context.SystemOwner;
-            var networkObject = NetworkObject.AddSceneObject(ObjectInfo, m_ReceivedNetworkVariableData, networkManager);
+            // If a client receives a create object message and it is still synchronizing, then defer the object creation until it has finished synchronizing
+            if (networkManager.SceneManager.ShouldDeferCreateObject())
+            {
+                networkManager.SceneManager.DeferCreateObject(context.SenderId, context.MessageSize, ObjectInfo, m_ReceivedNetworkVariableData);
+            }
+            else
+            {
+                CreateObject(ref networkManager, context.SenderId, context.MessageSize, ObjectInfo, m_ReceivedNetworkVariableData);
+            }
+        }
 
-            networkManager.NetworkMetrics.TrackObjectSpawnReceived(context.SenderId, networkObject, context.MessageSize);
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal static void CreateObject(ref NetworkManager networkManager, ulong senderId, uint messageSize, NetworkObject.SceneObject sceneObject, FastBufferReader networkVariableData)
+        {
+            try
+            {
+                var networkObject = NetworkObject.AddSceneObject(sceneObject, networkVariableData, networkManager);
+                networkManager.NetworkMetrics.TrackObjectSpawnReceived(senderId, networkObject, messageSize);
+            }
+            catch (System.Exception ex)
+            {
+                UnityEngine.Debug.LogException(ex);
+            }
         }
     }
 }

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

@@ -25,9 +25,9 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
         public void Handle(ref NetworkContext context)
         {
             var networkManager = (NetworkManager)context.SystemOwner;
-            if (!networkManager.ShutdownInProgress)
+            if (!networkManager.ShutdownInProgress && networkManager.CustomMessagingManager != null)
             {
-                ((NetworkManager)context.SystemOwner).CustomMessagingManager.InvokeNamedMessage(Hash, context.SenderId, m_ReceiveData, context.SerializedHeaderSize);
+                networkManager.CustomMessagingManager.InvokeNamedMessage(Hash, context.SenderId, m_ReceiveData, context.SerializedHeaderSize);
             }
         }
     }

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

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using Unity.Collections;
 using UnityEngine;
 using UnityEngine.SceneManagement;
 
@@ -597,6 +598,47 @@ internal void EndSceneEvent(uint sceneEventId)
             }
         }
 
+        /// <summary>
+        /// Used for integration tests, normal runtime mode this will always be LoadSceneMode.Single
+        /// </summary>
+        internal LoadSceneMode DeferLoadingFilter = LoadSceneMode.Single;
+        /// <summary>
+        /// Determines if a remote client should defer object creation initiated by CreateObjectMessage
+        /// until a scene event is completed.
+        /// </summary>
+        /// <remarks>
+        /// Deferring object creation should only occur when there is a possibility the objects could be
+        /// instantiated in a currently active scene that will be unloaded during single mode scene loading
+        /// to prevent the newly created objects from being destroyed when the scene is unloaded.
+        /// </remarks>
+        internal bool ShouldDeferCreateObject()
+        {
+            // This applies only to remote clients and when scene management is enabled
+            if (!NetworkManager.NetworkConfig.EnableSceneManagement || NetworkManager.IsServer)
+            {
+                return false;
+            }
+            var synchronizeEventDetected = false;
+            var loadingEventDetected = false;
+            foreach (var entry in SceneEventDataStore)
+            {
+                if (entry.Value.SceneEventType == SceneEventType.Synchronize)
+                {
+                    synchronizeEventDetected = true;
+                }
+
+                // When loading a scene and the load scene mode is single we should defer object creation
+                if (entry.Value.SceneEventType == SceneEventType.Load && entry.Value.LoadSceneMode == DeferLoadingFilter)
+                {
+                    loadingEventDetected = true;
+                }
+            }
+
+            // Synchronizing while in client synchronization mode single --> Defer
+            // When not synchronizing but loading a scene in single mode --> Defer
+            return (synchronizeEventDetected && ClientSynchronizationMode == LoadSceneMode.Single) || (!synchronizeEventDetected && loadingEventDetected);
+        }
+
         /// <summary>
         /// Gets the scene name from full path to the scene
         /// </summary>
@@ -969,17 +1011,16 @@ private void SendSceneEventData(uint sceneEventId, ulong[] targetClientIds)
         /// <returns></returns>
         private SceneEventProgress ValidateSceneEventUnloading(Scene scene)
         {
-            if (!NetworkManager.IsServer)
+            if (!NetworkManager.NetworkConfig.EnableSceneManagement)
             {
-                throw new NotServerException("Only server can start a scene event!");
+                Debug.LogWarning($"{nameof(LoadScene)} was called, but {nameof(NetworkConfig.EnableSceneManagement)} was not enabled! Enable {nameof(NetworkConfig.EnableSceneManagement)} prior to starting a client, host, or server prior to using {nameof(NetworkSceneManager)}!");
+                return new SceneEventProgress(null, SceneEventProgressStatus.SceneManagementNotEnabled);
             }
 
-            if (!NetworkManager.NetworkConfig.EnableSceneManagement)
+            if (!NetworkManager.IsServer)
             {
-                //Log message about enabling SceneManagement
-                throw new Exception(
-                    $"{nameof(NetworkConfig.EnableSceneManagement)} flag is not enabled in the {nameof(Netcode.NetworkManager)}'s {nameof(NetworkConfig)}. " +
-                    $"Please set {nameof(NetworkConfig.EnableSceneManagement)} flag to true before calling {nameof(LoadScene)} or {nameof(UnloadScene)}.");
+                Debug.LogWarning($"[{nameof(SceneEventProgressStatus.ServerOnlyAction)}][Unload] Clients cannot invoke the {nameof(UnloadScene)} method!");
+                return new SceneEventProgress(null, SceneEventProgressStatus.ServerOnlyAction);
             }
 
             if (!scene.isLoaded)
@@ -998,16 +1039,16 @@ private SceneEventProgress ValidateSceneEventUnloading(Scene scene)
         /// <returns></returns>
         private SceneEventProgress ValidateSceneEventLoading(string sceneName)
         {
-            if (!NetworkManager.IsServer)
+            if (!NetworkManager.NetworkConfig.EnableSceneManagement)
             {
-                throw new NotServerException("Only server can start a scene event!");
+                Debug.LogWarning($"{nameof(LoadScene)} was called, but {nameof(NetworkConfig.EnableSceneManagement)} was not enabled! Enable {nameof(NetworkConfig.EnableSceneManagement)} prior to starting a client, host, or server prior to using {nameof(NetworkSceneManager)}!");
+                return new SceneEventProgress(null, SceneEventProgressStatus.SceneManagementNotEnabled);
             }
-            if (!NetworkManager.NetworkConfig.EnableSceneManagement)
+
+            if (!NetworkManager.IsServer)
             {
-                //Log message about enabling SceneManagement
-                throw new Exception(
-                    $"{nameof(NetworkConfig.EnableSceneManagement)} flag is not enabled in the {nameof(Netcode.NetworkManager)}'s {nameof(NetworkConfig)}. " +
-                    $"Please set {nameof(NetworkConfig.EnableSceneManagement)} flag to true before calling {nameof(LoadScene)} or {nameof(UnloadScene)}.");
+                Debug.LogWarning($"[{nameof(SceneEventProgressStatus.ServerOnlyAction)}][Load] Clients cannot invoke the {nameof(LoadScene)} method!");
+                return new SceneEventProgress(null, SceneEventProgressStatus.ServerOnlyAction);
             }
 
             return ValidateSceneEvent(sceneName);
@@ -1112,6 +1153,7 @@ public SceneEventProgressStatus UnloadScene(Scene scene)
         {
             var sceneName = scene.name;
             var sceneHandle = scene.handle;
+
             if (!scene.isLoaded)
             {
                 Debug.LogWarning($"{nameof(UnloadScene)} was called, but the scene {scene.name} is not currently loaded!");
@@ -1697,6 +1739,9 @@ private void OnClientLoadedScene(uint sceneEventId, Scene scene)
             SendSceneEventData(sceneEventId, new ulong[] { NetworkManager.ServerClientId });
             m_IsSceneEventActive = false;
 
+            // Process any pending create object messages that the client received while loading a scene
+            ProcessDeferredCreateObjectMessages();
+
             // Notify local client that the scene was loaded
             OnSceneEvent?.Invoke(new SceneEvent()
             {
@@ -2058,6 +2103,9 @@ private void HandleClientSceneEvent(uint sceneEventId)
                             // If needed, migrate dynamically spawned NetworkObjects to the same scene as they are on the server
                             SynchronizeNetworkObjectScene();
 
+                            // Process any pending create object messages that the client received during synchronization
+                            ProcessDeferredCreateObjectMessages();
+
                             sceneEventData.SceneEventType = SceneEventType.SynchronizeComplete;
                             SendSceneEventData(sceneEventId, new ulong[] { NetworkManager.ServerClientId });
 
@@ -2549,5 +2597,50 @@ internal struct DeferredObjectsMovedEvent
             internal Dictionary<int, List<ulong>> ObjectsMigratedTable;
         }
         internal List<DeferredObjectsMovedEvent> DeferredObjectsMovedEvents = new List<DeferredObjectsMovedEvent>();
+
+        internal struct DeferredObjectCreation
+        {
+            internal ulong SenderId;
+            internal uint MessageSize;
+            internal NetworkObject.SceneObject SceneObject;
+            internal FastBufferReader FastBufferReader;
+        }
+
+        internal List<DeferredObjectCreation> DeferredObjectCreationList = new List<DeferredObjectCreation>();
+        internal int DeferredObjectCreationCount;
+
+        internal void DeferCreateObject(ulong senderId, uint messageSize, NetworkObject.SceneObject sceneObject, FastBufferReader fastBufferReader)
+        {
+            var deferredObjectCreationEntry = new DeferredObjectCreation()
+            {
+                SenderId = senderId,
+                MessageSize = messageSize,
+                SceneObject = sceneObject,
+            };
+
+            unsafe
+            {
+                deferredObjectCreationEntry.FastBufferReader = new FastBufferReader(fastBufferReader.GetUnsafePtrAtCurrentPosition(), Allocator.Persistent, fastBufferReader.Length - fastBufferReader.Position);
+            }
+
+            DeferredObjectCreationList.Add(deferredObjectCreationEntry);
+        }
+
+        private void ProcessDeferredCreateObjectMessages()
+        {
+            // If no pending create object messages exit early
+            if (DeferredObjectCreationList.Count == 0)
+            {
+                return;
+            }
+            var networkManager = NetworkManager;
+            // Process all deferred create object messages.
+            foreach (var deferredObjectCreation in DeferredObjectCreationList)
+            {
+                CreateObjectMessage.CreateObject(ref networkManager, deferredObjectCreation.SenderId, deferredObjectCreation.MessageSize, deferredObjectCreation.SceneObject, deferredObjectCreation.FastBufferReader);
+            }
+            DeferredObjectCreationCount = DeferredObjectCreationList.Count;
+            DeferredObjectCreationList.Clear();
+        }
     }
 }

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

@@ -47,6 +47,14 @@ public enum SceneEventProgressStatus
         /// If you receive this event then it is most likely due to a bug (<em>please open a GitHub issue with steps to replicate</em>).<br/>
         /// </summary>
         InternalNetcodeError,
+        /// <summary>
+        /// This is returned when an unload or load action is attempted and scene management is disabled
+        /// </summary>
+        SceneManagementNotEnabled,
+        /// <summary>
+        /// This is returned when a client attempts to perform a server only action
+        /// </summary>
+        ServerOnlyAction,
     }
 
     /// <summary>

com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectSpawnManyObjectsTests.cs

@@ -57,9 +57,9 @@ public IEnumerator WhenManyObjectsAreSpawnedAtOnce_AllAreReceived()
                 serverObject.NetworkManagerOwner = m_ServerNetworkManager;
                 serverObject.Spawn();
             }
-            // ensure all objects are replicated before spawning more
-            yield return WaitForConditionOrTimeOut(() => SpawnObjecTrackingComponent.SpawnedObjects < k_SpawnedObjects);
-            Assert.False(s_GlobalTimeoutHelper.TimedOut, $"Timed out waiting for the client to spawn {k_SpawnedObjects} objects!");
+            // ensure all objects are replicated
+            yield return WaitForConditionOrTimeOut(() => SpawnObjecTrackingComponent.SpawnedObjects == k_SpawnedObjects);
+            AssertOnTimeout($"Timed out waiting for the client to spawn {k_SpawnedObjects} objects!");
         }
     }
 }