Skip to content

Commit 2ad7ccb

Browse files
andrews-unityNoelStephensUnitynetcode-ci-service
authored
fix: OnNetworkDespawn not called during shutdown (#1390) (#1460)
* fix Renaming and refactoring DestroyNonSceneObjects to be DespawnNetworkObjects as this is really what it is doing. The destroy portion is either up to the handler, if an in-scene NetworkObject it lets the unloading of the scene destroy it, and the rest are always destroyed. * fix updates to the changes in NetworkSpawnManager * fix Adding a "ShutdownInProgress" flag to be able to easily check if we are shutting down. Adding a check for the NetworkManager.LocalClient since we are despawning in the shutdown method. Ignoring the de-parenting of NetworkObjects when shutting down (this will have to be discussed a bit). * fix Test fix: I think this was just an unexposed bug in the NetworkRigidBody2D test, but will verify this before making this final. Fixed some additional minor issues like checking for IsSpawned as opposed to NetworkManager.IsListening and IsSpawned as opposed to NetworkObject.IsSpawned. * fix Avoiding spam when despawning and shutting down. Renaming DespawnNetworkObjects to DespawnAndDestroyNetworkObjects. If a NetworkObject is not spawned it gets destroyed. * test fix Fixing issue with editor tests and the NetworkManager.Singleton instance. * fix Adjusting the logic a bit on getting the NetworkObject associated with a NetworkBehaviour. * revert and fix After discussing this with Luke, it turned out there was a bug in the NetworkRigidBody2D where it was resetting the isKinematic to true. This reverts the changes made to the NetworkRigidbody2DDynamicTest and fixes the bug with NetworkRigidBody2D. * test Tests that validate OnNetworkDespawn is invoked during shutdown. This tests server, host, and client relative instances. * update changelog update the changelog with the fixed information * fix fixing issue with test and recent changes for when the internal shutdown method is invoked. * refactor ShutdownInProgress now returns m_ShuttingDown. Setting m_ShuttingDown to false at the end of ShutdownInternal in case the NetworkManager is going to be re-used. * style line endings * Revert "style" This reverts commit e2ca11a. * fix fixing timing related issue for NetworkObjectOnNetworkDespawnTests. Co-authored-by: Noel Stephens <noel.stephens@unity3d.com> Co-authored-by: Unity Netcode CI <74025435+netcode-ci-service@users.noreply.github.com>
1 parent 8c84575 commit 2ad7ccb

File tree

8 files changed

+188
-26
lines changed

8 files changed

+188
-26
lines changed

‎com.unity.netcode.gameobjects/Components/NetworkRigidbody2D.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ private void Awake()
3232

3333
private void FixedUpdate()
3434
{
35-
if (NetworkManager.IsListening)
35+
if (IsSpawned)
3636
{
3737
if (HasAuthority != m_IsAuthority)
3838
{
@@ -74,7 +74,6 @@ public override void OnNetworkSpawn()
7474
/// <inheritdoc />
7575
public override void OnNetworkDespawn()
7676
{
77-
m_IsAuthority = false;
7877
UpdateRigidbodyKinematicMode();
7978
}
8079
}

‎com.unity.netcode.gameobjects/Components/NetworkTransform.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,7 @@ private void SetStateServerRpc(Vector3 pos, Quaternion rot, Vector3 scale, bool
807807
// conditional to users only making transform update changes in FixedUpdate.
808808
protected virtual void Update()
809809
{
810-
if (!NetworkObject.IsSpawned)
810+
if (!IsSpawned)
811811
{
812812
return;
813813
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,9 +282,13 @@ public NetworkObject NetworkObject
282282
m_NetworkObject = GetComponentInParent<NetworkObject>();
283283
}
284284

285-
if (m_NetworkObject == null && NetworkLog.CurrentLogLevel <= LogLevel.Normal)
285+
if (m_NetworkObject == null || NetworkManager.Singleton == null ||
286+
(NetworkManager.Singleton != null && !NetworkManager.Singleton.ShutdownInProgress))
286287
{
287-
NetworkLog.LogWarning($"Could not get {nameof(NetworkObject)} for the {nameof(NetworkBehaviour)}. Are you missing a {nameof(NetworkObject)} component?");
288+
if (NetworkLog.CurrentLogLevel < LogLevel.Normal)
289+
{
290+
NetworkLog.LogWarning($"Could not get {nameof(NetworkObject)} for the {nameof(NetworkBehaviour)}. Are you missing a {nameof(NetworkObject)} component?");
291+
}
288292
}
289293

290294
return m_NetworkObject;

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,9 @@ public IReadOnlyList<ulong> ConnectedClientsIds
336336
/// </summary>
337337
public bool IsConnectedClient { get; internal set; }
338338

339+
340+
public bool ShutdownInProgress { get { return m_ShuttingDown; } }
341+
339342
/// <summary>
340343
/// The callback to invoke once a client connects. This callback is only ran on the server and on the local client that connects.
341344
/// </summary>
@@ -1098,7 +1101,7 @@ internal void ShutdownInternal()
10981101
if (SpawnManager != null)
10991102
{
11001103
SpawnManager.CleanupAllTriggers();
1101-
SpawnManager.DestroyNonSceneObjects();
1104+
SpawnManager.DespawnAndDestroyNetworkObjects();
11021105
SpawnManager.ServerResetShudownStateForSceneObjects();
11031106

11041107
SpawnManager = null;

‎com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs‎

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ private bool TryGetNetworkClient(ulong clientId, out NetworkClient networkClient
225225
return NetworkManager.ConnectedClients.TryGetValue(clientId, out networkClient);
226226
}
227227

228-
if (clientId == NetworkManager.LocalClient.ClientId)
228+
if (NetworkManager.LocalClient != null && clientId == NetworkManager.LocalClient.ClientId)
229229
{
230230
networkClient = NetworkManager.LocalClient;
231231
return true;
@@ -598,25 +598,33 @@ internal void ServerDestroySpawnedSceneObjects()
598598
}
599599
}
600600

601-
internal void DestroyNonSceneObjects()
601+
internal void DespawnAndDestroyNetworkObjects()
602602
{
603603
var networkObjects = UnityEngine.Object.FindObjectsOfType<NetworkObject>();
604604

605605
for (int i = 0; i < networkObjects.Length; i++)
606606
{
607607
if (networkObjects[i].NetworkManager == NetworkManager)
608608
{
609-
if (networkObjects[i].IsSceneObject != null && networkObjects[i].IsSceneObject.Value == false)
609+
if (NetworkManager.PrefabHandler.ContainsHandler(networkObjects[i]))
610610
{
611-
if (NetworkManager.PrefabHandler.ContainsHandler(networkObjects[i]))
612-
{
613-
NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(networkObjects[i]);
614-
OnDespawnObject(networkObjects[i], false);
615-
}
616-
else
617-
{
618-
UnityEngine.Object.Destroy(networkObjects[i].gameObject);
619-
}
611+
OnDespawnObject(networkObjects[i], false);
612+
// Leave destruction up to the handler
613+
NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(networkObjects[i]);
614+
}
615+
else if (networkObjects[i].IsSpawned)
616+
{
617+
// If it is an in-scene placed NetworkObject then just despawn
618+
// and let it be destroyed when the scene is unloaded. Otherwise,
619+
// despawn and destroy it.
620+
var shouldDestroy = !(networkObjects[i].IsSceneObject != null
621+
&& networkObjects[i].IsSceneObject.Value);
622+
623+
OnDespawnObject(networkObjects[i], shouldDestroy);
624+
}
625+
else
626+
{
627+
UnityEngine.Object.Destroy(networkObjects[i].gameObject);
620628
}
621629
}
622630
}
@@ -686,17 +694,21 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
686694
return;
687695
}
688696

689-
// Move child NetworkObjects to the root when parent NetworkObject is destroyed
690-
foreach (var spawnedNetObj in SpawnedObjectsList)
697+
// If we are shutting down the NetworkManager, then ignore resetting the parent
698+
if (!NetworkManager.ShutdownInProgress)
691699
{
692-
var (isReparented, latestParent) = spawnedNetObj.GetNetworkParenting();
693-
if (isReparented && latestParent == networkObject.NetworkObjectId)
700+
// Move child NetworkObjects to the root when parent NetworkObject is destroyed
701+
foreach (var spawnedNetObj in SpawnedObjectsList)
694702
{
695-
spawnedNetObj.gameObject.transform.parent = null;
696-
697-
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
703+
var (isReparented, latestParent) = spawnedNetObj.GetNetworkParenting();
704+
if (isReparented && latestParent == networkObject.NetworkObjectId)
698705
{
699-
NetworkLog.LogWarning($"{nameof(NetworkObject)} #{spawnedNetObj.NetworkObjectId} moved to the root because its parent {nameof(NetworkObject)} #{networkObject.NetworkObjectId} is destroyed");
706+
spawnedNetObj.gameObject.transform.parent = null;
707+
708+
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
709+
{
710+
NetworkLog.LogWarning($"{nameof(NetworkObject)} #{spawnedNetObj.NetworkObjectId} moved to the root because its parent {nameof(NetworkObject)} #{networkObject.NetworkObjectId} is destroyed");
711+
}
700712
}
701713
}
702714
}

‎com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOnNetworkDespawnTests.cs‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
using System.Collections;
2+
using NUnit.Framework;
3+
using UnityEngine;
4+
using UnityEngine.TestTools;
5+
6+
7+
namespace Unity.Netcode.RuntimeTests
8+
{
9+
/// <summary>
10+
/// Tests that check OnNetworkDespawn being invoked
11+
/// </summary>
12+
public class NetworkObjectOnNetworkDespawnTests
13+
{
14+
private NetworkManager m_ServerHost;
15+
private NetworkManager[] m_Clients;
16+
17+
private GameObject m_ObjectToSpawn;
18+
private NetworkObject m_NetworkObject;
19+
20+
internal class OnNetworkDespawnTestComponent : NetworkBehaviour
21+
{
22+
public bool OnNetworkDespawnCalled { get; internal set; }
23+
24+
public override void OnNetworkSpawn()
25+
{
26+
OnNetworkDespawnCalled = false;
27+
base.OnNetworkSpawn();
28+
}
29+
30+
public override void OnNetworkDespawn()
31+
{
32+
OnNetworkDespawnCalled = true;
33+
base.OnNetworkDespawn();
34+
}
35+
}
36+
37+
[UnitySetUp]
38+
public IEnumerator Setup()
39+
{
40+
Assert.IsTrue(MultiInstanceHelpers.Create(1, out m_ServerHost, out m_Clients));
41+
42+
m_ObjectToSpawn = new GameObject();
43+
m_NetworkObject = m_ObjectToSpawn.AddComponent<NetworkObject>();
44+
m_ObjectToSpawn.AddComponent<OnNetworkDespawnTestComponent>();
45+
46+
// Make it a prefab
47+
MultiInstanceHelpers.MakeNetworkObjectTestPrefab(m_NetworkObject);
48+
49+
var networkPrefab = new NetworkPrefab();
50+
networkPrefab.Prefab = m_ObjectToSpawn;
51+
m_ServerHost.NetworkConfig.NetworkPrefabs.Add(networkPrefab);
52+
53+
foreach (var client in m_Clients)
54+
{
55+
client.NetworkConfig.NetworkPrefabs.Add(networkPrefab);
56+
}
57+
58+
yield return null;
59+
}
60+
61+
[UnityTearDown]
62+
public IEnumerator Teardown()
63+
{
64+
// Shutdown and clean up both of our NetworkManager instances
65+
if (m_ObjectToSpawn)
66+
{
67+
Object.Destroy(m_ObjectToSpawn);
68+
m_ObjectToSpawn = null;
69+
}
70+
MultiInstanceHelpers.Destroy();
71+
yield return null;
72+
}
73+
74+
public enum InstanceType
75+
{
76+
Server,
77+
Host,
78+
Client
79+
}
80+
81+
/// <summary>
82+
/// Tests that a spawned NetworkObject's associated NetworkBehaviours will have
83+
/// their OnNetworkDespawn invoked during NetworkManager shutdown.
84+
/// </summary>
85+
[UnityTest]
86+
public IEnumerator TestNetworkObjectDespawnOnShutdown([Values(InstanceType.Server, InstanceType.Host, InstanceType.Client)] InstanceType despawnCheck)
87+
{
88+
var useHost = despawnCheck == InstanceType.Server ? false : true;
89+
var networkManager = despawnCheck == InstanceType.Host || despawnCheck == InstanceType.Server ? m_ServerHost : m_Clients[0];
90+
91+
// Start the instances
92+
if (!MultiInstanceHelpers.Start(useHost, m_ServerHost, m_Clients))
93+
{
94+
Debug.LogError("Failed to start instances");
95+
Assert.Fail("Failed to start instances");
96+
}
97+
98+
// [Client-Side] Wait for a connection to the server
99+
yield return MultiInstanceHelpers.WaitForClientsConnected(m_Clients, null, 512);
100+
101+
// [Host-Server-Side] Check to make sure all clients are connected
102+
var clientCount = useHost ? m_Clients.Length + 1 : m_Clients.Length;
103+
yield return MultiInstanceHelpers.WaitForClientsConnectedToServer(m_ServerHost, clientCount, null, 512);
104+
105+
// Spawn the test object
106+
var spawnedObject = Object.Instantiate(m_NetworkObject);
107+
var spawnedNetworkObject = spawnedObject.GetComponent<NetworkObject>();
108+
spawnedNetworkObject.NetworkManagerOwner = m_ServerHost;
109+
spawnedNetworkObject.Spawn(true);
110+
111+
// Get the spawned object relative to which NetworkManager instance we are testing.
112+
var relativeSpawnedObject = new MultiInstanceHelpers.CoroutineResultWrapper<NetworkObject>();
113+
yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.GetNetworkObjectByRepresentation((x => x.GetComponent<OnNetworkDespawnTestComponent>() != null), networkManager, relativeSpawnedObject));
114+
var onNetworkDespawnTestComponent = relativeSpawnedObject.Result.GetComponent<OnNetworkDespawnTestComponent>();
115+
116+
// Confirm it is not set before shutting down the NetworkManager
117+
Assert.IsFalse(onNetworkDespawnTestComponent.OnNetworkDespawnCalled);
118+
119+
// Shutdown the NetworkManager instance we are testing.
120+
networkManager.Shutdown();
121+
122+
// Since shutdown is now delayed until the post frame update
123+
// just wait 2 frames before checking to see if OnNetworkDespawnCalled is true
124+
var currentFrame = Time.frameCount + 2;
125+
yield return new WaitUntil(() => Time.frameCount <= currentFrame);
126+
127+
// Confirm that OnNetworkDespawn is invoked after shutdown
128+
Assert.IsTrue(onNetworkDespawnTestComponent.OnNetworkDespawnCalled);
129+
}
130+
}
131+
}
132+

‎com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOnNetworkDespawnTests.cs.meta‎

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎com.unity.netcode.gameobjects/Tests/Runtime/Physics/NetworkRigidbody2DTest.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ public IEnumerator TestRigidbodyKinematicEnableDisable()
7070

7171
yield return NetworkRigidbodyTestBase.WaitForFrames(5);
7272

73+
// This should equal Kinematic
7374
Assert.IsTrue(serverPlayer.GetComponent<Rigidbody2D>().isKinematic == Kinematic);
7475

7576
yield return NetworkRigidbodyTestBase.WaitForFrames(5);

0 commit comments

Comments
 (0)