From df798fb620ada1f678d48918ab10dbab4f571bdc Mon Sep 17 00:00:00 2001 From: Paul Chote Date: Mon, 20 Sep 2021 22:44:49 +0100 Subject: [PATCH] Overhaul client latency calculations. The ping/pong orders are replaced with a dedicated (and much smaller) Ping packet that is handled directly in the client and server Connection wrappers. This allows clients to respond when the orders are processed, instead of queuing the pong order to be sent in the next frame (which added an extra 120ms of unwanted latency). The ping frequency has been raised to 1Hz, and pings are now routed through the server events queue in preparation for the future dynamic latency system. The raw ping numbers are no longer sent to clients, the server instead evaluates a single ConnectionQuality value that in the future may be based on more than just the ping times. --- OpenRA.Game/Network/Connection.cs | 8 ++ OpenRA.Game/Network/Order.cs | 1 + OpenRA.Game/Network/OrderIO.cs | 21 ++++ OpenRA.Game/Network/Session.cs | 34 +----- OpenRA.Game/Network/UnitOrders.cs | 18 ++- OpenRA.Game/Server/Connection.cs | 39 +++++- OpenRA.Game/Server/ProtocolVersion.cs | 4 +- OpenRA.Game/Server/Server.cs | 114 +++++++++--------- .../ServerTraits/LobbyCommands.cs | 14 --- .../ServerTraits/PlayerPinger.cs | 12 +- .../Logic/Lobby/LatencyTooltipLogic.cs | 5 +- .../Widgets/Logic/Lobby/LobbyUtils.cs | 41 +++---- 12 files changed, 161 insertions(+), 150 deletions(-) diff --git a/OpenRA.Game/Network/Connection.cs b/OpenRA.Game/Network/Connection.cs index 99d73b5f66..1de67d2edb 100644 --- a/OpenRA.Game/Network/Connection.cs +++ b/OpenRA.Game/Network/Connection.cs @@ -304,6 +304,14 @@ namespace OpenRA.Network orderManager.ReceiveDisconnect(disconnect.ClientId, disconnect.Frame); else if (OrderIO.TryParseSync(p.Data, out var sync)) orderManager.ReceiveSync(sync); + else if (OrderIO.TryParsePing(p.FromClient, p.Data, out var ping)) + { + // The Ping packet is sent back directly without changes + // Note that processing this here, rather than in NetworkConnectionReceive, + // so that poor world tick performance can be reflected in the latency measurement + Send(ping); + record = false; + } else if (OrderIO.TryParseAck(p, out var ackFrame)) { if (!sentOrders.TryDequeue(out var q)) diff --git a/OpenRA.Game/Network/Order.cs b/OpenRA.Game/Network/Order.cs index edad793cf7..f718ccd5b9 100644 --- a/OpenRA.Game/Network/Order.cs +++ b/OpenRA.Game/Network/Order.cs @@ -19,6 +19,7 @@ namespace OpenRA public enum OrderType : byte { Ack = 0x10, + Ping = 0x20, SyncHash = 0x65, Disconnect = 0xBF, Handshake = 0xFE, diff --git a/OpenRA.Game/Network/OrderIO.cs b/OpenRA.Game/Network/OrderIO.cs index 8fe0e9666d..483d26dd47 100644 --- a/OpenRA.Game/Network/OrderIO.cs +++ b/OpenRA.Game/Network/OrderIO.cs @@ -109,6 +109,27 @@ namespace OpenRA.Network return true; } + public static bool TryParsePing(int fromClient, byte[] packet, out byte[] ping) + { + // Valid Ping packets are only ever generated by the server + if (fromClient != 0 || packet.Length != 13 || packet[4] != (byte)OrderType.Ping) + { + ping = null; + return false; + } + + // Valid Ping packets always have frame 0 + var frame = BitConverter.ToInt32(packet, 0); + if (frame != 0) + { + ping = null; + return false; + } + + ping = packet; + return true; + } + public static bool TryParseAck((int FromClient, byte[] Data) packet, out int frame) { // Ack packets are only accepted from the server diff --git a/OpenRA.Game/Network/Session.cs b/OpenRA.Game/Network/Session.cs index a5ef0dd492..64dff89ba7 100644 --- a/OpenRA.Game/Network/Session.cs +++ b/OpenRA.Game/Network/Session.cs @@ -22,7 +22,6 @@ namespace OpenRA.Network public class Session { public List Clients = new List(); - public List ClientPings = new List(); // Keyed by the PlayerReference id that the slot corresponds to public Dictionary Slots = new Dictionary(); @@ -60,10 +59,6 @@ namespace OpenRA.Network session.Clients.Add(Client.Deserialize(node.Value)); break; - case "ClientPing": - session.ClientPings.Add(ClientPing.Deserialize(node.Value)); - break; - case "GlobalSettings": session.GlobalSettings = Global.Deserialize(node.Value); break; @@ -122,6 +117,8 @@ namespace OpenRA.Network public enum ClientState { NotReady, Invalid, Ready, Disconnected = 1000 } + public enum ConnectionQuality { Good, Moderate, Poor } + public class Client { public static Client Deserialize(MiniYaml data) @@ -142,6 +139,7 @@ namespace OpenRA.Network public string IPAddress; public string AnonymizedIPAddress; public string Location; + public ConnectionQuality ConnectionQuality = ConnectionQuality.Good; public ClientState State = ClientState.Invalid; public int Team; @@ -164,29 +162,6 @@ namespace OpenRA.Network } } - public ClientPing PingFromClient(Client client) - { - return ClientPings.SingleOrDefault(p => p.Index == client.Index); - } - - public class ClientPing - { - public int Index; - public long Latency = -1; - public long LatencyJitter = -1; - public long[] LatencyHistory = { }; - - public static ClientPing Deserialize(MiniYaml data) - { - return FieldLoader.Load(data); - } - - public MiniYamlNode Serialize() - { - return new MiniYamlNode($"ClientPing@{Index}", FieldSaver.Save(this)); - } - } - public class Slot { public string PlayerReference; // PlayerReference to bind against. @@ -293,9 +268,6 @@ namespace OpenRA.Network foreach (var client in Clients) sessionData.Add(client.Serialize()); - foreach (var clientPing in ClientPings) - sessionData.Add(clientPing.Serialize()); - foreach (var slot in Slots) sessionData.Add(slot.Value.Serialize()); diff --git a/OpenRA.Game/Network/UnitOrders.cs b/OpenRA.Game/Network/UnitOrders.cs index 6d97171eaa..bd9fea2d3b 100644 --- a/OpenRA.Game/Network/UnitOrders.cs +++ b/OpenRA.Game/Network/UnitOrders.cs @@ -292,24 +292,20 @@ namespace OpenRA.Network break; } - case "SyncClientPings": + case "SyncConnectionQuality": { - var pings = new List(); var nodes = MiniYaml.FromString(order.TargetString); foreach (var node in nodes) { var strings = node.Key.Split('@'); - if (strings[0] == "ClientPing") - pings.Add(Session.ClientPing.Deserialize(node.Value)); + if (strings[0] == "ConnectionQuality") + { + var client = orderManager.LobbyInfo.Clients.FirstOrDefault(c => c.Index == int.Parse(strings[1])); + if (client != null) + client.ConnectionQuality = FieldLoader.GetValue("ConnectionQuality", node.Value.Value); + } } - orderManager.LobbyInfo.ClientPings = pings; - break; - } - - case "Ping": - { - orderManager.IssueOrder(Order.FromTargetString("Pong", order.TargetString, true)); break; } diff --git a/OpenRA.Game/Server/Connection.cs b/OpenRA.Game/Server/Connection.cs index 9aa203455e..5ced0c3c96 100644 --- a/OpenRA.Game/Server/Connection.cs +++ b/OpenRA.Game/Server/Connection.cs @@ -12,6 +12,8 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics; +using System.IO; using System.Linq; using System.Net; using System.Net.Sockets; @@ -23,6 +25,9 @@ namespace OpenRA.Server { public const int MaxOrderLength = 131072; + // Cap ping history at 15 seconds as a balance between expiring stale state and having enough data for decent statistics + const int MaxPingSamples = 15; + public readonly int PlayerIndex; public readonly string AuthToken; public readonly EndPoint EndPoint; @@ -36,6 +41,7 @@ namespace OpenRA.Server long lastReceivedTime = 0; readonly BlockingCollection sendQueue = new BlockingCollection(); + readonly Queue pingHistory = new Queue(); public Connection(Server server, Socket socket, string authToken) { @@ -50,6 +56,17 @@ namespace OpenRA.Server }.Start((server, socket)); } + static byte[] CreatePingFrame() + { + var ms = new MemoryStream(21); + ms.WriteArray(BitConverter.GetBytes(13)); + ms.WriteArray(BitConverter.GetBytes(0)); + ms.WriteArray(BitConverter.GetBytes(0)); + ms.WriteByte((byte)OrderType.Ping); + ms.WriteArray(BitConverter.GetBytes(Game.RunTime)); + return ms.GetBuffer(); + } + void SendReceiveLoop(object s) { var (server, socket) = (ValueTuple)s; @@ -61,6 +78,7 @@ namespace OpenRA.Server var state = ReceiveState.Header; var expectLength = 8; var frame = 0; + var lastPingSent = Stopwatch.StartNew(); try { @@ -107,7 +125,19 @@ namespace OpenRA.Server case ReceiveState.Data: { - server.OnConnectionPacket(this, frame, bytes); + // Ping packets are sent and processed internally within this thread to reduce + // server-introduced latencies from polling loops + if (expectLength == 9 && bytes[0] == (byte)OrderType.Ping) + { + if (pingHistory.Count == MaxPingSamples) + pingHistory.Dequeue(); + + pingHistory.Enqueue((int)(Game.RunTime - BitConverter.ToInt64(bytes, 1))); + server.OnConnectionPing(this, pingHistory.ToArray()); + } + else + server.OnConnectionPacket(this, frame, bytes); + expectLength = 8; state = ReceiveState.Header; @@ -121,6 +151,13 @@ namespace OpenRA.Server if (sendQueue.IsCompleted) return; + // Regularly check player ping + if (lastPingSent.ElapsedMilliseconds > 1000) + { + sendQueue.Add(CreatePingFrame()); + lastPingSent.Restart(); + } + // Send all data immediately, we will block again on read while (sendQueue.TryTake(out var data, 0)) { diff --git a/OpenRA.Game/Server/ProtocolVersion.cs b/OpenRA.Game/Server/ProtocolVersion.cs index 6c0ab18f5d..3e75a2a115 100644 --- a/OpenRA.Game/Server/ProtocolVersion.cs +++ b/OpenRA.Game/Server/ProtocolVersion.cs @@ -38,6 +38,8 @@ namespace OpenRA.Server // - Order-specific data - see OpenRA.Game/Server/Order.cs for details // - 0x10: Order acknowledgement (sent from the server to a client in response to a packet with world orders) // - Int32 containing the frame number that the client should apply the orders it sent + // - 0x20: Ping + // - Int64 containing the server timestamp when the ping was generated // // A connection handshake begins when a client opens a connection to the server: // - Server sends: @@ -71,6 +73,6 @@ namespace OpenRA.Server // The protocol for server and world orders // This applies after the handshake has completed, and is provided to support // alternative server implementations that wish to support multiple versions in parallel - public const int Orders = 15; + public const int Orders = 16; } } diff --git a/OpenRA.Game/Server/Server.cs b/OpenRA.Game/Server/Server.cs index 3574ed7a4b..1682946d76 100644 --- a/OpenRA.Game/Server/Server.cs +++ b/OpenRA.Game/Server/Server.cs @@ -12,6 +12,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.IO; using System.Linq; @@ -76,6 +77,7 @@ namespace OpenRA.Server ReplayRecorder recorder; GameInformation gameInfo; readonly List worldPlayers = new List(); + Stopwatch pingUpdated = Stopwatch.StartNew(); public ServerState State { @@ -321,6 +323,11 @@ namespace OpenRA.Server events.Add(new ConnectionPacketEvent(conn, frame, data)); } + internal void OnConnectionPing(Connection conn, int[] pingHistory) + { + events.Add(new ConnectionPingEvent(conn, pingHistory)); + } + internal void OnConnectionDisconnect(Connection conn) { events.Add(new ConnectionDisconnectEvent(conn)); @@ -469,9 +476,6 @@ namespace OpenRA.Server LobbyInfo.Clients.Add(client); newConn.Validated = true; - var clientPing = new Session.ClientPing { Index = client.Index }; - LobbyInfo.ClientPings.Add(clientPing); - Log.Write("server", "Client {0}: Accepted connection from {1}.", newConn.PlayerIndex, newConn.EndPoint); if (client.Fingerprint != null) @@ -487,9 +491,6 @@ namespace OpenRA.Server if (Type != ServerType.Local) SendMessage($"{client.Name} has joined the game."); - // Send initial ping - SendOrderTo(newConn, "Ping", Game.RunTime.ToString(CultureInfo.InvariantCulture)); - if (Type == ServerType.Dedicated) { var motdFile = Path.Combine(Platform.SupportDir, "motd.txt"); @@ -893,37 +894,6 @@ namespace OpenRA.Server case "Chat": DispatchOrdersToClients(conn, 0, o.Serialize()); break; - case "Pong": - { - if (!OpenRA.Exts.TryParseInt64Invariant(o.TargetString, out var pingSent)) - { - Log.Write("server", "Invalid order pong payload: {0}", o.TargetString); - break; - } - - var client = GetClient(conn); - if (client == null) - return; - - var pingFromClient = LobbyInfo.PingFromClient(client); - if (pingFromClient == null) - return; - - var history = pingFromClient.LatencyHistory.ToList(); - history.Add(Game.RunTime - pingSent); - - // Cap ping history at 5 values (25 seconds) - if (history.Count > 5) - history.RemoveRange(0, history.Count - 5); - - pingFromClient.Latency = history.Sum() / history.Count; - pingFromClient.LatencyJitter = (history.Max() - history.Min()) / 2; - pingFromClient.LatencyHistory = history.ToArray(); - - SyncClientPing(); - - break; - } case "GameSaveTraitData": { @@ -995,12 +965,7 @@ namespace OpenRA.Server foreach (var c in LobbyInfo.Clients) { if (c.Bot != null) - { LobbyInfo.Clients.Remove(c); - var ping = LobbyInfo.PingFromClient(c); - if (ping != null) - LobbyInfo.ClientPings.Remove(ping); - } else c.Slot = null; } @@ -1033,7 +998,6 @@ namespace OpenRA.Server SyncLobbyInfo(); SyncLobbyClients(); - SyncClientPing(); break; } @@ -1041,6 +1005,36 @@ namespace OpenRA.Server } } + public void ReceivePing(Connection conn, int[] pingHistory) + { + // Levels set relative to the default order lag of 3 net ticks (360ms) + // TODO: Adjust this once dynamic lag is implemented + var latency = pingHistory.Sum() / pingHistory.Length; + + var quality = latency < 240 ? Session.ConnectionQuality.Good : + latency < 360 ? Session.ConnectionQuality.Moderate : + Session.ConnectionQuality.Poor; + + lock (LobbyInfo) + { + foreach (var c in LobbyInfo.Clients) + if (c.Index == conn.PlayerIndex || (c.Bot != null && c.BotControllerClientIndex == conn.PlayerIndex)) + c.ConnectionQuality = quality; + + // Update ping without forcing a full update + // Note that syncing pings doesn't trigger INotifySyncLobbyInfo + if (pingUpdated.ElapsedMilliseconds > 5000) + { + var nodes = new List(); + foreach (var c in LobbyInfo.Clients) + nodes.Add(new MiniYamlNode($"ConnectionQuality@{c.Index}", FieldSaver.FormatValue(c.ConnectionQuality))); + + DispatchServerOrdersToClients(Order.FromTargetString("SyncConnectionQuality", nodes.WriteToString(), true)); + pingUpdated.Restart(); + } + } + } + public Session.Client GetClient(Connection conn) { if (conn == null) @@ -1068,7 +1062,6 @@ namespace OpenRA.Server SendMessage($"{dropClient.Name}{suffix} has disconnected."); LobbyInfo.Clients.RemoveAll(c => c.Index == toDrop.PlayerIndex); - LobbyInfo.ClientPings.RemoveAll(p => p.Index == toDrop.PlayerIndex); // Client was the server admin // TODO: Reassign admin for game in progress via an order @@ -1137,6 +1130,10 @@ namespace OpenRA.Server foreach (var t in serverTraits.WithInterface()) t.LobbyInfoSynced(this); + + // The full LobbyInfo includes ping info, so we can delay the next partial ping update + // TODO: Replace the special-case ping updates with more general LobbyInfo delta updates + pingUpdated.Restart(); } } @@ -1173,18 +1170,6 @@ namespace OpenRA.Server } } - public void SyncClientPing() - { - lock (LobbyInfo) - { - // TODO: Split this further into per client ping orders - var clientPings = LobbyInfo.ClientPings.Select(ping => ping.Serialize()).ToList(); - - // Note that syncing pings doesn't trigger INotifySyncLobbyInfo - DispatchServerOrdersToClients(Order.FromTargetString("SyncClientPings", clientPings.WriteToString(), true)); - } - } - public void StartGame() { lock (LobbyInfo) @@ -1362,6 +1347,23 @@ namespace OpenRA.Server } } + class ConnectionPingEvent : IServerEvent + { + readonly Connection connection; + readonly int[] pingHistory; + + public ConnectionPingEvent(Connection connection, int[] pingHistory) + { + this.connection = connection; + this.pingHistory = pingHistory; + } + + void IServerEvent.Invoke(Server server) + { + server.ReceivePing(connection, pingHistory); + } + } + class CallbackEvent : IServerEvent { readonly Action action; diff --git a/OpenRA.Mods.Common/ServerTraits/LobbyCommands.cs b/OpenRA.Mods.Common/ServerTraits/LobbyCommands.cs index 5c19690c47..fd4760c4ab 100644 --- a/OpenRA.Mods.Common/ServerTraits/LobbyCommands.cs +++ b/OpenRA.Mods.Common/ServerTraits/LobbyCommands.cs @@ -272,12 +272,6 @@ namespace OpenRA.Mods.Common.Server { server.LobbyInfo.Clients.Remove(occupant); server.SyncLobbyClients(); - var ping = server.LobbyInfo.PingFromClient(occupant); - if (ping != null) - { - server.LobbyInfo.ClientPings.Remove(ping); - server.SyncClientPing(); - } } else { @@ -311,15 +305,7 @@ namespace OpenRA.Mods.Common.Server // Slot may have a bot in it var occupant = server.LobbyInfo.ClientInSlot(s); if (occupant != null && occupant.Bot != null) - { server.LobbyInfo.Clients.Remove(occupant); - var ping = server.LobbyInfo.PingFromClient(occupant); - if (ping != null) - { - server.LobbyInfo.ClientPings.Remove(ping); - server.SyncClientPing(); - } - } server.SyncLobbyClients(); diff --git a/OpenRA.Mods.Common/ServerTraits/PlayerPinger.cs b/OpenRA.Mods.Common/ServerTraits/PlayerPinger.cs index 668cab5619..70264a08ce 100644 --- a/OpenRA.Mods.Common/ServerTraits/PlayerPinger.cs +++ b/OpenRA.Mods.Common/ServerTraits/PlayerPinger.cs @@ -21,9 +21,6 @@ namespace OpenRA.Mods.Common.Server static readonly int ConnReportInterval = 20000; // Report every 20 seconds static readonly int ConnTimeout = 60000; // Drop unresponsive clients after 60 seconds - // TickTimeout is in microseconds - public int TickTimeout => PingInterval * 100; - long lastPing = 0; long lastConnReport = 0; bool isInitialPing = true; @@ -40,13 +37,7 @@ namespace OpenRA.Mods.Common.Server lock (server.LobbyInfo) nonBotClientCount = server.LobbyInfo.NonBotClients.Count(); - if (nonBotClientCount < 2 && server.Type != ServerType.Dedicated) - { - foreach (var c in server.Conns.ToList()) - if (c.Validated) - server.SendOrderTo(c, "Ping", Game.RunTime.ToString()); - } - else + if (nonBotClientCount >= 2 || server.Type == ServerType.Dedicated) { foreach (var c in server.Conns.ToList()) { @@ -63,7 +54,6 @@ namespace OpenRA.Mods.Common.Server if (c.TimeSinceLastResponse < ConnTimeout) { - server.SendOrderTo(c, "Ping", Game.RunTime.ToString()); if (!c.TimeoutMessageShown && c.TimeSinceLastResponse > PingInterval * 2) { server.SendMessage(client.Name + " is experiencing connection problems."); diff --git a/OpenRA.Mods.Common/Widgets/Logic/Lobby/LatencyTooltipLogic.cs b/OpenRA.Mods.Common/Widgets/Logic/Lobby/LatencyTooltipLogic.cs index 727456690f..6215bc4a54 100644 --- a/OpenRA.Mods.Common/Widgets/Logic/Lobby/LatencyTooltipLogic.cs +++ b/OpenRA.Mods.Common/Widgets/Logic/Lobby/LatencyTooltipLogic.cs @@ -34,9 +34,8 @@ namespace OpenRA.Mods.Common.Widgets.Logic widget.Bounds.Width = latency.Bounds.X + latencyFont.Measure(latency.GetText()).X + rightMargin; }; - var ping = orderManager.LobbyInfo.PingFromClient(client); - latency.GetText = () => LobbyUtils.LatencyDescription(ping); - latency.GetColor = () => LobbyUtils.LatencyColor(ping); + latency.GetText = () => LobbyUtils.LatencyDescription(client); + latency.GetColor = () => LobbyUtils.LatencyColor(client); } } } diff --git a/OpenRA.Mods.Common/Widgets/Logic/Lobby/LobbyUtils.cs b/OpenRA.Mods.Common/Widgets/Logic/Lobby/LobbyUtils.cs index 5c257c5ed8..c28ccd08ed 100644 --- a/OpenRA.Mods.Common/Widgets/Logic/Lobby/LobbyUtils.cs +++ b/OpenRA.Mods.Common/Widgets/Logic/Lobby/LobbyUtils.cs @@ -306,34 +306,32 @@ namespace OpenRA.Mods.Common.Widgets.Logic return AvailableSpawnPoints(spawnPoints, lobbyInfo).Count < lobbyInfo.Clients.Count(c => !c.IsObserver); } - public static Color LatencyColor(Session.ClientPing ping) + public static Color LatencyColor(Session.Client client) { - if (ping == null) + if (client == null) return Color.Gray; - // Levels set relative to the default order lag of 3 net ticks (360ms) - // TODO: Adjust this once dynamic lag is implemented - if (ping.Latency < 0) - return Color.Gray; - if (ping.Latency < 300) - return Color.LimeGreen; - if (ping.Latency < 600) - return Color.Orange; - return Color.Red; + switch (client.ConnectionQuality) + { + case Session.ConnectionQuality.Good: return Color.LimeGreen; + case Session.ConnectionQuality.Moderate: return Color.Orange; + case Session.ConnectionQuality.Poor: return Color.Red; + default: return Color.Gray; + } } - public static string LatencyDescription(Session.ClientPing ping) + public static string LatencyDescription(Session.Client client) { - if (ping == null) + if (client == null) return "Unknown"; - if (ping.Latency < 0) - return "Unknown"; - if (ping.Latency < 300) - return "Good"; - if (ping.Latency < 600) - return "Moderate"; - return "Poor"; + switch (client.ConnectionQuality) + { + case Session.ConnectionQuality.Good: return "Good"; + case Session.ConnectionQuality.Moderate: return "Moderate"; + case Session.ConnectionQuality.Poor: return "Poor"; + default: return "Unknown"; + } } public static void SetupLatencyWidget(Widget parent, Session.Client c, OrderManager orderManager) @@ -345,8 +343,7 @@ namespace OpenRA.Mods.Common.Widgets.Logic block.IsVisible = () => visible; if (visible) - block.Get("LATENCY_COLOR").GetColor = () => LatencyColor( - orderManager.LobbyInfo.PingFromClient(c)); + block.Get("LATENCY_COLOR").GetColor = () => LatencyColor(c); } var tooltip = parent.Get("LATENCY_REGION");