From 49a3220577399220bd5616476c616a25a4ff4d80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Sat, 19 Feb 2022 02:54:11 +0100 Subject: [PATCH 1/3] can only add a plugin once --- crates/bevy_app/src/app.rs | 109 ++++++++++++++++++++++++++-- crates/bevy_app/src/plugin.rs | 10 ++- crates/bevy_app/src/plugin_group.rs | 17 ++++- 3 files changed, 126 insertions(+), 10 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index fe48094497f5a..a3f099fc187d8 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -11,7 +11,7 @@ use bevy_ecs::{ world::World, }; use bevy_utils::{tracing::debug, HashMap}; -use std::fmt::Debug; +use std::{any::TypeId, fmt::Debug}; #[cfg(feature = "trace")] use bevy_utils::tracing::info_span; @@ -57,6 +57,7 @@ pub struct App { /// A container of [`Stage`]s set to be run in a linear order. pub schedule: Schedule, sub_apps: HashMap, SubApp>, + plugins: HashMap>, } /// Each `SubApp` has its own [`Schedule`] and [`World`], enabling a separation of concerns. @@ -100,6 +101,7 @@ impl App { schedule: Default::default(), runner: Box::new(run_once), sub_apps: HashMap::default(), + plugins: HashMap::default(), } } @@ -368,7 +370,6 @@ impl App { stage_label: impl StageLabel, system: impl IntoSystemDescriptor, ) -> &mut Self { - use std::any::TypeId; assert!( stage_label.type_id() != TypeId::of::(), "add systems to a startup stage using App::add_startup_system_to_stage" @@ -403,7 +404,6 @@ impl App { stage_label: impl StageLabel, system_set: SystemSet, ) -> &mut Self { - use std::any::TypeId; assert!( stage_label.type_id() != TypeId::of::(), "add system sets to a startup stage using App::add_startup_system_set_to_stage" @@ -749,6 +749,14 @@ impl App { self } + /// Check that a plugin has already been added to the app. + pub fn is_plugin_added(&self) -> bool + where + T: Plugin, + { + self.plugins.contains_key(&std::any::TypeId::of::()) + } + /// Adds a single [`Plugin`]. /// /// One of Bevy's core principles is modularity. All Bevy engine features are implemented @@ -767,11 +775,41 @@ impl App { where T: Plugin, { - debug!("added plugin: {}", plugin.name()); + debug!("added plugin {}", plugin.name()); + if plugin.is_unique() { + self.register_plugin(&std::any::TypeId::of::(), plugin.name(), None); + } plugin.build(self); self } + /// Checks that a plugin has not already been added to an application. It will panic with an + /// helpful message the second time a plugin is being added. + pub(crate) fn register_plugin( + &mut self, + plugin_type: &TypeId, + plugin_name: &str, + from_group: Option<&'static str>, + ) { + if let Some(existing_from_group) = self.plugins.insert(*plugin_type, from_group) { + match (from_group, existing_from_group) { + (None, None) => panic!("Plugin \"{}\" was already added", plugin_name), + (None, Some(existing_from_group)) => panic!( + "Plugin \"{}\" was already added with group \"{}\"", + plugin_name, existing_from_group + ), + (Some(from_group), None) => panic!( + "Plugin \"{}\" from group \"{}\" was already added", + plugin_name, from_group + ), + (Some(from_group), Some(existing_from_group)) => panic!( + "Plugin \"{}\" from group \"{}\" was already added with group \"{}\"", + plugin_name, from_group, existing_from_group + ), + }; + } + } + /// Adds a group of [`Plugin`]s. /// /// [`Plugin`]s can be grouped into a set by using a [`PluginGroup`]. @@ -794,9 +832,15 @@ impl App { /// .add_plugins(MinimalPlugins); /// ``` pub fn add_plugins(&mut self, mut group: T) -> &mut Self { + if self.plugins.insert(TypeId::of::(), None).is_some() { + panic!( + "Plugin Group \"{}\" was already added", + std::any::type_name::() + ); + } let mut plugin_group_builder = PluginGroupBuilder::default(); group.build(&mut plugin_group_builder); - plugin_group_builder.finish(self); + plugin_group_builder.finish::(self); self } @@ -835,10 +879,17 @@ impl App { T: PluginGroup, F: FnOnce(&mut PluginGroupBuilder) -> &mut PluginGroupBuilder, { + if self.plugins.insert(TypeId::of::(), None).is_some() { + panic!( + "Plugin Group \"{}\" was already added", + std::any::type_name::() + ); + } + let mut plugin_group_builder = PluginGroupBuilder::default(); group.build(&mut plugin_group_builder); func(&mut plugin_group_builder); - plugin_group_builder.finish(self); + plugin_group_builder.finish::(self); self } @@ -928,3 +979,49 @@ fn run_once(mut app: App) { /// frame is over. #[derive(Debug, Clone, Default)] pub struct AppExit; + +#[cfg(test)] +mod tests { + use crate::{App, Plugin}; + + struct PluginA; + impl Plugin for PluginA { + fn build(&self, _app: &mut crate::App) {} + } + struct PluginB; + impl Plugin for PluginB { + fn build(&self, _app: &mut crate::App) {} + } + struct PluginC(T); + impl Plugin for PluginC { + fn build(&self, _app: &mut crate::App) {} + } + struct PluginD; + impl Plugin for PluginD { + fn build(&self, _app: &mut crate::App) {} + fn is_unique(&self) -> bool { + false + } + } + + #[test] + fn can_add_two_plugins() { + App::new().add_plugin(PluginA).add_plugin(PluginB); + } + + #[test] + #[should_panic] + fn cant_add_twice_the_same_plugin() { + App::new().add_plugin(PluginA).add_plugin(PluginA); + } + + #[test] + fn can_add_twice_the_same_plugin_with_different_type_param() { + App::new().add_plugin(PluginC(0)).add_plugin(PluginC(true)); + } + + #[test] + fn can_add_twice_the_same_plugin_not_unique() { + App::new().add_plugin(PluginD).add_plugin(PluginD); + } +} diff --git a/crates/bevy_app/src/plugin.rs b/crates/bevy_app/src/plugin.rs index e6e70cf856029..04bef9e5bb41b 100644 --- a/crates/bevy_app/src/plugin.rs +++ b/crates/bevy_app/src/plugin.rs @@ -4,7 +4,10 @@ use std::any::Any; /// A collection of Bevy app logic and configuration. /// /// Plugins configure an [`App`]. When an [`App`] registers a plugin, -/// the plugin's [`Plugin::build`] function is run. +/// the plugin's [`Plugin::build`] function is run. By default, a plugin +/// can only be added once to an [`App`]. If the plugin may need to be +/// added twice or more, the function [`is_unique`](Plugin::is_unique) +/// should be overriden to return `false`. pub trait Plugin: Any + Send + Sync { /// Configures the [`App`] to which this plugin is added. fn build(&self, app: &mut App); @@ -12,6 +15,11 @@ pub trait Plugin: Any + Send + Sync { fn name(&self) -> &str { std::any::type_name::() } + /// If the plugin can be instantiated several times in an [`App`](crate::App), override this + /// method to return `false`. + fn is_unique(&self) -> bool { + true + } } /// A type representing an unsafe function that returns a mutable pointer to a [`Plugin`]. diff --git a/crates/bevy_app/src/plugin_group.rs b/crates/bevy_app/src/plugin_group.rs index 31b4a044438ff..c316f4d808e23 100644 --- a/crates/bevy_app/src/plugin_group.rs +++ b/crates/bevy_app/src/plugin_group.rs @@ -3,7 +3,7 @@ use bevy_utils::{tracing::debug, HashMap}; use std::any::TypeId; /// Combines multiple [`Plugin`]s into a single unit. -pub trait PluginGroup { +pub trait PluginGroup: 'static { /// Configures the [`Plugin`]s that are to be added. fn build(&mut self, group: &mut PluginGroupBuilder); } @@ -110,11 +110,22 @@ impl PluginGroupBuilder { } /// Consumes the [`PluginGroupBuilder`] and [builds](Plugin::build) the contained [`Plugin`]s. - pub fn finish(self, app: &mut App) { + pub fn finish(self, app: &mut App) { for ty in self.order.iter() { if let Some(entry) = self.plugins.get(ty) { if entry.enabled { - debug!("added plugin: {}", entry.plugin.name()); + debug!( + "added plugin {} from group {}", + entry.plugin.name(), + std::any::type_name::() + ); + if entry.plugin.is_unique() { + app.register_plugin( + ty, + entry.plugin.name(), + Some(std::any::type_name::()), + ); + } entry.plugin.build(app); } } From 81f6027bbb63097c69cf0e311b1ef7289fc168b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Sat, 19 Feb 2022 04:04:59 +0100 Subject: [PATCH 2/3] remove useless qualification --- crates/bevy_app/src/app.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index a3f099fc187d8..ff97f9c603c11 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -754,7 +754,7 @@ impl App { where T: Plugin, { - self.plugins.contains_key(&std::any::TypeId::of::()) + self.plugins.contains_key(&TypeId::of::()) } /// Adds a single [`Plugin`]. From 2fc76ec7f33eb21e504be9109579160a54010b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Sat, 19 Feb 2022 04:19:37 +0100 Subject: [PATCH 3/3] doc comment suggestion Co-Authored-By: Alice Cecile --- crates/bevy_app/src/plugin.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_app/src/plugin.rs b/crates/bevy_app/src/plugin.rs index 04bef9e5bb41b..af33e9514d251 100644 --- a/crates/bevy_app/src/plugin.rs +++ b/crates/bevy_app/src/plugin.rs @@ -15,8 +15,8 @@ pub trait Plugin: Any + Send + Sync { fn name(&self) -> &str { std::any::type_name::() } - /// If the plugin can be instantiated several times in an [`App`](crate::App), override this - /// method to return `false`. + /// If the plugin can be meaningfully instantiated several times in an [`App`](crate::App), + /// override this method to return `false`. fn is_unique(&self) -> bool { true }