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.

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

[Crash] Fix dangling Group member pointers for Bots. #3134

Merged
merged 2 commits into from
Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
126 changes: 74 additions & 52 deletions zone/bot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -425,11 +425,6 @@ Bot::Bot(uint32 botID, uint32 botOwnerCharacterID, uint32 botSpellsID, double to

cur_end = max_end;

// Safety Check to confirm we have a valid group
if (HasGroup() && !GetGroup()->IsGroupMember(GetBotOwner())) {
Bot::RemoveBotFromGroup(this, GetGroup());
}

// Safety Check to confirm we have a valid raid
if (HasRaid() && !GetRaid()->IsRaidMember(GetBotOwner()->CastToClient())) {
Bot::RemoveBotFromRaid(this);
Expand Down Expand Up @@ -3111,7 +3106,6 @@ Client* Bot::SetLeashOwner(Client* bot_owner, Group* bot_group, Raid* raid, uint
raid->GetGroupLeader(r_group)->CastToClient() : bot_owner;

} else if (bot_group) {
bot_group->VerifyGroup();
leash_owner = (bot_group->GetLeader() && bot_group->GetLeader()->IsClient() ? bot_group->GetLeader()->CastToClient() : bot_owner);

} else {
Expand Down Expand Up @@ -3321,11 +3315,18 @@ bool Bot::Spawn(Client* botCharacterOwner) {
}
}

if (Raid* raid = entity_list.GetRaidByBotName(GetName()))
{
Raid* raid = nullptr;
Group* group = nullptr;

if (raid = entity_list.GetRaidByBotName(GetName())) {
raid->VerifyRaid();
SetRaidGrouped(true);
}
else if (group = entity_list.GetGroupByMobName(GetName())) {
group->VerifyGroup();
SetGrouped(true);
}

return true;
}

Expand Down Expand Up @@ -3863,21 +3864,18 @@ bool Bot::RemoveBotFromGroup(Bot* bot, Group* group) {

bool Bot::AddBotToGroup(Bot* bot, Group* group) {
bool Result = false;
if (bot && group) {
// Add bot to this group
if (group->AddMember(bot)) {
if (group->GetLeader()) {
bot->SetFollowID(group->GetLeader()->GetID());
// Need to send this only once when a group is formed with a bot so the client knows it is also the group leader
if (group->GroupCount() == 2 && group->GetLeader()->IsClient()) {
group->UpdateGroupAAs();
Mob *TempLeader = group->GetLeader();
group->SendUpdate(groupActUpdate, TempLeader);
}
if (bot && group && group->AddMember(bot)) {
if (group->GetLeader()) {
bot->SetFollowID(group->GetLeader()->GetID());
// Need to send this only once when a group is formed with a bot so the client knows it is also the group leader
if (group->GroupCount() == 2 && group->GetLeader()->IsClient()) {
group->UpdateGroupAAs();
Mob *TempLeader = group->GetLeader();
group->SendUpdate(groupActUpdate, TempLeader);
}
group->VerifyGroup();
Result = true;
}
group->VerifyGroup();
Result = true;
}
return Result;
}
Expand Down Expand Up @@ -6680,7 +6678,11 @@ void Bot::Camp(bool save_to_database) {
Save();
}

Depop();
if (HasGroup() || HasRaid()) {
Zone();
} else {
Depop();
}
}

void Bot::Zone() {
Expand Down Expand Up @@ -8582,24 +8584,34 @@ void Bot::SpawnBotGroupByName(Client* c, const std::string& botgroup_name, uint3
return;
}

if (!leader->Spawn(c)) {
c->Message(
Chat::White,
fmt::format(
"Could not spawn bot-group leader {} for '{}'.",
leader->GetName(),
botgroup_name
).c_str()
);
safe_delete(leader);
return;
if (!leader->spawned) {
if (!leader->Spawn(c)) {
c->Message(
Chat::White,
fmt::format(
"Could not spawn bot-group leader {} for '{}'.",
leader->GetName(),
botgroup_name
).c_str()
);
safe_delete(leader);
return;
}
}

auto* g = new Group(leader);
auto group = leader->GetGroupByLeaderName();
auto raid = leader->GetRaid();

if (!raid && group) {
group->SetLeader(leader);
}
else if (!raid) {
group = new Group(leader);
entity_list.AddGroup(group);
database.SetGroupID(leader->GetCleanName(), group->GetID(), leader->GetBotID());
database.SetGroupLeaderName(group->GetID(), leader->GetCleanName());
}

entity_list.AddGroup(g);
database.SetGroupID(leader->GetCleanName(), g->GetID(), leader->GetBotID());
database.SetGroupLeaderName(g->GetID(), leader->GetCleanName());
leader->SetFollowID(c->GetID());

uint32 botgroup_id = 0;
Expand Down Expand Up @@ -8686,23 +8698,33 @@ void Bot::SpawnBotGroupByName(Client* c, const std::string& botgroup_name, uint3
continue;
}

if (!member->Spawn(c)) {
c->Message(
Chat::White,
fmt::format(
"Could not spawn bot '{}' (ID {}).",
member->GetName(),
member_iter
).c_str()
);
safe_delete(member);
return;
}
if (!member->spawned) {
if (!member->Spawn(c)) {
c->Message(
Chat::White,
fmt::format(
"Could not spawn bot '{}' (ID {}).",
member->GetName(),
member_iter
).c_str()
);
safe_delete(member);
return;
}

spawned_bot_count++;
bot_class_spawned_count[member->GetClass() - 1]++;
spawned_bot_count++;
bot_class_spawned_count[member->GetClass() - 1]++;

Bot::AddBotToGroup(member, g);
if (group) {
Bot::AddBotToGroup(member, group);
}
}
}

if (group) {
group->VerifyGroup();
} else if (raid) {
raid->VerifyRaid();
}

c->Message(
Expand Down
1 change: 1 addition & 0 deletions zone/bot.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ class Bot : public NPC {
bool HasGroup() final { return GetGroup() != nullptr; }
Raid* GetRaid() final { return entity_list.GetRaidByBot(this); }
Group* GetGroup() final { return entity_list.GetGroupByMob(this); }
Group* GetGroupByLeaderName() { return entity_list.GetGroupByLeaderName(GetName()); }

// Common, but informal "interfaces" with Client object
uint32 CharacterID() const { return GetBotID(); }
Expand Down
12 changes: 12 additions & 0 deletions zone/entity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2119,6 +2119,18 @@ Group *EntityList::GetGroupByMob(Mob *mob)
return nullptr;
}

Group *EntityList::GetGroupByMobName(const char* name)
{
for (const auto& g : group_list) {
for (const auto& m : g->membername) {
if (strcmp(m, name) == 0) {
return g;
}
}
}
return nullptr;
}

Group *EntityList::GetGroupByLeaderName(const char *leader)
{
std::list<Group *>::iterator iterator;
Expand Down
1 change: 1 addition & 0 deletions zone/entity.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ class EntityList
NPC* GetRandomNPC(const glm::vec3& location = glm::vec3(0.f), float distance = 0, NPC* exclude_npc = nullptr);
Mob* GetRandomMob(const glm::vec3& location = glm::vec3(0.f), float distance = 0, Mob* exclude_mob = nullptr);
Group* GetGroupByMob(Mob* mob);
Group* GetGroupByMobName(const char* name);
Group* GetGroupByBot(Bot* bot);
bool IsInSameGroupOrRaidGroup(Client *client1, Client *client2);
Group *GetGroupByClient(Client* client);
Expand Down
18 changes: 12 additions & 6 deletions zone/groups.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -547,11 +547,13 @@ bool Group::UpdatePlayer(Mob* update) {
void Group::MemberZoned(Mob* removemob) {
uint32 i;

if (removemob == nullptr)
if (!removemob) {
return;
}

if(removemob == GetLeader())
if (removemob == GetLeader()) {
SetLeader(nullptr);
}

//should NOT clear the name, it is used for world communication.
for (auto & m : members) {
Expand All @@ -560,17 +562,21 @@ void Group::MemberZoned(Mob* removemob) {
}
}

if(removemob->IsClient() && HasRole(removemob, RoleAssist))
if (removemob->IsClient() && HasRole(removemob, RoleAssist)) {
SetGroupAssistTarget(0);
}

if(removemob->IsClient() && HasRole(removemob, RoleTank))
if (removemob->IsClient() && HasRole(removemob, RoleTank)) {
SetGroupTankTarget(0);
}

if(removemob->IsClient() && HasRole(removemob, RolePuller))
if (removemob->IsClient() && HasRole(removemob, RolePuller)) {
SetGroupPullerTarget(0);
}

if (removemob->IsClient() && removemob == mentoree)
if (removemob->IsClient() && removemob == mentoree) {
mentoree = nullptr;
}

if (RuleB(Bots, Enabled)) {
Bot::UpdateGroupCastingRoles(this);
Expand Down