Use Guild shared_ptr instead of raw pointers (#4682)

* Use Guild shared_ptr * Member rank level default to constexpr * Use const when possible * @ramon-bernardo Use const when possible * Use shared_ptr alias * Fix * Fix * Fix * Fix * Fix delete * Fix * No need to reuse the variable * Use const shared ptr * Use const& ptr * Inline rank check
otland · May 26, 2024 · 187a517 · 187a517
1 parent 33e9698
commit 187a517
Show file tree

Hide file tree

Showing 10 changed files with 85 additions and 103 deletions.
diff --git a/src/chat.cpp b/src/chat.cpp
@@ -76,8 +76,7 @@ bool ChatChannel::addUser(Player& player)
 
 	// TODO: Move to script when guild channels can be scripted
 	if (id == CHANNEL_GUILD) {
-		Guild* guild = player.getGuild();
-		if (guild && !guild->getMotd().empty()) {
+		if (const auto& guild = player.getGuild(); !guild->getMotd().empty()) {
 			g_scheduler.addEvent(
 			    createSchedulerTask(150, [playerID = player.getID()]() { g_game.sendGuildMotd(playerID); }));
 		}
@@ -329,8 +328,7 @@ ChatChannel* Chat::createChannel(const Player& player, uint16_t channelId)
 
 	switch (channelId) {
 		case CHANNEL_GUILD: {
-			Guild* guild = player.getGuild();
-			if (guild) {
+			if (const auto& guild = player.getGuild()) {
 				auto ret =
 				    guildChannels.emplace(std::make_pair(guild->getId(), ChatChannel(channelId, guild->getName())));
 				return &ret.first->second;
@@ -376,7 +374,7 @@ bool Chat::deleteChannel(const Player& player, uint16_t channelId)
 {
 	switch (channelId) {
 		case CHANNEL_GUILD: {
-			Guild* guild = player.getGuild();
+			const auto& guild = player.getGuild();
 			if (!guild) {
 				return false;
 			}
@@ -478,7 +476,7 @@ bool Chat::talkToChannel(const Player& player, SpeakClasses type, const std::str
 	}
 
 	if (channelId == CHANNEL_GUILD) {
-		GuildRank_ptr rank = player.getGuildRank();
+		const auto& rank = player.getGuildRank();
 		if (rank && rank->level > 1) {
 			type = TALKTYPE_CHANNEL_O;
 		} else if (type != TALKTYPE_CHANNEL_Y) {
@@ -553,8 +551,7 @@ ChatChannel* Chat::getChannel(const Player& player, uint16_t channelId)
 {
 	switch (channelId) {
 		case CHANNEL_GUILD: {
-			Guild* guild = player.getGuild();
-			if (guild) {
+			if (const auto& guild = player.getGuild()) {
 				auto it = guildChannels.find(guild->getId());
 				if (it != guildChannels.end()) {
 					return &it->second;

diff --git a/src/game.cpp b/src/game.cpp
@@ -61,13 +61,6 @@ Game::Game()
 	offlineTrainingWindow.priority = true;
 }
 
-Game::~Game()
-{
-	for (const auto& it : guilds) {
-		delete it.second;
-	}
-}
-
 void Game::start(ServiceManager* manager)
 {
 	serviceManager = manager;
@@ -5066,8 +5059,7 @@ void Game::sendGuildMotd(uint32_t playerId)
 		return;
 	}
 
-	Guild* guild = player->getGuild();
-	if (guild) {
+	if (const auto& guild = player->getGuild()) {
 		player->sendChannelMessage("Message of the Day", guild->getMotd(), TALKTYPE_CHANNEL_R1, CHANNEL_GUILD);
 	}
 }
@@ -5702,7 +5694,7 @@ void Game::addMonster(Monster* monster) { monsters[monster->getID()] = monster;
 
 void Game::removeMonster(Monster* monster) { monsters.erase(monster->getID()); }
 
-Guild* Game::getGuild(uint32_t id) const
+Guild_ptr Game::getGuild(uint32_t id) const
 {
 	auto it = guilds.find(id);
 	if (it == guilds.end()) {
@@ -5711,7 +5703,7 @@ Guild* Game::getGuild(uint32_t id) const
 	return it->second;
 }
 
-void Game::addGuild(Guild* guild) { guilds[guild->getId()] = guild; }
+void Game::addGuild(Guild_ptr guild) { guilds[guild->getId()] = guild; }
 
 void Game::removeGuild(uint32_t guildId) { guilds.erase(guildId); }
 

diff --git a/src/game.h b/src/game.h
@@ -73,7 +73,6 @@ class Game
 {
 public:
 	Game();
-	~Game();
 
 	// non-copyable
 	Game(const Game&) = delete;
@@ -464,8 +463,8 @@ class Game
 	void addMonster(Monster* monster);
 	void removeMonster(Monster* monster);
 
-	Guild* getGuild(uint32_t id) const;
-	void addGuild(Guild* guild);
+	Guild_ptr getGuild(uint32_t id) const;
+	void addGuild(Guild_ptr guild);
 	void removeGuild(uint32_t guildId);
 	void decreaseBrowseFieldRef(const Position& pos);
 
@@ -510,7 +509,7 @@ class Game
 	std::unordered_map<uint32_t, Player*> players;
 	std::unordered_map<std::string, Player*> mappedPlayerNames;
 	std::unordered_map<uint32_t, Player*> mappedPlayerGuids;
-	std::unordered_map<uint32_t, Guild*> guilds;
+	std::unordered_map<uint32_t, Guild_ptr> guilds;
 	std::unordered_map<uint16_t, Item*> uniqueItems;
 
 	std::list<Item*> decayItems[EVENT_DECAY_BUCKETS];

diff --git a/src/guild.cpp b/src/guild.cpp
@@ -17,10 +17,14 @@ void Guild::removeMember(Player* player)
 
 	if (membersOnline.empty()) {
 		g_game.removeGuild(id);
-		delete this;
 	}
 }
 
+void Guild::addRank(uint32_t rankId, std::string_view rankName, uint8_t level)
+{
+	ranks.emplace_back(std::make_shared<GuildRank>(rankId, rankName, level));
+}
+
 GuildRank_ptr Guild::getRankById(uint32_t rankId)
 {
 	for (auto rank : ranks) {
@@ -51,27 +55,23 @@ GuildRank_ptr Guild::getRankByLevel(uint8_t level) const
 	return nullptr;
 }
 
-void Guild::addRank(uint32_t rankId, std::string_view rankName, uint8_t level)
-{
-	ranks.emplace_back(std::make_shared<GuildRank>(rankId, rankName, level));
-}
-
-Guild* IOGuild::loadGuild(uint32_t guildId)
+Guild_ptr IOGuild::loadGuild(uint32_t guildId)
 {
 	Database& db = Database::getInstance();
-	if (DBResult_ptr result = db.storeQuery(fmt::format("SELECT `name` FROM `guilds` WHERE `id` = {:d}", guildId))) {
-		Guild* guild = new Guild(guildId, result->getString("name"));
+	DBResult_ptr result = db.storeQuery(fmt::format("SELECT `name` FROM `guilds` WHERE `id` = {:d}", guildId));
+	if (!result) {
+		return nullptr;
+	}
 
-		if ((result = db.storeQuery(
-		         fmt::format("SELECT `id`, `name`, `level` FROM `guild_ranks` WHERE `guild_id` = {:d}", guildId)))) {
-			do {
-				guild->addRank(result->getNumber<uint32_t>("id"), result->getString("name"),
-				               result->getNumber<uint16_t>("level"));
-			} while (result->next());
-		}
-		return guild;
+	const auto& guild = std::make_shared<Guild>(guildId, result->getString("name"));
+	if (auto result = db.storeQuery(
+	        fmt::format("SELECT `id`, `name`, `level` FROM `guild_ranks` WHERE `guild_id` = {:d}", guildId))) {
+		do {
+			guild->addRank(result->getNumber<uint32_t>("id"), result->getString("name"),
+			               result->getNumber<uint16_t>("level"));
+		} while (result->next());
 	}
-	return nullptr;
+	return guild;
 }
 
 uint32_t IOGuild::getGuildIdByName(const std::string& name)

diff --git a/src/guild.h b/src/guild.h
@@ -6,6 +6,8 @@
 
 class Player;
 
+using GuildWarVector = std::vector<uint32_t>;
+
 struct GuildRank
 {
 	uint32_t id;
@@ -20,22 +22,24 @@ using GuildRank_ptr = std::shared_ptr<GuildRank>;
 class Guild
 {
 public:
-	Guild(uint32_t id, std::string_view name) : name{name}, id{id} {}
+	static constexpr uint8_t MEMBER_RANK_LEVEL_DEFAULT = 1;
 
-	void addMember(Player* player);
-	void removeMember(Player* player);
+	Guild(uint32_t id, std::string_view name) : name{name}, id{id} {}
 
 	uint32_t getId() const { return id; }
 	const std::string& getName() const { return name; }
+
+	void addMember(Player* player);
+	void removeMember(Player* player);
 	const std::list<Player*>& getMembersOnline() const { return membersOnline; }
 	uint32_t getMemberCount() const { return memberCount; }
 	void setMemberCount(uint32_t count) { memberCount = count; }
 
+	void addRank(uint32_t rankId, std::string_view rankName, uint8_t level);
 	const std::vector<GuildRank_ptr>& getRanks() const { return ranks; }
 	GuildRank_ptr getRankById(uint32_t rankId);
 	GuildRank_ptr getRankByName(const std::string& name) const;
 	GuildRank_ptr getRankByLevel(uint8_t level) const;
-	void addRank(uint32_t rankId, std::string_view rankName, uint8_t level);
 
 	const std::string& getMotd() const { return motd; }
 	void setMotd(const std::string& motd) { this->motd = motd; }
@@ -49,10 +53,10 @@ class Guild
 	uint32_t memberCount = 0;
 };
 
-using GuildWarVector = std::vector<uint32_t>;
+using Guild_ptr = std::shared_ptr<Guild>;
 
 namespace IOGuild {
-Guild* loadGuild(uint32_t guildId);
+Guild_ptr loadGuild(uint32_t guildId);
 uint32_t getGuildIdByName(const std::string& name);
 }; // namespace IOGuild
 

diff --git a/src/house.cpp b/src/house.cpp
@@ -431,15 +431,14 @@ void AccessList::addPlayer(const std::string& name)
 
 namespace {
 
-const Guild* getGuildByName(const std::string& name)
+const Guild_ptr getGuildByName(const std::string& name)
 {
 	uint32_t guildId = IOGuild::getGuildIdByName(name);
 	if (guildId == 0) {
 		return nullptr;
 	}
 
-	const Guild* guild = g_game.getGuild(guildId);
-	if (guild) {
+	if (const auto& guild = g_game.getGuild(guildId)) {
 		return guild;
 	}
 
@@ -450,20 +449,17 @@ const Guild* getGuildByName(const std::string& name)
 
 void AccessList::addGuild(const std::string& name)
 {
-	const Guild* guild = getGuildByName(name);
-	if (guild) {
-		for (auto rank : guild->getRanks()) {
+	if (const auto& guild = getGuildByName(name)) {
+		for (const auto& rank : guild->getRanks()) {
 			guildRankList.insert(rank->id);
 		}
 	}
 }
 
 void AccessList::addGuildRank(const std::string& name, const std::string& rankName)
 {
-	const Guild* guild = getGuildByName(name);
-	if (guild) {
-		GuildRank_ptr rank = guild->getRankByName(rankName);
-		if (rank) {
+	if (const auto& guild = getGuildByName(name)) {
+		if (const auto& rank = guild->getRankByName(rankName)) {
 			guildRankList.insert(rank->id);
 		}
 	}
@@ -480,7 +476,7 @@ bool AccessList::isInList(const Player* player) const
 		return true;
 	}
 
-	GuildRank_ptr rank = player->getGuildRank();
+	const auto& rank = player->getGuildRank();
 	return rank && guildRankList.find(rank->id) != guildRankList.end();
 }
 

diff --git a/src/iologindata.cpp b/src/iologindata.cpp
@@ -422,7 +422,7 @@ bool IOLoginData::loadPlayer(Player* player, DBResult_ptr result)
 		uint32_t playerRankId = result->getNumber<uint32_t>("rank_id");
 		player->guildNick = result->getString("nick");
 
-		Guild* guild = g_game.getGuild(guildId);
+		auto guild = g_game.getGuild(guildId);
 		if (!guild) {
 			guild = IOGuild::loadGuild(guildId);
 			if (guild) {
@@ -435,7 +435,7 @@ bool IOLoginData::loadPlayer(Player* player, DBResult_ptr result)
 
 		if (guild) {
 			player->guild = guild;
-			GuildRank_ptr rank = guild->getRankById(playerRankId);
+			auto rank = guild->getRankById(playerRankId);
 			if (!rank) {
 				if ((result = db.storeQuery(fmt::format(
 				         "SELECT `id`, `name`, `level` FROM `guild_ranks` WHERE `id` = {:d}", playerRankId)))) {