Overhaul player disconnect notifications.

OpenRA.Game/Network/Connection.cs

@@ -300,8 +300,8 @@ namespace OpenRA.Network
 			while (receivedPackets.TryDequeue(out var p))
 			{
 				var record = true;
-				if (OrderIO.TryParseDisconnect(p.Data, out var disconnectClient))
+				if (OrderIO.TryParseDisconnect(p, out var disconnect))
-					orderManager.ReceiveDisconnect(disconnectClient);
+					orderManager.ReceiveDisconnect(disconnect.ClientId, disconnect.Frame);
 				else if (OrderIO.TryParseSync(p.Data, out var sync))
 					orderManager.ReceiveSync(sync);
 				else if (OrderIO.TryParseAck(p, out var ackFrame))

OpenRA.Game/Network/OrderIO.cs

@@ -79,16 +79,19 @@ namespace OpenRA.Network
 			return ms.GetBuffer();
 		}
 
-		public static bool TryParseDisconnect(byte[] packet, out int clientId)
+		public static bool TryParseDisconnect((int FromClient, byte[] Data) packet, out (int Frame, int ClientId) disconnect)
 		{
-			if (packet.Length == Order.DisconnectOrderLength + 4 && packet[4] == (byte)OrderType.Disconnect)
+			// Valid Disconnect packets are only ever generated by the server
+			if (packet.FromClient != 0 || packet.Data.Length != Order.DisconnectOrderLength + 4 || packet.Data[4] != (byte)OrderType.Disconnect)
 			{
-				clientId = BitConverter.ToInt32(packet, 5);
+				disconnect = (0, 0);
-				return true;
+				return false;
 			}
 
-			clientId = 0;
+			var frame = BitConverter.ToInt32(packet.Data, 0);
-			return false;
+			var clientId = BitConverter.ToInt32(packet.Data, 5);
+			disconnect = (frame, clientId);
+			return true;
 		}
 
 		public static bool TryParseSync(byte[] packet, out (int Frame, int SyncHash, ulong DefeatState) data)

OpenRA.Game/Network/OrderManager.cs

