Refactor send and receive Orders loop

OpenRA.Game/Game.cs

@@ -19,7 +19,6 @@ using System.Net;
 using System.Reflection;
 using System.Runtime;
 using System.Threading;
-using System.Threading.Tasks;
 using OpenRA.Graphics;
 using OpenRA.Network;
 using OpenRA.Primitives;
@@ -192,8 +191,6 @@ namespace OpenRA
 			Ui.MouseFocusWidget = null;
 			Ui.KeyboardFocusWidget = null;
 
-			OrderManager.LocalFrameNumber = 0;
-			OrderManager.LastTickTime = RunTime;
 			OrderManager.StartGame();
 			worldRenderer.RefreshPalette();
 			Cursor.SetCursor(ChromeMetrics.Get<string>("DefaultCursor"));
@@ -585,49 +582,28 @@ namespace OpenRA
 
 			var world = orderManager.World;
 
-			var uiTickDelta = tick - Ui.LastTickTime;
+			if (Ui.LastTickTime.ShouldAdvance(tick))
-			if (uiTickDelta >= Ui.Timestep)
 			{
-				// Explained below for the world tick calculation
+				Ui.LastTickTime.AdvanceTickTime(tick);
-				var integralTickTimestep = (uiTickDelta / Ui.Timestep) * Ui.Timestep;
-				Ui.LastTickTime += integralTickTimestep >= TimestepJankThreshold ? integralTickTimestep : Ui.Timestep;
-
 				Sync.RunUnsynced(Settings.Debug.SyncCheckUnsyncedCode, world, Ui.Tick);
 				Cursor.Tick();
 			}
 
-			var worldTimestep = world == null ? Ui.Timestep :
+			if (orderManager.LastTickTime.ShouldAdvance(tick))
-				world.IsLoadingGameSave ? 1 :
-				world.IsReplay ? world.ReplayTimestep :
-				world.Timestep;
-
-			var worldTickDelta = tick - orderManager.LastTickTime;
-			if (worldTimestep != 0 && worldTickDelta >= worldTimestep)
 			{
 				using (new PerfSample("tick_time"))
 				{
-					// Tick the world to advance the world time to match real time:
+					orderManager.LastTickTime.AdvanceTickTime(tick);
-					//    If dt < TickJankThreshold then we should try and catch up by repeatedly ticking
-					//    If dt >= TickJankThreshold then we should accept the jank and progress at the normal rate
-					// dt is rounded down to an integer tick count in order to preserve fractional tick components.
-					var integralTickTimestep = (worldTickDelta / worldTimestep) * worldTimestep;
-					orderManager.LastTickTime += integralTickTimestep >= TimestepJankThreshold ? integralTickTimestep : worldTimestep;
 
 					Sound.Tick();
+
 					Sync.RunUnsynced(Settings.Debug.SyncCheckUnsyncedCode, world, orderManager.TickImmediate);
 
 					if (world == null)
 						return;
 
-					var isNetTick = LocalTick % NetTickScale == 0;
+					if (orderManager.TryTick())
-
-					if (!isNetTick || orderManager.IsReadyForNextFrame)
 					{
-						++orderManager.LocalFrameNumber;
-
-						if (isNetTick)
-							orderManager.Tick();
-
 						Sync.RunUnsynced(Settings.Debug.SyncCheckUnsyncedCode, world, () =>
 						{
 							world.OrderGenerator.Tick(world);
@@ -637,8 +613,6 @@ namespace OpenRA
 
 						PerfHistory.Tick();
 					}
-					else if (orderManager.NetFrameNumber == 0)
-						orderManager.LastTickTime = RunTime;
 
 					// Wait until we have done our first world Tick before TickRendering
 					if (orderManager.LocalFrameNumber > 0)

OpenRA.Game/Network/OrderManager.cs

@@ -15,6 +15,7 @@ using System.IO;
 using System.Linq;
 using OpenRA.Primitives;
 using OpenRA.Support;
+using OpenRA.Widgets;
 
 namespace OpenRA.Network
 {
@@ -35,7 +36,7 @@ namespace OpenRA.Network
 		public int LocalFrameNumber;
 		public int FramesAhead = 0;
 
-		public long LastTickTime = Game.RunTime;
+		public TickTime LastTickTime;
 
 		public bool GameStarted => NetFrameNumber != 0;
 		public IConnection Connection { get; private set; }
@@ -84,6 +85,8 @@ namespace OpenRA.Network
 			generateSyncReport = !(Connection is ReplayConnection) && LobbyInfo.GlobalSettings.EnableSyncReports;
 
 			NetFrameNumber = 1;
+			LocalFrameNumber = 0;
+			LastTickTime.Value = Game.RunTime;
 
 			if (GameSaveLastFrame < 0)
 				for (var i = NetFrameNumber; i <= FramesAhead; i++)
@@ -95,6 +98,8 @@ namespace OpenRA.Network
 			Connection = conn;
 			syncReport = new SyncReport(this);
 			AddTextNotification += CacheTextNotification;
+
+			LastTickTime = new TickTime(() => SuggestedTimestep, Game.RunTime);
 		}
 
 		public void IssueOrders(Order[] orders)
@@ -117,12 +122,15 @@ namespace OpenRA.Network
 			notificationsCache.Add(notification);
 		}
 
-		public void TickImmediate()
+		void SendImmediateOrders()
 		{
 			if (localImmediateOrders.Count != 0 && GameSaveLastFrame < NetFrameNumber + FramesAhead)
 				Connection.SendImmediate(localImmediateOrders.Select(o => o.Serialize()));
 			localImmediateOrders.Clear();
+		}
 
+		void ReceiveAllOrdersAndCheckSync()
+		{
 			Connection.Receive(
 				(clientId, packet) =>
 				{
@@ -184,18 +192,39 @@ namespace OpenRA.Network
 				syncForFrame.Add(frame, packet);
 		}
 
-		public bool IsReadyForNextFrame => GameStarted && pendingPackets.All(p => p.Value.Count > 0);
+		bool IsReadyForNextFrame => GameStarted && pendingPackets.All(p => p.Value.Count > 0);
 
-		public void Tick()
+		int SuggestedTimestep
 		{
-			if (!IsReadyForNextFrame)
+			get
-				throw new InvalidOperationException();
+			{
+				if (World == null)
+					return Ui.Timestep;
+
+				if (World.IsLoadingGameSave)
+					return 1;
+
+				if (World.IsReplay)
+					return World.ReplayTimestep;
+
+				return World.Timestep;
+			}
+		}
+
+		void SendOrders()
+		{
+			if (!GameStarted)
+				return;
 
 			if (GameSaveLastFrame < NetFrameNumber + FramesAhead)
+			{
 				Connection.Send(NetFrameNumber + FramesAhead, localOrders.Select(o => o.Serialize()).ToList());
+				localOrders.Clear();
+			}
+		}
 
-			localOrders.Clear();
+		void ProcessOrders()
-
+		{
 			var clientOrders = new List<ClientOrder>();
 
 			foreach (var (clientId, clientPackets) in pendingPackets)
@@ -210,6 +239,7 @@ namespace OpenRA.Network
 				var frameNumber = BitConverter.ToInt32(frameData, 0);
 				if (frameNumber != NetFrameNumber)
 					throw new InvalidDataException($"Attempted to process orders from client {clientId} for frame {frameNumber} on frame {NetFrameNumber}");
+
 				foreach (var order in frameData.ToOrderList(World))
 				{
 					UnitOrders.ProcessOrder(this, World, clientId, order);
@@ -241,5 +271,44 @@ namespace OpenRA.Network
 			disposed = true;
 			Connection?.Dispose();
 		}
+
+		public void TickImmediate()
+		{
+			SendImmediateOrders();
+
+			ReceiveAllOrdersAndCheckSync();
+		}
+
+		public bool TryTick()
+		{
+			var shouldTick = true;
+
+			if (IsNetTick)
+			{
+				// Check whether or not we will be ready for a tick next frame
+				// We don't need to include ourselves in the equation because we can always generate orders this frame
+				shouldTick = pendingPackets.All(p => p.Key == Connection.LocalClientId || p.Value.Count > 0);
+
+				// Send orders only if we are currently ready, this prevents us sending orders too soon if we are
+				// stalling
+				if (shouldTick)
+					SendOrders();
+			}
+
+			var willTick = shouldTick;
+			if (willTick && IsNetTick)
+			{
+				willTick = IsReadyForNextFrame;
+				if (willTick)
+					ProcessOrders();
+			}
+
+			if (willTick)
+				LocalFrameNumber++;
+
+			return willTick;
+		}
+
+		bool IsNetTick => LocalFrameNumber % Game.NetTickScale == 0;
 	}
 }

OpenRA.Game/Network/ReplayConnection.cs

@@ -26,7 +26,7 @@ namespace OpenRA.Network
 		}
 
 		Queue<Chunk> chunks = new Queue<Chunk>();
-		List<byte[]> sync = new List<byte[]>();
+		Queue<byte[]> sync = new Queue<byte[]>();
 
 		readonly int orderLatency;
 		int ordersFrame;
@@ -139,7 +139,7 @@ namespace OpenRA.Network
 			var ms = new MemoryStream(4 + syncData.Length);
 			ms.WriteArray(BitConverter.GetBytes(frame));
 			ms.WriteArray(syncData);
-			sync.Add(ms.GetBuffer());
+			sync.Enqueue(ms.GetBuffer());
 
 			// Store the current frame so Receive() can return the next chunk of orders.
 			ordersFrame = frame + orderLatency;
@@ -148,10 +148,7 @@ namespace OpenRA.Network
 		public void Receive(Action<int, byte[]> packetFn)
 		{
 			while (sync.Count != 0)
-			{
+				packetFn(LocalClientId, sync.Dequeue());
-				packetFn(LocalClientId, sync[0]);
-				sync.RemoveAt(0);
-			}
 
 			while (chunks.Count != 0 && chunks.Peek().Frame <= ordersFrame)
 				foreach (var o in chunks.Dequeue().Packets)

OpenRA.Game/Network/TickTime.cs

@@ -0,0 +1,57 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2021 The OpenRA Developers (see AUTHORS)
+ * This file is part of OpenRA, which is free software. It is made
+ * available to you under the terms of the GNU General Public License
+ * as published by the Free Software Foundation, either version 3 of
+ * the License, or (at your option) any later version. For more
+ * information, see COPYING.
+ */
+#endregion
+
+using System;
+
+namespace OpenRA.Network
+{
+	public sealed class TickTime
+	{
+		readonly Func<int> timestep;
+
+		long lastTickTime;
+
+		public TickTime(Func<int> timestep, long lastTickTime)
+		{
+			this.timestep = timestep;
+			this.lastTickTime = lastTickTime;
+		}
+
+		public long Value
+		{
+			get => lastTickTime;
+			set => lastTickTime = value;
+		}
+
+		public bool ShouldAdvance(long tick)
+		{
+			var i = timestep();
+
+			if (i == 0)
+				return false;
+
+			var tickDelta = tick - lastTickTime;
+			return tickDelta >= i;
+		}
+
+		public void AdvanceTickTime(long tick)
+		{
+			var tickDelta = tick - lastTickTime;
+
+			var currentTimestep = timestep();
+
+			var integralTickTimestep = (tickDelta / currentTimestep) * currentTimestep;
+			lastTickTime += integralTickTimestep >= Game.TimestepJankThreshold
+				? integralTickTimestep
+				: currentTimestep;
+		}
+	}
+}

OpenRA.Game/Server/Server.cs

@@ -42,7 +42,7 @@ namespace OpenRA.Server
 		Dedicated = 2
 	}
 
-	public class Server
+	public sealed class Server
 	{
 		public readonly string TwoHumansRequiredText = "This server requires at least two human players to start a match.";
 
@@ -66,7 +66,7 @@ namespace OpenRA.Server
 		readonly TypeDictionary serverTraits = new TypeDictionary();
 		readonly PlayerDatabase playerDatabase;
 
-		protected volatile ServerState internalState = ServerState.WaitingPlayers;
+		volatile ServerState internalState = ServerState.WaitingPlayers;
 
 		readonly BlockingCollection<IServerEvent> events = new BlockingCollection<IServerEvent>();
 
@@ -77,7 +77,7 @@ namespace OpenRA.Server
 		public ServerState State
 		{
 			get => internalState;
-			protected set => internalState = value;
+			set => internalState = value;
 		}
 
 		public static void SyncClientToPlayerReference(Session.Client c, PlayerReference pr)
@@ -779,7 +779,7 @@ namespace OpenRA.Server
 			RecordOrder(frame, data, from);
 		}
 
-		public void DispatchOrders(Connection conn, int frame, byte[] data)
+		public void ReceiveOrders(Connection conn, int frame, byte[] data)
 		{
 			if (frame == 0)
 				InterpretServerOrders(conn, data);
@@ -1298,7 +1298,7 @@ namespace OpenRA.Server
 
 			void IServerEvent.Invoke(Server server)
 			{
-				server.DispatchOrders(connection, frame, data);
+				server.ReceiveOrders(connection, frame, data);
 			}
 		}
 

OpenRA.Game/Widgets/Widget.cs

@@ -13,6 +13,7 @@ using System;
 using System.Collections.Generic;
 using System.Linq;
 using OpenRA.Graphics;
+using OpenRA.Network;
 using OpenRA.Primitives;
 using OpenRA.Support;
 
@@ -24,7 +25,7 @@ namespace OpenRA.Widgets
 
 		public static Widget Root = new ContainerWidget();
 
-		public static long LastTickTime = Game.RunTime;
+		public static TickTime LastTickTime = new TickTime(() => Timestep, Game.RunTime);
 
 		static readonly Stack<Widget> WindowList = new Stack<Widget>();
 

OpenRA.Mods.Common/Widgets/Logic/Ingame/GameTimerLogic.cs

@@ -24,10 +24,10 @@ namespace OpenRA.Mods.Common.Widgets.Logic
 			var timer = widget.GetOrNull<LabelWidget>("GAME_TIMER");
 			var status = widget.GetOrNull<LabelWidget>("GAME_TIMER_STATUS");
 			var tlm = world.WorldActor.TraitOrDefault<TimeLimitManager>();
-			var startTick = Ui.LastTickTime;
+			var startTick = Ui.LastTickTime.Value;
 
 			Func<bool> shouldShowStatus = () => (world.Paused || world.ReplayTimestep != world.Timestep)
-				&& (Ui.LastTickTime - startTick) / 1000 % 2 == 0;
+				&& (Ui.LastTickTime.Value - startTick) / 1000 % 2 == 0;
 
 			Func<string> statusText = () =>
 			{