refactor: assign auto-incremented GlobalObjectIdHash as a fallback in MultiInstanceHelpers.MakeNetworkObjectTestPrefab() + fix flaky tests exposed by this fix (#1094)

* refactor: assign auto-incremented GlobalObjectIdHash as a fallback in MultiInstanceHelpers.MakeNetworkObjectTestPrefab()

* fix MultiClientConnectionApproval.ConnectionApproval() test

* refactor

Removing DefaultPayerGlobalObjectIdHashValue with new GlobalObjectIdHash goodness.

* fix NetworkObjectMetricsTests

* remove `DefaultPayerGlobalObjectIdHashValue` from OwnershipChangeMetricsTests

Author: 0xFA11

com.unity.netcode.gameobjects/Tests/Runtime/BaseMultiInstanceTest.cs

@@ -14,8 +14,6 @@ public abstract class BaseMultiInstanceTest
         protected NetworkManager m_ServerNetworkManager;
         protected NetworkManager[] m_ClientNetworkManagers;
 
-        internal static uint DefaultPayerGlobalObjectIdHashValue = 7777777;
-
         protected abstract int NbClients { get; }
 
         [UnitySetUp]
@@ -96,7 +94,7 @@ public IEnumerator StartSomeClientsAndServerWithPlayers(bool useHost, int nbClie
              * at runtime without it being treated as a SceneObject or causing other conflicts with the Netcode.
              */
             // Make it a prefab
-            MultiInstanceHelpers.MakeNetworkedObjectTestPrefab(networkObject, DefaultPayerGlobalObjectIdHashValue);
+            MultiInstanceHelpers.MakeNetworkObjectTestPrefab(networkObject);
 
             updatePlayerPrefab(m_PlayerPrefab); // update player prefab with whatever is needed before players are spawned
 

com.unity.netcode.gameobjects/Tests/Runtime/Metrics/NetworkObjectMetricsTests.cs

@@ -13,31 +13,40 @@ namespace Unity.Netcode.RuntimeTests.Metrics
 {
     public class NetworkObjectMetricsTests : SingleClientMetricTestBase
     {
-        const string NewNetworkObjectName = "TestNetworkObjectToSpawn";
-
-        NetworkObject m_NewNetworkObject;
+        private const string k_NewNetworkObjectName = "TestNetworkObjectToSpawn";
+        private NetworkObject m_NewNetworkPrefab;
 
         protected override Action<GameObject> UpdatePlayerPrefab => _ =>
         {
-            var gameObject = new GameObject(NewNetworkObjectName);
-            m_NewNetworkObject = gameObject.AddComponent<NetworkObject>();
-            m_NewNetworkObject.NetworkManagerOwner = m_ServerNetworkManager;
-            MultiInstanceHelpers.MakeNetworkedObjectTestPrefab(m_NewNetworkObject);
+            var gameObject = new GameObject(k_NewNetworkObjectName);
+            m_NewNetworkPrefab = gameObject.AddComponent<NetworkObject>();
+            MultiInstanceHelpers.MakeNetworkObjectTestPrefab(m_NewNetworkPrefab);
+
             var networkPrefab = new NetworkPrefab { Prefab = gameObject };
             m_ServerNetworkManager.NetworkConfig.NetworkPrefabs.Add(networkPrefab);
-
             foreach (var client in m_ClientNetworkManagers)
             {
                 client.NetworkConfig.NetworkPrefabs.Add(networkPrefab);
             }
         };
 
+        private NetworkObject SpawnNetworkObject()
+        {
+            // Spawn another network object so we can hide multiple.
+            var gameObject = UnityEngine.Object.Instantiate(m_NewNetworkPrefab); // new GameObject(NewNetworkObjectName);
+            var networkObject = gameObject.GetComponent<NetworkObject>();
+            networkObject.NetworkManagerOwner = Server;
+            networkObject.Spawn();
+
+            return networkObject;
+        }
+
         [UnityTest]
         public IEnumerator TrackNetworkObjectSpawnSentMetric()
         {
             var waitForMetricEvent = new WaitForMetricValues<ObjectSpawnedEvent>(ServerMetrics.Dispatcher, MetricNames.ObjectSpawnedSent);
 
-            m_NewNetworkObject.Spawn();
+            SpawnNetworkObject();
 
             yield return waitForMetricEvent.WaitForMetricsReceived();
 
@@ -46,7 +55,7 @@ public IEnumerator TrackNetworkObjectSpawnSentMetric()
 
             var objectSpawned = objectSpawnedSentMetricValues.First();
             Assert.AreEqual(Client.LocalClientId, objectSpawned.Connection.Id);
-            Assert.AreEqual(NewNetworkObjectName, objectSpawned.NetworkId.Name);
+            Assert.AreEqual($"{k_NewNetworkObjectName}(Clone)", objectSpawned.NetworkId.Name);
             Assert.AreNotEqual(0, objectSpawned.BytesCount);
         }
 
@@ -55,7 +64,7 @@ public IEnumerator TrackNetworkObjectSpawnReceivedMetric()
         {
             var waitForMetricEvent = new WaitForMetricValues<ObjectSpawnedEvent>(ClientMetrics.Dispatcher, MetricNames.ObjectSpawnedReceived);
 
-            m_NewNetworkObject.Spawn();
+            var networkObject = SpawnNetworkObject();
 
             yield return waitForMetricEvent.WaitForMetricsReceived();
 
@@ -64,21 +73,21 @@ public IEnumerator TrackNetworkObjectSpawnReceivedMetric()
 
             var objectSpawned = objectSpawnedReceivedMetricValues.First();
             Assert.AreEqual(Server.LocalClientId, objectSpawned.Connection.Id);
-            Assert.AreEqual(m_NewNetworkObject.NetworkObjectId, objectSpawned.NetworkId.NetworkId);
-            Assert.AreEqual($"{NewNetworkObjectName}(Clone)", objectSpawned.NetworkId.Name);
+            Assert.AreEqual(networkObject.NetworkObjectId, objectSpawned.NetworkId.NetworkId);
+            Assert.AreEqual($"{k_NewNetworkObjectName}(Clone)", objectSpawned.NetworkId.Name);
             Assert.AreNotEqual(0, objectSpawned.BytesCount);
         }
 
         [UnityTest]
         public IEnumerator TrackNetworkObjectDestroySentMetric()
         {
-            m_NewNetworkObject.Spawn();
+            var networkObject = SpawnNetworkObject();
 
             yield return new WaitForSeconds(0.2f);
 
             var waitForMetricEvent = new WaitForMetricValues<ObjectDestroyedEvent>(ServerMetrics.Dispatcher, MetricNames.ObjectDestroyedSent);
 
-            Server.SpawnManager.OnDespawnObject(m_NewNetworkObject, true);
+            Server.SpawnManager.OnDespawnObject(networkObject, true);
 
             yield return waitForMetricEvent.WaitForMetricsReceived();
 
@@ -87,20 +96,20 @@ public IEnumerator TrackNetworkObjectDestroySentMetric()
 
             var objectDestroyed = objectDestroyedSentMetricValues.Last();
             Assert.AreEqual(Client.LocalClientId, objectDestroyed.Connection.Id);
-            Assert.AreEqual(NewNetworkObjectName, objectDestroyed.NetworkId.Name);
+            Assert.AreEqual($"{k_NewNetworkObjectName}(Clone)", objectDestroyed.NetworkId.Name);
             Assert.AreNotEqual(0, objectDestroyed.BytesCount);
         }
 
         [UnityTest]
         public IEnumerator TrackNetworkObjectDestroyReceivedMetric()
         {
-            m_NewNetworkObject.Spawn();
+            var networkObject = SpawnNetworkObject();
 
             yield return new WaitForSeconds(0.2f);
 
             var waitForMetricEvent = new WaitForMetricValues<ObjectDestroyedEvent>(ClientMetrics.Dispatcher, MetricNames.ObjectDestroyedReceived);
 
-            Server.SpawnManager.OnDespawnObject(m_NewNetworkObject, true);
+            Server.SpawnManager.OnDespawnObject(networkObject, true);
 
             yield return waitForMetricEvent.WaitForMetricsReceived();
 
@@ -109,31 +118,26 @@ public IEnumerator TrackNetworkObjectDestroyReceivedMetric()
 
             var objectDestroyed = objectDestroyedReceivedMetricValues.First();
             Assert.AreEqual(Server.LocalClientId, objectDestroyed.Connection.Id);
-            Assert.AreEqual(m_NewNetworkObject.NetworkObjectId, objectDestroyed.NetworkId.NetworkId);
-            Assert.AreEqual($"{NewNetworkObjectName}(Clone)", objectDestroyed.NetworkId.Name);
+            Assert.AreEqual(networkObject.NetworkObjectId, objectDestroyed.NetworkId.NetworkId);
+            Assert.AreEqual($"{k_NewNetworkObjectName}(Clone)", objectDestroyed.NetworkId.Name);
             Assert.AreNotEqual(0, objectDestroyed.BytesCount);
         }
 
         [UnityTest]
         public IEnumerator TrackMultipleNetworkObjectSpawnSentMetric()
         {
-            m_NewNetworkObject.Spawn();
-
-            // Spawn another network object so we can hide multiple.
-            var gameObject = new GameObject(NewNetworkObjectName);
-            var anotherNetworkObject = gameObject.AddComponent<NetworkObject>();
-            anotherNetworkObject.NetworkManagerOwner = Server;
-            anotherNetworkObject.Spawn();
+            var networkObject1 = SpawnNetworkObject();
+            var networkObject2 = SpawnNetworkObject();
 
             yield return new WaitForSeconds(0.2f);
 
-            NetworkObject.NetworkHide(new List<NetworkObject>{m_NewNetworkObject, anotherNetworkObject}, Client.LocalClientId);
+            NetworkObject.NetworkHide(new List<NetworkObject> { networkObject1, networkObject2 }, Client.LocalClientId);
 
             yield return new WaitForSeconds(0.2f);
 
             var waitForMetricEvent = new WaitForMetricValues<ObjectSpawnedEvent>(ServerMetrics.Dispatcher, MetricNames.ObjectSpawnedSent);
 
-            NetworkObject.NetworkShow(new List<NetworkObject>{m_NewNetworkObject, anotherNetworkObject}, Client.LocalClientId);
+            NetworkObject.NetworkShow(new List<NetworkObject> { networkObject1, networkObject2 }, Client.LocalClientId);
 
             yield return waitForMetricEvent.WaitForMetricsReceived();
 
@@ -143,14 +147,14 @@ public IEnumerator TrackMultipleNetworkObjectSpawnSentMetric()
                 objectSpawnedSentMetricValues,
                 Has.Exactly(1).Matches<ObjectSpawnedEvent>(
                     x => Client.LocalClientId == x.Connection.Id
-                         && x.NetworkId.NetworkId == m_NewNetworkObject.NetworkObjectId
-                         && x.NetworkId.Name == m_NewNetworkObject.name));
+                         && x.NetworkId.NetworkId == networkObject1.NetworkObjectId
+                         && x.NetworkId.Name == networkObject1.name));
             Assert.That(
                 objectSpawnedSentMetricValues,
                 Has.Exactly(1).Matches<ObjectSpawnedEvent>(
                     x => Client.LocalClientId == x.Connection.Id
-                         && x.NetworkId.NetworkId == anotherNetworkObject.NetworkObjectId
-                         && x.NetworkId.Name == anotherNetworkObject.name));
+                         && x.NetworkId.NetworkId == networkObject2.NetworkObjectId
+                         && x.NetworkId.Name == networkObject2.name));
 
             Assert.AreEqual(1, objectSpawnedSentMetricValues.Select(x => x.BytesCount).Distinct().Count());
             Assert.That(objectSpawnedSentMetricValues.Select(x => x.BytesCount), Has.All.Not.EqualTo(0));
