fix: notify networkmanager and networkobject not allowed (#1777)

* fix

notify the user when they try to add a NetworkObject to a GameObject that already has a NetworkManager assigned to it.

* fix

extending the INetworkManagerHelper interface.

* update and style

Renamed the method being invoked to NotifyUserNetworkObjectRemoved in order to be more explicit in what was happening.
Updated comments

* Update CHANGELOG.md

* test and update

adding a test to validate that checking for a NetworkObject, removing the NetworkObject, and notifying the user that a NetworkManager cannot exist if a NetworkObject resides on the GameObject.  This also now checks children containing a NetworkObject.  It does not need to check for a parent as this is checked after to invoking NetworkManagerCheckForParent.

* update

making a method to return the error message.
adjusting the error message to include children in the description.
adding comments for clarity.

* update

Adding a validation check to assure the NetworkObject has been removed.

* style

whitespace fix

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -11,6 +11,12 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Fixed
 - Fixed issue when spawning new player if an already existing player exists it does not remove IsPlayer from the previous player (#1779)
 
+## [Unreleased]
+### Added
+### Changed
+### Fixed
+- Fixed lack of notification that NetworkManager and NetworkObject cannot be added to the same GameObject with in-editor notifications (#1777)
+
 ## [1.0.0-pre.6] - 2022-03-02
 
 ### Added

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

@@ -45,13 +45,63 @@ private static void EditorApplication_playModeStateChanged(PlayModeStateChange p
             }
         }
 
+        /// <summary>
+        /// Invoked only when the hierarchy changes
+        /// </summary>
         private static void EditorApplication_hierarchyChanged()
         {
             var allNetworkManagers = Resources.FindObjectsOfTypeAll<NetworkManager>();
             foreach (var networkManager in allNetworkManagers)
             {
-                networkManager.NetworkManagerCheckForParent();
+                if (!networkManager.NetworkManagerCheckForParent())
+                {
+                    Singleton.CheckAndNotifyUserNetworkObjectRemoved(networkManager);
+                }
+            }
+        }
+
+        /// <summary>
+        /// Handles notifying users that they cannot add a NetworkObject component
+        /// to a GameObject that also has a NetworkManager component. The NetworkObject
+        /// will always be removed.
+        /// GameObject + NetworkObject then NetworkManager = NetworkObject removed
+        /// GameObject + NetworkManager then NetworkObject = NetworkObject removed
+        /// Note: Since this is always invoked after <see cref="NetworkManagerCheckForParent"/>
+        /// we do not need to check for parent when searching for a NetworkObject component
+        /// </summary>
+        public void CheckAndNotifyUserNetworkObjectRemoved(NetworkManager networkManager, bool editorTest = false)
+        {
+            // Check for any NetworkObject at the same gameObject relative layer
+            var networkObject = networkManager.gameObject.GetComponent<NetworkObject>();
+
+            if (networkObject == null)
+            {
+                // if none is found, check to see if any children have a NetworkObject
+                networkObject = networkManager.gameObject.GetComponentInChildren<NetworkObject>();
+                if (networkObject == null)
+                {
+                    return;
+                }
             }
+
+            if (!EditorApplication.isUpdating)
+            {
+                Object.DestroyImmediate(networkObject);
+
+                if (!EditorApplication.isPlaying && !editorTest)
+                {
+                    EditorUtility.DisplayDialog($"Removing {nameof(NetworkObject)}", NetworkManagerAndNetworkObjectNotAllowedMessage(), "OK");
+                }
+                else
+                {
+                    Debug.LogError(NetworkManagerAndNetworkObjectNotAllowedMessage());
+                }
+            }
+        }
+
+        public string NetworkManagerAndNetworkObjectNotAllowedMessage()
+        {
+            return $"A {nameof(GameObject)} cannot have both a {nameof(NetworkManager)} and {nameof(NetworkObject)} assigned to it or any children under it.";
         }
 
         /// <summary>

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

@@ -1014,6 +1014,7 @@ static internal string GenerateNestedNetworkManagerMessage(Transform transform)
         internal interface INetworkManagerHelper
         {
             bool NotifyUserOfNestedNetworkManager(NetworkManager networkManager, bool ignoreNetworkManagerCache = false, bool editorTest = false);
+            void CheckAndNotifyUserNetworkObjectRemoved(NetworkManager networkManager, bool editorTest = false);
         }
 #endif
 

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

@@ -34,5 +34,49 @@ public void NestedNetworkManagerCheck()
             // Clean up
             Object.DestroyImmediate(parent);
         }
+
+        public enum NetworkObjectPlacement
+        {
+            Root,   // Added to the same root GameObject
+            Child   // Added to a child GameObject
+        }
+
+        [Test]
+        public void NetworkObjectNotAllowed([Values] NetworkObjectPlacement networkObjectPlacement)
+        {
+            var gameObject = new GameObject(nameof(NetworkManager));
+            var targetforNetworkObject = gameObject;
+
+            if (networkObjectPlacement == NetworkObjectPlacement.Child)
+            {
+                var childGameObject = new GameObject($"{nameof(NetworkManager)}-Child");
+                childGameObject.transform.parent = targetforNetworkObject.transform;
+                targetforNetworkObject = childGameObject;
+            }
+
+            var networkManager = gameObject.AddComponent<NetworkManager>();
+
+            // Trap for the error message generated when a NetworkObject is discovered on the same GameObject or any children under it
+            LogAssert.Expect(LogType.Error, NetworkManagerHelper.Singleton.NetworkManagerAndNetworkObjectNotAllowedMessage());
+
+            // Add the NetworkObject
+            var networkObject = targetforNetworkObject.AddComponent<NetworkObject>();
+
+            // Since this is an in-editor test, we must force this invocation
+            NetworkManagerHelper.Singleton.CheckAndNotifyUserNetworkObjectRemoved(networkManager, true);
+
+            // Validate that the NetworkObject has been removed
+            if (networkObjectPlacement == NetworkObjectPlacement.Root)
+            {
+                Assert.IsNull(networkManager.gameObject.GetComponent<NetworkObject>(), $"There is still a {nameof(NetworkObject)} on {nameof(NetworkManager)}'s GameObject!");
+            }
+            else
+            {
+                Assert.IsNull(networkManager.gameObject.GetComponentInChildren<NetworkObject>(), $"There is still a {nameof(NetworkObject)} on {nameof(NetworkManager)}'s child GameObject!");
+            }
+
+            // Clean up
+            Object.DestroyImmediate(gameObject);
+        }
     }
 }