improve: review of everything using set (opentibiabr#1705)

Refactor set implementation across the codebase for optimal performance.

Detailed Description:
This encompasses a comprehensive review and overhaul of `std::set` usage within the project, replacing it with more efficient collections as applicable. The refactoring targets instances where set operations are utilized, with a critical analysis of performance considerations.

Key Changes:
• Avoid `std::set` due to ordering overhead when order is not a requirement.
• Employ `std::unordered_set` for scenarios with infrequent insertions and deletions but frequent searches.
• Integrate `phmap::flat_hash_set` as a balanced option for various operations without linear searches.
• Introduce `vector_set` for batch insertions and short-term unique data handling within methods.

Performance Insights:
The `vector_set` is particularly notable for its batch insertion efficiency and automatic sorting and deduplication upon search or deletion operations, making it suitable for maintaining a unique set of elements temporarily.

Example Usage:
stdext::vector_set<int> unique_list;
unique_list.insertAll({1, 3, 4, 5});
unique_list.insertAll({4, 2, 87, 2});
unique_list.insertAll({3, 1, 99, 5});

unique_list now contains 1, 2, 3, 4, 5, 87, 99 without duplicates

src/creatures/combat/combat.cpp

@@ -1311,7 +1311,7 @@ std::vector<std::pair<Position, std::vector<uint32_t>>> Combat::pickChainTargets
 
 	std::vector<std::pair<Position, std::vector<uint32_t>>> resultMap;
 	std::vector<std::shared_ptr<Creature>> targets;
-	std::set<uint32_t> visited;
+	phmap::flat_hash_set<uint32_t> visited;
 
 	if (initialTarget && initialTarget != caster) {
 		targets.push_back(initialTarget);

src/creatures/creature.cpp

@@ -431,22 +431,22 @@ void Creature::checkSummonMove(const Position &newPos, bool teleportSummon) {
 	if (hasSummons()) {
 		std::vector<std::shared_ptr<Creature>> despawnMonsterList;
 		for (const auto &summon : getSummons()) {
-			const Position &pos = summon->getPosition();
-			std::shared_ptr<Monster> monster = summon->getMonster();
-			auto tile = getTile();
+			const auto &pos = summon->getPosition();
+			const auto &monster = summon->getMonster();
+			const auto &tile = getTile();
 			bool protectionZoneCheck = tile ? tile->hasFlag(TILESTATE_PROTECTIONZONE) : false;
 			// Check if any of our summons is out of range (+/- 0 floors or 15 tiles away)
 			bool checkSummonDist = Position::getDistanceZ(newPos, pos) > 0 || (std::max<int32_t>(Position::getDistanceX(newPos, pos), Position::getDistanceY(newPos, pos)) > 15);
 			// Check if any of our summons is out of range (+/- 2 floors or 30 tiles away)
 			bool checkRemoveDist = Position::getDistanceZ(newPos, pos) > 2 || (std::max<int32_t>(Position::getDistanceX(newPos, pos), Position::getDistanceY(newPos, pos)) > 30);
 
 			if (monster && monster->isFamiliar() && checkSummonDist || teleportSummon && !protectionZoneCheck && checkSummonDist) {
-				auto creatureMaster = summon->getMaster();
+				const auto &creatureMaster = summon->getMaster();
 				if (!creatureMaster) {
 					continue;
 				}
 
-				if (std::shared_ptr<Tile> masterTile = creatureMaster->getTile()) {
+				if (const auto &masterTile = creatureMaster->getTile()) {
 					if (masterTile->hasFlag(TILESTATE_TELEPORT)) {
 						g_logger().warn("[{}] cannot teleport summon, position has teleport. {}", __FUNCTION__, creatureMaster->getPosition().toString());
 					} else {
@@ -461,7 +461,7 @@ void Creature::checkSummonMove(const Position &newPos, bool teleportSummon) {
 			}
 		}
 
-		for (std::shared_ptr<Creature> despawnCreature : despawnMonsterList) {
+		for (const auto &despawnCreature : despawnMonsterList) {
 			if (!despawnMonsterList.empty()) {
 				g_game().removeCreature(despawnCreature, true);
 			}
@@ -1243,13 +1243,16 @@ bool Creature::setMaster(std::shared_ptr<Creature> newMaster, bool reloadCreatur
 		g_game().reloadCreature(self);
 	}
 	if (newMaster) {
-		newMaster->m_summons.insert(self);
+		newMaster->m_summons.emplace_back(self);
 	}
 
 	m_master = newMaster;
 
 	if (oldMaster) {
-		oldMaster->m_summons.erase(self);
+		const auto &it = std::ranges::find(oldMaster->m_summons, self);
+		if (it != oldMaster->m_summons.end()) {
+			oldMaster->m_summons.erase(it);
+		}
 	}
 	return true;
 }
@@ -1796,9 +1799,8 @@ void Creature::setIncreasePercent(CombatType_t combat, int32_t value) {
 	}
 }
 
-phmap::flat_hash_set<std::shared_ptr<Zone>> Creature::getZones() {
-	auto tile = getTile();
-	if (tile) {
+std::unordered_set<std::shared_ptr<Zone>> Creature::getZones() {
+	if (const auto &tile = getTile()) {
 		return tile->getZones();
 	}
 	return {};

src/creatures/creature.hpp

@@ -267,7 +267,7 @@ class Creature : virtual public Thing, public SharedObject {
 		return ZONE_NORMAL;
 	}
 
-	phmap::flat_hash_set<std::shared_ptr<Zone>> getZones();
+	std::unordered_set<std::shared_ptr<Zone>> getZones();
 
 	// walk functions
 	void startAutoWalk(const std::vector<Direction> &listDir, bool ignoreConditions = false);
@@ -341,7 +341,7 @@ class Creature : virtual public Thing, public SharedObject {
 		return m_master.lock();
 	}
 
-	const phmap::flat_hash_set<std::shared_ptr<Creature>> &getSummons() const {
+	const auto &getSummons() const {
 		return m_summons;
 	}
 
@@ -666,7 +666,7 @@ class Creature : virtual public Thing, public SharedObject {
 
 	CountMap damageMap;
 
-	phmap::flat_hash_set<std::shared_ptr<Creature>> m_summons;
+	std::vector<std::shared_ptr<Creature>> m_summons;
 	CreatureEventList eventsList;
 	ConditionList conditions;
 

src/creatures/monsters/monster.hpp

@@ -17,7 +17,6 @@ class Creature;
 class Game;
 class Spawn;
 
-using CreatureHashSet = phmap::flat_hash_set<std::shared_ptr<Creature>>;
 using CreatureList = std::list<std::shared_ptr<Creature>>;
 
 using CreatureWeakHashMap = phmap::flat_hash_map<uint32_t, std::weak_ptr<Creature>>;
@@ -99,7 +98,7 @@ class Monster final : public Creature {
 		if (master && master->getMonster()) {
 			return master->getMonster()->isEnemyFaction(faction);
 		}
-		return mType->info.enemyFactions.empty() ? false : mType->info.enemyFactions.find(faction) != mType->info.enemyFactions.end();
+		return mType->info.enemyFactions.empty() ? false : mType->info.enemyFactions.contains(faction);
 	}
 
 	bool isPushable() override {
@@ -204,17 +203,19 @@ class Monster final : public Creature {
 		}
 		return list;
 	}
-	CreatureHashSet getFriendList() {
-		CreatureHashSet set;
+
+	std::vector<std::shared_ptr<Creature>> getFriendList() {
+		std::vector<std::shared_ptr<Creature>> list;
+
 		for (auto it = friendList.begin(); it != friendList.end();) {
 			if (auto friendCreature = it->second.lock()) {
-				set.insert(friendCreature);
+				list.emplace_back(friendCreature);
 				++it;
 			} else {
 				it = friendList.erase(it);
 			}
 		}
-		return set;
+		return list;
 	}
 
 	bool isTarget(std::shared_ptr<Creature> creature);

src/creatures/monsters/monsters.hpp

@@ -136,7 +136,7 @@ class MonsterType {
 		bool targetPreferMaster = false;
 
 		Faction_t faction = FACTION_DEFAULT;
-		phmap::flat_hash_set<Faction_t> enemyFactions;
+		stdext::vector_set<Faction_t> enemyFactions;
 
 		bool canPushItems = false;
 		bool canPushCreatures = false;

src/creatures/npcs/npc.cpp

@@ -151,7 +151,7 @@ void Npc::onPlayerAppear(std::shared_ptr<Player> player) {
 	if (player->hasFlag(PlayerFlags_t::IgnoredByNpcs) || playerSpectators.contains(player)) {
 		return;
 	}
-	playerSpectators.emplace_back(player);
+	playerSpectators.emplace(player);
 	manageIdle();
 }
 
@@ -531,7 +531,7 @@ void Npc::onThinkWalk(uint32_t interval) {
 
 void Npc::onCreatureWalk() {
 	Creature::onCreatureWalk();
-	playerSpectators.erase_if([this](const auto &creature) { return !this->canSee(creature->getPosition()); });
+	phmap::erase_if(playerSpectators, [this](const auto &creature) { return !this->canSee(creature->getPosition()); });
 }
 
 void Npc::onPlacedCreature() {
@@ -542,7 +542,7 @@ void Npc::loadPlayerSpectators() {
 	auto spec = Spectators().find<Player>(position, true);
 	for (const auto &creature : spec) {
 		if (!creature->getPlayer()->hasFlag(PlayerFlags_t::IgnoredByNpcs)) {
-			playerSpectators.emplace_back(creature->getPlayer());
+			playerSpectators.emplace(creature->getPlayer());
 		}
 	}
 }

src/creatures/npcs/npc.hpp

@@ -188,7 +188,7 @@ class Npc final : public Creature {
 
 	bool ignoreHeight;
 
-	stdext::vector_set<std::shared_ptr<Player>> playerSpectators;
+	phmap::flat_hash_set<std::shared_ptr<Player>> playerSpectators;
 	Position masterPos;
 
 	friend class LuaScriptInterface;

src/creatures/players/player.cpp

@@ -484,54 +484,48 @@ uint32_t Player::getClientIcons() {
 }
 
 void Player::addMonsterToCyclopediaTrackerList(const std::shared_ptr<MonsterType> mtype, bool isBoss, bool reloadClient /* = false */) {
-	if (client) {
-		uint16_t raceId = mtype ? mtype->info.raceid : 0;
-		// Bostiary tracker logic
-		if (isBoss) {
-			m_bosstiaryMonsterTracker.insert(mtype);
-			if (reloadClient && raceId != 0) {
+	if (!client) {
+		return;
+	}
+
+	const uint16_t raceId = mtype ? mtype->info.raceid : 0;
+	auto &tracker = isBoss ? m_bosstiaryMonsterTracker : m_bestiaryMonsterTracker;
+	if (tracker.emplace(mtype).second) {
+		if (reloadClient && raceId != 0) {
+			if (isBoss) {
 				client->parseSendBosstiary();
+			} else {
+				client->sendBestiaryEntryChanged(raceId);
 			}
-			client->refreshCyclopediaMonsterTracker(m_bosstiaryMonsterTracker, true);
-			return;
 		}
 
-		// Bestiary tracker logic
-		m_bestiaryMonsterTracker.insert(mtype);
-		if (reloadClient && raceId != 0) {
-			client->sendBestiaryEntryChanged(raceId);
-		}
-		client->refreshCyclopediaMonsterTracker(m_bestiaryMonsterTracker, false);
+		client->refreshCyclopediaMonsterTracker(tracker, isBoss);
 	}
 }
 
 void Player::removeMonsterFromCyclopediaTrackerList(std::shared_ptr<MonsterType> mtype, bool isBoss, bool reloadClient /* = false */) {
-	if (client) {
-		uint16_t raceId = mtype ? mtype->info.raceid : 0;
-		// Bostiary tracker logic
-		if (isBoss) {
-			m_bosstiaryMonsterTracker.erase(mtype);
-			if (reloadClient && raceId != 0) {
+	if (!client) {
+		return;
+	}
+
+	const uint16_t raceId = mtype ? mtype->info.raceid : 0;
+	auto &tracker = isBoss ? m_bosstiaryMonsterTracker : m_bestiaryMonsterTracker;
+
+	if (tracker.erase(mtype) > 0) {
+		if (reloadClient && raceId != 0) {
+			if (isBoss) {
 				client->parseSendBosstiary();
+			} else {
+				client->sendBestiaryEntryChanged(raceId);
 			}
-			client->refreshCyclopediaMonsterTracker(m_bosstiaryMonsterTracker, true);
-			return;
 		}
 
-		// Bestiary tracker logic
-		m_bestiaryMonsterTracker.erase(mtype);
-		if (reloadClient && raceId != 0) {
-			client->sendBestiaryEntryChanged(raceId);
-		}
-		client->refreshCyclopediaMonsterTracker(m_bestiaryMonsterTracker, false);
+		client->refreshCyclopediaMonsterTracker(tracker, isBoss);
 	}
 }
 
-bool Player::isBossOnBosstiaryTracker(const std::shared_ptr<MonsterType> monsterType) const {
-	if (!monsterType) {
-		return false;
-	}
-	return m_bosstiaryMonsterTracker.contains(monsterType);
+bool Player::isBossOnBosstiaryTracker(const std::shared_ptr<MonsterType> &monsterType) const {
+	return monsterType ? m_bosstiaryMonsterTracker.contains(monsterType) : false;
 }
 
 void Player::updateInventoryWeight() {
@@ -2925,7 +2919,7 @@ void Player::removePlayer(bool displayEffect, bool forced /*= true*/) {
 	}
 }
 
-void Player::notifyStatusChange(std::shared_ptr<Player> loginPlayer, VipStatus_t status, bool message) {
+void Player::notifyStatusChange(std::shared_ptr<Player> loginPlayer, VipStatus_t status, bool message) const {
 	if (!client) {
 		return;
 	}
@@ -2989,7 +2983,7 @@ bool Player::addVIPInternal(uint32_t vipGuid) {
 	return VIPList.insert(vipGuid).second;
 }
 
-bool Player::editVIP(uint32_t vipGuid, const std::string &description, uint32_t icon, bool notify) {
+bool Player::editVIP(uint32_t vipGuid, const std::string &description, uint32_t icon, bool notify) const {
 	auto it = VIPList.find(vipGuid);
 	if (it == VIPList.end()) {
 		return false; // player is not in VIP

src/creatures/players/player.hpp

@@ -304,7 +304,7 @@ class Player final : public Creature, public Cylinder, public Bankable {
 		return guildWarVector;
 	}
 
-	const phmap::parallel_flat_hash_set<std::shared_ptr<MonsterType>> &getCyclopediaMonsterTrackerSet(bool isBoss) const {
+	const std::unordered_set<std::shared_ptr<MonsterType>> &getCyclopediaMonsterTrackerSet(bool isBoss) const {
 		return isBoss ? m_bosstiaryMonsterTracker : m_bestiaryMonsterTracker;
 	}
 
@@ -318,17 +318,17 @@ class Player final : public Creature, public Cylinder, public Bankable {
 		}
 	}
 
-	void refreshCyclopediaMonsterTracker(bool isBoss = false) const {
+	void refreshCyclopediaMonsterTracker(bool isBoss = false) {
 		refreshCyclopediaMonsterTracker(getCyclopediaMonsterTrackerSet(isBoss), isBoss);
 	}
 
-	void refreshCyclopediaMonsterTracker(const phmap::parallel_flat_hash_set<std::shared_ptr<MonsterType>> &trackerList, bool isBoss) const {
+	void refreshCyclopediaMonsterTracker(const std::unordered_set<std::shared_ptr<MonsterType>> &trackerList, bool isBoss) const {
 		if (client) {
 			client->refreshCyclopediaMonsterTracker(trackerList, isBoss);
 		}
 	}
 
-	bool isBossOnBosstiaryTracker(const std::shared_ptr<MonsterType> monsterType) const;
+	bool isBossOnBosstiaryTracker(const std::shared_ptr<MonsterType> &monsterType) const;
 
 	Vocation* getVocation() const {
 		return vocation;
@@ -804,11 +804,11 @@ class Player final : public Creature, public Cylinder, public Bankable {
 	}
 
 	// V.I.P. functions
-	void notifyStatusChange(std::shared_ptr<Player> player, VipStatus_t status, bool message = true);
+	void notifyStatusChange(std::shared_ptr<Player> player, VipStatus_t status, bool message = true) const;
 	bool removeVIP(uint32_t vipGuid);
 	bool addVIP(uint32_t vipGuid, const std::string &vipName, VipStatus_t status);
 	bool addVIPInternal(uint32_t vipGuid);
-	bool editVIP(uint32_t vipGuid, const std::string &description, uint32_t icon, bool notify);
+	bool editVIP(uint32_t vipGuid, const std::string &description, uint32_t icon, bool notify) const;
 
 	// follow functions
 	bool setFollowCreature(std::shared_ptr<Creature> creature) override;
@@ -2609,7 +2609,6 @@ class Player final : public Creature, public Cylinder, public Bankable {
 	bool onKilledMonster(const std::shared_ptr<Monster> &target, bool lastHit);
 
 	phmap::flat_hash_set<uint32_t> attackedSet;
-
 	phmap::flat_hash_set<uint32_t> VIPList;
 
 	std::map<uint8_t, OpenContainer> openContainers;
@@ -2646,8 +2645,8 @@ class Player final : public Creature, public Cylinder, public Bankable {
 	// TODO: This variable is only temporarily used when logging in, get rid of it somehow.
 	std::forward_list<std::shared_ptr<Condition>> storedConditionList;
 
-	phmap::parallel_flat_hash_set<std::shared_ptr<MonsterType>> m_bestiaryMonsterTracker;
-	phmap::parallel_flat_hash_set<std::shared_ptr<MonsterType>> m_bosstiaryMonsterTracker;
+	std::unordered_set<std::shared_ptr<MonsterType>> m_bestiaryMonsterTracker;
+	std::unordered_set<std::shared_ptr<MonsterType>> m_bosstiaryMonsterTracker;
 
 	std::string name;
 	std::string guildNick;