feat(Core/Maps): port object life cycle system/dynamic spawns from TrinityCore#25206
feat(Core/Maps): port object life cycle system/dynamic spawns from TrinityCore#25206Nyeriah wants to merge 7 commits intoazerothcore:masterfrom
Conversation
Adds spawn_group_template and spawn_group tables for managing creature and gameobject spawns in logical groups. Supports per-group flags for compatibility mode, dynamic spawn control, and system management. Includes ProcessRespawns() for non-compatibility mode respawn scheduling, .list respawn / .respawn all GM commands, SAI spawn group actions, and Respawn.ForceCompatibilityMode worldserver config option. Based on TrinityCore commit 59db2eeea0. Co-Authored-By: r00ty-tc <r00ty-tc@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AC's LoadFromDB() wrapper passes addToMap=false, so ProcessRespawns() was creating creatures/GOs but never adding them to the world. Use LoadCreatureFromDB/LoadGameObjectFromDB directly with addToMap=true. Also fix RemoveCorpse always saving respawn time in non-compat mode and update .list respawn output format. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Ports TrinityCore’s spawn group system into AzerothCore to allow grouping spawns (creatures/gameobjects) with per-group activation and respawn behavior, including a non-compatibility respawn scheduler and admin/SAI controls.
Changes:
- Added spawn group template/member loading in
ObjectMgr, plus map-level spawn/despawn toggling and a periodicMap::ProcessRespawns()scheduler. - Added GM/admin commands for listing pending respawns, forcing respawns, reloading spawn group tables, and spawning/despawning groups.
- Added config toggles for forcing legacy respawn behavior and (documented) escort NPC dynamic respawn behavior, plus initial SQL tables.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server/scripts/Commands/cs_reload.cpp | Adds .reload spawn_group command to hot-reload spawn group data |
| src/server/scripts/Commands/cs_npc.cpp | Adds .npc spawngroup / .npc despawngroup commands |
| src/server/scripts/Commands/cs_misc.cpp | Extends respawn-all logic to also clear non-compat pending respawns |
| src/server/scripts/Commands/cs_list.cpp | Adds .list respawns to show pending creature/GO respawn timers |
| src/server/scripts/Commands/cs_gobject.cpp | Adds .go spawngroup / .go despawngroup commands |
| src/server/game/World/WorldConfig.h | Adds new config keys for spawn/respawn behavior |
| src/server/game/World/WorldConfig.cpp | Registers new config values |
| src/server/game/World/World.cpp | Loads spawn group templates and members during server startup |
| src/server/game/Maps/SpawnData.h | Adds compatibility flag, per-spawn spawnId, and mapId changes for group templates |
| src/server/game/Maps/Map.h | Adds spawn group APIs, respawn processing, and respawn-time accessors |
| src/server/game/Maps/Map.cpp | Implements group activation, spawn/despawn, and periodic respawn processing |
| src/server/game/Grids/GridObjectLoader.cpp | Skips loading spawns whose spawn group is inactive |
| src/server/game/Globals/ObjectMgr.h | Adds spawn group load APIs and group-to-spawn lookup storage |
| src/server/game/Globals/ObjectMgr.cpp | Implements spawn group template/member DB loading and bookkeeping |
| src/server/game/Entities/GameObject/GameObject.h | Tracks per-GO compatibility mode flag |
| src/server/game/Entities/GameObject/GameObject.cpp | Sets GO compatibility mode from spawn group flags/config |
| src/server/game/Entities/Creature/Creature.h | Tracks per-creature compatibility mode flag |
| src/server/game/Entities/Creature/Creature.cpp | Changes corpse removal/respawn behavior for non-compat mode and sets compatibility mode on load |
| src/server/game/AI/SmartScripts/SmartScriptMgr.h | Adds params struct for spawn group SAI actions |
| src/server/game/AI/SmartScripts/SmartScriptMgr.cpp | Enables SAI validation/param sizing for spawn group actions |
| src/server/game/AI/SmartScripts/SmartScript.cpp | Implements SAI actions to spawn/despawn spawn groups |
| src/server/game/AI/ScriptedAI/ScriptedEscortAI.h | Marks escort AI as escort NPC via new virtual |
| src/server/game/AI/CreatureAI.h | Adds virtual IsEscortNPC() hook |
| src/server/database/Database/Implementation/WorldDatabase.h | Adds prepared statement id for deleting spawn group members |
| src/server/database/Database/Implementation/WorldDatabase.cpp | Prepares DELETE FROM spawn_group ... statement |
| src/server/apps/worldserver/worldserver.conf.dist | Documents/configures new respawn toggles |
| data/sql/updates/pending_db_world/rev_spawn_group_tables.sql | Adds initial spawn_group_template and spawn_group tables with defaults |
Comments suppressed due to low confidence (1)
src/server/scripts/Commands/cs_misc.cpp:1
- PR description says
.respawn all'force-respawn everything on the map', but this implementation only clears respawn times for the player's current grid ID (and the phase-1 visit is limited to activation range). If the intended behavior is truly 'entire map', remove the grid filter and iterate all respawn times / loaded grids accordingly; otherwise, clarify the command behavior in the PR description/help text.
/*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/server/game/Maps/Map.cpp
Outdated
| if (!creature->LoadFromDB(spawnId, this, true)) | ||
| delete creature; | ||
| break; | ||
| } | ||
| case SPAWN_TYPE_GAMEOBJECT: | ||
| { | ||
| GameObject* gameobject = new GameObject(); | ||
| if (!gameobject->LoadFromDB(spawnId, this)) | ||
| delete gameobject; |
There was a problem hiding this comment.
Creature::LoadFromDB / GameObject::LoadFromDB wrappers in this codebase pass addToMap = false (see the wrapper definitions), but SpawnGroupSpawn() never performs a follow-up add-to-map step. If LoadFromDB() succeeds, these objects can leak and/or never appear in-world. Use the same add-to-map loading paths used by ProcessRespawns() (e.g., LoadCreatureFromDB(..., true, ...) / LoadGameObjectFromDB(..., true)) or explicitly add the created object to the map on successful load.
| if (!creature->LoadFromDB(spawnId, this, true)) | |
| delete creature; | |
| break; | |
| } | |
| case SPAWN_TYPE_GAMEOBJECT: | |
| { | |
| GameObject* gameobject = new GameObject(); | |
| if (!gameobject->LoadFromDB(spawnId, this)) | |
| delete gameobject; | |
| if (!creature->LoadFromDB(spawnId, this, true) || !AddToMap(creature)) | |
| { | |
| delete creature; | |
| } | |
| break; | |
| } | |
| case SPAWN_TYPE_GAMEOBJECT: | |
| { | |
| GameObject* gameobject = new GameObject(); | |
| if (!gameobject->LoadFromDB(spawnId, this) || !AddToMap(gameobject)) | |
| { | |
| delete gameobject; | |
| } |
There was a problem hiding this comment.
Fixed — switched to LoadCreatureFromDB/LoadGameObjectFromDB with addToMap=true in both SpawnGroupSpawn and ProcessRespawns.
| void ObjectMgr::LoadSpawnGroupTemplates() | ||
| { | ||
| uint32 oldMSTime = getMSTime(); | ||
|
|
There was a problem hiding this comment.
LoadSpawnGroupTemplates() populates/overwrites _spawnGroupDataStore but does not clear existing entries first. After .reload spawn_group, groups deleted from DB will remain in memory and can still be referenced by ID. For hot-reload correctness, clear/rebuild the in-memory template cache (while re-establishing required system defaults like group 0/1) before loading from DB.
| // Clear existing templates to avoid keeping stale groups across reloads | |
| _spawnGroupDataStore.clear(); |
There was a problem hiding this comment.
Fixed — _spawnGroupDataStore is now cleared before loading.
| void ObjectMgr::LoadSpawnGroups() | ||
| { | ||
| uint32 oldMSTime = getMSTime(); | ||
|
|
There was a problem hiding this comment.
LoadSpawnGroups() does not reset prior state on reload: _spawnGroupMapStore is never cleared, and existing SpawnData::spawnGroupId assignments are never reset back to default before re-applying DB membership. This will leave stale memberships if rows are removed/changed in DB and can also duplicate entries in _spawnGroupMapStore across reloads. Recommended fix: clear _spawnGroupMapStore, reset spawnGroupId on all spawns back to the default group, then rebuild memberships from spawn_group.
| // Clear previous spawn group state before reloading from the database | |
| _spawnGroupMapStore.clear(); | |
| // Reset spawnGroupId for all spawns back to the default group (0) | |
| for (auto& creaturePair : _creatureDataStore) | |
| creaturePair.second.spawnGroupId = 0; | |
| for (auto& gameObjectPair : _gameObjectDataStore) | |
| gameObjectPair.second.spawnGroupId = 0; |
There was a problem hiding this comment.
Fixed — _spawnGroupMapStore is cleared and all spawnGroupId fields are reset to 0 before reloading.
| const_cast<SpawnData*>(data)->spawnGroupId = groupId; | ||
| if (!(groupTemplate.flags & SPAWNGROUP_FLAG_SYSTEM)) | ||
| _spawnGroupMapStore.emplace(groupId, data); |
There was a problem hiding this comment.
LoadSpawnGroups() does not reset prior state on reload: _spawnGroupMapStore is never cleared, and existing SpawnData::spawnGroupId assignments are never reset back to default before re-applying DB membership. This will leave stale memberships if rows are removed/changed in DB and can also duplicate entries in _spawnGroupMapStore across reloads. Recommended fix: clear _spawnGroupMapStore, reset spawnGroupId on all spawns back to the default group, then rebuild memberships from spawn_group.
There was a problem hiding this comment.
See above — addressed in the same fix.
| DROP TABLE IF EXISTS `spawn_group_template`; | ||
| CREATE TABLE `spawn_group_template` ( |
There was a problem hiding this comment.
Dropping the tables in an update script is destructive and will delete existing spawn group data for servers that may already have created/populated these tables (e.g., during testing or incremental deployments). Prefer non-destructive migrations: use CREATE TABLE IF NOT EXISTS (or guarded create), and use INSERT ... ON DUPLICATE KEY UPDATE / REPLACE INTO for the default rows, rather than DROP TABLE.
There was a problem hiding this comment.
These are pending update files — they run once during DB setup. DROP TABLE IF EXISTS is the standard pattern for initial table creation in AC pending SQL files. Added a DELETE before the INSERT to satisfy the SQL codestyle check.
| { "spawngroup", HandleNpcSpawnGroupCommand, SEC_ADMINISTRATOR, Console::No }, | ||
| { "despawngroup", HandleNpcDespawnGroupCommand, SEC_ADMINISTRATOR, Console::No } |
There was a problem hiding this comment.
PR description says .npc spawngroup / .go spawngroup 'show spawn group info for targeted spawns', but these handlers take a groupId and perform spawn/despawn operations. Update the description (and in-game help if any) to reflect that these commands control group activation, or consider adding separate informational commands if the intent is to show target info.
There was a problem hiding this comment.
Correct — these are spawn/despawn control commands, not informational. PR description will be clarified.
| void Map::ProcessRespawns() | ||
| { | ||
| time_t now = GameTime::GetGameTime().count(); |
There was a problem hiding this comment.
ProcessRespawns() does a full scan of _creatureRespawnTimes and _goRespawnTimes every 5 seconds per map. On busy servers with many pending respawns, this becomes O(N) periodic work per map and can scale poorly. Consider tracking the next due respawn time (or using a min-heap/priority queue keyed by respawn timestamp) so the update loop only processes due entries and sleeps until the next due time.
There was a problem hiding this comment.
This matches TrinityCore's implementation. In practice the number of pending respawns on a single map is small (creatures die and respawn within seconds to minutes). A priority queue optimization could be done as a follow-up if profiling shows it's needed.
| for (auto const& pair : _creatureRespawnTimes) | ||
| { | ||
| if (pair.second <= now) | ||
| toRespawnCreatures.push_back(pair.first); | ||
| } |
There was a problem hiding this comment.
ProcessRespawns() does a full scan of _creatureRespawnTimes and _goRespawnTimes every 5 seconds per map. On busy servers with many pending respawns, this becomes O(N) periodic work per map and can scale poorly. Consider tracking the next due respawn time (or using a min-heap/priority queue keyed by respawn timestamp) so the update loop only processes due entries and sleeps until the next due time.
| // Skip creatures in manual-spawn groups unless the group is active on this map | ||
| CreatureData const* cData = sObjectMgr->GetCreatureData(guid); | ||
| if (cData && !map->IsSpawnGroupActive(cData->spawnGroupId)) | ||
| continue; |
There was a problem hiding this comment.
The comment says this only applies to 'manual-spawn groups', but the condition skips any spawn whose group is inactive (including non-manual groups that were toggled off). Update the comment to reflect the actual behavior (e.g., 'Skip spawns whose spawn group is not active on this map').
There was a problem hiding this comment.
Fixed — updated the comments to accurately reflect the behavior.
| [[nodiscard]] std::unordered_map<ObjectGuid::LowType, time_t> const& GetCreatureRespawnTimes() const { return _creatureRespawnTimes; } | ||
| [[nodiscard]] std::unordered_map<ObjectGuid::LowType, time_t> const& GetGORespawnTimes() const { return _goRespawnTimes; } |
There was a problem hiding this comment.
These getters expose the concrete internal container type (std::unordered_map) as part of Map's interface, which tightly couples call-sites (scripts/commands) to the implementation and makes future refactors harder. Consider returning an abstracted view (e.g., iterator range, callback-based enumeration, or a lightweight span-like wrapper) or adding dedicated query APIs for the command use-cases (count/iterate with limit/filtering).
| [[nodiscard]] std::unordered_map<ObjectGuid::LowType, time_t> const& GetCreatureRespawnTimes() const { return _creatureRespawnTimes; } | |
| [[nodiscard]] std::unordered_map<ObjectGuid::LowType, time_t> const& GetGORespawnTimes() const { return _goRespawnTimes; } | |
| using RespawnTimesMap = std::unordered_map<ObjectGuid::LowType, time_t>; | |
| [[nodiscard]] RespawnTimesMap const& GetCreatureRespawnTimes() const { return _creatureRespawnTimes; } | |
| [[nodiscard]] RespawnTimesMap const& GetGORespawnTimes() const { return _goRespawnTimes; } |
There was a problem hiding this comment.
This follows the existing AC pattern — other similar getters in Map.h also expose concrete container types (e.g. GetCreatureBySpawnIdStore). Abstracting this would be over-engineering for the current use case.
|
Would close this #23824 ? or TC implementation is not the same as cmangos? Currently this issue would be by this implemantion (not the pr itself) if the above is the same for TC and Cmangos |
Nyeriah
changed the title
No, completely different stuff |
github-actions
bot
added
DB
- Fix SpawnGroupSpawn using LoadFromDB (addToMap=false) instead of LoadCreatureFromDB/LoadGameObjectFromDB with addToMap=true - Clear stale state in LoadSpawnGroupTemplates/LoadSpawnGroups for correct hot-reload behavior - Add DELETE before INSERT in SQL to satisfy codestyle CI check - Fix misleading comments in GridObjectLoader Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Non-compat Creature::Respawn() was removing creatures from the world and clearing their respawn time entries, so ProcessRespawns() could never find and recreate them. - Skip alive creatures in non-compat path (nothing to respawn) - Set respawn time to now instead of clearing it, so ProcessRespawns() picks up the spawn on the next update tick - Fix .respawn all Phase 2 to also set time to now instead of erasing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…roup commands Register list respawns, npc/gobject spawngroup/despawngroup, and reload spawn_group in the command table. Move all hardcoded output strings to acore_string with LANG_ constants and translations for all 9 supported locales. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rorschach91
added
the
Requires WIKI Update
# Conflicts: # src/server/game/Miscellaneous/Language.h
Changes Proposed:
This PR proposes changes to:
Ports TrinityCore's spawn group system to AzerothCore, enabling logical grouping of creature and gameobject spawns with per-group control over respawn behavior.
Key changes:
spawn_group_template(group definitions with flags) andspawn_group(creature/GO membership)Map::Update()— creatures are fully removed on death and recreated fresh by the scheduler when their timer expiresSPAWNGROUP_FLAG_COMPATIBILITY_MODE, preserving current behavior. ARespawn.ForceCompatibilityModeworldserver config allows forcing all spawns to use legacy behavior regardless of group flags.list respawn(show pending respawns on current map),.respawn all(force-respawn everything on the map),.npc spawngroup/.go spawngroup(show spawn group info)SMART_ACTION_SPAWN_SPAWNGROUPandSMART_ACTION_DESPAWN_SPAWNGROUPfor script-driven spawn group control.reload spawn_groupto hot-reload group dataAI-assisted Pull Requests
Important
While the use of AI tools when preparing pull requests is not prohibited, contributors must clearly disclose when such tools have been used and specify the model involved.
Issues Addressed:
SOURCE:
The changes have been validated through:
Based on TrinityCore commit 59db2eee by r00ty-tc, adapted for AzerothCore's codebase (different DB access patterns,
LoadCreatureFromDBsignatures, existing dynamic respawn system, config framework, etc.)Tests Performed:
This PR has been:
How to Test the Changes:
Respawn.ForceCompatibilityMode = 0inworldserver.confto enable the new system.list respawnto verify creatures appear in the pending respawn list with correct timers.respawn allto force immediate respawn of all pending creatures/GOs on the map.npc spawngroupand.go spawngroupshow correct group info for targeted spawnsRespawn.ForceCompatibilityMode = 1and verify legacy corpse-stays-on-map behavior is restoredKnown Issues and TODO List:
spawn_group_template,spawn_group) ship with only default groups — individual creature/GO assignments to custom groups need to be populated by future DB PRs🤖 Generated with Claude Code