Skip to content

Automatically add systems to sets based on their world accesses #8595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 54 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
cf88758
allow system sets that are automatically added based on world accesses
joseph-gio Mar 1, 2023
cf0453c
add extension methods for making an access set
joseph-gio Mar 1, 2023
af6f6cf
add a test case
joseph-gio Mar 1, 2023
3b374e2
don't re-initialize components made via descriptors
joseph-gio Mar 1, 2023
dbef14c
ensure access set components are initialized
joseph-gio Mar 1, 2023
8a59837
fix ci
joseph-gio Mar 1, 2023
3855257
simplify test
joseph-gio Mar 1, 2023
77d6f31
add a test case for immutable access sets
joseph-gio Mar 1, 2023
0e314f2
add extension methods for resource access sets
joseph-gio Mar 1, 2023
ad50b81
track whether a component descriptor is a resource
joseph-gio Mar 1, 2023
6742f7e
add a test case for resource access sets
joseph-gio Mar 1, 2023
345f5f7
Merge remote-tracking branch 'upstream/main' into access-system-sets
joseph-gio Mar 3, 2023
8c3377e
Merge remote-tracking branch 'upstream/main' into access-system-sets
joseph-gio May 10, 2023
d046de1
merge errors
joseph-gio May 10, 2023
b97e6d8
Update lib.rs
joseph-gio May 10, 2023
e381a9d
use primitive types instead of extension methods
joseph-gio May 10, 2023
18c1e39
disallow outside configuration
joseph-gio May 10, 2023
7417f19
use a macro to deduplicate structs
joseph-gio May 10, 2023
e507e0b
clarify behavior of read sets
joseph-gio May 11, 2023
80e6669
add sets for reads or writes
joseph-gio May 11, 2023
46a463e
Merge remote-tracking branch 'upstream/main' into access-system-sets
joseph-gio May 11, 2023
4c1966d
store all resources in tables
joseph-gio May 11, 2023
7c0458f
remove useless system sets
joseph-gio May 11, 2023
0f15e93
simplify loops for set population
joseph-gio May 11, 2023
eab5712
make access sets consistent
joseph-gio May 11, 2023
20fea21
simplify `ComponentDescriptor::new_with_layout`
joseph-gio May 11, 2023
b58b7bc
don't use a moved value
joseph-gio May 11, 2023
d6d0044
simplify access set initialization
joseph-gio May 11, 2023
4320711
fix a borrow checker error
joseph-gio May 11, 2023
50c7c68
remove an unused method
joseph-gio May 11, 2023
8e4ae7f
improve ergonomics for built-in access sets
joseph-gio May 11, 2023
f6292fb
unify access set initialization with regular initialization
joseph-gio May 11, 2023
c55e078
remove an unused macro arm
joseph-gio May 11, 2023
c90476d
don't initialize access sets multiple times
joseph-gio May 11, 2023
55ca2e4
Merge remote-tracking branch 'upstream/main' into access-system-sets
joseph-gio May 11, 2023
0ab0813
make built-in sets non-exhaustive
joseph-gio May 11, 2023
d913308
use an enum for greater code clarity
joseph-gio May 11, 2023
cd3d4be
fix wonky formatting
joseph-gio May 11, 2023
1774469
remove a check
joseph-gio May 11, 2023
05ede9f
remove a comment
joseph-gio May 11, 2023
1b8c586
add `SystemSet::is_access_set`
joseph-gio May 11, 2023
20902dc
make it impossible to construct _Marker enum variants
joseph-gio May 11, 2023
280b963
tweak `add_set`
joseph-gio May 12, 2023
791953b
store access sets in a SparseSet
joseph-gio May 15, 2023
c56d9ec
simplify an option pattern match
joseph-gio May 15, 2023
4d40962
Merge remote-tracking branch 'upstream/main' into access-system-sets
joseph-gio May 16, 2023
1c2e582
add a unit test for `&World` behavior
joseph-gio May 19, 2023
968a0a3
don't include `&World` in read access sets
joseph-gio May 19, 2023
f66f09c
add a unit test for access sets added after a system is initialized
joseph-gio May 19, 2023
b4b8027
add a fixme
joseph-gio May 19, 2023
276ad90
fix a bug with sets added after systems are initialized
joseph-gio May 31, 2023
723ae2a
Merge branch 'main' into access-system-sets
joseph-gio Sep 22, 2023
47854c6
Schedule::default
joseph-gio Sep 22, 2023
5fb8033
cargo fmt
joseph-gio Sep 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion crates/bevy_ecs/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ pub struct ComponentDescriptor {
// actually Send + Sync
is_send_and_sync: bool,
type_id: Option<TypeId>,
is_resource: bool,
layout: Layout,
// SAFETY: this function must be safe to call with pointers pointing to items of the type
// this descriptor describes.
Expand Down Expand Up @@ -357,6 +358,7 @@ impl ComponentDescriptor {
storage_type: T::Storage::STORAGE_TYPE,
is_send_and_sync: true,
type_id: Some(TypeId::of::<T>()),
is_resource: false,
layout: Layout::new::<T>(),
drop: needs_drop::<T>().then_some(Self::drop_ptr::<T> as _),
}
Expand All @@ -378,6 +380,7 @@ impl ComponentDescriptor {
storage_type,
is_send_and_sync: true,
type_id: None,
is_resource: false,
Copy link
Member

@alice-i-cecile alice-i-cecile Oct 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike new, there's no requirement that this method only be used for component types. I think we should pass in an argument to let users control that field directly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I can add an argument for that. Seems like every one of my PRs is bound to have at least one breaking change at this point 😄

layout,
drop,
}
Expand All @@ -394,6 +397,7 @@ impl ComponentDescriptor {
storage_type: StorageType::Table,
is_send_and_sync: true,
type_id: Some(TypeId::of::<T>()),
is_resource: true,
layout: Layout::new::<T>(),
drop: needs_drop::<T>().then_some(Self::drop_ptr::<T> as _),
}
Expand All @@ -405,6 +409,7 @@ impl ComponentDescriptor {
storage_type,
is_send_and_sync: false,
type_id: Some(TypeId::of::<T>()),
is_resource: true,
layout: Layout::new::<T>(),
drop: needs_drop::<T>().then_some(Self::drop_ptr::<T> as _),
}
Expand Down Expand Up @@ -468,7 +473,18 @@ impl Components {
storages: &mut Storages,
descriptor: ComponentDescriptor,
) -> ComponentId {
let index = Components::init_component_inner(&mut self.components, storages, descriptor);
let index = if let Some(type_id) = descriptor.type_id {
let indices = if descriptor.is_resource {
&mut self.resource_indices
} else {
&mut self.indices
};
*indices.entry(type_id).or_insert_with(|| {
Components::init_component_inner(&mut self.components, storages, descriptor)
})
} else {
Components::init_component_inner(&mut self.components, storages, descriptor)
};
ComponentId(index)
}

Expand Down
129 changes: 129 additions & 0 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ type TypeIdMap<V> = rustc_hash::FxHashMap<TypeId, V>;
mod tests {
use crate as bevy_ecs;
use crate::prelude::Or;
use crate::schedule::{
ComponentAccessSet, IntoSystemConfigs, ResourceAccessSet, Schedule, ScheduleBuildSettings,
};
use crate::system::{Query, Res, ResMut};
use crate::{
bundle::Bundle,
change_detection::Ref,
Expand Down Expand Up @@ -1725,4 +1729,129 @@ mod tests {
"new entity was spawned and received C component"
);
}

#[test]
fn read_component_with_access_set() {
#[derive(Component)]
struct Conflict;

fn before_read(_: Query<&mut Conflict>) {}
fn does_read(_: Query<&A>, _: Query<&mut Conflict>) {}

let mut world = World::new();
let mut schedule = Schedule::default();

schedule.add_systems((
before_read.before(ComponentAccessSet::Reads::<A>),
does_read,
));

// Panic if there are ambiguities.
schedule.set_build_settings(ScheduleBuildSettings {
ambiguity_detection: bevy_ecs::schedule::LogLevel::Error,
..Default::default()
});

schedule.run(&mut world);
}

#[test]
fn write_component_with_access_set() {
fn does_write(_q: Query<&mut A>) {}
fn after_write(_q: Query<&A>) {}

let mut world = World::new();
let mut schedule = Schedule::default();

schedule.add_systems(does_write);
schedule.add_systems(after_write.after(ComponentAccessSet::Writes::<A>));

// Panic if there are ambiguities.
schedule.set_build_settings(ScheduleBuildSettings {
ambiguity_detection: bevy_ecs::schedule::LogLevel::Error,
..Default::default()
});

schedule.run(&mut world);
}

#[test]
fn resource_with_access_set() {
#[derive(Resource, Default)]
struct R;

fn send_system(_: ResMut<R>) {}
fn read_system(_: Res<R>) {}

let mut world = World::new();
let mut schedule = Schedule::default();

world.init_resource::<R>();
schedule.add_systems((
send_system.before(ResourceAccessSet::Reads::<R>),
read_system,
));

// Panic if there are ambiguities.
schedule.set_build_settings(ScheduleBuildSettings {
ambiguity_detection: bevy_ecs::schedule::LogLevel::Error,
..Default::default()
});

schedule.run(&mut world);
}

#[test]
fn access_set_late_init() {
#[derive(Component)]
struct C;

fn read_system(_: Query<&C>) {}
fn write_system(_: Query<&mut C>) {}

let mut world = World::new();
let mut schedule = Schedule::default();

// Make sure the system with write-access is initialized *before* the access set is mentioned.
schedule.add_systems(write_system);
schedule.run(&mut world);

schedule.add_systems(read_system.after(ComponentAccessSet::Writes::<C>));

// Panic if there are ambiguities.
schedule.set_build_settings(ScheduleBuildSettings {
ambiguity_detection: bevy_ecs::schedule::LogLevel::Error,
..Default::default()
});

schedule.run(&mut world);
}

#[test]
fn access_set_reads_all_not_included() {
#[derive(Resource, Default)]
struct R;

fn read_system(_: Res<R>) {}
fn read_all_system(_: &World) {}

let mut world = World::new();
let mut schedule = Schedule::default();

world.init_resource::<R>();
schedule.add_systems((
read_system,
// This will cause a dependency cycle if `read_all_system` is included
// in the read-access set.
read_all_system.after(ResourceAccessSet::Reads::<R>),
));

// Panic if there are ambiguities.
schedule.set_build_settings(ScheduleBuildSettings {
ambiguity_detection: bevy_ecs::schedule::LogLevel::Error,
..Default::default()
});

schedule.run(&mut world);
}
}
7 changes: 7 additions & 0 deletions crates/bevy_ecs/src/query/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ impl<T: SparseSetIndex> Access<T> {
self.reads_all || self.reads_and_writes.contains(index.sparse_set_index())
}

/// Returns `true` if this has read-only access to the element given by `index`,
/// unless the access comes from [`Self::has_read_all`].
pub(crate) fn has_granular_read_only(&self, index: T) -> bool {
let index = index.sparse_set_index();
self.reads_and_writes.contains(index) && !self.writes.contains(index)
}

/// Returns `true` if this can access anything.
pub fn has_any_read(&self) -> bool {
self.reads_all || !self.reads_and_writes.is_clear()
Expand Down
12 changes: 12 additions & 0 deletions crates/bevy_ecs/src/schedule/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ impl SystemConfigs {
impl<T> NodeConfigs<T> {
/// Adds a new boxed system set to the systems.
pub fn in_set_dyn(&mut self, set: BoxedSystemSet) {
assert!(
set.reads_component().is_none() && set.writes_component().is_none(),
"adding arbitrary systems to a world access set is not allowed"
);
match self {
Self::NodeConfig(config) => {
config.graph_info.sets.push(set);
Expand Down Expand Up @@ -329,6 +333,10 @@ impl IntoSystemConfigs<()> for SystemConfigs {
set.system_type().is_none(),
"adding arbitrary systems to a system type set is not allowed"
);
assert!(
set.reads_component().is_none() && set.writes_component().is_none(),
"adding arbitrary systems to a world access set is not allowed"
);

self.in_set_dyn(set.dyn_clone());

Expand Down Expand Up @@ -409,6 +417,10 @@ impl SystemSetConfig {
set.system_type().is_none(),
"configuring system type sets is not allowed"
);
assert!(
set.reads_component().is_none() && set.writes_component().is_none(),
"configuring world access sets is not allowed"
);

Self {
node: set,
Expand Down
Loading