Merge pull request #4001 from IvanSavenko/stabilization

[1.5.2] Stabilization

AI/Nullkiller/AIGateway.h

@@ -21,7 +21,6 @@
 #include "../../lib/CTownHandler.h"
 #include "../../lib/mapObjects/MiscObjects.h"
 #include "../../lib/spells/CSpellHandler.h"
-#include "../../lib/CondSh.h"
 #include "Pathfinding/AIPathfinder.h"
 #include "Engine/Nullkiller.h"
 

AI/VCAI/VCAI.h

@@ -23,7 +23,6 @@
 #include "../../lib/CTownHandler.h"
 #include "../../lib/mapObjects/MiscObjects.h"
 #include "../../lib/spells/CSpellHandler.h"
-#include "../../lib/CondSh.h"
 #include "Pathfinding/AIPathfinder.h"
 
 VCMI_LIB_NAMESPACE_BEGIN

android/vcmi-app/build.gradle

@@ -10,7 +10,7 @@ android {
 		applicationId "is.xyz.vcmi"
 		minSdk 19
 		targetSdk 33
-		versionCode 1514
+		versionCode 1515
 		versionName "1.5.1"
 		setProperty("archivesBaseName", "vcmi")
 	}

client/CMT.cpp

@@ -442,6 +442,8 @@ static void mainLoop()
 
 [[noreturn]] static void quitApplication()
 {
+	CSH->endNetwork();
+
 	if(!settings["session"]["headless"].Bool())
 	{
 		if(CSH->client)
@@ -450,6 +452,8 @@ static void mainLoop()
 		GH.windows().clear();
 	}
 
+	vstd::clear_pointer(CSH);
+
 	CMM.reset();
 
 	if(!settings["session"]["headless"].Bool())
@@ -473,7 +477,6 @@ static void mainLoop()
 		vstd::clear_pointer(graphics);
 	}
 
-	vstd::clear_pointer(CSH);
 	vstd::clear_pointer(VLC);
 
 	// sometimes leads to a hang. TODO: investigate

client/CMakeLists.txt

