diff --git a/server/db/schema.sql b/server/db/schema.sql index 62f64db..ab719f9 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -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 -- that lists what WOULD be imported needs them then. 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', sha256 CHAR(64) NOT NULL, bytes INT NOT NULL DEFAULT 0, @@ -708,6 +708,21 @@ CREATE TABLE IF NOT EXISTS shard_assets ( -- same test and costs one re-fetch. 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). -- -- **Deliberately NOT a column on `shard_spawn_creatures`.** That table is diff --git a/server/model/shardAssets/shardAssets.db.js b/server/model/shardAssets/shardAssets.db.js index 9a611b6..4fbbbe5 100644 --- a/server/model/shardAssets/shardAssets.db.js +++ b/server/model/shardAssets/shardAssets.db.js @@ -33,7 +33,8 @@ async function batched(conn, sql, rows) { /** Every asset row we hold, as a Map of key → row. */ async function allAssets() { 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() @@ -47,6 +48,7 @@ async function allAssets() { width: Number(row.width) || 0, height: Number(row.height) || 0, body: row.body === null ? null : Number(row.body), + action: row.action === null ? null : Number(row.action), direction: row.direction === null ? null : Number(row.direction), file: row.file || null, catalog: row.catalog || null, @@ -81,6 +83,7 @@ async function saveAssets(rows, meta) { r.width ?? 0, r.height ?? 0, r.body ?? null, + r.action ?? null, r.direction ?? null, r.file ?? null, r.catalog ?? meta?.catalog ?? null, @@ -88,12 +91,13 @@ async function saveAssets(rows, meta) { await batched( conn, - 'INSERT INTO shard_assets (asset_key, family, sha256, bytes, width, height, body, direction, file, catalog) ' + - 'VALUES (?,?,?,?,?,?,?,?,?,?) ' + + 'INSERT INTO shard_assets ' + + '(asset_key, family, sha256, bytes, width, height, body, action, direction, file, catalog) ' + + 'VALUES (?,?,?,?,?,?,?,?,?,?,?) ' + 'ON DUPLICATE KEY UPDATE family = VALUES(family), sha256 = VALUES(sha256), ' + 'bytes = VALUES(bytes), width = VALUES(width), height = VALUES(height), ' + - 'body = VALUES(body), direction = VALUES(direction), file = VALUES(file), ' + - 'catalog = VALUES(catalog), imported_at = CURRENT_TIMESTAMP', + 'body = VALUES(body), action = VALUES(action), direction = VALUES(direction), ' + + 'file = VALUES(file), catalog = VALUES(catalog), imported_at = CURRENT_TIMESTAMP', values, ) @@ -260,17 +264,24 @@ async function countBodies() { * first-class state everywhere it is consumed and the expected one for two thirds * of the player bodies (§5.2). * - * **The join is pinned to the catalogue key, not merely to the body id.** Today - * one body has exactly one asset, so `a.body = b.body` alone would be correct — - * and it would stop being correct the moment phase 6 adds `body/400/a2/f0`, at - * which point one slug would match dozens of rows and whichever the engine - * returned last would become the portrait. Naming the key here means that phase - * adds rows without changing what a creature page shows. + * **The join is pinned to the catalogue key, not merely to the body id** — and as + * of phase 6 that key is no longer always `a0`. 73 of this client's bodies have + * no art at action 0 and are catalogued at the first action that does (§11.2), so + * a join hardcoding `a0` would silently drop exactly the creatures this phase + * added — a horse among them. It reads the row's own `action` instead, which + * 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() { const rows = await query( '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", ) diff --git a/server/model/shardAssets/shardAssets.model.js b/server/model/shardAssets/shardAssets.model.js index 3d8301c..859bd80 100644 --- a/server/model/shardAssets/shardAssets.model.js +++ b/server/model/shardAssets/shardAssets.model.js @@ -25,7 +25,13 @@ const log = require('../../core').logger('shardAssets') // **The catalogue** (§4.8, §11) is one thumbnail per creature body: the shard // walks bodies 0–2047, validates each index entry, decodes the ones that are real // 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 // 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, height: got.height || row.height, body: got.body ?? row.body, + action: got.action ?? row.action ?? 0, direction: got.direction ?? row.direction, file: name ?? existing?.file ?? null, }) diff --git a/server/model/shardAssets/shardItemArt.model.js b/server/model/shardAssets/shardItemArt.model.js index 3a87775..5c52281 100644 --- a/server/model/shardAssets/shardItemArt.model.js +++ b/server/model/shardAssets/shardItemArt.model.js @@ -13,7 +13,7 @@ const log = require('../../core').logger('shardItemArt') // ── 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, -// 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. // // This side has no set. The shard's client addresses 49,152 item graphics and diff --git a/server/test/assetBridge.test.js b/server/test/assetBridge.test.js index 84cc8db..7b47a39 100644 --- a/server/test/assetBridge.test.js +++ b/server/test/assetBridge.test.js @@ -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) }) +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) => { // 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 diff --git a/server/test/shardAssets.model.test.js b/server/test/shardAssets.model.test.js index 6f3fd99..bdbbf2d 100644 --- a/server/test/shardAssets.model.test.js +++ b/server/test/shardAssets.model.test.js @@ -221,6 +221,37 @@ test('only the keys whose hash moved are fetched', async (t) => { 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) => { // 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 diff --git a/server/utils/assetBridge.js b/server/utils/assetBridge.js index d295154..69e140d 100644 --- a/server/utils/assetBridge.js +++ b/server/utils/assetBridge.js @@ -332,6 +332,11 @@ async function readManifest({ family = FAMILY } = {}) { width: Number(row?.width) || 0, height: Number(row?.height) || 0, 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, }) } @@ -374,7 +379,7 @@ async function readManifest({ family = FAMILY } = {}) { /** * 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 * 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 @@ -449,6 +454,7 @@ async function fetchAssets({ keys, catalog } = {}) { width: Number(row.width) || 0, height: Number(row.height) || 0, 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, // Phase 5's art families carry these; the body catalogue does not, and a // consumer that wants neither is unaffected by either.