fix(client/pads): the gyro cut-off asked about the session, not the pad
Supersedes the check77797a9eshipped an hour ago. The suppression, the log-once, and the "unknown must not suppress" rule all stand; the field it reads does not.77797a9eread `Welcome.gamepad` — the backend the host resolved for the SESSION — and stopped sending motion when it had no motion plane. But the host does not build pads from that. It builds each virtual device from that pad's own `GamepadArrival` (`Pads::set_kind`) and falls back to the session default only for a pad that never declares one, which is precisely why `declared_kind` exists and why its doc comment says an explicit setting has to be re-declared per pad. So the check had a false negative, and it is an ordinary living-room setup. Under "Automatic" the Hello carries the ACTIVE pad's kind (`auto_pref`), so a couch with an X-Box pad on slot 0 and a DualSense on slot 1 echoes Xbox360 — while the host, reading pad 1's arrival, builds it a DualSense with a working motion plane. The old check read the echo, saw no motion plane, and killed pad 1's gyro. That is the exact failure 77797a9e's own commit message names as the worse of the two ("a false negative kills working motion"), introduced by the fix for the other one. The question is per pad, so the slot now carries what it declared, beside the physical `pref` it already held. The two are deliberately separate fields answering different questions: `pref` is the controller in the user's hands, which is what the local feedback paths must keep reading, and `declared` is the one the host is pretending to have. Three facts decide the predicate, and they are written out in `pad_motion_reaches` rather than at the call site because all three clients need the same reasoning: - the echo is not this pad's answer when the pad declared something else; - the host FOLDS what it cannot build — a Switch Pro on Windows, any UHID backend on a host whose /dev/uhid is unusable — and nothing client-side can predict it; - but the echo IS one observed sample of that fold, for the kind the Hello asked about, so it is authoritative for a pad that declared exactly that. Hence: trust the echo when declared == asked, else fall back to the declaration. That keeps both motivating cases — a generic pad under Automatic (declares X-Box 360, suppressed, the sweep's H5c) and an explicit Switch Pro folded to X-Box 360 by a Windows host (declared == asked, so the echo catches it, H5d) — where either field alone gets one of them wrong. `requested_gamepad` is kept on the client next to `resolved_gamepad` for this: the pair is what makes the echo usable per pad, and a lone field would only tempt the next reader back into the session-level question. The residual gap is a pad whose declared kind differs from the session's AND gets folded: we keep sending and the host keeps dropping. That is the direction to be wrong in, and it is what the session-level check was worth in the first place — wasted datagrams, not a dead gyro. Non-vacuity proven both directions rather than assumed. Reverting to `resolved.has_motion()` fails on the mixed-pad row; reverting to `declared.has_motion()` (no echo at all) fails on the Switch-Pro-on-Windows row. Each case in the table is a session someone can actually sit down to, and the comment on each says which of the three inputs decides it. Gate (Linux CI image, pf-lxcheck2): fmt, `build -p punktfunk-core`, `build -p pf-client-core`, `clippy --locked --all-targets -D warnings`, and both test suites — green, with the new case observed in the run's own `... ok` line rather than inferred from a green gate, and pf-client-core's 163 unchanged.
This commit is contained in:
@@ -844,6 +844,12 @@ struct Slot {
|
||||
/// Resolved controller kind (captured at open) — selects the Deck rumble keep-alive and the
|
||||
/// DualSense raw-effect feedback path without re-querying SDL metadata under a `&mut` borrow.
|
||||
pref: GamepadPref,
|
||||
/// The kind this slot DECLARED to the host in its [`InputKind::GamepadArrival`]
|
||||
/// ([`declared_kind`] of the setting and `pref`) — what the host actually built this pad from,
|
||||
/// which under `Auto` differs per pad. Captured at open beside `pref` for the same reason, and
|
||||
/// kept distinct from it because the two answer different questions: `pref` is the controller
|
||||
/// in the user's hands (local feedback), this is the one the host is pretending to have.
|
||||
declared: GamepadPref,
|
||||
/// Wire axis state — zeroed on the wire when this slot closes (detach / unplug).
|
||||
last_axis: [i32; 6],
|
||||
held_buttons: Vec<u32>,
|
||||
@@ -881,12 +887,19 @@ struct Slot {
|
||||
}
|
||||
|
||||
impl Slot {
|
||||
fn new(id: u32, index: u8, pref: GamepadPref, pad: sdl3::gamepad::Gamepad) -> Slot {
|
||||
fn new(
|
||||
id: u32,
|
||||
index: u8,
|
||||
pref: GamepadPref,
|
||||
declared: GamepadPref,
|
||||
pad: sdl3::gamepad::Gamepad,
|
||||
) -> Slot {
|
||||
Slot {
|
||||
id,
|
||||
index,
|
||||
pad,
|
||||
pref,
|
||||
declared,
|
||||
last_axis: [i32::MIN; 6],
|
||||
held_buttons: Vec::new(),
|
||||
held_touches: std::collections::HashSet::new(),
|
||||
@@ -1242,7 +1255,7 @@ impl Worker {
|
||||
let declared = declared_kind(self.kind_override, pref);
|
||||
match self.subsystem.open(sdl3::sys::joystick::SDL_JoystickID(id)) {
|
||||
Ok(pad) => {
|
||||
let mut slot = Slot::new(id, index, pref, pad);
|
||||
let mut slot = Slot::new(id, index, pref, declared, pad);
|
||||
Self::set_slot_sensors(&mut slot, true);
|
||||
slot.audio_caps = self.pad_audio_caps_for(id, &slot.pad);
|
||||
// Declare this pad's kind BEFORE any of its input, so the host builds a matching
|
||||
@@ -2012,19 +2025,28 @@ impl Worker {
|
||||
}
|
||||
}
|
||||
SensorType::Gyroscope => {
|
||||
// The host echoes the backend it actually RESOLVED, which is not
|
||||
// necessarily the one we asked for: an X-Box class pad has no motion plane,
|
||||
// so every sample below would be decoded and dropped. Say so once — the
|
||||
// player's gyro is silently doing nothing and the fix is the controller-type
|
||||
// setting — and stop paying to send ~250 Hz of them.
|
||||
if !c.resolved_gamepad.has_motion() {
|
||||
// An X-Box class pad has no motion plane, so every sample below would be
|
||||
// decoded and dropped. Say so once — the player's gyro is silently doing
|
||||
// nothing and the fix is the controller-type setting — and stop paying to
|
||||
// send ~250 Hz of them.
|
||||
//
|
||||
// Asked PER PAD, off this slot's own declaration. The session echo alone is
|
||||
// the wrong question: under `Auto` the Hello carries the active pad's kind,
|
||||
// so a couch with an X-Box pad on 0 and a DualSense on 1 echoes X-Box 360
|
||||
// while the host builds pad 1 a DualSense with a working gyro.
|
||||
if !punktfunk_core::config::pad_motion_reaches(
|
||||
slot.declared,
|
||||
c.requested_gamepad,
|
||||
c.resolved_gamepad,
|
||||
) {
|
||||
if !slot.motion_unreachable_logged {
|
||||
slot.motion_unreachable_logged = true;
|
||||
tracing::warn!(
|
||||
pad = slot.index,
|
||||
declared = ?slot.declared,
|
||||
resolved = ?c.resolved_gamepad,
|
||||
"this controller has a gyro but the host session resolved a \
|
||||
backend without one — motion will not reach the game; pick a \
|
||||
"this controller has a gyro but the host built it a backend \
|
||||
without one — motion will not reach the game; pick a \
|
||||
DualSense-class controller type to get it"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -325,6 +325,14 @@ pub struct NativeClient {
|
||||
/// The virtual gamepad backend the host actually resolved ([`Welcome::gamepad`]).
|
||||
/// `Auto` = an older host that didn't say (assume X-Box 360, no DualSense feedback).
|
||||
pub resolved_gamepad: GamepadPref,
|
||||
/// The session default this client's Hello ASKED for, kept beside the host's answer above.
|
||||
///
|
||||
/// The pair is what makes the echo usable per pad: the host applies the same fold to a pad's
|
||||
/// own declaration as it did to this, so `resolved` is that pad's answer exactly when the pad
|
||||
/// declared `requested_gamepad` — and only a guess otherwise. See
|
||||
/// [`pad_motion_reaches`](crate::config::pad_motion_reaches), which is the one place that
|
||||
/// reasoning lives.
|
||||
pub requested_gamepad: GamepadPref,
|
||||
/// The encoder bitrate the host actually configured ([`Welcome::bitrate_kbps`], kbps): our
|
||||
/// requested rate clamped to the host's range, or its default if we requested `0`. `0` = an
|
||||
/// older host that didn't report it.
|
||||
@@ -704,6 +712,9 @@ impl NativeClient {
|
||||
host_fingerprint: negotiated.host_fingerprint,
|
||||
resolved_compositor: negotiated.compositor,
|
||||
resolved_gamepad: negotiated.gamepad,
|
||||
// What we asked for, not what came back — the two together are what let a client ask
|
||||
// the motion question per pad (see the field's doc).
|
||||
requested_gamepad: gamepad,
|
||||
resolved_bitrate_kbps: negotiated.bitrate_kbps,
|
||||
shard_payload: negotiated.shard_payload,
|
||||
clock_offset_ns: negotiated.clock_offset_ns,
|
||||
|
||||
@@ -194,9 +194,13 @@ impl GamepadPref {
|
||||
///
|
||||
/// The X-Box classes have no gyro in their HID contract, so a client whose local pad HAS one
|
||||
/// is streaming ~250 Hz of datagrams into a void: the host parses each and discards it, and
|
||||
/// the player sees a controller whose gyro silently does nothing. Read this off
|
||||
/// [`Welcome::gamepad`](crate::quic::Welcome::gamepad) — the backend the host actually
|
||||
/// resolved, which is not necessarily the one the client asked for.
|
||||
/// the player sees a controller whose gyro silently does nothing.
|
||||
///
|
||||
/// This answers for ONE backend. To ask it of a particular pad, go through
|
||||
/// [`pad_motion_reaches`] — the session's [`Welcome::gamepad`](crate::quic::Welcome::gamepad)
|
||||
/// echo is not that pad's answer, because the host builds each virtual device from the pad's
|
||||
/// own `GamepadArrival` and falls back to the session default only for a pad that never
|
||||
/// declared one.
|
||||
///
|
||||
/// `Auto` answers `true` on purpose. It means "unknown": either a host too old to echo the
|
||||
/// field, or one that hasn't resolved yet. Suppressing motion on unknown would silently break
|
||||
@@ -303,6 +307,45 @@ impl GamepadPref {
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether motion sent for ONE pad can reach the game: `declared` is the kind that pad announced
|
||||
/// in its [`InputKind::GamepadArrival`](crate::input::InputKind::GamepadArrival), `asked` is the
|
||||
/// session default the Hello carried, and `resolved` is the host's
|
||||
/// [`Welcome::gamepad`](crate::quic::Welcome::gamepad) echo.
|
||||
///
|
||||
/// Three facts make this a per-pad question rather than a session one:
|
||||
///
|
||||
/// 1. The host builds each virtual device from that pad's arrival — `Pads::set_kind` — and uses
|
||||
/// the session default only for a pad that never declares. So the echo is simply not this
|
||||
/// pad's answer when the two differ.
|
||||
/// 2. The host FOLDS what it cannot build (`resolve_gamepad`/`resolve_pad_kind` share one
|
||||
/// `pick_gamepad`): a Switch Pro on a Windows host, or any UHID backend on a host whose
|
||||
/// `/dev/uhid` is unusable, lands on X-Box 360 with the motion plane gone. Nothing local can
|
||||
/// predict that.
|
||||
/// 3. But the echo IS one observed sample of that fold — for the kind the Hello asked about. When
|
||||
/// a pad declared exactly that kind, the host ran the same fold on the same input, so the echo
|
||||
/// is authoritative for it.
|
||||
///
|
||||
/// Hence: trust the echo for a pad that declared what we asked for, and otherwise fall back to
|
||||
/// what the declaration alone can tell us. That keeps both motivating cases: a generic pad under
|
||||
/// `Auto` (declares X-Box 360, no motion plane, suppressed) and an explicit Switch Pro folded to
|
||||
/// X-Box 360 by a Windows host (declared == asked, so the echo catches it).
|
||||
///
|
||||
/// The residual gap is a pad whose declared kind differs from the session's AND gets folded — we
|
||||
/// keep sending, and the host keeps dropping. That is the direction to be wrong in: the failure
|
||||
/// is wasted datagrams, where guessing the other way would silently kill a working gyro.
|
||||
pub const fn pad_motion_reaches(
|
||||
declared: GamepadPref,
|
||||
asked: GamepadPref,
|
||||
resolved: GamepadPref,
|
||||
) -> bool {
|
||||
// `==` on a fieldless enum, spelled as a match because PartialEq::eq is not const.
|
||||
if declared.to_u8() == asked.to_u8() {
|
||||
resolved.has_motion()
|
||||
} else {
|
||||
declared.has_motion()
|
||||
}
|
||||
}
|
||||
|
||||
/// Per-block FEC parameters. Recovery count is derived from `fec_percent` exactly as
|
||||
/// GameStream does: `m = ceil(k * fec_percent / 100)`.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
@@ -814,6 +857,45 @@ mod tests {
|
||||
assert!(GamepadPref::Auto.has_motion());
|
||||
}
|
||||
|
||||
/// The per-pad question, case by case. Each row is a session a player can actually sit down
|
||||
/// to; the comment says which of the three inputs decides it.
|
||||
#[test]
|
||||
fn motion_reach_is_answered_per_pad_not_per_session() {
|
||||
use GamepadPref::*;
|
||||
// The case this predicate exists for, and the one a session-level check gets WRONG:
|
||||
// "Automatic" with mixed pads. The Hello carries the active pad's kind (an X-Box pad), so
|
||||
// the echo says X-Box 360 — but pad 1 declared a DualSense and the host built it one, with
|
||||
// a motion plane. Reading the echo here kills a gyro that works.
|
||||
assert!(pad_motion_reaches(DualSense, Xbox360, Xbox360));
|
||||
// Its mirror: the pad that DID declare the X-Box kind still has nowhere to put motion.
|
||||
assert!(!pad_motion_reaches(Xbox360, Xbox360, Xbox360));
|
||||
|
||||
// A generic pad (8BitDo &c.) under Automatic — the sweep's motivating case. Detection
|
||||
// lands on X-Box 360, the pad declares it, and its gyro has no plane to reach.
|
||||
assert!(!pad_motion_reaches(Xbox360, Xbox360, Xbox360));
|
||||
|
||||
// An explicit Switch Pro against a WINDOWS host, which folds it to X-Box 360. Declared ==
|
||||
// asked, so the echo is this pad's answer and catches a fold nothing local could predict.
|
||||
assert!(!pad_motion_reaches(SwitchPro, SwitchPro, Xbox360));
|
||||
// The same declaration against a Linux host that builds it: unchanged, motion reaches.
|
||||
assert!(pad_motion_reaches(SwitchPro, SwitchPro, SwitchPro));
|
||||
|
||||
// A DualSense wish on a host with no usable /dev/uhid degrades the same way.
|
||||
assert!(!pad_motion_reaches(DualSense, DualSense, Xbox360));
|
||||
|
||||
// Nobody connected at dial time, so the Hello asked `Auto` and the host resolved it from
|
||||
// its own env. A pad that shows up later declares its own kind and is judged on that —
|
||||
// whichever way the session went.
|
||||
assert!(pad_motion_reaches(DualSense, Auto, Xbox360));
|
||||
assert!(!pad_motion_reaches(Xbox360, Auto, DualSense));
|
||||
|
||||
// An old host that echoes nothing leaves `Auto`, which must not suppress: it may well have
|
||||
// resolved a DualSense, and silently killing gyro is the worse of the two failures.
|
||||
assert!(pad_motion_reaches(DualSense, DualSense, Auto));
|
||||
// Even then the declaration still speaks when it is the thing without a plane.
|
||||
assert!(!pad_motion_reaches(Xbox360, DualSense, Auto));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gamepad_pref_wire_and_names() {
|
||||
for p in [
|
||||
|
||||
Reference in New Issue
Block a user