fix: NetworkManager always sent to the DDOL and cannot be nested

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -42,6 +42,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - ServerRpcParams and ClientRpcParams must be the last parameter of an RPC in order to function properly. Added a compile-time check to ensure this is the case and trigger an error if they're placed elsewhere. (#1318)
 - The SDK no longer limits message size to 64k. (The transport may still impose its own limits, but the SDK no longer does.) (#1384)
 - Updated com.unity.collections to 1.1.0
+- NetworkManager's GameObject is no longer allowed to be nested under one or more GameObject(s).(#1484)
+- NetworkManager DontDestroy property was removed and now NetworkManager always is migrated into the DontDestroyOnLoad scene. (#1484)
 
 
 ## [1.0.0-pre.3] - 2021-10-22

com.unity.netcode.gameobjects/Editor/NetworkManagerEditor.cs

@@ -15,7 +15,6 @@ public class NetworkManagerEditor : UnityEditor.Editor
         private static GUIStyle s_HelpBoxStyle;
 
         // Properties
-        private SerializedProperty m_DontDestroyOnLoadProperty;
         private SerializedProperty m_RunInBackgroundProperty;
         private SerializedProperty m_LogLevelProperty;
 
@@ -85,7 +84,6 @@ private void Initialize()
             m_NetworkManager = (NetworkManager)target;
 
             // Base properties
-            m_DontDestroyOnLoadProperty = serializedObject.FindProperty(nameof(NetworkManager.DontDestroy));
             m_RunInBackgroundProperty = serializedObject.FindProperty(nameof(NetworkManager.RunInBackground));
             m_LogLevelProperty = serializedObject.FindProperty(nameof(NetworkManager.LogLevel));
             m_NetworkConfigProperty = serializedObject.FindProperty(nameof(NetworkManager.NetworkConfig));
@@ -112,7 +110,6 @@ private void Initialize()
         private void CheckNullProperties()
         {
             // Base properties
-            m_DontDestroyOnLoadProperty = serializedObject.FindProperty(nameof(NetworkManager.DontDestroy));
             m_RunInBackgroundProperty = serializedObject.FindProperty(nameof(NetworkManager.RunInBackground));
             m_LogLevelProperty = serializedObject.FindProperty(nameof(NetworkManager.LogLevel));
             m_NetworkConfigProperty = serializedObject.FindProperty(nameof(NetworkManager.NetworkConfig));
@@ -223,7 +220,6 @@ public override void OnInspectorGUI()
             if (!m_NetworkManager.IsServer && !m_NetworkManager.IsClient)
             {
                 serializedObject.Update();
-                EditorGUILayout.PropertyField(m_DontDestroyOnLoadProperty);
                 EditorGUILayout.PropertyField(m_RunInBackgroundProperty);
                 EditorGUILayout.PropertyField(m_LogLevelProperty);
                 EditorGUILayout.Space();

com.unity.netcode.gameobjects/Editor/NetworkManagerHelper.cs

@@ -0,0 +1,108 @@
+using System.Collections.Generic;
+using UnityEngine;
+using UnityEditor;
+
+namespace Unity.Netcode.Editor
+{
+#if UNITY_EDITOR
+    /// <summary>
+    /// Specialized editor specific NetworkManager code
+    /// </summary>
+    public class NetworkManagerHelper : NetworkManager.INetworkManagerHelper
+    {
+        internal static NetworkManagerHelper Singleton;
+
+        // This is primarily to handle multiInstance scenarios where more than 1 NetworkManager could exist
+        private static Dictionary<NetworkManager, Transform> s_LastKnownNetworkManagerParents = new Dictionary<NetworkManager, Transform>();
+
+        /// <summary>
+        /// Initializes the singleton instance and registers for:
+        /// Hierarchy changed notification: to notify the user when they nest a NetworkManager
+        /// Play mode state change notification: to capture when entering or exiting play mode (currently only exiting)
+        /// </summary>
+        [InitializeOnLoadMethod]
+        private static void InitializeOnload()
+        {
+            Singleton = new NetworkManagerHelper();
+            NetworkManager.NetworkManagerHelper = Singleton;
+
+            EditorApplication.playModeStateChanged -= EditorApplication_playModeStateChanged;
+            EditorApplication.hierarchyChanged -= EditorApplication_hierarchyChanged;
+
+            EditorApplication.playModeStateChanged += EditorApplication_playModeStateChanged;
+            EditorApplication.hierarchyChanged += EditorApplication_hierarchyChanged;
+        }
+
+        private static void EditorApplication_playModeStateChanged(PlayModeStateChange playModeStateChange)
+        {
+            switch (playModeStateChange)
+            {
+                case PlayModeStateChange.ExitingEditMode:
+                    {
+                        s_LastKnownNetworkManagerParents.Clear();
+                        break;
+                    }
+            }
+        }
+
+        private static void EditorApplication_hierarchyChanged()
+        {
+            var allNetworkManagers = Resources.FindObjectsOfTypeAll<NetworkManager>();
+            foreach (var networkManager in allNetworkManagers)
+            {
+                networkManager.NetworkManagerCheckForParent();
+            }
+        }
+
+        /// <summary>
+        /// Handles notifying the user, via display dialog window, that they have nested a NetworkManager.
+        /// When in edit mode it provides the option to automatically fix the issue
+        /// When in play mode it just notifies the user when entering play mode as well as when the user
+        /// tries to start a network session while a NetworkManager is still nested.
+        /// </summary>
+        public bool NotifyUserOfNestedNetworkManager(NetworkManager networkManager, bool ignoreNetworkManagerCache = false, bool editorTest = false)
+        {
+            var gameObject = networkManager.gameObject;
+            var transform = networkManager.transform;
+            var isParented = transform.root != transform;
+
+            var message = NetworkManager.GenerateNestedNetworkManagerMessage(transform);
+            if (s_LastKnownNetworkManagerParents.ContainsKey(networkManager) && !ignoreNetworkManagerCache)
+            {
+                // If we have already notified the user, then don't notify them again
+                if (s_LastKnownNetworkManagerParents[networkManager] == transform.root)
+                {
+                    return isParented;
+                }
+                else // If we are no longer a child, then we can remove ourself from this list
+                if (transform.root == gameObject.transform)
+                {
+                    s_LastKnownNetworkManagerParents.Remove(networkManager);
+                }
+            }
+            if (!EditorApplication.isUpdating && isParented)
+            {
+                if (!EditorApplication.isPlaying && !editorTest)
+                {
+                    message += $"Click 'Auto-Fix' to automatically remove it from {transform.root.gameObject.name} or 'Manual-Fix' to fix it yourself in the hierarchy view.";
+                    if (EditorUtility.DisplayDialog("Invalid Nested NetworkManager", message, "Auto-Fix", "Manual-Fix"))
+                    {
+                        transform.parent = null;
+                        isParented = false;
+                    }
+                }
+                else
+                {
+                    Debug.LogError(message);
+                }
+
+                if (!s_LastKnownNetworkManagerParents.ContainsKey(networkManager) && isParented)
+                {
+                    s_LastKnownNetworkManagerParents.Add(networkManager, networkManager.transform.root);
+                }
+            }
+            return isParented;
+        }
+    }
+#endif
+}

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

@@ -223,11 +223,6 @@ public GameObject GetNetworkPrefabOverride(GameObject gameObject)
 
         public NetworkTime ServerTime => NetworkTickSystem?.ServerTime ?? default;
 
-        /// <summary>
-        /// Gets or sets if the NetworkManager should be marked as DontDestroyOnLoad
-        /// </summary>
-        [HideInInspector] public bool DontDestroy = true;
-
         /// <summary>
         /// Gets or sets if the application should be set to run in background
         /// </summary>
@@ -516,6 +511,13 @@ private void OnValidate()
 
         private void Initialize(bool server)
         {
+            // Don't allow the user to start a network session if the NetworkManager is
+            // still parented under another GameObject
+            if (NetworkManagerCheckForParent(true))
+            {
+                return;
+            }
+
             if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
             {
                 NetworkLog.LogInfo(nameof(Initialize));
@@ -970,11 +972,6 @@ public void SetSingleton()
 
         private void OnEnable()
         {
-            if (DontDestroy)
-            {
-                DontDestroyOnLoad(gameObject);
-            }
-
             if (RunInBackground)
             {
                 Application.runInBackground = true;
@@ -984,6 +981,11 @@ private void OnEnable()
             {
                 SetSingleton();
             }
+
+            if (!NetworkManagerCheckForParent())
+            {
+                DontDestroyOnLoad(gameObject);
+            }
         }
 
         private void Awake()
@@ -1727,5 +1729,48 @@ internal void ApprovedPlayerSpawn(ulong clientId, uint playerPrefabHash)
                 NetworkMetrics.TrackObjectSpawnSent(clientPair.Key, ConnectedClients[clientId].PlayerObject, size);
             }
         }
+
+        /// <summary>
+        /// Handle runtime detection for parenting the NetworkManager's GameObject under another GameObject
+        /// </summary>
+        private void OnTransformParentChanged()
+        {
+            NetworkManagerCheckForParent();
+        }
+
+        /// <summary>
+        /// Determines if the NetworkManager's GameObject is parented under another GameObject and
+        /// notifies the user that this is not allowed for the NetworkManager.
+        /// </summary>
+        internal bool NetworkManagerCheckForParent(bool ignoreNetworkManagerCache = false)
+        {
+#if UNITY_EDITOR
+            var isParented = NetworkManagerHelper.NotifyUserOfNestedNetworkManager(this, ignoreNetworkManagerCache);
+#else
+            var isParented = transform.root != transform;
+            if (isParented)
+            {
+                throw new Exception(GenerateNestedNetworkManagerMessage(transform));
+            }
+#endif
+            return isParented;
+        }
+
+        static internal string GenerateNestedNetworkManagerMessage(Transform transform)
+        {
+            return $"{transform.name} is nested under {transform.root.name}. NetworkManager cannot be nested.\n";
+        }
+
+#if UNITY_EDITOR
+        static internal INetworkManagerHelper NetworkManagerHelper;
+        /// <summary>
+        /// Interface for NetworkManagerHelper
+        /// </summary>
+        internal interface INetworkManagerHelper
+        {
+            bool NotifyUserOfNestedNetworkManager(NetworkManager networkManager, bool ignoreNetworkManagerCache = false, bool editorTest = false);
+        }
+#endif
     }
+
 }

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

@@ -584,20 +584,8 @@ internal NetworkSceneManager(NetworkManager networkManager)
 
             GenerateScenesInBuild();
 
-            // If NetworkManager has this set to true, then we can get the DDOL (DontDestroyOnLoad) from its GaemObject
-            if (networkManager.DontDestroy)
-            {
-                DontDestroyOnLoadScene = networkManager.gameObject.scene;
-            }
-            else
-            {
-                // Otherwise, we have to create a GameObject and move it into the DDOL in order to
-                // register the DDOL scene handle with NetworkSceneManager
-                var myDDOLObject = new GameObject("DDOL-NWSM");
-                UnityEngine.Object.DontDestroyOnLoad(myDDOLObject);
-                DontDestroyOnLoadScene = myDDOLObject.scene;
-                UnityEngine.Object.Destroy(myDDOLObject);
-            }
+            // Since NetworkManager is now always migrated to the DDOL we will use this to get the DDOL scene
+            DontDestroyOnLoadScene = networkManager.gameObject.scene;
 
             ServerSceneHandleToClientSceneHandle.Add(DontDestroyOnLoadScene.handle, DontDestroyOnLoadScene.handle);
             ScenesLoaded.Add(DontDestroyOnLoadScene.handle, DontDestroyOnLoadScene);

com.unity.netcode.gameobjects/Tests/Editor/NetworkManagerConfigurationTests.cs

@@ -0,0 +1,38 @@
+using NUnit.Framework;
+using UnityEngine;
+using Unity.Netcode.Editor;
+using UnityEngine.TestTools;
+
+namespace Unity.Netcode.EditorTests
+{
+    public class NetworkManagerConfigurationTests
+    {
+        /// <summary>
+        /// Does a simple check to make sure the nested network manager will
+        /// notify the user when in the editor.  This is just a unit test to
+        /// validate this is functioning
+        /// </summary>
+        [Test]
+        public void NestedNetworkManagerCheck()
+        {
+            var parent = new GameObject("ParentObject");
+            var networkManagerObject = new GameObject(nameof(NestedNetworkManagerCheck));
+            var networkManager = networkManagerObject.AddComponent<NetworkManager>();
+
+            // Make our NetworkManager's GameObject nested
+            networkManagerObject.transform.parent = parent.transform;
+
+            // Pre-generate the error message we are expecting to see
+            var messageToCheck = NetworkManager.GenerateNestedNetworkManagerMessage(networkManagerObject.transform);
+
+            // Trap for the nested NetworkManager exception
+            LogAssert.Expect(LogType.Error, messageToCheck);
+
+            // Since this is an in-editor test, we must force this invocation
+            NetworkManagerHelper.Singleton.NotifyUserOfNestedNetworkManager(networkManager, false, true);
+
+            // Clean up
+            Object.DestroyImmediate(parent);
+        }
+    }
+}

com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformTests.cs

@@ -200,6 +200,7 @@ public override IEnumerator Teardown()
         }
     }
 
+
     /// <summary>
     /// This test simulates a pooled NetworkObject being re-used over time with a NetworkTransform
     /// This test validates that pooled NetworkObjects' NetworkTransforms are completely reset in
@@ -249,6 +250,8 @@ private void Update()
 
         public NetworkObject Instantiate(ulong ownerClientId, Vector3 position, Quaternion rotation)
         {
+            m_ClientSideObject.transform.position = position;
+            m_ClientSideObject.transform.rotation = rotation;
             m_ClientSideSpawned = true;
             m_ClientSideObject.SetActive(true);
             return m_ClientSideObject.GetComponent<NetworkObject>();
@@ -317,14 +320,16 @@ public IEnumerator RespawnedPositionTest()
             yield return new WaitUntil(() => m_ClientSideSpawned);
 
             // Let the object move a bit
-            yield return new WaitForSeconds(0.5f);
+            yield return WaitForFrames(100);
 
             // Make sure it moved on the client side
             Assert.IsTrue(m_ClientSideObject.transform.position != Vector3.zero);
 
             m_ServerNetworkManager.SpawnManager.DespawnObject(m_DefaultNetworkObject);
             yield return new WaitUntil(() => !m_ClientSideSpawned);
 
+            yield return WaitForFrames(30);
+
             // Re-spawn the same NetworkObject
             m_DefaultNetworkObject.Spawn();
             yield return new WaitUntil(() => m_ClientSideSpawned);
@@ -343,6 +348,13 @@ public IEnumerator RespawnedPositionTest()
             m_DefaultNetworkObject.Despawn();
         }
 
+
+        private IEnumerator WaitForFrames(int framesToWait)
+        {
+            var frameCountToWaitFor = Time.frameCount + framesToWait;
+            yield return new WaitUntil(() => frameCountToWaitFor <= Time.frameCount);
+        }
+
         public override IEnumerator Teardown()
         {
             if (m_ClientSideObject != null)