From d631186dca6b0561f4143d37f5427760a38ac8cd Mon Sep 17 00:00:00 2001 From: George King <98261225+GeorgeK1ng@users.noreply.github.com> Date: Thu, 7 May 2026 12:19:58 +0200 Subject: [PATCH] Address comments --- client/CServerHandler.cpp | 8 ++++--- client/LobbyClientNetPackVisitors.h | 1 + client/NetPacksLobbyClient.cpp | 31 ++++++++++++++++---------- client/lobby/CLobbyScreen.cpp | 10 +-------- client/lobby/CLobbyScreen.h | 1 - client/lobby/SelectionTab.cpp | 8 +++++-- client/lobby/SelectionTab.h | 2 ++ server/CGameHandler.cpp | 24 +++++++++++++++++++- server/CGameHandler.h | 3 ++- server/CVCMIServer.cpp | 34 ++++++++++------------------- 10 files changed, 70 insertions(+), 52 deletions(-) diff --git a/client/CServerHandler.cpp b/client/CServerHandler.cpp index bb518dc10..3eaed44d9 100644 --- a/client/CServerHandler.cpp +++ b/client/CServerHandler.cpp @@ -344,11 +344,13 @@ bool CServerHandler::isGuest() const bool CServerHandler::hasRemoteClientInLobby() const { - std::set connectedClients; for(const auto & playerEntry : playerNames) - connectedClients.insert(playerEntry.second.connection); + { + if(playerEntry.second.connection != hostClientId) + return true; + } - return connectedClients.size() > 1; + return false; } const std::string & CServerHandler::getLocalHostname() const diff --git a/client/LobbyClientNetPackVisitors.h b/client/LobbyClientNetPackVisitors.h index fb44b86a2..41bdde21c 100644 --- a/client/LobbyClientNetPackVisitors.h +++ b/client/LobbyClientNetPackVisitors.h @@ -53,6 +53,7 @@ public: { } + void visitLobbyClientConnected(LobbyClientConnected & pack) override; void visitLobbyClientDisconnected(LobbyClientDisconnected & pack) override; void visitLobbyChatMessage(LobbyChatMessage & pack) override; void visitLobbyGuiAction(LobbyGuiAction & pack) override; diff --git a/client/NetPacksLobbyClient.cpp b/client/NetPacksLobbyClient.cpp index 139931143..e1d2f8b0b 100644 --- a/client/NetPacksLobbyClient.cpp +++ b/client/NetPacksLobbyClient.cpp @@ -51,10 +51,11 @@ void ApplyOnLobbyHandlerNetPackVisitor::visitLobbyQuickLoadGame(LobbyQuickLoadGa void ApplyOnLobbyHandlerNetPackVisitor::visitLobbyClientConnected(LobbyClientConnected & pack) { - result = false; + const bool isLocalClient = pack.uuid == handler.logicConnection->uuid; + result = !isLocalClient; // Check if it's LobbyClientConnected for our client - if(pack.uuid == handler.logicConnection->uuid) + if(isLocalClient) { handler.logicConnection->setSerializationVersion(pack.version); handler.logicConnection->connectionID = pack.clientId; @@ -94,22 +95,28 @@ void ApplyOnLobbyHandlerNetPackVisitor::visitLobbyClientConnected(LobbyClientCon } } -void ApplyOnLobbyHandlerNetPackVisitor::visitLobbyClientDisconnected(LobbyClientDisconnected & pack) +void ApplyOnLobbyHandlerNetPackVisitor::visitLobbyClientDisconnected(LobbyClientDisconnected &) { - if(pack.clientId != handler.logicConnection->connectionID) - { - result = false; +} + +void ApplyOnLobbyScreenNetPackVisitor::visitLobbyClientConnected(LobbyClientConnected & pack) +{ + if(!lobby || pack.clientId == handler.logicConnection->connectionID) return; - } } void ApplyOnLobbyScreenNetPackVisitor::visitLobbyClientDisconnected(LobbyClientDisconnected & pack) { - if(auto w = ENGINE->windows().topWindow()) - ENGINE->windows().popWindow(w); - - if(ENGINE->windows().count() > 0) - ENGINE->windows().popWindows(1); + if(pack.clientId == handler.logicConnection->connectionID) + { + if(auto w = ENGINE->windows().topWindow()) + ENGINE->windows().popWindow(w); + + if(ENGINE->windows().count() > 0) + ENGINE->windows().popWindows(1); + return; + } + } void ApplyOnLobbyScreenNetPackVisitor::visitLobbyChatMessage(LobbyChatMessage & pack) diff --git a/client/lobby/CLobbyScreen.cpp b/client/lobby/CLobbyScreen.cpp index 0920961e3..40658d9c7 100644 --- a/client/lobby/CLobbyScreen.cpp +++ b/client/lobby/CLobbyScreen.cpp @@ -219,15 +219,6 @@ void CLobbyScreen::toggleTab(std::shared_ptr tab) CSelectionBase::toggleTab(tab); } -void CLobbyScreen::tick(uint32_t msPassed) -{ - CSelectionBase::tick(msPassed); - - updateHostLobbyChatState(); - if(curTab != tabBattleOnlyMode) - updateStartButtonState(); -} - void CLobbyScreen::start(bool campaign) { if(curTab == tabBattleOnlyMode) @@ -336,6 +327,7 @@ void CLobbyScreen::updateAfterStateChange() updateHostLobbyChatState(); const bool shouldFilterByPlayerCount = screenType == ESelectionScreen::newGame && GAME->server().loadMode == ELoadMode::MULTI; const size_t requiredHumanPlayers = shouldFilterByPlayerCount ? std::max(2, GAME->server().playerNames.size()) : 0; + tabSel->setRequiredHumanPlayers(requiredHumanPlayers); if(!compatibilityFilterInitialized || (shouldFilterByPlayerCount && requiredHumanPlayers != lastRequiredHumanPlayers)) { diff --git a/client/lobby/CLobbyScreen.h b/client/lobby/CLobbyScreen.h index feb050a7d..dba808666 100644 --- a/client/lobby/CLobbyScreen.h +++ b/client/lobby/CLobbyScreen.h @@ -31,7 +31,6 @@ public: CLobbyScreen(ESelectionScreen type, bool hideScreen = false); ~CLobbyScreen(); void toggleTab(std::shared_ptr tab) final; - void tick(uint32_t msPassed) override; void start(bool campaign); void startCampaign(); void startScenario(bool allowOnlyAI = false); diff --git a/client/lobby/SelectionTab.cpp b/client/lobby/SelectionTab.cpp index 405519a1c..092bc8a77 100644 --- a/client/lobby/SelectionTab.cpp +++ b/client/lobby/SelectionTab.cpp @@ -1012,8 +1012,12 @@ bool SelectionTab::isMapCompatibleWithLobbyPlayerCount(const ElementInfo & info) size_t SelectionTab::getRequiredHumanPlayers() const { - const size_t minimumPlayers = (GAME->server().loadMode == ELoadMode::MULTI || GAME->server().hotseatMode) ? 2 : 1; - return std::max(minimumPlayers, GAME->server().playerNames.size()); + return requiredHumanPlayers; +} + +void SelectionTab::setRequiredHumanPlayers(size_t players) +{ + requiredHumanPlayers = players; } void SelectionTab::parseMaps(const std::unordered_set & files) diff --git a/client/lobby/SelectionTab.h b/client/lobby/SelectionTab.h index 478e7f4a2..8488f7a8e 100644 --- a/client/lobby/SelectionTab.h +++ b/client/lobby/SelectionTab.h @@ -91,6 +91,7 @@ public: int currentMapSizeFilter = 0; bool showRandom; std::string lastCompatibilityNotice; + size_t requiredHumanPlayers = 1; std::shared_ptr inputName; @@ -115,6 +116,7 @@ public: void selectFileName(std::string fname); void selectNewestFile(); std::shared_ptr getSelectedMapInfo() const; + void setRequiredHumanPlayers(size_t players); void rememberCurrentSelection(); void restoreLastSelection(); diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index e417010f9..da64b7b12 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -434,7 +434,7 @@ void CGameHandler::changeSecSkill(const CGHeroInstance * hero, SecondarySkill wh } -void CGameHandler::handleClientDisconnection(GameConnectionID connectionID) +void CGameHandler::handleClientDisconnection(GameConnectionID connectionID, const std::vector & disconnectedPlayerIds) { if(gameServer().getState() == EServerState::SHUTDOWN || !gameState().getStartInfo()) { @@ -457,6 +457,28 @@ void CGameHandler::handleClientDisconnection(GameConnectionID connectionID) remainingPlayers.push_back(player.first); } + if(disconnectedPlayers.empty() && !disconnectedPlayerIds.empty()) + { + for(const auto & player : gameState().players) + { + if (gameInfo().getPlayerState(player.first)->status != EPlayerStatus::INGAME) + continue; + + const auto playerSettings = gameInfo().getPlayerSettings(player.first); + if(!playerSettings) + continue; + + for(const auto disconnectedPlayerId : disconnectedPlayerIds) + { + if(vstd::contains(playerSettings->connectedPlayerIDs, disconnectedPlayerId)) + { + disconnectedPlayers.push_back(player.first); + break; + } + } + } + } + for (const auto & inGamePlayer : remainingPlayers) { for (const auto & lostPlayer : disconnectedPlayers) diff --git a/server/CGameHandler.h b/server/CGameHandler.h index ab2d3466e..877391bbe 100644 --- a/server/CGameHandler.h +++ b/server/CGameHandler.h @@ -17,6 +17,7 @@ #include "../lib/gameState/GameStatistics.h" #include "../lib/networkPacks/PacksForServer.h" #include "../lib/serializer/GameConnectionID.h" +#include "../lib/serializer/PlayerConnectionID.h" VCMI_LIB_NAMESPACE_BEGIN @@ -205,7 +206,7 @@ public: ////////////////////////////////////////////////////////////////////////// void init(StartInfo *si, Load::ProgressAccumulator & progressTracking); - void handleClientDisconnection(GameConnectionID connectionI); + void handleClientDisconnection(GameConnectionID connectionID, const std::vector & disconnectedPlayerIds = {}); void handleReceivedPack(GameConnectionID connectionId, CPackForServer & pack); bool hasPlayerAt(PlayerColor player, GameConnectionID connectionId) const; bool hasBothPlayersAtSameConnection(PlayerColor left, PlayerColor right) const; diff --git a/server/CVCMIServer.cpp b/server/CVCMIServer.cpp index fad141d0d..2f550a9d7 100644 --- a/server/CVCMIServer.cpp +++ b/server/CVCMIServer.cpp @@ -520,26 +520,20 @@ void CVCMIServer::clientDisconnected(std::shared_ptr connection) vstd::erase(activeConnections, connection); std::vector disconnectedPlayerIds; - std::vector disconnectedPlayerNames; for(const auto & playerEntry : playerNames) { - if(playerEntry.second.connection == connection->connectionID) - { - disconnectedPlayerIds.push_back(playerEntry.first); - disconnectedPlayerNames.push_back(playerEntry.second.name); - } - } + if(playerEntry.second.connection != connection->connectionID) + continue; - for(const auto & playerName : disconnectedPlayerNames) - { - logNetwork->info("Player disconnected from lobby: name='%s', connectionId=%d", playerName, static_cast(connection->connectionID)); + disconnectedPlayerIds.push_back(playerEntry.first); + logNetwork->info("Player disconnected from lobby: name='%s', connectionId=%d", playerEntry.second.name, static_cast(connection->connectionID)); MetaString disconnectMessage; disconnectMessage.appendTextID("vcmi.lobby.system.playerDisconnected"); - disconnectMessage.replaceRawString(playerName); + disconnectMessage.replaceRawString(playerEntry.second.name); announceTxt(disconnectMessage); } - if(disconnectedPlayerNames.empty()) + if(disconnectedPlayerIds.empty()) logNetwork->info("Connection %d disconnected from lobby with no mapped player names", static_cast(connection->connectionID)); if(activeConnections.empty() || hostClientId == connection->connectionID) @@ -550,23 +544,17 @@ void CVCMIServer::clientDisconnected(std::shared_ptr connection) if(gh && getState() == EServerState::GAMEPLAY) { - gh->handleClientDisconnection(connection->connectionID); + gh->handleClientDisconnection(connection->connectionID, disconnectedPlayerIds); } for(const auto & playerId : disconnectedPlayerIds) playerNames.erase(playerId); - if(!disconnectedPlayerIds.empty()) + for(auto & playerInfoEntry : si->playerInfos) { - for(auto & playerInfoEntry : si->playerInfos) - { - auto & connectedPlayerIDs = playerInfoEntry.second.connectedPlayerIDs; - for(const auto & playerId : disconnectedPlayerIds) - connectedPlayerIDs.erase(playerId); - - if(connectedPlayerIDs.empty()) - setPlayerConnectedId(playerInfoEntry.second, PlayerConnectionID::PLAYER_AI); - } + auto & connectedPlayerIDs = playerInfoEntry.second.connectedPlayerIDs; + for(const auto & playerId : disconnectedPlayerIds) + connectedPlayerIDs.erase(playerId); } }