Skip to content
1 change: 1 addition & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Fixed

- Fixed issue where either an `AttachableBehaviour` or an `AttachableNode` can throw an exception if they are attached during a scene unload where one of the two persists the scene unload event and the other does not. (#3931)
- Fixed issue where attempts to use `NetworkLog` when there is no `NetworkManager` instance would result in an exception. (#3917)

### Security
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,23 +261,30 @@ internal void ForceDetach()
ForceComponentChange(false, true);

InternalDetach();
// Notify of the changed attached state
NotifyAttachedStateChanged(m_AttachState, m_AttachableNode);

m_AttachedNodeReference = new NetworkBehaviourReference(null);

// When detaching, we want to make our final action
// the invocation of the AttachableNode's Detach method.
if (m_AttachableNode)
if (m_AttachableNode != null && !m_AttachableNode.IsDestroying)
{
// Notify of the changed attached state
NotifyAttachedStateChanged(m_AttachState, m_AttachableNode);
Comment thread
NoelStephensUnity marked this conversation as resolved.
// Only notify of the detach if the node is still valid.
m_AttachableNode.Detach(this);
m_AttachableNode = null;
}

m_AttachedNodeReference = new NetworkBehaviourReference(null);
m_AttachableNode = null;
}

/// <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();
Expand All @@ -286,7 +293,7 @@ public override void OnNetworkPreDespawn()
}

/// <summary>
/// This will apply the final attach or detatch state based on the current value of <see cref="m_AttachedNodeReference"/>.
/// This will apply the final attach or detach state based on the current value of <see cref="m_AttachedNodeReference"/>.
/// </summary>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void UpdateAttachedState()
Expand All @@ -304,16 +311,18 @@ private void UpdateAttachedState()
return;
}

// If we are attached to some other AttachableNode, then detach from that before attaching to a new one.
// If we are attaching and already attached to some other AttachableNode,
// then detach from that before attaching to a new one.
if (isAttaching && m_AttachableNode != null && m_AttachState == AttachState.Attached)
{
// Run through the same process without being triggerd by a NetVar update.
// Detach the current attachable
NotifyAttachedStateChanged(AttachState.Detaching, m_AttachableNode);
InternalDetach();
NotifyAttachedStateChanged(AttachState.Detached, m_AttachableNode);

m_AttachableNode.Detach(this);
m_AttachableNode = null;

// Now attach the new attachable
}

// Change the state to attaching or detaching
Expand Down Expand Up @@ -392,7 +401,8 @@ internal void ForceComponentChange(bool isAttaching, bool forcedChange)

