Fixed unresponsive UI when switching server/char with Away dialog open

The UI became unresponsive as a result of not actually deleting the
OkDialog. The dialog is now managed by the AwayListener, which now
schedules Away dialog for deletion when necessary, using a DeathListener
to clear the reference to the dialog.

The WindowContainer now uses an std::set instead of std::list to keep
track of widgets scheduled for deletion, to avoid crashing when a widget
is scheduled for deletion multiple times.

5 files changed, 84 insertions, 49 deletions

diff --git a/src/commandhandler.cpp b/src/commandhandler.cpp
index 75cca1ab..e1a0f6f9 100644
--- a/src/commandhandler.cpp
+++ b/src/commandhandler.cpp
@@ -561,5 +561,7 @@ void CommandHandler::handleUnignore(const std::string &args, ChatTab *tab)
 
 void CommandHandler::handleAway(const std::string &args, ChatTab *tab)
 {
-    local_player->setAway(args);
+    if (!args.empty())
+        config.afkMessage = args;
+    local_player->setAwayMode(!local_player->getAwayMode());
 }
diff --git a/src/gui/widgets/windowcontainer.cpp b/src/gui/widgets/windowcontainer.cpp
index 36f0998f..4e350a9e 100644
--- a/src/gui/widgets/windowcontainer.cpp
+++ b/src/gui/widgets/windowcontainer.cpp
@@ -24,16 +24,15 @@
 #include "gui/gui.h"
 #include "gui/widgets/window.h"
 
-#include "utils/dtor.h"
-
 #include <guichan/focushandler.hpp>
 
 WindowContainer *windowContainer = nullptr;
 
 void WindowContainer::logic()
 {
-    delete_all(mDeathList);
-    mDeathList.clear();
+    for (auto widget : mScheduledDeletions)
+        delete widget;
+    mScheduledDeletions.clear();
 
     gcn::Container::logic();
 }
@@ -48,7 +47,7 @@ void WindowContainer::draw(gcn::Graphics *graphics)
 
 void WindowContainer::scheduleDelete(gcn::Widget *widget)
 {
-    mDeathList.push_back(widget);
+    mScheduledDeletions.insert(widget);
 }
 
 void WindowContainer::adjustAfterResize(int oldScreenWidth,
diff --git a/src/gui/widgets/windowcontainer.h b/src/gui/widgets/windowcontainer.h
index 6324b059..ff03a903 100644
--- a/src/gui/widgets/windowcontainer.h
+++ b/src/gui/widgets/windowcontainer.h
@@ -23,6 +23,8 @@
 
 #include "gui/widgets/container.h"
 
+#include <set>
+
 /**
  * A window container. This container adds functionality for more convenient
  * widget (windows in particular) destruction.
@@ -64,9 +66,9 @@ class WindowContainer : public Container
         bool widgetIsVisible(gcn::Widget *widget);
 
         /**
-         * List of widgets that are scheduled to be deleted.
+         * Set of widgets that are scheduled to be deleted.
          */
-        std::list<gcn::Widget *> mDeathList;
+        std::set<gcn::Widget *> mScheduledDeletions;
 };
 
 extern WindowContainer *windowContainer;
diff --git a/src/localplayer.cpp b/src/localplayer.cpp
index d7375c55..5810ca21 100644
--- a/src/localplayer.cpp
+++ b/src/localplayer.cpp
@@ -55,24 +55,18 @@ constexpr unsigned ACTION_TIMEOUT = 182;
 
 LocalPlayer *local_player = nullptr;
 
-LocalPlayer::LocalPlayer(int id, int subtype):
-    Being(id, PLAYER, subtype, nullptr)
+LocalPlayer::LocalPlayer(int id, int subtype)
+    : Being(id, PLAYER, subtype, nullptr)
+    , mAwayListener(std::make_unique<AwayListener>())
 {
-    listen(Event::AttributesChannel);
-
-    mAwayListener = new AwayListener();
-
     setShowName(config.showOwnName);
 
-    listen(Event::ConfigChannel);
     listen(Event::ActorSpriteChannel);
+    listen(Event::AttributesChannel);
+    listen(Event::ConfigChannel);
 }
 
-LocalPlayer::~LocalPlayer()
-{
-    delete mAwayDialog;
-    delete mAwayListener;
-}
+LocalPlayer::~LocalPlayer() = default;
 
 void LocalPlayer::logic()
 {
@@ -1028,27 +1022,28 @@ void LocalPlayer::updateStatusEffect(int id, bool newStatus)
     Being::updateStatusEffect(id, newStatus);
 }
 
