diff --git a/README.md b/README.md index 11a50e8..377d574 100644 --- a/README.md +++ b/README.md @@ -17,8 +17,8 @@ machinery for it is still there and still tested: a game listed in `GameCatalog` with `available = false` appears in the menu marked *Coming soon* **on dev builds only**, so a production release lists just what can actually be played and nobody installs it and taps into a dead end. The menu decides this from the installed -APK's own version name, so it follows the build rather than the channel selected -for future updates. +APK's own version name — the same source the updater uses to know which channel +it is on. Adding a game means writing a `GameRules` implementation, a `GameAi` and a screen, then flipping `available` — the menu, lobby, networking, settings and OTA @@ -91,13 +91,14 @@ it: every rule above is a pure function and unit-tested on the JVM. - **Sound** on/off. - **Animation speed**, 0.5×–2×. One multiplier drives card movement and how long the computer appears to think. -- **Updates** — check on launch, or check on demand, from either of two - channels: - - **Production** — stable builds, published from `main`. - - **Dev** — preview builds, published from `dev` on every green CI run. +- **Updates** — check on launch, or check on demand. **There is no channel to + choose**: a build updates from its own channel and no other. A production + install is offered production builds from `main`; a dev install is offered dev + builds from `dev`. The settings screen states which one is running rather than + offering a switch. - The two install as separate apps, so both can be kept on one device — see - [Both channels on one device](#both-channels-on-one-device). + Wanting both means installing both — they are separate apps and each updates + itself, see [Both channels on one device](#both-channels-on-one-device). ## Branding @@ -184,8 +185,9 @@ the versioned copy, so an installed build can always be traced back to its tag. There is no equivalent link for the dev channel: `/releases/latest` skips prereleases by design, and that is exactly what keeps a production install from -being handed a dev build. Dev builds are reached through the in-app updater with -the channel set to dev, or from the releases page. +being handed a dev build. A dev build is reached by the in-app updater of a dev +install — which looks at the prerelease list rather than that endpoint — or from +the releases page. ### The version series @@ -239,9 +241,16 @@ aimed elsewhere does nothing rather than shipping unreviewed code. **The two channels have independent version sequences** — a build's `versionCode` is the run number of the workflow that produced it, and the two workflows count -separately. Versions are therefore only ever compared *within* a channel; the app -reads the `-dev` suffix in its own `versionName` to know which channel it is on, -and treats a change of channel as an explicit switch rather than an upgrade. +separately. Dev is on 69 while production is on 40, so the numbers say nothing +across the divide. + +That is why **an install only ever updates within its own channel**, and why +there is no setting for it. The app reads the `-dev` suffix in its own +`versionName` to know which line it is on, asks only that channel, and refuses +anything that comes back belonging to the other one — checked again at the point +of decision rather than trusted from the fetch. The rule is a pure function, +`verdictFor` in `update/UpdatePolicy.kt`, with no Android in it, so it is +unit-tested on the JVM like the rules engines are. ### Both channels on one device @@ -272,9 +281,10 @@ reference `@color/tartan_field` rather than a literal, which is how one set of vectors serves both colourways. `BuildConfig.DEV_BUILD` carries the same flag into Kotlin, where `AppSett` picks the matching Compose sett for the splash. -Switching channels in Settings therefore installs the other channel's app -alongside this one and leaves this one in place; the settings screen says so -when the selected channel is not the installed one. +Because they are separate apps, having both is a matter of installing both, and +each then updates itself down its own line. Nothing in the app moves a phone +from one channel to the other: the settings screen states which build is +running and where its updates come from, and that is all it does. The debug keystore is committed on purpose: every build is signed with the same key, so an OTA update installs over the previous one instead of being rejected @@ -331,7 +341,7 @@ game/ rules engines and AI — pure Kotlin, no Android, unit-tested net/ wire protocol, LAN transport, lobby, match driver score/ scorekeeper sheet — pure Kotlin, unit-tested — and its store settings/ DataStore-backed preferences -update/ GitHub Releases OTA updater +update/ GitHub Releases OTA updater, and the channel rule — pure, tested ui/ Compose screens ``` diff --git a/app/src/main/java/org/prolibertate/games/settings/SettingsRepository.kt b/app/src/main/java/org/prolibertate/games/settings/SettingsRepository.kt index c086124..ccbb2d0 100644 --- a/app/src/main/java/org/prolibertate/games/settings/SettingsRepository.kt +++ b/app/src/main/java/org/prolibertate/games/settings/SettingsRepository.kt @@ -10,7 +10,6 @@ import androidx.datastore.preferences.core.stringPreferencesKey import androidx.datastore.preferences.preferencesDataStore import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map -import org.prolibertate.games.update.UpdateChannel private val Context.dataStore: DataStore by preferencesDataStore(name = "settings") @@ -32,8 +31,6 @@ data class Settings( * snapped back before a real name could be typed. */ val playerName: String = "", - /** Which builds the updater offers. Production unless deliberately changed. */ - val updateChannel: UpdateChannel = UpdateChannel.PRODUCTION, ) { /** * The name other players actually see. This is where the default lives, so @@ -60,7 +57,11 @@ class SettingsRepository(private val context: Context) { val SPEED = floatPreferencesKey("animation_speed") val UPDATE_ON_LAUNCH = booleanPreferencesKey("check_updates_on_launch") val NAME = stringPreferencesKey("player_name") - val CHANNEL = stringPreferencesKey("update_channel") + // There is deliberately no update-channel key. The channel is a + // property of the installed APK, not a preference — an install that + // could be pointed at the other channel's builds is exactly the mix-up + // that separate applicationIds exist to prevent. Anything an older + // version left under "update_channel" is simply never read again. } val settings: Flow = context.dataStore.data.map { prefs -> @@ -69,16 +70,9 @@ class SettingsRepository(private val context: Context) { animationSpeed = prefs[Keys.SPEED] ?: 1.0f, checkForUpdatesOnLaunch = prefs[Keys.UPDATE_ON_LAUNCH] ?: true, playerName = prefs[Keys.NAME] ?: "", - updateChannel = prefs[Keys.CHANNEL] - ?.let { stored -> runCatching { UpdateChannel.valueOf(stored) }.getOrNull() } - ?: UpdateChannel.PRODUCTION, ) } - suspend fun setUpdateChannel(channel: UpdateChannel) { - context.dataStore.edit { it[Keys.CHANNEL] = channel.name } - } - suspend fun setSoundEnabled(enabled: Boolean) { context.dataStore.edit { it[Keys.SOUND] = enabled } } diff --git a/app/src/main/java/org/prolibertate/games/ui/SettingsScreen.kt b/app/src/main/java/org/prolibertate/games/ui/SettingsScreen.kt index ebfea7e..26410ef 100644 --- a/app/src/main/java/org/prolibertate/games/ui/SettingsScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/SettingsScreen.kt @@ -8,7 +8,6 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.verticalScroll import androidx.compose.material3.Divider -import androidx.compose.material3.FilterChip import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Slider @@ -26,8 +25,9 @@ import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp import kotlinx.coroutines.launch import org.prolibertate.games.settings.Settings +import org.prolibertate.games.update.Release import org.prolibertate.games.update.UpdateChannel -import org.prolibertate.games.update.Updater +import org.prolibertate.games.update.UpdateVerdict @Composable fun SettingsScreen( @@ -42,7 +42,7 @@ fun SettingsScreen( // caret jumped and a cleared field refilled itself. var editedName by remember { mutableStateOf(null) } var updateStatus by remember { mutableStateOf(null) } - var pendingRelease by remember { mutableStateOf(null) } + var pendingRelease by remember { mutableStateOf(null) } ScreenScaffold(title = "Settings", onBack = onBack) { modifier -> Column( @@ -111,45 +111,32 @@ fun SettingsScreen( Divider() Text("Updates", fontWeight = FontWeight.Bold, style = MaterialTheme.typography.titleMedium) - Text( - text = "Installed: v${env.updater.installedVersionName()} " + - "(${env.updater.installedChannel().label} channel).", - style = MaterialTheme.typography.bodySmall, - ) + // The channel is not a setting. It is read off the running APK, and + // this build updates down its own line and no other — so it is + // stated here rather than offered. + val channel = env.updater.installedChannel() Column { - Text("Update channel", fontWeight = FontWeight.Bold) - Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - UpdateChannel.entries.forEach { channel -> - FilterChip( - selected = settings.updateChannel == channel, - onClick = { - scope.launch { - env.settingsRepository.setUpdateChannel(channel) - } - updateStatus = null - pendingRelease = null - }, - label = { Text(channel.label) }, - ) - } - } Text( - text = settings.updateChannel.blurb + ".", + text = "Installed: v${env.updater.installedVersionName()} — " + + "${channel.label} channel.", + style = MaterialTheme.typography.bodySmall, + ) + Text( + text = "${channel.blurb}. This build updates from the " + + "${channel.label} channel only.", + style = MaterialTheme.typography.bodySmall, + ) + Text( + text = if (channel == UpdateChannel.DEV) { + "The production app is a separate install with its own updates. " + + "Install it from the releases page if you want both." + } else { + "Dev builds are a separate install with their own updates, from " + + "the releases page." + }, style = MaterialTheme.typography.bodySmall, ) - if (settings.updateChannel != env.updater.installedChannel()) { - // Each channel installs under its own package, so the other - // one arrives as a second app rather than replacing this - // one — no downgrade for Android to refuse. - Text( - text = "You're on a ${env.updater.installedChannel().label} build. " + - "The two channels install as separate apps, so this adds the " + - "${settings.updateChannel.label} build alongside this one and " + - "leaves it in place.", - style = MaterialTheme.typography.bodySmall, - ) - } } SettingRow( @@ -166,26 +153,20 @@ fun SettingsScreen( PrimaryAction(text = "Check for updates now") { scope.launch { - updateStatus = "Checking the ${settings.updateChannel.label} channel…" - when (val result = env.updater.check(settings.updateChannel)) { - is Updater.Result.UpToDate -> { - updateStatus = - "You're on the latest ${settings.updateChannel.label} build." + updateStatus = "Checking the ${channel.label} channel…" + when (val verdict = env.updater.check()) { + is UpdateVerdict.UpToDate -> { + updateStatus = "You're on the latest ${channel.label} build." pendingRelease = null } - is Updater.Result.Available -> { - pendingRelease = result.release - updateStatus = if (result.isChannelSwitch) { - "${result.release.tag} is the current " + - "${result.release.channel.label} build." - } else { - "Version ${result.release.tag} is available." - } + is UpdateVerdict.Install -> { + pendingRelease = verdict.release + updateStatus = "Version ${verdict.release.tag} is available." } - is Updater.Result.Failed -> { - updateStatus = result.reason + is UpdateVerdict.Refused -> { + updateStatus = verdict.reason pendingRelease = null } } diff --git a/app/src/main/java/org/prolibertate/games/update/UpdatePolicy.kt b/app/src/main/java/org/prolibertate/games/update/UpdatePolicy.kt new file mode 100644 index 0000000..b1eac5c --- /dev/null +++ b/app/src/main/java/org/prolibertate/games/update/UpdatePolicy.kt @@ -0,0 +1,99 @@ +package org.prolibertate.games.update + +/** + * Which line of builds an APK belongs to. + * + * This is a property of the build, never a preference. The two channels are + * published by different workflows and have entirely separate version + * sequences — a dev build's versionCode is its CI run number, a production + * build's is its release run number, and the two are unrelated. There is no + * meaning to be had from comparing across them, so nothing does. + * + * They also carry different applicationIds — a dev build installs as + * `…games.dev` — so both can sit on one device, each updating itself down its + * own line and leaving the other alone. + */ +enum class UpdateChannel(val label: String, val blurb: String) { + PRODUCTION("Production", "Stable builds released from main"), + DEV("Dev", "Preview builds from the dev branch — newer, less tested"), +} + +/** Marks a tag or a version name as belonging to the dev channel. */ +const val DEV_SUFFIX = "-dev" + +/** One published build, as read off a GitHub release. */ +data class Release( + val tag: String, + val versionCode: Int, + val apkUrl: String, + val apkName: String, + val channel: UpdateChannel, +) + +sealed interface UpdateVerdict { + data class Install(val release: Release) : UpdateVerdict + data object UpToDate : UpdateVerdict + data class Refused(val reason: String) : UpdateVerdict +} + +/** + * The channel an installed build belongs to, read from its own version name. + * + * The APK says what it is, so this cannot disagree with what is actually + * running — which is the whole point of not storing it anywhere. + */ +fun channelOfVersionName(versionName: String): UpdateChannel = + if (versionName.endsWith(DEV_SUFFIX)) UpdateChannel.DEV else UpdateChannel.PRODUCTION + +/** + * The channel a release belongs to. + * + * Two independent marks, and either is enough: the `-dev` suffix the workflow + * puts on the tag, and GitHub's own prerelease flag. A release would have to + * lose both to be mistaken for the other channel. + */ +fun channelOfTag(tag: String, prerelease: Boolean): UpdateChannel = + if (tag.endsWith(DEV_SUFFIX) || prerelease) UpdateChannel.DEV else UpdateChannel.PRODUCTION + +/** + * The run number out of a `v.` tag, which is what an APK's + * versionCode is. + * + * Only the trailing number is read. The series is a name for people, and + * bumping it must never make an older build look newer. + */ +fun versionCodeOfTag(tag: String): Int? = + tag.substringAfterLast('.').removeSuffix(DEV_SUFFIX).toIntOrNull() + +/** + * Whether [latest] should be offered to a build of [installedChannel] at + * [installedVersionCode]. + * + * **A build only ever updates within its own channel.** A dev install is + * offered dev builds and a production install production ones; there is no + * setting for this and no way to cross over, because crossing over is not an + * upgrade — it is a different app with an unrelated version sequence, and + * offering it as though it were newer is how a phone ends up being handed a + * build nobody meant it to have. + * + * The channel is checked here rather than trusted from the fetch, so a release + * that does not belong to this build is refused whatever the endpoint returned. + */ +fun verdictFor( + latest: Release?, + installedChannel: UpdateChannel, + installedVersionCode: Int, +): UpdateVerdict = when { + latest == null -> + UpdateVerdict.Refused("No ${installedChannel.label.lowercase()} build has been published yet.") + + latest.channel != installedChannel -> + UpdateVerdict.Refused( + "${latest.tag} is a ${latest.channel.label.lowercase()} build, and this is a " + + "${installedChannel.label.lowercase()} install. The two update separately." + ) + + latest.versionCode > installedVersionCode -> UpdateVerdict.Install(latest) + + else -> UpdateVerdict.UpToDate +} diff --git a/app/src/main/java/org/prolibertate/games/update/Updater.kt b/app/src/main/java/org/prolibertate/games/update/Updater.kt index 67aa69d..598bf7a 100644 --- a/app/src/main/java/org/prolibertate/games/update/Updater.kt +++ b/app/src/main/java/org/prolibertate/games/update/Updater.kt @@ -14,27 +14,6 @@ import java.io.File import java.net.HttpURLConnection import java.net.URL -/** - * Which builds the updater offers. - * - * The two channels are published by different workflows and so have entirely - * separate version sequences — a dev build's versionCode is its CI run number, - * a production build's is its release run number, and the two are unrelated. - * Versions are therefore only ever compared *within* a channel; moving between - * them is treated as an explicit switch rather than an upgrade. - * - * The two also carry different applicationIds — a dev build installs as - * `…games.dev` — so a switch installs the other channel beside this one instead - * of over it, and both can be kept on the same device. - */ -enum class UpdateChannel(val label: String, val blurb: String) { - PRODUCTION("Production", "Stable builds released from main"), - DEV("Dev", "Preview builds from the dev branch — newer, less tested"), -} - -/** Marks a version name as belonging to the dev channel. */ -private const val DEV_SUFFIX = "-dev" - /** * Over-the-air updates from the repository's GitHub Releases. * @@ -42,50 +21,29 @@ private const val DEV_SUFFIX = "-dev" * number, so comparing the two is enough to tell whether a newer build exists. * The repository's releases must be publicly downloadable — no token is * embedded in the app, and GitHub blocks anonymous access to private assets. + * + * There is nothing to choose here. The channel comes from the running APK, so + * a dev install looks only at dev builds and a production install only at + * production ones — see [verdictFor], where that rule lives and is tested. */ class Updater(private val activity: Activity) { - data class Release( - val tag: String, - val versionCode: Int, - val apkUrl: String, - val apkName: String, - val channel: UpdateChannel, - ) - - sealed interface Result { - /** - * [isChannelSwitch] means this build is from a different channel than - * the one installed, so it is an explicit move rather than a newer - * version of the same line — and, because the channels have separate - * applicationIds, it lands as a second app rather than an upgrade. - */ - data class Available(val release: Release, val isChannelSwitch: Boolean) : Result - - data object UpToDate : Result - data class Failed(val reason: String) : Result - } - - suspend fun check(channel: UpdateChannel): Result = withContext(Dispatchers.IO) { + /** + * Looks for a newer build of whatever is installed. + * + * Takes no channel: asking for one would be asking which line of builds + * this phone is on, and the phone already knows. + */ + suspend fun check(): UpdateVerdict = withContext(Dispatchers.IO) { try { - val latest = fetchLatestRelease(channel) - ?: return@withContext Result.Failed( - "No ${channel.label.lowercase()} build has been published yet." - ) - - when { - // Different channel: version numbers are not comparable, so - // always offer it and let the user decide. - latest.channel != installedChannel() -> - Result.Available(latest, isChannelSwitch = true) - - latest.versionCode > installedVersionCode() -> - Result.Available(latest, isChannelSwitch = false) - - else -> Result.UpToDate - } + val channel = installedChannel() + verdictFor( + latest = fetchLatestRelease(channel), + installedChannel = channel, + installedVersionCode = installedVersionCode(), + ) } catch (e: Exception) { - Result.Failed(e.message ?: "Update check failed.") + UpdateVerdict.Refused(e.message ?: "Update check failed.") } } @@ -112,19 +70,8 @@ class Updater(private val activity: Activity) { private fun toRelease(json: JSONObject): Release? { val tag = json.optString("tag_name").ifBlank { return null } - // Tags are v., with -dev on a prerelease. Only the trailing - // run number is read: the series is a name for people and changing it - // must not make an older build look newer. - val versionCode = tag.substringAfterLast('.') - .removeSuffix(DEV_SUFFIX) - .toIntOrNull() - ?: return null - val channel = - if (tag.endsWith(DEV_SUFFIX) || json.optBoolean("prerelease")) { - UpdateChannel.DEV - } else { - UpdateChannel.PRODUCTION - } + val versionCode = versionCodeOfTag(tag) ?: return null + val channel = channelOfTag(tag, json.optBoolean("prerelease")) val assets = json.optJSONArray("assets") ?: return null val apks = (0 until assets.length()) @@ -178,9 +125,7 @@ class Updater(private val activity: Activity) { } /** Read back from the installed APK's own version name. */ - fun installedChannel(): UpdateChannel = - if (installedVersionName().endsWith(DEV_SUFFIX)) UpdateChannel.DEV - else UpdateChannel.PRODUCTION + fun installedChannel(): UpdateChannel = channelOfVersionName(installedVersionName()) /** Downloads the APK and hands it to the system installer. */ suspend fun downloadAndInstall(release: Release): String? = withContext(Dispatchers.IO) { diff --git a/app/src/test/java/org/prolibertate/games/update/UpdatePolicyTest.kt b/app/src/test/java/org/prolibertate/games/update/UpdatePolicyTest.kt new file mode 100644 index 0000000..8113c2e --- /dev/null +++ b/app/src/test/java/org/prolibertate/games/update/UpdatePolicyTest.kt @@ -0,0 +1,140 @@ +package org.prolibertate.games.update + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The wall between the two channels. + * + * A dev install may only ever be offered dev builds and a production install + * production ones. There is no setting for it, so the only way this can go + * wrong now is here. + */ +class UpdatePolicyTest { + + private fun release( + tag: String, + versionCode: Int, + channel: UpdateChannel, + ) = Release( + tag = tag, + versionCode = versionCode, + apkUrl = "https://example.invalid/$tag.apk", + apkName = "ProLibertateGames-$tag.apk", + channel = channel, + ) + + private val devBuild = release("v1.1.69-dev", 69, UpdateChannel.DEV) + private val productionBuild = release("v1.1.40", 40, UpdateChannel.PRODUCTION) + + // -- Reading what a build is -------------------------------------------- + + @Test + fun `a version name says which channel it belongs to`() { + assertEquals(UpdateChannel.DEV, channelOfVersionName("1.1.69-dev")) + assertEquals(UpdateChannel.PRODUCTION, channelOfVersionName("1.1.40")) + } + + @Test + fun `either mark is enough to make a release a dev build`() { + assertEquals(UpdateChannel.DEV, channelOfTag("v1.1.69-dev", prerelease = false)) + assertEquals(UpdateChannel.DEV, channelOfTag("v1.1.69", prerelease = true)) + assertEquals(UpdateChannel.PRODUCTION, channelOfTag("v1.1.40", prerelease = false)) + } + + @Test + fun `only the run number is read from a tag`() { + assertEquals(69, versionCodeOfTag("v1.1.69-dev")) + assertEquals(40, versionCodeOfTag("v1.1.40")) + // A series bump must never read as a downgrade. + assertTrue(versionCodeOfTag("v2.0.41")!! > versionCodeOfTag("v1.1.40")!!) + assertNull(versionCodeOfTag("nightly")) + } + + // -- The wall ----------------------------------------------------------- + + @Test + fun `a dev install is never offered a production build`() { + val verdict = verdictFor( + latest = productionBuild, + installedChannel = UpdateChannel.DEV, + // Deliberately far behind: even a much "newer" number on the other + // channel must not get through, because the numbers are unrelated. + installedVersionCode = 1, + ) + assertTrue("a dev install was offered $productionBuild", verdict is UpdateVerdict.Refused) + } + + @Test + fun `a production install is never offered a dev build`() { + val verdict = verdictFor( + latest = devBuild, + installedChannel = UpdateChannel.PRODUCTION, + installedVersionCode = 1, + ) + assertTrue("a production install was offered $devBuild", verdict is UpdateVerdict.Refused) + } + + // -- Within a channel --------------------------------------------------- + + @Test + fun `a newer build on the same channel is offered`() { + assertEquals( + UpdateVerdict.Install(devBuild), + verdictFor(devBuild, UpdateChannel.DEV, installedVersionCode = 62), + ) + assertEquals( + UpdateVerdict.Install(productionBuild), + verdictFor(productionBuild, UpdateChannel.PRODUCTION, installedVersionCode = 39), + ) + } + + @Test + fun `the same build is not offered to itself`() { + assertEquals( + UpdateVerdict.UpToDate, + verdictFor(devBuild, UpdateChannel.DEV, installedVersionCode = 69), + ) + } + + @Test + fun `an older build is not offered`() { + assertEquals( + UpdateVerdict.UpToDate, + verdictFor(devBuild, UpdateChannel.DEV, installedVersionCode = 70), + ) + } + + @Test + fun `nothing published yet says so rather than failing silently`() { + val verdict = verdictFor(null, UpdateChannel.DEV, installedVersionCode = 69) + assertTrue(verdict is UpdateVerdict.Refused) + assertTrue( + "the reason should name the channel: ${(verdict as UpdateVerdict.Refused).reason}", + verdict.reason.contains("dev"), + ) + } + + /** + * The two sequences genuinely overlap — production is on 40 while dev is on + * 69 — so a build's own number is meaningless on the other channel. This is + * the case the old channel picker could produce and this one cannot. + */ + @Test + fun `overlapping version numbers never cross the channels`() { + listOf(1, 40, 69, 200).forEach { installed -> + assertTrue( + "production build offered to a dev install on $installed", + verdictFor(productionBuild, UpdateChannel.DEV, installed) + is UpdateVerdict.Refused, + ) + assertTrue( + "dev build offered to a production install on $installed", + verdictFor(devBuild, UpdateChannel.PRODUCTION, installed) + is UpdateVerdict.Refused, + ) + } + } +}