@@ -159,19 +163,14 @@ public IEnumerator TrackMultipleNetworkObjectSpawnSentMetric()
         [UnityTest]
         public IEnumerator TrackMultipleNetworkObjectDestroySentMetric()
         {
-            m_NewNetworkObject.Spawn();
-
-            // Spawn another network object so we can hide multiple.
-            var gameObject = new GameObject(NewNetworkObjectName);
-            var anotherNetworkObject = gameObject.AddComponent<NetworkObject>();
-            anotherNetworkObject.NetworkManagerOwner = Server;
-            anotherNetworkObject.Spawn();
+            var networkObject1 = SpawnNetworkObject();
+            var networkObject2 = SpawnNetworkObject();
 
             yield return new WaitForSeconds(0.2f);
 
             var waitForMetricEvent = new WaitForMetricValues<ObjectDestroyedEvent>(ServerMetrics.Dispatcher, MetricNames.ObjectDestroyedSent);
 
-            NetworkObject.NetworkHide(new List<NetworkObject>{m_NewNetworkObject, anotherNetworkObject}, Client.LocalClientId);
+            NetworkObject.NetworkHide(new List<NetworkObject> { networkObject1, networkObject2 }, Client.LocalClientId);
 
             yield return waitForMetricEvent.WaitForMetricsReceived();
 
@@ -181,14 +180,14 @@ public IEnumerator TrackMultipleNetworkObjectDestroySentMetric()
                 objectDestroyedSentMetricValues,
                 Has.Exactly(1).Matches<ObjectDestroyedEvent>(
                     x => Client.LocalClientId == x.Connection.Id
-                         && x.NetworkId.NetworkId == m_NewNetworkObject.NetworkObjectId
-                         && x.NetworkId.Name == m_NewNetworkObject.name));
+                         && x.NetworkId.NetworkId == networkObject1.NetworkObjectId
+                         && x.NetworkId.Name == networkObject1.name));
             Assert.That(
                 objectDestroyedSentMetricValues,
                 Has.Exactly(1).Matches<ObjectDestroyedEvent>(
                     x => Client.LocalClientId == x.Connection.Id
-                         && x.NetworkId.NetworkId == anotherNetworkObject.NetworkObjectId
-                         && x.NetworkId.Name == anotherNetworkObject.name));
+                         && x.NetworkId.NetworkId == networkObject2.NetworkObjectId
+                         && x.NetworkId.Name == networkObject2.name));
 
             Assert.AreEqual(1, objectDestroyedSentMetricValues.Select(x => x.BytesCount).Distinct().Count());
             Assert.That(objectDestroyedSentMetricValues.Select(x => x.BytesCount), Has.All.Not.EqualTo(0));

