Merge pull request #3359 from IvanSavenko/crashfixes

Crashfixes

client/CMT.cpp

@@ -55,12 +55,14 @@
 namespace po = boost::program_options;
 namespace po_style = boost::program_options::command_line_style;
 
+static std::atomic<bool> quitRequestedDuringOpeningPlayback = false;
 static po::variables_map vm;
 
 #ifndef VCMI_IOS
 void processCommand(const std::string &message);
 #endif
 void playIntro();
+[[noreturn]] static void quitApplication();
 static void mainLoop();
 
 static CBasicLogConfigurator *logConfig;
@@ -313,7 +315,6 @@ int main(int argc, char * argv[])
 		GH.screenHandler().clearScreen();
 	}
 
-
 #ifndef VCMI_NO_THREADED_LOAD
 	#ifdef VCMI_ANDROID // android loads the data quite slowly so we display native progressbar to prevent having only black screen for few seconds
 	{
@@ -327,6 +328,9 @@ int main(int argc, char * argv[])
 	#endif // ANDROID
 #endif // THREADED
 
+	if (quitRequestedDuringOpeningPlayback)
+		quitApplication();
+
 	if(!settings["session"]["headless"].Bool())
 	{
 		pomtime.getDiff();
@@ -451,7 +455,7 @@ static void mainLoop()
 	}
 }
 
-static void quitApplication()
+[[noreturn]] static void quitApplication()
 {
 	if(!settings["session"]["headless"].Bool())
 	{
@@ -516,6 +520,15 @@ static void quitApplication()
 
 void handleQuit(bool ask)
 {
+	// FIXME: avoids crash if player attempts to close game while opening is still playing
+	// use cursor handler as indicator that loading is not done yet
+	// proper solution would be to abort init thread (or wait for it to finish)
+	if (!CCS->curh)
+	{
+		quitRequestedDuringOpeningPlayback = true;
+		return;
+	}
+
 	if(ask)
 	{
 		CCS->curh->set(Cursor::Map::POINTER);

client/NetPacksLobbyClient.cpp

@@ -47,7 +47,7 @@ void ApplyOnLobbyHandlerNetPackVisitor::visitLobbyClientConnected(LobbyClientCon
 
 void ApplyOnLobbyHandlerNetPackVisitor::visitLobbyClientDisconnected(LobbyClientDisconnected & pack)
 {
-	if(pack.clientId != pack.c->connectionID)
+	if(pack.clientId != handler.c->connectionID)
 	{
 		result = false;
 		return;

lib/bonuses/BonusEnum.h

@@ -20,8 +20,6 @@ class JsonNode;
 	BONUS_NAME(MOVEMENT) /*Subtype is 1 - land, 0 - sea*/ \
 	BONUS_NAME(MORALE) \
 	BONUS_NAME(LUCK) \
-	BONUS_NAME(MAX_MORALE) /*cheat bonus*/ \
-	BONUS_NAME(MAX_LUCK) /*cheat bonus*/ \
 	BONUS_NAME(PRIMARY_SKILL) /*uses subtype to pick skill; additional info if set: 1 - only melee, 2 - only distance*/  \
 	BONUS_NAME(SIGHT_RADIUS) \
 	BONUS_NAME(MANA_REGENERATION) /*points per turn*/  \
@@ -30,7 +28,6 @@ class JsonNode;
 	BONUS_NAME(SURRENDER_DISCOUNT) /*%*/  \
 	BONUS_NAME(STACKS_SPEED)  /*additional info - percent of speed bonus applied after direct bonuses; >0 - added, <0 - subtracted to this part*/ \
 	BONUS_NAME(FLYING_MOVEMENT) /*value - penalty percentage*/ \
-	BONUS_NAME(UNLIMITED_MOVEMENT) /*cheat bonus*/ \
 	BONUS_NAME(SPELL_DURATION) \
 	BONUS_NAME(WATER_WALKING) /*value - penalty percentage*/ \
 	BONUS_NAME(NEGATE_ALL_NATURAL_IMMUNITIES) \
@@ -172,7 +169,10 @@ class JsonNode;
 	BONUS_NAME(MAX_LEARNABLE_SPELL_LEVEL) /*This can work as wisdom before. val = max learnable spell level*/\
 	BONUS_NAME(SPELL_SCHOOL_IMMUNITY) /*This bonus will work as spell school immunity for all spells, subtype - spell school: 0 - air, 1 - fire, 2 - water, 3 - earth. Any is not handled for reducing overlap from LEVEL_SPELL_IMMUNITY*/\
 	BONUS_NAME(NEGATIVE_EFFECTS_IMMUNITY) /*This bonus will work as spell school immunity for negative effects from spells of school, subtype - spell school: -1 - any, 0 - air, 1 - fire, 2 - water, 3 - earth*/\
-	BONUS_NAME(TERRAIN_NATIVE)
+	BONUS_NAME(TERRAIN_NATIVE) \
+	BONUS_NAME(UNLIMITED_MOVEMENT) /*cheat bonus*/ \
+	BONUS_NAME(MAX_MORALE) /*cheat bonus*/ \
+	BONUS_NAME(MAX_LUCK) /*cheat bonus*/ \
 	/* end of list */
 
 

lib/rewardable/Configuration.cpp

@@ -34,14 +34,16 @@ std::optional<int> Rewardable::Configuration::getVariable(const std::string & ca
 	return std::nullopt;
 }
 
-JsonNode Rewardable::Configuration::getPresetVariable(const std::string & category, const std::string & name) const
+const JsonNode & Rewardable::Configuration::getPresetVariable(const std::string & category, const std::string & name) const
 {
+	static const JsonNode emptyNode;
+
 	std::string variableID = category + '@' + name;
 
 	if (variables.preset.count(variableID))
 		return variables.preset.at(variableID);
 	else
-		return JsonNode();
+		return emptyNode;
 }
 
 void Rewardable::Configuration::presetVariable(const std::string & category, const std::string & name, const JsonNode & value)

lib/rewardable/Configuration.h

@@ -167,7 +167,7 @@ struct DLL_LINKAGE Configuration
 	ui16 getResetDuration() const;
 
 	std::optional<int> getVariable(const std::string & category, const std::string & name) const;
-	JsonNode getPresetVariable(const std::string & category, const std::string & name) const;
+	const JsonNode & getPresetVariable(const std::string & category, const std::string & name) const;
 	void presetVariable(const std::string & category, const std::string & name, const JsonNode & value);
 	void initVariable(const std::string & category, const std::string & name, int value);
 	

lib/rewardable/Info.cpp

@@ -272,18 +272,18 @@ void Rewardable::Info::replaceTextPlaceholders(MetaString & target, const Variab
 
 void Rewardable::Info::configureRewards(
 		Rewardable::Configuration & object,
-		CRandomGenerator & rng, const
-		JsonNode & source,
+		CRandomGenerator & rng,
+		const JsonNode & source,
 		Rewardable::EEventType event,
 		const std::string & modeName) const
 {
 	for(size_t i = 0; i < source.Vector().size(); ++i)
 	{
-		const JsonNode reward = source.Vector()[i];
+		const JsonNode & reward = source.Vector().at(i);
 
 		if (!reward["appearChance"].isNull())
 		{
-			JsonNode chance = reward["appearChance"];
+			const JsonNode & chance = reward["appearChance"];
 			std::string diceID = std::to_string(chance["dice"].Integer());
 
 			auto diceValue = object.getVariable("dice", diceID);