From 418a88e760743138d8790174197bac3311c79d5c Mon Sep 17 00:00:00 2001
From: Leon Friedrich <60421075+ElectroJr@users.noreply.github.com>
Date: Sun, 21 Aug 2022 10:32:09 +1200
Subject: [PATCH] Fixed PVS changes. (#10740)

---
 Content.Client/Entry/EntryPoint.cs            |  3 ++
 .../EscapeMenu/UI/Tabs/NetworkTab.xaml        |  2 +-
 .../EscapeMenu/UI/Tabs/NetworkTab.xaml.cs     | 15 ++++-----
 Content.IntegrationTests/PoolManager.cs       |  1 -
 .../Networking/SimplePredictReconcileTest.cs  | 31 ++++++++++++-------
 Content.Shared/Entry/EntryPoint.cs            |  5 +--
 .../en-US/escape-menu/ui/options-menu.ftl     |  6 +++-
 .../Entities/Structures/Wallmounts/switch.yml |  1 -
 8 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/Content.Client/Entry/EntryPoint.cs b/Content.Client/Entry/EntryPoint.cs
index 17f2f66bb5..83564d7ebd 100644
--- a/Content.Client/Entry/EntryPoint.cs
+++ b/Content.Client/Entry/EntryPoint.cs
@@ -45,6 +45,8 @@ using Robust.Client.Input;
 using Robust.Client.Player;
 using Robust.Client.State;
 using Robust.Client.UserInterface;
+using Robust.Shared;
+using Robust.Shared.Configuration;
 using Robust.Shared.ContentPack;
 using Robust.Shared.GameObjects;
 using Robust.Shared.IoC;
@@ -129,6 +131,7 @@ namespace Content.Client.Entry
             IoCManager.Resolve<PlayTimeTrackingManager>().Initialize();
 
             IoCManager.InjectDependencies(this);
+            IoCManager.Resolve<IConfigurationManager>().OverrideDefault(CVars.NetBufferSize, 2);
 
             _escapeMenuOwner.Initialize();
 
diff --git a/Content.Client/EscapeMenu/UI/Tabs/NetworkTab.xaml b/Content.Client/EscapeMenu/UI/Tabs/NetworkTab.xaml
index 7d2741ab14..1b530fba8e 100644
--- a/Content.Client/EscapeMenu/UI/Tabs/NetworkTab.xaml
+++ b/Content.Client/EscapeMenu/UI/Tabs/NetworkTab.xaml
@@ -8,7 +8,7 @@
                 <Label Text="{Loc 'ui-options-net-interp-ratio'}" />
                 <Control MinSize="8 0" />
                 <Slider Name="NetInterpRatioSlider"
-                            MinValue="0"
+                            ToolTip="{Loc 'ui-options-net-interp-ratio-tooltip'}"
                             MaxValue="6"
                             HorizontalExpand="True"
                             MinSize="80 0"
diff --git a/Content.Client/EscapeMenu/UI/Tabs/NetworkTab.xaml.cs b/Content.Client/EscapeMenu/UI/Tabs/NetworkTab.xaml.cs
index 5a1a16e8c6..aa9533d189 100644
--- a/Content.Client/EscapeMenu/UI/Tabs/NetworkTab.xaml.cs
+++ b/Content.Client/EscapeMenu/UI/Tabs/NetworkTab.xaml.cs
@@ -1,15 +1,10 @@
-using System;
-using Content.Shared.CCVar;
 using Robust.Client.AutoGenerated;
-using Robust.Client.Graphics;
 using Robust.Client.UserInterface;
 using Robust.Client.UserInterface.Controls;
 using Robust.Client.UserInterface.XAML;
 using Robust.Shared;
-using Robust.Shared.Maths;
 using Robust.Shared.Configuration;
-using Robust.Shared.IoC;
-using Robust.Shared.Localization;
+using Robust.Client.GameStates;
 
 namespace Content.Client.EscapeMenu.UI.Tabs
 {
@@ -17,6 +12,7 @@ namespace Content.Client.EscapeMenu.UI.Tabs
     public sealed partial class NetworkTab : Control
     {
         [Dependency] private readonly IConfigurationManager _cfg = default!;
+        [Dependency] private readonly IClientGameStateManager _stateMan = default!;
 
         public NetworkTab()
         {
@@ -26,6 +22,7 @@ namespace Content.Client.EscapeMenu.UI.Tabs
             ApplyButton.OnPressed += OnApplyButtonPressed;
             ResetButton.OnPressed += OnResetButtonPressed;
             NetInterpRatioSlider.OnValueChanged += OnNetInterpRatioSliderChanged;
+            NetInterpRatioSlider.MinValue = _stateMan.MinBufferSize;
 
             Reset();
         }
@@ -45,7 +42,7 @@ namespace Content.Client.EscapeMenu.UI.Tabs
 
         private void OnApplyButtonPressed(BaseButton.ButtonEventArgs args)
         {
-            _cfg.SetCVar(CVars.NetInterpRatio, (int) NetInterpRatioSlider.Value);
+            _cfg.SetCVar(CVars.NetBufferSize, (int) NetInterpRatioSlider.Value - _stateMan.MinBufferSize);
             _cfg.SaveToFile();
             UpdateChanges();
         }
@@ -57,13 +54,13 @@ namespace Content.Client.EscapeMenu.UI.Tabs
 
         private void Reset()
         {
-            NetInterpRatioSlider.Value = _cfg.GetCVar(CVars.NetInterpRatio);
+            NetInterpRatioSlider.Value = _cfg.GetCVar(CVars.NetBufferSize) + _stateMan.MinBufferSize;
             UpdateChanges();
         }
 
         private void UpdateChanges()
         {
-            var isEverythingSame = NetInterpRatioSlider.Value == _cfg.GetCVar(CVars.NetInterpRatio);
+            var isEverythingSame = NetInterpRatioSlider.Value == _cfg.GetCVar(CVars.NetBufferSize) + _stateMan.MinBufferSize;
             ApplyButton.Disabled = isEverythingSame;
             ResetButton.Disabled = isEverythingSame;
             NetInterpRatioLabel.Text = NetInterpRatioSlider.Value.ToString();
diff --git a/Content.IntegrationTests/PoolManager.cs b/Content.IntegrationTests/PoolManager.cs
index a43f480cec..39f699b38f 100644
--- a/Content.IntegrationTests/PoolManager.cs
+++ b/Content.IntegrationTests/PoolManager.cs
@@ -45,7 +45,6 @@ public static class PoolManager
         (CCVars.GameMapForced.Name, "true", true),
         (CCVars.AdminLogsQueueSendDelay.Name, "0", true),
         (CCVars.NetPVS.Name, "false", true),
-        (CCVars.NetInterp.Name, "false", true),
         (CCVars.NPCMaxUpdates.Name, "999999", true),
         (CCVars.GameMapForced.Name, "true", true),
         (CCVars.SysWinTickPeriod.Name, "0", true),
diff --git a/Content.IntegrationTests/Tests/Networking/SimplePredictReconcileTest.cs b/Content.IntegrationTests/Tests/Networking/SimplePredictReconcileTest.cs
index 63a0d4d35d..869be4c4fe 100644
--- a/Content.IntegrationTests/Tests/Networking/SimplePredictReconcileTest.cs
+++ b/Content.IntegrationTests/Tests/Networking/SimplePredictReconcileTest.cs
@@ -6,8 +6,11 @@ using System.Threading.Tasks;
 using NUnit.Framework;
 using Robust.Client.GameObjects;
 using Robust.Client.GameStates;
+using Robust.Client.Timing;
 using Robust.Server.GameStates;
 using Robust.Server.Player;
+using Robust.Shared;
+using Robust.Shared.Configuration;
 using Robust.Shared.GameObjects;
 using Robust.Shared.GameStates;
 using Robust.Shared.IoC;
@@ -35,7 +38,7 @@ namespace Content.IntegrationTests.Tests.Networking
         [Test]
         public async Task Test()
         {
-            await using var pairTracker = await PoolManager.GetServerClient(new (){Fresh = true, DisableInterpolate = true, DummyTicker = true});
+            await using var pairTracker = await PoolManager.GetServerClient(new (){Fresh = true, DummyTicker = true});
             var server = pairTracker.Pair.Server;
             var client = pairTracker.Pair.Client;
 
@@ -45,8 +48,12 @@ namespace Content.IntegrationTests.Tests.Networking
             var sEntityManager = server.ResolveDependency<IEntityManager>();
             var cEntityManager = client.ResolveDependency<IEntityManager>();
             var sGameTiming = server.ResolveDependency<IGameTiming>();
-            var cGameTiming = client.ResolveDependency<IGameTiming>();
+            var cGameTiming = client.ResolveDependency<IClientGameTiming>();
             var cGameStateManager = client.ResolveDependency<IClientGameStateManager>();
+            var cfg = client.ResolveDependency<IConfigurationManager>();
+            var log = cfg.GetCVar(CVars.NetLogging);
+
+            //cfg.SetCVar(CVars.NetLogging, true);
 
             EntityUid serverEnt = default;
             PredictionTestComponent serverComponent = default!;
@@ -72,14 +79,12 @@ namespace Content.IntegrationTests.Tests.Networking
             // Run some ticks so that
             await PoolManager.RunTicksSync(pairTracker.Pair, 3);
 
-            // Due to technical things with the game state processor it has an extra state in the buffer here.
-            // This burns through it real quick, but I'm not sure it should be there?
-            // Under normal operation (read: not integration test) this gets corrected for via tick time adjustment,
-            // so it's probably not an issue?
-            await client.WaitRunTicks(1);
+            // Check client buffer is full
+            Assert.That(cGameStateManager.CurrentBufferSize, Is.EqualTo(cGameStateManager.TargetBufferSize));
 
-            // 2 is target buffer size.
-            Assert.That(cGameStateManager.CurrentBufferSize, Is.EqualTo(2));
+            // This isn't required anymore, but the test had this for the sake of "technical things", and I cbf shifting
+            // all the tick times over. So it stays.
+            await client.WaitRunTicks(1);
 
             await client.WaitPost(() =>
             {
@@ -97,7 +102,7 @@ namespace Content.IntegrationTests.Tests.Networking
 
             // Client last ran tick 15 meaning it's ahead of the last server tick it processed (12)
             Assert.That(cGameTiming.CurTick, Is.EqualTo(new GameTick(16)));
-            Assert.That(cGameStateManager.CurServerTick, Is.EqualTo(new GameTick(12)));
+            Assert.That(cGameTiming.LastProcessedTick, Is.EqualTo(new GameTick(12)));
 
             // *** I am using block scopes to visually distinguish these sections of the test to make it more readable.
 
@@ -173,7 +178,7 @@ namespace Content.IntegrationTests.Tests.Networking
             // Assert timing is still correct, should be but it's a good reference for the rest of the test.
             Assert.That(sGameTiming.CurTick, Is.EqualTo(new GameTick(18)));
             Assert.That(cGameTiming.CurTick, Is.EqualTo(new GameTick(20)));
-            Assert.That(cGameStateManager.CurServerTick, Is.EqualTo(new GameTick(16)));
+            Assert.That(cGameTiming.LastProcessedTick, Is.EqualTo(new GameTick(16)));
 
             {
                 // Send event to server to change flag again, this time to disable it..
@@ -244,7 +249,7 @@ namespace Content.IntegrationTests.Tests.Networking
             // Assert timing is still correct.
             Assert.That(sGameTiming.CurTick, Is.EqualTo(new GameTick(22)));
             Assert.That(cGameTiming.CurTick, Is.EqualTo(new GameTick(24)));
-            Assert.That(cGameStateManager.CurServerTick, Is.EqualTo(new GameTick(20)));
+            Assert.That(cGameTiming.LastProcessedTick, Is.EqualTo(new GameTick(20)));
 
             {
                 // Send first event to disable the flag (reminder: it never got accepted by the server).
@@ -359,6 +364,8 @@ namespace Content.IntegrationTests.Tests.Networking
                     Assert.That(clientComponent.Foo, Is.True);
                 }
             }
+            
+            cfg.SetCVar(CVars.NetLogging, log);
             await pairTracker.CleanReturnAsync();
         }
 
diff --git a/Content.Shared/Entry/EntryPoint.cs b/Content.Shared/Entry/EntryPoint.cs
index 264b5f8a7e..aed1da13b8 100644
--- a/Content.Shared/Entry/EntryPoint.cs
+++ b/Content.Shared/Entry/EntryPoint.cs
@@ -39,9 +39,7 @@ namespace Content.Shared.Entry
             IoCManager.Resolve<MarkingManager>().Initialize();
 
             var configMan = IoCManager.Resolve<IConfigurationManager>();
-#if FULL_RELEASE
-            configMan.OverrideDefault(CVars.NetInterpRatio, 2);
-#elif DEBUG
+#if DEBUG
             configMan.OverrideDefault(CVars.NetFakeLagMin, 0.075f);
             configMan.OverrideDefault(CVars.NetFakeLoss, 0.005f);
             configMan.OverrideDefault(CVars.NetFakeDuplicates, 0.005f);
@@ -50,7 +48,6 @@ namespace Content.Shared.Entry
             // just leaving this disabled.
             // configMan.OverrideDefault(CVars.NetFakeLagRand, 0.01f);
 #endif
-
         }
 
         private void InitTileDefinitions()
diff --git a/Resources/Locale/en-US/escape-menu/ui/options-menu.ftl b/Resources/Locale/en-US/escape-menu/ui/options-menu.ftl
index 0902186943..1ebbd51992 100644
--- a/Resources/Locale/en-US/escape-menu/ui/options-menu.ftl
+++ b/Resources/Locale/en-US/escape-menu/ui/options-menu.ftl
@@ -169,4 +169,8 @@ ui-options-function-shuttle-brake = Brake
 
 ## Network menu
 
-ui-options-net-interp-ratio = Network Smoothing
+ui-options-net-interp-ratio = State buffer size
+ui-options-net-interp-ratio-tooltip = Increasing this will generally make the game
+                                      more resistant to packet-loss, however in doing
+                                      so it effectively adds slightly more latency and
+                                      requires the client to predict more future ticks.
diff --git a/Resources/Prototypes/Entities/Structures/Wallmounts/switch.yml b/Resources/Prototypes/Entities/Structures/Wallmounts/switch.yml
index 5dfb21d0da..b497f5a26e 100644
--- a/Resources/Prototypes/Entities/Structures/Wallmounts/switch.yml
+++ b/Resources/Prototypes/Entities/Structures/Wallmounts/switch.yml
@@ -130,7 +130,6 @@
     - type: Construction
       graph: LeverGraph
       node: LeverNode
-    - type: Fixtures
     - type: SignalTransmitter
       outputs:
         Left: []