Rewrite a bit of the team chat logic

OpenRA.Game/Network/Order.cs

@@ -148,9 +148,11 @@ public static Order Deserialize(World world, BinaryReader r)
 					case 0xfe:
 					{
 						var name = r.ReadString();
-						var data = r.ReadString();
+						var flags = (OrderFields)r.ReadByte();
+						var targetString = flags.HasField(OrderFields.TargetString) ? r.ReadString() : null;
+						var extraData = flags.HasField(OrderFields.ExtraData) ? r.ReadUInt32() : 0;
 
-						return new Order(name, null, false) { IsImmediate = true, TargetString = data };
+						return new Order(name, null, false) { IsImmediate = true, TargetString = targetString, ExtraData = extraData };
 					}
 
 					default:
@@ -193,9 +195,9 @@ static bool TryGetActorFromUInt(World world, uint aID, out Actor ret)
 
 		// Named constructors for Orders.
 		// Now that Orders are resolved by individual Actors, these are weird; you unpack orders manually, but not pack them.
-		public static Order Chat(bool team, string text)
+		public static Order Chat(string text, uint teamNumber = 0)
 		{
-			return new Order(team ? "TeamChat" : "Chat", null, false) { IsImmediate = true, TargetString = text };
+			return new Order("Chat", null, false) { IsImmediate = true, TargetString = text, ExtraData = teamNumber };
 		}
 
 		public static Order HandshakeResponse(string text)
