fix: mtt-1088 review. Safer handling of out-of-order or old messages (Unity-Technologies#1091)

* fix: Addressing latest review comments, making it safer when receiving OOO messages or old messages

* fix: Addressing latest review comments, whitespace

* fix: removing extraneous cast to ushort

Author: jeffreyrainy

Runtime/Core/SnapshotSystem.cs

@@ -959,26 +959,45 @@ internal void ReadSnapshot(ulong clientId, Stream snapshotStream)
                 snapshotTick = reader.ReadInt32Packed();
                 var sequence = reader.ReadUInt16();
 
-                // todo: check we didn't miss any and deal with gaps
-
-                if (m_ClientData[clientId].ReceivedSequenceMask != 0)
+                if (sequence >= m_ClientData[clientId].LastReceivedSequence)
                 {
-                    // 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].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
+                        var shift = sequence - m_ClientData[clientId].LastReceivedSequence;
+                        if (shift < sizeof(ushort) * 8)
+                        {
+                            m_ClientData[clientId].ReceivedSequenceMask <<= shift;
+                        }
+                        else
+                        {
+                            m_ClientData[clientId].ReceivedSequenceMask = 0;
+                        }
+                    }
 
-                if (m_ClientData[clientId].LastReceivedSequence != 0)
+                    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
+                        var shift = sequence - 1 - m_ClientData[clientId].LastReceivedSequence;
+                        if (shift < sizeof(ushort) * 8)
+                        {
+                            m_ClientData[clientId].ReceivedSequenceMask |= (ushort)(1 << shift);
+                        }
+                    }
+
+                    m_ClientData[clientId].LastReceivedSequence = sequence;
+                }
+                else
                 {
-                    // 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));
+                    // todo: Missing: dealing with out-of-order message acknowledgments
+                    // we should set m_ClientData[clientId].ReceivedSequenceMask accordingly
+                    // testing this will require a way to reorder SnapshotMessages, which we lack at the moment
+                    //
+                    // without this, we incur extra retransmit, not a catastrophic failure
                 }
 
-                m_ClientData[clientId].LastReceivedSequence = sequence;
-
                 var sentinel = reader.ReadUInt16();
                 if (sentinel != SentinelBefore)
                 {