[Merged by Bors] - Utility methods for Val #6134

Pietrek14 · 2022-09-30T22:56:08Z

Objective

Adds a better interface for performing mathematical operations with UI unit Val. Fixes #6080.

Solution

Added try_add and try_sub methods to Val.
Removed the Add and AddAssign impls for Val that introduced unintuitive and bug-prone behaviour.
As a consequence of the prior, ~~changed the Add and Sub impls for the Size struct to take a (Val, Val) instead of Vec2~~ deleted the Add and Sub impls for the Size struct
Added a From<(Val, Val)> impl for the Size struct
Added evaluate(size: f32) method that converts from Val::Percent to Val::Px.
Added try_add_with_size and try_sub_with_size methods to Val, which evaluate Val::Percent values into Val::Px values before adding.

Migration Guide

Instead of using the + and - operators, perform calculations on Vals using the new try_add and try_sub methods. Multiplication and division remained unchanged. Also, when adding or subtracting from Size, ~~use a Val tuple instead of Vec2~~ perform the addition on width and height separately.

Co-authored-by: Mike <mike.hsu@gmail.com>

crates/bevy_ui/src/ui_node.rs

crates/bevy_ui/src/geometry.rs

… api and added add/sub assigning with size

alice-i-cecile · 2022-10-04T19:52:40Z

@Pietrek14 feel free to resolve comments as you address them; it makes it much easier for other reviewers to see what still needs to be done :)

Pietrek14 · 2022-10-05T15:36:44Z

I think I covered it all. Sorry it took so long, I was busy with Ludum Dare.

crates/bevy_ui/src/geometry.rs

crates/bevy_ui/src/ui_node.rs

…ich is private

Pietrek14 · 2022-10-08T16:55:16Z

Should I adjust the migration guide in the PR description, since I deleted the addition and subtraction of Size?

alice-i-cecile · 2022-10-08T17:02:41Z

Yes please :)

tguichaoua

There is two methods without doc.

Other than that LGTM.

crates/bevy_ui/src/ui_node.rs

alice-i-cecile

Great work, I'm happy with how this turned out. Love the error enum. We can add more impls and methods later once we see how this batch plays out.

bors r+

# Objective Adds a better interface for performing mathematical operations with UI unit `Val`. Fixes #6080. ## Solution - Added `try_add` and `try_sub` methods to Val. - Removed the `Add` and `AddAssign` impls for `Val` that introduced unintuitive and bug-prone behaviour. - As a consequence of the prior, ~~changed the `Add` and `Sub` impls for the `Size` struct to take a `(Val, Val)` instead of `Vec2`~~ deleted the `Add` and `Sub` impls for the `Size` struct - Added a `From<(Val, Val)>` impl for the `Size` struct - Added `evaluate(size: f32)` method that converts from `Val::Percent` to `Val::Px`. - Added `try_add_with_size` and `try_sub_with_size` methods to `Val`, which evaluate `Val::Percent` values into `Val::Px` values before adding. --- ## Migration Guide Instead of using the + and - operators, perform calculations on `Val`s using the new `try_add` and `try_sub` methods. Multiplication and division remained unchanged. Also, when adding or subtracting from `Size`, ~~use a `Val` tuple instead of `Vec2`~~ perform the addition on `width` and `height` separately. Co-authored-by: Dawid Piotrowski <41804418+Pietrek14@users.noreply.github.com>

bors · 2022-10-24T14:14:05Z

Build failed (retrying...):

check-compiles

# Objective Adds a better interface for performing mathematical operations with UI unit `Val`. Fixes #6080. ## Solution - Added `try_add` and `try_sub` methods to Val. - Removed the `Add` and `AddAssign` impls for `Val` that introduced unintuitive and bug-prone behaviour. - As a consequence of the prior, ~~changed the `Add` and `Sub` impls for the `Size` struct to take a `(Val, Val)` instead of `Vec2`~~ deleted the `Add` and `Sub` impls for the `Size` struct - Added a `From<(Val, Val)>` impl for the `Size` struct - Added `evaluate(size: f32)` method that converts from `Val::Percent` to `Val::Px`. - Added `try_add_with_size` and `try_sub_with_size` methods to `Val`, which evaluate `Val::Percent` values into `Val::Px` values before adding. --- ## Migration Guide Instead of using the + and - operators, perform calculations on `Val`s using the new `try_add` and `try_sub` methods. Multiplication and division remained unchanged. Also, when adding or subtracting from `Size`, ~~use a `Val` tuple instead of `Vec2`~~ perform the addition on `width` and `height` separately. Co-authored-by: Dawid Piotrowski <41804418+Pietrek14@users.noreply.github.com>

