Fix saving of settings when pressing ESC

libs/s25main/WindowManager.cpp

@@ -117,13 +117,12 @@ bool WindowManager::IsDesktopActive()
 /// Sendet eine Tastaturnachricht an die Fenster.
 void WindowManager::RelayKeyboardMessage(KeyboardMsgHandler msg, const KeyEvent& ke)
 {
-    // ist der Desktop gültig?
+    // When there is no desktop, don't check it or any window
     if(!curDesktop)
         return;
-    // ist der Desktop aktiv?
     if(curDesktop->IsActive())
     {
-        // Ja, dann Nachricht an Desktop weiterleiten
+        // Desktop active -> relay msg to desktop
         CALL_MEMBER_FN(*curDesktop, msg)(ke);
         curDesktop->RelayKeyboardMessage(msg, ke);
         return;
@@ -132,18 +131,20 @@ void WindowManager::RelayKeyboardMessage(KeyboardMsgHandler msg, const KeyEvent&
     if(windows.empty())
         return; // No windows -> nothing to do
 
-    // Letztes Fenster schließen? (Escape oder Alt-W)
+    // ESC or ALT+W closes the active window
     if(ke.kt == KeyType::Escape || (ke.c == 'w' && ke.alt))
     {
-        Close(GetTopMostWindow());
-        return;
-    }
-    // Nein, dann Nachricht an letztes Fenster weiterleiten
-    if(!CALL_MEMBER_FN(*windows.back(), msg)(ke))
+        // Find one which isn't yet marked for closing so multiple ESC in between draw calls can close multiple windows
+        const auto itActiveWnd =
+          std::find_if(windows.rbegin(), windows.rend(), [](const auto& wnd) { return !wnd->ShouldBeClosed(); });
+        if(itActiveWnd != windows.rend())
+            (*itActiveWnd)->Close();
+    } else if(!CALL_MEMBER_FN(*windows.back(), msg)(ke)) // send to active window
     {
+        // If not handled yet, relay to active window
         if(!windows.back()->RelayKeyboardMessage(msg, ke))
         {
-            // Falls Nachrichten nicht behandelt wurden, an Desktop wieder senden
+            // If message was not handled send to desktop
             CALL_MEMBER_FN(*curDesktop, msg)(ke);
             curDesktop->RelayKeyboardMessage(msg, ke);
         }
@@ -649,28 +650,24 @@ void WindowManager::Msg_ScreenResize(const Extent& newSize)
     }
 }
 
-const IngameWindow* WindowManager::GetTopMostWindow() const
+IngameWindow* WindowManager::GetTopMostWindow() const
 {
     if(windows.empty())
         return nullptr;
     else
         return windows.back().get();
 }
 
-void WindowManager::Close(const IngameWindow* window)
+void WindowManager::DoClose(IngameWindow* window)
 {
-    // ist das Fenster gültig?
-    if(!window)
-        return;
-
     const auto it =
       std::find_if(windows.begin(), windows.end(), [window](const auto& it) { return it.get() == window; });
-    if(it == windows.end())
-        return; // Window already closed -> Out
+
+    RTTR_Assert(it != windows.end());
 
     SetToolTip(nullptr, "");
 
-    // War es an vorderster Stelle?
+    // Store if this was the active window
     const bool isActiveWnd = window == GetTopMostWindow();
 
     // Remove from list and notify parent, hold onto it till parent is notified
@@ -689,21 +686,26 @@ void WindowManager::Close(const IngameWindow* window)
 /**
  *  Closes _ALL_ windows with the given ID
  *
- *  @param[in] id ID des/der Fenster(s) welche(s) geschlossen werden soll
+ *  @param[in] id ID of the window to be closed
  */
 void WindowManager::Close(unsigned id)
 {
-    auto isId = [id](const auto& curWnd) { return curWnd->GetID() == id; };
-    auto it = std::find_if(windows.begin(), windows.end(), isId);
-    while(it != windows.end())
+    for(auto& wnd : windows)
     {
-        Close(it->get());
-        it = std::find_if(windows.begin(), windows.end(), isId);
+        if(wnd->GetID() == id && !wnd->ShouldBeClosed())
+            wnd->Close();
     }
 }
 
+void WindowManager::CloseNow(IngameWindow* window)
+{
+    if(!window->ShouldBeClosed())
+        window->Close();
+    DoClose(window);
+}
+
 /**
- *  wechselt den Desktop in den neuen Desktop
+ *  Actually process the desktop change
  */
 void WindowManager::DoDesktopSwitch()
 {
@@ -712,14 +714,11 @@ void WindowManager::DoDesktopSwitch()
 
     SetToolTip(nullptr, "");
 
-    // haben wir einen aktuell gültigen Desktop?
+    // If we have a current desktop close all windows
     if(curDesktop)
-    {
-        // Alle (alten) Fenster zumachen
         windows.clear();
-    }
 
-    // Desktop auf Neuen umstellen
+    // Do the switch
     curDesktop = std::move(nextdesktop);
     curDesktop->SetActive(true);
 
@@ -740,7 +739,7 @@ void WindowManager::CloseMarkedIngameWnds()
     auto it = std::find_if(windows.begin(), windows.end(), isWndMarkedForClose);
     while(it != windows.end())
     {
-        Close(it->get());
+        DoClose(it->get());
         it = std::find_if(windows.begin(), windows.end(), isWndMarkedForClose);
     }
 }

libs/s25main/WindowManager.h

@@ -79,10 +79,10 @@ class WindowManager : public Singleton<WindowManager>, public VideoDriverLoaderI
     }
     /// Registers a window to be shown after a desktop switch
     IngameWindow* ShowAfterSwitch(std::unique_ptr<IngameWindow> window);
-    /// schliesst ein IngameWindow und entfernt es aus der Fensterliste.
-    void Close(const IngameWindow* window);
     /// Sucht ein Fenster mit der entsprechenden Fenster-ID und schließt es (falls es so eins gibt)
     void Close(unsigned id);
+    /// Close the window right away and free it.
+    void CloseNow(IngameWindow* window);
     /// merkt einen Desktop zum Wechsel vor.
     Desktop* Switch(std::unique_ptr<Desktop> desktop);
     /// Verarbeitung des Drückens der Linken Maustaste.
@@ -111,7 +111,7 @@ class WindowManager : public Singleton<WindowManager>, public VideoDriverLoaderI
     void Msg_ScreenResize(const Extent& newSize);
 
     /// Return the window currently on the top (probably active)
-    const IngameWindow* GetTopMostWindow() const;
+    IngameWindow* GetTopMostWindow() const;
     IngameWindow* FindWindowAtPos(const Position& pos) const;
     IngameWindow* FindNonModalWindow(unsigned id) const;
 
@@ -133,6 +133,8 @@ class WindowManager : public Singleton<WindowManager>, public VideoDriverLoaderI
     void DoDesktopSwitch();
     /// Actually close all ingame windows marked for closing
     void CloseMarkedIngameWnds();
+    /// Close the window and remove it from the window list
+    void DoClose(IngameWindow* window);
 
     Cursor cursor_;
     std::unique_ptr<Desktop> curDesktop;  /// aktueller Desktop

libs/s25main/desktops/dskGameInterface.cpp

@@ -658,7 +658,8 @@ bool dskGameInterface::Msg_LeftDown(const MouseCoords& mc)
 
         // Bisheriges Actionfenster schließen, falls es eins gab
         // aktuelle Mausposition merken, da diese durch das Schließen verändert werden kann
-        WINDOWMANAGER.Close(actionwindow);
+        if(actionwindow)
+            actionwindow->Close();
         VIDEODRIVER.SetMousePos(mc.GetPos());
 
         ShowActionWindow(action_tabs, cSel, mc.GetPos(), enable_military_buildings);
@@ -1153,7 +1154,7 @@ void dskGameInterface::GI_FlagDestroyed(const MapPoint pt)
     if(actionwindow)
     {
         if(actionwindow->GetSelectedPt() == pt)
-            WINDOWMANAGER.Close(actionwindow);
+            actionwindow->Close();
     }
 }
 

