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

Panic due to incompatible queries #1695

Closed
runfalk opened this issue Mar 19, 2021 · 5 comments
Closed

Panic due to incompatible queries #1695

runfalk opened this issue Mar 19, 2021 · 5 comments
Labels
A-ECS Entities, components, systems, and events

Comments

@runfalk
Copy link

runfalk commented Mar 19, 2021

Bevy version

348e2a3 (master at the time of writing)

Operating system & version

Fedora 33

What you did

I tried to port https://github.com/SarthakSingh31/bevy_4x_camera to work with current master. I'm testing Bevy for some visualization stuff. The changes required to make it compile were pretty minor.

This part is causing me issues though: https://github.com/SarthakSingh31/bevy_4x_camera/blob/440d8ce75736c2c210d4b07e06675e585d97b4ef/src/lib.rs#L87

What you expected to happen

The compiled code should run.

What actually happened

The compiled code panics with this error immediately:

thread 'main' panicked at 'Query<&mut bevy_transform::components::transform::Transform, bevy_ecs::query::filter::With<bevy_render::camera::camera::Camera>> in system &rust_visualizer::camera::camera_rig_movement accesses component(s) bevy_transform::components::transform::Transform in a way that conflicts with a previous system parameter. Allowing this would break Rust's mutability rules. Consider merging conflicting Queries into a QuerySet.', /home/ante/.cargo/git/checkouts/bevy-f7ffde730c324c74/348e2a3/crates/bevy_ecs/src/system/system_param.rs:142:5

Additional information

I suspect there is a problem with the new ECS v2 that was introduced 2 weeks ago. Did the previous system not check for mutability like this? It doesn't know that the second query is only used to query children of the first element. However, I'm still very new to this so that may not be correct.

@alice-i-cecile alice-i-cecile added the A-ECS Entities, components, systems, and events label Mar 19, 2021
@TheRawMeatball
Copy link
Member

Hmm, I believe this is because we no longer use archetypes for detection but components, which tighten the requirements. #1481 is the long-term solution but it's not ready yet. For your particular problem, putting Without<Camera> on that line should fix the issue.

@alice-i-cecile
Copy link
Member

This is on the to-do list for the 0.5 migration; introduced in #1525 in response to #1320.

The TL;DR of it is:

  • Mutability used to be checked dynamically on a per-archetype basis
  • This caused problems because it could lead to systems working for a while and then panicking at some hard-to-debug point in the future when components were added to random entities
  • Now they're checked on a per-component basis unless we have a guarantee that the archetypes won't clash
  • Use the solution given by Meatball until archetype invariants land :)

@cart
Copy link
Member

cart commented Mar 19, 2021

I think we should pitch archetype invariants as "a solution" not "the solution". Archetype invariants are "global rules" which I don't think apply to most query conflicts. Like in this case, the solution shouldn't be "assert that all Transform and Camera components are disjoint" as that is clearly not true (as evidenced by the various camera Bundles).

I think we should generally recommend:

  1. Making queries disjoint via with/without filters
  2. Adding conflicting queries to QuerySets
  3. Creating "global" archetype invariant rules

(In that order)

@runfalk
Copy link
Author

runfalk commented Mar 19, 2021

Wow, thanks a lot for the quick replies! @TheRawMeatball's solution worked great for that particular problem. I do get the same kind problem for the other system as well.

This one does however not use children:

fn camera_rig_follow(
    time: Res<Time>,
    mut rig_query: Query<(&mut Transform, &mut CameraRig)>,
    mut follow_query: Query<(&mut Transform, &CameraRigFollow), Changed<Transform>>,
                          // ^ this &mut can be removed but the same issue persists
) {
    // ...
}

With the error:

thread 'main' panicked at 'Query<(&mut bevy_transform::components::transform::Transform, &rust_visualizer::camera::CameraRigFollow), bevy_ecs::query::filter::Changed<bevy_transform::components::transform::Transform>> in system &rust_visualizer::camera::camera_rig_follow accesses component(s) bevy_transform::components::transform::Transform in a way that conflicts with a previous system parameter. Allowing this would break Rust's mutability rules. Consider merging conflicting Queries into a QuerySet.', /home/ante/.cargo/git/checkouts/bevy-f7ffde730c324c74/348e2a3/crates/bevy_ecs/src/system/system_param.rs:142:5

I think this one is used to make the camera track a given entity. You add the CameraRigFollow component to the entity you want to track.

For now I can just remove that, as I'm not really interested in it. Just adding it here for completeness.

@alice-i-cecile
Copy link
Member

I'm going to close this out: I think the other issues we have open address this well :)

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
Projects
None yet
Development

No branches or pull requests

4 participants