Skip to content

Commit 3a664b0

Browse files
cBournhonesquecBournhonesque
authored
Separate component and resource access (#14561)
# Objective - Fixes #13139 - Fixes #7255 - Separates component from resource access so that we can correctly handles edge cases like the issue above - Inspired from #14472 ## Solution - Update access to have `component` fields and `resource` fields ## Testing - Added some unit tests
1 parent 0d0f77a commit 3a664b0

File tree

11 files changed

+580
-232
lines changed

11 files changed

+580
-232
lines changed

‎crates/bevy_ecs/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,8 +1420,8 @@ mod tests {
14201420
let mut expected = FilteredAccess::<ComponentId>::default();
14211421
let a_id = world.components.get_id(TypeId::of::<A>()).unwrap();
14221422
let b_id = world.components.get_id(TypeId::of::<B>()).unwrap();
1423-
expected.add_write(a_id);
1424-
expected.add_read(b_id);
1423+
expected.add_component_write(a_id);
1424+
expected.add_component_read(b_id);
14251425
assert!(
14261426
query.component_access.eq(&expected),
14271427
"ComponentId access from query fetch and query filter should be combined"

‎crates/bevy_ecs/src/query/access.rs

Lines changed: 424 additions & 156 deletions
Large diffs are not rendered by default.

‎crates/bevy_ecs/src/query/builder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,14 @@ impl<'w, D: QueryData, F: QueryFilter> QueryBuilder<'w, D, F> {
142142
/// Adds `&T` to the [`FilteredAccess`] of self.
143143
pub fn ref_id(&mut self, id: ComponentId) -> &mut Self {
144144
self.with_id(id);
145-
self.access.add_read(id);
145+
self.access.add_component_read(id);
146146
self
147147
}
148148

149149
/// Adds `&mut T` to the [`FilteredAccess`] of self.
150150
pub fn mut_id(&mut self, id: ComponentId) -> &mut Self {
151151
self.with_id(id);
152-
self.access.add_write(id);
152+
self.access.add_component_write(id);
153153
self
154154
}
155155

‎crates/bevy_ecs/src/query/fetch.rs

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -480,10 +480,10 @@ unsafe impl<'a> WorldQuery for EntityRef<'a> {
480480

481481
fn update_component_access(_state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
482482
assert!(
483-
!access.access().has_any_write(),
483+
!access.access().has_any_component_write(),
484484
"EntityRef conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.",
485485
);
486-
access.read_all();
486+
access.read_all_components();
487487
}
488488

489489
fn init_state(_world: &mut World) {}
@@ -556,10 +556,10 @@ unsafe impl<'a> WorldQuery for EntityMut<'a> {
556556

557557
fn update_component_access(_state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
558558
assert!(
559-
!access.access().has_any_read(),
559+
!access.access().has_any_component_read(),
560560
"EntityMut conflicts with a previous access in this query. Exclusive access cannot coincide with any other accesses.",
561561
);
562-
access.write_all();
562+
access.write_all_components();
563563
}
564564

565565
fn init_state(_world: &mut World) {}
@@ -600,7 +600,7 @@ unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> {
600600
_this_run: Tick,
601601
) -> Self::Fetch<'w> {
602602
let mut access = Access::default();
603-
access.read_all();
603+
access.read_all_components();
604604
(world, access)
605605
}
606606

@@ -612,9 +612,9 @@ unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> {
612612
_table: &Table,
613613
) {
614614
let mut access = Access::default();
615-
state.access.reads().for_each(|id| {
615+
state.access.component_reads().for_each(|id| {
616616
if archetype.contains(id) {
617-
access.add_read(id);
617+
access.add_component_read(id);
618618
}
619619
});
620620
fetch.1 = access;
@@ -623,9 +623,9 @@ unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> {
623623
#[inline]
624624
unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table) {
625625
let mut access = Access::default();
626-
state.access.reads().for_each(|id| {
626+
state.access.component_reads().for_each(|id| {
627627
if table.has_column(id) {
628-
access.add_read(id);
628+
access.add_component_read(id);
629629
}
630630
});
631631
fetch.1 = access;
@@ -703,7 +703,7 @@ unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> {
703703
_this_run: Tick,
704704
) -> Self::Fetch<'w> {
705705
let mut access = Access::default();
706-
access.write_all();
706+
access.write_all_components();
707707
(world, access)
708708
}
709709

@@ -715,14 +715,14 @@ unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> {
715715
_table: &Table,
716716
) {
717717
let mut access = Access::default();
718-
state.access.reads().for_each(|id| {
718+
state.access.component_reads().for_each(|id| {
719719
if archetype.contains(id) {
720-
access.add_read(id);
720+
access.add_component_read(id);
721721
}
722722
});
723-
state.access.writes().for_each(|id| {
723+
state.access.component_writes().for_each(|id| {
724724
if archetype.contains(id) {
725-
access.add_write(id);
725+
access.add_component_write(id);
726726
}
727727
});
728728
fetch.1 = access;
@@ -731,14 +731,14 @@ unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> {
731731
#[inline]
732732
unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table) {
733733
let mut access = Access::default();
734-
state.access.reads().for_each(|id| {
734+
state.access.component_reads().for_each(|id| {
735735
if table.has_column(id) {
736-
access.add_read(id);
736+
access.add_component_read(id);
737737
}
738738
});
739-
state.access.writes().for_each(|id| {
739+
state.access.component_writes().for_each(|id| {
740740
if table.has_column(id) {
741-
access.add_write(id);
741+
access.add_component_write(id);
742742
}
743743
});
744744
fetch.1 = access;
@@ -988,11 +988,11 @@ unsafe impl<T: Component> WorldQuery for &T {
988988
access: &mut FilteredAccess<ComponentId>,
989989
) {
990990
assert!(
991-
!access.access().has_write(component_id),
991+
!access.access().has_component_write(component_id),
992992
"&{} conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.",
993-
std::any::type_name::<T>(),
993+
std::any::type_name::<T>(),
994994
);
995-
access.add_read(component_id);
995+
access.add_component_read(component_id);
996996
}
997997

998998
fn init_state(world: &mut World) -> ComponentId {
@@ -1183,11 +1183,11 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> {
11831183
access: &mut FilteredAccess<ComponentId>,
11841184
) {
11851185
assert!(
1186-
!access.access().has_write(component_id),
1186+
!access.access().has_component_write(component_id),
11871187
"&{} conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.",
1188-
std::any::type_name::<T>(),
1188+
std::any::type_name::<T>(),
11891189
);
1190-
access.add_read(component_id);
1190+
access.add_component_read(component_id);
11911191
}
11921192

11931193
fn init_state(world: &mut World) -> ComponentId {
@@ -1378,11 +1378,11 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
13781378
access: &mut FilteredAccess<ComponentId>,
13791379
) {
13801380
assert!(
1381-
!access.access().has_read(component_id),
1381+
!access.access().has_component_read(component_id),
13821382
"&mut {} conflicts with a previous access in this query. Mutable component access must be unique.",
1383-
std::any::type_name::<T>(),
1383+
std::any::type_name::<T>(),
13841384
);
1385-
access.add_write(component_id);
1385+
access.add_component_write(component_id);
13861386
}
13871387

13881388
fn init_state(world: &mut World) -> ComponentId {
@@ -1476,11 +1476,11 @@ unsafe impl<'__w, T: Component> WorldQuery for Mut<'__w, T> {
14761476
// Update component access here instead of in `<&mut T as WorldQuery>` to avoid erroneously referencing
14771477
// `&mut T` in error message.
14781478
assert!(
1479-
!access.access().has_read(component_id),
1479+
!access.access().has_component_read(component_id),
14801480
"Mut<{}> conflicts with a previous access in this query. Mutable component access mut be unique.",
1481-
std::any::type_name::<T>(),
1481+
std::any::type_name::<T>(),
14821482
);
1483-
access.add_write(component_id);
1483+
access.add_component_write(component_id);
14841484
}
14851485

14861486
// Forwarded to `&mut T`

‎crates/bevy_ecs/src/query/filter.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -688,10 +688,10 @@ unsafe impl<T: Component> WorldQuery for Added<T> {
688688

689689
#[inline]
690690
fn update_component_access(&id: &ComponentId, access: &mut FilteredAccess<ComponentId>) {
691-
if access.access().has_write(id) {
691+
if access.access().has_component_write(id) {
692692
panic!("$state_name<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.",std::any::type_name::<T>());
693693
}
694-
access.add_read(id);
694+
access.add_component_read(id);
695695
}
696696

697697
fn init_state(world: &mut World) -> ComponentId {
@@ -899,10 +899,10 @@ unsafe impl<T: Component> WorldQuery for Changed<T> {
899899

900900
#[inline]
901901
fn update_component_access(&id: &ComponentId, access: &mut FilteredAccess<ComponentId>) {
902-
if access.access().has_write(id) {
902+
if access.access().has_component_write(id) {
903903
panic!("$state_name<{}> conflicts with a previous access in this query. Shared access cannot coincide with exclusive access.",std::any::type_name::<T>());
904904
}
905-
access.add_read(id);
905+
access.add_component_read(id);
906906
}
907907

908908
fn init_state(world: &mut World) -> ComponentId {

‎crates/bevy_ecs/src/query/state.rs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -495,16 +495,22 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
495495
archetype: &Archetype,
496496
access: &mut Access<ArchetypeComponentId>,
497497
) {
498-
self.component_access.access.reads().for_each(|id| {
499-
if let Some(id) = archetype.get_archetype_component_id(id) {
500-
access.add_read(id);
501-
}
502-
});
503-
self.component_access.access.writes().for_each(|id| {
504-
if let Some(id) = archetype.get_archetype_component_id(id) {
505-
access.add_write(id);
506-
}
507-
});
498+
self.component_access
499+
.access
500+
.component_reads()
501+
.for_each(|id| {
502+
if let Some(id) = archetype.get_archetype_component_id(id) {
503+
access.add_component_read(id);
504+
}
505+
});
506+
self.component_access
507+
.access
508+
.component_writes()
509+
.for_each(|id| {
510+
if let Some(id) = archetype.get_archetype_component_id(id) {
511+
access.add_component_write(id);
512+
}
513+
});
508514
}
509515

510516
/// Use this to transform a [`QueryState`] into a more generic [`QueryState`].

‎crates/bevy_ecs/src/schedule/mod.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,9 @@ mod tests {
737737
#[derive(Event)]
738738
struct E;
739739

740+
#[derive(Resource, Component)]
741+
struct RC;
742+
740743
fn empty_system() {}
741744
fn res_system(_res: Res<R>) {}
742745
fn resmut_system(_res: ResMut<R>) {}
@@ -746,6 +749,8 @@ mod tests {
746749
fn write_component_system(_query: Query<&mut A>) {}
747750
fn with_filtered_component_system(_query: Query<&mut A, With<B>>) {}
748751
fn without_filtered_component_system(_query: Query<&mut A, Without<B>>) {}
752+
fn entity_ref_system(_query: Query<EntityRef>) {}
753+
fn entity_mut_system(_query: Query<EntityMut>) {}
749754
fn event_reader_system(_reader: EventReader<E>) {}
750755
fn event_writer_system(_writer: EventWriter<E>) {}
751756
fn event_resource_system(_events: ResMut<Events<E>>) {}
@@ -788,6 +793,8 @@ mod tests {
788793
nonsend_system,
789794
read_component_system,
790795
read_component_system,
796+
entity_ref_system,
797+
entity_ref_system,
791798
event_reader_system,
792799
event_reader_system,
793800
read_world_system,
@@ -893,6 +900,73 @@ mod tests {
893900
assert_eq!(schedule.graph().conflicting_systems().len(), 3);
894901
}
895902

903+
/// Test that when a struct is both a Resource and a Component, they do not
904+
/// conflict with each other.
905+
#[test]
906+
fn shared_resource_mut_component() {
907+
let mut world = World::new();
908+
world.insert_resource(RC);
909+
910+
let mut schedule = Schedule::default();
911+
schedule.add_systems((|_: ResMut<RC>| {}, |_: Query<&mut RC>| {}));
912+
913+
let _ = schedule.initialize(&mut world);
914+
915+
assert_eq!(schedule.graph().conflicting_systems().len(), 0);
916+
}
917+
918+
#[test]
919+
fn resource_mut_and_entity_ref() {
920+
let mut world = World::new();
921+
world.insert_resource(R);
922+
923+
let mut schedule = Schedule::default();
924+
schedule.add_systems((resmut_system, entity_ref_system));
925+
926+
let _ = schedule.initialize(&mut world);
927+
928+
assert_eq!(schedule.graph().conflicting_systems().len(), 0);
929+
}
930+
931+
#[test]
932+
fn resource_and_entity_mut() {
933+
let mut world = World::new();
934+
world.insert_resource(R);
935+
936+
let mut schedule = Schedule::default();
937+
schedule.add_systems((res_system, nonsend_system, entity_mut_system));
938+
939+
let _ = schedule.initialize(&mut world);
940+
941+
assert_eq!(schedule.graph().conflicting_systems().len(), 0);
942+
}
943+
944+
#[test]
945+
fn write_component_and_entity_ref() {
946+
let mut world = World::new();
947+
world.insert_resource(R);
948+
949+
let mut schedule = Schedule::default();
950+
schedule.add_systems((write_component_system, entity_ref_system));
951+
952+
let _ = schedule.initialize(&mut world);
953+
954+
assert_eq!(schedule.graph().conflicting_systems().len(), 1);
955+
}
956+
957+
#[test]
958+
fn read_component_and_entity_mut() {
959+
let mut world = World::new();
960+
world.insert_resource(R);
961+
962+
let mut schedule = Schedule::default();
963+
schedule.add_systems((read_component_system, entity_mut_system));
964+
965+
let _ = schedule.initialize(&mut world);
966+
967+
assert_eq!(schedule.graph().conflicting_systems().len(), 1);
968+
}
969+
896970
#[test]
897971
fn exclusive() {
898972
let mut world = World::new();

‎crates/bevy_ecs/src/system/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,7 +1495,7 @@ mod tests {
14951495
assert_eq!(
14961496
system
14971497
.archetype_component_access()
1498-
.reads()
1498+
.component_reads()
14991499
.collect::<HashSet<_>>(),
15001500
expected_ids
15011501
);
@@ -1525,7 +1525,7 @@ mod tests {
15251525
assert_eq!(
15261526
system
15271527
.archetype_component_access()
1528-
.reads()
1528+
.component_reads()
15291529
.collect::<HashSet<_>>(),
15301530
expected_ids
15311531
);
@@ -1543,7 +1543,7 @@ mod tests {
15431543
assert_eq!(
15441544
system
15451545
.archetype_component_access()
1546-
.reads()
1546+
.component_reads()
15471547
.collect::<HashSet<_>>(),
15481548
expected_ids
15491549
);

0 commit comments

Comments
 (0)