Skip to content

Fix Imp Fire Shield owner targeting while grouped. #3329

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

Open
mserajnik wants to merge 4 commits into
base: development
Choose a base branch
from
+9 −8
Open

Fix Imp Fire Shield owner targeting while grouped. #3329

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e89495a
Fix Imp Fire Shield owner targeting while grouped.
mserajnik Apr 3, 2026
0eeacbc
Remove explicit "autocast" mention from comment.
mserajnik Apr 4, 2026
264f32a
Refine comment wording.
mserajnik Apr 6, 2026
656807e
Add comment about charmed handling.
mserajnik Apr 8, 2026
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
15 changes: 7 additions & 8 deletions src/game/AI/PetAI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,12 +366,14 @@ void PetAI::UpdateAllies()
return;

//owner is in group; group members filled in already (no raid -> subgroupcount = whole count)
if (group && !group->isRaidGroup() && m_AllySet.size() == (group->GetMembersCount() + 2))
if (group && !group->isRaidGroup() && m_AllySet.size() == (group->GetMembersCount() + 1))
return;

m_AllySet.clear();
m_AllySet.insert(m_creature->GetObjectGuid());
if (group) //add group
m_AllySet.insert(owner->GetObjectGuid()); // The pet owner must always be a valid friendly target.
Copy link
Copy Markdown
Contributor

@ratkosrb ratkosrb Apr 7, 2026

Choose a reason for hiding this comment

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

If the owner is charmed it will not be a friendly target.

Copy link
Copy Markdown
Contributor Author

@mserajnik mserajnik Apr 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Good point, I forgot about that.

This makes the set size checks questionable because a lot can change within 10 seconds in regards to party composition and party members getting charmed.

I will have a look at this later and try to come up with a good solution.

Copy link
Copy Markdown
Contributor Author

@mserajnik mserajnik Apr 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Had a few minutes and checked how CMaNGOS does it and it looks like they also add the owner unconditionally: https://github.com/cmangos/mangos-classic/blob/db3c5cba390b8f02c709675780bc086dc88fec37/src/game/AI/BaseAI/PetAI.cpp#L429

But maybe they check if the potential ally is charmed later; will investigate once I am home, if they have a good solution it may be worth it aligning our handling here.

Copy link
Copy Markdown
Contributor Author

@mserajnik mserajnik Apr 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I looked into this:

Unless I misunderstood something when reading the code, with this PR we are essentially matching current CMaNGOS behavior: the owner is added unconditionally to m_AllySet and a charmed owner or party members are not filtered there. Instead, the charm state is validated later at cast time. I added a comment in UpdateAllies() to make that explicit.

In VMaNGOS, friendly-target pet casts go through Spell::CheckPetCast(), which rejects hostile targets via IsHostileTo(), so a charmed target is filtered there rather than when adding them to m_AllySet. CMaNGOS handles this in the same way conceptually as well, but through CanAssistSpell() / CanAssist() instead.

If we wanted to, we could additionally rebuild m_AllySet earlier when a charmed party member is detected and explicitly exclude charmed targets from the set, but I am not sure if that is worth adding here; since m_AllySet is only rebuilt every 10 seconds, that would mostly just invert the stale cache problem: a temporarily charmed target would stay excluded until the next set rebuild even after the charm ends. And since in that time frame also other charms can happen, we can not blindly rely on the set being correct anyway.

Unrelated, but I also noticed that CMaNGOS likely has a bug in the GetMembersCount() + 2 check, because GetMembersCount() already includes the owner there as well, just like in VMaNGOS (so it should probably be + 1 instead there as well).


if (group)
{
for (GroupReference* itr = group->GetFirstMember(); itr != nullptr; itr = itr->next())
{
Expand All @@ -385,8 +387,6 @@ void PetAI::UpdateAllies()
m_AllySet.insert(target->GetObjectGuid());
}
}
else //remove group
m_AllySet.insert(owner->GetObjectGuid());
}

void PetAI::KilledUnit(Unit* victim)
Expand Down Expand Up @@ -523,7 +523,7 @@ std::pair<Unit*, ePetSelectTargetReason> PetAI::SelectNextTarget() const
Unit* owner = m_creature->GetCharmerOrOwner();
if (!owner)
return std::make_pair(nullptr, PSTR_FAIL_NO_OWNER);

if (Creature const* pOwnerCreature = owner->ToCreature())
{
// Owner is creature and is evading. We must not re-aggro.
Expand Down Expand Up @@ -552,7 +552,7 @@ std::pair<Unit*, ePetSelectTargetReason> PetAI::SelectNextTarget() const
{
if (Unit* pVictim = owner->GetVictim())
{
if (!pVictim->HasAuraPetShouldAvoidBreaking() &&
if (!pVictim->HasAuraPetShouldAvoidBreaking() &&
(!m_creature->GetCharmInfo()->IsAtStay() || m_creature->CanReachWithMeleeAutoAttack(pVictim)))
return std::make_pair(pVictim, PSTR_SUCCESS_OWNER_VICTIM);
}
Expand Down Expand Up @@ -727,7 +727,7 @@ bool PetAI::CanAttack(Unit* target)
// CC - mobs under crowd control can be attacked if owner commanded
if (target->HasAuraPetShouldAvoidBreaking())
return m_creature->GetCharmInfo()->IsCommandAttack();

// Returning - pets ignore attacks only if owner clicked follow
if (m_creature->GetCharmInfo()->IsReturning())
return !m_creature->GetCharmInfo()->IsCommandFollow();
Expand Down Expand Up @@ -801,4 +801,3 @@ void PetAI::AttackedBy(Unit* attacker)
// Continue to evaluate and attack if necessary
AttackStart(attacker);
}