Commit graph

17 commits

Author SHA1 Message Date
Boxy
407c080e59 Replace ReadOnlyFetch with ReadOnlyWorldQuery (#4626)
# Objective

- Fix a type inference regression introduced by #3001
- Make read only bounds on world queries more user friendly

ptrification required you to write `Q::Fetch: ReadOnlyFetch` as `for<'w> QueryFetch<'w, Q>: ReadOnlyFetch` which has the same type inference problem as `for<'w> QueryFetch<'w, Q>: FilterFetch<'w>` had, i.e. the following code would error:
```rust
#[derive(Component)]
struct Foo;

fn bar(a: Query<(&Foo, Without<Foo>)>) {
    foo(a);
}

fn foo<Q: WorldQuery>(a: Query<Q, ()>)
where
    for<'w> QueryFetch<'w, Q>: ReadOnlyFetch,
{
}
```
`for<..>` bounds are also rather user unfriendly..

## Solution

Remove the `ReadOnlyFetch` trait in favour of a `ReadOnlyWorldQuery` trait, and remove `WorldQueryGats::ReadOnlyFetch` in favor of `WorldQuery::ReadOnly` allowing the previous code snippet to be written as:
```rust
#[derive(Component)]
struct Foo;

fn bar(a: Query<(&Foo, Without<Foo>)>) {
    foo(a);
}

fn foo<Q: ReadOnlyWorldQuery>(a: Query<Q, ()>) {}
``` 
This avoids the `for<...>` bound which makes the code simpler and also fixes the type inference issue.

The reason for moving the two functions out of `FetchState` and into `WorldQuery` is to allow the world query `&mut T` to share a `State` with the `&T` world query so that it can have `type ReadOnly = &T`. Presumably it would be possible to instead have a `ReadOnlyRefMut<T>` world query and then do `type ReadOnly = ReadOnlyRefMut<T>` much like how (before this PR) we had a `ReadOnlyWriteFetch<T>`. A side benefit of the current solution in this PR is that it will likely make it easier in the future to support an API such as `Query<&mut T> -> Query<&T>`. The primary benefit IMO is just that `ReadOnlyRefMut<T>` and its associated fetch would have to reimplement all of the logic that the `&T` world query impl does but this solution avoids that :)

---

## Changelog/Migration Guide

The trait `ReadOnlyFetch` has been replaced with `ReadOnlyWorldQuery` along with the `WorldQueryGats::ReadOnlyFetch` assoc type which has been replaced with `<WorldQuery::ReadOnly as WorldQueryGats>::Fetch`
- Any where clauses such as `QueryFetch<Q>: ReadOnlyFetch` should be replaced with `Q: ReadOnlyWorldQuery`.
- Any custom world query impls should implement `ReadOnlyWorldQuery` insead of `ReadOnlyFetch`

Functions `update_component_access` and `update_archetype_component_access` have been moved from the `FetchState` trait to `WorldQuery`
- Any callers should now call `Q::update_component_access(state` instead of `state.update_component_access` (and `update_archetype_component_access` respectively)
- Any custom world query impls should move the functions from the `FetchState` impl to `WorldQuery` impl

