Skip to content

Commit

Permalink
style: tryAddEvent to safeCall (opentibiabr#3045)
Browse files Browse the repository at this point in the history
safeCall basically keeps calls safe when they are being executed
asynchronously, that is, if the calls inside safeCall are being executed
in a thread other than the dispatcher, they will simply be sent to that
thread, thus avoiding concurrency.

What I just wrote is already being done with tryAddEvent, but it is not
a very intuitive name, and it is visually polluted. In the safeCall
method, besides being short, you can simply send 'this' as a parameter
to the lambda, without having to fear that its execution will be done
when the reference no longer exists, because inside safeCall, there is a
reference check, if it has already been destroyed, it will simply not be
executed.
  • Loading branch information
mehah authored Nov 7, 2024
1 parent 3bef034 commit 4ae6bdf
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 49 deletions.
26 changes: 19 additions & 7 deletions src/creatures/creature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,18 +270,17 @@ void Creature::addEventWalk(bool firstStep) {
return;
}

g_dispatcher().context().tryAddEvent([ticks, self = getCreature()]() {
safeCall([this, ticks]() {
// Take first step right away, but still queue the next
if (ticks == 1) {
g_game().checkCreatureWalk(self->getID());
g_game().checkCreatureWalk(getID());
}

self->eventWalk = g_dispatcher().scheduleEvent(
eventWalk = g_dispatcher().scheduleEvent(
static_cast<uint32_t>(ticks),
[creatureId = self->getID()] { g_game().checkCreatureWalk(creatureId); }, "Game::checkCreatureWalk"
[creatureId = getID()] { g_game().checkCreatureWalk(creatureId); }, "Game::checkCreatureWalk"
);
},
"addEventWalk");
});
}

void Creature::stopEventWalk() {
Expand Down Expand Up @@ -1082,7 +1081,7 @@ void Creature::getPathSearchParams(const std::shared_ptr<Creature> &, FindPathPa

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

setAsyncTaskFlag(Pathfinder, true);
Expand Down Expand Up @@ -2002,3 +2001,16 @@ void Creature::sendAsyncTasks() {
},
TaskGroup::WalkParallel);
}

void Creature::safeCall(std::function<void(void)> &&action) const {
if (g_dispatcher().context().isAsync()) {
g_dispatcher().addEvent([weak_self = std::weak_ptr<const SharedObject>(shared_from_this()), action = std::move(action)] {
if (weak_self.lock()) {
action();
}
},
g_dispatcher().context().getName());
} else {
action();
}
}
3 changes: 3 additions & 0 deletions src/creatures/creature.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,9 @@ class Creature : virtual public Thing, public SharedObject {

virtual void onExecuteAsyncTasks() {};

// This method maintains safety in asynchronous calls, avoiding competition between threads.
void safeCall(std::function<void(void)> &&action) const;

private:
bool canFollowMaster() const;
bool isLostSummon();
Expand Down
11 changes: 3 additions & 8 deletions src/creatures/players/player.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3783,14 +3783,9 @@ void Player::addInFightTicks(bool pzlock /*= false*/) {

updateImbuementTrackerStats();

// this method can be called asynchronously.
g_dispatcher().context().tryAddEvent([self = std::weak_ptr<Player>(getPlayer())] {
if (const auto &player = self.lock()) {
const auto &condition = Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_INFIGHT, g_configManager().getNumber(PZ_LOCKED), 0);
player->addCondition(condition);
}
},
"Player::addInFightTicks");
safeCall([this] {
addCondition(Condition::createCondition(CONDITIONID_DEFAULT, CONDITION_INFIGHT, g_configManager().getNumber(PZ_LOCKED), 0));
});
}

void Player::setDailyReward(uint8_t reward) {
Expand Down
5 changes: 2 additions & 3 deletions src/game/game.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6440,10 +6440,9 @@ void Game::addCreatureCheck(const std::shared_ptr<Creature> &creature) {

creature->inCheckCreaturesVector.store(true);

g_dispatcher().context().tryAddEvent([creature] {
creature->safeCall([this, creature] {
checkCreatureLists[uniform_random(0, EVENT_CREATURECOUNT - 1)].emplace_back(creature);
},
"addCreatureCheck");
});
}

void Game::removeCreatureCheck(const std::shared_ptr<Creature> &creature) {
Expand Down
16 changes: 0 additions & 16 deletions src/game/scheduling/dispatcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,22 +251,6 @@ void Dispatcher::stopEvent(uint64_t eventId) {
}
}

void DispatcherContext::addEvent(std::function<void(void)> &&f, std::string_view context) const {
g_dispatcher().addEvent(std::move(f), context);
}

void DispatcherContext::tryAddEvent(std::function<void(void)> &&f, std::string_view context) const {
if (!f) {
return;
}

if (isAsync()) {
g_dispatcher().addEvent(std::move(f), context);
} else {
f();
}
}

bool DispatcherContext::isOn() {
return OTSYS_TIME() != 0;
}
6 changes: 0 additions & 6 deletions src/game/scheduling/dispatcher.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,6 @@ struct DispatcherContext {
return type;
}

// postpone the event
void addEvent(std::function<void(void)> &&f, std::string_view context) const;

// if the context is async, the event will be postponed, if not, it will be executed immediately.
void tryAddEvent(std::function<void(void)> &&f, std::string_view context) const;

private:
void reset() {
group = TaskGroup::ThreadPool;
Expand Down
14 changes: 14 additions & 0 deletions src/items/tile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "lua/creature/movement.hpp"
#include "map/spectators.hpp"
#include "utils/tools.hpp"
#include "game/scheduling/dispatcher.hpp"

auto real_nullptr_tile = std::make_shared<StaticTile>(0xFFFF, 0xFFFF, 0xFF);
const std::shared_ptr<Tile> &Tile::nullptr_tile = real_nullptr_tile;
Expand Down Expand Up @@ -1942,3 +1943,16 @@ void Tile::clearZones() {
zones.erase(zone);
}
}

void Tile::safeCall(std::function<void(void)> &&action) const {
if (g_dispatcher().context().isAsync()) {
g_dispatcher().addEvent([weak_self = std::weak_ptr<const SharedObject>(shared_from_this()), action = std::move(action)] {
if (weak_self.lock()) {
action();
}
},
g_dispatcher().context().getName());
} else {
action();
}
}
3 changes: 3 additions & 0 deletions src/items/tile.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@ class Tile : public Cylinder, public SharedObject {
}
}

// This method maintains safety in asynchronous calls, avoiding competition between threads.
void safeCall(std::function<void(void)> &&action) const;

private:
void onAddTileItem(const std::shared_ptr<Item> &item);
void onUpdateTileItem(const std::shared_ptr<Item> &oldItem, const ItemType &oldType, const std::shared_ptr<Item> &newItem, const ItemType &newType);
Expand Down
14 changes: 5 additions & 9 deletions src/map/mapcache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,11 @@ std::shared_ptr<Tile> MapCache::getOrCreateTileFromCache(const std::shared_ptr<F

tile->setFlag(static_cast<TileFlags_t>(cachedTile->flags));

// add zone synchronously
g_dispatcher().context().tryAddEvent(
[tile, pos] {
for (const auto &zone : Zone::getZones(pos)) {
tile->addZone(zone);
}
},
"Zone::getZones"
);
tile->safeCall([tile, pos] {
for (const auto &zone : Zone::getZones(pos)) {
tile->addZone(zone);
}
});

floor->setTile(x, y, tile);

Expand Down

0 comments on commit 4ae6bdf

Please sign in to comment.