Skip to content

Commit

Permalink
fix: add missing extensions in the asset loader (#254)
Browse files Browse the repository at this point in the history
* fix: fix missing extensions in the asset loader

* Actually call `finish` on script plugins and add appropriate docs
  • Loading branch information
makspll authored Feb 8, 2025
1 parent b863175 commit 21c3158
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 6 deletions.
1 change: 1 addition & 0 deletions crates/bevy_mod_scripting_core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ derivative = "2.2"
profiling = { workspace = true }
[dev-dependencies]
test_utils = { workspace = true }
tokio = { version = "1", features = ["rt", "macros"] }

[lints]
workspace = true
8 changes: 8 additions & 0 deletions crates/bevy_mod_scripting_core/src/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ pub struct ScriptAssetSettings {
pub script_id_mapper: AssetPathToScriptIdMapper,
/// Strategies for mapping asset paths to languages
pub script_language_mappers: Vec<AssetPathToLanguageMapper>,

/// The currently supported asset extensions
/// Should be updated by each scripting plugin to include the extensions it supports.
///
/// Will be used to populate the script asset loader with the supported extensions
pub supported_extensions: &'static [&'static str],
}

impl ScriptAssetSettings {
Expand All @@ -136,6 +142,7 @@ impl Default for ScriptAssetSettings {
map: (|path: &AssetPath| path.path().to_string_lossy().into_owned().into()),
},
script_language_mappers: vec![],
supported_extensions: &[],
}
}
}
Expand Down Expand Up @@ -374,6 +381,7 @@ mod tests {

fn make_test_settings() -> ScriptAssetSettings {
ScriptAssetSettings {
supported_extensions: &[],
script_id_mapper: AssetPathToScriptIdMapper {
map: |path| path.path().to_string_lossy().into_owned().into(),
},
Expand Down
107 changes: 101 additions & 6 deletions crates/bevy_mod_scripting_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ pub enum ScriptingSystemSet {

/// Types which act like scripting plugins, by selecting a context and runtime
/// Each individual combination of context and runtime has specific infrastructure built for it and does not interact with other scripting plugins
///
/// When implementing a new scripting plugin, also ensure the following implementations exist:
/// - [`Plugin`] for the plugin, both [`Plugin::build`] and [`Plugin::finish`] methods need to be dispatched to the underlying [`ScriptingPlugin`] struct
/// - [`AsMut<ScriptingPlugin<Self>`] for the plugin struct
pub trait IntoScriptPluginParams: 'static {
/// The language of the scripts
const LANGUAGE: Language;
Expand Down Expand Up @@ -85,6 +89,9 @@ pub struct ScriptingPlugin<P: IntoScriptPluginParams> {
pub context_initializers: Vec<ContextInitializer<P>>,
/// initializers for the contexts run every time before handling events
pub context_pre_handling_initializers: Vec<ContextPreHandlingInitializer<P>>,

/// Supported extensions to be added to the asset settings without the dot
pub supported_extensions: &'static [&'static str],
}

impl<P: IntoScriptPluginParams> Plugin for ScriptingPlugin<P> {
Expand All @@ -107,6 +114,8 @@ impl<P: IntoScriptPluginParams> Plugin for ScriptingPlugin<P> {
register_script_plugin_systems::<P>(app);
once_per_app_init(app);

app.add_supported_script_extensions(self.supported_extensions);

app.world_mut()
.resource_mut::<ScriptAssetSettings>()
.as_mut()
Expand All @@ -115,6 +124,10 @@ impl<P: IntoScriptPluginParams> Plugin for ScriptingPlugin<P> {

register_types(app);
}

fn finish(&self, app: &mut App) {
once_per_app_finalize(app);
}
}

impl<P: IntoScriptPluginParams> ScriptingPlugin<P> {
Expand Down Expand Up @@ -197,6 +210,33 @@ impl<P: IntoScriptPluginParams + AsMut<ScriptingPlugin<P>>> ConfigureScriptPlugi
}
}

fn once_per_app_finalize(app: &mut App) {
#[derive(Resource)]
struct BMSFinalized;

if app.world().contains_resource::<BMSFinalized>() {
return;
}
app.insert_resource(BMSFinalized);

// read extensions from asset settings
let asset_settings_extensions = app
.world_mut()
.get_resource_or_init::<ScriptAssetSettings>()
.supported_extensions;

// convert extensions to static array
bevy::log::info!(
"Initializing BMS with Supported extensions: {:?}",
asset_settings_extensions
);

app.register_asset_loader(ScriptAssetLoader {
extensions: asset_settings_extensions,
preprocessor: None,
});
}

// One of registration of things that need to be done only once per app
fn once_per_app_init(app: &mut App) {
#[derive(Resource)]
Expand All @@ -205,19 +245,14 @@ fn once_per_app_init(app: &mut App) {
if app.world().contains_resource::<BMSInitialized>() {
return;
}

app.insert_resource(BMSInitialized);

app.add_event::<ScriptErrorEvent>()
.add_event::<ScriptCallbackEvent>()
.init_resource::<AppReflectAllocator>()
.init_resource::<Scripts>()
.init_asset::<ScriptAsset>()
.init_resource::<AppScriptFunctionRegistry>()
.register_asset_loader(ScriptAssetLoader {
extensions: &[],
preprocessor: None,
});
.init_resource::<AppScriptFunctionRegistry>();

app.add_systems(
PostUpdate,
Expand Down Expand Up @@ -274,3 +309,63 @@ impl AddRuntimeInitializer for App {
self
}
}

/// Trait for adding a supported extension to the script asset settings.
///
/// This is only valid in the plugin building phase, as the asset loader will be created in the `finalize` phase.
/// Any changes to the asset settings after that will not be reflected in the asset loader.
pub trait ConfigureScriptAssetSettings {
/// Adds a supported extension to the asset settings
fn add_supported_script_extensions(&mut self, extensions: &[&'static str]) -> &mut Self;
}

impl ConfigureScriptAssetSettings for App {
fn add_supported_script_extensions(&mut self, extensions: &[&'static str]) -> &mut Self {
let mut asset_settings = self
.world_mut()
.get_resource_or_init::<ScriptAssetSettings>();

let mut new_arr = Vec::from(asset_settings.supported_extensions);

new_arr.extend(extensions);

let new_arr_static = Vec::leak(new_arr);

asset_settings.supported_extensions = new_arr_static;

self
}
}

#[cfg(test)]
mod test {
use super::*;

#[tokio::test]
async fn test_asset_extensions_correctly_accumulate() {
let mut app = App::new();
app.init_resource::<ScriptAssetSettings>();
app.add_plugins(AssetPlugin::default());

app.world_mut()
.resource_mut::<ScriptAssetSettings>()
.supported_extensions = &["lua", "rhai"];

once_per_app_finalize(&mut app);

let asset_loader = app
.world()
.get_resource::<AssetServer>()
.expect("Asset loader not found");

asset_loader
.get_asset_loader_with_extension("lua")
.await
.expect("Lua loader not found");

asset_loader
.get_asset_loader_with_extension("rhai")
.await
.expect("Rhai loader not found");
}
}
5 changes: 5 additions & 0 deletions crates/languages/bevy_mod_scripting_lua/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ impl Default for LuaScriptingPlugin {
.map_err(ScriptError::from_mlua_error)?;
Ok(())
}],
supported_extensions: &["lua"],
},
}
}
Expand All @@ -150,6 +151,10 @@ impl Plugin for LuaScriptingPlugin {
fn build(&self, app: &mut bevy::prelude::App) {
self.scripting_plugin.build(app);
}

fn finish(&self, app: &mut bevy::app::App) {
self.scripting_plugin.finish(app);
}
}
#[profiling::function]
/// Load a lua context from a script
Expand Down
5 changes: 5 additions & 0 deletions crates/languages/bevy_mod_scripting_rhai/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ impl Default for RhaiScriptingPlugin {
context.scope.set_or_push("script_id", script.to_owned());
Ok(())
}],
supported_extensions: &["rhai"],
},
}
}
Expand All @@ -171,6 +172,10 @@ impl Plugin for RhaiScriptingPlugin {
fn build(&self, app: &mut bevy::prelude::App) {
self.scripting_plugin.build(app);
}

fn finish(&self, app: &mut bevy::app::App) {
self.scripting_plugin.finish(app);
}
}

/// Load a rhai context from a script.
Expand Down

0 comments on commit 21c3158

Please sign in to comment.