Skip to content

Val::MAX constant - #25096

Open
ickshonpe wants to merge 6 commits into
bevyengine:mainfrom
ickshonpe:const-val-max
Open

Val::MAX constant#25096
ickshonpe wants to merge 6 commits into
bevyengine:mainfrom
ickshonpe:const-val-max

Conversation

@ickshonpe

@ickshonpe ickshonpe commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Objective

ATM you wrap f32::MAX by an arbitrary scalar Val variant to express a maximum value,
To make it clearer that a max value is being used intentionally, add a canonical Val::MAX constant.

Solution

Add a Val::MAX constant equal to Val::Px(f32::MAX)

@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 S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 21, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in UI Jul 21, 2026
@ickshonpe ickshonpe added the C-Usability A targeted quality-of-life change that makes Bevy easier to use label Jul 21, 2026
Comment thread crates/bevy_ui/src/ui_node.rs Outdated

/// Maximum curvature. The UI Node will take a capsule shape or circular if width and height are equal.
pub const MAX: Self = Self::all_circular(Val::Px(f32::MAX));
pub const MAX: Self = Self::all_circular(Val::MAX);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it confusing that “px” has now been lost at call sites though? I suppose MAX matches what ZERO does, but possibly 0 is the same end result whether it’s in pixels or percents but is f32::MAX pixels the same as f32::MAX percent?

Also if I think of a maximum value of percentage units, I tend to think of that as 100% even though I know you can definitely provide a larger f32 than 100.0 when constructing a percentage Val.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops yeah, what's written in the objective about wrapping f32::MAX by an arbitrary variant is wrong, I was just struggling to think of something smart to say hehe.

Using Val::percent(f32::Max) is not a good idea. The base value could be 0. in which case the resolved value will be 0., or inf if it's greater than 1.

Maybe that's an argument in favor of having a constant though, if you naively try to express a maximum value using one of the non-Px variants you might end up with a panic

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Using Val::percent(f32::Max) is not a good idea.

Yep I'm not surprised this is a bad idea. I was just using f32::Max for percents as a way to argue the question "why do pixels/Px get to claim the MAX name under Val even though there are many enum variants in it"?


Thinking about the change though, the const here is BorderRadius::Max and that's the one users are going to use, but it's really just an implementation detail what you put on the right hand side of that, right? Self::all_circular(Val::Px(f32::MAX)); seems a fine thing to put on the right hand side of it, and it seems fine for that to read as an implementation detail, no? That said, if there are cases where users want Val::MAX themselves, then maybe the const (or similar) is good to add, but do you think those use cases exist, or is Val::MAX really the only "user" here?

impl Val {
pub const DEFAULT: Self = Self::Auto;
pub const ZERO: Self = Self::Px(0.0);
pub const MAX: Self = Self::Px(f32::MAX);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Docs on this and Zero that they're in Px units please.

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 21, 2026
@ickshonpe ickshonpe added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Aug 19, 2026
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-Review Needs reviewer attention (from anyone!) to move forward

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

3 participants