-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Implement WorldQuery
for MainWorld
and RenderWorld
components
#15745
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
Implement WorldQuery
for MainWorld
and RenderWorld
components
#15745
Conversation
Co-authored-by: Trashtalk217 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation is valid, but I agree with @chescock on adjusting the implementation to explicitly forward to the underlying implementation in all cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doc comment should be adjusted to mention the new impl instead.
Objective
#15320 is a particularly painful breaking change, and the new
RenderEntity
in particular is very noisy, with a lot oflet entity = entity.id()
spam.Solution
Implement
WorldQuery
,QueryData
andReadOnlyQueryData
forRenderEntity
andWorldEntity
.These work the same as the
Entity
impls from a user-facing perspective: they simply return an owned (copied)Entity
identifier. This dramatically reduces noise and eases migration.Under the hood, these impls defer to the implementations for
&T
for everything other than the "call .id() for the user" bit, as they involve read-only access to component data. Doing it this way (as opposed to implementing a custom fetch, as tried in the first commit) dramatically reduces the maintenance risk of complex unsafe code outside ofbevy_ecs
.To make this easier (and encourage users to do this themselves!), I've made
ReadFetch
andWriteFetch
slightly more public: they're no longerdoc(hidden)
. This is a good change, since trying to vendor the logic is much worse than just deferring to the existing tested impls.Testing
I've run a handful of rendering examples (breakout, alien_cake_addict, auto_exposure, fog_volumes, box_shadow) and nothing broke.
Follow-up
We should lint for the uses of
&RenderEntity
and&MainEntity
in queries: this is just less nice for no reason.