Merge remote-tracking branch 'upstream/develop' into develop

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
 		{

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

@@ -741,7 +741,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 < GameConstants::CREATURES_PER_TOWN; level++)
+	for(int level = 0; level < developmentInfo.town->town->creatures.size(); 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
 
@@ -217,8 +215,8 @@ BuildingInfo BuildAnalyzer::getBuildingOrPrerequisite(
 
 	if(BuildingID::DWELL_FIRST <= toBuild && toBuild <= BuildingID::DWELL_UP_LAST)
 	{
-		creatureLevel = (toBuild - BuildingID::DWELL_FIRST) % GameConstants::CREATURES_PER_TOWN;
-		creatureUpgrade = (toBuild - BuildingID::DWELL_FIRST) / GameConstants::CREATURES_PER_TOWN;
+		creatureLevel = BuildingID::getLevelFromDwelling(toBuild);
+		creatureUpgrade = BuildingID::getUpgradedFromDwelling(toBuild);
 	}
 	else if(toBuild == BuildingID::HORDE_1 || toBuild == BuildingID::HORDE_1_UPGR)
 	{

AI/Nullkiller/Behaviors/CaptureObjectsBehavior.cpp

@@ -204,7 +204,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

@@ -151,12 +151,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;
@@ -360,7 +358,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);
 				}
@@ -368,7 +366,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;
 

ChangeLog.md

@@ -4,9 +4,25 @@
 * Saved game size reduced by approximately 3 times, especially for large maps or games with a large number of mods.
 * Added option to start vcmi server on randomly selected TCP port
 * Fixed potential desynchronization between server and clients on randomization of map objects if client and server run on different operating systems
+* It is now possible to generate game statistics using `!statistic` command in game chat
+
+### Stability
+* Fixed possible crash on connecting bluetooth mouse during gameplay on Android
+* VCMI will now write more detailed information to log file on crash due to uncaught exception
+
+### Multiplayer
+* Implemented handicap system, with options to reduce income and growth in addition to starting resources restriction
+
+### Mechanics
+* Arrow tower will now prefer to attack more units that are viewed most dangerous instead of simply attacking top-most unit
+* Score in campaigns will now be correctly tracked for games loaded from a save
+* Fixed incorrect direction of Dragon Breath attack in some cases if wide creature attacks another wide creature
+* Map events and town events are now triggered on start of turn of player affected by event, in line with H3 instead of triggering on start of new day for all players
 
 ### Interface
 * Implemented spell quick selection panel in combat
+* Implemented adventure map overlay accessible via Alt key that highlights all interactive objects on screen
+* Added hotkeys to reorder list of owned towns or heroes
 * The number of units resurrected using the Life Drain ability is now written to the combat log.
 * Fixed playback of audio stream with different formats from video files in some Heroes 3 versions
 * Video playback will not be replaced by a black square when another dialogue box is on top of the video.
@@ -19,17 +35,40 @@
 * Semi-transparent shadows now correctly update their transparency during fading effects, such as resource pickups
 * Game will now save all names for human player in hotseat mode
 * Added unassigned by default shortcuts for toggling visibility of visitable and blocked tiles
+* Spellbook button in battle is now blocked if hero has no spellbook 
+* Adventure map will no longer scroll if window is not in focus
+
+### Random Maps Generator
+* Implemented connection option 'forcePortal'
+* It is now possible to connect zone to itself using pair of portals
 
 ### AI
 * Fixed bug where BattleAI attempts to move double-wide unit to an unreachable hex
 * Fixed several cases where Nullkiller AI can count same dangerous object twice, doubling expected army loss.
 * Nullkiller is now capable of visiting configurable objects from mods
+* Nullkiller now uses whirlpools for map movement
+* AI can now correctly estimate effect of Dragon Breath and other similar abilities
+
+### Map Editor
+* Implemented tracking of building requirements for Building Dialog
+* Added build/demolish/enable/disable all buildings options to Building Dialog in town properties
+* It is now possible to set spells allowed or required to be present in Mages Guild
+* It is now possible to add timed events to a town
+* Fixed editor not marking mod as dependency if spells from mod are used in town Mages Guild or in hero starting spells
 
 ### Modding
+* Fixed multiple issues with configurable town buildings
+* Added documentation for configurable town buildings. See docs/Moddders/Entities_Format/Town_Buildings_Format.md
+* Replaced some of hardcoded town buildings with configurable buildings. These building types are now deprecated and will be removed in future.
 * Added support for custom music and opening sound for a battlefield
 * Added support for multiple music tracks for towns
 * Added support for multiple music tracks for terrains on adventure map
 * Fixed several cases where vcmi will report errors in json without specifying filename of invalid file
+* It is now possible to use .zip archive for VCMI campaigns instead of raw gzip stream
+* Added support for custom region definitions (such as background images) for VCMI campaigns 
+* It is now possible to change minimal values of primary skills for heroes
+* Added support for HotA bank building from Factory
+* Added support for HotA-style 8th creature in town
 
 # 1.5.5 -> 1.5.6
 

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];
-	auto & rightSide = info->sides[1];
+	const auto & leftSide = info->getSide(BattleSide::LEFT_SIDE);
+	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(rightSide.color, 1);
-	callBattleStart(PlayerColor::UNFLAGGABLE, 1);
+	callBattleStart(leftSide.color, BattleSide::LEFT_SIDE);
+	callBattleStart(rightSide.color, BattleSide::RIGHT_SIDE);
+	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)
-		if(battleCallbacks.count(side.color))
-			battleCallbacks[side.color]->onBattleEnded(battleID);
+	for(auto side : { BattleSide::ATTACKER, BattleSide::DEFENDER })
+	{
+		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)->sides[1].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)->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,
-		pack.info->tile, pack.info->sides[0].hero, pack.info->sides[1].hero);
-	callOnlyThatBattleInterface(cl, pack.info->sides[1].color, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->sides[0].armyObject, pack.info->sides[1].armyObject,
-		pack.info->tile, pack.info->sides[0].hero, pack.info->sides[1].hero);
-	callOnlyThatBattleInterface(cl, PlayerColor::SPECTATOR, &IBattleEventsReceiver::battleStartBefore, pack.battleID, pack.info->sides[0].armyObject, pack.info->sides[1].armyObject,
-		pack.info->tile, pack.info->sides[0].hero, pack.info->sides[1].hero);
+	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->getSide(BattleSide::ATTACKER).hero, pack.info->getSide(BattleSide::DEFENDER).hero);
+	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->getSide(BattleSide::ATTACKER).hero, pack.info->getSide(BattleSide::DEFENDER).hero);
+	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->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()
-			? gs.getBattle(pack.battleID)->sides[1].color
-			: gs.getBattle(pack.battleID)->sides[0].color);
+		playerToCall = gs.getBattle(pack.battleID)->getSide(BattleSide::ATTACKER).color == activated->unitOwner()
+			? gs.getBattle(pack.battleID)->getSide(BattleSide::DEFENDER).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_SW, townLowerPos > -1, "altDown", "vcmi.radialWheel.moveDown", [townIndex, townLowerPos](){ LOCPLINT->localState->swapOwnedTowns(townIndex, townLowerPos); } },
-		{ RadialMenuConfig::ITEM_ALT_SS, townLowerPos > -1, "altDownBottom", "vcmi.radialWheel.moveBottom", [townIndex, towns]()
+		{ 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", [updateList, townIndex, townLowerPos](){ LOCPLINT->localState->swapOwnedTowns(townIndex, townLowerPos); updateList(); } },
+		{ 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->side = owner.defendingHeroInstance ? (owner.curInt->playerID == owner.defendingHeroInstance->tempOwner) : false;
+	heroSpellToCast->stackNumber = -1;
+	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 ? "SP07_B.DEF" : "SP07_A.DEF"), rightHero));
+			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 == 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));
-			sideNames[i] = heroInfo.name;
+			icons.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), heroInfo.getIconIndex(), 0, xs[static_cast<int>(i)], 38));
+			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));
-				sideNames[i] = (*best)->unitType()->getNamePluralTranslated();
+				icons.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("TWCRPORT"), (*best)->unitType()->getIconIndex(), 0, xs[static_cast<int>(i)], 38));
+				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

@@ -293,5 +293,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,
-	std::optional<ColorRGBA> background)
+	const std::string & text,
+	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
-		? std::next(beginIt, GameConstants::CREATURES_PER_TOWN)
+	const auto afterLastIt = town->creatures.size() > town->town->creatures.size()
+		? 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(404,421));
+		positions.push_back(Point(206,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/factions/castle.json

@@ -174,7 +174,20 @@
 				"horde1":         { "id" : 18, "upgrades" : "dwellingLvl3" },
 				"horde1Upgr":     { "id" : 19, "upgrades" : "dwellingUpLvl3", "requires" : [ "horde1" ], "mode" : "auto" },
 				"ship":           { "id" : 20, "upgrades" : "shipyard" },
-				"special2":       { "type" : "stables", "requires" : [ "dwellingLvl4" ] },
+				"special2":       {
+					"type" : "configurable",
+					"requires" : [ "dwellingLvl4" ],
+					"configuration" : {
+						"visitMode" : "bonus",
+						"rewards" : [
+							{
+								"message" : "@core.genrltxt.580",
+								"movePoints" : 400,
+								"bonuses" : [ { "type" : "MOVEMENT", "subtype" : "heroMovementLand",  "val" : 400, "valueType" : "ADDITIVE_VALUE", "duration" : "ONE_WEEK"} ]
+							}
+						]
+					}
+				},
 				"special3":       { "type" : "brotherhoodOfSword", "upgrades" : "tavern" },
 				"grail":          { "id" : 26, "mode" : "grail", "produce": { "gold": 5000 }, "bonuses": [ { "type": "MORALE", "val": 2, "propagator": "PLAYER_PROPAGATOR" } ] },
 

config/factions/dungeon.json

@@ -174,9 +174,39 @@
 				"special1":       { "type" : "artifactMerchant", "requires" : [ "marketplace" ] },
 				"horde1":         { "id" : 18, "upgrades" : "dwellingLvl1" },
 				"horde1Upgr":     { "id" : 19, "upgrades" : "dwellingUpLvl1", "requires" : [ "horde1" ], "mode" : "auto" },
-				"special2":       { "type" : "manaVortex", "requires" : [ "mageGuild1" ] },
+				"special2":       {
+					"type" : "configurable",
+					"requires" : [ "mageGuild1" ],
+					"configuration" : {
+						"resetParameters" : {
+							"period" : 7,
+							"visitors" : true
+						},
+						"visitMode" : "hero", // Should be 'once' to match (somewhat buggy) H3 logic
+						"rewards" : [
+							{
+								"limiter" : {
+									"noneOf" : [ { "manaPercentage" : 200 } ]
+								},
+								"message" : "@core.genrltxt.579",
+								"manaPercentage" : 200
+							}
+						]
+					}
+				},
 				"special3":       { "type" : "portalOfSummoning" },
-				"special4":       { "type" : "experienceVisitingBonus" },
+				"special4":       {
+					"type" : "configurable",
+					"configuration" : {
+						"visitMode" : "hero",
+						"rewards" : [
+							{
+								"message" : "@core.genrltxt.583",
+								"heroExperience" : 1000
+							}
+						]
+					}
+				},
 				"grail":          { "id" : 26, "mode" : "grail", "produce": { "gold": 5000 },
 					"bonuses": [ { "type": "PRIMARY_SKILL", "subtype": "primarySkill.spellpower", "val": 12 } ] },
 

config/factions/fortress.json

@@ -169,7 +169,19 @@
 				"resourceSilo":   { "id" : 15, "requires" : [ "marketplace" ], "produce": { "wood": 1, "ore": 1 } },
 				"blacksmith":     { "id" : 16 },
 
-				"special1":       { "type" : "defenceVisitingBonus", "requires" : [ "allOf", [ "townHall" ], [ "special2" ] ] },
+				"special1":       {
+					"type" : "configurable",
+					"requires" : [ "allOf", [ "townHall" ], [ "special2" ] ],
+					"configuration" : {
+						"visitMode" : "hero",
+						"rewards" : [
+							{
+								"message" : "@core.genrltxt.585",
+								"primary" : { "defence" : 1 }
+							}
+						]
+					}
+				},
 				"horde1":         { "id" : 18, "upgrades" : "dwellingLvl1" },
 				"horde1Upgr":     { "id" : 19, "upgrades" : "dwellingUpLvl1", "requires" : [ "horde1" ], "mode" : "auto" },
 				"ship":           { "id" : 20, "upgrades" : "shipyard" },

config/factions/inferno.json

@@ -175,7 +175,19 @@
 				"horde1Upgr":     { "id" : 19, "upgrades" : "dwellingUpLvl1", "requires" : [ "horde1" ], "mode" : "auto" },
 				"special2":       { "type" : "spellPowerGarrisonBonus", "requires" : [ "fort" ] },
 				"special3":       { "type" : "castleGate", "requires" : [ "citadel" ] },
-				"special4":       { "type" : "spellPowerVisitingBonus", "requires" : [ "mageGuild1" ] },
+				"special4":       {
+					"type" : "configurable",
+					"requires" : [ "mageGuild1" ],
+					"configuration" : {
+						"visitMode" : "hero",
+						"rewards" : [
+							{
+								"message" : "@core.genrltxt.582",
+								"primary" : { "spellpower" : 1 }
+							}
+						]
+					}
+				},
 				"horde2":         { "id" : 24, "upgrades" : "dwellingLvl3" },
 				"horde2Upgr":     { "id" : 25, "upgrades" : "dwellingUpLvl3", "requires" : [ "horde2" ], "mode" : "auto" },
 				"grail":          { "id" : 26, "mode" : "grail", "produce": { "gold": 5000 }},

config/factions/stronghold.json

@@ -171,7 +171,19 @@
 				"horde1Upgr":     { "id" : 19, "upgrades" : "dwellingUpLvl1", "requires" : [ "horde1" ], "mode" : "auto" },
 				"special2":       { "type" : "freelancersGuild", "requires" : [ "marketplace" ] },
 				"special3":       { "type" : "ballistaYard", "requires" : [ "blacksmith" ] },
-				"special4":       { "type" : "attackVisitingBonus", "requires" : [ "fort" ] },
+				"special4":       { 
+					"type" : "configurable",
+					"requires" : [ "fort" ],
+					"configuration" : {
+						"visitMode" : "hero",
+						"rewards" : [
+							{
+								"message" : "@core.genrltxt.584",
+								"primary" : { "attack" : 1 }
+							}
+						]
+					}
+				},
 				"grail":          { "id" : 26, "mode" : "grail", "produce": { "gold": 5000 },
 					"bonuses": [ { "type": "PRIMARY_SKILL", "subtype": "primarySkill.attack", "val": 20 } ] },
 

config/factions/tower.json

@@ -174,7 +174,19 @@
 				"horde1Upgr":     { "id" : 19, "upgrades" : "dwellingUpLvl2", "requires" : [ "horde1" ], "mode" : "auto" },
 				"special2":       { "type" : "lookoutTower", "height" : "high", "requires" : [ "fort" ] },
 				"special3":       { "type" : "library", "requires" : [ "mageGuild1" ] },
-				"special4":       { "type" : "knowledgeVisitingBonus", "requires" : [ "mageGuild1" ] },
+				"special4":       {
+					"type" : "configurable",
+					"requires" : [ "mageGuild1" ],
+					"configuration" : {
+						"visitMode" : "hero",
+						"rewards" : [
+							{
+								"message" : "@core.genrltxt.581",
+								"primary" : { "knowledge" : 1 }
+							}
+						]
+					}
+				},
 				"grail":          { "height" : "skyship",  "produce" : { "gold": 5000 }, "bonuses": [ { "type": "PRIMARY_SKILL", "subtype": "primarySkill.knowledge", "val": 15 } ] },
 
 				"dwellingLvl1":   { "id" : 30, "requires" : [ "fort" ] },

config/schemas/bonus.json

@@ -179,7 +179,10 @@
 			"description" : "stacking"
 		},
 		"description" : {
-			"type" : "string",
+			"anyOf" : [
+				{ "type" : "string" },
+				{ "type" : "number" }
+			],
 			"description" : "description"
 		}
 	}

