Update App:is_plugin_added to work inside Plugin::finish and `Plu…

…gin::clean` (#12761)

# Objective

I have been trying to check for the existing of some plugins via
`App::is_plugin_added` to conditionally run some behaviour in the
`Plugin::finish` part of my plugin, before realizing that the plugin
registry is actually not available during this step.
This is because the `App::is_plugin_added` using the plugin registry to
check for previous registration.

## Solution

- Switch the `App::is_plugin_added` to use the list of plugin names to
check for previous registrations
- Add a unit test showcasing that `App::is_plugin_added` works during
`Plugin::finish`

crates/bevy_app/src/app.rs

@@ -209,15 +209,15 @@ impl App {
         let mut overall_plugins_state = match self.main_mut().plugins_state {
             PluginsState::Adding => {
                 let mut state = PluginsState::Ready;
-                let plugins = std::mem::take(&mut self.main_mut().plugins);
-                for plugin in &plugins.registry {
+                let plugins = std::mem::take(&mut self.main_mut().plugin_registry);
+                for plugin in &plugins {
                     // plugins installed to main need to see all sub-apps
                     if !plugin.ready(self) {
                         state = PluginsState::Adding;
                         break;
                     }
                 }
-                self.main_mut().plugins = plugins;
+                self.main_mut().plugin_registry = plugins;
                 state
             }
             state => state,
@@ -235,12 +235,12 @@ impl App {
     /// plugins are ready, but can be useful for situations where you want to use [`App::update`].
     pub fn finish(&mut self) {
         // plugins installed to main should see all sub-apps
-        let plugins = std::mem::take(&mut self.main_mut().plugins);
-        for plugin in &plugins.registry {
+        let plugins = std::mem::take(&mut self.main_mut().plugin_registry);
+        for plugin in &plugins {
             plugin.finish(self);
         }
         let main = self.main_mut();
-        main.plugins = plugins;
+        main.plugin_registry = plugins;
         main.plugins_state = PluginsState::Finished;
         self.sub_apps.iter_mut().skip(1).for_each(|s| s.finish());
     }
@@ -249,12 +249,12 @@ impl App {
     /// [`App::finish`], but can be useful for situations where you want to use [`App::update`].
     pub fn cleanup(&mut self) {
         // plugins installed to main should see all sub-apps
-        let plugins = std::mem::take(&mut self.main_mut().plugins);
-        for plugin in &plugins.registry {
+        let plugins = std::mem::take(&mut self.main_mut().plugin_registry);
+        for plugin in &plugins {
             plugin.cleanup(self);
         }
         let main = self.main_mut();
-        main.plugins = plugins;
+        main.plugin_registry = plugins;
         main.plugins_state = PluginsState::Cleaned;
         self.sub_apps.iter_mut().skip(1).for_each(|s| s.cleanup());
     }
@@ -490,8 +490,7 @@ impl App {
         if plugin.is_unique()
             && !self
                 .main_mut()
-                .plugins
-                .names
+                .plugin_names
                 .insert(plugin.name().to_string())
         {
             Err(AppError::DuplicatePlugin {
@@ -501,10 +500,9 @@ impl App {
 
         // Reserve position in the plugin registry. If the plugin adds more plugins,
         // they'll all end up in insertion order.
-        let index = self.main().plugins.registry.len();
+        let index = self.main().plugin_registry.len();
         self.main_mut()
-            .plugins
-            .registry
+            .plugin_registry
             .push(Box::new(PlaceholderPlugin));
 
         self.main_mut().plugin_build_depth += 1;
@@ -515,7 +513,7 @@ impl App {
             resume_unwind(payload);
         }
 
-        self.main_mut().plugins.registry[index] = plugin;
+        self.main_mut().plugin_registry[index] = plugin;
         Ok(self)
     }
 
@@ -1008,6 +1006,18 @@ mod tests {
         }
     }
 
+    struct PluginE;
+
+    impl Plugin for PluginE {
+        fn build(&self, _app: &mut App) {}
+
+        fn finish(&self, app: &mut App) {
+            if app.is_plugin_added::<PluginA>() {
+                panic!("cannot run if PluginA is already registered");
+            }
+        }
+    }
+
     #[test]
     fn can_add_two_plugins() {
         App::new().add_plugins((PluginA, PluginB));
@@ -1078,6 +1088,15 @@ mod tests {
         assert_eq!(app.world().entities().len(), 2);
     }
 
+    #[test]
+    #[should_panic]
+    fn test_is_plugin_added_works_during_finish() {
+        let mut app = App::new();
+        app.add_plugins(PluginA);
+        app.add_plugins(PluginE);
+        app.finish();
+    }
+
     #[test]
     fn test_derive_app_label() {
         use super::AppLabel;

crates/bevy_app/src/sub_app.rs

@@ -10,17 +10,11 @@ use bevy_ecs::{
 };
 #[cfg(feature = "trace")]
 use bevy_utils::tracing::info_span;
-use bevy_utils::{default, HashMap, HashSet};
+use bevy_utils::{HashMap, HashSet};
 use std::fmt::Debug;
 
 type ExtractFn = Box<dyn Fn(&mut World, &mut World) + Send>;
 
-#[derive(Default)]
-pub(crate) struct PluginStore {
-    pub(crate) registry: Vec<Box<dyn Plugin>>,
-    pub(crate) names: HashSet<String>,
-}
-
 /// A secondary application with its own [`World`]. These can run independently of each other.
 ///
 /// These are useful for situations where certain processes (e.g. a render thread) need to be kept
@@ -67,8 +61,11 @@ pub(crate) struct PluginStore {
 pub struct SubApp {
     /// The data of this application.
     world: World,
-    /// Metadata for installed plugins.
-    pub(crate) plugins: PluginStore,
+    /// List of plugins that have been added.
+    pub(crate) plugin_registry: Vec<Box<dyn Plugin>>,
+    /// The names of plugins that have been added to this app. (used to track duplicates and
+    /// already-registered plugins)
+    pub(crate) plugin_names: HashSet<String>,
     /// Panics if an update is attempted while plugins are building.
     pub(crate) plugin_build_depth: usize,
     pub(crate) plugins_state: PluginsState,
@@ -91,7 +88,8 @@ impl Default for SubApp {
         world.init_resource::<Schedules>();
         Self {
             world,
-            plugins: default(),
+            plugin_registry: Vec::default(),
+            plugin_names: HashSet::default(),
             plugin_build_depth: 0,
             plugins_state: PluginsState::Adding,
             update_schedule: None,
@@ -380,19 +378,15 @@ impl SubApp {
     where
         T: Plugin,
     {
-        self.plugins
-            .registry
-            .iter()
-            .any(|p| p.downcast_ref::<T>().is_some())
+        self.plugin_names.contains(std::any::type_name::<T>())
     }
 
     /// See [`App::get_added_plugins`].
     pub fn get_added_plugins<T>(&self) -> Vec<&T>
     where
         T: Plugin,
     {
-        self.plugins
-            .registry
+        self.plugin_registry
             .iter()
             .filter_map(|p| p.downcast_ref())
             .collect()
@@ -409,16 +403,16 @@ impl SubApp {
         match self.plugins_state {
             PluginsState::Adding => {
                 let mut state = PluginsState::Ready;
-                let plugins = std::mem::take(&mut self.plugins);
+                let plugins = std::mem::take(&mut self.plugin_registry);
                 self.run_as_app(|app| {
-                    for plugin in &plugins.registry {
+                    for plugin in &plugins {
                         if !plugin.ready(app) {
                             state = PluginsState::Adding;
                             return;
                         }
                     }
                 });
-                self.plugins = plugins;
+                self.plugin_registry = plugins;
                 state
             }
             state => state,
@@ -427,25 +421,25 @@ impl SubApp {
 
     /// Runs [`Plugin::finish`] for each plugin.
     pub fn finish(&mut self) {
-        let plugins = std::mem::take(&mut self.plugins);
+        let plugins = std::mem::take(&mut self.plugin_registry);
         self.run_as_app(|app| {
-            for plugin in &plugins.registry {
+            for plugin in &plugins {
                 plugin.finish(app);
             }
         });
-        self.plugins = plugins;
+        self.plugin_registry = plugins;
         self.plugins_state = PluginsState::Finished;
     }
 
     /// Runs [`Plugin::cleanup`] for each plugin.
     pub fn cleanup(&mut self) {
-        let plugins = std::mem::take(&mut self.plugins);
+        let plugins = std::mem::take(&mut self.plugin_registry);
         self.run_as_app(|app| {
-            for plugin in &plugins.registry {
+            for plugin in &plugins {
                 plugin.cleanup(app);
             }
         });
-        self.plugins = plugins;
+        self.plugin_registry = plugins;
         self.plugins_state = PluginsState::Cleaned;
     }