Merge branch 'vcmi:develop' into extended_statistic

AI/BattleAI/AttackPossibility.cpp

@@ -26,7 +26,7 @@ void DamageCache::cacheDamage(const battle::Unit * attacker, const battle::Unit
 }
 
 
-void DamageCache::buildDamageCache(std::shared_ptr<HypotheticBattle> hb, int side)
+void DamageCache::buildDamageCache(std::shared_ptr<HypotheticBattle> hb, BattleSide side)
 {
 	auto stacks = hb->battleGetUnitsIf([=](const battle::Unit * u) -> bool
 		{
@@ -245,7 +245,7 @@ AttackPossibility AttackPossibility::evaluate(
 
 	std::vector<BattleHex> defenderHex;
 	if(attackInfo.shooting)
-		defenderHex = defender->getHexes();
+		defenderHex.push_back(defender->getPosition());
 	else
 		defenderHex = CStack::meleeAttackHexes(attacker, defender, hex);
 

AI/BattleAI/AttackPossibility.h

@@ -27,7 +27,7 @@ public:
 	void cacheDamage(const battle::Unit * attacker, const battle::Unit * defender, std::shared_ptr<CBattleInfoCallback> hb);
 	int64_t getDamage(const battle::Unit * attacker, const battle::Unit * defender, std::shared_ptr<CBattleInfoCallback> hb);
 	int64_t getOriginalDamage(const battle::Unit * attacker, const battle::Unit * defender, std::shared_ptr<CBattleInfoCallback> hb);
-	void buildDamageCache(std::shared_ptr<HypotheticBattle> hb, int side);
+	void buildDamageCache(std::shared_ptr<HypotheticBattle> hb, BattleSide side);
 };
 
 /// <summary>

AI/BattleAI/BattleAI.cpp

@@ -32,7 +32,7 @@
 #define LOGFL(text, formattingEl) print(boost::str(boost::format(text) % formattingEl))
 
 CBattleAI::CBattleAI()
-	: side(-1),
+	: side(BattleSide::NONE),
 	wasWaitingForRealize(false),
 	wasUnlockingGs(false)
 {
@@ -100,7 +100,7 @@ void CBattleAI::yourTacticPhase(const BattleID & battleID, int distance)
 	cb->battleMakeTacticAction(battleID, BattleAction::makeEndOFTacticPhase(cb->getBattle(battleID)->battleGetTacticsSide()));
 }
 
-static float getStrengthRatio(std::shared_ptr<CBattleInfoCallback> cb, int side)
+static float getStrengthRatio(std::shared_ptr<CBattleInfoCallback> cb, BattleSide side)
 {
 	auto stacks = cb->battleGetAllStacks();
 	auto our = 0;
@@ -243,7 +243,7 @@ BattleAction CBattleAI::useCatapult(const BattleID & battleID, const CStack * st
 	return attack;
 }
 
-void CBattleAI::battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool Side, bool replayAllowed)
+void CBattleAI::battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, BattleSide Side, bool replayAllowed)
 {
 	LOG_TRACE(logAi);
 	side = Side;

AI/BattleAI/BattleAI.h

@@ -27,7 +27,7 @@ struct CurrentOffensivePotential
 	std::map<const CStack *, PotentialTargets> ourAttacks;
 	std::map<const CStack *, PotentialTargets> enemyAttacks;
 
-	CurrentOffensivePotential(ui8 side)
+	CurrentOffensivePotential(BattleSide side)
 	{
 		for(auto stack : cbc->battleGetStacks())
 		{
@@ -54,7 +54,7 @@ struct CurrentOffensivePotential
 
 class CBattleAI : public CBattleGameInterface
 {
-	int side;
+	BattleSide side;
 	std::shared_ptr<CBattleCallback> cb;
 	std::shared_ptr<Environment> env;
 
@@ -80,7 +80,7 @@ public:
 	BattleAction useCatapult(const BattleID & battleID, const CStack *stack);
 	BattleAction useHealingTent(const BattleID & battleID, const CStack *stack);
 
-	void battleStart(const BattleID & battleID, const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, bool Side, bool replayAllowed) override;
+	void battleStart(const BattleID & battleID, const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, BattleSide side, bool replayAllowed) override;
 	//void actionFinished(const BattleAction &action) override;//occurs AFTER every action taken by any stack or by the hero
 	//void actionStarted(const BattleAction &action) override;//occurs BEFORE every action taken by any stack or by the hero
 	//void battleAttack(const BattleAttack *ba) override; //called when stack is performing attack
@@ -93,7 +93,7 @@ public:
 	//void battleSpellCast(const BattleSpellCast *sc) override;
 	//void battleStacksEffectsSet(const SetStackEffect & sse) override;//called when a specific effect is set to stacks
 	//void battleTriggerEffect(const BattleTriggerEffect & bte) override;
-	//void battleStart(const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool side) override; //called by engine when battle starts; side=0 - left, side=1 - right
+	//void battleStart(const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, BattleSide side) override; //called by engine when battle starts; side=0 - left, side=1 - right
 	//void battleCatapultAttacked(const CatapultAttack & ca) override; //called when catapult makes an attack
 	AutocombatPreferences autobattlePreferences = AutocombatPreferences();
 };

AI/BattleAI/BattleEvaluator.cpp

@@ -686,7 +686,7 @@ bool BattleEvaluator::attemptCastingSpell(const CStack * activeStack)
 		spellcast.spell = castToPerform.spell->id;
 		spellcast.setTarget(castToPerform.dest);
 		spellcast.side = side;
-		spellcast.stackNumber = (!side) ? -1 : -2;
+		spellcast.stackNumber = -1;
 		cb->battleMakeSpellAction(battleID, spellcast);
 		activeActionMade = true;
 

AI/BattleAI/BattleEvaluator.h

@@ -33,7 +33,7 @@ class BattleEvaluator
 	std::optional<AttackPossibility> cachedAttack;
 	PlayerColor playerID;
 	BattleID battleID;
-	int side;
+	BattleSide side;
 	float cachedScore;
 	DamageCache damageCache;
 	float strengthRatio;
@@ -54,7 +54,7 @@ public:
 		const battle::Unit * activeStack,
 		PlayerColor playerID,
 		BattleID battleID,
-		int side,
+		BattleSide side,
 		float strengthRatio)
 		:scoreEvaluator(cb->getBattle(battleID), env, strengthRatio), cachedAttack(), playerID(playerID), side(side), env(env), cb(cb), strengthRatio(strengthRatio), battleID(battleID)
 	{
@@ -73,7 +73,7 @@ public:
 		const battle::Unit * activeStack,
 		PlayerColor playerID,
 		BattleID battleID,
-		int side,
+		BattleSide side,
 		float strengthRatio)
 		:scoreEvaluator(cb->getBattle(battleID), env, strengthRatio), cachedAttack(), playerID(playerID), side(side), env(env), cb(cb), hb(hb), damageCache(damageCache), strengthRatio(strengthRatio), battleID(battleID)
 	{

AI/BattleAI/BattleExchangeVariant.cpp

@@ -500,7 +500,7 @@ BattleScore BattleExchangeEvaluator::calculateExchange(
 	logAi->trace("Battle exchange at %d", ap.attack.shooting ? ap.dest.hex : ap.from.hex);
 #endif
 
-	if(cb->battleGetMySide() == BattlePerspective::LEFT_SIDE
+	if(cb->battleGetMySide() == BattleSide::LEFT_SIDE
 		&& cb->battleGetGateState() == EGateState::BLOCKED
 		&& ap.attack.defender->coversPos(BattleHex::GATE_BRIDGE))
 	{

AI/BattleAI/StackWithBonuses.cpp

@@ -116,7 +116,7 @@ uint32_t StackWithBonuses::unitId() const
 	return id;
 }
 
-ui8 StackWithBonuses::unitSide() const
+BattleSide StackWithBonuses::unitSide() const
 {
 	return side;
 }
@@ -467,7 +467,7 @@ int64_t HypotheticBattle::getActualDamage(const DamageRange & damage, int32_t at
 	return (damage.min + damage.max) / 2;
 }
 
-std::vector<SpellID> HypotheticBattle::getUsedSpells(ui8 side) const
+std::vector<SpellID> HypotheticBattle::getUsedSpells(BattleSide side) const
 {
 	// TODO
 	return {};

AI/BattleAI/StackWithBonuses.h

@@ -24,7 +24,7 @@ class HypotheticBattle;
 class RNGStub final : public vstd::RNG
 {
 public:
-	virtual int nextInt() override
+	int nextInt() override
 	{
 		return 0;
 	}
@@ -85,7 +85,7 @@ public:
 	int32_t unitBaseAmount() const override;
 
 	uint32_t unitId() const override;
-	ui8 unitSide() const override;
+	BattleSide unitSide() const override;
 	PlayerColor unitOwner() const override;
 	SlotID unitSlot() const override;
 
@@ -111,7 +111,7 @@ private:
 	const CCreature * type;
 	ui32 baseAmount;
 	uint32_t id;
-	ui8 side;
+	BattleSide side;
 	PlayerColor player;
 	SlotID slot;
 };
@@ -158,7 +158,7 @@ public:
 	uint32_t nextUnitId() const override;
 
 	int64_t getActualDamage(const DamageRange & damage, int32_t attackerCount, vstd::RNG & rng) const override;
-	std::vector<SpellID> getUsedSpells(ui8 side) const override;
+	std::vector<SpellID> getUsedSpells(BattleSide side) const override;
 	int3 getLocation() const override;
 	bool isCreatureBank() const override;
 

AI/Nullkiller/AIGateway.cpp

@@ -1148,7 +1148,7 @@ void AIGateway::recruitCreatures(const CGDwelling * d, const CArmedInstance * re
 	}
 }
 
-void AIGateway::battleStart(const BattleID & battleID, const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, bool side, bool replayAllowed)
+void AIGateway::battleStart(const BattleID & battleID, const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, BattleSide side, bool replayAllowed)
 {
 	NET_EVENT_HANDLER;
 	assert(!playerID.isValidPlayer() || status.getBattle() == UPCOMING_BATTLE);

AI/Nullkiller/AIGateway.h

@@ -156,7 +156,7 @@ public:
 	void showWorldViewEx(const std::vector<ObjectPosInfo> & objectPositions, bool showTerrain) override;
 	std::optional<BattleAction> makeSurrenderRetreatDecision(const BattleID & battleID, const BattleStateInfoForRetreat & battleState) override;
 
-	void battleStart(const BattleID & battleID, const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, bool side, bool replayAllowed) override;
+	void battleStart(const BattleID & battleID, const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, BattleSide side, bool replayAllowed) override;
 	void battleEnd(const BattleID & battleID, const BattleResult * br, QueryID queryID) override;
 
 	void makeTurn();

AI/Nullkiller/Analyzers/BuildAnalyzer.cpp

@@ -31,16 +31,14 @@ void BuildAnalyzer::updateTownDwellings(TownDevelopmentInfo & developmentInfo)
 		}
 	}
 
-	BuildingID prefixes[] = {BuildingID::DWELL_UP_FIRST, BuildingID::DWELL_FIRST};
+	for(int level = 0; level < developmentInfo.town->town->creatures.size(); level++)
-
-	for(int level = 0; level < GameConstants::CREATURES_PER_TOWN; level++)
 	{
 		logAi->trace("Checking dwelling level %d", level);
 		BuildingInfo nextToBuild = BuildingInfo();
 
-		for(BuildingID prefix : prefixes)
+		for(int upgradeIndex : {1, 0})
 		{
-			BuildingID building = BuildingID(prefix + level);
+			BuildingID building = BuildingID(BuildingID::getDwellingFromLevel(level, upgradeIndex));
 
 			if(!vstd::contains(buildings, building))
 				continue; // no such building in town
@@ -211,8 +209,8 @@ BuildingInfo BuildAnalyzer::getBuildingOrPrerequisite(
 
 	if(BuildingID::DWELL_FIRST <= toBuild && toBuild <= BuildingID::DWELL_UP_LAST)
 	{
-		creatureLevel = (toBuild - BuildingID::DWELL_FIRST) % GameConstants::CREATURES_PER_TOWN;
+		creatureLevel = BuildingID::getLevelFromDwelling(toBuild);
-		creatureUpgrade = (toBuild - BuildingID::DWELL_FIRST) / GameConstants::CREATURES_PER_TOWN;
+		creatureUpgrade = BuildingID::getUpgradedFromDwelling(toBuild);
 	}
 	else if(toBuild == BuildingID::HORDE_1 || toBuild == BuildingID::HORDE_1_UPGR)
 	{

AI/Nullkiller/Behaviors/CaptureObjectsBehavior.cpp

@@ -212,7 +212,7 @@ void CaptureObjectsBehavior::decomposeObjects(
 				vstd::concatenate(tasksLocal, getVisitGoals(paths, nullkiller, objToVisit, specificObjects));
 			}
 
-			std::lock_guard<std::mutex> lock(sync); // FIXME: consider using tbb::parallel_reduce instead to avoid mutex overhead
+			std::lock_guard lock(sync); // FIXME: consider using tbb::parallel_reduce instead to avoid mutex overhead
 			vstd::concatenate(result, tasksLocal);
 		});
 }

AI/Nullkiller/Engine/PriorityEvaluator.cpp

@@ -141,12 +141,10 @@ int32_t getResourcesGoldReward(const TResources & res)
 {
 	int32_t result = 0;
 
-	for(EGameResID r = EGameResID(0); r < EGameResID::COUNT; r.advance(1))
+	for(auto r : GameResID::ALL_RESOURCES())
 	{
 		if(res[r] > 0)
-		{
 			result += r == EGameResID::GOLD ? res[r] : res[r] * 100;
-		}
 	}
 
 	return result;
@@ -350,7 +348,7 @@ uint64_t RewardEvaluator::getArmyReward(
 			{
 				for(auto artID : info.reward.artifacts)
 				{
-					const CArtifact * art = dynamic_cast<const CArtifact *>(VLC->artifacts()->getById(artID));
+					const auto * art = dynamic_cast<const CArtifact *>(VLC->artifacts()->getById(artID));
 
 					rewardValue += evaluateArtifactArmyValue(art);
 				}
@@ -358,7 +356,7 @@ uint64_t RewardEvaluator::getArmyReward(
 
 			if(!info.reward.creatures.empty())
 			{
-				for(auto stackInfo : info.reward.creatures)
+				for(const auto & stackInfo : info.reward.creatures)
 				{
 					rewardValue += stackInfo.getType()->getAIValue() * stackInfo.getCount();
 				}

AI/Nullkiller/Pathfinding/ObjectGraphCalculator.cpp

@@ -321,7 +321,7 @@ void ObjectGraphCalculator::addObjectActor(const CGObjectInstance * obj)
 
 void ObjectGraphCalculator::addJunctionActor(const int3 & visitablePos, bool isVirtualBoat)
 {
-	std::lock_guard<std::mutex> lock(syncLock);
+	std::lock_guard lock(syncLock);
 
 	auto internalCb = temporaryActorHeroes.front()->cb;
 	auto objectActor = temporaryActorHeroes.emplace_back(std::make_unique<CGHeroInstance>(internalCb)).get();

AI/StupidAI/StupidAI.cpp

@@ -18,7 +18,7 @@
 #include "../../lib/CRandomGenerator.h"
 
 CStupidAI::CStupidAI()
-	: side(-1)
+	: side(BattleSide::NONE)
 	, wasWaitingForRealize(false)
 	, wasUnlockingGs(false)
 {
@@ -262,7 +262,7 @@ void CStupidAI::battleStacksEffectsSet(const BattleID & battleID, const SetStack
 	print("battleStacksEffectsSet called");
 }
 
-void CStupidAI::battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool Side, bool replayAllowed)
+void CStupidAI::battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, BattleSide Side, bool replayAllowed)
 {
 	print("battleStart called");
 	side = Side;

AI/StupidAI/StupidAI.h

@@ -17,7 +17,7 @@ class EnemyInfo;
 
 class CStupidAI : public CBattleGameInterface
 {
-	int side;
+	BattleSide side;
 	std::shared_ptr<CBattleCallback> cb;
 	std::shared_ptr<Environment> env;
 
@@ -47,7 +47,7 @@ public:
 	void battleSpellCast(const BattleID & battleID, const BattleSpellCast *sc) override;
 	void battleStacksEffectsSet(const BattleID & battleID, const SetStackEffect & sse) override;//called when a specific effect is set to stacks
 	//void battleTriggerEffect(const BattleTriggerEffect & bte) override;
-	void battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool side, bool replayAllowed) override; //called by engine when battle starts; side=0 - left, side=1 - right
+	void battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, BattleSide side, bool replayAllowed) override; //called by engine when battle starts; side=0 - left, side=1 - right
 	void battleCatapultAttacked(const BattleID & battleID, const CatapultAttack & ca) override; //called when catapult makes an attack
 
 private:

AI/VCAI/BuildingManager.cpp

@@ -143,9 +143,9 @@ static const std::vector<BuildingID> basicGoldSource = { BuildingID::TOWN_HALL,
 static const std::vector<BuildingID> defence = { BuildingID::FORT, BuildingID::CITADEL, BuildingID::CASTLE };
 static const std::vector<BuildingID> capitolAndRequirements = { BuildingID::FORT, BuildingID::CITADEL, BuildingID::CASTLE, BuildingID::CAPITOL };
 static const std::vector<BuildingID> unitsSource = { BuildingID::DWELL_LVL_1, BuildingID::DWELL_LVL_2, BuildingID::DWELL_LVL_3,
-BuildingID::DWELL_LVL_4, BuildingID::DWELL_LVL_5, BuildingID::DWELL_LVL_6, BuildingID::DWELL_LVL_7 };
+BuildingID::DWELL_LVL_4, BuildingID::DWELL_LVL_5, BuildingID::DWELL_LVL_6, BuildingID::DWELL_LVL_7, BuildingID::DWELL_LVL_8 };
 static const std::vector<BuildingID> unitsUpgrade = { BuildingID::DWELL_LVL_1_UP, BuildingID::DWELL_LVL_2_UP, BuildingID::DWELL_LVL_3_UP,
-BuildingID::DWELL_LVL_4_UP, BuildingID::DWELL_LVL_5_UP, BuildingID::DWELL_LVL_6_UP, BuildingID::DWELL_LVL_7_UP };
+BuildingID::DWELL_LVL_4_UP, BuildingID::DWELL_LVL_5_UP, BuildingID::DWELL_LVL_6_UP, BuildingID::DWELL_LVL_7_UP, BuildingID::DWELL_LVL_8_UP };
 static const std::vector<BuildingID> unitGrowth = { BuildingID::HORDE_1, BuildingID::HORDE_1_UPGR, BuildingID::HORDE_2, BuildingID::HORDE_2_UPGR };
 static const std::vector<BuildingID> _spells = { BuildingID::MAGES_GUILD_1, BuildingID::MAGES_GUILD_2, BuildingID::MAGES_GUILD_3,
 BuildingID::MAGES_GUILD_4, BuildingID::MAGES_GUILD_5 };

AI/VCAI/Goals/GatherTroops.cpp

@@ -109,7 +109,7 @@ TGoalVec GatherTroops::getAllPossibleSubgoals()
 			if(upgradeNumber < 0)
 				continue;
 
-			BuildingID bid(BuildingID::DWELL_FIRST + creature->getLevel() - 1 + upgradeNumber * GameConstants::CREATURES_PER_TOWN);
+			BuildingID bid(BuildingID::DWELL_FIRST + creature->getLevel() - 1 + upgradeNumber * t->town->creatures.size());
 			if(t->hasBuilt(bid) && ai->ah->freeResources().canAfford(creature->getFullRecruitCost())) //this assumes only creatures with dwellings are assigned to faction
 			{
 				solutions.push_back(sptr(BuyArmy(t, creature->getAIValue() * this->value).setobjid(objid)));

AI/VCAI/VCAI.cpp

@@ -1566,7 +1566,7 @@ void VCAI::completeGoal(Goals::TSubgoal goal)
 
 }
 
-void VCAI::battleStart(const BattleID & battleID, const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, bool side, bool replayAllowed)
+void VCAI::battleStart(const BattleID & battleID, const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, BattleSide side, bool replayAllowed)
 {
 	NET_EVENT_HANDLER;
 	assert(!playerID.isValidPlayer() || status.getBattle() == UPCOMING_BATTLE);

AI/VCAI/VCAI.h

@@ -187,7 +187,7 @@ public:
 	void showMarketWindow(const IMarket * market, const CGHeroInstance * visitor, QueryID queryID) override;
 	void showWorldViewEx(const std::vector<ObjectPosInfo> & objectPositions, bool showTerrain) override;
 
-	void battleStart(const BattleID & battleID, const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, bool side, bool replayAllowed) override;
+	void battleStart(const BattleID & battleID, const CCreatureSet * army1, const CCreatureSet * army2, int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2, BattleSide side, bool replayAllowed) override;
 	void battleEnd(const BattleID & battleID, const BattleResult * br, QueryID queryID) override;
 	std::optional<BattleAction> makeSurrenderRetreatDecision(const BattleID & battleID, const BattleStateInfoForRetreat & battleState) override;
 

client/CPlayerInterface.cpp

@@ -624,7 +624,7 @@ void CPlayerInterface::battleStartBefore(const BattleID & battleID, const CCreat
 		waitForAllDialogs();
 }
 
-void CPlayerInterface::battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool side, bool replayAllowed)
+void CPlayerInterface::battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, BattleSide side, bool replayAllowed)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
 

client/CPlayerInterface.h

@@ -160,7 +160,7 @@ protected: // Call-ins from server, should not be called directly, but only via
 	void battleTriggerEffect(const BattleID & battleID, const BattleTriggerEffect & bte) override; //various one-shot effect
 	void battleStacksAttacked(const BattleID & battleID, const std::vector<BattleStackAttacked> & bsa, bool ranged) override;
 	void battleStartBefore(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2) override; //called by engine just before battle starts; side=0 - left, side=1 - right
-	void battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool side, bool replayAllowed) override; //called by engine when battle starts; side=0 - left, side=1 - right
+	void battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, BattleSide side, bool replayAllowed) override; //called by engine when battle starts; side=0 - left, side=1 - right
 	void battleUnitsChanged(const BattleID & battleID, const std::vector<UnitChanges> & units) override;
 	void battleObstaclesChanged(const BattleID & battleID, const std::vector<ObstacleChanges> & obstacles) override;
 	void battleCatapultAttacked(const BattleID & battleID, const CatapultAttack & ca) override; //called when catapult makes an attack

client/Client.cpp

@@ -443,8 +443,8 @@ void CClient::battleStarted(const BattleInfo * info)
 {
 	std::shared_ptr<CPlayerInterface> att;
 	std::shared_ptr<CPlayerInterface> def;
-	auto & leftSide = info->sides[0];
+	const auto & leftSide = info->getSide(BattleSide::LEFT_SIDE);
-	auto & rightSide = info->sides[1];
+	const auto & rightSide = info->getSide(BattleSide::RIGHT_SIDE);
 
 	for(auto & battleCb : battleCallbacks)
 	{
@@ -453,17 +453,17 @@ void CClient::battleStarted(const BattleInfo * info)
 	}
 
 	//If quick combat is not, do not prepare interfaces for battleint
-	auto callBattleStart = [&](PlayerColor color, ui8 side)
+	auto callBattleStart = [&](PlayerColor color, BattleSide side)
 	{
 		if(vstd::contains(battleints, color))
 			battleints[color]->battleStart(info->battleID, leftSide.armyObject, rightSide.armyObject, info->tile, leftSide.hero, rightSide.hero, side, info->replayAllowed);
 	};
 	
-	callBattleStart(leftSide.color, 0);
+	callBattleStart(leftSide.color, BattleSide::LEFT_SIDE);
-	callBattleStart(rightSide.color, 1);
+	callBattleStart(rightSide.color, BattleSide::RIGHT_SIDE);
-	callBattleStart(PlayerColor::UNFLAGGABLE, 1);
+	callBattleStart(PlayerColor::UNFLAGGABLE, BattleSide::RIGHT_SIDE);
 	if(settings["session"]["spectate"].Bool() && !settings["session"]["spectate-skip-battle"].Bool())
-		callBattleStart(PlayerColor::SPECTATOR, 1);
+		callBattleStart(PlayerColor::SPECTATOR, BattleSide::RIGHT_SIDE);
 	
 	if(vstd::contains(playerint, leftSide.color) && playerint[leftSide.color]->human)
 		att = std::dynamic_pointer_cast<CPlayerInterface>(playerint[leftSide.color]);
@@ -480,9 +480,9 @@ void CClient::battleStarted(const BattleInfo * info)
 			{
 				auto side = interface->cb->getBattle(info->battleID)->playerToSide(interface->playerID);
 
-				if(interface->playerID == info->sides[info->tacticsSide].color)
+				if(interface->playerID == info->getSide(info->tacticsSide).color)
 				{
-					auto action = BattleAction::makeEndOFTacticPhase(*side);
+					auto action = BattleAction::makeEndOFTacticPhase(side);
 					interface->cb->battleMakeTacticAction(info->battleID, action);
 				}
 			}
@@ -514,7 +514,7 @@ void CClient::battleStarted(const BattleInfo * info)
 
 	if(info->tacticDistance)
 	{
-		auto tacticianColor = info->sides[info->tacticsSide].color;
+		auto tacticianColor = info->getSide(info->tacticsSide).color;
 
 		if (vstd::contains(battleints, tacticianColor))
 			battleints[tacticianColor]->yourTacticPhase(info->battleID, info->tacticDistance);
@@ -523,9 +523,11 @@ void CClient::battleStarted(const BattleInfo * info)
 
 void CClient::battleFinished(const BattleID & battleID)
 {
-	for(auto & side : gs->getBattle(battleID)->sides)
+	for(auto side : { BattleSide::ATTACKER, BattleSide::DEFENDER })
-		if(battleCallbacks.count(side.color))
+	{
-			battleCallbacks[side.color]->onBattleEnded(battleID);
+		if(battleCallbacks.count(gs->getBattle(battleID)->getSide(side).color))
+			battleCallbacks[gs->getBattle(battleID)->getSide(side).color]->onBattleEnded(battleID);
+	}
 
 	if(settings["session"]["spectate"].Bool() && !settings["session"]["spectate-skip-battle"].Bool())
 		battleCallbacks[PlayerColor::SPECTATOR]->onBattleEnded(battleID);

client/NetPacksClient.cpp

@@ -108,8 +108,8 @@ void callBattleInterfaceIfPresentForBothSides(CClient & cl, const BattleID & bat
 		return;
 	}
 
-	callOnlyThatBattleInterface(cl, cl.gameState()->getBattle(battleID)->sides[0].color, ptr, std::forward<Args2>(args)...);
+	callOnlyThatBattleInterface(cl, cl.gameState()->getBattle(battleID)->getSide(BattleSide::ATTACKER).color, ptr, std::forward<Args2>(args)...);
-	callOnlyThatBattleInterface(cl, cl.gameState()->getBattle(battleID)->sides[1].color, ptr, std::forward<Args2>(args)...);
+	callOnlyThatBattleInterface(cl, cl.gameState()->getBattle(battleID)->getSide(BattleSide::DEFENDER).color, ptr, std::forward<Args2>(args)...);
 	if(settings["session"]["spectate"].Bool() && !settings["session"]["spectate-skip-battle"].Bool() && LOCPLINT->battleInt)
 	{
 		callOnlyThatBattleInterface(cl, PlayerColor::SPECTATOR, ptr, std::forward<Args2>(args)...);
@@ -769,12 +769,12 @@ void ApplyClientNetPackVisitor::visitMapObjectSelectDialog(MapObjectSelectDialog
 void ApplyFirstClientNetPackVisitor::visitBattleStart(BattleStart & pack)
 {
 	// Cannot use the usual code because curB is not set yet
-	callOnlyThatBattleInterface(cl, pack.info->sides[0].color, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->sides[0].armyObject, pack.info->sides[1].armyObject,
+	callOnlyThatBattleInterface(cl, pack.info->getSide(BattleSide::ATTACKER).color, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->getSide(BattleSide::ATTACKER).armyObject, pack.info->getSide(BattleSide::DEFENDER).armyObject,
-		pack.info->tile, pack.info->sides[0].hero, pack.info->sides[1].hero);
+		pack.info->tile, pack.info->getSide(BattleSide::ATTACKER).hero, pack.info->getSide(BattleSide::DEFENDER).hero);
-	callOnlyThatBattleInterface(cl, pack.info->sides[1].color, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->sides[0].armyObject, pack.info->sides[1].armyObject,
+	callOnlyThatBattleInterface(cl, pack.info->getSide(BattleSide::DEFENDER).color, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->getSide(BattleSide::ATTACKER).armyObject, pack.info->getSide(BattleSide::DEFENDER).armyObject,
-		pack.info->tile, pack.info->sides[0].hero, pack.info->sides[1].hero);
+		pack.info->tile, pack.info->getSide(BattleSide::ATTACKER).hero, pack.info->getSide(BattleSide::DEFENDER).hero);
-	callOnlyThatBattleInterface(cl, PlayerColor::SPECTATOR, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->sides[0].armyObject, pack.info->sides[1].armyObject,
+	callOnlyThatBattleInterface(cl, PlayerColor::SPECTATOR, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->getSide(BattleSide::ATTACKER).armyObject, pack.info->getSide(BattleSide::DEFENDER).armyObject,
-		pack.info->tile, pack.info->sides[0].hero, pack.info->sides[1].hero);
+		pack.info->tile, pack.info->getSide(BattleSide::ATTACKER).hero, pack.info->getSide(BattleSide::DEFENDER).hero);
 }
 
 void ApplyClientNetPackVisitor::visitBattleStart(BattleStart & pack)
@@ -801,9 +801,9 @@ void ApplyClientNetPackVisitor::visitBattleSetActiveStack(BattleSetActiveStack &
 	PlayerColor playerToCall; //pack.player that will move activated stack
 	if (activated->hasBonusOfType(BonusType::HYPNOTIZED))
 	{
-		playerToCall = (gs.getBattle(pack.battleID)->sides[0].color == activated->unitOwner()
+		playerToCall = gs.getBattle(pack.battleID)->getSide(BattleSide::ATTACKER).color == activated->unitOwner()
-			? gs.getBattle(pack.battleID)->sides[1].color
+			? gs.getBattle(pack.battleID)->getSide(BattleSide::DEFENDER).color
-			: gs.getBattle(pack.battleID)->sides[0].color);
+			: gs.getBattle(pack.battleID)->getSide(BattleSide::ATTACKER).color;
 	}
 	else
 	{

client/adventureMap/AdventureMapShortcuts.cpp

@@ -530,7 +530,6 @@ bool AdventureMapShortcuts::optionCanVisitObject()
 	auto * hero = LOCPLINT->localState->getCurrentHero();
 	auto objects = LOCPLINT->cb->getVisitableObjs(hero->visitablePos());
 
-	//assert(vstd::contains(objects,hero));
 	return objects.size() > 1; // there is object other than our hero
 }
 

client/adventureMap/CList.cpp

@@ -18,10 +18,12 @@
 #include "../widgets/ObjectLists.h"
 #include "../widgets/RadialMenu.h"
 #include "../windows/InfoWindows.h"
+#include "../windows/CCastleInterface.h"
 #include "../CGameInfo.h"
 #include "../CPlayerInterface.h"
 #include "../PlayerLocalState.h"
 #include "../gui/CGuiHandler.h"
+#include "../gui/Shortcut.h"
 #include "../gui/WindowHandler.h"
 #include "../render/Canvas.h"
 #include "../render/Colors.h"
@@ -32,6 +34,8 @@
 #include "../../lib/mapObjects/CGHeroInstance.h"
 #include "../../lib/mapObjects/CGTownInstance.h"
 
+#include "../../CCallback.h"
+
 CList::CListItem::CListItem(CList * Parent)
 	: CIntObject(LCLICK | SHOW_POPUP | HOVER),
 	parent(Parent),
@@ -229,7 +233,7 @@ CHeroList::CHeroItem::CHeroItem(CHeroList *parent, const CGHeroInstance * Hero)
 
 	update();
 
-	addUsedEvents(GESTURE);
+	addUsedEvents(GESTURE | KEYBOARD);
 }
 
 void CHeroList::CHeroItem::update()
@@ -300,6 +304,55 @@ void CHeroList::CHeroItem::gesture(bool on, const Point & initialPosition, const
 	GH.windows().createAndPushWindow<RadialMenu>(pos.center(), menuElements, true);
 }
 
+void CHeroList::CHeroItem::keyPressed(EShortcut key)
+{
+	if(!hero)
+		return;
+
+	if(parent->selected != this->shared_from_this())
+		return;
+
+	auto & heroes = LOCPLINT->localState->getWanderingHeroes();
+
+	if(key == EShortcut::LIST_HERO_DISMISS)
+	{
+		LOCPLINT->showYesNoDialog(CGI->generaltexth->allTexts[22], [=](){ LOCPLINT->cb->dismissHero(hero); }, nullptr);
+		return;
+	}
+
+	if(heroes.size() < 2)
+		return;
+
+	size_t heroPos = vstd::find_pos(heroes, hero);
+	const CGHeroInstance * heroUpper = (heroPos < 1) ? nullptr : heroes.at(heroPos - 1);
+	const CGHeroInstance * heroLower = (heroPos > heroes.size() - 2) ? nullptr : heroes.at(heroPos + 1);
+
+	switch(key)
+	{
+	case EShortcut::LIST_HERO_UP:
+		if(heroUpper)
+			LOCPLINT->localState->swapWanderingHero(heroPos, heroPos - 1);
+		break;
+
+	case EShortcut::LIST_HERO_DOWN:
+		if(heroLower)
+			LOCPLINT->localState->swapWanderingHero(heroPos, heroPos + 1);
+		break;
+
+	case EShortcut::LIST_HERO_TOP:
+		if(heroUpper)
+			for (size_t i = heroPos; i > 0; i--)
+				LOCPLINT->localState->swapWanderingHero(i, i - 1);
+		break;
+
+	case EShortcut::LIST_HERO_BOTTOM:
+		if(heroLower)
+			for (int i = heroPos; i < heroes.size() - 1; i++)
+				LOCPLINT->localState->swapWanderingHero(i, i + 1);
+		break;
+	}
+}
+
 std::shared_ptr<CIntObject> CHeroList::createItem(size_t index)
 {
 	if (LOCPLINT->localState->getWanderingHeroes().size() > index)
@@ -369,7 +422,7 @@ CTownList::CTownItem::CTownItem(CTownList *parent, const CGTownInstance *Town):
 	pos = picture->pos;
 	update();
 
-	addUsedEvents(GESTURE);
+	addUsedEvents(GESTURE | KEYBOARD);
 }
 
 std::shared_ptr<CIntObject> CTownList::CTownItem::genSelection()
@@ -418,24 +471,77 @@ void CTownList::CTownItem::gesture(bool on, const Point & initialPosition, const
 	int townUpperPos = (townIndex < 1) ? -1 : townIndex - 1;
 	int townLowerPos = (townIndex > towns.size() - 2) ? -1 : townIndex + 1;
 
+	auto updateList = [](){
+		for (auto ki : GH.windows().findWindows<CCastleInterface>())
+			ki->townChange(); //update list
+	};
+
 	std::vector<RadialMenuConfig> menuElements = {
-		{ RadialMenuConfig::ITEM_ALT_NN, townUpperPos > -1, "altUpTop", "vcmi.radialWheel.moveTop", [townIndex]()
+		{ RadialMenuConfig::ITEM_ALT_NN, townUpperPos > -1, "altUpTop", "vcmi.radialWheel.moveTop", [updateList, townIndex]()
 		{
 			for (int i = townIndex; i > 0; i--)
 				LOCPLINT->localState->swapOwnedTowns(i, i - 1);
+			updateList();
 		} },
-		{ RadialMenuConfig::ITEM_ALT_NW, townUpperPos > -1, "altUp", "vcmi.radialWheel.moveUp", [townIndex, townUpperPos](){LOCPLINT->localState->swapOwnedTowns(townIndex, townUpperPos); } },
+		{ RadialMenuConfig::ITEM_ALT_NW, townUpperPos > -1, "altUp", "vcmi.radialWheel.moveUp", [updateList, townIndex, townUpperPos](){LOCPLINT->localState->swapOwnedTowns(townIndex, townUpperPos); updateList(); } },
-		{ RadialMenuConfig::ITEM_ALT_SW, townLowerPos > -1, "altDown", "vcmi.radialWheel.moveDown", [townIndex, townLowerPos](){ LOCPLINT->localState->swapOwnedTowns(townIndex, townLowerPos); } },
+		{ RadialMenuConfig::ITEM_ALT_SW, townLowerPos > -1, "altDown", "vcmi.radialWheel.moveDown", [updateList, townIndex, townLowerPos](){ LOCPLINT->localState->swapOwnedTowns(townIndex, townLowerPos); updateList(); } },
-		{ RadialMenuConfig::ITEM_ALT_SS, townLowerPos > -1, "altDownBottom", "vcmi.radialWheel.moveBottom", [townIndex, towns]()
+		{ RadialMenuConfig::ITEM_ALT_SS, townLowerPos > -1, "altDownBottom", "vcmi.radialWheel.moveBottom", [updateList, townIndex, towns]()
 		{
 			for (int i = townIndex; i < towns.size() - 1; i++)
 				LOCPLINT->localState->swapOwnedTowns(i, i + 1);
+			updateList();
 		} },
 	};
 
 	GH.windows().createAndPushWindow<RadialMenu>(pos.center(), menuElements, true);
 }
 
+void CTownList::CTownItem::keyPressed(EShortcut key)
+{
+	if(parent->selected != this->shared_from_this())
+		return;
+
+	const std::vector<const CGTownInstance *> towns = LOCPLINT->localState->getOwnedTowns();
+	size_t townIndex = vstd::find_pos(towns, town);
+
+	if(townIndex + 1 > towns.size() || !towns.at(townIndex))
+		return;
+
+	if(towns.size() < 2)
+		return;
+
+	int townUpperPos = (townIndex < 1) ? -1 : townIndex - 1;
+	int townLowerPos = (townIndex > towns.size() - 2) ? -1 : townIndex + 1;
+
+	switch(key)
+	{
+	case EShortcut::LIST_TOWN_UP:
+		if(townUpperPos > -1)
+			LOCPLINT->localState->swapOwnedTowns(townIndex, townUpperPos);
+		break;
+
+	case EShortcut::LIST_TOWN_DOWN:
+		if(townLowerPos > -1)
+			LOCPLINT->localState->swapOwnedTowns(townIndex, townLowerPos);
+		break;
+
+	case EShortcut::LIST_TOWN_TOP:
+		if(townUpperPos > -1)
+			for (int i = townIndex; i > 0; i--)
+				LOCPLINT->localState->swapOwnedTowns(i, i - 1);
+		break;
+
+	case EShortcut::LIST_TOWN_BOTTOM:
+		if(townLowerPos > -1)
+			for (int i = townIndex; i < towns.size() - 1; i++)
+				LOCPLINT->localState->swapOwnedTowns(i, i + 1);
+		break;
+	}
+
+	for (auto ki : GH.windows().findWindows<CCastleInterface>())
+		ki->townChange(); //update list
+}
+
 std::string CTownList::CTownItem::getHoverText()
 {
 	return town->getObjectName();

client/adventureMap/CList.h

@@ -29,9 +29,10 @@ class CList : public Scrollable
 protected:
 	class CListItem : public CIntObject, public std::enable_shared_from_this<CListItem>
 	{
-		CList * parent;
 		std::shared_ptr<CIntObject> selection;
 	public:
+		CList * parent;
+
 		CListItem(CList * parent);
 		~CListItem();
 
@@ -55,9 +56,6 @@ protected:
 
 private:
 	const size_t size;
-
-	//for selection\deselection
-	std::shared_ptr<CListItem> selected;
 	void select(std::shared_ptr<CListItem> which);
 	friend class CListItem;
 
@@ -81,6 +79,9 @@ protected:
 	void update();
 
 public:
+	//for selection\deselection
+	std::shared_ptr<CListItem> selected;
+
 	/// functions that will be called when selection changes
 	CFunctionList<void()> onSelect;
 
@@ -128,6 +129,7 @@ class CHeroList	: public CList
 		void open() override;
 		void showTooltip() override;
 		void gesture(bool on, const Point & initialPosition, const Point & finalPosition) override;
+		void keyPressed(EShortcut key) override;
 		std::string getHoverText() override;
 	};
 
@@ -162,6 +164,7 @@ class CTownList	: public CList
 		void open() override;
 		void showTooltip() override;
 		void gesture(bool on, const Point & initialPosition, const Point & finalPosition) override;
+		void keyPressed(EShortcut key) override;
 		std::string getHoverText() override;
 	};
 

client/battle/BattleActionsController.cpp

@@ -312,8 +312,8 @@ void BattleActionsController::castThisSpell(SpellID spellID)
 	heroSpellToCast = std::make_shared<BattleAction>();
 	heroSpellToCast->actionType = EActionType::HERO_SPELL;
 	heroSpellToCast->spell = spellID;
-	heroSpellToCast->stackNumber = (owner.attackingHeroInstance->tempOwner == owner.curInt->playerID) ? -1 : -2;
+	heroSpellToCast->stackNumber = -1;
-	heroSpellToCast->side = owner.defendingHeroInstance ? (owner.curInt->playerID == owner.defendingHeroInstance->tempOwner) : false;
+	heroSpellToCast->side = owner.curInt->cb->getBattle(owner.getBattleID())->battleGetMySide();
 
 	//choosing possible targets
 	const CGHeroInstance *castingHero = (owner.attackingHeroInstance->tempOwner == owner.curInt->playerID) ? owner.attackingHeroInstance : owner.defendingHeroInstance;

client/battle/BattleInterface.cpp

@@ -229,19 +229,19 @@ void BattleInterface::stacksAreAttacked(std::vector<StackAttackedInfo> attackedI
 {
 	stacksController->stacksAreAttacked(attackedInfos);
 
-	std::array<int, 2> killedBySide = {0, 0};
+	BattleSideArray<int> killedBySide;
 
 	for(const StackAttackedInfo & attackedInfo : attackedInfos)
 	{
-		ui8 side = attackedInfo.defender->unitSide();
+		BattleSide side = attackedInfo.defender->unitSide();
 		killedBySide.at(side) += attackedInfo.amountKilled;
 	}
 
-	for(ui8 side = 0; side < 2; side++)
+	for(BattleSide side : { BattleSide::ATTACKER, BattleSide::DEFENDER })
 	{
-		if(killedBySide.at(side) > killedBySide.at(1-side))
+		if(killedBySide.at(side) > killedBySide.at(getBattle()->otherSide(side)))
 			setHeroAnimation(side, EHeroAnimType::DEFEAT);
-		else if(killedBySide.at(side) < killedBySide.at(1-side))
+		else if(killedBySide.at(side) < killedBySide.at(getBattle()->otherSide(side)))
 			setHeroAnimation(side, EHeroAnimType::VICTORY);
 	}
 }
@@ -271,14 +271,14 @@ void BattleInterface::giveCommand(EActionType action, BattleHex tile, SpellID sp
 	}
 
 	auto side = getBattle()->playerToSide(curInt->playerID);
-	if(!side)
+	if(side == BattleSide::NONE)
 	{
 		logGlobal->error("Player %s is not in battle", curInt->playerID.toString());
 		return;
 	}
 
 	BattleAction ba;
-	ba.side = side.value();
+	ba.side = side;
 	ba.actionType = action;
 	ba.aimToHex(tile);
 	ba.spell = spell;
@@ -409,7 +409,7 @@ void BattleInterface::spellCast(const BattleSpellCast * sc)
 		}
 		else
 		{
-			auto hero = sc->side ? defendingHero : attackingHero;
+			auto hero = sc->side == BattleSide::DEFENDER ? defendingHero : attackingHero;
 			assert(hero);
 
 			addToAnimationStage(EAnimationEvents::BEFORE_HIT, [=]()
@@ -466,11 +466,11 @@ void BattleInterface::spellCast(const BattleSpellCast * sc)
 	{
 		Point leftHero = Point(15, 30);
 		Point rightHero = Point(755, 30);
-		bool side = sc->side;
+		BattleSide side = sc->side;
 
 		addToAnimationStage(EAnimationEvents::AFTER_HIT, [=](){
-			stacksController->addNewAnim(new EffectAnimation(*this, AnimationPath::builtin(side ? "SP07_A.DEF" : "SP07_B.DEF"), leftHero));
+			stacksController->addNewAnim(new EffectAnimation(*this, AnimationPath::builtin(side == BattleSide::DEFENDER ? "SP07_A.DEF" : "SP07_B.DEF"), leftHero));
-			stacksController->addNewAnim(new EffectAnimation(*this, AnimationPath::builtin(side ? "SP07_B.DEF" : "SP07_A.DEF"), rightHero));
+			stacksController->addNewAnim(new EffectAnimation(*this, AnimationPath::builtin(side == BattleSide::DEFENDER ? "SP07_B.DEF" : "SP07_A.DEF"), rightHero));
 		});
 	}
 
@@ -483,7 +483,7 @@ void BattleInterface::battleStacksEffectsSet(const SetStackEffect & sse)
 		fieldController->redrawBackgroundWithHexes();
 }
 
-void BattleInterface::setHeroAnimation(ui8 side, EHeroAnimType phase)
+void BattleInterface::setHeroAnimation(BattleSide side, EHeroAnimType phase)
 {
 	if(side == BattleSide::ATTACKER)
 	{
@@ -656,7 +656,7 @@ void BattleInterface::tacticPhaseEnd()
 	tacticsMode = false;
 
 	auto side = tacticianInterface->cb->getBattle(battleID)->playerToSide(tacticianInterface->playerID);
-	auto action = BattleAction::makeEndOFTacticPhase(*side);
+	auto action = BattleAction::makeEndOFTacticPhase(side);
 
 	tacticianInterface->cb->battleMakeTacticAction(battleID, action);
 }

client/battle/BattleInterface.h

@@ -170,7 +170,7 @@ public:
 
 	void showInterface(Canvas & to);
 
-	void setHeroAnimation(ui8 side, EHeroAnimType phase);
+	void setHeroAnimation(BattleSide side, EHeroAnimType phase);
 
 	void executeSpellCast(); //called when a hero casts a spell
 

client/battle/BattleInterfaceClasses.cpp

@@ -433,7 +433,7 @@ QuickSpellPanel::QuickSpellPanel(BattleInterface & owner)
 	create();
 }
 
-std::vector<std::tuple<SpellID, bool>> QuickSpellPanel::getSpells()
+std::vector<std::tuple<SpellID, bool>> QuickSpellPanel::getSpells() const
 {
 	std::vector<SpellID> spellIds;
 	std::vector<bool> spellIdsFromSetting;
@@ -746,7 +746,7 @@ BattleResultWindow::BattleResultWindow(const BattleResult & br, CPlayerInterface
 		labels.push_back(std::make_shared<CLabel>(232, 520, FONT_MEDIUM, ETextAlignment::CENTER, Colors::WHITE, CGI->generaltexth->translate("vcmi.battleResultsWindow.applyResultsLabel")));
 	}
 
-	if(br.winner == 0) //attacker won
+	if(br.winner == BattleSide::ATTACKER)
 	{
 		labels.push_back(std::make_shared<CLabel>(59, 124, FONT_SMALL, ETextAlignment::CENTER, Colors::WHITE, CGI->generaltexth->allTexts[410]));
 	}
@@ -754,8 +754,8 @@ BattleResultWindow::BattleResultWindow(const BattleResult & br, CPlayerInterface
 	{
 		labels.push_back(std::make_shared<CLabel>(59, 124, FONT_SMALL, ETextAlignment::CENTER, Colors::WHITE, CGI->generaltexth->allTexts[411]));
 	}
-
+	
-	if(br.winner == 1)
+	if(br.winner == BattleSide::DEFENDER)
 	{
 		labels.push_back(std::make_shared<CLabel>(412, 124, FONT_SMALL, ETextAlignment::CENTER, Colors::WHITE, CGI->generaltexth->allTexts[410]));
 	}
@@ -770,15 +770,15 @@ BattleResultWindow::BattleResultWindow(const BattleResult & br, CPlayerInterface
 
 	std::string sideNames[2] = {"N/A", "N/A"};
 
-	for(int i = 0; i < 2; i++)
+	for(auto i : {BattleSide::ATTACKER, BattleSide::DEFENDER})
 	{
 		auto heroInfo = owner.cb->getBattle(br.battleID)->battleGetHeroInfo(i);
 		const int xs[] = {21, 392};
 
 		if(heroInfo.portraitSource.isValid()) //attacking hero
 		{
-			icons.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), heroInfo.getIconIndex(), 0, xs[i], 38));
+			icons.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), heroInfo.getIconIndex(), 0, xs[static_cast<int>(i)], 38));
-			sideNames[i] = heroInfo.name;
+			sideNames[static_cast<int>(i)] = heroInfo.name;
 		}
 		else
 		{
@@ -795,8 +795,8 @@ BattleResultWindow::BattleResultWindow(const BattleResult & br, CPlayerInterface
 
 			if(best != stacks.end()) //should be always but to be safe...
 			{
-				icons.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("TWCRPORT"), (*best)->unitType()->getIconIndex(), 0, xs[i], 38));
+				icons.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("TWCRPORT"), (*best)->unitType()->getIconIndex(), 0, xs[static_cast<int>(i)], 38));
-				sideNames[i] = (*best)->unitType()->getNamePluralTranslated();
+				sideNames[static_cast<int>(i)] = (*best)->unitType()->getNamePluralTranslated();
 			}
 		}
 	}
@@ -806,16 +806,16 @@ BattleResultWindow::BattleResultWindow(const BattleResult & br, CPlayerInterface
 	labels.push_back(std::make_shared<CLabel>(381, 53, FONT_SMALL, ETextAlignment::BOTTOMRIGHT, Colors::WHITE, sideNames[1]));
 
 	//printing casualties
-	for(int step = 0; step < 2; ++step)
+	for(auto step : {BattleSide::ATTACKER, BattleSide::DEFENDER})
 	{
 		if(br.casualties[step].size()==0)
 		{
-			labels.push_back(std::make_shared<CLabel>(235, 360 + 97 * step, FONT_SMALL, ETextAlignment::CENTER, Colors::WHITE, CGI->generaltexth->allTexts[523]));
+			labels.push_back(std::make_shared<CLabel>(235, 360 + 97 * static_cast<int>(step), FONT_SMALL, ETextAlignment::CENTER, Colors::WHITE, CGI->generaltexth->allTexts[523]));
 		}
 		else
 		{
 			int xPos = 235 - ((int)br.casualties[step].size()*32 + ((int)br.casualties[step].size() - 1)*10)/2; //increment by 42 with each picture
-			int yPos = 344 + step * 97;
+			int yPos = 344 + static_cast<int>(step) * 97;
 			for(auto & elem : br.casualties[step])
 			{
 				auto creature = CGI->creatures()->getByIndex(elem.first);
@@ -842,9 +842,9 @@ BattleResultWindow::BattleResultWindow(const BattleResult & br, CPlayerInterface
 BattleResultResources BattleResultWindow::getResources(const BattleResult & br)
 {
 	//printing result description
-	bool weAreAttacker = !(owner.cb->getBattle(br.battleID)->battleGetMySide());
+	bool weAreAttacker = owner.cb->getBattle(br.battleID)->battleGetMySide() == BattleSide::ATTACKER;
 	bool weAreDefender = !weAreAttacker;
-	bool weWon = (br.winner == 0 && weAreAttacker) || (br.winner == 1 && !weAreAttacker);
+	bool weWon = (br.winner == BattleSide::ATTACKER && weAreAttacker) || (br.winner == BattleSide::DEFENDER && !weAreAttacker);
 	bool isSiege = owner.cb->getBattle(br.battleID)->battleGetDefendedTown() != nullptr;
 
 	BattleResultResources resources;
@@ -884,7 +884,7 @@ BattleResultResources BattleResultWindow::getResources(const BattleResult & br)
 		{
 			resources.resultText.appendTextID("core.genrltxt.305");
 			resources.resultText.replaceTextID(ourHero->getNameTranslated());
-			resources.resultText.replaceNumber(br.exp[weAreAttacker ? 0 : 1]);
+			resources.resultText.replaceNumber(br.exp[weAreAttacker ? BattleSide::ATTACKER : BattleSide::DEFENDER]);
 		}
 	}
 	else // we lose

client/battle/BattleInterfaceClasses.h

@@ -169,7 +169,7 @@ public:
 
 	void create();
 
-	std::vector<std::tuple<SpellID, bool>> getSpells();
+	std::vector<std::tuple<SpellID, bool>> getSpells() const;
 
 	void show(Canvas & to) override;
 	void inputModeChanged(InputMode modi) override;

client/battle/BattleObstacleController.cpp

@@ -102,7 +102,7 @@ void BattleObstacleController::obstaclePlaced(const std::vector<std::shared_ptr<
 	{
 		auto side = owner.getBattle()->playerToSide(owner.curInt->playerID);
 
-		if(!oi->visibleForSide(side.value(), owner.getBattle()->battleHasNativeStack(side.value())))
+		if(!oi->visibleForSide(side, owner.getBattle()->battleHasNativeStack(side)))
 			continue;
 
 		auto animation = GH.renderHandler().loadAnimation(oi->getAppearAnimation(), EImageBlitMode::ALPHA);

client/battle/BattleOverlayLogVisualizer.cpp

@@ -27,7 +27,7 @@ BattleOverlayLogVisualizer::BattleOverlayLogVisualizer(
 {
 }
 
-void BattleOverlayLogVisualizer::drawText(BattleHex hex, int lineNumber, std::string text)
+void BattleOverlayLogVisualizer::drawText(BattleHex hex, int lineNumber, const std::string & text)
 {
 	Point offset = owner.fieldController->hexPositionLocal(hex).topLeft() + Point(20, 20);
 	int h = graphics->fonts[EFonts::FONT_TINY]->getLineHeight();

client/battle/BattleOverlayLogVisualizer.h

@@ -24,5 +24,5 @@ private:
 public:
 	BattleOverlayLogVisualizer(BattleRenderer::RendererRef & target, BattleInterface & owner);
 
-	void drawText(BattleHex hex, int lineNumber, std::string text) override;
+	void drawText(BattleHex hex, int lineNumber, const std::string & text) override;
 };

client/gui/CIntObject.h

@@ -212,7 +212,7 @@ class EmptyStatusBar : public IStatusBar
 	virtual void setEnteredText(const std::string & text){};
 };
 
-class ObjectConstruction
+class ObjectConstruction : boost::noncopyable
 {
 public:
 	ObjectConstruction(CIntObject *obj);

client/gui/Shortcut.h

@@ -294,5 +294,15 @@ enum class EShortcut
 	SPELLBOOK_TAB_ADVENTURE,
 	SPELLBOOK_TAB_COMBAT,
 
+	LIST_HERO_UP,
+	LIST_HERO_DOWN,
+	LIST_HERO_TOP,
+	LIST_HERO_BOTTOM,
+	LIST_HERO_DISMISS,
+	LIST_TOWN_UP,
+	LIST_TOWN_DOWN,
+	LIST_TOWN_TOP,
+	LIST_TOWN_BOTTOM,
+
 	AFTER_LAST
 };

client/gui/ShortcutHandler.cpp

@@ -275,6 +275,15 @@ EShortcut ShortcutHandler::findShortcut(const std::string & identifier ) const
 		{"heroCostumeLoad9",         EShortcut::HERO_COSTUME_LOAD_9       },
 		{"spellbookTabAdventure",    EShortcut::SPELLBOOK_TAB_ADVENTURE   },
 		{"spellbookTabCombat",       EShortcut::SPELLBOOK_TAB_COMBAT      },
+		{"listHeroUp",               EShortcut::LIST_HERO_UP              },
+		{"listHeroDown",             EShortcut::LIST_HERO_DOWN            },
+		{"listHeroTop",              EShortcut::LIST_HERO_TOP             },
+		{"listHeroBottom",           EShortcut::LIST_HERO_BOTTOM          },
+		{"listHeroDismiss",          EShortcut::LIST_HERO_DISMISS         },
+		{"listTownUp",               EShortcut::LIST_TOWN_UP              },
+		{"listTownDown",             EShortcut::LIST_TOWN_DOWN            },
+		{"listTownTop",              EShortcut::LIST_TOWN_TOP             },
+		{"listTownBottom",           EShortcut::LIST_TOWN_BOTTOM          },
 		{"mainMenuHotseat",          EShortcut::MAIN_MENU_HOTSEAT         },
 		{"mainMenuHostGame",         EShortcut::MAIN_MENU_HOST_GAME       },
 		{"mainMenuJoinGame",         EShortcut::MAIN_MENU_JOIN_GAME       },

client/lobby/OptionsTab.cpp

@@ -494,7 +494,7 @@ void OptionsTab::SelectionWindow::reopen()
 	if(type == SelType::HERO && SEL->getStartInfo()->playerInfos.find(color)->second.castle == FactionID::RANDOM)
 		close();
 	else{
-		auto window = std::shared_ptr<SelectionWindow>(new SelectionWindow(color, type, slider ? slider->getValue() : 0));
+		auto window = std::make_shared<SelectionWindow>(color, type, slider ? slider->getValue() : 0);
 		close();
 		if(CSH->isMyColor(color) || CSH->isHost())
 			GH.windows().pushWindow(window);

client/mainmenu/CMainMenu.cpp

@@ -475,7 +475,7 @@ void CMultiMode::joinTCP()
 	GH.windows().createAndPushWindow<CMultiPlayers>(getPlayersNames(), savedScreenType, false, ELoadMode::MULTI);
 }
 
-const std::vector<std::string> CMultiMode::getPlayersNames()
+std::vector<std::string> CMultiMode::getPlayersNames()
 {
 	std::vector<std::string> playerNames;
 

client/mainmenu/CMainMenu.h

@@ -97,7 +97,7 @@ public:
 	void joinTCP();
 
 	/// Get all configured player names. The first name would always be present and initialized to its default value.
-	const std::vector<std::string> getPlayersNames();
+	std::vector<std::string> getPlayersNames();
 
 	void onNameChange(std::string newText);
 };

client/mapView/MapOverlayLogVisualizer.cpp

@@ -54,8 +54,8 @@ void MapOverlayLogVisualizer::drawLine(int3 start, int3 end)
 void MapOverlayLogVisualizer::drawText(
 	int3 tile,
 	int lineNumber,
-	std::string text,
+	const std::string & text,
-	std::optional<ColorRGBA> background)
+	const std::optional<ColorRGBA> & background)
 {
 	const Point offset = Point(6, 6);
 

client/mapView/MapOverlayLogVisualizer.h

@@ -29,5 +29,5 @@ private:
 public:
 	MapOverlayLogVisualizer(Canvas & target, std::shared_ptr<MapViewModel> model);
 	void drawLine(int3 start, int3 end) override;
-	void drawText(int3 tile, int lineNumber, std::string text, std::optional<ColorRGBA> color) override;
+	void drawText(int3 tile, int lineNumber, const std::string & text, const std::optional<ColorRGBA> & color) override;
 };

client/mapView/MapRendererContext.cpp

@@ -291,7 +291,7 @@ ColorRGBA MapRendererAdventureContext::overlayTextColor(const int3 & coordinates
 	if (!tile.visitable)
 		return {};
 
-	auto * object = tile.visitableObjects.back();
+	const auto * object = tile.visitableObjects.back();
 
 	if (object->getOwner() == LOCPLINT->playerID)
 		return { 0, 192, 0};

client/widgets/Images.cpp

@@ -188,17 +188,6 @@ CAnimImage::CAnimImage(const AnimationPath & name, size_t Frame, size_t Group, i
 	init();
 }
 
-//CAnimImage::CAnimImage(std::shared_ptr<CAnimation> Anim, size_t Frame, size_t Group, int x, int y, ui8 Flags):
-//	anim(Anim),
-//	frame(Frame),
-//	group(Group),
-//	flags(Flags)
-//{
-//	pos.x += x;
-//	pos.y += y;
-//	init();
-//}
-
 CAnimImage::CAnimImage(const AnimationPath & name, size_t Frame, Rect targetPos, size_t Group, ui8 Flags):
 	anim(GH.renderHandler().loadAnimation(name, EImageBlitMode::COLORKEY)),
 	frame(Frame),

client/widgets/Images.h

@@ -114,7 +114,6 @@ public:
 	bool visible;
 
 	CAnimImage(const AnimationPath & name, size_t Frame, size_t Group=0, int x=0, int y=0, ui8 Flags=0);
-//	CAnimImage(std::shared_ptr<CAnimation> Anim, size_t Frame, size_t Group=0, int x=0, int y=0, ui8 Flags=0);
 	CAnimImage(const AnimationPath & name, size_t Frame, Rect targetPos, size_t Group=0, ui8 Flags=0);
 	~CAnimImage();
 

client/windows/CCastleInterface.cpp

@@ -163,7 +163,7 @@ void CBuildingRect::showPopupWindow(const Point & cursorPosition)
 	}
 	else
 	{
-		int level = ( bid - BuildingID::DWELL_FIRST ) % GameConstants::CREATURES_PER_TOWN;
+		int level = BuildingID::getLevelFromDwelling(bid);
 		GH.windows().createAndPushWindow<CDwellingInfoBox>(parent->pos.x+parent->pos.w / 2, parent->pos.y+parent->pos.h  /2, town, level);
 	}
 }
@@ -237,7 +237,7 @@ std::string CBuildingRect::getSubtitle()//hover text for building
 		return town->town->buildings.at(getBuilding()->bid)->getNameTranslated();
 	else//dwellings - recruit %creature%
 	{
-		auto & availableCreatures = town->creatures[(bid-30)%GameConstants::CREATURES_PER_TOWN].second;
+		auto & availableCreatures = town->creatures[(bid-30)%town->town->creatures.size()].second;
 		if(availableCreatures.size())
 		{
 			int creaID = availableCreatures.back();//taking last of available creatures
@@ -688,7 +688,7 @@ void CCastleBuildings::buildingClicked(BuildingID building, BuildingSubID::EBuil
 
 	if (building >= BuildingID::DWELL_FIRST)
 	{
-		enterDwelling((building-BuildingID::DWELL_FIRST)%GameConstants::CREATURES_PER_TOWN);
+		enterDwelling((BuildingID::getLevelFromDwelling(building)));
 	}
 	else
 	{
@@ -800,10 +800,10 @@ void CCastleBuildings::buildingClicked(BuildingID building, BuildingSubID::EBuil
 						break;
 
 				case BuildingSubID::PORTAL_OF_SUMMONING:
-						if (town->creatures[GameConstants::CREATURES_PER_TOWN].second.empty())//No creatures
+						if (town->creatures[town->town->creatures.size()].second.empty())//No creatures
 							LOCPLINT->showInfoDialog(CGI->generaltexth->tcommands[30]);
 						else
-							enterDwelling(GameConstants::CREATURES_PER_TOWN);
+							enterDwelling(town->town->creatures.size());
 						break;
 
 				case BuildingSubID::BALLISTA_YARD:
@@ -921,8 +921,8 @@ void CCastleBuildings::enterDwelling(int level)
 void CCastleBuildings::enterToTheQuickRecruitmentWindow()
 {
 	const auto beginIt = town->creatures.cbegin();
-	const auto afterLastIt = town->creatures.size() > GameConstants::CREATURES_PER_TOWN
+	const auto afterLastIt = town->creatures.size() > town->town->creatures.size()
-		? std::next(beginIt, GameConstants::CREATURES_PER_TOWN)
+		? std::next(beginIt, town->town->creatures.size())
 		: town->creatures.cend();
 	const auto hasSomeoneToRecruit = std::any_of(beginIt, afterLastIt,
 		[](const auto & creatureInfo) { return creatureInfo.first > 0; });
@@ -1759,7 +1759,7 @@ CFortScreen::CFortScreen(const CGTownInstance * town):
 {
 	OBJECT_CONSTRUCTION;
 	ui32 fortSize = static_cast<ui32>(town->creatures.size());
-	if(fortSize > GameConstants::CREATURES_PER_TOWN && town->creatures.back().second.empty())
+	if(fortSize > town->town->creatures.size() && town->creatures.back().second.empty())
 		fortSize--;
 
 	const CBuilding * fortBuilding = town->town->buildings.at(BuildingID(town->fortLevel()+6));
@@ -1777,25 +1777,25 @@ CFortScreen::CFortScreen(const CGTownInstance * town):
 
 	if(fortSize == GameConstants::CREATURES_PER_TOWN)
 	{
-		positions.push_back(Point(206,421));
+		positions.push_back(Point(10, 421));
+		positions.push_back(Point(404,421));
 	}
 	else
 	{
-		positions.push_back(Point(10, 421));
+		positions.push_back(Point(206,421));
-		positions.push_back(Point(404,421));
 	}
 
 	for(ui32 i=0; i<fortSize; i++)
 	{
 		BuildingID buildingID;
-		if(fortSize == GameConstants::CREATURES_PER_TOWN)
+		if(fortSize == town->town->creatures.size())
 		{
-			BuildingID dwelling = BuildingID::DWELL_UP_FIRST+i;
+			BuildingID dwelling = BuildingID::getDwellingFromLevel(i, 1);
 
 			if(vstd::contains(town->builtBuildings, dwelling))
-				buildingID = BuildingID(BuildingID::DWELL_UP_FIRST+i);
+				buildingID = BuildingID(BuildingID::getDwellingFromLevel(i, 1));
 			else
-				buildingID = BuildingID(BuildingID::DWELL_FIRST+i);
+				buildingID = BuildingID(BuildingID::getDwellingFromLevel(i, 0));
 		}
 		else
 		{
@@ -1817,13 +1817,13 @@ CFortScreen::CFortScreen(const CGTownInstance * town):
 ImagePath CFortScreen::getBgName(const CGTownInstance * town)
 {
 	ui32 fortSize = static_cast<ui32>(town->creatures.size());
-	if(fortSize > GameConstants::CREATURES_PER_TOWN && town->creatures.back().second.empty())
+	if(fortSize > town->town->creatures.size() && town->creatures.back().second.empty())
 		fortSize--;
 
 	if(fortSize == GameConstants::CREATURES_PER_TOWN)
-		return ImagePath::builtin("TPCASTL7");
-	else
 		return ImagePath::builtin("TPCASTL8");
+	else
+		return ImagePath::builtin("TPCASTL7");
 }
 
 void CFortScreen::creaturesChangedEventHandler()
@@ -1897,9 +1897,9 @@ const CCreature * CFortScreen::RecruitArea::getMyCreature()
 
 const CBuilding * CFortScreen::RecruitArea::getMyBuilding()
 {
-	BuildingID myID = BuildingID(BuildingID::DWELL_FIRST + level);
+	BuildingID myID = BuildingID(BuildingID::getDwellingFromLevel(level, 0));
 
-	if (level == GameConstants::CREATURES_PER_TOWN)
+	if (level == town->town->creatures.size())
 		return town->town->getSpecialBuilding(BuildingSubID::PORTAL_OF_SUMMONING);
 
 	if (!town->town->buildings.count(myID))
@@ -1910,7 +1910,7 @@ const CBuilding * CFortScreen::RecruitArea::getMyBuilding()
 	{
 		if (town->hasBuilt(myID))
 			build = town->town->buildings.at(myID);
-		myID.advance(GameConstants::CREATURES_PER_TOWN);
+		myID.advance(town->town->creatures.size());
 	}
 	return build;
 }

client/windows/CMapOverview.cpp

@@ -41,7 +41,7 @@
 #include "../../lib/texts/CGeneralTextHandler.h"
 #include "../../lib/texts/TextOperations.h"
 
-CMapOverview::CMapOverview(std::string mapName, std::string fileName, std::string date, std::string author, std::string version, ResourcePath resource, ESelectionScreen tabType)
+CMapOverview::CMapOverview(const std::string & mapName, const std::string & fileName, const std::string & date, const std::string & author, const std::string & version, const ResourcePath & resource, ESelectionScreen tabType)
 	: CWindowObject(BORDERED | RCLICK_POPUP), resource(resource), mapName(mapName), fileName(fileName), date(date), author(author), version(version), tabType(tabType)
 {
 

client/windows/CMapOverview.h

@@ -57,5 +57,5 @@ public:
 	const std::string version;
 	const ESelectionScreen tabType;
 
-	CMapOverview(std::string mapName, std::string fileName, std::string date, std::string author, std::string version, ResourcePath resource, ESelectionScreen tabType);
+	CMapOverview(const std::string & mapName, const std::string & fileName, const std::string & date, const std::string & author, const std::string & version, const ResourcePath & resource, ESelectionScreen tabType);
 };

client/windows/CPuzzleWindow.cpp

@@ -54,7 +54,7 @@ CPuzzleWindow::CPuzzleWindow(const int3 & GrailPos, double discoveredRatio)
 	{
 		const SPuzzleInfo & info = elem;
 
-		auto piece = std::make_shared<CPicture>(info.filename, info.x, info.y);
+		auto piece = std::make_shared<CPicture>(info.filename, info.position.x, info.position.y);
 
 		//piece that will slowly disappear
 		if(info.whenUncovered <= GameConstants::PUZZLE_MAP_PIECES * discoveredRatio)

client/windows/CSpellWindow.cpp

@@ -98,7 +98,7 @@ public:
 	}
 };
 
-CSpellWindow::CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _myInt, bool openOnBattleSpells, std::function<void(SpellID)> onSpellSelect):
+CSpellWindow::CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _myInt, bool openOnBattleSpells, const std::function<void(SpellID)> & onSpellSelect):
 	CWindowObject(PLAYER_COLORED | (settings["gameTweaks"]["enableLargeSpellbook"].Bool() ? BORDERED : 0)),
 	battleSpellsOnly(openOnBattleSpells),
 	selectedTab(4),

client/windows/CSpellWindow.h

@@ -119,7 +119,7 @@ class CSpellWindow : public CWindowObject
 	std::function<void(SpellID)> onSpellSelect; //external processing of selected spell
 
 public:
-	CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _myInt, bool openOnBattleSpells = true, std::function<void(SpellID)> onSpellSelect = nullptr);
+	CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _myInt, bool openOnBattleSpells = true, const std::function<void(SpellID)> & onSpellSelect = nullptr);
 	~CSpellWindow();
 
 	void fexitb();

client/windows/QuickRecruitmentWindow.cpp

@@ -51,7 +51,7 @@ void QuickRecruitmentWindow::setCreaturePurchaseCards()
 {
 	int availableAmount = getAvailableCreatures();
 	Point position = Point((pos.w - 100*availableAmount - 8*(availableAmount-1))/2,64);
-	for (int i = 0; i < GameConstants::CREATURES_PER_TOWN; i++)
+	for (int i = 0; i < town->town->creatures.size(); i++)
 	{
 		if(!town->town->creatures.at(i).empty() && !town->creatures.at(i).second.empty() && town->creatures[i].first)
 		{
@@ -106,7 +106,16 @@ void QuickRecruitmentWindow::purchaseUnits()
 	{
 		if(selected->slider->getValue())
 		{
-			auto onRecruit = [=](CreatureID id, int count){ LOCPLINT->cb->recruitCreatures(town, town->getUpperArmy(), id, count, selected->creatureOnTheCard->getLevel()-1); };
+			int level = 0;
+			int i = 0;
+			for(auto c : town->town->creatures)
+			{
+				for(auto c2 : c)
+					if(c2 == selected->creatureOnTheCard->getId())
+						level = i;
+				i++;
+			}
+			auto onRecruit = [=](CreatureID id, int count){ LOCPLINT->cb->recruitCreatures(town, town->getUpperArmy(), id, count, level); };
 			CreatureID crid =  selected->creatureOnTheCard->getId();
 			SlotID dstslot = town -> getSlotFor(crid);
 			if(!dstslot.validSlot())
@@ -120,7 +129,7 @@ void QuickRecruitmentWindow::purchaseUnits()
 int QuickRecruitmentWindow::getAvailableCreatures()
 {
 	int creaturesAmount = 0;
-	for (int i=0; i< GameConstants::CREATURES_PER_TOWN; i++)
+	for (int i=0; i< town->town->creatures.size(); i++)
 		if(!town->town->creatures.at(i).empty() && !town->creatures.at(i).second.empty() && town->creatures[i].first)
 			creaturesAmount++;
 	return creaturesAmount;

config/campaign_regions.json

@@ -2,7 +2,7 @@
 	"campaign_regions": [
 		{
 			"prefix": "G1",
-			"color_suffix_length": 1,
+			"colorSuffixLength": 1,
 			"desc": [
 				{ "infix": "A", "x": 57, "y": 314 },
 				{ "infix": "B", "x": 137, "y": 309 },
@@ -12,7 +12,7 @@
 
 		{
 			"prefix": "G2",
-			"color_suffix_length": 1,
+			"colorSuffixLength": 1,
 			"desc": [
 				{ "infix": "A", "x": 56, "y": 90 },
 				{ "infix": "B", "x": 316, "y": 49 },
@@ -23,7 +23,7 @@
 
 		{
 			"prefix": "G3",
-			"color_suffix_length": 1,
+			"colorSuffixLength": 1,
 			"desc": [
 				{ "infix": "A", "x": 289, "y": 376 },
 				{ "infix": "B", "x": 60, "y": 147 },
@@ -33,7 +33,7 @@
 
 		{
 			"prefix": "E1",
-			"color_suffix_length": 1,
+			"colorSuffixLength": 1,
 			"desc": [
 				{ "infix": "A", "x": 270, "y": 332 },
 				{ "infix": "B", "x": 138, "y": 113 },
@@ -47,7 +47,7 @@
 
 		{
 			"prefix": "E2",
-			"color_suffix_length": 1,
+			"colorSuffixLength": 1,
 			"desc": [
 				{ "infix": "A", "x": 131, "y": 202 },
 				{ "infix": "B", "x": 60, "y": 145 },
@@ -58,7 +58,7 @@
 
 		{
 			"prefix": "N1",
-			"color_suffix_length": 1,
+			"colorSuffixLength": 1,
 			"desc": [
 				{ "infix": "A", "x": 42, "y": 94 },
 				{ "infix": "B", "x": 309, "y": 290 },
@@ -68,7 +68,7 @@
 
 		{
 			"prefix": "S1",
-			"color_suffix_length": 1,
+			"colorSuffixLength": 1,
 			"desc": [
 				{ "infix": "A", "x": 263, "y": 199 },
 				{ "infix": "B", "x": 182, "y": 210 },
@@ -78,7 +78,7 @@
 
 		{
 			"prefix": "BR",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 18, "y": 233 },
 				{ "infix": "B", "x": 125, "y": 381 },
@@ -89,7 +89,7 @@
 
 		{
 			"prefix": "IS",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 294, "y": 399 },
 				{ "infix": "B", "x": 183, "y": 293 },
@@ -100,7 +100,7 @@
 
 		{
 			"prefix": "KR",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 148, "y": 323 },
 				{ "infix": "B", "x": 192, "y": 235 },
@@ -111,7 +111,7 @@
 
 		{
 			"prefix": "NI",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 118, "y": 111 },
 				{ "infix": "B", "x": 223, "y": 145 },
@@ -122,7 +122,7 @@
 
 		{
 			"prefix": "TA",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 228, "y": 233 },
 				{ "infix": "B", "x": 147, "y": 194 },
@@ -132,7 +132,7 @@
 
 		{
 			"prefix": "AR",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 135, "y": 238 },
 				{ "infix": "B", "x": 135, "y": 121 },
@@ -147,7 +147,7 @@
 
 		{
 			"prefix": "HS",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 141, "y": 326 },
 				{ "infix": "B", "x": 238, "y": 275 },
@@ -158,7 +158,7 @@
 
 		{
 			"prefix": "BB",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 167, "y": 342 },
 				{ "infix": "B", "x": 217, "y": 263 },
@@ -170,7 +170,7 @@
 
 		{
 			"prefix": "NB",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 6, "y": 292 },
 				{ "infix": "B", "x": 161, "y": 334 },
@@ -181,7 +181,7 @@
 
 		{
 			"prefix": "EL",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 11, "y": 73 },
 				{ "infix": "B", "x": 0, "y": 241 },
@@ -192,7 +192,7 @@
 
 		{
 			"prefix": "RN",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 84, "y": 319 },
 				{ "infix": "B", "x": 194, "y": 275 },
@@ -203,7 +203,7 @@
 
 		{
 			"prefix": "UA",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 157, "y": 409 },
 				{ "infix": "B", "x": 62, "y": 346 },
@@ -222,7 +222,7 @@
 
 		{
 			"prefix": "SP",
-			"color_suffix_length": 2,
+			"colorSuffixLength": 2,
 			"desc": [
 				{ "infix": "A", "x": 7, "y": 295 },
 				{ "infix": "B", "x": 44, "y": 141 },

config/shortcutsConfig.json

@@ -243,6 +243,15 @@
 		"townOpenThievesGuild":     "G",
 		"townOpenVisitingHero":     "Ctrl+H",
 		"townSwapArmies":           "Space",
+		"listHeroUp":               "Ctrl+PageUp",
+		"listHeroDown":             "Ctrl+PageDown",
+		"listHeroTop":              "Ctrl+Home",
+		"listHeroBottom":           "Ctrl+End",
+		"listHeroDismiss":          "Delete",
+		"listTownUp":               "Ctrl+PageUp",
+		"listTownDown":             "Ctrl+PageDown",
+		"listTownTop":              "Ctrl+Home",
+		"listTownBottom":           "Ctrl+End",
 		
 		// Controller-specific
 		"mouseCursorX":             [],

docs/modders/Campaign_Format.md

@@ -182,8 +182,10 @@ Predefined campaign regions are located in file `campaign_regions.json`
 
 ```js
 {
+    "background": "ownRegionBackground.png",
+		"suffix": ["Enabled", "Selected", "Conquered"],
     "prefix": "G3",
-    "color_suffix_length": 1,
+    "colorSuffixLength": 1,
     "desc": [
         { "infix": "A", "x": 289, "y": 376 },
         { "infix": "B", "x": 60, "y": 147 },
@@ -192,9 +194,11 @@ Predefined campaign regions are located in file `campaign_regions.json`
 },
 ```
 
-- `"prefix"` used to identify all images related to campaign. In this example, background picture will be `G3_BG`
+- `"background"` optional - use own image name for background instead of adding "_BG" to the prefix as name
-- `"inflix"` ised to identify all images related to region. In this example, it will be pictures starting from `G3A_..., G3B_..., G3C_..."` 
+- `"prefix"` used to identify all images related to campaign. In this example (if background parameter wouldn't exists), background picture will be `G3_BG`
-- `"color_suffix_length"` identifies suffix length for region colourful frames. 1 is used for `R, B, N, G, O, V, T, P`, value 2 is used for `Re, Bl, Br, Gr, Or, Vi, Te, Pi`
+- `"suffix"` optional - use other suffixes than the default `En`, `Se` and `Co` for the three different images
+- `"infix"` used to identify all images related to region. In this example, it will be pictures whose files names begin with `G3A_..., G3B_..., G3C_..."` 
+- `"colorSuffixLength"` identifies suffix length for region colourful frames. 0 is no color suffix (no colorisation), 1 is used for `R, B, N, G, O, V, T, P`, value 2 is used for `Re, Bl, Br, Gr, Or, Vi, Te, Pi`
 
 ## Packing campaign
 

lib/CArtHandler.cpp

@@ -760,7 +760,7 @@ const CArtifactInstance * CArtifactSet::getArtByInstanceId(const ArtifactInstanc
 	return nullptr;
 }
 
-const ArtifactPosition CArtifactSet::getArtPos(const CArtifactInstance * artInst) const
+ArtifactPosition CArtifactSet::getArtPos(const CArtifactInstance * artInst) const
 {
 	if(artInst)
 	{

lib/CArtHandler.h

@@ -207,7 +207,7 @@ public:
 	/// Looks for equipped artifact with given ID and returns its slot ID or -1 if none
 	/// (if more than one such artifact lower ID is returned)
 	ArtifactPosition getArtPos(const ArtifactID & aid, bool onlyWorn = true, bool allowLocked = true) const;
-	const ArtifactPosition getArtPos(const CArtifactInstance * art) const;
+	ArtifactPosition getArtPos(const CArtifactInstance * art) const;
 	std::vector<ArtifactPosition> getAllArtPositions(const ArtifactID & aid, bool onlyWorn, bool allowLocked, bool getAll) const;
 	std::vector<ArtifactPosition> getBackpackArtPositions(const ArtifactID & aid) const;
 	const CArtifactInstance * getArtByInstanceId(const ArtifactInstanceID & artInstId) const;

lib/CConsoleHandler.cpp

@@ -168,6 +168,7 @@ LONG WINAPI onUnhandledException(EXCEPTION_POINTERS* exception)
 
 #endif
 
+#ifdef NDEBUG
 [[noreturn]] static void onTerminate()
 {
 	logGlobal->error("Disaster happened.");
@@ -205,6 +206,7 @@ LONG WINAPI onUnhandledException(EXCEPTION_POINTERS* exception)
 #endif
 	std::abort();
 }
+#endif
 
 void CConsoleHandler::setColor(EConsoleTextColor::EConsoleTextColor color)
 {
@@ -296,14 +298,14 @@ CConsoleHandler::CConsoleHandler():
 
 	GetConsoleScreenBufferInfo(handleErr, &csbi);
 	defErrColor = csbi.wAttributes;
-#ifndef _DEBUG
+#ifdef NDEBUG
 	SetUnhandledExceptionFilter(onUnhandledException);
 #endif
 #else
 	defColor = "\x1b[0m";
 #endif
 
-#ifndef _DEBUG
+#ifdef NDEBUG
 	std::set_terminate(onTerminate);
 #endif
 }

lib/CCreatureHandler.cpp

@@ -765,7 +765,7 @@ void CCreatureHandler::loadCrExpBon(CBonusSystemNode & globalEffects)
 		auto addBonusForTier = [&](int tier, std::shared_ptr<Bonus> b) {
 			assert(vstd::iswithin(tier, 1, 7));
 			//bonuses from level 7 are given to high-level creatures too
-			auto max = tier == GameConstants::CREATURES_PER_TOWN ? std::numeric_limits<int>::max() : tier + 1;
+			auto max = tier == 7 ? std::numeric_limits<int>::max() : tier + 1;
 			auto limiter = std::make_shared<CreatureLevelLimiter>(tier, max);
 			b->addLimiter(limiter);
 			globalEffects.addNewBonus(b);

lib/CGameInterface.cpp

@@ -168,7 +168,7 @@ void CAdventureAI::battleCatapultAttacked(const BattleID & battleID, const Catap
 }
 
 void CAdventureAI::battleStart(const BattleID & battleID, const CCreatureSet * army1, const CCreatureSet * army2, int3 tile,
-							   const CGHeroInstance * hero1, const CGHeroInstance * hero2, bool side, bool replayAllowed)
+							   const CGHeroInstance * hero1, const CGHeroInstance * hero2, BattleSide side, bool replayAllowed)
 {
 	assert(!battleAI);
 	assert(cbc);

lib/CGameInterface.h

@@ -146,7 +146,7 @@ public:
 
 	void battleNewRound(const BattleID & battleID) override;
 	void battleCatapultAttacked(const BattleID & battleID, const CatapultAttack & ca) override;
-	void battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool side, bool replayAllowed) override;
+	void battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, BattleSide side, bool replayAllowed) override;
 	void battleStacksAttacked(const BattleID & battleID, const std::vector<BattleStackAttacked> & bsa, bool ranged) override;
 	void actionStarted(const BattleID & battleID, const BattleAction &action) override;
 	void battleNewRoundFirst(const BattleID & battleID) override;

lib/CMakeLists.txt

@@ -403,6 +403,7 @@ set(lib_MAIN_HEADERS
 	battle/BattleAttackInfo.h
 	battle/BattleHex.h
 	battle/BattleInfo.h
+	battle/BattleSide.h
 	battle/BattleStateInfoForRetreat.h
 	battle/BattleProxy.h
 	battle/CBattleInfoCallback.h

lib/CStack.cpp

@@ -24,7 +24,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 
 ///CStack
-CStack::CStack(const CStackInstance * Base, const PlayerColor & O, int I, ui8 Side, const SlotID & S):
+CStack::CStack(const CStackInstance * Base, const PlayerColor & O, int I, BattleSide Side, const SlotID & S):
 	CBonusSystemNode(STACK_BATTLE),
 	base(Base),
 	ID(I),
@@ -45,7 +45,7 @@ CStack::CStack():
 {
 }
 
-CStack::CStack(const CStackBasicDescriptor * stack, const PlayerColor & O, int I, ui8 Side, const SlotID & S):
+CStack::CStack(const CStackBasicDescriptor * stack, const PlayerColor & O, int I, BattleSide Side, const SlotID & S):
 	CBonusSystemNode(STACK_BATTLE),
 	ID(I),
 	type(stack->type),
@@ -367,7 +367,7 @@ uint32_t CStack::unitId() const
 	return ID;
 }
 
-ui8 CStack::unitSide() const
+BattleSide CStack::unitSide() const
 {
 	return side;
 }

lib/CStack.h

@@ -32,7 +32,7 @@ private:
 	ui32 baseAmount = -1;
 
 	PlayerColor owner; //owner - player color (255 for neutrals)
-	ui8 side = 1;
+	BattleSide side = BattleSide::NONE;
 
 	SlotID slot;  //slot - position in garrison (may be 255 for neutrals/called creatures)
 
@@ -41,8 +41,8 @@ public:
 	
 	BattleHex initialPosition; //position on battlefield; -2 - keep, -3 - lower tower, -4 - upper tower
 
-	CStack(const CStackInstance * base, const PlayerColor & O, int I, ui8 Side, const SlotID & S);
+	CStack(const CStackInstance * base, const PlayerColor & O, int I, BattleSide Side, const SlotID & S);
-	CStack(const CStackBasicDescriptor * stack, const PlayerColor & O, int I, ui8 Side, const SlotID & S = SlotID(255));
+	CStack(const CStackBasicDescriptor * stack, const PlayerColor & O, int I, BattleSide Side, const SlotID & S = SlotID(255));
 	CStack();
 	~CStack();
 
@@ -74,7 +74,7 @@ public:
 	int32_t unitBaseAmount() const override;
 
 	uint32_t unitId() const override;
-	ui8 unitSide() const override;
+	BattleSide unitSide() const override;
 	PlayerColor unitOwner() const override;
 	SlotID unitSlot() const override;
 

lib/IGameEventsReceiver.h

@@ -68,7 +68,7 @@ public:
 	virtual void battleStacksEffectsSet(const BattleID & battleID, const SetStackEffect & sse){};//called when a specific effect is set to stacks
 	virtual void battleTriggerEffect(const BattleID & battleID, const BattleTriggerEffect & bte){}; //called for various one-shot effects
 	virtual void battleStartBefore(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2) {}; //called just before battle start
-	virtual void battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool side, bool replayAllowed){}; //called by engine when battle starts; side=0 - left, side=1 - right
+	virtual void battleStart(const BattleID & battleID, const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, BattleSide side, bool replayAllowed){}; //called by engine when battle starts; side=0 - left, side=1 - right
 	virtual void battleUnitsChanged(const BattleID & battleID, const std::vector<UnitChanges> & units){};
 	virtual void battleObstaclesChanged(const BattleID & battleID, const std::vector<ObstacleChanges> & obstacles){};
 	virtual void battleCatapultAttacked(const BattleID & battleID, const CatapultAttack & ca){}; //called when catapult makes an attack

lib/battle/AccessibilityInfo.cpp

@@ -15,7 +15,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-bool AccessibilityInfo::tileAccessibleWithGate(BattleHex tile, ui8 side) const
+bool AccessibilityInfo::tileAccessibleWithGate(BattleHex tile, BattleSide side) const
 {
 	//at(otherHex) != EAccessibility::ACCESSIBLE && (at(otherHex) != EAccessibility::GATE || side != BattleSide::DEFENDER)
 	if(at(tile) != EAccessibility::ACCESSIBLE)
@@ -29,7 +29,7 @@ bool AccessibilityInfo::accessible(BattleHex tile, const battle::Unit * stack) c
 	return accessible(tile, stack->doubleWide(), stack->unitSide());
 }
 
-bool AccessibilityInfo::accessible(BattleHex tile, bool doubleWide, ui8 side) const
+bool AccessibilityInfo::accessible(BattleHex tile, bool doubleWide, BattleSide side) const
 {
 	// All hexes that stack would cover if standing on tile have to be accessible.
 	//do not use getHexes for speed reasons

lib/battle/AccessibilityInfo.h

@@ -37,9 +37,9 @@ struct DLL_LINKAGE AccessibilityInfo : TAccessibilityArray
 {
 	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
+		bool accessible(BattleHex tile, bool doubleWide, BattleSide side) const; //checks for both tiles if stack is double wide
 	private:
-		bool tileAccessibleWithGate(BattleHex tile, ui8 side) const;
+		bool tileAccessibleWithGate(BattleHex tile, BattleSide side) const;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/battle/BattleAction.cpp

@@ -18,7 +18,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 static const int32_t INVALID_UNIT_ID = -1000;
 
 BattleAction::BattleAction():
-	side(-1),
+	side(BattleSide::NONE),
 	stackNumber(-1),
 	actionType(EActionType::NO_ACTION)
 {
@@ -96,7 +96,7 @@ BattleAction BattleAction::makeMove(const battle::Unit * stack, BattleHex dest)
 	return ba;
 }
 
-BattleAction BattleAction::makeEndOFTacticPhase(ui8 side)
+BattleAction BattleAction::makeEndOFTacticPhase(BattleSide side)
 {
 	BattleAction ba;
 	ba.side = side;
@@ -104,7 +104,7 @@ BattleAction BattleAction::makeEndOFTacticPhase(ui8 side)
 	return ba;
 }
 
-BattleAction BattleAction::makeSurrender(ui8 side)
+BattleAction BattleAction::makeSurrender(BattleSide side)
 {
 	BattleAction ba;
 	ba.side = side;
@@ -112,7 +112,7 @@ BattleAction BattleAction::makeSurrender(ui8 side)
 	return ba;
 }
 
-BattleAction BattleAction::makeRetreat(ui8 side)
+BattleAction BattleAction::makeRetreat(BattleSide side)
 {
 	BattleAction ba;
 	ba.side = side;

lib/battle/BattleAction.h

@@ -24,7 +24,7 @@ namespace battle
 class DLL_LINKAGE BattleAction
 {
 public:
-	ui8 side; //who made this action
+	BattleSide side; //who made this action
 	ui32 stackNumber; //stack ID, -1 left hero, -2 right hero,
 	EActionType actionType; //use ActionType enum for values
 
@@ -39,9 +39,9 @@ public:
 	static BattleAction makeShotAttack(const battle::Unit * shooter, const battle::Unit * target);
 	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 makeEndOFTacticPhase(BattleSide side);
-	static BattleAction makeRetreat(ui8 side);
+	static BattleAction makeRetreat(BattleSide side);
-	static BattleAction makeSurrender(ui8 side);
+	static BattleAction makeSurrender(BattleSide side);
 
 	bool isTacticsAction() const;
 	bool isUnitAction() const;

lib/battle/BattleHex.cpp

@@ -184,7 +184,7 @@ void BattleHex::checkAndPush(BattleHex tile, std::vector<BattleHex> & ret)
 		ret.push_back(tile);
 }
 
-BattleHex BattleHex::getClosestTile(ui8 side, BattleHex initialPos, std::set<BattleHex> & possibilities)
+BattleHex BattleHex::getClosestTile(BattleSide side, BattleHex initialPos, std::set<BattleHex> & possibilities)
 {
 	std::vector<BattleHex> sortedTiles (possibilities.begin(), possibilities.end()); //set can't be sorted properly :(
 	BattleHex initialHex = BattleHex(initialPos);

lib/battle/BattleHex.h

@@ -9,19 +9,12 @@
  */
 #pragma once
 
+#include "BattleSide.h"
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 //TODO: change to enum class
 
-namespace BattleSide
-{
-	enum Type
-	{
-		ATTACKER = 0,
-		DEFENDER = 1
-	};
-}
-
 namespace GameConstants
 {
 	const int BFIELD_WIDTH = 17;
@@ -29,8 +22,6 @@ namespace GameConstants
 	const int BFIELD_SIZE = BFIELD_WIDTH * BFIELD_HEIGHT;
 }
 
-using BattleSideOpt = std::optional<ui8>;
-
 // for battle stacks' positions
 struct DLL_LINKAGE BattleHex //TODO: decide if this should be changed to class for better code design
 {
@@ -102,7 +93,7 @@ struct DLL_LINKAGE BattleHex //TODO: decide if this should be changed to class f
 	static EDir mutualPosition(BattleHex hex1, BattleHex hex2);
 	static uint8_t getDistance(BattleHex hex1, BattleHex hex2);
 	static void checkAndPush(BattleHex tile, std::vector<BattleHex> & ret);
-	static BattleHex getClosestTile(ui8 side, BattleHex initialPos, std::set<BattleHex> & possibilities); //TODO: vector or set? copying one to another is bad
+	static BattleHex getClosestTile(BattleSide side, BattleHex initialPos, std::set<BattleHex> & possibilities); //TODO: vector or set? copying one to another is bad
 
 	template <typename Handler>
 	void serialize(Handler &h)

lib/battle/BattleInfo.cpp

@@ -27,10 +27,20 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-///BattleInfo
+const SideInBattle & BattleInfo::getSide(BattleSide side) const
-CStack * BattleInfo::generateNewStack(uint32_t id, const CStackInstance & base, ui8 side, const SlotID & slot, BattleHex position)
 {
-	PlayerColor owner = sides[side].color;
+	return sides.at(side);
+}
+
+SideInBattle & BattleInfo::getSide(BattleSide side)
+{
+	return sides.at(side);
+}
+
+///BattleInfo
+CStack * BattleInfo::generateNewStack(uint32_t id, const CStackInstance & base, BattleSide side, const SlotID & slot, BattleHex position)
+{
+	PlayerColor owner = getSide(side).color;
 	assert(!owner.isValidPlayer() || (base.armyObj && base.armyObj->tempOwner == owner));
 
 	auto * ret = new CStack(&base, owner, id, side, slot);
@@ -39,9 +49,9 @@ CStack * BattleInfo::generateNewStack(uint32_t id, const CStackInstance & base,
 	return ret;
 }
 
-CStack * BattleInfo::generateNewStack(uint32_t id, const CStackBasicDescriptor & base, ui8 side, const SlotID & slot, BattleHex position)
+CStack * BattleInfo::generateNewStack(uint32_t id, const CStackBasicDescriptor & base, BattleSide side, const SlotID & slot, BattleHex position)
 {
-	PlayerColor owner = sides[side].color;
+	PlayerColor owner = getSide(side).color;
 	auto * ret = new CStack(&base, owner, id, side, slot);
 	ret->initialPosition = position;
 	stacks.push_back(ret);
@@ -50,7 +60,7 @@ CStack * BattleInfo::generateNewStack(uint32_t id, const CStackBasicDescriptor &
 
 void BattleInfo::localInit()
 {
-	for(int i = 0; i < 2; i++)
+	for(BattleSide i : { BattleSide::ATTACKER, BattleSide::DEFENDER})
 	{
 		auto * armyObj = battleGetArmyObject(i);
 		armyObj->battle = this;
@@ -162,12 +172,12 @@ struct RangeGenerator
 	std::function<int()> myRand;
 };
 
-BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const BattleField & battlefieldType, const CArmedInstance * armies[2], const CGHeroInstance * heroes[2], bool creatureBank, const CGTownInstance * town)
+BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const BattleField & battlefieldType, BattleSideArray<const CArmedInstance *> armies, BattleSideArray<const CGHeroInstance *> heroes, bool creatureBank, const CGTownInstance * town)
 {
 	CMP_stack cmpst;
 	auto * curB = new BattleInfo();
 
-	for(auto i = 0u; i < curB->sides.size(); i++)
+	for(auto i : { BattleSide::LEFT_SIDE, BattleSide::RIGHT_SIDE})
 		curB->sides[i].init(heroes[i], armies[i]);
 
 
@@ -315,36 +325,32 @@ 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];
+	BattleSideArray<std::vector<std::vector<int>>> looseFormations;
-	std::vector<std::vector<int>> tightFormations[2];
+	BattleSideArray<std::vector<std::vector<int>>> tightFormations;
-	std::vector<std::vector<int>> creBankFormations[2];
+	BattleSideArray<std::vector<std::vector<int>>> creBankFormations;
-	std::vector<int> commanderField;
+	BattleSideArray<int> commanderField;
-	std::vector<int> commanderBank;
+	BattleSideArray<int> commanderBank;
 	const JsonNode config(JsonPath::builtin("config/battleStartpos.json"));
 	const JsonVector &positions = config["battle_positions"].Vector();
 
-	CGH::readBattlePositions(positions[0]["levels"], looseFormations[0]);
+	CGH::readBattlePositions(positions[0]["levels"], looseFormations[BattleSide::ATTACKER]);
-	CGH::readBattlePositions(positions[1]["levels"], looseFormations[1]);
+	CGH::readBattlePositions(positions[1]["levels"], looseFormations[BattleSide::DEFENDER]);
-	CGH::readBattlePositions(positions[2]["levels"], tightFormations[0]);
+	CGH::readBattlePositions(positions[2]["levels"], tightFormations[BattleSide::ATTACKER]);
-	CGH::readBattlePositions(positions[3]["levels"], tightFormations[1]);
+	CGH::readBattlePositions(positions[3]["levels"], tightFormations[BattleSide::DEFENDER]);
-	CGH::readBattlePositions(positions[4]["levels"], creBankFormations[0]);
+	CGH::readBattlePositions(positions[4]["levels"], creBankFormations[BattleSide::ATTACKER]);
-	CGH::readBattlePositions(positions[5]["levels"], creBankFormations[1]);
+	CGH::readBattlePositions(positions[5]["levels"], creBankFormations[BattleSide::DEFENDER]);
 
-	for (auto position : config["commanderPositions"]["field"].Vector())
+	commanderField[BattleSide::ATTACKER] = config["commanderPositions"]["field"][0].Integer();
-	{
+	commanderField[BattleSide::DEFENDER] = config["commanderPositions"]["field"][1].Integer();
-		commanderField.push_back(static_cast<int>(position.Float()));
-	}
-	for (auto position : config["commanderPositions"]["creBank"].Vector())
-	{
-		commanderBank.push_back(static_cast<int>(position.Float()));
-	}
 
+	commanderBank[BattleSide::ATTACKER] = config["commanderPositions"]["creBank"][0].Integer();
+	commanderBank[BattleSide::DEFENDER] = config["commanderPositions"]["creBank"][1].Integer();
 
 	//adding war machines
 	if(!creatureBank)
 	{
 		//Checks if hero has artifact and create appropriate stack
-		auto handleWarMachine = [&](int side, const ArtifactPosition & artslot, BattleHex hex)
+		auto handleWarMachine = [&](BattleSide side, const ArtifactPosition & artslot, BattleHex hex)
 		{
 			const CArtifactInstance * warMachineArt = heroes[side]->getArt(artslot);
 
@@ -357,28 +363,28 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 			}
 		};
 
-		if(heroes[0])
+		if(heroes[BattleSide::ATTACKER])
 		{
 
-			handleWarMachine(0, ArtifactPosition::MACH1, 52);
+			handleWarMachine(BattleSide::ATTACKER, ArtifactPosition::MACH1, 52);
-			handleWarMachine(0, ArtifactPosition::MACH2, 18);
+			handleWarMachine(BattleSide::ATTACKER, ArtifactPosition::MACH2, 18);
-			handleWarMachine(0, ArtifactPosition::MACH3, 154);
+			handleWarMachine(BattleSide::ATTACKER, ArtifactPosition::MACH3, 154);
 			if(town && town->hasFort())
-				handleWarMachine(0, ArtifactPosition::MACH4, 120);
+				handleWarMachine(BattleSide::ATTACKER, ArtifactPosition::MACH4, 120);
 		}
 
-		if(heroes[1])
+		if(heroes[BattleSide::DEFENDER])
 		{
 			if(!town) //defending hero shouldn't receive ballista (bug #551)
-				handleWarMachine(1, ArtifactPosition::MACH1, 66);
+				handleWarMachine(BattleSide::DEFENDER, ArtifactPosition::MACH1, 66);
-			handleWarMachine(1, ArtifactPosition::MACH2, 32);
+			handleWarMachine(BattleSide::DEFENDER, ArtifactPosition::MACH2, 32);
-			handleWarMachine(1, ArtifactPosition::MACH3, 168);
+			handleWarMachine(BattleSide::DEFENDER, ArtifactPosition::MACH3, 168);
 		}
 	}
 	//war machines added
 
 	//battleStartpos read
-	for(int side = 0; side < 2; side++)
+	for(BattleSide side : {BattleSide::ATTACKER, BattleSide::DEFENDER})
 	{
 		int formationNo = armies[side]->stacksCount() - 1;
 		vstd::abetween(formationNo, 0, GameConstants::ARMY_SIZE - 1);
@@ -397,14 +403,14 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 
 			BattleHex pos = (k < formationVector->size() ? formationVector->at(k) : 0);
 			if(creatureBank && i->second->type->isDoubleWide())
-				pos += side ? BattleHex::LEFT : BattleHex::RIGHT;
+				pos += side == BattleSide::RIGHT_SIDE ? BattleHex::LEFT : BattleHex::RIGHT;
 
 			curB->generateNewStack(curB->nextUnitId(), *i->second, side, i->first, pos);
 		}
 	}
 
 	//adding commanders
-	for (int i = 0; i < 2; ++i)
+	for(BattleSide i : {BattleSide::ATTACKER, BattleSide::DEFENDER})
 	{
 		if (heroes[i] && heroes[i]->commander && heroes[i]->commander->alive)
 		{
@@ -416,14 +422,14 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 	if (curB->town && curB->town->fortLevel() >= CGTownInstance::CITADEL)
 	{
 		// keep tower
-		curB->generateNewStack(curB->nextUnitId(), CStackBasicDescriptor(CreatureID::ARROW_TOWERS, 1), 1, SlotID::ARROW_TOWERS_SLOT, BattleHex::CASTLE_CENTRAL_TOWER);
+		curB->generateNewStack(curB->nextUnitId(), CStackBasicDescriptor(CreatureID::ARROW_TOWERS, 1), BattleSide::DEFENDER, SlotID::ARROW_TOWERS_SLOT, BattleHex::CASTLE_CENTRAL_TOWER);
 
 		if (curB->town->fortLevel() >= CGTownInstance::CASTLE)
 		{
 			// lower tower + upper tower
-			curB->generateNewStack(curB->nextUnitId(), CStackBasicDescriptor(CreatureID::ARROW_TOWERS, 1), 1, SlotID::ARROW_TOWERS_SLOT, BattleHex::CASTLE_UPPER_TOWER);
+			curB->generateNewStack(curB->nextUnitId(), CStackBasicDescriptor(CreatureID::ARROW_TOWERS, 1), BattleSide::DEFENDER, SlotID::ARROW_TOWERS_SLOT, BattleHex::CASTLE_UPPER_TOWER);
 
-			curB->generateNewStack(curB->nextUnitId(), CStackBasicDescriptor(CreatureID::ARROW_TOWERS, 1), 1, SlotID::ARROW_TOWERS_SLOT, BattleHex::CASTLE_BOTTOM_TOWER);
+			curB->generateNewStack(curB->nextUnitId(), CStackBasicDescriptor(CreatureID::ARROW_TOWERS, 1), BattleSide::DEFENDER, SlotID::ARROW_TOWERS_SLOT, BattleHex::CASTLE_BOTTOM_TOWER);
 		}
 
 		//Moat generating is done on server
@@ -453,11 +459,9 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 	//tactics
 	bool isTacticsAllowed = !creatureBank; //no tactics in creature banks
 
-	constexpr int sideSize = 2;
+	BattleSideArray<int> battleRepositionHex = {};
-
+	BattleSideArray<int> battleRepositionHexBlock = {};
-	std::array<int, sideSize> battleRepositionHex = {};
+	for(auto i : {BattleSide::ATTACKER, BattleSide::DEFENDER})
-	std::array<int, sideSize> battleRepositionHexBlock = {};
-	for(int i = 0; i < sideSize; i++)
 	{
 		if(heroes[i])
 		{
@@ -508,14 +512,14 @@ const CGHeroInstance * BattleInfo::getHero(const PlayerColor & player) const
 	return nullptr;
 }
 
-ui8 BattleInfo::whatSide(const PlayerColor & player) const
+BattleSide BattleInfo::whatSide(const PlayerColor & player) const
 {
-	for(int i = 0; i < sides.size(); i++)
+	for(auto i : {BattleSide::ATTACKER, BattleSide::DEFENDER})
 		if(sides[i].color == player)
 			return i;
 
 	logGlobal->warn("BattleInfo::whatSide: Player %s is not in battle!", player.toString());
-	return -1;
+	return BattleSide::NONE;
 }
 
 CStack * BattleInfo::getStack(int stackID, bool onlyAlive)
@@ -529,7 +533,7 @@ BattleInfo::BattleInfo():
 	town(nullptr),
 	tile(-1,-1,-1),
 	battlefieldType(BattleField::NONE),
-	tacticsSide(0),
+	tacticsSide(BattleSide::NONE),
 	tacticDistance(0)
 {
 	setNodeType(BATTLE);
@@ -555,7 +559,7 @@ BattleInfo::~BattleInfo()
 	for (auto & elem : stacks)
 		delete elem;
 
-	for(int i = 0; i < 2; i++)
+	for(auto i : {BattleSide::ATTACKER, BattleSide::DEFENDER})
 		if(auto * _armyObj = battleGetArmyObject(i))
 			_armyObj->battle = nullptr;
 }
@@ -600,27 +604,27 @@ IBattleInfo::ObstacleCList BattleInfo::getAllObstacles() const
 	return ret;
 }
 
-PlayerColor BattleInfo::getSidePlayer(ui8 side) const
+PlayerColor BattleInfo::getSidePlayer(BattleSide side) const
 {
-	return sides.at(side).color;
+	return getSide(side).color;
 }
 
-const CArmedInstance * BattleInfo::getSideArmy(ui8 side) const
+const CArmedInstance * BattleInfo::getSideArmy(BattleSide side) const
 {
-	return sides.at(side).armyObject;
+	return getSide(side).armyObject;
 }
 
-const CGHeroInstance * BattleInfo::getSideHero(ui8 side) const
+const CGHeroInstance * BattleInfo::getSideHero(BattleSide side) const
 {
-	return sides.at(side).hero;
+	return getSide(side).hero;
 }
 
-ui8 BattleInfo::getTacticDist() const
+uint8_t BattleInfo::getTacticDist() const
 {
 	return tacticDistance;
 }
 
-ui8 BattleInfo::getTacticsSide() const
+BattleSide BattleInfo::getTacticsSide() const
 {
 	return tacticsSide;
 }
@@ -640,14 +644,14 @@ EGateState BattleInfo::getGateState() const
 	return si.gateState;
 }
 
-uint32_t BattleInfo::getCastSpells(ui8 side) const
+uint32_t BattleInfo::getCastSpells(BattleSide side) const
 {
-	return sides.at(side).castSpellsCount;
+	return getSide(side).castSpellsCount;
 }
 
-int32_t BattleInfo::getEnchanterCounter(ui8 side) const
+int32_t BattleInfo::getEnchanterCounter(BattleSide side) const
 {
-	return sides.at(side).enchanterCounter;
+	return getSide(side).enchanterCounter;
 }
 
 const IBonusBearer * BattleInfo::getBonusBearer() const
@@ -685,14 +689,14 @@ bool BattleInfo::isCreatureBank() const
 }
 
 
-std::vector<SpellID> BattleInfo::getUsedSpells(ui8 side) const
+std::vector<SpellID> BattleInfo::getUsedSpells(BattleSide side) const
 {
-	return sides.at(side).usedSpellsHistory;
+	return getSide(side).usedSpellsHistory;
 }
 
 void BattleInfo::nextRound()
 {
-	for(int i = 0; i < 2; ++i)
+	for(auto i : {BattleSide::ATTACKER, BattleSide::DEFENDER})
 	{
 		sides.at(i).castSpellsCount = 0;
 		vstd::amax(--sides.at(i).enchanterCounter, 0);
@@ -994,12 +998,12 @@ void BattleInfo::removeObstacle(uint32_t id)
 	}
 }
 
-CArmedInstance * BattleInfo::battleGetArmyObject(ui8 side) const
+CArmedInstance * BattleInfo::battleGetArmyObject(BattleSide side) const
 {
 	return const_cast<CArmedInstance*>(CBattleInfoEssentials::battleGetArmyObject(side));
 }
 
-CGHeroInstance * BattleInfo::battleGetFightingHero(ui8 side) const
+CGHeroInstance * BattleInfo::battleGetFightingHero(BattleSide side) const
 {
 	return const_cast<CGHeroInstance*>(CBattleInfoEssentials::battleGetFightingHero(side));
 }
@@ -1009,7 +1013,7 @@ scripting::Pool * BattleInfo::getContextPool() const
 {
 	//this is real battle, use global scripting context pool
 	//TODO: make this line not ugly
-	return battleGetFightingHero(0)->cb->getGlobalContextPool();
+	return battleGetFightingHero(BattleSide::ATTACKER)->cb->getGlobalContextPool();
 }
 #endif
 
@@ -1045,7 +1049,7 @@ bool CMP_stack::operator()(const battle::Unit * a, const battle::Unit * b) const
 	return false;
 }
 
-CMP_stack::CMP_stack(int Phase, int Turn, uint8_t Side):
+CMP_stack::CMP_stack(int Phase, int Turn, BattleSide Side):
 	phase(Phase), 
 	turn(Turn), 
 	side(Side) 

lib/battle/BattleInfo.h

@@ -25,15 +25,10 @@ class BattleField;
 
 class DLL_LINKAGE BattleInfo : public CBonusSystemNode, public CBattleInfoCallback, public IBattleState
 {
+	BattleSideArray<SideInBattle> sides; //sides[0] - attacker, sides[1] - defender
 public:
 	BattleID battleID = BattleID(0);
 
-	enum BattleSide
-	{
-		ATTACKER = 0,
-		DEFENDER
-	};
-	std::array<SideInBattle, 2> sides; //sides[0] - attacker, sides[1] - defender
 	si32 round;
 	si32 activeStack;
 	const CGTownInstance * town; //used during town siege, nullptr if this is not a siege (note that fortless town IS also a siege)
@@ -47,7 +42,7 @@ public:
 	BattleField battlefieldType; //like !!BA:B
 	TerrainId terrainType; //used for some stack nativity checks (not the bonus limiters though that have their own copy)
 
-	ui8 tacticsSide; //which side is requested to play tactics phase
+	BattleSide tacticsSide; //which side is requested to play tactics phase
 	ui8 tacticDistance; //how many hexes we can go forward (1 = only hexes adjacent to margin line)
 
 	template <typename Handler> void serialize(Handler &h)
@@ -92,19 +87,19 @@ public:
 
 	ObstacleCList getAllObstacles() const override;
 
-	PlayerColor getSidePlayer(ui8 side) const override;
+	PlayerColor getSidePlayer(BattleSide side) const override;
-	const CArmedInstance * getSideArmy(ui8 side) const override;
+	const CArmedInstance * getSideArmy(BattleSide side) const override;
-	const CGHeroInstance * getSideHero(ui8 side) const override;
+	const CGHeroInstance * getSideHero(BattleSide side) const override;
 
 	ui8 getTacticDist() const override;
-	ui8 getTacticsSide() const override;
+	BattleSide getTacticsSide() const override;
 
 	const CGTownInstance * getDefendedTown() const override;
 	EWallState getWallState(EWallPart partOfWall) const override;
 	EGateState getGateState() const override;
 
-	uint32_t getCastSpells(ui8 side) const override;
+	uint32_t getCastSpells(BattleSide side) const override;
-	int32_t getEnchanterCounter(ui8 side) const override;
+	int32_t getEnchanterCounter(BattleSide side) const override;
 
 	const IBonusBearer * getBonusBearer() const override;
 
@@ -115,7 +110,7 @@ public:
 	int3 getLocation() const override;
 	bool isCreatureBank() const override;
 
-	std::vector<SpellID> getUsedSpells(ui8 side) const override;
+	std::vector<SpellID> getUsedSpells(BattleSide side) const override;
 
 	//////////////////////////////////////////////////////////////////////////
 	// IBattleState
@@ -144,19 +139,22 @@ public:
 	//////////////////////////////////////////////////////////////////////////
 	CStack * getStack(int stackID, bool onlyAlive = true);
 	using CBattleInfoEssentials::battleGetArmyObject;
-	CArmedInstance * battleGetArmyObject(ui8 side) const;
+	CArmedInstance * battleGetArmyObject(BattleSide side) const;
 	using CBattleInfoEssentials::battleGetFightingHero;
-	CGHeroInstance * battleGetFightingHero(ui8 side) const;
+	CGHeroInstance * battleGetFightingHero(BattleSide side) const;
 
-	CStack * generateNewStack(uint32_t id, const CStackInstance & base, ui8 side, const SlotID & slot, BattleHex position);
+	CStack * generateNewStack(uint32_t id, const CStackInstance & base, BattleSide side, const SlotID & slot, BattleHex position);
-	CStack * generateNewStack(uint32_t id, const CStackBasicDescriptor & base, ui8 side, const SlotID & slot, BattleHex position);
+	CStack * generateNewStack(uint32_t id, const CStackBasicDescriptor & base, BattleSide side, const SlotID & slot, BattleHex position);
+
+	const SideInBattle & getSide(BattleSide side) const;
+	SideInBattle & getSide(BattleSide side);
 
 	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);
+	static BattleInfo * setupBattle(const int3 & tile, TerrainId, const BattleField & battlefieldType, BattleSideArray<const CArmedInstance *> armies, BattleSideArray<const CGHeroInstance *> heroes, bool creatureBank, const CGTownInstance * town);
 
-	ui8 whatSide(const PlayerColor & player) const;
+	BattleSide whatSide(const PlayerColor & player) const;
 
 protected:
 #if SCRIPTING_ENABLED
@@ -169,10 +167,10 @@ class DLL_LINKAGE CMP_stack
 {
 	int phase; //rules of which phase will be used
 	int turn;
-	uint8_t side;
+	BattleSide side;
 public:
 	bool operator()(const battle::Unit * a, const battle::Unit * b) const;
-	CMP_stack(int Phase = 1, int Turn = 0, uint8_t Side = BattleSide::ATTACKER);
+	CMP_stack(int Phase = 1, int Turn = 0, BattleSide Side = BattleSide::ATTACKER);
 };
 
 VCMI_LIB_NAMESPACE_END

lib/battle/BattleProxy.cpp

@@ -65,17 +65,17 @@ IBattleInfo::ObstacleCList BattleProxy::getAllObstacles() const
 	return subject->battleGetAllObstacles();
 }
 
-PlayerColor BattleProxy::getSidePlayer(ui8 side) const
+PlayerColor BattleProxy::getSidePlayer(BattleSide side) const
 {
 	return subject->sideToPlayer(side);
 }
 
-const CArmedInstance * BattleProxy::getSideArmy(ui8 side) const
+const CArmedInstance * BattleProxy::getSideArmy(BattleSide side) const
 {
 	return subject->battleGetArmyObject(side);
 }
 
-const CGHeroInstance * BattleProxy::getSideHero(ui8 side) const
+const CGHeroInstance * BattleProxy::getSideHero(BattleSide side) const
 {
 	return subject->battleGetFightingHero(side);
 }
@@ -85,7 +85,7 @@ ui8 BattleProxy::getTacticDist() const
 	return subject->battleTacticDist();
 }
 
-ui8 BattleProxy::getTacticsSide() const
+BattleSide BattleProxy::getTacticsSide() const
 {
 	return subject->battleGetTacticsSide();
 }
@@ -105,12 +105,12 @@ EGateState BattleProxy::getGateState() const
 	return subject->battleGetGateState();
 }
 
-uint32_t BattleProxy::getCastSpells(ui8 side) const
+uint32_t BattleProxy::getCastSpells(BattleSide side) const
 {
 	return subject->battleCastSpells(side);
 }
 
-int32_t BattleProxy::getEnchanterCounter(ui8 side) const
+int32_t BattleProxy::getEnchanterCounter(BattleSide side) const
 {
 	return subject->battleGetEnchanterCounter(side);
 }

lib/battle/BattleProxy.h

@@ -38,19 +38,19 @@ public:
 
 	ObstacleCList getAllObstacles() const override;
 
-	PlayerColor getSidePlayer(ui8 side) const override;
+	PlayerColor getSidePlayer(BattleSide side) const override;
-	const CArmedInstance * getSideArmy(ui8 side) const override;
+	const CArmedInstance * getSideArmy(BattleSide side) const override;
-	const CGHeroInstance * getSideHero(ui8 side) const override;
+	const CGHeroInstance * getSideHero(BattleSide side) const override;
 
 	ui8 getTacticDist() const override;
-	ui8 getTacticsSide() const override;
+	BattleSide getTacticsSide() const override;
 
 	const CGTownInstance * getDefendedTown() const override;
 	EWallState getWallState(EWallPart partOfWall) const override;
 	EGateState getGateState() const override;
 
-	uint32_t getCastSpells(ui8 side) const override;
+	uint32_t getCastSpells(BattleSide side) const override;
-	int32_t getEnchanterCounter(ui8 side) const override;
+	int32_t getEnchanterCounter(BattleSide side) const override;
 
 	const IBonusBearer * getBonusBearer() const override;
 protected:

lib/battle/BattleSide.h

@@ -0,0 +1,53 @@
+/*
+ * BattleSide.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#pragma once
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+enum class BattleSide : int8_t
+{
+	NONE = -1,
+	INVALID = -2,
+	ALL_KNOWING = -3,
+
+	ATTACKER = 0,
+	DEFENDER = 1,
+
+	// Aliases for convenience
+	LEFT_SIDE = ATTACKER,
+	RIGHT_SIDE = DEFENDER,
+};
+
+template<typename T>
+class BattleSideArray : public std::array<T, 2>
+{
+public:
+	const T & at(BattleSide side) const
+	{
+		return std::array<T, 2>::at(static_cast<int>(side));
+	}
+
+	T & at(BattleSide side)
+	{
+		return std::array<T, 2>::at(static_cast<int>(side));
+	}
+
+	const T & operator[](BattleSide side) const
+	{
+		return std::array<T, 2>::at(static_cast<int>(side));
+	}
+
+	T & operator[](BattleSide side)
+	{
+		return std::array<T, 2>::at(static_cast<int>(side));
+	}
+};
+
+VCMI_LIB_NAMESPACE_END

lib/battle/BattleStateInfoForRetreat.cpp

@@ -23,7 +23,7 @@ BattleStateInfoForRetreat::BattleStateInfoForRetreat():
 	isLastTurnBeforeDie(false),
 	ourHero(nullptr),
 	enemyHero(nullptr),
-	ourSide(-1)
+	ourSide(BattleSide::NONE)
 {
 }
 

lib/battle/BattleStateInfoForRetreat.h

@@ -9,6 +9,8 @@
  */
 #pragma once
 
+#include "BattleSide.h"
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 namespace battle
@@ -24,7 +26,7 @@ public:
 	bool canFlee;
 	bool canSurrender;
 	bool isLastTurnBeforeDie;
-	ui8 ourSide;
+	BattleSide ourSide;
 	std::vector<const battle::Unit *> ourStacks;
 	std::vector<const battle::Unit *> enemyStacks;
 	const CGHeroInstance * ourHero;

lib/battle/CBattleInfoCallback.cpp

@@ -110,9 +110,9 @@ ESpellCastProblem CBattleInfoCallback::battleCanCastSpell(const spells::Caster *
 	}
 	const PlayerColor player = caster->getCasterOwner();
 	const auto side = playerToSide(player);
-	if(!side)
+	if(side == BattleSide::NONE)
 		return ESpellCastProblem::INVALID;
-	if(!battleDoWeKnowAbout(side.value()))
+	if(!battleDoWeKnowAbout(side))
 	{
 		logGlobal->warn("You can't check if enemy can cast given spell!");
 		return ESpellCastProblem::INVALID;
@@ -125,7 +125,7 @@ ESpellCastProblem CBattleInfoCallback::battleCanCastSpell(const spells::Caster *
 	{
 	case spells::Mode::HERO:
 	{
-		if(battleCastSpells(side.value()) > 0)
+		if(battleCastSpells(side) > 0)
 			return ESpellCastProblem::CASTS_PER_TURN_LIMIT;
 
 		const auto * hero = dynamic_cast<const CGHeroInstance *>(caster);
@@ -173,6 +173,9 @@ bool CBattleInfoCallback::battleIsInsideWalls(BattleHex from) const
 
 bool CBattleInfoCallback::battleHasPenaltyOnLine(BattleHex from, BattleHex dest, bool checkWall, bool checkMoat) const
 {
+	if (!from.isAvailable() || !dest.isAvailable())
+		throw std::runtime_error("Invalid hex (" + std::to_string(from.hex) + " and " + std::to_string(dest.hex) + ") received in battleHasPenaltyOnLine!" );
+
 	auto isTileBlocked = [&](BattleHex tile)
 	{
 		EWallPart wallPart = battleHexToWallPart(tile);
@@ -373,7 +376,7 @@ battle::Units CBattleInfoCallback::battleAliveUnits() const
 	});
 }
 
-battle::Units CBattleInfoCallback::battleAliveUnits(ui8 side) const
+battle::Units CBattleInfoCallback::battleAliveUnits(BattleSide side) const
 {
 	return battleGetUnitsIf([=](const battle::Unit * unit)
 	{
@@ -385,7 +388,7 @@ using namespace battle;
 
 //T is battle::Unit descendant
 template <typename T>
-const T * takeOneUnit(std::vector<const T*> & allUnits, const int turn, int8_t & sideThatLastMoved, int phase)
+const T * takeOneUnit(std::vector<const T*> & allUnits, const int turn, BattleSide & sideThatLastMoved, int phase)
 {
 	const T * returnedUnit = nullptr;
 	size_t currentUnitIndex = 0;
@@ -414,13 +417,13 @@ const T * takeOneUnit(std::vector<const T*> & allUnits, const int turn, int8_t &
 			}
 			else if(currentUnitInitiative == returnedUnitInitiative)
 			{
-				if(sideThatLastMoved == -1 && turn <= 0 && currentUnit->unitSide() == BattleSide::ATTACKER
+				if(sideThatLastMoved == BattleSide::NONE && turn <= 0 && currentUnit->unitSide() == BattleSide::ATTACKER
 					&& !(returnedUnit->unitSide() == currentUnit->unitSide() && returnedUnit->unitSlot() < currentUnit->unitSlot())) // Turn 0 attacker priority
 				{
 					returnedUnit = currentUnit;
 					currentUnitIndex = i;
 				}
-				else if(sideThatLastMoved != -1 && currentUnit->unitSide() != sideThatLastMoved
+				else if(sideThatLastMoved != BattleSide::NONE && currentUnit->unitSide() != sideThatLastMoved
 					&& !(returnedUnit->unitSide() == currentUnit->unitSide() && returnedUnit->unitSlot() < currentUnit->unitSlot())) // Alternate equal speeds units
 				{
 					returnedUnit = currentUnit;
@@ -435,7 +438,7 @@ const T * takeOneUnit(std::vector<const T*> & allUnits, const int turn, int8_t &
 				returnedUnit = currentUnit;
 				currentUnitIndex = i;
 			}
-			else if(currentUnitInitiative == returnedUnitInitiative && sideThatLastMoved != -1 && currentUnit->unitSide() != sideThatLastMoved
+			else if(currentUnitInitiative == returnedUnitInitiative && sideThatLastMoved != BattleSide::NONE && currentUnit->unitSide() != sideThatLastMoved
 				&& !(returnedUnit->unitSide() == currentUnit->unitSide() && returnedUnit->unitSlot() < currentUnit->unitSlot())) // Alternate equal speeds units
 			{
 				returnedUnit = currentUnit;
@@ -455,7 +458,7 @@ const T * takeOneUnit(std::vector<const T*> & allUnits, const int turn, int8_t &
 	return returnedUnit;
 }
 
-void CBattleInfoCallback::battleGetTurnOrder(std::vector<battle::Units> & turns, const size_t maxUnits, const int maxTurns, const int turn, int8_t sideThatLastMoved) const
+void CBattleInfoCallback::battleGetTurnOrder(std::vector<battle::Units> & turns, const size_t maxUnits, const int maxTurns, const int turn, BattleSide sideThatLastMoved) const
 {
 	RETURN_IF_NOT_BATTLE();
 
@@ -497,7 +500,7 @@ void CBattleInfoCallback::battleGetTurnOrder(std::vector<battle::Units> & turns,
 
 		//its first or current turn, turn priority for active stack side
 		//TODO: what if active stack mind-controlled?
-		if(turn <= 0 && sideThatLastMoved < 0)
+		if(turn <= 0 && sideThatLastMoved == BattleSide::NONE)
 			sideThatLastMoved = activeUnit->unitSide();
 	}
 
@@ -557,7 +560,7 @@ void CBattleInfoCallback::battleGetTurnOrder(std::vector<battle::Units> & turns,
 		}
 	}
 
-	if(sideThatLastMoved < 0)
+	if(sideThatLastMoved == BattleSide::NONE)
 		sideThatLastMoved = BattleSide::ATTACKER;
 
 	if(!turnsIsFull() && (maxTurns == 0 || turns.size() < maxTurns))
@@ -884,7 +887,7 @@ bool CBattleInfoCallback::handleObstacleTriggersForUnit(SpellCastEnvironment & s
 				spellEnv.apply(&bocp);
 			};
 			const auto side = unit.unitSide();
-			auto shouldReveal = !spellObstacle->hidden || !battleIsObstacleVisibleForSide(*obstacle, (BattlePerspective::BattlePerspective)side);
+			auto shouldReveal = !spellObstacle->hidden || !battleIsObstacleVisibleForSide(*obstacle, side);
 			const auto * hero = battleGetFightingHero(spellObstacle->casterSide);
 			auto caster = spells::ObstacleCasterProxy(getBattle()->getSidePlayer(spellObstacle->casterSide), hero, *spellObstacle);
 
@@ -1095,7 +1098,7 @@ bool CBattleInfoCallback::isInObstacle(
 	return false;
 }
 
-std::set<BattleHex> CBattleInfoCallback::getStoppers(BattlePerspective::BattlePerspective whichSidePerspective) const
+std::set<BattleHex> CBattleInfoCallback::getStoppers(BattleSide whichSidePerspective) const
 {
 	std::set<BattleHex> ret;
 	RETURN_IF_NOT_BATTLE(ret);
@@ -1158,7 +1161,7 @@ std::pair<const battle::Unit *, BattleHex> CBattleInfoCallback::getNearestStack(
 		return std::make_pair<const battle::Unit * , BattleHex>(nullptr, BattleHex::INVALID);
 }
 
-BattleHex CBattleInfoCallback::getAvailableHex(const CreatureID & creID, ui8 side, int initialPos) const
+BattleHex CBattleInfoCallback::getAvailableHex(const CreatureID & creID, BattleSide side, int initialPos) const
 {
 	bool twoHex = VLC->creatures()->getById(creID)->isDoubleWide();
 
@@ -1205,8 +1208,13 @@ bool CBattleInfoCallback::isInTacticRange(BattleHex dest) const
 	auto side = battleGetTacticsSide();
 	auto dist = battleGetTacticDist();
 
-	return ((!side && dest.getX() > 0 && dest.getX() <= dist)
+	if (side == BattleSide::ATTACKER && dest.getX() > 0 && dest.getX() <= dist)
-			|| (side && dest.getX() < GameConstants::BFIELD_WIDTH - 1 && dest.getX() >= GameConstants::BFIELD_WIDTH - dist - 1));
+		return true;
+
+	if (side == BattleSide::DEFENDER && dest.getX() < GameConstants::BFIELD_WIDTH - 1 && dest.getX() >= GameConstants::BFIELD_WIDTH - dist - 1)
+		return true;
+
+	return false;
 }
 
 ReachabilityInfo CBattleInfoCallback::getReachability(const battle::Unit * unit) const
@@ -1449,7 +1457,7 @@ std::set<const CStack*> CBattleInfoCallback::getAttackedCreatures(const CStack*
 	return attackedCres;
 }
 
-static bool isHexInFront(BattleHex hex, BattleHex testHex, BattleSide::Type side )
+static bool isHexInFront(BattleHex hex, BattleHex testHex, BattleSide side )
 {
 	static const std::set<BattleHex::EDir> rightDirs { BattleHex::BOTTOM_RIGHT, BattleHex::TOP_RIGHT, BattleHex::RIGHT };
 	static const std::set<BattleHex::EDir> leftDirs  { BattleHex::BOTTOM_LEFT, BattleHex::TOP_LEFT, BattleHex::LEFT };
@@ -1474,7 +1482,7 @@ bool CBattleInfoCallback::isToReverse(const battle::Unit * attacker, const battl
 	if (attackerHex < 0 ) //turret
 		return false;
 
-	if(isHexInFront(attackerHex, defenderHex, static_cast<BattleSide::Type>(attacker->unitSide())))
+	if(isHexInFront(attackerHex, defenderHex, attacker->unitSide()))
 		return false;
 
 	auto defenderOtherHex = defenderHex;
@@ -1484,7 +1492,7 @@ bool CBattleInfoCallback::isToReverse(const battle::Unit * attacker, const battl
 	{
 		defenderOtherHex = battle::Unit::occupiedHex(defenderHex, true, defender->unitSide());
 
-		if(isHexInFront(attackerHex, defenderOtherHex, static_cast<BattleSide::Type>(attacker->unitSide())))
+		if(isHexInFront(attackerHex, defenderOtherHex, attacker->unitSide()))
 			return false;
 	}
 
@@ -1492,7 +1500,7 @@ bool CBattleInfoCallback::isToReverse(const battle::Unit * attacker, const battl
 	{
 		attackerOtherHex = battle::Unit::occupiedHex(attackerHex, true, attacker->unitSide());
 
-		if(isHexInFront(attackerOtherHex, defenderHex, static_cast<BattleSide::Type>(attacker->unitSide())))
+		if(isHexInFront(attackerOtherHex, defenderHex, attacker->unitSide()))
 			return false;
 	}
 
@@ -1500,7 +1508,7 @@ bool CBattleInfoCallback::isToReverse(const battle::Unit * attacker, const battl
 	// 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(attackerOtherHex, defenderOtherHex, static_cast<BattleSide::Type>(attacker->unitSide())))
+		if(isHexInFront(attackerOtherHex, defenderOtherHex, attacker->unitSide()))
 			return false;
 	}
 	return true;
@@ -1759,7 +1767,7 @@ SpellID CBattleInfoCallback::getRandomBeneficialSpell(vstd::RNG & rand, const ba
 		case SpellID::PROTECTION_FROM_FIRE:
 		case SpellID::PROTECTION_FROM_WATER:
 		{
-			const ui8 enemySide = 1 - subject->unitSide();
+			const BattleSide enemySide = otherSide(subject->unitSide());
 			//todo: only if enemy has spellbook
 			if (!battleHasHero(enemySide)) //only if there is enemy hero
 				continue;
@@ -1850,10 +1858,9 @@ int CBattleInfoCallback::battleGetSurrenderCost(const PlayerColor & Player) cons
 	if(!battleCanSurrender(Player))
 		return -1;
 
-	const auto sideOpt = playerToSide(Player);
+	const BattleSide side = playerToSide(Player);
-	if(!sideOpt)
+	if(side == BattleSide::NONE)
 		return -1;
-	const auto side = sideOpt.value();
 
 	int ret = 0;
 	double discount = 0;
@@ -1869,7 +1876,7 @@ int CBattleInfoCallback::battleGetSurrenderCost(const PlayerColor & Player) cons
 	return ret;
 }
 
-si8 CBattleInfoCallback::battleMinSpellLevel(ui8 side) const
+si8 CBattleInfoCallback::battleMinSpellLevel(BattleSide side) const
 {
 	const IBonusBearer * node = nullptr;
 	if(const CGHeroInstance * h = battleGetFightingHero(side))
@@ -1887,7 +1894,7 @@ si8 CBattleInfoCallback::battleMinSpellLevel(ui8 side) const
 	return 0;
 }
 
-si8 CBattleInfoCallback::battleMaxSpellLevel(ui8 side) const
+si8 CBattleInfoCallback::battleMaxSpellLevel(BattleSide side) const
 {
 	const IBonusBearer *node = nullptr;
 	if(const CGHeroInstance * h = battleGetFightingHero(side))
@@ -1906,21 +1913,21 @@ si8 CBattleInfoCallback::battleMaxSpellLevel(ui8 side) const
 	return GameConstants::SPELL_LEVELS;
 }
 
-std::optional<int> CBattleInfoCallback::battleIsFinished() const
+std::optional<BattleSide> CBattleInfoCallback::battleIsFinished() const
 {
 	auto units = battleGetUnitsIf([=](const battle::Unit * unit)
 	{
 		return unit->alive() && !unit->isTurret() && !unit->hasBonusOfType(BonusType::SIEGE_WEAPON);
 	});
 
-	std::array<bool, 2> hasUnit = {false, false}; //index is BattleSide
+	BattleSideArray<bool> hasUnit = {false, false}; //index is BattleSide
 
 	for(auto & unit : units)
 	{
 		//todo: move SIEGE_WEAPON check to Unit state
 		hasUnit.at(unit->unitSide()) = true;
 
-		if(hasUnit[0] && hasUnit[1])
+		if(hasUnit[BattleSide::ATTACKER] && hasUnit[BattleSide::DEFENDER])
 			return std::nullopt;
 	}
 	
@@ -1934,12 +1941,12 @@ std::optional<int> CBattleInfoCallback::battleIsFinished() const
 		}
 	}
 
-	if(!hasUnit[0] && !hasUnit[1])
+	if(!hasUnit[BattleSide::ATTACKER] && !hasUnit[BattleSide::DEFENDER])
-		return 2;
+		return BattleSide::NONE;
-	if(!hasUnit[1])
+	if(!hasUnit[BattleSide::DEFENDER])
-		return 0;
+		return BattleSide::ATTACKER;
 	else
-		return 1;
+		return BattleSide::DEFENDER;
 }
 
 VCMI_LIB_NAMESPACE_END

lib/battle/CBattleInfoCallback.h

@@ -56,7 +56,7 @@ struct DLL_LINKAGE BattleClientInterfaceData
 class DLL_LINKAGE CBattleInfoCallback : public virtual CBattleInfoEssentials
 {
 public:
-	std::optional<int> battleIsFinished() const override; //return none if battle is ongoing; otherwise the victorious side (0/1) or 2 if it is a draw
+	std::optional<BattleSide> battleIsFinished() const override; //return none if battle is ongoing; otherwise the victorious side (0/1) or 2 if it is a draw
 
 	std::vector<std::shared_ptr<const CObstacleInstance>> battleGetAllObstaclesOnPos(BattleHex tile, bool onlyBlocking = true) const override;
 	std::vector<std::shared_ptr<const CObstacleInstance>> getAllAffectedObstaclesByStack(const battle::Unit * unit, const std::set<BattleHex> & passed) const override;
@@ -70,9 +70,9 @@ public:
 	///returns all alive units excluding turrets
 	battle::Units battleAliveUnits() const;
 	///returns all alive units from particular side excluding turrets
-	battle::Units battleAliveUnits(ui8 side) const;
+	battle::Units battleAliveUnits(BattleSide side) const;
 
-	void battleGetTurnOrder(std::vector<battle::Units> & out, const size_t maxUnits, const int maxTurns, const int turn = 0, int8_t lastMoved = -1) const;
+	void battleGetTurnOrder(std::vector<battle::Units> & out, const size_t maxUnits, const int maxTurns, const int turn = 0, BattleSide lastMoved = BattleSide::NONE) const;
 
 	///returns reachable hexes (valid movement destinations), DOES contain stack current position
 	std::vector<BattleHex> battleGetAvailableHexes(const battle::Unit * unit, bool obtainMovementRange, bool addOccupiable, std::vector<BattleHex> * attackable) const;
@@ -116,8 +116,8 @@ public:
 	bool isWallPartAttackable(EWallPart wallPart) const; // returns true if the wall part is actually attackable, false if not
 	std::vector<BattleHex> getAttackableBattleHexes() const;
 
-	si8 battleMinSpellLevel(ui8 side) const; //calculates maximum spell level possible to be cast on battlefield - takes into account artifacts of both heroes; if no effects are set, 0 is returned
+	si8 battleMinSpellLevel(BattleSide side) const; //calculates maximum spell level possible to be cast on battlefield - takes into account artifacts of both heroes; if no effects are set, 0 is returned
-	si8 battleMaxSpellLevel(ui8 side) const; //calculates minimum spell level possible to be cast on battlefield - takes into account artifacts of both heroes; if no effects are set, 0 is returned
+	si8 battleMaxSpellLevel(BattleSide side) const; //calculates minimum spell level possible to be cast on battlefield - takes into account artifacts of both heroes; if no effects are set, 0 is returned
 	int32_t battleGetSpellCost(const spells::Spell * sp, const CGHeroInstance * caster) const; //returns cost of given spell
 	ESpellCastProblem battleCanCastSpell(const spells::Caster * caster, spells::Mode mode) const; //returns true if there are no general issues preventing from casting a spell
 
@@ -163,12 +163,12 @@ public:
 	AccessibilityInfo getAccessibility(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 getAvailableHex(const CreatureID & creID, ui8 side, int initialPos = -1) const; //find place for adding new stack
+	BattleHex getAvailableHex(const CreatureID & creID, BattleSide 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;
 	bool isInObstacle(BattleHex hex, const std::set<BattleHex> & obstacles, const ReachabilityInfo::Parameters & params) const;
-	std::set<BattleHex> getStoppers(BattlePerspective::BattlePerspective whichSidePerspective) const; //get hexes with stopping obstacles (quicksands)
+	std::set<BattleHex> getStoppers(BattleSide whichSidePerspective) const; //get hexes with stopping obstacles (quicksands)
 };
 
 VCMI_LIB_NAMESPACE_END

lib/battle/CBattleInfoEssentials.cpp

@@ -35,13 +35,13 @@ BattleField CBattleInfoEssentials::battleGetBattlefieldType() const
 	return getBattle()->getBattlefieldType();
 }
 
-int32_t CBattleInfoEssentials::battleGetEnchanterCounter(ui8 side) const
+int32_t CBattleInfoEssentials::battleGetEnchanterCounter(BattleSide side) const
 {
 	RETURN_IF_NOT_BATTLE(0);
 	return getBattle()->getEnchanterCounter(side);
 }
 
-std::vector<std::shared_ptr<const CObstacleInstance>> CBattleInfoEssentials::battleGetAllObstacles(std::optional<BattlePerspective::BattlePerspective> perspective) const
+std::vector<std::shared_ptr<const CObstacleInstance>> CBattleInfoEssentials::battleGetAllObstacles(std::optional<BattleSide> perspective) const
 {
 	std::vector<std::shared_ptr<const CObstacleInstance> > ret;
 	RETURN_IF_NOT_BATTLE(ret);
@@ -82,13 +82,13 @@ std::shared_ptr<const CObstacleInstance> CBattleInfoEssentials::battleGetObstacl
 	return std::shared_ptr<const CObstacleInstance>();
 }
 
-bool CBattleInfoEssentials::battleIsObstacleVisibleForSide(const CObstacleInstance & coi, BattlePerspective::BattlePerspective side) const
+bool CBattleInfoEssentials::battleIsObstacleVisibleForSide(const CObstacleInstance & coi, BattleSide side) const
 {
 	RETURN_IF_NOT_BATTLE(false);
-	return side == BattlePerspective::ALL_KNOWING || coi.visibleForSide(side, battleHasNativeStack(side));
+	return side == BattleSide::ALL_KNOWING || coi.visibleForSide(side, battleHasNativeStack(side));
 }
 
-bool CBattleInfoEssentials::battleHasNativeStack(ui8 side) const
+bool CBattleInfoEssentials::battleHasNativeStack(BattleSide side) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 
@@ -160,18 +160,18 @@ const CGTownInstance * CBattleInfoEssentials::battleGetDefendedTown() const
 	return getBattle()->getDefendedTown();
 }
 
-BattlePerspective::BattlePerspective CBattleInfoEssentials::battleGetMySide() const
+BattleSide CBattleInfoEssentials::battleGetMySide() const
 {
-	RETURN_IF_NOT_BATTLE(BattlePerspective::INVALID);
+	RETURN_IF_NOT_BATTLE(BattleSide::INVALID);
 	if(!getPlayerID() || getPlayerID()->isSpectator())
-		return BattlePerspective::ALL_KNOWING;
+		return BattleSide::ALL_KNOWING;
 	if(*getPlayerID() == getBattle()->getSidePlayer(BattleSide::ATTACKER))
-		return BattlePerspective::LEFT_SIDE;
+		return BattleSide::LEFT_SIDE;
 	if(*getPlayerID() == getBattle()->getSidePlayer(BattleSide::DEFENDER))
-		return BattlePerspective::RIGHT_SIDE;
+		return BattleSide::RIGHT_SIDE;
 
 	logGlobal->error("Cannot find player %s in battle!", getPlayerID()->toString());
-	return BattlePerspective::INVALID;
+	return BattleSide::INVALID;
 }
 
 const CStack* CBattleInfoEssentials::battleGetStackByID(int ID, bool onlyAlive) const
@@ -189,11 +189,11 @@ const CStack* CBattleInfoEssentials::battleGetStackByID(int ID, bool onlyAlive)
 		return stacks[0];
 }
 
-bool CBattleInfoEssentials::battleDoWeKnowAbout(ui8 side) const
+bool CBattleInfoEssentials::battleDoWeKnowAbout(BattleSide side) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 	auto p = battleGetMySide();
-	return p == BattlePerspective::ALL_KNOWING || p == side;
+	return p == BattleSide::ALL_KNOWING || p == side;
 }
 
 si8 CBattleInfoEssentials::battleTacticDist() const
@@ -202,16 +202,16 @@ si8 CBattleInfoEssentials::battleTacticDist() const
 	return getBattle()->getTacticDist();
 }
 
-si8 CBattleInfoEssentials::battleGetTacticsSide() const
+BattleSide CBattleInfoEssentials::battleGetTacticsSide() const
 {
-	RETURN_IF_NOT_BATTLE(-1);
+	RETURN_IF_NOT_BATTLE(BattleSide::NONE);
 	return getBattle()->getTacticsSide();
 }
 
-const CGHeroInstance * CBattleInfoEssentials::battleGetFightingHero(ui8 side) const
+const CGHeroInstance * CBattleInfoEssentials::battleGetFightingHero(BattleSide side) const
 {
 	RETURN_IF_NOT_BATTLE(nullptr);
-	if(side > 1)
+	if(side != BattleSide::DEFENDER && side != BattleSide::ATTACKER)
 	{
 		logGlobal->error("FIXME: %s wrong argument!", __FUNCTION__);
 		return nullptr;
@@ -226,10 +226,10 @@ const CGHeroInstance * CBattleInfoEssentials::battleGetFightingHero(ui8 side) co
 	return getBattle()->getSideHero(side);
 }
 
-const CArmedInstance * CBattleInfoEssentials::battleGetArmyObject(ui8 side) const
+const CArmedInstance * CBattleInfoEssentials::battleGetArmyObject(BattleSide side) const
 {
 	RETURN_IF_NOT_BATTLE(nullptr);
-	if(side > 1)
+	if(side != BattleSide::DEFENDER && side != BattleSide::ATTACKER)
 	{
 		logGlobal->error("FIXME: %s wrong argument!", __FUNCTION__);
 		return nullptr;
@@ -242,7 +242,7 @@ const CArmedInstance * CBattleInfoEssentials::battleGetArmyObject(ui8 side) cons
 	return getBattle()->getSideArmy(side);
 }
 
-InfoAboutHero CBattleInfoEssentials::battleGetHeroInfo(ui8 side) const
+InfoAboutHero CBattleInfoEssentials::battleGetHeroInfo(BattleSide side) const
 {
 	const auto * hero = getBattle()->getSideHero(side);
 	if(!hero)
@@ -253,7 +253,7 @@ InfoAboutHero CBattleInfoEssentials::battleGetHeroInfo(ui8 side) const
 	return InfoAboutHero(hero, infoLevel);
 }
 
-uint32_t CBattleInfoEssentials::battleCastSpells(ui8 side) const
+uint32_t CBattleInfoEssentials::battleCastSpells(BattleSide side) const
 {
 	RETURN_IF_NOT_BATTLE(-1);
 	return getBattle()->getCastSpells(side);
@@ -268,10 +268,10 @@ bool CBattleInfoEssentials::battleCanFlee(const PlayerColor & player) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 	const auto side = playerToSide(player);
-	if(!side)
+	if(side == BattleSide::NONE)
 		return false;
 
-	const CGHeroInstance * myHero = battleGetFightingHero(side.value());
+	const CGHeroInstance * myHero = battleGetFightingHero(side);
 
 	//current player have no hero
 	if(!myHero)
@@ -292,28 +292,28 @@ bool CBattleInfoEssentials::battleCanFlee(const PlayerColor & player) const
 	return true;
 }
 
-BattleSideOpt CBattleInfoEssentials::playerToSide(const PlayerColor & player) const
+BattleSide CBattleInfoEssentials::playerToSide(const PlayerColor & player) const
 {
-	RETURN_IF_NOT_BATTLE(std::nullopt);
+	RETURN_IF_NOT_BATTLE(BattleSide::NONE);
 
 	if(getBattle()->getSidePlayer(BattleSide::ATTACKER) == player)
-		return BattleSideOpt(BattleSide::ATTACKER);
+		return BattleSide::ATTACKER;
 
 	if(getBattle()->getSidePlayer(BattleSide::DEFENDER) == player)
-		return BattleSideOpt(BattleSide::DEFENDER);
+		return BattleSide::DEFENDER;
 
 	logGlobal->warn("Cannot find side for player %s", player.toString());
 
-	return std::nullopt;
+	return BattleSide::INVALID;
 }
 
-PlayerColor CBattleInfoEssentials::sideToPlayer(ui8 side) const
+PlayerColor CBattleInfoEssentials::sideToPlayer(BattleSide side) const
 {
 	RETURN_IF_NOT_BATTLE(PlayerColor::CANNOT_DETERMINE);
     return getBattle()->getSidePlayer(side);
 }
 
-ui8 CBattleInfoEssentials::otherSide(ui8 side) const
+BattleSide CBattleInfoEssentials::otherSide(BattleSide side)
 {
 	if(side == BattleSide::ATTACKER)
 		return BattleSide::DEFENDER;
@@ -326,19 +326,19 @@ PlayerColor CBattleInfoEssentials::otherPlayer(const PlayerColor & player) const
 	RETURN_IF_NOT_BATTLE(PlayerColor::CANNOT_DETERMINE);
 
 	auto side = playerToSide(player);
-    if(!side)
+	if(side == BattleSide::NONE)
 		return PlayerColor::CANNOT_DETERMINE;
 
-	return getBattle()->getSidePlayer(otherSide(side.value()));
+	return getBattle()->getSidePlayer(otherSide(side));
 }
 
 bool CBattleInfoEssentials::playerHasAccessToHeroInfo(const PlayerColor & player, const CGHeroInstance * h) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 	const auto side = playerToSide(player);
-	if(side)
+	if(side != BattleSide::NONE)
 	{
-		auto opponentSide = otherSide(side.value());
+		auto opponentSide = otherSide(side);
 		if(getBattle()->getSideHero(opponentSide) == h)
 			return true;
 	}
@@ -355,14 +355,14 @@ bool CBattleInfoEssentials::battleCanSurrender(const PlayerColor & player) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 	const auto side = playerToSide(player);
-	if(!side)
+	if(side == BattleSide::NONE)
 		return false;
-	bool iAmSiegeDefender = (side.value() == BattleSide::DEFENDER && getBattle()->getDefendedTown() != nullptr);
+	bool iAmSiegeDefender = (side == BattleSide::DEFENDER && getBattle()->getDefendedTown() != nullptr);
 	//conditions like for fleeing (except escape tunnel presence) + enemy must have a hero
-	return battleCanFlee(player) && !iAmSiegeDefender && battleHasHero(otherSide(side.value()));
+	return battleCanFlee(player) && !iAmSiegeDefender && battleHasHero(otherSide(side));
 }
 
-bool CBattleInfoEssentials::battleHasHero(ui8 side) const
+bool CBattleInfoEssentials::battleHasHero(BattleSide side) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 	return getBattle()->getSideHero(side) != nullptr;
@@ -413,9 +413,9 @@ const CGHeroInstance * CBattleInfoEssentials::battleGetOwnerHero(const battle::U
 {
 	RETURN_IF_NOT_BATTLE(nullptr);
 	const auto side = playerToSide(battleGetOwner(unit));
-	if(!side)
+	if(side == BattleSide::NONE)
 		return nullptr;
-	return getBattle()->getSideHero(side.value());
+	return getBattle()->getSideHero(side);
 }
 
 bool CBattleInfoEssentials::battleMatchOwner(const battle::Unit * attacker, const battle::Unit * defender, const boost::logic::tribool positivness) const

lib/battle/CBattleInfoEssentials.h

@@ -9,6 +9,7 @@
  */
 #pragma once
 #include "IBattleInfoCallback.h"
+#include "BattleSide.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -22,21 +23,10 @@ class CArmedInstance;
 using TStacks = std::vector<const CStack *>;
 using TStackFilter = std::function<bool (const CStack *)>;
 
-namespace BattlePerspective
-{
-	enum BattlePerspective
-	{
-		INVALID = -2,
-		ALL_KNOWING = -1,
-		LEFT_SIDE,
-		RIGHT_SIDE
-	};
-}
-
 class DLL_LINKAGE CBattleInfoEssentials : public IBattleInfoCallback
 {
 protected:
-	bool battleDoWeKnowAbout(ui8 side) const;
+	bool battleDoWeKnowAbout(BattleSide side) const;
 
 public:
 	enum EStackOwnership
@@ -45,14 +35,14 @@ public:
 	};
 
 	bool duringBattle() const;
-	BattlePerspective::BattlePerspective battleGetMySide() const;
+	BattleSide battleGetMySide() const;
 	const IBonusBearer * getBonusBearer() const override;
 
 	TerrainId battleTerrainType() const override;
 	BattleField battleGetBattlefieldType() const override;
-	int32_t battleGetEnchanterCounter(ui8 side) const;
+	int32_t battleGetEnchanterCounter(BattleSide side) const;
 
-	std::vector<std::shared_ptr<const CObstacleInstance>> battleGetAllObstacles(std::optional<BattlePerspective::BattlePerspective> perspective = std::nullopt) const; //returns all obstacles on the battlefield
+	std::vector<std::shared_ptr<const CObstacleInstance>> battleGetAllObstacles(std::optional<BattleSide> perspective = std::nullopt) const; //returns all obstacles on the battlefield
 
 	std::shared_ptr<const CObstacleInstance> battleGetObstacleByID(uint32_t ID) const;
 
@@ -70,27 +60,27 @@ public:
 
 	uint32_t battleNextUnitId() const override;
 
-	bool battleHasNativeStack(ui8 side) const;
+	bool battleHasNativeStack(BattleSide side) const;
 	const CGTownInstance * battleGetDefendedTown() const; //returns defended town if current battle is a siege, nullptr instead
 
 	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 (?)
+	BattleSide battleGetTacticsSide() const override; //returns which side is in tactics phase, undefined if none (?)
 
 	bool battleCanFlee(const PlayerColor & player) const;
 	bool battleCanSurrender(const PlayerColor & player) const;
 
-	ui8 otherSide(ui8 side) const;
+	static BattleSide otherSide(BattleSide side);
 	PlayerColor otherPlayer(const PlayerColor & player) const;
 
-	BattleSideOpt playerToSide(const PlayerColor & player) const;
+	BattleSide playerToSide(const PlayerColor & player) const;
-	PlayerColor sideToPlayer(ui8 side) const;
+	PlayerColor sideToPlayer(BattleSide side) 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;
+	bool battleHasHero(BattleSide side) const;
-	uint32_t battleCastSpells(ui8 side) const; //how many spells has given side cast
+	uint32_t battleCastSpells(BattleSide side) const; //how many spells has given side cast
-	const CGHeroInstance * battleGetFightingHero(ui8 side) const; //deprecated for players callback, easy to get wrong
+	const CGHeroInstance * battleGetFightingHero(BattleSide side) const; //deprecated for players callback, easy to get wrong
-	const CArmedInstance * battleGetArmyObject(ui8 side) const;
+	const CArmedInstance * battleGetArmyObject(BattleSide side) const;
-	InfoAboutHero battleGetHeroInfo(ui8 side) const;
+	InfoAboutHero battleGetHeroInfo(BattleSide side) const;
 
 	// for determining state of a part of the wall; format: parameter [0] - keep, [1] - bottom tower, [2] - bottom wall,
 	// [3] - below gate, [4] - over gate, [5] - upper wall, [6] - uppert tower, [7] - gate; returned value: 1 - intact, 2 - damaged, 3 - destroyed; 0 - no battle
@@ -103,7 +93,7 @@ public:
 	TStacks battleGetAllStacks(bool includeTurrets = false) const;
 
 	const CStack * battleGetStackByID(int ID, bool onlyAlive = true) const; //returns stack info by given ID
-	bool battleIsObstacleVisibleForSide(const CObstacleInstance & coi, BattlePerspective::BattlePerspective side) const;
+	bool battleIsObstacleVisibleForSide(const CObstacleInstance & coi, BattleSide side) const;
 
 	///returns player that controls given stack; mind control included
 	PlayerColor battleGetOwner(const battle::Unit * unit) const;

lib/battle/CObstacleInstance.cpp

@@ -52,7 +52,7 @@ std::vector<BattleHex> CObstacleInstance::getAffectedTiles() const
 	}
 }
 
-bool CObstacleInstance::visibleForSide(ui8 side, bool hasNativeStack) const
+bool CObstacleInstance::visibleForSide(BattleSide side, bool hasNativeStack) const
 {
 	//by default obstacle is visible for everyone
 	return true;
@@ -134,7 +134,7 @@ SpellCreatedObstacle::SpellCreatedObstacle()
 	: turnsRemaining(-1),
 	casterSpellPower(0),
 	spellLevel(0),
-	casterSide(0),
+	casterSide(BattleSide::NONE),
 	hidden(false),
 	passable(false),
 	trigger(false),
@@ -148,7 +148,7 @@ SpellCreatedObstacle::SpellCreatedObstacle()
 	obstacleType = SPELL_CREATED;
 }
 
-bool SpellCreatedObstacle::visibleForSide(ui8 side, bool hasNativeStack) const
+bool SpellCreatedObstacle::visibleForSide(BattleSide side, bool hasNativeStack) const
 {
 	//we hide mines and not discovered quicksands
 	//quicksands are visible to the caster or if owned unit stepped into that particular patch

lib/battle/CObstacleInstance.h

@@ -50,7 +50,7 @@ struct DLL_LINKAGE CObstacleInstance : public Serializeable
 	virtual SpellID getTrigger() const;
 
 	virtual std::vector<BattleHex> getAffectedTiles() const;
-	virtual bool visibleForSide(ui8 side, bool hasNativeStack) const; //0 attacker
+	virtual bool visibleForSide(BattleSide side, bool hasNativeStack) const; //0 attacker
 
 	virtual void battleTurnPassed(){};
 
@@ -80,7 +80,7 @@ struct DLL_LINKAGE SpellCreatedObstacle : CObstacleInstance
 	int32_t casterSpellPower;
 	int32_t spellLevel;
 	int32_t minimalDamage; //How many damage should it do regardless of power and level of caster
-	si8 casterSide; //0 - obstacle created by attacker; 1 - by defender
+	BattleSide casterSide;
 
 	SpellID trigger;
 
@@ -102,7 +102,7 @@ struct DLL_LINKAGE SpellCreatedObstacle : CObstacleInstance
 	SpellCreatedObstacle();
 
 	std::vector<BattleHex> getAffectedTiles() const override;
-	bool visibleForSide(ui8 side, bool hasNativeStack) const override;
+	bool visibleForSide(BattleSide side, bool hasNativeStack) const override;
 
 	bool blocksTiles() const override;
 	bool stopsMovement() const override;

lib/battle/CPlayerBattleCallback.cpp

@@ -76,7 +76,7 @@ const CGHeroInstance * CPlayerBattleCallback::battleGetMyHero() const
 
 InfoAboutHero CPlayerBattleCallback::battleGetEnemyHero() const
 {
-	return battleGetHeroInfo(!battleGetMySide());
+	return battleGetHeroInfo(otherSide(battleGetMySide()));
 }
 
 

lib/battle/CUnitState.cpp

@@ -921,7 +921,7 @@ uint32_t CUnitStateDetached::unitId() const
 	return unit->unitId();
 }
 
-ui8 CUnitStateDetached::unitSide() const
+BattleSide CUnitStateDetached::unitSide() const
 {
 	return unit->unitSide();
 }

lib/battle/CUnitState.h

@@ -289,7 +289,7 @@ public:
 	CUnitStateDetached & operator= (const CUnitState & other);
 
 	uint32_t unitId() const override;
-	ui8 unitSide() const override;
+	BattleSide unitSide() const override;
 
 	const CCreature * unitType() const override;
 	PlayerColor unitOwner() const override;

lib/battle/IBattleInfoCallback.h

@@ -65,10 +65,10 @@ public:
 	virtual BattleField battleGetBattlefieldType() const = 0;
 
 	///return none if battle is ongoing; otherwise the victorious side (0/1) or 2 if it is a draw
-	virtual std::optional<int> battleIsFinished() const = 0;
+	virtual std::optional<BattleSide> battleIsFinished() const = 0;
 
 	virtual si8 battleTacticDist() const = 0; //returns tactic distance in current tactics phase; 0 if not in tactics phase
-	virtual si8 battleGetTacticsSide() const = 0; //returns which side is in tactics phase, undefined if none (?)
+	virtual BattleSide battleGetTacticsSide() const = 0; //returns which side is in tactics phase, undefined if none (?)
 
 	virtual uint32_t battleNextUnitId() const = 0;
 

lib/battle/IBattleState.h

@@ -55,17 +55,17 @@ public:
 	virtual EWallState getWallState(EWallPart partOfWall) const = 0;
 	virtual EGateState getGateState() const = 0;
 
-	virtual PlayerColor getSidePlayer(ui8 side) const = 0;
+	virtual PlayerColor getSidePlayer(BattleSide side) const = 0;
-	virtual const CArmedInstance * getSideArmy(ui8 side) const = 0;
+	virtual const CArmedInstance * getSideArmy(BattleSide side) const = 0;
-	virtual const CGHeroInstance * getSideHero(ui8 side) const = 0;
+	virtual const CGHeroInstance * getSideHero(BattleSide side) const = 0;
 	/// Returns list of all spells used by specified side (and that can be learned by opposite hero)
-	virtual std::vector<SpellID> getUsedSpells(ui8 side) const = 0;
+	virtual std::vector<SpellID> getUsedSpells(BattleSide side) const = 0;
 
-	virtual uint32_t getCastSpells(ui8 side) const = 0;
+	virtual uint32_t getCastSpells(BattleSide side) const = 0;
-	virtual int32_t getEnchanterCounter(ui8 side) const = 0;
+	virtual int32_t getEnchanterCounter(BattleSide side) const = 0;
 
 	virtual ui8 getTacticDist() const = 0;
-	virtual ui8 getTacticsSide() const = 0;
+	virtual BattleSide getTacticsSide() const = 0;
 
 	virtual uint32_t nextUnitId() const = 0;
 

lib/battle/IUnitInfo.h

@@ -11,6 +11,7 @@
 #pragma once
 
 #include "../GameConstants.h"
+#include "BattleSide.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -35,7 +36,7 @@ public:
 	virtual int32_t unitBaseAmount() const = 0;
 
 	virtual uint32_t unitId() const = 0;
-	virtual ui8 unitSide() const = 0;
+	virtual BattleSide unitSide() const = 0;
 	virtual PlayerColor unitOwner() const = 0;
 	virtual SlotID unitSlot() const = 0;
 

lib/battle/ReachabilityInfo.cpp

@@ -15,7 +15,7 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 ReachabilityInfo::Parameters::Parameters(const battle::Unit * Stack, BattleHex StartPosition):
-	perspective(static_cast<BattlePerspective::BattlePerspective>(Stack->unitSide())),
+	perspective(static_cast<BattleSide>(Stack->unitSide())),
 	startPosition(StartPosition),
 	doubleWide(Stack->doubleWide()),
 	side(Stack->unitSide()),

lib/battle/ReachabilityInfo.h

@@ -25,14 +25,14 @@ struct DLL_LINKAGE ReachabilityInfo
 
 	struct DLL_LINKAGE Parameters
 	{
-		ui8 side = 0;
+		BattleSide side = BattleSide::NONE;
 		bool doubleWide = false;
 		bool flying = false;
 		bool ignoreKnownAccessible = false; //Ignore obstacles if it is in accessible hexes
 		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 = BattlePerspective::ALL_KNOWING; //some obstacles (eg. quicksands) may be invisible for some side
+		BattleSide perspective = BattleSide::ALL_KNOWING; //some obstacles (eg. quicksands) may be invisible for some side
 
 		Parameters() = default;
 		Parameters(const battle::Unit * Stack, BattleHex StartPosition);

lib/battle/Unit.cpp

@@ -41,7 +41,7 @@ bool Unit::isTurret() const
 std::string Unit::getDescription() const
 {
 	boost::format fmt("Unit %d of side %d");
-	fmt % unitId() % unitSide();
+	fmt % unitId() % static_cast<int>(unitSide());
 	return fmt.str();
 }
 
@@ -58,7 +58,7 @@ std::vector<BattleHex> Unit::getSurroundingHexes(BattleHex assumedPosition) cons
 	return getSurroundingHexes(hex, doubleWide(), unitSide());
 }
 
-std::vector<BattleHex> Unit::getSurroundingHexes(BattleHex position, bool twoHex, ui8 side)
+std::vector<BattleHex> Unit::getSurroundingHexes(BattleHex position, bool twoHex, BattleSide side)
 {
 	std::vector<BattleHex> hexes;
 	if(twoHex)
@@ -135,7 +135,7 @@ std::vector<BattleHex> Unit::getHexes(BattleHex assumedPos) const
 	return getHexes(assumedPos, doubleWide(), unitSide());
 }
 
-std::vector<BattleHex> Unit::getHexes(BattleHex assumedPos, bool twoHex, ui8 side)
+std::vector<BattleHex> Unit::getHexes(BattleHex assumedPos, bool twoHex, BattleSide side)
 {
 	std::vector<BattleHex> hexes;
 	hexes.push_back(assumedPos);
@@ -156,7 +156,7 @@ BattleHex Unit::occupiedHex(BattleHex assumedPos) const
 	return occupiedHex(assumedPos, doubleWide(), unitSide());
 }
 
-BattleHex Unit::occupiedHex(BattleHex assumedPos, bool twoHex, ui8 side)
+BattleHex Unit::occupiedHex(BattleHex assumedPos, bool twoHex, BattleSide side)
 {
 	if(twoHex)
 	{

lib/battle/Unit.h

@@ -129,17 +129,17 @@ public:
 
 	std::vector<BattleHex> getSurroundingHexes(BattleHex assumedPosition = BattleHex::INVALID) const; // get six or 8 surrounding hexes depending on creature size
 	std::vector<BattleHex> getAttackableHexes(const Unit * attacker) const;
-	static std::vector<BattleHex> getSurroundingHexes(BattleHex position, bool twoHex, ui8 side);
+	static std::vector<BattleHex> getSurroundingHexes(BattleHex position, bool twoHex, BattleSide side);
 
 	bool coversPos(BattleHex position) const; //checks also if unit is double-wide
 
 	std::vector<BattleHex> getHexes() const; //up to two occupied hexes, starting from front
 	std::vector<BattleHex> getHexes(BattleHex assumedPos) const; //up to two occupied hexes, starting from front
-	static std::vector<BattleHex> getHexes(BattleHex assumedPos, bool twoHex, ui8 side);
+	static std::vector<BattleHex> getHexes(BattleHex assumedPos, bool twoHex, BattleSide side);
 
 	BattleHex occupiedHex() const; //returns number of occupied hex (not the position) if stack is double wide; otherwise -1
 	BattleHex occupiedHex(BattleHex assumedPos) const; //returns number of occupied hex (not the position) if stack is double wide and would stand on assumedPos; otherwise -1
-	static BattleHex occupiedHex(BattleHex assumedPos, bool twoHex, ui8 side);
+	static BattleHex occupiedHex(BattleHex assumedPos, bool twoHex, BattleSide side);
 
 	///MetaStrings
 	void addText(MetaString & text, EMetaText type, int32_t serial, const boost::logic::tribool & plural = boost::logic::indeterminate) const;
@@ -166,7 +166,7 @@ public:
     uint32_t id = 0;
 	TQuantity count = 0;
 	CreatureID type;
-	ui8 side = 0;
+	BattleSide side = BattleSide::NONE;
 	BattleHex position;
 	bool summoned = false;
 

lib/bonuses/BonusEnum.h

@@ -222,7 +222,7 @@ namespace BonusDuration  //when bonus is automatically removed
 {
 	// We use uint16_t directly because std::bitset<11> eats whole 8 byte word.
 	using Type = uint16_t;
-	constexpr static size_t Size = 11;
+	constexpr size_t Size = 11;
 
 	enum BonusDuration : Type {
 		PERMANENT = 1 << 0,

lib/campaign/CampaignHandler.cpp

@@ -604,15 +604,15 @@ std::vector< std::vector<ui8> > CampaignHandler::getFile(std::unique_ptr<CInputS
 	if (magic == zipHeaderMagic) // ZIP archive - assume VCMP format
 	{
 		CInputStream * buffer(file.get());
-		std::shared_ptr<CIOApi> ioApi(new CProxyROIOApi(buffer));
+		auto ioApi = std::make_shared<CProxyROIOApi>(buffer);
 		CZipLoader loader("", "_", ioApi);
 
 		// load header
-		JsonPath resource = JsonPath::builtin(VCMP_HEADER_FILE_NAME);
+		JsonPath jsonPath = JsonPath::builtin(VCMP_HEADER_FILE_NAME);
-		if(!loader.existsResource(resource))
+		if(!loader.existsResource(jsonPath))
-			throw std::runtime_error(resource.getName() + " not found in " + filename);
+			throw std::runtime_error(jsonPath.getName() + " not found in " + filename);
-		auto data = loader.load(resource)->readAll();
+		auto data = loader.load(jsonPath)->readAll();
-		ret.push_back(std::vector<ui8>(data.first.get(), data.first.get() + data.second));
+		ret.emplace_back(data.first.get(), data.first.get() + data.second);
 
 		if(headerOnly)
 			return ret;
@@ -621,11 +621,11 @@ std::vector< std::vector<ui8> > CampaignHandler::getFile(std::unique_ptr<CInputS
 		JsonNode header(reinterpret_cast<const std::byte*>(data.first.get()), data.second, VCMP_HEADER_FILE_NAME);
 		for(auto scenario : header["scenarios"].Vector())
 		{
-			ResourcePath resource = ResourcePath(scenario["map"].String(), EResType::MAP);
+			ResourcePath mapPath(scenario["map"].String(), EResType::MAP);
-			if(!loader.existsResource(resource))
+			if(!loader.existsResource(mapPath))
-				throw std::runtime_error(resource.getName() + " not found in " + filename);
+				throw std::runtime_error(mapPath.getName() + " not found in " + filename);
-			auto data = loader.load(resource)->readAll();
+			auto data = loader.load(mapPath)->readAll();
-			ret.push_back(std::vector<ui8>(data.first.get(), data.first.get() + data.second));
+			ret.emplace_back(data.first.get(), data.first.get() + data.second);
 		}
 
 		return ret;

lib/campaign/CampaignState.cpp

@@ -45,7 +45,9 @@ CampaignRegions CampaignRegions::fromJson(const JsonNode & node)
 {
 	CampaignRegions cr;
 	cr.campPrefix = node["prefix"].String();
-	cr.colorSuffixLength = static_cast<int>(node["color_suffix_length"].Float());
+	cr.colorSuffixLength = static_cast<int>(node["colorSuffixLength"].Float());
+	cr.campSuffix = node["suffix"].isNull() ? std::vector<std::string>() : std::vector<std::string>{node["suffix"].Vector()[0].String(), node["suffix"].Vector()[1].String(), node["suffix"].Vector()[2].String()};
+	cr.campBackground = node["background"].isNull() ? "" : node["background"].String();
 
 	for(const JsonNode & desc : node["desc"].Vector())
 		cr.regions.push_back(CampaignRegions::RegionDescription::fromJson(desc));
@@ -68,7 +70,10 @@ CampaignRegions CampaignRegions::getLegacy(int campId)
 
 ImagePath CampaignRegions::getBackgroundName() const
 {
-	return ImagePath::builtin(campPrefix + "_BG.BMP");
+	if(campBackground.empty())
+		return ImagePath::builtin(campPrefix + "_BG.BMP");
+	else
+		return ImagePath::builtin(campBackground);
 }
 
 Point CampaignRegions::getPosition(CampaignScenarioID which) const
@@ -81,30 +86,39 @@ ImagePath CampaignRegions::getNameFor(CampaignScenarioID which, int colorIndex,
 {
 	auto const & region = regions[which.getNum()];
 
-	static const std::string colors[2][8] =
+	static const std::array<std::array<std::string, 8>, 3> colors = {{
-	{
+		{ "", "", "", "", "", "", "", "" },
-		{"R", "B", "N", "G", "O", "V", "T", "P"},
+		{ "R", "B", "N", "G", "O", "V", "T", "P" },
-		{"Re", "Bl", "Br", "Gr", "Or", "Vi", "Te", "Pi"}
+		{ "Re", "Bl", "Br", "Gr", "Or", "Vi", "Te", "Pi" }
-	};
+	}};
 
-	std::string color = colors[colorSuffixLength - 1][colorIndex];
+	std::string color = colors[colorSuffixLength][colorIndex];
 
 	return ImagePath::builtin(campPrefix + region.infix + "_" + type + color + ".BMP");
 }
 
 ImagePath CampaignRegions::getAvailableName(CampaignScenarioID which, int color) const
 {
-	return getNameFor(which, color, "En");
+	if(campSuffix.empty())
+		return getNameFor(which, color, "En");
+	else
+		return getNameFor(which, color, campSuffix[0]);
 }
 
 ImagePath CampaignRegions::getSelectedName(CampaignScenarioID which, int color) const
 {
-	return getNameFor(which, color, "Se");
+	if(campSuffix.empty())
+		return getNameFor(which, color, "Se");
+	else
+		return getNameFor(which, color, campSuffix[1]);
 }
 
 ImagePath CampaignRegions::getConqueredName(CampaignScenarioID which, int color) const
 {
-	return getNameFor(which, color, "Co");
+	if(campSuffix.empty())
+		return getNameFor(which, color, "Co");
+	else
+		return getNameFor(which, color, campSuffix[2]);
 }
 
 

lib/campaign/CampaignState.h

@@ -33,6 +33,8 @@ class IGameCallback;
 class DLL_LINKAGE CampaignRegions
 {
 	std::string campPrefix;
+	std::vector<std::string> campSuffix;
+	std::string campBackground;
 	int colorSuffixLength;
 
 	struct DLL_LINKAGE RegionDescription
@@ -67,6 +69,11 @@ public:
 		h & campPrefix;
 		h & colorSuffixLength;
 		h & regions;
+		if (h.version >= Handler::Version::CAMPAIGN_REGIONS)
+		{
+			h & campSuffix;
+			h & campBackground;
+		}
 	}
 
 	static CampaignRegions fromJson(const JsonNode & node);