Merge pull request 'fix(shard): decode the atlas places objects and render them' (#32) from fix/atlas-places-decode into feat/protocol-3-visibility

Reviewed-on: #32

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<String, Int> = emptyMap(),
-    /** Region/landmark names where it appears — the detail route only. */
-    val places: List<String> = 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<String>`
+     * made the detail route fail to decode entirely.
+     */
+    val places: List<AtlasPlaceDto> = 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<AtlasSpawnerDto> = emptyList(),
     val spawnersTruncated: Boolean = false,
     /** Creatures sharing its spawners — the detail route only. */
     val alsoHere: List<AtlasCreatureDto> = 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.
  *

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)) })
         }

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.

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)

app/src/main/res/values/strings.xml

@@ -417,6 +417,9 @@
     <!-- ── Leaderboards (Protocol 3.0 §7, M11) ─────────────────────────── -->
     <string name="leaderboards_empty">This shard isn\'t publishing any leaderboards yet.</string>
     <string name="leaderboards_board_empty">Nobody has scored here yet.</string>
+    <!-- Where a score would sit on the placeholder row of an unscored board. An em
+         dash, not "0" — nobody has scored zero, nobody has scored at all. -->
+    <string name="leaderboards_no_score">—</string>
     <string name="leaderboards_players">%1$d players</string>
     <!-- Only shown for capped systems; most systems on a real shard are uncapped. -->
     <string name="leaderboards_cap">Cap: %1$d</string>
@@ -444,9 +447,17 @@
     <!-- ── Spawn atlas (Protocol 3.0 §6, M11) ──────────────────────────── -->
     <string name="atlas_search_label">Search creatures</string>
     <string name="atlas_empty">No creatures match that search.</string>
-    <string name="atlas_spawner_count">%1$d spawners</string>
+    <!-- A place can legitimately hold a single spawner, and the aggregate list is full
+         of them — "1 spawners" on every other row is worth a plural for. -->
+    <plurals name="atlas_spawner_count">
+        <item quantity="one">%1$d spawner</item>
+        <item quantity="other">%1$d spawners</item>
+    </plurals>
     <string name="atlas_total_alive">Up to %1$d alive at once</string>
     <string name="atlas_facet_count">%1$s (%2$d)</string>
+    <!-- The aggregate: "where is it", as opposed to the raw coordinates below it. -->
+    <string name="atlas_section_places">Where it spawns</string>
+    <string name="atlas_place_max_alive">up to %1$d at once</string>
     <string name="atlas_section_spawners">Spawn points</string>
     <string name="atlas_section_also_here">Also spawns here</string>
     <string name="atlas_spawners_truncated">More spawn points than shown.</string>

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<String>` 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<AtlasCreatureDto>(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<AtlasCreatureDto>(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<AtlasCreatureDto>("""{"slug":"orc"}""")
+        assertEquals("orc", lean.slug)
+        assertTrue(lean.places.isEmpty())
+        assertTrue(lean.spawners.isEmpty())
+        assertNull(lean.art)
+
+        val bare = json.decodeFromString<AtlasCreatureDto>("""{"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<MarketListingDto>(
+            """{"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<MarketListingDto>(
+            """{"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<PointsBoardDto>(
+            """{"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<RulesetDto>(
+            """{"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"])
+    }
+}

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",