Skip to content

Commit

Permalink
System param for dynamic resources (#15189)
Browse files Browse the repository at this point in the history
# Objective

Support accessing dynamic resources in a dynamic system, including
accessing them by component id. This is similar to how dynamic
components can be queried using `Query<FilteredEntityMut>`.

## Solution

Create `FilteredResources` and `FilteredResourcesMut` types that act
similar to `FilteredEntityRef` and `FilteredEntityMut` and that can be
used as system parameters.

## Example

```rust
// Use `FilteredResourcesParamBuilder` to declare access to resources.
let system = (FilteredResourcesParamBuilder::new(|builder| {
    builder.add_read::<B>().add_read::<C>();
}),)
    .build_state(&mut world)
    .build_system(resource_system);

world.init_resource::<A>();
world.init_resource::<C>();

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());
    // The resource doesn't exist, so we can't read it.
    assert!(res.get::<B>().is_none());
    // 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.
    let c: Res<C> = res.get().unwrap();
}
```

## Future Work

As a follow-up PR, `ReflectResource` can be modified to take `impl
Into<FilteredResources>`, similar to how `ReflectComponent` takes `impl
Into<FilteredEntityRef>`. That will allow dynamic resources to be
accessed using reflection.
  • Loading branch information
chescock authored Oct 3, 2024
1 parent 1e61092 commit 46180a7
Show file tree
Hide file tree
Showing 7 changed files with 1,005 additions and 22 deletions.
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ pub mod prelude {
SystemParamFunction, WithParamWarnPolicy,
},
world::{
Command, EntityMut, EntityRef, EntityWorldMut, FromWorld, OnAdd, OnInsert, OnRemove,
OnReplace, World,
Command, EntityMut, EntityRef, EntityWorldMut, FilteredResources, FilteredResourcesMut,
FromWorld, OnAdd, OnInsert, OnRemove, OnReplace, World,
},
};

Expand Down
53 changes: 53 additions & 0 deletions crates/bevy_ecs/src/query/access.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::component::ComponentId;
use crate::storage::SparseSetIndex;
use crate::world::World;
use core::{fmt, fmt::Debug, marker::PhantomData};
use fixedbitset::FixedBitSet;

Expand Down Expand Up @@ -727,6 +729,25 @@ impl<T: SparseSetIndex> Access<T> {
AccessConflicts::Individual(conflicts)
}

/// Returns the indices of the resources this has access to.
pub fn resource_reads_and_writes(&self) -> impl Iterator<Item = T> + '_ {
self.resource_read_and_writes
.ones()
.map(T::get_sparse_set_index)
}

/// Returns the indices of the resources this has non-exclusive access to.
pub fn resource_reads(&self) -> impl Iterator<Item = T> + '_ {
self.resource_read_and_writes
.difference(&self.resource_writes)
.map(T::get_sparse_set_index)
}

/// Returns the indices of the resources this has exclusive access to.
pub fn resource_writes(&self) -> impl Iterator<Item = T> + '_ {
self.resource_writes.ones().map(T::get_sparse_set_index)
}

/// Returns the indices of the components that this has an archetypal access to.
///
/// These are components whose values are not accessed (and thus will never cause conflicts),
Expand Down Expand Up @@ -863,6 +884,24 @@ impl AccessConflicts {
}
}

pub(crate) fn format_conflict_list(&self, world: &World) -> String {
match self {
AccessConflicts::All => String::new(),
AccessConflicts::Individual(indices) => format!(
" {}",
indices
.ones()
.map(|index| world
.components
.get_info(ComponentId::get_sparse_set_index(index))
.unwrap()
.name())
.collect::<Vec<&str>>()
.join(", ")
),
}
}

