Fixed a crash when trying to switch servers

Ownership of the charInfo global variable wasn't well defined. It was
being locked, unlocked and generally modified from a lot of places, and
somewhere in this mess it ended up crashing when switching servers.

Now the CharHandler instances, for eAthena and manaserv respectively,
own this list of characters. A new class, Net::Character wraps up the
slot index in combination with the player dummy. The list is passed on
to the CharSelectDialog each time it changes.

Both related and unrelated cleanups were made as well.

Reviewed-by: Jared Adams

6 files changed, 198 insertions, 172 deletions

diff --git a/src/gui/charcreatedialog.cpp b/src/gui/charcreatedialog.cpp
index 3d240271..8fb54f76 100644
--- a/src/gui/charcreatedialog.cpp
+++ b/src/gui/charcreatedialog.cpp
@@ -90,13 +90,13 @@ CharCreateDialog::CharCreateDialog(CharSelectDialog *parent, int slot):
     mFemale->addActionListener(this);
 
     mPlayerBox = new PlayerBox(mPlayer);
-
     mPlayerBox->setWidth(74);
 
     mNameField->setActionEventId("create");
     mNameField->addActionListener(this);
 
-    mAttributesLeft = new Label(strprintf(_("Please distribute %d points"), 99));
+    mAttributesLeft = new Label(
+            strprintf(_("Please distribute %d points"), 99));
 
     int w = 200;
     int h = 330;
@@ -168,8 +168,9 @@ void CharCreateDialog::action(const gcn::ActionEvent &event)
             }
 
             Net::getCharHandler()->newCharacter(getName(), mSlot,
-                                mFemale->isSelected(), mHairStyle,
-                                mHairColor, atts);
+                                                mFemale->isSelected(),
+                                                mHairStyle,
+                                                mHairColor, atts);
         }
         else
         {
@@ -246,11 +247,13 @@ void CharCreateDialog::updateSliders()
         mCreateButton->setEnabled(false);
         if (pointsLeft > 0)
         {
-            mAttributesLeft->setCaption(strprintf(_("Please distribute %d points"), pointsLeft));
+            mAttributesLeft->setCaption(
+                    strprintf(_("Please distribute %d points"), pointsLeft));
         }
         else
         {
-            mAttributesLeft->setCaption(strprintf(_("Please remove %d points"), -pointsLeft));
+            mAttributesLeft->setCaption(
+                    strprintf(_("Please remove %d points"), -pointsLeft));
         }
     }
 
@@ -273,7 +276,7 @@ int CharCreateDialog::getDistributedPoints() const
     return points;
 }
 
