Merge pull request #3312 from IvanSavenko/crashfixes

Crashfixes

android/vcmi-app/build.gradle

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

client/CMT.cpp

@@ -487,7 +487,8 @@ static void quitApplication()
 	vstd::clear_pointer(CSH);
 	vstd::clear_pointer(VLC);
 
-	vstd::clear_pointer(console);// should be removed after everything else since used by logging
+	// sometimes leads to a hang. TODO: investigate
+	//vstd::clear_pointer(console);// should be removed after everything else since used by logging
 
 	if(!settings["session"]["headless"].Bool())
 		GH.screenHandler().close();
@@ -501,10 +502,16 @@ static void quitApplication()
 
 	std::cout << "Ending...\n";
 
-	// this method is always called from event/network threads, which keep interface mutex locked
-	// unlock it here to avoid assertion failure on GH destruction in exit()
-	GH.interfaceMutex.unlock();
-	exit(0);
+	// Perform quick exit without executing static destructors and let OS cleanup anything that we did not
+	// We generally don't care about them and this leads to numerous issues, e.g.
+	// destruction of locked mutexes (fails an assertion), even in third-party libraries (as well as native libs on Android)
+	// Android - std::quick_exit is available only starting from API level 21
+	// Mingw, macOS and iOS - std::quick_exit is unavailable (at least in current version of CI)
+#if (defined(__ANDROID_API__) && __ANDROID_API__ < 21) || (defined(__MINGW32__)) || defined(VCMI_APPLE)
+	::exit(0);
+#else
+	std::quick_exit(0);
+#endif
 }
 
 void handleQuit(bool ask)

client/battle/BattleInterface.cpp

