Add new angle-based constant factor for circular dynamic group grow styles

[wtodom]

Description

This PR adds a new angle-based constant factor for circular dynamic group grow styles. It is similar to the existing circular growers Clockwise and Counter Clockwise, but with a few key differences. With this new constant factor:

• Spacing is constant, and is determined by a combination of the user-defined radius and angle.
• Auras fill the group sequentially from one end to the other, essentially being placed as points along that curve in order.

The motivation for this new style was a request from a friend - they were trying to use the existing circular grow styles to build a new UI that was based on circles using Masque and some custom textures, but they couldn't get some of their aura groups to fit and grow the way they wanted. My initial solution for them was to write a custom grow function that placed the auras manually, and I posted in the discord to ask about my use of an "internal" variable that felt too hacky. @mrbuds responded after a little while (a few messages down from the linked one), and after some discussion I decided that it may be worth actually adding something similar to my custom grower in a more official manner.

Type of change

• New feature (non-breaking change which adds functionality)

How Has This Been Tested

I have done a moderate amount of manual testing in-game. I've tested variations on almost all of the options that are enabled for this new constant factor with the notable exception of Group by Frame, since I haven't ever use that myself in any grow style. I chose to leave it enabled/visible since it's also an option for the existing circular growers and this new one is quite similar to those.

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings

I did not add any significant comments. The PR only adds a small amount of code to the existing circular growers, and given the nature and context of the changes and the additions should be self-explanatory.

I also didn't update any documentation since I'm not aware of any that specifically describes the individual grow styles, but I'm happy to do so if that actually exists and someone can point me in the right direction to find it.

Potential points of discussion

I tried to make design choices that seemed appropriate and intuitive, but I do have a few questions about my choices that I can see as potential points of discussion -

1. Constant Factor Naming: This initial submission calls the new constant factor "Angle and Radius", but the two existing factors are only one word each and one is already called "Radius". I'm not attached to this name, it just seemed descriptive.
2. Region Option Naming: I added one new region option, currently called "Angle Between Auras". This option allows the user to set the angle between auras in the group, essentially exposing the internal dAngle to the user for this new constant factor. Additionally, internally this option is referred to as stepAngle. This makes sense to me, and while it is hypothetically confusing to have sAngle and stepAngle I think the best resolution would be refactoring sAngle to startAngle or similar. That said, I'm completely open to a new name for this variable.
3. stepAngle default value and range: I chose 15 for the default value, which seems reasonable for my expected common use cases, but this could be biased. Additionally, I limited the slider to a max of 180 since small changes to the stepAngle can have a large impact on the overall group. I can even see a smaller value being appropriate given how I expect the setting to be used, but it didn't seem worth limiting it unnecessarily.
4. Other addon versions: So far I've only added this to the retail addon, but if/when we get the details sorted out and it's approved I'm happy to look into adding it to the other versions as well. I only did one version in this PR since it's my first and I wanted to reduce the scope of changes for my first attempt.

I'm very open to any and all feedback on this new grow style. I personally think it is something that adds to WeakAuras overall and that people would use, but I certainly don't think I have a complete perspective on every player's wants and needs, or on exactly how things in the codebase should be implemented.

Examples

Finally, here are some quick pictures of a few really basic examples that I used while coding and testing. They aren't meant to be an exhaustive list of the potential uses at all, but simply demonstrate visually what the new grower does.

First, of course, a picture of the group settings with the new constant factor selected.

My friends circle-based UI, that started all this.

DK runes

One simple Curve-style dynamic group tracking the 6 dk runes, that shrinks and fills intuitively and automatically. Would probably be amazing with an actual progress texture rather than a basic one, but I was just testing and not going for perfect utility or aesthetics.

HUD-like cooldown tracking

This is two groups - the left one for externals and the right one for personals.

[InfusOnWoW]

This code seems identical to me with the CIRCLE one if constanFactor is "RADIUS".

[wtodom]

A lot of it is the same - my changes were based on one of the circle growers. But there are key differences, which I mentioned in the PR, even when circle has the constant factor set to radius.

Am I missing your point, or have I not clearly described how this grower is different?

[InfusOnWoW]

Well, your description does not match the code. The code is identical as far as I can tell.

[wtodom]

I feel like you have to have missed something. This PR contains exactly the code that's on my hard drive, which is also the code that the WoW client loads when I test. My description matches exactly what happens in the game, and the code in this PR for the circle types and the curve type is also different. The code differences are verifiable with whatever diff tool you prefer. I'm really not sure what's going on, because even 12 lines up from the line you commented on there's an if block that doesn't exist in the circle growers. I can see it while typing this reply, in the PR UI, without even using a diff tool. If it helps, here's a copy/paste of the two growers compared in a diff tool - https://www.diffchecker.com/BnB0Ba1l/.

[InfusOnWoW]

Great:
So let's ignore reverseFill for now.
Then you renamed numVisible to maxVisible
If we assume constatnFactor is "RADIUS", in the circle grow r is assigned radius, same as your code.

