bevyengine / bevy

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

`Res`-style system params equivalents for `Query::single` #15264

Open MiniaczQ opened 1 day ago

MiniaczQ commented 1 day ago

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

We should support entities that act as resources. For that, we should provide system parameters that perform a query, while expecting only a single matching entity.

What solution would you like?

Introduce the following system params:

Option<_>s should fail if there were multiple matching entities, but return None if no entity matched.

What alternative(s) have you considered?

Making a system param that wraps Query is possible, but it requires an accessor that defers the errors from system param gathering to call of the system.

MiniaczQ commented 1 day ago

I'll be happy to implement it if someone gives me some directions

alice-i-cecile commented 1 day ago

So I quite like this idea from an ergonomics standpoint, and would use it in my own personal projects.

I'm loathe to expose more panicking APIs though, see #14275. I'd be interested in seeing Single return an Option. I'd also be nervous to use this in learning materials other than that dedicated to this concept, since teaching users a single blessed tool is really helpful conceptually.

MiniaczQ commented 1 day ago

Until #14275 gets resolved I feel like we should stick to the existing convention. The scope is already massive, one more feature won't make a dent in the breaking changes

alice-i-cecile commented 1 day ago

I'm fine with that. I still think that this suggestion is controversial, independent of #14275, because of the impact on new learners / style due to introducing a redundant method to do the same thing.

janhohenheim commented 1 day ago

@alice-i-cecile imo we should not introduce more Options, but only use Results where possible in new API. Otherwise we will have to change it again once we an error handling overhaul happens.

MiniaczQ commented 1 day ago

@alice-i-cecile imo we should not introduce more Options, but only use Results where possible in new API. Otherwise we will have to change it again once we an error handling overhaul happens.

This is exactly the problem I have with #14275, keep the discussion there and unless there is a consensus for dealing with this, don't bother other issues, it's very disruptive.

janhohenheim commented 1 day ago

@MiniaczQ alright, sorry!

alice-i-cecile commented 1 day ago

I'm happy to consider this idea unblocked due to the ideas in #15625. Once that's fully implemented, this should be recommended as the default way to work with singleton queries and used consistently through our examples.