fix: Nested NetworkBehaviours don't de-register or Invoke OnNetworkDespawn if destroyed while the parent NetworkObject remains spawned [NCCBUG-137]

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -15,6 +15,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Fixed
 
 - Fixed issue when attempting to spawn a parent `GameObject`, with `NetworkObject` component attached, that has one or more child `GameObject`s, that are inactive in the hierarchy, with `NetworkBehaviour` components it will no longer attempt to spawn the associated NetworkBehaviour(s) or invoke ownership changed notifications but will log a warning message. (#2096)
+- Fixed issue where destroying `GameObject`, with an attached NetworkBehaviour component, that was a child or any child generation of the assigned `NetworkObject` component's `GameObject` would not deregister with the NetworkObject nor would it have its OnNetworkSpawn method invoked. (#2091)
 - Fixed issue where `NetworkObject.NetworkHide` was despawning and destroying, as opposed to only despawning, in-scene placed `NetworkObject`s. (#2086)
 - Fixed issue where `NetworkAnimator` would not synchronize a looping animation for late joining clients if it was at the very end of its loop. (#2076)
 - Fixed issue where `NetworkAnimator` was not removing its subscription from `OnClientConnectedCallback` when despawned during the shutdown sequence. (#2074)

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

@@ -331,7 +331,8 @@ public NetworkObject NetworkObject
                 // in Update and/or in FixedUpdate could still be checking NetworkBehaviour.NetworkObject directly (i.e. does it exist?)
                 // or NetworkBehaviour.IsSpawned (i.e. to early exit if not spawned) which, in turn, could generate several Warning messages
                 // per spawned NetworkObject.  Checking for ShutdownInProgress prevents these unnecessary LogWarning messages.
-                if (m_NetworkObject == null && (NetworkManager.Singleton == null || !NetworkManager.Singleton.ShutdownInProgress))
+                // We must check IsSpawned, otherwise a warning will be logged under certain valid conditions (see OnDestroy)
+                if (IsSpawned && m_NetworkObject == null && (NetworkManager.Singleton == null || !NetworkManager.Singleton.ShutdownInProgress))
                 {
                     if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
                     {
@@ -759,6 +760,14 @@ protected NetworkObject GetNetworkObject(ulong networkId)
         /// </summary>
         public virtual void OnDestroy()
         {
+            if (NetworkObject != null && NetworkObject.IsSpawned && IsSpawned)
+            {
+                // If the associated NetworkObject is still spawned then this
+                // NetworkBehaviour will be removed from the NetworkObject's
+                // ChildNetworkBehaviours list.
+                NetworkObject.OnNetworkBehaviourDestroyed(this);
+            }
+
             // this seems odd to do here, but in fact especially in tests we can find ourselves
             //  here without having called InitializedVariables, which causes problems if any
             //  of those variables use native containers (e.g. NetworkList) as they won't be
@@ -770,6 +779,7 @@ public virtual void OnDestroy()
                 InitializeVariables();
             }
 
+
             for (int i = 0; i < NetworkVariableFields.Count; i++)
             {
                 NetworkVariableFields[i].Dispose();

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

@@ -1219,5 +1219,21 @@ internal uint HostCheckForGlobalObjectIdHashOverride()
 
             return GlobalObjectIdHash;
         }
+
+        /// <summary>
+        /// Removes a NetworkBehaviour from the ChildNetworkBehaviours list when destroyed
+        /// while the NetworkObject is still spawned.
+        /// </summary>
+        internal void OnNetworkBehaviourDestroyed(NetworkBehaviour networkBehaviour)
+        {
+            if (networkBehaviour.IsSpawned && IsSpawned)
+            {
+                if (NetworkManager.LogLevel == LogLevel.Developer)
+                {
+                    NetworkLog.LogWarning($"{nameof(NetworkBehaviour)}-{networkBehaviour.name} is being destroyed while {nameof(NetworkObject)}-{name} is still spawned! (could break state synchronization)");
+                }
+                ChildNetworkBehaviours.Remove(networkBehaviour);
+            }
+        }
     }
 }

com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourGenericTests.cs

@@ -22,6 +22,13 @@ protected override bool CanStartServerAndClients()
 
         public class SimpleNetworkBehaviour : NetworkBehaviour
         {
+            public bool OnNetworkDespawnCalled;
+
+            public override void OnNetworkDespawn()
+            {
+                OnNetworkDespawnCalled = true;
+                base.OnNetworkDespawn();
+            }
         }
 
         protected override IEnumerator OnSetup()
@@ -83,6 +90,9 @@ public IEnumerator ValidateNoSpam()
             // set the log level to developer
             m_ServerNetworkManager.LogLevel = LogLevel.Developer;
 
+            // The only valid condition for this would be if the NetworkBehaviour is spawned.
+            simpleNetworkBehaviour.IsSpawned = true;
+
             // Verify the warning gets logged under normal conditions
             var isNull = simpleNetworkBehaviour.NetworkObject == null;
             LogAssert.Expect(LogType.Warning, $"[Netcode] Could not get {nameof(NetworkObject)} for the {nameof(NetworkBehaviour)}. Are you missing a {nameof(NetworkObject)} component?");
@@ -98,5 +108,44 @@ public IEnumerator ValidateNoSpam()
             networkObjectToTest.Despawn();
             Object.Destroy(networkObjectToTest);
         }
+
+        /// <summary>
+        /// This validates the fix for when a child GameObject with a NetworkBehaviour
+        /// is deleted while the parent GameObject with a NetworkObject is spawned and
+        /// is not deleted until a later time would cause an exception due to the
+        /// NetworkBehaviour not being removed from the NetworkObject.ChildNetworkBehaviours
+        /// list.
+        /// </summary>
+        [UnityTest]
+        public IEnumerator ValidateDeleteChildNetworkBehaviour()
+        {
+            m_AllowServerToStart = true;
+
+            yield return s_DefaultWaitForTick;
+
+            // Now just start the Host
+            yield return StartServerAndClients();
+
+            var parentObject = new GameObject();
+            var childObject = new GameObject();
+            childObject.transform.parent = parentObject.transform;
+            var parentNetworkObject = parentObject.AddComponent<NetworkObject>();
+            childObject.AddComponent<SimpleNetworkBehaviour>();
+
+            parentNetworkObject.Spawn();
+            yield return s_DefaultWaitForTick;
+
+            // Destroy the child object with child NetworkBehaviour
+            Object.Destroy(childObject);
+
+            yield return s_DefaultWaitForTick;
+
+            // Assure no log messages are logged when they should not be logged
+            LogAssert.NoUnexpectedReceived();
+
+            // Destroy the parent object which should not cause any exceptions
+            // (validating the fix)
+            Object.Destroy(parentObject);
+        }
     }
 }