From 49dc52e41652da23500f131afb5166d10de2e1cf Mon Sep 17 00:00:00 2001
From: Thierry Berger <contact@thierryberger.com>
Date: Sat, 24 Dec 2022 23:43:41 +0000
Subject: [PATCH] #4231: panic when App::run() is called from Plugin::build()
 (#4241)

# Objective

Fixes #4231.

## Solution

This PR implements the solution suggested by @bjorn3 : Use an internal property within `App` to detect `App::run()` calls from `Plugin::build()`.

---

## Changelog

- panic when App::run() is called from Plugin::build()
---
 crates/bevy_app/src/app.rs | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs
index d88a82059126c9..16caea1be6c478 100644
--- a/crates/bevy_app/src/app.rs
+++ b/crates/bevy_app/src/app.rs
@@ -73,6 +73,8 @@ pub struct App {
     sub_apps: HashMap<AppLabelId, SubApp>,
     plugin_registry: Vec<Box<dyn Plugin>>,
     plugin_name_added: HashSet<String>,
+    /// A private marker to prevent incorrect calls to `App::run()` from `Plugin::build()`
+    is_building_plugin: bool,
 }
 
 impl Debug for App {
@@ -136,6 +138,7 @@ impl App {
             sub_apps: HashMap::default(),
             plugin_registry: Vec::default(),
             plugin_name_added: Default::default(),
+            is_building_plugin: false,
         }
     }
 
@@ -161,11 +164,18 @@ impl App {
     ///
     /// Finalizes the [`App`] configuration. For general usage, see the example on the item
     /// level documentation.
+    ///
+    /// # Panics
+    ///
+    /// Panics if called from `Plugin::build()`, because it would prevent other plugins to properly build.
     pub fn run(&mut self) {
         #[cfg(feature = "trace")]
         let _bevy_app_run_span = info_span!("bevy_app").entered();
 
         let mut app = std::mem::replace(self, App::empty());
+        if app.is_building_plugin {
+            panic!("App::run() was called from within Plugin::Build(), which is not allowed.");
+        }
         let runner = std::mem::replace(&mut app.runner, Box::new(run_once));
         (runner)(app);
     }
@@ -858,7 +868,9 @@ impl App {
                 plugin_name: plugin.name().to_string(),
             })?;
         }
+        self.is_building_plugin = true;
         plugin.build(self);
+        self.is_building_plugin = false;
         self.plugin_registry.push(plugin);
         Ok(self)
     }
@@ -1105,4 +1117,16 @@ mod tests {
     fn can_add_twice_the_same_plugin_not_unique() {
         App::new().add_plugin(PluginD).add_plugin(PluginD);
     }
+
+    #[test]
+    #[should_panic]
+    fn cant_call_app_run_from_plugin_build() {
+        struct PluginRun;
+        impl Plugin for PluginRun {
+            fn build(&self, app: &mut crate::App) {
+                app.run();
+            }
+        }
+        App::new().add_plugin(PluginRun);
+    }
 }