BUG: Overlays are not removed properly when only using "return", + Wiki/Docs: How to clear Overlay.
Arcitec opened this issue ยท 20 comments
https://github.com/WeakAuras/WeakAuras2/wiki/Dynamic-Information#relative
The documentation has two issues:
-
It says "Three values are expected", but then it shows two values being used (
return "forward", 100000
). The docs should note that the third value (offset) is optional. -
Both this and the "absolute overlay" say nothing about how to HIDE the overlay. I am guessing that my function should
return "forward", 0
on every call? This works, because it tells WeakAuras to calculate an offset that's at the same spot, so you can't see the overlay. But is there a better way to remove the overlay? And is it "harmful/wasteful" that I return"forward", 0
on every function call?
///////
Edit: Direct link to a demo aura to show the return
bug: #1803 (comment)
This isn't the right forum to ask questions, this is for tracking bugs or feature requests to the addon.
As to your question, returning nothing works, as does returning a essential empty overlay.
@InfusOnWoW Thanks for replying so quickly! :-) It's not just a question. I said that https://github.com/WeakAuras/WeakAuras2/wiki/Dynamic-Information#relative documentation needs fixing as mentioned in the first post.
returning nothing works
I tried that but it doesn't turn off the overlay that I had created by previously (in other calls) returning "forward", 20
. I tried return
with no data when the overlay should be removed, but it stayed on screen. And I used print()
statements to test to make sure the empty return was the last thing that was given, and the overlay still stayed...
as does returning a essential empty overlay.
What does that mean? You mean return "forward", 0
?
@InfusOnWoW Just tried the "returning nothing" method again, and yeah it does not work.
function()
local e = aura_env
local c = e.config
if c.immoPred and e.immototal > 0 then
print('go value')
return "forward", e.immototal
else
print('go empty')
return -- fails
--return "forward", 0 -- works
end
end
The screen prints "go value" and I see the overlay... and then finally it prints "go empty" but never turns off the overlay. If I comment out --return
and enable return "forward", 0
instead, then the overlay vanishes.
@InfusOnWoW I can make a separate ticket for the "return nothing" bug, and keep this one just for the documentation issue, if you want?
Actually assuming we're using the same DH power bar aura here, returning nothing does in fact wipe the overlay. Tested on 2.15.5 just then.
@1ps3 I'm on WeakAuras 2.15.4, let's see if .5 fixes it.
@1ps3 No, it doesn't. And no we're not talking about the same aura. I'm gonna try making and exporting a small bar which is just meant for testing this bug.
I'm currently running this and it's working as expected. That is to not display the overlay when e.immototal is less than 40.
function()
local e = aura_env
if e.immototal > 40 then
if (e.prep - GetTime()) *8 > 0 then
return e.power + ((e.prep - GetTime()) *8), min(e.immototal + e.power + ((e.prep - GetTime()) *8), e.powerMax)
else
return "forward", e.immototal
end
else
return
end
end
@1ps3 Yes but that's due to how that aura is written. Your aura doesn't trigger the bug in WeakAuras.
@InfusOnWoW Here is an aura to reproduce the bug, in the latest WeakAuras.
!DBvupnooq4Fl3kDs7kDfLwaP9EypjOuo6j2uU6WcCAvACtCA8rQDLTtlWd53(nJTttO0LfDR09WjkwoJh)nJNz8344(XrXKLXKm8hmvVIQU)gEMPioSFG)7j55AM524qqWdUpSZFSZCQiTqQUsYfMyYWrHrJM639uPHA4sHvTNj5ZYmwm5KRJMet4PGgNgtmk(IfmLgCMqRZ5fGtR0Sz05APAUDNG2vcoyUvL0hbnizS5v55rpUcq9IrxE15xFjSF7NPvAJeoNc6sgITPr0m36AWJQ0otybfaxxnNTMjmxPy58hIjKRgDjaOvgyagTeIsEyMSMPaVOFmjVsKIhU3)H6)OoX9xPmLwwNWQt(uDcTsrNXeRBxUDg7a)X90QfwLZPLAyB96vNqyM6etbxdJs)k)sDIubFRQyoPAgoPagMxT4G9zcEEDIqAS2IVK5baMboCsVwX)gy9(wSeDGP1pTk9PTB9fQSsbLcV3P4h2hc6c5MzsxCZIK3REM82iJi7BDA2fQ97YkMPsjQtExUuTHQYEhe76h0XayG(BUlmbCbpddxoZCWoUgm0ulCgKGTf87TwObXaR9XrB2ZbqLWxDrRmY2Q2xdNT7gwc(dUfS18q1yAbl9Ea3vzudBNI1b)GfR)OXEy44VBcWzKU3k2Vj2xc7gQW4VTu0n3HxoGsTn8Ys8EeV8N2t95E9Mx7afShq6w02wMOxXklhN55HAOzia3fsZmB4jKOzKOtMAxUs0HbSPGWMPn4Yui)TMf5uXslg2diUnog1zPYsPYXLI)gyhp0oEeoI6jmmLGw(fG11stp4iWPyL57qMpNQg(c0cSOf0IwgtdmPkSAB(ZAFCNJ)VA(u2cWmDi4TeWyHgyHzW)ama04kShmZ0f0m5MBBA10Vzdas46ETCt)5v(pFdhBRE5s8eEUI)uDYFwrZa)MwNe9CB39uhS)tDhTVRXt71VrEBS01RZkDnxZNxA7IT1vCTUcLcMx4Z6QoomC00zxo68OM2T31PbSv))gQp45p2M0uyRwwKn6eojCuZoVD3DIMNWFckF6patah(Vlbi()Fcy64F)I)dYaG6fm(IcmKFCmPus9pqJLAFZwvPHBjqWNYus16xivJGTJqJ)o5PLatK)9xNDM9DpNKH(olJL9z6d4L3nUNboiiy315cCDL9ICu3RV4HJV01(jY(QkLbN00tQ51wmGBc4)8vscmEdE2skx821oNl46I3S(EUshZZwpRlowUqUZFLZ8uTZDB1sMQ5y8Qj)JDOHAhoqOd9LoJTeG20061gKVWN)o0NIRuW2pTK)0tqldlcqDWKvnofyMNKGj3(ozmiBKPRBiM)yWGGJBWMNLXadfo6lyvg4E58f(dKcEKlbDfQ96QRE7CuQlzrgoD0OqyHYvfovQWtXh)1XKhUFmFY53Grc0STK4d)(xOLko0gZhaUyY0X)1KWOtU06Cz8oNslGrnbKXy)NCAk7RdPWDhXItPQVE94ETF1JyJgqhO07xOKvISxNri4GJ9L6XKbh0)y4tYAq3JgG(z8)m
This will import an aura with two overlays that constantly blink on/off every second.
Go into Trigger 1: Overlay 1, and edit its code. Set e.triggerBug = true
. The overlays will now fail to vanish. What does triggerbug do? Simple:
- TriggerBug False: 1st Overlay uses
return
, 2nd Overlay usesreturn "forward", 0
to turn themselves off. - TriggerBug True: 1st Overlay uses
return
, 2nd Overlay usesreturn
to turn themselves off. This fails.
PS: It seems like the return
bug only happens when an aura has more than 1 Overlay.
This is strange to be honest. I don't know what's wrong with the aura you linked but for some reason it doesn't want to hide the overlay, but using the same code in another aura that I recently created makes it work perfectly fine. Tested with 1 overlay and 2 overlays it's all the same.
@1ps3 Not sure, I created this aura from a blank default Bar template now, but I've seen this bug in multiple other auras, so it's something in WeakAuras itself. It simply doesn't hide overlays when you always use return
for hiding instead of the more explicit return "forward", 0
.
Ah okay I think I see the problem now. It seems like the overlay isn't updated until the bar is updated. So if you use a static number as your progress the overlay never gets removed.
@1ps3 Ah, yeah that's a good theory. Although it doesn't explain why return "forward", 0
works (only return
fails). Despite us never changing Duration Info.
Either way, perhaps WeakAuras needs some code to say: "Redraw Bar if Duration Info changed, or if [Any Overlay has changed since last time]"
Many auras can have static bar numbers (like power/energy bars, where the power stays static), which would break due to this bug...
PS: I'm not in WoW now, taking a break, but if you want to check your theory, make the Duration Info function return a random number between 0-100 as its first return value, to make it always update the Duration Info.
@emptyrivers Ah okay, well if someone can suggest the best ways to turn off overlays, I could edit the wiki. The 2nd question of the 1st post above asked about the best way, and hasn't been answered: Is return
the best (after bugfix)? Is return "forward", 0
equal or worse performance wise?
Ah okay I think I see the problem now. It seems like the overlay isn't updated until the bar is updated. So if you use a static number as your progress the overlay never gets removed.
Turns out your theory had something to do with it. Import my aura and edit the Trigger 1 Duration Info script to this, and the overlays will seemingly work even with triggerBug = true
:
function()
return random(0, 100), 100, true
end
I also tried deleting Overlay 2, to just try a single overlay (Overlay 1 is hardcoded to always use return
), and it didn't affect things (single vs multi overlay does not matter for this bug), and I can now summarize:
- If the
Duration Info
doesn't update:return
fails,return "forward", 0
works. - If the
Duration Info
updates (current/max changes to new values),return
andreturn "forward", 0
both work.
@InfusOnWoW Ah, yeah I can see what WeakAuras overlay code does now via the code in your patch...
InfusOnWoW@cc89b7f#diff-bd574ccbd8e51c13ccfac8ccc83bcb9fL329-R362
If there's no return value, it instantly sets all properties to nil
. But if there is a return value it just does some very minimal checks and variable assignment, so there really shouldn't be any measurable difference, as you say.
As for your bugfix: I haven't tested it, but reading it seems to indicate it's not a fix. Look at the code link I gave above, to see the whole highlighted section. Here's what the code does:
- If the function returns nothing: The
if (not ok) then
code runs, which just sets all of the overlay's properties tonil
. This is the problem! When usingreturn
, the overlay is not flagged aschanged = true
, so it doesn't update the bar. - Then there are some elseif/else checks which look for modified properties in the return value compared to what was already know previously... those flag the changed=true state if needed.
- You added some code in the else-check, meaning when the Overlay callback DID return a value, which says
if additionalProgress.direction then changed = true end
, which isn't executed at all whenreturn
is used. And also doesn't seem to be a good code change at all (why check for a non-nil direction value and then always flag it as changed even when it didn't change?).
I think I can see the actual bug though, by reading the code. It's here:
local ok, a, b, c = xpcall(overlayFunc, geterrorhandler(), event.trigger, state);
if (not ok) then
additionalProgress.min = nil;
additionalProgress.max = nil;
additionalProgress.direction = nil;
additionalProgress.width = nil;
additionalProgress.offset = nil;
The problem is right there... When the Overlay callback returns nothing, we just instantly set the properties to nil without flagging changed = true
. Of course, always flagging changed would be wrong too (inefficient). The correct code would be something like this:
local ok, a, b, c = xpcall(overlayFunc, geterrorhandler(), event.trigger, state);
if (not ok) then
if (additionalProgress.min ~= nil or additionalProgress.max ~= nil or
additionalProgress.direction ~= nil or additionalProgress.width ~= nil or
additionalProgress.offset ~= nil) then
changed = true;
end
additionalProgress.min = nil;
additionalProgress.max = nil;
additionalProgress.direction = nil;
additionalProgress.width = nil;
additionalProgress.offset = nil;
That should fix this bug / ticket.
As for both of the elseif/else blocks (whenever the callback did return something)... those blocks could probably benefit too by introducing the same kind of "if not nil then changed = true" checks above all of the spots where they hard-code certain variables to nil. That way they'll detect changes in those values too.