fix: issue #1151 - client RPCs invoked in OnNetworkSpawn being dropped

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

@@ -1517,10 +1517,11 @@ internal void HandleApproval(ulong ownerClientId, bool createPlayerObject, uint?
                 m_ConnectedClientsList.Add(client);
                 m_ConnectedClientIds.Add(client.ClientId);
 
+                NetworkObject networkObject = null;
                 if (createPlayerObject)
                 {
-                    var networkObject = SpawnManager.CreateLocalNetworkObject(false, playerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash, ownerClientId, null, position, rotation);
-                    SpawnManager.SpawnNetworkObjectLocally(networkObject, SpawnManager.GetNetworkObjectId(), false, true, ownerClientId, false);
+                    networkObject = SpawnManager.CreateLocalNetworkObject(false, playerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash, ownerClientId, null, position, rotation);
+                    SpawnManager.SpawnNetworkObjectLocally(networkObject, SpawnManager.GetNetworkObjectId(), false, true, ownerClientId, false, false);
 
                     ConnectedClients[ownerClientId].PlayerObject = networkObject;
                 }
@@ -1567,6 +1568,12 @@ internal void HandleApproval(ulong ownerClientId, bool createPlayerObject, uint?
 
                 // Separating this into a contained function call for potential further future separation of when this notification is sent.
                 ApprovedPlayerSpawn(ownerClientId, playerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash);
+
+                // Moving this to the end here to ensure it's invoked after any client spawn messages have been sent.
+                if (createPlayerObject)
+                {
+                    networkObject.InvokeBehaviourNetworkSpawn();
+                }
             }
             else
             {

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

@@ -464,7 +464,7 @@ private void SpawnInternal(bool destroyWithScene, ulong? ownerClientId, bool pla
                 throw new NotServerException($"Only server can spawn {nameof(NetworkObject)}s");
             }
 
-            NetworkManager.SpawnManager.SpawnNetworkObjectLocally(this, NetworkManager.SpawnManager.GetNetworkObjectId(), false, playerObject, ownerClientId, destroyWithScene);
+            NetworkManager.SpawnManager.SpawnNetworkObjectLocally(this, NetworkManager.SpawnManager.GetNetworkObjectId(), false, playerObject, ownerClientId, destroyWithScene, false);
 
             if (NetworkManager.NetworkConfig.UseSnapshotSpawn)
             {
@@ -479,6 +479,9 @@ private void SpawnInternal(bool destroyWithScene, ulong? ownerClientId, bool pla
                     NetworkManager.SpawnManager.SendSpawnCallForObject(NetworkManager.ConnectedClientsList[i].ClientId, this);
                 }
             }
+
+            // Moving this to the end here to ensure it's invoked after any client spawn messages have been sent.
+            InvokeBehaviourNetworkSpawn();
         }
 
         /// <summary>

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

@@ -301,7 +301,7 @@ internal NetworkObject CreateLocalNetworkObject(bool isSceneObject, uint globalO
         }
 
         // Ran on both server and client
-        internal void SpawnNetworkObjectLocally(NetworkObject networkObject, ulong networkId, bool sceneObject, bool playerObject, ulong? ownerClientId, bool destroyWithScene)
+        internal void SpawnNetworkObjectLocally(NetworkObject networkObject, ulong networkId, bool sceneObject, bool playerObject, ulong? ownerClientId, bool destroyWithScene, bool invokeOnNetworkSpawn = true)
         {
             if (networkObject == null)
             {
@@ -313,12 +313,12 @@ internal void SpawnNetworkObjectLocally(NetworkObject networkObject, ulong netwo
                 throw new SpawnStateException("Object is already spawned");
             }
 
-            SpawnNetworkObjectLocallyCommon(networkObject, networkId, sceneObject, playerObject, ownerClientId, destroyWithScene);
+            SpawnNetworkObjectLocallyCommon(networkObject, networkId, sceneObject, playerObject, ownerClientId, destroyWithScene, invokeOnNetworkSpawn);
         }
 
         // Ran on both server and client
         internal void SpawnNetworkObjectLocally(NetworkObject networkObject, in NetworkObject.SceneObject sceneObject,
-            FastBufferReader variableData, bool destroyWithScene)
+            FastBufferReader variableData, bool destroyWithScene, bool invokeOnNetworkSpawn = true)
         {
             if (networkObject == null)
             {
@@ -335,10 +335,10 @@ internal void SpawnNetworkObjectLocally(NetworkObject networkObject, in NetworkO
                 networkObject.SetNetworkVariableData(variableData);
             }
 
-            SpawnNetworkObjectLocallyCommon(networkObject, sceneObject.Header.NetworkObjectId, sceneObject.Header.IsSceneObject, sceneObject.Header.IsPlayerObject, sceneObject.Header.OwnerClientId, destroyWithScene);
+            SpawnNetworkObjectLocallyCommon(networkObject, sceneObject.Header.NetworkObjectId, sceneObject.Header.IsSceneObject, sceneObject.Header.IsPlayerObject, sceneObject.Header.OwnerClientId, destroyWithScene, invokeOnNetworkSpawn);
         }
 
-        private void SpawnNetworkObjectLocallyCommon(NetworkObject networkObject, ulong networkId, bool sceneObject, bool playerObject, ulong? ownerClientId, bool destroyWithScene)
+        private void SpawnNetworkObjectLocallyCommon(NetworkObject networkObject, ulong networkId, bool sceneObject, bool playerObject, ulong? ownerClientId, bool destroyWithScene, bool invokeOnNetworkSpawn)
         {
             if (SpawnedObjects.ContainsKey(networkId))
             {
@@ -402,7 +402,10 @@ private void SpawnNetworkObjectLocallyCommon(NetworkObject networkObject, ulong
             networkObject.SetCachedParent(networkObject.transform.parent);
             networkObject.ApplyNetworkParenting();
             NetworkObject.CheckOrphanChildren();
-            networkObject.InvokeBehaviourNetworkSpawn();
+            if (invokeOnNetworkSpawn)
+            {
+                networkObject.InvokeBehaviourNetworkSpawn();
+            }
         }
 
         internal void SendSpawnCallForObject(ulong clientId, NetworkObject networkObject)

testproject/Assets/Tests/Runtime/MessageOrdering.cs

@@ -18,6 +18,7 @@ public IEnumerator SetUp()
             // Make sure these static values are reset
             Support.SpawnRpcDespawn.ClientUpdateCount = 0;
             Support.SpawnRpcDespawn.ServerUpdateCount = 0;
+            Support.SpawnRpcDespawn.ClientNetworkSpawnRpcCalled = false;
             yield break;
         }
 
@@ -32,6 +33,7 @@ public IEnumerator Teardown()
                 m_Prefab = null;
                 Support.SpawnRpcDespawn.ClientUpdateCount = 0;
                 Support.SpawnRpcDespawn.ServerUpdateCount = 0;
+                Support.SpawnRpcDespawn.ClientNetworkSpawnRpcCalled = false;
             }
             yield break;
         }
@@ -169,5 +171,78 @@ public IEnumerator SpawnRpcDespawn()
             yield return new WaitUntil(() => Time.frameCount >= lastFrameNumber);
             Assert.True(handler.WasDestroyed);
         }
