Browse Source

(lib) Bonus subtype is now stored as metaidentifier that can store any
other identifier inside it

Ivan Savenko 2 years ago
parent
commit
0a10fc30b8
54 changed files with 455 additions and 395 deletions
  1. 3 3
      AI/Nullkiller/Analyzers/HeroManager.cpp
  2. 1 1
      AI/Nullkiller/Engine/PriorityEvaluator.cpp
  3. 4 4
      AI/Nullkiller/Pathfinding/AINodeStorage.cpp
  4. 1 1
      AI/Nullkiller/Pathfinding/Rules/AILayerTransitionRule.cpp
  5. 1 1
      AI/VCAI/Pathfinding/AINodeStorage.cpp
  6. 1 1
      AI/VCAI/Pathfinding/Rules/AILayerTransitionRule.cpp
  7. 1 4
      config/schemas/bonus.json
  8. 5 3
      docs/modders/Bonus/Bonus_Types.md
  9. 1 1
      include/vcmi/FactionMember.h
  10. 1 1
      lib/ArtifactUtils.cpp
  11. 6 6
      lib/BasicTypes.cpp
  12. 1 1
      lib/CArtifactInstance.cpp
  13. 20 23
      lib/CBonusTypeHandler.cpp
  14. 43 45
      lib/CCreatureHandler.cpp
  15. 2 1
      lib/CCreatureHandler.h
  16. 3 3
      lib/CGameInfoCallback.cpp
  17. 3 3
      lib/CHeroHandler.cpp
  18. 1 1
      lib/CStack.cpp
  19. 9 9
      lib/CTownHandler.cpp
  20. 4 3
      lib/CTownHandler.h
  21. 37 61
      lib/JsonNode.cpp
  22. 0 9
      lib/JsonNode.h
  23. 3 3
      lib/battle/BattleInfo.cpp
  24. 2 2
      lib/battle/CBattleInfoCallback.cpp
  25. 6 6
      lib/battle/CUnitState.cpp
  26. 13 12
      lib/battle/DamageCalculator.cpp
  27. 5 28
      lib/bonuses/Bonus.cpp
  28. 43 4
      lib/bonuses/Bonus.h
  29. 34 41
      lib/bonuses/BonusParams.cpp
  30. 1 2
      lib/bonuses/BonusParams.h
  31. 5 5
      lib/bonuses/IBonusBearer.cpp
  32. 3 3
      lib/bonuses/IBonusBearer.h
  33. 4 4
      lib/bonuses/Limiters.cpp
  34. 108 0
      lib/constants/EntityIdentifiers.h
  35. 2 12
      lib/constants/Enumerations.h
  36. 1 1
      lib/gameState/CGameState.cpp
  37. 13 15
      lib/gameState/CGameStateCampaign.cpp
  38. 1 1
      lib/mapObjects/CGCreature.cpp
  39. 27 37
      lib/mapObjects/CGHeroInstance.cpp
  40. 1 1
      lib/mapObjects/CGHeroInstance.h
  41. 1 1
      lib/mapObjects/CGTownBuilding.cpp
  42. 1 1
      lib/mapObjects/CGTownInstance.cpp
  43. 2 2
      lib/mapObjects/MiscObjects.cpp
  44. 2 2
      lib/pathfinder/CPathfinder.cpp
  45. 1 1
      lib/pathfinder/CPathfinder.h
  46. 4 4
      lib/pathfinder/TurnInfo.cpp
  47. 4 2
      lib/pathfinder/TurnInfo.h
  48. 1 1
      lib/rewardable/Reward.cpp
  49. 1 1
      lib/spells/AbilityCaster.cpp
  50. 5 5
      lib/spells/CSpellHandler.cpp
  51. 6 6
      lib/spells/TargetCondition.cpp
  52. 2 2
      lib/spells/effects/Damage.cpp
  53. 4 4
      lib/spells/effects/Timed.cpp
  54. 1 1
      lib/spells/effects/UnitEffect.cpp

+ 3 - 3
AI/Nullkiller/Analyzers/HeroManager.cpp

@@ -313,7 +313,7 @@ void ExistingSkillRule::evaluateScore(const CGHeroInstance * hero, SecondarySkil
 		if(heroSkill.first == skill)
 			return;
 
-		upgradesLeft += SecSkillLevel::EXPERT - heroSkill.second;
+		upgradesLeft += MasteryLevel::EXPERT - heroSkill.second;
 	}
 
 	if(score >= 2 || (score >= 1 && upgradesLeft <= 1))
@@ -327,7 +327,7 @@ void WisdomRule::evaluateScore(const CGHeroInstance * hero, SecondarySkill skill
 
 	auto wisdomLevel = hero->getSecSkillLevel(SecondarySkill::WISDOM);
 
-	if(hero->level > 10 && wisdomLevel == SecSkillLevel::NONE)
+	if(hero->level > 10 && wisdomLevel == MasteryLevel::NONE)
 		score += 1.5;
 }
 
@@ -345,7 +345,7 @@ void AtLeastOneMagicRule::evaluateScore(const CGHeroInstance * hero, SecondarySk
 	
 	bool heroHasAnyMagic = vstd::contains_if(magicSchools, [&](SecondarySkill skill) -> bool
 	{
-		return hero->getSecSkillLevel(skill) > SecSkillLevel::NONE;
+		return hero->getSecSkillLevel(skill) > MasteryLevel::NONE;
 	});
 
 	if(!heroHasAnyMagic)

+ 1 - 1
AI/Nullkiller/Engine/PriorityEvaluator.cpp

@@ -538,7 +538,7 @@ float RewardEvaluator::evaluateWitchHutSkillScore(const CGObjectInstance * hut,
 	if(!hut->wasVisited(hero->tempOwner))
 		return role == HeroRole::SCOUT ? 2 : 0;
 
-	if(hero->getSecSkillLevel(skill) != SecSkillLevel::NONE
+	if(hero->getSecSkillLevel(skill) != MasteryLevel::NONE
 		|| hero->secSkills.size() >= GameConstants::SKILL_PER_HERO)
 		return 0;
 

+ 4 - 4
AI/Nullkiller/Pathfinding/AINodeStorage.cpp

@@ -984,7 +984,7 @@ std::vector<CGPathNode *> AINodeStorage::calculateTeleportations(
 struct TowmPortalFinder
 {
 	const std::vector<CGPathNode *> & initialNodes;
-	SecSkillLevel::SecSkillLevel townPortalSkillLevel;
+	MasteryLevel townPortalSkillLevel;
 	uint64_t movementNeeded;
 	const ChainActor * actor;
 	const CGHeroInstance * hero;
@@ -1006,8 +1006,8 @@ struct TowmPortalFinder
 		townPortal = spellID.toSpell();
 
 		// TODO: Copy/Paste from TownPortalMechanics
-		townPortalSkillLevel = SecSkillLevel::SecSkillLevel(hero->getSpellSchoolLevel(townPortal));
-		movementNeeded = GameConstants::BASE_MOVEMENT_COST * (townPortalSkillLevel >= SecSkillLevel::EXPERT ? 2 : 3);
+		townPortalSkillLevel = MasteryLevel(hero->getSpellSchoolLevel(townPortal));
+		movementNeeded = GameConstants::BASE_MOVEMENT_COST * (townPortalSkillLevel >= MasteryLevel::EXPERT ? 2 : 3);
 	}
 
 	bool actorCanCastTownPortal()
@@ -1028,7 +1028,7 @@ struct TowmPortalFinder
 				continue;
 			}
 
-			if(townPortalSkillLevel < SecSkillLevel::ADVANCED)
+			if(townPortalSkillLevel < MasteryLevel::ADVANCED)
 			{
 				const CGTownInstance * nearestTown = *vstd::minElementByFun(targetTowns, [&](const CGTownInstance * t) -> int
 				{

+ 1 - 1
AI/Nullkiller/Pathfinding/Rules/AILayerTransitionRule.cpp

@@ -139,7 +139,7 @@ namespace AIPathfinding
 			auto summonBoatSpell = SpellID(SpellID::SUMMON_BOAT).toSpell();
 
 			if(hero->canCastThisSpell(summonBoatSpell)
-				&& hero->getSpellSchoolLevel(summonBoatSpell) >= SecSkillLevel::ADVANCED)
+				&& hero->getSpellSchoolLevel(summonBoatSpell) >= MasteryLevel::ADVANCED)
 			{
 				// TODO: For lower school level we might need to check the existance of some boat
 				summonableVirtualBoats[hero] = std::make_shared<SummonBoatAction>();

+ 1 - 1
AI/VCAI/Pathfinding/AINodeStorage.cpp

@@ -250,7 +250,7 @@ void AINodeStorage::calculateTownPortalTeleportations(
 			return;
 		}
 
-		if(skillLevel < SecSkillLevel::ADVANCED)
+		if(skillLevel < MasteryLevel::ADVANCED)
 		{
 			const CGTownInstance * nearestTown = *vstd::minElementByFun(towns, [&](const CGTownInstance * t) -> int
 			{

+ 1 - 1
AI/VCAI/Pathfinding/Rules/AILayerTransitionRule.cpp

@@ -77,7 +77,7 @@ namespace AIPathfinding
 		auto summonBoatSpell = SpellID(SpellID::SUMMON_BOAT).toSpell();
 
 		if(hero->canCastThisSpell(summonBoatSpell)
-			&& hero->getSpellSchoolLevel(summonBoatSpell) >= SecSkillLevel::ADVANCED)
+			&& hero->getSpellSchoolLevel(summonBoatSpell) >= MasteryLevel::ADVANCED)
 		{
 			// TODO: For lower school level we might need to check the existance of some boat
 			summonableVirtualBoat.reset(new SummonBoatAction());

+ 1 - 4
config/schemas/bonus.json

@@ -44,10 +44,7 @@
 			"description" : "type"
 		},
 		"subtype" : {
-			"anyOf" : [
-				{ "type" : "string" },
-				{ "type" : "number" }
-			],
+			"type" : "string",
 			"description" : "subtype"
 		},
 		"sourceID" : {

+ 5 - 3
docs/modders/Bonus/Bonus_Types.md

@@ -294,8 +294,9 @@ Heroes affected by this bonus can not retreat or surrender in battle
 
 ### NEGATE_ALL_NATURAL_IMMUNITIES
 
-- subtype: TODO
-Orb of Vulnerability
+Negates all natural immunities for affected stacks. (Orb of Vulnerability)
+
+- subtype: 0 - battle-wide immunity negation, 1 - negation only for enemy stacks
 
 ### OPENING_BATTLE_SPELL
 
@@ -878,13 +879,14 @@ Affected unit will deal more damage in all attacks (Adela specialty)
 
 Affected heroes will be under effect of Disguise spell, hiding some of their information from opposing players
 
-- subtype: spell mastery level
+- val: spell mastery level
 
 ### VISIONS
 
 Affected heroes will be under effect of Visions spell, revealing information of enemy objects in specific range
 
 - val: multiplier to effect range. Information is revealed within (val \* hero spell power) range
+- subtype: 0 - reveal information on monsters, 1 - reveal information on heroes, 2 - reveal information on towns
 
 ### BLOCK_MAGIC_BELOW
 

+ 1 - 1
include/vcmi/FactionMember.h

@@ -15,7 +15,7 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 class BonusList;
-enum class PrimarySkill : int8_t;
+class PrimarySkill;
 
 class DLL_LINKAGE AFactionMember: public IConstBonusProvider, public INativeTerrainProvider
 {

+ 1 - 1
lib/ArtifactUtils.cpp

@@ -145,7 +145,7 @@ DLL_LINKAGE CArtifactInstance * ArtifactUtils::createScroll(const SpellID & sid)
 {
 	auto ret = new CArtifactInstance(VLC->arth->objects[ArtifactID::SPELL_SCROLL]);
 	auto bonus = std::make_shared<Bonus>(BonusDuration::PERMANENT, BonusType::SPELL,
-		BonusSource::ARTIFACT_INSTANCE, -1, ArtifactID::SPELL_SCROLL, sid);
+		BonusSource::ARTIFACT_INSTANCE, -1, ArtifactID::SPELL_SCROLL, TBonusSubtype(sid));
 	ret->addNewBonus(bonus);
 	return ret;
 }

+ 6 - 6
lib/BasicTypes.cpp

@@ -35,7 +35,7 @@ TerrainId AFactionMember::getNativeTerrain() const
 {
 	constexpr auto any = TerrainId(ETerrainId::ANY_TERRAIN);
 	const std::string cachingStringNoTerrainPenalty = "type_NO_TERRAIN_PENALTY_sANY";
-	static const auto selectorNoTerrainPenalty = Selector::typeSubtype(BonusType::NO_TERRAIN_PENALTY, any);
+	static const auto selectorNoTerrainPenalty = Selector::typeSubtype(BonusType::NO_TERRAIN_PENALTY, TBonusSubtype(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.
@@ -54,7 +54,7 @@ int AFactionMember::getAttack(bool ranged) const
 {
 	const std::string cachingStr = "type_PRIMARY_SKILLs_ATTACK";
 
-	static const auto selector = Selector::typeSubtype(BonusType::PRIMARY_SKILL, static_cast<int>(PrimarySkill::ATTACK));
+	static const auto selector = Selector::typeSubtype(BonusType::PRIMARY_SKILL, TBonusSubtype(PrimarySkill::ATTACK));
 
 	return getBonusBearer()->valOfBonuses(selector, cachingStr);
 }
@@ -63,7 +63,7 @@ int AFactionMember::getDefense(bool ranged) const
 {
 	const std::string cachingStr = "type_PRIMARY_SKILLs_DEFENSE";
 
-	static const auto selector = Selector::typeSubtype(BonusType::PRIMARY_SKILL, static_cast<int>(PrimarySkill::DEFENSE));
+	static const auto selector = Selector::typeSubtype(BonusType::PRIMARY_SKILL, TBonusSubtype(PrimarySkill::DEFENSE));
 
 	return getBonusBearer()->valOfBonuses(selector, cachingStr);
 }
@@ -71,14 +71,14 @@ int AFactionMember::getDefense(bool ranged) const
 int AFactionMember::getMinDamage(bool ranged) const
 {
 	const std::string cachingStr = "type_CREATURE_DAMAGEs_0Otype_CREATURE_DAMAGEs_1";
-	static const auto selector = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, 0).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, 1));
+	static const auto selector = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageBoth).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageMin));
 	return getBonusBearer()->valOfBonuses(selector, cachingStr);
 }
 
 int AFactionMember::getMaxDamage(bool ranged) const
 {
 	const std::string cachingStr = "type_CREATURE_DAMAGEs_0Otype_CREATURE_DAMAGEs_2";
-	static const auto selector = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, 0).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, 2));
+	static const auto selector = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageBoth).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageMax));
 	return getBonusBearer()->valOfBonuses(selector, cachingStr);
 }
 
@@ -87,7 +87,7 @@ int AFactionMember::getPrimSkillLevel(PrimarySkill id) const
 	static const CSelector selectorAllSkills = Selector::type()(BonusType::PRIMARY_SKILL);
 	static const std::string keyAllSkills = "type_PRIMARY_SKILL";
 	auto allSkills = getBonusBearer()->getBonuses(selectorAllSkills, keyAllSkills);
-	auto ret = allSkills->valOfBonuses(Selector::subtype()(static_cast<int>(id)));
+	auto ret = allSkills->valOfBonuses(Selector::subtype()(TBonusSubtype(id)));
 	auto minSkillValue = (id == PrimarySkill::SPELL_POWER || id == PrimarySkill::KNOWLEDGE) ? 1 : 0;
 	return std::max(ret, minSkillValue); //otherwise, some artifacts may cause negative skill value effect, sp=0 works in old saves
 }

+ 1 - 1
lib/CArtifactInstance.cpp

@@ -68,7 +68,7 @@ SpellID CScrollArtifactInstance::getScrollSpellID() const
 		logMod->warn("Warning: %s doesn't bear any spell!", artInst->nodeName());
 		return SpellID::NONE;
 	}
-	return SpellID(bonus->subtype);
+	return bonus->subtype.as<SpellID>();
 }
 
 void CGrowingArtifactInstance::growingUp()

