chore: clean up some logic and optimize

Cargo.toml

@@ -258,7 +258,6 @@ bevy = { workspace = true, features = [
     "bevy_ui",
     "default_font",
     "custom_cursor",
-    "debug",
 ] }
 bevy_platform = { workspace = true }
 clap = { workspace = true, features = ["derive"] }

crates/bevy_mod_scripting_bindings/src/allocator.rs

@@ -19,8 +19,7 @@ use std::{
     cmp::Ordering,
     fmt::{Display, Formatter},
     hash::Hasher,
-    num::NonZero,
-    sync::{Arc, atomic::AtomicU64},
+    sync::{Arc, atomic::AtomicUsize},
 };
 
 /// The path used for the total number of allocations diagnostic
@@ -34,15 +33,11 @@ pub const ALLOCATOR_TOTAL_COLLECTED_DIAG_PATH: DiagnosticPath =
 /// Unique identifier for an allocation
 #[derive(Clone, DebugWithTypeInfo)]
 #[debug_with_type_info(bms_display_path = "bevy_mod_scripting_display")]
-pub struct ReflectAllocationId(pub(crate) Arc<u64>);
+pub struct ReflectAllocationId(pub(crate) Arc<usize>);
 
 impl From<&ReflectAllocationId> for WorldAccessRange {
     fn from(val: &ReflectAllocationId) -> Self {
-        WorldAccessRange::External(unsafe {
-            // Safety: trivially 1 or more
-            // if we run out of u64's we have much bigger problems
-            NonZero::new_unchecked(val.0.checked_add(1).unwrap_or(1))
-        })
+        WorldAccessRange::External(*val.0)
     }
 }
 
