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

UiRect axes constructor #7656

Merged
merged 9 commits into from
Apr 13, 2023

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Feb 13, 2023

Objective

We don't have a constructor function for UiRect that sets uniform horizontal and vertical values, even though it is a common pattern.

Solution

Add a constructor function to UiRect called axes, that sets both left and right to the same given horizontal value,
and sets both top and bottom to same given vertical value.

Changelog

  • Added a constructor function axes to UiRect.

* Added a constructor function `vertical_horizontal` for `UiRect`.
* Added a constructor function `horizontal_vertical` to `UiRect`.
@ickshonpe ickshonpe changed the title Add a constructor function for UiRect that sets the vertical and horizontal fields to uniform values Add a constructor function for UiRect that sets the vertical and horizontal fields to uniform values Feb 13, 2023
@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Feb 13, 2023
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I'm similarly unhappy with that name.

uniform_axes is better, but not great. xy is very terse but not super clear. paired_axes or matching_axes isn't any better.

What about UiRect::symmetric(horizontal, vertical)?

@ickshonpe
Copy link
Contributor Author

I'm not sure about symmetric either, it might not be very intuitive to people without a maths background.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Feb 13, 2023

Part of the problem is like UiRect isn't even used to represent rectangles, but renaming it to RetangularAnnulus is never going to fly.

@alice-i-cecile
Copy link
Member

renaming it to RetangularAnnulus is never going to fly.

#shipit 🚀 🚀 😎

Can we split out the different uses of UiRect? It's always felt like we're punning on the fact that the data can be represented in the same way, even though the semantic meaning of "rectangle" vs "annulus" is very different.

RectRing perhaps?

@mockersf mockersf mentioned this pull request Feb 15, 2023
@SDesya74
Copy link
Contributor

RectRing perhaps?

I also recently thought that the the name does not describe what the UiRect is. Perhaps the word Borders is appropriate?

@ickshonpe
Copy link
Contributor Author

I really hate the name axis, axis is singular not plural 😞

Maybe axes is a good enough compromise? It at least isn't likely to get confused with anything else.

@SDesya74
Copy link
Contributor

SDesya74 commented Feb 16, 2023

I like axes, word has the same length, but I'm not so familiar with English to speak with confidence

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Feb 16, 2023

RectRing perhaps?

I also recently thought that the the name does not describe what the UiRect is. Perhaps the word Borders is appropriate?

I thought about the name Frame. UiRect is used for a lot of very different things though. It would be really weird to have the type used to represent positions named Borders or Frame or RectRing.

Another option is to have separate types Margin, Border, Padding, Position, etc and use macros or cut and paste to implement UiRect's functions for each one.

At the moment the UiRect docs are a bit confusing, with different sections for each different property it can represent.
If it was split up, there could be longer docs for each property explaining in detail how to use each one.

@alice-i-cecile
Copy link
Member

I like axes as well for the name of this constructor method.

As for the type question, I like Frame quite a bit, and would support a PR renaming UiRect to that. Splitting the types is also very appealing.

@ickshonpe
Copy link
Contributor Author

Definitely axes.

@ickshonpe ickshonpe mentioned this pull request Mar 3, 2023
@ickshonpe ickshonpe changed the title Add a constructor function for UiRect that sets the vertical and horizontal fields to uniform values UiRect axes constructor Apr 13, 2023
@ickshonpe
Copy link
Contributor Author

This is ready to be merged I think.

@james7132 james7132 added this pull request to the merge queue Apr 13, 2023
@james7132 james7132 added this to the 0.11 milestone Apr 13, 2023
Merged via the queue into bevyengine:main with commit 0cbabef Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants