From eaac730617103b61fd5ede9dca463243f2d7900e Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Sat, 11 Feb 2023 23:07:16 +0000 Subject: [PATCH] Fix the `Size` helper functions using the wrong default value and improve the UI examples (#7626) # Objective `Size::width` sets the `height` field to `Val::DEFAULT` which is `Val::Undefined`, but the default for `Size` `height` is `Val::Auto`. `Size::height` has the same problem, but with the `width` field. The UI examples specify numeric values in many places where they could either be elided or replaced by composition of the Flex enum properties. related: https://github.com/bevyengine/bevy/pull/7468 fixes: https://github.com/bevyengine/bevy/issues/6498 ## Solution Change `Size::width` so it sets `height` to `Val::AUTO` and change `Size::height` so it sets `width` to `Val::AUTO`. Added some tests so this doesn't happen again. ## Changelog Changed `Size::width` so it sets the `height` to `Val::AUTO`. Changed `Size::height` so it sets the `width` to `Val::AUTO`. Added tests to `geometry.rs` for `Size` and `UiRect` to ensure correct behaviour. Simplified the UI examples. Replaced numeric values with the Flex property enums or elided them where possible, and removed the remaining use of auto margins. ## Migration Guide The `Size::width` constructor function now sets the `height` to `Val::Auto` instead of `Val::Undefined`. The `Size::height` constructor function now sets the `width` to `Val::Auto` instead of `Val::Undefined`. --- crates/bevy_ui/src/geometry.rs | 71 +++++++++++++++++++++++-- examples/ui/button.rs | 2 +- examples/ui/relative_cursor_position.rs | 6 +-- examples/ui/transparency_ui.rs | 16 +----- examples/ui/ui.rs | 39 ++++++-------- examples/ui/z_index.rs | 2 +- 6 files changed, 91 insertions(+), 45 deletions(-) diff --git a/crates/bevy_ui/src/geometry.rs b/crates/bevy_ui/src/geometry.rs index 8f80c346e5..76168c98e0 100644 --- a/crates/bevy_ui/src/geometry.rs +++ b/crates/bevy_ui/src/geometry.rs @@ -339,7 +339,7 @@ pub struct Size { } impl Size { - pub const DEFAULT: Self = Self::all(Val::Auto); + pub const DEFAULT: Self = Self::AUTO; /// Creates a new [`Size`] from a width and a height. /// @@ -369,14 +369,14 @@ impl Size { pub const fn width(width: Val) -> Self { Self { width, - height: Val::DEFAULT, + height: Val::Auto, } } /// Creates a new [`Size`] where `height` takes the given value. pub const fn height(height: Val) -> Self { Self { - width: Val::DEFAULT, + width: Val::Auto, height, } } @@ -443,6 +443,20 @@ impl DivAssign for Size { mod tests { use super::*; + #[test] + fn uirect_default_equals_const_default() { + assert_eq!( + UiRect::default(), + UiRect { + left: Val::Undefined, + right: Val::Undefined, + top: Val::Undefined, + bottom: Val::Undefined + } + ); + assert_eq!(UiRect::default(), UiRect::DEFAULT); + } + #[test] fn test_size_from() { let size: Size = (Val::Px(20.), Val::Px(30.)).into(); @@ -476,4 +490,55 @@ mod tests { size /= 2.; assert_eq!(size, Size::new(Val::Px(10.), Val::Px(10.))); } + + #[test] + fn test_size_all() { + let length = Val::Px(10.); + + assert_eq!( + Size::all(length), + Size { + width: length, + height: length + } + ); + } + + #[test] + fn test_size_width() { + let width = Val::Px(10.); + + assert_eq!( + Size::width(width), + Size { + width, + ..Default::default() + } + ); + } + + #[test] + fn test_size_height() { + let height = Val::Px(7.); + + assert_eq!( + Size::height(height), + Size { + height, + ..Default::default() + } + ); + } + + #[test] + fn size_default_equals_const_default() { + assert_eq!( + Size::default(), + Size { + width: Val::Auto, + height: Val::Auto + } + ); + assert_eq!(Size::default(), Size::DEFAULT); + } } diff --git a/examples/ui/button.rs b/examples/ui/button.rs index ab8b625e09..aa364baf8b 100644 --- a/examples/ui/button.rs +++ b/examples/ui/button.rs @@ -49,7 +49,7 @@ fn setup(mut commands: Commands, asset_server: Res) { commands .spawn(NodeBundle { style: Style { - size: Size::new(Val::Percent(100.0), Val::Percent(100.0)), + size: Size::width(Val::Percent(100.0)), align_items: AlignItems::Center, justify_content: JustifyContent::Center, ..default() diff --git a/examples/ui/relative_cursor_position.rs b/examples/ui/relative_cursor_position.rs index a02a5b3a2d..54e9c527c6 100644 --- a/examples/ui/relative_cursor_position.rs +++ b/examples/ui/relative_cursor_position.rs @@ -18,7 +18,7 @@ fn setup(mut commands: Commands, asset_server: Res) { commands .spawn(NodeBundle { style: Style { - size: Size::new(Val::Percent(100.0), Val::Percent(100.0)), + size: Size::width(Val::Percent(100.)), align_items: AlignItems::Center, justify_content: JustifyContent::Center, flex_direction: FlexDirection::Column, @@ -30,8 +30,8 @@ fn setup(mut commands: Commands, asset_server: Res) { parent .spawn(NodeBundle { style: Style { - size: Size::new(Val::Px(250.0), Val::Px(250.0)), - margin: UiRect::new(Val::Px(0.), Val::Px(0.), Val::Px(0.), Val::Px(15.)), + size: Size::all(Val::Px(250.0)), + margin: UiRect::bottom(Val::Px(15.)), ..default() }, background_color: Color::rgb(235., 35., 12.).into(), diff --git a/examples/ui/transparency_ui.rs b/examples/ui/transparency_ui.rs index a70193b940..b37b3edf58 100644 --- a/examples/ui/transparency_ui.rs +++ b/examples/ui/transparency_ui.rs @@ -19,9 +19,9 @@ fn setup(mut commands: Commands, asset_server: Res) { commands .spawn(NodeBundle { style: Style { - size: Size::new(Val::Percent(50.0), Val::Percent(100.0)), + size: Size::width(Val::Percent(100.0)), align_items: AlignItems::Center, - justify_content: JustifyContent::Center, + justify_content: JustifyContent::SpaceAround, ..default() }, ..default() @@ -49,19 +49,7 @@ fn setup(mut commands: Commands, asset_server: Res) { }, )); }); - }); - commands - .spawn(NodeBundle { - style: Style { - size: Size::new(Val::Percent(50.0), Val::Percent(100.0)), - align_items: AlignItems::Center, - justify_content: JustifyContent::Center, - ..default() - }, - ..default() - }) - .with_children(|parent| { // Button with a different color, // to demonstrate the text looks different due to its transparency. parent diff --git a/examples/ui/ui.rs b/examples/ui/ui.rs index 08d5a710f9..b8f2d21e35 100644 --- a/examples/ui/ui.rs +++ b/examples/ui/ui.rs @@ -24,7 +24,7 @@ fn setup(mut commands: Commands, asset_server: Res) { commands .spawn(NodeBundle { style: Style { - size: Size::new(Val::Percent(100.0), Val::Percent(100.0)), + size: Size::width(Val::Percent(100.0)), justify_content: JustifyContent::SpaceBetween, ..default() }, @@ -35,7 +35,7 @@ fn setup(mut commands: Commands, asset_server: Res) { parent .spawn(NodeBundle { style: Style { - size: Size::new(Val::Px(200.0), Val::Percent(100.0)), + size: Size::width(Val::Px(200.0)), border: UiRect::all(Val::Px(2.0)), ..default() }, @@ -47,7 +47,7 @@ fn setup(mut commands: Commands, asset_server: Res) { parent .spawn(NodeBundle { style: Style { - size: Size::new(Val::Percent(100.0), Val::Percent(100.0)), + size: Size::width(Val::Percent(100.0)), ..default() }, background_color: Color::rgb(0.15, 0.15, 0.15).into(), @@ -77,7 +77,8 @@ fn setup(mut commands: Commands, asset_server: Res) { style: Style { flex_direction: FlexDirection::Column, justify_content: JustifyContent::Center, - size: Size::new(Val::Px(200.0), Val::Percent(100.0)), + align_items: AlignItems::Center, + size: Size::width(Val::Px(200.0)), ..default() }, background_color: Color::rgb(0.15, 0.15, 0.15).into(), @@ -95,12 +96,7 @@ fn setup(mut commands: Commands, asset_server: Res) { }, ) .with_style(Style { - size: Size::new(Val::Undefined, Val::Px(25.)), - margin: UiRect { - left: Val::Auto, - right: Val::Auto, - ..default() - }, + size: Size::height(Val::Px(25.)), ..default() }), ); @@ -109,8 +105,8 @@ fn setup(mut commands: Commands, asset_server: Res) { .spawn(NodeBundle { style: Style { flex_direction: FlexDirection::Column, - align_self: AlignSelf::Center, - size: Size::new(Val::Percent(100.0), Val::Percent(50.0)), + align_self: AlignSelf::Stretch, + size: Size::height(Val::Percent(50.0)), overflow: Overflow::Hidden, ..default() }, @@ -126,6 +122,7 @@ fn setup(mut commands: Commands, asset_server: Res) { flex_direction: FlexDirection::Column, flex_grow: 1.0, max_size: Size::UNDEFINED, + align_items: AlignItems::Center, ..default() }, ..default() @@ -148,11 +145,6 @@ fn setup(mut commands: Commands, asset_server: Res) { .with_style(Style { flex_shrink: 0., size: Size::new(Val::Undefined, Val::Px(20.)), - margin: UiRect { - left: Val::Auto, - right: Val::Auto, - ..default() - }, ..default() }), ); @@ -211,7 +203,8 @@ fn setup(mut commands: Commands, asset_server: Res) { .with_children(|parent| { parent.spawn(NodeBundle { style: Style { - size: Size::new(Val::Px(100.0), Val::Px(100.0)), + // Take the size of the parent node. + size: Size::all(Val::Percent(100.)), position_type: PositionType::Absolute, position: UiRect { left: Val::Px(20.0), @@ -225,7 +218,7 @@ fn setup(mut commands: Commands, asset_server: Res) { }); parent.spawn(NodeBundle { style: Style { - size: Size::new(Val::Px(100.0), Val::Px(100.0)), + size: Size::all(Val::Percent(100.)), position_type: PositionType::Absolute, position: UiRect { left: Val::Px(40.0), @@ -239,7 +232,7 @@ fn setup(mut commands: Commands, asset_server: Res) { }); parent.spawn(NodeBundle { style: Style { - size: Size::new(Val::Px(100.0), Val::Px(100.0)), + size: Size::all(Val::Percent(100.)), position_type: PositionType::Absolute, position: UiRect { left: Val::Px(60.0), @@ -254,7 +247,7 @@ fn setup(mut commands: Commands, asset_server: Res) { // alpha test parent.spawn(NodeBundle { style: Style { - size: Size::new(Val::Px(100.0), Val::Px(100.0)), + size: Size::all(Val::Percent(100.)), position_type: PositionType::Absolute, position: UiRect { left: Val::Px(80.0), @@ -272,7 +265,7 @@ fn setup(mut commands: Commands, asset_server: Res) { parent .spawn(NodeBundle { style: Style { - size: Size::new(Val::Percent(100.0), Val::Percent(100.0)), + size: Size::width(Val::Percent(100.)), position_type: PositionType::Absolute, justify_content: JustifyContent::Center, align_items: AlignItems::FlexStart, @@ -284,7 +277,7 @@ fn setup(mut commands: Commands, asset_server: Res) { // bevy logo (image) parent.spawn(ImageBundle { style: Style { - size: Size::new(Val::Px(500.0), Val::Auto), + size: Size::width(Val::Px(500.0)), ..default() }, image: asset_server.load("branding/bevy_logo_dark_big.png").into(), diff --git a/examples/ui/z_index.rs b/examples/ui/z_index.rs index d15f5dcbeb..08a51b0a68 100644 --- a/examples/ui/z_index.rs +++ b/examples/ui/z_index.rs @@ -22,7 +22,7 @@ fn setup(mut commands: Commands) { commands .spawn(NodeBundle { style: Style { - size: Size::new(Val::Percent(100.0), Val::Percent(100.0)), + size: Size::width(Val::Percent(100.0)), align_items: AlignItems::Center, justify_content: JustifyContent::Center, ..default()