fix: NetworkManager instantiate & destroy notifications and player spawn prefab offset (backport) (#3089)

NoelStephensUnity · web-flow · commit 8477fd432cbe · 2024-10-08T17:52:41.000-05:00
* update

Adding NetworkManager.Instantiated and NetworkManager.Destroying event notifications.

* test

Adding validation test.

* update

Adding changelog entries.

* fix

Fixing player instantiation position and rotation issue introduced in an earlier PR (but not yet released).

* test

test to validate the player spawn position fix.
adding the observers test from v2.0.0 (just good to have that in v1.x.x)

* style

remove whitespaces
diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md
@@ -10,6 +10,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Added
 
+- Added a static `NetworkManager.OnInstantiated` event notification to be able to track when a new `NetworkManager` instance has been instantiated. (#3089)
+- Added a static `NetworkManager.OnDestroying` event notification to be able to track when an existing `NetworkManager` instance is being destroyed. (#3089)
 - Added message size validation to named and unnamed message sending functions for better error messages. (#3043)
 - Added "Check for NetworkObject Component" property to the Multiplayer->Netcode for GameObjects project settings. When disabled, this will bypass the in-editor `NetworkObject` check on `NetworkBehaviour` components. (#3034)
 
diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs
@@ -738,9 +738,8 @@ internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.Conne
 
                 if (response.CreatePlayerObject && (response.PlayerPrefabHash.HasValue || NetworkManager.NetworkConfig.PlayerPrefab != null))
                 {
-                    var playerObject = response.PlayerPrefabHash.HasValue ? NetworkManager.SpawnManager.GetNetworkObjectToSpawn(response.PlayerPrefabHash.Value, ownerClientId, response.Position.GetValueOrDefault(), response.Rotation.GetValueOrDefault())
-                        : NetworkManager.SpawnManager.GetNetworkObjectToSpawn(NetworkManager.NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash, ownerClientId, response.Position.GetValueOrDefault(), response.Rotation.GetValueOrDefault());
-
+                    var playerObject = response.PlayerPrefabHash.HasValue ? NetworkManager.SpawnManager.GetNetworkObjectToSpawn(response.PlayerPrefabHash.Value, ownerClientId, response.Position ?? null, response.Rotation ?? null)
+                        : NetworkManager.SpawnManager.GetNetworkObjectToSpawn(NetworkManager.NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash, ownerClientId, response.Position ?? null, response.Rotation ?? null);
                     // Spawn the player NetworkObject locally
                     NetworkManager.SpawnManager.SpawnNetworkObjectLocally(
                         playerObject,
diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs
@@ -16,6 +16,16 @@ namespace Unity.Netcode
     [AddComponentMenu("Netcode/Network Manager", -100)]
     public class NetworkManager : MonoBehaviour, INetworkUpdateSystem
     {
+        /// <summary>
+        /// Subscribe to this static event to get notifications when a <see cref="NetworkManager"/> instance has been instantiated.
+        /// </summary>
+        public static event Action<NetworkManager> OnInstantiated;
+
+        /// <summary>
+        /// Subscribe to this static event to get notifications when a <see cref="NetworkManager"/> instance is being destroyed.
+        /// </summary>
+        public static event Action<NetworkManager> OnDestroying;
+
         // TODO: Deprecate...
         // The following internal values are not used, but because ILPP makes them public in the assembly, they cannot
         // be removed thanks to our semver validation.
@@ -715,6 +725,8 @@ private void Awake()
 #if UNITY_EDITOR
             EditorApplication.playModeStateChanged += ModeChanged;
 #endif
+            // Notify we have instantiated a new instance of NetworkManager.
+            OnInstantiated?.Invoke(this);
         }
 
         private void OnEnable()
@@ -1274,6 +1286,9 @@ private void OnDestroy()
 
             UnityEngine.SceneManagement.SceneManager.sceneUnloaded -= OnSceneUnloaded;
 
+            // Notify we are destroying NetworkManager
+            OnDestroying?.Invoke(this);
+
             if (Singleton == this)
             {
                 Singleton = null;
diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs
@@ -417,14 +417,14 @@ internal NetworkObject InstantiateAndSpawnNoParameterChecks(NetworkObject networ
         /// Gets the right NetworkObject prefab instance to spawn. If a handler is registered or there is an override assigned to the 
         /// passed in globalObjectIdHash value, then that is what will be instantiated, spawned, and returned.
         /// </summary>
-        internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ownerId, Vector3 position = default, Quaternion rotation = default, bool isScenePlaced = false)
+        internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ownerId, Vector3? position, Quaternion? rotation, bool isScenePlaced = false)
         {
             NetworkObject networkObject = null;
             // If the prefab hash has a registered INetworkPrefabInstanceHandler derived class
             if (NetworkManager.PrefabHandler.ContainsHandler(globalObjectIdHash))
             {
                 // Let the handler spawn the NetworkObject
-                networkObject = NetworkManager.PrefabHandler.HandleNetworkPrefabSpawn(globalObjectIdHash, ownerId, position, rotation);
+                networkObject = NetworkManager.PrefabHandler.HandleNetworkPrefabSpawn(globalObjectIdHash, ownerId, position ?? default, rotation ?? default);
                 networkObject.NetworkManagerOwner = NetworkManager;
             }
             else
@@ -476,6 +476,8 @@ internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ow
                 {
                     // Create prefab instance
                     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;
                 }
diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerEventsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerEventsTests.cs
@@ -13,6 +13,61 @@ public class NetworkManagerEventsTests
         private NetworkManager m_ClientManager;
         private NetworkManager m_ServerManager;
 
+        private NetworkManager m_NetworkManagerInstantiated;
+        private bool m_Instantiated;
+        private bool m_Destroyed;
+
+        /// <summary>
+        /// Validates the <see cref="NetworkManager.OnInstantiated"/> and <see cref="NetworkManager.OnDestroying"/> event notifications
+        /// </summary>
+        [UnityTest]
+        public IEnumerator InstantiatedAndDestroyingNotifications()
+        {
+            NetworkManager.OnInstantiated += NetworkManager_OnInstantiated;
+            NetworkManager.OnDestroying += NetworkManager_OnDestroying;
+            var waitPeriod = new WaitForSeconds(0.01f);
+            var prefab = new GameObject("InstantiateDestroy");
+            var networkManagerPrefab = prefab.AddComponent<NetworkManager>();
+
+            Assert.IsTrue(m_Instantiated, $"{nameof(NetworkManager)} prefab did not get instantiated event notification!");
+            Assert.IsTrue(m_NetworkManagerInstantiated == networkManagerPrefab, $"{nameof(NetworkManager)} prefab parameter did not match!");
+
+            m_Instantiated = false;
+            m_NetworkManagerInstantiated = null;
+
+            for (int i = 0; i < 3; i++)
+            {
+                var instance = Object.Instantiate(prefab);
+                var networkManager = instance.GetComponent<NetworkManager>();
+                Assert.IsTrue(m_Instantiated, $"{nameof(NetworkManager)} instance-{i} did not get instantiated event notification!");
+                Assert.IsTrue(m_NetworkManagerInstantiated == networkManager, $"{nameof(NetworkManager)} instance-{i} parameter did not match!");
+                Object.DestroyImmediate(instance);
+                Assert.IsTrue(m_Destroyed, $"{nameof(NetworkManager)} instance-{i} did not get destroying event notification!");
+                m_Instantiated = false;
+                m_NetworkManagerInstantiated = null;
+                m_Destroyed = false;
+            }
+            m_NetworkManagerInstantiated = networkManagerPrefab;
+            Object.Destroy(prefab);
+            yield return null;
+            Assert.IsTrue(m_Destroyed, $"{nameof(NetworkManager)} prefab did not get destroying event notification!");
+            NetworkManager.OnInstantiated -= NetworkManager_OnInstantiated;
+            NetworkManager.OnDestroying -= NetworkManager_OnDestroying;
+        }
+
+        private void NetworkManager_OnInstantiated(NetworkManager networkManager)
+        {
+            m_Instantiated = true;
+            m_NetworkManagerInstantiated = networkManager;
+        }
+
+        private void NetworkManager_OnDestroying(NetworkManager networkManager)
+        {
+            m_Destroyed = true;
+            Assert.True(m_NetworkManagerInstantiated == networkManager, $"Destroying {nameof(NetworkManager)} and current instance is not a match for the one passed into the event!");
+        }
+
+
         [UnityTest]
         public IEnumerator OnServerStoppedCalledWhenServerStops()
         {
@@ -228,6 +283,9 @@ in NetworkTimeSystem.Sync */
         [UnityTearDown]
         public virtual IEnumerator Teardown()
         {
+            NetworkManager.OnInstantiated -= NetworkManager_OnInstantiated;
+            NetworkManager.OnDestroying -= NetworkManager_OnDestroying;
+
             NetcodeIntegrationTestHelpers.Destroy();
             if (m_ServerManager != null)
             {
diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/PlayerObjectTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/PlayerObjectTests.cs
@@ -1,4 +1,5 @@
 using System.Collections;
+using System.Linq;
 using NUnit.Framework;
 using Unity.Netcode.TestHelpers.Runtime;
 using UnityEngine;
@@ -48,4 +49,112 @@ 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.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;
+
+            var playerObjects = m_ServerNetworkManager.SpawnManager.SpawnedObjectsList.Where((c) => c.IsPlayerObject).ToList();
+
+            // 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 playerObjects)
+                {
+                    Assert.IsFalse(client.SpawnManager.SpawnedObjects.ContainsKey(playerObject.NetworkObjectId), $"Client-{client.LocalClientId} spawned player object for Client-{playerObject.NetworkObjectId}!");
+                }
+            }
+        }
+    }
+
+    /// <summary>
+    /// This test validates the player position and rotation is correct
+    /// relative to the prefab's initial settings if no changes are applied.
+    /// </summary>
+    [TestFixture(HostOrServer.Host)]
+    [TestFixture(HostOrServer.Server)]
+    internal class PlayerSpawnPositionTests : IntegrationTestWithApproximation
+    {
+        protected override int NumberOfClients => 2;
+
+        public PlayerSpawnPositionTests(HostOrServer hostOrServer)
+        {
+            m_UseHost = hostOrServer == HostOrServer.Host;
+        }
+
+        private Vector3 m_PlayerPosition;
+        private Quaternion m_PlayerRotation;
+
+        protected override void OnCreatePlayerPrefab()
+        {
+            var playerNetworkObject = m_PlayerPrefab.GetComponent<NetworkObject>();
+            m_PlayerPosition = GetRandomVector3(-10.0f, 10.0f);
+            m_PlayerRotation = Quaternion.Euler(GetRandomVector3(-180.0f, 180.0f));
+            playerNetworkObject.transform.position = m_PlayerPosition;
+            playerNetworkObject.transform.rotation = m_PlayerRotation;
+            base.OnCreatePlayerPrefab();
+        }
+
+        private void PlayerTransformMatches(NetworkObject player)
+        {
+            var position = player.transform.position;
+            var rotation = player.transform.rotation;
+            Assert.True(Approximately(m_PlayerPosition, position), $"Client-{player.OwnerClientId} position {position} does not match the prefab position {m_PlayerPosition}!");
+            Assert.True(Approximately(m_PlayerRotation, rotation), $"Client-{player.OwnerClientId} rotation {rotation.eulerAngles} does not match the prefab rotation {m_PlayerRotation.eulerAngles}!");
+        }
+
+        [UnityTest]
+        public IEnumerator PlayerSpawnPosition()
+        {
+            if (m_ServerNetworkManager.IsHost)
+            {
+                PlayerTransformMatches(m_ServerNetworkManager.LocalClient.PlayerObject);
+
+                foreach (var client in m_ClientNetworkManagers)
+                {
+                    yield return WaitForConditionOrTimeOut(() => client.SpawnManager.SpawnedObjects.ContainsKey(m_ServerNetworkManager.LocalClient.PlayerObject.NetworkObjectId));
+                    AssertOnTimeout($"Client-{client.LocalClientId} does not contain a player prefab instance for client-{m_ServerNetworkManager.LocalClientId}!");
+                    PlayerTransformMatches(client.SpawnManager.SpawnedObjects[m_ServerNetworkManager.LocalClient.PlayerObject.NetworkObjectId]);
+                }
+            }
+
+            foreach (var client in m_ClientNetworkManagers)
+            {
+                yield return WaitForConditionOrTimeOut(() => m_ServerNetworkManager.SpawnManager.SpawnedObjects.ContainsKey(client.LocalClient.PlayerObject.NetworkObjectId));
+                AssertOnTimeout($"Client-{m_ServerNetworkManager.LocalClientId} does not contain a player prefab instance for client-{client.LocalClientId}!");
+                PlayerTransformMatches(m_ServerNetworkManager.SpawnManager.SpawnedObjects[client.LocalClient.PlayerObject.NetworkObjectId]);
+                foreach (var subClient in m_ClientNetworkManagers)
+                {
+                    yield return WaitForConditionOrTimeOut(() => subClient.SpawnManager.SpawnedObjects.ContainsKey(client.LocalClient.PlayerObject.NetworkObjectId));
+                    AssertOnTimeout($"Client-{subClient.LocalClientId} does not contain a player prefab instance for client-{client.LocalClientId}!");
+                    PlayerTransformMatches(subClient.SpawnManager.SpawnedObjects[client.LocalClient.PlayerObject.NetworkObjectId]);
+                }
+            }
+        }
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -417,14 +417,14 @@ internal NetworkObject InstantiateAndSpawnNoParameterChecks(NetworkObject networ`
`417`	`417`	`/// Gets the right NetworkObject prefab instance to spawn. If a handler is registered or there is an override assigned to the`
`418`	`418`	`/// passed in globalObjectIdHash value, then that is what will be instantiated, spawned, and returned.`
`419`	`419`	`/// </summary>`
`420`		`- internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ownerId, Vector3 position = default, Quaternion rotation = default, bool isScenePlaced = false)`
	`420`	`+ internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ownerId, Vector3? position, Quaternion? rotation, bool isScenePlaced = false)`
`421`	`421`	`{`
`422`	`422`	`NetworkObject networkObject = null;`
`423`	`423`	`// If the prefab hash has a registered INetworkPrefabInstanceHandler derived class`
`424`	`424`	`if (NetworkManager.PrefabHandler.ContainsHandler(globalObjectIdHash))`
`425`	`425`	`{`
`426`	`426`	`// Let the handler spawn the NetworkObject`
`427`		`- networkObject = NetworkManager.PrefabHandler.HandleNetworkPrefabSpawn(globalObjectIdHash, ownerId, position, rotation);`
	`427`	`+ networkObject = NetworkManager.PrefabHandler.HandleNetworkPrefabSpawn(globalObjectIdHash, ownerId, position ?? default, rotation ?? default);`
`428`	`428`	`networkObject.NetworkManagerOwner = NetworkManager;`
`429`	`429`	`}`
`430`	`430`	`else`
`@@ -476,6 +476,8 @@ internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ow`
`476`	`476`	`{`
`477`	`477`	`// Create prefab instance`
`478`	`478`	`networkObject = UnityEngine.Object.Instantiate(networkPrefabReference).GetComponent<NetworkObject>();`
	`479`	`+ networkObject.transform.position = position ?? networkObject.transform.position;`
	`480`	`+ networkObject.transform.rotation = rotation ?? networkObject.transform.rotation;`
`479`	`481`	`networkObject.NetworkManagerOwner = NetworkManager;`
`480`	`482`	`networkObject.PrefabGlobalObjectIdHash = globalObjectIdHash;`
`481`	`483`	`}`