diff --git a/distribution/changelog.txt b/distribution/changelog.txt index c29dfb1fcb..3b0050b44b 100644 --- a/distribution/changelog.txt +++ b/distribution/changelog.txt @@ -9,6 +9,7 @@ - Fix: [#9534] Screams no longer cut-off on steep diagonal drops - Fix: [#19823] Parkobj: disallow overriding objects of different object types. - Fix: [#20083] Cannot use terrain surfaces with ID > 32 and terrain edges with ID > 16. +- Fix: [#20089] Potential crash when a window is closed from another window. - Fix: [#20103] [Plugin] Crash when custom plugin actions fail due to immutable state. - Fix: [#20111] All coaster types can access the new diagonal slope pieces. - Fix: [#20155] Fairground organ style 2 shows up as regular music, rather than for the merry-go-round. diff --git a/src/openrct2-ui/interface/Window.cpp b/src/openrct2-ui/interface/Window.cpp index 64b8a77057..5d2f8d753a 100644 --- a/src/openrct2-ui/interface/Window.cpp +++ b/src/openrct2-ui/interface/Window.cpp @@ -38,6 +38,8 @@ static bool WindowFitsBetweenOthers(const ScreenCoordsXY& loc, int32_t width, in { for (auto& w : g_window_list) { + if (w->flags & WF_DEAD) + continue; if (w->flags & WF_STICK_TO_BACK) continue; @@ -131,6 +133,8 @@ static ScreenCoordsXY GetAutoPositionForNewWindow(int32_t width, int32_t height) // Place window next to another for (auto& w : g_window_list) { + if (w->flags & WF_DEAD) + continue; if (w->flags & WF_STICK_TO_BACK) continue; @@ -158,6 +162,8 @@ static ScreenCoordsXY GetAutoPositionForNewWindow(int32_t width, int32_t height) // Overlap for (auto& w : g_window_list) { + if (w->flags & WF_DEAD) + continue; if (w->flags & WF_STICK_TO_BACK) continue; @@ -222,6 +228,8 @@ WindowBase* WindowCreate( // Close least recently used window for (auto& w : g_window_list) { + if (w->flags & WF_DEAD) + continue; if (!(w->flags & (WF_STICK_TO_BACK | WF_STICK_TO_FRONT | WF_NO_AUTO_CLOSE))) { WindowClose(*w.get()); diff --git a/src/openrct2/interface/Window.cpp b/src/openrct2/interface/Window.cpp index 715b56a5e2..b0f6155d14 100644 --- a/src/openrct2/interface/Window.cpp +++ b/src/openrct2/interface/Window.cpp @@ -80,8 +80,7 @@ static constexpr float window_scroll_locations[][2] = { namespace WindowCloseFlags { static constexpr uint32_t None = 0; - static constexpr uint32_t IterateReverse = (1 << 0); - static constexpr uint32_t CloseSingle = (1 << 1); + static constexpr uint32_t CloseSingle = (1 << 0); } // namespace WindowCloseFlags static void WindowDrawCore(DrawPixelInfo& dpi, WindowBase& w, int32_t left, int32_t top, int32_t right, int32_t bottom); @@ -99,6 +98,8 @@ void WindowVisitEach(std::function func) auto windowList = g_window_list; for (auto& w : windowList) { + if (w->flags & WF_DEAD) + continue; func(w.get()); } } @@ -129,6 +130,8 @@ void WindowUpdateAllViewports() */ void WindowUpdateAll() { + WindowFlushDead(); + // Periodic update happens every second so 40 ticks. if (gCurrentRealTimeTicks >= gWindowUpdateTicks) { @@ -165,6 +168,8 @@ static void WindowCloseSurplus(int32_t cap, WindowClass avoid_classification) WindowBase* foundW{}; for (auto& w : g_window_list) { + if (w->flags & WF_DEAD) + continue; if (!(w->flags & (WF_STICK_TO_BACK | WF_STICK_TO_FRONT | WF_NO_AUTO_CLOSE))) { foundW = w.get(); @@ -205,76 +210,40 @@ void WindowSetWindowLimit(int32_t value) */ void WindowClose(WindowBase& w) { - auto itWindow = WindowGetIterator(&w); - if (itWindow == g_window_list.end()) - return; - - // Explicit copy of the shared ptr to keep the memory valid. - std::shared_ptr window = *itWindow; - - WindowEventCloseCall(window.get()); + WindowEventCloseCall(&w); // Remove viewport - window->RemoveViewport(); + w.RemoveViewport(); // Invalidate the window (area) - window->Invalidate(); + w.Invalidate(); - // The window list may have been modified in the close event - itWindow = WindowGetIterator(&w); - if (itWindow != g_window_list.end()) - g_window_list.erase(itWindow); + w.flags |= WF_DEAD; +} + +void WindowFlushDead() +{ + // Remove all windows in g_window_list that have the WF_DEAD flag + g_window_list.remove_if([](auto&& w) -> bool { return w->flags & WF_DEAD; }); } template static void WindowCloseByCondition(TPred pred, uint32_t flags = WindowCloseFlags::None) { - bool listUpdated; - do + for (auto it = g_window_list.rbegin(); it != g_window_list.rend(); ++it) { - listUpdated = false; + auto& wnd = *(*it); + if (wnd.flags & WF_DEAD) + continue; - auto closeSingle = [&](std::shared_ptr window) -> bool { - if (!pred(window.get())) + if (pred(&wnd)) + { + WindowClose(wnd); + if (flags & WindowCloseFlags::CloseSingle) { - return false; + return; } - - // Keep track of current amount, if a new window is created upon closing - // we need to break this current iteration and restart. - size_t previousCount = g_window_list.size(); - - WindowClose(*window.get()); - - if ((flags & WindowCloseFlags::CloseSingle) != 0) - { - // Only close a single one. - return true; - } - - if (previousCount >= g_window_list.size()) - { - // A new window was created during the close event. - return true; - } - - // Keep closing windows. - return false; - }; - - // The closest to something like for_each_if is using find_if in order to avoid duplicate code - // to change the loop direction. - auto windowList = g_window_list; - if ((flags & WindowCloseFlags::IterateReverse) != 0) - listUpdated = std::find_if(windowList.rbegin(), windowList.rend(), closeSingle) != windowList.rend(); - else - listUpdated = std::find_if(windowList.begin(), windowList.end(), closeSingle) != windowList.end(); - - // If requested to close only a single window and a new window was created during close - // we ignore it. - if ((flags & WindowCloseFlags::CloseSingle) != 0) - break; - - } while (listUpdated); + } + } } /** @@ -314,6 +283,8 @@ WindowBase* WindowFindByClass(WindowClass cls) { for (auto& w : g_window_list) { + if (w->flags & WF_DEAD) + continue; if (w->classification == cls) { return w.get(); @@ -333,6 +304,8 @@ WindowBase* WindowFindByNumber(WindowClass cls, rct_windownumber number) { for (auto& w : g_window_list) { + if (w->flags & WF_DEAD) + continue; if (w->classification == cls && w->number == number) { return w.get(); @@ -363,7 +336,7 @@ void WindowCloseTop() } auto pred = [](WindowBase* w) -> bool { return !(w->flags & (WF_STICK_TO_BACK | WF_STICK_TO_FRONT)); }; - WindowCloseByCondition(pred, WindowCloseFlags::CloseSingle | WindowCloseFlags::IterateReverse); + WindowCloseByCondition(pred, WindowCloseFlags::CloseSingle); } /** @@ -380,7 +353,6 @@ void WindowCloseAll() void WindowCloseAllExceptClass(WindowClass cls) { WindowCloseByClass(WindowClass::Dropdown); - WindowCloseByCondition([cls](WindowBase* w) -> bool { return w->classification != cls && !(w->flags & (WF_STICK_TO_BACK | WF_STICK_TO_FRONT)); }); @@ -416,6 +388,9 @@ WindowBase* WindowFindFromPoint(const ScreenCoordsXY& screenCoords) for (auto it = g_window_list.rbegin(); it != g_window_list.rend(); it++) { auto& w = *it; + if (w->flags & WF_DEAD) + continue; + if (screenCoords.x < w->windowPos.x || screenCoords.x >= w->windowPos.x + w->width || screenCoords.y < w->windowPos.y || screenCoords.y >= w->windowPos.y + w->height) continue; @@ -811,6 +786,8 @@ WindowBase* WindowGetMain() { for (auto& w : g_window_list) { + if (w->flags & WF_DEAD) + continue; if (w->classification == WindowClass::MainWindow) { return w.get(); @@ -1198,6 +1175,8 @@ static void WindowDrawCore(DrawPixelInfo& dpi, WindowBase& w, int32_t left, int3 for (auto it = WindowGetIterator(&w); it != g_window_list.end(); it++) { auto* v = (*it).get(); + if (v->flags & WF_DEAD) + continue; if ((&w == v || (v->flags & WF_TRANSPARENT)) && WindowIsVisible(*v)) { WindowDrawSingle(dpi, *v, left, top, right, bottom); @@ -1943,6 +1922,8 @@ bool WindowIsVisible(WindowBase& w) for (auto it = std::next(itPos); it != g_window_list.end(); it++) { auto& w_other = *(*it); + if (w_other.flags & WF_DEAD) + continue; // if covered by a higher window, no rendering needed if (w_other.windowPos.x <= w.windowPos.x && w_other.windowPos.y <= w.windowPos.y @@ -1989,6 +1970,8 @@ Viewport* WindowGetPreviousViewport(Viewport* current) for (auto it = g_window_list.rbegin(); it != g_window_list.rend(); it++) { auto& w = **it; + if (w.flags & WF_DEAD) + continue; if (w.viewport != nullptr) { if (foundPrevious) @@ -2050,6 +2033,9 @@ WindowBase* WindowGetListening() for (auto it = g_window_list.rbegin(); it != g_window_list.rend(); it++) { auto& w = **it; + if (w.flags & WF_DEAD) + continue; + auto viewport = w.viewport; if (viewport != nullptr) { diff --git a/src/openrct2/interface/Window.h b/src/openrct2/interface/Window.h index dc7f910ea8..42000967b5 100644 --- a/src/openrct2/interface/Window.h +++ b/src/openrct2/interface/Window.h @@ -294,6 +294,7 @@ enum WINDOW_FLAGS WF_SCROLLING_TO_LOCATION = (1 << 3), WF_TRANSPARENT = (1 << 4), WF_NO_BACKGROUND = (1 << 5), // Instead of half transparency, completely remove the window background + WF_DEAD = (1U << 6), // Window is closed and will be deleted in the next update. WF_7 = (1 << 7), WF_RESIZABLE = (1 << 8), WF_NO_AUTO_CLOSE = (1 << 9), // Don't auto close this window if too many windows are open @@ -584,6 +585,7 @@ T* WindowFocusOrCreate(WindowClass cls, int32_t width, int32_t height, uint32_t } void WindowClose(WindowBase& window); +void WindowFlushDead(); void WindowCloseByClass(WindowClass cls); void WindowCloseByNumber(WindowClass cls, rct_windownumber number); void WindowCloseByNumber(WindowClass cls, EntityId number);