bevyengine / bevy

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

Optional `SystemParam` (especially gizmos) #14949

Open Aceeri opened 2 weeks ago

Aceeri commented 2 weeks ago

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

Gizmos cannot be used optionally in system parameters, which is annoying if you want to configurably not have rendering enabled for a game. E.g. for a headless server.

What solution would you like?

impl<Config, Clear> SystemParam for Option<Gizmos<Config, Clear>> { ... }

Potentially implementing forwarding methods to the underlying methods, but that is a lot of boilerplate... I could do it, just not sure whether this usecase is worth that maintenance burden.

What alternative(s) have you considered?

Removing the gizmos entirely, but that is just kind of sad since it adds a lot of useful information when I am using a non-headless mode.

Additional context

alice-i-cecile commented 2 weeks ago

Ideally we should impl SystemParam for all Option system params in the same way as queries.

Guvante commented 2 weeks ago

WorldQuery has a pretty natural meaning of Option<T> it is "always succeed but if you fail then Item is None". But looking at the API for SystemParam it is less clear how to allow that.

Specifically how do you know when you have a match and need to call into it versus when you don't and cannot.

After all the APIs are infallible from the SystemParam side.

WorldQuery uses the existing matching logic to provide this an all of the existing Option implementations of SystemParam are hand rolled logic implementations. For instance Res panics on missing a resource while Option<Res> just returns None but without panic recovery that transformation is impossible to achieve.

alice-i-cecile commented 2 weeks ago

Hmm, fair enough. I think that adding more special casing is right then.