Add rumble support to bevy_gilrs #3868

nicopap · 2022-02-05T10:02:40Z

This adds the RumbleRequest event and a system to read them and rumble
controllers accordingly.

It gives users two ways of controlling controller rumble:

A very primitive API with RumbleIntensity that is easy to
understand and use.
A direct access to the girls ff::Effect system for complete
fine-grained control over how the gamepad rumbles.

MinerSebas · 2022-02-05T11:00:22Z

crates/bevy_gilrs/src/rumble.rs

+            Medium => ff::BaseEffectType::Strong { magnitude: 40_000 },
+            Weak => ff::BaseEffectType::Weak { magnitude: 40_000 },


This magnitudes shouldn't be the same.

In the gilrs example They use the same magnitude with a different BaseEffectType (you'll notice here we use Strong for Medium and Weak for Weak) to demonstrate a periodic change in force feedback intensity. Testing it myself with two different controllers, I find all different RumbleIntensity to be distinct enough.

Didn't notice the different Types here. Then everything is fine here

This adds the `RumbleRequest` event and a system to read them and rumble controllers accordingly. It gives users two ways of controlling controller rumble: 1. A very primitive API with `RumbleIntensity` that is easy to understand and use. 2. A direct access to the girls `ff::Effect` system for complete fine-grained control over how the gamepad rumbles.

nicopap · 2022-02-05T13:19:01Z

Notes:

Re-export of `gilrs::ff`

This implementation re-exports gilrs::ff so that user can create their own rumbling patterns (as demonstrated in the examples) however, we could be more selective in what we re-export. Would it be better this way?

Or is it not even OK to re-export and should instead create our own wrapper types. I really don't want to do that, because the EffectBuilder API of gilrs seems very expensive, and would be difficult to replicate.

Error handling in `gilrs_rumble_system`

I don't like how that function look (so much error handling diluting the actual logic, yuk!) But what's the alternative here? I thought that:

Define a RumbleError enum
implement From<ff::Error> for RumbleError
Extract the inner body of the for loop into it's own funciton that returns Result<(), RumbleError>
match the function call

This separates the error handling from core logic handling, but it's much more code. What's the best choice?

nicopap · 2022-02-11T08:29:57Z

There is still an open question: how do we want this to play out with mobile rumble? (and by extension, web)

It might or might not be wishable to have a unified interface to interact with mobile rumble. Having a unified API means the gilrs force feedback system needs to be encapsulated and not exposed to the end user.

alice-i-cecile · 2022-05-16T19:26:51Z

crates/bevy_gilrs/src/lib.rs

@@ -20,10 +23,13 @@ impl Plugin for GilrsPlugin {
        {
            Ok(gilrs) => {
                app.insert_non_send_resource(gilrs)
+                    .add_event::<RumbleRequest>()
+                    .init_non_send_resource::<rumble::RumblesManager>()


Suggested change

.init_non_send_resource::<rumble::RumblesManager>()

.init_non_send_resource::<RumblesManager>()

This should just be imported.

alice-i-cecile · 2022-05-16T19:27:00Z

crates/bevy_gilrs/src/lib.rs

                    .add_startup_system_to_stage(
                        StartupStage::PreStartup,
                        gilrs_event_startup_system,
                    )
+                    .add_system_to_stage(CoreStage::PostUpdate, rumble::gilrs_rumble_system)


Suggested change

.add_system_to_stage(CoreStage::PostUpdate, rumble::gilrs_rumble_system)

.add_system_to_stage(CoreStage::PostUpdate, gilrs_rumble_system)

alice-i-cecile · 2022-05-16T19:27:38Z

crates/bevy_gilrs/src/lib.rs

 use gilrs::GilrsBuilder;
 use gilrs_system::{gilrs_event_startup_system, gilrs_event_system};
+pub use rumble::{RumbleIntensity, RumbleRequest};


Suggested change

pub use rumble::{RumbleIntensity, RumbleRequest};

pub use rumble::{gilrs_rumble_system, RumbleIntensity, RumblesManager, RumbleRequest};

alice-i-cecile · 2022-05-16T19:28:58Z

crates/bevy_gilrs/src/rumble.rs

+    fn effect_type(&self) -> ff::BaseEffectType {
+        use RumbleIntensity::*;
+        match self {
+            Strong => ff::BaseEffectType::Strong { magnitude: 63_000 },


These values seem super arbitrary and hard-coded.

alice-i-cecile · 2022-05-16T19:29:23Z

crates/bevy_gilrs/src/rumble.rs

+    }
+}
+
+/// Request `pad` rumble in `gilrs_effect` pattern for `duration_seconds`


The phrasing here is a little unclear.

alice-i-cecile · 2022-05-16T19:30:22Z

crates/bevy_gilrs/src/rumble.rs

+/// # Notes
+///
+/// * Does nothing if `pad` does not support rumble
+/// * If a new `RumbleRequest` is sent while another one is still executing, it


I'm not sure if this is always the correct behavior. Could we have an overwriting field perhaps?

alice-i-cecile · 2022-05-16T19:31:42Z

crates/bevy_gilrs/src/rumble.rs

+    }
+}
+
+struct RunningRumble {


This needs docs; I'm not immediately sure what it does.

alice-i-cecile · 2022-05-16T19:32:42Z

crates/bevy_gilrs/src/rumble.rs

+        .insert(pad_id, RunningRumble { deadline, effect });
+    Ok(())
+}
+pub(crate) fn gilrs_rumble_system(


Suggested change

pub(crate) fn gilrs_rumble_system(

pub(crate) fn play_gilrs_rumble(

alice-i-cecile

I really want this functionality and this API is a great start.

However, I feel pretty strongly that we should be wrapping this. I want this to work across platforms: vibration is a critical feature for web / console / mobile / VR, and gilrs isn't going to cover those.

alice-i-cecile · 2022-05-16T19:32:57Z

crates/bevy_gilrs/src/rumble.rs

+        .insert(pad_id, RunningRumble { deadline, effect });
+    Ok(())
+}
+pub(crate) fn gilrs_rumble_system(


I'm not sure if this should be pub?

alice-i-cecile · 2022-05-16T19:33:31Z

crates/bevy_gilrs/src/rumble.rs

+                log::debug!("Tried to rumble {pad:?} but an error occurred: {err}")
+            }
+            Err(RumbleError::GamepadNotFound) => {
+                log::error!("Tried to rumble {pad:?} but it doesn't exist!")


Suggested change

log::error!("Tried to rumble {pad:?} but it doesn't exist!")

log::warn!("Tried to rumble {pad:?} but it doesn't exist!")

alice-i-cecile · 2022-05-16T19:34:18Z

examples/README.md

@@ -194,6 +194,7 @@ Example | File | Description
 `char_input_events` | [`input/char_input_events.rs`](./input/char_input_events.rs) | Prints out all chars as they are inputted.
 `gamepad_input` | [`input/gamepad_input.rs`](./input/gamepad_input.rs) | Shows handling of gamepad input, connections, and disconnections
 `gamepad_input_events` | [`input/gamepad_input_events.rs`](./input/gamepad_input_events.rs) | Iterates and prints gamepad input and connection events
+`gamepad_rumble` | [`input/gamepad_rumble.rs`](./input/gamepad_rumble.rs) | Make a controller rumble


Suggested change

`gamepad_rumble` | [`input/gamepad_rumble.rs`](./input/gamepad_rumble.rs) | Make a controller rumble

`gamepad_rumble` | [`input/gamepad_rumble.rs`](./input/gamepad_rumble.rs) | Makes a controller vibrate for force-feedback purposes

Keyword stuffing!

alice-i-cecile · 2022-05-16T19:35:05Z

examples/input/gamepad_rumble.rs

@@ -0,0 +1,70 @@
+use bevy::{


This needs a module-style doc comment explaining what the example does.

johanhelsing · 2023-04-16T06:10:27Z

Hey, I'm considering picking this one up. Anything I should know about @nicopap @alice-i-cecile ?

nicopap · 2023-04-16T07:17:36Z

@johanhelsing nope. I think everything relevant should be in the diff and comments of this PR.

johanhelsing · 2023-04-16T08:14:13Z

I merged main into this branch and got it working again. My branch is here https://github.com/bevyengine/bevy/compare/main...johanhelsing:bevy:rumble?expand=1 I'll create a PR when/if I have something I want feedback on, but that branch is probably a good starting point if I end up abandoning it and someone else wants to pick this up :)

# Objective Provide the ability to trigger controller rumbling (force-feedback) with a cross-platform API. ## Solution This adds the `GamepadRumbleRequest` event to `bevy_input` and adds a system in `bevy_gilrs` to read them and rumble controllers accordingly. It's a relatively primitive API with a `duration` in seconds and `GamepadRumbleIntensity` with values for the weak and strong gamepad motors. It's is an almost 1-to-1 mapping to platform APIs. Some platforms refer to these motors as left and right, and low frequency and high frequency, but by convention, they're usually the same. I used #3868 as a starting point, updated to main, removed the low-level gilrs effect API, and moved the requests to `bevy_input` and exposed the strong and weak intensities. I intend this to hopefully be a non-controversial cross-platform starting point we can build upon to eventually support more fine-grained control (closer to the gilrs effect API) --- ## Changelog ### Added - Gamepads can now be rumbled by sending the `GamepadRumbleRequest` event. --------- Co-authored-by: Nicola Papale <nico@nicopap.ch> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com> Co-authored-by: Bruce Reif (Buswolley) <bruce.reif@dynata.com>

github-actions bot added the S-Needs-Triage This issue needs to be labelled label Feb 5, 2022

MinerSebas reviewed Feb 5, 2022

View reviewed changes

nicopap force-pushed the main branch from 06209f1 to 0707bdc Compare February 5, 2022 13:05

Rework error handling in rumble system

8bfbfda

mockersf added A-Input Player input via keyboard, mouse, gamepad, and more C-Enhancement A new feature and removed S-Needs-Triage This issue needs to be labelled labels Feb 6, 2022

alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label May 16, 2022

alice-i-cecile reviewed May 16, 2022

View reviewed changes

crates/bevy_gilrs/src/rumble.rs

}

}

struct RunningRumble {

Copy link

Member

alice-i-cecile May 16, 2022

Choose a reason for hiding this comment

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

This needs docs; I'm not immediately sure what it does.

alice-i-cecile reviewed May 16, 2022

View reviewed changes

alice-i-cecile requested changes May 16, 2022

View reviewed changes

nicopap marked this pull request as draft June 18, 2022 12:15

nicopap closed this Sep 1, 2022

alice-i-cecile added the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label Sep 1, 2022

johanhelsing mentioned this pull request Apr 16, 2023

Add gamepad rumble support to bevy_input #8398

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rumble support to bevy_gilrs #3868

Add rumble support to bevy_gilrs #3868

nicopap commented Feb 5, 2022

MinerSebas Feb 5, 2022

nicopap Feb 5, 2022

MinerSebas Feb 5, 2022

nicopap commented Feb 5, 2022

nicopap commented Feb 11, 2022

alice-i-cecile May 16, 2022

alice-i-cecile May 16, 2022

alice-i-cecile May 16, 2022

alice-i-cecile May 16, 2022

alice-i-cecile May 16, 2022

alice-i-cecile May 16, 2022

alice-i-cecile May 16, 2022

alice-i-cecile May 16, 2022

alice-i-cecile left a comment

alice-i-cecile May 16, 2022

alice-i-cecile May 16, 2022

alice-i-cecile May 16, 2022

alice-i-cecile May 16, 2022

johanhelsing commented Apr 16, 2023

nicopap commented Apr 16, 2023

johanhelsing commented Apr 16, 2023

		Medium => ff::BaseEffectType::Strong { magnitude: 40_000 },
		Weak => ff::BaseEffectType::Weak { magnitude: 40_000 },

	.init_non_send_resource::<rumble::RumblesManager>()
	.init_non_send_resource::<RumblesManager>()

	.add_system_to_stage(CoreStage::PostUpdate, rumble::gilrs_rumble_system)
	.add_system_to_stage(CoreStage::PostUpdate, gilrs_rumble_system)

	pub use rumble::{RumbleIntensity, RumbleRequest};
	pub use rumble::{gilrs_rumble_system, RumbleIntensity, RumblesManager, RumbleRequest};

	pub(crate) fn gilrs_rumble_system(
	pub(crate) fn play_gilrs_rumble(

	log::error!("Tried to rumble {pad:?} but it doesn't exist!")
	log::warn!("Tried to rumble {pad:?} but it doesn't exist!")

	`gamepad_rumble` \| [`input/gamepad_rumble.rs`](./input/gamepad_rumble.rs) \| Make a controller rumble
	`gamepad_rumble` \| [`input/gamepad_rumble.rs`](./input/gamepad_rumble.rs) \| Makes a controller vibrate for force-feedback purposes

Add rumble support to bevy_gilrs #3868

Add rumble support to bevy_gilrs #3868

Conversation

nicopap commented Feb 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicopap commented Feb 5, 2022

Re-export of gilrs::ff

Error handling in gilrs_rumble_system

nicopap commented Feb 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanhelsing commented Apr 16, 2023

nicopap commented Apr 16, 2023

johanhelsing commented Apr 16, 2023

Re-export of `gilrs::ff`

Error handling in `gilrs_rumble_system`