fix

The actual fix for the issue with additional handling when an attachable's NetworkObject is destroyed while it  is still attached to something else.

Author: NoelStephensUnity

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

@@ -205,6 +205,8 @@ 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)
         {
@@ -238,6 +240,37 @@ protected virtual void Awake()
             OnAwake();
         }
 
+        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 detatched 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;
+            // Just destroy the GameObject for this attachable since
+            // the associated NetworkObject is being destroyed.
+            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
@@ -458,16 +491,9 @@ public void Attach(AttachableNode attachableNode)
         /// </summary>
         internal void InternalDetach()
         {
-            if (m_AttachableNode)
+            if (!IsDestroying && m_AttachableNode && !m_AttachableNode.IsDestroying)
             {
-                // TODO-FIX: We might track if something has been "destroyed" in order
-                // to be able to be 100% sure in the event a user disables the world item
-                // when detatched. Otherwise, we keep this in place and make note of it
-                // in documentation.
-                // Issue:
-                // Edge-case where the parent could be in the middle of being destroyed.
-                // If not active in the hierarchy, then don't attempt to set the parent.
-                if (m_DefaultParent && m_DefaultParent.activeInHierarchy)
+                if (m_DefaultParent)
                 {
                     // Set the original parent and origianl local position and rotation
                     transform.SetParent(m_DefaultParent.transform, false);

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

@@ -24,6 +24,8 @@ 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>
@@ -32,6 +34,7 @@ public class AttachableNode : NetworkBehaviour
     /// <inheritdoc/>
     protected override void OnNetworkPreSpawn(ref NetworkManager networkManager)
     {
+        IsDestroying = false;
         m_AttachedBehaviours.Clear();
         base.OnNetworkPreSpawn(ref networkManager);
     }
@@ -84,23 +87,15 @@ public override void OnNetworkPreDespawn()
                 }
                 else
                 {
-                    // TODO-FIX: We might track if something has been "destroyed" in order
-                    // to be able to be 100% sure this is specific to being destroyed.
-                    // Otherwise, we keep this in place and make note of it
-                    // in documentation that you cannot detatch from something already despawned.
-                    // Issue: When trying to detatch if the thing attached is no longer 
-                    // spawned. Instantiation order recently changed such that
-                    // the attachable =or= the attach node target could be despawned 
-                    // and in the middle of being destroyed. Resolution for this 
-                    // is to skip over destroyed object (default) and then only sort
-                    // through the things the local NetworkManager instance has authority
-                    // over. Even then, we have to check if the attached object is still
-                    // spawned before attempting to detatch it.
                     if (attachable.IsSpawned)
                     {
                         // Detach the normal way with authority
                         attachable.Detach();
                     }
+                    else if (!attachable.IsDestroying)
+                    {
+                        attachable.ForceDetach();
+                    }
                 }
             }
         }
@@ -109,6 +104,7 @@ 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/NetworkObject.cs

@@ -1688,8 +1688,14 @@ public static void NetworkHide(List<NetworkObject> networkObjects, ulong clientI
             }
         }
 
+        /// <summary>
+        /// Invoked when the NetworkObject is destroyed.
+        /// </summary>
+        internal event Action OnDestroying;
+
         private void OnDestroy()
         {
+            OnDestroying?.Invoke();
             var networkManager = NetworkManager;
             // If no NetworkManager is assigned, then just exit early
             if (!networkManager)