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

docs: Documentation and clean up of bevy_input #3692

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion crates/bevy_gilrs/src/converter.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use bevy_input::gamepad::{Gamepad, GamepadAxisType, GamepadButtonType};

pub fn convert_gamepad_id(gamepad_id: gilrs::GamepadId) -> Gamepad {
Gamepad(gamepad_id.into())
Gamepad::new(gamepad_id.into())
}

pub fn convert_button(button: gilrs::Button) -> Option<GamepadButtonType> {
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_gilrs/src/gilrs_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use gilrs::{EventType, Gilrs};

pub fn gilrs_event_startup_system(gilrs: NonSend<Gilrs>, mut events: EventWriter<GamepadEventRaw>) {
for (id, _) in gilrs.gamepads() {
events.send(GamepadEventRaw(
events.send(GamepadEventRaw::new(
Copy link
Member

Choose a reason for hiding this comment

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

On review, I think this is nicer than the tuple structs.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if I like it better than a named struct though.

Copy link
Author

Choose a reason for hiding this comment

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

In my eyes tuple structs should be avoided in most cases, because they are a pain to work with. I'm fine with both the named struct and the new() constructor. Would probably be a good idea to have a place where we note things like this so we can keep things consistent.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we have an engine style guide: https://github.com/bevyengine/bevy/blob/main/.github/contributing/engine_style_guide.md

@cart, do you have strong feelings?

convert_gamepad_id(id),
GamepadEventType::Connected,
));
Expand All @@ -17,28 +17,28 @@ pub fn gilrs_event_system(mut gilrs: NonSendMut<Gilrs>, mut events: EventWriter<
while let Some(gilrs_event) = gilrs.next_event() {
match gilrs_event.event {
EventType::Connected => {
events.send(GamepadEventRaw(
events.send(GamepadEventRaw::new(
convert_gamepad_id(gilrs_event.id),
GamepadEventType::Connected,
));
}
EventType::Disconnected => {
events.send(GamepadEventRaw(
events.send(GamepadEventRaw::new(
convert_gamepad_id(gilrs_event.id),
GamepadEventType::Disconnected,
));
}
EventType::ButtonChanged(gilrs_button, value, _) => {
if let Some(button_type) = convert_button(gilrs_button) {
events.send(GamepadEventRaw(
events.send(GamepadEventRaw::new(
convert_gamepad_id(gilrs_event.id),
GamepadEventType::ButtonChanged(button_type, value),
));
}
}
EventType::AxisChanged(gilrs_axis, value, _) => {
if let Some(axis_type) = convert_axis(gilrs_axis) {
events.send(GamepadEventRaw(
events.send(GamepadEventRaw::new(
convert_gamepad_id(gilrs_event.id),
GamepadEventType::AxisChanged(axis_type, value),
));
Expand Down
2 changes: 2 additions & 0 deletions crates/bevy_input/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@ bevy_utils = { path = "../bevy_utils", version = "0.6.0" }

# other
serde = { version = "1", features = ["derive"], optional = true }
strum = { version = "0.23" }
strum_macros = { version = "0.23" }
52 changes: 30 additions & 22 deletions crates/bevy_input/src/axis.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use bevy_utils::HashMap;
use std::hash::Hash;

/// Stores the position data of input devices of type T
/// Stores the position data of the input devices of type `T`.
///
/// Values are stored as `f32` values, which range from `min` to `max`.
/// The valid range is from -1.0 to 1.0, inclusive.
/// Values are stored as `f32` values, which range from -1.0 to 1.0, inclusive.
#[derive(Debug)]
pub struct Axis<T> {
/// The position data of the input devices.
axis_data: HashMap<T, f32>,
}

Expand All @@ -25,17 +25,13 @@ impl<T> Axis<T>
where
T: Copy + Eq + Hash,
{
pub const MIN: f32 = -1.0;
pub const MAX: f32 = 1.0;

/// Inserts a position data for an input device,
/// restricting the position data to an interval `min..=max`.
///
/// If the input device wasn't present before, [None] is returned.
/// Inserts a position data for an input device, restricting the position data to an interval `min..=max`.
///
/// If the input device was present, the position data is updated, and the old value is returned.
/// If the `input_device`:
/// - was present before, the position data is updated, and the old value is returned.
/// - wasn't present before, [None] is returned.
pub fn set(&mut self, input_device: T, position_data: f32) -> Option<f32> {
let new_position_data = position_data.clamp(Self::MIN, Self::MAX);
let new_position_data = position_data.clamp(-1.0, 1.0);
self.axis_data.insert(input_device, new_position_data)
}

Expand All @@ -44,8 +40,7 @@ where
self.axis_data.get(&input_device).copied()
}

/// Removes the position data of the input device,
/// returning the position data if the input device was previously set.
/// Removes the position data of the input device, returning the position data if the input device was previously set.
pub fn remove(&mut self, input_device: T) -> Option<f32> {
self.axis_data.remove(&input_device)
}
Expand All @@ -59,7 +54,7 @@ mod tests {
};

#[test]
fn test_axis_set() {
fn test_set() {
let cases = [
(-1.5, Some(-1.0)),
(-1.1, Some(-1.0)),
Expand All @@ -75,7 +70,8 @@ mod tests {
];

for (value, expected) in cases {
let gamepad_button = GamepadButton(Gamepad(1), GamepadButtonType::RightTrigger);
let gamepad_button =
GamepadButton::new(Gamepad::new(1), GamepadButtonType::RightTrigger);
let mut axis = Axis::<GamepadButton>::default();

axis.set(gamepad_button, value);
Expand All @@ -86,18 +82,30 @@ mod tests {
}

#[test]
fn test_axis_remove() {
fn test_get() {
let cases = [-1.0, -0.9, -0.1, 0.0, 0.1, 0.9, 1.0];

for value in cases {
let gamepad_button = GamepadButton(Gamepad(1), GamepadButtonType::RightTrigger);
let mut axis = Axis::<GamepadButton>::default();
let button = GamepadButton::new(Gamepad::new(1), GamepadButtonType::RightTrigger);
axis.axis_data.insert(button, value);
assert_eq!(axis.get(button).unwrap(), value);
}
}

axis.set(gamepad_button, value);
assert!(axis.get(gamepad_button).is_some());
#[test]
fn test_remove() {
let cases = [-1.0, -0.9, -0.1, 0.0, 0.1, 0.9, 1.0];

axis.remove(gamepad_button);
let actual = axis.get(gamepad_button);
for value in cases {
let button = GamepadButton::new(Gamepad::new(1), GamepadButtonType::RightTrigger);
let mut axis = Axis::<GamepadButton>::default();

axis.set(button, value);
assert!(axis.get(button).is_some());

axis.remove(button);
let actual = axis.get(button);
let expected = None;

assert_eq!(expected, actual);
Expand Down
Loading