From 93c2bbf9abe5a051d0b3cd8ff4fdd91fd74a052b Mon Sep 17 00:00:00 2001
From: Konstantin <ria.freelander@gmail.com>
Date: Wed, 15 Feb 2023 01:44:59 +0300
Subject: [PATCH] vcmi: modernize lib/battle

---
 lib/battle/AccessibilityInfo.cpp         | 14 +++-
 lib/battle/AccessibilityInfo.h           |  9 ++-
 lib/battle/BattleAction.cpp              |  6 +-
 lib/battle/BattleAction.h                |  2 +-
 lib/battle/BattleHex.cpp                 | 24 +++---
 lib/battle/BattleHex.h                   | 23 +++---
 lib/battle/BattleInfo.cpp                | 96 ++++++++++++------------
 lib/battle/BattleInfo.h                  | 13 ++--
 lib/battle/BattleProxy.cpp               |  6 +-
 lib/battle/BattleStateInfoForRetreat.cpp | 13 +++-
 lib/battle/CBattleInfoCallback.cpp       | 77 +++++++++----------
 lib/battle/CBattleInfoCallback.h         |  6 +-
 lib/battle/CBattleInfoEssentials.cpp     | 20 ++---
 lib/battle/CBattleInfoEssentials.h       | 14 ++--
 lib/battle/CCallbackBase.cpp             | 11 +--
 lib/battle/CCallbackBase.h               |  4 +-
 lib/battle/CObstacleInstance.cpp         | 12 ---
 lib/battle/CObstacleInstance.h           |  9 +--
 lib/battle/CUnitState.cpp                | 73 +++---------------
 lib/battle/CUnitState.h                  | 14 ++--
 lib/battle/DamageCalculator.cpp          |  6 +-
 lib/battle/Destination.cpp               | 18 -----
 lib/battle/Destination.h                 |  6 +-
 lib/battle/ReachabilityInfo.cpp          | 20 ++---
 lib/battle/ReachabilityInfo.h            | 14 ++--
 lib/battle/SideInBattle.cpp              |  9 ---
 lib/battle/SideInBattle.h                | 11 ++-
 lib/battle/SiegeInfo.cpp                 |  4 +-
 lib/battle/Unit.cpp                      | 14 +---
 lib/battle/Unit.h                        | 10 +--
 30 files changed, 234 insertions(+), 324 deletions(-)

diff --git a/lib/battle/AccessibilityInfo.cpp b/lib/battle/AccessibilityInfo.cpp
index 2681dfeed..d30b870eb 100644
--- a/lib/battle/AccessibilityInfo.cpp
+++ b/lib/battle/AccessibilityInfo.cpp
@@ -9,11 +9,21 @@
  */
 #include "StdInc.h"
 #include "AccessibilityInfo.h"
+#include "BattleHex.h"
 #include "Unit.h"
 #include "../GameConstants.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
+bool AccessibilityInfo::tileAccessibleWithGate(BattleHex tile, ui8 side) const
+{
+	//at(otherHex) != EAccessibility::ACCESSIBLE && (at(otherHex) != EAccessibility::GATE || side != BattleSide::DEFENDER)
+	if(at(tile) != EAccessibility::ACCESSIBLE)
+		if(at(tile) != EAccessibility::GATE || side != BattleSide::DEFENDER)
+			return false;
+	return true;
+}
+
 bool AccessibilityInfo::accessible(BattleHex tile, const battle::Unit * stack) const
 {
 	return accessible(tile, stack->doubleWide(), stack->unitSide());
@@ -25,7 +35,7 @@ bool AccessibilityInfo::accessible(BattleHex tile, bool doubleWide, ui8 side) co
 	//do not use getHexes for speed reasons
 	if(!tile.isValid())
 		return false;
-	if(at(tile) != EAccessibility::ACCESSIBLE && !(at(tile) == EAccessibility::GATE && side == BattleSide::DEFENDER))
+	if(!tileAccessibleWithGate(tile, side))
 		return false;
 
 	if(doubleWide)
@@ -33,7 +43,7 @@ bool AccessibilityInfo::accessible(BattleHex tile, bool doubleWide, ui8 side) co
 		auto otherHex = battle::Unit::occupiedHex(tile, doubleWide, side);
 		if(!otherHex.isValid())
 			return false;
-		if(at(otherHex) != EAccessibility::ACCESSIBLE && !(at(otherHex) == EAccessibility::GATE && side == BattleSide::DEFENDER))
+		if(!tileAccessibleWithGate(tile, side))
 			return false;
 	}
 
diff --git a/lib/battle/AccessibilityInfo.h b/lib/battle/AccessibilityInfo.h
index 597b1473e..c84c1346d 100644
--- a/lib/battle/AccessibilityInfo.h
+++ b/lib/battle/AccessibilityInfo.h
@@ -31,12 +31,15 @@ enum class EAccessibility
 };
 
 
-typedef std::array<EAccessibility, GameConstants::BFIELD_SIZE> TAccessibilityArray;
+using TAccessibilityArray = std::array<EAccessibility, GameConstants::BFIELD_SIZE>;
 
 struct DLL_LINKAGE AccessibilityInfo : TAccessibilityArray
 {
-	bool accessible(BattleHex tile, const battle::Unit * stack) const; //checks for both tiles if stack is double wide
-	bool accessible(BattleHex tile, bool doubleWide, ui8 side) const; //checks for both tiles if stack is double wide
+	public:
+		bool accessible(BattleHex tile, const battle::Unit * stack) const; //checks for both tiles if stack is double wide
+		bool accessible(BattleHex tile, bool doubleWide, ui8 side) const; //checks for both tiles if stack is double wide
+	private:
+		bool tileAccessibleWithGate(BattleHex tile, ui8 side) const;
 };
 
 VCMI_LIB_NAMESPACE_END
diff --git a/lib/battle/BattleAction.cpp b/lib/battle/BattleAction.cpp
index a03bf8a95..c73fe1b3d 100644
--- a/lib/battle/BattleAction.cpp
+++ b/lib/battle/BattleAction.cpp
@@ -76,7 +76,7 @@ BattleAction BattleAction::makeShotAttack(const battle::Unit * shooter, const ba
 	return ba;
 }
 
