discordjs / voice

Implementation of the Discord Voice API for discord.js and other JS/TS libraries
Apache License 2.0
328 stars 112 forks source link

Allow using a "finish" event as an eventListener for AudioPlayer #186

Closed Nalin-Angrish closed 2 years ago

Nalin-Angrish commented 2 years ago

Is your feature request related to a problem? Please describe. I am making a music player bot, and in it, I want to be able to know when the playback of an audio file finishes, so that I can play the next song in queue. In the previous versions (v12), I could use something like this:

connection.play(file)
  .on('finish', ()=>{
    playNext()
  })

But now, such an event handler is not present with the VoiceConnection nor AudioPlayer. AudioPlayer has an event for Idle, but surprisingly, sometimes, when a song finishes, it remains idle for a bit too long and plays the next 2+ songs, and my bot's architecture forces it to play only the last song, thus skipping a few tracks.

Describe the ideal solution An event handler similar to the one provided in the previous versions, where the event is only fired once a track finishes, or either a bug fix of this issue if it is so.

Describe alternatives you've considered No alternatives come to my mind.

Additional context image Here, the first song played well, but then the next 3 songs were immediately fired when the first finished, and the bot finally played the last song, thus skipping 2. This is just one example, but the same happened a few more times, with one time skipping 5 songs.

mxscowc1ty commented 2 years ago

I confirm. The AudioPlayer.Idle function works extremely unstable, sometimes 2+ times on finish

iim-ayush commented 2 years ago

Please post code that can reproduce this situation

iim-ayush commented 2 years ago

I confirm. The AudioPlayer.Idle function works extremely unstable, sometimes 2+ times on finish

If you are attaching .on events twice, it will fire twice. Not a issue of audioplayer. Try to change on event to .once and try to run your code again

Nalin-Angrish commented 2 years ago

Okay I think I found out the issue. I think because I was looping through the play queue and added the listener whenever I played a track, it double-fired everytime. Still not confirmed, but I think using the .once would fix it. Thanks @killer069 for this suggestion.

@mxscowc1ty maybe try this thing.

Nalin-Angrish commented 2 years ago

But wait, sometimes it more-than-double-fired (3 songs together as in the start of the issue) so can't say of that point.

mxscowc1ty commented 2 years ago

Okay I think I found out the issue. I think because I was looping through the play queue and added the listener whenever I played a track, it double-fired everytime. Still not confirmed, but I think using the .once would fix it. Thanks @killer069 for this suggestion.

@mxscowc1ty maybe try this thing.

Hey! Thanks for the answer. Yes, fortunately it helps. But I'm still not clear why the function is called multiple times otherwise.

iim-ayush commented 2 years ago

Okay I think I found out the issue. I think because I was looping through the play queue and added the listener whenever I played a track, it double-fired everytime. Still not confirmed, but I think using the .once would fix it. Thanks @killer069 for this suggestion. @mxscowc1ty maybe try this thing.

Hey! Thanks for the answer. Yes, fortunately it helps. But I'm still not clear why the function is called multiple times otherwise.

I guess you both are attaching a event when a audio is finished playing. So it is causing the issue.

But wait, sometimes it more-than-double-fired (3 songs together as in the start of the issue) so can't say of that point.

It will fire more than 2 times if you do this on 3 songs or more than that. Since you are attaching a event on every song ends. So if 2 songs are completed, you are attaching one more event [ that totals to 3 events attached ]. So it fires multiple times.

@mxscowc1ty @Nalin-2005 Check your listeners count by this :

console.log(<AudioPlayer>.listenerCount(AudioPlayerStatus.Idle))
iim-ayush commented 2 years ago

@amishshah OR @Nalin-2005

Can you please close this issue ??

Nalin-Angrish commented 2 years ago

Not yet please. I was busy with some work so haven't tested it. Will update this issue within the next 48 hours.

iim-ayush commented 2 years ago

Not yet please. I was busy with some work so haven't tested it. Will update this issue within the next 48 hours.

hmm. 48 hours ??

Nalin-Angrish commented 2 years ago

Ya i meant 2 days. I was not at home.

Now, it is tested and it works now. So i'm closing the issue.

Thanks for the help @killer069 !