Fixed imbuement system behavior and a crash related to memory referen…

…ce (#345) Fixed: - Item only gets inmediate bonus when imbued on equipment slots - Clearing the imbuement will make you lose it if the item is equipped - You should be able to use bank balance for imbuements - Elemental damage shouldn't be affected - Critical hit chance should not get higher than 10% if the item already has critical hit chance - Falcon Coif should be able to get Intricate Epiphany and Dark Whispers only Basic Epiphany - Item description should show crit chance without positive sign and "critical extra damage" - Move map deletion to outside the loop to avoid memory reference crash on Game::checkImbuements function Enhancement: Where the "imbuementslot" value is from 1 to 3 and the imbuement name values are also from 1 to 3, which is the imbuement level (basic, intricate or powerful), the name must be placed as above, as it is from the name that the value of each imbuement is called by ItemNode.
opentibiabr · Dec 21, 2021 · 6f4fddb · 6f4fddb
1 parent 79955d9
commit 6f4fddb
Show file tree

Hide file tree

Showing 13 changed files with 2,026 additions and 1,949 deletions.
diff --git a/data/items/items.xml b/data/items/items.xml
diff --git a/src/creatures/combat/combat.cpp b/src/creatures/combat/combat.cpp
@@ -535,6 +535,10 @@ CombatDamage Combat::applyImbuementElementalDamage(Item* item, CombatDamage dama
 			continue;
 		}
 
+		if (imbuementInfo.imbuement->combatType == COMBAT_NONE) {
+			continue;
+		}
+
 		float damagePercent = imbuementInfo.imbuement->elementDamage / 100.0;
 
 		damage.secondary.type = imbuementInfo.imbuement->combatType;

diff --git a/src/creatures/players/imbuements/imbuements.cpp b/src/creatures/players/imbuements/imbuements.cpp
@@ -375,7 +375,7 @@ std::vector<Imbuement*> Imbuements::getImbuements(const Player* player, Item* it
 
 		// Send only the imbuements registered on item (in items.xml) to the imbuement window
 		const CategoryImbuement* categoryImbuement = getCategoryByID(imbuement->getCategory());
-		if (!item->hasImbuementType(static_cast<ImbuementTypes_t>(categoryImbuement->id))) {
+		if (!item->hasImbuementType(static_cast<ImbuementTypes_t>(categoryImbuement->id), imbuement->getBaseID())) {
 			continue;
 		}
 

diff --git a/src/creatures/players/player.cpp b/src/creatures/players/player.cpp
@@ -1239,7 +1239,7 @@ void Player::onApplyImbuement(Imbuement *imbuement, Item *item, uint8_t slot, bo
 	uint32_t price = baseImbuement->price;
 	price += protectionCharm ? baseImbuement->protectionPrice : 0;
 
-	if (!g_game.removeMoney(this, price, 0, false))
+	if (!g_game.removeMoney(this, price, 0, true))
 	{
 		std::string message = "You don't have " + std::to_string(price) + " gold coins.";
 
@@ -1274,9 +1274,11 @@ void Player::onApplyImbuement(Imbuement *imbuement, Item *item, uint8_t slot, bo
 		return;
 	}
 
-	item->setImbuement(slot, imbuement->getID(), baseImbuement->duration);
+	if (item->getParent() == this) {
+		addItemImbuementStats(imbuement);
+	}
 
-	addItemImbuementStats(imbuement);
+	item->setImbuement(slot, imbuement->getID(), baseImbuement->duration);
 	openImbuementWindow(item);
 }
 
@@ -1299,7 +1301,7 @@ void Player::onClearImbuement(Item* item, uint8_t slot)
 		return;
 	}
 
-	if (!g_game.removeMoney(this, baseImbuement->removeCost, 0, false))
+	if (!g_game.removeMoney(this, baseImbuement->removeCost, 0, true))
 	{
 		std::string message = "You don't have " + std::to_string(baseImbuement->removeCost) + " gold coins.";
 
@@ -1309,6 +1311,10 @@ void Player::onClearImbuement(Item* item, uint8_t slot)
 		return;
 	}
 
+	if (item->getParent() == this) {
+		removeItemImbuementStats(imbuementInfo.imbuement);
+	}
+
 	item->setImbuement(slot, imbuementInfo.imbuement->getID(), 0);
 	this->openImbuementWindow(item);
 }
@@ -5439,11 +5445,6 @@ void Player::addItemImbuementStats(const Imbuement* imbuement)
 		}
 	}
 
