GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — rework event handling and rework how extensions are set up by akiraveliara · Pull Request #2045 · DSharpPlus/DSharpPlus

Conversation

@akiraveliara

@akiraveliara akiraveliara commented Jul 27, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • removes corelib dependence on AsyncEvent`2 for dispatching events and instead replaces it with a type-agnostic dispatcher assuming immutable events (removing most of the cruft from AsyncEvent`2 is also nice for perf)
  • enables event handlers expressed as types by implementing IEventHandler<TEventArgs>. these can be registered using EventHandlingBuilder.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.
  • fixes a memory leak in Interactivity
    • interactivity generally is rather janky and remains rather janky here
  • changes every extension setup to be along the lines of IServiceCollection.AddExtension(Action<Extension> setup, ExtensionConfiguration);, with matching methods on DiscordClientBuilder, following an UseExtension scheme according to past convention (open to changing this).
    • the setup parameter fills the need for calling additional configuration on the extension, so for example the following code would be refactored as follows
    • in the future, extensions fully supported throughout the v5 lifecycle should add first-class support without needing a setup parameter
// old
DiscordClientBuilder builder = // we get a client here;
DiscordClient client = builder.Build();

CommandsExtension extension = client.UseCommands(myConfig);
extension.AddCommands([typeof(Command)]);
extension.AddConverters(...);

await client.ConnectAsync();
// new
DiscordClientBuilder builder = // we get a client here;
builder.UseCommands(extension => 
    {
        extension.AddCommands([typeof(MyCommand)]);
        extension.AddConverters(...);
    },
    myConfig
);

await builder.ConnectAsync();

closes #1820 as complete

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

OoLunar reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
akiraveliara added 11 commits July 17, 2024 20:00
@akiraveliara
implement canonicalization of events and rewire Dispatch
b91d035
@akiraveliara
this is miserable
7d85de6
@akiraveliara
Merge branch 'master' into aki/new-events
d656aef
@akiraveliara
remove legacy event hooks
171ba59
@akiraveliara
type safety is an obstacle /s
e0ce0d9
@akiraveliara
merge conflicts
b60e53f
@akiraveliara
CommandsNext
5a9e8a6
@akiraveliara
VoiceNext
1a008d3
@akiraveliara
SlashCommands
3b86d2e
@akiraveliara
lavalink is dead, so just make sure it compiles
0d2e107
@akiraveliara
actually remove the old event handling code
90336cd
@akiraveliara akiraveliara added enhancement commands-next For issues related to DSharpPlus.CommandsNext obsolete: voicenext interactivity core big-change obsolete: lavalink obsolete: slash-commands commands For issues related to DSharpPlus.Commands epic-ioc for pull requests and issues relating to IOC in v5 labels Jul 27, 2024
@akiraveliara akiraveliara added this to the v5.0 milestone Jul 27, 2024
@akiraveliara akiraveliara removed the obsolete: lavalink label Jul 27, 2024
@akiraveliara akiraveliara mentioned this pull request Jul 27, 2024
v5 mega-issue #1580
Open
19 of 31 tasks
akiraveliara added 3 commits July 30, 2024 22:35
@akiraveliara
interactivity support
11c6782
@akiraveliara
merge
90e83e8
@akiraveliara
update tests
3a2c5ac
@akiraveliara
akiraveliara marked this pull request as ready for review July 30, 2024 20:44
akiraveliara added 2 commits July 30, 2024 22:54
@akiraveliara
delete legacy BaseExtension
7cfabb3
@akiraveliara
internal convenience for interactivity
299eb94
Naamloos
Naamloos approved these changes Jul 31, 2024
View reviewed changes

@Naamloos Naamloos left a comment

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread DSharpPlus.Commands/Processors/SlashCommands/SlashCommandProcessor.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread DSharpPlus.CommandsNext/CommandsNextExtension.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread DSharpPlus.Interactivity/EventHandling/EventWaiter.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread DSharpPlus.Tests/Commands/CommandFiltering/Tests.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread DSharpPlus/Clients/DefaultEventDispatcher.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread DSharpPlus/IEventHandler.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread obsolete/DSharpPlus.Lavalink/DiscordClientExtensions.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

akiraveliara added 7 commits July 31, 2024 17:28
@akiraveliara
make the extensions disposable
154a3f5
@akiraveliara
merge conflicts
8334d8c
@akiraveliara
shaking my head
5d1fbf0
@akiraveliara
fix the return type
e0650b3
@akiraveliara
Merge branch 'master' into aki/new-events
cfb0782
@akiraveliara
this was solved differently in master
9dc8c22
@akiraveliara
update docs
3d8eb74
@akiraveliara

akiraveliara commented Aug 5, 2024

Copy link
Copy Markdown
Member Author

tested, documented, should be rtm as far as i'm concerned

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Plerx2493
Plerx2493 approved these changes Aug 11, 2024
View reviewed changes
VelvetToroyashi
VelvetToroyashi reviewed Aug 11, 2024
View reviewed changes

@VelvetToroyashi VelvetToroyashi left a comment

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread DSharpPlus.Tests/Commands/CommandFiltering/Tests.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread DSharpPlus/Clients/EventHandlerCollection.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@akiraveliara
pr feedback
2de98d6
VelvetToroyashi
VelvetToroyashi approved these changes Aug 11, 2024
View reviewed changes

@VelvetToroyashi VelvetToroyashi left a comment

Copy link
Copy Markdown
Member

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.

:shipit:

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@akiraveliara

akiraveliara commented Aug 11, 2024

Copy link
Copy Markdown
Member Author

:shipit:

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@akiraveliara
akiraveliara merged commit e810e2c into master Aug 11, 2024
@akiraveliara
akiraveliara deleted the aki/new-events branch August 11, 2024 17:27
@akiraveliara akiraveliara mentioned this pull request Aug 11, 2024
v5 breaking changes tracking issue #1585
Open
OoLunar pushed a commit that referenced this pull request Oct 3, 2024
@akiraveliara
rework event handling and rework how extensions are set up (#2045)
edf9290
* 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
Instellate pushed a commit that referenced this pull request Nov 4, 2024
@akiraveliara @Instellate
rework event handling and rework how extensions are set up (#2045)
340ca32
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Naamloos Naamloos Naamloos approved these changes
@VelvetToroyashi VelvetToroyashi VelvetToroyashi approved these changes
@Plerx2493 Plerx2493 Plerx2493 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

big-change commands For issues related to DSharpPlus.Commands commands-next For issues related to DSharpPlus.CommandsNext core enhancement epic-ioc for pull requests and issues relating to IOC in v5 interactivity obsolete: slash-commands obsolete: voicenext

Projects

None yet

Milestone

v5.0

Development

Successfully merging this pull request may close these issues.

v5 extensions and events design

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@akiraveliara @Naamloos @VelvetToroyashi @Plerx2493
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.