Skip to content

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Aug 11, 2025

Objective

impl From<f32> and From<i32> for Val

Solution

impl From<f32> and From<i32> for Val

@ickshonpe ickshonpe added D-Trivial Nice and easy! A great choice to get started with Bevy A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 11, 2025
@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Aug 12, 2025
@alice-i-cecile
Copy link
Member

Related to #15937.

@alice-i-cecile alice-i-cecile requested a review from cart August 12, 2025 02:54
@alice-i-cecile alice-i-cecile added S-Needs-SME Decision or review from an SME is required and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 12, 2025
@TheBlckbird
Copy link
Contributor

TheBlckbird commented Aug 12, 2025

I don't think, this is a good idea for the reasons mentioned in #15937 (comment)

And besides, if we have helper functions, four characters more for writing px(10) doesn't really make the difference. It only makes the code more readable and sticks more to the Rust idiom of not doing implicit stuff.

And besides (I already mentioned this in the issue), how should users know, that this is pixels? We still don't have a good place for documentation and just assuming, everyone knows, that a unitless value will be in pixels, will lead to confusion.

Picture yourself, not knowing anything about Bevy and you're reading code where a width of 100 is specified. You will need to google this or just assume it's going to be in pixels without being sure.
Now the other option would to call the helper function, which will immediately tell you the unit and therefore resolve any confusion.

So I'm definitely for closing this and adding the trait someone proposed to the other PR (#20518 (comment)).

@ickshonpe
Copy link
Contributor Author

I don't think, this is a good idea for the reasons mentioned in #15937 (comment)

And besides, if we have helper functions, four characters more for writing px(10) doesn't really make the difference. It only makes the code more readable and sticks more to the Rust idiom of not doing implicit stuff.

And besides (I already mentioned this in the issue), how should users know, that this is pixels? We still don't have a good place for documentation and just assuming, everyone knows, that a unitless value will be in pixels, will lead to confusion.

Picture yourself, not knowing anything about Bevy and you're reading code where a width of 100 is specified. You will need to google this or just assume it's going to be in pixels without being sure. Now the other option would to call the helper function, which will immediately tell you the unit and therefore resolve any confusion.

So I'm definitely for closing this and adding the trait someone proposed to the other PR (#20518 (comment)).

IIRC the problem with the AsValNum trait is it wouldn't work with bsn!.

I don't see From<f32> mapping to Val::Px as confusing at all. To me its very natural, CSS and most other layout styling languages do the same thing.

@TheBlckbird
Copy link
Contributor

TheBlckbird commented Aug 12, 2025

To me its very natural, CSS and most other layout styling languages do the same thing.

No, you can't have unitless values in CSS (except for 0), you always have to specify it

@ickshonpe
Copy link
Contributor Author

Oh yeah you're right, it's been years since I've done any css. Most other layout models like figma allow you to ellide the units though.

@cart
Copy link
Member

cart commented Aug 12, 2025

I don't think making px the default implicit unit is the right path. I'd rather be explicit about this, and make this easier using Px(1.) flattened exports.

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 D-Trivial Nice and easy! A great choice to get started with Bevy S-Needs-SME Decision or review from an SME is required X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants