search

SFX-WoW / Masque

A button-skinning engine for World of Warcraft.
Other
44 stars 11 forks source link

Fix Reskinning of buttons that were skinned, but are in a disabled group #298

Closed InfusOnWoW closed 1 year ago

InfusOnWoW commented 1 year ago

In WeakAuras we heavly reuse buttons even between groups. In the issue a button that was previously in a enabled group is now moved to a disabled group. In that case there's a skin on it (that is it has Regions), so ReSkin should apply the default skin again.

Fixes WeakAuras Issue: https://github.com/WeakAuras/WeakAuras2/issues/4168

InfusOnWoW commented 1 year ago

@StormFX I don't know whether that's the correct fix, since I don't really have a good understanding of the Masque internals. But I think Reskin should for buttons that were at one point skinned, do the same as RemoveButton would do. That is call SkinButton(Button, Regions, false).

I suspect that AddButton should actually have a similar fix. If the group is disabled, but the button was skinned (that is has regions), then it should be skinned via SkinButton(Button, Regions, false).

But I didn't include that in this PR as I wasn't sure whether my fix is even correct.

StormFX commented 1 year ago

ReSkin is meant to be used to reapply settings to a button that was modified outside of Masque, so should always exit if the group is disabled. I do agree, however, that Masque should account for buttons that were previously in another group in the event that the new group is disabled. The problem is that if a new button is added to a disabled group, it, too, will be "reskinned" with the default skin, which should not happen (because not all buttons have the DF art style by default). I'll have to look into it further.

StormFX commented 1 year ago

FWIW, I'm already working on some minor API changes so I'll add this to my list. I've moved this to #299.

InfusOnWoW commented 1 year ago

Feel free to ping me if you have something to test or need more information.

StormFX commented 1 year ago

This should be fixed in c848221 so closing. If there's any problems with it, you can let me know in #299. Thanks. :)

InfusOnWoW commented 1 year ago

Thanks, I'll forward that message.

StormFX commented 1 year ago

I'll be dropping a beta soon, so someone may want to test that specifically.

EJeyp commented 1 year ago

This should be fixed in c848221 so closing. If there's any problems with it, you can let me know in #299. Thanks. :)

Hey, I have been running into an issue with my WA where some icons sometimes appeared to get rescaled randomly. After quite of bit of searching I figured out it was linked to Masque and ended up here. Now the issue seems to match this bug, as it only happens when I have masque enabled but disabled for the icons of my WA.

Trouble is, I am currently on version 10.0.5 of Masque, which I think should already have the fix you mention above? Here's an example of it happening, all 3 lines of DoTs should look similar, but on the last one the icon for the Curse of Agony seems to be half its height and much wider than it should. image

This is on the WotLK Classic version of WoW.

StormFX commented 1 year ago

I've moved the conversation to #308, as to keep it separate and to avoid notification spam for others who may be subscribed to this pull request.