Merge pull request #1822 from rilian-la-te/spell-mechanics-v3

Bonus refactoring, part3 (save-incompatible)

Mods/vcmi/config/vcmi/chinese.json

@@ -201,8 +201,6 @@
 	"core.bonus.DOUBLE_DAMAGE_CHANCE.description": "${val}%几率造成双倍基础伤害",
 	"core.bonus.DRAGON_NATURE.name": "龙",
 	"core.bonus.DRAGON_NATURE.description": "生物拥有龙的特性",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.name": "魔法直伤免疫",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.description": "免疫直接造成伤害的魔法",
 	"core.bonus.EARTH_IMMUNITY.name": "土系免疫",
 	"core.bonus.EARTH_IMMUNITY.description": "免疫所有土系魔法",
 	"core.bonus.ENCHANTER.name": "强化师",

Mods/vcmi/config/vcmi/english.json

@@ -201,8 +201,6 @@
 	"core.bonus.DOUBLE_DAMAGE_CHANCE.description": "Has a ${val}% chance of dealing double base damage when attacking",
 	"core.bonus.DRAGON_NATURE.name": "Dragon",
 	"core.bonus.DRAGON_NATURE.description": "Creature has a Dragon Nature",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.name": "Direct Damage Immunity",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.description": "Immune to direct damage spells",
 	"core.bonus.EARTH_IMMUNITY.name": "Earth immunity",
 	"core.bonus.EARTH_IMMUNITY.description": "Immune to all spells from the school of Earth magic",
 	"core.bonus.ENCHANTER.name": "Enchanter",

Mods/vcmi/config/vcmi/german.json

@@ -196,8 +196,6 @@
 	"core.bonus.DOUBLE_DAMAGE_CHANCE.description": "${val}% Chance auf doppelten Schaden",
 	"core.bonus.DRAGON_NATURE.name": "Drache",
 	"core.bonus.DRAGON_NATURE.description": "Kreatur hat eine Drachennatur",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.name": "Direkte Schadensimmunität",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.description": "Immun gegen Direktschadenszauber",
 	"core.bonus.EARTH_IMMUNITY.name": "Erdimmunität",
 	"core.bonus.EARTH_IMMUNITY.description": "Immun gegen alle Zauber der Erdschule",
 	"core.bonus.ENCHANTER.name": "Verzauberer",

Mods/vcmi/config/vcmi/polish.json

@@ -175,8 +175,6 @@
 	"core.bonus.DOUBLE_DAMAGE_CHANCE.description": "${val}% szans na podwójne obrażenia",
 	"core.bonus.DRAGON_NATURE.name": "Smok",
 	"core.bonus.DRAGON_NATURE.description": "Stworzenie posiada smoczą naturę",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.name": "Odporność na bezpośrednie obrażenia",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.description": "Odporny na czary zadające bezpośrednie obrażenia",
 	"core.bonus.EARTH_IMMUNITY.name": "Odporność na ziemię",
 	"core.bonus.EARTH_IMMUNITY.description": "Odporny na wszystkie czary szkoły ziemi",
 	"core.bonus.ENCHANTER.name": "Czarodziej",

Mods/vcmi/config/vcmi/russian.json

@@ -199,8 +199,6 @@
 	"core.bonus.DOUBLE_DAMAGE_CHANCE.description": "Шанс ${val}% на двойной урон",
 	"core.bonus.DRAGON_NATURE.name": "Дракон",
 	"core.bonus.DRAGON_NATURE.description": "Это существо - дракон",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.name": "Иммунитет к магии прямого урона",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.description": "Заклинания прямого урона не могут быть применены",
 	"core.bonus.EARTH_IMMUNITY.name": "Иммунитет к земле",
 	"core.bonus.EARTH_IMMUNITY.description": "Иммунитет ко всем заклинаниям Магии Земли",
 	"core.bonus.ENCHANTER.name": "Заклинатель (массовое)",

Mods/vcmi/config/vcmi/spanish.json

@@ -201,8 +201,6 @@
 	"core.bonus.DOUBLE_DAMAGE_CHANCE.description": "${val}% de probabilidad de doble daño",
 	"core.bonus.DRAGON_NATURE.name": "Dragón",
 	"core.bonus.DRAGON_NATURE.description": "La criatura tiene la naturaleza de dragón",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.name": "Inmunidad al Daño Directo",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.description": "Inmune a hechizos de daño directo",
 	"core.bonus.EARTH_IMMUNITY.name": "Inmunidad a la Tierra",
 	"core.bonus.EARTH_IMMUNITY.description": "Inmune a todos los hechizos de la escuela de tierra",
 	"core.bonus.ENCHANTER.name": "Encantador",

Mods/vcmi/config/vcmi/ukrainian.json

@@ -175,8 +175,6 @@
 	"core.bonus.DOUBLE_DAMAGE_CHANCE.description" : "${val}% шанс нанести подвійної шкоди",
 	"core.bonus.DRAGON_NATURE.name" : "Дракон",
 	"core.bonus.DRAGON_NATURE.description" : "Істота має драконячу природу",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.name" : "Імунітет до прямої шкоди",
-	"core.bonus.DIRECT_DAMAGE_IMMUNITY.description" : "Імунітет до заклять, що завдають прямої шкоди",
 	"core.bonus.EARTH_IMMUNITY.name" : "Імунітет Землі",
 	"core.bonus.EARTH_IMMUNITY.description" : "Імунітет до всіх заклять школи Землі",
 	"core.bonus.ENCHANTER.name" : "Чарівник",

client/ClientCommandManager.cpp

@@ -387,7 +387,7 @@ void ClientCommandManager::handleBonusesCommand(std::istringstream & singleWordB
 		return ss.str();
 	};
 		printCommandMessage("Bonuses of " + LOCPLINT->localState->getCurrentArmy()->getObjectName() + "\n");
-		printCommandMessage(format(LOCPLINT->localState->getCurrentArmy()->getBonusList()) + "\n");
+		printCommandMessage(format(*LOCPLINT->localState->getCurrentArmy()->getAllBonuses(Selector::all, Selector::all)) + "\n");
 
 	printCommandMessage("\nInherited bonuses:\n");
 	TCNodes parents;

client/NetPacksClient.cpp

@@ -329,13 +329,12 @@ void ApplyClientNetPackVisitor::visitGiveBonus(GiveBonus & pack)
 	case GiveBonus::ETarget::HERO:
 		{
 			const CGHeroInstance *h = gs.getHero(ObjectInstanceID(pack.id));
-			callInterfaceIfPresent(cl, h->tempOwner, &IGameEventsReceiver::heroBonusChanged, h, *h->getBonusList().back(), true);
+			callInterfaceIfPresent(cl, h->tempOwner, &IGameEventsReceiver::heroBonusChanged, h, pack.bonus, true);
 		}
 		break;
 	case GiveBonus::ETarget::PLAYER:
 		{
-			const PlayerState *p = gs.getPlayerState(PlayerColor(pack.id));
-			callInterfaceIfPresent(cl, PlayerColor(pack.id), &IGameEventsReceiver::playerBonusChanged, *p->getBonusList().back(), true);
+			callInterfaceIfPresent(cl, PlayerColor(pack.id), &IGameEventsReceiver::playerBonusChanged, pack.bonus, true);
 		}
 		break;
 	}

client/windows/CSpellWindow.cpp

@@ -82,11 +82,11 @@ public:
 			return false;
 
 
-		for(ui8 schoolId = 0; schoolId < 4; schoolId++)
+		for(auto schoolId = 0; schoolId < GameConstants::DEFAULT_SCHOOLS; schoolId++)
 		{
-			if(A->school.at((ESpellSchool)schoolId) && !B->school.at((ESpellSchool)schoolId))
+			if(A->school.at(SpellSchool(schoolId)) && !B->school.at(SpellSchool(schoolId)))
 				return true;
-			if(!A->school.at((ESpellSchool)schoolId) && B->school.at((ESpellSchool)schoolId))
+			if(!A->school.at(SpellSchool(schoolId)) && B->school.at(SpellSchool(schoolId)))
 				return false;
 		}
 