+
+        [UnityTest]
+        public IEnumerator RpcOnNetworkSpawn()
+        {
+            // Must be 1 for this test.
+            const int numClients = 1;
+            Assert.True(MultiInstanceHelpers.Create(numClients, out NetworkManager server, out NetworkManager[] clients));
+            m_Prefab = new GameObject("Object");
+            m_Prefab.AddComponent<SpawnRpcDespawn>();
+            Support.SpawnRpcDespawn.TestStage = NetworkUpdateStage.EarlyUpdate;
+            var networkObject = m_Prefab.AddComponent<NetworkObject>();
+
+            // Make it a prefab
+            MultiInstanceHelpers.MakeNetworkObjectTestPrefab(networkObject);
+            var handler = new SpawnRpcDespawnInstanceHandler(networkObject.GlobalObjectIdHash);
+            foreach (var client in clients)
+            {
+                client.PrefabHandler.AddHandler(networkObject, handler);
+            }
+
+            var validNetworkPrefab = new NetworkPrefab();
+            validNetworkPrefab.Prefab = m_Prefab;
+            server.NetworkConfig.NetworkPrefabs.Add(validNetworkPrefab);
+            foreach (var client in clients)
+            {
+                client.NetworkConfig.NetworkPrefabs.Add(validNetworkPrefab);
+            }
+
+            // Start the instances
+            if (!MultiInstanceHelpers.Start(false, server, clients))
+            {
+                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, null, 512));
+
+            var serverObject = Object.Instantiate(m_Prefab, Vector3.zero, Quaternion.identity);
+            NetworkObject serverNetworkObject = serverObject.GetComponent<NetworkObject>();
+            serverNetworkObject.NetworkManagerOwner = server;
+            serverNetworkObject.Spawn();
+
+            // Wait until all objects have spawned.
+            const int maxFrames = 240;
+            var doubleCheckTime = Time.realtimeSinceStartup + 5.0f;
+            while (!Support.SpawnRpcDespawn.ClientNetworkSpawnRpcCalled)
+            {
+                if (Time.frameCount > maxFrames)
+                {
+                    // This is here in the event a platform is running at a higher
+                    // frame rate than expected
+                    if (doubleCheckTime < Time.realtimeSinceStartup)
+                    {
+                        Assert.Fail("Did not successfully call all expected client RPCs");
+                        break;
+                    }
+                }
+                var nextFrameNumber = Time.frameCount + 1;
+                yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber);
+            }
+
+            Assert.True(handler.WasSpawned);
+            Assert.True(Support.SpawnRpcDespawn.ClientNetworkSpawnRpcCalled);
+            var lastFrameNumber = Time.frameCount + 1;
+            Object.Destroy(serverObject);
+            yield return new WaitUntil(() => Time.frameCount >= lastFrameNumber);
+            Assert.True(handler.WasDestroyed);
+        }
     }
 }

testproject/Assets/Tests/Runtime/Support/SpawnRpcDespawn.cs

@@ -10,6 +10,7 @@ public class SpawnRpcDespawn : NetworkBehaviour, INetworkUpdateSystem
         public static NetworkUpdateStage TestStage;
         public static int ClientUpdateCount;
         public static int ServerUpdateCount;
+        public static bool ClientNetworkSpawnRpcCalled;
         public static NetworkUpdateStage StageExecutedByReceiver;
 
         private bool m_Active = false;
@@ -37,6 +38,22 @@ public void Activate()
             m_Active = true;
         }
 
+        public override void OnNetworkSpawn()
+        {
+            if (!IsServer)
+            {
+                return;
+            }
+
+            TestClientRpc();
+        }
+
+        [ClientRpc]
+        private void TestClientRpc()
+        {
+            ClientNetworkSpawnRpcCalled = true;
+        }
+
         public void NetworkStart()
         {
             Debug.Log($"Network Start on client {NetworkManager.LocalClientId.ToString()}");
@@ -66,7 +83,7 @@ private void RunTest()
             Debug.Log("Running test...");
             GetComponent<NetworkObject>().Spawn();
             IncrementUpdateCount();
-            GetComponent<NetworkObject>().Despawn(false);
+            Destroy(gameObject);
             m_Active = false;
         }