Skip to content

Commit 1bb751c

Browse files
committed
Plugins own their settings. Rework PluginGroup trait. (bevyengine#6336)
# Objective Fixes bevyengine#5884 bevyengine#2879 Alternative to bevyengine#2988 bevyengine#5885 bevyengine#2886 "Immutable" Plugin settings are currently represented as normal ECS resources, which are read as part of plugin init. This presents a number of problems: 1. If a user inserts the plugin settings resource after the plugin is initialized, it will be silently ignored (and use the defaults instead) 2. Users can modify the plugin settings resource after the plugin has been initialized. This creates a false sense of control over settings that can no longer be changed. (1) and (2) are especially problematic and confusing for the `WindowDescriptor` resource, but this is a general problem. ## Solution Immutable Plugin settings now live on each Plugin struct (ex: `WindowPlugin`). PluginGroups have been reworked to support overriding plugin values. This also removes the need for the `add_plugins_with` api, as the `add_plugins` api can use the builder pattern directly. Settings that can be used at runtime continue to be represented as ECS resources. Plugins are now configured like this: ```rust app.add_plugin(AssetPlugin { watch_for_changes: true, ..default() }) ``` PluginGroups are now configured like this: ```rust app.add_plugins(DefaultPlugins .set(AssetPlugin { watch_for_changes: true, ..default() }) ) ``` This is an alternative to bevyengine#2988, which is similar. But I personally prefer this solution for a couple of reasons: * ~~bevyengine#2988 doesn't solve (1)~~ bevyengine#2988 does solve (1) and will panic in that case. I was wrong! * This PR directly ties plugin settings to Plugin types in a 1:1 relationship, rather than a loose "setup resource" <-> plugin coupling (where the setup resource is consumed by the first plugin that uses it). * I'm not a huge fan of overloading the ECS resource concept and implementation for something that has very different use cases and constraints. ## Changelog - PluginGroups can now be configured directly using the builder pattern. Individual plugin values can be overridden by using `plugin_group.set(SomePlugin {})`, which enables overriding default plugin values. - `WindowDescriptor` plugin settings have been moved to `WindowPlugin` and `AssetServerSettings` have been moved to `AssetPlugin` - `app.add_plugins_with` has been replaced by using `add_plugins` with the builder pattern. ## Migration Guide The `WindowDescriptor` settings have been moved from a resource to `WindowPlugin::window`: ```rust // Old (Bevy 0.8) app .insert_resource(WindowDescriptor { width: 400.0, ..default() }) .add_plugins(DefaultPlugins) // New (Bevy 0.9) app.add_plugins(DefaultPlugins.set(WindowPlugin { window: WindowDescriptor { width: 400.0, ..default() }, ..default() })) ``` The `AssetServerSettings` resource has been removed in favor of direct `AssetPlugin` configuration: ```rust // Old (Bevy 0.8) app .insert_resource(AssetServerSettings { watch_for_changes: true, ..default() }) .add_plugins(DefaultPlugins) // New (Bevy 0.9) app.add_plugins(DefaultPlugins.set(AssetPlugin { watch_for_changes: true, ..default() })) ``` `add_plugins_with` has been replaced by `add_plugins` in combination with the builder pattern: ```rust // Old (Bevy 0.8) app.add_plugins_with(DefaultPlugins, |group| group.disable::<AssetPlugin>()); // New (Bevy 0.9) app.add_plugins(DefaultPlugins.build().disable::<AssetPlugin>()); ```
1 parent beab0bd commit 1bb751c

35 files changed

+358
-344
lines changed

crates/bevy_app/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ bevy_utils = { path = "../bevy_utils", version = "0.9.0-dev" }
2424
# other
2525
serde = { version = "1.0", features = ["derive"], optional = true }
2626
ron = { version = "0.8.0", optional = true }
27+
downcast-rs = "1.2.0"
2728

2829

2930
[target.'cfg(target_arch = "wasm32")'.dependencies]

crates/bevy_app/src/app.rs

+6-57
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::{CoreStage, Plugin, PluginGroup, PluginGroupBuilder, StartupSchedule, StartupStage};
1+
use crate::{CoreStage, Plugin, PluginGroup, StartupSchedule, StartupStage};
22
pub use bevy_derive::AppLabel;
33
use bevy_derive::{Deref, DerefMut};
44
use bevy_ecs::{
@@ -838,7 +838,8 @@ impl App {
838838
/// The [`PluginGroup`]s available by default are `DefaultPlugins` and `MinimalPlugins`.
839839
///
840840
/// To customize the plugins in the group (reorder, disable a plugin, add a new plugin
841-
/// before / after another plugin), see [`add_plugins_with`](Self::add_plugins_with).
841+
/// before / after another plugin), call [`build()`](PluginGroup::build) on the group,
842+
/// which will convert it to a [`PluginGroupBuilder`](crate::PluginGroupBuilder).
842843
///
843844
/// ## Examples
844845
/// ```
@@ -847,61 +848,9 @@ impl App {
847848
/// App::new()
848849
/// .add_plugins(MinimalPlugins);
849850
/// ```
850-
pub fn add_plugins<T: PluginGroup>(&mut self, mut group: T) -> &mut Self {
851-
let mut plugin_group_builder = PluginGroupBuilder::default();
852-
group.build(&mut plugin_group_builder);
853-
plugin_group_builder.finish(self);
854-
self
855-
}
856-
857-
/// Adds a group of [`Plugin`]s with an initializer method.
858-
///
859-
/// Can be used to add a group of [`Plugin`]s, where the group is modified
860-
/// before insertion into a Bevy application. For example, you can add
861-
/// additional [`Plugin`]s at a specific place in the [`PluginGroup`], or deactivate
862-
/// specific [`Plugin`]s while keeping the rest using a [`PluginGroupBuilder`].
863-
///
864-
/// # Examples
865-
///
866-
/// ```
867-
/// # use bevy_app::{prelude::*, PluginGroupBuilder};
868-
/// #
869-
/// # // Dummies created to avoid using `bevy_internal` and `bevy_log`,
870-
/// # // which pulls in too many dependencies and breaks rust-analyzer
871-
/// # pub mod bevy_log {
872-
/// # use bevy_app::prelude::*;
873-
/// # #[derive(Default)]
874-
/// # pub struct LogPlugin;
875-
/// # impl Plugin for LogPlugin{
876-
/// # fn build(&self, app: &mut App) {}
877-
/// # }
878-
/// # }
879-
/// # struct DefaultPlugins;
880-
/// # impl PluginGroup for DefaultPlugins {
881-
/// # fn build(&mut self, group: &mut PluginGroupBuilder){
882-
/// # group.add(bevy_log::LogPlugin::default());
883-
/// # }
884-
/// # }
885-
/// #
886-
/// # struct MyOwnPlugin;
887-
/// # impl Plugin for MyOwnPlugin {
888-
/// # fn build(&self, app: &mut App){;}
889-
/// # }
890-
/// #
891-
/// App::new()
892-
/// .add_plugins_with(DefaultPlugins, |group| {
893-
/// group.add_before::<bevy_log::LogPlugin, _>(MyOwnPlugin)
894-
/// });
895-
/// ```
896-
pub fn add_plugins_with<T, F>(&mut self, mut group: T, func: F) -> &mut Self
897-
where
898-
T: PluginGroup,
899-
F: FnOnce(&mut PluginGroupBuilder) -> &mut PluginGroupBuilder,
900-
{
901-
let mut plugin_group_builder = PluginGroupBuilder::default();
902-
group.build(&mut plugin_group_builder);
903-
func(&mut plugin_group_builder);
904-
plugin_group_builder.finish(self);
851+
pub fn add_plugins<T: PluginGroup>(&mut self, group: T) -> &mut Self {
852+
let builder = group.build();
853+
builder.finish(self);
905854
self
906855
}
907856

crates/bevy_app/src/plugin.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1+
use downcast_rs::{impl_downcast, Downcast};
2+
13
use crate::App;
24
use std::any::Any;
35

46
/// A collection of Bevy app logic and configuration.
57
///
68
/// Plugins configure an [`App`]. When an [`App`] registers a plugin,
79
/// the plugin's [`Plugin::build`] function is run.
8-
pub trait Plugin: Any + Send + Sync {
10+
pub trait Plugin: Downcast + Any + Send + Sync {
911
/// Configures the [`App`] to which this plugin is added.
1012
fn build(&self, app: &mut App);
1113
/// Configures a name for the [`Plugin`] which is primarily used for debugging.
@@ -14,6 +16,8 @@ pub trait Plugin: Any + Send + Sync {
1416
}
1517
}
1618

19+
impl_downcast!(Plugin);
20+
1721
/// A type representing an unsafe function that returns a mutable pointer to a [`Plugin`].
1822
/// It is used for dynamically loading plugins.
1923
///

crates/bevy_app/src/plugin_group.rs

+66-36
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,26 @@ use bevy_utils::{tracing::debug, tracing::warn, HashMap};
33
use std::any::TypeId;
44

55
/// Combines multiple [`Plugin`]s into a single unit.
6-
pub trait PluginGroup {
6+
pub trait PluginGroup: Sized {
77
/// Configures the [`Plugin`]s that are to be added.
8-
fn build(&mut self, group: &mut PluginGroupBuilder);
8+
fn build(self) -> PluginGroupBuilder;
9+
/// Sets the value of the given [`Plugin`], if it exists
10+
fn set<T: Plugin>(self, plugin: T) -> PluginGroupBuilder {
11+
self.build().set(plugin)
12+
}
913
}
1014

1115
struct PluginEntry {
1216
plugin: Box<dyn Plugin>,
1317
enabled: bool,
1418
}
1519

20+
impl PluginGroup for PluginGroupBuilder {
21+
fn build(self) -> PluginGroupBuilder {
22+
self
23+
}
24+
}
25+
1626
/// Facilitates the creation and configuration of a [`PluginGroup`].
1727
/// Provides a build ordering to ensure that [`Plugin`]s which produce/require a [`Resource`](bevy_ecs::system::Resource)
1828
/// are built before/after dependent/depending [`Plugin`]s. [`Plugin`]s inside the group
@@ -25,7 +35,7 @@ pub struct PluginGroupBuilder {
2535

2636
impl PluginGroupBuilder {
2737
/// Finds the index of a target [`Plugin`]. Panics if the target's [`TypeId`] is not found.
28-
fn index_of<Target: Plugin>(&mut self) -> usize {
38+
fn index_of<Target: Plugin>(&self) -> usize {
2939
let index = self
3040
.order
3141
.iter()
@@ -68,9 +78,27 @@ impl PluginGroupBuilder {
6878
}
6979
}
7080

81+
/// Sets the value of the given [`Plugin`], if it exists.
82+
///
83+
/// # Panics
84+
///
85+
/// Panics if the [`Plugin`] does not exist.
86+
pub fn set<T: Plugin>(mut self, plugin: T) -> Self {
87+
let entry = self.plugins.get_mut(&TypeId::of::<T>()).unwrap_or_else(|| {
88+
panic!(
89+
"{} does not exist in this PluginGroup",
90+
std::any::type_name::<T>(),
91+
)
92+
});
93+
entry.plugin = Box::new(plugin);
94+
self
95+
}
96+
7197
/// Adds the plugin [`Plugin`] at the end of this [`PluginGroupBuilder`]. If the plugin was
7298
/// already in the group, it is removed from its previous place.
73-
pub fn add<T: Plugin>(&mut self, plugin: T) -> &mut Self {
99+
// This is not confusing, clippy!
100+
#[allow(clippy::should_implement_trait)]
101+
pub fn add<T: Plugin>(mut self, plugin: T) -> Self {
74102
let target_index = self.order.len();
75103
self.order.push(TypeId::of::<T>());
76104
self.upsert_plugin_state(plugin, target_index);
@@ -80,7 +108,7 @@ impl PluginGroupBuilder {
80108
/// Adds a [`Plugin`] in this [`PluginGroupBuilder`] before the plugin of type `Target`.
81109
/// If the plugin was already the group, it is removed from its previous place. There must
82110
/// be a plugin of type `Target` in the group or it will panic.
83-
pub fn add_before<Target: Plugin, T: Plugin>(&mut self, plugin: T) -> &mut Self {
111+
pub fn add_before<Target: Plugin, T: Plugin>(mut self, plugin: T) -> Self {
84112
let target_index = self.index_of::<Target>();
85113
self.order.insert(target_index, TypeId::of::<T>());
86114
self.upsert_plugin_state(plugin, target_index);
@@ -90,7 +118,7 @@ impl PluginGroupBuilder {
90118
/// Adds a [`Plugin`] in this [`PluginGroupBuilder`] after the plugin of type `Target`.
91119
/// If the plugin was already the group, it is removed from its previous place. There must
92120
/// be a plugin of type `Target` in the group or it will panic.
93-
pub fn add_after<Target: Plugin, T: Plugin>(&mut self, plugin: T) -> &mut Self {
121+
pub fn add_after<Target: Plugin, T: Plugin>(mut self, plugin: T) -> Self {
94122
let target_index = self.index_of::<Target>() + 1;
95123
self.order.insert(target_index, TypeId::of::<T>());
96124
self.upsert_plugin_state(plugin, target_index);
@@ -102,7 +130,7 @@ impl PluginGroupBuilder {
102130
/// [`Plugin`]s within a [`PluginGroup`] are enabled by default. This function is used to
103131
/// opt back in to a [`Plugin`] after [disabling](Self::disable) it. If there are no plugins
104132
/// of type `T` in this group, it will panic.
105-
pub fn enable<T: Plugin>(&mut self) -> &mut Self {
133+
pub fn enable<T: Plugin>(mut self) -> Self {
106134
let mut plugin_entry = self
107135
.plugins
108136
.get_mut(&TypeId::of::<T>())
@@ -116,7 +144,7 @@ impl PluginGroupBuilder {
116144
/// still be used for ordering with [`add_before`](Self::add_before) or
117145
/// [`add_after`](Self::add_after), or it can be [re-enabled](Self::enable). If there are no
118146
/// plugins of type `T` in this group, it will panic.
119-
pub fn disable<T: Plugin>(&mut self) -> &mut Self {
147+
pub fn disable<T: Plugin>(mut self) -> Self {
120148
let mut plugin_entry = self
121149
.plugins
122150
.get_mut(&TypeId::of::<T>())
@@ -152,7 +180,9 @@ impl PluginGroupBuilder {
152180
pub struct NoopPluginGroup;
153181

154182
impl PluginGroup for NoopPluginGroup {
155-
fn build(&mut self, _: &mut PluginGroupBuilder) {}
183+
fn build(self) -> PluginGroupBuilder {
184+
PluginGroupBuilder::default()
185+
}
156186
}
157187

158188
#[cfg(test)]
@@ -177,10 +207,10 @@ mod tests {
177207

178208
#[test]
179209
fn basic_ordering() {
180-
let mut group = PluginGroupBuilder::default();
181-
group.add(PluginA);
182-
group.add(PluginB);
183-
group.add(PluginC);
210+
let group = PluginGroupBuilder::default()
211+
.add(PluginA)
212+
.add(PluginB)
213+
.add(PluginC);
184214

185215
assert_eq!(
186216
group.order,
@@ -194,10 +224,10 @@ mod tests {
194224

195225
#[test]
196226
fn add_after() {
197-
let mut group = PluginGroupBuilder::default();
198-
group.add(PluginA);
199-
group.add(PluginB);
200-
group.add_after::<PluginA, PluginC>(PluginC);
227+
let group = PluginGroupBuilder::default()
228+
.add(PluginA)
229+
.add(PluginB)
230+
.add_after::<PluginA, PluginC>(PluginC);
201231

202232
assert_eq!(
203233
group.order,
@@ -211,10 +241,10 @@ mod tests {
211241

212242
#[test]
213243
fn add_before() {
214-
let mut group = PluginGroupBuilder::default();
215-
group.add(PluginA);
216-
group.add(PluginB);
217-
group.add_before::<PluginB, PluginC>(PluginC);
244+
let group = PluginGroupBuilder::default()
245+
.add(PluginA)
246+
.add(PluginB)
247+
.add_before::<PluginB, PluginC>(PluginC);
218248

219249
assert_eq!(
220250
group.order,
@@ -228,11 +258,11 @@ mod tests {
228258

229259
#[test]
230260
fn readd() {
231-
let mut group = PluginGroupBuilder::default();
232-
group.add(PluginA);
233-
group.add(PluginB);
234-
group.add(PluginC);
235-
group.add(PluginB);
261+
let group = PluginGroupBuilder::default()
262+
.add(PluginA)
263+
.add(PluginB)
264+
.add(PluginC)
265+
.add(PluginB);
236266

237267
assert_eq!(
238268
group.order,
@@ -246,11 +276,11 @@ mod tests {
246276

247277
#[test]
248278
fn readd_after() {
249-
let mut group = PluginGroupBuilder::default();
250-
group.add(PluginA);
251-
group.add(PluginB);
252-
group.add(PluginC);
253-
group.add_after::<PluginA, PluginC>(PluginC);
279+
let group = PluginGroupBuilder::default()
280+
.add(PluginA)
281+
.add(PluginB)
282+
.add(PluginC)
283+
.add_after::<PluginA, PluginC>(PluginC);
254284

255285
assert_eq!(
256286
group.order,
@@ -264,11 +294,11 @@ mod tests {
264294

265295
#[test]
266296
fn readd_before() {
267-
let mut group = PluginGroupBuilder::default();
268-
group.add(PluginA);
269-
group.add(PluginB);
270-
group.add(PluginC);
271-
group.add_before::<PluginB, PluginC>(PluginC);
297+
let group = PluginGroupBuilder::default()
298+
.add(PluginA)
299+
.add(PluginB)
300+
.add(PluginC)
301+
.add_before::<PluginB, PluginC>(PluginC);
272302

273303
assert_eq!(
274304
group.order,

crates/bevy_asset/src/asset_server.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ pub struct AssetServerInternal {
8383
/// # use bevy_asset::*;
8484
/// # use bevy_app::*;
8585
/// # let mut app = App::new();
86-
/// // AssetServerSettings must be inserted before adding the AssetPlugin or DefaultPlugins.
87-
/// app.insert_resource(AssetServerSettings {
86+
/// // The asset plugin can be configured to watch for asset changes.
87+
/// app.add_plugin(AssetPlugin {
8888
/// watch_for_changes: true,
8989
/// ..Default::default()
9090
/// });
@@ -293,7 +293,7 @@ impl AssetServer {
293293
/// `"CARGO_MANIFEST_DIR"` is automatically set to the root folder of your crate (workspace).
294294
///
295295
/// The name of the asset folder is set inside the
296-
/// [`AssetServerSettings`](crate::AssetServerSettings) resource. The default name is
296+
/// [`AssetPlugin`](crate::AssetPlugin). The default name is
297297
/// `"assets"`.
298298
///
299299
/// The asset is loaded asynchronously, and will generally not be available by the time

crates/bevy_asset/src/assets.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ mod tests {
430430
struct MyAsset;
431431
let mut app = App::new();
432432
app.add_plugin(bevy_core::CorePlugin)
433-
.add_plugin(crate::AssetPlugin);
433+
.add_plugin(crate::AssetPlugin::default());
434434
app.add_asset::<MyAsset>();
435435
let mut assets_before = app.world.resource_mut::<Assets<MyAsset>>();
436436
let handle = assets_before.add(MyAsset);

crates/bevy_asset/src/debug_asset_server.rs

+5-8
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ use std::{
1616
};
1717

1818
use crate::{
19-
Asset, AssetEvent, AssetPlugin, AssetServer, AssetServerSettings, Assets, FileAssetIo, Handle,
20-
HandleUntyped,
19+
Asset, AssetEvent, AssetPlugin, AssetServer, Assets, FileAssetIo, Handle, HandleUntyped,
2120
};
2221

2322
/// A helper [`App`] used for hot reloading internal assets, which are compiled-in to Bevy plugins.
@@ -75,12 +74,10 @@ impl Plugin for DebugAssetServerPlugin {
7574
.build()
7675
});
7776
let mut debug_asset_app = App::new();
78-
debug_asset_app
79-
.insert_resource(AssetServerSettings {
80-
asset_folder: "crates".to_string(),
81-
watch_for_changes: true,
82-
})
83-
.add_plugin(AssetPlugin);
77+
debug_asset_app.add_plugin(AssetPlugin {
78+
asset_folder: "crates".to_string(),
79+
watch_for_changes: true,
80+
});
8481
app.insert_non_send_resource(DebugAssetApp(debug_asset_app));
8582
app.add_system(run_debug_asset_app);
8683
}

0 commit comments

Comments
 (0)