Pet/Guardian AI hook re-organizing#19824
Merged
Merged
Conversation
- Adjust OwnerAttacked/OwnerAttackedBy hooks on CreatureAI to fire for all owned units, not just player pets. This should allow guardians to more reliably recognize valid targets. - Kill off the AttackedBy hook. While it was defined in CreatureAI.h as virtual, it was only ever invoked for player pets in specific situations. This makes it classic developer bait. - Adjust PetAI to use DamageTaken instead of AttackedBy. - Adjust behavior of AttackStart on PetAI to compensate.
Member
|
Shouldn't AttackedBy fire on events that cause aggro but not deal damage like taunt/debuff/dot without instant damage component? |
Contributor
Author
|
In current core, EnterCombat is used if it's initial aggro, plus various spell related hooks. |
Member
|
Well, all of the scenarios I described are valid even as non-initial-aggro |
Contributor
Author
|
Yes, but currently
|
ccrs
reviewed
May 30, 2017
| if (!attacker || !me->IsAlive()) | ||
| return; | ||
|
|
||
| if (!me->HasReactState(REACT_PASSIVE) && me->CanStartAttack(attacker, false)) |
Contributor
There was a problem hiding this comment.
me->CanStartAttack(attacker, false)
prevents combat with neutral units that the owner is attacking
me->CanStartAttack(attacker, true)
testing with force = true
Contributor
There was a problem hiding this comment.
actually makes sense for it to be true, faction check can be sort of ignored and distance check is useless
You were supposed to tell me whether it worked without force, not lead me into darkness.
Shauren
requested changes
May 30, 2017
|
|
||
| // Called when owner attacks something | ||
| virtual void OwnerAttacked(Unit* /*target*/) { } | ||
| virtual void OwnerAttacked(Unit* target) { CreatureAI::OwnerAttackedBy(target); } |
Member
There was a problem hiding this comment.
You talked about traps for scripters but here you add another one - it would be better to introduce a separate (nonvirtual) method that is called by both OwnerAttacked and OwnerAttackedBy. Someone less experienced might think that if he overrides OwnerAttackedBy in his own script but not OwnerAttacked that it will call his overriden OwnerAttackedBy (but it will be the base CreatureAI one)
Aokromes
added a commit
to Aokromes/TrinityCore
that referenced
this pull request
Jun 7, 2017
* Pet/Guardian AI hook re-organizing: - Adjust OwnerAttacked/OwnerAttackedBy hooks on CreatureAI to fire for all owned units, not just player pets. This should allow guardians to more reliably recognize valid targets. - Kill off the AttackedBy hook. While it was defined in CreatureAI.h as virtual, it was only ever invoked for player pets in specific situations. This makes it classic developer bait. - Adjust PetAI to use DamageTaken instead of AttackedBy. - Adjust behavior of AttackStart on PetAI to compensate.
Carbenium
pushed a commit
to Carbenium/TrinityCore
that referenced
this pull request
Jun 24, 2020
* Pet/Guardian AI hook re-organizing: - Adjust OwnerAttacked/OwnerAttackedBy hooks on CreatureAI to fire for all owned units, not just player pets. This should allow guardians to more reliably recognize valid targets. - Kill off the AttackedBy hook. While it was defined in CreatureAI.h as virtual, it was only ever invoked for player pets in specific situations. This makes it classic developer bait. - Adjust PetAI to use DamageTaken instead of AttackedBy. - Adjust behavior of AttackStart on PetAI to compensate. (cherry picked from commit 1660bb7)
Carbenium
pushed a commit
to Carbenium/TrinityCore
that referenced
this pull request
Jun 25, 2020
* Pet/Guardian AI hook re-organizing: - Adjust OwnerAttacked/OwnerAttackedBy hooks on CreatureAI to fire for all owned units, not just player pets. This should allow guardians to more reliably recognize valid targets. - Kill off the AttackedBy hook. While it was defined in CreatureAI.h as virtual, it was only ever invoked for player pets in specific situations. This makes it classic developer bait. - Adjust PetAI to use DamageTaken instead of AttackedBy. - Adjust behavior of AttackStart on PetAI to compensate. (cherry picked from commit 1660bb7)
Shauren
pushed a commit
to Carbenium/TrinityCore
that referenced
this pull request
Jul 16, 2020
* Pet/Guardian AI hook re-organizing: - Adjust OwnerAttacked/OwnerAttackedBy hooks on CreatureAI to fire for all owned units, not just player pets. This should allow guardians to more reliably recognize valid targets. - Kill off the AttackedBy hook. While it was defined in CreatureAI.h as virtual, it was only ever invoked for player pets in specific situations. This makes it classic developer bait. - Adjust PetAI to use DamageTaken instead of AttackedBy. - Adjust behavior of AttackStart on PetAI to compensate. (cherry picked from commit 1660bb7)
Shauren
pushed a commit
to Carbenium/TrinityCore
that referenced
this pull request
Jul 16, 2020
* Pet/Guardian AI hook re-organizing: - Adjust OwnerAttacked/OwnerAttackedBy hooks on CreatureAI to fire for all owned units, not just player pets. This should allow guardians to more reliably recognize valid targets. - Kill off the AttackedBy hook. While it was defined in CreatureAI.h as virtual, it was only ever invoked for player pets in specific situations. This makes it classic developer bait. - Adjust PetAI to use DamageTaken instead of AttackedBy. - Adjust behavior of AttackStart on PetAI to compensate. (cherry picked from commit 1660bb7)
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
3 participants
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.
Pet/Guardian AI hook re-organizing: