Remove non-const iterators to BattleHexArray container

Replaced with several non-const methods to prevent modification of
internal storage without corresponding modification of internal flags

AI/BattleAI/BattleEvaluator.cpp

@@ -371,7 +371,8 @@ BattleAction BattleEvaluator::goTowardsNearest(const CStack * stack, const Battl
 
 	if (siegeDefense)
 	{
-		vstd::erase_if(avHexes, [&](const BattleHex & hex) {
+		avHexes.erase_if([&](const BattleHex & hex)
+		{
 			return !cb->getBattle(battleID)->battleIsInsideWalls(hex);
 		});
 	}
@@ -383,7 +384,7 @@ BattleAction BattleEvaluator::goTowardsNearest(const CStack * stack, const Battl
 
 	BattleHexArray targetHexes = hexes;
 
-	std::sort(targetHexes.begin(), targetHexes.end(), [&](const BattleHex & h1, const BattleHex & h2) -> bool
+	targetHexes.sort([&](const BattleHex & h1, const BattleHex & h2) -> bool
 		{
 			return reachability.distances[h1.toInt()] < reachability.distances[h2.toInt()];
 		});

AI/StupidAI/StupidAI.cpp

@@ -289,7 +289,7 @@ BattleAction CStupidAI::goTowards(const BattleID & battleID, const CStack * stac
 		return BattleAction::makeDefend(stack);
 	}
 
-	std::sort(hexes.begin(), hexes.end(), [&](const BattleHex & h1, const BattleHex & h2) -> bool
+	hexes.sort([&](const BattleHex & h1, const BattleHex & h2) -> bool
 	{
 		return reachability.distances[h1.toInt()] < reachability.distances[h2.toInt()];
 	});

lib/battle/BattleHexArray.cpp

@@ -30,16 +30,6 @@ void BattleHexArray::insert(const BattleHexArray & other) noexcept
 	}
 }
 
-void BattleHexArray::erase(iterator first, iterator last) noexcept
-{
-	for(auto it = first; it != last && it != internalStorage.end(); ++it)
-	{
-		presenceFlags[it->toInt()] = false;
-	}
-
-	internalStorage.erase(first, last);
-}
-
 void BattleHexArray::clear() noexcept
 {
 	for(const auto & hex : internalStorage)
@@ -129,4 +119,4 @@ const std::map<BattleSide, BattleHexArray::ArrayOfBattleHexArrays> BattleHexArra
 	   { BattleSide::DEFENDER, precalculateNeighbouringTilesDoubleWide(BattleSide::DEFENDER) }
 	};
 
-VCMI_LIB_NAMESPACE_END
+VCMI_LIB_NAMESPACE_END

lib/battle/BattleHexArray.h

@@ -12,6 +12,7 @@
 
 #include "BattleHex.h"
 #include <boost/container/small_vector.hpp>
+#include <vstd/RNG.h>
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -41,9 +42,9 @@ public:
 	using pointer = value_type *;
 	using const_pointer = const value_type *;
 	using difference_type = typename StorageType::difference_type;
-	using iterator = typename StorageType::iterator;
+//	using iterator = typename StorageType::iterator;
 	using const_iterator = typename StorageType::const_iterator;
-	using reverse_iterator = typename StorageType::reverse_iterator;
+//	using reverse_iterator = typename StorageType::reverse_iterator;
 	using const_reverse_iterator = typename StorageType::const_reverse_iterator;
 
 	BattleHexArray() = default;
@@ -71,7 +72,7 @@ public:
 	{
 		if(tile.isAvailable() && !contains(tile))
 		{
-			presenceFlags[tile.toInt()] = true;
+			presenceFlags.set(tile.toInt());
 			internalStorage.emplace_back(tile);
 		}
 	}
@@ -81,7 +82,7 @@ public:
 		if(contains(hex))
 			return;
 
-		presenceFlags[hex.toInt()] = true;
+		presenceFlags.set(hex.toInt());
 		internalStorage.emplace_back(hex);
 	}
 
@@ -98,18 +99,18 @@ public:
 		if(contains(hex))
 			return;
 
-		presenceFlags[hex.toInt()] = true;
+		presenceFlags.set(hex.toInt());
 		internalStorage[index] = hex;
 	}
 
-	iterator insert(iterator pos, const BattleHex & hex) noexcept
-	{
-		if(contains(hex))
-			return pos;
-
-		presenceFlags[hex.toInt()] = true;
-		return internalStorage.insert(pos, hex);
-	}
+//	iterator insert(iterator pos, const BattleHex & hex) noexcept
+//	{
+//		if(contains(hex))
+//			return pos;
+//
+//		presenceFlags.set(hex.toInt());
+//		return internalStorage.insert(pos, hex);
+//	}
 
 	void insert(const BattleHexArray & other) noexcept;
 
@@ -123,17 +124,44 @@ public:
 		}
 	}
 
