Skip to content

Commit

Permalink
Cleanup system sets called labels (bevyengine#7678)
Browse files Browse the repository at this point in the history
# Objective

We have a few old system labels that are now system sets but are still named or documented as labels. Documentation also generally mentioned system labels in some places.


## Solution

- Clean up naming and documentation regarding system sets

## Migration Guide

`PrepareAssetLabel` is now called `PrepareAssetSet`
  • Loading branch information
NiklasEi authored and myreprise1 committed Feb 15, 2023
1 parent 5c3f1c4 commit e99a0c4
Show file tree
Hide file tree
Showing 15 changed files with 39 additions and 39 deletions.
2 changes: 1 addition & 1 deletion .github/contributing/example_style_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ For more advice on writing examples, see the [relevant section](../../CONTRIBUTI
4. In Queries, prefer `With<T>` filters over actually fetching unused data with `&T`.
5. Prefer disjoint queries using `With` and `Without` over param sets when you need more than one query in a single system.
6. Prefer structs with named fields over tuple structs except in the case of single-field wrapper types.
7. Use enum-labels over string-labels for system / schedule / etc. labels.
7. Use enum-labels over string-labels for app / schedule / etc. labels.

## "Feature" examples

Expand Down
12 changes: 6 additions & 6 deletions benches/benches/bevy_ecs/scheduling/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ pub fn build_schedule(criterion: &mut Criterion) {
// Use multiple different kinds of label to ensure that dynamic dispatch
// doesn't somehow get optimized away.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
struct NumLabel(usize);
struct NumSet(usize);
#[derive(Debug, Clone, Copy, SystemSet, PartialEq, Eq, Hash)]
struct DummyLabel;
struct DummySet;

impl SystemSet for NumLabel {
impl SystemSet for NumSet {
fn dyn_clone(&self) -> Box<dyn SystemSet> {
Box::new(self.clone())
}
Expand All @@ -81,7 +81,7 @@ pub fn build_schedule(criterion: &mut Criterion) {
// Method: generate a set of `graph_size` systems which have a One True Ordering.
// Add system to the schedule with full constraints. Hopefully this should be maximimally
// difficult for bevy to figure out.
let labels: Vec<_> = (0..1000).map(|i| NumLabel(i)).collect();
let labels: Vec<_> = (0..1000).map(|i| NumSet(i)).collect();

// Benchmark graphs of different sizes.
for graph_size in [100, 500, 1000] {
Expand All @@ -100,12 +100,12 @@ pub fn build_schedule(criterion: &mut Criterion) {
group.bench_function(format!("{graph_size}_schedule"), |bencher| {
bencher.iter(|| {
let mut app = App::new();
app.add_system(empty_system.in_set(DummyLabel));
app.add_system(empty_system.in_set(DummySet));

// Build a fully-connected dependency graph describing the One True Ordering.
// Not particularly realistic but this can be refined later.
for i in 0..graph_size {
let mut sys = empty_system.in_set(labels[i]).before(DummyLabel);
let mut sys = empty_system.in_set(labels[i]).before(DummySet);
for label in labels.iter().take(i) {
sys = sys.after(*label);
}
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 @@ -313,12 +313,12 @@ impl App {
/// for each state variant, an instance of [`apply_state_transition::<S>`] in
/// [`CoreSet::StateTransitions`] so that transitions happen before [`CoreSet::Update`] and
/// a instance of [`run_enter_schedule::<S>`] in [`CoreSet::StateTransitions`] with a
/// with a [`run_once`](`run_once_condition`) condition to run the on enter schedule of the
/// [`run_once`](`run_once_condition`) condition to run the on enter schedule of the
/// initial state.
///
/// This also adds an [`OnUpdate`] system set for each state variant,
/// which run during [`CoreSet::Update`] after the transitions are applied.
/// These systems sets only run if the [`State<S>`] resource matches their label.
/// which runs during [`CoreSet::Update`] after the transitions are applied.
/// These system sets only run if the [`State<S>`] resource matches the respective state variant.
///
/// If you would like to control how other systems run based on the current state,
/// you can emulate this behavior using the [`in_state`] [`Condition`](bevy_ecs::schedule::Condition).
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_app/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl CoreSet {
/// Sets up the base structure of [`CoreSchedule::Main`].
///
/// The sets defined in this enum are configured to run in order,
/// and a copy of [`apply_system_buffers`] is inserted at each `*Flush` label.
/// and a copy of [`apply_system_buffers`] is inserted into each `*Flush` set.
pub fn base_schedule() -> Schedule {
use CoreSet::*;
let mut schedule = Schedule::new();
Expand Down Expand Up @@ -183,7 +183,7 @@ impl StartupSet {
/// Sets up the base structure of [`CoreSchedule::Startup`].
///
/// The sets defined in this enum are configured to run in order,
/// and a copy of [`apply_system_buffers`] is inserted at each `*Flush` label.
/// and a copy of [`apply_system_buffers`] is inserted into each `*Flush` set.
pub fn base_schedule() -> Schedule {
use StartupSet::*;
let mut schedule = Schedule::new();
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 @@ -32,7 +32,7 @@ impl DerefMut for DebugAssetApp {
}
}

/// A label describing the system that runs [`DebugAssetApp`].
/// A set containing the system that runs [`DebugAssetApp`].
#[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)]
pub struct DebugAssetAppRun;

Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_ecs/macros/src/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ 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
/// Derive a set trait
///
/// # Args
///
/// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait
/// - `trait_path`: The path [`syn::Path`] to the label trait
/// - `input`: The [`syn::DeriveInput`] for the struct that we want to derive the set trait for
/// - `trait_path`: The [`syn::Path`] to the set trait
pub fn derive_set(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream {
let ident = input.ident;

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_pbr/src/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use bevy_render::{
extract_component::ExtractComponentPlugin,
mesh::{Mesh, MeshVertexBufferLayout},
prelude::Image,
render_asset::{PrepareAssetLabel, RenderAssets},
render_asset::{PrepareAssetSet, RenderAssets},
render_phase::{
AddRenderCommand, DrawFunctions, PhaseItem, RenderCommand, RenderCommandResult,
RenderPhase, SetItemPipeline, TrackedRenderPass,
Expand Down Expand Up @@ -197,7 +197,7 @@ where
.add_system(
prepare_materials::<M>
.in_set(RenderSet::Prepare)
.after(PrepareAssetLabel::PreAssetPrepare),
.after(PrepareAssetSet::PreAssetPrepare),
)
.add_system(queue_material_meshes::<M>.in_set(RenderSet::Queue));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl RenderSet {
/// Sets up the base structure of the rendering [`Schedule`].
///
/// The sets defined in this enum are configured to run in order,
/// and a copy of [`apply_system_buffers`] is inserted at each `*Flush` label.
/// and a copy of [`apply_system_buffers`] is inserted into each `*Flush` set.
pub fn base_schedule() -> Schedule {
use RenderSet::*;

Expand Down
18 changes: 9 additions & 9 deletions crates/bevy_render/src/render_asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub trait RenderAsset: Asset {
}

#[derive(Clone, Hash, Debug, Default, PartialEq, Eq, SystemSet)]
pub enum PrepareAssetLabel {
pub enum PrepareAssetSet {
PreAssetPrepare,
#[default]
AssetPrepare,
Expand All @@ -54,14 +54,14 @@ pub enum PrepareAssetLabel {
/// Therefore it sets up the [`ExtractSchedule`](crate::ExtractSchedule) and
/// [`RenderSet::Prepare`](crate::RenderSet::Prepare) steps for the specified [`RenderAsset`].
pub struct RenderAssetPlugin<A: RenderAsset> {
prepare_asset_label: PrepareAssetLabel,
prepare_asset_set: PrepareAssetSet,
phantom: PhantomData<fn() -> A>,
}

impl<A: RenderAsset> RenderAssetPlugin<A> {
pub fn with_prepare_asset_label(prepare_asset_label: PrepareAssetLabel) -> Self {
pub fn with_prepare_asset_set(prepare_asset_set: PrepareAssetSet) -> Self {
Self {
prepare_asset_label,
prepare_asset_set,
phantom: PhantomData,
}
}
Expand All @@ -70,7 +70,7 @@ impl<A: RenderAsset> RenderAssetPlugin<A> {
impl<A: RenderAsset> Default for RenderAssetPlugin<A> {
fn default() -> Self {
Self {
prepare_asset_label: Default::default(),
prepare_asset_set: Default::default(),
phantom: PhantomData,
}
}
Expand All @@ -82,9 +82,9 @@ impl<A: RenderAsset> Plugin for RenderAssetPlugin<A> {
render_app
.configure_sets(
(
PrepareAssetLabel::PreAssetPrepare,
PrepareAssetLabel::AssetPrepare,
PrepareAssetLabel::PostAssetPrepare,
PrepareAssetSet::PreAssetPrepare,
PrepareAssetSet::AssetPrepare,
PrepareAssetSet::PostAssetPrepare,
)
.chain()
.in_set(RenderSet::Prepare),
Expand All @@ -93,7 +93,7 @@ impl<A: RenderAsset> Plugin for RenderAssetPlugin<A> {
.init_resource::<RenderAssets<A>>()
.init_resource::<PrepareNextFrameAssets<A>>()
.add_system_to_schedule(ExtractSchedule, extract_render_asset::<A>)
.add_system(prepare_assets::<A>.in_set(self.prepare_asset_label.clone()));
.add_system(prepare_assets::<A>.in_set(self.prepare_asset_set.clone()));
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_render/src/texture/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub use image_texture_loader::*;
pub use texture_cache::*;

use crate::{
render_asset::{PrepareAssetLabel, RenderAssetPlugin},
render_asset::{PrepareAssetSet, RenderAssetPlugin},
renderer::RenderDevice,
RenderApp, RenderSet,
};
Expand Down Expand Up @@ -84,8 +84,8 @@ impl Plugin for ImagePlugin {
app.init_asset_loader::<HdrTextureLoader>();
}

app.add_plugin(RenderAssetPlugin::<Image>::with_prepare_asset_label(
PrepareAssetLabel::PreAssetPrepare,
app.add_plugin(RenderAssetPlugin::<Image>::with_prepare_asset_set(
PrepareAssetSet::PreAssetPrepare,
))
.register_type::<Image>()
.add_asset::<Image>()
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_render/src/view/visibility/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl ComputedVisibility {

/// Whether this entity is visible to something this frame. This is true if and only if [`Self::is_visible_in_hierarchy`] and [`Self::is_visible_in_view`]
/// are true. This is the canonical method to call to determine if an entity should be drawn.
/// This value is updated in [`CoreSet::PostUpdate`] during the [`VisibilitySystems::CheckVisibility`] system label.
/// This value is updated in [`CoreSet::PostUpdate`] by the [`VisibilitySystems::CheckVisibility`] system set.
/// Reading it during [`CoreSet::Update`] will yield the value from the previous frame.
#[inline]
pub fn is_visible(&self) -> bool {
Expand Down Expand Up @@ -356,7 +356,7 @@ fn propagate_recursive(

/// System updating the visibility of entities each frame.
///
/// The system is labelled with [`VisibilitySystems::CheckVisibility`]. Each frame, it updates the
/// The system is part of the [`VisibilitySystems::CheckVisibility`] set. Each frame, it updates the
/// [`ComputedVisibility`] of all entities, and for each view also compute the [`VisibleEntities`]
/// for that view.
pub fn check_visibility(
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_sprite/src/mesh2d/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use bevy_render::{
extract_component::ExtractComponentPlugin,
mesh::{Mesh, MeshVertexBufferLayout},
prelude::Image,
render_asset::{PrepareAssetLabel, RenderAssets},
render_asset::{PrepareAssetSet, RenderAssets},
render_phase::{
AddRenderCommand, DrawFunctions, PhaseItem, RenderCommand, RenderCommandResult,
RenderPhase, SetItemPipeline, TrackedRenderPass,
Expand Down Expand Up @@ -163,7 +163,7 @@ where
.add_system(
prepare_materials_2d::<M>
.in_set(RenderSet::Prepare)
.after(PrepareAssetLabel::PreAssetPrepare),
.after(PrepareAssetSet::PreAssetPrepare),
)
.add_system(queue_material2d_meshes::<M>.in_set(RenderSet::Queue));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_transform/src/components/global_transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use bevy_reflect::{std_traits::ReflectDefault, FromReflect, Reflect};
///
/// [`GlobalTransform`] is the position of an entity relative to the reference frame.
///
/// [`GlobalTransform`] is updated from [`Transform`] in the systems labeled
/// [`GlobalTransform`] is updated from [`Transform`] by systems in the system set
/// [`TransformPropagate`](crate::TransformSystem::TransformPropagate).
///
/// This system runs during [`CoreSet::PostUpdate`](crate::CoreSet::PostUpdate). If you
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_transform/src/components/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::ops::Mul;
///
/// [`GlobalTransform`] is the position of an entity relative to the reference frame.
///
/// [`GlobalTransform`] is updated from [`Transform`] in the systems labeled
/// [`GlobalTransform`] is updated from [`Transform`] by systems in the system set
/// [`TransformPropagate`](crate::TransformSystem::TransformPropagate).
///
/// This system runs during [`CoreSet::PostUpdate`](crate::CoreSet::PostUpdate). If you
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use systems::{propagate_transforms, sync_simple_transforms};
///
/// [`GlobalTransform`] is the position of an entity relative to the reference frame.
///
/// [`GlobalTransform`] is updated from [`Transform`] in the systems labeled
/// [`GlobalTransform`] is updated from [`Transform`] by systems in the system set
/// [`TransformPropagate`](crate::TransformSystem::TransformPropagate).
///
/// This system runs during [`CoreSet::PostUpdate`](crate::CoreSet::PostUpdate). If you
Expand Down Expand Up @@ -77,7 +77,7 @@ impl From<Transform> for TransformBundle {
Self::from_transform(transform)
}
}
/// Label enum for the systems relating to transform propagation
/// Set enum for the systems relating to transform propagation
#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)]
pub enum TransformSystem {
/// Propagates changes in transform to children's [`GlobalTransform`](crate::components::GlobalTransform)
Expand Down

0 comments on commit e99a0c4

Please sign in to comment.