Skip to content
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

Opt-out (or opt-in) warnings when missing an event #7089

Open
CatThingy opened this issue Jan 4, 2023 · 1 comment
Open

Opt-out (or opt-in) warnings when missing an event #7089

CatThingy opened this issue Jan 4, 2023 · 1 comment
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis A-ECS Entities, components, systems, and events D-Complex Quite challenging from either a design or technical perspective. Ask for help!

Comments

@CatThingy
Copy link
Contributor

What problem does this solve or what need does it fill?

Not handling an event can be a source of hard-to-diagnose issues, when the expectation is that all events are handled. This may occur, for example, when events are read with a system governed by a FixedTimestep run criterion.

Note that all events are necessarily required to be handled. Many events emitted by the engine are of that nature: see #6595. However, events sent from user code usually must be handled to have correct behaviour.

What solution would you like?

Adding the warning back, but with the ability to opt-out (or opt-in) to the warning for missed events.

What alternative(s) have you considered?

Always using ManualEventReader and checking for missed events.

Additional context

Previously introduced in #5730 for all events, removed in #6730 as it raised too many false positives.

@CatThingy CatThingy added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 4, 2023
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events D-Complex Quite challenging from either a design or technical perspective. Ask for help! A-Diagnostics Logging, crash handling, error reporting and performance analysis and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 4, 2023
@alice-i-cecile
Copy link
Member

Agreed that we want this in some form. It needs to be overrideable though, and can't come at a performance cost in release mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis A-ECS Entities, components, systems, and events D-Complex Quite challenging from either a design or technical perspective. Ask for help!
Projects
None yet
Development

No branches or pull requests

2 participants