Fixed handling of match server crash

Mods/vcmi/config/vcmi/english.json

@@ -63,7 +63,6 @@
 	"vcmi.mainMenu.serverClosing" : "Closing...",
 	"vcmi.mainMenu.hostTCP" : "Host TCP/IP game",
 	"vcmi.mainMenu.joinTCP" : "Join TCP/IP game",
-	"vcmi.mainMenu.playerName" : "Player",
 	
 	"vcmi.lobby.filepath" : "File path",
 	"vcmi.lobby.creationDate" : "Creation date",
@@ -94,10 +93,10 @@
 
 	"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.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}",
-	"vcmi.server.confirmReconnect"       : "Do you want to reconnect to the last session?",
 	"vcmi.server.errors.modNoDependency" : "Failed to load mod {'%s'}!\n It depends on mod {'%s'} which is not active!\n",
 	"vcmi.server.errors.modConflict" : "Failed to load mod {'%s'}!\n Conflicts with active mod {'%s'}!\n",
 	"vcmi.server.errors.unknownEntity" : "Failed to load save! Unknown entity '%s' found in saved game! Save may not be compatible with currently installed version of mods!",

client/CPlayerInterface.cpp

@@ -1870,14 +1870,9 @@ void CPlayerInterface::proposeLoadingGame()
 		CGI->generaltexth->allTexts[68],
 		[]()
 		{
-			GH.dispatchMainThread(
-				[]()
-				{
-					CSH->endGameplay();
-					GH.defActionsDef = 63;
-					CMM->menu->switchToTab("load");
-				}
-			);
+			CSH->endGameplay();
+			GH.defActionsDef = 63;
+			CMM->menu->switchToTab("load");
 		},
 		nullptr
 	);

client/CServerHandler.cpp

@@ -144,6 +144,7 @@ CServerHandler::CServerHandler()
 	, applier(std::make_unique<CApplier<CBaseForLobbyApply>>())
 	, threadNetwork(&CServerHandler::threadRunNetwork, this)
 	, state(EClientState::NONE)
+	, serverPort(0)
 	, campaignStateToSend(nullptr)
 	, screenType(ESelectionScreen::unknown)
 	, serverMode(EServerMode::NONE)
