Skip to content

Commit

Permalink
refactor: spectators system (#1645)
Browse files Browse the repository at this point in the history
This refactoring consists of:

1- Improved caching algorithm
2- Applied pipeline concept
3- Use of vector_set instead of hashset.

**code sample:**

```c++
// [OLD]
SpectatorHashSet spectators;
getSpectators(spectators, Position(x, y, z), ...);
for(const auto spec: spectators) {}
```

```c++
// [NEW]

// Get All Creatures
Spectators().find<Creature>(Position(x, y, z), ...);

// Get All Players
Spectators().find<Player>(Position(x, y, z), ...);

// Method Filter
// Note: This method returns a new instance of spectators
{
  auto spectators = Spectators().find<Creature>(Position(x, y, z), ...);
  
  // Get All Npcs from spectators variable
  auto npcSpectators = spectators.filter<Npc>();
  
  // Get All Players from spectators variable
  auto playerSpectators = spectators.filter<Player>();
}

// Pipeline Code
{
  auto spectators = Spectators()
    .find<Creature>({100, 200, 100})
    .find<Player>({110, 210, 110})
    .find<Creature>({99, 101, 50});
  
  for(const auto spec : spectators) {}
  for(const auto spec : spectators.filter<Monster>()) {}
  for(const auto spec : spectators.filter<Npc>()) {}
}
```

**[BENCHMARK]** (Set SPECTATORS_USE_HASHSET if you want to benchmark
with 'hashset'.)
<details>

Vector

![image](https://github.com/opentibiabr/canary/assets/2267386/a74564c9-ee99-4dc8-a290-c1420a394bf4)

Hashset

![hashset_cache](https://github.com/opentibiabr/canary/assets/2267386/468a2c1e-bdf7-4c3b-8df7-efbed40b4ddf)
</details>

**[BENCHMARK CODE]**
<details>

```C++
void Map::moveCreature(Creature &creature, Tile &newTile, bool forceTeleport /* = false*/) {
	.
	.
	.
	auto spectators = Spectators()
				  .find<Creature>(oldPos, true)
				  .find<Creature>(newPos, true);

	const int64_t start = OTSYS_TIME();

	for (int_fast32_t i = -1; ++i < 999999;) {
		for (const auto spec : Spectators()
						.find<Creature>(oldPos, true)
						.find<Creature>(newPos, true)) { }
	}

	g_logger().info("Benchmark std::vector({}) | cache enabled: {}ms", spectators.size(), static_cast<double>(OTSYS_TIME() - start) / 1000.f);
	.
	.
	.
}
```
</details>

---------

Co-authored-by: GitHub Actions <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Luan Santos <[email protected]>
  • Loading branch information
3 people authored Oct 3, 2023
1 parent 653648f commit 1e1fd0c
Show file tree
Hide file tree
Showing 33 changed files with 742 additions and 681 deletions.
18 changes: 8 additions & 10 deletions src/creatures/combat/combat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "creatures/monsters/monster.hpp"
#include "creatures/monsters/monsters.hpp"
#include "items/weapons/weapons.hpp"
#include "map/spectators.hpp"

int32_t Combat::getLevelFormula(std::shared_ptr<Player> player, const std::shared_ptr<Spell> wheelSpell, const CombatDamage &damage) const {
if (!player) {
Expand Down Expand Up @@ -754,7 +755,7 @@ void Combat::CombatNullFunc(std::shared_ptr<Creature> caster, std::shared_ptr<Cr
CombatDispelFunc(caster, target, params, nullptr);
}

void Combat::combatTileEffects(const SpectatorHashSet &spectators, std::shared_ptr<Creature> caster, std::shared_ptr<Tile> tile, const CombatParams &params) {
void Combat::combatTileEffects(const CreatureVector &spectators, std::shared_ptr<Creature> caster, std::shared_ptr<Tile> tile, const CombatParams &params) {
if (params.itemId != 0) {
uint16_t itemId = params.itemId;
switch (itemId) {
Expand Down Expand Up @@ -998,7 +999,6 @@ void Combat::CombatFunc(std::shared_ptr<Creature> caster, const Position &origin
getCombatArea(pos, pos, area, tileList);
}

SpectatorHashSet spectators;
uint32_t maxX = 0;
uint32_t maxY = 0;

Expand All @@ -1019,7 +1019,6 @@ void Combat::CombatFunc(std::shared_ptr<Creature> caster, const Position &origin

const int32_t rangeX = maxX + MAP_MAX_VIEW_PORT_X;
const int32_t rangeY = maxY + MAP_MAX_VIEW_PORT_Y;
g_game().map.getSpectators(spectators, pos, true, true, rangeX, rangeX, rangeY, rangeY);

int affected = 0;
for (std::shared_ptr<Tile> tile : tileList) {
Expand Down Expand Up @@ -1071,6 +1070,7 @@ void Combat::CombatFunc(std::shared_ptr<Creature> caster, const Position &origin
}

// Wheel of destiny get beam affected total
auto spectators = Spectators().find<Player>(pos, true, rangeX, rangeX, rangeY, rangeY);
std::shared_ptr<Player> casterPlayer = caster ? caster->getPlayer() : nullptr;
uint8_t beamAffectedTotal = casterPlayer ? casterPlayer->wheel()->getBeamAffectedTotal(tmpDamage) : 0;
uint8_t beamAffectedCurrent = 0;
Expand Down Expand Up @@ -1110,7 +1110,7 @@ void Combat::CombatFunc(std::shared_ptr<Creature> caster, const Position &origin
}
}
}
combatTileEffects(spectators, caster, tile, params);
combatTileEffects(spectators.data(), caster, tile, params);
}

// Wheel of destiny update beam mastery damage
Expand Down Expand Up @@ -1339,11 +1339,10 @@ void Combat::doCombatDefault(std::shared_ptr<Creature> caster, std::shared_ptr<C

void Combat::doCombatDefault(std::shared_ptr<Creature> caster, std::shared_ptr<Creature> target, const Position &origin, const CombatParams &params) {
if (!params.aggressive || (caster != target && Combat::canDoCombat(caster, target, params.aggressive) == RETURNVALUE_NOERROR)) {
SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, target->getPosition(), true, true);
auto spectators = Spectators().find<Player>(target->getPosition(), true);

CombatNullFunc(caster, target, params, nullptr);
combatTileEffects(spectators, caster, target->getTile(), params);
combatTileEffects(spectators.data(), caster, target->getTile(), params);

if (params.targetCallback) {
params.targetCallback->onTargetCombat(caster, target);
Expand Down Expand Up @@ -1396,13 +1395,12 @@ std::vector<std::pair<Position, std::vector<uint32_t>>> Combat::pickChainTargets
const int maxBacktrackingAttempts = 10; // Can be adjusted as needed
while (!targets.empty() && targets.size() <= maxTargets) {
auto currentTarget = targets.back();
SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, currentTarget->getPosition(), false, false, chainDistance, chainDistance, chainDistance, chainDistance);
auto spectators = Spectators().find<Creature>(currentTarget->getPosition(), false, chainDistance, chainDistance, chainDistance, chainDistance);
g_logger().debug("Combat::pickChainTargets: currentTarget: {}, spectators: {}", currentTarget->getName(), spectators.size());

double closestDistance = std::numeric_limits<double>::max();
std::shared_ptr<Creature> closestSpectator = nullptr;
for (std::shared_ptr<Creature> spectator : spectators) {
for (const auto &spectator : spectators) {
if (!spectator || visited.contains(spectator->getID())) {
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion src/creatures/combat/combat.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ class Combat {
static void CombatDispelFunc(std::shared_ptr<Creature> caster, std::shared_ptr<Creature> target, const CombatParams &params, CombatDamage* data);
static void CombatNullFunc(std::shared_ptr<Creature> caster, std::shared_ptr<Creature> target, const CombatParams &params, CombatDamage* data);

static void combatTileEffects(const SpectatorHashSet &spectators, std::shared_ptr<Creature> caster, std::shared_ptr<Tile> tile, const CombatParams &params);
static void combatTileEffects(const CreatureVector &spectators, std::shared_ptr<Creature> caster, std::shared_ptr<Tile> tile, const CombatParams &params);

/**
* @brief Calculate the level formula for combat.
Expand Down
6 changes: 3 additions & 3 deletions src/creatures/combat/condition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "game/game.hpp"
#include "game/scheduling/dispatcher.hpp"
#include "io/fileloader.hpp"
#include "map/spectators.hpp"

/**
* Condition
Expand Down Expand Up @@ -1190,13 +1191,12 @@ bool ConditionRegeneration::executeCondition(std::shared_ptr<Creature> creature,
message.primary.color = TEXTCOLOR_PASTELRED;
player->sendTextMessage(message);

SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, player->getPosition(), false, true);
auto spectators = Spectators().find<Player>(player->getPosition());
spectators.erase(player);
if (!spectators.empty()) {
message.type = MESSAGE_HEALED_OTHERS;
message.text = player->getName() + " was healed for " + healString;
for (std::shared_ptr<Creature> spectator : spectators) {
for (const auto &spectator : spectators) {
spectator->getPlayer()->sendTextMessage(message);
}
}
Expand Down
19 changes: 5 additions & 14 deletions src/creatures/creature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "creatures/monsters/monster.hpp"
#include "game/scheduling/scheduler.hpp"
#include "game/zones/zone.hpp"
#include "map/spectators.hpp"

double Creature::speedA = 857.36;
double Creature::speedB = 261.29;
Expand Down Expand Up @@ -1201,8 +1202,7 @@ void Creature::onGainExperience(uint64_t gainExp, std::shared_ptr<Creature> targ
master->onGainExperience(gainExp, target);

if (!m->isFamiliar()) {
SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, position, false, true);
auto spectators = Spectators().find<Player>(position);
if (spectators.empty()) {
return;
}
Expand All @@ -1212,7 +1212,7 @@ void Creature::onGainExperience(uint64_t gainExp, std::shared_ptr<Creature> targ
message.primary.color = TEXTCOLOR_WHITE_EXP;
message.primary.value = gainExp;

for (std::shared_ptr<Creature> spectator : spectators) {
for (const auto &spectator : spectators) {
spectator->getPlayer()->sendTextMessage(message);
}
}
Expand Down Expand Up @@ -1800,16 +1800,7 @@ void Creature::iconChanged() {
return;
}

SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, tile->getPosition(), true);
for (auto spectator : spectators) {
if (!spectator) {
continue;
}

auto player = spectator->getPlayer();
if (player) {
player->sendCreatureIcon(getCreature());
}
for (const auto &spectator : Spectators().find<Player>(tile->getPosition(), true)) {
spectator->getPlayer()->sendCreatureIcon(getCreature());
}
}
8 changes: 3 additions & 5 deletions src/creatures/monsters/monster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "lua/creature/events.hpp"
#include "lua/callbacks/event_callback.hpp"
#include "lua/callbacks/events_callbacks.hpp"
#include "map/spectators.hpp"

int32_t Monster::despawnRange;
int32_t Monster::despawnRadius;
Expand Down Expand Up @@ -349,11 +350,8 @@ void Monster::updateTargetList() {
}
}

SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, position, true);
spectators.erase(this);
for (auto spectator : spectators) {
if (canSee(spectator->getPosition())) {
for (const auto &spectator : Spectators().find<Creature>(position, true)) {
if (spectator.get() != this && canSee(spectator->getPosition())) {
onCreatureFound(spectator);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/creatures/monsters/spawns/spawn_monster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "lua/callbacks/event_callback.hpp"
#include "lua/callbacks/events_callbacks.hpp"
#include "utils/pugicast.hpp"
#include "map/spectators.hpp"

static constexpr int32_t MONSTER_MINSPAWN_INTERVAL = 1000; // 1 second
static constexpr int32_t MONSTER_MAXSPAWN_INTERVAL = 86400000; // 1 day
Expand Down Expand Up @@ -155,9 +156,8 @@ SpawnMonster::~SpawnMonster() {
}

bool SpawnMonster::findPlayer(const Position &pos) {
SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, pos, false, true);
for (std::shared_ptr<Creature> spectator : spectators) {
auto spectators = Spectators().find<Player>(pos);
for (const auto &spectator : spectators) {
if (!spectator->getPlayer()->hasFlag(PlayerFlags_t::IgnoredByMonsters)) {
return true;
}
Expand Down
14 changes: 7 additions & 7 deletions src/creatures/npcs/npc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "lua/callbacks/creaturecallback.hpp"
#include "game/scheduling/dispatcher.hpp"
#include "game/scheduling/scheduler.hpp"
#include "map/spectators.hpp"

int32_t Npc::despawnRange;
int32_t Npc::despawnRadius;
Expand Down Expand Up @@ -151,7 +152,7 @@ void Npc::onPlayerAppear(std::shared_ptr<Player> player) {
if (player->hasFlag(PlayerFlags_t::IgnoredByNpcs) || playerSpectators.contains(player)) {
return;
}
playerSpectators.insert(player);
playerSpectators.emplace_back(player);
manageIdle();
}

Expand Down Expand Up @@ -531,19 +532,18 @@ void Npc::onThinkWalk(uint32_t interval) {

void Npc::onCreatureWalk() {
Creature::onCreatureWalk();
phmap::erase_if(playerSpectators, [this](const auto &creature) { return !this->canSee(creature->getPosition()); });
playerSpectators.erase_if([this](const auto &creature) { return !this->canSee(creature->getPosition()); });
}

void Npc::onPlacedCreature() {
loadPlayerSpectators();
}

void Npc::loadPlayerSpectators() {
SpectatorHashSet spec;
g_game().map.getSpectators(spec, position, true, true);
for (auto creature : spec) {
if (creature->getPlayer() || creature->getPlayer()->hasFlag(PlayerFlags_t::IgnoredByNpcs)) {
playerSpectators.insert(creature);
auto spec = Spectators().find<Player>(position, true);
for (const auto &creature : spec) {
if (!creature->getPlayer()->hasFlag(PlayerFlags_t::IgnoredByNpcs)) {
playerSpectators.emplace_back(creature->getPlayer());
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/creatures/npcs/npc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ class Npc final : public Creature {

bool ignoreHeight;

SpectatorHashSet playerSpectators;
stdext::vector_set<std::shared_ptr<Player>> playerSpectators;
Position masterPos;

friend class LuaScriptInterface;
Expand Down
6 changes: 3 additions & 3 deletions src/creatures/npcs/spawns/spawn_npc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "lua/callbacks/event_callback.hpp"
#include "lua/callbacks/events_callbacks.hpp"
#include "utils/pugicast.hpp"
#include "map/spectators.hpp"

static constexpr int32_t MINSPAWN_INTERVAL = 1000; // 1 second
static constexpr int32_t MAXSPAWN_INTERVAL = 86400000; // 1 day
Expand Down Expand Up @@ -141,9 +142,8 @@ SpawnNpc::~SpawnNpc() {
}

bool SpawnNpc::findPlayer(const Position &pos) {
SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, pos, false, true);
for (std::shared_ptr<Creature> spectator : spectators) {
auto spectators = Spectators().find<Player>(pos);
for (const auto &spectator : spectators) {
if (!spectator->getPlayer()->hasFlag(PlayerFlags_t::IgnoredByNpcs)) {
return true;
}
Expand Down
41 changes: 15 additions & 26 deletions src/creatures/players/player.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "items/bed.hpp"
#include "items/weapons/weapons.hpp"
#include "core.hpp"
#include "map/spectators.hpp"

MuteCountMap Player::muteCountMap;

Expand Down Expand Up @@ -2244,8 +2245,7 @@ void Player::addExperience(std::shared_ptr<Creature> target, uint64_t exp, bool
message.primary.color = TEXTCOLOR_WHITE_EXP;
sendTextMessage(message);

SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, position, false, true);
auto spectators = Spectators().find<Player>(position);
spectators.erase(static_self_cast<Player>());
if (!spectators.empty()) {
message.type = MESSAGE_EXPERIENCE_OTHERS;
Expand Down Expand Up @@ -2338,8 +2338,7 @@ void Player::removeExperience(uint64_t exp, bool sendText /* = false*/) {
message.primary.color = TEXTCOLOR_RED;
sendTextMessage(message);

SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, position, false, true);
auto spectators = Spectators().find<Player>(position);
spectators.erase(static_self_cast<Player>());
if (!spectators.empty()) {
message.type = MESSAGE_EXPERIENCE_OTHERS;
Expand Down Expand Up @@ -2770,14 +2769,9 @@ bool Player::spawn() {
return false;
}

SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, position, true);
for (std::shared_ptr<Creature> spectator : spectators) {
if (!spectator) {
continue;
}

if (std::shared_ptr<Player> tmpPlayer = spectator->getPlayer()) {
auto spectators = Spectators().find<Creature>(position, true);
for (const auto &spectator : spectators) {
if (const auto &tmpPlayer = spectator->getPlayer()) {
tmpPlayer->sendCreatureAppear(static_self_cast<Player>(), pos, true);
}

Expand Down Expand Up @@ -2813,18 +2807,13 @@ void Player::despawn() {

std::vector<int32_t> oldStackPosVector;

SpectatorHashSet spectators;
g_game().map.getSpectators(spectators, tile->getPosition(), true);
auto spectators = Spectators().find<Creature>(tile->getPosition(), true);
size_t i = 0;
for (std::shared_ptr<Creature> spectator : spectators) {
if (!spectator) {
continue;
}

if (const auto player = spectator->getPlayer()) {
for (const auto &spectator : spectators) {
if (const auto &player = spectator->getPlayer()) {
oldStackPosVector.push_back(player->canSeeCreature(static_self_cast<Player>()) ? tile->getStackposOfCreature(player, getPlayer()) : -1);
}
if (auto player = spectator->getPlayer()) {
if (const auto &player = spectator->getPlayer()) {
player->sendRemoveTileThing(tile->getPosition(), oldStackPosVector[i++]);
}

Expand Down Expand Up @@ -6772,7 +6761,7 @@ bool Player::saySpell(
SpeakClasses type,
const std::string &text,
bool ghostMode,
SpectatorHashSet* spectatorsPtr /* = nullptr*/,
Spectators* spectatorsPtr /* = nullptr*/,
const Position* pos /* = nullptr*/
) {
if (text.empty()) {
Expand All @@ -6784,17 +6773,17 @@ bool Player::saySpell(
pos = &getPosition();
}

SpectatorHashSet spectators;
Spectators spectators;

if (!spectatorsPtr || spectatorsPtr->empty()) {
// This somewhat complex construct ensures that the cached SpectatorHashSet
// This somewhat complex construct ensures that the cached Spectators
// is used if available and if it can be used, else a local vector is
// used (hopefully the compiler will optimize away the construction of
// the temporary when it's not used).
if (type != TALKTYPE_YELL && type != TALKTYPE_MONSTER_YELL) {
g_game().map.getSpectators(spectators, *pos, false, false, MAP_MAX_CLIENT_VIEW_PORT_X, MAP_MAX_CLIENT_VIEW_PORT_X, MAP_MAX_CLIENT_VIEW_PORT_Y, MAP_MAX_CLIENT_VIEW_PORT_Y);
spectators.find<Creature>(*pos, false, MAP_MAX_CLIENT_VIEW_PORT_X, MAP_MAX_CLIENT_VIEW_PORT_X, MAP_MAX_CLIENT_VIEW_PORT_Y, MAP_MAX_CLIENT_VIEW_PORT_Y);
} else {
g_game().map.getSpectators(spectators, *pos, true, false, (MAP_MAX_CLIENT_VIEW_PORT_X + 1) * 2, (MAP_MAX_CLIENT_VIEW_PORT_X + 1) * 2, (MAP_MAX_CLIENT_VIEW_PORT_Y + 1) * 2, (MAP_MAX_CLIENT_VIEW_PORT_Y + 1) * 2);
spectators.find<Creature>(*pos, true, (MAP_MAX_CLIENT_VIEW_PORT_X + 1) * 2, (MAP_MAX_CLIENT_VIEW_PORT_X + 1) * 2, (MAP_MAX_CLIENT_VIEW_PORT_Y + 1) * 2, (MAP_MAX_CLIENT_VIEW_PORT_Y + 1) * 2);
}
} else {
spectators = (*spectatorsPtr);
Expand Down
3 changes: 2 additions & 1 deletion src/creatures/players/player.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class PreySlot;
class TaskHuntingSlot;
class Spell;
class PlayerWheel;
class Spectators;

enum class ForgeConversion_t : uint8_t {
FORGE_ACTION_FUSION = 0,
Expand Down Expand Up @@ -2285,7 +2286,7 @@ class Player final : public Creature, public Cylinder, public Bankable {
SpeakClasses type,
const std::string &text,
bool ghostMode,
SpectatorHashSet* spectatorsPtr = nullptr,
Spectators* spectatorsPtr = nullptr,
const Position* pos = nullptr
);

Expand Down
Loading

0 comments on commit 1e1fd0c

Please sign in to comment.