fix: Exception thrown when player prefab is null (#3965)

* fix

Fixes the issue where if the player prefab is null it will throw an exception.

* update

Adding changelog entry.

* test

Adding test to validate this PR.

Author: NoelStephensUnity

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
 

com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs

@@ -946,6 +946,37 @@ internal void ProcessClientsToDisconnect()
             m_ClientsToDisconnect.Clear();
         }
 
+        /// <summary>
+        /// 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).
+        /// </summary>
+        [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<NetworkObject>();
+                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);
+        }
+
         /// <summary>
         /// Server Side: Handles the approval of a client
         /// </summary>
@@ -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<NetworkObject>()?.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)
                 {

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
+{
+    /// <summary>
+    /// Generic <see cref="NetworkManager"/> test to validate clients can connect
+    /// without having set a player prefab.
+    /// </summary>
+    [TestFixture(HostOrServer.Server)]
+    [TestFixture(HostOrServer.Host)]
+    [TestFixture(HostOrServer.DAHost)]
+    internal class NetworkManagerPlayerPrefab : NetcodeIntegrationTest
+    {
+        protected override int NumberOfClients => 1;
+
+        public NetworkManagerPlayerPrefab(HostOrServer hostOrServer) : base(hostOrServer)
+        {
+        }
+
+        /// <summary>
+        /// Assure no player prefab is assigned.
+        /// </summary>
+        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);
+        }
+
+        /// <summary>
+        /// Do not wait for spawned players as there are none.
+        /// </summary>
+        /// <returns></returns>
+        protected override bool ShouldCheckForSpawnedPlayers()
+        {
+            return false;
+        }
+
+        /// <summary>
+        /// Validates NetworkManager can start as a host and/or clients
+        /// can join when there is no player prefab assigned to the
+        /// NetworkManager.
+        /// </summary>
+        [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.
+        }
+    }
+}

com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerPlayerPrefab.cs.meta

@@ -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