@@ -165,7 +166,7 @@ void CServerHandler::threadRunNetwork()
 void CServerHandler::resetStateForLobby(EStartMode mode, ESelectionScreen screen, EServerMode newServerMode, const std::vector<std::string> & names)
 {
 	hostClientId = -1;
-	state = EClientState::NONE;
+	setState(EClientState::NONE);
 	serverMode = newServerMode;
 	mapToStart = nullptr;
 	th = std::make_unique<CStopWatch>();
@@ -263,7 +264,7 @@ void CServerHandler::startLocalServerAndConnect(bool connectToLobby)
 void CServerHandler::connectToServer(const std::string & addr, const ui16 port)
 {
 	logNetwork->info("Establishing connection to %s:%d...", addr, port);
-	state = EClientState::CONNECTING;
+	setState(EClientState::CONNECTING);
 	serverHostname = addr;
 	serverPort = port;
 
@@ -281,7 +282,7 @@ void CServerHandler::connectToServer(const std::string & addr, const ui16 port)
 
 void CServerHandler::onConnectionFailed(const std::string & errorMessage)
 {
-	assert(state == EClientState::CONNECTING);
+	assert(getState() == EClientState::CONNECTING);
 	boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
 
 	if (isServerLocal())
@@ -293,7 +294,7 @@ void CServerHandler::onConnectionFailed(const std::string & errorMessage)
 	else
 	{
 		// remote server refused connection - show error message
-		state = EClientState::CONNECTION_FAILED;
+		setState(EClientState::NONE);
 		CInfoWindow::showInfoDialog(CGI->generaltexth->translate("vcmi.mainMenu.serverConnectionFailed"), {});
 	}
 }
@@ -302,7 +303,7 @@ void CServerHandler::onTimer()
 {
 	boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
 
-	if(state == EClientState::CONNECTION_CANCELLED)
+	if(getState() == EClientState::CONNECTION_CANCELLED)
 	{
 		logNetwork->info("Connection aborted by player!");
 		return;
@@ -314,7 +315,7 @@ void CServerHandler::onTimer()
 
 void CServerHandler::onConnectionEstablished(const NetworkConnectionPtr & netConnection)
 {
-	assert(state == EClientState::CONNECTING);
+	assert(getState() == EClientState::CONNECTING);
 
 	boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
 
@@ -361,6 +362,16 @@ ui8 CServerHandler::myFirstId() const
 	return clientFirstId(c->connectionID);
 }
 
+EClientState CServerHandler::getState() const
+{
+	return state;
+}
+
+void CServerHandler::setState(EClientState newState)
+{
+	state = newState;
+}
+
 bool CServerHandler::isServerLocal() const
 {
 	return threadRunLocalServer.joinable();
@@ -418,13 +429,13 @@ void CServerHandler::sendClientConnecting() const
 void CServerHandler::sendClientDisconnecting()
 {
 	// FIXME: This is workaround needed to make sure client not trying to sent anything to non existed server
-	if(state == EClientState::DISCONNECTING)
+	if(getState() == EClientState::DISCONNECTING)
 	{
 		assert(0);
 		return;
 	}
 
-	state = EClientState::DISCONNECTING;
+	setState(EClientState::DISCONNECTING);
 	mapToStart = nullptr;
 	LobbyClientDisconnected lcd;
 	lcd.clientId = c->connectionID;
@@ -439,13 +450,14 @@ void CServerHandler::sendClientDisconnecting()
 		logNetwork->info("Sent leaving signal to the server");
 	}
 	sendLobbyPack(lcd);
+	networkConnection->close();
 	networkConnection.reset();
 	c.reset();
 }
 
 void CServerHandler::setCampaignState(std::shared_ptr<CampaignState> newCampaign)
 {
-	state = EClientState::LOBBY_CAMPAIGN;
+	setState(EClientState::LOBBY_CAMPAIGN);
 	LobbySetCampaign lsc;
 	lsc.ourCampaign = newCampaign;
 	sendLobbyPack(lsc);
@@ -453,7 +465,7 @@ void CServerHandler::setCampaignState(std::shared_ptr<CampaignState> newCampaign
 
 void CServerHandler::setCampaignMap(CampaignScenarioID mapId) const
 {
-	if(state == EClientState::GAMEPLAY) // FIXME: UI shouldn't sent commands in first place
+	if(getState() == EClientState::GAMEPLAY) // FIXME: UI shouldn't sent commands in first place
 		return;
 
 	LobbySetCampaignMap lscm;
@@ -463,7 +475,7 @@ void CServerHandler::setCampaignMap(CampaignScenarioID mapId) const
 
 void CServerHandler::setCampaignBonus(int bonusId) const
 {
-	if(state == EClientState::GAMEPLAY) // FIXME: UI shouldn't sent commands in first place
+	if(getState() == EClientState::GAMEPLAY) // FIXME: UI shouldn't sent commands in first place
 		return;
 
 	LobbySetCampaignBonus lscb;
@@ -673,7 +685,7 @@ void CServerHandler::startGameplay(VCMI_LIB_WRAP_NAMESPACE(CGameState) * gameSta
 	}
 	// After everything initialized we can accept CPackToClient netpacks
 	c->enterGameplayConnectionMode(client->gameState());
-	state = EClientState::GAMEPLAY;
+	setState(EClientState::GAMEPLAY);
 }
 
 void CServerHandler::endGameplay()
@@ -780,7 +792,7 @@ int CServerHandler::howManyPlayerInterfaces()
 
 ELoadMode CServerHandler::getLoadMode()
 {
-	if(loadMode != ELoadMode::TUTORIAL && state == EClientState::GAMEPLAY)
+	if(loadMode != ELoadMode::TUTORIAL && getState() == EClientState::GAMEPLAY)
 	{
 		if(si->campState)
 			return ELoadMode::CAMPAIGN;
@@ -874,7 +886,7 @@ void CServerHandler::onPacketReceived(const std::shared_ptr<INetworkConnection>
 {
 	boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
 
-	if(state == EClientState::DISCONNECTING)
+	if(getState() == EClientState::DISCONNECTING)
 	{
 		assert(0); //Should not be possible - socket must be closed at this point
 		return;
@@ -887,7 +899,7 @@ void CServerHandler::onPacketReceived(const std::shared_ptr<INetworkConnection>
 
 void CServerHandler::onDisconnected(const std::shared_ptr<INetworkConnection> & connection, const std::string & errorMessage)
 {
-	if(state == EClientState::DISCONNECTING)
+	if(getState() == EClientState::DISCONNECTING)
 	{
 		assert(networkConnection == nullptr);
 		// Note: this branch can be reached on app shutdown, when main thread holds mutex till destruction
@@ -898,18 +910,13 @@ void CServerHandler::onDisconnected(const std::shared_ptr<INetworkConnection> &
 	boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
 
 	logNetwork->error("Lost connection to server! Connection has been closed");
-	networkConnection.reset();
 
 	if(client)
 	{
-		state = EClientState::DISCONNECTING;
-
-		GH.dispatchMainThread([]()
-		{
-			CSH->endGameplay();
-			GH.defActionsDef = 63;
-			CMM->menu->switchToTab("main");
-		});
+		CSH->endGameplay();
+		GH.defActionsDef = 63;
+		CMM->menu->switchToTab("main");
+		CSH->showServerError(CGI->generaltexth->translate("vcmi.server.errors.disconnected"));
 	}
 	else
 	{
@@ -917,6 +924,8 @@ void CServerHandler::onDisconnected(const std::shared_ptr<INetworkConnection> &
 		lcd.clientId = c->connectionID;
 		applyPackOnLobbyScreen(lcd);
 	}
+
+	networkConnection.reset();
 }
 
 void CServerHandler::visitForLobby(CPackForLobby & lobbyPack)
@@ -970,15 +979,13 @@ void CServerHandler::threadRunServer(bool connectToLobby)
 	}
 	else
 	{
-		if (state != EClientState::DISCONNECTING)
+		boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
+
+		if (getState() == EClientState::CONNECTING)
 		{
-			if (state == EClientState::CONNECTING)
-				CInfoWindow::showInfoDialog(CGI->generaltexth->translate("vcmi.server.errors.existingProcess"), {});
-			else
-				CInfoWindow::showInfoDialog(CGI->generaltexth->translate("vcmi.server.errors.serverCrashed"), {});
+			showServerError(CGI->generaltexth->translate("vcmi.server.errors.existingProcess"));
+			setState(EClientState::CONNECTION_CANCELLED); // stop attempts to reconnect
 		}
-
-		state = EClientState::CONNECTION_CANCELLED; // stop attempts to reconnect
 		logNetwork->error("Error: server failed to close correctly or crashed!");
 		logNetwork->error("Check %s for more info", logName);
 	}
@@ -987,6 +994,6 @@ void CServerHandler::threadRunServer(bool connectToLobby)
 
 void CServerHandler::sendLobbyPack(const CPackForLobby & pack) const
 {
-	if(state != EClientState::STARTING)
+	if(getState() != EClientState::STARTING)
 		c->sendPack(&pack);
 }

client/CServerHandler.h

@@ -54,7 +54,6 @@ enum class EClientState : ui8
 	STARTING, // Gameplay interfaces being created, we pause netpacks retrieving
 	GAMEPLAY, // In-game, used by some UI
 	DISCONNECTING, // We disconnecting, drop all netpacks
-	CONNECTION_FAILED // We could not connect to server
 };
 
 enum class EServerMode : uint8_t
@@ -108,6 +107,8 @@ class CServerHandler final : public IServerAPI, public LobbyInfo, public INetwor
 	boost::thread threadRunLocalServer;
 	boost::thread threadNetwork;
 
+	std::atomic<EClientState> state;
+
 	void threadRunNetwork();
 	void threadRunServer(bool connectToLobby);
 
@@ -129,7 +130,6 @@ class CServerHandler final : public IServerAPI, public LobbyInfo, public INetwor
 public:
 	std::shared_ptr<CConnection> c;
 
-	std::atomic<EClientState> state;
 	////////////////////
 	// FIXME: Bunch of crutches to glue it all together
 
@@ -160,6 +160,9 @@ public:
 	bool isMyColor(PlayerColor color) const;
 	ui8 myFirstId() const; // Used by chat only!
 
+	EClientState getState() const;
+	void setState(EClientState newState);
+
 	bool isHost() const;
 	bool isGuest() const;
 

client/NetPacksLobbyClient.cpp

@@ -73,7 +73,7 @@ void ApplyOnLobbyHandlerNetPackVisitor::visitLobbyClientConnected(LobbyClientCon
 
 			GH.windows().createAndPushWindow<CLobbyScreen>(handler.screenType);
 		}
-		handler.state = EClientState::LOBBY;
+		handler.setState(EClientState::LOBBY);
 	}
 }
 
@@ -136,13 +136,10 @@ void ApplyOnLobbyScreenNetPackVisitor::visitLobbyGuiAction(LobbyGuiAction & pack
 
 void ApplyOnLobbyHandlerNetPackVisitor::visitLobbyRestartGame(LobbyRestartGame & pack)
 {
-	if(handler.state == EClientState::GAMEPLAY)
-	{
-		handler.restartGameplay();
-	}
-	
-	if (handler.validateGameStart())
-		handler.sendStartGame();
+	assert(handler.getState() == EClientState::GAMEPLAY);
+
+	handler.restartGameplay();
+	handler.sendStartGame();
 }
 
 void ApplyOnLobbyHandlerNetPackVisitor::visitLobbyPrepareStartGame(LobbyPrepareStartGame & pack)
@@ -160,7 +157,7 @@ void ApplyOnLobbyHandlerNetPackVisitor::visitLobbyStartGame(LobbyStartGame & pac
 		return;
 	}
 	
-	handler.state = EClientState::STARTING;
+	handler.setState(EClientState::STARTING);
 	if(handler.si->mode != EStartMode::LOAD_GAME || pack.clientId == handler.c->connectionID)
 	{
 		auto modeBackup = handler.si->mode;
@@ -206,7 +203,7 @@ void ApplyOnLobbyScreenNetPackVisitor::visitLobbyUpdateState(LobbyUpdateState &
 	if(!lobby) //stub: ignore message for game mode
 		return;
 		
-	if(!lobby->bonusSel && handler.si->campState && handler.state == EClientState::LOBBY_CAMPAIGN)
+	if(!lobby->bonusSel && handler.si->campState && handler.getState() == EClientState::LOBBY_CAMPAIGN)
 	{
 		lobby->bonusSel = std::make_shared<CBonusSelection>();
 		GH.windows().pushWindow(lobby->bonusSel);

client/lobby/CBonusSelection.cpp

@@ -307,15 +307,12 @@ void CBonusSelection::createBonusesIcons()
 
 void CBonusSelection::updateAfterStateChange()
 {
-	if(CSH->state != EClientState::GAMEPLAY)
+	if(CSH->getState() != EClientState::GAMEPLAY)
 	{
 		buttonRestart->disable();
 		buttonVideo->disable();
 		buttonStart->enable();
-		if(!getCampaign()->conqueredScenarios().empty())
-			buttonBack->block(true);
-		else
-			buttonBack->block(false);
+		buttonBack->block(!getCampaign()->conqueredScenarios().empty());
 	}
 	else
 	{
@@ -358,7 +355,7 @@ void CBonusSelection::updateAfterStateChange()
 
 void CBonusSelection::goBack()
 {
-	if(CSH->state != EClientState::GAMEPLAY)
+	if(CSH->getState() != EClientState::GAMEPLAY)
 	{
 		GH.windows().popWindows(2);
 	}

client/lobby/CLobbyScreen.cpp

@@ -112,7 +112,7 @@ CLobbyScreen::CLobbyScreen(ESelectionScreen screenType)
 CLobbyScreen::~CLobbyScreen()
 {
 	// TODO: For now we always destroy whole lobby when leaving bonus selection screen
-	if(CSH->state == EClientState::LOBBY_CAMPAIGN)
+	if(CSH->getState() == EClientState::LOBBY_CAMPAIGN)
 		CSH->sendClientDisconnecting();
 }
 

client/mainmenu/CMainMenu.cpp

@@ -490,7 +490,7 @@ std::string CMultiMode::getPlayerName()
 {
 	std::string name = settings["general"]["playerName"].String();
 	if(name == "Player")
-		name = CGI->generaltexth->translate("vcmi.mainMenu.playerName");
+		name = CGI->generaltexth->translate("core.genrltxt.434");
 	return name;
 }
 
@@ -588,15 +588,8 @@ void CSimpleJoinScreen::connectToServer()
 
 void CSimpleJoinScreen::leaveScreen()
 {
-	if(CSH->state == EClientState::CONNECTING)
-	{
-		textTitle->setText(CGI->generaltexth->translate("vcmi.mainMenu.serverClosing"));
-		CSH->state = EClientState::CONNECTION_CANCELLED;
-	}
-	else if(GH.windows().isTopWindow(this))
-	{
-		close();
-	}
+	textTitle->setText(CGI->generaltexth->translate("vcmi.mainMenu.serverClosing"));
+	CSH->setState(EClientState::CONNECTION_CANCELLED);
 }
 
 void CSimpleJoinScreen::onChange(const std::string & newText)

client/windows/settings/SettingsMainWindow.cpp

@@ -143,12 +143,9 @@ void SettingsMainWindow::mainMenuButtonCallback()
 		[this]()
 		{
 			close();
-			GH.dispatchMainThread( []()
-			{
-				CSH->endGameplay();
-				GH.defActionsDef = 63;
-				CMM->menu->switchToTab("main");
-			});
+			CSH->endGameplay();
+			GH.defActionsDef = 63;
+			CMM->menu->switchToTab("main");
 		},
 		0
 	);

lib/network/NetworkConnection.cpp

@@ -79,13 +79,13 @@ void NetworkConnection::sendPacket(const std::vector<std::byte> & message)
 {
 	boost::system::error_code ec;
 
+	// create array with single element - boost::asio::buffer can be constructed from containers, but not from plain integer
 	std::array<uint32_t, 1> messageSize{static_cast<uint32_t>(message.size())};
 
 	boost::asio::write(*socket, boost::asio::buffer(messageSize), ec );
 	boost::asio::write(*socket, boost::asio::buffer(message), ec );
 
-	if (ec)
-		listener.onDisconnected(shared_from_this(), ec.message());
+	//Note: ignoring error code, intended
 }
 
 void NetworkConnection::close()
@@ -93,7 +93,7 @@ void NetworkConnection::close()
 	boost::system::error_code ec;
 	socket->close(ec);
 
-	//NOTE: ignoring error code
+	//NOTE: ignoring error code, intended
 }
 
 VCMI_LIB_NAMESPACE_END