Browse Source

SonarCloud recomendations.
Code review follow-up:
- Replace std::vector with boost::small_vector
- Rename function merge to insert

MichalZr6 9 months ago
parent
commit
ac8104d56d

+ 1 - 1
AI/BattleAI/BattleEvaluator.cpp

@@ -431,7 +431,7 @@ BattleAction BattleEvaluator::goTowardsNearest(const CStack * stack, BattleHexAr
 				auto triggerIsNegative = triggerAbility->isNegative() || triggerAbility->isDamage();
 
 				if(triggerIsNegative)
-					obstacleHexes.merge(obst->getAffectedTiles());
+					obstacleHexes.insert(obst->getAffectedTiles());
 			}
 		}
 		// Flying stack doesn't go hex by hex, so we can't backtrack using predecessors.

+ 1 - 1
AI/StupidAI/StupidAI.cpp

@@ -247,7 +247,7 @@ void CStupidAI::battleNewRound(const BattleID & battleID)
 	print("battleNewRound called");
 }
 
-void CStupidAI::battleStackMoved(const BattleID & battleID, const CStack * stack, BattleHexArray dest, int distance, bool teleport)
+void CStupidAI::battleStackMoved(const BattleID & battleID, const CStack * stack, const BattleHexArray & dest, int distance, bool teleport)
 {
 	print("battleStackMoved called");
 }

+ 1 - 1
AI/StupidAI/StupidAI.h

@@ -43,7 +43,7 @@ public:
 	//void battleResultsApplied() override; //called when all effects of last battle are applied
 	void battleNewRoundFirst(const BattleID & battleID) override; //called at the beginning of each turn before changes are applied;
 	void battleNewRound(const BattleID & battleID) override; //called at the beginning of each turn, round=-1 is the tactic phase, round=0 is the first "normal" turn
-	void battleStackMoved(const BattleID & battleID, const CStack * stack, BattleHexArray dest, int distance, bool teleport) override;
+	void battleStackMoved(const BattleID & battleID, const CStack * stack, const BattleHexArray & dest, int distance, bool teleport) override;
 	void battleSpellCast(const BattleID & battleID, const BattleSpellCast *sc) override;
 	void battleStacksEffectsSet(const BattleID & battleID, const SetStackEffect & sse) override;//called when a specific effect is set to stacks
 	//void battleTriggerEffect(const BattleTriggerEffect & bte) override;

+ 1 - 1
client/CPlayerInterface.cpp

@@ -846,7 +846,7 @@ void CPlayerInterface::battleLogMessage(const BattleID & battleID, const std::ve
 	battleInt->displayBattleLog(lines);
 }
 
-void CPlayerInterface::battleStackMoved(const BattleID & battleID, const CStack * stack, BattleHexArray dest, int distance, bool teleport)
+void CPlayerInterface::battleStackMoved(const BattleID & battleID, const CStack * stack, const BattleHexArray & dest, int distance, bool teleport)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
 	BATTLE_EVENT_POSSIBLE_RETURN;

+ 1 - 1
client/CPlayerInterface.h

@@ -154,7 +154,7 @@ protected: // Call-ins from server, should not be called directly, but only via
 	void battleNewRoundFirst(const BattleID & battleID) override; //called at the beginning of each turn before changes are applied; used for HP regen handling
 	void battleNewRound(const BattleID & battleID) override; //called at the beginning of each turn, round=-1 is the tactic phase, round=0 is the first "normal" turn
 	void battleLogMessage(const BattleID & battleID, const std::vector<MetaString> & lines) override;
-	void battleStackMoved(const BattleID & battleID, const CStack * stack, BattleHexArray dest, int distance, bool teleport) override;
+	void battleStackMoved(const BattleID & battleID, const CStack * stack, const BattleHexArray & dest, int distance, bool teleport) override;
 	void battleSpellCast(const BattleID & battleID, const BattleSpellCast *sc) override;
 	void battleStacksEffectsSet(const BattleID & battleID, const SetStackEffect & sse) override; //called when a specific effect is set to stacks
 	void battleTriggerEffect(const BattleID & battleID, const BattleTriggerEffect & bte) override; //various one-shot effect

+ 3 - 3
client/battle/BattleAnimationClasses.cpp

@@ -422,7 +422,7 @@ MovementAnimation::~MovementAnimation()
 		CCS->soundh->stopSound(moveSoundHandler);
 }
 
-MovementAnimation::MovementAnimation(BattleInterface & owner, const CStack *stack, BattleHexArray _destTiles, int _distance)
+MovementAnimation::MovementAnimation(BattleInterface & owner, const CStack *stack, const BattleHexArray & _destTiles, int _distance)
 	: StackMoveAnimation(owner, stack, stack->getPosition(), _destTiles.front()),
 	  destTiles(_destTiles),
 	  currentMoveIndex(0),
@@ -892,10 +892,10 @@ EffectAnimation::EffectAnimation(BattleInterface & owner, const AnimationPath &
 	logAnim->debug("CPointEffectAnimation::init: effect %s", animationName.getName());
 }
 
-EffectAnimation::EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, BattleHexArray hex, int effects, bool reversed):
+EffectAnimation::EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, const BattleHexArray & hexes, int effects, bool reversed):
 	EffectAnimation(owner, animationName, effects, 1.0f, reversed)
 {
-	battlehexes = hex;
+	battlehexes = hexes;
 }
 
 EffectAnimation::EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, BattleHex hex, int effects, float transparencyFactor, bool reversed):

+ 7 - 7
client/battle/BattleAnimationClasses.h

@@ -143,7 +143,7 @@ class MovementAnimation : public StackMoveAnimation
 private:
 	int moveSoundHandler; // sound handler used when moving a unit
 
-	BattleHexArray destTiles; //full path, includes already passed hexes
+	const BattleHexArray & destTiles; //full path, includes already passed hexes
 	ui32 currentMoveIndex; // index of nextHex in destTiles
 
 	double begX, begY; // starting position
@@ -159,7 +159,7 @@ public:
 	bool init() override;
 	void tick(uint32_t msPassed) override;
 
-	MovementAnimation(BattleInterface & owner, const CStack *_stack, BattleHexArray _destTiles, int _distance);
+	MovementAnimation(BattleInterface & owner, const CStack *_stack, const BattleHexArray & _destTiles, int _distance);
 	~MovementAnimation();
 };
 
@@ -339,14 +339,14 @@ public:
 	EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, int effects = 0, float transparencyFactor = 1.f, bool reversed = false);
 
 	/// Create animation positioned at point(s). Note that positions must be are absolute, including battleint position offset
-	EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, Point pos                 , int effects = 0, bool reversed = false);
-	EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, std::vector<Point> pos    , int effects = 0, bool reversed = false);
+	EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, Point pos                   , int effects = 0, bool reversed = false);
+	EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, std::vector<Point> pos      , int effects = 0, bool reversed = false);
 
 	/// Create animation positioned at certain hex(es)
-	EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, BattleHex hex             , int effects = 0, float transparencyFactor = 1.0f, bool reversed = false);
-	EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, BattleHexArray hex, int effects = 0, bool reversed = false);
+	EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, BattleHex hex               , int effects = 0, float transparencyFactor = 1.0f, bool reversed = false);
+	EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, const BattleHexArray & hexes, int effects = 0, bool reversed = false);
 
-	EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, Point pos, BattleHex hex,   int effects = 0, bool reversed = false);
+	EffectAnimation(BattleInterface & owner, const AnimationPath & animationName, Point pos, BattleHex hex,     int effects = 0, bool reversed = false);
 	 ~EffectAnimation();
 
 	bool init() override;

+ 1 - 1
client/battle/BattleFieldController.cpp

