From 54680cc8912a2ae578ee891ecfd3521be857ead1 Mon Sep 17 00:00:00 2001 From: sganggs Date: Sun, 4 Oct 2026 20:47:49 +0800 Subject: [PATCH] feedback3 WJ: emote manifest timeout falls back to the glyph --- public/js/data.js | 100 ++++++++++++++++++++++++++++------ public/js/ui/emotes.js | 7 +-- test/ui/emotes.test.js | 119 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 206 insertions(+), 20 deletions(-) diff --git a/public/js/data.js b/public/js/data.js index 03b6fc45..f599c9da 100644 --- a/public/js/data.js +++ b/public/js/data.js @@ -6,7 +6,10 @@ // GAME_FILES) in the background once the player is in a room, and the match screen waits for them, so no text of the // match UI ever appears late. A transient failure (network error, HTTP 5xx) is retried twice (RETRY_DELAYS_MS); files // that stay unavailable (404, repeated failures, bad JSON) resolve to `null` and are reported once on the console — the -// UI must degrade gracefully while data is being generated. +// UI must degrade gracefully while data is being generated. The emote manifests (`local`, `assets`) are the exception: +// one attempt is abandoned after ART_MANIFEST_TIMEOUT_MS (GitHub #99) and reported `missing` at once, so the 交流 button +// draws its glyph instead of staying blank; a later retry that succeeds replaces the glyph. Other files stay `loading` +// across their retries. // // Each file is indexed tolerantly so the getters work whether a file is // - an array of records carrying an id field (id / chessId / bondId / itemId / …), @@ -78,6 +81,16 @@ export function buildIndex(name, json) { /** Waits (ms) before retrying a data file whose download failed transiently (network error, HTTP 5xx / 408 / 429). */ export const RETRY_DELAYS_MS = Object.freeze([600, 2000]); +/** + * How long one attempt at an emote manifest (`local`, `assets`) may hang before it counts as a failure. + * [ASSUMED] 8 s: long enough for a slow link, short enough that the 交流 button does not stay blank for the session + * (GitHub #99). Other files are not on this clock. + */ +export const ART_MANIFEST_TIMEOUT_MS = 8000; + +/** Manifests the emote button waits on. A hang here used to leave every cell blank (GitHub #99). */ +const ART_MANIFESTS = new Set(['local', 'assets']); + /** * A failed load worth retrying: the request itself failed (network error) or the server answered 5xx / 408 / 429 — not a * definite 4xx (the file is not there) nor a delivered file that is not valid JSON (`badJson`). @@ -92,14 +105,19 @@ const transientFailure = (err) => { * Create a data store bound to a fetch implementation (injectable for tests). * A file is downloaded once per page (the texts of the game are static data, never fetched again during a match — * user playtest #3 item 9); a transient failure is retried (RETRY_DELAYS_MS) while the file stays 'loading', so a - * network hiccup does not leave the texts of a whole session missing. - * @param {{ fetch?: typeof fetch, base?: string, retryDelays?: number[], wait?: (ms: number) => Promise }} [opts] + * network hiccup does not leave the texts of a whole session missing. `local` and `assets` are reported `missing` on + * the first failure or timeout (the emote glyph) and stay that way through a retry; a later success is `ready`. + * @param {{ fetch?: typeof fetch, base?: string, retryDelays?: number[], wait?: (ms: number) => Promise, timeoutMs?: number, setTimeout?: typeof setTimeout, clearTimeout?: typeof clearTimeout }} [opts] + * `timeoutMs` 0 turns the art-manifest clock off. `setTimeout` / `clearTimeout` let a test fire that clock. */ export function createDataStore(opts = {}) { const base = opts.base ?? '/data/'; const doFetch = opts.fetch || ((...a) => globalThis.fetch(...a)); const retryDelays = Array.isArray(opts.retryDelays) ? opts.retryDelays : RETRY_DELAYS_MS; const wait = opts.wait || ((ms) => new Promise((resolve) => setTimeout(resolve, ms))); + const timeoutMs = opts.timeoutMs === undefined ? ART_MANIFEST_TIMEOUT_MS : Number(opts.timeoutMs); + const armTimer = opts.setTimeout || ((fn, ms) => setTimeout(fn, ms)); + const disarmTimer = opts.clearTimeout || ((id) => clearTimeout(id)); /** @type {Map, value: any, index: Map|null }>} */ const entries = new Map(); const listeners = new Set(); @@ -113,40 +131,88 @@ export function createDataStore(opts = {}) { const urlFor = (name) => base + (DATA_FILES[name] || `${name}.json`); + /** + * One attempt at a file. Art manifests reject with a transient `{ timeout: true }` when the clock fires; a response + * that arrives after that is ignored (the retry is a new request). [ASSUMED] + */ + function readJson(name) { + const run = async () => { + const res = await doFetch(urlFor(name), { cache: 'no-cache' }); + if (!res || !res.ok) throw Object.assign(new Error(`HTTP ${res ? res.status : '???'}`), { status: res ? res.status : null }); + try { + return await res.json(); + } catch (err) { + throw Object.assign(err instanceof Error ? err : new Error(String(err)), { badJson: true }); + } + }; + if (!ART_MANIFESTS.has(name) || !(timeoutMs > 0)) return run(); + let timer = null; + let done = false; + const finish = () => { + if (done) return false; + done = true; + if (timer != null) disarmTimer(timer); + return true; + }; + const timed = new Promise((_, reject) => { + timer = armTimer(() => { + if (!finish()) return; + reject(Object.assign(new Error('timeout'), { timeout: true })); + }, timeoutMs); + }); + const req = run().then( + (json) => { finish(); return json; }, + (err) => { finish(); throw err; }, + ); + return Promise.race([req, timed]); + } + function load(name) { if (typeof name !== 'string' || !/^[A-Za-z0-9_-]+$/.test(name)) return Promise.resolve(null); const cur = entries.get(name); if (cur) return cur.promise; const entry = { status: 'loading', promise: null, value: null, index: null }; + const art = ART_MANIFESTS.has(name); entry.promise = (async () => { + let toldMissing = false; for (let attempt = 0; ; attempt++) { try { - const res = await doFetch(urlFor(name), { cache: 'no-cache' }); - if (!res || !res.ok) throw Object.assign(new Error(`HTTP ${res ? res.status : '???'}`), { status: res ? res.status : null }); - let json; - try { json = await res.json(); } catch (err) { throw Object.assign(err instanceof Error ? err : new Error(String(err)), { badJson: true }); } - entry.value = json; + entry.value = await readJson(name); entry.status = 'ready'; break; } catch (err) { - // a transient failure is tried again (still 'loading'), unless the load was superseded meanwhile - if (transientFailure(err) && attempt < retryDelays.length && entries.get(name) === entry) { + const current = entries.get(name) === entry; + // Decided before notify. Invalidating inside that notify leaves `again` true; the wait then sees + // the superseded entry and stops, so this attempt does not fetch again. + const again = transientFailure(err) && attempt < retryDelays.length && current; + // Emote art: glyph now. Stay `missing` through the retry wait — flipping back to `loading` blanks the button. + if (art && current && entry.status !== 'missing') { + entry.value = null; + entry.index = null; + entry.status = 'missing'; + toldMissing = true; + notify(name); + } + if (again) { await wait(retryDelays[attempt]); if (entries.get(name) === entry) continue; - entry.status = 'missing'; // superseded by invalidate() meanwhile: the new load reports for itself break; } - if (!warned.has(name)) { - warned.add(name); - console.warn(`[data] ${urlFor(name)} unavailable (${err?.message || err}); continuing without it`); + if (current) { + if (!warned.has(name)) { + warned.add(name); + console.warn(`[data] ${urlFor(name)} unavailable (${err?.message || err}); continuing without it`); + } + entry.value = null; + entry.index = null; + entry.status = 'missing'; } - entry.value = null; - entry.status = 'missing'; break; } } // A load superseded by invalidate() must not announce itself (its entry is no longer cached). - if (entries.get(name) === entry) notify(name); + // An art manifest already announced `missing` does not announce that same status again. + if (entries.get(name) === entry && !(toldMissing && entry.status === 'missing')) notify(name); return entry.value; })(); entries.set(name, entry); diff --git a/public/js/ui/emotes.js b/public/js/ui/emotes.js index ec3a78a6..05dd4032 100644 --- a/public/js/ui/emotes.js +++ b/public/js/ui/emotes.js @@ -4,8 +4,9 @@ // // EmoteArt the emote picture: the local-client art (data/local-assets.json → emoticon//) first, else // the copy setup downloads from the public mirror (data/assets.json → ui['emoticon//']; -// GitHub issue #42), each tried in turn when one fails to load; a neutral glyph when neither is there, -// an empty box while a manifest is still loading. +// GitHub issue #42), each tried in turn when one fails to load; a neutral glyph when neither is there. +// An empty box only while a manifest is still in flight; a timeout or a failed manifest (GitHub #99) +// uses that glyph, and a later success replaces it. // EmoteBubble the pop bubble beside the sender's avatar in the team panel (official emoji_bubble_bkg: a dark rounded // square with a tail pointing left + the icon only); pop-in, 3 s, fade. The parent keys it by the emote's // seq so a newer emote replaces the old one and pops again, and passes the arrival time (`at`) so a @@ -68,7 +69,7 @@ export function emoteArtUrl(id) { return emoteArtUrls(id)[0] || null; } -/** True while the local-art manifest or the asset manifest has not settled (EmoteArt shows an empty box meanwhile). */ +/** True while local or assets is still in flight (idle or loading). A timeout or a failure is `missing`: the glyph. */ const artManifestsPending = () => ['local', 'assets'].some((n) => { const st = data.status(n); return st === 'loading' || st === 'idle'; }); /** Official emote UI sprite (ui/battle: emoji_bubble_bkg, emoji_bkg, emoji_cell_bkg, emoji_btn, emoji_btn_disable). */ diff --git a/test/ui/emotes.test.js b/test/ui/emotes.test.js index e52157cd..0d88c65c 100644 --- a/test/ui/emotes.test.js +++ b/test/ui/emotes.test.js @@ -351,6 +351,125 @@ describe('public/js/ui/emotes.js helpers', () => { assert.match(readFileSync(path.join(ROOT, 'public/index.html'), 'utf8'), /href="\/css\/emotes\.css"/); }); + test('a hung emote manifest is missing when the clock fires (glyph), and a later retry replaces it', async () => { + const { createDataStore } = await import('../../public/js/data.js'); + const timers = []; + let calls = 0; + let releaseWait; + const store = createDataStore({ + timeoutMs: 1000, + retryDelays: [0], + wait: () => new Promise((r) => { releaseWait = r; }), + setTimeout: (fn, ms) => { timers.push({ fn, ms }); return timers.length; }, + clearTimeout: () => {}, + fetch: () => { + calls++; + if (calls === 1) return new Promise(() => {}); + return Promise.resolve({ ok: true, status: 200, json: async () => ({ ok: true, groups: { a: { pic: { path: '/p.png' } } } }) }); + }, + }); + const states = []; + store.subscribe(() => states.push(store.status('local'))); + const p = store.load('local'); + assert.equal(store.status('local'), 'loading'); + assert.equal(timers.length, 1); + assert.equal(timers[0].ms, 1000); + timers[0].fn(); + await new Promise((r) => setImmediate(r)); + assert.equal(store.status('local'), 'missing', 'glyph as soon as the attempt times out, before the retry'); + assert.equal(calls, 1, 'still on the first attempt while the retry waits'); + releaseWait(); + const value = await p; + assert.equal(calls, 2); + assert.equal(store.status('local'), 'ready'); + assert.equal(value.ok, true); + assert.equal(store.get('local').groups.a.pic.path, '/p.png'); + assert.deepEqual(states, ['missing', 'ready']); + for (const t of timers) t.fn(); + assert.equal(store.status('local'), 'ready', 'a timer that fires after success does not demote the file'); + }); + + test('a failed emote manifest (no clock) is missing before the retry, then ready; a 404 stays missing', async () => { + const { createDataStore } = await import('../../public/js/data.js'); + let calls = 0; + const store = createDataStore({ + timeoutMs: 1000, + retryDelays: [0], + wait: async () => {}, + setTimeout: () => 1, + clearTimeout: () => {}, + fetch: async () => { + calls++; + if (calls === 1) throw new TypeError('Failed to fetch'); + return { ok: true, status: 200, json: async () => ({ groups: { emoticon: {} } }) }; + }, + }); + const states = []; + store.subscribe(() => states.push(store.status('assets'))); + const value = await store.load('assets'); + assert.equal(value.groups.emoticon !== undefined, true); + assert.deepEqual(states, ['missing', 'ready']); + const warn = console.warn; + console.warn = () => {}; + try { + const timers = []; + const gone = createDataStore({ + timeoutMs: 5, + retryDelays: [], + setTimeout: (fn) => { timers.push(fn); return timers.length; }, + clearTimeout: () => {}, + fetch: () => new Promise(() => {}), + }); + const p = gone.load('local'); + assert.equal(timers.length, 1); + timers[0](); + assert.equal(await p, null); + assert.equal(gone.status('local'), 'missing'); + let n = 0; + const missing = createDataStore({ + timeoutMs: 5, + retryDelays: [0], + wait: async () => {}, + setTimeout: () => 1, + clearTimeout: () => {}, + fetch: async () => { n++; return { ok: false, status: 404, json: async () => ({}) }; }, + }); + assert.equal(await missing.load('assets'), null); + assert.equal(missing.status('assets'), 'missing'); + assert.equal(n, 1, 'a 404 is not retried'); + } finally { console.warn = warn; } + }); + + test('the art-manifest clock is not armed for other files; the default wait is ART_MANIFEST_TIMEOUT_MS', async () => { + const { createDataStore, ART_MANIFEST_TIMEOUT_MS } = await import('../../public/js/data.js'); + assert.equal(ART_MANIFEST_TIMEOUT_MS, 8000); + const arms = []; + let release; + const store = createDataStore({ + setTimeout: (fn, ms) => { arms.push(ms); return arms.length; }, + clearTimeout: () => {}, + fetch: (url) => new Promise((resolve) => { release = () => resolve(String(url).includes('chess') + ? { ok: false, status: 404, json: async () => ({}) } + : { ok: true, status: 200, json: async () => ({ ok: 1 }) }); }), + }); + const local = store.load('local'); + assert.deepEqual(arms, [8000]); + release(); + assert.equal((await local).ok, 1); + arms.length = 0; + const warn = console.warn; + console.warn = () => {}; + try { + const chess = store.load('chess'); + await new Promise((r) => setImmediate(r)); + assert.deepEqual(arms, [], 'chess is not on the emote clock'); + assert.equal(store.status('chess'), 'loading'); + release(); + await chess; + assert.equal(store.status('chess'), 'missing'); + } finally { console.warn = warn; } + }); + test('emote UI never renders an emote label as text (aria-label only)', () => { const src = readFileSync(path.join(ROOT, 'public/js/ui/emotes.js'), 'utf8'); assert.doesNotMatch(src, /EMOTE_TEXT|EMOTE_LABEL/, 'no text table in the UI');