-	if (requestUpdate) {
-		sendSkills();
-		requestUpdate = false;
-	}
-
 	// Check imbuement magic level
 	for (int32_t s = STAT_FIRST; s <= STAT_LAST; ++s) {
 		if (imbuement->stats[s]) {
@@ -5483,11 +5484,6 @@ void Player::removeItemImbuementStats(const Imbuement* imbuement)
 		}
 	}
 
-	if (requestUpdate) {
-		sendSkills();
-		requestUpdate = false;
-	}
-
 	// Check imbuement magic level
 	for (int32_t s = STAT_FIRST; s <= STAT_LAST; ++s) {
 		if (imbuement->stats[s]) {

diff --git a/src/creatures/players/player.h b/src/creatures/players/player.h
@@ -857,11 +857,14 @@ class Player final : public Creature, public Cylinder
 		}
 
 		uint16_t getSkillLevel(uint8_t skill) const {
-			if (skill == SKILL_LIFE_LEECH_CHANCE || skill == SKILL_MANA_LEECH_CHANCE) {
-				return std::min<uint16_t>(100, std::max<uint16_t>(0, skills[skill].level + varSkills[skill]));
+			uint16_t skillLevel = std::max<uint16_t>(0, skills[skill].level + varSkills[skill]);
+
+			auto it = maxValuePerSkill.find(skill);
+			if (it != maxValuePerSkill.end()) {
+				skillLevel = std::min<uint16_t>(it->second, skillLevel);
 			}
 
-			return std::max<uint16_t>(0, skills[skill].level + varSkills[skill]);
+			return skillLevel;
 		}
 		uint16_t getBaseSkill(uint8_t skill) const {
 			return skills[skill].level;
@@ -2006,6 +2009,12 @@ class Player final : public Creature, public Cylinder
 		std::map<uint8_t, int64_t> moduleDelayMap;
 		std::map<uint32_t, int32_t> storageMap;
 
+		std::map<uint8_t, uint16_t> maxValuePerSkill = {
+			{SKILL_LIFE_LEECH_CHANCE, 100},
+			{SKILL_MANA_LEECH_CHANCE, 100},
+			{SKILL_CRITICAL_HIT_CHANCE, 10}
+		};
+
 		std::map<uint32_t, Reward*> rewardMap;
 
 		std::map<ObjectCategory_t, Container*> quickLootContainers;

diff --git a/src/game/game.cpp b/src/game/game.cpp
@@ -6496,15 +6496,21 @@ void Game::checkImbuements()
 {
 	g_scheduler.addEvent(createSchedulerTask(EVENT_IMBUEMENT_INTERVAL, std::bind(&Game::checkImbuements, this)));
 
+	std::vector<uint32_t> toErase;
+
 	for (const auto& [key, value] : playersActiveImbuements) {
 		Player* player = getPlayerByID(key);
 		if (!player) {
-			setPlayerActiveImbuements(key, 0);
+			toErase.push_back(key);
 			continue;
 		}
 
 		player->updateInventoryImbuement();
 	}
+
+	for (uint32_t playerId : toErase) {
+		setPlayerActiveImbuements(playerId, 0);
+	}
 }
 
 void Game::checkLight()

diff --git a/src/items/item.cpp b/src/items/item.cpp
@@ -1009,7 +1009,16 @@ std::vector<std::pair<std::string, std::string>>
 				}
 
 				ss.str("");
-				ss << std::showpos << it.abilities->skills[i] << '%' << std::noshowpos;
+
+				if (i != SKILL_CRITICAL_HIT_CHANCE) {
+					ss << std::showpos;
+				}
+
+				ss << it.abilities->skills[i] << '%';
+				if (i != SKILL_CRITICAL_HIT_CHANCE) {
+					ss << std::noshowpos;
+				}
+
 				descriptions.emplace_back(getSkillName(i), ss.str());
 			}
 
@@ -1257,7 +1266,15 @@ std::vector<std::pair<std::string, std::string>>
 				}
 
 				ss.str("");
