fix: clamp spawntimeout to recommended range (#3174)

* update Clamping spawntimeout * update improving parenting failed message when either the child or parent NetworkObject is not spawnd. * update Update the local SceneEventData.SceneEventType on the authority side for SceneLoadComplete. * style removing whitespaces
Unity-Technologies · Dec 12, 2024 · 2b3893c · 2b3893c
1 parent c69aa97
commit 2b3893c
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 4 deletions.
diff --git a/com.unity.netcode.gameobjects/Runtime/Configuration/NetworkConfig.cs b/com.unity.netcode.gameobjects/Runtime/Configuration/NetworkConfig.cs
@@ -13,6 +13,14 @@ namespace Unity.Netcode
     [Serializable]
     public class NetworkConfig
     {
+        // Clamp spawn time outs to prevent dropping messages during scene events
+        // Note: The legacy versions of NGO defaulted to 1s which was too low. As
+        // well, the SpawnTimeOut is now being clamped to within this recommended
+        // range both via UI and when NetworkManager is validated.
+        internal const float MinSpawnTimeout = 10.0f;
+        // Clamp spawn time outs to no more than 1 hour (really that is a bit high)
+        internal const float MaxSpawnTimeout = 3600.0f;
+
         /// <summary>
         /// The protocol version. Different versions doesn't talk to each other.
         /// </summary>
@@ -132,6 +140,8 @@ public class NetworkConfig
         /// The amount of time a message will be held (deferred) if the destination NetworkObject needed to process the message doesn't exist yet. If the NetworkObject is not spawned within this time period, all deferred messages for that NetworkObject will be dropped.
         /// </summary>
         [Tooltip("The amount of time a message will be held (deferred) if the destination NetworkObject needed to process the message doesn't exist yet. If the NetworkObject is not spawned within this time period, all deferred messages for that NetworkObject will be dropped.")]
+
+        [Range(MinSpawnTimeout, MaxSpawnTimeout)]
         public float SpawnTimeout = 10f;
 
         /// <summary>
@@ -176,6 +186,21 @@ public class NetworkConfig
         [Tooltip("Enable (default) if you want to profile network messages with development builds and defaults to being disabled in release builds. When disabled, network messaging profiling will be disabled in development builds.")]
         public bool NetworkProfilingMetrics = true;
 
+        /// <summary>
+        /// Invoked by <see cref="NetworkManager"/> when it is validated.
+        /// </summary>
+        /// <remarks>
+        /// Used to check for potential legacy values that have already been serialized and/or
+        /// runtime modifications to a property outside of the recommended range.
+        /// For each property checked below, provide a brief description of the reason.
+        /// </remarks>
+        internal void OnValidate()
+        {
+            // Legacy NGO versions defaulted this value to 1 second that has since been determiend
+            // any range less than 10 seconds can lead to dropped messages during scene events.
+            SpawnTimeout = Mathf.Clamp(SpawnTimeout, MinSpawnTimeout, MaxSpawnTimeout);
+        }
+
         /// <summary>
         /// Returns a base64 encoded version of the configuration
         /// </summary>

diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs
@@ -955,6 +955,9 @@ internal void OnValidate()
                 return; // May occur when the component is added
             }
 
+            // Do a validation pass on NetworkConfig properties
+            NetworkConfig.OnValidate();
+
             if (GetComponentInChildren<NetworkObject>() != null)
             {
                 if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)

diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs
@@ -2007,12 +2007,14 @@ public bool TrySetParent(NetworkObject parent, bool worldPositionStays = true)
 
         internal bool InternalTrySetParent(NetworkObject parent, bool worldPositionStays = true)
         {
-            if (parent != null && (IsSpawned ^ parent.IsSpawned))
+            if (parent != null && (IsSpawned ^ parent.IsSpawned) && NetworkManager != null && !NetworkManager.ShutdownInProgress)
             {
-                if (NetworkManager != null && !NetworkManager.ShutdownInProgress)
+                if (NetworkManager.LogLevel <= LogLevel.Developer)
                 {
-                    return false;
+                    var nameOfNotSpawnedObject = IsSpawned ? $" the parent ({parent.name})" : $"the child ({name})";
+                    NetworkLog.LogWarning($"Parenting failed because {nameOfNotSpawnedObject} is not spawned!");
                 }
+                return false;
             }
 
             m_CachedWorldPositionStays = worldPositionStays;

diff --git a/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs b/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs
@@ -1903,10 +1903,12 @@ private void OnSessionOwnerLoadedScene(uint sceneEventId, Scene scene)
             SendSceneEventData(sceneEventData.SceneEventId, NetworkManager.ConnectedClientsIds.Where(c => c != sessionOwner).ToArray());
 
             m_IsSceneEventActive = false;
+
+            sceneEventData.SceneEventType = SceneEventType.LoadComplete;
             //First, notify local server that the scene was loaded
             OnSceneEvent?.Invoke(new SceneEvent()
             {
-                SceneEventType = SceneEventType.LoadComplete,
+                SceneEventType = sceneEventData.SceneEventType,
                 LoadSceneMode = sceneEventData.LoadSceneMode,
                 SceneName = SceneNameFromHash(sceneEventData.SceneHash),
                 ClientId = NetworkManager.LocalClientId,