spells: rework isMagical()

Now it is a flag of a spell, and not a target condition.
This fixes resistance to bind effect

config/schemas/spell.json

@@ -233,7 +233,11 @@
 						"special":{
 								"type": "boolean",
 								"description": "Special spell. Can be given only by Bonus::SPELL"
-						}
+						},
+						"nonMagical":{
+							"type": "boolean",
+							"description": "Non-magical ability. Usually used by some creatures. Should not be affected by sorcery and generic magic resistance. School resistances apply. Examples: dendroid bind, efreet fire shield."
+					}
 				}
 		},
 		"immunity":{

config/spells/ability.json

@@ -119,6 +119,7 @@
 			}
 		},
 		"flags" : {
+			"nonMagical" : true,
 			"indifferent": true
 		}
 	},

config/spells/moats.json

@@ -34,6 +34,7 @@
 		"flags" : {
 			"damage": true,
 			"negative": true,
+			"nonMagical" : true,
 			"special": true
 		},
 		"targetCondition" : {
@@ -82,10 +83,10 @@
             }
         },
         "flags" : {
+			"nonMagical" : true,
             "indifferent": true
         },
         "targetCondition" : {
-            "nonMagical" : true
         }
     },
     "rampartMoatTrigger" :
@@ -123,6 +124,7 @@
 		"flags" : {
 			"damage": true,
 			"negative": true,
+			"nonMagical" : true,
 			"special": true
 		},
 		"targetCondition" : {
@@ -171,10 +173,10 @@
             }
         },
         "flags" : {
+			"nonMagical" : true,
             "indifferent": true
         },
         "targetCondition" : {
-            "nonMagical" : true
         }
     },
     "towerMoat": {
@@ -223,10 +225,10 @@
             }
         },
         "flags" : {
+			"nonMagical" : true,
             "indifferent": true
         },
         "targetCondition" : {
-            "nonMagical" : true
         }
     },
     "infernoMoatTrigger" :
@@ -264,6 +266,7 @@
 		"flags" : {
 			"damage": true,
 			"negative": true,
+			"nonMagical" : true,
 			"special": true
 		},
 		"targetCondition" : {
@@ -312,10 +315,10 @@
             }
         },
         "flags" : {
+			"nonMagical" : true,
             "indifferent": true
         },
         "targetCondition" : {
-            "nonMagical" : true
         }
     },
     "necropolisMoatTrigger" :
@@ -353,6 +356,7 @@
 		"flags" : {
 			"damage": true,
 			"negative": true,
+			"nonMagical" : true,
 			"special": true
 		},
 		"targetCondition" : {
@@ -401,10 +405,10 @@
             }
         },
         "flags" : {
+			"nonMagical" : true,
             "indifferent": true
         },
         "targetCondition" : {
-            "nonMagical" : true
         }
     },
     "dungeonMoatTrigger" :
@@ -442,6 +446,7 @@
 		"flags" : {
 			"damage": true,
 			"negative": true,
+			"nonMagical" : true,
 			"special": true
 		},
 		"targetCondition" : {
@@ -490,10 +495,10 @@
             }
         },
         "flags" : {
+			"nonMagical" : true,
             "indifferent": true
         },
         "targetCondition" : {
-            "nonMagical" : true
         }
     },
     "strongholdMoatTrigger" :
@@ -531,6 +536,7 @@
 		"flags" : {
 			"damage": true,
 			"negative": true,
+			"nonMagical" : true,
 			"special": true
 		},
 		"targetCondition" : {
@@ -579,10 +585,10 @@
             }
         },
         "flags" : {
+			"nonMagical" : true,
             "indifferent": true
         },
         "targetCondition" : {
-            "nonMagical" : true
         }
     },
     "fortressMoatTrigger" :
@@ -620,6 +626,7 @@
 		"flags" : {
 			"damage": true,
 			"negative": true,
+			"nonMagical" : true,
 			"special": true
 		},
 		"targetCondition" : {
@@ -668,10 +675,10 @@
             }
         },
         "flags" : {
+			"nonMagical" : true,
             "indifferent": true
         },
         "targetCondition" : {
-            "nonMagical" : true
         }
     }
 }

config/spells/vcmiAbility.json

@@ -93,10 +93,10 @@
 			}
 		},
 		"flags" : {
+			"nonMagical" : true,
 			"positive": true
 		},
 		"targetCondition" : {
-			"nonMagical" : true,
 			"noneOf" : {
 				"bonus.SIEGE_WEAPON" : "absolute"
 			}
@@ -179,10 +179,8 @@
 			}
 		},
 		"flags" : {
+			"nonMagical" : true,
 			"indifferent": true
-		},
-		"targetCondition" : {
-			"nonMagical" : true
 		}
 	},
 	"cyclopsShot" : {
@@ -227,10 +225,8 @@
 			"expert" : {}
 		},
 		"flags" : {
+			"nonMagical" : true,
 			"indifferent": true
-		},
-		"targetCondition" : {
-			"nonMagical" : true
 		}
 	}
 }

include/vcmi/spells/Spell.h

@@ -41,6 +41,7 @@ public:
 	virtual bool isDamage() const = 0;
 	virtual bool isOffensive() const = 0;
 	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 void forEachSchool(const SchoolCallback & cb) const = 0;
 	virtual const std::string & getCastSound() const = 0;

lib/spells/BattleSpellMechanics.cpp

@@ -360,7 +360,7 @@ void BattleSpellMechanics::beforeCast(BattleSpellCast & sc, vstd::RNG & rng, con
 
 	auto filterResisted = [&, this](const battle::Unit * unit) -> bool
 	{
-		if(isNegativeSpell())
+		if(isNegativeSpell() && isMagicalEffect())
 		{
 			//magic resistance
 			const int prob = std::min(unit->magicResistance(), 100); //probability of resistance in %

lib/spells/CSpellHandler.cpp

@@ -97,6 +97,7 @@ CSpell::CSpell():
 	damage(false),
 	offensive(false),
 	special(true),
+	nonMagical(false),
 	targetType(spells::AimType::NO_TARGET)
 {
 	levels.resize(GameConstants::SPELL_SCHOOL_LEVELS);
@@ -236,6 +237,11 @@ bool CSpell::isCreatureAbility() const
 	return creatureAbility;
 }
 
+bool CSpell::isMagical() const
+{
+	return !nonMagical;
+}
+
 bool CSpell::isPositive() const
 {
 	return positiveness == POSITIVE;
@@ -751,6 +757,8 @@ CSpell * CSpellHandler::loadFromJson(const std::string & scope, const JsonNode &
 
 	spell->damage = flags["damage"].Bool(); //do this before "offensive"
 
+	spell->nonMagical = flags["nonMagical"].Bool();
+
 	if(flags["offensive"].Bool())
 	{
 		spell->setIsOffensive(true);

lib/spells/CSpellHandler.h

@@ -246,6 +246,7 @@ public:
 	bool isPositive() const override;
 	bool isNegative() const override;
 	bool isNeutral() const override;
+	bool isMagical() const override;
 
 	bool isDamage() const override;
 	bool isOffensive() const override;
@@ -297,6 +298,7 @@ public:
 		h & levels;
 		h & school;
 		h & animationInfo;
+		h & nonMagical;
 	}
 	friend class CSpellHandler;
 	friend class Graphics;
@@ -338,6 +340,7 @@ private:
 	bool damage;
 	bool offensive;
 	bool special;
+	bool nonMagical; //For creature abilities like bind
 
 	std::string attributes; //reference only attributes //todo: remove or include in configuration format, currently unused
 

lib/spells/ISpellMechanics.cpp

@@ -617,6 +617,11 @@ bool BaseMechanics::isPositiveSpell() const
 	return owner->isPositive();
 }
 
+bool BaseMechanics::isMagicalEffect() const
+{
+	return owner->isMagical();
+}
+
 int64_t BaseMechanics::adjustEffectValue(const battle::Unit * target) const
 {
 	return owner->adjustRawDamage(caster, target, getEffectValue());

lib/spells/ISpellMechanics.h

@@ -228,6 +228,7 @@ public:
 
 	virtual bool isNegativeSpell() const = 0;
 	virtual bool isPositiveSpell() const = 0;
+	virtual bool isMagicalEffect() const = 0;
 
 	virtual int64_t adjustEffectValue(const battle::Unit * target) const = 0;
 	virtual int64_t applySpellBonus(int64_t value, const battle::Unit * target) const = 0;
@@ -288,6 +289,7 @@ public:
 
 	bool isNegativeSpell() const override;
 	bool isPositiveSpell() const override;
+	bool isMagicalEffect() const override;
 
 	int64_t adjustEffectValue(const battle::Unit * target) const override;
 	int64_t applySpellBonus(int64_t value, const battle::Unit * target) const override;

lib/spells/TargetCondition.cpp

@@ -114,6 +114,10 @@ public:
 protected:
 	bool check(const Mechanics * m, const battle::Unit * target) const override
 	{
+
+		if(!m->isMagicalEffect()) //Always pass on non-magical
+			return true;
+
 		std::stringstream cachingStr;
 		cachingStr << "type_" << Bonus::LEVEL_SPELL_IMMUNITY << "addInfo_1";
 
@@ -189,6 +193,8 @@ public:
 protected:
 	bool check(const Mechanics * m, const battle::Unit * target) const override
 	{
+		if(!m->isMagicalEffect()) //Always pass on non-magical
+			return true;
 		TConstBonusListPtr levelImmunities = target->getBonuses(Selector::type()(Bonus::LEVEL_SPELL_IMMUNITY));
 		return levelImmunities->size() == 0 ||
 		levelImmunities->totalValue() < m->getSpellLevel() ||
@@ -426,7 +432,6 @@ bool TargetCondition::isReceptive(const Mechanics * m, const battle::Unit * targ
 
 void TargetCondition::serializeJson(JsonSerializeFormat & handler, const ItemFactory * itemFactory)
 {
-	bool isNonMagical = false;
 	if(handler.saving)
 	{
 		logGlobal->error("Spell target condition saving is not supported");
@@ -438,17 +443,12 @@ void TargetCondition::serializeJson(JsonSerializeFormat & handler, const ItemFac
 	negation.clear();
 
 	absolute.push_back(itemFactory->createAbsoluteSpell());
-
-	handler.serializeBool("nonMagical", isNonMagical);
-	if(!isNonMagical)
-	{
-		absolute.push_back(itemFactory->createAbsoluteLevel());
-		normal.push_back(itemFactory->createElemental());
-		normal.push_back(itemFactory->createNormalLevel());
-		normal.push_back(itemFactory->createNormalSpell());
-		negation.push_back(itemFactory->createReceptiveFeature());
-		negation.push_back(itemFactory->createImmunityNegation());
-	}
+	absolute.push_back(itemFactory->createAbsoluteLevel());
+	normal.push_back(itemFactory->createElemental());
+	normal.push_back(itemFactory->createNormalLevel());
+	normal.push_back(itemFactory->createNormalSpell());
+	negation.push_back(itemFactory->createReceptiveFeature());
+	negation.push_back(itemFactory->createImmunityNegation());
 
 	{
 		auto anyOf = handler.enterStruct("anyOf");