[6.x] Core/Spells: Removed a faulty aura handler #17590
Conversation
…ations Instances have since MoP (Possibly even Cataclysm) been able to set new locations for players when they zone into the instance. A theoretic example of this could be after a certain boss has been defeated, a new entrance location is set to bring players entering the instance closer to the relevant content. A real example would be Siege of Orgrimmar changing entrance locations many times as the raid progresses. Added SetEntranceLocation for setting entrance locations that will be saved to DB the next time the instance is saved, and SetTemporaryEntranceLocation for when you don't want the value to be saved to the database.
…ance locations" This reverts commit c95ef34.
|
what interrupt flag? |
|
None in particular...? |
|
RemoveAurasWithInterruptFlags properly interrupts auras when needed, since the spells have the interrupt flags, and this function just goes in and removes all triggered auras, with no exceptions. |
|
I really don't see how this will work as expected for proc trinkets applying a buff which in turn periodically triggers another buff. |
|
The exception should not govern the general behavior. The fact is that this function is completely 100% wrong |
|
I gave examples of why its needed, expecting some proof on why this function is wrong now |
|
Okay fair enough, so where would be best to fix these cases, can it be done in db? |
|
These are spells that periodically applies an aura, and then expires, expecting the aura to still be applied. |
|
I'm pretty sure Blizzard does not design spells like the one you linked anymore. I think it's best to just deal with the spells like the one that you just linked and handle them case by case with a spellscript / db entry (not sure if db can deal with this). |
|
In this case I think you should provide such script for the trinkets that are known to be broken by this |
|
I don't mind. |
|
Shall I update this PR with the spell proc fixes or make a new one, refering to this PR? |
|
Added some aura proc fixes to this PR, let me know if you want separate them. |
|
The aura scripts could be replaced by a single generic script, so that it can be easily added to other spells using db, and would save a LOT of code duplication. Just check in validate() if the spell has a correct aura with existing triggered spell, and in register() register the handler only to that effect. No need to check if the spell you're scripting exists in db as the script loader does that for you, you only need to check existence of spells you're referencing. |
|
I disagree with using a single script to handle multiple spells, just because some of them happen to use the same behavior as other scripts. |
|
This branch has conflicts that must be resolved |
Aokromes
added
Feedback-TestRequest
and removed
Feedback-PatchFix
Feedback-TestRequest
labels
|
This branch has conflicts that must be resolved |
|
Files changed: 575? That can't be right |
|
Happened in commit d4dd6e2 (the latest one) from what I can see. |
|
Yeah I tried updating from base branch, and got a load of merge errors that I can't fix, not sure what happened. |
Krudor
force-pushed
the
spell_triggered_aura_removal
branch
from
d4dd6e2
to
5cad011
Compare
|
Not showing up in history, but I've reverted the bad merges, let's see if I can do it right this time |
…to spell_triggered_aura_removal # Conflicts: # src/server/scripts/Spells/spell_item.cpp
|
Alright, seems to have solved it. |
|
Looks a lot better, it should be good to test for other users now. 👍 |
Changes proposed:
Target branch(es): 6x
Tests performed: Build and in-game tested
Description:
Currently, auras triggered by SPELL_AURA_PERIODIC_TRIGGER_SPELL are improperly being removed upon the parent aura's removal.
Auras that do obey this behavior will now properly have it handled by Unit::RemoveAurasWithInterruptFlags when called upon, see Unit.h AURA_INTERRUPT_FLAG_...
If anyone knows if this applies to 3.3.5 branch as well, leave a comment and I'll fix the target branches.