Use an inventory store listener for animation parts and VFX update instead of updating them directly. Slightly more flexible, reduces InventoryStore dependencies and solves a crash during character creation due to the preview doll's animation not being registered in World.

apps/openmw/mwrender/characterpreview.cpp

@@ -71,7 +71,7 @@ namespace MWRender
         mNode = renderRoot->createChildSceneNode();
 
         mAnimation = new NpcAnimation(mCharacter, mNode,
-                                      0, (renderHeadOnly() ? NpcAnimation::VM_HeadOnly : NpcAnimation::VM_Normal));
+                                      0, true, (renderHeadOnly() ? NpcAnimation::VM_HeadOnly : NpcAnimation::VM_Normal));
         mAnimation->updateParts();
 
         Ogre::Vector3 scale = mNode->getScale();
@@ -102,7 +102,6 @@ namespace MWRender
     {
         if (mSceneMgr)
         {
-            //Ogre::TextureManager::getSingleton().remove(mName);
             mSceneMgr->destroyAllCameras();
             delete mAnimation;
             Ogre::Root::getSingleton().destroySceneManager(mSceneMgr);
@@ -114,7 +113,7 @@ namespace MWRender
         assert(mAnimation);
         delete mAnimation;
         mAnimation = new NpcAnimation(mCharacter, mNode,
-                                      0, (renderHeadOnly() ? NpcAnimation::VM_HeadOnly : NpcAnimation::VM_Normal));
+                                      0, true, (renderHeadOnly() ? NpcAnimation::VM_HeadOnly : NpcAnimation::VM_Normal));
         mAnimation->updateParts();
 
         float scale=1.f;

apps/openmw/mwrender/npcanimation.cpp

@@ -14,6 +14,7 @@
 #include "../mwbase/environment.hpp"
 #include "../mwbase/world.hpp"
 #include "../mwbase/mechanicsmanager.hpp"
+#include "../mwbase/soundmanager.hpp"
 
 #include "renderconst.hpp"
 #include "camera.hpp"
@@ -58,17 +59,19 @@ const NpcAnimation::PartBoneMap NpcAnimation::sPartList = createPartListMap();
 
 NpcAnimation::~NpcAnimation()
 {
+    if (!mListenerDisabled)
+        mPtr.getClass().getInventoryStore(mPtr).setListener(NULL);
+
     Ogre::SceneManager *sceneMgr = mInsert->getCreator();
     for(size_t i = 0;i < ESM::PRT_Count;i++)
         destroyObjectList(sceneMgr, mObjectParts[i]);
 }
 
 
-NpcAnimation::NpcAnimation(const MWWorld::Ptr& ptr, Ogre::SceneNode* node, int visibilityFlags, ViewMode viewMode)
+NpcAnimation::NpcAnimation(const MWWorld::Ptr& ptr, Ogre::SceneNode* node, int visibilityFlags, bool disableListener, ViewMode viewMode)
   : Animation(ptr, node),
-    mStateID(-1),
     mVisibilityFlags(visibilityFlags),
-
+    mListenerDisabled(disableListener),
     mViewMode(viewMode),
     mShowWeapons(false),
     mFirstPersonOffset(0.f, 0.f, 0.f)
@@ -80,6 +83,11 @@ NpcAnimation::NpcAnimation(const MWWorld::Ptr& ptr, Ogre::SceneNode* node, int v
         mPartslots[i] = -1;  //each slot is empty
         mPartPriorities[i] = 0;
     }
+
+    if (!disableListener)
+        mPtr.getClass().getInventoryStore(mPtr).setListener(this);
+
+    updateNpcBase();
 }
 
 void NpcAnimation::setViewMode(NpcAnimation::ViewMode viewMode)
