forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

FilteredResource returns a Result instead of a simple Option (#18265)

Browse Source 
# Objective
FilteredResource::get should return a Result instead of Option

Fixes #17480 

---

## Migration Guide

Users will need to handle the different return type on
FilteredResource::get, FilteredResource::get_id,
FilteredResource::get_mut as it is now a Result not an Option.
This commit is contained in:
andristarr 2025-03-17 19:54:13 +01:00 committed by GitHub
parent d229475a3f
commit 2b21b6cc13
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 94 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
crates/bevy_ecs/src/reflect/resource.rs
View File

@ -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<Mut<'w, dyn Reflect>>,
pub reflect_mut: for<'w> fn(
FilteredResourcesMut<'w, '_>,
) -> Result<Mut<'w, dyn Reflect>, ResourceFetchError>,
/// Function pointer implementing [`ReflectResource::reflect_unchecked_mut()`].
///
/// # Safety
@ -118,7 +124,7 @@ impl ReflectResource {
pub fn reflect<'w, 's>(
&self,
resources: impl Into<FilteredResources<'w, 's>>,
) -> 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<FilteredResourcesMut<'w, 's>>,
) -> Option<Mut<'w, dyn Reflect>> {
) -> Result<Mut<'w, dyn Reflect>, ResourceFetchError> {
(self.0.reflect_mut)(resources.into())
}

14
crates/bevy_ecs/src/world/error.rs
View File

@ -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),
}

109
crates/bevy_ecs/src/world/filtered_resource.rs
View File

@ -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::<A>().is_none());
/// assert!(res.get::<A>().is_err());
/// // The resource doesn't exist, so we can't read it.
/// assert!(res.get::<B>().is_none());
/// assert!(res.get::<B>().is_err());
/// // The resource exists and we have access, so we can read it.
/// let c = res.get::<C>().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<R: Resource>(&self) -> bool {
let component_id = self.world.components().resource_id::<R>();
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<R: Resource>(&self) -> Option<Ref<'w, R>> {
let component_id = self.world.components().resource_id::<R>()?;
pub fn get<R: Resource>(&self) -> Result<Ref<'w, R>, ResourceFetchError> {
let component_id = self
.world
.components()
.resource_id::<R>()
.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<Ptr<'w>> {
pub fn get_by_id(&self, component_id: ComponentId) -> Result<Ptr<'w>, 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::<A>().is_none());
/// assert!(res.get_mut::<A>().is_none());
/// assert!(res.get::<A>().is_err());
/// assert!(res.get_mut::<A>().is_err());
/// // The resource doesn't exist, so we can't read it or write it.
/// assert!(res.get::<B>().is_none());
/// assert!(res.get_mut::<B>().is_none());
/// assert!(res.get::<B>().is_err());
/// assert!(res.get_mut::<B>().is_err());
/// // The resource exists and we have read access, so we can read it but not write it.
/// let c = res.get::<C>().unwrap();
/// assert!(res.get_mut::<C>().is_none());
/// assert!(res.get_mut::<C>().is_err());
/// // The resource exists and we have write access, so we can read it or write it.
/// let d = res.get::<D>().unwrap();
/// let d = res.get_mut::<D>().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<R: Resource>(&self) -> bool {
let component_id = self.world.components().resource_id::<R>();
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<R: Resource>(&self) -> bool {
let component_id = self.world.components().resource_id::<R>();
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<R: Resource>(&self) -> Option<Ref<'_, R>> {
pub fn get<R: Resource>(&self) -> Result<Ref<'_, R>, 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<Ptr<'_>> {
pub fn get_by_id(&self, component_id: ComponentId) -> Result<Ptr<'_>, 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<R: Resource>(&mut self) -> Option<Mut<'_, R>> {
pub fn get_mut<R: Resource>(&mut self) -> Result<Mut<'_, R>, 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<MutUntyped<'_>> {
pub fn get_mut_by_id(
&mut self,
component_id: ComponentId,
) -> Result<MutUntyped<'_>, 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<R: Resource>(mut self) -> Option<Mut<'w, R>> {
pub fn into_mut<R: Resource>(mut self) -> Result<Mut<'w, R>, 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<MutUntyped<'w>> {
pub fn into_mut_by_id(
mut self,
component_id: ComponentId,
) -> Result<MutUntyped<'w>, 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<R: Resource>(&mut self) -> Option<Mut<'w, R>> {
let component_id = self.world.components().resource_id::<R>()?;
unsafe fn get_mut_unchecked<R: Resource>(&mut self) -> Result<Mut<'w, R>, ResourceFetchError> {
let component_id = self
.world
.components()
.resource_id::<R>()
.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<MutUntyped<'w>> {
) -> Result<MutUntyped<'w>, 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()) },
})
}
}

4
crates/bevy_remote/src/builtin_methods.rs
View File

@ -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

3
crates/bevy_scene/src/dynamic_scene_builder.rs
View File

@ -376,7 +376,8 @@ impl<'w> DynamicSceneBuilder<'w> {
let resource = type_registration
.data::<ReflectResource>()?
.reflect(self.original_world)?;
.reflect(self.original_world)
.ok()?;
let resource =
clone_reflect_value(resource.as_partial_reflect(), type_registration);