vcmi: use SpellSchool identifier instead of enum

Use identifier instead of enum inside callbacks. It is better and more
expandable solution.

client/windows/CSpellWindow.cpp

@@ -122,9 +122,9 @@ CSpellWindow::CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _m
 
 		++sitesPerOurTab[4];
 
-		spell->forEachSchool([&sitesPerOurTab](const ESpellSchool & school, bool & stop)
+		spell->forEachSchool([&sitesPerOurTab](const SpellSchool & school, bool & stop)
 		{
-			++sitesPerOurTab[(ui8)school];
+			++sitesPerOurTab[school];
 		});
 	}
 	if(sitesPerTabAdv[4] % 12 == 0)

include/vcmi/spells/Spell.h

@@ -24,7 +24,7 @@ class Caster;
 class DLL_LINKAGE Spell: public EntityT<SpellID>
 {
 public:
-	using SchoolCallback = std::function<void(const ESpellSchool &, 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;
@@ -43,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/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/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 ESpellSchool & cnf, bool & stop)
+	spell->forEachSchool([&, this](const SpellSchool & cnf, bool & stop)
 	{
-		int32_t thisSchool = valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, SpellSchool(cnf)); //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 = SpellSchool(cnf);
+				*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 ESpellSchool & cnf, bool & stop)
+	spell->forEachSchool([&maxSchoolBonus, this](const SpellSchool & cnf, bool & stop)
 	{
-		vstd::amax(maxSchoolBonus, valOfBonuses(BonusType::SPELL_DAMAGE, SpellSchool(cnf)));
+		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 ESpellSchool & cnf, bool & stop)
+	spell->forEachSchool([this, &schoolBonus](const SpellSchool & cnf, bool & stop)
 	{
-		if(hasBonusOfType(BonusType::SPELLS_OF_SCHOOL, SpellSchool(cnf)))
+		if(hasBonusOfType(BonusType::SPELLS_OF_SCHOOL, cnf))
 		{
 			schoolBonus = stop = true;
 		}

lib/spells/CSpellHandler.cpp

@@ -124,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);
 }
@@ -149,7 +149,7 @@ spells::AimType CSpell::getTargetType() const
 	return targetType;
 }
 
-void CSpell::forEachSchool(const std::function<void(const ESpellSchool &, bool &)>& cb) const
+void CSpell::forEachSchool(const std::function<void(const SpellSchool &, bool &)>& cb) const
 {
 	bool stop = false;
 	for(auto iter : SpellConfig::SCHOOL_ORDER)
@@ -157,7 +157,7 @@ void CSpell::forEachSchool(const std::function<void(const ESpellSchool &, bool &
 		const spells::SchoolInfo & cnf = SpellConfig::SCHOOL[iter];
 		if(school.at(cnf.id))
 		{
-			cb(cnf.id.toEnum(), stop);
+			cb(cnf.id, stop);
 
 			if(stop)
 				break;
@@ -381,11 +381,11 @@ int64_t CSpell::adjustRawDamage(const spells::Caster * caster, const battle::Uni
 	{
 		const auto * bearer = affectedCreature->getBonusBearer();
 		//applying protections - when spell has more then one elements, only one protection should be applied (I think)
-		forEachSchool([&](const ESpellSchool & cnf, bool & stop)
+		forEachSchool([&](const SpellSchool & cnf, bool & stop)
 		{
-			if(bearer->hasBonusOfType(BonusType::SPELL_DAMAGE_REDUCTION, SpellSchool(cnf)))
+			if(bearer->hasBonusOfType(BonusType::SPELL_DAMAGE_REDUCTION, cnf))
 			{
-				ret *= 100 - bearer->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, SpellSchool(cnf));
+				ret *= 100 - bearer->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, cnf);
 				ret /= 100;
 				stop = true; //only bonus from one school is used
 			}

lib/spells/CSpellHandler.h

@@ -201,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 ESpellSchool &, bool &)> & cb) const override;
+	void forEachSchool(const std::function<void(const SpellSchool &, bool &)> & cb) const override;
 
 	spells::AimType getTargetType() const;
 

lib/spells/TargetCondition.cpp

@@ -177,16 +177,16 @@ protected:
 		bool elementalImmune = false;
 		auto bearer = target->getBonusBearer();
 
-		m->getSpell()->forEachSchool([&](const ESpellSchool & cnf, bool & stop) 
+		m->getSpell()->forEachSchool([&](const SpellSchool & cnf, bool & stop) 
 		{
-			if (bearer->hasBonusOfType(BonusType::SPELL_SCHOOL_IMMUNITY, SpellSchool(cnf)))
+			if (bearer->hasBonusOfType(BonusType::SPELL_SCHOOL_IMMUNITY, cnf))
 			{
 				elementalImmune = true;
 				stop = true; //only bonus from one school is used
 			}
 			else if(!m->isPositiveSpell()) //negative or indifferent
 			{
-				if (bearer->hasBonusOfType(BonusType::NEGATIVE_EFFECTS_IMMUNITY, SpellSchool(cnf)))
+				if (bearer->hasBonusOfType(BonusType::NEGATIVE_EFFECTS_IMMUNITY, cnf))
 				{
 					elementalImmune = true;
 					stop = true; //only bonus from one school is used

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 ESpellSchool & cnf, bool & stop)
+	m->getSpell()->forEachSchool([&](const SpellSchool & cnf, bool & stop)
 	{
-		isImmune |= (unit->getBonusBearer()->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, SpellSchool(cnf)) >= 100); //100% reduction is immunity
+		isImmune |= (unit->getBonusBearer()->valOfBonuses(BonusType::SPELL_DAMAGE_REDUCTION, cnf) >= 100); //100% reduction is immunity
 	});
 
 	return !isImmune;

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/ElementalConditionTest.cpp

@@ -30,8 +30,8 @@ public:
 		EXPECT_CALL(spellMock, forEachSchool(NotNull())).Times(AtLeast(1)).WillRepeatedly([](const spells::Spell::SchoolCallback & cb)
 		{
 			bool stop = false;
-			cb(ESpellSchool::AIR, stop);
-			cb(ESpellSchool::FIRE, stop);
+			cb(SpellSchool(ESpellSchool::AIR), stop);
+			cb(SpellSchool(ESpellSchool::FIRE), stop);
 		});
 
 		EXPECT_CALL(mechanicsMock, isPositiveSpell()).WillRepeatedly(Return(isPositive));