@@ -320,7 +320,7 @@ void CSpellWindow::computeSpellsPerArea()
 	for(const CSpell * spell : mySpells)
 	{
 		if(spell->isCombat() ^ !battleSpellsOnly
-			&& ((selectedTab == 4) || spell->school.at((ESpellSchool)selectedTab))
+			&& ((selectedTab == 4) || spell->school.at(SpellSchool(selectedTab)))
 			)
 		{
 			spellsCurSite.push_back(spell);

config/artifacts.json

@@ -1184,7 +1184,8 @@
 	{
 		"bonuses" : [
 			{
-				"type" : "AIR_SPELL_DMG_PREMY",
+				"type" : "SPELL_DAMAGE",
+				"subtype" : "spellSchool.air",
 				"val" : 50,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -1196,7 +1197,8 @@
 	{
 		"bonuses" : [
 			{
-				"type" : "EARTH_SPELL_DMG_PREMY",
+				"type" : "SPELL_DAMAGE",
+				"subtype" : "spellSchool.earth",
 				"val" : 50,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -1208,7 +1210,8 @@
 	{
 		"bonuses" : [
 			{
-				"type" : "FIRE_SPELL_DMG_PREMY",
+				"type" : "SPELL_DAMAGE",
+				"subtype" : "spellSchool.fire",
 				"val" : 50,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -1220,7 +1223,8 @@
 	{
 		"bonuses" : [
 			{
-				"type" : "WATER_SPELL_DMG_PREMY",
+				"type" : "SPELL_DAMAGE",
+				"subtype" : "spellSchool.water",
 				"val" : 50,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -1271,7 +1275,8 @@
 	{
 		"bonuses" : [
 			{
-				"type" : "FIRE_SPELLS",
+				"type" : "SPELLS_OF_SCHOOL",
+				"subtype" : 1,
 				"val" : 0,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -1283,7 +1288,8 @@
 	{
 		"bonuses" : [
 			{
-				"type" : "AIR_SPELLS",
+				"type" : "SPELLS_OF_SCHOOL",
+				"subtype" : 0,
 				"val" : 0,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -1295,7 +1301,8 @@
 	{
 		"bonuses" : [
 			{
-				"type" : "WATER_SPELLS",
+				"type" : "SPELLS_OF_SCHOOL",
+				"subtype" : 2,
 				"val" : 0,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -1307,7 +1314,8 @@
 	{
 		"bonuses" : [
 			{
-				"type" : "EARTH_SPELLS",
+				"type" : "SPELLS_OF_SCHOOL",
+				"subtype" : 3,
 				"val" : 0,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -2433,8 +2441,8 @@
 				"valueType" : "BASE_NUMBER"
 			},
 			{
-				"type" : "DIRECT_DAMAGE_IMMUNITY",
-				"val" : 0,
+				"type" : "SPELL_DAMAGE_REDUCTION",
+				"val" : 100,
 				"valueType" : "BASE_NUMBER"
 			}
 		],

config/battlefields.json

@@ -13,7 +13,7 @@
 		"bonuses": [
 			{
 				"type" : "MAGIC_SCHOOL_SKILL",
-				"subtype" : 0,
+				"subtype" : "spellSchool.any",
 				"val" : 3,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -29,7 +29,7 @@
 		"bonuses": [
 			{
 				"type" : "MAGIC_SCHOOL_SKILL",
-				"subtype" : 2,
+				"subtype" : "spellSchool.fire",
 				"val" : 3,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -41,7 +41,7 @@
 		"bonuses": [
 			{
 				"type" : "MAGIC_SCHOOL_SKILL",
-				"subtype" : 8,
+				"subtype" : "spellSchool.earth",
 				"val" : 3,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -53,7 +53,7 @@
 		"bonuses": [
 			{
 				"type" : "MAGIC_SCHOOL_SKILL",
-				"subtype" : 1,
+				"subtype" : "spellSchool.air",
 				"val" : 3,
 				"valueType" : "BASE_NUMBER"
 			}
@@ -65,7 +65,7 @@
 		"bonuses": [
 			{
 				"type" : "MAGIC_SCHOOL_SKILL",
-				"subtype" : 4,
+				"subtype" : "spellSchool.water",
 				"val" : 3,
 				"valueType" : "BASE_NUMBER"
 			}

config/bonuses.json

@@ -116,14 +116,6 @@
 		}
 	},
 
-	"DIRECT_DAMAGE_IMMUNITY":
-	{
-		"graphics":
-		{
-			"icon":  "zvs/Lib1.res/E_SPDIR"
-		}
-	},
-
 	"DISGUISED":
 	{
 		"hidden": true

config/creatures/neutral.json

@@ -10,7 +10,7 @@
 			"magicResistance" :
 			{
 				"type" : "SPELL_DAMAGE_REDUCTION",
-				"subtype" : -1,
+				"subtype" : "spellSchool.any",
 				"val" : 85
 			},
 			"nonliving" :
@@ -41,7 +41,7 @@
 			"magicResistance" :
 			{
 				"type" : "SPELL_DAMAGE_REDUCTION",
-				"subtype" : -1,
+				"subtype" : "spellSchool.any",
 				"val" : 95
 			},
 			"nonliving" :

config/creatures/tower.json

@@ -105,7 +105,7 @@
 			"magicResistance" :
 			{
 				"type" : "SPELL_DAMAGE_REDUCTION",
-				"subtype" : -1,
+				"subtype" : "spellSchool.any",
 				"val" : 50
 			},
 			"nonliving" :
@@ -137,7 +137,7 @@
 			"magicResistance" :
 			{
 				"type" : "SPELL_DAMAGE_REDUCTION",
-				"subtype" : -1,
+				"subtype" : "spellSchool.any",
 				"val" : 75
 			},
 			"nonliving" :

config/gameConfig.json

@@ -215,6 +215,7 @@
 				"spellDamage" : 
 				{
 					"type" : "SPELL_DAMAGE",
+					"subtype" : "spellSchool.any",
 					"val" : 100,
 					"valueType" : "BASE_NUMBER"
 				},

config/heroes/dungeon.json

@@ -186,6 +186,7 @@
 				"sorcery" : {
 					"targetSourceType" : "SECONDARY_SKILL",
 					"type" : "SPELL_DAMAGE",
+					"subtype" : "spellSchool.any",
 					"updater" : "TIMES_HERO_LEVEL",
 					"val" : 5,
 					"valueType" : "PERCENT_TO_TARGET_TYPE"

config/heroes/fortress.json

@@ -262,6 +262,7 @@
 				"sorcery" : {
 					"targetSourceType" : "SECONDARY_SKILL",
 					"type" : "SPELL_DAMAGE",
+					"subtype" : "spellSchool.any",
 					"updater" : "TIMES_HERO_LEVEL",
 					"val" : 5,
 					"valueType" : "PERCENT_TO_TARGET_TYPE"

config/heroes/inferno.json

@@ -261,6 +261,7 @@
 				"sorcery" : {
 					"targetSourceType" : "SECONDARY_SKILL",
 					"type" : "SPELL_DAMAGE",
+					"subtype" : "spellSchool.any",
 					"updater" : "TIMES_HERO_LEVEL",
 					"val" : 5,
 					"valueType" : "PERCENT_TO_TARGET_TYPE"

config/heroes/necropolis.json

@@ -192,6 +192,7 @@
 				"sorcery" : {
 					"targetSourceType" : "SECONDARY_SKILL",
 					"type" : "SPELL_DAMAGE",
+					"subtype" : "spellSchool.any",
 					"updater" : "TIMES_HERO_LEVEL",
 					"val" : 5,
 					"valueType" : "PERCENT_TO_TARGET_TYPE"

config/heroes/stronghold.json

@@ -135,6 +135,7 @@
 				"sorcery" : {
 					"targetSourceType" : "SECONDARY_SKILL",
 					"type" : "SPELL_DAMAGE",
+					"subtype" : "spellSchool.any",
 					"updater" : "TIMES_HERO_LEVEL",
 					"val" : 5,
 					"valueType" : "PERCENT_TO_TARGET_TYPE"
@@ -239,6 +240,7 @@
 				"sorcery" : {
 					"targetSourceType" : "SECONDARY_SKILL",
 					"type" : "SPELL_DAMAGE",
+					"subtype" : "spellSchool.any",
 					"updater" : "TIMES_HERO_LEVEL",
 					"val" : 5,
 					"valueType" : "PERCENT_TO_TARGET_TYPE"

config/skills.json

@@ -406,7 +406,7 @@
 		"base" : {
 			"effects" : {
 				"main" : {
-					"subtype" : 2,
+					"subtype" : "spellSchool.fire",
 					"type" : "MAGIC_SCHOOL_SKILL",
 					"valueType" : "BASE_NUMBER"
 				}
@@ -434,7 +434,7 @@
 		"base" : {
 			"effects" : {
 				"main" : {
-					"subtype" : 1,
+					"subtype" : "spellSchool.air",
 					"type" : "MAGIC_SCHOOL_SKILL",
 					"valueType" : "BASE_NUMBER"
 				}
@@ -462,7 +462,7 @@
 		"base" : {
 			"effects" : {
 				"main" : {
-					"subtype" : 4,
+					"subtype" : "spellSchool.water",
 					"type" : "MAGIC_SCHOOL_SKILL",
 					"valueType" : "BASE_NUMBER"
 				}
@@ -490,7 +490,7 @@
 		"base" : {
 			"effects" : {
 				"main" : {
-					"subtype" : 8,
+					"subtype" : "spellSchool.earth",
 					"type" : "MAGIC_SCHOOL_SKILL",
 					"valueType" : "BASE_NUMBER"
 				}
@@ -737,6 +737,7 @@
 			"effects" : {
 				"main" : {
 					"type" : "SPELL_DAMAGE",
+					"subtype" : "spellSchool.any",
 					"valueType" : "BASE_NUMBER"
 				}
 			}

config/spells/ability.json

@@ -425,9 +425,6 @@
 			"indifferent": true
 		},
 		"targetCondition" : {
-			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
-			}
 		}
 	}
 }

config/spells/offensive.json

@@ -30,9 +30,6 @@
 			"negative": true
 		},
 		"targetCondition" : {
-			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
-			}
 		}
 	},
 	"iceBolt" : {
@@ -66,9 +63,6 @@
 			"negative": true
 		},
 		"targetCondition" : {
-			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
-			}
 		}
 	},
 	"lightningBolt" : {
@@ -95,9 +89,6 @@
 			"negative": true
 		},
 		"targetCondition" : {
-			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
-			}
 		}
 	},
 	"implosion" : {
@@ -125,7 +116,6 @@
 		},
 		"targetCondition" : {
 			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal",
 				"bonus.SIEGE_WEAPON" : "absolute"
 			}
 		}
@@ -197,7 +187,6 @@
 		},
 		"targetCondition" : {
 			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
 			}
 		}
 	},
@@ -226,7 +215,6 @@
 		},
 		"targetCondition" : {
 			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
 			}
 		}
 	},
@@ -255,7 +243,6 @@
 		},
 		"targetCondition" : {
 			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
 			}
 		}
 	},
@@ -284,7 +271,6 @@
 		},
 		"targetCondition" : {
 			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
 			}
 		}
 	},
@@ -313,7 +299,6 @@
 		},
 		"targetCondition" : {
 			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal",
 				"bonus.SIEGE_WEAPON" : "absolute",
 				"bonus.UNDEAD" : "absolute"
 			}
@@ -345,9 +330,6 @@
 		"targetCondition" : {
 			"allOf" : {
 				"bonus.UNDEAD" : "absolute"
-			},
-			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
 			}
 		}
 	},
@@ -375,9 +357,6 @@
 			"negative": true
 		},
 		"targetCondition" : {
-			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
-			}
 		}
 	},
 	"titanBolt" : {

config/spells/other.json

@@ -105,9 +105,6 @@
 			"special": true
 		},
 		"targetCondition" : {
-			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
-			}
 		}
 	},
 	"landMine" : {
@@ -163,9 +160,6 @@
 			"indifferent": true
 		},
 		"targetCondition" : {
-			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
-			}
 		}
 	},
 	"forceField" : {
@@ -287,9 +281,6 @@
 			"special": true
 		},
 		"targetCondition" : {
-			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
-			}
 		}
 	},
 	"fireWall" : {
@@ -358,9 +349,6 @@
 			"indifferent": true
 		},
 		"targetCondition" : {
-			"noneOf" : {
-				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
-			}
 		}
 	},
 	"earthquake" : {

config/spells/timed.json

@@ -104,7 +104,7 @@
 				"effects" : {
 					"spellDamageReduction" : {
 						"type" : "SPELL_DAMAGE_REDUCTION",
-						"subtype" : 0,
+						"subtype" : "spellSchool.air",
 						"duration" : "N_TURNS",
 						"val" : 30
 					}
@@ -147,7 +147,7 @@
 				"effects" : {
 					"spellDamageReduction" : {
 						"type" : "SPELL_DAMAGE_REDUCTION",
-						"subtype" : 1,
+						"subtype" : "spellSchool.fire",
 						"duration" : "N_TURNS",
 						"val" : 30
 					}
@@ -190,7 +190,7 @@
 				"effects" : {
 					"spellDamageReduction" : {
 						"type" : "SPELL_DAMAGE_REDUCTION",
-						"subtype" : 2,
+						"subtype" : "spellSchool.water",
 						"duration" : "N_TURNS",
 						"val" : 30
 					}
@@ -233,7 +233,7 @@
 				"effects" : {
 					"spellDamageReduction" : {
 						"type" : "SPELL_DAMAGE_REDUCTION",
-						"subtype" : 3,
+						"subtype" : "spellSchool.earth",
 						"duration" : "N_TURNS",
 						"val" : 30
 					}

lib/BasicTypes.cpp

@@ -13,6 +13,7 @@
 #include "VCMI_Lib.h"
 #include "GameConstants.h"
 #include "GameSettings.h"
+#include "JsonNode.h"
 #include "bonuses/BonusList.h"
 #include "bonuses/Bonus.h"
 #include "bonuses/IBonusBearer.h"

lib/BattleFieldHandler.cpp

@@ -11,6 +11,7 @@
 
 #include <vcmi/Entity.h>
 #include "BattleFieldHandler.h"
+#include "JsonNode.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 

lib/CCreatureHandler.cpp

@@ -1059,7 +1059,7 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 		b.type = BonusType::FEAR; break;
 	case 'g':
 		b.type = BonusType::SPELL_DAMAGE_REDUCTION;
-		b.subtype = -1; //all magic schools
+		b.subtype = SpellSchool(ESpellSchool::ANY);
 		break;
 	case 'P':
 		b.type = BonusType::CASTS; break;
@@ -1178,8 +1178,9 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 				b.subtype = 1; //not positive
 				break;
 			case 'O':
-				b.type = BonusType::FIRE_IMMUNITY;
-				b.subtype = 2; //only direct damage
+				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
+				b.subtype = SpellSchool(ESpellSchool::FIRE);
+				b.val = 100; //Full damage immunity
 				break;
 			case 'f':
 				b.type = BonusType::FIRE_IMMUNITY;
@@ -1190,31 +1191,36 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 				b.subtype = 1; //not positive
 				break;
 			case 'W':
-				b.type = BonusType::WATER_IMMUNITY;
-				b.subtype = 2; //only direct damage
+				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
+				b.subtype = SpellSchool(ESpellSchool::WATER);
+				b.val = 100; //Full damage immunity
 				break;
 			case 'w':
 				b.type = BonusType::WATER_IMMUNITY;
 				b.subtype = 0; //all
 				break;
 			case 'E':
-				b.type = BonusType::EARTH_IMMUNITY;
-				b.subtype = 2; //only direct damage
+				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
+				b.subtype = SpellSchool(ESpellSchool::EARTH);
+				b.val = 100; //Full damage immunity
 				break;
 			case 'e':
 				b.type = BonusType::EARTH_IMMUNITY;
 				b.subtype = 0; //all
 				break;
 			case 'A':
-				b.type = BonusType::AIR_IMMUNITY;
-				b.subtype = 2; //only direct damage
+				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
+				b.subtype = SpellSchool(ESpellSchool::AIR);
+				b.val = 100; //Full damage immunity
 				break;
 			case 'a':
 				b.type = BonusType::AIR_IMMUNITY;
 				b.subtype = 0; //all
 				break;
 			case 'D':
-				b.type = BonusType::DIRECT_DAMAGE_IMMUNITY;
+				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
+				b.subtype = SpellSchool(ESpellSchool::ANY);
+				b.val = 100; //Full damage immunity
 				break;
 			case '0':
 				b.type = BonusType::RECEPTIVE;

lib/CGameState.cpp

@@ -703,8 +703,6 @@ CGameState::CGameState()
 	applier = std::make_shared<CApplier<CBaseForGSApply>>();
 	registerTypesClientPacks1(*applier);
 	registerTypesClientPacks2(*applier);
-	//objCaller = new CObjectCallersHandler();
-	globalEffects.setDescription("Global effects");
 	globalEffects.setNodeType(CBonusSystemNode::GLOBAL_EFFECTS);
 }
 

lib/CHeroHandler.h

@@ -248,9 +248,6 @@ class DLL_LINKAGE CHeroHandler : public CHandlerBase<HeroTypeID, HeroType, CHero
 public:
 	CHeroClassHandler classes;
 
-	//default costs of going through terrains. -1 means terrain is impassable
-	std::map<TerrainId, int> terrCosts;
-
 	ui32 level(ui64 experience) const; //calculates level corresponding to given experience amount
 	ui64 reqExp(ui32 level) const; //calculates experience required for given level
 
@@ -271,7 +268,6 @@ public:
 		h & classes;
 		h & objects;
 		h & expPerLevel;
-		h & terrCosts;
 	}
 
 protected:

lib/CModHandler.cpp

@@ -745,6 +745,12 @@ void CModInfo::setEnabled(bool on)
 
 CModHandler::CModHandler() : content(std::make_shared<CContentHandler>())
 {
+	//TODO: moddable spell schools
+	for (auto i = 0; i < GameConstants::DEFAULT_SCHOOLS; ++i)
+		identifiers.registerObject(CModHandler::scopeBuiltin(), "spellSchool", SpellConfig::SCHOOL[i].jsonName, SpellConfig::SCHOOL[i].id);
+
+	identifiers.registerObject(CModHandler::scopeBuiltin(), "spellSchool", "any", SpellSchool(ESpellSchool::ANY));
+
 	for (int i = 0; i < GameConstants::RESOURCE_QUANTITY; ++i)
 	{
 		identifiers.registerObject(CModHandler::scopeBuiltin(), "resource", GameConstants::RESOURCE_NAMES[i], i);

lib/GameConstants.h

@@ -50,6 +50,7 @@ namespace GameConstants
 	constexpr int CREATURES_PER_TOWN = 7; //without upgrades
 	constexpr int SPELL_LEVELS = 5;
 	constexpr int SPELL_SCHOOL_LEVELS = 4;
+	constexpr int DEFAULT_SCHOOLS = 4;
 	constexpr int CRE_LEVELS = 10; // number of creature experience levels
 
 	constexpr int HERO_GOLD_COST = 2500;
@@ -1324,14 +1325,17 @@ class Obstacle : public BaseForID<Obstacle, si32>
 	DLL_LINKAGE static Obstacle fromString(const std::string & identifier);
 };
 
-enum class ESpellSchool: ui8
+enum class ESpellSchool: int8_t
 {
+	ANY 	= -1,
 	AIR 	= 0,
 	FIRE 	= 1,
 	WATER 	= 2,
-	EARTH 	= 3
+	EARTH 	= 3,
 };
 
+using SpellSchool = Identifier<ESpellSchool>;
+
 enum class EMetaclass: ui8
 {
 	INVALID = 0,

lib/JsonNode.cpp

@@ -842,25 +842,12 @@ static BonusParams convertDeprecatedBonus(const JsonNode &ability)
 											   ability["subtype"].isNumber() ? ability["subtype"].Integer() : -1);
 		if(params.isConverted)
 		{
-			if(!params.valRelevant) {
-				params.val = static_cast<si32>(ability["val"].Float());
-				params.valRelevant = true;
-			}
-			BonusValueType valueType = BonusValueType::ADDITIVE_VALUE;
-			if(!ability["valueType"].isNull())
-				valueType = bonusValueMap.find(ability["valueType"].String())->second;
-
-			if(ability["type"].String() == "SECONDARY_SKILL_PREMY" && valueType == BonusValueType::PERCENT_TO_BASE) //assume secondary skill special
+			if(ability["type"].String() == "SECONDARY_SKILL_PREMY" && bonusValueMap.find(ability["valueType"].String())->second == BonusValueType::PERCENT_TO_BASE) //assume secondary skill special
 			{
 				params.valueType = BonusValueType::PERCENT_TO_TARGET_TYPE;
 				params.targetType = BonusSource::SECONDARY_SKILL;
-				params.targetTypeRelevant = true;
 			}
 
-			if(!params.valueTypeRelevant) {
-				params.valueType = valueType;
-				params.valueTypeRelevant = true;
-			}
 			logMod->warn("Please, use this bonus:\n%s\nConverted sucessfully!", params.toJson().toJson());
 			return params;
 		}
@@ -930,10 +917,10 @@ bool JsonUtils::parseBonus(const JsonNode &ability, Bonus *b)
 			return false;
 		}
 		b->type = params->type;
-		b->val = params->val;
-		b->valType = params->valueType;
-		if(params->targetTypeRelevant)
-			b->targetSourceType = params->targetType;
+		b->val = params->val.value_or(0);
+		b->valType = params->valueType.value_or(BonusValueType::ADDITIVE_VALUE);
+		if(params->targetType)
+			b->targetSourceType = params->targetType.value();
 	}
 	else
 		b->type = it->second;
@@ -975,7 +962,15 @@ bool JsonUtils::parseBonus(const JsonNode &ability, Bonus *b)
 		switch (value->getType())
 		{
 		case JsonNode::JsonType::DATA_STRING:
-			b->duration = static_cast<BonusDuration>(parseByMap(bonusDurationMap, value, "duration type "));
+			b->duration = parseByMap(bonusDurationMap, value, "duration type ");
+			break;
+		case JsonNode::JsonType::DATA_VECTOR:
+			{
+				BonusDuration::Type dur = 0;
+				for (const JsonNode & d : value->Vector())
+					dur |= parseByMapN(bonusDurationMap, &d, "duration type ");
+				b->duration = dur;
+			}
 			break;
 		default:
 			logMod->error("Error! Wrong bonus duration format.");
@@ -1065,31 +1060,26 @@ CSelector JsonUtils::parseSelector(const JsonNode & ability)
 		ret = ret.And(Selector::subtype()(subtype));
 	}
 	value = &ability["sourceType"];
-	BonusSource src = BonusSource::OTHER; //Fixes for GCC false maybe-uninitialized
-	si32 id = 0;
-	auto sourceIDRelevant = false;
-	auto sourceTypeRelevant = false;
+	std::optional<BonusSource> src = std::nullopt; //Fixes for GCC false maybe-uninitialized
+	std::optional<si32> id = std::nullopt;
 	if(value->isString())
 	{
 		auto it = bonusSourceMap.find(value->String());
 		if(it != bonusSourceMap.end())
-		{
 			src = it->second;
-			sourceTypeRelevant = true;
-		}
-
 	}
+
 	value = &ability["sourceID"];
 	if(!value->isNull())
 	{
-		sourceIDRelevant = true;
-		resolveIdentifier(id, ability, "sourceID");
+		id = -1;
+		resolveIdentifier(*id, ability, "sourceID");
 	}
 
-	if(sourceIDRelevant && sourceTypeRelevant)
-		ret = ret.And(Selector::source(src, id));
-	else if(sourceTypeRelevant)
-		ret = ret.And(Selector::sourceTypeSel(src));
+	if(src && id)
+		ret = ret.And(Selector::source(*src, *id));
+	else if(src)
+		ret = ret.And(Selector::sourceTypeSel(*src));
 
 	
 	value = &ability["targetSourceType"];

lib/bonuses/Bonus.cpp

@@ -31,26 +31,6 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-const std::set<std::string> deprecatedBonusSet = {
-	"SECONDARY_SKILL_PREMY",
-	"SECONDARY_SKILL_VAL2",
-	"MAXED_SPELL",
-	"LAND_MOVEMENT",
-	"SEA_MOVEMENT",
-	"SIGHT_RADIOUS",
-	"NO_TYPE",
-	"SPECIAL_SECONDARY_SKILL",
-	"FULL_HP_REGENERATION",
-	"KING1",
-	"KING2",
-	"KING3",
-	"BLOCK_MORALE",
-	"BLOCK_LUCK",
-	"SELF_MORALE",
-	"SELF_LUCK",
-	"DIRECT_DAMAGE_IMMUNITY"
-};
-
 //This constructor should be placed here to avoid side effects
 CAddInfo::CAddInfo() = default;
 
@@ -154,7 +134,7 @@ std::string Bonus::Description(std::optional<si32> customValue) const
 	return str.str();
 }
 
-JsonNode subtypeToJson(BonusType type, int subtype)
+static JsonNode subtypeToJson(BonusType type, int subtype)
 {
 	switch(type)
 	{
@@ -177,7 +157,7 @@ JsonNode subtypeToJson(BonusType type, int subtype)
 	}
 }
 
-JsonNode additionalInfoToJson(BonusType type, CAddInfo addInfo)
+static JsonNode additionalInfoToJson(BonusType type, CAddInfo addInfo)
 {
 	switch(type)
 	{
@@ -216,7 +196,7 @@ JsonNode Bonus::toJsonNode() const
 	if(effectRange != BonusLimitEffect::NO_LIMIT)
 		root["effectRange"].String() = vstd::findKey(bonusLimitEffect, effectRange);
 	if(duration != BonusDuration::PERMANENT)
-		root["duration"].String() = vstd::findKey(bonusDurationMap, duration);
+		root["duration"] = BonusDuration::toJson(duration);
 	if(turnsRemain)
 		root["turns"].Integer() = turnsRemain;
 	if(limiter)
@@ -228,7 +208,7 @@ JsonNode Bonus::toJsonNode() const
 	return root;
 }
 
-Bonus::Bonus(BonusDuration 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, std::string Desc, si32 Subtype):
 	duration(Duration),
 	type(Type),
 	subtype(Subtype),
@@ -241,7 +221,7 @@ Bonus::Bonus(BonusDuration Duration, BonusType Type, BonusSource Src, si32 Val,
 	targetSourceType = BonusSource::OTHER;
 }
 
-Bonus::Bonus(BonusDuration 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, si32 Subtype, BonusValueType ValType):
 	duration(Duration),
 	type(Type),
 	subtype(Subtype),
@@ -270,7 +250,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);
-	printField(duration);
+	printField(duration.to_ulong());
 	printField(source);
 	printField(sid);
 	if(bonus.additionalInfo != CAddInfo::NONE)

lib/bonuses/Bonus.h

@@ -10,7 +10,6 @@
 #pragma once
 
 #include "BonusEnum.h"
-#include "../JsonNode.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -21,6 +20,7 @@ class ILimiter;
 class IPropagator;
 class IUpdater;
 class BonusList;
+class CSelector;
 
 using TBonusSubtype = int32_t;
 using TBonusListPtr = std::shared_ptr<BonusList>;
@@ -52,7 +52,7 @@ public:
 /// Struct for handling bonuses of several types. Can be transferred to any hero
 struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 {
-	BonusDuration duration = BonusDuration::PERMANENT; //uses BonusDuration values
+	BonusDuration::Type duration = BonusDuration::PERMANENT; //uses BonusDuration values
 	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
@@ -75,8 +75,8 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 
 	std::string description;
 
-	Bonus(BonusDuration Duration, BonusType Type, BonusSource Src, si32 Val, ui32 ID, std::string Desc, si32 Subtype=-1);
-	Bonus(BonusDuration 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 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() = default;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
@@ -107,43 +107,53 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 	}
 	static bool NDays(const Bonus *hb)
 	{
-		return hb->duration == BonusDuration::N_DAYS;
+		auto set = hb->duration & BonusDuration::N_DAYS;
+		return set.any();
 	}
 	static bool NTurns(const Bonus *hb)
 	{
-		return hb->duration == BonusDuration::N_TURNS;
+		auto set = hb->duration & BonusDuration::N_TURNS;
+		return set.any();
 	}
 	static bool OneDay(const Bonus *hb)
 	{
-		return hb->duration == BonusDuration::ONE_DAY;
+		auto set = hb->duration & BonusDuration::ONE_DAY;
+		return set.any();
 	}
 	static bool OneWeek(const Bonus *hb)
 	{
-		return hb->duration == BonusDuration::ONE_WEEK;
+		auto set = hb->duration & BonusDuration::ONE_WEEK;
+		return set.any();
 	}
 	static bool OneBattle(const Bonus *hb)
 	{
-		return hb->duration == BonusDuration::ONE_BATTLE;
+		auto set = hb->duration & BonusDuration::ONE_BATTLE;
+		return set.any();
 	}
 	static bool Permanent(const Bonus *hb)
 	{
-		return hb->duration == BonusDuration::PERMANENT;
+		auto set = hb->duration & BonusDuration::PERMANENT;
+		return set.any();
 	}
 	static bool UntilGetsTurn(const Bonus *hb)
 	{
-		return hb->duration == BonusDuration::STACK_GETS_TURN;
+		auto set = hb->duration & BonusDuration::STACK_GETS_TURN;
+		return set.any();
 	}
 	static bool UntilAttack(const Bonus *hb)
 	{
-		return hb->duration == BonusDuration::UNTIL_ATTACK;
+		auto set = hb->duration & BonusDuration::UNTIL_ATTACK;
+		return set.any();
 	}
 	static bool UntilBeingAttacked(const Bonus *hb)
 	{
-		return hb->duration == BonusDuration::UNTIL_BEING_ATTACKED;
+		auto set = hb->duration & BonusDuration::UNTIL_BEING_ATTACKED;
+		return set.any();
 	}
 	static bool UntilCommanderKilled(const Bonus *hb)
 	{
-		return hb->duration == BonusDuration::COMMANDER_KILLED;
+		auto set = hb->duration & BonusDuration::COMMANDER_KILLED;
+		return set.any();
 	}
 	inline bool operator == (const BonusType & cf) const
 	{
@@ -178,6 +188,4 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 
 DLL_LINKAGE std::ostream & operator<<(std::ostream &out, const Bonus &bonus);
 
-extern DLL_LINKAGE const std::set<std::string> deprecatedBonusSet;
-
 VCMI_LIB_NAMESPACE_END

lib/bonuses/BonusEnum.cpp

@@ -13,6 +13,8 @@
 
 #include "BonusEnum.h"
 
+#include "../JsonNode.h"
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 #define BONUS_NAME(x) { #x, BonusType::x },
@@ -30,7 +32,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 #undef BONUS_SOURCE
 
 #define BONUS_ITEM(x) { #x, BonusDuration::x },
-const std::map<std::string, BonusDuration> bonusDurationMap =
+const std::map<std::string, BonusDuration::Type> bonusDurationMap =
 {
 	BONUS_ITEM(PERMANENT)
 	BONUS_ITEM(ONE_BATTLE)
@@ -55,4 +57,28 @@ const std::map<std::string, BonusLimitEffect> bonusLimitEffect =
 };
 #undef BONUS_ITEM
 
+namespace BonusDuration
+{
+	JsonNode toJson(const Type & duration)
+	{
+		std::vector<std::string> durationNames;
+		for(auto durBit = 0; durBit < duration.size(); durBit++)
+		{
+			if(duration[durBit])
+				durationNames.push_back(vstd::findKey(bonusDurationMap, duration & Type().set(durBit)));
+		}
+		if(durationNames.size() == 1)
+		{
+			return JsonUtils::stringNode(durationNames[0]);
+		}
+		else
+		{
+			JsonNode node(JsonNode::JsonType::DATA_VECTOR);
+			for(const std::string & dur : durationNames)
+				node.Vector().push_back(JsonUtils::stringNode(dur));
+			return node;
+		}
+	}
+}
+
 VCMI_LIB_NAMESPACE_END

lib/bonuses/BonusEnum.h

@@ -12,6 +12,8 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
+class JsonNode;
+
 #define BONUS_LIST										\
 	BONUS_NAME(NONE) 									\
 	BONUS_NAME(LEVEL_COUNTER) /* for commander artifacts*/ \
@@ -20,29 +22,22 @@ VCMI_LIB_NAMESPACE_BEGIN
 	BONUS_NAME(LUCK) \
 	BONUS_NAME(PRIMARY_SKILL) /*uses subtype to pick skill; additional info if set: 1 - only melee, 2 - only distance*/  \
 	BONUS_NAME(SIGHT_RADIUS) \
-	BONUS_NAME(MANA_REGENERATION) /*points per turn apart from normal (1 + mysticism)*/  \
+	BONUS_NAME(MANA_REGENERATION) /*points per turn*/  \
 	BONUS_NAME(FULL_MANA_REGENERATION) /*all mana points are replenished every day*/  \
 	BONUS_NAME(NONEVIL_ALIGNMENT_MIX) /*good and neutral creatures can be mixed without morale penalty*/  \
 	BONUS_NAME(SURRENDER_DISCOUNT) /*%*/  \
 	BONUS_NAME(STACKS_SPEED)  /*additional info - percent of speed bonus applied after direct bonuses; >0 - added, <0 - subtracted to this part*/ \
 	BONUS_NAME(FLYING_MOVEMENT) /*value - penalty percentage*/ \
 	BONUS_NAME(SPELL_DURATION) \
-	BONUS_NAME(AIR_SPELL_DMG_PREMY) \
-	BONUS_NAME(EARTH_SPELL_DMG_PREMY) \
-	BONUS_NAME(FIRE_SPELL_DMG_PREMY) \
-	BONUS_NAME(WATER_SPELL_DMG_PREMY) \
 	BONUS_NAME(WATER_WALKING) /*value - penalty percentage*/ \
 	BONUS_NAME(NEGATE_ALL_NATURAL_IMMUNITIES) \
 	BONUS_NAME(STACK_HEALTH) \
-	BONUS_NAME(FIRE_SPELLS) \
-	BONUS_NAME(AIR_SPELLS) \
-	BONUS_NAME(WATER_SPELLS) \
-	BONUS_NAME(EARTH_SPELLS) \
 	BONUS_NAME(GENERATE_RESOURCE) /*daily value, uses subtype (resource type)*/  \
 	BONUS_NAME(CREATURE_GROWTH) /*for legion artifacts: value - week growth bonus, subtype - monster level if aplicable*/  \
 	BONUS_NAME(WHIRLPOOL_PROTECTION) /*hero won't lose army when teleporting through whirlpool*/  \
 	BONUS_NAME(SPELL) /*hero knows spell, val - skill level (0 - 3), subtype - spell id*/  \
 	BONUS_NAME(SPELLS_OF_LEVEL) /*hero knows all spells of given level, val - skill level; subtype - level*/  \
+	BONUS_NAME(SPELLS_OF_SCHOOL) /*hero knows all spells of given school, subtype - spell school; 0 - air, 1 - fire, 2 - water, 3 - earth*/  \
 	BONUS_NAME(BATTLE_NO_FLEEING) /*for shackles of war*/ \
 	BONUS_NAME(MAGIC_SCHOOL_SKILL) /* //eg. for magic plains terrain, subtype: school of magic (0 - all, 1 - fire, 2 - air, 4 - water, 8 - earth), value - level*/ \
 	BONUS_NAME(FREE_SHOOTING) /*stacks can shoot even if otherwise blocked (sharpshooter's bow effect)*/ \
@@ -79,7 +74,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 	BONUS_NAME(SPELL_LIKE_ATTACK) /*subtype - spell, value - spell level; range is taken from spell, but damage from creature; eg. magog*/ \
 	BONUS_NAME(THREE_HEADED_ATTACK) /*eg. cerberus*/	\
 	BONUS_NAME(GENERAL_DAMAGE_PREMY)						\
-	BONUS_NAME(FIRE_IMMUNITY)	/*subtype 0 - all, 1 - all except positive, 2 - only damage spells*/						\
+	BONUS_NAME(FIRE_IMMUNITY)	/*subtype 0 - all, 1 - all except positive*/						\
 	BONUS_NAME(WATER_IMMUNITY)							\
 	BONUS_NAME(EARTH_IMMUNITY)							\
 	BONUS_NAME(AIR_IMMUNITY)							\
@@ -120,7 +115,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 	BONUS_NAME(NO_MORALE) /*eg. when fighting on cursed ground*/ \
 	BONUS_NAME(DARKNESS) /*val = radius */ \
 	BONUS_NAME(SPECIAL_SPELL_LEV) /*subtype = id, val = value per level in percent*/\
-	BONUS_NAME(SPELL_DAMAGE) /*val = value, now works for sorcery*/\
+	BONUS_NAME(SPELL_DAMAGE) /*val = value, now works for sorcery, subtype - spell school; -1 - all, 0 - air, 1 - fire, 2 - water, 3 - earth*/\
 	BONUS_NAME(SPECIFIC_SPELL_DAMAGE) /*subtype = id of spell, val = value*/\
 	BONUS_NAME(SPECIAL_PECULIAR_ENCHANT) /*blesses and curses with id = val dependent on unit's level, subtype = 0 or 1 for Coronius*/\
 	BONUS_NAME(SPECIAL_UPGRADE) /*subtype = base, additionalInfo = target */\
@@ -133,7 +128,6 @@ VCMI_LIB_NAMESPACE_BEGIN
 	BONUS_NAME(BIND_EFFECT) /*doesn't do anything particular, works as a marker)*/\
 	BONUS_NAME(ACID_BREATH) /*additional val damage per creature after attack, additional info - chance in percent*/\
 	BONUS_NAME(RECEPTIVE) /*accepts friendly spells even with immunity*/\
-	BONUS_NAME(DIRECT_DAMAGE_IMMUNITY) /*direct damage spells, that is*/\
 	BONUS_NAME(CASTS) /*how many times creature can cast activated spell*/ \
 	BONUS_NAME(SPECIFIC_SPELL_POWER) /* value used for Thunderbolt and Resurrection cast by units, subtype - spell id */\
 	BONUS_NAME(CREATURE_SPELL_POWER) /* value per unit, divided by 100 (so faerie Dragons have 800)*/ \
@@ -221,18 +215,20 @@ enum class BonusType
     BONUS_LIST
 #undef BONUS_NAME
 };
-enum class BonusDuration : uint16_t //when bonus is automatically removed
+namespace BonusDuration  //when bonus is automatically removed
 {
-    PERMANENT = 1,
-    ONE_BATTLE = 2, //at the end of battle
-    ONE_DAY = 4,   //at the end of day
-    ONE_WEEK = 8, //at the end of week (bonus lasts till the end of week, thats NOT 7 days
-    N_TURNS = 16, //used during battles, after battle bonus is always removed
-    N_DAYS = 32,
-    UNTIL_BEING_ATTACKED = 64, /*removed after attack and counterattacks are performed*/
-    UNTIL_ATTACK = 128, /*removed after attack and counterattacks are performed*/
-    STACK_GETS_TURN = 256, /*removed when stack gets its turn - used for defensive stance*/
-    COMMANDER_KILLED = 512
+	using Type = std::bitset<10>;
+	extern JsonNode toJson(const Type & duration);
+	constexpr Type PERMANENT = 1 << 0;
+	constexpr Type ONE_BATTLE = 1 << 1; //at the end of battle
+	constexpr Type ONE_DAY = 1 << 2;   //at the end of day
+	constexpr Type ONE_WEEK = 1 << 3; //at the end of week (bonus lasts till the end of week, thats NOT 7 days
+	constexpr Type N_TURNS = 1 << 4; //used during battles, after battle bonus is always removed
+	constexpr Type N_DAYS = 1 << 5;
+	constexpr Type UNTIL_BEING_ATTACKED = 1 << 6; /*removed after attack and counterattacks are performed*/
+	constexpr Type UNTIL_ATTACK = 1 << 7; /*removed after attack and counterattacks are performed*/
+	constexpr Type STACK_GETS_TURN = 1 << 8; /*removed when stack gets its turn - used for defensive stance*/
+	constexpr Type COMMANDER_KILLED = 1 << 9;
 };
 enum class BonusSource
 {
@@ -258,7 +254,7 @@ enum class BonusValueType
 extern DLL_LINKAGE const std::map<std::string, BonusType> bonusNameMap;
 extern DLL_LINKAGE const std::map<std::string, BonusValueType> bonusValueMap;
 extern DLL_LINKAGE const std::map<std::string, BonusSource> bonusSourceMap;
-extern DLL_LINKAGE const std::map<std::string, BonusDuration> bonusDurationMap;
+extern DLL_LINKAGE const std::map<std::string, BonusDuration::Type> bonusDurationMap;
 extern DLL_LINKAGE const std::map<std::string, BonusLimitEffect> bonusLimitEffect;
 
 VCMI_LIB_NAMESPACE_END

lib/bonuses/BonusList.cpp

@@ -11,6 +11,8 @@
 #include "StdInc.h"
 #include "CBonusSystemNode.h"
 
+#include "../JsonNode.h"
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 BonusList::BonusList(bool BelongsToTree) : belongsToTree(BelongsToTree)

lib/bonuses/BonusParams.cpp

@@ -17,6 +17,34 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
+const std::set<std::string> deprecatedBonusSet = {
+	"SECONDARY_SKILL_PREMY",
+	"SECONDARY_SKILL_VAL2",
+	"MAXED_SPELL",
+	"LAND_MOVEMENT",
+	"SEA_MOVEMENT",
+	"SIGHT_RADIOUS",
+	"NO_TYPE",
+	"SPECIAL_SECONDARY_SKILL",
+	"FULL_HP_REGENERATION",
+	"KING1",
+	"KING2",
+	"KING3",
+	"BLOCK_MORALE",
+	"BLOCK_LUCK",
+	"SELF_MORALE",
+	"SELF_LUCK",
+	"DIRECT_DAMAGE_IMMUNITY",
+	"AIR_SPELL_DMG_PREMY",
+	"EARTH_SPELL_DMG_PREMY"
+	"FIRE_SPELL_DMG_PREMY"
+	"WATER_SPELL_DMG_PREMY",
+	"FIRE_SPELLS",
+	"AIR_SPELLS",
+	"WATER_SPELLS",
+	"EARTH_SPELLS"
+};
+
 BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSubtypeStr, int deprecatedSubtype):
 	isConverted(true)
 {
@@ -44,92 +72,79 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 		{
 			type = BonusType::MANA_PER_KNOWLEDGE;
 			valueType = BonusValueType::PERCENT_TO_BASE;
-			valueTypeRelevant = true;
 		}
 		else if(deprecatedSubtype == SecondarySkill::SORCERY || deprecatedSubtypeStr == "skill.sorcery")
+		{
 			type = BonusType::SPELL_DAMAGE;
+			subtype = SpellSchool(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;
-			subtypeRelevant = true;
 			type = BonusType::PERCENTAGE_DAMAGE_BOOST;
 		}
 		else if(deprecatedSubtype == SecondarySkill::OFFENCE || deprecatedSubtypeStr == "skill.offence")
 		{
 			subtype = 0;
-			subtypeRelevant = true;
 			type = BonusType::PERCENTAGE_DAMAGE_BOOST;
 		}
 		else if(deprecatedSubtype == SecondarySkill::ARMORER || deprecatedSubtypeStr == "skill.armorer")
 		{
 			subtype = -1;
-			subtypeRelevant = true;
 			type = BonusType::GENERAL_DAMAGE_REDUCTION;
 		}
 		else if(deprecatedSubtype == SecondarySkill::NAVIGATION || deprecatedSubtypeStr == "skill.navigation")
 		{
 			subtype = 0;
-			subtypeRelevant = true;
 			valueType = BonusValueType::PERCENT_TO_BASE;
-			valueTypeRelevant = true;
 			type = BonusType::MOVEMENT;
 		}
 		else if(deprecatedSubtype == SecondarySkill::LOGISTICS || deprecatedSubtypeStr == "skill.logistics")
 		{
 			subtype = 1;
-			subtypeRelevant = true;
 			valueType = BonusValueType::PERCENT_TO_BASE;
-			valueTypeRelevant = true;
 			type = BonusType::MOVEMENT;
 		}
 		else if(deprecatedSubtype == SecondarySkill::ESTATES || deprecatedSubtypeStr == "skill.estates")
 		{
 			type = BonusType::GENERATE_RESOURCE;
 			subtype = GameResID(EGameResID::GOLD);
-			subtypeRelevant = true;
 		}
 		else if(deprecatedSubtype == SecondarySkill::AIR_MAGIC || deprecatedSubtypeStr == "skill.airMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtypeRelevant = true;
-			subtype = 4;
+			subtype = SpellSchool(ESpellSchool::AIR);
 		}
 		else if(deprecatedSubtype == SecondarySkill::WATER_MAGIC || deprecatedSubtypeStr == "skill.waterMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtypeRelevant = true;
-			subtype = 1;
+			subtype = SpellSchool(ESpellSchool::WATER);
 		}
 		else if(deprecatedSubtype == SecondarySkill::FIRE_MAGIC || deprecatedSubtypeStr == "skill.fireMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtypeRelevant = true;
-			subtype = 2;
+			subtype = SpellSchool(ESpellSchool::FIRE);
 		}
 		else if(deprecatedSubtype == SecondarySkill::EARTH_MAGIC || deprecatedSubtypeStr == "skill.earthMagic")
 		{
 			type = BonusType::MAGIC_SCHOOL_SKILL;
-			subtypeRelevant = true;
-			subtype = 8;
+			subtype = SpellSchool(ESpellSchool::EARTH);
 		}
 		else if (deprecatedSubtype == SecondarySkill::ARTILLERY || deprecatedSubtypeStr == "skill.artillery")
 		{
 			type = BonusType::BONUS_DAMAGE_CHANCE;
-			subtypeRelevant = true;
 			subtypeStr = "core:creature.ballista";
 		}
 		else if (deprecatedSubtype == SecondarySkill::FIRST_AID || deprecatedSubtypeStr == "skill.firstAid")
 		{
 			type = BonusType::SPECIFIC_SPELL_POWER;
-			subtypeRelevant = true;
 			subtypeStr = "core:spell.firstAid";
 		}
 		else if (deprecatedSubtype == SecondarySkill::BALLISTICS || deprecatedSubtypeStr == "skill.ballistics")
 		{
 			type = BonusType::CATAPULT_EXTRA_SHOTS;
-			subtypeRelevant = true;
 			subtypeStr = "core:spell.catapultShot";
 		}
 		else
@@ -142,7 +157,6 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 		else if (deprecatedSubtype == SecondarySkill::ARTILLERY || deprecatedSubtypeStr == "skill.artillery")
 		{
 			type = BonusType::HERO_GRANTS_ATTACKS;
-			subtypeRelevant = true;
 			subtypeStr = "core:creature.ballista";
 		}
 		else
@@ -151,52 +165,41 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 	else if (deprecatedTypeStr == "SEA_MOVEMENT")
 	{
 		subtype = 0;
-		subtypeRelevant = true;
 		valueType = BonusValueType::ADDITIVE_VALUE;
-		valueTypeRelevant = true;
 		type = BonusType::MOVEMENT;
 	}
 	else if (deprecatedTypeStr == "LAND_MOVEMENT")
 	{
 		subtype = 1;
-		subtypeRelevant = true;
 		valueType = BonusValueType::ADDITIVE_VALUE;
-		valueTypeRelevant = true;
 		type = BonusType::MOVEMENT;
 	}
 	else if (deprecatedTypeStr == "MAXED_SPELL")
 	{
 		type = BonusType::SPELL;
 		subtypeStr = deprecatedSubtypeStr;
-		subtypeRelevant = true;
 		valueType = BonusValueType::INDEPENDENT_MAX;
-		valueTypeRelevant = true;
 		val = 3;
-		valRelevant = true;
 	}
 	else if (deprecatedTypeStr == "FULL_HP_REGENERATION")
 	{
 		type = BonusType::HP_REGENERATION;
 		val = 100000; //very high value to always chose stack health
-		valRelevant = true;
 	}
 	else if (deprecatedTypeStr == "KING1")
 	{
 		type = BonusType::KING;
 		val = 0;
-		valRelevant = true;
 	}
 	else if (deprecatedTypeStr == "KING2")
 	{
 		type = BonusType::KING;
 		val = 2;
-		valRelevant = true;
 	}
 	else if (deprecatedTypeStr == "KING3")
 	{
 		type = BonusType::KING;
 		val = 3;
-		valRelevant = true;
 	}
 	else if (deprecatedTypeStr == "SIGHT_RADIOUS")
 		type = BonusType::SIGHT_RADIUS;
@@ -204,17 +207,59 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 	{
 		type = BonusType::MORALE;
 		val = 1;
-		valRelevant = true;
 		valueType = BonusValueType::INDEPENDENT_MAX;
-		valueTypeRelevant = true;
 	}
 	else if (deprecatedTypeStr == "SELF_LUCK")
 	{
 		type = BonusType::LUCK;
 		val = 1;
-		valRelevant = true;
 		valueType = BonusValueType::INDEPENDENT_MAX;
-		valueTypeRelevant = true;
+	}
+	else if (deprecatedTypeStr == "DIRECT_DAMAGE_IMMUNITY")
+	{
+		type = BonusType::SPELL_DAMAGE_REDUCTION;
+		subtype = SpellSchool(ESpellSchool::ANY);
+		val = 100;
+	}
+	else if (deprecatedTypeStr == "AIR_SPELL_DMG_PREMY")
+	{
+		type = BonusType::SPELL_DAMAGE;
+		subtype = SpellSchool(ESpellSchool::AIR);
+	}
+	else if (deprecatedTypeStr == "FIRE_SPELL_DMG_PREMY")
+	{
+		type = BonusType::SPELL_DAMAGE;
+		subtype = SpellSchool(ESpellSchool::FIRE);
+	}
+	else if (deprecatedTypeStr == "WATER_SPELL_DMG_PREMY")
+	{
+		type = BonusType::SPELL_DAMAGE;
+		subtype = SpellSchool(ESpellSchool::WATER);
+	}
+	else if (deprecatedTypeStr == "EARTH_SPELL_DMG_PREMY")
+	{
+		type = BonusType::SPELL_DAMAGE;
+		subtype = SpellSchool(ESpellSchool::EARTH);
+	}
+	else if (deprecatedTypeStr == "AIR_SPELLS")
+	{
+		type = BonusType::SPELLS_OF_SCHOOL;
+		subtype = SpellSchool(ESpellSchool::AIR);
+	}
+	else if (deprecatedTypeStr == "FIRE_SPELLS")
+	{
+		type = BonusType::SPELLS_OF_SCHOOL;
+		subtype = SpellSchool(ESpellSchool::FIRE);
+	}
+	else if (deprecatedTypeStr == "WATER_SPELLS")
+	{
+		type = BonusType::SPELLS_OF_SCHOOL;
+		subtype = SpellSchool(ESpellSchool::WATER);
+	}
+	else if (deprecatedTypeStr == "EARTH_SPELLS")
+	{
+		type = BonusType::SPELLS_OF_SCHOOL;
+		subtype = SpellSchool(ESpellSchool::EARTH);
 	}
 	else
 		isConverted = false;
@@ -226,16 +271,16 @@ const JsonNode & BonusParams::toJson()
 	if(ret.isNull())
 	{
 		ret["type"].String() = vstd::findKey(bonusNameMap, type);
-		if(subtypeRelevant && !subtypeStr.empty())
-			ret["subtype"].String() = subtypeStr;
-		else if(subtypeRelevant)
-			ret["subtype"].Integer() = subtype;
-		if(valueTypeRelevant)
-			ret["valueType"].String() = vstd::findKey(bonusValueMap, valueType);
-		if(valRelevant)
-			ret["val"].Float() = val;
-		if(targetTypeRelevant)
-			ret["targetSourceType"].String() = vstd::findKey(bonusSourceMap, targetType);
+		if(subtypeStr)
+			ret["subtype"].String() = *subtypeStr;
+		else if(subtype)
+			ret["subtype"].Integer() = *subtype;
+		if(valueType)
+			ret["valueType"].String() = vstd::findKey(bonusValueMap, *valueType);
+		if(val)
+			ret["val"].Float() = *val;
+		if(targetType)
+			ret["targetSourceType"].String() = vstd::findKey(bonusSourceMap, *targetType);
 		jsonCreated = true;
 	}
 	return ret;
@@ -244,16 +289,19 @@ const JsonNode & BonusParams::toJson()
 CSelector BonusParams::toSelector()
 {
 	assert(isConverted);
-	if(subtypeRelevant && !subtypeStr.empty())
-		JsonUtils::resolveIdentifier(subtype, toJson(), "subtype");
+	if(subtypeStr)
+	{
+		subtype = -1;
+		JsonUtils::resolveIdentifier(*subtype, toJson(), "subtype");
+	}
 
 	auto ret = Selector::type()(type);
-	if(subtypeRelevant)
-		ret = ret.And(Selector::subtype()(subtype));
-	if(valueTypeRelevant)
-		ret = ret.And(Selector::valueType(valueType));
-	if(targetTypeRelevant)
-		ret = ret.And(Selector::targetSourceType()(targetType));
+	if(subtype)
+		ret = ret.And(Selector::subtype()(*subtype));
+	if(valueType)
+		ret = ret.And(Selector::valueType(*valueType));
+	if(targetType)
+		ret = ret.And(Selector::targetSourceType()(*targetType));
 	return ret;
 }
 

lib/bonuses/BonusParams.h

@@ -19,15 +19,11 @@ VCMI_LIB_NAMESPACE_BEGIN
 struct DLL_LINKAGE BonusParams {
 	bool isConverted;
 	BonusType type = BonusType::NONE;
-	TBonusSubtype subtype = -1;
-	std::string subtypeStr;
-	bool subtypeRelevant = false;
-	BonusValueType valueType = BonusValueType::BASE_NUMBER;
-	bool valueTypeRelevant = false;
-	si32 val = 0;
-	bool valRelevant = false;
-	BonusSource targetType = BonusSource::SECONDARY_SKILL;
-	bool targetTypeRelevant = false;
+	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;
 
 	BonusParams(bool isConverted = true) : isConverted(isConverted) {};
 	BonusParams(std::string deprecatedTypeStr, std::string deprecatedSubtypeStr = "", int deprecatedSubtype = 0);
@@ -38,4 +34,6 @@ private:
 	bool jsonCreated = false;
 };
 
+extern DLL_LINKAGE const std::set<std::string> deprecatedBonusSet;
+
 VCMI_LIB_NAMESPACE_END

lib/bonuses/CBonusSystemNode.cpp

@@ -23,11 +23,6 @@ constexpr bool CBonusSystemNode::cachingEnabled = true;
 #define FOREACH_PARENT(pname) 	TNodes lparents; getParents(lparents); for(CBonusSystemNode *pname : lparents)
 #define FOREACH_RED_CHILD(pname) 	TNodes lchildren; getRedChildren(lchildren); for(CBonusSystemNode *pname : lchildren)
 
-PlayerColor CBonusSystemNode::retrieveNodeOwner(const CBonusSystemNode * node)
-{
-	return node ? node->getOwner() : PlayerColor::CANNOT_DETERMINE;
-}
-
 std::shared_ptr<Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelector & selector)
 {
 	auto ret = bonuses.getFirst(selector);
@@ -51,19 +46,15 @@ std::shared_ptr<const Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelecto
 
 void CBonusSystemNode::getParents(TCNodes & out) const /*retrieves list of parent nodes (nodes to inherit bonuses from) */
 {
-	for(const auto & elem : parents)
-	{
-		const CBonusSystemNode *parent = elem;
-		out.insert(parent);
-	}
+	for(const auto * elem : parents)
+		out.insert(elem);
 }
 
 void CBonusSystemNode::getParents(TNodes &out)
 {
-	for (auto & elem : parents)
+	for (auto * elem : parents)
 	{
-		const CBonusSystemNode *parent = elem;
-		out.insert(const_cast<CBonusSystemNode*>(parent));
+		out.insert(elem);
 	}
 }
 
@@ -218,11 +209,6 @@ std::shared_ptr<Bonus> CBonusSystemNode::getUpdatedBonus(const std::shared_ptr<B
 	return updater->createUpdatedBonus(b, * this);
 }
 
-CBonusSystemNode::CBonusSystemNode()
-	:CBonusSystemNode(false)
-{
-}
-
 CBonusSystemNode::CBonusSystemNode(bool isHypotetic):
 	bonuses(true),
 	exportedBonuses(true),
@@ -245,7 +231,6 @@ CBonusSystemNode::CBonusSystemNode(CBonusSystemNode && other) noexcept:
 	bonuses(std::move(other.bonuses)),
 	exportedBonuses(std::move(other.exportedBonuses)),
 	nodeType(other.nodeType),
-	description(other.description),
 	cachedLast(0),
 	isHypotheticNode(other.isHypotheticNode)
 {
@@ -466,18 +451,13 @@ bool CBonusSystemNode::isIndependentNode() const
 
 std::string CBonusSystemNode::nodeName() const
 {
-	return !description.empty()
-		? description
-		: std::string("Bonus system node of type ") + typeid(*this).name();
+	return std::string("Bonus system node of type ") + typeid(*this).name();
 }
 
 std::string CBonusSystemNode::nodeShortInfo() const
 {
 	std::ostringstream str;
 	str << "'" << typeid(* this).name() << "'";
-	description.length() > 0 
-		? str << " (" << description << ")"
-		: str << " (no description)";
 	return str.str();
 }
 
@@ -594,21 +574,11 @@ CBonusSystemNode::ENodeTypes CBonusSystemNode::getNodeType() const
 	return nodeType;
 }
 
-const BonusList& CBonusSystemNode::getBonusList() const
-{
-	return bonuses;
-}
-
 const TNodesVector& CBonusSystemNode::getParentNodes() const
 {
 	return parents;
 }
 
-const TNodesVector& CBonusSystemNode::getChildrenNodes() const
-{
-	return children;
-}
-
 void CBonusSystemNode::setNodeType(CBonusSystemNode::ENodeTypes type)
 {
 	nodeType = type;
@@ -624,16 +594,6 @@ const BonusList & CBonusSystemNode::getExportedBonusList() const
 	return exportedBonuses;
 }
 
-const std::string& CBonusSystemNode::getDescription() const
-{
-	return description;
-}
-
-void CBonusSystemNode::setDescription(const std::string &description)
-{
-	this->description = description;
-}
-
 void CBonusSystemNode::limitBonuses(const BonusList &allBonuses, BonusList &out) const
 {
 	assert(&allBonuses != &out); //todo should it work in-place?

lib/bonuses/CBonusSystemNode.h

@@ -9,6 +9,8 @@
  */
 #pragma once
 
+#include "GameConstants.h"
+
 #include "BonusList.h"
 #include "IBonusBearer.h"
 
@@ -35,7 +37,6 @@ private:
 	TNodesVector children;
 
 	ENodeTypes nodeType;
-	std::string description;
 	bool isHypotheticNode;
 
 	static const bool cachingEnabled;
@@ -53,9 +54,31 @@ private:
 	TConstBonusListPtr getAllBonusesWithoutCaching(const CSelector &selector, const CSelector &limit, const CBonusSystemNode *root = nullptr) const;
 	std::shared_ptr<Bonus> getUpdatedBonus(const std::shared_ptr<Bonus> & b, const TUpdaterPtr & updater) const;
 
+	void getRedParents(TNodes &out);  //retrieves list of red parent nodes (nodes bonuses propagate from)
+	void getRedAncestors(TNodes &out);
+	void getRedChildren(TNodes &out);
+
+	void getAllParents(TCNodes & out) const;
+
+	void newChildAttached(CBonusSystemNode & child);
+	void childDetached(CBonusSystemNode & child);
+	void propagateBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & source);
+	void unpropagateBonus(const std::shared_ptr<Bonus> & b);
+	bool actsAsBonusSourceOnly() const;
+
+	void newRedDescendant(CBonusSystemNode & descendant); //propagation needed
+	void removedRedDescendant(CBonusSystemNode & descendant); //de-propagation needed
+
+	std::string nodeShortInfo() const;
+
+	void exportBonus(const std::shared_ptr<Bonus> & b);
+
+protected:
+	bool isIndependentNode() const; //node is independent when it has no parents nor children
+	void exportBonuses();
+
 public:
-	explicit CBonusSystemNode();
-	explicit CBonusSystemNode(bool isHypotetic);
+	explicit CBonusSystemNode(bool isHypotetic = false);
 	explicit CBonusSystemNode(ENodeTypes NodeType);
 	CBonusSystemNode(CBonusSystemNode && other) noexcept;
 	virtual ~CBonusSystemNode();
@@ -68,12 +91,6 @@ public:
 
 	//non-const interface
 	void getParents(TNodes &out);  //retrieves list of parent nodes (nodes to inherit bonuses from)
-
-	void getRedParents(TNodes &out);  //retrieves list of red parent nodes (nodes bonuses propagate from)
-	void getRedAncestors(TNodes &out);
-	void getRedChildren(TNodes &out);
-	void getAllParents(TCNodes & out) const;
-	static PlayerColor retrieveNodeOwner(const CBonusSystemNode * node);
 	std::shared_ptr<Bonus> getBonusLocalFirst(const CSelector & selector);
 
 	void attachTo(CBonusSystemNode & parent);
@@ -82,38 +99,23 @@ public:
 	virtual void addNewBonus(const std::shared_ptr<Bonus>& b);
 	void accumulateBonus(const std::shared_ptr<Bonus>& b); //add value of bonus with same type/subtype or create new
 
-	void newChildAttached(CBonusSystemNode & child);
-	void childDetached(CBonusSystemNode & child);
-	void propagateBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & source);
-	void unpropagateBonus(const std::shared_ptr<Bonus> & b);
 	void removeBonus(const std::shared_ptr<Bonus>& b);
 	void removeBonuses(const CSelector & selector);
 	void removeBonusesRecursive(const CSelector & s);
-	void newRedDescendant(CBonusSystemNode & descendant); //propagation needed
-	void removedRedDescendant(CBonusSystemNode & descendant); //de-propagation needed
 
-	bool isIndependentNode() const; //node is independent when it has no parents nor children
-	bool actsAsBonusSourceOnly() const;
 	///updates count of remaining turns and removes outdated bonuses by selector
 	void reduceBonusDurations(const CSelector &s);
 	virtual std::string bonusToString(const std::shared_ptr<Bonus>& bonus, bool description) const {return "";}; //description or bonus name
 	virtual std::string nodeName() const;
-	virtual std::string nodeShortInfo() const;
 	bool isHypothetic() const { return isHypotheticNode; }
 
 	void deserializationFix();
-	void exportBonus(const std::shared_ptr<Bonus> & b);
-	void exportBonuses();
 
-	const BonusList &getBonusList() const;
 	BonusList & getExportedBonusList();
 	const BonusList & getExportedBonusList() const;
 	CBonusSystemNode::ENodeTypes getNodeType() const;
 	void setNodeType(CBonusSystemNode::ENodeTypes type);
-	const TNodesVector &getParentNodes() const;
-	const TNodesVector &getChildrenNodes() const;
-	const std::string &getDescription() const;
-	void setDescription(const std::string &description);
+	const TNodesVector & getParentNodes() const;
 
 	static void treeHasChanged();
 
@@ -129,7 +131,6 @@ public:
 //		h & bonuses;
 		h & nodeType;
 		h & exportedBonuses;
-		h & description;
 		BONUS_TREE_DESERIALIZATION_FIX
 		//h & parents & children;
 	}

lib/bonuses/IBonusBearer.cpp

@@ -15,55 +15,71 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-int IBonusBearer::valOfBonuses(BonusType type, int subtype) const
-{
-	//This part is performance-critical
-	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + "_" + std::to_string(subtype);
-
-	CSelector s = Selector::type()(type);
-	if(subtype != -1)
-		s = s.And(Selector::subtype()(subtype));
-
-	return valOfBonuses(s, cachingStr);
-}
-
 int IBonusBearer::valOfBonuses(const CSelector &selector, const std::string &cachingStr) const
 {
-	CSelector limit = nullptr;
-	TConstBonusListPtr hlp = getAllBonuses(selector, limit, nullptr, cachingStr);
+	TConstBonusListPtr hlp = getAllBonuses(selector, nullptr, nullptr, cachingStr);
 	return hlp->totalValue();
 }
+
 bool IBonusBearer::hasBonus(const CSelector &selector, const std::string &cachingStr) const
 {
 	//TODO: We don't need to count all bonuses and could break on first matching
-	return getBonuses(selector, cachingStr)->size() > 0;
+	return !getBonuses(selector, cachingStr)->empty();
 }
 
 bool IBonusBearer::hasBonus(const CSelector &selector, const CSelector &limit, const std::string &cachingStr) const
 {
-	return getBonuses(selector, limit, cachingStr)->size() > 0;
+	return !getBonuses(selector, limit, cachingStr)->empty();
 }
 
-bool IBonusBearer::hasBonusOfType(BonusType type, int subtype) const
+TConstBonusListPtr IBonusBearer::getBonuses(const CSelector &selector, const std::string &cachingStr) const
 {
-	//This part is performance-ciritcal
-	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + "_" + std::to_string(subtype);
+	return getAllBonuses(selector, nullptr, nullptr, cachingStr);
+}
+
+TConstBonusListPtr IBonusBearer::getBonuses(const CSelector &selector, const CSelector &limit, const std::string &cachingStr) const
+{
+	return getAllBonuses(selector, limit, nullptr, cachingStr);
+}
+
+int IBonusBearer::valOfBonuses(BonusType type) const
+{
+	//This part is performance-critical
+	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type));
+
+	CSelector s = Selector::type()(type);
+
+	return valOfBonuses(s, cachingStr);
+}
+
+bool IBonusBearer::hasBonusOfType(BonusType type) const
+{
+	//This part is performance-critical
+	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type));
 
 	CSelector s = Selector::type()(type);
-	if(subtype != -1)
-		s = s.And(Selector::subtype()(subtype));
 
 	return hasBonus(s, cachingStr);
 }
 
-TConstBonusListPtr IBonusBearer::getBonuses(const CSelector &selector, const std::string &cachingStr) const
+int IBonusBearer::valOfBonuses(BonusType type, int subtype) const
 {
-	return getAllBonuses(selector, nullptr, nullptr, cachingStr);
+	//This part is performance-critical
+	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + "_" + std::to_string(subtype);
+
+	CSelector s = Selector::typeSubtype(type, subtype);
+
+	return valOfBonuses(s, cachingStr);
 }
 
-TConstBonusListPtr IBonusBearer::getBonuses(const CSelector &selector, const CSelector &limit, const std::string &cachingStr) const
+bool IBonusBearer::hasBonusOfType(BonusType type, int subtype) const
 {
-	return getAllBonuses(selector, limit, nullptr, cachingStr);
+	//This part is performance-critical
+	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + "_" + std::to_string(subtype);
+
+	CSelector s = Selector::typeSubtype(type, subtype);
+
+	return hasBonus(s, cachingStr);
 }
 
 bool IBonusBearer::hasBonusFrom(BonusSource source, ui32 sourceID) const

lib/bonuses/IBonusBearer.h

@@ -33,8 +33,10 @@ public:
 	std::shared_ptr<const Bonus> getBonus(const CSelector &selector) const; //returns any bonus visible on node that matches (or nullptr if none matches)
 
 	//Optimized interface (with auto-caching)
-	int valOfBonuses(BonusType type, int subtype = -1) const; //subtype -> subtype of bonus, if -1 then anyt;
-	bool hasBonusOfType(BonusType type, int subtype = -1) const;//determines if hero has a bonus of given type (and optionally subtype)
+	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)
 	bool hasBonusFrom(BonusSource source, ui32 sourceID) const;
 
 	virtual int64_t getTreeVersion() const = 0;

lib/bonuses/Limiters.cpp

@@ -430,7 +430,7 @@ OppositeSideLimiter::OppositeSideLimiter(PlayerColor Owner):
 
 ILimiter::EDecision OppositeSideLimiter::limit(const BonusLimitationContext & context) const
 {
-	auto contextOwner = CBonusSystemNode::retrieveNodeOwner(& context.node);
+	auto contextOwner = context.node.getOwner();
 	auto decision = (owner == contextOwner || owner == PlayerColor::CANNOT_DETERMINE) ? ILimiter::EDecision::DISCARD : ILimiter::EDecision::ACCEPT;
 	return decision;
 }

lib/bonuses/Limiters.h

@@ -9,10 +9,14 @@
  */
 
 #include "Bonus.h"
-#include "battle/BattleHex.h"
+
+#include "../GameConstants.h"
+#include "../battle/BattleHex.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
+class CCreature;
+
 extern DLL_LINKAGE const std::map<std::string, TLimiterPtr> bonusLimiterMap;
 
 struct BonusLimitationContext

lib/bonuses/Updaters.cpp

@@ -197,7 +197,7 @@ JsonNode OwnerUpdater::toJsonNode() const
 
 std::shared_ptr<Bonus> OwnerUpdater::createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const
 {
-	auto owner = CBonusSystemNode::retrieveNodeOwner(&context);
+	auto owner = context.getOwner();
 
 	if(owner == PlayerColor::UNFLAGGABLE)
 		owner = PlayerColor::NEUTRAL;

lib/mapObjects/CGHeroInstance.cpp

@@ -589,7 +589,7 @@ int32_t CGHeroInstance::getSpellSchoolLevel(const spells::Spell * spell, int32_t
 
 	spell->forEachSchool([&, this](const spells::SchoolInfo & cnf, bool & stop)
 	{
-		int32_t thisSchool = valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, 1 << (static_cast<ui8>(cnf.id))); //FIXME: Bonus shouldn't be additive (Witchking Artifacts : Crown of Skies)
+		int32_t thisSchool = valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, cnf.id); //FIXME: Bonus shouldn't be additive (Witchking Artifacts : Crown of Skies)
 		if(thisSchool > skill)
 		{
 			skill = thisSchool;
@@ -598,7 +598,7 @@ int32_t CGHeroInstance::getSpellSchoolLevel(const spells::Spell * spell, int32_t
 		}
 	});
 
-	vstd::amax(skill, valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, 0)); //any school bonus
+	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, 0); //in case we don't know any school
@@ -611,7 +611,7 @@ 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)) / 100.0);
+		base = static_cast<int64_t>(base * (valOfBonuses(BonusType::SPELL_DAMAGE, SpellSchool(ESpellSchool::ANY))) / 100.0);
 
 	base = static_cast<int64_t>(base * (100 + valOfBonuses(BonusType::SPECIFIC_SPELL_DAMAGE, spell->getIndex())) / 100.0);
 
@@ -619,7 +619,7 @@ int64_t CGHeroInstance::getSpellBonus(const spells::Spell * spell, int64_t base,
 
 	spell->forEachSchool([&maxSchoolBonus, this](const spells::SchoolInfo & cnf, bool & stop)
 	{
-		vstd::amax(maxSchoolBonus, valOfBonuses(cnf.damagePremyBonus));
+		vstd::amax(maxSchoolBonus, valOfBonuses(BonusType::SPELL_DAMAGE, cnf.id));
 	});
 
 	base = static_cast<int64_t>(base * (100 + maxSchoolBonus) / 100.0);
@@ -708,7 +708,7 @@ bool CGHeroInstance::canCastThisSpell(const spells::Spell * spell) const
 
 	spell->forEachSchool([this, &schoolBonus](const spells::SchoolInfo & cnf, bool & stop)
 	{
-		if(hasBonusOfType(cnf.knoledgeBonus))
+		if(hasBonusOfType(BonusType::SPELLS_OF_SCHOOL, cnf.id))
 		{
 			schoolBonus = stop = true;
 		}

lib/mapObjects/CObjectHandler.cpp

@@ -272,7 +272,7 @@ int3 CGObjectInstance::getVisitableOffset() const
 	return appearance->getVisitableOffset();
 }
 
-void CGObjectInstance::giveDummyBonus(const ObjectInstanceID & heroID, BonusDuration duration) const
+void CGObjectInstance::giveDummyBonus(const ObjectInstanceID & heroID, BonusDuration::Type duration) const
 {
 	GiveBonus gbonus;
 	gbonus.bonus.type = BonusType::NONE;

lib/mapObjects/CObjectHandler.h

@@ -232,7 +232,7 @@ protected:
 	virtual void setPropertyDer(ui8 what, ui32 val);
 
 	/// Gives dummy bonus from this object to hero. Can be used to track visited state
-	void giveDummyBonus(const ObjectInstanceID & heroID, BonusDuration duration = BonusDuration::ONE_DAY) const;
+	void giveDummyBonus(const ObjectInstanceID & heroID, BonusDuration::Type duration = BonusDuration::ONE_DAY) const;
 
 	///Serialize object-type specific options
 	virtual void serializeJsonOptions(JsonSerializeFormat & handler);

lib/rmg/TreasurePlacer.cpp

@@ -343,7 +343,7 @@ void TreasurePlacer::addAllPossibleObjects()
 			std::vector <CSpell *> spells;
 			for(auto spell : VLC->spellh->objects)
 			{
-				if(map.isAllowedSpell(spell->id) && spell->school[static_cast<ESpellSchool>(i)])
+				if(map.isAllowedSpell(spell->id) && spell->school[SpellSchool(i)])
 					spells.push_back(spell);
 			}
 			

lib/serializer/BinaryDeserializer.h

@@ -550,6 +550,29 @@ public:
 		for(ui32 i = 0; i < length; i++)
 			load(data.data()[i]);
 	}
+	template <std::size_t T>
+	void load(std::bitset<T> &data)
+	{
+		static_assert(T <= 64);
+		if constexpr (T <= 16)
+		{
+			uint16_t read;
+			load(read);
+			data = read;
+		}
+		else if constexpr (T <= 32)
+		{
+			uint32_t read;
+			load(read);
+			data = read;
+		}
+		else if constexpr (T <= 64)
+		{
+			uint64_t read;
+			load(read);
+			data = read;
+		}
+	}
 };
 
 class DLL_LINKAGE CLoadFile : public IBinaryReader

lib/serializer/BinarySerializer.h

@@ -364,6 +364,26 @@ public:
 		for(ui32 i = 0; i < length; i++)
 			save(data.data()[i]);
 	}
+	template <std::size_t T>
+	void save(const std::bitset<T> &data)
+	{
+		static_assert(T <= 64);
+		if constexpr (T <= 16)
+		{
+			auto writ = static_cast<uint16_t>(data.to_ulong());
+			save(writ);
+		}
+		else if constexpr (T <= 32)
+		{
+			auto writ = static_cast<uint32_t>(data.to_ulong());
+			save(writ);
+		}
+		else if constexpr (T <= 64)
+		{
+			auto writ = static_cast<uint64_t>(data.to_ulong());
+			save(writ);
+		}
+	}
 };
 
 class DLL_LINKAGE CSaveFile : public IBinaryWriter

