Merge branch 'dispose_lua_ui_correctly' into 'master'

Dispose Lua UI elements correctly

See merge request OpenMW/openmw!1561

12 files changed, 59 insertions, 49 deletions

diff --git a/apps/openmw/mwgui/windowmanagerimp.cpp b/apps/openmw/mwgui/windowmanagerimp.cpp
index 8e7951667b..37807fdf2a 100644
--- a/apps/openmw/mwgui/windowmanagerimp.cpp
+++ b/apps/openmw/mwgui/windowmanagerimp.cpp
@@ -53,7 +53,7 @@
 #include <components/misc/resourcehelpers.hpp>
 #include <components/misc/frameratelimiter.hpp>
 
-#include <components/lua_ui/widgetlist.hpp>
+#include <components/lua_ui/util.hpp>
 
 #include "../mwbase/inputmanager.hpp"
 #include "../mwbase/statemanager.hpp"
@@ -510,6 +510,8 @@ namespace MWGui
     {
         try
         {
+            LuaUi::clearUserInterface();
+
             mStatsWatcher.reset();
 
             MyGUI::LanguageManager::getInstance().eventRequestTag.clear();
diff --git a/apps/openmw/mwlua/luamanagerimp.cpp b/apps/openmw/mwlua/luamanagerimp.cpp
index fc6dc86285..8211c37abf 100644
--- a/apps/openmw/mwlua/luamanagerimp.cpp
+++ b/apps/openmw/mwlua/luamanagerimp.cpp
@@ -12,6 +12,8 @@
 
 #include <components/lua/utilpackage.hpp>
 
+#include <components/lua_ui/util.hpp>
+
 #include "../mwbase/windowmanager.hpp"
 
 #include "../mwworld/class.hpp"
@@ -237,6 +239,7 @@ namespace MWLua
 
     void LuaManager::clear()
     {
+        LuaUi::clearUserInterface();
         mActiveLocalScripts.clear();
         mLocalEvents.clear();
         mGlobalEvents.clear();
@@ -254,7 +257,6 @@ namespace MWLua
             mPlayer.getRefData().setLuaScripts(nullptr);
             mPlayer = MWWorld::Ptr();
         }
-        clearUserInterface();
         mGlobalStorage.clearTemporary();
         mPlayerStorage.clearTemporary();
     }
@@ -454,6 +456,8 @@ namespace MWLua
     void LuaManager::reloadAllScripts()
     {
         Log(Debug::Info) << "Reload Lua";
+
+        LuaUi::clearUserInterface(); 
         mLua.dropScriptCache();
         initConfiguration();
 
@@ -470,7 +474,6 @@ namespace MWLua
                 continue;
             ESM::LuaScripts data;
             scripts->save(data);
-            clearUserInterface();
             scripts->load(data);
         }
         for (LocalScripts* scripts : mActiveLocalScripts)
diff --git a/apps/openmw/mwlua/uibindings.cpp b/apps/openmw/mwlua/uibindings.cpp
index 987304d4c3..61e7c471c1 100644
--- a/apps/openmw/mwlua/uibindings.cpp
+++ b/apps/openmw/mwlua/uibindings.cpp
@@ -1,4 +1,4 @@
-#include <components/lua_ui/widgetlist.hpp>
+#include <components/lua_ui/util.hpp>
 #include <components/lua_ui/element.hpp>
 #include <components/lua_ui/layers.hpp>
 #include <components/lua_ui/content.hpp>
@@ -11,8 +11,6 @@ namespace MWLua
 {
     namespace
     {
-        std::set<LuaUi::Element*> allElements;
-
         class UiAction final : public Action
         {
             public:
@@ -189,7 +187,6 @@ namespace MWLua
         {
             if (element->mDestroy)
                 return;
-            allElements.erase(element.get());
             element->mDestroy = true;
             context.mLuaManager->addAction(std::make_unique<UiAction>(UiAction::DESTROY, element, context.mLua));
         };
@@ -205,8 +202,7 @@ namespace MWLua
         };
         api["create"] = [context](const sol::table& layout)
         {
-            auto element = std::make_shared<LuaUi::Element>(layout);
-            allElements.emplace(element.get());
+            auto element = LuaUi::Element::make(layout);
             context.mLuaManager->addAction(std::make_unique<UiAction>(UiAction::CREATE, element, context.mLua));
             return element;
         };
@@ -244,11 +240,4 @@ namespace MWLua
 
         return LuaUtil::makeReadOnly(api);
     }
-    
-    void clearUserInterface()
-    {
-        for (auto element : allElements)
-            element->destroy();
-        allElements.clear();
-    }
 }
diff --git a/components/CMakeLists.txt b/components/CMakeLists.txt
index 59f1e331e4..89b27b0d94 100644
--- a/components/CMakeLists.txt
+++ b/components/CMakeLists.txt
@@ -162,7 +162,7 @@ add_component_dir (queries
     )
     
 add_component_dir (lua_ui
-    widget widgetlist element layers content
+    widget element util layers content
     text textedit window
     )
 
diff --git a/components/lua_ui/element.cpp b/components/lua_ui/element.cpp
index 5147068a7e..dcb21c32a0 100644
--- a/components/lua_ui/element.cpp
+++ b/components/lua_ui/element.cpp
@@ -3,11 +3,10 @@
 #include <MyGUI_Gui.h>
 
 #include "content.hpp"
-#include "widgetlist.hpp"
+#include "util.hpp"
 
 namespace LuaUi
 {
-
     std::string widgetType(const sol::table& layout)
     {
         return layout.get_or("type", std::string("LuaWidget"));
@@ -70,7 +69,7 @@ namespace LuaUi
         if (!ext)
             throw std::runtime_error("Invalid widget!");
 
-        ext->create(layout.lua_state(), widget);
+        ext->initialize(layout.lua_state(), widget);
         if (parent != nullptr)
             widget->attachToWidget(parent->widget());
 
@@ -87,7 +86,7 @@ namespace LuaUi
 
     void destroyWidget(LuaUi::WidgetExtension* ext)
     {
-        ext->destroy();
+        ext->deinitialize();
         MyGUI::Gui::getInstancePtr()->destroyWidget(ext->widget());
     }
 
@@ -136,6 +135,23 @@ namespace LuaUi
         }
     }
 
+    std::map<Element*, std::shared_ptr<Element>> Element::sAllElements;
+
+    Element::Element(sol::table layout)
+        : mRoot{ nullptr }
+        , mLayout{ std::move(layout) }
+        , mUpdate{ false }
+        , mDestroy{ false }
+    {}
+
+
+    std::shared_ptr<Element> Element::make(sol::table layout)
+    {
+        std::shared_ptr<Element> ptr(new Element(std::move(layout)));
+        sAllElements[ptr.get()] = ptr;
+        return ptr;
+    }
+
     void Element::create()
     {
         assert(!mRoot);
@@ -161,5 +177,6 @@ namespace LuaUi
         if (mRoot)
             destroyWidget(mRoot);
         mRoot = nullptr;
+        sAllElements.erase(this);
     }
 }
