mirror of
https://github.com/ZHANGTIANYAO1/teamspeak-music-bot.git
synced 2026-10-01 20:42:50 +08:00
fix(profile): apply custom avatar immediately on idle setCustomAvatar / connect
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
0f9c7b3c4c
commit
88ff62c829
4 files changed
+130
-27
No files matched your search
+9
-4
@@ -94,10 +94,15 @@ export class BotInstance extends EventEmitter {
|
|||||||
options.tsOptions.nickname,
|
options.tsOptions.nickname,
|
||||||
);
|
);
|
||||||
|
|
||||||
const relPath = this.database.getCustomAvatarPath(this.id);
|
// Best-effort: a corrupted/locked avatar file must not block bot startup.
|
||||||
if (relPath) {
|
try {
|
||||||
const buf = this.avatarStore.read(relPath);
|
const relPath = this.database.getCustomAvatarPath(this.id);
|
||||||
if (buf) this.profileManager.setCustomAvatar(buf);
|
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();
|
this.setupPlayerEvents();
|
||||||
|
|||||||
+87
-20
@@ -1,6 +1,7 @@
|
|||||||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||||
import { BotProfileManager } from "./profile.js";
|
import { BotProfileManager } from "./profile.js";
|
||||||
import type { TS3Client } from "../ts-protocol/client.js";
|
import type { TS3Client } from "../ts-protocol/client.js";
|
||||||
|
import type { QueuedSong } from "../audio/queue.js";
|
||||||
|
|
||||||
function makeMockTs(): TS3Client & {
|
function makeMockTs(): TS3Client & {
|
||||||
uploadCalls: Buffer[];
|
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 cfgOn = { avatarEnabled: true, descriptionEnabled: false, nicknameEnabled: false, awayStatusEnabled: false, channelDescEnabled: false, nowPlayingMsgEnabled: false };
|
||||||
const cfgOff = { ...cfgOn, avatarEnabled: 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", () => {
|
describe("BotProfileManager custom avatar precedence", () => {
|
||||||
let ts: ReturnType<typeof makeMockTs>;
|
let ts: ReturnType<typeof makeMockTs>;
|
||||||
beforeEach(() => { ts = makeMockTs(); });
|
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 pm = new BotProfileManager(ts as any, noopLogger, cfgOn, "Bot");
|
||||||
const custom = Buffer.from([1, 2, 3, 4]);
|
pm.setCustomAvatar(Buffer.from([1, 2, 3]));
|
||||||
pm.setCustomAvatar(custom);
|
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);
|
await pm.onSongChange(null);
|
||||||
expect(ts.uploadCalls.at(-1)?.equals(custom)).toBe(true);
|
expect(ts.uploadCalls.at(-1)?.equals(Buffer.from([1, 2, 3, 4]))).toBe(true);
|
||||||
expect(ts.clearCalls).toBe(0);
|
expect(ts.clearCalls).toBe(clearsBefore); // no extra clear
|
||||||
});
|
});
|
||||||
|
|
||||||
it("on stop with no custom avatar, falls back to clear", async () => {
|
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);
|
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");
|
const pm = new BotProfileManager(ts as any, noopLogger, cfgOff, "Bot");
|
||||||
pm.setCustomAvatar(Buffer.from([9, 9]));
|
pm.setCustomAvatar(Buffer.from([9, 9]));
|
||||||
await pm.onConnect();
|
await flush();
|
||||||
// onConnect fires the upload but may be async fire-and-forget; flush microtasks:
|
ts.uploadCalls.length = 0;
|
||||||
await new Promise((r) => setImmediate(r));
|
pm.onConnect();
|
||||||
|
await flush();
|
||||||
expect(ts.uploadCalls.length).toBe(1);
|
expect(ts.uploadCalls.length).toBe(1);
|
||||||
expect(ts.uploadCalls[0].equals(Buffer.from([9, 9]))).toBe(true);
|
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");
|
const pm = new BotProfileManager(ts as any, noopLogger, cfgOff, "Bot");
|
||||||
await pm.onConnect();
|
pm.onConnect();
|
||||||
await new Promise((r) => setImmediate(r));
|
await flush();
|
||||||
expect(ts.uploadCalls.length).toBe(0);
|
expect(ts.uploadCalls.length).toBe(0);
|
||||||
expect(ts.clearCalls).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);
|
|
||||||
});
|
|
||||||
});
|
});
|
||||||
+27
-2
@@ -26,6 +26,12 @@ export class BotProfileManager {
|
|||||||
private config: ProfileConfig;
|
private config: ProfileConfig;
|
||||||
private defaultNickname: string;
|
private defaultNickname: string;
|
||||||
private customAvatar: Buffer | null = null;
|
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. */
|
/** Per-feature permission-denied flags. Reset on reconnect. */
|
||||||
private permDenied = {
|
private permDenied = {
|
||||||
@@ -59,9 +65,24 @@ export class BotProfileManager {
|
|||||||
|
|
||||||
// --- Public API ---
|
// --- 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 {
|
setCustomAvatar(buffer: Buffer | null): void {
|
||||||
this.customAvatar = buffer;
|
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<void> {
|
async onSongChange(song: QueuedSong | null): Promise<void> {
|
||||||
const gen = ++this.generation;
|
const gen = ++this.generation;
|
||||||
|
this.currentSong = song;
|
||||||
|
|
||||||
// 1. Avatar first — file transfer uses its own response tracker and
|
// 1. Avatar first — file transfer uses its own response tracker and
|
||||||
// must run before sendCommandNoWait calls whose orphaned responses
|
// 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. */
|
/** Reset permission-denied flags and bump generation on new connection. */
|
||||||
onConnect(): void {
|
onConnect(): void {
|
||||||
this.generation++;
|
this.generation++;
|
||||||
|
this.currentSong = null;
|
||||||
this.permDenied = {
|
this.permDenied = {
|
||||||
avatar: false,
|
avatar: false,
|
||||||
description: false,
|
description: false,
|
||||||
@@ -105,7 +128,9 @@ export class BotProfileManager {
|
|||||||
channelDesc: false,
|
channelDesc: false,
|
||||||
nowPlayingMsg: 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;
|
const gen = this.generation;
|
||||||
void this.applyIdleAvatar(gen);
|
void this.applyIdleAvatar(gen);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,12 +3,15 @@
|
|||||||
</template>
|
</template>
|
||||||
|
|
||||||
<script setup lang="ts">
|
<script setup lang="ts">
|
||||||
import { ref, onMounted, watch } from 'vue';
|
import { ref, onMounted, watch, nextTick } from 'vue';
|
||||||
import axios from 'axios';
|
import axios from 'axios';
|
||||||
import AvatarUpload from './AvatarUpload.vue';
|
import AvatarUpload from './AvatarUpload.vue';
|
||||||
|
|
||||||
const props = defineProps<{ botId: string }>();
|
const props = defineProps<{ botId: string }>();
|
||||||
const avatarDataUrl = ref<string | null>(null);
|
const avatarDataUrl = ref<string | null>(null);
|
||||||
|
// Stays true until the watcher queued by the load-time assignment has run,
|
||||||
|
// so the initial null → loaded-data-url transition does not fire a redundant
|
||||||
|
// PUT echoing the just-fetched bytes back to the server.
|
||||||
let initializing = true;
|
let initializing = true;
|
||||||
|
|
||||||
async function loadCurrent() {
|
async function loadCurrent() {
|
||||||
@@ -22,6 +25,9 @@ async function loadCurrent() {
|
|||||||
}
|
}
|
||||||
avatarDataUrl.value = null;
|
avatarDataUrl.value = null;
|
||||||
} finally {
|
} finally {
|
||||||
|
// Wait for the watcher's flush queue to drain (it'll see initializing=true
|
||||||
|
// and bail), then release for real user-driven changes.
|
||||||
|
await nextTick();
|
||||||
initializing = false;
|
initializing = false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user