-BattleAction BattleAction::makeCreatureSpellcast(const battle::Unit * stack, const battle::Target & target, SpellID spellID)
+BattleAction BattleAction::makeCreatureSpellcast(const battle::Unit * stack, const battle::Target & target, const SpellID & spellID)
 {
 	BattleAction ba;
 	ba.actionType = EActionType::MONSTER_SPELL;
@@ -170,7 +170,7 @@ battle::Target BattleAction::getTarget(const CBattleInfoCallback * cb) const
 {
 	battle::Target ret;
 
-	for(auto & destination : target)
+	for(const auto & destination : target)
 	{
 		if(destination.unitValue == INVALID_UNIT_ID)
 			ret.emplace_back(destination.hexValue);
@@ -184,7 +184,7 @@ battle::Target BattleAction::getTarget(const CBattleInfoCallback * cb) const
 void BattleAction::setTarget(const battle::Target & target_)
 {
     target.clear();
-	for(auto & destination : target_)
+	for(const auto & destination : target_)
 	{
 		if(destination.unitValue == nullptr)
 			aimToHex(destination.hexValue);
diff --git a/lib/battle/BattleAction.h b/lib/battle/BattleAction.h
index 5df753eb8..db65ac37e 100644
--- a/lib/battle/BattleAction.h
+++ b/lib/battle/BattleAction.h
@@ -37,7 +37,7 @@ public:
 	static BattleAction makeWait(const battle::Unit * stack);
 	static BattleAction makeMeleeAttack(const battle::Unit * stack, BattleHex destination, BattleHex attackFrom, bool returnAfterAttack = true);
 	static BattleAction makeShotAttack(const battle::Unit * shooter, const battle::Unit * target);
-	static BattleAction makeCreatureSpellcast(const battle::Unit * stack, const battle::Target & target, SpellID spellID);
+	static BattleAction makeCreatureSpellcast(const battle::Unit * stack, const battle::Target & target, const SpellID & spellID);
 	static BattleAction makeMove(const battle::Unit * stack, BattleHex dest);
 	static BattleAction makeEndOFTacticPhase(ui8 side);
 	static BattleAction makeRetreat(ui8 side);
diff --git a/lib/battle/BattleHex.cpp b/lib/battle/BattleHex.cpp
index 94d8657e2..4c934f79c 100644
--- a/lib/battle/BattleHex.cpp
+++ b/lib/battle/BattleHex.cpp
@@ -55,7 +55,7 @@ void BattleHex::setXY(si16 x, si16 y, bool hasToBeValid)
 {
 	if(hasToBeValid)
 	{
-		if(!(x >= 0 && x < GameConstants::BFIELD_WIDTH && y >= 0 && y < GameConstants::BFIELD_HEIGHT))
+		if(x < 0 || x >= GameConstants::BFIELD_WIDTH || y < 0 || y >= GameConstants::BFIELD_HEIGHT)
 			throw std::runtime_error("Valid hex required");
 	}
 
@@ -84,7 +84,8 @@ std::pair<si16, si16> BattleHex::getXY() const
 
 BattleHex& BattleHex::moveInDirection(EDir dir, bool hasToBeValid)
 {
-	si16 x(getX()), y(getY());
+	si16 x = getX();
+	si16 y = getY();
 	switch(dir)
 	{
 	case TOP_LEFT:
@@ -135,7 +136,7 @@ std::vector<BattleHex> BattleHex::neighbouringTiles() const
 {
 	std::vector<BattleHex> ret;
 	ret.reserve(6);
-	for(EDir dir = EDir(0); dir <= EDir(5); dir = EDir(dir+1))
+	for(auto dir : hexagonalDirections())
 		checkAndPush(cloneInDirection(dir, false), ret);
 	return ret;
 }
@@ -145,7 +146,7 @@ std::vector<BattleHex> BattleHex::allNeighbouringTiles() const
 	std::vector<BattleHex> ret;
 	ret.resize(6);
 
-	for(EDir dir = EDir(0); dir <= EDir(5); dir = EDir(dir+1))
+	for(auto dir : hexagonalDirections())
 		ret[dir] = cloneInDirection(dir, false);
 
 	return ret;
@@ -153,20 +154,23 @@ std::vector<BattleHex> BattleHex::allNeighbouringTiles() const
 
 BattleHex::EDir BattleHex::mutualPosition(BattleHex hex1, BattleHex hex2)
 {
-	for(EDir dir = EDir(0); dir <= EDir(5); dir = EDir(dir+1))
-		if(hex2 == hex1.cloneInDirection(dir,false))
+	for(auto dir : hexagonalDirections())
+		if(hex2 == hex1.cloneInDirection(dir, false))
 			return dir;
 	return NONE;
 }
 
 uint8_t BattleHex::getDistance(BattleHex hex1, BattleHex hex2)
 {
-	int y1 = hex1.getY(), y2 = hex2.getY();
+	int y1 = hex1.getY();
+	int y2 = hex2.getY();
 
-	// FIXME: Omit floating point arithmetics
-	int x1 = (int)(hex1.getX() + y1 * 0.5), x2 = (int)(hex2.getX() + y2 * 0.5);
+	// 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 xDst = x2 - x1, yDst = y2 - y1;
+	int xDst = x2 - x1;
+	int yDst = y2 - y1;
 
 	if ((xDst >= 0 && yDst >= 0) || (xDst < 0 && yDst < 0))
 		return std::max(std::abs(xDst), std::abs(yDst));
diff --git a/lib/battle/BattleHex.h b/lib/battle/BattleHex.h
index eeaef2b5e..b7e26d944 100644
--- a/lib/battle/BattleHex.h
+++ b/lib/battle/BattleHex.h
@@ -29,26 +29,26 @@ namespace GameConstants
 	const int BFIELD_SIZE = BFIELD_WIDTH * BFIELD_HEIGHT;
 }
 
-typedef boost::optional<ui8> BattleSideOpt;
+using BattleSideOpt =  boost::optional<ui8>;
 
 // for battle stacks' positions
 struct DLL_LINKAGE BattleHex //TODO: decide if this should be changed to class for better code design
 {
 	// helpers for siege
-	static const si16 CASTLE_CENTRAL_TOWER = -2;
-	static const si16 CASTLE_BOTTOM_TOWER = -3;
-	static const si16 CASTLE_UPPER_TOWER = -4;
+	static constexpr si16 CASTLE_CENTRAL_TOWER = -2;
+	static constexpr si16 CASTLE_BOTTOM_TOWER = -3;
+	static constexpr si16 CASTLE_UPPER_TOWER = -4;
 
 	// hexes for interaction with heroes
-	static const si16 HERO_ATTACKER = 0;
-	static const si16 HERO_DEFENDER = GameConstants::BFIELD_WIDTH - 1;
+	static constexpr si16 HERO_ATTACKER = 0;
+	static constexpr si16 HERO_DEFENDER = GameConstants::BFIELD_WIDTH - 1;
 
 	// helpers for rendering
-	static const si16 HEX_BEFORE_ALL = std::numeric_limits<si16>::min();
-	static const si16 HEX_AFTER_ALL = std::numeric_limits<si16>::max();
+	static constexpr si16 HEX_BEFORE_ALL = std::numeric_limits<si16>::min();
+	static constexpr si16 HEX_AFTER_ALL = std::numeric_limits<si16>::max();
 
 	si16 hex;
-	static const si16 INVALID = -1;
+	static constexpr si16 INVALID = -1;
 	enum EDir
 	{
 		NONE = -1,
@@ -106,6 +106,11 @@ struct DLL_LINKAGE BattleHex //TODO: decide if this should be changed to class f
     using NeighbouringTilesCache = std::vector<NeighbouringTiles>;
 
     static const NeighbouringTilesCache neighbouringTilesCache;
+private:
+	//Constexpr defined array with all directions used in battle
+	static constexpr auto hexagonalDirections() {
+		return std::array<EDir,6>{BattleHex::TOP_LEFT, BattleHex::TOP_RIGHT, BattleHex::RIGHT, BattleHex::BOTTOM_RIGHT, BattleHex::BOTTOM_LEFT, BattleHex::LEFT};
+	}
 };
 
 DLL_EXPORT std::ostream & operator<<(std::ostream & os, const BattleHex & hex);
diff --git a/lib/battle/BattleInfo.cpp b/lib/battle/BattleInfo.cpp
index 541df08fb..4cedc49b7 100644
--- a/lib/battle/BattleInfo.cpp
+++ b/lib/battle/BattleInfo.cpp
@@ -47,7 +47,7 @@ std::pair< std::vector<BattleHex>, int > BattleInfo::getPath(BattleHex start, Ba
 
 void BattleInfo::calculateCasualties(std::map<ui32,si32> * casualties) const
 {
-	for(auto & elem : stacks)//setting casualties
+	for(const auto & elem : stacks) //setting casualties
 	{
 		const CStack * const st = elem;
 		si32 killed = st->getKilled();
@@ -56,22 +56,22 @@ void BattleInfo::calculateCasualties(std::map<ui32,si32> * casualties) const
 	}
 }
 
-CStack * BattleInfo::generateNewStack(uint32_t id, const CStackInstance & base, ui8 side, SlotID slot, BattleHex position)
+CStack * BattleInfo::generateNewStack(uint32_t id, const CStackInstance & base, ui8 side, const SlotID & slot, BattleHex position)
 {
 	PlayerColor owner = sides[side].color;
 	assert((owner >= PlayerColor::PLAYER_LIMIT) ||
 		(base.armyObj && base.armyObj->tempOwner == owner));
 
-	auto ret = new CStack(&base, owner, id, side, slot);
+	auto * ret = new CStack(&base, owner, id, side, slot);
 	ret->initialPosition = getAvaliableHex(base.getCreatureID(), side, position); //TODO: what if no free tile on battlefield was found?
 	stacks.push_back(ret);
 	return ret;
 }
 
-CStack * BattleInfo::generateNewStack(uint32_t id, const CStackBasicDescriptor & base, ui8 side, SlotID slot, BattleHex position)
+CStack * BattleInfo::generateNewStack(uint32_t id, const CStackBasicDescriptor & base, ui8 side, const SlotID & slot, BattleHex position)
 {
 	PlayerColor owner = sides[side].color;
-	auto ret = new CStack(&base, owner, id, side, slot);
+	auto * ret = new CStack(&base, owner, id, side, slot);
 	ret->initialPosition = position;
 	stacks.push_back(ret);
 	return ret;
@@ -81,7 +81,7 @@ void BattleInfo::localInit()
 {
 	for(int i = 0; i < 2; i++)
 	{
-		auto armyObj = battleGetArmyObject(i);
+		auto * armyObj = battleGetArmyObject(i);
 		armyObj->battle = this;
 		armyObj->attachTo(*this);
 	}
@@ -101,7 +101,7 @@ namespace CGH
 			std::vector<int> pom;
 			for(const JsonNode &value : level.Vector())
 			{
-				pom.push_back((int)value.Float());
+				pom.push_back(static_cast<int>(value.Float()));
 			}
 
 			dest.push_back(pom);
@@ -118,9 +118,9 @@ struct RandGen
 	{
 		seed = s;
 	}
-	void srand(int3 pos)
+	void srand(const int3 & pos)
 	{
-		srand(110291 * ui32(pos.x) + 167801 * ui32(pos.y) + 81569);
+		srand(110291 * static_cast<ui32>(pos.x) + 167801 * static_cast<ui32>(pos.y) + 81569);
 	}
 	int rand()
 	{
@@ -147,11 +147,11 @@ struct RangeGenerator
 		min(_min),
 		remainingCount(_max - _min + 1),
 		remaining(remainingCount, true),
-		myRand(_myRand)
+		myRand(std::move(_myRand))
 	{
 	}
 
-	int generateNumber()
+	int generateNumber() const
 	{
 		if(!remainingCount)
 			throw ExhaustedPossibilities();
@@ -161,7 +161,7 @@ struct RangeGenerator
 	}
 
 	//get number fulfilling predicate. Never gives the same number twice.
-	int getSuchNumber(std::function<bool(int)> goodNumberPred = nullptr)
+	int getSuchNumber(const std::function<bool(int)> & goodNumberPred = nullptr)
 	{
 		int ret = -1;
 		do
@@ -193,7 +193,7 @@ struct RangeGenerator
 BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const BattleField & battlefieldType, const CArmedInstance * armies[2], const CGHeroInstance * heroes[2], bool creatureBank, const CGTownInstance * town)
 {
 	CMP_stack cmpst;
-	auto curB = new BattleInfo();
+	auto * curB = new BattleInfo();
 
 	for(auto i = 0u; i < curB->sides.size(); i++)
 		curB->sides[i].init(heroes[i], armies[i]);
@@ -224,7 +224,7 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 
 		curB->si.wallState[EWallPart::GATE] = EWallState::INTACT;
 
-		for (auto const wall : {EWallPart::BOTTOM_WALL, EWallPart::BELOW_GATE, EWallPart::OVER_GATE, EWallPart::UPPER_WALL} )
+		for(const auto wall : {EWallPart::BOTTOM_WALL, EWallPart::BELOW_GATE, EWallPart::OVER_GATE, EWallPart::UPPER_WALL})
 		{
 			if (town->hasBuilt(BuildingID::CASTLE))
 				curB->si.wallState[wall] = EWallState::REINFORCED;
@@ -245,7 +245,7 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 	//randomize obstacles
  	if (town == nullptr && !creatureBank) //do it only when it's not siege and not creature bank
  	{
-		RandGen r;
+		RandGen r{};
 		auto ourRand = [&](){ return r.rand(); };
 		r.srand(tile);
 		r.rand(1,8); //battle sound ID to play... can't do anything with it here
@@ -255,15 +255,15 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 
 		auto appropriateAbsoluteObstacle = [&](int id)
 		{
-			auto * info = Obstacle(id).getInfo();
+			const auto * info = Obstacle(id).getInfo();
 			return info && info->isAbsoluteObstacle && info->isAppropriate(curB->terrainType, battlefieldType);
 		};
 		auto appropriateUsualObstacle = [&](int id)
 		{
-			auto * info = Obstacle(id).getInfo();
+			const auto * info = Obstacle(id).getInfo();
 			return info && !info->isAbsoluteObstacle && info->isAppropriate(curB->terrainType, battlefieldType);
 		};
-		
+
 		if(r.rand(1,100) <= 40) //put cliff-like obstacle
 		{
 			try
@@ -341,8 +341,11 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 
 	//reading battleStartpos - add creatures AFTER random obstacles are generated
 	//TODO: parse once to some structure
-	std::vector< std::vector<int> > looseFormations[2], tightFormations[2], creBankFormations[2];
-	std::vector <int> commanderField, commanderBank;
+	std::vector<std::vector<int>> looseFormations[2];
+	std::vector<std::vector<int>> tightFormations[2];
+	std::vector<std::vector<int>> creBankFormations[2];
+	std::vector<int> commanderField;
+	std::vector<int> commanderBank;
 	const JsonNode config(ResourceID("config/battleStartpos.json"));
 	const JsonVector &positions = config["battle_positions"].Vector();
 
@@ -355,11 +358,11 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 
 	for (auto position : config["commanderPositions"]["field"].Vector())
 	{
-		commanderField.push_back ((int)position.Float());
+		commanderField.push_back(static_cast<int>(position.Float()));
 	}
 	for (auto position : config["commanderPositions"]["creBank"].Vector())
 	{
-		commanderBank.push_back ((int)position.Float());
+		commanderBank.push_back(static_cast<int>(position.Float()));
 	}
 
 
@@ -367,7 +370,7 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 	if(!creatureBank)
 	{
 		//Checks if hero has artifact and create appropriate stack
-		auto handleWarMachine= [&](int side, ArtifactPosition artslot, BattleHex hex)
+		auto handleWarMachine = [&](int side, const ArtifactPosition & artslot, BattleHex hex)
 		{
 			const CArtifactInstance * warMachineArt = heroes[side]->getArt(artslot);
 
@@ -462,7 +465,7 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 	auto good = std::make_shared<CreatureAlignmentLimiter>(EAlignment::GOOD);
 	auto evil = std::make_shared<CreatureAlignmentLimiter>(EAlignment::EVIL);
 
-	auto bgInfo = VLC->battlefields()->getById(battlefieldType);
+	const auto * bgInfo = VLC->battlefields()->getById(battlefieldType);
 
 	for(const std::shared_ptr<Bonus> & bonus : bgInfo->bonuses)
 	{
@@ -500,18 +503,17 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 	return curB;
 }
 
-
-const CGHeroInstance * BattleInfo::getHero(PlayerColor player) const
+const CGHeroInstance * BattleInfo::getHero(const PlayerColor & player) const
 {
-	for(int i = 0; i < sides.size(); i++)
-		if(sides[i].color == player)
-			return sides[i].hero;
+	for(const auto & side : sides)
+		if(side.color == player)
+			return side.hero;
 
 	logGlobal->error("Player %s is not in battle!", player.getStr());
 	return nullptr;
 }
 
-ui8 BattleInfo::whatSide(PlayerColor player) const
+ui8 BattleInfo::whatSide(const PlayerColor & player) const
 {
 	for(int i = 0; i < sides.size(); i++)
 		if(sides[i].color == player)
@@ -532,7 +534,6 @@ BattleInfo::BattleInfo():
 	town(nullptr),
 	tile(-1,-1,-1),
 	battlefieldType(BattleField::NONE),
-	terrainType(),
 	tacticsSide(0),
 	tacticDistance(0)
 {
@@ -576,8 +577,8 @@ IBattleInfo::ObstacleCList BattleInfo::getAllObstacles() const
 {
 	ObstacleCList ret;
 
-	for(auto iter = obstacles.cbegin(); iter != obstacles.cend(); iter++)
-		ret.push_back(*iter);
+	for(const auto & obstacle : obstacles)
+		ret.push_back(obstacle);
 
 	return ret;
 }
@@ -698,7 +699,7 @@ 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 = new CStack(&base, owner, info.id, info.side, SlotID::SUMMONED_SLOT_PLACEHOLDER);
 	ret->initialPosition = info.position;
 	stacks.push_back(ret);
 	ret->localInit(this);
@@ -707,7 +708,7 @@ void BattleInfo::addUnit(uint32_t id, const JsonNode & data)
 
 void BattleInfo::moveUnit(uint32_t id, BattleHex destination)
 {
-	auto sta = getStack(id);
+	auto * sta = getStack(id);
 	if(!sta)
 	{
 		logGlobal->error("Cannot find stack %d", id);
@@ -791,7 +792,7 @@ void BattleInfo::removeUnit(uint32_t id)
 	while(!ids.empty())
 	{
 		auto toRemoveId = *ids.begin();
-		auto toRemove = getStack(toRemoveId, false);
+		auto * toRemove = getStack(toRemoveId, false);
 
 		if(!toRemove)
 		{
@@ -813,7 +814,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;
@@ -904,7 +905,7 @@ void BattleInfo::addOrUpdateUnitBonus(CStack * sta, const Bonus & value, bool fo
 	{
 		logBonus->trace("%s updated bonus: %s", sta->nodeName(), value.Description());
 
-		for(auto stackBonus : sta->getExportedBonusList()) //TODO: optimize
+		for(const auto & stackBonus : sta->getExportedBonusList()) //TODO: optimize
 		{
 			if(stackBonus->source == value.source && stackBonus->sid == value.sid && stackBonus->type == value.type && stackBonus->subtype == value.subtype)
 			{
@@ -932,11 +933,11 @@ void BattleInfo::updateObstacle(const ObstacleChanges& changes)
 	std::shared_ptr<SpellCreatedObstacle> changedObstacle = std::make_shared<SpellCreatedObstacle>();
 	changedObstacle->fromInfo(changes);
 
-	for(int i = 0; i < obstacles.size(); ++i)
+	for(auto & obstacle : obstacles)
 	{
-		if(obstacles[i]->uniqueID == changes.id) // update this obstacle
+		if(obstacle->uniqueID == changes.id) // update this obstacle
 		{
-			SpellCreatedObstacle * spellObstacle = dynamic_cast<SpellCreatedObstacle *>(obstacles[i].get());
+			auto * spellObstacle = dynamic_cast<SpellCreatedObstacle *>(obstacle.get());
 			assert(spellObstacle);
 
 			// Currently we only support to update the "revealed" property
@@ -978,7 +979,7 @@ scripting::Pool * BattleInfo::getContextPool() const
 }
 #endif
 
-bool CMP_stack::operator()(const battle::Unit * a, const battle::Unit * b)
+bool CMP_stack::operator()(const battle::Unit * a, const battle::Unit * b) const
 {
 	switch(phase)
 	{
@@ -988,7 +989,8 @@ bool CMP_stack::operator()(const battle::Unit * a, const battle::Unit * b)
 	case 2:
 	case 3:
 		{
-			int as = a->getInitiative(turn), bs = b->getInitiative(turn);
+			int as = a->getInitiative(turn);
+			int bs = b->getInitiative(turn);
 
 			if(as != bs)
 				return as > bs;
@@ -1009,11 +1011,11 @@ bool CMP_stack::operator()(const battle::Unit * a, const battle::Unit * b)
 	return false;
 }
 
-CMP_stack::CMP_stack(int Phase, int Turn, uint8_t Side)
+CMP_stack::CMP_stack(int Phase, int Turn, uint8_t Side):
+	phase(Phase), 
+	turn(Turn), 
+	side(Side) 
 {
-	phase = Phase;
-	turn = Turn;
-	side = Side;
 }
 
 VCMI_LIB_NAMESPACE_END
diff --git a/lib/battle/BattleInfo.h b/lib/battle/BattleInfo.h
index 1bafc74dd..ab53fed25 100644
--- a/lib/battle/BattleInfo.h
+++ b/lib/battle/BattleInfo.h
@@ -121,7 +121,7 @@ public:
 	void updateObstacle(const ObstacleChanges& changes) override;
 	void removeObstacle(uint32_t id) override;
 
-	void addOrUpdateUnitBonus(CStack * sta, const Bonus & value, bool forceAdd);
+	static void addOrUpdateUnitBonus(CStack * sta, const Bonus & value, bool forceAdd);
 
 	//////////////////////////////////////////////////////////////////////////
 	CStack * getStack(int stackID, bool onlyAlive = true);
@@ -134,15 +134,15 @@ public:
 
 	void calculateCasualties(std::map<ui32,si32> * casualties) const; //casualties are array of maps size 2 (attacker, defeneder), maps are (crid => amount)
 
-	CStack * generateNewStack(uint32_t id, const CStackInstance &base, ui8 side, SlotID slot, BattleHex position);
-	CStack * generateNewStack(uint32_t id, const CStackBasicDescriptor &base, ui8 side, SlotID slot, BattleHex position);
+	CStack * generateNewStack(uint32_t id, const CStackInstance & base, ui8 side, const SlotID & slot, BattleHex position);
+	CStack * generateNewStack(uint32_t id, const CStackBasicDescriptor & base, ui8 side, const SlotID & slot, BattleHex position);
 
-	const CGHeroInstance * getHero(PlayerColor player) const; //returns fighting hero that belongs to given player
+	const CGHeroInstance * getHero(const PlayerColor & player) const; //returns fighting hero that belongs to given player
 
 	void localInit();
 	static BattleInfo * setupBattle(const int3 & tile, TerrainId, const BattleField & battlefieldType, const CArmedInstance * armies[2], const CGHeroInstance * heroes[2], bool creatureBank, const CGTownInstance * town);
 
-	ui8 whatSide(PlayerColor player) const;
+	ui8 whatSide(const PlayerColor & player) const;
 
 protected:
 #if SCRIPTING_ENABLED
@@ -157,8 +157,7 @@ class DLL_LINKAGE CMP_stack
 	int turn;
 	uint8_t side;
 public:
-
-	bool operator ()(const battle::Unit * a, const battle::Unit * b);
+	bool operator()(const battle::Unit * a, const battle::Unit * b) const;
 	CMP_stack(int Phase = 1, int Turn = 0, uint8_t Side = BattleSide::ATTACKER);
 };
 
diff --git a/lib/battle/BattleProxy.cpp b/lib/battle/BattleProxy.cpp
index c5912e50f..0209f0d10 100644
--- a/lib/battle/BattleProxy.cpp
+++ b/lib/battle/BattleProxy.cpp
@@ -15,8 +15,8 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 ///BattleProxy
 
-BattleProxy::BattleProxy(Subject subject_)
-	: subject(subject_)
+BattleProxy::BattleProxy(Subject subject_): 
+	subject(std::move(subject_))
 {
 	setBattle(this);
 	player = subject->getPlayerID();
@@ -26,7 +26,7 @@ BattleProxy::~BattleProxy() = default;
 
 int32_t BattleProxy::getActiveStackID() const
 {
-	auto ret = subject->battleActiveUnit();
+	const auto * ret = subject->battleActiveUnit();
 	if(ret)
 		return ret->unitId();
 	else
diff --git a/lib/battle/BattleStateInfoForRetreat.cpp b/lib/battle/BattleStateInfoForRetreat.cpp
index 8366f76fc..676c0cc12 100644
--- a/lib/battle/BattleStateInfoForRetreat.cpp
+++ b/lib/battle/BattleStateInfoForRetreat.cpp
@@ -17,12 +17,17 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-BattleStateInfoForRetreat::BattleStateInfoForRetreat()
-: canFlee(false), canSurrender(false), isLastTurnBeforeDie(false), ourStacks(), enemyStacks(), ourHero(nullptr), enemyHero(nullptr), ourSide(-1)
+BattleStateInfoForRetreat::BattleStateInfoForRetreat():
+	canFlee(false),
+	canSurrender(false),
+	isLastTurnBeforeDie(false),
+	ourHero(nullptr),
+	enemyHero(nullptr),
+	ourSide(-1)
 {
 }
 
-uint64_t getFightingStrength(std::vector<const battle::Unit *> stacks, const CGHeroInstance * hero = nullptr)
+uint64_t getFightingStrength(const std::vector<const battle::Unit *> & stacks, const CGHeroInstance * hero = nullptr)
 {
 	uint64_t result = 0;
 
@@ -33,7 +38,7 @@ uint64_t getFightingStrength(std::vector<const battle::Unit *> stacks, const CGH
 
 	if(hero)
 	{
-		result = (uint64_t)(result * hero->getFightingStrength());
+		result = static_cast<uint64_t>(result * hero->getFightingStrength());
 	}
 
 	return result;
diff --git a/lib/battle/CBattleInfoCallback.cpp b/lib/battle/CBattleInfoCallback.cpp
index 3b171cb55..e3765dbd8 100644
--- a/lib/battle/CBattleInfoCallback.cpp
+++ b/lib/battle/CBattleInfoCallback.cpp
@@ -66,7 +66,7 @@ static const std::pair<int, EWallPart> wallParts[] =
 
 static EWallPart hexToWallPart(BattleHex hex)
 {
-	for(auto & elem : wallParts)
+	for(const auto & elem : wallParts)
 	{
 		if(elem.first == hex)
 			return elem.second;
@@ -77,7 +77,7 @@ static EWallPart hexToWallPart(BattleHex hex)
 
 static BattleHex WallPartToHex(EWallPart part)
 {
-	for(auto & elem : wallParts)
+	for(const auto & elem : wallParts)
 	{
 		if(elem.second == part)
 			return elem.first;
@@ -117,7 +117,7 @@ ESpellCastProblem::ESpellCastProblem CBattleInfoCallback::battleCanCastSpell(con
 		if(battleCastSpells(side.get()) > 0)
 			return ESpellCastProblem::CASTS_PER_TURN_LIMIT;
 
-		auto hero = dynamic_cast<const CGHeroInstance *>(caster);
+		const auto * hero = dynamic_cast<const CGHeroInstance *>(caster);
 
 		if(!hero)
 			return ESpellCastProblem::NO_HERO_TO_CAST_SPELL;
@@ -242,7 +242,7 @@ std::vector<PossiblePlayerBattleAction> CBattleInfoCallback::getClientActionsFor
 		if(stack->canMove() && stack->Speed(0, true)) //probably no reason to try move war machines or bound stacks
 			allowedActionList.push_back(PossiblePlayerBattleAction::MOVE_STACK);
 
-		auto siegedTown = battleGetDefendedTown();
+		const auto * siegedTown = battleGetDefendedTown();
 		if(siegedTown && siegedTown->hasFort() && stack->hasBonusOfType(Bonus::CATAPULT)) //TODO: check shots
 			allowedActionList.push_back(PossiblePlayerBattleAction::CATAPULT);
 		if(stack->hasBonusOfType(Bonus::HEALER))
@@ -315,7 +315,7 @@ SpellID CBattleInfoCallback::battleGetRandomStackSpell(CRandomGenerator & rand,
 const CStack* CBattleInfoCallback::battleGetStackByPos(BattleHex pos, bool onlyAlive) const
 {
 	RETURN_IF_NOT_BATTLE(nullptr);
-	for(auto s : battleGetAllStacks(true))
+	for(const auto * s : battleGetAllStacks(true))
 		if(vstd::contains(s->getHexes(), pos) && (!onlyAlive || s->alive()))
 			return s;
 
@@ -484,7 +484,7 @@ void CBattleInfoCallback::battleGetTurnOrder(std::vector<battle::Units> & out, c
 		return;
 	}
 
-	for(auto one : all)
+	for(const auto * one : all)
 	{
 		if((actualTurn == 0 && !one->willMove()) //we are considering current round and unit won't move
 		|| (actualTurn > 0 && !one->canMove(turn)) //unit won't be able to move in later rounds
@@ -574,11 +574,11 @@ std::vector<BattleHex> CBattleInfoCallback::battleGetAvailableHexes(const Reacha
 		else
 		{
 			//Not tactics phase -> destination must be reachable and within unit range.
-			if(cache.distances[i] > (int)unitSpeed)
+			if(cache.distances[i] > static_cast<int>(unitSpeed))
 				continue;
 		}
 
-		ret.push_back(i);
+		ret.emplace_back(i);
 	}
 
 	return ret;
@@ -596,6 +596,7 @@ std::vector<BattleHex> CBattleInfoCallback::battleGetAvailableHexes(const battle
 	{
 		std::vector<BattleHex> occupiable;
 
+		occupiable.reserve(ret.size());
 		for(auto hex : ret)
 			occupiable.push_back(unit->occupiedHex(hex));
 
@@ -615,7 +616,7 @@ std::vector<BattleHex> CBattleInfoCallback::battleGetAvailableHexes(const battle
 			});
 			return availableNeighbor != ret.end();
 		};
-		for(auto otherSt : battleAliveUnits(otherSide(unit->unitSide())))
+		for(const auto * otherSt : battleAliveUnits(otherSide(unit->unitSide())))
 		{
 			if(!otherSt->isValidTarget(false))
 				continue;
@@ -852,7 +853,7 @@ AccessibilityInfo CBattleInfoCallback::getAccesibility() const
 	}
 
 	//tiles occupied by standing stacks
-	for(auto unit : battleAliveUnits())
+	for(const auto * unit : battleAliveUnits())
 	{
 		for(auto hex : unit->getHexes())
 			if(hex.isAvailable()) //towers can have <0 pos; we don't also want to overwrite side columns
@@ -883,7 +884,7 @@ AccessibilityInfo CBattleInfoCallback::getAccesibility() const
 			std::make_pair(EWallPart::UPPER_WALL, BattleHex(ESiegeHex::DESTRUCTIBLE_WALL_1))
 		};
 
-		for(auto & elem : lockedIfNotDestroyed)
+		for(const auto & elem : lockedIfNotDestroyed)
 		{
 			if(battleGetWallState(elem.first) != EWallState::DESTROYED)
 				ret[elem.second] = EAccessibility::DESTRUCTIBLE_WALL;
@@ -928,7 +929,7 @@ ReachabilityInfo CBattleInfoCallback::makeBFS(const AccessibilityInfo &accessibi
 	hexq.push(params.startPosition);
 	ret.distances[params.startPosition] = 0;
 
-	std::array<bool, GameConstants::BFIELD_SIZE> accessibleCache;
+	std::array<bool, GameConstants::BFIELD_SIZE> accessibleCache{};
 	for(int hex = 0; hex < GameConstants::BFIELD_SIZE; hex++)
 		accessibleCache[hex] = accessibility.accessible(hex, params.doubleWide, params.side);
 
@@ -1032,7 +1033,7 @@ std::pair<const battle::Unit *, BattleHex> CBattleInfoCallback::getNearestStack(
 			}
 	}
 
-	if (stackPairs.size())
+	if(!stackPairs.empty())
 	{
 		auto comparator = [](DistStack lhs, DistStack rhs) { return lhs.distanceToPred < rhs.distanceToPred; };
 		auto minimal = boost::min_element(stackPairs, comparator);
@@ -1042,7 +1043,7 @@ std::pair<const battle::Unit *, BattleHex> CBattleInfoCallback::getNearestStack(
 		return std::make_pair<const battle::Unit * , BattleHex>(nullptr, BattleHex::INVALID);
 }
 
-BattleHex CBattleInfoCallback::getAvaliableHex(CreatureID creID, ui8 side, int initialPos) const
+BattleHex CBattleInfoCallback::getAvaliableHex(const CreatureID & creID, ui8 side, int initialPos) const
 {
 	bool twoHex = VLC->creh->objects[creID]->isDoubleWide();
 
@@ -1141,7 +1142,7 @@ AttackableTiles CBattleInfoCallback::getPotentiallyAttackableHexes (const  battl
 
 	BattleHex attackOriginHex = (attackerPos != BattleHex::INVALID) ? attackerPos : attacker->getPosition(); //real or hypothetical (cursor) position
 
-	auto defender = battleGetUnitByPos(destinationTile, true);
+	const auto * defender = battleGetUnitByPos(destinationTile, true);
 	if (!defender)
 		return at; // can't attack thin air
 
@@ -1161,7 +1162,7 @@ AttackableTiles CBattleInfoCallback::getPotentiallyAttackableHexes (const  battl
 		{
 			if((BattleHex::mutualPosition(tile, destinationTile) > -1 && BattleHex::mutualPosition(tile, attackOriginHex) > -1)) //adjacent both to attacker's head and attacked tile
 			{
-				auto st = battleGetUnitByPos(tile, true);
+				const auto * st = battleGetUnitByPos(tile, true);
 				if(st && battleMatchOwner(st, attacker)) //only hostile stacks - does it work well with Berserk?
 					at.hostileCreaturePositions.insert(tile);
 			}
@@ -1181,7 +1182,7 @@ AttackableTiles CBattleInfoCallback::getPotentiallyAttackableHexes (const  battl
 		for(BattleHex tile : hexes)
 		{
 			//friendly stacks can also be damaged by Dragon Breath
-			auto st = battleGetUnitByPos(tile, true);
+			const auto * st = battleGetUnitByPos(tile, true);
 			if(st && st != attacker)
 				at.friendlyCreaturePositions.insert(tile);
 		}
@@ -1208,7 +1209,7 @@ AttackableTiles CBattleInfoCallback::getPotentiallyAttackableHexes (const  battl
 			if (nextHex.isValid())
 			{
 				//friendly stacks can also be damaged by Dragon Breath
-				auto st = battleGetUnitByPos(nextHex, true);
+				const auto * st = battleGetUnitByPos(nextHex, true);
 				if(st != nullptr)
 					at.friendlyCreaturePositions.insert(nextHex);
 			}
@@ -1308,7 +1309,7 @@ static bool isHexInFront(BattleHex hex, BattleHex testHex, BattleSide::Type side
 }
 
 //TODO: this should apply also to mechanics and cursor interface
-bool CBattleInfoCallback::isToReverse (const battle::Unit * attacker, const battle::Unit * defender) const
+bool CBattleInfoCallback::isToReverse(const battle::Unit * attacker, const battle::Unit * defender) const
 {
 	BattleHex attackerHex = attacker->getPosition();
 	BattleHex defenderHex = defender->getPosition();
@@ -1316,18 +1317,18 @@ bool CBattleInfoCallback::isToReverse (const battle::Unit * attacker, const batt
 	if (attackerHex < 0 ) //turret
 		return false;
 
-	if (isHexInFront(attackerHex, defenderHex, BattleSide::Type(attacker->unitSide())))
+	if(isHexInFront(attackerHex, defenderHex, static_cast<BattleSide::Type>(attacker->unitSide())))
 		return false;
 
 	if (defender->doubleWide())
 	{
-		if (isHexInFront(attackerHex,defender->occupiedHex(), BattleSide::Type(attacker->unitSide())))
+		if(isHexInFront(attackerHex, defender->occupiedHex(), static_cast<BattleSide::Type>(attacker->unitSide())))
 			return false;
 	}
 
 	if (attacker->doubleWide())
 	{
-		if (isHexInFront(attacker->occupiedHex(), defenderHex, BattleSide::Type(attacker->unitSide())))
+		if(isHexInFront(attacker->occupiedHex(), defenderHex, static_cast<BattleSide::Type>(attacker->unitSide())))
 			return false;
 	}
 
@@ -1335,7 +1336,7 @@ bool CBattleInfoCallback::isToReverse (const battle::Unit * attacker, const batt
 	// but this is how H3 handles it which is important, e.g. for direction of dragon breath attacks
 	if (attacker->doubleWide() && defender->doubleWide())
 	{
-		if (isHexInFront(attacker->occupiedHex(), defender->occupiedHex(), BattleSide::Type(attacker->unitSide())))
+		if(isHexInFront(attacker->occupiedHex(), defender->occupiedHex(), static_cast<BattleSide::Type>(attacker->unitSide())))
 			return false;
 	}
 	return true;
@@ -1364,7 +1365,7 @@ bool CBattleInfoCallback::battleHasDistancePenalty(const IBonusBearer * shooter,
 	if(shooter->hasBonus(selectorNoDistancePenalty, cachingStrNoDistancePenalty))
 		return false;
 
-	if(auto target = battleGetUnitByPos(destHex, true))
+	if(const auto * target = battleGetUnitByPos(destHex, true))
 	{
 		//If any hex of target creature is within range, there is no penalty
 		int range = GameConstants::BATTLE_PENALTY_DISTANCE;
@@ -1418,14 +1419,14 @@ std::vector<BattleHex> CBattleInfoCallback::getAttackableBattleHexes() const
 	std::vector<BattleHex> attackableBattleHexes;
 	RETURN_IF_NOT_BATTLE(attackableBattleHexes);
 
-	for(auto & wallPartPair : wallParts)
+	for(const auto & wallPartPair : wallParts)
 	{
 		if(isWallPartPotentiallyAttackable(wallPartPair.second))
 		{
 			auto wallState = battleGetWallState(wallPartPair.second);
 			if(wallState == EWallState::REINFORCED || wallState == EWallState::INTACT || wallState == EWallState::DAMAGED)
 			{
-				attackableBattleHexes.push_back(BattleHex(wallPartPair.first));
+				attackableBattleHexes.emplace_back(wallPartPair.first);
 			}
 		}
 	}
@@ -1445,7 +1446,7 @@ int32_t CBattleInfoCallback::battleGetSpellCost(const spells::Spell * sp, const
 	int32_t manaReduction = 0;
 	int32_t manaIncrease = 0;
 
-	for(auto unit : battleAliveUnits())
+	for(const auto * unit : battleAliveUnits())
 	{
 		if(unit->unitOwner() == caster->tempOwner && unit->hasBonusOfType(Bonus::CHANGES_SPELL_COST_FOR_ALLY))
 		{
@@ -1472,7 +1473,7 @@ bool CBattleInfoCallback::battleIsUnitBlocked(const battle::Unit * unit) const
 	if(unit->hasBonusOfType(Bonus::SIEGE_WEAPON)) //siege weapons cannot be blocked
 		return false;
 
-	for(auto adjacent : battleAdjacentUnits(unit))
+	for(const auto * adjacent : battleAdjacentUnits(unit))
 	{
 		if(adjacent->unitOwner() != unit->unitOwner()) //blocked by enemy stack
 			return true;
@@ -1487,7 +1488,7 @@ std::set<const battle::Unit *> CBattleInfoCallback::battleAdjacentUnits(const ba
 
 	for(auto hex : unit->getSurroundingHexes())
 	{
-		if(auto neighbour = battleGetUnitByPos(hex, true))
+		if(const auto * neighbour = battleGetUnitByPos(hex, true))
 			ret.insert(neighbour);
 	}
 
@@ -1552,22 +1553,22 @@ SpellID CBattleInfoCallback::getRandomBeneficialSpell(CRandomGenerator & rand, c
 		case SpellID::SHIELD:
 		case SpellID::FIRE_SHIELD: // not if all enemy units are shooters
 		{
-			auto walker = getAliveEnemy([&](const CStack * stack) //look for enemy, non-shooting stack
+			const auto * walker = getAliveEnemy([&](const CStack * stack) //look for enemy, non-shooting stack
 			{
 				return !stack->canShoot();
 			});
 
-			if (!walker)
+			if(!walker)
 				continue;
 		}
 			break;
 		case SpellID::AIR_SHIELD: //only against active shooters
 		{
-			auto shooter = getAliveEnemy([&](const CStack * stack) //look for enemy, non-shooting stack
+			const auto * shooter = getAliveEnemy([&](const CStack * stack) //look for enemy, non-shooting stack
 			{
 				return stack->canShoot();
 			});
-			if (!shooter)
+			if(!shooter)
 				continue;
 		}
 			break;
@@ -1605,7 +1606,7 @@ SpellID CBattleInfoCallback::getRandomBeneficialSpell(CRandomGenerator & rand, c
 			break;
 		case SpellID::SLAYER://only if monsters are present
 		{
-			auto kingMonster = getAliveEnemy([&](const CStack * stack) -> bool //look for enemy, non-shooting stack
+			const auto * kingMonster = getAliveEnemy([&](const CStack * stack) -> bool //look for enemy, non-shooting stack
 			{
 				const auto isKing = Selector::type()(Bonus::KING1)
 									.Or(Selector::type()(Bonus::KING2))
@@ -1640,12 +1641,12 @@ SpellID CBattleInfoCallback::getRandomCastedSpell(CRandomGenerator & rand,const
 	if (!bl->size())
 		return SpellID::NONE;
 	int totalWeight = 0;
-	for(auto b : *bl)
+	for(const auto & b : *bl)
 	{
 		totalWeight += std::max(b->additionalInfo[0], 1); //minimal chance to cast is 1
 	}
 	int randomPos = rand.nextInt(totalWeight - 1);
-	for(auto b : *bl)
+	for(const auto & b : *bl)
 	{
 		randomPos -= std::max(b->additionalInfo[0], 1);
 		if(randomPos < 0)
@@ -1657,7 +1658,7 @@ SpellID CBattleInfoCallback::getRandomCastedSpell(CRandomGenerator & rand,const
 	return SpellID::NONE;
 }
 
-int CBattleInfoCallback::battleGetSurrenderCost(PlayerColor Player) const
+int CBattleInfoCallback::battleGetSurrenderCost(const PlayerColor & Player) const
 {
 	RETURN_IF_NOT_BATTLE(-3);
 	if(!battleCanSurrender(Player))
@@ -1671,7 +1672,7 @@ int CBattleInfoCallback::battleGetSurrenderCost(PlayerColor Player) const
 	int ret = 0;
 	double discount = 0;
 
-	for(auto unit : battleAliveUnits(side))
+	for(const auto * unit : battleAliveUnits(side))
 		ret += unit->getRawSurrenderCost();
 
 	if(const CGHeroInstance * h = battleGetFightingHero(side))
diff --git a/lib/battle/CBattleInfoCallback.h b/lib/battle/CBattleInfoCallback.h
index 6789b9504..e98f6c77a 100644
--- a/lib/battle/CBattleInfoCallback.h
+++ b/lib/battle/CBattleInfoCallback.h
@@ -106,7 +106,7 @@ public:
 
 	std::vector<BattleHex> battleGetAvailableHexes(const ReachabilityInfo & cache, const battle::Unit * unit) const;
 
-	int battleGetSurrenderCost(PlayerColor Player) const; //returns cost of surrendering battle, -1 if surrendering is not possible
+	int battleGetSurrenderCost(const PlayerColor & Player) const; //returns cost of surrendering battle, -1 if surrendering is not possible
 	ReachabilityInfo::TDistances battleGetDistances(const battle::Unit * unit, BattleHex assumedPosition) const;
 	std::set<BattleHex> battleGetAttackedHexes(const CStack* attacker, BattleHex destinationTile, BattleHex attackerPos = BattleHex::INVALID) const;
 	bool isEnemyUnitWithinSpecifiedRange(BattleHex attackerPosition, const battle::Unit * defenderUnit, unsigned int range) const;
@@ -156,7 +156,7 @@ public:
 	AttackableTiles getPotentiallyShootableHexes(const  battle::Unit* attacker, BattleHex destinationTile, BattleHex attackerPos) const;
 	std::vector<const battle::Unit *> getAttackedBattleUnits(const battle::Unit* attacker, BattleHex destinationTile, bool rangedAttack, BattleHex attackerPos = BattleHex::INVALID) const; //calculates range of multi-hex attacks
 	std::set<const CStack*> getAttackedCreatures(const CStack* attacker, BattleHex destinationTile, bool rangedAttack, BattleHex attackerPos = BattleHex::INVALID) const; //calculates range of multi-hex attacks
-	bool isToReverse(const battle::Unit *attacker, const battle::Unit *defender) const; //determines if attacker standing at attackerHex should reverse in order to attack defender
+	bool isToReverse(const battle::Unit * attacker, const battle::Unit * defender) const; //determines if attacker standing at attackerHex should reverse in order to attack defender
 
 	ReachabilityInfo getReachability(const battle::Unit * unit) const;
 	ReachabilityInfo getReachability(const ReachabilityInfo::Parameters & params) const;
@@ -165,7 +165,7 @@ public:
 	AccessibilityInfo getAccesibility(const std::vector<BattleHex> & accessibleHexes) const; //given hexes will be marked as accessible
 	std::pair<const battle::Unit *, BattleHex> getNearestStack(const battle::Unit * closest) const;
 
-	BattleHex getAvaliableHex(CreatureID creID, ui8 side, int initialPos = -1) const; //find place for adding new stack
+	BattleHex getAvaliableHex(const CreatureID & creID, ui8 side, int initialPos = -1) const; //find place for adding new stack
 protected:
 	ReachabilityInfo getFlyingReachability(const ReachabilityInfo::Parameters & params) const;
 	ReachabilityInfo makeBFS(const AccessibilityInfo & accessibility, const ReachabilityInfo::Parameters & params) const;
diff --git a/lib/battle/CBattleInfoEssentials.cpp b/lib/battle/CBattleInfoEssentials.cpp
index e7e0062aa..081030394 100644
--- a/lib/battle/CBattleInfoEssentials.cpp
+++ b/lib/battle/CBattleInfoEssentials.cpp
@@ -53,9 +53,9 @@ std::vector<std::shared_ptr<const CObstacleInstance>> CBattleInfoEssentials::bat
 		}
 	}
 
-	for(auto obstacle : getBattle()->getAllObstacles())
+	for(const auto & obstacle : getBattle()->getAllObstacles())
 	{
-		if(battleIsObstacleVisibleForSide(*(obstacle.get()), *perspective))
+		if(battleIsObstacleVisibleForSide(*(obstacle), *perspective))
 			ret.push_back(obstacle);
 	}
 
@@ -108,7 +108,7 @@ TStacks CBattleInfoEssentials::battleGetAllStacks(bool includeTurrets) const
 TStacks CBattleInfoEssentials::battleGetStacksIf(TStackFilter predicate) const
 {
 	RETURN_IF_NOT_BATTLE(TStacks());
-	return getBattle()->getStacksIf(predicate);
+	return getBattle()->getStacksIf(std::move(predicate));
 }
 
 battle::Units CBattleInfoEssentials::battleGetUnitsIf(battle::UnitFilter predicate)  const
@@ -240,7 +240,7 @@ const CArmedInstance * CBattleInfoEssentials::battleGetArmyObject(ui8 side) cons
 
 InfoAboutHero CBattleInfoEssentials::battleGetHeroInfo(ui8 side) const
 {
-	auto hero = getBattle()->getSideHero(side);
+	const auto * hero = getBattle()->getSideHero(side);
 	if(!hero)
 	{
 		return InfoAboutHero();
@@ -260,7 +260,7 @@ const IBonusBearer * CBattleInfoEssentials::getBattleNode() const
 	return getBattle()->asBearer();
 }
 
-bool CBattleInfoEssentials::battleCanFlee(PlayerColor player) const
+bool CBattleInfoEssentials::battleCanFlee(const PlayerColor & player) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 	const auto side = playerToSide(player);
@@ -280,7 +280,7 @@ bool CBattleInfoEssentials::battleCanFlee(PlayerColor player) const
 	//we are besieged defender
 	if(side.get() == BattleSide::DEFENDER && battleGetSiegeLevel())
 	{
-		auto town = battleGetDefendedTown();
+		const auto * town = battleGetDefendedTown();
 		if(!town->hasBuilt(BuildingSubID::ESCAPE_TUNNEL))
 			return false;
 	}
@@ -288,7 +288,7 @@ bool CBattleInfoEssentials::battleCanFlee(PlayerColor player) const
 	return true;
 }
 
-BattleSideOpt CBattleInfoEssentials::playerToSide(PlayerColor player) const
+BattleSideOpt CBattleInfoEssentials::playerToSide(const PlayerColor & player) const
 {
 	RETURN_IF_NOT_BATTLE(boost::none);
 
@@ -317,7 +317,7 @@ ui8 CBattleInfoEssentials::otherSide(ui8 side) const
 		return BattleSide::ATTACKER;
 }
 
-PlayerColor CBattleInfoEssentials::otherPlayer(PlayerColor player) const
+PlayerColor CBattleInfoEssentials::otherPlayer(const PlayerColor & player) const
 {
 	RETURN_IF_NOT_BATTLE(PlayerColor::CANNOT_DETERMINE);
 
@@ -328,7 +328,7 @@ PlayerColor CBattleInfoEssentials::otherPlayer(PlayerColor player) const
 	return getBattle()->getSidePlayer(otherSide(side.get()));
 }
 
-bool CBattleInfoEssentials::playerHasAccessToHeroInfo(PlayerColor player, const CGHeroInstance * h) const
+bool CBattleInfoEssentials::playerHasAccessToHeroInfo(const PlayerColor & player, const CGHeroInstance * h) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 	const auto side = playerToSide(player);
@@ -347,7 +347,7 @@ ui8 CBattleInfoEssentials::battleGetSiegeLevel() const
 	return getBattle()->getDefendedTown() ? getBattle()->getDefendedTown()->fortLevel() : CGTownInstance::NONE;
 }
 
-bool CBattleInfoEssentials::battleCanSurrender(PlayerColor player) const
+bool CBattleInfoEssentials::battleCanSurrender(const PlayerColor & player) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 	const auto side = playerToSide(player);
diff --git a/lib/battle/CBattleInfoEssentials.h b/lib/battle/CBattleInfoEssentials.h
index 117e352b3..ca0b0b93f 100644
--- a/lib/battle/CBattleInfoEssentials.h
+++ b/lib/battle/CBattleInfoEssentials.h
@@ -20,8 +20,8 @@ class IBonusBearer;
 struct InfoAboutHero;
 class CArmedInstance;
 
-typedef std::vector<const CStack *> TStacks;
-typedef std::function<bool(const CStack *)> TStackFilter;
+using TStacks = std::vector<const CStack *>;
+using TStackFilter = std::function<bool (const CStack *)>;
 
 namespace BattlePerspective
 {
@@ -76,15 +76,15 @@ public:
 	si8 battleTacticDist() const override; //returns tactic distance in current tactics phase; 0 if not in tactics phase
 	si8 battleGetTacticsSide() const override; //returns which side is in tactics phase, undefined if none (?)
 
-	bool battleCanFlee(PlayerColor player) const;
-	bool battleCanSurrender(PlayerColor player) const;
+	bool battleCanFlee(const PlayerColor & player) const;
+	bool battleCanSurrender(const PlayerColor & player) const;
 
 	ui8 otherSide(ui8 side) const;
-	PlayerColor otherPlayer(PlayerColor player) const;
+	PlayerColor otherPlayer(const PlayerColor & player) const;
 
-	BattleSideOpt playerToSide(PlayerColor player) const;
+	BattleSideOpt playerToSide(const PlayerColor & player) const;
 	PlayerColor sideToPlayer(ui8 side) const;
-	bool playerHasAccessToHeroInfo(PlayerColor player, const CGHeroInstance * h) const;
+	bool playerHasAccessToHeroInfo(const PlayerColor & player, const CGHeroInstance * h) const;
 	ui8 battleGetSiegeLevel() const; //returns 0 when there is no siege, 1 if fort, 2 is citadel, 3 is castle
 	bool battleHasHero(ui8 side) const;
 	uint32_t battleCastSpells(ui8 side) const; //how many spells has given side cast
diff --git a/lib/battle/CCallbackBase.cpp b/lib/battle/CCallbackBase.cpp
index 6bf4f7840..86d1b21f7 100644
--- a/lib/battle/CCallbackBase.cpp
+++ b/lib/battle/CCallbackBase.cpp
@@ -23,13 +23,10 @@ const IBattleInfo * CCallbackBase::getBattle() const
 	return battle;
 }
 
-CCallbackBase::CCallbackBase(boost::optional<PlayerColor> Player)
-	: battle(nullptr), player(Player)
-{}
-
-CCallbackBase::CCallbackBase()
-	: battle(nullptr)
-{}
+CCallbackBase::CCallbackBase(boost::optional<PlayerColor> Player):
+	player(std::move(Player))
+{
+}
 
 void CCallbackBase::setBattle(const IBattleInfo * B)
 {
diff --git a/lib/battle/CCallbackBase.h b/lib/battle/CCallbackBase.h
index b83a8e82e..b409b938b 100644
--- a/lib/battle/CCallbackBase.h
+++ b/lib/battle/CCallbackBase.h
@@ -23,7 +23,7 @@ class CBattleInfoEssentials;
 //Basic class for various callbacks (interfaces called by players to get info about game and so forth)
 class DLL_LINKAGE CCallbackBase
 {
-	const IBattleInfo * battle; //battle to which the player is engaged, nullptr if none or not applicable
+	const IBattleInfo * battle = nullptr; //battle to which the player is engaged, nullptr if none or not applicable
 
 	const IBattleInfo * getBattle() const;
 
@@ -31,7 +31,7 @@ protected:
 	boost::optional<PlayerColor> player; // not set gives access to all information, otherwise callback provides only information "visible" for player
 
 	CCallbackBase(boost::optional<PlayerColor> Player);
-	CCallbackBase();
+	CCallbackBase() = default;
 
 	void setBattle(const IBattleInfo * B);
 	bool duringBattle() const;
diff --git a/lib/battle/CObstacleInstance.cpp b/lib/battle/CObstacleInstance.cpp
index 94e1d9d01..715412e62 100644
--- a/lib/battle/CObstacleInstance.cpp
+++ b/lib/battle/CObstacleInstance.cpp
@@ -20,18 +20,6 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-CObstacleInstance::CObstacleInstance()
-{
-	obstacleType = USUAL;
-	uniqueID = -1;
-	ID = -1;
-}
-
-CObstacleInstance::~CObstacleInstance()
-{
-
-}
-
 const ObstacleInfo & CObstacleInstance::getInfo() const
 {
 	assert( obstacleType == USUAL || obstacleType == ABSOLUTE_OBSTACLE);
diff --git a/lib/battle/CObstacleInstance.h b/lib/battle/CObstacleInstance.h
index a4731d608..7b4b52a05 100644
--- a/lib/battle/CObstacleInstance.h
+++ b/lib/battle/CObstacleInstance.h
@@ -25,12 +25,11 @@ struct DLL_LINKAGE CObstacleInstance
 	};
 
 	BattleHex pos; //position on battlefield, typically left bottom corner
-	EObstacleType obstacleType;
-	si32 uniqueID;
-	si32 ID; //ID of obstacle (defines type of it)
+	EObstacleType obstacleType = USUAL;
+	si32 uniqueID = -1;
+	si32 ID = -1; //ID of obstacle (defines type of it)
 
-	CObstacleInstance();
-	virtual ~CObstacleInstance();
+	virtual ~CObstacleInstance() = default;
 
 	const ObstacleInfo &getInfo() const; //allowed only when not generated by spell (usual or absolute)
 
diff --git a/lib/battle/CUnitState.cpp b/lib/battle/CUnitState.cpp
index 06745b334..2923575c0 100644
--- a/lib/battle/CUnitState.cpp
+++ b/lib/battle/CUnitState.cpp
@@ -24,22 +24,14 @@ VCMI_LIB_NAMESPACE_BEGIN
 namespace battle
 {
 ///CAmmo
-CAmmo::CAmmo(const battle::Unit * Owner, CSelector totalSelector)
-	: used(0),
+CAmmo::CAmmo(const battle::Unit * Owner, CSelector totalSelector):
+	used(0),
 	owner(Owner),
-	totalProxy(Owner, totalSelector)
+	totalProxy(Owner, std::move(totalSelector))
 {
 	reset();
 }
 
-CAmmo::CAmmo(const CAmmo & other)
-	: used(other.used),
-	owner(other.owner),
-	totalProxy(other.totalProxy)
-{
-
-}
-
 CAmmo & CAmmo::operator= (const CAmmo & other)
 {
 	used = other.used;
@@ -98,13 +90,6 @@ CShots::CShots(const battle::Unit * Owner)
 {
 }
 
-CShots::CShots(const CShots & other)
-	: CAmmo(other),
-	env(other.env),
-	shooter(other.shooter)
-{
-}
-
 CShots & CShots::operator=(const CShots & other)
 {
 	CAmmo::operator=(other);
@@ -136,17 +121,6 @@ CCasts::CCasts(const battle::Unit * Owner):
 {
 }
 
-CCasts::CCasts(const CCasts & other)
-	: CAmmo(other)
-{
-}
-
-CCasts & CCasts::operator=(const CCasts & other)
-{
-	CAmmo::operator=(other);
-	return *this;
-}
-
 ///CRetaliations
 CRetaliations::CRetaliations(const battle::Unit * Owner)
 	: CAmmo(Owner, Selector::type()(Bonus::ADDITIONAL_RETALIATION)),
@@ -156,23 +130,6 @@ CRetaliations::CRetaliations(const battle::Unit * Owner)
 {
 }
 
-CRetaliations::CRetaliations(const CRetaliations & other)
-	: CAmmo(other),
-	totalCache(other.totalCache),
-	noRetaliation(other.noRetaliation),
-	unlimited(other.unlimited)
-{
-}
-
-CRetaliations & CRetaliations::operator=(const CRetaliations & other)
-{
-	CAmmo::operator=(other);
-	totalCache = other.totalCache;
-	noRetaliation = other.noRetaliation;
-	unlimited = other.unlimited;
-	return *this;
-}
-
 bool CRetaliations::isLimited() const
 {
 	return !unlimited.getHasBonus() || noRetaliation.getHasBonus();
@@ -209,15 +166,6 @@ CHealth::CHealth(const battle::Unit * Owner):
 	reset();
 }
 
-CHealth::CHealth(const CHealth & other):
-	owner(other.owner),
-	firstHPleft(other.firstHPleft),
-	fullUnits(other.fullUnits),
-	resurrected(other.resurrected)
-{
-
-}
-
 CHealth & CHealth::operator=(const CHealth & other)
 {
 	//do not change owner
@@ -373,8 +321,8 @@ void CHealth::serializeJson(JsonSerializeFormat & handler)
 }
 
 ///CUnitState
-CUnitState::CUnitState()
-	: env(nullptr),
+CUnitState::CUnitState():
+	env(nullptr),
 	cloned(false),
 	defending(false),
 	defendingAnim(false),
@@ -398,8 +346,7 @@ CUnitState::CUnitState()
 	defence(this, Selector::typeSubtype(Bonus::PRIMARY_SKILL, PrimarySkill::DEFENSE), 0),
 	inFrenzy(this, Selector::type()(Bonus::IN_FRENZY)),
 	cloneLifetimeMarker(this, Selector::type()(Bonus::NONE).And(Selector::source(Bonus::SPELL_EFFECT, SpellID::CLONE))),
-	cloneID(-1),
-	position()
+	cloneID(-1)
 {
 
 }
@@ -702,8 +649,8 @@ int CUnitState::getAttack(bool ranged) const
 
 	if(!inFrenzy->empty())
 	{
-		double frenzyPower = (double)inFrenzy->totalValue() / 100;
-		frenzyPower *= (double) (ranged ? defence.getRangedValue() : defence.getMeleeValue());
+		double frenzyPower = static_cast<double>(inFrenzy->totalValue()) / 100;
+		frenzyPower *= static_cast<double>(ranged ? defence.getRangedValue() : defence.getMeleeValue());
 		ret += static_cast<int>(frenzyPower);
 	}
 
@@ -893,12 +840,10 @@ void CUnitState::onRemoved()
 	ghost = true;
 }
 
-CUnitStateDetached::CUnitStateDetached(const IUnitInfo * unit_, const IBonusBearer * bonus_)
-	: CUnitState(),
+CUnitStateDetached::CUnitStateDetached(const IUnitInfo * unit_, const IBonusBearer * bonus_):
 	unit(unit_),
 	bonus(bonus_)
 {
-
 }
 
 TConstBonusListPtr CUnitStateDetached::getAllBonuses(const CSelector & selector, const CSelector & limit, const CBonusSystemNode * root, const std::string & cachingStr) const
diff --git a/lib/battle/CUnitState.h b/lib/battle/CUnitState.h
index 36a716299..2c9dee6a2 100644
--- a/lib/battle/CUnitState.h
+++ b/lib/battle/CUnitState.h
@@ -32,7 +32,7 @@ public:
 	explicit CAmmo(const battle::Unit * Owner, CSelector totalSelector);
 
 	//only copy construction is allowed for acquire(), serializeJson should be used for any other "assignment"
-	CAmmo(const CAmmo & other);
+	CAmmo(const CAmmo & other) = default;
 	CAmmo(CAmmo && other) = delete;
 
 	CAmmo & operator=(const CAmmo & other);
@@ -56,7 +56,7 @@ class DLL_LINKAGE CShots : public CAmmo
 {
 public:
 	explicit CShots(const battle::Unit * Owner);
-	CShots(const CShots & other);
+	CShots(const CShots & other) = default;
 	CShots & operator=(const CShots & other);
 	bool isLimited() const override;
 	int32_t total() const override;
@@ -72,16 +72,16 @@ class DLL_LINKAGE CCasts : public CAmmo
 {
 public:
 	explicit CCasts(const battle::Unit * Owner);
-	CCasts(const CCasts & other);
-	CCasts & operator=(const CCasts & other);
+	CCasts(const CCasts & other) = default;
+	CCasts & operator=(const CCasts & other) = default;
 };
 
 class DLL_LINKAGE CRetaliations : public CAmmo
 {
 public:
 	explicit CRetaliations(const battle::Unit * Owner);
-	CRetaliations(const CRetaliations & other);
-	CRetaliations & operator=(const CRetaliations & other);
+	CRetaliations(const CRetaliations & other) = default;
+	CRetaliations & operator=(const CRetaliations & other) = default;
 	bool isLimited() const override;
 	int32_t total() const override;
 	void reset() override;
@@ -98,7 +98,7 @@ class DLL_LINKAGE CHealth
 {
 public:
 	explicit CHealth(const battle::Unit * Owner);
-	CHealth(const CHealth & other);
+	CHealth(const CHealth & other) = default;
 
 	CHealth & operator=(const CHealth & other);
 
diff --git a/lib/battle/DamageCalculator.cpp b/lib/battle/DamageCalculator.cpp
index e176e4bde..0f702754f 100644
--- a/lib/battle/DamageCalculator.cpp
+++ b/lib/battle/DamageCalculator.cpp
@@ -30,7 +30,7 @@ TDmgRange DamageCalculator::getBaseDamageSingle() const
 
 	if(info.attacker->creatureIndex() == CreatureID::ARROW_TOWERS)
 	{
-		auto town = callback.battleGetDefendedTown();
+		const auto * town = callback.battleGetDefendedTown();
 		assert(town);
 
 		switch(info.attacker->getPosition())
@@ -78,8 +78,8 @@ TDmgRange DamageCalculator::getBaseDamageBlessCurse() const
 
 	TDmgRange baseDamage = getBaseDamageSingle();
 	TDmgRange modifiedDamage = {
-		std::max( int64_t(1), baseDamage.first  + curseBlessAdditiveModifier),
-		std::max( int64_t(1), baseDamage.second + curseBlessAdditiveModifier)
+		std::max(static_cast<int64_t>(1), baseDamage.first + curseBlessAdditiveModifier),
+		std::max(static_cast<int64_t>(1), baseDamage.second + curseBlessAdditiveModifier)
 	};
 
 	if(curseEffects->size() && blessEffects->size() )
diff --git a/lib/battle/Destination.cpp b/lib/battle/Destination.cpp
index 406b51f97..363285cf6 100644
--- a/lib/battle/Destination.cpp
+++ b/lib/battle/Destination.cpp
@@ -25,8 +25,6 @@ Destination::Destination()
 
 }
 
-Destination::~Destination() = default;
-
 Destination::Destination(const battle::Unit * destination)
 	: unitValue(destination),
 	hexValue(destination->getPosition())
@@ -47,22 +45,6 @@ Destination::Destination(const Unit * destination, const BattleHex & exactHex)
 {
 
 }
-
-Destination::Destination(const Destination & other)
-	: unitValue(other.unitValue),
-	hexValue(other.hexValue)
-{
-
-}
-
-Destination & Destination::operator=(const Destination & other)
-{
-	unitValue = other.unitValue;
-	hexValue = other.hexValue;
-	return *this;
-}
-
-
 }
 
 VCMI_LIB_NAMESPACE_END
diff --git a/lib/battle/Destination.h b/lib/battle/Destination.h
index 904e8fc8d..2dc0a450c 100644
--- a/lib/battle/Destination.h
+++ b/lib/battle/Destination.h
@@ -23,14 +23,14 @@ class DLL_LINKAGE Destination
 {
 public:
 	Destination();
-	~Destination();
+	~Destination() = default;
 	explicit Destination(const Unit * destination);
 	explicit Destination(const BattleHex & destination);
 	explicit Destination(const Unit * destination, const BattleHex & exactHex);
 
-	Destination(const Destination & other);
+	Destination(const Destination & other) = default;
 
-	Destination & operator=(const Destination & other);
+	Destination & operator=(const Destination & other) = default;
 
 	const Unit * unitValue;
 	BattleHex hexValue;
diff --git a/lib/battle/ReachabilityInfo.cpp b/lib/battle/ReachabilityInfo.cpp
index d61e294a8..b7be03882 100644
--- a/lib/battle/ReachabilityInfo.cpp
+++ b/lib/battle/ReachabilityInfo.cpp
@@ -14,21 +14,13 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-
-ReachabilityInfo::Parameters::Parameters()
+ReachabilityInfo::Parameters::Parameters(const battle::Unit * Stack, BattleHex StartPosition):
+	perspective(static_cast<BattlePerspective::BattlePerspective>(Stack->unitSide())),
+	startPosition(StartPosition),
+	doubleWide(Stack->doubleWide()),
+	side(Stack->unitSide()),
+	flying(Stack->hasBonusOfType(Bonus::FLYING))
 {
-	perspective = BattlePerspective::ALL_KNOWING;
-	side = 0;
-	doubleWide = flying = false;
-}
-
-ReachabilityInfo::Parameters::Parameters(const battle::Unit * Stack, BattleHex StartPosition)
-{
-	perspective = (BattlePerspective::BattlePerspective)(Stack->unitSide());
-	startPosition = StartPosition;
-	doubleWide = Stack->doubleWide();
-	side = Stack->unitSide();
-	flying = Stack->hasBonusOfType(Bonus::FLYING);
 	knownAccessible = battle::Unit::getHexes(startPosition, doubleWide, side);
 }
 
diff --git a/lib/battle/ReachabilityInfo.h b/lib/battle/ReachabilityInfo.h
index 1e4e03e27..134390605 100644
--- a/lib/battle/ReachabilityInfo.h
+++ b/lib/battle/ReachabilityInfo.h
@@ -18,22 +18,22 @@ VCMI_LIB_NAMESPACE_BEGIN
 // startPosition and perpective.
 struct DLL_LINKAGE ReachabilityInfo
 {
-	typedef std::array<uint32_t, GameConstants::BFIELD_SIZE> TDistances;
-	typedef std::array<BattleHex, GameConstants::BFIELD_SIZE> TPredecessors;
+	using TDistances = std::array<uint32_t, GameConstants::BFIELD_SIZE>;
+	using TPredecessors = std::array<BattleHex, GameConstants::BFIELD_SIZE>;
 
 	enum { INFINITE_DIST = 1000000 };
 
 	struct DLL_LINKAGE Parameters
 	{
-		ui8 side;
-		bool doubleWide;
-		bool flying;
+		ui8 side = 0;
+		bool doubleWide = false;
+		bool flying = false;
 		std::vector<BattleHex> knownAccessible; //hexes that will be treated as accessible, even if they're occupied by stack (by default - tiles occupied by stack we do reachability for, so it doesn't block itself)
 
 		BattleHex startPosition; //assumed position of stack
-		BattlePerspective::BattlePerspective perspective; //some obstacles (eg. quicksands) may be invisible for some side
+		BattlePerspective::BattlePerspective perspective = BattlePerspective::ALL_KNOWING; //some obstacles (eg. quicksands) may be invisible for some side
 
-		Parameters();
+		Parameters() = default;
 		Parameters(const battle::Unit * Stack, BattleHex StartPosition);
 	};
 
diff --git a/lib/battle/SideInBattle.cpp b/lib/battle/SideInBattle.cpp
index 638de4929..1fd2c63d0 100644
--- a/lib/battle/SideInBattle.cpp
+++ b/lib/battle/SideInBattle.cpp
@@ -13,15 +13,6 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-SideInBattle::SideInBattle()
-{
-	color = PlayerColor::CANNOT_DETERMINE;
-	hero = nullptr;
-	armyObject = nullptr;
-	castSpellsCount = 0;
-	enchanterCounter = 0;
-}
-
 void SideInBattle::init(const CGHeroInstance * Hero, const CArmedInstance * Army)
 {
 	hero = Hero;
diff --git a/lib/battle/SideInBattle.h b/lib/battle/SideInBattle.h
index 14ff2cc13..d7ca30dc2 100644
--- a/lib/battle/SideInBattle.h
+++ b/lib/battle/SideInBattle.h
@@ -17,15 +17,14 @@ class CArmedInstance;
 
 struct DLL_LINKAGE SideInBattle
 {
-	PlayerColor color;
-	const CGHeroInstance * hero; //may be NULL if army is not commanded by hero
-	const CArmedInstance * armyObject; //adv. map object with army that participates in battle; may be same as hero
+	PlayerColor color = PlayerColor::CANNOT_DETERMINE;
+	const CGHeroInstance * hero = nullptr; //may be NULL if army is not commanded by hero
+	const CArmedInstance * armyObject = nullptr; //adv. map object with army that participates in battle; may be same as hero
 
-	uint32_t castSpellsCount; //how many spells each side has been cast this turn
+	uint32_t castSpellsCount = 0; //how many spells each side has been cast this turn
 	std::vector<SpellID> usedSpellsHistory; //every time hero casts spell, it's inserted here -> eagle eye skill
-	int32_t enchanterCounter; //tends to pass through 0, so sign is needed
+	int32_t enchanterCounter = 0; //tends to pass through 0, so sign is needed
 
-	SideInBattle();
 	void init(const CGHeroInstance * Hero, const CArmedInstance * Army);
 
 
diff --git a/lib/battle/SiegeInfo.cpp b/lib/battle/SiegeInfo.cpp
index 4666671e3..10877860f 100644
--- a/lib/battle/SiegeInfo.cpp
+++ b/lib/battle/SiegeInfo.cpp
@@ -15,9 +15,9 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 SiegeInfo::SiegeInfo()
 {
-	for(int i = 0; i < int(EWallPart::PARTS_COUNT); ++i)
+	for(int i = 0; i < static_cast<int>(EWallPart::PARTS_COUNT); ++i)
 	{
-		wallState[EWallPart(i)] = EWallState::NONE;
+		wallState[static_cast<EWallPart>(i)] = EWallState::NONE;
 	}
 	gateState = EGateState::NONE;
 }
diff --git a/lib/battle/Unit.cpp b/lib/battle/Unit.cpp
index d98cfe2e9..8e6a82eba 100644
--- a/lib/battle/Unit.cpp
+++ b/lib/battle/Unit.cpp
@@ -59,7 +59,7 @@ std::vector<BattleHex> Unit::getSurroundingHexes(BattleHex position, bool twoHex
 
 		if(side == BattleSide::ATTACKER)
 		{
-			for(BattleHex::EDir dir = BattleHex::EDir(0); dir <= BattleHex::EDir(4); dir = BattleHex::EDir(dir+1))
+			for(auto dir = static_cast<BattleHex::EDir>(0); dir <= static_cast<BattleHex::EDir>(4); dir = static_cast<BattleHex::EDir>(dir + 1))
 				BattleHex::checkAndPush(position.cloneInDirection(dir, false), hexes);
 
 			BattleHex::checkAndPush(otherHex.cloneInDirection(BattleHex::EDir::BOTTOM_LEFT, false), hexes);
@@ -70,7 +70,7 @@ std::vector<BattleHex> Unit::getSurroundingHexes(BattleHex position, bool twoHex
 		{
 			BattleHex::checkAndPush(position.cloneInDirection(BattleHex::EDir::TOP_LEFT, false), hexes);
 
-			for(BattleHex::EDir dir = BattleHex::EDir(0); dir <= BattleHex::EDir(4); dir = BattleHex::EDir(dir+1))
+			for(auto dir = static_cast<BattleHex::EDir>(0); dir <= static_cast<BattleHex::EDir>(4); dir = static_cast<BattleHex::EDir>(dir + 1))
 				BattleHex::checkAndPush(otherHex.cloneInDirection(dir, false), hexes);
 
 			BattleHex::checkAndPush(position.cloneInDirection(BattleHex::EDir::BOTTOM_LEFT, false), hexes);
@@ -206,16 +206,6 @@ int Unit::getRawSurrenderCost() const
 }
 
 ///UnitInfo
-UnitInfo::UnitInfo()
-	: id(0),
-	count(0),
-	type(),
-	side(0),
-	position(),
-	summoned(false)
-{
-}
-
 void UnitInfo::serializeJson(JsonSerializeFormat & handler)
 {
 	handler.serializeInt("count", count);
diff --git a/lib/battle/Unit.h b/lib/battle/Unit.h
index 5a0e858d7..0eb7ad497 100644
--- a/lib/battle/Unit.h
+++ b/lib/battle/Unit.h
@@ -116,14 +116,12 @@ public:
 class DLL_LINKAGE UnitInfo
 {
 public:
-    uint32_t id;
-	TQuantity count;
+    uint32_t id = 0;
+	TQuantity count = 0;
 	CreatureID type;
-	ui8 side;
+	ui8 side = 0;
 	BattleHex position;
-	bool summoned;
-
-	UnitInfo();
+	bool summoned = false;
 
 	void serializeJson(JsonSerializeFormat & handler);