Merge pull request #4068 from IvanSavenko/bugfixing

[1.5.3] Bugfixing

client/battle/BattleWindow.cpp

@@ -763,8 +763,8 @@ void BattleWindow::blockUI(bool on)
 	setShortcutBlocked(EShortcut::BATTLE_SELECT_ACTION, on || owner.tacticsMode);
 	setShortcutBlocked(EShortcut::BATTLE_AUTOCOMBAT, (settings["battle"]["endWithAutocombat"].Bool() && onlyOnePlayerHuman) ? on || owner.tacticsMode || owner.actionsController->spellcastingModeActive() : owner.actionsController->spellcastingModeActive());
 	setShortcutBlocked(EShortcut::BATTLE_END_WITH_AUTOCOMBAT, on || owner.tacticsMode || !onlyOnePlayerHuman || owner.actionsController->spellcastingModeActive());
-	setShortcutBlocked(EShortcut::BATTLE_TACTICS_END, on && owner.tacticsMode);
-	setShortcutBlocked(EShortcut::BATTLE_TACTICS_NEXT, on && owner.tacticsMode);
+	setShortcutBlocked(EShortcut::BATTLE_TACTICS_END, on || !owner.tacticsMode);
+	setShortcutBlocked(EShortcut::BATTLE_TACTICS_NEXT, on || !owner.tacticsMode);
 	setShortcutBlocked(EShortcut::BATTLE_CONSOLE_DOWN, on && !owner.tacticsMode);
 	setShortcutBlocked(EShortcut::BATTLE_CONSOLE_UP, on && !owner.tacticsMode);
 }

client/windows/CExchangeWindow.cpp

@@ -46,9 +46,7 @@ static bool isQuickExchangeLayoutAvailable()
 
 CExchangeWindow::CExchangeWindow(ObjectInstanceID hero1, ObjectInstanceID hero2, QueryID queryID)
 	: CWindowObject(PLAYER_COLORED | BORDERED, ImagePath::builtin(isQuickExchangeLayoutAvailable() ? QUICK_EXCHANGE_BG : "TRADE2")),
