From c419a21a4d9f93c5bc2cd8ec2fd637f8381b18d9 Mon Sep 17 00:00:00 2001 From: Artur Knopik Linux Date: Tue, 24 Dec 2024 08:50:14 +0100 Subject: [PATCH 1/7] fix weapons memory leaks --- src/weapons.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/weapons.cpp b/src/weapons.cpp index bed996c957..3049b74d71 100644 --- a/src/weapons.cpp +++ b/src/weapons.cpp @@ -36,6 +36,7 @@ void Weapons::clear(bool fromLua) { for (auto it = weapons.begin(); it != weapons.end();) { if (fromLua == it->second->fromLua) { + delete(it->second); it = weapons.erase(it); } else { ++it; From 4e0fd25f2390084107096d9c1eee2ed55c1ce012 Mon Sep 17 00:00:00 2001 From: Artur Knopik Linux Date: Tue, 24 Dec 2024 09:05:48 +0100 Subject: [PATCH 2/7] fix weapons memory leaks --- src/weapons.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/weapons.cpp b/src/weapons.cpp index 3049b74d71..20942dcce6 100644 --- a/src/weapons.cpp +++ b/src/weapons.cpp @@ -36,7 +36,7 @@ void Weapons::clear(bool fromLua) { for (auto it = weapons.begin(); it != weapons.end();) { if (fromLua == it->second->fromLua) { - delete(it->second); + delete it->second; it = weapons.erase(it); } else { ++it; From ba5a525cf785bb4bd050d9a9cb96be3c6ba770c5 Mon Sep 17 00:00:00 2001 From: Artur Knopik Linux Date: Tue, 24 Dec 2024 13:29:59 +0100 Subject: [PATCH 3/7] fix scheduler/tasks leaks --- src/game.cpp | 68 +++++++++++++++++++++++------------------------ src/player.cpp | 18 ++++++------- src/player.h | 7 ++--- src/scheduler.cpp | 24 ++++++++--------- src/scheduler.h | 10 ++++--- src/tasks.cpp | 19 ++++++------- src/tasks.h | 16 ++++++----- 7 files changed, 83 insertions(+), 79 deletions(-) diff --git a/src/game.cpp b/src/game.cpp index 9a871adb52..9f16a2a974 100644 --- a/src/game.cpp +++ b/src/game.cpp @@ -634,11 +634,11 @@ void Game::playerMoveThing(uint32_t playerId, const Position& fromPos, uint16_t } if (movingCreature->getPosition().isInRange(player->getPosition(), 1, 1, 0)) { - SchedulerTask* task = createSchedulerTask( + SchedulerTask_ptr task = createSchedulerTask( MOVE_CREATURE_INTERVAL, [=, this, playerID = player->getID(), creatureID = movingCreature->getID()]() { playerMoveCreatureByID(playerID, creatureID, fromPos, toPos); }); - player->setNextActionTask(task); + player->setNextActionTask(std::move(task)); } else { playerMoveCreature(player, movingCreature, movingCreature->getPosition(), tile); } @@ -680,12 +680,12 @@ void Game::playerMoveCreature(Player* player, Creature* movingCreature, const Po { if (!player->canDoAction()) { uint32_t delay = player->getNextActionTime(); - SchedulerTask* task = + SchedulerTask_ptr task = createSchedulerTask(delay, [=, this, playerID = player->getID(), movingCreatureID = movingCreature->getID(), toPos = toTile->getPosition()]() { playerMoveCreatureByID(playerID, movingCreatureID, movingCreatureOrigPos, toPos); }); - player->setNextActionTask(task); + player->setNextActionTask(std::move(task)); return; } @@ -703,13 +703,13 @@ void Game::playerMoveCreature(Player* player, Creature* movingCreature, const Po g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask* task = + SchedulerTask_ptr task = createSchedulerTask(RANGE_MOVE_CREATURE_INTERVAL, [=, this, playerID = player->getID(), movingCreatureID = movingCreature->getID(), toPos = toTile->getPosition()] { playerMoveCreatureByID(playerID, movingCreatureID, movingCreatureOrigPos, toPos); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } @@ -891,10 +891,10 @@ void Game::playerMoveItem(Player* player, const Position& fromPos, uint16_t spri { if (!player->canDoAction()) { uint32_t delay = player->getNextActionTime(); - SchedulerTask* task = createSchedulerTask(delay, [=, this, playerID = player->getID()]() { + SchedulerTask_ptr task = createSchedulerTask(delay, [=, this, playerID = player->getID()]() { playerMoveItemByPlayerID(playerID, fromPos, spriteId, fromStackPos, toPos, count); }); - player->setNextActionTask(task); + player->setNextActionTask(std::move(task)); return; } @@ -960,11 +960,11 @@ void Game::playerMoveItem(Player* player, const Position& fromPos, uint16_t spri g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask* task = + SchedulerTask_ptr task = createSchedulerTask(RANGE_MOVE_ITEM_INTERVAL, [=, this, playerID = player->getID()]() { playerMoveItemByPlayerID(playerID, fromPos, spriteId, fromStackPos, toPos, count); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } @@ -1022,12 +1022,12 @@ void Game::playerMoveItem(Player* player, const Position& fromPos, uint16_t spri g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask* task = createSchedulerTask( + SchedulerTask_ptr task = createSchedulerTask( RANGE_MOVE_ITEM_INTERVAL, [this, playerID = player->getID(), itemPos, spriteId, itemStackPos, toPos, count]() { playerMoveItemByPlayerID(playerID, itemPos, spriteId, itemStackPos, toPos, count); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } @@ -2137,10 +2137,10 @@ void Game::playerUseItemEx(uint32_t playerId, const Position& fromPos, uint8_t f g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask* task = createSchedulerTask(RANGE_USE_ITEM_EX_INTERVAL, [=, this]() { + SchedulerTask_ptr task = createSchedulerTask(RANGE_USE_ITEM_EX_INTERVAL, [=, this]() { playerUseItemEx(playerId, itemPos, itemStackPos, fromSpriteId, toPos, toStackPos, toSpriteId); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } @@ -2153,10 +2153,10 @@ void Game::playerUseItemEx(uint32_t playerId, const Position& fromPos, uint8_t f if (!player->canDoAction()) { uint32_t delay = player->getNextActionTime(); - SchedulerTask* task = createSchedulerTask(delay, [=, this]() { + SchedulerTask_ptr task = createSchedulerTask(delay, [=, this]() { playerUseItemEx(playerId, fromPos, fromStackPos, fromSpriteId, toPos, toStackPos, toSpriteId); }); - player->setNextActionTask(task); + player->setNextActionTask(std::move(task)); return; } @@ -2198,9 +2198,9 @@ void Game::playerUseItem(uint32_t playerId, const Position& pos, uint8_t stackPo g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask* task = createSchedulerTask( + SchedulerTask_ptr task = createSchedulerTask( RANGE_USE_ITEM_INTERVAL, [=, this]() { playerUseItem(playerId, pos, stackPos, index, spriteId); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); return; } @@ -2213,9 +2213,9 @@ void Game::playerUseItem(uint32_t playerId, const Position& pos, uint8_t stackPo if (!player->canDoAction()) { uint32_t delay = player->getNextActionTime(); - SchedulerTask* task = + SchedulerTask_ptr task = createSchedulerTask(delay, [=, this]() { playerUseItem(playerId, pos, stackPos, index, spriteId); }); - player->setNextActionTask(task); + player->setNextActionTask(std::move(task)); return; } @@ -2297,10 +2297,10 @@ void Game::playerUseWithCreature(uint32_t playerId, const Position& fromPos, uin g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask* task = createSchedulerTask(RANGE_USE_WITH_CREATURE_INTERVAL, [=, this]() { + SchedulerTask_ptr task = createSchedulerTask(RANGE_USE_WITH_CREATURE_INTERVAL, [=, this]() { playerUseWithCreature(playerId, itemPos, itemStackPos, creatureId, spriteId); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } @@ -2313,9 +2313,9 @@ void Game::playerUseWithCreature(uint32_t playerId, const Position& fromPos, uin if (!player->canDoAction()) { uint32_t delay = player->getNextActionTime(); - SchedulerTask* task = createSchedulerTask( + SchedulerTask_ptr task = createSchedulerTask( delay, [=, this]() { playerUseWithCreature(playerId, fromPos, fromStackPos, creatureId, spriteId); }); - player->setNextActionTask(task); + player->setNextActionTask(std::move(task)); return; } @@ -2416,9 +2416,9 @@ void Game::playerRotateItem(uint32_t playerId, const Position& pos, uint8_t stac g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask* task = createSchedulerTask( + SchedulerTask_ptr task = createSchedulerTask( RANGE_ROTATE_ITEM_INTERVAL, [=, this]() { playerRotateItem(playerId, pos, stackPos, spriteId); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } @@ -2512,9 +2512,9 @@ void Game::playerBrowseField(uint32_t playerId, const Position& pos) g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask* task = + SchedulerTask_ptr task = createSchedulerTask(RANGE_BROWSE_FIELD_INTERVAL, [=, this]() { playerBrowseField(playerId, pos); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } @@ -2618,9 +2618,9 @@ void Game::playerWrapItem(uint32_t playerId, const Position& position, uint8_t s g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask* task = createSchedulerTask( + SchedulerTask_ptr task = createSchedulerTask( RANGE_WRAP_ITEM_INTERVAL, [=, this]() { playerWrapItem(playerId, position, stackPos, spriteId); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } @@ -2690,10 +2690,10 @@ void Game::playerRequestTrade(uint32_t playerId, const Position& pos, uint8_t st g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask* task = createSchedulerTask(RANGE_REQUEST_TRADE_INTERVAL, [=, this]() { + SchedulerTask_ptr task = createSchedulerTask(RANGE_REQUEST_TRADE_INTERVAL, [=, this]() { playerRequestTrade(playerId, pos, stackPos, tradePlayerId, spriteId); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } @@ -3413,9 +3413,9 @@ void Game::playerRequestEditPodium(uint32_t playerId, const Position& position, g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask* task = createSchedulerTask( + SchedulerTask_ptr task = createSchedulerTask( 400, [=, this]() { playerRequestEditPodium(playerId, position, stackPos, spriteId); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } diff --git a/src/player.cpp b/src/player.cpp index 356d04b837..d284204a0a 100644 --- a/src/player.cpp +++ b/src/player.cpp @@ -1508,18 +1508,17 @@ void Player::checkTradeState(const Item* item) } } -void Player::setNextWalkActionTask(SchedulerTask* task) +void Player::setNextWalkActionTask(SchedulerTask_ptr task) { if (walkTaskEvent != 0) { g_scheduler.stopEvent(walkTaskEvent); walkTaskEvent = 0; } - delete walkTask; - walkTask = task; + walkTask = std::move(task); } -void Player::setNextActionTask(SchedulerTask* task, bool resetIdleTime /*= true */) +void Player::setNextActionTask(SchedulerTask_ptr task, bool resetIdleTime /*= true */) { if (actionTaskEvent != 0) { g_scheduler.stopEvent(actionTaskEvent); @@ -1527,7 +1526,7 @@ void Player::setNextActionTask(SchedulerTask* task, bool resetIdleTime /*= true } if (task) { - actionTaskEvent = g_scheduler.addEvent(task); + actionTaskEvent = g_scheduler.addEvent(std::move(task)); if (resetIdleTime) { this->resetIdleTime(); } @@ -3407,13 +3406,13 @@ void Player::doAttacking(uint32_t) result = Weapon::useFist(this, attackedCreature); } - SchedulerTask* task = createSchedulerTask(std::max(SCHEDULER_MINTICKS, delay), + SchedulerTask_ptr task = createSchedulerTask(std::max(SCHEDULER_MINTICKS, delay), [id = getID()]() { g_game.checkCreatureAttack(id); }); if (!classicSpeed) { - setNextActionTask(task, false); + setNextActionTask(std::move(task), false); } else { g_scheduler.stopEvent(classicAttackEvent); - classicAttackEvent = g_scheduler.addEvent(task); + classicAttackEvent = g_scheduler.addEvent(std::move(task)); } if (result) { @@ -3469,8 +3468,7 @@ void Player::onWalkAborted() void Player::onWalkComplete() { if (walkTask) { - walkTaskEvent = g_scheduler.addEvent(walkTask); - walkTask = nullptr; + walkTaskEvent = g_scheduler.addEvent(std::move(walkTask)); } } diff --git a/src/player.h b/src/player.h index 59eb2e2fb7..4ef9172b0e 100644 --- a/src/player.h +++ b/src/player.h @@ -15,6 +15,7 @@ #include "protocolgame.h" #include "town.h" #include "vocation.h" +#include "scheduler.h" class House; struct Mount; @@ -1135,8 +1136,8 @@ class Player final : public Creature, public Cylinder void updateInventoryWeight(); - void setNextWalkActionTask(SchedulerTask* task); - void setNextActionTask(SchedulerTask* task, bool resetIdleTime = true); + void setNextWalkActionTask(SchedulerTask_ptr task); + void setNextActionTask(SchedulerTask_ptr task, bool resetIdleTime = true); void death(Creature* lastHitCreature) override; bool dropCorpse(Creature* lastHitCreature, Creature* mostDamageCreature, bool lastHitUnjustified, @@ -1226,7 +1227,7 @@ class Player final : public Creature, public Cylinder Npc* shopOwner = nullptr; Party* party = nullptr; Player* tradePartner = nullptr; - SchedulerTask* walkTask = nullptr; + SchedulerTask_ptr walkTask = nullptr; const Town* town = nullptr; Vocation* vocation = nullptr; StoreInbox* storeInbox = nullptr; diff --git a/src/scheduler.cpp b/src/scheduler.cpp index bd59de6479..bb2eb7d89b 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -5,33 +5,33 @@ #include "scheduler.h" -uint32_t Scheduler::addEvent(SchedulerTask* task) +uint32_t Scheduler::addEvent(SchedulerTask_ptr task) { - // check if the event has a valid id if (task->getEventId() == 0) { task->setEventId(++lastEventId); } - boost::asio::post(io_context, [this, task]() { + auto eventId = task->getEventId(); + + boost::asio::post(io_context, [this, scheduledTask = std::move(task)]() mutable { // insert the event id in the list of active events - auto it = eventIdTimerMap.emplace(task->getEventId(), boost::asio::steady_timer{io_context}); + auto it = eventIdTimerMap.emplace(scheduledTask->getEventId(), boost::asio::steady_timer{io_context}); auto& timer = it.first->second; - timer.expires_after(std::chrono::milliseconds(task->getDelay())); - timer.async_wait([this, task](const boost::system::error_code& error) { - eventIdTimerMap.erase(task->getEventId()); + timer.expires_after(std::chrono::milliseconds(scheduledTask->getDelay())); + timer.async_wait([this, scheduledTask = std::move(scheduledTask)](const boost::system::error_code& error) mutable { + eventIdTimerMap.erase(scheduledTask->getEventId()); if (error == boost::asio::error::operation_aborted || getState() == THREAD_STATE_TERMINATED) { - // the timer has been manually canceled(timer->cancel()) or Scheduler::shutdown has been called - delete task; + // The timer was manually canceled or Scheduler::shutdown was called return; } - g_dispatcher.addTask(task); + g_dispatcher.addTask(std::move(scheduledTask)); }); }); - return task->getEventId(); + return eventId; } void Scheduler::stopEvent(uint32_t eventId) @@ -62,4 +62,4 @@ void Scheduler::shutdown() }); } -SchedulerTask* createSchedulerTask(uint32_t delay, TaskFunc&& f) { return new SchedulerTask(delay, std::move(f)); } +SchedulerTask_ptr createSchedulerTask(uint32_t delay, TaskFunc&& f) { return SchedulerTask_ptr(new SchedulerTask(delay, std::move(f))); } diff --git a/src/scheduler.h b/src/scheduler.h index 221c3c1b6c..4df141c31a 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -9,6 +9,10 @@ static constexpr int32_t SCHEDULER_MINTICKS = 50; +class SchedulerTask; + +using SchedulerTask_ptr = std::unique_ptr; + class SchedulerTask : public Task { public: @@ -23,15 +27,15 @@ class SchedulerTask : public Task uint32_t eventId = 0; uint32_t delay = 0; - friend SchedulerTask* createSchedulerTask(uint32_t, TaskFunc&&); + friend SchedulerTask_ptr createSchedulerTask(uint32_t delay, TaskFunc&& f); }; -SchedulerTask* createSchedulerTask(uint32_t delay, TaskFunc&& f); +SchedulerTask_ptr createSchedulerTask(uint32_t delay, TaskFunc&& f); class Scheduler : public ThreadHolder { public: - uint32_t addEvent(SchedulerTask* task); + uint32_t addEvent(SchedulerTask_ptr task); void stopEvent(uint32_t eventId); void shutdown(); diff --git a/src/tasks.cpp b/src/tasks.cpp index 6a48de2977..e72f231712 100644 --- a/src/tasks.cpp +++ b/src/tasks.cpp @@ -10,13 +10,13 @@ extern Game g_game; -Task* createTask(TaskFunc&& f) { return new Task(std::move(f)); } +Task_ptr createTask(TaskFunc&& f) { return std::make_unique(std::move(f)); } -Task* createTask(uint32_t expiration, TaskFunc&& f) { return new Task(expiration, std::move(f)); } +Task_ptr createTask(uint32_t expiration, TaskFunc&& f) { return std::make_unique(expiration, std::move(f)); } void Dispatcher::threadMain() { - std::vector tmpTaskList; + std::vector> tmpTaskList; // NOTE: second argument defer_lock is to prevent from immediate locking std::unique_lock taskLockUnique(taskLock, std::defer_lock); @@ -30,19 +30,18 @@ void Dispatcher::threadMain() tmpTaskList.swap(taskList); taskLockUnique.unlock(); - for (Task* task : tmpTaskList) { + for (auto &task : tmpTaskList) { if (!task->hasExpired()) { ++dispatcherCycle; // execute it (*task)(); } - delete task; } tmpTaskList.clear(); } } -void Dispatcher::addTask(Task* task) +void Dispatcher::addTask(Task_ptr task) { bool do_signal = false; @@ -50,9 +49,7 @@ void Dispatcher::addTask(Task* task) if (getState() == THREAD_STATE_RUNNING) { do_signal = taskList.empty(); - taskList.push_back(task); - } else { - delete task; + taskList.push_back(std::move(task)); } taskLock.unlock(); @@ -65,13 +62,13 @@ void Dispatcher::addTask(Task* task) void Dispatcher::shutdown() { - Task* task = createTask([this]() { + Task_ptr task = createTask([this]() { setState(THREAD_STATE_TERMINATED); taskSignal.notify_one(); }); std::lock_guard lockClass(taskLock); - taskList.push_back(task); + taskList.push_back(std::move(task)); taskSignal.notify_one(); } diff --git a/src/tasks.h b/src/tasks.h index 1f39aa5519..56f27f5273 100644 --- a/src/tasks.h +++ b/src/tasks.h @@ -10,6 +10,10 @@ using TaskFunc = std::function; const int DISPATCHER_TASK_EXPIRATION = 2000; const auto SYSTEM_TIME_ZERO = std::chrono::system_clock::time_point(std::chrono::milliseconds(0)); +class Task; + +using Task_ptr = std::unique_ptr; + class Task { public: @@ -41,17 +45,17 @@ class Task TaskFunc func; }; -Task* createTask(TaskFunc&& f); -Task* createTask(uint32_t expiration, TaskFunc&& f); +Task_ptr createTask(TaskFunc&& f); +Task_ptr createTask(uint32_t expiration, TaskFunc&& f); class Dispatcher : public ThreadHolder { public: - void addTask(Task* task); + void addTask(Task_ptr task); - void addTask(TaskFunc&& f) { addTask(new Task(std::move(f))); } + void addTask(TaskFunc&& f) { addTask( std::make_unique(std::move(f))); } - void addTask(uint32_t expiration, TaskFunc&& f) { addTask(new Task(expiration, std::move(f))); } + void addTask(uint32_t expiration, TaskFunc&& f) { addTask(make_unique(expiration, std::move(f))); } void shutdown(); @@ -63,7 +67,7 @@ class Dispatcher : public ThreadHolder std::mutex taskLock; std::condition_variable taskSignal; - std::vector taskList; + std::vector taskList; uint64_t dispatcherCycle = 0; }; From abdf6c5cb271d3174a53befb3d73940774eaa845 Mon Sep 17 00:00:00 2001 From: Artur Knopik Linux Date: Tue, 24 Dec 2024 13:32:25 +0100 Subject: [PATCH 4/7] fix scheduler/tasks leaks --- src/weapons.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/weapons.cpp b/src/weapons.cpp index 20942dcce6..bed996c957 100644 --- a/src/weapons.cpp +++ b/src/weapons.cpp @@ -36,7 +36,6 @@ void Weapons::clear(bool fromLua) { for (auto it = weapons.begin(); it != weapons.end();) { if (fromLua == it->second->fromLua) { - delete it->second; it = weapons.erase(it); } else { ++it; From 32d01a99a0cf28b9135ae7b15ef626d7af056f38 Mon Sep 17 00:00:00 2001 From: Artur Knopik Linux Date: Tue, 24 Dec 2024 13:37:49 +0100 Subject: [PATCH 5/7] fix formating --- src/player.cpp | 2 +- src/player.h | 2 +- src/scheduler.cpp | 22 +++++++++++++--------- src/tasks.cpp | 2 +- src/tasks.h | 2 +- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/player.cpp b/src/player.cpp index d284204a0a..1a61059e9f 100644 --- a/src/player.cpp +++ b/src/player.cpp @@ -3407,7 +3407,7 @@ void Player::doAttacking(uint32_t) } SchedulerTask_ptr task = createSchedulerTask(std::max(SCHEDULER_MINTICKS, delay), - [id = getID()]() { g_game.checkCreatureAttack(id); }); + [id = getID()]() { g_game.checkCreatureAttack(id); }); if (!classicSpeed) { setNextActionTask(std::move(task), false); } else { diff --git a/src/player.h b/src/player.h index 4ef9172b0e..c1b97d27c4 100644 --- a/src/player.h +++ b/src/player.h @@ -13,9 +13,9 @@ #include "guild.h" #include "inbox.h" #include "protocolgame.h" +#include "scheduler.h" #include "town.h" #include "vocation.h" -#include "scheduler.h" class House; struct Mount; diff --git a/src/scheduler.cpp b/src/scheduler.cpp index bb2eb7d89b..ad9e7b898d 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -19,16 +19,17 @@ uint32_t Scheduler::addEvent(SchedulerTask_ptr task) auto& timer = it.first->second; timer.expires_after(std::chrono::milliseconds(scheduledTask->getDelay())); - timer.async_wait([this, scheduledTask = std::move(scheduledTask)](const boost::system::error_code& error) mutable { - eventIdTimerMap.erase(scheduledTask->getEventId()); + timer.async_wait( + [this, scheduledTask = std::move(scheduledTask)](const boost::system::error_code& error) mutable { + eventIdTimerMap.erase(scheduledTask->getEventId()); - if (error == boost::asio::error::operation_aborted || getState() == THREAD_STATE_TERMINATED) { - // The timer was manually canceled or Scheduler::shutdown was called - return; - } + if (error == boost::asio::error::operation_aborted || getState() == THREAD_STATE_TERMINATED) { + // The timer was manually canceled or Scheduler::shutdown was called + return; + } - g_dispatcher.addTask(std::move(scheduledTask)); - }); + g_dispatcher.addTask(std::move(scheduledTask)); + }); }); return eventId; @@ -62,4 +63,7 @@ void Scheduler::shutdown() }); } -SchedulerTask_ptr createSchedulerTask(uint32_t delay, TaskFunc&& f) { return SchedulerTask_ptr(new SchedulerTask(delay, std::move(f))); } +SchedulerTask_ptr createSchedulerTask(uint32_t delay, TaskFunc&& f) +{ + return SchedulerTask_ptr(new SchedulerTask(delay, std::move(f))); +} diff --git a/src/tasks.cpp b/src/tasks.cpp index e72f231712..e73b2b19d9 100644 --- a/src/tasks.cpp +++ b/src/tasks.cpp @@ -30,7 +30,7 @@ void Dispatcher::threadMain() tmpTaskList.swap(taskList); taskLockUnique.unlock(); - for (auto &task : tmpTaskList) { + for (auto& task : tmpTaskList) { if (!task->hasExpired()) { ++dispatcherCycle; // execute it diff --git a/src/tasks.h b/src/tasks.h index 56f27f5273..87292d68a2 100644 --- a/src/tasks.h +++ b/src/tasks.h @@ -53,7 +53,7 @@ class Dispatcher : public ThreadHolder public: void addTask(Task_ptr task); - void addTask(TaskFunc&& f) { addTask( std::make_unique(std::move(f))); } + void addTask(TaskFunc&& f) { addTask(std::make_unique(std::move(f))); } void addTask(uint32_t expiration, TaskFunc&& f) { addTask(make_unique(expiration, std::move(f))); } From 3f233082e96acd764e99ec89890c29612835191d Mon Sep 17 00:00:00 2001 From: Artur Knopik Linux Date: Fri, 27 Dec 2024 11:20:39 +0100 Subject: [PATCH 6/7] review fixes --- src/game.cpp | 57 ++++++++++++++++++++++------------------------- src/player.cpp | 4 ++-- src/scheduler.cpp | 2 +- src/scheduler.h | 4 ++-- src/tasks.cpp | 2 +- src/tasks.h | 11 +++++---- 6 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/game.cpp b/src/game.cpp index 9f16a2a974..d27d9fc04b 100644 --- a/src/game.cpp +++ b/src/game.cpp @@ -634,7 +634,7 @@ void Game::playerMoveThing(uint32_t playerId, const Position& fromPos, uint16_t } if (movingCreature->getPosition().isInRange(player->getPosition(), 1, 1, 0)) { - SchedulerTask_ptr task = createSchedulerTask( + auto task = createSchedulerTask( MOVE_CREATURE_INTERVAL, [=, this, playerID = player->getID(), creatureID = movingCreature->getID()]() { playerMoveCreatureByID(playerID, creatureID, fromPos, toPos); }); @@ -703,12 +703,11 @@ void Game::playerMoveCreature(Player* player, Creature* movingCreature, const Po g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask_ptr task = - createSchedulerTask(RANGE_MOVE_CREATURE_INTERVAL, [=, this, playerID = player->getID(), - movingCreatureID = movingCreature->getID(), - toPos = toTile->getPosition()] { - playerMoveCreatureByID(playerID, movingCreatureID, movingCreatureOrigPos, toPos); - }); + auto task = createSchedulerTask(RANGE_MOVE_CREATURE_INTERVAL, [=, this, playerID = player->getID(), + movingCreatureID = movingCreature->getID(), + toPos = toTile->getPosition()] { + playerMoveCreatureByID(playerID, movingCreatureID, movingCreatureOrigPos, toPos); + }); player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); @@ -891,7 +890,7 @@ void Game::playerMoveItem(Player* player, const Position& fromPos, uint16_t spri { if (!player->canDoAction()) { uint32_t delay = player->getNextActionTime(); - SchedulerTask_ptr task = createSchedulerTask(delay, [=, this, playerID = player->getID()]() { + auto task = createSchedulerTask(delay, [=, this, playerID = player->getID()]() { playerMoveItemByPlayerID(playerID, fromPos, spriteId, fromStackPos, toPos, count); }); player->setNextActionTask(std::move(task)); @@ -960,10 +959,9 @@ void Game::playerMoveItem(Player* player, const Position& fromPos, uint16_t spri g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask_ptr task = - createSchedulerTask(RANGE_MOVE_ITEM_INTERVAL, [=, this, playerID = player->getID()]() { - playerMoveItemByPlayerID(playerID, fromPos, spriteId, fromStackPos, toPos, count); - }); + auto task = createSchedulerTask(RANGE_MOVE_ITEM_INTERVAL, [=, this, playerID = player->getID()]() { + playerMoveItemByPlayerID(playerID, fromPos, spriteId, fromStackPos, toPos, count); + }); player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); @@ -1022,11 +1020,10 @@ void Game::playerMoveItem(Player* player, const Position& fromPos, uint16_t spri g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask_ptr task = createSchedulerTask( - RANGE_MOVE_ITEM_INTERVAL, - [this, playerID = player->getID(), itemPos, spriteId, itemStackPos, toPos, count]() { - playerMoveItemByPlayerID(playerID, itemPos, spriteId, itemStackPos, toPos, count); - }); + auto task = createSchedulerTask(RANGE_MOVE_ITEM_INTERVAL, [this, playerID = player->getID(), itemPos, + spriteId, itemStackPos, toPos, count]() { + playerMoveItemByPlayerID(playerID, itemPos, spriteId, itemStackPos, toPos, count); + }); player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); @@ -2137,7 +2134,7 @@ void Game::playerUseItemEx(uint32_t playerId, const Position& fromPos, uint8_t f g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask_ptr task = createSchedulerTask(RANGE_USE_ITEM_EX_INTERVAL, [=, this]() { + auto task = createSchedulerTask(RANGE_USE_ITEM_EX_INTERVAL, [=, this]() { playerUseItemEx(playerId, itemPos, itemStackPos, fromSpriteId, toPos, toStackPos, toSpriteId); }); player->setNextWalkActionTask(std::move(task)); @@ -2153,7 +2150,7 @@ void Game::playerUseItemEx(uint32_t playerId, const Position& fromPos, uint8_t f if (!player->canDoAction()) { uint32_t delay = player->getNextActionTime(); - SchedulerTask_ptr task = createSchedulerTask(delay, [=, this]() { + auto task = createSchedulerTask(delay, [=, this]() { playerUseItemEx(playerId, fromPos, fromStackPos, fromSpriteId, toPos, toStackPos, toSpriteId); }); player->setNextActionTask(std::move(task)); @@ -2198,7 +2195,7 @@ void Game::playerUseItem(uint32_t playerId, const Position& pos, uint8_t stackPo g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask_ptr task = createSchedulerTask( + auto task = createSchedulerTask( RANGE_USE_ITEM_INTERVAL, [=, this]() { playerUseItem(playerId, pos, stackPos, index, spriteId); }); player->setNextWalkActionTask(std::move(task)); return; @@ -2213,7 +2210,7 @@ void Game::playerUseItem(uint32_t playerId, const Position& pos, uint8_t stackPo if (!player->canDoAction()) { uint32_t delay = player->getNextActionTime(); - SchedulerTask_ptr task = + auto task = createSchedulerTask(delay, [=, this]() { playerUseItem(playerId, pos, stackPos, index, spriteId); }); player->setNextActionTask(std::move(task)); return; @@ -2297,7 +2294,7 @@ void Game::playerUseWithCreature(uint32_t playerId, const Position& fromPos, uin g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask_ptr task = createSchedulerTask(RANGE_USE_WITH_CREATURE_INTERVAL, [=, this]() { + auto task = createSchedulerTask(RANGE_USE_WITH_CREATURE_INTERVAL, [=, this]() { playerUseWithCreature(playerId, itemPos, itemStackPos, creatureId, spriteId); }); player->setNextWalkActionTask(std::move(task)); @@ -2313,7 +2310,7 @@ void Game::playerUseWithCreature(uint32_t playerId, const Position& fromPos, uin if (!player->canDoAction()) { uint32_t delay = player->getNextActionTime(); - SchedulerTask_ptr task = createSchedulerTask( + auto task = createSchedulerTask( delay, [=, this]() { playerUseWithCreature(playerId, fromPos, fromStackPos, creatureId, spriteId); }); player->setNextActionTask(std::move(task)); return; @@ -2416,8 +2413,8 @@ void Game::playerRotateItem(uint32_t playerId, const Position& pos, uint8_t stac g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask_ptr task = createSchedulerTask( - RANGE_ROTATE_ITEM_INTERVAL, [=, this]() { playerRotateItem(playerId, pos, stackPos, spriteId); }); + auto task = createSchedulerTask(RANGE_ROTATE_ITEM_INTERVAL, + [=, this]() { playerRotateItem(playerId, pos, stackPos, spriteId); }); player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); @@ -2512,7 +2509,7 @@ void Game::playerBrowseField(uint32_t playerId, const Position& pos) g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask_ptr task = + auto task = createSchedulerTask(RANGE_BROWSE_FIELD_INTERVAL, [=, this]() { playerBrowseField(playerId, pos); }); player->setNextWalkActionTask(std::move(task)); } else { @@ -2618,8 +2615,8 @@ void Game::playerWrapItem(uint32_t playerId, const Position& position, uint8_t s g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask_ptr task = createSchedulerTask( - RANGE_WRAP_ITEM_INTERVAL, [=, this]() { playerWrapItem(playerId, position, stackPos, spriteId); }); + auto task = createSchedulerTask(RANGE_WRAP_ITEM_INTERVAL, + [=, this]() { playerWrapItem(playerId, position, stackPos, spriteId); }); player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); @@ -2690,7 +2687,7 @@ void Game::playerRequestTrade(uint32_t playerId, const Position& pos, uint8_t st g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask_ptr task = createSchedulerTask(RANGE_REQUEST_TRADE_INTERVAL, [=, this]() { + auto task = createSchedulerTask(RANGE_REQUEST_TRADE_INTERVAL, [=, this]() { playerRequestTrade(playerId, pos, stackPos, tradePlayerId, spriteId); }); player->setNextWalkActionTask(std::move(task)); @@ -3413,7 +3410,7 @@ void Game::playerRequestEditPodium(uint32_t playerId, const Position& position, g_dispatcher.addTask([this, playerID = player->getID(), listDir = std::move(listDir)]() { playerAutoWalk(playerID, listDir); }); - SchedulerTask_ptr task = createSchedulerTask( + auto task = createSchedulerTask( 400, [=, this]() { playerRequestEditPodium(playerId, position, stackPos, spriteId); }); player->setNextWalkActionTask(std::move(task)); } else { diff --git a/src/player.cpp b/src/player.cpp index 1a61059e9f..39cf51eb6b 100644 --- a/src/player.cpp +++ b/src/player.cpp @@ -3406,8 +3406,8 @@ void Player::doAttacking(uint32_t) result = Weapon::useFist(this, attackedCreature); } - SchedulerTask_ptr task = createSchedulerTask(std::max(SCHEDULER_MINTICKS, delay), - [id = getID()]() { g_game.checkCreatureAttack(id); }); + auto task = createSchedulerTask(std::max(SCHEDULER_MINTICKS, delay), + [id = getID()]() { g_game.checkCreatureAttack(id); }); if (!classicSpeed) { setNextActionTask(std::move(task), false); } else { diff --git a/src/scheduler.cpp b/src/scheduler.cpp index ad9e7b898d..5b1ca1a241 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -65,5 +65,5 @@ void Scheduler::shutdown() SchedulerTask_ptr createSchedulerTask(uint32_t delay, TaskFunc&& f) { - return SchedulerTask_ptr(new SchedulerTask(delay, std::move(f))); + return std::make_unique(delay, std::forward(f)); } diff --git a/src/scheduler.h b/src/scheduler.h index 4df141c31a..8677e5b5d8 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -16,14 +16,14 @@ using SchedulerTask_ptr = std::unique_ptr; class SchedulerTask : public Task { public: + SchedulerTask(uint32_t delay, TaskFunc&& f) : Task(std::forward(f)), delay(delay) {} + void setEventId(uint32_t id) { eventId = id; } uint32_t getEventId() const { return eventId; } uint32_t getDelay() const { return delay; } private: - SchedulerTask(uint32_t delay, TaskFunc&& f) : Task(std::move(f)), delay(delay) {} - uint32_t eventId = 0; uint32_t delay = 0; diff --git a/src/tasks.cpp b/src/tasks.cpp index e73b2b19d9..bac5a97fe7 100644 --- a/src/tasks.cpp +++ b/src/tasks.cpp @@ -34,7 +34,7 @@ void Dispatcher::threadMain() if (!task->hasExpired()) { ++dispatcherCycle; // execute it - (*task)(); + std::invoke(*task); } } tmpTaskList.clear(); diff --git a/src/tasks.h b/src/tasks.h index 87292d68a2..04625c3904 100644 --- a/src/tasks.h +++ b/src/tasks.h @@ -18,9 +18,9 @@ class Task { public: // DO NOT allocate this class on the stack - explicit Task(TaskFunc&& f) : func(std::move(f)) {} + explicit Task(TaskFunc&& f) : func(std::forward(f)) {} Task(uint32_t ms, TaskFunc&& f) : - expiration(std::chrono::system_clock::now() + std::chrono::milliseconds(ms)), func(std::move(f)) + expiration(std::chrono::system_clock::now() + std::chrono::milliseconds(ms)), func(std::forward(f)) {} virtual ~Task() = default; @@ -53,9 +53,12 @@ class Dispatcher : public ThreadHolder public: void addTask(Task_ptr task); - void addTask(TaskFunc&& f) { addTask(std::make_unique(std::move(f))); } + void addTask(TaskFunc&& f) { addTask(std::make_unique(std::forward(f))); } - void addTask(uint32_t expiration, TaskFunc&& f) { addTask(make_unique(expiration, std::move(f))); } + void addTask(uint32_t expiration, TaskFunc&& f) + { + addTask(std::make_unique(expiration, std::forward(f))); + } void shutdown(); From 869945117413aede29ddf95dbd84b860302ef6ff Mon Sep 17 00:00:00 2001 From: KrecikOnDexin Date: Mon, 30 Dec 2024 13:50:50 +0100 Subject: [PATCH 7/7] up --- src/scheduler.h | 8 +++----- src/tasks.h | 4 +--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/scheduler.h b/src/scheduler.h index 8677e5b5d8..dd4151bd19 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -7,11 +7,11 @@ #include "tasks.h" #include "thread_holder_base.h" -static constexpr int32_t SCHEDULER_MINTICKS = 50; +#include -class SchedulerTask; +static constexpr int32_t SCHEDULER_MINTICKS = 50; -using SchedulerTask_ptr = std::unique_ptr; +using SchedulerTask_ptr = std::unique_ptr; class SchedulerTask : public Task { @@ -26,8 +26,6 @@ class SchedulerTask : public Task private: uint32_t eventId = 0; uint32_t delay = 0; - - friend SchedulerTask_ptr createSchedulerTask(uint32_t delay, TaskFunc&& f); }; SchedulerTask_ptr createSchedulerTask(uint32_t delay, TaskFunc&& f); diff --git a/src/tasks.h b/src/tasks.h index 04625c3904..f0f1c0ac58 100644 --- a/src/tasks.h +++ b/src/tasks.h @@ -10,9 +10,7 @@ using TaskFunc = std::function; const int DISPATCHER_TASK_EXPIRATION = 2000; const auto SYSTEM_TIME_ZERO = std::chrono::system_clock::time_point(std::chrono::milliseconds(0)); -class Task; - -using Task_ptr = std::unique_ptr; +using Task_ptr = std::unique_ptr; class Task {