Skip to content

Commit

Permalink
bevyengine#4231: panic when App::run() is called from Plugin::build() (
Browse files Browse the repository at this point in the history
…bevyengine#4241)

# Objective

Fixes bevyengine#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()
  • Loading branch information
Vrixyz authored and ItsDoot committed Feb 1, 2023
1 parent f1546a1 commit 49dc52e
Showing 1 changed file with 24 additions and 0 deletions.
24 changes: 24 additions & 0 deletions crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -136,6 +138,7 @@ impl App {
sub_apps: HashMap::default(),
plugin_registry: Vec::default(),
plugin_name_added: Default::default(),
is_building_plugin: false,
}
}

Expand All @@ -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);
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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);
}
}

0 comments on commit 49dc52e

Please sign in to comment.