Add sensitivity

[100-TomatoJuice]

Closes #322

The design of the how to implement sensitivity was partly discussed in #322. However, I will paste my design comment for easy reading.

How to add the multiplier to inputs

My design would be something like this:

input_map: InputMap::default()
    .insert(DualAxis::mouse_motion().with_sensitivity(0.1), Action::Look)
    .insert(DualAxis::left_stick().with_sensitivity(1.5), Action::Look)

The value passed in for with_sensitivity() would be a multiplier, 1.0 would be the default.

Where to apply the multiplier

It might be possible to place it in input_value() in input_streams.rs. It would look something like this:

let value_in_axis_range = |axis: &SingleAxis, value: f32| -> f32 {
    if value >= axis.negative_low && value <= axis.positive_low && include_deadzone {
        0.0
    } else if axis.inverted {
        -value * axis.sensitivity
    } else {
        value * axis.sensitivity
    }
};

Every axis, as far as I know, uses value_in_axis_range(), so it might be as easy as just adding * axis.sensitivity. Also, from looking around a bit, every input seems to at some point call input_value(). So, calculating the sensitivity there will make sure all inputs will be modified if needed.

Where to store the data

I was thinking we could store the sensitivity value in SingleAxis as DualAxis is just two SingleAxis. Then, we could add constructor methods as shown above for both SingleAxis and DualAxis.

// `SingleAxis` verision
#[must_use]
pub fn with_sensitivity(self, sensitivity: f32) -> SingleAxis {
    self.sensitivity = sensitivity;
    self
}

// `DualAxis` verision
#[must_use]
pub fn with_sensitivity(self, y_sensitivity: f32, y_sensitivity: f32) -> SingleAxis {
    self.x.sensitivity = x_sensitivity;
    self.y.sensitivity = y_sensitivity;
    self
}

How to change the data

I think a way to change the sensitivity would be to query for InputMap and use get(). It does return &PetitSet<UserInput, 16> so that may be hard to work with as you have to index into the correct UserInput. I don't know if there can even be an easy way to do this. Do users have to look up to order they inserted the UserInput into InputMap to change the correct one? This is still an open question that I would love to hear about!

fn set_input_sensitivity(query: Query<&mut InputMap>) {
    let map = query.single();
    // Changing the mouse input sensitivity set in above example
    let input = map.get(Action::Look).get_at_mut(0).unwrap();
    input.sensitivity = 0.4;
}

Things left to do:

• Add docs
• Add a doc example
• Add tests
• Update RELEASE.md
• Update PartialEq and Hash for SingleAxis

[100-TomatoJuice]

Continuing from #322

A global resource would not allow that.

I don't think it would necessarily disallow a resource, as mouse motion is a DualAxis same with a controller stick. But, it would be a lot of work to track all these different inputs. A resource also creates the question of when to add to this resource? Making the users add to that even if that don't care about sensitivity seems a bit verbose. Maybe using a Query would be the best option? As, it allows for filtering to get the correct player. Whatever the end result for changing sensitivity, I would like to create an example to showcase how to do it.

[100-TomatoJuice]

Is there any way to tell the difference from, say, a controller stick and mouse motion DualAxis? I was thinking of using a HashMap with a InputKind as a key and a UserInput as a value, but I don't know if that would work, or even be any better than the current running implementation. The solution will need access to InputMap as that is what is needed. The question is how to cleanly extract the needed UserInput from map?

[100-TomatoJuice]

Changing the code

// This actually only gets the `UserInput` so there is quite a bit of matching needed to get to a Dual/Single Axis
let input = map.get(Action::Look).get_at_mut(0).unwrap();

with

let input = map.get_with_type(Action::Look, LeftStick)

seems really nice, but I can't think of a way to say "I want the left stick for this action" or "I want the mouse motion for this action". The closest to this is InputKind, but I don't know if it is enough. The line of thought here is a user would choose what action and what input to get the DualAxis, SingleAxis, or whatever input they want. This could also be used to change other things like the deadzone as well.

[100-TomatoJuice]

After a bit of testing, this is the current way to change input from an InputMap

