Tighten hold buffer lifetime semantics

src/fighter/state/jump.rs

@@ -107,15 +107,11 @@ impl FighterStateImpl for JumpSquatState {
 
     fn on_enter(&mut self, state_context: &mut EnterContext) {
         state_context.play_animation(AnimKind::JumpSquat, false);
-        // Preserve buffered inputs while jump squat makes the transition
-        // temporarily unavailable, matching Ultimate/HDR-style hold input.
         state_context.input.enable_hold_all(10);
         state_context.input.clear_command(FighterCommands::Jump);
     }
 
     fn on_exit(&mut self, state_context: &mut crate::fighter::state::ExitContext) {
-        // The hold policy is state-scoped and must never leak into the next
-        // state. Command lifetime itself remains independent of this policy.
         state_context.input.disable_hold_all();
     }
 

src/input/buffer.rs

@@ -1,9 +1,4 @@
 //! Rollback-safe input history and command buffering.
-//!
-//! Raw mapped bindings, button edges, and command lifetimes are deliberately
-//! separate. This preserves combinations such as pressing a second shield
-//! binding while the first one is held, while still allowing a command to
-//! survive after its source button is released.
 
 use bevy::prelude::*;
 use bevy_ggrs::{PlayerInputs, Rollback};
@@ -101,8 +96,7 @@ impl FighterAttackCommandType {
 struct CommandSlot {
     lifetime: u8,
     age: u16,
-    hold_remaining: u16,
-    hold_latched: bool,
+    hold_remaining: HoldLimit,
     consumed: bool,
 }
 
@@ -111,24 +105,45 @@ impl Default for CommandSlot {
         Self {
             lifetime: 0,
             age: 0,
-            hold_remaining: 0,
-            hold_latched: false,
+            hold_remaining: HoldLimit::Disabled,
             consumed: false,
         }
     }
 }
 
-#[derive(Clone, Copy, Debug, Hash)]
-struct HoldLimit {
-    enabled: bool,
-    max_frames: u16,
+#[derive(Clone, Copy, Debug, Default, Hash, PartialEq, Eq)]
+enum HoldLimit {
+    #[default]
+    Disabled,
+    Frames(u16),
+    Infinite,
 }
 
-impl Default for HoldLimit {
-    fn default() -> Self {
-        Self {
-            enabled: false,
-            max_frames: 0,
+#[derive(Clone, Copy, Debug, Default, Hash, PartialEq, Eq)]
+enum HoldOverride {
+    #[default]
+    Inherit,
+    Disabled,
+    Frames(u16),
+    Infinite,
+}
+
+impl HoldOverride {
+    fn from_max_frames(max_frames: u16) -> Self {
+        if max_frames == 0 {
+            Self::Infinite
+        } else {
+            Self::Frames(max_frames)
+        }
+    }
+}
+
+impl HoldLimit {
+    fn from_max_frames(max_frames: u16) -> Self {
+        if max_frames == 0 {
+            Self::Infinite
+        } else {
+            Self::Frames(max_frames)
         }
     }
 }
@@ -181,7 +196,7 @@ impl ButtonHistory {
 #[require(Rollback)]
 pub struct FighterInput {
     command_slots: [CommandSlot; FighterCommands::COUNT],
-    hold_limits: [HoldLimit; FighterCommands::COUNT],
+    hold_limits: [HoldOverride; FighterCommands::COUNT],
     hold_all: HoldLimit,
     button_history: [ButtonHistory; 3],
     prev_frame: FighterInputFrame,
@@ -195,7 +210,7 @@ impl Default for FighterInput {
     fn default() -> Self {
         Self {
             command_slots: [CommandSlot::default(); FighterCommands::COUNT],
-            hold_limits: [HoldLimit::default(); FighterCommands::COUNT],
+            hold_limits: [HoldOverride::default(); FighterCommands::COUNT],
             hold_all: HoldLimit::default(),
             button_history: [ButtonHistory::default(); 3],
             prev_frame: FighterInputFrame::default(),
@@ -224,8 +239,7 @@ impl FighterInput {
     pub fn clear_command(&mut self, command: FighterCommands) {
         let slot = &mut self.command_slots[command.index()];
         slot.lifetime = 0;
-        slot.hold_remaining = 0;
-        slot.hold_latched = false;
+        slot.hold_remaining = HoldLimit::Disabled;
         slot.consumed = true;
     }
 
@@ -242,8 +256,7 @@ impl FighterInput {
         let slot = &mut self.command_slots[command.index()];
         slot.lifetime = lifetime;
         slot.age = 0;
-        slot.hold_remaining = 0;
-        slot.hold_latched = false;
+        slot.hold_remaining = HoldLimit::Disabled;
         slot.consumed = false;
         self.latch_if_hold_enabled(command);
     }
@@ -258,40 +271,64 @@ impl FighterInput {
             .saturating_sub(1);
     }
 
+    /// Holds all inputs for n frames
     pub fn enable_hold_all(&mut self, max_frames: u16) {
-        self.hold_all = HoldLimit {
-            enabled: true,
-            max_frames: if max_frames == 0 { u16::MAX } else { max_frames },
-        };
+        self.hold_all = HoldLimit::from_max_frames(max_frames);
         self.latch_active_commands();
     }
 
     pub fn disable_hold_all(&mut self) {
-        self.hold_all = HoldLimit::default();
+        self.hold_all = HoldLimit::Disabled;
+    }
+
+    /// Stops global enrollment and immediately cancels every latched command.
+    pub fn cancel_hold_all(&mut self) {
+        self.disable_hold_all();
+        for slot in &mut self.command_slots {
+            slot.hold_remaining = HoldLimit::Disabled;
+        }
     }
 
     pub fn enable_hold(&mut self, command: FighterCommands, max_frames: u16) {
-        self.hold_limits[command.index()] = HoldLimit {
-            enabled: true,
-            max_frames: if max_frames == 0 { u16::MAX } else { max_frames },
-        };
+        self.hold_limits[command.index()] = HoldOverride::from_max_frames(max_frames);
         if self.has_command(command) {
             self.latch_if_hold_enabled(command);
         }
     }
 
+    /// Explicitly prevents this command from using either command-specific or
+    /// global hold buffering, while preserving an existing latch.
     pub fn disable_hold(&mut self, command: FighterCommands) {
-        self.hold_limits[command.index()] = HoldLimit::default();
+        self.hold_limits[command.index()] = HoldOverride::Disabled;
+    }
+
+    /// Removes the command-specific policy so this command inherits the
+    /// global policy again.
+    pub fn reset_hold_override(&mut self, command: FighterCommands) {
+        self.hold_limits[command.index()] = HoldOverride::Inherit;
+    }
+
+    /// Stops enrollment and immediately cancels this command's hold lifetime.
+    pub fn cancel_hold(&mut self, command: FighterCommands) {
+        self.disable_hold(command);
+        self.command_slots[command.index()].hold_remaining = HoldLimit::Disabled;
     }
 
     fn latch_if_hold_enabled(&mut self, command: FighterCommands) {
         let limit = self.hold_limit(command);
-        if !limit.enabled {
+        let slot = &mut self.command_slots[command.index()];
+        slot.hold_remaining = match limit {
+            HoldLimit::Disabled => return,
+            HoldLimit::Infinite => HoldLimit::Infinite,
+            // The limit includes tap-buffer frames already elapsed.
+            HoldLimit::Frames(max_frames) => {
+                let remaining = max_frames.saturating_sub(slot.age);
+                if remaining == 0 {
                     return;
                 }
-        let slot = &mut self.command_slots[command.index()];
-        slot.hold_latched = true;
-        slot.hold_remaining = limit.max_frames;
+                HoldLimit::Frames(remaining)
+            }
+        };
     }
 
     fn latch_active_commands(&mut self) {
@@ -341,10 +378,11 @@ impl FighterInput {
 
     fn hold_limit(&self, command: FighterCommands) -> HoldLimit {
         let specific = self.hold_limits[command.index()];
-        if specific.enabled {
-            specific
-        } else {
-            self.hold_all
+        match specific {
+            HoldOverride::Inherit => self.hold_all,
+            HoldOverride::Disabled => HoldLimit::Disabled,
+            HoldOverride::Frames(frames) => HoldLimit::Frames(frames),
+            HoldOverride::Infinite => HoldLimit::Infinite,
         }
     }
 
@@ -352,15 +390,27 @@ impl FighterInput {
         let index = command.index();
         let slot = &mut self.command_slots[index];
         slot.age = slot.age.saturating_add(1);
-        if slot.hold_latched && slot.hold_remaining != 0 {
-            if slot.hold_remaining != u16::MAX {
-                slot.hold_remaining = slot.hold_remaining.saturating_sub(1);
-            }
+        match slot.hold_remaining {
+            HoldLimit::Disabled => {}
+            HoldLimit::Infinite => {
                 if slot.lifetime == 0 {
                     slot.lifetime = 1;
                 }
-        } else if slot.hold_latched {
-            slot.hold_latched = false;
+            }
+            HoldLimit::Frames(remaining) => {
+                // This tick consumes one of the total persistent frames, but
+                // the command remains active for the tick that consumes the
+                // final frame.
+                let remaining = remaining.saturating_sub(1);
+                slot.hold_remaining = if remaining == 0 {
+                    HoldLimit::Disabled
+                } else {
+                    HoldLimit::Frames(remaining)
+                };
+                if slot.lifetime == 0 {
+                    slot.lifetime = 1;
+                }
+            }
         }
     }
 }
@@ -496,16 +546,9 @@ pub fn postprocess_input(
             input.set_attack_command_type(attack_type);
         }
 
-        // Extend only commands whose source is still held. Recognition above
-        // has already reset age and cleared the consumed marker for new edges.
-        input.update_hold(FighterCommands::Jump);
-        input.update_hold(FighterCommands::Shield);
-        input.update_hold(FighterCommands::GuardHold);
-        input.update_hold(FighterCommands::Attack);
-        input.update_hold(FighterCommands::SmashMoveUp);
-        input.update_hold(FighterCommands::SmashMoveDown);
-        input.update_hold(FighterCommands::SmashMoveLeft);
-        input.update_hold(FighterCommands::SmashMoveRight);
+        for command in FighterCommands::iter() {
+            input.update_hold(command);
+        }
     }
 }
 
@@ -514,6 +557,11 @@ mod tests {
     use super::*;
     use crate::input::ActionBindingMask;
 
+    fn advance_command(input: &mut FighterInput, command: FighterCommands) {
+        input.reduce_command_lifetime(command);
+        input.update_hold(command);
+    }
+
     #[test]
     fn second_binding_is_an_edge_not_a_level() {
         let first = ActionBindingMask(0b0001);
@@ -556,19 +604,80 @@ mod tests {
     fn hold_buffer_keeps_released_attack_through_jump_squat() {
         let mut input = FighterInput::default();
         input.set_lifetime(FighterCommands::Attack, 3);
+        // Recognition happened before jump squat enabled persistence.
+        input.update_hold(FighterCommands::Attack);
         input.enable_hold_all(10);
 
-        // The command remains latched even though its physical source is no
-        // longer held. Five simulation ticks cover the test fighter's squat.
+        // Five simulation ticks cover the test fighter's jump squat. The
+        // physical attack binding is released, but persistence belongs to the
+        // recognized command rather than the binding's held state.
         for _ in 0..5 {
-            for command in FighterCommands::iter() {
-                input.reduce_command_lifetime(command);
-            }
-            input.update_hold(FighterCommands::Attack);
+            advance_command(&mut input, FighterCommands::Attack);
         }
 
         assert!(input.has_command(FighterCommands::Attack));
+        // Jump-squat exit stops enrollment. The latch crosses into Jump and
+        // remains visible on its first airborne interrupt frame.
         input.disable_hold_all();
+        advance_command(&mut input, FighterCommands::Attack);
+        assert!(input.has_command(FighterCommands::Attack));
+        assert!(input.consume_command(FighterCommands::Attack));
+        assert!(!input.has_command(FighterCommands::Attack));
+    }
+
+    #[test]
+    fn finite_hold_limit_includes_elapsed_tap_frames() {
+        let mut input = FighterInput::default();
+        input.set_lifetime(FighterCommands::Attack, 3);
+        // The recognition frame has already elapsed when jump squat starts.
+        input.update_hold(FighterCommands::Attack);
+        input.enable_hold_all(10);
+
+        for _ in 0..9 {
+            advance_command(&mut input, FighterCommands::Attack);
             assert!(input.has_command(FighterCommands::Attack));
         }
+        advance_command(&mut input, FighterCommands::Attack);
+        assert!(!input.has_command(FighterCommands::Attack));
+    }
+
+    #[test]
+    fn stopping_enrollment_and_cancelling_latches_are_distinct() {
+        let mut input = FighterInput::default();
+        input.enable_hold_all(10);
+        input.set_lifetime(FighterCommands::Shield, 1);
+        input.disable_hold_all();
+        advance_command(&mut input, FighterCommands::Shield);
+        assert!(input.has_command(FighterCommands::Shield));
+
+        input.cancel_hold_all();
+        advance_command(&mut input, FighterCommands::Shield);
+        assert!(!input.has_command(FighterCommands::Shield));
+    }
+
+    #[test]
+    fn command_override_can_disable_global_hold() {
+        let mut input = FighterInput::default();
+        input.enable_hold_all(10);
+        input.disable_hold(FighterCommands::Attack);
+        input.set_lifetime(FighterCommands::Attack, 1);
+        advance_command(&mut input, FighterCommands::Attack);
+        assert!(!input.has_command(FighterCommands::Attack));
+
+        input.reset_hold_override(FighterCommands::Attack);
+        input.set_lifetime(FighterCommands::Attack, 1);
+        advance_command(&mut input, FighterCommands::Attack);
+        assert!(input.has_command(FighterCommands::Attack));
+    }
+
+    #[test]
+    fn infinite_hold_uses_an_explicit_state() {
+        let mut input = FighterInput::default();
+        input.enable_hold(FighterCommands::Shield, 0);
+        input.set_lifetime(FighterCommands::Shield, 1);
+        for _ in 0..u16::MAX as usize + 1 {
+            advance_command(&mut input, FighterCommands::Shield);
+        }
+        assert!(input.has_command(FighterCommands::Shield));
+    }
 }