@@ -20,6 +20,8 @@ namespace OpenRA.Network
 {
 	public sealed class OrderManager : IDisposable
 	{
+		const OrderPacket ClientDisconnected = null;
+
 		readonly SyncReport syncReport;
 		readonly Dictionary<int, Queue<(int Frame, OrderPacket Orders)>> pendingOrders = new Dictionary<int, Queue<(int, OrderPacket)>>();
 		readonly Dictionary<int, (int SyncHash, ulong DefeatState)> syncForFrame = new Dictionary<int, (int, ulong)>();
@@ -45,6 +47,9 @@ namespace OpenRA.Network
 		readonly List<Order> localOrders = new List<Order>();
 		readonly List<Order> localImmediateOrders = new List<Order>();
 
+		readonly List<ClientOrder> processClientOrders = new List<ClientOrder>();
+		readonly List<int> processClientsToRemove = new List<int>();
+
 		readonly List<TextNotification> notificationsCache = new List<TextNotification>();
 
 		public IReadOnlyList<TextNotification> NotificationsCache => notificationsCache;
@@ -126,9 +131,18 @@ namespace OpenRA.Network
 			localImmediateOrders.Clear();
 		}
 
-		public void ReceiveDisconnect(int clientIndex)
+		public void ReceiveDisconnect(int clientId, int frame)
 		{
-			pendingOrders.Remove(clientIndex);
+			// All clients must process the disconnect on the same world tick to allow synced actions to run deterministically.
+			// The server guarantees that we will not receive any more order packets from this client from this frame, so we
+			// can insert a marker in the orders stream and process the synced disconnect behaviours on the first tick of that frame.
+			if (GameStarted)
+				ReceiveOrders(clientId, (frame, ClientDisconnected));
+
+			// The Client state field is not synced; update it immediately so it can be shown in the UI
+			var client = LobbyInfo.ClientWithIndex(clientId);
+			if (client != null)
+				client.State = Session.ClientState.Disconnected;
 		}
 
 		public void ReceiveSync((int Frame, int SyncHash, ulong DefeatState) sync)
@@ -198,8 +212,6 @@ namespace OpenRA.Network
 
 		void ProcessOrders()
 		{
-			var clientOrders = new List<ClientOrder>();
-
 			foreach (var (clientId, frameOrders) in pendingOrders)
 			{
 				// The IsReadyForNextFrame check above guarantees that all clients have sent a packet
@@ -211,13 +223,24 @@ namespace OpenRA.Network
 				if (frameNumber != NetFrameNumber)
 					throw new InvalidDataException($"Attempted to process orders from client {clientId} for frame {frameNumber} on frame {NetFrameNumber}");
 
+				if (orders == ClientDisconnected)
+				{
+					processClientsToRemove.Add(clientId);
+					World.OnClientDisconnected(clientId);
+
+					continue;
+				}
+
 				foreach (var order in orders.GetOrders(World))
 				{
 					UnitOrders.ProcessOrder(this, World, clientId, order);
-					clientOrders.Add(new ClientOrder { Client = clientId, Order = order });
+					processClientOrders.Add(new ClientOrder { Client = clientId, Order = order });
 				}
 			}
 
+			foreach (var clientId in processClientsToRemove)
+				pendingOrders.Remove(clientId);
+
 			if (NetFrameNumber >= GameSaveLastSyncFrame)
 			{
 				var defeatState = 0UL;
@@ -232,7 +255,10 @@ namespace OpenRA.Network
 
 			if (generateSyncReport)
 				using (new PerfSample("sync_report"))
-					syncReport.UpdateSyncReport(clientOrders);
+					syncReport.UpdateSyncReport(processClientOrders);
+
+			processClientOrders.Clear();
+			processClientsToRemove.Clear();
 
 			++NetFrameNumber;
 		}

OpenRA.Game/Network/ReplayConnection.cs

@@ -147,8 +147,8 @@ namespace OpenRA.Network
 			{
 				foreach (var o in chunks.Dequeue().Packets)
 				{
-					if (OrderIO.TryParseDisconnect(o.Packet, out var disconnectClient))
+					if (OrderIO.TryParseDisconnect(o, out var disconnect))
-						orderManager.ReceiveDisconnect(disconnectClient);
+						orderManager.ReceiveDisconnect(disconnect.ClientId, disconnect.Frame);
 					else if (OrderIO.TryParseSync(o.Packet, out var sync))
 						orderManager.ReceiveSync(sync);
 					else if (OrderIO.TryParseOrderPacket(o.Packet, out var orders))

OpenRA.Game/Network/SyncReport.cs

@@ -51,20 +51,21 @@ namespace OpenRA.Network
 				syncReports[i] = new Report();
 		}
 
-		internal void UpdateSyncReport(List<OrderManager.ClientOrder> orders)
+		internal void UpdateSyncReport(IEnumerable<OrderManager.ClientOrder> orders)
 		{
 			GenerateSyncReport(syncReports[curIndex], orders);
 			curIndex = ++curIndex % NumSyncReports;
 		}
 
-		void GenerateSyncReport(Report report, List<OrderManager.ClientOrder> orders)
+		void GenerateSyncReport(Report report, IEnumerable<OrderManager.ClientOrder> orders)
 		{
 			report.Frame = orderManager.NetFrameNumber;
 			report.SyncedRandom = orderManager.World.SharedRandom.Last;
 			report.TotalCount = orderManager.World.SharedRandom.TotalCount;
 			report.Traits.Clear();
 			report.Effects.Clear();
-			report.Orders = orders;
+			report.Orders.Clear();
+			report.Orders.AddRange(orders);
 
 			foreach (var actor in orderManager.World.ActorsHavingTrait<ISync>())
 			{

OpenRA.Game/Network/UnitOrders.cs

@@ -34,21 +34,6 @@ namespace OpenRA.Network
 					TextNotificationsManager.AddSystemLine(order.TargetString);
 					break;
 
-				// Reports that the target player disconnected
-				case "Disconnected":
-					{
-						var client = orderManager.LobbyInfo.ClientWithIndex(clientId);
-						if (client != null)
-						{
-							client.State = Session.ClientState.Disconnected;
-							var player = world?.FindPlayerByClient(client);
-							if (player != null)
-								world.OnPlayerDisconnected(player);
-						}
-
-						break;
-					}
-
 				case "Chat":
 					{
 						var client = orderManager.LobbyInfo.ClientWithIndex(clientId);

OpenRA.Game/Player.cs

@@ -82,6 +82,7 @@ namespace OpenRA
 		readonly bool inMissionMap;
 		readonly bool spectating;
 		readonly IUnlocksRenderPlayer[] unlockRenderPlayer;
+		readonly INotifyPlayerDisconnected[] notifyDisconnected;
 
 		// Each player is identified with a unique bit in the set
 		// Cache masks for the player's index and ally/enemy player indices for performance.
@@ -226,6 +227,7 @@ namespace OpenRA
 			stanceColors.Neutrals = ChromeMetrics.Get<Color>("PlayerStanceColorNeutrals");
 
 			unlockRenderPlayer = PlayerActor.TraitsImplementing<IUnlocksRenderPlayer>().ToArray();
+			notifyDisconnected = PlayerActor.TraitsImplementing<INotifyPlayerDisconnected>().ToArray();
 		}
 
 		public override string ToString()
@@ -280,6 +282,12 @@ namespace OpenRA
 			return stanceColors.Neutrals;
 		}
 
+		internal void PlayerDisconnected(Player p)
+		{
+			foreach (var np in notifyDisconnected)
+				np.PlayerDisconnected(PlayerActor, p);
+		}
+
 		#region Scripting interface
 
 		Lazy<ScriptPlayerInterface> luaInterface;

OpenRA.Game/Server/Connection.cs

@@ -28,10 +28,10 @@ namespace OpenRA.Server
 		public readonly EndPoint EndPoint;
 
 		public long TimeSinceLastResponse => Game.RunTime - lastReceivedTime;
-		public int MostRecentFrame { get; private set; }
 
 		public bool TimeoutMessageShown;
 		public bool Validated;
+		public int LastOrdersFrame;
 
 		long lastReceivedTime = 0;
 
@@ -107,9 +107,6 @@ namespace OpenRA.Server
 
 								case ReceiveState.Data:
 								{
-									if (MostRecentFrame < frame)
-										MostRecentFrame = frame;
-
 									onPacket(this, frame, bytes);
 									expectLength = 8;
 									state = ReceiveState.Header;

OpenRA.Game/Server/ProtocolVersion.cs

@@ -28,6 +28,7 @@ namespace OpenRA.Server
 		//   - UInt64 containing the current defeat state (a bit set
 		//     to 1 means the corresponding player is defeated)
 		// - 0xBF: Player disconnected
+		//   - Int32 specifying the client ID that disconnected
 		// - 0xFE: Handshake (also used for ServerOrders for ProtocolVersion.Orders < 8)
 		//   - Length-prefixed string specifying a name or key
 		//   - Length-prefixed string specifying a value / data
@@ -70,6 +71,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 = 14;
+		public const int Orders = 15;
 	}
 }

OpenRA.Game/Server/Server.cs

@@ -778,10 +778,9 @@ namespace OpenRA.Server
 			DispatchServerOrdersToClients(order.Serialize());
 		}
 
-		public void DispatchServerOrdersToClients(byte[] data)
+		public void DispatchServerOrdersToClients(byte[] data, int frame = 0)
 		{
 			var from = 0;
-			var frame = 0;
 			var frameData = CreateFrame(from, frame, data);
 			foreach (var c in Conns.ToList())
 				if (c.Validated)
@@ -792,6 +791,10 @@ namespace OpenRA.Server
 
 		public void ReceiveOrders(Connection conn, int frame, byte[] data)
 		{
+			// Make sure we don't accidentally forward on orders from clients who we have just dropped
+			if (!Conns.Contains(conn))
+				return;
+
 			if (frame == 0)
 				InterpretServerOrders(conn, data);
 			else
@@ -806,6 +809,11 @@ namespace OpenRA.Server
 				{
 					frame += OrderLatency;
 					DispatchFrameToClient(conn, conn.PlayerIndex, CreateAckFrame(frame));
+
+					// Track the last frame for each client so the disconnect handling can write
+					// an EndOfOrders marker with the correct frame number.
+					// TODO: This should be handled by the order buffering system too
+					conn.LastOrdersFrame = frame;
 				}
 
 				DispatchOrdersToClients(conn, frame, data);
@@ -1059,15 +1067,6 @@ namespace OpenRA.Server
 					suffix = dropClient.IsObserver ? " (Spectator)" : dropClient.Team != 0 ? $" (Team {dropClient.Team})" : "";
 				SendMessage($"{dropClient.Name}{suffix} has disconnected.");
 
-				// Send disconnected order, even if still in the lobby
-				DispatchOrdersToClients(toDrop, 0, Order.FromTargetString("Disconnected", "", true).Serialize());
-
-				if (gameInfo != null && !dropClient.IsObserver)
-				{
-					var disconnectedPlayer = gameInfo.Players.First(p => p.ClientIndex == toDrop.PlayerIndex);
-					disconnectedPlayer.DisconnectFrame = toDrop.MostRecentFrame;
-				}
-
 				LobbyInfo.Clients.RemoveAll(c => c.Index == toDrop.PlayerIndex);
 				LobbyInfo.ClientPings.RemoveAll(p => p.Index == toDrop.PlayerIndex);
 
@@ -1091,7 +1090,11 @@ namespace OpenRA.Server
 				var disconnectPacket = new MemoryStream(5);
 				disconnectPacket.WriteByte((byte)OrderType.Disconnect);
 				disconnectPacket.Write(toDrop.PlayerIndex);
-				DispatchServerOrdersToClients(disconnectPacket.ToArray());
+				DispatchServerOrdersToClients(disconnectPacket.ToArray(), toDrop.LastOrdersFrame + 1);
+
+				if (gameInfo != null)
+					foreach (var player in gameInfo.Players.Where(p => p.ClientIndex == toDrop.PlayerIndex))
+						player.DisconnectFrame = toDrop.LastOrdersFrame + 1;
 
 				// All clients have left: clean up
 				if (!Conns.Any(c => c.Validated))
@@ -1281,13 +1284,13 @@ namespace OpenRA.Server
 				{
 					for (var i = 0; i < OrderLatency; i++)
 					{
-						var frame = firstFrame + i;
+						from.LastOrdersFrame = firstFrame + i;
-						var frameData = CreateFrame(from.PlayerIndex, frame, Array.Empty<byte>());
+						var frameData = CreateFrame(from.PlayerIndex, from.LastOrdersFrame, Array.Empty<byte>());
 						foreach (var to in conns)
 							DispatchFrameToClient(to, from.PlayerIndex, frameData);
 
-						RecordOrder(frame, Array.Empty<byte>(), from.PlayerIndex);
+						RecordOrder(from.LastOrdersFrame, Array.Empty<byte>(), from.PlayerIndex);
-						GameSave?.DispatchOrders(from, frame, Array.Empty<byte>());
+						GameSave?.DispatchOrders(from, from.LastOrdersFrame, Array.Empty<byte>());
 					}
 				}
 			}

OpenRA.Game/Traits/TraitsInterfaces.cs

@@ -605,4 +605,10 @@ namespace OpenRA.Traits
 	{
 		IEnumerable<VariableObserver> GetVariableObservers();
 	}
+
+	[RequireExplicitImplementation]
+	public interface INotifyPlayerDisconnected
+	{
+		void PlayerDisconnected(Actor self, Player p);
+	}
 }

OpenRA.Game/World.cs

@@ -136,6 +136,7 @@ namespace OpenRA
 		public readonly WorldType Type;
 
 		public readonly IValidateOrder[] OrderValidators;
+		readonly INotifyPlayerDisconnected[] notifyDisconnected;
 
 		readonly GameInformation gameInfo;
 
@@ -201,6 +202,7 @@ namespace OpenRA
 			ScreenMap = WorldActor.Trait<ScreenMap>();
 			Selection = WorldActor.Trait<ISelection>();
 			OrderValidators = WorldActor.TraitsImplementing<IValidateOrder>().ToArray();
+			notifyDisconnected = WorldActor.TraitsImplementing<INotifyPlayerDisconnected>().ToArray();
 
 			LongBitSet<PlayerBitMask>.Reset();
 
@@ -519,13 +521,20 @@ namespace OpenRA
 			}
 		}
 
-		public void OnPlayerDisconnected(Player player)
+		internal void OnClientDisconnected(int clientId)
 		{
-			var pi = gameInfo.GetPlayer(player);
+			foreach (var player in Players.Where(p => p.ClientIndex == clientId && p.PlayerReference.Playable))
-			if (pi == null)
+			{
-				return;
+				foreach (var np in notifyDisconnected)
+					np.PlayerDisconnected(WorldActor, player);
 
-			pi.DisconnectFrame = OrderManager.NetFrameNumber;
+				foreach (var p in Players)
+					p.PlayerDisconnected(player);
+
+				var pi = gameInfo.GetPlayer(player);
+				if (pi != null)
+					pi.DisconnectFrame = OrderManager.NetFrameNumber;
+			}
 		}
 
 		public void RequestGameSave(string filename)