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

gateway: make IdentifyData's intents field an optional.Uint #275

Merged
merged 1 commit into from
Sep 10, 2021

Conversation

samhza
Copy link
Contributor

@samhza samhza commented Sep 8, 2021

https://discord.com/developers/docs/topics/gateway#identify
The intents field is required, and 0 is a valid value for intents. Currently if you try to connect with intents = 0, you get an error from Discord because the Identify would be sent without an intents field.

@diamondburned
Copy link
Owner

This breaks if the client connecting is a user account.

Using an optional int would be better.

@samhza
Copy link
Contributor Author

samhza commented Sep 9, 2021

So should I make the Intents field an option.Uint? What should be done about type Intents

@diamondburned
Copy link
Owner

So should I make the Intents field an option.Uint?

That could work. The user could use the AddIntents() method that initializes the field if it's not already. AddIntents(0) would do what you want.

@samhza samhza changed the title gateway: don't omit 0 intents field in IdentifyData gateway: make IdentifyData's intents field an optional.Uint Sep 9, 2021
@samhza
Copy link
Contributor Author

samhza commented Sep 9, 2021

made the requested changes

@diamondburned
Copy link
Owner

On second thought, it would be a better idea to have a type Intents option.Uint instead of changing the field's type and not using the previous
type anymore.

Also, the CI is failing.

@samhza
Copy link
Contributor Author

samhza commented Sep 10, 2021

I've fixed the tests, CI is now passing

@diamondburned diamondburned merged commit 4023a58 into diamondburned:v3 Sep 10, 2021
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.

None yet

2 participants