lib/serializer/CSerializer.h

@@ -14,8 +14,8 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-const ui32 SERIALIZATION_VERSION = 822;
-const ui32 MINIMAL_SERIALIZATION_VERSION = 822;
+const ui32 SERIALIZATION_VERSION = 823;
+const ui32 MINIMAL_SERIALIZATION_VERSION = 823;
 const std::string SAVEGAME_MAGIC = "VCMISVG";
 
 class CHero;
@@ -54,7 +54,7 @@ struct VectorizedObjectInfo
 class DLL_LINKAGE CSerializer
 {
 	template<typename T>
-	static si32 idToNumber(const T &t, typename boost::enable_if<boost::is_convertible<T,si32> >::type * dummy = 0)
+	static si32 idToNumber(const T &t, typename std::enable_if<std::is_convertible<T,si32>::value>::type * dummy = 0)
 	{
 		return t;
 	}

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, 0));
+		vstd::amax(skill, unit->valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, SpellSchool(ESpellSchool::ANY)));
 	}
 
 	vstd::amax(skill, 0);

lib/spells/CSpellHandler.cpp

@@ -38,44 +38,32 @@ namespace SpellConfig
 {
 static const std::string LEVEL_NAMES[] = {"none", "basic", "advanced", "expert"};
 
-static const spells::SchoolInfo SCHOOL[4] =
+const spells::SchoolInfo SCHOOL[4] =
 {
 	{
 		ESpellSchool::AIR,
-		BonusType::AIR_SPELL_DMG_PREMY,
 		BonusType::AIR_IMMUNITY,
-		"air",
-		SecondarySkill::AIR_MAGIC,
-		BonusType::AIR_SPELLS
+		"air"
 	},
 	{
 		ESpellSchool::FIRE,
-		BonusType::FIRE_SPELL_DMG_PREMY,
 		BonusType::FIRE_IMMUNITY,
-		"fire",
-		SecondarySkill::FIRE_MAGIC,
-		BonusType::FIRE_SPELLS
+		"fire"
 	},
 	{
 		ESpellSchool::WATER,
-		BonusType::WATER_SPELL_DMG_PREMY,
 		BonusType::WATER_IMMUNITY,
-		"water",
-		SecondarySkill::WATER_MAGIC,
-		BonusType::WATER_SPELLS
+		"water"
 	},
 	{
 		ESpellSchool::EARTH,
-		BonusType::EARTH_SPELL_DMG_PREMY,
 		BonusType::EARTH_IMMUNITY,
-		"earth",
-		SecondarySkill::EARTH_MAGIC,
-		BonusType::EARTH_SPELLS
+		"earth"
 	}
 };
 
 //order as described in http://bugs.vcmi.eu/view.php?id=91
-static const ESpellSchool SCHOOL_ORDER[4] =
+static const SpellSchool SCHOOL_ORDER[4] =
 {
 	ESpellSchool::AIR,  //=0
 	ESpellSchool::FIRE, //=1
@@ -162,9 +150,9 @@ spells::AimType CSpell::getTargetType() const
 void CSpell::forEachSchool(const std::function<void(const spells::SchoolInfo &, bool &)>& cb) const
 {
 	bool stop = false;
-	for(ESpellSchool iter : SpellConfig::SCHOOL_ORDER)
+	for(auto iter : SpellConfig::SCHOOL_ORDER)
 	{
-		const spells::SchoolInfo & cnf = SpellConfig::SCHOOL[static_cast<ui8>(iter)];
+		const spells::SchoolInfo & cnf = SpellConfig::SCHOOL[iter];
 		if(school.at(cnf.id))
 		{
 			cb(cnf, stop);
@@ -393,15 +381,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 spells::SchoolInfo & cnf, bool & stop)
 		{
-			if(bearer->hasBonusOfType(BonusType::SPELL_DAMAGE_REDUCTION, static_cast<ui8>(cnf.id)))
+			if(bearer->hasBonusOfType(BonusType::SPELL_DAMAGE_REDUCTION, cnf.id))
 			{
-				ret *= 100 - bearer->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, static_cast<ui8>(cnf.id));
+				ret *= 100 - bearer->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, cnf.id);
 				ret /= 100;
 				stop = true; //only bonus from one school is used
 			}
 		});
 
-		CSelector selector = Selector::type()(BonusType::SPELL_DAMAGE_REDUCTION).And(Selector::subtype()(-1));
+		CSelector selector = Selector::typeSubtype(BonusType::SPELL_DAMAGE_REDUCTION, SpellSchool(ESpellSchool::ANY));
 
 		//general spell dmg reduction, works only on magical effects
 		if(bearer->hasBonus(selector) && isMagical())

lib/spells/CSpellHandler.h

@@ -43,16 +43,18 @@ class IBattleCast;
 
 struct SchoolInfo
 {
-	ESpellSchool id; //backlink
-	BonusType damagePremyBonus;
+	SpellSchool id; //backlink
 	BonusType immunityBonus;
 	std::string jsonName;
-	SecondarySkill::ESecondarySkill skill;
-	BonusType knoledgeBonus;
 };
 
 }
 
