From 8af0689b5f357630e4b6eb26ef44d58a0513c9e2 Mon Sep 17 00:00:00 2001 From: wraitii Date: Sun, 6 Jun 2021 08:02:28 +0000 Subject: [PATCH] Fix gamesetup player assignment issue when joining Switch some logic from C++ to JS in PREGAME for player assignments. Refs #3049 Fixes #6204 Reported by: Imarok Tested By: Imarok Differential Revision: https://code.wildfiregames.com/D4092 This was SVN commit r25699. --- .../PlayerAssignmentsController.js | 45 ++++++++++++++----- source/network/NetServer.cpp | 42 ++++++++--------- 2 files changed, 56 insertions(+), 31 deletions(-) diff --git a/binaries/data/mods/public/gui/gamesetup/Controllers/PlayerAssignmentsController.js b/binaries/data/mods/public/gui/gamesetup/Controllers/PlayerAssignmentsController.js index ffc2414591..f420a547c3 100644 --- a/binaries/data/mods/public/gui/gamesetup/Controllers/PlayerAssignmentsController.js +++ b/binaries/data/mods/public/gui/gamesetup/Controllers/PlayerAssignmentsController.js @@ -25,6 +25,10 @@ class PlayerAssignmentsController }; } + // Keep a list of last assigned slot for each player, so we can try to re-assign them + // if they disconnect/rejoin. + this.lastAssigned = {}; + g_GameSettings.playerCount.watch(() => this.unassignInvalidPlayers(), ["nbPlayers"]); setupWindow.registerLoadHandler(this.onLoad.bind(this)); @@ -97,24 +101,41 @@ class PlayerAssignmentsController // Assign the client (or only buddies if prefered) to a free slot if (newGUID != Engine.GetPlayerGUID()) { - let assignOption = Engine.ConfigDB_GetValue("user", this.ConfigAssignPlayers); + const 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) + const possibleSlots = [...Array(g_GameSettings.playerCount.nbPlayers).keys()].map(i => i + 1); + + let slot; + const newName = newAssignments[newGUID].name; + // First check if we know them and try to give them their old assignment back. + if (this.lastAssigned[newName] > 0 && this.lastAssigned[newName] <= g_GameSettings.playerCount.nbPlayers) + { + let free = true; + for (const guid in newAssignments) + if (newAssignments[guid].player === this.lastAssigned[newName]) + { + free = false; + break; + } + if (free) + slot = this.lastAssigned[newName]; + } + if (!slot) + slot = possibleSlots.find(i => { + for (const guid in newAssignments) + if (newAssignments[guid].player == i) + return false; + return true; + }); + if (slot === undefined) return; - this.assignClient(newGUID, firstFreeSlot + 1); + this.assignClient(newGUID, slot); } /** @@ -123,7 +144,9 @@ class PlayerAssignmentsController updatePlayerAssignments() { Engine.ProfileStart("updatePlayerAssignments"); - for (let handler of this.playerAssignmentsChangeHandlers) + for (const guid in g_PlayerAssignments) + this.lastAssigned[g_PlayerAssignments[guid].name] = g_PlayerAssignments[guid].player; + for (const handler of this.playerAssignmentsChangeHandlers) handler(); Engine.ProfileStop(); } diff --git a/source/network/NetServer.cpp b/source/network/NetServer.cpp index 9cf1eff7d0..6e68a4b6c6 100644 --- a/source/network/NetServer.cpp +++ b/source/network/NetServer.cpp @@ -770,34 +770,36 @@ void CNetServerWorker::AddPlayer(const CStr& guid, const CStrW& name) usedIDs.insert(p.second.m_PlayerID); // If the player is rejoining after disconnecting, try to give them - // back their old player ID + // back their old player ID. Don't do this in pregame however, + // as that ID might be invalid for various reasons. i32 playerID = -1; - // Try to match GUID first - for (PlayerAssignmentMap::iterator it = m_PlayerAssignments.begin(); it != m_PlayerAssignments.end(); ++it) + if (m_State != SERVER_STATE_UNCONNECTED && m_State != SERVER_STATE_PREGAME) { - if (!it->second.m_Enabled && it->first == guid && usedIDs.find(it->second.m_PlayerID) == usedIDs.end()) + // Try to match GUID first + for (PlayerAssignmentMap::iterator it = m_PlayerAssignments.begin(); it != m_PlayerAssignments.end(); ++it) { - playerID = it->second.m_PlayerID; - m_PlayerAssignments.erase(it); // delete the old mapping, since we've got a new one now - goto found; + if (!it->second.m_Enabled && it->first == guid && usedIDs.find(it->second.m_PlayerID) == usedIDs.end()) + { + playerID = it->second.m_PlayerID; + m_PlayerAssignments.erase(it); // delete the old mapping, since we've got a new one now + goto found; + } + } + + // Try to match username next + 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()) + { + playerID = it->second.m_PlayerID; + m_PlayerAssignments.erase(it); // delete the old mapping, since we've got a new one now + goto found; + } } } - // Try to match username next - 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()) - { - playerID = it->second.m_PlayerID; - m_PlayerAssignments.erase(it); // delete the old mapping, since we've got a new one now - goto found; - } - } - - // Otherwise leave the player ID as -1 (observer) and let gamesetup change it as needed. - found: PlayerAssignment assignment; assignment.m_Enabled = true;