# Android regression fixes — H1, M2, M3, M4, backup .tmp, generate() double-tap guard Source: `plans/reports/code-reviewer-260831-2314-precommit-review.md` Scope: `android/` only. No commit made. ## H-1 — voicePlayer.release() kills audio on relaunch-in-process Chose the **recreatable holder**, not the `cancel()` fallback: `cancel()` would fully abandon L1's intent (never releasing the ExoPlayer on explicit exit, reverting to pre-L1 behavior). The holder keeps L1's actual release on "Thoát" while surviving a relaunch into the same cached process. - New `android/app/src/main/java/com/miti99/loto/audio/VoicePlayerHolder.kt`: nullable-instance holder, `value` rebuilds via a factory after `release()`. - `LotoApplication.kt`: `voicePlayer` now backed by a private `VoicePlayerHolder`; added `releaseVoicePlayer()`. - `MainActivity.kt`: `onDestroy()` calls `releaseVoicePlayer()` instead of `voicePlayer.release()`. - Testability: `LotoApplication`/`ExoVoicePlayer` need a real `Context` (no Robolectric in this project — `testOptions.unitTests` only sets `isReturnDefaultValues`), so the recreate-after-release contract is extracted into `VoicePlayerHolder` and unit-tested in isolation (`VoicePlayerHolderTest.kt`, 4 tests) against `FakeVoicePlayer`. The `LotoApplication`/`MainActivity` wiring itself stays unverified by a JVM test, same residual gap the reviewer already flagged — out of scope to close without adding Robolectric. ## M-2 — settings/loaded sibling-collector race Replaced the two independent `stateIn` collectors PlayerBoardViewModel depended on (`settingsState: StateFlow` + a separate loaded boolean) with one: `LotoApplication.settingsOrNull: StateFlow` (null = not loaded), the sole collector over `settingsRepository.settingsFlow`. `settingsState` (used by `MainActivity`, `MasterPanelViewModel`, `SettingsViewModel`, `SettingsSheetTest`) is now *derived* from `settingsOrNull` rather than an independent collection, removing the top-level race for everyone, not just the reported call site. `PlayerBoardViewModel` now takes `settingsOrNull` directly and reads `settingsOrNull.value` for both "is it loaded" and "what is the mode" — by construction the same snapshot, so they can never disagree. `LotoViewModelFactory` updated to wire `app.settingsOrNull` (+ `app.settingsRepository.defaults` as the pre-load fallback for the rare `onCellClick`-on-a-restored-card-before-settings-load path). ## M-3 — generate()/clearMarks() bypass the settings gate - `PlayerUiState.loading` now folds in settings-loaded, not just restore: `publishLoading() = !restoreDone || settingsOrNull.value == null`, called from `restore()` and from the `init` combine's collector (which now combines `masterStore.state` with `settingsOrNull` directly — settings changes still re-trigger `onMasterState`, same as before). The existing UI gate (`state.loading` disables both buttons) now also covers the settings-unresolved window. - Defense in depth, mirroring `restore()`'s own H1 guard: `generate()` and `clearMarks()` early-return (no-op) while `settingsOrNull.value == null`, so a caller that acts before the UI gate resolves can't corrupt the auto-cross cursor under a placeholder mode. - Updated the M4-residual test in `PlayerBoardViewModelTest.kt` to actually exercise the ordering the review said it dodged: `masterStore.restore()` now resolves *before* `generate()` is tapped (previously the reverse), asserting the tap while gated is a no-op and the full replay lands once settings resolve and the user taps again. The other ordering test ("a settings arrival that lands before masterStore's restore...") is unchanged/kept. - New: `clearMarks() is also a no-op while settings have not resolved yet` and `loading stays true while settings have not resolved... (M3)`. ## M-4 — non-IO settings read failure deadlocks the gate `SettingsRepository.settingsFlow`'s `catch` now falls back to `emptyPreferences()` (→ defaults) for **any** read failure, not only `IOException` — the fail-open fix applies at the source now that `LotoApplication.settingsOrNull` is the sole downstream collector (a fix scoped only to the separate `loaded: Flow`, as the review's snippet suggested, would no longer reach the actual production consumer after the M2 unification). `loaded` is kept as a plain derived signal for tests/other consumers; it no longer needs its own `.catch` since upstream never throws. `ThrowingDataStore` gained an optional `exception: Throwable` constructor param (defaults to `IOException`, unchanged for existing callers) so tests can exercise a non-IO failure. Added `non-IO failure on read still falls back to defaults...` and `loaded still flips true after a non-IO read failure` to `SettingsRepositoryTest.kt`. ## Low — backup `.tmp` sibling Added `datastore/loto_game_state.preferences_pb.tmp` exclusion alongside the existing `.preferences_pb` one in both `data_extraction_rules.xml` and `backup_rules.xml`. ## Low — generate() double-tap guard Added `private var generating` to `PlayerBoardViewModel`: `generate()` no-ops while a previous call's off-main compute hasn't resolved yet; also folded into `restore()`'s clobber guard (`!generating`) per the review's "participates in the restore guard" suggestion, since `grid` alone can't detect an in-flight generation. Test: `a second rapid generate() tap while the first is still computing is a no-op (L-a)`, asserting `FakeVoicePlayer.cancel()` (new `cancelCount` counter) fires only once. ## Files Modified - `android/app/src/main/java/com/miti99/loto/audio/VoicePlayerHolder.kt` (new) - `android/app/src/main/java/com/miti99/loto/LotoApplication.kt` - `android/app/src/main/java/com/miti99/loto/MainActivity.kt` - `android/app/src/main/java/com/miti99/loto/settings/SettingsRepository.kt` - `android/app/src/main/java/com/miti99/loto/state/LotoViewModelFactory.kt` - `android/app/src/main/java/com/miti99/loto/state/PlayerBoardViewModel.kt` - `android/app/src/main/res/xml/data_extraction_rules.xml` - `android/app/src/main/res/xml/backup_rules.xml` - `android/app/src/test/java/com/miti99/loto/audio/VoicePlayerHolderTest.kt` (new) - `android/app/src/test/java/com/miti99/loto/audio/FakeVoicePlayer.kt` - `android/app/src/test/java/com/miti99/loto/ThrowingDataStore.kt` - `android/app/src/test/java/com/miti99/loto/settings/SettingsRepositoryTest.kt` - `android/app/src/test/java/com/miti99/loto/state/PlayerBoardViewModelTest.kt` Untouched by me but pre-existing modified/untracked in the working tree (part of the earlier fix cycle under review, not this task): `build.gradle.kts`, `AndroidManifest.xml`, `GameStateRepository.kt`, `GameStateRepositoryTest.kt`, `MasterPanelViewModel.kt`, `MasterPanelViewModelTest.kt`, `MasterStore.kt`, `MasterStoreTest.kt`, `SlowDataStore.kt`, various `ui/*` files, `strings.xml`. ## Tests Status - `JAVA_HOME=jdk-21 ./gradlew :app:testDebugUnitTest` — **BUILD SUCCESSFUL**, 129 tests, 0 failures, 0 errors (was 120; +9 new: 4 `VoicePlayerHolderTest`, 3 `PlayerBoardViewModelTest`, 2 `SettingsRepositoryTest`). - `JAVA_HOME=jdk-21 ./gradlew :app:lintDebug` — **BUILD SUCCESSFUL**, 0 errors, 17 warnings (all pre-existing: dependency-version notices, one Vietnamese string typo heuristic, two plurals-candidate heuristics, `enableOnBackInvokedCallback` API-33 note — none touch the changed files). ## Issues Encountered None. No file-ownership conflicts (single-agent task). `local.properties` was not read. ## Unresolved Questions 1. H-1: the `LotoApplication`/`MainActivity` glue around `VoicePlayerHolder` (real `finish()` + relaunch through a real `Application`/`Context`) is still not exercised by any automated test — this project has no Robolectric dependency, and adding one was out of scope. The holder's own recreate-after-release contract is unit-tested; the wiring is equivalent to what the reviewer already called out as untested. 2. `fallbackSettings` for `PlayerBoardViewModel` (used only if `onCellClick` fires on a restored card before settings ever resolve) is wired to the real manifest-derived defaults in production (`app.settingsRepository.defaults`) and to a placeholder `Settings(voice = "")` in the constructor's own default parameter (never hit by existing tests, all of which pass a non-null-typed settings flow). Flagging this only so the placeholder default doesn't surprise a future reader who instantiates `PlayerBoardViewModel` with a null-capable flow without supplying `fallbackSettings`. Status: DONE Summary: Fixed H1 (recreatable voice player holder), M2 (single settingsOrNull source of truth), M3 (settings-loaded folded into loading + generate()/clearMarks() guards, test reordered to hit the previously-dodged case), M4 (fail-open on any settingsFlow read failure), plus the backup .tmp exclusion and a generate() double-tap/in-flight guard; all 129 unit tests and lint pass clean.