Further gamesetup fixes & tweaks.

Fixes dc18d94030

- Player assignment still had some trouble. This cleans things up by
moving a little more logic in the controller & fixing other usage.
 - As a consequence, in MP, if the host swaps two player, the change
will take a few frames to register. It shouldn't be too much of a
problem in practice.
 - Fix issues with AI being assigned to player slots for joiners.
- Fix issues with similar color not being correctly picked when swapping
a map.
- Fix issues with rating & locked team settings.

Reported by: langbart
Differential Revision: https://code.wildfiregames.com/D3723
This was SVN commit r25096.

binaries/data/mods/public/gui/gamesettings/attributes/PlayerColor.js

@@ -59,12 +59,10 @@ GameSettings.prototype.Attributes.PlayerColor = class PlayerColor extends GameSe
 		// Reset.
 		this.locked = this.locked.map(x => this.settings.map.type == "scenario");
 		this.trigger("locked");
-
 		if (this.settings.map.type === "scenario")
 			this._resize(0);
 		this._updateAvailable();
 		this.maybeUpdate();
-		this.maybeUpdate();
 	}
 
 	maybeUpdate()
@@ -80,20 +78,12 @@ GameSettings.prototype.Attributes.PlayerColor = class PlayerColor extends GameSe
 		let inUse = this.values.findIndex((otherColor, i) =>
 			color && otherColor &&
 			sameColor(color, otherColor));
-		if (inUse !== -1 && inUse !== playerIndex)
+		if (inUse != -1 && inUse != playerIndex)
 		{
-			if (sameColor(this.values[playerIndex], this.values[inUse]))
+			// Swap colors.
-			{
+			let col = this.values[playerIndex];
-				this.values[playerIndex] = undefined;
+			this.values[playerIndex] = undefined;
-				color = undefined;
+			this._set(inUse, col);
-			}
-			else
-			{
-				// Swap colors.
-				let col = this.values[playerIndex];
-				this.values[playerIndex] = undefined;
-				this._set(inUse, col);
-			}
 		}
 		if (!color || this.available.indexOf(color) == -1)
 		{
@@ -140,20 +130,34 @@ GameSettings.prototype.Attributes.PlayerColor = class PlayerColor extends GameSe
 		// Pick colors that the map specifies, add most unsimilar default colors
 		// Provide the access to g_MaxPlayers different colors, regardless of current playercount.
 		let values = [];
+		let mapColors = false;
 		for (let i = 0; i < g_MaxPlayers; ++i)
-			values.push(this._getMapData(i) ||
+		{
-				this.defaultColors[i] || this._findFarthestUnusedColor(values));
+			let col = this._getMapData(i);
+			if (col)
+				mapColors = true;
+			if (mapColors)
+				values.push(col || this._findFarthestUnusedColor(values));
+			else
+				values.push(this.defaultColors[i]);
+		}
 		this.available = values;
 	}
 
 	_findClosestColor(targetColor, colors)
 	{
-		let colorDistances = colors.map(color => colorDistance(color, targetColor));
+		let closestColor;
-
+		let closestColorDistance = 0;
-		let smallestDistance = colorDistances.find(
+		for (let color of colors)
-			distance => colorDistances.every(distance2 => distance2 >= distance));
+		{
-
+			let dist = colorDistance(targetColor, color);
-		return colors.find(color => colorDistance(color, targetColor) == smallestDistance);
+			if (!closestColor || dist < closestColorDistance)
+			{
+				closestColor = color;
+				closestColorDistance = dist;
+			}
+		}
+		return closestColor;
 	}
 
 	_findFarthestUnusedColor(values)

binaries/data/mods/public/gui/gamesettings/attributes/Rating.js

@@ -15,7 +15,7 @@ GameSettings.prototype.Attributes.Rating = class Rating extends GameSetting
 
 	fromInitAttributes(attribs)
 	{
-		if (this.getLegacySetting(attribs, "RatingEnabled"))
+		if (this.getLegacySetting(attribs, "RatingEnabled") !== undefined)
 		{
 			this.available = this.hasXmppClient && this.settings.playerCount.nbPlayers === 2;
 			this.enabled = this.available && !!this.getLegacySetting(attribs, "RatingEnabled");

binaries/data/mods/public/gui/gamesetup/Controls/PlayerAssignmentsControl.js

@@ -17,10 +17,11 @@ class PlayerAssignmentsControl
 			// Replace empty player name when entering a single-player match for the first time.
 			Engine.ConfigDB_CreateAndWriteValueToFile("user", this.ConfigNameSingleplayer, name, "config/user.cfg");
 
+			// By default, assign the player to the first slot.
 			g_PlayerAssignments = {
 				"local": {
 					"name": name,
-					"player": -1
+					"player": 1
 				}
 			};
 		}
@@ -30,6 +31,8 @@ class PlayerAssignmentsControl
 		setupWindow.registerLoadHandler(this.onLoad.bind(this));
 		setupWindow.registerGetHotloadDataHandler(this.onGetHotloadData.bind(this));
 		netMessages.registerNetMessageHandler("players", this.onPlayerAssignmentMessage.bind(this));
+
+		this.registerClientJoinHandler(this.onClientJoin.bind(this));
 	}
 
 	registerPlayerAssignmentsChangeHandler(handler)
