Add spell effect argument validation (bug #4927)

CHANGELOG.md

@@ -50,6 +50,7 @@
     Bug #4911: Editor: QOpenGLContext::swapBuffers() warning with Qt5
     Bug #4916: Specular power (shininess) material parameter is ignored when shaders are used.
     Bug #4922: Werewolves can not attack if the transformation happens during attack
+    Bug #4927: Spell effect having both a skill and an attribute assigned is a fatal error
     Bug #4938: Strings from subrecords with actually empty headers can't be empty
     Bug #4942: Hand-to-Hand attack type is chosen randomly when "always use best attack" is turned off
     Feature #2229: Improve pathfinding AI

apps/openmw/mwworld/esmstore.cpp

@@ -157,7 +157,7 @@ void ESMStore::validate()
 
     // Validate NPCs for non-existing class and faction.
     // We will replace invalid entries by fixed ones
-    std::vector<ESM::NPC> entitiesToReplace;
+    std::vector<ESM::NPC> npcsToReplace;
     for (ESM::NPC npc : mNpcs)
     {
         bool changed = false;
@@ -188,14 +188,68 @@ void ESMStore::validate()
         }
 
         if (changed)
-            entitiesToReplace.push_back(npc);
+            npcsToReplace.push_back(npc);
     }
 
-    for (const ESM::NPC &npc : entitiesToReplace)
+    for (const ESM::NPC &npc : npcsToReplace)
     {
         mNpcs.eraseStatic(npc.mId);
         mNpcs.insertStatic(npc);
     }
+
+    // Validate spell effects for invalid arguments
+    std::vector<ESM::Spell> spellsToReplace;
+    for (ESM::Spell spell : mSpells)
+    {
+        if (spell.mEffects.mList.empty())
+            continue;
+
+        bool changed = false;
+        for (ESM::ENAMstruct& effect : spell.mEffects.mList)
+        {
+            const ESM::MagicEffect* mgef = mMagicEffects.search(effect.mEffectID);
+            if (!mgef) // Do nothing for now
+                continue;
+
+            if (mgef->mData.mFlags & ESM::MagicEffect::TargetSkill)
+            {
+                if (effect.mAttribute != -1)
+                {
+                    effect.mAttribute = -1;
+                    Log(Debug::Verbose) << ESM::MagicEffect::effectIdToString(effect.mEffectID) <<
+                        " effect of spell '" << spell.mId << "'  has an attribute argument present, dropping it.";
+                    changed = true;
+                }
+            }
+            else if (mgef->mData.mFlags & ESM::MagicEffect::TargetAttribute)
+            {
+                if (effect.mSkill != -1)
+                {
+                    effect.mSkill = -1;
+                    Log(Debug::Verbose) << ESM::MagicEffect::effectIdToString(effect.mEffectID) <<
+                        " effect of spell '" << spell.mId << "' has a skill argument present, dropping it.";
+                    changed = true;
+                }
+            }
+            else if (effect.mSkill != -1 || effect.mAttribute != -1)
+            {
+                effect.mSkill = -1;
+                effect.mAttribute = -1;
+                Log(Debug::Verbose) << ESM::MagicEffect::effectIdToString(effect.mEffectID) <<
+                    " effect of spell '" << spell.mId << "' has argument(s) present, dropping them.";
+                changed = true;
+            }
+        }
+
+        if (changed)
+            spellsToReplace.emplace_back(spell);
+    }
+
+    for (const ESM::Spell &spell : spellsToReplace)
+    {
+        mSpells.eraseStatic(spell.mId);
+        mSpells.insertStatic(spell);
+    }
 }
 
     int ESMStore::countSavedGameRecords() const