Masque

Masque

7M Downloads

Feature Request: Delay Skinning Until All Add-Ons are Loaded

Adirelle opened this issue ยท 8 comments

commented

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):
image

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 ?

commented

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.

commented

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.

commented

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.

commented

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.

commented

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.

commented

Not every addons, just the groups that use the newly-registered skin, which was not loaded before anyway. See 3017607.

commented

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.