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

Fix mouse accumulation #543

Merged
merged 21 commits into from
Jun 18, 2024
Merged
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
6 changes: 2 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ egui = ['dep:bevy_egui']

[dependencies]
leafwing_input_manager_macros = { path = "macros", version = "0.13" }
bevy = { version = "0.14.0-rc.2", default-features = false, features = [
bevy = { version = "0.14.0-rc.3", default-features = false, features = [
"serialize",
] }
bevy_ecs = { version = "0.14.0-rc.2", features = ["serde"] }
bevy_egui = { git = "https://github.com/Friz64/bevy_egui", branch = "bevy-0.14", optional = true }

derive_more = { version = "0.99", default-features = false, features = [
Expand All @@ -55,8 +54,7 @@ dyn-hash = "0.2"
once_cell = "1.19"

[dev-dependencies]
bevy_ecs = { version = "0.14.0-rc.2", features = ["serde"] }
bevy = { version = "0.14.0-rc.2", default-features = false, features = [
bevy = { version = "0.14.0-rc.3", default-features = false, features = [
"bevy_asset",
"bevy_sprite",
"bevy_text",
Expand Down
1 change: 1 addition & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ Input processors allow you to create custom logic for axis-like input manipulati
- fixed a bug where enabling a pressed action would read as `just_pressed`, and disabling a pressed action would read as `just_released`.
- fixed a bug in `InputStreams::button_pressed()` where unrelated gamepads were not filtered out when an `associated_gamepad` is defined.
- inputs are now handled correctly in the `FixedUpdate` schedule! Previously, the `ActionState`s were only updated in the `PreUpdate` schedule, so you could have situations where an action was marked as `just_pressed` multiple times in a row (if the `FixedUpdate` schedule ran multiple times in a frame) or was missed entirely (if the `FixedUpdate` schedule ran 0 times in a frame).
- Mouse motion and mouse scroll are now computed more efficiently and reliably, through the use of the new `AccumulatedMouseMovement` and `AccumulatedMouseScroll` resources.

### Tech debt

Expand Down
5 changes: 3 additions & 2 deletions src/action_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,7 @@ mod tests {
use crate::input_map::InputMap;
use crate::input_mocking::MockInput;
use crate::input_streams::InputStreams;
use crate::plugin::AccumulatorPlugin;
use crate::prelude::InputChord;
use bevy::input::InputPlugin;
use bevy::prelude::*;
Expand All @@ -754,7 +755,7 @@ mod tests {
#[test]
fn press_lifecycle() {
let mut app = App::new();
app.add_plugins(InputPlugin);
app.add_plugins(InputPlugin).add_plugins(AccumulatorPlugin);

#[derive(Actionlike, Clone, Copy, PartialEq, Eq, Hash, Debug, bevy::prelude::Reflect)]
enum Action {
Expand Down Expand Up @@ -840,7 +841,7 @@ mod tests {
input_map.insert(Action::OneAndTwo, InputChord::new([Digit1, Digit2]));

let mut app = App::new();
app.add_plugins(InputPlugin);
app.add_plugins(InputPlugin).add_plugins(AccumulatorPlugin);

// Action state
let mut action_state = ActionState::<Action>::default();
Expand Down
2 changes: 1 addition & 1 deletion src/axislike.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ impl DualAxisData {
Dir2::new(self.xy).ok()
}

/// The [`Rotation`] (measured clockwise from midnight) that this axis is pointing towards, if any
/// The [`Rot2`] that this axis is pointing towards, if any.
///
/// If the axis is neutral (x,y) = (0,0), this will be `None`
#[must_use]
Expand Down
13 changes: 0 additions & 13 deletions src/errors.rs

This file was deleted.

22 changes: 13 additions & 9 deletions src/input_mocking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,13 @@ pub trait MockInput {
/// use bevy::prelude::*;
/// use bevy::input::InputPlugin;
/// use leafwing_input_manager::input_mocking::QueryInput;
/// use leafwing_input_manager::plugin::AccumulatorPlugin;
/// use leafwing_input_manager::prelude::*;
///
/// let mut app = App::new();
///
/// // This functionality requires Bevy's InputPlugin (included with DefaultPlugins)
/// app.add_plugins(InputPlugin);
/// app.add_plugins((InputPlugin, AccumulatorPlugin));
///
/// // Check if a key is currently pressed down.
/// let pressed = app.pressed(KeyCode::KeyB);
Expand Down Expand Up @@ -388,7 +389,7 @@ impl MockInput for MutableInputStreams<'_> {
*self.gamepad_axes = Default::default();
*self.keycodes = Default::default();
*self.mouse_buttons = Default::default();
*self.mouse_wheel = Default::default();
*self.mouse_scroll = Default::default();
*self.mouse_motion = Default::default();
}
}
Expand All @@ -412,17 +413,20 @@ impl MutableInputStreams<'_> {
}

fn send_mouse_scroll(&mut self, delta: Vec2) {
self.mouse_wheel.send(MouseWheel {
let event = MouseWheel {
unit: MouseScrollUnit::Pixel,
x: delta.x,
y: delta.y,
// FIXME: MouseScrollUnit is not recorded and is always assumed to be Pixel
unit: MouseScrollUnit::Pixel,
window: Entity::PLACEHOLDER,
});
};

self.mouse_scroll_events.send(event);
}

fn send_mouse_move(&mut self, delta: Vec2) {
self.mouse_motion.send(MouseMotion { delta });
let event = MouseMotion { delta };

self.mouse_motion_events.send(event);
}

fn send_gamepad_button_state(
Expand Down Expand Up @@ -686,6 +690,7 @@ impl MockUIInteraction for App {
#[cfg(test)]
mod test {
use crate::input_mocking::{MockInput, QueryInput};
use crate::plugin::AccumulatorPlugin;
use crate::user_input::*;
use bevy::input::gamepad::{
GamepadConnection, GamepadConnectionEvent, GamepadEvent, GamepadInfo,
Expand All @@ -695,7 +700,7 @@ mod test {

fn test_app() -> App {
let mut app = App::new();
app.add_plugins(InputPlugin);
app.add_plugins(InputPlugin).add_plugins(AccumulatorPlugin);

let gamepad = Gamepad::new(0);
let mut gamepad_events = app.world_mut().resource_mut::<Events<GamepadEvent>>();
Expand Down Expand Up @@ -786,7 +791,6 @@ mod test {
}

#[test]
#[ignore = "Mouse axis input clearing is buggy. Try again after https://github.com/bevyengine/bevy/pull/13762 is released."]
fn mouse_inputs() {
let mut app = test_app();

Expand Down
70 changes: 42 additions & 28 deletions src/input_streams.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
//! Unified input streams for working with [`bevy::input`] data.

use bevy::ecs::prelude::{Event, Events, ResMut, World};
use bevy::ecs::prelude::{Events, ResMut, World};
use bevy::ecs::system::SystemState;
use bevy::input::mouse::{MouseMotion, MouseWheel};
use bevy::input::{
gamepad::{Gamepad, GamepadAxis, GamepadButton, GamepadEvent, Gamepads},
keyboard::{KeyCode, KeyboardInput},
mouse::{MouseButton, MouseButtonInput, MouseMotion, MouseWheel},
mouse::{MouseButton, MouseButtonInput},
Axis, ButtonInput,
};

use crate::user_input::{AccumulatedMouseMovement, AccumulatedMouseScroll};

/// A collection of [`ButtonInput`] structs, which can be used to update an [`InputMap`](crate::input_map::InputMap).
///
/// These are typically collected via a system from the [`World`] as resources.
Expand All @@ -26,10 +29,10 @@ pub struct InputStreams<'a> {
pub keycodes: Option<&'a ButtonInput<KeyCode>>,
/// A [`MouseButton`] [`Input`](ButtonInput) stream
pub mouse_buttons: Option<&'a ButtonInput<MouseButton>>,
/// A [`MouseWheel`] event stream
pub mouse_wheel: Option<Vec<MouseWheel>>,
/// A [`MouseMotion`] event stream
pub mouse_motion: Vec<MouseMotion>,
/// The [`AccumulatedMouseScroll`] for the frame
pub mouse_scroll: &'a AccumulatedMouseScroll,
/// The [`AccumulatedMouseMovement`] for the frame
pub mouse_motion: &'a AccumulatedMouseMovement,
/// The [`Gamepad`] that this struct will detect inputs from
pub associated_gamepad: Option<Gamepad>,
}
Expand All @@ -44,11 +47,8 @@ impl<'a> InputStreams<'a> {
let gamepads = world.resource::<Gamepads>();
let keycodes = world.get_resource::<ButtonInput<KeyCode>>();
let mouse_buttons = world.get_resource::<ButtonInput<MouseButton>>();
let mouse_wheel = world.resource::<Events<MouseWheel>>();
let mouse_motion = world.resource::<Events<MouseMotion>>();

let mouse_wheel: Vec<MouseWheel> = collect_events_cloned(mouse_wheel);
let mouse_motion: Vec<MouseMotion> = collect_events_cloned(mouse_motion);
let mouse_wheel = world.resource::<AccumulatedMouseScroll>();
let mouse_motion = world.resource::<AccumulatedMouseMovement>();

InputStreams {
gamepad_buttons,
Expand All @@ -57,19 +57,13 @@ impl<'a> InputStreams<'a> {
gamepads,
keycodes,
mouse_buttons,
mouse_wheel: Some(mouse_wheel),
mouse_scroll: mouse_wheel,
mouse_motion,
associated_gamepad: gamepad,
}
}
}

// Clones and collects the received events into a `Vec`.
#[inline]
fn collect_events_cloned<T: Event + Clone>(events: &Events<T>) -> Vec<T> {
events.get_reader().read(events).cloned().collect()
}

/// A mutable collection of [`ButtonInput`] structs, which can be used for mocking user inputs.
///
/// These are typically collected via a system from the [`World`] as resources.
Expand All @@ -96,10 +90,14 @@ pub struct MutableInputStreams<'a> {
pub mouse_buttons: &'a mut ButtonInput<MouseButton>,
/// Events used for mocking [`MouseButton`] inputs
pub mouse_button_events: &'a mut Events<MouseButtonInput>,
/// A [`MouseWheel`] event stream
pub mouse_wheel: &'a mut Events<MouseWheel>,
/// A [`MouseMotion`] event stream
pub mouse_motion: &'a mut Events<MouseMotion>,
/// The [`AccumulatedMouseScroll`] for the frame
pub mouse_scroll: &'a mut AccumulatedMouseScroll,
/// The [`AccumulatedMouseMovement`] for the frame
pub mouse_motion: &'a mut AccumulatedMouseMovement,
/// Mouse scroll events used for mocking mouse scroll inputs
pub mouse_scroll_events: &'a mut Events<MouseWheel>,
/// Mouse motion events used for mocking mouse motion inputs
pub mouse_motion_events: &'a mut Events<MouseMotion>,

/// The [`Gamepad`] that this struct will detect inputs from
pub associated_gamepad: Option<Gamepad>,
Expand All @@ -108,6 +106,16 @@ pub struct MutableInputStreams<'a> {
impl<'a> MutableInputStreams<'a> {
/// Construct a [`MutableInputStreams`] from the [`World`]
pub fn from_world(world: &'a mut World, gamepad: Option<Gamepad>) -> Self {
// Initialize accumulated mouse movement and scroll resources if they don't exist
// These are special-cased because they are not initialized by the InputPlugin
if !world.contains_resource::<AccumulatedMouseMovement>() {
world.init_resource::<AccumulatedMouseMovement>();
}

if !world.contains_resource::<AccumulatedMouseScroll>() {
world.init_resource::<AccumulatedMouseScroll>();
}

let mut input_system_state: SystemState<(
ResMut<ButtonInput<GamepadButton>>,
ResMut<Axis<GamepadButton>>,
Expand All @@ -118,6 +126,8 @@ impl<'a> MutableInputStreams<'a> {
ResMut<Events<KeyboardInput>>,
ResMut<ButtonInput<MouseButton>>,
ResMut<Events<MouseButtonInput>>,
ResMut<AccumulatedMouseScroll>,
ResMut<AccumulatedMouseMovement>,
ResMut<Events<MouseWheel>>,
ResMut<Events<MouseMotion>>,
)> = SystemState::new(world);
Expand All @@ -132,8 +142,10 @@ impl<'a> MutableInputStreams<'a> {
keyboard_events,
mouse_buttons,
mouse_button_events,
mouse_wheel,
mouse_scroll,
mouse_motion,
mouse_scroll_events,
mouse_motion_events,
) = input_system_state.get_mut(world);

MutableInputStreams {
Expand All @@ -146,8 +158,10 @@ impl<'a> MutableInputStreams<'a> {
keyboard_events: keyboard_events.into_inner(),
mouse_buttons: mouse_buttons.into_inner(),
mouse_button_events: mouse_button_events.into_inner(),
mouse_wheel: mouse_wheel.into_inner(),
mouse_scroll: mouse_scroll.into_inner(),
mouse_motion: mouse_motion.into_inner(),
mouse_scroll_events: mouse_scroll_events.into_inner(),
mouse_motion_events: mouse_motion_events.into_inner(),
associated_gamepad: gamepad,
}
}
Expand All @@ -171,8 +185,8 @@ impl<'a> From<MutableInputStreams<'a>> for InputStreams<'a> {
gamepads: mutable_streams.gamepads,
keycodes: Some(mutable_streams.keycodes),
mouse_buttons: Some(mutable_streams.mouse_buttons),
mouse_wheel: Some(collect_events_cloned(mutable_streams.mouse_wheel)),
mouse_motion: collect_events_cloned(mutable_streams.mouse_motion),
mouse_scroll: mutable_streams.mouse_scroll,
mouse_motion: mutable_streams.mouse_motion,
associated_gamepad: mutable_streams.associated_gamepad,
}
}
Expand All @@ -187,8 +201,8 @@ impl<'a> From<&'a MutableInputStreams<'a>> for InputStreams<'a> {
gamepads: mutable_streams.gamepads,
keycodes: Some(mutable_streams.keycodes),
mouse_buttons: Some(mutable_streams.mouse_buttons),
mouse_wheel: Some(collect_events_cloned(mutable_streams.mouse_wheel)),
mouse_motion: collect_events_cloned(mutable_streams.mouse_motion),
mouse_scroll: mutable_streams.mouse_scroll,
mouse_motion: mutable_streams.mouse_motion,
associated_gamepad: mutable_streams.associated_gamepad,
}
}
Expand Down
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ pub mod axislike;
pub mod buttonlike;
pub mod clashing_inputs;
pub mod common_conditions;
pub mod errors;
pub mod input_map;
pub mod input_mocking;
pub mod input_processing;
Expand Down
41 changes: 40 additions & 1 deletion src/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::action_state::{ActionData, ActionState};
use crate::clashing_inputs::ClashStrategy;
use crate::input_map::InputMap;
use crate::input_processing::*;
use crate::systems::{accumulate_mouse_movement, accumulate_mouse_scroll};
#[cfg(feature = "timing")]
use crate::timing::Timing;
use crate::user_input::*;
Expand Down Expand Up @@ -95,6 +96,11 @@ impl<A: Actionlike + TypePath + bevy::reflect::GetTypeRegistration> Plugin

match self.machine {
Machine::Client => {
// TODO: this should be part of `bevy_input`
if !app.is_plugin_added::<AccumulatorPlugin>() {
app.add_plugins(AccumulatorPlugin);
}

// Main schedule
app.add_systems(
PreUpdate,
Expand Down Expand Up @@ -166,7 +172,9 @@ impl<A: Actionlike + TypePath + bevy::reflect::GetTypeRegistration> Plugin
}
};

app.register_type::<ActionState<A>>()
app.register_type::<AccumulatedMouseMovement>()
.register_type::<AccumulatedMouseScroll>()
.register_type::<ActionState<A>>()
.register_type::<InputMap<A>>()
.register_type::<ActionData>()
.register_type::<ActionState<A>>()
Expand Down Expand Up @@ -218,10 +226,41 @@ pub enum InputManagerSystem {
///
/// Cleans up the state of the input manager, clearing `just_pressed` and `just_released`
Tick,
/// Accumulates various input event streams into a total delta for the frame.
Accumulate,
/// Collects input data to update the [`ActionState`]
Update,
/// Manually control the [`ActionState`]
///
/// Must run after [`InputManagerSystem::Update`] or the action state will be overridden
ManualControl,
}

/// A plugin to handle accumulating mouse movement and scroll events.
///
/// This is a clearer, more reliable and more efficient approach to computing the total mouse movement and scroll for the frame.
///
/// This plugin is public to allow it to be used in tests: users should always have this plugin implicitly added by [`InputManagerPlugin`].
/// Ultimately, this should be included as part of [`InputPlugin`](bevy::input::InputPlugin): see [bevy#13915](https://github.com/bevyengine/bevy/issues/13915).
pub struct AccumulatorPlugin;

impl Plugin for AccumulatorPlugin {
fn build(&self, app: &mut App) {
app.init_resource::<AccumulatedMouseMovement>();
app.init_resource::<AccumulatedMouseScroll>();

// TODO: these should be part of bevy_input
app.add_systems(
PreUpdate,
(accumulate_mouse_movement, accumulate_mouse_scroll)
.in_set(InputManagerSystem::Accumulate),
);

app.configure_sets(
PreUpdate,
InputManagerSystem::Accumulate
.after(InputSystem)
.before(InputManagerSystem::Update),
);
}
}
Loading