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.

Sign up for GitHub

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

Made UIRect initialisation functions const #16823

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Made UIRect initialisation functions const #16823

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e41f52
Update geometry.rs
Kees-van-Beilen Dec 14, 2024
5579218
Revert "Update geometry.rs"
Kees-van-Beilen Dec 14, 2024
52ad0f7
Update geometry.rs
Kees-van-Beilen Dec 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 31 additions & 37 deletions crates/bevy_ui/src/geometry.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,12 +390,11 @@ impl UiRect {
/// assert_eq!(ui_rect.top, Val::ZERO);
/// assert_eq!(ui_rect.bottom, Val::ZERO);
/// ```
pub fn horizontal(value: Val) -> Self {
UiRect {
left: value,
right: value,
..Default::default()
}
pub const fn horizontal(value: Val) -> Self {
let mut rect = UiRect::DEFAULT;
rect.left = value;
rect.right = value;
rect
}

/// Creates a new [`UiRect`] where `top` and `bottom` take the given value,
Expand All @@ -413,12 +412,11 @@ impl UiRect {
/// assert_eq!(ui_rect.top, Val::Px(10.0));
/// assert_eq!(ui_rect.bottom, Val::Px(10.0));
/// ```
pub fn vertical(value: Val) -> Self {
UiRect {
top: value,
bottom: value,
..Default::default()
}
pub const fn vertical(value: Val) -> Self {
let mut rect = UiRect::DEFAULT;
rect.top = value;
rect.bottom = value;
rect
}

/// Creates a new [`UiRect`] where both `left` and `right` take the value of `horizontal`, and both `top` and `bottom` take the value of `vertical`.
Expand All @@ -435,7 +433,7 @@ impl UiRect {
/// assert_eq!(ui_rect.top, Val::Percent(15.0));
/// assert_eq!(ui_rect.bottom, Val::Percent(15.0));
/// ```
pub fn axes(horizontal: Val, vertical: Val) -> Self {
pub const fn axes(horizontal: Val, vertical: Val) -> Self {
UiRect {
left: horizontal,
right: horizontal,
Expand All @@ -459,11 +457,10 @@ impl UiRect {
/// assert_eq!(ui_rect.top, Val::ZERO);
/// assert_eq!(ui_rect.bottom, Val::ZERO);
/// ```
pub fn left(value: Val) -> Self {
UiRect {
left: value,
..Default::default()
}
pub const fn left(value: Val) -> Self {
let mut rect = UiRect::DEFAULT;
rect.left = value;
rect
}

Copy link
Contributor

@ickshonpe ickshonpe Dec 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'd prefer that we used struct update syntax for the constructor functions, like:

Suggested change
pub const fn left(value: Val) -> Self {
let mut rect = UiRect::DEFAULT;
rect.left = value;
rect
}
pub const fn left(left: Val) -> Self {
Self {
left,
..Self::DEFAULT
}
}

is simpler and clearer.

Copy link
Contributor Author

@Kees-van-Beilen Kees-van-Beilen Dec 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

That's the entire reason I've made this PR, ..Self::DEFAULT cannot be used in const functions. I also would love to use that notation, but it is going to be a while before rust will support this Sorry I assumed that that wasn't possible as the exact same scenario didn't work on my computer, but i just checked your code change suggestion and that does work. Will refactor and update 👍 thank you

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought you can, can't you? Self::DEFAULT is a const value, it's not Self::default()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently when a struct implements Drop or one of its field implements Drop then you cannot use the struct updater syntax. So for instance you can't write:

const MY_NODE:Node = Node {..Node::DEFAULT};

because Node has a field of type Vec which implements Drop. However this shouldn't be a problem in UIRect's case.

/// Creates a new [`UiRect`] where `right` takes the given value,
Expand All @@ -481,11 +478,10 @@ impl UiRect {
/// assert_eq!(ui_rect.top, Val::ZERO);
/// assert_eq!(ui_rect.bottom, Val::ZERO);
/// ```
pub fn right(value: Val) -> Self {
UiRect {
right: value,
..Default::default()
}
pub const fn right(value: Val) -> Self {
let mut rect = UiRect::DEFAULT;
rect.right = value;
rect
}

/// Creates a new [`UiRect`] where `top` takes the given value,
Expand All @@ -503,11 +499,10 @@ impl UiRect {
/// assert_eq!(ui_rect.top, Val::Px(10.0));
/// assert_eq!(ui_rect.bottom, Val::ZERO);
/// ```
pub fn top(value: Val) -> Self {
UiRect {
top: value,
..Default::default()
}
pub const fn top(value: Val) -> Self {
let mut rect = UiRect::DEFAULT;
rect.top = value;
rect
}

/// Creates a new [`UiRect`] where `bottom` takes the given value,
Expand All @@ -525,11 +520,10 @@ impl UiRect {
/// assert_eq!(ui_rect.top, Val::ZERO);
/// assert_eq!(ui_rect.bottom, Val::Px(10.0));
/// ```
pub fn bottom(value: Val) -> Self {
UiRect {
bottom: value,
..Default::default()
}
pub const fn bottom(value: Val) -> Self {
let mut rect = UiRect::DEFAULT;
rect.bottom = value;
rect
}

/// Returns the [`UiRect`] with its `left` field set to the given value.
Expand All @@ -546,7 +540,7 @@ impl UiRect {
/// assert_eq!(ui_rect.bottom, Val::Px(20.0));
/// ```
#[inline]
pub fn with_left(mut self, left: Val) -> Self {
pub const fn with_left(mut self, left: Val) -> Self {
self.left = left;
self
}
Expand All @@ -565,7 +559,7 @@ impl UiRect {
/// assert_eq!(ui_rect.bottom, Val::Px(20.0));
/// ```
#[inline]
pub fn with_right(mut self, right: Val) -> Self {
pub const fn with_right(mut self, right: Val) -> Self {
self.right = right;
self
}
Expand All @@ -584,7 +578,7 @@ impl UiRect {
/// assert_eq!(ui_rect.bottom, Val::Px(20.0));
/// ```
#[inline]
pub fn with_top(mut self, top: Val) -> Self {
pub const fn with_top(mut self, top: Val) -> Self {
self.top = top;
self
}
Expand All @@ -603,7 +597,7 @@ impl UiRect {
/// assert_eq!(ui_rect.bottom, Val::Px(10.0));
/// ```
#[inline]
pub fn with_bottom(mut self, bottom: Val) -> Self {
pub const fn with_bottom(mut self, bottom: Val) -> Self {
self.bottom = bottom;
self
}
Expand Down
Loading