Merge pull request #4022 from vcmi/master

Merge master -> beta

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 1513
+		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 @@ public: // TODO: make private
 	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 @@ public: // public interface for use by client via LOCPLINT access
 	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");
+	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 @@ public:
 
 	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 @@ public:
 	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());
 

client/battle/BattleInterface.cpp

@@ -38,7 +38,6 @@
 #include "../../lib/CConfigHandler.h"
 #include "../../lib/CGeneralTextHandler.h"
 #include "../../lib/CHeroHandler.h"
-#include "../../lib/CondSh.h"
 #include "../../lib/gameState/InfoAboutArmy.h"
 #include "../../lib/mapObjects/CGTownInstance.h"
 #include "../../lib/networkPacks/PacksForClientBattle.h"
@@ -95,7 +94,7 @@ BattleInterface::BattleInterface(const BattleID & battleID, const CCreatureSet *
 	obstacleController.reset(new BattleObstacleController(*this));
 
 	adventureInt->onAudioPaused();
-	ongoingAnimationsState.set(true);
+	ongoingAnimationsState.setBusy();
 
 	GH.windows().pushWindow(windowObject);
 	windowObject->blockUI(true);
@@ -744,6 +743,11 @@ void BattleInterface::castThisSpell(SpellID spellID)
 	actionsController->castThisSpell(spellID);
 }
 