-void CharCreateDialog::setAttributes(std::vector<std::string> labels,
+void CharCreateDialog::setAttributes(const std::vector<std::string> &labels,
                                      int available, int min, int max)
 {
     mMaxPoints = available;
@@ -303,7 +306,8 @@ void CharCreateDialog::setAttributes(std::vector<std::string> labels,
         add(mAttributeLabel[i]);
 
         mAttributeSlider[i] = new Slider(min, max);
-        mAttributeSlider[i]->setDimension(gcn::Rectangle(75, 140 + i*20, 100, 10));
+        mAttributeSlider[i]->setDimension(gcn::Rectangle(75, 140 + i * 20,
+                                                         100, 10));
         mAttributeSlider[i]->setActionEventId("statslider");
         mAttributeSlider[i]->addActionListener(this);
         add(mAttributeSlider[i]);
@@ -346,11 +350,6 @@ void CharCreateDialog::setFixedGender(bool fixed, Gender gender)
     }
 }
 
-void CharCreateDialog::success()
-{
-    mCharSelectDialog->update(mSlot);
-}
-
 void CharCreateDialog::updateHair()
 {
     mHairStyle %= Being::getNumOfHairstyles();
diff --git a/src/gui/charcreatedialog.h b/src/gui/charcreatedialog.h
index c4838712..a6d4b49b 100644
--- a/src/gui/charcreatedialog.h
+++ b/src/gui/charcreatedialog.h
@@ -19,12 +19,11 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#ifndef CHAR_CREATE_H
-#define CHAR_CREATE_H
+#ifndef CHAR_CREATE_DIALOG_H
+#define CHAR_CREATE_DIALOG_H
 
 #include "player.h"
 #include "guichanfwd.h"
-#include "lockedarray.h"
 
 #include "gui/charselectdialog.h"
 
@@ -63,19 +62,12 @@ class CharCreateDialog : public Window, public gcn::ActionListener
          */
         void unlock();
 
-        void setAttributes(std::vector<std::string> labels, int available,
+        void setAttributes(const std::vector<std::string> &labels,
+                           int available,
                            int min, int max);
 
         void setFixedGender(bool fixed, Gender gender = GENDER_FEMALE);
 
-        /**
-         * Notify the CharSelectDialog the character was created successfully.
-         */
-        void success();
-
-        CharSelectDialog *getSelectDialog() const
-        { return mCharSelectDialog; }
-
     private:
         int getDistributedPoints() const;
 
@@ -127,4 +119,4 @@ class CharCreateDialog : public Window, public gcn::ActionListener
         int mSlot;
 };
 
-#endif // CHAR_CREATE_H
+#endif // CHAR_CREATE_DIALOG_H
diff --git a/src/gui/charselectdialog.cpp b/src/gui/charselectdialog.cpp
index 0489a6cb..dd2368ef 100644
--- a/src/gui/charselectdialog.cpp
+++ b/src/gui/charselectdialog.cpp
@@ -25,6 +25,7 @@
 #include "localplayer.h"
 #include "main.h"
 #include "units.h"
+#include "log.h"
 
 #include "gui/changeemaildialog.h"
 #include "gui/changepassworddialog.h"
@@ -56,6 +57,7 @@
 #include <guichan/font.hpp>
 
 #include <string>
+#include <cassert>
 
 /**
  * Listener for confirming character deletion.
@@ -63,48 +65,45 @@
 class CharDeleteConfirm : public ConfirmDialog
 {
     public:
-        CharDeleteConfirm(CharSelectDialog *m):
+        CharDeleteConfirm(CharSelectDialog *m, int index):
             ConfirmDialog(_("Confirm Character Delete"),
                     _("Are you sure you want to delete this character?"), m),
-            master(m)
+            mMaster(m),
+            mIndex(index)
         {
         }
 
         void action(const gcn::ActionEvent &event)
         {
-            //ConfirmDialog::action(event);
             if (event.getId() == "yes")
-            {
-                master->attemptCharDelete();
-            }
+                mMaster->attemptCharacterDelete(mIndex);
+
             ConfirmDialog::action(event);
         }
 
     private:
-        CharSelectDialog *master;
+        CharSelectDialog *mMaster;
+        int mIndex;
 };
 
-class CharEntry : public Container
+class CharacterDisplay : public Container
 {
     public:
-        CharEntry(CharSelectDialog *m, char slot, LocalPlayer *chr);
+        CharacterDisplay(CharSelectDialog *charSelectDialog);
 
-        char getSlot() const
-        { return mSlot; }
+        void setCharacter(Net::Character *character);
 
-        LocalPlayer *getChar() const
+        Net::Character *getCharacter() const
         { return mCharacter; }
 
-        void setChar(LocalPlayer *chr);
-
         void requestFocus();
 
+        void setActive(bool active);
+
+    private:
         void update();
 
-    protected:
-        friend class CharSelectDialog;
-        char mSlot;
-        LocalPlayer *mCharacter;
+        Net::Character *mCharacter;
 
         PlayerBox *mPlayerBox;
         Label *mName;
@@ -114,10 +113,11 @@ class CharEntry : public Container
         Button *mDelete;
 };
 
-CharSelectDialog::CharSelectDialog(LockedArray<LocalPlayer*> *charInfo,
-                                   LoginData *loginData):
+CharSelectDialog::CharSelectDialog(LoginData *loginData):
     Window(_("Account and Character Management")),
-    mCharInfo(charInfo),
+    mLocked(false),
+    mUnregisterButton(0),
+    mChangeEmailButton(0),
     mLoginData(loginData),
     mCharHandler(Net::getCharHandler())
 {
@@ -128,12 +128,6 @@ CharSelectDialog::CharSelectDialog(LockedArray<LocalPlayer*> *charInfo,
     mChangePasswordButton = new Button(_("Change Password"), "change_password",
                                        this);
 
-    for (int i = 0; i < MAX_CHARACTER_COUNT; i++)
-    {
-        charInfo->select(i);
-        mCharEntries[i] = new CharEntry(this, i, charInfo->getEntry());
-    }
-
     int optionalActions = Net::getLoginHandler()->supportedOptionalActions();
 
     ContainerPlacer place;
@@ -144,79 +138,85 @@ CharSelectDialog::CharSelectDialog(LockedArray<LocalPlayer*> *charInfo,
 
     if (optionalActions & Net::LoginHandler::Unregister)
     {
-        gcn::Button *unregisterButton = new Button(_("Unregister"),
-                                                   "unregister", this);
-        place(3, 1, unregisterButton);
+        mUnregisterButton = new Button(_("Unregister"),
+                                       "unregister", this);
+        place(3, 1, mUnregisterButton);
     }
 
     place(0, 2, mChangePasswordButton);
 
     if (optionalActions & Net::LoginHandler::ChangeEmail)
     {
-        gcn::Button *changeEmailButton = new Button(_("Change Email"),
-                                                    "change_email", this);
-        place(3, 2, changeEmailButton);
+        mChangeEmailButton = new Button(_("Change Email"),
+                                        "change_email", this);
+        place(3, 2, mChangeEmailButton);
     }
 
     place = getPlacer(0, 1);
-    place(0, 0, mCharEntries[0]);
-    place(1, 0, mCharEntries[1]);
-    place(2, 0, mCharEntries[2]);
+
+    for (int i = 0; i < MAX_CHARACTER_COUNT; i++) {
+        mCharacterEntries[i] = new CharacterDisplay(this);
+        place(i, 0, mCharacterEntries[i]);
+    }
 
     reflowLayout();
 
     addKeyListener(this);
 
     center();
-    mCharEntries[0]->requestFocus();
+    mCharacterEntries[0]->requestFocus();
     setVisible(true);
 
     Net::getCharHandler()->setCharSelectDialog(this);
 }
 
+CharSelectDialog::~CharSelectDialog()
+{
+}
+
 void CharSelectDialog::action(const gcn::ActionEvent &event)
 {
+    // Check if a button of a character was pressed
     const gcn::Widget *sourceParent = event.getSource()->getParent();
+    int selected = -1;
+    for (unsigned i = 0; i < MAX_CHARACTER_COUNT; ++i)
+        if (mCharacterEntries[i] == sourceParent)
+            selected = i;
 
-    // Update which character is selected when applicable
-    if (const CharEntry *entry = dynamic_cast<const CharEntry*>(sourceParent))
-        mCharInfo->select(entry->getSlot());
+    const std::string &eventId = event.getId();
 
-    if (event.getId() == "use")
-    {
-        chooseSelected();
-    }
-    else if (event.getId() == "switch")
+    if (selected != -1)
     {
-        mCharInfo->clear();
-        state = STATE_SWITCH_LOGIN;
-    }
-    else if (event.getId() == "new")
-    {
-        if (!(mCharInfo->getEntry()))
+        if (eventId == "use")
+        {
+            attemptCharacterSelect(selected);
+        }
+        else if (eventId == "new" && !mCharacterEntries[selected]->getCharacter())
         {
             // Start new character dialog
             CharCreateDialog *charCreateDialog =
-                new CharCreateDialog(this, mCharInfo->getPos());
+                    new CharCreateDialog(this, selected);
             mCharHandler->setCharCreateDialog(charCreateDialog);
         }
-    }
-    else if (event.getId() == "delete")
-    {
-        if (mCharInfo->getEntry())
+        else if (eventId == "delete"
+                 && mCharacterEntries[selected]->getCharacter())
         {
-            new CharDeleteConfirm(this);
+            new CharDeleteConfirm(this, selected);
         }
     }
-    else if (event.getId() == "change_password")
+    else if (eventId == "switch")
+    {
+        state = STATE_SWITCH_LOGIN;
+    }
+    else if (eventId == "change_password")
     {
         state = STATE_CHANGEPASSWORD;
     }
-    else if (event.getId() == "change_email")
+    else if (eventId == "change_email")
     {
         state = STATE_CHANGEEMAIL;
     }
-    else if (event.getId() == "unregister")
+    else if (eventId == "unregister")
     {
         state = STATE_UNREGISTER;
     }
@@ -228,90 +228,111 @@ void CharSelectDialog::keyPressed(gcn::KeyEvent &keyEvent)
 
     if (key.getValue() == Key::ESCAPE)
     {
-        action(gcn::ActionEvent(mSwitchLoginButton, mSwitchLoginButton->getActionEventId()));
+        action(gcn::ActionEvent(mSwitchLoginButton,
+                                mSwitchLoginButton->getActionEventId()));
     }
 }
 
-void CharSelectDialog::attemptCharDelete()
+/**
+ * Communicate character deletion to the server.
+ */
+void CharSelectDialog::attemptCharacterDelete(int index)
 {
-    mCharHandler->deleteCharacter(mCharInfo->getPos(), mCharInfo->getEntry());
-    mCharInfo->lock();
+    if (mLocked)
+        return;
+
+    mCharHandler->deleteCharacter(mCharacterEntries[index]->getCharacter());
+    lock();
 }
 
-void CharSelectDialog::attemptCharSelect()
+/**
+ * Communicate character selection to the server.
+ */
+void CharSelectDialog::attemptCharacterSelect(int index)
 {
-    mCharHandler->chooseCharacter(mCharInfo->getPos(), mCharInfo->getEntry());
-    mCharInfo->lock();
+    if (mLocked)
+        return;
+
+    mCharHandler->chooseCharacter(mCharacterEntries[index]->getCharacter());
+    lock();
 }
 
-bool CharSelectDialog::selectByName(const std::string &name)
+void CharSelectDialog::setCharacters(const Net::Characters &characters)
 {
-    if (mCharInfo->isLocked())
-        return false;
-
-    unsigned int oldPos = mCharInfo->getPos();
+    // Reset previous characters
+    for (int i = 0; i < MAX_CHARACTER_COUNT; ++i)
+        mCharacterEntries[i]->setCharacter(0);
 
-    mCharInfo->select(0);
-    do
+    Net::Characters::const_iterator i, i_end = characters.end();
+    for (i = characters.begin(); i != i_end; ++i)
     {
-        LocalPlayer *player = mCharInfo->getEntry();
-
-        if (player && player->getName() == name)
-        {
-            mCharEntries[mCharInfo->getPos()]->requestFocus();
-            return true;
+        Net::Character *character = *i;
+        if (character->slot >= MAX_CHARACTER_COUNT) {
+            logger->log("Warning: slot out of range: %d", character->slot);
+            continue;
         }
 
-        mCharInfo->next();
-    } while (mCharInfo->getPos());
+        mCharacterEntries[character->slot]->setCharacter(character);
+    }
+}
 
-    mCharInfo->select(oldPos);
+void CharSelectDialog::lock()
+{
+    assert(!mLocked);
+    setLocked(true);
+}
 
-    return false;
+void CharSelectDialog::unlock()
+{
+    setLocked(false);
 }
 
-bool CharSelectDialog::chooseSelected()
+void CharSelectDialog::setLocked(bool locked)
 {
-    if (!mCharInfo->getSize())
-        return false;
+    mLocked = locked;
 
-    setVisible(false);
-    attemptCharSelect();
+    mSwitchLoginButton->setEnabled(!locked);
+    mChangePasswordButton->setEnabled(!locked);
+    if (mUnregisterButton)
+        mUnregisterButton->setEnabled(!locked);
+    if (mChangeEmailButton)
+        mChangeEmailButton->setEnabled(!locked);
 
-    return true;
+    for (int i = 0; i < MAX_CHARACTER_COUNT; ++i)
+        mCharacterEntries[i]->setActive(!mLocked);
 }
 
-void CharSelectDialog::update(int slot)
+bool CharSelectDialog::selectByName(const std::string &name,
+                                    SelectAction action)
 {
-    if (slot >= 0 && slot < MAX_CHARACTER_COUNT)
-    {
-        mCharInfo->select(slot);
-        mCharEntries[slot]->setChar(mCharInfo->getEntry());
-        mCharEntries[slot]->requestFocus();
-    }
-    else
-    {
-        int slot = mCharInfo->getPos();
-        for (int i = 0; i < MAX_CHARACTER_COUNT; i++)
-        {
-            mCharInfo->select(slot);
-            mCharEntries[slot]->setChar(mCharInfo->getEntry());
+    if (mLocked)
+        return false;
+
+    for (unsigned i = 0; i < MAX_CHARACTER_COUNT; ++i) {
+        if (Net::Character *character = mCharacterEntries[i]->getCharacter()) {
+            if (character->dummy->getName() == name) {
+                mCharacterEntries[i]->requestFocus();
+                if (action == Choose)
+                    attemptCharacterSelect(i);
+                return true;
+            }
         }
-        mCharInfo->select(slot);
     }
+
+    return false;
 }
 
-CharEntry::CharEntry(CharSelectDialog *m, char slot, LocalPlayer *chr):
-        mSlot(slot),
-        mCharacter(chr),
-        mPlayerBox(new PlayerBox(chr))
+
+CharacterDisplay::CharacterDisplay(CharSelectDialog *charSelectDialog):
+    mCharacter(0),
+    mPlayerBox(new PlayerBox)
 {
-    mButton = new Button("wwwwwwwww", "go", m);
+    mButton = new Button("wwwwwwwww", "go", charSelectDialog);
     mName = new Label("wwwwwwwwwwwwwwwwwwwwwwww");
     mLevel = new Label("(888)");
     mMoney = new Label("wwwwwwwww");
 
-    mDelete = new Button(_("Delete"), "delete", m);
+    mDelete = new Button(_("Delete"), "delete", charSelectDialog);
 
     LayoutHelper h(this);
     ContainerPlacer place = h.getPlacer(0, 0);
@@ -337,29 +358,37 @@ CharEntry::CharEntry(CharSelectDialog *m, char slot, LocalPlayer *chr):
             mMoney->getHeight() + mButton->getHeight() + mDelete->getHeight());
 }
 
-void CharEntry::setChar(LocalPlayer *chr)
+void CharacterDisplay::setCharacter(Net::Character *character)
 {
-    mCharacter = chr;
-
-    mPlayerBox->setPlayer(chr);
+    if (mCharacter == character)
+        return;
 
+    mCharacter = character;
+    mPlayerBox->setPlayer(character ? character->dummy : 0);
     update();
 }
 
-void CharEntry::requestFocus()
+void CharacterDisplay::requestFocus()
 {
     mButton->requestFocus();
 }
 
-void CharEntry::update()
+void CharacterDisplay::setActive(bool active)
+{
+    mButton->setEnabled(active);
+    mDelete->setEnabled(active);
+}
+
+void CharacterDisplay::update()
 {
     if (mCharacter)
     {
+        const LocalPlayer *character = mCharacter->dummy;
         mButton->setCaption(_("Choose"));
         mButton->setActionEventId("use");
-        mName->setCaption(strprintf("%s", mCharacter->getName().c_str()));
-        mLevel->setCaption(strprintf("Level %d", mCharacter->getLevel()));
-        mMoney->setCaption(Units::formatCurrency(mCharacter->getMoney()));
+        mName->setCaption(strprintf("%s", character->getName().c_str()));
+        mLevel->setCaption(strprintf("Level %d", character->getLevel()));
+        mMoney->setCaption(Units::formatCurrency(character->getMoney()));
 
         mDelete->setVisible(true);
     }
diff --git a/src/gui/charselectdialog.h b/src/gui/charselectdialog.h
index fa9591bf..9e455462 100644
--- a/src/gui/charselectdialog.h
+++ b/src/gui/charselectdialog.h
@@ -23,16 +23,17 @@
 #define CHAR_SELECT_H
 
 #include "guichanfwd.h"
-#include "lockedarray.h"
 #include "main.h"
 #include "player.h"
 
 #include "gui/widgets/window.h"
 
+#include "net/charhandler.h"
+
 #include <guichan/actionlistener.hpp>
 #include <guichan/keylistener.hpp>
 
-class CharEntry;
+class CharacterDisplay;
 class LocalPlayer;
 class LoginData;
 class PlayerBox;
@@ -51,49 +52,55 @@ class CharSelectDialog : public Window, public gcn::ActionListener,
 {
     public:
         friend class CharDeleteConfirm;
+        friend class Net::CharHandler;
 
         /**
          * Constructor.
          */
-        CharSelectDialog(LockedArray<LocalPlayer*> *charInfo,
-                         LoginData *loginData);
+        CharSelectDialog(LoginData *loginData);
 
-        ~CharSelectDialog() { mCharInfo->clear(); };
+        ~CharSelectDialog();
 
         void action(const gcn::ActionEvent &event);
 
         void keyPressed(gcn::KeyEvent &keyEvent);
 
-        bool selectByName(const std::string &name);
+        enum SelectAction {
+            Focus,
+            Choose
+        };
 
         /**
-         * Send selection to character server
-         * @return false if the selection or the number of existing character
-         * is empty.
-         */ 
-        bool chooseSelected();
-
-        void update(int slot = -1);
+         * Attempt to select the character with the given name. Returns whether
+         * a character with the given name was found.
+         *
+         * \param action determines what to do when a character with the given
+         *               name was found (just focus or also try to choose this
+         *               character).
+         */
+        bool selectByName(const std::string &name,
+                          SelectAction action = Focus);
 
     private:
-        /**
-         * Communicate character deletion to the server.
-         */
-        void attemptCharDelete();
+        void attemptCharacterDelete(int index);
+        void attemptCharacterSelect(int index);
 
-        /**
-         * Communicate character selection to the server.
-         */
-        void attemptCharSelect();
+        void setCharacters(const Net::Characters &characters);
+
+        void lock();
+        void unlock();
+        void setLocked(bool locked);
 
-        LockedArray<LocalPlayer*> *mCharInfo;
+        bool mLocked;
 
         gcn::Label *mAccountNameLabel;
 
         gcn::Button *mSwitchLoginButton;
         gcn::Button *mChangePasswordButton;
+        gcn::Button *mUnregisterButton;
+        gcn::Button *mChangeEmailButton;
 
-        CharEntry *mCharEntries[MAX_CHARACTER_COUNT];
+        CharacterDisplay *mCharacterEntries[MAX_CHARACTER_COUNT];
 
         LoginData *mLoginData;
 
diff --git a/src/gui/unregisterdialog.cpp b/src/gui/unregisterdialog.cpp
index 11cb2c57..95e7e5f5 100644
--- a/src/gui/unregisterdialog.cpp
+++ b/src/gui/unregisterdialog.cpp
@@ -91,8 +91,7 @@ UnRegisterDialog::~UnRegisterDialog()
     delete mWrongDataNoticeListener;
 }
 
-void
-UnRegisterDialog::action(const gcn::ActionEvent &event)
+void UnRegisterDialog::action(const gcn::ActionEvent &event)
 {
     if (event.getId() == "cancel")
     {
diff --git a/src/gui/widgets/playerbox.h b/src/gui/widgets/playerbox.h
index 243668cf..d3329234 100644
--- a/src/gui/widgets/playerbox.h
+++ b/src/gui/widgets/playerbox.h
@@ -39,7 +39,7 @@ class PlayerBox : public gcn::ScrollArea
          * Constructor. Takes the initial player character that this box should
          * display, which defaults to <code>NULL</code>.
          */
-        PlayerBox(const Player *player = NULL);
+        PlayerBox(const Player *player = 0);
 
         /**
          * Destructor.