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.

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

Make UserInput generic over the maximum number of chord inputs #241

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@
### Usability

- Implemented `Eq` for `Timing` and `InputMap`.
- `UserInput` is now generic over the maximum size of inputs for a `Chord` (the default is still `8`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we should probably lower the default to something much smaller, like 4, now that it can be configured, to save space for the 99% of use cases that don't want more.

- If an iterator with too many inputs is passed to `UserInput::chord`, the rest of the inputs are now ignored instead of panicing.
- If you want to react to this case, the new `UserInput::try_make_chord` function returns a `Result` instead.

### Breaking changes

- `UserInput::chord` does not panic anymore if you provide an iterator with more inputs than the maximum allowed number. Instead, the rest of the elements are silently ignored.

## Version 0.5

Expand Down
55 changes: 48 additions & 7 deletions src/user_input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,41 @@ use crate::{
buttonlike::{MouseMotionDirection, MouseWheelDirection},
};

/// An [`UserInput::Chord`] could not be created because there were too many input elements
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub struct ChordCreationError;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably return the passed in iterator to make error recovery more feasible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that does make sense, but at the point of error the iterator will already be consumed for the most part, so I'm not sure how useful it will be for recovery


/// Some combination of user input, which may cross [`Input`]-mode boundaries
///
/// Suitable for use in an [`InputMap`](crate::input_map::InputMap)
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub enum UserInput {
pub enum UserInput<const CHORD_MAX_SIZE: usize = 8> {
/// A single button
Single(InputKind),
/// A combination of buttons, pressed simultaneously
///
/// Up to 8 (!!) buttons can be chorded together at once.
/// The number of buttons you can chord together is configurable via the `CHORD_MAX_SIZE` generic (8 by default).
/// Chords are considered to belong to all of the [InputMode]s of their constituent buttons.
Chord(PetitSet<InputKind, 8>),
Chord(PetitSet<InputKind, CHORD_MAX_SIZE>),
/// A virtual DPad that you can get an [`AxisPair`] from
VirtualDPad(VirtualDPad),
}

impl UserInput {
impl<const CHORD_MAX_SIZE: usize> UserInput<CHORD_MAX_SIZE> {
/// Creates a [`UserInput::Chord`] from an iterator of [`InputKind`]s
///
/// If `inputs` has a length of 1, a [`UserInput::Single`] variant will be returned instead.
///
/// If `inputs` contains more than `CHORD_MAX_SIZE` elements, the rest of the elements are ignored.
/// If you want to react to this case with a [`Result`], use [`UserInput::try_make_chord`] instead.
pub fn chord(inputs: impl IntoIterator<Item = impl Into<InputKind>>) -> Self {
// We can't just check the length unless we add an ExactSizeIterator bound :(
let mut length: u8 = 0;
let mut length: usize = 0;

let mut set: PetitSet<InputKind, CHORD_MAX_SIZE> = PetitSet::default();

let mut set: PetitSet<InputKind, 8> = PetitSet::default();
for button in inputs {
// If the iterator contains more than the maximum number of elements, ignore the rest
for button in inputs.into_iter().take(CHORD_MAX_SIZE) {
length += 1;
set.insert(button.into());
}
Expand All @@ -47,6 +56,38 @@ impl UserInput {
}
}

/// Creates a [`UserInput::Chord`] from an iterator of [`InputKind`]s
///
/// If `inputs` has a length of 1, a [`UserInput::Single`] variant will be returned instead.
///
/// If `inputs` contains more than `CHORD_MAX_SIZE` elements, a [`ChordCreationError`] is returned.
/// If you want to silently ignore the rest of the elements, use the [`UserInput::chord`] function instead.
pub fn try_make_chord(
Copy link
Contributor

@alice-i-cecile alice-i-cecile Aug 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can use something super generic here? Like new or try_create or something?

This feels like it should be the default method for configurable keybindings.

inputs: impl IntoIterator<Item = impl Into<InputKind>>,
) -> Result<Self, ChordCreationError> {
// We can't just check the length unless we add an ExactSizeIterator bound :(
let mut length: usize = 0;

let mut set: PetitSet<InputKind, CHORD_MAX_SIZE> = PetitSet::default();

// If the iterator contains more than the maximum number of elements, ignore the rest
for button in inputs.into_iter().take(CHORD_MAX_SIZE) {
length += 1;

if length >= CHORD_MAX_SIZE {
return Err(ChordCreationError);
}
set.insert(button.into());
}

let user_input = match length {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matching behavior is inconsistent with the behavior in the infallible version. It's also surprising, as the user has explicitly tried to make a chord.

Might still be correct, but if so we'll want a different name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it inconsistent? I copied this logic from the infallible version

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. Never mind!

1 => UserInput::Single(set.into_iter().next().unwrap()),
_ => UserInput::Chord(set),
};

Ok(user_input)
}

/// The number of logical inputs that make up the [`UserInput`].
///
/// - A [`Single`][UserInput::Single] input returns 1
Expand Down