`WorldQuery` has been made an `unsafe trait`, `FetchState` has been made a safe `trait`. (I think this is how it should have always been, but regardless this is _definitely_ necessary now that the two functions have been moved to `WorldQuery`)
- If you have a custom `FetchState` impl make it a normal `impl` instead of `unsafe impl`
- If you have a custom `WorldQuery` impl make it an `unsafe impl`, if your code was sound before it is going to still be sound
2022-06-13 23:35:54 +00:00
ira
92ddfe8ad4 Add methods for querying lists of entities. (#4879)
# Objective
Improve querying ergonomics around collections and iterators of entities.

Example how queries over Children might be done currently. 
```rust
fn system(foo_query: Query<(&Foo, &Children)>, bar_query: Query<(&Bar, &Children)>) {
    for (foo, children) in &foo_query {
        for child in children.iter() {
            if let Ok((bar, children)) = bar_query.get(*child) {
                for child in children.iter() {
                    if let Ok((foo, children)) = foo_query.get(*child) {
                        // D:
                    }
                }
            }
        }
    }
}
```
Answers #4868
Partially addresses #4864
Fixes #1470
## Solution
Based on the great work by @deontologician in #2563 

Added `iter_many` and `many_for_each_mut` to `Query`.
These take a list of entities (Anything that implements `IntoIterator<Item: Borrow<Entity>>`).

`iter_many` returns a `QueryManyIter` iterator over immutable results of a query (mutable data will be cast to an immutable form).

`many_for_each_mut` calls a closure for every result of the query, ensuring not aliased mutability. 
This iterator goes over the list of entities in order and returns the result from the query for it. Skipping over any entities that don't match the query.

Also added `unsafe fn iter_many_unsafe`.

### Examples
```rust
#[derive(Component)]
struct Counter {
    value: i32
}

#[derive(Component)]
struct Friends {
    list: Vec<Entity>,
}

fn system(
    friends_query: Query<&Friends>,
    mut counter_query: Query<&mut Counter>,
) {
    for friends in &friends_query {
        for counter in counter_query.iter_many(&friends.list) {
            println!("Friend's counter: {:?}", counter.value);
        }
        
        counter_query.many_for_each_mut(&friends.list, |mut counter| {
            counter.value += 1;
            println!("Friend's counter: {:?}", counter.value);
        });
    }
}

```

Here's how example in the Objective section can be written with this PR.
```rust
fn system(foo_query: Query<(&Foo, &Children)>, bar_query: Query<(&Bar, &Children)>) {
    for (foo, children) in &foo_query {
        for (bar, children) in bar_query.iter_many(children) {
            for (foo, children) in foo_query.iter_many(children) {
                // :D
            }
        }
    }
}
```
## Additional changes
Implemented `IntoIterator` for `&Children` because why not.
## Todo
- Bikeshed!

Co-authored-by: deontologician <deontologician@gmail.com>

Co-authored-by: devil-ira <justthecooldude@gmail.com>
2022-06-06 16:09:16 +00:00
Yutao Yuan
c4080c6832 Fix release workflow (#4903)
# Objective

While playing with the code, I found some problems in the recently merged version-bumping workflow:
- Most importantly, now that we are using `0.8.0-dev` in development, the workflow will try to bump it to `0.9.0` 😭 
- The crate filter is outdated now that we have more crates in `tools`.
- We are using `bevy@users.noreply.github.com`, but according to [Github help](https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/setting-your-commit-email-address#about-commit-email-addresses), that email address means "old no-reply email format for the user `bevy`". It is currently not associated with any account, but I feel this is still not appropriate here.

## Solution

- Create a new workflow, `Post-release version bump`, that should be run after a release and bumps version from `0.X.0` to `0.X+1.0-dev`. Unfortunately, cargo-release doesn't have a builtin way to do this, so we need to parse and increment the version manually.
- Add the new crates in `tools` to exclusion list. Also removes the dependency version specifier from `bevy_ecs_compile_fail_tests`. It is not in the workspace so the dependency version will not get automatically updated by cargo-release.
- Change the author email to `41898282+github-actions[bot]@users.noreply.github.com`. According to the discussion [here](https://github.com/actions/checkout/issues/13#issuecomment-724415212) and [here](https://github.community/t/github-actions-bot-email-address/17204/6), this is the email address associated with the github-actions bot account.
- Also add the workflows to our release checklist.

See infmagic2047#5 and infmagic2047#6 for examples of release and post-release PRs.
2022-06-06 15:47:51 +00:00
Daniel McNab
38a940dbbe Make derived SystemParam readonly if possible (#4650)
Required for https://github.com/bevyengine/bevy/pull/4402.

# Objective

- derived `SystemParam` implementations were never `ReadOnlySystemParamFetch`
- We want them to be, e.g. for `EventReader`

## Solution

- If possible, 'forward' the impl of `ReadOnlySystemParamFetch`.
2022-05-09 16:09:33 +00:00
Boxy
b9102b8836 Introduce tests for derive(WorldQuery) (#4625)
The only tests we had for `derive(WorldQuery)` checked that the derive doesnt panic/emit a `compiler_error!`. This PR adds tests that actually assert the returned values of a query using the derived `WorldQuery` impl. Also adds a compile fail test to check that we correctly error on read only world queries containing mutable world queries.
2022-04-28 21:06:20 +00:00
Yutao Yuan
8d67832dfa Bump Bevy to 0.8.0-dev (#4505)
# Objective

We should bump our version to 0.8.0-dev after releasing 0.7.0, according to our release checklist.

## Solution

Do it.
2022-04-17 23:04:52 +00:00
Carter Anderson
83c6ffb73c release 0.7.0 (#4487) 2022-04-15 18:05:37 +00:00
Boxy
dba7790012 REMOVE unsound lifetime annotations on EntityMut (#4096)
Fixes #3408
#3001 also solves this but I dont see it getting merged any time soon so...
# Objective
make bevy ecs a lil bit less unsound

## Solution
make `EntityMut::get_component_mut` return borrows from self instead of `'w`
2022-04-04 21:33:33 +00:00
Alice Cecile
b33dae31ec Rename get_multiple APIs to get_many (#4384)
# Objective

-  std's new APIs do the same thing as `Query::get_multiple_mut`, but are called `get_many`: https://github.com/rust-lang/rust/pull/83608

## Solution

- Find and replace `get_multiple` with `get_many`
2022-03-31 20:59:26 +00:00
Alice Cecile
509548190b Add get_multiple and get_multiple_mut APIs for Query and QueryState (#4298)
# Objective

- The inability to have multiple active mutable borrows into a query is a common source of borrow-checker pain for users.
- This is a pointless restriction if and only if we can guarantee that the entities they are accessing are unique.
- This could already by bypassed with get_unchecked, but that is an extremely unsafe API.
- Closes https://github.com/bevyengine/bevy/issues/2042.

## Solution

- Add `get_multiple`, `get_multiple_mut` and their unchecked equivalents (`multiple` and `multiple_mut`) to `Query` and `QueryState`.
- Improve the `QueryEntityError` type to provide more useful error information.

## Changelog

- Added `get_multiple`, `get_multiple_mut` and their unchecked equivalents (`multiple` and `multiple_mut`) to Query and QueryState.

## Migration Guide

- The `QueryEntityError` enum now has a `AliasedMutability variant, and returns the offending entity.

## Context

This is a fresh attempt at #3333; rebasing was behaving very badly and it was important to rebase on top of the recent query soundness fixes. Many thanks to all the reviewers in that thread, especially @BoxyUwU for the help with lifetimes.

## To-do

- [x] Add compile fail tests
- [x] Successfully deduplicate code
- [x] Decide what to do about failing doc tests
- [x] Get some reviews for lifetime soundness
2022-03-30 19:16:48 +00:00
bilsen
63fee2572b ParamSet for conflicting SystemParam:s (#2765)
# Objective
Add a system parameter `ParamSet` to be used as container for conflicting parameters.

## Solution
Added two methods to the SystemParamState trait, which gives the access used by the parameter. Did the implementation. Added some convenience methods to FilteredAccessSet. Changed `get_conflicts` to return every conflicting component instead of breaking on the first conflicting `FilteredAccess`.


Co-authored-by: bilsen <40690317+bilsen@users.noreply.github.com>
2022-03-29 23:39:38 +00:00
Boxy
024d98457c yeet unsound lifetime annotations on Query methods (#4243)
# Objective
Continuation of #2964 (I really should have checked other methods when I made that PR)

yeet unsound lifetime annotations on `Query` methods.
Example unsoundness:
```rust
use bevy::prelude::*;

fn main() {
    App::new().add_startup_system(bar).add_system(foo).run();
}

pub fn bar(mut cmds: Commands) {
    let e = cmds.spawn().insert(Foo { a: 10 }).id();
    cmds.insert_resource(e);
}

#[derive(Component, Debug, PartialEq, Eq)]
pub struct Foo {
    a: u32,
}
pub fn foo(mut query: Query<&mut Foo>, e: Res<Entity>) {
    dbg!("hi");
    {
        let data: &Foo = query.get(*e).unwrap();
        let data2: Mut<Foo> = query.get_mut(*e).unwrap();
        assert_eq!(data, &*data2); // oops UB
    }

    {
        let data: &Foo = query.single();
        let data2: Mut<Foo> = query.single_mut();
        assert_eq!(data, &*data2); // oops UB
    }

    {
        let data: &Foo = query.get_single().unwrap();
        let data2: Mut<Foo> = query.get_single_mut().unwrap();
        assert_eq!(data, &*data2); // oops UB
    }

    {
        let data: &Foo = query.iter().next().unwrap();
        let data2: Mut<Foo> = query.iter_mut().next().unwrap();
        assert_eq!(data, &*data2); // oops UB
    }

    {
        let mut opt_data: Option<&Foo> = None;
        let mut opt_data_2: Option<Mut<Foo>> = None;
        query.for_each(|data| opt_data = Some(data));
        query.for_each_mut(|data| opt_data_2 = Some(data));
        assert_eq!(opt_data.unwrap(), &*opt_data_2.unwrap()); // oops UB
    }
    dbg!("bye");
}

```

## Solution
yeet unsound lifetime annotations on `Query` methods

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2022-03-22 02:49:41 +00:00
Alice Cecile
7ce3ae43e3 Bump Bevy to 0.7.0-dev (#4230)
# Objective

- The [dev docs](https://dev-docs.bevyengine.org/bevy/index.html#) show version 0.6.0, which is actively misleading.

[Image of the problem](https://cdn.discordapp.com/attachments/695741366520512563/953513612943704114/Screenshot_20220316-154100_Firefox-01.jpeg)

Noticed by @ickk, fix proposed by @mockersf.

## Solution

- Bump the version across all Bevy crates to 0.7.0 dev.
- Set a reminder in the Release Checklist to remember to do this each release.
2022-03-19 03:54:15 +00:00
Carter Anderson
2ee38cb9e0 Release 0.6.0 (#3587) 2022-01-08 10:18:22 +00:00
François
c6fec1f0c2 Fix clippy lints for 1.57 (#3238)
# Objective

- New clippy lints with rust 1.57 are failing

## Solution

- Fixed clippy lints following suggestions
- I ignored clippy in old renderer because there was many and it will be removed soon
2021-12-02 23:40:37 +00:00
Joshua Chapman
274ace790b Implement iter() for mutable Queries (#2305)
A sample implementation of how to have `iter()` work on mutable queries without breaking aliasing rules.

# Objective

- Fixes #753

## Solution

- Added a ReadOnlyFetch to WorldQuery that is the `&T` version of `&mut T` that is used to specify the return type for read only operations like `iter()`.
- ~~As the comment suggests specifying the bound doesn't work due to restrictions on defining recursive implementations (like `Or`). However bounds on the functions are fine~~ Never mind I misread how `Or` was constructed, bounds now exist.
- Note that the only mutable one has a new `Fetch` for readonly as the `State` has to be the same for any of this to work


Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2021-12-01 23:28:10 +00:00
Niklas Eicker
d0f423d653 Assert compiler errors for compile_fail tests (#3067)
# Objective

bevy_ecs has several compile_fail tests that assert lifetime safety. In the past, these tests have been green for the wrong reasons (see e.g. #2984). This PR makes sure, that they will fail if the compiler error changes.

## Solution

Use [trybuild](https://crates.io/crates/trybuild) to assert the compiler errors.

The UI tests are in a separate crate that is not part of the Bevy workspace. This is to ensure that they do not break Bevy's crater builds. The tests get executed by the CI workflow on the stable toolchain.
2021-11-13 22:43:19 +00:00