Repository: Leafwing-Studios/leafwing-input-manager (rust)

Issue:
Axis values support for network diffs
We used the network diffing functionality of this library for our game jam game: https://prawl.itch.io/stellar-squeezebox which overall turned out great and it was really helpful for getting something off the ground quick!

Unfortunately, gamepad support was really lacking when diffing actions with a couple major issues:

A. There is no support for axis data for ActionDiff
B. ~~If you use an axis as a button (e.g. turn left) you can't set a deadzone or it stops being recognized on the server. Example:
  `SingleAxis::negative_only(GamepadAxisType::LeftStickX, 0.1).with_deadzone(deadzone)`~~ Was user error. See [comment](https://github.com/Leafwing-Studios/leafwing-input-manager/issues/342#issuecomment-1510452715)
  
I'd like to contribute towards this, so if you have any thoughts about how you want this to work let me know. (I'll probably taking a few days to relax now that the jam is over, so might be a bit)

Pull request description:
Add ValueChange and AxisPairChange variants to ActionDiff
In this PR I added an `ActionDiff::AxisChanged` variant. This is meant to allow for transmitting changes to axis input types across the network. I've updated the relevant systems to handle creating events for this variant on the client and consuming them on the server. I've been using this solution in my own projects for a little bit and it seems to be working but I'm new to rust in addition to the bevy ecosystem so would really love some feedback as to if this seems like a reasonable solution. There are three potentially breaking changes. First I had to remove `Eq` and `Hash` from ActionDiff as f32 doesn't implement them.  Second I had to add a local resource to the `generate_action_diffs` system to keep track of if the axis actually changed. Lastly I had to add a requirement that the `ID` type implement the `Hash` trait as it is being used a key for a hashmap.

Fixes #342

Patch:
diff --git a/RELEASES.md b/RELEASES.md
index 117d1f4..518f239 100644
--- a/RELEASES.md
+++ b/RELEASES.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- added support in `ActionDiff` for value and axis_pair changes
 - Added `InputMap::Clear`.
 
 ## Version 0.11.2
diff --git a/examples/send_actions_over_network.rs b/examples/send_actions_over_network.rs
index 283a101..8886946 100644
--- a/examples/send_actions_over_network.rs
+++ b/examples/send_actions_over_network.rs
@@ -24,7 +24,7 @@ enum FpsAction {
 }
 
 /// This identifier uniquely identifies entities across the network
