Bevy 0.13 Update PR

[RobWalt]

Ready for review ✅

The PR is in a review-ready state now. All tests are green and we get now compile errors.

---

Irrelevant and superseded by the section above now, just leaving this first version of the description for transparency

This is most probably a very naive try to update this crate to bevy 0.13. The main things that I changed were:

• bump versions
• split the ReadOnlyWorldQuery impl into QueryData + ReadOnlyQueryData impls
• add QueryFilter impls for OneAdded and OneChanged
• add the missing get_state method on the WorldQuery impl
• the get_state change includes adding a new get method for TraitQueryState which mostly follows the init method
• adjusting the proc macros to reflect these changes

Most of these adjustments are super naive. I tried to mirror existing code as closely as possible (e.g. I consulted the bevy main repo for the trait impls). Most notably:

• For starters: At least it compiles :D
• The tests all still fail D:
• I didn't find any counter part for update_archetype_component_access in bevy 0.13

I'm happy to continue working on this update (and also future update) but I'm afraid I need a bit of mentoring as I'm stuck now

---

Related to #54

[RobWalt]

I tried looking deeper into the failing tests and here's a short analysis of the all1 test.

Here's the relevant test code:

use super::*;
use bevy_ecs::prelude::*;
use std::fmt::{Debug, Display};

// Required for proc macros.
use crate as bevy_trait_query;

#[derive(Resource, Default)]
pub struct Output(Vec<String>);

#[queryable]
pub trait Person {
    fn name(&self) -> &str;
    fn age(&self) -> u32;
    fn set_age(&mut self, age: u32);
}

#[derive(Component)]
struct Fem;

#[derive(Component)]
pub struct Human(String, u32);

impl Person for Human {
    fn name(&self) -> &str {
        &self.0
    }
    fn age(&self) -> u32 {
        self.1
    }
    fn set_age(&mut self, age: u32) {
        self.1 = age;
    }
}

#[derive(Component)]
pub struct Dolphin(u32);

impl Person for Dolphin {
    fn name(&self) -> &str {
        "Reginald"
    }
    fn age(&self) -> u32 {
        self.0
    }
    fn set_age(&mut self, age: u32) {
        self.0 = age;
    }
}

#[test]
fn all1() {
    let mut world = World::new();
    world.init_resource::<Output>();
    world
        .register_component_as::<dyn Person, Human>()
        .register_component_as::<dyn Person, Dolphin>();

    world.spawn(Human("Henry".to_owned(), 22));
    world.spawn((Human("Eliza".to_owned(), 31), Fem, Dolphin(6)));
    world.spawn((Human("Garbanzo".to_owned(), 17), Fem, Dolphin(17)));
    world.spawn(Dolphin(27));

    let mut schedule = Schedule::default();
    schedule.add_systems((print_all_info, (age_up_fem, age_up_not)).chain());

    schedule.run(&mut world);
    schedule.run(&mut world);

    assert_eq!(
        world.resource::<Output>().0,
        &[
            "All people:",
            "Henry: 22",
            "Eliza: 31",
            "Reginald: 6",
            "Garbanzo: 17",
            "Reginald: 17",
            "Reginald: 27",
            "",
            "All people:",
            "Henry: 23",
            "Eliza: 32",
            "Reginald: 7",
            "Garbanzo: 18",
            "Reginald: 18",
            "Reginald: 28",
            "",
        ]
    );
}

// Prints the name and age of every `Person`.
fn print_all_info(people: Query<&dyn Person>, mut output: ResMut<Output>) {
    output.0.push("All people:".to_string());
    for all in &people {
        for person in all {
            output
                .0
                .push(format!("{}: {}", person.name(), person.age()));
        }
    }
    output.0.push(Default::default());
}

fn age_up_fem(mut q: Query<&mut dyn Person, With<Fem>>) {
    for all in &mut q {
        for mut p in all {
            let age = p.age();
            p.set_age(age + 1);
        }
    }
}

fn age_up_not(mut q: Query<&mut dyn Person, Without<Fem>>) {
    for all in &mut q {
        for mut p in all {
            let age = p.age();
            p.set_age(age + 1);
        }
    }
}

The results of the test are:

