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

Expose VoiceServerUpdate events #984

Merged
merged 7 commits into from
May 4, 2018
Merged

Expose VoiceServerUpdate events #984

merged 7 commits into from
May 4, 2018

Conversation

devoxin
Copy link
Contributor

@devoxin devoxin commented Mar 16, 2018

Resolves #983 by exposing VoiceServerUpdates to the user.

This should not break any existing functionality within the lib, and adds an event that will be helpful in supporting external audio clients such as Lavalink.

@@ -0,0 +1,19 @@
using System.Diagnostics;

namespace Discord.WebSocket.Entities.Guilds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be namespaced just to Discord.WebSocket

@@ -1474,6 +1475,9 @@ public override RestVoiceRegion GetVoiceRegion(string id)
await UnknownGuildAsync(type, data.GuildId).ConfigureAwait(false);
return;
}

SocketVoiceServer VoiceServer = new SocketVoiceServer(data.GuildId, data.Endpoint, data.Token);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer to use var, and local variables should be camel-cased

@@ -1474,6 +1475,9 @@ public override RestVoiceRegion GetVoiceRegion(string id)
await UnknownGuildAsync(type, data.GuildId).ConfigureAwait(false);
return;
}

SocketVoiceServer VoiceServer = new SocketVoiceServer(data.GuildId, data.Endpoint, data.Token);
await TimedInvokeAsync(_voiceServerUpdatedEvent, nameof(UserVoiceStateUpdated), VoiceServer).ConfigureAwait(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reckon this should probably be moved up a few lines, so that Lavalink implementations can properly intercept the event and throw an exception to stop the event handler before the library opens its own voice connection.

[DebuggerDisplay(@"{DebuggerDisplay,nq}")]
public class SocketVoiceServer
{
public ulong GuildId { get; private set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably be a Cacheable<ulong, IGuild> instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not really sure how I'd go about using the Cacheable class...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are quite a few events in the DiscordSocketClass that use Cacheable<TId,TEntity>, take a look at them for a guideline. Basically gist is you're going to have a bool for if the entity is cached, check the cache for the item, and construct a new Cacheable<,> with the guild id, the cached guild, the isCached boolean, and a Func<> to retrieve the object via REST if it is not cached.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, done

public string Endpoint { get; private set; }
public string Token { get; private set; }

internal SocketVoiceServer(ulong GuildId, string Endpoint, string Token)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameters should be camelCased, and this should be avoided per style consistency with the lib.

@@ -165,6 +165,13 @@ public partial class BaseSocketClient
remove { _userVoiceStateUpdatedEvent.Remove(value); }
}
internal readonly AsyncEvent<Func<SocketUser, SocketVoiceState, SocketVoiceState, Task>> _userVoiceStateUpdatedEvent = new AsyncEvent<Func<SocketUser, SocketVoiceState, SocketVoiceState, Task>>();
/// <summary> Fired when the bot connects/disconnects to a Discord voice server. </summary>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Discord's docs, this event fires only on connecting, not disconnecting, as well as when the current voice instances changes servers.


namespace Discord.WebSocket
{
[DebuggerDisplay(@"{DebuggerDisplay,nq}")]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to implement a function named DebuggerDisplay for attribute to do its job. See SocketRole for an example.

@@ -5,6 +5,7 @@
using Discord.Net.Udp;
using Discord.Net.WebSockets;
using Discord.Rest;
using Discord.WebSocket;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This namespace shouldn't need to be imported given that the DiscordSocketClient class is already defined within it.

@devoxin
Copy link
Contributor Author

devoxin commented Mar 18, 2018

Anything else need changing?

Endpoint = endpoint;
Token = token;
}

private string DebuggerDisplay => $"SocketVoiceServer ({GuildId})";
private string DebuggerDisplay => $"SocketVoiceServer ({Guild.Id})";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will null ref if the guild isn't cached (literally should never happen, but worth pointing out. Opt for using the ID directly from the cacheable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought .Id did return the ID (ulong)?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my bad, misread.

@@ -1464,6 +1464,12 @@ public override RestVoiceRegion GetVoiceRegion(string id)

var data = (payload as JToken).ToObject<VoiceServerUpdateEvent>(_serializer);
var guild = State.GetGuild(data.GuildId);
var cacheable = new Cacheable<IGuild, ulong>(guild, data.GuildId, guild != null,
async () => (IGuild) await ApiClient.GetGuildAsync(data.GuildId));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the cast necessary? Not sure if the compiler would actually complain about that or not, but my first instinct just from reading is that it shouldn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devoxin
Copy link
Contributor Author

devoxin commented Mar 21, 2018

@foxbot?

@foxbot foxbot merged commit e775853 into discord-net:dev May 4, 2018
FiniteReality added a commit to FiniteReality/Discord.Net that referenced this pull request May 4, 2018
- Removed unnecessary parameter in SocketVoiceServer

- Moved SocketVoiceServer into Entities/Voice

- Fixed a bug where trying to download the cached guild would throw

- Fixed a potential bug where Discord might not give us a port when
  connecting to voice
foxbot pushed a commit that referenced this pull request May 4, 2018
- Removed unnecessary parameter in SocketVoiceServer

- Moved SocketVoiceServer into Entities/Voice

- Fixed a bug where trying to download the cached guild would throw

- Fixed a potential bug where Discord might not give us a port when
  connecting to voice
FiniteReality pushed a commit to FiniteReality/Discord.Net that referenced this pull request May 5, 2018
* Expose VoiceServerUpdate events

* Amend based on feedback

* Move this out of guild entity

* Fix namespace issue

* Adjust based on feedback discord-net#2

* Use cacheable instead

* Change based on feedback
FiniteReality added a commit to FiniteReality/Discord.Net that referenced this pull request May 5, 2018
…d-net#1051)

- Removed unnecessary parameter in SocketVoiceServer

- Moved SocketVoiceServer into Entities/Voice

- Fixed a bug where trying to download the cached guild would throw

- Fixed a potential bug where Discord might not give us a port when
  connecting to voice
Still34 pushed a commit to Still34/Discord.Net that referenced this pull request May 21, 2018
commit dece19d
Merge: fdaa689 e764daf
Author: Still Hsu <341464@gmail.com>
Date:   Mon May 21 11:47:47 2018 +0800

    Merge branch 'dev' into docs/faq-n-patches-offline

commit e764daf
Author: Quahu <quahuu@gmail.com>
Date:   Sun May 13 15:34:40 2018 +0200

    Add ViewChannel to Voice channel permissions (discord-net#1059)

    Previously, Voice channels did not have ViewChannel in their "all" permissions

commit 32fc2df
Author: Alex Gravely <tcbskater@hotmail.com>
Date:   Sat May 12 20:47:44 2018 -0400

    Remove unused field in EmbedFieldBuilder. (discord-net#1018)

commit 39dffe8
Author: Finite Reality <FiniteReality@users.noreply.github.com>
Date:   Sun May 13 01:46:07 2018 +0100

    Audit Logs implementation (discord-net#1055)

    * Copy audit logs impl from old branch and clean up

    I suck at using git, so I'm gonna use brute force.

    * Remove unnecessary TODOs

    Category channels do not provide any new information, and the other
    I forgot to remove beforehand

    * Add invite update data, clean up after feedback

    * Remove TODOs, add WebhookType enum for future use

    WebhookType is a future-use type, as currently audit logs are the only
    thing which may return it.

commit fdaa689
Author: Still Hsu <341464@gmail.com>
Date:   Wed May 9 06:04:59 2018 +0800

    Add explanation for RunMode

commit ea82c25
Author: Still Hsu <341464@gmail.com>
Date:   Tue May 8 16:30:48 2018 +0800

    Initial proofread of the articles

commit 124f1a2
Merge: 2555721 97c8931
Author: Still Hsu <341464@gmail.com>
Date:   Tue May 8 05:02:01 2018 +0800

    Merge branch 'dev' into docs/faq-n-patches-offline

commit 97c8931
Author: Still Hsu <341464@gmail.com>
Date:   Mon May 7 06:22:49 2018 +0800

    Implement GetBanAsync (discord-net#1056)

commit 2555721
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 16:11:19 2018 +0800

    Add details to SpotifyGame

commit c7b236d
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 15:58:23 2018 +0800

    Add more IGuild docs

commit 1bb06cc
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 15:40:31 2018 +0800

    Replace all langword placements with code block

commit ac47d84
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 15:22:17 2018 +0800

    Replace langword null to code block null instead

    - Because DocFX sucks at rendering langword

commit 0b15bbc
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 15:20:34 2018 +0800

    Add XML docs

commit 65d4e43
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 06:57:53 2018 +0800

    Add BaseSocketClient docs

commit 8f64c04
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 06:37:55 2018 +0800

    Replace note block

commit d8bb9e7
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 06:31:50 2018 +0800

    Add warning for bulk-delete endpoint

commit adae5ff
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 06:07:28 2018 +0800

    Fix missing Username prop

commit 3e59197
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 06:01:34 2018 +0800

    Add properties examples to overwrite

commit 0ad66f6
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 04:55:15 2018 +0800

    Fix minor consistencies & redundant impl

commit 124efdf
Author: Still Hsu <341464@gmail.com>
Date:   Sun May 6 04:40:14 2018 +0800

    XML Docs

commit 3aa5d36
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 18:22:46 2018 +0800

    Add 'last modified' plugin

    Source: https://github.com/Still34/DocFx.Plugin.LastModified
    Licensed under MIT License

commit 2014870
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 15:57:40 2018 +0800

    Fix letter-casing for files

commit f27d659
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 15:50:00 2018 +0800

    Document exposed TypeReaders

commit 5a824a5
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 15:44:15 2018 +0800

    Add missing exceptions

commit c2de0c0
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 15:40:16 2018 +0800

    Fix seealso for preconditions and add missing descriptions

commit 3a7d7ee
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 15:36:22 2018 +0800

    Minor fixes in documentations
    + Fix unescaped '<'
    + Fix typo

commit 45839bd
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 15:29:47 2018 +0800

    Add XML Docs

commit 9e62546
Merge: aea0678 bb4bb13
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 14:48:13 2018 +0800

    Merge branch 'dev' into docs/faq-n-patches-offline

commit aea0678
Merge: 27dc483 9ddd709
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 13:51:07 2018 +0800

    Merge branch 'docs/faq-n-patches-offline' of https://github.com/Still34/Discord.Net into docs/faq-n-patches-offline

commit 27dc483
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 13:50:27 2018 +0800

    Add XML Docs

commit baa8beb
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 13:49:42 2018 +0800

    Add XML Docs

commit 089f97a
Author: Still Hsu <341464@gmail.com>
Date:   Sat May 5 13:43:04 2018 +0800

    Add details regarding userbot support

commit bb4bb13
Author: Finite Reality <FiniteReality@users.noreply.github.com>
Date:   Fri May 4 11:42:54 2018 +0100

    Fix issues with discord-net#984, remove extraneous whitespace (discord-net#1051)

    - Removed unnecessary parameter in SocketVoiceServer

    - Moved SocketVoiceServer into Entities/Voice

    - Fixed a bug where trying to download the cached guild would throw

    - Fixed a potential bug where Discord might not give us a port when
      connecting to voice

commit 9ddd709
Merge: f197174 7cfed7f
Author: Still Hsu <341464@gmail.com>
Date:   Fri May 4 09:37:04 2018 +0800

    Merge branch 'dev' into docs/faq-n-patches-offline

commit f197174
Author: Still Hsu <341464@gmail.com>
Date:   Fri May 4 09:36:53 2018 +0800

    Fix embed docs consistency

commit 7cfed7f
Author: Alex Gravely <tcbskater@hotmail.com>
Date:   Thu May 3 21:30:13 2018 -0400

    Fix nullref when passing null to GetShardIdFor. (discord-net#1049)

commit e775853
Author: Luke <dev@crimsonxv.pro>
Date:   Fri May 4 02:29:51 2018 +0100

    Expose VoiceServerUpdate events (discord-net#984)

    * Expose VoiceServerUpdate events

    * Amend based on feedback

    * Move this out of guild entity

    * Fix namespace issue

    * Adjust based on feedback #2

    * Use cacheable instead

    * Change based on feedback

commit 157acc4
Author: Still Hsu <341464@gmail.com>
Date:   Thu May 3 23:03:35 2018 +0800

    Add tag examples

commit 57ea571
Author: Still Hsu <341464@gmail.com>
Date:   Thu May 3 22:48:33 2018 +0800

    Fix sample link & add missing pictures

commit 6769c37
Author: Still Hsu <341464@gmail.com>
Date:   Thu May 3 22:39:26 2018 +0800

    Compress some assets & add OAuth2 URL generator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants