Merge pull request #2632 from rilian-la-te/resist-rework-pt1

Spell resistance rework: part 1

client/battle/BattleStacksController.cpp

@@ -271,7 +271,16 @@ std::shared_ptr<IImage> BattleStacksController::getStackAmountBox(const CStack *
 	int effectsPositivness = 0;
 
 	for(const auto & spellID : activeSpells)
-		effectsPositivness += CGI->spellh->objects.at(spellID)->positiveness;
+	{
+		auto positiveness = CGI->spells()->getByIndex(spellID)->getPositiveness();
+		if(!boost::logic::indeterminate(positiveness))
+		{
+			if(positiveness)
+				effectsPositivness++;
+			else
+				effectsPositivness--;
+		}
+	}
 
 	if (effectsPositivness > 0)
 		return amountPositive;

client/windows/CSpellWindow.cpp

@@ -74,9 +74,9 @@ class SpellbookSpellSorter
 public:
 	bool operator()(const CSpell * A, const CSpell * B)
 	{
-		if(A->level < B->level)
+		if(A->getLevel() < B->getLevel())
 			return true;
-		if(A->level > B->level)
+		if(A->getLevel() > B->getLevel())
 			return false;
 
 
@@ -122,9 +122,9 @@ CSpellWindow::CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _m
 
 		++sitesPerOurTab[4];
 
-		spell->forEachSchool([&sitesPerOurTab](const spells::SchoolInfo & school, bool & stop)
+		spell->forEachSchool([&sitesPerOurTab](const SpellSchool & school, bool & stop)
 		{
-			++sitesPerOurTab[(ui8)school.id];
+			++sitesPerOurTab[school];
 		});
 	}
 	if(sitesPerTabAdv[4] % 12 == 0)
@@ -562,7 +562,7 @@ void CSpellWindow::SpellArea::hover(bool on)
 	if(mySpell)
 	{
 		if(on)
-			owner->statusBar->write(boost::str(boost::format("%s (%s)") % mySpell->getNameTranslated() % CGI->generaltexth->allTexts[171+mySpell->level]));
+			owner->statusBar->write(boost::str(boost::format("%s (%s)") % mySpell->getNameTranslated() % CGI->generaltexth->allTexts[171+mySpell->getLevel()]));
 		else
 			owner->statusBar->clear();
 	}
@@ -609,12 +609,12 @@ void CSpellWindow::SpellArea::setSpell(const CSpell * spell)
 		if(schoolLevel > 0)
 		{
 			boost::format fmt("%s/%s");
-			fmt % CGI->generaltexth->allTexts[171 + mySpell->level];
+			fmt % CGI->generaltexth->allTexts[171 + mySpell->getLevel()];
 			fmt % CGI->generaltexth->levels[3+(schoolLevel-1)];//lines 4-6
 			level->setText(fmt.str());
 		}
 		else
-			level->setText(CGI->generaltexth->allTexts[171 + mySpell->level]);
+			level->setText(CGI->generaltexth->allTexts[171 + mySpell->getLevel()]);
 
 		cost->color = secondLineColor;
 		boost::format costfmt("%s: %d");

config/bonuses.json

@@ -19,14 +19,6 @@
 		}
 	},
 
-	"AIR_IMMUNITY":
-	{
-		"graphics":
-		{
-			"icon":  "zvs/Lib1.res/E_SPAIR1"
-		}
-	},
-
 	"ATTACKS_ALL_ADJACENT":
 	{
 		"graphics":
@@ -137,14 +129,6 @@
 		"hidden": true
 	},
 
-	"EARTH_IMMUNITY":
-	{
-		"graphics":
-		{
-			"icon":  "zvs/Lib1.res/E_SPEATH1"
-		}
-	},
-
 	"ENCHANTER":
 	{
 		"graphics":
@@ -169,14 +153,6 @@
 		}
 	},
 
-	"FIRE_IMMUNITY":
-	{
-		"graphics":
-		{
-			"icon":  "zvs/Lib1.res/E_SPFIRE1"
-		}
-	},
-
 	"FIRE_SHIELD":
 	{
 		"graphics":
@@ -585,14 +561,6 @@
 		"hidden": true
 	},
 	
-	"WATER_IMMUNITY":
-	{
-		"graphics":
-		{
-			"icon":  "zvs/Lib1.res/E_SPWATER1"
-		}
-	},
-
 	"WIDE_BREATH":
 	{
 		"graphics":

config/creatures/conflux.json

@@ -154,8 +154,8 @@
 			},
 			"immuneToFire" : 
 			{
-				"type" : "FIRE_IMMUNITY",
-				"subtype" : 0
+				"type" : "SPELL_SCHOOL_IMMUNITY",
+				"subtype" : "spellSchool.fire"
 			},
 			"frostRingVulnerablity" :
 			{
@@ -246,10 +246,15 @@
 				"subtype" : "spell.armageddon",
 				"val" : 100
 			},
-			"immuneToWater" :
+			"immuneToIceBolt" :
 			{
-				"type" : "WATER_IMMUNITY",
-				"subtype" : 2 //immune to damage spells only
+				"type" : "SPELL_IMMUNITY",
+				"subtype" : "spell.iceBolt"
+			},
+			"immuneToFrostRing" :
+			{
+				"type" : "SPELL_IMMUNITY",
+				"subtype" : "spell.frostRing"
 			},
 			"oppositeFire" :
 			{
@@ -440,10 +445,15 @@
 				"subtype" : "spell.armageddon",
 				"val" : 100
 			},
