mirror of
https://github.com/ZHANGTIANYAO1/teamspeak-music-bot.git
synced 2026-10-01 20:42:50 +08:00
fix(queue): 随机模式下 !pn 插入的歌真正下一首播放
Random / RandomLoop 下 next() 从 shuffle bag(playedIndices)里随机挑,完全 不看数组顺序,所以 addNext() 把歌插到 currentIndex+1 之后,它只是和别的歌一 样等着被随机抽中。!pn / !playnext 和 WebUI 的「下一首播放」按钮都受影响,而 两者都回了一句「Up next: …」,等于在骗人。 addNext() 现在在随机模式下把插入位置记到 forwardStack —— next() 本来就会先 看这个栈(原本用于 prev 的回退位置),所以不用改 next() 的挑选逻辑。栈是后进 先出,正好和连续 !pn 在队列里呈现的顺序一致(每次插入都排在上一次前面), 与顺序模式表现相同。 只加这一句是不够的,另外两处会让它失效: - addNext() 原本只把 playedIndices 和 history 中大于 currentIndex 的下标 +1, 没管 forwardStack。连续 !pn 两次会得到两个相同的下标,第二次 pop 出来的旧 下标恰好等于 currentIndex,被静默丢弃,先插入的那首就永远不会播。 - remove() 同样只修 playedIndices 和 history。删掉队列中靠前的歌之后, forwardStack 里的下标会指向挤上来的另一首歌;删得多了甚至越界,此时 next() 返回 undefined,而 BotInstance.playNext 把假值当作队列播完直接停止 播放。 所以一并给 forwardStack 补上和另外两个结构相同的平移/清理规则,并让 next() 像 prev() 处理失效 history 那样,循环跳过越界或指向当前曲目的条目。上限行为 也对齐 history:超出 HISTORY_LIMIT 时丢最旧的,而不是拒绝刚插入的那首。 新增测试覆盖两种随机模式、连续插入的顺序、shuffle bag 播完后插入、删除前后 的下标同步、prev 标记与插入条目共栈,以及 200 步交错操作不产生失效下标。已用 变异测试逐条回退上述四处改动确认这些用例确实会失败。 Closes #141 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
4e4354282d
commit
cc3684ff86
2 files changed
+182
-10
No files matched your search
@@ -605,4 +605,141 @@ describe("PlayQueue", () => {
|
||||
expect(q.list().map((s) => s.id)).toEqual(["A"]);
|
||||
});
|
||||
});
|
||||
|
||||
// Issue #141: in Random/RandomLoop, next() picks from the shuffle bag and
|
||||
// ignores array order, so a song spliced in by addNext (!pn) was NOT played
|
||||
// next — it just waited for its random turn like any other song. addNext now
|
||||
// records the insert slot on the forward stack, which next() honours first.
|
||||
describe("addNext in random modes (issue #141)", () => {
|
||||
for (const mode of [PlayMode.Random, PlayMode.RandomLoop]) {
|
||||
it(`plays the inserted song next in ${mode} mode`, () => {
|
||||
queue.setMode(mode);
|
||||
for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id));
|
||||
queue.play(); // current = 0 (a)
|
||||
queue.addNext(makeSong("x"));
|
||||
expect(queue.next()?.id).toBe("x");
|
||||
});
|
||||
}
|
||||
|
||||
it("plays consecutive inserts in the order the queue displays them", () => {
|
||||
queue.setMode(PlayMode.RandomLoop);
|
||||
for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id));
|
||||
queue.play(); // current = 0 (a)
|
||||
queue.addNext(makeSong("x"));
|
||||
queue.addNext(makeSong("y")); // splices in front of x, as in sequential
|
||||
expect(queue.list().map((s) => s.id)).toEqual(["a", "y", "x", "b", "c", "d"]);
|
||||
expect(queue.next()?.id).toBe("y");
|
||||
expect(queue.next()?.id).toBe("x");
|
||||
});
|
||||
|
||||
it("honours the insert even after the shuffle bag is exhausted", () => {
|
||||
// Random (non-loop) returns null once every song has played. Songs added
|
||||
// afterwards must still be reachable via !pn — and with TWO of them the
|
||||
// order can only come from the forward stack, not from the bag having a
|
||||
// single remaining candidate.
|
||||
queue.setMode(PlayMode.Random);
|
||||
for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id));
|
||||
queue.play();
|
||||
for (let i = 0; i < 3; i++) queue.next();
|
||||
expect(queue.next()).toBeNull(); // bag exhausted
|
||||
queue.addNext(makeSong("x"));
|
||||
queue.addNext(makeSong("y"));
|
||||
queue.addNext(makeSong("z"));
|
||||
expect(queue.next()?.id).toBe("z");
|
||||
expect(queue.next()?.id).toBe("y");
|
||||
expect(queue.next()?.id).toBe("x");
|
||||
});
|
||||
|
||||
it("pops past a prev() marker to reach the pending insert", () => {
|
||||
// prev() shares the forward stack, and in random mode with no history it
|
||||
// pushes the current index and then returns null. next() must walk past
|
||||
// those self-referencing markers instead of consuming one and giving up
|
||||
// to the shuffle bag.
|
||||
queue.setMode(PlayMode.Random);
|
||||
for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id));
|
||||
queue.play(); // a
|
||||
queue.addNext(makeSong("x"));
|
||||
expect(queue.prev()).toBeNull();
|
||||
expect(queue.prev()).toBeNull();
|
||||
expect(queue.next()?.id).toBe("x");
|
||||
});
|
||||
|
||||
it("plays each song exactly once — the insert is not replayed later", () => {
|
||||
queue.setMode(PlayMode.Random);
|
||||
for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id));
|
||||
queue.play(); // a
|
||||
queue.addNext(makeSong("x"));
|
||||
queue.addNext(makeSong("y"));
|
||||
|
||||
const played = [queue.current()!.id];
|
||||
for (let i = 0; i < 5; i++) played.push(queue.next()!.id);
|
||||
expect(queue.next()).toBeNull(); // bag exhausted
|
||||
expect(played.slice(0, 3)).toEqual(["a", "y", "x"]);
|
||||
expect(new Set(played).size).toBe(6);
|
||||
});
|
||||
|
||||
it("keeps the insert reachable after an earlier song is removed", () => {
|
||||
queue.setMode(PlayMode.RandomLoop);
|
||||
for (const id of ["a", "b", "c", "d"]) queue.add(makeSong(id));
|
||||
queue.playAt(2); // current = 2 (c)
|
||||
queue.addNext(makeSong("x")); // [a, b, c, x, d]
|
||||
queue.remove(0); // [b, c, x, d] — x slides from 3 to 2
|
||||
expect(queue.next()?.id).toBe("x");
|
||||
});
|
||||
|
||||
it("drops the entry when the inserted song is itself removed", () => {
|
||||
// Leaving the stale entry behind would not throw — index 2 still exists
|
||||
// after the removal, it just points at a different song. So the queue is
|
||||
// arranged with exactly one song the shuffle bag can legally return:
|
||||
// anything else means the dead forward entry was honoured.
|
||||
queue.setMode(PlayMode.RandomLoop);
|
||||
for (const id of ["a", "b", "c"]) queue.add(makeSong(id));
|
||||
queue.playAt(0); // current = 0 (a), played = {0}
|
||||
queue.next(); // b or c — two of the three are now played
|
||||
const remaining = queue.list().find((s) => s.id !== "a" && s.id !== queue.current()!.id)!;
|
||||
queue.addNext(makeSong("x")); // spliced at currentIndex+1
|
||||
queue.remove(queue.getCurrentIndex() + 1); // …and removed again
|
||||
expect(queue.list().map((s) => s.id)).not.toContain("x");
|
||||
expect(queue.next()?.id).toBe(remaining.id);
|
||||
});
|
||||
|
||||
it("never yields a stale index under interleaved inserts and removals", () => {
|
||||
// The forward stack holds array indices, so every splice has to shift
|
||||
// them. next() returning `undefined` here (an out-of-range index) reads
|
||||
// as end-of-queue to BotInstance.playNext and silently stops playback.
|
||||
queue.setMode(PlayMode.RandomLoop);
|
||||
for (let i = 0; i < 6; i++) queue.add(makeSong(`s${i}`));
|
||||
queue.play();
|
||||
for (let step = 0; step < 200; step++) {
|
||||
const roll = step % 4;
|
||||
if (roll === 0) queue.addNext(makeSong(`x${step}`));
|
||||
else if (roll === 1 && queue.size() > 1) queue.remove(step % queue.size());
|
||||
else {
|
||||
const song = queue.next();
|
||||
expect(song === null || song === queue.current()).toBe(true);
|
||||
if (song !== null) expect(song).toBeDefined();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("leaves sequential/loop behaviour untouched", () => {
|
||||
queue.setMode(PlayMode.Sequential);
|
||||
for (const id of ["a", "b", "c"]) queue.add(makeSong(id));
|
||||
queue.play(); // a
|
||||
queue.addNext(makeSong("x"));
|
||||
expect(queue.next()?.id).toBe("x");
|
||||
expect(queue.next()?.id).toBe("b");
|
||||
expect(queue.next()?.id).toBe("c");
|
||||
expect(queue.next()).toBeNull();
|
||||
});
|
||||
|
||||
it("still appends (no forward entry) when nothing is playing", () => {
|
||||
queue.setMode(PlayMode.Random);
|
||||
queue.add(makeSong("a"));
|
||||
queue.addNext(makeSong("x")); // currentIndex is still -1 → plain push
|
||||
expect(queue.list().map((s) => s.id)).toEqual(["a", "x"]);
|
||||
queue.play(); // a — a stray forward entry would have hijacked this
|
||||
expect(queue.current()?.id).toBe("a");
|
||||
});
|
||||
});
|
||||
});
|
||||
+45
-10
@@ -60,8 +60,12 @@ export class PlayQueue {
|
||||
* or queue empty), so the existing "add → idle bot starts playing"
|
||||
* flow continues to work.
|
||||
*
|
||||
* Shifts playedIndices and history entries > currentIndex by +1 so
|
||||
* their references stay valid after the splice.
|
||||
* Shifts playedIndices, history and forwardStack entries > currentIndex
|
||||
* by +1 so their references stay valid after the splice.
|
||||
*
|
||||
* In the random modes the array position alone means nothing — next()
|
||||
* picks from the shuffle bag — so the insert slot is also recorded on
|
||||
* the forward stack, which next() consults first (issue #141).
|
||||
*/
|
||||
addNext(song: QueuedSong): void {
|
||||
if (this.currentIndex < 0 || this.songs.length === 0) {
|
||||
@@ -80,6 +84,23 @@ export class PlayQueue {
|
||||
this.history = this.history.map((i) =>
|
||||
i > this.currentIndex ? i + 1 : i,
|
||||
);
|
||||
|
||||
this.forwardStack = this.forwardStack.map((i) =>
|
||||
i > this.currentIndex ? i + 1 : i,
|
||||
);
|
||||
|
||||
// Push AFTER the shift, or the slot we just claimed would be shifted
|
||||
// too. Stacking makes repeated !pn play in the order the queue shows
|
||||
// them (each insert lands in front of the previous one), matching what
|
||||
// sequential mode does with the same array. Bounded like history: drop the
|
||||
// OLDEST pending entry rather than refusing the newest, so the song the
|
||||
// user just asked for is always the one that gets honoured.
|
||||
if (this.mode === PlayMode.Random || this.mode === PlayMode.RandomLoop) {
|
||||
this.forwardStack.push(insertAt);
|
||||
if (this.forwardStack.length > PlayQueue.HISTORY_LIMIT) {
|
||||
this.forwardStack.shift();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
remove(index: number): QueuedSong | null {
|
||||
@@ -106,6 +127,13 @@ export class PlayQueue {
|
||||
.filter((idx) => idx !== index)
|
||||
.map((idx) => (idx > index ? idx - 1 : idx));
|
||||
|
||||
// …and for the forward stack, which now also carries !pn insert slots
|
||||
// (issue #141). Left unshifted, a removal elsewhere in the queue would
|
||||
// silently repoint the entry at whatever song slid into that slot.
|
||||
this.forwardStack = this.forwardStack
|
||||
.filter((idx) => idx !== index)
|
||||
.map((idx) => (idx > index ? idx - 1 : idx));
|
||||
|
||||
return removed;
|
||||
}
|
||||
|
||||
@@ -158,15 +186,22 @@ export class PlayQueue {
|
||||
}
|
||||
case PlayMode.Random:
|
||||
case PlayMode.RandomLoop: {
|
||||
// 优先回到前进栈记录的位置(prev 退回的歌)
|
||||
if (this.forwardStack.length > 0) {
|
||||
// 优先回到前进栈记录的位置(prev 退回的歌,或 !pn 插入的歌)。
|
||||
// Keep popping past entries that no longer point anywhere useful,
|
||||
// the way prev() walks past stale history entries. Without the loop a
|
||||
// prev() that pushed the current index would swallow the pending !pn
|
||||
// entry behind it. The range check is belt-and-braces — addNext and
|
||||
// remove keep the stack in sync — but an out-of-range index here would
|
||||
// set currentIndex out of bounds and hand back `undefined`, which
|
||||
// BotInstance.playNext reads as end-of-queue and stops playback.
|
||||
while (this.forwardStack.length > 0) {
|
||||
const target = this.forwardStack.pop()!;
|
||||
if (target !== this.currentIndex) {
|
||||
this.pushHistory(this.currentIndex);
|
||||
this.currentIndex = target;
|
||||
this.playedIndices.add(target);
|
||||
return this.songs[target];
|
||||
}
|
||||
if (target < 0 || target >= this.songs.length) continue;
|
||||
if (target === this.currentIndex) continue;
|
||||
this.pushHistory(this.currentIndex);
|
||||
this.currentIndex = target;
|
||||
this.playedIndices.add(target);
|
||||
return this.songs[target];
|
||||
}
|
||||
|
||||
// Shuffle bag: pick uniformly from the songs not yet played this
|
||||
|
||||
Reference in new issue
Block a user