/// An [`AccessConflicts`] which represents the absence of any conflict
pub(crate) fn empty() -> Self {
Self::Individual(FixedBitSet::new())
Expand Down Expand Up @@ -1239,6 +1278,20 @@ impl<T: SparseSetIndex> FilteredAccessSet<T> {
self.add(filter);
}

/// Adds read access to all resources to the set.
pub(crate) fn add_unfiltered_read_all_resources(&mut self) {
let mut filter = FilteredAccess::default();
filter.access.read_all_resources();
self.add(filter);
}

/// Adds write access to all resources to the set.
pub(crate) fn add_unfiltered_write_all_resources(&mut self) {
let mut filter = FilteredAccess::default();
filter.access.write_all_resources();
self.add(filter);
}

/// Adds all of the accesses from the passed set to `self`.
pub fn extend(&mut self, filtered_access_set: FilteredAccessSet<T>) {
self.combined_access
Expand Down
263 changes: 262 additions & 1 deletion crates/bevy_ecs/src/system/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ use crate::{
system::{
DynSystemParam, DynSystemParamState, Local, ParamSet, Query, SystemMeta, SystemParam,
},
world::{FromWorld, World},
world::{
FilteredResources, FilteredResourcesBuilder, FilteredResourcesMut,
FilteredResourcesMutBuilder, FromWorld, World,
},
};
use core::fmt::Debug;

Expand Down Expand Up @@ -77,6 +80,10 @@ use super::{init_query_param, Res, ResMut, Resource, SystemState};
///
/// [`LocalBuilder`] can build a [`Local`] to supply the initial value for the `Local`.
///
/// [`FilteredResourcesParamBuilder`] can build a [`FilteredResources`],
/// and [`FilteredResourcesMutParamBuilder`] can build a [`FilteredResourcesMut`],
/// to configure the resources that can be accessed.
///
/// [`DynParamBuilder`] can build a [`DynSystemParam`] to determine the type of the inner parameter,
/// and to supply any `SystemParamBuilder` it needs.
///
Expand Down Expand Up @@ -526,6 +533,147 @@ unsafe impl<'s, T: FromWorld + Send + 'static> SystemParamBuilder<Local<'s, T>>
}
}

/// A [`SystemParamBuilder`] for a [`FilteredResources`].
/// See the [`FilteredResources`] docs for examples.
pub struct FilteredResourcesParamBuilder<T>(T);

impl<T> FilteredResourcesParamBuilder<T> {
/// Creates a [`SystemParamBuilder`] for a [`FilteredResources`] that accepts a callback to configure the [`FilteredResourcesBuilder`].
pub fn new(f: T) -> Self
where
T: FnOnce(&mut FilteredResourcesBuilder),
{
Self(f)
}
}

impl<'a> FilteredResourcesParamBuilder<Box<dyn FnOnce(&mut FilteredResourcesBuilder) + 'a>> {
/// Creates a [`SystemParamBuilder`] for a [`FilteredResources`] that accepts a callback to configure the [`FilteredResourcesBuilder`].
/// This boxes the callback so that it has a common type.
pub fn new_box(f: impl FnOnce(&mut FilteredResourcesBuilder) + 'a) -> Self {
Self(Box::new(f))
}
}

// SAFETY: Resource ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this FilteredResources
// conflicts with any prior access, a panic will occur.
unsafe impl<'w, 's, T: FnOnce(&mut FilteredResourcesBuilder)>
SystemParamBuilder<FilteredResources<'w, 's>> for FilteredResourcesParamBuilder<T>
{
fn build(
self,
world: &mut World,
meta: &mut SystemMeta,
) -> <FilteredResources<'w, 's> as SystemParam>::State {
let mut builder = FilteredResourcesBuilder::new(world);
(self.0)(&mut builder);
let access = builder.build();

let combined_access = meta.component_access_set.combined_access();
let conflicts = combined_access.get_conflicts(&access);
if !conflicts.is_empty() {
let accesses = conflicts.format_conflict_list(world);
let system_name = &meta.name;
panic!("error[B0002]: FilteredResources in system {system_name} accesses resources(s){accesses} in a way that conflicts with a previous system parameter. Consider removing the duplicate access. See: https://bevyengine.org/learn/errors/#b0002");
}

if access.has_read_all_resources() {
meta.component_access_set
.add_unfiltered_read_all_resources();
meta.archetype_component_access.read_all_resources();
} else {
for component_id in access.resource_reads_and_writes() {
meta.component_access_set
.add_unfiltered_resource_read(component_id);

let archetype_component_id = world.initialize_resource_internal(component_id).id();
meta.archetype_component_access
.add_resource_read(archetype_component_id);
}
}

access
}
}