@@ -365,6 +365,7 @@ set(client_HEADERS
 	Client.h
 	ClientCommandManager.h
 	ClientNetPackVisitors.h
+	ConditionalWait.h
 	HeroMovementController.h
 	GameChatHandler.h
 	LobbyClientNetPackVisitors.h

client/CPlayerInterface.cpp

@@ -72,7 +72,6 @@
 #include "../lib/CStopWatch.h"
 #include "../lib/CThreadHelper.h"
 #include "../lib/CTownHandler.h"
-#include "../lib/CondSh.h"
 #include "../lib/GameConstants.h"
 #include "../lib/RoadHandler.h"
 #include "../lib/StartInfo.h"
@@ -140,7 +139,7 @@ CPlayerInterface::CPlayerInterface(PlayerColor Player):
 	battleInt = nullptr;
 	castleInt = nullptr;
 	makingTurn = false;
-	showingDialog = new CondSh<bool>(false);
+	showingDialog = new ConditionalWait();
 	cingconsole = new CInGameConsole();
 	firstCall = 1; //if loading will be overwritten in serialize
 	autosaveCount = 0;
@@ -1005,7 +1004,7 @@ void CPlayerInterface::showInfoDialog(const std::string &text, const std::vector
 	if (makingTurn && GH.windows().count() > 0 && LOCPLINT == this)
 	{
 		CCS->soundh->playSound(static_cast<soundBase::soundID>(soundID));
-		showingDialog->set(true);
+		showingDialog->setBusy();
 		movementController->requestMovementAbort(); // interrupt movement to show dialog
 		GH.windows().pushWindow(temp);
 	}
@@ -1028,7 +1027,7 @@ void CPlayerInterface::showInfoDialogAndWait(std::vector<Component> & components
 void CPlayerInterface::showYesNoDialog(const std::string &text, CFunctionList<void()> onYes, CFunctionList<void()> onNo, const std::vector<std::shared_ptr<CComponent>> & components)
 {
 	movementController->requestMovementAbort();
-	LOCPLINT->showingDialog->setn(true);
+	LOCPLINT->showingDialog->setBusy();
 	CInfoWindow::showYesNoDialog(text, components, onYes, onNo, playerID);
 }
 
@@ -1217,7 +1216,7 @@ void CPlayerInterface::loadGame( BinaryDeserializer & h )
 void CPlayerInterface::moveHero( const CGHeroInstance *h, const CGPath& path )
 {
 	assert(h);
-	assert(!showingDialog->get());
+	assert(!showingDialog->isBusy());
 	assert(dialogs.empty());
 
 	LOG_TRACE(logGlobal);
@@ -1227,7 +1226,7 @@ void CPlayerInterface::moveHero( const CGHeroInstance *h, const CGPath& path )
 		return; //can't find hero
 
 	//It shouldn't be possible to move hero with open dialog (or dialog waiting in bg)
-	if (showingDialog->get() || !dialogs.empty())
+	if (showingDialog->isBusy() || !dialogs.empty())
 		return;
 
 	if (localState->isHeroSleeping(h))
@@ -1395,9 +1394,7 @@ void CPlayerInterface::waitWhileDialog()
 	}
 
 	auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
-	boost::unique_lock<boost::mutex> un(showingDialog->mx);
-	while(showingDialog->data)
-		showingDialog->cond.wait(un);
+	showingDialog->waitWhileBusy();
 }
 
 void CPlayerInterface::showShipyardDialog(const IShipyard *obj)
@@ -1502,9 +1499,9 @@ void CPlayerInterface::update()
 		return;
 
 	//if there are any waiting dialogs, show them
-	if ((CSH->howManyPlayerInterfaces() <= 1 || makingTurn) && !dialogs.empty() && !showingDialog->get())
+	if ((CSH->howManyPlayerInterfaces() <= 1 || makingTurn) && !dialogs.empty() && !showingDialog->isBusy())
 	{
-		showingDialog->set(true);
+		showingDialog->setBusy();
 		GH.windows().pushWindow(dialogs.front());
 		dialogs.pop_front();
 	}
@@ -1516,6 +1513,11 @@ void CPlayerInterface::update()
 	GH.windows().simpleRedraw();
 }
 
+void CPlayerInterface::endNetwork()
+{
+	showingDialog->requestTermination();
+}
+
 int CPlayerInterface::getLastIndex( std::string namePrefix)
 {
 	using namespace boost::filesystem;

client/CPlayerInterface.h

@@ -25,7 +25,7 @@ struct CGPath;
 class CCreatureSet;
 class CGObjectInstance;
 struct UpgradeInfo;
-template <typename T> struct CondSh;
+class ConditionalWait;
 struct CPathsInfo;
 
 VCMI_LIB_NAMESPACE_END
@@ -74,7 +74,7 @@ class CPlayerInterface : public CGameInterface, public IUpdateable
 	std::unique_ptr<PlayerLocalState> localState;
 
 	//minor interfaces
-	CondSh<bool> *showingDialog; //indicates if dialog box is displayed
+	ConditionalWait * showingDialog; //indicates if dialog box is displayed
 
 	bool makingTurn; //if player is already making his turn
 
@@ -202,6 +202,7 @@ class CPlayerInterface : public CGameInterface, public IUpdateable
 	void proposeLoadingGame();
 	void performAutosave();
 	void gamePause(bool pause);
+	void endNetwork();
 
 	///returns true if all events are processed internally
 	bool capturedAllEvents();

client/CServerHandler.cpp

@@ -29,6 +29,7 @@
 
 #include "../lib/CConfigHandler.h"
 #include "../lib/CGeneralTextHandler.h"
+#include "ConditionalWait.h"
 #include "../lib/CThreadHelper.h"
 #include "../lib/StartInfo.h"
 #include "../lib/TurnTimerInfo.h"
@@ -131,6 +132,17 @@ CServerHandler::~CServerHandler()
 	}
 }
 
+void CServerHandler::endNetwork()
+{
+	if (client)
+		client->endNetwork();
+	networkHandler->stop();
+	{
+		auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
+		threadNetwork.join();
+	}
+}
+
 CServerHandler::CServerHandler()
 	: networkHandler(INetworkHandler::createHandler())
 	, lobbyClient(std::make_unique<GlobalLobbyClient>())
@@ -158,7 +170,14 @@ void CServerHandler::threadRunNetwork()
 {
 	logGlobal->info("Starting network thread");
 	setThreadName("runNetwork");
-	networkHandler->run();
+	try {
+		networkHandler->run();
+	}
+	catch (const TerminationRequestedException & e)
+	{
+		logGlobal->info("Terminating network thread");
+		return;
+	}
 	logGlobal->info("Ending network thread");
 }
 

client/CServerHandler.h

@@ -13,7 +13,6 @@
 
 #include "../lib/network/NetworkInterface.h"
 #include "../lib/StartInfo.h"
-#include "../lib/CondSh.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -208,6 +207,7 @@ class CServerHandler final : public IServerAPI, public LobbyInfo, public INetwor
 
 	void startGameplay(VCMI_LIB_WRAP_NAMESPACE(CGameState) * gameState = nullptr);
 	void showHighScoresAndEndGameplay(PlayerColor player, bool victory);
+	void endNetwork();
 	void endGameplay();
 	void restartGameplay();
 	void startCampaignScenario(HighScoreParameter param, std::shared_ptr<CampaignState> cs = {});