foreach (var componentControllerEntry in ComponentControllers)
{
if (componentControllerEntry.AutoTrigger.HasFlag(triggerType))
// Only if the component controller still exists and has the appropriate flag.
if (componentControllerEntry.ComponentController && componentControllerEntry.AutoTrigger.HasFlag(triggerType))
Comment thread
NoelStephensUnity marked this conversation as resolved.
{
componentControllerEntry.ComponentController.ForceChangeEnabled(componentControllerEntry.EnableOnAttach ? isAttaching : !isAttaching, forcedChange);
}
Expand Down Expand Up @@ -457,7 +467,9 @@ public void Attach(AttachableNode attachableNode)
/// </summary>
internal void InternalDetach()
{
if (m_AttachableNode)
// If this instance is not in the middle of being destroyed, the attachable node is not null, and the node is not destroying
// =or= the scene it is located in is in the middle of being unloaded, then re-parent under the default parent.
if (!IsDestroying && m_AttachableNode && (!m_AttachableNode.IsDestroying || m_AttachableNode.gameObject.scene.isLoaded))
Comment thread
NoelStephensUnity marked this conversation as resolved.
{
if (m_DefaultParent)
{
Expand Down Expand Up @@ -553,12 +565,33 @@ private void UpdateAttachStateRpc(NetworkBehaviourReference attachedNodeReferenc
/// </summary>
internal void OnAttachNodeDestroy()
{
// If this instance should force a detach on destroy
if (AutoDetach.HasFlag(AutoDetachTypes.OnAttachNodeDestroy))
// We force a detach on destroy if there is a flag =or= if we are attached to a node that is being destroyed.
if (AutoDetach.HasFlag(AutoDetachTypes.OnAttachNodeDestroy) ||
(AutoDetach.HasFlag(AutoDetachTypes.OnDespawn) && m_AttachState == AttachState.Attached && m_AttachableNode && m_AttachableNode.IsDestroying))
Comment thread
NoelStephensUnity marked this conversation as resolved.
{
ForceDetach();
}
}


/// <summary>
/// When we know this instance is being destroyed or will be destroyed
/// by something outside of NGO's realm of control, this gets invoked.
/// We should detach from any AttachableNode when this is invoked.
/// </summary>
protected internal override void OnIsDestroying()
{
// If we are not already marked as being destroyed, attached, this instance is the authority instance, and the node we are attached
// to is not in the middle of being destroyed...detach normally.
if (!IsDestroying && HasAuthority && m_AttachState == AttachState.Attached && m_AttachableNode && !m_AttachableNode.IsDestroying)
{
Detach();
}
else // Otherwise force the detach.
{
// Force a detach
ForceDetach();
}
base.OnIsDestroying();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,20 +71,20 @@ public override void OnNetworkPreDespawn()
{
for (int i = m_AttachedBehaviours.Count - 1; i >= 0; i--)
{
if (!m_AttachedBehaviours[i])
var attachable = m_AttachedBehaviours[i];
if (!attachable)
{
continue;
}
// If we don't have authority but should detach on despawn,
// then proceed to detach.
if (!m_AttachedBehaviours[i].HasAuthority)

if (attachable.HasAuthority && attachable.IsSpawned)
{
m_AttachedBehaviours[i].ForceDetach();
// Detach the normal way with authority
attachable.Detach();
}
else
else if (!attachable.HasAuthority || !attachable.IsDestroying)
Comment thread
NoelStephensUnity marked this conversation as resolved.
{
// Detach the normal way with authority
m_AttachedBehaviours[i].Detach();
attachable.ForceDetach();
}
}
}
Expand All @@ -93,12 +93,10 @@ public override void OnNetworkPreDespawn()

internal override void InternalOnDestroy()
{
// Notify any attached behaviours that this node is being destroyed.
for (int i = m_AttachedBehaviours.Count - 1; i >= 0; i--)
if (m_AttachedBehaviours.Count > 0)
{
m_AttachedBehaviours[i]?.OnAttachNodeDestroy();
OnIsDestroying();
}
m_AttachedBehaviours.Clear();
base.InternalOnDestroy();
}

Expand Down Expand Up @@ -141,4 +139,21 @@ internal void Detach(AttachableBehaviour attachableBehaviour)
m_AttachedBehaviours.Remove(attachableBehaviour);
OnDetached(attachableBehaviour);
}

/// <summary>
/// When we know this instance is being destroyed or will be destroyed
/// by something outside of NGO's realm of control, this gets invoked.
/// We should notify any attached AttachableBehaviour that this node
/// is being destroyed.
/// </summary>
protected internal override void OnIsDestroying()
{
// Notify any attached behaviours that this node is being destroyed.
for (int i = m_AttachedBehaviours.Count - 1; i >= 0; i--)
{
m_AttachedBehaviours[i]?.OnAttachNodeDestroy();
}
m_AttachedBehaviours.Clear();
base.OnIsDestroying();
}
}
36 changes: 36 additions & 0 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,42 @@ 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.
/// </summary>
/// <remarks>
/// <see cref="SetIsDestroying"/>
/// </remarks>
internal bool IsDestroying { get; private set; }

/// <summary>
/// This provides us with a way to track when something is in the middle
/// of being destroyed or will be destroyed by something like SceneManager.
/// </summary>
protected internal virtual void OnIsDestroying()
{
}

/// <summary>
/// Invoked by <see cref="NetworkObject.SetIsDestroying"/>.
/// </summary>
/// <remarks>
/// We want to invoke the virtual method prior to setting the
/// IsDestroying flag to be able to distinguish between knowing
/// when something will be destroyed (i.e. scene manager unload
/// or load in single mode) or is in the middle of being
/// destroyed.
/// Setting the flag provides a way for other instances or internals
/// to determine if this <see cref="NetworkBehaviour"/> instance is
/// in the middle of being destroyed.
/// </remarks>
internal void SetIsDestroying()
{
// We intentionally invoke this before setting the IsDestroying flag.
OnIsDestroying();
Comment thread
NoelStephensUnity marked this conversation as resolved.
IsDestroying = true;
}

/// <summary>
/// Updates properties with network session related
/// dependencies such as a NetworkObject's spawned
Expand Down
44 changes: 44 additions & 0 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1688,8 +1688,52 @@ public static void NetworkHide(List<NetworkObject> networkObjects, ulong clientI
}
}

/// <summary>
/// Returns true if the NetworkObject is in the middle of being destroyed.
/// </summary>
/// <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>
internal void SetIsDestroying()
{
if (IsDestroying)
{
return;
}

if (m_ChildNetworkBehaviours != null)
{
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.SetIsDestroying();
}
}
IsDestroying = true;
}

private void OnDestroy()
{
// Apply the is destroying flag
SetIsDestroying();

var networkManager = NetworkManager;
// If no NetworkManager is assigned, then just exit early
if (!networkManager)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,10 +318,19 @@ public void MoveObjectsFromSceneToDontDestroyOnLoad(ref NetworkManager networkMa
}
else if (networkObject.HasAuthority)
{
// We know this instance is going to be destroyed (when it receives the destroy object message).
// We have to invoke this prior to invoking despawn in order to know that we are de-spawning in
// preparation of being destroyed by the SceneManager.
networkObject.SetIsDestroying();
// This sends a de-spawn message prior to the scene event.
networkObject.Despawn();
}
else // We are a client, migrate the object into the DDOL temporarily until it receives the destroy command from the server
{
// We know this instance is going to be destroyed (when it receives the destroy object message).
// We have to invoke this prior to invoking despawn in order to know that we are de-spawning in
// preparation of being destroyed by the SceneManager.
networkObject.SetIsDestroying();
UnityEngine.Object.DontDestroyOnLoad(networkObject.gameObject);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2709,7 +2709,11 @@ internal void MoveObjectsToDontDestroyOnLoad()
}
else if (networkObject.HasAuthority)
{
networkObject.Despawn();
networkObject.SetIsDestroying();
var isSceneObject = networkObject.IsSceneObject;
// Only destroy non-scene placed NetworkObjects to avoid warnings about destroying in-scene placed NetworkObjects.
// (MoveObjectsToDontDestroyOnLoad is only invoked during a scene event type of load and the load scene mode is single)
networkObject.Despawn(isSceneObject.HasValue && isSceneObject.Value == false);
}
}
}
Expand Down
Loading
Loading