Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vehicles: Additional cleanup #583

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Vehicles: Additional cleanup #583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1208a51
Vehicles: Additional cleanup
insunaa Jan 4, 2025
1873d7a
Vehicle: Set Info to nullptr after cleanup
insunaa Jan 4, 2025
c375349
Chat: Fix wrong argument count in gob nearspawned
insunaa Jan 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/game/Chat/Level2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1409,7 +1409,7 @@ bool ChatHandler::HandleGameObjectNearSpawnedCommand(char* args)
uint32 spawnGroupId = 0;
if (SpawnGroupEntry* groupEntry = player->GetMap()->GetMapDataContainer().GetSpawnGroupByGuid(guid, TYPEID_GAMEOBJECT))
spawnGroupId = groupEntry->Id;
PSendSysMessage(LANG_GO_MIXED_LIST_CHAT, guid.GetCounter(), PrepareStringNpcOrGoSpawnInformation<GameObject>(guid).c_str(), entry, guid.GetCounter(), entry, goInfo->name, x, y, z, go->GetMapId(), spawnGroupId);
PSendSysMessage(LANG_GO_MIXED_LIST_CHAT, guid.GetCounter(), PrepareStringNpcOrGoSpawnInformation<GameObject>(guid).c_str(), entry, guid.GetCounter(), goInfo->name, x, y, z, go->GetMapId(), spawnGroupId);
}

PSendSysMessage(LANG_COMMAND_NEAROBJMESSAGE, distance, gameobjects.size());
Expand Down
5 changes: 5 additions & 0 deletions src/game/Entities/Unit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10631,6 +10631,11 @@ void Unit::CleanupsBeforeDelete()
ClearComboPointHolders();
RemoveAllAuras(AURA_REMOVE_BY_DELETE);
}
if (m_vehicleInfo)
{
m_vehicleInfo->Cleanup();
m_vehicleInfo = nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the nullptr is unnecessary. It will happen when unit is deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought so too, and then I had a crash
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I mean sure, that fails due to MANGOS_ASSERT(m_passengers.empty()); but I fail to see how calling destructor (by setting the ptr to nullptr) fixes that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because setting it to nullptr early means that IsVehicle() or if (m_vehicleInfo) returns false, both of which are used in other aspects of the cleanup.

I haven't yet been able to provoke this same crash again, the server has been running stably for ~2 hours now with no crashes since I added the = nullptr; and I'll keep it running some more to see if maybe the crash re-occurs. If it doesn't then I'll just assume setting it to nullptr fixed the issue. If it does I'll have to investigate some more. In any case this crash occurred as shown in the crashlog and I couldn't find any other reason for why passengers might not be empty when the crash occurred

}
WorldObject::CleanupsBeforeDelete();
}

Expand Down