Skip to content

Commit

Permalink
Fix axis settings constructor (#7233)
Browse files Browse the repository at this point in the history
# Objective

Currently, the `AxisSettings::new` function is unusable due to
an implementation quirk. It only allows `AxisSettings` where
the bounds that are supposed to be positive are negative!

## Solution

- We fix the bound check
- We add a test to make sure the method is usable


Seems like the error slipped through because of the relatively
verbose code style. With all those `if/else`, very long names,
range syntax, the bound check is actually hard to spot. I first
refactored a lot of code, but I left out the refactor because the
fix should be integrated independently.

---

## Changelog

- Fix `AxisSettings::new` only accepting invalid bounds
  • Loading branch information
nicopap committed Jan 16, 2023
1 parent 8302899 commit 2f4cf76
Showing 1 changed file with 18 additions and 7 deletions.
25 changes: 18 additions & 7 deletions crates/bevy_input/src/gamepad.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ impl ButtonSettings {
/// Otherwise, values will not be rounded.
///
/// The valid range is `[-1.0, 1.0]`.
#[derive(Debug, Clone, Reflect, FromReflect)]
#[derive(Debug, Clone, Reflect, FromReflect, PartialEq)]
#[reflect(Debug, Default)]
pub struct AxisSettings {
/// Values that are higher than `livezone_upperbound` will be rounded up to -1.0.
Expand Down Expand Up @@ -611,7 +611,7 @@ impl Default for AxisSettings {
}

impl AxisSettings {
/// Creates a new `AxisSettings` instance.
/// Creates a new [`AxisSettings`] instance.
///
/// # Arguments
///
Expand All @@ -622,9 +622,10 @@ impl AxisSettings {
/// + `threshold` - the minimum value by which input must change before the change is registered.
///
/// Restrictions:
/// + `-1.0 <= ``livezone_lowerbound`` <= ``deadzone_lowerbound`` <= 0.0 <= ``deadzone_upperbound`` <=
/// ``livezone_upperbound`` <= 1.0`
/// + `0.0 <= ``threshold`` <= 2.0`
///
/// + `-1.0 <= livezone_lowerbound <= deadzone_lowerbound <= 0.0`
/// + `0.0 <= deadzone_upperbound <= livezone_upperbound <= 1.0`
/// + `0.0 <= threshold <= 2.0`
///
/// # Errors
///
Expand All @@ -646,11 +647,11 @@ impl AxisSettings {
Err(AxisSettingsError::DeadZoneLowerBoundOutOfRange(
deadzone_lowerbound,
))
} else if !(-1.0..=0.0).contains(&deadzone_upperbound) {
} else if !(0.0..=1.0).contains(&deadzone_upperbound) {
Err(AxisSettingsError::DeadZoneUpperBoundOutOfRange(
deadzone_upperbound,
))
} else if !(-1.0..=0.0).contains(&livezone_upperbound) {
} else if !(0.0..=1.0).contains(&livezone_upperbound) {
Err(AxisSettingsError::LiveZoneUpperBoundOutOfRange(
livezone_upperbound,
))
Expand Down Expand Up @@ -1489,6 +1490,16 @@ mod tests {
#[test]
fn test_try_out_of_range_axis_settings() {
let mut axis_settings = AxisSettings::default();
assert_eq!(
AxisSettings::new(-0.95, -0.05, 0.05, 0.95, 0.001),
Ok(AxisSettings {
livezone_lowerbound: -0.95,
deadzone_lowerbound: -0.05,
deadzone_upperbound: 0.05,
livezone_upperbound: 0.95,
threshold: 0.001,
})
);
assert_eq!(
Err(AxisSettingsError::LiveZoneLowerBoundOutOfRange(-2.0)),
axis_settings.try_set_livezone_lowerbound(-2.0)
Expand Down

0 comments on commit 2f4cf76

Please sign in to comment.