WeakAuras

WeakAuras

206M Downloads

Masque & Dynamic Group Names

StormFX opened this issue ยท 29 comments

commented

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.

commented

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
commented

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.

commented

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.

commented

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.

commented

I'll look into it and see if I can find some way to work around it.

commented

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.

commented

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.

commented

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 basic CheckButton regions plus Icon.
      • "Action" - Supports regions available in ActionButtonTemplate and its derivatives (PetAction, etc).
      • "Item" - Supports regions available in ItemButtonTemplate and its derivatives (ContaineFrameItem, etc).
        • Border is still available due to the skinning limitatons of IconBorder.
      • "Aura" - Supports regions available in AuraButtonTemplate plus Border.
        • 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.
  • The AddButton() Group method now has a fourth, boolean parameter, Strict, that if set to true will cause Masque to skip locating missing regions.
commented

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.

commented

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.

commented

Right, we can do that.

commented

Any progress on this?

commented

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.

commented

@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.

commented

I'll try to to remember to let ya know. I'll probably push one more beta first.

commented

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");
commented

WoWScrnShot_090519_110051

I just created a couple of static icons with static text ("Icon 1" and "Icon 2"), Doing /fstack shows that they're on the same frame level.

commented

Odd, I can't repro at all. Can you try with an alpha version of WA?

commented

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.
commented

image
๐Ÿค”

commented

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.

commented

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.

commented

That's odd, I tested with Masque being loaded but disabled and it was fine.

commented

i couldn't reproduce the issue either

commented

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

commented

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.

commented

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.

commented

I did something that is mostly what you suggested, although I didn't add the backwards compatibility for old Masque versions.

commented

Looks good. Thanks. :)