**Actionable comments posted: 5**

🧹 Nitpick comments (2)

app/src/main/java/app/gamenative/PrefManager.kt (1)
`219-231`: **Normalize vibration preference values at the persistence boundary.** `vibrationMode` currently persists arbitrary strings, and `vibrationIntensity` is only bounded on write. Normalizing on read+write avoids undefined behavior from stale/corrupt prefs.

♻️ Suggested hardening patch
```diff + private val VALID_VIBRATION_MODES = setOf("off", "controller", "device", "both") + private val VIBRATION_MODE = stringPreferencesKey("vibration_mode") var vibrationMode: String - get() = getPref(VIBRATION_MODE, "controller") + get() = getPref(VIBRATION_MODE, "controller") + .takeIf { it in VALID_VIBRATION_MODES } ?: "controller" set(value) { - setPref(VIBRATION_MODE, value) + setPref(VIBRATION_MODE, value.takeIf { it in VALID_VIBRATION_MODES } ?: "controller") } private val VIBRATION_INTENSITY = intPreferencesKey("vibration_intensity") var vibrationIntensity: Int - get() = getPref(VIBRATION_INTENSITY, 100) + get() = getPref(VIBRATION_INTENSITY, 100).coerceIn(0, 100) set(value) { setPref(VIBRATION_INTENSITY, value.coerceIn(0, 100)) } ```

🤖 Prompt for AI Agents
``` Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/PrefManager.kt` around lines 219 - 231, Persisted vibration preferences aren't normalized on read and only partially bounded on write, so stale or corrupt values can cause undefined behavior; update vibrationMode and vibrationIntensity accessors to validate and normalize both on get and set: for vibrationMode (symbol VIBRATION_MODE and property vibrationMode) accept only a defined set of allowed values (e.g., include "controller" as the default) and on get return the default if stored value is missing/unknown, and on set constrain input to the allowed set before calling setPref; for vibrationIntensity (symbol VIBRATION_INTENSITY and property vibrationIntensity) apply value.coerceIn(0,100) on both set and when reading from getPref (or fallback to the default 100 if stored value is out of range) so getPref/getters never return invalid values; use existing getPref and setPref helpers to centralize this normalization. ```

app/src/main/java/com/winlator/winhandler/WinHandler.java (1)
`104-107`: **Validate `vibrationMode` before storing it.** This is persisted as a free-form string, and an unexpected value or casing currently falls through every branch in `startVibrationForPlayer()` while still marking the slot as rumbling. Normalizing to the supported set here avoids silent no-op sessions from malformed container data.

Suggested normalization
```diff public void setVibrationMode(String mode) { - this.vibrationMode = mode != null ? mode : "controller"; + String normalized = mode == null ? "controller" : mode.toLowerCase(java.util.Locale.ROOT); + switch (normalized) { + case "off": + case "controller": + case "device": + case "both": + this.vibrationMode = normalized; + break; + default: + Log.w(TAG, "Unknown vibration mode '" + mode + "', defaulting to controller"); + this.vibrationMode = "controller"; + break; + } Log.i(TAG, "Vibration mode set to: " + this.vibrationMode); } ```

