fix: snapshot system. last fixes for release (#1129)

* feat: snapshot. MTT-1087 first part. separating the side effect (storing the sent info) from preparing the buffer to send

* feat: snapshot. MTT-1087 second part. Writing SnapshotMessage that fit in the specified limit, instead of stopping once the limit is busted

* feat: snapshot. MTT-1087 part 3. Writing spawns after acks, because spawns are limited in message size

* feat: snapshot. MTT-1087 part 4. Using max spawn message size from NetworkConfig

* feat: snapshot. mtt-1087. Forgotten , for performance

* feat: snapshot. MTT-1089 pessimistic resend control. spawn part

* feat: snapshot. MTT-1089 pessimistic resend control. despawn part

* fix: snapshot. Cleaning up our clientData.SentSpawns data structure. Otherwise it grows unbound

* feat: snapshot. Last fixes for release. Prepares for MTT-1089, but leaves it disabled

* feat: snapshot. Last fixes for release. PR review comments

* style: snapshot. Coding standards

* style: snapshot. Coding standards

Author: jeffreyrainy

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

@@ -394,17 +394,15 @@ private void OnDestroy()
 
         private SnapshotDespawnCommand GetDespawnCommand()
         {
-            SnapshotDespawnCommand command;
+            var command = new SnapshotDespawnCommand();
             command.NetworkObjectId = NetworkObjectId;
-            command.TickWritten = default; // value will be set internally by SnapshotSystem
-            command.TargetClientIds = default;
 
             return command;
         }
 
         private SnapshotSpawnCommand GetSpawnCommand()
         {
-            SnapshotSpawnCommand command;
+            var command = new SnapshotSpawnCommand();
             command.NetworkObjectId = NetworkObjectId;
             command.OwnerClientId = OwnerClientId;
             command.IsPlayerObject = IsPlayerObject;
@@ -426,8 +424,6 @@ private SnapshotSpawnCommand GetSpawnCommand()
             command.ObjectPosition = transform.position;
             command.ObjectRotation = transform.rotation;
             command.ObjectScale = transform.localScale;
-            command.TickWritten = default; // value will be set internally by SnapshotSystem
-            command.TargetClientIds = default;
 
             return command;
         }

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

@@ -35,6 +35,7 @@ internal struct SnapshotDespawnCommand
         // snapshot internal
         internal int TickWritten;
         internal List<ulong> TargetClientIds;
+        internal int TimesWritten;
     }
 
     internal struct SnapshotSpawnCommand
@@ -57,6 +58,7 @@ internal struct SnapshotSpawnCommand
         // snapshot internal
         internal int TickWritten;
         internal List<ulong> TargetClientIds;
+        internal int TimesWritten;
     }
 
     // A table of NetworkVariables that constitutes a Snapshot.
@@ -191,6 +193,11 @@ internal void AddSpawn(SnapshotSpawnCommand command)
                     command.TargetClientIds = GetClientList();
                 }
 
+                // todo: store, for each client, the spawn not ack'ed yet,
+                // to prevent sending despawns to them.
+                // for clientData in client list
+                // clientData.SpawnSet.Add(command.NetworkObjectId);
+
                 // todo:
                 // this 'if' might be temporary, but is needed to help in debugging
                 // or maybe it stays
@@ -302,7 +309,7 @@ internal Entry ReadEntry(NetworkReader reader)
 
         internal SnapshotSpawnCommand ReadSpawn(NetworkReader reader)
         {
-            SnapshotSpawnCommand command;
+            var command = new SnapshotSpawnCommand();
 
             command.NetworkObjectId = reader.ReadUInt64Packed();
             command.GlobalObjectIdHash = (uint)reader.ReadUInt64Packed();
@@ -315,18 +322,16 @@ internal SnapshotSpawnCommand ReadSpawn(NetworkReader reader)
             command.ObjectScale = reader.ReadVector3();
 
             command.TickWritten = reader.ReadInt32Packed();
-            command.TargetClientIds = default;
 
             return command;
         }
 
         internal SnapshotDespawnCommand ReadDespawn(NetworkReader reader)
         {
-            SnapshotDespawnCommand command;
+            var command = new SnapshotDespawnCommand();
 
             command.NetworkObjectId = reader.ReadUInt64Packed();
             command.TickWritten = reader.ReadInt32Packed();
-            command.TargetClientIds = default;
 
             return command;
         }
@@ -499,8 +504,11 @@ internal void ReadAcks(ulong clientId, ClientData clientData, NetworkReader read
         internal void ProcessSingleAck(ushort ackSequence, ulong clientId, ClientData clientData, ConnectionRtt connection)
         {
             // look through the spawns sent
-            foreach (var sent in clientData.SentSpawns)
+            for (int index = 0; index < clientData.SentSpawns.Count; /*no increment*/)
             {
+                // needless copy, but I didn't find a way around
+                ClientData.SentSpawn sent = clientData.SentSpawns[index];
+
                 // for those with the sequence number being ack'ed
                 if (sent.SequenceNumber == ackSequence)
                 {
@@ -548,6 +556,16 @@ internal void ProcessSingleAck(ushort ackSequence, ulong clientId, ClientData cl
                             }
                         }
                     }
+
+                    // remove current `sent`, by moving last over,
+                    // as it was acknowledged.
+                    // skip incrementing index
+                    clientData.SentSpawns[index] = clientData.SentSpawns[clientData.SentSpawns.Count - 1];
+                    clientData.SentSpawns.RemoveAt(clientData.SentSpawns.Count - 1);
+                }
+                else
+                {
+                    index++;
                 }
             }
 
