Skip to content

fix: dont destroy with owner not being honored when client exits #3477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Fixed

- Fixed issue where the `NetworkObject.DontDestroyWithOwner` was not being honored. (#3477)
- Fixed issue where non-authority `NetworkTransform` instances would not allow non-synchronized axis values to be updated locally. (#3471)
- 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)
- 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
{
// If destroying with owner, then always despawn and destroy (or defer destroying to prefab handler)
// Handle an object with no observers other than the current disconnecting client as destroying with owner
if (!ownedObject.DontDestroyWithOwner || ownedObject.Observers.Count == 0 || (ownedObject.Observers.Count == 1 && ownedObject.Observers.Contains(clientId)))
if (!ownedObject.DontDestroyWithOwner && (ownedObject.Observers.Count == 0 || (ownedObject.Observers.Count == 1 && ownedObject.Observers.Contains(clientId))))
{
if (NetworkManager.PrefabHandler.ContainsHandler(ownedObject.GlobalObjectIdHash))
{
Expand Down Expand Up @@ -1244,7 +1244,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
}

// Skip destroy with owner objects as they will be processed by the outer loop
if (!childObject.DontDestroyWithOwner || childObject.Observers.Count == 0 || (childObject.Observers.Count == 1 && childObject.Observers.Contains(clientId)))
if (!childObject.DontDestroyWithOwner && (childObject.Observers.Count == 0 || (childObject.Observers.Count == 1 && childObject.Observers.Contains(clientId))))
{
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,19 @@ protected override bool UseCMBService()
}

protected GameObject m_PrefabToSpawn;
protected GameObject m_PrefabNoObserversSpawn;

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

protected override void OnServerAndClientsCreated()
{
m_PrefabToSpawn = CreateNetworkObjectPrefab("ClientOwnedObject");
m_PrefabToSpawn.GetComponent<NetworkObject>().DontDestroyWithOwner = true;

m_PrefabNoObserversSpawn = CreateNetworkObjectPrefab("NoObserversObject");
var prefabNoObserversNetworkObject = m_PrefabNoObserversSpawn.GetComponent<NetworkObject>();
prefabNoObserversNetworkObject.SpawnWithObservers = false;
prefabNoObserversNetworkObject.DontDestroyWithOwner = true;
}

[UnityTest]
Expand Down Expand Up @@ -74,5 +80,30 @@ public IEnumerator DontDestroyWithOwnerTest()
Assert.That(networkObject.OwnerClientId == m_ServerNetworkManager.LocalClientId);
}
}

[UnityTest]
public IEnumerator NetworkShowThenClientDisconnects()
{
var authorityManager = GetAuthorityNetworkManager();
var networkObject = SpawnObject(m_PrefabNoObserversSpawn, authorityManager).GetComponent<NetworkObject>();
var longWait = new WaitForSeconds(0.25f);
yield return longWait;
var nonAuthorityManager = GetNonAuthorityNetworkManager();
Assert.False(nonAuthorityManager.SpawnManager.SpawnedObjects.ContainsKey(networkObject.NetworkObjectId), $"[Client-{nonAuthorityManager.LocalClientId}] " +
$"Already has an instance of {networkObject.name} when it should not!");
networkObject.NetworkShow(nonAuthorityManager.LocalClientId);
networkObject.ChangeOwnership(nonAuthorityManager.LocalClientId);

yield return WaitForConditionOrTimeOut(() => nonAuthorityManager.SpawnManager.SpawnedObjects.ContainsKey(networkObject.NetworkObjectId)
&& nonAuthorityManager.SpawnManager.SpawnedObjects[networkObject.NetworkObjectId].OwnerClientId == nonAuthorityManager.LocalClientId);
AssertOnTimeout($"[Client-{nonAuthorityManager.LocalClientId}] Failed to spawn {networkObject.name} when it was shown!");

yield return s_DefaultWaitForTick;

nonAuthorityManager.Shutdown();

yield return longWait;
Assert.True(networkObject.IsSpawned, $"The spawned test prefab was despawned on the authority side when it shouldn't have been!");
}
}
}