fix: distributed authority spawning prefab overrides (#3160)

* fix

All clients in distributed authority should invoke CheckForGlobalObjectIdHashOverride when spawning a NetworkPrefab since each client has the authority to spawn (i.e. each should be treated like a host/server in regards to spawning and prefab overrides).

* update

adding changelog entry.

* update

adding changelog PR number.

* test

updating existing test to run through with distributed authority and client-server network topologies.
(still need one more integration test to validate the final spawned object on the non-authority sides)

* style

removing whitespace

* fix

Fixed issue with server only NetworkManager instance spawning the network prefab itself as opposed to creating an instance.
Extracted the instantiation portion of a network prefab into an internal method that can be used for integration testing prefab instantiation and potentially other edge case scenarios.

* test

Migrated all runtime network prefab related tests into a Prefabs subfolder.
Added new NetworkPrefabOverrideTests to validate the actual spawned instances on all clients using both client-server and distributed authority network topologies.
Added helper method `NettcodeIntegrationTestHelpers.CreateNetworkObject`.

* style

correcting some typos in comments

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -15,6 +15,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue where a sever only `NetworkManager` instance would spawn the actual `NetworkPrefab`'s `GameObject` as opposed to creating an instance of it. (#3160)
+- Fixed issue where only the session owner (as opposed to all clients) would handle spawning prefab overrides properly when using a distributed authority network topology. (#3160)
 - Fixed issue where an exception was thrown when calling `NetworkManager.Shutdown` after calling `UnityTransport.Shutdown`. (#3118)
 - Fixed issue where `NetworkList` properties on in-scene placed `NetworkObject`s could cause small memory leaks when entering playmode. (#3147)
 - Fixed in-scene `NertworkObject` synchronization issue when loading a scene with currently connected clients connected to a session created by a `NetworkManager` started as a server (i.e. not as a host). (#3133)

com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

@@ -3246,14 +3246,15 @@ internal void UpdateForSceneChanges()
         }
 
         /// <summary>
-        /// Only applies to Hosts or session owners (for now)
+        /// Client-Server: Only applies to spawn authority (i.e. Server)
+        /// Distributed Authority: Applies to all clients since they all have spawn authority.
         /// Will return the registered source NetworkPrefab's GlobalObjectIdHash if one exists.
         /// Server and Clients will always return the NetworkObject's GlobalObjectIdHash.
         /// </summary>
         /// <returns>appropriate hash value</returns>
         internal uint CheckForGlobalObjectIdHashOverride()
         {
-            if (NetworkManager.IsServer || (NetworkManager.DistributedAuthorityMode && NetworkManager.LocalClient.IsSessionOwner))
+            if (NetworkManager.IsServer || NetworkManager.DistributedAuthorityMode)
             {
                 if (NetworkManager.PrefabHandler.ContainsHandler(this))
                 {

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

@@ -736,12 +736,20 @@ public NetworkObject InstantiateAndSpawn(NetworkObject networkPrefab, ulong owne
         internal NetworkObject InstantiateAndSpawnNoParameterChecks(NetworkObject networkPrefab, ulong ownerClientId = NetworkManager.ServerClientId, bool destroyWithScene = false, bool isPlayerObject = false, bool forceOverride = false, Vector3 position = default, Quaternion rotation = default)
         {
             var networkObject = networkPrefab;
-            // Host spawns the ovveride and server spawns the original prefab unless forceOverride is set to true where both server or host will spawn the override.
-            // In distributed authority mode, we alaways get the override
-            if (forceOverride || NetworkManager.IsHost || NetworkManager.DistributedAuthorityMode)
+            // - Host and clients always instantiate the override if one exists.
+            // - Server instantiates the original prefab unless:
+            // -- forceOverride is set to true =or=
+            // -- The prefab has a registered prefab handler, then we let user code determine what to spawn.
+            // - Distributed authority mode always spawns the override if one exists.
+            if (forceOverride || NetworkManager.IsClient || NetworkManager.DistributedAuthorityMode || NetworkManager.PrefabHandler.ContainsHandler(networkPrefab.GlobalObjectIdHash))
             {
                 networkObject = GetNetworkObjectToSpawn(networkPrefab.GlobalObjectIdHash, ownerClientId, position, rotation);
             }
+            else // Under this case, server instantiate the prefab passed in.
+            {
+                networkObject = InstantiateNetworkPrefab(networkPrefab.gameObject, networkPrefab.GlobalObjectIdHash, position, rotation);
+            }
+
             if (networkObject == null)
             {
                 Debug.LogError($"Failed to instantiate and spawn {networkPrefab.name}!");
@@ -824,16 +832,37 @@ internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ow
                 else
                 {
                     // Create prefab instance while applying any pre-assigned position and rotation values
-                    networkObject = UnityEngine.Object.Instantiate(networkPrefabReference).GetComponent<NetworkObject>();
-                    networkObject.transform.position = position ?? networkObject.transform.position;
-                    networkObject.transform.rotation = rotation ?? networkObject.transform.rotation;
-                    networkObject.NetworkManagerOwner = NetworkManager;
-                    networkObject.PrefabGlobalObjectIdHash = globalObjectIdHash;
+                    networkObject = InstantiateNetworkPrefab(networkPrefabReference, globalObjectIdHash, position, rotation);
                 }
             }
             return networkObject;
         }
 
+        /// <summary>
+        /// Instantiates a network prefab instance, assigns the base prefab <see cref="NetworkObject.GlobalObjectIdHash"/>, positions, and orients
+        /// the instance.
+        /// !!! Should only be invoked by <see cref="GetNetworkObjectToSpawn"/> unless used by an integration test !!!
+        /// </summary>
+        /// <remarks>
+        /// <param name="prefabGlobalObjectIdHash"> should be the base prefab <see cref="NetworkObject.GlobalObjectIdHash"/> value and not the
+        /// overrided value.
+        /// (Can be used for integration testing)
+        /// </remarks>
+        /// <param name="networkPrefab">prefab to instantiate</param>
+        /// <param name="prefabGlobalObjectIdHash"><see cref="NetworkObject.GlobalObjectIdHash"/> of the base prefab instance</param>
+        /// <param name="position">conditional position in place of the network prefab's default position</param>
+        /// <param name="rotation">conditional rotation in place of the network prefab's default rotation</param>
+        /// <returns>the instance of the <see cref="NetworkObject"/></returns>
+        internal NetworkObject InstantiateNetworkPrefab(GameObject networkPrefab, uint prefabGlobalObjectIdHash, Vector3? position, Quaternion? rotation)
+        {
+            var networkObject = UnityEngine.Object.Instantiate(networkPrefab).GetComponent<NetworkObject>();
+            networkObject.transform.position = position ?? networkObject.transform.position;
+            networkObject.transform.rotation = rotation ?? networkObject.transform.rotation;
+            networkObject.NetworkManagerOwner = NetworkManager;
+            networkObject.PrefabGlobalObjectIdHash = prefabGlobalObjectIdHash;
+            return networkObject;
+        }
+
         /// <summary>
         /// Creates a local NetowrkObject to be spawned.
         /// </summary>

com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTestHelpers.cs

@@ -559,6 +559,29 @@ public static void MakeNetworkObjectTestPrefab(NetworkObject networkObject, uint
             }
         }
 
+        /// <summary>
+        /// Creates a <see cref="NetworkObject"/> to be used with integration testing
+        /// </summary>
+        /// <param name="baseName">namr of the object</param>
+        /// <param name="owner">owner of the object</param>
+        /// <param name="moveToDDOL">when true, the instance is automatically migrated into the DDOL</param>
+        /// <returns></returns>
+        internal static GameObject CreateNetworkObject(string baseName, NetworkManager owner, bool moveToDDOL = false)
+        {
+            var gameObject = new GameObject
+            {
+                name = baseName
+            };
+            var networkObject = gameObject.AddComponent<NetworkObject>();
+            networkObject.NetworkManagerOwner = owner;
+            MakeNetworkObjectTestPrefab(networkObject);
+            if (moveToDDOL)
+            {
+                Object.DontDestroyOnLoad(gameObject);
+            }
+            return gameObject;
+        }
+
         public static GameObject CreateNetworkObjectPrefab(string baseName, NetworkManager server, params NetworkManager[] clients)
         {
             void AddNetworkPrefab(NetworkConfig config, NetworkPrefab prefab)
@@ -570,13 +593,7 @@ void AddNetworkPrefab(NetworkConfig config, NetworkPrefab prefab)
             Assert.IsNotNull(server, prefabCreateAssertError);
             Assert.IsFalse(server.IsListening, prefabCreateAssertError);
 
-            var gameObject = new GameObject
-            {
-                name = baseName
-            };
-            var networkObject = gameObject.AddComponent<NetworkObject>();
-            networkObject.NetworkManagerOwner = server;
-            MakeNetworkObjectTestPrefab(networkObject);
+            var gameObject = CreateNetworkObject(baseName, server);
             var networkPrefab = new NetworkPrefab() { Prefab = gameObject };
 
             // We could refactor this test framework to share a NetworkPrefabList instance, but at this point it's

com.unity.netcode.gameobjects/Tests/Runtime/Prefabs.meta

@@ -85,9 +85,14 @@ public void NetworkConfigInvalidNetworkPrefabTest()
         private const string k_PrefabObjectName = "NetworkPrefabHandlerTestObject";
 
         [Test]
-        public void NetworkPrefabHandlerClass()
+        public void NetworkPrefabHandlerClass([Values] bool distributedAuthority)
         {
-            Assert.IsTrue(NetworkManagerHelper.StartNetworkManager(out _));
+            var networkConfig = new NetworkConfig()
+            {
+                NetworkTopology = distributedAuthority ? NetworkTopologyTypes.DistributedAuthority : NetworkTopologyTypes.ClientServer,
+            };
+
+            Assert.IsTrue(NetworkManagerHelper.StartNetworkManager(out _, networkConfig: networkConfig));
             var testPrefabObjectName = k_PrefabObjectName;
 
             Guid baseObjectID = NetworkManagerHelper.AddGameNetworkObject(testPrefabObjectName);