config/schemas/rewardable.json

@@ -0,0 +1,326 @@
+{
+	"type" : "object",
+	"$schema" : "http://json-schema.org/draft-04/schema",
+	"title" : "VCMI map object format",
+	"description" : "Description of map object class",
+	"required" : [ "rewards" ],
+	"additionalProperties" : false,
+	
+	"definitions" : {
+		"value" : {
+			"anyOf" : [
+				{
+					"type" : "number"
+				},
+				{
+					"type" : "string" // variable name
+				},
+				{
+					"type" : "array",
+					"items" : {
+						"$ref" : "#/definitions/value"
+					}
+				},
+				{
+					"type" : "object",
+					"additionalProperties" : true,
+					"properties" : {
+						"amount" : { "$ref" : "#/definitions/value" },
+						"min" : { "$ref" : "#/definitions/value" },
+						"max" : { "$ref" : "#/definitions/value" }
+					}
+				}
+			]
+		},
+		"identifier" : {
+			"anyOf" : [
+				{
+					"type" : "string"
+				},
+				{
+					"type" : "object",
+					"additionalProperties" : true,
+					"properties" : {
+						"type" : {
+							"$ref" : "#/definitions/identifier"
+						},
+						"anyOf" : {
+							"type" : "array",
+							"items" : {
+								"$ref" : "#/definitions/identifier"
+							}
+						},
+						"noneOf" : {
+							"type" : "array",
+							"items" : {
+								"$ref" : "#/definitions/identifier"
+							}
+						}
+					}
+				}
+			]
+		},
+		"identifierList" : {
+			"type" : "array",
+			"items" : {
+				"$ref" : "#/definitions/identifier"
+			}
+		},
+		"identifierWithValueList" : {
+			"anyOf" : [
+				{
+					"type" : "array",
+					"items" : {
+						"allOf" : [
+							{ "$ref" : "#/definitions/identifier" },
+							{ "$ref" : "#/definitions/value" }
+						]
+					}
+				},
+				{
+					"type" : "object",
+					"additionalProperties" : {
+						"$ref" : "#/definitions/value"
+					}
+				},
+			],
+		},
+		"reward" : {
+			"type" : "object",
+			"additionalProperties" : false,
+			"properties" : {
+				"appearChance" : {
+					"type" : "object", 
+					"additionalProperties" : false,
+					"properties" : {
+						"dice" : { "type" : "number" },
+						"min" : { "type" : "number", "minimum" : 0, "exclusiveMaximum" : 100 },
+						"max" : { "type" : "number", "exclusiveMinimum" : 0, "maximum" : 100 }
+					}
+				},
+				"limiter" : { "$ref" : "#/definitions/limiter" },
+				"message" : { "$ref" : "#/definitions/message" },
+				"description" : { "$ref" : "#/definitions/message" },
+				
+				"heroExperience" : { "$ref" : "#/definitions/value" },
+				"heroLevel" : { "$ref" : "#/definitions/value" },
+				"movePercentage" : { "$ref" : "#/definitions/value" },
+				"movePoints" : { "$ref" : "#/definitions/value" },
+				"manaPercentage" : { "$ref" : "#/definitions/value" },
+				"manaPoints" : { "$ref" : "#/definitions/value" },
+				"manaOverflowFactor" : { "$ref" : "#/definitions/value" },
+
+				"removeObject" : { "type" : "boolean" },
+				"bonuses" : {
+					"type":"array",
+					"description": "List of bonuses that will be granted to visiting hero",
+					"items": { "$ref" : "bonus.json" }
+				},
+
+				"resources" : { "$ref" : "#/definitions/identifierWithValueList" },
+				"secondary" : { "$ref" : "#/definitions/identifierWithValueList" },
+				"creatures" : { "$ref" : "#/definitions/identifierWithValueList" },
+				"primary" : { "$ref" : "#/definitions/identifierWithValueList" },
+
+				"artifacts" : { "$ref" : "#/definitions/identifierList" },
+				"spells" : { "$ref" : "#/definitions/identifierList" },
+
+				"spellCast" : {
+					"type" : "object", 
+					"additionalProperties" : false,
+					"properties" : {
+						"spell" : { "$ref" : "#/definitions/identifier" },
+						"schoolLevel" : { "type" : "number" }
+					}
+				},
+				"revealTiles" : {
+					"type" : "object", 
+					"additionalProperties" : false,
+					"properties" : {
+						"hide" : { "type" : "boolean" },
+						"radius" : { "type" : "number" },
+						"surface" : { "type" : "number" },
+						"subterra" : { "type" : "number" },
+						"water" : { "type" : "number" },
+						"rock" : { "type" : "number" }
+					}
+				},
+				"changeCreatures" : {
+					"type" : "object", 
+					"additionalProperties" : { "type" : "string" }
+				}
+			}
+		},
+		"limiter" : {
+			"type" : "object",
+			"additionalProperties" : false,
+			"properties" : {
+				"dayOfWeek" : { "$ref" : "#/definitions/value" },
+				"daysPassed" : { "$ref" : "#/definitions/value" },
+				"heroExperience" : { "$ref" : "#/definitions/value" },
+				"heroLevel" : { "$ref" : "#/definitions/value" },
+				"manaPercentage" : { "$ref" : "#/definitions/value" },
+				"manaPoints" : { "$ref" : "#/definitions/value" },
+
+				"canLearnSkills" : { "type" : "boolean" },
+
+				"resources" : { "$ref" : "#/definitions/identifierWithValueList" },
+				"secondary" : { "$ref" : "#/definitions/identifierWithValueList" },
+				"creatures" : { "$ref" : "#/definitions/identifierWithValueList" },
+				"primary" : { "$ref" : "#/definitions/identifierWithValueList" },
+
+				"canLearnSpells" : { "$ref" : "#/definitions/identifierList" },
+				"heroClasses" : { "$ref" : "#/definitions/identifierList" },
+				"artifacts" : { "$ref" : "#/definitions/identifierList" },
+				"spells" : { "$ref" : "#/definitions/identifierList" },
+				"colors" : { "$ref" : "#/definitions/identifierList" },
+				"heroes" : { "$ref" : "#/definitions/identifierList" },
+				
+				"anyOf" : {
+					"type" : "array",
+					"items" : { "$ref" : "#/definitions/limiter" }
+				},
+				"allOf" : {
+					"type" : "array",
+					"items" : { "$ref" : "#/definitions/limiter" }
+				},
+				"noneOf" : {
+					"type" : "array",
+					"items" : { "$ref" : "#/definitions/limiter" }
+				},
+			}
+		},
+		"message" : {
+			"anyOf" : [
+				{
+					"type" : "array",
+					"items" : {
+						"anyOf" : [
+							{ "type" : "number" },
+							{ "type" : "string" }
+						]
+					}
+				},
+				{
+					"type" : "number"
+				},
+				{
+					"type" : "string"
+				}
+			]
+		},
+		"variableList" : {
+			"type" : "object",
+			"additionalProperties" : { 
+				"$ref" : "#/definitions/identifier"
+			}
+		}
+	},
+
+	"properties" : {
+		"rewards" : {
+			"type" : "array",
+			"items" : { "$ref" : "#/definitions/reward" }
+		},
+		"onVisited" : {
+			"type" : "array",
+			"items" : { "$ref" : "#/definitions/reward" }
+		},
+		"onEmpty" : {
+			"type" : "array",
+			"items" : { "$ref" : "#/definitions/reward" }
+		},
+		
+		"variables" : {
+			"type" : "object",
+			"additionalProperties" : false,
+			"properties" : {
+				"number" : {
+					"type" : "object",
+					"additionalProperties" : { 
+						"$ref" : "#/definitions/value"
+					}
+				},
+				"artifact" : {
+					"$ref" : "#/definitions/variableList"
+				},
+				"spell" : {
+					"$ref" : "#/definitions/variableList"
+				},
+				"primarySkill" : {
+					"$ref" : "#/definitions/variableList"
+				},
+				"secondarySkill" : {
+					"$ref" : "#/definitions/variableList"
+				},
+			},
+		},
+
+		"onSelectMessage" : {
+			"$ref" : "#/definitions/message"
+		},
+		"description" : {
+			"$ref" : "#/definitions/message"
+		},
+		"notVisitedTooltip" : {
+			"$ref" : "#/definitions/message"
+		},
+		"visitedTooltip" : {
+			"$ref" : "#/definitions/message"
+		},
+		"onVisitedMessage" : {
+			"$ref" : "#/definitions/message"
+		},
+		"onEmptyMessage" : {
+			"$ref" : "#/definitions/message"
+		},
+		
+		"canRefuse": {
+			"type" : "boolean"
+		},
+		
+		"showScoutedPreview": {
+			"type" : "boolean"
+		},
+		
+		"showInInfobox": {
+			"type" : "boolean"
+		},
+		
+		"visitMode": {
+			"enum" : [ "unlimited", "once", "hero", "bonus", "limiter", "player" ],
+			"type" : "string"
+		},
+		
+		"visitLimiter": {
+			"type" : "object"
+		},
+		
+		"selectMode": {
+			"enum" : [ "selectFirst", "selectPlayer", "selectRandom", "selectAll" ],
+			"type" : "string"
+		},
+		
+		"resetParameters" : {
+			"type" : "object",
+			"additionalProperties" : false,
+			"properties" : {
+				"visitors" : { "type" : "boolean" },
+				"rewards" : { "type" : "boolean" },
+				"period" : { "type" : "number" }
+			}
+		},
+		
+		// Properties that might appear since this node is shared with object config
+		"compatibilityIdentifiers" : { },
+		"blockedVisitable" : { },
+		"removable" : { },
+		"aiValue" : { },
+		"index" : { },
+		"base" : { },
+		"rmg" : { },
+		"templates" : { },
+		"battleground" : { },
+		"sounds" : { }
+	}
+}