diff --git a/components/lua_ui/element.hpp b/components/lua_ui/element.hpp
index 10e10d8960..daaf340660 100644
--- a/components/lua_ui/element.hpp
+++ b/components/lua_ui/element.hpp
@@ -7,13 +7,7 @@ namespace LuaUi
 {
     struct Element
     {
-        Element(sol::table layout)
-            : mRoot{ nullptr }
-            , mLayout{ layout }
-            , mUpdate{ false }
-            , mDestroy{ false }
-        {
-        }
+        static std::shared_ptr<Element> make(sol::table layout);
 
         LuaUi::WidgetExtension* mRoot;
         sol::table mLayout;
@@ -25,6 +19,12 @@ namespace LuaUi
         void update();
 
         void destroy();
+
+        friend void clearUserInterface();
+
+        private:
+            Element(sol::table layout);
+            static std::map<Element*, std::shared_ptr<Element>> sAllElements;
     };
 }
 
diff --git a/components/lua_ui/text.cpp b/components/lua_ui/text.cpp
index 4ae9865ac3..f93f8eecf0 100644
--- a/components/lua_ui/text.cpp
+++ b/components/lua_ui/text.cpp
@@ -7,11 +7,6 @@ namespace LuaUi
         : mAutoSized(true)
     {}
 
