Skip to content

Steam Controller (2026, "Triton") — full wireless support (BLE, no dongle, no root)#1706

Open
JonahSimon wants to merge 58 commits into
utkarshdalal:masterfrom
JonahSimon:steam-controller-2026
Open

Steam Controller (2026, "Triton") — full wireless support (BLE, no dongle, no root)#1706
JonahSimon wants to merge 58 commits into
utkarshdalal:masterfrom
JonahSimon:steam-controller-2026

Conversation

@JonahSimon

@JonahSimon JonahSimon commented Jul 12, 2026

Copy link
Copy Markdown

Description

Full-functionality support for the Valve Steam Controller (2026, codename "Triton") inside GameNative — wirelessly, no dongle, no root: all four rear paddles, both trackpads, dual sticks, gyro, analog triggers, grips, and haptics, usable in any GameNative game (not just ones with built-in controller support).

Scope note: this is a new feature, which CONTRIBUTING.md puts out of scope unless approved beforehand. It was green-lit by the team ("please open a PR") — opening per that go-ahead. Designed from day one to be additive and non-invasive so it can't affect GameNative's existing controller code (see Isolation below).

Primary test targets: ToME4, DOOM/DOOM II, and WoW 3.3.5a + ConsolePort, but the goal is any game.

Why the Steam Controller doesn't "just work"

  1. It isn't a normal gamepad. The 2026 Triton talks over a custom BLE GATT service — Android never enumerates it as an InputDevice, so nothing in GameNative (or Android) sees a controller at all.
  2. There's no Steam Input in a container. Games run under Goldberg; "Use Steam Input" is a gamepad-only VDF→keymap with no vocabulary for trackpads/gyro/paddles. The rich behavior a Steam Controller normally gets from Steam simply isn't present.
  3. The one real transport blocker was the BLE MTU: the 45-byte input report exceeds Android's default 23-byte MTU, so reports never arrive until you requestMtu(517) before service discovery. After that the full report streams at ~70 Hz.

How it works (architecture)

