Skip to content

Commit

Permalink
fix: player prefab spawning not honoring when spawnwithobservers is d…
Browse files Browse the repository at this point in the history
…isabled (#3077)

* fix

When SpawnWithObservers is disabled on player prefab, the instantiated instance should only spawn on the authority side.
For client-server this will always be the server/host.
For distributed authority this will always be the owner.

* test

Validation test for the SpawnWithObservers regression bug.

* update

updating changelog entry
  • Loading branch information
NoelStephensUnity authored Sep 27, 2024
1 parent 390c332 commit cb7ec98
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 8 deletions.
1 change: 1 addition & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
### Fixed

- Fixed issue where applying the position and/or rotation to the `NetworkManager.ConnectionApprovalResponse` when connection approval and auto-spawn player prefab were enabled would not apply the position and/or rotation when the player prefab was instantiated. (#3078)
- Fixed issue where `NetworkObject.SpawnWithObservers` was not being honored when spawning the player prefab. (#3077)
- Fixed issue with the client count not being correct on the host or server side when a client disconnects itself from a session. (#3075)

### Changed
Expand Down
18 changes: 14 additions & 4 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1613,7 +1613,12 @@ internal void SpawnInternal(bool destroyWithScene, ulong ownerClientId, bool pla
}
else if (NetworkManager.DistributedAuthorityMode && !NetworkManager.DAHost)
{
NetworkManager.SpawnManager.SendSpawnCallForObject(NetworkManager.ServerClientId, this);
// If spawning with observers or if not spawning with observers but the observer count is greater than 1 (i.e. owner/authority creating),
// then we want to send a spawn notification.
if (SpawnWithObservers || !SpawnWithObservers && Observers.Count > 1)
{
NetworkManager.SpawnManager.SendSpawnCallForObject(NetworkManager.ServerClientId, this);
}
}
else
{
Expand Down Expand Up @@ -3053,10 +3058,15 @@ internal static NetworkObject AddSceneObject(in SceneObject sceneObject, FastBuf
}
}

// Add all known players to the observers list if they don't already exist
foreach (var player in networkManager.SpawnManager.PlayerObjects)
// Only add all other players as observers if we are spawning with observers,
// otherwise user controls via NetworkShow.
if (networkObject.SpawnWithObservers)
{
networkObject.Observers.Add(player.OwnerClientId);
// Add all known players to the observers list if they don't already exist
foreach (var player in networkManager.SpawnManager.PlayerObjects)
{
networkObject.Observers.Add(player.OwnerClientId);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,22 @@ private void AddPlayerObject(NetworkObject playerObject)
return;
}
}

foreach (var player in m_PlayerObjects)
{
player.Observers.Add(playerObject.OwnerClientId);
playerObject.Observers.Add(player.OwnerClientId);
// If the player's SpawnWithObservers is not set then do not add the new player object's owner as an observer.
if (player.SpawnWithObservers)
{
player.Observers.Add(playerObject.OwnerClientId);
}

// If the new player object's SpawnWithObservers is not set then do not add this player as an observer to the new player object.
if (playerObject.SpawnWithObservers)
{
playerObject.Observers.Add(player.OwnerClientId);
}
}

m_PlayerObjects.Add(playerObject);
if (!m_PlayerObjectsTable.ContainsKey(playerObject.OwnerClientId))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -545,9 +545,14 @@ protected IEnumerator CreateAndStartNewClient()
private bool AllPlayerObjectClonesSpawned(NetworkManager joinedClient)
{
m_InternalErrorLog.Clear();
// If we are not checking for spawned players then exit early with a success
if (!ShouldCheckForSpawnedPlayers())
{
return true;
}

// Continue to populate the PlayerObjects list until all player object (local and clone) are found
ClientNetworkManagerPostStart(joinedClient);

var playerObjectRelative = m_ServerNetworkManager.SpawnManager.PlayerObjects.Where((c) => c.OwnerClientId == joinedClient.LocalClientId).FirstOrDefault();
if (playerObjectRelative == null)
{
Expand Down
70 changes: 69 additions & 1 deletion com.unity.netcode.gameobjects/Tests/Runtime/PlayerObjectTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace Unity.Netcode.RuntimeTests
[TestFixture(HostOrServer.Server)]
internal class PlayerObjectTests : NetcodeIntegrationTest
{
protected override int NumberOfClients => 1;
protected override int NumberOfClients => 2;

protected GameObject m_NewPlayerToSpawn;

Expand Down Expand Up @@ -52,4 +52,72 @@ public IEnumerator SpawnAndReplaceExistingPlayerObject()
Assert.False(s_GlobalTimeoutHelper.TimedOut, "Timed out waiting for client-side player object to change!");
}
}

/// <summary>
/// Validate that when auto-player spawning but SpawnWithObservers is disabled,
/// the player instantiated is only spawned on the authority side.
/// </summary>
[TestFixture(HostOrServer.DAHost)]
[TestFixture(HostOrServer.Host)]
[TestFixture(HostOrServer.Server)]
internal class PlayerSpawnNoObserversTest : NetcodeIntegrationTest
{
protected override int NumberOfClients => 2;

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

protected override bool ShouldCheckForSpawnedPlayers()
{
return false;
}

protected override void OnCreatePlayerPrefab()
{
var playerNetworkObject = m_PlayerPrefab.GetComponent<NetworkObject>();
playerNetworkObject.SpawnWithObservers = false;
base.OnCreatePlayerPrefab();
}

[UnityTest]
public IEnumerator SpawnWithNoObservers()
{
yield return s_DefaultWaitForTick;

if (!m_DistributedAuthority)
{
// Make sure clients did not spawn their player object on any of the clients including the owner.
foreach (var client in m_ClientNetworkManagers)
{
foreach (var playerObject in m_ServerNetworkManager.SpawnManager.PlayerObjects)
{
Assert.IsFalse(client.SpawnManager.SpawnedObjects.ContainsKey(playerObject.NetworkObjectId), $"Client-{client.LocalClientId} spawned player object for Client-{playerObject.NetworkObjectId}!");
}
}
}
else
{
// For distributed authority, we want to make sure the player object is only spawned on the authority side and all non-authority instances did not spawn it.
var playerObjectId = m_ServerNetworkManager.LocalClient.PlayerObject.NetworkObjectId;
foreach (var client in m_ClientNetworkManagers)
{
Assert.IsFalse(client.SpawnManager.SpawnedObjects.ContainsKey(playerObjectId), $"Client-{client.LocalClientId} spawned player object for Client-{m_ServerNetworkManager.LocalClientId}!");
}

foreach (var clientPlayer in m_ClientNetworkManagers)
{
playerObjectId = clientPlayer.LocalClient.PlayerObject.NetworkObjectId;
Assert.IsFalse(m_ServerNetworkManager.SpawnManager.SpawnedObjects.ContainsKey(playerObjectId), $"Client-{m_ServerNetworkManager.LocalClientId} spawned player object for Client-{clientPlayer.LocalClientId}!");
foreach (var client in m_ClientNetworkManagers)
{
if (clientPlayer == client)
{
continue;
}
Assert.IsFalse(client.SpawnManager.SpawnedObjects.ContainsKey(playerObjectId), $"Client-{client.LocalClientId} spawned player object for Client-{clientPlayer.LocalClientId}!");
}
}

}
}
}
}

0 comments on commit cb7ec98

Please sign in to comment.