@@ -281,7 +281,7 @@ void BattleFieldController::redrawBackgroundWithHexes()
 	if(settings["battle"]["stackRange"].Bool())
 	{
 		BattleHexArray hexesToShade = occupiableHexes;
-		hexesToShade.merge(attackableHexes);
+		hexesToShade.insert(attackableHexes);
 		for(BattleHex hex : hexesToShade)
 		{
 			showHighlightedHex(*backgroundWithHexes, cellShade, hex, false);

+ 1 - 1
lib/CGameInterface.cpp

@@ -204,7 +204,7 @@ void CAdventureAI::battleObstaclesChanged(const BattleID & battleID, const std::
 	battleAI->battleObstaclesChanged(battleID, obstacles);
 }
 
-void CAdventureAI::battleStackMoved(const BattleID & battleID, const CStack * stack, BattleHexArray dest, int distance, bool teleport)
+void CAdventureAI::battleStackMoved(const BattleID & battleID, const CStack * stack, const BattleHexArray & dest, int distance, bool teleport)
 {
 	battleAI->battleStackMoved(battleID, stack, dest, distance, teleport);
 }

+ 1 - 1
lib/CGameInterface.h

@@ -151,7 +151,7 @@ public:
 	void actionFinished(const BattleID & battleID, const BattleAction &action) override;
 	void battleStacksEffectsSet(const BattleID & battleID, const SetStackEffect & sse) override;
 	void battleObstaclesChanged(const BattleID & battleID, const std::vector<ObstacleChanges> & obstacles) override;
-	void battleStackMoved(const BattleID & battleID, const CStack * stack, BattleHexArray dest, int distance, bool teleport) override;
+	void battleStackMoved(const BattleID & battleID, const CStack * stack, const BattleHexArray & dest, int distance, bool teleport) override;
 	void battleAttack(const BattleID & battleID, const BattleAttack *ba) override;
 	void battleSpellCast(const BattleID & battleID, const BattleSpellCast *sc) override;
 	void battleEnd(const BattleID & battleID, const BattleResult *br, QueryID queryID) override;

+ 1 - 1
lib/IGameEventsReceiver.h

@@ -63,7 +63,7 @@ public:
 	virtual void battleNewRoundFirst(const BattleID & battleID){}; //called at the beginning of each turn before changes are applied;
 	virtual void battleNewRound(const BattleID & battleID){}; //called at the beginning of each turn, round=-1 is the tactic phase, round=0 is the first "normal" turn
 	virtual void battleLogMessage(const BattleID & battleID, const std::vector<MetaString> & lines){};
-	virtual void battleStackMoved(const BattleID & battleID, const CStack * stack, BattleHexArray dest, int distance, bool teleport){};
+	virtual void battleStackMoved(const BattleID & battleID, const CStack * stack, const BattleHexArray & dest, int distance, bool teleport){};
 	virtual void battleSpellCast(const BattleID & battleID, const BattleSpellCast *sc){};
 	virtual void battleStacksEffectsSet(const BattleID & battleID, const SetStackEffect & sse){};//called when a specific effect is set to stacks
 	virtual void battleTriggerEffect(const BattleID & battleID, const BattleTriggerEffect & bte){}; //called for various one-shot effects

+ 2 - 3
lib/battle/BattleHex.h

@@ -97,9 +97,8 @@ struct DLL_LINKAGE BattleHex //TODO: decide if this should be changed to class f
 		int y1 = hex1.getY();
 		int y2 = hex2.getY();
 
-		// FIXME: why there was * 0.5 instead of / 2?
-		int x1 = static_cast<int>(hex1.getX() + y1 / 2);
-		int x2 = static_cast<int>(hex2.getX() + y2 / 2);
+		int x1 = hex1.getX() + y1 / 2;
+		int x2 = hex2.getX() + y2 / 2;
 
 		int xDst = x2 - x1;
 		int yDst = y2 - y1;

+ 1 - 1
lib/battle/BattleHexArray.cpp

@@ -120,7 +120,7 @@ BattleHexArray BattleHexArray::generateNeighbouringTiles(BattleHex hex)
 	return ret;
 }
 
-void BattleHexArray::merge(const BattleHexArray & other) noexcept
+void BattleHexArray::insert(const BattleHexArray & other) noexcept
 {
 	for(auto hex : other)
 	{

+ 24 - 35
lib/battle/BattleHexArray.h

@@ -11,6 +11,7 @@
 #pragma once
 
 #include "BattleHex.h"
+#include <boost/container/small_vector.hpp>
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -19,7 +20,7 @@ class DLL_LINKAGE BattleHexArray
 {
 public:
 	static constexpr uint8_t totalSize = GameConstants::BFIELD_SIZE;
-	using StorageType = std::vector<BattleHex>;
+	using StorageType = boost::container::small_vector<BattleHex, 8>;
 
 	using value_type = BattleHex;
 	using size_type = StorageType::size_type;
@@ -38,10 +39,7 @@ public:
 	static const ArrayOfBattleHexArrays neighbouringTilesCache;
 	static const std::map<BattleSide, ArrayOfBattleHexArrays> neighbouringTilesDblWide;
 
-	BattleHexArray() noexcept
-	{
-		internalStorage.reserve(totalSize);
-	}
+	BattleHexArray() = default;
 
 	template <typename Container, typename = std::enable_if_t<
 		std::is_convertible_v<typename Container::value_type, BattleHex>>>
@@ -95,9 +93,6 @@ public:
 
 	void insert(BattleHex hex) noexcept
 	{
-		/*if(isNotValidForInsertion(hex))
-			return;*/
-
 		if(contains(hex))
 			return;
 
@@ -107,9 +102,6 @@ public:
 
 	void set(size_type index, BattleHex hex)
 	{
-		/*if(isNotValidForInsertion(hex))
-			return;*/
-
 		if(index >= internalStorage.size())
 		{
 			logGlobal->error("Invalid BattleHexArray::set index parameter. It is " + std::to_string(index)
@@ -125,11 +117,8 @@ public:
 		internalStorage[index] = hex;
 	}
 
-	iterator BattleHexArray::insert(iterator pos, BattleHex hex) noexcept
+	iterator insert(iterator pos, BattleHex hex) noexcept
 	{
-		/*if(isNotValidForInsertion(hex))
-			return pos;*/
-
 		if(contains(hex))
 			return pos;
 
@@ -139,11 +128,11 @@ public:
 
 	BattleHex getClosestTile(BattleSide side, BattleHex initialPos) const;
 
-	void merge(const BattleHexArray & other) noexcept;
+	void insert(const BattleHexArray & other) noexcept;
 
 	template <typename Container, typename = std::enable_if_t<
 		std::is_convertible_v<typename Container::value_type, BattleHex>>>
-		void merge(const Container & container) noexcept
+		void insert(const Container & container) noexcept
 	{
 		for(auto value : container)
 		{
@@ -167,7 +156,7 @@ public:
 
 	inline std::vector<BattleHex> toVector() const noexcept
 	{
-		return internalStorage;
+		return std::vector<BattleHex>(internalStorage.begin(), internalStorage.end());
 	}
 
 	template <typename Predicate>
@@ -196,7 +185,7 @@ public:
 		return filtered;
 	}
 
-	[[nodiscard]] inline bool BattleHexArray::contains(BattleHex hex) const noexcept
+	[[nodiscard]] inline bool contains(BattleHex hex) const noexcept
 	{
 		if(hex.isValid())
 			return presenceFlags[hex];
@@ -220,22 +209,22 @@ public:
 		}
 	}
 
-	[[nodiscard]] inline const BattleHex & BattleHexArray::back() const noexcept
+	[[nodiscard]] inline const BattleHex & back() const noexcept
 	{
 		return internalStorage.back();
 	}
 
-	[[nodiscard]] inline const BattleHex & BattleHexArray::front() const noexcept
+	[[nodiscard]] inline const BattleHex & front() const noexcept
 	{
 		return internalStorage.front();
 	}
 
-	[[nodiscard]] inline const BattleHex & BattleHexArray::operator[](size_type index) const noexcept
+	[[nodiscard]] inline const BattleHex & operator[](size_type index) const noexcept
 	{
 		return internalStorage[index];
 	}
 
-	[[nodiscard]] inline const BattleHex & BattleHexArray::at(size_type index) const
+	[[nodiscard]] inline const BattleHex & at(size_type index) const
 	{
 		return internalStorage.at(index);
 	}
@@ -245,47 +234,47 @@ public:
 		return internalStorage.size();
 	}
 
-	[[nodiscard]] inline BattleHexArray::iterator BattleHexArray::begin() noexcept
+	[[nodiscard]] inline iterator begin() noexcept
 	{
 		return internalStorage.begin();
 	}
 
-	[[nodiscard]] inline BattleHexArray::const_iterator BattleHexArray::begin() const noexcept
+	[[nodiscard]] inline const_iterator begin() const noexcept
 	{
 		return internalStorage.begin();
 	}
 
-	[[nodiscard]] inline bool BattleHexArray::empty() const noexcept
+	[[nodiscard]] inline bool empty() const noexcept
 	{
 		return internalStorage.empty();
 	}
 
-	[[nodiscard]] inline BattleHexArray::iterator BattleHexArray::end() noexcept
+	[[nodiscard]] inline iterator end() noexcept
 	{
 		return internalStorage.end();
 	}
 
-	[[nodiscard]] inline BattleHexArray::const_iterator BattleHexArray::end() const noexcept
+	[[nodiscard]] inline const_iterator end() const noexcept
 	{
 		return internalStorage.end();
 	}
 
-	[[nodiscard]] inline BattleHexArray::reverse_iterator BattleHexArray::rbegin() noexcept
+	[[nodiscard]] inline reverse_iterator rbegin() noexcept
 	{
 		return reverse_iterator(end());
 	}
 
-	[[nodiscard]] inline BattleHexArray::const_reverse_iterator BattleHexArray::rbegin() const noexcept
+	[[nodiscard]] inline const_reverse_iterator rbegin() const noexcept
 	{
 		return const_reverse_iterator(end());
 	}
 
-	[[nodiscard]] inline BattleHexArray::reverse_iterator BattleHexArray::rend() noexcept
+	[[nodiscard]] inline reverse_iterator rend() noexcept
 	{
 		return reverse_iterator(begin());
 	}
 
-	[[nodiscard]] inline BattleHexArray::const_reverse_iterator BattleHexArray::rend() const noexcept
+	[[nodiscard]] inline const_reverse_iterator rend() const noexcept
 	{
 		return const_reverse_iterator(begin());
 	}
@@ -294,7 +283,7 @@ private:
 	StorageType internalStorage;
 	std::bitset<totalSize> presenceFlags = {};
 
-	[[nodiscard]] inline bool BattleHexArray::isNotValidForInsertion(BattleHex hex) const
+	[[nodiscard]] inline bool isNotValidForInsertion(BattleHex hex) const
 	{
 		if(isTower(hex))
 			return true;
@@ -313,8 +302,8 @@ private:
 	}
 
 	/// returns all valid neighbouring tiles
-	static BattleHexArray::ArrayOfBattleHexArrays calculateNeighbouringTiles();
-	static BattleHexArray::ArrayOfBattleHexArrays calculateNeighbouringTilesDblWide(BattleSide side);
+	static ArrayOfBattleHexArrays calculateNeighbouringTiles();
+	static ArrayOfBattleHexArrays calculateNeighbouringTilesDblWide(BattleSide side);
 	static BattleHexArray generateNeighbouringTiles(BattleHex hex);
 };
 

+ 4 - 4
lib/battle/CBattleInfoCallback.cpp

@@ -630,7 +630,7 @@ BattleHexArray CBattleInfoCallback::battleGetAvailableHexes(const battle::Unit *
 		for(auto hex : ret)
 			occupiable.insert(unit->occupiedHex(hex));
 
-		ret.merge(occupiable);
+		ret.insert(occupiable);
 	}
 
 
@@ -655,7 +655,7 @@ BattleHexArray CBattleInfoCallback::battleGetAvailableHexes(const battle::Unit *
 
 			if(battleCanShoot(unit, otherSt->getPosition()))
 			{
-				attackable->merge(occupied);
+				attackable->insert(occupied);
 				continue;
 			}
 
@@ -1336,7 +1336,7 @@ AttackableTiles CBattleInfoCallback::getPotentiallyAttackableHexes(
 	}
 	if(attacker->hasBonusOfType(BonusType::ATTACKS_ALL_ADJACENT))
 	{
-		at.hostileCreaturePositions.merge(attacker->getSurroundingHexes(attackerPos));
+		at.hostileCreaturePositions.insert(attacker->getSurroundingHexes(attackerPos));
 	}
 	if(attacker->hasBonusOfType(BonusType::THREE_HEADED_ATTACK))
 	{
@@ -1428,7 +1428,7 @@ AttackableTiles CBattleInfoCallback::getPotentiallyShootableHexes(const battle::
 
 	if(attacker->hasBonusOfType(BonusType::SHOOTS_ALL_ADJACENT) && !BattleHexArray::neighbouringTilesCache[attackerPos].contains(destinationTile))
 	{
-		at.hostileCreaturePositions.merge(BattleHexArray::neighbouringTilesCache[destinationTile]);
+		at.hostileCreaturePositions.insert(BattleHexArray::neighbouringTilesCache[destinationTile]);
 		at.hostileCreaturePositions.insert(destinationTile);
 	}
 

+ 2 - 2
lib/battle/ReachabilityInfo.cpp

@@ -71,11 +71,11 @@ uint32_t ReachabilityInfo::distToNearestNeighbour(
 		{
 			// It can be back to back attack  o==o  or head to head  =oo=.
 			// In case of back-to-back the distance between heads (unit positions) may be up to 3 tiles
-			attackableHexes.merge(battle::Unit::getHexes(defender->occupiedHex(), true, defender->unitSide()));
+			attackableHexes.insert(battle::Unit::getHexes(defender->occupiedHex(), true, defender->unitSide()));
 		}
 		else
 		{
-			attackableHexes.merge(battle::Unit::getHexes(defender->getPosition(), true, defender->unitSide()));
+			attackableHexes.insert(battle::Unit::getHexes(defender->getPosition(), true, defender->unitSide()));
 		}
 	}
 

+ 1 - 1
lib/battle/Unit.cpp

@@ -88,7 +88,7 @@ BattleHexArray Unit::getAttackableHexes(const Unit * attacker) const
 			hexes.pop_back();
 
 		for(auto hex : hexes)
-			targetableHexes.merge(BattleHexArray::neighbouringTilesCache[hex]);
+			targetableHexes.insert(BattleHexArray::neighbouringTilesCache[hex]);
 	}
 
 	return targetableHexes;

+ 1 - 1
lib/bonuses/Limiters.h

@@ -10,7 +10,7 @@
 
 #include "Bonus.h"
 
-#include "../battle/BattleHex.h"
+#include "../battle/BattleHexArray.h"
 #include "../serializer/Serializeable.h"
 #include "../constants/Enumerations.h"
 

+ 2 - 2
lib/spells/effects/Moat.cpp

@@ -103,7 +103,7 @@ void Moat::convertBonus(const Mechanics * m, std::vector<Bonus> & converted) con
 		BattleHexArray flatMoatHexes;
 
 		for(const auto & moatPatch : moatHexes)
-			flatMoatHexes.merge(moatPatch);
+			flatMoatHexes.insert(moatPatch);
 
 		nb.limiter = std::make_shared<UnitOnHexLimiter>(std::move(flatMoatHexes));
 		converted.push_back(nb);
@@ -168,7 +168,7 @@ void Moat::placeObstacles(ServerCallback * server, const Mechanics * m, const Ef
 		obstacle.appearSound = sideOptions.appearSound; //For dispellable moats
 		obstacle.appearAnimation = sideOptions.appearAnimation; //For dispellable moats
 		obstacle.animation = sideOptions.animation;
-		obstacle.customSize.merge(destination);
+		obstacle.customSize.insert(destination);
 		obstacle.animationYOffset = sideOptions.offsetY;
 		pack.changes.emplace_back();
 		obstacle.toInfo(pack.changes.back());