Skip to content

Commit

Permalink
perf: refactor imbuement decay to per-item management
Browse files Browse the repository at this point in the history
Refactored the imbuement decay system to manage individual items directly instead of iterating through all players. This improves performance by:

Tracking only items with active imbuements.
Scheduling events dynamically based on items needing updates.
Reducing redundant checks and player-wide loops.
Simplifying imbuement decay logic while preserving existing functionality, including protection zone, fight mode, and backpack-specific rules.
This change optimizes the handling of imbuements and ensures better scalability.
  • Loading branch information
dudantas committed Nov 20, 2024
1 parent b5f71a8 commit c001339
Show file tree
Hide file tree
Showing 10 changed files with 211 additions and 80 deletions.
2 changes: 1 addition & 1 deletion src/creatures/creature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,7 @@ void Creature::getPathSearchParams(const std::shared_ptr<Creature> &, FindPathPa

void Creature::goToFollowCreature_async(std::function<void()> &&onComplete) {
if (!hasAsyncTaskFlag(Pathfinder) && onComplete) {
g_dispatcher().addEvent(std::move(onComplete), "goToFollowCreature_async");
g_dispatcher().addEvent(std::move(onComplete), "Creature::goToFollowCreature_async");
}

setAsyncTaskFlag(Pathfinder, true);
Expand Down
167 changes: 166 additions & 1 deletion src/creatures/players/imbuements/imbuements.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@
#include "config/configmanager.hpp"
#include "creatures/players/player.hpp"
#include "items/item.hpp"
#include "items/tile.hpp"
#include "game/scheduling/dispatcher.hpp"
#include "lib/di/container.hpp"
#include "utils/pugicast.hpp"
#include <utils/tools.hpp>
#include "utils/const.hpp"
#include "utils/tools.hpp"

Imbuements &Imbuements::getInstance() {
return inject<Imbuements>();
Expand Down Expand Up @@ -417,3 +420,165 @@ const std::vector<std::pair<uint16_t, uint16_t>> &Imbuement::getItems() const {
uint16_t Imbuement::getIconID() const {
return icon + (baseid - 1);
}

ImbuementDecay &ImbuementDecay::getInstance() {
return inject<ImbuementDecay>();
}

void ImbuementDecay::startImbuementDecay(const std::shared_ptr<Item> &item) {
if (!item) {
g_logger().error("[{}] item is nullptr", __FUNCTION__);
return;
}

if (m_itemsToDecay.find(item) != m_itemsToDecay.end()) {
return;
}

g_logger().debug("Starting imbuement decay for item {}", item->getName());

m_itemsToDecay.insert(item);

if (m_eventId == 0) {
m_lastUpdateTime = OTSYS_TIME();
m_eventId = g_dispatcher().scheduleEvent(
60000, [this] { checkImbuementDecay(); }, "ImbuementDecay::checkImbuementDecay"
);
g_logger().trace("Scheduled imbuement decay check every 60 seconds.");
}
}

void ImbuementDecay::stopImbuementDecay(const std::shared_ptr<Item> &item) {
if (!item) {
g_logger().error("[{}] item is nullptr", __FUNCTION__);
return;
}

g_logger().debug("Stopping imbuement decay for item {}", item->getName());

int64_t currentTime = OTSYS_TIME();
int64_t elapsedTime = currentTime - m_lastUpdateTime;

for (uint8_t slotid = 0; slotid < item->getImbuementSlot(); slotid++) {
ImbuementInfo imbuementInfo;
if (!item->getImbuementInfo(slotid, &imbuementInfo)) {
continue;
}

uint32_t duration = imbuementInfo.duration > elapsedTime / 1000 ? imbuementInfo.duration - static_cast<uint32_t>(elapsedTime / 1000) : 0;
item->decayImbuementTime(slotid, imbuementInfo.imbuement->getID(), duration);

if (duration == 0) {
if (auto player = item->getHoldingPlayer()) {
player->removeItemImbuementStats(imbuementInfo.imbuement);
player->updateImbuementTrackerStats();
}
}
}

m_itemsToDecay.erase(item);

if (m_itemsToDecay.empty() && m_eventId != 0) {
g_dispatcher().stopEvent(m_eventId);
m_eventId = 0;
g_logger().trace("No more items to decay. Stopped imbuement decay scheduler.");
}
}

void ImbuementDecay::checkImbuementDecay() {
int64_t currentTime = OTSYS_TIME();
int64_t elapsedTime = currentTime - m_lastUpdateTime;
m_lastUpdateTime = currentTime;

g_logger().trace("Checking imbuement decay for {} items.", m_itemsToDecay.size());

std::vector<std::shared_ptr<Item>> itemsToRemove;

for (const auto &item : m_itemsToDecay) {
if (!item) {
g_logger().error("[{}] item is nullptr", __FUNCTION__);
itemsToRemove.push_back(item);
continue;
}

// Get the player holding the item (if any)
auto player = item->getHoldingPlayer();
if (!player) {
g_logger().debug("Item {} is not held by any player. Skipping decay.", item->getName());
continue;
}

bool removeItem = true;

for (uint8_t slotid = 0; slotid < item->getImbuementSlot(); ++slotid) {
ImbuementInfo imbuementInfo;
if (!item->getImbuementInfo(slotid, &imbuementInfo)) {
g_logger().debug("No imbuement info found for slot {} in item {}", slotid, item->getName());
continue;
}

// Get the tile the player is currently on
const auto &playerTile = player->getTile();
// Check if the player is in a protection zone
const bool &isInProtectionZone = playerTile && playerTile->hasFlag(TILESTATE_PROTECTIONZONE);
// Check if the player is in fight mode
bool isInFightMode = player->hasCondition(CONDITION_INFIGHT);
bool nonAggressiveFightOnly = g_configManager().getBoolean(TOGGLE_IMBUEMENT_NON_AGGRESSIVE_FIGHT_ONLY);

// Imbuement from imbuementInfo, this variable reduces code complexity
const auto imbuement = imbuementInfo.imbuement;
// Get the category of the imbuement
const CategoryImbuement* categoryImbuement = g_imbuements().getCategoryByID(imbuement->getCategory());
// Parent of the imbued item
const auto &parent = item->getParent();
const bool &isInBackpack = parent && parent->getContainer();
// 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 || nonAggressiveFightOnly) && (isInProtectionZone || !isInFightMode || isInBackpack)) {
continue;
}
// If the item is not in the backpack slot and it's not a agressive imbuement, ignore it.
if (categoryImbuement && !categoryImbuement->agressive && parent && parent != player) {
continue;
}

// If the imbuement's duration is 0, remove its stats and continue to the next slot
if (imbuementInfo.duration == 0) {
player->removeItemImbuementStats(imbuement);
player->updateImbuementTrackerStats();
continue;
}

g_logger().debug("Decaying imbuement {} from item {} of player {}", imbuement->getName(), item->getName(), player->getName());
// Calculate the new duration of the imbuement, making sure it doesn't go below 0
uint32_t duration = imbuementInfo.duration > elapsedTime / 1000 ? imbuementInfo.duration - static_cast<uint32_t>(elapsedTime / 1000) : 0;
item->decayImbuementTime(slotid, imbuement->getID(), duration);

if (duration == 0) {
player->removeItemImbuementStats(imbuement);
player->updateImbuementTrackerStats();
}

removeItem = false;
}

if (removeItem) {
itemsToRemove.push_back(item);
}
}

// Remove items whose imbuements have expired
for (const auto &item : itemsToRemove) {
m_itemsToDecay.erase(item);
}

// Reschedule the event if there are still items
if (!m_itemsToDecay.empty()) {
m_eventId = g_dispatcher().scheduleEvent(
60000, [this] { checkImbuementDecay(); }, "ImbuementDecay::checkImbuementDecay"
);
g_logger().trace("Re-scheduled imbuement decay check.");
} else {
m_eventId = 0;
g_logger().trace("No more items to decay. Stopped imbuement decay scheduler.");
}
}
22 changes: 22 additions & 0 deletions src/creatures/players/imbuements/imbuements.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,25 @@ class Imbuement {

std::vector<std::pair<uint16_t, uint16_t>> items;
};

class ImbuementDecay {
public:
ImbuementDecay() = default;

// Non-copyable
ImbuementDecay(const ImbuementDecay &) = delete;
ImbuementDecay &operator=(const ImbuementDecay &) = delete;

static ImbuementDecay &getInstance();

void startImbuementDecay(const std::shared_ptr<Item> &item);
void stopImbuementDecay(const std::shared_ptr<Item> &item);
void checkImbuementDecay();

private:
std::unordered_set<std::shared_ptr<Item>> m_itemsToDecay;
int64_t m_lastUpdateTime = 0;
uint32_t m_eventId { 0 };
};

constexpr auto g_imbuementDecay = ImbuementDecay::getInstance;
59 changes: 2 additions & 57 deletions src/creatures/players/player.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "io/iobestiary.hpp"
#include "io/iologindata.hpp"
#include "io/ioprey.hpp"
#include "creatures/players/imbuements/imbuements.hpp"
#include "items/bed.hpp"
#include "items/containers/depot/depotchest.hpp"
#include "items/containers/depot/depotlocker.hpp"
Expand Down Expand Up @@ -680,63 +681,6 @@ void Player::updateInventoryWeight() {
}
}

void Player::updateInventoryImbuement() {
// Get the tile the player is currently on
const auto &playerTile = getTile();
// Check if the player is in a protection zone
const bool &isInProtectionZone = playerTile && playerTile->hasFlag(TILESTATE_PROTECTIONZONE);
// Check if the player is in fight mode
bool isInFightMode = hasCondition(CONDITION_INFIGHT);
bool nonAggressiveFightOnly = g_configManager().getBoolean(TOGGLE_IMBUEMENT_NON_AGGRESSIVE_FIGHT_ONLY);

// Iterate through all items in the player's inventory
for (const auto &[slodNumber, item] : getAllSlotItems()) {
// 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)) {
// If no imbuement is found, continue to the next slot
continue;
}

// Imbuement from imbuementInfo, this variable reduces code complexity
const auto imbuement = imbuementInfo.imbuement;
// Get the category of the imbuement
const CategoryImbuement* categoryImbuement = g_imbuements().getCategoryByID(imbuement->getCategory());
// Parent of the imbued item
const auto &parent = item->getParent();
const bool &isInBackpack = parent && parent->getContainer();
// 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 || nonAggressiveFightOnly) && (isInProtectionZone || !isInFightMode || isInBackpack)) {
continue;
}
// If the item is not in the backpack slot and it's not a agressive imbuement, ignore it.
if (categoryImbuement && !categoryImbuement->agressive && parent && parent != getPlayer()) {
continue;
}

// If the imbuement's duration is 0, remove its stats and continue to the next slot
if (imbuementInfo.duration == 0) {
removeItemImbuementStats(imbuement);
updateImbuementTrackerStats();
continue;
}

g_logger().trace("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
const uint32_t duration = std::max<uint32_t>(0, imbuementInfo.duration - EVENT_IMBUEMENT_INTERVAL / 1000);
// Update the imbuement's duration in the item
item->decayImbuementTime(slotid, imbuement->getID(), duration);

if (duration == 0) {
removeItemImbuementStats(imbuement);
updateImbuementTrackerStats();
}
}
}
}