🤖 Prompt for AI Agents
``` Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/winhandler/WinHandler.java` around lines 104 - 107, Validate and normalize the incoming vibrationMode in setVibrationMode before storing: create a canonical set (e.g., SUPPORTED_VIBRATION_MODES) and normalize the input (trim + toLowerCase) and map known aliases to canonical values; if the normalized value is not in SUPPORTED_VIBRATION_MODES, fall back to "controller" and log the normalization. Update setVibrationMode to use this check/mapping so vibrationMode only ever contains a supported value and startVibrationForPlayer will not receive unexpected free-form strings. ```

🤖 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/cpp/extras/evshim.c`: - Around line 78-90: OnRumble() is writing last_rumble_low/last_rumble_high concurrently with vjoy_updater() reading them, causing a data race; acquire shm_mutex around updates and the pwrite snapshot so the pair of low/high values are written atomically (i.e., lock shm_mutex, update last_rumble_low[idx] and last_rumble_high[idx] and prepare vals[], call pwrite while still holding the mutex, then unlock), but move any call to SDL_JoystickRumble() to after unlocking; apply the same mutex-protected snapshot fix to the other occurrence that updates last_rumble_* (the block referenced at lines 161-167) so both sites use shm_mutex for consistent reads/writes. In `@app/src/main/java/app/gamenative/ui/component/dialog/ControllerTab.kt`: - Around line 77-104: The UI shows a mismatch because vibrationModeIndex is derived from vibrationModeValues using config.vibrationMode but the slider visibility checks the raw config.vibrationMode; normalize the vibration mode once (e.g., map/normalize config.vibrationMode to a valid value from vibrationModeValues) before using it in the UI and persist that normalized value back into state.config (or use a local normalizedMode for both the dropdown and the slider visibility check). Update the logic around vibrationModeIndex, the remember block for intensitySlider, and the conditional "if (config.vibrationMode != \"off\")" to use the normalized value (reference: vibrationModeIndex, vibrationModeValues, config.vibrationMode, state.config.value, intensitySlider). In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`: - Around line 1546-1547: The code passes raw extras to WinHandler.setVibrationMode via handler.setVibrationMode(container.getExtra("vibrationMode", "controller")), which can store variants like "Controller" and later break startVibrationForPlayer that only recognizes "off/controller/device/both"; normalize the value before calling setVibrationMode (e.g. read container.getExtra("vibrationMode", "controller"), trim and toLowerCase it, then map common synonyms/capitalizations to the canonical tokens "off", "controller", "device", or "both") so WinHandler.setVibrationMode always receives one of the accepted lowercase values. In `@app/src/main/java/app/gamenative/utils/ContainerUtils.kt`: - Around line 146-147: The new container creation is bypassing the updated defaults: update createNewContainer(...) so the ContainerData it constructs includes vibrationMode and vibrationIntensity from PrefManager (or simply initialize it from getDefaultContainerData()) instead of hardcoding "controller"/100; modify the ContainerData construction inside createNewContainer to set vibrationMode = PrefManager.vibrationMode and vibrationIntensity = PrefManager.vibrationIntensity (or replace the manual builder with a call to getDefaultContainerData()) to ensure fresh containers respect saved defaults. In `@app/src/main/java/com/winlator/winhandler/WinHandler.java`: - Around line 616-645: The keepalive logic is re-triggering the 60s phone one-shot because unchanged rumble calls startVibrationForPlayer(p, lowFreq, highFreq) which routes "device"/"both" into vibrateDevice(); change the keepalive path so device vibration is not restarted every keepalive tick—either add a new method like refreshDeviceVibrationKeepalive(playerId, lowFreq, highFreq) or add a flag/overload to startVibrationForPlayer to distinguish initial/start vs keepalive; ensure the initial start still calls vibrateDevice() with DEVICE_RUMBLE_MS but keepalive calls a separate device refresh with a much longer cadence (or a no-op for device) and adjust rumbleKeepaliveCtr handling accordingly so controller rumble continues to be refreshed while device vibration is not restarted every ~240ms. --- Nitpick comments: In `@app/src/main/java/app/gamenative/PrefManager.kt`: - Around line 219-231: Persisted vibration preferences aren't normalized on read and only partially bounded on write, so stale or corrupt values can cause undefined behavior; update vibrationMode and vibrationIntensity accessors to validate and normalize both on get and set: for vibrationMode (symbol VIBRATION_MODE and property vibrationMode) accept only a defined set of allowed values (e.g., include "controller" as the default) and on get return the default if stored value is missing/unknown, and on set constrain input to the allowed set before calling setPref; for vibrationIntensity (symbol VIBRATION_INTENSITY and property vibrationIntensity) apply value.coerceIn(0,100) on both set and when reading from getPref (or fallback to the default 100 if stored value is out of range) so getPref/getters never return invalid values; use existing getPref and setPref helpers to centralize this normalization. In `@app/src/main/java/com/winlator/winhandler/WinHandler.java`: - Around line 104-107: Validate and normalize the incoming vibrationMode in setVibrationMode before storing: create a canonical set (e.g., SUPPORTED_VIBRATION_MODES) and normalize the input (trim + toLowerCase) and map known aliases to canonical values; if the normalized value is not in SUPPORTED_VIBRATION_MODES, fall back to "controller" and log the normalization. Update setVibrationMode to use this check/mapping so vibrationMode only ever contains a supported value and startVibrationForPlayer will not receive unexpected free-form strings. ```

🪄 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**: `2ea17b0f-27f1-4d46-b5d7-819da35201fa`

📥 Commits

Reviewing files that changed from the base of the PR and between a29bb8953556449ab4acc876aef7df2ad164e471 and 0998922be44ed8ebde6803f956e839f747f85059.

⛔ Files ignored due to path filters (1)

* `app/src/main/jniLibs/arm64-v8a/libevshim.so` is excluded by `!**/*.so`

📒 Files selected for processing (11)

* `app/src/main/cpp/extras/CMakeLists.txt` * `app/src/main/cpp/extras/evshim.c` * `app/src/main/cpp/extras/sdl2_stub/SDL2/SDL.h` * `app/src/main/java/app/gamenative/PrefManager.kt` * `app/src/main/java/app/gamenative/ui/component/dialog/ControllerTab.kt` * `app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` * `app/src/main/java/app/gamenative/utils/ContainerUtils.kt` * `app/src/main/java/com/winlator/container/ContainerData.kt` * `app/src/main/java/com/winlator/winhandler/WinHandler.java` * `app/src/main/java/com/winlator/xenvironment/components/BionicProgramLauncherComponent.java` * `app/src/main/res/values/strings.xml`