feat: snapshot. MTT-1088 Snapshot acknowledgment gaps (#1083)

* feat: snapshot. Attempt at applying snapshot spawn branch over new message queue

* feat: snapshot. Reusing spawn entries.

* feat: snapshot fix, prefab handling

* feat: snapshot. reduce log spam

* snapshot: merge preparation. Removing old acks, removing unused variables

* feat: snapshot. merge preparation. Removing old acks, removing unused variables

* feat: snapshot. more reasonable sentinel checks around snapshot send/receive

* feat: snapshot. small clenaup

* feat: snapshot. Moving configuration items to NetworkConfig

* feat: snapshot. Adjusting to code standard

* refactor: moved code from SpawnInternal into separate SendToSnapshot function. This will allow reusing it from NetworkShow. But as-is, no impact

* refactor: calling networkShow(NetworkObject) code in networkshow(List<NetworkObject>) instead of calling a deeper-nested function. Make the code more uniform and prepares for incoming snapshot changes

* style: whitespace

* refactor: also using NetworkHide(NetworkObject) to implement NetworkHide(List<NetworkObject>). Same reasons as for NetworkShow

* feat: snapshot. Safer access to Connection RTT structures

* feat: snapshot. placeholder comment for code to come later

* feat: snapshot. Snapshot fully working for spawn test.
Keeps reference to proper network manager.
Allows sending spawn to individual instances

* feat: snapshot. despawn going via snapshot. snapshot message size limitation. snapshot message packing in a LRU fashion

* feat: snapshot. Incrementing the local span and despawn store (amortized linear) when game code demands too much spawn/despawns

* feat: snapshot. Using different list to keep track of spawn despawn times. Probably not needed, might be revert in the future, but eases debugging. Also debug prints, obviously temporary

* feat: snapshot. extra logging for debugging

* fix: snapshot. MTT-1056 despawn loss

* feat: snapshot. Proper test for length used in snapshot message. Removed redundant condition for target client list

* fix: snapshot. NetworkHide in snapshot mode was incorrectly hiding the object on all machines

* feat: snapshot. Keeping snapshot disabled for this release

* style: coding standards fix

* test: making the networkshow/hide test more resistant to timing difference on the test machine, waiting half a second instead of a tenth.

* test: removing Sleep() code in NetworkShowHide that attempted to deal with timing. Used explicit timeout instead. Less flaky

* feat: snapshot, computing the mask of past received messages. Not used yet.

* feat: snapshot. First pass at PR code review

* feat: snapshot. Adjusting to int ticks

* feat: snapshot. Reducing the amount of logging, as this is getting merged to develop

* feat: snapshot. MTT-1088. Acknowledgement for multiple messages, not just the latest

* feat: snapshot. Enabling snapshot in this branch, by default

* feat: snapshot. Fixing a bad merge from develop

* feat: snapshot. prep for PR, MTT-1088, plus packet loss for testing

* feat: snapshot. prep 2 for PR, MTT-1088

* feat: snapshot. prep 3 for PR, MTT-1088. disbaling snapshot, and whitespace fix

Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com>

Author: jeffreyrainy

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

@@ -472,10 +472,30 @@ internal void ReadSpawns(NetworkReader reader)
             }
         }
 
-        internal ushort ReadAcks(ulong clientId, ClientData clientData, NetworkReader reader)
+        internal void ReadAcks(ulong clientId, ClientData clientData, NetworkReader reader, ConnectionRtt connection)
         {
             ushort ackSequence = reader.ReadUInt16();
+            ushort seqMask = reader.ReadUInt16();
 
+            // process the latest acknowledgment
+            ProcessSingleAck(ackSequence, clientId, clientData, connection);
+
+            // for each bit in the mask, acknowledge one message before
+            while (seqMask != 0)
+            {
+                ackSequence--;
+                // extract least bit
+                if (seqMask % 2 == 1)
+                {
+                    ProcessSingleAck(ackSequence, clientId, clientData, connection);
+                }
+                // move to next bit
+                seqMask >>= 1;
+            }
+        }
+
+        internal void ProcessSingleAck(ushort ackSequence, ulong clientId, ClientData clientData, ConnectionRtt connection)
+        {
             // look through the spawns sent
             foreach (var sent in clientData.SentSpawns)
             {
@@ -529,7 +549,8 @@ internal ushort ReadAcks(ulong clientId, ClientData clientData, NetworkReader re
                 }
             }
 
-            return ackSequence;
+            // keep track of RTTs, using the sequence number acknowledgement as a marker
+            connection.NotifyAck(ackSequence, Time.unscaledTime);
         }
 
         /// <summary>
@@ -564,6 +585,7 @@ internal struct SentSpawn // this struct also stores Despawns, not just Spawns
 
         internal ushort SequenceNumber = 0; // the next sequence number to use for this client
         internal ushort LastReceivedSequence = 0; // the last sequence number received by this client
