Add force ack threshold (#4423)

Robust.Server/GameStates/PvsSystem.cs

@@ -35,10 +35,16 @@ internal sealed partial class PvsSystem : EntitySystem
     public const float ChunkSize = 8;
 
     // TODO make this a cvar. Make it in terms of seconds and tie it to tick rate?
+    // Main issue is that I CBF figuring out the logic for handling it changing mid-game.
     public const int DirtyBufferSize = 20;
     // Note: If a client has ping higher than TickBuffer / TickRate, then the server will treat every entity as if it
     // had entered PVS for the first time. Note that due to the PVS budget, this buffer is easily overwhelmed.
 
+    /// <summary>
+    /// See <see cref="CVars.NetForceAckThreshold"/>.
+    /// </summary>
+    public int ForceAckThreshold { get; private set; }
+
     /// <summary>
     /// Maximum number of pooled objects
     /// </summary>
@@ -139,6 +145,7 @@ internal sealed partial class PvsSystem : EntitySystem
 
         _configManager.OnValueChanged(CVars.NetPVS, SetPvs, true);
         _configManager.OnValueChanged(CVars.NetMaxUpdateRange, OnViewsizeChanged, true);
+        _configManager.OnValueChanged(CVars.NetForceAckThreshold, OnForceAckChanged, true);
 
         _serverGameStateManager.ClientAck += OnClientAck;
         _serverGameStateManager.ClientRequestFull += OnClientRequestFull;
@@ -156,6 +163,7 @@ internal sealed partial class PvsSystem : EntitySystem
 
         _configManager.UnsubValueChanged(CVars.NetPVS, SetPvs);
         _configManager.UnsubValueChanged(CVars.NetMaxUpdateRange, OnViewsizeChanged);
+        _configManager.UnsubValueChanged(CVars.NetForceAckThreshold, OnForceAckChanged);
 
         _serverGameStateManager.ClientAck -= OnClientAck;
         _serverGameStateManager.ClientRequestFull -= OnClientRequestFull;
@@ -211,6 +219,11 @@ internal sealed partial class PvsSystem : EntitySystem
         _viewSize = obj * 2;
     }
 
+    private void OnForceAckChanged(int value)
+    {
+        ForceAckThreshold = value;
+    }
+
     private void SetPvs(bool value)
     {
         _seenAllEnts.Clear();

Robust.Server/GameStates/ServerGameStateManager.cs

@@ -373,6 +373,21 @@ Oldest acked clients: {string.Join(", ", players)}
             // If the state is too big we let Lidgren send it reliably. This is to avoid a situation where a state is so
             // large that it (or part of it) consistently gets dropped. When we send reliably, we immediately update the
             // ack so that the next state will not also be huge.
+            //
+            // We also do this if the client's last ack is too old. This helps prevent things like the entity deletion
+            // history from becoming too bloated if a bad client fails to send acks for whatever reason.
+
+            if (_gameTiming.CurTick.Value > lastAck.Value + _pvs.ForceAckThreshold)
+            {
+                stateUpdateMessage.ForceSendReliably = true;
+
+                // Aside from the time shortly after connecting, this shouldn't be common. If it is happening,
+                // something is probably wrong. If it is more frequent than I think, this can be downgraded to a warning.
+                var connectedTime = (DateTime.UtcNow - session.ConnectedTime).TotalMinutes;
+                if (lastAck > GameTick.Zero && connectedTime > 1)
+                    _logger.Error($"Client {session} exceeded ack-tick threshold. Last ack: {lastAck}. Cur tick: {_gameTiming.CurTick}. Connect time: {connectedTime} minutes");
+            }
+
             if (stateUpdateMessage.ShouldSendReliably())
             {
                 sessionData.LastReceivedAck = _gameTiming.CurTick;

Robust.Shared/CVars.cs

@@ -172,6 +172,13 @@ namespace Robust.Shared
         public static readonly CVarDef<float> NetMaxUpdateRange =
             CVarDef.Create("net.maxupdaterange", 12.5f, CVar.ARCHIVE | CVar.REPLICATED | CVar.SERVER);
 
+        /// <summary>
+        /// Maximum allowed delay between the current tick and a client's last acknowledged tick before we send the
+        /// next game state reliably and simply force update the acked tick,
+        /// </summary>
+        public static readonly CVarDef<int> NetForceAckThreshold =
+            CVarDef.Create("net.force_ack_threshold", 40, CVar.ARCHIVE | CVar.SERVERONLY);
+
         /// <summary>
         /// This limits the number of new entities that can be sent to a client in a single game state. This exists to
         /// avoid stuttering on the client when it has to spawn a bunch of entities in a single tick. If ever entity

Robust.Shared/Network/Messages/MsgState.cs

@@ -95,6 +95,8 @@ namespace Robust.Shared.Network.Messages
             MsgSize = buffer.LengthBytes;
         }
 
+        public bool ForceSendReliably;
+
         /// <summary>
         ///     Whether this state message is large enough to warrant being sent reliably.
         ///     This is only valid after
@@ -103,7 +105,7 @@ namespace Robust.Shared.Network.Messages
         public bool ShouldSendReliably()
         {
             DebugTools.Assert(_hasWritten, "Attempted to determine sending method before determining packet size.");
-            return MsgSize > ReliableThreshold;
+            return ForceSendReliably || MsgSize > ReliableThreshold;
         }
 
         public override NetDeliveryMethod DeliveryMethod