@@ -162,13 +170,6 @@ void NpcAnimation::updateNpcBase()
 
 void NpcAnimation::updateParts()
 {
-    if (!mSkelBase)
-    {
-        // First update?
-        updateNpcBase();
-        return;
-    }
-
     const MWWorld::Class &cls = MWWorld::Class::get(mPtr);
     MWWorld::InventoryStore &inv = cls.getInventoryStore(mPtr);
 
@@ -417,9 +418,6 @@ NifOgre::ObjectList NpcAnimation::insertBoundedPart(const std::string &model, in
 
 Ogre::Vector3 NpcAnimation::runAnimation(float timepassed)
 {
-    if (!mSkelBase)
-        updateNpcBase();
-
     Ogre::Vector3 ret = Animation::runAnimation(timepassed);
 
     Ogre::SkeletonInstance *baseinst = mSkelBase->getSkeleton();
@@ -586,4 +584,32 @@ void NpcAnimation::showWeapons(bool showWeapon)
     }
 }
 
+void NpcAnimation::permanentEffectAdded(const ESM::MagicEffect *magicEffect, bool isNew, bool playSound)
+{
+    // During first auto equip, we don't play any sounds.
+    // Basically we don't want sounds when the actor is first loaded,
+    // the items should appear as if they'd always been equipped.
+    if (playSound)
+    {
+        static const std::string schools[] = {
+            "alteration", "conjuration", "destruction", "illusion", "mysticism", "restoration"
+        };
+
+        MWBase::SoundManager *sndMgr = MWBase::Environment::get().getSoundManager();
+        if(!magicEffect->mHitSound.empty())
+            sndMgr->playSound3D(mPtr, magicEffect->mHitSound, 1.0f, 1.0f);
+        else
+            sndMgr->playSound3D(mPtr, 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;
+        // Don't play particle VFX unless the effect is new or it should be looping.
+        if (isNew || loop)
+            addEffect("meshes\\" + castStatic->mModel, magicEffect->mIndex, loop, "");
+    }
+}
+
 }

apps/openmw/mwrender/npcanimation.hpp

@@ -3,23 +3,22 @@
 
 #include "animation.hpp"
 
-#include "../mwworld/containerstore.hpp"
+#include "../mwworld/inventorystore.hpp"
 
 namespace ESM
 {
     struct NPC;
 }
 
-namespace MWWorld
-{
-    class InventoryStore;
-}
-
 namespace MWRender
 {
 
-class NpcAnimation : public Animation
+class NpcAnimation : public Animation, public MWWorld::InventoryStoreListener
 {
+public:
+    virtual void equipmentChanged() { updateParts(); }
+    virtual void permanentEffectAdded(const ESM::MagicEffect *magicEffect, bool isNew, bool playSound);
+
 public:
     typedef std::map<ESM::PartReferenceType,std::string> PartBoneMap;
 
@@ -32,7 +31,7 @@ public:
 private:
     static const PartBoneMap sPartList;
 
-    int mStateID;
+    bool mListenerDisabled;
 
     // Bounded Parts
     NifOgre::ObjectList mObjectParts[ESM::PRT_Count];
@@ -62,7 +61,17 @@ private:
     void addPartGroup(int group, int priority, const std::vector<ESM::PartReference> &parts);
 
 public:
-    NpcAnimation(const MWWorld::Ptr& ptr, Ogre::SceneNode* node, int visibilityFlags,
+    /**
+     * @param ptr
+     * @param node
+     * @param visibilityFlags
+     * @param disableListener  Don't listen for equipment changes and magic effects. InventoryStore only supports
+     *                         one listener at a time, so you shouldn't do this if creating several NpcAnimations
+     *                         for the same Ptr, eg preview dolls for the player.
+     *                         Those need to be manually rendered anyway.
+     * @param viewMode
+     */
+    NpcAnimation(const MWWorld::Ptr& ptr, Ogre::SceneNode* node, int visibilityFlags, bool disableListener = false,
                  ViewMode viewMode=VM_Normal);
     virtual ~NpcAnimation();
 

apps/openmw/mwscript/containerextensions.cpp

@@ -135,7 +135,10 @@ namespace MWScript
 
                     MWWorld::ContainerStore& store = MWWorld::Class::get (ptr).getContainerStore (ptr);
 
-                    std::string itemName = "";
+                    std::string itemName;
+                    for (MWWorld::ContainerStoreIterator iter(store.begin()); iter != store.end(); ++iter)
+                        if (Misc::StringUtils::ciEqual(iter->getCellRef().mRefID, item))
+                            itemName = iter->getClass().getName(*iter);
 
                     int numRemoved = store.remove(item, count, ptr);
 

apps/openmw/mwworld/containerstore.cpp

@@ -63,7 +63,7 @@ namespace
     }
 }
 
-MWWorld::ContainerStore::ContainerStore() : mStateId (0), mCachedWeight (0), mWeightUpToDate (false) {}
+MWWorld::ContainerStore::ContainerStore() : mCachedWeight (0), mWeightUpToDate (false) {}
 
 MWWorld::ContainerStore::~ContainerStore() {}
 

apps/openmw/mwworld/containerstore.hpp

@@ -49,7 +49,6 @@ namespace MWWorld
             MWWorld::CellRefList<ESM::Probe>             probes;
             MWWorld::CellRefList<ESM::Repair>            repairs;
             MWWorld::CellRefList<ESM::Weapon>            weapons;
-            int mStateId;
             mutable float mCachedWeight;
             mutable bool mWeightUpToDate;
             ContainerStoreIterator addImp (const Ptr& ptr);

apps/openmw/mwworld/inventorystore.cpp

@@ -9,9 +9,6 @@
 #include "../mwbase/environment.hpp"
 #include "../mwbase/world.hpp"
 #include "../mwbase/windowmanager.hpp"
-#include "../mwbase/soundmanager.hpp"
-
-#include "../mwrender/animation.hpp"
 
 #include "../mwmechanics/npcstats.hpp"
 
@@ -46,6 +43,7 @@ MWWorld::InventoryStore::InventoryStore()
  : mSelectedEnchantItem(end())
  , mUpdatesEnabled (true)
  , mFirstAutoEquip(true)
+ , mListener(NULL)
 {
     initSlots (mSlots);
 }
@@ -53,6 +51,8 @@ MWWorld::InventoryStore::InventoryStore()
 MWWorld::InventoryStore::InventoryStore (const InventoryStore& store)
 : ContainerStore (store)
  , mSelectedEnchantItem(end())
+ , mListener(NULL)
+ , mUpdatesEnabled(true)
 {
     mMagicEffects = store.mMagicEffects;
     mFirstAutoEquip = store.mFirstAutoEquip;
@@ -122,9 +122,8 @@ void MWWorld::InventoryStore::equip (int slot, const ContainerStoreIterator& ite
 
     flagAsModified();
 
-    updateActorModel(actor);
+    fireEquipmentChangedEvent();
-
+    updateMagicEffects();
-    updateMagicEffects(actor);
 }
 
 void MWWorld::InventoryStore::unequipAll(const MWWorld::Ptr& actor)
@@ -255,11 +254,10 @@ void MWWorld::InventoryStore::autoEquip (const MWWorld::Ptr& npc)
     if (changed)
     {
         mSlots.swap (slots_);
-        updateActorModel(npc);
+        fireEquipmentChangedEvent();
-        updateMagicEffects(npc);
+        updateMagicEffects();
         flagAsModified();
     }
-    mFirstAutoEquip = false;
 }
 
 const MWMechanics::MagicEffects& MWWorld::InventoryStore::getMagicEffects() const
@@ -267,12 +265,16 @@ const MWMechanics::MagicEffects& MWWorld::InventoryStore::getMagicEffects() cons
     return mMagicEffects;
 }
 
-void MWWorld::InventoryStore::updateMagicEffects(const Ptr& actor)
+void MWWorld::InventoryStore::updateMagicEffects()
 {
     // To avoid excessive updates during auto-equip
     if (!mUpdatesEnabled)
         return;
 
+    // Delay update until the listener is set up
+    if (!mListener)
+        return;
+
     mMagicEffects = MWMechanics::MagicEffects();
 
     for (TSlots::const_iterator iter (mSlots.begin()); iter!=mSlots.end(); ++iter)
@@ -296,6 +298,16 @@ void MWWorld::InventoryStore::updateMagicEffects(const Ptr& actor)
             for (unsigned int i=0; i<random.size();++i)
                 random[i] = static_cast<float> (std::rand()) / RAND_MAX;
 
+            bool existed = (mPermanentMagicEffectMagnitudes.find((**iter).getCellRef().mRefID) != mPermanentMagicEffectMagnitudes.end());
+            if (!existed)
+            {
+                // 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;
+            }
+
             int i=0;
             for (std::vector<ESM::ENAMstruct>::const_iterator effectIt (enchantment.mEffects.mList.begin());
                 effectIt!=enchantment.mEffects.mList.end(); ++effectIt)
@@ -304,42 +316,13 @@ void MWWorld::InventoryStore::updateMagicEffects(const Ptr& actor)
                     MWBase::Environment::get().getWorld()->getStore().get<ESM::MagicEffect>().find (
                     effectIt->mEffectID);
 
-                if (mPermanentMagicEffectMagnitudes.find((**iter).getCellRef().mRefID) == mPermanentMagicEffectMagnitudes.end())
+                if (!existed)
                 {
-                    // 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;
-
                     // During first auto equip, we don't play any sounds.
                     // Basically we don't want sounds when the actor is first loaded,
                     // the items should appear as if they'd always been equipped.
-                    if (!mFirstAutoEquip)
+                    mListener->permanentEffectAdded(magicEffect, !mFirstAutoEquip,
-                    {
+                                                        !mFirstAutoEquip && effectIt == enchantment.mEffects.mList.begin());
-                        // 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;
-                        // Similar as above, we don't want particles during first autoequip either, unless they're continuous.
-                        if (!mFirstAutoEquip || loop)
-                            MWBase::Environment::get().getWorld()->getAnimation(actor)->addEffect("meshes\\" + castStatic->mModel, magicEffect->mIndex, loop, "");
-                    }
                 }
 
                 mMagicEffects.add (*effectIt, random[i]);
@@ -367,6 +350,8 @@ void MWWorld::InventoryStore::updateMagicEffects(const Ptr& actor)
         else
             ++it;
     }
+
+    mFirstAutoEquip = false;
 }
 
 void MWWorld::InventoryStore::flagAsModified()
