Skip to content

Issue with custom WorldQuery with Changed and mutable #10147

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

Closed
mattzque opened this issue Oct 16, 2023 · 4 comments
Closed

Issue with custom WorldQuery with Changed and mutable #10147

mattzque opened this issue Oct 16, 2023 · 4 comments
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior

Comments

@mattzque
Copy link

Bevy version

0.11.3

What you did

I tried to cleanup a system query, I made a full example here:

use bevy::{prelude::*, ecs::query::WorldQuery};

#[derive(Component)]
struct MyComponent;

fn my_system0(mut query: Query<&mut MyComponent, Changed<MyComponent>>) {
    for mut component in query.iter_mut() {
        // do something with component
    }
}

// so to clean it up I extract the query into a WorldQuery:

#[derive(WorldQuery)]
#[world_query(mutable)]
struct MyQuery1 {
    entity: Entity,
    component: &'static MyComponent,
    _a: Changed<MyComponent>,
}

fn my_system1(mut query: Query<MyQuery1>) {
    for mut component in query.iter_mut() {
        // do something with component
    }
}

// which works, however, this (with &mut) crashes with a runtime error:

#[derive(WorldQuery)]
#[world_query(mutable)]
struct MyQuery2 {
    entity: Entity,
    component: &'static mut MyComponent, // <- added mut
    _a: Changed<MyComponent>,
}

fn my_system2(mut query: Query<MyQuery2>) {
    for mut component in query.iter_mut() {
        // do something with component
    }
}

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Update, (
            my_system0,
            my_system1,
            my_system2, // <- runtime crash
        ))
        .run();
}

What went wrong

The error on my_system2 / MyQuery2 was '$state_name<bevy_minimal::MyComponent> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.'.

Backtrace:

thread 'main' panicked at '$state_name<bevy_minimal::MyComponent> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.', /home/apoc/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ecs-0.11.3/src/query/filter.rs:600:1
stack backtrace:
   0: rust_begin_unwind
             at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/core/src/panicking.rs:67:14
   2: <bevy_ecs::query::filter::Changed<T> as bevy_ecs::query::fetch::WorldQuery>::update_component_access
             at /home/apoc/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ecs-0.11.3/src/query/filter.rs:536:21
   3: bevy_minimal::_::<impl bevy_ecs::query::fetch::WorldQuery for bevy_minimal::MyQuery2>::update_component_access
             at ./src/main.rs:26:10
   4: bevy_ecs::query::state::QueryState<Q,F>::new
             at /home/apoc/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ecs-0.11.3/src/query/state.rs:102:9
   5: <bevy_ecs::system::query::Query<Q,F> as bevy_ecs::system::system_param::SystemParam>::init_state
             at /home/apoc/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ecs-0.11.3/src/system/system_param.rs:169:21
   6: <(P0,) as bevy_ecs::system::system_param::SystemParam>::init_state
             at /home/apoc/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ecs-0.11.3/src/system/system_param.rs:1384:21
   7: <bevy_ecs::system::function_system::FunctionSystem<Marker,F> as bevy_ecs::system::system::System>::initialize
             at /home/apoc/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ecs-0.11.3/src/system/function_system.rs:483:33
   8: bevy_ecs::schedule::schedule::ScheduleGraph::initialize
             at /home/apoc/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ecs-0.11.3/src/schedule/schedule.rs:849:21
   9: bevy_ecs::schedule::schedule::Schedule::initialize
[...]

This only happens when I use "&mut MyComponent" with a "Changed", otherwise WorldQuery works as expected.

This specific case is not mentioned in the examples: https://github.com/bevyengine/bevy/blob/main/examples/ecs/custom_query_param.rs

As an aside, I found those examples in custom_query_param.rs really difficult to understand but perhaps that's just me, at first I thought I was doing something wrong, or perhaps I am?

@mattzque mattzque added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Oct 16, 2023
@rparrett
Copy link
Contributor

rparrett commented Oct 16, 2023

That seems like a super confusing error.

But I think the root of your problem is that there are two generic arguments in Query. Query<Q, F> where Q is the "query fetch" and F is the "filter."

Changed is not valid in the fetch position of the query (does not implement ReadOnlyWorldQuery.)

I think the best you can do here is derive a second

#[derive(WorldQuery)]
struct MyQueryFilter {
    changed: Changed<MyComponent>
}

and do

query: Query<MyQuery, MyQueryFilter>

It also seems like you might want SystemParam instead.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels Oct 16, 2023
@alice-i-cecile
Copy link
Member

This should be fixed with #9918 as a result.

@mattzque
Copy link
Author

thank you, also thanks for the tip with SystemParam that is great to cleaning up my code even more

@james7132
Copy link
Member

Changed directly reads the change ticks of each component. &mut T will mutate them. This results in aliased access within a single query. Even in a filter position, this should panic due to the undefined behavior it would otherwise introduce, and it correctly does that. Even with #9918, this will still happen, and it is working as intended.

@james7132 james7132 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 19, 2024
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-Bug An unexpected or incorrect behavior
Projects
None yet
Development

No branches or pull requests

4 participants