BattleInfo stacks are now unique_ptr's

lib/CStack.cpp

@@ -341,9 +341,9 @@ const IBonusBearer* CStack::getBonusBearer() const
 
 bool CStack::unitHasAmmoCart(const battle::Unit * unit) const
 {
-	for(const CStack * st : battle->stacks)
+	for(const auto & st : battle->stacks)
 	{
-		if(battle->battleMatchOwner(st, unit, true) && st->unitType()->getId() == CreatureID::AMMO_CART)
+		if(battle->battleMatchOwner(st.get(), unit, true) && st->unitType()->getId() == CreatureID::AMMO_CART)
 		{
 			return st->alive();
 		}

lib/battle/BattleInfo.cpp

@@ -40,24 +40,22 @@ SideInBattle & BattleInfo::getSide(BattleSide side)
 }
 
 ///BattleInfo
-CStack * BattleInfo::generateNewStack(uint32_t id, const CStackInstance & base, BattleSide side, const SlotID & slot, const BattleHex & position)
+void BattleInfo::generateNewStack(uint32_t id, const CStackInstance & base, BattleSide side, const SlotID & slot, const BattleHex & position)
 {
 	PlayerColor owner = getSide(side).color;
 	assert(!owner.isValidPlayer() || (base.armyObj && base.armyObj->tempOwner == owner));
 
-	auto * ret = new CStack(&base, owner, id, side, slot);
+	auto ret = std::make_unique<CStack>(&base, owner, id, side, slot);
 	ret->initialPosition = getAvailableHex(base.getCreatureID(), side, position.toInt()); //TODO: what if no free tile on battlefield was found?
-	stacks.push_back(ret);
-	return ret;
+	stacks.push_back(std::move(ret));
 }
 
-CStack * BattleInfo::generateNewStack(uint32_t id, const CStackBasicDescriptor & base, BattleSide side, const SlotID & slot, const BattleHex & position)
+void BattleInfo::generateNewStack(uint32_t id, const CStackBasicDescriptor & base, BattleSide side, const SlotID & slot, const BattleHex & position)
 {
 	PlayerColor owner = getSide(side).color;
-	auto * ret = new CStack(&base, owner, id, side, slot);
+	auto ret = std::make_unique<CStack>(&base, owner, id, side, slot);
 	ret->initialPosition = position;
-	stacks.push_back(ret);
-	return ret;
+	stacks.push_back(std::move(ret));
 }
 
 void BattleInfo::localInit()
@@ -69,7 +67,7 @@ void BattleInfo::localInit()
 		armyObj->attachTo(*this);
 	}
 
-	for(CStack * s : stacks)
+	for(auto & s : stacks)
 		s->localInit(this);
 
 	exportBonuses();
@@ -166,8 +164,6 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 	for(auto i : { BattleSide::LEFT_SIDE, BattleSide::RIGHT_SIDE})
 		currentBattle->sides[i].init(heroes[i], armies[i]);
 
-	std::vector<CStack*> & stacks = (currentBattle->stacks);
-
 	currentBattle->tile = tile;
 	currentBattle->terrainType = terrain;
 	currentBattle->battlefieldType = battlefieldType;
@@ -371,7 +367,7 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 		//Moat generating is done on server
 	}
 
-	std::stable_sort(stacks.begin(),stacks.end(),cmpst);
+	std::stable_sort(currentBattle->stacks.begin(), currentBattle->stacks.end(), [cmpst](const auto & left, const auto & right){ return cmpst(left.get(), right.get());});
 
 	auto neutral = std::make_shared<CreatureAlignmentLimiter>(EAlignment::NEUTRAL);
 	auto good = std::make_shared<CreatureAlignmentLimiter>(EAlignment::GOOD);
@@ -502,8 +498,7 @@ std::optional<PlayerColor> BattleInfo::getPlayerID() const
 
 BattleInfo::~BattleInfo()
 {
-	for (auto & elem : stacks)
-		delete elem;
+	stacks.clear();
 
 	for(auto i : {BattleSide::ATTACKER, BattleSide::DEFENDER})
 		if(auto * _armyObj = battleGetArmyObject(i))
@@ -518,14 +513,18 @@ int32_t BattleInfo::getActiveStackID() const
 TStacks BattleInfo::getStacksIf(const TStackFilter & predicate) const
 {
 	TStacks ret;
-	vstd::copy_if(stacks, std::back_inserter(ret), predicate);
+	for (const auto & stack : stacks)
+		if (predicate(stack.get()))
+			ret.push_back(stack.get());
 	return ret;
 }
 
 battle::Units BattleInfo::getUnitsIf(const battle::UnitFilter & predicate) const
 {
 	battle::Units ret;
-	vstd::copy_if(stacks, std::back_inserter(ret), predicate);
+	for (const auto & stack : stacks)
+		if (predicate(stack.get()))
+			ret.push_back(stack.get());
 	return ret;
 }
 
@@ -643,7 +642,7 @@ void BattleInfo::nextRound()
 	}
 	round += 1;
 
