A bit of cleanup in the way the Client pops up error dialogs

No point in using mCurrentDialog for the OkDialog, since it will clean
up after itself, as was pointed out in many places throughout the code.

Now there are two convenience functions for popping up the OkDialog and
having it change to a specified state afterwards. This also gets rid of
the 4 different ActionListener classes that existed solely for this
purpose.

Reviewed-by: Yohann Ferreira

3 files changed, 52 insertions, 82 deletions

diff --git a/src/client.cpp b/src/client.cpp
index 336abfbd..bac3684f 100644
--- a/src/client.cpp
+++ b/src/client.cpp
@@ -107,7 +107,6 @@ static const int defaultMusicVolume = 60;
 
 // TODO: Get rid fo these globals
 std::string errorMessage;
-ErrorListener errorListener;
 LoginData loginData;
 
 Configuration config;         /**< XML file configuration reader */
@@ -124,11 +123,6 @@ ItemDB *itemDb;
 
 Sound sound;
 
-void ErrorListener::action(const gcn::ActionEvent &)
-{
-    Client::setState(STATE_CHOOSE_SERVER);
-}
-
 volatile int tick_time;       /**< Tick counter */
 volatile int fps = 0;         /**< Frames counted in the last second */
 volatile int frame_count = 0; /**< Counts the frames during one second */
@@ -189,37 +183,6 @@ bool isDoubleClick(int selected)
     return false;
 }
 
-// This anonymous namespace hides whatever is inside from other modules.
-namespace {
-
-class AccountListener : public gcn::ActionListener
-{
-public:
-    void action(const gcn::ActionEvent &)
-    {
-        Client::setState(STATE_CHAR_SELECT);
-    }
-} accountListener;
-
-class LoginListener : public gcn::ActionListener
-{
-public:
-    void action(const gcn::ActionEvent &)
-    {
-        Client::setState(STATE_LOGIN);
-    }
-} loginListener;
-
-class ServerChoiceListener : public gcn::ActionListener
-{
-public:
-    void action(const gcn::ActionEvent &)
-    {
-        Client::setState(STATE_CHOOSE_SERVER);
-    }
-} serverChoiceListener;
-
-} // anonymous namespace
 
 Client *Client::mInstance = 0;
 
@@ -232,6 +195,7 @@ Client::Client(const Options &options):
     mSetupButton(0),
     mState(STATE_CHOOSE_SERVER),
     mOldState(STATE_START),
+    mStateAfterOkDialog(mState),
     mIcon(0),
     mLogicCounterId(0),
     mSecondsCounterId(0),
@@ -822,11 +786,9 @@ int Client::exec()
                         errorMessage =
                             _("This server is missing needed world data. "
                               "Please contact the administrator(s).");
-                        mCurrentDialog = new OkDialog(
-                            _("ItemDB: Error while loading " ITEMS_DB_FILE "!"),
-                            errorMessage);
-                        mCurrentDialog->addActionListener(&serverChoiceListener);
-                        mCurrentDialog = NULL; // OkDialog deletes itself
+                        showOkDialog(_("ItemDB: Error while loading "
+                                       ITEMS_DB_FILE "!"), errorMessage,
+                                     STATE_CHOOSE_SERVER);
                         break;
                     }
                     Being::load(); // Hairstyles
@@ -916,16 +878,12 @@ int Client::exec()
 
                 case STATE_LOGIN_ERROR:
                     logger->log("State: LOGIN ERROR");
-                    mCurrentDialog = new OkDialog(_("Error"), errorMessage);
-                    mCurrentDialog->addActionListener(&loginListener);
-                    mCurrentDialog = NULL; // OkDialog deletes itself
+                    showErrorDialog(errorMessage, STATE_LOGIN);
                     break;
 
                 case STATE_ACCOUNTCHANGE_ERROR:
                     logger->log("State: ACCOUNT CHANGE ERROR");
-                    mCurrentDialog = new OkDialog(_("Error"), errorMessage);
-                    mCurrentDialog->addActionListener(&accountListener);
-                    mCurrentDialog = NULL; // OkDialog deletes itself
+                    showErrorDialog(errorMessage, STATE_CHAR_SELECT);
                     break;
 
                 case STATE_REGISTER_PREP:
@@ -959,10 +917,9 @@ int Client::exec()
 
                 case STATE_CHANGEPASSWORD_SUCCESS:
                     logger->log("State: CHANGE PASSWORD SUCCESS");
-                    mCurrentDialog = new OkDialog(_("Password Change"),
-                            _("Password changed successfully!"));
-                    mCurrentDialog->addActionListener(&accountListener);
-                    mCurrentDialog = NULL; // OkDialog deletes itself
+                    showOkDialog(_("Password Change"),
+                                 _("Password changed successfully!"),
+                                 STATE_CHAR_SELECT);
                     loginData.password = loginData.newPassword;
                     loginData.newPassword = "";
                     break;
@@ -979,10 +936,9 @@ int Client::exec()
 
                 case STATE_CHANGEEMAIL_SUCCESS:
                     logger->log("State: CHANGE EMAIL SUCCESS");