+ 20 - 23
lib/CBonusTypeHandler.cpp

@@ -77,10 +77,10 @@ std::string CBonusTypeHandler::bonusToString(const std::shared_ptr<Bonus> & bonu
 		boost::algorithm::replace_all(text, "${val}", std::to_string(bearer->valOfBonuses(Selector::typeSubtype(bonus->type, bonus->subtype))));
 
 	if (text.find("${subtype.creature}") != std::string::npos)
-		boost::algorithm::replace_all(text, "${subtype.creature}", CreatureID(bonus->subtype).toCreature()->getNamePluralTranslated());
+		boost::algorithm::replace_all(text, "${subtype.creature}", bonus->subtype.as<CreatureID>().toCreature()->getNamePluralTranslated());
 
 	if (text.find("${subtype.spell}") != std::string::npos)
-		boost::algorithm::replace_all(text, "${subtype.spell}", SpellID(bonus->subtype).toSpell()->getNameTranslated());
+		boost::algorithm::replace_all(text, "${subtype.spell}", bonus->subtype.as<SpellID>().toSpell()->getNameTranslated());
 
 	return text;
 }
@@ -95,57 +95,57 @@ ImagePath CBonusTypeHandler::bonusToGraphics(const std::shared_ptr<Bonus> & bonu
 	case BonusType::SPELL_IMMUNITY:
 	{
 		fullPath = true;
-		const CSpell * sp = SpellID(bonus->subtype).toSpell();
+		const CSpell * sp = bonus->subtype.as<SpellID>().toSpell();
 		fileName = sp->getIconImmune();
 		break;
 	}
 	case BonusType::SPELL_DAMAGE_REDUCTION: //Spell damage reduction for all schools
 	{
-		if (bonus->subtype == SpellSchool::ANY.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::ANY)
 			fileName = "E_GOLEM.bmp";
 
-		if (bonus->subtype == SpellSchool::AIR.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::AIR)
 			fileName = "E_LIGHT.bmp";
 
-		if (bonus->subtype == SpellSchool::FIRE.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::FIRE)
 			fileName = "E_FIRE.bmp";
 
-		if (bonus->subtype == SpellSchool::WATER.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::WATER)
 			fileName = "E_COLD.bmp";
 
-		if (bonus->subtype == SpellSchool::EARTH.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::EARTH)
 			fileName = "E_SPEATH1.bmp"; //No separate icon for earth damage
 
 		break;
 	}
 	case BonusType::SPELL_SCHOOL_IMMUNITY: //for all school
 	{
-		if (bonus->subtype == SpellSchool::AIR.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::AIR)
 			fileName = "E_SPAIR.bmp";
 
-		if (bonus->subtype == SpellSchool::FIRE.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::FIRE)
 			fileName = "E_SPFIRE.bmp";
 
-		if (bonus->subtype == SpellSchool::WATER.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::WATER)
 			fileName = "E_SPWATER.bmp";
 
-		if (bonus->subtype == SpellSchool::EARTH.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::EARTH)
 			fileName = "E_SPEATH.bmp";
 
 		break;
 	}
 	case BonusType::NEGATIVE_EFFECTS_IMMUNITY:
 	{
-		if (bonus->subtype == SpellSchool::AIR.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::AIR)
 			fileName = "E_SPAIR1.bmp";
 
-		if (bonus->subtype == SpellSchool::FIRE.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::FIRE)
 			fileName = "E_SPFIRE1.bmp";
 
-		if (bonus->subtype == SpellSchool::WATER.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::WATER)
 			fileName = "E_SPWATER1.bmp";
 
-		if (bonus->subtype == SpellSchool::EARTH.getNum())
+		if (bonus->subtype.as<SpellSchool>() == SpellSchool::EARTH)
 			fileName = "E_SPEATH1.bmp";
 
 		break;
@@ -168,15 +168,12 @@ ImagePath CBonusTypeHandler::bonusToGraphics(const std::shared_ptr<Bonus> & bonu
 	}
 	case BonusType::GENERAL_DAMAGE_REDUCTION:
 	{
-		switch(bonus->subtype)
-		{
-		case 0:
+		if (bonus->subtype == BonusSubtypes::damageTypeMelee)
 			fileName = "DamageReductionMelee.bmp";
-			break;
-		case 1:
+
+		if (bonus->subtype == BonusSubtypes::damageTypeRanged)
 			fileName = "DamageReductionRanged.bmp";
-			break;
-		}
+
 		break;
 	}
 

+ 43 - 45
lib/CCreatureHandler.cpp

@@ -113,25 +113,25 @@ FactionID CCreature::getFaction() const
 
 int32_t CCreature::getBaseAttack() const
 {
-	static const auto SELECTOR = Selector::typeSubtype(BonusType::PRIMARY_SKILL, static_cast<int>(PrimarySkill::ATTACK)).And(Selector::sourceTypeSel(BonusSource::CREATURE_ABILITY));
+	static const auto SELECTOR = Selector::typeSubtype(BonusType::PRIMARY_SKILL, TBonusSubtype(PrimarySkill::ATTACK)).And(Selector::sourceTypeSel(BonusSource::CREATURE_ABILITY));
 	return getExportedBonusList().valOfBonuses(SELECTOR);
 }
 
 int32_t CCreature::getBaseDefense() const
 {
-	static const auto SELECTOR = Selector::typeSubtype(BonusType::PRIMARY_SKILL, static_cast<int>(PrimarySkill::DEFENSE)).And(Selector::sourceTypeSel(BonusSource::CREATURE_ABILITY));
+	static const auto SELECTOR = Selector::typeSubtype(BonusType::PRIMARY_SKILL, TBonusSubtype(PrimarySkill::DEFENSE)).And(Selector::sourceTypeSel(BonusSource::CREATURE_ABILITY));
 	return getExportedBonusList().valOfBonuses(SELECTOR);
 }
 
 int32_t CCreature::getBaseDamageMin() const
 {
-	static const auto SELECTOR = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, 1).And(Selector::sourceTypeSel(BonusSource::CREATURE_ABILITY));
+	static const auto SELECTOR = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageMin).And(Selector::sourceTypeSel(BonusSource::CREATURE_ABILITY));
 	return getExportedBonusList().valOfBonuses(SELECTOR);
 }
 
 int32_t CCreature::getBaseDamageMax() const
 {
-	static const auto SELECTOR = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, 2).And(Selector::sourceTypeSel(BonusSource::CREATURE_ABILITY));
+	static const auto SELECTOR = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageMax).And(Selector::sourceTypeSel(BonusSource::CREATURE_ABILITY));
 	return getExportedBonusList().valOfBonuses(SELECTOR);
 }
 
@@ -291,7 +291,7 @@ CCreature::CCreature()
 	fightValue = AIValue = growth = hordeGrowth = ammMin = ammMax = 0;
 }
 