-	controller(hero1, hero2),
-	moveStackLeftButtons(),
-	moveStackRightButtons()
+	controller(hero1, hero2)
 {
 	const bool qeLayout = isQuickExchangeLayoutAvailable();
 
@@ -193,54 +191,52 @@ CExchangeWindow::CExchangeWindow(ObjectInstanceID hero1, ObjectInstanceID hero2,
 
 	if(qeLayout)
 	{
-		moveAllGarrButtonLeft    = std::make_shared<CButton>(Point(325, 118), AnimationPath::builtin("quick-exchange/armRight.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[1]),
-			[this](){ this->moveUnitsShortcut(false); });
-		exchangeGarrButton       = std::make_shared<CButton>(Point(377, 118), AnimationPath::builtin("quick-exchange/swapAll.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[2]),
-			[this](){ controller.swapArmy(); });
-		moveAllGarrButtonRight   = std::make_shared<CButton>(Point(425, 118), AnimationPath::builtin("quick-exchange/armLeft.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[1]),
-			[this](){ this->moveUnitsShortcut(true); });
-		moveArtifactsButtonLeft  = std::make_shared<CButton>(Point(325, 154), AnimationPath::builtin("quick-exchange/artRight.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[3]),
-			[this](){ this->moveArtifactsCallback(false);});
-		exchangeArtifactsButton  = std::make_shared<CButton>(Point(377, 154), AnimationPath::builtin("quick-exchange/swapAll.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[4]),
-			[this](){ this->swapArtifactsCallback(); });
-		moveArtifactsButtonRight = std::make_shared<CButton>(Point(425, 154), AnimationPath::builtin("quick-exchange/artLeft.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[3]),
-			[this](){ this->moveArtifactsCallback(true);});
-
-		backpackButtonLeft       = std::make_shared<CButton>(Point(325, 518), AnimationPath::builtin("heroBackpack"), CButton::tooltipLocalized("vcmi.heroWindow.openBackpack"),
+		buttonMoveUnitsFromLeftToRight = std::make_shared<CButton>(Point(325, 118), AnimationPath::builtin("quick-exchange/armRight.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[1]), [this](){ this->moveUnitsShortcut(true); });
+		buttonMoveUnitsFromRightToLeft = std::make_shared<CButton>(Point(425, 118), AnimationPath::builtin("quick-exchange/armLeft.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[1]), [this](){ this->moveUnitsShortcut(false); });
+		buttonMoveArtifactsFromLeftToRight = std::make_shared<CButton>(Point(325, 154), AnimationPath::builtin("quick-exchange/artRight.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[3]), [this](){ this->moveArtifactsCallback(true);});
+		buttonMoveArtifactsFromRightToLeft = std::make_shared<CButton>(Point(425, 154), AnimationPath::builtin("quick-exchange/artLeft.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[3]), [this](){ this->moveArtifactsCallback(false);});
+
+		exchangeUnitsButton = std::make_shared<CButton>(Point(377, 118), AnimationPath::builtin("quick-exchange/swapAll.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[2]), [this](){ controller.swapArmy(); });
+		exchangeArtifactsButton  = std::make_shared<CButton>(Point(377, 154), AnimationPath::builtin("quick-exchange/swapAll.DEF"), CButton::tooltip(CGI->generaltexth->qeModCommands[4]), [this](){ this->swapArtifactsCallback(); });
+
+		backpackButtonLeft = std::make_shared<CButton>(Point(325, 518), AnimationPath::builtin("heroBackpack"), CButton::tooltipLocalized("vcmi.heroWindow.openBackpack"),
 			[this](){ this->backpackShortcut(true); });
-		backpackButtonRight      = std::make_shared<CButton>(Point(419, 518), AnimationPath::builtin("heroBackpack"), CButton::tooltipLocalized("vcmi.heroWindow.openBackpack"),
+		backpackButtonRight = std::make_shared<CButton>(Point(419, 518), AnimationPath::builtin("heroBackpack"), CButton::tooltipLocalized("vcmi.heroWindow.openBackpack"),
 			[this](){ this->backpackShortcut(false); });
 		backpackButtonLeft->setOverlay(std::make_shared<CPicture>(ImagePath::builtin("heroWindow/backpackButtonIcon")));
 		backpackButtonRight->setOverlay(std::make_shared<CPicture>(ImagePath::builtin("heroWindow/backpackButtonIcon")));
 
 		auto leftHeroBlock = heroInst[0]->tempOwner != LOCPLINT->cb->getPlayerID();
 		auto rightHeroBlock = heroInst[1]->tempOwner != LOCPLINT->cb->getPlayerID();
-		moveAllGarrButtonLeft->block(leftHeroBlock);
-		exchangeGarrButton->block(leftHeroBlock || rightHeroBlock);
-		moveAllGarrButtonRight->block(rightHeroBlock);
-		moveArtifactsButtonLeft->block(leftHeroBlock);
+
+		buttonMoveUnitsFromLeftToRight->block(leftHeroBlock);
+		buttonMoveUnitsFromRightToLeft->block(rightHeroBlock);
+		buttonMoveArtifactsFromLeftToRight->block(leftHeroBlock);
+		buttonMoveArtifactsFromRightToLeft->block(rightHeroBlock);
+
+		exchangeUnitsButton->block(leftHeroBlock || rightHeroBlock);
 		exchangeArtifactsButton->block(leftHeroBlock || rightHeroBlock);
-		moveArtifactsButtonRight->block(rightHeroBlock);
+
 		backpackButtonLeft->block(leftHeroBlock);
 		backpackButtonRight->block(rightHeroBlock);
 
 		for(int i = 0; i < GameConstants::ARMY_SIZE; i++)
 		{
-			moveStackLeftButtons.push_back(
+			moveUnitFromRightToLeftButtons.push_back(
 				std::make_shared<CButton>(
 					Point(484 + 35 * i, 154),
 					AnimationPath::builtin("quick-exchange/unitLeft.DEF"),
 					CButton::tooltip(CGI->generaltexth->qeModCommands[1]),
 					std::bind(&CExchangeController::moveStack, &controller, false, SlotID(i))));
-			moveStackLeftButtons.back()->block(leftHeroBlock);
+			moveUnitFromRightToLeftButtons.back()->block(leftHeroBlock);
 
-			moveStackRightButtons.push_back(
+			moveUnitFromLeftToRightButtons.push_back(
 				std::make_shared<CButton>(
 					Point(66 + 35 * i, 154),
 					AnimationPath::builtin("quick-exchange/unitRight.DEF"),
 					CButton::tooltip(CGI->generaltexth->qeModCommands[1]),
 					std::bind(&CExchangeController::moveStack, &controller, true, SlotID(i))));
-			moveStackLeftButtons.back()->block(rightHeroBlock);
+			moveUnitFromLeftToRightButtons.back()->block(rightHeroBlock);
 		}
 	}
 

client/windows/CExchangeWindow.h

@@ -41,14 +41,16 @@ class CExchangeWindow : public CStatusbarWindow, public IGarrisonHolder, public
 	std::array<std::shared_ptr<CButton>, 2> questlogButton;
 
 	std::shared_ptr<CGarrisonInt> garr;
-	std::shared_ptr<CButton> moveAllGarrButtonLeft;
-	std::shared_ptr<CButton> exchangeGarrButton;
-	std::shared_ptr<CButton> moveAllGarrButtonRight;
-	std::shared_ptr<CButton> moveArtifactsButtonLeft;
+	std::shared_ptr<CButton> buttonMoveUnitsFromLeftToRight;
+	std::shared_ptr<CButton> buttonMoveUnitsFromRightToLeft;
+	std::shared_ptr<CButton> buttonMoveArtifactsFromLeftToRight;
+	std::shared_ptr<CButton> buttonMoveArtifactsFromRightToLeft;
+
+	std::shared_ptr<CButton> exchangeUnitsButton;
 	std::shared_ptr<CButton> exchangeArtifactsButton;
-	std::shared_ptr<CButton> moveArtifactsButtonRight;
-	std::vector<std::shared_ptr<CButton>> moveStackLeftButtons;
-	std::vector<std::shared_ptr<CButton>> moveStackRightButtons;
+
+	std::vector<std::shared_ptr<CButton>> moveUnitFromLeftToRightButtons;
+	std::vector<std::shared_ptr<CButton>> moveUnitFromRightToLeftButtons;
 	std::shared_ptr<CButton> backpackButtonLeft;
 	std::shared_ptr<CButton> backpackButtonRight;
 	CExchangeController controller;

lib/CCreatureHandler.cpp

@@ -646,10 +646,15 @@ CCreature * CCreatureHandler::loadFromJson(const std::string & scope, const Json
 			registerObject(scope, type_name, extraName.String(), cre->getIndex());
 	}
 
+	if (!cre->special &&
+		!CResourceHandler::get()->existsResource(cre->animDefName) &&
+		!CResourceHandler::get()->existsResource(cre->animDefName.addPrefix("SPRITES/")))
+		throw ModLoadingException(scope, "creature " + cre->getJsonKey() + " has no combat animation but is not marked as special!" );
+
 	JsonNode advMapFile = node["graphics"]["map"];
 	JsonNode advMapMask = node["graphics"]["mapMask"];
 
-	VLC->identifiers()->requestIdentifier(scope, "object", "monster", [cre, scope, advMapFile, advMapMask](si32 index)
+	VLC->identifiers()->requestIdentifier(scope, "object", "monster", [cre, scope, advMapFile, advMapMask](si32 monsterIndex)
 	{
 		JsonNode conf;
 		conf.setModScope(scope);
@@ -672,7 +677,7 @@ CCreature * CCreatureHandler::loadFromJson(const std::string & scope, const Json
 		if (VLC->objtypeh->getHandlerFor(Obj::MONSTER, cre->getId().num)->getTemplates().empty())
 		{
 			if (!cre->special)
-				throw DataLoadingException("Mod " + scope + " is corrupted! Please disable or reinstall this mod. Reason: creature " + cre->getJsonKey() + " has no adventure map animation but is not marked as special!" );
+				throw ModLoadingException(scope, "creature " + cre->getJsonKey() + " has no adventure map animation but is not marked as special!" );
 
 			VLC->objtypeh->removeSubObject(Obj::MONSTER, cre->getId().num);
 		}
@@ -737,7 +742,6 @@ void CCreatureHandler::loadCrExpMod()
 	}
 }
 
-
 void CCreatureHandler::loadCrExpBon(CBonusSystemNode & globalEffects)
 {
 	if (VLC->settings()->getBoolean(EGameSettings::MODULE_STACK_EXPERIENCE)) 	//reading default stack experience bonuses

lib/CHeroHandler.cpp

@@ -218,8 +218,7 @@ void CHeroClass::serializeJson(JsonSerializeFormat & handler)
 CHeroClass::CHeroClass():
 	faction(0),
 	affinity(0),
-	defaultTavernChance(0),
-	commander(nullptr) 
+	defaultTavernChance(0)
 {
 }
 
@@ -302,7 +301,7 @@ CHeroClass * CHeroClassHandler::loadFromJson(const std::string & scope, const Js
 	VLC->identifiers()->requestIdentifier ("creature", node["commander"],
 	[=](si32 commanderID)
 	{
-		heroClass->commander = CreatureID(commanderID).toCreature();
+		heroClass->commander = CreatureID(commanderID);
 	});
 
 	heroClass->defaultTavernChance = static_cast<ui32>(node["defaultTavern"].Float());

lib/CHeroHandler.h

@@ -121,7 +121,7 @@ public:
 	// resulting chance = sqrt(town.chance * heroClass.chance)
 	ui32 defaultTavernChance;
 
-	const CCreature * commander;
+	CreatureID commander;
 
 	std::vector<int> primarySkillInitial;  // initial primary skills
 	std::vector<int> primarySkillLowLevel; // probability (%) of getting point of primary skill when getting level

lib/ExceptionsCommon.h

@@ -14,3 +14,11 @@ class DLL_LINKAGE DataLoadingException: public std::runtime_error
 public:
     using std::runtime_error::runtime_error;
 };
+
+class DLL_LINKAGE ModLoadingException: public DataLoadingException
+{
+public:
+	ModLoadingException(const std::string & modName, const std::string & reason)
+		: DataLoadingException("Mod " + modName + " is corrupted! Please disable or reinstall this mod. Reason: " + reason)
+	{}
+};

lib/gameState/CGameState.cpp

@@ -1499,8 +1499,8 @@ bool CGameState::checkForVictory(const PlayerColor & player, const EventConditio
 		case EventCondition::TRANSPORT:
 		{
 			const auto * t = getTown(condition.objectID);
-			return (t->visitingHero && t->visitingHero->hasArt(condition.objectType.as<ArtifactID>())) ||
-				   (t->garrisonHero && t->garrisonHero->hasArt(condition.objectType.as<ArtifactID>()));
+			return (t->visitingHero && t->visitingHero->getOwner() == player && t->visitingHero->hasArt(condition.objectType.as<ArtifactID>())) ||
+				   (t->garrisonHero && t->garrisonHero->getOwner() == player && t->garrisonHero->hasArt(condition.objectType.as<ArtifactID>()));
 		}
 		case EventCondition::DAYS_PASSED:
 		{

lib/mapObjects/CGHeroInstance.cpp

@@ -402,9 +402,9 @@ void CGHeroInstance::initHero(CRandomGenerator & rand)
 		addNewBonus(bonus);
 	}
 
-	if (VLC->settings()->getBoolean(EGameSettings::MODULE_COMMANDERS) && !commander)
+	if (VLC->settings()->getBoolean(EGameSettings::MODULE_COMMANDERS) && !commander && type->heroClass->commander.hasValue())
 	{
-		commander = new CCommanderInstance(type->heroClass->commander->getId());
+		commander = new CCommanderInstance(type->heroClass->commander);
 		commander->setArmyObj (castToArmyObj()); //TODO: separate function for setting commanders
 		commander->giveStackExp (exp); //after our exp is set
 	}

server/queries/BattleQueries.cpp

@@ -27,7 +27,7 @@ void CBattleQuery::notifyObjectAboutRemoval(const CObjectVisitQuery & objectVisi
 {
 	assert(result);
 
-	if(result && !isAiVsHuman)
+	if(result)
 		objectVisit.visitedObject->battleFinished(objectVisit.visitingHero, *result);
 }
 
@@ -38,8 +38,6 @@ CBattleQuery::CBattleQuery(CGameHandler * owner, const IBattleInfo * bi):
 	belligerents[0] = bi->getSideArmy(0);
 	belligerents[1] = bi->getSideArmy(1);
 
-	isAiVsHuman = bi->getSidePlayer(1).isValidPlayer() && gh->getPlayerState(bi->getSidePlayer(0))->isHuman() != gh->getPlayerState(bi->getSidePlayer(1))->isHuman();
-
 	addPlayer(bi->getSidePlayer(0));
 	addPlayer(bi->getSidePlayer(1));
 }
@@ -89,7 +87,9 @@ CBattleDialogQuery::CBattleDialogQuery(CGameHandler * owner, const IBattleInfo *
 
 void CBattleDialogQuery::onRemoval(PlayerColor color)
 {
-	if (!gh->getPlayerState(color)->isHuman())
+	// answer to this query was already processed when handling 1st player
+	// this removal call for 2nd player which can be safely ignored
+	if (resultProcessed)
 		return;
 
 	assert(answer);
@@ -108,13 +108,7 @@ void CBattleDialogQuery::onRemoval(PlayerColor color)
 	}
 	else
 	{
-		auto hero = bi->getSideHero(BattleSide::ATTACKER);
-		auto visitingObj = bi->getDefendedTown() ? bi->getDefendedTown() : gh->getVisitingObject(hero);
-		bool isAiVsHuman = bi->getSidePlayer(1).isValidPlayer() && gh->getPlayerState(bi->getSidePlayer(0))->isHuman() != gh->getPlayerState(bi->getSidePlayer(1))->isHuman();
-		
 		gh->battles->endBattleConfirm(bi->getBattleID());
-
-		if(visitingObj && result && isAiVsHuman)
-			visitingObj->battleFinished(hero, *result);
 	}
+	resultProcessed = true;
 }

server/queries/BattleQueries.h

@@ -23,7 +23,6 @@ public:
 	std::array<const CArmedInstance *,2> belligerents;
 	std::array<int, 2> initialHeroMana;
 
-	bool isAiVsHuman;
 	BattleID battleID;
 	std::optional<BattleResult> result;
 
@@ -37,6 +36,7 @@ public:
 
 class CBattleDialogQuery : public CDialogQuery
 {
+	bool resultProcessed = false;
 public:
 	CBattleDialogQuery(CGameHandler * owner, const IBattleInfo * Bi, std::optional<BattleResult> Br);