-    void LuaText::initialize()
-    {
-        WidgetExtension::initialize();
-    }
-
     void LuaText::setProperties(sol::object props)
     {
         setCaption(parseProperty(props, "caption", std::string()));
diff --git a/components/lua_ui/text.hpp b/components/lua_ui/text.hpp
index d87a9001a2..533dc4f453 100644
--- a/components/lua_ui/text.hpp
+++ b/components/lua_ui/text.hpp
@@ -13,7 +13,6 @@ namespace LuaUi
 
         public:
             LuaText();
-            virtual void initialize() override;
             virtual void setProperties(sol::object) override;
 
         private:
diff --git a/components/lua_ui/widgetlist.cpp b/components/lua_ui/util.cpp
index c2a9bef990..8cadbc9cc2 100644
--- a/components/lua_ui/widgetlist.cpp
+++ b/components/lua_ui/util.cpp
@@ -1,4 +1,4 @@
-#include "widgetlist.hpp"
+#include "util.hpp"
 
 #include <MyGUI_FactoryManager.h>
 
@@ -7,6 +7,8 @@
 #include "textedit.hpp"
 #include "window.hpp"
 
+#include "element.hpp"
+
 namespace LuaUi
 {
 
@@ -28,4 +30,10 @@ namespace LuaUi
         };
         return types;
     }
+
+    void clearUserInterface()
+    {
+        while (!Element::sAllElements.empty())
+            Element::sAllElements.begin()->second->destroy();
+    }
 }
diff --git a/components/lua_ui/widgetlist.hpp b/components/lua_ui/util.hpp
index ff033fb6ca..3851e6c947 100644
--- a/components/lua_ui/widgetlist.hpp
+++ b/components/lua_ui/util.hpp
@@ -9,6 +9,8 @@ namespace LuaUi
     void registerAllWidgets();
 
     const std::unordered_map<std::string, std::string>& widgetTypeToName();
+
+    void clearUserInterface();
 }
 
 #endif // OPENMW_LUAUI_WIDGETLIST
diff --git a/components/lua_ui/widget.cpp b/components/lua_ui/widget.cpp
index 6d7bb5063c..50c6a193e6 100644
--- a/components/lua_ui/widget.cpp
+++ b/components/lua_ui/widget.cpp
@@ -26,7 +26,7 @@ namespace LuaUi
             it->second(argument, mLayout);
     }
 
-    void WidgetExtension::create(lua_State* lua, MyGUI::Widget* self)
+    void WidgetExtension::initialize(lua_State* lua, MyGUI::Widget* self)
     {
         mLua = lua;
         mWidget = self;
@@ -54,17 +54,9 @@ namespace LuaUi
         mWidget->eventKeyLostFocus += MyGUI::newDelegate(this, &WidgetExtension::focusLoss);
     }
 
-    void WidgetExtension::destroy()
-    {
-        clearCallbacks();
-        deinitialize();
-
-        for (WidgetExtension* child : mContent)
-            child->destroy();
-    }
-
     void WidgetExtension::deinitialize()
     {
+        clearCallbacks();
         mWidget->eventKeyButtonPressed.clear();
         mWidget->eventKeyButtonReleased.clear();
         mWidget->eventMouseButtonClick.clear();
@@ -78,6 +70,9 @@ namespace LuaUi
         mWidget->eventMouseLostFocus.clear();
         mWidget->eventKeySetFocus.clear();
         mWidget->eventKeyLostFocus.clear();
+
+        for (WidgetExtension* child : mContent)
+            child->deinitialize();
     }
 
     sol::table WidgetExtension::makeTable() const
diff --git a/components/lua_ui/widget.hpp b/components/lua_ui/widget.hpp
index 99d430f71c..f8ba105be5 100644
--- a/components/lua_ui/widget.hpp
+++ b/components/lua_ui/widget.hpp
@@ -22,9 +22,9 @@ namespace LuaUi
     public:
         WidgetExtension();
         // must be called after creating the underlying MyGUI::Widget
-        void create(lua_State* lua, MyGUI::Widget* self);
+        void initialize(lua_State* lua, MyGUI::Widget* self);
         // must be called after before destroying the underlying MyGUI::Widget
-        void destroy();
+        virtual void deinitialize();
 
         void addChild(WidgetExtension* ext);
         WidgetExtension* childAt(size_t index) const;
@@ -46,11 +46,11 @@ namespace LuaUi
         void setLayout(const sol::table& layout) { mLayout = layout; }
 
     protected:
+        virtual void initialize();
         sol::table makeTable() const;
         sol::object keyEvent(MyGUI::KeyCode) const;
         sol::object mouseEvent(int left, int top, MyGUI::MouseButton button) const;
-        virtual void initialize();
-        virtual void deinitialize();
+        
         virtual MyGUI::IntSize calculateSize();
         virtual MyGUI::IntPoint calculatePosition(const MyGUI::IntSize& size);
         MyGUI::IntCoord calculateCoord();