-void CCreature::addBonus(int val, BonusType type, int subtype)
+void CCreature::addBonus(int val, BonusType type, TBonusSubtype subtype)
 {
 	auto selector = Selector::typeSubtype(type, subtype).And(Selector::source(BonusSource::CREATURE_ABILITY, getIndex()));
 	BonusList & exported = getExportedBonusList();
@@ -345,16 +345,16 @@ void CCreature::updateFrom(const JsonNode & data)
 			addBonus(configNode["speed"].Integer(), BonusType::STACKS_SPEED);
 
 		if(!configNode["attack"].isNull())
-			addBonus(configNode["attack"].Integer(), BonusType::PRIMARY_SKILL, static_cast<int>(PrimarySkill::ATTACK));
+			addBonus(configNode["attack"].Integer(), BonusType::PRIMARY_SKILL, TBonusSubtype(PrimarySkill::ATTACK));
 
 		if(!configNode["defense"].isNull())
-			addBonus(configNode["defense"].Integer(), BonusType::PRIMARY_SKILL, static_cast<int>(PrimarySkill::DEFENSE));
+			addBonus(configNode["defense"].Integer(), BonusType::PRIMARY_SKILL, TBonusSubtype(PrimarySkill::DEFENSE));
 
 		if(!configNode["damage"]["min"].isNull())
-			addBonus(configNode["damage"]["min"].Integer(), BonusType::CREATURE_DAMAGE, 1);
+			addBonus(configNode["damage"]["min"].Integer(), BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageMin);
 
 		if(!configNode["damage"]["max"].isNull())
-			addBonus(configNode["damage"]["max"].Integer(), BonusType::CREATURE_DAMAGE, 2);
+			addBonus(configNode["damage"]["max"].Integer(), BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageMax);
 
 		if(!configNode["shots"].isNull())
 			addBonus(configNode["shots"].Integer(), BonusType::SHOTS);
@@ -604,11 +604,11 @@ CCreature * CCreatureHandler::loadFromJson(const std::string & scope, const Json
 
 	cre->addBonus(node["hitPoints"].Integer(), BonusType::STACK_HEALTH);
 	cre->addBonus(node["speed"].Integer(), BonusType::STACKS_SPEED);
-	cre->addBonus(node["attack"].Integer(), BonusType::PRIMARY_SKILL, static_cast<int>(PrimarySkill::ATTACK));
-	cre->addBonus(node["defense"].Integer(), BonusType::PRIMARY_SKILL, static_cast<int>(PrimarySkill::DEFENSE));
+	cre->addBonus(node["attack"].Integer(), BonusType::PRIMARY_SKILL, TBonusSubtype(PrimarySkill::ATTACK));
+	cre->addBonus(node["defense"].Integer(), BonusType::PRIMARY_SKILL, TBonusSubtype(PrimarySkill::DEFENSE));
 
-	cre->addBonus(node["damage"]["min"].Integer(), BonusType::CREATURE_DAMAGE, 1);
-	cre->addBonus(node["damage"]["max"].Integer(), BonusType::CREATURE_DAMAGE, 2);
+	cre->addBonus(node["damage"]["min"].Integer(), BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageMin);
+	cre->addBonus(node["damage"]["max"].Integer(), BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageMax);
 
 	assert(node["damage"]["min"].Integer() <= node["damage"]["max"].Integer());
 
@@ -1025,19 +1025,19 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 		break;
 	case 'A':
 		b.type = BonusType::PRIMARY_SKILL;
-		b.subtype = static_cast<int>(PrimarySkill::ATTACK);
+		b.subtype = TBonusSubtype(PrimarySkill::ATTACK);
 		break;
 	case 'D':
 		b.type = BonusType::PRIMARY_SKILL;
-		b.subtype = static_cast<int>(PrimarySkill::DEFENSE);
+		b.subtype = TBonusSubtype(PrimarySkill::DEFENSE);
 		break;
 	case 'M': //Max damage
 		b.type = BonusType::CREATURE_DAMAGE;
-		b.subtype = 2;
+		b.subtype = BonusSubtypes::creatureDamageMax;
 		break;
 	case 'm': //Min damage
 		b.type = BonusType::CREATURE_DAMAGE;
-		b.subtype = 1;
+		b.subtype = BonusSubtypes::creatureDamageMin;
 		break;
 	case 'S':
 		b.type = BonusType::STACKS_SPEED; break;
@@ -1051,17 +1051,16 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 		b.type = BonusType::DEFENSIVE_STANCE; break;
 	case 'e':
 		b.type = BonusType::DOUBLE_DAMAGE_CHANCE;
-		b.subtype = 0;
 		break;
 	case 'E':
 		b.type = BonusType::DEATH_STARE;
-		b.subtype = 0; //Gorgon
+		b.subtype = BonusSubtypes::deathStareGorgon;
 		break;
 	case 'F':
 		b.type = BonusType::FEAR; break;
 	case 'g':
 		b.type = BonusType::SPELL_DAMAGE_REDUCTION;
-		b.subtype = SpellSchool(ESpellSchool::ANY);
+		b.subtype = TBonusSubtype(SpellSchool::ANY);
 		break;
 	case 'P':
 		b.type = BonusType::CASTS; break;
@@ -1069,7 +1068,6 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 		b.type = BonusType::ADDITIONAL_RETALIATION; break;
 	case 'W':
 		b.type = BonusType::MAGIC_RESISTANCE;
-		b.subtype = 0; //otherwise creature window goes crazy
 		break;
 	case 'f': //on-off skill
 		enable = true; //sometimes format is: 2 -> 0, 1 -> 1
@@ -1103,7 +1101,7 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 				b.type = BonusType::MIND_IMMUNITY; break;
 			case 'r':
 				b.type = BonusType::REBIRTH; //on/off? makes sense?
-				b.subtype = 0;
+				b.subtype = BonusSubtypes::rebirthRegular;
 				b.val = 20; //arbitrary value
 				break;
 			case 'R':
@@ -1126,42 +1124,42 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 		{
 			case 'B': //Blind
 				b.type = BonusType::SPELL_IMMUNITY;
-				b.subtype = SpellID::BLIND;
+				b.subtype = TBonusSubtype(SpellID(SpellID::BLIND));
 				b.additionalInfo = 0;//normal immunity
 				break;
 			case 'H': //Hypnotize
 				b.type = BonusType::SPELL_IMMUNITY;
-				b.subtype = SpellID::HYPNOTIZE;
+				b.subtype = TBonusSubtype(SpellID(SpellID::HYPNOTIZE));
 				b.additionalInfo = 0;//normal immunity
 				break;
 			case 'I': //Implosion
 				b.type = BonusType::SPELL_IMMUNITY;
-				b.subtype = SpellID::IMPLOSION;
+				b.subtype = TBonusSubtype(SpellID(SpellID::IMPLOSION));
 				b.additionalInfo = 0;//normal immunity
 				break;
 			case 'K': //Berserk
 				b.type = BonusType::SPELL_IMMUNITY;
-				b.subtype = SpellID::BERSERK;
+				b.subtype = TBonusSubtype(SpellID(SpellID::BERSERK));
 				b.additionalInfo = 0;//normal immunity
 				break;
 			case 'M': //Meteor Shower
 				b.type = BonusType::SPELL_IMMUNITY;
-				b.subtype = SpellID::METEOR_SHOWER;
+				b.subtype = TBonusSubtype(SpellID(SpellID::METEOR_SHOWER));
 				b.additionalInfo = 0;//normal immunity
 				break;
 			case 'N': //dispell beneficial spells
 				b.type = BonusType::SPELL_IMMUNITY;
-				b.subtype = SpellID::DISPEL_HELPFUL_SPELLS;
+				b.subtype = TBonusSubtype(SpellID(SpellID::DISPEL_HELPFUL_SPELLS));
 				b.additionalInfo = 0;//normal immunity
 				break;
 			case 'R': //Armageddon
 				b.type = BonusType::SPELL_IMMUNITY;
-				b.subtype = SpellID::ARMAGEDDON;
+				b.subtype = TBonusSubtype(SpellID(SpellID::ARMAGEDDON));
 				b.additionalInfo = 0;//normal immunity
 				break;
 			case 'S': //Slow
 				b.type = BonusType::SPELL_IMMUNITY;
-				b.subtype = SpellID::SLOW;
+				b.subtype = TBonusSubtype(SpellID(SpellID::SLOW));
 				b.additionalInfo = 0;//normal immunity
 				break;
 			case '6':
@@ -1177,51 +1175,51 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 				break;
 			case 'F':
 				b.type = BonusType::NEGATIVE_EFFECTS_IMMUNITY;
-				b.subtype = SpellSchool(ESpellSchool::FIRE); 
+				b.subtype = TBonusSubtype(SpellSchool::FIRE);
 				break;
 			case 'O':
 				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
-				b.subtype = SpellSchool(ESpellSchool::FIRE);
+				b.subtype = TBonusSubtype(SpellSchool::FIRE);
 				b.val = 100; //Full damage immunity
 				break;
 			case 'f':
 				b.type = BonusType::SPELL_SCHOOL_IMMUNITY;
-				b.subtype = SpellSchool(ESpellSchool::FIRE); 
+				b.subtype = TBonusSubtype(SpellSchool::FIRE);
 				break;
 			case 'C':
 				b.type = BonusType::NEGATIVE_EFFECTS_IMMUNITY;
-				b.subtype = SpellSchool(ESpellSchool::WATER);
+				b.subtype = TBonusSubtype(SpellSchool::WATER);
 				break;
 			case 'W':
 				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
-				b.subtype = SpellSchool(ESpellSchool::WATER);
+				b.subtype = TBonusSubtype(SpellSchool::WATER);
 				b.val = 100; //Full damage immunity
 				break;
 			case 'w':
 				b.type = BonusType::SPELL_SCHOOL_IMMUNITY;
-				b.subtype = SpellSchool(ESpellSchool::WATER);
+				b.subtype = TBonusSubtype(SpellSchool::WATER);
 				break;
 			case 'E':
 				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
-				b.subtype = SpellSchool(ESpellSchool::EARTH);
+				b.subtype = TBonusSubtype(SpellSchool::EARTH);
 				b.val = 100; //Full damage immunity
 				break;
 			case 'e':
 				b.type = BonusType::SPELL_SCHOOL_IMMUNITY;
-				b.subtype = SpellSchool(ESpellSchool::EARTH);
+				b.subtype = TBonusSubtype(SpellSchool::EARTH);
 				break;
 			case 'A':
 				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
-				b.subtype = SpellSchool(ESpellSchool::AIR);
+				b.subtype = TBonusSubtype(SpellSchool::AIR);
 				b.val = 100; //Full damage immunity
 				break;
 			case 'a':
 				b.type = BonusType::SPELL_SCHOOL_IMMUNITY;
-				b.subtype = SpellSchool(ESpellSchool::AIR);
+				b.subtype = TBonusSubtype(SpellSchool::AIR);
 				break;
 			case 'D':
 				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
-				b.subtype = SpellSchool(ESpellSchool::ANY);
+				b.subtype = TBonusSubtype(SpellSchool::ANY);
 				b.val = 100; //Full damage immunity
 				break;
 			case '0':
@@ -1250,16 +1248,16 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 	case 'K':
 	case 'k':
 		b.type = BonusType::SPELL_AFTER_ATTACK;
-		b.subtype = stringToNumber(mod);
+		b.subtype = TBonusSubtype(SpellID(stringToNumber(mod)));
 		break;
 	case 'h':
 		b.type = BonusType::HATE;
-		b.subtype = stringToNumber(mod);
+		b.subtype = TBonusSubtype(CreatureID(stringToNumber(mod)));
 		break;
 	case 'p':
 	case 'J':
 		b.type = BonusType::SPELL_BEFORE_ATTACK;
-		b.subtype = stringToNumber(mod);
+		b.subtype = TBonusSubtype(SpellID(stringToNumber(mod)));
 		b.additionalInfo = 3; //always expert?
 		break;
 	case 'r':
@@ -1268,7 +1266,7 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 		break;
 	case 's':
 		b.type = BonusType::ENCHANTED;
-		b.subtype = stringToNumber(mod);
+		b.subtype = TBonusSubtype(SpellID(stringToNumber(mod)));
 		b.valType = BonusValueType::INDEPENDENT_MAX;
 		break;
 	default:

+ 2 - 1
lib/CCreatureHandler.h

@@ -194,7 +194,8 @@ public:
 
 	bool valid() const;
 
-	void addBonus(int val, BonusType type, int subtype = -1);
+	void addBonus(int val, BonusType type);
+	void addBonus(int val, BonusType type, TBonusSubtype subtype);
 	std::string nodeName() const override;
 
 	template<typename RanGen>

+ 3 - 3
lib/CGameInfoCallback.cpp

@@ -268,7 +268,7 @@ bool CGameInfoCallback::getTownInfo(const CGObjectInstance * town, InfoAboutTown
 		{
 			const auto * selectedHero = dynamic_cast<const CGHeroInstance *>(selectedObject);
 			if(nullptr != selectedHero)
-				detailed = selectedHero->hasVisions(town, 1);
+				detailed = selectedHero->hasVisions(town, BonusSubtypes::visionsTowns);
 		}
 
 		dest.initFromTown(dynamic_cast<const CGTownInstance *>(town), detailed);
@@ -322,7 +322,7 @@ bool CGameInfoCallback::getHeroInfo(const CGObjectInstance * hero, InfoAboutHero
 	{
 		const auto * selectedHero = dynamic_cast<const CGHeroInstance *>(selectedObject);
 		if(nullptr != selectedHero)
-			if(selectedHero->hasVisions(hero, 1))
+			if(selectedHero->hasVisions(hero, BonusSubtypes::visionsHeroes))
 				infoLevel = InfoAboutHero::EInfoLevel::DETAILED;
 	}
 
@@ -332,7 +332,7 @@ bool CGameInfoCallback::getHeroInfo(const CGObjectInstance * hero, InfoAboutHero
 	if(getPlayerRelations(*getPlayerID(), hero->tempOwner) == PlayerRelations::ENEMIES)
 	{
 		//todo: bonus cashing
-		int disguiseLevel = h->valOfBonuses(Selector::typeSubtype(BonusType::DISGUISED, 0));
+		int disguiseLevel = h->valOfBonuses(BonusType::DISGUISED);
 
 		auto doBasicDisguise = [](InfoAboutHero & info)
 		{

+ 3 - 3
lib/CHeroHandler.cpp

@@ -480,7 +480,7 @@ void CHeroHandler::loadHeroSkills(CHero * hero, const JsonNode & node) const
 	for(const JsonNode &set : node["skills"].Vector())
 	{
 		int skillLevel = static_cast<int>(boost::range::find(NSecondarySkill::levels, set["level"].String()) - std::begin(NSecondarySkill::levels));
-		if (skillLevel < SecSkillLevel::LEVELS_SIZE)
+		if (skillLevel < MasteryLevel::LEVELS_SIZE)
 		{
 			size_t currentIndex = hero->secSkillsInit.size();
 			hero->secSkillsInit.emplace_back(SecondarySkill(-1), skillLevel);
@@ -547,7 +547,7 @@ static std::vector<std::shared_ptr<Bonus>> createCreatureSpecialty(CreatureID ba
 		{
 			std::shared_ptr<Bonus> bonus = std::make_shared<Bonus>();
 			bonus->type = BonusType::PRIMARY_SKILL;
-			bonus->subtype = static_cast<int>(PrimarySkill::ATTACK);
+			bonus->subtype = TBonusSubtype(PrimarySkill::ATTACK);
 			bonus->val = 0;
 			bonus->limiter.reset(new CCreatureTypeLimiter(specCreature, false));
 			bonus->updater.reset(new GrowsWithLevelUpdater(specCreature.getAttack(false), stepSize));
@@ -557,7 +557,7 @@ static std::vector<std::shared_ptr<Bonus>> createCreatureSpecialty(CreatureID ba
 		{
 			std::shared_ptr<Bonus> bonus = std::make_shared<Bonus>();
 			bonus->type = BonusType::PRIMARY_SKILL;
-			bonus->subtype = static_cast<int>(PrimarySkill::DEFENSE);
+			bonus->subtype = TBonusSubtype(PrimarySkill::DEFENSE);
 			bonus->val = 0;
 			bonus->limiter.reset(new CCreatureTypeLimiter(specCreature, false));
 			bonus->updater.reset(new GrowsWithLevelUpdater(specCreature.getDefense(false), stepSize));

+ 1 - 1
lib/CStack.cpp

@@ -220,7 +220,7 @@ void CStack::prepareAttacked(BattleStackAttacked & bsa, vstd::RNG & rand, const
 					resurrectedCount += 1;
 			}
 
-			if(customState->hasBonusOfType(BonusType::REBIRTH, 1))
+			if(customState->hasBonusOfType(BonusType::REBIRTH, BonusSubtypes::rebirthSpecial))
 			{
 				// resurrect at least one Sacred Phoenix
 				vstd::amax(resurrectedCount, 1);

+ 9 - 9
lib/CTownHandler.cpp

@@ -527,16 +527,16 @@ void CTownHandler::addBonusesForVanilaBuilding(CBuilding * building) const
 		b = createBonus(building, BonusType::LUCK, +2);
 		break;
 	case BuildingSubID::SPELL_POWER_GARRISON_BONUS:
-		b = createBonus(building, BonusType::PRIMARY_SKILL, +2, static_cast<int>(PrimarySkill::SPELL_POWER));
+		b = createBonus(building, BonusType::PRIMARY_SKILL, +2, TBonusSubtype(PrimarySkill::SPELL_POWER));
 		break;
 	case BuildingSubID::ATTACK_GARRISON_BONUS:
-		b = createBonus(building, BonusType::PRIMARY_SKILL, +2, static_cast<int>(PrimarySkill::ATTACK));
+		b = createBonus(building, BonusType::PRIMARY_SKILL, +2, TBonusSubtype(PrimarySkill::ATTACK));
 		break;
 	case BuildingSubID::DEFENSE_GARRISON_BONUS:
-		b = createBonus(building, BonusType::PRIMARY_SKILL, +2, static_cast<int>(PrimarySkill::DEFENSE));
+		b = createBonus(building, BonusType::PRIMARY_SKILL, +2, TBonusSubtype(PrimarySkill::DEFENSE));
 		break;
 	case BuildingSubID::LIGHTHOUSE:
-		b = createBonus(building, BonusType::MOVEMENT, +500, playerPropagator, 0);
+		b = createBonus(building, BonusType::MOVEMENT, +500, BonusSubtypes::heroMovementSea, playerPropagator);
 		break;
 	}
 
@@ -544,12 +544,12 @@ void CTownHandler::addBonusesForVanilaBuilding(CBuilding * building) const
 		building->addNewBonus(b, building->buildingBonuses);
 }
 
-std::shared_ptr<Bonus> CTownHandler::createBonus(CBuilding * build, BonusType type, int val, int subtype) const
+std::shared_ptr<Bonus> CTownHandler::createBonus(CBuilding * build, BonusType type, int val, TBonusSubtype subtype) const
 {
-	return createBonus(build, type, val, emptyPropagator(), subtype);
+	return createBonus(build, type, val, subtype, emptyPropagator());
 }
 
-std::shared_ptr<Bonus> CTownHandler::createBonus(CBuilding * build, BonusType type, int val, TPropagatorPtr & prop, int subtype) const
+std::shared_ptr<Bonus> CTownHandler::createBonus(CBuilding * build, BonusType type, int val, TBonusSubtype subtype, TPropagatorPtr & prop) const
 {
 	std::ostringstream descr;
 	descr << build->getNameTranslated();
@@ -561,9 +561,9 @@ std::shared_ptr<Bonus> CTownHandler::createBonusImpl(const BuildingID & building
 													 int val,
 													 TPropagatorPtr & prop,
 													 const std::string & description,
-													 int subtype) const
+													 TBonusSubtype subtype) const
 {
-	auto b = std::make_shared<Bonus>(BonusDuration::PERMANENT, type, BonusSource::TOWN_STRUCTURE, val, building, description, subtype);
+	auto b = std::make_shared<Bonus>(BonusDuration::PERMANENT, type, BonusSource::TOWN_STRUCTURE, val, building, subtype, description);
 
 	if(prop)
 		b->addPropagator(prop);

+ 4 - 3
lib/CTownHandler.h

@@ -392,14 +392,15 @@ class DLL_LINKAGE CTownHandler : public CHandlerBase<FactionID, Faction, CFactio
 	void loadBuilding(CTown * town, const std::string & stringID, const JsonNode & source);
 	void loadBuildings(CTown * town, const JsonNode & source);
 
-	std::shared_ptr<Bonus> createBonus(CBuilding * build, BonusType type, int val, int subtype = -1) const;
-	std::shared_ptr<Bonus> createBonus(CBuilding * build, BonusType type, int val, TPropagatorPtr & prop, int subtype = -1) const;
+	std::shared_ptr<Bonus> createBonus(CBuilding * build, BonusType type, int val) const;
+	std::shared_ptr<Bonus> createBonus(CBuilding * build, BonusType type, int val, TBonusSubtype subtype) const;
+	std::shared_ptr<Bonus> createBonus(CBuilding * build, BonusType type, int val, TBonusSubtype subtype, TPropagatorPtr & prop) const;
 	std::shared_ptr<Bonus> createBonusImpl(const BuildingID & building,
 												  BonusType type,
 												  int val,
 												  TPropagatorPtr & prop,
 												  const std::string & description,
-												  int subtype = -1) const;
+												  TBonusSubtype subtype) const;
 
 	/// loads CStructure's into town
 	void loadStructure(CTown & town, const std::string & stringID, const JsonNode & source) const;

+ 37 - 61
lib/JsonNode.cpp

@@ -417,13 +417,31 @@ std::string JsonNode::toJson(bool compact) const
 
 ///JsonUtils
 
-void JsonUtils::parseTypedBonusShort(const JsonVector & source, const std::shared_ptr<Bonus> & dest)
+static void loadBonusSubtype(TBonusSubtype & subtype, BonusType type, const JsonNode & node)
 {
-	dest->val = static_cast<si32>(source[1].Float());
-	resolveIdentifier(source[2],dest->subtype);
-	dest->additionalInfo = static_cast<si32>(source[3].Float());
-	dest->duration = BonusDuration::PERMANENT; //TODO: handle flags (as integer)
-	dest->turnsRemain = 0;
+	if (node.isNull())
+	{
+		subtype = TBonusSubtype::NONE;
+		return;
+	}
+
+	if (!node.isString())
+	{
+		logMod->warn("Bonus subtype must be string!");
+		subtype = TBonusSubtype::NONE;
+		return;
+	}
+
+	VLC->identifiers()->requestIdentifier(node, [&subtype, node](int32_t identifier)
+	{
+		assert(0); //TODO
+		subtype = TBonusSubtype("type", node.String(), identifier);
+	});
+}
+
+static void loadBonusSourceInstance(int32_t & sourceInstance, BonusSource sourceType, const JsonNode & node)
+{
+
 }
 
 std::shared_ptr<Bonus> JsonUtils::parseBonus(const JsonVector & ability_vec)
@@ -438,7 +456,11 @@ std::shared_ptr<Bonus> JsonUtils::parseBonus(const JsonVector & ability_vec)
 	}
 	b->type = it->second;
 
-	parseTypedBonusShort(ability_vec, b);
+	b->val = static_cast<si32>(ability_vec[1].Float());
+	loadBonusSubtype(b->subtype, b->type, ability_vec[2]);
+	b->additionalInfo = static_cast<si32>(ability_vec[3].Float());
+	b->duration = BonusDuration::PERMANENT; //TODO: handle flags (as integer)
+	b->turnsRemain = 0;
 	return b;
 }
 
@@ -473,31 +495,6 @@ const T parseByMapN(const std::map<std::string, T> & map, const JsonNode * val,
 		return parseByMap<T>(map, val, err);
 }
 
-void JsonUtils::resolveIdentifier(si32 & var, const JsonNode & node, const std::string & name)
-{
-	const JsonNode &value = node[name];
-	if (!value.isNull())
-	{
-		switch (value.getType())
-		{
-			case JsonNode::JsonType::DATA_INTEGER:
-				var = static_cast<si32>(value.Integer());
-				break;
-			case JsonNode::JsonType::DATA_FLOAT:
-				var = static_cast<si32>(value.Float());
-				break;
-			case JsonNode::JsonType::DATA_STRING:
-				VLC->identifiers()->requestIdentifier(value, [&](si32 identifier)
-				{
-					var = identifier;
-				});
-				break;
-			default:
-				logMod->error("Error! Wrong identifier used for value of %s.", name);
-		}
-	}
-}
-
 void JsonUtils::resolveAddInfo(CAddInfo & var, const JsonNode & node)
 {
 	const JsonNode & value = node["addInfo"];
@@ -549,27 +546,6 @@ void JsonUtils::resolveAddInfo(CAddInfo & var, const JsonNode & node)
 	}
 }
 
-void JsonUtils::resolveIdentifier(const JsonNode &node, si32 &var)
-{
-	switch (node.getType())
-	{
-		case JsonNode::JsonType::DATA_INTEGER:
-			var = static_cast<si32>(node.Integer());
-			break;
-		case JsonNode::JsonType::DATA_FLOAT:
-			var = static_cast<si32>(node.Float());
-			break;
-		case JsonNode::JsonType::DATA_STRING:
-			VLC->identifiers()->requestIdentifier(node, [&](si32 identifier)
-			{
-				var = identifier;
-			});
-			break;
-		default:
-			logMod->error("Error! Wrong identifier used for identifier!");
-	}
-}
-
 std::shared_ptr<ILimiter> JsonUtils::parseLimiter(const JsonNode & limiter)
 {
 	switch(limiter.getType())
@@ -660,7 +636,7 @@ std::shared_ptr<ILimiter> JsonUtils::parseLimiter(const JsonNode & limiter)
 								bonusLimiter->source = sourceIt->second;
 								bonusLimiter->isSourceRelevant = true;
 								if(!parameter["id"].isNull()) {
-									resolveIdentifier(parameter["id"], bonusLimiter->sid);
+									loadBonusSourceInstance(bonusLimiter->sid, bonusLimiter->source, parameter["id"]);
 									bonusLimiter->isSourceIDRelevant = true;
 								}
 							}
@@ -673,7 +649,7 @@ std::shared_ptr<ILimiter> JsonUtils::parseLimiter(const JsonNode & limiter)
 							return bonusLimiter;
 						else
 						{
-							resolveIdentifier(parameters[1], bonusLimiter->subtype);
+							loadBonusSubtype(bonusLimiter->subtype, bonusLimiter->type, parameters[1]);
 							bonusLimiter->isSubtypeRelevant = true;
 							if(parameters.size() > 2)
 								findSource(parameters[2]);
@@ -765,7 +741,7 @@ std::shared_ptr<Bonus> JsonUtils::parseBuildingBonus(const JsonNode & ability, c
 		source = BonusSource::TOWN_STRUCTURE
 		bonusType, val, subtype - get from json
 	*/
-	auto b = std::make_shared<Bonus>(BonusDuration::PERMANENT, BonusType::NONE, BonusSource::TOWN_STRUCTURE, 0, building, description, -1);
+	auto b = std::make_shared<Bonus>(BonusDuration::PERMANENT, BonusType::NONE, BonusSource::TOWN_STRUCTURE, 0, building, description);
 
 	if(!parseBonus(ability, b.get()))
 		return nullptr;
@@ -865,7 +841,7 @@ bool JsonUtils::parseBonus(const JsonNode &ability, Bonus *b)
 	else
 		b->type = it->second;
 
-	resolveIdentifier(b->subtype, params->isConverted ? params->toJson() : ability, "subtype");
+	loadBonusSubtype(b->subtype, b->type, params->isConverted ? params->toJson() : ability);
 
 	if(!params->isConverted)
 	{
@@ -996,7 +972,8 @@ CSelector JsonUtils::parseSelector(const JsonNode & ability)
 	if(!value->isNull())
 	{
 		TBonusSubtype subtype;
-		resolveIdentifier(subtype, ability, "subtype");
+		assert(0); //TODO
+		loadBonusSubtype(subtype, BonusType::NONE, ability);
 		ret = ret.And(Selector::subtype()(subtype));
 	}
 	value = &ability["sourceType"];
@@ -1010,10 +987,9 @@ CSelector JsonUtils::parseSelector(const JsonNode & ability)
 	}
 
 	value = &ability["sourceID"];
-	if(!value->isNull())
+	if(!value->isNull() && src.has_value())
 	{
-		id = -1;
-		resolveIdentifier(*id, ability, "sourceID");
+		loadBonusSourceInstance(*id, *src, ability);
 	}
 
 	if(src && id)

+ 0 - 9
lib/JsonNode.h

@@ -127,21 +127,12 @@ public:
 
 namespace JsonUtils
 {
-	/**
-	 * @brief parse short bonus format, excluding type
-	 * @note sets duration to Permament
-	 */
-	DLL_LINKAGE void parseTypedBonusShort(const JsonVector & source, const std::shared_ptr<Bonus> & dest);
-
-	///
 	DLL_LINKAGE std::shared_ptr<Bonus> parseBonus(const JsonVector & ability_vec);
 	DLL_LINKAGE std::shared_ptr<Bonus> parseBonus(const JsonNode & ability);
 	DLL_LINKAGE std::shared_ptr<Bonus> parseBuildingBonus(const JsonNode & ability, const BuildingID & building, const std::string & description);
 	DLL_LINKAGE bool parseBonus(const JsonNode & ability, Bonus * placement);
 	DLL_LINKAGE std::shared_ptr<ILimiter> parseLimiter(const JsonNode & limiter);
 	DLL_LINKAGE CSelector parseSelector(const JsonNode &ability);
-	DLL_LINKAGE void resolveIdentifier(si32 & var, const JsonNode & node, const std::string & name);
-	DLL_LINKAGE void resolveIdentifier(const JsonNode & node, si32 & var);
 	DLL_LINKAGE void resolveAddInfo(CAddInfo & var, const JsonNode & node);
 
 	/**

+ 3 - 3
lib/battle/BattleInfo.cpp

@@ -442,9 +442,9 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 	//native terrain bonuses
 	static auto nativeTerrain = std::make_shared<CreatureTerrainLimiter>();
 	
-	curB->addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::STACKS_SPEED, BonusSource::TERRAIN_NATIVE, 1, 0, 0)->addLimiter(nativeTerrain));
-	curB->addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::PRIMARY_SKILL, BonusSource::TERRAIN_NATIVE, 1, 0, static_cast<int>(PrimarySkill::ATTACK))->addLimiter(nativeTerrain));
-	curB->addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::PRIMARY_SKILL, BonusSource::TERRAIN_NATIVE, 1, 0, static_cast<int>(PrimarySkill::DEFENSE))->addLimiter(nativeTerrain));
+	curB->addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::STACKS_SPEED, BonusSource::TERRAIN_NATIVE, 1, 0)->addLimiter(nativeTerrain));
+	curB->addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::PRIMARY_SKILL, BonusSource::TERRAIN_NATIVE, 1, 0, TBonusSubtype(PrimarySkill::ATTACK))->addLimiter(nativeTerrain));
+	curB->addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::PRIMARY_SKILL, BonusSource::TERRAIN_NATIVE, 1, 0, TBonusSubtype(PrimarySkill::DEFENSE))->addLimiter(nativeTerrain));
 	//////////////////////////////////////////////////////////////////////////
 
 	//tactics

+ 2 - 2
lib/battle/CBattleInfoCallback.cpp

@@ -1755,7 +1755,7 @@ SpellID CBattleInfoCallback::getRandomCastedSpell(CRandomGenerator & rand,const
 		return SpellID::NONE;
 
 	if(bl->size() == 1)
-		return SpellID(bl->front()->subtype);
+		return bl->front()->subtype.as<SpellID>();
 
 	int totalWeight = 0;
 	for(const auto & b : *bl)
@@ -1772,7 +1772,7 @@ SpellID CBattleInfoCallback::getRandomCastedSpell(CRandomGenerator & rand,const
 		randomPos -= std::max(b->additionalInfo[0], 0);
 		if(randomPos < 0)
 		{
-			return SpellID(b->subtype);
+			return b->subtype.as<SpellID>();
 		}
 	}
 

+ 6 - 6
lib/battle/CUnitState.cpp

@@ -340,10 +340,10 @@ CUnitState::CUnitState():
 	health(this),
 	shots(this),
 	totalAttacks(this, Selector::type()(BonusType::ADDITIONAL_ATTACK), 1),
-	minDamage(this, Selector::typeSubtype(BonusType::CREATURE_DAMAGE, 0).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, 1)), 0),
-	maxDamage(this, Selector::typeSubtype(BonusType::CREATURE_DAMAGE, 0).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, 2)), 0),
-	attack(this, Selector::typeSubtype(BonusType::PRIMARY_SKILL, static_cast<int>(PrimarySkill::ATTACK)), 0),
-	defence(this, Selector::typeSubtype(BonusType::PRIMARY_SKILL, static_cast<int>(PrimarySkill::DEFENSE)), 0),
+	minDamage(this, Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageBoth).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageMin)), 0),
+	maxDamage(this, Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageBoth).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusSubtypes::creatureDamageMax)), 0),
+	attack(this, Selector::typeSubtype(BonusType::PRIMARY_SKILL, TBonusSubtype(PrimarySkill::ATTACK)), 0),
+	defence(this, Selector::typeSubtype(BonusType::PRIMARY_SKILL, TBonusSubtype(PrimarySkill::DEFENSE)), 0),
 	inFrenzy(this, Selector::type()(BonusType::IN_FRENZY)),
 	cloneLifetimeMarker(this, Selector::type()(BonusType::NONE).And(Selector::source(BonusSource::SPELL_EFFECT, SpellID::CLONE))),
 	cloneID(-1)
@@ -430,7 +430,7 @@ const CGHeroInstance * CUnitState::getHeroCaster() const
 
 int32_t CUnitState::getSpellSchoolLevel(const spells::Spell * spell, int32_t * outSelectedSchool) const
 {
-	int32_t skill = valOfBonuses(Selector::typeSubtype(BonusType::SPELLCASTER, spell->getIndex()));
+	int32_t skill = valOfBonuses(Selector::typeSubtype(BonusType::SPELLCASTER, TBonusSubtype(spell->getId())));
 	vstd::abetween(skill, 0, 3);
 	return skill;
 }
@@ -466,7 +466,7 @@ int32_t CUnitState::getEnchantPower(const spells::Spell * spell) const
 
 int64_t CUnitState::getEffectValue(const spells::Spell * spell) const
 {
-	return static_cast<int64_t>(getCount()) * valOfBonuses(BonusType::SPECIFIC_SPELL_POWER, spell->getIndex());
+	return static_cast<int64_t>(getCount()) * valOfBonuses(BonusType::SPECIFIC_SPELL_POWER, TBonusSubtype(spell->getId()));
 }
 
 PlayerColor CUnitState::getCasterOwner() const

+ 13 - 12
lib/battle/DamageCalculator.cpp

@@ -52,7 +52,7 @@ DamageRange DamageCalculator::getBaseDamageSingle() const
 	{
 		auto retrieveHeroPrimSkill = [&](PrimarySkill skill) -> int
 		{
-			std::shared_ptr<const Bonus> b = info.attacker->getBonus(Selector::sourceTypeSel(BonusSource::HERO_BASE_SKILL).And(Selector::typeSubtype(BonusType::PRIMARY_SKILL, static_cast<int>(skill))));
+			std::shared_ptr<const Bonus> b = info.attacker->getBonus(Selector::sourceTypeSel(BonusSource::HERO_BASE_SKILL).And(Selector::typeSubtype(BonusType::PRIMARY_SKILL, TBonusSubtype(skill))));
 			return b ? b->val : 0;
 		};
 
@@ -142,8 +142,9 @@ int DamageCalculator::getActorAttackSlayer() const
 
 		if(isAffected)
 		{
-			int attackBonus = SpellID(SpellID::SLAYER).toSpell()->getLevelPower(spLevel);
-			if(info.attacker->hasBonusOfType(BonusType::SPECIAL_PECULIAR_ENCHANT, SpellID::SLAYER))
+			SpellID spell(SpellID::SLAYER);
+			int attackBonus = spell.toSpell()->getLevelPower(spLevel);
+			if(info.attacker->hasBonusOfType(BonusType::SPECIAL_PECULIAR_ENCHANT, TBonusSubtype(spell)))
 			{
 				ui8 attackerTier = info.attacker->unitType()->getLevel();
 				ui8 specialtyBonus = std::max(5 - attackerTier, 0);
@@ -205,11 +206,11 @@ double DamageCalculator::getAttackOffenseArcheryFactor() const
 	if(info.shooting)
 	{
 		const std::string cachingStrArchery = "type_PERCENTAGE_DAMAGE_BOOSTs_1";
-		static const auto selectorArchery = Selector::typeSubtype(BonusType::PERCENTAGE_DAMAGE_BOOST, 1);
+		static const auto selectorArchery = Selector::typeSubtype(BonusType::PERCENTAGE_DAMAGE_BOOST, BonusSubtypes::damageTypeRanged);
 		return info.attacker->valOfBonuses(selectorArchery, cachingStrArchery) / 100.0;
 	}
 	const std::string cachingStrOffence = "type_PERCENTAGE_DAMAGE_BOOSTs_0";
-	static const auto selectorOffence = Selector::typeSubtype(BonusType::PERCENTAGE_DAMAGE_BOOST, 0);
+	static const auto selectorOffence = Selector::typeSubtype(BonusType::PERCENTAGE_DAMAGE_BOOST, BonusSubtypes::damageTypeMelee);
 	return info.attacker->valOfBonuses(selectorOffence, cachingStrOffence) / 100.0;
 }
 
@@ -231,7 +232,7 @@ double DamageCalculator::getAttackDoubleDamageFactor() const
 {
 	if(info.doubleDamage) {
 		const auto cachingStr = "type_BONUS_DAMAGE_PERCENTAGEs_" + std::to_string(info.attacker->creatureIndex());
-		const auto selector = Selector::typeSubtype(BonusType::BONUS_DAMAGE_PERCENTAGE, info.attacker->creatureIndex());
+		const auto selector = Selector::typeSubtype(BonusType::BONUS_DAMAGE_PERCENTAGE, TBonusSubtype(info.attacker->creatureId()));
 		return info.attacker->valOfBonuses(selector, cachingStr) / 100.0;
 	}
 	return 0.0;
@@ -259,7 +260,7 @@ double DamageCalculator::getAttackHateFactor() const
 
 	auto allHateEffects = info.attacker->getBonuses(selectorHate, cachingStrHate);
 
-	return allHateEffects->valOfBonuses(Selector::subtype()(info.defender->creatureIndex())) / 100.0;
+	return allHateEffects->valOfBonuses(Selector::subtype()(TBonusSubtype(info.defender->creatureId()))) / 100.0;
 }
 
 double DamageCalculator::getDefenseSkillFactor() const
@@ -281,7 +282,7 @@ double DamageCalculator::getDefenseSkillFactor() const
 double DamageCalculator::getDefenseArmorerFactor() const
 {
 	const std::string cachingStrArmorer = "type_GENERAL_DAMAGE_REDUCTIONs_N1_NsrcSPELL_EFFECT";
-	static const auto selectorArmorer = Selector::typeSubtype(BonusType::GENERAL_DAMAGE_REDUCTION, -1).And(Selector::sourceTypeSel(BonusSource::SPELL_EFFECT).Not());
+	static const auto selectorArmorer = Selector::typeSubtype(BonusType::GENERAL_DAMAGE_REDUCTION, BonusSubtypes::damageTypeAll).And(Selector::sourceTypeSel(BonusSource::SPELL_EFFECT).Not());
 	return info.defender->valOfBonuses(selectorArmorer, cachingStrArmorer) / 100.0;
 
 }
@@ -289,10 +290,10 @@ double DamageCalculator::getDefenseArmorerFactor() const
 double DamageCalculator::getDefenseMagicShieldFactor() const
 {
 	const std::string cachingStrMeleeReduction = "type_GENERAL_DAMAGE_REDUCTIONs_0";
-	static const auto selectorMeleeReduction = Selector::typeSubtype(BonusType::GENERAL_DAMAGE_REDUCTION, 0);
+	static const auto selectorMeleeReduction = Selector::typeSubtype(BonusType::GENERAL_DAMAGE_REDUCTION, BonusSubtypes::damageTypeMelee);
 
 	const std::string cachingStrRangedReduction = "type_GENERAL_DAMAGE_REDUCTIONs_1";
-	static const auto selectorRangedReduction = Selector::typeSubtype(BonusType::GENERAL_DAMAGE_REDUCTION, 1);
+	static const auto selectorRangedReduction = Selector::typeSubtype(BonusType::GENERAL_DAMAGE_REDUCTION, BonusSubtypes::damageTypeRanged);
 
 	//handling spell effects - shield and air shield
 	if(info.shooting)
@@ -313,7 +314,7 @@ double DamageCalculator::getDefenseRangePenaltiesFactor() const
 		{
 			return bonus->source == BonusSource::SPELL_EFFECT
 					&& bonus->sid == SpellID::AIR_SHIELD
-					&& bonus->val >= SecSkillLevel::ADVANCED;
+					&& bonus->val >= MasteryLevel::ADVANCED;
 		};
 
 		const bool distPenalty = callback.battleHasDistancePenalty(info.attacker, attackerPos, defenderPos);
@@ -386,7 +387,7 @@ double DamageCalculator::getDefensePetrificationFactor() const
 {
 	// Creatures that are petrified by a Basilisk's Petrifying attack or a Medusa's Stone gaze take 50% damage (R8 = 0.50) from ranged and melee attacks. Taking damage also deactivates the effect.
 	const std::string cachingStrAllReduction = "type_GENERAL_DAMAGE_REDUCTIONs_N1_srcSPELL_EFFECT";
-	static const auto selectorAllReduction = Selector::typeSubtype(BonusType::GENERAL_DAMAGE_REDUCTION, -1).And(Selector::sourceTypeSel(BonusSource::SPELL_EFFECT));
+	static const auto selectorAllReduction = Selector::typeSubtype(BonusType::GENERAL_DAMAGE_REDUCTION, BonusSubtypes::damageTypeAll).And(Selector::sourceTypeSel(BonusSource::SPELL_EFFECT));
 
 	return info.defender->valOfBonuses(selectorAllReduction, cachingStrAllReduction) / 100.0;
 }

+ 5 - 28
lib/bonuses/Bonus.cpp

@@ -134,29 +134,6 @@ std::string Bonus::Description(std::optional<si32> customValue) const
 	return str.str();
 }
 
-static JsonNode subtypeToJson(BonusType type, int subtype)
-{
-	switch(type)
-	{
-	case BonusType::PRIMARY_SKILL:
-		return JsonUtils::stringNode("primSkill." + NPrimarySkill::names[subtype]);
-	case BonusType::SPECIAL_SPELL_LEV:
-	case BonusType::SPECIFIC_SPELL_DAMAGE:
-	case BonusType::SPELL:
-	case BonusType::SPECIAL_PECULIAR_ENCHANT:
-	case BonusType::SPECIAL_ADD_VALUE_ENCHANT:
-	case BonusType::SPECIAL_FIXED_VALUE_ENCHANT:
-		return JsonUtils::stringNode(ModUtility::makeFullIdentifier("", "spell", SpellID::encode(subtype)));
-	case BonusType::IMPROVED_NECROMANCY:
-	case BonusType::SPECIAL_UPGRADE:
-		return JsonUtils::stringNode(ModUtility::makeFullIdentifier("", "creature", CreatureID::encode(subtype)));
-	case BonusType::GENERATE_RESOURCE:
-		return JsonUtils::stringNode("resource." + GameConstants::RESOURCE_NAMES[subtype]);
-	default:
-		return JsonUtils::intNode(subtype);
-	}
-}
-
 static JsonNode additionalInfoToJson(BonusType type, CAddInfo addInfo)
 {
 	switch(type)
@@ -173,8 +150,8 @@ JsonNode Bonus::toJsonNode() const
 	JsonNode root(JsonNode::JsonType::DATA_STRUCT);
 	// only add values that might reasonably be found in config files
 	root["type"].String() = vstd::findKey(bonusNameMap, type);
-	if(subtype != -1)
-		root["subtype"] = subtypeToJson(type, subtype);
+	if(subtype != TBonusSubtype::NONE)
+		root["subtype"].String() = subtype.toString();
 	if(additionalInfo != CAddInfo::NONE)
 		root["addInfo"] = additionalInfoToJson(type, additionalInfo);
 	if(source != BonusSource::OTHER)
@@ -206,7 +183,7 @@ JsonNode Bonus::toJsonNode() const
 	return root;
 }
 
-Bonus::Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, ui32 ID, std::string Desc, si32 Subtype):
+Bonus::Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, ui32 ID, TBonusSubtype Subtype, std::string Desc):
 	duration(Duration),
 	type(Type),
 	subtype(Subtype),
@@ -219,7 +196,7 @@ Bonus::Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32
 	targetSourceType = BonusSource::OTHER;
 }
 
-Bonus::Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, ui32 ID, si32 Subtype, BonusValueType ValType):
+Bonus::Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, ui32 ID, TBonusSubtype Subtype, BonusValueType ValType):
 	duration(Duration),
 	type(Type),
 	subtype(Subtype),
@@ -247,7 +224,7 @@ DLL_LINKAGE std::ostream & operator<<(std::ostream &out, const Bonus &bonus)
 
 #define printField(field) out << "\t" #field ": " << (int)bonus.field << "\n"
 	printField(val);
-	printField(subtype);
+	out << "\tSubtype: " << bonus.subtype.toString() << "\n";
 	printField(duration.to_ulong());
 	printField(source);
 	printField(sid);

+ 43 - 4
lib/bonuses/Bonus.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include "BonusEnum.h"
+#include "../constants/EntityIdentifiers.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -22,13 +23,48 @@ class IUpdater;
 class BonusList;
 class CSelector;
 
-using TBonusSubtype = int32_t;
+using TBonusSubtype = MetaIdentifier;
 using TBonusListPtr = std::shared_ptr<BonusList>;
 using TConstBonusListPtr = std::shared_ptr<const BonusList>;
 using TLimiterPtr = std::shared_ptr<ILimiter>;
 using TPropagatorPtr = std::shared_ptr<IPropagator>;
 using TUpdaterPtr = std::shared_ptr<IUpdater>;
 
+namespace BonusSubtypes
+{
+
+static const TBonusSubtype creatureDamageBoth; // 0
+static const TBonusSubtype creatureDamageMin;  // 1
+static const TBonusSubtype creatureDamageMax;  // 2
+
+static const TBonusSubtype damageTypeAll;    // -1
+static const TBonusSubtype damageTypeMelee;  // 0
+static const TBonusSubtype damageTypeRanged; // 1
+
+static const TBonusSubtype heroMovementLand; // 1
+static const TBonusSubtype heroMovementSea; // 0
+
+static const TBonusSubtype heroMovementPenalty; // 2
+static const TBonusSubtype heroMovementFull; // 1
+
+static const TBonusSubtype deathStareGorgon; // 0
+static const TBonusSubtype deathStareCommander;
+
+static const TBonusSubtype rebirthRegular; // 0
+static const TBonusSubtype rebirthSpecial; // 1
+
+static const TBonusSubtype visionsMonsters; // 0
+static const TBonusSubtype visionsHeroes;  // 1
+static const TBonusSubtype visionsTowns;  // 2
+
+static const TBonusSubtype immunityBattleWide; // 0
+static const TBonusSubtype immunityEnemyHero; // 1
+
+TBonusSubtype spellLevel(int level);
+TBonusSubtype creatureLevel(int level);
+
+}
+
 class DLL_LINKAGE CAddInfo : public std::vector<si32>
 {
 public:
@@ -56,7 +92,7 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 	si16 turnsRemain = 0; //used if duration is N_TURNS, N_DAYS or ONE_WEEK
 
 	BonusType type = BonusType::NONE; //uses BonusType values - says to what is this bonus - 1 byte
-	TBonusSubtype subtype = -1; //-1 if not applicable - 4 bytes
+	TBonusSubtype subtype;
 
 	BonusSource source = BonusSource::OTHER; //source type" uses BonusSource values - what gave that bonus
 	BonusSource targetSourceType;//Bonuses of what origin this amplifies, uses BonusSource values. Needed for PERCENT_TO_TARGET_TYPE.
@@ -75,8 +111,11 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 
 	std::string description;
 
-	Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, ui32 ID, std::string Desc, si32 Subtype=-1);
-	Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, ui32 ID, si32 Subtype=-1, BonusValueType ValType = BonusValueType::ADDITIVE_VALUE);
+	Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, ui32 sourceID);
+	Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, ui32 sourceID, std::string Desc);
+	Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, ui32 sourceID, TBonusSubtype subtype);
+	Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, ui32 sourceID, TBonusSubtype subtype, std::string Desc);
+	Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, ui32 sourceID, TBonusSubtype subtype, BonusValueType ValType);
 	Bonus() = default;
 
 	template <typename Handler> void serialize(Handler &h, const int version)

+ 34 - 41
lib/bonuses/BonusParams.cpp

@@ -81,76 +81,76 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 		else if(deprecatedSubtype == SecondarySkill::SORCERY || deprecatedSubtypeStr == "skill.sorcery")
 		{
 			type = BonusType::SPELL_DAMAGE;
-			subtype = SpellSchool(ESpellSchool::ANY);
+			subtype = TBonusSubtype(ESpellSchool::ANY);
 		}
 		else if(deprecatedSubtype == SecondarySkill::SCHOLAR || deprecatedSubtypeStr == "skill.scholar")
 			type = BonusType::LEARN_MEETING_SPELL_LIMIT;
 		else if(deprecatedSubtype == SecondarySkill::ARCHERY|| deprecatedSubtypeStr == "skill.archery")
 		{
-			subtype = 1;
+			subtype = BonusSubtypes::damageTypeRanged;
 			type = BonusType::PERCENTAGE_DAMAGE_BOOST;
 		}
 		else if(deprecatedSubtype == SecondarySkill::OFFENCE || deprecatedSubtypeStr == "skill.offence")
 		{
-			subtype = 0;
+			subtype = BonusSubtypes::damageTypeMelee;
 			type = BonusType::PERCENTAGE_DAMAGE_BOOST;
 		}
 		else if(deprecatedSubtype == SecondarySkill::ARMORER || deprecatedSubtypeStr == "skill.armorer")
 		{
-			subtype = -1;
+			subtype = BonusSubtypes::damageTypeAll;
 			type = BonusType::GENERAL_DAMAGE_REDUCTION;
 		}
 		else if(deprecatedSubtype == SecondarySkill::NAVIGATION || deprecatedSubtypeStr == "skill.navigation")
 		{
-			subtype = 0;
+			subtype = BonusSubtypes::heroMovementSea;
 			valueType = BonusValueType::PERCENT_TO_BASE;
 			type = BonusType::MOVEMENT;
 		}
 		else if(deprecatedSubtype == SecondarySkill::LOGISTICS || deprecatedSubtypeStr == "skill.logistics")
 		{
-			subtype = 1;
+			subtype = BonusSubtypes::heroMovementLand;
 			valueType = BonusValueType::PERCENT_TO_BASE;
 			type = BonusType::MOVEMENT;
 		}
 		else if(deprecatedSubtype == SecondarySkill::ESTATES || deprecatedSubtypeStr == "skill.estates")
 		{
 			type = BonusType::GENERATE_RESOURCE;
-			subtype = GameResID(EGameResID::GOLD);
+			subtype = TBonusSubtype(GameResID(EGameResID::GOLD));
 		}
 		else if(deprecatedSubtype == SecondarySkill::AIR_MAGIC || deprecatedSubtypeStr == "skill.airMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtype = SpellSchool(ESpellSchool::AIR);
+			subtype = TBonusSubtype(ESpellSchool::AIR);
 		}
 		else if(deprecatedSubtype == SecondarySkill::WATER_MAGIC || deprecatedSubtypeStr == "skill.waterMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtype = SpellSchool(ESpellSchool::WATER);
+			subtype = TBonusSubtype(ESpellSchool::WATER);
 		}
 		else if(deprecatedSubtype == SecondarySkill::FIRE_MAGIC || deprecatedSubtypeStr == "skill.fireMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtype = SpellSchool(ESpellSchool::FIRE);
+			subtype = TBonusSubtype(ESpellSchool::FIRE);
 		}
 		else if(deprecatedSubtype == SecondarySkill::EARTH_MAGIC || deprecatedSubtypeStr == "skill.earthMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtype = SpellSchool(ESpellSchool::EARTH);
+			subtype = TBonusSubtype(ESpellSchool::EARTH);
 		}
 		else if (deprecatedSubtype == SecondarySkill::ARTILLERY || deprecatedSubtypeStr == "skill.artillery")
 		{
 			type = BonusType::BONUS_DAMAGE_CHANCE;
-			subtypeStr = "core:creature.ballista";
+			subtype = TBonusSubtype(CreatureID(CreatureID::BALLISTA));
 		}
 		else if (deprecatedSubtype == SecondarySkill::FIRST_AID || deprecatedSubtypeStr == "skill.firstAid")
 		{
 			type = BonusType::SPECIFIC_SPELL_POWER;
-			subtypeStr = "core:spell.firstAid";
+			subtype = TBonusSubtype("spell", "firstAid");
 		}
 		else if (deprecatedSubtype == SecondarySkill::BALLISTICS || deprecatedSubtypeStr == "skill.ballistics")
 		{
 			type = BonusType::CATAPULT_EXTRA_SHOTS;
-			subtypeStr = "core:spell.catapultShot";
+			subtype = TBonusSubtype("spell", "catapultShot");
 		}
 		else
 			isConverted = false;
@@ -162,27 +162,27 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 		else if (deprecatedSubtype == SecondarySkill::ARTILLERY || deprecatedSubtypeStr == "skill.artillery")
 		{
 			type = BonusType::HERO_GRANTS_ATTACKS;
-			subtypeStr = "core:creature.ballista";
+			subtype = TBonusSubtype(CreatureID(CreatureID::BALLISTA));
 		}
 		else
 			isConverted = false;
 	}
 	else if (deprecatedTypeStr == "SEA_MOVEMENT")
 	{
-		subtype = 0;
+		subtype = BonusSubtypes::heroMovementSea;
 		valueType = BonusValueType::ADDITIVE_VALUE;
 		type = BonusType::MOVEMENT;
 	}
 	else if (deprecatedTypeStr == "LAND_MOVEMENT")
 	{
-		subtype = 1;
+		subtype = BonusSubtypes::heroMovementLand;
 		valueType = BonusValueType::ADDITIVE_VALUE;
 		type = BonusType::MOVEMENT;
 	}
 	else if (deprecatedTypeStr == "MAXED_SPELL")
 	{
 		type = BonusType::SPELL;
-		subtypeStr = deprecatedSubtypeStr;
+		subtype = TBonusSubtype("spell", deprecatedSubtypeStr);
 		valueType = BonusValueType::INDEPENDENT_MAX;
 		val = 3;
 	}
@@ -223,52 +223,52 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 	else if (deprecatedTypeStr == "DIRECT_DAMAGE_IMMUNITY")
 	{
 		type = BonusType::SPELL_DAMAGE_REDUCTION;
-		subtype = SpellSchool(ESpellSchool::ANY);
+		subtype = MetaIdentifier(SpellSchool::ANY);
 		val = 100;
 	}
 	else if (deprecatedTypeStr == "AIR_SPELL_DMG_PREMY")
 	{
 		type = BonusType::SPELL_DAMAGE;
-		subtype = SpellSchool(ESpellSchool::AIR);
+		subtype = MetaIdentifier(SpellSchool::AIR);
 	}
 	else if (deprecatedTypeStr == "FIRE_SPELL_DMG_PREMY")
 	{
 		type = BonusType::SPELL_DAMAGE;
-		subtype = SpellSchool(ESpellSchool::FIRE);
+		subtype = MetaIdentifier(SpellSchool::FIRE);
 	}
 	else if (deprecatedTypeStr == "WATER_SPELL_DMG_PREMY")
 	{
 		type = BonusType::SPELL_DAMAGE;
-		subtype = SpellSchool(ESpellSchool::WATER);
+		subtype = MetaIdentifier(SpellSchool::WATER);
 	}
 	else if (deprecatedTypeStr == "EARTH_SPELL_DMG_PREMY")
 	{
 		type = BonusType::SPELL_DAMAGE;
-		subtype = SpellSchool(ESpellSchool::EARTH);
+		subtype = MetaIdentifier(SpellSchool::EARTH);
 	}
 	else if (deprecatedTypeStr == "AIR_SPELLS")
 	{
 		type = BonusType::SPELLS_OF_SCHOOL;
-		subtype = SpellSchool(ESpellSchool::AIR);
+		subtype = MetaIdentifier(SpellSchool::AIR);
 	}
 	else if (deprecatedTypeStr == "FIRE_SPELLS")
 	{
 		type = BonusType::SPELLS_OF_SCHOOL;
-		subtype = SpellSchool(ESpellSchool::FIRE);
+		subtype = MetaIdentifier(SpellSchool::FIRE);
 	}
 	else if (deprecatedTypeStr == "WATER_SPELLS")
 	{
 		type = BonusType::SPELLS_OF_SCHOOL;
-		subtype = SpellSchool(ESpellSchool::WATER);
+		subtype = MetaIdentifier(SpellSchool::WATER);
 	}
 	else if (deprecatedTypeStr == "EARTH_SPELLS")
 	{
 		type = BonusType::SPELLS_OF_SCHOOL;
-		subtype = SpellSchool(ESpellSchool::EARTH);
+		subtype = MetaIdentifier(SpellSchool::EARTH);
 	}
 	else if (deprecatedTypeStr == "AIR_IMMUNITY")
 	{
-		subtype = SpellSchool(ESpellSchool::AIR);
+		subtype = MetaIdentifier(SpellSchool::AIR);
 		switch(deprecatedSubtype)
 		{
 			case 0:
@@ -284,7 +284,7 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 	}
 	else if (deprecatedTypeStr == "FIRE_IMMUNITY")
 	{
-		subtype = SpellSchool(ESpellSchool::FIRE);
+		subtype = MetaIdentifier(SpellSchool::FIRE);
 		switch(deprecatedSubtype)
 		{
 			case 0:
@@ -300,7 +300,7 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 	}
 	else if (deprecatedTypeStr == "WATER_IMMUNITY")
 	{
-		subtype = SpellSchool(ESpellSchool::WATER);
+		subtype = MetaIdentifier(SpellSchool::WATER);
 		switch(deprecatedSubtype)
 		{
 			case 0:
@@ -316,7 +316,7 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 	}
 	else if (deprecatedTypeStr == "EARTH_IMMUNITY")
 	{
-		subtype = SpellSchool(ESpellSchool::EARTH);
+		subtype = MetaIdentifier(SpellSchool::EARTH);
 		switch(deprecatedSubtype)
 		{
 			case 0:
@@ -340,10 +340,8 @@ const JsonNode & BonusParams::toJson()
 	if(ret.isNull())
 	{
 		ret["type"].String() = vstd::findKey(bonusNameMap, type);
-		if(subtypeStr)
-			ret["subtype"].String() = *subtypeStr;
-		else if(subtype)
-			ret["subtype"].Integer() = *subtype;
+		if(subtype)
+			ret["subtype"].String() = subtype->toString();
 		if(valueType)
 			ret["valueType"].String() = vstd::findKey(bonusValueMap, *valueType);
 		if(val)
@@ -358,11 +356,6 @@ const JsonNode & BonusParams::toJson()
 CSelector BonusParams::toSelector()
 {
 	assert(isConverted);
-	if(subtypeStr)
-	{
-		subtype = -1;
-		JsonUtils::resolveIdentifier(*subtype, toJson(), "subtype");
-	}
 
 	auto ret = Selector::type()(type);
 	if(subtype)
@@ -374,4 +367,4 @@ CSelector BonusParams::toSelector()
 	return ret;
 }
 
-VCMI_LIB_NAMESPACE_END
+VCMI_LIB_NAMESPACE_END

+ 1 - 2
lib/bonuses/BonusParams.h

@@ -20,7 +20,6 @@ struct DLL_LINKAGE BonusParams {
 	bool isConverted;
 	BonusType type = BonusType::NONE;
 	std::optional<TBonusSubtype> subtype = std::nullopt;
-	std::optional<std::string> subtypeStr = std::nullopt;
 	std::optional<BonusValueType> valueType = std::nullopt;
 	std::optional<si32> val = std::nullopt;
 	std::optional<BonusSource> targetType = std::nullopt;
@@ -36,4 +35,4 @@ private:
 
 extern DLL_LINKAGE const std::set<std::string> deprecatedBonusSet;
 
-VCMI_LIB_NAMESPACE_END
+VCMI_LIB_NAMESPACE_END

+ 5 - 5
lib/bonuses/IBonusBearer.cpp

@@ -62,20 +62,20 @@ bool IBonusBearer::hasBonusOfType(BonusType type) const
 	return hasBonus(s, cachingStr);
 }
 
-int IBonusBearer::valOfBonuses(BonusType type, int subtype) const
+int IBonusBearer::valOfBonuses(BonusType type, TBonusSubtype subtype) const
 {
 	//This part is performance-critical
-	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + "_" + std::to_string(subtype);
+	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + "_" + subtype.toString();
 
 	CSelector s = Selector::typeSubtype(type, subtype);
 
 	return valOfBonuses(s, cachingStr);
 }
 
-bool IBonusBearer::hasBonusOfType(BonusType type, int subtype) const
+bool IBonusBearer::hasBonusOfType(BonusType type, TBonusSubtype subtype) const
 {
 	//This part is performance-critical
-	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + "_" + std::to_string(subtype);
+	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + "_" + subtype.toString();
 
 	CSelector s = Selector::typeSubtype(type, subtype);
 
@@ -95,4 +95,4 @@ std::shared_ptr<const Bonus> IBonusBearer::getBonus(const CSelector &selector) c
 	return bonuses->getFirst(Selector::all);
 }
 
-VCMI_LIB_NAMESPACE_END
+VCMI_LIB_NAMESPACE_END

+ 3 - 3
lib/bonuses/IBonusBearer.h

@@ -35,11 +35,11 @@ public:
 	//Optimized interface (with auto-caching)
 	int valOfBonuses(BonusType type) const; //subtype -> subtype of bonus;
 	bool hasBonusOfType(BonusType type) const;//determines if hero has a bonus of given type (and optionally subtype)
-	int valOfBonuses(BonusType type, int subtype) const; //subtype -> subtype of bonus;
-	bool hasBonusOfType(BonusType type, int subtype) const;//determines if hero has a bonus of given type (and optionally subtype)
+	int valOfBonuses(BonusType type, TBonusSubtype subtype) const; //subtype -> subtype of bonus;
+	bool hasBonusOfType(BonusType type, TBonusSubtype subtype) const;//determines if hero has a bonus of given type (and optionally subtype)
 	bool hasBonusFrom(BonusSource source, ui32 sourceID) const;
 
 	virtual int64_t getTreeVersion() const = 0;
 };
 
-VCMI_LIB_NAMESPACE_END
+VCMI_LIB_NAMESPACE_END

+ 4 - 4
lib/bonuses/Limiters.cpp

@@ -136,7 +136,7 @@ JsonNode CCreatureTypeLimiter::toJsonNode() const
 }
 
 HasAnotherBonusLimiter::HasAnotherBonusLimiter( BonusType bonus )
-	: type(bonus), subtype(0), isSubtypeRelevant(false), isSourceRelevant(false), isSourceIDRelevant(false)
+	: type(bonus), isSubtypeRelevant(false), isSourceRelevant(false), isSourceIDRelevant(false)
 {
 }
 
@@ -184,8 +184,8 @@ std::string HasAnotherBonusLimiter::toString() const
 	std::string typeName = vstd::findKey(bonusNameMap, type);
 	if(isSubtypeRelevant)
 	{
-		boost::format fmt("HasAnotherBonusLimiter(type=%s, subtype=%d)");
-		fmt % typeName % subtype;
+		boost::format fmt("HasAnotherBonusLimiter(type=%s, subtype=%s)");
+		fmt % typeName % subtype.toString();
 		return fmt.str();
 	}
 	else
@@ -205,7 +205,7 @@ JsonNode HasAnotherBonusLimiter::toJsonNode() const
 	root["type"].String() = "HAS_ANOTHER_BONUS_LIMITER";
 	root["parameters"].Vector().push_back(JsonUtils::stringNode(typeName));
 	if(isSubtypeRelevant)
-		root["parameters"].Vector().push_back(JsonUtils::intNode(subtype));
+		root["parameters"].Vector().push_back(JsonUtils::stringNode(subtype.toString()));
 	if(isSourceRelevant)
 		root["parameters"].Vector().push_back(JsonUtils::stringNode(sourceTypeName));
 

+ 108 - 0
lib/constants/EntityIdentifiers.h

@@ -349,6 +349,27 @@ public:
 	static std::string entityType();
 };
 
+class DLL_LINKAGE PrimarySkill : public Identifier<PrimarySkill>
+{
+public:
+	using Identifier<PrimarySkill>::Identifier;
+
+	static const PrimarySkill NONE;
+	static const PrimarySkill ATTACK;
+	static const PrimarySkill DEFENSE;
+	static const PrimarySkill SPELL_POWER;
+	static const PrimarySkill KNOWLEDGE;
+
+	static const PrimarySkill BEGIN;
+	static const PrimarySkill END;
+
+	static const PrimarySkill EXPERIENCE;
+
+	static si32 decode(const std::string& identifier);
+	static std::string encode(const si32 index);
+	static std::string entityType();
+};
+
 class DLL_LINKAGE FactionID : public Identifier<FactionID>
 {
 public:
@@ -919,6 +940,10 @@ public:
 	static const SpellSchool FIRE;
 	static const SpellSchool WATER;
 	static const SpellSchool EARTH;
+
+	DLL_LINKAGE static si32 decode(const std::string & identifier);
+	DLL_LINKAGE static std::string encode(const si32 index);
+	static std::string entityType();
 };
 
 class GameResIDBase : public IdentifierBase
@@ -946,6 +971,8 @@ class GameResID : public IdentifierWithEnum<GameResID, GameResIDBase>
 public:
 	using IdentifierWithEnum<GameResID, GameResIDBase>::IdentifierWithEnum;
 
+	DLL_LINKAGE static si32 decode(const std::string & identifier);
+	DLL_LINKAGE static std::string encode(const si32 index);
 	static std::string entityType();
 };
 
@@ -958,4 +985,85 @@ using River = RiverId;
 using Road = RoadId;
 using ETerrainId = TerrainId;
 
+/// This class represents field that may contain value of multiple different identifer types
+class MetaIdentifier
+{
+	std::string entityType;
+	std::string stringForm;
+	int32_t integerForm;
+
+	void onDeserialized();
+public:
+
+	static const MetaIdentifier NONE;
+
+	MetaIdentifier():
+		integerForm(-1)
+	{}
+
+	explicit MetaIdentifier(const std::string & entityType, const std::string & identifier)
+		: entityType(entityType)
+		, stringForm(identifier)
+		, integerForm(-1)
+	{
+		onDeserialized();
+	}
+
+	explicit MetaIdentifier(const std::string & entityType, const std::string & identifier, int32_t value)
+		: entityType(entityType)
+		, stringForm(identifier)
+		, integerForm(value)
+	{
+	}
+
+	template<typename IdentifierType>
+	explicit MetaIdentifier(const IdentifierType & identifier )
+		: entityType(IdentifierType::entityType())
+		, integerForm(identifier.getNum())
+		, stringForm(IdentifierType::encode(identifier.getNum()))
+	{
+		static_assert(std::is_base_of<IdentifierBase, IdentifierType>::value, "MetaIdentifier can only be constructed from Identifer class");
+	}
+
+	int32_t getNum() const
+	{
+		return integerForm;
+	}
+
+	std::string toString() const
+	{
+		return stringForm;
+	}
+
+	template<typename IdentifierType>
+	IdentifierType as() const
+	{
+		IdentifierType result(integerForm);
+		return result;
+	}
+
+	template <typename Handler> void serialize(Handler &h, const int version)
+	{
+		h & stringForm;
+
+		if (!h.saving)
+			onDeserialized();
+	}
+
+	bool operator == (const MetaIdentifier & other) const
+	{
+		assert( (stringForm == other.stringForm) ? (integerForm == other.integerForm) : true );
+
+		return stringForm == other.stringForm;
+	}
+
+	bool operator != (const MetaIdentifier & other) const
+	{
+		return !(*this == other);
+	}
+
+	bool operator < (const MetaIdentifier & other) const;
+};
+
+
 VCMI_LIB_NAMESPACE_END

+ 2 - 12
lib/constants/Enumerations.h

@@ -11,16 +11,6 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-enum class PrimarySkill : int8_t
-{
-	NONE = -1,
-	ATTACK,
-	DEFENSE,
-	SPELL_POWER,
-	KNOWLEDGE,
-	EXPERIENCE = 4 //for some reason changePrimSkill uses it
-};
-
 enum class EAlignment : int8_t
 {
 	GOOD,
@@ -143,9 +133,9 @@ enum class ETeleportChannelType : int8_t
 	MIXED
 };
 
-namespace SecSkillLevel
+namespace MasteryLevel
 {
-	enum SecSkillLevel
+	enum Type
 	{
 		NONE,
 		BASIC,

+ 1 - 1
lib/gameState/CGameState.cpp

@@ -1871,7 +1871,7 @@ struct statsHLP
 		//Heroes can produce gold as well - skill, specialty or arts
 		for(const auto & h : ps->heroes)
 		{
-			totalIncome += h->valOfBonuses(Selector::typeSubtype(BonusType::GENERATE_RESOURCE, GameResID(EGameResID::GOLD)));
+			totalIncome += h->valOfBonuses(Selector::typeSubtype(BonusType::GENERATE_RESOURCE, TBonusSubtype(GameResID(GameResID::GOLD))));
 
 			if(!heroOrTown)
 				heroOrTown = h;

+ 13 - 15
lib/gameState/CGameStateCampaign.cpp

@@ -84,10 +84,10 @@ void CGameStateCampaign::trimCrossoverHeroesParameters(std::vector<CampaignHeroR
 		//trimming prim skills
 		for(CGHeroInstance * cgh : crossoverHeroes)
 		{
-			for(int g=0; g<GameConstants::PRIMARY_SKILLS; ++g)
+			for(auto g = PrimarySkill::BEGIN; g < PrimarySkill::END; ++g)
 			{
 				auto sel = Selector::type()(BonusType::PRIMARY_SKILL)
-					.And(Selector::subtype()(g))
+					.And(Selector::subtype()(TBonusSubtype(g)))
 					.And(Selector::sourceType()(BonusSource::HERO_BASE_SKILL));
 
 				cgh->getBonusLocalFirst(sel)->val = cgh->type->heroClass->primarySkillInitial[g];
@@ -118,7 +118,7 @@ void CGameStateCampaign::trimCrossoverHeroesParameters(std::vector<CampaignHeroR
 		//trimming artifacts
 		for(CGHeroInstance * hero : crossoverHeroes)
 		{
-			auto const & checkAndRemoveArtifact = [&](const ArtifactPosition & artifactPosition )
+			const auto & checkAndRemoveArtifact = [&](const ArtifactPosition & artifactPosition)
 			{
 				if(artifactPosition == ArtifactPosition::SPELLBOOK)
 					return; // do not handle spellbook this way
@@ -141,7 +141,7 @@ void CGameStateCampaign::trimCrossoverHeroesParameters(std::vector<CampaignHeroR
 
 			// process on copy - removal of artifact will invalidate container
 			auto artifactsWorn = hero->artifactsWorn;
-			for (auto const & art : artifactsWorn)
+			for(const auto & art : artifactsWorn)
 				checkAndRemoveArtifact(art.first);
 
 			// process in reverse - removal of artifact will shift all artifacts after this one
@@ -308,16 +308,14 @@ void CGameStateCampaign::giveCampaignBonusToHero(CGHeroInstance * hero)
 		case CampaignBonusType::PRIMARY_SKILL:
 		{
 			const ui8 * ptr = reinterpret_cast<const ui8 *>(&curBonus->info2);
-			for(int g = 0; g < GameConstants::PRIMARY_SKILLS; ++g)
+			for(auto g = PrimarySkill::BEGIN; g < PrimarySkill::END; ++g)
 			{
-				int val = ptr[g];
+				int val = ptr[g.getNum()];
 				if(val == 0)
-				{
 					continue;
-				}
-				auto bb = std::make_shared<Bonus>(
-					BonusDuration::PERMANENT, BonusType::PRIMARY_SKILL, BonusSource::CAMPAIGN_BONUS, val, static_cast<int>(*gameState->scenarioOps->campState->currentScenario()), g
-				);
+
+				int currentScenario = static_cast<int>(*gameState->scenarioOps->campState->currentScenario());
+				auto bb = std::make_shared<Bonus>( BonusDuration::PERMANENT, BonusType::PRIMARY_SKILL, BonusSource::CAMPAIGN_BONUS, val, currentScenario, TBonusSubtype(g) );
 				hero->addNewBonus(bb);
 			}
 			break;
@@ -386,9 +384,9 @@ std::vector<CampaignHeroReplacement> CGameStateCampaign::generateCampaignHeroesT
 	}
 
 	//selecting heroes by type
-	for (auto const * placeholder : placeholdersByType)
+	for(const auto * placeholder : placeholdersByType)
 	{
-		auto const & node = campaignState->getHeroByType(*placeholder->heroType);
+		const auto & node = campaignState->getHeroByType(*placeholder->heroType);
 		if (node.isNull())
 		{
 			logGlobal->info("Hero crossover: Unable to replace placeholder for %d (%s)!", placeholder->heroType->getNum(), VLC->heroTypes()->getById(*placeholder->heroType)->getNameTranslated());
@@ -412,10 +410,10 @@ std::vector<CampaignHeroReplacement> CGameStateCampaign::generateCampaignHeroesT
 			return *a->powerRank > *b->powerRank;
 		});
 
-		auto const & nodeList = campaignState->getHeroesByPower(lastScenario.value());
+		const auto & nodeList = campaignState->getHeroesByPower(lastScenario.value());
 		auto nodeListIter = nodeList.begin();
 
-		for (auto const * placeholder : placeholdersByPower)
+		for(const auto * placeholder : placeholdersByPower)
 		{
 			if (nodeListIter == nodeList.end())
 				break;

+ 1 - 1
lib/mapObjects/CGCreature.cpp

@@ -46,7 +46,7 @@ std::string CGCreature::getHoverText(PlayerColor player) const
 std::string CGCreature::getHoverText(const CGHeroInstance * hero) const
 {
 	std::string hoverName;
-	if(hero->hasVisions(this, 0))
+	if(hero->hasVisions(this, BonusSubtypes::visionsMonsters))
 	{
 		MetaString ms;
 		ms.appendNumber(stacks.begin()->second->count);

+ 27 - 37
lib/mapObjects/CGHeroInstance.cpp

@@ -95,7 +95,7 @@ ui32 CGHeroInstance::getTileMovementCost(const TerrainTile & dest, const Terrain
 	}
 	else if(ti->nativeTerrain != from.terType->getId() &&//the terrain is not native
 			ti->nativeTerrain != ETerrainId::ANY_TERRAIN && //no special creature bonus
-			!ti->hasBonusOfType(BonusType::NO_TERRAIN_PENALTY, from.terType->getIndex())) //no special movement bonus
+			!ti->hasBonusOfType(BonusType::NO_TERRAIN_PENALTY, TBonusSubtype(from.terType->getId()))) //no special movement bonus
 	{
 
 		ret = VLC->terrainTypeHandler->getById(from.terType->getId())->moveCost;
@@ -249,14 +249,14 @@ void CGHeroInstance::updateArmyMovementBonus(bool onLand, const TurnInfo * ti) c
 		lowestCreatureSpeed = realLowestSpeed;
 		//Let updaters run again
 		treeHasChanged();
-		ti->updateHeroBonuses(BonusType::MOVEMENT, Selector::subtype()(!!onLand));
+		ti->updateHeroBonuses(BonusType::MOVEMENT, Selector::subtype()(onLand ? BonusSubtypes::heroMovementLand : BonusSubtypes::heroMovementSea));
 	}
 }
 
 int CGHeroInstance::movementPointsLimitCached(bool onLand, const TurnInfo * ti) const
 {
 	updateArmyMovementBonus(onLand, ti);
-	return ti->valOfBonuses(BonusType::MOVEMENT, !!onLand);
+	return ti->valOfBonuses(BonusType::MOVEMENT, onLand ? BonusSubtypes::heroMovementLand : BonusSubtypes::heroMovementSea);
 }
 
 CGHeroInstance::CGHeroInstance():
@@ -638,7 +638,7 @@ int32_t CGHeroInstance::getSpellSchoolLevel(const spells::Spell * spell, int32_t
 
 	spell->forEachSchool([&, this](const SpellSchool & cnf, bool & stop)
 	{
-		int32_t thisSchool = valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, cnf); //FIXME: Bonus shouldn't be additive (Witchking Artifacts : Crown of Skies)
+		int32_t thisSchool = valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, TBonusSubtype(cnf)); //FIXME: Bonus shouldn't be additive (Witchking Artifacts : Crown of Skies)
 		if(thisSchool > skill)
 		{
 			skill = thisSchool;
@@ -647,8 +647,8 @@ int32_t CGHeroInstance::getSpellSchoolLevel(const spells::Spell * spell, int32_t
 		}
 	});
 
-	vstd::amax(skill, valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, SpellSchool(ESpellSchool::ANY))); //any school bonus
-	vstd::amax(skill, valOfBonuses(BonusType::SPELL, spell->getIndex())); //given by artifact or other effect
+	vstd::amax(skill, valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, TBonusSubtype(SpellSchool::ANY))); //any school bonus
+	vstd::amax(skill, valOfBonuses(BonusType::SPELL, TBonusSubtype(spell->getId()))); //given by artifact or other effect
 
 	vstd::amax(skill, 0); //in case we don't know any school
 	vstd::amin(skill, 3);
@@ -660,28 +660,28 @@ int64_t CGHeroInstance::getSpellBonus(const spells::Spell * spell, int64_t base,
 	//applying sorcery secondary skill
 
 	if(spell->isMagical())
-		base = static_cast<int64_t>(base * (valOfBonuses(BonusType::SPELL_DAMAGE, SpellSchool(ESpellSchool::ANY))) / 100.0);
+		base = static_cast<int64_t>(base * (valOfBonuses(BonusType::SPELL_DAMAGE, TBonusSubtype(SpellSchool::ANY))) / 100.0);
 
-	base = static_cast<int64_t>(base * (100 + valOfBonuses(BonusType::SPECIFIC_SPELL_DAMAGE, spell->getIndex())) / 100.0);
+	base = static_cast<int64_t>(base * (100 + valOfBonuses(BonusType::SPECIFIC_SPELL_DAMAGE, TBonusSubtype(spell->getId()))) / 100.0);
 
 	int maxSchoolBonus = 0;
 
 	spell->forEachSchool([&maxSchoolBonus, this](const SpellSchool & cnf, bool & stop)
 	{
-		vstd::amax(maxSchoolBonus, valOfBonuses(BonusType::SPELL_DAMAGE, cnf));
+		vstd::amax(maxSchoolBonus, valOfBonuses(BonusType::SPELL_DAMAGE, TBonusSubtype(cnf)));
 	});
 
 	base = static_cast<int64_t>(base * (100 + maxSchoolBonus) / 100.0);
 
 	if(affectedStack && affectedStack->creatureLevel() > 0) //Hero specials like Solmyr, Deemer
-		base = static_cast<int64_t>(base * static_cast<double>(100 + valOfBonuses(BonusType::SPECIAL_SPELL_LEV, spell->getIndex()) / affectedStack->creatureLevel()) / 100.0);
+		base = static_cast<int64_t>(base * static_cast<double>(100 + valOfBonuses(BonusType::SPECIAL_SPELL_LEV, TBonusSubtype(spell->getId())) / affectedStack->creatureLevel()) / 100.0);
 
 	return base;
 }
 
 int64_t CGHeroInstance::getSpecificSpellBonus(const spells::Spell * spell, int64_t base) const
 {
-	base = static_cast<int64_t>(base * (100 + valOfBonuses(BonusType::SPECIFIC_SPELL_DAMAGE, spell->getIndex())) / 100.0);
+	base = static_cast<int64_t>(base * (100 + valOfBonuses(BonusType::SPECIFIC_SPELL_DAMAGE, TBonusSubtype(spell->getId()))) / 100.0);
 	return base;
 }
 
@@ -751,19 +751,19 @@ bool CGHeroInstance::canCastThisSpell(const spells::Spell * spell) const
 	const bool isAllowed = IObjectInterface::cb->isAllowed(0, spell->getIndex());
 
 	const bool inSpellBook = vstd::contains(spells, spell->getId()) && hasSpellbook();
-	const bool specificBonus = hasBonusOfType(BonusType::SPELL, spell->getIndex());
+	const bool specificBonus = hasBonusOfType(BonusType::SPELL, TBonusSubtype(spell->getId()));
 
 	bool schoolBonus = false;
 
 	spell->forEachSchool([this, &schoolBonus](const SpellSchool & cnf, bool & stop)
 	{
-		if(hasBonusOfType(BonusType::SPELLS_OF_SCHOOL, cnf))
+		if(hasBonusOfType(BonusType::SPELLS_OF_SCHOOL, TBonusSubtype(cnf)))
 		{
 			schoolBonus = stop = true;
 		}
 	});
 
-	const bool levelBonus = hasBonusOfType(BonusType::SPELLS_OF_LEVEL, spell->getLevel());
+	const bool levelBonus = hasBonusOfType(BonusType::SPELLS_OF_LEVEL, BonusSubtypes::spellLevel(spell->getLevel()));
 
 	if(spell->isSpecial())
 	{
@@ -845,13 +845,6 @@ CStackBasicDescriptor CGHeroInstance::calculateNecromancy (const BattleResult &b
 		TConstBonusListPtr improvedNecromancy = getBonuses(Selector::type()(BonusType::IMPROVED_NECROMANCY));
 		if(!improvedNecromancy->empty())
 		{
-			auto getCreatureID = [](const std::shared_ptr<Bonus> & bonus) -> CreatureID
-			{
-				assert(bonus->subtype >=0);
-				if(bonus->subtype >= 0)
-					return CreatureID(bonus->subtype);
-				return CreatureID::NONE;
-			};
 			int maxCasualtyLevel = 1;
 			for(const auto & casualty : casualties)
 				vstd::amax(maxCasualtyLevel, VLC->creatures()->getByIndex(casualty.first)->getLevel());
@@ -868,9 +861,9 @@ CStackBasicDescriptor CGHeroInstance::calculateNecromancy (const BattleResult &b
 				}
 				else
 				{
-					auto quality = [getCreatureID](const std::shared_ptr<Bonus> & pick) -> std::tuple<int, int, int>
+					auto quality = [](const std::shared_ptr<Bonus> & pick) -> std::tuple<int, int, int>
 					{
-						const auto * c = getCreatureID(pick).toCreature();
+						const auto * c = pick->subtype.as<CreatureID>().toCreature();
 						return std::tuple<int, int, int> {c->getLevel(), static_cast<int>(c->getFullRecruitCost().marketValue()), -pick->additionalInfo[1]};
 					};
 					if(quality(topPick) < quality(newPick))
@@ -879,7 +872,7 @@ CStackBasicDescriptor CGHeroInstance::calculateNecromancy (const BattleResult &b
 			}
 			if(topPick)
 			{
-				creatureTypeRaised = getCreatureID(topPick);
+				creatureTypeRaised = topPick->subtype.as<CreatureID>();
 				requiredCasualtyLevel = std::max(topPick->additionalInfo[1], 1);
 			}
 		}
@@ -1015,12 +1008,12 @@ int32_t CGHeroInstance::getSpellCost(const spells::Spell * sp) const
 
 void CGHeroInstance::pushPrimSkill( PrimarySkill which, int val )
 {
-	auto sel = Selector::typeSubtype(BonusType::PRIMARY_SKILL, static_cast<int>(which))
+	auto sel = Selector::typeSubtype(BonusType::PRIMARY_SKILL, TBonusSubtype(which))
 		.And(Selector::sourceType()(BonusSource::HERO_BASE_SKILL));
 	if(hasBonus(sel))
 		removeBonuses(sel);
 		
-	addNewBonus(std::make_shared<Bonus>(BonusDuration::PERMANENT, BonusType::PRIMARY_SKILL, BonusSource::HERO_BASE_SKILL, val, id.getNum(), static_cast<int>(which)));
+	addNewBonus(std::make_shared<Bonus>(BonusDuration::PERMANENT, BonusType::PRIMARY_SKILL, BonusSource::HERO_BASE_SKILL, val, id.getNum(), TBonusSubtype(which)));
 }
 
 EAlignment CGHeroInstance::getAlignment() const
@@ -1283,7 +1276,7 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills()
 
 	for(const auto & elem : secSkills)
 	{
-		if(elem.second < SecSkillLevel::EXPERT)
+		if(elem.second < MasteryLevel::EXPERT)
 			basicAndAdv.insert(elem.first);
 		else
 			expert.insert(elem.first);
@@ -1403,7 +1396,7 @@ void CGHeroInstance::setPrimarySkill(PrimarySkill primarySkill, si64 value, ui8
 	if(primarySkill < PrimarySkill::EXPERIENCE)
 	{
 		auto skill = getBonusLocalFirst(Selector::type()(BonusType::PRIMARY_SKILL)
-			.And(Selector::subtype()(static_cast<int>(primarySkill)))
+			.And(Selector::subtype()(TBonusSubtype(primarySkill)))
 			.And(Selector::sourceType()(BonusSource::HERO_BASE_SKILL)));
 		assert(skill);
 
@@ -1474,13 +1467,10 @@ void CGHeroInstance::levelUpAutomatically(CRandomGenerator & rand)
 	}
 }
 
-bool CGHeroInstance::hasVisions(const CGObjectInstance * target, const int subtype) const
+bool CGHeroInstance::hasVisions(const CGObjectInstance * target, TBonusSubtype subtype) const
 {
 	//VISIONS spell support
-
-	const auto cached = "type_" + std::to_string(vstd::to_underlying(BonusType::VISIONS)) + "__subtype_" + std::to_string(subtype);
-
-	const int visionsMultiplier = valOfBonuses(Selector::typeSubtype(BonusType::VISIONS,subtype), cached);
+	const int visionsMultiplier = valOfBonuses(BonusType::VISIONS, subtype);
 
 	int visionsRange =  visionsMultiplier * getPrimSkillLevel(PrimarySkill::SPELL_POWER);
 
@@ -1567,11 +1557,11 @@ void CGHeroInstance::serializeCommonOptions(JsonSerializeFormat & handler)
 		{
 			auto primarySkills = handler.enterStruct("primarySkills");
 
-			for(int i = 0; i < GameConstants::PRIMARY_SKILLS; ++i)
+			for(auto i = PrimarySkill::BEGIN; i < PrimarySkill::END; ++i)
 			{
-				int value = valOfBonuses(Selector::typeSubtype(BonusType::PRIMARY_SKILL, i).And(Selector::sourceType()(BonusSource::HERO_BASE_SKILL)));
+				int value = valOfBonuses(Selector::typeSubtype(BonusType::PRIMARY_SKILL, TBonusSubtype(i)).And(Selector::sourceType()(BonusSource::HERO_BASE_SKILL)));
 
-				handler.serializeInt(NPrimarySkill::names[i], value, 0);
+				handler.serializeInt(NPrimarySkill::names[i.getNum()], value, 0);
 			}
 		}
 	}
@@ -1762,7 +1752,7 @@ bool CGHeroInstance::isMissionCritical() const
 
 void CGHeroInstance::fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &stack) const
 {
-	TConstBonusListPtr lista = getBonuses(Selector::typeSubtype(BonusType::SPECIAL_UPGRADE, stack.type->getId()));
+	TConstBonusListPtr lista = getBonuses(Selector::typeSubtype(BonusType::SPECIAL_UPGRADE, TBonusSubtype(stack.type->getId())));
 	for(const auto & it : *lista)
 	{
 		auto nid = CreatureID(it->additionalInfo[0]);

+ 1 - 1
lib/mapObjects/CGHeroInstance.h

@@ -248,7 +248,7 @@ public:
 
 	void fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &stack) const override;
 
-	bool hasVisions(const CGObjectInstance * target, const int subtype) const;
+	bool hasVisions(const CGObjectInstance * target, TBonusSubtype masteryLevel) const;
 	/// If this hero perishes, the scenario is failed
 	bool isMissionCritical() const;
 

+ 1 - 1
lib/mapObjects/CGTownBuilding.cpp

@@ -146,7 +146,7 @@ void COPWBonus::onHeroVisit (const CGHeroInstance * h) const
 			if(!h->hasBonusFrom(BonusSource::OBJECT, Obj::STABLES)) //does not stack with advMap Stables
 			{
 				GiveBonus gb;
-				gb.bonus = Bonus(BonusDuration::ONE_WEEK, BonusType::MOVEMENT, BonusSource::OBJECT, 600, 94, VLC->generaltexth->arraytxt[100], 1);
+				gb.bonus = Bonus(BonusDuration::ONE_WEEK, BonusType::MOVEMENT, BonusSource::OBJECT, 600, Obj::STABLES, BonusSubtypes::heroMovementLand, VLC->generaltexth->arraytxt[100]);
 				gb.id = heroID.getNum();
 				cb->giveHeroBonus(&gb);
 

+ 1 - 1
lib/mapObjects/CGTownInstance.cpp

@@ -160,7 +160,7 @@ GrowthInfo CGTownInstance::getGrowthInfo(int level) const
 	}
 
 	//other *-of-legion-like bonuses (%d to growth cumulative with grail)
-	TConstBonusListPtr bonuses = getBonuses(Selector::type()(BonusType::CREATURE_GROWTH).And(Selector::subtype()(level)));
+	TConstBonusListPtr bonuses = getBonuses(Selector::typeSubtype(BonusType::CREATURE_GROWTH, BonusSubtypes::creatureLevel(level)));
 	for(const auto & b : *bonuses)
 		ret.entries.emplace_back(b->val, b->Description());
 

+ 2 - 2
lib/mapObjects/MiscObjects.cpp

@@ -836,7 +836,7 @@ void CGArtifact::serializeJsonOptions(JsonSerializeFormat& handler)
 	if(handler.saving && ID == Obj::SPELL_SCROLL)
 	{
 		const std::shared_ptr<Bonus> b = storedArtifact->getBonusLocalFirst(Selector::type()(BonusType::SPELL));
-		SpellID spellId(b->subtype);
+		SpellID spellId(b->subtype.as<SpellID>());
 
 		handler.serializeId("spell", spellId, SpellID::NONE);
 	}
@@ -1204,7 +1204,7 @@ void CGLighthouse::giveBonusTo(const PlayerColor & player, bool onInit) const
 	gb.bonus.duration = BonusDuration::PERMANENT;
 	gb.bonus.source = BonusSource::OBJECT;
 	gb.bonus.sid = id.getNum();
-	gb.bonus.subtype = 0;
+	gb.bonus.subtype = BonusSubtypes::heroMovementSea;
 
 	// FIXME: This is really dirty hack
 	// Proper fix would be to make CGLighthouse into bonus system node

+ 2 - 2
lib/pathfinder/CPathfinder.cpp

@@ -531,9 +531,9 @@ const TurnInfo * CPathfinderHelper::getTurnInfo() const
 	return turnsInfo[turn];
 }
 
-bool CPathfinderHelper::hasBonusOfType(const BonusType type, const int subtype) const
+bool CPathfinderHelper::hasBonusOfType(const BonusType type) const
 {
-	return turnsInfo[turn]->hasBonusOfType(type, subtype);
+	return turnsInfo[turn]->hasBonusOfType(type);
 }
 
 int CPathfinderHelper::getMaxMovePoints(const EPathfindingLayer & layer) const

+ 1 - 1
lib/pathfinder/CPathfinder.h

@@ -83,7 +83,7 @@ public:
 	void updateTurnInfo(const int turn = 0);
 	bool isLayerAvailable(const EPathfindingLayer & layer) const;
 	const TurnInfo * getTurnInfo() const;
-	bool hasBonusOfType(const BonusType type, const int subtype = -1) const;
+	bool hasBonusOfType(BonusType type) const;
 	int getMaxMovePoints(const EPathfindingLayer & layer) const;
 
 	std::vector<int3> getCastleGates(const PathNodeInfo & source) const;

+ 4 - 4
lib/pathfinder/TurnInfo.cpp

@@ -23,7 +23,7 @@ TurnInfo::BonusCache::BonusCache(const TConstBonusListPtr & bl)
 	for(const auto & terrain : VLC->terrainTypeHandler->objects)
 	{
 		noTerrainPenalty.push_back(static_cast<bool>(
-				bl->getFirst(Selector::type()(BonusType::NO_TERRAIN_PENALTY).And(Selector::subtype()(terrain->getIndex())))));
+				bl->getFirst(Selector::type()(BonusType::NO_TERRAIN_PENALTY).And(Selector::subtype()(TBonusSubtype(terrain->getId()))))));
 	}
 
 	freeShipBoarding = static_cast<bool>(bl->getFirst(Selector::type()(BonusType::FREE_SHIP_BOARDING)));
@@ -71,7 +71,7 @@ bool TurnInfo::isLayerAvailable(const EPathfindingLayer & layer) const
 	return true;
 }
 
-bool TurnInfo::hasBonusOfType(BonusType type, int subtype) const
+bool TurnInfo::hasBonusOfType(BonusType type, TBonusSubtype subtype) const
 {
 	switch(type)
 	{
@@ -82,14 +82,14 @@ bool TurnInfo::hasBonusOfType(BonusType type, int subtype) const
 	case BonusType::WATER_WALKING:
 		return bonusCache->waterWalking;
 	case BonusType::NO_TERRAIN_PENALTY:
-		return bonusCache->noTerrainPenalty[subtype];
+		return bonusCache->noTerrainPenalty[subtype.getNum()];
 	}
 
 	return static_cast<bool>(
 			bonuses->getFirst(Selector::type()(type).And(Selector::subtype()(subtype))));
 }
 
-int TurnInfo::valOfBonuses(BonusType type, int subtype) const
+int TurnInfo::valOfBonuses(BonusType type, TBonusSubtype subtype) const
 {
 	switch(type)
 	{

+ 4 - 2
lib/pathfinder/TurnInfo.h

@@ -42,8 +42,10 @@ struct DLL_LINKAGE TurnInfo
 
 	TurnInfo(const CGHeroInstance * Hero, const int Turn = 0);
 	bool isLayerAvailable(const EPathfindingLayer & layer) const;
-	bool hasBonusOfType(const BonusType type, const int subtype = -1) const;
-	int valOfBonuses(const BonusType type, const int subtype = -1) const;
+	bool hasBonusOfType(const BonusType type) const;
+	bool hasBonusOfType(const BonusType type, const TBonusSubtype subtype) const;
+	int valOfBonuses(const BonusType type) const;
+	int valOfBonuses(const BonusType type, const TBonusSubtype subtype) const;
 	void updateHeroBonuses(BonusType type, const CSelector& sel) const;
 	int getMaxMovePoints(const EPathfindingLayer & layer) const;
 };

+ 1 - 1
lib/rewardable/Reward.cpp

@@ -37,7 +37,7 @@ Rewardable::Reward::Reward()
 	, movePercentage(-1)
 	, primary(4, 0)
 	, removeObject(false)
-	, spellCast(SpellID::NONE, SecSkillLevel::NONE)
+	, spellCast(SpellID::NONE, MasteryLevel::NONE)
 {
 }
 

+ 1 - 1
lib/spells/AbilityCaster.cpp

@@ -35,7 +35,7 @@ int32_t AbilityCaster::getSpellSchoolLevel(const Spell * spell, int32_t * outSel
 
 	if(spell->getLevel() > 0)
 	{
-		vstd::amax(skill, unit->valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, SpellSchool(ESpellSchool::ANY)));
+		vstd::amax(skill, unit->valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, TBonusSubtype(SpellSchool::ANY)));
 	}
 
 	vstd::amax(skill, 0);

+ 5 - 5
lib/spells/CSpellHandler.cpp

@@ -383,15 +383,15 @@ int64_t CSpell::adjustRawDamage(const spells::Caster * caster, const battle::Uni
 		//applying protections - when spell has more then one elements, only one protection should be applied (I think)
 		forEachSchool([&](const SpellSchool & cnf, bool & stop)
 		{
-			if(bearer->hasBonusOfType(BonusType::SPELL_DAMAGE_REDUCTION, cnf))
+			if(bearer->hasBonusOfType(BonusType::SPELL_DAMAGE_REDUCTION, TBonusSubtype(cnf)))
 			{
-				ret *= 100 - bearer->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, cnf);
+				ret *= 100 - bearer->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, TBonusSubtype(cnf));
 				ret /= 100;
 				stop = true; //only bonus from one school is used
 			}
 		});
 
-		CSelector selector = Selector::typeSubtype(BonusType::SPELL_DAMAGE_REDUCTION, SpellSchool(ESpellSchool::ANY));
+		CSelector selector = Selector::typeSubtype(BonusType::SPELL_DAMAGE_REDUCTION, TBonusSubtype(SpellSchool::ANY));
 		auto cachingStr = "type_SPELL_DAMAGE_REDUCTION_s_ANY";
 
 		//general spell dmg reduction, works only on magical effects
@@ -402,9 +402,9 @@ int64_t CSpell::adjustRawDamage(const spells::Caster * caster, const battle::Uni
 		}
 
 		//dmg increasing
-		if(bearer->hasBonusOfType(BonusType::MORE_DAMAGE_FROM_SPELL, id))
+		if(bearer->hasBonusOfType(BonusType::MORE_DAMAGE_FROM_SPELL, TBonusSubtype(id)))
 		{
-			ret *= 100 + bearer->valOfBonuses(BonusType::MORE_DAMAGE_FROM_SPELL, id.toEnum());
+			ret *= 100 + bearer->valOfBonuses(BonusType::MORE_DAMAGE_FROM_SPELL, TBonusSubtype(id));
 			ret /= 100;
 		}
 	}

+ 6 - 6
lib/spells/TargetCondition.cpp

@@ -157,7 +157,7 @@ protected:
 	{
 		std::stringstream cachingStr;
 		cachingStr << "type_" << vstd::to_underlying(BonusType::SPELL_IMMUNITY) << "subtype_" << m->getSpellIndex() << "addInfo_1";
-		return !target->hasBonus(Selector::typeSubtypeInfo(BonusType::SPELL_IMMUNITY, m->getSpellIndex(), 1), cachingStr.str());
+		return !target->hasBonus(Selector::typeSubtypeInfo(BonusType::SPELL_IMMUNITY, TBonusSubtype(m->getSpellId()), 1), cachingStr.str());
 	}
 };
 
@@ -179,14 +179,14 @@ protected:
 
 		m->getSpell()->forEachSchool([&](const SpellSchool & cnf, bool & stop) 
 		{
-			if (bearer->hasBonusOfType(BonusType::SPELL_SCHOOL_IMMUNITY, cnf))
+			if (bearer->hasBonusOfType(BonusType::SPELL_SCHOOL_IMMUNITY, TBonusSubtype(cnf)))
 			{
 				elementalImmune = true;
 				stop = true; //only bonus from one school is used
 			}
 			else if(!m->isPositiveSpell()) //negative or indifferent
 			{
-				if (bearer->hasBonusOfType(BonusType::NEGATIVE_EFFECTS_IMMUNITY, cnf))
+				if (bearer->hasBonusOfType(BonusType::NEGATIVE_EFFECTS_IMMUNITY, TBonusSubtype(cnf)))
 				{
 					elementalImmune = true;
 					stop = true; //only bonus from one school is used
@@ -231,7 +231,7 @@ public:
 protected:
 	bool check(const Mechanics * m, const battle::Unit * target) const override
 	{
-		return !target->hasBonusOfType(BonusType::SPELL_IMMUNITY, m->getSpellIndex());
+		return !target->hasBonusOfType(BonusType::SPELL_IMMUNITY, TBonusSubtype(m->getSpellId()));
 	}
 };
 
@@ -292,8 +292,8 @@ class ImmunityNegationCondition : public TargetConditionItemBase
 protected:
 	bool check(const Mechanics * m, const battle::Unit * target) const override
 	{
-		const bool battleWideNegation = target->hasBonusOfType(BonusType::NEGATE_ALL_NATURAL_IMMUNITIES, 0);
-		const bool heroNegation = target->hasBonusOfType(BonusType::NEGATE_ALL_NATURAL_IMMUNITIES, 1);
+		const bool battleWideNegation = target->hasBonusOfType(BonusType::NEGATE_ALL_NATURAL_IMMUNITIES, BonusSubtypes::immunityBattleWide);
+		const bool heroNegation = target->hasBonusOfType(BonusType::NEGATE_ALL_NATURAL_IMMUNITIES, BonusSubtypes::immunityEnemyHero);
 		//Non-magical effects is not affected by orb of vulnerability
 		if(!m->isMagicalEffect())
 			return false;

+ 2 - 2
lib/spells/effects/Damage.cpp

@@ -89,11 +89,11 @@ bool Damage::isReceptive(const Mechanics * m, const battle::Unit * unit) const
 	if(!UnitEffect::isReceptive(m, unit))
 		return false;
 
-	bool isImmune = m->getSpell()->isMagical() && (unit->getBonusBearer()->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, SpellSchool(ESpellSchool::ANY)) >= 100); //General spell damage immunity
+	bool isImmune = m->getSpell()->isMagical() && (unit->getBonusBearer()->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, TBonusSubtype(SpellSchool::ANY)) >= 100); //General spell damage immunity
 	//elemental immunity for damage
 	m->getSpell()->forEachSchool([&](const SpellSchool & cnf, bool & stop)
 	{
-		isImmune |= (unit->getBonusBearer()->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, cnf) >= 100); //100% reduction is immunity
+		isImmune |= (unit->getBonusBearer()->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, TBonusSubtype(cnf)) >= 100); //100% reduction is immunity
 	});
 
 	return !isImmune;

+ 4 - 4
lib/spells/effects/Timed.cpp

@@ -48,7 +48,7 @@ static void describeEffect(std::vector<MetaString> & log, const Mechanics * m, c
 		{
 		case BonusType::NOT_ACTIVE:
 			{
-				switch(bonus.subtype)
+				switch(bonus.subtype.as<SpellID>().toEnum())
 				{
 				case SpellID::STONE_GAZE:
 					addLogLine(558, boost::logic::indeterminate);
@@ -109,9 +109,9 @@ void Timed::apply(ServerCallback * server, const Mechanics * m, const EffectTarg
 	const auto *casterHero = dynamic_cast<const CGHeroInstance *>(m->caster);
 	if(casterHero)
 	{ 
-		peculiarBonus = casterHero->getBonusLocalFirst(Selector::typeSubtype(BonusType::SPECIAL_PECULIAR_ENCHANT, m->getSpellIndex()));
-		addedValueBonus = casterHero->getBonusLocalFirst(Selector::typeSubtype(BonusType::SPECIAL_ADD_VALUE_ENCHANT, m->getSpellIndex()));
-		fixedValueBonus = casterHero->getBonusLocalFirst(Selector::typeSubtype(BonusType::SPECIAL_FIXED_VALUE_ENCHANT, m->getSpellIndex()));
+		peculiarBonus = casterHero->getBonusLocalFirst(Selector::typeSubtype(BonusType::SPECIAL_PECULIAR_ENCHANT, TBonusSubtype(m->getSpellId())));
+		addedValueBonus = casterHero->getBonusLocalFirst(Selector::typeSubtype(BonusType::SPECIAL_ADD_VALUE_ENCHANT, TBonusSubtype(m->getSpellId())));
+		fixedValueBonus = casterHero->getBonusLocalFirst(Selector::typeSubtype(BonusType::SPECIAL_FIXED_VALUE_ENCHANT, TBonusSubtype(m->getSpellId())));
 	}	
 	//TODO: does hero specialty should affects his stack casting spells?
 

+ 1 - 1
lib/spells/effects/UnitEffect.cpp

@@ -253,7 +253,7 @@ bool UnitEffect::isReceptive(const Mechanics * m, const battle::Unit * unit) con
 		//SPELL_IMMUNITY absolute case
 		std::stringstream cachingStr;
 		cachingStr << "type_" << vstd::to_underlying(BonusType::SPELL_IMMUNITY) << "subtype_" << m->getSpellIndex() << "addInfo_1";
-		return !unit->hasBonus(Selector::typeSubtypeInfo(BonusType::SPELL_IMMUNITY, m->getSpellIndex(), 1), cachingStr.str());
+		return !unit->hasBonus(Selector::typeSubtypeInfo(BonusType::SPELL_IMMUNITY, TBonusSubtype(m->getSpellId()), 1), cachingStr.str());
 	}
 	else
 	{