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

Jump to bottom

Add utility methods to Val #6080

Closed
alice-i-cecile opened this issue Sep 23, 2022 · 4 comments
Closed

Add utility methods to Val #6080

alice-i-cecile opened this issue Sep 23, 2022 · 4 comments
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

The Val type is an expressive way to represent UI scales across unit types.

However, working with these values can be quite frustrating; basic utility methods are missing.

What solution would you like?

  1. Add try_add and try_sub methods to Val, which only return Ok if the enum variants are identical.
  2. Remove the Add and AddAssign impls for Val.
  3. Add evaluate(size: f32) method that converts from Val::Percent to Val::Px.
  4. Add try_add_with_size and try_sub_with_size methods to Val, which evaluate Val::Percent values into Val::Px values before adding.

What alternative(s) have you considered?

We could implement the Add and Sub traits (and already implement the Add trait).

However, this results in some fairly tricky implicit behavior. For example, what happens when we add Val::Undefined to Val::Auto? This implicit introduction of NaN-like behavior can be very frustrating to debug.
@alice-i-cecile 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 Sep 23, 2022
@mockersf
Copy link
Member

previous try in #5525 / #5555

@Pietrek14
Copy link
Contributor

I'll try to implement this.

@Pietrek14
Copy link
Contributor

When adding or subtracting non-numeric variants, like Undefined, should it return the same variant or an error?

@alice-i-cecile
Copy link
Member Author

I think the same variant should be fine.

@Pietrek14 Pietrek14 mentioned this issue Sep 30, 2022
Closed
bors bot pushed a commit that referenced this issue Oct 24, 2022
@Pietrek14
Utility methods for Val (#6134)
9663115 
# 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 bors bot closed this as completed in 9066d51 Oct 24, 2022
james7132 pushed a commit to james7132/bevy that referenced this issue Oct 28, 2022
@Pietrek14 @james7132
Utility methods for Val (bevyengine#6134)
5678d0e 
# 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 added a commit to Pietrek14/bevy that referenced this issue Dec 17, 2022
@Pietrek14
Utility methods for Val (bevyengine#6134)
11a7157 
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>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
@Pietrek14 @ItsDoot
Utility methods for Val (bevyengine#6134)
ee3ba9e 
# 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Merged by Bors] - Utility methods for Val Pietrek14/bevy
3 participants
@alice-i-cecile @mockersf @Pietrek14