fix: networkshow client synchronization duplicate players (#3488)

The PR resolves the issue discovered when investigating the
[Forum-1651980
issue](https://discussions.unity.com/t/netcode-for-gameobjects-2-4-0-released/1651980/2).
The initial client synchronization pre-serialization preparation was not
excluding any spawned `NetworkObject` instances that had a pending
visibility update for the client being synchronized. This fix adds this
check to that process.

[MTTB-1372](https://jira.unity3d.com/browse/MTTB-1372)

## Changelog

- Fixed: Issue where the initial client synchronization
pre-serialization process was not excluding spawned `NetworkObjects`
that already had pending visibility for the client being synchronized.

## Testing and Documentation

- Includes the `PlayerSpawnObjectVisibilityTests`.
- No documentation changes or additions were necessary.

<!-- Uncomment and mark items off with a * if this PR deprecates any
API:
### Deprecated API
- [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter
yyyy-mm-dd)` entry.
- [ ] An [api updater] was added.
- [ ] Deprecation of the API is explained in the CHANGELOG.
- [ ] The users can understand why this API was removed and what they
should use instead.
-->

## Backport

This requires a backport to v1.0 (#3493)

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -12,7 +12,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
-- Fixed: Issue where there was a potential for a small memory leak in the `ConnectionApprovedMessage`. (#3486)
+- Fixed issue where the initial client synchronization pre-serialization process was not excluding spawned `NetworkObject` instances that already had pending visibility for the client being synchronized. (#3488)
+- Fixed issue where there was a potential for a small memory leak in the `ConnectionApprovedMessage`. (#3486)
 
 ### Changed
 

com.unity.netcode.gameobjects/Runtime/SceneManagement/SceneEventData.cs

@@ -324,6 +324,12 @@ internal void AddSpawnedNetworkObjects()
             var distributedAuthoritySendingToService = m_NetworkManager.DistributedAuthorityMode && TargetClientId == NetworkManager.ServerClientId;
             foreach (var sobj in m_NetworkManager.SpawnManager.SpawnedObjectsList)
             {
+                var spawnedObject = sobj;
+                // Don't synchronize objects that have pending visibility as that will be sent as a CreateObjectMessage towards the end of the current frame
+                if (TargetClientId != NetworkManager.ServerClientId && m_NetworkManager.SpawnManager.IsObjectVisibilityPending(TargetClientId, ref spawnedObject))
+                {
+                    continue;
+                }
                 if (sobj.Observers.Contains(TargetClientId) || distributedAuthoritySendingToService)
                 {
                     m_NetworkObjectsSync.Add(sobj);

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

@@ -0,0 +1,91 @@
+using System.Collections;
+using System.Text.RegularExpressions;
+using NUnit.Framework;
+using Unity.Netcode.TestHelpers.Runtime;
+using UnityEngine;
+using UnityEngine.TestTools;
+
+
+namespace Unity.Netcode.RuntimeTests
+{
+    [TestFixture(HostOrServer.Server)]
+    [TestFixture(HostOrServer.Host)]
+    internal class PlayerSpawnObjectVisibilityTests : NetcodeIntegrationTest
+    {
+        protected override int NumberOfClients => 0;
+
+        public enum PlayerSpawnStages
+        {
+            OnNetworkSpawn,
+            OnNetworkPostSpawn,
+        }
+
+        public PlayerSpawnObjectVisibilityTests(HostOrServer hostOrServer) : base(hostOrServer) { }
+
+        public class PlayerVisibilityTestComponent : NetworkBehaviour
+        {
+            public PlayerSpawnStages Stage;
+
+            private void Awake()
+            {
+                var networkObject = GetComponent<NetworkObject>();
+                // Assure the player prefab will not spawn with observers.
+                // This assures that when the server/host spawns the connecting client's
+                // player prefab, the spawn object will initially not be spawnd on the client side.
+                networkObject.SpawnWithObservers = false;
+            }
+
+            public override void OnNetworkSpawn()
+            {
+                ShowToClient(PlayerSpawnStages.OnNetworkSpawn);
+                base.OnNetworkSpawn();
+            }
+
+            protected override void OnNetworkPostSpawn()
+            {
+                ShowToClient(PlayerSpawnStages.OnNetworkPostSpawn);
+                base.OnNetworkPostSpawn();
+            }
+
+            private void ShowToClient(PlayerSpawnStages currentStage)
+            {
+                if (!HasAuthority || Stage != currentStage)
+                {
+                    return;
+                }
+                NetworkObject.NetworkShow(OwnerClientId);
+            }
+        }
+
+        protected override void OnCreatePlayerPrefab()
+        {
+            m_PlayerPrefab.AddComponent<PlayerVisibilityTestComponent>();
+            base.OnCreatePlayerPrefab();
+        }
+
+        /// <summary>
+        /// Tests the scenario where under a client-server network topology if a player prefab
+        /// is spawned by the server with no observers but the player prefab itself has server
+        /// side script that will network show the spawned object to the owning client.
+        ///
+        /// Because NetworkShow will defer the CreateObjectMessage until the late update, the
+        /// server/host needs to filter out including anything within the synchronization
+        /// message that already has pending visibility.
+        /// </summary>
+        /// <param name="spawnStage">Spawn stages to test</param>
+        /// <returns>IEnumerator</returns>
+        [UnityTest]
+        public IEnumerator NetworkShowOnSpawnTest([Values] PlayerSpawnStages spawnStage)
+        {
+            m_PlayerPrefab.GetComponent<PlayerVisibilityTestComponent>().Stage = spawnStage;
+
+            yield return CreateAndStartNewClient();
+
+            yield return new WaitForSeconds(0.25f);
+
+            NetcodeLogAssert.LogWasNotReceived(LogType.Warning, new Regex("but it is already in the spawned list!"));
+            var client = GetNonAuthorityNetworkManager();
+            Assert.True(client.LocalClient.PlayerObject != null, $"Client-{client.LocalClientId} does not have a player object!");
+        }
+    }
+}