From 1992ce920eb5268be9487b3bba6d28353d871111 Mon Sep 17 00:00:00 2001 From: Thorbjørn Lindeijer Date: Mon, 15 May 2023 16:09:09 +0200 Subject: Manage CharacterData using std::unique_ptr Fixes many memory leaks, but also made it clear that we're very often loading all the character data only to immediately throw it away again, even when most of the time all we really need is the database ID or the name. --- src/account-server/account.cpp | 22 +++--- src/account-server/account.h | 9 ++- src/account-server/accounthandler.cpp | 43 +++++----- src/account-server/character.h | 3 +- src/account-server/serverhandler.cpp | 54 +++++++------ src/account-server/serverhandler.h | 2 +- src/account-server/storage.cpp | 143 +++++++++++++++++----------------- src/account-server/storage.h | 18 ++--- src/chat-server/chathandler.cpp | 2 +- src/chat-server/guild.h | 2 +- src/chat-server/guildhandler.cpp | 21 ++--- src/chat-server/partyhandler.cpp | 18 ++--- src/chat-server/post.cpp | 31 ++++---- src/chat-server/post.h | 11 ++- src/common/transaction.h | 2 + src/net/netcomputer.cpp | 4 +- 16 files changed, 187 insertions(+), 198 deletions(-) (limited to 'src') diff --git a/src/account-server/account.cpp b/src/account-server/account.cpp index 11773c47..5e90d091 100644 --- a/src/account-server/account.cpp +++ b/src/account-server/account.cpp @@ -24,10 +24,6 @@ Account::~Account() { - for (auto &character : mCharacters) - { - delete character.second; - } } bool Account::isSlotEmpty(unsigned slot) const @@ -35,17 +31,17 @@ bool Account::isSlotEmpty(unsigned slot) const return mCharacters.find(slot) == mCharacters.end(); } -void Account::setCharacters(const Characters &characters) +void Account::setCharacters(Characters characters) { - mCharacters = characters; + mCharacters = std::move(characters); } -void Account::addCharacter(CharacterData *character) +void Account::addCharacter(std::unique_ptr character) { - auto slot = (unsigned) character->getCharacterSlot(); + auto slot = character->getCharacterSlot(); assert(isSlotEmpty(slot)); - mCharacters[slot] = character; + mCharacters[slot] = std::move(character); } void Account::delCharacter(unsigned slot) @@ -55,13 +51,17 @@ void Account::delCharacter(unsigned slot) { if ((*iter).second->getCharacterSlot() == slot) { - delete (*iter).second; - (*iter).second = 0; mCharacters.erase(iter); } } } +const CharacterData *Account::getCharacter(unsigned slot) const +{ + auto it = mCharacters.find(slot); + return it != mCharacters.end() ? it->second.get() : nullptr; +} + void Account::setID(int id) { assert(mID < 0); diff --git a/src/account-server/account.h b/src/account-server/account.h index 2eb582c2..4f4b807e 100644 --- a/src/account-server/account.h +++ b/src/account-server/account.h @@ -147,14 +147,14 @@ class Account * * @param characters a list of characters. */ - void setCharacters(const Characters& characters); + void setCharacters(Characters characters); /** * Adds a new character. * * @param character the new character. */ - void addCharacter(CharacterData *character); + void addCharacter(std::unique_ptr character); /** * Removes a character from the account. @@ -179,6 +179,11 @@ class Account const Characters &getCharacters() const { return mCharacters; } + /** + * Get the character at the given slot. + */ + const CharacterData *getCharacter(unsigned slot) const; + /** * Get account ID. * diff --git a/src/account-server/accounthandler.cpp b/src/account-server/accounthandler.cpp index 9cb32af1..6c1e10c8 100644 --- a/src/account-server/accounthandler.cpp +++ b/src/account-server/accounthandler.cpp @@ -281,17 +281,17 @@ void AccountHandler::computerDisconnected(NetComputer *comp) delete client; // ~AccountClient unsets the account } -static void sendCharacterData(MessageOut &charInfo, const CharacterData *ch) +static void sendCharacterData(MessageOut &charInfo, const CharacterData &ch) { - charInfo.writeInt8(ch->getCharacterSlot()); - charInfo.writeString(ch->getName()); - charInfo.writeInt8(ch->getGender()); - charInfo.writeInt8(ch->getHairStyle()); - charInfo.writeInt8(ch->getHairColor()); - charInfo.writeInt16(ch->getAttributePoints()); - charInfo.writeInt16(ch->getCorrectionPoints()); - - auto &possessions = ch->getPossessions(); + charInfo.writeInt8(ch.getCharacterSlot()); + charInfo.writeString(ch.getName()); + charInfo.writeInt8(ch.getGender()); + charInfo.writeInt8(ch.getHairStyle()); + charInfo.writeInt8(ch.getHairColor()); + charInfo.writeInt16(ch.getAttributePoints()); + charInfo.writeInt16(ch.getCorrectionPoints()); + + auto &possessions = ch.getPossessions(); auto &equipData = possessions.getEquipment(); auto &inventoryData = possessions.getInventory(); charInfo.writeInt8(equipData.size()); @@ -303,8 +303,8 @@ static void sendCharacterData(MessageOut &charInfo, const CharacterData *ch) charInfo.writeInt16(it->second.itemId); } - charInfo.writeInt8(ch->getAttributes().size()); - for (auto &it : ch->getAttributes()) + charInfo.writeInt8(ch.getAttributes().size()); + for (auto &it : ch.getAttributes()) { // {id, base value in 256ths, modified value in 256ths }* charInfo.writeInt32(it.first); @@ -318,7 +318,7 @@ static void sendFullCharacterData(AccountClient *client, { MessageOut msg(APMSG_CHAR_INFO); for (auto &charIt : chars) - sendCharacterData(msg, charIt.second); + sendCharacterData(msg, *charIt.second); client->send(msg); } @@ -501,7 +501,7 @@ void AccountHandler::handleLoginMessage(AccountClient &client, MessageIn &msg) sendFullCharacterData(&client, chars); } else { for (auto &charIt : chars) - sendCharacterData(reply, charIt.second); + sendCharacterData(reply, *charIt.second); client.send(reply); } @@ -853,7 +853,7 @@ void AccountHandler::handleCharacterCreateMessage(AccountClient &client, } else { - auto newCharacter = new CharacterData(name); + auto newCharacter = std::make_unique(name); // Set the initial attributes provided by the client for (unsigned i = 0; i < mModifiableAttributes.size(); ++i) @@ -873,7 +873,11 @@ void AccountHandler::handleCharacterCreateMessage(AccountClient &client, Point startingPos(Configuration::getValue("char_startX", 1024), Configuration::getValue("char_startY", 1024)); newCharacter->setPosition(startingPos); - acc->addCharacter(newCharacter); + + reply.writeInt8(ERRMSG_OK); + sendCharacterData(reply, *newCharacter); + + acc->addCharacter(std::move(newCharacter)); LOG_INFO("Character " << name << " was created for " << acc->getName() << "'s account."); @@ -882,15 +886,12 @@ void AccountHandler::handleCharacterCreateMessage(AccountClient &client, // log transaction Transaction trans; - trans.mCharacterId = newCharacter->getDatabaseID(); + trans.mCharacterId = acc->getCharacter(slot)->getDatabaseID(); trans.mAction = TRANS_CHAR_CREATE; trans.mMessage = acc->getName() + " created character "; trans.mMessage.append("called " + name); storage->addTransaction(trans); - reply.writeInt8(ERRMSG_OK); - - sendCharacterData(reply, newCharacter); client.send(reply); return; } @@ -1116,7 +1117,7 @@ void AccountHandler::handleStellarLogin(const std::string &token, const std::str addServerInfo(reply); for (auto &charIt : acc->getCharacters()) - sendCharacterData(reply, charIt.second); + sendCharacterData(reply, *charIt.second); client->send(reply); diff --git a/src/account-server/character.h b/src/account-server/character.h index e3c1b49c..d17fc9d1 100644 --- a/src/account-server/character.h +++ b/src/account-server/character.h @@ -21,6 +21,7 @@ #ifndef CHARACTERDATA_H #define CHARACTERDATA_H +#include #include #include #include @@ -277,6 +278,6 @@ class CharacterData /** * Type definition for a list of Characters. */ -using Characters = std::map; +using Characters = std::map>; #endif diff --git a/src/account-server/serverhandler.cpp b/src/account-server/serverhandler.cpp index aa9f78cd..4354997f 100644 --- a/src/account-server/serverhandler.cpp +++ b/src/account-server/serverhandler.cpp @@ -153,13 +153,13 @@ bool GameServerHandler::getGameServerFromMap(int mapId, } static void registerGameClient(GameServer *s, const std::string &token, - const CharacterData &ptr) + const CharacterData &character) { MessageOut msg(AGMSG_PLAYER_ENTER); msg.writeString(token, MAGIC_TOKEN_LENGTH); - msg.writeInt32(ptr.getDatabaseID()); - msg.writeString(ptr.getName()); - ptr.serialize(msg); + msg.writeInt32(character.getDatabaseID()); + msg.writeString(character.getName()); + character.serialize(msg); s->send(msg); } @@ -284,15 +284,14 @@ void ServerHandler::processMessage(NetComputer *comp, MessageIn &msg) { LOG_DEBUG("GAMSG_PLAYER_DATA"); int id = msg.readInt32(); - if (CharacterData *ptr = storage->getCharacter(id, nullptr)) + if (auto character = storage->getCharacter(id, nullptr)) { - ptr->deserialize(msg); - if (!storage->updateCharacter(ptr)) + character->deserialize(msg); + if (!storage->updateCharacter(*character)) { LOG_ERROR("Failed to update character " << id << '.'); } - delete ptr; } else { @@ -312,12 +311,12 @@ void ServerHandler::processMessage(NetComputer *comp, MessageIn &msg) LOG_DEBUG("GAMSG_REDIRECT"); int id = msg.readInt32(); std::string magic_token(utils::getMagicToken()); - if (CharacterData *ptr = storage->getCharacter(id, nullptr)) + if (auto character = storage->getCharacter(id, nullptr)) { - int mapId = ptr->getMapId(); + int mapId = character->getMapId(); if (GameServer *s = getGameServerFromMap(mapId)) { - registerGameClient(s, magic_token, *ptr); + registerGameClient(s, magic_token, *character); MessageOut result(AGMSG_REDIRECT_RESPONSE); result.writeInt32(id); result.writeString(magic_token, MAGIC_TOKEN_LENGTH); @@ -330,7 +329,6 @@ void ServerHandler::processMessage(NetComputer *comp, MessageIn &msg) LOG_ERROR("Server Change: No game server for map " << mapId << '.'); } - delete ptr; } else { @@ -345,11 +343,10 @@ void ServerHandler::processMessage(NetComputer *comp, MessageIn &msg) int id = msg.readInt32(); std::string magic_token = msg.readString(MAGIC_TOKEN_LENGTH); - if (CharacterData *ptr = storage->getCharacter(id, nullptr)) + if (auto character = storage->getCharacter(id, nullptr)) { - int accountID = ptr->getAccountID(); + int accountID = character->getAccountID(); AccountClientHandler::prepareReconnect(magic_token, accountID); - delete ptr; } else { @@ -415,10 +412,9 @@ void ServerHandler::processMessage(NetComputer *comp, MessageIn &msg) int level = msg.readInt16(); // get the character so we can get the account id - CharacterData *c = storage->getCharacter(id, nullptr); - if (c) + if (auto character = storage->getCharacter(id, nullptr)) { - storage->setAccountLevel(c->getAccountID(), level); + storage->setAccountLevel(character->getAccountID(), level); } } break; @@ -461,8 +457,8 @@ void ServerHandler::processMessage(NetComputer *comp, MessageIn &msg) result.writeInt32(characterId); // get the character based on the id - CharacterData *ptr = storage->getCharacter(characterId, nullptr); - if (!ptr) + auto character = storage->getCharacter(characterId, nullptr); + if (!character) { // Invalid character LOG_ERROR("Error finding character id for post"); @@ -470,7 +466,7 @@ void ServerHandler::processMessage(NetComputer *comp, MessageIn &msg) } // get the post for that character and send the post if valid - if (Post *post = postalManager->getPost(ptr)) + if (Post *post = postalManager->getPost(character.get())) { for (unsigned i = 0; i < post->getNumberOfLetters(); ++i) { @@ -487,7 +483,7 @@ void ServerHandler::processMessage(NetComputer *comp, MessageIn &msg) } // clean up - postalManager->clearPost(ptr); + postalManager->clearPost(character.get()); } comp->send(result); @@ -507,8 +503,8 @@ void ServerHandler::processMessage(NetComputer *comp, MessageIn &msg) result.writeInt32(senderId); // get their characters - CharacterData *sender = storage->getCharacter(senderId, nullptr); - CharacterData *receiver = storage->getCharacter(receiverName); + auto sender = storage->getCharacter(senderId, nullptr); + auto receiver = storage->getCharacter(receiverName); if (!sender || !receiver) { // Invalid character @@ -519,7 +515,9 @@ void ServerHandler::processMessage(NetComputer *comp, MessageIn &msg) // save the letter LOG_DEBUG("Creating letter"); - auto letter = new Letter(0, sender, receiver); + auto letter = new Letter(0, + std::move(sender), + std::move(receiver)); letter->addText(msg.readString()); while (msg.getUnreadLength() >= 4) { @@ -624,13 +622,13 @@ void GameServerHandler::dumpStatistics(std::ostream &os) } } -void GameServerHandler::sendPartyChange(CharacterData *ptr, int partyId) +void GameServerHandler::sendPartyChange(const CharacterData &character, int partyId) { - GameServer *s = ::getGameServerFromMap(ptr->getMapId()); + GameServer *s = ::getGameServerFromMap(character.getMapId()); if (s) { MessageOut msg(CGMSG_CHANGED_PARTY); - msg.writeInt32(ptr->getDatabaseID()); + msg.writeInt32(character.getDatabaseID()); msg.writeInt32(partyId); s->send(msg); } diff --git a/src/account-server/serverhandler.h b/src/account-server/serverhandler.h index 57732d1d..c85d790e 100644 --- a/src/account-server/serverhandler.h +++ b/src/account-server/serverhandler.h @@ -65,7 +65,7 @@ namespace GameServerHandler /** * Sends chat party information */ - void sendPartyChange(CharacterData *ptr, int partyId); + void sendPartyChange(const CharacterData &character, int partyId); /** * Takes a GAMSG_PLAYER_SYNC from the gameserver and stores all changes in diff --git a/src/account-server/storage.cpp b/src/account-server/storage.cpp index 96590524..9ff25f69 100644 --- a/src/account-server/storage.cpp +++ b/src/account-server/storage.cpp @@ -222,10 +222,9 @@ std::unique_ptr Storage::getAccountBySQL() for (int k = 0; k < size; ++k) { - if (CharacterData *ptr = - getCharacter(characterIDs[k], account.get())) + if (auto character = getCharacter(characterIDs[k], account.get())) { - characters[ptr->getCharacterSlot()] = ptr; + characters[character->getCharacterSlot()] = std::move(character); } else { @@ -234,7 +233,7 @@ std::unique_ptr Storage::getAccountBySQL() } } - account->setCharacters(characters); + account->setCharacters(std::move(characters)); } return account; @@ -337,9 +336,9 @@ std::unique_ptr Storage::getAccount(int accountID) return nullptr; } -CharacterData *Storage::getCharacterBySQL(Account *owner) +std::unique_ptr Storage::getCharacterBySQL(Account *owner) { - CharacterData *character = nullptr; + std::unique_ptr character = nullptr; string_to< unsigned > toUint; string_to< int > toInt; @@ -356,7 +355,7 @@ CharacterData *Storage::getCharacterBySQL(Account *owner) string_to< unsigned short > toUshort; string_to< double > toDouble; - character = new CharacterData(charInfo(0, 2), toUint(charInfo(0, 0))); + character = std::make_unique(charInfo(0, 2), toUint(charInfo(0, 0))); character->setGender(toUshort(charInfo(0, 3))); character->setHairStyle(toUshort(charInfo(0, 4))); character->setHairColor(toUshort(charInfo(0, 5))); @@ -527,7 +526,7 @@ CharacterData *Storage::getCharacterBySQL(Account *owner) return character; } -CharacterData *Storage::getCharacter(int id, Account *owner) +std::unique_ptr Storage::getCharacter(int id, Account *owner) { std::ostringstream sql; sql << "SELECT * FROM " << CHARACTERS_TBL_NAME << " WHERE id = ?"; @@ -539,7 +538,7 @@ CharacterData *Storage::getCharacter(int id, Account *owner) return nullptr; } -CharacterData *Storage::getCharacter(const std::string &name) +std::unique_ptr Storage::getCharacter(const std::string &name) { std::ostringstream sql; sql << "SELECT * FROM " << CHARACTERS_TBL_NAME << " WHERE name = ?"; @@ -676,7 +675,7 @@ bool Storage::doesCharacterNameExist(const std::string& name) return true; } -bool Storage::updateCharacter(CharacterData *character) +bool Storage::updateCharacter(const CharacterData &character) { dal::PerformTransaction transaction(mDb); @@ -687,16 +686,16 @@ bool Storage::updateCharacter(CharacterData *character) sqlUpdateCharacterInfo << "update " << CHARACTERS_TBL_NAME << " " << "set " - << "gender = '" << character->getGender() << "', " - << "hair_style = '" << character->getHairStyle() << "', " - << "hair_color = '" << character->getHairColor() << "', " - << "char_pts = '" << character->getAttributePoints() << "', " - << "correct_pts = '"<< character->getCorrectionPoints() << "', " - << "x = '" << character->getPosition().x << "', " - << "y = '" << character->getPosition().y << "', " - << "map_id = '" << character->getMapId() << "', " - << "slot = '" << character->getCharacterSlot() << "' " - << "where id = '" << character->getDatabaseID() << "';"; + << "gender = '" << character.getGender() << "', " + << "hair_style = '" << character.getHairStyle() << "', " + << "hair_color = '" << character.getHairColor() << "', " + << "char_pts = '" << character.getAttributePoints() << "', " + << "correct_pts = '"<< character.getCorrectionPoints() << "', " + << "x = '" << character.getPosition().x << "', " + << "y = '" << character.getPosition().y << "', " + << "map_id = '" << character.getMapId() << "', " + << "slot = '" << character.getCharacterSlot() << "' " + << "where id = '" << character.getDatabaseID() << "';"; mDb->execSql(sqlUpdateCharacterInfo.str()); } @@ -709,9 +708,9 @@ bool Storage::updateCharacter(CharacterData *character) // Character attributes. try { - for (AttributeMap::const_iterator it = character->mAttributes.begin(), - it_end = character->mAttributes.end(); it != it_end; ++it) - updateAttribute(character->getDatabaseID(), it->first, + for (AttributeMap::const_iterator it = character.mAttributes.begin(), + it_end = character.mAttributes.end(); it != it_end; ++it) + updateAttribute(character.getDatabaseID(), it->first, it->second.base, it->second.modified); } catch (const dal::DbSqlQueryExecFailure &e) @@ -724,10 +723,10 @@ bool Storage::updateCharacter(CharacterData *character) try { std::map::const_iterator kill_it; - for (kill_it = character->getKillCountBegin(); - kill_it != character->getKillCountEnd(); ++kill_it) + for (kill_it = character.getKillCountBegin(); + kill_it != character.getKillCountEnd(); ++kill_it) { - updateKillCount(character->getDatabaseID(), + updateKillCount(character.getDatabaseID(), kill_it->first, kill_it->second); } } @@ -745,16 +744,16 @@ bool Storage::updateCharacter(CharacterData *character) std::ostringstream insertSql; deleteSql << "DELETE FROM " << CHAR_ABILITIES_TBL_NAME << " WHERE char_id='" - << character->getDatabaseID() << "';"; + << character.getDatabaseID() << "';"; mDb->execSql(deleteSql.str()); // In with the new - for (int abilityId : character->getAbilities()) + for (int abilityId : character.getAbilities()) { insertSql.str(""); insertSql << "INSERT INTO " << CHAR_ABILITIES_TBL_NAME << " (char_id, ability_id)" << " VALUES (" - << " '" << character->getDatabaseID() << "'," + << " '" << character.getDatabaseID() << "'," << " '" << abilityId << "');"; mDb->execSql(insertSql.str()); @@ -774,17 +773,17 @@ bool Storage::updateCharacter(CharacterData *character) std::ostringstream insertSql; deleteSql << "DELETE FROM " << QUESTLOG_TBL_NAME << " WHERE char_id='" - << character->getDatabaseID() << "';"; + << character.getDatabaseID() << "';"; mDb->execSql(deleteSql.str()); // In with the new - for (QuestInfo &quest : character->mQuests) + for (const QuestInfo &quest : character.mQuests) { insertSql.str(""); insertSql << "INSERT INTO " << QUESTLOG_TBL_NAME << " (char_id, quest_id, quest_state, " << "quest_title, quest_description)" << " VALUES (" - << character->getDatabaseID() << "," + << character.getDatabaseID() << "," << " " << quest.id << "," << " " << quest.state << "," << " ?," @@ -812,7 +811,7 @@ bool Storage::updateCharacter(CharacterData *character) std::ostringstream sqlDeleteCharacterInventory; sqlDeleteCharacterInventory << "delete from " << INVENTORIES_TBL_NAME - << " where owner_id = '" << character->getDatabaseID() << "';"; + << " where owner_id = '" << character.getDatabaseID() << "';"; mDb->execSql(sqlDeleteCharacterInventory.str()); } catch (const dal::DbSqlQueryExecFailure& e) @@ -828,10 +827,10 @@ bool Storage::updateCharacter(CharacterData *character) sql << "insert into " << INVENTORIES_TBL_NAME << " (owner_id, slot, class_id, amount, equipped) values (" - << character->getDatabaseID() << ", "; + << character.getDatabaseID() << ", "; std::string base = sql.str(); - const Possessions &poss = character->getPossessions(); + const Possessions &poss = character.getPossessions(); const InventoryData &inventoryData = poss.getInventory(); for (const auto &itemIt : inventoryData) { @@ -860,7 +859,7 @@ bool Storage::updateCharacter(CharacterData *character) std::ostringstream sql; sql << "delete from " << CHAR_STATUS_EFFECTS_TBL_NAME - << " where char_id = '" << character->getDatabaseID() << "';"; + << " where char_id = '" << character.getDatabaseID() << "';"; mDb->execSql(sql.str()); } @@ -872,10 +871,10 @@ bool Storage::updateCharacter(CharacterData *character) try { std::map::const_iterator status_it; - for (status_it = character->getStatusEffectBegin(); - status_it != character->getStatusEffectEnd(); ++status_it) + for (status_it = character.getStatusEffectBegin(); + status_it != character.getStatusEffectEnd(); ++status_it) { - insertStatusEffect(character->getDatabaseID(), + insertStatusEffect(character.getDatabaseID(), status_it->first, status_it->second.time); } } @@ -928,7 +927,7 @@ void Storage::addAccount(Account &account) } } -void Storage::flush(const Account &account) +void Storage::flush(Account &account) { assert(account.getID() >= 0); @@ -963,13 +962,14 @@ void Storage::flush(const Account &account) } // Get the list of characters that belong to this account. - const Characters &characters = account.getCharacters(); + Characters &characters = account.getCharacters(); // Insert or update the characters. - for (auto it : characters) + for (auto& it : characters) { - CharacterData *character = it.second; - if (character->getDatabaseID() >= 0) + CharacterData &character = *it.second; + + if (character.getDatabaseID() >= 0) { updateCharacter(character); } @@ -985,31 +985,31 @@ void Storage::flush(const Account &account) << " char_pts, correct_pts," << " x, y, map_id, slot) values (" << account.getID() << ", ?, " - << character->getGender() << ", " - << character->getHairStyle() << ", " - << character->getHairColor() << ", " - << character->getAttributePoints() << ", " - << character->getCorrectionPoints() << ", " - << character->getPosition().x << ", " - << character->getPosition().y << ", " - << character->getMapId() << ", " - << character->getCharacterSlot() + << character.getGender() << ", " + << character.getHairStyle() << ", " + << character.getHairColor() << ", " + << character.getAttributePoints() << ", " + << character.getCorrectionPoints() << ", " + << character.getPosition().x << ", " + << character.getPosition().y << ", " + << character.getMapId() << ", " + << character.getCharacterSlot() << ");"; mDb->prepareSql(sqlInsertCharactersTable.str()); - mDb->bindValue(1, character->getName()); + mDb->bindValue(1, character.getName()); mDb->processSql(); // Update the character ID. - character->setDatabaseID(mDb->getLastId()); + character.setDatabaseID(mDb->getLastId()); // Update all attributes. AttributeMap::const_iterator attr_it, attr_end; - for (attr_it = character->mAttributes.begin(), - attr_end = character->mAttributes.end(); + for (attr_it = character.mAttributes.begin(), + attr_end = character.mAttributes.end(); attr_it != attr_end; ++attr_it) { - updateAttribute(character->getDatabaseID(), attr_it->first, + updateAttribute(character.getDatabaseID(), attr_it->first, attr_it->second.base, attr_it->second.modified); } @@ -1036,7 +1036,7 @@ void Storage::flush(const Account &account) for (unsigned i = 0; i < charInMemInfo.rows(); ++i) // In database { charFound = false; - for (auto character : characters) // In memory + for (auto& character : characters) // In memory { if (charInMemInfo(i, 0) == character.second->getName()) { @@ -1427,12 +1427,14 @@ std::map Storage::getGuildList() string_to< unsigned > toUint; // Add the members to the guilds. - for (auto &guild : guilds) + for (auto &idAndGuild : guilds) { + const Guild &guild = *idAndGuild.second; + std::ostringstream memberSql; memberSql << "select member_id, rights from " << GUILD_MEMBERS_TBL_NAME - << " where guild_id = '" << guild.second->getId() << "';"; + << " where guild_id = '" << guild.getId() << "';"; const dal::RecordSet& memberInfo = mDb->execSql(memberSql.str()); std::list > members; @@ -1444,10 +1446,10 @@ std::map Storage::getGuildList() for (auto i : members) { - if (CharacterData *character = getCharacter(i.first, nullptr)) + if (auto character = getCharacter(i.first, nullptr)) { - character->addGuild(guild.second->getName()); - guild.second->addMember(character->getDatabaseID(), i.second); + character->addGuild(guild.getName()); + idAndGuild.second->addMember(character->getDatabaseID(), i.second); } } } @@ -1739,11 +1741,6 @@ void Storage::delCharacter(int charId) const } } -void Storage::delCharacter(CharacterData *character) const -{ - delCharacter(character->getDatabaseID()); -} - void Storage::checkBannedAccounts() { try @@ -1877,10 +1874,12 @@ Post *Storage::getStoredPost(int playerId) for (unsigned i = 0; i < post.rows(); i++ ) { // Load sender and receiver - CharacterData *sender = getCharacter(toUint(post(i, 1)), nullptr); - CharacterData *receiver = getCharacter(toUint(post(i, 2)), nullptr); + auto sender = getCharacter(toUint(post(i, 1)), nullptr); + auto receiver = getCharacter(toUint(post(i, 2)), nullptr); - auto letter = new Letter(toUint( post(0,3) ), sender, receiver); + auto letter = new Letter(toUint( post(0,3) ), + std::move(sender), + std::move(receiver)); letter->setId( toUint(post(0, 0)) ); letter->setExpiry( toUint(post(0, 4)) ); diff --git a/src/account-server/storage.h b/src/account-server/storage.h index 5d5d7f1a..53bdca13 100644 --- a/src/account-server/storage.h +++ b/src/account-server/storage.h @@ -87,7 +87,7 @@ class Storage * * @return the character associated to the Id. */ - CharacterData *getCharacter(int id, Account *owner); + std::unique_ptr getCharacter(int id, Account *owner); /** * Gets a character by character name. @@ -96,7 +96,7 @@ class Storage * * @return the character associated to the name */ - CharacterData *getCharacter(const std::string &name); + std::unique_ptr getCharacter(const std::string &name); /** * Gets the id of a character by its name. @@ -183,14 +183,6 @@ class Storage */ void delCharacter(int charId) const; - /** - * Delete a character in the database. The object itself is not touched - * by this function! - * - * @param character character object. - */ - void delCharacter(CharacterData *character) const; - /** * Removes expired bans from accounts */ @@ -233,7 +225,7 @@ class Storage * * @return true on success */ - bool updateCharacter(CharacterData *ptr); + bool updateCharacter(const CharacterData &ptr); /** * Add a new guild. @@ -318,7 +310,7 @@ class Storage * * @param Account object to update. */ - void flush(const Account &); + void flush(Account &); /** * Gets the value of a quest variable. @@ -458,7 +450,7 @@ class Storage * * @return the character found by the query. */ - CharacterData *getCharacterBySQL(Account *owner); + std::unique_ptr getCharacterBySQL(Account *owner); /** * Fix improper character slots diff --git a/src/chat-server/chathandler.cpp b/src/chat-server/chathandler.cpp index a33d08a9..cfdb5da6 100644 --- a/src/chat-server/chathandler.cpp +++ b/src/chat-server/chathandler.cpp @@ -83,7 +83,7 @@ void ChatHandler::tokenMatched(ChatClient *client, Pending *p) client->characterName = p->character; client->accountLevel = p->level; - CharacterData *c = storage->getCharacter(p->character); + auto c = storage->getCharacter(p->character); if (!c) { diff --git a/src/chat-server/guild.h b/src/chat-server/guild.h index 9cff757e..7f46f88c 100644 --- a/src/chat-server/guild.h +++ b/src/chat-server/guild.h @@ -102,7 +102,7 @@ class Guild /** * Returns a list of the members in this guild. */ - std::list getMembers() const + const std::list &getMembers() const { return mMembers; } /** diff --git a/src/chat-server/guildhandler.cpp b/src/chat-server/guildhandler.cpp index a47f8073..e69079e5 100644 --- a/src/chat-server/guildhandler.cpp +++ b/src/chat-server/guildhandler.cpp @@ -116,14 +116,11 @@ void ChatHandler::sendGuildListUpdate(Guild *guild, msg.writeInt16(guild->getId()); msg.writeString(characterName); msg.writeInt8(eventId); - std::map::const_iterator chr; - std::list members = guild->getMembers(); - for (std::list::const_iterator itr = members.begin(); - itr != members.end(); ++itr) + for (auto guildMember : guild->getMembers()) { - CharacterData *c = storage->getCharacter((*itr)->mId, nullptr); - chr = mPlayerMap.find(c->getName()); + auto c = storage->getCharacter(guildMember->mId, nullptr); + auto chr = mPlayerMap.find(c->getName()); if (chr != mPlayerMap.end()) { chr->second->send(msg); @@ -283,13 +280,11 @@ void ChatHandler::handleGuildGetMembers(ChatClient &client, MessageIn &msg) { reply.writeInt8(ERRMSG_OK); reply.writeInt16(guildId); - std::list memberList = guild->getMembers(); - std::list::const_iterator itr_end = memberList.end(); - for (auto itr = memberList.begin(); - itr != itr_end; ++itr) + + for (auto guildMember : guild->getMembers()) { - CharacterData *c = storage->getCharacter((*itr)->mId, nullptr); - std::string memberName = c->getName(); + auto c = storage->getCharacter(guildMember->mId, nullptr); + const std::string &memberName = c->getName(); reply.writeString(memberName); reply.writeInt8(mPlayerMap.find(memberName) != mPlayerMap.end()); } @@ -313,7 +308,7 @@ void ChatHandler::handleGuildMemberLevelChange(ChatClient &client, std::string user = msg.readString(); short level = msg.readInt8(); Guild *guild = guildManager->findById(guildId); - CharacterData *c = storage->getCharacter(user); + auto c = storage->getCharacter(user); if (guild && c) { diff --git a/src/chat-server/partyhandler.cpp b/src/chat-server/partyhandler.cpp index b7e1c8d9..fa615824 100644 --- a/src/chat-server/partyhandler.cpp +++ b/src/chat-server/partyhandler.cpp @@ -22,8 +22,9 @@ #include "chatclient.h" #include "party.h" -#include "account-server/storage.h" +#include "account-server/character.h" #include "account-server/serverhandler.h" +#include "account-server/storage.h" #include "common/manaserv_protocol.h" @@ -32,10 +33,10 @@ using namespace ManaServ; -void updateInfo(ChatClient *client, int partyId) +static void updateInfo(ChatClient *client, int partyId) { - CharacterData *character = storage->getCharacter(client->characterName); - GameServerHandler::sendPartyChange(character, partyId); + if (auto character = storage->getCharacter(client->characterName)) + GameServerHandler::sendPartyChange(*character, partyId); } void ChatHandler::removeExpiredPartyInvites() @@ -201,16 +202,13 @@ void ChatHandler::removeUserFromParty(ChatClient &client) void ChatHandler::informPartyMemberQuit(ChatClient &client) { - std::map::iterator itr; - std::map::const_iterator itr_end = mPlayerMap.end(); - - for (itr = mPlayerMap.begin(); itr != itr_end; ++itr) + for (auto& nameAndClient : mPlayerMap) { - if (itr->second->party == client.party) + if (nameAndClient.second->party == client.party) { MessageOut out(CPMSG_PARTY_MEMBER_LEFT); out.writeInt32(client.characterId); - itr->second->send(out); + nameAndClient.second->send(out); } } } diff --git a/src/chat-server/post.cpp b/src/chat-server/post.cpp index cfd9506b..78fab0ed 100644 --- a/src/chat-server/post.cpp +++ b/src/chat-server/post.cpp @@ -20,21 +20,22 @@ #include "post.h" -#include "../account-server/character.h" -#include "../common/configuration.h" +#include "account-server/character.h" +#include "common/configuration.h" -Letter::Letter(unsigned type, CharacterData *sender, CharacterData *receiver) - : mId(0), mType(type), mExpiry(0), mSender(sender), mReceiver(receiver) +Letter::Letter(unsigned type, + std::unique_ptr sender, + std::unique_ptr receiver) + : mId(0) + , mType(type) + , mExpiry(0) + , mSender(std::move(sender)) + , mReceiver(std::move(receiver)) { } Letter::~Letter() { - if (mSender) - delete mSender; - - if (mReceiver) - delete mReceiver; } void Letter::setExpiry(unsigned long expiry) @@ -72,12 +73,12 @@ bool Letter::addAttachment(InventoryItem item) CharacterData *Letter::getReceiver() const { - return mReceiver; + return mReceiver.get(); } CharacterData *Letter::getSender() const { - return mSender; + return mSender.get(); } const std::vector &Letter::getAttachments() const @@ -113,10 +114,6 @@ bool Post::addLetter(Letter *letter) Letter* Post::getLetter(int letter) const { - if (letter < 0 || (size_t) letter > mLetters.size()) - { - return nullptr; - } return mLetters[letter]; } @@ -137,9 +134,7 @@ void PostManager::addLetter(Letter *letter) { auto post = new Post(); post->addLetter(letter); - mPostBox.insert( - std::pair(letter->getReceiver(), post) - ); + mPostBox.insert(std::make_pair(letter->getReceiver(), post)); } } diff --git a/src/chat-server/post.h b/src/chat-server/post.h index de15bf1a..eb8cdb71 100644 --- a/src/chat-server/post.h +++ b/src/chat-server/post.h @@ -22,10 +22,11 @@ #define POST_H #include +#include #include #include -#include "../common/inventorydata.h" +#include "common/inventorydata.h" class CharacterData; @@ -41,7 +42,9 @@ public: * @param sender Pointer to character that sent the letter * @param receiver Pointer to character that will receive the letter */ - Letter(unsigned type, CharacterData *sender, CharacterData *receiver); + Letter(unsigned type, + std::unique_ptr sender, + std::unique_ptr receiver); ~Letter(); @@ -117,8 +120,8 @@ private: unsigned long mExpiry; std::string mContents; std::vector mAttachments; - CharacterData *mSender; - CharacterData *mReceiver; + std::unique_ptr mSender; + std::unique_ptr mReceiver; }; class Post diff --git a/src/common/transaction.h b/src/common/transaction.h index 12a1d61a..518c9844 100644 --- a/src/common/transaction.h +++ b/src/common/transaction.h @@ -21,6 +21,8 @@ #ifndef TRANSACTION_H #define TRANSACTION_H +#include + struct Transaction { unsigned mAction; diff --git a/src/net/netcomputer.cpp b/src/net/netcomputer.cpp index f2d9607b..d423fc84 100644 --- a/src/net/netcomputer.cpp +++ b/src/net/netcomputer.cpp @@ -26,8 +26,8 @@ #include "messageout.h" #include "netcomputer.h" -#include "../utils/logger.h" -#include "../utils/processorutils.h" +#include "utils/logger.h" +#include "utils/processorutils.h" NetComputer::NetComputer(ENetPeer *peer): mPeer(peer) -- cgit v1.2.3-60-g2f50