phmap::flat_hash_map<uint8_t, std::shared_ptr<Item>> Player::getAllSlotItems() const {
phmap::flat_hash_map<uint8_t, std::shared_ptr<Item>> itemMap;
for (uint8_t i = CONST_SLOT_FIRST; i <= CONST_SLOT_LAST; ++i) {
Expand Down Expand Up @@ -2338,6 +2282,7 @@ void Player::onApplyImbuement(const Imbuement* imbuement, const std::shared_ptr<
addItemImbuementStats(imbuement);
}

g_imbuementDecay().startImbuementDecay(item);
item->addImbuement(slot, imbuement->getID(), baseImbuement->duration);
openImbuementWindow(item);
}
Expand Down
5 changes: 0 additions & 5 deletions src/creatures/players/player.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1283,11 +1283,6 @@ class Player final : public Creature, public Cylinder, public Bankable {
void removeExperience(uint64_t exp, bool sendText = false);

void updateInventoryWeight();
/**
* @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();

void setNextWalkActionTask(const std::shared_ptr<Task> &task);
void setNextWalkTask(const std::shared_ptr<Task> &task);
Expand Down
13 changes: 0 additions & 13 deletions src/game/game.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -541,9 +541,6 @@ void Game::start(ServiceManager* manager) {
g_dispatcher().cycleEvent(
EVENT_CHECK_CREATURE_INTERVAL, [this] { checkCreatures(); }, "Game::checkCreatures"
);
g_dispatcher().cycleEvent(
EVENT_IMBUEMENT_INTERVAL, [this] { checkImbuements(); }, "Game::checkImbuements"
);
g_dispatcher().cycleEvent(
EVENT_LUA_GARBAGE_COLLECTION, [this] { g_luaEnvironment().collectGarbage(); }, "Calling GC"
);
Expand Down Expand Up @@ -8008,16 +8005,6 @@ void Game::addDistanceEffect(const CreatureVector &spectators, const Position &f
}
}

void Game::checkImbuements() const {
for (const auto &[mapPlayerId, mapPlayer] : getPlayers()) {
if (!mapPlayer) {
continue;
}

mapPlayer->updateInventoryImbuement();
}
}

void Game::checkLight() {
lightHour += lightHourDelta;

Expand Down
1 change: 0 additions & 1 deletion src/game/game.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,6 @@ class Game {
std::map<uint32_t, int32_t> forgeMonsterEventIds;
std::unordered_set<uint32_t> fiendishMonsters;
std::unordered_set<uint32_t> influencedMonsters;
void checkImbuements() const;
bool playerSaySpell(const std::shared_ptr<Player> &player, SpeakClasses type, const std::string &text);
void playerWhisper(const std::shared_ptr<Player> &player, const std::string &text);
bool playerYell(const std::shared_ptr<Player> &player, const std::string &text);
Expand Down
5 changes: 3 additions & 2 deletions src/game/scheduling/task.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class Task {
"Game::checkCreatureAttack",
"Game::checkCreatureWalk",
"Game::checkCreatures",
"Game::checkImbuements",
"Game::checkLight",
"Game::createFiendishMonsters",
"Game::createInfluencedMonsters",
Expand All @@ -88,7 +87,9 @@ class Task {
"SpawnNpc::checkSpawnNpc",
"Webhook::run",
"Protocol::sendRecvMessageCallback",
"Player::addInFightTicks"
"Player::addInFightTicks",
"Map::moveCreature",
"Creature::goToFollowCreature_async"
};

return tasksContext.contains(context);
Expand Down
Loading

0 comments on commit c001339

Please sign in to comment.