muse icon indicating copy to clipboard operation
muse copied to clipboard

The /skip command puts the bot's queue into an invalid state

Open Hialus opened this issue 10 months ago • 1 comments

What's happening? The skip command can put the bot's queue into an invalid state. Setup:

  1. Add at least 2 songs into the queue
  2. The first song starts playing
  3. The /queue command shows the first song as now playing and the second+ songs as in the queue

Then send the /skip command (without specifying any amount).

The observed behavior is as follows:

  1. The bots starts playing the next song
  2. The /queue command shows the third song (or "🚫 ope: queue is empty" if there is none) as now playing and the fourth+ songs as in the queue

The expected behavior would be:

  1. The bots starts playing the next song
  2. The /queue command shows the second song as now playing and the third+ songs as in the queue

Note: I had cases where it worked fine, but it more often than not displays the incorrect behavior.

Logs N/A - No relevant logs are printed

Screenshots N/A

Additional context I've debugged this locally and here is what I found happens internally:

  1. The skip commands calls the services/Player.ts::forward method
  2. This advances the queuePosition by 1
  3. The play method is called
  4. The play method gets the new song to play
  5. The play method calls the getStream method
  6. The getStream method stops the audioPlayer
  7. This usually leads to the onAudioPlayerIdle method being called
  8. The onAudioPlayerIdle method then calls this.forward(1) again, which then does not call the play method, but does increment the queuePosition by 1 again
  9. Then the play method continues and actually starts playing the previously acquired new song to play
  10. The skip command then finishes

So the main problem is that the skip command leads to the queuePosition being incremented twice instead of only once.

I do not have sufficient knowledge of this project to suggest a concrete fix for this, but I'd suggest adding a check in the respective third if-statement of the onAudioPlayerIdle method to only call this.forward(1) when the idling was not caused by a skip/seek. One approach that works for the specific scenario I listed:

if (this.getCurrent() === this.nowPlaying) {
  await this.forward(1);
}

Runtime I'm running Muse:

  • [x] Directly from the cloned repository
  • [x] Inside a Docker container
  • [ ] Something else (please elaborate)

Versions

  • Muse: latest (Docker) / master branch
  • Docker: 26.1.4
  • OS: Debian / macOS
  • Node.js: 23.7.0
  • ffmpeg: 7.1

Hialus avatar Feb 18 '25 19:02 Hialus

it seems both /skip and /next have the exact issue

mrhashishah avatar Apr 21 '25 21:04 mrhashishah