Browse Source

Fixes for multiple new issues from Sonar

Ivan Savenko 8 tháng trước cách đây
mục cha
commit
2362c6da21
50 tập tin đã thay đổi với 190 bổ sung223 xóa
  1. 2 2
      AI/BattleAI/BattleEvaluator.cpp
  2. 3 3
      AI/BattleAI/BattleExchangeVariant.cpp
  3. 3 4
      AI/BattleAI/BattleExchangeVariant.h
  4. 3 3
      AI/Nullkiller/Analyzers/BuildAnalyzer.cpp
  5. 1 1
      AI/Nullkiller/Behaviors/DefenceBehavior.cpp
  6. 29 32
      AI/Nullkiller/Engine/PriorityEvaluator.cpp
  7. 4 4
      AI/Nullkiller/Pathfinding/AINodeStorage.h
  8. 2 2
      AI/Nullkiller/Pathfinding/AIPathfinderConfig.cpp
  9. 1 1
      AI/VCAI/BuildingManager.cpp
  10. 1 1
      client/adventureMap/CMinimap.cpp
  11. 8 8
      client/battle/BattleFieldController.cpp
  12. 5 5
      client/battle/BattleFieldController.h
  13. 10 12
      client/render/AssetGenerator.cpp
  14. 7 9
      client/render/AssetGenerator.h
  15. 8 8
      client/renderSDL/CTrueTypeFont.cpp
  16. 6 13
      client/renderSDL/RenderHandler.cpp
  17. 1 1
      client/renderSDL/RenderHandler.h
  18. 5 5
      client/renderSDL/ScalableImage.cpp
  19. 10 10
      client/windows/CCastleInterface.cpp
  20. 1 1
      client/windows/CCastleInterface.h
  21. 2 2
      client/windows/CMapOverview.cpp
  22. 1 1
      client/windows/CMapOverview.h
  23. 3 1
      client/windows/GUIClasses.cpp
  24. 2 2
      launcher/modManager/chroniclesextractor.cpp
  25. 3 3
      lib/CCreatureHandler.cpp
  26. 12 28
      lib/CCreatureSet.cpp
  27. 1 3
      lib/CCreatureSet.h
  28. 1 1
      lib/battle/BattleHexArray.h
  29. 4 10
      lib/battle/CBattleInfoCallback.cpp
  30. 2 5
      lib/battle/CUnitState.cpp
  31. 4 4
      lib/constants/EntityIdentifiers.h
  32. 1 1
      lib/entities/faction/CTownHandler.cpp
  33. 2 2
      lib/mapObjects/CGHeroInstance.cpp
  34. 1 1
      lib/mapObjects/CQuest.cpp
  35. 2 2
      lib/mapping/CMapHeader.cpp
  36. 2 3
      lib/mapping/MapFormatH3M.cpp
  37. 1 1
      lib/mapping/MapReaderH3M.cpp
  38. 2 2
      lib/network/NetworkConnection.cpp
  39. 2 2
      lib/networkPacks/NetPacksLib.cpp
  40. 1 1
      lib/pathfinder/PathfinderCache.cpp
  41. 1 1
      lib/rmg/CZonePlacer.cpp
  42. 1 1
      lib/rmg/modificators/ObjectManager.cpp
  43. 3 3
      mapeditor/inspector/armywidget.cpp
  44. 5 3
      mapeditor/inspector/heroskillswidget.cpp
  45. 3 1
      mapeditor/inspector/herospellwidget.cpp
  46. 2 2
      mapeditor/inspector/inspector.cpp
  47. 3 1
      mapeditor/inspector/portraitwidget.cpp
  48. 7 7
      mapeditor/inspector/questwidget.cpp
  49. 3 1
      mapeditor/inspector/townbuildingswidget.cpp
  50. 3 3
      server/CGameHandler.cpp

+ 2 - 2
AI/BattleAI/BattleEvaluator.cpp

