Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Incorrect handling of EntityRef/Mut Queries + Resources resulting in false conflicts #13139

Closed
iiYese opened this issue Apr 29, 2024 · 6 comments · Fixed by #14561
Closed

Incorrect handling of EntityRef/Mut Queries + Resources resulting in false conflicts #13139

iiYese opened this issue Apr 29, 2024 · 6 comments · Fixed by #14561
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@iiYese
Copy link
Contributor

iiYese commented Apr 29, 2024

Bevy version

0.13

What you did

Have a system with EntityRef & ResMut<T>.

What went wrong

System should not have any access conflicts but it does.

Additional information

Minimal example to reproduce:

use bevy::prelude::*;

#[derive(Resource)]
struct Foo;

fn sys(_: Query<EntityRef>, _: ResMut<Foo>) {}

fn main() {
    App::new()
        .insert_resource(Foo)
        .add_systems(Update, sys)
        .run();
}
thread 'main' panicked at /home/yogii/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ecs-0.13.2/src/system/system_param.rs:542:13:
error[B0002]: ResMut<bevy_bug::Foo> in system bevy_bug::sys conflicts with a previous Res<bevy_bug::Foo> access. Consider removing the duplicate access.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!
@iiYese iiYese added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events labels Apr 29, 2024
@iiYese
Copy link
Contributor Author

iiYese commented Apr 29, 2024

#13120 would help prevent these kinds of bugs & surprises.

@alice-i-cecile
Copy link
Member

Yeah, I don't think our access model is quite right. We have reads_all and writes_all, but that's inadequately granular: components and resources are distinct.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! label Apr 29, 2024
@Wiwip
Copy link
Contributor

Wiwip commented May 27, 2024

Probably obvious, but I'd like to point out that the opposite EntityMut and Res will also return the same error.

use bevy::prelude::*;

#[derive(Resource)]
struct Foo;

fn sys(_: Query<EntityMut>, _: Res<Foo>) {}

fn main() {
    App::new()
        .insert_resource(Foo)
        .add_systems(Update, sys)
        .run();
}

@rendaoer
Copy link
Contributor

I've encountered this too, is there any progress here?

@thebluefish
Copy link
Contributor

World::resource_scope is a convenient work-around until the accesses for EntityRef/EntityMut are fixed. Though it does feel quite boilerplate-y:

fn sys(world: &mut World, state: &mut SystemState<Query<EntityRef>>) {
    world.resource_scope(|world, mut foo: Mut<Foo>| {
        let _query = state.get(world);
    });
}

@notmd
Copy link
Contributor

notmd commented Jul 25, 2024

Just hit this as well, I have to switch to Res and queue my resource update with Commands

github-merge-queue bot pushed a commit that referenced this issue Aug 6, 2024
# Objective

- Fixes #13139
- Fixes #7255
- Separates component from resource access so that we can correctly
handles edge cases like the issue above
- Inspired from #14472

## Solution

- Update access to have `component` fields and `resource` fields

## Testing

- Added some unit tests
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
6 participants