`Player#track` is not unset when a track is manually (`Player#stopTrack()`) or automatically (`"end"` event) stopped.

[vxern]

Player#playTrack() can be used to play a given track. It goes ahead and sets Player#track to the track that is being played, and from there you can do Player#track !== null to check that a track is being played.

However, Player#stopTrack() does not unset this value, meaning if I do Player#playTrack(), and then Player#stopTrack(), Player#track will still be set, even if the player is not playing anything at all.

Similarly, I believe when the library emits "end" in Player#onPlayerEvent(), it also doesn't unset the track.

[Deivu]

It entirely depends on implementation in my opinion for this case. stopTrack() does what it says, it stops the playback, which sets the position to 0. I didn't set the track to null because I don't think stopTrack() means clear. Eg: see how musicbee implements stop in the video, they just set the time to 0 and stops the playback, which is I think very fitting. Most other players don't even implement stop button nowdays, see spotify for that.

Eitherways, I don't think I need to set the track to null when you stop since you are just stopping the playback, you aren't clearing it, hence setting the position to 0 which signifies the playback has been stopped is fine in this case.

Changing this however would be breaking, since changing this would require other users to change something in their code if they get the last played track from Player#track, but we'll see I guess?

https://github.com/shipgirlproject/Shoukaku/assets/36309350/f1799034-89c8-4426-b22d-3c4afe288b0d

[Deivu]

If you really need that functionality however, there is a way to do that without forking it by extending the player class yourself, then doing something like:

async stopTrack() {
  await super.stopTrack();
  this.track = null;
}

onPlayerEvent(data) {
  if (json.type === "TrackEndEvent") this.track = null;
  super.onPlayerEvent(data);
}

then put it on options.structures of Shoukaku

[vxern]

I didn't set the track to null because I don't think stopTrack() means clear

Well, no, but clear() is an action that's intended for resetting the player to its base state, it can't be used for getting rid of the current track.

Changing this however would be breaking, since changing this would require other users to change something in their code if they get the last played track from Player#track, but we'll see I guess?

Was track not intended to be for getting the current track, and not the last track? I'd argue if a developer uses an API in a way that wasn't intended, that's not the responsibility of the library developer to maintain the behaviour of, and therefore would not be a breaking change if changed.

[vxern]

In the Lavalink documentation, the track value is also documented as being the 'currently playing track'.

[Deivu]

Was track not intended to be for getting the current track, and not the last track? I'd argue if a developer uses an API in a way that wasn't intended, that's not the responsibility of the library developer to maintain the behaviour of, and therefore would not be a breaking change if changed.

track is intended for the current track. I didn't notice that I was being illogical to myself there. What I meant in here if they use the #track property on TrackEndEvent for doing something about the track, like storing it somewhere, or using it to decide what to do next. While it is included in the event payload here, it would be breaking if some user do use the track prop instead.

But eitherways, I don't think we should be still clearing the Player#track prop on stopTrack() because you are just stopping the playback as I said earlier, we dont "clear" it. But personally this is up to one's implementation and understanding of what stop is.

On lavalink src, they don't also clear it. see: https://github.com/lavalink-devs/Lavalink/blob/0ea9afdef8d485d6bf9ad5f09af8352f6da561c6/LavalinkServer/src/main/java/lavalink/server/player/LavalinkPlayer.kt#L85 and see: https://github.com/lavalink-devs/Lavalink/blob/0ea9afdef8d485d6bf9ad5f09af8352f6da561c6/LavalinkServer/src/main/java/lavalink/server/player/EventEmitter.kt#L59

[Deivu]

Theres one caveat though, lavaplayer do have activeTrack and previousTrack in their audioPlayer class, but Shoukaku don't implement that, nor really need it, hence imitating the behavior of music players seems to be a good choice in here.

[vxern]

Right, well, it's a small gotcha. I don't suppose it's that much of a problem, though, especially since it can be worked around. And, sure, I suppose it's up to the implementation.

Thanks anyway for having a look at it.

Closing.