Fixes following review

AI/Nullkiller/AIGateway.cpp

@@ -788,29 +788,30 @@ bool AIGateway::makePossibleUpgrades(const CArmedInstance * obj)
 	{
 		if(const CStackInstance * s = obj->getStackPtr(SlotID(i)))
 		{
-			UpgradeInfo ui(s->getId());
+			UpgradeInfo upgradeInfo(s->getId());
 			do
 			{
-				myCb->fillUpgradeInfo(obj, SlotID(i), ui);
+				myCb->fillUpgradeInfo(obj, SlotID(i), upgradeInfo);
 
-				if(ui.hasUpgrades())
+				if(upgradeInfo.hasUpgrades())
 				{
 					// creature at given slot might have alternative upgrades, pick best one
-					CreatureID upgID = *vstd::maxElementByFun(ui.getAvailableUpgrades(), [](const CreatureID & id)
+					CreatureID upgID = *vstd::maxElementByFun(upgradeInfo.getAvailableUpgrades(), [](const CreatureID & id)
 						{
 							return id.toCreature()->getAIValue();
 						});
-					if(nullkiller->getFreeResources().canAfford(ui.getUpgradeCostsFor(upgID) * s->count))
+					if(nullkiller->getFreeResources().canAfford(upgradeInfo.getUpgradeCostsFor(upgID) * s->count))
 					{
 						myCb->upgradeCreature(obj, SlotID(i), upgID);
 						upgraded = true;
-						logAi->debug("Upgraded %d %s to %s", s->count, ui.oldID.toCreature()->getNamePluralTranslated(), ui.getNextUpgrade().toCreature()->getNamePluralTranslated());
+						logAi->debug("Upgraded %d %s to %s", s->count, upgradeInfo.oldID.toCreature()->getNamePluralTranslated(), 
+							upgradeInfo.getNextUpgrade().toCreature()->getNamePluralTranslated());
 					}
 					else
 						break;
 				}
 			}
-			while(ui.hasUpgrades());
+			while(upgradeInfo.hasUpgrades());
 		}
 	}
 

AI/VCAI/VCAI.cpp

@@ -754,28 +754,29 @@ void makePossibleUpgrades(const CArmedInstance * obj)
 	{
 		if(const CStackInstance * s = obj->getStackPtr(SlotID(i)))
 		{
-			UpgradeInfo ui(s->getId());
+			UpgradeInfo upgradeInfo(s->getId());
 			do
 			{
-				cb->fillUpgradeInfo(obj, SlotID(i), ui);
+				cb->fillUpgradeInfo(obj, SlotID(i), upgradeInfo);
 
-				if(ui.hasUpgrades())
+				if(upgradeInfo.hasUpgrades())
 				{
 					// creature at given slot might have alternative upgrades, pick best one
-					CreatureID upgID = *vstd::maxElementByFun(ui.getAvailableUpgrades(), [](const CreatureID & id)
+					CreatureID upgID = *vstd::maxElementByFun(upgradeInfo.getAvailableUpgrades(), [](const CreatureID & id)
 						{
 							return id.toCreature()->getAIValue();
 						});
-					if(cb->getResourceAmount().canAfford(ui.getUpgradeCostsFor(upgID) * s->count))
+					if(cb->getResourceAmount().canAfford(upgradeInfo.getUpgradeCostsFor(upgID) * s->count))
 					{
 						cb->upgradeCreature(obj, SlotID(i), upgID);
-						logAi->debug("Upgraded %d %s to %s", s->count, ui.oldID.toCreature()->getNamePluralTranslated(), ui.getNextUpgrade().toCreature()->getNamePluralTranslated());
+						logAi->debug("Upgraded %d %s to %s", s->count, upgradeInfo.oldID.toCreature()->getNamePluralTranslated(), 
+							upgradeInfo.getNextUpgrade().toCreature()->getNamePluralTranslated());
 					}
 					else
 						break;
 				}
 			}
-			while(ui.hasUpgrades());
+			while(upgradeInfo.hasUpgrades());
 		}
 	}
 }

client/widgets/CGarrisonInt.cpp

@@ -177,7 +177,7 @@ bool CGarrisonSlot::viewInfo()
 		elem->block(true);
 
 	redraw();
-	GH.windows().createAndPushWindow<CStackWindow>(myStack, dism, std::move(pom), upgr);
+	GH.windows().createAndPushWindow<CStackWindow>(myStack, dism, pom, upgr);
 	return true;
 }
 

