Feature Request: Delay Skinning Until All Add-Ons are Loaded
Adirelle opened this issue ยท 8 comments
I noticed I have some issues with Masque not applying the skin properly at login. After some tests, I found it was related to custom skins and the order in which addon are loaded.
I have made a reproducible test case, using an addon written for this (whose files are there: https://gist.github.com/Adirelle/9b6f2078996305045824e74db8927953).
To do the test, I enabled only these addons :
- !MasqueTest
- Masque
- Masque_Caith
Here is the output (sorry for the guild MOTD):
Please notice that:
- !MasqueTest is loaded before Masque_Caith, and Caith skins are not available at that time.
- when the callback is called with "Caith" as skinID, the said skin is not available ("my skin: nil"). It means !MasqueTest is not skinned with its configured skin.
This happens because addons without dependency relationships are loaded in alphabetical order on Windows. Please note, that even if they were loaded in random order, this bug could still happen.
Would it be possible to reskin the groups when custom skins are added ?
This is because you're creating the group as soon as the file is processed. Try moving the group creation to an ADDON_LOADED or PLAYER_LOGIN event function.
My memory isn't what it used to be so I couldn't remember which event to use. The ADDON_LOADED
event fires when the calling add-on is loaded, including its saved variables. The correct event is PLAYER_LOGIN
, which fires after all UI elements (including add-ons) have loaded, just before the PLAYER_ENTERING_WORLD
event.
The reason for this is specifically due to the issue you're having, in that some add-ons would theoretically be loaded before their dependencies and it's not always feasible to list every potential dependency (skins, in this case) in the ToC file.
I currently test on both Bartender4 and Dominos, both of which load before any Masque skins (Masque itself is listed as an optional dependency). Skins load just fine on them.
Masque knows when the skins are loaded and which groups use them, it could easily reskin them to be ensure all skins are properly applied to all group, whatever the loading/event order is.
I don't want to sound rude, but forcing a re-skin of every add-on when each skin is loaded is a lot of redundant calls that don't need to be made when the problem can be addressed by using the proper event. Masque doesn't even load its own options until the PLAYER_LOGIN
event for that very reason.
Done (see updated gist: https://gist.github.com/Adirelle/9b6f2078996305045824e74db8927953/0b34edbbb7ad0c299ff1a49816b1b84d22c3ad11).
New output:
As I've already explained, the issue you're having can be solved simply by registering the PLAYER_LOGIN
event and calling Masque in a relevant function. If there's a reason that you can't use this event, I'd be happy to hear it. Outside of that, there's no reason to call Masque prior to that event, especially directly in the code as its processed.
Not every addons, just the groups that use the newly-registered skin, which was not loaded before anyway. See 3017607.
While the proper method is as I mentioned above, the confusion around it lies solely on me for failing to get the documentation up to date. Additionally, I can't objectively say that there won't be a case when using the recommended event isn't feasible. Nor is it fair to ask someone to provide me with an example of such a case.
That said, having a fail-safe isn't a bad idea so in a future build, Masque will delay skinning any group that's created prior to its current skin being loaded until the PLAYER_LOGIN
event. Note that while this will ensure the groups are skinned properly, examples like your test will still yield similar results until that event.