@@ -58,12 +53,12 @@ impl DisplayWithTypeInfo for ReflectAllocationId {
 
 impl ReflectAllocationId {
     /// Returns the id of the allocation
-    pub fn id(&self) -> u64 {
+    pub fn id(&self) -> usize {
         *self.0
     }
 
     /// Creates a new [`ReflectAllocationId`] from its id
-    pub(crate) fn new(id: u64) -> Self {
+    pub(crate) fn new(id: usize) -> Self {
         Self(Arc::new(id))
     }
 
@@ -215,7 +210,7 @@ impl ReflectAllocator {
 
     /// Allocates a new boxed `PartialReflect` value and returns an [`ReflectAllocationId`] which can be used to access it later.
     pub fn allocate_boxed(&mut self, value: Box<dyn PartialReflect>) -> ReflectAllocationId {
-        static COUNTER: AtomicU64 = AtomicU64::new(0);
+        static COUNTER: AtomicUsize = AtomicUsize::new(0);
 
         let id =
             ReflectAllocationId::new(COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed));

crates/bevy_mod_scripting_bindings/src/error.rs

@@ -13,7 +13,7 @@ use bevy_mod_scripting_world::{
     DynWorldAccessError, WorldAccessGuard, WorldAccessRange, WorldGuard,
 };
 use bevy_reflect::{ApplyError, PartialReflect, Reflect};
-use std::{any::TypeId, borrow::Cow, error::Error, fmt::Display, panic::Location, sync::Arc};
+use std::{any::TypeId, borrow::Cow, error::Error, fmt::Display, sync::Arc};
 
 /// A wrapper around a reflect value to implement various traits useful for error reporting.
 #[derive(Clone)]
@@ -70,9 +70,7 @@ impl From<DynWorldAccessError> for InteropError {
     fn from(value: DynWorldAccessError) -> Self {
         match value {
             DynWorldAccessError::MissingWorld => Self::MissingWorld,
-            DynWorldAccessError::CannotClaimAccess(key, location, msg) => {
-                Self::cannot_claim_access(key, location, msg)
-            }
+            DynWorldAccessError::CannotClaimAccess(key, msg) => Self::cannot_claim_access(key, msg),
             DynWorldAccessError::UnregisteredResource(type_id)
             | DynWorldAccessError::UnregisteredComponent(type_id) => {
                 Self::unregistered_component_or_resource_type(type_id)
@@ -93,34 +91,31 @@ impl DisplayWithTypeInfo for WorldAccessRangeWithDisplay {
     ) -> std::fmt::Result {
         if let Some(provider) = type_info_provider {
             match self.0 {
-                WorldAccessRange::ComponentOrResource(component_range) => {
+                WorldAccessRange::ComponentOrResource(component_id) => {
                     f.write_str("Component or Resource: ")?;
-                    let component_id: ComponentId = component_range.into();
+                    let component_id: ComponentId = component_id;
                     write!(
                         f,
                         "{}",
                         WithTypeInfo::new_with_info(&component_id, provider)
                     )
                 }
-                WorldAccessRange::External(non_zero) => {
+                WorldAccessRange::External(idx) => {
                     f.write_str("Allocation to: ")?;
 
                     write!(
                         f,
                         "{}",
-                        WithTypeInfo::new_with_info(
-                            &ReflectAllocationId::new(non_zero.get()),
-                            provider
-                        )
+                        WithTypeInfo::new_with_info(&ReflectAllocationId::new(idx), provider)
                     )
                 }
                 WorldAccessRange::Global => f.write_str("World Access"),
             }
         } else {
             match self.0 {
-                WorldAccessRange::ComponentOrResource(component_range) => {
+                WorldAccessRange::ComponentOrResource(component_id) => {
                     f.write_str("Component or Resource: ")?;
-                    let component_id: ComponentId = component_range.into();
+                    let component_id: ComponentId = component_id;
                     f.write_str(&component_id.index().to_string())
                 }
                 WorldAccessRange::External(non_zero) => {
@@ -193,8 +188,6 @@ pub enum InteropError {
     CannotClaimAccess {
         /// The id of the access
         base: Box<WorldAccessRangeWithDisplay>,
-        /// The location of the access
-        location: Box<Option<Location<'static>>>,
         /// The context of the access
         context: Box<Cow<'static, str>>,
     },
@@ -384,12 +377,10 @@ impl InteropError {
     /// Creates a new cannot claim access error.
     pub fn cannot_claim_access(
         base: WorldAccessRange,
-        location: Option<Location<'static>>,
         context: impl Into<Cow<'static, str>>,
     ) -> Self {
         Self::CannotClaimAccess {
             base: Box::new(WorldAccessRangeWithDisplay(base)),
-            location: Box::new(location),
             context: Box::new(context.into()),
         }
     }
@@ -591,29 +582,13 @@ impl DisplayWithTypeInfo for InteropError {
                     WithTypeInfo::new_with_opt_info(&got.as_ref().or_fake_id(), type_info_provider)
                 )
             }
-            InteropError::CannotClaimAccess {
-                base,
-                location,
-                context,
-            } => {
-                if let Some(location) = location.as_ref() {
-                    write!(
-                        f,
-                        "Cannot claim access to {} at {}:{}:{}: {}",
-                        WithTypeInfo::new_with_opt_info(base, type_info_provider),
-                        location.file(),
-                        location.line(),
-                        location.column(),
-                        context
-                    )
-                } else {
-                    write!(
-                        f,
-                        "Cannot claim access to {}: {}",
-                        WithTypeInfo::new_with_opt_info(base, type_info_provider),
-                        context
-                    )
-                }
+            InteropError::CannotClaimAccess { base, context } => {
+                write!(
+                    f,
+                    "Cannot claim access to {}: {}",
+                    WithTypeInfo::new_with_opt_info(base, type_info_provider),
+                    context
+                )
             }
             InteropError::Invariant(i) => {
                 write!(f, "Invariant broken: {i}")

crates/bevy_mod_scripting_bindings/src/function/from.rs

@@ -274,23 +274,21 @@ impl<T: FromReflect> FromScript for R<'_, T> {
         match value {
             ScriptValue::Reference(reflect_reference) => {
                 let raid: WorldAccessRange = (&reflect_reference.base.base_id).into();
-                match world.claim_read_access(raid) {
-                    Ok(()) => {
-                        // Safety: we just claimed access
-                        let ref_ = unsafe { reflect_reference.reflect_unsafe_non_empty(world) }?;
-                        let cast = ref_.try_downcast_ref::<T>().ok_or_else(|| {
-                            InteropError::type_mismatch(
-                                std::any::TypeId::of::<T>(),
-                                ref_.get_represented_type_info().map(|i| i.type_id()),
-                            )
-                        })?;
-                        Ok(R(cast))
-                    }
-                    Err(access) => Err(InteropError::cannot_claim_access(
+                if world.claim_read_access(raid) {
+                    // Safety: we just claimed access
+                    let ref_ = unsafe { reflect_reference.reflect_unsafe_non_empty(world) }?;
+                    let cast = ref_.try_downcast_ref::<T>().ok_or_else(|| {
+                        InteropError::type_mismatch(
+                            std::any::TypeId::of::<T>(),
+                            ref_.get_represented_type_info().map(|i| i.type_id()),
+                        )
+                    })?;
+                    Ok(R(cast))
+                } else {
+                    Err(InteropError::cannot_claim_access(
                         raid,
-                        Some(access.owner.location),
                         format!("In conversion to type: R<{}>", std::any::type_name::<T>()),
-                    )),
+                    ))
                 }
             }
             _ => Err(InteropError::value_mismatch(
@@ -348,22 +346,19 @@ impl<T: FromReflect> FromScript for M<'_, T> {
             ScriptValue::Reference(reflect_reference) => {
                 let raid: WorldAccessRange = (&reflect_reference.base.base_id).into();
 
-                match world.claim_write_access(raid) {
-                    Ok(()) => {
-                        // Safety: we just claimed write access
-                        let ref_ =
-                            unsafe { reflect_reference.reflect_mut_unsafe_non_empty(world) }?;
-                        let type_id = ref_.get_represented_type_info().map(|i| i.type_id());
-                        let cast = ref_.try_downcast_mut::<T>().ok_or_else(|| {
-                            InteropError::type_mismatch(std::any::TypeId::of::<T>(), type_id)
-                        })?;
-                        Ok(M(cast))
-                    }
-                    Err(access) => Err(InteropError::cannot_claim_access(
+                if world.claim_write_access(raid) {
+                    // Safety: we just claimed write access
+                    let ref_ = unsafe { reflect_reference.reflect_mut_unsafe_non_empty(world) }?;
+                    let type_id = ref_.get_represented_type_info().map(|i| i.type_id());
+                    let cast = ref_.try_downcast_mut::<T>().ok_or_else(|| {
+                        InteropError::type_mismatch(std::any::TypeId::of::<T>(), type_id)
+                    })?;
+                    Ok(M(cast))
+                } else {
+                    Err(InteropError::cannot_claim_access(
                         raid,
-                        Some(access.owner.location),
                         format!("In conversion to type: Mut<{}>", std::any::type_name::<T>()),
-                    )),
+                    ))
                 }
             }
             _ => Err(InteropError::value_mismatch(

crates/bevy_mod_scripting_bindings/src/reference.rs

@@ -366,7 +366,7 @@ impl ReflectReference {
                 .remove(id)
                 .ok_or_else(|| InteropError::garbage_collected_allocation(self.clone()))?;
 
-            if let Ok(()) = world.claim_write_access(id) {
+            if world.claim_write_access(id) {
                 // Safety: we claim write access, nobody else is accessing this
                 if unsafe { &*arc.get_ptr() }.try_as_reflect().is_some() {
                     // Safety: the only accesses exist in this function
@@ -790,7 +790,7 @@ impl From<&ReflectBase> for WorldAccessRange {
             ReflectBase::Component(_, component_id)
             | ReflectBase::Resource(component_id)
             | ReflectBase::Asset(_, component_id) => {
-                WorldAccessRange::ComponentOrResource((*component_id).into())
+                WorldAccessRange::ComponentOrResource(*component_id)
             }
             ReflectBase::Owned(reflect_allocation_id) => reflect_allocation_id.into(),
         }

crates/bevy_mod_scripting_core/src/extractors.rs

@@ -8,7 +8,6 @@ use bevy_ecs::{
     storage::SparseSetIndex,
 };
 
-use bevy_mod_scripting_world::WorldAccessRange;
 use fixedbitset::FixedBitSet;
 
 // /// A wrapper around a world which pre-populates access, to safely co-exist with other system params,
@@ -140,7 +139,7 @@ fn individual_conflicts(conflicts: AccessConflicts) -> FixedBitSet {
     }
 }
 
-pub(crate) fn get_all_access_ids(access: &Access) -> Vec<(WorldAccessRange, bool)> {
+pub(crate) fn get_all_access_ids(access: &Access) -> Vec<(ComponentId, bool)> {
     let mut access_all_read = Access::default();
     access_all_read.read_all();
 
@@ -157,10 +156,10 @@ pub(crate) fn get_all_access_ids(access: &Access) -> Vec<(WorldAccessRange, bool
 
     let mut result = Vec::new();
     for c in read.ones() {
-        result.push((ComponentId::get_sparse_set_index(c).into(), false));
+        result.push((ComponentId::get_sparse_set_index(c), false));
     }
     for c in written.ones() {
-        result.push((ComponentId::get_sparse_set_index(c).into(), true));
+        result.push((ComponentId::get_sparse_set_index(c), true));
     }
 
     result

crates/bevy_mod_scripting_core/src/script_system.rs

@@ -30,7 +30,7 @@ use bevy_mod_scripting_bindings::{
     ScriptQueryBuilder, ScriptQueryResult, ScriptResourceRegistration, V, WorldExtensions,
 };
 use bevy_mod_scripting_script::ScriptAttachment;
-use bevy_mod_scripting_world::{WorldAccessGuard, WorldAccessRange, WorldGuard};
+use bevy_mod_scripting_world::{AccessByteSet, WorldAccessGuard, WorldGuard};
 use bevy_reflect::TypeRegistryArc;
 use bevy_system_reflection::{ReflectSchedule, ReflectSystem};
 use bevy_utils::prelude::DebugName;
@@ -185,7 +185,7 @@ struct ScriptSystemState<P: IntoScriptPluginParams> {
     schedule_registry: AppScheduleRegistry,
     component_registry: AppScriptComponentRegistry,
     allocator: AppReflectAllocator,
-    subset: HashSet<WorldAccessRange>,
+    subset: AccessByteSet,
     callback_label: CallbackLabel,
     system_params: Vec<ScriptSystemParam>,
     script_contexts: ScriptContexts<P>,
@@ -263,10 +263,6 @@ impl<P: IntoScriptPluginParams> System for DynamicScriptSystem<P> {
         }
     }
 
-    // fn component_access(&self) -> &Access {
-    //     self.component_access_set.combined_access()
-    // }
-
     unsafe fn run_unsafe(
         &mut self,
         _input: SystemIn<'_, Self>,
@@ -391,7 +387,7 @@ impl<P: IntoScriptPluginParams> System for DynamicScriptSystem<P> {
         // - queries, more difficult the queries need to be built, and archetype access registered on top of component access
 
         // start with resources
-        let mut subset = HashSet::default();
+        let mut subset = HashSet::<ComponentId>::new();
         let mut system_params = Vec::with_capacity(self.system_param_descriptors.len());
         let mut component_access_set = FilteredAccessSet::new();
         for param in &self.system_param_descriptors {
@@ -410,15 +406,14 @@ impl<P: IntoScriptPluginParams> System for DynamicScriptSystem<P> {
 
                     access.add_resource_write(component_id);
                     component_access_set.add(access);
-                    let raid: WorldAccessRange = component_id.into();
                     #[allow(
                         clippy::panic,
                         reason = "WIP, to be dealt with in validate params better, but panic will still remain"
                     )]
-                    if subset.contains(&raid) {
-                        panic!("Duplicate resource access in system: {raid:?}.");
+                    if subset.contains(&component_id) {
+                        panic!("Duplicate resource access in system: {component_id:?}.");
                     }
-                    subset.insert(raid);
+                    subset.insert(component_id);
                 }
                 ScriptSystemParamDescriptor::EntityQuery(query) => {
                     let components: Vec<_> = query
@@ -453,6 +448,9 @@ impl<P: IntoScriptPluginParams> System for DynamicScriptSystem<P> {
             }
         }
 
+        let final_subset =
+            AccessByteSet::from_allowed_list(&subset.iter().map(|c| c.index()).collect::<Vec<_>>());
+
         self.state = Some(ScriptSystemState {
             type_registry: world.get_resource_or_init::<AppTypeRegistry>().clone().0,
             function_registry: world
@@ -463,7 +461,7 @@ impl<P: IntoScriptPluginParams> System for DynamicScriptSystem<P> {
             component_registry: world
                 .get_resource_or_init::<AppScriptComponentRegistry>()
                 .clone(),
-            subset,
+            subset: final_subset,
             callback_label: self.name.to_string().into(),
             system_params,
             script_contexts: world.get_resource_or_init::<ScriptContexts<P>>().clone(),