@@ -380,7 +365,7 @@ bool MWWorld::InventoryStore::stacks(const Ptr& ptr1, const Ptr& ptr2)
     if (!canStack)
         return false;
 
-    // don't stack if 'stack' (the item being checked against) is currently equipped.
+    // don't stack if either item is currently equipped
     for (TSlots::const_iterator iter (mSlots.begin());
         iter!=mSlots.end(); ++iter)
     {
@@ -481,8 +466,8 @@ MWWorld::ContainerStoreIterator MWWorld::InventoryStore::unequipSlot(int slot, c
             }
         }
 
-        updateActorModel(actor);
+        fireEquipmentChangedEvent();
-        updateMagicEffects(actor);
+        updateMagicEffects();
 
         return retval;
     }
@@ -502,8 +487,16 @@ MWWorld::ContainerStoreIterator MWWorld::InventoryStore::unequipItem(const MWWor
     throw std::runtime_error ("attempt to unequip an item that is not currently equipped");
 }
 
-void MWWorld::InventoryStore::updateActorModel(const MWWorld::Ptr& actor)
+void MWWorld::InventoryStore::setListener(InventoryStoreListener *listener)
 {
-    if (mUpdatesEnabled)
+    mListener = listener;
-        MWBase::Environment::get().getWorld()->updateAnimParts(actor);
+    updateMagicEffects();
+}
+
+void MWWorld::InventoryStore::fireEquipmentChangedEvent()
+{
+    if (!mUpdatesEnabled)
+        return;
+    if (mListener)
+        mListener->equipmentChanged();
 }

