Skip to content

Commit

Permalink
Base Sets (#7466)
Browse files Browse the repository at this point in the history
# Objective

NOTE: This depends on #7267 and should not be merged until #7267 is merged. If you are reviewing this before that is merged, I highly recommend viewing the Base Sets commit instead of trying to find my changes amongst those from #7267.

"Default sets" as described by the [Stageless RFC](bevyengine/rfcs#45) have some [unfortunate consequences](#7365).

## Solution

This adds "base sets" as a variant of `SystemSet`:

A set is a "base set" if `SystemSet::is_base` returns `true`. Typically this will be opted-in to using the `SystemSet` derive:

```rust
#[derive(SystemSet, Clone, Hash, Debug, PartialEq, Eq)]
#[system_set(base)]
enum MyBaseSet {
  A,
  B,
}
``` 

**Base sets are exclusive**: a system can belong to at most one "base set". Adding a system to more than one will result in an error. When possible we fail immediately during system-config-time with a nice file + line number. For the more nested graph-ey cases, this will fail at the final schedule build. 

**Base sets cannot belong to other sets**: this is where the word "base" comes from

Systems and Sets can only be added to base sets using `in_base_set`. Calling `in_set` with a base set will fail. As will calling `in_base_set` with a normal set.

```rust
app.add_system(foo.in_base_set(MyBaseSet::A))
       // X must be a normal set ... base sets cannot be added to base sets
       .configure_set(X.in_base_set(MyBaseSet::A))
```

Base sets can still be configured like normal sets:

```rust
app.add_system(MyBaseSet::B.after(MyBaseSet::Ap))
``` 

The primary use case for base sets is enabling a "default base set":

```rust
schedule.set_default_base_set(CoreSet::Update)
  // this will belong to CoreSet::Update by default
  .add_system(foo)
  // this will override the default base set with PostUpdate
  .add_system(bar.in_base_set(CoreSet::PostUpdate))
```

This allows us to build apis that work by default in the standard Bevy style. This is a rough analog to the "default stage" model, but it use the new "stageless sets" model instead, with all of the ordering flexibility (including exclusive systems) that it provides.

---

## Changelog

- Added "base sets" and ported CoreSet to use them.

## Migration Guide

TODO
  • Loading branch information
cart committed Feb 6, 2023
1 parent 206c7ce commit 3badeaf
Show file tree
Hide file tree
Showing 84 changed files with 920 additions and 273 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_animation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ impl Plugin for AnimationPlugin {
.register_type::<AnimationPlayer>()
.add_system(
animation_player
.in_set(CoreSet::PostUpdate)
.in_base_set(CoreSet::PostUpdate)
.before(TransformSystem::TransformPropagate),
);
}
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,14 +328,14 @@ impl App {
apply_state_transition::<S>,
)
.chain()
.in_set(CoreSet::StateTransitions),
.in_base_set(CoreSet::StateTransitions),
);

let main_schedule = self.get_schedule_mut(CoreSchedule::Main).unwrap();
for variant in S::variants() {
main_schedule.configure_set(
OnUpdate(variant.clone())
.in_set(CoreSet::StateTransitions)
.in_base_set(CoreSet::StateTransitions)
.run_if(state_equals(variant))
.after(apply_state_transition::<S>),
);
Expand Down Expand Up @@ -580,7 +580,7 @@ impl App {
{
if !self.world.contains_resource::<Events<T>>() {
self.init_resource::<Events<T>>()
.add_system(Events::<T>::update_system.in_set(CoreSet::First));
.add_system(Events::<T>::update_system.in_base_set(CoreSet::First));
}
self
}
Expand Down
43 changes: 27 additions & 16 deletions crates/bevy_app/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ pub mod prelude {

use bevy_ecs::{
schedule_v3::{
apply_system_buffers, IntoSystemConfig, IntoSystemSetConfig, Schedule, ScheduleLabel,
SystemSet,
apply_system_buffers, IntoSystemConfig, IntoSystemSetConfig, IntoSystemSetConfigs,
Schedule, ScheduleLabel, SystemSet,
},
system::Local,
world::World,
Expand Down Expand Up @@ -90,6 +90,7 @@ impl CoreSchedule {
/// that runs immediately after the matching system set.
/// These can be useful for ordering, but you almost never want to add your systems to these sets.
#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)]
#[system_set(base)]
pub enum CoreSet {
/// Runs before all other members of this set.
First,
Expand Down Expand Up @@ -129,20 +130,30 @@ impl CoreSet {
let mut schedule = Schedule::new();

// Create "stage-like" structure using buffer flushes + ordering
schedule.add_system(apply_system_buffers.in_set(FirstFlush));
schedule.add_system(apply_system_buffers.in_set(PreUpdateFlush));
schedule.add_system(apply_system_buffers.in_set(UpdateFlush));
schedule.add_system(apply_system_buffers.in_set(PostUpdateFlush));
schedule.add_system(apply_system_buffers.in_set(LastFlush));

schedule.configure_set(First.before(FirstFlush));
schedule.configure_set(PreUpdate.after(FirstFlush).before(PreUpdateFlush));
schedule.configure_set(StateTransitions.after(PreUpdateFlush).before(FixedUpdate));
schedule.configure_set(FixedUpdate.after(StateTransitions).before(Update));
schedule.configure_set(Update.after(FixedUpdate).before(UpdateFlush));
schedule.configure_set(PostUpdate.after(UpdateFlush).before(PostUpdateFlush));
schedule.configure_set(Last.after(PostUpdateFlush).before(LastFlush));

schedule
.set_default_base_set(Update)
.add_system(apply_system_buffers.in_base_set(FirstFlush))
.add_system(apply_system_buffers.in_base_set(PreUpdateFlush))
.add_system(apply_system_buffers.in_base_set(UpdateFlush))
.add_system(apply_system_buffers.in_base_set(PostUpdateFlush))
.add_system(apply_system_buffers.in_base_set(LastFlush))
.configure_sets(
(
First,
FirstFlush,
PreUpdate,
PreUpdateFlush,
StateTransitions,
FixedUpdate,
Update,
UpdateFlush,
PostUpdate,
PostUpdateFlush,
Last,
LastFlush,
)
.chain(),
);
schedule
}
}
Expand Down
14 changes: 3 additions & 11 deletions crates/bevy_asset/src/asset_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ pub fn free_unused_assets_system(asset_server: Res<AssetServer>) {
mod test {
use super::*;
use crate::{loader::LoadedAsset, update_asset_storage_system};
use bevy_app::{App, CoreSet};
use bevy_app::App;
use bevy_ecs::prelude::*;
use bevy_reflect::TypeUuid;
use bevy_utils::BoxedFuture;
Expand Down Expand Up @@ -852,16 +852,8 @@ mod test {
let mut app = App::new();
app.insert_resource(assets);
app.insert_resource(asset_server);
app.add_system(
free_unused_assets_system
.in_set(FreeUnusedAssets)
.in_set(CoreSet::Update),
);
app.add_system(
update_asset_storage_system::<PngAsset>
.after(FreeUnusedAssets)
.in_set(CoreSet::Update),
);
app.add_system(free_unused_assets_system.in_set(FreeUnusedAssets));
app.add_system(update_asset_storage_system::<PngAsset>.after(FreeUnusedAssets));

fn load_asset(path: AssetPath, world: &World) -> HandleUntyped {
let asset_server = world.resource::<AssetServer>();
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_asset/src/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,8 @@ impl AddAsset for App {
};

self.insert_resource(assets)
.add_system(Assets::<T>::asset_event_system.in_set(AssetSet::AssetEvents))
.add_system(update_asset_storage_system::<T>.in_set(AssetSet::LoadAssets))
.add_system(Assets::<T>::asset_event_system.in_base_set(AssetSet::AssetEvents))
.add_system(update_asset_storage_system::<T>.in_base_set(AssetSet::LoadAssets))
.register_type::<Handle<T>>()
.add_event::<AssetEvent<T>>()
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_asset/src/debug_asset_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl Plugin for DebugAssetServerPlugin {
watch_for_changes: true,
});
app.insert_non_send_resource(DebugAssetApp(debug_asset_app));
app.add_system(run_debug_asset_app.in_set(CoreSet::Update));
app.add_system(run_debug_asset_app);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl<T: Asset> Default for AssetCountDiagnosticsPlugin<T> {
impl<T: Asset> Plugin for AssetCountDiagnosticsPlugin<T> {
fn build(&self, app: &mut App) {
app.add_startup_system(Self::setup_system.in_set(StartupSet::Startup))
.add_system(Self::diagnostic_system.in_set(CoreSet::Update));
.add_system(Self::diagnostic_system);
}
}

Expand Down
7 changes: 3 additions & 4 deletions crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ use bevy_ecs::prelude::*;

/// [`SystemSet`]s for asset loading in an [`App`] schedule.
#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)]
#[system_set(base)]
pub enum AssetSet {
/// Asset storages are updated.
LoadAssets,
Expand Down Expand Up @@ -109,22 +110,20 @@ impl Plugin for AssetPlugin {

app.configure_set(
AssetSet::LoadAssets
.no_default_set()
.before(CoreSet::PreUpdate)
.after(CoreSet::First),
)
.configure_set(
AssetSet::AssetEvents
.no_default_set()
.after(CoreSet::PostUpdate)
.before(CoreSet::Last),
)
.add_system(asset_server::free_unused_assets_system.in_set(CoreSet::PreUpdate));
.add_system(asset_server::free_unused_assets_system.in_base_set(CoreSet::PreUpdate));

#[cfg(all(
feature = "filesystem_watcher",
all(not(target_arch = "wasm32"), not(target_os = "android"))
))]
app.add_system(io::filesystem_watcher_system.in_set(AssetSet::LoadAssets));
app.add_system(io::filesystem_watcher_system.in_base_set(AssetSet::LoadAssets));
}
}
4 changes: 2 additions & 2 deletions crates/bevy_audio/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl Plugin for AudioPlugin {
.add_asset::<AudioSource>()
.add_asset::<AudioSink>()
.init_resource::<Audio<AudioSource>>()
.add_system(play_queued_audio_system::<AudioSource>.in_set(CoreSet::PostUpdate));
.add_system(play_queued_audio_system::<AudioSource>.in_base_set(CoreSet::PostUpdate));

#[cfg(any(feature = "mp3", feature = "flac", feature = "wav", feature = "vorbis"))]
app.init_asset_loader::<AudioLoader>();
Expand All @@ -71,6 +71,6 @@ impl AddAudioSource for App {
self.add_asset::<T>()
.init_resource::<Audio<T>>()
.init_resource::<AudioOutput<T>>()
.add_system(play_queued_audio_system::<T>.in_set(CoreSet::PostUpdate))
.add_system(play_queued_audio_system::<T>.in_base_set(CoreSet::PostUpdate))
}
}
4 changes: 2 additions & 2 deletions crates/bevy_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl Plugin for TaskPoolPlugin {
self.task_pool_options.create_default_pools();

#[cfg(not(target_arch = "wasm32"))]
app.add_system(tick_global_task_pools.in_set(bevy_app::CoreSet::Last));
app.add_system(tick_global_task_pools.in_base_set(bevy_app::CoreSet::Last));
}
}
/// A dummy type that is [`!Send`](Send), to force systems to run on the main thread.
Expand Down Expand Up @@ -142,7 +142,7 @@ pub struct FrameCountPlugin;
impl Plugin for FrameCountPlugin {
fn build(&self, app: &mut App) {
app.init_resource::<FrameCount>();
app.add_system(update_frame_count.in_set(CoreSet::Last));
app.add_system(update_frame_count.in_base_set(CoreSet::Last));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub struct EntityCountDiagnosticsPlugin;
impl Plugin for EntityCountDiagnosticsPlugin {
fn build(&self, app: &mut App) {
app.add_startup_system(Self::setup_system.in_set(StartupSet::Startup))
.add_system(Self::diagnostic_system.in_set(CoreSet::Update));
.add_system(Self::diagnostic_system);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub struct FrameTimeDiagnosticsPlugin;
impl Plugin for FrameTimeDiagnosticsPlugin {
fn build(&self, app: &mut bevy_app::App) {
app.add_startup_system(Self::setup_system.in_set(StartupSet::Startup))
.add_system(Self::diagnostic_system.in_set(CoreSet::Update));
.add_system(Self::diagnostic_system);
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_diagnostic/src/log_diagnostics_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ impl Plugin for LogDiagnosticsPlugin {
});

if self.debug {
app.add_system(Self::log_diagnostics_debug_system.in_set(CoreSet::PostUpdate));
app.add_system(Self::log_diagnostics_debug_system.in_base_set(CoreSet::PostUpdate));
} else {
app.add_system(Self::log_diagnostics_system.in_set(CoreSet::PostUpdate));
app.add_system(Self::log_diagnostics_system.in_base_set(CoreSet::PostUpdate));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub struct SystemInformationDiagnosticsPlugin;
impl Plugin for SystemInformationDiagnosticsPlugin {
fn build(&self, app: &mut App) {
app.add_startup_system(internal::setup_system.in_set(StartupSet::Startup))
.add_system(internal::diagnostic_system.in_set(CoreSet::Update));
.add_system(internal::diagnostic_system);
}
}

Expand Down
7 changes: 4 additions & 3 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ extern crate proc_macro;

mod component;
mod fetch;
mod set;

use crate::fetch::derive_world_query_impl;
use bevy_macro_utils::{derive_boxed_label, derive_set, get_named_struct_fields, BevyManifest};
use crate::{fetch::derive_world_query_impl, set::derive_set};
use bevy_macro_utils::{derive_boxed_label, get_named_struct_fields, BevyManifest};
use proc_macro::TokenStream;
use proc_macro2::Span;
use quote::{format_ident, quote};
Expand Down Expand Up @@ -537,7 +538,7 @@ pub fn derive_schedule_label(input: TokenStream) -> TokenStream {
}

/// Derive macro generating an impl of the trait `SystemSet`.
#[proc_macro_derive(SystemSet)]
#[proc_macro_derive(SystemSet, attributes(system_set))]
pub fn derive_system_set(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
let mut trait_path = bevy_ecs_path();
Expand Down
84 changes: 84 additions & 0 deletions crates/bevy_ecs/macros/src/set.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
use proc_macro::TokenStream;
use quote::{quote, ToTokens};
use syn::parse::{Parse, ParseStream};

pub static SYSTEM_SET_ATTRIBUTE_NAME: &str = "system_set";
pub static BASE_ATTRIBUTE_NAME: &str = "base";

/// Derive a label trait
///
/// # Args
///
/// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait
/// - `trait_path`: The path [`syn::Path`] to the label trait
pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream {
let ident = input.ident;

let mut is_base = false;
for attr in &input.attrs {
if !attr
.path
.get_ident()
.map_or(false, |ident| ident == SYSTEM_SET_ATTRIBUTE_NAME)
{
continue;
}

attr.parse_args_with(|input: ParseStream| {
let meta = input.parse_terminated::<syn::Meta, syn::token::Comma>(syn::Meta::parse)?;
for meta in meta {
let ident = meta.path().get_ident().unwrap_or_else(|| {
panic!(
"Unrecognized attribute: `{}`",
meta.path().to_token_stream()
)
});
if ident == BASE_ATTRIBUTE_NAME {
if let syn::Meta::Path(_) = meta {
is_base = true;
} else {
panic!(
"The `{BASE_ATTRIBUTE_NAME}` attribute is expected to have no value or arguments",
);
}
} else {
panic!(
"Unrecognized attribute: `{}`",
meta.path().to_token_stream()
);
}
}
Ok(())
})
.unwrap_or_else(|_| panic!("Invalid `{SYSTEM_SET_ATTRIBUTE_NAME}` attribute format"));
}

let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause {
where_token: Default::default(),
predicates: Default::default(),
});
where_clause.predicates.push(
syn::parse2(quote! {
Self: 'static + Send + Sync + Clone + Eq + ::std::fmt::Debug + ::std::hash::Hash
})
.unwrap(),
);

(quote! {
impl #impl_generics #trait_path for #ident #ty_generics #where_clause {
fn is_system_type(&self) -> bool {
false
}

fn is_base(&self) -> bool {
#is_base
}

fn dyn_clone(&self) -> std::boxed::Box<dyn #trait_path> {
std::boxed::Box::new(std::clone::Clone::clone(self))
}
}
})
.into()
}
Loading

0 comments on commit 3badeaf

Please sign in to comment.