From cb7ec98cccc462f8d131ba3995e17f84fcd59bc2 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Fri, 27 Sep 2024 09:23:10 -0500 Subject: [PATCH] fix: player prefab spawning not honoring when spawnwithobservers is disabled (#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 --- com.unity.netcode.gameobjects/CHANGELOG.md | 1 + .../Runtime/Core/NetworkObject.cs | 18 +++-- .../Runtime/Spawning/NetworkSpawnManager.cs | 15 +++- .../Runtime/NetcodeIntegrationTest.cs | 7 +- .../Tests/Runtime/PlayerObjectTests.cs | 70 ++++++++++++++++++- 5 files changed, 103 insertions(+), 8 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 8817dfbe9c..1089c4012f 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -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 diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index f89bc1d345..5cca18a66a 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -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 { @@ -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); + } } } } diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index ce359c180a..06b5b6c364 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -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)) { diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs index 159d923d69..a84a7ccb7c 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs @@ -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) { diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/PlayerObjectTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/PlayerObjectTests.cs index 8fba758de9..dc75f41337 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/PlayerObjectTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/PlayerObjectTests.cs @@ -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; @@ -52,4 +52,72 @@ public IEnumerator SpawnAndReplaceExistingPlayerObject() Assert.False(s_GlobalTimeoutHelper.TimedOut, "Timed out waiting for client-side player object to change!"); } } + + /// + /// Validate that when auto-player spawning but SpawnWithObservers is disabled, + /// the player instantiated is only spawned on the authority side. + /// + [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(); + 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}!"); + } + } + + } + } + } }