Shift Client creation to the client, sent in the handshake response. Fixes the `Newbie' bug and removes a lot of fragmented behaviour on player join.

OpenRA.FileFormats/FieldLoader.cs

@@ -262,6 +262,11 @@ namespace OpenRA.FileFormats
 					FormatValue( o, f ) ) ).ToList() );
 		}
 
+		public static MiniYamlNode SaveField(object o, string field)
+		{
+			return new MiniYamlNode(field, FieldSaver.FormatValue( o, o.GetType().GetField(field) ));
+		}
+		
 		public static string FormatValue(object o, FieldInfo f)
 		{
 			var v = f.GetValue(o);

OpenRA.Game/Network/Handshake.cs

@@ -17,30 +17,36 @@ namespace OpenRA.Network
 {
 	public class HandshakeResponse
 	{
-		public string Name;
+		[FieldLoader.Load] public string[] Mods;
-		public Color Color1;
+		[FieldLoader.Load] public string Password;
-		public Color Color2;
+		public Session.Client Client;
-		public string[] Mods = { "ra" };	// mod names
-		public string Password;
 		
 		public string Serialize()
 		{
 			var data = new List<MiniYamlNode>();
-			data.Add(new MiniYamlNode("Handshake", FieldSaver.Save(this)));
+			data.Add( new MiniYamlNode( "Handshake", null,
-			System.Console.WriteLine("Serializing handshake response:");
+				new string[]{ "Mods", "Password" }.Select( p => FieldSaver.SaveField(this, p) ).ToList() ) );
-			System.Console.WriteLine(data.WriteToString());
+			data.Add(new MiniYamlNode("Client", FieldSaver.Save(Client)));
 			
 			return data.WriteToString();
 		}
 
 		public static HandshakeResponse Deserialize(string data)
 		{
-			System.Console.WriteLine("Deserializing handshake response:");
-			System.Console.WriteLine(data);
-			
 			var handshake = new HandshakeResponse();
+			handshake.Client = new Session.Client();
+			
 			var ys = MiniYaml.FromString(data);
-			FieldLoader.Load(handshake, ys.First().Value);
+			foreach (var y in ys)
+				switch (y.Key)
+				{
+					case "Handshake":
+						FieldLoader.Load(handshake, y.Value);
+					break;
+					case "Client":
+						FieldLoader.Load(handshake.Client, y.Value);
+					break;
+				}
 			return handshake;
 		}
 	}

OpenRA.Game/Network/UnitOrders.cs

@@ -100,7 +100,6 @@ namespace OpenRA.Network
 				
 				case "HandshakeRequest":
 				{
-					Console.WriteLine("Client: Recieved HandshakeRequest");
 					// Check valid mods/versions
 					var serverInfo = Session.Deserialize(order.TargetString);
 					var serverMods = serverInfo.GlobalSettings.Mods;
@@ -112,22 +111,31 @@ namespace OpenRA.Network
 					if (serverMods.SymmetricDifference(localMods).Count() > 0)
 						throw new InvalidOperationException("Version mismatch. Client: `{0}`, Server: `{1}`"
 					                                    .F(string.Join(",",localMods), string.Join(",",serverMods)));
-					
+										
+					var info = new Session.Client()
+					{
+						Name = Game.Settings.Player.Name,
+						Color1 = Game.Settings.Player.Color1,
+						Color2 = Game.Settings.Player.Color2,
+						Country = "random",
+						SpawnPoint = 0,
+						Team = 0,
+						State = Session.ClientState.NotReady
+					};
+				
 					var response = new HandshakeResponse()
 					{
-						Name = "Test Player",
+						Client = info,
-						Color1 = Color.PaleGreen,
-						Color2 = Color.PeachPuff,
 						Mods = localMods,
 						Password = "Foo"
 					};
+				
 					orderManager.IssueOrder(Order.HandshakeResponse(response.Serialize()));
 					break;
 				}
 				
 				case "SyncInfo":
 					{
-						Console.WriteLine("Client: Recieved SyncInfo");
 						orderManager.LobbyInfo = Session.Deserialize(order.TargetString);
 
 						if (orderManager.FramesAhead != orderManager.LobbyInfo.GlobalSettings.OrderLatency

OpenRA.Game/Server/Server.cs

@@ -172,9 +172,8 @@ namespace OpenRA.Server
 			catch (Exception) { DropClient(newConn); }
 		}
 		
-		void AcceptPlayer(Connection newConn)
+		void AcceptClient(Connection newConn, Session.Client client)
 		{
-			
 			try
 			{
 				if (GameStarted)
@@ -185,17 +184,52 @@ namespace OpenRA.Server
 					return;
 				}
 
+				// Promote connection to a valid client
 				preConns.Remove(newConn);
 				conns.Add(newConn);
 
+				// Enforce correct PlayerIndex and Slot
+				client.Index = newConn.PlayerIndex;
+				client.Slot = ChooseFreeSlot();
+				
+				var slotData = lobbyInfo.Slots.FirstOrDefault( x => x.Index == client.Slot );
+				if (slotData != null && slotData.MapPlayer != null)
+					SyncClientToPlayerReference(client, Map.Players[slotData.MapPlayer]);
+				
+				lobbyInfo.Clients.Add(client);
+	
+				Log.Write("server", "Client {0}: Accepted connection from {1}",
+					newConn.PlayerIndex, newConn.socket.RemoteEndPoint);
+				SendChat(newConn, "has joined the game.");
+				
 				foreach (var t in ServerTraits.WithInterface<IClientJoined>())
 					t.ClientJoined(this, newConn);
 				
-				Console.WriteLine("Server: Accepted connection as player");
+				SyncLobbyInfo();
 			}
 			catch (Exception) { DropClient(newConn); }
 		}
 
+		int ChooseFreeSlot()
+		{
+			return lobbyInfo.Slots.First(s => !s.Closed && s.Bot == null 
+				&& !lobbyInfo.Clients.Any( c => c.Slot == s.Index )).Index;
+		}
+		
+		
+		public static void SyncClientToPlayerReference(Session.Client c, PlayerReference pr)
+		{
+			if (pr == null)
+				return;
+			if (pr.LockColor)
+			{
+				c.Color1 = pr.Color;
+				c.Color2 = pr.Color2;
+			}
+			if (pr.LockRace)
+				c.Country = pr.Race;
+		}
+		
 		public void UpdateInFlightFrames(Connection conn)
 		{
 			if (conn.Frame == 0)
@@ -288,15 +322,14 @@ namespace OpenRA.Server
 				
 					break;
 				case "HandshakeResponse":
-					Console.WriteLine("Server Recieved Handshake response");
 					var response = HandshakeResponse.Deserialize(so.Data);
+					
+					// TODO: Validate password
 				
-					// Validate versions again
+					// Accept connection, set initial client info
-				
+					AcceptClient(conn, response.Client);
-					// Validate password
+					SyncLobbyInfo();
-				
+					
-					// Accept connection; set name, color, etc.
-					AcceptPlayer(conn);
 					break;
 				case "Chat":
 				case "TeamChat":

OpenRA.Mods.RA/ServerTraits/LobbyCommands.cs

@@ -18,7 +18,7 @@ using S = OpenRA.Server.Server;
 
 namespace OpenRA.Mods.RA.Server
 {
-	public class LobbyCommands : ServerTrait, IInterpretCommand, INotifyServerStart, IClientJoined
+	public class LobbyCommands : ServerTrait, IInterpretCommand, INotifyServerStart
 	{
 		public static int MaxSpectators = 4; // How many spectators to allow // @todo Expose this as an option
 
@@ -82,7 +82,7 @@ namespace OpenRA.Mods.RA.Server
 								return true;
 	
 							client.Slot = slotData.Index;
-							SyncClientToPlayerReference(client, slotData.MapPlayer != null ? server.Map.Players[slotData.MapPlayer] : null);
+							S.SyncClientToPlayerReference(client, slotData.MapPlayer != null ? server.Map.Players[slotData.MapPlayer] : null);
 
 						server.SyncLobbyInfo();
 						return true;
@@ -99,7 +99,7 @@ namespace OpenRA.Mods.RA.Server
 							return false;
 
 						client.Slot = slot;
-						SyncClientToPlayerReference(client, slotData.MapPlayer != null ? server.Map.Players[slotData.MapPlayer] : null);
+						S.SyncClientToPlayerReference(client, slotData.MapPlayer != null ? server.Map.Players[slotData.MapPlayer] : null);
 						
 						server.SyncLobbyInfo();
 						return true;
@@ -202,7 +202,7 @@ namespace OpenRA.Mods.RA.Server
 							c.SpawnPoint = 0;
 							var slotData = server.lobbyInfo.Slots.FirstOrDefault( x => x.Index == c.Slot );
 							if (slotData != null && slotData.MapPlayer != null)
-								SyncClientToPlayerReference(c, server.Map.Players[slotData.MapPlayer]);
+								S.SyncClientToPlayerReference(c, server.Map.Players[slotData.MapPlayer]);
 				
 							c.State = Session.ClientState.NotReady;
 						}
@@ -291,55 +291,5 @@ namespace OpenRA.Mods.RA.Server
 					Bot = null
 				});
 		}
-		
-		public void ClientJoined(S server, Connection newConn)
-		{
-			var defaults = new GameRules.PlayerSettings();
-			
-			var client = new Session.Client()
-			{
-				Index = newConn.PlayerIndex,
-				Color1 = defaults.Color1,
-				Color2 = defaults.Color2,
-				Name = defaults.Name,
-				Country = "random",
-				State = Session.ClientState.NotReady,
-				SpawnPoint = 0,
-				Team = 0,
-				Slot = ChooseFreeSlot(server),
-			};
-			
-			var slotData = server.lobbyInfo.Slots.FirstOrDefault( x => x.Index == client.Slot );
-			if (slotData != null && slotData.MapPlayer != null)
-				SyncClientToPlayerReference(client, server.Map.Players[slotData.MapPlayer]);
-			
-			server.lobbyInfo.Clients.Add(client);
-
-			Log.Write("server", "Client {0}: Accepted connection from {1}",
-				newConn.PlayerIndex, newConn.socket.RemoteEndPoint);
-
-			server.SendChat(newConn, "has joined the game.");
-			server.SyncLobbyInfo();
-		}
-		
-		static int ChooseFreeSlot(S server)
-		{
-			return server.lobbyInfo.Slots.First(s => !s.Closed && s.Bot == null 
-				&& !server.lobbyInfo.Clients.Any( c => c.Slot == s.Index )).Index;
-		}
-		
-		
-		public static void SyncClientToPlayerReference(Session.Client c, PlayerReference pr)
-		{
-			if (pr == null)
-				return;
-			if (pr.LockColor)
-			{
-				c.Color1 = pr.Color;
-				c.Color2 = pr.Color2;
-			}
-			if (pr.LockRace)
-				c.Country = pr.Race;
-		}
 	}
 }

OpenRA.Mods.RA/Widgets/Delegates/LobbyDelegate.cs

@@ -115,9 +115,6 @@ namespace OpenRA.Mods.RA.Widgets.Delegates
 			
 			// Todo: Only show if the map requirements are met for player slots
 			startGameButton.IsVisible = () => Game.IsHost;
-			
-			Game.LobbyInfoChanged += JoinedServer;
-			Game.ConnectionStateChanged += ResetConnectionState;
 			Game.LobbyInfoChanged += UpdatePlayerList;
 			
 			Game.AddChatLine += lobby.GetWidget<ChatDisplayWidget>("CHAT_DISPLAY").AddLine;
@@ -171,29 +168,6 @@ namespace OpenRA.Mods.RA.Widgets.Delegates
 			title.Text = "OpenRA Multiplayer Lobby - " + orderManager.LobbyInfo.GlobalSettings.ServerName;
 		}
 
-		bool hasJoined = false;
-		void JoinedServer()
-		{
-			if (hasJoined)
-				return;
-			hasJoined = true;
-			
-			if (orderManager.LocalClient.Name != Game.Settings.Player.Name)
-				orderManager.IssueOrder(Order.Command("name " + Game.Settings.Player.Name));
-			
-			var c1 = Game.Settings.Player.Color1;
-			var c2 = Game.Settings.Player.Color2;
-			
-			if (orderManager.LocalClient.Color1 != c1 || orderManager.LocalClient.Color2 != c2)			
-				orderManager.IssueOrder(Order.Command("color {0},{1},{2},{3},{4},{5}".F(c1.R,c1.G,c1.B,c2.R,c2.G,c2.B)));
-		}
-
-		void ResetConnectionState( OrderManager orderManager )
-		{
-			if( orderManager.Connection.ConnectionState == ConnectionState.PreConnecting )
-				hasJoined = false;
-		}
-
 		Session.Client GetClientInSlot(Session.Slot slot)
 		{
 			return orderManager.LobbyInfo.ClientInSlot( slot );