From 4f85021be23e9b7ec9c8a2787a5130b7d1aa350c Mon Sep 17 00:00:00 2001 From: wtclaude Date: Sat, 1 Aug 2026 00:59:23 -0500 Subject: [PATCH] fix(shard): decode the atlas `places` objects and render them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AtlasCreatureDto.places` was typed `List` while the server sends `{facet, label, spawners, maxAlive}` objects. The detail route answers 200 with ~49 KB, kotlinx throws on decode, and the screen renders "Something went wrong on the server" — so the whole Atlas creature page was dead, and the error blamed a server that was fine. Nullable-with-defaults protects against a missing field, never a wrong element type. Adds AtlasPlaceDto, plus the `art` field the server also sends, so a decode cannot depend on that staying absent (neither client renders art yet). `places` was never rendered either, so the aggregate the atlas exists to give — "Shrines, Isamu-Jima, Yew", resolved server-side by point-in-rect — was missing from the app while the web page led with it. Adds a "Where it spawns" section above the individual spawners, matching web's ordering, and a plural for the spawner count now that single-spawner places are on screen in bulk. Adds ShardContentDtoTest — the first decode test any of the four Protocol 3.0 DTOs has had, fed payloads captured from a live server. That absence is the root cause: the fakes in data/api/fake/ construct DTOs in Kotlin, so no test in the suite could see a wire mismatch, even though PLAN.md §9 already required "DTO decode for each new shape". Also renders a placeholder row on an unscored leaderboard (the instance name, em dash where a score goes) rather than a blank card — deliberately not shaped like a real entry, since a placeholder that looked like a standing would be a fabricated one. Found by the on-device five-rung walk against a live shard; all four screens re-verified on the emulator afterwards. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U7CBg11prhLimL9iHSX1bP --- .../app/data/api/dto/ShardContentDto.kt | 32 +++- .../java/com/runicgateway/app/ui/RunicApp.kt | 2 +- .../runicgateway/app/ui/shard/AtlasScreen.kt | 41 ++++- .../app/ui/shard/LeaderboardsScreen.kt | 44 ++++- app/src/main/res/values/strings.xml | 13 +- .../app/data/api/dto/ShardContentDtoTest.kt | 153 ++++++++++++++++++ .../app/ui/shard/ShardContentHelpersTest.kt | 12 ++ 7 files changed, 289 insertions(+), 8 deletions(-) create mode 100644 app/src/test/java/com/runicgateway/app/data/api/dto/ShardContentDtoTest.kt diff --git a/app/src/main/java/com/runicgateway/app/data/api/dto/ShardContentDto.kt b/app/src/main/java/com/runicgateway/app/data/api/dto/ShardContentDto.kt index 0999f84..6f35cbc 100644 --- a/app/src/main/java/com/runicgateway/app/data/api/dto/ShardContentDto.kt +++ b/app/src/main/java/com/runicgateway/app/data/api/dto/ShardContentDto.kt @@ -287,14 +287,42 @@ data class AtlasCreatureDto( val points: Int? = null, /** Spawner count per facet. */ val facets: Map = emptyMap(), - /** Region/landmark names where it appears — the detail route only. */ - val places: List = emptyList(), + /** + * Where it appears, aggregated per named place — the detail route only, and the + * answer the whole screen exists to give. **Objects, not strings:** the server + * sends `{facet, label, spawners, maxAlive}`, and typing this `List` + * made the detail route fail to decode entirely. + */ + val places: List = emptyList(), + /** + * Operator-supplied sprite file name under `/uploads/atlas/`, or null — which is + * the normal state, since no artwork ships. Neither client renders it yet; the + * field is carried so a decode never depends on that staying true. + */ + val art: String? = null, val spawners: List = emptyList(), val spawnersTruncated: Boolean = false, /** Creatures sharing its spawners — the detail route only. */ val alsoHere: List = emptyList(), ) +/** + * One named place a creature spawns in, already aggregated across its spawners. + * + * [label] is the server's point-in-rect resolution of raw coordinates ("Shrines", + * "Isamu-Jima", "Yew"), falling back to the nearest landmark and finally + * "Wilderness" — turning a list of coordinates into an answer. + */ +@Serializable +data class AtlasPlaceDto( + val facet: String? = null, + val label: String? = null, + /** Spawners in this place. */ + val spawners: Int? = null, + /** How many can be alive at once here, summed across those spawners. */ + val maxAlive: Int? = null, +) + /** * One spawn point. * diff --git a/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt b/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt index 532a7f6..ddcc9a0 100644 --- a/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt +++ b/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt @@ -319,7 +319,7 @@ private fun RunicNavHost( // here" from its own 404/403, so a deep link to a gated feature still lands on // an honest answer even though the menu hides the entry. composable(Routes.SHARD_RULES) { RulesScreen() } - composable(Routes.SHARD_LEADERBOARDS) { LeaderboardsScreen() } + composable(Routes.SHARD_LEADERBOARDS) { LeaderboardsScreen(brand = brand) } composable(Routes.SHARD_MARKET) { MarketScreen(onOpenVendor = { serial -> navController.navigate(Routes.marketVendor(serial)) }) } diff --git a/app/src/main/java/com/runicgateway/app/ui/shard/AtlasScreen.kt b/app/src/main/java/com/runicgateway/app/ui/shard/AtlasScreen.kt index 8ceb220..0619dd2 100644 --- a/app/src/main/java/com/runicgateway/app/ui/shard/AtlasScreen.kt +++ b/app/src/main/java/com/runicgateway/app/ui/shard/AtlasScreen.kt @@ -24,6 +24,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier +import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.input.ImeAction import androidx.compose.ui.text.style.TextOverflow @@ -32,6 +33,7 @@ import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.runicgateway.app.R import com.runicgateway.app.data.api.dto.AtlasCreatureDto +import com.runicgateway.app.data.api.dto.AtlasPlaceDto import com.runicgateway.app.data.api.dto.AtlasSpawnerDto import com.runicgateway.app.ui.UiState import com.runicgateway.app.ui.components.EmptyView @@ -111,7 +113,7 @@ private fun CreatureCard(creature: AtlasCreatureDto, onOpenCreature: (String) -> // and only the detail route sends it. creature.points?.let { Text( - text = stringResource(R.string.atlas_spawner_count, it), + text = pluralStringResource(R.plurals.atlas_spawner_count, it, it), style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.onSurfaceVariant, ) @@ -178,6 +180,17 @@ fun AtlasCreatureScreen( } } } + // The aggregate comes first: "where is it" is the question, and the + // individual coordinates below are the follow-up. Same ordering as web. + if (creature.places.isNotEmpty()) { + item { SectionLabel(stringResource(R.string.atlas_section_places)) } + items( + creature.places, + key = { "${it.facet.orEmpty()}:${it.label.orEmpty()}" }, + ) { place -> + PlaceRow(place) + } + } if (creature.spawners.isNotEmpty()) { item { SectionLabel(stringResource(R.string.atlas_section_spawners)) } items(creature.spawners, key = { it.id ?: it.hashCode().toLong() }) { spawner -> @@ -208,6 +221,24 @@ fun AtlasCreatureScreen( } } +@Composable +private fun PlaceRow(place: AtlasPlaceDto) { + Column(Modifier.fillMaxWidth().padding(vertical = 4.dp)) { + Text( + text = placeLabel(place), + style = MaterialTheme.typography.bodyMedium, + ) + val meta = listOfNotNull( + place.facet, + place.spawners?.let { pluralStringResource(R.plurals.atlas_spawner_count, it, it) }, + place.maxAlive?.let { stringResource(R.string.atlas_place_max_alive, it) }, + ).joinToString(" · ") + if (meta.isNotBlank()) { + Text(meta, style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.onSurfaceVariant) + } + } +} + @Composable private fun SpawnerRow(spawner: AtlasSpawnerDto) { Column(Modifier.fillMaxWidth().padding(vertical = 4.dp)) { @@ -250,6 +281,14 @@ internal fun spawnerPlace(spawner: AtlasSpawnerDto): String { } } +/** + * The name of an aggregated place. [AtlasPlaceDto.label] is already the server's + * resolved answer and falls back to "Wilderness" there, so the only case left here is + * a place that carried no label at all — then the facet is better than nothing. + */ +internal fun placeLabel(place: AtlasPlaceDto): String = + place.label?.takeIf { it.isNotBlank() } ?: place.facet.orEmpty() + /** * A creature's facets as one line, most spawners first — "where is it *mostly*" is the * question a search result answers. diff --git a/app/src/main/java/com/runicgateway/app/ui/shard/LeaderboardsScreen.kt b/app/src/main/java/com/runicgateway/app/ui/shard/LeaderboardsScreen.kt index bf22614..dc6cb14 100644 --- a/app/src/main/java/com/runicgateway/app/ui/shard/LeaderboardsScreen.kt +++ b/app/src/main/java/com/runicgateway/app/ui/shard/LeaderboardsScreen.kt @@ -22,6 +22,7 @@ import androidx.compose.ui.unit.dp import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.runicgateway.app.R +import com.runicgateway.app.data.api.dto.BrandDto import com.runicgateway.app.data.api.dto.PointsBoardDto import com.runicgateway.app.data.api.dto.PointsEntryDto import com.runicgateway.app.ui.components.SectionLabel @@ -32,6 +33,7 @@ import com.runicgateway.app.ui.components.SectionLabel */ @Composable fun LeaderboardsScreen( + brand: BrandDto? = null, modifier: Modifier = Modifier, viewModel: LeaderboardsViewModel = hiltViewModel(), ) { @@ -45,11 +47,11 @@ fun LeaderboardsScreen( onRetry = viewModel::load, key = { it.system.orEmpty() }, modifier = modifier, - ) { board -> BoardCard(board) } + ) { board -> BoardCard(board, placeholderName(brand)) } } @Composable -private fun BoardCard(board: PointsBoardDto) { +private fun BoardCard(board: PointsBoardDto, placeholderName: String) { Card(Modifier.fillMaxWidth()) { Column(Modifier.padding(16.dp), verticalArrangement = Arrangement.spacedBy(2.dp)) { Row(Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.SpaceBetween) { @@ -75,9 +77,33 @@ private fun BoardCard(board: PointsBoardDto) { } if (board.top.isEmpty()) { + // A board nobody has scored on still gets a row, so the page reads as a + // set of standings waiting to be filled rather than a stack of blanks. + // It is deliberately NOT shaped like an entry — no rank, no score, the + // instance's own name — because a placeholder that looked like a real + // standing would be a fabricated one. The first real entry replaces it. + HorizontalDivider(Modifier.padding(vertical = 8.dp)) + Row( + Modifier.fillMaxWidth().padding(vertical = 3.dp), + horizontalArrangement = Arrangement.SpaceBetween, + ) { + Text( + text = placeholderName, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f), + ) + Text( + text = stringResource(R.string.leaderboards_no_score), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } Text( stringResource(R.string.leaderboards_board_empty), - style = MaterialTheme.typography.bodySmall, + style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.onSurfaceVariant, modifier = Modifier.padding(top = 6.dp), ) @@ -129,3 +155,15 @@ internal fun boardLabel(board: PointsBoardDto): String { .replace(Regex("([a-z0-9])([A-Z])"), "$1 $2") .replaceFirstChar { it.uppercaseChar() } } + +/** + * The name to stand in for an empty board: this instance's, falling back to the app + * name — the same resolution the app bar uses, so a shard that publishes no branding + * still reads as *something* rather than as a blank row. + * + * Pure and separate so the fallback order is testable; [BrandDto.name] can be present + * but blank, which is a shard that set the key and left it empty. + */ +@Composable +internal fun placeholderName(brand: BrandDto?): String = + brand?.name?.takeIf { it.isNotBlank() } ?: stringResource(R.string.app_name) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 17e3773..5317643 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -417,6 +417,9 @@ This shard isn\'t publishing any leaderboards yet. Nobody has scored here yet. + + — %1$d players Cap: %1$d @@ -444,9 +447,17 @@ Search creatures No creatures match that search. - %1$d spawners + + + %1$d spawner + %1$d spawners + Up to %1$d alive at once %1$s (%2$d) + + Where it spawns + up to %1$d at once Spawn points Also spawns here More spawn points than shown. diff --git a/app/src/test/java/com/runicgateway/app/data/api/dto/ShardContentDtoTest.kt b/app/src/test/java/com/runicgateway/app/data/api/dto/ShardContentDtoTest.kt new file mode 100644 index 0000000..ba2e864 --- /dev/null +++ b/app/src/test/java/com/runicgateway/app/data/api/dto/ShardContentDtoTest.kt @@ -0,0 +1,153 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.data.api.dto + +import kotlinx.serialization.json.Json +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Decode tests for the four Protocol 3.0 content DTOs, against payloads captured from a + * **live** server rather than hand-written to match the Kotlin types. + * + * These exist because the fakes in `data/api/fake/` construct DTOs directly, so no test + * in the suite ever fed one real JSON — and `AtlasCreatureDto.places` shipped typed + * `List` while the server sends objects. That decodes to an exception, the + * screen renders "something went wrong on the server", and 336 green tests say nothing. + * Nullable-with-defaults protects against a *missing* field, never a *wrong type*. + */ +class ShardContentDtoTest { + + private val json = Json { + ignoreUnknownKeys = true + explicitNulls = false + coerceInputValues = true + } + + // ── Spawn atlas (§6) ───────────────────────────────────────────────── + + /** Trimmed from `GET /api/v1/public/atlas/creatures/seaserpent` on a real shard. */ + private val seaSerpent = """ + {"slug":"seaserpent","name":"SeaSerpent","total":6048,"points":477, + "facets":{"Felucca":237,"Trammel":240},"art":"seaserpent.png", + "places":[{"facet":"Felucca","label":"Wilderness","spawners":91,"maxAlive":1194}, + {"facet":"Trammel","label":"Britain","spawners":12,"maxAlive":96}], + "spawners":[{"id":1617,"facet":"Felucca","name":"SeaLife#68","x":1691,"y":1623, + "width":350,"height":350,"range":175,"maxCount":15,"minDelay":300, + "maxDelay":600,"todStart":0,"todEnd":0,"todMode":0, + "region":"Britain","landmark":null,"label":"Britain"}], + "spawnersTruncated":true, + "alsoHere":[{"slug":"waterelemental","name":"WaterElemental","shared":242}]} + """.trimIndent() + + @Test fun atlasCreatureDetailDecodesTheRealPayload() { + val creature = json.decodeFromString(seaSerpent) + + assertEquals("seaserpent", creature.slug) + assertEquals(6048, creature.total) + assertEquals(477, creature.points) + assertEquals(240, creature.facets["Trammel"]) + assertTrue(creature.spawnersTruncated) + assertEquals("seaserpent.png", creature.art) + } + + @Test fun atlasPlacesAreObjectsNotStrings() { + // The regression. `places` is the aggregate the screen exists to show, and it + // arrives as {facet,label,spawners,maxAlive} — never as a bare place name. + val places = json.decodeFromString(seaSerpent).places + + assertEquals(2, places.size) + assertEquals("Wilderness", places[0].label) + assertEquals("Felucca", places[0].facet) + assertEquals(91, places[0].spawners) + assertEquals(1194, places[0].maxAlive) + } + + @Test fun atlasCreatureSurvivesAProjectedOrEmptyPayload() { + // The search route sends no `places`/`spawners`/`art`, and the visibility + // framework can drop any field from any of them. + val lean = json.decodeFromString("""{"slug":"orc"}""") + assertEquals("orc", lean.slug) + assertTrue(lean.places.isEmpty()) + assertTrue(lean.spawners.isEmpty()) + assertNull(lean.art) + + val bare = json.decodeFromString("""{"places":[{}]}""") + assertNull(bare.places[0].label) + assertNull(bare.places[0].spawners) + } + + // ── Market (§8) ────────────────────────────────────────────────────── + + @Test fun marketListingDecodesWithItsNestedVendorAndLocation() { + // `location` nests on the wire so one visibility rule covers map/x/y/region/house. + val listing = json.decodeFromString( + """{"serial":"0x40014A57","itemId":3937,"hue":1878,"amount":1,"price":115, + "name":null,"cliloc":1023937,"displayName":"longsword","child":false, + "vendor":{"serial":"0x2CB","shopName":"Seed Shop 225","ownerSerial":"0x201", + "ownerName":"Seed004A", + "location":{"map":"Felucca","x":1562,"y":1604,"z":0, + "region":"Britain","house":"Seed House 4"}}}""", + ) + + assertEquals("longsword", listing.displayName) + assertEquals(115L, listing.price) + assertEquals("Seed Shop 225", listing.vendor?.shopName) + assertEquals("Felucca", listing.vendor?.location?.map) + } + + @Test fun marketListingSurvivesTheFieldsAVisitorMayNotSee() { + // Below the `staff` rung the server omits ownerName/ownerSerial, and below + // `player` the whole nested location. Neither may break the decode. + val projected = json.decodeFromString( + """{"serial":"0x40014A57","price":115,"displayName":"longsword", + "vendor":{"serial":"0x2CB","shopName":"Seed Shop 225"}}""", + ) + + assertEquals("Seed Shop 225", projected.vendor?.shopName) + assertNull(projected.vendor?.ownerName) + assertNull(projected.vendor?.location) + } + + // ── Points boards (§7) ─────────────────────────────────────────────── + + @Test fun pointsBoardDecodesAnEmptyBoardAndItsCap() { + // A shard with nothing scored yet is the common case, not an error, and + // maxPoints 0 is the "uncapped" sentinel rather than a cap of zero. + val board = json.decodeFromString( + """{"kind":"points.board","system":"QueensLoyalty","nameNumber":1095163, + "nameString":null,"players":0,"maxPoints":15000,"showOnGump":true, + "top":[],"t":1785556444154,"updatedAt":"2026-08-01T03:54:04.000Z"}""", + ) + + assertEquals("QueensLoyalty", board.system) + assertEquals(15000L, board.maxPoints) + assertNull(board.nameString) + assertTrue(board.top.isEmpty()) + } + + // ── Ruleset (§5) ───────────────────────────────────────────────────── + + @Test fun rulesetDecodesTheNestedSectionsAndTolerantlySkipsUnknownOnes() { + // The frame is built from an allowlist that grows with the shard's config; a + // key this client has never heard of must not break the rules page. + val ruleset = json.decodeFromString( + """{"kind":"world.ruleset","shard":"My Shard","expansion":"EJ", + "caps":{"skill":1000,"totalSkill":7000,"stat":225,"str":125}, + "systems":{"factions":false,"vvv":true,"siege":false}, + "accounts":{"charSlots":7,"perIp":3}, + "somethingAddedLater":{"nested":true}}""", + ) + + assertEquals("My Shard", ruleset.shard) + assertEquals("EJ", ruleset.expansion) + // Caps arrive in tenths; the DTO's computed property is what the screen shows. + assertEquals(7000, ruleset.caps?.totalSkill) + assertEquals(700.0, ruleset.caps?.totalSkillCap!!, 0.0) + assertEquals(false, ruleset.systems["factions"]) + assertEquals(true, ruleset.systems["vvv"]) + } +} diff --git a/app/src/test/java/com/runicgateway/app/ui/shard/ShardContentHelpersTest.kt b/app/src/test/java/com/runicgateway/app/ui/shard/ShardContentHelpersTest.kt index a05a2d9..0f17772 100644 --- a/app/src/test/java/com/runicgateway/app/ui/shard/ShardContentHelpersTest.kt +++ b/app/src/test/java/com/runicgateway/app/ui/shard/ShardContentHelpersTest.kt @@ -4,6 +4,7 @@ package com.runicgateway.app.ui.shard import com.runicgateway.app.data.api.dto.AtlasCreatureDto +import com.runicgateway.app.data.api.dto.AtlasPlaceDto import com.runicgateway.app.data.api.dto.AtlasSpawnerDto import com.runicgateway.app.data.api.dto.MarketListingDto import com.runicgateway.app.data.api.dto.MarketLocationDto @@ -161,6 +162,17 @@ class ShardContentHelpersTest { ) } + @Test fun placeLabelUsesTheServersResolvedNameAndFallsBackToTheFacet() { + assertEquals( + "Isamu-Jima", + placeLabel(AtlasPlaceDto(facet = "Tokuno", label = "Isamu-Jima", spawners = 4)), + ) + // The server already falls back to "Wilderness", so a label-less place is the + // degenerate case; the facet still says something, an empty row does not. + assertEquals("Tokuno", placeLabel(AtlasPlaceDto(facet = "Tokuno"))) + assertEquals("Tokuno", placeLabel(AtlasPlaceDto(facet = "Tokuno", label = " "))) + } + @Test fun facetSummaryLeadsWithWhereItMostlyIs() { val creature = AtlasCreatureDto( slug = "lizardman",