@@ -245,38 +247,45 @@ public Order(string orderString, Actor subject, Target target, bool queued)
 
 		public byte[] Serialize()
 		{
-			var minLength = OrderString.Length + 1 + (IsImmediate ? 1 + TargetString.Length + 1 : 6);
+			var minLength = OrderString.Length + 1 + (IsImmediate ? 1 + 1 + TargetString.Length + 1 + 4 : 6);
 			var ret = new MemoryStream(minLength);
 			var w = new BinaryWriter(ret);
 
+			OrderFields fields = 0;
+			if (TargetString != null)
+				fields |= OrderFields.TargetString;
+
+			if (ExtraData != 0)
+				fields |= OrderFields.ExtraData;
+
 			if (IsImmediate)
 			{
 				w.Write((byte)0xFE);
 				w.Write(OrderString);
-				w.Write(TargetString);
+				w.Write((byte)fields);
+
+				if (fields.HasField(OrderFields.TargetString))
+					w.Write(TargetString);
+
+				if (fields.HasField(OrderFields.ExtraData))
+					w.Write(ExtraData);
+
 				return ret.ToArray();
 			}
 
 			w.Write((byte)0xFF);
 			w.Write(OrderString);
 			w.Write(UIntFromActor(Subject));
 
-			OrderFields fields = 0;
 			if (Target.SerializableType != TargetType.Invalid)
 				fields |= OrderFields.Target;
 
-			if (TargetString != null)
-				fields |= OrderFields.TargetString;
-
 			if (Queued)
 				fields |= OrderFields.Queued;
 
 			if (ExtraLocation != CPos.Zero)
 				fields |= OrderFields.ExtraLocation;
 
-			if (ExtraData != 0)
-				fields |= OrderFields.ExtraData;
-
 			if (Target.SerializableCell != null)
 				fields |= OrderFields.TargetIsCell;
 

OpenRA.Game/Network/UnitOrders.cs

@@ -24,10 +24,7 @@ public static class UnitOrders
 
 		static Player FindPlayerByClient(this World world, Session.Client c)
 		{
-			/* TODO: this is still a hack.
-			 * the cases we're trying to avoid are the extra players on the host's client -- Neutral, other MapPlayers,..*/
-			return world.Players.FirstOrDefault(
-				p => (p.ClientIndex == c.Index && p.PlayerReference.Playable));
+			return world.Players.FirstOrDefault(p => (p.ClientIndex == c.Index && p.PlayerReference.Playable));
 		}
 
 		internal static void ProcessOrder(OrderManager orderManager, World world, int clientId, Order order)
@@ -41,66 +38,78 @@ internal static void ProcessOrder(OrderManager orderManager, World world, int cl
 
 			switch (order.OrderString)
 			{
+				// Server message
+				case "Message":
+					Game.AddChatLine(Color.White, ServerChatName, 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;
+						break;
+					}
+
 				case "Chat":
 					{
 						var client = orderManager.LobbyInfo.ClientWithIndex(clientId);
+						if (client == null)
+							break;
 
 						// Cut chat messages to the hard limit to avoid exploits
 						var message = order.TargetString;
 						if (message.Length > ChatMessageMaxLength)
 							message = order.TargetString.Substring(0, ChatMessageMaxLength);
 
-						if (client != null)
+						// ExtraData 0 means this is a normal chat order, everything else is team chat
+						if (order.ExtraData == 0)
 						{
-							var player = world != null ? world.FindPlayerByClient(client) : null;
-							var suffix = (player != null && player.WinState == WinState.Lost) ? " (Dead)" : "";
+							var p = world != null ? world.FindPlayerByClient(client) : null;
+							var suffix = (p != null && p.WinState == WinState.Lost) ? " (Dead)" : "";
 							suffix = client.IsObserver ? " (Spectator)" : suffix;
 
 							if (orderManager.LocalClient != null && client != orderManager.LocalClient && client.Team > 0 && client.Team == orderManager.LocalClient.Team)
 								suffix += " (Ally)";
 
 							Game.AddChatLine(client.Color, client.Name + suffix, message);
+							break;
 						}
-						else
-							Game.AddChatLine(Color.White, "(player {0})".F(clientId), message);
-						break;
-					}
 
-				case "Message": // Server message
-					Game.AddChatLine(Color.White, ServerChatName, order.TargetString);
-					break;
+						// We are still in the lobby
+						if (world == null)
+						{
+							var prefix = order.ExtraData == uint.MaxValue ? "[Spectators] " : "[Team] ";
+							if (orderManager.LocalClient != null && client.Team == orderManager.LocalClient.Team)
+								Game.AddChatLine(client.Color, prefix + client.Name, message);
 
-				case "Disconnected": /* reports that the target player disconnected */
-					{
-						var client = orderManager.LobbyInfo.ClientWithIndex(clientId);
-						if (client != null)
-							client.State = Session.ClientState.Disconnected;
-						break;
-					}
+							break;
+						}
 
-				case "TeamChat":
-					{
-						var client = orderManager.LobbyInfo.ClientWithIndex(clientId);
+						if (orderManager.LocalClient == null)
+							break;
 
-						if (client != null)
+						var player = world.FindPlayerByClient(client);
+						var localClientIsObserver = orderManager.LocalClient.IsObserver || (world.LocalPlayer != null && world.LocalPlayer.WinState != WinState.Undefined);
+
+						// ExtraData gives us the team number, uint.MaxValue means Spectators
+						if (order.ExtraData == uint.MaxValue && (localClientIsObserver || world.IsReplay))
 						{
-							if (world == null)
-							{
-								if (orderManager.LocalClient != null && client.Team == orderManager.LocalClient.Team)
-									Game.AddChatLine(client.Color, "[Team] " + client.Name, order.TargetString);
-							}
-							else
-							{
-								var player = world.FindPlayerByClient(client);
-								if (player != null && player.WinState == WinState.Lost)
-									Game.AddChatLine(client.Color, client.Name + " (Dead)", order.TargetString);
-								else if ((player != null && world.LocalPlayer != null && player.Stances[world.LocalPlayer] == Stance.Ally) || (world.IsReplay && player != null))
-									Game.AddChatLine(client.Color, "[Team" + (world.IsReplay ? " " + client.Team : "") + "] " + client.Name, order.TargetString);
-								else if ((orderManager.LocalClient != null && orderManager.LocalClient.IsObserver && client.IsObserver) || (world.IsReplay  && client.IsObserver))
-									Game.AddChatLine(client.Color, "[Spectators] " + client.Name, order.TargetString);
-							}
+							// Validate before adding the line
+							if (client.IsObserver || (player != null && player.WinState != WinState.Undefined))
+								Game.AddChatLine(client.Color, "[Spectators] " + client.Name, message);
+
+							break;
 						}
 
+						var valid = client.Team == order.ExtraData && player != null && player.WinState == WinState.Undefined;
+						var isSameTeam = order.ExtraData == orderManager.LocalClient.Team && world.LocalPlayer != null
+							&& world.LocalPlayer.WinState == WinState.Undefined;
+
+						if (valid && (isSameTeam || world.IsReplay))
+							Game.AddChatLine(client.Color, "[Team" + (world.IsReplay ? " " + order.ExtraData : "") + "] " + client.Name, message);
+
 						break;
 					}
 

OpenRA.Game/Server/Server.cs

@@ -631,7 +631,6 @@ void InterpretServerOrder(Connection conn, ServerOrder so)
 					}
 
 				case "Chat":
-				case "TeamChat":
 				case "PauseGame":
 					DispatchOrdersToClients(conn, 0, so.Serialize());
 					break;

OpenRA.Game/Server/ServerOrder.cs

@@ -18,11 +18,13 @@ class ServerOrder
 	{
 		public readonly string Name;
 		public readonly string Data;
+		public readonly uint ExtraData;
 
-		public ServerOrder(string name, string data)
+		public ServerOrder(string name, string data, uint extraData = 0)
 		{
 			Name = name;
 			Data = data;
+			ExtraData = extraData;
 		}
 
 		public static ServerOrder Deserialize(BinaryReader r)
@@ -40,9 +42,11 @@ public static ServerOrder Deserialize(BinaryReader r)
 				case 0xfe:
 					{
 						var name = r.ReadString();
-						var data = r.ReadString();
+						var flags = (OrderFields)r.ReadByte();
+						var data = flags.HasField(OrderFields.TargetString) ? r.ReadString() : null;
+						var extraData = flags.HasField(OrderFields.ExtraData) ? r.ReadUInt32() : 0;
 
-						return new ServerOrder(name, data);
+						return new ServerOrder(name, data, extraData);
 					}
 
 				default:
@@ -52,12 +56,26 @@ public static ServerOrder Deserialize(BinaryReader r)
 
 		public byte[] Serialize()
 		{
-			var ms = new MemoryStream(1 + Name.Length + 1 + Data.Length + 1);
+			var ms = new MemoryStream(1 + Name.Length + 1 + 1 + Data.Length + 1 + 4);
 			var bw = new BinaryWriter(ms);
 
+			OrderFields fields = 0;
+			if (Data != null)
+				fields |= OrderFields.TargetString;
+
+			if (ExtraData != 0)
+				fields |= OrderFields.ExtraData;
+
 			bw.Write((byte)0xfe);
 			bw.Write(Name);
-			bw.Write(Data);
+			bw.Write((byte)fields);
+
+			if (fields.HasField(OrderFields.TargetString))
+				bw.Write(Data);
+
+			if (fields.HasField(OrderFields.ExtraData))
+				bw.Write(ExtraData);
+
 			return ms.ToArray();
 		}
 	}

OpenRA.Mods.Common/Traits/Player/PlayerStatistics.cs

@@ -96,7 +96,6 @@ public void ResolveOrder(Actor self, Order order)
 			switch (order.OrderString)
 			{
 				case "Chat":
-				case "TeamChat":
 				case "HandshakeResponse":
 				case "PauseGame":
 				case "StartGame":