-                    mCurrentDialog = new OkDialog(_("Email Change"),
-                            _("Email changed successfully!"));
-                    mCurrentDialog->addActionListener(&accountListener);
-                    mCurrentDialog = NULL; // OkDialog deletes itself
+                    showOkDialog(_("Email Change"),
+                                 _("Email changed successfully!"),
+                                 STATE_CHAR_SELECT);
                     break;
 
                 case STATE_UNREGISTER:
@@ -1000,12 +956,10 @@ int Client::exec()
                     logger->log("State: UNREGISTER SUCCESS");
                     Net::getLoginHandler()->disconnect();
 
-                    mCurrentDialog = new OkDialog(_("Unregister Successful"),
-                            _("Farewell, come back any time..."));
+                    showOkDialog(_("Unregister Successful"),
+                                 _("Farewell, come back any time..."),
+                                 STATE_CHOOSE_SERVER);
                     loginData.clear();
-                    //The errorlistener sets the state to STATE_CHOOSE_SERVER
-                    mCurrentDialog->addActionListener(&errorListener);
-                    mCurrentDialog = NULL; // OkDialog deletes itself
                     break;
 
                 case STATE_SWITCH_SERVER:
@@ -1058,9 +1012,7 @@ int Client::exec()
                 case STATE_ERROR:
                     logger->log("State: ERROR");
                     logger->log("Error: %s", errorMessage.c_str());
-                    mCurrentDialog = new OkDialog(_("Error"), errorMessage);
-                    mCurrentDialog->addActionListener(&errorListener);
-                    mCurrentDialog = NULL; // OkDialog deletes itself
+                    showErrorDialog(errorMessage, STATE_CHOOSE_SERVER);
                     Net::getGameHandler()->disconnect();
                     break;
 
@@ -1074,6 +1026,20 @@ int Client::exec()
     return 0;
 }
 
+void Client::showOkDialog(const std::string &title,
+                             const std::string &message,
+                             State state)
+{
+    OkDialog *okDialog = new OkDialog(title, message);
+    okDialog->addActionListener(this);
+    mStateAfterOkDialog = state;
+}
+
+void Client::showErrorDialog(const std::string &message, State state)
+{
+    showOkDialog(_("Error"), message, state);
+}
+
 void Client::event(Event::Channel channel, const Event &event)
 {
     if (channel == Event::ConfigChannel &&
@@ -1101,6 +1067,10 @@ void Client::action(const gcn::ActionEvent &event)
         if (window->isVisible())
             window->requestMoveToTop();
     }
+
+    // If this came from the OkDialog used by showOkDialog
+    if (event.getId() == "ok")
+        mState = mStateAfterOkDialog;
 }
 
 void Client::initRootDir()
diff --git a/src/client.h b/src/client.h
index 85fb7d37..4a0bc750 100644
--- a/src/client.h
+++ b/src/client.h
@@ -52,14 +52,7 @@ extern volatile int fps;
 extern volatile int tick_time;
 extern volatile int cur_time;
 
-class ErrorListener : public gcn::ActionListener
-{
-    public:
-        void action(const gcn::ActionEvent &event);
-};
-
 extern std::string errorMessage;
-extern ErrorListener errorListener;
 extern LoginData loginData;
 
 /**
@@ -167,6 +160,20 @@ public:
 
     int exec();
 
+    /**
+     * Pops up an OkDialog with the given \a title and \a message, and
+     * switches to the given \a state when Ok is pressed.
+     */
+    void showOkDialog(const std::string &title,
+                      const std::string &message,
+                      State state);
+
+    /**
+     * Pops up an error dialog with the given \a message, and switches to the
+     * given \a state when Ok is pressed.
+     */
+    void showErrorDialog(const std::string &message, State state);
+
     static void setState(State state)
     { instance()->mState = state; }
 
@@ -218,6 +225,7 @@ private:
 
     State mState;
     State mOldState;
+    State mStateAfterOkDialog;
 
     SDL_Surface *mIcon;
 
diff --git a/src/game.cpp b/src/game.cpp
index d2d2ecfd..8c8fb205 100644
--- a/src/game.cpp
+++ b/src/game.cpp
@@ -96,7 +96,6 @@ Joystick *joystick = NULL;
 OkDialog *weightNotice = NULL;
 OkDialog *deathNotice = NULL;
 QuitDialog *quitDialog = NULL;
-OkDialog *disconnectedDialog = NULL;
 
 ChatWindow *chatWindow;
 StatusWindow *statusWindow;
@@ -226,8 +225,6 @@ Game::Game():
     assert(!mInstance);
     mInstance = this;
 
-    disconnectedDialog = NULL;
-
     // Create the viewport
     viewport = new Viewport;
     viewport->setDimension(gcn::Rectangle(0, 0, graphics->getWidth(),
@@ -386,14 +383,9 @@ void Game::logic()
             return; // Disconnect gets handled by STATE_ERROR
 
         errorMessage = _("The connection to the server was lost.");
-
-        if (!disconnectedDialog)
-        {
-            disconnectedDialog = new OkDialog(_("Network Error"),
-                                              errorMessage);
-            disconnectedDialog->addActionListener(&errorListener);
-            disconnectedDialog->requestMoveToTop();
-        }
+        Client::instance()->showOkDialog(_("Network Error"),
+                                         errorMessage,
+                                         STATE_CHOOSE_SERVER);
     }
 }