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

Check for conflicting system resource parameters (#864)

Browse Source
This commit is contained in:
memoryruins 2020-11-15 15:42:43 -05:00 committed by GitHub
parent bb4a7392c0
commit 4bdff66b80
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 108 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
crates/bevy_ecs/src/system/into_system.rs
View File

@ -9,6 +9,7 @@ pub struct SystemState {
pub(crate) is_initialized: bool,
pub(crate) archetype_component_access: TypeAccess<ArchetypeComponent>,
pub(crate) resource_access: TypeAccess<TypeId>,
pub(crate) local_resource_access: TypeAccess<TypeId>,
pub(crate) query_archetype_component_accesses: Vec<TypeAccess<ArchetypeComponent>>,
pub(crate) query_accesses: Vec<Vec<QueryAccess>>,
pub(crate) query_type_names: Vec<&'static str>,
@ -148,6 +149,7 @@ macro_rules! impl_into_system {
name: std::any::type_name::<Self>().into(),
archetype_component_access: TypeAccess::default(),
resource_access: TypeAccess::default(),
local_resource_access: TypeAccess::default(),
is_initialized: false,
id: SystemId::new(),
commands: Commands::default(),
@ -204,13 +206,13 @@ impl_into_system!(A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P);
mod tests {
use super::IntoSystem;
use crate::{
resource::{ResMut, Resources},
resource::{Res, ResMut, Resources},
schedule::Schedule,
ChangedRes, Query, QuerySet, System,
ChangedRes, Local, Query, QuerySet, System,
};
use bevy_hecs::{Entity, Or, With, World};
#[derive(Debug, Eq, PartialEq)]
#[derive(Debug, Eq, PartialEq, Default)]
struct A;
struct B;
struct C;
@ -441,4 +443,60 @@ mod tests {
schedule.initialize(world, resources);
schedule.run(world, resources);
}
#[derive(Default)]
struct BufferRes {
_buffer: Vec<u8>,
}
fn test_for_conflicting_resources(sys: Box<dyn System>) {
let mut world = World::default();
let mut resources = Resources::default();
resources.insert(BufferRes::default());
resources.insert(A);
resources.insert(B);
run_system(&mut world, &mut resources, sys);
}
#[test]
#[should_panic]
fn conflicting_system_resources() {
fn sys(_: ResMut<BufferRes>, _: Res<BufferRes>) {}
test_for_conflicting_resources(sys.system())
}
#[test]
#[should_panic]
fn conflicting_system_resources_reverse_order() {
fn sys(_: Res<BufferRes>, _: ResMut<BufferRes>) {}
test_for_conflicting_resources(sys.system())
}
#[test]
#[should_panic]
fn conflicting_system_resources_multiple_mutable() {
fn sys(_: ResMut<BufferRes>, _: ResMut<BufferRes>) {}
test_for_conflicting_resources(sys.system())
}
#[test]
#[should_panic]
fn conflicting_changed_and_mutable_resource() {
// A tempting pattern, but unsound if allowed.
fn sys(_: ResMut<BufferRes>, _: ChangedRes<BufferRes>) {}
test_for_conflicting_resources(sys.system())
}
#[test]
#[should_panic]
fn conflicting_system_local_resources() {
fn sys(_: Local<BufferRes>, _: Local<BufferRes>) {}
test_for_conflicting_resources(sys.system())
}
#[test]
fn nonconflicting_system_resources() {
fn sys(_: Local<BufferRes>, _: ResMut<BufferRes>, _: Local<A>, _: ResMut<A>) {}
test_for_conflicting_resources(sys.system())
}
}

48
crates/bevy_ecs/src/system/system_param.rs
View File

@ -113,6 +113,14 @@ impl SystemParam for Arc<Mutex<Commands>> {
impl<'a, T: Resource> SystemParam for Res<'a, T> {
fn init(system_state: &mut SystemState, _world: &World, _resources: &mut Resources) {
if system_state.resource_access.is_write(&TypeId::of::<T>()) {
panic!(
"System `{}` has a `Res<{res}>` parameter that conflicts with \
another parameter with mutable access to the same `{res}` resource.",
system_state.name,
res = std::any::type_name::<T>()
);
}
system_state.resource_access.add_read(TypeId::of::<T>());
}
@ -130,6 +138,19 @@ impl<'a, T: Resource> SystemParam for Res<'a, T> {
impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
fn init(system_state: &mut SystemState, _world: &World, _resources: &mut Resources) {
// If a system already has access to the resource in another parameter, then we fail early.
// e.g. `fn(Res<Foo>, ResMut<Foo>)` or `fn(ResMut<Foo>, ResMut<Foo>)` must not be allowed.
if system_state
.resource_access
.is_read_or_write(&TypeId::of::<T>())
{
panic!(
"System `{}` has a `ResMut<{res}>` parameter that conflicts with \
another parameter to the same `{res}` resource. `ResMut` must have unique access.",
system_state.name,
res = std::any::type_name::<T>()
);
}
system_state.resource_access.add_write(TypeId::of::<T>());
}
@ -147,6 +168,14 @@ impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
impl<'a, T: Resource> SystemParam for ChangedRes<'a, T> {
fn init(system_state: &mut SystemState, _world: &World, _resources: &mut Resources) {
if system_state.resource_access.is_write(&TypeId::of::<T>()) {
panic!(
"System `{}` has a `ChangedRes<{res}>` parameter that conflicts with \
another parameter with mutable access to the same `{res}` resource.",
system_state.name,
res = std::any::type_name::<T>()
);
}
system_state.resource_access.add_read(TypeId::of::<T>());
}
@ -169,11 +198,28 @@ impl<'a, T: Resource> SystemParam for ChangedRes<'a, T> {
impl<'a, T: Resource + FromResources> SystemParam for Local<'a, T> {
fn init(system_state: &mut SystemState, _world: &World, resources: &mut Resources) {
system_state.resource_access.add_write(TypeId::of::<T>());
if system_state
.local_resource_access
.is_read_or_write(&TypeId::of::<T>())
{
panic!(
"System `{}` has multiple parameters requesting access to a local resource of type `{}`. \
There may be at most one `Local` parameter per resource type.",
system_state.name,
std::any::type_name::<T>()
);
}
// A resource could have been already initialized by another system with
// `Commands::insert_local_resource` or `Resources::insert_local`
if resources.get_local::<T>(system_state.id).is_none() {
let value = T::from_resources(resources);
resources.insert_local(system_state.id, value);
}
system_state
.local_resource_access
.add_write(TypeId::of::<T>());
}
#[inline]