From 45f13ecd76a8d7e54ee4a7f429f634f52a59619a Mon Sep 17 00:00:00 2001 From: makspll Date: Thu, 22 Oct 2026 16:19:25 +0000 Subject: [PATCH 1/2] feat: optional components, change filters and disjoint queries - `optional` / `optional_read` retrieve components entities don't need to have (nil when absent), `added` / `changed` filter by ticks since the system's last run (outside systems: since the last update) - queries of a system may share components when bevy considers their accesses disjoint, the archetype filter restricts reads and writes separately so a component written by one query and read by another can't be written on the reading query's entities - fix: script systems never stored their last run tick --- ...tional_components_and_disjoint_queries.lua | 79 ++++++++++ .../bevy_mod_scripting_bindings/src/query.rs | 149 ++++++++++++++++-- .../src/reference.rs | 4 +- .../src/world_extensions.rs | 33 ++-- .../src/script_system.rs | 131 ++++++++++----- .../bevy_mod_scripting_functions/src/core.rs | 74 ++++++++- .../src/archetype_filter.rs | 94 +++++++---- crates/bevy_mod_scripting_world/src/world.rs | 8 +- .../src/lib.rs | 2 +- docs/src/ReleaseNotes/0.23-to-0.24.md | 1 + docs/src/ScriptSystems/introduction.md | 11 ++ 11 files changed, 471 insertions(+), 115 deletions(-) create mode 100644 assets/tests/add_system/optional_components_and_disjoint_queries.lua diff --git a/assets/tests/add_system/optional_components_and_disjoint_queries.lua b/assets/tests/add_system/optional_components_and_disjoint_queries.lua new file mode 100644 index 0000000000..66595aec10 --- /dev/null +++ b/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 diff --git a/crates/bevy_mod_scripting_bindings/src/query.rs b/crates/bevy_mod_scripting_bindings/src/query.rs index c31d3c1df0..95c01768a5 100644 --- a/crates/bevy_mod_scripting_bindings/src/query.rs +++ b/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, @@ -433,10 +434,16 @@ pub struct ScriptQueryBuilder { pub components: Vec, /// Whether each of the components is written, components without an entry are written. writes: Vec, + /// Whether each of the components is optional, components without an entry are required. + optionals: Vec, /// Components that must be present. pub with: Vec, /// Components that must not be present. pub without: Vec, + /// Components that must have been added since the last run (of the system, or of the app's update outside systems). + pub added: Vec, + /// Components that must have been changed since the last run (of the system, or of the app's update outside systems). + pub changed: Vec, } #[profiling::all_functions] @@ -455,21 +462,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 } @@ -478,6 +502,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 + '_ { + 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 { + // 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) -> &mut Self { self.with.extend(with); @@ -515,10 +620,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 declare_writes && self.is_written(index) { - dynamic_query.mut_id(c.component_id()); + let write = declare_writes && 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); } } @@ -540,6 +669,6 @@ impl ScriptQueryBuilder { pub struct ScriptQueryResult { /// The entity that matched the query. pub entity: Entity, - /// The components that matched the query. - pub components: Vec, + /// The components that matched the query, `None` for absent optional components. + pub components: Vec>, } diff --git a/crates/bevy_mod_scripting_bindings/src/reference.rs b/crates/bevy_mod_scripting_bindings/src/reference.rs index 27636856f7..8eaa9d9ae3 100644 --- a/crates/bevy_mod_scripting_bindings/src/reference.rs +++ b/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 { diff --git a/crates/bevy_mod_scripting_bindings/src/world_extensions.rs b/crates/bevy_mod_scripting_bindings/src/world_extensions.rs index 9d62ed71fb..e5abdb9f07 100644 --- a/crates/bevy_mod_scripting_bindings/src/world_extensions.rs +++ b/crates/bevy_mod_scripting_bindings/src/world_extensions.rs @@ -39,7 +39,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}; @@ -509,26 +509,17 @@ impl<'w> WorldExtensions for WorldAccessGuard<'w> { query: crate::ScriptQueryBuilder, ) -> Result, InteropError> { self.with_world_mut(|world| { - let mut built_query = query.as_query_state::(world, false); - 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, - } + let mut built_query = query.as_query_state::(world, false); + // 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()) })? diff --git a/crates/bevy_mod_scripting_core/src/script_system.rs b/crates/bevy_mod_scripting_core/src/script_system.rs index feb743436f..a7881f993e 100644 --- a/crates/bevy_mod_scripting_core/src/script_system.rs +++ b/crates/bevy_mod_scripting_core/src/script_system.rs @@ -41,7 +41,7 @@ use bevy_mod_scripting_bindings::{ AppScheduleRegistry, AppScriptCallbackRunners, AppScriptComponentRegistry, AppScriptFunctionRegistry, AppScriptTypeCatalogue, CurrentScriptAttachment, InteropError, IntoScript, ReflectReference, ScriptComponentRegistration, ScriptQueryBuilder, - ScriptQueryResult, ScriptResourceRegistration, V, WorldExtensions, + ScriptResourceRegistration, V, WorldExtensions, }; use bevy_mod_scripting_display::DisplayProxy; use bevy_mod_scripting_script::ScriptAttachment; @@ -987,8 +987,8 @@ pub enum ScriptSystemParam { EntityQuery { /// The internal state of the query query: Box>, - /// the components in correct order describing the necessary references - components: Vec<(ComponentId, TypeId)>, + /// The query, which builds the results for the matched entities + builder: Box, }, } @@ -1044,12 +1044,14 @@ impl DynamicScriptSystem

{ fn build_archetype_filter(params: &[ScriptSystemParam]) -> ArchetypeFilter { let mut filter = ArchetypeFilter::default(); for param in params { - if let ScriptSystemParam::EntityQuery { query, components } = param { - // only components the query reads or writes, components used just as filters (`with`) are not accessible - for (component_id, _) in components { - filter.restrict(*component_id); + if let ScriptSystemParam::EntityQuery { query, builder } = param { + // only components the query reads or writes, components used just as filters (`with`) are not accessible. + // Writes are restricted to the archetypes of queries writing the component, as another query of the + // system may only read it on other archetypes + for (component_id, write) in builder.accessed_components() { + filter.restrict(component_id); for archetype_id in query.matched_archetypes() { - filter.allow(*component_id, archetype_id); + filter.allow(component_id, archetype_id, write); } } } @@ -1085,7 +1087,8 @@ impl System for DynamicScriptSystem

{ return Ok(()); } - let _change_tick = world.increment_change_tick(); + let this_run = world.increment_change_tick(); + let last_run = self.last_run; let Some(state) = &mut self.state else { return Err(RunSystemError::Skipped( @@ -1176,25 +1179,13 @@ impl System for DynamicScriptSystem

{ let res_ref = ReflectReference::new_resource_ref_by_id(*component_id, *type_id); payload.push(res_ref.into_script_inline_error(guard.clone())); } - ScriptSystemParam::EntityQuery { query, components } => { - // TODO: is this the right way to use this world cell for queries? - let entities = unsafe { query.iter_unchecked(world) }.collect::>(); - let results = entities - .into_iter() - .map(|entity| { - V(ScriptQueryResult { - entity, - components: components - .iter() - .map(|(component_id, type_id)| { - ReflectReference::new_component_ref_by_id( - entity, - *component_id, - *type_id, - ) - }) - .collect(), - }) + ScriptSystemParam::EntityQuery { query, builder } => { + // Safety: the query's access is declared in `initialize` + let results = unsafe { query.iter_unchecked(world) } + .filter_map(|entity| { + let entity = world.get_entity(entity).ok()?; + // Safety: the components with change filters are declared as read + unsafe { builder.result_for(entity, last_run, this_run) }.map(V) }) .collect::>(); @@ -1214,6 +1205,8 @@ impl System for DynamicScriptSystem

{ if let Err(err) = result { self.report(err); } + // change filters are relative to the last time the callback ran + self.last_run = this_run; Ok(()) } @@ -1259,13 +1252,18 @@ impl System for DynamicScriptSystem

{ } subset.insert(component_id); } - ScriptSystemParamDescriptor::EntityQuery(query) => { - let components: Vec<_> = query - .components - .iter() - .map(|c| (c.component_id, c.type_registration().type_id())) - .collect(); - let query = query.as_query_state::(world, true); + ScriptSystemParamDescriptor::EntityQuery(builder) => { + let query = builder.as_query_state::(world, true); + + // queries may access the same components if bevy considers them disjoint, i.e. `With` vs `Without` + if !component_access_set + .get_conflicts_single(query.component_access()) + .is_empty() + { + initialization_errors.push( + "A query conflicts with another parameter of the system, i.e. two queries write the same component on the same entities.".to_owned(), + ); + } // The access carries the query filters, so bevy runs this system in parallel with systems accessing // the same components on other archetypes (i.e. `With` vs `Without`). @@ -1280,18 +1278,12 @@ impl System for DynamicScriptSystem

{ .filter(|(_, write)| *write) .map(|(id, _)| *id), ); - let new_raids = accesses.into_iter().map(|(a, _)| a).collect::>(); - - if !subset.is_disjoint(&new_raids) { - initialization_errors - .push("Non-disjoint query in dynamic system parameters.".to_owned()); - } + subset.extend(accesses.into_iter().map(|(a, _)| a)); system_params.push(ScriptSystemParam::EntityQuery { query: query.into(), - components, + builder: Box::new(builder.clone()), }); - subset.extend(new_raids); } } } @@ -1815,4 +1807,57 @@ mod test { .unwrap() ); } + + #[derive(bevy_ecs::component::Component, Reflect)] + struct OtherTestComponent; + + #[test] + fn query_change_filters_and_optional_components() { + let mut world = World::new(); + let component = |_: &mut World, registration, id| { + ScriptComponentRegistration::new( + ScriptTypeRegistration::new(Arc::new(registration)), + id, + ) + }; + let id = world.register_component::(); + let test_component = component( + &mut world, + bevy_reflect::TypeRegistration::of::(), + id, + ); + let id = world.register_component::(); + let other_component = component( + &mut world, + bevy_reflect::TypeRegistration::of::(), + id, + ); + + let unchanged = world.spawn(AccessTestComponent).id(); + let changed = world.spawn((AccessTestComponent, OtherTestComponent)).id(); + world.clear_trackers(); + bevy_ecs::change_detection::DetectChangesMut::set_changed( + &mut world.get_mut::(changed).unwrap(), + ); + + let mut query = ScriptQueryBuilder::new(); + query + .optional_read(other_component.clone()) + .changed_component(test_component.clone()); + let cache = WorldAccessGuard::setup_cache(&world, CurrentScriptAttachment::empty()); + let guard = WorldAccessGuard::new_exclusive(&mut world, cache); + let results = guard.query(query).unwrap(); + assert_eq!(results.len(), 1, "only the changed entity matches"); + assert_eq!(results[0].entity, changed); + assert!(results[0].components[0].is_some()); + + let mut query = ScriptQueryBuilder::new(); + query.optional_read(other_component); + let mut results = guard.query(query).unwrap(); + results.retain(|result| result.entity == unchanged); + assert!( + results[0].components[0].is_none(), + "absent optionals are none" + ); + } } diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index a0659b0d0d..45d404324a 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -1422,6 +1422,76 @@ impl ScriptQueryBuilder { V(builder) } + /// Adds a component to be retrieved by the query, which entities don't need to have, it is nil for those which don't. + /// + /// Arguments: + /// * `query`: The query to add the component to + /// * `component`: The component to add + /// Returns: + /// * `query`: The query with the component added + fn optional( + query: V, + component: V, + ) -> V { + profiling::function_scope!("optional"); + let mut builder = query.into_inner(); + builder.optional(component.into_inner()); + V(builder) + } + + /// Like `optional`, but the component can only be read. + /// + /// Arguments: + /// * `query`: The query to add the component to + /// * `component`: The component to add + /// Returns: + /// * `query`: The query with the component added + fn optional_read( + query: V, + component: V, + ) -> V { + profiling::function_scope!("optional_read"); + let mut builder = query.into_inner(); + builder.optional_read(component.into_inner()); + V(builder) + } + + /// Only matches entities whose component was added since the system last ran (outside of systems: since the last update). + /// This component will NOT be retrieved. + /// + /// Arguments: + /// * `query`: The query to add the filter to + /// * `component`: The component to filter by + /// Returns: + /// * `query`: The query with the filter added + fn added( + query: V, + component: V, + ) -> V { + profiling::function_scope!("added"); + let mut builder = query.into_inner(); + builder.added_component(component.into_inner()); + V(builder) + } + + /// Only matches entities whose component was changed since the system last ran (outside of systems: since the last update). + /// This component will NOT be retrieved. + /// + /// Arguments: + /// * `query`: The query to add the filter to + /// * `component`: The component to filter by + /// Returns: + /// * `query`: The query with the filter added + fn changed( + query: V, + component: V, + ) -> V { + profiling::function_scope!("changed"); + let mut builder = query.into_inner(); + builder.changed_component(component.into_inner()); + V(builder) + } + /// Adds a component to filter the query by. This component will NOT be retrieved. /// /// Arguments: @@ -1497,13 +1567,13 @@ impl ScriptQueryResult { /// Retrieves the components from the query result. /// - /// These are ordered by the order they were added to the query. + /// These are ordered by the order they were added to the query, absent optional components are nil. /// /// Arguments: /// * `query`: The query result to retrieve the components from. /// Returns: /// * `components`: The components from the query result. - fn components(query: R) -> Vec { + fn components(query: R) -> Vec> { profiling::function_scope!("components"); query.components.to_vec() } diff --git a/crates/bevy_mod_scripting_world/src/archetype_filter.rs b/crates/bevy_mod_scripting_world/src/archetype_filter.rs index a9ff970beb..f0ee7c3f8c 100644 --- a/crates/bevy_mod_scripting_world/src/archetype_filter.rs +++ b/crates/bevy_mod_scripting_world/src/archetype_filter.rs @@ -34,51 +34,78 @@ use bevy_ecs::{archetype::ArchetypeId, component::ComponentId}; /// Components without a restriction are not affected, i.e. resources and components accessed by exclusive guards. #[derive(Clone, Debug, Default)] pub struct ArchetypeFilter { - /// Restricted components, each with a bitset indexed by [`ArchetypeId::index`]. + /// Restricted components. /// Systems restrict a handful of components, so a linear scan beats indexing by component id. - restricted: Vec<(ComponentId, Vec)>, + restricted: Vec, +} + +/// The archetypes a component may be read and written on, bitsets indexed by [`ArchetypeId::index`]. +/// +/// Reads and writes are restricted separately, as queries of a system may read a component on some archetypes +/// and write it on others, i.e. `C with A` written and `C without A` read. +#[derive(Clone, Debug)] +struct RestrictedComponent { + id: ComponentId, + read: Vec, + write: Vec, +} + +fn set_bit(bits: &mut Vec, index: usize) { + let (word, bit) = (index / 64, index % 64); + if word >= bits.len() { + bits.resize(word + 1, 0); + } + bits[word] |= 1 << bit; +} + +fn has_bit(bits: &[u64], index: usize) -> bool { + bits.get(index / 64) + .is_some_and(|word| word & (1 << (index % 64)) != 0) } impl ArchetypeFilter { - fn bits_mut(&mut self, component_id: ComponentId) -> &mut Vec { - let index = match self - .restricted - .iter() - .position(|(id, _)| *id == component_id) - { + fn restricted_mut(&mut self, component_id: ComponentId) -> &mut RestrictedComponent { + let index = match self.restricted.iter().position(|c| c.id == component_id) { Some(index) => index, None => { - self.restricted.push((component_id, Vec::new())); + self.restricted.push(RestrictedComponent { + id: component_id, + read: Vec::new(), + write: Vec::new(), + }); self.restricted.len() - 1 } }; - &mut self.restricted[index].1 + &mut self.restricted[index] } - /// Allows access to the given component on entities in the given archetype. + /// Allows reading, and if `write` writing, the given component on entities in the given archetype. /// /// Once called for a component, the component is restricted to the archetypes allowed this way. - pub fn allow(&mut self, component_id: ComponentId, archetype_id: ArchetypeId) { - let bits = self.bits_mut(component_id); - let (word, bit) = (archetype_id.index() / 64, archetype_id.index() % 64); - if word >= bits.len() { - bits.resize(word + 1, 0); + pub fn allow(&mut self, component_id: ComponentId, archetype_id: ArchetypeId, write: bool) { + let restricted = self.restricted_mut(component_id); + set_bit(&mut restricted.read, archetype_id.index()); + if write { + set_bit(&mut restricted.write, archetype_id.index()); } - bits[word] |= 1 << bit; } /// Restricts the given component to no archetypes, until [`Self::allow`] is called for it. pub fn restrict(&mut self, component_id: ComponentId) { - self.bits_mut(component_id); + self.restricted_mut(component_id); } - /// Returns true if the component may be accessed on an entity in the given archetype. + /// Returns true if the component may be read, or if `write` written, on an entity in the given archetype. #[inline] - pub fn is_allowed(&self, component_id: ComponentId, archetype_id: ArchetypeId) -> bool { - match self.restricted.iter().find(|(id, _)| *id == component_id) { - Some((_, bits)) => bits - .get(archetype_id.index() / 64) - .is_some_and(|word| word & (1 << (archetype_id.index() % 64)) != 0), + pub fn is_allowed( + &self, + component_id: ComponentId, + archetype_id: ArchetypeId, + write: bool, + ) -> bool { + match self.restricted.iter().find(|c| c.id == component_id) { + Some(c) if write => has_bit(&c.write, archetype_id.index()), + Some(c) => has_bit(&c.read, archetype_id.index()), None => true, } } @@ -91,7 +118,7 @@ mod test { #[test] fn unrestricted_components_are_allowed_everywhere() { let filter = ArchetypeFilter::default(); - assert!(filter.is_allowed(ComponentId::new(3), ArchetypeId::new(70))); + assert!(filter.is_allowed(ComponentId::new(3), ArchetypeId::new(70), true)); } #[test] @@ -99,14 +126,15 @@ mod test { let mut filter = ArchetypeFilter::default(); let (component, other) = (ComponentId::new(2), ComponentId::new(5)); filter.restrict(component); - assert!(!filter.is_allowed(component, ArchetypeId::new(1))); + assert!(!filter.is_allowed(component, ArchetypeId::new(1), false)); - filter.allow(component, ArchetypeId::new(1)); - filter.allow(component, ArchetypeId::new(130)); - assert!(filter.is_allowed(component, ArchetypeId::new(1))); - assert!(filter.is_allowed(component, ArchetypeId::new(130))); - assert!(!filter.is_allowed(component, ArchetypeId::new(0))); - assert!(!filter.is_allowed(component, ArchetypeId::new(64))); - assert!(filter.is_allowed(other, ArchetypeId::new(0))); + filter.allow(component, ArchetypeId::new(1), true); + filter.allow(component, ArchetypeId::new(130), false); + assert!(filter.is_allowed(component, ArchetypeId::new(1), true)); + assert!(filter.is_allowed(component, ArchetypeId::new(130), false)); + assert!(!filter.is_allowed(component, ArchetypeId::new(130), true)); + assert!(!filter.is_allowed(component, ArchetypeId::new(0), false)); + assert!(!filter.is_allowed(component, ArchetypeId::new(64), false)); + assert!(filter.is_allowed(other, ArchetypeId::new(0), true)); } } diff --git a/crates/bevy_mod_scripting_world/src/world.rs b/crates/bevy_mod_scripting_world/src/world.rs index 64d6bf28f5..6ebf30764e 100644 --- a/crates/bevy_mod_scripting_world/src/world.rs +++ b/crates/bevy_mod_scripting_world/src/world.rs @@ -388,7 +388,7 @@ impl<'w> WorldAccessGuard<'w> { } } - /// Checks the guard's [`ArchetypeFilter`] allows accessing the component on the given entity. + /// Checks the guard's [`ArchetypeFilter`] allows reading, or if `write` writing, the component on the given entity. /// /// Must be checked before handing out references to components, on top of claiming the component's access. /// Missing entities pass, as there is nothing to alias. @@ -397,6 +397,7 @@ impl<'w> WorldAccessGuard<'w> { &self, entity: Entity, component_id: ComponentId, + write: bool, ) -> Result<(), DynWorldAccessError> { let Some(filter) = &self.inner.archetype_filter else { return Ok(()); @@ -405,13 +406,14 @@ impl<'w> WorldAccessGuard<'w> { let Ok(entity_cell) = self.as_unsafe_world_cell()?.get_entity(entity) else { return Ok(()); }; - if filter.is_allowed(component_id, entity_cell.archetype().id()) { + if filter.is_allowed(component_id, entity_cell.archetype().id(), write) { Ok(()) } else { + let access = if write { "writing" } else { "accessing" }; Err(DynWorldAccessError::cannot_claim_access( component_id.into(), format!( - "entity {entity} is not matched by any query of this system accessing the component" + "entity {entity} is not matched by any query of this system {access} the component" ), )) } diff --git a/crates/bindings/bevy_mod_scripting_bevy_ecs_bindings/src/lib.rs b/crates/bindings/bevy_mod_scripting_bevy_ecs_bindings/src/lib.rs index 76b0cb602b..572e7719ab 100644 --- a/crates/bindings/bevy_mod_scripting_bevy_ecs_bindings/src/lib.rs +++ b/crates/bindings/bevy_mod_scripting_bevy_ecs_bindings/src/lib.rs @@ -167,7 +167,7 @@ impl World { let mut query = ScriptQueryBuilder::new(); query.component(component); for result in world.query(query)? { - let Some(name_reference) = result.components.first() else { + let Some(Some(name_reference)) = result.components.first() else { continue; }; let matches = name_reference.with_reflect(world.clone(), |value| { diff --git a/docs/src/ReleaseNotes/0.23-to-0.24.md b/docs/src/ReleaseNotes/0.23-to-0.24.md index 4d34b07cd8..fb59d28e5b 100644 --- a/docs/src/ReleaseNotes/0.23-to-0.24.md +++ b/docs/src/ReleaseNotes/0.23-to-0.24.md @@ -64,6 +64,7 @@ Other changes: - An attachment whose script is still loading holds back the attachments and detachments after it, and callbacks reach scripts in a deterministic order (attachment order, until scripts are removed), see [Determinism](../ScriptPipeline/pipeline.md#determinism). Lua's `math.random` is seeded with `0` in new contexts. - Script systems are identified by attachment and callback label (adding one again replaces it) and are removed with their script. `ScriptSystemSet` holds a recycled slot instead of the callback label, `ScriptSystemBuilder` no longer implements `IntoSystem` (use `ScriptSystemBuilder::build`), and their errors are sent as `ScriptErrorMessage`s instead of being logged. `WorldAccessGuard::new_non_exclusive` takes an `Option>`, restricting component accesses to the given archetypes (`None` for no restriction). - Script system parameters are declared to Bevy as they're accessed: `resource` and query `component`s as writes (they were declared as reads, or as accessing nothing for resources, while scripts could write them), `resource_read` / `component_read` as reads. Script systems writing the same resources or components no longer run in parallel. `ScriptQueryBuilder::as_query_state` takes `declare_writes`, `false` declares all components as read (i.e. for `EntityRef` queries). +- `ScriptQueryResult::components` is a `Vec>`, `None` for absent optional components. `ArchetypeFilter::allow` / `is_allowed` and `WorldAccessGuard::check_archetype_access` take whether the component is written. - `AccessByteSet::from_allowed_lists(read_write, read_only)` creates subsets with read only accesses. At most 63 accesses to the same value can be claimed at once, claims beyond fail (the count used to overflow). - `WorldAccessRange::External` holds the value's owner (`AccessOwner`) instead of an id, and the access map keeps it alive until the claim is released. Build it from the value (`(&allocation_id).into()`); `WorldAccessRange` is no longer `Copy`. diff --git a/docs/src/ScriptSystems/introduction.md b/docs/src/ScriptSystems/introduction.md index cfce2366ff..ed7e1a0d41 100644 --- a/docs/src/ScriptSystems/introduction.md +++ b/docs/src/ScriptSystems/introduction.md @@ -114,6 +114,17 @@ bms.system_builder("my_system", attachment) :resource_read(Time) ``` +Queries can also retrieve components entities don't need to have (`optional`, `optional_read`, nil for entities without them), and only match entities whose components were added or changed since the system last ran (`added`, `changed`, filters like `with`). Outside of systems, `added` and `changed` are relative to the last app update: + +```lua +world.query() + :component(Transform) + :optional_read(Velocity) + :changed(Health) +``` + +Queries of a system may access the same components if they can't match the same entities, i.e. `component(A):with(B)` and `component(A):without(B)`, otherwise the system reports an error and doesn't run. + ## Exclusive systems An exclusive system can be created using the `exclusive` function call on the system builder. From 92c89b27e7eb48c8bcb11aeaa463cf16e1ce9756 Mon Sep 17 00:00:00 2001 From: makspll Date: Fri, 23 Oct 2026 12:10:28 +0000 Subject: [PATCH 2/2] feat: name the conflicting components when script system queries are not disjoint --- .../src/script_system.rs | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/crates/bevy_mod_scripting_core/src/script_system.rs b/crates/bevy_mod_scripting_core/src/script_system.rs index a7881f993e..7e3f271583 100644 --- a/crates/bevy_mod_scripting_core/src/script_system.rs +++ b/crates/bevy_mod_scripting_core/src/script_system.rs @@ -14,7 +14,7 @@ use ::{ bevy_ecs::{ component::ComponentId, entity::Entity, - query::{FilteredAccessSet, QueryBuilder, QueryState}, + query::{AccessConflicts, FilteredAccessSet, QueryBuilder, QueryState}, reflect::AppTypeRegistry, schedule::SystemSet, system::{System, SystemParamValidationError}, @@ -1256,13 +1256,26 @@ impl System for DynamicScriptSystem

{ let query = builder.as_query_state::(world, true); // queries may access the same components if bevy considers them disjoint, i.e. `With` vs `Without` - if !component_access_set - .get_conflicts_single(query.component_access()) - .is_empty() - { - initialization_errors.push( - "A query conflicts with another parameter of the system, i.e. two queries write the same component on the same entities.".to_owned(), - ); + let conflicts = + component_access_set.get_conflicts_single(query.component_access()); + if !conflicts.is_empty() { + let components = match conflicts { + AccessConflicts::All => "all components".to_owned(), + AccessConflicts::Individual(ids) => ids + .iter() + .map(|id| { + world.components().get_name(id).map_or_else( + || format!("{id:?}"), + |name| name.shortname().to_string(), + ) + }) + .collect::>() + .join(", "), + }; + initialization_errors.push(format!( + "Query parameter #{} conflicts with an earlier parameter of the system on: {components}. Use `with`/`without` to make them disjoint.", + system_params.len() + 1 + )); } // The access carries the query filters, so bevy runs this system in parallel with systems accessing