feat(assets): a creature's picture is whichever action has one (Phase 6)
Some checks failed
PR Checks / frozen-manifest (pull_request) Successful in 51s
PR Checks / client-build (pull_request) Successful in 8m9s
PR Checks / server-tests (pull_request) Failing after 14m27s

The shard's catalogue can now answer for 73 bodies it used to report absent —
they have no art at action 0 and real art at a later one, and their key says
which (`body/820/a23` is a horse). This side stores that action and stops
assuming `a0` anywhere.

The atlas join is the part that mattered. It read

  a.asset_key = CONCAT('body/', b.body, '/a0')

which would have silently dropped exactly the creatures this phase adds. It now
reads the row's own action, with COALESCE for rows written before the column
existed — a NULL inside CONCAT makes the whole comparison NULL, which would have
taken every portrait off the site on upgrade with the database perfectly correct
and nothing in any log. It still matches at most one row per slug: a deeper key
(`body/820/a23/f4`) does not equal the catalogue key.

Verified against a real MariaDB with the live shard's own 1,095-row manifest: the
ALTER applies to an installed-shape table and is idempotent, the horse joins to
its a23 picture, a pre-phase-6 NULL-action row keeps its portrait, and a stored
frame key does not become a second candidate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4
This commit is contained in:
2026-09-14 01:09:50 -05:00
parent 53b3aca0c1
commit c6c51b190d
7 changed files with 125 additions and 16 deletions

View File

