bevyengine / bevy

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

`Option` and `Has` ignore sparse components in transmuted dense queries #16397

Open chescock opened 2 hours ago

chescock commented 2 hours ago

Bevy version

e155fe1d86

What you did

Transmuted a QueryState<EntityMut> to either QueryState<Option<&S>> or QueryState<Has<S>>, where S is a sparse set component.

#[derive(Component)]
#[component(storage = "SparseSet")]
struct S;

let mut world = World::new();
world.spawn(S);

let mut query = world
    .query::<EntityMut>()
    .transmute::<(Option<&S>, Has<S>)>(&world);
let (option, has) = query.single(&world);

assert!(option.is_some());
assert!(has);

What went wrong

The matched entity has an S component, but Option<&S> yields None and Has<S> yields false.

Additional information

Option and Has cache whether the component exists in set_table or set_archetype. To make this work for sparse set components, they set IS_DENSE = false and require sparse iteration. But EntityMut performs dense iteration, so the transmuted query is dense and calls set_table. Option and Has don't find the component in the table, since sparse set components are not stored in tables, and return None or false for the entire table.

14628 is also caused by transmuting a dense query to a sparse one.

16396 would make this also occur in cases involving FilteredEnittyMut.

chescock commented 2 hours ago

I see two basic ways to fix this: either prevent dense to sparse transmutes, or make Option and Has always work on dense queries.

Preventing dense to sparse transmutes could be done by having QueryState::transmute_filtered do assert!(!self.is_dense || (NewD::IS_DENSE && NewF::IS_DENSE)) alongside the other assert. To support cases where those transmutes are needed, we could provide ways to force the original query to be sparse, like a QueryBuilder::force_sparse() method and a ForceSparse QueryFilter struct. That would also resolve #14628.

Making Has work on dense queries isn't too bad. You can store the &ComponentSparseSet in the Fetch when working with a sparse set component and call contains() on each entity. You could even store an Option<bool> alongside it to cache the result when the query winds up being sparse anyway. Making Option work is similar, but requires a lot more code since you have to add plumbing to QueryData.

Making Has and Option always be dense might even improve performance, if the better iteration order offsets the per-entity lookups!

I'm happy to make PRs for any of these ideas if those designs sound reasonable!