-	for(CStack * s : stacks)
+	for(auto & s : stacks)
 	{
 		// new turn effects
 		s->reduceBonusDurations(Bonus::NTurns);
@@ -675,11 +674,11 @@ void BattleInfo::addUnit(uint32_t id, const JsonNode & data)
 
 	PlayerColor owner = getSidePlayer(info.side);
 
-	auto * ret = new CStack(&base, owner, info.id, info.side, SlotID::SUMMONED_SLOT_PLACEHOLDER);
+	auto ret = std::make_unique<CStack>(&base, owner, info.id, info.side, SlotID::SUMMONED_SLOT_PLACEHOLDER);
 	ret->initialPosition = info.position;
-	stacks.push_back(ret);
-	ret->localInit(this);
-	ret->summoned = info.summoned;
+	stacks.push_back(std::move(ret));
+	stacks.back()->localInit(this);
+	stacks.back()->summoned = info.summoned;
 }
 
 void BattleInfo::moveUnit(uint32_t id, const BattleHex & destination)
@@ -755,7 +754,7 @@ void BattleInfo::setUnitState(uint32_t id, const JsonNode & data, int64_t health
 
 	if(!changedStack->alive() && changedStack->isClone())
 	{
-		for(CStack * s : stacks)
+		for(auto & s : stacks)
 		{
 			if(s->cloneID == changedStack->unitId())
 				s->cloneID = -1;
@@ -793,7 +792,7 @@ void BattleInfo::removeUnit(uint32_t id)
 			}
 
 			//cleanup remaining clone links if any
-			for(auto * s : stacks)
+			for(auto & s : stacks)
 			{
 				if(s->cloneID == toRemoveId)
 					s->cloneID = -1;

lib/battle/BattleInfo.h

@@ -36,7 +36,7 @@ public:
 	const CGTownInstance * town; //used during town siege, nullptr if this is not a siege (note that fortless town IS also a siege)
 	int3 tile; //for background and bonuses
 	bool replayAllowed;
-	std::vector<CStack*> stacks;
+	std::vector<std::unique_ptr<CStack>> stacks;
 	std::vector<std::shared_ptr<CObstacleInstance> > obstacles;
 	SiegeInfo si;
 
@@ -145,8 +145,8 @@ public:
 	using CBattleInfoEssentials::battleGetFightingHero;
 	CGHeroInstance * battleGetFightingHero(BattleSide side) const;
 
-	CStack * generateNewStack(uint32_t id, const CStackInstance & base, BattleSide side, const SlotID & slot, const BattleHex & position);
-	CStack * generateNewStack(uint32_t id, const CStackBasicDescriptor & base, BattleSide side, const SlotID & slot, const BattleHex & position);
+	void generateNewStack(uint32_t id, const CStackInstance & base, BattleSide side, const SlotID & slot, const BattleHex & position);
+	void generateNewStack(uint32_t id, const CStackBasicDescriptor & base, BattleSide side, const SlotID & slot, const BattleHex & position);
 
 	const SideInBattle & getSide(BattleSide side) const;
 	SideInBattle & getSide(BattleSide side);

test/game/CGameStateTest.cpp

@@ -260,12 +260,12 @@ TEST_F(CGameStateTest, DISABLED_issue2765)
 	const CStack * att = nullptr;
 	const CStack * def = nullptr;
 
-	for(const CStack * s : gameState->currentBattles.front()->stacks)
+	for(const auto & s : gameState->currentBattles.front()->stacks)
 	{
 		if(s->unitType()->getId() == CreatureID::BALLISTA && s->unitSide() == BattleSide::DEFENDER)
-			def = s;
+			def = s.get();
 		else if(s->unitType()->getId() == CreatureID(69) && s->unitSide() == BattleSide::ATTACKER)
-			att = s;
+			att = s.get();
 	}
 	ASSERT_NE(att, nullptr);
 	ASSERT_NE(def, nullptr);