Added gyro aiming support (gamepad & mouse)#1173
Draft
unbelievableflavour wants to merge 1 commit intoutkarshdalal:masterfrom
Draft
Added gyro aiming support (gamepad & mouse)#1173unbelievableflavour wants to merge 1 commit intoutkarshdalal:masterfrom
unbelievableflavour wants to merge 1 commit intoutkarshdalal:masterfrom
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds end-to-end gyro controller support: persisted gyro preferences, a SensorEventListener-based GyroController, UI controls in QuickMenu/XServerScreen, gyro mixing in InputControlsView, string resources, and unit tests. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as QuickMenu / XServerScreen
participant Prefs as PrefManager
participant ICView as InputControlsView
participant Gyro as GyroController
participant Sensor as SensorManager
participant Handler as WinHandler
User->>UI: change gyro settings (mode/sensitivity/invert)
UI->>Prefs: persist settings
UI->>ICView: setGyroMode / setGyroSensitivity / setGyroInvert*
ICView->>Gyro: apply configuration (mode,sens,invert,hasProfile)
Gyro->>Sensor: register/unregister listener (attach / mode/profile/edit changes)
Sensor->>Gyro: onSensorChanged(gyroData)
Gyro->>Gyro: remap by rotation, apply invert/sensitivity/deadzone
Gyro->>ICView: onGyroOutput(x,y,rightStick,isMouse)
ICView->>ICView: mix gyroThumb + baseThumb → GamepadState
ICView->>Handler: sendGamepadState / sendVirtualGamepadState
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Closed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/winlator/widget/InputControlsView.java (1)
283-293:⚠️ Potential issue | 🟠 MajorReset thumb state on any profile swap.
baseThumb*andgyroThumb*live on the view, not onControlsProfile. Replacing one non-null profile with another keeps the previous profile’s stick contribution alive, so the first gyro/controller update on the new profile can inherit a stale axis value and latch movement/aim.Proposed fix
public synchronized void setProfile(ControlsProfile profile) { + if (this.profile != profile) { + resetThumbContributions(); + } if (profile != null) { this.profile = profile; deselectAllElements(); } - else { + else { this.profile = null; - resetThumbContributions(); } gyroController.setHasProfile(this.profile != null); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/widget/InputControlsView.java` around lines 283 - 293, When swapping profiles in setProfile(ControlsProfile profile) ensure you reset the view-local thumb state so previous profile stick contributions can't carry over: always call resetThumbContributions() when the active profile reference changes (both when replacing a non-null profile with another and when clearing it). Update setProfile to compare/replace the profile and invoke resetThumbContributions() before/after assigning this.profile (and still call deselectAllElements() for the non-null branch), then keep the existing gyroController.setHasProfile(this.profile != null) call.
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (2)
2083-2087: Clamp sensitivity at the callback boundary too.The UI currently sends clamped values, but this callback persists and applies whatever it receives. Reapplying the same
0.1f..2.0fguard here keeps future callers from pushing invalid values into prefs orInputControlsView.Suggested change
gyroSensitivity = controlsGyroSensitivity, onGyroSensitivityChanged = { sensitivity -> - controlsGyroSensitivity = sensitivity - PrefManager.setFloat(PREF_CONTROLS_GYRO_SENSITIVITY, sensitivity) - PluviaApp.inputControlsView?.setGyroSensitivity(sensitivity) + val clamped = sensitivity.coerceIn(0.1f, 2.0f) + controlsGyroSensitivity = clamped + PrefManager.setFloat(PREF_CONTROLS_GYRO_SENSITIVITY, clamped) + PluviaApp.inputControlsView?.setGyroSensitivity(clamped) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` around lines 2083 - 2087, The onGyroSensitivityChanged callback currently writes and forwards whatever value it receives; enforce the same 0.1f..2.0f clamp at the callback boundary by clamping the incoming sensitivity before assigning to controlsGyroSensitivity, before calling PrefManager.setFloat(PREF_CONTROLS_GYRO_SENSITIVITY, ...), and before calling PluviaApp.inputControlsView?.setGyroSensitivity(...); update the callback in XServerScreen (the onGyroSensitivityChanged lambda) to compute a local clamped value and use that everywhere so invalid values never reach prefs or InputControlsView.
197-217: Share the gyro mode representation instead of duplicating raw ints.These helpers are the source of truth for gyro mode, but
QuickMenu.ktnow has to hardcode0/1/2to match them. That coupling is easy to drift and would silently remap both saved prefs and the runtime mode sent toInputControlsView. Please move this into a shared enum/constants type and reuse it from both files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` around lines 197 - 217, Replace the duplicated raw-int gyro representation with a shared enum: create an enum GyroMode { DISABLED, LEFT_STICK, RIGHT_STICK } (include a companion fromPref(value: String?): GyroMode and a toPrefValue(): String and an explicit int property if numeric values are required), then remove or refactor the existing constants GYRO_MODE_DISABLED/GYRO_MODE_LEFT_STICK/GYRO_MODE_RIGHT_STICK and the helpers parseGyroMode and gyroModeToPrefValue to use this enum (or delegate to the enum methods); finally update QuickMenu.kt (and any other callers) to use GyroMode.* instead of hardcoded 0/1/2 so all pref parsing, saving and runtime mode passing (e.g., into InputControlsView) uses the shared GyroMode type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/winlator/widget/GyroController.java`:
- Around line 56-59: When entering edit mode in setEditMode(boolean editMode),
clear the active stick before stopping sensors so the UI doesn't retain the last
gyro vector; call whatever routine the view uses to accept stick/input updates
(e.g. invoke InputControlsView's method that applies a stick vector or the
existing gyro input handler) to emit a zero-value vector, then proceed to
updateRegistration(); modify setEditMode to, when editMode is true, send a zero
stick/update to InputControlsView (clearActiveStick or equivalent) before
calling updateRegistration().
---
Outside diff comments:
In `@app/src/main/java/com/winlator/widget/InputControlsView.java`:
- Around line 283-293: When swapping profiles in setProfile(ControlsProfile
profile) ensure you reset the view-local thumb state so previous profile stick
contributions can't carry over: always call resetThumbContributions() when the
active profile reference changes (both when replacing a non-null profile with
another and when clearing it). Update setProfile to compare/replace the profile
and invoke resetThumbContributions() before/after assigning this.profile (and
still call deselectAllElements() for the non-null branch), then keep the
existing gyroController.setHasProfile(this.profile != null) call.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 2083-2087: The onGyroSensitivityChanged callback currently writes
and forwards whatever value it receives; enforce the same 0.1f..2.0f clamp at
the callback boundary by clamping the incoming sensitivity before assigning to
controlsGyroSensitivity, before calling
PrefManager.setFloat(PREF_CONTROLS_GYRO_SENSITIVITY, ...), and before calling
PluviaApp.inputControlsView?.setGyroSensitivity(...); update the callback in
XServerScreen (the onGyroSensitivityChanged lambda) to compute a local clamped
value and use that everywhere so invalid values never reach prefs or
InputControlsView.
- Around line 197-217: Replace the duplicated raw-int gyro representation with a
shared enum: create an enum GyroMode { DISABLED, LEFT_STICK, RIGHT_STICK }
(include a companion fromPref(value: String?): GyroMode and a toPrefValue():
String and an explicit int property if numeric values are required), then remove
or refactor the existing constants
GYRO_MODE_DISABLED/GYRO_MODE_LEFT_STICK/GYRO_MODE_RIGHT_STICK and the helpers
parseGyroMode and gyroModeToPrefValue to use this enum (or delegate to the enum
methods); finally update QuickMenu.kt (and any other callers) to use GyroMode.*
instead of hardcoded 0/1/2 so all pref parsing, saving and runtime mode passing
(e.g., into InputControlsView) uses the shared GyroMode type.
🪄 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: 72a80493-dd69-41fe-91a2-3f380c9ca073
📒 Files selected for processing (7)
app/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/widget/GyroController.javaapp/src/main/java/com/winlator/widget/InputControlsView.javaapp/src/main/res/values/strings.xmlapp/src/test/java/com/winlator/widget/GyroControllerTest.kt
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/com/winlator/widget/GyroController.java">
<violation number="1" location="app/src/main/java/com/winlator/widget/GyroController.java:152">
P2: Disabling gyro via edit mode or missing profile only unregisters the sensor without clearing the active stick value, so the last non-zero gyro input can remain latched until another subsystem overwrites it.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:1156">
P2: Gamepad motion events are double-dispatched to physical and overlay handlers, allowing duplicate axis/gyro processing.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
Outdated
Show resolved
Hide resolved
unbelievableflavour
force-pushed
the
gyro-aiming-v2
branch
from
1b49efc to
3e73314
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/winlator/widget/InputControlsView.java (1)
283-293:⚠️ Potential issue | 🟠 MajorReset thumb contributions on every profile swap, not only when profile is null.
At Line 284-Line 287, switching to another non-null
ControlsProfilekeeps previousbaseThumb*/gyroThumb*values alive, so the new profile can inherit stale stick state until the next input update.Proposed fix
public synchronized void setProfile(ControlsProfile profile) { - if (profile != null) { - this.profile = profile; - deselectAllElements(); - } - else { - this.profile = null; - resetThumbContributions(); - } + // Always clear mixed stick contributions when profile changes. + resetThumbContributions(); + + if (profile != null) { + this.profile = profile; + deselectAllElements(); + } else { + this.profile = null; + } gyroController.setHasProfile(this.profile != null); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/widget/InputControlsView.java` around lines 283 - 293, The current setProfile(ControlsProfile) only calls resetThumbContributions() when profile is null, leaving previous baseThumb*/gyroThumb* state when swapping between non-null profiles; modify setProfile so thumb contributions are reset on every profile change — call resetThumbContributions() whenever the incoming profile differs from the current (or unconditionally on any setProfile call before assigning the new profile), then assign this.profile, call deselectAllElements() for non-null profiles, and still call gyroController.setHasProfile(this.profile != null) so stale stick state cannot be carried into the new ControlsProfile.
🧹 Nitpick comments (1)
app/src/main/java/com/winlator/widget/InputControlsView.java (1)
1026-1031: Extract duplicated gamepad-dispatch block into one helper.Both blocks send identical updates to
WinHandler. Centralizing this avoids drift between touch and gyro paths.Refactor sketch
+ private void dispatchGamepadState(GamepadState state) { + WinHandler winHandler = xServer != null ? xServer.getWinHandler() : null; + if (winHandler != null) { + ExternalController controller = winHandler.getCurrentController(); + if (controller != null) controller.state.copy(state); + winHandler.sendGamepadState(); + winHandler.sendVirtualGamepadState(state); + } + }Then replace both call sites with
dispatchGamepadState(state);.Also applies to: 1075-1081
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/widget/InputControlsView.java` around lines 1026 - 1031, Extract the duplicated WinHandler dispatch logic into a single helper method on InputControlsView (e.g., dispatchGamepadState(State state)): inside it, null-check winHandler, get ExternalController via winHandler.getCurrentController(), copy the state into controller.state if controller != null, then call winHandler.sendGamepadState() and winHandler.sendVirtualGamepadState(state). Replace both duplicated blocks that currently reference winHandler, ExternalController, sendGamepadState and sendVirtualGamepadState with calls to dispatchGamepadState(state).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/src/main/java/com/winlator/widget/InputControlsView.java`:
- Around line 283-293: The current setProfile(ControlsProfile) only calls
resetThumbContributions() when profile is null, leaving previous
baseThumb*/gyroThumb* state when swapping between non-null profiles; modify
setProfile so thumb contributions are reset on every profile change — call
resetThumbContributions() whenever the incoming profile differs from the current
(or unconditionally on any setProfile call before assigning the new profile),
then assign this.profile, call deselectAllElements() for non-null profiles, and
still call gyroController.setHasProfile(this.profile != null) so stale stick
state cannot be carried into the new ControlsProfile.
---
Nitpick comments:
In `@app/src/main/java/com/winlator/widget/InputControlsView.java`:
- Around line 1026-1031: Extract the duplicated WinHandler dispatch logic into a
single helper method on InputControlsView (e.g., dispatchGamepadState(State
state)): inside it, null-check winHandler, get ExternalController via
winHandler.getCurrentController(), copy the state into controller.state if
controller != null, then call winHandler.sendGamepadState() and
winHandler.sendVirtualGamepadState(state). Replace both duplicated blocks that
currently reference winHandler, ExternalController, sendGamepadState and
sendVirtualGamepadState with calls to dispatchGamepadState(state).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e42c0667-4684-41e3-87a9-e0fa19f3a6fc
📒 Files selected for processing (7)
app/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/widget/GyroController.javaapp/src/main/java/com/winlator/widget/InputControlsView.javaapp/src/main/res/values/strings.xmlapp/src/test/java/com/winlator/widget/GyroControllerTest.kt
✅ Files skipped from review due to trivial changes (2)
- app/src/main/res/values/strings.xml
- app/src/main/java/com/winlator/widget/GyroController.java
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/test/java/com/winlator/widget/GyroControllerTest.kt
- app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
- app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
unbelievableflavour
changed the title
unbelievableflavour
force-pushed
the
gyro-aiming-v2
branch
from
3e73314 to
25dc489
Compare
Contributor
There was a problem hiding this comment.
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 (1)
app/src/main/java/com/winlator/widget/InputControlsView.java (1)
290-300:⚠️ Potential issue | 🟠 MajorClear gyro output before dropping the active profile.
Line 296 sets
this.profiletonullbeforegyroController.setHasProfile(false)runs. In left/right-stick modes that makesGyroController.clearCurrentStick()ineffective, becauseupdateGyroStick()immediately returns onceprofile == null, so the last gyro deflection can stay latched.🛠️ Suggested fix
public synchronized void setProfile(ControlsProfile profile) { - if (profile != null) { + if (profile == null && this.profile != null) { + gyroController.setHasProfile(false); + resetThumbContributions(); + this.profile = null; + return; + } + if (profile != null) { this.profile = profile; deselectAllElements(); - } - else { + } else { this.profile = null; resetThumbContributions(); } gyroController.setHasProfile(this.profile != null); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/widget/InputControlsView.java` around lines 290 - 300, The bug: when clearing the active ControlsProfile in setProfile the code sets this.profile = null before informing GyroController, which lets updateGyroStick() short-circuit and leaves the last gyro deflection latched; to fix, ensure gyro output is cleared before dropping the profile by calling GyroController.clearCurrentStick() (or at minimum calling gyroController.setHasProfile(false)) while this.profile is still non-null — adjust setProfile so you either call gyroController.clearCurrentStick() then reset this.profile to null and resetThumbContributions(), or call gyroController.setHasProfile(false) before setting this.profile = null; update the method references setProfile, gyroController.setHasProfile, GyroController.clearCurrentStick, resetThumbContributions, deselectAllElements accordingly.
🧹 Nitpick comments (1)
app/src/main/java/com/winlator/widget/GyroController.java (1)
114-145: Add coverage for the remaining rotation branches.The provided tests only exercise
Surface.ROTATION_90and inversion atSurface.ROTATION_0(app/src/test/java/com/winlator/widget/GyroControllerTest.kt:25-50).ROTATION_180,ROTATION_270, and invert+rotation combinations are still unverified here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/widget/GyroController.java` around lines 114 - 145, Add unit tests covering Surface.ROTATION_180 and Surface.ROTATION_270 branches and combinations with invertX/invertY for the GyroController.mapToStick behavior; locate mapToStick in GyroController and extend GyroControllerTest (app/src/test/java/com/winlator/widget/GyroControllerTest.kt) to assert expected mapped outputs for raw inputs under ROTATION_180 and ROTATION_270 and repeat assertions with invertX/invertY toggled, also include small-deadzone behavior (values within 0.03 clamp to 0) and clamping to [-1,1] so each rotation/inversion path is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 2090-2101: When disabling gyro in the onGyroEnabledChanged lambda,
persist the current in-memory target before overwriting the preference with
"disabled": ensure you read the active target (controlsGyroLastTarget /
controlsGyroMode), write it to PrefManager.controlsGyroLastTarget and call
PrefManager.setGyroMode(gyroModeToPrefValue(target)) (or equivalent) so the last
target is saved, then set controlsGyroMode = GYRO_MODE_DISABLED and finally call
PrefManager.setGyroMode("disabled") and
PluviaApp.inputControlsView?.setGyroMode(GYRO_MODE_DISABLED); update the order
in onGyroEnabledChanged to save the target first to avoid losing the mapping.
In `@app/src/main/java/com/winlator/widget/GyroController.java`:
- Line 31: The code currently calls updateRegistration() from setMode(),
setEditMode(), and setHasProfile() without checking whether the view is actually
attached, allowing the gyroscope to be registered while off-screen; add an
attached-state flag updated in onAttachedToWindow() and onDetachedFromWindow()
(or use isAttachedToWindow()) and modify updateRegistration() to require that
attached==true before registering the sensor, and always unregister in
onDetachedFromWindow(); update references to isRegistered, updateRegistration(),
setMode(), setEditMode(), and setHasProfile() accordingly so registration only
happens when the view is attached and is unregistered on detach.
---
Outside diff comments:
In `@app/src/main/java/com/winlator/widget/InputControlsView.java`:
- Around line 290-300: The bug: when clearing the active ControlsProfile in
setProfile the code sets this.profile = null before informing GyroController,
which lets updateGyroStick() short-circuit and leaves the last gyro deflection
latched; to fix, ensure gyro output is cleared before dropping the profile by
calling GyroController.clearCurrentStick() (or at minimum calling
gyroController.setHasProfile(false)) while this.profile is still non-null —
adjust setProfile so you either call gyroController.clearCurrentStick() then
reset this.profile to null and resetThumbContributions(), or call
gyroController.setHasProfile(false) before setting this.profile = null; update
the method references setProfile, gyroController.setHasProfile,
GyroController.clearCurrentStick, resetThumbContributions, deselectAllElements
accordingly.
---
Nitpick comments:
In `@app/src/main/java/com/winlator/widget/GyroController.java`:
- Around line 114-145: Add unit tests covering Surface.ROTATION_180 and
Surface.ROTATION_270 branches and combinations with invertX/invertY for the
GyroController.mapToStick behavior; locate mapToStick in GyroController and
extend GyroControllerTest
(app/src/test/java/com/winlator/widget/GyroControllerTest.kt) to assert expected
mapped outputs for raw inputs under ROTATION_180 and ROTATION_270 and repeat
assertions with invertX/invertY toggled, also include small-deadzone behavior
(values within 0.03 clamp to 0) and clamping to [-1,1] so each
rotation/inversion path is exercised.
🪄 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: e55f8b64-7b82-46ae-9c4e-715b66e65298
📒 Files selected for processing (7)
app/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/widget/GyroController.javaapp/src/main/java/com/winlator/widget/InputControlsView.javaapp/src/main/res/values/strings.xmlapp/src/test/java/com/winlator/widget/GyroControllerTest.kt
✅ Files skipped from review due to trivial changes (2)
- app/src/main/res/values/strings.xml
- app/src/test/java/com/winlator/widget/GyroControllerTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/PrefManager.kt
unbelievableflavour
force-pushed
the
gyro-aiming-v2
branch
from
25dc489 to
ecf1d18
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/winlator/widget/InputControlsView.java (1)
290-300:⚠️ Potential issue | 🟡 MinorReset thumb contributions on every profile swap.
baseThumb*/gyroThumb*are view-scoped, but they are only cleared whenprofile == null.XServerScreen.ktswitches this view between non-null profiles during in-game profile creation/save flows, so a held stick or gyro vector can bleed into the newly assigned profile until another input event overwrites it.Proposed fix
public synchronized void setProfile(ControlsProfile profile) { + if (this.profile != profile) { + resetThumbContributions(); + } if (profile != null) { this.profile = profile; deselectAllElements(); } else { this.profile = null; - resetThumbContributions(); } gyroController.setHasProfile(this.profile != null); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/widget/InputControlsView.java` around lines 290 - 300, The profile swap currently only clears view-scoped thumb state when profile == null; modify setProfile(ControlsProfile) to reset the thumb contribution fields on every profile change (not just when setting null) by invoking resetThumbContributions() whenever this.profile is replaced (e.g., call resetThumbContributions() before assigning a new profile or whenever profile != incoming profile), while preserving deselectAllElements() for non-null assignments and keeping gyroController.setHasProfile(this.profile != null).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 1163-1169: The short-circuit in XServerScreen (the handled =
physicalControllerHandler?.onGenericMotionEvent(...) then if (!handled) call to
PluviaApp.inputControlsView?.onGenericMotionEvent(...)) drops gyro mixing
because PhysicalControllerHandler.handleInputEvent(...) assigns raw stick values
directly; change the routing so both handlers can process motion events: remove
the early-exit logic and always call
PluviaApp.inputControlsView.onGenericMotionEvent(...) after
physicalControllerHandler.onGenericMotionEvent(...), or alternatively update
PhysicalControllerHandler.handleInputEvent(...) to perform the same additive
gyro blend (use baseThumb* + gyroThumb* like InputControlsView.handleInputEvent
does) for stick axes; also ensure both handlers write to the same shared gamepad
state object (the same state instance) and avoid double-applying axes/triggers
by making PhysicalControllerHandler only consume events that must stop
propagation (e.g., button/axis releases) and returning true only in those cases.
---
Outside diff comments:
In `@app/src/main/java/com/winlator/widget/InputControlsView.java`:
- Around line 290-300: The profile swap currently only clears view-scoped thumb
state when profile == null; modify setProfile(ControlsProfile) to reset the
thumb contribution fields on every profile change (not just when setting null)
by invoking resetThumbContributions() whenever this.profile is replaced (e.g.,
call resetThumbContributions() before assigning a new profile or whenever
profile != incoming profile), while preserving deselectAllElements() for
non-null assignments and keeping gyroController.setHasProfile(this.profile !=
null).
🪄 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: 4f30ad62-081a-4bf2-8817-e70471971cee
📒 Files selected for processing (7)
app/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/widget/GyroController.javaapp/src/main/java/com/winlator/widget/InputControlsView.javaapp/src/main/res/values/strings.xmlapp/src/test/java/com/winlator/widget/GyroControllerTest.kt
✅ Files skipped from review due to trivial changes (3)
- app/src/main/res/values/strings.xml
- app/src/main/java/app/gamenative/PrefManager.kt
- app/src/test/java/com/winlator/widget/GyroControllerTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
unbelievableflavour
force-pushed
the
gyro-aiming-v2
branch
from
ecf1d18 to
0e23f4f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A better gyro implementation not relying on code from original winlator implementations. This better fits in gamenative as it works for both gamepad joysticks & mouse.
Recording
Another vids can be found in discord (they were too big for the github upload):
https://discord.com/channels/1378308569287622737/1452868088277241979
Checklist
#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.CONTRIBUTING.md.Summary by CodeRabbit
New Features
Bug Fixes
Tests