+namespace SpellConfig
+{
+	extern const spells::SchoolInfo SCHOOL[4];
+}
+
 enum class VerticalPosition : ui8{TOP, CENTER, BOTTOM};
 
 class DLL_LINKAGE CSpell : public spells::Spell
@@ -188,7 +190,7 @@ public:
 
 	si32 level;
 
-	std::map<ESpellSchool, bool> school;
+	std::map<SpellSchool, bool> school;
 
 	si32 power; //spell's power
 

lib/spells/TargetCondition.cpp

@@ -89,6 +89,18 @@ private:
 	si32 maxVal = std::numeric_limits<si32>::max();
 };
 
+class ResistanceCondition : public TargetConditionItemBase
+{
+protected:
+	bool check(const Mechanics * m, const battle::Unit * target) const override
+	{
+		if(m->isPositiveSpell()) //Always pass on positive
+			return true;
+
+		return target->magicResistance() < 100;
+	}
+};
+
 class CreatureCondition : public TargetConditionItemBase
 {
 public:
@@ -279,6 +291,10 @@ protected:
 	{
 		const bool battleWideNegation = target->hasBonusOfType(BonusType::NEGATE_ALL_NATURAL_IMMUNITIES, 0);
 		const bool heroNegation = target->hasBonusOfType(BonusType::NEGATE_ALL_NATURAL_IMMUNITIES, 1);
+		//Non-magical effects is not affected by orb of vulnerability
+		if(!m->isMagicalEffect())
+			return false;
+
 		//anyone can cast on artifact holder`s stacks
 		if(heroNegation)
 		{
@@ -315,6 +331,12 @@ public:
 		return elementalCondition;
 	}
 
+	Object createResistance() const override
+	{
+		static auto elementalCondition = std::make_shared<ResistanceCondition>();
+		return elementalCondition;
+	}
+
 	Object createNormalLevel() const override
 	{
 		static std::shared_ptr<TargetConditionItem> nlCondition = std::make_shared<NormalLevelCondition>();
@@ -340,8 +362,8 @@ public:
 			auto params = BonusParams(identifier, "", -1);
 			if(params.isConverted)
 			{
-				if(params.valRelevant)
-					return std::make_shared<SelectorCondition>(params.toSelector(), params.val, params.val);
+				if(params.val)
+					return std::make_shared<SelectorCondition>(params.toSelector(), *params.val, *params.val);
 				return std::make_shared<SelectorCondition>(params.toSelector());
 			}
 
@@ -447,6 +469,7 @@ void TargetCondition::serializeJson(JsonSerializeFormat & handler, const ItemFac
 	absolute.push_back(itemFactory->createAbsoluteSpell());
 	absolute.push_back(itemFactory->createAbsoluteLevel());
 	normal.push_back(itemFactory->createElemental());
+	normal.push_back(itemFactory->createResistance());
 	normal.push_back(itemFactory->createNormalLevel());
 	normal.push_back(itemFactory->createNormalSpell());
 	negation.push_back(itemFactory->createReceptiveFeature());

lib/spells/TargetCondition.h

@@ -49,6 +49,7 @@ public:
 	virtual Object createElemental() const = 0;
 	virtual Object createNormalLevel() const = 0;
 	virtual Object createNormalSpell() const = 0;
+	virtual Object createResistance() const = 0;
 
 	virtual Object createConfigurable(std::string scope, std::string type, std::string identifier) const = 0;
 	virtual Object createFromJsonStruct(const JsonNode & jsonStruct) const = 0;

lib/spells/effects/Damage.cpp

@@ -11,6 +11,7 @@
 
 #include "Damage.h"
 #include "Registry.h"
+#include "../CSpellHandler.h"
 #include "../ISpellMechanics.h"
 
 #include "../../NetPacks.h"
@@ -20,6 +21,8 @@
 #include "../../CGeneralTextHandler.h"
 #include "../../serializer/JsonSerializeFormat.h"
 
+#include <vcmi/spells/Spell.h>
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 namespace spells
@@ -82,16 +85,14 @@ 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
 	//elemental immunity for damage
-	auto filter = m->getElementalImmunity();
-
-	for(auto element : filter)
+	m->getSpell()->forEachSchool([&](const SchoolInfo & cnf, bool & stop)
 	{
-		if(!m->isPositiveSpell() && unit->hasBonusOfType(element, 2))
-			return false;
-	}
+		isImmune |= (unit->getBonusBearer()->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, cnf.id) >= 100); //100% reduction is immunity
+	});
 
-	return true;
+	return !isImmune;
 }
 
 void Damage::serializeJsonUnitEffect(JsonSerializeFormat & handler)

scripting/lua/LuaStack.h

@@ -351,6 +351,14 @@ public:
 		return 1;
 	}
 
