From 187d2bccda53da425f49bc7e652bee29c63822cf Mon Sep 17 00:00:00 2001 From: scrawl Date: Wed, 3 Feb 2016 16:06:25 +0100 Subject: [PATCH 1/2] Warn about adding a local script twice (Bug #2806) --- apps/openmw/mwworld/localscripts.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/openmw/mwworld/localscripts.cpp b/apps/openmw/mwworld/localscripts.cpp index 2004a2ff3..374f5c632 100644 --- a/apps/openmw/mwworld/localscripts.cpp +++ b/apps/openmw/mwworld/localscripts.cpp @@ -110,6 +110,14 @@ void MWWorld::LocalScripts::add (const std::string& scriptName, const Ptr& ptr) { ptr.getRefData().setLocals (*script); + for (std::list >::iterator iter = mScripts.begin(); iter!=mScripts.end(); ++iter) + if (iter->second==ptr) + { + std::cout << "warning, tried to add local script twice for " << ptr.getCellRef().getRefId() << std::endl; + remove(ptr); + break; + } + mScripts.push_back (std::make_pair (scriptName, ptr)); } catch (const std::exception& exception) From cc3563359e3ab2d7e3b02feb0b62533ed03e3d64 Mon Sep 17 00:00:00 2001 From: scrawl Date: Wed, 3 Feb 2016 16:09:20 +0100 Subject: [PATCH 2/2] Refactor local script iteration (Fixes #2806, Fixes #3108) This should be much safer. Don't use recursion. Don't fail if mIgnore happens to be in the list twice. Don't rely on preconditions / assertions. --- apps/openmw/engine.cpp | 6 ++---- apps/openmw/mwworld/localscripts.cpp | 28 ++++++++-------------------- apps/openmw/mwworld/localscripts.hpp | 8 +++----- 3 files changed, 13 insertions(+), 29 deletions(-) diff --git a/apps/openmw/engine.cpp b/apps/openmw/engine.cpp index 3fcd46f7c..f347bc350 100644 --- a/apps/openmw/engine.cpp +++ b/apps/openmw/engine.cpp @@ -69,11 +69,9 @@ void OMW::Engine::executeLocalScripts() MWWorld::LocalScripts& localScripts = mEnvironment.getWorld()->getLocalScripts(); localScripts.startIteration(); - - while (!localScripts.isFinished()) + std::pair script; + while (localScripts.getNext(script)) { - std::pair script = localScripts.getNext(); - MWScript::InterpreterContext interpreterContext ( &script.second.getRefData().getLocals(), script.second); mEnvironment.getScriptManager()->run (script.first, interpreterContext); diff --git a/apps/openmw/mwworld/localscripts.cpp b/apps/openmw/mwworld/localscripts.cpp index 374f5c632..7ccc213c4 100644 --- a/apps/openmw/mwworld/localscripts.cpp +++ b/apps/openmw/mwworld/localscripts.cpp @@ -76,32 +76,20 @@ void MWWorld::LocalScripts::startIteration() mIter = mScripts.begin(); } -bool MWWorld::LocalScripts::isFinished() const +bool MWWorld::LocalScripts::getNext(std::pair& script) { - if (mIter==mScripts.end()) - return true; - - if (!mIgnore.isEmpty() && mIter->second==mIgnore) + while (mIter!=mScripts.end()) { - std::list >::iterator iter = mIter; - return ++iter==mScripts.end(); + std::list >::iterator iter = mIter++; + if (mIgnore.isEmpty() || iter->second!=mIgnore) + { + script = *iter; + return true; + } } - return false; } -std::pair MWWorld::LocalScripts::getNext() -{ - assert (!isFinished()); - - std::list >::iterator iter = mIter++; - - if (mIgnore.isEmpty() || iter->second!=mIgnore) - return *iter; - - return getNext(); -} - void MWWorld::LocalScripts::add (const std::string& scriptName, const Ptr& ptr) { if (const ESM::Script *script = mStore.get().search (scriptName)) diff --git a/apps/openmw/mwworld/localscripts.hpp b/apps/openmw/mwworld/localscripts.hpp index 6ef4633a1..6c2118ea8 100644 --- a/apps/openmw/mwworld/localscripts.hpp +++ b/apps/openmw/mwworld/localscripts.hpp @@ -31,11 +31,9 @@ namespace MWWorld void startIteration(); ///< Set the iterator to the begin of the script list. - bool isFinished() const; - ///< Is iteration finished? - - std::pair getNext(); - ///< Get next local script (must not be called if isFinished()) + bool getNext(std::pair& script); + ///< Get next local script + /// @return Did we get a script? void add (const std::string& scriptName, const Ptr& ptr); ///< Add script to collection of active local scripts.