From 0cfdbddf2e6350b11bd9dce2d4fe89ac9c99aec9 Mon Sep 17 00:00:00 2001 From: Ida Iyes Date: Tue, 1 Nov 2022 11:35:44 +0000 Subject: [PATCH] bevy_dynamic_plugin: make it possible to handle loading errors (#6437) # Objective Currently, `bevy_dynamic_plugin` simply panics on error. This makes it impossible to handle failures in applications that use this feature. For example, I'd like to build an optional expansion for my game, that may not be distributed to all users. I want to use `bevy_dynamic_plugin` for loading it. I want my game to try to load it on startup, but continue without it if it cannot be loaded. ## Solution - Make the `dynamically_load_plugin` function return a `Result`, so it can gracefully return loading errors. - Create an error enum type, to provide useful information about the kind of error. This adds `thiserror` to the dependencies of `bevy_dynamic_plugin`, but that dependency is already used in other parts of bevy (such as `bevy_asset`), so not a big deal. I chose not to change the behavior of the builder method in the App extension trait. I kept it as panicking. There is no clean way (that I'm aware of) to make a builder-style API that has fallible methods. So it is either a panic or a warning. I feel the panic is more appropriate. --- ## Changelog ### Changed - `bevy_dynamic_plugin::dynamically_load_plugin` now returns `Result` instead of panicking, to allow for error handling --- crates/bevy_dynamic_plugin/Cargo.toml | 1 + crates/bevy_dynamic_plugin/src/loader.rs | 24 +++++++++++++++++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/crates/bevy_dynamic_plugin/Cargo.toml b/crates/bevy_dynamic_plugin/Cargo.toml index 1d906d8f2f0e1..2891c71418217 100644 --- a/crates/bevy_dynamic_plugin/Cargo.toml +++ b/crates/bevy_dynamic_plugin/Cargo.toml @@ -16,3 +16,4 @@ bevy_app = { path = "../bevy_app", version = "0.9.0-dev" } # other libloading = { version = "0.7" } +thiserror = "1.0" diff --git a/crates/bevy_dynamic_plugin/src/loader.rs b/crates/bevy_dynamic_plugin/src/loader.rs index 6cc33e1bd996b..046bc1165be01 100644 --- a/crates/bevy_dynamic_plugin/src/loader.rs +++ b/crates/bevy_dynamic_plugin/src/loader.rs @@ -1,7 +1,17 @@ use libloading::{Library, Symbol}; +use thiserror::Error; use bevy_app::{App, CreatePlugin, Plugin}; +/// Errors that can occur when loading a dynamic plugin +#[derive(Debug, Error)] +pub enum DynamicPluginLoadError { + #[error("cannot load library for dynamic plugin: {0}")] + Library(libloading::Error), + #[error("dynamic library does not contain a valid Bevy dynamic plugin")] + Plugin(libloading::Error), +} + /// Dynamically links a plugin at the given path. The plugin must export a function with the /// [`CreatePlugin`] signature named `_bevy_create_plugin`. /// @@ -10,11 +20,15 @@ use bevy_app::{App, CreatePlugin, Plugin}; /// The specified plugin must be linked against the exact same libbevy.so as this program. /// In addition the `_bevy_create_plugin` symbol must not be manually created, but instead created /// by deriving `DynamicPlugin` on a unit struct implementing [`Plugin`]. -pub unsafe fn dynamically_load_plugin(path: &str) -> (Library, Box) { - let lib = Library::new(path).unwrap(); - let func: Symbol = lib.get(b"_bevy_create_plugin").unwrap(); +pub unsafe fn dynamically_load_plugin( + path: &str, +) -> Result<(Library, Box), DynamicPluginLoadError> { + let lib = Library::new(path).map_err(DynamicPluginLoadError::Library)?; + let func: Symbol = lib + .get(b"_bevy_create_plugin") + .map_err(DynamicPluginLoadError::Plugin)?; let plugin = Box::from_raw(func()); - (lib, plugin) + Ok((lib, plugin)) } pub trait DynamicPluginExt { @@ -26,7 +40,7 @@ pub trait DynamicPluginExt { impl DynamicPluginExt for App { unsafe fn load_plugin(&mut self, path: &str) -> &mut Self { - let (lib, plugin) = dynamically_load_plugin(path); + let (lib, plugin) = dynamically_load_plugin(path).unwrap(); std::mem::forget(lib); // Ensure that the library is not automatically unloaded plugin.build(self); self