forked from unom/punktfunk
fix(clients/android): the console's Android rows were nested where serde flattens them
Turning "Controller-optimized UI" off in the console did nothing: the console stayed up, because the setting never left the console. `trust::Settings::extra` is `#[serde(flatten)]`, so the `android.*` keys are TOP-LEVEL keys of the settings document, beside `width` and `codec`. `ConsoleJson` wrote and read them nested under an `"extra"` object instead. Serde put that whole object into the map under the literal key `"extra"`, so no console row ever found `android.gamepad_ui_enabled` — the row showed its own default, and the value the console saved came back to Kotlin as the one Kotlin had just sent. `applySettings` then saw no change, raised no callback, and `App` never recomputed `gamepadUiActive`. Every Android-only row rode the same broken path: low latency, phone rumble/gyro, SC2 and DualSense capture, and the console-UI mode picker. A store written by the nesting build carries the dead wrapper; it is dropped on the next write rather than echoed for the life of the install. The new test pins the shape from both sides. A round-trip alone could not have caught this — both halves agreed on the same wrong nesting, which is exactly how it survived review.
This commit is contained in:
@@ -317,16 +317,20 @@ internal object ConsoleJson {
|
||||
j.put("invert_scroll", s.invertScroll)
|
||||
j.put("pad_haptics", s.padHaptics)
|
||||
j.put("pad_speaker", if (s.padSpeaker) "pad" else "off")
|
||||
// Android-only rows ride `extra` (WP5 gives them RowIds); nothing on the desktop reads them.
|
||||
val extra = j.optJSONObject("extra") ?: JSONObject()
|
||||
extra.put("android.low_latency", s.lowLatencyMode)
|
||||
extra.put("android.rumble_on_phone", s.rumbleOnPhone)
|
||||
extra.put("android.gyro_on_phone", s.gyroOnPhone)
|
||||
extra.put("android.sc2_capture", s.sc2Capture)
|
||||
extra.put("android.ds_capture", s.dsCapture)
|
||||
extra.put("android.gamepad_ui_mode", s.gamepadUiMode)
|
||||
extra.put("android.gamepad_ui_enabled", s.gamepadUiEnabled)
|
||||
j.put("extra", extra)
|
||||
// Android-only rows ride `Settings::extra`, which is `#[serde(flatten)]` — so they are
|
||||
// TOP-LEVEL keys of this document, not a nested `extra` object. Nesting them put the
|
||||
// whole object into the map under the literal key "extra", where no console row could
|
||||
// read it and every value the console wrote came straight back as the one we had sent.
|
||||
j.put("android.low_latency", s.lowLatencyMode)
|
||||
j.put("android.rumble_on_phone", s.rumbleOnPhone)
|
||||
j.put("android.gyro_on_phone", s.gyroOnPhone)
|
||||
j.put("android.sc2_capture", s.sc2Capture)
|
||||
j.put("android.ds_capture", s.dsCapture)
|
||||
j.put("android.gamepad_ui_mode", s.gamepadUiMode)
|
||||
j.put("android.gamepad_ui_enabled", s.gamepadUiEnabled)
|
||||
// A store written by the nesting build carries the stale wrapper; drop it rather than
|
||||
// round-trip a copy of these keys that nothing reads for the life of the install.
|
||||
j.remove("extra")
|
||||
return j
|
||||
}
|
||||
|
||||
@@ -336,7 +340,8 @@ internal object ConsoleJson {
|
||||
*/
|
||||
fun applySettings(s: Settings, j: JSONObject): Settings {
|
||||
fun str(k: String, cur: String) = j.optString(k, cur).ifEmpty { cur }
|
||||
val extra = j.optJSONObject("extra") ?: JSONObject()
|
||||
// The `android.*` keys are TOP-LEVEL here, not nested: `Settings::extra` is
|
||||
// `#[serde(flatten)]`, so the console writes them beside `width` and `codec`.
|
||||
return s.copy(
|
||||
width = j.optInt("width", s.width),
|
||||
height = j.optInt("height", s.height),
|
||||
@@ -373,14 +378,14 @@ internal object ConsoleJson {
|
||||
"off" -> false
|
||||
else -> s.padSpeaker
|
||||
},
|
||||
lowLatencyMode = extra.optBoolean("android.low_latency", s.lowLatencyMode),
|
||||
rumbleOnPhone = extra.optBoolean("android.rumble_on_phone", s.rumbleOnPhone),
|
||||
gyroOnPhone = extra.optBoolean("android.gyro_on_phone", s.gyroOnPhone),
|
||||
sc2Capture = extra.optBoolean("android.sc2_capture", s.sc2Capture),
|
||||
dsCapture = extra.optBoolean("android.ds_capture", s.dsCapture),
|
||||
gamepadUiMode = extra.optString("android.gamepad_ui_mode", s.gamepadUiMode)
|
||||
lowLatencyMode = j.optBoolean("android.low_latency", s.lowLatencyMode),
|
||||
rumbleOnPhone = j.optBoolean("android.rumble_on_phone", s.rumbleOnPhone),
|
||||
gyroOnPhone = j.optBoolean("android.gyro_on_phone", s.gyroOnPhone),
|
||||
sc2Capture = j.optBoolean("android.sc2_capture", s.sc2Capture),
|
||||
dsCapture = j.optBoolean("android.ds_capture", s.dsCapture),
|
||||
gamepadUiMode = j.optString("android.gamepad_ui_mode", s.gamepadUiMode)
|
||||
.ifEmpty { s.gamepadUiMode },
|
||||
gamepadUiEnabled = extra.optBoolean("android.gamepad_ui_enabled", s.gamepadUiEnabled),
|
||||
gamepadUiEnabled = j.optBoolean("android.gamepad_ui_enabled", s.gamepadUiEnabled),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
package io.unom.punktfunk
|
||||
|
||||
import io.unom.punktfunk.console.ConsoleJson
|
||||
import org.json.JSONObject
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* The Android-only console settings ride `trust::Settings::extra`, which is `#[serde(flatten)]`:
|
||||
* they are TOP-LEVEL keys of the settings document, beside `width` and `codec`.
|
||||
*
|
||||
* They were written and read nested under an `"extra"` object instead. Serde put that whole
|
||||
* object into the map under the literal key `"extra"`, so no console row ever found
|
||||
* `android.gamepad_ui_enabled` — and the value the console saved came back to Kotlin as the one
|
||||
* Kotlin had just sent. On glass that was a "Controller-optimized UI" switch you could turn off
|
||||
* with nothing happening: the console stayed up, because the setting never moved.
|
||||
*/
|
||||
class ConsoleSettingsExtraTest {
|
||||
@Test
|
||||
fun androidKeysAreWrittenFlat() {
|
||||
val j = ConsoleJson.settings(Settings(gamepadUiEnabled = false, lowLatencyMode = false), null)
|
||||
assertTrue("the console reads this key at the top level", j.has("android.gamepad_ui_enabled"))
|
||||
assertFalse(j.getBoolean("android.gamepad_ui_enabled"))
|
||||
assertFalse(j.getBoolean("android.low_latency"))
|
||||
assertFalse("a nested wrapper is what serde swallows whole", j.has("extra"))
|
||||
}
|
||||
|
||||
/** A store written by the nesting build must not keep echoing its dead wrapper. */
|
||||
@Test
|
||||
fun aStaleNestedWrapperIsDropped() {
|
||||
val base = JSONObject().put(
|
||||
"extra",
|
||||
JSONObject().put("android.gamepad_ui_enabled", true),
|
||||
)
|
||||
assertFalse(ConsoleJson.settings(Settings(gamepadUiEnabled = false), base).has("extra"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun theConsolesOwnSaveIsReadBack() {
|
||||
val saved = JSONObject()
|
||||
.put("android.gamepad_ui_enabled", false)
|
||||
.put("android.gamepad_ui_mode", GAMEPAD_UI_ALWAYS)
|
||||
.put("android.ds_capture", false)
|
||||
val next = ConsoleJson.applySettings(Settings(), saved)
|
||||
assertFalse("turning the console off must reach the store", next.gamepadUiEnabled)
|
||||
assertEquals(GAMEPAD_UI_ALWAYS, next.gamepadUiMode)
|
||||
assertFalse(next.dsCapture)
|
||||
}
|
||||
|
||||
/** Both halves against each other — the shape only holds if they agree. */
|
||||
@Test
|
||||
fun theRoundTripKeepsEveryAndroidRow() {
|
||||
val want = Settings(
|
||||
gamepadUiEnabled = false,
|
||||
gamepadUiMode = GAMEPAD_UI_ALWAYS,
|
||||
lowLatencyMode = false,
|
||||
rumbleOnPhone = true,
|
||||
gyroOnPhone = true,
|
||||
sc2Capture = false,
|
||||
dsCapture = false,
|
||||
)
|
||||
val got = ConsoleJson.applySettings(Settings(), ConsoleJson.settings(want, null))
|
||||
assertEquals(want.gamepadUiEnabled, got.gamepadUiEnabled)
|
||||
assertEquals(want.gamepadUiMode, got.gamepadUiMode)
|
||||
assertEquals(want.lowLatencyMode, got.lowLatencyMode)
|
||||
assertEquals(want.rumbleOnPhone, got.rumbleOnPhone)
|
||||
assertEquals(want.gyroOnPhone, got.gyroOnPhone)
|
||||
assertEquals(want.sc2Capture, got.sc2Capture)
|
||||
assertEquals(want.dsCapture, got.dsCapture)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user