ADOPT ME: Use a HashMap inside of Action-mapping types

README.md

@@ -16,7 +16,7 @@ use leafwing_input_manager::prelude::*;
 
 // We're modelling https://leagueoflegends.fandom.com/wiki/Zyra/LoL
 // to show off this crate's features!
-#[derive(Actionlike, Abilitylike, Clone, Copy, PartialEq, Reflect)]
+#[derive(Actionlike, Abilitylike, Clone, Copy, PartialEq, Eq, Hash, Reflect)]
 pub enum ZyraAbility {
     GardenOfThorns,
     DeadlySpines,

RELEASES.md

@@ -1,5 +1,18 @@
 # Release Notes
 
+## Version 0.7 (unreleased)
+
+### Usability
+
+- `ChargeState`, `CooldownState` and `AbilityCosts` now store internal `HashMap`s
+  - The signatures of `trigger`, `get` and `get_mut` now match the standard signatures of map-like types
+    - Ability arguments are passed by reference
+    - Return values are `Option<&(mut) T>`
+    - `iter` and `iter_mut` now return key-value pairs
+  - As a result, these types no longer implements `PartialEq` and `Eq`
+  - `Abilitylike` now requires the `PartialEq`, `Eq` and `Hash` traits
+  - These changes are in support of [similar changes upstream in LWIM](https://github.com/Leafwing-Studios/leafwing-input-manager/pull/415)
+
 ## Version 0.6
 
 ### Dependencies

examples/cooldown.rs

@@ -30,7 +30,7 @@ fn main() {
         .run();
 }
 
-#[derive(Actionlike, Reflect, Abilitylike, Clone, Copy, PartialEq, Debug, Default)]
+#[derive(Actionlike, Reflect, Abilitylike, Clone, Copy, PartialEq, Debug, Default, Eq, Hash)]
 enum CookieAbility {
     #[default]
     AddOne,
@@ -135,7 +135,7 @@ fn handle_add_one_ability(
     if actions.just_pressed(CookieAbility::AddOne) {
         // Calling .trigger checks if the cooldown can be used, then triggers it if so
         // Note that this may miss other important limitations on when abilities can be used
-        if cooldowns.trigger(CookieAbility::AddOne).is_ok() {
+        if cooldowns.trigger(&CookieAbility::AddOne).is_ok() {
             // The result returned should be checked to decide how to respond
             score.0 += 1;
         }

src/ability_state.rs

@@ -181,7 +181,7 @@ mod tests {
     use bevy::{prelude::*, reflect::Reflect};
     use leafwing_input_manager::{action_state::ActionState, Actionlike};
 
-    #[derive(Actionlike, Reflect, Abilitylike, Clone, Debug)]
+    #[derive(Actionlike, Reflect, Abilitylike, Clone, Debug, PartialEq, Eq, Hash)]
     enum TestAction {
         Duck,
         Cover,

src/charges.rs

@@ -2,8 +2,10 @@
 //! Actions may only be used if at least one charge is available.
 //! Unlike pools, charges are not shared across abilities.
 
-use bevy::ecs::prelude::{Component, Resource};
-use std::marker::PhantomData;
+use bevy::{
+    ecs::prelude::{Component, Resource},
+    utils::HashMap,
+};
 
 use crate::{Abilitylike, CannotUseAbility};
 
@@ -17,7 +19,7 @@ use crate::{Abilitylike, CannotUseAbility};
 /// use leafwing_abilities::premade_pools::mana::{Mana, ManaPool};
 /// use leafwing_input_manager::Actionlike;
 ///
-/// #[derive(Actionlike, Abilitylike, Clone, Reflect)]
+/// #[derive(Actionlike, Abilitylike, Clone, Reflect, PartialEq, Eq, Hash)]
 /// enum Action {
 ///     // Neither cooldowns nor charges
 ///     Move,
@@ -82,15 +84,13 @@ use crate::{Abilitylike, CannotUseAbility};
 #[derive(Resource, Component, Clone, PartialEq, Eq, Debug)]
 pub struct ChargeState<A: Abilitylike> {
     /// The underlying [`Charges`], stored in [`Actionlike::variants`] order.
-    charges_vec: Vec<Option<Charges>>,
-    _phantom: PhantomData<A>,
+    charges_map: HashMap<A, Charges>,
 }
 
 impl<A: Abilitylike> Default for ChargeState<A> {
     fn default() -> Self {
         ChargeState {
-            charges_vec: A::variants().map(|_| None).collect(),
-            _phantom: PhantomData,
+            charges_map: HashMap::new(),
         }
     }
 }
@@ -177,7 +177,7 @@ impl<A: Abilitylike> ChargeState<A> {
     /// Returns `true` if the underlying [`Charges`] is [`None`].
     #[inline]
     #[must_use]
-    pub fn available(&self, action: A) -> bool {
+    pub fn available(&self, action: &A) -> bool {
         if let Some(charges) = self.get(action) {
             charges.available()
         } else {
@@ -192,7 +192,7 @@ impl<A: Abilitylike> ChargeState<A> {
     ///
     /// Returns `true` if the underlying [`Charges`] is [`None`].
     #[inline]
-    pub fn expend(&mut self, action: A) -> Result<(), CannotUseAbility> {
+    pub fn expend(&mut self, action: &A) -> Result<(), CannotUseAbility> {
         if let Some(charges) = self.get_mut(action) {
             charges.expend()
         } else {
@@ -205,7 +205,7 @@ impl<A: Abilitylike> ChargeState<A> {
     /// The exact effect is determined by the [`Charges`]'s [`ReplenishStrategy`].
     /// If the `action` is not associated with a [`Charges`], this has no effect.
     #[inline]
-    pub fn replenish(&mut self, action: A) {
+    pub fn replenish(&mut self, action: &A) {
         if let Some(charges) = self.get_mut(action) {
             charges.replenish();
         }
@@ -214,24 +214,23 @@ impl<A: Abilitylike> ChargeState<A> {
     /// Returns a reference to the underlying [`Charges`] for `action`, if set.
     #[inline]
     #[must_use]
-    pub fn get(&self, action: A) -> &Option<Charges> {
-        &self.charges_vec[action.index()]
+    pub fn get(&self, action: &A) -> Option<&Charges> {
+        self.charges_map.get(action)
     }
 
     /// Returns a mutable reference to the underlying [`Charges`] for `action`, if set.
     #[inline]
     #[must_use]
-    pub fn get_mut(&mut self, action: A) -> &mut Option<Charges> {
-        &mut self.charges_vec[action.index()]
+    pub fn get_mut(&mut self, action: &A) -> Option<&mut Charges> {
+        self.charges_map.get_mut(action)
     }
 
     /// Sets the underlying [`Charges`] for `action` to the provided value.
     ///
     /// Unless you're building a new [`ChargeState`] struct, you likely want to use [`Self::get_mut`].
     #[inline]
     pub fn set(&mut self, action: A, charges: Charges) -> &mut Self {
-        let data = self.get_mut(action);
-        *data = Some(charges);
+        self.charges_map.insert(action, charges);
 
         self
     }
@@ -247,14 +246,14 @@ impl<A: Abilitylike> ChargeState<A> {
 
     /// Returns an iterator of references to the underlying non-[`None`] [`Charges`]
     #[inline]
-    pub fn iter(&self) -> impl Iterator<Item = &Charges> {
-        self.charges_vec.iter().flatten()
+    pub fn iter(&self) -> impl Iterator<Item = (&A, &Charges)> {
+        self.charges_map.iter()
     }
 
     /// Returns an iterator of mutable references to the underlying non-[`None`] [`Charges`]
     #[inline]
-    pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut Charges> {
-        self.charges_vec.iter_mut().flatten()
+    pub fn iter_mut(&mut self) -> impl Iterator<Item = (&A, &mut Charges)> {
+        self.charges_map.iter_mut()
     }
 }
 

src/cooldown.rs

@@ -5,8 +5,11 @@ use crate::{
     Abilitylike, CannotUseAbility,
 };
 
-use bevy::ecs::prelude::{Component, Resource};
 use bevy::utils::Duration;
+use bevy::{
+    ecs::prelude::{Component, Resource},
+    utils::HashMap,
+};
 use serde::{Deserialize, Serialize};
 use std::marker::PhantomData;
 
@@ -28,7 +31,7 @@ use std::marker::PhantomData;
 /// use leafwing_abilities::prelude::*;
 /// use leafwing_input_manager::prelude::*;
 ///
-/// #[derive(Actionlike, Abilitylike, Clone, Copy, PartialEq, Eq, Debug, Reflect)]
+/// #[derive(Actionlike, Abilitylike, Clone, Reflect, PartialEq, Eq, Hash)]
 /// enum Action {
 ///     Run,
 ///     Jump,
@@ -40,22 +43,20 @@ use std::marker::PhantomData;
 /// action_state.press(Action::Jump);
 ///
 /// // This will only perform a limited check; consider using the `Abilitylike::ready` method instead
-/// if action_state.just_pressed(Action::Jump) && cooldowns.ready(Action::Jump).is_ok() {
+/// if action_state.just_pressed(Action::Jump) && cooldowns.ready(&Action::Jump).is_ok() {
 ///    // Actually do the jumping thing here
 ///    // Remember to actually begin the cooldown if you jumped!
-///    cooldowns.trigger(Action::Jump);
+///    cooldowns.trigger(&Action::Jump);
 /// }
 ///
 /// // We just jumped, so the cooldown isn't ready yet
-/// assert_eq!(cooldowns.ready(Action::Jump), Err(CannotUseAbility::OnCooldown));
+/// assert_eq!(cooldowns.ready(&Action::Jump), Err(CannotUseAbility::OnCooldown));
 /// ```
-#[derive(Resource, Component, Debug, Clone, PartialEq, Eq)]
+#[derive(Resource, Component, Debug, Clone)]
 pub struct CooldownState<A: Abilitylike> {
     /// The [`Cooldown`] of each action
-    ///
-    /// The position in this vector corresponds to [`Actionlike::index`].
-    /// If [`None`], the action can always be used
-    cooldown_vec: Vec<Option<Cooldown>>,
+    /// If an entry is missing, the action can always be used
+    cooldown_map: HashMap<A, Cooldown>,
     /// A shared cooldown between all actions of type `A`.
     ///
     /// No action of type `A` will be ready unless this is ready.
@@ -69,7 +70,7 @@ impl<A: Abilitylike> Default for CooldownState<A> {
     /// By default, cooldowns are not set.
     fn default() -> Self {
         CooldownState {
-            cooldown_vec: A::variants().map(|_| None).collect(),
+            cooldown_map: HashMap::new(),
             global_cooldown: None,
             _phantom: PhantomData,
         }
@@ -90,7 +91,7 @@ impl<A: Abilitylike> CooldownState<A> {
     /// use leafwing_abilities::Abilitylike;
     /// use leafwing_input_manager::Actionlike;
     ///
-    /// #[derive(Actionlike, Abilitylike, Clone, Copy, PartialEq, Eq, Debug, Reflect)]
+    /// #[derive(Actionlike, Abilitylike, Clone, Reflect, PartialEq, Eq, Hash)]
     /// enum Action {
     ///     Run,
     ///     Jump,
@@ -119,7 +120,7 @@ impl<A: Abilitylike> CooldownState<A> {
     /// or this can be used on its own,
     /// reading the returned [`Result`] to determine if the ability was used.
     #[inline]
-    pub fn trigger(&mut self, action: A) -> Result<(), CannotUseAbility> {
+    pub fn trigger(&mut self, action: &A) -> Result<(), CannotUseAbility> {
         if let Some(cooldown) = self.get_mut(action) {
             cooldown.trigger()?;
         }
@@ -136,7 +137,7 @@ impl<A: Abilitylike> CooldownState<A> {
     /// This will be `Ok` if the underlying [`Cooldown::ready`] call is true,
     /// or if no cooldown is stored for this action.
     #[inline]
-    pub fn ready(&self, action: A) -> Result<(), CannotUseAbility> {
+    pub fn ready(&self, action: &A) -> Result<(), CannotUseAbility> {
         self.gcd_ready()?;
 
         if let Some(cooldown) = self.get(action) {
@@ -163,46 +164,48 @@ impl<A: Abilitylike> CooldownState<A> {
     /// When you have a [`Option<Mut<ActionCharges<A>>>`](bevy::ecs::change_detection::Mut),
     /// use `charges.map(|res| res.into_inner())` to convert it to the correct form.
     pub fn tick(&mut self, delta_time: Duration, maybe_charges: Option<&mut ChargeState<A>>) {
+        let action_list: Vec<A> = self.cooldown_map.keys().cloned().collect();
+
         if let Some(charge_state) = maybe_charges {
-            for action in A::variants() {
-                if let Some(ref mut cooldown) = self.get_mut(action.clone()) {
-                    let charges = charge_state.get_mut(action.clone());
+            for action in &action_list {
+                if let Some(ref mut cooldown) = self.get_mut(action) {
+                    let charges = charge_state.get_mut(action);
                     cooldown.tick(delta_time, charges);
                 }
             }
         } else {
-            for action in A::variants() {
-                if let Some(ref mut cooldown) = self.get_mut(action.clone()) {
-                    cooldown.tick(delta_time, &mut None);
+            for action in &action_list {
+                if let Some(ref mut cooldown) = self.get_mut(action) {
+                    cooldown.tick(delta_time, None);
                 }
             }
         }
 
         if let Some(global_cooldown) = self.global_cooldown.as_mut() {
-            global_cooldown.tick(delta_time, &mut None);
+            global_cooldown.tick(delta_time, None);
         }
     }
 
     /// The cooldown associated with the specified `action`, if any.
     #[inline]
     #[must_use]
-    pub fn get(&self, action: A) -> &Option<Cooldown> {
-        &self.cooldown_vec[action.index()]
+    pub fn get(&self, action: &A) -> Option<&Cooldown> {
+        self.cooldown_map.get(action)
     }
 
     /// A mutable reference to the cooldown associated with the specified `action`, if any.
     #[inline]
     #[must_use]
-    pub fn get_mut(&mut self, action: A) -> &mut Option<Cooldown> {
-        &mut self.cooldown_vec[action.index()]
+    pub fn get_mut(&mut self, action: &A) -> Option<&mut Cooldown> {
+        self.cooldown_map.get_mut(action)
     }
 
     /// Set a cooldown for the specified `action`.
     ///
     /// If a cooldown already existed, it will be replaced by a new cooldown with the specified duration.
     #[inline]
     pub fn set(&mut self, action: A, cooldown: Cooldown) -> &mut Self {
-        *self.get_mut(action) = Some(cooldown);
+        self.cooldown_map.insert(action, cooldown);
         self
     }
 
@@ -217,14 +220,14 @@ impl<A: Abilitylike> CooldownState<A> {
 
     /// Returns an iterator of references to the underlying non-[`None`] [`Cooldown`]s
     #[inline]
-    pub fn iter(&self) -> impl Iterator<Item = &Cooldown> {
-        self.cooldown_vec.iter().flatten()
+    pub fn iter(&self) -> impl Iterator<Item = (&A, &Cooldown)> {
+        self.cooldown_map.iter()
     }
 
     /// Returns an iterator of mutable references to the underlying non-[`None`] [`Cooldown`]s
     #[inline]
-    pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut Cooldown> {
-        self.cooldown_vec.iter_mut().flatten()
+    pub fn iter_mut(&mut self) -> impl Iterator<Item = (&A, &mut Cooldown)> {
+        self.cooldown_map.iter_mut()
     }
 }
 
@@ -246,10 +249,10 @@ impl<A: Abilitylike> CooldownState<A> {
 /// cooldown.trigger();
 /// assert_eq!(cooldown.remaining(), Duration::from_secs(3));
 ///
-/// cooldown.tick(Duration::from_secs(1), &mut None);
+/// cooldown.tick(Duration::from_secs(1), None);
 /// assert_eq!(cooldown.ready(), Err(CannotUseAbility::OnCooldown));
 ///
-/// cooldown.tick(Duration::from_secs(5), &mut None);
+/// cooldown.tick(Duration::from_secs(5), None);
 /// let triggered = cooldown.trigger();
 /// assert!(triggered.is_ok());
 ///
@@ -295,7 +298,7 @@ impl Cooldown {
     /// Advance the cooldown by `delta_time`.
     ///
     /// If the elapsed time is enough to reset the cooldown, the number of available charges.
-    pub fn tick(&mut self, delta_time: Duration, charges: &mut Option<Charges>) {
+    pub fn tick(&mut self, delta_time: Duration, charges: Option<&mut Charges>) {
         // Don't tick cooldowns when they are fully elapsed
         if self.elapsed_time == self.max_time {
             return;
@@ -429,7 +432,7 @@ mod tick_tests {
     fn tick_has_no_effect_on_fresh_cooldown() {
         let cooldown = Cooldown::from_secs(1.);
         let mut cloned_cooldown = cooldown.clone();
-        cloned_cooldown.tick(Duration::from_secs_f32(1.234), &mut None);
+        cloned_cooldown.tick(Duration::from_secs_f32(1.234), None);
         assert_eq!(cooldown, cloned_cooldown);
     }
 
@@ -456,7 +459,7 @@ mod tick_tests {
         let _ = cloned_cooldown.trigger();
         assert!(cooldown != cloned_cooldown);
 
-        cloned_cooldown.tick(Duration::from_millis(123), &mut None);
+        cloned_cooldown.tick(Duration::from_millis(123), None);
         assert!(cooldown != cloned_cooldown);
     }
 
@@ -466,7 +469,7 @@ mod tick_tests {
         let _ = cooldown.trigger();
         assert_eq!(cooldown.ready(), Err(CannotUseAbility::OnCooldown));
 
-        cooldown.tick(Duration::from_secs(3), &mut None);
+        cooldown.tick(Duration::from_secs(3), None);
         assert!(cooldown.ready().is_ok());
     }