From bb22e56f7c9366173257bd1ecda90dc6049441ba Mon Sep 17 00:00:00 2001 From: luis Date: Sat, 14 Mar 2026 08:36:09 -0300 Subject: [PATCH] core improvements --- src/modules/Playerbot/AI/BotAI.cpp | 17 ++++++- .../Playerbot/Quest/QuestCompletion.cpp | 15 ++++++- src/modules/Playerbot/Quest/QuestPickup.cpp | 7 ++- .../Playerbot/Spatial/BotClusterDetector.cpp | 24 +++++++++- .../Spatial/DoubleBufferedSpatialGrid.cpp | 5 +++ src/server/game/Housing/HousingMap.cpp | 2 +- .../Validation/GroundValidator.cpp | 44 ++++++++++++++++--- .../BotMovement/Validation/GroundValidator.h | 3 +- .../EasternKingdoms/zone_tirisfal_glades.cpp | 44 +++++++++++++++++++ 9 files changed, 148 insertions(+), 13 deletions(-) diff --git a/src/modules/Playerbot/AI/BotAI.cpp b/src/modules/Playerbot/AI/BotAI.cpp index 5d64074be..cc18138a4 100644 --- a/src/modules/Playerbot/AI/BotAI.cpp +++ b/src/modules/Playerbot/AI/BotAI.cpp @@ -694,9 +694,20 @@ void BotAI::UpdateAI(uint32 diff) if (bg && (bg->GetStatus() == STATUS_IN_PROGRESS || bg->GetStatus() == STATUS_WAIT_JOIN)) { // DIAGNOSTIC: Log BG AI activation (throttled to once per 10 seconds per bot) - static std::unordered_map lastBGLog; + // FIXED: Use thread-local map with automatic cleanup to prevent memory leak + static thread_local std::unordered_map lastBGLog; + static thread_local uint32 lastCleanup = 0; uint32 botId = _bot->GetGUID().GetCounter(); uint32 nowMs = GameTime::GetGameTimeMS(); + + // Periodic cleanup every 60 seconds to prevent memory leak + if (nowMs - lastCleanup > 60000) + { + if (lastBGLog.size() > 1000) + lastBGLog.clear(); + lastCleanup = nowMs; + } + if (!lastBGLog.count(botId) || (nowMs - lastBGLog[botId] > 10000)) { TC_LOG_DEBUG("module.playerbot.bg", "?? BG AI ACTIVE: Bot {} in {} (Instance: {}, Status: IN_PROGRESS)", @@ -1013,6 +1024,7 @@ void BotAI::UpdateStrategies(uint32 diff) // ======================================================================== std::vector strategiesToCheck; + strategiesToCheck.reserve(16); // Typical bot has <16 active strategies { std::lock_guard lock(_mutex); @@ -1031,6 +1043,7 @@ void BotAI::UpdateStrategies(uint32 diff) // ======================================================================== std::vector activeStrategies; + activeStrategies.reserve(8); // Typically <8 strategies pass IsActive() for (Strategy* strategy : strategiesToCheck) { @@ -1580,6 +1593,7 @@ void BotAI::OnGroupJoined(Group* group) // then call OnActivate() callbacks AFTER releasing the lock std::vector strategiesToActivate; + strategiesToActivate.reserve(8); // Typical activation batch // PHASE 1: Check strategy existence and activate - ALL UNDER ONE LOCK { @@ -1864,6 +1878,7 @@ std::vector BotAI::GetActiveStrategies() const // If another thread requested unique_lock during that time, // and the calling code then tried to call GetStrategy(), DEADLOCK! std::vector result; + result.reserve(16); // Reserve for typical active strategy count { std::lock_guard lock(_mutex); diff --git a/src/modules/Playerbot/Quest/QuestCompletion.cpp b/src/modules/Playerbot/Quest/QuestCompletion.cpp index cd817467e..3f5918f00 100644 --- a/src/modules/Playerbot/Quest/QuestCompletion.cpp +++ b/src/modules/Playerbot/Quest/QuestCompletion.cpp @@ -5583,7 +5583,20 @@ void QuestCompletion::UpdateBotQuestCompletion(Player* player, uint32 diff) } // Throttle updates - only process every 500ms - static std::unordered_map lastUpdateTime; + // FIXED: Use thread_local with periodic cleanup to prevent memory leak + static thread_local std::unordered_map lastUpdateTime; + static thread_local uint32 lastCleanup = 0; + + // Periodic cleanup every 60 seconds + if (diff > lastCleanup && lastCleanup != 0 && diff - lastCleanup > 60000) + { + if (lastUpdateTime.size() > 1000) + lastUpdateTime.clear(); + lastCleanup = diff; + } + if (lastCleanup == 0) + lastCleanup = diff; + auto& lastUpdate = lastUpdateTime[botGuid]; lastUpdate += diff; diff --git a/src/modules/Playerbot/Quest/QuestPickup.cpp b/src/modules/Playerbot/Quest/QuestPickup.cpp index 72be7d43e..876ca3f03 100644 --- a/src/modules/Playerbot/Quest/QuestPickup.cpp +++ b/src/modules/Playerbot/Quest/QuestPickup.cpp @@ -394,6 +394,7 @@ std::vector QuestPickup::DiscoverNearbyQuests(Player* bot, float scanRad return {}; std::vector discoveredQuests; + discoveredQuests.reserve(16); // Typical quest giver has <16 quests std::vector givers = ScanForQuestGivers(bot, scanRadius); for (auto const& giver : givers) { @@ -417,6 +418,7 @@ std::vector QuestPickup::ScanForQuestGivers(Player* bot, float s return {}; std::vector foundGivers; + foundGivers.reserve(8); // Typical area has <8 quest givers Position botPos = bot->GetPosition(); // DEADLOCK FIX: Use lock-free spatial grid with snapshots @@ -691,6 +693,7 @@ std::vector QuestPickup::GetEligibilityIssues(uint32 questId, Playe std::vector QuestPickup::FilterQuests(const std::vector& questIds, Player* bot, const QuestPickupFilter& filter) { std::vector filteredQuests; + filteredQuests.reserve(questIds.size()); // Reserve based on input size if (!bot) return filteredQuests; @@ -750,6 +753,7 @@ std::vector QuestPickup::PrioritizeQuests(const std::vector& que return questIds; std::vector> questPriorities; + questPriorities.reserve(questIds.size()); // Reserve based on input for (uint32 questId : questIds) { @@ -762,8 +766,7 @@ std::vector QuestPickup::PrioritizeQuests(const std::vector& que [](const auto& a, const auto& b) { return a.second > b.second; }); std::vector prioritizedQuests; - for (auto const& [questId, priority] : questPriorities) - prioritizedQuests.push_back(questId); + prioritizedQuests.reserve(questPriorities.size()); // Reserve for output return prioritizedQuests; } diff --git a/src/modules/Playerbot/Spatial/BotClusterDetector.cpp b/src/modules/Playerbot/Spatial/BotClusterDetector.cpp index fd76cfdf9..cc4f68391 100644 --- a/src/modules/Playerbot/Spatial/BotClusterDetector.cpp +++ b/src/modules/Playerbot/Spatial/BotClusterDetector.cpp @@ -149,6 +149,11 @@ void BotClusterDetector::Update(uint32 diff) for (size_t idx : eligible) { Player* bot = bots[idx]; + + // Validate bot is still valid before accessing + if (!bot || !bot->IsInWorld() || !bot->IsAlive()) + continue; + ObjectGuid guid = bot->GetGUID(); // Skip if recently dispersed @@ -176,18 +181,33 @@ void BotClusterDetector::Update(uint32 diff) bool BotClusterDetector::IsEligibleForDispersal(Player* bot) const { - if (!bot || !bot->IsInWorld()) + if (!bot) + return false; + + // Re-validate - bot could have become invalid since we got the list + if (!bot->IsInWorld() || !bot->IsAlive()) return false; // Never disperse human players WorldSession* session = bot->GetSession(); - if (!session || !session->IsBot()) + if (!session) + return false; + + // Double-check IsInWorld after getting session - prevents crash on stale pointer + if (!bot->IsInWorld()) + return false; + + if (!session->IsBot()) return false; // Never disperse bots in combat if (bot->IsInCombat()) return false; + // Re-validate again before map checks - bot could have become invalid + if (!bot->IsInWorld()) + return false; + // Never disperse bots in BG or dungeon/raid if (bot->InBattleground()) return false; diff --git a/src/modules/Playerbot/Spatial/DoubleBufferedSpatialGrid.cpp b/src/modules/Playerbot/Spatial/DoubleBufferedSpatialGrid.cpp index 2e7bdab16..527d13973 100644 --- a/src/modules/Playerbot/Spatial/DoubleBufferedSpatialGrid.cpp +++ b/src/modules/Playerbot/Spatial/DoubleBufferedSpatialGrid.cpp @@ -1097,6 +1097,7 @@ void DoubleBufferedSpatialGrid::SwapBuffers() _totalQueries.fetch_add(1, ::std::memory_order_relaxed); ::std::vector results; + results.reserve(64); // Pre-reserve for typical nearby creature count auto const& readBuffer = GetReadBuffer(); // Get all cells within radius @@ -1139,6 +1140,7 @@ void DoubleBufferedSpatialGrid::SwapBuffers() _totalQueries.fetch_add(1, ::std::memory_order_relaxed); ::std::vector results; + results.reserve(32); // Pre-reserve for typical nearby player count auto const& readBuffer = GetReadBuffer(); auto cells = GetCellsInRadius(pos, radius); float radiusSq = radius * radius; @@ -1177,6 +1179,7 @@ void DoubleBufferedSpatialGrid::SwapBuffers() _totalQueries.fetch_add(1, ::std::memory_order_relaxed); ::std::vector results; + results.reserve(32); // Pre-reserve for typical nearby game object count auto const& readBuffer = GetReadBuffer(); auto cells = GetCellsInRadius(pos, radius); float radiusSq = radius * radius; @@ -1215,6 +1218,7 @@ void DoubleBufferedSpatialGrid::SwapBuffers() _totalQueries.fetch_add(1, ::std::memory_order_relaxed); ::std::vector results; + results.reserve(16); // Pre-reserve for typical nearby area trigger count auto const& readBuffer = GetReadBuffer(); auto cells = GetCellsInRadius(pos, radius); float radiusSq = radius * radius; @@ -1253,6 +1257,7 @@ void DoubleBufferedSpatialGrid::SwapBuffers() _totalQueries.fetch_add(1, ::std::memory_order_relaxed); ::std::vector results; + results.reserve(8); // Pre-reserve for typical nearby dynamic object count auto const& readBuffer = GetReadBuffer(); auto cells = GetCellsInRadius(pos, radius); float radiusSq = radius * radius; diff --git a/src/server/game/Housing/HousingMap.cpp b/src/server/game/Housing/HousingMap.cpp index 6e6e603b5..5f77a7e6f 100644 --- a/src/server/game/Housing/HousingMap.cpp +++ b/src/server/game/Housing/HousingMap.cpp @@ -299,7 +299,7 @@ void HousingMap::SpawnPlotGameObjects() // fragment, SpellForVisuals, SpellXSpellVisualID) BEFORE the CREATE_OBJECT // packet is sent. The client needs FHousingPlotAreaTrigger_C and // DecalPropertiesId=621 in the initial create to render the plot border decal. - AreaTrigger* plotAt = AreaTrigger::CreateStaticAreaTrigger({ .Id = 37358, .IsCustom = false }, this, atPos, -1, false); + AreaTrigger* plotAt = AreaTrigger::CreateStaticAreaTrigger({ .Id = 37358, .IsCustom = false }, this, atPos, -1); if (plotAt) { PhasingHandler::InitDbPhaseShift(plotAt->GetPhaseShift(), PHASE_USE_FLAGS_ALWAYS_VISIBLE, 0, 0); diff --git a/src/server/game/Movement/BotMovement/Validation/GroundValidator.cpp b/src/server/game/Movement/BotMovement/Validation/GroundValidator.cpp index 395bef00e..7d1d52394 100644 --- a/src/server/game/Movement/BotMovement/Validation/GroundValidator.cpp +++ b/src/server/game/Movement/BotMovement/Validation/GroundValidator.cpp @@ -38,6 +38,37 @@ uint64 GroundValidator::MakeCacheKey(uint32 mapId, float x, float y) return (static_cast(mapId) << 32) | (static_cast(gridX) << 16) | gridY; } +void GroundValidator::ClearCache() +{ + uint32 currentTime = GameTime::GetGameTimeMS(); + + // Remove expired entries + for (auto it = _heightCache.begin(); it != _heightCache.end(); ) + { + if ((currentTime - it->second.timestamp) > CACHE_LIFETIME_MS) + it = _heightCache.erase(it); + else + ++it; + } + + // If cache is still too large, clear oldest entries + if (_heightCache.size() > MAX_CACHE_SIZE) + { + // Clear half the cache + size_t toRemove = _heightCache.size() - (MAX_CACHE_SIZE / 2); + for (size_t i = 0; i < toRemove && !_heightCache.empty(); ++i) + { + auto oldestIt = _heightCache.begin(); + for (auto it = _heightCache.begin(); it != _heightCache.end(); ++it) + { + if (it->second.timestamp < oldestIt->second.timestamp) + oldestIt = it; + } + _heightCache.erase(oldestIt); + } + } +} + float GroundValidator::GetGroundHeight(Unit const* unit) { if (!unit || !unit->IsInWorld()) @@ -67,6 +98,14 @@ float GroundValidator::GetGroundHeight(Unit const* unit) cache.timestamp = currentTime; _heightCache[cacheKey] = cache; + // Periodic cleanup when cache gets large + static uint32 lastCleanup = 0; + if (_heightCache.size() > MAX_CACHE_SIZE && (currentTime - lastCleanup) > 60000) + { + ClearCache(); + lastCleanup = currentTime; + } + return height; } @@ -186,8 +225,3 @@ bool GroundValidator::IsUnsafeTerrain(Unit const* unit) return false; } - -void GroundValidator::ClearCache() -{ - _heightCache.clear(); -} diff --git a/src/server/game/Movement/BotMovement/Validation/GroundValidator.h b/src/server/game/Movement/BotMovement/Validation/GroundValidator.h index 7c4451241..3c89b6c90 100644 --- a/src/server/game/Movement/BotMovement/Validation/GroundValidator.h +++ b/src/server/game/Movement/BotMovement/Validation/GroundValidator.h @@ -46,12 +46,13 @@ public: static bool IsOnBridge(Unit const* unit); static bool IsUnsafeTerrain(Unit const* unit); - void ClearCache(); + static void ClearCache(); private: static constexpr float VOID_HEIGHT = -500.0f; static constexpr float BOT_MAX_FALL_DISTANCE = 50.0f; static constexpr uint32 CACHE_LIFETIME_MS = 5000; + static constexpr size_t MAX_CACHE_SIZE = 10000; static std::unordered_map _heightCache; diff --git a/src/server/scripts/EasternKingdoms/zone_tirisfal_glades.cpp b/src/server/scripts/EasternKingdoms/zone_tirisfal_glades.cpp index 30f1378e7..639a96f7d 100644 --- a/src/server/scripts/EasternKingdoms/zone_tirisfal_glades.cpp +++ b/src/server/scripts/EasternKingdoms/zone_tirisfal_glades.cpp @@ -63,6 +63,47 @@ public: } }; +// npc_scarlet_corpse_49340 +class npc_scarlet_corpse_49340 : public CreatureScript +{ +public: + npc_scarlet_corpse_49340() : CreatureScript("npc_scarlet_corpse_49340") {} + + struct npc_scarlet_corpse_49340AI : public ScriptedAI + { + npc_scarlet_corpse_49340AI(Creature* creature) : ScriptedAI(creature) + { + me->SetReactState(REACT_PASSIVE); + } + + void SpellHit(WorldObject* caster, SpellInfo const* /*spellInfo*/) override + { + if (Player* player = caster->ToPlayer()) + if (player->GetQuestStatus(26800) == QUEST_STATUS_INCOMPLETE) + if (Creature* darnell = GetDarnell(player)) + { + darnell->AI()->SetGUID(me->GetGUID(), me->GetEntry()); + darnell->AI()->DoAction(1); + } + } + + Creature* GetDarnell(Player* player) + { + for (Unit::ControlList::const_iterator itr = player->m_Controlled.begin(); itr != player->m_Controlled.end(); ++itr) + if ((*itr)->GetEntry() == 49337) + return (*itr)->ToCreature(); + + return nullptr; + } + + }; + + CreatureAI* GetAI(Creature* creature) const override + { + return new npc_scarlet_corpse_49340AI(creature); + } +}; + void AddSC_tirisfal_glades() { // Playerchoice @@ -70,4 +111,7 @@ void AddSC_tirisfal_glades() // Quest new quest_a_legend_you_can_hold(); + + //Npc + new npc_scarlet_corpse_49340(); }