bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.09k stars 3.56k forks source link

Log when an event was triggered but no observer consumed it #15921

Open ItsDoot opened 3 weeks ago

ItsDoot commented 3 weeks ago

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

We can easily see where an event was triggered from, but not that it didn't cause any observers to run. We should add a (feature flagged) debug message for when an event is triggered, but no observer consumes it.

What solution would you like?

Log when an event is not observed by anything. This should be behind a non-default feature flag. We should also consider adding an umbrella "debug" feature flags for all our various ones.

mockersf commented 2 weeks ago

I think this feature is controversial, because it's expected that Bevy or third party plugins trigger events, and not all of them will have observers plugged. Events are one of the extension points, and won't be always used and it's perfectly normal

BenjaminBrienen commented 6 days ago

I like this idea because it allows a developer to opt-in to pedantic, but potentially useful messages. It kind of reminds me of a lint. "Maybe you forgot to use this?"

I'll put this on my to-do.

mockersf commented 6 days ago

I'm interested in PoC-ing a "warning as entities" approach for this

The basic idea is:

[derive(Enum)]

enum Cause { UnconsumedEvent(type_id) }


* Instead of logging a warning, spawn a new entity. The system can check if there's already a warning for the target entity/cause

This will be easily reusable by the editor to show these warnings.

We can also have a `log_on_warning` system that would log newly spawned/changed `Warning` components

In my pseudo-code the cause is an enum, but it should be possible for third party plugins to extend it so that they can benefit from the same integrations. So probably a `Box<dyn Something>`

@alice-i-cecile do you have opinions on this?
alice-i-cecile commented 6 days ago

Since this is entity-targeted, it feels like this makes sense as a Warning<Cause: Error> sparse set component on the entity that triggered it? It seems much easier to check for pre-existing warnings that way, and also much more discoverable in inspectors.