Layered so the engine is testable on a PC and the device-specific parts are isolated:

  • Transport — TritonBle. Connects the GATT service, raises the MTU, subscribes the input characteristic (100F6C7A), decodes reports → TritonState. Writes lizard-off + a 2 s heartbeat so the OS cursor doesn't wander. Output reports (trackpad haptics + rumble) route to per-id characteristics. BLE is the sole transport.
  • Mapping engine — ProfileInterpreter + ScProfile. Since there's no Steam Input, the mapping engine is built here: modes (mouse / joystick / flick / scroll / d-pad / radial + touch menus), activators (regular / double / long / turbo / on-release), action sets, action layers, mode-shift, response curves, movement radials, an on-screen split keyboard, and haptic feel. Unit-tested headlessly by replaying a captured input trace.
  • Output — hybrid, via the ScOutputSink seam. Gamepad-style outputs (sticks/buttons/triggers) drive a virtual XInput/DInput pad via WinHandler.sendVirtualGamepadState; everything with no XInput equivalent (trackpads, gyro, paddles → key/mouse/scroll/menus) goes through XServer.inject*.
  • Config system. A .vdf importer (SteamControllerVdfUtils, reusing GameNative's own VDF parser) is the coverage guarantee — any Steam Input config parses into our model. A per-game named-config registry (ScConfigStore) holds multiple configs with an active selector; editing a .vdf-backed config writes a lossless overlay so menus/layers/mode-shift the editor doesn't touch are preserved exactly.
  • In-game editor (a Steam-Input clone). Full binding editor + command picker, analog/trigger/gyro/haptics panes, menu authoring, and layers — all applying live via TritonMapper.reload() (no relaunch). Everything is controller-navigable (explicit d-pad selection model + pad-mouse cursor), since the BLE controller can't reach Compose focus on its own.

Isolation — why this can't break existing controller support

  • The SC input path is fully parallel — the controller is never registered as an Android InputDevice, so it never enters GameNative's existing controller code. PhysicalControllerHandler and the legacy gamepad path are untouched.
  • The only shared surfaces are output seams we call but don't change (WinHandler.sendVirtualGamepadState / XServer.inject*), plus two small, passive, opt-in taps documented inline: a nullable WinHandler.RumbleForwarder (null unless an SC session is live → zero change to legacy rumble) and InputDeviceManager.onGuestSuspended() (stops main-thread key auto-repeat before the guest is suspended — a latent ANR fix that also benefits the base app).
  • Everything is gated behind transportReady/opt-in, so it never contends with a normal controller.

Guide for reviewers (where to look)

The bulk of the diff is the isolated steamcontroller/ package + the in-game editor + test fixtures. Suggested reading order, and I've left inline comments on the key/non-obvious lines:

Area Files What to know
Transport steamcontroller/TritonBle.kt, TritonProtocol.kt GATT connect, the MTU fix, report decode, output-report char routing
Haptics steamcontroller/TritonHaptics.kt Regenerates the click/detent feel lizard-off kills; builds the rumble report (SDL wire format)
Driver/lifecycle steamcontroller/TritonMapper.kt Parallel BLE path, rumble forward + resend, live reload()
Engine steamcontroller/ProfileInterpreter.kt, ScProfile.kt The mapping engine; apply(state) is the per-report entry point
Importer utils/SteamControllerVdfUtils.kt .vdf → profile (the coverage guarantee)
Output seam steamcontroller/ScOutputSink.kt + XServerScreen.kt wiring How outputs reach WinHandler/XServer
Shared touches WinHandler.java, xserver/InputDeviceManager.java, QuickMenu.kt, PrefManager.kt The only edits to existing files — each annotated inline explaining why it's safe/needed
Tests app/src/test/java/app/gamenative/steamcontroller/*, utils/SteamControllerProfileImporterTest.kt, src/test/resources/sc/*.vdf ~24 test classes; the large .vdf fixtures are real Steam Input exports used as importer regression cases

Recording

SC_PR_recording_small.mp4

Type of Change

  • Bug fix
  • Performance / stability improvement
  • Compatibility improvements
  • Other (requires prior approval) — new hardware support; approved beforehand by the team

Checklist

  • If I have access to #code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.
  • This change aligns with the current project scope (core functionality, stability, or performance). If not, it has been explicitly approved beforehand. — approved beforehand.
  • I have attached a recording of the change. — to be attached by the author.
  • I have read and agree to the contribution guidelines in CONTRIBUTING.md.

🤖 The transport/protocol/engine work was developed with heavy AI assistance (Claude Code), directed and reviewed by the author. CONTRIBUTING.md has no AI-contribution restriction.


Summary by cubic

Adds full wireless support for the Valve Steam Controller (2026 “Triton”) over BLE (no dongle, no root), with a Steam Input–style mapping engine, in‑game editor, overlays, and per‑game configs — all isolated from the existing Android controller path.

  • New Features

    • Direct BLE GATT transport with MTU raise, input decode, and haptics; forwards game rumble to the controller.
    • Mapping engine (ProfileInterpreter/ScProfile) for pads/sticks/triggers/gyro, activators, action sets/layers, mode‑shift; outputs to virtual XInput via WinHandler and keys/mouse via XServer.
    • Steam Input .vdf import to runtime config; per‑game named configs (ScConfigStore) with live reload().
    • In‑game Steam‑style editor hub, fully controller‑navigable with pad‑mouse cursor; radial/touch menu HUD with per‑menu placement; split on‑screen keyboard.
    • Modes/outputs: absolute/region mouse (warp), pad‑as‑joystick, stick‑as‑dpad, mouse_joystick, mouse_position, directional swipe; gyro gates (grips/pad/stick/any/all touch) and activation (enable/suppress/toggle); per‑surface tuning.
    • Android additions: BLE scan/connect perms; optional rumble forwarder seam in WinHandler; no changes to the legacy Android controller path.
  • Bug Fixes

    • Stability: cancel X key auto‑repeat on guest suspend to prevent ANRs; mark TritonMapper fields @Volatile for cross‑thread reads; ScConfigStore registry writes use a unique temp file + atomic rename to avoid truncation and concurrent clobber.
    • Rumble: forward XInput rumble to the controller and cancel local phone/controller vibration when forwarding.
    • Importer correctness (SteamControllerVdfUtils): map absolute_mouse to relative mouse; treat curve_exponent as a true power exponent; accept mouse_joystick on sticks/pads; fix gyro activation key (gyro_button_invert) and touch‑gate decode.
    • UI: fix stuck editor cursor after closing the QuickMenu; hide cursor on capture end; resolve QuickMenu action ID clash; smoother scrollbar drag handling.
    • Cleanup: remove dead USB‑only code from TritonProtocol; move diagnostics flag to PrefManager to avoid ART VerifyError.

Written for commit c76f023. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

Summary

  • New Features
    • Added Steam Controller support over BLE, including required Bluetooth permission declarations.
    • Introduced a full Steam Controller live input/mapping engine with an on-screen split keyboard.
    • Added in-game Steam Controller config management: import/edit configs, manage per-menu labels, and edit/save HUD/keyboard overlay placement.
  • Bug Fixes
    • Improved Steam Controller session suspend/resume behavior, QuickMenu capture routing, and rumble forwarding consistency.
    • Diagnostics setting is now persisted for wrapper behavior.
  • Tests
    • Expanded Robolectric/unit coverage for mappings, menus, gestures, haptics, config storage, and VDF import.

JonahSimon and others added 30 commits June 26, 2026 22:09
Checkpoint of the full Steam Controller feature on the steam-controller-2026 branch.

Transport & engine:
- BLE GATT transport (no dongle, no root; MTU 517) + USB fallback; Triton protocol decode + haptics.
- Mapping engine (ProfileInterpreter/ScProfile): buttons, sticks/pads (mouse/joystick/flick/scroll/dpad),
  radial & touch menus, button-pad grids, triggers (axis/staged), gyro, activators
  (Regular/Double/Long/Turbo/OnRelease), action sets, action layers, mode-shift. Overlay HUD + split keyboard.
- Per-game named-config registry (ScConfigRegistry) + live reload() apply seam.

In-game editor (Steam-Input clone), full-coverage authoring:
- Lossless .vdf editing: base .vdf + editable overlay + INHERIT (ScConfigStore.applyOverlay) - never flattens
  menus/layers/mode-shift.
- Authors: digital binds, multi-key combos, menus (radial/touch/button-pad/dpad), action layers (LayerOp +
  Layer toggle + derived setSources), ShowKeyboard/OpenQuickMenu, MouseNudge, triggers/gyro/haptics.
- Mode-shift is authored as a single-source hold-layer (no separate primitive); imported vdf mode_shift preserved.

Editor controller-navigation (in progress):
- B-back wired to each dialog's dismiss (ScNavDialogStack); LB/RB -> command-picker tab intents; scroll indicators.

Fixes:
- Intermittent ANR: cancel X-server key auto-repeat + clear held keys on guest suspend (InputDeviceManager).
- Radial HUD slot scaling at n=1.

Tested headlessly via ScOutputSink + golden-trace replay; validated on-device (Z Fold 7, BLE) through Phase D.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
# Conflicts:
#	app/src/main/AndroidManifest.xml
Make the in-game Steam Controller editor usable with any controller, validated
on-device (Z Fold 7, BLE) after the upstream merge.

Mechanism: Compose's own focus traversal (moveFocus) is unreliable inside
Dialog/FlowRow/scroll layouts (diagnosed on-device: key consumed, focus never
moved), so navigation is an explicit (line,col) selection model with a visible
highlight + bringIntoView scroll.

New files:
- ScListNav.kt        ScNavState/ScNavItem/ScNavDialogColumn (the nav model)
- ScCursorController.kt SC pad-mouse cursor (decor view + MotionEvent tap inject)
- ScTextEditField.kt   in-dialog keyboard for text fields (no system IME)
- ScNavChoiceDialog.kt navigable choice modal replacing DropdownMenu popups

Editor (SteamControllerBindingEditor.kt):
- Item-by-item d-pad nav in the Bindings editor (auto-scroll; left/right across
  set chips / Layer-Default-Delete / Reset-Cancel-Save); A activates.
- LB/RB switch command-picker tabs AND action sets (+ BumperHint badges).
- Picker grid nav (key grid, modifier/mouse/gamepad/advanced chips, Apply/Cancel/Clear).
- Dropdowns open ScNavChoiceDialog; sliders adjust with d-pad L/R (1/step, hold-repeat).
- Analog/Trigger/Gyro/Haptics sub-pickers navigable via ScNavDialogColumn.
- Set-name field d-pad reachable (controlled ScTextEditField).
- Browser-style scrollbar (ScScrollbar.kt).
- Fixes: stale-lambda Delete/Default/Layer (rememberUpdatedState); Trigger/Gyro
  Behavior dropdown now reflects the actual mode instead of hardcoded "Inherit".

Engine:
- ProfileInterpreter.handleMenuNav: held-direction auto-repeat (d-pad or left
  stick); handleMenuCursor: pad-mouse move/tap + detent-tick & click haptics.
- ScUiBridge: moveCursor/cursorTap.

XServerScreen.kt:
- Extracted SC editor dialogs into ScLiveEditorDialogs (XServerScreen was at the
  ART verifier method-size limit -> adding nav code caused a launch VerifyError).
- Hub d-pad nav + one-level B-back (scReturnToRoot); cursor bridge wiring.

SC unit tests green.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…verlay)

Completes d-pad controller navigation across every SC editor surface, reusing
the explicit (line,col) ScNavState/ScNavItem/ScNavDialogColumn model.

- ScTuningDialog: deadzone/smoothing sliders adjust with d-pad L/R (live-apply
  each step); commit chips + Done are nav items.
- ScMenuLabelEditor: per-slot OutlinedTextField -> ScTextEditField (read-only
  chip -> A/tap opens the in-dialog on-screen keyboard, no system IME); one nav
  line per slot; Cancel/Save on a fixed high line.
- ScOverlayEditor: keep touch drag/pinch + the SC pad-cursor; add d-pad rows
  (Horizontal/Vertical position, Size) + a navigable button row so a regular
  controller with no trackpad can place the overlay.
- XServerScreen: remove the temporary ScNav Log.i diagnostic + its unused locals.

Pure Compose-UI (no engine/model change). assembleModernDebug green; verified
on-device (Z Fold 7, BLE).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
# Conflicts:
#	app/src/main/AndroidManifest.xml
Remove dead debug-only code and a redundant converter, no behavior change:
- Delete the undocumented `installeditable` SC_SELFTEST mode and its sole
  consumer ScConfigStore.saveEditable (the editor saves via saveEditableConfig).
- Collapse EditBinding.fromOutput to Binding(out).toEdit() (provably identical:
  a bare Binding carries the default Regular activator).

SC unit tests green.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
The Steam Controller (Triton) config UI was bleeding into the generic
container-config Controller tab, where it's irrelevant to almost all users.
Move it out entirely; the QuickMenu is now the single home for SC settings.

- ControllerTab: delete the whole SteamControllerConfigSection (config
  selector / duplicate / rename / delete / import .vdf / remove-labels /
  overlay + keyboard HUD / deadzone-smoothing-commit). Keep only the real
  upstream controller settings (SDL / Steam Input / XInput / DInput / shooter /
  external display). Drop the now-vestigial containerId plumbing from
  ContainerConfigDialog + its BaseAppScreen/PluviaMain callers.
- QuickMenu (ScLiveEditorDialogs): add two hub items so nothing is lost —
  "Manage configs" (new ScConfigManagerDialog: active-config selector,
  duplicate/rename/delete, import .vdf, remove labels; controller-navigable via
  ScNavState + reuses ScNavChoiceDialog + ScOnScreenKeyboardDialog) and
  "Keyboard Layout" (the KEYBOARD-target overlay editor; the hub previously only
  exposed the MENU overlay). Both call tritonMapper.reload() so edits apply live.
- Make ScOnScreenKeyboardDialog reusable (was file-private) for name entry.

Tradeoff: editing the *shared default* config (the default-template editor) is
no longer surfaced — the QuickMenu edits the running game's config. Per-game
in-game is the right model for the ~99% case; a shared-default affordance can be
added to the config manager later if wanted.

Builds (:app:assembleModernDebug). On-device verify pending.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Removed comments explaining applicationIdSuffix limitations for debug builds.
The split-trackpad keyboard was letters + digits + , . - only. Added a
symbol page toggled by a ?123/ABC key (right half, previously an Empty
cell) covering ! @ # $ % ^ & * ( ) - _ = + [ ] { } backslash pipe ; : ' "
grave ~ / ? < >. Shifted glyphs inject their base keycode with Shift held
via a new Chr.forceShift flag; the active page threads through
ScKeyboardSpec so the overlay renders it. Unit-tested (symbol type + toggle).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Removed the standalone "Menu labels" hub entry. Slot labels now live in
one place: authored menus rename slots inside the surface behavior editor
(already did), and inherited menus (from a .vdf / built-in default) get a
"Rename slots..." affordance on their analog-source row, opening the label
editor scoped to that (set, location). ScMenuLabelEditorDialog gained
filterSetId/filterLocation params for the scoped view.

Dropped the now-dead standalone plumbing: showScLabels state/params/render
block in ScLiveEditorDialogs, the SC_LABELS QuickMenuAction dispatch + const,
and the XServerScreen import. Labels apply live on the bindings-editor
reload (no "next launch" needed).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Each menu can now sit in its own spot instead of sharing one game-wide HUD
placement. The interpreter threads the host surface id (LEFT_PAD / RIGHT_PAD /
LEFT_STICK / RIGHT_STICK) into ScMenuSpec.menuId; ScMenuOverlayView resolves
placement per menuId at draw (cached, gameKey mode) via a new
ScOverlayStore.forMenu with a graceful fallback: per-menu-per-game →
per-menu-global → whole-HUD per-game → global → built-in, so an existing
whole-HUD placement still applies to every menu until a per-menu override
refines one. TritonMapper.reload() clears the cache so edits apply live.

The overlay editor (menu target) gained a "Placing: <menu>" selector that
cycles All menus + each menu present in the config; Save/Use-global act on the
selected scope. Tests: forMenu fallback chain (ScOverlayStoreTest) + menuId
threading assertion in MenuModesTest.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Added PadMode.AbsoluteMouse: while the pad is touched, the finger's position
maps 1:1 onto a screen region (default = full screen), warping the cursor
there instead of emitting relative deltas. New ScOutputSink.mouseMoveAbs(nx,ny)
(normalized 0..1) scaled to the X screen via injectPointerMove + screenInfo.
The importer now decodes absolute_mouse / mouse_region to this instead of the
old relative-Mouse approximation (region sub-rect params still TODO, needs a
reference config to decode safely). Editor preserves it losslessly as INHERIT
(no authoring UI yet). Tests: interpreter position + region mapping.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Added LEFT_PAD_TOUCH / RIGHT_PAD_TOUCH gyro gates (gyro-mouse active only
while the aim trackpad is touched — the most common gyro style, beyond the
existing grip gates). Wired into the interpreter gate check + the gyro
picker's "Active when" dropdown; round-trips via EditGyro's gate string.
Test: pad-touch gate aims only while touched.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…(2dscroll)

From the captured controller_triton schema: enrich PadMode.AbsoluteMouse to
Steam's center+size params (position_x/y, scale, scale_x/y, invert_x/y) so
mouse_region maps to the real screen sub-rect; add PadMode.SingleButton (whole
pad = one button) and PadMode.DirectionalSwipe (flick a cardinal dir -> pulse,
Steam mode "2dscroll"). Importer decodes all three (were else->None); interpreter
handlers + editor preserves them as INHERIT. Tests for each.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
New ScOutput.MousePosition (Steam controller_action MOUSE_POSITION x y return):
on the press edge warps the cursor to an absolute screen fraction via
mouseMoveAbs. Importer parses the controller_action (x,y in 0..32767 -> 0..1);
editor preserves it as INHERIT (advancedDesc). Auto-return-after not yet honored
(needs the sink to read current position). Test: button press -> warp.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
New GyroMode.Joystick: gyro rate -> virtual stick deflection (yaw->X, pitch->Y),
added on top of the physical stick when gated open. Importer decodes the
confirmed vdf modes gyro_to_mouse / gyro_to_joystick(_camera) / _deflection;
editor round-trips via new GyroEditMode.JOYSTICK (summaries updated). Extracted
a shared gyroGateOpen() helper. Fixed a real ordering bug the test caught:
applyGyro ran AFTER sink.gamepad(gp), so a gyro-driven stick never reached the
sink that frame — moved it before the push. Test: gyro deflects the stick.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Imported configs were stored as "Imported (.vdf)" while the selector already
appends "  (.vdf)" for VDF kind -> doubled suffix. Now derive the name from the
vdf's top-level `title` (e.g. "testGyroect123"), else the SAF file name, else
"Imported"; dropped "(.vdf)" from the default + migration label.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
A playable controller_triton .vdf for DOOM (KEX) that exercises the FPS-relevant
new modes: sticks->xinput, gyro->right stick (gyro_to_joystick camera), left pad
->2dscroll weapon-cycle swipe, right pad->relative mouse aim, triggers/bumpers/
paddles->xinput. Test imports it and asserts the intended modes parse. Resource
at app/src/test/resources/sc/doom_sc_test.vdf.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
The old sc_fullcoverage_test.vdf wrote the right pad as plain `mouse`
and left single_button unbound, so B1 (mouse_region) looked like a
relative-mouse bug and B2 was untestable. The importer decodes
mouse_region -> AbsoluteMouse and single_button -> SingleButton
correctly; the config was the problem.

Add sc_region_single_test.vdf (right pad = mouse_region, left-quarter /
30% wide / inverted; left pad = single_button -> key F) + a test
asserting the decode. Both re-tested on-device 2026-07-04: working.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
A trackpad in joystick_move mode was dropped (else -> None). Add
PadMode.Joystick: the finger's absolute position drives a virtual XInput
stick while touched, recentering on lift, reusing the stick curve/deadzone
math. Importer defaults to the RIGHT (camera) stick; output_joystick 0
forces LEFT. Editor preserves it losslessly (null = inherit).

Also fixes a latent ordering bug: sink.gamepad(gp) was pushed before
applyPad, so a pad's gp contribution would miss the frame. Moved the push
after every gp contributor (buttons/sticks/triggers/gyro/pad-joystick).

Tests: importer v2 template right-pad now decodes to Joystick(RIGHT);
interpreter deflect-while-touched + recenter-on-lift (RecordingSink gains
last-frame stick values).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
A stick in dpad mode was dropped (else -> None). Add StickMode.DPad:
deflection past the deadzone presses the matching edge output(s),
recentering to neutral. Extracted the pad's 8-way edge logic into a shared
driveDpad() helper (pad + stick both use it); an all-empty dpad collapses
to None. Editor preserves it losslessly (stick dpad isn't an editor option).

This was silently dropping real bindings: the v3 WASD template (stick ->
WASD) and Delf's ToME4 community config (right stick -> scroll + mouse
warp) both decoded to None before; now correct. Tests updated + a new
interpreter deflect/recenter test + an inline importer decode test.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
sc_newmodes_test.vdf drives both 2026-07-04 modes for on-device testing:
left stick = dpad->WASD (stick-as-dpad), left trackpad = joystick_move
(pad-as-joystick, RIGHT stick). Parse-verified by a new importer test.
Referenced by docs/TESTING-GUIDE-2026-07-05.md.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Per-binding settings (edge outputs): delay_start/delay_end (Fire Start/End
Delay, ms) via a per-frame timed scheduler on the injected clock, with
"fire anyway" (a press scheduled on the rise fires even if released during
the delay; the release is clamped to stay after the press); toggle (press
latches the output on, next press off). Zero-delay bindings stay identical.

Macros (ScOutput.Macro): repeated same-type activator blocks = a sequence
of commands, played ONCE on press (one-shot; hold/release do not affect it).
Each command's outputs are pressed together (a chord); commands run in order
framed by their delay_start/delay_end plus a small step/gap. Supports
key/mouse/gamepad sub-outputs (gamepad via a per-frame overlay OR'd into gp;
gp.buttons is now reset each frame so overlaid bits clear on macro end).
Importer treats duplicate activator keys as a list; a single command with
multiple bindings stays a held combo (preserves Alt+Tab).

Also: fix pad-joystick output_joystick mapping to the confirmed schema
(1=Left, 2=Right; absent -> RIGHT/camera default).

Editor: a macro seeds as INHERIT (preserved) via advancedDesc; authoring UI
for delays/toggle/macros is a later wave. Per-binding delay/toggle on a
gamepad LEVEL button is deferred (separate code path).

Tests: MacroDelayTest (delay defer, fire-anyway tap, toggle latch, macro
order + one-shot) + importer decode (macro, per-binding settings, combo
preserved).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
absolute_mouse is Steam's legacy name for the standard RELATIVE trackpad mouse (As-Mouse settings, no region); only mouse_region is the absolute region. The 2026-07-02 region work wrongly mapped both to PadMode.AbsoluteMouse, so ToME4's right pad (absolute_mouse) flipped from relative mouse to absolute warp - broke in-game and showed as INHERIT in the editor (AbsoluteMouse is not editor-authorable). Split them: mouse_region -> AbsoluteMouse, absolute_mouse -> relative Mouse. Tests updated.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
… stick + regression-guard test

Audit of every mode-string mapping (stick/pad/gyro) for the class of bug that made absolute_mouse an absolute region. Found the same class: mouse_joystick on a STICK dropped to None (else branch) - a shared/referenced group carrying the pad name on a stick source would vanish. Now stick accepts joystick_mouse OR mouse_joystick -> StickMode.Mouse; pad accepts both too. Added a regression-guard test locking the full mouse family in both contexts (only mouse_region is absolute).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…ck, pad mouse_joystick velocity

A5: gyro aim now uses the natural direction by default (yaw-right->aim-right; the raw sign felt backwards on device) + per-axis invertX/invertY on GyroMode.Mouse/Joystick. New GyroGate.LEFT/RIGHT_STICK_TOUCH (touch-to-aim on a stick). gyro_to_joystick reads output_joystick (1=L/2=R) for the target stick. New PadMode.MouseJoystick: pad = self-centering joystick that drives the mouse (displacement from center = cursor velocity), replacing the relative-mouse approximation for mouse_joystick/joystick_mouse. Editor preserves it (INHERIT). Tests + a parse-verified device config (sc_gyro_mousejoy_test.vdf).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
JonahSimon and others added 18 commits July 7, 2026 09:34
Add GyroActivation {ENABLE, SUPPRESS, TOGGLE} to GyroMode.Mouse/Joystick — what
the gyro gate button DOES:
- ENABLE (default, unchanged): aim while the gate is held.
- SUPPRESS: aim unless the gate is held.
- TOGGLE: each gate press flips aim on/off (hands-free + ratcheting: toggle off,
  recenter, toggle on — the workflow the user asked for).

Interpreter gyroActive() combines gate + activation with a persistent toggle
state (press-edge flip). Importer reads Steam `gyro_button` (provisional map
1=Enable/2=Suppress/3=Toggle, ponytail-flagged — captures show 1/2/6, none a
labeled toggle export; the mechanism is validated on-device regardless). Editor
EditGyro round-trips the activation. 3 unit tests (toggle edge, suppress, import).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Labeled Steam exports (testGyroEnable/Suppress/Toggle, controlled same-group
diff) confirm the Enable/Suppress/Toggle mode is `gyro_button_invert`, not
`gyro_button` (which reads "1" for all three modes — not the mode at all, as the
user pointed out). Map: absent=Enable, "0"=Suppress, "2"=Toggle (value 1
unobserved). The button chord stays `gyro_ratchet_button_mask` (the gate).

Importer + unit test corrected; toggle test config uses gyro_button_invert "2".

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Feedback: the in-game Gyro dialog was missing the Enable/Suppress/Toggle mode
picker, and the gate shouldn't be grip-only.

- GyroPickerDialog: add a "Gyro mode" dropdown (hold-to-enable / hold-to-suppress
  / toggle) and expand the "Gyro button" gate dropdown to any single button
  (grips, L4/L5/R4/R5 paddles, bumpers, A/B/X/Y, L3/R3, pad/stick touches,
  any/all touch). Both now show for JOYSTICK gyro too (was mouse-only).
- GyroGate enum + interpreter gyroGateOpen gain the single-button gates.
- Summary row shows mode + gate.

Follow-up: arbitrary multi-button CHORD gate needs a mask-based gate + multi-
select UI; single-button covers the common case. 1 unit test (R4 paddle gate).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
"Everything in Gyro" except the 7 conversion styles (deferred to next session).

Gyro-mouse feel set (GyroMode.Mouse): H/V mixer (gyro_vertical_horizontal_ratio),
speed deadzone (gyro_speed_deadzone, raw), precision speed (gyro_precision_speed,
raw), acceleration (acceleration 0..3 -> GyroAccel Off/Linear/Relaxed/Aggressive).
Importer + interpreter (speed-gated, precision-scaled, accel-boosted, axis-mixed)
+ editor sliders/dropdown.

Gyro-joystick shaping (GyroMode.Joystick): power curve (gyro_to_joystick_power_curve
/100), output range (min/max), lock-at-edges. shapeJoy helper; importer (power
curve) + interpreter + editor.

Joystick editor authoring: the Gyro dialog now edits deflection-vs-camera style +
output stick (were model-only). EditGyro round-trips all new fields losslessly.

6 new unit tests. Deferred (need device tuning / a capture): Lean L/R, input-speed
range, response-axis Circular, momentum, angle-catch-up, drag-center, relative
roll/pitch, exact Dots-per-360.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Stuck cursor: the pad-mouse nav cursor (white dot) is only driven while a
menu/editor captures, but nothing removed it when capture ended, so it froze
centre-screen after closing the QuickMenu. Interpreter now fires
uiBridge.hideCursor() on the capture falling edge; the bridge also detaches on
any out-of-capture move. Unit test (QuickMenuNavTest).

Header reorg (user UX feedback — header was too big): drop the title + long
description from the fixed header (description now behind a Y: Help dialog);
action-set chips stay pinned at top; move the Set name / Layer / Default / Delete
row into the top of the scroll list so it scrolls away. Nav indexing unchanged
(chips=line 0, set-name row=line 1, sources=line 2+).

Deferred (styling pass / chord build): real cursor glyph vs the white dot;
gate dropdown missing dpad / stick-touch / special buttons.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
The SC isn't an Android input device, so during menu/editor capture only the
interpreter's nav vocabulary reaches Compose. Y wasn't in it, so the "Y: Help"
button did nothing (Y was mapped to the game binding, e.g. F). Add ScNavKey.HELP:
handleMenuNav sends it on BTN_Y, the bridge dispatches KEYCODE_BUTTON_Y, and the
bindings editor's onPreviewKeyEvent opens the help dialog. Unit test.

Backlog (user): formalize d-pad/A/B/LB/RB/Y into a standard NON-editable
QuickMenu-nav config so tooltips are accurate across all controllers (docs).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Add a branch section to the fork README so anyone viewing the
steam-controller-2026 branch / PR sees what it adds (full wireless SC support:
BLE GATT, no dongle/root, Steam-Input-style engine, in-game editor), how it's
isolated from GameNative's existing controller code, and where the design docs
live. Upstream content unchanged below it.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…BLE-only transport

Batch of the 2026-07-09..07-12 sessions (all validated on-device):

- Auto-save extended to EVERY editor (binding picker + Sticks/Pads,
  Triggers, Gyro, Haptics): commit on B/scrim, Apply/Cancel rows and
  NavApplyCancelRow removed. Trigger full-pull clicks folded into the
  trigger editors; digital button section renamed "Bumpers".
- Full styling pass to the bindings-menu visual spec: ScRootMenuDialog
  hub (gradient ring, purple select, oval Back), flat surface cards,
  LabeledDropdown row style, ScSectionHeader, themed On/Off chip, one
  ScScrollbar everywhere. Chip language: solid purple = bound, hollow
  white = option, rotating ring = nav cursor.
- Overlay placement editor redesigned as direct manipulation: left
  stick moves, RT/LT resize (new ScNavKey.ZOOM_IN/OUT plumbing), LB/RB
  cycle real menus one by one with actual-kind preview, Y resets,
  semi-transparent control bar, auto-saves.
- QuickMenu nav formalized in ScMenuNav (fixed d-pad/A/B/LB/RB/Y/Start
  table); merge-induced XServerScreen VerifyError fixed by moving the
  diagnostics param to PrefManager.
- Tuning folded into the per-surface editors; standalone ScTuningDialog
  and the global menu-commit override deleted.
- USB transport cut: TritonUsb + both USB self-tests deleted,
  TritonMapper is BLE-only (BLE wires its own haptics).
- Ponytail cuts (dead ScScrollbarColumn, ScActionChip.enabled), shared
  Modifier.scCaptureFocus(), d-pad key handling routed through
  ScNavDialogColumn; menu-HUD slot text scales to the cell; Slots list
  row inset; unused imports tidied.

Co-Authored-By: Claude Fable 5 <[email protected]>
…2026

# Conflicts:
#	app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
A raw BLE Steam Controller is not an Android input device, so GameNative's
rumble poller had no controller vibrator to target and fell back to buzzing
the phone. Route the game's XInput rumble to the SC's rumble motors instead.

- TritonHaptics.rumble()/rumbleReport(): builds the Triton rumble output
  report (id 0x80, 10-byte packed LE MsgHapticRumble — left=low-freq motor,
  right=high-freq motor, 0 dB gain), matching SDL's triton driver. Routes
  over BLE through the existing writeOutputReport char lookup (0x80 -> 6CB5).
- WinHandler: optional static RumbleForwarder tap at startVibration/
  stopVibration. Null (default) = unchanged legacy behavior; set only while a
  TritonMapper session is live, in which case game rumble goes to the SC and
  the phone/controller fallback is skipped.
- TritonMapper: registers the forwarder, marshals it onto the main looper,
  and resends a held rumble every 40ms (firmware stops the motors ~50ms after
  the last report). Cleared on stop().

HW-verify next session (motor report format is from SDL, not yet driven on
our hardware): confirm the motors buzz and tune magnitude if needed.

Co-Authored-By: Claude Fable 5 <[email protected]>
The upstream merge collided on QuickMenuAction 9. Upstream is the source of
truth for its own numbering, so keep SHOOTER_MODE = 9 and move our
SC_BINDINGS to 10 (SC_LAYOUT/SC_ROOT already 12/13). All values referenced
by name only; no persisted ints.

Co-Authored-By: Claude Fable 5 <[email protected]>
The BLE bring-up harness has done its job (feature validated on hardware),
so remove it rather than ship it in release builds:
- Delete ScTestReceiver, TritonBleSelfTest, TritonBleEngineSelfTest,
  TritonBleCapture and the src/debug AndroidManifest (SC_SELFTEST receiver +
  debug-only MANAGE_EXTERNAL_STORAGE — the latter was unrelated to the SC
  feature).
- Revert the SC self-test buttons in SettingsGroupDebug.
- Remove the now-dead debug hooks the harness reached through: TritonMapper
  .live / .setTuning / .armPadProbe and TritonBle's rest-jitter pad probe.
  The live in-game tuning path (reload -> ProfileInterpreter.setPadTuning)
  is untouched.
- Relocate the tests' shared SMOKE_CONFIG fixture out of the deleted
  TritonBleEngineSelfTest into the test source set (ScTestSupport).

Net -440 lines. Live transport never depended on any of it (the coupling
was one-way: debug UI -> driver). Build + full SC/importer suite green.

Co-Authored-By: Claude Fable 5 <[email protected]>
Not part of the Steam Controller feature — restore it to match main so the
PR doesn't carry an unrelated email-format change.

Co-Authored-By: Claude Fable 5 <[email protected]>
Keep the PR strictly about the Steam Controller feature: these 7 imports
(PrefManager PulseAudioComponent; PluviaMain TarCompressorUtils/ImageFs;
MainViewModel Orientation/EnumSet; QuickMenu Spring/spring) were already
unused in upstream and got swept up by an earlier import-tidy pass. Put them
back so our diff to these files is only our functional changes (the
VerifyError-fix diagnostics reroute + the SC-live QuickMenu gate).

Co-Authored-By: Claude Fable 5 <[email protected]>
@JonahSimon JonahSimon requested a review from utkarshdalal as a code owner July 12, 2026 21:08
@cubic-dev-ai

cubic-dev-ai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

This PR is large and would use a significant portion of your monthly review quota. Comment @cubic-dev-ai review this to confirm that you want cubic to review it.

@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds direct BLE Steam Controller support with protocol decoding, profile interpretation, configurable action sets, menus, keyboard input, haptics, VDF importing, persistent configurations, Compose editors, overlays, and QuickMenu/XServer integration.

Changes

Steam Controller platform and runtime

Layer / File(s) Summary
BLE transport and reports
app/src/main/java/app/gamenative/steamcontroller/*, AndroidManifest.xml, WinHandler.java, InputDeviceManager.java
Adds BLE connection, protocol decoding, haptics, rumble forwarding, permissions, and guest-suspension handling.
Profiles and interpreter
steamcontroller/ScProfile.kt, ProfileInterpreter.kt, TritonMapper.kt
Adds profile models, action sets, layers, activators, menus, gyro/pad mappings, output sinks, and runtime state translation.
Configuration and overlays
ScConfigStore.kt, ScTuningStore.kt, ScOverlayStore.kt, ScMenuLabels.kt
Adds named per-game configs, VDF overlays, authored configs, tuning persistence, menu labels, and overlay placement storage.
Keyboard and menu HUDs
ScKeyboard.kt, ScKeyboardOverlayView.kt, ScMenuOverlay.kt, ScMenuOverlayView.kt
Adds controller-driven keyboard and radial/grid menu overlays with cursor, highlight, fade, toast, and haptic behavior.
Compose editor experience
ui/component/dialog/*
Adds controller navigation primitives and dialogs for bindings, analog modes, gyro, haptics, labels, configs, keyboard input, and overlay layout.
Application wiring
XServerScreen.kt, QuickMenu.kt, MainViewModel.kt, PrefManager.kt
Starts and stops the BLE mapper, exposes live Steam Controller editor navigation, reloads changes, and persists diagnostics configuration.
Steam VDF importer
SteamControllerVdfUtils.kt, app/src/test/resources/sc/*
Imports Steam Input profiles, action sets, layers, mode shifts, analog modes, activators, and bindings, with broad fixture coverage.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Suggested reviewers: utkarshdalal

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.71% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and accurately summarizes the main change: adding full wireless Steam Controller support over BLE.
Description check ✅ Passed The description includes the required sections and provides clear details on what changed, why, recording, type, and checklist items.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@JonahSimon JonahSimon left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left inline notes on the key/non-obvious lines to make review easier. High level: the whole SC input path is parallel and never becomes an Android input device, so the only edits to existing files are the two seams and one small QuickMenu gate flagged below.

Log.i(TAG, "connected (status=$status); requesting high priority + MTU $TRITON_MTU")
g.requestConnectionPriority(BluetoothGatt.CONNECTION_PRIORITY_HIGH)
// MUST raise the MTU before service discovery, or the 45-byte report can't be delivered.
if (!g.requestMtu(TRITON_MTU)) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one non-obvious transport fix. The 45-byte input report exceeds Android's default 23-byte MTU, so no reports arrive at all until this raises it to 517 — and it must happen before service discovery. iOS auto-negotiates a large MTU; Android doesn't, which is why this was the last blocker to streaming.

* (report 0x82 → 6CB7). No-op until the output chars are discovered. Uses WRITE_NO_RESPONSE so frequent
* detent ticks stay cheap. Serialized through [opQueue] like every other GATT op.
*/
fun writeOutputReport(report: ByteArray) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Output reports (haptics/rumble) are USB-style [id, payload...]. Over BLE the id selects a per-id characteristic (haptics 0x826CB7, rumble 0x806CB5) and is dropped over the air. WRITE_NO_RESPONSE keeps frequent detent ticks cheap.

* right (small / high-freq motor). We map XInput low/high-freq magnitudes straight to the motor speeds
* with 0 dB gain, exactly as SDL does.
*/
fun rumbleReport(lowFreq: Int, highFreq: Int): ByteArray = ByteArray(10).also {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rumble wire format (report 0x80, packed little-endian) is taken from SDL's Triton driver — left motor = low-freq, right = high-freq, 0 dB gain. Note the trackpad click/detent feel is also regenerated in this class (report 0x82), because disabling lizard mode to read raw reports turns off the firmware's own click-feel.

// firmware's ~50ms cutoff; it stops itself once rumble returns to zero.
@Volatile private var rumbleLow = 0
@Volatile private var rumbleHigh = 0
private val rumbleResend = object : Runnable {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The firmware cuts the motors ~50 ms after the last rumble report, but GameNative's rumble poller only fires on change — so a held rumble is resent every 40 ms here (matches SDL's resend interval). It stops itself once rumble returns to zero.

private var navCursorTickAccum = 0f
private var prevCapturing = false

fun apply(s: TritonState) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per-report entry point: every decoded TritonState flows through here → bindings / modes / activators / layers → ScOutputSink. This entire engine is unit-tested headlessly by replaying a captured input trace, so most of the logic is verifiable with no device.

}

private void startVibration(short lowFreq, short highFreq) {
// A live Steam Controller owns rumble output (its own motors); skip the phone/controller fallback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tap itself: when an SC owns rumble, forward and return before the phone/controller fallback. Entirely guarded by the forwarder being non-null → zero behavior change when no SC is present.

* re-arm itself (it re-posts while the key stays in {@code keyboard.getPressedKeys()}). Pure local state — sends
* no X events. Call BEFORE suspending the guest.
*/
public void onGuestSuspended() {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isolation seam #2 (also a latent base-app fix). When the guest is suspended (the editor/overlay SIGSTOPs it) it stops draining its X socket; left running, the next key auto-repeat → blocking ClientSocket.write would hang the UI thread into an ANR. This is pure local state and sends no X events — safe to call before suspending regardless of the SC feature.

)
if (hasPhysicalController) {
// The generic gamepad mapper doesn't apply to the BLE Steam Controller — hide it when an SC is live.
if (hasPhysicalController && !isSteamControllerLive) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only change to existing logic in this file: hide the generic gamepad mapper entry when a Steam Controller is live (the BLE Triton isn't an Android input device, so that mapper doesn't apply to it). The SC's own editor entry is added just above, gated on isSteamControllerLive.

// context instead of threading it as a parameter through the (very large) XServerScreen composable — adding a
// parameter there pushed the method past ART's bytecode-verifier limit (VerifyError on launch). See MainViewModel.
private val WRAPPER_DIAGNOSTICS = booleanPreferencesKey("wrapper_diagnostics")
var wrapperDiagnostics: Boolean

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this indirection exists: adding a parameter to the very large XServerScreen composable pushed it past ART's bytecode-verifier method-size limit (a VerifyError that crashed all launches). Persisting the diagnostics flag here and reading it in setupXEnvironment avoids threading a new parameter through that method. Unrelated to the SC feature per se — surfaced by the merge that grew the composable.

* (what `CHANGE_PRESET` / [ScOutput.SwitchActionSet] target), plus the launch set. Feed this to
* [app.gamenative.steamcontroller.ProfileInterpreter] for config-driven action-set switching.
*/
fun importConfig(vdfText: String): ScConfig {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The coverage guarantee: parses any Steam Input .vdf (reusing GameNative's own VDF parser) into the profile model. Exercised against real worst-case Steam exports in the test fixtures (e.g. the ~10k-line KSP config) so the parser isn't just tested on toy inputs.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
app/src/test/java/app/gamenative/steamcontroller/HapticsRumbleTest.kt (1)

1-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Good rumble-packing coverage; consider adding tests for the touch/slide detent logic.

These tests solidly pin down rumbleReport's byte layout, but TritonHaptics.update/handlePad (rising-edge click detection, jitter-filtered slide accumulation, detent-tick firing) is untested here. That's the more stateful, bug-prone part of this class (e.g. edge-detection math, accumulator reset/carry behavior) and would benefit from a few TritonState-driven scenarios (touch-down click, incremental slide crossing detentStep, jitter below moveNoise being ignored).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/test/java/app/gamenative/steamcontroller/HapticsRumbleTest.kt` around
lines 1 - 37, Add TritonState-driven tests covering
TritonHaptics.update/handlePad: verify a touch-down rising edge triggers the
click, incremental slide movement fires detent ticks when the accumulated
movement crosses detentStep, and movement below moveNoise is ignored. Include
assertions for accumulator reset/carry behavior while preserving the existing
rumbleReport tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/src/main/java/app/gamenative/steamcontroller/TritonProtocol.kt`:
- Around line 68-87: Update the length guard in decodeState to reject buffers
shorter than 46 bytes, since decodeFrom(buf, 1) accesses through index 45. Leave
decodeBleState’s existing 45-byte guard unchanged.

In `@app/src/main/java/app/gamenative/utils/SteamControllerVdfUtils.kt`:
- Around line 809-820: Update readCurve so curve_exponent is interpreted as its
stored setting value rather than passed to ResponseCurve.values() as an ordinal.
Explicitly map supported values such as 1, 2, and 4 to their intended
ResponseCurve values, preserving LINEAR as the fallback for unknown or invalid
values and retaining the existing custom_curve_exponent handling.

In `@app/src/test/java/app/gamenative/steamcontroller/ScKeyboardTest.kt`:
- Around line 80-97: Update the test method `special keys fire space backspace
enter and close dismisses` to exercise and assert the Space key, matching the
existing Enter and Backspace press/release flow and verifying
`XKeycode.KEY_SPACE`; keep the Close dismissal assertion unchanged.

In `@app/src/test/java/app/gamenative/steamcontroller/ScTestSupport.kt`:
- Around line 90-128: Update the SMOKE_CONFIG controller_action targets to match
the documented behavior: make Default’s right_bumper select the Combat preset,
make Combat’s right_bumper return to Default, and change the left_bumper
hold_layer target to the declared Overlay layer. Keep the existing preset and
layer declarations unchanged.

---

Nitpick comments:
In `@app/src/test/java/app/gamenative/steamcontroller/HapticsRumbleTest.kt`:
- Around line 1-37: Add TritonState-driven tests covering
TritonHaptics.update/handlePad: verify a touch-down rising edge triggers the
click, incremental slide movement fires detent ticks when the accumulated
movement crosses detentStep, and movement below moveNoise is ignored. Include
assertions for accumulator reset/carry behavior while preserving the existing
rumbleReport tests.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b997c21c-dbc5-4ec9-bc70-7fba1ec56fff

📥 Commits

Reviewing files that changed from the base of the PR and between aaeafe7 and 6ebc6d9.

⛔ Files ignored due to path filters (1)
  • app/src/test/resources/sc/golden_ble_001.bin is excluded by !**/*.bin
📒 Files selected for processing (81)
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/app/gamenative/PrefManager.kt
  • app/src/main/java/app/gamenative/steamcontroller/ProfileInterpreter.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScKeyboard.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScKeyboardOverlayView.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScMenuLabels.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScMenuNav.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScMenuOverlay.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScMenuOverlayView.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScOutputSink.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScOverlayStore.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScProfile.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScProfileEditor.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScTuningStore.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScUiBridge.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonBle.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonHaptics.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonMapper.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonProtocol.kt
  • app/src/main/java/app/gamenative/ui/PluviaMain.kt
  • app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ControllerTab.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScConfigManagerDialog.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScCursorController.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScEditorStyle.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScInputGlyph.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScListNav.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScMenuLabelEditor.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScNavChoiceDialog.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScNavDialogStack.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScOverlayEditor.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScRootMenuDialog.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScScrollbar.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScSelectionRing.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScTextEditField.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/SteamControllerBindingEditor.kt
  • app/src/main/java/app/gamenative/ui/model/MainViewModel.kt
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
  • app/src/main/java/app/gamenative/utils/SteamControllerVdfUtils.kt
  • app/src/main/java/com/winlator/winhandler/WinHandler.java
  • app/src/main/java/com/winlator/xserver/InputDeviceManager.java
  • app/src/main/res/values/strings.xml
  • app/src/test/java/app/gamenative/steamcontroller/ActionLayersTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ActionSetsTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ActivatorsTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/AdvancedModesTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/EditableConfigTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/HapticsRumbleTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/MacroDelayTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/MenuModesTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ModeShiftTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/PadModesTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ProfileInterpreterTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/QuickMenuNavTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScConfigStoreRegistryTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScEditableAnalogTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScKeyboardTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScMenuLabelToolTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScOverlayStoreTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScProfileEditorTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScTestSupport.kt
  • app/src/test/java/app/gamenative/steamcontroller/TriggerModesTest.kt
  • app/src/test/java/app/gamenative/utils/SteamControllerProfileImporterTest.kt
  • app/src/test/resources/sc/actionlayers_v3.vdf
  • app/src/test/resources/sc/actionsets_v3.vdf
  • app/src/test/resources/sc/chord_triton.vdf
  • app/src/test/resources/sc/controller_xboxone_wasd.vdf
  • app/src/test/resources/sc/delf_tome4_neptune.vdf
  • app/src/test/resources/sc/doom_sc_test.vdf
  • app/src/test/resources/sc/gamepad_joystick.vdf
  • app/src/test/resources/sc/kitchensink_v3.vdf
  • app/src/test/resources/sc/ksp_worstcase_xboxone.vdf
  • app/src/test/resources/sc/modeshift_v3.vdf
  • app/src/test/resources/sc/sc_bindsettings_test.vdf
  • app/src/test/resources/sc/sc_gyro_mousejoy_test.vdf
  • app/src/test/resources/sc/sc_gyro_touch_test.vdf
  • app/src/test/resources/sc/sc_newmodes_test.vdf
  • app/src/test/resources/sc/sc_region_single_test.vdf
  • app/src/test/resources/sc/sc_touch_bind_test.vdf
  • app/src/test/resources/sc/testgyro_gate_capture.vdf
💤 Files with no reviewable changes (1)
  • app/src/main/java/app/gamenative/ui/PluviaMain.kt

Comment on lines +68 to +87
/**
* Decode a raw USB interrupt report (64-byte; `buf[0]`=report type, `buf[1]`=seq) into a TritonState,
* or null if it isn't a controller-state report.
*/
fun decodeState(buf: ByteArray, len: Int): TritonState? {
if (len < 45) return null
if (!isStateReport(buf[0].toInt() and 0xFF)) return null
return decodeFrom(buf, 1) // payload (seq) starts at buf[1]
}

/**
* Decode a BLE GATT input-characteristic value. The BLE transport delivers the SAME `TritonMTUNoQuat`
* payload but **without** the leading USB report-type byte, so seq is at offset 0 (buttons at offset 1).
* Confirmed vs CollinKite/SteamControllerKit (docs/BLE-GATT-PATH.md).
*/
fun decodeBleState(buf: ByteArray, len: Int): TritonState? {
// Need indices 0..44 (gyroZ s16 at offset 43-44). BLE value arrays are exact-size, so guard >= 45.
if (len < 45) return null
return decodeFrom(buf, 0) // payload (seq) starts at offset 0
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether TritonProtocol.decodeState (USB path) has any callers in the repo.
rg -n --type=kt 'TritonProtocol\.decodeState\('

Repository: utkarshdalal/GameNative

Length of output: 2067


Fix the USB length guard in decodeState. decodeFrom(buf, 1) reads through index 45, so len < 45 still allows a 45-byte buffer to reach an out-of-bounds access. Bump this to len < 46.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/app/gamenative/steamcontroller/TritonProtocol.kt` around
lines 68 - 87, Update the length guard in decodeState to reject buffers shorter
than 46 bytes, since decodeFrom(buf, 1) accesses through index 45. Leave
decodeBleState’s existing 45-byte guard unchanged.

Comment thread app/src/main/java/app/gamenative/utils/SteamControllerVdfUtils.kt
Comment thread app/src/test/java/app/gamenative/steamcontroller/ScKeyboardTest.kt
Comment on lines +90 to +128
/**
* Mode-shift / layer / preset config used by the importer + config-store unit tests. Press map:
* A/B -> keys 1/2 in the base set; hold LB -> Overlay layer (A -> 8); tap RB -> Combat preset (A -> 3);
* hold the lower-left back paddle -> mode-shift (A -> 9). (Formerly TritonBleEngineSelfTest.SMOKE_CONFIG,
* relocated to the test source set when the on-device self-test harness was removed.)
*/
const val SMOKE_CONFIG = """
"controller_mappings"
{
"version" "3"
"title" "Engine Smoke Test"
"controller_type" "controller_triton"
"actions" { "Default" { "title" "Default" "legacy_set" "1" } "Combat" { "title" "Combat" "legacy_set" "1" } }
"action_layers" { "Overlay" { "title" "Overlay" "legacy_set" "1" "set_layer" "1" "parent_set_name" "Default" } }

"group" { "id" "0" "mode" "four_buttons" "inputs" {
"button_a" { "activators" { "Full_Press" { "bindings" { "binding" "key_press 1" } } } }
"button_b" { "activators" { "Full_Press" { "bindings" { "binding" "key_press 2" } } } } } }
"group" { "id" "1" "mode" "four_buttons" "inputs" {
"button_a" { "activators" { "Full_Press" { "bindings" { "binding" "key_press 9" } } } } } }
"group" { "id" "2" "mode" "switches" "inputs" {
"left_bumper" { "activators" { "Full_Press" { "bindings" { "binding" "controller_action hold_layer 3 0 0" } } } }
"right_bumper" { "activators" { "Full_Press" { "bindings" { "binding" "controller_action CHANGE_PRESET 2 0 0" } } } }
"button_back_left" { "activators" { "Full_Press" { "bindings" { "binding" "mode_shift button_diamond 1" } } } } } }
"group" { "id" "3" "mode" "four_buttons" "inputs" {
"button_a" { "activators" { "Full_Press" { "bindings" { "binding" "key_press 3" } } } } } }
"group" { "id" "4" "mode" "switches" "inputs" {
"right_bumper" { "activators" { "Full_Press" { "bindings" { "binding" "controller_action CHANGE_PRESET 1 0 0" } } } } } }
"group" { "id" "5" "mode" "four_buttons" "inputs" {
"button_a" { "activators" { "Full_Press" { "bindings" { "binding" "key_press 8" } } } } } }

"preset" { "id" "0" "name" "Default" "group_source_bindings" {
"0" "button_diamond active" "2" "switch active" "1" "button_diamond active modeshift" } }
"preset" { "id" "1" "name" "Combat" "group_source_bindings" {
"3" "button_diamond active" "4" "switch active" } }
"preset" { "id" "2" "name" "Overlay" "group_source_bindings" {
"5" "button_diamond active" } }
}
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how the importer resolves controller_action hold_layer / CHANGE_PRESET numeric targets,
# and find every test that consumes SMOKE_CONFIG to see what behavior it actually asserts.
fd SteamControllerVdfUtils.kt --exec cat -n {}
rg -n "hold_layer|add_layer|remove_layer|CHANGE_PRESET|legacy_set" app/src/main/java/app/gamenative/utils/SteamControllerVdfUtils.kt -A5 -B5
rg -n "SMOKE_CONFIG" -g '*.kt' -A3 -B3

Repository: utkarshdalal/GameNative

Length of output: 2067


🌐 Web query:

Steam controller VDF controller_action CHANGE_PRESET hold_layer numeric target indexing preset id declaration order

💡 Result:

In Steam Input VDF configuration files, controller_action commands such as CHANGE_PRESET and hold_layer are utilized to manipulate action sets and layers [1][2][3]. CHANGE_PRESET Format The syntax for CHANGE_PRESET within a binding is: controller_action CHANGE_PRESET [preset_id] [flag1] [flag2] - [preset_id]: This numeric value corresponds to the ID of the action set or layer you wish to switch to [1][4]. - [flag1] and [flag2]: These are typically set to 1 or 0, representing boolean flags that often control whether the transition is immediate or handles state persistence [1][2][5][4]. While official documentation is limited, community analysis and VDF files frequently show these as 1 1 or 0 0 in operational configurations [2][5][4]. hold_layer Format The syntax for hold_layer is: controller_action hold_layer [layer_id] [flag1] [flag2] - [layer_id]: The numeric ID of the action layer to be activated while the input is held [3][6]. - [flag1] and [flag2]: Similar to CHANGE_PRESET, these parameters accompany the command [3][6]. Declaration and Indexing - VDF Structure: Steam Input configurations (VDFs) are Valve Data Files, which use a hierarchical, whitespace-separated key-value structure rather than JSON [7]. - Preset/Group Declaration: Action sets and layers are defined within the controller_mappings block. Groups, which contain the bindings, are indexed by unique ID strings (e.g., "id" "13") [1][4]. - Mapping Consistency: When creating or merging complex configurations, it is critical to ensure that the IDs referenced in a binding (like those in CHANGE_PRESET) correctly match the "id" defined in the corresponding group or preset block [8]. Misaligned IDs will cause the action to fail or trigger unexpected behavior [7][8]. Because Steam Input VDFs are interpreted by the Steam client, the order of declaration is generally determined by the internal parsing of the configuration file structure; however, maintaining logical grouping—where definitions precede the bindings that reference them—is best practice for compatibility [7][8].

Citations:


SMOKE_CONFIG's controller_action targets don't match the doc comment.

  • Default's right_bumper uses CHANGE_PRESET 2, which selects Overlay, not Combat.
  • Combat's right_bumper self-targets with CHANGE_PRESET 1 instead of returning to Default.
  • hold_layer 3 also doesn't line up with the single declared Overlay layer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/test/java/app/gamenative/steamcontroller/ScTestSupport.kt` around
lines 90 - 128, Update the SMOKE_CONFIG controller_action targets to match the
documented behavior: make Default’s right_bumper select the Combat preset, make
Combat’s right_bumper return to Default, and change the left_bumper hold_layer
target to the declared Overlay layer. Keep the existing preset and layer
declarations unchanged.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/steamcontroller/TritonProtocol.kt (1)

68-131: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Off-by-one length guard in decodeState is still unresolved.

decodeFrom(buf, 1) reads through buf[45] (gyroZ at p+43 = offset 44, spanning bytes 44-45), so the len < 45 guard still lets a 45-byte report through and reads one byte past the guaranteed range. decodeBleState is correctly guarded (p=0, max index 44), but decodeState needs len < 46.

🐛 Proposed fix
     fun decodeState(buf: ByteArray, len: Int): TritonState? {
-        if (len < 45) return null
+        if (len < 46) return null
         if (!isStateReport(buf[0].toInt() and 0xFF)) return null
         return decodeFrom(buf, 1)  // payload (seq) starts at buf[1]
     }

Separately, the PR objectives state USB transport was removed before submission, yet this file still carries the USB-only decodeState/settingReport/lizardOffReport/imuEnableReport. Worth confirming these still have live callers (e.g., in TritonBle.kt) — if not, they're dead code that should be dropped along with the rest of the USB path.

#!/bin/bash
rg -n 'TritonProtocol\.decodeState\(|TritonProtocol\.lizardOffReport\(|TritonProtocol\.imuEnableReport\(|TritonProtocol\.settingReport' -g '*.kt' -g '*.java'
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/app/gamenative/steamcontroller/TritonProtocol.kt` around
lines 68 - 131, Update TritonProtocol.decodeState to require len >= 46 before
calling decodeFrom, while leaving decodeBleState’s 45-byte guard unchanged.
Audit callers of decodeState, settingReport, lizardOffReport, and
imuEnableReport; if none remain after USB transport removal, remove these unused
USB-only APIs and their supporting code.
🧹 Nitpick comments (4)
app/src/test/java/app/gamenative/steamcontroller/ActionLayersTest.kt (1)

18-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Centralize the shared test resource loader in ScTestSupport.kt.
ActionLayersTest.kt, ActionSetsTest.kt, MenuModesTest.kt, and ModeShiftTest.kt all define the same load() helper. ScTestSupport.kt already houses shared Steam Controller test utilities, so adding a common text-resource loader there would remove duplication.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/test/java/app/gamenative/steamcontroller/ActionLayersTest.kt` around
lines 18 - 21, Move the duplicated load(name: String) helper from
ActionLayersTest, ActionSetsTest, MenuModesTest, and ModeShiftTest into
ScTestSupport.kt as a shared text-resource loader. Update each test to use the
centralized helper and remove its local implementation, preserving the existing
sc/ resource lookup, UTF-8 decoding, and missing-resource error behavior.
app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt (1)

80-83: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Non-atomic manifest write can corrupt the registry under live autosave.

writeText truncates then rewrites in place; a crash/kill mid-write leaves a truncated/invalid <key>.configs.json. Given the PR adds live autosave/reload, consider writing to a temp file and atomically renaming over the manifest.

♻️ Atomic write via temp + rename
     private fun writeRegistry(context: Context, key: String, reg: ScConfigRegistry): Boolean =
-        runCatching { manifestFile(context, key).writeText(json.encodeToString(ScConfigRegistry.serializer(), reg)); true }
+        runCatching {
+            val target = manifestFile(context, key)
+            val tmp = File(target.parentFile, "${target.name}.tmp")
+            tmp.writeText(json.encodeToString(ScConfigRegistry.serializer(), reg))
+            if (!tmp.renameTo(target)) { target.writeText(tmp.readText()); tmp.delete() }
+            true
+        }
             .onFailure { Log.w(TAG, "writeRegistry($key) failed: ${it.message}") }
             .getOrDefault(false)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt` around
lines 80 - 83, Update writeRegistry to write the serialized registry to a
temporary file in the same directory as manifestFile(context, key), then
atomically replace the manifest with the temp file using the platform file-move
API. Preserve the existing Boolean result and failure logging, and clean up the
temporary file if the operation fails.
app/src/main/java/app/gamenative/ui/component/dialog/ScSelectionRing.kt (1)

42-47: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff

Consider Modifier.Node for a per-item selection ring.

@Composable fun Modifier.scSelectionRing(...) is a Composable Modifier Factory — per Android's official guidance, such factories are never skipped and re-run on every recomposition of the caller, unlike a Modifier.Node implementation. If this ring is applied to many nav-list items, migrating to Modifier.Node would avoid re-executing this function body (and reallocating the sweep brush/paint/path) on every parent recomposition.

Not urgent given the animation work itself is already deferred to the draw phase — flagging as a nice-to-have.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/app/gamenative/ui/component/dialog/ScSelectionRing.kt`
around lines 42 - 47, Consider migrating the scSelectionRing modifier factory to
a Modifier.Node-based implementation so per-item selection rings avoid
recreating modifier-related drawing objects during parent recomposition.
Preserve the existing selected-state behavior, shape, width, and animation
timing while keeping animation work deferred to the draw phase.
app/src/main/java/app/gamenative/ui/component/dialog/ScScrollbar.kt (1)

150-167: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid launching a coroutine per drag-move event.

onDrag fires many times per gesture and each call spins up a new coroutine to invoke the suspending scrollTo. ScrollableState.dispatchRawDelta is the documented non-suspending API for direct pointer-driven scroll updates — it bypasses the scroll mutex/animation machinery entirely, avoiding the churn of overlapping coroutines during a drag.

♻️ Suggested refactor
                     .pointerInput(maxValue) {
                         detectDragGestures(
                             onDragStart = { dragProgress = scrollProgress; isDragging = true; isVisible = true },
                             onDragEnd = {
                                 isDragging = false
                                 scope.launch { delay(hideDelay); if (!isTouchScrolling) isVisible = false }
                             },
                             onDragCancel = {
                                 isDragging = false
                                 scope.launch { delay(hideDelay); if (!isTouchScrolling) isVisible = false }
                             },
                             onDrag = { change, dragAmount ->
                                 change.consume()
                                 val deltaProgress = dragAmount.y / maxOffset.coerceAtLeast(1f)
                                 dragProgress = (dragProgress + deltaProgress).coerceIn(0f, 1f)
-                                scope.launch { scrollState.scrollTo((dragProgress * maxValue).roundToInt()) }
+                                val target = (dragProgress * maxValue).roundToInt()
+                                scrollState.dispatchRawDelta((target - scrollState.value).toFloat())
                             },
                         )
                     },

Verify scroll direction/scaling on-device after the change since dispatchRawDelta semantics differ slightly from scrollTo.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/app/gamenative/ui/component/dialog/ScScrollbar.kt` around
lines 150 - 167, Update the onDrag handler inside the pointerInput(maxValue)
detectDragGestures block to avoid launching a coroutine for every drag event.
Replace the suspending scrollState.scrollTo call with the non-suspending
scrollState.dispatchRawDelta API, passing the drag delta scaled to the scroll
range while preserving dragProgress clamping; verify the resulting scroll
direction and scaling remain correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/src/main/java/app/gamenative/steamcontroller/ScProfile.kt`:
- Around line 94-108: The mergeProfiles function must distinguish explicitly
defined layer buttons from defaults supplied by ScProfile.default(). Add or
propagate an explicit defined-button mask for each layer, then merge button
bindings per bit using only layer-defined entries instead of unconditionally
unioning base.buttons with layer.buttons; preserve the existing source-based
merging for sticks, pads, triggers, gyro, and haptics.

In `@app/src/main/java/app/gamenative/steamcontroller/TritonMapper.kt`:
- Around line 43-46: Update TritonMapper’s shared-state handling for ble,
bleRetries, and interpreter: make cross-thread reads and writes safely visible
by marking the fields volatile or synchronizing every access. Cover BLE callback
updates, reconnect/main-handler reads, and interpreter access from reload(),
while preserving the existing behavior.

In `@app/src/main/java/app/gamenative/ui/component/dialog/ScOverlayEditor.kt`:
- Around line 74-88: Key the selectedMenu state in the ScOverlayEditor
composition by storeKey, so it is reinitialized from the current menuOptions
when the store changes. Keep the existing selectedMenu-based layout reload
behavior intact and ensure the new default selection cannot reference a menu
from the previous store.

In `@app/src/main/java/app/gamenative/utils/SteamControllerVdfUtils.kt`:
- Around line 1095-1102: Update the KEY_MAP used by keyOf() to add Steam’s exact
BACKSLASH and BACK_TICK aliases alongside the existing BACK_SLASH and
BACK_QUOTE/TILDE entries, mapping each to the same XKeycode values so direct
uppercase lookup resolves both spellings.

In `@app/src/main/java/com/winlator/winhandler/WinHandler.java`:
- Around line 676-679: Update the forwarder-switch path that assigns
scRumbleForwarder, including the setup in TritonMapper, to stop or cancel any
active local phone/controller vibration before enabling the forwarder. Preserve
startVibration’s forwarding behavior once scRumbleForwarder is available.

---

Duplicate comments:
In `@app/src/main/java/app/gamenative/steamcontroller/TritonProtocol.kt`:
- Around line 68-131: Update TritonProtocol.decodeState to require len >= 46
before calling decodeFrom, while leaving decodeBleState’s 45-byte guard
unchanged. Audit callers of decodeState, settingReport, lizardOffReport, and
imuEnableReport; if none remain after USB transport removal, remove these unused
USB-only APIs and their supporting code.

---

Nitpick comments:
In `@app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt`:
- Around line 80-83: Update writeRegistry to write the serialized registry to a
temporary file in the same directory as manifestFile(context, key), then
atomically replace the manifest with the temp file using the platform file-move
API. Preserve the existing Boolean result and failure logging, and clean up the
temporary file if the operation fails.

In `@app/src/main/java/app/gamenative/ui/component/dialog/ScScrollbar.kt`:
- Around line 150-167: Update the onDrag handler inside the
pointerInput(maxValue) detectDragGestures block to avoid launching a coroutine
for every drag event. Replace the suspending scrollState.scrollTo call with the
non-suspending scrollState.dispatchRawDelta API, passing the drag delta scaled
to the scroll range while preserving dragProgress clamping; verify the resulting
scroll direction and scaling remain correct.

In `@app/src/main/java/app/gamenative/ui/component/dialog/ScSelectionRing.kt`:
- Around line 42-47: Consider migrating the scSelectionRing modifier factory to
a Modifier.Node-based implementation so per-item selection rings avoid
recreating modifier-related drawing objects during parent recomposition.
Preserve the existing selected-state behavior, shape, width, and animation
timing while keeping animation work deferred to the draw phase.

In `@app/src/test/java/app/gamenative/steamcontroller/ActionLayersTest.kt`:
- Around line 18-21: Move the duplicated load(name: String) helper from
ActionLayersTest, ActionSetsTest, MenuModesTest, and ModeShiftTest into
ScTestSupport.kt as a shared text-resource loader. Update each test to use the
centralized helper and remove its local implementation, preserving the existing
sc/ resource lookup, UTF-8 decoding, and missing-resource error behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b997c21c-dbc5-4ec9-bc70-7fba1ec56fff

📥 Commits

Reviewing files that changed from the base of the PR and between aaeafe7 and 6ebc6d9.

⛔ Files ignored due to path filters (1)
  • app/src/test/resources/sc/golden_ble_001.bin is excluded by !**/*.bin
📒 Files selected for processing (81)
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/app/gamenative/PrefManager.kt
  • app/src/main/java/app/gamenative/steamcontroller/ProfileInterpreter.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScKeyboard.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScKeyboardOverlayView.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScMenuLabels.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScMenuNav.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScMenuOverlay.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScMenuOverlayView.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScOutputSink.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScOverlayStore.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScProfile.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScProfileEditor.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScTuningStore.kt
  • app/src/main/java/app/gamenative/steamcontroller/ScUiBridge.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonBle.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonHaptics.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonMapper.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonProtocol.kt
  • app/src/main/java/app/gamenative/ui/PluviaMain.kt
  • app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ControllerTab.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScConfigManagerDialog.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScCursorController.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScEditorStyle.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScInputGlyph.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScListNav.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScMenuLabelEditor.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScNavChoiceDialog.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScNavDialogStack.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScOverlayEditor.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScRootMenuDialog.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScScrollbar.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScSelectionRing.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScTextEditField.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/SteamControllerBindingEditor.kt
  • app/src/main/java/app/gamenative/ui/model/MainViewModel.kt
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
  • app/src/main/java/app/gamenative/utils/SteamControllerVdfUtils.kt
  • app/src/main/java/com/winlator/winhandler/WinHandler.java
  • app/src/main/java/com/winlator/xserver/InputDeviceManager.java
  • app/src/main/res/values/strings.xml
  • app/src/test/java/app/gamenative/steamcontroller/ActionLayersTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ActionSetsTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ActivatorsTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/AdvancedModesTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/EditableConfigTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/HapticsRumbleTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/MacroDelayTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/MenuModesTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ModeShiftTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/PadModesTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ProfileInterpreterTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/QuickMenuNavTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScConfigStoreRegistryTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScEditableAnalogTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScKeyboardTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScMenuLabelToolTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScOverlayStoreTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScProfileEditorTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScTestSupport.kt
  • app/src/test/java/app/gamenative/steamcontroller/TriggerModesTest.kt
  • app/src/test/java/app/gamenative/utils/SteamControllerProfileImporterTest.kt
  • app/src/test/resources/sc/actionlayers_v3.vdf
  • app/src/test/resources/sc/actionsets_v3.vdf
  • app/src/test/resources/sc/chord_triton.vdf
  • app/src/test/resources/sc/controller_xboxone_wasd.vdf
  • app/src/test/resources/sc/delf_tome4_neptune.vdf
  • app/src/test/resources/sc/doom_sc_test.vdf
  • app/src/test/resources/sc/gamepad_joystick.vdf
  • app/src/test/resources/sc/kitchensink_v3.vdf
  • app/src/test/resources/sc/ksp_worstcase_xboxone.vdf
  • app/src/test/resources/sc/modeshift_v3.vdf
  • app/src/test/resources/sc/sc_bindsettings_test.vdf
  • app/src/test/resources/sc/sc_gyro_mousejoy_test.vdf
  • app/src/test/resources/sc/sc_gyro_touch_test.vdf
  • app/src/test/resources/sc/sc_newmodes_test.vdf
  • app/src/test/resources/sc/sc_region_single_test.vdf
  • app/src/test/resources/sc/sc_touch_bind_test.vdf
  • app/src/test/resources/sc/testgyro_gate_capture.vdf
💤 Files with no reviewable changes (1)
  • app/src/main/java/app/gamenative/ui/PluviaMain.kt

Comment thread app/src/main/java/app/gamenative/steamcontroller/ScProfile.kt
Comment thread app/src/main/java/app/gamenative/steamcontroller/TritonMapper.kt Outdated
Comment on lines +1095 to +1102
put("BACK_SLASH", XKeycode.KEY_BACKSLASH)
put("SEMICOLON", XKeycode.KEY_SEMICOLON)
put("SINGLE_QUOTE", XKeycode.KEY_APOSTROPHE)
put("APOSTROPHE", XKeycode.KEY_APOSTROPHE)
put("LEFT_BRACKET", XKeycode.KEY_BRACKET_LEFT)
put("RIGHT_BRACKET", XKeycode.KEY_BRACKET_RIGHT)
put("BACK_QUOTE", XKeycode.KEY_GRAVE)
put("TILDE", XKeycode.KEY_GRAVE)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm which Steam key tokens the fixtures actually use vs what KEY_MAP handles.
rg -n --no-heading -o 'key_press [A-Z_]+' app/src/test/resources/sc \
  | grep -oE '[A-Z_]+$' | sort -u > /tmp/tokens.txt
echo "== tokens in fixtures containing SLASH/TICK/QUOTE =="
grep -Ei 'SLASH|TICK|QUOTE|GRAVE' /tmp/tokens.txt
echo "== tokens present in KEY_MAP =="
rg -n 'put\("' app/src/main/java/app/gamenative/utils/SteamControllerVdfUtils.kt

Repository: utkarshdalal/GameNative

Length of output: 2851


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect how key tokens are resolved and whether any normalization covers
# BACKSLASH / BACK_TICK before KEY_MAP lookup.
FILE=app/src/main/java/app/gamenative/utils/SteamControllerVdfUtils.kt

ast-grep outline "$FILE" --view expanded

echo "== keyOf / KEY_MAP references =="
rg -n 'keyOf|KEY_MAP|BACKSLASH|BACK_TICK|BACK_SLASH|BACK_QUOTE|FORWARD_SLASH' "$FILE"

echo "== surrounding lookup implementation =="
sed -n '1000,1145p' "$FILE"

Repository: utkarshdalal/GameNative

Length of output: 13550


Add Steam’s BACKSLASH / BACK_TICK aliases

keyOf() does a direct uppercase lookup, so key_press BACKSLASH and key_press BACK_TICK never match the current KEY_MAP entries (BACK_SLASH, BACK_QUOTE/TILDE) and get dropped. Add the Steam spellings alongside the existing aliases.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/app/gamenative/utils/SteamControllerVdfUtils.kt` around
lines 1095 - 1102, Update the KEY_MAP used by keyOf() to add Steam’s exact
BACKSLASH and BACK_TICK aliases alongside the existing BACK_SLASH and
BACK_QUOTE/TILDE entries, mapping each to the same XKeycode values so direct
uppercase lookup resolves both spellings.

Comment thread app/src/main/java/com/winlator/winhandler/WinHandler.java Outdated
…safety, tests)

On-device validated on the Z Fold 7 (scrollbar drag, launch, BLE input,
haptics, rumble, editors, autosave — all pass).

- TritonProtocol: delete dead USB-only fns (decodeState/isStateReport/
  settingReport/lizardOffReport/imuEnableReport) left over from the USB removal
  — zero callers; also removes the flagged off-by-one length guard.
- SteamControllerVdfUtils: curve_exponent is a literal power exponent, not an
  enum ordinal — map 1->LINEAR, 2->AGGRESSIVE (m^2), 3->WIDE (m^3), 4->WIDE.
  Fixes 1 importing as AGGRESSIVE. Importer test updated.
- TritonMapper: @volatile on ble/interpreter/bleRetries (cross-thread reads).
- ScConfigStore: writeRegistry via temp file + atomic rename (no truncated
  manifest under live autosave).
- WinHandler: cancel an in-flight local phone/controller buzz when the SC
  rumble forwarder takes over, so it doesn't linger.
- ScScrollbar: dispatchRawDelta instead of a coroutine per drag event.
- Tests: dedup the load() helper into ScTestSupport; add HapticsRumbleTest
  coverage for the click/detent/jitter path; add the missing Space assertion;
  correct the SMOKE_CONFIG doc to describe structural (not press) intent.

Deferred per author: KEY_MAP BACKSLASH/BACK_TICK aliases (speculative), and
ScSelectionRing -> Modifier.Node (rewrite of working code). mergeProfiles is a
non-issue: layer profiles are built from scratch, so no default() buttons leak.

Co-Authored-By: Claude Fable 5 <[email protected]>
@cubic-dev-ai

cubic-dev-ai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

This PR is large and would use a significant portion of your monthly review quota. Comment @cubic-dev-ai review this to confirm that you want cubic to review it.

@JonahSimon

Copy link
Copy Markdown
Author

Addressed the automated review in 9f907d4f (on-device validated on a Z Fold 7 — scrollbar drag, game launch, BLE input, haptics, rumble, editors, autosave all pass):

  • TritonProtocol — deleted the dead USB-only functions left over from the USB-transport removal (zero callers); this also removes the flagged off-by-one length guard.
  • curve_exponent — now mapped as a literal power exponent (1→LINEAR, 2→AGGRESSIVE, 3→WIDE, 4→WIDE) instead of an enum ordinal; fixes 1 importing as AGGRESSIVE. Importer test updated.
  • TritonMapper@Volatile on the cross-thread ble/interpreter/bleRetries fields.
  • ScConfigStorewriteRegistry now writes a temp file + atomic rename (no truncated manifest under live autosave).
  • WinHandler — cancel an in-flight local buzz when the SC rumble forwarder takes over.
  • ScScrollbardispatchRawDelta instead of a coroutine per drag event.
  • Tests — deduped the load() helper, added HapticsRumbleTest coverage for the click/detent/jitter path, added the missing Space assertion, corrected the SMOKE_CONFIG doc.

Intentionally deferred: the KEY_MAP BACKSLASH/BACK_TICK aliases (speculative — no capture confirms Steam emits those spellings; existing BACK_SLASH/BACK_QUOTE cover the confirmed names) and ScSelectionRingModifier.Node (rewrite of working code, marginal gain).

mergeProfiles is a non-issue (details on the inline thread): layer profiles are built from scratch, so no default() buttons leak into the union.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt (4)

180-190: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Update the registry before deleting payload files. Both methods delete payload files from disk before successfully persisting the registry changes. If writeRegistry fails, the registry will stubbornly point to payload files that no longer exist, breaking referential integrity.

  • app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt#L180-L190: Call writeRegistry first, returning false if it fails, and only execute the when (entry.kind) delete block on success.
  • app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt#L298-L311: Similarly, execute the toRemove.forEach delete block only after writeRegistry succeeds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt` around
lines 180 - 190, Update deleteConfig and the corresponding removal flow at
app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt lines 180-190
and 298-311 so writeRegistry persists the registry changes first; return false
immediately when it fails, and only then execute the entry.kind or
toRemove.forEach payload deletion blocks.

213-231: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Fail fast on payload parse errors to prevent silent overwrite.

If a payload file exists but fails to parse (e.g., due to a syntax error or partial JSON corruption), loadEditableConfig silently catches the error and falls through to seed the editor with the default profile (resolveConfig(context, key)). When the user subsequently saves or autosaves, the editor will overwrite their corrupted (but potentially recoverable) config with the default settings.

Explicitly return null on parse failure to abort loading, rather than falling through to the destructive fallback.

💚 Proposed fix
         when (active?.kind) {
             ScConfigKind.AUTHORED -> setsPayload(context, key, active.id).takeIf { it.isFile }?.let { f ->
-                runCatching { json.decodeFromString(ScEditableConfig.serializer(), f.readText()) }
-                    .onFailure { Log.w(TAG, "loadEditableConfig($key) failed: ${it.message}") }
-                    .getOrNull()?.let { return it }
+                return runCatching { json.decodeFromString(ScEditableConfig.serializer(), f.readText()) }
+                    .onFailure { Log.w(TAG, "loadEditableConfig($key) failed: ${it.message}") }
+                    .getOrNull()
             }
             // A .vdf-active config edits via an overlay: load the prior overlay if any, else seed from THIS game's
             // parsed vdf so the editor shows its action sets/bindings (advanced outputs seed as INHERIT = preserved).
             ScConfigKind.VDF -> {
                 overlayPayload(context, key, active.id).takeIf { it.isFile }?.let { f ->
-                    runCatching { json.decodeFromString(ScEditableConfig.serializer(), f.readText()) }
-                        .onFailure { Log.w(TAG, "loadEditableConfig($key) overlay failed: ${it.message}") }
-                        .getOrNull()?.let { return it }
+                    return runCatching { json.decodeFromString(ScEditableConfig.serializer(), f.readText()) }
+                        .onFailure { Log.w(TAG, "loadEditableConfig($key) overlay failed: ${it.message}") }
+                        .getOrNull()
                 }
                 vdfPayload(context, key, active.id).takeIf { it.isFile }?.let { parseVdf(it) }
                     ?.let { return ScEditableConfig.fromScConfig(it) }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt` around
lines 213 - 231, Update loadEditableConfig so existing authored or overlay
payload files return null when JSON deserialization fails, instead of continuing
to the fallback profile or VDF seeding path. Preserve successful decoding and
missing-file behavior, while ensuring each runCatching parse failure exits the
function immediately after logging.

98-127: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win

Prevent permanent loss of legacy configs on migration failure.

If migrating a legacy config fails (e.g., copyTo or writeText throws an IOException due to a full disk), the exception is caught and logged, but the legacy source file is still unconditionally deleted and the entry appended to the registry. This permanently wipes the user's config and breaks the registry state.

Only delete the legacy file and append the registry entry if the I/O write succeeds.

🛡️ Proposed fix to verify write success before deleting
         legacyVdf(context, key).takeIf { it.isFile }?.let { f ->
             val id = "vdf"
-            runCatching { f.copyTo(vdfPayload(context, key, id), overwrite = true); f.delete() }
-                .onFailure { Log.w(TAG, "migrate vdf($key) failed: ${it.message}") }
-            entries += ScConfigEntry(id, "Imported", ScConfigKind.VDF)
-            active = id
+            if (runCatching { f.copyTo(vdfPayload(context, key, id), overwrite = true); true }.getOrDefault(false)) {
+                f.delete()
+                entries += ScConfigEntry(id, "Imported", ScConfigKind.VDF)
+                active = id
+            } else {
+                Log.w(TAG, "migrate vdf($key) failed")
+            }
         }
         // Authored: prefer the multi-set `.sets.json`, else convert a legacy single `.json`.
         val sets = legacySets(context, key)
         val single = legacyJson(context, key)
         if (sets.isFile) {
             val id = "authored"
             val name = runCatching { json.decodeFromString(ScEditableConfig.serializer(), sets.readText()) }
                 .getOrNull()?.let { it.sets.firstOrNull()?.name } ?: "Custom bindings"
-            runCatching { sets.copyTo(setsPayload(context, key, id), overwrite = true); sets.delete() }
-                .onFailure { Log.w(TAG, "migrate sets($key) failed: ${it.message}") }
-            entries += ScConfigEntry(id, name.ifBlank { "Custom bindings" }, ScConfigKind.AUTHORED)
-            if (active.isEmpty()) active = id
+            if (runCatching { sets.copyTo(setsPayload(context, key, id), overwrite = true); true }.getOrDefault(false)) {
+                sets.delete()
+                entries += ScConfigEntry(id, name.ifBlank { "Custom bindings" }, ScConfigKind.AUTHORED)
+                if (active.isEmpty()) active = id
+            } else {
+                Log.w(TAG, "migrate sets($key) failed")
+            }
         } else if (single.isFile) {
             val id = "authored"
             val profile = runCatching { json.decodeFromString(ScEditableProfile.serializer(), single.readText()) }.getOrNull()
             if (profile != null) {
                 val cfg = ScEditableConfig.fromSingle(profile)
-                runCatching { setsPayload(context, key, id).writeText(json.encodeToString(ScEditableConfig.serializer(), cfg)) }
-                    .onFailure { Log.w(TAG, "migrate json($key) failed: ${it.message}") }
-                single.delete()
-                entries += ScConfigEntry(id, profile.name.ifBlank { "Custom bindings" }, ScConfigKind.AUTHORED)
-                if (active.isEmpty()) active = id
+                if (runCatching { setsPayload(context, key, id).writeText(json.encodeToString(ScEditableConfig.serializer(), cfg)); true }.getOrDefault(false)) {
+                    single.delete()
+                    entries += ScConfigEntry(id, profile.name.ifBlank { "Custom bindings" }, ScConfigKind.AUTHORED)
+                    if (active.isEmpty()) active = id
+                } else {
+                    Log.w(TAG, "migrate json($key) failed")
+                }
             }
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt` around
lines 98 - 127, Update the migration logic in the legacy VDF, multi-set, and
single JSON branches so source files are deleted and ScConfigEntry records are
appended only after the destination copy or write succeeds. Track each
runCatching operation’s success, log failures without mutating registry state,
and ensure the single JSON path also avoids deletion when serialization or
writeText fails.

247-267: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Use atomic writes for config payloads to prevent data loss on crash.

While writeRegistry thoughtfully uses a temporary file to avoid truncation on crash, saveEditableConfig (as well as saveLabels and importVdfConfig) writes the actual payload JSON directly via writeText. A crash or battery death mid-write will truncate the user's config file to 0 bytes, completely losing their bindings.

Extract the atomic temp-file logic from writeRegistry into a helper and reuse it for payload saves.

♻️ Proposed refactor for atomic payload writes

Add a helper to ScConfigStore:

    private fun writeAtomically(target: File, text: String): Boolean =
        runCatching {
            val tmp = File.createTempFile("${target.name}-", ".tmp", target.parentFile)
            tmp.writeText(text)
            if (!tmp.renameTo(target)) { target.writeText(tmp.readText()); tmp.delete() }
            true
        }.getOrDefault(false)

Then use it for payloads:

-            ScConfigKind.AUTHORED -> return runCatching { setsPayload(context, key, active.id).writeText(text); true }
-                .onFailure { Log.w(TAG, "saveEditableConfig($key) failed: ${it.message}") }
-                .getOrDefault(false)
-            ScConfigKind.VDF -> return runCatching { overlayPayload(context, key, active.id).writeText(text); true }
-                .onFailure { Log.w(TAG, "saveEditableConfig($key) overlay failed: ${it.message}") }
-                .getOrDefault(false)
+            ScConfigKind.AUTHORED -> return writeAtomically(setsPayload(context, key, active.id), text)
+            ScConfigKind.VDF -> return writeAtomically(overlayPayload(context, key, active.id), text)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt` around
lines 247 - 267, Extract the temporary-file replacement logic from writeRegistry
into a reusable writeAtomically helper, preserving its success/failure behavior.
Update saveEditableConfig, saveLabels, and importVdfConfig to write payload JSON
through writeAtomically instead of direct writeText calls, while keeping their
existing error handling and target paths unchanged.
🧹 Nitpick comments (2)
app/src/test/java/app/gamenative/steamcontroller/HapticsRumbleTest.kt (2)

32-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Rename test to accurately reflect behavior.

The rumbleReport function truncates/packs values rather than mathematically clamping them (e.g., passing 0x1FFFF would wrap to 0xFFFF, not clamp). Since this test specifically validates that the maximum 16-bit value (0xFFFF) properly sets all bits in the two target bytes, consider renaming it to prevent confusion about how out-of-bounds values are handled.

♻️ Proposed rename
-    `@Test` fun rumbleReport_fullScaleClampsToTwoBytes() {
+    `@Test` fun rumbleReport_max16BitValuePacksToTwoBytes() {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/test/java/app/gamenative/steamcontroller/HapticsRumbleTest.kt` around
lines 32 - 37, Rename the test function rumbleReport_fullScaleClampsToTwoBytes
to describe verifying full-scale 16-bit packing into the two target byte pairs,
without implying mathematical clamping or out-of-range handling. Keep the test
inputs and assertions unchanged.

43-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplify byte comparisons.

You can avoid the .toInt() conversions and bitwise masking by casting your constants to Byte and directly comparing the array elements. This makes the condition slightly cleaner and more idiomatic.

♻️ Proposed refactor
     private fun List<ByteArray>.count(cmd: Int) =
-        count { it.size >= 3 && (it[0].toInt() and 0xFF) == TritonHaptics.ID_OUT_HAPTIC_COMMAND && it[2].toInt() == cmd }
+        count { it.size >= 3 && it[0] == TritonHaptics.ID_OUT_HAPTIC_COMMAND.toByte() && it[2] == cmd.toByte() }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/test/java/app/gamenative/steamcontroller/HapticsRumbleTest.kt` around
lines 43 - 44, Simplify the predicate in the List<ByteArray>.count extension by
comparing byte-array elements directly with the relevant constants converted to
Byte. Remove the toInt() conversions and 0xFF masking while preserving the
existing size guard and command-matching behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt`:
- Around line 80-89: Update writeRegistry to create a uniquely named temporary
file for each save instead of using the fixed "${target.name}.tmp" path, while
preserving the existing write, rename, fallback, and cleanup behavior.

In `@app/src/test/java/app/gamenative/steamcontroller/HapticsRumbleTest.kt`:
- Around line 62-69: Strengthen the test `jitter below moveNoise fires no tick`
by applying multiple consecutive RTOUCH movements, each below `cfg.moveNoise`,
whose cumulative movement exceeds `cfg.detentStep`; then assert that
`TritonHaptics.CMD_TICK` remains absent. Keep the setup and callback capture
unchanged so the test verifies sub-noise jitter is ignored rather than
accumulated.

---

Outside diff comments:
In `@app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt`:
- Around line 180-190: Update deleteConfig and the corresponding removal flow at
app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt lines 180-190
and 298-311 so writeRegistry persists the registry changes first; return false
immediately when it fails, and only then execute the entry.kind or
toRemove.forEach payload deletion blocks.
- Around line 213-231: Update loadEditableConfig so existing authored or overlay
payload files return null when JSON deserialization fails, instead of continuing
to the fallback profile or VDF seeding path. Preserve successful decoding and
missing-file behavior, while ensuring each runCatching parse failure exits the
function immediately after logging.
- Around line 98-127: Update the migration logic in the legacy VDF, multi-set,
and single JSON branches so source files are deleted and ScConfigEntry records
are appended only after the destination copy or write succeeds. Track each
runCatching operation’s success, log failures without mutating registry state,
and ensure the single JSON path also avoids deletion when serialization or
writeText fails.
- Around line 247-267: Extract the temporary-file replacement logic from
writeRegistry into a reusable writeAtomically helper, preserving its
success/failure behavior. Update saveEditableConfig, saveLabels, and
importVdfConfig to write payload JSON through writeAtomically instead of direct
writeText calls, while keeping their existing error handling and target paths
unchanged.

---

Nitpick comments:
In `@app/src/test/java/app/gamenative/steamcontroller/HapticsRumbleTest.kt`:
- Around line 32-37: Rename the test function
rumbleReport_fullScaleClampsToTwoBytes to describe verifying full-scale 16-bit
packing into the two target byte pairs, without implying mathematical clamping
or out-of-range handling. Keep the test inputs and assertions unchanged.
- Around line 43-44: Simplify the predicate in the List<ByteArray>.count
extension by comparing byte-array elements directly with the relevant constants
converted to Byte. Remove the toInt() conversions and 0xFF masking while
preserving the existing size guard and command-matching behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 33d812a3-ee73-4e1d-85c1-e6fe9f2c3006

📥 Commits

Reviewing files that changed from the base of the PR and between 6ebc6d9 and 9f907d4.

📒 Files selected for processing (15)
  • app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonMapper.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonProtocol.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScOverlayEditor.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScScrollbar.kt
  • app/src/main/java/app/gamenative/utils/SteamControllerVdfUtils.kt
  • app/src/main/java/com/winlator/winhandler/WinHandler.java
  • app/src/test/java/app/gamenative/steamcontroller/ActionLayersTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ActionSetsTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/HapticsRumbleTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/MenuModesTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ModeShiftTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScKeyboardTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScTestSupport.kt
  • app/src/test/java/app/gamenative/utils/SteamControllerProfileImporterTest.kt
💤 Files with no reviewable changes (5)
  • app/src/test/java/app/gamenative/steamcontroller/ActionLayersTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/ModeShiftTest.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonProtocol.kt
  • app/src/test/java/app/gamenative/steamcontroller/ActionSetsTest.kt
  • app/src/test/java/app/gamenative/steamcontroller/MenuModesTest.kt
🚧 Files skipped from review as they are similar to previous changes (8)
  • app/src/main/java/com/winlator/winhandler/WinHandler.java
  • app/src/test/java/app/gamenative/steamcontroller/ScTestSupport.kt
  • app/src/test/java/app/gamenative/steamcontroller/ScKeyboardTest.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScOverlayEditor.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ScScrollbar.kt
  • app/src/main/java/app/gamenative/steamcontroller/TritonMapper.kt
  • app/src/test/java/app/gamenative/utils/SteamControllerProfileImporterTest.kt
  • app/src/main/java/app/gamenative/utils/SteamControllerVdfUtils.kt

Comment thread app/src/main/java/app/gamenative/steamcontroller/ScConfigStore.kt
- ScConfigStore.writeRegistry: use File.createTempFile for a unique temp name
  so concurrent writes (autosave + a UI action) can't clobber a shared .tmp
  before the atomic rename.
- HapticsRumbleTest: strengthen the jitter test — feed many sub-moveNoise steps
  whose sum exceeds detentStep and assert still no tick, which actually proves
  jitter is rejected (not accumulated) rather than just being below one detent.

(mergeProfiles finding was withdrawn by the reviewer after clarification.)

Co-Authored-By: Claude Fable 5 <[email protected]>
@cubic-dev-ai

cubic-dev-ai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

This PR is large and would use a significant portion of your monthly review quota. Comment @cubic-dev-ai review this to confirm that you want cubic to review it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant