fix: resetting the NetworkVariable dirty status at end of frame, … (#2344)

* fix: resetting the NetworkVariable dirty status at end of frame, even if a given NetworkVariable is not sent to any client

* delaying NetworkShow until end of frame

* handling NetworkHide of objects pending their NetworkShow

Author: jeffreyrainy

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -12,6 +12,9 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Changed
 
+- `NetworkShow()` of `NetworkObject`s are delayed until the end of the frame to ensure consistency of delta-driven variables like `NetworkList`.
+- Dirty `NetworkObject` are reset at end-of-frame and not at serialization time.
+- `NetworkHide()` of an object that was just `NetworkShow()`n produces a warning, as remote clients will _not_ get a spawn/despawn pair.
 - The default listen address of `UnityTransport` is now 0.0.0.0. (#2307)
 - Renamed the NetworkTransform.SetState parameter `shouldGhostsInterpolate` to `teleportDisabled` for better clarity of what that parameter does. (#2228)
 

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

@@ -72,6 +72,23 @@ internal void NetworkBehaviourUpdate(NetworkManager networkManager)
                         }
                     }
                 }
+
+                foreach (var dirtyObj in m_DirtyNetworkObjects)
+                {
+                    for (int k = 0; k < dirtyObj.ChildNetworkBehaviours.Count; k++)
+                    {
+                        var behaviour = dirtyObj.ChildNetworkBehaviours[k];
+                        for (int i = 0; i < behaviour.NetworkVariableFields.Count; i++)
+                        {
+                            if (behaviour.NetworkVariableFields[i].IsDirty() &&
+                                !behaviour.NetworkVariableIndexesToResetSet.Contains(i))
+                            {
+                                behaviour.NetworkVariableIndexesToResetSet.Add(i);
+                                behaviour.NetworkVariableIndexesToReset.Add(i);
+                            }
+                        }
+                    }
+                }
                 // Now, reset all the no-longer-dirty variables
                 foreach (var dirtyobj in m_DirtyNetworkObjects)
                 {

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

@@ -67,6 +67,9 @@ internal void MarkNetworkObjectDirty(NetworkObject networkObject)
 
         internal Dictionary<ulong, ConnectionApprovalResponse> ClientsToApprove = new Dictionary<ulong, ConnectionApprovalResponse>();
 
+        // Stores the objects that need to be shown at end-of-frame
+        internal Dictionary<ulong, List<NetworkObject>> ObjectsToShowToClient = new Dictionary<ulong, List<NetworkObject>>();
+
         /// <summary>
         /// The <see cref="NetworkPrefabHandler"/> instance created after starting the <see cref="NetworkManager"/>
         /// </summary>
@@ -1663,6 +1666,17 @@ private void OnNetworkManagerTick()
             // Do NetworkVariable updates
             BehaviourUpdater.NetworkBehaviourUpdate(this);
 
+            // Handle NetworkObjects to show
+            foreach (var client in ObjectsToShowToClient)
+            {
+                ulong clientId = client.Key;
+                foreach (var networkObject in client.Value)
+                {
+                    SpawnManager.SendSpawnCallForObject(clientId, networkObject);
+                }
+            }
+            ObjectsToShowToClient.Clear();
+
             int timeSyncFrequencyTicks = (int)(k_TimeSyncFrequency * NetworkConfig.TickRate);
             if (IsServer && NetworkTickSystem.ServerTime.Tick % timeSyncFrequencyTicks == 0)
             {
@@ -2339,5 +2353,37 @@ internal void ApprovedPlayerSpawn(ulong clientId, uint playerPrefabHash)
                 NetworkMetrics.TrackObjectSpawnSent(clientPair.Key, ConnectedClients[clientId].PlayerObject, size);
             }
         }