client/windows/CCreatureWindow.cpp

@@ -58,8 +58,8 @@ public:
 	struct StackUpgradeInfo
 	{
 		StackUpgradeInfo() = delete;
-		StackUpgradeInfo(UpgradeInfo && upgradeInfo)
-			: info(std::move(upgradeInfo))
+		StackUpgradeInfo(const UpgradeInfo & upgradeInfo)
+			: info(upgradeInfo)
 		{ }
 		UpgradeInfo info;
 		std::function<void(CreatureID)> callback;
@@ -759,7 +759,7 @@ CStackWindow::CStackWindow(const CStackInstance * stack, bool popup)
 	init();
 }
 
-CStackWindow::CStackWindow(const CStackInstance * stack, std::function<void()> dismiss, UpgradeInfo && upgradeInfo, std::function<void(CreatureID)> callback)
+CStackWindow::CStackWindow(const CStackInstance * stack, std::function<void()> dismiss, const UpgradeInfo & upgradeInfo, std::function<void(CreatureID)> callback)
 	: CWindowObject(BORDERED),
 	info(new UnitView())
 {
@@ -767,7 +767,7 @@ CStackWindow::CStackWindow(const CStackInstance * stack, std::function<void()> d
 	info->creature = stack->getCreature();
 	info->creatureCount = stack->count;
 
-	info->upgradeInfo = std::make_optional(UnitView::StackUpgradeInfo(std::move(upgradeInfo)));
+	info->upgradeInfo = std::make_optional(UnitView::StackUpgradeInfo(upgradeInfo));
 	info->dismissInfo = std::make_optional(UnitView::StackDismissInfo());
 	info->upgradeInfo->callback = callback;
 	info->dismissInfo->callback = dismiss;

client/windows/CCreatureWindow.h

@@ -204,7 +204,7 @@ public:
 
 	// for normal stacks in armies
 	CStackWindow(const CStackInstance * stack, bool popup);
-	CStackWindow(const CStackInstance * stack, std::function<void()> dismiss, UpgradeInfo && info, std::function<void(CreatureID)> callback);
+	CStackWindow(const CStackInstance * stack, std::function<void()> dismiss, const UpgradeInfo & info, std::function<void(CreatureID)> callback);
 
 	// for commanders & commander level-up dialog
 	CStackWindow(const CCommanderInstance * commander, bool popup);

lib/gameState/CGameState.cpp

@@ -1779,11 +1779,12 @@ ArtifactID CGameState::pickRandomArtifact(vstd::RNG & rand, int flags)
 	return pickRandomArtifact(rand, flags, [](const ArtifactID &) { return true; });
 }
 
-void UpgradeInfo::addUpgrade(const CreatureID & upgradeID, ResourceSet && upgradeCost, bool available)
+void UpgradeInfo::addUpgrade(const CreatureID & upgradeID, const Creature * creature, int costModifier)
 {
-	isAvailable = available;
+	isAvailable = costModifier >= 0;
 	upgradesIDs.push_back(upgradeID);
 	
+	ResourceSet upgradeCost = (upgradeID.toCreature()->getFullRecruitCost() - creature->getFullRecruitCost()) * costModifier / 100;
 	upgradeCost.positive(); //upgrade cost can't be negative, ignore missing resources
 	upgradesCosts.push_back(std::move(upgradeCost));
 

lib/gameState/CGameState.h

@@ -49,7 +49,7 @@ public:
 
 	CreatureID oldID; //creature to be upgraded
 
-	const auto & getAvailableUpgrades() const
+	const std::vector<CreatureID> & getAvailableUpgrades() const
 	{
 		return upgradesIDs;
 	}
@@ -59,7 +59,7 @@ public:
 		return upgradesIDs.back();
 	}
 
-	const auto & getUpgradeCostsFor(CreatureID id) const
+	const ResourceSet & getUpgradeCostsFor(CreatureID id) const
 	{
 		auto idIt = std::find(upgradesIDs.begin(), upgradesIDs.end(), id);
 
@@ -68,12 +68,12 @@ public:
 		return upgradesCosts[std::distance(upgradesIDs.begin(), idIt)];
 	}
 
-	const auto & getUpgradeCosts() const
+	const std::vector<ResourceSet> & getUpgradeCosts() const
 	{
 		return upgradesCosts;
 	}
 
-	const auto & getNextUpgradeCosts() const
+	const ResourceSet & getNextUpgradeCosts() const
 	{
 		return upgradesCosts.back();
 	}
@@ -89,7 +89,7 @@ public:
 	}
 
 	// Adds a new upgrade and ensures alignment and sorted order
