diff --git a/src/game.cpp b/src/game.cpp index 9a871adb52..d27d9fc04b 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( + auto 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,12 @@ 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 = - createSchedulerTask(RANGE_MOVE_CREATURE_INTERVAL, [=, this, playerID = player->getID(), - movingCreatureID = movingCreature->getID(), - toPos = toTile->getPosition()] { - playerMoveCreatureByID(playerID, movingCreatureID, movingCreatureOrigPos, toPos); - }); - player->setNextWalkActionTask(task); + 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,10 +890,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()]() { + auto 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 +959,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* task = - createSchedulerTask(RANGE_MOVE_ITEM_INTERVAL, [=, this, playerID = player->getID()]() { - playerMoveItemByPlayerID(playerID, fromPos, spriteId, fromStackPos, toPos, count); - }); - player->setNextWalkActionTask(task); + 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,12 +1020,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 = createSchedulerTask( - RANGE_MOVE_ITEM_INTERVAL, - [this, playerID = player->getID(), itemPos, spriteId, itemStackPos, toPos, count]() { - playerMoveItemByPlayerID(playerID, itemPos, spriteId, itemStackPos, toPos, count); - }); - player->setNextWalkActionTask(task); + 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,10 +2134,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]() { + auto 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 +2150,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]() { + auto task = createSchedulerTask(delay, [=, this]() { playerUseItemEx(playerId, fromPos, fromStackPos, fromSpriteId, toPos, toStackPos, toSpriteId); }); - player->setNextActionTask(task); + player->setNextActionTask(std::move(task)); return; } @@ -2198,9 +2195,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( + auto task = createSchedulerTask( RANGE_USE_ITEM_INTERVAL, [=, this]() { playerUseItem(playerId, pos, stackPos, index, spriteId); }); - player->setNextWalkActionTask(task); + player->setNextWalkActionTask(std::move(task)); return; } @@ -2213,9 +2210,9 @@ void Game::playerUseItem(uint32_t playerId, const Position& pos, uint8_t stackPo if (!player->canDoAction()) { uint32_t delay = player->getNextActionTime(); - SchedulerTask* task = + auto task = createSchedulerTask(delay, [=, this]() { playerUseItem(playerId, pos, stackPos, index, spriteId); }); - player->setNextActionTask(task); + player->setNextActionTask(std::move(task)); return; } @@ -2297,10 +2294,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]() { + auto 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 +2310,9 @@ void Game::playerUseWithCreature(uint32_t playerId, const Position& fromPos, uin if (!player->canDoAction()) { uint32_t delay = player->getNextActionTime(); - SchedulerTask* task = createSchedulerTask( + auto task = createSchedulerTask( delay, [=, this]() { playerUseWithCreature(playerId, fromPos, fromStackPos, creatureId, spriteId); }); - player->setNextActionTask(task); + player->setNextActionTask(std::move(task)); return; } @@ -2416,9 +2413,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( - RANGE_ROTATE_ITEM_INTERVAL, [=, this]() { playerRotateItem(playerId, pos, stackPos, spriteId); }); - player->setNextWalkActionTask(task); + auto task = createSchedulerTask(RANGE_ROTATE_ITEM_INTERVAL, + [=, this]() { playerRotateItem(playerId, pos, stackPos, spriteId); }); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } @@ -2512,9 +2509,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 = + auto 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 +2615,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( - RANGE_WRAP_ITEM_INTERVAL, [=, this]() { playerWrapItem(playerId, position, stackPos, spriteId); }); - player->setNextWalkActionTask(task); + auto task = createSchedulerTask(RANGE_WRAP_ITEM_INTERVAL, + [=, this]() { playerWrapItem(playerId, position, stackPos, spriteId); }); + player->setNextWalkActionTask(std::move(task)); } else { player->sendCancelMessage(RETURNVALUE_THEREISNOWAY); } @@ -2690,10 +2687,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]() { + auto 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 +3410,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( + auto 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..39cf51eb6b 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), - [id = getID()]() { g_game.checkCreatureAttack(id); }); + auto 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..c1b97d27c4 100644 --- a/src/player.h +++ b/src/player.h @@ -13,6 +13,7 @@ #include "guild.h" #include "inbox.h" #include "protocolgame.h" +#include "scheduler.h" #include "town.h" #include "vocation.h" @@ -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..5b1ca1a241 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -5,33 +5,34 @@ #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; - 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(task); - }); + g_dispatcher.addTask(std::move(scheduledTask)); + }); }); - return task->getEventId(); + return eventId; } void Scheduler::stopEvent(uint32_t eventId) @@ -62,4 +63,7 @@ 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 std::make_unique(delay, std::forward(f)); +} diff --git a/src/scheduler.h b/src/scheduler.h index 221c3c1b6c..dd4151bd19 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -7,31 +7,33 @@ #include "tasks.h" #include "thread_holder_base.h" +#include + static constexpr int32_t SCHEDULER_MINTICKS = 50; +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; - - friend SchedulerTask* createSchedulerTask(uint32_t, TaskFunc&&); }; -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..bac5a97fe7 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)(); + std::invoke(*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..f0f1c0ac58 100644 --- a/src/tasks.h +++ b/src/tasks.h @@ -10,13 +10,15 @@ 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)); +using Task_ptr = std::unique_ptr; + 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; @@ -41,17 +43,20 @@ 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::forward(f))); } - void addTask(uint32_t expiration, TaskFunc&& f) { addTask(new Task(expiration, std::move(f))); } + void addTask(uint32_t expiration, TaskFunc&& f) + { + addTask(std::make_unique(expiration, std::forward(f))); + } void shutdown(); @@ -63,7 +68,7 @@ class Dispatcher : public ThreadHolder std::mutex taskLock; std::condition_variable taskSignal; - std::vector taskList; + std::vector taskList; uint64_t dispatcherCycle = 0; };