Skip to content

rework event handling to allow framework to be dispatched concurrently #3324

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 20, 2025
Merged

rework event handling to allow framework to be dispatched concurrently #3324

merged 2 commits into from
Mar 20, 2025

Conversation

@jamesbt365
Copy link
Member

@jamesbt365 jamesbt365 commented Mar 17, 2025 •
edited
Loading

I specifically decided to completely rework the way that users will handle events for a couple reasons:

  1. It was easier for me to do this then spam references on all of the stuff within FullEvent, then provide lifetimes for it
  2. This finally would allow us to add fields to events without having to do a breaking version bump

The main reason for this PR to begin with was that the current pattern would allow commands in poise (which are done in the framework event handler) to block the execution of the regular event handler for an extended period of time (say, you were for example using a paginator, it would block for the entire duration of the command which would take a very long time with a paginator).

This PR should fix that by swapping the event handler to use references and then using tokio::join!.

Also the ratelimit one can't be handled the new way because Context isn't available and making it Option for a single event is wrong imo

@github-actions github-actions bot added gateway Related to the `gateway` module. examples Related to Serenity's examples. labels Mar 17, 2025
@jamesbt365 jamesbt365 added breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users and removed gateway Related to the `gateway` module. examples Related to Serenity's examples. labels Mar 17, 2025
@github-actions github-actions bot added model Related to the `model` module. cache Related to the `cache`-feature. gateway Related to the `gateway` module. examples Related to Serenity's examples. labels Mar 17, 2025
@arqunis arqunis added the enhancement An improvement to Serenity. label Mar 20, 2025
@arqunis arqunis merged commit 9d22019 into serenity-rs:next Mar 20, 2025
24 checks passed
@jamesbt365 jamesbt365 mentioned this pull request Mar 20, 2025
Merged
@mkrasnitski mkrasnitski mentioned this pull request Mar 25, 2025
Closed
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Mar 26, 2025
@jamesbt365 @GnomedDev
Rework the EventHandler trait to have a single dispatch function (s…
95ff55d 
…erenity-rs#3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
GnomedDev pushed a commit to GnomedDev/serenity that referenced this pull request Mar 26, 2025
@jamesbt365 @GnomedDev
Rework the EventHandler trait to have a single dispatch function (s…
fa7a3fb 
…erenity-rs#3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
@jamesbt365 jamesbt365 mentioned this pull request Mar 28, 2025
Closed
GnomedDev pushed a commit that referenced this pull request Apr 28, 2025
@jamesbt365 @GnomedDev
Rework the EventHandler trait to have a single dispatch function (#…
ae66b07 
…3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
GnomedDev pushed a commit that referenced this pull request May 19, 2025
@jamesbt365 @GnomedDev
Rework the EventHandler trait to have a single dispatch function (#…
067a122 
…3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Jun 30, 2025
@jamesbt365 @mkrasnitski
Rework the EventHandler trait to have a single dispatch function (s…
3858c41 
…erenity-rs#3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Jun 30, 2025
@jamesbt365 @mkrasnitski
Rework the EventHandler trait to have a single dispatch function (s…
3e46d2b 
…erenity-rs#3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Jun 30, 2025
@jamesbt365 @mkrasnitski
Rework the EventHandler trait to have a single dispatch function (s…
b8a4ff3 
…erenity-rs#3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Jul 28, 2025
@jamesbt365 @mkrasnitski
Rework the EventHandler trait to have a single dispatch function (s…
c2ce4c5 
…erenity-rs#3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Jul 28, 2025
@jamesbt365 @mkrasnitski
Rework the EventHandler trait to have a single dispatch function (s…
e5bdbd5 
…erenity-rs#3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Oct 7, 2025
@jamesbt365 @mkrasnitski
Rework the EventHandler trait to have a single dispatch function (s…
c095fd5 
…erenity-rs#3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Oct 7, 2025
@jamesbt365 @mkrasnitski
Rework the EventHandler trait to have a single dispatch function (s…
fa3ab01 
…erenity-rs#3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Oct 7, 2025
@jamesbt365 @mkrasnitski
Rework the EventHandler trait to have a single dispatch function (s…
03f82d7 
…erenity-rs#3324)

The primary motivation for this change is to allow for concurrent execution
of the event handler and the framework, but also brings other benefits:

- The trait becomes much simpler, only having three functions now.
- New fields to events will no longer be breaking changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arqunis arqunis arqunis approved these changes

Assignees

No one assigned

Labels

breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users cache Related to the `cache`-feature. enhancement An improvement to Serenity. examples Related to Serenity's examples. gateway Related to the `gateway` module. model Related to the `model` module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jamesbt365 @arqunis