Repository navigation
Conversation
Resurrectable corpses are world objects owned by the CorpseManager and outlive their grid. When the map itself goes away - an ended battleground, an unloaded instance - the corpse kept pointing at the deleted map, and the next visibility check of its dead owner (Creature::IsVisibleInGridForPlayer -> InReach -> InCommonFrame) read the freed map and crashed the server. Seen with a player who died in Warsong Gulch, logged out as a ghost and logged in after the battleground was gone. The grid unloader now takes the corpses of the grid out of the world and clears their map; the grid loader puts them back as before. InCommonFrame uses FindMap(), and the corpse users (both visibility checks, corpse reclaim) skip a corpse that is not in the world.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 6 |
| Duplication | 2 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Corpse unloading can detach corpses from still-loaded cells or unrelated maps, leaving players unable to see or reclaim them.
2 open issues
What changed in this PR
This PR addresses a dead-player login crash by detaching persistent player corpses from unloaded maps and guarding operations that require an in-world corpse.
Changes:
- Add a world-object unload visitor that removes corpses from the world and clears their map pointers.
- Use nullable map lookup in position comparisons.
- Skip out-of-world corpses during visibility checks and corpse reclaim.
| File | Description |
|---|---|
| src/game/WorldHandlers/ObjectGridLoader.h | Declare the corpse-detachment visitor. |
| src/game/WorldHandlers/ObjectGridLoader.cpp | Visit world-object corpses during unloading and detach them. |
| src/game/WorldHandlers/MiscHandler.cpp | Reject reclaim requests for out-of-world corpses. |
| src/game/Object/WorldObjectPosition.cpp | Use FindMap() for nullable map access. |
| src/game/Object/PlayerVisibility.cpp | Ignore out-of-world corpses in player visibility checks. |
| src/game/Object/Object.h | Add ForgetMap() to clear retained map pointers. |
| src/game/Object/Creature.cpp | Ignore out-of-world corpses in creature visibility checks. |
Review effort: Balanced
Comment on lines
+464
to
+466
| corpse->RemoveFromWorld(); | ||
| } | ||
| corpse->ForgetMap(); |
Author
There was a problem hiding this comment.
Agreed, and that matching was already wrong before this change (a corpse could be linked into another map's grid with the same coordinates). Fixed in 30fedf4: CorpseManager::AddCorpsesToGrid() now skips corpses whose map ID is not the loading map's, before the instance check.
AddCorpsesToGrid() links every corpse of a grid into the cell that loaded it, so detaching corpses in ObjectGridUnloader::Unload() also ran for Map::UnloadCell() and could take corpses of cells that stay loaded out of the world. Detach them in UnloadN() only. AddCorpsesToGrid() also matched the grid coordinates without the map, so loading a continent grid could link another map's corpse into it. Skip corpses of other maps.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resurrectable player corpses are world objects owned by the
CorpseManagerand outlive their grid.ObjectGridUnloaderonly visits theGridTypeMapContainerandWorldObject::ResetMap()is empty, so a corpse kept itsm_currMapafter the grid unloaded. When the map itself was deleted - a battleground that ended while its owner was offline as a ghost - the next visibility check of the dead owner read the freed map and crashed the server:Creature::IsVisibleInGridForPlayer->InReach(*corpse, ...)->InCommonFrame->corpse->GetMap()->AsTransport()ObjectGridUnloader::UnloadN(), newObjectWorldUnloader,WorldObject::ForgetMap()); a single-cell unload (Map::UnloadCell()) leaves them alone, sinceAddCorpsesToGrid()links every corpse of the grid into the loading cell.LoadHelper(CellCorpseSet)puts them back withSetMap/AddToWorldwhen the grid loads again, as beforeCorpseManager::AddCorpsesToGrid(): it compared only the grid coordinates, so another map's corpse could be linked into the gridFindMap()instead ofGetMap()inInCommonFrameCreature::IsVisibleInGridForPlayer,Player::IsVisibleInGridForPlayerandHandleReclaimCorpseOpcode, as the note onMap::GetCorpsealready asksVerification
mangosdin Release and RelWithDebInfoInCommonFrameread aboveMAX_OFFLINE_TIME, the battleground map unloaded (corpse detached), then logged in dead with the corpse out of the world - no crash, then revived normallyCorpses loaded from the database already stay out of the world until their grid loads; a corpse in an unloaded grid now behaves the same, so
CorpseManager::Findno longer returns it until then (FindForPlayeris unchanged).No database, configuration, or submodule changes are included.
This change is