Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for dynamic grouping #31

Merged
merged 20 commits into from
Jul 21, 2024
Merged

Add support for dynamic grouping #31

merged 20 commits into from
Jul 21, 2024

Conversation

trevorm4
Copy link
Contributor

@trevorm4 trevorm4 commented Jul 8, 2024

Since #16 has gotten stale and I wanted new functionality, making a new pr.

What:
This PR adds support for dynamic groups that will either define static positions based on how many elements in the group or automatically redraw each element based on how many elements are currently triggered.

Why:
Adding support for this greatly simplifies setup of positioning of groups. Instead of having to go and manually position every element in a group, you can just choose an offset to do the work automatically. In addition, dynamic redrawing adds new functionality for keeping groups that grow and shrink neat and organized.

How:
It achieves dynamic redrawing by making Draw(...) return a boolean value indicating whether the element was drawn or not, which we can then use to maintain an index in Group::Draw(...)

With dynamic redrawing
https://github.com/DelvUI/DelvCD/assets/11250934/2d4f17e8-6788-4912-9ba4-a0b33d475862

Whenever an icon in the group becomes not triggered, it moves elements above it in the group down.

Without dynamic redrawing
image

@trevorm4 trevorm4 marked this pull request as ready for review July 8, 2024 16:05
@trevorm4 trevorm4 marked this pull request as draft July 8, 2024 16:08
@trevorm4 trevorm4 marked this pull request as ready for review July 8, 2024 16:12
@trevorm4
Copy link
Contributor Author

@Tischel Would you be able to review this PR or close it if you don't want it in the project?

@Tischel
Copy link
Contributor

Tischel commented Jul 16, 2024

A feature like this should include "growth directions" aka let the user decide in which directions the elements are gonna be drawn.
DelvUI already has a lot of code to accomplish this, feel free to use that as a guideline (check StatusEffectListHud as an example).

@Zeffuro
Copy link
Contributor

Zeffuro commented Jul 16, 2024

If I may make a suggestion as well, a way of having it grow evenly among the X or Y axis would be lovely to have as well. That'd be nice to keep a set of auras centered on someone's screen.

@trevorm4 trevorm4 marked this pull request as draft July 17, 2024 02:13
@trevorm4 trevorm4 changed the title [rfc] Add support for dynamic grouping Add support for dynamic grouping Jul 18, 2024
@trevorm4 trevorm4 marked this pull request as ready for review July 18, 2024 00:25
@trevorm4
Copy link
Contributor Author

trevorm4 commented Jul 18, 2024

I added support for growth directions, supporting all the ones from the DelvUI example

DelvCD groups don't have the concept of bounding box, so all of the centering is based on row/col with the user specifying a max elements per row. The main reason for this is in order to center an icon half way between 2 icons, we would need to know the size but that is all abstracted away by the draw function. In the future the base classes could be modified to expose a size attribute, but left that outside of the scope of this PR for now.

https://streamable.com/fisx84

@Zeffuro
Copy link
Contributor

Zeffuro commented Jul 18, 2024

That's looking pretty good :O

@Tischel
Copy link
Contributor

Tischel commented Jul 20, 2024

What does "dynamically redraw elements" do?

@trevorm4
Copy link
Contributor Author

trevorm4 commented Jul 20, 2024

What does "dynamically redraw elements" do?

If an icon is not drawn, then the next element drawn will take its place.

Example: if icon 1 would be drawn at place a and icon 2 would be drawn at place b, then if icon 1 does not draw itself (due to conditions) then icon 2 will be attempted to be drawn at place a instead. This essentially results in there not being gaps between icons if certain icons do not meet conditions

Demo: https://streamable.com/4bh8dy

Notice how when I consume my creature, the hammer stamp icon moves up to take its place since the group is configured as grow Right and Down. This essentially emulates the same behavior of the DelvUI status bar where it keeps all the icons grouped together.

@Tischel
Copy link
Contributor

Tischel commented Jul 20, 2024

What does "dynamically redraw elements" do?

If an icon is not drawn, then the next element drawn will take its place.

Example: if icon 1 would be drawn at place a and icon 2 would be drawn at place b, then if icon 1 does not draw itself (due to conditions) then icon 2 will be attempted to be drawn at place a instead. This essentially results in there not being gaps between icons if certain icons do not meet conditions

Demo: https://streamable.com/4bh8dy

Notice how when I consume my creature, the hammer stamp icon moves up to take its place since the group is configured as grow Right and Down. This essentially emulates the same behavior of the DelvUI status bar where it keeps all the icons grouped together.

Why would I ever want spacing between the icons though?
I'm trying to figure out a scenario where I'd want that toggle deactivated.

@trevorm4
Copy link
Contributor Author

What does "dynamically redraw elements" do?

If an icon is not drawn, then the next element drawn will take its place.
Example: if icon 1 would be drawn at place a and icon 2 would be drawn at place b, then if icon 1 does not draw itself (due to conditions) then icon 2 will be attempted to be drawn at place a instead. This essentially results in there not being gaps between icons if certain icons do not meet conditions
Demo: streamable.com/4bh8dy
Notice how when I consume my creature, the hammer stamp icon moves up to take its place since the group is configured as grow Right and Down. This essentially emulates the same behavior of the DelvUI status bar where it keeps all the icons grouped together.

Why would I ever want spacing between the icons though? I'm trying to figure out a scenario where I'd want that toggle deactivated.

Yea that is fair, I can change it to always to be on if you'd like but was going to defer it to the user in case somebody had some niche scenario in mind.

@Tischel
Copy link
Contributor

Tischel commented Jul 20, 2024

I think in general the simpler and more intuitive the settings are the better.
In this particular case I think we should just remove that setting and leave it on by default.

@trevorm4
Copy link
Contributor Author

I think in general the simpler and more intuitive the settings are the better. In this particular case I think we should just remove that setting and leave it on by default.

Updated to remove the option

@Tischel Tischel merged commit b0e2ff0 into DelvUI:main Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants