Conflicting Queries Panic - Systems in Different Stages

[aevyrie]

Bevy version

0.4

Operating system & version

Windows 10

What you did

Creating a demo that shows the Interaction component being used in 3d space with mod_picking alongside the UI:

https://github.com/aevyrie/bevy_mod_picking/blob/63cf2dcaedfa8dd5817ba3a2e317b64b8b256109/examples/3d_scene.rs#L4-L24

The panic happens iff lines 21 and 14 are both uncommented. You can uncomment one, but not both. My hunch is that something is going sideways with my use of the Interaction component.

What you expected to happen

The demo should run with a button overlaying a 3d scene, with both the button and meshes interactable.

What actually happened

The following panic occurs:

thread 'main' panicked at 'System &bevy_mod_picking::focus::mesh_focus has conflicting queries. &bevy_ui::focus::Interaction conflicts with 
the component access [bevy_ui::focus::Interaction] in this prior query: (&mut bevy_ui::focus::Interaction, core::option::Option<&bevy_ui::focus::FocusPolicy>, bevy_ecs::core::entities::Entity).', [path-redacted]\bevy_ecs-0.4.0\src\system\into_system.rs:73:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: process didn't exit successfully: `target\debug\examples\3d_scene.exe` (exit code: 101)

This line in the panic:

in this prior query: (&mut bevy_ui::focus::Interaction, core::option::Option<&bevy_ui::focus::FocusPolicy>, bevy_ecs::core::entities::Entity).

appears to be referencing the following query in mesh_focus():

https://github.com/aevyrie/bevy_mod_picking/blob/63cf2dcaedfa8dd5817ba3a2e317b64b8b256109/src/focus.rs#L8

The message:

&bevy_ui::focus::Interaction conflicts with 
the component access [bevy_ui::focus::Interaction]

Implies to me that the UI system is attempting to access Interaction while it is mutably borrowed in the mesh_focus() system.

The system in question does have another query with an immutable borrow of Interaction, but it works fine when the ui stage in my example is commented out.

Additional information

@alice-i-cecile mentioned the following:

I'm concerned that it's a particularly nasty soundness bug, hence the unusual level of concern
You managed to hit a check that @Ratysz was saying he's never seen triggered and was considering removing

[alice-i-cecile]

First step to tracking this down will be to make a minimum reproducible example. I'll try to give it a shot, this week, since I've poked at this code base before.

[cart]

This doesn't seem broken or unsafe to me / it appears to be the system working as intended. First: we don't do any cross-system safety checks. That safety rests solely in the hands of the scheduler to enforce. This error is completely internal to the mesh_focus system.

We currently disallow systems with multiple Queries that access conflicting components (within an archetype). We detect this case and panic because the alternative would be opening ourselves up to soundness errors.

The "fix" is to wrap your queries in a QuerySet, to ensure they are tied to the same lifetime, or alternatively to rephrase your queries in a way that doesn't result in conflicting accesses.

We currently only panic when there is a conflicting archetype/component. This is maximally permissive, which is both a good thing and a bad thing. It only fails when there is real conflicting data. But it also creates situations (which im assuming is this situation) where a system works, up until the point where it doesn't (when new data is added that creates a conflicting Archetype/Component access across queries).

I'm relatively convinced at this point that we need to be less permissive. We should fail (or at the very least, warn) when queries might conflict.

Additionally, we should include the "wrap in QuerySet" recommendation in these errors to give people an easy fix.

[aevyrie]

It only fails when there is real conflicting data. But it also creates situations (which im assuming is this situation) where a system works, up until the point where it doesn't (when new data is added that creates a conflicting Archetype/Component access across queries).

I think you're right. I was about to post that if I remove the node_query, which accesses Interactable with an immutable reference, that everything works again.

I'm relatively convinced at this point that we need to be less permissive. We should fail (or at the very least, warn) when queries might conflict.

I agree. Based on my experience with Bevy, I was surprised to hear that it can fail sometimes due to the permissiveness of the rules. I assumed that if it ran once, my component access must be correct.

Edit: I'm going to attempt to fix this with your suggestions, and close the issue if successful. Let me know if there's anything else I can do to help.

[cart]

Awesome!

Let me know if there's anything else I can do to help.

Opening a new issue to track the "less permissive" check would be very helpful 😄

[aevyrie]

It's working!!!

I ended up using a QuerySet<>. I see now how these queries were problematic. I will open an issue for less permissive system conflict errors.

bevy.2021-01-25.18-33-09_Trim.mp4

[Ratysz]

@alice-i-cecile mentioned the following:

I'm concerned that it's a particularly nasty soundness bug, hence the unusual level of concern
You managed to hit a check that @Ratysz was saying he's never seen triggered and was considering removing

That's not what that was about at all. The "safety bit" was an internal safeguard I wrote into the new executor, it is not a thing in Bevy - or the new executor since a couple of days ago, for that matter (the refactors trivialized the case it was trying to guard against). For posterity, all it did was prevent running a system more than once per algorithm iteration.