thread 'tests::all1' panicked at src/tests.rs:143:5:
assertion `left == right` failed
  left: ["All people:", "Eliza: 31", "Reginald: 6", "Garbanzo: 17", "Reginald: 17", "", "All people:", "Eliza: 32", "Reginald: 7", "Garbanzo: 18", "Reginald: 18", ""]
 right: ["All people:", "Henry: 22", "Eliza: 31", "Reginald: 6", "Garbanzo: 17", "Reginald: 17", "Reginald: 27", "", "All people:", "Henry: 23", "Eliza: 32", "Reginald: 7", "Garbanzo: 18", "Reginald: 18", "Reginald: 28", ""]

If we look closer at the results, it seems like the query only iterates over the entities which have two entities which implement the trait. Actually it seems like it iterates only over entities which have all of the components which implement the trait. Let's quickly verify that. I added:

#[derive(Component)]
pub struct Alien(u32);

impl Person for Alien {
    fn name(&self) -> &str {
        "Goobert"
    }
    fn age(&self) -> u32 {
        self.0
    }
    fn set_age(&mut self, age: u32) {
        self.0 = age;
    }
}

...

    world
        .register_component_as::<dyn Person, Human>()
        .register_component_as::<dyn Person, Alien>()
        .register_component_as::<dyn Person, Dolphin>();

    world.spawn(Human("Henry".to_owned(), 22));
    world.spawn(Alien(222));
    world.spawn((Human("Eliza".to_owned(), 31), Fem, Dolphin(6), Alien(100)));
    world.spawn((Human("Garbanzo".to_owned(), 17), Fem, Dolphin(17)));
    world.spawn(Dolphin(27));

...

And the test result reports

thread 'tests::all1' panicked at src/tests.rs:160:5:
assertion `left == right` failed
  left: ["All people:", "Eliza: 31", "Goobert: 100", "Reginald: 6", "", "All people:", "Eliza: 32", "Goobert: 101", "Reginald: 7", ""]
 right: ["All people:", "Henry: 22", "Eliza: 31", "Reginald: 6", "Garbanzo: 17", "Reginald: 17", "Reginald: 27", "", "All people:", "Henry: 23", "Eliza: 32", "Reginald: 7", "Garbanzo: 18", "Reginald: 18", "Reginald: 28", ""]

So that might be true.

[RobWalt]

Big shoutout and credit to @Azorlogh for helping me out here!

[Azorlogh]

leftover comment

[SkiFire13]

The same fix should also apply here.

[RobWalt]

@SkiFire13 First: Thanks for the review!

I'm still not quiet sure what the required field even does, but I made the changes anyways. Note, that the tests passed even before the change. Nevertheless they seem to be even greener than before now 😄 and it proved to be pretty useful since it surfaced a bug, fixed in df16eeb

[joseph-gio]

Thanks for updating this guys :)
I'll try to review this on the weekend

[brandon-reinhart]

Need any help with getting this one wrapped up?

[allsey87]

I have not had a close look at the code, but I have played around with this PR and haven't found any issues so far. I tried both a system-based query and a world query (World::query) which both worked nicely.

[RobWalt]

One question that just popped up for us is:

• Should One implement QueryFilter?

Previously you could use it in the filter position, now it doesn't work anymore. It might be misleading to have it work in both positions, so maybe another struct WithOne which only implements QueryFilter (similar to OneAdded) might make sense.

Note to self: AtLeastOne is probably the better filter name as it comes closer to what it actually does, I think it's querying for components with exactly one impl, so the name idea was good already

[RobWalt]

FYI: PR seems to be unaffected by 0.13.1 - the tests are still green

[brandon-reinhart]

FWIW, I've been using this PR in my project for a few weeks and it works great.

[joseph-gio]

Apologies for the delay on this, I've been busy with IRL stuff. I'll try to get this merged in soon

[joseph-gio]

What's the purpose of the new_access and intermediate variables? Can we not insert directly into the filtered access?

[SkiFire13]

This similar to how AnyOf is implemented in Bevy. The expected result of the whole method is that access contains the result of (access AND write component1) OR (access AND write component2) OR ...

The way this is achieved is by computing each access AND write componentN in an intermediate FilteredAccess, and then OR them together in an accumulator, which is new_access FilteredAccess. new_access needs to be different from access because to compute each intermediate you need the original access, so directly modifying access will produce incorrect results.

[joseph-gio]

Makes sense, thank you for explaining.