From 6edd7009107a4b441ee1fdc18072d5bd73acfe41 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sun, 26 Jul 2026 15:32:24 +0200 Subject: [PATCH] =?UTF-8?q?fix(client/gamepad):=20honor=20an=20explicit=20?= =?UTF-8?q?controller=20type=20=E2=80=94=20the=20per-pad=20arrival=20was?= =?UTF-8?q?=20re-declaring=20the=20physical=20pad?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Controller type" setting reached the Hello and stopped there. Every pad then opened with a GamepadArrival carrying its DETECTED kind, and the host builds each virtual device from that arrival — the session default is only the fallback for a pad that never declares one. So "emulate my DualSense as a DualShock 4" put a DualSense on the host the instant the controller connected, and no amount of reconnecting helped. Apple and the native clients both; Android already applied the setting per pad. The setting now rides the declaration too: explicit wins for every slot, Automatic keeps per-pad detection so a mixed session stays honest. The physical kind still drives the LOCAL feedback paths — a DualSense emulated as a DualShock 4 keeps its lightbar and adaptive triggers, and a Deck keeps its rumble keep-alive. Regressed in 97c67b26 / 76be4c3e (multi-controller support); ships in 0.19.x. Co-Authored-By: Claude Opus 5 (1M context) --- .../Sources/PunktfunkClient/ContentView.swift | 4 +- .../PunktfunkKit/Gamepad/GamepadCapture.swift | 17 ++-- .../PunktfunkKit/Gamepad/GamepadManager.swift | 29 ++++++- .../PunktfunkKitTests/GamepadWireTests.swift | 19 +++++ clients/session/src/main.rs | 14 ++- crates/pf-client-core/src/gamepad.rs | 85 ++++++++++++++++++- 6 files changed, 156 insertions(+), 12 deletions(-) diff --git a/clients/apple/Sources/PunktfunkClient/ContentView.swift b/clients/apple/Sources/PunktfunkClient/ContentView.swift index 9937cb73..b5398679 100644 --- a/clients/apple/Sources/PunktfunkClient/ContentView.swift +++ b/clients/apple/Sources/PunktfunkClient/ContentView.swift @@ -990,7 +990,9 @@ struct ContentView: View { rawValue: UInt32(clamping: gamepadType)) ?? .auto) if let name = ProcessInfo.processInfo.environment["PUNKTFUNK_REMOTE_GAMEPAD"], let g = PunktfunkConnection.GamepadType(name: name) { - pad = g + // Back through resolveType so the lever is adopted as the session's setting: the + // per-pad arrivals declare it too, which is what the host actually builds from. + pad = GamepadManager.shared.resolveType(setting: g) } var bitrate = UInt32(clamping: bitrateKbps) if let kbps = ProcessInfo.processInfo.environment["PUNKTFUNK_BITRATE_KBPS"], diff --git a/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadCapture.swift b/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadCapture.swift index b5224180..8236b2b7 100644 --- a/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadCapture.swift +++ b/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadCapture.swift @@ -55,7 +55,11 @@ public final class GamepadCapture { /// Wire pad index (GamepadManager's stable lowest-free assignment), threaded onto every /// event this controller sends — the low byte of `flags`. let pad: UInt32 - /// The controller KIND declared to the host (GamepadArrival) when the slot opened. + /// The controller KIND declared to the host (GamepadArrival) when the slot opened — the + /// user's explicit "Controller type" setting when they picked one, else the detected + /// kind (`GamepadManager.declaredKind(for:)`). NOT the physical pad's kind: local + /// feedback keys off the live `GCController` subclass instead, so an emulated type never + /// costs a DualSense its lightbar or adaptive triggers. let pref: PunktfunkConnection.GamepadType var buttons: UInt32 = 0 var axes: [Int32] = [0, 0, 0, 0, 0, 0] @@ -166,7 +170,7 @@ public final class GamepadCapture { private func openSlot(_ dc: GamepadManager.DiscoveredController) { guard let pad = manager.padIndex(for: dc), let ext = dc.controller.extendedGamepad else { return } let c = dc.controller - let slot = Slot(controller: c, pad: UInt32(pad), pref: dc.kind) + let slot = Slot(controller: c, pad: UInt32(pad), pref: manager.declaredKind(for: dc)) slots.append(slot) ext.valueChangedHandler = { [weak self, weak slot] g, _ in @@ -192,9 +196,12 @@ public final class GamepadCapture { } } // Declare this pad's controller KIND before any of its input, so the host builds a - // matching virtual device (mixed types — pad 0 a DualSense, pad 1 an Xbox pad). The core - // re-sends it a few times against datagram loss; an older host ignores it and uses the - // session-default kind. Then wake the host pad (pads are created lazily from the first + // matching virtual device — the user's chosen type when they picked one, else per-pad + // detection (mixed types — pad 0 a DualSense, pad 1 an Xbox pad). This declaration is + // what the host actually builds from, so it MUST carry an explicit setting; the + // handshake's session default is only the fallback for a pad that never declares. The + // core re-sends it a few times against datagram loss; an older host ignores it and uses + // the session-default kind. Then wake the host pad (pads are created lazily from the first // event; a DualSense's UHID handshake + initial lightbar write only start then). connection.send(.gamepadArrival(pref: slot.pref.rawValue, pad: slot.pad)) connection.send(.gamepadAxis(GamepadWire.axisLSX, value: 0, pad: slot.pad)) diff --git a/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadManager.swift b/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadManager.swift index dc161caf..0e13eb96 100644 --- a/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadManager.swift +++ b/clients/apple/Sources/PunktfunkKit/Gamepad/GamepadManager.swift @@ -131,13 +131,40 @@ public final class GamepadManager: ObservableObject { GCController.stopWirelessControllerDiscovery() } + /// The user's controller-type choice AS CHOSEN (not resolved) for the session being dialed — + /// adopted by `resolveType` and read back by `declaredKind(for:)`. `.auto` = detect per pad. + public private(set) var typeSetting: PunktfunkConnection.GamepadType = .auto + + /// The kind to DECLARE to the host for one forwarded controller (its `GamepadArrival`). + /// An explicit setting wins for every pad — the handshake's session default alone does NOT + /// stick, because a current host honors the per-pad arrival over it (punktfunk-host's + /// `Pads::set_kind`), so a client that declared only the detected kind here would silently + /// undo the user's choice. `.auto` keeps per-pad detection, which is what makes a mixed + /// session (pad 0 a DualSense, pad 1 an Xbox pad) honest. + public func declaredKind( + for controller: DiscoveredController + ) -> PunktfunkConnection.GamepadType { + Self.declaredKind(setting: typeSetting, detected: controller.kind) + } + + /// The pure fold behind `declaredKind(for:)` (pf-client-core's `declared_kind`). + nonisolated static func declaredKind( + setting: PunktfunkConnection.GamepadType, + detected: PunktfunkConnection.GamepadType + ) -> PunktfunkConnection.GamepadType { + setting == .auto ? detected : setting + } + /// Connect-time resolution of the user's controller-type setting: an explicit choice /// wins; `.auto` matches the virtual pad to the active physical controller (DualSense → /// DualSense, DualShock 4 → DualShock 4, an Xbox pad → Xbox One, anything else → Xbox - /// 360); no controller at all defers to the host. + /// 360); no controller at all defers to the host. Called once per dial with the RAW setting, + /// which it also adopts for `declaredKind(for:)` so the handshake default and every pad's + /// arrival can never disagree about an explicit choice. public func resolveType( setting: PunktfunkConnection.GamepadType ) -> PunktfunkConnection.GamepadType { + typeSetting = setting guard setting == .auto else { return setting } // Refresh from the LIVE controller list first. `active` is otherwise only populated by the // async `.GCControllerDidConnect` notification, so at connect time it can still be nil even diff --git a/clients/apple/Tests/PunktfunkKitTests/GamepadWireTests.swift b/clients/apple/Tests/PunktfunkKitTests/GamepadWireTests.swift index f93e8f38..d9786458 100644 --- a/clients/apple/Tests/PunktfunkKitTests/GamepadWireTests.swift +++ b/clients/apple/Tests/PunktfunkKitTests/GamepadWireTests.swift @@ -119,6 +119,25 @@ final class GamepadWireTests: XCTestCase { XCTAssertEqual(GamepadWire.maxPads, 16) } + func testAnExplicitControllerTypeIsWhatEveryPadDeclares() { + // The regression this pins: the "Controller type" setting reached the Hello only, and each + // pad's arrival then re-declared the DETECTED kind — which the host honors over the + // session default, so "emulate my DualSense as a DualShock 4" produced a DualSense. + // (pf-client-core's `declared_kind` test is the same table.) + XCTAssertEqual( + GamepadManager.declaredKind(setting: .dualShock4, detected: .dualSense), .dualShock4) + // Every physical pad in a mixed session follows the one explicit choice. + for detected: PunktfunkConnection.GamepadType in [ + .dualSense, .xbox360, .switchPro, .dualSenseEdge, + ] { + XCTAssertEqual( + GamepadManager.declaredKind(setting: .xbox360, detected: detected), .xbox360) + } + // Automatic keeps per-pad detection — otherwise a mixed session collapses to one type. + XCTAssertEqual(GamepadManager.declaredKind(setting: .auto, detected: .dualSense), .dualSense) + XCTAssertEqual(GamepadManager.declaredKind(setting: .auto, detected: .switchPro), .switchPro) + } + func testTouchpadConversionCorners() { // GC ±1 with +y up → wire 0...65535 with origin top-left, +y down. let topLeft = GamepadWire.touchpad(x: -1, y: 1) diff --git a/clients/session/src/main.rs b/clients/session/src/main.rs index a254fbbc..5fc2d1b9 100644 --- a/clients/session/src/main.rs +++ b/clients/session/src/main.rs @@ -204,9 +204,17 @@ mod session_main { mode, compositor: CompositorPref::from_name(&settings.compositor) .unwrap_or(CompositorPref::Auto), - gamepad: match GamepadPref::from_name(&settings.gamepad) { - Some(GamepadPref::Auto) | None => gamepad.auto_pref(), - Some(explicit) => explicit, + gamepad: { + // The setting AS CHOSEN goes to the pad service too, not just the Hello: the host + // builds each virtual pad from that pad's arrival and only falls back to this + // session default for a pad that never declares one, so an explicit choice that + // stopped here would be undone the moment a controller connected. + let chosen = GamepadPref::from_name(&settings.gamepad).unwrap_or(GamepadPref::Auto); + gamepad.set_kind_override(chosen); + match chosen { + GamepadPref::Auto => gamepad.auto_pref(), + explicit => explicit, + } }, bitrate_kbps: settings.bitrate_kbps, audio_channels: settings.audio_channels, diff --git a/crates/pf-client-core/src/gamepad.rs b/crates/pf-client-core/src/gamepad.rs index 2ef54988..015fc0bd 100644 --- a/crates/pf-client-core/src/gamepad.rs +++ b/crates/pf-client-core/src/gamepad.rs @@ -299,6 +299,23 @@ fn pref_for_type(t: sdl3::gamepad::GamepadType) -> GamepadPref { } } +/// The kind a slot DECLARES to the host ([`InputKind::GamepadArrival`]) given the user's +/// controller-type `setting` and the pad's `physical` kind: an explicit setting emulates that pad +/// for every slot, `Auto` keeps per-pad detection (what makes a mixed session honest). +/// +/// This has to be applied per pad and not just in the Hello: the host builds each virtual device +/// from that pad's arrival and only falls back to the session default for a pad that never +/// declares one, so a client that always declared the detected kind would silently undo the +/// setting the moment a controller connected. The physical kind is still what the LOCAL feedback +/// paths use (DualSense raw effects, the Deck rumble keep-alive) — those talk to the controller in +/// the user's hands, not the one the host is pretending to have. +fn declared_kind(setting: GamepadPref, physical: GamepadPref) -> GamepadPref { + match setting { + GamepadPref::Auto => physical, + explicit => explicit, + } +} + /// Best-effort "this machine is a Steam Deck". The Gaming-Mode env short-circuits; desktop /// mode falls back to DMI (Valve board, Jupiter = LCD / Galileo = OLED — readable inside the /// flatpak sandbox). Cached: the answer can't change while we run. @@ -318,6 +335,7 @@ enum Ctl { Attach(Arc), Detach, Pin(Option), + KindOverride(GamepadPref), MenuMode(bool), MenuRumble(MenuPulse), } @@ -452,6 +470,18 @@ impl GamepadService { let _ = self.ctl.send(Ctl::Pin(key)); } + /// Adopt the user's explicit controller-type setting for the session about to start + /// (`GamepadPref::Auto` = detect per pad, the default). + /// + /// This is NOT redundant with the session default in the Hello: a current host honors a pad's + /// [`InputKind::GamepadArrival`] over the session default, so a client that declared only the + /// detected kind would silently undo the setting the moment a controller connected. Call it + /// before [`Self::attach`] — slots declare their kind at open time and the host does not + /// hot-swap a device that already exists. + pub fn set_kind_override(&self, pref: GamepadPref) { + let _ = self.ctl.send(Ctl::KindOverride(pref)); + } + pub fn attach(&self, connector: Arc) { let _ = self.ctl.send(Ctl::Attach(connector)); } @@ -691,6 +721,10 @@ struct Worker { /// connected pads, so it survives restarts and disconnects. A pin forwards ONLY that pad /// (an explicit single-player choice); Automatic forwards every real controller. pinned: Option, + /// The user's explicit "controller type" setting ([`GamepadService::set_kind_override`]); + /// `Auto` = per-pad detection. Applied at slot open to the kind DECLARED to the host, never + /// to [`Slot::pref`] — the local feedback paths must keep reading the physical pad. + kind_override: GamepadPref, attached: Option>, /// Raises the UI escape signal; the escape chord fires it once per press. escape_tx: async_channel::Sender<()>, @@ -886,6 +920,7 @@ impl Worker { Some(p) => p.pref, None => GamepadPref::Xbox360, }; + 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); @@ -895,7 +930,13 @@ impl Worker { // re-sends it a few times against datagram loss; an older host ignores it and // uses the session-default kind. if let Some(c) = &self.attached { - send(c, InputKind::GamepadArrival, pref.to_u8() as u32, 0, index); + send( + c, + InputKind::GamepadArrival, + declared.to_u8() as u32, + 0, + index, + ); // Declare the actuator's quirks to the shared rumble policy engine. ALWAYS // set (defaults for a well-behaved pad): wire indices are reused within a // connection, so a Deck slot that closes must not leave its keepalive quirk @@ -911,7 +952,13 @@ impl Worker { }; c.set_rumble_quirks(index as u16, quirks); } - tracing::info!(id, index, pref = ?pref, "gamepad forwarding (slot opened)"); + tracing::info!( + id, + index, + pref = ?pref, + declared = ?declared, + "gamepad forwarding (slot opened)" + ); self.slots.push(slot); } Err(e) => tracing::warn!(id, error = %e, "gamepad open failed"), @@ -1219,6 +1266,7 @@ impl Worker { self.pinned = key; self.refresh_active(); } + Ok(Ctl::KindOverride(pref)) => self.kind_override = pref, Ok(Ctl::MenuMode(on)) => { self.menu_mode = on; if on { @@ -1558,6 +1606,7 @@ impl Worker { menu_open: None, order: Vec::new(), pinned: None, + kind_override: GamepadPref::Auto, attached: None, escape_tx, disconnect_tx, @@ -1823,6 +1872,38 @@ mod slot_tests { assert_eq!(lowest_free_index(&but_seven), Some(7)); } + #[test] + fn an_explicit_setting_is_what_every_pad_declares() { + // The regression this pins: the setting used to reach the Hello only, and each pad's + // arrival then re-declared the DETECTED kind — which the host honors over the session + // default, so "emulate my DualSense as a DualShock 4" produced a DualSense. + assert_eq!( + declared_kind(GamepadPref::DualShock4, GamepadPref::DualSense), + GamepadPref::DualShock4 + ); + // Every physical pad in a mixed session follows the one explicit choice. + for physical in [ + GamepadPref::DualSense, + GamepadPref::Xbox360, + GamepadPref::SwitchPro, + GamepadPref::SteamDeck, + ] { + assert_eq!( + declared_kind(GamepadPref::Xbox360, physical), + GamepadPref::Xbox360 + ); + } + // Automatic keeps per-pad detection — otherwise a mixed session collapses to one type. + assert_eq!( + declared_kind(GamepadPref::Auto, GamepadPref::DualSense), + GamepadPref::DualSense + ); + assert_eq!( + declared_kind(GamepadPref::Auto, GamepadPref::SteamDeck), + GamepadPref::SteamDeck + ); + } + #[test] fn hidout_pad_reads_every_variant() { assert_eq!(