Ghost joins

[Fabricio20]

Hey Fre_d, I've been having some issues recently with this client. My bot has experiencing ghost reconnects to voice channels.

The problem looks like this:

• Join voice.
• Send OP Play.
• Send OP Stop.
• Send OP Disconnect.
• Wait for some random amount of time.
• Suddenly the bot joins the voice channel (No audio playing, just a random join).

Attached is a grep: LavaLink.txt

Any clues on what is causing this? Disconnect code:

[freyacodes]

This could likely be a problem with your code. Here is what I suggest you do:

1. Find all cases where you attempt to join a channel, and log the reason.
2. Log all joins by your bot via your JDA event listener.
3. Compare the requests with the events.

[weeryan17]

I know this is an old issue, but I was running into this same issue, and I was able to track it down. When the bot re-connects to discord and the reconnected event is fired, you re-join the bot to the last voice channel. however the last voice channel never gets unset, nore are links ever removed from the map. This causes it to where whenever the bot needs to re-connect all channels it was joined to. This is the code that causes it. https://github.com/Frederikam/Lavalink-Client/blob/82883a7ead971fcf295c878905095e9f72973523/src/main/java/lavalink/client/io/jda/JdaLavalink.java#L118-L128 I can pr a fix for this, but before that I want to make sure we're on the same page when it comes to how this should be fixed. I'm thinking when the bot disconnects from a voice channel we remove the link from the map. It could also set the last channel to null, but I was thinking removing the link from the map in order to save on some memory, but ultimately it's up to you. This shouldn't be too hard as we can just hook into this code and add it so it removes the link from the map https://github.com/Frederikam/Lavalink-Client/blob/82883a7ead971fcf295c878905095e9f72973523/src/main/java/lavalink/client/io/jda/JDAVoiceInterceptor.java#L38-L43

[freyacodes]

The Link gets removed from the map when the Link gets destroyed. It is the library user's responsibility to destroy a link when no-longer needed. This is also why I could never reproduce this.

I'm wondering if it might make sense to simply just deprecate the disconnect() method in favour of destroy(). The former allows the library user to reconnect later with the same audio player (if it isn't cleaned up), but it is prone to misuse and not all that useful.

[weeryan17]

Ahhh that makes sense, but you might want to add that to the readme, as currently I don't think it mentions anywhere it's the responsibility of the implementation to remove links, you do mention the destroy method, but don't mention the context for how it should be use just that it removes the reference. Also I wouldn't depreciate the disconnect() method, I would just make that method destroys as well (and maybe add a disconnect(boolean destroy) method for people who still want to disconnect and not destory), and clarify in the readme that you should destroy links when no longer in use.

[freyacodes]

I am not very keen on spending time supporting the current behaviour. This class is more complex than it has to be because of this. I would need to hear a compelling case to keep it in.

IIRC @Fabricio20 even told me that he moved to destroying instead.

[weeryan17]

Calling disconnect() makes a lot more sense then calling destroy() when you're trying to disconnect. And also there is the matter of destroying on events, so it makes more sense logically to disconnect when you want it to disconnect, and destroy on disconnect events to get rid of the link. If nothing else make disconnect() just run destory()

[freyacodes]

Calling disconnect() makes a lot more sense then calling destroy() when you're trying to disconnect.

Sure, but it doesn't make much sense that calling disconnect on the client sends destroy to the server.

But wouldn't it also be confusing that the disconnect() method would destroy the player state as a side-effect?

it makes more sense logically to disconnect when you want it to disconnect, and destroy on disconnect events to get rid of the link.

I understand the lifecycle you're explaining, but I fail to understand how this benefits anyone. Why would we care about the voice state after we've requested to end it?

If nothing else make disconnect() just run destory()

1. This is needless API duplication
2. Deprecation gives the user a heads up before we change anything

[weeryan17]

Fair enough.

Going back a bit though, just make sure you document the need to destroy unneeded players in the readme, and make sure to specify it isn't automatically done when the bot is disconnected. Often times users won't use proper leave methods and just r-click and disconnect the bot via that method, so not further clarifying in the readme could lead to situations where this issue gets brought up again as you expect the implementation to handle those, and the implementation expects this to handle those

[freyacodes]

Closing as the method was deprecated in a0934a040917e39524e4ce573a1ff1658ea0e55d