-
-
Notifications
You must be signed in to change notification settings - Fork 315
rework event handling and rework how extensions are set up #2045
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM. Some discussion points but nothing major. Ping me in the server if you want to discuss any.
Seems ready for testing, at least.
tested, documented, should be rtm as far as i'm concerned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good! Some small things I caught at a glance, but this is definitely a huge step forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
* implement canonicalization of events and rewire Dispatch * this is miserable * remove legacy event hooks * type safety is an obstacle /s * CommandsNext * VoiceNext * SlashCommands * lavalink is dead, so just make sure it compiles * actually remove the old event handling code * interactivity support * update tests * delete legacy BaseExtension * internal convenience for interactivity * make the extensions disposable * shaking my head * fix the return type * this was solved differently in master * update docs * pr feedback
* implement canonicalization of events and rewire Dispatch * this is miserable * remove legacy event hooks * type safety is an obstacle /s * CommandsNext * VoiceNext * SlashCommands * lavalink is dead, so just make sure it compiles * actually remove the old event handling code * interactivity support * update tests * delete legacy BaseExtension * internal convenience for interactivity * make the extensions disposable * shaking my head * fix the return type * this was solved differently in master * update docs * pr feedback
IEventHandler<TEventArgs>
. these can be registered usingEventHandlingBuilder.AddEventHandlers<T>(ServiceLifetime)
; a type can contain an indefinite amount of handlers and the type will have a respected service lifetime according to the parameter.IServiceCollection.AddExtension(Action<Extension> setup, ExtensionConfiguration);
, with matching methods onDiscordClientBuilder
, following anUseExtension
scheme according to past convention (open to changing this).setup
parameter fills the need for calling additional configuration on the extension, so for example the following code would be refactored as followssetup
parametercloses #1820 as complete