BUG: Overlays are not removed properly when only using "return", + Wiki/Docs: How to clear Overlay.

[Arcitec]

https://github.com/WeakAuras/WeakAuras2/wiki/Dynamic-Information#relative

The documentation has two issues:

1. 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.

2. 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)

[InfusOnWoW]

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.

[Arcitec]

@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?

[Arcitec]

@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]

That's a bug then.

[Arcitec]

@InfusOnWoW I can make a separate ticket for the "return nothing" bug, and keep this one just for the documentation issue, if you want?

[1ps3]

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.

[Arcitec]

@1ps3 I'm on WeakAuras 2.15.4, let's see if .5 fixes it.

[Arcitec]

@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.

[1ps3]

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

[Arcitec]

@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 uses return "forward", 0 to turn themselves off.
• TriggerBug True: 1st Overlay uses return, 2nd Overlay uses return to turn themselves off. This fails.

PS: It seems like the return bug only happens when an aura has more than 1 Overlay.

[1ps3]

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.

[Arcitec]

@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.

[1ps3]

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.

[Arcitec]

@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]

FYI our wiki is public, anyone can edit it.

[Arcitec]

@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?

@1ps3

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 and return "forward", 0 both work.

[InfusOnWoW]

I don't think there should be any measurable difference between those.

[Arcitec]

@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 to nil. This is the problem! When using return, the overlay is not flagged as changed = 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 when return 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.

[Arcitec]

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.

[InfusOnWoW]

You didn't correctly understand the code. Please, test it, before running into a dead-end.