diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 9edd4232e3..063b6c3375 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -22,6 +22,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed issue where if the `NetworkManager` player prefab is not assigned an exception is thrown upon starting a host and/or when a client joins. (#3965) ### Security diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index bcdb8ba05a..aaeb67f4db 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -946,6 +946,37 @@ internal void ProcessClientsToDisconnect() m_ClientsToDisconnect.Clear(); } + /// + /// The checks to find the right GlobalObjectIdHash value + /// are complex enough to deserve a method that includes + /// an easy to follow logical flow. + /// This also makes it a quick check to determine if there + /// even is a player prefab to spawn (it is valid to not + /// have any player spawned upon connection). + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private (bool IsValid, uint GlobalObjectIdHash) GetPlayerPrefabHash(uint? playerPrefabHash) + { + if (playerPrefabHash != null && playerPrefabHash.HasValue) + { + return (true, playerPrefabHash.Value); + } + else + if (NetworkManager.NetworkConfig.PlayerPrefab != null) + { + var networkObject = NetworkManager.NetworkConfig.PlayerPrefab.GetComponent(); + if (networkObject != null) + { + return (true, networkObject.GlobalObjectIdHash); + } + else + { + NetworkManager.Log.Error(new Logging.Context(LogLevel.Error, $"Player prefab {NetworkManager.NetworkConfig.PlayerPrefab.name} has no {nameof(NetworkObject)}!")); + } + } + return (false, 0); + } + /// /// Server Side: Handles the approval of a client /// @@ -972,10 +1003,10 @@ internal void HandleConnectionApproval(ulong ownerClientId, bool createPlayerObj } // Server-side spawning (only if there is a prefab hash or player prefab provided) - var idHashToSpawn = playerPrefabHash ?? NetworkManager.NetworkConfig.PlayerPrefab?.GetComponent()?.GlobalObjectIdHash; - if (!NetworkManager.DistributedAuthorityMode && createPlayerObject && idHashToSpawn.HasValue) + var idHashToSpawn = GetPlayerPrefabHash(playerPrefabHash); + if (!NetworkManager.DistributedAuthorityMode && createPlayerObject && idHashToSpawn.IsValid) { - var playerObject = NetworkManager.SpawnManager.GetNetworkObjectToSpawn(idHashToSpawn.Value, ownerClientId, playerPosition, playerRotation); + var playerObject = NetworkManager.SpawnManager.GetNetworkObjectToSpawn(idHashToSpawn.GlobalObjectIdHash, ownerClientId, playerPosition, playerRotation); if (playerObject == null) { diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerPlayerPrefab.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerPlayerPrefab.cs new file mode 100644 index 0000000000..2eeb1dac6f --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerPlayerPrefab.cs @@ -0,0 +1,65 @@ +using System.Collections; +using NUnit.Framework; +using Unity.Netcode.TestHelpers.Runtime; +using UnityEngine.TestTools; + +namespace Unity.Netcode.RuntimeTests +{ + /// + /// Generic test to validate clients can connect + /// without having set a player prefab. + /// + [TestFixture(HostOrServer.Server)] + [TestFixture(HostOrServer.Host)] + [TestFixture(HostOrServer.DAHost)] + internal class NetworkManagerPlayerPrefab : NetcodeIntegrationTest + { + protected override int NumberOfClients => 1; + + public NetworkManagerPlayerPrefab(HostOrServer hostOrServer) : base(hostOrServer) + { + } + + /// + /// Assure no player prefab is assigned. + /// + protected override void OnServerAndClientsCreated() + { + foreach (var networkManager in m_NetworkManagers) + { + networkManager.NetworkConfig.PlayerPrefab = null; + } + base.OnServerAndClientsCreated(); + } + + protected override void OnNewClientCreated(NetworkManager networkManager) + { + networkManager.NetworkConfig.PlayerPrefab = null; + base.OnNewClientCreated(networkManager); + } + + /// + /// Do not wait for spawned players as there are none. + /// + /// + protected override bool ShouldCheckForSpawnedPlayers() + { + return false; + } + + /// + /// Validates NetworkManager can start as a host and/or clients + /// can join when there is no player prefab assigned to the + /// NetworkManager. + /// + [UnityTest] + public IEnumerator VerifyNetworkManagerHandlesNoPlayerPrefab() + { + // If we make it to here, then the 1st client and the authority + // connected with no exceptions. + // Now just late join a 2nd client. + yield return CreateAndStartNewClient(); + // If it makes it to here without an exception then the test passes. + } + } +} diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerPlayerPrefab.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerPlayerPrefab.cs.meta new file mode 100644 index 0000000000..4957638cbf --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerPlayerPrefab.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: b3132959e2f4cbe42b605770cb364a4e \ No newline at end of file diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs index fe22bdeb52..a6f4a83275 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs @@ -1165,6 +1165,12 @@ protected virtual void OnTimeTravelServerAndClientsConnected() private void ClientNetworkManagerPostStart(NetworkManager networkManager) { networkManager.name = $"NetworkManager - Client - {networkManager.LocalClientId}"; + + // Always make sure we have a player to check. + if (!ShouldCheckForSpawnedPlayers()) + { + return; + } Assert.NotNull(networkManager.LocalClient.PlayerObject, $"{nameof(StartServerAndClients)} detected that client {networkManager.LocalClientId} does not have an assigned player NetworkObject!"); // Go ahead and create an entry for this new client