Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make use of discord.VoiceProtocol instead of socket_response #99

Closed
wants to merge 26 commits into from

Conversation

PredaaA
Copy link
Member

@PredaaA PredaaA commented Aug 7, 2021

This PR makes use of discord.VoiceProtocol instead of listening to socket_response event. Which will be useful for next discord.py major release where that event no longer exists unless you pass a parameter to Client.
But it also makes things more "cleaner", as it "bind" the Red Lavalink Player object to discord.Guild object (guild.voice_client).
Though it comes with several breaking changes, that might break on current implementations of Red-Lavalink (in the case of Red's audio it doesn't seems to be a lot).

Breaking changes:

  • player_manager.PlayerManager no longer exists. The needed methods are now in node.Node.
  • Therefore Player.manager no longer exists too, node.Node can be accessed instead. Node.player_manager too.
  • voice_ws_func parameter is no longer needed to be passed in node.Node, and therefore the get_voice_ws method has been removed too.
  • player_manager.user_id & player_manager.channel_finder_func are removed.
  • RESTClient now takes Player instead of Node as parameter.
  • The rest_api.reset_session & rest_api.close methods are removed.

Fixes #98 & closes #97.

@Jackenmen
Copy link
Member

It still make use of bot._connection._get_websocket though, maybe if there is an official API for that it would be better to use it

It seems we only use it here:
https://github.com/PredaaA/Red-Lavalink/blob/cff92370f2909f0796e662299c9e37cf46d71287/lavalink/player_manager.py#L246-L248

Which can probably be replaced with bot.is_closed().

@PredaaA
Copy link
Member Author

PredaaA commented Aug 9, 2021

It still make use of bot._connection._get_websocket though, maybe if there is an official API for that it would be better to use it

It seems we only use it here:
PredaaA/Red-Lavalink@cff9237/lavalink/player_manager.py#L246-L248

Which can probably be replaced with bot.is_closed().

I ended up using Shard.is_closed() instead, which seems to be doing the intended behavior (even on Red's cog side).
So the get_voice_ws method is removed too.

@FightMan01
Copy link

I'm testing this PR for 2 week now with my bot (9300+ guilds) and everything works fine. Thanks.

@PredaaA
Copy link
Member Author

PredaaA commented Sep 3, 2021

Sending that here so it won't be lost. There are mem leaks problems, and players state not being updated when stopped/disconnected.

lavalink/node.py Outdated Show resolved Hide resolved
lavalink/node.py Outdated Show resolved Hide resolved
lavalink/__init__.py Outdated Show resolved Hide resolved
@Jackenmen Jackenmen added this to the 0.10.0 milestone Jan 19, 2022
@PredaaA
Copy link
Member Author

PredaaA commented Feb 16, 2022

Pushed changes from latest review that @aikaterna done and was pushed on another remote.

@PredaaA PredaaA marked this pull request as ready for review February 20, 2022 21:00
@PredaaA
Copy link
Member Author

PredaaA commented Feb 20, 2022

PR ready for review. (finally)

lavalink/__init__.py Outdated Show resolved Hide resolved
@aikaterna
Copy link
Member

aikaterna commented Feb 20, 2022

It should be 0.10.0

(nvm, I see what you're saying now Life)

@Lifeismana

This comment was marked as outdated.

@PredaaA PredaaA force-pushed the feature/dpy-voiceprotocol branch from 2b0aa70 to 412203a Compare February 20, 2022 22:31
@Jackenmen
Copy link
Member

Superseded by #122

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Does not work with discord.py v2.0 Use d.py's public interfaces (i.e. VoiceProtocol) instead of private ones
5 participants