+        internal ushort ReceivedSequenceMask = 0; // bitmask of the messages before the last one that we received.
 
         internal int NextSpawnIndex = 0; // index of the last spawn sent. Used to cycle through spawns (LRU scheme)
         internal int NextDespawnIndex = 0; // same as above, but for despawns.
@@ -686,8 +708,6 @@ private void SendSnapshot(ulong clientId)
 
             m_ConnectionRtts[clientId].NotifySend(m_ClientData[clientId].SequenceNumber, Time.unscaledTime);
 
-            // Send the entry index and the buffer where the variables are serialized
-
             var context = m_NetworkManager.MessageQueueContainer.EnterInternalCommandContext(
                 MessageQueueContainer.MessageType.SnapshotData, NetworkChannel.SnapshotExchange,
                 new[] { clientId }, NetworkUpdateLoop.UpdateStage);
@@ -698,20 +718,27 @@ private void SendSnapshot(ulong clientId)
                 {
                     var sequence = m_ClientData[clientId].SequenceNumber;
 
+                    // write the tick and sequence header
                     nonNullContext.NetworkWriter.WriteInt32Packed(m_CurrentTick);
                     nonNullContext.NetworkWriter.WriteUInt16(sequence);
 
                     var buffer = (NetworkBuffer)nonNullContext.NetworkWriter.GetStream();
 
                     using (var writer = PooledNetworkWriter.Get(buffer))
                     {
+                        // write the snapshot: buffer, index, spawns, despawns
                         writer.WriteUInt16(SentinelBefore);
                         WriteBuffer(buffer);
                         WriteIndex(buffer);
                         WriteSpawns(buffer, clientId);
                         WriteAcks(buffer, clientId);
                         writer.WriteUInt16(SentinelAfter);
 
+                        m_ClientData[clientId].LastReceivedSequence = 0;
+
+                        // todo: this is incorrect (well, sub-optimal)
+                        // we should still continue ack'ing past messages, in case this one is dropped
+                        m_ClientData[clientId].ReceivedSequenceMask = 0;
                         m_ClientData[clientId].SequenceNumber++;
                     }
                 }
@@ -806,7 +833,9 @@ private void WriteAcks(NetworkBuffer buffer, ulong clientId)
         {
             using (var writer = PooledNetworkWriter.Get(buffer))
             {
+                // todo: revisit whether 16-bit is enough for LastReceivedSequence
                 writer.WriteUInt16(m_ClientData[clientId].LastReceivedSequence);
+                writer.WriteUInt16(m_ClientData[clientId].ReceivedSequenceMask);
             }
         }
 
@@ -911,12 +940,6 @@ private void WriteVariableToSnapshot(Snapshot snapshot, INetworkVariable network
         /// <param name="snapshotStream">The stream to read from</param>
         internal void ReadSnapshot(ulong clientId, Stream snapshotStream)
         {
-            // poor man packet loss simulation
-            //if (Random.Range(0, 10) > 5)
-            //{
-            //    return;
-            //}
-
             // todo: temporary hack around bug
             if (!m_NetworkManager.IsServer)
             {
@@ -937,6 +960,23 @@ internal void ReadSnapshot(ulong clientId, Stream snapshotStream)
                 var sequence = reader.ReadUInt16();
 
                 // todo: check we didn't miss any and deal with gaps
+
+                if (m_ClientData[clientId].ReceivedSequenceMask != 0)
+                {
+                    // since each bit in ReceivedSequenceMask is relative to the last received sequence
+                    // we need to shift all the bits by the difference in sequence
+                    m_ClientData[clientId].ReceivedSequenceMask <<=
+                        (sequence - m_ClientData[clientId].LastReceivedSequence);
+                }
+
+                if (m_ClientData[clientId].LastReceivedSequence != 0)
+                {
+                    // because the bit we're adding for the previous ReceivedSequenceMask
+                    // was implicit, it needs to be shift by one less
+                    m_ClientData[clientId].ReceivedSequenceMask +=
+                        (ushort)(1 << (ushort)((sequence - 1) - m_ClientData[clientId].LastReceivedSequence));
+                }
+
                 m_ClientData[clientId].LastReceivedSequence = sequence;
 
                 var sentinel = reader.ReadUInt16();
@@ -948,10 +988,7 @@ internal void ReadSnapshot(ulong clientId, Stream snapshotStream)
                 m_Snapshot.ReadBuffer(reader, snapshotStream);
                 m_Snapshot.ReadIndex(reader);
                 m_Snapshot.ReadSpawns(reader);
-                var ackSequence = m_Snapshot.ReadAcks(clientId, m_ClientData[clientId], reader);
-
-                // keep track of RTTs, using the sequence number acknowledgement as a marker
-                GetConnectionRtt(clientId).NotifyAck(ackSequence, Time.unscaledTime);
+                m_Snapshot.ReadAcks(clientId, m_ClientData[clientId], reader, GetConnectionRtt(clientId));
 
                 sentinel = reader.ReadUInt16();
                 if (sentinel != SentinelAfter)