From dfc7567561adc98ae40246f5628de3ec5a5b7726 Mon Sep 17 00:00:00 2001
From: JoshuaJB <joshuajb@wildfiregames.com>
Date: Mon, 19 May 2014 16:02:42 +0000
Subject: [PATCH] Fix multiple observers on multiplayer games to work more
 reliably. Fixes #2529

This was SVN commit r15171.
---
 .../mods/public/gui/gamesetup/gamesetup.js    | 14 +++++++---
 source/network/NetServer.cpp                  | 28 ++++++-------------
 2 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/binaries/data/mods/public/gui/gamesetup/gamesetup.js b/binaries/data/mods/public/gui/gamesetup/gamesetup.js
index f0e49b6cfc..fe61b38b96 100644
--- a/binaries/data/mods/public/gui/gamesetup/gamesetup.js
+++ b/binaries/data/mods/public/gui/gamesetup/gamesetup.js
@@ -26,7 +26,7 @@ var g_IsNetworked;
 // Is this user in control of game settings (i.e. is a network server, or offline player)
 var g_IsController;
 
-//Server name, if user is a server, connected to the multiplayer lobby
+// Server name, if user is a server, connected to the multiplayer lobby
 var g_ServerName;
 
 // Are we currently updating the GUI in response to network messages instead of user input
@@ -73,6 +73,9 @@ var g_NavalWarning = "\n\n" + sprintf(
 	{ warning: "[font=\"sans-bold-12\"][color=\"orange\"]" + translate("Warning:") + "[/color][/font]" }
 );
 
+// Current number of assigned human players.
+var g_AssignedCount = 0;
+
 // To prevent the display locking up while we load the map metadata,
 // we'll start with a 'loading' message and switch to the main screen in the
 // tick handler
@@ -436,6 +439,9 @@ function handleNetMessage(message)
 		{
 			if (! g_PlayerAssignments[host])
 			{
+				// If we have extra player slots and we are the controller, give the player an ID.
+				if (g_IsController && message.hosts[host].player === -1 && g_AssignedCount < g_GameAttributes.settings.PlayerData.length)
+					Engine.AssignNetworkPlayer(g_AssignedCount + 1, host);
 				addChatMessage({ "type": "connect", "username": message.hosts[host].name });
 				newPlayer = host;
 			}
@@ -1329,7 +1335,7 @@ function updatePlayerList()
 	var assignments = [];
 	var aiAssignments = {};
 	var noAssignment;
-	var assignedCount = 0;
+	g_AssignedCount = 0;
 
 	for (var guid in g_PlayerAssignments)
 	{
@@ -1342,12 +1348,12 @@ function updatePlayerList()
 		assignments[player] = hostID;
 
 		if (player != -1)
-			assignedCount++;
+			g_AssignedCount++;
 	}
 
 	// Only enable start button if we have enough assigned players
 	if (g_IsController)
-		Engine.GetGUIObjectByName("startGame").enabled = (assignedCount > 0);
+		Engine.GetGUIObjectByName("startGame").enabled = (g_AssignedCount > 0);
 
 	for each (var ai in g_AIs)
 	{
diff --git a/source/network/NetServer.cpp b/source/network/NetServer.cpp
index 47072aa8e6..a69b50b872 100644
--- a/source/network/NetServer.cpp
+++ b/source/network/NetServer.cpp
@@ -618,7 +618,6 @@ void CNetServerWorker::AddPlayer(const CStr& guid, const CStrW& name)
 	// back their old player ID
 
 	i32 playerID = -1;
-	bool foundPlayerID = false;
 
 	// Try to match GUID first
 	for (PlayerAssignmentMap::iterator it = m_PlayerAssignments.begin(); it != m_PlayerAssignments.end(); ++it)
@@ -626,36 +625,25 @@ void CNetServerWorker::AddPlayer(const CStr& guid, const CStrW& name)
 		if (!it->second.m_Enabled && it->first == guid && usedIDs.find(it->second.m_PlayerID) == usedIDs.end())
 		{
 			playerID = it->second.m_PlayerID;
-			foundPlayerID = true;
 			m_PlayerAssignments.erase(it); // delete the old mapping, since we've got a new one now
-			break;
+			goto found;
 		}
 	}
 
 	// Try to match username next
-	if (!foundPlayerID)
+	for (PlayerAssignmentMap::iterator it = m_PlayerAssignments.begin(); it != m_PlayerAssignments.end(); ++it)
 	{
-		for (PlayerAssignmentMap::iterator it = m_PlayerAssignments.begin(); it != m_PlayerAssignments.end(); ++it)
+		if (!it->second.m_Enabled && it->second.m_Name == name && usedIDs.find(it->second.m_PlayerID) == usedIDs.end())
 		{
-			if (!it->second.m_Enabled && it->second.m_Name == name && usedIDs.find(it->second.m_PlayerID) == usedIDs.end())
-			{
-				playerID = it->second.m_PlayerID;
-				foundPlayerID = true;
-				m_PlayerAssignments.erase(it); // delete the old mapping, since we've got a new one now
-				break;
-			}
+			playerID = it->second.m_PlayerID;
+			m_PlayerAssignments.erase(it); // delete the old mapping, since we've got a new one now
+			goto found;
 		}
 	}
 
-	// Otherwise pick the first free player ID
-	if (!foundPlayerID)
-	{
-		for (playerID = 1; usedIDs.find(playerID) != usedIDs.end(); ++playerID)
-		{
-			// (do nothing)
-		}
-	}
+	// Otherwise leave the player ID as -1 (observer) and let gamesetup change it as needed.
 
+found:
 	PlayerAssignment assignment;
 	assignment.m_Enabled = true;
 	assignment.m_Name = name;