-			"immuneToWater" :
+			"immuneToIceBolt" :
 			{
-				"type" : "WATER_IMMUNITY",
-				"subtype" : 2 //immune to damage spells only
+				"type" : "SPELL_IMMUNITY",
+				"subtype" : "spell.iceBolt"
+			},
+			"immuneToFrostRing" :
+			{
+				"type" : "SPELL_IMMUNITY",
+				"subtype" : "spell.frostRing"
 			},
 			"oppositeFire" :
 			{
@@ -661,8 +671,8 @@
 			},
 			"immuneToFire" : 
 			{
-				"type" : "FIRE_IMMUNITY",
-				"subtype" : 0
+				"type" : "SPELL_SCHOOL_IMMUNITY",
+				"subtype" : "spellSchool.fire"
 			},
 			"frostRingVulnerablity" :
 			{
@@ -732,8 +742,8 @@
 		{
 			"immuneToFire" :
 			{
-				"type" : "FIRE_IMMUNITY",
-				"subtype" : 0 //this IS important
+				"type" : "SPELL_SCHOOL_IMMUNITY",
+				"subtype" : "spellSchool.fire"
 			}
 		},
 		"graphics" :
@@ -763,8 +773,8 @@
 			},
 			"immuneToFire" :
 			{
-				"type" : "FIRE_IMMUNITY",
-				"subtype" : 0 //this IS important
+				"type" : "SPELL_SCHOOL_IMMUNITY",
+				"subtype" : "spellSchool.fire"
 			},
 			"rebirth" : 
 			{

config/creatures/inferno.json

@@ -272,8 +272,8 @@
 			},
 			"immuneToFire" :
 			{
-				"type" : "FIRE_IMMUNITY",
-				"subtype" : 0
+				"type" : "SPELL_SCHOOL_IMMUNITY",
+				"subtype" : "spellSchool.fire"
 			}
 		},
 		"upgrades": ["efreetSultan"],