-#[derive(Component, Clone, PartialEq, Eq, Debug)]
+#[derive(Component, Clone, PartialEq, Eq, Hash, Debug)]
 struct StableId(u64);
 
 fn main() {
diff --git a/src/action_state.rs b/src/action_state.rs
index 4c7077e..97d8ee3 100644
--- a/src/action_state.rs
+++ b/src/action_state.rs
@@ -4,6 +4,7 @@ use crate::Actionlike;
 use crate::{axislike::DualAxisData, buttonlike::ButtonState};
 
 use bevy::ecs::{component::Component, entity::Entity};
+use bevy::math::Vec2;
 use bevy::prelude::{Event, Resource};
 use bevy::reflect::Reflect;
 use bevy::utils::hashbrown::hash_set::Iter;
@@ -758,7 +759,7 @@ impl Timing {
 ///
 /// `ID` should be a component type that stores a unique stable identifier for the entity
 /// that stores the corresponding [`ActionState`].
-#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Event)]
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Event)]
 pub enum ActionDiff<A: Actionlike, ID: Eq + Clone + Component> {
     /// The action was pressed
     Pressed {
@@ -774,6 +775,24 @@ pub enum ActionDiff<A: Actionlike, ID: Eq + Clone + Component> {
         /// The stable identifier of the entity
         id: ID,
     },
+    /// The value of the action changed
+    ValueChanged {
+        /// The value of the action
+        action: A,
+        /// The stable identifier of the entity
+        id: ID,
+        /// The new value of the action
+        value: f32,
+    },
+    /// The axis pair of the action changed
+    AxisPairChanged {
+        /// The value of the action
+        action: A,
+        /// The stable identifier of the entity
+        id: ID,
+        /// The new value of the axis
+        axis_pair: Vec2,
+    },
 }
 
 #[cfg(test)]
diff --git a/src/systems.rs b/src/systems.rs
index 62622c9..ff12e8f 100644
--- a/src/systems.rs
+++ b/src/systems.rs
@@ -4,6 +4,7 @@
 use crate::action_state::ActionStateDriver;
 use crate::{
     action_state::{ActionDiff, ActionState},
+    axislike::DualAxisData,
     clashing_inputs::ClashStrategy,
     input_map::InputMap,
     input_streams::InputStreams,
@@ -12,8 +13,6 @@ use crate::{
     Actionlike,
 };
 
-use bevy::time::Time;
-use bevy::utils::Instant;
 use bevy::{ecs::prelude::*, prelude::ScanCode};
 use bevy::{
     input::{
@@ -22,8 +21,11 @@ use bevy::{
         mouse::{MouseButton, MouseMotion, MouseWheel},
         Axis, Input,
     },
-    time::Real,
+    math::Vec2,
+    time::{Real, Time},
+    utils::{HashMap, Instant},
 };
+use core::hash::Hash;
 
 #[cfg(feature = "ui")]
 use bevy::ui::Interaction;
@@ -187,23 +189,100 @@ pub fn update_action_state_from_interaction<A: Actionlike>(
 /// suitable to be sent across a network.
 ///
 /// This system is not part of the [`InputManagerPlugin`](crate::plugin::InputManagerPlugin) and must be added manually.
-pub fn generate_action_diffs<A: Actionlike, ID: Eq + Clone + Component>(
+pub fn generate_action_diffs<A: Actionlike, ID: Eq + Clone + Component + Hash>(
     action_state_query: Query<(&ActionState<A>, &ID)>,
     mut action_diffs: EventWriter<ActionDiff<A, ID>>,
+    mut previous_values: Local<HashMap<A, HashMap<ID, f32>>>,
+    mut previous_axis_pairs: Local<HashMap<A, HashMap<ID, Vec2>>>,
 ) {
     for (action_state, id) in action_state_query.iter() {
         for action in action_state.get_just_pressed() {
-            action_diffs.send(ActionDiff::Pressed {
-                action: action.clone(),
-                id: id.clone(),
-            });
+            match action_state.action_data(action.clone()).axis_pair {
+                Some(axis_pair) => {
+                    action_diffs.send(ActionDiff::AxisPairChanged {
+                        action: action.clone(),
+                        id: id.clone(),
+                        axis_pair: axis_pair.into(),
+                    });
+                    previous_axis_pairs
+                        .raw_entry_mut()
+                        .from_key(&action)
+                        .or_insert_with(|| (action.clone(), HashMap::default()))
+                        .1
+                        .insert(id.clone(), axis_pair.xy());
+                }
+                None => {
+                    let value = action_state.value(action.clone());
+                    action_diffs.send(if value == 1. {
+                        ActionDiff::Pressed {
+                            action: action.clone(),
+                            id: id.clone(),
+                        }
+                    } else {
+                        ActionDiff::ValueChanged {
+                            action: action.clone(),
+                            id: id.clone(),
+                            value,
+                        }
+                    });
+                    previous_values
+                        .raw_entry_mut()
+                        .from_key(&action)
+                        .or_insert_with(|| (action.clone(), HashMap::default()))
+                        .1
+                        .insert(id.clone(), value);
+                }
+            }
         }
+        for action in action_state.get_pressed() {
+            if action_state.just_pressed(action.clone()) {
+                continue;
+            }
+            match action_state.action_data(action.clone()).axis_pair {
+                Some(axis_pair) => {
+                    let previous_axis_pairs = previous_axis_pairs.get_mut(&action).unwrap();
+
+                    if let Some(previous_axis_pair) = previous_axis_pairs.get(&id.clone()) {
+                        if *previous_axis_pair == axis_pair.xy() {
+                            continue;
+                        }
+                    }
+                    action_diffs.send(ActionDiff::AxisPairChanged {
+                        action: action.clone(),
+                        id: id.clone(),
+                        axis_pair: axis_pair.into(),
+                    });
+                    previous_axis_pairs.insert(id.clone(), axis_pair.xy());
+                }
+                None => {
+                    let value = action_state.value(action.clone());
+                    let previous_values = previous_values.get_mut(&action).unwrap();
 
+                    if let Some(previous_value) = previous_values.get(&id.clone()) {
+                        if *previous_value == value {
+                            continue;
+                        }
+                    }
+                    action_diffs.send(ActionDiff::ValueChanged {
+                        action: action.clone(),
+                        id: id.clone(),
+                        value,
+                    });
+                    previous_values.insert(id.clone(), value);
+                }
+            }
+        }
         for action in action_state.get_just_released() {
             action_diffs.send(ActionDiff::Released {
                 action: action.clone(),
                 id: id.clone(),
             });
+            if let Some(previous_axes) = previous_axis_pairs.get_mut(&action) {
+                previous_axes.remove(&id.clone());
+            }
+            if let Some(previous_values) = previous_values.get_mut(&action) {
+                previous_values.remove(&id.clone());
+            }
         }
     }
 }
@@ -228,6 +307,7 @@ pub fn process_action_diffs<A: Actionlike, ID: Eq + Component + Clone>(
                 } => {
                     if event_id == id {
                         action_state.press(action.clone());
+                        action_state.action_data_mut(action.clone()).value = 1.;
                         continue;
                     }
                 }
@@ -237,6 +317,33 @@ pub fn process_action_diffs<A: Actionlike, ID: Eq + Component + Clone>(
                 } => {
                     if event_id == id {
                         action_state.release(action.clone());
+                        let action_data = action_state.action_data_mut(action.clone());
+                        action_data.value = 0.;
+                        action_data.axis_pair = None;
+                        continue;
+                    }
+                }
+                ActionDiff::ValueChanged {
+                    action,
+                    id: event_id,
+                    value,
+                } => {
+                    if event_id == id {
+                        action_state.press(action.clone());
+                        action_state.action_data_mut(action.clone()).value = *value;
+                        continue;
+                    }
+                }
+                ActionDiff::AxisPairChanged {
+                    action,
+                    id: event_id,
+                    axis_pair,
+                } => {
+                    if event_id == id {
+                        action_state.press(action.clone());
+                        let action_data = action_state.action_data_mut(action.clone());
+                        action_data.axis_pair = Some(DualAxisData::from_xy(*axis_pair));
+                        action_data.value = axis_pair.length();
                         continue;
                     }
                 }


Test patch:
diff --git a/tests/action_diffs.rs b/tests/action_diffs.rs
new file mode 100644
index 0000000..c65f456
--- /dev/null
+++ b/tests/action_diffs.rs
@@ -0,0 +1,395 @@
+use bevy::{input::InputPlugin, prelude::*};
+use leafwing_input_manager::{
+    action_state::ActionDiff,
+    axislike::DualAxisData,
+    prelude::*,
+    systems::{generate_action_diffs, process_action_diffs},
+};
+
+#[derive(Actionlike, Clone, Copy, Debug, Reflect, PartialEq, Eq, Hash)]
+enum Action {
+    PayTheBills,
+}
+
+#[derive(Clone, Copy, Component, Debug, Reflect, PartialEq, Eq, Hash)]
+struct BankAccountId(u32);
+
+#[derive(Default)]
+struct Counter(pub u8);
+
+fn spawn_da_bills(mut commands: Commands) {
+    commands.spawn((BankAccountId(1337), ActionState::<Action>::default()));
+}
+
+fn pay_da_bills(
+    mutation: impl Fn(Mut<ActionState<Action>>) -> (),
+) -> impl Fn(Query<&mut ActionState<Action>>, Local<Counter>) -> () {
+    move |mut action_state_query: Query<&mut ActionState<Action>>, mut counter: Local<Counter>| {
+        if let Ok(mut action_state) = action_state_query.get_single_mut() {
+            if !action_state.pressed(Action::PayTheBills) {
+                action_state.press(Action::PayTheBills);
+                mutation(action_state);
+            } else if counter.0 > 1 {
+                action_state.release(Action::PayTheBills);
+            }
+            counter.0 += 1;
+        }
+    }
+}
+
+fn create_app() -> App {
+    let mut app = App::new();
+    app.add_plugins((
+        MinimalPlugins,
+        InputPlugin,
+        InputManagerPlugin::<Action>::default(),
+    ))
+    .add_systems(Startup, spawn_da_bills)
+    .add_event::<ActionDiff<Action, BankAccountId>>();
+    app
+}
+
+fn get_events<E: Event>(app: &App) -> &Events<E> {
+    app.world.resource()
+}
+fn get_events_mut<E: Event>(app: &mut App) -> Mut<Events<E>> {
+    app.world.resource_mut()
+}
+
+fn send_action_diff(app: &mut App, action_diff: ActionDiff<Action, BankAccountId>) {
+    let mut action_diff_events = get_events_mut::<ActionDiff<Action, BankAccountId>>(app);
+    action_diff_events.send(action_diff);
+}
+
+fn assert_has_no_action_diffs(app: &mut App) {
+    let action_diff_events = get_events::<ActionDiff<Action, BankAccountId>>(app);
+    let action_diff_event_reader = &mut action_diff_events.get_reader();
+    match action_diff_event_reader.read(action_diff_events).next() {
+        Some(action_diff) => panic!(
+            "Expected no `ActionDiff` variants. Received: {:?}",
+            action_diff
+        ),
+        None => {}
+    }
+}
+
+fn assert_action_diff_created(
+    app: &mut App,
+    predicate: impl Fn(&ActionDiff<Action, BankAccountId>),
+) {
+    let mut action_diff_events = get_events_mut::<ActionDiff<Action, BankAccountId>>(app);
+    let action_diff_event_reader = &mut action_diff_events.get_reader();
+    assert!(action_diff_event_reader.len(action_diff_events.as_ref()) < 2);
+    match action_diff_event_reader
+        .read(action_diff_events.as_ref())
+        .next()
+    {
+        Some(action_diff) => predicate(action_diff),
+        None => panic!("Expected an `ActionDiff` variant. Received none."),
+    };
+    action_diff_events.clear();
+}
+
+fn assert_action_diff_received(app: &mut App, action_diff: ActionDiff<Action, BankAccountId>) {
+    let mut action_state_query = app.world.query::<&ActionState<Action>>();
+    let action_state = action_state_query.get_single(&app.world).unwrap();
+    match action_diff {
+        ActionDiff::Pressed { id: _, action } => {
+            assert!(action_state.pressed(action));
+            assert!(action_state.value(action) == 1.);
+        }
+        ActionDiff::Released { id: _, action } => {
+            assert!(action_state.released(action));
+            assert!(action_state.value(action) == 0.);
+            assert!(action_state.axis_pair(action).is_none());
+        }
+        ActionDiff::ValueChanged {
+            id: _,
+            action,
+            value,
+        } => {
+            assert!(action_state.pressed(action));
+            assert!(action_state.value(action) == value);
+        }
+        ActionDiff::AxisPairChanged {
+            id: _,
+            action,
+            axis_pair,
+        } => {
+            assert!(action_state.pressed(action));
+            match action_state.axis_pair(action) {
+                Some(axis_pair_data) => {
+                    assert!(axis_pair_data.xy() == axis_pair);
+                    assert!(action_state.value(action) == axis_pair_data.xy().length());
+                }
+                None => panic!("Expected an `AxisPair` variant. Received none."),
+            }
+        }
+    }
+}
+
+#[test]
+fn generate_binary_action_diffs() {
+    let mut app = create_app();
+    app.add_systems(
+        Update,
+        pay_da_bills(|mut action_state| {
+            action_state.action_data_mut(Action::PayTheBills).value = 1.;
+        }),
+    )
+    .add_systems(PostUpdate, generate_action_diffs::<Action, BankAccountId>);
+
+    app.update();
+    assert_action_diff_created(&mut app, |action_diff| match action_diff {
+        ActionDiff::Pressed { id, action } => {
+            assert!(*id == BankAccountId(1337));
+            assert!(*action == Action::PayTheBills);
+        }
+        ActionDiff::Released { action: _, id: _ } => {
+            panic!("Expected a `Pressed` variant got a `Released` variant")
+        }
+        ActionDiff::ValueChanged {
+            action: _,
+            id: _,
+            value: _,
+        } => panic!("Expected a `Pressed` variant got a `ValueChanged` variant"),
+        ActionDiff::AxisPairChanged {
+            action: _,
+            id: _,
+            axis_pair: _,
+        } => panic!("Expected a `Pressed` variant got a `AxisPairChanged` variant"),
+    });
+
+    app.update();
+
+    assert_has_no_action_diffs(&mut app);
+
+    app.update();
+
+    assert_action_diff_created(&mut app, |action_diff| match action_diff {
+        ActionDiff::Released { id, action } => {
+            assert!(*id == BankAccountId(1337));
+            assert!(*action == Action::PayTheBills);
+        }
+        ActionDiff::Pressed { action: _, id: _ } => {
+            panic!("Expected a `Released` variant got a `Pressed` variant")
+        }
+        ActionDiff::ValueChanged {
+            action: _,
+            id: _,
+            value: _,
+        } => panic!("Expected a `Released` variant got a `ValueChanged` variant"),
+        ActionDiff::AxisPairChanged {
+            action: _,
+            id: _,
+            axis_pair: _,
+        } => panic!("Expected a `Released` variant got a `AxisPairChanged` variant"),
+    });
+}
+
+#[test]
+fn generate_value_action_diffs() {
+    let input_value = 0.5;
+    let mut app = create_app();
+    app.add_systems(
+        Update,
+        pay_da_bills(move |mut action_state| {
+            action_state.action_data_mut(Action::PayTheBills).value = input_value;
+        }),
+    )
+    .add_systems(PostUpdate, generate_action_diffs::<Action, BankAccountId>)
+    .add_event::<ActionDiff<Action, BankAccountId>>();
+
+    app.update();
+
+    assert_action_diff_created(&mut app, |action_diff| match action_diff {
+        ActionDiff::ValueChanged { id, action, value } => {
+            assert!(*id == BankAccountId(1337));
+            assert!(*action == Action::PayTheBills);
+            assert!(*value == input_value);
+        }
+        ActionDiff::Released { action: _, id: _ } => {
+            panic!("Expected a `ValueChanged` variant got a `Released` variant")
+        }
+        ActionDiff::Pressed { action: _, id: _ } => {
+            panic!("Expected a `ValueChanged` variant got a `Pressed` variant")
+        }
+        ActionDiff::AxisPairChanged {
+            action: _,
+            id: _,
+            axis_pair: _,
+        } => panic!("Expected a `ValueChanged` variant got a `AxisPairChanged` variant"),
+    });
+
+    app.update();
+
+    assert_has_no_action_diffs(&mut app);
+
+    app.update();
+
+    assert_action_diff_created(&mut app, |action_diff| match action_diff {
+        ActionDiff::Released { id, action } => {
+            assert!(*id == BankAccountId(1337));
+            assert!(*action == Action::PayTheBills);
+        }
+        ActionDiff::Pressed { action: _, id: _ } => {
+            panic!("Expected a `Released` variant got a `Pressed` variant")
+        }
+        ActionDiff::ValueChanged {
+            action: _,
+            id: _,
+            value: _,
+        } => panic!("Expected a `Released` variant got a `ValueChanged` variant"),
+        ActionDiff::AxisPairChanged {
+            action: _,
+            id: _,
+            axis_pair: _,
+        } => panic!("Expected a `Released` variant got a `AxisPairChanged` variant"),
+    });
+}
+
+#[test]
+fn generate_axis_action_diffs() {
+    let input_axis_pair = Vec2 { x: 5., y: 8. };
+    let mut app = create_app();
+    app.add_systems(
+        Update,
+        pay_da_bills(move |mut action_state| {
+            action_state.action_data_mut(Action::PayTheBills).axis_pair =
+                Some(DualAxisData::from_xy(input_axis_pair));
+        }),
+    )
+    .add_systems(PostUpdate, generate_action_diffs::<Action, BankAccountId>)
+    .add_event::<ActionDiff<Action, BankAccountId>>();
+
+    app.update();
+
+    assert_action_diff_created(&mut app, |action_diff| match action_diff {
+        ActionDiff::AxisPairChanged {
+            id,
+            action,
+            axis_pair,
+        } => {
+            assert!(*id == BankAccountId(1337));
+            assert!(*action == Action::PayTheBills);
+            assert!(*axis_pair == input_axis_pair);
+        }
+        ActionDiff::Released { action: _, id: _ } => {
+            panic!("Expected a `AxisPairChanged` variant got a `Released` variant")
+        }
+        ActionDiff::Pressed { action: _, id: _ } => {
+            panic!("Expected a `AxisPairChanged` variant got a `Pressed` variant")
+        }
+        ActionDiff::ValueChanged {
+            action: _,
+            id: _,
+            value: _,
+        } => panic!("Expected a `AxisPairChanged` variant got a `ValueChanged` variant"),
+    });
+
+    app.update();
+
+    assert_has_no_action_diffs(&mut app);
+
+    app.update();
+
+    assert_action_diff_created(&mut app, |action_diff| match action_diff {
+        ActionDiff::Released { id, action } => {
+            assert!(*id == BankAccountId(1337));
+            assert!(*action == Action::PayTheBills);
+        }
+        ActionDiff::Pressed { action: _, id: _ } => {
+            panic!("Expected a `Released` variant got a `Pressed` variant")
+        }
+        ActionDiff::ValueChanged {
+            action: _,
+            id: _,
+            value: _,
+        } => panic!("Expected a `Released` variant got a `ValueChanged` variant"),
+        ActionDiff::AxisPairChanged {
+            action: _,
+            id: _,
+            axis_pair: _,
+        } => panic!("Expected a `Released` variant got a `AxisPairChanged` variant"),
+    });
+}
+
+#[test]
+fn process_binary_action_diffs() {
+    let mut app = create_app();
+    app.add_systems(PreUpdate, process_action_diffs::<Action, BankAccountId>);
+
+    let action_diff = ActionDiff::Pressed {
+        id: BankAccountId(1337),
+        action: Action::PayTheBills,
+    };
+    send_action_diff(&mut app, action_diff.clone());
+
+    app.update();
+
+    assert_action_diff_received(&mut app, action_diff);
+
+    let action_diff = ActionDiff::Released {
+        id: BankAccountId(1337),
+        action: Action::PayTheBills,
+    };
+    send_action_diff(&mut app, action_diff.clone());
+
+    app.update();
+
+    assert_action_diff_received(&mut app, action_diff);
+}
+
+#[test]
+fn process_value_action_diff() {
+    let mut app = create_app();
+    app.add_systems(PreUpdate, process_action_diffs::<Action, BankAccountId>);
+
+    let action_diff = ActionDiff::ValueChanged {
+        id: BankAccountId(1337),
+        action: Action::PayTheBills,
+        value: 0.5,
+    };
+    send_action_diff(&mut app, action_diff.clone());
+
+    app.update();
+
+    assert_action_diff_received(&mut app, action_diff);
+
+    let action_diff = ActionDiff::Released {
+        id: BankAccountId(1337),
+        action: Action::PayTheBills,
+    };
+    send_action_diff(&mut app, action_diff.clone());
+
+    app.update();
+
+    assert_action_diff_received(&mut app, action_diff);
+}
+
+#[test]
+fn process_axis_action_diff() {
+    let mut app = create_app();
+    app.add_systems(PreUpdate, process_action_diffs::<Action, BankAccountId>);
+
+    let action_diff = ActionDiff::AxisPairChanged {
+        id: BankAccountId(1337),
+        action: Action::PayTheBills,
+        axis_pair: Vec2 { x: 1., y: 0. },
+    };
+    send_action_diff(&mut app, action_diff.clone());
+
+    app.update();
+
+    assert_action_diff_received(&mut app, action_diff);
+
+    let action_diff = ActionDiff::Released {
+        id: BankAccountId(1337),
+        action: Action::PayTheBills,
+    };
+    send_action_diff(&mut app, action_diff.clone());
+
+    app.update();
+
+    assert_action_diff_received(&mut app, action_diff);
+}