fix(Scripts/BossAI): add optional variable to BossAI class to make sure the boss calls for help

[elthehablo]

Some bosses make sure all nearby friendly creatures assist the boss when engaged. The problem here can be that this is only called on being engaged, which means that if a hunter uses FD right after pull, the boss will move to other targets (due to SetInCombatWithZone) but all the aggroed creatures will reset. This PR makes sure this call for help is called every 2 seconds during combat.

Problems may arise when we implement this on bosses that can move around a lot, as they would start calling every creature in the vicinity as they move around, which wouldn't be intentional. For the current 2 bosses I added this to in the PR, however, this is not an issue, as they have a boundary. And even at the max range of that boundary the range for call to help does not reach creatures it shouldn't.

Changes Proposed:

This PR proposes changes to:

• Core (units, players, creatures, game systems).
• Scripts (bosses, spell scripts, creature scripts).
• Database (SAI, creatures, etc).

Issues Addressed:

• Closes [TK] Voidreaver Trash not linked to boss chromiecraft/chromiecraft#6867

SOURCE:

The changes have been validated through:

• Live research (checked on live servers, e.g Classic WotLK, Retail, etc.)
• Sniffs (remember to share them with the open source community!)
• Video evidence, knowledge databases or other public sources (e.g forums, Wowhead, etc.)
• The changes promoted by this pull request come partially or entirely from another project (cherry-pick). Cherry-picks must be committed using the proper --author tag in order to be accepted, thus crediting the original authors, unless otherwise unable to be found

Tests Performed:

This PR has been:

• Tested in-game by the author.
• Tested in-game by other community members/someone else other than the author/has been live on production servers.
• This pull request requires further testing and may have edge cases to be tested.

How to Test the Changes:

• This pull request can be tested by following the reproduction steps provided in the linked issue
• This pull request requires further testing. Provide steps to test your changes. If it requires any specific setup e.g multiple players please specify it as well.

Known Issues and TODO List:

• implement for other relevant bosses (but that's as easy as giving a value for the float in the constructor, but bearing in mind my comment in the beginning about movement)

How to Test AzerothCore PRs

When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].

You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:

http://www.azerothcore.org/wiki/How-to-test-a-PR

REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).

For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.

[balleny]

this kind of "making sure groups arent split by buguse" should be implemented also for trash groups in raids and selectively applied to the group (so not calling the room but calling the npcs that belong together). e.g. the 4 initial mobs in AQ40, literally ALL groups formations in raids

[elthehablo]

this kind of "making sure groups arent split by buguse" should be implemented also for trash groups in raids and selectively applied to the group (so not calling the room but calling the npcs that belong together). e.g. the 4 initial mobs in AQ40, literally ALL groups formations in raids

Yes but that is not a trivial fix. So this comes first

[balleny]

of course its not trivial, thats also why this issue reappears every raid again.
unfortunately, all implementations so far have been hacky, incorrect or simply not working

bwl
chromiecraft/chromiecraft#3321 into chromiecraft/chromiecraft#4172 (last was also never reopened)

AQ
chromiecraft/chromiecraft#4297
chromiecraft/chromiecraft#6422

[Johaine]

Well, is there some consensus on how it should actually work? As far as I can tell, there a several issues, that are similar but not the same.

1. Calling for help: E.g. are NPCs "linked" dynamically on pull? What are the conditions and rules for that?
2. Separating statically linked groups.
3. Respawning statically linked groups

[balleny]

tldr:

1. when you engage a group, the full group engages (this usually works fine)
2. the group stays in combat together.

typical scenarios are:

1. hunter pulls & feign deaths, rogue pulls and vanish. everything resets
2. same as 1) but before the hunter feign deaths or the rogue vanishes, 1 player gets 1 or more ability / hit on a certain mob. that causes the feign death/vanish to reset all "other" mobs except for the one hit by the 2nd player.

and in 2) is the issue. I guess, the easiest solution would be as following: IF a mob is part of a group, engaging with that pack (e.g. by attacking a single mob, healing, buffing etc.) should add the player to the threat table of all mobs of that group or formation. It should not cause them to change their current aggro target, but they should be on the aggro table.

that way, if hunter pulls a group and someone attacks 1 target, a feign death will cause all mobs to go for that 2nd player after feign death.
technically, I dont know if 0 threat can be added on "first" interaction with a pack for every player or if 0 as a value would be ignored.
but from technical point of view, i guess it would be the easiest to cause the first "engagement" with a mob that is part of a formation to become part of the aggro table of each mob of that group.

alternatively, 1 approach would be that mobs "resetting", re-engage on the current target of a mob that is part of a formation and already attacking a target. so either the mobs check - or an mob infight calls their formation for assistance every second.

[balleny]

that way, you would be closer to blizzlike from what I would say, and remove a lot of exploit behavior - given mobs are "grouped" coherently across all raids

[Johaine]

Well, it is exactly that "alternatively, 1 approach would be that [...]" part that I'm curious about. Does anyone know if attacking a mob of a group actually adds threat for all mobs of that group of official servers? Or if its the latter, mobs automatically assisting their group members if their own threat table is empty for example.

Because both solutions don't really sound that complicated to implement but it's usually nice to know the correct approach before starting to code.

[balleny]

i understand that approach. its the decision between a temporarily solution or the correct solution.
talking from point of view of ChromieCraft, the exploitation would surely valid a temp. solution but AZ point of view might be different.

[Johaine]

It's not so much about temporary solution vs permanent solution but about correct vs incorrect solution. (Although a temporary fix is usually bad as "Nothing is more permanent than a temporary solution." When something is fixed 90% people tend to care much less about the remaining 10% and it will never be fixed completely.)

E.g., a temporary solution might be added to the current threat system. A permanent solution could probably be part of a complete threat rewrite ( #5985 ).

Even a permanent solution would probably not be accepted if it was incorrect and lead to regressions. From my experience, people can live with long-time bugs but will scream loudly as soon as something that worked before, suddenly doesn't.
So it must be abundantly clear what the correct behaviour is before thinking about any solution.

But, for example, can you tell me what should happen in this case:
Mob A and mob B are in the same group.
Hunter ("P1") shoots mob A. Mob B aggros as well.
Warrior ("P2") taunts and tanks Mob B.
Priest ("P3") heals P2 a lot.
P1 feigns death and disengages with mob A.

Currently, mob A would reset. Instead, which player should mob A attack now? P2 or P3?

[balleny]

Definite answer I cannot give cause I cannot test it on retail, but:
If P3 was in range of mob A while casting the heals, the heals will have added up to the threat table (because heals generate threat). The same way, rage gain etc. will have caused threat gain on mob A passively.

in case both have been out of range / distance to generate threat on mob A, imo mob A should start attacking the next closest target that is also on the threat table of mob B (or in cause of a group, on the next closest target that is on the combined threat table of the mobs forming a group)

an approach ala "the next best/closest target" if otherwise would force a reset - although that could have other issues.

regarding breaking:
yes, people will complain more about breaking changes than non-breaking changes. We have had a breaking change recently with respect to the movement of mobs and that was bad enough (and didnt even slow the issue fully).
But, so far I dont see what other mechanic would / could be affected, if mobs that are explicitly defined as a formation, stay engaged as a formation. Foremost, it would prevent exploitation.

there is also an approach to reverse this matter, that has other advantages and disadvantages:
so far we discussed how that 1 mob should act. Instead, what would also be possible (to circumvent the threat discussion) is reflecting the state of the mob pulled first, to all mobs on the group. If the mob pulled first, resets / evades, the whole formation should reset / evade, too (reset and if x npcs of that formation died, respawn accordingly)

would be a different approach - not be to blizzlike - but to reduce exploiting