@@ -315,8 +315,8 @@
 			},
 			"immuneToFire" :
 			{
-				"type" : "FIRE_IMMUNITY",
-				"subtype" : 0
+				"type" : "SPELL_SCHOOL_IMMUNITY",
+				"subtype" : "spellSchool.fire"
 			},
 			"fireShield" :
 			{

include/vcmi/spells/Spell.h

@@ -19,13 +19,12 @@ enum class ESpellSchool: int8_t;
 
 namespace spells
 {
-struct SchoolInfo;
 class Caster;
 
 class DLL_LINKAGE Spell: public EntityT<SpellID>
 {
 public:
-	using SchoolCallback = std::function<void(const SchoolInfo &, bool &)>;
+	using SchoolCallback = std::function<void(const Identifier<ESpellSchool> &, bool &)>;
 
 	///calculate spell damage on stack taking caster`s secondary skills into account
 	virtual int64_t calculateDamage(const Caster * caster) const = 0;
@@ -44,7 +43,7 @@ public:
 	virtual bool isSpecial() const = 0;
 	virtual bool isMagical() const = 0; //Should this spell considered as magical effect or as ability (like dendroid's bind)
 
-	virtual bool hasSchool(ESpellSchool school) const = 0;
+	virtual bool hasSchool(Identifier<ESpellSchool> school) const = 0;
 	virtual void forEachSchool(const SchoolCallback & cb) const = 0;
 	virtual const std::string & getCastSound() const = 0;
 	virtual int32_t getCost(const int32_t skillLevel) const = 0;

lib/CBonusTypeHandler.cpp

@@ -99,60 +99,66 @@ std::string CBonusTypeHandler::bonusToGraphics(const std::shared_ptr<Bonus> & bo
 		fileName = sp->getIconImmune();
 		break;
 	}
-	case BonusType::FIRE_IMMUNITY:
+	case BonusType::SPELL_DAMAGE_REDUCTION: //Spell damage reduction for all schools
+	{
 		switch(bonus->subtype)
 		{
-		case 0:
-			fileName = "E_SPFIRE.bmp";
-			break;//all
-		case 1:
-			fileName = "E_SPFIRE1.bmp";
-			break;//not positive
-		case 2:
+		case SpellSchool(ESpellSchool::ANY):
+			fileName = "E_GOLEM.bmp";
+			break;	
+		case SpellSchool(ESpellSchool::AIR):
+			fileName = "E_LIGHT.bmp";
+			break;
+		case SpellSchool(ESpellSchool::FIRE):
 			fileName = "E_FIRE.bmp";
-			break;//direct damage
+			break;
+		case SpellSchool(ESpellSchool::WATER):
+			fileName = "E_COLD.bmp";
+			break;
+		case SpellSchool(ESpellSchool::EARTH):
+			fileName = "E_SPEATH1.bmp"; //No separate icon for earth damage
+			break;
 		}
 		break;
-	case BonusType::WATER_IMMUNITY:
+	}
+	case BonusType::SPELL_SCHOOL_IMMUNITY: //for all school
+	{
 		switch(bonus->subtype)
 		{
-		case 0:
+		case SpellSchool(ESpellSchool::AIR):
+			fileName = "E_SPAIR.bmp";
+			break;
+		case SpellSchool(ESpellSchool::FIRE):
+			fileName = "E_SPFIRE.bmp";
+			break;
+		case SpellSchool(ESpellSchool::WATER):
 			fileName = "E_SPWATER.bmp";
-			break;//all
-		case 1:
-			fileName = "E_SPWATER1.bmp";
-			break;//not positive
-		case 2:
-			fileName = "E_SPCOLD.bmp";
-			break;//direct damage
+			break;
+		case SpellSchool(ESpellSchool::EARTH):
+			fileName = "E_SPEATH.bmp";
+			break;
 		}
 		break;
-	case BonusType::AIR_IMMUNITY:
+	}
+	case BonusType::NEGATIVE_EFFECTS_IMMUNITY:
+	{
 		switch(bonus->subtype)
 		{
-		case 0:
-			fileName = "E_SPAIR.bmp";
-			break;//all
-		case 1:
+		case SpellSchool(ESpellSchool::AIR):
 			fileName = "E_SPAIR1.bmp";
-			break;//not positive
-		case 2:
-			fileName = "E_LIGHT.bmp";
-			break;//direct damage
-		}
-		break;
-	case BonusType::EARTH_IMMUNITY:
-		switch(bonus->subtype)
-		{
-		case 0:
-			fileName = "E_SPEATH.bmp";
-			break;//all
-		case 1:
-		case 2://no specific icon for direct damage immunity
+			break;
+		case SpellSchool(ESpellSchool::FIRE):
+			fileName = "E_SPFIRE1.bmp";
+			break;
+		case SpellSchool(ESpellSchool::WATER):
+			fileName = "E_SPWATER1.bmp";
+			break;
+		case SpellSchool(ESpellSchool::EARTH):
 			fileName = "E_SPEATH1.bmp";
-			break;//not positive
+			break;
 		}
 		break;
+	}
 	case BonusType::LEVEL_SPELL_IMMUNITY:
 	{
 		if(vstd::iswithin(bonus->val, 1, 5))

lib/CCreatureHandler.cpp

@@ -1181,8 +1181,8 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 				b.val = GameConstants::SPELL_LEVELS; //in case someone adds higher level spells?
 				break;
 			case 'F':
-				b.type = BonusType::FIRE_IMMUNITY;
-				b.subtype = 1; //not positive
+				b.type = BonusType::NEGATIVE_EFFECTS_IMMUNITY;
+				b.subtype = SpellSchool(ESpellSchool::FIRE); 
 				break;
 			case 'O':
 				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
@@ -1190,12 +1190,12 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 				b.val = 100; //Full damage immunity
 				break;
 			case 'f':
-				b.type = BonusType::FIRE_IMMUNITY;
-				b.subtype = 0; //all
+				b.type = BonusType::SPELL_SCHOOL_IMMUNITY;
+				b.subtype = SpellSchool(ESpellSchool::FIRE); 
 				break;
 			case 'C':
-				b.type = BonusType::WATER_IMMUNITY;
-				b.subtype = 1; //not positive
+				b.type = BonusType::NEGATIVE_EFFECTS_IMMUNITY;
+				b.subtype = SpellSchool(ESpellSchool::WATER);
 				break;
 			case 'W':
 				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
@@ -1203,8 +1203,8 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 				b.val = 100; //Full damage immunity
 				break;
 			case 'w':
-				b.type = BonusType::WATER_IMMUNITY;
-				b.subtype = 0; //all
+				b.type = BonusType::SPELL_SCHOOL_IMMUNITY;
+				b.subtype = SpellSchool(ESpellSchool::WATER);
 				break;
 			case 'E':
 				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
@@ -1212,8 +1212,8 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 				b.val = 100; //Full damage immunity
 				break;
 			case 'e':
-				b.type = BonusType::EARTH_IMMUNITY;
-				b.subtype = 0; //all
+				b.type = BonusType::SPELL_SCHOOL_IMMUNITY;
+				b.subtype = SpellSchool(ESpellSchool::EARTH);
 				break;
 			case 'A':
 				b.type = BonusType::SPELL_DAMAGE_REDUCTION;
@@ -1221,8 +1221,8 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 				b.val = 100; //Full damage immunity
 				break;
 			case 'a':
-				b.type = BonusType::AIR_IMMUNITY;
-				b.subtype = 0; //all
+				b.type = BonusType::SPELL_SCHOOL_IMMUNITY;
+				b.subtype = SpellSchool(ESpellSchool::AIR);
 				break;
 			case 'D':
 				b.type = BonusType::SPELL_DAMAGE_REDUCTION;

lib/JsonRandom.cpp

@@ -260,7 +260,7 @@ namespace JsonRandom
 
 			vstd::erase_if(spells, [=](const SpellID & spell)
 			{
-				return !VLC->spellh->getById(spell)->hasSchool(ESpellSchool(schoolID));
+				return !VLC->spellh->getById(spell)->hasSchool(SpellSchool(schoolID));
 			});
 		}
 

lib/bonuses/BonusEnum.h

@@ -74,10 +74,6 @@ class JsonNode;
 	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*/						\
-	BONUS_NAME(WATER_IMMUNITY)							\
-	BONUS_NAME(EARTH_IMMUNITY)							\
-	BONUS_NAME(AIR_IMMUNITY)							\
 	BONUS_NAME(MIND_IMMUNITY)							\
 	BONUS_NAME(FIRE_SHIELD)								\
 	BONUS_NAME(UNDEAD)									\
@@ -175,6 +171,8 @@ class JsonNode;
 	BONUS_NAME(BONUS_DAMAGE_PERCENTAGE) /*If hero can grant conditional damage to creature, value is percentage, subtype is creatureID*/\
 	BONUS_NAME(BONUS_DAMAGE_CHANCE) /*If hero can grant additional damage to creature, value is chance, subtype is creatureID*/\
 	BONUS_NAME(MAX_LEARNABLE_SPELL_LEVEL) /*This can work as wisdom before. val = max learnable spell level*/\
+	BONUS_NAME(SPELL_SCHOOL_IMMUNITY) /*This bonus will work as spell school immunity for all spells, subtype - spell school: 0 - air, 1 - fire, 2 - water, 3 - earth. Any is not handled for reducing overlap from LEVEL_SPELL_IMMUNITY*/\
+	BONUS_NAME(NEGATIVE_EFFECTS_IMMUNITY) /*This bonus will work as spell school immunity for negative effects from spells of school, subtype - spell school: -1 - any, 0 - air, 1 - fire, 2 - water, 3 - earth*/\
 	/* end of list */
 
 

lib/bonuses/BonusParams.cpp

@@ -10,6 +10,7 @@
 
 #include "StdInc.h"
 
+#include "BonusEnum.h"
 #include "BonusParams.h"
 #include "BonusSelector.h"
 
@@ -42,7 +43,11 @@ const std::set<std::string> deprecatedBonusSet = {
 	"FIRE_SPELLS",
 	"AIR_SPELLS",
 	"WATER_SPELLS",
-	"EARTH_SPELLS"
+	"EARTH_SPELLS",
+	"FIRE_IMMUNITY",
+	"AIR_IMMUNITY",
+	"WATER_IMMUNITY",
+	"EARTH_IMMUNITY"
 };
 
 BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSubtypeStr, int deprecatedSubtype):
@@ -261,6 +266,70 @@ BonusParams::BonusParams(std::string deprecatedTypeStr, std::string deprecatedSu
 		type = BonusType::SPELLS_OF_SCHOOL;
 		subtype = SpellSchool(ESpellSchool::EARTH);
 	}
+	else if (deprecatedTypeStr == "AIR_IMMUNITY")
+	{
+		subtype = SpellSchool(ESpellSchool::AIR);
+		switch(deprecatedSubtype)
+		{
+			case 0:
+				type = BonusType::SPELL_SCHOOL_IMMUNITY;
+				break;
+			case 1:
+				type = BonusType::NEGATIVE_EFFECTS_IMMUNITY;
+				break;
+			default:
+				type = BonusType::SPELL_DAMAGE_REDUCTION;
+				val = 100;
+		}
+	}
+	else if (deprecatedTypeStr == "FIRE_IMMUNITY")
+	{
+		subtype = SpellSchool(ESpellSchool::FIRE);
+		switch(deprecatedSubtype)
+		{
+			case 0:
+				type = BonusType::SPELL_SCHOOL_IMMUNITY;
+				break;
+			case 1:
+				type = BonusType::NEGATIVE_EFFECTS_IMMUNITY;
+				break;
+			default:
+				type = BonusType::SPELL_DAMAGE_REDUCTION;
+				val = 100;
+		}
+	}
+	else if (deprecatedTypeStr == "WATER_IMMUNITY")
+	{
+		subtype = SpellSchool(ESpellSchool::WATER);
+		switch(deprecatedSubtype)
+		{
+			case 0:
+				type = BonusType::SPELL_SCHOOL_IMMUNITY;
+				break;
+			case 1:
+				type = BonusType::NEGATIVE_EFFECTS_IMMUNITY;
+				break;
+			default:
+				type = BonusType::SPELL_DAMAGE_REDUCTION;
+				val = 100;
+		}
+	}
+	else if (deprecatedTypeStr == "EARTH_IMMUNITY")
+	{
+		subtype = SpellSchool(ESpellSchool::EARTH);
+		switch(deprecatedSubtype)
+		{
+			case 0:
+				type = BonusType::SPELL_SCHOOL_IMMUNITY;
+				break;
+			case 1:
+				type = BonusType::NEGATIVE_EFFECTS_IMMUNITY;
+				break;
+			default:
+				type = BonusType::SPELL_DAMAGE_REDUCTION;
+				val = 100;
+		}
+	}
 	else
 		isConverted = false;
 }

lib/gameState/CGameState.cpp

@@ -1082,7 +1082,7 @@ void CGameState::initTowns()
 		for(ui32 z=0; z<vti->obligatorySpells.size();z++)
 		{
 			const auto * s = vti->obligatorySpells[z].toSpell();
-			vti->spells[s->level-1].push_back(s->id);
+			vti->spells[s->getLevel()-1].push_back(s->id);
 			vti->possibleSpells -= s->id;
 		}
 		while(!vti->possibleSpells.empty())
@@ -1110,7 +1110,7 @@ void CGameState::initTowns()
 				sel=0;
 
 			const auto * s = vti->possibleSpells[sel].toSpell();
-			vti->spells[s->level-1].push_back(s->id);
+			vti->spells[s->getLevel()-1].push_back(s->id);
 			vti->possibleSpells -= s->id;
 		}
 		vti->possibleSpells.clear();

