From 371eac070f7ad5e890874d7df07cf56bcfdac68c Mon Sep 17 00:00:00 2001
From: Ivan Savenko <saven.ivan@gmail.com>
Date: Fri, 4 Oct 2024 12:48:50 +0000
Subject: [PATCH] Fixes for player disconnection handling

- Fixed lack of notification if player disconnects via connection loss,
e.g. app crash / network going down
- Replaced notification via chat message with notification via info window
---
 Mods/vcmi/config/vcmi/english.json |  1 +
 server/CGameHandler.cpp            | 24 +++++++++++++++---
 server/CVCMIServer.cpp             | 40 +++++++++++++++++-------------
 server/NetPacksLobbyServer.cpp     |  1 +
 4 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/Mods/vcmi/config/vcmi/english.json b/Mods/vcmi/config/vcmi/english.json
index b62ee25d2..49e940526 100644
--- a/Mods/vcmi/config/vcmi/english.json
+++ b/Mods/vcmi/config/vcmi/english.json
@@ -143,6 +143,7 @@
 	"vcmi.client.errors.invalidMap" : "{Invalid map or campaign}\n\nFailed to start game! Selected map or campaign might be invalid or corrupted. Reason:\n%s",
 	"vcmi.client.errors.missingCampaigns" : "{Missing data files}\n\nCampaigns data files were not found! You may be using incomplete or corrupted Heroes 3 data files. Please reinstall game data.",
 	"vcmi.server.errors.disconnected" : "{Network Error}\n\nConnection to game server has been lost!",
+	"vcmi.server.errors.playerLeft" : "{Player Left}\n\n%s player have disconnected from the game!", //%s -> player color
 	"vcmi.server.errors.existingProcess" : "Another VCMI server process is running. Please terminate it before starting a new game.",
 	"vcmi.server.errors.modsToEnable"    : "{Following mods are required}",
 	"vcmi.server.errors.modsToDisable"   : "{Following mods must be disabled}",
diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp
index 383e3bbe3..7c282eadd 100644
--- a/server/CGameHandler.cpp
+++ b/server/CGameHandler.cpp
@@ -431,10 +431,28 @@ void CGameHandler::handleClientDisconnection(std::shared_ptr<CConnection> c)
 			continue;
 		
 		auto playerConnection = vstd::find(playerConnections.second, c);
-		if(playerConnection != playerConnections.second.end())
+		if(playerConnection == playerConnections.second.end())
+			continue;
+
+		logGlobal->trace("Player %s disconnected. Notifying remaining players", playerId.toString());
+
+		// this player have left the game - broadcast infowindow to all in-game players
+		for (auto i = gs->players.cbegin(); i!=gs->players.cend(); i++)
 		{
-			std::string messageText = boost::str(boost::format("%s (cid %d) was disconnected") % playerSettings->name % c->connectionID);
-			playerMessages->broadcastMessage(playerId, messageText);
+			if (i->first == playerId)
+				continue;
+
+			if (getPlayerState(i->first)->status != EPlayerStatus::INGAME)
+				continue;
+
+			logGlobal->trace("Notifying player %s", i->first);
+
+			InfoWindow out;
+			out.player = i->first;
+			out.text.appendTextID("vcmi.server.errors.playerLeft");
+			out.text.replaceName(playerId);
+			out.components.emplace_back(ComponentType::FLAG, playerId);
+			sendAndApply(&out);
 		}
 	}
 }
diff --git a/server/CVCMIServer.cpp b/server/CVCMIServer.cpp
index 0d8cd9280..7f52c801e 100644
--- a/server/CVCMIServer.cpp
+++ b/server/CVCMIServer.cpp
@@ -297,25 +297,19 @@ void CVCMIServer::onDisconnected(const std::shared_ptr<INetworkConnection> & con
 	logNetwork->error("Network error receiving a pack. Connection has been closed");
 
 	std::shared_ptr<CConnection> c = findConnection(connection);
-	if (!c)
-		return; // player have already disconnected via clientDisconnected call
 
-	vstd::erase(activeConnections, c);
-
-	if(activeConnections.empty() || hostClientId == c->connectionID)
+	// player may have already disconnected via clientDisconnected call
+	if (c)
 	{
-		setState(EServerState::SHUTDOWN);
-		return;
-	}
+		//clientDisconnected(c);
 
-	if(gh && getState() == EServerState::GAMEPLAY)
-	{
-		gh->handleClientDisconnection(c);
-
-		auto lcd = std::make_unique<LobbyClientDisconnected>();
-		lcd->c = c;
-		lcd->clientId = c->connectionID;
-		handleReceivedPack(std::move(lcd));
+		if(gh && getState() == EServerState::GAMEPLAY)
+		{
+			auto lcd = std::make_unique<LobbyClientDisconnected>();
+			lcd->c = c;
+			lcd->clientId = c->connectionID;
+			handleReceivedPack(std::move(lcd));
+		}
 	}
 }
 
@@ -434,9 +428,21 @@ void CVCMIServer::clientConnected(std::shared_ptr<CConnection> c, std::vector<st
 
 void CVCMIServer::clientDisconnected(std::shared_ptr<CConnection> connection)
 {
-	connection->getConnection()->close();
+	assert(vstd::contains(activeConnections, connection));
+	logGlobal->trace("Received disconnection request");
 	vstd::erase(activeConnections, connection);
 
+	if(activeConnections.empty() || hostClientId == connection->connectionID)
+	{
+		setState(EServerState::SHUTDOWN);
+		return;
+	}
+
+	if(gh && getState() == EServerState::GAMEPLAY)
+	{
+		gh->handleClientDisconnection(connection);
+	}
+
 //	PlayerReinitInterface startAiPack;
 //	startAiPack.playerConnectionId = PlayerSettings::PLAYER_AI;
 //
diff --git a/server/NetPacksLobbyServer.cpp b/server/NetPacksLobbyServer.cpp
index 4cf091ac0..17df7c364 100644
--- a/server/NetPacksLobbyServer.cpp
+++ b/server/NetPacksLobbyServer.cpp
@@ -108,6 +108,7 @@ void ClientPermissionsCheckerNetPackVisitor::visitLobbyClientDisconnected(LobbyC
 
 void ApplyOnServerNetPackVisitor::visitLobbyClientDisconnected(LobbyClientDisconnected & pack)
 {
+	pack.c->getConnection()->close();
 	srv.clientDisconnected(pack.c);
 	result = true;
 }