-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Add an opt-in warning on missed events #7092
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
I like this quite a bit. A few nits, and this is blocked, but I think that this is a good strategy. I think we either: a) need to show how to enable this only in debug mode I prefer b), so you can debug third party events more easily. We also need to benchmark this with the flag turned off (or compare the assembly) to verify there's no perf regression. |
d08017d
to
1fdfee4
Compare
1fdfee4
to
2adff99
Compare
2adff99
to
9635465
Compare
Backlog cleanup: closing due to inactivity and ongoing WIP status, but obviously could be adopted/re-opened. Noting that the dependent PR was merged, and a tracking issue (#7089) already exists. |
Objective
Add an opt-in warning for events that get missed. Addresses #7089. Based on (and requires) #7086.
Solution
Add an associated const on the
Event
trait, which is set with an additional attribute macro. Only emit the warning of that associated const is true.I believe this should have no performance impact for events that do not enable the warning, as the branch should be trivially optimized out.
Notes
Should this be inverted i.e. warn by default, add
#[event(ignore_missed)]
? Should the attribute be formatted in a different way? I did not add the warning to any engine events, although some should probably have them (AssetEvent comes to mind).This implementation only allows for the warning to be enabled/disabled when the event is declared. Events sent by third-party plugins that are intended to be handled by the user are required to make a call on whether or an event must be handled, which can differ depending on the application.
I added the documentation for the attribute to
Events
, as that is where most of the documentation around events currently resides. Should this go onEvent
instead?Changelog
Added
Users can add the
#[event(warn_missed)]
attribute to emit a warning if that event is not handled.