lib/mapObjects/CGHeroInstance.cpp

@@ -620,14 +620,14 @@ int32_t CGHeroInstance::getSpellSchoolLevel(const spells::Spell * spell, int32_t
 {
 	int32_t skill = -1; //skill level
 
-	spell->forEachSchool([&, this](const spells::SchoolInfo & cnf, bool & stop)
+	spell->forEachSchool([&, this](const SpellSchool & cnf, bool & stop)
 	{
-		int32_t thisSchool = valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, cnf.id); //FIXME: Bonus shouldn't be additive (Witchking Artifacts : Crown of Skies)
+		int32_t thisSchool = valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, cnf); //FIXME: Bonus shouldn't be additive (Witchking Artifacts : Crown of Skies)
 		if(thisSchool > skill)
 		{
 			skill = thisSchool;
 			if(outSelectedSchool)
-				*outSelectedSchool = static_cast<ui8>(cnf.id);
+				*outSelectedSchool = cnf;
 		}
 	});
 
@@ -650,9 +650,9 @@ int64_t CGHeroInstance::getSpellBonus(const spells::Spell * spell, int64_t base,
 
 	int maxSchoolBonus = 0;
 
-	spell->forEachSchool([&maxSchoolBonus, this](const spells::SchoolInfo & cnf, bool & stop)
+	spell->forEachSchool([&maxSchoolBonus, this](const SpellSchool & cnf, bool & stop)
 	{
-		vstd::amax(maxSchoolBonus, valOfBonuses(BonusType::SPELL_DAMAGE, cnf.id));
+		vstd::amax(maxSchoolBonus, valOfBonuses(BonusType::SPELL_DAMAGE, cnf));
 	});
 
 	base = static_cast<int64_t>(base * (100 + maxSchoolBonus) / 100.0);
@@ -739,9 +739,9 @@ bool CGHeroInstance::canCastThisSpell(const spells::Spell * spell) const
 
 	bool schoolBonus = false;
 
-	spell->forEachSchool([this, &schoolBonus](const spells::SchoolInfo & cnf, bool & stop)
+	spell->forEachSchool([this, &schoolBonus](const SpellSchool & cnf, bool & stop)
 	{
-		if(hasBonusOfType(BonusType::SPELLS_OF_SCHOOL, cnf.id))
+		if(hasBonusOfType(BonusType::SPELLS_OF_SCHOOL, cnf))
 		{
 			schoolBonus = stop = true;
 		}

lib/rmg/modificators/TreasurePlacer.cpp

@@ -213,7 +213,7 @@ void TreasurePlacer::addAllPossibleObjects()
 			
 			for(auto spell : VLC->spellh->objects) //spellh size appears to be greater (?)
 			{
-				if(map.isAllowedSpell(spell->id) && spell->level == i + 1)
+				if(map.isAllowedSpell(spell->id) && spell->getLevel() == i + 1)
 				{
 					out.push_back(spell->id);
 				}
@@ -328,7 +328,7 @@ void TreasurePlacer::addAllPossibleObjects()
 			std::vector <CSpell *> spells;
 			for(auto spell : VLC->spellh->objects)
 			{
-				if(map.isAllowedSpell(spell->id) && spell->level == i)
+				if(map.isAllowedSpell(spell->id) && spell->getLevel() == i)
 					spells.push_back(spell);
 			}
 			

lib/spells/CSpellHandler.cpp

@@ -43,22 +43,18 @@ const spells::SchoolInfo SCHOOL[4] =
 {
 	{
 		ESpellSchool::AIR,
-		BonusType::AIR_IMMUNITY,
 		"air"
 	},
 	{
 		ESpellSchool::FIRE,
-		BonusType::FIRE_IMMUNITY,
 		"fire"
 	},
 	{
 		ESpellSchool::WATER,
-		BonusType::WATER_IMMUNITY,
 		"water"
 	},
 	{
 		ESpellSchool::EARTH,
-		BonusType::EARTH_IMMUNITY,
 		"earth"
 	}
 };
@@ -128,7 +124,7 @@ int64_t CSpell::calculateDamage(const spells::Caster * caster) const
 	return caster->getSpellBonus(this, rawDamage, nullptr);
 }
 
-bool CSpell::hasSchool(ESpellSchool which) const
+bool CSpell::hasSchool(SpellSchool which) const
 {
 	return school.count(which) && school.at(which);
 }
@@ -153,7 +149,7 @@ spells::AimType CSpell::getTargetType() const
 	return targetType;
 }
 
-void CSpell::forEachSchool(const std::function<void(const spells::SchoolInfo &, bool &)>& cb) const
+void CSpell::forEachSchool(const std::function<void(const SpellSchool &, bool &)>& cb) const
 {
 	bool stop = false;
 	for(auto iter : SpellConfig::SCHOOL_ORDER)
@@ -161,7 +157,7 @@ void CSpell::forEachSchool(const std::function<void(const spells::SchoolInfo &,
 		const spells::SchoolInfo & cnf = SpellConfig::SCHOOL[iter];
 		if(school.at(cnf.id))
 		{
-			cb(cnf, stop);
+			cb(cnf.id, stop);
 
 			if(stop)
 				break;
@@ -383,24 +379,25 @@ int64_t CSpell::adjustRawDamage(const spells::Caster * caster, const battle::Uni
 	//affected creature-specific part
 	if(nullptr != affectedCreature)
 	{
-		const auto * bearer = affectedCreature;
+		const auto * bearer = affectedCreature->getBonusBearer();
 		//applying protections - when spell has more then one elements, only one protection should be applied (I think)
-		forEachSchool([&](const spells::SchoolInfo & cnf, bool & stop)
+		forEachSchool([&](const SpellSchool & cnf, bool & stop)
 		{
-			if(bearer->hasBonusOfType(BonusType::SPELL_DAMAGE_REDUCTION, cnf.id))
+			if(bearer->hasBonusOfType(BonusType::SPELL_DAMAGE_REDUCTION, cnf))
 			{
-				ret *= 100 - bearer->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, cnf.id);
+				ret *= 100 - bearer->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, cnf);
 				ret /= 100;
 				stop = true; //only bonus from one school is used
 			}
 		});
 
 		CSelector selector = Selector::typeSubtype(BonusType::SPELL_DAMAGE_REDUCTION, SpellSchool(ESpellSchool::ANY));
+		auto cachingStr = "type_SPELL_DAMAGE_REDUCTION_s_ANY";
 
 		//general spell dmg reduction, works only on magical effects
-		if(bearer->hasBonus(selector) && isMagical())
+		if(bearer->hasBonus(selector, cachingStr) && isMagical())
 		{
-			ret *= 100 - bearer->valOfBonuses(selector);
+			ret *= 100 - bearer->valOfBonuses(selector, cachingStr);
 			ret /= 100;
 		}
 

lib/spells/CSpellHandler.h

@@ -44,7 +44,6 @@ class IBattleCast;
 struct SchoolInfo
 {
 	SpellSchool id; //backlink
-	BonusType immunityBonus;
 	std::string jsonName;
 };
 
@@ -188,17 +187,10 @@ public:
 
 	using BTVector = std::vector<BonusType>;
 
-	si32 level;
 
 	std::map<SpellSchool, bool> school;
-
-	si32 power; //spell's power
-
 	std::map<FactionID, si32> probabilities; //% chance to gain for castles
 
-	bool combat; //is this spell combat (true) or adventure (false)
-	bool creatureAbility; //if true, only creatures can use this spell
-	si8 positiveness; //1 if spell is positive for influenced stacks, 0 if it is indifferent, -1 if it's negative
 	bool onlyOnWaterMap; //Spell will be banned on maps without water
 	std::vector<SpellID> counteredSpells; //spells that are removed when effect of this spell is placed on creature (for bless-curse, haste-slow, and similar pairs)
 
@@ -209,14 +201,14 @@ public:
 
 	int64_t calculateDamage(const spells::Caster * caster) const override;
 
-	bool hasSchool(ESpellSchool school) const override;
+	bool hasSchool(SpellSchool school) const override;
 
 	/**
 	 * Calls cb for each school this spell belongs to
 	 *
 	 * Set stop to true to abort looping
 	 */
-	void forEachSchool(const std::function<void(const spells::SchoolInfo &, bool &)> & cb) const override;
+	void forEachSchool(const std::function<void(const SpellSchool &, bool &)> & cb) const override;
 
 	spells::AimType getTargetType() const;
 
@@ -365,6 +357,12 @@ private:
 
 	std::vector<LevelInfo> levels;
 
+	si32 level;
+	si32 power; //spell's power
+	bool combat; //is this spell combat (true) or adventure (false)
+	bool creatureAbility; //if true, only creatures can use this spell
+	si8 positiveness; //1 if spell is positive for influenced stacks, 0 if it is indifferent, -1 if it's negative
+
 	std::unique_ptr<spells::ISpellMechanicsFactory> mechanics;//(!) do not serialize
 	std::unique_ptr<IAdventureSpellMechanics> adventureMechanics;//(!) do not serialize
 };

lib/spells/ISpellMechanics.cpp

@@ -620,18 +620,6 @@ int64_t BaseMechanics::calculateRawEffectValue(int32_t basePowerMultiplier, int3
 	return owner->calculateRawEffectValue(getEffectLevel(), basePowerMultiplier, levelPowerMultiplier);
 }
 
-std::vector<BonusType> BaseMechanics::getElementalImmunity() const
-{
-	std::vector<BonusType> ret;
-
-	owner->forEachSchool([&](const SchoolInfo & cnf, bool & stop)
-	{
-		ret.push_back(cnf.immunityBonus);
-	});
-
-	return ret;
-}
-
 bool BaseMechanics::ownerMatches(const battle::Unit * unit) const
 {
     return ownerMatches(unit, owner->getPositiveness());
@@ -750,7 +738,7 @@ std::unique_ptr<IAdventureSpellMechanics> IAdventureSpellMechanics::createMechan
 	case SpellID::VIEW_AIR:
 		return std::make_unique<ViewAirMechanics>(s);
 	default:
-		return s->combat ? std::unique_ptr<IAdventureSpellMechanics>() : std::make_unique<AdventureSpellMechanics>(s);
+		return s->isCombat() ? std::unique_ptr<IAdventureSpellMechanics>() : std::make_unique<AdventureSpellMechanics>(s);
 	}
 }
 

lib/spells/ISpellMechanics.h

@@ -235,8 +235,6 @@ public:
 	virtual int64_t applySpecificSpellBonus(int64_t value) const = 0;
 	virtual int64_t calculateRawEffectValue(int32_t basePowerMultiplier, int32_t levelPowerMultiplier) const = 0;
 
-	virtual std::vector<BonusType> getElementalImmunity() const = 0;
-
 	//Battle facade
 	virtual bool ownerMatches(const battle::Unit * unit) const = 0;
 	virtual bool ownerMatches(const battle::Unit * unit, const boost::logic::tribool positivness) const = 0;
@@ -296,8 +294,6 @@ public:
 	int64_t applySpecificSpellBonus(int64_t value) const override;
 	int64_t calculateRawEffectValue(int32_t basePowerMultiplier, int32_t levelPowerMultiplier) const override;
 
-	std::vector<BonusType> getElementalImmunity() const override;
-
 	bool ownerMatches(const battle::Unit * unit) const override;
 	bool ownerMatches(const battle::Unit * unit, const boost::logic::tribool positivness) const override;
 

lib/spells/TargetCondition.cpp

@@ -23,6 +23,8 @@
 #include "../serializer/JsonSerializeFormat.h"
 #include "../VCMI_Lib.h"
 
+#include <vcmi/spells/Spell.h>
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 
@@ -173,25 +175,25 @@ protected:
 	bool check(const Mechanics * m, const battle::Unit * target) const override
 	{
 		bool elementalImmune = false;
+		auto bearer = target->getBonusBearer();
 
-		auto filter = m->getElementalImmunity();
-
-		for(auto element : filter)
+		m->getSpell()->forEachSchool([&](const SpellSchool & cnf, bool & stop) 
 		{
-			if(target->hasBonusOfType(element, 0)) //always resist if immune to all spells altogether
+			if (bearer->hasBonusOfType(BonusType::SPELL_SCHOOL_IMMUNITY, cnf))
 			{
 				elementalImmune = true;
-				break;
+				stop = true; //only bonus from one school is used
 			}
 			else if(!m->isPositiveSpell()) //negative or indifferent
 			{
-				if(target->hasBonusOfType(element, 1))
+				if (bearer->hasBonusOfType(BonusType::NEGATIVE_EFFECTS_IMMUNITY, cnf))
 				{
 					elementalImmune = true;
-					break;
+					stop = true; //only bonus from one school is used
 				}
 			}
-		}
+		});
+
 		return elementalImmune;
 	}
 };

lib/spells/effects/Damage.cpp

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

server/CGameHandler.cpp

@@ -1532,7 +1532,7 @@ void CGameHandler::useScholarSkill(ObjectInstanceID fromHero, ObjectInstanceID t
 	cs1.learn = true;
 	cs1.hid = toHero;//giving spells to first hero
 	for (auto it : h1->getSpellsInSpellbook())
-		if (h2Lvl >= it.toSpell()->level && !h2->spellbookContainsSpell(it))//hero can learn it and don't have it yet
+		if (h2Lvl >= it.toSpell()->getLevel() && !h2->spellbookContainsSpell(it))//hero can learn it and don't have it yet
 			cs1.spells.insert(it);//spell to learn
 
 	ChangeSpells cs2;
@@ -1540,7 +1540,7 @@ void CGameHandler::useScholarSkill(ObjectInstanceID fromHero, ObjectInstanceID t
 	cs2.hid = fromHero;
 
 	for (auto it : h2->getSpellsInSpellbook())
-		if (h1Lvl >= it.toSpell()->level && !h1->spellbookContainsSpell(it))
+		if (h1Lvl >= it.toSpell()->getLevel() && !h1->spellbookContainsSpell(it))
 			cs2.spells.insert(it);
 
 	if (!cs1.spells.empty() || !cs2.spells.empty())//create a message

server/battles/BattleActionProcessor.cpp

@@ -1339,7 +1339,9 @@ int64_t BattleActionProcessor::applyBattleEffects(BattleAttack & bat, std::share
 	if(!bat.shot() &&
 		!def->isClone() &&
 		def->hasBonusOfType(BonusType::FIRE_SHIELD) &&
-		!attackerState->hasBonusOfType(BonusType::FIRE_IMMUNITY) &&
+		!attackerState->hasBonusOfType(BonusType::SPELL_SCHOOL_IMMUNITY, SpellSchool(ESpellSchool::FIRE)) &&
+		!attackerState->hasBonusOfType(BonusType::NEGATIVE_EFFECTS_IMMUNITY, SpellSchool(ESpellSchool::FIRE)) &&
+		attackerState->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, SpellSchool(ESpellSchool::FIRE)) < 100 &&
 		CStack::isMeleeAttackPossible(attackerState.get(), def) // attacked needs to be adjacent to defender for fire shield to trigger (e.g. Dragon Breath attack)
 			)
 	{

test/mock/mock_spells_Mechanics.h

@@ -64,8 +64,6 @@ public:
 	MOCK_CONST_METHOD1(applySpecificSpellBonus,int64_t(int64_t));
 	MOCK_CONST_METHOD2(calculateRawEffectValue, int64_t(int32_t, int32_t));
 
-	MOCK_CONST_METHOD0(getElementalImmunity, std::vector<BonusType>());
-
 	MOCK_CONST_METHOD1(ownerMatches, bool(const battle::Unit *));
 	MOCK_CONST_METHOD2(ownerMatches, bool(const battle::Unit *, const boost::logic::tribool));
 

test/mock/mock_spells_Spell.h

@@ -45,7 +45,7 @@ public:
 	MOCK_CONST_METHOD0(isOffensive, bool());
 	MOCK_CONST_METHOD0(isSpecial, bool());
 	MOCK_CONST_METHOD0(isMagical, bool());
-	MOCK_CONST_METHOD1(hasSchool, bool(ESpellSchool));
+	MOCK_CONST_METHOD1(hasSchool, bool(SpellSchool));
 	MOCK_CONST_METHOD1(forEachSchool, void(const SchoolCallback &));
 	MOCK_CONST_METHOD0(getCastSound, const std::string &());
 	MOCK_CONST_METHOD1(registerIcons, void(const IconRegistar &));

test/spells/targetConditions/BonusConditionTest.cpp

@@ -49,7 +49,7 @@ TEST_F(BonusConditionTest, ReceptiveIfMatchesType)
 TEST_F(BonusConditionTest, ImmuneIfTypeMismatch)
 {
 	setDefaultExpectations();
-	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::FIRE_IMMUNITY, BonusSource::OTHER, 0, 0));
+	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::SPELL_SCHOOL_IMMUNITY, BonusSource::OTHER, 0, SpellSchool(ESpellSchool::FIRE)));
 	EXPECT_FALSE(subject->isReceptive(&mechanicsMock, &unitMock));
 }
 

test/spells/targetConditions/ElementalConditionTest.cpp

@@ -20,18 +20,20 @@ class ElementalConditionTest : public TargetConditionItemTest, public WithParamI
 {
 public:
 	bool isPositive;
+
 	void setDefaultExpectations()
 	{
 		EXPECT_CALL(unitMock, getAllBonuses(_, _, _, _)).Times(AtLeast(1));
 		EXPECT_CALL(unitMock, getTreeVersion()).Times(AtLeast(0));
 
-		std::vector<BonusType> immunityList =
+		EXPECT_CALL(mechanicsMock, getSpell()).Times(AtLeast(1)).WillRepeatedly(Return(&spellMock));
+		EXPECT_CALL(spellMock, forEachSchool(NotNull())).Times(AtLeast(1)).WillRepeatedly([](const spells::Spell::SchoolCallback & cb)
 		{
-			BonusType::AIR_IMMUNITY,
-			BonusType::FIRE_IMMUNITY,
-		};
+			bool stop = false;
+			cb(SpellSchool(ESpellSchool::AIR), stop);
+			cb(SpellSchool(ESpellSchool::FIRE), stop);
+		});
 
-		EXPECT_CALL(mechanicsMock, getElementalImmunity()).Times(AtLeast(1)).WillRepeatedly(Return(immunityList));
 		EXPECT_CALL(mechanicsMock, isPositiveSpell()).WillRepeatedly(Return(isPositive));
 	}
 
@@ -54,15 +56,23 @@ TEST_P(ElementalConditionTest, ReceptiveIfNoBonus)
 TEST_P(ElementalConditionTest, ImmuneIfBonusMatches)
 {
 	setDefaultExpectations();
-	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::AIR_IMMUNITY, BonusSource::SPELL_EFFECT, 0, 0, 0));
+	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::SPELL_SCHOOL_IMMUNITY, BonusSource::SPELL_EFFECT, 0, 0, SpellSchool(ESpellSchool::AIR)));
 
 	EXPECT_FALSE(subject->isReceptive(&mechanicsMock, &unitMock));
 }
 
+TEST_P(ElementalConditionTest, NotImmuneIfBonusMismatches)
+{
+	setDefaultExpectations();
+	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::SPELL_SCHOOL_IMMUNITY, BonusSource::SPELL_EFFECT, 0, 0, SpellSchool(ESpellSchool::WATER)));
+
+	EXPECT_TRUE(subject->isReceptive(&mechanicsMock, &unitMock));
+}
+
 TEST_P(ElementalConditionTest, DependsOnPositivness)
 {
 	setDefaultExpectations();
-	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::AIR_IMMUNITY, BonusSource::SPELL_EFFECT, 0, 0, 1));
+	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::NEGATIVE_EFFECTS_IMMUNITY, BonusSource::SPELL_EFFECT, 0, 0, SpellSchool(ESpellSchool::AIR)));
 
 	EXPECT_EQ(isPositive, subject->isReceptive(&mechanicsMock, &unitMock));
 }
