feat: optional components, added/changed filters and disjoint queries

assets/tests/add_system/optional_components_and_disjoint_queries.lua

@@ -0,0 +1,79 @@
+-- optional components are nil on entities without them, queries of a system may access the same component
+-- on disjoint entities, but not on the same ones
+local ScriptAttachment = import_namespace("bms").ScriptAttachment
+local bms = import_namespace("bms")
+local world = bms.world
+runs = {}
+local ComponentA = world.get_type_by_name("test_utils::test_data::CompWithFromWorldAndComponentData")
+local ComponentB = world.get_type_by_name("test_utils::test_data::CompWithDefaultAndComponentData")
+
+function on_test()
+    local post_update_schedule = world.get_schedule_by_name("PostUpdate")
+    local script_attachment = ScriptAttachment.new_entity_script(bms.current_entity(), bms.current_script())
+
+    local both = world.spawn()
+    world.add_default_component(both, ComponentA)
+    world.add_default_component(both, ComponentB)
+    local only_a = world.spawn()
+    world.add_default_component(only_a, ComponentA)
+
+    world.add_system(
+        post_update_schedule,
+        bms.system_builder("optional_system", script_attachment)
+        :query(world.query():component_read(ComponentA):optional_read(ComponentB))
+    )
+    world.add_system(
+        post_update_schedule,
+        bms.system_builder("disjoint_system", script_attachment)
+        :query(world.query():component(ComponentA):with(ComponentB))
+        :query(world.query():component_read(ComponentA):without(ComponentB))
+    )
+    -- both queries write `A` on entities with `B`
+    world.add_system(
+        post_update_schedule,
+        bms.system_builder("conflicting_system", script_attachment)
+        :query(world.query():component(ComponentA))
+        :query(world.query():component(ComponentA):with(ComponentB))
+    )
+    return true
+end
+
+function optional_system(query)
+    local with_b, without_b = 0, 0
+    for _, result in pairs(query) do
+        local components = result:components()
+        assert(components[1] ~= nil, "the required component should be present")
+        if components[2] == nil then
+            without_b = without_b + 1
+        else
+            assert(type(components[2][1]) == "string", "the optional component should be readable")
+            with_b = with_b + 1
+        end
+    end
+    assert(with_b >= 1 and without_b >= 1, "expected entities with and without B, got " .. with_b .. " and " .. without_b)
+    runs[#runs + 1] = "optional_system"
+end
+
+function disjoint_system(with_b, without_b)
+    assert(#with_b >= 1 and #without_b >= 1, "expected results for both queries")
+    -- `A` is written on entities with `B`, but only read on the others
+    with_b[1]:components()[1][1] = "written"
+    local ok, err = pcall(function() without_b[1]:components()[1][1] = "written" end)
+    assert(not ok and tostring(err):find("writing the component", 1, true), tostring(err))
+    runs[#runs + 1] = "disjoint_system"
+end
+
+function conflicting_system()
+    runs[#runs + 1] = "conflicting_system"
+end
+
+function on_test_post_update()
+    return true
+end
+
+function on_test_last()
+    table.sort(runs)
+    local order = table.concat(runs, ",")
+    assert(order == "disjoint_system,optional_system", "unexpected runs: " .. order)
+    return true
+end

crates/bevy_mod_scripting_bindings/src/query.rs

@@ -14,11 +14,12 @@ use crate::{
 };
 use ::{
     bevy_ecs::{
+        change_detection::Tick,
         component::ComponentId,
         entity::Entity,
         query::{QueryData, QueryState},
         reflect::{ReflectBundle, ReflectComponent, ReflectFromWorld},
-        world::World,
+        world::{World, unsafe_world_cell::UnsafeEntityCell},
     },
     bevy_reflect::{
         PartialReflect, Reflect, ReflectFromReflect, TypeRegistration, std_traits::ReflectDefault,
@@ -449,10 +450,16 @@ pub struct ScriptQueryBuilder {
     pub components: Vec<ScriptComponentRegistration>,
     /// Whether each of the components is written, components without an entry are written.
     writes: Vec<bool>,
+    /// Whether each of the components is optional, components without an entry are required.
+    optionals: Vec<bool>,
     /// Components that must be present.
     pub with: Vec<ScriptComponentRegistration>,
     /// Components that must not be present.
     pub without: Vec<ScriptComponentRegistration>,
+    /// Components that must have been added since the last run (of the system, or of the app's update outside systems).
+    pub added: Vec<ScriptComponentRegistration>,
+    /// Components that must have been changed since the last run (of the system, or of the app's update outside systems).
+    pub changed: Vec<ScriptComponentRegistration>,
 }
 
 #[profiling::all_functions]
@@ -471,21 +478,38 @@ impl ScriptQueryBuilder {
     }
     /// Adds a component to the query, which can be read and written.
     pub fn component(&mut self, component: ScriptComponentRegistration) -> &mut Self {
-        self.push_component(component, true)
+        self.push_component(component, true, false)
     }
 
     /// Adds a component to the query, which can only be read.
     ///
     /// Systems reading a component can run in parallel with each other.
     pub fn component_read(&mut self, component: ScriptComponentRegistration) -> &mut Self {
-        self.push_component(component, false)
+        self.push_component(component, false, false)
     }
 
-    fn push_component(&mut self, component: ScriptComponentRegistration, write: bool) -> &mut Self {
-        // components pushed to the public field directly are written
+    /// Adds a component to the query which entities don't need to have, which can be read and written.
+    pub fn optional(&mut self, component: ScriptComponentRegistration) -> &mut Self {
+        self.push_component(component, true, true)
+    }
+
+    /// Adds a component to the query which entities don't need to have, which can only be read.
+    pub fn optional_read(&mut self, component: ScriptComponentRegistration) -> &mut Self {
+        self.push_component(component, false, true)
+    }
+
+    fn push_component(
+        &mut self,
+        component: ScriptComponentRegistration,
+        write: bool,
+        optional: bool,
+    ) -> &mut Self {
+        // components pushed to the public field directly are written and required
         self.writes.resize(self.components.len(), true);
+        self.optionals.resize(self.components.len(), false);
         self.components.push(component);
         self.writes.push(write);
+        self.optionals.push(optional);
         self
     }
 
@@ -494,6 +518,87 @@ impl ScriptQueryBuilder {
         self.writes.get(index).copied().unwrap_or(true)
     }
 
+    /// Returns true if the component at the given index of [`Self::components`] is optional
+    pub fn is_optional(&self, index: usize) -> bool {
+        self.optionals.get(index).copied().unwrap_or(false)
+    }
+
+    /// Only matches entities whose component was added since the last run, see [`Self::added`]
+    pub fn added_component(&mut self, component: ScriptComponentRegistration) -> &mut Self {
+        self.added.push(component);
+        self
+    }
+
+    /// Only matches entities whose component was changed since the last run, see [`Self::changed`]
+    pub fn changed_component(&mut self, component: ScriptComponentRegistration) -> &mut Self {
+        self.changed.push(component);
+        self
+    }
+
+    /// The components the query accesses, and whether it writes them:
+    /// the retrieved ones, and the ones whose changes it checks (read)
+    pub fn accessed_components(&self) -> impl Iterator<Item = (ComponentId, bool)> + '_ {
+        self.components
+            .iter()
+            .enumerate()
+            .map(|(index, c)| (c.component_id(), self.is_written(index)))
+            .chain(
+                self.added
+                    .iter()
+                    .chain(&self.changed)
+                    .map(|c| (c.component_id(), false)),
+            )
+    }
+
+    /// Builds the result for an entity matched by the query state from [`Self::as_query_state`],
+    /// or returns `None` if it doesn't pass the [`Self::added`] and [`Self::changed`] filters, which query states can't express.
+    ///
+    /// Absent optional components are `None`.
+    ///
+    /// # Safety
+    /// The caller must be allowed to read the change ticks of the components in the [`Self::added`] and [`Self::changed`] filters
+    pub unsafe fn result_for(
+        &self,
+        entity: UnsafeEntityCell,
+        last_run: Tick,
+        this_run: Tick,
+    ) -> Option<ScriptQueryResult> {
+        // Safety: the caller can read the ticks
+        let ticks = |c: &ScriptComponentRegistration| unsafe {
+            entity.get_change_ticks_by_id(c.component_id())
+        };
+        let passes = self
+            .added
+            .iter()
+            .all(|c| ticks(c).is_some_and(|t| t.is_added(last_run, this_run)))
+            && self
+                .changed
+                .iter()
+                .all(|c| ticks(c).is_some_and(|t| t.is_changed(last_run, this_run)));
+        if !passes {
+            return None;
+        }
+
+        let components = self
+            .components
+            .iter()
+            .enumerate()
+            .map(|(index, c)| {
+                (!self.is_optional(index) || entity.contains_id(c.component_id())).then(|| {
+                    ReflectReference::new_component_ref_by_id(
+                        entity.id(),
+                        c.component_id(),
+                        c.type_registration().type_id(),
+                    )
+                })
+            })
+            .collect();
+        Some(ScriptQueryResult {
+            entity: entity.id(),
+            components,
+        })
+    }
+
     /// Adds components to the query that must be present.
     pub fn with_components(&mut self, with: Vec<ScriptComponentRegistration>) -> &mut Self {
         self.with.extend(with);
@@ -528,10 +633,34 @@ impl ScriptQueryBuilder {
         // we don't actually want to fetch the data for components now, only figure out
         // which entities match the query, and declare the access
         for (index, c) in self.components.iter().enumerate() {
-            if access == QueryAccess::Declared && self.is_written(index) {
-                dynamic_query.mut_id(c.component_id());
+            let write = access == QueryAccess::Declared && self.is_written(index);
+            let id = c.component_id();
+            if self.is_optional(index) {
+                dynamic_query.optional(|optional| {
+                    if write {
+                        optional.mut_id(id);
+                    } else {
+                        optional.ref_id(id);
+                    }
+                });
+            } else if write {
+                dynamic_query.mut_id(id);
             } else {
-                dynamic_query.ref_id(c.component_id());
+                dynamic_query.ref_id(id);
+            }
+        }
+
+        // change filters read the components' ticks, and imply the components are present
+        for c in self.added.iter().chain(&self.changed) {
+            let id = c.component_id();
+            dynamic_query.with_id(id);
+            let retrieved = self
+                .components
+                .iter()
+                .enumerate()
+                .any(|(index, other)| other.component_id() == id && !self.is_optional(index));
+            if !retrieved {
+                dynamic_query.ref_id(id);
             }
         }
 
@@ -553,6 +682,6 @@ impl ScriptQueryBuilder {
 pub struct ScriptQueryResult {
     /// The entity that matched the query.
     pub entity: Entity,
-    /// The components that matched the query.
-    pub components: Vec<ReflectReference>,
+    /// The components that matched the query, `None` for absent optional components.
+    pub components: Vec<Option<ReflectReference>>,
 }

crates/bevy_mod_scripting_bindings/src/reference.rs

@@ -485,7 +485,7 @@ impl ReflectReference {
             .ok_or_else(|| InteropError::unregistered_base(self.base.clone()))?;
 
         if let ReflectBase::Component(entity, component_id) = &self.base.base_id {
-            world.check_archetype_access(*entity, *component_id)?;
+            world.check_archetype_access(*entity, *component_id, false)?;
         }
 
         let ptr = unsafe {
@@ -561,7 +561,7 @@ impl ReflectReference {
             .ok_or_else(|| InteropError::unregistered_base(self.base.clone()))?;
 
         if let ReflectBase::Component(entity, component_id) = &self.base.base_id {
-            world.check_archetype_access(*entity, *component_id)?;
+            world.check_archetype_access(*entity, *component_id, true)?;
         }
 
         let ptr = unsafe {

crates/bevy_mod_scripting_bindings/src/world_extensions.rs

@@ -41,7 +41,7 @@ use bevy_ecs::{
     observer::{Observer, On},
     resource::Resource,
     system::Commands,
-    world::{CommandQueue, DeferredWorld, EntityRef, Mut},
+    world::{CommandQueue, DeferredWorld, Mut},
 };
 use bevy_mod_scripting_asset::ScriptAsset;
 use bevy_mod_scripting_script::{Recipients, ScriptAttachment};
@@ -529,26 +529,17 @@ impl<'w> WorldExtensions for WorldAccessGuard<'w> {
     ) -> Result<VecDeque<ScriptQueryResult>, InteropError> {
         self.with_world_mut(|world| {
             let mut built_query =
-                query.as_query_state::<EntityRef>(world, crate::QueryAccess::MatchOnly);
-            let query_result = built_query.iter(world);
-
-            Ok(query_result
-                .map(|r| {
-                    let references: Vec<_> = query
-                        .components
-                        .iter()
-                        .map(|c| ReflectReference {
-                            base: super::ReflectBaseType {
-                                type_id: c.type_registration().type_id(),
-                                base_id: super::ReflectBase::Component(r.id(), c.component_id()),
-                            },
-                            reflect_path: Default::default(),
-                        })
-                        .collect();
-                    ScriptQueryResult {
-                        entity: r.id(),
-                        components: references,
-                    }
+                query.as_query_state::<Entity>(world, crate::QueryAccess::MatchOnly);
+            // outside of systems, changes are relative to the last update, like `World::query`
+            let (last_run, this_run) = (world.last_change_tick(), world.read_change_tick());
+            let world = world.as_unsafe_world_cell_readonly();
+            // Safety: we have exclusive access to the world, and only read it
+            let entities = unsafe { built_query.iter_unchecked(world) };
+            Ok(entities
+                .filter_map(|entity| {
+                    let entity = world.get_entity(entity).ok()?;
+                    // Safety: we have exclusive access to the world
+                    unsafe { query.result_for(entity, last_run, this_run) }
                 })
                 .collect())
         })?