@@ -107,7 +107,8 @@ void BattleInterface::playIntroSoundAndUnlockInterface()
 {
 	auto onIntroPlayed = [this]()
 	{
-		if(LOCPLINT->battleInt)
+		// Make sure that battle have not ended while intro was playing AND that a different one has not started
+		if(LOCPLINT->battleInt.get() == this)
 			onIntroSoundPlayed();
 	};
 

client/lobby/OptionsTab.cpp

@@ -503,7 +503,7 @@ void OptionsTab::SelectionWindow::recreate()
 			int count = 0;
 			for(auto & elem : allowedHeroes)
 			{
-				CHero * type = VLC->heroh->objects[elem];
+				const CHero * type = elem.toHeroType();
 				if(type->heroClass->faction == selectedFaction)
 				{
 					count++;

lib/mapObjectConstructors/CommonConstructors.cpp

@@ -77,7 +77,7 @@ void CTownInstanceConstructor::afterLoadFinalization()
 	{
 		filters[entry.first] = LogicalExpression<BuildingID>(entry.second, [this](const JsonNode & node)
 		{
-			return BuildingID(VLC->identifiers()->getIdentifier("building." + faction->getJsonKey(), node.Vector()[0]).value());
+			return BuildingID(VLC->identifiers()->getIdentifier("building." + faction->getJsonKey(), node.Vector()[0]).value_or(-1));
 		});
 	}
 }

lib/mapObjects/CArmedInstance.cpp

@@ -16,6 +16,7 @@
 #include "../CGeneralTextHandler.h"
 #include "../gameState/CGameState.h"
 #include "../CPlayerState.h"
+#include "../MetaString.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -110,7 +111,12 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 	else if (!factions.empty()) // no bonus from empty garrison
 	{
 		b->val = 2 - static_cast<si32>(factionsInArmy);
-		description = boost::str(boost::format(VLC->generaltexth->arraytxt[114]) % factionsInArmy % b->val); //Troops of %d alignments %d
+		MetaString formatter;
+		formatter.appendTextID("core.arraytxt.114"); //Troops of %d alignments %d
+		formatter.replaceNumber(factionsInArmy);
+		formatter.replaceNumber(b->val);
+
+		description = formatter.toString();
 		description = description.substr(0, description.size()-3);//trim value
 	}
 	

lib/mapping/MapFormatH3M.cpp

@@ -1145,42 +1145,47 @@ CGObjectInstance * CMapLoaderH3M::readWitchHut(const int3 & position, std::share
 	auto * object = readGeneric(position, objectTemplate);
 	auto * rewardable = dynamic_cast<CRewardableObject*>(object);
 
-	assert(rewardable);
-
 	// AB and later maps have allowed abilities defined in H3M
 	if(features.levelAB)
 	{
 		std::set<SecondarySkill> allowedAbilities;
 		reader->readBitmaskSkills(allowedAbilities, false);
 
-		if(allowedAbilities.size() != 1)
+		if (rewardable)
 		{
-			auto defaultAllowed = VLC->skillh->getDefaultAllowed();
+			if(allowedAbilities.size() != 1)
+			{
+				auto defaultAllowed = VLC->skillh->getDefaultAllowed();
 
-			for(int skillID = features.skillsCount; skillID < defaultAllowed.size(); ++skillID)
-				if(defaultAllowed.count(skillID))
-					allowedAbilities.insert(SecondarySkill(skillID));
-		}
+				for(int skillID = features.skillsCount; skillID < defaultAllowed.size(); ++skillID)
+					if(defaultAllowed.count(skillID))
+						allowedAbilities.insert(SecondarySkill(skillID));
+			}
 
-		JsonNode variable;
-		if (allowedAbilities.size() == 1)
-		{
-			variable.String() = VLC->skills()->getById(*allowedAbilities.begin())->getJsonKey();
+			JsonNode variable;
+			if (allowedAbilities.size() == 1)
+			{
+				variable.String() = VLC->skills()->getById(*allowedAbilities.begin())->getJsonKey();
+			}
+			else
+			{
+				JsonVector anyOfList;
+				for (auto const & skill : allowedAbilities)
+				{
+					JsonNode entry;
+					entry.String() = VLC->skills()->getById(skill)->getJsonKey();
+					anyOfList.push_back(entry);
+				}
+				variable["anyOf"].Vector() = anyOfList;
+			}
+
+			variable.setMeta(ModScope::scopeGame()); // list may include skills from all mods
+			rewardable->configuration.presetVariable("secondarySkill", "gainedSkill", variable);
 		}
 		else
 		{
-			JsonVector anyOfList;
-			for (auto const & skill : allowedAbilities)
-			{
-				JsonNode entry;
-				entry.String() = VLC->skills()->getById(skill)->getJsonKey();
-				anyOfList.push_back(entry);
-			}
-			variable["anyOf"].Vector() = anyOfList;
+			logGlobal->warn("Failed to set allowed secondary skills to a Witch Hut! Object is not rewardable!");
 		}
-
-		variable.setMeta(ModScope::scopeGame()); // list may include skills from all mods
-		rewardable->configuration.presetVariable("secondarySkill", "gainedSkill", variable);
 	}
 	return object;
 }
@@ -1362,16 +1367,21 @@ CGObjectInstance * CMapLoaderH3M::readShrine(const int3 & position, std::shared_
 	auto * object = readGeneric(position, objectTemplate);
 	auto * rewardable = dynamic_cast<CRewardableObject*>(object);
 
-	assert(rewardable);
-
 	SpellID spell = reader->readSpell32();
 
-	if(spell != SpellID::NONE)
+	if (rewardable)
+	{
+		if(spell != SpellID::NONE)
+		{
+			JsonNode variable;
+			variable.String() = VLC->spells()->getById(spell)->getJsonKey();
+			variable.setMeta(ModScope::scopeGame()); // list may include spells from all mods
+			rewardable->configuration.presetVariable("spell", "gainedSpell", variable);
+		}
+	}
+	else
 	{
-		JsonNode variable;
-		variable.String() = VLC->spells()->getById(spell)->getJsonKey();
-		variable.setMeta(ModScope::scopeGame()); // list may include spells from all mods
-		rewardable->configuration.presetVariable("spell", "gainedSpell", variable);
+		logGlobal->warn("Failed to set selected spell to a Shrine!. Object is not rewardable!");
 	}
 	return object;
 }

server/processors/TurnOrderProcessor.cpp

@@ -196,7 +196,10 @@ void TurnOrderProcessor::doStartNewDay()
 	}
 
 	if(!activePlayer)
+	{
 		gameHandler->gameLobby()->setState(EServerState::GAMEPLAY_ENDED);
+		return;
+	}
 
 	std::swap(actedPlayers, awaitingPlayers);