+
+        internal void MarkObjectForShowingTo(NetworkObject networkObject, ulong clientId)
+        {
+            if (!ObjectsToShowToClient.ContainsKey(clientId))
+            {
+                ObjectsToShowToClient.Add(clientId, new List<NetworkObject>());
+            }
+            ObjectsToShowToClient[clientId].Add(networkObject);
+        }
+
+        // returns whether any matching objects would have become visible and were returned to hidden state
+        internal bool RemoveObjectFromShowingTo(NetworkObject networkObject, ulong clientId)
+        {
+            var ret = false;
+            if (!ObjectsToShowToClient.ContainsKey(clientId))
+            {
+                return false;
+            }
+
+            // probably overkill, but deals with multiple entries
+            while (ObjectsToShowToClient[clientId].Contains(networkObject))
+            {
+                Debug.LogWarning(
+                    "Object was shown and hidden from the same client in the same Network frame. As a result, the client will _not_ receive a NetworkSpawn");
+                ObjectsToShowToClient[clientId].Remove(networkObject);
+                ret = true;
+            }
+
+            networkObject.Observers.Remove(clientId);
+
+            return ret;
+        }
     }
 }

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

@@ -265,9 +265,8 @@ public void NetworkShow(ulong clientId)
                 throw new VisibilityChangeException("The object is already visible");
             }
 
+            NetworkManager.MarkObjectForShowingTo(this, clientId);
             Observers.Add(clientId);
-
-            NetworkManager.SpawnManager.SendSpawnCallForObject(clientId, this);
         }
 
 
@@ -351,26 +350,28 @@ public void NetworkHide(ulong clientId)
                 throw new NotServerException("Only server can change visibility");
             }
 
-            if (!Observers.Contains(clientId))
-            {
-                throw new VisibilityChangeException("The object is already hidden");
-            }
-
             if (clientId == NetworkManager.ServerClientId)
             {
                 throw new VisibilityChangeException("Cannot hide an object from the server");
             }
 
-            Observers.Remove(clientId);
-
-            var message = new DestroyObjectMessage
+            if (!NetworkManager.RemoveObjectFromShowingTo(this, clientId))
             {
-                NetworkObjectId = NetworkObjectId,
-                DestroyGameObject = !IsSceneObject.Value
-            };
-            // Send destroy call
-            var size = NetworkManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, clientId);
-            NetworkManager.NetworkMetrics.TrackObjectDestroySent(clientId, this, size);
+                if (!Observers.Contains(clientId))
+                {
+                    throw new VisibilityChangeException("The object is already hidden");
+                }
+                Observers.Remove(clientId);
+
+                var message = new DestroyObjectMessage
+                {
+                    NetworkObjectId = NetworkObjectId,
+                    DestroyGameObject = !IsSceneObject.Value
+                };
+                // Send destroy call
+                var size = NetworkManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, clientId);
+                NetworkManager.NetworkMetrics.TrackObjectDestroySent(clientId, this, size);
+            }
         }
 
         /// <summary>

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

@@ -64,6 +64,16 @@ public void Serialize(FastBufferWriter writer, int targetVersion)
                     shouldWrite = false;
                 }
 
+                // The object containing the behaviour we're about to process is about to be shown to this client
+                // As a result, the client will get the fully serialized NetworkVariable and would be confused by
+                // an extraneous delta
+                if (NetworkBehaviour.NetworkManager.ObjectsToShowToClient.ContainsKey(TargetClientId) &&
+                    NetworkBehaviour.NetworkManager.ObjectsToShowToClient[TargetClientId]
+                    .Contains(NetworkBehaviour.NetworkObject))
+                {
+                    shouldWrite = false;
+                }
+
                 if (NetworkBehaviour.NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
                 {
                     if (!shouldWrite)
@@ -96,12 +106,6 @@ public void Serialize(FastBufferWriter writer, int targetVersion)
                         networkVariable.WriteDelta(writer);
                     }
 
-                    if (!NetworkBehaviour.NetworkVariableIndexesToResetSet.Contains(i))
-                    {
-                        NetworkBehaviour.NetworkVariableIndexesToResetSet.Add(i);
-                        NetworkBehaviour.NetworkVariableIndexesToReset.Add(i);
-                    }
-
                     NetworkBehaviour.NetworkManager.NetworkMetrics.TrackNetworkVariableDeltaSent(
                         TargetClientId,
                         NetworkBehaviour.NetworkObject,