Skip to content
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

Allow users to read / write from all components of a given entity using an All WorldQuery types #4278

Closed
alice-i-cecile opened this issue Mar 21, 2022 · 7 comments · Fixed by #6960 or #9419
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible
Milestone

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Mar 21, 2022

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

From time to time, users want to fetch all of the components from a given set of entities.

To do so, they must use the EntityRef or EntityMut API, in an exclusive system.

Fundamentally though, there's no reason this must be done in an exclusive system: this does not impact resource data, and we can limit the archetypes affected when combined with query filters.

What solution would you like?

Create a All type that parallels the API of EntityRef.

This would enable users to write e.g. Query<&All, With<Networked>> in an ordinary system, allowing them to read from any component of the given entity.

This will require some interesting new logic around defining which fetches are needed, but should be relatively straightforward.

Alternatives considered

Initially, I thought we could use EntityRef and EntityMut for this by implementing the WorldQuery trait on them directly. However, EntityMut allows users to add components, which cannot be done in parallel due to the potential impacts on archetypes.

Without the ability to reuse the APIs, IMO it makes more sense just to create a new type and use the standard & vs &mut distinction.

Alternatively alternatively, we could instead use one of the more 🧪 ideas to allow for instant component addition / removals in parallel systems and then just use EntityRef and EntityMut, likely by deferring the archetype updates until the next sync point and storing new component data in a scratch space of sorts.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events labels Mar 21, 2022
@james7132
Copy link
Member

You can sort of do this with ReflectComponent, though this both requires Reflect impls, still requires an exclusive system, and is mostly for the dynamic case where the types are not known at compile time. However, I'm not sure what kind of use case for AllComponents would work where we know the types at compile time that isn't served by generic systems and the existing query type.

@alice-i-cecile
Copy link
Member Author

Agreed, this would be dramatically more useful after we have dynamic components.

@maniwani
Copy link
Contributor

This will require some interesting new logic around defining which fetches are needed, but should be relatively straightforward.

Pretty sure the only thing this query can return is the de-powered version of EntityRef/Mut.

The type system doesn't allow queries to yield a mix of different component fetch tuples.

@mockersf
Copy link
Member

This would enable users to write e.g. Query<&AllComponents, With<Networked>> in an ordinary system, allowing them to read from any component of the given entity.

Could you expand on how that would be used?

fn my_system(
    query: Query<&AllComponents, With<Networked>>
) {
    for all_components in query.iter() {
        // how do you use this all_components?
    }
}

Without going full dynamic, you'll need to provide the concrete component type, so why not put it in the query anyway?

@maniwani
Copy link
Contributor

Maybe we trade concurrency for ergonomics and provide this as a separate SystemParam that just doesn't block resources.

fn my_system(
    mut all: AllComponentsMut,
) {
    for (mut a, b) in all.query::<(&mut A, &B)>() {
        /* ... */
    }
}

@alice-i-cecile alice-i-cecile changed the title Allow users to read / write from all components of a given entity using an AllComponents API Allow users to read / write from all components of a given entity using an All WorldQuery type Mar 21, 2022
@alice-i-cecile
Copy link
Member Author

Maybe we trade concurrency for ergonomics and provide this as a separate SystemParam that just doesn't block resources.

This is better than nothing, but IMO the ability to additionally filter this down based on specific components is an important usability win. In most of the cases I can foresee (networking, saving), you're going to want to restrict this based on the presence or absence of a marker component.

github-merge-queue bot pushed a commit that referenced this issue Jun 22, 2023
# Objective
Partially address #5504. Fix #4278. Provide "whole entity" access in
queries. This can be useful when you don't know at compile time what
you're accessing (i.e. reflection via `ReflectComponent`).

## Solution
Implement `WorldQuery` for `EntityRef`. 

- This provides read-only access to the entire entity, and supports
anything that `EntityRef` can normally do.
- It matches all archetypes and tables and will densely iterate when
possible.
- It marks all of the ArchetypeComponentIds of a matched archetype as
read.
- Adding it to a query will cause it to panic if used in conjunction
with any other mutable access.
 - Expanded the docs on Query to advertise this feature.
 - Added tests to ensure the panics were working as intended.
 - Added `EntityRef` to the ECS prelude.

To make this safe, `EntityRef::world` was removed as it gave potential
`UnsafeCell`-like access to other parts of the `World` including aliased
mutable access to the components it would otherwise read safely.

## Performance
Not great beyond the additional parallelization opportunity over
exclusive systems. The `EntityRef` is fetched from `Entities` like any
other call to `World::entity`, which can be very random access heavy.
This could be simplified if `ArchetypeRow` is available in
`WorldQuery::fetch`'s arguments, but that's likely not something we
should optimize for.

## Future work
An equivalent API where it gives mutable access to all components on a
entity can be done with a scoped version of `EntityMut` where it does
not provide `&mut World` access nor allow for structural changes to the
entity is feasible as well. This could be done as a safe alternative to
exclusive system when structural mutation isn't required or the target
set of entities is scoped.

---

