Fix some leftover code that still calculated random magnitude per spell rather than per effect. Major cleanup of InventoryStore: Magic effects are now updated when needed, rather than cached. Also allows to get rid of 'mutable' hacks and non-const method that should be const. Play sounds and particles when equipping a permanent enchantment item.

apps/openmw/mwmechanics/magiceffects.cpp

@@ -68,28 +68,6 @@ namespace MWMechanics
         }
     }
 
-    void MagicEffects::add (const ESM::EffectList& list, float magnitude)
-    {
-        for (std::vector<ESM::ENAMstruct>::const_iterator iter (list.mList.begin()); iter!=list.mList.end();
-            ++iter)
-        {
-            EffectParam param;
-
-            if (iter->mMagnMin>=iter->mMagnMax)
-                param.mMagnitude = iter->mMagnMin;
-            else
-            {
-                if (magnitude==-1)
-                    magnitude = static_cast<float> (std::rand()) / RAND_MAX;
-
-                param.mMagnitude = static_cast<int> (
-                    (iter->mMagnMax-iter->mMagnMin+1)*magnitude + iter->mMagnMin);
-            }
-
-            add (*iter, param);
-        }
-    }
-
     MagicEffects& MagicEffects::operator+= (const MagicEffects& effects)
     {
         if (this==&effects)

apps/openmw/mwmechanics/magiceffects.hpp

@@ -33,6 +33,8 @@ namespace MWMechanics
 
         EffectParam();
 
+        EffectParam(int magnitude) : mMagnitude(magnitude) {}
+
         EffectParam& operator+= (const EffectParam& param);
 
         EffectParam& operator-= (const EffectParam& param);
@@ -69,9 +71,6 @@ namespace MWMechanics
 
             void add (const EffectKey& key, const EffectParam& param);
 
-            void add (const ESM::EffectList& list, float magnitude = -1);
-            ///< \param magnitude normalised magnitude (-1: random)
-
             MagicEffects& operator+= (const MagicEffects& effects);
 
             EffectParam get (const EffectKey& key) const;

apps/openmw/mwmechanics/spells.cpp

@@ -27,7 +27,15 @@ namespace MWMechanics
     void Spells::add (const std::string& spellId)
     {
         if (mSpells.find (spellId)==mSpells.end())
-            mSpells.insert (std::make_pair (spellId, static_cast<float> (std::rand()) / RAND_MAX));
+        {
+            const ESM::Spell* spell = MWBase::Environment::get().getWorld()->getStore().get<ESM::Spell>().find(spellId);
+
+            std::vector<float> random;
+            random.resize(spell->mEffects.mList.size());
+            for (int i=0; i<random.size();++i)
+                random[i] = static_cast<float> (std::rand()) / RAND_MAX;
+            mSpells.insert (std::make_pair (spellId, random));
+        }
     }
 
     void Spells::remove (const std::string& spellId)
@@ -52,7 +60,14 @@ namespace MWMechanics
 
             if (spell->mData.mType==ESM::Spell::ST_Ability || spell->mData.mType==ESM::Spell::ST_Blight ||
                 spell->mData.mType==ESM::Spell::ST_Disease || spell->mData.mType==ESM::Spell::ST_Curse)
-                effects.add (spell->mEffects, iter->second);
+            {
+                int i=0;
+                for (std::vector<ESM::ENAMstruct>::const_iterator it = spell->mEffects.mList.begin(); it != spell->mEffects.mList.end(); ++it)
+                {
+                    effects.add (*it, iter->second[i]);
+                    ++i;
+                }
+            }
         }
 
         return effects;

apps/openmw/mwmechanics/spells.hpp

