Stacks of armed instance are now unique_ptr

AI/Nullkiller/AIGateway.cpp

@@ -1180,7 +1180,7 @@ void AIGateway::recruitCreatures(const CGDwelling * d, const CArmedInstance * re
 
 		if(!recruiter->getSlotFor(creID).validSlot())
 		{
-			for(auto stack : recruiter->Slots())
+			for(const auto & stack : recruiter->Slots())
 			{
 				if(!stack.second->getType())
 					continue;

AI/Nullkiller/AIUtility.cpp

@@ -633,7 +633,7 @@ int getDuplicatingSlots(const CArmedInstance * army)
 {
 	int duplicatingSlots = 0;
 
-	for(auto stack : army->Slots())
+	for(const auto & stack : army->Slots())
 	{
 		if(stack.second->getCreature() && army->getSlotFor(stack.second->getCreature()) != stack.first)
 			duplicatingSlots++;
@@ -708,7 +708,7 @@ bool shouldVisit(const Nullkiller * ai, const CGHeroInstance * h, const CGObject
 	}
 	case Obj::HILL_FORT:
 	{
-		for(auto slot : h->Slots())
+		for(const auto & slot : h->Slots())
 		{
 			if(slot.second->getType()->hasUpgrades())
 				return true; //TODO: check price?

AI/Nullkiller/Analyzers/ArmyManager.cpp

@@ -489,7 +489,7 @@ void ArmyManager::update()
 
 	for(auto army : total)
 	{
-		for(auto slot : army->Slots())
+		for(const auto & slot : army->Slots())
 		{
 			totalArmy[slot.second->getCreatureID()].count += slot.second->count;
 		}
@@ -506,7 +506,7 @@ std::vector<SlotInfo> ArmyManager::convertToSlots(const CCreatureSet * army) con
 {
 	std::vector<SlotInfo> result;
 
-	for(auto slot : army->Slots())
+	for(const auto & slot : army->Slots())
 	{
 		SlotInfo slotInfo;
 
@@ -524,7 +524,7 @@ std::vector<StackUpgradeInfo> ArmyManager::getHillFortUpgrades(const CCreatureSe
 {
 	std::vector<StackUpgradeInfo> upgrades;
 
-	for(auto creature : army->Slots())
+	for(const auto & creature : army->Slots())
 	{
 		CreatureID initial = creature.second->getCreatureID();
 		auto possibleUpgrades = initial.toCreature()->upgrades;
@@ -552,7 +552,7 @@ std::vector<StackUpgradeInfo> ArmyManager::getDwellingUpgrades(const CCreatureSe
 {
 	std::vector<StackUpgradeInfo> upgrades;
 
-	for(auto creature : army->Slots())
+	for(const auto & creature : army->Slots())
 	{
 		CreatureID initial = creature.second->getCreatureID();
 		auto possibleUpgrades = initial.toCreature()->upgrades;

AI/Nullkiller/Engine/FuzzyEngines.cpp

@@ -61,7 +61,7 @@ armyStructure evaluateArmyStructure(const CArmedInstance * army)
 	static const CSelector selectorSTACKS_SPEED = Selector::type()(BonusType::STACKS_SPEED);
 	static const std::string keySTACKS_SPEED = "type_"+std::to_string((int32_t)BonusType::STACKS_SPEED);
 
-	for(auto s : army->Slots())
+	for(const auto & s : army->Slots())
 	{
 		bool walker = true;
 		auto bearer = s.second->getType()->getBonusBearer();

AI/Nullkiller/Engine/PriorityEvaluator.cpp

@@ -696,7 +696,7 @@ int32_t getArmyCost(const CArmedInstance * army)
 {
 	int32_t value = 0;
 
-	for(auto stack : army->Slots())
+	for(const auto & stack : army->Slots())
 	{
 		value += stack.second->getCreatureID().toCreature()->getFullRecruitCost().marketValue() * stack.second->count;
 	}
@@ -990,7 +990,7 @@ public:
 		std::map<int, float> totalPowerByCreatureID;
 
 		// Calculate hero power and total power by CreatureID
-		for (auto slot : hero->Slots())
+		for (const auto & slot : hero->Slots())
 		{
 			int creatureID = slot.second->getCreatureID();
 			float slotPower = slot.second->getPower();

AI/Nullkiller/Goals/BuyArmy.cpp

@@ -62,7 +62,7 @@ void BuyArmy::accept(AIGateway * ai)
 			{
 				SlotID lowestValueSlot;
 				int lowestValue = std::numeric_limits<int>::max();
-				for (auto slot : town->getUpperArmy()->Slots())
+				for (const auto & slot : town->getUpperArmy()->Slots())
 				{
 					if (slot.second->getCreatureID() != CreatureID::NONE)
 					{

AI/Nullkiller/Helpers/ArmyFormation.cpp

@@ -25,7 +25,7 @@ void ArmyFormation::addSingleCreatureStacks(const CGHeroInstance * hero)
 
 	while(!freeSlots.empty())
 	{
-		auto weakestCreature = vstd::minElementByFun(hero->Slots(), [](const std::pair<SlotID, CStackInstance *> & slot) -> int
+		TSlots::const_iterator weakestCreature = vstd::minElementByFun(hero->Slots(), [](const auto & slot) -> int
 			{
 				return slot.second->getCount() == 1
 					? std::numeric_limits<int>::max()
@@ -50,8 +50,8 @@ void ArmyFormation::rearrangeArmyForSiege(const CGTownInstance * town, const CGH
 	{
 		std::vector<CStackInstance *> stacks;
 
-		for(auto slot : attacker->Slots())
-			stacks.push_back(slot.second);
+		for(const auto & slot : attacker->Slots())
+			stacks.push_back(slot.second.get());
 
 		boost::sort(
 			stacks,
@@ -67,7 +67,7 @@ void ArmyFormation::rearrangeArmyForSiege(const CGTownInstance * town, const CGH
 
 		for(int i = 0; i < stacks.size(); i++)
 		{
-			auto pos = vstd::findKey(attacker->Slots(), stacks[i]);
+			auto pos = stacks[i]->armyObj->findStack(stacks[i]);
 
 			if(pos.getNum() != i)
 				cb->swapCreatures(attacker, attacker, static_cast<SlotID>(i), pos);

AI/Nullkiller/Pathfinding/AINodeStorage.cpp

@@ -284,7 +284,7 @@ void AINodeStorage::commit(CDestinationNodeInfo & destination, const PathNodeInf
 					return;
 				}
 
-				auto weakest = vstd::minElementByFun(dstNode->actor->creatureSet->Slots(), [](std::pair<SlotID, const CStackInstance *> pair) -> int
+				const auto & weakest = vstd::minElementByFun(dstNode->actor->creatureSet->Slots(), [](const auto & pair) -> int
 					{
 						return pair.second->getCount() * pair.second->getCreatureID().toCreature()->getAIValue();
 					});

AI/Nullkiller/Pathfinding/Actors.cpp

@@ -358,9 +358,9 @@ HeroExchangeArmy * HeroExchangeMap::tryUpgrade(
 	}
 	else
 	{
-		for(auto slot : army->Slots())
+		for(const auto & slot : army->Slots())
 		{
-			auto targetSlot = target->getSlotFor(slot.second->getCreatureID());
+			const auto & targetSlot = target->getSlotFor(slot.second->getCreatureID());
 
 			target->addToSlot(targetSlot, slot.second->getCreatureID(), slot.second->count);
 		}

AI/VCAI/FuzzyEngines.cpp

@@ -60,7 +60,7 @@ armyStructure evaluateArmyStructure(const CArmedInstance * army)
 	static const CSelector selectorSTACKS_SPEED = Selector::type()(BonusType::STACKS_SPEED);
 	static const std::string keySTACKS_SPEED = "type_"+std::to_string((int32_t)BonusType::STACKS_SPEED);
 
-	for(auto s : army->Slots())
+	for(const auto & s : army->Slots())
 	{
 		bool walker = true;
 		auto bearer = s.second->getType()->getBonusBearer();

AI/VCAI/Goals/GatherTroops.cpp

@@ -29,7 +29,7 @@ int GatherTroops::getCreaturesCount(const CArmedInstance * army)
 {
 	int count = 0;
 
-	for(auto stack : army->Slots())
+	for(const auto & stack : army->Slots())
 	{
 		if(objid == stack.second->getCreatureID().num)
 		{

AI/VCAI/VCAI.cpp

@@ -2851,7 +2851,7 @@ bool shouldVisit(HeroPtr h, const CGObjectInstance * obj)
 	}
 	case Obj::HILL_FORT:
 	{
-		for(auto slot : h->Slots())
+		for(const auto & slot : h->Slots())
 		{
 			if(slot.second->getType()->hasUpgrades())
 				return true; //TODO: check price?

Global.h

@@ -574,7 +574,7 @@ namespace vstd
 
 	//Returns iterator to the element for which the value of ValueFunction is minimal
 	template<class ForwardRange, class ValueFunction>
-	auto minElementByFun(const ForwardRange& rng, ValueFunction vf) -> decltype(std::begin(rng))
+	auto minElementByFun(const ForwardRange& rng, ValueFunction vf)
 	{
 		/* Clang crashes when instantiating this function template and having PCH compilation enabled.
 		 * There is a bug report here: http://llvm.org/bugs/show_bug.cgi?id=18744
@@ -589,7 +589,7 @@ namespace vstd
 
 	//Returns iterator to the element for which the value of ValueFunction is maximal
 	template<class ForwardRange, class ValueFunction>
-	auto maxElementByFun(const ForwardRange& rng, ValueFunction vf) -> decltype(std::begin(rng))
+	auto maxElementByFun(const ForwardRange& rng, ValueFunction vf)
 	{
 		/* Clang crashes when instantiating this function template and having PCH compilation enabled.
 		 * There is a bug report here: http://llvm.org/bugs/show_bug.cgi?id=18744

client/widgets/CExchangeController.cpp

@@ -27,14 +27,8 @@ CExchangeController::CExchangeController(ObjectInstanceID hero1, ObjectInstanceI
 
 void CExchangeController::swapArmy()
 {
-	auto getStacks = [](const CArmedInstance * source) -> std::vector<std::pair<SlotID, CStackInstance*>>
-	{
-		auto slots = source->Slots();
-		return std::vector<std::pair<SlotID, CStackInstance*>>(slots.begin(), slots.end());
-	};
-
-	auto leftSlots = getStacks(left);
-	auto rightSlots = getStacks(right);
+	const auto & leftSlots = left->Slots();
+	const auto & rightSlots = right->Slots();
 
 	auto i = leftSlots.begin();
 	auto j = rightSlots.begin();
@@ -73,8 +67,8 @@ void CExchangeController::moveArmy(bool leftToRight, std::optional<SlotID> heldS
 
 	if(!heldSlot.has_value())
 	{
-		auto weakestSlot = vstd::minElementByFun(source->Slots(),
-			[](const std::pair<SlotID, CStackInstance*> & s) -> int
+		const auto & weakestSlot = vstd::minElementByFun(source->Slots(),
+			[](const auto & s) -> int
 			{
 				return s.second->getCreatureID().toCreature()->getAIValue();
 			});

client/windows/CCastleInterface.cpp

@@ -344,9 +344,8 @@ auto CHeroGSlot::getUpgradableSlots(const CArmedInstance *obj) const
 {
 	struct result { bool isCreatureUpgradePossible; bool canAffordAny; bool canAffordAll; TResources totalCosts; std::vector<std::pair<SlotID, UpgradeInfo>> upgradeInfos; };
 
-	auto slots = std::map<SlotID, const CStackInstance*>(obj->Slots().begin(), obj->Slots().end());
 	std::vector<std::pair<SlotID, UpgradeInfo>> upgradeInfos;
-	for(const auto & slot : slots)
+	for(const auto & slot : obj->Slots())
 	{
 		auto upgradeInfo = std::make_pair(slot.first, UpgradeInfo(slot.second->getCreatureID()));
 		GAME->interface()->cb->fillUpgradeInfo(slot.second->armyObj, slot.first, upgradeInfo.second);
@@ -364,7 +363,7 @@ auto CHeroGSlot::getUpgradableSlots(const CArmedInstance *obj) const
 	std::vector<SlotID> slotInfosToDelete;
 	for(const auto & upgradeInfo : upgradeInfos)
 	{
-		TResources upgradeCosts = upgradeInfo.second.getUpgradeCosts() * slots[upgradeInfo.first]->getCount();
+		TResources upgradeCosts = upgradeInfo.second.getUpgradeCosts() * obj->Slots().at(upgradeInfo.first)->getCount();
 		if(GAME->interface()->cb->getResourceAmount().canAfford(costs + upgradeCosts))
 			costs += upgradeCosts;
 		else

lib/CCreatureSet.cpp

@@ -73,7 +73,7 @@ bool CCreatureSet::setCreature(SlotID slot, CreatureID type, TQuantity quantity)
 	auto * armyObj = castToArmyObj();
 	bool isHypotheticArmy = armyObj ? armyObj->isHypothetic() : false;
 
-	putStack(slot, new CStackInstance(type, quantity, isHypotheticArmy));
+	putStack(slot, std::make_unique<CStackInstance>(type, quantity, isHypotheticArmy));
 	return true;
 }
 
@@ -315,17 +315,17 @@ void CCreatureSet::addToSlot(const SlotID & slot, const CreatureID & cre, TQuant
 	}
 }
 
-void CCreatureSet::addToSlot(const SlotID & slot, CStackInstance * stack, bool allowMerging)
+void CCreatureSet::addToSlot(const SlotID & slot, std::unique_ptr<CStackInstance> stack, bool allowMerging)
 {
 	assert(stack->valid(true));
 
 	if(!hasStackAtSlot(slot))
 	{
-		putStack(slot, stack);
+		putStack(slot, std::move(stack));
 	}
 	else if(allowMerging && stack->getType() == getCreature(slot))
 	{
-		joinStack(slot, stack);
+		joinStack(slot, std::move(stack));
 	}
 	else
 	{
@@ -473,15 +473,14 @@ const CStackInstance & CCreatureSet::getStack(const SlotID & slot) const
 CStackInstance * CCreatureSet::getStackPtr(const SlotID & slot) const
 {
 	if(hasStackAtSlot(slot))
-		return stacks.find(slot)->second;
+		return stacks.find(slot)->second.get();
 	else return nullptr;
 }
 
 void CCreatureSet::eraseStack(const SlotID & slot)
 {
 	assert(hasStackAtSlot(slot));
-	CStackInstance *toErase = detachStack(slot);
-	vstd::clear_pointer(toErase);
+	detachStack(slot);
 }
 
 bool CCreatureSet::contains(const CStackInstance *stack) const
@@ -490,7 +489,7 @@ bool CCreatureSet::contains(const CStackInstance *stack) const
 		return false;
 
 	for(const auto & elem : stacks)
-		if(elem.second == stack)
+		if(elem.second.get() == stack)
 			return true;
 
 	return false;
@@ -506,7 +505,7 @@ SlotID CCreatureSet::findStack(const CStackInstance *stack) const
 		return SlotID();
 
 	for(const auto & elem : stacks)
-		if(elem.second == stack)
+		if(elem.second.get() == stack)
 			return elem.first;
 
 	return SlotID();
@@ -517,16 +516,16 @@ CArmedInstance * CCreatureSet::castToArmyObj()
 	return dynamic_cast<CArmedInstance *>(this);
 }
 
-void CCreatureSet::putStack(const SlotID & slot, CStackInstance * stack)
+void CCreatureSet::putStack(const SlotID & slot, std::unique_ptr<CStackInstance> stack)
 {
 	assert(slot.getNum() < GameConstants::ARMY_SIZE);
 	assert(!hasStackAtSlot(slot));
-	stacks[slot] = stack;
+	stacks[slot] = std::move(stack);
 	stack->setArmyObj(castToArmyObj());
 	armyChanged();
 }
 
-void CCreatureSet::joinStack(const SlotID & slot, CStackInstance * stack)
+void CCreatureSet::joinStack(const SlotID & slot, std::unique_ptr<CStackInstance> stack)
 {
 	[[maybe_unused]] const CCreature *c = getCreature(slot);
 	assert(c == stack->getType());
@@ -534,7 +533,6 @@ void CCreatureSet::joinStack(const SlotID & slot, CStackInstance * stack)
 
 	//TODO move stuff
 	changeStackCount(slot, stack->count);
-	vstd::clear_pointer(stack);
 }
 
 void CCreatureSet::changeStackCount(const SlotID & slot, TQuantity toAdd)
@@ -554,15 +552,15 @@ void CCreatureSet::setToArmy(CSimpleArmy &src)
 	{
 		auto i = src.army.begin();
 
-		putStack(i->first, new CStackInstance(i->second.first, i->second.second));
+		putStack(i->first, std::make_unique<CStackInstance>(i->second.first, i->second.second));
 		src.army.erase(i);
 	}
 }
 
-CStackInstance * CCreatureSet::detachStack(const SlotID & slot)
+std::unique_ptr<CStackInstance> CCreatureSet::detachStack(const SlotID & slot)
 {
 	assert(hasStackAtSlot(slot));
-	CStackInstance *ret = stacks[slot];
+	std::unique_ptr<CStackInstance> ret = std::move(stacks[slot]);
 
 	//if(CArmedInstance *armedObj = castToArmyObj())
 	if(ret)
@@ -579,8 +577,7 @@ CStackInstance * CCreatureSet::detachStack(const SlotID & slot)
 void CCreatureSet::setStackType(const SlotID & slot, const CreatureID & type)
 {
 	assert(hasStackAtSlot(slot));
-	CStackInstance *s = stacks[slot];
-	s->setType(type);
+	stacks[slot]->setType(type);
 	armyChanged();
 }
 
@@ -674,9 +671,9 @@ void CCreatureSet::serializeJson(JsonSerializeFormat & handler, const std::strin
 
 			if(amount > 0)
 			{
-				auto * new_stack = new CStackInstance();
+				auto new_stack = std::make_unique<CStackInstance>();
 				new_stack->serializeJson(handler);
-				putStack(SlotID(static_cast<si32>(idx)), new_stack);
+				putStack(SlotID(static_cast<si32>(idx)), std::move(new_stack));
 			}
 		}
 	}

lib/CCreatureSet.h

@@ -179,7 +179,7 @@ public:
 	}
 };
 
-using TSlots = std::map<SlotID, CStackInstance *>;
+using TSlots = std::map<SlotID, std::unique_ptr<CStackInstance>>;
 using TSimpleSlots = std::map<SlotID, std::pair<CreatureID, TQuantity>>;
 
 using TPairCreatureSlot = std::pair<const CCreature *, SlotID>;
@@ -238,22 +238,22 @@ public:
 	const TSlots &Slots() const {return stacks;}
 
 	void addToSlot(const SlotID & slot, const CreatureID & cre, TQuantity count, bool allowMerging = true); //Adds stack to slot. Slot must be empty or with same type creature
-	void addToSlot(const SlotID & slot, CStackInstance * stack, bool allowMerging = true); //Adds stack to slot. Slot must be empty or with same type creature
+	void addToSlot(const SlotID & slot, std::unique_ptr<CStackInstance> stack, bool allowMerging = true); //Adds stack to slot. Slot must be empty or with same type creature
 	void clearSlots() override;
 	void setFormation(EArmyFormation tight);
 	CArmedInstance *castToArmyObj();
 
 	//basic operations
-	void putStack(const SlotID & slot, CStackInstance * stack); //adds new stack to the army, slot must be empty
+	void putStack(const SlotID & slot, std::unique_ptr<CStackInstance> stack); //adds new stack to the army, slot must be empty
 	void setStackCount(const SlotID & slot, TQuantity count); //stack must exist!
-	CStackInstance * detachStack(const SlotID & slot); //removes stack from army but doesn't destroy it (so it can be moved somewhere else or safely deleted)
+	std::unique_ptr<CStackInstance> detachStack(const SlotID & slot); //removes stack from army but doesn't destroy it (so it can be moved somewhere else or safely deleted)
 	void setStackType(const SlotID & slot, const CreatureID & type);
 	void giveStackExp(TExpType exp);
 	void setStackExp(const SlotID & slot, TExpType exp);
 
 	//derivative
 	void eraseStack(const SlotID & slot); //slot must be occupied
-	void joinStack(const SlotID & slot, CStackInstance * stack); //adds new stack to the existing stack of the same type
+	void joinStack(const SlotID & slot, std::unique_ptr<CStackInstance> stack); //adds new stack to the existing stack of the same type
 	void changeStackCount(const SlotID & slot, TQuantity toAdd); //stack must exist!
 	bool setCreature (SlotID slot, CreatureID type, TQuantity quantity) override; //replaces creature in stack; slots 0 to 6, if quantity=0 erases stack
 	void setToArmy(CSimpleArmy &src); //erases all our army and moves stacks from src to us; src MUST NOT be an armed object! WARNING: use it wisely. Or better do not use at all.

lib/gameState/CGameStateCampaign.cpp

@@ -172,16 +172,21 @@ void CGameStateCampaign::trimCrossoverHeroesParameters(const CampaignTravel & tr
 	//trimming creatures
 	for(auto & hero : campaignHeroReplacements)
 	{
-		auto shouldSlotBeErased = [&](const std::pair<SlotID, CStackInstance *> & j) -> bool
+		auto shouldSlotBeErased = [&](CStackInstance & j) -> bool
 		{
-			CreatureID crid = j.second->getCreatureID();
+			CreatureID crid = j.getCreatureID();
 			return !travelOptions.monstersKeptByHero.count(crid);
 		};
 
-		auto stacksCopy = hero.hero->stacks; //copy of the map, so we can iterate iover it and remove stacks
-		for(auto &slotPair : stacksCopy)
-			if(shouldSlotBeErased(slotPair))
-				hero.hero->eraseStack(slotPair.first);
+		//generate list of slots without removing anything first to avoid iterator invalidation
+		std::vector<SlotID> slotsToErase;
+
+		for(auto &slotPair : hero.hero->Slots())
+			if(shouldSlotBeErased(*slotPair.second))
+				slotsToErase.push_back(slotPair.first);
+
+		for (const auto slotID : slotsToErase)
+			hero.hero->eraseStack(slotID);
 	}
 
 	// Removing short-term bonuses

lib/mapObjectConstructors/DwellingInstanceConstructor.cpp

@@ -104,7 +104,7 @@ void DwellingInstanceConstructor::randomizeObject(CGDwelling * dwelling, vstd::R
 		JsonRandom::Variables emptyVariables;
 		for(auto & stack : randomizer.loadCreatures(guards, rng, emptyVariables))
 		{
-			dwelling->putStack(SlotID(dwelling->stacksCount()), new CStackInstance(stack.getId(), stack.count));
+			dwelling->putStack(SlotID(dwelling->stacksCount()), std::make_unique<CStackInstance>(stack.getId(), stack.count));
 		}
 	}
 	else if (dwelling->ID == Obj::CREATURE_GENERATOR1 || dwelling->ID == Obj::CREATURE_GENERATOR4)
@@ -125,7 +125,7 @@ void DwellingInstanceConstructor::randomizeObject(CGDwelling * dwelling, vstd::R
 		for(auto creatureEntry : availableCreatures)
 		{
 			const CCreature * crea = creatureEntry.at(0);
-			dwelling->putStack(SlotID(dwelling->stacksCount()), new CStackInstance(crea->getId(), crea->getGrowth() * 3));
+			dwelling->putStack(SlotID(dwelling->stacksCount()), std::make_unique<CStackInstance>(crea->getId(), crea->getGrowth() * 3));
 		}
 	}
 }

lib/mapObjects/CArmedInstance.cpp

@@ -73,15 +73,14 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 
 	for(const auto & slot : Slots())
 	{
-		const CStackInstance * inst = slot.second;
-		const auto * creature  = inst->getCreatureID().toEntity(LIBRARY);
+		const auto * creature  = slot.second->getCreatureID().toEntity(LIBRARY);
 
 		factions.insert(creature->getFactionID());
 		// Check for undead flag instead of faction (undead mummies are neutral)
 		if (!hasUndead)
 		{
 			//this is costly check, let's skip it at first undead
-			hasUndead |= inst->hasBonus(undeadSelector, undeadCacheKey);
+			hasUndead |= slot.second->hasBonus(undeadSelector, undeadCacheKey);
 		}
 	}
 

lib/mapObjects/CGCreature.cpp

@@ -454,7 +454,7 @@ void CGCreature::joinDecision(const CGHeroInstance *h, int cost, ui32 accept) co
 
 		giveReward(h);
 
-		for(std::pair<const SlotID, CStackInstance*> stack : this->stacks)
+		for(auto & stack : this->stacks)
 			stack.second->count = getJoiningAmount();
 
 		cb->tryJoiningArmy(this, h, true, true);
@@ -672,10 +672,10 @@ void CGCreature::serializeJsonOptions(JsonSerializeFormat & handler)
 	{
 		si32 amount = 0;
 		handler.serializeInt("amount", amount);
-		auto * hlp = new CStackInstance();
+		auto hlp = std::make_unique<CStackInstance>();
 		hlp->count = amount;
 		//type will be set during initialization
-		putStack(SlotID(0), hlp);
+		putStack(SlotID(0), std::move(hlp));
 	}
 
 	resources.serializeJson(handler, "rewardResources");

lib/mapObjects/CGTownInstance.cpp

@@ -533,7 +533,7 @@ void CGTownInstance::initializeNeutralTownGarrison(vstd::RNG & rand)
 		CreatureID guardID = getTown()->creatures[guard.tier].at(0);
 		int guardSize = rand.nextInt(guard.min, guard.max);
 
-		putStack(getFreeSlot(), new CStackInstance(guardID, guardSize));
+		putStack(getFreeSlot(), std::make_unique<CStackInstance>(guardID, guardSize));
 	}
 }
 
@@ -583,33 +583,25 @@ void CGTownInstance::mergeGarrisonOnSiege() const
 {
 	auto getWeakestStackSlot = [&](ui64 powerLimit)
 	{
-		std::vector<SlotID> weakSlots;
-		auto stacksList = getVisitingHero()->stacks;
-		std::pair<SlotID, CStackInstance *> pair;
-		while(!stacksList.empty())
+		SlotID weakestSlot;
+
+		for ( const auto & pair : getVisitingHero()->stacks)
 		{
-			pair = *vstd::minElementByFun(stacksList, [&](const std::pair<SlotID, CStackInstance *> & elem) { return elem.second->getPower(); });
 			if(powerLimit > pair.second->getPower() &&
-				(weakSlots.empty() || pair.second->getPower() == getVisitingHero()->getStack(weakSlots.front()).getPower()))
+			   (weakestSlot == SlotID() || pair.second->getPower() < getVisitingHero()->getStack(weakestSlot).getPower()))
 			{
-				weakSlots.push_back(pair.first);
-				stacksList.erase(pair.first);
+				weakestSlot = pair.first;
 			}
-			else
-				break;
 		}
 
-		if(!weakSlots.empty())
-			return *std::max_element(weakSlots.begin(), weakSlots.end());
-
-		return SlotID();
+		return weakestSlot;
 	};
 
 	auto count = static_cast<int>(stacks.size());
 
 	for(int i = 0; i < count; i++)
 	{
-		auto pair = *vstd::maxElementByFun(stacks, [&](const std::pair<SlotID, CStackInstance *> & elem)
+		const auto & pair = *vstd::maxElementByFun(stacks, [&](const auto & elem)
 		{
 			ui64 power = elem.second->getPower();
 			auto dst = getVisitingHero()->getSlotFor(elem.second->getCreatureID());

lib/mapObjects/CRewardableObject.cpp

@@ -361,7 +361,7 @@ void CRewardableObject::initializeGuards()
 			if (!slotID.validSlot())
 				return;
 
-			putStack(slotID, new CStackInstance(guard.getId(), guard.getCount()));
+			putStack(slotID, std::make_unique<CStackInstance>(guard.getId(), guard.getCount()));
 		}
 	}
 }

lib/mapObjects/MiscObjects.cpp

@@ -102,8 +102,8 @@ void CGMine::initObj(vstd::RNG & rand)
 	{
 		//set guardians
 		int howManyTroglodytes = rand.nextInt(100, 199);
-		auto * troglodytes = new CStackInstance(CreatureID::TROGLODYTES, howManyTroglodytes);
-		putStack(SlotID(0), troglodytes);
+		auto troglodytes = std::make_unique<CStackInstance>(CreatureID::TROGLODYTES, howManyTroglodytes);
+		putStack(SlotID(0), std::move(troglodytes));
 
 		assert(!abandonedMineResources.empty());
 		if (!abandonedMineResources.empty())

lib/mapping/MapFormatH3M.cpp

@@ -1189,11 +1189,11 @@ std::shared_ptr<CGObjectInstance> CMapLoaderH3M::readMonster(const int3 & mapPos
 		map->questIdentifierToId[object->identifier] = objectInstanceID;
 	}
 
-	auto * hlp = new CStackInstance();
+	auto hlp = std::make_unique<CStackInstance>();
 	hlp->count = reader->readUInt16();
 
 	//type will be set during initialization
-	object->putStack(SlotID(0), hlp);
+	object->putStack(SlotID(0), std::move(hlp));
 
 	//TODO: 0-4 is h3 range. 5 is hota extension for exact aggression?
 	object->character = reader->readInt8Checked(0, 5);
@@ -1978,7 +1978,7 @@ void CMapLoaderH3M::readCreatureSet(CCreatureSet * out, int number)
 		if(creatureID == CreatureID::NONE)
 			continue;
 
-		auto * result = new CStackInstance();
+		auto result = std::make_unique<CStackInstance>();
 		result->count = count;
 
 		if(creatureID < CreatureID::NONE)
@@ -1996,7 +1996,7 @@ void CMapLoaderH3M::readCreatureSet(CCreatureSet * out, int number)
 			result->setType(creatureID);
 		}
 
-		out->putStack(SlotID(index), result);
+		out->putStack(SlotID(index), std::move(result));
 	}
 
 	out->validTypes(true);

lib/networkPacks/NetPacksLib.cpp

@@ -1544,17 +1544,17 @@ void SwapStacks::applyGs(CGameState *gs)
 	if(!dstObj)
 		throw std::runtime_error("SwapStacks: invalid army object " + std::to_string(dstArmy.getNum()) + ", possible game state corruption.");
 
-	CStackInstance * s1 = srcObj->detachStack(srcSlot);
-	CStackInstance * s2 = dstObj->detachStack(dstSlot);
+	auto s1 = srcObj->detachStack(srcSlot);
+	auto s2 = dstObj->detachStack(dstSlot);
 
-	srcObj->putStack(srcSlot, s2);
-	dstObj->putStack(dstSlot, s1);
+	srcObj->putStack(srcSlot, std::move(s2));
+	dstObj->putStack(dstSlot, std::move(s1));
 }
 
 void InsertNewStack::applyGs(CGameState *gs)
 {
 	if(auto * obj = gs->getArmyInstance(army))
-		obj->putStack(slot, new CStackInstance(type, count));
+		obj->putStack(slot, std::make_unique<CStackInstance>(type, count));
 	else
 		throw std::runtime_error("InsertNewStack: invalid army object " + std::to_string(army.getNum()) + ", possible game state corruption.");
 }
@@ -1636,8 +1636,8 @@ void RebalanceStacks::applyGs(CGameState *gs)
 		}
 		else //move stack to an empty slot, no exp change needed
 		{
-			CStackInstance *stackDetached = srcObj->detachStack(src.slot);
-			dstObj->putStack(dst.slot, stackDetached);
+			auto stackDetached = srcObj->detachStack(src.slot);
+			dstObj->putStack(dst.slot, std::move(stackDetached));
 		}
 	}
 	else

lib/rewardable/Interface.cpp

@@ -179,7 +179,7 @@ void Rewardable::Interface::grantRewardAfterLevelup(const Rewardable::VisitInfo
 	{
 		for(const auto & slot : hero->Slots())
 		{
-			const CStackInstance * heroStack = slot.second;
+			const auto & heroStack = slot.second;
 
 			for(const auto & change : info.reward.creaturesChange)
 			{
@@ -197,7 +197,7 @@ void Rewardable::Interface::grantRewardAfterLevelup(const Rewardable::VisitInfo
 	{
 		CCreatureSet creatures;
 		for(const auto & crea : info.reward.creatures)
-			creatures.addToSlot(creatures.getFreeSlot(), new CStackInstance(crea.getCreature(), crea.count));
+			creatures.addToSlot(creatures.getFreeSlot(), std::make_unique<CStackInstance>(crea.getCreature(), crea.count));
 
 		if(auto * army = dynamic_cast<const CArmedInstance*>(this)) //TODO: to fix that, CArmedInstance must be split on map instance part and interface part
 			cb->giveCreatures(army, hero, creatures, false);

lib/rewardable/Limiter.cpp

@@ -83,7 +83,7 @@ bool Rewardable::Limiter::heroAllowed(const CGHeroInstance * hero) const
 		size_t count = 0;
 		for(const auto & slot : hero->Slots())
 		{
-			const CStackInstance * heroStack = slot.second;
+			const auto & heroStack = slot.second;
 			if (heroStack->getType() == reqStack.getType())
 				count += heroStack->count;
 		}

lib/rmg/modificators/ObjectManager.cpp

@@ -777,9 +777,9 @@ std::shared_ptr<CGCreature> ObjectManager::chooseGuard(si32 strength, bool zoneG
 
 	auto guard = std::dynamic_pointer_cast<CGCreature>(guardFactory->create(map.mapInstance->cb, nullptr));
 	guard->character = CGCreature::HOSTILE;
-	auto * hlp = new CStackInstance(creId, amount);
+	auto hlp = std::make_unique<CStackInstance>(creId, amount);
 	//will be set during initialization
-	guard->putStack(SlotID(0), hlp);
+	guard->putStack(SlotID(0), std::move(hlp));
 	return guard;
 }
 

mapeditor/inspector/armywidget.cpp

@@ -99,7 +99,7 @@ bool ArmyWidget::commitChanges()
 			{
 				if(army.hasStackAtSlot(SlotID(i)))
 					army.eraseStack(SlotID(i));
-				army.putStack(SlotID(i), new CStackInstance(creId, amount, false));
+				army.putStack(SlotID(i), std::make_unique<CStackInstance>(creId, amount, false));
 			}
 		}
 	}

mapeditor/inspector/inspector.cpp

@@ -79,7 +79,7 @@ void Initializer::initialize(CGCreature * o)
 	
 	o->character = CGCreature::Character::HOSTILE;
 	if(!o->hasStackAtSlot(SlotID(0)))
-	   o->putStack(SlotID(0), new CStackInstance(CreatureID(o->subID), 0, false));
+		o->putStack(SlotID(0), std::make_unique<CStackInstance>(CreatureID(o->subID), 0, false));
 }
 
 void Initializer::initialize(CGDwelling * o)

server/CGameHandler.cpp

@@ -4287,7 +4287,7 @@ void CGameHandler::createWanderingMonster(const int3 & visitablePosition, Creatu
 	cre->character = 2;
 	cre->gainedArtifact = ArtifactID::NONE;
 	cre->identifier = -1;
-	cre->addToSlot(SlotID(0), new CStackInstance(creature, -1)); //add placeholder stack
+	cre->addToSlot(SlotID(0), std::make_unique<CStackInstance>(creature, -1)); //add placeholder stack
 
 	newObject(createdObject, PlayerColor::NEUTRAL);
 }

server/processors/NewTurnProcessor.cpp

@@ -227,12 +227,12 @@ ResourceSet NewTurnProcessor::generatePlayerIncome(PlayerColor playerID, bool ne
 	{
 		bool hasCrystalGenCreature = false;
 		for (const auto & hero : state.getHeroes())
-			for(auto stack : hero->stacks)
+			for(const auto & stack : hero->stacks)
 				if(stack.second->hasBonusOfType(BonusType::SPECIAL_CRYSTAL_GENERATION))
 					hasCrystalGenCreature = true;
 
 		for(const auto & town : state.getTowns())
-			for(auto stack : town->stacks)
+			for(const auto & stack : town->stacks)
 				if(stack.second->hasBonusOfType(BonusType::SPECIAL_CRYSTAL_GENERATION))
 					hasCrystalGenCreature = true;