-void LocalPlayer::changeAwayMode()
+static std::string afkMessage()
+{
+    return config.afkMessage.empty() ? _("I am away from keyboard")
+                                     : config.afkMessage;
+}
+
+void LocalPlayer::setAwayMode(bool away)
 {
-    mAwayMode = !mAwayMode;
-    mAfkTimer.reset();
+    if (mAwayMode == away)
+        return;
+
+    mAwayMode = away;
 
     if (mAwayMode)
     {
-        auto msg = config.afkMessage.empty() ? _("I am away from keyboard")
-                                             : config.afkMessage;
-        mAwayDialog = new OkDialog(_("Away"), msg);
-        mAwayDialog->addActionListener(mAwayListener);
+        mAwayListener->showDialog(afkMessage());
+        mAfkTimer.reset();
+    }
+    else
+    {
+        mAwayListener->closeDialog();
     }
-
-    mAwayDialog = nullptr;
-}
-
-void LocalPlayer::setAway(const std::string &message)
-{
-    if (!message.empty())
-        config.afkMessage = message;
-    changeAwayMode();
 }
 
 void LocalPlayer::afkRespond(ChatTab *tab, const std::string &nick)
@@ -1056,9 +1051,7 @@ void LocalPlayer::afkRespond(ChatTab *tab, const std::string &nick)
     if (!mAwayMode || !mAfkTimer.passed())
         return;
 
-    auto msg = config.afkMessage.empty() ? _("I am away from keyboard")
-                                         : config.afkMessage;
-    msg = strprintf(_("*AFK*: %s"), msg.c_str());
+    auto msg = strprintf(_("*AFK*: %s"), afkMessage().c_str());
 
     Net::getChatHandler()->privateMessage(nick, msg);
     if (!tab)
@@ -1074,10 +1067,40 @@ void LocalPlayer::afkRespond(ChatTab *tab, const std::string &nick)
     mAfkTimer.set(AWAY_MESSAGE_TIMEOUT);
 }
 
-void AwayListener::action(const gcn::ActionEvent &event)
+AwayListener::~AwayListener()
 {
-    if (event.getId() == "ok" && local_player->getAwayMode())
+    if (mAwayDialog)
     {
-        local_player->changeAwayMode();
+        mAwayDialog->removeActionListener(this);
+        mAwayDialog->removeDeathListener(this);
+        mAwayDialog->scheduleDelete();
     }
 }
+
+void AwayListener::showDialog(const std::string &message)
+{
+    if (mAwayDialog)
+        return;
+
+    mAwayDialog = new OkDialog(_("Away"), message);
+    mAwayDialog->addActionListener(this);
+    mAwayDialog->addDeathListener(this);
+}
+
+void AwayListener::closeDialog()
+{
+    if (mAwayDialog)
+        mAwayDialog->scheduleDelete();
+}
+
+void AwayListener::action(const gcn::ActionEvent &event)
+{
+    if (event.getId() == "ok")
+        local_player->setAwayMode(false);
+}
+
+void AwayListener::death(const gcn::Event &event)
+{
+    if (mAwayDialog == event.getSource())
+        mAwayDialog = nullptr;
+}
diff --git a/src/localplayer.h b/src/localplayer.h
index ba4011c1..a5b08449 100644
--- a/src/localplayer.h
+++ b/src/localplayer.h
@@ -28,6 +28,9 @@
 #include "utils/time.h"
 
 #include <guichan/actionlistener.hpp>
+#include <guichan/deathlistener.hpp>
+
+#include <memory>
 
 class ChatTab;
 class FloorItem;
@@ -35,10 +38,20 @@ class ImageSet;
 class Map;
 class OkDialog;
 
-class AwayListener : public gcn::ActionListener
+class AwayListener : public gcn::ActionListener, public gcn::DeathListener
 {
     public:
+        AwayListener() = default;
+        ~AwayListener() override;
+
+        void showDialog(const std::string &message);
+        void closeDialog();
+
         void action(const gcn::ActionEvent &event) override;
+        void death(const gcn::Event &event) override;
+
+    private:
+        OkDialog *mAwayDialog = nullptr;
 };
 
 /**
@@ -169,13 +182,10 @@ class LocalPlayer final : public Being
         bool isPathSetByMouse() const
         { return mPathSetByMouse; }
 
-        void changeAwayMode();
-
+        void setAwayMode(bool away);
         bool getAwayMode() const
         { return mAwayMode; }
 
-        void setAway(const std::string &message);
-
         void afkRespond(ChatTab *tab, const std::string &nick);
 
         void addMessageToQueue(const std::string &message,
@@ -229,8 +239,7 @@ class LocalPlayer final : public Being
 
         bool mShowIp = false;
 
-        AwayListener *mAwayListener;
-        OkDialog *mAwayDialog = nullptr;
+        std::unique_ptr<AwayListener> mAwayListener;
         Timer mAfkTimer;
         bool mAwayMode = false;
 };