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

AI/Nullkiller/Analyzers/ArmyManager.cpp

@@ -96,14 +96,14 @@ public:
 std::vector<SlotInfo> ArmyManager::getBestArmy(const IBonusBearer * armyCarrier, const CCreatureSet * target, const CCreatureSet * source) const
 {
 	auto sortedSlots = getSortedSlots(target, source);
-	std::map<TFaction, uint64_t> alignmentMap;
+	std::map<FactionID, uint64_t> alignmentMap;
 
 	for(auto & slot : sortedSlots)
 	{
-		alignmentMap[slot.creature->getFactionIndex()] += slot.power;
+		alignmentMap[slot.creature->getFaction()] += slot.power;
 	}
 
-	std::set<TFaction> allowedFactions;
+	std::set<FactionID> allowedFactions;
 	std::vector<SlotInfo> resultingArmy;
 	uint64_t armyValue = 0;
 
@@ -121,7 +121,7 @@ std::vector<SlotInfo> ArmyManager::getBestArmy(const IBonusBearer * armyCarrier,
 
 	while(allowedFactions.size() < alignmentMap.size())
 	{
-		auto strongestAlignment = vstd::maxElementByFun(alignmentMap, [&](std::pair<TFaction, uint64_t> pair) -> uint64_t
+		auto strongestAlignment = vstd::maxElementByFun(alignmentMap, [&](std::pair<FactionID, uint64_t> pair) -> uint64_t
 		{
 			return vstd::contains(allowedFactions, pair.first) ? 0 : pair.second;
 		});
@@ -134,7 +134,7 @@ std::vector<SlotInfo> ArmyManager::getBestArmy(const IBonusBearer * armyCarrier,
 
 		for(auto & slot : sortedSlots)
 		{
-			if(vstd::contains(allowedFactions, slot.creature->getFactionIndex()))
+			if(vstd::contains(allowedFactions, slot.creature->getFaction()))
 			{
 				auto slotID = newArmyInstance.getSlotFor(slot.creature);
 

AI/VCAI/Goals/GatherTroops.cpp

@@ -94,7 +94,7 @@ TGoalVec GatherTroops::getAllPossibleSubgoals()
 		}
 
 		auto creature = VLC->creatures()->getByIndex(objid);
-		if(t->subID == creature->getFactionIndex()) //TODO: how to force AI to build unupgraded creatures? :O
+		if(t->subID == creature->getFaction()) //TODO: how to force AI to build unupgraded creatures? :O
 		{
 			auto creatures = vstd::tryAt(t->town->creatures, creature->getLevel() - 1);
 			if(!creatures)

client/widgets/MiscWidgets.cpp

@@ -448,7 +448,7 @@ CCreaturePic::CCreaturePic(int x, int y, const CCreature * cre, bool Big, bool A
 	pos.x+=x;
 	pos.y+=y;
 
-	TFaction faction = cre->getFactionIndex();
+	auto faction = cre->getFaction();
 
 	assert(CGI->townh->size() > faction);
 

client/windows/CCastleInterface.cpp

@@ -892,7 +892,7 @@ void CCastleBuildings::enterFountain(const BuildingID & building, BuildingSubID:
 	std::string hasNotProduced;
 	std::string hasProduced;
 
-	if(this->town->town->faction->getIndex() == (TFaction)ETownType::RAMPART)
+	if(this->town->town->faction->getIndex() == ETownType::RAMPART)
 	{
 		hasNotProduced = CGI->generaltexth->allTexts[677];
 		hasProduced = CGI->generaltexth->allTexts[678];

cmake_modules/VCMI_lib.cmake

@@ -163,6 +163,7 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 
 		${MAIN_LIB_DIR}/vstd/StringUtils.cpp
 
+		${MAIN_LIB_DIR}/BasicTypes.cpp
 		${MAIN_LIB_DIR}/BattleFieldHandler.cpp
 		${MAIN_LIB_DIR}/CAndroidVMHelper.cpp
 		${MAIN_LIB_DIR}/CArtHandler.cpp

include/vcmi/Creature.h

@@ -18,7 +18,7 @@ class CreatureID;
 class ResourceSet;
 enum class EGameResID : int8_t;
 
-class DLL_LINKAGE Creature : public EntityWithBonuses<CreatureID>
+class DLL_LINKAGE Creature : public EntityWithNativeTerrain<CreatureID>
 {
 protected:
 	// use getNamePlural/Singular instead
@@ -41,7 +41,6 @@ public:
 	virtual int32_t getLevel() const = 0;
 	virtual int32_t getGrowth() const = 0;
 	virtual int32_t getHorde() const = 0;
-	virtual int32_t getFactionIndex() const = 0;
 
 	virtual int32_t getBaseAttack() const = 0;
 	virtual int32_t getBaseDefense() const = 0;

include/vcmi/Entity.h

@@ -13,13 +13,33 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 class IBonusBearer;
+class FactionID;
+enum class ETerrainId;
+template<typename T> class Identifier;
 
-class DLL_LINKAGE WithBonuses
+class DLL_LINKAGE IConstBonusProvider
 {
 public:
 	virtual const IBonusBearer * getBonusBearer() const = 0;
 };
 
+class DLL_LINKAGE INativeTerrainProvider
+{
+public:
+	virtual Identifier<ETerrainId> getNativeTerrain() const = 0;
+	virtual FactionID getFaction() const = 0;
+	virtual bool isNativeTerrain(Identifier<ETerrainId> terrain) const;
+};
+
+class DLL_LINKAGE IConstBonusNativeTerrainProvider: public IConstBonusProvider, public INativeTerrainProvider
+{
+public:
+	/**
+	Returns native terrain considering some terrain bonuses.
+	*/
+	virtual Identifier<ETerrainId> getNativeTerrain() const;
+};
+
 class DLL_LINKAGE Entity
 {
 public:
@@ -44,7 +64,12 @@ public:
 };
 
 template <typename IdType>
-class DLL_LINKAGE EntityWithBonuses : public EntityT<IdType>, public WithBonuses
+class DLL_LINKAGE EntityWithBonuses : public EntityT<IdType>, public IConstBonusProvider
+{
+};
+
+template <typename IdType>
+class DLL_LINKAGE EntityWithNativeTerrain : public EntityT<IdType>, public IConstBonusNativeTerrainProvider
 {
 };
 

include/vcmi/Faction.h

@@ -15,15 +15,12 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 class FactionID;
-enum class ETerrainId;
 enum class EAlignment : uint8_t;
-template<typename T> class Identifier;
 
-class DLL_LINKAGE Faction : public EntityT<FactionID>
+class DLL_LINKAGE Faction : public EntityT<FactionID>, public INativeTerrainProvider
 {
 public:
 	virtual bool hasTown() const = 0;
-	virtual Identifier<ETerrainId> getNativeTerrain() const = 0;
 	virtual EAlignment getAlignment() const = 0;
 };
 

lib/BasicTypes.cpp

@@ -0,0 +1,41 @@
+/*
+ * BasicTypes.cpp, 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
+ *
+ */
+
+#include "StdInc.h"
+
+#include "VCMI_Lib.h"
+#include "GameConstants.h"
+#include "HeroBonus.h"
+
+#include <vcmi/Entity.h>
+#include <vcmi/Faction.h>
+#include <vcmi/FactionService.h>
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+bool INativeTerrainProvider::isNativeTerrain(TerrainId terrain) const
+{
+	auto native = getNativeTerrain();
+	return native == terrain || native == ETerrainId::ANY_TERRAIN;
+}
+
+TerrainId IConstBonusNativeTerrainProvider::getNativeTerrain() const
+{
+	constexpr auto any = TerrainId(ETerrainId::ANY_TERRAIN);
+	const std::string cachingStringNoTerrainPenalty = "type_NO_TERRAIN_PENALTY_sANY";
+	static const auto selectorNoTerrainPenalty = Selector::typeSubtype(Bonus::NO_TERRAIN_PENALTY, any);
+
+	//this code is used in the CreatureTerrainLimiter::limit to setup battle bonuses
+	//and in the CGHeroInstance::getNativeTerrain() to setup movement bonuses or/and penalties.
+	return getBonusBearer()->hasBonus(selectorNoTerrainPenalty, cachingStringNoTerrainPenalty)
+		? any : VLC->factions()->getById(getFaction())->getNativeTerrain();
+}
+
+VCMI_LIB_NAMESPACE_END

lib/CCreatureHandler.cpp

@@ -109,9 +109,9 @@ int32_t CCreature::getHorde() const
 	return hordeGrowth;
 }
 
-int32_t CCreature::getFactionIndex() const
+FactionID CCreature::getFaction() const
 {
-	return faction;
+	return FactionID(faction);
 }
 
 int32_t CCreature::getBaseAttack() const
@@ -330,24 +330,6 @@ std::string CCreature::nodeName() const
 	return "\"" + getNamePluralTextID() + "\"";
 }
 
-bool CCreature::isItNativeTerrain(TerrainId terrain) const
-{
-	auto native = getNativeTerrain();
-	return native == terrain || native == ETerrainId::ANY_TERRAIN;
-}
-
-TerrainId CCreature::getNativeTerrain() const
-{
-	const std::string cachingStringNoTerrainPenalty = "type_NO_TERRAIN_PENALTY_sANY";
-	static const auto selectorNoTerrainPenalty = Selector::typeSubtype(Bonus::NO_TERRAIN_PENALTY, static_cast<int>(ETerrainId::ANY_TERRAIN));
-
-	//this code is used in the CreatureTerrainLimiter::limit to setup battle bonuses
-	//and in the CGHeroInstance::getNativeTerrain() to setup movement bonuses or/and penalties.
-	return hasBonus(selectorNoTerrainPenalty, cachingStringNoTerrainPenalty)
-		? TerrainId(ETerrainId::ANY_TERRAIN)
-		: VLC->factions()->getByIndex(faction)->getNativeTerrain();
-}
-
 void CCreature::updateFrom(const JsonNode & data)
 {
 	JsonUpdater handler(nullptr, data);
@@ -421,13 +403,6 @@ CCreatureHandler::CCreatureHandler()
 	: expAfterUpgrade(0)
 {
 	VLC->creh = this;
-
-	allCreatures.setDescription("All creatures");
-	allCreatures.setNodeType(CBonusSystemNode::ENodeTypes::ALL_CREATURES);
-	creaturesOfLevel[0].setDescription("Creatures of unnormalized tier");
-
-	for(int i = 1; i < ARRAY_COUNT(creaturesOfLevel); i++)
-		creaturesOfLevel[i].setDescription("Creatures of tier " + std::to_string(i));
 	loadCommanders();
 }
 
@@ -692,10 +667,24 @@ std::vector<bool> CCreatureHandler::getDefaultAllowed() const
 	return ret;
 }
 
-void CCreatureHandler::loadCrExpBon()
+void CCreatureHandler::loadCrExpBon(CBonusSystemNode & globalEffects)
 {
 	if (VLC->settings()->getBoolean(EGameSettings::MODULE_STACK_EXPERIENCE)) 	//reading default stack experience bonuses
 	{
+		logGlobal->debug("\tLoading stack experience bonuses");
+		auto addBonusForAllCreatures = [&](std::shared_ptr<Bonus> b) {
+			auto limiter = std::make_shared<CreatureLevelLimiter>();
+			b->addLimiter(limiter);
+			globalEffects.addNewBonus(b);
+		};
+		auto addBonusForTier = [&](int tier, std::shared_ptr<Bonus> b) {
+			assert(vstd::iswithin(tier, 1, 7));
+			//bonuses from level 7 are given to high-level creatures too
+			auto max = tier == GameConstants::CREATURES_PER_TOWN ? std::numeric_limits<int>::max() : tier + 1;
+			auto limiter = std::make_shared<CreatureLevelLimiter>(tier, max);
+			b->addLimiter(limiter);
+			globalEffects.addNewBonus(b);
+		};
 		CLegacyConfigParser parser("DATA/CREXPBON.TXT");
 
 		Bonus b; //prototype with some default properties
@@ -733,10 +722,7 @@ void CCreatureHandler::loadCrExpBon()
 			bl.clear();
 			loadStackExp(b, bl, parser);
 			for(const auto & b : bl)
-			{
 				addBonusForTier(7, b);
-				creaturesOfLevel[0].addNewBonus(b); //bonuses from level 7 are given to high-level creatures
-			}
 			parser.endLine();
 		}
 		do //parse everything that's left
@@ -924,7 +910,7 @@ void CCreatureHandler::loadCreatureJson(CCreature * creature, const JsonNode & c
 
 	VLC->modh->identifiers.requestIdentifier("faction", config["faction"], [=](si32 faction)
 	{
-		creature->faction = faction;
+		creature->faction = FactionID(faction);
 	});
 
 	for(const JsonNode &value : config["upgrades"].Vector())
@@ -1352,12 +1338,10 @@ CreatureID CCreatureHandler::pickRandomMonster(CRandomGenerator & rand, int tier
 	{
 		assert(vstd::iswithin(tier, 1, 7));
 		std::vector<CreatureID> allowed;
-		for(const CBonusSystemNode *b : creaturesOfLevel[tier].getChildrenNodes())
+		for(const auto & creature : objects)
 		{
-			assert(b->getNodeType() == CBonusSystemNode::CREATURE);
-			const auto * crea = dynamic_cast<const CCreature *>(b);
-			if(crea && !crea->special)
-				allowed.push_back(crea->getId());
+			if(!creature->special && creature->level == tier)
+				allowed.push_back(creature->getId());
 		}
 
 		if(allowed.empty())
@@ -1372,49 +1356,10 @@ CreatureID CCreatureHandler::pickRandomMonster(CRandomGenerator & rand, int tier
 	return CreatureID(r);
 }
 
-void CCreatureHandler::addBonusForTier(int tier, const std::shared_ptr<Bonus> & b)
-{
-	assert(vstd::iswithin(tier, 1, 7));
-	creaturesOfLevel[tier].addNewBonus(b);
-}
-
-void CCreatureHandler::addBonusForAllCreatures(const std::shared_ptr<Bonus> & b)
-{
-	const auto & exportedBonuses = allCreatures.getExportedBonusList();
-	for(const auto & bonus : exportedBonuses)
-	{
-		if(bonus->type == b->type && bonus->subtype == b->subtype)
-			return;
-	}
-	allCreatures.addNewBonus(b);
-}
-
-void CCreatureHandler::removeBonusesFromAllCreatures()
-{
-	allCreatures.removeBonuses(Selector::all);
-}
-
-void CCreatureHandler::buildBonusTreeForTiers()
-{
-	for(CCreature * c : objects)
-	{
-		if(vstd::isbetween(c->level, 0, ARRAY_COUNT(creaturesOfLevel)))
-			c->attachTo(creaturesOfLevel[c->level]);
-		else
-			c->attachTo(creaturesOfLevel[0]);
-	}
-	for(CBonusSystemNode &b : creaturesOfLevel)
-		b.attachTo(allCreatures);
-}
 
 void CCreatureHandler::afterLoadFinalization()
 {
 
 }
 
-void CCreatureHandler::deserializationFix()
-{
-	buildBonusTreeForTiers();
-}
-
 VCMI_LIB_NAMESPACE_END

lib/CCreatureHandler.h

@@ -39,7 +39,7 @@ class DLL_LINKAGE CCreature : public Creature, public CBonusSystemNode
 
 	CreatureID idNumber;
 
-	TFaction faction = 0;
+	FactionID faction = FactionID::NEUTRAL;
 	ui8 level = 0; // 0 - unknown; 1-7 for "usual" creatures
 
 	//stats that are not handled by bonus system
@@ -162,11 +162,7 @@ public:
 	std::string getNamePluralTextID() const override;
 	std::string getNameSingularTextID() const override;
 
-	bool isItNativeTerrain(TerrainId terrain) const;
-	/**
-	Returns creature native terrain considering some terrain bonuses.
-	*/
-	TerrainId getNativeTerrain() const;
+	FactionID getFaction() const override;
 	int32_t getIndex() const override;
 	int32_t getIconIndex() const override;
 	std::string getJsonKey() const override;
@@ -182,7 +178,6 @@ public:
 	int32_t getLevel() const override;
 	int32_t getGrowth() const override;
 	int32_t getHorde() const override;
-	int32_t getFactionIndex() const override;
 
 	int32_t getBaseAttack() const override;
 	int32_t getBaseDefense() const override;
@@ -262,9 +257,6 @@ private:
 class DLL_LINKAGE CCreatureHandler : public CHandlerBase<CreatureID, Creature, CCreature, CreatureService>
 {
 private:
-	CBonusSystemNode allCreatures;
-	CBonusSystemNode creaturesOfLevel[GameConstants::CREATURES_PER_TOWN + 1];//index 0 is used for creatures of unknown tier or outside <1-7> range
-
 	void loadJsonAnimation(CCreature * creature, const JsonNode & graphics) const;
 	void loadStackExperience(CCreature * creature, const JsonNode & input) const;
 	void loadCreatureJson(CCreature * creature, const JsonNode & config) const;
@@ -304,19 +296,13 @@ public:
 
 	const CCreature * getCreature(const std::string & scope, const std::string & identifier) const;
 
-	void deserializationFix();
 	CreatureID pickRandomMonster(CRandomGenerator & rand, int tier = -1) const; //tier <1 - CREATURES_PER_TOWN> or -1 for any
-	void addBonusForTier(int tier, const std::shared_ptr<Bonus> & b); //tier must be <1-7>
-	void addBonusForAllCreatures(const std::shared_ptr<Bonus> & b); //due to CBonusSystem::addNewBonus(const std::shared_ptr<Bonus>& b);
-	void removeBonusesFromAllCreatures();
 
 	CCreatureHandler();
 	~CCreatureHandler();
 
-	/// load all creatures from H3 files
-	void loadCrExpBon();
-	/// generates tier-specific bonus tree entries
-	void buildBonusTreeForTiers();
+	/// load all stack experience bonuses from H3 files
+	void loadCrExpBon(CBonusSystemNode & globalEffects);
 
 	void afterLoadFinalization() override;
 
@@ -335,9 +321,6 @@ public:
 		h & skillLevels;
 		h & skillRequirements;
 		h & commanderLevelPremy;
-		h & allCreatures;
-		h & creaturesOfLevel;
-		BONUS_TREE_DESERIALIZATION_FIX
 	}
 };
 

lib/CCreatureSet.cpp

@@ -25,6 +25,9 @@
 #include "serializer/JsonSerializeFormat.h"
 #include "NetPacksBase.h"
 
+#include <vcmi/FactionService.h>
+#include <vcmi/Faction.h>
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 
@@ -912,6 +915,19 @@ void CStackInstance::serializeJson(JsonSerializeFormat & handler)
 	}
 }
 
+FactionID CStackInstance::getFaction() const
+{
+	if(type)
+		return type->getFaction();
+		
+	return FactionID::NEUTRAL;
+}
+
+const IBonusBearer* CStackInstance::getBonusBearer() const
+{
+	return this;
+}
+
 CCommanderInstance::CCommanderInstance()
 {
 	init();

lib/CCreatureSet.h

@@ -14,6 +14,8 @@
 #include "CArtHandler.h"
 #include "CCreatureHandler.h"
 
+#include <vcmi/Entity.h>
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 class JsonNode;
@@ -61,7 +63,7 @@ public:
 	void serializeJson(JsonSerializeFormat & handler);
 };
 
-class DLL_LINKAGE CStackInstance : public CBonusSystemNode, public CStackBasicDescriptor, public CArtifactSet
+class DLL_LINKAGE CStackInstance : public CBonusSystemNode, public CStackBasicDescriptor, public CArtifactSet, public IConstBonusNativeTerrainProvider
 {
 protected:
 	const CArmedInstance *_armyObj; //stack must be part of some army, army must be part of some object
@@ -92,6 +94,11 @@ public:
 	std::string bonusToString(const std::shared_ptr<Bonus>& bonus, bool description) const override; // how would bonus description look for this particular type of node
 	std::string bonusToGraphics(const std::shared_ptr<Bonus> & bonus) const; //file name of graphics from StackSkills , in future possibly others
 
+	//IConstBonusProvider
+	const IBonusBearer* getBonusBearer() const override;
+	//INativeTerrainProvider
+	FactionID getFaction() const override;
+
 	virtual ui64 getPower() const;
 	CCreature::CreatureQuantityId getQuantityID() const;
 	std::string getQuantityTXT(bool capitalized = true) const;

lib/CGameInfoCallback.cpp

@@ -359,7 +359,7 @@ bool CGameInfoCallback::getHeroInfo(const CGObjectInstance * hero, InfoAboutHero
 
 			for(auto creature : VLC->creh->objects)
 			{
-				if(static_cast<si16>(creature->getFactionIndex()) == factionIndex && static_cast<int>(creature->getAIValue()) > maxAIValue)
+				if(creature->getFaction() == factionIndex && static_cast<int>(creature->getAIValue()) > maxAIValue)
 				{
 					maxAIValue = creature->getAIValue();
 					mostStrong = creature;

lib/CGameState.cpp

@@ -372,7 +372,7 @@ CGHeroInstance * CGameState::HeroesPool::pickHeroFor(bool native,
 			    ( !bannedClass || elem.second->type->heroClass != bannedClass) ) // and his class is not same as other hero
 			{
 				pool.push_back(elem.second);
-				sum += elem.second->type->heroClass->selectionProbability[town->faction->getIndex()]; //total weight
+				sum += elem.second->type->heroClass->selectionProbability[town->faction->getId()]; //total weight
 			}
 		}
 		if(pool.empty() || sum == 0)
@@ -384,7 +384,7 @@ CGHeroInstance * CGameState::HeroesPool::pickHeroFor(bool native,
 		r = rand.nextInt(sum - 1);
 		for (auto & elem : pool)
 		{
-			r -= elem->type->heroClass->selectionProbability[town->faction->getIndex()];
+			r -= elem->type->heroClass->selectionProbability[town->faction->getId()];
 			if(r < 0)
 			{
 				ret = elem;
@@ -958,6 +958,7 @@ void CGameState::initGlobalBonuses()
 		bonus->sid = -1; //there is one global object
 		globalEffects.addNewBonus(bonus);
 	}
+	VLC->creh->loadCrExpBon(globalEffects);
 }
 
 void CGameState::initGrailPosition()
@@ -1746,8 +1747,8 @@ void CGameState::initTowns()
 		}
 		if(vti->getNameTranslated().empty())
 		{
-			size_t nameID = getRandomGenerator().nextInt(vti->town->getRandomNamesCount() - 1);
-			vti->setNameTranslated(vti->town->getRandomNameTranslated(nameID));
+			size_t nameID = getRandomGenerator().nextInt(vti->getTown()->getRandomNamesCount() - 1);
+			vti->setNameTranslated(vti->getTown()->getRandomNameTranslated(nameID));
 		}
 
 		//init buildings
@@ -1777,14 +1778,14 @@ void CGameState::initTowns()
 			if (vstd::contains(vti->builtBuildings, (BuildingID::HORDE_PLACEHOLDER1 - i))) //if we have horde for this level
 			{
 				vti->builtBuildings.erase(BuildingID(BuildingID::HORDE_PLACEHOLDER1 - i));//remove old ID
-				if (vti->town->hordeLvl.at(0) == i)//if town first horde is this one
+				if (vti->getTown()->hordeLvl.at(0) == i)//if town first horde is this one
 				{
 					vti->builtBuildings.insert(BuildingID::HORDE_1);//add it
 					//if we have upgraded dwelling as well
 					if (vstd::contains(vti->builtBuildings, (BuildingID::DWELL_UP_FIRST + i)))
 						vti->builtBuildings.insert(BuildingID::HORDE_1_UPGR);//add it as well
 				}
-				if (vti->town->hordeLvl.at(1) == i)//if town second horde is this one
+				if (vti->getTown()->hordeLvl.at(1) == i)//if town second horde is this one
 				{
 					vti->builtBuildings.insert(BuildingID::HORDE_2);
 					if (vstd::contains(vti->builtBuildings, (BuildingID::DWELL_UP_FIRST + i)))
@@ -1797,7 +1798,7 @@ void CGameState::initTowns()
 		//But DO NOT remove horde placeholders before they are replaced
 		vstd::erase_if(vti->builtBuildings, [vti](const BuildingID & bid)
 			{
-				return !vti->town->buildings.count(bid) || !vti->town->buildings.at(bid);
+				return !vti->getTown()->buildings.count(bid) || !vti->getTown()->buildings.at(bid);
 			});
 
 		if (vstd::contains(vti->builtBuildings, BuildingID::SHIPYARD) && vti->shipyardStatus()==IBoatGenerator::TILE_BLOCKED)
@@ -1806,7 +1807,7 @@ void CGameState::initTowns()
 		//Early check for #1444-like problems
 		for(const auto & building : vti->builtBuildings)
 		{
-			assert(vti->town->buildings.at(building) != nullptr);
+			assert(vti->getTown()->buildings.at(building) != nullptr);
 			MAYBE_UNUSED(building);
 		}
 
@@ -1817,9 +1818,9 @@ void CGameState::initTowns()
 				if (vstd::contains(ev.buildings,(-31-i))) //if we have horde for this level
 				{
 					ev.buildings.erase(BuildingID(-31-i));
-					if (vti->town->hordeLvl.at(0) == i)
+					if (vti->getTown()->hordeLvl.at(0) == i)
 						ev.buildings.insert(BuildingID::HORDE_1);
-					if (vti->town->hordeLvl.at(1) == i)
+					if (vti->getTown()->hordeLvl.at(1) == i)
 						ev.buildings.insert(BuildingID::HORDE_2);
 				}
 		}
@@ -1838,7 +1839,7 @@ void CGameState::initTowns()
 			int sel = -1;
 
 			for(ui32 ps=0;ps<vti->possibleSpells.size();ps++)
-				total += vti->possibleSpells[ps].toSpell()->getProbability(vti->subID);
+				total += vti->possibleSpells[ps].toSpell()->getProbability(vti->getFaction());
 
 			if (total == 0) // remaining spells have 0 probability
 				break;
@@ -1846,7 +1847,7 @@ void CGameState::initTowns()
 			auto r = getRandomGenerator().nextInt(total - 1);
 			for(ui32 ps=0; ps<vti->possibleSpells.size();ps++)
 			{
-				r -= vti->possibleSpells[ps].toSpell()->getProbability(vti->subID);
+				r -= vti->possibleSpells[ps].toSpell()->getProbability(vti->getFaction());
 				if(r<0)
 				{
 					sel = ps;
@@ -1869,7 +1870,6 @@ void CGameState::initTowns()
 void CGameState::initMapObjects()
 {
 	logGlobal->debug("\tObject initialization");
-	VLC->creh->removeBonusesFromAllCreatures();
 
 //	objCaller->preInit();
 	for(CGObjectInstance *obj : map->objects)
@@ -3133,7 +3133,7 @@ void InfoAboutTown::initFromTown(const CGTownInstance *t, bool detailed)
 	built = t->builded;
 	fortLevel = t->fortLevel();
 	name = t->getNameTranslated();
-	tType = t->town;
+	tType = t->getTown();
 
 	vstd::clear_pointer(details);
 

lib/CHeroHandler.cpp

@@ -289,7 +289,7 @@ CHeroClass * CHeroClassHandler::loadFromJson(const std::string & scope, const Js
 		VLC->modh->identifiers.requestIdentifier(tavern.second.meta, "faction", tavern.first,
 		[=](si32 factionID)
 		{
-			heroClass->selectionProbability[factionID] = value;
+			heroClass->selectionProbability[FactionID(factionID)] = value;
 		});
 	}
 
@@ -361,11 +361,11 @@ void CHeroClassHandler::afterLoadFinalization()
 		{
 			if (!faction->town)
 				continue;
-			if (heroClass->selectionProbability.count(faction->getIndex()))
+			if (heroClass->selectionProbability.count(faction->getId()))
 				continue;
 
 			auto chance = static_cast<float>(heroClass->defaultTavernChance * faction->town->defaultTavernChance);
-			heroClass->selectionProbability[faction->getIndex()] = static_cast<int>(sqrt(chance) + 0.5); //FIXME: replace with std::round once MVS supports it
+			heroClass->selectionProbability[faction->getId()] = static_cast<int>(sqrt(chance) + 0.5); //FIXME: replace with std::round once MVS supports it
 		}
 		// set default probabilities for gaining secondary skills where not loaded previously
 		heroClass->secSkillProbability.resize(VLC->skillh->size(), -1);

lib/CHeroHandler.h

@@ -131,7 +131,7 @@ public:
 	};
 
 	//double aggression; // not used in vcmi.
-	TFaction faction;
+	FactionID faction;
 	ui8 affinity; // affinity, using EClassAffinity enum
 
 	// default chance for hero of specific class to appear in tavern, if field "tavern" was not set
@@ -146,7 +146,7 @@ public:
 
 	std::vector<int> secSkillProbability; //probabilities of gaining secondary skills (out of 112), in id order
 
-	std::map<TFaction, int> selectionProbability; //probability of selection in towns
+	std::map<FactionID, int> selectionProbability; //probability of selection in towns
 
 	std::string imageBattleMale;
 	std::string imageBattleFemale;

lib/CModHandler.cpp

@@ -1170,10 +1170,6 @@ void CModHandler::load()
 			allMods[modName].validation = CModInfo::FAILED;
 
 	logMod->info("\tLoading mod data: %d ms", timer.getDiff());
-
-	VLC->creh->loadCrExpBon();
-	VLC->creh->buildBonusTreeForTiers(); //do that after all new creatures are loaded
-
 	identifiers.finalize();
 	logMod->info("\tResolving identifiers: %d ms", timer.getDiff());
 

lib/CStack.cpp

@@ -78,7 +78,7 @@ void CStack::localInit(BattleInfo * battleInfo)
 		attachTo(*army);
 		attachTo(const_cast<CCreature&>(*type));
 	}
-	nativeTerrain = type->getNativeTerrain(); //save nativeTerrain in the variable on the battle start to avoid dead lock
+	nativeTerrain = getNativeTerrain(); //save nativeTerrain in the variable on the battle start to avoid dead lock
 	CUnitState::localInit(this); //it causes execution of the CStack::isOnNativeTerrain where nativeTerrain will be considered
 	position = initialPosition;
 }
@@ -338,6 +338,11 @@ int32_t CStack::unitBaseAmount() const
 	return baseAmount;
 }
 
+const IBonusBearer* CStack::getBonusBearer() const
+{
+	return this;
+}
+
 bool CStack::unitHasAmmoCart(const battle::Unit * unit) const
 {
 	for(const CStack * st : battle->stacks)

lib/CStack.h

@@ -84,6 +84,8 @@ public:
 
 	void spendMana(ServerCallback * server, const int spellCost) const override;
 
+	const IBonusBearer* getBonusBearer() const override;
+	
 	PlayerColor getOwner() const override
 	{
 		return this->owner;

lib/CTownHandler.cpp

@@ -157,6 +157,11 @@ FactionID CFaction::getId() const
 	return FactionID(index);
 }
 
+FactionID CFaction::getFaction() const
+{
+	return FactionID(index);
+}
+
 bool CFaction::hasTown() const
 {
 	return town != nullptr;
@@ -562,13 +567,7 @@ void CTownHandler::loadSpecialBuildingBonuses(const JsonNode & source, BonusList
 		if(bonus == nullptr)
 			continue;
 
-		if(bonus->limiter != nullptr)
-		{
-			auto * limPtr = dynamic_cast<CreatureFactionLimiter *>(bonus->limiter.get());
-
-			if(limPtr != nullptr && limPtr->faction == static_cast<TFaction>(-1))
-			limPtr->faction = building->town->faction->getIndex();
-		}
+		bonus->sid = Bonus::getSid32(building->town->faction->getIndex(), building->bid);
 		//JsonUtils::parseBuildingBonus produces UNKNOWN type propagator instead of empty.
 		if(bonus->propagator != nullptr
 			&& bonus->propagator->getPropagatorType() == CBonusSystemNode::ENodeTypes::UNKNOWN)
@@ -947,7 +946,7 @@ void CTownHandler::loadTown(CTown * town, const JsonNode & source)
 
 		VLC->modh->identifiers.requestIdentifier(node.second.meta, "heroClass",node.first, [=](si32 classID)
 		{
-			VLC->heroh->classes[HeroClassID(classID)]->selectionProbability[town->faction->getIndex()] = chance;
+			VLC->heroh->classes[HeroClassID(classID)]->selectionProbability[town->faction->getId()] = chance;
 		});
 	}
 
@@ -957,7 +956,7 @@ void CTownHandler::loadTown(CTown * town, const JsonNode & source)
 
 		VLC->modh->identifiers.requestIdentifier(node.second.meta, "spell", node.first, [=](si32 spellID)
 		{
-			VLC->spellh->objects.at(spellID)->probabilities[town->faction->getIndex()] = chance;
+			VLC->spellh->objects.at(spellID)->probabilities[town->faction->getId()] = chance;
 		});
 	}
 
@@ -1003,7 +1002,7 @@ CFaction * CTownHandler::loadFromJson(const std::string & scope, const JsonNode
 
 	auto * faction = new CFaction();
 
-	faction->index = static_cast<TFaction>(index);
+	faction->index = static_cast<FactionID>(index);
 	faction->modScope = scope;
 	faction->identifier = identifier;
 
@@ -1206,9 +1205,9 @@ std::vector<bool> CTownHandler::getDefaultAllowed() const
 	return allowedFactions;
 }
 
-std::set<TFaction> CTownHandler::getAllowedFactions(bool withTown) const
+std::set<FactionID> CTownHandler::getAllowedFactions(bool withTown) const
 {
-	std::set<TFaction> allowedFactions;
+	std::set<FactionID> allowedFactions;
 	std::vector<bool> allowed;
 	if (withTown)
 		allowed = getDefaultAllowed();
@@ -1217,7 +1216,7 @@ std::set<TFaction> CTownHandler::getAllowedFactions(bool withTown) const
 
 	for (size_t i=0; i<allowed.size(); i++)
 		if (allowed[i])
-			allowedFactions.insert(static_cast<TFaction>(i));
+			allowedFactions.insert(static_cast<FactionID>(i));
 
 	return allowedFactions;
 }

lib/CTownHandler.h

@@ -190,7 +190,9 @@ class DLL_LINKAGE CFaction : public Faction
 	std::string modScope;
 	std::string identifier;
 
-	TFaction index = 0;
+	FactionID index = FactionID::NEUTRAL;
+
+	FactionID getFaction() const override; //This function should not be used
 
 public:
 	TerrainId nativeTerrain;
@@ -421,7 +423,7 @@ public:
 	void afterLoadFinalization() override;
 
 	std::vector<bool> getDefaultAllowed() const override;
-	std::set<TFaction> getAllowedFactions(bool withTown = true) const;
+	std::set<FactionID> getAllowedFactions(bool withTown = true) const;
 
 	static void loadSpecialBuildingBonuses(const JsonNode & source, BonusList & bonusList, CBuilding * building);
 

lib/GameConstants.cpp

@@ -187,7 +187,8 @@ std::string PlayerColor::getStrCap(bool L10n) const
 	return ret;
 }
 
-const FactionID FactionID::ANY = FactionID(-1);
+const FactionID FactionID::NONE = FactionID(-2);
+const FactionID FactionID::DEFAULT = FactionID(-1);
 const FactionID FactionID::CASTLE = FactionID(0);
 const FactionID FactionID::RAMPART = FactionID(1);
 const FactionID FactionID::TOWER = FactionID(2);
@@ -205,7 +206,7 @@ si32 FactionID::decode(const std::string & identifier)
 	if(rawId)
 		return rawId.get();
 	else
-		return -1;
+		return FactionID::DEFAULT;
 }
 
 std::string FactionID::encode(const si32 index)

lib/GameConstants.h

@@ -442,7 +442,8 @@ class FactionID : public BaseForID<FactionID, int32_t>
 {
 	INSTID_LIKE_CLASS_COMMON(FactionID, si32)
 
-	DLL_LINKAGE static const FactionID ANY;
+	DLL_LINKAGE static const FactionID NONE;
+	DLL_LINKAGE static const FactionID DEFAULT;
 	DLL_LINKAGE static const FactionID CASTLE;
 	DLL_LINKAGE static const FactionID RAMPART;
 	DLL_LINKAGE static const FactionID TOWER;
@@ -1313,7 +1314,6 @@ enum class EHealPower : ui8
 };
 
 // Typedef declarations
-typedef ui8 TFaction;
 typedef si64 TExpType;
 typedef si32 TBonusSubtype;
 typedef si32 TQuantity;

lib/HeroBonus.cpp

@@ -75,9 +75,10 @@ const std::map<std::string, TLimiterPtr> bonusLimiterMap =
 	{"DRAGON_NATURE", std::make_shared<HasAnotherBonusLimiter>(Bonus::DRAGON_NATURE)},
 	{"IS_UNDEAD", std::make_shared<HasAnotherBonusLimiter>(Bonus::UNDEAD)},
 	{"CREATURE_NATIVE_TERRAIN", std::make_shared<CreatureTerrainLimiter>()},
-	{"CREATURE_FACTION", std::make_shared<CreatureFactionLimiter>()},
+	{"CREATURE_FACTION", std::make_shared<AllOfLimiter>(std::initializer_list<TLimiterPtr>{std::make_shared<CreatureLevelLimiter>(), std::make_shared<FactionLimiter>()})},
+	{"SAME_FACTION", std::make_shared<FactionLimiter>()},
+	{"CREATURES_ONLY", std::make_shared<CreatureLevelLimiter>()},
 	{"OPPOSITE_SIDE", std::make_shared<OppositeSideLimiter>()},
-	{"UNIT_ON_HEXES", std::make_shared<UnitOnHexLimiter>()}
 };
 
 const std::map<std::string, TPropagatorPtr> bonusPropagatorMap =
@@ -87,8 +88,7 @@ const std::map<std::string, TPropagatorPtr> bonusPropagatorMap =
 	{"PLAYER_PROPAGATOR", std::make_shared<CPropagatorNodeType>(CBonusSystemNode::PLAYER)},
 	{"HERO", std::make_shared<CPropagatorNodeType>(CBonusSystemNode::HERO)},
 	{"TEAM_PROPAGATOR", std::make_shared<CPropagatorNodeType>(CBonusSystemNode::TEAM)}, //untested
-	{"GLOBAL_EFFECT", std::make_shared<CPropagatorNodeType>(CBonusSystemNode::GLOBAL_EFFECTS)},
-	{"ALL_CREATURES", std::make_shared<CPropagatorNodeType>(CBonusSystemNode::ALL_CREATURES)}
+	{"GLOBAL_EFFECT", std::make_shared<CPropagatorNodeType>(CBonusSystemNode::GLOBAL_EFFECTS)}
 }; //untested
 
 const std::map<std::string, TUpdaterPtr> bonusUpdaterMap =
@@ -2301,12 +2301,6 @@ CBonusSystemNode::ENodeTypes IPropagator::getPropagatorType() const
 	return CBonusSystemNode::ENodeTypes::NONE;
 }
 
-CPropagatorNodeType::CPropagatorNodeType()
-	:nodeType(CBonusSystemNode::ENodeTypes::UNKNOWN)
-{
-
-}
-
 CPropagatorNodeType::CPropagatorNodeType(CBonusSystemNode::ENodeTypes NodeType)
 	: nodeType(NodeType)
 {
@@ -2367,36 +2361,78 @@ JsonNode CreatureTerrainLimiter::toJsonNode() const
 	return root;
 }
 
-CreatureFactionLimiter::CreatureFactionLimiter(TFaction creatureFaction)
+FactionLimiter::FactionLimiter(FactionID creatureFaction)
 	: faction(creatureFaction)
 {
 }
 
-CreatureFactionLimiter::CreatureFactionLimiter():
-	faction(static_cast<TFaction>(-1))
+ILimiter::EDecision FactionLimiter::limit(const BonusLimitationContext &context) const
 {
+	const auto * bearer = dynamic_cast<const INativeTerrainProvider*>(&context.node);
+
+	if(bearer)
+	{
+		if(faction != FactionID::DEFAULT)
+			return bearer->getFaction() == faction ? ILimiter::EDecision::ACCEPT : ILimiter::EDecision::DISCARD;
+
+		switch(context.b->source)
+		{
+			case Bonus::CREATURE_ABILITY:
+				return faction == CreatureID(context.b->sid).toCreature()->getFaction() ? ILimiter::EDecision::ACCEPT : ILimiter::EDecision::DISCARD;
+			
+			case Bonus::TOWN_STRUCTURE:
+				return faction == FactionID(Bonus::getHighFromSid32(context.b->sid)) ? ILimiter::EDecision::ACCEPT : ILimiter::EDecision::DISCARD;
+
+			//TODO: other sources of bonuses
+		}
+	}
+	return ILimiter::EDecision::DISCARD; //Discard by default
 }
 
-ILimiter::EDecision CreatureFactionLimiter::limit(const BonusLimitationContext &context) const
+std::string FactionLimiter::toString() const
 {
-	const CCreature *c = retrieveCreature(&context.node);
-	auto accept = c && c->getFactionIndex() == faction;
+	boost::format fmt("FactionLimiter(faction=%s)");
+	fmt % VLC->factions()->getByIndex(faction)->getJsonKey();
+	return fmt.str();
+}
+
+JsonNode FactionLimiter::toJsonNode() const
+{
+	JsonNode root(JsonNode::JsonType::DATA_STRUCT);
+
+	root["type"].String() = "FACTION_LIMITER";
+	root["parameters"].Vector().push_back(JsonUtils::stringNode(VLC->factions()->getByIndex(faction)->getJsonKey()));
+
+	return root;
+}
+
+CreatureLevelLimiter::CreatureLevelLimiter(uint32_t minLevel, uint32_t maxLevel) :
+	minLevel(minLevel),
+	maxLevel(maxLevel)
+{
+}
+
+ILimiter::EDecision CreatureLevelLimiter::limit(const BonusLimitationContext &context) const
+{
+	const auto *c = retrieveCreature(&context.node);
+	auto accept = c && (c->getLevel() < maxLevel && c->getLevel() >= minLevel);
 	return accept ? ILimiter::EDecision::ACCEPT : ILimiter::EDecision::DISCARD; //drop bonus for non-creatures or non-native residents
 }
 
-std::string CreatureFactionLimiter::toString() const
+std::string CreatureLevelLimiter::toString() const
 {
-	boost::format fmt("CreatureFactionLimiter(faction=%s)");
-	fmt % VLC->factions()->getByIndex(faction)->getJsonKey();
+	boost::format fmt("CreatureLevelLimiter(minLevel=%d,maxLevel=%d)");
+	fmt % minLevel % maxLevel;
 	return fmt.str();
 }
 
-JsonNode CreatureFactionLimiter::toJsonNode() const
+JsonNode CreatureLevelLimiter::toJsonNode() const
 {
 	JsonNode root(JsonNode::JsonType::DATA_STRUCT);
 
-	root["type"].String() = "CREATURE_FACTION_LIMITER";
-	root["parameters"].Vector().push_back(JsonUtils::stringNode(VLC->factions()->getByIndex(faction)->getJsonKey()));
+	root["type"].String() = "CREATURE_LEVEL_LIMITER";
+	root["parameters"].Vector().push_back(JsonUtils::intNode(minLevel));
+	root["parameters"].Vector().push_back(JsonUtils::intNode(maxLevel));
 
 	return root;
 }
@@ -2461,35 +2497,8 @@ ILimiter::EDecision RankRangeLimiter::limit(const BonusLimitationContext &contex
 	return ILimiter::EDecision::DISCARD;
 }
 
-ILimiter::EDecision StackOwnerLimiter::limit(const BonusLimitationContext &context) const
-{
-	const CStack * s = retrieveStackBattle(&context.node);
-	if(s && s->owner == owner)
-		return ILimiter::EDecision::ACCEPT;
-
-	const CStackInstance * csi = retrieveStackInstance(&context.node);
-	if(csi && csi->armyObj && csi->armyObj->tempOwner == owner)
-		return ILimiter::EDecision::ACCEPT;
-	return ILimiter::EDecision::DISCARD;
-}
-
-StackOwnerLimiter::StackOwnerLimiter()
-	: owner(-1)
-{
-}
-
-StackOwnerLimiter::StackOwnerLimiter(const PlayerColor & Owner):
-	owner(Owner)
-{
-}
-
-OppositeSideLimiter::OppositeSideLimiter():
-	owner(PlayerColor::CANNOT_DETERMINE)
-{
-}
-
-OppositeSideLimiter::OppositeSideLimiter(const PlayerColor & Owner):
-	owner(Owner)
+OppositeSideLimiter::OppositeSideLimiter(PlayerColor Owner):
+	owner(std::move(Owner))
 {
 }
 
@@ -2502,6 +2511,11 @@ ILimiter::EDecision OppositeSideLimiter::limit(const BonusLimitationContext & co
 
 // Aggregate/Boolean Limiters
 
+AggregateLimiter::AggregateLimiter(std::vector<TLimiterPtr> limiters):
+	limiters(std::move(limiters))
+{
+}
+
 void AggregateLimiter::add(const TLimiterPtr & limiter)
 {
 	if(limiter)
@@ -2523,6 +2537,11 @@ const std::string & AllOfLimiter::getAggregator() const
 	return aggregator;
 }
 
+AllOfLimiter::AllOfLimiter(std::vector<TLimiterPtr> limiters):
+	AggregateLimiter(limiters)
+{
+}
+
 ILimiter::EDecision AllOfLimiter::limit(const BonusLimitationContext & context) const
 {
 	bool wasntSure = false;
@@ -2545,6 +2564,11 @@ const std::string & AnyOfLimiter::getAggregator() const
 	return aggregator;
 }
 
+AnyOfLimiter::AnyOfLimiter(std::vector<TLimiterPtr> limiters):
+	AggregateLimiter(limiters)
+{
+}
+
 ILimiter::EDecision AnyOfLimiter::limit(const BonusLimitationContext & context) const
 {
 	bool wasntSure = false;
@@ -2567,6 +2591,11 @@ const std::string & NoneOfLimiter::getAggregator() const
 	return aggregator;
 }
 
+NoneOfLimiter::NoneOfLimiter(std::vector<TLimiterPtr> limiters):
+	AggregateLimiter(limiters)
+{
+}
+
 ILimiter::EDecision NoneOfLimiter::limit(const BonusLimitationContext & context) const
 {
 	bool wasntSure = false;

lib/HeroBonus.h

@@ -12,6 +12,7 @@
 #include "GameConstants.h"
 #include "JsonNode.h"
 #include "battle/BattleHex.h"
+#include <limits>
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -522,6 +523,16 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 		return (high << 16) + low;
 	}
 
+	STRONG_INLINE static ui32 getHighFromSid32(ui32 sid)
+	{
+		return sid >> 16;
+	}
+
+	STRONG_INLINE static ui32 getLowFromSid32(ui32 sid)
+	{
+		return sid & 0x0000FFFF;
+	}
+
 	std::string Description(boost::optional<si32> customValue = {}) const;
 	JsonNode toJsonNode() const;
 	std::string nameForBonus() const; // generate suitable name for bonus - e.g. for storing in json struct
@@ -894,8 +905,7 @@ class DLL_LINKAGE CPropagatorNodeType : public IPropagator
 	CBonusSystemNode::ENodeTypes nodeType;
 
 public:
-	CPropagatorNodeType();
-	CPropagatorNodeType(CBonusSystemNode::ENodeTypes NodeType);
+	CPropagatorNodeType(CBonusSystemNode::ENodeTypes NodeType = CBonusSystemNode::ENodeTypes::UNKNOWN);
 	bool shouldBeAttached(CBonusSystemNode *dest) override;
 	CBonusSystemNode::ENodeTypes getPropagatorType() const override;
 
@@ -995,6 +1005,7 @@ class DLL_LINKAGE AggregateLimiter : public ILimiter
 protected:
 	std::vector<TLimiterPtr> limiters;
 	virtual const std::string & getAggregator() const = 0;
+	AggregateLimiter(std::vector<TLimiterPtr> limiters = {});
 public:
 	void add(const TLimiterPtr & limiter);
 	JsonNode toJsonNode() const override;
@@ -1011,6 +1022,7 @@ class DLL_LINKAGE AllOfLimiter : public AggregateLimiter
 protected:
 	const std::string & getAggregator() const override;
 public:
+	AllOfLimiter(std::vector<TLimiterPtr> limiters = {});
 	static const std::string aggregator;
 	EDecision limit(const BonusLimitationContext & context) const override;
 };
@@ -1020,6 +1032,7 @@ class DLL_LINKAGE AnyOfLimiter : public AggregateLimiter
 protected:
 	const std::string & getAggregator() const override;
 public:
+	AnyOfLimiter(std::vector<TLimiterPtr> limiters = {});
 	static const std::string aggregator;
 	EDecision limit(const BonusLimitationContext & context) const override;
 };
@@ -1029,6 +1042,7 @@ class DLL_LINKAGE NoneOfLimiter : public AggregateLimiter
 protected:
 	const std::string & getAggregator() const override;
 public:
+	NoneOfLimiter(std::vector<TLimiterPtr> limiters = {});
 	static const std::string aggregator;
 	EDecision limit(const BonusLimitationContext & context) const override;
 };
@@ -1106,12 +1120,13 @@ public:
 	}
 };
 
-class DLL_LINKAGE CreatureFactionLimiter : public ILimiter //applies only to creatures of given faction
+class DLL_LINKAGE CreatureLevelLimiter : public ILimiter //applies only to creatures of given faction
 {
 public:
-	TFaction faction;
-	CreatureFactionLimiter();
-	CreatureFactionLimiter(TFaction faction);
+	uint32_t minLevel;
+	uint32_t maxLevel;
+	//accept all levels by default, accept creatures of minLevel <= creature->getLevel() < maxLevel
+	CreatureLevelLimiter(uint32_t minLevel = std::numeric_limits<uint32_t>::min(), uint32_t maxLevel = std::numeric_limits<uint32_t>::max());
 
 	EDecision limit(const BonusLimitationContext &context) const override;
 	std::string toString() const override;
@@ -1120,15 +1135,16 @@ public:
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & static_cast<ILimiter&>(*this);
-		h & faction;
+		h & minLevel;
+		h & maxLevel;
 	}
 };
 
-class DLL_LINKAGE CreatureAlignmentLimiter : public ILimiter //applies only to creatures of given alignment
+class DLL_LINKAGE FactionLimiter : public ILimiter //applies only to creatures of given faction
 {
 public:
-	EAlignment alignment;
-	CreatureAlignmentLimiter(EAlignment Alignment = EAlignment::NEUTRAL);
+	FactionID faction;
+	FactionLimiter(FactionID faction = FactionID::DEFAULT);
 
 	EDecision limit(const BonusLimitationContext &context) const override;
 	std::string toString() const override;
@@ -1137,23 +1153,24 @@ public:
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & static_cast<ILimiter&>(*this);
-		h & alignment;
+		h & faction;
 	}
 };
 
-class DLL_LINKAGE StackOwnerLimiter : public ILimiter //applies only to creatures of given alignment
+class DLL_LINKAGE CreatureAlignmentLimiter : public ILimiter //applies only to creatures of given alignment
 {
 public:
-	PlayerColor owner;
-	StackOwnerLimiter();
-	StackOwnerLimiter(const PlayerColor & Owner);
+	EAlignment alignment;
+	CreatureAlignmentLimiter(EAlignment Alignment = EAlignment::NEUTRAL);
 
 	EDecision limit(const BonusLimitationContext &context) const override;
+	std::string toString() const override;
+	JsonNode toJsonNode() const override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & static_cast<ILimiter&>(*this);
-		h & owner;
+		h & alignment;
 	}
 };
 
@@ -1161,8 +1178,7 @@ class DLL_LINKAGE OppositeSideLimiter : public ILimiter //applies only to creatu
 {
 public:
 	PlayerColor owner;
-	OppositeSideLimiter();
-	OppositeSideLimiter(const PlayerColor & Owner);
+	OppositeSideLimiter(PlayerColor Owner = PlayerColor::CANNOT_DETERMINE);
 
 	EDecision limit(const BonusLimitationContext &context) const override;
 

lib/JsonNode.cpp

@@ -749,15 +749,26 @@ std::shared_ptr<ILimiter> JsonUtils::parseLimiter(const JsonNode & limiter)
 				else
 					return std::make_shared<CreatureAlignmentLimiter>(static_cast<EAlignment>(alignment));
 			}
-			else if(limiterType == "CREATURE_FACTION_LIMITER")
+			else if(limiterType == "FACTION_LIMITER")
 			{
-				std::shared_ptr<CreatureFactionLimiter> factionLimiter = std::make_shared<CreatureFactionLimiter>();
+				std::shared_ptr<FactionLimiter> factionLimiter = std::make_shared<FactionLimiter>();
 				VLC->modh->identifiers.requestIdentifier("faction", parameters[0], [=](si32 faction)
 				{
-					factionLimiter->faction = faction;
+					factionLimiter->faction = FactionID(faction);
 				});
 				return factionLimiter;
 			}
+			else if(limiterType == "CREATURE_LEVEL_LIMITER")
+			{
+				auto levelLimiter = std::make_shared<CreatureLevelLimiter>();
+				if(!parameters.empty()) //If parameters is empty, level limiter works as CREATURES_ONLY limiter
+				{
+					levelLimiter->minLevel = parameters[0].Integer();
+					if(parameters[1].isNumber())
+						levelLimiter->maxLevel = parameters[1].Integer();
+				}
+				return levelLimiter;
+			}
 			else if(limiterType == "CREATURE_TERRAIN_LIMITER")
 			{
 				std::shared_ptr<CreatureTerrainLimiter> terrainLimiter = std::make_shared<CreatureTerrainLimiter>();
@@ -991,7 +1002,17 @@ bool JsonUtils::parseBonus(const JsonNode &ability, Bonus *b)
 
 	value = &ability["propagator"];
 	if (!value->isNull())
-		b->propagator = parseByMap(bonusPropagatorMap, value, "propagator type ");
+	{
+		//ALL_CREATURES old propagator compatibility
+		if(value->String() == "ALL_CREATURES") 
+		{
+			logMod->warn("ALL_CREATURES propagator is deprecated. Use GLOBAL_EFFECT propagator with CREATURES_ONLY limiter");
+			b->addLimiter(std::make_shared<CreatureLevelLimiter>());
+			b->propagator = bonusPropagatorMap.at("GLOBAL_EFFECT");
+		}
+		else
+			b->propagator = parseByMap(bonusPropagatorMap, value, "propagator type ");
+	}
 
 	value = &ability["updater"];
 	if(!value->isNull())

lib/battle/CBattleInfoEssentials.cpp

@@ -85,9 +85,9 @@ bool CBattleInfoEssentials::battleHasNativeStack(ui8 side) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 
-	for(const CStack * s : battleGetAllStacks())
+	for(const auto * s : battleGetAllStacks())
 	{
-		if(s->side == side && s->getCreature()->isItNativeTerrain(getBattle()->getTerrainType()))
+		if(s->side == side && s->isNativeTerrain(getBattle()->getTerrainType()))
 			return true;
 	}
 

lib/battle/CUnitState.cpp

@@ -413,6 +413,11 @@ int32_t CUnitState::creatureIconIndex() const
 	return unitType()->getIconIndex();
 }
 
+FactionID CUnitState::getFaction() const
+{
+	return unitType()->getFaction();
+}
+
 int32_t CUnitState::getCasterUnitId() const
 {
 	return static_cast<int32_t>(unitId());

lib/battle/CUnitState.h

@@ -248,6 +248,8 @@ public:
 	void localInit(const IUnitEnvironment * env_);
 	void serializeJson(JsonSerializeFormat & handler);
 
+	FactionID getFaction() const override;
+
 	void afterAttack(bool ranged, bool counter);
 
 	void afterNewRound();

lib/battle/IBattleInfoCallback.h

@@ -47,7 +47,7 @@ namespace scripting
 }
 #endif
 
-class DLL_LINKAGE IBattleInfoCallback : public WithBonuses
+class DLL_LINKAGE IBattleInfoCallback : public IConstBonusProvider
 {
 public:
 #if SCRIPTING_ENABLED

lib/battle/IBattleState.h

@@ -30,7 +30,7 @@ namespace battle
 	class UnitInfo;
 }
 
-class DLL_LINKAGE IBattleInfo : public WithBonuses
+class DLL_LINKAGE IBattleInfo : public IConstBonusProvider
 {
 public:
 	using ObstacleCList = std::vector<std::shared_ptr<const CObstacleInstance>>;

lib/battle/Unit.cpp

@@ -18,6 +18,9 @@
 #include "../serializer/JsonDeserializer.h"
 #include "../serializer/JsonSerializer.h"
 
+#include <vcmi/Faction.h>
+#include <vcmi/FactionService.h>
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 namespace battle
@@ -43,6 +46,12 @@ std::string Unit::getDescription() const
 	return fmt.str();
 }
 
+//TODO: deduplicate these functions
+const IBonusBearer* Unit::getBonusBearer() const
+{
+	return this;
+}
+
 std::vector<BattleHex> Unit::getSurroundingHexes(BattleHex assumedPosition) const
 {
 	BattleHex hex = (assumedPosition != BattleHex::INVALID) ? assumedPosition : getPosition(); //use hypothetical position

lib/battle/Unit.h

@@ -10,6 +10,7 @@
 
 #pragma once
 
+#include <vcmi/Entity.h>
 #include <vcmi/spells/Caster.h>
 
 #include "../HeroBonus.h"
@@ -40,7 +41,7 @@ namespace BattlePhases
 
 class CUnitState;
 
-class DLL_LINKAGE Unit : public IUnitInfo, public spells::Caster, public virtual IBonusBearer
+class DLL_LINKAGE Unit : public IUnitInfo, public spells::Caster, public virtual IBonusBearer, public IConstBonusNativeTerrainProvider
 {
 public:
 	virtual ~Unit();
@@ -126,6 +127,9 @@ public:
 
 	int getRawSurrenderCost() const;
 
+	//IConstBonusProvider
+	const IBonusBearer* getBonusBearer() const override;
+
 	//NOTE: save could possibly be const, but this requires heavy changes to Json serialization,
 	//also this method should be called only after modifying object
 	virtual void save(JsonNode & data) = 0;

lib/mapObjects/CArmedInstance.cpp

@@ -65,7 +65,7 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 	}
 
 	//number of alignments and presence of undead
-	std::set<TFaction> factions;
+	std::set<FactionID> factions;
 	bool hasUndead = false;
 
 	const std::string undeadCacheKey = "type_UNDEAD";
@@ -76,7 +76,7 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 		const CStackInstance * inst = slot.second;
 		const CCreature * creature  = VLC->creh->objects[inst->getCreatureID()];
 
-		factions.insert(creature->getFactionIndex());
+		factions.insert(creature->getFaction());
 		// Check for undead flag instead of faction (undead mummies are neutral)
 		if (!hasUndead)
 		{
@@ -91,7 +91,7 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 	{
 		size_t mixableFactions = 0;
 
-		for(TFaction f : factions)
+		for(auto f : factions)
 		{
 			if (VLC->factions()->getByIndex(f)->getAlignment() != EAlignment::EVIL)
 				mixableFactions++;
@@ -156,4 +156,9 @@ CBonusSystemNode & CArmedInstance::whatShouldBeAttached()
 	return *this;
 }
 
+const IBonusBearer* CArmedInstance::getBonusBearer() const
+{
+	return this;
+}
+
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/CArmedInstance.h

@@ -17,7 +17,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 class BattleInfo;
 class CGameState;
 
-class DLL_LINKAGE CArmedInstance: public CGObjectInstance, public CBonusSystemNode, public CCreatureSet
+class DLL_LINKAGE CArmedInstance: public CGObjectInstance, public CBonusSystemNode, public CCreatureSet, public IConstBonusProvider
 {
 private:
 	CCheckProxy nonEvilAlignmentMix;
@@ -32,6 +32,8 @@ public:
 	void armyChanged() override;
 
 	//////////////////////////////////////////////////////////////////////////
+	//IConstBonusProvider
+	const IBonusBearer* getBonusBearer() const override;
 //	int valOfGlobalBonuses(CSelector selector) const; //used only for castle interface								???
 	virtual CBonusSystemNode & whereShouldBeAttached(CGameState * gs);
 	virtual CBonusSystemNode & whatShouldBeAttached();

lib/mapObjects/CGHeroInstance.cpp

@@ -84,6 +84,16 @@ ui32 CGHeroInstance::getTileCost(const TerrainTile & dest, const TerrainTile & f
 	return static_cast<ui32>(ret);
 }
 
+FactionID CGHeroInstance::getFaction() const
+{
+	return FactionID(type->heroClass->faction);
+}
+
+const IBonusBearer* CGHeroInstance::getBonusBearer() const
+{
+	return this;
+}
+
 TerrainId CGHeroInstance::getNativeTerrain() const
 {
 	// NOTE: in H3 neutral stacks will ignore terrain penalty only if placed as topmost stack(s) in hero army.
@@ -96,7 +106,7 @@ TerrainId CGHeroInstance::getNativeTerrain() const
 
 	for(const auto & stack : stacks)
 	{
-		TerrainId stackNativeTerrain = stack.second->type->getNativeTerrain(); //consider terrain bonuses e.g. Lodestar.
+		TerrainId stackNativeTerrain = stack.second->getNativeTerrain(); //consider terrain bonuses e.g. Lodestar.
 
 		if(stackNativeTerrain == ETerrainId::NONE)
 			continue;

lib/mapObjects/CGHeroInstance.h

@@ -40,7 +40,7 @@ public:
 };
 
 
-class DLL_LINKAGE CGHeroInstance : public CArmedInstance, public IBoatGenerator, public CArtifactSet, public spells::Caster
+class DLL_LINKAGE CGHeroInstance : public CArmedInstance, public IBoatGenerator, public CArtifactSet, public spells::Caster, public IConstBonusNativeTerrainProvider
 {
 	// We serialize heroes into JSON for crossover
 	friend class CCampaignState;
@@ -102,20 +102,6 @@ public:
 		}
 	} patrol;
 
-	// deprecated - used only for loading of old saves
-	struct HeroSpecial : CBonusSystemNode
-	{
-		bool growsWithLevel;
-
-		HeroSpecial(){growsWithLevel = false;};
-
-		template <typename Handler> void serialize(Handler &h, const int version)
-		{
-			h & static_cast<CBonusSystemNode&>(*this);
-			h & growsWithLevel;
-		}
-	};
-
 	struct DLL_LINKAGE SecondarySkillsInfo
 	{
 		//skills are determined, initialized at map start
@@ -170,7 +156,9 @@ public:
 	bool needsLastStack()const override;
 
 	ui32 getTileCost(const TerrainTile & dest, const TerrainTile & from, const TurnInfo * ti) const; //move cost - applying pathfinding skill, road and terrain modifiers. NOT includes diagonal move penalty, last move levelling
-	TerrainId getNativeTerrain() const;
+	//INativeTerrainProvider
+	FactionID getFaction() const override;
+	TerrainId getNativeTerrain() const override;
 	int getLowestCreatureSpeed() const;
 	si32 manaRegain() const; //how many points of mana can hero regain "naturally" in one day
 	si32 getManaNewTurn() const; //calculate how much mana this hero is going to have the next day
@@ -260,6 +248,9 @@ public:
 	std::string nodeName() const override;
 	si32 manaLimit() const override;
 
+	///IConstBonusProvider
+	const IBonusBearer* getBonusBearer() const override;
+
 	CBonusSystemNode * whereShouldBeAttachedOnSiege(const bool isBattleOutsideTown) const;
 	CBonusSystemNode * whereShouldBeAttachedOnSiege(CGameState * gs);
 

lib/mapObjects/CGTownInstance.cpp

@@ -938,7 +938,7 @@ void CGTownInstance::newTurn(CRandomGenerator & rand) const
 				std::vector<SlotID> nativeCrits; //slots
 				for(const auto & elem : Slots())
 				{
-					if (elem.second->type->getFactionIndex() == subID) //native
+					if (elem.second->type->getFaction() == subID) //native
 					{
 						nativeCrits.push_back(elem.first); //collect matching slots
 					}
@@ -1232,12 +1232,7 @@ void CGTownInstance::recreateBuildingsBonuses()
 			continue;
 
 		for(auto & bonus : building->buildingBonuses)
-		{
-			if(bonus->propagator != nullptr && bonus->propagator->getPropagatorType() == ALL_CREATURES)
-				VLC->creh->addBonusForAllCreatures(bonus);
-			else
-				addNewBonus(bonus);
-		}
+			addNewBonus(bonus);
 	}
 }
 
@@ -1617,6 +1612,16 @@ void CGTownInstance::serializeJsonOptions(JsonSerializeFormat & handler)
 	}
 }
 
+FactionID CGTownInstance::getFaction() const
+{
+	return town->faction->getId();
+}
+
+TerrainId CGTownInstance::getNativeTerrain() const
+{
+	return town->faction->getNativeTerrain();
+}
+
 PlayerColor CGTownBuilding::getOwner() const
 {
 	return town->getOwner();
@@ -1764,7 +1769,7 @@ void CTownBonus::onHeroVisit (const CGHeroInstance * h) const
 			break;
 
 		case BuildingSubID::CUSTOM_VISITING_BONUS:
-			const auto building = town->town->buildings.at(bID);
+			const auto building = town->getTown()->buildings.at(bID);
 			if(!h->hasBonusFrom(Bonus::TOWN_STRUCTURE, Bonus::getSid32(building->town->faction->getIndex(), building->bid)))
 			{
 				const auto & bonuses = building->onVisitBonuses;
@@ -1848,7 +1853,7 @@ int GrowthInfo::totalGrowth() const
 
 std::string CGTownBuilding::getVisitingBonusGreeting() const
 {
-	auto bonusGreeting = town->town->getGreeting(bType);
+	auto bonusGreeting = town->getTown()->getGreeting(bType);
 
 	if(!bonusGreeting.empty())
 		return bonusGreeting;
@@ -1874,15 +1879,15 @@ std::string CGTownBuilding::getVisitingBonusGreeting() const
 		bonusGreeting = std::string(VLC->generaltexth->translate("vcmi.townHall.greetingDefence"));
 		break;
 	}
-	auto buildingName = town->town->getSpecialBuilding(bType)->getNameTranslated();
+	auto buildingName = town->getTown()->getSpecialBuilding(bType)->getNameTranslated();
 
 	if(bonusGreeting.empty())
 	{
 		bonusGreeting = "Error: Bonus greeting for '%s' is not localized.";
-		logGlobal->error("'%s' building of '%s' faction has not localized bonus greeting.", buildingName, town->town->faction->getNameTranslated());
+		logGlobal->error("'%s' building of '%s' faction has not localized bonus greeting.", buildingName, town->getTown()->faction->getNameTranslated());
 	}
 	boost::algorithm::replace_first(bonusGreeting, "%s", buildingName);
-	town->town->setGreeting(bType, bonusGreeting);
+	town->getTown()->setGreeting(bType, bonusGreeting);
 	return bonusGreeting;
 }
 
@@ -1891,7 +1896,7 @@ std::string CGTownBuilding::getCustomBonusGreeting(const Bonus & bonus) const
 	if(bonus.type == Bonus::TOWN_MAGIC_WELL)
 	{
 		auto bonusGreeting = std::string(VLC->generaltexth->translate("vcmi.townHall.greetingInTownMagicWell"));
-		auto buildingName = town->town->getSpecialBuilding(bType)->getNameTranslated();
+		auto buildingName = town->getTown()->getSpecialBuilding(bType)->getNameTranslated();
 		boost::algorithm::replace_first(bonusGreeting, "%s", buildingName);
 		return bonusGreeting;
 	}

lib/mapObjects/CGTownInstance.h

@@ -202,7 +202,7 @@ struct DLL_LINKAGE GrowthInfo
 	int totalGrowth() const;
 };
 
-class DLL_LINKAGE CGTownInstance : public CGDwelling, public IShipyard, public IMarket
+class DLL_LINKAGE CGTownInstance : public CGDwelling, public IShipyard, public IMarket, public INativeTerrainProvider
 {
 	std::string name; // name of town
 public:
@@ -341,7 +341,11 @@ public:
 	/// Returns damage range for central tower(keep) of this town
 	DamageRange getKeepDamageRange() const;
 
-	const CTown * getTown() const ;
+	const CTown * getTown() const;
+
+	/// INativeTerrainProvider
+	FactionID getFaction() const override;
+	TerrainId getNativeTerrain() const override;
 
 	CGTownInstance();
 	virtual ~CGTownInstance();

lib/mapObjects/JsonRandom.h

@@ -15,7 +15,7 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 class JsonNode;
-typedef std::vector<JsonNode> JsonVector;
+using JsonVector = std::vector<JsonNode>;
 class CRandomGenerator;
 
 struct Bonus;

lib/mapping/CMap.h

@@ -67,7 +67,7 @@ struct DLL_LINKAGE PlayerInfo
 	bool canComputerPlay;
 	EAiTactic::EAiTactic aiTactic; /// The default value is EAiTactic::RANDOM.
 
-	std::set<TFaction> allowedFactions;
+	std::set<FactionID> allowedFactions;
 	bool isFactionRandom;
 
 	///main hero instance (VCMI maps only)

lib/mapping/MapFormatH3M.cpp

@@ -251,10 +251,10 @@ void CMapLoaderH3M::readPlayerInfo()
 		{
 			mapHeader->players[i].allowedFactions.clear();
 
-			for(int fact = 0; fact < totalFactions; ++fact)
+			for(auto fact = 0; fact < totalFactions; ++fact)
 			{
 				if(allowedFactions & (1 << fact))
-					mapHeader->players[i].allowedFactions.insert(fact);
+					mapHeader->players[i].allowedFactions.insert(FactionID(fact));
 			}
 		}
 

lib/mapping/MapFormatJson.cpp

@@ -382,7 +382,7 @@ RoadType * CMapFormatJson::getRoadByCode(const std::string & code)
 	return nullptr;
 }
 
-void CMapFormatJson::serializeAllowedFactions(JsonSerializeFormat & handler, std::set<TFaction> & value) const
+void CMapFormatJson::serializeAllowedFactions(JsonSerializeFormat & handler, std::set<FactionID> & value) const
 {
 	//TODO: unify allowed factions with others - make them std::vector<bool>
 
@@ -404,7 +404,7 @@ void CMapFormatJson::serializeAllowedFactions(JsonSerializeFormat & handler, std
 		value.clear();
 		for (std::size_t i=0; i<temp.size(); i++)
 			if(temp[i])
-				value.insert(static_cast<TFaction>(i));
+				value.insert(static_cast<FactionID>(i));
 	}
 }
 

lib/mapping/MapFormatJson.h

@@ -64,7 +64,7 @@ protected:
 	static RiverType * getRiverByCode(const std::string & code);
 	static RoadType * getRoadByCode(const std::string & code);
 
-	void serializeAllowedFactions(JsonSerializeFormat & handler, std::set<TFaction> & value) const;
+	void serializeAllowedFactions(JsonSerializeFormat & handler, std::set<FactionID> & value) const;
 
 	///common part of header saving/loading
 	void serializeHeader(JsonSerializeFormat & handler);

lib/registerTypes/RegisterTypes.h

@@ -179,10 +179,10 @@ void registerTypesMapObjects2(Serializer &s)
 	s.template registerType<ILimiter, CCreatureTypeLimiter>();
 	s.template registerType<ILimiter, HasAnotherBonusLimiter>();
 	s.template registerType<ILimiter, CreatureTerrainLimiter>();
-	s.template registerType<ILimiter, CreatureFactionLimiter>();
+	s.template registerType<ILimiter, FactionLimiter>();
+	s.template registerType<ILimiter, CreatureLevelLimiter>();
 	s.template registerType<ILimiter, CreatureAlignmentLimiter>();
 	s.template registerType<ILimiter, RankRangeLimiter>();
-	s.template registerType<ILimiter, StackOwnerLimiter>();
 	s.template registerType<ILimiter, UnitOnHexLimiter>();
 
 //	s.template registerType<CBonusSystemNode>();
@@ -194,7 +194,6 @@ void registerTypesMapObjects2(Serializer &s)
 	s.template registerType<CBonusSystemNode, PlayerState>();
 	s.template registerType<CBonusSystemNode, TeamState>();
 	//s.template registerType<CGameState>(); //TODO
-	s.template registerType<CBonusSystemNode, CGHeroInstance::HeroSpecial>();
 	//s.template registerType<CArmedInstance>();
 	s.template registerType<CBonusSystemNode, CStack>();
 	s.template registerType<CBonusSystemNode, BattleInfo>();

lib/rmg/CMapGenOptions.cpp

@@ -135,14 +135,14 @@ void CMapGenOptions::setMonsterStrength(EMonsterStrength::EMonsterStrength value
 void CMapGenOptions::resetPlayersMap()
 {
 
-	std::map<PlayerColor, TFaction> rememberTownTypes;
+	std::map<PlayerColor, FactionID> rememberTownTypes;
 	std::map<PlayerColor, TeamID> rememberTeam;
 
 	for(const auto & p : players)
 	{
 		auto town = p.second.getStartingTown();
 		if (town != RANDOM_SIZE)
-			rememberTownTypes[p.first] = town;
+			rememberTownTypes[p.first] = FactionID(town);
 		rememberTeam[p.first] = p.second.getTeam();
 	}
 

lib/rmg/CRmgTemplate.cpp

@@ -199,33 +199,33 @@ void ZoneOptions::setTerrainTypes(const std::set<TerrainId> & value)
 	terrainTypes = value;
 }
 
-std::set<TFaction> ZoneOptions::getDefaultTownTypes() const
+std::set<FactionID> ZoneOptions::getDefaultTownTypes() const
 {
-	std::set<TFaction> defaultTowns;
+	std::set<FactionID> defaultTowns;
 	auto towns = VLC->townh->getDefaultAllowed();
 	for(int i = 0; i < towns.size(); ++i)
 	{
-		if(towns[i]) defaultTowns.insert(i);
+		if(towns[i]) defaultTowns.insert(FactionID(i));
 	}
 	return defaultTowns;
 }
 
-const std::set<TFaction> & ZoneOptions::getTownTypes() const
+const std::set<FactionID> & ZoneOptions::getTownTypes() const
 {
 	return townTypes;
 }
 
-void ZoneOptions::setTownTypes(const std::set<TFaction> & value)
+void ZoneOptions::setTownTypes(const std::set<FactionID> & value)
 {
 	townTypes = value;
 }
 
-void ZoneOptions::setMonsterTypes(const std::set<TFaction> & value)
+void ZoneOptions::setMonsterTypes(const std::set<FactionID> & value)
 {
 	monsterTypes = value;
 }
 
-const std::set<TFaction> & ZoneOptions::getMonsterTypes() const
+const std::set<FactionID> & ZoneOptions::getMonsterTypes() const
 {
 	return monsterTypes;
 }
@@ -373,8 +373,8 @@ void ZoneOptions::serializeJson(JsonSerializeFormat & handler)
 	}
 
 	handler.serializeBool("townsAreSameType", townsAreSameType, false);
-	handler.serializeIdArray<TFaction, FactionID>("allowedMonsters", monsterTypes, VLC->townh->getAllowedFactions(false));
-	handler.serializeIdArray<TFaction, FactionID>("allowedTowns", townTypes, VLC->townh->getAllowedFactions(true));
+	handler.serializeIdArray<FactionID, FactionID>("allowedMonsters", monsterTypes, VLC->townh->getAllowedFactions(false));
+	handler.serializeIdArray<FactionID, FactionID>("allowedTowns", townTypes, VLC->townh->getAllowedFactions(true));
 
 	{
 		//TODO: add support for std::map to serializeEnum

lib/rmg/CRmgTemplate.h

@@ -131,12 +131,12 @@ public:
 
 	const CTownInfo & getPlayerTowns() const;
 	const CTownInfo & getNeutralTowns() const;
-	std::set<TFaction> getDefaultTownTypes() const;
-	const std::set<TFaction> & getTownTypes() const;
-	const std::set<TFaction> & getMonsterTypes() const;
+	std::set<FactionID> getDefaultTownTypes() const;
+	const std::set<FactionID> & getTownTypes() const;
+	const std::set<FactionID> & getMonsterTypes() const;
 
-	void setTownTypes(const std::set<TFaction> & value);
-	void setMonsterTypes(const std::set<TFaction> & value);
+	void setTownTypes(const std::set<FactionID> & value);
+	void setMonsterTypes(const std::set<FactionID> & value);
 
 	void setMinesInfo(const std::map<TResource, ui16> & value);
 	std::map<TResource, ui16> getMinesInfo() const;
@@ -173,8 +173,8 @@ protected:
 	std::set<TerrainId> terrainTypes;
 	bool townsAreSameType;
 
-	std::set<TFaction> townTypes;
-	std::set<TFaction> monsterTypes;
+	std::set<FactionID> townTypes;
+	std::set<FactionID> monsterTypes;
 
 	std::map<TResource, ui16> mines; //obligatory mines to spawn in this zone
 

lib/rmg/ObjectManager.cpp

@@ -426,7 +426,7 @@ CGCreature * ObjectManager::chooseGuard(si32 strength, bool zoneGuard)
 			continue;
 		if(!cre->getAIValue()) //bug #2681
 			continue;
-		if(!vstd::contains(zone.getMonsterTypes(), cre->getFactionIndex()))
+		if(!vstd::contains(zone.getMonsterTypes(), cre->getFaction()))
 			continue;
 		if((static_cast<si32>(cre->getAIValue() * (cre->ammMin + cre->ammMax) / 2) < strength) && (strength < static_cast<si32>(cre->getAIValue()) * 100)) //at least one full monster. size between average size of given stack and 100
 		{

lib/rmg/RmgMap.cpp

@@ -274,13 +274,13 @@ float RmgMap::getNearestObjectDistance(const int3 &tile) const
 	return tiles[tile.x][tile.y][tile.z].getNearestObjectDistance();
 }
 
-void RmgMap::registerZone(TFaction faction)
+void RmgMap::registerZone(FactionID faction)
 {
 	zonesPerFaction[faction]++;
 	zonesTotal++;
 }
 
-ui32 RmgMap::getZoneCount(TFaction faction)
+ui32 RmgMap::getZoneCount(FactionID faction)
 {
 	return zonesPerFaction[faction];
 }

lib/rmg/RmgMap.h

@@ -60,8 +60,8 @@ public:
 	
 	Zones & getZones();
 	
-	void registerZone(TFaction faction);
-	ui32 getZoneCount(TFaction faction);
+	void registerZone(FactionID faction);
+	ui32 getZoneCount(FactionID faction);
 	ui32 getTotalZoneCount() const;
 	void initTiles(CMapGenerator & generator);
 	void addModificators();
@@ -75,7 +75,7 @@ private:
 	
 private:
 	Zones zones;
-	std::map<TFaction, ui32> zonesPerFaction;
+	std::map<FactionID, ui32> zonesPerFaction;
 	ui32 zonesTotal; //zones that have their main town only
 	const CMapGenOptions& mapGenOptions;
 	boost::multi_array<TileInfo, 3> tiles; //[x][y][z]

lib/rmg/TownPlacer.cpp

@@ -85,7 +85,7 @@ void TownPlacer::placeTowns(ObjectManager & manager)
 		
 		totalTowns++;
 		//register MAIN town of zone only
-		map.registerZone(town->subID);
+		map.registerZone(town->getFaction());
 		
 		if(playerInfo.canAnyonePlay()) //configure info for owning player
 		{
@@ -201,7 +201,7 @@ void TownPlacer::addNewTowns(int count, bool hasFort, const PlayerColor & player
 		{
 			//FIXME: discovered bug with small zones - getPos is close to map boarder and we have outOfMap exception
 			//register MAIN town of zone
-			map.registerZone(town->subID);
+			map.registerZone(town->getFaction());
 			//first town in zone goes in the middle
 			placeMainTown(manager, *town);
 		}
@@ -216,8 +216,8 @@ si32 TownPlacer::getRandomTownType(bool matchUndergroundType)
 	auto townTypesAllowed = (!zone.getTownTypes().empty() ? zone.getTownTypes() : zone.getDefaultTownTypes());
 	if(matchUndergroundType)
 	{
-		std::set<TFaction> townTypesVerify;
-		for(TFaction factionIdx : townTypesAllowed)
+		std::set<FactionID> townTypesVerify;
+		for(auto factionIdx : townTypesAllowed)
 		{
 			bool preferUnderground = (*VLC->townh)[factionIdx]->preferUndergroundPlacement;
 			if(zone.isUnderground() ? preferUnderground : !preferUnderground)

lib/rmg/TreasurePlacer.cpp

@@ -135,7 +135,7 @@ void TreasurePlacer::addAllPossibleObjects()
 	std::vector<CCreature *> creatures; //native creatures for this zone
 	for(auto cre : VLC->creh->objects)
 	{
-		if(!cre->special && cre->getFactionIndex() == zone.getTownType())
+		if(!cre->special && cre->getFaction() == zone.getTownType())
 		{
 			creatures.push_back(cre);
 		}
@@ -164,9 +164,9 @@ void TreasurePlacer::addAllPossibleObjects()
 				continue;
 
 			const auto * cre = creatures.front();
-			if(cre->getFactionIndex() == zone.getTownType())
+			if(cre->getFaction() == zone.getTownType())
 			{
-				auto nativeZonesCount = static_cast<float>(map.getZoneCount(cre->getFactionIndex()));
+				auto nativeZonesCount = static_cast<float>(map.getZoneCount(cre->getFaction()));
 				oi.value = static_cast<ui32>(cre->getAIValue() * cre->getGrowth() * (1 + (nativeZonesCount / map.getTotalZoneCount()) + (nativeZonesCount / 2)));
 				oi.probability = 40;
 
@@ -294,7 +294,7 @@ void TreasurePlacer::addAllPossibleObjects()
 			return obj;
 		};
 		oi.setTemplate(Obj::PANDORAS_BOX, 0, zone.getTerrainType());
-		oi.value = static_cast<ui32>((2 * (creature->getAIValue()) * creaturesAmount * (1 + static_cast<float>(map.getZoneCount(creature->getFactionIndex())) / map.getTotalZoneCount())) / 3);
+		oi.value = static_cast<ui32>((2 * (creature->getAIValue()) * creaturesAmount * (1 + static_cast<float>(map.getZoneCount(creature->getFaction())) / map.getTotalZoneCount())) / 3);
 		oi.probability = 3;
 		addObjectToRandomPool(oi);
 	}
@@ -453,7 +453,7 @@ void TreasurePlacer::addAllPossibleObjects()
 				return obj;
 			};
 			oi.setTemplate(Obj::SEER_HUT, randomAppearance, zone.getTerrainType());
-			oi.value = static_cast<ui32>(((2 * (creature->getAIValue()) * creaturesAmount * (1 + static_cast<float>(map.getZoneCount(creature->getFactionIndex())) / map.getTotalZoneCount())) - 4000) / 3);
+			oi.value = static_cast<ui32>(((2 * (creature->getAIValue()) * creaturesAmount * (1 + static_cast<float>(map.getZoneCount(creature->getFaction())) / map.getTotalZoneCount())) - 4000) / 3);
 			oi.probability = 3;
 			addObjectToRandomPool(oi);
 		}

lib/rmg/Zone.cpp

@@ -123,9 +123,9 @@ rmg::Area & Zone::freePaths()
 	return dAreaFree;
 }
 
-si32 Zone::getTownType() const
+FactionID Zone::getTownType() const
 {
-	return townType;
+	return FactionID(townType);
 }
 
 void Zone::setTownType(si32 town)

lib/rmg/Zone.h

@@ -98,7 +98,7 @@ public:
 	void clearTiles();
 	void fractalize();
 	
-	si32 getTownType() const;
+	FactionID getTownType() const;
 	void setTownType(si32 town);
 	TerrainId getTerrainType() const;
 	void setTerrainType(TerrainId terrain);

lib/serializer/CSerializer.h

@@ -14,8 +14,8 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-const ui32 SERIALIZATION_VERSION = 821;
-const ui32 MINIMAL_SERIALIZATION_VERSION = 821;
+const ui32 SERIALIZATION_VERSION = 822;
+const ui32 MINIMAL_SERIALIZATION_VERSION = 822;
 const std::string SAVEGAME_MAGIC = "VCMISVG";
 
 class CHero;

lib/spells/CSpellHandler.cpp

@@ -340,7 +340,7 @@ int32_t CSpell::getLevelPower(const int32_t skillLevel) const
 	return getLevelInfo(skillLevel).power;
 }
 
-si32 CSpell::getProbability(const TFaction factionId) const
+si32 CSpell::getProbability(const FactionID & factionId) const
 {
 	if(!vstd::contains(probabilities,factionId))
 	{
@@ -722,7 +722,7 @@ CSpell * CSpellHandler::loadFromJson(const std::string & scope, const JsonNode &
 
 		VLC->modh->identifiers.requestIdentifier(node.second.meta, "faction", node.first, [=](si32 factionID)
 		{
-			spell->probabilities[factionID] = chance;
+			spell->probabilities[FactionID(factionID)] = chance;
 		});
 	}
 

lib/spells/CSpellHandler.h

@@ -192,7 +192,7 @@ public:
 
 	si32 power; //spell's power
 
-	std::map<TFaction, si32> probabilities; //% chance to gain for castles
+	std::map<FactionID, si32> probabilities; //% chance to gain for castles
 
 	bool combat; //is this spell combat (true) or adventure (false)
 	bool creatureAbility; //if true, only creatures can use this spell
@@ -223,7 +223,7 @@ public:
 
 	int32_t getCost(const int32_t skillLevel) const override;
 
-	si32 getProbability(const TFaction factionId) const;
+	si32 getProbability(const FactionID & factionId) const;
 
 	int32_t getBasePower() const override;
 	int32_t getLevelPower(const int32_t skillLevel) const override;

lib/spells/effects/Moat.cpp

@@ -87,7 +87,7 @@ void Moat::convertBonus(const Mechanics * m, std::vector<Bonus> & converted) con
 
 		if(m->battle()->battleGetDefendedTown() && m->battle()->battleGetSiegeLevel() >= CGTownInstance::CITADEL)
 		{
-			nb.sid = Bonus::getSid32(m->battle()->battleGetDefendedTown()->town->faction->getIndex(), BuildingID::CITADEL);
+			nb.sid = Bonus::getSid32(m->battle()->battleGetDefendedTown()->getFaction(), BuildingID::CITADEL);
 			nb.source = Bonus::TOWN_STRUCTURE;
 		}
 		else

mapeditor/playerparams.cpp

@@ -29,7 +29,7 @@ PlayerParams::PlayerParams(MapController & ctrl, int playerId, QWidget *parent)
 	{
 		CFaction * faction = VLC->townh->objects.at(idx);
 		auto * item = new QListWidgetItem(QString::fromStdString(faction->getNameTranslated()));
-		item->setData(Qt::UserRole, QVariant::fromValue(idx));
+		item->setData(Qt::UserRole, QVariant::fromValue(idx.getNum()));
 		item->setFlags(item->flags() | Qt::ItemIsUserCheckable);
 		ui->allowedFactions->addItem(item);
 		if(playerInfo.allowedFactions.count(idx))
@@ -124,9 +124,9 @@ void PlayerParams::on_randomFaction_stateChanged(int arg1)
 void PlayerParams::allowedFactionsCheck(QListWidgetItem * item)
 {
 	if(item->checkState() == Qt::Checked)
-		playerInfo.allowedFactions.insert(item->data(Qt::UserRole).toInt());
+		playerInfo.allowedFactions.insert(FactionID(item->data(Qt::UserRole).toInt()));
 	else
-		playerInfo.allowedFactions.erase(item->data(Qt::UserRole).toInt());
+		playerInfo.allowedFactions.erase(FactionID(item->data(Qt::UserRole).toInt()));
 }
 
 

scripting/lua/api/Artifact.cpp

@@ -34,7 +34,7 @@ const std::vector<ArtifactProxy::CustomRegType> ArtifactProxy::REGISTER_CUSTOM =
 	{"getName", LuaMethodWrapper<Artifact, decltype(&Entity::getNameTranslated), &Entity::getNameTranslated>::invoke, false},
 
 	{"getId", LuaMethodWrapper<Artifact, decltype(&EntityT<ArtifactID>::getId), &EntityT<ArtifactID>::getId>::invoke, false},
-	{"getBonusBearer", LuaMethodWrapper<Artifact, decltype(&EntityWithBonuses<ArtifactID>::getBonusBearer), &EntityWithBonuses<ArtifactID>::getBonusBearer>::invoke, false},
+	{"getBonusBearer", LuaMethodWrapper<Artifact, decltype(&IConstBonusProvider::getBonusBearer), &IConstBonusProvider::getBonusBearer>::invoke, false},
 
 	{"getDescription", LuaMethodWrapper<Artifact, decltype(&Artifact::getDescriptionTranslated), &Artifact::getDescriptionTranslated>::invoke, false},
 	{"getEventText", LuaMethodWrapper<Artifact, decltype(&Artifact::getEventTranslated), &Artifact::getEventTranslated>::invoke, false},

scripting/lua/api/Creature.cpp

@@ -32,7 +32,7 @@ const std::vector<CreatureProxy::CustomRegType> CreatureProxy::REGISTER_CUSTOM =
 	{"getIndex", LuaMethodWrapper<Creature, decltype(&Entity::getIndex), &Entity::getIndex>::invoke, false},
 	{"getJsonKey", LuaMethodWrapper<Creature, decltype(&Entity::getJsonKey), &Entity::getJsonKey>::invoke, false},
 	{"getName", LuaMethodWrapper<Creature, decltype(&Entity::getNameTranslated), &Entity::getNameTranslated>::invoke, false},
-	{"getBonusBearer", LuaMethodWrapper<Creature, decltype(&EntityWithBonuses<CreatureID>::getBonusBearer), &EntityWithBonuses<CreatureID>::getBonusBearer>::invoke, false},
+	{"getBonusBearer", LuaMethodWrapper<Creature, decltype(&IConstBonusProvider::getBonusBearer), &IConstBonusProvider::getBonusBearer>::invoke, false},
 
 	{"getMaxHealth", LuaMethodWrapper<Creature,decltype(&Creature::getMaxHealth), &Creature::getMaxHealth>::invoke, false},
 	{"getPluralName", LuaMethodWrapper<Creature, decltype(&Creature::getNamePluralTranslated), &Creature::getNamePluralTranslated>::invoke, false},
@@ -45,7 +45,7 @@ const std::vector<CreatureProxy::CustomRegType> CreatureProxy::REGISTER_CUSTOM =
 	{"getLevel", LuaMethodWrapper<Creature, decltype(&Creature::getLevel), &Creature::getLevel>::invoke, false},
 	{"getGrowth", LuaMethodWrapper<Creature, decltype(&Creature::getGrowth), &Creature::getGrowth>::invoke, false},
 	{"getHorde", LuaMethodWrapper<Creature, decltype(&Creature::getHorde), &Creature::getHorde>::invoke, false},
-	{"getFactionIndex", LuaMethodWrapper<Creature, decltype(&Creature::getFactionIndex), &Creature::getFactionIndex>::invoke, false},
+	{"getFaction", LuaMethodWrapper<Creature, decltype(&Creature::getFaction), &Creature::getFaction>::invoke, false},
 
 	{"getBaseAttack", LuaMethodWrapper<Creature, decltype(&Creature::getBaseAttack), &Creature::getBaseAttack>::invoke, false},
 	{"getBaseDefense", LuaMethodWrapper<Creature, decltype(&Creature::getBaseDefense), &Creature::getBaseDefense>::invoke, false},

scripts/lib/erm/MA.lua

@@ -80,7 +80,7 @@ MA.I = createModifier({}, "aiValue" ,"getAIValue")
 MA.L = createModifier({}, "level" , "getLevel")
 MA.M = createModifier({"damage"}, "min", "getBaseDamageMin")
 MA.N = createModifier({}, "shots" , "getBaseShots")
-MA.O = createModifier({}, "faction" ,"getFactionIndex")
+MA.O = createModifier({}, "faction" ,"getFaction")
 MA.P = createModifier({}, "hitPoints" ,"getBaseHitPoints")
 MA.R = createModifier({}, "horde" , "getHorde")
 MA.S = createModifier({}, "speed" , "getBaseSpeed")

server/CGameHandler.cpp

@@ -1753,7 +1753,7 @@ void CGameHandler::newTurn()
 					{
 						newMonster.second = VLC->creh->pickRandomMonster(getRandomGenerator());
 					} while (VLC->creh->objects[newMonster.second] &&
-						(*VLC->townh)[VLC->creatures()->getById(newMonster.second)->getFactionIndex()]->town == nullptr); // find first non neutral creature
+						(*VLC->townh)[VLC->creatures()->getById(newMonster.second)->getFaction()]->town == nullptr); // find first non neutral creature
 					n.creatureid = newMonster.second;
 				}
 			}

server/CVCMIServer.cpp

@@ -853,7 +853,7 @@ void CVCMIServer::optionNextCastle(PlayerColor player, int dir)
 		else
 		{
 			assert(dir >= -1 && dir <= 1); //othervice std::advance may go out of range
-			auto iter = allowed.find((ui8)cur);
+			auto iter = allowed.find(FactionID(cur));
 			std::advance(iter, dir);
 			cur = *iter;
 		}

test/entity/CCreatureTest.cpp

@@ -96,7 +96,7 @@ TEST_F(CCreatureTest, JsonUpdate)
 
 	EXPECT_EQ(subject->getFightValue(), 2420);
 	EXPECT_EQ(subject->getLevel(), 6);
-	EXPECT_EQ(subject->getFactionIndex(), 55);
+	EXPECT_EQ(subject->getFaction(), 55);
 	EXPECT_TRUE(subject->isDoubleWide());
 }
 

test/erm/ERM_MA.cpp

@@ -111,7 +111,7 @@ TEST_F(ERM_MA, Example)
 	EXPECT_CALL(oldCreature, getAIValue()).WillOnce(Return(AI_VALUE));
 	EXPECT_CALL(oldCreature, getFightValue()).WillOnce(Return(FIGHT_VALUE));
 	EXPECT_CALL(oldCreature, getLevel()).WillOnce(Return(LEVEL));
-	EXPECT_CALL(oldCreature, getFactionIndex()).WillOnce(Return(FACTION));
+	EXPECT_CALL(oldCreature, getFaction()).WillOnce(Return(FACTION));
 
 	EXPECT_CALL(oldCreature, isDoubleWide()).WillRepeatedly(Return(false));
 

test/mock/mock_Creature.h

@@ -13,6 +13,7 @@
 #include <vcmi/Creature.h>
 
 class IBonusBearer;
+class FactionID;
 
 class CreatureMock : public Creature
 {
@@ -36,7 +37,7 @@ public:
 	MOCK_CONST_METHOD0(getLevel, int32_t());
 	MOCK_CONST_METHOD0(getGrowth, int32_t());
 	MOCK_CONST_METHOD0(getHorde, int32_t());
-	MOCK_CONST_METHOD0(getFactionIndex, int32_t());
+	MOCK_CONST_METHOD0(getFaction, FactionID());
 
 	MOCK_CONST_METHOD0(getBaseAttack, int32_t());
 	MOCK_CONST_METHOD0(getBaseDefense, int32_t());