@@ -23,7 +23,7 @@ namespace MWMechanics
     {
         public:
 
-            typedef std::map<std::string, float> TContainer; // ID, normalised magnitude
+            typedef std::map<std::string, std::vector<float> > TContainer; // ID, normalised magnitudes
             typedef TContainer::const_iterator TIterator;
 
         private:

apps/openmw/mwworld/containerstore.cpp

@@ -333,15 +333,9 @@ void MWWorld::ContainerStore::clear()
 
 void MWWorld::ContainerStore::flagAsModified()
 {
-    ++mStateId;
     mWeightUpToDate = false;
 }
 
-int MWWorld::ContainerStore::getStateId() const
-{
-    return mStateId;
-}
-
 float MWWorld::ContainerStore::getWeight() const
 {
     if (!mWeightUpToDate)

apps/openmw/mwworld/containerstore.hpp

@@ -104,11 +104,6 @@ namespace MWWorld
             ///< \attention This function is internal to the world model and should not be called from
             /// outside.
 
-            int getStateId() const;
-            ///< This ID is changed every time the container is modified or items in the container
-            /// are accessed in a way that may be used to modify the item.
-            /// \note This method of change-tracking will ocasionally yield false positives.
-
             float getWeight() const;
             ///< Return total weight of the items contained in *this.
 

apps/openmw/mwworld/inventorystore.cpp

@@ -9,6 +9,9 @@
 #include "../mwbase/environment.hpp"
 #include "../mwbase/world.hpp"
 #include "../mwbase/windowmanager.hpp"
+#include "../mwbase/soundmanager.hpp"
+
+#include "../mwrender/animation.hpp"
 
 #include "../mwmechanics/npcstats.hpp"
 
@@ -39,9 +42,9 @@ void MWWorld::InventoryStore::initSlots (TSlots& slots_)
         slots_.push_back (end());
 }
 
-MWWorld::InventoryStore::InventoryStore() : mMagicEffectsUpToDate (false)
+MWWorld::InventoryStore::InventoryStore()
- , mSelectedEnchantItem(end())
+ : mSelectedEnchantItem(end())
- , mActorModelUpdateEnabled (true)
+ , mUpdatesEnabled (true)
 {
     initSlots (mSlots);
 }
@@ -51,15 +54,15 @@ MWWorld::InventoryStore::InventoryStore (const InventoryStore& store)
  , mSelectedEnchantItem(end())
 {
     mMagicEffects = store.mMagicEffects;
-    mMagicEffectsUpToDate = store.mMagicEffectsUpToDate;
     mSelectedEnchantItem = store.mSelectedEnchantItem;
+    mPermanentMagicEffectMagnitudes = store.mPermanentMagicEffectMagnitudes;
     copySlots (store);
 }
 
 MWWorld::InventoryStore& MWWorld::InventoryStore::operator= (const InventoryStore& store)
 {
     mMagicEffects = store.mMagicEffects;
-    mMagicEffectsUpToDate = store.mMagicEffectsUpToDate;
+    mPermanentMagicEffectMagnitudes = store.mPermanentMagicEffectMagnitudes;
     ContainerStore::operator= (store);
     mSlots.clear();
     copySlots (store);
@@ -117,6 +120,8 @@ void MWWorld::InventoryStore::equip (int slot, const ContainerStoreIterator& ite
     flagAsModified();
 
     updateActorModel(actor);
+
+    updateMagicEffects(actor);
 }
 
 void MWWorld::InventoryStore::unequipAll(const MWWorld::Ptr& actor)
@@ -135,9 +140,9 @@ MWWorld::ContainerStoreIterator MWWorld::InventoryStore::getSlot (int slot)
 
     if (mSlots[slot]->getRefData().getCount()<1)
     {
-        // object has been deleted
+        // Object has been deleted
-        mSlots[slot] = end();
+        // This should no longer happen, since the new remove function will unequip first
-        return end();
+        throw std::runtime_error("Invalid slot, make sure you are not calling RefData::setCount for a container object");
     }
 
     return mSlots[slot];
@@ -152,7 +157,7 @@ void MWWorld::InventoryStore::autoEquip (const MWWorld::Ptr& npc)
     initSlots (slots_);
 
     // Disable model update during auto-equip
-    mActorModelUpdateEnabled = false;
+    mUpdatesEnabled = false;
 
     for (ContainerStoreIterator iter (begin()); iter!=end(); ++iter)
     {
@@ -242,26 +247,35 @@ void MWWorld::InventoryStore::autoEquip (const MWWorld::Ptr& npc)
             changed = true;
         }
 
-    mActorModelUpdateEnabled = true;
+    mUpdatesEnabled = true;
 
     if (changed)
     {
         mSlots.swap (slots_);
         updateActorModel(npc);
+        updateMagicEffects(npc);
         flagAsModified();
     }
 }
 
-const MWMechanics::MagicEffects& MWWorld::InventoryStore::getMagicEffects()
+const MWMechanics::MagicEffects& MWWorld::InventoryStore::getMagicEffects() const
 {
-    // TODO: Add VFX; update when needed instead of update on request
+    return mMagicEffects;
-    if (!mMagicEffectsUpToDate)
+}
+
+void MWWorld::InventoryStore::updateMagicEffects(const Ptr& actor)
 {
+    // To avoid excessive updates during auto-equip
+    if (!mUpdatesEnabled)
+        return;
+
     mMagicEffects = MWMechanics::MagicEffects();
 
     for (TSlots::const_iterator iter (mSlots.begin()); iter!=mSlots.end(); ++iter)
-            if (*iter!=end())
     {
+        if (*iter==end())
+            continue;
+
         std::string enchantmentId = MWWorld::Class::get (**iter).getEnchantment (**iter);
 
         if (!enchantmentId.empty())
@@ -269,21 +283,83 @@ const MWMechanics::MagicEffects& MWWorld::InventoryStore::getMagicEffects()
             const ESM::Enchantment& enchantment =
                 *MWBase::Environment::get().getWorld()->getStore().get<ESM::Enchantment>().find (enchantmentId);
 
-                    if (enchantment.mData.mType==ESM::Enchantment::ConstantEffect)
+            if (enchantment.mData.mType != ESM::Enchantment::ConstantEffect)
-                        mMagicEffects.add (enchantment.mEffects);
+                continue;
+
+            // Roll some dice, one for each effect
+            std::vector<float> random;
+            random.resize(enchantment.mEffects.mList.size());
+            for (unsigned int i=0; i<random.size();++i)
+                random[i] = static_cast<float> (std::rand()) / RAND_MAX;
+
+            int i=0;
+            for (std::vector<ESM::ENAMstruct>::const_iterator effectIt (enchantment.mEffects.mList.begin());
+                effectIt!=enchantment.mEffects.mList.end(); ++effectIt)
+            {
+                const ESM::MagicEffect *magicEffect =
+                    MWBase::Environment::get().getWorld()->getStore().get<ESM::MagicEffect>().find (
+                    effectIt->mEffectID);
+
+                if (mPermanentMagicEffectMagnitudes.find((**iter).getCellRef().mRefID) == mPermanentMagicEffectMagnitudes.end())
+                {
+                    // Note that using the RefID as a key here is not entirely correct.
+                    // Consider equipping the same item twice (e.g. a ring)
+                    // However, permanent enchantments with a random magnitude are kind of an exploit anyway,
+                    // so it doesn't really matter if both items will get the same magnitude. *Extreme* edge case.
+                    mPermanentMagicEffectMagnitudes[(**iter).getCellRef().mRefID] = random;
+
+                    // Only the sound of the first effect plays
+                    if (effectIt == enchantment.mEffects.mList.begin())
+                    {
+                        static const std::string schools[] = {
+                            "alteration", "conjuration", "destruction", "illusion", "mysticism", "restoration"
+                        };
+
+                        MWBase::SoundManager *sndMgr = MWBase::Environment::get().getSoundManager();
+                        if(!magicEffect->mHitSound.empty())
+                            sndMgr->playSound3D(actor, magicEffect->mHitSound, 1.0f, 1.0f);
+                        else
+                            sndMgr->playSound3D(actor, schools[magicEffect->mData.mSchool]+" hit", 1.0f, 1.0f);
+                    }
+
+                    if (!magicEffect->mHit.empty())
+                    {
+                        const ESM::Static* castStatic = MWBase::Environment::get().getWorld()->getStore().get<ESM::Static>().find (magicEffect->mHit);
+                        bool loop = magicEffect->mData.mFlags & ESM::MagicEffect::ContinuousVfx;
+                        MWBase::Environment::get().getWorld()->getAnimation(actor)->addEffect("meshes\\" + castStatic->mModel, magicEffect->mIndex, loop, "");
                     }
                 }
 
-        mMagicEffectsUpToDate = true;
+                mMagicEffects.add (*effectIt, random[i]);
+                ++i;
+            }
+        }
     }
 
-    return mMagicEffects;
+    // Now drop expired effects
+    for (TEffectMagnitudes::iterator it = mPermanentMagicEffectMagnitudes.begin();
+         it != mPermanentMagicEffectMagnitudes.end();)
+    {
+        bool found = false;
+        for (TSlots::const_iterator iter (mSlots.begin()); iter!=mSlots.end(); ++iter)
+        {
+            if (*iter == end())
+                continue;
+            if ((**iter).getCellRef().mRefID == it->first)
+            {
+                found = true;
+            }
+        }
+        if (!found)
+            mPermanentMagicEffectMagnitudes.erase(it++);
+        else
+            ++it;
+    }
 }
 
 void MWWorld::InventoryStore::flagAsModified()
 {
     ContainerStore::flagAsModified();
-    mMagicEffectsUpToDate = false;
 }
 
 bool MWWorld::InventoryStore::stacks(const Ptr& ptr1, const Ptr& ptr2)
