Do not add scripts from levelled creatures twice (Bug #2806)

Do not insert objects from within a CellStore visitor

apps/openmw/mwworld/cellstore.hpp

@@ -246,6 +246,7 @@ namespace MWWorld
             /// Call visitor (MWWorld::Ptr) for each reference. visitor must return a bool. Returning
             /// false will abort the iteration.
             /// \note Prefer using forEachConst when possible.
+            /// \note Do not modify this cell (i.e. remove/add objects) during the forEach, doing this may result in unintended behaviour.
             /// \attention This function also lists deleted (count 0) objects!
             /// \return Iteration completed?
             template<class Visitor>
@@ -269,6 +270,7 @@ namespace MWWorld
 
             /// Call visitor (MWWorld::ConstPtr) for each reference. visitor must return a bool. Returning
             /// false will abort the iteration.
+            /// \note Do not modify this cell (i.e. remove/add objects) during the forEach, doing this may result in unintended behaviour.
             /// \attention This function also lists deleted (count 0) objects!
             /// \return Iteration completed?
             template<class Visitor>
@@ -291,6 +293,7 @@ namespace MWWorld
 
             /// Call visitor (ref) for each reference of given type. visitor must return a bool. Returning
             /// false will abort the iteration.
+            /// \note Do not modify this cell (i.e. remove/add objects) during the forEach, doing this may result in unintended behaviour.
             /// \attention This function also lists deleted (count 0) objects!
             /// \return Iteration completed?
             template <class T, class Visitor>

apps/openmw/mwworld/scene.cpp

@@ -86,10 +86,13 @@ namespace
         MWPhysics::PhysicsSystem& mPhysics;
         MWRender::RenderingManager& mRendering;
 
+        std::vector<MWWorld::Ptr> mToInsert;
+
         InsertVisitor (MWWorld::CellStore& cell, bool rescale, Loading::Listener& loadingListener,
             MWPhysics::PhysicsSystem& physics, MWRender::RenderingManager& rendering);
 
         bool operator() (const MWWorld::Ptr& ptr);
+        void insert();
     };
 
     InsertVisitor::InsertVisitor (MWWorld::CellStore& cell, bool rescale,
@@ -102,31 +105,41 @@ namespace
 
     bool InsertVisitor::operator() (const MWWorld::Ptr& ptr)
     {
-        if (mRescale)
-        {
-            if (ptr.getCellRef().getScale()<0.5)
-                ptr.getCellRef().setScale(0.5);
-            else if (ptr.getCellRef().getScale()>2)
-                ptr.getCellRef().setScale(2);
-        }
+        // do not insert directly as we can't modify the cell from within the visitation
+        // CreatureLevList::insertObjectRendering may spawn a new creature
+        mToInsert.push_back(ptr);
+        return true;
+    }
 
-        if (!ptr.getRefData().isDeleted() && ptr.getRefData().isEnabled())
+    void InsertVisitor::insert()
+    {
+        for (std::vector<MWWorld::Ptr>::iterator it = mToInsert.begin(); it != mToInsert.end(); ++it)
         {
-            try
+            MWWorld::Ptr ptr = *it;
+            if (mRescale)
             {
-                addObject(ptr, mPhysics, mRendering);
-                updateObjectRotation(ptr, mPhysics, mRendering, false);
+                if (ptr.getCellRef().getScale()<0.5)
+                    ptr.getCellRef().setScale(0.5);
+                else if (ptr.getCellRef().getScale()>2)
+                    ptr.getCellRef().setScale(2);
             }
-            catch (const std::exception& e)
+
+            if (!ptr.getRefData().isDeleted() && ptr.getRefData().isEnabled())
             {
-                std::string error ("error during rendering '" + ptr.getCellRef().getRefId() + "': ");
-                std::cerr << error + e.what() << std::endl;
+                try
+                {
+                    addObject(ptr, mPhysics, mRendering);
+                    updateObjectRotation(ptr, mPhysics, mRendering, false);
+                }
+                catch (const std::exception& e)
+                {
+                    std::string error ("error during rendering '" + ptr.getCellRef().getRefId() + "': ");
+                    std::cerr << error + e.what() << std::endl;
+                }
             }
-        }
-
-        mLoadingListener.increaseProgress (1);
 
-        return true;
+            mLoadingListener.increaseProgress (1);
+        }
     }
 
     struct AdjustPositionVisitor
@@ -248,6 +261,10 @@ namespace MWWorld
 
             cell->respawn();
 
+            // register local scripts
+            // do this before insertCell, to make sure we don't add scripts from levelled creature spawning twice
+            MWBase::Environment::get().getWorld()->getLocalScripts().addCell (cell);
+
             // ... then references. This is important for adjustPosition to work correctly.
             /// \todo rescale depending on the state of a new GMST
             insertCell (*cell, true, loadingListener);
@@ -267,10 +284,6 @@ namespace MWWorld
             if (!cell->isExterior() && !(cell->getCell()->mData.mFlags & ESM::Cell::QuasiEx))
                 mRendering.configureAmbient(cell->getCell());
         }
-
-        // register local scripts
-        // ??? Should this go into the above if block ???
-        MWBase::Environment::get().getWorld()->getLocalScripts().addCell (cell);
     }
 
     void Scene::changeToVoid()
@@ -534,6 +547,7 @@ namespace MWWorld
     {
         InsertVisitor insertVisitor (cell, rescale, *loadingListener, *mPhysics, mRendering);
         cell.forEach (insertVisitor);
+        insertVisitor.insert();
 
         // do adjustPosition (snapping actors to ground) after objects are loaded, so we don't depend on the loading order
         AdjustPositionVisitor adjustPosVisitor;