Skip to content

Commit 985453a

Browse files
fix: don't destroy with owner not being honored when client exits (#3477)
Based on issue #3472, this PR resolves the issue where the `NetworkObject.DontDestroyWithOwner` was not being honored. <!-- Add short version of the JIRA ticket to the PR title (e.g. "feat: new shiny feature [MTT-123]") --> ## Changelog - Fixed: Issue where the `NetworkObject.DontDestroyWithOwner` was not being honored. ## Testing and Documentation - Includes integration test `NetworkObjectDontDestroyWithOwnerTests.NetworkShowThenClientDisconnects`. - 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 The v1.x branch does no have this issue, but went ahead and backported the test in #3478. <!-- 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 e24c2e5 commit 985453a

File tree

3 files changed

+34
-2
lines changed

3 files changed

+34
-2
lines changed

com.unity.netcode.gameobjects/CHANGELOG.md

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

1616
### Fixed
1717

18+
- Fixed issue where the `NetworkObject.DontDestroyWithOwner` was not being honored. (#3477)
1819
- Fixed issue where non-authority `NetworkTransform` instances would not allow non-synchronized axis values to be updated locally. (#3471)
1920
- 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. (#3468)
2021
- 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. (#3466)

com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,7 +1178,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
11781178
{
11791179
// If destroying with owner, then always despawn and destroy (or defer destroying to prefab handler)
11801180
// Handle an object with no observers other than the current disconnecting client as destroying with owner
1181-
if (!ownedObject.DontDestroyWithOwner || ownedObject.Observers.Count == 0 || (ownedObject.Observers.Count == 1 && ownedObject.Observers.Contains(clientId)))
1181+
if (!ownedObject.DontDestroyWithOwner && (ownedObject.Observers.Count == 0 || (ownedObject.Observers.Count == 1 && ownedObject.Observers.Contains(clientId))))
11821182
{
11831183
if (NetworkManager.PrefabHandler.ContainsHandler(ownedObject.GlobalObjectIdHash))
11841184
{
@@ -1244,7 +1244,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
12441244
}
12451245

12461246
// Skip destroy with owner objects as they will be processed by the outer loop
1247-
if (!childObject.DontDestroyWithOwner || childObject.Observers.Count == 0 || (childObject.Observers.Count == 1 && childObject.Observers.Contains(clientId)))
1247+
if (!childObject.DontDestroyWithOwner && (childObject.Observers.Count == 0 || (childObject.Observers.Count == 1 && childObject.Observers.Contains(clientId))))
12481248
{
12491249
continue;
12501250
}

com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDontDestroyWithOwnerTests.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,19 @@ protected override bool UseCMBService()
2323
}
2424

2525
protected GameObject m_PrefabToSpawn;
26+
protected GameObject m_PrefabNoObserversSpawn;
2627

2728
public NetworkObjectDontDestroyWithOwnerTests(HostOrServer hostOrServer) : base(hostOrServer) { }
2829

2930
protected override void OnServerAndClientsCreated()
3031
{
3132
m_PrefabToSpawn = CreateNetworkObjectPrefab("ClientOwnedObject");
3233
m_PrefabToSpawn.GetComponent<NetworkObject>().DontDestroyWithOwner = true;
34+
35+
m_PrefabNoObserversSpawn = CreateNetworkObjectPrefab("NoObserversObject");
36+
var prefabNoObserversNetworkObject = m_PrefabNoObserversSpawn.GetComponent<NetworkObject>();
37+
prefabNoObserversNetworkObject.SpawnWithObservers = false;
38+
prefabNoObserversNetworkObject.DontDestroyWithOwner = true;
3339
}
3440

3541
[UnityTest]
@@ -74,5 +80,30 @@ public IEnumerator DontDestroyWithOwnerTest()
7480
Assert.That(networkObject.OwnerClientId == m_ServerNetworkManager.LocalClientId);
7581
}
7682
}
83+
84+
[UnityTest]
85+
public IEnumerator NetworkShowThenClientDisconnects()
86+
{
87+
var authorityManager = GetAuthorityNetworkManager();
88+
var networkObject = SpawnObject(m_PrefabNoObserversSpawn, authorityManager).GetComponent<NetworkObject>();
89+
var longWait = new WaitForSeconds(0.25f);
90+
yield return longWait;
91+
var nonAuthorityManager = GetNonAuthorityNetworkManager();
92+
Assert.False(nonAuthorityManager.SpawnManager.SpawnedObjects.ContainsKey(networkObject.NetworkObjectId), $"[Client-{nonAuthorityManager.LocalClientId}] " +
93+
$"Already has an instance of {networkObject.name} when it should not!");
94+
networkObject.NetworkShow(nonAuthorityManager.LocalClientId);
95+
networkObject.ChangeOwnership(nonAuthorityManager.LocalClientId);
96+
97+
yield return WaitForConditionOrTimeOut(() => nonAuthorityManager.SpawnManager.SpawnedObjects.ContainsKey(networkObject.NetworkObjectId)
98+
&& nonAuthorityManager.SpawnManager.SpawnedObjects[networkObject.NetworkObjectId].OwnerClientId == nonAuthorityManager.LocalClientId);
99+
AssertOnTimeout($"[Client-{nonAuthorityManager.LocalClientId}] Failed to spawn {networkObject.name} when it was shown!");
100+
101+
yield return s_DefaultWaitForTick;
102+
103+
nonAuthorityManager.Shutdown();
104+
105+
yield return longWait;
106+
Assert.True(networkObject.IsSpawned, $"The spawned test prefab was despawned on the authority side when it shouldn't have been!");
107+
}
77108
}
78109
}

0 commit comments

Comments
 (0)