+	template<std::size_t T>
+	static int quickRetInt(lua_State * L, const std::bitset<T> & value)
+	{
+		lua_settop(L, 0);
+		lua_pushinteger(L, static_cast<int32_t>(value.to_ulong()));
+		return 1;
+	}
+
 	static int quickRetStr(lua_State * L, const std::string & value)
 	{
 		lua_settop(L, 0);

scripting/lua/api/BonusSystem.cpp

@@ -11,6 +11,8 @@
 
 #include "BonusSystem.h"
 
+#include "../../../lib/JsonNode.h"
+
 #include "../../../lib/bonuses/BonusList.h"
 #include "../../../lib/bonuses/Bonus.h"
 #include "../../../lib/bonuses/IBonusBearer.h"
@@ -155,8 +157,8 @@ int BonusProxy::toJsonNode(lua_State * L)
 	return 1;
 }
 
-template <typename T>
-static void publishMap(lua_State * L, const T & map)
+template <typename T, typename N>
+static void publishMap(lua_State * L, const std::map<T , N> & map)
 {
 	for(auto & p : map)
 	{
@@ -169,6 +171,20 @@ static void publishMap(lua_State * L, const T & map)
 	}
 }
 
+template <typename T, std::size_t N>
+static void publishMap(lua_State * L, const std::map<T , std::bitset<N>> & map)
+{
+	for(auto & p : map)
+	{
+		const std::string & name = p.first;
+		auto id = static_cast<int32_t>(p.second.to_ulong());
+
+		lua_pushstring(L, name.c_str());
+		lua_pushinteger(L, id);
+		lua_rawset(L, -3);
+	}
+}
+
 void BonusProxy::adjustStaticTable(lua_State * L) const
 {
 	publishMap(L, bonusNameMap);

test/CMakeLists.txt

@@ -67,6 +67,7 @@ set(test_SRCS
  		spells/targetConditions/NormalLevelConditionTest.cpp
  		spells/targetConditions/NormalSpellConditionTest.cpp
  		spells/targetConditions/ReceptiveFeatureConditionTest.cpp
+		spells/targetConditions/ResistanceConditionTest.cpp
  		spells/targetConditions/SpellEffectConditionTest.cpp
  		spells/targetConditions/TargetConditionItemFixture.cpp
 		

test/mock/mock_battle_Unit.h

@@ -33,6 +33,9 @@ public:
 	MOCK_CONST_METHOD0(manaLimit, int32_t());
 	MOCK_CONST_METHOD0(getHeroCaster, CGHeroInstance*());
 
+	//ACreature
+	MOCK_CONST_METHOD0(magicResistance, int32_t());
+
 	MOCK_CONST_METHOD0(unitBaseAmount, int32_t());
 	MOCK_CONST_METHOD0(unitId, uint32_t());
 	MOCK_CONST_METHOD0(unitSide, ui8());

test/mock/mock_spells_Spell.h

@@ -11,6 +11,7 @@
 #pragma once
 
 #include <vcmi/spells/Spell.h>
+#include "GameConstants.h"
 
 namespace spells
 {

test/spells/AbilityCasterTest.cpp

@@ -56,7 +56,7 @@ TEST_F(AbilityCasterTest, MagicAbilityAffectedByGenericBonus)
 {
 	EXPECT_CALL(spellMock, getLevel()).WillRepeatedly(Return(1));
 
-	casterBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::MAGIC_SCHOOL_SKILL, BonusSource::OTHER, 2, 0, 0));
+	casterBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::MAGIC_SCHOOL_SKILL, BonusSource::OTHER, 2, 0, SpellSchool(ESpellSchool::ANY)));
 
 	EXPECT_CALL(actualCaster, getAllBonuses(_, _, _, _)).Times(AtLeast(1));
 	EXPECT_CALL(actualCaster, getTreeVersion()).Times(AtLeast(0));
