Multiple Querys with mut acces and changed filters cant be used without QuerySet #1791

Activity

1. alice-i-cecile

   added

   C-BugAn unexpected or incorrect behaviorAn unexpected or incorrect behavior

   A-ECSEntities, components, systems, and eventsEntities, components, systems, and events

   on Mar 31, 2021
2. MinerSebas commented on Mar 31, 2021

   

   MinerSebas

   on Mar 31, 2021

   ContributorAuthor

   More actions

   After remembering that before #1525 the conflict detection depended on the actually spawned entities, I expanded the Example:

   use bevy::prelude::*;
   
   fn main() {
       App::build().add_startup_system(test_startup.system()).add_system(test_system.system()).run();
   }
   
   struct TestStruct;
   
   fn test_startup(
       commands: &mut Commands,
   ) {
       commands.spawn((TestStruct,));
   }
   
   fn test_system(
       mut query1: Query<(Entity, &mut TestStruct)>,
       query2: Query<Entity, Added<TestStruct>>,
   ) {
   
   }

   This causes a Panic even before #1525 with this message:
   thread 'main' panicked at 'System &hello_world::test_system has conflicting queries. bevy_ecs::core::entities::Entity conflicts with the component access [hello_world::TestStruct] in this prior query: (bevy_ecs::core::entities::Entity, &mut hello_world::TestStruct).', crates\bevy_ecs\src\system\into_system.rs:75:13

   This means that this Bug isn't actually a regression caused by #1525.
3. cart commented on Mar 31, 2021

   

   cart

   on Mar 31, 2021

   Member

   More actions

   Supporting Added filters is technically safe here but would require special casing for it. We can't support Changed<T> filters in this context because they read state that &mut T queries set.

   I'm also a little hesitant to support this case right now because we're hoping to eventually make immediate component add/removes possible from within a system, which might not be safe if we allow these queries to exist side by side (hard to tell before we have a design for that feature)
4. alice-i-cecile

   added

   S-Needs-Design-DocThis issue or PR is particularly complex, and needs an approved design doc before it can be mergedThis issue or PR is particularly complex, and needs an approved design doc before it can be merged

   S-Needs-DesignThis issue requires design work to think about how it would best be accomplishedThis issue requires design work to think about how it would best be accomplished

   and removed

   S-Needs-Design-DocThis issue or PR is particularly complex, and needs an approved design doc before it can be mergedThis issue or PR is particularly complex, and needs an approved design doc before it can be merged

   on Dec 12, 2021
5. alice-i-cecile commented on Dec 12, 2021

   

   alice-i-cecile

   on Dec 12, 2021

   Member

   More actions

   bevyengine/rfcs#30 is useful prior art for anyone interested in tackling these issues. Like I laid out in that closed RFC, there are actually 4 access levels here, which we don't differentiate fully in the current design:

   • Forge: add and remove components
   • Write: modify component values
   • Read: read component values
   • Presence: check if a component exists

   Added filters only need presence-level permissions, while Changed needs read-level permissions. We could implement this model independently of the more problematic elements of the linked RFC, which would fix this issue without blocking future instant-addition work.
6. alice-i-cecile commented on Jan 27, 2022

   

   alice-i-cecile

   on Jan 27, 2022

   Member

   More actions

   This resolved! Use the SystemState API.
7. alice-i-cecile

   closed this as completedon Jan 27, 2022