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

v3: voice: udp.Manager for seamless reconnection #241

Merged
merged 2 commits into from
Jan 19, 2022
Merged

Conversation

diamondburned
Copy link
Owner

This pull request breaks the previous v2 voice API to add in a udp.Manager that effectively manages reconnection of the voice UDP connection. This fixes a previous issue where changing the voice region would break the voice connection.

The biggest breaking change in this PR (as far as voice sending goes) actually has nothing to do with this fix. The change is written below:

 func (s *Session) JoinChannelCtx(
-	ctx context.Context, gID discord.GuildID, cID discord.ChannelID, mute, deaf bool) error {
+	ctx context.Context, chID discord.ChannelID, mute, deaf bool) error {

This change is made primarily for convenience.

A small side effect of this change is that the minimum supported Go version has been bumped to 1.16 for some reason. By the time v3 is finalized, however, Go 1.17 would've already been released, so this is fine.

This pull request fixes #238.

@diamondburned diamondburned marked this pull request as draft July 4, 2021 21:55
@1lann
Copy link
Contributor

1lann commented Jan 5, 2022

I've been using the 238 branch for a while (only like the last 3 weeks cause I totally forgot to switch to that branch for the project I was working on lol) for a persistent bot that stays in voice chat and it's been working great, it doesn't seem to noticably permanently lose connection from voice chat anymore. I've also had no problems with it (apart from #242, I haven't been running it for weeks with -race however). I think it might be good enough™️ to merge anyhow, so it makes it into v3.

@diamondburned
Copy link
Owner Author

Interesting. I'll rebase it tomorrow and see how feasible it is. Chances are more testing would have to be done to ensure that the gateway refactor works with the PR.

@diamondburned diamondburned force-pushed the 238-branch branch 2 times, most recently from cdcc997 to ad1dfdf Compare January 6, 2022 21:48
@diamondburned diamondburned marked this pull request as ready for review January 19, 2022 05:27
diamondburned and others added 2 commits January 18, 2022 21:31
This commit refactors a lot of voice's internals to be more stable and
handle more edge cases from Discord's voice servers. It should result in
an overall more stable voice connection.

A few helper functions have been added into voice.Session. Some fields
will have been broken and changed to accomodate for the refactor, as
well.

Below are some commits that have been squashed in:

    voice: Fix Speaking() panic on closed
    voice: StopSpeaking should not error out
        The rationale is added as a comment into the Speaking() method.
    voice: Add TestKickedOut
    voice: Fix region change disconnecting
@diamondburned
Copy link
Owner Author

A few people have tested this branch, and it appears to be working normally, except for the case when the user moves the bot around.

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.

Voice: v2: ReadPacket() never finishes when region changes
2 participants