@@ -394,6 +470,7 @@ MWWorld::ContainerStoreIterator MWWorld::InventoryStore::unequipSlot(int slot, c
         }
 
         updateActorModel(actor);
+        updateMagicEffects(actor);
 
         return retval;
     }
@@ -415,6 +492,6 @@ MWWorld::ContainerStoreIterator MWWorld::InventoryStore::unequipItem(const MWWor
 
 void MWWorld::InventoryStore::updateActorModel(const MWWorld::Ptr& actor)
 {
-    if (mActorModelUpdateEnabled)
+    if (mUpdatesEnabled)
         MWBase::Environment::get().getWorld()->updateAnimParts(actor);
 }

apps/openmw/mwworld/inventorystore.hpp

@@ -43,13 +43,20 @@ namespace MWWorld
 
         private:
 
-            mutable MWMechanics::MagicEffects mMagicEffects;
+            MWMechanics::MagicEffects mMagicEffects;
-            mutable bool mMagicEffectsUpToDate;
+
-            bool mActorModelUpdateEnabled;
+            // Enables updates of magic effects and actor model whenever items are equipped or unequipped.
+            // This is disabled during autoequip to avoid excessive updates
+            bool mUpdatesEnabled;
+
+            // Vanilla allows permanent effects with a random magnitude, so it needs to be stored here.
+            // We also need this to only play sounds and particle effects when the item is equipped, rather than on every update.
+            typedef std::map<std::string, std::vector<float> > TEffectMagnitudes;
+            TEffectMagnitudes mPermanentMagicEffectMagnitudes;
 
             typedef std::vector<ContainerStoreIterator> TSlots;
 
-            mutable TSlots mSlots;
+            TSlots mSlots;
 
             // selected magic item (for using enchantments of type "Cast once" or "Cast when used")
             ContainerStoreIterator mSelectedEnchantItem;
@@ -60,6 +67,8 @@ namespace MWWorld
 
             void updateActorModel (const Ptr& actor);
 
+            void updateMagicEffects(const Ptr& actor);
+
         public:
 
             InventoryStore();
@@ -98,10 +107,8 @@ namespace MWWorld
             void autoEquip (const MWWorld::Ptr& npc);
             ///< Auto equip items according to stats and item value.
 
-            const MWMechanics::MagicEffects& getMagicEffects();
+            const MWMechanics::MagicEffects& getMagicEffects() const;
             ///< Return magic effects from worn items.
-            ///
-            /// \todo make this const again, after the constness of Ptrs and iterators has been addressed.
 
             virtual void flagAsModified();
             ///< \attention This function is internal to the world model and should not be called from