## Changelog
Added: `Access::has_any_write`
Added: `EntityRef` now implements `WorldQuery`. Allows read-only access
to the entire entity, incompatible with any other mutable access, can be
mixed with `With`/`Without` filters for more targeted use.
Added: `EntityRef` to `bevy::ecs::prelude`.
Removed: `EntityRef::world`

## Migration Guide
TODO

---------

Co-authored-by: Carter Weinberg <[email protected]>
Co-authored-by: Jakob Hellermann <[email protected]>
Co-authored-by: Carter Anderson <[email protected]>
@james7132 james7132 reopened this Jun 22, 2023
@james7132
Copy link
Member

This still doesn't give mutable access to Entity components yet. We need a version of EntityMut that doesn't give access to structural mutations first.

@alice-i-cecile alice-i-cecile added this to the 0.12 milestone Jun 22, 2023
@alice-i-cecile alice-i-cecile changed the title Allow users to read / write from all components of a given entity using an All WorldQuery type Allow users to read / write from all components of a given entity using EntityRef and EntityMut WorldQuery types Jun 23, 2023
@alice-i-cecile alice-i-cecile changed the title Allow users to read / write from all components of a given entity using EntityRef and EntityMut WorldQuery types Allow users to read / write from all components of a given entity using an All WorldQuery types Jun 23, 2023
github-merge-queue bot pushed a commit that referenced this issue Aug 29, 2023
# Objective

Fix #4278
Fix #5504
Fix #9422

Provide safe ways to borrow an entire entity, while allowing disjoint
mutable access. `EntityRef` and `EntityMut` are not suitable for this,
since they provide access to the entire world -- they are just helper
types for working with `&World`/`&mut World`.

This has potential uses for reflection and serialization

## Solution

Remove `EntityRef::world`, which allows it to soundly be used within
queries.

`EntityMut` no longer supports structural world mutations, which allows
multiple instances of it to exist for different entities at once.
Structural world mutations are performed using the new type
`EntityWorldMut`.

```rust
fn disjoint_system(
     q2: Query<&mut A>,
     q1: Query<EntityMut, Without<A>>,
) { ... }

let [entity1, entity2] = world.many_entities_mut([id1, id2]);
*entity1.get_mut::<T>().unwrap() = *entity2.get().unwrap();

for entity in world.iter_entities_mut() {
    ...
}
```

---

## Changelog

- Removed `EntityRef::world`, to fix a soundness issue with queries.
+ Removed the ability to structurally mutate the world using
`EntityMut`, which allows it to be used in queries.
+ Added `EntityWorldMut`, which is used to perform structural mutations
that are no longer allowed using `EntityMut`.

## Migration Guide

**Note for maintainers: ensure that the guide for #9604 is updated
accordingly.**

Removed the method `EntityRef::world`, to fix a soundness issue with
queries. If you need access to `&World` while using an `EntityRef`,
consider passing the world as a separate parameter.

`EntityMut` can no longer perform 'structural' world mutations, such as
adding or removing components, or despawning the entity. Additionally,
`EntityMut::world`, `EntityMut::world_mut` , and
`EntityMut::world_scope` have been removed.
Instead, use the newly-added type `EntityWorldMut`, which is a helper
type for working with `&mut World`.

---------

Co-authored-by: Alice Cecile <[email protected]>
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this issue Jan 9, 2024
# Objective

Fix bevyengine#4278
Fix bevyengine#5504
Fix bevyengine#9422

Provide safe ways to borrow an entire entity, while allowing disjoint
mutable access. `EntityRef` and `EntityMut` are not suitable for this,
since they provide access to the entire world -- they are just helper
types for working with `&World`/`&mut World`.

This has potential uses for reflection and serialization

## Solution

Remove `EntityRef::world`, which allows it to soundly be used within
queries.

`EntityMut` no longer supports structural world mutations, which allows
multiple instances of it to exist for different entities at once.
Structural world mutations are performed using the new type
`EntityWorldMut`.

```rust
fn disjoint_system(
     q2: Query<&mut A>,
     q1: Query<EntityMut, Without<A>>,
) { ... }

let [entity1, entity2] = world.many_entities_mut([id1, id2]);
*entity1.get_mut::<T>().unwrap() = *entity2.get().unwrap();

for entity in world.iter_entities_mut() {
    ...
}
```

---

## Changelog

- Removed `EntityRef::world`, to fix a soundness issue with queries.
+ Removed the ability to structurally mutate the world using
`EntityMut`, which allows it to be used in queries.
+ Added `EntityWorldMut`, which is used to perform structural mutations
that are no longer allowed using `EntityMut`.

## Migration Guide

**Note for maintainers: ensure that the guide for bevyengine#9604 is updated
accordingly.**

Removed the method `EntityRef::world`, to fix a soundness issue with
queries. If you need access to `&World` while using an `EntityRef`,
consider passing the world as a separate parameter.

`EntityMut` can no longer perform 'structural' world mutations, such as
adding or removing components, or despawning the entity. Additionally,
`EntityMut::world`, `EntityMut::world_mut` , and
`EntityMut::world_scope` have been removed.
Instead, use the newly-added type `EntityWorldMut`, which is a helper
type for working with `&mut World`.

---------

Co-authored-by: Alice Cecile <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible
Projects
None yet
4 participants