config/schemas/townBuilding.json

@@ -31,11 +31,12 @@
 			"type" : "string"
 		},
 		"description" : {
-			"description" : "Localizable decsription of this building",
+			"description" : "Localizable description of this building",
 			"type" : "string"
 		},
 		"type" : {
 			"type" : "string",
+			"enum" : [ "mysticPond", "artifactMerchant", "freelancersGuild", "magicUniversity", "castleGate", "creatureTransformer", "portalOfSummoning", "ballistaYard", "lookoutTower", "library", "brotherhoodOfSword", "fountainOfFortune", "spellPowerGarrisonBonus", "attackGarrisonBonus", "defenseGarrisonBonus", "escapeTunnel", "lighthouse", "treasury", "thievesGuild", "bank", "configurable" ],
 			"description" : "Subtype for some special buildings"
 		},
 		"mode" : {
@@ -56,6 +57,10 @@
 			"description" : "Optional, indicates that this building upgrades another base building",
 			"type" : "string"
 		},
+		"configuration" : {
+			"description" : "Configuration of building. Only used if 'type' is set to 'configurable'",
+			"$ref" : "rewardable.json"
+		},
 		"cost" : {
 			"type" : "object",
 			"additionalProperties" : false,

config/shortcutsConfig.json

@@ -242,6 +242,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`
-- `"inflix"` ised to identify all images related to region. In this example, it will be pictures starting from `G3A_..., G3B_..., G3C_..."` 
-- `"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`
+- `"background"` optional - use own image name for background instead of adding "_BG" to the prefix as name
+- `"prefix"` used to identify all images related to campaign. In this example (if background parameter wouldn't exists), background picture will be `G3_BG`
+- `"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
 

docs/modders/Entities_Format/Faction_Format.md

@@ -341,100 +341,7 @@ Each town requires a set of buildings (Around 30-45 buildings)
 ```
 
 ## Building node
-
-```jsonc
-{
-	// Numeric identifier of this building
-	"id" : 0,
-	
-	// Localizable name of this building
-	"name" : "",
-	
-	// Localizable decsription of this building
-	"description" : "",
-	
-	// Optional, indicates that this building upgrades another base building
-	"upgrades" : "baseBuilding",
-	
-	// List of town buildings that must be built before this one. See below for full format
-	"requires" : [ "allOf", [ "mageGuild1" ], [ "tavern" ] ],
-	
-	// Resources needed to build building
-	"cost" : { ... }, 
-	
-	// TODO: Document me: Subtype for some special buildings
-	"type" : "",
-	
-	// TODO: Document me: Height for lookout towers and some grails
-	"height" : "average"
-	
-	// Resources produced each day by this building
-	"produce" : { ... }, 
-
-	//determine how this building can be built. Possible values are:
-	// normal  - default value. Fulfill requirements, use resources, spend one day
-	// auto    - building appears when all requirements are built
-	// special - building can not be built manually
-	// grail   - building requires grail to be built
-	"mode" : "auto",
-	
-	// Buildings which bonuses should be overridden with bonuses of the current building
-	"overrides" : [ "anotherBuilding ]
-	
-	// Bonuses, provided by this special building on build using bonus system
-	"bonuses" : BONUS_FORMAT
-	
-	// Bonuses, provided by this special building on hero visit and applied to the visiting hero
-	"onVisitBonuses" : BONUS_FORMAT
-}
-```
-
-Building requirements can be described using logical expressions:
-
-```jsonc
-"requires" :
-[
-    "allOf", // Normal H3 "build all" mode
-    [ "mageGuild1" ],
-    [
-        "noneOf",  // available only when none of these building are built
-        [ "dwelling5A" ],
-        [ "dwelling5AUpgrade" ]
-    ],
-    [
-        "anyOf", // any non-zero number of these buildings must be built
-        [ "tavern" ],
-        [ "blacksmith" ]
-    ]
-]
-```
+See [Town Building Format](Town_Building_Format.md)
 
 ## Structure node
-
-```jsonc
-{
-	// Main animation file for this building
-	"animation" : "", 
-	
-	// Horizontal position on town screen
-	"x" : 0,
-	
-	// Vertical  position on town screen
-	"y" : 0,
-	
-	// used for blit order. Higher value places structure close to screen and drawn on top of buildings with lower values
-	"z" : 0, 
-	
-	// Path to image with golden border around building, displayed when building is selected
-	"border" : "", 
-	
-	// Path to image with area that indicate when building is selected
-	"area" : "",
-	
-	//TODO: describe me
-	"builds": "",
-	
-	// If upgrade, this building will replace parent animation but will not alter its behaviour
-	"hidden" : false 
-}
-```
+See [Town Building Format](Town_Building_Format.md)

docs/modders/Entities_Format/Town_Building_Format.md

@@ -0,0 +1,194 @@
+# Town Building Format
+
+# Required data
+
+Each building requires following assets:
+
+-   Town animation file (1 animation file)
+-   Selection highlight (1 image)
+-   Selection area (1 image)
+-   Town hall icon (1 image)
+
+## Town Building node
+
+```jsonc
+{
+	// Numeric identifier of this building
+	"id" : 0,
+	
+	// Localizable name of this building
+	"name" : "",
+	
+	// Localizable decsription of this building
+	"description" : "",
+	
+	// Optional, indicates that this building upgrades another base building
+	"upgrades" : "baseBuilding",
+	
+	// List of town buildings that must be built before this one. See below for full format
+	"requires" : [ "allOf", [ "mageGuild1" ], [ "tavern" ] ],
+	
+	// Resources needed to build building
+	"cost" : { 
+		"wood" : 20,
+		"ore" : 20,
+		"gold" : 10000
+	}, 
+	
+	// Allows to define additional functionality of this building, usually using logic of one of original H3 town building
+	// Generally only needs to be specified for "special" buildings
+	// See 'List of unique town buildings' section below for detailed description of this field
+	"type" : "",
+	
+	// If set, building will have Lookout Tower logic - extend sight radius of a town.
+	// Possible values: 
+	// low - increases town sight radius by 5 tiles
+	// average - sight radius extended by 15 tiles
+	// high - sight radius extended by 20 tiles
+	// skyship - entire map will be revealed
+	// If not set, building will not affect sight radius of a town
+	"height" : "average"
+	
+	// Resources produced each day by this building
+	"produce" : { 
+		"sulfur" : 1,
+		"gold" : 2000
+	}, 
+
+	//determine how this building can be built. Possible values are:
+	// normal  - default value. Fulfill requirements, use resources, spend one day
+	// auto    - building appears when all requirements are built
+	// special - building can not be built manually
+	// grail   - building requires grail to be built
+	"mode" : "auto",
+	
+	// Buildings which bonuses should be overridden with bonuses of the current building
+	"overrides" : [ "anotherBuilding ]
+	
+	// Bonuses, provided by this special building on build using bonus system
+	"bonuses" : BONUS_FORMAT
+	
+	// Bonuses, provided by this special building on hero visit and applied to the visiting hero
+	"onVisitBonuses" : BONUS_FORMAT
+}
+```
+
+Building requirements can be described using logical expressions:
+
+```jsonc
+"requires" :
+[
+    "allOf", // Normal H3 "build all" mode
+    [ "mageGuild1" ],
+    [
+        "noneOf",  // available only when none of these building are built
+        [ "dwelling5A" ],
+        [ "dwelling5AUpgrade" ]
+    ],
+    [
+        "anyOf", // any non-zero number of these buildings must be built
+        [ "tavern" ],
+        [ "blacksmith" ]
+    ]
+]
+```
+### List of unique town buildings
+
+Following Heroes III buildings can be used as unique buildings for a town. Their functionality should be identical to a corresponding H3 building:
+- `mysticPond`
+- `artifactMerchant`
+- `freelancersGuild`
+- `magicUniversity`
+- `castleGate`
+- `creatureTransformer`
+- `portalOfSummoning`
+- `ballistaYard`
+- `stables`
+- `manaVortex`
+- `lookoutTower`
+- `library`
+- `brotherhoodOfSword`
+- `fountainOfFortune`
+- `escapeTunnel`
+- `lighthouse`
+- `treasury`
+- `spellPowerGarrisonBonus`
+- `attackGarrisonBonus`
+- `defenseGarrisonBonus`
+
+Following HotA buildings can be used as unique building for a town. Functionality should match corresponding HotA building:
+- `thievesGuild`
+- `bank`
+
+In addition to above, it is possible to use same format as [Rewardable](../Map_Objects/Rewardable.md) map objects for town buildings. In order to do that, town building type must be set to `configurable` and configuration of a rewardable object must be placed into `configuration` node 
+
+Example 1 - Order of Fire from Inferno:
+```jsonc
+"special4": { // 
+	"type" : "configurable",
+	"requires" : [ "mageGuild1" ],
+	"configuration" : {
+		"visitMode" : "hero",
+		"rewards" : [
+			{
+				"message" : "@core.genrltxt.582", // NOTE: this forces vcmi to load string from H3 text file. In order to define own string simply write your own message without '@' symbol
+				"primary" : { "spellpower" : 1 }
+			}
+		]
+	}
+}
+``` 
+
+Example 2 - Mana Vortex from Dungeon
+```jsonc
+"special2": {
+	"type" : "configurable",
+	"requires" : [ "mageGuild1" ],
+	"configuration" : {
+		"resetParameters" : {
+			"period" : 7,
+			"visitors" : true
+		},
+		"visitMode" : "once",
+		"rewards" : [
+			{
+				"limiter" : {
+					"noneOf" : [ { "manaPercentage" : 200 } ]
+				},
+				"message" : "@core.genrltxt.579",
+				"manaPercentage" : 200
+			}
+		]
+	}
+}
+```
+
+### Town Structure node
+
+```jsonc
+{
+	// Main animation file for this building
+	"animation" : "", 
+	
+	// Horizontal position on town screen
+	"x" : 0,
+	
+	// Vertical  position on town screen
+	"y" : 0,
+	
+	// used for blit order. Higher value places structure close to screen and drawn on top of buildings with lower values
+	"z" : 0, 
+	
+	// Path to image with golden border around building, displayed when building is selected
+	"border" : "", 
+	
+	// Path to image with area that indicate when building is selected
+	"area" : "",
+	
+	//TODO: describe me
+	"builds": "",
+	
+	// If upgrade, this building will replace parent animation but will not alter its behaviour
+	"hidden" : false 
+}
+```

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 CStackBasicDescriptor * stack, const PlayerColor & O, int I, ui8 Side, const SlotID & S = SlotID(255));
+	CStack(const CStackInstance * base, const PlayerColor & O, int I, BattleSide Side, const SlotID & S);
+	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 makeRetreat(ui8 side);
-	static BattleAction makeSurrender(ui8 side);
+	static BattleAction makeEndOFTacticPhase(BattleSide side);
+	static BattleAction makeRetreat(BattleSide 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
 
+const SideInBattle & BattleInfo::getSide(BattleSide side) const
+{
+	return sides.at(side);
+}
+
+SideInBattle & BattleInfo::getSide(BattleSide side)
+{
+	return sides.at(side);
+}
+
 ///BattleInfo
-CStack * BattleInfo::generateNewStack(uint32_t id, const CStackInstance & base, ui8 side, const SlotID & slot, BattleHex position)
+CStack * BattleInfo::generateNewStack(uint32_t id, const CStackInstance & base, BattleSide side, const SlotID & slot, BattleHex position)
 {
-	PlayerColor owner = sides[side].color;
+	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];
-	std::vector<std::vector<int>> tightFormations[2];
-	std::vector<std::vector<int>> creBankFormations[2];
-	std::vector<int> commanderField;
-	std::vector<int> commanderBank;
+	BattleSideArray<std::vector<std::vector<int>>> looseFormations;
+	BattleSideArray<std::vector<std::vector<int>>> tightFormations;
+	BattleSideArray<std::vector<std::vector<int>>> creBankFormations;
+	BattleSideArray<int> commanderField;
+	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[1]["levels"], looseFormations[1]);
-	CGH::readBattlePositions(positions[2]["levels"], tightFormations[0]);
-	CGH::readBattlePositions(positions[3]["levels"], tightFormations[1]);
-	CGH::readBattlePositions(positions[4]["levels"], creBankFormations[0]);
-	CGH::readBattlePositions(positions[5]["levels"], creBankFormations[1]);
+	CGH::readBattlePositions(positions[0]["levels"], looseFormations[BattleSide::ATTACKER]);
+	CGH::readBattlePositions(positions[1]["levels"], looseFormations[BattleSide::DEFENDER]);
+	CGH::readBattlePositions(positions[2]["levels"], tightFormations[BattleSide::ATTACKER]);
+	CGH::readBattlePositions(positions[3]["levels"], tightFormations[BattleSide::DEFENDER]);
+	CGH::readBattlePositions(positions[4]["levels"], creBankFormations[BattleSide::ATTACKER]);
+	CGH::readBattlePositions(positions[5]["levels"], creBankFormations[BattleSide::DEFENDER]);
 
-	for (auto position : config["commanderPositions"]["field"].Vector())
-	{
-		commanderField.push_back(static_cast<int>(position.Float()));
-	}
-	for (auto position : config["commanderPositions"]["creBank"].Vector())
-	{
-		commanderBank.push_back(static_cast<int>(position.Float()));
-	}
+	commanderField[BattleSide::ATTACKER] = config["commanderPositions"]["field"][0].Integer();
+	commanderField[BattleSide::DEFENDER] = config["commanderPositions"]["field"][1].Integer();
 
+	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(0, ArtifactPosition::MACH2, 18);
-			handleWarMachine(0, ArtifactPosition::MACH3, 154);
+			handleWarMachine(BattleSide::ATTACKER, ArtifactPosition::MACH1, 52);
+			handleWarMachine(BattleSide::ATTACKER, ArtifactPosition::MACH2, 18);
+			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(1, ArtifactPosition::MACH2, 32);
-			handleWarMachine(1, ArtifactPosition::MACH3, 168);
+				handleWarMachine(BattleSide::DEFENDER, ArtifactPosition::MACH1, 66);
+			handleWarMachine(BattleSide::DEFENDER, ArtifactPosition::MACH2, 32);
+			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;
-
-	std::array<int, sideSize> battleRepositionHex = {};
-	std::array<int, sideSize> battleRepositionHexBlock = {};
-	for(int i = 0; i < sideSize; i++)
+	BattleSideArray<int> battleRepositionHex = {};
+	BattleSideArray<int> battleRepositionHexBlock = {};
+	for(auto i : {BattleSide::ATTACKER, BattleSide::DEFENDER})
 	{
 		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;
-	const CArmedInstance * getSideArmy(ui8 side) const override;
-	const CGHeroInstance * getSideHero(ui8 side) const override;
+	PlayerColor getSidePlayer(BattleSide side) const override;
+	const CArmedInstance * getSideArmy(BattleSide 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;
-	int32_t getEnchanterCounter(ui8 side) const override;
+	uint32_t getCastSpells(BattleSide 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, BattleSide side, const SlotID & slot, BattleHex position);
+	CStack * generateNewStack(uint32_t id, const CStackBasicDescriptor & base, BattleSide side, const SlotID & slot, BattleHex position);
 
-	CStack * generateNewStack(uint32_t id, const CStackInstance & base, ui8 side, const SlotID & slot, BattleHex position);
-	CStack * generateNewStack(uint32_t id, const CStackBasicDescriptor & base, ui8 side, const SlotID & slot, BattleHex position);
+	const 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;
-	const CArmedInstance * getSideArmy(ui8 side) const override;
-	const CGHeroInstance * getSideHero(ui8 side) const override;
+	PlayerColor getSidePlayer(BattleSide side) const override;
+	const CArmedInstance * getSideArmy(BattleSide 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;
-	int32_t getEnchanterCounter(ui8 side) const override;
+	uint32_t getCastSpells(BattleSide 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)
-			|| (side && dest.getX() < GameConstants::BFIELD_WIDTH - 1 && dest.getX() >= GameConstants::BFIELD_WIDTH - dist - 1));
+	if (side == BattleSide::ATTACKER && dest.getX() > 0 && dest.getX() <= dist)
+		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);
-	if(!sideOpt)
+	const BattleSide side = playerToSide(Player);
+	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])
-		return 2;
-	if(!hasUnit[1])
-		return 0;
+	if(!hasUnit[BattleSide::ATTACKER] && !hasUnit[BattleSide::DEFENDER])
+		return BattleSide::NONE;
+	if(!hasUnit[BattleSide::DEFENDER])
+		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 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 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(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;
-	PlayerColor sideToPlayer(ui8 side) const;
+	BattleSide playerToSide(const PlayerColor & player) 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;
-	uint32_t battleCastSpells(ui8 side) const; //how many spells has given side cast
-	const CGHeroInstance * battleGetFightingHero(ui8 side) const; //deprecated for players callback, easy to get wrong
-	const CArmedInstance * battleGetArmyObject(ui8 side) const;
-	InfoAboutHero battleGetHeroInfo(ui8 side) const;
+	bool battleHasHero(BattleSide side) const;
+	uint32_t battleCastSpells(BattleSide side) const; //how many spells has given side cast
+	const CGHeroInstance * battleGetFightingHero(BattleSide side) const; //deprecated for players callback, easy to get wrong
+	const CArmedInstance * battleGetArmyObject(BattleSide 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();
 }