+void BattleInterface::endNetwork()
+{
+	ongoingAnimationsState.requestTermination();
+}
+
 void BattleInterface::executeStagedAnimations()
 {
 	EAnimationEvents earliestStage = EAnimationEvents::COUNT;
@@ -775,19 +779,19 @@ void BattleInterface::executeAnimationStage(EAnimationEvents event)
 
 void BattleInterface::onAnimationsStarted()
 {
-	ongoingAnimationsState.setn(true);
+	ongoingAnimationsState.setBusy();
 }
 
 void BattleInterface::onAnimationsFinished()
 {
-	ongoingAnimationsState.setn(false);
+	ongoingAnimationsState.setFree();
 }
 
 void BattleInterface::waitForAnimations()
 {
 	{
 		auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
-		ongoingAnimationsState.waitUntil(false);
+		ongoingAnimationsState.waitWhileBusy();
 	}
 
 	assert(!hasAnimations());
@@ -802,7 +806,7 @@ void BattleInterface::waitForAnimations()
 
 bool BattleInterface::hasAnimations()
 {
-	return ongoingAnimationsState.get();
+	return ongoingAnimationsState.isBusy();
 }
 
 void BattleInterface::checkForAnimations()

client/battle/BattleInterface.h

@@ -12,7 +12,7 @@
 #include "BattleConstants.h"
 #include "../gui/CIntObject.h"
 #include "../../lib/spells/CSpellHandler.h" //CSpell::TAnimation
-#include "../../lib/CondSh.h"
+#include "../ConditionalWait.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -99,7 +99,7 @@ class BattleInterface
 	};
 
 	/// Conditional variables that are set depending on ongoing animations on the battlefield
-	CondSh<bool> ongoingAnimationsState;
+	ConditionalWait ongoingAnimationsState;
 
 	/// List of events that are waiting to be triggered
 	std::vector<AwaitingAnimationEvents> awaitingEvents;
@@ -186,6 +186,7 @@ public:
 	void setBattleQueueVisibility(bool visible);
 	void setStickyHeroWindowsVisibility(bool visible);
 
+	void endNetwork();
 	void executeStagedAnimations();
 	void executeAnimationStage( EAnimationEvents event);
 	void onAnimationsStarted();

client/battle/BattleInterfaceClasses.cpp

@@ -52,7 +52,6 @@
 #include "../../lib/CTownHandler.h"
 #include "../../lib/CHeroHandler.h"
 #include "../../lib/StartInfo.h"
-#include "../../lib/CondSh.h"
 #include "../../lib/mapObjects/CGTownInstance.h"
 #include "../../lib/networkPacks/PacksForClientBattle.h"
 #include "../../lib/TextOperations.h"
@@ -778,7 +777,7 @@ BattleResultWindow::BattleResultWindow(const BattleResult & br, CPlayerInterface
 
 void BattleResultWindow::activate()
 {
-	owner.showingDialog->set(true);
+	owner.showingDialog->setBusy();
 	CIntObject::activate();
 }
 
@@ -871,7 +870,7 @@ void BattleResultWindow::buttonPressed(int button)
 
 	//Result window and battle interface are gone. We requested all dialogs to be closed before opening the battle,
 	//so we can be sure that there is no dialogs left on GUI stack.
-	intTmp.showingDialog->setn(false);
+	intTmp.showingDialog->setFree();
 	CCS->videoh->close();
 }
 

client/battle/BattleStacksController.cpp

@@ -39,7 +39,6 @@
 #include "../../lib/battle/BattleHex.h"
 #include "../../lib/CRandomGenerator.h"
 #include "../../lib/CStack.h"
-#include "../../lib/CondSh.h"
 #include "../../lib/TextOperations.h"
 
 static void onAnimationFinished(const CStack *stack, std::weak_ptr<CreatureAnimation> anim)

client/gui/CGuiHandler.cpp

@@ -9,7 +9,6 @@
  */
 #include "StdInc.h"
 #include "CGuiHandler.h"
-#include "../lib/CondSh.h"
 
 #include "CIntObject.h"
 #include "CursorHandler.h"

client/gui/CGuiHandler.h

@@ -10,7 +10,6 @@
 #pragma once
 
 VCMI_LIB_NAMESPACE_BEGIN
-template <typename T> struct CondSh;
 class Point;
 class Rect;
 VCMI_LIB_NAMESPACE_END

client/mainmenu/CMainMenu.cpp

@@ -58,7 +58,6 @@
 #include "../../lib/CConfigHandler.h"
 #include "../../lib/GameConstants.h"
 #include "../../lib/CRandomGenerator.h"
-#include "../../lib/CondSh.h"
 
 std::shared_ptr<CMainMenu> CMM;
 ISelectionScreenInfo * SEL;
@@ -275,7 +274,8 @@ CMainMenuConfig::CMainMenuConfig()
 	: campaignSets(JsonPath::builtin("config/campaignSets.json"))
 	, config(JsonPath::builtin("config/mainmenu.json"))
 {
-
+	if (config["game-select"].Vector().empty())
+		handleFatalError("Main menu config is invalid or corrupted. Please disable any mods or reinstall VCMI", false);
 }
 
 const CMainMenuConfig & CMainMenuConfig::get()

client/mapView/IMapRendererObserver.h

@@ -25,6 +25,8 @@ public:
 	virtual ~IMapObjectObserver();
 
 	virtual bool hasOngoingAnimations() = 0;
+	virtual void waitForOngoingAnimations(){};
+	virtual void endNetwork(){};
 
 	/// Plays fade-in animation and adds object to map
 	virtual void onObjectFadeIn(const CGObjectInstance * obj, const PlayerColor & initiator) = 0;

client/mapView/MapViewController.cpp

@@ -25,6 +25,7 @@
 
 #include "../../lib/CConfigHandler.h"
 #include "../../lib/StartInfo.h"
+#include "../../lib/UnlockGuard.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"
 #include "../../lib/mapObjects/MiscObjects.h"
 #include "../../lib/pathfinder/CGPathNode.h"
@@ -346,6 +347,7 @@ bool MapViewController::isEventVisible(const CGHeroInstance * obj, const int3 &
 
 void MapViewController::fadeOutObject(const CGObjectInstance * obj)
 {
+	animationWait.setBusy();
 	logGlobal->debug("Starting fade out animation");
 	fadingOutContext = std::make_shared<MapRendererAdventureFadingContext>(*state);
 	fadingOutContext->animationTime = adventureContext->animationTime;
@@ -366,6 +368,7 @@ void MapViewController::fadeOutObject(const CGObjectInstance * obj)
 
 void MapViewController::fadeInObject(const CGObjectInstance * obj)
 {
+	animationWait.setBusy();
 	logGlobal->debug("Starting fade in animation");
 	fadingInContext = std::make_shared<MapRendererAdventureFadingContext>(*state);
 	fadingInContext->animationTime = adventureContext->animationTime;
@@ -505,6 +508,7 @@ void MapViewController::onAfterHeroTeleported(const CGHeroInstance * obj, const
 
 	if(isEventVisible(obj, from, dest))
 	{
+		animationWait.setBusy();
 		logGlobal->debug("Starting teleport animation");
 		teleportContext = std::make_shared<MapRendererAdventureTransitionContext>(*state);
 		teleportContext->animationTime = adventureContext->animationTime;
@@ -540,6 +544,7 @@ void MapViewController::onHeroMoved(const CGHeroInstance * obj, const int3 & fro
 
 	if(movementTime > 1)
 	{
+		animationWait.setBusy();
 		logGlobal->debug("Starting movement animation");
 		movementContext = std::make_shared<MapRendererAdventureMovingContext>(*state);
 		movementContext->animationTime = adventureContext->animationTime;
@@ -577,6 +582,17 @@ bool MapViewController::hasOngoingAnimations()
 	return false;
 }
 
+void MapViewController::waitForOngoingAnimations()
+{
+	auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
+	animationWait.waitWhileBusy();
+}
+
+void MapViewController::endNetwork()
+{
+	animationWait.requestTermination();
+}
+
 void MapViewController::activateAdventureContext(uint32_t animationTime)
 {
 	resetContext();
@@ -642,6 +658,7 @@ void MapViewController::resetContext()
 	worldViewContext.reset();
 	spellViewContext.reset();
 	puzzleMapContext.reset();
+	animationWait.setFree();
 }
 
 void MapViewController::setTerrainVisibility(bool showAllTerrain)

client/mapView/MapViewController.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include "IMapRendererObserver.h"
+#include "../ConditionalWait.h"
 #include "../../lib/Point.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
@@ -34,6 +35,8 @@ class MapRendererPuzzleMapContext;
 /// such as its position and any animations
 class MapViewController : public IMapObjectObserver
 {
+	ConditionalWait animationWait;
+
 	std::shared_ptr<IMapRendererContext> context;
 	std::shared_ptr<MapRendererContextState> state;
 	std::shared_ptr<MapViewModel> model;
@@ -68,6 +71,9 @@ private:
 
 	// IMapObjectObserver impl
 	bool hasOngoingAnimations() override;
+	void waitForOngoingAnimations() override;
+	void endNetwork() override;
+
 	void onObjectFadeIn(const CGObjectInstance * obj, const PlayerColor & initiator) override;
 	void onObjectFadeOut(const CGObjectInstance * obj, const PlayerColor & initiator) override;
 	void onObjectInstantAdd(const CGObjectInstance * obj, const PlayerColor & initiator) override;

client/mapView/mapHandler.cpp

@@ -19,7 +19,6 @@
 
 #include "../../lib/CGeneralTextHandler.h"
 #include "../../lib/TerrainHandler.h"
-#include "../../lib/UnlockGuard.h"
 #include "../../lib/mapObjectConstructors/CObjectClassesHandler.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"
 #include "../../lib/mapObjects/ObjectTemplate.h"
@@ -36,13 +35,19 @@ bool CMapHandler::hasOngoingAnimations()
 
 void CMapHandler::waitForOngoingAnimations()
 {
-	while(CGI->mh->hasOngoingAnimations())
+	for(auto * observer : observers)
 	{
-		auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
-		boost::this_thread::sleep_for(boost::chrono::milliseconds(1));
+		if (observer->hasOngoingAnimations())
+			observer->waitForOngoingAnimations();
 	}
 }
 
+void CMapHandler::endNetwork()
+{
+	for(auto * observer : observers)
+		observer->endNetwork();
+}
+
 std::string CMapHandler::getTerrainDescr(const int3 & pos, bool rightClick) const
 {
 	const TerrainTile & t = map->getTile(pos);

client/mapView/mapHandler.h

@@ -71,6 +71,7 @@ public:
 
 	/// blocking wait until all ongoing animatins are over
 	void waitForOngoingAnimations();
+	void endNetwork();
 
 	static bool compareObjectBlitOrder(const CGObjectInstance * a, const CGObjectInstance * b);
 };

client/windows/CTutorialWindow.cpp

@@ -12,7 +12,7 @@
 
 #include "../eventsSDL/InputHandler.h"
 #include "../../lib/CConfigHandler.h"
-#include "../../lib/CondSh.h"
+#include "../ConditionalWait.h"
 #include "../../lib/CGeneralTextHandler.h"
 #include "../CPlayerInterface.h"
 #include "../CGameInfo.h"
@@ -67,7 +67,7 @@ void CTutorialWindow::openWindowFirstTime(const TutorialMode & m)
 	if(GH.input().hasTouchInputDevice() && !persistentStorage["gui"]["tutorialCompleted" + std::to_string(m)].Bool())
 	{
 		if(LOCPLINT)
-			LOCPLINT->showingDialog->set(true);
+			LOCPLINT->showingDialog->setBusy();
 		GH.windows().pushWindow(std::make_shared<CTutorialWindow>(m));
 
 		Settings s = persistentStorage.write["gui"]["tutorialCompleted" + std::to_string(m)];
@@ -78,7 +78,7 @@ void CTutorialWindow::openWindowFirstTime(const TutorialMode & m)
 void CTutorialWindow::exit()
 {
 	if(LOCPLINT)
-		LOCPLINT->showingDialog->setn(false);
+		LOCPLINT->showingDialog->setFree();
 
 	close();
 }

client/windows/GUIClasses.cpp

@@ -56,7 +56,7 @@
 #include "../lib/CGeneralTextHandler.h"
 #include "../lib/CHeroHandler.h"
 #include "../lib/GameSettings.h"
-#include "../lib/CondSh.h"
+#include "ConditionalWait.h"
 #include "../lib/CSkillHandler.h"
 #include "../lib/filesystem/Filesystem.h"
 #include "../lib/TextOperations.h"
@@ -402,7 +402,7 @@ CLevelWindow::CLevelWindow(const CGHeroInstance * hero, PrimarySkill pskill, std
 {
 	OBJECT_CONSTRUCTION_CAPTURING(255-DISPOSE);
 
-	LOCPLINT->showingDialog->setn(true);
+	LOCPLINT->showingDialog->setBusy();
 
 	if(!skills.empty())
 	{
@@ -445,7 +445,7 @@ CLevelWindow::~CLevelWindow()
 	if (box && box->selectedIndex() != -1)
 		cb(box->selectedIndex());
 
-	LOCPLINT->showingDialog->setn(false);
+	LOCPLINT->showingDialog->setFree();
 }
 
 CTavernWindow::CTavernWindow(const CGObjectInstance * TavernObj, const std::function<void()> & onWindowClosed)

client/windows/InfoWindows.cpp

@@ -29,7 +29,7 @@
 #include "../../CCallback.h"
 
 #include "../../lib/CConfigHandler.h"
-#include "../../lib/CondSh.h"
+#include "../ConditionalWait.h"
 #include "../../lib/gameState/InfoAboutArmy.h"
 #include "../../lib/mapObjects/CGCreature.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"
@@ -140,7 +140,7 @@ void CInfoWindow::close()
 	WindowBase::close();
 
 	if(LOCPLINT)
-		LOCPLINT->showingDialog->setn(false);
+		LOCPLINT->showingDialog->setFree();
 }
 
 void CInfoWindow::showAll(Canvas & to)
@@ -158,7 +158,7 @@ void CInfoWindow::showInfoDialog(const std::string & text, const TCompsInfo & co
 
 void CInfoWindow::showYesNoDialog(const std::string & text, const TCompsInfo & components, const CFunctionList<void()> & onYes, const CFunctionList<void()> & onNo, PlayerColor player)
 {
-	assert(!LOCPLINT || LOCPLINT->showingDialog->get());
+	assert(!LOCPLINT || LOCPLINT->showingDialog->isBusy());
 	std::vector<std::pair<AnimationPath, CFunctionList<void()>>> pom;
 	pom.emplace_back(AnimationPath::builtin("IOKAY.DEF"), nullptr);
 	pom.emplace_back(AnimationPath::builtin("ICANCEL.DEF"), nullptr);

docs/players/Cheat_Codes.md

@@ -142,5 +142,4 @@ Below a list of supported commands, with their arguments wrapped in `<>`
 `activate <0/1/2>` - activate game windows (no current use, apparently broken long ago)  
 `redraw` - force full graphical redraw  
 `screen` - show value of screenBuf variable, which prints "screen" when adventure map has current focus, "screen2" otherwise, and dumps values of both screen surfaces to .bmp files  
-`not dialog` - set the state indicating if dialog box is active to "no"  
 `tell hs <hero ID> <artifact slot ID>` - write what artifact is present on artifact slot with specified ID for hero with specified ID. (must be called during gameplay)  

lib/CCreatureHandler.cpp

@@ -648,7 +648,7 @@ CCreature * CCreatureHandler::loadFromJson(const std::string & scope, const Json
 	JsonNode advMapFile = node["graphics"]["map"];
 	JsonNode advMapMask = node["graphics"]["mapMask"];
 
-	VLC->identifiers()->requestIdentifier(scope, "object", "monster", [=](si32 index)
+	VLC->identifiers()->requestIdentifier(scope, "object", "monster", [cre, scope, advMapFile, advMapMask](si32 index)
 	{
 		JsonNode conf;
 		conf.setModScope(scope);
@@ -669,7 +669,12 @@ CCreature * CCreatureHandler::loadFromJson(const std::string & scope, const Json
 
 		// object does not have any templates - this is not usable object (e.g. pseudo-creature like Arrow Tower)
 		if (VLC->objtypeh->getHandlerFor(Obj::MONSTER, cre->getId().num)->getTemplates().empty())
+		{
+			assert(cre->special);
+			if (!cre->special)
+				logMod->error("Creature %s does not have valid map object but is not marked as special!", cre->getJsonKey());
 			VLC->objtypeh->removeSubObject(Obj::MONSTER, cre->getId().num);
+		}
 	});
 
 	return cre;

lib/CGeneralTextHandler.cpp

@@ -23,6 +23,8 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
+std::recursive_mutex TextLocalizationContainer::globalTextMutex;
+
 /// Detects language and encoding of H3 text files based on matching against pregenerated footprints of H3 file
 void CGeneralTextHandler::detectInstallParameters()
 {
@@ -251,6 +253,8 @@ bool CLegacyConfigParser::endLine()
 
 void TextLocalizationContainer::registerStringOverride(const std::string & modContext, const std::string & language, const TextIdentifier & UID, const std::string & localized)
 {
+	std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
+
 	assert(!modContext.empty());
 	assert(!language.empty());
 
@@ -265,12 +269,16 @@ void TextLocalizationContainer::registerStringOverride(const std::string & modCo
 
 void TextLocalizationContainer::addSubContainer(const TextLocalizationContainer & container)
 {
+	std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
+
 	assert(!vstd::contains(subContainers, &container));
 	subContainers.push_back(&container);
 }
 
 void TextLocalizationContainer::removeSubContainer(const TextLocalizationContainer & container)
 {
+	std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
+
 	assert(vstd::contains(subContainers, &container));
 
 	subContainers.erase(std::remove(subContainers.begin(), subContainers.end(), &container), subContainers.end());
@@ -278,6 +286,8 @@ void TextLocalizationContainer::removeSubContainer(const TextLocalizationContain
 
 const std::string & TextLocalizationContainer::deserialize(const TextIdentifier & identifier) const
 {
+	std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
+
 	if(stringsLocalizations.count(identifier.get()) == 0)
 	{
 		for(auto containerIter = subContainers.rbegin(); containerIter != subContainers.rend(); ++containerIter)
@@ -297,6 +307,8 @@ const std::string & TextLocalizationContainer::deserialize(const TextIdentifier
 
 void TextLocalizationContainer::registerString(const std::string & modContext, const TextIdentifier & UID, const std::string & localized, const std::string & language)
 {
+	std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
+
 	assert(!modContext.empty());
 	assert(!Languages::getLanguageOptions(language).identifier.empty());
 	assert(UID.get().find("..") == std::string::npos); // invalid identifier - there is section that was evaluated to empty string
@@ -327,6 +339,8 @@ void TextLocalizationContainer::registerString(const std::string & modContext, c
 
 bool TextLocalizationContainer::validateTranslation(const std::string & language, const std::string & modContext, const JsonNode & config) const
 {
+	std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
+
 	bool allPresent = true;
 
 	for(const auto & string : stringsLocalizations)
@@ -384,11 +398,15 @@ void TextLocalizationContainer::loadTranslationOverrides(const std::string & lan
 
 bool TextLocalizationContainer::identifierExists(const TextIdentifier & UID) const
 {
+	std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
+
 	return stringsLocalizations.count(UID.get());
 }
 
 void TextLocalizationContainer::exportAllTexts(std::map<std::string, std::map<std::string, std::string>> & storage) const
 {
+	std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
+
 	for (auto const & subContainer : subContainers)
 		subContainer->exportAllTexts(storage);
 
@@ -418,6 +436,8 @@ std::string TextLocalizationContainer::getModLanguage(const std::string & modCon
 
 void TextLocalizationContainer::jsonSerialize(JsonNode & dest) const
 {
+	std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
+
 	for(auto & s : stringsLocalizations)
 	{
 		dest.Struct()[s.first].String() = s.second.baseValue;
@@ -692,6 +712,7 @@ std::string CGeneralTextHandler::getInstalledEncoding()
 
 std::vector<std::string> CGeneralTextHandler::findStringsWithPrefix(const std::string & prefix)
 {
+	std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
 	std::vector<std::string> result;
 
 	for(const auto & entry : stringsLocalizations)

lib/CGeneralTextHandler.h

@@ -117,6 +117,8 @@ public:
 class DLL_LINKAGE TextLocalizationContainer
 {
 protected:
+	static std::recursive_mutex globalTextMutex;
+
 	struct StringState
 	{
 		/// Human-readable string that was added on registration
@@ -153,6 +155,9 @@ protected:
 	
 	std::string getModLanguage(const std::string & modContext);
 	
+	// returns true if identifier with such name was registered, even if not translated to current language
+	bool identifierExists(const TextIdentifier & UID) const;
+
 public:
 	/// validates translation of specified language for specified mod
 	/// returns true if localization is valid and complete
@@ -163,9 +168,6 @@ public:
 	/// Any entries loaded by this will have priority over texts registered normally
 	void loadTranslationOverrides(const std::string & language, const std::string & modContext, JsonNode const & file);
 
-	// returns true if identifier with such name was registered, even if not translated to current language
-	bool identifierExists(const TextIdentifier & UID) const;
-	
 	/// add selected string to internal storage
 	void registerString(const std::string & modContext, const TextIdentifier & UID, const std::string & localized);
 	void registerString(const std::string & modContext, const TextIdentifier & UID, const std::string & localized, const std::string & language);
@@ -196,6 +198,8 @@ public:
 	template <typename Handler>
 	void serialize(Handler & h)
 	{
+		std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
+
 		std::string key;
 		auto sz = stringsLocalizations.size();
 		h & sz;

lib/CMakeLists.txt

@@ -649,7 +649,6 @@ set(lib_MAIN_HEADERS
 	CGameInterface.h
 	CGeneralTextHandler.h
 	CHeroHandler.h
-	CondSh.h
 	ConstTransitivePtr.h
 	Color.h
 	CPlayerState.h

lib/CondSh.h

@@ -1,71 +0,0 @@
-/*
- * CondSh.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
-
-VCMI_LIB_NAMESPACE_BEGIN
-
-/// Used for multithreading, wraps boost functions
-template <typename T> struct CondSh
-{
-	T data;
-	boost::condition_variable cond;
-	boost::mutex mx;
-
-	CondSh() : data(T()) {}
-
-	CondSh(T t) : data(t) {}
-
-	// set data
-	void set(T t)
-	{
-		boost::unique_lock<boost::mutex> lock(mx);
-		data = t;
-	}
-
-	// set data and notify
-	void setn(T t)
-	{
-		set(t);
-		cond.notify_all();
-	};
-
-	// get stored value
-	T get()
-	{
-		boost::unique_lock<boost::mutex> lock(mx);
-		return data;
-	}
-
-	// waits until data is set to false
-	void waitWhileTrue()
-	{
-		boost::unique_lock<boost::mutex> un(mx);
-		while(data)
-			cond.wait(un);
-	}
-
-	// waits while data is set to arg
-	void waitWhile(const T & t)
-	{
-		boost::unique_lock<boost::mutex> un(mx);
-		while(data == t)
-			cond.wait(un);
-	}
-
-	// waits until data is set to arg
-	void waitUntil(const T & t)
-	{
-		boost::unique_lock<boost::mutex> un(mx);
-		while(data != t)
-			cond.wait(un);
-	}
-};
-
-VCMI_LIB_NAMESPACE_END

lib/network/NetworkConnection.cpp

@@ -117,7 +117,9 @@ void NetworkConnection::onPacketReceived(const boost::system::error_code & ec, u
 
 	if (readBuffer.size() < expectedPacketSize)
 	{
-		throw std::runtime_error("Failed to read packet! " + std::to_string(readBuffer.size()) + " bytes read, but " + std::to_string(expectedPacketSize) + " bytes expected!");
+		// FIXME: figure out what causes this. This should not be possible without error set
+		std::string errorMessage = "Failed to read packet! " + std::to_string(readBuffer.size()) + " bytes read, but " + std::to_string(expectedPacketSize) + " bytes expected!";
+		onError(errorMessage);
 	}
 
 	std::vector<std::byte> message(expectedPacketSize);

server/CVCMIServer.cpp

@@ -986,7 +986,7 @@ void CVCMIServer::multiplayerWelcomeMessage()
 {
 	int humanPlayer = 0;
 	for (auto & pi : si->playerInfos)
-        if(gh->getPlayerState(pi.first)->isHuman())
+		if(pi.second.isControlledByHuman())
 			humanPlayer++;
 
 	if(humanPlayer < 2) // Singleplayer