diff --git a/src/server/game/Pools/PoolMgr.cpp b/src/server/game/Pools/PoolMgr.cpp index 479fd6ede..efcd9e360 100644 --- a/src/server/game/Pools/PoolMgr.cpp +++ b/src/server/game/Pools/PoolMgr.cpp @@ -1038,6 +1038,20 @@ void PoolMgr::ChangeWeeklyQuests() SaveQuestsToDB(false, true, false); } +void PoolMgr::ReSpawnPoolQuests() +{ + for (auto& [questId, poolId] : mQuestSearchMap) + { + if (IsSpawnedObject(questId)) + { + PoolObject tempObj(questId, 0.0f); + auto it = mPoolQuestGroups.find(poolId); + if (it != mPoolQuestGroups.end()) + it->second.Spawn1Object(&tempObj); + } + } +} + // Call to spawn a pool, if cache if true the method will spawn only if cached entry is different // If it's same, the creature is respawned only (added back to map) template<> diff --git a/src/server/game/Pools/PoolMgr.h b/src/server/game/Pools/PoolMgr.h index cf251352c..447d3a5be 100644 --- a/src/server/game/Pools/PoolMgr.h +++ b/src/server/game/Pools/PoolMgr.h @@ -130,10 +130,12 @@ public: void ChangeDailyQuests(); void ChangeWeeklyQuests(); + void ReSpawnPoolQuests(); PooledQuestRelation mQuestCreatureRelation; PooledQuestRelation mQuestGORelation; + friend class PoolQuestReloadFixTest; private: template void SpawnPool(uint32 pool_id, uint32 db_guid_or_pool_id); diff --git a/src/server/scripts/Commands/cs_reload.cpp b/src/server/scripts/Commands/cs_reload.cpp index 03ce1d185..01db0d650 100644 --- a/src/server/scripts/Commands/cs_reload.cpp +++ b/src/server/scripts/Commands/cs_reload.cpp @@ -30,6 +30,7 @@ #include "MapMgr.h" #include "MotdMgr.h" #include "ObjectMgr.h" +#include "PoolMgr.h" #include "ScriptMgr.h" #include "ServerMailMgr.h" #include "SkillDiscovery.h" @@ -270,6 +271,7 @@ public: LOG_INFO("server.loading", "Reloading Quests Relations..."); sObjectMgr->LoadQuestStartersAndEnders(); + sPoolMgr->ReSpawnPoolQuests(); handler->SendGlobalGMSysMessage("DB tables `*_queststarter` and `*_questender` reloaded."); return true; } @@ -490,6 +492,7 @@ public: { LOG_INFO("server.loading", "Loading Quests Relations... (`creature_queststarter`)"); sObjectMgr->LoadCreatureQuestStarters(); + sPoolMgr->ReSpawnPoolQuests(); handler->SendGlobalGMSysMessage("DB table `creature_queststarter` reloaded."); return true; } @@ -532,6 +535,7 @@ public: { LOG_INFO("server.loading", "Loading Quests Relations... (`gameobject_queststarter`)"); sObjectMgr->LoadGameobjectQuestStarters(); + sPoolMgr->ReSpawnPoolQuests(); handler->SendGlobalGMSysMessage("DB table `gameobject_queststarter` reloaded."); return true; } diff --git a/src/test/server/game/Pools/PoolQuestReloadTest.cpp b/src/test/server/game/Pools/PoolQuestReloadTest.cpp new file mode 100644 index 000000000..8cb97cc0e --- /dev/null +++ b/src/test/server/game/Pools/PoolQuestReloadTest.cpp @@ -0,0 +1,299 @@ +/* + * This file is part of the AzerothCore Project. See AUTHORS file for Copyright information + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + */ + +/* + * Regression test for pool quest reload bug. + * + * When `.reload creature_queststarter` is executed, LoadQuestRelationsHelper() + * clears the creature quest relation map (_creatureQuestRelations) which + * contains the active pooled daily quest. It repopulates the pool mapping + * (mQuestCreatureRelation) but never calls Spawn1Object() to re-insert the + * active quest into the creature relation map. This causes ALL pool-based + * daily quests (Dalaran cooking, fishing, jewelcrafting, etc.) to vanish + * from their NPCs. + */ + +#include "ObjectMgr.h" +#include "PoolMgr.h" +#include "QuestDef.h" +#include "gtest/gtest.h" + +namespace +{ + +// Test IDs chosen to avoid collisions with real data +static constexpr uint32 TEST_QUEST_ID = 99998; +static constexpr uint32 TEST_CREATURE_ID = 99999; + +class PoolQuestReloadTest : public ::testing::Test +{ +protected: + void SetUp() override + { + creatureQuestMap = sObjectMgr->GetCreatureQuestRelationMap(); + + // Establish the pool-side mapping: quest -> creature + // This is what LoadQuestRelationsHelper populates for pooled quests + sPoolMgr->mQuestCreatureRelation.insert( + PooledQuestRelation::value_type(TEST_QUEST_ID, TEST_CREATURE_ID)); + } + + void TearDown() override + { + // Clean up: remove test entries from both maps + auto range = sPoolMgr->mQuestCreatureRelation.equal_range(TEST_QUEST_ID); + sPoolMgr->mQuestCreatureRelation.erase(range.first, range.second); + + // Remove any test entries left in creature quest relations + auto crRange = creatureQuestMap->equal_range(TEST_CREATURE_ID); + for (auto it = crRange.first; it != crRange.second; ) + { + if (it->second == TEST_QUEST_ID) + it = creatureQuestMap->erase(it); + else + ++it; + } + } + + /// Helper: count how many times quest appears on a creature in the quest relation map + uint32 CountQuestOnCreature(uint32 creatureId, uint32 questId) + { + uint32 count = 0; + auto range = creatureQuestMap->equal_range(creatureId); + for (auto it = range.first; it != range.second; ++it) + if (it->second == questId) + ++count; + return count; + } + + /// Simulate what Spawn1Object does: copy the active pool quest + /// from mQuestCreatureRelation into the creature quest relation map + void SimulateSpawn1Object(uint32 questId) + { + PoolGroup poolGroup; + PoolObject obj(questId, 0.0f); + poolGroup.Spawn1Object(&obj); + } + + /// Simulate what LoadQuestRelationsHelper does on reload: + /// 1. Clear the creature quest relation map + /// 2. Clear and repopulate mQuestCreatureRelation + /// Non-pooled quests would be re-added to the creature map, but + /// pooled quests only go into mQuestCreatureRelation. + void SimulateReload() + { + // Step 1: map.clear() — line 8589 of ObjectMgr.cpp + creatureQuestMap->clear(); + + // Step 2: poolRelationMap->clear() — line 8604 of ObjectMgr.cpp + sPoolMgr->mQuestCreatureRelation.clear(); + + // Step 3: Repopulate poolRelationMap from DB + // (In the real code this re-reads creature_queststarter LEFT JOIN pool_quest) + sPoolMgr->mQuestCreatureRelation.insert( + PooledQuestRelation::value_type(TEST_QUEST_ID, TEST_CREATURE_ID)); + + // NOTE: The real reload handler does NOT call Spawn1Object here. + // That is the bug. + } + + QuestRelations* creatureQuestMap = nullptr; +}; + +// ------------------------------------------------------------------ +// Baseline: Spawn1Object correctly adds pooled quest to NPC +// ------------------------------------------------------------------ +// cppcheck-suppress syntaxError +TEST_F(PoolQuestReloadTest, Spawn1ObjectAddsQuestToCreatureRelationMap) +{ + // Initially the quest should NOT be on the creature + EXPECT_EQ(CountQuestOnCreature(TEST_CREATURE_ID, TEST_QUEST_ID), 0u) + << "Quest should not be on creature before Spawn1Object"; + + // Spawn1Object reads from mQuestCreatureRelation and inserts into + // the creature quest relation map + SimulateSpawn1Object(TEST_QUEST_ID); + + EXPECT_EQ(CountQuestOnCreature(TEST_CREATURE_ID, TEST_QUEST_ID), 1u) + << "Quest should appear on creature after Spawn1Object"; +} + +// ------------------------------------------------------------------ +// BUG: Reload clears pooled quest without re-spawning it +// ------------------------------------------------------------------ +TEST_F(PoolQuestReloadTest, ReloadCreatureQuestStarterRemovesPooledQuest) +{ + // 1. Spawn the pool quest onto the NPC (normal startup behavior) + SimulateSpawn1Object(TEST_QUEST_ID); + ASSERT_EQ(CountQuestOnCreature(TEST_CREATURE_ID, TEST_QUEST_ID), 1u) + << "Precondition: quest must be on creature before reload"; + + // 2. Simulate `.reload creature_queststarter` + SimulateReload(); + + // 3. THE BUG: quest is gone from the NPC even though it's still + // the active daily in the pool + EXPECT_EQ(CountQuestOnCreature(TEST_CREATURE_ID, TEST_QUEST_ID), 0u) + << "BUG: After reload, pooled quest vanishes from creature " + "because Spawn1Object is never called"; + + // 4. Verify mQuestCreatureRelation still has the mapping + // (the pool system KNOWS about the quest, it's just not on the NPC) + auto range = sPoolMgr->mQuestCreatureRelation.equal_range(TEST_QUEST_ID); + EXPECT_NE(range.first, range.second) + << "Pool mapping should still exist after reload"; +} + +// ------------------------------------------------------------------ +// Calling Spawn1Object after reload would fix the problem +// ------------------------------------------------------------------ +TEST_F(PoolQuestReloadTest, Spawn1ObjectAfterReloadRestoresQuest) +{ + // Setup: spawn, reload (quest disappears) + SimulateSpawn1Object(TEST_QUEST_ID); + SimulateReload(); + ASSERT_EQ(CountQuestOnCreature(TEST_CREATURE_ID, TEST_QUEST_ID), 0u) + << "Precondition: quest must be missing after reload"; + + // Fix: call Spawn1Object again for the active pool quest + SimulateSpawn1Object(TEST_QUEST_ID); + + EXPECT_EQ(CountQuestOnCreature(TEST_CREATURE_ID, TEST_QUEST_ID), 1u) + << "Spawn1Object after reload should restore the quest on the NPC"; +} + +// ------------------------------------------------------------------ +// Non-pooled quests survive reload (contrast with pooled quests) +// ------------------------------------------------------------------ +TEST_F(PoolQuestReloadTest, NonPooledQuestSurvivesReload) +{ + static constexpr uint32 REGULAR_QUEST_ID = 99990; + + // A non-pooled quest is added directly to the creature quest map + // (this is what LoadQuestRelationsHelper does for quests without pool_entry) + creatureQuestMap->insert(QuestRelations::value_type(TEST_CREATURE_ID, REGULAR_QUEST_ID)); + + // Simulate reload: clear and repopulate + creatureQuestMap->clear(); + // Non-pooled quests get re-inserted directly (simulating the DB reload) + creatureQuestMap->insert(QuestRelations::value_type(TEST_CREATURE_ID, REGULAR_QUEST_ID)); + + EXPECT_EQ(CountQuestOnCreature(TEST_CREATURE_ID, REGULAR_QUEST_ID), 1u) + << "Non-pooled quests survive reload because they are re-inserted directly"; + + // Cleanup + auto range = creatureQuestMap->equal_range(TEST_CREATURE_ID); + for (auto it = range.first; it != range.second; ) + { + if (it->second == REGULAR_QUEST_ID) + it = creatureQuestMap->erase(it); + else + ++it; + } +} + +} // namespace + +// ------------------------------------------------------------------ +// PoolQuestReloadFixTest: exercises the actual ReSpawnPoolQuests() fix +// by setting up PoolMgr private state (friend class access). +// Must be at global scope to match the friend declaration in PoolMgr. +// ------------------------------------------------------------------ +class PoolQuestReloadFixTest : public ::testing::Test +{ +protected: + void SetUp() override + { + creatureQuestMap = sObjectMgr->GetCreatureQuestRelationMap(); + + // Set up pool infrastructure (private members via friend access) + sPoolMgr->mPoolTemplate[TEST_POOL_ID].MaxLimit = 1; + + // Create the pool group entry (Spawn1Object doesn't use pool group + // internals, it only reads mQuestCreatureRelation) + sPoolMgr->mPoolQuestGroups[TEST_POOL_ID].SetPoolId(TEST_POOL_ID); + + sPoolMgr->mQuestSearchMap[TEST_QUEST_ID] = TEST_POOL_ID; + + // Mark the quest as active/spawned + sPoolMgr->mSpawnedData.ActivateObject(TEST_QUEST_ID, TEST_POOL_ID); + + // Set up pool-side mapping: quest -> creature + sPoolMgr->mQuestCreatureRelation.insert( + PooledQuestRelation::value_type(TEST_QUEST_ID, TEST_CREATURE_ID)); + } + + void TearDown() override + { + // Clean up all test state from the singletons + sPoolMgr->mPoolTemplate.erase(TEST_POOL_ID); + sPoolMgr->mPoolQuestGroups.erase(TEST_POOL_ID); + sPoolMgr->mQuestSearchMap.erase(TEST_QUEST_ID); + sPoolMgr->mSpawnedData.RemoveObject(TEST_QUEST_ID, TEST_POOL_ID); + + auto range = sPoolMgr->mQuestCreatureRelation.equal_range(TEST_QUEST_ID); + sPoolMgr->mQuestCreatureRelation.erase(range.first, range.second); + + auto crRange = creatureQuestMap->equal_range(TEST_CREATURE_ID); + for (auto it = crRange.first; it != crRange.second; ) + { + if (it->second == TEST_QUEST_ID) + it = creatureQuestMap->erase(it); + else + ++it; + } + } + + static constexpr uint32 TEST_QUEST_ID = 99998; + static constexpr uint32 TEST_CREATURE_ID = 99999; + static constexpr uint32 TEST_POOL_ID = 99997; + + QuestRelations* creatureQuestMap = nullptr; +}; + +TEST_F(PoolQuestReloadFixTest, ReSpawnPoolQuestsRestoresQuestAfterReload) +{ + // 1. Spawn the quest onto the NPC (simulates normal startup) + PoolGroup poolGroup; + PoolObject obj(TEST_QUEST_ID, 0.0f); + poolGroup.Spawn1Object(&obj); + + auto count = [&]() { + uint32 n = 0; + auto range = creatureQuestMap->equal_range(TEST_CREATURE_ID); + for (auto it = range.first; it != range.second; ++it) + if (it->second == TEST_QUEST_ID) + ++n; + return n; + }; + + ASSERT_EQ(count(), 1u) << "Quest should be on creature before reload"; + + // 2. Simulate reload: clear creature quest map and repopulate pool mapping + creatureQuestMap->clear(); + sPoolMgr->mQuestCreatureRelation.clear(); + sPoolMgr->mQuestCreatureRelation.insert( + PooledQuestRelation::value_type(TEST_QUEST_ID, TEST_CREATURE_ID)); + + ASSERT_EQ(count(), 0u) << "Quest should be gone after reload clears the map"; + + // 3. THE FIX: ReSpawnPoolQuests re-inserts active pool quests + sPoolMgr->ReSpawnPoolQuests(); + + EXPECT_EQ(count(), 1u) + << "ReSpawnPoolQuests should restore active pool quests after reload"; +}