refactor

Going ahead and adding an (currently) internal "IsDestroying" flag that is set only when the NetworkObject.OnDestroy method is invoked.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/Runtime/Components/Helpers/AttachableBehaviour.cs

@@ -205,8 +205,6 @@ public enum AttachState
         private Vector3 m_OriginalLocalPosition;
         private Quaternion m_OriginalLocalRotation;
 
-        internal bool IsDestroying { get; private set; }
-
         /// <inheritdoc/>
         protected override void OnSynchronize<T>(ref BufferSerializer<T> serializer)
         {
@@ -240,42 +238,6 @@ protected virtual void Awake()
             OnAwake();
         }
 
-        /// <inheritdoc/>
-        protected override void OnNetworkPreSpawn(ref NetworkManager networkManager)
-        {
-            IsDestroying = false;
-            // When attached to something else, the attachable needs to know if the
-            // default parent has been destroyed in order to not attempt to re-parent
-            // when detached (especially if it is being detached because it should be destroyed).
-            NetworkObject.OnDestroying += OnDefaultParentDestroying;
-
-            base.OnNetworkPreSpawn(ref networkManager);
-        }
-
-        private void OnDefaultParentDestroying()
-        {
-            NetworkObject.OnDestroying -= OnDefaultParentDestroying;
-            // Exit early if we are already being destroyed
-            if (IsDestroying)
-            {
-                return;
-            }
-            IsDestroying = true;
-            // If not completely detached, then destroy the GameObject for
-            // this attachable since the associated NetworkObject is being
-            // destroyed.
-            if (m_AttachState != AttachState.Detached)
-            {
-                Destroy(gameObject);
-            }
-        }
-
-        internal override void InternalOnDestroy()
-        {
-            IsDestroying = true;
-            base.InternalOnDestroy();
-        }
-
         /// <inheritdoc/>
         /// <remarks>
         /// If you create a custom <see cref="AttachableBehaviour"/> and override this method, you will want to
@@ -316,6 +278,14 @@ internal void ForceDetach()
         /// <inheritdoc/>
         public override void OnNetworkPreDespawn()
         {
+            // If the NetworkObject is being destroyed and not completely detached, then destroy the GameObject for
+            // this attachable since the associated default parent is being destroyed.
+            if (IsDestroying && m_AttachState != AttachState.Detached)
+            {
+                Destroy(gameObject);
+                return;
+            }
+
             if (NetworkManager.ShutdownInProgress || AutoDetach.HasFlag(AutoDetachTypes.OnDespawn))
             {
                 ForceDetach();

com.unity.netcode.gameobjects/Runtime/Components/Helpers/AttachableNode.cs

@@ -24,8 +24,6 @@ public class AttachableNode : NetworkBehaviour
     /// </summary>
     public bool DetachOnDespawn = true;
 
-    internal bool IsDestroying { get; private set; }
-
     /// <summary>
     /// A <see cref="List{T}"/> of the currently attached <see cref="AttachableBehaviour"/>s.
     /// </summary>
@@ -34,7 +32,6 @@ public class AttachableNode : NetworkBehaviour
     /// <inheritdoc/>
     protected override void OnNetworkPreSpawn(ref NetworkManager networkManager)
     {
-        IsDestroying = false;
         m_AttachedBehaviours.Clear();
         base.OnNetworkPreSpawn(ref networkManager);
     }
@@ -104,7 +101,6 @@ public override void OnNetworkPreDespawn()
 
     internal override void InternalOnDestroy()
     {
-        IsDestroying = true;
         // Notify any attached behaviours that this node is being destroyed.
         for (int i = m_AttachedBehaviours.Count - 1; i >= 0; i--)
         {

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

@@ -643,6 +643,20 @@ protected NetworkBehaviour GetNetworkBehaviour(ushort behaviourId)
         /// </summary>
         public ulong OwnerClientId { get; internal set; }
 
+        /// <summary>
+        /// Returns true if the NetworkObject is in the middle of being destroyed or
+        /// if there is no valid assigned NetworkObject.
+        /// </summary>
+        /// <remarks>
+        /// <see cref="NetworkObject.IsDestroying"/>
+        /// </remarks>
+        internal bool IsDestroying { get; private set; }
+
+        internal void SetDestroying()
+        {
+            IsDestroying = true;
+        }
+
         /// <summary>
         /// Updates properties with network session related
         /// dependencies such as a NetworkObject's spawned

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

@@ -1689,13 +1689,50 @@ public static void NetworkHide(List<NetworkObject> networkObjects, ulong clientI
         }
 
         /// <summary>
-        /// Invoked when the NetworkObject is destroyed.
+        /// Returns true if the NetworkObject is in the middle of being destroyed.
         /// </summary>
-        internal event Action OnDestroying;
+        /// <remarks>
+        /// This is particularly useful when determining if something is being de-spawned
+        /// normally or if it is being de-spawned because the NetworkObject/GameObject is
+        /// being destroyed.
+        /// </remarks>
+        internal bool IsDestroying { get; private set; }
+
+        /// <summary>
+        /// Applies the despawning flag for the local instance and
+        /// its child NetworkBehaviours. Private to assure this is
+        /// only invoked from within OnDestroy.
+        /// </summary>
+        private void SetIsDestroying()
+        {
+            IsDestroying = true;
+
+            // Exit early if null
+            if (m_ChildNetworkBehaviours == null)
+            {
+                return;
+            }
+
+            foreach(var childBehaviour in m_ChildNetworkBehaviours)
+            {
+                // Just ignore and continue processing through the entries
+                if (!childBehaviour)
+                {
+                    continue;
+                }
+
+                // Keeping the property a private set to assure this is
+                // the only way it can be set as it should never be reset
+                // back to false once invoked.
+                childBehaviour.SetDestroying();
+            }
+        }
 
         private void OnDestroy()
         {
-            OnDestroying?.Invoke();
+            // Apply the is destroying flag
+            SetIsDestroying();
+
             var networkManager = NetworkManager;
             // If no NetworkManager is assigned, then just exit early
             if (!networkManager)