+	template<typename Predicate>
+	void sort(Predicate pred)
+	{
+		std::sort(internalStorage.begin(), internalStorage.end(), pred);
+	}
+
+	template<typename Predicate>
+	void erase_if(Predicate pred)
+	{
+		vstd::erase_if(internalStorage, pred);
+		// reinit presence flags
+		presenceFlags = {};
+		for(const auto & hex : internalStorage)
+			presenceFlags.set(hex.toInt()) = true;
+	}
+
+	void shuffle(vstd::RNG & rand)
+	{
+		int64_t n = internalStorage.size();
+
+		for(int64_t i = n - 1; i > 0; --i)
+		{
+			auto randIndex = rand.nextInt64(0, i);
+			std::swap(internalStorage[i], internalStorage[randIndex]);
+		}
+	}
+
 	void clear() noexcept;
 	inline void erase(const BattleHex & target) noexcept
 	{
 		assert(contains(target));
 		vstd::erase(internalStorage, target);
-		presenceFlags[target.toInt()] = false;
+		presenceFlags.reset(target.toInt());
 	}
-	void erase(iterator first, iterator last) noexcept;
+//	void erase(iterator first, iterator last) noexcept;
 	inline void pop_back() noexcept
 	{
-		presenceFlags[internalStorage.back().toInt()] = false;
+		presenceFlags.reset(internalStorage.back().toInt());
 		internalStorage.pop_back();
 	}
 
@@ -156,11 +184,11 @@ public:
 		return result;
 	}
 
-	template <typename Predicate>
-	iterator findIf(Predicate predicate) noexcept
-	{
-		return std::find_if(begin(), end(), predicate);
-	}
+//	template <typename Predicate>
+//	iterator findIf(Predicate predicate) noexcept
+//	{
+//		return std::find_if(begin(), end(), predicate);
+//	}
 
 	template <typename Predicate>
 	const_iterator findIf(Predicate predicate) const noexcept
@@ -216,7 +244,7 @@ public:
 	[[nodiscard]] inline bool contains(const BattleHex & hex) const noexcept
 	{
 		if(hex.isValid())
-			return presenceFlags[hex.toInt()];
+			return presenceFlags.test(hex.toInt());
 		
 		return true;
 	}
@@ -228,7 +256,7 @@ public:
 		if(!s.saving)
 		{
 			for(const auto & hex : internalStorage)
-				presenceFlags[hex.toInt()] = true;
+				presenceFlags.set(hex.toInt()) = true;
 		}
 	}
 
@@ -257,10 +285,10 @@ public:
 		return internalStorage.size();
 	}
 
-	[[nodiscard]] inline iterator begin() noexcept
-	{
-		return internalStorage.begin();
-	}
+//	[[nodiscard]] inline iterator begin() noexcept
+//	{
+//		return internalStorage.begin();
+//	}
 
 	[[nodiscard]] inline const_iterator begin() const noexcept
 	{
@@ -272,30 +300,30 @@ public:
 		return internalStorage.empty();
 	}
 
-	[[nodiscard]] inline iterator end() noexcept
-	{
-		return internalStorage.end();
-	}
+//	[[nodiscard]] inline iterator end() noexcept
+//	{
+//		return internalStorage.end();
+//	}
 
 	[[nodiscard]] inline const_iterator end() const noexcept
 	{
 		return internalStorage.end();
 	}
 
-	[[nodiscard]] inline reverse_iterator rbegin() noexcept
-	{
-		return reverse_iterator(end());
-	}
+//	[[nodiscard]] inline reverse_iterator rbegin() noexcept
+//	{
+//		return reverse_iterator(end());
+//	}
 
 	[[nodiscard]] inline const_reverse_iterator rbegin() const noexcept
 	{
 		return const_reverse_iterator(end());
 	}
 
-	[[nodiscard]] inline reverse_iterator rend() noexcept
-	{
-		return reverse_iterator(begin());
-	}
+//	[[nodiscard]] inline reverse_iterator rend() noexcept
+//	{
+//		return reverse_iterator(begin());
+//	}
 
 	[[nodiscard]] inline const_reverse_iterator rend() const noexcept
 	{

lib/battle/ReachabilityInfo.cpp

@@ -79,7 +79,7 @@ uint32_t ReachabilityInfo::distToNearestNeighbour(
 		}
 	}
 
-	vstd::erase_if(attackableHexes, [defender](const BattleHex & h) -> bool
+	attackableHexes.erase_if([defender](const BattleHex & h) -> bool
 		{
 			return h.getY() != defender->getPosition().getY() || !h.isAvailable();
 		});

lib/spells/effects/Obstacle.cpp

@@ -194,7 +194,7 @@ void Obstacle::apply(ServerCallback * server, const Mechanics * m, const EffectT
 			for(const auto & destination : target)
 				insertAvailable(destination.hexValue, availableTiles);
 
-		RandomGeneratorUtil::randomShuffle(availableTiles, *server->getRNG());
+		availableTiles.shuffle(*server->getRNG());
 		const int patchesToPut = std::min(patchCount, static_cast<int>(availableTiles.size()));
 		EffectTarget randomTarget;
 		randomTarget.reserve(patchesToPut);