pycord
pycord copied to clipboard
feat: event redesign
Summary
This redesigns the event system in discord.Client
according to #189.
Checklist
- [x] 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 - [ ] This PR fixes an issue.
- [x] 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, ...)
I don't really see the enum used anywhere except for passing to an event decorator, so not sure why that's needed
Make sure the
Client
subclasses support this as well, and custom events should still work
Changing the rest of the code to use the enum is a WIP, hence why this PR was opened as a draft.
I don't really see the enum used anywhere except for passing to an event decorator, so not sure why that's needed
Make sure the
Client
subclasses support this as well, and custom events should still workChanging the rest of the code to use the enum is a WIP, hence why this PR was opened as a draft.
Didn't notice it was a draft, apologies
I don’t see why this is needed? Seems useless from my perspective. also replace "according to #189." with "closes #189"
Do you have constructive feedback to provide? Saying "seems useless" is not helpful unless you're able to provide more specifics.
Personally I think this PR is a good idea and will help newcomers to the library dispatch events easier.
I don’t see why this is needed? Seems useless from my perspective. also replace "according to #189." with "closes #189"
Do you have constructive feedback to provide? Saying "seems useless" is not helpful unless you're able to provide more specifics.
Personally I think this PR is a good idea and will help newcomers to the library dispatch events easier.
It only adds an enum, nothing else is changed, it doesn’t even optimize the current systems slowdowns. I honestly think a class-based event system like hikaris would be better, if the current system needs changing at all.
The enum values for events also seem to be undocumented, I find it better for newcomers if these were instead documented, like the current events.
I don’t see why this is needed? Seems useless from my perspective. also replace "according to #189." with "closes #189"
Do you have constructive feedback to provide? Saying "seems useless" is not helpful unless you're able to provide more specifics.
Personally I think this PR is a good idea and will help newcomers to the library dispatch events easier.
It only adds an enum, nothing else is changed, it doesn’t even optimize the current systems slowdowns. I honestly think a class-based event system like hikaris would be better, if the current system needs changing at all.
The enum values for events also seem to be undocumented, I find it better for newcomers if these were instead documented, like the current events.
First of all, about the documentation thing: this PR is still a work in progress. So stuff like documentation haven't been added yet because this isn't entirely finished yet.
Second of all, this does change one thing about the event system according to the original feature request: instead of setting all of the event handlers as literal attributes of the Client, it's just in a dictionary. Sure, it's not a major change, but it is a change for sure.