Index: source/gui/GUIManager.h =================================================================== --- source/gui/GUIManager.h +++ source/gui/GUIManager.h @@ -25,6 +25,7 @@ #include "scriptinterface/StructuredClone.h" #include +#include #include #include @@ -96,8 +97,8 @@ /** * See CGUI::SendEventToAll; applies to the currently active page. */ - void SendEventToAll(const CStr& eventName) const; - void SendEventToAll(const CStr& eventName, JS::HandleValueArray paramData) const; + void SendEventToAll(const CStr& eventName); + void SendEventToAll(const CStr& eventName, JS::HandleValueArray paramData); /** * See CGUI::TickObjects; applies to @em all loaded pages. @@ -130,6 +131,17 @@ void DisplayLoadProgress(int percent, const wchar_t* pending_task); private: + /** + * Copy the m_CurrentPages to m_NextPages if it needs to. Has to be called before the page stack is + * edited while iterating. + */ + void MakeNextPages(); + /** + * Copy m_NextPages back to m_CurrentPages if it needs to. Has to be called after JS-code might have + * edited the stack and we finished iterating. + */ + void UpdatePages(); + struct SGUIPage { // COPYABLE, because event handlers may invalidate page stack iterators by open or close pages, @@ -178,7 +190,8 @@ * Therefore use std::deque over std::vector. */ using PageStackType = std::deque; - PageStackType m_PageStack; + PageStackType m_CurrentPages; + std::optional m_NextPages; CTemplateLoader m_TemplateLoader; }; Index: source/gui/GUIManager.cpp =================================================================== --- source/gui/GUIManager.cpp +++ source/gui/GUIManager.cpp @@ -88,11 +88,15 @@ size_t CGUIManager::GetPageCount() const { - return m_PageStack.size(); + if (m_NextPages.has_value()) + return m_NextPages->size(); + + return m_CurrentPages.size(); } void CGUIManager::SwitchPage(const CStrW& pageName, const ScriptInterface* srcScriptInterface, JS::HandleValue initData) { + MakeNextPages(); // The page stack is cleared (including the script context where initData came from), // therefore we have to clone initData. @@ -103,11 +107,11 @@ initDataClone = Script::WriteStructuredClone(rq, initData); } - if (!m_PageStack.empty()) + if (!m_NextPages->empty()) { // Make sure we unfocus anything on the current page. - m_PageStack.back().gui->SendFocusMessage(GUIM_LOST_FOCUS); - m_PageStack.clear(); + m_NextPages->back().gui->SendFocusMessage(GUIM_LOST_FOCUS); + m_NextPages->clear(); } PushPage(pageName, initDataClone, JS::UndefinedHandleValue); @@ -115,37 +119,39 @@ void CGUIManager::PushPage(const CStrW& pageName, Script::StructuredClone initData, JS::HandleValue callbackFunction) { + MakeNextPages(); // Store the callback handler in the current GUI page before opening the new one - if (!m_PageStack.empty() && !callbackFunction.isUndefined()) + if (!m_NextPages->empty() && !callbackFunction.isUndefined()) { - m_PageStack.back().SetCallbackFunction(*m_ScriptInterface, callbackFunction); + m_NextPages->back().SetCallbackFunction(*m_ScriptInterface, callbackFunction); // Make sure we unfocus anything on the current page. - m_PageStack.back().gui->SendFocusMessage(GUIM_LOST_FOCUS); + m_NextPages->back().gui->SendFocusMessage(GUIM_LOST_FOCUS); } // Push the page prior to loading its contents, because that may push // another GUI page on init which should be pushed on top of this new page. - m_PageStack.emplace_back(pageName, initData); - m_PageStack.back().LoadPage(m_ScriptContext); + m_NextPages->emplace_back(pageName, initData); + m_NextPages->back().LoadPage(m_ScriptContext); } void CGUIManager::PopPage(Script::StructuredClone args) { - if (m_PageStack.size() < 2) + MakeNextPages(); + if (m_NextPages->size() < 2) { debug_warn(L"Tried to pop GUI page when there's < 2 in the stack"); return; } // Make sure we unfocus anything on the current page. - m_PageStack.back().gui->SendFocusMessage(GUIM_LOST_FOCUS); + m_NextPages->back().gui->SendFocusMessage(GUIM_LOST_FOCUS); - m_PageStack.pop_back(); - m_PageStack.back().PerformCallbackFunction(args); + m_NextPages->pop_back(); + m_NextPages->back().PerformCallbackFunction(args); // We return to a page where some object might have been focused. - m_PageStack.back().gui->SendFocusMessage(GUIM_GOT_FOCUS); + m_NextPages->back().gui->SendFocusMessage(GUIM_GOT_FOCUS); } CGUIManager::SGUIPage::SGUIPage(const CStrW& pageName, const Script::StructuredClone initData) @@ -292,12 +298,13 @@ Status CGUIManager::ReloadChangedFile(const VfsPath& path) { - for (SGUIPage& p : m_PageStack) + UpdatePages(); + + for (SGUIPage& p : m_CurrentPages) if (p.inputs.find(path) != p.inputs.end()) { LOGMESSAGE("GUI file '%s' changed - reloading page '%s'", path.string8(), utf8_from_wstring(p.m_Name)); p.LoadPage(m_ScriptContext); - // TODO: this can crash if LoadPage runs an init script which modifies the page stack and breaks our iterators } return INFO::OK; @@ -305,8 +312,9 @@ Status CGUIManager::ReloadAllPages() { - // TODO: this can crash if LoadPage runs an init script which modifies the page stack and breaks our iterators - for (SGUIPage& p : m_PageStack) + UpdatePages(); + + for (SGUIPage& p : m_CurrentPages) p.LoadPage(m_ScriptContext); return INFO::OK; @@ -354,22 +362,19 @@ return IN_PASS; } -void CGUIManager::SendEventToAll(const CStr& eventName) const +void CGUIManager::SendEventToAll(const CStr& eventName) { - // Save an immutable copy so iterators aren't invalidated by handlers - PageStackType pageStack = m_PageStack; + UpdatePages(); - for (const SGUIPage& p : pageStack) + for (const SGUIPage& p : m_CurrentPages) p.gui->SendEventToAll(eventName); - } -void CGUIManager::SendEventToAll(const CStr& eventName, JS::HandleValueArray paramData) const +void CGUIManager::SendEventToAll(const CStr& eventName, JS::HandleValueArray paramData) { - // Save an immutable copy so iterators aren't invalidated by handlers - PageStackType pageStack = m_PageStack; + UpdatePages(); - for (const SGUIPage& p : pageStack) + for (const SGUIPage& p : m_CurrentPages) p.gui->SendEventToAll(eventName, paramData); } @@ -381,10 +386,9 @@ // This call makes sure we trigger GC regularly even if the simulation is not running. m_ScriptInterface->GetContext()->MaybeIncrementalGC(1.0f); - // Save an immutable copy so iterators aren't invalidated by tick handlers - PageStackType pageStack = m_PageStack; + UpdatePages(); - for (const SGUIPage& p : pageStack) + for (const SGUIPage& p : m_CurrentPages) p.gui->TickObjects(); } @@ -392,16 +396,15 @@ { PROFILE3_GPU("gui"); - for (const SGUIPage& p : m_PageStack) + for (const SGUIPage& p : m_CurrentPages) p.gui->Draw(canvas); } void CGUIManager::UpdateResolution() { - // Save an immutable copy so iterators aren't invalidated by event handlers - PageStackType pageStack = m_PageStack; + UpdatePages(); - for (const SGUIPage& p : pageStack) + for (const SGUIPage& p : m_CurrentPages) { p.gui->UpdateResolution(); p.gui->SendEventToAll(EVENT_NAME_WINDOW_RESIZED); @@ -443,6 +446,24 @@ // calls SwitchPage) std::shared_ptr CGUIManager::top() const { - ENSURE(m_PageStack.size()); - return m_PageStack.back().gui; + if (m_NextPages.has_value()) + { + ENSURE(!m_NextPages->empty()); + return m_NextPages->back().gui; + } + + ENSURE(!m_CurrentPages.empty()); + return m_CurrentPages.back().gui; +} + +void CGUIManager::MakeNextPages() +{ + if (!m_NextPages.has_value()) + m_NextPages.emplace(m_CurrentPages); +} + +void CGUIManager::UpdatePages() +{ + if (m_NextPages.has_value()) + m_CurrentPages = *std::exchange(m_NextPages, std::nullopt); }