com.unity.netcode.gameobjects/Tests/Runtime/Metrics/OwnershipChangeMetricsTests.cs

@@ -12,15 +12,39 @@ namespace Unity.Netcode.RuntimeTests.Metrics
 {
     public class OwnershipChangeMetricsTests : SingleClientMetricTestBase
     {
-        [UnityTest]
-        public IEnumerator TrackOwnershipChangeSentMetric()
+        private const string k_NewNetworkObjectName = "TestNetworkObjectToSpawn";
+        private NetworkObject m_NewNetworkPrefab;
+
+        protected override Action<GameObject> UpdatePlayerPrefab => _ =>
         {
-            var gameObject = new GameObject(Guid.NewGuid().ToString());
-            var networkObject = gameObject.AddComponent<NetworkObject>();
-            MultiInstanceHelpers.MakeNetworkedObjectTestPrefab(networkObject, DefaultPayerGlobalObjectIdHashValue);
+            var gameObject = new GameObject(k_NewNetworkObjectName);
+            m_NewNetworkPrefab = gameObject.AddComponent<NetworkObject>();
+            MultiInstanceHelpers.MakeNetworkObjectTestPrefab(m_NewNetworkPrefab);
+
+            var networkPrefab = new NetworkPrefab { Prefab = gameObject };
+            m_ServerNetworkManager.NetworkConfig.NetworkPrefabs.Add(networkPrefab);
+            foreach (var client in m_ClientNetworkManagers)
+            {
+                client.NetworkConfig.NetworkPrefabs.Add(networkPrefab);
+            }
+        };
+
+        private NetworkObject SpawnNetworkObject()
+        {
+            // Spawn another network object so we can hide multiple.
+            var gameObject = UnityEngine.Object.Instantiate(m_NewNetworkPrefab); // new GameObject(NewNetworkObjectName);
+            var networkObject = gameObject.GetComponent<NetworkObject>();
             networkObject.NetworkManagerOwner = Server;
             networkObject.Spawn();
 
+            return networkObject;
+        }
+
+        [UnityTest]
+        public IEnumerator TrackOwnershipChangeSentMetric()
+        {
+            var networkObject = SpawnNetworkObject();
+
             yield return new WaitForSeconds(0.2f);
 
             var waitForMetricValues = new WaitForMetricValues<OwnershipChangeEvent>(ServerMetrics.Dispatcher, MetricNames.OwnershipChangeSent);
@@ -40,11 +64,7 @@ public IEnumerator TrackOwnershipChangeSentMetric()
         [UnityTest]
         public IEnumerator TrackOwnershipChangeReceivedMetric()
         {
-            var gameObject = new GameObject(Guid.NewGuid().ToString());
-            var networkObject = gameObject.AddComponent<NetworkObject>();
-            MultiInstanceHelpers.MakeNetworkedObjectTestPrefab(networkObject, DefaultPayerGlobalObjectIdHashValue);
-            networkObject.NetworkManagerOwner = Server;
-            networkObject.Spawn();
+            var networkObject = SpawnNetworkObject();
 
             yield return new WaitForSeconds(0.2f);
 

com.unity.netcode.gameobjects/Tests/Runtime/MultiInstanceHelpers.cs

@@ -204,6 +204,7 @@ public class CoroutineResultWrapper<T>
             public T Result;
         }
 
+        private static uint s_AutoIncrementGlobalObjectIdHashCounter = 111111;
 
         /// <summary>
         /// Normally we would only allow player prefabs to be set to a prefab. Not runtime created objects.
@@ -214,7 +215,7 @@ public class CoroutineResultWrapper<T>
         /// </summary>
         /// <param name="networkObject">The networkObject to be treated as Prefab</param>
         /// <param name="globalObjectIdHash">The GlobalObjectId to force</param>
-        public static void MakeNetworkedObjectTestPrefab(NetworkObject networkObject, uint globalObjectIdHash = default)
+        public static void MakeNetworkObjectTestPrefab(NetworkObject networkObject, uint globalObjectIdHash = default)
         {
             // Set a globalObjectId for prefab
             if (globalObjectIdHash != default)
@@ -225,6 +226,12 @@ public static void MakeNetworkedObjectTestPrefab(NetworkObject networkObject, ui
             // Force generation
             networkObject.GenerateGlobalObjectIdHash();
 
+            // Fallback to auto-increment if generation fails
+            if (networkObject.GlobalObjectIdHash == default)
+            {
+                networkObject.GlobalObjectIdHash = ++s_AutoIncrementGlobalObjectIdHashCounter;
+            }
+
             // Prevent object from being snapped up as a scene object
             networkObject.IsSceneObject = false;
         }

com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourUpdaterTests.cs

@@ -65,7 +65,7 @@ void AddNetworkBehaviour(Type type, GameObject prefab)
             var networkObjectPrefab = prefabToSpawn.AddComponent<NetworkObject>();
             AddNetworkBehaviour(firstNetworkBehaviour, prefabToSpawn);
             AddNetworkBehaviour(secondNetworkBehaviour, prefabToSpawn);
-            MultiInstanceHelpers.MakeNetworkedObjectTestPrefab(networkObjectPrefab);
+            MultiInstanceHelpers.MakeNetworkObjectTestPrefab(networkObjectPrefab);
             m_ServerNetworkManager.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab() { Prefab = prefabToSpawn });
             foreach (var clientNetworkManager in m_ClientNetworkManagers)
             {

com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs

@@ -61,7 +61,7 @@ public IEnumerator Setup()
             m_DummyPrefab = new GameObject("DummyPrefabPrototype");
             m_DummyPrefab.AddComponent<NetworkObject>();
             m_DummyPrefab.AddComponent<NetworkObjectOwnershipComponent>();
-            MultiInstanceHelpers.MakeNetworkedObjectTestPrefab(m_DummyPrefab.GetComponent<NetworkObject>());
+            MultiInstanceHelpers.MakeNetworkObjectTestPrefab(m_DummyPrefab.GetComponent<NetworkObject>());
             m_ServerNetworkManager.NetworkConfig.NetworkPrefabs.Add(new NetworkPrefab { Prefab = m_DummyPrefab });
             foreach (var clientNetworkManager in m_ClientNetworkManagers)
             {

com.unity.netcode.gameobjects/Tests/Runtime/NetworkPrefabHandlerTests.cs

@@ -22,7 +22,7 @@ private GameObject MakeValidNetworkPrefab()
         {
             Guid baseObjectID = NetworkManagerHelper.AddGameNetworkObject(k_TestPrefabObjectName + m_GlobalObjectIdHashBase.ToString());
             NetworkObject validPrefab = NetworkManagerHelper.InstantiatedNetworkObjects[baseObjectID];
-            MultiInstanceHelpers.MakeNetworkedObjectTestPrefab(validPrefab, m_GlobalObjectIdHashBase);
+            MultiInstanceHelpers.MakeNetworkObjectTestPrefab(validPrefab, m_GlobalObjectIdHashBase);
             m_GlobalObjectIdHashBase++;
             return validPrefab.gameObject;
         }