From d3bc5bc8025bc6280f3cf73fd06aa0b7d8a640b2 Mon Sep 17 00:00:00 2001 From: phosit Date: Sat, 21 Sep 2024 20:20:35 +0200 Subject: [PATCH] Enable page-inits to return a Promise Allows to return the page-completion-value instead of passing it to an `Engine` function. Closes: #7000 --- binaries/data/mods/mod/gui/msgbox/msgbox.js | 4 +- source/gui/GUIManager.cpp | 106 ++++++++++++------ source/gui/GUIManager.h | 33 +++++- .../gui/Scripting/JSInterface_GUIManager.cpp | 2 +- source/gui/tests/test_GuiManager.h | 12 +- 5 files changed, 107 insertions(+), 50 deletions(-) diff --git a/binaries/data/mods/mod/gui/msgbox/msgbox.js b/binaries/data/mods/mod/gui/msgbox/msgbox.js index 652dbc0566..3d0f5d6e26 100644 --- a/binaries/data/mods/mod/gui/msgbox/msgbox.js +++ b/binaries/data/mods/mod/gui/msgbox/msgbox.js @@ -2,7 +2,7 @@ * Currently limited to at most 3 buttons per message box. * The convention is to have "cancel" appear first. */ -async function init(data) +function init(data) { // Set title Engine.GetGUIObjectByName("mbTitleBar").caption = data.title; @@ -27,5 +27,5 @@ async function init(data) const closePromise = setButtonCaptionsAndVisibility(mbButton, captions, mbCancelHotkey, "mbButton"); distributeButtonsHorizontally(mbButton, captions); - Engine.PopGuiPage(await closePromise); + return closePromise; } diff --git a/source/gui/GUIManager.cpp b/source/gui/GUIManager.cpp index 8399b4b328..3c00b6adab 100644 --- a/source/gui/GUIManager.cpp +++ b/source/gui/GUIManager.cpp @@ -124,9 +124,10 @@ JS::Value CGUIManager::PushPage(const CStrW& pageName, Script::StructuredClone i if (m_PageStack.empty()) return JS::UndefinedValue(); + CGUI& currentPage = *m_PageStack.back().gui; // Make sure we unfocus anything on the current page. - m_PageStack.back().gui->SendFocusMessage(GUIM_LOST_FOCUS); - return m_PageStack.back().ReplacePromise(m_ScriptInterface); + currentPage.SendFocusMessage(GUIM_LOST_FOCUS); + return m_PageStack.back().ReplacePromise(*currentPage.GetScriptInterface()); }()}; // Push the page prior to loading its contents, because that may push @@ -137,26 +138,15 @@ JS::Value CGUIManager::PushPage(const CStrW& pageName, Script::StructuredClone i return promise; } -void CGUIManager::PopPage(Script::StructuredClone args) +void CGUIManager::PopPage(JS::HandleValue arg) { - if (m_PageStack.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_PageStack.pop_back(); - m_PageStack.back().ResolvePromise(args); - - // We return to a page where some object might have been focused. - m_PageStack.back().gui->SendFocusMessage(GUIM_GOT_FOCUS); + SGUIPage& topmostPage{m_PageStack.back()}; + const ScriptRequest rq{topmostPage.gui->GetScriptInterface()}; + JS::ResolvePromise(rq.cx, *topmostPage.sendingPromise, arg); } CGUIManager::SGUIPage::SGUIPage(const CStrW& pageName, const Script::StructuredClone initData) - : m_Name(pageName), initData(initData), inputs(), gui(), callbackFunction() + : m_Name(pageName), initData(initData) { } @@ -178,6 +168,9 @@ void CGUIManager::SGUIPage::LoadPage(ScriptContext& scriptContext) g_VideoMode.ResetCursor(); inputs.clear(); gui.reset(new CGUI(scriptContext)); + const ScriptRequest rq{gui->GetScriptInterface()}; + sendingPromise = std::make_shared(rq.cx, + JS::NewPromiseObject(rq.cx, nullptr)); gui->AddObjectTypes(); @@ -232,9 +225,6 @@ void CGUIManager::SGUIPage::LoadPage(ScriptContext& scriptContext) gui->LoadedXmlFiles(); - std::shared_ptr scriptInterface = gui->GetScriptInterface(); - ScriptRequest rq(scriptInterface); - JS::RootedValue initDataVal(rq.cx); JS::RootedValue hotloadDataVal(rq.cx); JS::RootedValue global(rq.cx, rq.globalValue()); @@ -245,40 +235,68 @@ void CGUIManager::SGUIPage::LoadPage(ScriptContext& scriptContext) if (hotloadData) Script::ReadStructuredClone(rq, hotloadData, &hotloadDataVal); - if (Script::HasProperty(rq, global, "init") && - !ScriptFunction::CallVoid(rq, global, "init", initDataVal, hotloadDataVal)) + if (!Script::HasProperty(rq, global, "init")) + return; + + JS::RootedValue returnValue{rq.cx}; + if (!ScriptFunction::Call(rq, global, "init", &returnValue, initDataVal, hotloadDataVal)) + { LOGERROR("GUI page '%s': Failed to call init() function", utf8_from_wstring(m_Name)); + return; + } + + if (!returnValue.isObject()) + return; + + JS::RootedObject returnObject{rq.cx, &returnValue.toObject()}; + if (!JS::IsPromiseObject(returnObject)) + return; + + sendingPromise = std::make_shared(rq.cx, returnObject); } JS::Value CGUIManager::SGUIPage::ReplacePromise(ScriptInterface& scriptInterface) { - JSContext* generalContext{scriptInterface.GetGeneralJSContext()}; - callbackFunction = std::make_shared(generalContext, - JS::NewPromiseObject(generalContext, nullptr)); - return JS::ObjectValue(**callbackFunction); + const ScriptRequest rq{scriptInterface}; + receivingPromise = std::make_shared(rq.cx, + JS::NewPromiseObject(rq.cx, nullptr)); + + return JS::ObjectValue(**receivingPromise); } -void CGUIManager::SGUIPage::ResolvePromise(Script::StructuredClone args) +std::optional CGUIManager::SGUIPage::MaybeClose() { - if (!callbackFunction) - return; + if (JS::GetPromiseState(*sendingPromise) == JS::PromiseState::Pending) + return std::nullopt; + + // Make sure we unfocus anything on the current page. + gui->SendFocusMessage(GUIM_LOST_FOCUS); + + const ScriptRequest rq{gui->GetScriptInterface()}; + JS::RootedValue arg{rq.cx, JS::GetPromiseResult(*sendingPromise)}; + return CGUIManager::SGUIPage::CloseResult{Script::WriteStructuredClone(rq, arg), + JS::GetPromiseState(*sendingPromise) == JS::PromiseState::Rejected}; +} + +void CGUIManager::SGUIPage::Refocus(const CloseResult& result) +{ + ENSURE(receivingPromise); std::shared_ptr scriptInterface = gui->GetScriptInterface(); ScriptRequest rq(scriptInterface); JS::RootedObject globalObj(rq.cx, rq.glob); - JS::RootedObject funcVal(rq.cx, *callbackFunction); - - // Delete the callback function, so that it is not called again - callbackFunction.reset(); + JS::RootedObject recv(rq.cx, *std::exchange(receivingPromise, nullptr)); JS::RootedValue argVal(rq.cx); - if (args) - Script::ReadStructuredClone(rq, args, &argVal); + Script::ReadStructuredClone(rq, result.arg, &argVal); // This only resolves the promise, it doesn't call the continuation. - JS::ResolvePromise(rq.cx, funcVal, argVal); + (result.rejected ? JS::RejectPromise : JS::ResolvePromise)(rq.cx, recv, argVal); + + // We return to a page where some object might have been focused. + gui->SendFocusMessage(GUIM_GOT_FOCUS); } Status CGUIManager::ReloadChangedFile(const VfsPath& path) @@ -379,6 +397,20 @@ void CGUIManager::TickObjects() p.gui->TickObjects(); m_ScriptContext.RunJobs(); + + while (!m_PageStack.empty()) + { + const size_t stackSize{m_PageStack.size()}; + const std::optional result{m_PageStack.back().MaybeClose()}; + if (!result.has_value()) + break; + ENSURE(m_PageStack.size() == stackSize); + m_PageStack.pop_back(); + if (!m_PageStack.empty()) + m_PageStack.back().Refocus(result.value()); + + m_ScriptContext.RunJobs(); + } } void CGUIManager::Draw(CCanvas2D& canvas) const diff --git a/source/gui/GUIManager.h b/source/gui/GUIManager.h index 318c6f232c..d0756ff55d 100644 --- a/source/gui/GUIManager.h +++ b/source/gui/GUIManager.h @@ -25,6 +25,7 @@ #include "scriptinterface/StructuredClone.h" #include +#include #include #include @@ -76,7 +77,7 @@ public: * Unload the currently active GUI page, and make the previous page active. * (There must be at least two pages when you call this.) */ - void PopPage(Script::StructuredClone args); + void PopPage(JS::HandleValue arg); /** * Called when a file has been modified, to hotload changes. @@ -151,10 +152,24 @@ private: */ JS::Value ReplacePromise(ScriptInterface& scriptInterface); + struct CloseResult + { + Script::StructuredClone arg; + bool rejected; + }; /** - * Execute the stored callback function with the given arguments. + * If the page should be closed this function closes the page and + * returns the result of the @c init function. + * If this page wasn't closed an empty optional is returned. */ - void ResolvePromise(Script::StructuredClone args); + std::optional MaybeClose(); + + /** + * This function should be called when a child page got closed. The + * result of the closed page should be the argument of this + * function. This function resolves the @c receivingPromise. + */ + void Refocus(const CloseResult& result); std::wstring m_Name; std::unordered_set inputs; // for hotloading @@ -162,10 +177,16 @@ private: std::shared_ptr gui; // the actual GUI page /** - * Function executed by this parent GUI page when the child GUI page it pushed is popped. - * Notice that storing it in the SGUIPage instead of CGUI means that it will survive the hotloading CGUI reset. + * When this promise is settled this page wants to be closed. It + * settles with the page completion value. */ - std::shared_ptr callbackFunction; + std::shared_ptr sendingPromise; + + /** + * The parent page waits on this promise. It also gets the + * completion value through this promise. + */ + std::shared_ptr receivingPromise; }; std::shared_ptr top() const; diff --git a/source/gui/Scripting/JSInterface_GUIManager.cpp b/source/gui/Scripting/JSInterface_GUIManager.cpp index 18d6da7f8a..490d1be21a 100644 --- a/source/gui/Scripting/JSInterface_GUIManager.cpp +++ b/source/gui/Scripting/JSInterface_GUIManager.cpp @@ -51,7 +51,7 @@ void PopGuiPage(const ScriptRequest& rq, JS::HandleValue args) return; } - g_GUI->PopPage(Script::WriteStructuredClone(rq, args)); + g_GUI->PopPage(args); } void SetCursor(const std::wstring& name) diff --git a/source/gui/tests/test_GuiManager.h b/source/gui/tests/test_GuiManager.h index 17e9823ebb..fc8a021bbd 100644 --- a/source/gui/tests/test_GuiManager.h +++ b/source/gui/tests/test_GuiManager.h @@ -203,6 +203,12 @@ public: UnloadHotkeys(); } + static void CloseTopmostPage() + { + g_GUI->PopPage(JS::NullHandleValue); + g_GUI->TickObjects(); + } + void test_PageRegainedFocusEvent() { // Load up a test page. @@ -221,7 +227,7 @@ public: TS_ASSERT_EQUALS(g_GUI->GetPageCount(), 1); g_GUI->PushPage(L"regainFocus/page_emptyPage.xml", data); TS_ASSERT_EQUALS(g_GUI->GetPageCount(), 2); - g_GUI->PopPage(data); + CloseTopmostPage(); TS_ASSERT_EQUALS(g_GUI->GetPageCount(), 1); // This page instantly pushes an empty page with a callback that pops another page again. @@ -229,9 +235,7 @@ public: TS_ASSERT_EQUALS(g_GUI->GetPageCount(), 3); // Pop the empty page - g_GUI->PopPage(data); - TS_ASSERT_EQUALS(g_GUI->GetPageCount(), 2); - scriptInterface->GetContext().RunJobs(); + CloseTopmostPage(); TS_ASSERT_EQUALS(g_GUI->GetPageCount(), 1); } };