@@ -70,7 +70,7 @@ TEST_F(AbilityCasterTest, MagicAbilityIngoresSchoolBonus)
 {
 	EXPECT_CALL(spellMock, getLevel()).WillRepeatedly(Return(1));
 
-	casterBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::MAGIC_SCHOOL_SKILL, BonusSource::OTHER, 2, 0, 1));
+	casterBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::MAGIC_SCHOOL_SKILL, BonusSource::OTHER, 2, 0, SpellSchool(ESpellSchool::AIR)));
 
 	EXPECT_CALL(actualCaster, getAllBonuses(_, _, _, _)).Times(AtLeast(1));
 	EXPECT_CALL(actualCaster, getTreeVersion()).Times(AtLeast(0));

test/spells/TargetConditionTest.cpp

@@ -41,6 +41,7 @@ public:
 	MOCK_CONST_METHOD0(createAbsoluteLevel, Object());
 	MOCK_CONST_METHOD0(createAbsoluteSpell, Object());
 	MOCK_CONST_METHOD0(createElemental, Object());
+	MOCK_CONST_METHOD0(createResistance, Object());
 	MOCK_CONST_METHOD0(createNormalLevel, Object());
 	MOCK_CONST_METHOD0(createNormalSpell, Object());
 	MOCK_CONST_METHOD1(createFromJsonStruct, Object(const JsonNode &));