@@ -675,7 +675,7 @@ CREATE TABLE IF NOT EXISTS shard_atlas_pending (
-- because the manifest reports them before the pixels are fetched and a screen -- because the manifest reports them before the pixels are fetched and a screen
-- that lists what WOULD be imported needs them then. -- that lists what WOULD be imported needs them then.
CREATE TABLE IF NOT EXISTS shard_assets ( CREATE TABLE IF NOT EXISTS shard_assets (
asset_key VARCHAR(191) NOT NULL PRIMARY KEY, -- §5's key: `body/34/a0` asset_key VARCHAR(191) NOT NULL PRIMARY KEY, -- §5's key: `body/34/a0`, `body/820/a23`
family VARCHAR(24) NOT NULL DEFAULT 'body', family VARCHAR(24) NOT NULL DEFAULT 'body',
sha256 CHAR(64) NOT NULL, sha256 CHAR(64) NOT NULL,
bytes INT NOT NULL DEFAULT 0, bytes INT NOT NULL DEFAULT 0,
@@ -708,6 +708,21 @@ CREATE TABLE IF NOT EXISTS shard_assets (
-- same test and costs one re-fetch. -- same test and costs one re-fetch.
ALTER TABLE shard_assets ADD COLUMN IF NOT EXISTS catalog VARCHAR(32) NULL; ALTER TABLE shard_assets ADD COLUMN IF NOT EXISTS catalog VARCHAR(32) NULL;
-- Which action a body's thumbnail came from (§11.2, phase 6).
--
-- The catalogue is still one row per body and still a first frame; what changed
-- is that a body with no art at action 0 is catalogued at the first action that
-- has any, and the key says so — `body/820/a23` is a horse whose action 0 is
-- empty. 73 of a stock client's bodies are in that state, and they rendered as
-- text on the bestiary until this phase looked one action further.
--
-- It is stored rather than parsed back out of the key because the atlas join
-- needs it in SQL, and re-deriving it there with SUBSTRING_INDEX would put a
-- second, weaker parser of §5's key scheme in the schema. NULL means a row
-- written before this column existed, which is action 0 by definition — every
-- key the catalogue had then ended in `a0`.
ALTER TABLE shard_assets ADD COLUMN IF NOT EXISTS action TINYINT NULL;
-- Slug → body id, as the shard itself answered it (§8). -- Slug → body id, as the shard itself answered it (§8).
-- --
-- **Deliberately NOT a column on `shard_spawn_creatures`.** That table is -- **Deliberately NOT a column on `shard_spawn_creatures`.** That table is

View File

@@ -33,7 +33,8 @@ async function batched(conn, sql, rows) {
/** Every asset row we hold, as a Map of key → row. */ /** Every asset row we hold, as a Map of key → row. */
async function allAssets() { async function allAssets() {
const rows = await query( const rows = await query(
'SELECT asset_key, family, sha256, bytes, width, height, body, direction, file, catalog FROM shard_assets', 'SELECT asset_key, family, sha256, bytes, width, height, body, action, direction, file, catalog ' +
'FROM shard_assets',
) )
const map = new Map() const map = new Map()
@@ -47,6 +48,7 @@ async function allAssets() {
width: Number(row.width) || 0, width: Number(row.width) || 0,
height: Number(row.height) || 0, height: Number(row.height) || 0,
body: row.body === null ? null : Number(row.body), body: row.body === null ? null : Number(row.body),
action: row.action === null ? null : Number(row.action),
direction: row.direction === null ? null : Number(row.direction), direction: row.direction === null ? null : Number(row.direction),
file: row.file || null, file: row.file || null,
catalog: row.catalog || null, catalog: row.catalog || null,
@@ -81,6 +83,7 @@ async function saveAssets(rows, meta) {
r.width ?? 0, r.width ?? 0,
r.height ?? 0, r.height ?? 0,
r.body ?? null, r.body ?? null,
r.action ?? null,
r.direction ?? null, r.direction ?? null,
r.file ?? null, r.file ?? null,
r.catalog ?? meta?.catalog ?? null, r.catalog ?? meta?.catalog ?? null,
@@ -88,12 +91,13 @@ async function saveAssets(rows, meta) {
await batched( await batched(
conn, conn,
'INSERT INTO shard_assets (asset_key, family, sha256, bytes, width, height, body, direction, file, catalog) ' + 'INSERT INTO shard_assets ' +
'VALUES (?,?,?,?,?,?,?,?,?,?) ' + '(asset_key, family, sha256, bytes, width, height, body, action, direction, file, catalog) ' +
'VALUES (?,?,?,?,?,?,?,?,?,?,?) ' +
'ON DUPLICATE KEY UPDATE family = VALUES(family), sha256 = VALUES(sha256), ' + 'ON DUPLICATE KEY UPDATE family = VALUES(family), sha256 = VALUES(sha256), ' +
'bytes = VALUES(bytes), width = VALUES(width), height = VALUES(height), ' + 'bytes = VALUES(bytes), width = VALUES(width), height = VALUES(height), ' +
'body = VALUES(body), direction = VALUES(direction), file = VALUES(file), ' + 'body = VALUES(body), action = VALUES(action), direction = VALUES(direction), ' +
'catalog = VALUES(catalog), imported_at = CURRENT_TIMESTAMP', 'file = VALUES(file), catalog = VALUES(catalog), imported_at = CURRENT_TIMESTAMP',
values, values,
) )
@@ -260,17 +264,24 @@ async function countBodies() {
* first-class state everywhere it is consumed and the expected one for two thirds * first-class state everywhere it is consumed and the expected one for two thirds
* of the player bodies (§5.2). * of the player bodies (§5.2).
* *
* **The join is pinned to the catalogue key, not merely to the body id.** Today * **The join is pinned to the catalogue key, not merely to the body id** — and as
* one body has exactly one asset, so `a.body = b.body` alone would be correct — * of phase 6 that key is no longer always `a0`. 73 of this client's bodies have
* and it would stop being correct the moment phase 6 adds `body/400/a2/f0`, at * no art at action 0 and are catalogued at the first action that does (§11.2), so
* which point one slug would match dozens of rows and whichever the engine * a join hardcoding `a0` would silently drop exactly the creatures this phase
* returned last would become the portrait. Naming the key here means that phase * added — a horse among them. It reads the row's own `action` instead, which
* adds rows without changing what a creature page shows. * still excludes any deeper key a later phase adds (`body/400/a2/f0` does not
* equal `body/400/a2`), so one slug still matches at most one row.
*
* `COALESCE(a.action, 0)` because a row written before this column existed has
* NULL there and a NULL inside `CONCAT` makes the whole comparison NULL — which
* would have dropped every portrait on the site until the next import, with the
* database perfectly correct.
*/ */
async function artBySlug() { async function artBySlug() {
const rows = await query( const rows = await query(
'SELECT b.slug, a.file FROM shard_creature_bodies b ' + 'SELECT b.slug, a.file FROM shard_creature_bodies b ' +
"JOIN shard_assets a ON a.asset_key = CONCAT('body/', b.body, '/a0') " + "JOIN shard_assets a ON a.body = b.body AND a.family = 'body' " +
"AND a.asset_key = CONCAT('body/', b.body, '/a', COALESCE(a.action, 0)) " +
"WHERE b.status = 'ok' AND b.body IS NOT NULL AND a.file IS NOT NULL", "WHERE b.status = 'ok' AND b.body IS NOT NULL AND a.file IS NOT NULL",
) )

View File

@@ -25,7 +25,13 @@ const log = require('../../core').logger('shardAssets')
// **The catalogue** (§4.8, §11) is one thumbnail per creature body: the shard // **The catalogue** (§4.8, §11) is one thumbnail per creature body: the shard
// walks bodies 02047, validates each index entry, decodes the ones that are real // walks bodies 02047, validates each index entry, decodes the ones that are real
// and hands back `{ key, sha256 }` first and the PNG second. On a stock client // and hands back `{ key, sha256 }` first and the PNG second. On a stock client
// that is **787 sprites**, not the 1,144 the decoder claims — see below. // that is **1,095 sprites** — 787 out of the legacy anim files, 235 more out of
// the UOP packages (phase 4), and 73 more since phase 6, which have no art at
// action 0 and real art at a later one. Never the 1,144 the decoder claims.
//
// A key therefore names its action — `body/820/a23` is a horse whose action 0 is
// empty — and the key is still one per body. Nothing here treats `a0` as the
// shape of a body key; the atlas join reads the row's own action (§11.2).
// //
// **Body resolution** (§8) is the join. The atlas knows a creature by the class // **Body resolution** (§8) is the join. The atlas knows a creature by the class
// name in `Spawns/*.xml`; the client knows it by a body id; nothing in the ServUO // name in `Spawns/*.xml`; the client knows it by a body id; nothing in the ServUO
@@ -296,6 +302,7 @@ async function importAssets({ force = false, approve = false } = {}) {
width: got.width || row.width, width: got.width || row.width,
height: got.height || row.height, height: got.height || row.height,
body: got.body ?? row.body, body: got.body ?? row.body,
action: got.action ?? row.action ?? 0,
direction: got.direction ?? row.direction, direction: got.direction ?? row.direction,
file: name ?? existing?.file ?? null, file: name ?? existing?.file ?? null,
}) })

View File

@@ -13,7 +13,7 @@ const log = require('../../core').logger('shardItemArt')
// ── Why this is not the body catalogue with a different prefix ───────── // ── Why this is not the body catalogue with a different prefix ─────────
// //
// The bestiary wants every creature, so phase 3 imports a SET: walk a manifest, // The bestiary wants every creature, so phase 3 imports a SET: walk a manifest,
// diff the hashes, fetch what moved. That works because the set is 1,022 rows // diff the hashes, fetch what moved. That works because the set is 1,095 rows
// and one megabyte. // and one megabyte.
// //
// This side has no set. The shard's client addresses 49,152 item graphics and // This side has no set. The shard's client addresses 49,152 item graphics and

View File

@@ -250,6 +250,45 @@ test('a fetch passes the catalogue id and decodes the PNG', async (t) => {
assert.equal(assets.get('body/12/a0').width, 24) assert.equal(assets.get('body/12/a0').width, 24)
}) })
test('a body catalogued at a later action keeps that action in its row', async (t) => {
// §11.2, phase 6. 73 of a stock client's bodies have no art at action 0 and are
// catalogued at the first action that does — body 820's is 23, and it is a
// horse. The action travels with the row because the atlas join needs it in
// SQL; re-deriving it from the key would put a second parser of §5's scheme in
// the schema.
stub({
manifest: [
manifestPage([
{ ...row(12), action: 0 },
{ key: 'body/820/a23', sha256: 'bb', bytes: 900, width: 68, height: 69, body: 820, action: 23, direction: 1 },
]),
],
})
t.after(restore)
const { rows } = await bridge.readManifest({})
assert.deepEqual(
rows.map((r) => [r.key, r.action]),
[
['body/12/a0', 0],
['body/820/a23', 23],
],
)
})
test('an overlay older than phase 6 reads as action 0 rather than as unknown', async (t) => {
// A phase-3 through phase-5 overlay omits `action` entirely, and every key it
// ever produced ended in `a0`. Reading that as null would make the atlas join
// COALESCE it back to 0 anyway; reading it as 0 here says so once.
stub({ manifest: [manifestPage([row(12)])] })
t.after(restore)
const { rows } = await bridge.readManifest({})
assert.equal(rows[0].action, 0)
})
test('an absent asset is a counted row, not a failed fetch', async (t) => { test('an absent asset is a counted row, not a failed fetch', async (t) => {
// The whole reason this is not an error: two thirds of the playable ghost and // The whole reason this is not an error: two thirds of the playable ghost and
// gargoyle bodies have no art on a stock client (§5.2), and an import that // gargoyle bodies have no art on a stock client (§5.2), and an import that

View File

@@ -221,6 +221,37 @@ test('only the keys whose hash moved are fetched', async (t) => {
assert.equal(result.written, 1) assert.equal(result.written, 1)
}) })
test('a body catalogued at a later action is imported under that key', async (t) => {
// §11.2, phase 6. Body 820 has no art at action 0 and a horse at action 23, so
// its key is `body/820/a23` — and the filename, the stored row and the atlas
// join all have to agree on that. A name built as `uo-body-820-a0-…` would be
// a file nothing ever asks for, with the creature page still showing text.
const dir = useTempUploads(t)
const seen = stubEverything({
manifest: manifestOf([
{ ...row(820, 'new'), key: 'body/820/a23', action: 23 },
]),
fetched: {
assets: new Map([['body/820/a23', { ...sprite('new'), action: 23 }]]),
missing: { absent: 0, unsupported: 0 },
},
})
t.after(restore)
const result = await model.importAssets({ force: true })
assert.equal(result.status, 'imported')
assert.deepEqual(seen.fetchedKeys, ['body/820/a23'])
const saved = seen.saved[0]
assert.equal(saved.key, 'body/820/a23')
assert.equal(saved.action, 23)
// Content-addressed, and the stem is the key: the action is IN the filename.
assert.equal(saved.file, 'uo-body-820-a23-new.png')
assert.ok(fs.existsSync(path.join(dir, model.ART_SUBDIR, saved.file)))
})
test('an unchanged key whose file is missing from disk is fetched again', async (t) => { test('an unchanged key whose file is missing from disk is fetched again', async (t) => {
// The row and the file can disagree — a wiped uploads volume, a restore from a // The row and the file can disagree — a wiped uploads volume, a restore from a
// database dump. Trusting the row alone leaves a broken image on a creature // database dump. Trusting the row alone leaves a broken image on a creature

View File

@@ -332,6 +332,11 @@ async function readManifest({ family = FAMILY } = {}) {
width: Number(row?.width) || 0, width: Number(row?.width) || 0,
height: Number(row?.height) || 0, height: Number(row?.height) || 0,
body: Number.isFinite(Number(row?.body)) ? Number(row.body) : null, body: Number.isFinite(Number(row?.body)) ? Number(row.body) : null,
// Which action the thumbnail came from (§11.2, phase 6). All but 73 of
// this client's bodies answer 0; the rest have no art there and are
// catalogued deeper, with the key naming the action. An overlay older
// than phase 6 omits it, and 0 is the right reading of that.
action: Number.isFinite(Number(row?.action)) ? Number(row.action) : 0,
direction: Number.isFinite(Number(row?.direction)) ? Number(row.direction) : null, direction: Number.isFinite(Number(row?.direction)) ? Number(row.direction) : null,
}) })
} }
@@ -374,7 +379,7 @@ async function readManifest({ family = FAMILY } = {}) {
/** /**
* The bytes for an explicit list of keys. * The bytes for an explicit list of keys.
* *
* Returns a Map of key → `{ sha256, bytes, width, height, body, direction, png }` * Returns a Map of key → `{ sha256, bytes, width, height, body, action, direction, png }`
* where `png` is a Buffer. A key the shard could not serve is **absent from the * where `png` is a Buffer. A key the shard could not serve is **absent from the
* map** rather than present with a null — the caller then decides what that means * map** rather than present with a null — the caller then decides what that means
* for its own row, and the two ways it happens (`absent`, `unsupported`) are * for its own row, and the two ways it happens (`absent`, `unsupported`) are
@@ -449,6 +454,7 @@ async function fetchAssets({ keys, catalog } = {}) {
width: Number(row.width) || 0, width: Number(row.width) || 0,
height: Number(row.height) || 0, height: Number(row.height) || 0,
body: Number.isFinite(Number(row.body)) ? Number(row.body) : null, body: Number.isFinite(Number(row.body)) ? Number(row.body) : null,
action: Number.isFinite(Number(row.action)) ? Number(row.action) : null,
direction: Number.isFinite(Number(row.direction)) ? Number(row.direction) : null, direction: Number.isFinite(Number(row.direction)) ? Number(row.direction) : null,
// Phase 5's art families carry these; the body catalogue does not, and a // Phase 5's art families carry these; the body catalogue does not, and a
// consumer that wants neither is unaffected by either. // consumer that wants neither is unaffected by either.