fix: NetworkObject.SpawnWithObservers was not being honored for late joining clients [MTT-6934] (#2623)

* fix

This resolves the issue where NetworkObjects with SpawnWithObservers set to false was not being honored for late joining clients.
This also resolves an issue when scene management is disabled the NetworkObjects' observers were not being updated for newly connected clients.

* test

This validates the fix for the late joining client observer issue.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -10,11 +10,14 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Added
 
+
 ### Fixed
-- Fixed a failing UTP test that was failing when you install Unity Transport package 2.0.0 or newer.
+
+- Fixed issue where `NetworkObject.SpawnWithObservers` was not being honored for late joining clients. (#2623)
 
 ## Changed
 
+
 ## [1.5.1] - 2023-06-07
 
 ### Added

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

@@ -628,6 +628,8 @@ internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.Conne
                     };
                     if (!NetworkManager.NetworkConfig.EnableSceneManagement)
                     {
+                        // Update the observed spawned NetworkObjects for the newly connected player when scene management is disabled
+                        NetworkManager.SpawnManager.UpdateObservedNetworkObjects(ownerClientId);
                         if (NetworkManager.SpawnManager.SpawnedObjectsList.Count != 0)
                         {
                             message.SpawnedObjectsList = NetworkManager.SpawnManager.SpawnedObjectsList;
@@ -651,12 +653,12 @@ internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.Conne
                     SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, ownerClientId);
                     message.MessageVersions.Dispose();
 
-                    // If scene management is enabled, then let NetworkSceneManager handle the initial scene and NetworkObject synchronization
+                    // If scene management is disabled, then we are done and notify the local host-server the client is connected
                     if (!NetworkManager.NetworkConfig.EnableSceneManagement)
                     {
                         InvokeOnClientConnectedCallback(ownerClientId);
                     }
-                    else
+                    else // Otherwise, let NetworkSceneManager handle the initial scene and NetworkObject synchronization
                     {
                         NetworkManager.SceneManager.SynchronizeNetworkObjects(ownerClientId);
                     }

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

@@ -952,27 +952,35 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
         }
 
         /// <summary>
-        /// Updates all spawned <see cref="NetworkObject.Observers"/> for the specified client
+        /// Updates all spawned <see cref="NetworkObject.Observers"/> for the specified newly connected client 
         /// Note: if the clientId is the server then it is observable to all spawned <see cref="NetworkObject"/>'s
         /// </summary>
+        /// <remarks>
+        /// This method is to only to be used for newly connected clients in order to update the observers list for
+        /// each NetworkObject instance.
+        /// </remarks>
         internal void UpdateObservedNetworkObjects(ulong clientId)
         {
             foreach (var sobj in SpawnedObjectsList)
             {
+                // If the NetworkObject has no visibility check then prepare to add this client as an observer
                 if (sobj.CheckObjectVisibility == null)
                 {
-                    if (!sobj.Observers.Contains(clientId))
+                    // If the client is not part of the observers and spawn with observers is enabled on this instance or the clientId is the server
+                    if (!sobj.Observers.Contains(clientId) && (sobj.SpawnWithObservers || clientId == NetworkManager.ServerClientId))
                     {
                         sobj.Observers.Add(clientId);
                     }
                 }
                 else
                 {
+                    // CheckObject visibility overrides SpawnWithObservers under this condition
                     if (sobj.CheckObjectVisibility(clientId))
                     {
                         sobj.Observers.Add(clientId);
                     }
-                    else if (sobj.Observers.Contains(clientId))
+                    else // Otherwise, if the observers contains the clientId (shouldn't happen) then remove it since CheckObjectVisibility returned false
+                    if (sobj.Observers.Contains(clientId))
                     {
                         sobj.Observers.Remove(clientId);
                     }

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

@@ -61,8 +61,26 @@ private bool CheckClientsSideObserverTestObj()
             return true;
         }
 
+        /// <summary>
+        /// Assures the <see cref="ObserverSpawnTests"/> late joining client has all
+        /// NetworkPrefabs required to connect.
+        /// </summary>
+        protected override void OnNewClientCreated(NetworkManager networkManager)
+        {
+            foreach (var networkPrefab in m_ServerNetworkManager.NetworkConfig.Prefabs.Prefabs)
+            {
+                if (!networkManager.NetworkConfig.Prefabs.Contains(networkPrefab.Prefab))
+                {
+                    networkManager.NetworkConfig.Prefabs.Add(networkPrefab);
+                }
+            }
+            base.OnNewClientCreated(networkManager);
+        }
 
-
+        /// <summary>
+        /// This test validates <see cref="NetworkObject.SpawnWithObservers"/> property
+        /// </summary>
+        /// <param name="observerTestTypes">whether to spawn with or without observers</param>
         [UnityTest]
         public IEnumerator ObserverSpawnTests([Values] ObserverTestTypes observerTestTypes)
         {
@@ -92,6 +110,23 @@ public IEnumerator ObserverSpawnTests([Values] ObserverTestTypes observerTestTyp
                 m_ObserverTestType = ObserverTestTypes.WithObservers;
                 yield return WaitForConditionOrTimeOut(CheckClientsSideObserverTestObj);
                 AssertOnTimeout($"{k_WithObserversError} {k_ObserverTestObjName} object!");
+
+                // Validate that a late joining client does not see the NetworkObject when it spawns
+                yield return CreateAndStartNewClient();
+
+                m_ObserverTestType = ObserverTestTypes.WithoutObservers;
+                // Just give a little time to make sure nothing spawned
+                yield return s_DefaultWaitForTick;
+                yield return WaitForConditionOrTimeOut(CheckClientsSideObserverTestObj);
+                AssertOnTimeout($"{(withoutObservers ? k_WithoutObserversError : k_WithObserversError)} {k_ObserverTestObjName} object!");
+
+                // Now validate that we can make the NetworkObject visible to the newly joined client
+                m_ObserverTestNetworkObject.NetworkShow(m_ClientNetworkManagers[NumberOfClients].LocalClientId);
+
+                // Validate the NetworkObject is visible to all connected clients (including the recently joined client)
+                m_ObserverTestType = ObserverTestTypes.WithObservers;
+                yield return WaitForConditionOrTimeOut(CheckClientsSideObserverTestObj);
+                AssertOnTimeout($"{k_WithObserversError} {k_ObserverTestObjName} object!");
             }
         }
         /// <summary>