RichSteini / WeakAuras2-TBC-2.4.3

GNU General Public License v2.0
9 stars 9 forks source link

Closing options doesn't hide Aura's with a triggger to show when missing #6

Open tomtko opened 1 year ago

tomtko commented 1 year ago

Create an aura with the trigger stating to only show when buff is missing, apply buff to hide aura, open and close weakaura options, aura doesn't automatically hide.

Also doesn't seem to work at all with Spell ID's? Seems to only work with name.

Testing with Elixir of major agility

Spell ID: 28497 Name: Major Agility https://www.wowhead.com/tbc/spell=28497/major-agility

Edit: Weirdly, it works with Scroll of Strength V (Spell ID: 33082), doesn't work with Scroll of Agility V (Spell ID: 33077)

RichSteini commented 1 year ago

You are right, i am at them moment parsing the spellId from the spelllink and apparently this does not work for some spells. I have implemented now a combination of spellId cache and spellink if the latter is not available. The spellId cache only holds one spellId (the highest spellId) so if there exist multiple for one spell it could still be "wrong" or rather not as expected, but probably can't do anything about that. Not closing this yet, it would be nice if you can give me feedback if that resolves all issues. Improvement in Improve spellId of UnitAura with spellId cache.

tomtko commented 1 year ago

Spell ID's seem to work now, for the things I listed above at least, however i think for something like Haste Potion (spell id 28507) which shares the same spell name for the buff as a lot of things like the abacus trinket, it doesn't seem to work?

Also, the issue of opening and closing weakaura options doesn't automatically hide auras that should be hidden but that's not the biggest issue.

RichSteini commented 1 year ago

Yes, the first part is out of my control as i tried to explain already. The API does not have a function that gives you a spellId for an aura (you only get the spellname and rank from UnitBuff()) and with only the spellname you cannot distinguish between those two (i am just taking the highest spellId for the given spellname). So this functionality is limited by the 2.4.3 client.

For the second part, the auras that are hidden when typing /wa depend on what you have selected last time in the dialog. If you hide them with the eye icon then they are hidden next time and that seems to work for me consistently. What behaviour do you expect other than that?

tomtko commented 1 year ago

For the first, perhaps some sort of limiting based to the first spell id found? Auto-Cloning the aura basically is a good fix for it though, so probably nothing really needed to do.

For the second point, I mean something like, if an aura in it's trigger has it set to show only when missing (e.g, a consume checker, check if consume x is applied, hide if applied, reopening the options doesn't auto hide this aura despite it being applied) , or only when found, closing and opening the options doesn't respect this option and just always shows the aura, not really a big deal anyways though

XiconQoo commented 1 year ago

Yes, the first part is out of my control as i tried to explain already. The API does not have a function that gives you a spellId for an aura (you only get the spellname and rank from UnitBuff()) and with only the spellname you cannot distinguish between those two (i am just taking the highest spellId for the given spellname). So this functionality is limited by the 2.4.3 client.

For the second part, the auras that are hidden when typing /wa depend on what you have selected last time in the dialog. If you hide them with the eye icon then they are hidden next time and that seems to work for me consistently. What behaviour do you expect other than that?

Hey, wanting to give some inspiration. Cool backport!

You could just extend UnitAura to return at the 9th place or sth that is not expected by UnitBuff API yet and return a cached spellID list of possible spellIDs for a name. You would just need to cache[spellID] = name as well, to aggregate the list (and extend it with a icon[spellID] list). Maybe that would also help the spellName insert box, where you could pop up a tool tip with possible spells for a name.

RichSteini commented 1 year ago

Hey, wanting to give some inspiration. Cool backport! You could just extend UnitAura to return at the 9th place or sth that is not expected by UnitBuff API yet and return a cached > spellID list of possible spellIDs for a name. You would just need to cache[spellID] = name as well, to aggregate the list (and extend it with a icon[spellID] list). Maybe that would also help the spellName insert box, where you could pop up a tool tip with possible spells for a name

Yes that is exactly how i am doing it right now, but in addition if i get a rank for a spell then i am getting the spelllink and parse the spellId from there, that way i can always guarentee a correct spellId given a rank. And in the spellId cache lies the problem (if i don't get a rank), because for a given spellname there could for example exist 5 spellIds. Suppose you have 2 buffs with this spellname on the unit, which one do your return?

For the first, perhaps some sort of limiting based to the first spell id found? Auto-Cloning the aura basically is a good fix for it though, so probably nothing really needed to do.

Not a bad idea, but i would have to check all auras of the unit for each UnitAura call. The thing is as an example if there exist 5 spellIds for a given spellname then you still only get a 2 out of 5 which you cannot really match, and how would i choose the 2 spellids, at random? Would that make it better?

For the second point, I mean something like, if an aura in it's trigger has it set to show only when missing (e.g, a consume checker, check if consume x is applied, hide if applied, reopening the options doesn't auto hide this aura despite it being applied) , or only when found, closing and opening the options doesn't respect this option and just always shows the aura, not really a big deal anyways though

I need to check how the behaviour for that is on classic/retail, but not a big issue if it is incorrect.