fix(host/input): rumble comes back when a controller does
android / android (pull_request) Failing after 18s
ci / docs-site (pull_request) Successful in 1m7s
ci / web (pull_request) Successful in 1m9s
apple / swift (pull_request) Successful in 1m15s
apple / screenshots (pull_request) Skipped
ci / rust-arm64 (pull_request) Successful in 1m45s
ci / rust (pull_request) Successful in 6m9s
android / android (pull_request) Failing after 18s
ci / docs-site (pull_request) Successful in 1m7s
ci / web (pull_request) Successful in 1m9s
apple / swift (pull_request) Successful in 1m15s
apple / screenshots (pull_request) Skipped
ci / rust-arm64 (pull_request) Successful in 1m45s
ci / rust (pull_request) Successful in 6m9s
Unplug a pad mid-session and plug it back in, and roughly half the time it never rumbles again for the rest of the session. The removal arm restarted the pad's rumble sequence counter. The client's reorder gate does not restart: `rumble_last_seq` lives for the whole QUIC connection and has no reset path, so it still holds whatever the pad reached before the unplug. Restarting the host counter therefore hands the client a seq it has already seen, and its wrapping half-space compare drops every envelope until the counter climbs back past the stored value — up to 128 sends. Since the counter only advances on a level change or a ~120 ms renewal while a level is non-zero, that spans many separate rumble events, so it reads as a flaky controller rather than a clean outage. Whether it bites is decided by how much the pad rumbled beforehand, which is why it looks intermittent: a pad that never rumbled before the re-plug has `None` on the client side and always heals. The counter now survives, matching the sibling pad-state gate — whose comment eleven lines above already explains that a re-plug must arrive with a still- newer seq to be accepted. The three clears that actually end the stale lease move into `clear_pad_feedback`, whose signature deliberately has no seq parameter so the arm cannot regress by editing. Covered by a regression test that drives the real wire encoder and the real client gate, and asserts the pre-fix behaviour is genuinely rejected across the whole forward window, so it cannot pass vacuously. Found by the 2026-08-03 force-feedback sweep (B1/T5 — see the backlog in punktfunk-planning design/haptics-sweep-2026-08-03.md).
This commit is contained in:
@@ -629,6 +629,20 @@ const RUMBLE_RENEW_FLOOR_MS: u64 = 60;
|
||||
/// own expiry. `3` total zero sends = the immediate one + this many renewal re-sends.
|
||||
const RUMBLE_STOP_BURST: u8 = 2;
|
||||
|
||||
/// Clear a removed pad's rumble bookkeeping — the level, the "we have seen a level" flag, and any
|
||||
/// stop re-sends still owed. Together these end the pad's lease, so a re-plug on the same wire
|
||||
/// index inherits nothing that could buzz the new device.
|
||||
///
|
||||
/// The per-pad rumble **sequence is deliberately not a parameter**: it must stay monotonic for the
|
||||
/// life of the connection because the client gates on it with a wrapping half-space compare and
|
||||
/// never resets its side (`punktfunk-core/src/client/pump/datagram_task.rs`). Resetting it here is
|
||||
/// the bug pinned by [`tests::rumble_seq_survives_a_removal_so_the_client_gate_accepts`].
|
||||
fn clear_pad_feedback(state: &mut (u16, u16), seen: &mut bool, stop_burst: &mut u8) {
|
||||
*state = (0, 0);
|
||||
*seen = false;
|
||||
*stop_burst = 0;
|
||||
}
|
||||
|
||||
/// Send one rumble datagram on the universal 0xCA plane. `envelope_on` picks the self-terminating
|
||||
/// v2 form (`[level][seq][ttl_ms]`, the default) or the legacy v1 level datagram (the
|
||||
/// `PUNKTFUNK_RUMBLE_ENVELOPE=0` bisect hatch). Best-effort like every side-plane datagram.
|
||||
@@ -824,11 +838,22 @@ pub(super) fn input_thread(
|
||||
tracing::info!(pad = idx, "gamepad unplugged (native detach)");
|
||||
}
|
||||
// Fresh feedback bookkeeping so a later re-plug on this index inherits no
|
||||
// stale rumble lease/seq (a lease still ticking would buzz the new pad).
|
||||
rumble_state[idx] = (0, 0);
|
||||
rumble_seen[idx] = false;
|
||||
rumble_seq[idx] = 0;
|
||||
rumble_stop_burst[idx] = 0;
|
||||
// stale rumble lease (a lease still ticking would buzz the new pad).
|
||||
//
|
||||
// `rumble_seq` deliberately SURVIVES — do not reset it here. The client's
|
||||
// rumble reorder gate (`client/pump/datagram_task.rs`) is per-CONNECTION
|
||||
// and has no reset path, so restarting this counter strands every later
|
||||
// envelope for the re-plugged pad until the host climbs back past the
|
||||
// value the client already stored (up to 128 sends ≈ 15 s of continuous
|
||||
// rumble, or dozens of separate rumble events). The three clears below are
|
||||
// what actually kill a stale lease; the sibling `pad_seq` gate keeps its
|
||||
// value across a removal for exactly the same reason (see the comment at
|
||||
// the top of this arm).
|
||||
clear_pad_feedback(
|
||||
&mut rumble_state[idx],
|
||||
&mut rumble_seen[idx],
|
||||
&mut rumble_stop_burst[idx],
|
||||
);
|
||||
}
|
||||
}
|
||||
InputKind::GamepadArrival => {
|
||||
@@ -1071,6 +1096,72 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// A pad re-plug must not strand the client's rumble reorder gate.
|
||||
///
|
||||
/// The client's `rumble_last_seq` lives for the whole QUIC connection and has no reset path
|
||||
/// (`punktfunk-core/src/client/pump/datagram_task.rs`), so this host's per-pad rumble counter
|
||||
/// has to stay monotonic across a `GamepadRemove`. Regression: the removal arm used to do
|
||||
/// `rumble_seq[idx] = 0`, which made every envelope after a re-plug fail `seq_newer` until the
|
||||
/// counter climbed back past the value the client had already stored — up to 128 sends.
|
||||
///
|
||||
/// Drives the real wire encoder and the real gate, so it fails if either side's rule moves.
|
||||
#[test]
|
||||
fn rumble_seq_survives_a_removal_so_the_client_gate_accepts() {
|
||||
use punktfunk_core::input::GamepadSnapshot;
|
||||
use punktfunk_core::quic::{decode_rumble_envelope, encode_rumble_datagram_v2};
|
||||
|
||||
// The client half: one per-pad slot, per connection, never reset.
|
||||
let deliver = |seq: u8, gate: &mut Option<u8>| {
|
||||
let d = encode_rumble_datagram_v2(0, 0x4000, 0x8000, seq, 400);
|
||||
let env = decode_rumble_envelope(&d)
|
||||
.expect("v2 envelope decodes")
|
||||
.envelope
|
||||
.expect("v2 tail present");
|
||||
if GamepadSnapshot::seq_newer(env.seq, *gate) {
|
||||
*gate = Some(env.seq);
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
};
|
||||
|
||||
// The host half: one wrapping counter, bumped on every change and every renewal.
|
||||
let mut gate: Option<u8> = None;
|
||||
let mut seq = 0u8;
|
||||
|
||||
// A long rumble before the unplug pushes the client's stored seq well past zero.
|
||||
for _ in 0..100 {
|
||||
seq = seq.wrapping_add(1);
|
||||
assert!(deliver(seq, &mut gate));
|
||||
}
|
||||
assert_eq!(gate, Some(100));
|
||||
|
||||
// The pad is unplugged mid-buzz: the lease is cleared, the counter is not.
|
||||
let (mut state, mut seen, mut burst) = ((0x1234u16, 0x5678u16), true, RUMBLE_STOP_BURST);
|
||||
clear_pad_feedback(&mut state, &mut seen, &mut burst);
|
||||
assert_eq!(
|
||||
(state, seen, burst),
|
||||
((0, 0), false, 0),
|
||||
"lease not cleared"
|
||||
);
|
||||
|
||||
// It returns on the same wire index and the game rumbles again: the very first envelope
|
||||
// has to reach the actuator.
|
||||
seq = seq.wrapping_add(1);
|
||||
assert!(
|
||||
deliver(seq, &mut gate),
|
||||
"first envelope after a re-plug was dropped by the client's reorder gate"
|
||||
);
|
||||
|
||||
// Non-vacuity: the pre-fix behaviour (counter restarted at 0) really is rejected, and
|
||||
// stays rejected for the whole forward window — this is the bug, reproduced.
|
||||
let mut stranded = Some(100u8);
|
||||
assert!(
|
||||
(1..=100).all(|s| !deliver(s, &mut stranded)),
|
||||
"test is vacuous — a restarted counter should have been gated out"
|
||||
);
|
||||
}
|
||||
|
||||
/// Incremental wire events accumulate into the full pad frame the virtual xpad applies.
|
||||
#[test]
|
||||
fn gamepad_accumulator() {
|
||||
|
||||
Reference in New Issue
Block a user