Merge branch 'develop' into split-client+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

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

AI/BattleAI/BattleEvaluator.h

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

AI/BattleAI/BattleExchangeVariant.cpp

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

AI/BattleAI/StackWithBonuses.cpp

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

AI/BattleAI/StackWithBonuses.h

@@ -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/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/VCAI.cpp

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

AI/VCAI/VCAI.h

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

client/CPlayerInterface.cpp

@@ -623,7 +623,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/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

@@ -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/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);

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,

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/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);
@@ -376,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)
 	{
@@ -388,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;
@@ -417,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;
@@ -438,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;
@@ -458,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();
 
@@ -500,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();
 	}
 
@@ -560,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))
@@ -887,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);
 
@@ -1098,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);
@@ -1161,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();
 
@@ -1208,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
@@ -1452,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 };
@@ -1477,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;
@@ -1487,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;
 	}
 
@@ -1495,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;
 	}
 
@@ -1503,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;
@@ -1762,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;
@@ -1853,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;
@@ -1872,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))
@@ -1890,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))
@@ -1909,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;
 	}
 	
@@ -1937,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();
 }

lib/battle/CUnitState.h

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

lib/battle/IBattleInfoCallback.h

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

lib/battle/IBattleState.h

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

lib/battle/IUnitInfo.h

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

lib/battle/ReachabilityInfo.cpp

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

lib/battle/ReachabilityInfo.h