apps/openmw/mwworld/inventorystore.hpp

@@ -12,6 +12,25 @@ namespace MWMechanics
 
 namespace MWWorld
 {
+    class InventoryStoreListener
+    {
+    public:
+        /**
+         * Fired when items are equipped or unequipped
+         */
+        virtual void equipmentChanged () {}
+
+        /**
+         * @param effect
+         * @param isNew Is this effect new (e.g. the item for it was just now manually equipped)
+         *              or was it loaded from a savegame / initial game state? \n
+         *              If it isn't new, non-looping VFX should not be played.
+         * @param playSound Play effect sound?
+         */
+        virtual void permanentEffectAdded (const ESM::MagicEffect *magicEffect, bool isNew, bool playSound) {}
+
+    };
+
     ///< \brief Variant of the ContainerStore for NPCs
     class InventoryStore : public ContainerStore
     {
@@ -45,6 +64,8 @@ namespace MWWorld
 
             MWMechanics::MagicEffects mMagicEffects;
 
+            InventoryStoreListener* mListener;
+
             // 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;
@@ -67,9 +88,9 @@ namespace MWWorld
 
             void initSlots (TSlots& slots_);
 
-            void updateActorModel (const Ptr& actor);
+            void updateMagicEffects();
 
-            void updateMagicEffects(const Ptr& actor);
+            void fireEquipmentChangedEvent();
 
         public:
 
@@ -138,6 +159,9 @@ namespace MWWorld
             /// @return an iterator to the item that was previously in the slot
             /// (it can be re-stacked so its count may be different than when it
             /// was equipped).
+
+            void setListener (InventoryStoreListener* listener);
+            ///< Set a listener for various events, see \a InventoryStoreListener
     };
 }