Skip to content
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

fix: Destroying player object removes observers. #3085

Open
wants to merge 3 commits into
base: develop-2.0.0
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -1508,20 +1508,6 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
SpawnedObjectsList.Remove(networkObject);
}

// DANGO-TODO: When we fix the issue with observers not being applied to NetworkObjects,
// (client connect/disconnect) we can remove this hacky way of doing this.
// Basically, when a player disconnects and/or is destroyed they are removed as an observer from all other client
// NetworkOject instances.
if (networkObject.IsPlayerObject && !networkObject.IsOwner && networkObject.OwnerClientId != NetworkManager.LocalClientId)
{
foreach (var netObject in SpawnedObjects)
{
if (netObject.Value.Observers.Contains(networkObject.OwnerClientId))
{
netObject.Value.Observers.Remove(networkObject.OwnerClientId);
}
}
}
if (networkObject.IsPlayerObject)
{
RemovePlayerObject(networkObject);
Expand Down
139 changes: 139 additions & 0 deletions com.unity.netcode.gameobjects/Tests/Runtime/PlayerObjectTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,24 @@

namespace Unity.Netcode.RuntimeTests
{
public class ObserversTestClass : NetworkBehaviour
{
public NetworkVariable<int> NetVariable = new NetworkVariable<int>();

public bool ClientRPCCalled;

public void ResetRPCState()
{
ClientRPCCalled = false;
}

[ClientRpc]
public void TestClientRpc()
{
ClientRPCCalled = true;
}
}

[TestFixture(HostOrServer.DAHost)]
[TestFixture(HostOrServer.Host)]
[TestFixture(HostOrServer.Server)]
Expand Down Expand Up @@ -120,4 +138,125 @@ public IEnumerator SpawnWithNoObservers()
}
}
}

[TestFixture(HostOrServer.DAHost)]
[TestFixture(HostOrServer.Host)]
[TestFixture(HostOrServer.Server)]
internal class PlayerObjectSpawnTests : NetcodeIntegrationTest
{
protected override int NumberOfClients => 1;

protected GameObject m_NewPlayerToSpawn;
protected GameObject m_TestObserversObject;

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

protected override void OnServerAndClientsCreated()
{
m_TestObserversObject = CreateNetworkObjectPrefab("ObserversTest");
m_TestObserversObject.AddComponent<ObserversTestClass>();

m_NewPlayerToSpawn = CreateNetworkObjectPrefab("NewPlayerInstance");
base.OnServerAndClientsCreated();
}

private int GetObserverCount( System.Collections.Generic.HashSet<ulong>.Enumerator observerEnumerator )
{
int observerCount = 0;
while (observerEnumerator.MoveNext())
{
observerCount++;
}

return observerCount;
}
private ObserversTestClass GetObserversTestClassObjectForClient(ulong clientId)
{
#if UNITY_2023_1_OR_NEWER
var emptyComponents = Object.FindObjectsByType<ObserversTestClass>(FindObjectsSortMode.InstanceID);
#else
var emptyComponents = UnityEngine.Object.FindObjectsOfType<EmptyComponent>();
#endif
foreach (var component in emptyComponents)
{
if (component.IsSpawned && component.NetworkManager.LocalClientId == clientId)
{
return component;
}
}
return null;
}

// https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues/3059 surfaced a regression from 1.11 to 2.0
// where Destroying (not Disconnecting) a player object would remove it from all NetworkObjects observer arrays. Upon recreating
// the player object they were no longer an observer for exising network objects causing them to miss NetworkVariable and RPC updates.
// This test covers that case including testing RPCs and NetworkVariables still function after recreating the player object
[UnityTest]
public IEnumerator SpawnDestoryRespawnPlayerObjectMaintainsObservers()
{
yield return WaitForConditionOrTimeOut(() => m_PlayerNetworkObjects[m_ServerNetworkManager.LocalClientId].ContainsKey(m_ClientNetworkManagers[0].LocalClientId));
AssertOnTimeout("Timed out waiting for client-side player object to spawn!");

// So on the server we want to spawn the observer object and then check its observers
var serverObserverObject = SpawnObject(m_TestObserversObject, m_ServerNetworkManager);
var serverObserverComponent = serverObserverObject.GetComponent<ObserversTestClass>();
yield return WaitForConditionOrTimeOut(() => GetObserversTestClassObjectForClient(m_ClientNetworkManagers[0].LocalClientId) != null);
AssertOnTimeout("Timed out waiting for client-side observer object to spawn!");

var clientObserverComponent = GetObserversTestClassObjectForClient(m_ClientNetworkManagers[0].LocalClientId);
Assert.NotNull(clientObserverComponent);
Assert.AreNotEqual(serverObserverComponent, clientObserverComponent, "Client and Server Observer Test components are equal, the test is wrong.");

// The server object should be the owner
Assert.IsTrue(serverObserverObject.GetComponent<ObserversTestClass>().IsOwner);
Assert.IsFalse(clientObserverComponent.IsOwner);

// Test Networkvariables and RPCs function as expected
serverObserverComponent.NetVariable.Value = 123;
yield return WaitForConditionOrTimeOut(() => clientObserverComponent.NetVariable.Value == 123);
AssertOnTimeout("Timed out waiting for network variable to transmit!");

serverObserverComponent.TestClientRpc();
yield return WaitForConditionOrTimeOut(() => clientObserverComponent.ClientRPCCalled);
AssertOnTimeout("Timed out waiting for RPC to be called!");

serverObserverComponent.ResetRPCState();
clientObserverComponent.ResetRPCState();

// Destory the clients player object, this will remove the player object but not disconnect the client, it should leave the connection intact
bool foundPlayer = false;
ulong destroyedClientId = 0;
foreach( var c in m_ServerNetworkManager.ConnectedClients)
{
if (!c.Value.PlayerObject.GetComponent<NetworkObject>().IsOwner)
{
destroyedClientId = c.Key;
Object.Destroy(c.Value.PlayerObject);
foundPlayer = true;
break;
}
}
Assert.True(foundPlayer);

yield return WaitForConditionOrTimeOut(() => m_ServerNetworkManager.ConnectedClients[destroyedClientId].PlayerObject == null);
AssertOnTimeout("Timed out waiting for player object to be destroyed!");


// so lets respawn the player here
var newPlayer = Object.Instantiate(m_NewPlayerToSpawn);
newPlayer.GetComponent<NetworkObject>().SpawnAsPlayerObject(destroyedClientId);

yield return WaitForConditionOrTimeOut(() => m_ServerNetworkManager.ConnectedClients[destroyedClientId].PlayerObject != null);
AssertOnTimeout("Timed out waiting for player object to respawn!");

// Test Networkvariables and RPCs function as expected after recreating the client player object
serverObserverComponent.NetVariable.Value = 321;
yield return WaitForConditionOrTimeOut(() => clientObserverComponent.NetVariable.Value == 321);
AssertOnTimeout("Timed out waiting for network variable to transmit after respawn!");

serverObserverComponent.TestClientRpc();
yield return WaitForConditionOrTimeOut(() => clientObserverComponent.ClientRPCCalled);
AssertOnTimeout("Timed out waiting for RPC to be called after respawn!");
}
}
}
Loading