Skip to content

Conversation

@EndBug
Copy link
Member

@EndBug EndBug commented Mar 27, 2021

I noticed that the v12 ShardClientUtil does not contain the ID of the shard you're using. I figured out that the only way to get the shard id is to get a guild and take it from there.

I'm not sure about this though: that strategy relies on the fact that every guild cached will have the same shard ID. Are we sure about that? Is it possible that selecting a random guild will result in the wrong shard ID? Is there a better alternative (like maybe taking the most used shard ID)?

Let me know what you think

@EndBug EndBug added the type: bug Verified problems that need to be worked on label Mar 27, 2021
@EndBug EndBug requested a review from Snazzah March 27, 2021 07:44
@EndBug EndBug self-assigned this Mar 27, 2021
@codecov
Copy link

codecov bot commented Mar 27, 2021

Codecov Report

Merging #184 (e66761e) into master (2be800d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #184   +/-   ##
=======================================
  Coverage   97.67%   97.68%           
=======================================
  Files          45       45           
  Lines        1205     1210    +5     
  Branches      192      194    +2     
=======================================
+ Hits         1177     1182    +5     
  Misses         27       27           
  Partials        1        1           
Impacted Files Coverage Δ
src/Interface/Clients/DiscordJS.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2be800d...e66761e. Read the comment docs.

@Snazzah
Copy link
Member

Snazzah commented Mar 27, 2021

You can read off of client.options.shards (shard ID) and client.options.shardCount.

Actually, since client.options.shards can apparently hold multiple IDs, should probably add a check if the shards option is an array. And just mark the shard as unsupported if there is more than one ID.

@EndBug EndBug requested review from Snazzah and removed request for Snazzah March 28, 2021 12:02
@EndBug
Copy link
Member Author

EndBug commented Mar 28, 2021

I've updated the getter, is it better now?

Copy link
Member

@Snazzah Snazzah left a comment

Choose a reason for hiding this comment

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

After this, LGTM

EndBug and others added 3 commits March 28, 2021 21:12
Co-authored-by: Snazzah <7025343+Snazzah@users.noreply.github.com>
Line 41 was explicitly removing support for manual sharding.
If you're spawning shards manually you're supposed to provide the shard data yourself in the Poster options.
@EndBug EndBug merged commit 69ef071 into master Mar 28, 2021
@EndBug EndBug deleted the djs12-shards branch March 28, 2021 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug Verified problems that need to be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants