Skip to content
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

One-shot systems as Commands -- allow commands to use SystemParams #7252

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,9 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream {
}
}

impl<'_w, '_s, #(#param: CommandSystemParam,)*> CommandSystemParam for ParamSet<'_w, '_s, (#(#param,)*)>
{}

impl<'w, 's, #(#param: SystemParam,)*> ParamSet<'w, 's, (#(#param,)*)>
{
#(#param_fn_mut)*
Expand Down Expand Up @@ -458,6 +461,16 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
.push(syn::parse_quote!(#field_type: #path::system::ReadOnlySystemParam));
}

// Create a where clause for the `CommandSystemParam` impl.
// Ensure that each field implements `CommandSystemParam`.
let mut command_param_generics = generics.clone();
let command_param_where_clause = command_param_generics.make_where_clause();
for field_type in &field_types {
command_param_where_clause
.predicates
.push(syn::parse_quote!(#field_type: #path::system::CommandSystemParam));
}

let struct_name = &ast.ident;
let state_struct_visibility = &ast.vis;

Expand Down Expand Up @@ -513,6 +526,8 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {

// Safety: Each field is `ReadOnlySystemParam`, so this can only read from the `World`
unsafe impl<'w, 's, #punctuated_generics> #path::system::ReadOnlySystemParam for #struct_name #ty_generics #read_only_where_clause {}

impl<'w, 's, #punctuated_generics> #path::system::CommandSystemParam for #struct_name #ty_generics #command_param_where_clause {}
};
})
}
Expand Down
38 changes: 36 additions & 2 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod command_queue;
mod parallel_scope;
mod system_command;

use crate::{
bundle::Bundle,
Expand All @@ -10,6 +11,7 @@ use bevy_utils::tracing::{error, info};
pub use command_queue::CommandQueue;
pub use parallel_scope::*;
use std::marker::PhantomData;
pub use system_command::CommandSystemParam;

use super::Resource;

Expand Down Expand Up @@ -44,6 +46,18 @@ pub trait Command: Send + 'static {
fn write(self, world: &mut World);
}

pub trait IntoCommand<Marker> {
type Command: Command;
fn into_command(this: Self) -> Self::Command;
}

impl<T: Command> IntoCommand<()> for T {
type Command = Self;
fn into_command(this: Self) -> Self::Command {
this
}
}

/// A [`Command`] queue to perform impactful changes to the [`World`].
///
/// Since each command requires exclusive access to the `World`,
Expand Down Expand Up @@ -522,8 +536,8 @@ impl<'w, 's> Commands<'w, 's> {
/// # bevy_ecs::system::assert_is_system(add_three_to_counter_system);
/// # bevy_ecs::system::assert_is_system(add_twenty_five_to_counter_system);
/// ```
pub fn add<C: Command>(&mut self, command: C) {
self.queue.push(command);
pub fn add<Marker, C: IntoCommand<Marker>>(&mut self, command: C) {
self.queue.push(IntoCommand::into_command(command));
}
}

Expand Down Expand Up @@ -992,6 +1006,7 @@ mod tests {
use crate::{
self as bevy_ecs,
component::Component,
event::{EventWriter, Events},
system::{CommandQueue, Commands, Resource},
world::World,
};
Expand Down Expand Up @@ -1146,4 +1161,23 @@ mod tests {
assert!(!world.contains_resource::<W<i32>>());
assert!(world.contains_resource::<W<f64>>());
}

#[test]
fn system_commands() {
struct E;

let mut world = World::new();
world.init_resource::<Events<E>>();

let mut command_queue = CommandQueue::default();

let mut commands = Commands::new(&mut command_queue, &world);
commands.add(|mut events: EventWriter<E>| {
events.send(E);
});

command_queue.apply(&mut world);

assert!(!world.resource::<Events<E>>().is_empty());
}
}
65 changes: 65 additions & 0 deletions crates/bevy_ecs/src/system/commands/system_command.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use std::marker::PhantomData;

use crate::{
prelude::World,
system::{IntoSystem, SystemMeta, SystemParam, SystemParamFunction},
};

use super::{Command, IntoCommand};

pub trait CommandSystemParam: SystemParam {}

impl<Marker, T: IntoSystem<(), (), Marker>> IntoCommand<(IsSystemCommand, Marker)> for T
where
T::System: Command,
{
type Command = T::System;
fn into_command(this: Self) -> Self::Command {
IntoSystem::into_system(this)
}
}

#[doc(hidden)]
pub struct IsSystemCommand;

pub struct SystemCommand<Param, Marker, F>
where
Param: CommandSystemParam,
{
func: F,
marker: PhantomData<fn(Param) -> Marker>,
}

impl<Param, Marker, F> IntoCommand<(IsSystemCommand, Param, Marker)> for F
where
Param: CommandSystemParam + 'static,
Marker: 'static,
F: SystemParamFunction<(), (), Param, Marker>,
{
type Command = SystemCommand<Param, Marker, F>;
fn into_command(func: Self) -> Self::Command {
SystemCommand {
func,
marker: PhantomData,
}
}
}

impl<Param, Marker, F> Command for SystemCommand<Param, Marker, F>
where
Param: CommandSystemParam + 'static,
Marker: 'static,
F: SystemParamFunction<(), (), Param, Marker>,
{
fn write(mut self, world: &mut World) {
let change_tick = world.change_tick();

let mut system_meta = SystemMeta::new::<F>();
let mut param_state = Param::init_state(world, &mut system_meta);
let params =
// SAFETY: We have exclusive world access.
unsafe { Param::get_param(&mut param_state, &system_meta, world, change_tick) };
self.func.run((), params);
Param::apply(&mut param_state, &system_meta, world);
}
}
28 changes: 27 additions & 1 deletion crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
query::{
Access, FilteredAccess, FilteredAccessSet, QueryState, ReadOnlyWorldQuery, WorldQuery,
},
system::{CommandQueue, Commands, Query, SystemMeta},
system::{CommandQueue, CommandSystemParam, Commands, Query, SystemMeta},
world::{FromWorld, World},
};
pub use bevy_ecs_macros::Resource;
Expand Down Expand Up @@ -233,6 +233,11 @@ unsafe impl<Q: WorldQuery + 'static, F: ReadOnlyWorldQuery + 'static> SystemPara
}
}

impl<Q: WorldQuery + 'static, F: ReadOnlyWorldQuery + 'static> CommandSystemParam
for Query<'_, '_, Q, F>
{
}

fn assert_component_access_compatibility(
system_name: &str,
query_type: &'static str,
Expand Down Expand Up @@ -463,6 +468,8 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> {
}
}

impl<T: Resource> CommandSystemParam for Res<'_, T> {}

// SAFETY: Only reads a single World resource
unsafe impl<'a, T: Resource> ReadOnlySystemParam for Option<Res<'a, T>> {}

Expand Down Expand Up @@ -496,6 +503,8 @@ unsafe impl<'a, T: Resource> SystemParam for Option<Res<'a, T>> {
}
}

impl<T: Resource> CommandSystemParam for Option<Res<'_, T>> {}

// SAFETY: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res
// conflicts with any prior access, a panic will occur.
unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
Expand Down Expand Up @@ -556,6 +565,8 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
}
}

impl<T: Resource> CommandSystemParam for ResMut<'_, T> {}

// SAFETY: this impl defers to `ResMut`, which initializes and validates the correct world access.
unsafe impl<'a, T: Resource> SystemParam for Option<ResMut<'a, T>> {
type State = ComponentId;
Expand Down Expand Up @@ -586,6 +597,8 @@ unsafe impl<'a, T: Resource> SystemParam for Option<ResMut<'a, T>> {
}
}

impl<T: Resource> CommandSystemParam for Option<ResMut<'_, T>> {}

// SAFETY: Commands only accesses internal state
unsafe impl<'w, 's> ReadOnlySystemParam for Commands<'w, 's> {}

Expand Down Expand Up @@ -659,6 +672,8 @@ unsafe impl SystemParam for &'_ World {
}
}

impl CommandSystemParam for &World {}

/// A system local [`SystemParam`].
///
/// A local may only be accessed by the system itself and is therefore not visible to other systems.
Expand Down Expand Up @@ -1115,6 +1130,8 @@ unsafe impl<'a> SystemParam for &'a Archetypes {
}
}

impl CommandSystemParam for &Archetypes {}

// SAFETY: Only reads World components
unsafe impl<'a> ReadOnlySystemParam for &'a Components {}

Expand All @@ -1136,6 +1153,8 @@ unsafe impl<'a> SystemParam for &'a Components {
}
}

impl CommandSystemParam for &Components {}

// SAFETY: Only reads World entities
unsafe impl<'a> ReadOnlySystemParam for &'a Entities {}

Expand All @@ -1157,6 +1176,8 @@ unsafe impl<'a> SystemParam for &'a Entities {
}
}

impl CommandSystemParam for &Entities {}

// SAFETY: Only reads World bundles
unsafe impl<'a> ReadOnlySystemParam for &'a Bundles {}

Expand All @@ -1178,6 +1199,8 @@ unsafe impl<'a> SystemParam for &'a Bundles {
}
}

impl CommandSystemParam for &Bundles {}

/// A [`SystemParam`] that reads the previous and current change ticks of the system.
///
/// A system's change ticks are updated each time it runs:
Expand Down Expand Up @@ -1341,6 +1364,9 @@ macro_rules! impl_system_param_tuple {
($($param::get_param($param, _system_meta, _world, _change_tick),)*)
}
}

#[allow(non_snake_case)]
impl<$($param: CommandSystemParam),*> CommandSystemParam for ($($param,)*) {}
};
}

Expand Down