Graceful error handling for missing spells/factions (Fixes #1825, Bug #2176, Bug #2203)

apps/openmw/mwclass/creature.cpp

@@ -119,7 +119,13 @@ namespace MWClass
             // spells
             for (std::vector<std::string>::const_iterator iter (ref->mBase->mSpells.mList.begin());
                 iter!=ref->mBase->mSpells.mList.end(); ++iter)
-                data->mCreatureStats.getSpells().add (*iter);
+            {
+                const ESM::Spell* spell = MWBase::Environment::get().getWorld()->getStore().get<ESM::Spell>().search(*iter);
+                if (spell)
+                    data->mCreatureStats.getSpells().add (spell);
+                else /// \todo add option to make this a fatal error message pop-up, but default to warning for vanilla compatibility
+                    std::cerr << "Warning: ignoring nonexistent spell '" << spell->mId << "' on creature '" << ref->mBase->mId << "'" << std::endl;
+            }
 
             // inventory
             if (ref->mBase->mFlags & ESM::Creature::Weapon)

apps/openmw/mwclass/npc.cpp

@@ -300,15 +300,20 @@ namespace MWClass
             if (!ref->mBase->mFaction.empty())
             {
                 std::string faction = ref->mBase->mFaction;
-                Misc::StringUtils::toLower(faction);
-                if(ref->mBase->mNpdtType != ESM::NPC::NPC_WITH_AUTOCALCULATED_STATS)
+                const ESM::Faction* fact = MWBase::Environment::get().getWorld()->getStore().get<ESM::Faction>().search(faction);
+                if (fact)
                 {
-                    data->mNpcStats.setFactionRank(faction, (int)ref->mBase->mNpdt52.mRank);
+                    if(ref->mBase->mNpdtType != ESM::NPC::NPC_WITH_AUTOCALCULATED_STATS)
+                    {
+                        data->mNpcStats.setFactionRank(fact->mId, (int)ref->mBase->mNpdt52.mRank);
+                    }
+                    else
+                    {
+                        data->mNpcStats.setFactionRank(fact->mId, (int)ref->mBase->mNpdt12.mRank);
+                    }
                 }
                 else
-                {
-                    data->mNpcStats.setFactionRank(faction, (int)ref->mBase->mNpdt12.mRank);
-                }
+                    std::cerr << "Warning: ignoring nonexistent faction '" << fact->mId << "' on NPC '" << ref->mBase->mId << "'" << std::endl;
             }
 
             // creature stats
@@ -361,7 +366,11 @@ namespace MWClass
             for (std::vector<std::string>::const_iterator iter (race->mPowers.mList.begin());
                 iter!=race->mPowers.mList.end(); ++iter)
             {
-                data->mNpcStats.getSpells().add (*iter);
+                const ESM::Spell* spell = MWBase::Environment::get().getWorld()->getStore().get<ESM::Spell>().search(*iter);
+                if (spell)
+                    data->mNpcStats.getSpells().add (spell);
+                else
+                    std::cerr << "Warning: ignoring nonexistent race power '" << *iter << "' on NPC '" << ref->mBase->mId << "'" << std::endl;
             }
 
             if (data->mNpcStats.getFactionRanks().size())
@@ -385,7 +394,16 @@ namespace MWClass
             // spells
             for (std::vector<std::string>::const_iterator iter (ref->mBase->mSpells.mList.begin());
                 iter!=ref->mBase->mSpells.mList.end(); ++iter)
-                data->mNpcStats.getSpells().add (*iter);
+            {
+                const ESM::Spell* spell = MWBase::Environment::get().getWorld()->getStore().get<ESM::Spell>().search(*iter);
+                if (spell)
+                    data->mNpcStats.getSpells().add (spell);
+                else
+                {
+                    /// \todo add option to make this a fatal error message pop-up, but default to warning for vanilla compatibility
+                    std::cerr << "Warning: ignoring nonexistent spell '" << *iter << "' on NPC '" << ref->mBase->mId << "'" << std::endl;
+                }
+            }
 
             // inventory
             data->mInventoryStore.fill(ref->mBase->mInventory, getId(ptr), "",

apps/openmw/mwmechanics/spells.cpp

@@ -25,12 +25,10 @@ namespace MWMechanics
         return mSpells.end();
     }
 
-    void Spells::add (const std::string& spellId)
+    void Spells::add (const ESM::Spell* spell)
     {
-        if (mSpells.find (spellId)==mSpells.end())
+        if (mSpells.find (spell->mId)==mSpells.end())
         {
-            const ESM::Spell* spell = MWBase::Environment::get().getWorld()->getStore().get<ESM::Spell>().find(spellId);
-
             std::map<const int, float> random;
 
             // Determine the random magnitudes (unless this is a castable spell, in which case
@@ -50,13 +48,19 @@ namespace MWMechanics
                 corprus.mWorsenings = 0;
                 corprus.mNextWorsening = MWBase::Environment::get().getWorld()->getTimeStamp() + CorprusStats::sWorseningPeriod;
 
-                mCorprusSpells[spellId] = corprus;
+                mCorprusSpells[spell->mId] = corprus;
             }
 
-            mSpells.insert (std::make_pair (Misc::StringUtils::lowerCase(spellId), random));
+            mSpells.insert (std::make_pair (spell->mId, random));
         }
     }
 
+    void Spells::add (const std::string& spellId)
+    {
+        const ESM::Spell* spell = MWBase::Environment::get().getWorld()->getStore().get<ESM::Spell>().find(spellId);
+        add(spell);
+    }
+
     void Spells::remove (const std::string& spellId)
     {
         std::string lower = Misc::StringUtils::lowerCase(spellId);

apps/openmw/mwmechanics/spells.hpp

@@ -79,6 +79,9 @@ namespace MWMechanics
             void add (const std::string& spell);
             ///< Adding a spell that is already listed in *this is a no-op.
 
+            void add (const ESM::Spell* spell);
+            ///< Adding a spell that is already listed in *this is a no-op.
+
             void remove (const std::string& spell);
             ///< If the spell to be removed is the selected spell, the selected spell will be changed to
             /// no spell (empty string).