bors · 2022-10-24T14:52:54Z

Pull request successfully merged into main.

Build succeeded:

Pietrek14 · 2022-10-24T15:20:08Z

It's really not that important, but do PRs merged by bors count for Hacktoberfest?

# Objective Adds a better interface for performing mathematical operations with UI unit `Val`. Fixes bevyengine#6080. ## Solution - Added `try_add` and `try_sub` methods to Val. - Removed the `Add` and `AddAssign` impls for `Val` that introduced unintuitive and bug-prone behaviour. - As a consequence of the prior, ~~changed the `Add` and `Sub` impls for the `Size` struct to take a `(Val, Val)` instead of `Vec2`~~ deleted the `Add` and `Sub` impls for the `Size` struct - Added a `From<(Val, Val)>` impl for the `Size` struct - Added `evaluate(size: f32)` method that converts from `Val::Percent` to `Val::Px`. - Added `try_add_with_size` and `try_sub_with_size` methods to `Val`, which evaluate `Val::Percent` values into `Val::Px` values before adding. --- ## Migration Guide Instead of using the + and - operators, perform calculations on `Val`s using the new `try_add` and `try_sub` methods. Multiplication and division remained unchanged. Also, when adding or subtracting from `Size`, ~~use a `Val` tuple instead of `Vec2`~~ perform the addition on `width` and `height` separately. Co-authored-by: Dawid Piotrowski <41804418+Pietrek14@users.noreply.github.com>

Adds a better interface for performing mathematical operations with UI unit `Val`. Fixes bevyengine#6080. - Added `try_add` and `try_sub` methods to Val. - Removed the `Add` and `AddAssign` impls for `Val` that introduced unintuitive and bug-prone behaviour. - As a consequence of the prior, ~~changed the `Add` and `Sub` impls for the `Size` struct to take a `(Val, Val)` instead of `Vec2`~~ deleted the `Add` and `Sub` impls for the `Size` struct - Added a `From<(Val, Val)>` impl for the `Size` struct - Added `evaluate(size: f32)` method that converts from `Val::Percent` to `Val::Px`. - Added `try_add_with_size` and `try_sub_with_size` methods to `Val`, which evaluate `Val::Percent` values into `Val::Px` values before adding. --- Instead of using the + and - operators, perform calculations on `Val`s using the new `try_add` and `try_sub` methods. Multiplication and division remained unchanged. Also, when adding or subtracting from `Size`, ~~use a `Val` tuple instead of `Vec2`~~ perform the addition on `width` and `height` separately. Co-authored-by: Dawid Piotrowski <41804418+Pietrek14@users.noreply.github.com>

# Objective Adds a better interface for performing mathematical operations with UI unit `Val`. Fixes bevyengine#6080. ## Solution - Added `try_add` and `try_sub` methods to Val. - Removed the `Add` and `AddAssign` impls for `Val` that introduced unintuitive and bug-prone behaviour. - As a consequence of the prior, ~~changed the `Add` and `Sub` impls for the `Size` struct to take a `(Val, Val)` instead of `Vec2`~~ deleted the `Add` and `Sub` impls for the `Size` struct - Added a `From<(Val, Val)>` impl for the `Size` struct - Added `evaluate(size: f32)` method that converts from `Val::Percent` to `Val::Px`. - Added `try_add_with_size` and `try_sub_with_size` methods to `Val`, which evaluate `Val::Percent` values into `Val::Px` values before adding. --- ## Migration Guide Instead of using the + and - operators, perform calculations on `Val`s using the new `try_add` and `try_sub` methods. Multiplication and division remained unchanged. Also, when adding or subtracting from `Size`, ~~use a `Val` tuple instead of `Vec2`~~ perform the addition on `width` and `height` separately. Co-authored-by: Dawid Piotrowski <41804418+Pietrek14@users.noreply.github.com>