@@ -70,8 +80,8 @@ TEST_P(ElementalConditionTest, DependsOnPositivness)
 TEST_P(ElementalConditionTest, ImmuneIfBothBonusesPresent)
 {
 	setDefaultExpectations();
-	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::AIR_IMMUNITY, BonusSource::SPELL_EFFECT, 0, 0, 0));
-	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::AIR_IMMUNITY, BonusSource::SPELL_EFFECT, 0, 0, 1));
+	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::SPELL_SCHOOL_IMMUNITY, BonusSource::SPELL_EFFECT, 0, 0, SpellSchool(ESpellSchool::AIR)));
+	unitBonuses.addNewBonus(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::NEGATIVE_EFFECTS_IMMUNITY, BonusSource::SPELL_EFFECT, 0, 0, SpellSchool(ESpellSchool::AIR)));
 
 	EXPECT_FALSE(subject->isReceptive(&mechanicsMock, &unitMock));
 }

test/spells/targetConditions/TargetConditionItemFixture.h

@@ -17,6 +17,7 @@
 
 
 #include "mock/mock_spells_Mechanics.h"
+#include "mock/mock_spells_Spell.h"
 #include "mock/mock_BonusBearer.h"
 #include "mock/mock_battle_Unit.h"
 
@@ -30,6 +31,8 @@ public:
 
 	::testing::StrictMock<spells::MechanicsMock> mechanicsMock;
 	::testing::StrictMock<UnitMock> unitMock;
+	::testing::StrictMock<spells::SpellMock> spellMock;
+
 	BonusBearerMock unitBonuses;
 protected:
 	void SetUp() override