Bug: AddButton not Applying the Correct Skin
InfusOnWoW opened this issue ยท 10 comments
WeakAuras is not only dynamically creating groups based on the installed auras, but also is reusing buttons. That is we have a pool of buttons that gets reused between different groups. I recently fixed that we never informed MSQ that a button moved to a new group.
But due to user reports of that leading to bugs, we need to revert that:
WeakAuras/WeakAuras2#1131
The user has configured masque to be globally disabled.
If I read the code correctly, what is happening is that on the RemoveButton (which was entirely superflous since AddButton does that already), the Button is skinned with a "Blizzard" skin.
This is typically not a problem since that skin would later be overriden by the correct skin.
Except if the new group is disabled.
This can be reproduced by with the above PR reverted, and with these two auras, which show the buffs on the player's target/focus. Applying buffs with/without target/focus should show the issues pretty much immediately.
!nBvuZjUnq4Fn9Xmd2hbsEmHyAOtodf5KR9fLtylB0eBjFwYqipC)27URSbooAA60x6mmeLvR2D1333UcEapHZSC2DYnCwfNLHFatfnMTG15FjMZEDEEUv64tVy84HdUC0WbdgpiC0OXxZZVy4LC2UEhUCu41xgmACWWbxf(PGRW9VIZe601MMfgL2XztIItIwcjTwKk5XHC2kr6lq(A1qYJnAjCGuNYOTuT5enoybCYCLwzx7xdl9wH1UgvrHSbDpMUpDgWLzYvT55j7QHOEF0dlM(4dW(0)kABeHyDillNLz9X12UsUrQDm4uQx5SNNCdl5zwYnlHG1IjLvxk2bbFVRlAKKRSfrpabNSbjtkkDR5mTOsA7R0w9HsZ5n1iYuTwahgmacPSmVdMsMVakXsvH(aK1yCcey4XGVLgbXuTw5ZPLcRLVIUmPOXQ2sNQpR(Dp1Qv9M8uJoKRuvcNKcwQOuIqkBLPjdVYE2PrwafHhtZ2bxqvksF1WjmaxX0KxYxRf6mzg)wOcaY0j0UPaXAG4S8M7M9iRpUDQN4GraVcAGPav3aWgWJcNGkaVcAkA1Nx2KLrrXK8GWterGSKRk884bn1mnfCy)wm4jB(ZR)TXlE6Q6)avrozJwu(eOEiCniGctMQx)HisRdY986JnrGeEIJuOSmqqHgF2RVSsms2E5gHkWzRek9h379A(pM)E(EFJqrj0fdmPt(QlyQbK1iPhheEONfbMZ2FMAmLzMTA2wfKaKeBTotvceQhRZqjcRT7V9UgLvqcN)DTVGagRZjMsqA0xSrAXQsV45fPS(guy7wIaaLG))I)WehRWH(sqXHBgXj4Nq67prFpe)21DN7iGBNNKm)ZlN9R3N43iezUPLIciBZFm5HzXrC2APQynqFJgEwfA)0H)7deoBpYrdQovVGcKJyp6cmbUaaSxrtgNfZMDxKhA8nZRADoJE(gzdmATdm(aGw6hXLdJe6QK)XZC84nfeGdpaI9kBvzWyDc23Zn(UQqKjkRx7NyTVL7uIJ2GZ(f7rbaM51OE77F93Bfz4mVV)1KKFEi4BgtLVFT1z2JTqOQ9ZnZnql6rz(8rLoupbT6DgTsoEcjtJqd)wRO6llUDR4juEa0ML06V37W21qFqj0KyPc)hEa(9EY9Kxt)BEZ9hFd)WtUDVy3bmN73cCMhLPXxBKjEJF2KbC7fbdOEbuoC7p)eH1PsFz3DDti2pe7CYEkdBaaAyiQ24)1d
!nBvxVjoou0Fn7JvIKY225rGgMbjMaloDhP9bZ4K4eSQJDuSdu6dZV99ETtGSSSDz1(YiHOP3C)YNZ5En0aAcLyOKN57PKkkjh)aMkB0haRR(wmL82QIcd3sNF3Jp9uq4J3)Rp(0Opn(HWhPf3n(bk5yNdXb3tjmv2oDZATqzPKzrXjrBGkuZY404qkjLL9kK8wfuPyTIdbKzfALX1iwwJfEaISqOeMD(NHh9wHNTnIYsEd6ESR57mGpMZtBlksowdz9lrlxp)LLW7D)lRTHfI9bxkxKB8510MY3ZvwceL4nkz7SjKKTKKjBGK1IfLulzhHKtuSkExuUqGcWzs7UZjzDd3LeY6OLlr)AvNBnRV5By5IwdGdJgbbYLfDWuYQ1qlkfLQZqwJ2YqGHgd(k1mhT0A4BZKmJHM6omzOXQwPv0Jp(3EPvJ4D(LgTixjQywUlzzmjhHusQUjhpYE2PHxcnHhtZpcWGidPVAicnWveLZl(B1mvopNof6aGmTmLDoqSAipBM88Ixi959Kub0ncqdanvrdaUapYSmxd4vqZrR(6sMTjkk2jpC4jIiqvkeLEg5SMAHYLC49TyYFAA(SnYpLoLeGQilVrXK)oOEC4AqGln5IE9hIiTwO2RQhAYbsyeduOKCqqHg361xgoMjtVCZHkqSvmH629(KM)283Z3NgekLWilWKw(B2G5AqwJKECq4Gb0r)dZNzATmxFqroiGcGKyRXQRsGu9sDokriTD)T31O8sNW5)24liGX(CMwcsJ(MnsXsLEXZRCE9euyB3GaGRa)8I)WghdZI(6GIZNmhNGFcDFFV77X432UZChbmDvsYQVUzXN)sI)fHiZnxYkHQT6LKLlIJOKDCr5oG(Ey8vvO9Bh()Vq4QZidwuDPEbfidyp3bygCaayVYTLCrmzXZrEOXpmN2ATA1Q98gy1AhyCdGw2T4Y5vcDDY)AmdxVjGeC(2oCw5GihwX7G9tCJFQkezcz9o)gRtJCxsCUxqj)IzqcOK5nI3)X3)TwwoUZ7hFp5klbFxRR8ZRTw9jSfsvTFVjmhuYTdk91tRlQEgk9d2T6C8cw2TdLuzN)hhMoE04jO(a4nJtS)rxeB2bdcsykX468)YnWF0DU30vRxCj(GBM9xz3Jmx7xdCLRLDlW2Zt8g)Qohy37cg5MgqbX0)(LegRi71Jp3TJ40ASRj8DvypGqJdr9g9p)
So the issue is that RemoveButton is bypassing the group's disabled state and applying the default skin when it shouldn't, right?
Edit: Since you seem to have a testing environment in place already, try the following:
Change this line in AddButton:
From:
Parent:RemoveButton(Button)
to:
ButtonData = ButtonData or Parent.Buttons[Button]
Parent.Buttons[Button] = nil
The reskinning of the button in RemoveButton when called in AddButton is redundant. This should address that. It also allows for only the button to be passed when changing groups, as it pulls the ButtonData from the old group prior to the reassignment. This is just a quick fix to see if it fixes the issue. As I said in the other ticket, I'm in the middle of a rewrite so a permanent solution may be awhile.
I've made a commit that should address this. I'll leave the ticket open until I get a release finished and it's live-tested.
Sorry for not responding, I was trying to create a small test case of what we do, but somehow it didn't behave like I expected.
I just tested your actual commit, and with that it seems fine! Thanks!
I'm rewriting a lot of it in my spare time, but I've got quite a bit of it finished. I'd say I'm about 70-75% done. Hopefully, within the next two weeks, if everything goes well.
Please do tell us before a release, and we can arrange a bit of testing on our discord.
I've released an alpha for testing purposes. I still have some improvements to make, but at least this will get some of the stuff I needed to do out of the way.
I just tested this alpha and it seemed to work nicely. Once that masque version is out, I intend to commit something like InfusOnWoW/WeakAuras2@85bfb2a to WeakAuras.
Note, that not every aura currently has a uniqueId, so we won't be using staticIds for all auras yet, but that's coming at some point.