@@ -69,6 +72,13 @@ class PlayerAssignmentsControl
 			g_PlayerAssignments = hotloadData.playerAssignments;
 			this.updatePlayerAssignments();
 		}
+		else if (!g_IsNetworked)
+		{
+			// Simulate a net message for the local player to keep a common path.
+			this.onPlayerAssignmentMessage({
+				"newAssignments": g_PlayerAssignments
+			});
+		}
 	}
 
 	onGetHotloadData(object)
@@ -76,6 +86,38 @@ class PlayerAssignmentsControl
 		object.playerAssignments = g_PlayerAssignments;
 	}
 
+	/**
+	 * On client join, try to assign them to a free slot.
+	 * (This is called before g_PlayerAssignments is updated).
+	 */
+	onClientJoin(newGUID, newAssignments)
+	{
+		if (!g_IsController || newAssignments[newGUID].player != -1)
+			return;
+
+		// Assign the client (or only buddies if prefered) to a free slot
+		if (newGUID != Engine.GetPlayerGUID())
+		{
+			let assignOption = Engine.ConfigDB_GetValue("user", this.ConfigAssignPlayers);
+			if (assignOption == "disabled" ||
+				assignOption == "buddies" && g_Buddies.indexOf(splitRatingFromNick(newAssignments[newGUID].name).nick) == -1)
+				return;
+		}
+
+		// Find a player slot that no other player is assigned to.
+		let firstFreeSlot = [...Array(g_MaxPlayers).keys()];
+		firstFreeSlot = firstFreeSlot.find(i => {
+			for (let guid in newAssignments)
+				if (newAssignments[guid].player == i + 1)
+					return false;
+			return true;
+		});
+		if (firstFreeSlot === -1)
+			return;
+
+		this.assignClient(newGUID, firstFreeSlot + 1);
+	}
+
 	/**
 	 * To be called when g_PlayerAssignments is modified.
 	 */
