fix: imbuement decay time (#818)

Now the time decay logic of the imbuements is working as it should:
• Backpacks imbuements will only decay if equipped in the backpack slot;
• Normal item will only decay if in battle mode and out of protection zone, as well as in global.

Author: dudantas

src/creatures/players/imbuements/imbuements.cpp

@@ -51,7 +51,7 @@ bool Imbuements::loadFromXml(bool /* reloading */) {
 				pugi::cast<uint32_t>(baseNode.attribute("price").value()),
 				pugi::cast<uint32_t>(baseNode.attribute("protectionPrice").value()),
 				pugi::cast<uint32_t>(baseNode.attribute("removecost").value()),
-				pugi::cast<int32_t>(baseNode.attribute("duration").value()),
+				pugi::cast<uint32_t>(baseNode.attribute("duration").value()),
 				pugi::cast<uint16_t>(baseNode.attribute("percent").value())
 
 			);

src/creatures/players/imbuements/imbuements.h

@@ -20,15 +20,15 @@ class Item;
 class Imbuement;
 
 struct BaseImbuement {
-	BaseImbuement(uint16_t initId, std::string initName, uint32_t initProtectionPrice, uint32_t initPrice, uint32_t initRemoveCost, int32_t initDuration, uint8_t initPercent) :
+	BaseImbuement(uint16_t initId, std::string initName, uint32_t initProtectionPrice, uint32_t initPrice, uint32_t initRemoveCost, uint32_t initDuration, uint8_t initPercent) :
 		id(initId), name(std::move(initName)), protectionPrice(initProtectionPrice), price(initPrice), removeCost(initRemoveCost), duration(initDuration), percent(initPercent) {}
 
 	uint16_t id;
 	std::string name;
 	uint32_t protectionPrice;
 	uint32_t price;
 	uint32_t removeCost;
-	int32_t duration;
+	uint32_t duration;
 	uint8_t percent;
 };
 

src/creatures/players/player.cpp

@@ -462,45 +462,57 @@ void Player::updateInventoryWeight()
 	}
 }
 
-void Player::updateInventoryImbuement(bool init /* = false */)
+void Player::updateInventoryImbuement()
 {
+	// Get the tile the player is currently on
 	const Tile* playerTile = getTile();
+	// Check if the player is in a protection zone
 	bool isInProtectionZone = playerTile && playerTile->hasFlag(TILESTATE_PROTECTIONZONE);
+	// Check if the player is in fight mode
 	bool isInFightMode = hasCondition(CONDITION_INFIGHT);
-	
-	uint8_t imbuementsToCheck = g_game().getPlayerActiveImbuements(getID());
-	for (auto item : getAllInventoryItems()) {
+	// Iterate through all items in the player's inventory
+	for (auto item : getAllInventoryItems())
+	{
+		// Iterate through all imbuement slots on the item
 		for (uint8_t slotid = 0; slotid < item->getImbuementSlot(); slotid++)
 		{
 			ImbuementInfo imbuementInfo;
+			// Get the imbuement information for the current slot
 			if (!item->getImbuementInfo(slotid, &imbuementInfo))
 			{
-				continue;
+				// If no imbuement is found, continue to the next slot
+				break;
 			}
 
-			const CategoryImbuement* categoryImbuement = g_imbuements().getCategoryByID(imbuementInfo.imbuement->getCategory());
-			if (categoryImbuement && categoryImbuement->agressive)
-			{
-				if (isInProtectionZone || !isInFightMode)
-				{
-					break;
+			// Imbuement from imbuementInfo, this variable reduces code complexity
+			auto imbuement = imbuementInfo.imbuement;
+			// Get the category of the imbuement
+			const CategoryImbuement* categoryImbuement = g_imbuements().getCategoryByID(imbuement->getCategory());
+			// Parent of the imbued item
+			auto parent = item->getParent();
+			// If the imbuement is aggressive and the player is not in fight mode or is in a protection zone, or the item is in a container, ignore it.
+			if (categoryImbuement && categoryImbuement->agressive) {
+				if (!isInFightMode || isInProtectionZone || parent && parent->getContainer()) {
+					continue;
 				}
 			}
-
-			if (init)
-			{
-				g_game().increasePlayerActiveImbuements(getID());
+			// If the item is not in the backpack slot and it's not a agressive imbuement, ignore it.
+			if (categoryImbuement && !categoryImbuement->agressive && parent && parent != this) {
+				continue;
 			}
 
-			int32_t duration = std::max<int32_t>(0, imbuementInfo.duration - EVENT_IMBUEMENT_INTERVAL / 1000);
-			item->decayImbuementTime(slotid, imbuementInfo.imbuement->getID(), duration);
-			if (duration == 0)
+			// If the imbuement's duration is 0, remove its stats and continue to the next slot
+			if (imbuementInfo.duration == 0)
 			{
-				removeItemImbuementStats(imbuementInfo.imbuement);
-				g_game().decreasePlayerActiveImbuements(getID());
+				removeItemImbuementStats(imbuement);
+				continue;
 			}
 
-			imbuementsToCheck--;
+			SPDLOG_DEBUG("Decaying imbuement {} from item {} of player {}", imbuement->getName(), item->getName(), getName());
+			// Calculate the new duration of the imbuement, making sure it doesn't go below 0
+			uint64_t duration = std::max<uint64_t>(0, imbuementInfo.duration - EVENT_IMBUEMENT_INTERVAL / 1000);
+			// Update the imbuement's duration in the item
+			item->decayImbuementTime(slotid, imbuement->getID(), duration);
 		}
 	}
 }
@@ -1287,7 +1299,6 @@ void Player::onApplyImbuement(Imbuement *imbuement, Item *item, uint8_t slot, bo
 
 	item->addImbuement(slot, imbuement->getID(), baseImbuement->duration);
 	openImbuementWindow(item);
-	updateInventoryImbuement();
 }
 
 void Player::onClearImbuement(Item* item, uint8_t slot)

src/creatures/players/player.h

@@ -2198,7 +2198,7 @@ class Player final : public Creature, public Cylinder
 		 * @brief Starts checking the imbuements in the item so that the time decay is performed
 		 * Registers the player in an unordered_map in game.h so that the function can be initialized by the task
 		 */
-		void updateInventoryImbuement(bool init = false);
+		void updateInventoryImbuement();
 
 		void setNextWalkActionTask(SchedulerTask* task);
 		void setNextWalkTask(SchedulerTask* task);

src/game/game.cpp

@@ -6513,18 +6513,12 @@ void Game::checkImbuements()
 
 	std::vector<uint32_t> toErase;
 
-	for (const auto& [key, value] : playersActiveImbuements) {
-		Player* player = getPlayerByID(key);
-		if (!player) {
-			toErase.push_back(key);
+	for (const auto& [mapPlayerId, mapPlayer] : getPlayers()) {
+		if (!mapPlayer) {
 			continue;
 		}
 
-		player->updateInventoryImbuement();
-	}
-
-	for (uint32_t playerId : toErase) {
-		setPlayerActiveImbuements(playerId, 0);
+		mapPlayer->updateInventoryImbuement();
 	}
 
 }

src/game/game.h

@@ -537,27 +537,6 @@ class Game
 			return CharmList;
 		}
 
-		void increasePlayerActiveImbuements(uint32_t playerId) {
-			setPlayerActiveImbuements(playerId, playersActiveImbuements[playerId] + 1);
-		}
-
-		void decreasePlayerActiveImbuements(uint32_t playerId) {
-			setPlayerActiveImbuements(playerId, playersActiveImbuements[playerId] - 1);
-		}
-
-		void setPlayerActiveImbuements(uint32_t playerId, uint8_t value) {
-			if (value <= 0) {
-				playersActiveImbuements.erase(playerId);
-				return;
-			}
-			
-			playersActiveImbuements[playerId] = std::min<uint8_t>(255, value);
-		}
-
-		uint8_t getPlayerActiveImbuements(uint32_t playerId) {
-			return playersActiveImbuements[playerId];
-		}
-
 		FILELOADER_ERRORS loadAppearanceProtobuf(const std::string& file);
 		bool isMagicEffectRegistered(uint8_t type) const {
 			return std::find(registeredMagicEffects.begin(), registeredMagicEffects.end(), type) != registeredMagicEffects.end();
@@ -609,7 +588,6 @@ class Game
 		void playerSpeakToNpc(Player* player, const std::string& text);
 
 		phmap::flat_hash_map<uint32_t, Player*> players;
-		phmap::flat_hash_map<uint32_t, uint8_t> playersActiveImbuements;
 		phmap::flat_hash_map<std::string, Player*> mappedPlayerNames;
 		phmap::flat_hash_map<uint32_t, Guild*> guilds;
 		phmap::flat_hash_map<uint16_t, Item*> uniqueItems;

src/io/iologindata.cpp

@@ -734,7 +734,6 @@ bool IOLoginData::loadPlayer(Player* player, DBResult_ptr result)
 	player->initializeTaskHunting();
   player->updateBaseSpeed();
   player->updateInventoryWeight();
-  player->updateInventoryImbuement(true);
   player->updateItemsLight(true);
   return true;
 }

src/items/item.cpp

@@ -83,15 +83,16 @@ bool Item::getImbuementInfo(uint8_t slot, ImbuementInfo *imbuementInfo)
 	return imbuementInfo->duration && imbuementInfo->imbuement;
 }
 
-void Item::setImbuement(uint8_t slot, uint16_t imbuementId, int32_t duration)
+void Item::setImbuement(uint8_t slot, uint16_t imbuementId, uint32_t duration)
 {
 	std::string key = std::to_string(IMBUEMENT_SLOT + slot);
 	ItemAttributes::CustomAttribute customAttribute;
-	customAttribute.setInt64(duration > 0 ? (duration << 8) | imbuementId : 0);
+	auto convertSafeValue = std::clamp(static_cast<int64_t>(duration), (int64_t)0, std::numeric_limits<int64_t>::max());
+	customAttribute.setInt64(convertSafeValue > 0 ? (convertSafeValue << 8) | imbuementId : 0);
 	setCustomAttribute(key, customAttribute);
 }
 
-void Item::addImbuement(uint8_t slot, uint16_t imbuementId, int32_t duration)
+void Item::addImbuement(uint8_t slot, uint16_t imbuementId, uint32_t duration)
 {
 	Player* player = getHoldingPlayer();
 	if (!player) {
@@ -1493,8 +1494,8 @@ std::string Item::parseImbuementDescription(const Item* item)
 				continue;
 			}
 
-			int minutes = imbuementInfo.duration / 60;
-			int hours = minutes / 60;
+			auto minutes = imbuementInfo.duration / 60;
+			auto hours = minutes / 60;
 			s << fmt::format("{} {} {:02}:{:02}h", baseImbuement->name, imbuementInfo.imbuement->getName(), hours, minutes % 60);
 		}
 		s << ").";

src/items/item.h

@@ -1049,15 +1049,15 @@ class Item : virtual public Thing
 		 * @return false
 		 */
 		bool getImbuementInfo(uint8_t slot, ImbuementInfo *imbuementInfo);
-		void addImbuement(uint8_t slot, uint16_t imbuementId, int32_t duration);
+		void addImbuement(uint8_t slot, uint16_t imbuementId, uint32_t duration);
 		/**
 		 * @brief Decay imbuement time duration, only use this for decay the imbuement time
 		 * 
 		 * @param slot Slot id to decay
 		 * @param imbuementId Imbuement id to decay
 		 * @param duration New duration
 		 */
-		void decayImbuementTime(uint8_t slot, uint16_t imbuementId, int32_t duration) {
+		void decayImbuementTime(uint8_t slot, uint16_t imbuementId, uint32_t duration) {
 			return setImbuement(slot, imbuementId, duration);
 		}
 		void clearImbuement(uint8_t slot, uint16_t imbuementId) {
@@ -1146,7 +1146,7 @@ class Item : virtual public Thing
 		bool isLootTrackeable = false;
 
 	private:
-		void setImbuement(uint8_t slot, uint16_t imbuementId, int32_t duration);
+		void setImbuement(uint8_t slot, uint16_t imbuementId, uint32_t duration);
 		//Don't add variables here, use the ItemAttribute class.
 		friend class Decay;
 };

src/items/items_definitions.hpp

@@ -491,7 +491,7 @@ enum ItemParseAttributes_t {
 
 struct ImbuementInfo {
 	Imbuement *imbuement;
-	int32_t duration = 0;
+	uint32_t duration = 0;
 };
 
 #endif  // SRC_ITEMS_ITEMS_DEFINITIONS_HPP_

src/lua/creature/movement.cpp

@@ -487,8 +487,6 @@ uint32_t MoveEvent::EquipItem(MoveEvent *moveEvent, Player* player, Item* item,
 		}
 
 		player->addItemImbuementStats(imbuementInfo.imbuement);
-		g_game().increasePlayerActiveImbuements(player->getID());
-		player->updateInventoryImbuement(true);
 	}
 
 	if (it.abilities) {
@@ -585,8 +583,6 @@ uint32_t MoveEvent::DeEquipItem(MoveEvent*, Player* player, Item* item, Slots_t
 		}
 
 		player->removeItemImbuementStats(imbuementInfo.imbuement);
-		g_game().decreasePlayerActiveImbuements(player->getID());
-		player->updateInventoryImbuement(true);
 	}
 
 	if (it.abilities) {

src/lua/functions/items/item_functions.cpp

@@ -769,7 +769,7 @@ int ItemFunctions::luaItemGetImbuement(lua_State* L)
 		lua_createtable(L, 0, 3);
 		setField(L, "id", imbuement->getID());
 		setField(L, "name", imbuement->getName());
-		setField(L, "duration", imbuementInfo.duration);
+		setField(L, "duration", static_cast<lua_Number>(imbuementInfo.duration));
 	}
 	return 1;
 }