@@ -741,6 +759,38 @@ private void SendSnapshot(ulong clientId)
             }
         }
 
+        // Checks if a given SpawnCommand should be written to a Snapshot Message
+        // Performs exponential back off. To write a spawn a second time
+        // two ticks must have gone by. To write it a third time, four ticks, etc...
+        // This prioritize commands that have been re-sent less than others
+        private bool ShouldWriteSpawn(in SnapshotSpawnCommand spawnCommand)
+        {
+            if (m_CurrentTick < spawnCommand.TickWritten)
+            {
+                return false;
+            }
+
+            // 63 as we can't shift more than that.
+            var diff = Math.Min(63, m_CurrentTick - spawnCommand.TickWritten);
+
+            // -1 to make the first resend immediate
+            return (1 << diff) > (spawnCommand.TimesWritten - 1);
+        }
+
+        private bool ShouldWriteDespawn(in SnapshotDespawnCommand despawnCommand)
+        {
+            if (m_CurrentTick < despawnCommand.TickWritten)
+            {
+                return false;
+            }
+
+            // 63 as we can't shift more than that.
+            var diff = Math.Min(63, m_CurrentTick - despawnCommand.TickWritten);
+
+            // -1 to make the first resend immediate
+            return (1 << diff) > (despawnCommand.TimesWritten - 1);
+        }
+
         private void WriteSpawns(NetworkBuffer buffer, ulong clientId)
         {
             var spawnWritten = 0;
@@ -779,7 +829,8 @@ private void WriteSpawns(NetworkBuffer buffer, ulong clientId)
                 var index = clientData.NextSpawnIndex;
                 var savedPosition = writer.GetStream().Position;
 
-                if (m_Snapshot.Spawns[index].TargetClientIds.Contains(clientId))
+                // todo: re-enable ShouldWriteSpawn, once we have a mechanism to not let despawn pass in front of spawns
+                if (m_Snapshot.Spawns[index].TargetClientIds.Contains(clientId) /*&& ShouldWriteSpawn(m_Snapshot.Spawns[index])*/)
                 {
                     var sentSpawn = m_Snapshot.WriteSpawn(clientData, writer, in m_Snapshot.Spawns[index]);
 
@@ -792,6 +843,7 @@ private void WriteSpawns(NetworkBuffer buffer, ulong clientId)
                     }
                     else
                     {
+                        m_Snapshot.Spawns[index].TimesWritten++;
                         clientData.SentSpawns.Add(sentSpawn);
                         spawnWritten++;
                     }
@@ -805,15 +857,15 @@ private void WriteSpawns(NetworkBuffer buffer, ulong clientId)
             // ack'ed before sending a despawn for the same object.
             // Uncommenting this line would allow some despawn to be sent while spawns are pending.
             // As-is it is overly restrictive but allows us to go forward without the spawn/despawn dependency check
-
             // overSize = false;
 
             for (var j = 0; j < m_Snapshot.NumDespawns && !overSize; j++)
             {
                 var index = clientData.NextDespawnIndex;
                 var savedPosition = writer.GetStream().Position;
 
-                if (m_Snapshot.Despawns[index].TargetClientIds.Contains(clientId))
+                // todo: re-enable ShouldWriteSpawn, once we have a mechanism to not let despawn pass in front of spawns
+                if (m_Snapshot.Despawns[index].TargetClientIds.Contains(clientId) /*&& ShouldWriteDespawn(m_Snapshot.Despawns[index])*/)
                 {
                     var sentDespawn = m_Snapshot.WriteDespawn(clientData, writer, in m_Snapshot.Despawns[index]);
 
@@ -826,6 +878,7 @@ private void WriteSpawns(NetworkBuffer buffer, ulong clientId)
                     }
                     else
                     {
+                        m_Snapshot.Despawns[index].TimesWritten++;
                         clientData.SentSpawns.Add(sentDespawn);
                         despawnWritten++;
                     }
@@ -955,16 +1008,14 @@ internal void ReadSnapshot(ulong clientId, Stream snapshotStream)
                 clientId = m_NetworkManager.ServerClientId;
             }
 
-            int snapshotTick = default;
-
             using var reader = PooledNetworkReader.Get(snapshotStream);
             // make sure we have a ClientData entry for each client
             if (!m_ClientData.ContainsKey(clientId))
             {
                 m_ClientData.Add(clientId, new ClientData());
             }
 
-            snapshotTick = reader.ReadInt32Packed();
+            var snapshotTick = reader.ReadInt32Packed();
             var sequence = reader.ReadUInt16();
 
             if (sequence >= m_ClientData[clientId].LastReceivedSequence)