client/Client.cpp

@@ -346,6 +346,22 @@ void CClient::save(const std::string & fname)
 	sendRequest(&save_game, PlayerColor::NEUTRAL);
 }
 
+void CClient::endNetwork()
+{
+	if (CGI->mh)
+		CGI->mh->endNetwork();
+
+	if (CPlayerInterface::battleInt)
+		CPlayerInterface::battleInt->endNetwork();
+
+	for(auto & i : playerint)
+	{
+		auto interface = std::dynamic_pointer_cast<CPlayerInterface>(i.second);
+		if (interface)
+			interface->endNetwork();
+	}
+}
+
 void CClient::endGame()
 {
 #if SCRIPTING_ENABLED

client/Client.h

@@ -135,6 +135,7 @@ class CClient : public IGameCallback, public Environment
 	void serialize(BinaryDeserializer & h);
 
 	void save(const std::string & fname);
+	void endNetwork();
 	void endGame();
 
 	void initMapHandler();

client/ClientCommandManager.cpp

@@ -179,11 +179,6 @@ void ClientCommandManager::handleRedrawCommand()
 	GH.windows().totalRedraw();
 }
 
-void ClientCommandManager::handleNotDialogCommand()
-{
-	LOCPLINT->showingDialog->setn(false);
-}
-
 void ClientCommandManager::handleTranslateGameCommand()
 {
 	std::map<std::string, std::map<std::string, std::string>> textsByMod;
@@ -584,9 +579,6 @@ void ClientCommandManager::processCommand(const std::string & message, bool call
 	else if(commandName == "redraw")
 		handleRedrawCommand();
 
-	else if(commandName == "not dialog")
-		handleNotDialogCommand();
-
 	else if(message=="translate" || message=="translate game")
 		handleTranslateGameCommand();
 

client/ClientCommandManager.h

@@ -45,9 +45,6 @@ class ClientCommandManager //take mantis #2292 issue about account if thinking a
 	// Redraw the current screen
 	void handleRedrawCommand();
 
-	// Set the state indicating if dialog box is active to "no"
-	void handleNotDialogCommand();
-
 	// Extracts all translateable game texts into Translation directory, separating files on per-mod basis
 	void handleTranslateGameCommand();
 

client/ConditionalWait.h

@@ -0,0 +1,77 @@
+/*
+ * ConditionalWait.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#pragma once
+
+#include <condition_variable>
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+class TerminationRequestedException : public std::exception
+{
+public:
+	using exception::exception;
+
+	const char* what() const noexcept override
+	{
+		return "Thread termination requested";
+	}
+};
+
+class ConditionalWait
+{
+	bool isBusyValue = false;
+	bool isTerminating = false;
+	std::condition_variable cond;
+	std::mutex mx;
+
+	void set(bool value)
+	{
+		boost::unique_lock<std::mutex> lock(mx);
+		isBusyValue = value;
+	}
+
+public:
+	ConditionalWait() = default;
+
+	void setBusy()
+	{
+		set(true);
+	}
+
+	void setFree()
+	{
+		set(false);
+		cond.notify_all();
+	}
+
+	void requestTermination()
+	{
+		isTerminating = true;
+		setFree();
+	}
+
+	bool isBusy()
+	{
+		std::unique_lock<std::mutex> lock(mx);
+		return isBusyValue;
+	}
+
+	void waitWhileBusy()
+	{
+		std::unique_lock<std::mutex> un(mx);
+		while(isBusyValue)
+			cond.wait(un);
+
+		if (isTerminating)
+			throw TerminationRequestedException();
+	}
+};
+
+VCMI_LIB_NAMESPACE_END

client/HeroMovementController.cpp

@@ -22,7 +22,7 @@
 
 #include "../CCallback.h"
 
-#include "../lib/CondSh.h"
+#include "ConditionalWait.h"
 #include "../lib/pathfinder/CGPathNode.h"
 #include "../lib/mapObjects/CGHeroInstance.h"
 #include "../lib/networkPacks/PacksForClient.h"
@@ -237,7 +237,7 @@ void HeroMovementController::onMoveHeroApplied()
 	assert(currentlyMovingHero);
 	const auto * hero = currentlyMovingHero;
 
-	bool canMove = LOCPLINT->localState->hasPath(hero) && LOCPLINT->localState->getPath(hero).nextNode().turns == 0 && !LOCPLINT->showingDialog->get();
+	bool canMove = LOCPLINT->localState->hasPath(hero) && LOCPLINT->localState->getPath(hero).nextNode().turns == 0 && !LOCPLINT->showingDialog->isBusy();
 	bool wantStop = stoppingMovement;
 	bool canStop = !canMove || canHeroStopAtNode(LOCPLINT->localState->getPath(hero).currNode());