-				ss << std::showpos << it.abilities->skills[i] << '%' << std::noshowpos;
+				if (i != SKILL_CRITICAL_HIT_CHANCE) {
+					ss << std::showpos;
+				}
+				ss << it.abilities->skills[i] << '%';
+
+				if (i != SKILL_CRITICAL_HIT_CHANCE) {
+					ss << std::noshowpos;
+				}
+
 				descriptions.emplace_back(getSkillName(i), ss.str());
 			}
 
@@ -1574,7 +1591,15 @@ std::string Item::getDescription(const ItemType& it, int32_t lookDistance,
 					else {
 						s << ", ";
 					}
-					s << getSkillName(i) << ' ' << std::showpos << it.abilities->skills[i] << std::noshowpos;
+					s << getSkillName(i) << ' ';
+					if (i != SKILL_CRITICAL_HIT_CHANCE) {
+						s << std::showpos;
+					}
+					s << it.abilities->skills[i];
+
+					if (i != SKILL_CRITICAL_HIT_CHANCE) {
+						s << std::noshowpos;
+					}
 				}
 
 				if (it.abilities->stats[STAT_MAGICPOINTS]) {
@@ -1760,7 +1785,16 @@ std::string Item::getDescription(const ItemType& it, int32_t lookDistance,
 					else {
 						s << ", ";
 					}
-					s << getSkillName(i) << ' ' << std::showpos << it.abilities->skills[i] << std::noshowpos << '%';
+					s << getSkillName(i) << ' ';
+					if (i != SKILL_CRITICAL_HIT_CHANCE) {
+						s << std::showpos;
+					}
+					s << it.abilities->skills[i];
+
+					if (i != SKILL_CRITICAL_HIT_CHANCE) {
+						s << std::noshowpos;
+					}
+					s << '%';
 				}
 
 				if (it.abilities->stats[STAT_MAGICPOINTS]) {
@@ -1920,7 +1954,16 @@ std::string Item::getDescription(const ItemType& it, int32_t lookDistance,
 					s << ", ";
 				}
 
-				s << getSkillName(i) << ' ' << std::showpos << it.abilities->skills[i] << std::noshowpos << '%';
+				s << getSkillName(i) << ' ';
+				if (i != SKILL_CRITICAL_HIT_CHANCE) {
+					s << std::showpos;
+				}
+				s << it.abilities->skills[i];
+
+				if (i != SKILL_CRITICAL_HIT_CHANCE) {
+					s << std::noshowpos;
+				}
+				s << '%';
 			}
 
 			if (it.abilities->stats[STAT_MAGICPOINTS]) {

diff --git a/src/items/item.h b/src/items/item.h
@@ -1144,8 +1144,12 @@ class Item : virtual public Thing
 		 */
 		bool getImbuementInfo(uint8_t slot, ImbuementInfo *imbuementInfo);
 		void setImbuement(uint8_t slot, uint16_t id, int32_t duration);
-		bool hasImbuementType(ImbuementTypes_t imbuementType) {
-			return items[id].imbuementTypes.find(imbuementType) != items[id].imbuementTypes.end();
+		bool hasImbuementType(ImbuementTypes_t imbuementType, uint16_t imbuementTier) {
+			auto it = items[id].imbuementTypes.find(imbuementType);
+			if (it != items[id].imbuementTypes.end()) {
+				return (it->second >= imbuementTier);
+			}
+			return false;
 		}
 		bool hasImbuementCategoryId(uint16_t categoryId);
 		bool hasImbuements() {

diff --git a/src/items/items.cpp b/src/items/items.cpp
@@ -1011,12 +1011,16 @@ void Items::parseItemNode(const pugi::xml_node& itemNode, uint16_t id)
 				if (!subKeyAttribute) {
 					continue;
 				}
+				pugi::xml_attribute subValueAttribute = subAttributeNode.attribute("value");
+				if (!subValueAttribute) {
+					continue;
+				}
 
 				auto itemMap = ImbuementsTypeMap.find(asLowerCaseString(subKeyAttribute.as_string()));
 				if (itemMap != ImbuementsTypeMap.end()) {
 					ImbuementTypes_t imbuementType = getImbuementType(asLowerCaseString(subKeyAttribute.as_string()));
 					if (imbuementType != IMBUEMENT_NONE) {
-						it.setImbuementType(imbuementType);
+						it.setImbuementType(imbuementType, pugi::cast<uint16_t>(subValueAttribute.value()));
 					}
 				}
 				else

diff --git a/src/items/items.h b/src/items/items.h
@@ -273,8 +273,8 @@ class ItemType
 			return str;
 		}
 
-		void setImbuementType(ImbuementTypes_t imbuementType) {
-			imbuementTypes[imbuementType] = true;
+		void setImbuementType(ImbuementTypes_t imbuementType, uint16_t slotMaxTier) {
+			imbuementTypes[imbuementType] = std::min<uint16_t>(IMBUEMENT_MAX_TIER, slotMaxTier);
 		}
 
 		itemgroup_t group = ITEM_GROUP_NONE;
@@ -335,7 +335,7 @@ class ItemType
 		ShootType_t shootType = CONST_ANI_NONE;
 		RaceType_t corpseType = RACE_NONE;
 		FluidTypes_t fluidSource = FLUID_NONE;
-		std::map<ImbuementTypes_t, bool> imbuementTypes;
+		std::map<ImbuementTypes_t, uint16_t> imbuementTypes;
 
 		uint8_t floorChange = 0;
 		uint8_t alwaysOnTopOrder = 0;

diff --git a/src/server/network/protocol/protocolgame.cpp b/src/server/network/protocol/protocolgame.cpp
@@ -4431,7 +4431,15 @@ void ProtocolGame::sendMarketDetail(uint16_t itemId)
 				separator = true;
 			}
 
-			ss << getSkillName(i) << ' ' << std::showpos << it.abilities->skills[i] << std::noshowpos << '%';
+			ss << getSkillName(i) << ' ';
+			if (i != SKILL_CRITICAL_HIT_CHANCE) {
+				ss << std::showpos;
+			}
+			ss << it.abilities->skills[i] << '%';
+
+			if (i != SKILL_CRITICAL_HIT_CHANCE) {
+				ss << std::noshowpos;
+			}
 		}
 
 		if (it.abilities->stats[STAT_MAGICPOINTS] != 0)

diff --git a/src/utils/const.h b/src/utils/const.h
@@ -24,6 +24,7 @@ static constexpr size_t NETWORKMESSAGE_PLAYERNAME_MAXLENGTH = 30;
 static constexpr int32_t NETWORKMESSAGE_MAXSIZE = 24590;
 // This is in miliseconds
 static constexpr int32_t EVENT_IMBUEMENT_INTERVAL = 1000;
+static constexpr uint8_t IMBUEMENT_MAX_TIER = 3;
 
 enum MagicEffectClasses : uint8_t {
 	CONST_ME_NONE,

diff --git a/src/utils/tools.cpp b/src/utils/tools.cpp
@@ -891,7 +891,7 @@ std::string getSkillName(uint8_t skillid)
 			return "critical hit chance";
 
 		case SKILL_CRITICAL_HIT_DAMAGE:
-			return "critical hit damage";
+			return "critical extra damage";
 
 		case SKILL_LIFE_LEECH_CHANCE:
 			return "life leech chance";