/// A [`SystemParamBuilder`] for a [`FilteredResourcesMut`].
/// See the [`FilteredResourcesMut`] docs for examples.
pub struct FilteredResourcesMutParamBuilder<T>(T);

impl<T> FilteredResourcesMutParamBuilder<T> {
/// Creates a [`SystemParamBuilder`] for a [`FilteredResourcesMut`] that accepts a callback to configure the [`FilteredResourcesMutBuilder`].
pub fn new(f: T) -> Self
where
T: FnOnce(&mut FilteredResourcesMutBuilder),
{
Self(f)
}
}

impl<'a> FilteredResourcesMutParamBuilder<Box<dyn FnOnce(&mut FilteredResourcesMutBuilder) + 'a>> {
/// Creates a [`SystemParamBuilder`] for a [`FilteredResourcesMut`] that accepts a callback to configure the [`FilteredResourcesMutBuilder`].
/// This boxes the callback so that it has a common type.
pub fn new_box(f: impl FnOnce(&mut FilteredResourcesMutBuilder) + 'a) -> Self {
Self(Box::new(f))
}
}

// SAFETY: Resource ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this FilteredResources
// conflicts with any prior access, a panic will occur.
unsafe impl<'w, 's, T: FnOnce(&mut FilteredResourcesMutBuilder)>
SystemParamBuilder<FilteredResourcesMut<'w, 's>> for FilteredResourcesMutParamBuilder<T>
{
fn build(
self,
world: &mut World,
meta: &mut SystemMeta,
) -> <FilteredResourcesMut<'w, 's> as SystemParam>::State {
let mut builder = FilteredResourcesMutBuilder::new(world);
(self.0)(&mut builder);
let access = builder.build();

let combined_access = meta.component_access_set.combined_access();
let conflicts = combined_access.get_conflicts(&access);
if !conflicts.is_empty() {
let accesses = conflicts.format_conflict_list(world);
let system_name = &meta.name;
panic!("error[B0002]: FilteredResourcesMut in system {system_name} accesses resources(s){accesses} in a way that conflicts with a previous system parameter. Consider removing the duplicate access. See: https://bevyengine.org/learn/errors/#b0002");
}

if access.has_read_all_resources() {
meta.component_access_set
.add_unfiltered_read_all_resources();
meta.archetype_component_access.read_all_resources();
} else {
for component_id in access.resource_reads() {
meta.component_access_set
.add_unfiltered_resource_read(component_id);

let archetype_component_id = world.initialize_resource_internal(component_id).id();
meta.archetype_component_access
.add_resource_read(archetype_component_id);
}
}

if access.has_write_all_resources() {
meta.component_access_set
.add_unfiltered_write_all_resources();
meta.archetype_component_access.write_all_resources();
} else {
for component_id in access.resource_writes() {
meta.component_access_set
.add_unfiltered_resource_write(component_id);

let archetype_component_id = world.initialize_resource_internal(component_id).id();
meta.archetype_component_access
.add_resource_write(archetype_component_id);
}
}

access
}
}

