Merge pull request 'fix(shard): answer with the instance name when the shard is unnamed' (#119) from fix/ruleset-shard-name into edge

Reviewed-on: #119
Reviewed-by: Colby Whitlock <whitlocktech@gmail.com>

client/src/routes/public/Leaderboards.jsx

@@ -5,6 +5,7 @@ import { Loading, ErrorState } from '../../components/PageState.jsx'
 import { useAsync } from '../../lib/useAsync.js'
 import { useShardFeed } from '../../lib/useShardFeed.js'
 import { api } from '../../api/client.js'
+import { useSite } from '../../contexts/SiteContext.jsx'
 
 // Points / loyalty leaderboards (Protocol 3.0 §7). The shard carries ~25 separate
 // point currencies — Queen's Loyalty, Void Pool, Clean Up Britannia, the nine city
@@ -96,6 +97,7 @@ function Entry({ entry, best }) {
 }
 
 function Board({ board }) {
+  const { siteTitle } = useSite()
   const top = Array.isArray(board.top) ? board.top : []
   // Bars are relative to the board leader, not to maxPoints: most systems have no
   // cap (maxPoints 0), and where there is one the leader is often nowhere near it,
@@ -116,9 +118,28 @@ function Board({ board }) {
       </div>
 
       {top.length === 0 ? (
-        <p className="sans dim" style={{ margin: 0, fontSize: '0.84rem' }}>
+        // 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 medal, no bar, an em dash where
+        // a score goes — because a placeholder that looked like a real standing would
+        // be a fabricated one. The first real entry replaces it.
+        <div>
+          <div style={{ display: 'flex', justifyContent: 'space-between', alignItems: 'baseline', gap: 10, padding: '6px 0' }}>
+            <span
+              className="sans"
+              style={{
+                color: 'var(--muted)', fontSize: '0.86rem',
+                overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap',
+              }}
+            >
+              {siteTitle}
+            </span>
+            <span className="sans dim" style={{ fontSize: '0.82rem', flex: 'none' }}>&mdash;</span>
+          </div>
+          <p className="sans dim" style={{ margin: 0, fontSize: '0.78rem' }}>
             Nobody has earned points here yet.
           </p>
+        </div>
       ) : (
         <div>
           {top.map((entry) => (

server/src/model/settings/settings.model.js

@@ -70,6 +70,24 @@ async function isMobileAppLinksEnabled() {
   }
 }
 
+/**
+ * This instance's name, resolved exactly as `getPublic().brand.name` resolves it —
+ * the admin-editable site title wins over BRAND_NAME. Anything that has to *speak*
+ * the instance's name outside the settings payload must use this rather than
+ * `brand.name`, or an install that set only the site title gets two different names
+ * on two different pages.
+ *
+ * Never throws: a name is always better than an error, so a DB fault falls back to
+ * the env value.
+ */
+async function getInstanceName() {
+  try {
+    return (await settingsDb.get('site_title')) || brand.name
+  } catch {
+    return brand.name
+  }
+}
+
 async function get(key) {
   return settingsDb.get(key)
 }
@@ -154,6 +172,7 @@ module.exports = {
   setMany,
   getAll,
   getPublic,
+  getInstanceName,
   PUBLIC_KEYS,
   REGISTRATION_KEY,
   REGISTRATION_MODES,

server/src/utils/shardIngest.js

@@ -17,6 +17,7 @@ const shardStateModel = require('../model/shardState/shardState.model')
 const shardLinksModel = require('../model/shardLinks/shardLinks.model')
 const shardMarketModel = require('../model/shardMarket/shardMarket.model')
 const uoLinkConfigModel = require('../model/uoLinkConfig/uoLinkConfig.model')
+const settingsModel = require('../model/settings/settings.model')
 const broadcaster = require('./shardBroadcast')
 const pushDispatch = require('./pushDispatch')
 const defaultLog = require('./logger')('shard-ingest')
@@ -63,6 +64,31 @@ function shouldLog(event) {
   return LOGGED_KINDS.has(event.kind)
 }
 
+// ServUO's stock Server.cfg name. An operator who never set one publishes this
+// verbatim, so it carries no more information than a blank — matched
+// case-insensitively and trim-tolerantly, but ONLY as an exact whole value: a
+// shard genuinely called "My Shard Reborn" keeps its name.
+const STOCK_SHARD_NAME = 'my shard'
+
+/**
+ * The name to publish for the shard: its own, or this instance's when it has
+ * effectively not given one.
+ *
+ * Deliberately not a general "blank means brand" rule applied across the wire —
+ * it is scoped to this one field, where the two names denote the same thing.
+ */
+async function resolveShardName(shard, deps) {
+  const given = String(shard ?? '').trim()
+  if (given !== '' && given.toLowerCase() !== STOCK_SHARD_NAME) return given
+  try {
+    return (await deps.settings.getInstanceName()) || given
+  } catch {
+    // A ruleset that publishes the stock name is still better than one that
+    // fails to store because the settings read hiccuped.
+    return given
+  }
+}
+
 // Apply the state-change side effect for a kind (if any). Returns a promise.
 async function applyStateChange(event, deps) {
   const { shardState, uoLinkConfig, log } = deps
@@ -180,6 +206,15 @@ async function applyStateChange(event, deps) {
     // would put a duplicate row in the event log on every reconnect, and
     // server.hello already marks each of those.
     case 'world.ruleset':
+      // A shard whose operator never edited Server.cfg publishes ServUO's stock
+      // "My Shard". That is the shard saying *unnamed*, not a name, so the site
+      // answers with its own — the rules page reading "My Shard" under a header
+      // reading UOMysticmoon is the shard failing to introduce itself.
+      //
+      // Normalized HERE rather than on read because the ruleset is also live: the
+      // same `event` object is handed to the SSE broadcast a few lines below, and
+      // a read-time fix would be undone by the next reconnect's frame.
+      event.shard = await resolveShardName(event.shard, deps)
       await shardState.setRuleset(event)
       return
     // Board state, like guild.update — the newest frame for a system replaces the
@@ -225,6 +260,7 @@ function resolveDeps(deps) {
     shardLinks: deps.shardLinks || shardLinksModel,
     shardMarket: deps.shardMarket || shardMarketModel,
     uoLinkConfig: deps.uoLinkConfig || uoLinkConfigModel,
+    settings: deps.settings || settingsModel,
     broadcast: deps.broadcast || broadcaster.broadcast,
     pushDispatch: deps.pushDispatch || pushDispatch.fromShardEvent,
     log: deps.log || defaultLog,

server/src/utils/uoLinkSocket.js

@@ -142,9 +142,15 @@ async function backfill() {
     // snapshot() (which asserts an array under `key`). The shard also re-emits
     // world.ruleset on its own connect — this covers the other order, where the
     // sidecar was already up and holding the ruleset when WE reconnected.
+    //
+    // Routed through the dispatcher rather than straight to shardState, exactly as
+    // ingestEach does for the array-shaped boards: the two orders must produce the
+    // same stored frame, and calling setRuleset directly here made this a second
+    // write path that silently skipped the shard-name normalization the live frame
+    // gets. One writer, one set of rules.
     const ruleset = await uoLinkClient.getRuleset()
     if (ruleset.ok && ruleset.data && ruleset.data.ruleset) {
-      await shardState.setRuleset(ruleset.data.ruleset)
+      await shardIngest.ingest(ruleset.data.ruleset, { fromBackfill: true })
       log.info('snapshotted shard ruleset from /ruleset', { rev: ruleset.data.ruleset.rev })
     }
 

server/test/shardIngest.ruleset.test.js

@@ -96,3 +96,66 @@ test('a setRuleset failure does not throw or stop the broadcast', async () => {
   assert.equal(r.logged, false)
   assert.equal(deps.calls.broadcast.length, 1)
 })
+
+// ── Shard name fallback ────────────────────────────────────────────────────
+//
+// ServUO ships Server.cfg with `Name=My Shard`. An operator who never edited it
+// publishes that verbatim, which says "unnamed" rather than naming anything — so
+// the site answers with its own instance name instead of printing the stock
+// default under a header carrying the real one.
+//
+// Applied at INGEST, not on read, because world.ruleset is also broadcast live:
+// the same object goes to the SSE fan-out, so a read-time fix would be undone by
+// the next reconnect's frame. These tests assert both halves.
+
+function withSettings(deps, name) {
+  return { ...deps, settings: { getInstanceName: async () => name } }
+}
+
+test('the stock ServUO shard name is replaced with the instance name', async () => {
+  const deps = makeDeps()
+  await shardIngest.ingest({ ...FRAME, shard: 'My Shard' }, withSettings(deps, 'UOMysticmoon'))
+  assert.equal(deps.calls.rulesetSet[0].shard, 'UOMysticmoon')
+})
+
+test('the substituted name reaches the live broadcast, not just the store', async () => {
+  const deps = makeDeps()
+  await shardIngest.ingest({ ...FRAME, shard: 'My Shard' }, withSettings(deps, 'UOMysticmoon'))
+  assert.equal(deps.calls.broadcast.length, 1)
+  assert.equal(deps.calls.broadcast[0].shard, 'UOMysticmoon')
+})
+
+test('a missing or blank shard name gets the same treatment', async () => {
+  for (const shard of [undefined, null, '', '   ']) {
+    const deps = makeDeps()
+    await shardIngest.ingest({ ...FRAME, shard }, withSettings(deps, 'UOMysticmoon'))
+    assert.equal(deps.calls.rulesetSet[0].shard, 'UOMysticmoon')
+  }
+})
+
+// The match is on the whole value, case- and padding-insensitive. A shard that
+// deliberately calls itself "My Shard Reborn" has named itself and keeps it.
+test('a real name that merely contains the stock one is left alone', async () => {
+  const deps = makeDeps()
+  await shardIngest.ingest({ ...FRAME, shard: 'My Shard Reborn' }, withSettings(deps, 'UOMysticmoon'))
+  assert.equal(deps.calls.rulesetSet[0].shard, 'My Shard Reborn')
+
+  const padded = makeDeps()
+  await shardIngest.ingest({ ...FRAME, shard: '  MY SHARD  ' }, withSettings(padded, 'UOMysticmoon'))
+  assert.equal(padded.calls.rulesetSet[0].shard, 'UOMysticmoon')
+})
+
+test('a shard that named itself is never overridden by the brand', async () => {
+  const deps = makeDeps()
+  await shardIngest.ingest(FRAME, withSettings(deps, 'Some Other Brand'))
+  assert.equal(deps.calls.rulesetSet[0].shard, 'UOMysticmoon')
+})
+
+// The settings read is a DB call on a path that must never fail ingest.
+test('a settings read failure leaves the frame storable', async () => {
+  const deps = makeDeps()
+  const boom = { ...deps, settings: { getInstanceName: async () => { throw new Error('db down') } } }
+  await shardIngest.ingest({ ...FRAME, shard: 'My Shard' }, boom)
+  assert.equal(deps.calls.rulesetSet.length, 1)
+  assert.equal(deps.calls.rulesetSet[0].shard, 'My Shard')
+})