From 8712b76ce7756570bdbf6a1ea92a534a2713a993 Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Tue, 18 Jul 2023 17:25:21 +0200 Subject: [PATCH 1/5] Simplify PersistentWindowSettings * Remove constructors. * Set defaults in-class. --- libs/s25main/Settings.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/libs/s25main/Settings.h b/libs/s25main/Settings.h index a435a3242..d354c36b1 100644 --- a/libs/s25main/Settings.h +++ b/libs/s25main/Settings.h @@ -24,11 +24,8 @@ bool checkPort(int port); struct PersistentWindowSettings { - DrawPoint lastPos; - bool isOpen; - - PersistentWindowSettings(DrawPoint lastPos, bool isOpen) : lastPos(lastPos), isOpen(isOpen) {} - PersistentWindowSettings() : lastPos(DrawPoint::Invalid()), isOpen(false) {} + DrawPoint lastPos = DrawPoint::Invalid(); + bool isOpen = false; }; /// Configuration class From a3a11df3425219239f74ee04d0335b34df2026e6 Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Tue, 18 Jul 2023 17:35:11 +0200 Subject: [PATCH 2/5] Look up PersistentWindowSettings only once --- libs/s25main/ingameWindows/IngameWindow.cpp | 22 +++++++++------------ libs/s25main/ingameWindows/IngameWindow.h | 2 ++ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/libs/s25main/ingameWindows/IngameWindow.cpp b/libs/s25main/ingameWindows/IngameWindow.cpp index d62000ec5..1bf439fbb 100644 --- a/libs/s25main/ingameWindows/IngameWindow.cpp +++ b/libs/s25main/ingameWindows/IngameWindow.cpp @@ -43,6 +43,9 @@ IngameWindow::IngameWindow(unsigned id, const DrawPoint& pos, const Extent& size contentOffsetEnd.x = LOADER.GetImageN("resource", 39)->getWidth(); // right border contentOffsetEnd.y = LOADER.GetImageN("resource", 40)->getHeight(); // bottom bar + const auto it = SETTINGS.windows.persistentSettings.find(GetGUIID()); + windowSettings_ = (it == SETTINGS.windows.persistentSettings.cend() ? nullptr : &it->second); + // For compatibility we treat the given height as the window height, not the content height // First we have to make sure the size is not to small Window::Resize(elMax(contentOffset + contentOffsetEnd, GetSize())); @@ -54,9 +57,8 @@ IngameWindow::IngameWindow(unsigned id, const DrawPoint& pos, const Extent& size // Load last position or center the window if(pos == posLastOrCenter) { - const auto windowSettings = SETTINGS.windows.persistentSettings.find(GetGUIID()); - if(windowSettings != SETTINGS.windows.persistentSettings.cend() && windowSettings->second.lastPos.isValid()) - SetPos(windowSettings->second.lastPos); + if(windowSettings_ && windowSettings_->lastPos.isValid()) + SetPos(windowSettings_->lastPos); else MoveToCenter(); } else if(pos == posCenter) @@ -111,11 +113,8 @@ void IngameWindow::SetPos(DrawPoint newPos) newPos.y = screenSize.y - GetSize().y; // if possible save the position to settings - const auto windowSettings = SETTINGS.windows.persistentSettings.find(GetGUIID()); - if(windowSettings != SETTINGS.windows.persistentSettings.cend()) - { - windowSettings->second.lastPos = newPos; - } + if(windowSettings_) + windowSettings_->lastPos = newPos; Window::SetPos(newPos); } @@ -361,11 +360,8 @@ bool IngameWindow::IsMessageRelayAllowed() const void IngameWindow::SaveOpenStatus(bool isOpen) const { - auto windowSettings = SETTINGS.windows.persistentSettings.find(GetGUIID()); - if(windowSettings != SETTINGS.windows.persistentSettings.cend()) - { - windowSettings->second.isOpen = isOpen; - } + if(windowSettings_) + windowSettings_->isOpen = isOpen; } Rect IngameWindow::GetButtonBounds(IwButton btn) const diff --git a/libs/s25main/ingameWindows/IngameWindow.h b/libs/s25main/ingameWindows/IngameWindow.h index 482f53894..a0fe40de2 100644 --- a/libs/s25main/ingameWindows/IngameWindow.h +++ b/libs/s25main/ingameWindows/IngameWindow.h @@ -12,6 +12,7 @@ class glArchivItem_Bitmap; class MouseCoords; +struct PersistentWindowSettings; template struct Point; @@ -126,4 +127,5 @@ class IngameWindow : public Window bool isMoving; CloseBehavior closeBehavior_; helpers::EnumArray buttonStates_; + PersistentWindowSettings* windowSettings_; }; From bc017d0fd02a665ed485abbdfdf2137c278f332d Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Tue, 18 Jul 2023 17:52:22 +0200 Subject: [PATCH 3/5] Persist window minimized state --- libs/s25main/Settings.cpp | 2 ++ libs/s25main/Settings.h | 1 + libs/s25main/ingameWindows/IngameWindow.cpp | 17 ++++++++++++++--- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/libs/s25main/Settings.cpp b/libs/s25main/Settings.cpp index 6b9e7e6c0..c7899b16e 100644 --- a/libs/s25main/Settings.cpp +++ b/libs/s25main/Settings.cpp @@ -350,6 +350,7 @@ void Settings::LoadIngame() windows.persistentSettings[window.first].lastPos.x = iniWindow->getIntValue("pos_x"); windows.persistentSettings[window.first].lastPos.y = iniWindow->getIntValue("pos_y"); windows.persistentSettings[window.first].isOpen = iniWindow->getIntValue("is_open"); + windows.persistentSettings[window.first].isMinimized = iniWindow->getValue("is_minimized", false); } } catch(std::runtime_error& e) { @@ -502,6 +503,7 @@ void Settings::SaveIngame() iniWindow->setValue("pos_x", windows.persistentSettings[window.first].lastPos.x); iniWindow->setValue("pos_y", windows.persistentSettings[window.first].lastPos.y); iniWindow->setValue("is_open", windows.persistentSettings[window.first].isOpen); + iniWindow->setValue("is_minimized", windows.persistentSettings[window.first].isMinimized); } bfs::path settingsPathIngame = RTTRCONFIG.ExpandPath(s25::resources::ingameOptions); diff --git a/libs/s25main/Settings.h b/libs/s25main/Settings.h index d354c36b1..e30f49967 100644 --- a/libs/s25main/Settings.h +++ b/libs/s25main/Settings.h @@ -26,6 +26,7 @@ struct PersistentWindowSettings { DrawPoint lastPos = DrawPoint::Invalid(); bool isOpen = false; + bool isMinimized = false; }; /// Configuration class diff --git a/libs/s25main/ingameWindows/IngameWindow.cpp b/libs/s25main/ingameWindows/IngameWindow.cpp index 1bf439fbb..34ab66606 100644 --- a/libs/s25main/ingameWindows/IngameWindow.cpp +++ b/libs/s25main/ingameWindows/IngameWindow.cpp @@ -54,11 +54,18 @@ IngameWindow::IngameWindow(unsigned id, const DrawPoint& pos, const Extent& size // Save to settings that window is open SaveOpenStatus(true); - // Load last position or center the window + // Load lastPos before it is overwritten when restoring minimized state + const auto lastPos = (windowSettings_ ? windowSettings_->lastPos : DrawPoint::Invalid()); + + // Restore minimized state + if(windowSettings_ && windowSettings_->isMinimized) + SetMinimized(); + + // Restore last position or center the window if(pos == posLastOrCenter) { - if(windowSettings_ && windowSettings_->lastPos.isValid()) - SetPos(windowSettings_->lastPos); + if(lastPos.isValid()) + SetPos(lastPos); else MoveToCenter(); } else if(pos == posCenter) @@ -132,6 +139,10 @@ void IngameWindow::SetMinimized(bool minimized) fullSize.y = iwHeight + contentOffset.y + contentOffsetEnd.y; this->isMinimized_ = minimized; Resize(fullSize); + + // if possible save the minimized state to settings + if(windowSettings_) + windowSettings_->isMinimized = isMinimized_; } void IngameWindow::MouseLeftDown(const MouseCoords& mc) From 8a55f9e356bbc3571fb684b505b4dddd1b8b2a98 Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Tue, 18 Jul 2023 19:38:07 +0200 Subject: [PATCH 4/5] Fix bypassed virtual dispatch when restoring state Don't call unqualified (i.e., assumed to be derived) Resize() function from constructor. --- libs/s25main/ingameWindows/IngameWindow.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/libs/s25main/ingameWindows/IngameWindow.cpp b/libs/s25main/ingameWindows/IngameWindow.cpp index 34ab66606..5a48f62d2 100644 --- a/libs/s25main/ingameWindows/IngameWindow.cpp +++ b/libs/s25main/ingameWindows/IngameWindow.cpp @@ -54,18 +54,19 @@ IngameWindow::IngameWindow(unsigned id, const DrawPoint& pos, const Extent& size // Save to settings that window is open SaveOpenStatus(true); - // Load lastPos before it is overwritten when restoring minimized state - const auto lastPos = (windowSettings_ ? windowSettings_->lastPos : DrawPoint::Invalid()); - // Restore minimized state if(windowSettings_ && windowSettings_->isMinimized) - SetMinimized(); + { + isMinimized_ = true; + Extent minimizedSize(GetSize().x, contentOffset.y + contentOffsetEnd.y); + Window::Resize(minimizedSize); + } - // Restore last position or center the window + // Load last position or center the window if(pos == posLastOrCenter) { - if(lastPos.isValid()) - SetPos(lastPos); + if(windowSettings_ && windowSettings_->lastPos.isValid()) + SetPos(windowSettings_->lastPos); else MoveToCenter(); } else if(pos == posCenter) From 0578d3906aaacd77cde31c9a5f58a0bbd1ca8e7e Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Wed, 19 Jul 2023 15:34:55 +0200 Subject: [PATCH 5/5] Add unit test for persistent window minimization --- tests/s25Main/UI/testIngameWindow.cpp | 34 +++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/s25Main/UI/testIngameWindow.cpp b/tests/s25Main/UI/testIngameWindow.cpp index e9ab1392a..f5c69499a 100644 --- a/tests/s25Main/UI/testIngameWindow.cpp +++ b/tests/s25Main/UI/testIngameWindow.cpp @@ -2,7 +2,10 @@ // // SPDX-License-Identifier: GPL-2.0-or-later +#include "DrawPoint.h" +#include "Point.h" #include "PointOutput.h" +#include "Settings.h" #include "WindowManager.h" #include "controls/ctrlButton.h" #include "controls/ctrlComboBox.h" @@ -12,6 +15,7 @@ #include "controls/ctrlProgress.h" #include "desktops/dskGameLobby.h" #include "helpers/format.hpp" +#include "ingameWindows/IngameWindow.h" #include "ingameWindows/iwConnecting.h" #include "ingameWindows/iwDirectIPConnect.h" #include "ingameWindows/iwHelp.h" @@ -19,6 +23,7 @@ #include "ingameWindows/iwMsgbox.h" #include "uiHelper/uiHelpers.hpp" #include "gameTypes/GameTypesOutput.h" +#include "gameData/const_gui_ids.h" #include "rttr/test/random.hpp" #include "s25util/StringConversion.h" #include @@ -194,4 +199,33 @@ BOOST_AUTO_TEST_CASE(ConnectingWindow) } } +BOOST_AUTO_TEST_CASE(SaveAndRestoreMinimized) +{ + constexpr auto id = CGI_MINIMAP; + auto it = SETTINGS.windows.persistentSettings.find(id); + BOOST_REQUIRE(it != SETTINGS.windows.persistentSettings.end()); + + { + it->second.isMinimized = false; + + IngameWindow wnd(id, IngameWindow::posLastOrCenter, Extent(100, 100), "Test Window", nullptr); + BOOST_TEST(!wnd.IsMinimized()); + BOOST_TEST(wnd.GetSize() == Extent(100, 100)); + + wnd.SetMinimized(true); + BOOST_TEST(it->second.isMinimized); + } + + { + it->second.isMinimized = true; + + IngameWindow wnd(id, IngameWindow::posLastOrCenter, Extent(100, 100), "Test Window", nullptr); + BOOST_TEST(wnd.IsMinimized()); + BOOST_TEST(wnd.GetSize() != Extent(100, 100)); + + wnd.SetMinimized(false); + BOOST_TEST(!it->second.isMinimized); + } +} + BOOST_AUTO_TEST_SUITE_END()