From 92567ced7b7db7849ceaf11886f32875df390831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sat, 24 Dec 2022 11:57:03 +0100 Subject: [PATCH] Fix issues when closing multiple editor popups at the same time When using `UiClosePopupMenus` in a popup handler, the number of popups was first set to `0` and then decremented to `-1` due to the popup itself being closed, which causes the next popup to not open correctly and may also cause a crash due to an out-of-bounds access. The `UiClosePopupMenus` function is adjusted so the number of open popups never goes below `0`. Another check is added to ensure that `UI()->SetActiveItem(nullptr)` is only called when a popup is open, so the currently active item is not reset if no popup is open. Duplicate code in `UiDoPopupMenu` is reduced by calling the `UiClosePopupMenus` function to only close the top-most popup. This also fixes the same issue when a popup is closed by the popup handler and the escape key in the same frame, by only handling the escape key for closing the top-most popup when the popup was not already closed by the handler. Lastly, the explicit escape key handling is removed from the event popup, as this also caused the above issue and is not necessary, as the escape key is already handled implicitly for all popups. --- src/game/editor/editor.h | 2 +- src/game/editor/popups.cpp | 27 +++++++++------------------ 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/game/editor/editor.h b/src/game/editor/editor.h index 47fc71c41..30c9730a2 100644 --- a/src/game/editor/editor.h +++ b/src/game/editor/editor.h @@ -1096,7 +1096,7 @@ public: void UiInvokePopupMenu(void *pID, int Flags, float X, float Y, float W, float H, int (*pfnFunc)(CEditor *pEditor, CUIRect Rect, void *pContext), void *pContext = nullptr); void UiDoPopupMenu(); - void UiClosePopupMenus(); + void UiClosePopupMenus(int Menus = 0); bool UiPopupExists(void *pID); bool UiPopupOpen(); diff --git a/src/game/editor/popups.cpp b/src/game/editor/popups.cpp index a935276e5..f60554f72 100644 --- a/src/game/editor/popups.cpp +++ b/src/game/editor/popups.cpp @@ -86,29 +86,20 @@ void CEditor::UiDoPopupMenu() r.Draw(ColorRGBA(0, 0, 0, 0.75f), Corners, 3.0f); r.Margin(4.0f, &r); - if(s_UiPopups[i].m_pfnFunc(this, r, s_UiPopups[i].m_pContext)) - { - m_LockMouse = false; - UI()->SetActiveItem(nullptr); - g_UiNumPopups--; - m_PopupEventWasActivated = false; - } - - if(Input()->KeyPress(KEY_ESCAPE)) - { - m_LockMouse = false; - UI()->SetActiveItem(nullptr); - g_UiNumPopups--; - m_PopupEventWasActivated = false; - } + if(s_UiPopups[i].m_pfnFunc(this, r, s_UiPopups[i].m_pContext) || Input()->KeyPress(KEY_ESCAPE)) + UiClosePopupMenus(1); } } -void CEditor::UiClosePopupMenus() +void CEditor::UiClosePopupMenus(int Menus) { + if(Menus <= 0) + Menus = g_UiNumPopups; + if(Menus <= 0) + return; m_LockMouse = false; UI()->SetActiveItem(nullptr); - g_UiNumPopups = 0; + g_UiNumPopups = maximum(0, g_UiNumPopups - Menus); m_PopupEventWasActivated = false; } @@ -1252,7 +1243,7 @@ int CEditor::PopupEvent(CEditor *pEditor, CUIRect View, void *pContext) if(pEditor->m_PopupEventType != POPEVENT_LARGELAYER && pEditor->m_PopupEventType != POPEVENT_PREVENTUNUSEDTILES && pEditor->m_PopupEventType != POPEVENT_IMAGEDIV16 && pEditor->m_PopupEventType != POPEVENT_IMAGE_MAX) { static int s_AbortButton = 0; - if(pEditor->DoButton_Editor(&s_AbortButton, "Abort", 0, &Label, 0, nullptr) || pEditor->Input()->KeyPress(KEY_ESCAPE)) + if(pEditor->DoButton_Editor(&s_AbortButton, "Abort", 0, &Label, 0, nullptr)) { pEditor->m_PopupEventWasActivated = false; return 1;