fix: circular inclusion on tools.hpp/remove const ref from string view (

#3019)

This commit addresses two issues:

Circular Inclusion in tools.hpp:
• Resolved a circular inclusion problem in tools.hpp, which was causing compilation issues in certain contexts due to repeated and conflicting header dependencies. The inclusion logic has been adjusted to prevent this, improving the overall stability of the build process.

Removed const& from std::string_view:
• Removed the unnecessary const& from std::string_view parameters. Since std::string_view is a lightweight, non-owning reference type, passing it by reference doesn’t provide any performance benefit, and can instead introduce potential inefficiencies. This change improves both readability and performance.

These fixes contribute to a cleaner, more maintainable codebase and improve the compilation reliability.

src/creatures/combat/spells.hpp

@@ -333,15 +333,15 @@ class Spell : public BaseSpell {
 		return m_words;
 	}
 
-	void setWords(const std::string_view &newWord) {
+	void setWords(std::string_view newWord) {
 		m_words = newWord.data();
 	}
 
 	[[nodiscard]] const std::string &getSeparator() const {
 		return m_separator;
 	}
 
-	void setSeparator(const std::string_view &newSeparator) {
+	void setSeparator(std::string_view newSeparator) {
 		m_separator = newSeparator.data();
 	}
 

src/creatures/players/wheel/player_wheel.cpp

@@ -96,7 +96,7 @@ namespace {
 	}
 
 	template <typename SpellType>
-	int checkSpellAdditionalTarget(const std::array<SpellType, 5> &spellsTable, const std::string_view &spellName, uint8_t stage) {
+	int checkSpellAdditionalTarget(const std::array<SpellType, 5> &spellsTable, std::string_view spellName, uint8_t stage) {
 		for (const auto &spellTable : spellsTable) {
 			auto size = std::ssize(spellTable.grade);
 			g_logger().debug("spell target stage {}, grade {}", stage, size);
@@ -112,7 +112,7 @@ namespace {
 	}
 
 	template <typename SpellType>
-	int checkSpellAdditionalDuration(const std::array<SpellType, 5> &spellsTable, const std::string_view &spellName, uint8_t stage) {
+	int checkSpellAdditionalDuration(const std::array<SpellType, 5> &spellsTable, std::string_view spellName, uint8_t stage) {
 		for (const auto &spellTable : spellsTable) {
 			auto size = std::ssize(spellTable.grade);
 			g_logger().debug("spell duration stage {}, grade {}", stage, size);

src/database/database.cpp

@@ -105,7 +105,7 @@ bool Database::isRecoverableError(unsigned int error) const {
 	return error == CR_SERVER_LOST || error == CR_SERVER_GONE_ERROR || error == CR_CONN_HOST_ERROR || error == 1053 /*ER_SERVER_SHUTDOWN*/ || error == CR_CONNECTION_ERROR;
 }
 
-bool Database::retryQuery(const std::string_view &query, int retries) {
+bool Database::retryQuery(std::string_view query, int retries) {
 	while (retries > 0 && mysql_query(handle, query.data()) != 0) {
 		g_logger().error("Query: {}", query.substr(0, 256));
 		g_logger().error("MySQL error [{}]: {}", mysql_errno(handle), mysql_error(handle));
@@ -123,7 +123,7 @@ bool Database::retryQuery(const std::string_view &query, int retries) {
 	return true;
 }
 
-bool Database::executeQuery(const std::string_view &query) {
+bool Database::executeQuery(std::string_view query) {
 	if (!handle) {
 		g_logger().error("Database not initialized!");
 		return false;
@@ -142,7 +142,7 @@ bool Database::executeQuery(const std::string_view &query) {
 	return success;
 }
 
-DBResult_ptr Database::storeQuery(const std::string_view &query) {
+DBResult_ptr Database::storeQuery(std::string_view query) {
 	if (!handle) {
 		g_logger().error("Database not initialized!");
 		return nullptr;

src/database/database.hpp

@@ -36,10 +36,10 @@ class Database {
 
 	bool connect(const std::string* host, const std::string* user, const std::string* password, const std::string* database, uint32_t port, const std::string* sock);
 
-	bool retryQuery(const std::string_view &query, int retries);
-	bool executeQuery(const std::string_view &query);
+	bool retryQuery(std::string_view query, int retries);
+	bool executeQuery(std::string_view query);
 
-	DBResult_ptr storeQuery(const std::string_view &query);
+	DBResult_ptr storeQuery(std::string_view query);
 
 	std::string escapeString(const std::string &s) const;
 

src/enums/object_category.hpp

@@ -9,7 +9,7 @@
 
 #pragma once
 
-enum ObjectCategory_t {
+enum ObjectCategory_t : uint8_t {
 	OBJECTCATEGORY_NONE = 0,
 	OBJECTCATEGORY_ARMORS = 1,
 	OBJECTCATEGORY_NECKLACES = 2,

src/game/game.cpp

@@ -10242,7 +10242,7 @@ bool Game::removeFiendishMonster(uint32_t id, bool create /* = true*/) {
 
 void Game::updateForgeableMonsters() {
 	forgeableMonsters.clear();
-	for (auto [monsterId, monster] : monsters) {
+	for (const auto &[monsterId, monster] : monsters) {
 		auto monsterTile = monster->getTile();
 		if (!monsterTile) {
 			continue;

src/io/io_bosstiary.cpp

@@ -135,7 +135,7 @@ const std::map<uint16_t, std::string> &IOBosstiary::getBosstiaryMap() const {
 	return bosstiaryMap;
 }
 
-void IOBosstiary::setBossBoostedName(const std::string_view &name) {
+void IOBosstiary::setBossBoostedName(std::string_view name) {
 	boostedBoss = name;
 }
 

src/io/io_bosstiary.hpp

@@ -54,7 +54,7 @@ class IOBosstiary {
 		{ BosstiaryRarity_t::RARITY_NEMESIS, { { 1, 10 }, { 3, 30 }, { 5, 60 } } }
 	};
 
-	void setBossBoostedName(const std::string_view &name);
+	void setBossBoostedName(std::string_view name);
 	std::string getBoostedBossName() const;
 	void setBossBoostedId(uint16_t raceId);
 	uint16_t getBoostedBossId() const;

src/items/bed.cpp

@@ -169,7 +169,7 @@ bool BedItem::sleep(std::shared_ptr<Player> player) {
 	g_game().setBedSleeper(static_self_cast<BedItem>(), player->getGUID());
 
 	// make the player walk onto the bed
-	g_dispatcher().addWalkEvent([=] {
+	g_dispatcher().addWalkEvent([player, this] {
 		g_game().map.moveCreature(player, getTile());
 	});
 

src/items/functions/item/item_parse.cpp

@@ -1004,19 +1004,19 @@ void ItemParse::parseReflectDamage(const std::string &tmpStrValue, pugi::xml_att
 	}
 }
 
-void ItemParse::parseTransformOnUse(const std::string_view &tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType) {
+void ItemParse::parseTransformOnUse(std::string_view tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType) {
 	if (tmpStrValue == "transformonuse") {
 		itemType.m_transformOnUse = pugi::cast<uint16_t>(valueAttribute.value());
 	}
 }
 
-void ItemParse::parsePrimaryType(const std::string_view &tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType) {
+void ItemParse::parsePrimaryType(std::string_view tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType) {
 	if (tmpStrValue == "primarytype") {
 		itemType.m_primaryType = asLowerCaseString(valueAttribute.as_string());
 	}
 }
 
-void ItemParse::parseHouseRelated(const std::string_view &tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType) {
+void ItemParse::parseHouseRelated(std::string_view tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType) {
 	if (tmpStrValue == "usedbyhouseguests") {
 		g_logger().debug("[{}] item {}, used by guests {}", __FUNCTION__, itemType.id, valueAttribute.as_bool());
 		itemType.m_canBeUsedByGuests = valueAttribute.as_bool();
@@ -1260,7 +1260,7 @@ void ItemParse::createAndRegisterScript(ItemType &itemType, pugi::xml_node attri
 	}
 }
 
-void ItemParse::parseUnscriptedItems(const std::string_view &tmpStrValue, pugi::xml_node attributeNode, pugi::xml_attribute valueAttribute, ItemType &itemType) {
+void ItemParse::parseUnscriptedItems(std::string_view tmpStrValue, pugi::xml_node attributeNode, pugi::xml_attribute valueAttribute, ItemType &itemType) {
 	if (tmpStrValue == "script") {
 		std::string scriptName = valueAttribute.as_string();
 		auto tokens = split(scriptName.data(), ';');

src/items/functions/item/item_parse.hpp

@@ -320,10 +320,10 @@ class ItemParse : public Items {
 	static void parsePerfecShot(const std::string &tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
 	static void parseCleavePercent(const std::string &tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
 	static void parseReflectDamage(const std::string &tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
-	static void parseTransformOnUse(const std::string_view &tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
-	static void parsePrimaryType(const std::string_view &tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
-	static void parseHouseRelated(const std::string_view &tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
-	static void parseUnscriptedItems(const std::string_view &tmpStrValue, pugi::xml_node attributeNode, pugi::xml_attribute valueAttribute, ItemType &itemType);
+	static void parseTransformOnUse(std::string_view tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
+	static void parsePrimaryType(std::string_view tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
+	static void parseHouseRelated(std::string_view tmpStrValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
+	static void parseUnscriptedItems(std::string_view tmpStrValue, pugi::xml_node attributeNode, pugi::xml_attribute valueAttribute, ItemType &itemType);
 
 private:
 	// Parent of the function: static void parseField

src/items/items_definitions.hpp

@@ -33,7 +33,7 @@ enum Attr_ReadValue {
 	ATTR_READ_END,
 };
 
-enum ReturnValue {
+enum ReturnValue : uint16_t {
 	RETURNVALUE_NOERROR,
 	RETURNVALUE_NOTBOUGHTINSTORE,
 	RETURNVALUE_ITEMCANNOTBEMOVEDTHERE,

src/lib/metrics/metrics.cpp

@@ -87,7 +87,7 @@ void Metrics::shutdown() {
 	metrics_api::Provider::SetMeterProvider(none);
 }
 
-ScopedLatency::ScopedLatency(const std::string_view &name, const std::string &histogramName, const std::string &scopeKey) :
+ScopedLatency::ScopedLatency(std::string_view name, const std::string &histogramName, const std::string &scopeKey) :
 	ScopedLatency(name, g_metrics().latencyHistograms[histogramName], { { scopeKey, std::string(name) } }, g_metrics().defaultContext) {
 	if (histogram == nullptr) {
 		stopped = true;

src/lib/metrics/metrics.hpp

@@ -54,8 +54,8 @@ namespace metrics {
 
 	class ScopedLatency {
 	public:
-		explicit ScopedLatency(const std::string_view &name, const std::string &histogramName, const std::string &scopeKey);
-		explicit ScopedLatency([[maybe_unused]] const std::string_view &name, Histogram<double> &histogram, std::map<std::string, std::string> attrs = {}, opentelemetry::context::Context context = opentelemetry::context::Context()) :
+		explicit ScopedLatency(std::string_view name, const std::string &histogramName, const std::string &scopeKey);
+		explicit ScopedLatency([[maybe_unused]] std::string_view name, Histogram<double> &histogram, std::map<std::string, std::string> attrs = {}, opentelemetry::context::Context context = opentelemetry::context::Context()) :
 			begin(std::chrono::steady_clock::now()), histogram(histogram), attrs(std::move(attrs)), context(std::move(context)) {
 		}
 

src/lua/creature/talkaction.cpp

@@ -23,7 +23,7 @@ bool TalkActions::registerLuaEvent(const TalkAction_ptr &talkAction) {
 	return inserted;
 }
 
-bool TalkActions::checkWord(std::shared_ptr<Player> player, SpeakClasses type, const std::string &words, const std::string_view &word, const TalkAction_ptr &talkActionPtr) const {
+bool TalkActions::checkWord(std::shared_ptr<Player> player, SpeakClasses type, const std::string &words, std::string_view word, const TalkAction_ptr &talkActionPtr) const {
 	auto spacePos = std::ranges::find_if(words.begin(), words.end(), ::isspace);
 	std::string firstWord = words.substr(0, spacePos - words.begin());
 

src/lua/creature/talkaction.hpp

@@ -82,7 +82,7 @@ class TalkActions final : public Scripts {
 		return inject<TalkActions>();
 	}
 
-	bool checkWord(std::shared_ptr<Player> player, SpeakClasses type, const std::string &words, const std::string_view &word, const TalkAction_ptr &talkActionPtr) const;
+	bool checkWord(std::shared_ptr<Player> player, SpeakClasses type, const std::string &words, std::string_view word, const TalkAction_ptr &talkActionPtr) const;
 	TalkActionResult_t checkPlayerCanSayTalkAction(std::shared_ptr<Player> player, SpeakClasses type, const std::string &words) const;
 
 	bool registerLuaEvent(const TalkAction_ptr &talkAction);

src/lua/lua_definitions.hpp

@@ -74,7 +74,7 @@ enum CreatureEventType_t {
 	CREATURE_EVENT_EXTENDED_OPCODE,
 };
 
-enum MoveEvent_t {
+enum MoveEvent_t : uint8_t {
 	MOVE_EVENT_STEP_IN,
 	MOVE_EVENT_STEP_OUT,
 	MOVE_EVENT_EQUIP,

src/utils/tools.cpp

@@ -7,9 +7,11 @@
  * Website: https://docs.opentibiabr.com/
  */
 
+#include "utils/tools.hpp"
+
 #include "core.hpp"
 #include "items/item.hpp"
-#include "utils/tools.hpp"
+#include "enums/object_category.hpp"
 
 void printXMLError(const std::string &where, const std::string &fileName, const pugi::xml_parse_result &result) {
 	g_logger().error("[{}] Failed to load {}: {}", where, fileName, result.description());
@@ -185,7 +187,7 @@ std::string transformToSHA1(const std::string &input) {
 	return std::string(hexstring, 40);
 }
 
-uint16_t getStashSize(StashItemList itemList) {
+uint16_t getStashSize(const std::map<uint16_t, uint32_t> &itemList) {
 	uint16_t size = 0;
 	for (auto item : itemList) {
 		size += ceil(item.second / (float_t)Item::items[item.first].stackSize);
@@ -1906,6 +1908,39 @@ unsigned int getNumberOfCores() {
 	return cores;
 }
 
+Cipbia_Elementals_t getCipbiaElement(CombatType_t combatType) {
+	switch (combatType) {
+		case COMBAT_PHYSICALDAMAGE:
+			return CIPBIA_ELEMENTAL_PHYSICAL;
+		case COMBAT_ENERGYDAMAGE:
+			return CIPBIA_ELEMENTAL_ENERGY;
+		case COMBAT_EARTHDAMAGE:
+			return CIPBIA_ELEMENTAL_EARTH;
+		case COMBAT_FIREDAMAGE:
+			return CIPBIA_ELEMENTAL_FIRE;
+		case COMBAT_LIFEDRAIN:
+			return CIPBIA_ELEMENTAL_LIFEDRAIN;
+		case COMBAT_HEALING:
+			return CIPBIA_ELEMENTAL_HEALING;
+		case COMBAT_DROWNDAMAGE:
+			return CIPBIA_ELEMENTAL_DROWN;
+		case COMBAT_ICEDAMAGE:
+			return CIPBIA_ELEMENTAL_ICE;
+		case COMBAT_HOLYDAMAGE:
+			return CIPBIA_ELEMENTAL_HOLY;
+		case COMBAT_DEATHDAMAGE:
+			return CIPBIA_ELEMENTAL_DEATH;
+		case COMBAT_MANADRAIN:
+			return CIPBIA_ELEMENTAL_MANADRAIN;
+		case COMBAT_AGONYDAMAGE:
+			return CIPBIA_ELEMENTAL_AGONY;
+		case COMBAT_NEUTRALDAMAGE:
+			return CIPBIA_ELEMENTAL_AGONY;
+		default:
+			return CIPBIA_ELEMENTAL_UNDEFINED;
+	}
+}
+
 /**
  * @brief Formats a number to a string with commas
  * @param number The number to format

src/utils/tools.hpp

@@ -9,16 +9,33 @@
 
 #pragma once
 
-#include "utils/utils_definitions.hpp"
-#include "declarations.hpp"
-#include "enums/item_attribute.hpp"
-#include "game/movement/position.hpp"
-#include "enums/object_category.hpp"
-
 namespace pugi {
 	class xml_parse_result;
 }
 
+struct Position;
+
+enum CombatType_t : uint8_t;
+enum Direction : uint8_t;
+enum MagicEffectClasses : uint16_t;
+enum ShootType_t : uint8_t;
+enum Ammo_t : uint8_t;
+enum WeaponAction_t : uint8_t;
+enum Skulls_t : uint8_t;
+enum ImbuementTypes_t : int64_t;
+enum SpawnType_t : uint8_t;
+enum WeaponType_t : uint8_t;
+enum MoveEvent_t : uint8_t;
+enum NameEval_t : uint8_t;
+enum BedItemPart_t : uint8_t;
+enum ObjectCategory_t : uint8_t;
+enum ItemAttribute_t : uint64_t;
+enum ReturnValue : uint16_t;
+enum SpellGroup_t : uint8_t;
+enum Cipbia_Elementals_t : uint8_t;
+enum PlayerPronoun_t : uint8_t;
+enum PlayerSex_t : uint8_t;
+
 #ifndef USE_PRECOMPILED_HEADERS
 	#include <random>
 #endif
@@ -27,7 +44,7 @@ void printXMLError(const std::string &where, const std::string &fileName, const
 
 std::string transformToSHA1(const std::string &input);
 
-uint16_t getStashSize(StashItemList itemList);
+uint16_t getStashSize(const std::map<uint16_t, uint32_t> &itemList);
 
 std::string generateToken(const std::string &secret, uint32_t ticks);
 
@@ -160,38 +177,7 @@ std::string getFormattedTimeRemaining(uint32_t time);
 
 unsigned int getNumberOfCores();
 
-static inline Cipbia_Elementals_t getCipbiaElement(CombatType_t combatType) {
-	switch (combatType) {
-		case COMBAT_PHYSICALDAMAGE:
-			return CIPBIA_ELEMENTAL_PHYSICAL;
-		case COMBAT_ENERGYDAMAGE:
-			return CIPBIA_ELEMENTAL_ENERGY;
-		case COMBAT_EARTHDAMAGE:
-			return CIPBIA_ELEMENTAL_EARTH;
-		case COMBAT_FIREDAMAGE:
-			return CIPBIA_ELEMENTAL_FIRE;
-		case COMBAT_LIFEDRAIN:
-			return CIPBIA_ELEMENTAL_LIFEDRAIN;
-		case COMBAT_HEALING:
-			return CIPBIA_ELEMENTAL_HEALING;
-		case COMBAT_DROWNDAMAGE:
-			return CIPBIA_ELEMENTAL_DROWN;
-		case COMBAT_ICEDAMAGE:
-			return CIPBIA_ELEMENTAL_ICE;
-		case COMBAT_HOLYDAMAGE:
-			return CIPBIA_ELEMENTAL_HOLY;
-		case COMBAT_DEATHDAMAGE:
-			return CIPBIA_ELEMENTAL_DEATH;
-		case COMBAT_MANADRAIN:
-			return CIPBIA_ELEMENTAL_MANADRAIN;
-		case COMBAT_AGONYDAMAGE:
-			return CIPBIA_ELEMENTAL_AGONY;
-		case COMBAT_NEUTRALDAMAGE:
-			return CIPBIA_ELEMENTAL_AGONY;
-		default:
-			return CIPBIA_ELEMENTAL_UNDEFINED;
-	}
-}
+Cipbia_Elementals_t getCipbiaElement(CombatType_t combatType);
 
 std::string formatNumber(uint64_t number);
 

src/utils/utils_definitions.hpp

@@ -17,7 +17,7 @@ enum WieldInfo_t {
 	WIELDINFO_PREMIUM = 1 << 3,
 };
 
-enum SpawnType_t {
+enum SpawnType_t : uint8_t {
 	RESPAWN_IN_ALL = 0,
 	RESPAWN_IN_DAY = 1,
 	RESPAWN_IN_NIGHT = 2,

tests/unit/utils/position_functions_test.cpp

@@ -2,6 +2,7 @@
 
 #include <boost/ut.hpp>
 
+#include "game/movement/position.hpp"
 #include "utils/tools.hpp"
 
 using namespace boost::ut;