@@ -74,6 +75,7 @@ public:
 		ON_CALL(factoryMock, createAbsoluteLevel()).WillByDefault(Return(itemStub));
 		ON_CALL(factoryMock, createAbsoluteSpell()).WillByDefault(Return(itemStub));
 		ON_CALL(factoryMock, createElemental()).WillByDefault(Return(itemStub));
+		ON_CALL(factoryMock, createResistance()).WillByDefault(Return(itemStub));
 		ON_CALL(factoryMock, createNormalLevel()).WillByDefault(Return(itemStub));
 		ON_CALL(factoryMock, createNormalSpell()).WillByDefault(Return(itemStub));
 
@@ -139,6 +141,7 @@ TEST_F(TargetConditionTest, CreatesSpecialConditions)
 	EXPECT_CALL(factoryMock, createAbsoluteLevel()).Times(1);
 	EXPECT_CALL(factoryMock, createAbsoluteSpell()).Times(1);
 	EXPECT_CALL(factoryMock, createElemental()).Times(1);
+	EXPECT_CALL(factoryMock, createResistance()).Times(1);
 	EXPECT_CALL(factoryMock, createNormalLevel()).Times(1);
 	EXPECT_CALL(factoryMock, createNormalSpell()).Times(1);
 

test/spells/targetConditions/BonusConditionTest.cpp