-	void addUpgrade(const CreatureID & upgradeID, ResourceSet && upgradeCost, bool available = true);
+	void addUpgrade(const CreatureID & upgradeID, const Creature * creature, int costModifier = 100);
 
 	auto size() const
 	{

lib/mapObjects/CGHeroInstance.cpp

@@ -1847,7 +1847,7 @@ bool CGHeroInstance::isMissionCritical() const
 	return false;
 }
 
-void CGHeroInstance::fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &stack) const
+void CGHeroInstance::fillUpgradeInfo(UpgradeInfo & info, const CStackInstance & stack) const
 {
 	TConstBonusListPtr lista = getBonuses(Selector::typeSubtype(BonusType::SPECIAL_UPGRADE, BonusSubtypeID(stack.getId())));
 	for(const auto & it : *lista)
@@ -1855,7 +1855,7 @@ void CGHeroInstance::fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &s
 		auto nid = CreatureID(it->additionalInfo[0]);
 		if (nid != stack.getId()) //in very specific case the upgrade is available by default (?)
 		{
-			info.addUpgrade(std::move(nid), nid.toCreature()->getFullRecruitCost() - stack.getType()->getFullRecruitCost());
+			info.addUpgrade(nid, stack.getType());
 		}
 	}
 }

lib/mapObjects/CGTownInstance.cpp

@@ -1234,7 +1234,7 @@ void CGTownInstance::fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &s
 			{
 				if(vstd::contains(stack.getCreature()->upgrades, upgrID)) //possible upgrade
 				{
-					info.addUpgrade(upgrID, upgrID.toCreature()->getFullRecruitCost() - stack.getType()->getFullRecruitCost());
+					info.addUpgrade(upgrID, stack.getType());
 				}
 			}
 		}

lib/mapObjects/MiscObjects.cpp

@@ -1323,14 +1323,12 @@ void HillFort::fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &stack)
 
 	int costModifier = upgradeCostPercentage[index];
 
-	if(costModifier < 0)
+	if(costModifier < 0) // upgrade not allowed
 		return;
 
 	for(const auto & nid : stack.getCreature()->upgrades)
 	{
-		info.addUpgrade(nid, 
-						(nid.toCreature()->getFullRecruitCost() - stack.getType()->getFullRecruitCost() * costModifier / 100), 
-						costModifier >= 0);
+		info.addUpgrade(nid, stack.getType(), costModifier / 100);
 	}
 }
 

server/CGameHandler.cpp

@@ -2414,18 +2414,18 @@ bool CGameHandler::upgradeCreature(ObjectInstanceID objid, SlotID pos, CreatureI
 	{
 		COMPLAIN_RET("Cannot upgrade, no stack at slot " + std::to_string(pos));
 	}
-	UpgradeInfo ui(obj->getStackPtr(pos)->getId());
-	fillUpgradeInfo(obj, pos, ui);
+	UpgradeInfo upgradeInfo(obj->getStackPtr(pos)->getId());
+	fillUpgradeInfo(obj, pos, upgradeInfo);
 	PlayerColor player = obj->tempOwner;
 	const PlayerState *p = getPlayerState(player);
 	int crQuantity = obj->stacks.at(pos)->count;
 
 	//check if upgrade is possible
-	if (!ui.hasUpgrades() && complain("That upgrade is not possible!"))
+	if (!upgradeInfo.hasUpgrades() && complain("That upgrade is not possible!"))
 	{
 		return false;
 	}
-	TResources totalCost = ui.getUpgradeCostsFor(upgID) * crQuantity;
+	TResources totalCost = upgradeInfo.getUpgradeCostsFor(upgID) * crQuantity;
 
 	//check if player has enough resources
 	if (!p->resources.canAfford(totalCost))