diff --git a/crates/bevy_ecs/src/reflect/resource.rs b/crates/bevy_ecs/src/reflect/resource.rs index 34e593a6ef..60cf7bc609 100644 --- a/crates/bevy_ecs/src/reflect/resource.rs +++ b/crates/bevy_ecs/src/reflect/resource.rs @@ -8,7 +8,10 @@ use crate::{ change_detection::Mut, component::ComponentId, resource::Resource, - world::{unsafe_world_cell::UnsafeWorldCell, FilteredResources, FilteredResourcesMut, World}, + world::{ + error::ResourceFetchError, unsafe_world_cell::UnsafeWorldCell, FilteredResources, + FilteredResourcesMut, World, + }, }; use bevy_reflect::{FromReflect, FromType, PartialReflect, Reflect, TypePath, TypeRegistry}; @@ -52,9 +55,12 @@ pub struct ReflectResourceFns { /// Function pointer implementing [`ReflectResource::remove()`]. pub remove: fn(&mut World), /// Function pointer implementing [`ReflectResource::reflect()`]. - pub reflect: for<'w> fn(FilteredResources<'w, '_>) -> Option<&'w dyn Reflect>, + pub reflect: + for<'w> fn(FilteredResources<'w, '_>) -> Result<&'w dyn Reflect, ResourceFetchError>, /// Function pointer implementing [`ReflectResource::reflect_mut()`]. - pub reflect_mut: for<'w> fn(FilteredResourcesMut<'w, '_>) -> Option>, + pub reflect_mut: for<'w> fn( + FilteredResourcesMut<'w, '_>, + ) -> Result, ResourceFetchError>, /// Function pointer implementing [`ReflectResource::reflect_unchecked_mut()`]. /// /// # Safety @@ -118,7 +124,7 @@ impl ReflectResource { pub fn reflect<'w, 's>( &self, resources: impl Into>, - ) -> Option<&'w dyn Reflect> { + ) -> Result<&'w dyn Reflect, ResourceFetchError> { (self.0.reflect)(resources.into()) } @@ -128,7 +134,7 @@ impl ReflectResource { pub fn reflect_mut<'w, 's>( &self, resources: impl Into>, - ) -> Option> { + ) -> Result, ResourceFetchError> { (self.0.reflect_mut)(resources.into()) } diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index 1542a12851..3527967942 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -55,3 +55,17 @@ pub enum EntityMutableFetchError { #[error("The entity with ID {0} was requested mutably more than once")] AliasedMutability(Entity), } + +/// An error that occurs when getting a resource of a given type in a world. +#[derive(thiserror::Error, Debug, Clone, Copy, PartialEq, Eq)] +pub enum ResourceFetchError { + /// The resource has never been initialized or registered with the world. + #[error("The resource has never been initialized or registered with the world. Did you forget to add it using `app.insert_resource` / `app.init_resource`?")] + NotRegistered, + /// The resource with the given [`ComponentId`] does not currently exist in the world. + #[error("The resource with ID {0:?} does not currently exist in the world.")] + DoesNotExist(ComponentId), + /// Cannot get access to the resource with the given [`ComponentId`] in the world as it conflicts with an on going operation. + #[error("Cannot get access to the resource with ID {0:?} in the world as it conflicts with an on going operation.")] + NoResourceAccess(ComponentId), +} diff --git a/crates/bevy_ecs/src/world/filtered_resource.rs b/crates/bevy_ecs/src/world/filtered_resource.rs index dead18ff11..a9fac308fa 100644 --- a/crates/bevy_ecs/src/world/filtered_resource.rs +++ b/crates/bevy_ecs/src/world/filtered_resource.rs @@ -7,6 +7,8 @@ use crate::{ }; use bevy_ptr::{Ptr, UnsafeCellDeref}; +use super::error::ResourceFetchError; + /// Provides read-only access to a set of [`Resource`]s defined by the contained [`Access`]. /// /// Use [`FilteredResourcesMut`] if you need mutable access to some resources. @@ -42,9 +44,9 @@ use bevy_ptr::{Ptr, UnsafeCellDeref}; /// /// fn resource_system(res: FilteredResources) { /// // The resource exists, but we have no access, so we can't read it. -/// assert!(res.get::().is_none()); +/// assert!(res.get::().is_err()); /// // The resource doesn't exist, so we can't read it. -/// assert!(res.get::().is_none()); +/// assert!(res.get::().is_err()); /// // The resource exists and we have access, so we can read it. /// let c = res.get::().unwrap(); /// // The type parameter can be left out if it can be determined from use. @@ -144,38 +146,45 @@ impl<'w, 's> FilteredResources<'w, 's> { } /// Returns `true` if the `FilteredResources` has access to the given resource. - /// Note that [`Self::get()`] may still return `None` if the resource does not exist. + /// Note that [`Self::get()`] may still return `Err` if the resource does not exist. pub fn has_read(&self) -> bool { let component_id = self.world.components().resource_id::(); component_id.is_some_and(|component_id| self.access.has_resource_read(component_id)) } /// Gets a reference to the resource of the given type if it exists and the `FilteredResources` has access to it. - pub fn get(&self) -> Option> { - let component_id = self.world.components().resource_id::()?; + pub fn get(&self) -> Result, ResourceFetchError> { + let component_id = self + .world + .components() + .resource_id::() + .ok_or(ResourceFetchError::NotRegistered)?; if !self.access.has_resource_read(component_id) { - return None; + return Err(ResourceFetchError::NoResourceAccess(component_id)); } + // SAFETY: We have read access to this resource - unsafe { self.world.get_resource_with_ticks(component_id) }.map(|(value, ticks, caller)| { - Ref { - // SAFETY: `component_id` was obtained from the type ID of `R`. - value: unsafe { value.deref() }, - // SAFETY: We have read access to the resource, so no mutable reference can exist. - ticks: unsafe { Ticks::from_tick_cells(ticks, self.last_run, self.this_run) }, - // SAFETY: We have read access to the resource, so no mutable reference can exist. - changed_by: unsafe { caller.map(|caller| caller.deref()) }, - } + let (value, ticks, caller) = unsafe { self.world.get_resource_with_ticks(component_id) } + .ok_or(ResourceFetchError::DoesNotExist(component_id))?; + + Ok(Ref { + // SAFETY: `component_id` was obtained from the type ID of `R`. + value: unsafe { value.deref() }, + // SAFETY: We have read access to the resource, so no mutable reference can exist. + ticks: unsafe { Ticks::from_tick_cells(ticks, self.last_run, self.this_run) }, + // SAFETY: We have read access to the resource, so no mutable reference can exist. + changed_by: unsafe { caller.map(|caller| caller.deref()) }, }) } /// Gets a pointer to the resource with the given [`ComponentId`] if it exists and the `FilteredResources` has access to it. - pub fn get_by_id(&self, component_id: ComponentId) -> Option> { + pub fn get_by_id(&self, component_id: ComponentId) -> Result, ResourceFetchError> { if !self.access.has_resource_read(component_id) { - return None; + return Err(ResourceFetchError::NoResourceAccess(component_id)); } // SAFETY: We have read access to this resource unsafe { self.world.get_resource_by_id(component_id) } + .ok_or(ResourceFetchError::DoesNotExist(component_id)) } } @@ -279,14 +288,14 @@ impl<'w> From<&'w mut World> for FilteredResources<'w, 'static> { /// /// fn resource_system(mut res: FilteredResourcesMut) { /// // The resource exists, but we have no access, so we can't read it or write it. -/// assert!(res.get::().is_none()); -/// assert!(res.get_mut::().is_none()); +/// assert!(res.get::().is_err()); +/// assert!(res.get_mut::().is_err()); /// // The resource doesn't exist, so we can't read it or write it. -/// assert!(res.get::().is_none()); -/// assert!(res.get_mut::().is_none()); +/// assert!(res.get::().is_err()); +/// assert!(res.get_mut::().is_err()); /// // The resource exists and we have read access, so we can read it but not write it. /// let c = res.get::().unwrap(); -/// assert!(res.get_mut::().is_none()); +/// assert!(res.get_mut::().is_err()); /// // The resource exists and we have write access, so we can read it or write it. /// let d = res.get::().unwrap(); /// let d = res.get_mut::().unwrap(); @@ -405,49 +414,55 @@ impl<'w, 's> FilteredResourcesMut<'w, 's> { } /// Returns `true` if the `FilteredResources` has read access to the given resource. - /// Note that [`Self::get()`] may still return `None` if the resource does not exist. + /// Note that [`Self::get()`] may still return `Err` if the resource does not exist. pub fn has_read(&self) -> bool { let component_id = self.world.components().resource_id::(); component_id.is_some_and(|component_id| self.access.has_resource_read(component_id)) } /// Returns `true` if the `FilteredResources` has write access to the given resource. - /// Note that [`Self::get_mut()`] may still return `None` if the resource does not exist. + /// Note that [`Self::get_mut()`] may still return `Err` if the resource does not exist. pub fn has_write(&self) -> bool { let component_id = self.world.components().resource_id::(); component_id.is_some_and(|component_id| self.access.has_resource_write(component_id)) } /// Gets a reference to the resource of the given type if it exists and the `FilteredResources` has access to it. - pub fn get(&self) -> Option> { + pub fn get(&self) -> Result, ResourceFetchError> { self.as_readonly().get() } /// Gets a pointer to the resource with the given [`ComponentId`] if it exists and the `FilteredResources` has access to it. - pub fn get_by_id(&self, component_id: ComponentId) -> Option> { + pub fn get_by_id(&self, component_id: ComponentId) -> Result, ResourceFetchError> { self.as_readonly().get_by_id(component_id) } /// Gets a mutable reference to the resource of the given type if it exists and the `FilteredResources` has access to it. - pub fn get_mut(&mut self) -> Option> { + pub fn get_mut(&mut self) -> Result, ResourceFetchError> { // SAFETY: We have exclusive access to the resources in `access` for `'_`, and we shorten the returned lifetime to that. unsafe { self.get_mut_unchecked() } } /// Gets a mutable pointer to the resource with the given [`ComponentId`] if it exists and the `FilteredResources` has access to it. - pub fn get_mut_by_id(&mut self, component_id: ComponentId) -> Option> { + pub fn get_mut_by_id( + &mut self, + component_id: ComponentId, + ) -> Result, ResourceFetchError> { // SAFETY: We have exclusive access to the resources in `access` for `'_`, and we shorten the returned lifetime to that. unsafe { self.get_mut_by_id_unchecked(component_id) } } /// Consumes self and gets mutable access to resource of the given type with the world `'w` lifetime if it exists and the `FilteredResources` has access to it. - pub fn into_mut(mut self) -> Option> { + pub fn into_mut(mut self) -> Result, ResourceFetchError> { // SAFETY: This consumes self, so we have exclusive access to the resources in `access` for the entirety of `'w`. unsafe { self.get_mut_unchecked() } } /// Consumes self and gets mutable access to resource with the given [`ComponentId`] with the world `'w` lifetime if it exists and the `FilteredResources` has access to it. - pub fn into_mut_by_id(mut self, component_id: ComponentId) -> Option> { + pub fn into_mut_by_id( + mut self, + component_id: ComponentId, + ) -> Result, ResourceFetchError> { // SAFETY: This consumes self, so we have exclusive access to the resources in `access` for the entirety of `'w`. unsafe { self.get_mut_by_id_unchecked(component_id) } } @@ -455,8 +470,12 @@ impl<'w, 's> FilteredResourcesMut<'w, 's> { /// Gets a mutable pointer to the resource of the given type if it exists and the `FilteredResources` has access to it. /// # Safety /// It is the callers responsibility to ensure that there are no conflicting borrows of anything in `access` for the duration of the returned value. - unsafe fn get_mut_unchecked(&mut self) -> Option> { - let component_id = self.world.components().resource_id::()?; + unsafe fn get_mut_unchecked(&mut self) -> Result, ResourceFetchError> { + let component_id = self + .world + .components() + .resource_id::() + .ok_or(ResourceFetchError::NotRegistered)?; // SAFETY: THe caller ensures that there are no conflicting borrows. unsafe { self.get_mut_by_id_unchecked(component_id) } // SAFETY: The underlying type of the resource is `R`. @@ -469,20 +488,22 @@ impl<'w, 's> FilteredResourcesMut<'w, 's> { unsafe fn get_mut_by_id_unchecked( &mut self, component_id: ComponentId, - ) -> Option> { + ) -> Result, ResourceFetchError> { if !self.access.has_resource_write(component_id) { - return None; + return Err(ResourceFetchError::NoResourceAccess(component_id)); } - // SAFETY: We have access to this resource in `access`, and the caller ensures that there are no conflicting borrows for the duration of the returned value. - unsafe { self.world.get_resource_with_ticks(component_id) }.map(|(value, ticks, caller)| { - MutUntyped { - // SAFETY: We have exclusive access to the underlying storage. - value: unsafe { value.assert_unique() }, - // SAFETY: We have exclusive access to the underlying storage. - ticks: unsafe { TicksMut::from_tick_cells(ticks, self.last_run, self.this_run) }, - // SAFETY: We have exclusive access to the underlying storage. - changed_by: unsafe { caller.map(|caller| caller.deref_mut()) }, - } + + // SAFETY: We have read access to this resource + let (value, ticks, caller) = unsafe { self.world.get_resource_with_ticks(component_id) } + .ok_or(ResourceFetchError::DoesNotExist(component_id))?; + + Ok(MutUntyped { + // SAFETY: We have exclusive access to the underlying storage. + value: unsafe { value.assert_unique() }, + // SAFETY: We have exclusive access to the underlying storage. + ticks: unsafe { TicksMut::from_tick_cells(ticks, self.last_run, self.this_run) }, + // SAFETY: We have exclusive access to the underlying storage. + changed_by: unsafe { caller.map(|caller| caller.deref_mut()) }, }) } } diff --git a/crates/bevy_remote/src/builtin_methods.rs b/crates/bevy_remote/src/builtin_methods.rs index b42cf41055..60c96dcb8a 100644 --- a/crates/bevy_remote/src/builtin_methods.rs +++ b/crates/bevy_remote/src/builtin_methods.rs @@ -511,7 +511,7 @@ pub fn process_remote_get_resource_request( let reflect_resource = get_reflect_resource(&type_registry, &resource_path).map_err(BrpError::resource_error)?; - let Some(reflected) = reflect_resource.reflect(world) else { + let Ok(reflected) = reflect_resource.reflect(world) else { return Err(BrpError::resource_not_present(&resource_path)); }; @@ -978,7 +978,7 @@ pub fn process_remote_mutate_resource_request( // Get the actual resource value from the world as a `dyn Reflect`. let mut reflected_resource = reflect_resource .reflect_mut(world) - .ok_or_else(|| BrpError::resource_not_present(&resource_path))?; + .map_err(|_| BrpError::resource_not_present(&resource_path))?; // Get the type registration for the field with the given path. let value_registration = type_registry diff --git a/crates/bevy_scene/src/dynamic_scene_builder.rs b/crates/bevy_scene/src/dynamic_scene_builder.rs index 6742a32ded..5deac09aaa 100644 --- a/crates/bevy_scene/src/dynamic_scene_builder.rs +++ b/crates/bevy_scene/src/dynamic_scene_builder.rs @@ -376,7 +376,8 @@ impl<'w> DynamicSceneBuilder<'w> { let resource = type_registration .data::()? - .reflect(self.original_world)?; + .reflect(self.original_world) + .ok()?; let resource = clone_reflect_value(resource.as_partial_reflect(), type_registration);