#[cfg(test)]
mod tests {
use crate as bevy_ecs;
Expand All @@ -546,6 +694,9 @@ mod tests {
#[derive(Component)]
struct C;

#[derive(Resource, Default)]
struct R;

fn local_system(local: Local<u64>) -> u64 {
*local
}
Expand Down Expand Up @@ -774,4 +925,114 @@ mod tests {
let output = world.run_system_once(system).unwrap();
assert_eq!(output, 101);
}

#[test]
fn filtered_resource_conflicts_read_with_res() {
let mut world = World::new();
(
ParamBuilder::resource(),
FilteredResourcesParamBuilder::new(|builder| {
builder.add_read::<R>();
}),
)
.build_state(&mut world)
.build_system(|_r: Res<R>, _fr: FilteredResources| {});
}

#[test]
#[should_panic]
fn filtered_resource_conflicts_read_with_resmut() {
let mut world = World::new();
(
ParamBuilder::resource_mut(),
FilteredResourcesParamBuilder::new(|builder| {
builder.add_read::<R>();
}),
)
.build_state(&mut world)
.build_system(|_r: ResMut<R>, _fr: FilteredResources| {});
}

#[test]
#[should_panic]
fn filtered_resource_conflicts_read_all_with_resmut() {
let mut world = World::new();
(
ParamBuilder::resource_mut(),
FilteredResourcesParamBuilder::new(|builder| {
builder.add_read_all();
}),
)
.build_state(&mut world)
.build_system(|_r: ResMut<R>, _fr: FilteredResources| {});
}

#[test]
fn filtered_resource_mut_conflicts_read_with_res() {
let mut world = World::new();
(
ParamBuilder::resource(),
FilteredResourcesMutParamBuilder::new(|builder| {
builder.add_read::<R>();
}),
)
.build_state(&mut world)
.build_system(|_r: Res<R>, _fr: FilteredResourcesMut| {});
}

#[test]
#[should_panic]
fn filtered_resource_mut_conflicts_read_with_resmut() {
let mut world = World::new();
(
ParamBuilder::resource_mut(),
FilteredResourcesMutParamBuilder::new(|builder| {
builder.add_read::<R>();
}),
)
.build_state(&mut world)
.build_system(|_r: ResMut<R>, _fr: FilteredResourcesMut| {});
}

#[test]
#[should_panic]
fn filtered_resource_mut_conflicts_write_with_res() {
let mut world = World::new();
(
ParamBuilder::resource(),
FilteredResourcesMutParamBuilder::new(|builder| {
builder.add_write::<R>();
}),
)
.build_state(&mut world)
.build_system(|_r: Res<R>, _fr: FilteredResourcesMut| {});
}

#[test]
#[should_panic]
fn filtered_resource_mut_conflicts_write_all_with_res() {
let mut world = World::new();
(
ParamBuilder::resource(),
FilteredResourcesMutParamBuilder::new(|builder| {
builder.add_write_all();
}),
)
.build_state(&mut world)
.build_system(|_r: Res<R>, _fr: FilteredResourcesMut| {});
}

#[test]
#[should_panic]
fn filtered_resource_mut_conflicts_write_with_resmut() {
let mut world = World::new();
(
ParamBuilder::resource_mut(),
FilteredResourcesMutParamBuilder::new(|builder| {
builder.add_write::<R>();
}),
)
.build_state(&mut world)
.build_system(|_r: ResMut<R>, _fr: FilteredResourcesMut| {});
}
}
2 changes: 2 additions & 0 deletions crates/bevy_ecs/src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@
//! In addition, the following parameters can be used when constructing a dynamic system with [`SystemParamBuilder`],
//! but will only provide an empty value when used with an ordinary system:
//!
//! - [`FilteredResources`](crate::world::FilteredResources)
//! - [`FilteredResourcesMut`](crate::world::FilteredResourcesMut)
//! - [`DynSystemParam`]
//! - [`Vec<P>`] where `P: SystemParam`
//! - [`ParamSet<Vec<P>>`] where `P: SystemParam`
Expand Down
Loading

0 comments on commit 46180a7

Please sign in to comment.