From 88ff62c82953e8b0583dc0494640d453b5e1d797 Mon Sep 17 00:00:00 2001 From: saopig1 <4x7sw862st@gmail.com> Date: Thu, 7 May 2026 20:40:00 +0800 Subject: [PATCH] fix(profile): apply custom avatar immediately on idle setCustomAvatar / connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on PR #56: 1. setCustomAvatar(buf) now triggers applyIdleAvatar when the bot is idle (currentSong=null OR avatarEnabled=false). Spec said this; original impl only stored the buffer, so a fresh upload from Settings was invisible until next stop event. Track currentSong in BotProfileManager for the idle check. 2. onConnect drops the !avatarEnabled guard — on a fresh connect there's no song playing yet, so the spec matrix wants the custom avatar shown regardless of sync. Previously bots reconnected with a stale TS3 server-side avatar. 3. CustomAvatarRow: defer the initializing=false flip to nextTick so the load-time data-url assignment's queued watcher sees initializing=true and bails. Removes the redundant PUT-on-mount that echoed the just- loaded bytes back to the server. 4. BotInstance avatar load wrapped in try/catch — a corrupt/locked file no longer crashes startup; we log and continue with no custom avatar. Tests rewritten: 10 cases covering the full behavior matrix (idle vs playing × sync on/off × custom set/null × stop/connect). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/bot/instance.ts | 13 ++- src/bot/profile.test.ts | 107 ++++++++++++++++++++----- src/bot/profile.ts | 29 ++++++- web/src/components/CustomAvatarRow.vue | 8 +- 4 files changed, 130 insertions(+), 27 deletions(-) diff --git a/src/bot/instance.ts b/src/bot/instance.ts index 65b27c5..fae7d6a 100755 --- a/src/bot/instance.ts +++ b/src/bot/instance.ts @@ -94,10 +94,15 @@ export class BotInstance extends EventEmitter { options.tsOptions.nickname, ); - const relPath = this.database.getCustomAvatarPath(this.id); - if (relPath) { - const buf = this.avatarStore.read(relPath); - if (buf) this.profileManager.setCustomAvatar(buf); + // Best-effort: a corrupted/locked avatar file must not block bot startup. + try { + const relPath = this.database.getCustomAvatarPath(this.id); + if (relPath) { + const buf = this.avatarStore.read(relPath); + if (buf) this.profileManager.setCustomAvatar(buf); + } + } catch (err) { + this.logger.warn({ err }, "Failed to load custom avatar — skipping"); } this.setupPlayerEvents(); diff --git a/src/bot/profile.test.ts b/src/bot/profile.test.ts index 38f7d12..c0dc95b 100644 --- a/src/bot/profile.test.ts +++ b/src/bot/profile.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { BotProfileManager } from "./profile.js"; import type { TS3Client } from "../ts-protocol/client.js"; +import type { QueuedSong } from "../audio/queue.js"; function makeMockTs(): TS3Client & { uploadCalls: Buffer[]; @@ -32,17 +33,79 @@ const noopLogger: any = { child: () => noopLogger, info: () => {}, debug: () => const cfgOn = { avatarEnabled: true, descriptionEnabled: false, nicknameEnabled: false, awayStatusEnabled: false, channelDescEnabled: false, nowPlayingMsgEnabled: false }; const cfgOff = { ...cfgOn, avatarEnabled: false }; +const fakeSong: QueuedSong = { + id: "1", + name: "X", + artist: "Y", + album: "Z", + platform: "netease", + url: "u", + coverUrl: "c", + duration: 100, +}; + +const flush = () => new Promise((r) => setImmediate(r)); + describe("BotProfileManager custom avatar precedence", () => { let ts: ReturnType; beforeEach(() => { ts = makeMockTs(); }); - it("on stop with custom avatar set + sync on, uploads custom (does not clear)", async () => { + it("setCustomAvatar uploads immediately on a fresh idle bot (sync on)", async () => { const pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot"); - const custom = Buffer.from([1, 2, 3, 4]); - pm.setCustomAvatar(custom); + pm.setCustomAvatar(Buffer.from([1, 2, 3])); + await flush(); + expect(ts.uploadCalls.length).toBe(1); + expect(ts.uploadCalls[0].equals(Buffer.from([1, 2, 3]))).toBe(true); + }); + + it("setCustomAvatar uploads immediately when sync is off (always idle)", async () => { + const pm = new BotProfileManager(ts as any, noopLogger, cfgOff, "Bot"); + pm.setCustomAvatar(Buffer.from([7])); + await flush(); + expect(ts.uploadCalls.length).toBe(1); + }); + + it("setCustomAvatar while playing + sync on does NOT push (cover wins)", async () => { + const pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot"); + // Simulate the bot playing a song. We can't actually run updateAvatar's + // full HTTP fetch path, but onSongChange records currentSong before + // updateAvatar runs, which is enough for this assertion. + void pm.onSongChange(fakeSong); + await flush(); + const uploadsBefore = ts.uploadCalls.length; + pm.setCustomAvatar(Buffer.from([42])); + await flush(); + expect(ts.uploadCalls.length).toBe(uploadsBefore); // no new upload + }); + + it("setCustomAvatar while playing + sync off DOES push (sync-off is idle)", async () => { + const pm = new BotProfileManager(ts as any, noopLogger, cfgOff, "Bot"); + void pm.onSongChange(fakeSong); + await flush(); + const uploadsBefore = ts.uploadCalls.length; + pm.setCustomAvatar(Buffer.from([42])); + await flush(); + expect(ts.uploadCalls.length).toBe(uploadsBefore + 1); + }); + + it("setCustomAvatar(null) while idle clears the TS3 avatar", async () => { + const pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot"); + pm.setCustomAvatar(Buffer.from([1])); + await flush(); + const clearsBefore = ts.clearCalls; + pm.setCustomAvatar(null); + await flush(); + expect(ts.clearCalls).toBe(clearsBefore + 1); + }); + + it("on stop with custom avatar set + sync on, restores custom (does not clear)", async () => { + const pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot"); + pm.setCustomAvatar(Buffer.from([1, 2, 3, 4])); + await flush(); + const clearsBefore = ts.clearCalls; await pm.onSongChange(null); - expect(ts.uploadCalls.at(-1)?.equals(custom)).toBe(true); - expect(ts.clearCalls).toBe(0); + expect(ts.uploadCalls.at(-1)?.equals(Buffer.from([1, 2, 3, 4]))).toBe(true); + expect(ts.clearCalls).toBe(clearsBefore); // no extra clear }); it("on stop with no custom avatar, falls back to clear", async () => { @@ -52,29 +115,33 @@ describe("BotProfileManager custom avatar precedence", () => { expect(ts.uploadCalls.length).toBe(0); }); - it("on connect with sync off + custom avatar set, applies custom immediately", async () => { + it("on connect with custom avatar set + sync ON, applies custom (spec matrix row 1)", async () => { + const pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot"); + pm.setCustomAvatar(Buffer.from([5, 5])); + await flush(); + ts.uploadCalls.length = 0; // reset + pm.onConnect(); + await flush(); + expect(ts.uploadCalls.length).toBe(1); + expect(ts.uploadCalls[0].equals(Buffer.from([5, 5]))).toBe(true); + }); + + it("on connect with custom avatar set + sync OFF, applies custom", async () => { const pm = new BotProfileManager(ts as any, noopLogger, cfgOff, "Bot"); pm.setCustomAvatar(Buffer.from([9, 9])); - await pm.onConnect(); - // onConnect fires the upload but may be async fire-and-forget; flush microtasks: - await new Promise((r) => setImmediate(r)); + await flush(); + ts.uploadCalls.length = 0; + pm.onConnect(); + await flush(); expect(ts.uploadCalls.length).toBe(1); expect(ts.uploadCalls[0].equals(Buffer.from([9, 9]))).toBe(true); }); - it("on connect with sync off + no custom avatar, does not touch avatar", async () => { + it("on connect with no custom avatar, does not touch avatar", async () => { const pm = new BotProfileManager(ts as any, noopLogger, cfgOff, "Bot"); - await pm.onConnect(); - await new Promise((r) => setImmediate(r)); + pm.onConnect(); + await flush(); expect(ts.uploadCalls.length).toBe(0); expect(ts.clearCalls).toBe(0); }); - - it("setCustomAvatar(null) makes subsequent onSongChange(null) clear again", async () => { - const pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot"); - pm.setCustomAvatar(Buffer.from([1])); - pm.setCustomAvatar(null); - await pm.onSongChange(null); - expect(ts.clearCalls).toBe(1); - }); }); diff --git a/src/bot/profile.ts b/src/bot/profile.ts index 039ee5a..332638b 100644 --- a/src/bot/profile.ts +++ b/src/bot/profile.ts @@ -26,6 +26,12 @@ export class BotProfileManager { private config: ProfileConfig; private defaultNickname: string; private customAvatar: Buffer | null = null; + /** + * Tracks the last song handed to onSongChange. null means stopped/idle. + * Used by setCustomAvatar to decide whether the new buffer should be + * pushed immediately (idle) or wait for the next stop event (playing). + */ + private currentSong: QueuedSong | null = null; /** Per-feature permission-denied flags. Reset on reconnect. */ private permDenied = { @@ -59,9 +65,24 @@ export class BotProfileManager { // --- Public API --- - /** Set/clear the persistent idle avatar. Pass null to remove. */ + /** + * Set/clear the persistent idle avatar. Pass null to remove. + * + * If the bot is currently in an idle state (no song playing OR + * avatarEnabled is off), the new buffer is pushed to TS3 right away; + * otherwise the cover-art sync is in charge until the next stop event, + * at which point clearAvatar restores from this.customAvatar. + */ setCustomAvatar(buffer: Buffer | null): void { this.customAvatar = buffer; + const idle = this.currentSong === null || !this.config.avatarEnabled; + if (!idle) return; + const gen = ++this.generation; + if (buffer && buffer.length > 0) { + void this.applyIdleAvatar(gen); + } else { + void this.clearAvatar(gen); + } } /** @@ -77,6 +98,7 @@ export class BotProfileManager { */ async onSongChange(song: QueuedSong | null): Promise { const gen = ++this.generation; + this.currentSong = song; // 1. Avatar first — file transfer uses its own response tracker and // must run before sendCommandNoWait calls whose orphaned responses @@ -97,6 +119,7 @@ export class BotProfileManager { /** Reset permission-denied flags and bump generation on new connection. */ onConnect(): void { this.generation++; + this.currentSong = null; this.permDenied = { avatar: false, description: false, @@ -105,7 +128,9 @@ export class BotProfileManager { channelDesc: false, nowPlayingMsg: false, }; - if (!this.config.avatarEnabled && this.customAvatar) { + // No song is playing on a fresh connect, so the matrix says the + // custom avatar should be visible regardless of avatarEnabled. + if (this.customAvatar) { const gen = this.generation; void this.applyIdleAvatar(gen); } diff --git a/web/src/components/CustomAvatarRow.vue b/web/src/components/CustomAvatarRow.vue index e507220..22fa206 100644 --- a/web/src/components/CustomAvatarRow.vue +++ b/web/src/components/CustomAvatarRow.vue @@ -3,12 +3,15 @@