shipgirlproject / Shoukaku

A stable, powerful and updated wrapper around Lavalink
https://guide.shoukaku.shipgirl.moe/
MIT License
273 stars 84 forks source link

Enhancements #159

Closed flav-code closed 1 year ago

flav-code commented 1 year ago

Minor improvement

Added Rest.getLavalinkInfo() and his type

https://github.com/lavalink-devs/Lavalink/blob/master/IMPLEMENTATION.md#get-lavalink-info

flav-code commented 1 year ago

well, I made the function to avoid duplicating coe but you can add an argument for when you need to disconnect the bot or set channel_id to null before calling the function.

Deivu commented 1 year ago

I just dont really turn it to null before in a case you want to reconnect, but I guess that should be on developer's responsibility eh?

flav-code commented 1 year ago

That's why I didn't put the function for disconnect, I only put it where it's the same.

Deivu commented 1 year ago

Yeah, but I'm thinking if we should let the developer do that instead, after all I think anything outside the scope of lavalink & discord api should be in developer's responsibility, not our responsibility. I'll re-review once decided.

flav-code commented 1 year ago

but what are you talking about

Deivu commented 1 year ago

Reset every data in the connection class, before calling this.sendVoiceUpdate(), so it's up to the developer's (user) responsibility to cache the existing data

flav-code commented 1 year ago

so in disconnect you call the function and you want the user to delete the channel_id before calling the disconnect function?

Deivu commented 1 year ago

so in disconnect you call the function and you want the user to delete the channel_id before calling the disconnect function?

Tldr:

this.channel_id = null;
this.deafened = false;
this.muted = false;
this.sendVoiceUpdate();
// ... rest of the code in disconnect
flav-code commented 1 year ago

That's what I wanted to do... Is that all right with you? Shall I add it?

Deivu commented 1 year ago

For now yeah, as I stated above.

Yeah, but I'm thinking if we should let the developer do that instead, after all I think anything outside the scope of lavalink & discord api should be in developer's responsibility, not our responsibility. I'll re-review once decided.

To keep things simple and concise

flav-code commented 1 year ago

Okay, but this is just to make the code simpler and it won't change anything for the user.

Deivu commented 1 year ago

probably kek, if they dont cache channel id and on closed event, they get the channelid from this class, then sure it will break, but then again, im passing the responsibility of caching this to them now

flav-code commented 1 year ago

Oh okay, I get it, I'm actually using the Connection.channelId

flav-code commented 1 year ago

lol

flav-code commented 1 year ago

good