improve: CombatType enum handling with magic enum (#1279)

We propose the use of the Magic Enum library to simplify the handling of the CombatType_t enumeration values. This refactoring removes the need for manual conversion functions, replacing them with Magic Enum library functions, increasing the robustness and maintainability of the code. The specific changes include: • Refactoring of combatTypeToIndex function to use magic_enum::enum_index(), allowing automatic obtaining of the index for each enumeration value. • Refactoring of combatTypeToName function to use magic_enum::enum_name(), to automatically obtain the name for each enumeration value. • Addition of a new formatEnumName function to format enumeration names (replacing '_' with '' and converting to lowercase) before returning them. These changes should improve code clarity and reduce the likelihood of errors when handling CombatType_t values. This will prevent, for example, having changes (additions or removals) in the CombatType enum and forgetting to add/remove these functions, which would result in a log in the distro (false positive). Getting the information directly from the enum makes the code much easier to manage and maintain.
opentibiabr · Aug 22, 2023 · d9eaf2c · d9eaf2c
1 parent 0b675d8
commit d9eaf2c
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 81 deletions.
diff --git a/src/creatures/players/imbuements/imbuements.cpp b/src/creatures/players/imbuements/imbuements.cpp
@@ -251,7 +251,7 @@ bool Imbuements::loadFromXml(bool /* reloading */) {
 							continue;
 						}
 
-						CombatType_t combatType = getCombatType(attr.as_string());
+						CombatType_t combatType = getCombatTypeByName(attr.as_string());
 						if (combatType == COMBAT_NONE) {
 							g_logger().warn("Unknown combat type for element {}", attr.as_string());
 							continue;
@@ -272,7 +272,7 @@ bool Imbuements::loadFromXml(bool /* reloading */) {
 							continue;
 						}
 
-						CombatType_t combatType = getCombatType(attr.as_string());
+						CombatType_t combatType = getCombatTypeByName(attr.as_string());
 						if (combatType == COMBAT_NONE) {
 							g_logger().warn("Unknown combat type for element {}", attr.as_string());
 							continue;

diff --git a/src/creatures/players/player.h b/src/creatures/players/player.h
@@ -2642,9 +2642,11 @@ class Player final : public Creature, public Cylinder, public Bankable {
 		uint32_t inventoryWeight = 0;
 		uint32_t capacity = 40000;
 		uint32_t bonusCapacity = 0;
+
 		std::bitset<CombatType_t::COMBAT_COUNT> m_damageImmunities;
 		std::bitset<ConditionType_t::CONDITION_COUNT> m_conditionImmunities;
 		std::bitset<ConditionType_t::CONDITION_COUNT> m_conditionSuppressions;
+
 		uint32_t level = 1;
 		uint32_t magLevel = 0;
 		uint32_t actionTaskEvent = 0;
@@ -2814,8 +2816,10 @@ class Player final : public Creature, public Cylinder, public Bankable {
 		uint64_t getLostExperience() const override {
 			return skillLoss ? static_cast<uint64_t>(experience * getLostPercent()) : 0;
 		}
+
 		bool isSuppress(ConditionType_t conditionType) const override;
 		void addConditionSuppression(const std::array<ConditionType_t, ConditionType_t::CONDITION_COUNT> &addConditions);
+
 		uint16_t getLookCorpse() const override;
 		void getPathSearchParams(const Creature* creature, FindPathParams &fpp) const override;
 

diff --git a/src/utils/tools.cpp b/src/utils/tools.cpp
@@ -444,6 +444,13 @@ std::string formatTime(time_t time) {
 	return {};
 }
 
+std::string formatEnumName(std::string_view name) {
+	std::string result { name.begin(), name.end() };
+	std::replace(result.begin(), result.end(), '_', ' ');
+	std::transform(result.begin(), result.end(), result.begin(), [](unsigned char c) { return std::tolower(c); });
+	return result;
+}
+
 std::time_t getTimeNow() {
 	return std::chrono::system_clock::to_time_t(std::chrono::system_clock::now());
 }
@@ -768,6 +775,7 @@ CombatTypeNames combatTypeNames = {
 	{ COMBAT_LIFEDRAIN, "lifedrain" },
 	{ COMBAT_MANADRAIN, "manadrain" },
 	{ COMBAT_PHYSICALDAMAGE, "physical" },
+	{ COMBAT_NEUTRALDAMAGE, "neutral" },
 };
 
 AmmoTypeNames ammoTypeNames = {
@@ -864,22 +872,6 @@ ShootType_t getShootType(const std::string &strValue) {
 	return CONST_ANI_NONE;
 }
 
-std::string getCombatName(CombatType_t combatType) {
-	auto combatName = combatTypeNames.find(combatType);
-	if (combatName != combatTypeNames.end()) {
-		return combatName->second;
-	}
-	return "unknown";
-}
-
-CombatType_t getCombatType(const std::string &combatname) {
-	auto it = std::find_if(combatTypeNames.begin(), combatTypeNames.end(), [combatname](const std::pair<CombatType_t, std::string> &pair) {
-		return pair.second == combatname;
-	});
-
-	return it != combatTypeNames.end() ? it->first : COMBAT_NONE;
-}
-
 Ammo_t getAmmoType(const std::string &strValue) {
 	auto ammoType = ammoTypeNames.find(strValue);
 	if (ammoType != ammoTypeNames.end()) {
@@ -1057,73 +1049,43 @@ std::string getWeaponName(WeaponType_t weaponType) {
 	}
 }
 
+std::string getCombatName(CombatType_t combatType) {
+	auto combatName = combatTypeNames.find(combatType);
+	if (combatName != combatTypeNames.end()) {
+		return combatName->second;
+	}
+	return "unknown";
+}
+
+CombatType_t getCombatTypeByName(const std::string &combatname) {
+	auto it = std::find_if(combatTypeNames.begin(), combatTypeNames.end(), [combatname](const std::pair<CombatType_t, std::string> &pair) {
+		return pair.second == combatname;
+	});
+
+	return it != combatTypeNames.end() ? it->first : COMBAT_NONE;
+}
+
 size_t combatTypeToIndex(CombatType_t combatType) {
-	switch (combatType) {
-		case COMBAT_PHYSICALDAMAGE:
-			return 0;
-		case COMBAT_ENERGYDAMAGE:
-			return 1;
-		case COMBAT_EARTHDAMAGE:
-			return 2;
-		case COMBAT_FIREDAMAGE:
-			return 3;
-		case COMBAT_UNDEFINEDDAMAGE:
-			return 4;
-		case COMBAT_LIFEDRAIN:
-			return 5;
-		case COMBAT_MANADRAIN:
-			return 6;
-		case COMBAT_HEALING:
-			return 7;
-		case COMBAT_DROWNDAMAGE:
-			return 8;
-		case COMBAT_ICEDAMAGE:
-			return 9;
-		case COMBAT_HOLYDAMAGE:
-			return 10;
-		case COMBAT_DEATHDAMAGE:
-			return 11;
-		case COMBAT_NEUTRALDAMAGE:
-			return 12;
-		default:
-			g_logger().error("Combat type {} is out of range", fmt::underlying(combatType));
-			// Uncomment for catch the function call with debug
-			// throw std::out_of_range("Combat is out of range");
+	auto enum_index_opt = magic_enum::enum_index(combatType);
+	if (enum_index_opt.has_value() && enum_index_opt.value() < COMBAT_COUNT) {
+		return enum_index_opt.value();
+	} else {
+		g_logger().error("[{}] Combat type {} is out of range", __FUNCTION__, fmt::underlying(combatType));
+		// Uncomment for catch the function call with debug
+		// throw std::out_of_range("Combat is out of range");
 	}
 
-	return {};
+	return COMBAT_NONE;
 }
 
 std::string combatTypeToName(CombatType_t combatType) {
-	switch (combatType) {
-		case COMBAT_PHYSICALDAMAGE:
-			return "physical";
-		case COMBAT_ENERGYDAMAGE:
-			return "energy";
-		case COMBAT_EARTHDAMAGE:
-			return "earth";
-		case COMBAT_FIREDAMAGE:
-			return "fire";
-		case COMBAT_UNDEFINEDDAMAGE:
-			return "undefined";
-		case COMBAT_LIFEDRAIN:
-			return "life drain";
-		case COMBAT_MANADRAIN:
-			return "mana drain";
-		case COMBAT_HEALING:
-			return "healing";
-		case COMBAT_DROWNDAMAGE:
-			return "drown";
-		case COMBAT_ICEDAMAGE:
-			return "ice";
-		case COMBAT_HOLYDAMAGE:
-			return "holy";
-		case COMBAT_DEATHDAMAGE:
-			return "death";
-		default:
-			g_logger().error("Combat type {} is out of range", fmt::underlying(combatType));
-			// Uncomment for catch the function call with debug
-			// throw std::out_of_range("Combat is out of range");
+	std::string_view name = magic_enum::enum_name(combatType);
+	if (!name.empty() && combatType < COMBAT_COUNT) {
+		return formatEnumName(name);
+	} else {
+		g_logger().error("[{}] Combat type {} is out of range", __FUNCTION__, fmt::underlying(combatType));
+		// Uncomment for catch the function call with debug
+		// throw std::out_of_range("Index is out of range");
 	}
 
 	return {};

diff --git a/src/utils/tools.h b/src/utils/tools.h
@@ -63,6 +63,12 @@ std::string getFirstLine(const std::string &str);
 std::string formatDate(time_t time);
 std::string formatDateShort(time_t time);
 std::string formatTime(time_t time);
+/**
+ * @brief Format the enum name by replacing underscores with spaces and converting to lowercase.
+ * @param name The enum name to format.
+ * @return A string with the formatted enum name.
+ */
+std::string formatEnumName(std::string_view name);
 std::time_t getTimeNow();
 std::time_t getTimeMsNow();
 std::string convertIPToString(uint32_t ip);
@@ -75,13 +81,11 @@ Ammo_t getAmmoType(const std::string &strValue);
 WeaponAction_t getWeaponAction(const std::string &strValue);
 Skulls_t getSkullType(const std::string &strValue);
 ImbuementTypes_t getImbuementType(const std::string &strValue);
-std::string getCombatName(CombatType_t combatType);
 /**
  * @Deprecated
  * It will be dropped with monsters. Use RespawnPeriod_t instead.
  */
 SpawnType_t getSpawnType(const std::string &strValue);
-CombatType_t getCombatType(const std::string &combatname);
 
 std::string getSkillName(uint8_t skillid);
 
@@ -93,8 +97,25 @@ bool booleanString(const std::string &str);
 
 std::string getWeaponName(WeaponType_t weaponType);
 
+std::string getCombatName(CombatType_t combatType);
+CombatType_t getCombatTypeByName(const std::string &combatname);
+
+/**
+ * @brief Convert the CombatType_t enumeration to its corresponding index.
+ * @param combatType The CombatType_t enumeration to convert.
+ * @return The corresponding index of the CombatType_t enumeration.
+ * If the CombatType_t is out of range, this function will log an error and return an empty size_t.
+ */
 size_t combatTypeToIndex(CombatType_t combatType);
+
+/**
+ * @brief Convert the CombatType_t enumeration to its corresponding string representation.
+ * @param combatType The CombatType_t enumeration to convert.
+ * @return The corresponding string representation of the CombatType_t enumeration.
+ * If the CombatType_t is out of range, this function will log an error and return an empty string.
+ */
 std::string combatTypeToName(CombatType_t combatType);
+
 CombatType_t indexToCombatType(size_t v);
 
 ItemAttribute_t stringToItemAttribute(const std::string &str);
@@ -147,6 +168,8 @@ static inline Cipbia_Elementals_t getCipbiaElement(CombatType_t combatType) {
 			return CIPBIA_ELEMENTAL_DEATH;
 		case COMBAT_MANADRAIN:
 			return CIPBIA_ELEMENTAL_MANADRAIN;
+		case COMBAT_NEUTRALDAMAGE:
+			return CIPBIA_ELEMENTAL_NEUTRAL;
 		default:
 			return CIPBIA_ELEMENTAL_UNDEFINED;
 	}

diff --git a/src/utils/utils_definitions.hpp b/src/utils/utils_definitions.hpp
@@ -94,6 +94,7 @@ enum Cipbia_Elementals_t : uint8_t {
 	CIPBIA_ELEMENTAL_LIFEDRAIN = 9,
 	CIPBIA_ELEMENTAL_MANADRAIN = 10,
 	CIPBIA_ELEMENTAL_UNDEFINED = 11,
+	CIPBIA_ELEMENTAL_NEUTRAL = 12,
 };
 
 enum MagicEffectClasses : uint16_t {