So where's the difference?

[InfusOnWoW]

Ah okay the difference is #regionData vs #data.controlledChildren.

Okay, we could have saved a lot of time if you pointed out the only change you actually did.

[wtodom]

Great: So let's ignore reverseFill for now. Then you renamed numVisible to maxVisible If we assume constatnFactor is "RADIUS", in the circle grow r is assigned radius, same as your code.

So where's the difference?

It is quite literally not the same. I don't understand why this isn't clear.

The circle growers operate on active auras only. With the constantFactor set to radius the spacing between auras changes depending on how many auras are active at any given time. Functionally this also makes it look like the group fills in an alternating pattern, sort of bouncing from side to side.

The grower in this PR has the spacing between auras determined by the total number of auras in the group. It always places newly active auras next to the ones that were already active without anything else moving.

I could try to say more, but at this point I think the simplest way to understand would be to try it out in-game, or if you prefer I can record a video for you. They are very obviously different in function, and the "rename" is also part of an actual functional change too, not a simple rename. How would you like to proceed?

[wtodom]

Ah okay the difference is #regionData vs #data.controlledChildren.

Okay, we could have saved a lot of time if you pointed out the only change you actually did.

That isn't the only change. It's a little more complex than that. I don't understand why you're being so defiant and dismissive.

[wtodom]

Additionally, I did mention that specific part of the change in my PR in the section about potential discussion. It's point 3.

[InfusOnWoW]

Where are any other changes? The renaming does nothing functional as far as I can tell. Again point to the exact line where your renaming changes the behaviour. You need to be able to communicate clearly.

Now onto the meat of your PR. That's the wrong approach. Grow functions should not depend on the total number of child auras due to cloning auras.

The better approach would be a separate setting for the angle between auras.

[mrbuds]

From my understanding of the intended use case, it would be better solved by adding a circular distribution option to normal groups, same as Distribute Horizontally/Vertically

[InfusOnWoW]

Nah, it's dynamically layouting the children, that part seems fine. And I think there's value there, in that people would use it. The constant spacing option is probably more dubious than this mode.

And the "distribution options" in normal group. I wish we'd replace them with an interface that users actually understand.

[wtodom]

I feel like we got off to a bad start, and I'd really like to learn and do things the right/best way. I'm not at all attached to this functionality being added in the way that this PR does it, and I'm happy to scrap it completely and work on it from another angle if someone can point me in the right direction. I certainly don't think I as a complete newcomer to developing for the project know what's best, I just did this the way that seemed doable.

@InfusOnWoW, to make sure I understand your comment above, is your suggestion to modify and/or add to the RegionOptions for the two existing Circle grow styles that allows them to function essentially like this PR does, but doing it by setting an angle that dictates spacing and placement rather than using the quantity of child auras in the group as the determiner?

Again, I'm just trying to understand what the best approach is, and from there I'm happy to do the work to learn and figure things out. I don't have any attachment to "getting my way" or anything like that, I'm just new to the project and wanna make it even better than it already is.

[InfusOnWoW]

"but doing it by setting an angle that dictates spacing and placement rather than using the quantity of child auras in the group as the determiner?"

Yes, exactly. The "dAngle" should be a user setting.

I'm think, this probably should be merged into the Circle/Countercircle grow. That would be another mode, e.g."constant spacing", "constant radius, spread", "constant radius and angle". It looks that apart from a different way to determine r, and dAngle the code is essentially the same.

The "reverse fill" option has several problems, but those don't need to be immediately fixed:

• For circle, the reverse is a separate grow type "Countercircle". Curve should work the same way. (That would be a point for merging into Circle.)
• The reverseFill currently works by filling from the end towards the start angle. It should imho work the same way as the countercircle, that is start from the startAngle and fill in the opposite direction. That's the same functionality, but fits better to how the other grows work.

[wtodom]

Yep, that makes sense. The question of "should this reverseFill be an option or an additional type?" also occurred to me in my initial pass, so I'm not surprised to see that feedback either.

I'll start working on adding that new 3rd option for determining spacing/placement in the circular grow styles like you described. Would you prefer to see that as part of a new PR with this one closed, or just as further changes here with the current changes reverted and an updated title and description in the PR?

[InfusOnWoW]

Same PR is fine. You can clean up the commits, but whatever works for you.

[wtodom]

Alright @mrbuds and @InfusOnWoW, it's obviously not urgent but I think this is ready for a round 2.

[InfusOnWoW]

I added a small commit changing the hidden functions. That we have hidden functions instead of visible comes from the underlying ace options framework, which turns out to be a bad design. We usually, though not consistently, write them with a top-level not(), which makes them easier to maintain.

[wtodom]

Yea, I noticed that the hidden functions were written differently sometimes, but I just tried to stay consistent with what the specific one I was modifying was like. Changing them to be consistent with each other definitely makes sense.

Thanks for working with me on this and adding it! Hopefully this won’t be my last.

[InfusOnWoW]

I'm looking forward to your future PRs.