@@ -112,12 +154,13 @@ class PlayerAssignmentsControl
 
 	assignClient(guid, playerIndex)
 	{
-		g_GameSettings.playerAI.setAI(playerIndex - 1, undefined);
 		if (g_IsNetworked)
 			Engine.AssignNetworkPlayer(playerIndex, guid);
-		if (g_PlayerAssignments[guid])
+		else
+		{
 			g_PlayerAssignments[guid].player = playerIndex;
-		this.updatePlayerAssignments();
+			this.updatePlayerAssignments();
+		}
 	}
 
 	/**
@@ -151,9 +194,11 @@ class PlayerAssignmentsControl
 	unassignInvalidPlayers()
 	{
 		if (g_IsNetworked)
-			for (let playerID = g_GameSettings.playerCount.nbPlayers + 1; playerID <= g_MaxPlayers; ++playerID)
+		{
-				// Remove obsolete playerIDs from the servers playerassignments copy
+			for (let guid in g_PlayerAssignments)
-				Engine.AssignNetworkPlayer(playerID, "");
+				if (g_PlayerAssignments[guid].player > g_GameSettings.playerCount.nbPlayers)
+					Engine.AssignNetworkPlayer(g_PlayerAssignments[guid].player, "");
+		}
 		else if (g_PlayerAssignments.local.player > g_GameSettings.playerCount.nbPlayers)
 		{
 			g_PlayerAssignments.local.player = -1;
@@ -164,3 +209,6 @@ class PlayerAssignmentsControl
 
 PlayerAssignmentsControl.prototype.ConfigNameSingleplayer =
 	"playername.singleplayer";
+
+PlayerAssignmentsControl.prototype.ConfigAssignPlayers =
+	"gui.gamesetup.assignplayers";

binaries/data/mods/public/gui/gamesetup/Pages/GameSetupPage/GameSettings/PerPlayer/Dropdowns/PlayerAssignment.js

@@ -27,12 +27,11 @@ PlayerSettingControls.PlayerAssignment = class PlayerAssignment extends GameSett
 		this.assignedGUID = undefined;
 		this.fixedAI = undefined;
 
+		// Build the initial list of values with undefined & AI clients.
+		this.rebuildList();
+
 		g_GameSettings.playerAI.watch(() => this.render(), ["values"]);
 		g_GameSettings.playerCount.watch((_, oldNb) => this.OnPlayerNbChange(oldNb), ["nbPlayers"]);
-
-		// Sets up the dropdown and renders.
-		this.onPlayerAssignmentsChange();
-		this.playerAssignmentsControl.registerClientJoinHandler(this.onClientJoin.bind(this));
 	}
 
 	setControl()
@@ -41,13 +40,6 @@ PlayerSettingControls.PlayerAssignment = class PlayerAssignment extends GameSett
 		this.label = Engine.GetGUIObjectByName("playerAssignmentText[" + this.playerIndex + "]");
 	}
 
-	onLoad(initData, hotloadData)
-	{
-		if (!hotloadData && !g_IsNetworked)
-			this.onClientJoin("local", g_PlayerAssignments);
-		this.playerAssignmentsControl.updatePlayerAssignments();
-	}
-
 	OnPlayerNbChange(oldNb)
 	{
 		let isPlayerSlot = Object.values(g_PlayerAssignments).some(x => x.player === this.playerIndex + 1);
@@ -64,30 +56,10 @@ PlayerSettingControls.PlayerAssignment = class PlayerAssignment extends GameSett
 		}
 	}
 
-	onClientJoin(newGUID, newAssignments)
-	{
-		if (!g_IsController || this.fixedAI || newAssignments[newGUID].player != -1)
-			return;
-
-		// Assign the client (or only buddies if prefered) to a free slot
-		if (newGUID != Engine.GetPlayerGUID())
-		{
-			let assignOption = Engine.ConfigDB_GetValue("user", this.ConfigAssignPlayers);
-			if (assignOption == "disabled" ||
-				assignOption == "buddies" && g_Buddies.indexOf(splitRatingFromNick(newAssignments[newGUID].name).nick) == -1)
-				return;
-		}
-
-		for (let guid in newAssignments)
-			if (newAssignments[guid].player == this.playerIndex + 1)
-				return;
-
-		newAssignments[newGUID].player = this.playerIndex + 1;
-		this.playerAssignmentsControl.assignClient(newGUID, this.playerIndex + 1);
-	}
-
 	onPlayerAssignmentsChange()
 	{
+		// Rebuild the list to account for new/removed players.
+		this.rebuildList();
 		let newGUID;
 		for (let guid in g_PlayerAssignments)
 			if (g_PlayerAssignments[guid].player == this.playerIndex + 1)
@@ -95,8 +67,14 @@ PlayerSettingControls.PlayerAssignment = class PlayerAssignment extends GameSett
 				newGUID = guid;
 				break;
 			}
+		if (this.assignedGUID === newGUID)
+			return;
 		this.assignedGUID = newGUID;
-		this.rebuildList();
+		if (this.assignedGUID && g_GameSettings.playerAI.get(this.playerIndex))
+		{
+			g_GameSettings.playerAI.setAI(this.playerIndex, undefined);
+			this.gameSettingsControl.setNetworkInitAttributes();
+		}
 		this.render();
 	}
 
@@ -121,6 +99,7 @@ PlayerSettingControls.PlayerAssignment = class PlayerAssignment extends GameSett
 	rebuildList()
 	{
 		Engine.ProfileStart("updatePlayerAssignmentsList");
+		// TODO: this particular bit is done for each row, which is unnecessarily inefficient.
 		this.playerItems = sortGUIDsByPlayerID().map(
 			this.clientItemFactory.createItem.bind(this.clientItemFactory));
 		this.values = prepareForDropdown([
@@ -129,8 +108,10 @@ PlayerSettingControls.PlayerAssignment = class PlayerAssignment extends GameSett
 			this.unassignedItem
 		]);
 
+		let selected = this.dropdown.list_data?.[this.dropdown.selected];
 		this.dropdown.list = this.values.Caption;
 		this.dropdown.list_data = this.values.Value;
+		this.setSelectedValue(selected);
 		Engine.ProfileStop();
 	}
 
@@ -154,9 +135,6 @@ PlayerSettingControls.PlayerAssignment.prototype.Tooltip =
 
 PlayerSettingControls.PlayerAssignment.prototype.AutocompleteOrder = 100;
 
-PlayerSettingControls.PlayerAssignment.prototype.ConfigAssignPlayers =
-	"gui.gamesetup.assignplayers";
-
 {
 	PlayerAssignmentItem.Client = class
 	{

binaries/data/mods/public/gui/gamesetup/Pages/GameSetupPage/GameSettings/Single/Checkboxes/LockedTeams.js

@@ -4,7 +4,7 @@ GameSettingControls.LockedTeams = class LockedTeams extends GameSettingControlCh
 	{
 		super(...args);
 		g_GameSettings.map.watch(() => this.render(), ["type"]);
-		g_GameSettings.rating.watch(() => this.render(), ["available", "enabled"]);
+		g_GameSettings.lockedTeams.watch(() => this.render(), ["available", "enabled"]);
 		this.render();
 	}