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

Add BaseSocketClient object. #773

Merged
merged 12 commits into from Sep 27, 2017
Merged

Conversation

AntiTcb
Copy link
Collaborator

@AntiTcb AntiTcb commented Jul 27, 2017

Breaking changes:

- Removed leftover DiscordShardedClient.UserPresenceUpdated event.
(Will be done on next major release)

Additions:

  • Add BaseSocketClient object.
  • Add ShardConnected, ShardDisconnected, ShardReady, and ShardLatencyUpdated events to DiscordShardedClient.
    Minor:
    -- Added optional RequestOptions argument that was missing from various methods.
    -- Added docstrings for all events.

There is no currently existing bridge between DiscordSocketClient and DiscordShardedClient, leaving users to duplicate all websocket client functionality between the two if they wish to abstract code that should be shared to a BCL, or for addon developers that wish to support both client types out of the box.

While it makes logical sense that this wasn't implemented previously as DiscordShardedClient isn't technically a client itself, but a wrapper for multiple DiscordSocketClients, that comes at an unfortunate expense of usability for accessing events or methods only supported via websockets.

@@ -67,7 +67,7 @@ internal override void Dispose(bool disposing)

public Task StartAsync() => _connection.StartAsync();
public Task StopAsync() => _connection.StopAsync();

Copy link
Contributor

Choose a reason for hiding this comment

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

Wat.

(Same goes for the other line in this file.)

@Joe4evr
Copy link
Contributor

Joe4evr commented Jul 28, 2017

Remove all that superfluous whitespace (Ctrl+E, S in VS to toggle its visibility).

@foxbot
Copy link
Member

foxbot commented Jul 28, 2017

Looks good, but maybe we can leave DiscordShardedClient.UserPresenceUpdated intact for now, so that this can end up being a 1.x addition?

@AntiTcb
Copy link
Collaborator Author

AntiTcb commented Jul 29, 2017

Sounds good to me.

@foxbot foxbot merged commit 9b7afec into discord-net:dev Sep 27, 2017
@AntiTcb AntiTcb deleted the BaseSocketClient branch September 28, 2017 09:29
@foxbot foxbot removed this from Working Release (2.0) in Library Development Nov 22, 2017
FiniteReality pushed a commit to FiniteReality/Discord.Net that referenced this pull request May 5, 2018
* Add BaseDiscordClient. Add various missing RequestOptions args.

DiscordSocketClient and DiscordShardedClient's shared members now exist in this abstract class.

* Add ShardReady event.

* Style consistency. Remove extraneous overloads.

Remove extraneous overloads.

* Add BaseSocketClient#DownloadUsersAsync().

Style cleanups.

* Add ShardLatencyUpdated event.

Style cleanup.

* Hook LatencyUpdated for ShardedClient.

* Begone whitespace.

* I'm good at this, I swear. >_>

* Add back DiscordShardedClient.UserPresenceUpdated

* Add ObsoleteAttribute

* Removing the UserPresenceUpdated event.
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.

None yet

3 participants