@@ -384,7 +384,7 @@ BattleAction BattleEvaluator::goTowardsNearest(const CStack * stack, const Battl
 
 	BattleHexArray targetHexes = hexes;
 
-	targetHexes.sort([&](const BattleHex & h1, const BattleHex & h2) -> bool
+	targetHexes.sort([&reachability](const BattleHex & h1, const BattleHex & h2) -> bool
 		{
 			return reachability.distances[h1.toInt()] < reachability.distances[h2.toInt()];
 		});
@@ -435,7 +435,7 @@ BattleAction BattleEvaluator::goTowardsNearest(const CStack * stack, const Battl
 		}
 		// Flying stack doesn't go hex by hex, so we can't backtrack using predecessors.
 		// We just check all available hexes and pick the one closest to the target.
-		auto nearestAvailableHex = vstd::minElementByFun(avHexes, [&](const BattleHex & hex) -> int
+		auto nearestAvailableHex = vstd::minElementByFun(avHexes, [this, &bestNeighbour, &stack, &obstacleHexes](const BattleHex & hex) -> int
 		{
 			const int NEGATIVE_OBSTACLE_PENALTY = 100; // avoid landing on negative obstacle (moat, fire wall, etc)
 			const int BLOCKED_STACK_PENALTY = 100; // avoid landing on moat

+ 3 - 3
AI/BattleAI/BattleExchangeVariant.cpp

@@ -506,7 +506,7 @@ ReachabilityData BattleExchangeEvaluator::getExchangeUnits(
 	uint8_t turn,
 	PotentialTargets & targets,
 	std::shared_ptr<HypotheticBattle> hb,
-	battle::Units additionalUnits) const
+	const battle::Units & additionalUnits) const
 {
 	ReachabilityData result;
 
@@ -636,7 +636,7 @@ BattleScore BattleExchangeEvaluator::calculateExchange(
 	PotentialTargets & targets,
 	DamageCache & damageCache,
 	std::shared_ptr<HypotheticBattle> hb,
-	battle::Units additionalUnits) const
+	const battle::Units & additionalUnits) const
 {
 #if BATTLE_TRACE_LEVEL>=1
 	logAi->trace("Battle exchange at %d", ap.attack.shooting ? ap.dest.toInt() : ap.from.toInt());
@@ -1002,7 +1002,7 @@ const battle::Units & BattleExchangeEvaluator::getOneTurnReachableUnits(uint8_t
 }
 
 // avoid blocking path for stronger stack by weaker stack
-bool BattleExchangeEvaluator::checkPositionBlocksOurStacks(HypotheticBattle & hb, const battle::Unit * activeUnit, const BattleHex & position)
+bool BattleExchangeEvaluator::checkPositionBlocksOurStacks(const HypotheticBattle & hb, const battle::Unit * activeUnit, const BattleHex & position)
 {
 	const int BLOCKING_THRESHOLD = 70;
 	const int BLOCKING_OWN_ATTACK_PENALTY = 100;

+ 3 - 4
AI/BattleAI/BattleExchangeVariant.h

@@ -133,7 +133,6 @@ class ReachabilityMapCache
 	std::map<uint32_t, ReachabilityInfo> unitReachabilityMap; // unit ID -> reachability
 	std::map<uint32_t, PerTurnData> hexReachabilityPerTurn;
 
-	//const ReachabilityInfo & update();
 	battle::Units computeOneTurnReachableUnits(std::shared_ptr<CBattleInfoCallback> cb, std::shared_ptr<Environment> env, const std::vector<battle::Units> & turnOrder, uint8_t turn, const BattleHex & hex);
 public:
 	const battle::Units & getOneTurnReachableUnits(std::shared_ptr<CBattleInfoCallback> cb, std::shared_ptr<Environment> env, const std::vector<battle::Units> & turnOrder, uint8_t turn, const BattleHex & hex);
@@ -158,7 +157,7 @@ private:
 		PotentialTargets & targets,
 		DamageCache & damageCache,
 		std::shared_ptr<HypotheticBattle> hb,
-		battle::Units additionalUnits = {}) const;
+		const battle::Units & additionalUnits = {}) const;
 
 	bool canBeHitThisTurn(const AttackPossibility & ap);
 
@@ -193,9 +192,9 @@ public:
 		uint8_t turn,
 		PotentialTargets & targets,
 		std::shared_ptr<HypotheticBattle> hb,
-		battle::Units additionalUnits = {}) const;
+		const battle::Units & additionalUnits = {}) const;
 
-	bool checkPositionBlocksOurStacks(HypotheticBattle & hb, const battle::Unit * unit, const BattleHex & position);
+	bool checkPositionBlocksOurStacks(const HypotheticBattle & hb, const battle::Unit * unit, const BattleHex & position);
 
 	MoveTarget findMoveTowardsUnreachable(
 		const battle::Unit * activeStack,

+ 3 - 3
AI/Nullkiller/Analyzers/BuildAnalyzer.cpp

@@ -209,7 +209,7 @@ BuildingInfo BuildAnalyzer::getBuildingOrPrerequisite(
 	int creatureLevel = -1;
 	int creatureUpgrade = 0;
 
-	if(toBuild.IsDwelling())
+	if(toBuild.isDwelling())
 	{
 		creatureLevel = BuildingID::getLevelFromDwelling(toBuild);
 		creatureUpgrade = BuildingID::getUpgradedFromDwelling(toBuild);
@@ -267,7 +267,7 @@ BuildingInfo BuildAnalyzer::getBuildingOrPrerequisite(
 
 			auto otherDwelling = [](const BuildingID & id) -> bool
 			{
-				return id.IsDwelling();
+				return id.isDwelling();
 			};
 
 			if(vstd::contains_if(missingBuildings, otherDwelling))
@@ -429,7 +429,7 @@ BuildingInfo::BuildingInfo(
 		}
 		else
 		{
-			if(id.IsDwelling())
+			if(id.isDwelling())
 			{
 				creatureGrows = creature->getGrowth();
 

+ 1 - 1
AI/Nullkiller/Behaviors/DefenceBehavior.cpp

@@ -386,7 +386,7 @@ void DefenceBehavior::evaluateDefence(Goals::TGoalVec & tasks, const CGTownInsta
 					town->getObjectName());
 #endif
 
-			ExecuteHeroChain heroChain = ExecuteHeroChain(path, town);
+			ExecuteHeroChain heroChain(path, town);
 				
 			if (closestWay)
 			{

+ 29 - 32
AI/Nullkiller/Engine/PriorityEvaluator.cpp

@@ -895,7 +895,7 @@ public:
 		if(stayAtTown.town->mageGuildLevel() > 0)
 			evaluationContext.armyReward += evaluationContext.evaluator.getManaRecoveryArmyReward(stayAtTown.getHero());
 
-		if (evaluationContext.armyReward == 0)
+		if (vstd::isAlmostZero(evaluationContext.armyReward))
 			evaluationContext.isDefend = true;
 		else
 		{
@@ -1023,43 +1023,40 @@ public:
 		if(heroRole == HeroRole::MAIN)
 			evaluationContext.heroRole = heroRole;
 
-		if (hero)
-		{
-			// Assuming Slots() returns a collection of slots with slot.second->getCreatureID() and slot.second->getPower()
-			float heroPower = 0;
-			float totalPower = 0;
-
-			// Map to store the aggregated power of creatures by CreatureID
-			std::map<int, float> totalPowerByCreatureID;
+		// Assuming Slots() returns a collection of slots with slot.second->getCreatureID() and slot.second->getPower()
+		float heroPower = 0;
+		float totalPower = 0;
 
-			// Calculate hero power and total power by CreatureID
-			for (auto slot : hero->Slots())
-			{
-				int creatureID = slot.second->getCreatureID();
-				float slotPower = slot.second->getPower();
+		// Map to store the aggregated power of creatures by CreatureID
+		std::map<int, float> totalPowerByCreatureID;
 
-				// Add the power of this slot to the heroPower
-				heroPower += slotPower;
+		// Calculate hero power and total power by CreatureID
+		for (auto slot : hero->Slots())
+		{
+			int creatureID = slot.second->getCreatureID();
+			float slotPower = slot.second->getPower();
 
-				// Accumulate the total power for the specific CreatureID
-				if (totalPowerByCreatureID.find(creatureID) == totalPowerByCreatureID.end())
-				{
-					// First time encountering this CreatureID, retrieve total creatures' power
-					totalPowerByCreatureID[creatureID] = ai->armyManager->getTotalCreaturesAvailable(creatureID).power;
-				}
-			}
+			// Add the power of this slot to the heroPower
+			heroPower += slotPower;
 
-			// Calculate total power based on unique CreatureIDs
-			for (const auto& entry : totalPowerByCreatureID)
+			// Accumulate the total power for the specific CreatureID
+			if (totalPowerByCreatureID.find(creatureID) == totalPowerByCreatureID.end())
 			{
-				totalPower += entry.second;
+				// First time encountering this CreatureID, retrieve total creatures' power
+				totalPowerByCreatureID[creatureID] = ai->armyManager->getTotalCreaturesAvailable(creatureID).power;
 			}
+		}
 
-			// Compute the power ratio if total power is greater than zero
-			if (totalPower > 0)
-			{
-				evaluationContext.powerRatio = heroPower / totalPower;
-			}
+		// Calculate total power based on unique CreatureIDs
+		for (const auto& entry : totalPowerByCreatureID)
+		{
+			totalPower += entry.second;
+		}
+
+		// Compute the power ratio if total power is greater than zero
+		if (totalPower > 0)
+		{
+			evaluationContext.powerRatio = heroPower / totalPower;
 		}
 
 		if (target)
@@ -1249,7 +1246,7 @@ public:
 				auto potentialUpgradeValue = evaluationContext.evaluator.getUpgradeArmyReward(buildThis.town, bi);
 				
 				evaluationContext.addNonCriticalStrategicalValue(potentialUpgradeValue / 10000.0f / (float)bi.prerequisitesCount);
-				if(bi.id.IsDwelling())
+				if(bi.id.isDwelling())
 					evaluationContext.armyReward += bi.armyStrength - evaluationContext.evaluator.ai->armyManager->evaluateStackPower(bi.baseCreatureID.toCreature(), bi.creatureGrows);
 				else //This is for prerequisite-buildings
 					evaluationContext.armyReward += evaluationContext.evaluator.ai->armyManager->evaluateStackPower(bi.baseCreatureID.toCreature(), bi.creatureGrows);

+ 4 - 4
AI/Nullkiller/Pathfinding/AINodeStorage.h

@@ -41,8 +41,8 @@ struct AIPathNode : public CGPathNode
 {
 	std::shared_ptr<const SpecialAction> specialAction;
 
-	const AIPathNode * chainOther;
-	const ChainActor * actor;
+	const AIPathNode * chainOther = nullptr;
+	const ChainActor * actor = nullptr;
 
 	uint64_t danger;
 	uint64_t armyLoss;
@@ -129,8 +129,8 @@ struct AIPath
 
 struct ExchangeCandidate : public AIPathNode
 {
-	AIPathNode * carrierParent;
-	AIPathNode * otherParent;
+	AIPathNode * carrierParent = nullptr;
+	AIPathNode * otherParent = nullptr;
 };
 
 enum EHeroChainPass

+ 2 - 2
AI/Nullkiller/Pathfinding/AIPathfinderConfig.cpp

@@ -50,8 +50,8 @@ namespace AIPathfinding
 		options.allowLayerTransitioningAfterBattle = true;
 		options.useTeleportWhirlpool = true;
 		options.forceUseTeleportWhirlpool = true;
-		options.useTeleportOneWay = ai->settings->isOneWayMonolithUsageAllowed();;
-		options.useTeleportOneWayRandom = ai->settings->isOneWayMonolithUsageAllowed();;
+		options.useTeleportOneWay = ai->settings->isOneWayMonolithUsageAllowed();
+		options.useTeleportOneWayRandom = ai->settings->isOneWayMonolithUsageAllowed();
 	}
 
 	AIPathfinderConfig::~AIPathfinderConfig() = default;

+ 1 - 1
AI/VCAI/BuildingManager.cpp

@@ -222,7 +222,7 @@ bool BuildingManager::getBuildingOptions(const CGTownInstance * t)
 	std::vector<BuildingID> extraBuildings;
 	for (auto buildingInfo : t->getTown()->buildings)
 	{
-		if (buildingInfo.first.IsDwelling() && BuildingID::getUpgradedFromDwelling(buildingInfo.first) > 1)
+		if (buildingInfo.first.isDwelling() && BuildingID::getUpgradedFromDwelling(buildingInfo.first) > 1)
 			extraBuildings.push_back(buildingInfo.first);
 	}
 	return tryBuildAnyStructure(t, extraBuildings);

+ 1 - 1
client/adventureMap/CMinimap.cpp

@@ -96,7 +96,7 @@ CMinimap::CMinimap(const Rect & position)
 	double maxSideLengthSrc = std::max(LOCPLINT->cb->getMapSize().x, LOCPLINT->cb->getMapSize().y);
 	double maxSideLengthDst = std::max(position.w, position.h);
 	double resize = maxSideLengthSrc / maxSideLengthDst;
-	Point newMinimapSize = Point(LOCPLINT->cb->getMapSize().x/ resize, LOCPLINT->cb->getMapSize().y / resize);
+	Point newMinimapSize(LOCPLINT->cb->getMapSize().x/ resize, LOCPLINT->cb->getMapSize().y / resize);
 	Point offset = Point((std::max(newMinimapSize.x, newMinimapSize.y) - newMinimapSize.x) / 2, (std::max(newMinimapSize.x, newMinimapSize.y) - newMinimapSize.y) / 2);
 
 	pos.x += offset.x;

+ 8 - 8
client/battle/BattleFieldController.cpp

@@ -412,7 +412,7 @@ BattleHexArray BattleFieldController::getHighlightedHexesForMovementTarget()
 
 // Range limit highlight helpers
 
-BattleHexArray BattleFieldController::getRangeHexes(const BattleHex & sourceHex, uint8_t distance)
+BattleHexArray BattleFieldController::getRangeHexes(const BattleHex & sourceHex, uint8_t distance) const
 {
 	BattleHexArray rangeHexes;
 
@@ -430,21 +430,21 @@ BattleHexArray BattleFieldController::getRangeHexes(const BattleHex & sourceHex,
 	return rangeHexes;
 }
 
-BattleHexArray BattleFieldController::getRangeLimitHexes(const BattleHex & hoveredHex, const BattleHexArray & rangeHexes, uint8_t distanceToLimit)
+BattleHexArray BattleFieldController::getRangeLimitHexes(const BattleHex & sourceHex, const BattleHexArray & rangeHexes, uint8_t distanceToLimit) const
 {
 	BattleHexArray rangeLimitHexes;
 
 	// from range hexes get only the ones at the limit
 	for(auto & hex : rangeHexes)
 	{
-		if(BattleHex::getDistance(hoveredHex, hex) == distanceToLimit)
+		if(BattleHex::getDistance(sourceHex, hex) == distanceToLimit)
 			rangeLimitHexes.insert(hex);
 	}
 
 	return rangeLimitHexes;
 }
 
-bool BattleFieldController::IsHexInRangeLimit(const BattleHex & hex, const BattleHexArray & rangeLimitHexes, int * hexIndexInRangeLimit)
+bool BattleFieldController::isHexInRangeLimit(const BattleHex & hex, const BattleHexArray & rangeLimitHexes, int * hexIndexInRangeLimit) const
 {
 	bool  hexInRangeLimit = false;
 
@@ -459,7 +459,7 @@ bool BattleFieldController::IsHexInRangeLimit(const BattleHex & hex, const Battl
 }
 
 std::vector<std::vector<BattleHex::EDir>> BattleFieldController::getOutsideNeighbourDirectionsForLimitHexes(
-	const BattleHexArray & wholeRangeHexes, const BattleHexArray & rangeLimitHexes)
+	const BattleHexArray & wholeRangeHexes, const BattleHexArray & rangeLimitHexes) const
 {
 	std::vector<std::vector<BattleHex::EDir>> output;
 
@@ -565,11 +565,11 @@ void BattleFieldController::showHighlightedHexes(Canvas & canvas)
 
 		// calculate if hex is Ranged Full Damage Limit and its position in highlight array
 		int hexIndexInRangedFullDamageLimit = 0;
-		bool hexInRangedFullDamageLimit = IsHexInRangeLimit(hex, rangedFullDamageLimitHexes, &hexIndexInRangedFullDamageLimit);
+		bool hexInRangedFullDamageLimit = isHexInRangeLimit(hex, rangedFullDamageLimitHexes, &hexIndexInRangedFullDamageLimit);
 
 		// calculate if hex is Shooting Range Limit and its position in highlight array
 		int hexIndexInShootingRangeLimit = 0;
-		bool hexInShootingRangeLimit = IsHexInRangeLimit(hex, shootingRangeLimitHexes, &hexIndexInShootingRangeLimit);
+		bool hexInShootingRangeLimit = isHexInRangeLimit(hex, shootingRangeLimitHexes, &hexIndexInShootingRangeLimit);
 
 		if(stackMovement && mouse) // area where hovered stackMovement can move shown with highlight. Because also affected by mouse cursor, shade as well
 		{
@@ -663,7 +663,7 @@ BattleHex BattleFieldController::getHexAtPosition(Point hoverPos)
 	return BattleHex::INVALID;
 }
 
-BattleHex::EDir BattleFieldController::selectAttackDirection(const BattleHex & myNumber)
+BattleHex::EDir BattleFieldController::selectAttackDirection(const BattleHex & myNumber) const
 {
 	const bool doubleWide = owner.stacksController->getActiveStack()->doubleWide();
 	const BattleHexArray & neighbours = myNumber.getAllNeighbouringTiles();

+ 5 - 5
client/battle/BattleFieldController.h

@@ -65,16 +65,16 @@ class BattleFieldController : public CIntObject
 	// Range limit highlight helpers
 
 	/// get all hexes within a certain distance of given hex
-	BattleHexArray getRangeHexes(const BattleHex & sourceHex, uint8_t distance);
+	BattleHexArray getRangeHexes(const BattleHex & sourceHex, uint8_t distance) const;
 
 	/// get only hexes at the limit of a range
-	BattleHexArray getRangeLimitHexes(const BattleHex & hoveredHex, const BattleHexArray & hexRange, uint8_t distanceToLimit);
+	BattleHexArray getRangeLimitHexes(const BattleHex & hoveredHex, const BattleHexArray & hexRange, uint8_t distanceToLimit) const;
 
 	/// calculate if a hex is in range limit and return its index in range
-	bool IsHexInRangeLimit(const BattleHex & hex, const BattleHexArray & rangeLimitHexes, int * hexIndexInRangeLimit);
+	bool isHexInRangeLimit(const BattleHex & hex, const BattleHexArray & rangeLimitHexes, int * hexIndexInRangeLimit) const;
 
 	/// get an array that has for each hex in range, an array with all directions where an outside neighbour hex exists
-	std::vector<std::vector<BattleHex::EDir>> getOutsideNeighbourDirectionsForLimitHexes(const BattleHexArray & rangeHexes, const BattleHexArray & rangeLimitHexes);
+	std::vector<std::vector<BattleHex::EDir>> getOutsideNeighbourDirectionsForLimitHexes(const BattleHexArray & rangeHexes, const BattleHexArray & rangeLimitHexes) const;
 
 	/// calculates what image to use as range limit, depending on the direction of neighbours
 	/// a mask is used internally to mark the directions of all neighbours
@@ -135,7 +135,7 @@ public:
 	/// returns true if stack should render its stack count image in default position - outside own hex
 	bool stackCountOutsideHex(const BattleHex & number) const;
 
-	BattleHex::EDir selectAttackDirection(const BattleHex & myNumber);
+	BattleHex::EDir selectAttackDirection(const BattleHex & myNumber) const;
 
 	BattleHex fromWhichHexAttack(const BattleHex & myNumber);
 };

+ 10 - 12
client/render/AssetGenerator.cpp

@@ -29,10 +29,6 @@
 #include "../lib/RoadHandler.h"
 #include "../lib/TerrainHandler.h"
 
-AssetGenerator::AssetGenerator()
-{
-}
-
 void AssetGenerator::initialize()
 {
 	// clear to avoid non updated sprites after mod change (if base imnages are used)
@@ -81,7 +77,7 @@ std::map<AnimationPath, AssetGenerator::AnimationLayoutMap> AssetGenerator::gene
 	return animationFiles;
 }
 
-AssetGenerator::CanvasPtr AssetGenerator::createAdventureOptionsCleanBackground()
+AssetGenerator::CanvasPtr AssetGenerator::createAdventureOptionsCleanBackground() const
 {
 	auto locator = ImageLocator(ImagePath::builtin("ADVOPTBK"), EImageBlitMode::OPAQUE);
 
@@ -101,7 +97,7 @@ AssetGenerator::CanvasPtr AssetGenerator::createAdventureOptionsCleanBackground(
 	return image;
 }
 
-AssetGenerator::CanvasPtr AssetGenerator::createBigSpellBook()
+AssetGenerator::CanvasPtr AssetGenerator::createBigSpellBook() const
 {
 	auto locator = ImageLocator(ImagePath::builtin("SpelBack"), EImageBlitMode::OPAQUE);
 
@@ -153,7 +149,7 @@ AssetGenerator::CanvasPtr AssetGenerator::createBigSpellBook()
 	return image;
 }
 
-AssetGenerator::CanvasPtr AssetGenerator::createPlayerColoredBackground(const PlayerColor & player)
+AssetGenerator::CanvasPtr AssetGenerator::createPlayerColoredBackground(const PlayerColor & player) const
 {
 	auto locator = ImageLocator(ImagePath::builtin("DiBoxBck"), EImageBlitMode::OPAQUE);
 
@@ -185,7 +181,7 @@ AssetGenerator::CanvasPtr AssetGenerator::createPlayerColoredBackground(const Pl
 	return image;
 }
 
-AssetGenerator::CanvasPtr AssetGenerator::createCombatUnitNumberWindow(float multR, float multG, float multB)
+AssetGenerator::CanvasPtr AssetGenerator::createCombatUnitNumberWindow(float multR, float multG, float multB) const
 {
 	auto locator = ImageLocator(ImagePath::builtin("CMNUMWIN"), EImageBlitMode::OPAQUE);
 	locator.layer = EImageBlitMode::OPAQUE;
@@ -206,7 +202,7 @@ AssetGenerator::CanvasPtr AssetGenerator::createCombatUnitNumberWindow(float mul
 	return image;
 }
 
-AssetGenerator::CanvasPtr AssetGenerator::createCampaignBackground()
+AssetGenerator::CanvasPtr AssetGenerator::createCampaignBackground() const
 {
 	auto locator = ImageLocator(ImagePath::builtin("CAMPBACK"), EImageBlitMode::OPAQUE);
 
@@ -245,7 +241,7 @@ AssetGenerator::CanvasPtr AssetGenerator::createCampaignBackground()
 	return image;
 }
 
-AssetGenerator::CanvasPtr AssetGenerator::createChroniclesCampaignImages(int chronicle)
+AssetGenerator::CanvasPtr AssetGenerator::createChroniclesCampaignImages(int chronicle) const
 {
 	auto imgPathBg = ImagePath::builtin("chronicles_" + std::to_string(chronicle) + "/GamSelBk");
 	auto locator = ImageLocator(imgPathBg, EImageBlitMode::OPAQUE);
@@ -326,7 +322,9 @@ void AssetGenerator::generatePaletteShiftedAnimation(const AnimationPath & sprit
 			ImagePath spriteName = ImagePath::builtin(sprite.getName() + boost::str(boost::format("%02d") % tileIndex) + "_" + std::to_string(paletteIndex) + ".png");
 			layout[paletteIndex].push_back(ImageLocator(spriteName, EImageBlitMode::SIMPLE));
 
-			imageFiles[spriteName]  = [=](){ return createPaletteShiftedImage(sprite, paletteAnimations, tileIndex, paletteIndex);};
+			imageFiles[spriteName] = [this, sprite, paletteAnimations, tileIndex, paletteIndex](){
+				return createPaletteShiftedImage(sprite, paletteAnimations, tileIndex, paletteIndex);
+			};
 		}
 	}
 
@@ -334,7 +332,7 @@ void AssetGenerator::generatePaletteShiftedAnimation(const AnimationPath & sprit
 	animationFiles[shiftedPath] = layout;
 }
 
-AssetGenerator::CanvasPtr AssetGenerator::createPaletteShiftedImage(const AnimationPath & source, const std::vector<PaletteAnimation> & palette, int frameIndex, int paletteShiftCounter)
+AssetGenerator::CanvasPtr AssetGenerator::createPaletteShiftedImage(const AnimationPath & source, const std::vector<PaletteAnimation> & palette, int frameIndex, int paletteShiftCounter) const
 {
 	auto animation = GH.renderHandler().loadAnimation(source, EImageBlitMode::COLORKEY);
 

+ 7 - 9
client/render/AssetGenerator.h

@@ -24,8 +24,6 @@ public:
 	using AnimationLayoutMap = std::map<size_t, std::vector<ImageLocator>>;
 	using CanvasPtr = std::shared_ptr<CanvasImage>;
 
-	AssetGenerator();
-
 	void initialize();
 
 	std::shared_ptr<ISharedImage> generateImage(const ImagePath & image);
@@ -47,13 +45,13 @@ private:
 	std::map<ImagePath, ImageGenerationFunctor> imageFiles;
 	std::map<AnimationPath, AnimationLayoutMap> animationFiles;
 
-	CanvasPtr createAdventureOptionsCleanBackground();
-	CanvasPtr createBigSpellBook();
-	CanvasPtr createPlayerColoredBackground(const PlayerColor & player);
-	CanvasPtr createCombatUnitNumberWindow(float multR, float multG, float multB);
-	CanvasPtr createCampaignBackground();
-	CanvasPtr createChroniclesCampaignImages(int chronicle);
-	CanvasPtr createPaletteShiftedImage(const AnimationPath & source, const std::vector<PaletteAnimation> & animation, int frameIndex, int paletteShiftCounter);
+	CanvasPtr createAdventureOptionsCleanBackground() const;
+	CanvasPtr createBigSpellBook() const;
+	CanvasPtr createPlayerColoredBackground(const PlayerColor & player) const;
+	CanvasPtr createCombatUnitNumberWindow(float multR, float multG, float multB) const;
+	CanvasPtr createCampaignBackground() const;
+	CanvasPtr createChroniclesCampaignImages(int chronicle) const;
+	CanvasPtr createPaletteShiftedImage(const AnimationPath & source, const std::vector<PaletteAnimation> & animation, int frameIndex, int paletteShiftCounter) const;
 
 	void createPaletteShiftedSprites();
 	void generatePaletteShiftedAnimation(const AnimationPath & source, const std::vector<PaletteAnimation> & animation);

+ 8 - 8
client/renderSDL/CTrueTypeFont.cpp

@@ -125,27 +125,27 @@ size_t CTrueTypeFont::getStringWidthScaled(const std::string & text) const
 	return width;
 }
 
-void CTrueTypeFont::renderText(SDL_Surface * surface, const std::string & data, const ColorRGBA & color, const Point & pos) const
+void CTrueTypeFont::renderText(SDL_Surface * surface, const std::string & text, const ColorRGBA & color, const Point & pos) const
 {
-	if (data.empty())
+	if (text.empty())
 		return;
 
 	if (outline)
-		renderTextImpl(surface, data, Colors::BLACK, pos - Point(1,1) * getScalingFactor());
+		renderTextImpl(surface, text, Colors::BLACK, pos - Point(1,1) * getScalingFactor());
 
 	if (dropShadow || outline)
-		renderTextImpl(surface, data, Colors::BLACK, pos + Point(1,1) * getScalingFactor());
+		renderTextImpl(surface, text, Colors::BLACK, pos + Point(1,1) * getScalingFactor());
 
-	renderTextImpl(surface, data, color, pos);
+	renderTextImpl(surface, text, color, pos);
 }
 
-void CTrueTypeFont::renderTextImpl(SDL_Surface * surface, const std::string & data, const ColorRGBA & color, const Point & pos) const
+void CTrueTypeFont::renderTextImpl(SDL_Surface * surface, const std::string & text, const ColorRGBA & color, const Point & pos) const
 {
 	SDL_Surface * rendered;
 	if (blended)
-		rendered = TTF_RenderUTF8_Blended(font.get(), data.c_str(), CSDL_Ext::toSDL(color));
+		rendered = TTF_RenderUTF8_Blended(font.get(), text.c_str(), CSDL_Ext::toSDL(color));
 	else
-		rendered = TTF_RenderUTF8_Solid(font.get(), data.c_str(), CSDL_Ext::toSDL(color));
+		rendered = TTF_RenderUTF8_Solid(font.get(), text.c_str(), CSDL_Ext::toSDL(color));
 
 	assert(rendered);
 

+ 6 - 13
client/renderSDL/RenderHandler.cpp

@@ -72,7 +72,7 @@ std::shared_ptr<CDefFile> RenderHandler::getAnimationFile(const AnimationPath &
 	return result;
 }
 
-void RenderHandler::initFromJson(AnimationLayoutMap & source, const JsonNode & config, EImageBlitMode mode)
+void RenderHandler::initFromJson(AnimationLayoutMap & source, const JsonNode & config, EImageBlitMode mode) const
 {
 	std::string basepath;
 	basepath = config["basepath"].String();
@@ -155,20 +155,13 @@ RenderHandler::AnimationLayoutMap & RenderHandler::getAnimationLayout(const Anim
 
 	for(auto & loader : configList)
 	{
-		try {
-			auto stream = loader->load(jsonResource);
-			std::unique_ptr<ui8[]> textData(new ui8[stream->getSize()]);
-			stream->read(textData.get(), stream->getSize());
+		auto stream = loader->load(jsonResource);
+		std::unique_ptr<ui8[]> textData(new ui8[stream->getSize()]);
+		stream->read(textData.get(), stream->getSize());
 
-			const JsonNode config(reinterpret_cast<const std::byte*>(textData.get()), stream->getSize(), path.getOriginalName());
+		const JsonNode config(reinterpret_cast<const std::byte*>(textData.get()), stream->getSize(), path.getOriginalName());
 
-			initFromJson(result, config, mode);
-		}
-		catch (const DataLoadingException & e)
-		{
-			// FIXME: sometimes triggered by generated animation assets, e.g. lava/water tiles
-			logGlobal->error("Failed to load animation file! Reason: %s", e.what());
-		}
+		initFromJson(result, config, mode);
 	}
 
 	animationLayouts[actualPath] = result;

+ 1 - 1
client/renderSDL/RenderHandler.h

@@ -32,7 +32,7 @@ class RenderHandler final : public IRenderHandler
 
 	std::shared_ptr<CDefFile> getAnimationFile(const AnimationPath & path);
 	AnimationLayoutMap & getAnimationLayout(const AnimationPath & path, int scalingFactor, EImageBlitMode mode);
-	void initFromJson(AnimationLayoutMap & layout, const JsonNode & config, EImageBlitMode mode);
+	void initFromJson(AnimationLayoutMap & layout, const JsonNode & config, EImageBlitMode mode) const;
 
 	void addImageListEntry(size_t index, size_t group, const std::string & listName, const std::string & imageName);
 	void addImageListEntries(const EntityService * service);

+ 5 - 5
client/renderSDL/ScalableImage.cpp

@@ -411,9 +411,9 @@ void ScalableImageInstance::playerColored(const PlayerColor & player)
 	image->preparePlayerColoredImage(player);
 }
 
-void ScalableImageParameters::playerColored(PlayerColor player)
+void ScalableImageParameters::playerColored(PlayerColor playerColor)
 {
-	graphics->setPlayerPalette(palette, player);
+	graphics->setPlayerPalette(palette, playerColor);
 }
 
 void ScalableImageInstance::shiftPalette(uint32_t firstColorID, uint32_t colorsToMove, uint32_t distanceToMove)
@@ -451,9 +451,9 @@ std::shared_ptr<const ISharedImage> ScalableImageShared::loadOrGenerateImage(EIm
 	loadingLocator.playerColored = color;
 
 	// best case - requested image is already available in filesystem
-	auto loadedImage = GH.renderHandler().loadScaledImage(loadingLocator);
-	if (loadedImage)
-		return loadedImage;
+	auto loadedFullMatchImage = GH.renderHandler().loadScaledImage(loadingLocator);
+	if (loadedFullMatchImage)
+		return loadedFullMatchImage;
 
 	// optional images for 1x resolution - only try load them, don't attempt to generate
 	bool optionalImage =

+ 10 - 10
client/windows/CCastleInterface.cpp

@@ -161,7 +161,7 @@ void CBuildingRect::showPopupWindow(const Point & cursorPosition)
 
 	BuildingID bid = getBuilding()->bid;
 	const CBuilding *bld = town->getTown()->buildings.at(bid);
-	if (!bid.IsDwelling())
+	if (!bid.isDwelling())
 	{
 		CRClickPopup::createAndPush(CInfoWindow::genText(bld->getNameTranslated(), bld->getDescriptionTranslated()),
 									std::make_shared<CComponent>(ComponentType::BUILDING, BuildingTypeUniqueID(bld->town->faction->getId(), bld->bid)));
@@ -240,7 +240,7 @@ std::string CBuildingRect::getSubtitle()//hover text for building
 
 	auto bid = getBuilding()->bid;
 
-	if (!bid.IsDwelling())//non-dwellings - only building name
+	if (!bid.isDwelling())//non-dwellings - only building name
 		return town->getTown()->buildings.at(getBuilding()->bid)->getNameTranslated();
 	else//dwellings - recruit %creature%
 	{
@@ -339,13 +339,13 @@ CHeroGSlot::CHeroGSlot(int x, int y, int updown, const CGHeroInstance * h, HeroS
 
 CHeroGSlot::~CHeroGSlot() = default;
 
-auto CHeroGSlot::getUpgradableSlots(const CArmedInstance *obj)
+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(auto & slot : slots)
+	for(const auto & slot : slots)
 	{
 		auto upgradeInfo = std::make_pair(slot.first, UpgradeInfo(slot.second->getCreatureID()));
 		LOCPLINT->cb->fillUpgradeInfo(slot.second->armyObj, slot.first, upgradeInfo.second);
@@ -357,11 +357,11 @@ auto CHeroGSlot::getUpgradableSlots(const CArmedInstance *obj)
 	std::sort(upgradeInfos.begin(), upgradeInfos.end(), [&](const std::pair<SlotID, UpgradeInfo> & lhs, const std::pair<SlotID, UpgradeInfo> & rhs) {
 		return lhs.second.oldID.toCreature()->getLevel() > rhs.second.oldID.toCreature()->getLevel();
 	});
-	bool creaturesToUpgrade = static_cast<bool>(upgradeInfos.size());
+	bool hasCreaturesToUpgrade = !upgradeInfos.empty();
 
-	TResources costs = TResources();
+	TResources costs;
 	std::vector<SlotID> slotInfosToDelete;
-	for(auto & upgradeInfo : upgradeInfos)
+	for(const auto & upgradeInfo : upgradeInfos)
 	{
 		TResources upgradeCosts = upgradeInfo.second.getUpgradeCosts() * slots[upgradeInfo.first]->getCount();
 		if(LOCPLINT->cb->getResourceAmount().canAfford(costs + upgradeCosts))
@@ -373,7 +373,7 @@ auto CHeroGSlot::getUpgradableSlots(const CArmedInstance *obj)
 		return std::count(slotInfosToDelete.begin(), slotInfosToDelete.end(), item.first);
 	}), upgradeInfos.end());
 
-    return result { creaturesToUpgrade, static_cast<bool>(upgradeInfos.size()), !slotInfosToDelete.size(), costs, upgradeInfos };
+	return result { hasCreaturesToUpgrade, !upgradeInfos.empty(), slotInfosToDelete.empty(), costs, upgradeInfos };
 }
 
 void CHeroGSlot::gesture(bool on, const Point & initialPosition, const Point & finalPosition)
@@ -744,7 +744,7 @@ void CCastleBuildings::drawOverlays(Canvas & to, std::vector<std::shared_ptr<CBu
 		const auto & font = GH.renderHandler().loadFont(FONT_TINY);
 
 		auto backColor = Colors::GREEN; // Other
-		if(bid.IsDwelling())
+		if(bid.isDwelling())
 			backColor = Colors::PURPLE; // dwelling
 
 		auto contentRect = buildingRect->border->contentRect();
@@ -880,7 +880,7 @@ bool CCastleBuildings::buildingTryActivateCustomUI(BuildingID buildingToTest, Bu
 		return true;
 	}
 
-	if (buildingToTest.IsDwelling())
+	if (buildingToTest.isDwelling())
 	{
 		enterDwelling((BuildingID::getLevelFromDwelling(buildingToTest)));
 		return true;

+ 1 - 1
client/windows/CCastleInterface.h

@@ -103,7 +103,7 @@ class CHeroGSlot : public CIntObject
 	const CGHeroInstance * hero;
 	int upg; //0 - up garrison, 1 - down garrison
 
-	auto getUpgradableSlots(const CArmedInstance *obj);
+	auto getUpgradableSlots(const CArmedInstance *obj) const;
 
 public:
 	CHeroGSlot(int x, int y, int updown, const CGHeroInstance *h, HeroSlots * Owner);

+ 2 - 2
client/windows/CMapOverview.cpp

@@ -95,7 +95,7 @@ std::shared_ptr<CanvasImage> CMapOverviewWidget::createMinimapForLayer(std::uniq
 	return canvasImage;
 }
 
-std::vector<std::shared_ptr<CanvasImage>> CMapOverviewWidget::createMinimaps(ResourcePath resource) const
+std::vector<std::shared_ptr<CanvasImage>> CMapOverviewWidget::createMinimaps(const ResourcePath & resource) const
 {
 	std::vector<std::shared_ptr<CanvasImage>> ret;
 
@@ -138,7 +138,7 @@ std::shared_ptr<CPicture> CMapOverviewWidget::buildDrawMinimap(const JsonNode &
 	double maxSideLengthSrc = std::max(minimapRect.x, minimapRect.y);
 	double maxSideLengthDst = std::max(rect.w, rect.h);
 	double resize = maxSideLengthSrc / maxSideLengthDst;
-	Point newMinimapSize = Point(minimapRect.x / resize, minimapRect.y / resize);
+	Point newMinimapSize(minimapRect.x / resize, minimapRect.y / resize);
 
 	minimaps[id]->scaleTo(newMinimapSize, EScalingAlgorithm::NEAREST); // for sharp-looking minimap
 

+ 1 - 1
client/windows/CMapOverview.h

@@ -36,7 +36,7 @@ class CMapOverviewWidget : public InterfaceObjectConfigurable
 	std::vector<std::shared_ptr<CanvasImage>> minimaps;
 
 	std::shared_ptr<CanvasImage> createMinimapForLayer(std::unique_ptr<CMap> & map, int layer) const;
-	std::vector<std::shared_ptr<CanvasImage>> createMinimaps(ResourcePath resource) const;
+	std::vector<std::shared_ptr<CanvasImage>> createMinimaps(const ResourcePath & resource) const;
 	std::vector<std::shared_ptr<CanvasImage>> createMinimaps(std::unique_ptr<CMap> & map) const;
 
 	std::shared_ptr<CPicture> buildDrawMinimap(const JsonNode & config) const;

+ 3 - 1
client/windows/GUIClasses.cpp

@@ -1685,7 +1685,9 @@ void CObjectListWindow::keyPressed(EShortcut key)
 }
 
 VideoWindow::VideoWindow(const VideoPath & video, const ImagePath & rim, bool showBackground, float scaleFactor, const std::function<void(bool skipped)> & closeCb)
-	: CWindowObject(BORDERED | SHADOW_DISABLED | NEEDS_ANIMATED_BACKGROUND), closeCb(closeCb), showBackground(showBackground)
+	: CWindowObject(BORDERED | SHADOW_DISABLED | NEEDS_ANIMATED_BACKGROUND)
+	, showBackground(showBackground)
+	, closeCb(closeCb)
 {
 	OBJECT_CONSTRUCTION;
 

+ 2 - 2
launcher/modManager/chroniclesextractor.cpp

@@ -224,7 +224,7 @@ void ChroniclesExtractor::extractFiles(int no) const
 		auto mapping = std::map<std::string, int>{{ {"Intro", 1}, {"Intr2", 2}, {"Intr3", 3}, {"Intr4", 4}, {"Intro5", 7}, {"Intro6", 8} }};
 		std::vector<std::string> videoFiles;
 		for(auto & elem : mapping)
-			for(auto & ending : {".bik", ".smk"})
+			for(const auto & ending : {".bik", ".smk"})
 				videoFiles.push_back(elem.first + ending);
 		extract(tmpDirData, tmpDir, "Hchron.vid", videoFiles);
 		for(auto & ending : {".bik", ".smk"})
@@ -280,7 +280,7 @@ void ChroniclesExtractor::installChronicles(QStringList exe)
 		logGlobal->info("Creating base Chronicle mod");
 		createBaseMod();
 
-		for(auto & no : chronicleNo)
+		for(const auto & no : chronicleNo)
 		{
 			logGlobal->info("Creating Chronicle mod (%i)", no);
 			createChronicleMod(no);

+ 3 - 3
lib/CCreatureHandler.cpp

@@ -441,15 +441,15 @@ void CCreatureHandler::loadCommanders()
 		commanderLevelPremy.push_back(JsonUtils::parseBonus(bonus.Vector()));
 	}
 
-	int i = 0;
+	int level = 0;
 	for (auto skill : config["skillLevels"].Vector())
 	{
 		skillLevels.emplace_back();
 		for (auto skillLevel : skill["levels"].Vector())
 		{
-			skillLevels[i].push_back(static_cast<ui8>(skillLevel.Float()));
+			skillLevels[level].push_back(static_cast<ui8>(skillLevel.Float()));
 		}
-		++i;
+		++level;
 	}
 
 	for (auto ability : config["abilityRequirements"].Vector())

+ 12 - 28
lib/CCreatureSet.cpp

@@ -683,19 +683,21 @@ void CCreatureSet::serializeJson(JsonSerializeFormat & handler, const std::strin
 }
 
 CStackInstance::CStackInstance(bool isHypothetic)
-	: CBonusSystemNode(isHypothetic),
-	armyObj(_armyObj),
-	nativeTerrain(this, Selector::type()(BonusType::TERRAIN_NATIVE)),
-	initiative(this, Selector::type()(BonusType::STACKS_SPEED))
-
+	: CBonusSystemNode(isHypothetic)
+	, nativeTerrain(this, Selector::type()(BonusType::TERRAIN_NATIVE))
+	, initiative(this, Selector::type()(BonusType::STACKS_SPEED))
+	, armyObj(_armyObj)
 {
-	init();
+	experience = 0;
+	count = 0;
+	setType(nullptr);
+	_armyObj = nullptr;
+	setNodeType(STACK_INSTANCE);
 }
 
 CStackInstance::CStackInstance(const CreatureID & id, TQuantity Count, bool isHypothetic)
 	: CStackInstance(false)
 {
-	init();
 	setType(id);
 	count = Count;
 }
@@ -703,20 +705,10 @@ CStackInstance::CStackInstance(const CreatureID & id, TQuantity Count, bool isHy
 CStackInstance::CStackInstance(const CCreature *cre, TQuantity Count, bool isHypothetic)
 	: CStackInstance(false)
 {
-	init();
 	setType(cre);
 	count = Count;
 }
 
-void CStackInstance::init()
-{
-	experience = 0;
-	count = 0;
-	setType(nullptr);
-	_armyObj = nullptr;
-	setNodeType(STACK_INSTANCE);
-}
-
 CCreature::CreatureQuantityId CStackInstance::getQuantityID() const
 {
 	return CCreature::getQuantityID(count);
@@ -966,19 +958,9 @@ const IBonusBearer* CStackInstance::getBonusBearer() const
 	return this;
 }
 
-CCommanderInstance::CCommanderInstance()
-{
-	init();
-}
+CCommanderInstance::CCommanderInstance() = default;
 
 CCommanderInstance::CCommanderInstance(const CreatureID & id): name("Commando")
-{
-	init();
-	setType(id);
-	//TODO - parse them
-}
-
-void CCommanderInstance::init()
 {
 	alive = true;
 	experience = 0;
@@ -988,6 +970,8 @@ void CCommanderInstance::init()
 	_armyObj = nullptr;
 	setNodeType (CBonusSystemNode::COMMANDER);
 	secondarySkills.resize (ECommander::SPELL_POWER + 1);
+	setType(id);
+	//TODO - parse them
 }
 
 void CCommanderInstance::setAlive (bool Alive)

+ 1 - 3
lib/CCreatureSet.h

@@ -122,14 +122,13 @@ public:
 	virtual int getLevel() const; //different for regular stack and commander
 	CreatureID getCreatureID() const; //-1 if not available
 	std::string getName() const; //plural or singular
-	virtual void init();
 	CStackInstance(bool isHypothetic = false);
 	CStackInstance(const CreatureID & id, TQuantity count, bool isHypothetic = false);
 	CStackInstance(const CCreature *cre, TQuantity count, bool isHypothetic = false);
 	virtual ~CStackInstance() = default;
 
 	void setType(const CreatureID & creID);
-	void setType(const CCreature * c) override;
+	void setType(const CCreature * c) final;
 	void setArmyObj(const CArmedInstance *ArmyObj);
 	virtual void giveStackExp(TExpType exp);
 	bool valid(bool allowUnrandomized) const;
@@ -156,7 +155,6 @@ public:
 	std::vector <ui8> secondarySkills; //ID -> level
 	std::set <ui8> specialSkills;
 	//std::vector <CArtifactInstance *> arts;
-	void init() override;
 	CCommanderInstance();
 	CCommanderInstance(const CreatureID & id);
 	void setAlive (bool alive);

+ 1 - 1
lib/battle/BattleHexArray.h

@@ -159,7 +159,7 @@ public:
 		return std::vector<BattleHex>(internalStorage.begin(), internalStorage.end());
 	}
 
-	[[nodiscard]] std::string toString(std::string delimiter = ", ") const noexcept
+	[[nodiscard]] std::string toString(const std::string & delimiter = ", ") const noexcept
 	{
 		std::string result = "[";
 		for(auto it = internalStorage.begin(); it != internalStorage.end(); ++it)

+ 4 - 10
lib/battle/CBattleInfoCallback.cpp

@@ -31,9 +31,6 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-namespace SiegeStuffThatShouldBeMovedToHandlers // <=== TODO
-{
-
 static BattleHex lineToWallHex(int line) //returns hex with wall in given line (y coordinate)
 {
 	static const BattleHex lineToHex[] = {12, 29, 45, 62, 78, 96, 112, 130, 147, 165, 182};
@@ -90,9 +87,6 @@ static BattleHex WallPartToHex(EWallPart part)
 
 	return BattleHex::INVALID; //not found!
 }
-}
-
-using namespace SiegeStuffThatShouldBeMovedToHandlers;
 
 ESpellCastProblem CBattleInfoCallback::battleCanCastSpell(const spells::Caster * caster, spells::Mode mode) const
 {
@@ -1174,7 +1168,7 @@ std::pair<const battle::Unit *, BattleHex> CBattleInfoCallback::getNearestStack(
 
 	std::vector<DistStack> stackPairs;
 
-	battle::Units possible = battleGetUnitsIf([=](const battle::Unit * unit)
+	battle::Units possible = battleGetUnitsIf([closest](const battle::Unit * unit)
 	{
 		return unit->isValidTarget(false) && unit != closest;
 	});
@@ -1723,11 +1717,11 @@ battle::Units CBattleInfoCallback::battleAdjacentUnits(const battle::Unit * unit
 
 	const auto & hexes = unit->getSurroundingHexes();
 
-	const auto & units = battleGetUnitsIf([=](const battle::Unit * unit)
+	const auto & units = battleGetUnitsIf([&hexes](const battle::Unit * testedUnit)
 	{
-		if (unit->isDead())
+		if (testedUnit->isDead())
 			return false;
-		const auto & unitHexes = unit->getHexes();
+		const auto & unitHexes = testedUnit->getHexes();
 		for (const auto & hex : unitHexes)
 			if (hexes.contains(hex))
 				return true;

+ 2 - 5
lib/battle/CUnitState.cpp

@@ -916,11 +916,8 @@ void CUnitState::afterNewRound()
 	drainedMana = false;
 	counterAttacks.reset();
 
-	if(alive() && isClone())
-	{
-		if(!bonusCache.hasBonus(UnitBonusValuesProxy::CLONE_MARKER))
-			makeGhost();
-	}
+	if(alive() && isClone() && !bonusCache.hasBonus(UnitBonusValuesProxy::CLONE_MARKER))
+		makeGhost();
 }
 
 void CUnitState::afterGetsTurn()

+ 4 - 4
lib/constants/EntityIdentifiers.h

@@ -329,7 +329,7 @@ public:
 		{
 			return getDwellings().at(upgradeIndex).at(level);
 		}
-		catch (const std::out_of_range & e)
+		catch (const std::out_of_range &)
 		{
 			return Type::NONE;
 		}
@@ -337,7 +337,7 @@ public:
 
 	static int getLevelFromDwelling(BuildingIDBase dwelling)
 	{
-		for (const auto level : getDwellings())
+		for (const auto & level : getDwellings())
 		{
 			auto it = std::find(level.begin(), level.end(), dwelling);
 			if (it != level.end())
@@ -368,9 +368,9 @@ public:
 		dwelling.setNum(getDwellingFromLevel(level, upgrade + 1));
 	}
 
-	bool IsDwelling() const
+	bool isDwelling() const
 	{
-		for (const auto level : getDwellings())
+		for (const auto & level : getDwellings())
 		{
 			if (vstd::contains(level, num))
 				return true;

+ 1 - 1
lib/entities/faction/CTownHandler.cpp

@@ -331,7 +331,7 @@ void CTownHandler::loadBuilding(CTown * town, const std::string & stringID, cons
 
 	if(!source["mapObjectLikeBonuses"].isNull())
 	{
-		VLC->identifiers()->requestIdentifierOptional("object", source["mapObjectLikeBonuses"], [=](si32 identifier)
+		VLC->identifiers()->requestIdentifierOptional("object", source["mapObjectLikeBonuses"], [ret](si32 identifier)
 		{
 			ret->mapObjectLikeBonuses = MapObjectID(identifier);
 		});

+ 2 - 2
lib/mapObjects/CGHeroInstance.cpp

@@ -325,7 +325,7 @@ TObjectTypeHandler CGHeroInstance::getObjectHandler() const
 
 void CGHeroInstance::updateAppearance()
 {
-	auto handler = VLC->objtypeh->getHandlerFor(Obj::HERO, getHeroClass()->getIndex());;
+	auto handler = VLC->objtypeh->getHandlerFor(Obj::HERO, getHeroClass()->getIndex());
 	auto terrain = cb->gameState()->getTile(visitablePos())->getTerrainID();
 	auto app = handler->getOverride(terrain, this);
 	if (app)
@@ -341,7 +341,7 @@ void CGHeroInstance::initHero(vstd::RNG & rand)
 
 	if (ID == Obj::HERO)
 	{
-		auto handler = VLC->objtypeh->getHandlerFor(Obj::HERO, getHeroClass()->getIndex());;
+		auto handler = VLC->objtypeh->getHandlerFor(Obj::HERO, getHeroClass()->getIndex());
 		appearance = handler->getTemplates().front();
 	}
 

+ 1 - 1
lib/mapObjects/CQuest.cpp

@@ -156,7 +156,7 @@ void CQuest::completeQuest(IGameCallback * cb, const CGHeroInstance *h) const
 		const auto * assembly = h->getCombinedArtWithPart(elem);
 		if (assembly)
 		{
-			auto parts = assembly->getPartsInfo(); // FIXME: causes crashes on Google Play
+			auto parts = assembly->getPartsInfo();
 
 			// Remove the assembly
 			cb->removeArtifact(ArtifactLocation(h->id, h->getArtPos(assembly)));

+ 2 - 2
lib/mapping/CMapHeader.cpp

@@ -124,10 +124,10 @@ CMapHeader::CMapHeader()
 	, twoLevel(true)
 	, difficulty(EMapDifficulty::NORMAL)
 	, levelLimit(0)
-	, howManyTeams(0)
-	, areAnyPlayers(false)
 	, victoryIconIndex(0)
 	, defeatIconIndex(0)
+	, howManyTeams(0)
+	, areAnyPlayers(false)
 {
 	setupEvents();
 	allowedHeroes = VLC->heroh->getDefaultAllowed();

+ 2 - 3
lib/mapping/MapFormatH3M.cpp

@@ -2594,7 +2594,6 @@ CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_pt
 
 	for(int eventID = 0; eventID < eventsCount; ++eventID)
 	{
-		// TODO: a lot of copy-pasted code with map event
 		CCastleEvent event;
 		event.creatures.resize(7);
 
@@ -2613,7 +2612,7 @@ CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_pt
 			int16_t hotaSpecialB = reader->readInt16();
 
 			if (hotaSpecialA != 0 || hotaSpecialB != 0 || hotaAmount != 44)
-				logGlobal->warn("Map '%s': Town at %s: Constructing town-specific special buildings in event is not implemented!", mapName, position.toString());;
+				logGlobal->warn("Map '%s': Town at %s: Constructing town-specific special buildings in event is not implemented!", mapName, position.toString());
 
 			event.creatures.push_back(creatureGrowth8);
 		}
@@ -2646,7 +2645,7 @@ CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_pt
 				if (mapHeader->players[alignment].canAnyonePlay())
 					object->alignmentToPlayer = PlayerColor(alignment);
 				else
-					logGlobal->warn("Map '%s': Alignment of town at %s is invalid! Player %d is not present on map!", mapName, position.toString(), int(alignment));
+					logGlobal->warn("Map '%s': Alignment of town at %s is invalid! Player %d is not present on map!", mapName, position.toString(), static_cast<int>(alignment));
 			}
 			else
 			{

+ 1 - 1
lib/mapping/MapReaderH3M.cpp

@@ -138,7 +138,7 @@ HeroTypeID MapReaderH3M::readHeroPortrait()
 
 CreatureID MapReaderH3M::readCreature32()
 {
-	CreatureID result= CreatureID(reader->readUInt32());
+	CreatureID result(reader->readUInt32());
 
 	if(result.getNum() == features.creatureIdentifierInvalid)
 		return CreatureID::NONE;

+ 2 - 2
lib/network/NetworkConnection.cpp

@@ -209,8 +209,8 @@ void NetworkConnection::close()
 }
 
 InternalConnection::InternalConnection(INetworkConnectionListener & listener, const std::shared_ptr<NetworkContext> & context)
-	: listener(listener)
-	, io(context)
+	: io(context)
+	, listener(listener)
 {
 }
 

+ 2 - 2
lib/networkPacks/NetPacksLib.cpp

@@ -1985,10 +1985,10 @@ void SetObjectProperty::applyGs(CGameState *gs)
 		PlayerColor oldOwner = obj->getOwner();
 		PlayerColor newOwner = identifier.as<PlayerColor>();
 		if(oldOwner.isValidPlayer())
-			gs->getPlayerState(oldOwner)->removeOwnedObject(obj);;
+			gs->getPlayerState(oldOwner)->removeOwnedObject(obj);
 
 		if(newOwner.isValidPlayer())
-			gs->getPlayerState(newOwner)->addOwnedObject(obj);;
+			gs->getPlayerState(newOwner)->addOwnedObject(obj);
 	}
 
 	if(what == ObjProperty::OWNER && cai)

+ 1 - 1
lib/pathfinder/PathfinderCache.cpp

@@ -28,7 +28,7 @@ std::shared_ptr<PathfinderConfig> PathfinderCache::createConfig(const CGHeroInst
 
 std::shared_ptr<CPathsInfo> PathfinderCache::buildPaths(const CGHeroInstance * h)
 {
-	std::shared_ptr<CPathsInfo> result = std::make_shared<CPathsInfo>(cb->getMapSize(), h);
+	auto result = std::make_shared<CPathsInfo>(cb->getMapSize(), h);
 	auto config = createConfig(h, *result);
 
 	cb->calculatePaths(config);

+ 1 - 1
lib/rmg/CZonePlacer.cpp

@@ -783,7 +783,7 @@ void CZonePlacer::moveOneZone(TZoneMap& zones, TForceVector& totalForces, TDista
 		//Move one zone towards most distant zone to reduce distance
 
 		float maxDistance = 0;
-		for (auto con : misplacedZone->getConnections())
+		for (const auto & con : misplacedZone->getConnections())
 		{
 			if (con.getConnectionType() == rmg::EConnectionType::REPULSIVE)
 			{

+ 1 - 1
lib/rmg/modificators/ObjectManager.cpp

@@ -47,7 +47,7 @@ void ObjectManager::init()
 	// Consider only connected zones
 	auto id = zone.getId();
 	std::set<TRmgTemplateZoneId> connectedZones;
-	for(auto c : map.getMapGenOptions().getMapTemplate()->getConnectedZoneIds())
+	for(const auto & c : map.getMapGenOptions().getMapTemplate()->getConnectedZoneIds())
 	{
 		// Only consider connected zones
 		if (c.getZoneA() == id || c.getZoneB() == id)

+ 3 - 3
mapeditor/inspector/armywidget.cpp

@@ -113,9 +113,9 @@ ArmyWidget::~ArmyWidget()
 	delete ui;
 }
 
-
-
-ArmyDelegate::ArmyDelegate(CArmedInstance & t): army(t), BaseInspectorItemDelegate()
+ArmyDelegate::ArmyDelegate(CArmedInstance & t)
+	: BaseInspectorItemDelegate()
+	, army(t)
 {
 }
 

+ 5 - 3
mapeditor/inspector/heroskillswidget.cpp

@@ -116,7 +116,9 @@ void HeroSkillsWidget::commitChanges()
 	}
 }
 
-HeroSkillsDelegate::HeroSkillsDelegate(CGHeroInstance & h): hero(h), BaseInspectorItemDelegate()
+HeroSkillsDelegate::HeroSkillsDelegate(CGHeroInstance & h)
+	: BaseInspectorItemDelegate()
+	, hero(h)
 {
 }
 
@@ -157,7 +159,7 @@ void HeroSkillsDelegate::updateModelData(QAbstractItemModel * model, const QMode
 	textList += QString("%1: %2").arg(QString::fromStdString(VLC->generaltexth->primarySkillNames[PrimarySkill::DEFENSE])).arg(hero.getBasePrimarySkillValue(PrimarySkill::DEFENSE));
 	textList += QString("%1: %2").arg(QString::fromStdString(VLC->generaltexth->primarySkillNames[PrimarySkill::SPELL_POWER])).arg(hero.getBasePrimarySkillValue(PrimarySkill::SPELL_POWER));
 	textList += QString("%1: %2").arg(QString::fromStdString(VLC->generaltexth->primarySkillNames[PrimarySkill::KNOWLEDGE])).arg(hero.getBasePrimarySkillValue(PrimarySkill::KNOWLEDGE));
-;
+
 	auto heroSecondarySkills = hero.secSkills;
 	if(heroSecondarySkills.size() == 1 && heroSecondarySkills[0].first == SecondarySkill::NONE) 
 	{
@@ -168,7 +170,7 @@ void HeroSkillsDelegate::updateModelData(QAbstractItemModel * model, const QMode
 	{
 		textList += QObject::tr("Secondary skills:");
 	}
-	for(auto & [skill, skillLevel] : heroSecondarySkills)
+	for(const auto & [skill, skillLevel] : heroSecondarySkills)
 		textList += QString("%1 %2").arg(QString::fromStdString(VLC->skillh->getById(skill)->getNameTranslated())).arg(LevelIdentifiers[skillLevel - 1].first);
 
 	setModelTextData(model, index, textList);

+ 3 - 1
mapeditor/inspector/herospellwidget.cpp

@@ -101,7 +101,9 @@ void HeroSpellWidget::on_customizeSpells_toggled(bool checked)
 	initSpellLists();
 }
 
-HeroSpellDelegate::HeroSpellDelegate(CGHeroInstance & h) : hero(h), BaseInspectorItemDelegate()
+HeroSpellDelegate::HeroSpellDelegate(CGHeroInstance & h)
+	: BaseInspectorItemDelegate()
+	, hero(h)
 {
 }
 

+ 2 - 2
mapeditor/inspector/inspector.cpp

@@ -310,7 +310,7 @@ void Inspector::updateProperties(CGHeroInstance * o)
 	
 	{ //Gender
 		auto * delegate = new InspectorDelegate;
-		delegate->options = {{QObject::tr("MALE"), QVariant::fromValue(int(EHeroGender::MALE))}, {QObject::tr("FEMALE"), QVariant::fromValue(int(EHeroGender::FEMALE))}};
+		delegate->options = {{QObject::tr("MALE"), QVariant::fromValue(static_cast<int>(EHeroGender::MALE))}, {QObject::tr("FEMALE"), QVariant::fromValue(static_cast<int>(EHeroGender::FEMALE))}};
 		addProperty<std::string>(QObject::tr("Gender"), (o->gender == EHeroGender::FEMALE ? QObject::tr("FEMALE") : QObject::tr("MALE")).toStdString(), delegate , false);
 	}
 	addProperty(QObject::tr("Name"), o->getNameTranslated(), false);
@@ -893,7 +893,7 @@ QTableWidgetItem * Inspector::addProperty(CGCreature::Character value)
 	item->setFlags(Qt::NoItemFlags);
 	item->setData(Qt::UserRole, QVariant::fromValue(int(value)));
 	
-	for(auto & i : characterIdentifiers)
+	for(const auto & i : characterIdentifiers)
 	{
 		if(i.second.toInt() == value)
 		{

+ 3 - 1
mapeditor/inspector/portraitwidget.cpp

@@ -102,7 +102,9 @@ void PortraitWidget::on_buttonPrev_clicked()
 	drawPortrait();
 }
 
-PortraitDelegate::PortraitDelegate(CGHeroInstance & h): hero(h), BaseInspectorItemDelegate()
+PortraitDelegate::PortraitDelegate(CGHeroInstance & h)
+	: BaseInspectorItemDelegate()
+	, hero(h)
 {
 }
 

+ 7 - 7
mapeditor/inspector/questwidget.cpp

@@ -466,49 +466,49 @@ void QuestDelegate::updateModelData(QAbstractItemModel * model, const QModelInde
 	textList += QObject::tr("Resources: %1").arg(resourcesList.join(", "));
 
 	QStringList artifactsList;
-	for(auto artifact : quest.mission.artifacts)
+	for(const auto & artifact : quest.mission.artifacts)
 	{
 		artifactsList += QString::fromStdString(VLC->artifacts()->getById(artifact)->getNameTranslated());
 	}
 	textList += QObject::tr("Artifacts: %1").arg(artifactsList.join(", "));
 
 	QStringList spellsList;
-	for(auto spell : quest.mission.spells)
+	for(const auto & spell : quest.mission.spells)
 	{
 		spellsList += QString::fromStdString(VLC->spells()->getById(spell)->getNameTranslated());
 	}
 	textList += QObject::tr("Spells: %1").arg(spellsList.join(", "));
 
 	QStringList secondarySkillsList;
-	for(auto & [skill, skillLevel] : quest.mission.secondary)
+	for(const auto & [skill, skillLevel] : quest.mission.secondary)
 	{
 		secondarySkillsList += QString("%1 (%2)").arg(QString::fromStdString(VLC->skills()->getById(skill)->getNameTranslated())).arg(skillLevel);
 	}
 	textList += QObject::tr("Secondary Skills: %1").arg(secondarySkillsList.join(", "));
 
 	QStringList creaturesList;
-	for(auto & creature : quest.mission.creatures)
+	for(const auto & creature : quest.mission.creatures)
 	{
 		creaturesList += QString("%1 %2").arg(creature.count).arg(QString::fromStdString(creature.getType()->getNamePluralTranslated()));
 	}
 	textList += QObject::tr("Creatures: %1").arg(creaturesList.join(", "));
 
 	QStringList heroesList;
-	for(auto & hero : quest.mission.heroes)
+	for(const auto & hero : quest.mission.heroes)
 	{
 		heroesList += QString::fromStdString(VLC->heroTypes()->getById(hero)->getNameTranslated());
 	}
 	textList += QObject::tr("Heroes: %1").arg(heroesList.join(", "));
 
 	QStringList heroClassesList;
-	for(auto & heroClass : quest.mission.heroClasses)
+	for(const auto & heroClass : quest.mission.heroClasses)
 	{
 		heroClassesList += QString::fromStdString(VLC->heroClasses()->getById(heroClass)->getNameTranslated());
 	}
 	textList += QObject::tr("Hero Classes: %1").arg(heroClassesList.join(", "));
 
 	QStringList playersList;
-	for(auto & player : quest.mission.players)
+	for(const auto & player : quest.mission.players)
 	{
 		MetaString str;
 		str.appendName(player);

+ 3 - 1
mapeditor/inspector/townbuildingswidget.cpp

@@ -318,7 +318,9 @@ void TownBuildingsWidget::onItemChanged(const QStandardItem * item) {
 	connect(&model, &QStandardItemModel::itemChanged, this, &TownBuildingsWidget::onItemChanged);
 }
 
-TownBuildingsDelegate::TownBuildingsDelegate(CGTownInstance & t): town(t), BaseInspectorItemDelegate()
+TownBuildingsDelegate::TownBuildingsDelegate(CGTownInstance & t)
+	: BaseInspectorItemDelegate()
+	, town(t)
 {
 }
 

+ 3 - 3
server/CGameHandler.cpp

@@ -260,7 +260,7 @@ void CGameHandler::levelUpCommander (const CCommanderInstance * c, int skill)
 	}
 	else if (skill >= 100)
 	{
-		for(auto & bonus : VLC->creh->skillRequirements.at(skill - 100).first) 
+		for(const auto & bonus : VLC->creh->skillRequirements.at(skill - 100).first)
 		{
 			scp.which = SetCommanderProperty::SPECIAL_SKILL;
 			scp.accumulatedBonus = *bonus;
@@ -299,7 +299,7 @@ void CGameHandler::levelUpCommander(const CCommanderInstance * c)
 			clu.skills.push_back(i);
 	}
 	int i = 100;
-	for (auto specialSkill : VLC->creh->skillRequirements)
+	for (const auto & specialSkill : VLC->creh->skillRequirements)
 	{
 		if (c->secondarySkills.at(specialSkill.second.first) >= ECommander::MAX_SKILL_LEVEL - 1
 			&&  c->secondarySkills.at(specialSkill.second.second) >= ECommander::MAX_SKILL_LEVEL - 1
@@ -2085,7 +2085,7 @@ bool CGameHandler::buildStructure(ObjectInstanceID tid, BuildingID requestedID,
 	//Performs stuff that has to be done before new building is built
 	auto processBeforeBuiltStructure = [t, this](const BuildingID buildingID)
 	{
-		if(buildingID.IsDwelling())
+		if(buildingID.isDwelling())
 		{
 			int level = BuildingID::getLevelFromDwelling(buildingID);
 			int upgradeNumber = BuildingID::getUpgradedFromDwelling(buildingID);