-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Change UiScale
to a tuple struct
#9444
Conversation
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
Example |
Nice and simple, but I'm not fully sold. Can you say a bit more about why you prefer this syntax? |
It feels more consistent and looks nicer for me :) |
Would it be simpler to change the struct to a tuple struct? |
Example |
1 similar comment
Example |
I think so, but wouldn't it be a breaking change? |
Yep, but breaking changes are totally fine in Bevy at this stage. Especially for a very niche part of the API like this. I'd prefer a tuple struct over the current change as well, although I'm neutral on tuple vs named struct. |
Done |
UiScale
to a tuple struct
I prefer named struct when the name brings new information. I don't think a single field You can probably derive |
@mockersf Should I add a method like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the tuple struct too.
Example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UiScale doc comment isn't very informative atm. Maybe it could be changed to say something about how logical coordinates are multiplied by window scale factor and UiScale to get physical coordinates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO tuple structs are better for wrapping a single meaningless value.
Also here it avoids the repetition of the word "scale".
Example |
Example |
Example |
@alice-i-cecile I think this PR is ready to merge |
Objective
Inconvenient initialization of
UiScale
Solution
Change
UiScale
to a tuple structMigration Guide
Replace initialization of
UiScale
likeUiScale { scale: 1.0 }
withUiScale(1.0)