
# Objective Fixes a part of #14274. Bevy has an incredibly inconsistent naming convention for its system sets, both internally and across the ecosystem. <img alt="System sets in Bevy" src="https://github.com/user-attachments/assets/d16e2027-793f-4ba4-9cc9-e780b14a5a1b" width="450" /> *Names of public system set types in Bevy* Most Bevy types use a naming of `FooSystem` or just `Foo`, but there are also a few `FooSystems` and `FooSet` types. In ecosystem crates on the other hand, `FooSet` is perhaps the most commonly used name in general. Conventions being so wildly inconsistent can make it harder for users to pick names for their own types, to search for system sets on docs.rs, or to even discern which types *are* system sets. To reign in the inconsistency a bit and help unify the ecosystem, it would be good to establish a common recommended naming convention for system sets in Bevy itself, similar to how plugins are commonly suffixed with `Plugin` (ex: `TimePlugin`). By adopting a consistent naming convention in first-party Bevy, we can softly nudge ecosystem crates to follow suit (for types where it makes sense to do so). Choosing a naming convention is also relevant now, as the [`bevy_cli` recently adopted lints](https://github.com/TheBevyFlock/bevy_cli/pull/345) to enforce naming for plugins and system sets, and the recommended naming used for system sets is still a bit open. ## Which Name To Use? Now the contentious part: what naming convention should we actually adopt? This was discussed on the Bevy Discord at the end of last year, starting [here](<https://discord.com/channels/691052431525675048/692572690833473578/1310659954683936789>). `FooSet` and `FooSystems` were the clear favorites, with `FooSet` very narrowly winning an unofficial poll. However, it seems to me like the consensus was broadly moving towards `FooSystems` at the end and after the poll, with Cart ([source](https://discord.com/channels/691052431525675048/692572690833473578/1311140204974706708)) and later Alice ([source](https://discord.com/channels/691052431525675048/692572690833473578/1311092530732859533)) and also me being in favor of it. Let's do a quick pros and cons list! Of course these are just what I thought of, so take it with a grain of salt. `FooSet`: - Pro: Nice and short! - Pro: Used by many ecosystem crates. - Pro: The `Set` suffix comes directly from the trait name `SystemSet`. - Pro: Pairs nicely with existing APIs like `in_set` and `configure_sets`. - Con: `Set` by itself doesn't actually indicate that it's related to systems *at all*, apart from the implemented trait. A set of what? - Con: Is `FooSet` a set of `Foo`s or a system set related to `Foo`? Ex: `ContactSet`, `MeshSet`, `EnemySet`... `FooSystems`: - Pro: Very clearly indicates that the type represents a collection of systems. The actual core concept, system(s), is in the name. - Pro: Parallels nicely with `FooPlugins` for plugin groups. - Pro: Low risk of conflicts with other names or misunderstandings about what the type is. - Pro: In most cases, reads *very* nicely and clearly. Ex: `PhysicsSystems` and `AnimationSystems` as opposed to `PhysicsSet` and `AnimationSet`. - Pro: Easy to search for on docs.rs. - Con: Usually results in longer names. - Con: Not yet as widely used. Really the big problem with `FooSet` is that it doesn't actually describe what it is. It describes what *kind of thing* it is (a set of something), but not *what it is a set of*, unless you know the type or check its docs or implemented traits. `FooSystems` on the other hand is much more self-descriptive in this regard, at the cost of being a bit longer to type. Ultimately, in some ways it comes down to preference and how you think of system sets. Personally, I was originally in favor of `FooSet`, but have been increasingly on the side of `FooSystems`, especially after seeing what the new names would actually look like in Avian and now Bevy. I prefer it because it usually reads better, is much more clearly related to groups of systems than `FooSet`, and overall *feels* more correct and natural to me in the long term. For these reasons, and because Alice and Cart also seemed to share a preference for it when it was previously being discussed, I propose that we adopt a `FooSystems` naming convention where applicable. ## Solution Rename Bevy's system set types to use a consistent `FooSet` naming where applicable. - `AccessibilitySystem` → `AccessibilitySystems` - `GizmoRenderSystem` → `GizmoRenderSystems` - `PickSet` → `PickingSystems` - `RunFixedMainLoopSystem` → `RunFixedMainLoopSystems` - `TransformSystem` → `TransformSystems` - `RemoteSet` → `RemoteSystems` - `RenderSet` → `RenderSystems` - `SpriteSystem` → `SpriteSystems` - `StateTransitionSteps` → `StateTransitionSystems` - `RenderUiSystem` → `RenderUiSystems` - `UiSystem` → `UiSystems` - `Animation` → `AnimationSystems` - `AssetEvents` → `AssetEventSystems` - `TrackAssets` → `AssetTrackingSystems` - `UpdateGizmoMeshes` → `GizmoMeshSystems` - `InputSystem` → `InputSystems` - `InputFocusSet` → `InputFocusSystems` - `ExtractMaterialsSet` → `MaterialExtractionSystems` - `ExtractMeshesSet` → `MeshExtractionSystems` - `RumbleSystem` → `RumbleSystems` - `CameraUpdateSystem` → `CameraUpdateSystems` - `ExtractAssetsSet` → `AssetExtractionSystems` - `Update2dText` → `Text2dUpdateSystems` - `TimeSystem` → `TimeSystems` - `AudioPlaySet` → `AudioPlaybackSystems` - `SendEvents` → `EventSenderSystems` - `EventUpdates` → `EventUpdateSystems` A lot of the names got slightly longer, but they are also a lot more consistent, and in my opinion the majority of them read much better. For a few of the names I took the liberty of rewording things a bit; definitely open to any further naming improvements. There are still also cases where the `FooSystems` naming doesn't really make sense, and those I left alone. This primarily includes system sets like `Interned<dyn SystemSet>`, `EnterSchedules<S>`, `ExitSchedules<S>`, or `TransitionSchedules<S>`, where the type has some special purpose and semantics. ## Todo - [x] Should I keep all the old names as deprecated type aliases? I can do this, but to avoid wasting work I'd prefer to first reach consensus on whether these renames are even desired. - [x] Migration guide - [x] Release notes
237 lines
7.7 KiB
Rust
237 lines
7.7 KiB
Rust
use crate::{
|
|
render_resource::{encase::internal::WriteInto, DynamicUniformBuffer, ShaderType},
|
|
renderer::{RenderDevice, RenderQueue},
|
|
sync_component::SyncComponentPlugin,
|
|
sync_world::RenderEntity,
|
|
view::ViewVisibility,
|
|
Extract, ExtractSchedule, Render, RenderApp, RenderSystems,
|
|
};
|
|
use bevy_app::{App, Plugin};
|
|
use bevy_ecs::{
|
|
bundle::NoBundleEffect,
|
|
component::Component,
|
|
prelude::*,
|
|
query::{QueryFilter, QueryItem, ReadOnlyQueryData},
|
|
};
|
|
use core::{marker::PhantomData, ops::Deref};
|
|
|
|
pub use bevy_render_macros::ExtractComponent;
|
|
|
|
/// Stores the index of a uniform inside of [`ComponentUniforms`].
|
|
#[derive(Component)]
|
|
pub struct DynamicUniformIndex<C: Component> {
|
|
index: u32,
|
|
marker: PhantomData<C>,
|
|
}
|
|
|
|
impl<C: Component> DynamicUniformIndex<C> {
|
|
#[inline]
|
|
pub fn index(&self) -> u32 {
|
|
self.index
|
|
}
|
|
}
|
|
|
|
/// Describes how a component gets extracted for rendering.
|
|
///
|
|
/// Therefore the component is transferred from the "app world" into the "render world"
|
|
/// in the [`ExtractSchedule`] step.
|
|
pub trait ExtractComponent: Component {
|
|
/// ECS [`ReadOnlyQueryData`] to fetch the components to extract.
|
|
type QueryData: ReadOnlyQueryData;
|
|
/// Filters the entities with additional constraints.
|
|
type QueryFilter: QueryFilter;
|
|
|
|
/// The output from extraction.
|
|
///
|
|
/// Returning `None` based on the queried item will remove the component from the entity in
|
|
/// the render world. This can be used, for example, to conditionally extract camera settings
|
|
/// in order to disable a rendering feature on the basis of those settings, without removing
|
|
/// the component from the entity in the main world.
|
|
///
|
|
/// The output may be different from the queried component.
|
|
/// This can be useful for example if only a subset of the fields are useful
|
|
/// in the render world.
|
|
///
|
|
/// `Out` has a [`Bundle`] trait bound instead of a [`Component`] trait bound in order to allow use cases
|
|
/// such as tuples of components as output.
|
|
type Out: Bundle<Effect: NoBundleEffect>;
|
|
|
|
// TODO: https://github.com/rust-lang/rust/issues/29661
|
|
// type Out: Component = Self;
|
|
|
|
/// Defines how the component is transferred into the "render world".
|
|
fn extract_component(item: QueryItem<'_, Self::QueryData>) -> Option<Self::Out>;
|
|
}
|
|
|
|
/// This plugin prepares the components of the corresponding type for the GPU
|
|
/// by transforming them into uniforms.
|
|
///
|
|
/// They can then be accessed from the [`ComponentUniforms`] resource.
|
|
/// For referencing the newly created uniforms a [`DynamicUniformIndex`] is inserted
|
|
/// for every processed entity.
|
|
///
|
|
/// Therefore it sets up the [`RenderSystems::Prepare`] step
|
|
/// for the specified [`ExtractComponent`].
|
|
pub struct UniformComponentPlugin<C>(PhantomData<fn() -> C>);
|
|
|
|
impl<C> Default for UniformComponentPlugin<C> {
|
|
fn default() -> Self {
|
|
Self(PhantomData)
|
|
}
|
|
}
|
|
|
|
impl<C: Component + ShaderType + WriteInto + Clone> Plugin for UniformComponentPlugin<C> {
|
|
fn build(&self, app: &mut App) {
|
|
if let Some(render_app) = app.get_sub_app_mut(RenderApp) {
|
|
render_app
|
|
.insert_resource(ComponentUniforms::<C>::default())
|
|
.add_systems(
|
|
Render,
|
|
prepare_uniform_components::<C>.in_set(RenderSystems::PrepareResources),
|
|
);
|
|
}
|
|
}
|
|
}
|
|
|
|
/// Stores all uniforms of the component type.
|
|
#[derive(Resource)]
|
|
pub struct ComponentUniforms<C: Component + ShaderType> {
|
|
uniforms: DynamicUniformBuffer<C>,
|
|
}
|
|
|
|
impl<C: Component + ShaderType> Deref for ComponentUniforms<C> {
|
|
type Target = DynamicUniformBuffer<C>;
|
|
|
|
#[inline]
|
|
fn deref(&self) -> &Self::Target {
|
|
&self.uniforms
|
|
}
|
|
}
|
|
|
|
impl<C: Component + ShaderType> ComponentUniforms<C> {
|
|
#[inline]
|
|
pub fn uniforms(&self) -> &DynamicUniformBuffer<C> {
|
|
&self.uniforms
|
|
}
|
|
}
|
|
|
|
impl<C: Component + ShaderType> Default for ComponentUniforms<C> {
|
|
fn default() -> Self {
|
|
Self {
|
|
uniforms: Default::default(),
|
|
}
|
|
}
|
|
}
|
|
|
|
/// This system prepares all components of the corresponding component type.
|
|
/// They are transformed into uniforms and stored in the [`ComponentUniforms`] resource.
|
|
fn prepare_uniform_components<C>(
|
|
mut commands: Commands,
|
|
render_device: Res<RenderDevice>,
|
|
render_queue: Res<RenderQueue>,
|
|
mut component_uniforms: ResMut<ComponentUniforms<C>>,
|
|
components: Query<(Entity, &C)>,
|
|
) where
|
|
C: Component + ShaderType + WriteInto + Clone,
|
|
{
|
|
let components_iter = components.iter();
|
|
let count = components_iter.len();
|
|
let Some(mut writer) =
|
|
component_uniforms
|
|
.uniforms
|
|
.get_writer(count, &render_device, &render_queue)
|
|
else {
|
|
return;
|
|
};
|
|
let entities = components_iter
|
|
.map(|(entity, component)| {
|
|
(
|
|
entity,
|
|
DynamicUniformIndex::<C> {
|
|
index: writer.write(component),
|
|
marker: PhantomData,
|
|
},
|
|
)
|
|
})
|
|
.collect::<Vec<_>>();
|
|
commands.try_insert_batch(entities);
|
|
}
|
|
|
|
/// This plugin extracts the components into the render world for synced entities.
|
|
///
|
|
/// To do so, it sets up the [`ExtractSchedule`] step for the specified [`ExtractComponent`].
|
|
pub struct ExtractComponentPlugin<C, F = ()> {
|
|
only_extract_visible: bool,
|
|
marker: PhantomData<fn() -> (C, F)>,
|
|
}
|
|
|
|
impl<C, F> Default for ExtractComponentPlugin<C, F> {
|
|
fn default() -> Self {
|
|
Self {
|
|
only_extract_visible: false,
|
|
marker: PhantomData,
|
|
}
|
|
}
|
|
}
|
|
|
|
impl<C, F> ExtractComponentPlugin<C, F> {
|
|
pub fn extract_visible() -> Self {
|
|
Self {
|
|
only_extract_visible: true,
|
|
marker: PhantomData,
|
|
}
|
|
}
|
|
}
|
|
|
|
impl<C: ExtractComponent> Plugin for ExtractComponentPlugin<C> {
|
|
fn build(&self, app: &mut App) {
|
|
app.add_plugins(SyncComponentPlugin::<C>::default());
|
|
|
|
if let Some(render_app) = app.get_sub_app_mut(RenderApp) {
|
|
if self.only_extract_visible {
|
|
render_app.add_systems(ExtractSchedule, extract_visible_components::<C>);
|
|
} else {
|
|
render_app.add_systems(ExtractSchedule, extract_components::<C>);
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
/// This system extracts all components of the corresponding [`ExtractComponent`], for entities that are synced via [`crate::sync_world::SyncToRenderWorld`].
|
|
fn extract_components<C: ExtractComponent>(
|
|
mut commands: Commands,
|
|
mut previous_len: Local<usize>,
|
|
query: Extract<Query<(RenderEntity, C::QueryData), C::QueryFilter>>,
|
|
) {
|
|
let mut values = Vec::with_capacity(*previous_len);
|
|
for (entity, query_item) in &query {
|
|
if let Some(component) = C::extract_component(query_item) {
|
|
values.push((entity, component));
|
|
} else {
|
|
commands.entity(entity).remove::<C::Out>();
|
|
}
|
|
}
|
|
*previous_len = values.len();
|
|
commands.try_insert_batch(values);
|
|
}
|
|
|
|
/// This system extracts all components of the corresponding [`ExtractComponent`], for entities that are visible and synced via [`crate::sync_world::SyncToRenderWorld`].
|
|
fn extract_visible_components<C: ExtractComponent>(
|
|
mut commands: Commands,
|
|
mut previous_len: Local<usize>,
|
|
query: Extract<Query<(RenderEntity, &ViewVisibility, C::QueryData), C::QueryFilter>>,
|
|
) {
|
|
let mut values = Vec::with_capacity(*previous_len);
|
|
for (entity, view_visibility, query_item) in &query {
|
|
if view_visibility.get() {
|
|
if let Some(component) = C::extract_component(query_item) {
|
|
values.push((entity, component));
|
|
} else {
|
|
commands.entity(entity).remove::<C::Out>();
|
|
}
|
|
}
|
|
}
|
|
*previous_len = values.len();
|
|
commands.try_insert_batch(values);
|
|
}
|