Masque & Dynamic Group Names
StormFX opened this issue ยท 29 comments
I unwittingly pushed some changes I was working on to address issues with dynamic group naming. I have everything working on my machine and still have some changes to make to other stuff, but I wanted to touch on how dynamic group naming works in Masque.
Previously, Masque would assign a unique ID to each group. This ID was a concatenation of Addon's name and the Group name. Since WeakAuras often changes its group names, this causes problems with Masque. To address that, a 4th string parameter, StaticID
, will be available to the :Group()
method. When present, it will replace the Group name as Masque's internal ID, allowing the Group name to be changed.
Also note that the Group name is what's displayed in Masque's UI, so it can be display-friendly. IE, it doesn't need to be "myaura_myspell_01", it can actually be the name the user assigns it, "My Group - My Aura", for example. As long as the StaticID
is unique to the group, saved by WeakAuras and passed in the Group()
method, the Group name can be anything.
Additionally, a new group method, SetName
, is available that allows the group to be renamed and forces Masque to update its UI. It has a single, string parameter for the new name. Keep in mind that only sub-groups can be renamed and only if a StaticID
is passed when they're created.
Examples:
-- The old way:
MSQ:Group("WeakAuras", "group01_aura01_01")
-- Masque ID: "WeakAuras_group01_aura01_01"
-- Display in Masque's UI: "group01_aura01_01"
-- No renaming possible.
-- The new way:
local myGroup = MSQ:Group("WeakAuras", "My Aura", nil, "group01_aura01_01")
-- Masque ID: "WeakAuras_group01_aura01_01"
-- Display in Masque's UI: "My Aura"
myGroup:SetName("New Aura")
-- Masque ID: "WeakAuras_group01_aura01_01"
-- Display in Masque's UI: "New Aura"
One more thing, I'm not sure where I was when my commits got pushed, so there could be errors if using it as it sits now. I generally do a commit after I've tested all the changes, but I usually don't push unless I'm ready for external testing. If you get any errors, let me know what they are so I can update the current pointers to a working copy.
So, we don't have static ids yet, because we are in the middle of a transition which will take a bit of time.
But what I can tell you based on that information, is that we would need to have:
- a method to check if a group with a StaticId exists.
- a way to upgrade existing groups.
Because the way we would implement that in a backwards compatible way is:
- First check if a Group with the right StaticId exists
- If not, check if there's a old group and upgrade the old settings to a StaticId
- Or last create a new group
Alright, seems I've got a working fix. When the :Update()
method is called on a new group or profile change, it will check to see if the group has a StaticID and whether it's been "upgraded". If it hasn't, it will create a new pointer (using the new ID) to the group's previous DB and then delete the old pointer.
Edit/Update: When WA gets its static id implementation, all it needs to do is pass the old group name and new StaticID
the first time. After that, Masque will know that that group is now using its StaticID
and whatever name the user has chosen for the Group name can be passed thereafter.
On WA end, to check whether this has been done, simply use the existence of the old group name. Note that this is just an example and is in no way intended to represent WA code:
-- Wherever the Masque group is first created...
-- Check for the old ID and process accordingly.
if aura.oldID then
aura.MSQGroup = MSQ:Group("WeakAuras", aura.oldID, nil, aura.StaticID)
aura.MSQGroup:SetName(aura.name)
aura.oldID = nil
else
aura.MSQGroup = MSQ:Group("WeakAuras", aura.name, nil, aura.StaticID)
end
Given that I don't know how static ids and upgrading will be implemented within WA, this is just a generic example. But it's probably more efficient for WA to check if the upgrade has happened, knowing that passing the static ID the first time will force Masque to do its own upgrade. Masque providing a function to check is basically pointless, given that WA will still have to have the old ID on hand anyhow:
-- Note that the Masque function is unnecessary.
local msqIsUpdated = MSQ:GetStaticID("WeakAuras", aura.oldID, nil, aura.StaticID)
if not msqIsUpdated then
aura.MSQGroup = MSQ:Group("WeakAuras", aura.oldID, nil, aura.StaticID)
aura.MSQGroup:SetName(aura.name)
aura.oldID = nil
else
aura.MSQGroup = MSQ:Group("WeakAuras", aura.name, nil, aura.StaticID)
end
Once you start updating WA, we can discuss the necessity of such a function or any other issues.
Honestly, WeakAuras is more complicated than Masque, so it stands to logic to have a method to upgrade the settings. For Masque, however, it's simpler to just create a new group and have the user choose their skin settings again. Since Masque uses AceDB-3.0 default values for groups, checking for a group, even one that doesn't actually exist yet, will always return a non-nil value.
Sure, the upgrade logic will be in WeakAuras.
We just need a bit of help with the process, so if AceDB-3.0 doesn't provide such a method, then we would try to find a way to workaround that.
We do have around a million user, while that certainly dropped a bit after the bfa release and not all are using masque, but asking them all to redo their settings is not good. We try very hard to do that as rarely as possible.
I just checked the Masque code and while I certainly didn't understand all the code. It seems indeed that we don't need any function to check if a group with the static id already exists. I think, we can implement upgrading easily with the api there is.
As I mentioned, Masque will do its own upgrade automatically. As long as WA knows it needs to upgrade a group, it only needs to pass the old Group
value once, along with the new StaticID
. Masque will do its thing and then WA can consider that group upgraded, as far as Masque is concerned.
I've tagged another alpha. Some important bits from the changelog:
- The
AddButton()
Group method now has a third,string
parameter,"Type"
, that will tell Masque the type of button being passed.- If not passed, Masque will attempt to determine the
"Type"
by checking for specific regions. If none are found, it will default to"Button"
. - Masque will use this value to determine which regions to search for if unavailable in the
Regions
(ButtonData
) table. - The following are valid values:
"Button"
- Supports basicCheckButton
regions plusIcon
."Action"
- Supports regions available inActionButtonTemplate
and its derivatives (PetAction
, etc)."Item"
- Supports regions available inItemButtonTemplate
and its derivatives (ContaineFrameItem
, etc).Border
is still available due to the skinning limitatons ofIconBorder
.
"Aura"
- Supports regions available inAuraButtonTemplate
plusBorder
.- Can be used for generic "Aura" buttons.
"Debuff"
- Same as"Aura"
, but a different default texture with no skin color support."Enchant"
- Same as"Aura"
, but a different default texture and supports skin colors.
- If not passed, Masque will attempt to determine the
- The
AddButton()
Group method now has a fourth,boolean
parameter,Strict
, that if set totrue
will cause Masque to skip locating missing regions.
If I understood that correctly, I think that doesn't affect us, since we don't use a different kind of template depending on what exactly is tracked.
All our icons use a very basic setup of just having "Icon" and "Cooldown" and that seems to still work.
An update to the previous post: "Button" has been replaced with "Legacy", which covers all regions that Masque covered previously, and is the default if it's unable to determine what type of button it is.
In regards to your comment:
If the "Type"
parameter is not specified, Masque will attempt to determine the type based on the object type and available regions of the passed button or frame. If it's unable to find the type, it will default to "Legacy"
, which causes Masque to iterate all available layers looking for missing regions.
Masque also uses the "Type"
value to determine which additional layers to attempt to skin when the skin is applied to the button. If set to the default of "Legacy"
, it will attempt to skin every layer available.
TL;DR: In spite of the fact that Masque always skins the Icon
and Cooldown
regions so this technically won't affect WA auras, passing the proper "Type"
will increase the speed at which those "buttons" are skinned, because it filters out the regions that aren't available for that button type. Additionally, setting Strict
to true
forces Masque to skip looking for missing regions altogether.
So, if WA only passes the Icon
and Cooldown
regions to Masque, call the method like this:
region.MSQGroup:AddButton(button, {Icon = icon, Cooldown = cooldown}, "Aura", true)
Which tells Masque to not search for missing layers and to only attempt to skin regions available to the "Aura" type.
Note that Cooldown
currently isn't listed in the "Aura"
type, which is an oversight on my part and will be address in the next alpha.
I've release a beta for 8.2 that contains the relevant updates. If you all need to do some internal testing, etc, it would be a good idea to get that underway before a release of Masque is issued.
@StormFX sorry for taking a month to actually try that beta. I was quite busy to even try to keep up with the incoming bug reports.
I've tested the beta for a bit and as far as I can tell:
- Nothing with my styling broke
- The new api works as I expect it to. Once that version is released we would merge #1553, which makes use of the static id.
So, if you give us a heads-up before you release it, that would be great.
Looks like there's going to be another beta. I improved the mask-handling so that they're per-button rather than per-region. This allows for the same mask to be reused. It's also useful for color-only texture objects, as coloring an actual texture can sometimes cause the color to be off a little.
In regards to your WIP, if the only regions being passed to and skinned by Masque are Icon
and Cooldown
, you should pass the Type
and Strict
parameters to speed things up.
region.MSQGroup:AddButton(button, {Icon = icon, Cooldown = cooldown}, "Aura", true);
Not doing so means that Masque has to figure out what type of button it is and then search for every missing layer. I thought about adding a method to allow authors to specify their own type (and the associated regions) to speed things up even more, but I'm not sure anyone would use it. To demonstrate how that would work, it would go something like:
MSQ:AddType("WA_Aura", {"Icon", "Cooldown"})
-- Then, when adding buttons:
region.MSQGroup:AddButton(button, {Icon = icon, Cooldown = cooldown}, "WA_Aura");
Same thing. Steps to reproduce:
- New > Dynamic Group
- Grow Direction > Right
- New > Icon
- Size 64 x 64
- Untick Automatic > Choose Icon (search "HP") > Select "Heart".
- Display Text > "Icon 1"
- Trigger > Status > Health > 0
- Duplicate Icon 1
- Display Text > "Icon 2"
- Offset X > -48
- Reload UI.
I received a bug report recently that Masque is not playing well with our texts on icons, might be something for you to investigate as well? I sadly closed the Discord DM but the case was a dynamic group with 2 overlapping icons that both had a text and the text from the lower icon was bleeding through into the upper one.
I just did a quick test and this occurs without Masque even being loaded. And as far as I'm aware, WA only passes the Icon and Cooldown, so Masque doesn't touch the texts.
I made a video demonstrating it. It's ~93mb. If it doesn't play, you'll have to download it. Effing Google Drive.
https://drive.google.com/file/d/1G8uJPO1hEOr7fAHlCaS_N2P5Jeo5yuHE/view
Here's some updated code that will allow you to implemented this prior to Masque's release:
~ Line 4:
local MSQ, MSQ_Version = LibStub("Masque", true);
if MSQ and MSQ_Version > 80100 then
MSQ:AddType("WA_Aura", {"Icon", "Cooldown"})
end
~Line 368:
if MSQ then
local masqueId = data.id:lower():gsub(" ", "_");
if region.masqueId ~= masqueId then
region.masqueId = masqueId
local staticId, msqType
if MSQ_Version > 80100 then
staticId = data.uid
msqType = "WA_Aura"
end
region.MSQGroup = MSQ:Group("WeakAuras", region.masqueId, staticId);
if staticId then
region.MSQGroup:SetName(data.id)
end
region.MSQGroup:AddButton(button, {Icon = icon, Cooldown = cooldown}, msqType, true);
button.data = data
end
end
Another note: You probably don't need to create a button object. A simple frame would suffice as Masque supports frame objects and will create the Normal
texture if it doesn't exist.
The (hopefully) final beta of Masque is up. It includes the custom type API I mentioned earlier and reflects the code I posted in the comment above this one. Assuming all goes well, I'll push a release after the weekend, so it might not be a bad idea to get it tested. ;) @InfusOnWoW
Note in the code above, the version conditional is > 80100
. This is due to the current beta and release being bumped to 80200
to ensure maximum compatibility.
I did something that is mostly what you suggested, although I didn't add the backwards compatibility for old Masque versions.