fix: error when serializing ConnectionApprovalMessage with scene management disabled when one or more objects is hidden via the CheckObjectVisibility delegate

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -31,6 +31,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Fixed FastBufferReader being created with a length of 1 if provided an input of length 0. (#1480)
 - Fixed an exception being thrown during NetworkVariableDeltaMessage serialization when EnsureNetworkVariableLengthSafety is enabled (#1487)
 - Fixed: NetworkVariables containing more than 1300 bytes of data (such as large NetworkLists) no longer cause an OverflowException (the limit on data size is now whatever limit the chosen transport imposes on fragmented NetworkDelivery mechanisms) (#1481)
+
+- Fixed: Fixed error when serializing ConnectionApprovalMessage with scene management disabled when one or more objects is hidden via the CheckObjectVisibility delegate (#1509)
 - Fixed KeyNotFound exception when removing ownership of a newly spawned NetworkObject that is already owned by the server. (#1500)
 
 ### Changed

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

@@ -1655,7 +1655,6 @@ internal void HandleApproval(ulong ownerClientId, bool createPlayerObject, uint?
                     {
                         if (SpawnManager.SpawnedObjectsList.Count != 0)
                         {
-                            message.SceneObjectCount = SpawnManager.SpawnedObjectsList.Count;
                             message.SpawnedObjectsList = SpawnManager.SpawnedObjectsList;
                         }
                     }

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs

@@ -7,7 +7,6 @@ internal struct ConnectionApprovedMessage : INetworkMessage
     {
         public ulong OwnerClientId;
         public int NetworkTick;
-        public int SceneObjectCount;
 
         // Not serialized, held as references to serialize NetworkVariable data
         public HashSet<NetworkObject> SpawnedObjectsList;
@@ -23,10 +22,13 @@ public void Serialize(FastBufferWriter writer)
             }
             writer.WriteValue(OwnerClientId);
             writer.WriteValue(NetworkTick);
-            writer.WriteValue(SceneObjectCount);
 
-            if (SceneObjectCount != 0)
+            uint sceneObjectCount = 0;
+            if (SpawnedObjectsList != null)
             {
+                var pos = writer.Position;
+                writer.Seek(writer.Position + FastBufferWriter.GetWriteSize(sceneObjectCount));
+
                 // Serialize NetworkVariable data
                 foreach (var sobj in SpawnedObjectsList)
                 {
@@ -35,8 +37,16 @@ public void Serialize(FastBufferWriter writer)
                         sobj.Observers.Add(OwnerClientId);
                         var sceneObject = sobj.GetMessageSceneObject(OwnerClientId);
                         sceneObject.Serialize(writer);
+                        ++sceneObjectCount;
                     }
                 }
+                writer.Seek(pos);
+                writer.WriteValue(sceneObjectCount);
+                writer.Seek(writer.Length);
+            }
+            else
+            {
+                writer.WriteValue(sceneObjectCount);
             }
         }
 
@@ -56,7 +66,6 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context)
 
             reader.ReadValue(out OwnerClientId);
             reader.ReadValue(out NetworkTick);
-            reader.ReadValue(out SceneObjectCount);
             m_ReceivedSceneObjectData = reader;
             return true;
         }
@@ -77,10 +86,11 @@ public void Handle(ref NetworkContext context)
             if (!networkManager.NetworkConfig.EnableSceneManagement)
             {
                 networkManager.SpawnManager.DestroySceneObjects();
+                m_ReceivedSceneObjectData.ReadValue(out uint sceneObjectCount);
 
                 // Deserializing NetworkVariable data is deferred from Receive() to Handle to avoid needing
                 // to create a list to hold the data. This is a breach of convention for performance reasons.
-                for (ushort i = 0; i < SceneObjectCount; i++)
+                for (ushort i = 0; i < sceneObjectCount; i++)
                 {
                     var sceneObject = new NetworkObject.SceneObject();
                     sceneObject.Deserialize(m_ReceivedSceneObjectData);

com.unity.netcode.gameobjects/Tests/Runtime/Components/NetworkVisibilityComponent.cs

@@ -0,0 +1,13 @@
+namespace Unity.Netcode.RuntimeTests
+{
+    public class NetworkVisibilityComponent : NetworkBehaviour
+    {
+        public void Hide()
+        {
+            GetComponent<NetworkObject>().CheckObjectVisibility += HandleCheckObjectVisibility;
+        }
+
+        protected virtual bool HandleCheckObjectVisibility(ulong clientId) => false;
+
+    }
+}

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

@@ -302,15 +302,17 @@ private static void SceneManagerValidationAndTestRunnerInitialization(NetworkMan
             }
         }
 
+        public delegate void BeforeClientStartCallback();
+
         /// <summary>
         /// Starts NetworkManager instances created by the Create method.
         /// </summary>
         /// <param name="host">Whether or not to create a Host instead of Server</param>
         /// <param name="server">The Server NetworkManager</param>
         /// <param name="clients">The Clients NetworkManager</param>
-        /// <param name="startInitializationCallback">called immediately after server and client(s) are started</param>
+        /// <param name="callback">called immediately after server is started and before client(s) are started</param>
         /// <returns></returns>
-        public static bool Start(bool host, NetworkManager server, NetworkManager[] clients)
+        public static bool Start(bool host, NetworkManager server, NetworkManager[] clients, BeforeClientStartCallback callback = null)
         {
             if (s_IsStarted)
             {
@@ -335,6 +337,8 @@ public static bool Start(bool host, NetworkManager server, NetworkManager[] clie
             // if set, then invoke this for the server
             RegisterHandlers(server);
 
+            callback?.Invoke();
+
             if (ClientSceneHandler != null)
             {
                 throw new Exception("Some how ClientSceneHandler did not get disposed when Destroy was called?");

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

@@ -0,0 +1,72 @@
+using System.Collections;
+using NUnit.Framework;
+using UnityEngine;
+using UnityEngine.TestTools;
+
+namespace Unity.Netcode.RuntimeTests
+{
+    public class NetworkVisibilityTests
+    {
+        private NetworkObject m_NetSpawnedObject;
+        private GameObject m_TestNetworkPrefab;
+
+        [TearDown]
+        public void TearDown()
+        {
+            MultiInstanceHelpers.Destroy();
+            if (m_TestNetworkPrefab)
+            {
+                Object.Destroy(m_TestNetworkPrefab);
+                m_TestNetworkPrefab = null;
+            }
+        }
+
+        [UnityTest]
+        public IEnumerator HiddenObjectsTest()
+        {
+
+            const int numClients = 1;
+            Assert.True(MultiInstanceHelpers.Create(numClients, out NetworkManager server, out NetworkManager[] clients));
+            m_TestNetworkPrefab = new GameObject("Object");
+            var networkObject = m_TestNetworkPrefab.AddComponent<NetworkObject>();
+            m_TestNetworkPrefab.AddComponent<NetworkVisibilityComponent>();
+
+            // Make it a prefab
+            MultiInstanceHelpers.MakeNetworkObjectTestPrefab(networkObject);
+
+            var validNetworkPrefab = new NetworkPrefab();
+            validNetworkPrefab.Prefab = m_TestNetworkPrefab;
+            server.NetworkConfig.NetworkPrefabs.Add(validNetworkPrefab);
+            server.NetworkConfig.EnableSceneManagement = false;
+            foreach (var client in clients)
+            {
+                client.NetworkConfig.NetworkPrefabs.Add(validNetworkPrefab);
+                client.NetworkConfig.EnableSceneManagement = false;
+            }
+
+            // Start the instances
+            if (!MultiInstanceHelpers.Start(true, server, clients, () =>
+            {
+                var serverObject = Object.Instantiate(m_TestNetworkPrefab, Vector3.zero, Quaternion.identity);
+                NetworkObject serverNetworkObject = serverObject.GetComponent<NetworkObject>();
+                serverNetworkObject.NetworkManagerOwner = server;
+                serverNetworkObject.Spawn();
+                serverObject.GetComponent<NetworkVisibilityComponent>().Hide();
+            }))
+            {
+                Debug.LogError("Failed to start instances");
+                Assert.Fail("Failed to start instances");
+            }
+
+            // [Client-Side] Wait for a connection to the server
+            yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.WaitForClientsConnected(clients, null, 512));
+
+            // [Host-Side] Check to make sure all clients are connected
+            yield return MultiInstanceHelpers.Run(
+                MultiInstanceHelpers.WaitForClientsConnectedToServer(server, clients.Length + 1, null, 512));
+
+            Assert.AreEqual(2, Object.FindObjectsOfType<NetworkVisibilityComponent>().Length);
+        }
+    }
+}
+