@@ -42,7 +42,7 @@ TEST_F(BonusConditionTest, ImmuneByDefault)
 TEST_F(BonusConditionTest, ReceptiveIfMatchesType)
 {
 	setDefaultExpectations();
-	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::DIRECT_DAMAGE_IMMUNITY, BonusSource::OTHER, 0, 0));
+	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::SPELL_DAMAGE_REDUCTION, BonusSource::OTHER, 100, 0));
 	EXPECT_TRUE(subject->isReceptive(&mechanicsMock, &unitMock));
 }
 

test/spells/targetConditions/ImmunityNegationConditionTest.cpp

@@ -20,16 +20,19 @@ namespace test
 using namespace ::spells;
 using namespace ::testing;
 
-class ImmunityNegationConditionTest : public TargetConditionItemTest, public WithParamInterface<bool>
+class ImmunityNegationConditionTest : public TargetConditionItemTest, public WithParamInterface<std::tuple<bool, bool>>
 {
 public:
 	bool ownerMatches;
+	bool isMagicalEffect;
 
 	void setDefaultExpectations()
 	{
-		ownerMatches = GetParam();
+		ownerMatches = ::testing::get<0>(GetParam());
+		isMagicalEffect = ::testing::get<1>(GetParam());
 		EXPECT_CALL(unitMock, getAllBonuses(_, _, _, _)).Times(AtLeast(0));
 		EXPECT_CALL(unitMock, getTreeVersion()).Times(AtLeast(0));
+		EXPECT_CALL(mechanicsMock, isMagicalEffect()).Times(AtLeast(0)).WillRepeatedly(Return(isMagicalEffect));
 		EXPECT_CALL(mechanicsMock, ownerMatches(Eq(&unitMock), Field(&boost::logic::tribool::value, boost::logic::tribool::false_value))).WillRepeatedly(Return(ownerMatches));
 	}
 
@@ -56,7 +59,7 @@ TEST_P(ImmunityNegationConditionTest, WithHeroNegation)
 
 	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::NEGATE_ALL_NATURAL_IMMUNITIES, BonusSource::OTHER, 0, 0, 1));
 
-	EXPECT_TRUE(subject->isReceptive(&mechanicsMock, &unitMock));
+	EXPECT_EQ(isMagicalEffect, subject->isReceptive(&mechanicsMock, &unitMock));
 }
 
 TEST_P(ImmunityNegationConditionTest, WithBattleWideNegation)
@@ -66,14 +69,18 @@ TEST_P(ImmunityNegationConditionTest, WithBattleWideNegation)
 	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::NEGATE_ALL_NATURAL_IMMUNITIES, BonusSource::OTHER, 0, 0, 0));
 
 	//This should return if ownerMatches, because anyone should cast onto owner's stacks, but not on enemyStacks
-	EXPECT_EQ(ownerMatches, subject->isReceptive(&mechanicsMock, &unitMock));
+	EXPECT_EQ(ownerMatches && isMagicalEffect, subject->isReceptive(&mechanicsMock, &unitMock));
 }
 
 INSTANTIATE_TEST_SUITE_P
 (
 	ByUnitOwner,
 	ImmunityNegationConditionTest,
-	Values(false, true)
+	Combine
+	(
+		Values(false, true),
+		Values(false, true)
+	)
 );
 
 }

test/spells/targetConditions/NormalLevelConditionTest.cpp

@@ -17,14 +17,17 @@ using namespace ::spells;
 using namespace ::testing;
 
 
-class NormalLevelConditionTest : public TargetConditionItemTest
+class NormalLevelConditionTest : public TargetConditionItemTest, public WithParamInterface<bool>
 {
 public:
+	bool isMagicalEffect;
 
 	void setDefaultExpectations()
 	{
-		EXPECT_CALL(mechanicsMock, isMagicalEffect()).WillRepeatedly(Return(true));
-		EXPECT_CALL(unitMock, getAllBonuses(_, _, _, _)).Times(AtLeast(1));
+		isMagicalEffect = GetParam();
+		EXPECT_CALL(mechanicsMock, isMagicalEffect()).WillRepeatedly(Return(isMagicalEffect));
+		if(isMagicalEffect)
+			EXPECT_CALL(unitMock, getAllBonuses(_, _, _, _)).Times(AtLeast(1));
 		EXPECT_CALL(unitMock, getTreeVersion()).Times(AtLeast(0));
 	}
 
@@ -36,43 +39,53 @@ public:
 	}
 };
 
-TEST_F(NormalLevelConditionTest, DefaultForAbility)
+TEST_P(NormalLevelConditionTest, DefaultForAbility)
 {
 	setDefaultExpectations();
 	EXPECT_CALL(mechanicsMock, getSpellLevel()).WillRepeatedly(Return(0));
 	EXPECT_TRUE(subject->isReceptive(&mechanicsMock, &unitMock));
 }
 
-TEST_F(NormalLevelConditionTest, DefaultForNormal)
+TEST_P(NormalLevelConditionTest, DefaultForNormal)
 {
 	setDefaultExpectations();
 	EXPECT_CALL(mechanicsMock, getSpellLevel()).WillRepeatedly(Return(1));
 	EXPECT_TRUE(subject->isReceptive(&mechanicsMock, &unitMock));
 }
 
-TEST_F(NormalLevelConditionTest, ReceptiveNormal)
+TEST_P(NormalLevelConditionTest, ReceptiveNormal)
 {
 	setDefaultExpectations();
 	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::LEVEL_SPELL_IMMUNITY, BonusSource::OTHER, 3, 0));
-	EXPECT_CALL(mechanicsMock, getSpellLevel()).Times(AtLeast(1)).WillRepeatedly(Return(4));
+	if(isMagicalEffect)
+		EXPECT_CALL(mechanicsMock, getSpellLevel()).Times(AtLeast(1)).WillRepeatedly(Return(4));
 	EXPECT_TRUE(subject->isReceptive(&mechanicsMock, &unitMock));
 }
 
 //TODO: this tests covers fact that creature abilities ignored (by spell level == 0), should this be done by ability flag or by cast mode?
-TEST_F(NormalLevelConditionTest, ReceptiveAbility)
+TEST_P(NormalLevelConditionTest, ReceptiveAbility)
 {
 	setDefaultExpectations();
 	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::LEVEL_SPELL_IMMUNITY, BonusSource::OTHER, 5, 0));
-	EXPECT_CALL(mechanicsMock, getSpellLevel()).Times(AtLeast(1)).WillRepeatedly(Return(0));
+	if(isMagicalEffect)
+		EXPECT_CALL(mechanicsMock, getSpellLevel()).Times(AtLeast(1)).WillRepeatedly(Return(0));
 	EXPECT_TRUE(subject->isReceptive(&mechanicsMock, &unitMock));
 }
 
-TEST_F(NormalLevelConditionTest, ImmuneNormal)
+TEST_P(NormalLevelConditionTest, ImmuneNormal)
 {
 	setDefaultExpectations();
 	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::LEVEL_SPELL_IMMUNITY, BonusSource::OTHER, 4, 0));
-	EXPECT_CALL(mechanicsMock, getSpellLevel()).Times(AtLeast(1)).WillRepeatedly(Return(2));
-	EXPECT_FALSE(subject->isReceptive(&mechanicsMock, &unitMock));
+	if(isMagicalEffect)
+		EXPECT_CALL(mechanicsMock, getSpellLevel()).Times(AtLeast(1)).WillRepeatedly(Return(2));
+	EXPECT_EQ(!isMagicalEffect, subject->isReceptive(&mechanicsMock, &unitMock));
 }
 
+INSTANTIATE_TEST_SUITE_P
+(
+	ByMagicalEffect,
+	NormalLevelConditionTest,
+	Values(false, true)
+);
+
 }

test/spells/targetConditions/ResistanceConditionTest.cpp

@@ -0,0 +1,72 @@
+/*
+ * ElementalConditionTest.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#include "StdInc.h"
+
+#include "TargetConditionItemFixture.h"
+
+namespace test
+{
+using namespace ::spells;
+using namespace ::testing;
+
+class ResistanceConditionTest : public TargetConditionItemTest, public WithParamInterface<bool>
+{
+public:
+	bool isPositive;
+	void setDefaultExpectations()
+	{
+		EXPECT_CALL(unitMock, getTreeVersion()).Times(AtLeast(0));
+		EXPECT_CALL(mechanicsMock, isPositiveSpell()).WillRepeatedly(Return(isPositive));
+	}
+
+	void SetUp() override
+	{
+		TargetConditionItemTest::SetUp();
+		subject = TargetConditionItemFactory::getDefault()->createResistance();
+
+		isPositive = GetParam();
+	}
+};
+
+TEST_P(ResistanceConditionTest, ReceptiveIfNoBonus)
+{
+	setDefaultExpectations();
+
+	EXPECT_CALL(unitMock, magicResistance()).Times(AtLeast(0)).WillRepeatedly(Return(0));
+
+	EXPECT_TRUE(subject->isReceptive(&mechanicsMock, &unitMock));
+}
+
+TEST_P(ResistanceConditionTest, DependsOnPositivness)
+{
+	setDefaultExpectations();
+
+	EXPECT_CALL(unitMock, magicResistance()).Times(AtLeast(0)).WillRepeatedly(Return(100));
+
+	EXPECT_EQ(isPositive, subject->isReceptive(&mechanicsMock, &unitMock));
+}
+
+TEST_P(ResistanceConditionTest, ReceptiveIfResistanceIsLessThanHundred)
+{
+	setDefaultExpectations();
+
+	EXPECT_CALL(unitMock, magicResistance()).Times(AtLeast(0)).WillRepeatedly(Return(99));
+
+	EXPECT_TRUE(subject->isReceptive(&mechanicsMock, &unitMock));
+}
+
+INSTANTIATE_TEST_SUITE_P
+(
+	ByPositiveness,
+	ResistanceConditionTest,
+	Values(false, true)
+);
+
+}