@@ -25,14 +25,14 @@ struct DLL_LINKAGE ReachabilityInfo
 
 	struct DLL_LINKAGE Parameters
 	{
-		ui8 side = 0;
+		BattleSide side = BattleSide::NONE;
 		bool doubleWide = false;
 		bool flying = false;
 		bool ignoreKnownAccessible = false; //Ignore obstacles if it is in accessible hexes
 		std::vector<BattleHex> knownAccessible; //hexes that will be treated as accessible, even if they're occupied by stack (by default - tiles occupied by stack we do reachability for, so it doesn't block itself)
 
 		BattleHex startPosition; //assumed position of stack
-		BattlePerspective::BattlePerspective perspective = BattlePerspective::ALL_KNOWING; //some obstacles (eg. quicksands) may be invisible for some side
+		BattleSide perspective = BattleSide::ALL_KNOWING; //some obstacles (eg. quicksands) may be invisible for some side
 
 		Parameters() = default;
 		Parameters(const battle::Unit * Stack, BattleHex StartPosition);

lib/battle/Unit.cpp

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

lib/battle/Unit.h

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

lib/bonuses/IBonusBearer.cpp

@@ -84,11 +84,9 @@ bool IBonusBearer::hasBonusOfType(BonusType type, BonusSubtypeID subtype) const
 
 bool IBonusBearer::hasBonusFrom(BonusSource source, BonusSourceID sourceID) const
 {
-	boost::format fmt("source_%did_%s");
-	fmt % static_cast<int>(source) % sourceID.toString();
-
-	return hasBonus(Selector::source(source,sourceID), fmt.str());
+	return hasBonus(Selector::source(source,sourceID));
 }
+
 std::shared_ptr<const Bonus> IBonusBearer::getBonus(const CSelector &selector) const
 {
 	auto bonuses = getAllBonuses(selector, Selector::all);

lib/constants/StringConstants.h

@@ -203,7 +203,8 @@ namespace MappedKeys
 		{ "lighthouse", BuildingSubID::LIGHTHOUSE },
 		{ "treasury", BuildingSubID::TREASURY },
 		{ "thievesGuild", BuildingSubID::THIEVES_GUILD },
-		{ "bank", BuildingSubID::BANK }
+		{ "bank", BuildingSubID::BANK },
+		{ "configurable", BuildingSubID::CUSTOM_VISITING_REWARD}
 	};
 
 	static const std::map<std::string, EMarketMode> MARKET_NAMES_TO_TYPES =

lib/entities/faction/CTown.cpp

@@ -80,7 +80,7 @@ BuildingID CTown::getBuildingType(BuildingSubID::EBuildingSubID subID) const
 
 std::string CTown::getGreeting(BuildingSubID::EBuildingSubID subID) const
 {
-	return CTownHandler::getMappedValue<const std::string, BuildingSubID::EBuildingSubID>(subID, std::string(), specialMessages, false);
+	return vstd::find_or(specialMessages, subID, std::string());
 }
 
 void CTown::setGreeting(BuildingSubID::EBuildingSubID subID, const std::string & message) const

lib/entities/faction/CTownHandler.cpp

@@ -324,7 +324,7 @@ void CTownHandler::loadBuilding(CTown * town, const std::string & stringID, cons
 	assert(!source.getModScope().empty());
 
 	auto * ret = new CBuilding();
-	ret->bid = getMappedValue<BuildingID, std::string>(stringID, BuildingID::NONE, MappedKeys::BUILDING_NAMES_TO_TYPES, false);
+	ret->bid = vstd::find_or(MappedKeys::BUILDING_NAMES_TO_TYPES, stringID, BuildingID::NONE);
 	ret->subId = BuildingSubID::NONE;
 
 	if(ret->bid == BuildingID::NONE && !source["id"].isNull())
@@ -339,9 +339,9 @@ void CTownHandler::loadBuilding(CTown * town, const std::string & stringID, cons
 
 	ret->mode = ret->bid == BuildingID::GRAIL
 		? CBuilding::BUILD_GRAIL
-		: getMappedValue<CBuilding::EBuildMode>(source["mode"], CBuilding::BUILD_NORMAL, CBuilding::MODES);
+		: vstd::find_or(CBuilding::MODES, source["mode"].String(), CBuilding::BUILD_NORMAL);
 
-	ret->height = getMappedValue<CBuilding::ETowerHeight>(source["height"], CBuilding::HEIGHT_NO_TOWER, CBuilding::TOWER_TYPES);
+	ret->height = vstd::find_or(CBuilding::TOWER_TYPES, source["height"].String(), CBuilding::HEIGHT_NO_TOWER);
 
 	ret->identifier = stringID;
 	ret->modScope = source.getModScope();
@@ -361,7 +361,7 @@ void CTownHandler::loadBuilding(CTown * town, const std::string & stringID, cons
 
 		if(ret->buildingBonuses.empty())
 		{
-			ret->subId = getMappedValue<BuildingSubID::EBuildingSubID>(source["type"], BuildingSubID::NONE, MappedKeys::SPECIAL_BUILDINGS);
+			ret->subId = vstd::find_or(MappedKeys::SPECIAL_BUILDINGS, source["type"].String(), BuildingSubID::NONE);
 			addBonusesForVanilaBuilding(ret);
 		}
 
@@ -376,11 +376,8 @@ void CTownHandler::loadBuilding(CTown * town, const std::string & stringID, cons
 				bonus->sid = BonusSourceID(ret->getUniqueTypeID());
 		}
 		
-		if(source["type"].String() == "configurable" && ret->subId == BuildingSubID::NONE)
-		{
-			ret->subId = BuildingSubID::CUSTOM_VISITING_REWARD;
-			ret->rewardableObjectInfo.init(source, ret->getBaseTextID());
-		}
+		if(ret->subId == BuildingSubID::CUSTOM_VISITING_REWARD)
+			ret->rewardableObjectInfo.init(source["configuration"], ret->getBaseTextID());
 	}
 	//MODS COMPATIBILITY FOR 0.96
 	if(!ret->produce.nonZero())

lib/entities/faction/CTownHandler.h

@@ -68,11 +68,6 @@ class DLL_LINKAGE CTownHandler : public CHandlerBase<FactionID, Faction, CFactio
 	void loadRandomFaction();
 
 public:
-	template<typename R, typename K>
-	static R getMappedValue(const K key, const R defval, const std::map<K, R> & map, bool required = true);
-	template<typename R>
-	static R getMappedValue(const JsonNode & node, const R defval, const std::map<std::string, R> & map, bool required = true);
-
 	CTown * randomTown;
 	CFaction * randomFaction;
 
@@ -98,25 +93,4 @@ protected:
 	std::shared_ptr<CFaction> loadFromJson(const std::string & scope, const JsonNode & data, const std::string & identifier, size_t index) override;
 };
 
-template<typename R, typename K>
-R CTownHandler::getMappedValue(const K key, const R defval, const std::map<K, R> & map, bool required)
-{
-	auto it = map.find(key);
-
-	if(it != map.end())
-		return it->second;
-
-	if(required)
-		logMod->warn("Warning: Property: '%s' is unknown. Correct the typo or update VCMI.", key);
-	return defval;
-}
-
-template<typename R>
-R CTownHandler::getMappedValue(const JsonNode & node, const R defval, const std::map<std::string, R> & map, bool required)
-{
-	if(!node.isNull() && node.getType() == JsonNode::JsonType::DATA_STRING)
-		return getMappedValue<R, std::string>(node.String(), defval, map, required);
-	return defval;
-}
-
 VCMI_LIB_NAMESPACE_END

lib/gameState/CGameState.cpp

@@ -1020,7 +1020,7 @@ const BattleInfo * CGameState::getBattle(const PlayerColor & player) const
 		return nullptr;
 
 	for (const auto & battlePtr : currentBattles)
-		if (battlePtr->sides[0].color == player || battlePtr->sides[1].color == player)
+		if (battlePtr->getSide(BattleSide::ATTACKER).color == player || battlePtr->getSide(BattleSide::DEFENDER).color == player)
 			return battlePtr.get();
 
 	return nullptr;

lib/mapObjectConstructors/CRewardableConstructor.cpp

@@ -10,6 +10,7 @@
 #include "StdInc.h"
 #include "CRewardableConstructor.h"
 
+#include "../json/JsonUtils.h"
 #include "../mapObjects/CRewardableObject.h"
 #include "../texts/CGeneralTextHandler.h"
 #include "../IGameCallback.h"
@@ -23,6 +24,8 @@ void CRewardableConstructor::initTypeData(const JsonNode & config)
 
 	if (!config["name"].isNull())
 		VLC->generaltexth->registerString( config.getModScope(), getNameTextID(), config["name"].String());
+
+	JsonUtils::validate(config, "vcmi:rewardable", getJsonKey());
 	
 }
 

lib/mapObjects/CBank.cpp

@@ -387,7 +387,7 @@ void CBank::doVisit(const CGHeroInstance * hero) const
 
 void CBank::battleFinished(const CGHeroInstance *hero, const BattleResult &result) const
 {
-	if (result.winner == 0)
+	if (result.winner == BattleSide::ATTACKER)
 	{
 		doVisit(hero);
 	}

lib/mapObjects/CGCreature.cpp

@@ -480,12 +480,12 @@ void CGCreature::flee( const CGHeroInstance * h ) const
 
 void CGCreature::battleFinished(const CGHeroInstance *hero, const BattleResult &result) const
 {
-	if(result.winner == 0)
+	if(result.winner == BattleSide::ATTACKER)
 	{
 		giveReward(hero);
 		cb->removeObject(this, hero->getOwner());
 	}
-	else if(result.winner > 1) // draw
+	else if(result.winner == BattleSide::NONE) // draw
 	{
 		// guarded reward is lost forever on draw
 		cb->removeObject(this, hero->getOwner());

lib/mapObjects/CGDwelling.cpp

@@ -510,7 +510,7 @@ void CGDwelling::heroAcceptsCreatures( const CGHeroInstance *h) const
 
 void CGDwelling::battleFinished(const CGHeroInstance *hero, const BattleResult &result) const
 {
-	if (result.winner == 0)
+	if (result.winner == BattleSide::ATTACKER)
 	{
 		onHeroVisit(hero);
 	}

lib/mapObjects/CGHeroInstance.cpp

@@ -31,6 +31,7 @@
 #include "../StartInfo.h"
 #include "CGTownInstance.h"
 #include "../entities/faction/CTownHandler.h"
+#include "../battle/CBattleInfoEssentials.h"
 #include "../campaign/CampaignState.h"
 #include "../json/JsonBonus.h"
 #include "../pathfinder/TurnInfo.h"
@@ -880,7 +881,7 @@ CStackBasicDescriptor CGHeroInstance::calculateNecromancy (const BattleResult &b
 		double necromancySkill = valOfBonuses(BonusType::UNDEAD_RAISE_PERCENTAGE) / 100.0;
 		const ui8 necromancyLevel = valOfBonuses(BonusType::IMPROVED_NECROMANCY);
 		vstd::amin(necromancySkill, 1.0); //it's impossible to raise more creatures than all...
-		const std::map<CreatureID,si32> &casualties = battleResult.casualties[!battleResult.winner];
+		const std::map<CreatureID,si32> &casualties = battleResult.casualties[CBattleInfoEssentials::otherSide(battleResult.winner)];
 		// figure out what to raise - pick strongest creature meeting requirements
 		CreatureID creatureTypeRaised = CreatureID::NONE; //now we always have IMPROVED_NECROMANCY, no need for hardcode
 		int requiredCasualtyLevel = 1;

lib/mapObjects/CGPandoraBox.cpp

@@ -180,7 +180,7 @@ void CGPandoraBox::onHeroVisit(const CGHeroInstance * h) const
 
 void CGPandoraBox::battleFinished(const CGHeroInstance *hero, const BattleResult &result) const
 {
-	if(result.winner == 0)
+	if(result.winner == BattleSide::ATTACKER)
 	{
 		CRewardableObject::onHeroVisit(hero);
 	}

lib/mapObjects/CGTownBuilding.cpp

@@ -476,7 +476,7 @@ void CTownRewardableBuilding::onHeroVisit(const CGHeroInstance *h) const
 		cb->showBlockingDialog(&sd);
 	};
 	
-	if(!town->hasBuilt(bID) || cb->isVisitCoveredByAnotherQuery(town, h))
+	if(!town->hasBuilt(bID))
 		return;
 
 	if(!wasVisitedBefore(h))

lib/mapObjects/CGTownInstance.cpp

@@ -322,8 +322,9 @@ void CGTownInstance::onHeroVisit(const CGHeroInstance * h) const
 			cb->heroVisitCastle(this, h);
 		}
 	}
-	else if(h->visitablePos() == visitablePos())
+	else
 	{
+		assert(h->visitablePos() == this->visitablePos());
 		bool commander_recover = h->commander && !h->commander->alive;
 		if (commander_recover) // rise commander from dead
 		{
@@ -344,10 +345,6 @@ void CGTownInstance::onHeroVisit(const CGHeroInstance * h) const
 			cb->showInfoDialog(&iw);
 		}
 	}
-	else
-	{
-		logGlobal->error("%s visits allied town of %s from different pos?", h->getNameTranslated(), getNameTranslated());
-	}
 }
 
 void CGTownInstance::onHeroLeave(const CGHeroInstance * h) const
@@ -556,14 +553,6 @@ void CGTownInstance::newTurn(vstd::RNG & rand) const
 		for(const auto * manaVortex : getBonusingBuildings(BuildingSubID::MANA_VORTEX))
 			cb->setObjPropertyValue(id, ObjProperty::STRUCTURE_CLEAR_VISITORS, manaVortex->indexOnTV); //reset visitors for Mana Vortex
 
-		//get Mana Vortex or Stables bonuses
-		//same code is in the CGameHandler::buildStructure method
-		if (garrisonHero != nullptr) //garrison hero first - consistent with original H3 Mana Vortex and Battle Scholar Academy levelup windows order
-			cb->visitCastleObjects(this, garrisonHero);
-
-		if (visitingHero != nullptr)
-			cb->visitCastleObjects(this, visitingHero);
-
 		if (tempOwner == PlayerColor::NEUTRAL) //garrison growth for neutral towns
 		{
 			std::vector<SlotID> nativeCrits; //slots

lib/mapObjects/MiscObjects.cpp

@@ -205,7 +205,7 @@ ui32 CGMine::getProducedQuantity() const
 
 void CGMine::battleFinished(const CGHeroInstance *hero, const BattleResult &result) const
 {
-	if(result.winner == 0) //attacker won
+	if(result.winner == BattleSide::ATTACKER) //attacker won
 	{
 		if(isAbandoned())
 		{
@@ -344,7 +344,7 @@ void CGResource::collectRes(const PlayerColor & player) const
 
 void CGResource::battleFinished(const CGHeroInstance *hero, const BattleResult &result) const
 {
-	if(result.winner == 0) //attacker won
+	if(result.winner == BattleSide::ATTACKER) //attacker won
 		collectRes(hero->getOwner());
 }
 
@@ -911,7 +911,7 @@ BattleField CGArtifact::getBattlefield() const
 
 void CGArtifact::battleFinished(const CGHeroInstance *hero, const BattleResult &result) const
 {
-	if(result.winner == 0) //attacker won
+	if(result.winner == BattleSide::ATTACKER) //attacker won
 		pick(hero);
 }
 
@@ -1010,7 +1010,7 @@ bool CGGarrison::passableFor(PlayerColor player) const
 
 void CGGarrison::battleFinished(const CGHeroInstance *hero, const BattleResult &result) const
 {
-	if (result.winner == 0)
+	if (result.winner == BattleSide::ATTACKER)
 		onHeroVisit(hero);
 }
 

lib/mapping/CMap.cpp

@@ -44,8 +44,7 @@ DisposedHero::DisposedHero() : heroId(0), portrait(255)
 }
 
 CMapEvent::CMapEvent()
-	: players(0)
-	, humanAffected(false)
+	: humanAffected(false)
 	, computerAffected(false)
 	, firstOccurrence(0)
 	, nextOccurrence(0)
@@ -53,21 +52,51 @@ CMapEvent::CMapEvent()
 
 }
 
-bool CMapEvent::earlierThan(const CMapEvent & other) const
+bool CMapEvent::occursToday(int currentDay) const
 {
-	return firstOccurrence < other.firstOccurrence;
+	if (currentDay == firstOccurrence + 1)
+		return true;
+
+	if (nextOccurrence == 0)
+		return false;
+
+	if (currentDay < firstOccurrence)
+		return false;
+
+	return (currentDay - firstOccurrence - 1) % nextOccurrence == 0;
 }
 
-bool CMapEvent::earlierThanOrEqual(const CMapEvent & other) const
+bool CMapEvent::affectsPlayer(PlayerColor color, bool isHuman) const
 {
-	return firstOccurrence <= other.firstOccurrence;
+	if (players.count(color) == 0)
+		return false;
+
+	if (!isHuman && !computerAffected)
+		return false;
+
+	if (isHuman && !humanAffected)
+		return false;
+
+	return true;
 }
 
 void CMapEvent::serializeJson(JsonSerializeFormat & handler)
 {
 	handler.serializeString("name", name);
 	handler.serializeStruct("message", message);
-	handler.serializeInt("players", players);
+	if (!handler.saving && handler.getCurrent()["players"].isNumber())
+	{
+		// compatibility for old maps
+		int playersMask = 0;
+		handler.serializeInt("players", playersMask);
+		for (int i = 0; i < 8; ++i)
+			if ((playersMask & (1 << i)) != 0)
+				players.insert(PlayerColor(i));
+	}
+	else
+	{
+		handler.serializeIdArray("players", players);
+	}
 	handler.serializeInt("humanAffected", humanAffected);
 	handler.serializeInt("computerAffected", computerAffected);
 	handler.serializeInt("firstOccurrence", firstOccurrence);

lib/mapping/CMap.h

@@ -136,7 +136,7 @@ public:
 	std::set<SpellID> allowedSpells;
 	std::set<ArtifactID> allowedArtifact;
 	std::set<SecondarySkill> allowedAbilities;
-	std::list<CMapEvent> events;
+	std::vector<CMapEvent> events;
 	int3 grailPos;
 	int grailRadius;
 

lib/mapping/CMapDefines.h

@@ -30,13 +30,13 @@ public:
 	CMapEvent();
 	virtual ~CMapEvent() = default;
 
-	bool earlierThan(const CMapEvent & other) const;
-	bool earlierThanOrEqual(const CMapEvent & other) const;
+	bool occursToday(int currentDay) const;
+	bool affectsPlayer(PlayerColor player, bool isHuman) const;
 
 	std::string name;
 	MetaString message;
 	TResources resources;
-	ui8 players; // affected players, bit field?
+	std::set<PlayerColor> players;
 	bool humanAffected;
 	bool computerAffected;
 	ui32 firstOccurrence;
@@ -48,7 +48,18 @@ public:
 		h & name;
 		h & message;
 		h & resources;
-		h & players;
+		if (h.version >= Handler::Version::EVENTS_PLAYER_SET)
+		{
+			h & players;
+		}
+		else
+		{
+			ui8 playersMask = 0;
+			h & playersMask;
+			for (int i = 0; i < 8; ++i)
+				if ((playersMask & (1 << i)) != 0)
+					players.insert(PlayerColor(i));
+		}
 		h & humanAffected;
 		h & computerAffected;
 		h & firstOccurrence;

lib/mapping/MapFormatH3M.cpp

@@ -2244,7 +2244,7 @@ CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_pt
 
 		reader->readResources(event.resources);
 
-		event.players = reader->readUInt8();
+		reader->readBitmaskPlayers(event.players, false);
 		if(features.levelSOD)
 			event.humanAffected = reader->readBool();
 		else
@@ -2313,7 +2313,7 @@ void CMapLoaderH3M::readEvents()
 		event.message.appendTextID(readLocalizedString(TextIdentifier("event", eventID, "description")));
 
 		reader->readResources(event.resources);
-		event.players = reader->readUInt8();
+		reader->readBitmaskPlayers(event.players, false);
 		if(features.levelSOD)
 		{
 			event.humanAffected = reader->readBool();

lib/networkPacks/NetPackVisitor.h

@@ -55,8 +55,6 @@ public:
 	virtual void visitSetCommanderProperty(SetCommanderProperty & pack) {}
 	virtual void visitAddQuest(AddQuest & pack) {}
 	virtual void visitUpdateArtHandlerLists(UpdateArtHandlerLists & pack) {}
-	virtual void visitUpdateMapEvents(UpdateMapEvents & pack) {}
-	virtual void visitUpdateCastleEvents(UpdateCastleEvents & pack) {}
 	virtual void visitChangeFormation(ChangeFormation & pack) {}
 	virtual void visitRemoveObject(RemoveObject & pack) {}
 	virtual void visitTryMoveHero(TryMoveHero & pack) {}

lib/networkPacks/NetPacksLib.cpp

@@ -225,16 +225,6 @@ void UpdateArtHandlerLists::visitTyped(ICPackVisitor & visitor)
 	visitor.visitUpdateArtHandlerLists(*this);
 }
 
-void UpdateMapEvents::visitTyped(ICPackVisitor & visitor)
-{
-	visitor.visitUpdateMapEvents(*this);
-}
-
-void UpdateCastleEvents::visitTyped(ICPackVisitor & visitor)
-{
-	visitor.visitUpdateCastleEvents(*this);
-}
-
 void ChangeFormation::visitTyped(ICPackVisitor & visitor)
 {
 	visitor.visitChangeFormation(*this);
@@ -907,17 +897,6 @@ void UpdateArtHandlerLists::applyGs(CGameState * gs) const
 	gs->allocatedArtifacts = allocatedArtifacts;
 }
 
-void UpdateMapEvents::applyGs(CGameState * gs) const
-{
-	gs->map->events = events;
-}
-
-void UpdateCastleEvents::applyGs(CGameState * gs) const
-{
-	auto * t = gs->getTown(town);
-	t->events = events;
-}
-
 void ChangeFormation::applyGs(CGameState * gs) const
 {
 	gs->getHero(hid)->setFormation(formation);
@@ -2114,7 +2093,7 @@ void BattleResultAccepted::applyGs(CGameState * gs) const
 			res.hero->removeBonusesRecursive(Bonus::OneBattle);
 	}
 
-	if(winnerSide != 2)
+	if(winnerSide != BattleSide::NONE)
 	{
 		// Grow up growing artifacts
 		const auto hero = heroResult[winnerSide].hero;
@@ -2135,10 +2114,10 @@ void BattleResultAccepted::applyGs(CGameState * gs) const
 
 	if(VLC->settings()->getBoolean(EGameSettings::MODULE_STACK_EXPERIENCE))
 	{
-		if(heroResult[0].army)
-			heroResult[0].army->giveStackExp(heroResult[0].exp);
-		if(heroResult[1].army)
-			heroResult[1].army->giveStackExp(heroResult[1].exp);
+		if(heroResult[BattleSide::ATTACKER].army)
+			heroResult[BattleSide::ATTACKER].army->giveStackExp(heroResult[BattleSide::ATTACKER].exp);
+		if(heroResult[BattleSide::DEFENDER].army)
+			heroResult[BattleSide::DEFENDER].army->giveStackExp(heroResult[BattleSide::DEFENDER].exp);
 		CBonusSystemNode::treeHasChanged();
 	}
 
@@ -2242,19 +2221,14 @@ void StartAction::applyGs(CGameState *gs)
 	else
 	{
 		if(ba.actionType == EActionType::HERO_SPELL)
-			gs->getBattle(battleID)->sides[ba.side].usedSpellsHistory.push_back(ba.spell);
+			gs->getBattle(battleID)->getSide(ba.side).usedSpellsHistory.push_back(ba.spell);
 	}
 }
 
 void BattleSpellCast::applyGs(CGameState * gs) const
 {
-	if(castByHero)
-	{
-		if(side < 2)
-		{
-			gs->getBattle(battleID)->sides[side].castSpellsCount++;
-		}
-	}
+	if(castByHero && side != BattleSide::NONE)
+		gs->getBattle(battleID)->getSide(side).castSpellsCount++;
 }
 
 void SetStackEffect::applyGs(CGameState *gs)
@@ -2388,7 +2362,7 @@ void BattleSetStackProperty::applyGs(CGameState * gs) const
 		}
 		case ENCHANTER_COUNTER:
 		{
-			auto & counter = gs->getBattle(battleID)->sides[gs->getBattle(battleID)->whatSide(stack->unitOwner())].enchanterCounter;
+			auto & counter = gs->getBattle(battleID)->getSide(gs->getBattle(battleID)->whatSide(stack->unitOwner())).enchanterCounter;
 			if(absolute)
 				counter = val;
 			else
@@ -2457,11 +2431,26 @@ void EntitiesChanged::applyGs(CGameState * gs)
 void SetRewardableConfiguration::applyGs(CGameState * gs)
 {
 	auto * objectPtr = gs->getObjInstance(objectID);
-	auto * rewardablePtr = dynamic_cast<CRewardableObject *>(objectPtr);
 
-	assert(rewardablePtr);
+	if (!buildingID.hasValue())
+	{
+		auto * rewardablePtr = dynamic_cast<CRewardableObject *>(objectPtr);
+		assert(rewardablePtr);
+		rewardablePtr->configuration = configuration;
+	}
+	else
+	{
+		auto * townPtr = dynamic_cast<CGTownInstance*>(objectPtr);
+		CGTownBuilding * buildingPtr = nullptr;
 
-	rewardablePtr->configuration = configuration;
+		for (CGTownBuilding * building : townPtr->bonusingBuildings)
+			if (building->getBuildingType() == buildingID)
+				buildingPtr = building;
+
+		auto * rewardablePtr = dynamic_cast<CTownRewardableBuilding *>(buildingPtr);
+		assert(rewardablePtr);
+		rewardablePtr->configuration = configuration;
+	}
 }
 
 void SetBankConfiguration::applyGs(CGameState * gs)

lib/networkPacks/PacksForClient.h

@@ -545,34 +545,6 @@ struct DLL_LINKAGE UpdateArtHandlerLists : public CPackForClient
 	}
 };
 
-struct DLL_LINKAGE UpdateMapEvents : public CPackForClient
-{
-	std::list<CMapEvent> events;
-
-	void applyGs(CGameState * gs) const;
-	void visitTyped(ICPackVisitor & visitor) override;
-
-	template <typename Handler> void serialize(Handler & h)
-	{
-		h & events;
-	}
-};
-
-struct DLL_LINKAGE UpdateCastleEvents : public CPackForClient
-{
-	ObjectInstanceID town;
-	std::vector<CCastleEvent> events;
-
-	void applyGs(CGameState * gs) const;
-	void visitTyped(ICPackVisitor & visitor) override;
-
-	template <typename Handler> void serialize(Handler & h)
-	{
-		h & town;
-		h & events;
-	}
-};
-
 struct DLL_LINKAGE ChangeFormation : public CPackForClient
 {
 	ObjectInstanceID hid;

lib/networkPacks/PacksForClientBattle.h

@@ -109,8 +109,8 @@ struct DLL_LINKAGE BattleResultAccepted : public CPackForClient
 	};
 
 	BattleID battleID = BattleID::NONE;
-	std::array<HeroBattleResults, 2> heroResult;
-	ui8 winnerSide;
+	BattleSideArray<HeroBattleResults> heroResult;
+	BattleSide winnerSide;
 
 	template <typename Handler> void serialize(Handler & h)
 	{
@@ -127,9 +127,9 @@ struct DLL_LINKAGE BattleResult : public Query
 
 	BattleID battleID = BattleID::NONE;
 	EBattleResult result = EBattleResult::NORMAL;
-	ui8 winner = 2; //0 - attacker, 1 - defender, [2 - draw (should be possible?)]
-	std::map<CreatureID, si32> casualties[2]; //first => casualties of attackers - map crid => number
-	TExpType exp[2] = {0, 0}; //exp for attacker and defender
+	BattleSide winner = BattleSide::NONE; //0 - attacker, 1 - defender, [2 - draw (should be possible?)]
+	BattleSideArray<std::map<CreatureID, si32>> casualties; //first => casualties of attackers - map crid => number
+	BattleSideArray<TExpType> exp{0,0}; //exp for attacker and defender
 	std::set<ArtifactInstanceID> artifacts; //artifacts taken from loser to winner - currently unused
 
 	void visitTyped(ICPackVisitor & visitor) override;
@@ -140,8 +140,7 @@ struct DLL_LINKAGE BattleResult : public Query
 		h & queryID;
 		h & result;
 		h & winner;
-		h & casualties[0];
-		h & casualties[1];
+		h & casualties;
 		h & exp;
 		h & artifacts;
 		assert(battleID != BattleID::NONE);
@@ -369,7 +368,7 @@ struct DLL_LINKAGE BattleSpellCast : public CPackForClient
 
 	BattleID battleID = BattleID::NONE;
 	bool activeCast = true;
-	ui8 side = 0; //which hero did cast spell: 0 - attacker, 1 - defender
+	BattleSide side = BattleSide::NONE; //which hero did cast spell
 	SpellID spellID; //id of spell
 	ui8 manaGained = 0; //mana channeling ability
 	BattleHex tile; //destination tile (may not be set in some global/mass spells

lib/registerTypes/RegisterTypesClientPacks.h

@@ -44,8 +44,6 @@ void registerTypesClientPacks(Serializer &s)
 	s.template registerType<CPackForClient, PlayerReinitInterface>();
 	s.template registerType<CPackForClient, RemoveBonus>();
 	s.template registerType<CPackForClient, UpdateArtHandlerLists>();
-	s.template registerType<CPackForClient, UpdateMapEvents>();
-	s.template registerType<CPackForClient, UpdateCastleEvents>();
 	s.template registerType<CPackForClient, ChangeFormation>();
 	s.template registerType<CPackForClient, RemoveObject>();
 	s.template registerType<CPackForClient, TryMoveHero>();

lib/serializer/ESerializationVersion.h

@@ -50,6 +50,7 @@ enum class ESerializationVersion : int32_t
 	BANK_UNIT_PLACEMENT, // 843 Banks have unit placement flag
 
 	RELEASE_152 = BANK_UNIT_PLACEMENT,
+	RELEASE_156 = BANK_UNIT_PLACEMENT,
 
 	COMPACT_STRING_SERIALIZATION, // 844 - optimized serialization of previously encountered strings
 	COMPACT_INTEGER_SERIALIZATION, // 845 - serialize integers in forms similar to protobuf
@@ -61,6 +62,7 @@ enum class ESerializationVersion : int32_t
 	PLAYER_HANDICAP, // 851 - player handicap selection at game start
 	STATISTICS, // 852 - removed random number generators from library classes
 	CAMPAIGN_REGIONS, // 853 - configurable campaign regions
+	EVENTS_PLAYER_SET, // 854 - map & town events use std::set instead of bitmask to store player list
 
-	CURRENT = CAMPAIGN_REGIONS
+	CURRENT = EVENTS_PLAYER_SET
 };

lib/spells/BattleSpellMechanics.cpp

@@ -193,16 +193,16 @@ bool BattleSpellMechanics::canBeCast(Problem & problem) const
 		return adaptProblem(ESpellCastProblem::ADVMAP_SPELL_INSTEAD_OF_BATTLE_SPELL, problem);
 
 	const PlayerColor player = caster->getCasterOwner();
-	const auto side = battle()->playerToSide(player);
+	const BattleSide side = battle()->playerToSide(player);
 
-	if(!side)
+	if(side == BattleSide::NONE)
 		return adaptProblem(ESpellCastProblem::INVALID, problem);
 
 	//effect like Recanter's Cloak. Blocks also passive casting.
 	//TODO: check creature abilities to block
 	//TODO: check any possible caster
 
-	if(battle()->battleMaxSpellLevel(side.value()) < getSpellLevel() || battle()->battleMinSpellLevel(side.value()) > getSpellLevel())
+	if(battle()->battleMaxSpellLevel(side) < getSpellLevel() || battle()->battleMinSpellLevel(side) > getSpellLevel())
 		return adaptProblem(ESpellCastProblem::SPELL_LEVEL_LIMIT_EXCEEDED, problem);
 
 	return effects->applicable(problem, this);
@@ -284,7 +284,7 @@ void BattleSpellMechanics::cast(ServerCallback * server, const Target & target)
 	const CGHeroInstance * otherHero = nullptr;
 	{
 		//check it there is opponent hero
-		const ui8 otherSide = battle()->otherSide(casterSide);
+		const BattleSide otherSide = battle()->otherSide(casterSide);
 
 		if(battle()->battleHasHero(otherSide))
 			otherHero = battle()->battleGetFightingHero(otherSide);

lib/spells/ISpellMechanics.cpp

@@ -397,7 +397,7 @@ std::unique_ptr<ISpellMechanicsFactory> ISpellMechanicsFactory::get(const CSpell
 ///Mechanics
 Mechanics::Mechanics()
 	: caster(nullptr),
-	casterSide(0)
+	casterSide(BattleSide::NONE)
 {
 
 }
@@ -413,8 +413,7 @@ BaseMechanics::BaseMechanics(const IBattleCast * event):
 {
 	caster = event->getCaster();
 
-	//FIXME: do not crash on invalid side
-	casterSide = cb->playerToSide(caster->getCasterOwner()).value();
+	casterSide = cb->playerToSide(caster->getCasterOwner());
 
 	{
 		auto value = event->getSpellLevel();

lib/spells/ISpellMechanics.h

@@ -252,7 +252,7 @@ public:
 
 	const Caster * caster;
 
-	ui8 casterSide;
+	BattleSide casterSide;
 
 protected:
 	Mechanics();

lib/spells/effects/Moat.cpp

@@ -134,7 +134,7 @@ void Moat::placeObstacles(ServerCallback * server, const Mechanics * m, const Ef
 	BattleObstaclesChanged pack;
 	pack.battleID = m->battle()->getBattle()->getBattleID();
 
-	auto all = m->battle()->battleGetAllObstacles(BattlePerspective::ALL_KNOWING);
+	auto all = m->battle()->battleGetAllObstacles(BattleSide::ALL_KNOWING);
 
 	int obstacleIdToGive = 1;
 	for(auto & one : all)

lib/spells/effects/Obstacle.cpp

@@ -274,7 +274,7 @@ void Obstacle::placeObstacles(ServerCallback * server, const Mechanics * m, cons
 	BattleObstaclesChanged pack;
 	pack.battleID = m->battle()->getBattle()->getBattleID();
 
-	auto all = m->battle()->battleGetAllObstacles(BattlePerspective::ALL_KNOWING);
+	auto all = m->battle()->battleGetAllObstacles(BattleSide::ALL_KNOWING);
 
 	int obstacleIdToGive = 1;
 	for(auto & one : all)