fn change_input_values(mut query: Query<&mut InputMap<Action>, With<Player>>){
    let mut input_map = query.single_mut();
    
    // Added `.get_mut()` to `InputMap` to allow this
    let input = input_map.get_mut(Action::Look).get_at_mut(0).unwrap();
    if let UserInput::Single(kind) = input{
        if let InputKind::DualAxis(dual_axis) = kind{
            println!("X: {}, Y: {}, Shape: {:?}", dual_axis.x.sensitivity, dual_axis.y.sensitivity, dual_axis.deadzone);
            dual_axis.x.sensitivity = 0.8;
            dual_axis.y.sensitivity = 0.8;
            dual_axis.deadzone = DeadZoneShape::Rect { width: 1.0, height: 1.0 }
        }
    }
}

It is a little on the gross side, but I can't really think of a good way to select with input you want to modify. The first parameter is Actionlike, but the how to remove the get_at_mut(0) and the pattern matching is the next question. Passing in a UserInput like UserInput::Single(InputKind::DualAxis( **There is nothing to pass in here** )) won't work. The PetitSet in InputMap only holds a UserInput so indexing is the only way without changing a lot of the structure, I think.

[alice-i-cecile]

We can replace inverted with a negative sensitivity value.

[sixfold-origami]

We could do this mathematically, but I think it makes the API a lot less clear. I think we should keep them separate, and add some kind of non-negative constraint on sensitivity (or at least a warning).

[alice-i-cecile]

Okay, fair. Let's do that then.

[100-TomatoJuice]

Sorry, but what does "that" refer to? I'm guessing merging sensitivity and inverted as you said below, but what to do about making the API less clear, as plof27 said. I'm guessing just write useful docs on sensitivity to explain how inverting works now? I think this is addressed by leaving invert() functions for use, as it uses the same API as before. It's just inverted means reversing the sign instead of flipping a bool.

[100-TomatoJuice]

As a side note, instead of removing the invert functions, I can just invert the sensitivity to do the same thing.

/// Returns this [`SingleAxis`] inverted.
#[must_use]
pub fn inverted(mut self) -> Self {
    self.sensitivity = -self.sensitivity;
    self
}

This helps to keep the ergonomics of the invert bool without having it

[alice-i-cecile]

Ah, I like that :)

[100-TomatoJuice]

Ah, I like that :)

So, would you like that re-merged into one value? Or, close this as resolved and keep them separate. I am good with either solution.

[alice-i-cecile]

Alright, that's more straightforward than I expected. I'm not super concerned about being able to tweak these values later, the fact that it's possible to at all is about good enough for me.

We should collapse the inverted field into sensitivity, write up a change log and add some tests before merging and shipping this though.

[100-TomatoJuice]

Ok, I will get on that and all the changes presented :). Do you want a new example for how to change values at runtime, punt it to a new pr, or just not have an example? Also, do you want to include get_mut() for InputMap in this pr or make a follow-up and add that? The get_mut() is needed to change values at runtime.

[alice-i-cecile]

@plof27 convinced me: let's revert the change to merge inverted + sensitivity. It's technically correct, but a less clear API.

Add get_mut in this PR, and make a doc test demonstrating how to change the values :)

[100-TomatoJuice]

Should I add "some kind of non-negative constraint on sensitivity (or at least a warning)" as plof27 said? If so, where would that go, as there a many ways of creating an axis. Should I add just a doc on sensitivity that says negative values will invert it, so keep the values >=0.0? For this reason, I was for the merging of the two, but I understand how it can be a point of confusion. I did think having invert() functions was a good middle ground.

[alice-i-cecile]

Should I add "some kind of non-negative constraint on sensitivity (or at least a warning)" as plof27 said

I'm not fully sure where this should go. We could put it on every constructor / setter? But that's pretty duplicative, and would force us to fully lock down the construction.

We could also use a custom StrictlyPositivef32 type or something, but the ergonomics of working with those is very frustrating IME.

I'm tempted to just add it to the docs for now and get this merged and shipped.

[sixfold-origami]

Agreed