Made the server handle properly the characters slots.

I turned the vector storing character data into a map,
keeping the character's slot.

Fixed a memleak along the way.

Reviewed-by: Crush.

7 files changed, 212 insertions, 68 deletions

diff --git a/src/account-server/account.cpp b/src/account-server/account.cpp
index 58c1f6ac..7f630590 100644
--- a/src/account-server/account.cpp
+++ b/src/account-server/account.cpp
@@ -27,10 +27,19 @@ Account::~Account()
     for (Characters::iterator i = mCharacters.begin(),
          i_end = mCharacters.end(); i != i_end; ++i)
     {
-        delete *i;
+        delete (*i).second;
     }
 }
 
+bool Account::isSlotEmpty(unsigned int slot)
+{
+    Characters::iterator i = mCharacters.find(slot);
+    if (i != mCharacters.end())
+        return false;
+    else
+        return true;
+}
+
 void Account::setCharacters(const Characters& characters)
 {
     mCharacters = characters;
@@ -38,12 +47,24 @@ void Account::setCharacters(const Characters& characters)
 
 void Account::addCharacter(Character *character)
 {
-    mCharacters.push_back(character);
+    unsigned int slot = (unsigned int) character->getCharacterSlot();
+    assert(isSlotEmpty(slot));
+
+    mCharacters[slot] = character;
 }
 
-void Account::delCharacter(int i)
+void Account::delCharacter(unsigned int slot)
 {
-    mCharacters.erase(mCharacters.begin() + i);
+    for (Characters::iterator iter = mCharacters.begin(),
+             iter_end = mCharacters.end(); iter != iter_end; ++iter)
+    {
+        if ((*iter).second->getCharacterSlot() == slot)
+        {
+            delete (*iter).second;
+            (*iter).second = 0;
+            mCharacters.erase(iter);
+        }
+    }
 }
 
 void Account::setID(int id)
diff --git a/src/account-server/account.h b/src/account-server/account.h
index 2847a03c..5d9c5866 100644
--- a/src/account-server/account.h
+++ b/src/account-server/account.h
@@ -120,6 +120,13 @@ class Account
         { return mLevel; }
 
         /**
+         * Tells whether a slot can be used.
+         *
+         * @param slot slot index of the character.
+         */
+        bool isSlotEmpty(unsigned int slot);
+
+        /**
          * Set the characters.
          *
          * @param characters a list of characters.
@@ -136,9 +143,9 @@ class Account
         /**
          * Removes a character from the account.
          *
-         * @param i index of the character.
+         * @param slot slot index of the character.
          */
-        void delCharacter(int i);
+        void delCharacter(unsigned int slot);
 
         /**
          * Get all the characters.
diff --git a/src/account-server/accounthandler.cpp b/src/account-server/accounthandler.cpp
index 1463b37e..3a61077e 100644
--- a/src/account-server/accounthandler.cpp
+++ b/src/account-server/accounthandler.cpp
@@ -114,8 +114,10 @@ public:
      */
     TokenCollector<AccountHandler, AccountClient *, int> mTokenCollector;
 
-    static void sendCharacterData(AccountClient &client, int slot,
-                              const Character &ch);
+    /**
+     * Send the character data to the client.
+     */
+    static void sendCharacterData(AccountClient &client, const Character &ch);
 
 protected:
     /**
@@ -274,11 +276,11 @@ void AccountHandler::computerDisconnected(NetComputer *comp)
     delete client; // ~AccountClient unsets the account
 }
 
-void AccountHandler::sendCharacterData(AccountClient &client, int slot,
+void AccountHandler::sendCharacterData(AccountClient &client,
                               const Character &ch)
 {
     MessageOut charInfo(APMSG_CHAR_INFO);
-    charInfo.writeInt8(slot);
+    charInfo.writeInt8(ch.getCharacterSlot());
     charInfo.writeString(ch.getName());
     charInfo.writeInt8(ch.getGender());
     charInfo.writeInt8(ch.getHairStyle());
@@ -396,10 +398,9 @@ void AccountHandler::handleLoginMessage(AccountClient &client, MessageIn &msg)
     Characters &chars = acc->getCharacters();
 
     // Send characters list
-    for (unsigned int i = 0; i < chars.size(); i++)
-    {
-        sendCharacterData(client, i, *chars[i]);
-    }
+    for (Characters::const_iterator i = chars.begin(), i_end = chars.end();
+         i != i_end; ++i)
+        sendCharacterData(client, *(*i).second);
 }
 
 void AccountHandler::handleLogoutMessage(AccountClient &client)
@@ -685,12 +686,18 @@ void AccountHandler::handleCharacterCreateMessage(AccountClient &client,
     int hairStyle = msg.readInt8();
     int hairColor = msg.readInt8();
     int gender = msg.readInt8();
+
+    // Avoid creation of character from old clients.
+    int slot = -1;
+    if (msg.getUnreadLength() > 7)
+        slot = msg.readInt8();
+
     int numHairStyles = Configuration::getValue("char_numHairStyles", 17);
     int numHairColors = Configuration::getValue("char_numHairColors", 11);
     int numGenders = Configuration::getValue("char_numGenders", 2);
     unsigned int minNameLength = Configuration::getValue("char_minNameLength", 4);
     unsigned int maxNameLength = Configuration::getValue("char_maxNameLength", 25);
-    unsigned int maxCharacters = Configuration::getValue("account_maxCharacters", 3);
+    int maxCharacters = Configuration::getValue("account_maxCharacters", 3);
 
     MessageOut reply(APMSG_CHAR_CREATE_RESPONSE);
 
@@ -733,9 +740,18 @@ void AccountHandler::handleCharacterCreateMessage(AccountClient &client,
             return;
         }
 
-        // An account shouldn't have more than MAX_OF_CHARACTERS characters.
+        // An account shouldn't have more
+        // than <account_maxCharacters> characters.
         Characters &chars = acc->getCharacters();
-        if (chars.size() >= maxCharacters)
+        if (slot < 1 || slot > maxCharacters
+            || !acc->isSlotEmpty((unsigned int) slot))
+        {
+            reply.writeInt8(CREATE_INVALID_SLOT);
+            client.send(reply);
+            return;
+        }
+
+        if ((int)chars.size() >= maxCharacters)
         {
             reply.writeInt8(CREATE_TOO_MUCH_CHARACTERS);
             client.send(reply);
@@ -783,6 +799,7 @@ void AccountHandler::handleCharacterCreateMessage(AccountClient &client,
                                        (double) (attributes[i]))));
             newCharacter->mAttributes.insert(defAttr.begin(), defAttr.end());
             newCharacter->setAccount(acc);
+            newCharacter->setCharacterSlot(slot);
             newCharacter->setLevel(1);
             newCharacter->setCharacterPoints(0);
             newCharacter->setCorrectionPoints(0);
@@ -812,8 +829,7 @@ void AccountHandler::handleCharacterCreateMessage(AccountClient &client,
             client.send(reply);
 
             // Send new characters infos back to client
-            int slot = chars.size() - 1;
-            sendCharacterData(client, slot, *chars[slot]);
+            sendCharacterData(client, *chars[slot]);
             return;
         }
     }
@@ -834,19 +850,18 @@ void AccountHandler::handleCharacterSelectMessage(AccountClient &client,
         return; // not logged in
     }
 
-    unsigned charNum = msg.readInt8();
+    int slot = msg.readInt8();
     Characters &chars = acc->getCharacters();
 
-    // Character ID = 0 to Number of Characters - 1.
-    if (charNum >= chars.size())
+    if (slot < 1 || slot > (int)chars.size())
     {
-        // invalid char selection
+        // Invalid char selection
         reply.writeInt8(ERRMSG_INVALID_ARGUMENT);
         client.send(reply);
         return;
     }
 
-    Character *selectedChar = chars[charNum];
+    Character *selectedChar = chars[slot];
 
     std::string address;
     int port;
@@ -908,33 +923,33 @@ void AccountHandler::handleCharacterDeleteMessage(AccountClient &client,
         return; // not logged in
     }
 
-    unsigned charNum = msg.readInt8();
+    int slot = msg.readInt8();
     Characters &chars = acc->getCharacters();
 
-    // Character ID = 0 to Number of Characters - 1.
-    if (charNum >= chars.size())
+    if (slot < 1 || acc->isSlotEmpty(slot))
     {
-        // invalid char selection
+        // Invalid char selection
         reply.writeInt8(ERRMSG_INVALID_ARGUMENT);
         client.send(reply);
-        return; // not logged in
+        return;
     }
 
-    LOG_INFO("Character deleted:" << chars[charNum]->getName());
+    std::string characterName = chars[slot]->getName();
+    LOG_INFO("Character deleted:" << characterName);
 
-    acc->delCharacter(charNum);
-    storage->flush(acc);
-
-    reply.writeInt8(ERRMSG_OK);
-    client.send(reply);
-
-    // log transaction
+    // Log transaction
     Transaction trans;
-    trans.mCharacterId = chars[charNum]->getDatabaseID();
+    trans.mCharacterId = chars[slot]->getDatabaseID();
     trans.mAction = TRANS_CHAR_DELETED;
-    trans.mMessage = chars[charNum]->getName() + " deleted by ";
+    trans.mMessage = chars[slot]->getName() + " deleted by ";
     trans.mMessage.append(acc->getName());
     storage->addTransaction(trans);
+
+    acc->delCharacter(slot);
+    storage->flush(acc);
+
+    reply.writeInt8(ERRMSG_OK);
+    client.send(reply);
 }
 
 void AccountHandler::tokenMatched(AccountClient *client, int accountID)
@@ -953,10 +968,9 @@ void AccountHandler::tokenMatched(AccountClient *client, int accountID)
     Characters &chars = acc->getCharacters();
 
     // Send characters list
-    for (unsigned int i = 0; i < chars.size(); i++)
-    {
-        sendCharacterData(*client, i, *chars[i]);
-    }
+    for (Characters::const_iterator i = chars.begin(), i_end = chars.end();
+         i != i_end; ++i)
+        sendCharacterData(*client, *(*i).second);
 }
 
 void AccountHandler::deletePendingClient(AccountClient *client)
diff --git a/src/account-server/character.cpp b/src/account-server/character.cpp
index 48796020..535ee67b 100644
--- a/src/account-server/character.cpp
+++ b/src/account-server/character.cpp
@@ -25,6 +25,7 @@
 Character::Character(const std::string &name, int id):
     mName(name),
     mDatabaseID(id),
+    mCharacterSlot(0),
     mAccountID(-1),
     mAccount(NULL),
     mMapId(0),
diff --git a/src/account-server/character.h b/src/account-server/character.h
index e43b61a1..a0d4b61c 100644
--- a/src/account-server/character.h
+++ b/src/account-server/character.h
@@ -50,6 +50,15 @@ class Character
         int getDatabaseID() const { return mDatabaseID; }
         void setDatabaseID(int id) { mDatabaseID = id; }
 
+        /**
+         * Gets the slot of the character.
+         */
+        unsigned int getCharacterSlot() const
+        { return mCharacterSlot; }
+
+        void setCharacterSlot(unsigned int slot)
+        { mCharacterSlot = slot; }
+
         /** Gets the account the character belongs to. */
         Account *getAccount() const
         { return mAccount; }
@@ -234,6 +243,7 @@ class Character
         Possessions mPossessions; //!< All the possesions of the character.
         std::string mName;        //!< Name of the character.
         int mDatabaseID;          //!< Character database ID.
+        unsigned int mCharacterSlot;  //!< Character slot.
         int mAccountID;           //!< Account ID of the owner.
         Account *mAccount;        //!< Account owning the character.
         Point mPos;               //!< Position the being is at.
@@ -270,6 +280,6 @@ class Character
 /**
  * Type definition for a list of Characters.
  */
-typedef std::vector< Character * > Characters;
+typedef std::map<unsigned int, Character* > Characters;
 
 #endif
diff --git a/src/account-server/storage.cpp b/src/account-server/storage.cpp
index 3862bd1b..448fc289 100644
--- a/src/account-server/storage.cpp
+++ b/src/account-server/storage.cpp
@@ -39,7 +39,7 @@ static const char *DEFAULT_ITEM_FILE = "items.xml";
 
 // Defines the supported db version
 static const char *DB_VERSION_PARAMETER = "database_version";
-static const char *SUPPORTED_DB_VERSION = "13";
+static const char *SUPPORTED_DB_VERSION = "14";
 
 /*
  * MySQL specificities:
@@ -182,6 +182,10 @@ Account *Storage::getAccountBySQL()
         }
         account->setLevel(level);
 
+        // Correct on-the-fly the old 0 slot characters
+        // NOTE: Will be deprecated and removed at some point.
+        fixCharactersSlot(id);
+
         // Load the characters associated with the account.
         std::ostringstream sql;
         sql << "select id from " << CHARACTERS_TBL_NAME << " where user_id = '"
@@ -205,10 +209,14 @@ Account *Storage::getAccountBySQL()
             for (int k = 0; k < size; ++k)
             {
                 if (Character *ptr = getCharacter(characterIDs[k], account))
-                    characters.push_back(ptr);
+                {
+                    characters[ptr->getCharacterSlot()] = ptr;
+                }
                 else
+                {
                     LOG_ERROR("Failed to get character " << characterIDs[k]
                               << " for account " << id << '.');
+                }
             }
 
             account->setCharacters(characters);
@@ -225,6 +233,75 @@ Account *Storage::getAccountBySQL()
     return 0;
 }
 
+void Storage::fixCharactersSlot(int accountId)
+{
+    try
+    {
+        // Obtain all the characters slots from an account.
+        std::ostringstream sql;
+        sql << "SELECT id, slot FROM " << CHARACTERS_TBL_NAME
+            << " where user_id = " << accountId;
+        const dal::RecordSet &charInfo = mDb->execSql(sql.str());
+
+        // If the account is not even in the database then
+        // we can quit now.
+        if (charInfo.isEmpty())
+            return;
+
+        // Specialize the string_to functor to convert
+        // a string to an unsigned int.
+        string_to< unsigned > toUint;
+        std::map<unsigned, unsigned> slotsToUpdate;
+
+        int characterNumber = charInfo.rows();
+        unsigned currentSlot = 1;
+
+        // We parse all the characters slots to see how many are to be
+        // corrected.
+        for (int k = 0; k < characterNumber; ++k)
+        {
+            // If the slot found is equal to 0.
+            if (toUint(charInfo(k, 1)) == 0)
+            {
+                // Find the new slot number to assign.
+                for (int l = 0; l < characterNumber; ++l)
+                {
+                    if (toUint(charInfo(l, 1)) == currentSlot)
+                        currentSlot++;
+                }
+                slotsToUpdate.insert(std::make_pair(toUint(charInfo(k, 0)),
+                                                    currentSlot));
+            }
+        }
+
+        if (slotsToUpdate.size() > 0)
+        {
+            dal::PerformTransaction transaction(mDb);
+
+            // Update the slots in database.
+            for (std::map<unsigned, unsigned>::iterator i =
+                                                          slotsToUpdate.begin(),
+                i_end = slotsToUpdate.end(); i != i_end; ++i)
+            {
+                // Update the character slot.
+                sql.clear();
+                sql.str("");
+                sql << "UPDATE " << CHARACTERS_TBL_NAME
+                    << " SET slot = " << i->second
+                    << " where id = " << i->first;
+                mDb->execSql(sql.str());
+            }
+
+            transaction.commit();
+        }
+    }
+    catch (const dal::DbSqlQueryExecFailure &e)
+    {
+        utils::throwError("(DALStorage::fixCharactersSlots) "
+                          "SQL query failure: ", e);
+    }
+}
+
 Account *Storage::getAccount(const std::string &userName)
 {
     std::ostringstream sql;
@@ -293,6 +370,8 @@ Character *Storage::getCharacterBySQL(Account *owner)
             character->setMapId(Configuration::getValue("char_defaultMap", 1));
         }
 
+        character->setCharacterSlot(toUint(charInfo(0, 12)));
+
         // Fill the account-related fields. Last step, as it may require a new
         // SQL query.
         if (owner)
@@ -598,7 +677,8 @@ bool Storage::updateCharacter(Character *character)
             << "correct_pts = '"<< character->getCorrectionPoints() << "', "
             << "x = '"          << character->getPosition().x << "', "
             << "y = '"          << character->getPosition().y << "', "
-            << "map_id = '"     << character->getMapId() << "' "
+            << "map_id = '"     << character->getMapId() << "', "
+            << "slot = '"     << character->getCharacterSlot() << "' "
             << "where id = '"   << character->getDatabaseID() << "';";
 
         mDb->execSql(sqlUpdateCharacterInfo.str());
@@ -875,9 +955,10 @@ void Storage::flush(Account *account)
         for (Characters::const_iterator it = characters.begin(),
              it_end = characters.end(); it != it_end; ++it)
         {
-            if ((*it)->getDatabaseID() >= 0)
+            Character *character = (*it).second;
+            if (character->getDatabaseID() >= 0)
             {
-                updateCharacter(*it);
+                updateCharacter(character);
             }
             else
             {
@@ -889,42 +970,43 @@ void Storage::flush(Account *account)
                      << "insert into " << CHARACTERS_TBL_NAME
                      << " (user_id, name, gender, hair_style, hair_color,"
                      << " level, char_pts, correct_pts,"
-                     << " x, y, map_id) values ("
+                     << " x, y, map_id, slot) values ("
                      << account->getID() << ", \""
-                     << (*it)->getName() << "\", "
-                     << (*it)->getGender() << ", "
-                     << (int)(*it)->getHairStyle() << ", "
-                     << (int)(*it)->getHairColor() << ", "
-                     << (int)(*it)->getLevel() << ", "
-                     << (int)(*it)->getCharacterPoints() << ", "
-                     << (int)(*it)->getCorrectionPoints() << ", "
-                     << (*it)->getPosition().x << ", "
-                     << (*it)->getPosition().y << ", "
-                     << (*it)->getMapId()
+                     << character->getName() << "\", "
+                     << character->getGender() << ", "
+                     << (int)character->getHairStyle() << ", "
+                     << (int)character->getHairColor() << ", "
+                     << (int)character->getLevel() << ", "
+                     << (int)character->getCharacterPoints() << ", "
+                     << (int)character->getCorrectionPoints() << ", "
+                     << character->getPosition().x << ", "
+                     << character->getPosition().y << ", "
+                     << character->getMapId() << ", "
+                     << character->getCharacterSlot()
                      << ");";
 
                 mDb->execSql(sqlInsertCharactersTable.str());
 
                 // Update the character ID.
-                (*it)->setDatabaseID(mDb->getLastId());
+                character->setDatabaseID(mDb->getLastId());
 
                 // Update all attributes.
                 AttributeMap::const_iterator attr_it, attr_end;
-                for (attr_it =  (*it)->mAttributes.begin(),
-                     attr_end = (*it)->mAttributes.end();
+                for (attr_it =  character->mAttributes.begin(),
+                     attr_end = character->mAttributes.end();
                      attr_it != attr_end; ++attr_it)
                 {
-                    updateAttribute((*it)->getDatabaseID(), attr_it->first,
+                    updateAttribute(character->getDatabaseID(), attr_it->first,
                                     attr_it->second.first,
                                     attr_it->second.second);
                 }
 
                 // Update the characters skill
                 std::map<int, int>::const_iterator skill_it;
-                for (skill_it = (*it)->mExperience.begin();
-                     skill_it != (*it)->mExperience.end(); skill_it++)
+                for (skill_it = character->mExperience.begin();
+                     skill_it != character->mExperience.end(); skill_it++)
                 {
-                    updateExperience((*it)->getDatabaseID(),
+                    updateExperience(character->getDatabaseID(),
                                      skill_it->first, skill_it->second);
                 }
             }
@@ -955,7 +1037,7 @@ void Storage::flush(Account *account)
             for (Characters::const_iterator it = characters.begin(),
                  it_end = characters.end(); it != it_end; ++it) // In memory
             {
-                if (charInMemInfo(i, 0) == (*it)->getName())
+                if (charInMemInfo(i, 0) == (*it).second->getName())
                 {
                     charFound = true;
                     break;
diff --git a/src/account-server/storage.h b/src/account-server/storage.h
index cb7b6813..8e7f90f3 100644
--- a/src/account-server/storage.h
+++ b/src/account-server/storage.h
@@ -441,6 +441,15 @@ class Storage
         Character *getCharacterBySQL(Account *owner);
 
         /**
+         * Fix improper character slots
+         *
+         * @param accountId the account database Id.
+         *
+         * @note Will be deprecated in the future at some point.
+         */
+        void fixCharactersSlot(int accountId);
+
+        /**
          * Synchronizes the base data in the connected SQL database with the xml
          * files like items.xml.
          * This method is called once after initialization of DALStorage.