Unique plugin (#6411)

# Objective

- Make it impossible to add a plugin twice
- This is going to be more a risk for plugins with configurations, to avoid things like `App::new().add_plugins(DefaultPlugins).add_plugin(ImagePlugin::default_nearest())`

## Solution

- Panic when a plugin is added twice
- It's still possible to mark a plugin as not unique by overriding `is_unique`
- ~~Simpler version of~~ #3988 (not simpler anymore because of how `PluginGroupBuilder` implements `PluginGroup`)

crates/bevy_app/src/app.rs

@@ -10,7 +10,7 @@ use bevy_ecs::{
     system::Resource,
     world::World,
 };
-use bevy_utils::{tracing::debug, HashMap};
+use bevy_utils::{tracing::debug, HashMap, HashSet};
 use std::fmt::Debug;
 
 #[cfg(feature = "trace")]
@@ -27,6 +27,10 @@ bevy_utils::define_label!(
 #[derive(Resource, Clone, bevy_derive::Deref, bevy_derive::DerefMut, Default)]
 pub struct AppTypeRegistry(pub bevy_reflect::TypeRegistryArc);
 
+pub(crate) enum AppError {
+    DuplicatePlugin { plugin_name: String },
+}
+
 #[allow(clippy::needless_doctest_main)]
 /// A container of app logic and data.
 ///
@@ -68,6 +72,7 @@ pub struct App {
     pub schedule: Schedule,
     sub_apps: HashMap<AppLabelId, SubApp>,
     plugin_registry: Vec<Box<dyn Plugin>>,
+    plugin_name_added: HashSet<String>,
 }
 
 impl Debug for App {
@@ -132,6 +137,7 @@ impl App {
             runner: Box::new(run_once),
             sub_apps: HashMap::default(),
             plugin_registry: Vec::default(),
+            plugin_name_added: Default::default(),
         }
     }
 
@@ -822,19 +828,37 @@ impl App {
     /// # }
     /// App::new().add_plugin(bevy_log::LogPlugin::default());
     /// ```
+    ///
+    /// # Panics
+    ///
+    /// Panics if the plugin was already added to the application.
     pub fn add_plugin<T>(&mut self, plugin: T) -> &mut Self
     where
         T: Plugin,
     {
-        self.add_boxed_plugin(Box::new(plugin))
+        match self.add_boxed_plugin(Box::new(plugin)) {
+            Ok(app) => app,
+            Err(AppError::DuplicatePlugin { plugin_name }) => panic!(
+                "Error adding plugin {}: : plugin was already added in application",
+                plugin_name
+            ),
+        }
     }
 
     /// Boxed variant of `add_plugin`, can be used from a [`PluginGroup`]
-    pub(crate) fn add_boxed_plugin(&mut self, plugin: Box<dyn Plugin>) -> &mut Self {
+    pub(crate) fn add_boxed_plugin(
+        &mut self,
+        plugin: Box<dyn Plugin>,
+    ) -> Result<&mut Self, AppError> {
         debug!("added plugin: {}", plugin.name());
+        if plugin.is_unique() && !self.plugin_name_added.insert(plugin.name().to_string()) {
+            Err(AppError::DuplicatePlugin {
+                plugin_name: plugin.name().to_string(),
+            })?;
+        }
         plugin.build(self);
         self.plugin_registry.push(plugin);
-        self
+        Ok(self)
     }
 
     /// Checks if a [`Plugin`] has already been added.
@@ -897,6 +921,10 @@ impl App {
     /// App::new()
     ///     .add_plugins(MinimalPlugins);
     /// ```
+    ///
+    /// # Panics
+    ///
+    /// Panics if one of the plugin in the group was already added to the application.
     pub fn add_plugins<T: PluginGroup>(&mut self, group: T) -> &mut Self {
         let builder = group.build();
         builder.finish(self);
@@ -1030,3 +1058,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>(T);
+    impl<T: Send + Sync + 'static> Plugin for PluginC<T> {
+        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);
+    }
+}

crates/bevy_app/src/plugin.rs

@@ -6,7 +6,13 @@ 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()`](Self::is_unique)
+/// should be overriden to return `false`. Plugins are considered duplicate if they have the same
+/// [`name()`](Self::name). The default `name()` implementation returns the type name, which means
+/// generic plugins with different type parameters will not be considered duplicates.
 pub trait Plugin: Downcast + Any + Send + Sync {
     /// Configures the [`App`] to which this plugin is added.
     fn build(&self, app: &mut App);
@@ -14,6 +20,11 @@ pub trait Plugin: Downcast + Any + Send + Sync {
     fn name(&self) -> &str {
         std::any::type_name::<Self>()
     }
+    /// 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
+    }
 }
 
 impl_downcast!(Plugin);

crates/bevy_app/src/plugin_group.rs

@@ -1,4 +1,4 @@
-use crate::{App, Plugin};
+use crate::{App, AppError, Plugin};
 use bevy_utils::{tracing::debug, tracing::warn, HashMap};
 use std::any::TypeId;
 
@@ -6,6 +6,10 @@ use std::any::TypeId;
 pub trait PluginGroup: Sized {
     /// Configures the [`Plugin`]s that are to be added.
     fn build(self) -> PluginGroupBuilder;
+    /// Configures a name for the [`PluginGroup`] which is primarily used for debugging.
+    fn name() -> String {
+        std::any::type_name::<Self>().to_string()
+    }
     /// Sets the value of the given [`Plugin`], if it exists
     fn set<T: Plugin>(self, plugin: T) -> PluginGroupBuilder {
         self.build().set(plugin)
@@ -27,13 +31,22 @@ impl PluginGroup for PluginGroupBuilder {
 /// Provides a build ordering to ensure that [`Plugin`]s which produce/require a [`Resource`](bevy_ecs::system::Resource)
 /// are built before/after dependent/depending [`Plugin`]s. [`Plugin`]s inside the group
 /// can be disabled, enabled or reordered.
-#[derive(Default)]
 pub struct PluginGroupBuilder {
+    group_name: String,
     plugins: HashMap<TypeId, PluginEntry>,
     order: Vec<TypeId>,
 }
 
 impl PluginGroupBuilder {
+    /// Start a new builder for the [`PluginGroup`].
+    pub fn start<PG: PluginGroup>() -> Self {
+        Self {
+            group_name: PG::name(),
+            plugins: Default::default(),
+            order: Default::default(),
+        }
+    }
+
     /// Finds the index of a target [`Plugin`]. Panics if the target's [`TypeId`] is not found.
     fn index_of<Target: Plugin>(&self) -> usize {
         let index = self
@@ -155,12 +168,24 @@ impl PluginGroupBuilder {
 
     /// Consumes the [`PluginGroupBuilder`] and [builds](Plugin::build) the contained [`Plugin`]s
     /// in the order specified.
+    ///
+    /// # Panics
+    ///
+    /// Panics if one of the plugin in the group was already added to the application.
     pub fn finish(mut self, app: &mut App) {
         for ty in &self.order {
             if let Some(entry) = self.plugins.remove(ty) {
                 if entry.enabled {
                     debug!("added plugin: {}", entry.plugin.name());
-                    app.add_boxed_plugin(entry.plugin);
+                    if let Err(AppError::DuplicatePlugin { plugin_name }) =
+                        app.add_boxed_plugin(entry.plugin)
+                    {
+                        panic!(
+                            "Error adding plugin {} in group {}: plugin was already added in application",
+                            plugin_name,
+                            self.group_name
+                        );
+                    }
                 }
             }
         }
@@ -181,14 +206,14 @@ pub struct NoopPluginGroup;
 
 impl PluginGroup for NoopPluginGroup {
     fn build(self) -> PluginGroupBuilder {
-        PluginGroupBuilder::default()
+        PluginGroupBuilder::start::<Self>()
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::PluginGroupBuilder;
-    use crate::{App, Plugin};
+    use crate::{App, NoopPluginGroup, Plugin};
 
     struct PluginA;
     impl Plugin for PluginA {
@@ -207,7 +232,7 @@ mod tests {
 
     #[test]
     fn basic_ordering() {
-        let group = PluginGroupBuilder::default()
+        let group = PluginGroupBuilder::start::<NoopPluginGroup>()
             .add(PluginA)
             .add(PluginB)
             .add(PluginC);
@@ -224,7 +249,7 @@ mod tests {
 
     #[test]
     fn add_after() {
-        let group = PluginGroupBuilder::default()
+        let group = PluginGroupBuilder::start::<NoopPluginGroup>()
             .add(PluginA)
             .add(PluginB)
             .add_after::<PluginA, PluginC>(PluginC);
@@ -241,7 +266,7 @@ mod tests {
 
     #[test]
     fn add_before() {
-        let group = PluginGroupBuilder::default()
+        let group = PluginGroupBuilder::start::<NoopPluginGroup>()
             .add(PluginA)
             .add(PluginB)
             .add_before::<PluginB, PluginC>(PluginC);
@@ -258,7 +283,7 @@ mod tests {
 
     #[test]
     fn readd() {
-        let group = PluginGroupBuilder::default()
+        let group = PluginGroupBuilder::start::<NoopPluginGroup>()
             .add(PluginA)
             .add(PluginB)
             .add(PluginC)
@@ -276,7 +301,7 @@ mod tests {
 
     #[test]
     fn readd_after() {
-        let group = PluginGroupBuilder::default()
+        let group = PluginGroupBuilder::start::<NoopPluginGroup>()
             .add(PluginA)
             .add(PluginB)
             .add(PluginC)
@@ -294,7 +319,7 @@ mod tests {
 
     #[test]
     fn readd_before() {
-        let group = PluginGroupBuilder::default()
+        let group = PluginGroupBuilder::start::<NoopPluginGroup>()
             .add(PluginA)
             .add(PluginB)
             .add(PluginC)

crates/bevy_internal/src/default_plugins.rs

@@ -26,7 +26,7 @@ pub struct DefaultPlugins;
 
 impl PluginGroup for DefaultPlugins {
     fn build(self) -> PluginGroupBuilder {
-        let mut group = PluginGroupBuilder::default();
+        let mut group = PluginGroupBuilder::start::<Self>();
         group = group
             .add(bevy_log::LogPlugin::default())
             .add(bevy_core::CorePlugin::default())
@@ -127,7 +127,7 @@ pub struct MinimalPlugins;
 
 impl PluginGroup for MinimalPlugins {
     fn build(self) -> PluginGroupBuilder {
-        PluginGroupBuilder::default()
+        PluginGroupBuilder::start::<Self>()
             .add(bevy_core::CorePlugin::default())
             .add(bevy_time::TimePlugin::default())
             .add(bevy_app::ScheduleRunnerPlugin::default())

examples/app/plugin_group.rs

@@ -26,7 +26,7 @@ pub struct HelloWorldPlugins;
 
 impl PluginGroup for HelloWorldPlugins {
     fn build(self) -> PluginGroupBuilder {
-        PluginGroupBuilder::default()
+        PluginGroupBuilder::start::<Self>()
             .add(PrintHelloPlugin)
             .add(PrintWorldPlugin)
     }