libs/s25main/desktops/dskLobby.cpp

@@ -154,13 +154,8 @@ void dskLobby::Msg_TableRightButton(const unsigned ctrl_id, const boost::optiona
 
             if(boost::lexical_cast<unsigned>(item.c_str()) != 0)
             {
-                if(serverInfoWnd)
-                {
-                    if(serverInfoWnd->GetServerId() == boost::lexical_cast<unsigned>(item))
-                        return; // raus
-
-                    WINDOWMANAGER.Close(serverInfoWnd);
-                }
+                if(serverInfoWnd && serverInfoWnd->GetServerId() == boost::lexical_cast<unsigned>(item))
+                    return; // Already open -> Nothing to do
 
                 serverInfoWnd = &WINDOWMANAGER.ReplaceWindow(
                   std::make_unique<iwLobbyServerInfo>(boost::lexical_cast<unsigned>(item)));

libs/s25main/ingameWindows/iwAction.cpp

@@ -376,7 +376,7 @@ void iwAction::AddAttackControls(ctrlGroup* group, const unsigned attackers_coun
     }
 }
 
-iwAction::~iwAction()
+void iwAction::Close()
 {
     if(mousePosAtOpen_.isValid())
         VIDEODRIVER.SetMousePos(mousePosAtOpen_);

libs/s25main/ingameWindows/iwAction.h

@@ -72,7 +72,8 @@ class iwAction : public IngameWindow
 public:
     iwAction(GameInterface& gi, GameWorldView& gwv, const Tabs& tabs, MapPoint selectedPt, const DrawPoint& mousePos,
              Params params, bool military_buildings);
-    ~iwAction() override;
+
+    void Close() override;
 
     /// Gibt zurück, auf welchen Punkt es sich bezieht
     const MapPoint& GetSelectedPt() const { return selectedPt; }

tests/s25Main/UI/testFont.cpp

@@ -5,14 +5,19 @@
 #include "Loader.h"
 #include "ogl/glFont.h"
 #include "uiHelper/uiHelpers.hpp"
+#include "rttr/test/LogAccessor.hpp"
 #include <boost/test/unit_test.hpp>
 
 BOOST_AUTO_TEST_SUITE(Font)
 
 BOOST_FIXTURE_TEST_CASE(WrapInfoVaryingLen, uiHelper::Fixture)
 {
     LOADER.initResourceFolders();
-    BOOST_TEST(LOADER.LoadFonts());
+    {
+        rttr::test::LogAccessor logAcc;
+        BOOST_TEST(LOADER.LoadFonts());
+        logAcc.clearLog();
+    }
     const auto& font = *SmallFont;
     std::string input = "a\naa\naaa\naaaa\naaaaa\naa";
     auto output = std::vector<std::string>{"a", "aa", "aaa", "aaaa", "aaaa", "a", "aa"};

tests/s25Main/UI/testVideoDriver.cpp

@@ -6,6 +6,7 @@
 #include "helpers/containerUtils.h"
 #include "mockupDrivers/MockupVideoDriver.h"
 #include "uiHelper/uiHelpers.hpp"
+#include "rttr/test/LogAccessor.hpp"
 #include <rttr/test/stubFunction.hpp>
 #include <s25util/warningSuppression.h>
 #include <glad/glad.h>
@@ -62,21 +63,33 @@ RTTR_POP_DIAGNOSTIC
 BOOST_FIXTURE_TEST_CASE(CreateAndDestroyTextures, uiHelper::Fixture)
 {
     // Fresh start
-    VIDEODRIVER.DestroyScreen();
-    VIDEODRIVER.CreateScreen(VideoMode(800, 600), false);
+    {
+        rttr::test::LogAccessor logAcc;
+        VIDEODRIVER.DestroyScreen();
+        VIDEODRIVER.CreateScreen(VideoMode(800, 600), false);
+        logAcc.clearLog();
+    }
 
     RTTR_STUB_FUNCTION(glGenTextures, rttrOglMock2::glGenTextures);
     RTTR_STUB_FUNCTION(glDeleteTextures, rttrOglMock2::glDeleteTextures);
 
     for(unsigned i = 1u; i <= 5u; ++i)
         BOOST_TEST(VIDEODRIVER.GenerateTexture() == i);
     BOOST_TEST_REQUIRE(rttrOglMock2::activeTextures.size() == 5u);
-    VIDEODRIVER.DestroyScreen();
+    {
+        rttr::test::LogAccessor logAcc;
+        VIDEODRIVER.DestroyScreen();
+        logAcc.clearLog();
+    }
     BOOST_TEST_REQUIRE(rttrOglMock2::activeTextures.empty());
     // Next cleanup call is a no-op (validated inside glDeleteTextures)
     VIDEODRIVER.CleanUp();
 
-    VIDEODRIVER.CreateScreen(VideoMode(800, 600), false);
+    {
+        rttr::test::LogAccessor logAcc;
+        VIDEODRIVER.CreateScreen(VideoMode(800, 600), false);
+        logAcc.clearLog();
+    }
     glGenTextures = rttrOglMock2::glGenTextures;
     glDeleteTextures = rttrOglMock2::glDeleteTextures;
     for(unsigned i = 1u; i <= 5u; ++i)