Skip to content

Commit c065fec

Browse files
NoelStephensUnityNoelStephensUnity
authored
fix: Reset NetworkClient upon shutdown [Backport] (#3494)
This PR addresses some NetworkManager instance clean up when shutdown including the ConnectionManager's NetworkClient. This PR also includes a clean up for NetworkObject upon despawning in the event one is using an object pool. [MTT-12384](https://jira.unity3d.com/browse/MTT-12384) ## Changelog - Fixed: Issue where `NetworkClient` could persist some settings if re-using the same `NetworkManager` instance. - Fixed: Issue where a pooled `NetworkObject` was not resetting the internal latest parent property when despawned. ## Testing and Documentation - Includes integration test updates. - No documentation changes or additions were necessary. <!-- Uncomment and mark items off with a * if this PR deprecates any API: ### Deprecated API - [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter yyyy-mm-dd)` entry. - [ ] An [api updater] was added. - [ ] Deprecation of the API is explained in the CHANGELOG. - [ ] The users can understand why this API was removed and what they should use instead. --> ## Backport This is a back port of #3491. <!-- If this is a backport: - Add the following to the PR title: "\[Backport\] ..." . - Link to the original PR. If this needs a backport - state this here If a backport is not needed please provide the reason why. If the "Backports" section is not present it will lead to a CI test failure. -->
1 parent 0466b85 commit c065fec

File tree

6 files changed

+84
-10
lines changed

6 files changed

+84
-10
lines changed

‎com.unity.netcode.gameobjects/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
1515

1616
### Fixed
1717

18+
- Fixed issue where `NetworkClient` could persist some settings if re-using the same `NetworkManager` instance. (#3494)
19+
- Fixed issue where a pooled `NetworkObject` was not resetting the internal latest parent property when despawned. (#3494)
1820
- Fixed issue where the initial client synchronization pre-serialization process was not excluding spawned `NetworkObjects` that already had pending visibility for the client being synchronized. (#3493)
1921
- Fixed issue where invoking `NetworkObject.NetworkShow` and `NetworkObject.ChangeOwnership` consecutively within the same call stack location could result in an unnecessary change in ownership error message generated on the target client side. (#3493)
2022
- Fixed issue where `NetworkVariable`s on a `NetworkBehaviour` could fail to synchronize changes if one has `NetworkVariableUpdateTraits` set and is dirty but is not ready to send. (#3465)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,8 +1265,8 @@ internal void ShutdownInternal()
12651265
// In the event shutdown is invoked within OnClientStopped or OnServerStopped, set it to false again
12661266
m_ShuttingDown = false;
12671267

1268-
// Reset the client's roles
1269-
ConnectionManager.LocalClient.SetRole(false, false);
1268+
// Completely reset the NetworkClient
1269+
ConnectionManager.LocalClient = new NetworkClient();
12701270

12711271
// This cleans up the internal prefabs list
12721272
NetworkConfig?.Prefabs?.Shutdown();

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -913,6 +913,14 @@ public void Despawn(bool destroy = true)
913913
NetworkManager.SpawnManager.DespawnObject(this, destroy);
914914
}
915915

916+
internal void ResetOnDespawn()
917+
{
918+
// Always clear out the observers list when despawned
919+
Observers.Clear();
920+
IsSpawned = false;
921+
m_LatestParent = null;
922+
}
923+
916924
/// <summary>
917925
/// Removes all ownership of an object from any client. Can only be called from server
918926
/// </summary>

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,15 +1191,13 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
11911191
}
11921192
}
11931193

1194-
networkObject.IsSpawned = false;
1195-
11961194
if (SpawnedObjects.Remove(networkObject.NetworkObjectId))
11971195
{
11981196
SpawnedObjectsList.Remove(networkObject);
11991197
}
12001198

1201-
// Always clear out the observers list when despawned
1202-
networkObject.Observers.Clear();
1199+
// Reset the NetworkObject when despawned.
1200+
networkObject.ResetOnDespawn();
12031201

12041202
var gobj = networkObject.gameObject;
12051203
if (destroyGameObject && gobj != null)

‎com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,18 +150,27 @@ public IEnumerator ClientPlayerDisconnected([Values] ClientDisconnectType client
150150

151151
if (clientDisconnectType == ClientDisconnectType.ServerDisconnectsClient)
152152
{
153-
m_ClientNetworkManagers[0].OnClientDisconnectCallback += OnClientDisconnectCallback;
154-
m_ClientNetworkManagers[0].OnConnectionEvent += OnConnectionEvent;
153+
clientManager.OnClientDisconnectCallback += OnClientDisconnectCallback;
154+
clientManager.OnConnectionEvent += OnConnectionEvent;
155155
m_ServerNetworkManager.OnConnectionEvent += OnConnectionEvent;
156156
m_ServerNetworkManager.DisconnectClient(m_ClientId);
157157
}
158158
else
159159
{
160160
m_ServerNetworkManager.OnClientDisconnectCallback += OnClientDisconnectCallback;
161161
m_ServerNetworkManager.OnConnectionEvent += OnConnectionEvent;
162-
m_ClientNetworkManagers[0].OnConnectionEvent += OnConnectionEvent;
162+
clientManager.OnConnectionEvent += OnConnectionEvent;
163163

164-
yield return StopOneClient(m_ClientNetworkManagers[0]);
164+
yield return StopOneClient(clientManager);
165+
166+
if (clientManager.ConnectionManager != null)
167+
{
168+
Assert.False(clientManager.ConnectionManager.LocalClient.IsClient, $"{clientManager.name} still has IsClient setting!");
169+
Assert.False(clientManager.ConnectionManager.LocalClient.IsConnected, $"{clientManager.name} still has IsConnected setting!");
170+
Assert.False(clientManager.ConnectionManager.LocalClient.ClientId != 0, $"{clientManager.name} still has ClientId ({clientManager.ConnectionManager.LocalClient.ClientId}) setting!");
171+
Assert.False(clientManager.ConnectionManager.LocalClient.IsApproved, $"{clientManager.name} still has IsApproved setting!");
172+
Assert.IsNull(clientManager.ConnectionManager.LocalClient.PlayerObject, $"{clientManager.name} still has Player assigned!");
173+
}
165174
}
166175

167176
yield return WaitForConditionOrTimeOut(() => m_ClientDisconnected);
@@ -216,6 +225,15 @@ public IEnumerator ClientPlayerDisconnected([Values] ClientDisconnectType client
216225

217226
Assert.IsTrue(m_DisconnectedEvent.ContainsKey(m_ServerNetworkManager), $"Could not find the server {nameof(NetworkManager)} disconnect event entry!");
218227
Assert.IsTrue(m_DisconnectedEvent[m_ServerNetworkManager].ClientId == NetworkManager.ServerClientId, $"Expected ClientID {m_ClientId} but found ClientID {m_DisconnectedEvent[m_ServerNetworkManager].ClientId} for the server {nameof(NetworkManager)} disconnect event entry!");
228+
yield return s_DefaultWaitForTick;
229+
if (m_ServerNetworkManager.ConnectionManager != null)
230+
{
231+
Assert.False(m_ServerNetworkManager.ConnectionManager.LocalClient.IsClient, $"{m_ServerNetworkManager.name} still has IsClient setting!");
232+
Assert.False(m_ServerNetworkManager.ConnectionManager.LocalClient.IsConnected, $"{m_ServerNetworkManager.name} still has IsConnected setting!");
233+
Assert.False(m_ServerNetworkManager.ConnectionManager.LocalClient.ClientId != 0, $"{m_ServerNetworkManager.name} still has ClientId ({clientManager.ConnectionManager.LocalClient.ClientId}) setting!");
234+
Assert.False(m_ServerNetworkManager.ConnectionManager.LocalClient.IsApproved, $"{m_ServerNetworkManager.name} still has IsApproved setting!");
235+
Assert.IsNull(m_ServerNetworkManager.ConnectionManager.LocalClient.PlayerObject, $"{m_ServerNetworkManager.name} still has Player assigned!");
236+
}
219237
}
220238
}
221239
}

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,5 +352,53 @@ public override void OnNetworkDespawn()
352352
OnNetworkDespawnCalledCount++;
353353
}
354354
}
355+
356+
private bool AllClientsSpawnedObject()
357+
{
358+
foreach (var networkManager in m_ClientNetworkManagers)
359+
{
360+
if (!networkManager.SpawnManager.SpawnedObjects.ContainsKey(m_SpawnedInstanceId))
361+
{
362+
return false;
363+
}
364+
}
365+
return true;
366+
}
367+
368+
private bool AllClientsDespawnedObject()
369+
{
370+
foreach (var networkManager in m_ClientNetworkManagers)
371+
{
372+
if (networkManager.SpawnManager.SpawnedObjects.ContainsKey(m_SpawnedInstanceId))
373+
{
374+
return false;
375+
}
376+
}
377+
return true;
378+
}
379+
380+
private ulong m_SpawnedInstanceId;
381+
/// <summary>
382+
/// Validates that NetworkObject is reset properly when despawned but not destroyed.
383+
/// </summary>
384+
/// <returns>IEnumerator</returns>
385+
[UnityTest]
386+
public IEnumerator NetworkObjectResetOnDespawn()
387+
{
388+
var authorityNetworkManager = m_ServerNetworkManager;
389+
var instance = SpawnObject(m_ObserverPrefab, authorityNetworkManager).GetComponent<NetworkObject>();
390+
m_SpawnedInstanceId = instance.NetworkObjectId;
391+
yield return WaitForConditionOrTimeOut(AllClientsSpawnedObject);
392+
AssertOnTimeout($"Not all clients spawned an instance of {instance.name}!");
393+
394+
instance.Despawn(false);
395+
396+
yield return WaitForConditionOrTimeOut(AllClientsDespawnedObject);
397+
AssertOnTimeout($"Not all clients de-spawned an instance of {instance.name}!");
398+
399+
Assert.IsNull(instance.GetNetworkParenting(), "Last parent was not reset!");
400+
401+
Object.Destroy(instance.gameObject);
402+
}
355403
}
356404
}

0 commit comments

Comments
 (0)