Pietrek14 and others added 10 commits September 27, 2022 19:12

made resource_scope work for non-send resources

71056a3

Merge branch 'bevyengine:main' into main

d607e6b

fixed the non-send resource scope tests

83f2417

formatting

6da0e41

simplified panic in a non-send resource scope test

3500039

Co-authored-by: Mike <mike.hsu@gmail.com>

Merge branch 'bevyengine:main' into main

d823b1b

changed the name of non-send struct used for testing

f68eb33

added arithmetic functions for Val

56a5dd7

removed adding and subtracting of val with f32

ad9da5f

Merge branch 'bevyengine:main' into val-arithmetic

fef5ead

tguichaoua suggested changes Oct 1, 2022

View reviewed changes

Pietrek14 added 2 commits October 1, 2022 21:50

changed the names of val methods to be more in line with the standard…

c9dfd36

… api and added add/sub assigning with size

formatting

cb577bc

alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Oct 3, 2022

alice-i-cecile self-requested a review October 3, 2022 17:29

Pietrek14 added 2 commits October 4, 2022 21:46

deleted warnings from Val::try_add and Val::try_sub

1fb6049

deleted unnecessary use

7b33282

Pietrek14 added 4 commits October 4, 2022 23:15

Val::evaluate returns f32

bd2240f

clippy

3c57795

added docs for Add and Subs impls for Size struct

e039468

formatting

69fa054

alice-i-cecile requested changes Oct 5, 2022

View reviewed changes

crates/bevy_ui/src/geometry.rs Outdated Show resolved Hide resolved

crates/bevy_ui/src/ui_node.rs Show resolved Hide resolved

crates/bevy_ui/src/ui_node.rs Outdated Show resolved Hide resolved

Pietrek14 added 5 commits October 5, 2022 23:18

replace add and sub impls for Size struct with from<(Val, Val)>

2fdb45c

ValArithmeticError derives Clone and Copy

da41750

added error messages to ValArithmeticError

769362c

made the docs for Val::try_add_assign_with_size not reference self wh…

495fef9

…ich is private

formatting

54ef797

alice-i-cecile approved these changes Oct 5, 2022

View reviewed changes

Pietrek14 requested a review from tguichaoua October 11, 2022 14:39

tguichaoua suggested changes Oct 12, 2022

View reviewed changes

crates/bevy_ui/src/ui_node.rs Show resolved Hide resolved

crates/bevy_ui/src/ui_node.rs Show resolved Hide resolved

doc comments for Val::try_add_assign and Val::try_sub_assign

a8e9ca9

tguichaoua approved these changes Oct 17, 2022

View reviewed changes

alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 17, 2022

alice-i-cecile approved these changes Oct 24, 2022

View reviewed changes

bors bot changed the title ~~Utility methods for Val~~ [Merged by Bors] - Utility methods for Val Oct 24, 2022

bors bot closed this Oct 24, 2022

ickk added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Oct 26, 2022

Weibye added a commit to Weibye/bevy that referenced this pull request Nov 6, 2022

Remove Add and Sub operators in according to bevyengine#6134

41d896e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Utility methods for Val #6134

[Merged by Bors] - Utility methods for Val #6134

Pietrek14 commented Sep 30, 2022 •

edited

Loading

alice-i-cecile commented Oct 4, 2022

Pietrek14 commented Oct 5, 2022

Pietrek14 commented Oct 8, 2022

alice-i-cecile commented Oct 8, 2022

tguichaoua left a comment

alice-i-cecile left a comment

bors bot commented Oct 24, 2022

bors bot commented Oct 24, 2022

Pietrek14 commented Oct 24, 2022

[Merged by Bors] - Utility methods for Val #6134

[Merged by Bors] - Utility methods for Val #6134

Conversation

Pietrek14 commented Sep 30, 2022 • edited Loading

Objective

Solution

Migration Guide

alice-i-cecile commented Oct 4, 2022

Pietrek14 commented Oct 5, 2022

Pietrek14 commented Oct 8, 2022

alice-i-cecile commented Oct 8, 2022

tguichaoua left a comment

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

bors bot commented Oct 24, 2022

bors bot commented Oct 24, 2022

Pietrek14 commented Oct 24, 2022

Pietrek14 commented Sep 30, 2022 •

edited

Loading