From dec87cb0c6077032f9c95615e8381c6e61e73545 Mon Sep 17 00:00:00 2001 From: Renato Machado Date: Mon, 9 Dec 2024 16:29:12 -0300 Subject: [PATCH] Improve: creature actions (#3084) Some creature actions, such as checkCreatureAttack, checkCreatureWalk, updateCreatureWalk, were processed based on the creature ID, as canary works with smart pointer, we no longer have problems working with the object reference, before it used the ID to maintain security, because at the time of execution, the object could no longer exist. --- src/creatures/creature.cpp | 50 ++++++++++++++++++++++++------ src/creatures/creature.hpp | 6 ++++ src/creatures/monsters/monster.cpp | 2 +- src/creatures/players/player.cpp | 11 ++++--- src/game/game.cpp | 34 ++------------------ src/game/game.hpp | 4 --- src/game/scheduling/task.hpp | 3 +- 7 files changed, 57 insertions(+), 53 deletions(-) diff --git a/src/creatures/creature.cpp b/src/creatures/creature.cpp index 32f3c598b81..586f5b8c1b2 100644 --- a/src/creatures/creature.cpp +++ b/src/creatures/creature.cpp @@ -140,6 +140,20 @@ void Creature::onThink(uint32_t interval) { onThink(); } +void Creature::checkCreatureAttack(bool now) { + if (now) { + if (isAlive()) { + onAttacking(0); + } + return; + } + + g_dispatcher().addEvent([self = std::weak_ptr(getCreature())] { + if (const auto &creature = self.lock()) { + creature->checkCreatureAttack(true); + } }, "Creature::checkCreatureAttack"); +} + void Creature::onAttacking(uint32_t interval) { const auto &attackedCreature = getAttackedCreature(); if (!attackedCreature) { @@ -162,7 +176,7 @@ void Creature::onIdleStatus() { } void Creature::onCreatureWalk() { - if (checkingWalkCreature) { + if (checkingWalkCreature || isRemoved() || isDead()) { return; } @@ -170,7 +184,11 @@ void Creature::onCreatureWalk() { metrics::method_latency measure(__METRICS_METHOD_NAME__); - g_dispatcher().addWalkEvent([self = getCreature(), this] { + g_dispatcher().addWalkEvent([self = std::weak_ptr(getCreature()), this] { + if (!self.lock()) { + return; + } + checkingWalkCreature = false; if (isRemoved()) { return; @@ -269,12 +287,16 @@ void Creature::addEventWalk(bool firstStep) { safeCall([this, ticks]() { // Take first step right away, but still queue the next if (ticks == 1) { - g_game().checkCreatureWalk(getID()); + onCreatureWalk(); } eventWalk = g_dispatcher().scheduleEvent( - static_cast(ticks), - [creatureId = getID()] { g_game().checkCreatureWalk(creatureId); }, "Game::checkCreatureWalk" + static_cast(ticks), [self = std::weak_ptr(getCreature())] { + if (const auto &creature = self.lock()) { + creature->onCreatureWalk(); + } + }, + "Game::checkCreatureWalk" ); }); } @@ -421,7 +443,7 @@ void Creature::onCreatureMove(const std::shared_ptr &creature, const s if (followCreature && (creature.get() == this || creature == followCreature)) { if (hasFollowPath) { isUpdatingPath = true; - g_game().updateCreatureWalk(getID()); // internally uses addEventWalk. + updateCreatureWalk(); } if (newPos.z != oldPos.z || !canSee(followCreature->getPosition())) { @@ -436,7 +458,7 @@ void Creature::onCreatureMove(const std::shared_ptr &creature, const s } else { if (hasExtraSwing()) { // our target is moving lets see if we can get in hit - g_dispatcher().addEvent([creatureId = getID()] { g_game().checkCreatureAttack(creatureId); }, "Game::checkCreatureAttack"); + checkCreatureAttack(); } if (newTile->getZoneType() != oldTile->getZoneType()) { @@ -701,7 +723,13 @@ void Creature::changeHealth(int32_t healthChange, bool sendHealthChange /* = tru g_game().addCreatureHealth(static_self_cast()); } if (health <= 0) { - g_dispatcher().addEvent([creatureId = getID()] { g_game().executeDeath(creatureId); }, "Game::executeDeath"); + g_dispatcher().addEvent([self = std::weak_ptr(getCreature())] { + if (const auto &creature = self.lock()) { + if (!creature->isRemoved()) { + g_game().afterCreatureZoneChange(creature, creature->getZones(), {}); + creature->onDeath(); + } + } }, "Game::executeDeath"); } } @@ -874,6 +902,10 @@ void Creature::getPathSearchParams(const std::shared_ptr &, FindPathPa } void Creature::goToFollowCreature_async(std::function &&onComplete) { + if (isDead()) { + return; + } + if (!hasAsyncTaskFlag(Pathfinder) && onComplete) { g_dispatcher().addEvent(std::move(onComplete), "goToFollowCreature_async"); } @@ -1781,7 +1813,7 @@ void Creature::sendAsyncTasks() { setAsyncTaskFlag(AsyncTaskRunning, true); g_dispatcher().asyncEvent([self = std::weak_ptr(getCreature())] { if (const auto &creature = self.lock()) { - if (!creature->isRemoved()) { + if (!creature->isRemoved() && creature->isAlive()) { for (const auto &task : creature->asyncTasks) { task(); } diff --git a/src/creatures/creature.hpp b/src/creatures/creature.hpp index fcea0a6b0f2..18a4bd817ab 100644 --- a/src/creatures/creature.hpp +++ b/src/creatures/creature.hpp @@ -312,6 +312,9 @@ class Creature : virtual public Thing, public SharedObject { void addEventWalk(bool firstStep = false); void stopEventWalk(); + void updateCreatureWalk() { + goToFollowCreature_async(); + } void goToFollowCreature_async(std::function &&onComplete = nullptr); virtual void goToFollowCreature(); @@ -482,6 +485,9 @@ class Creature : virtual public Thing, public SharedObject { void setCreatureLight(LightInfo lightInfo); virtual void onThink(uint32_t interval); + + void checkCreatureAttack(bool now = false); + void onAttacking(uint32_t interval); virtual void onCreatureWalk(); virtual bool getNextStep(Direction &dir, uint32_t &flags); diff --git a/src/creatures/monsters/monster.cpp b/src/creatures/monsters/monster.cpp index 72816fd1a43..4189ffc6c29 100644 --- a/src/creatures/monsters/monster.cpp +++ b/src/creatures/monsters/monster.cpp @@ -954,7 +954,7 @@ bool Monster::selectTarget(const std::shared_ptr &creature) { if (isHostile() || isSummon()) { if (setAttackedCreature(creature)) { - g_dispatcher().addEvent([creatureId = getID()] { g_game().checkCreatureAttack(creatureId); }, __FUNCTION__); + checkCreatureAttack(); } } return setFollowCreature(creature); diff --git a/src/creatures/players/player.cpp b/src/creatures/players/player.cpp index 4335db2bd53..88223534497 100644 --- a/src/creatures/players/player.cpp +++ b/src/creatures/players/player.cpp @@ -3430,9 +3430,10 @@ void Player::doAttacking(uint32_t interval) { } const auto &task = createPlayerTask( - std::max(SCHEDULER_MINTICKS, delay), - [playerId = getID()] { g_game().checkCreatureAttack(playerId); }, - __FUNCTION__ + std::max(SCHEDULER_MINTICKS, delay), [self = std::weak_ptr(getCreature())] { + if (const auto &creature = self.lock()) { + creature->checkCreatureAttack(true); + } }, __FUNCTION__ ); if (!classicSpeed) { @@ -5296,7 +5297,7 @@ bool Player::setAttackedCreature(const std::shared_ptr &creature) { } if (creature) { - g_dispatcher().addEvent([creatureId = getID()] { g_game().checkCreatureAttack(creatureId); }, __FUNCTION__); + checkCreatureAttack(); } return true; } @@ -9835,7 +9836,7 @@ void Player::onCreatureMove(const std::shared_ptr &creature, const std const auto &followCreature = getFollowCreature(); if (hasFollowPath && (creature == followCreature || (creature.get() == this && followCreature))) { isUpdatingPath = false; - g_game().updateCreatureWalk(getID()); // internally uses addEventWalk. + updateCreatureWalk(); } if (creature != getPlayer()) { diff --git a/src/game/game.cpp b/src/game/game.cpp index 7a492348649..4dc0b9e3e20 100644 --- a/src/game/game.cpp +++ b/src/game/game.cpp @@ -1255,15 +1255,6 @@ bool Game::removeCreature(const std::shared_ptr &creature, bool isLogo return true; } -void Game::executeDeath(uint32_t creatureId) { - metrics::method_latency measure(__METRICS_METHOD_NAME__); - std::shared_ptr creature = getCreatureByID(creatureId); - if (creature && !creature->isRemoved()) { - afterCreatureZoneChange(creature, creature->getZones(), {}); - creature->onDeath(); - } -} - void Game::playerTeleport(uint32_t playerId, const Position &newPosition) { metrics::method_latency measure(__METRICS_METHOD_NAME__); const auto &player = getPlayerByID(playerId); @@ -5912,7 +5903,7 @@ void Game::playerSetAttackedCreature(uint32_t playerId, uint32_t creatureId) { } player->setAttackedCreature(attackCreature); - updateCreatureWalk(player->getID()); // internally uses addEventWalk. + player->updateCreatureWalk(); } void Game::playerFollowCreature(uint32_t playerId, uint32_t creatureId) { @@ -5922,7 +5913,7 @@ void Game::playerFollowCreature(uint32_t playerId, uint32_t creatureId) { } player->setAttackedCreature(nullptr); - updateCreatureWalk(player->getID()); // internally uses addEventWalk. + player->updateCreatureWalk(); player->setFollowCreature(getCreatureByID(creatureId)); } @@ -6433,27 +6424,6 @@ bool Game::internalCreatureSay(const std::shared_ptr &creature, SpeakC return true; } -void Game::checkCreatureWalk(uint32_t creatureId) { - const auto &creature = getCreatureByID(creatureId); - if (creature && creature->getHealth() > 0) { - creature->onCreatureWalk(); - } -} - -void Game::updateCreatureWalk(uint32_t creatureId) { - const auto &creature = getCreatureByID(creatureId); - if (creature && creature->getHealth() > 0) { - creature->goToFollowCreature_async(); - } -} - -void Game::checkCreatureAttack(uint32_t creatureId) { - const auto &creature = getCreatureByID(creatureId); - if (creature && creature->getHealth() > 0) { - creature->onAttacking(0); - } -} - void Game::addCreatureCheck(const std::shared_ptr &creature) { if (creature->isRemoved()) { return; diff --git a/src/game/game.hpp b/src/game/game.hpp index b1afd810100..ff2e127fd25 100644 --- a/src/game/game.hpp +++ b/src/game/game.hpp @@ -173,7 +173,6 @@ class Game { bool placeCreature(const std::shared_ptr &creature, const Position &pos, bool extendedPos = false, bool force = false); bool removeCreature(const std::shared_ptr &creature, bool isLogout = true); - void executeDeath(uint32_t creatureId); void addCreatureCheck(const std::shared_ptr &creature); static void removeCreatureCheck(const std::shared_ptr &creature); @@ -437,9 +436,6 @@ class Game { void setGameState(GameState_t newState); // Events - void checkCreatureWalk(uint32_t creatureId); - void updateCreatureWalk(uint32_t creatureId); - void checkCreatureAttack(uint32_t creatureId); void checkCreatures(); void checkLight(); diff --git a/src/game/scheduling/task.hpp b/src/game/scheduling/task.hpp index 6cd9eef342d..c4de64059bb 100644 --- a/src/game/scheduling/task.hpp +++ b/src/game/scheduling/task.hpp @@ -66,14 +66,13 @@ class Task { const static std::unordered_set tasksContext = { "Decay::checkDecay", "Dispatcher::asyncEvent", - "Game::checkCreatureAttack", + "Creature::checkCreatureAttack", "Game::checkCreatureWalk", "Game::checkCreatures", "Game::checkImbuements", "Game::checkLight", "Game::createFiendishMonsters", "Game::createInfluencedMonsters", - "Game::updateCreatureWalk", "Game::updateForgeableMonsters", "Game::addCreatureCheck", "GlobalEvents::think",