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

feat: 🧑‍💻 Make Client.intents writable #2687

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Paillat-dev
Copy link
Contributor

@Paillat-dev Paillat-dev commented Jan 7, 2025

Summary

I have no idea why intents were read-only in the first place. Maybe they should stay so, or maybe smth different should be done about it.

This PR allows to edit intents of a Client as long as the client wasn't connected yet.

Information

  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed).
  • This PR is not a code change (e.g. documentation, README, typehinting,
    examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.
  • I have updated the changelog to include these changes.

@Paillat-dev Paillat-dev requested a review from a team as a code owner January 7, 2025 18:57
@pullapprove4 pullapprove4 bot requested a review from BobDotCom January 7, 2025 18:58
@Paillat-dev Paillat-dev changed the title 🧑‍💻 Make Client.intents writable feat: 🧑‍💻 Make Client.intents writable Jan 7, 2025
Copy link
Member

@BobDotCom BobDotCom left a comment

Choose a reason for hiding this comment

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

I believe intents are read-only because they shouldn't be changed after initialization. Once the client connects, intents shouldn't change

@Paillat-dev
Copy link
Contributor Author

I'm going to add some kind of check to see if the connection state is connected or not and act accordingly to allow or disallow the setting.

@Lulalaby
Copy link
Member

Lulalaby commented Jan 7, 2025

I believe intents are read-only because they shouldn't be changed after initialization. Once the client connects, intents shouldn't change

Yeah that's the reason
If you'd find any way to change it after connect, discord would throw you out the connection

@Dorukyum
Copy link
Member

Dorukyum commented Jan 7, 2025

The issue is something else entirely. This pull request adda a setter for intents attributes, the issue resides by the attributes of the discord.Intents class itself.

@Paillat-dev
Copy link
Contributor Author

Yup I missed that. Let me rewrite all this a bit and come back

@Paillat-dev
Copy link
Contributor Author

There we go

@Paillat-dev Paillat-dev force-pushed the feat/intents-writable branch from c743b49 to 4829614 Compare January 7, 2025 21:20
@Lulalaby Lulalaby requested review from Dorukyum and BobDotCom January 7, 2025 21:43
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.

4 participants