From d98a90194bd5c59d653ee00cf70e927c50e33948 Mon Sep 17 00:00:00 2001
From: Capostrophic <alexdobrohotov@yandex.ru>
Date: Fri, 31 May 2019 11:23:58 +0300
Subject: [PATCH] Erase effects with invalid indices from spells (bug #5050)

---
 CHANGELOG.md                     |  1 +
 apps/openmw/mwworld/esmstore.cpp | 34 ++++++++++++++++++++------------
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 82621d794..6d996c909 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -96,6 +96,7 @@
     Bug #5028: Offered price caps are not trading-specific
     Bug #5038: Enchanting success chance calculations are blatantly wrong
     Bug #5047: # in cell names sets color
+    Bug #5050: Invalid spell effects are not handled gracefully
     Feature #1774: Handle AvoidNode
     Feature #2229: Improve pathfinding AI
     Feature #3025: Analogue gamepad movement controls
diff --git a/apps/openmw/mwworld/esmstore.cpp b/apps/openmw/mwworld/esmstore.cpp
index 1bbe73d0e..56b46d856 100644
--- a/apps/openmw/mwworld/esmstore.cpp
+++ b/apps/openmw/mwworld/esmstore.cpp
@@ -205,40 +205,48 @@ void ESMStore::validate()
             continue;
 
         bool changed = false;
-        for (ESM::ENAMstruct& effect : spell.mEffects.mList)
+        auto iter = spell.mEffects.mList.begin();
+        while (iter != spell.mEffects.mList.end())
         {
-            const ESM::MagicEffect* mgef = mMagicEffects.search(effect.mEffectID);
-            if (!mgef) // Do nothing for now
+            const ESM::MagicEffect* mgef = mMagicEffects.search(iter->mEffectID);
+            if (!mgef)
+            {
+                Log(Debug::Verbose) << "Spell '" << spell.mId << "' has an an invalid effect (index " << iter->mEffectID << ") present, dropping it.";
+                iter = spell.mEffects.mList.erase(iter);
+                changed = true;
                 continue;
+            }
 
             if (mgef->mData.mFlags & ESM::MagicEffect::TargetSkill)
             {
-                if (effect.mAttribute != -1)
+                if (iter->mAttribute != -1)
                 {
-                    effect.mAttribute = -1;
-                    Log(Debug::Verbose) << ESM::MagicEffect::effectIdToString(effect.mEffectID) <<
+                    iter->mAttribute = -1;
+                    Log(Debug::Verbose) << ESM::MagicEffect::effectIdToString(iter->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)
+                if (iter->mSkill != -1)
                 {
-                    effect.mSkill = -1;
-                    Log(Debug::Verbose) << ESM::MagicEffect::effectIdToString(effect.mEffectID) <<
+                    iter->mSkill = -1;
+                    Log(Debug::Verbose) << ESM::MagicEffect::effectIdToString(iter->mEffectID) <<
                         " effect of spell '" << spell.mId << "' has a skill argument present, dropping it.";
                     changed = true;
                 }
             }
-            else if (effect.mSkill != -1 || effect.mAttribute != -1)
+            else if (iter->mSkill != -1 || iter->mAttribute != -1)
             {
-                effect.mSkill = -1;
-                effect.mAttribute = -1;
-                Log(Debug::Verbose) << ESM::MagicEffect::effectIdToString(effect.mEffectID) <<
+                iter->mSkill = -1;
+                iter->mAttribute = -1;
+                Log(Debug::Verbose) << ESM::MagicEffect::effectIdToString(iter->mEffectID) <<
                     " effect of spell '" << spell.mId << "' has argument(s) present, dropping them.";
                 changed = true;
             }
+
+            ++iter;
         }
 
         if (changed)