From 5bb437aca2935dfbc91f7ceb85dff0066ca92935 Mon Sep 17 00:00:00 2001 From: makspll Date: Tue, 14 Jan 2025 22:23:32 +0000 Subject: [PATCH 01/34] feat: tighten testing, and frame hygiene --- .../assets/test_assets/test_script.lua | 1 + .../assets/test_assets/test_script.script | 1 + crates/bevy_mod_scripting_core/src/asset.rs | 459 ++++++++++++++- .../src/bindings/allocator.rs | 8 +- .../bevy_mod_scripting_core/src/commands.rs | 21 +- crates/bevy_mod_scripting_core/src/context.rs | 76 ++- crates/bevy_mod_scripting_core/src/handler.rs | 421 ++++++++++++- crates/bevy_mod_scripting_core/src/lib.rs | 180 +----- crates/bevy_mod_scripting_core/src/runtime.rs | 14 +- crates/bevy_mod_scripting_core/src/systems.rs | 551 ------------------ crates/xtask/src/main.rs | 121 ++-- 11 files changed, 1053 insertions(+), 800 deletions(-) create mode 100644 crates/bevy_mod_scripting_core/assets/test_assets/test_script.lua create mode 100644 crates/bevy_mod_scripting_core/assets/test_assets/test_script.script delete mode 100644 crates/bevy_mod_scripting_core/src/systems.rs diff --git a/crates/bevy_mod_scripting_core/assets/test_assets/test_script.lua b/crates/bevy_mod_scripting_core/assets/test_assets/test_script.lua new file mode 100644 index 0000000000..95d09f2b10 --- /dev/null +++ b/crates/bevy_mod_scripting_core/assets/test_assets/test_script.lua @@ -0,0 +1 @@ +hello world \ No newline at end of file diff --git a/crates/bevy_mod_scripting_core/assets/test_assets/test_script.script b/crates/bevy_mod_scripting_core/assets/test_assets/test_script.script new file mode 100644 index 0000000000..784bee3bb8 --- /dev/null +++ b/crates/bevy_mod_scripting_core/assets/test_assets/test_script.script @@ -0,0 +1 @@ +test script \ No newline at end of file diff --git a/crates/bevy_mod_scripting_core/src/asset.rs b/crates/bevy_mod_scripting_core/src/asset.rs index 92a5a0dbf9..179c404bb9 100644 --- a/crates/bevy_mod_scripting_core/src/asset.rs +++ b/crates/bevy_mod_scripting_core/src/asset.rs @@ -1,7 +1,15 @@ -use crate::{error::ScriptError, script::ScriptId}; +use crate::{ + commands::{CreateOrUpdateScript, DeleteScript}, + error::ScriptError, + script::ScriptId, + IntoScriptPluginParams, ScriptingSystemSet, +}; use bevy::{ - asset::{Asset, AssetId, AssetLoader}, + app::{App, PreUpdate}, + asset::{Asset, AssetEvent, AssetId, AssetLoader, Assets}, ecs::system::Resource, + log::{error, info, trace}, + prelude::{Commands, EventReader, IntoSystemConfigs, IntoSystemSetConfigs, Res, ResMut}, reflect::TypePath, utils::HashMap, }; @@ -130,7 +138,7 @@ pub struct ScriptMetadataStore { pub map: HashMap, ScriptMetadata>, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct ScriptMetadata { pub script_id: ScriptId, pub language: Language, @@ -150,3 +158,448 @@ impl ScriptMetadataStore { self.map.remove(&id) } } + +/// Listens to `AssetEvent::Added` events and populates the script metadata store +pub fn insert_script_metadata( + mut events: EventReader>, + script_assets: Res>, + mut asset_path_map: ResMut, + settings: Res, +) { + for event in events.read() { + if let AssetEvent::LoadedWithDependencies { id } = event { + let asset = script_assets.get(*id); + if let Some(asset) = asset { + let path = &asset.asset_path; + let converter = settings.script_id_mapper.map; + let script_id = converter(path); + + let language = settings.select_script_language(path); + let metadata = ScriptMetadata { + script_id, + language, + }; + info!("Populating script metadata for script: {:?}:", metadata); + asset_path_map.insert(*id, metadata); + } else { + error!("A script was added but it's asset was not found, failed to compute metadata. This script will not be loaded. {}", id); + } + } + } +} + +/// Listens to [`AssetEvent::Removed`] events and removes the corresponding script metadata +pub fn remove_script_metadata( + mut events: EventReader>, + mut asset_path_map: ResMut, +) { + for event in events.read() { + if let AssetEvent::Removed { id } = event { + let previous = asset_path_map.remove(*id); + if let Some(previous) = previous { + info!("Removed script metadata for removed script: {:?}", previous); + } + } + } +} + +/// Listens to [`AssetEvent`] events and dispatches [`CreateOrUpdateScript`] and [`DeleteScript`] commands accordingly. +/// +/// Allows for hot-reloading of scripts. +pub fn sync_script_data( + mut events: EventReader>, + script_assets: Res>, + script_metadata: Res, + mut commands: Commands, +) { + for event in events.read() { + trace!("{}: Received script asset event: {:?}", P::LANGUAGE, event); + match event { + // emitted when a new script asset is loaded for the first time + AssetEvent::LoadedWithDependencies { id } | AssetEvent::Modified { id } => { + let metadata = match script_metadata.get(*id) { + Some(m) => m, + None => { + error!( + "{}: Script metadata not found for script asset with id: {}. Cannot load script.", + P::LANGUAGE, + id + ); + continue; + } + }; + + if metadata.language != P::LANGUAGE { + trace!( + "{}: Script asset with id: {} is for a different langauge than this sync system. Skipping.", + P::LANGUAGE, + metadata.script_id + ); + continue; + } + + info!( + "{}: Dispatching Creation/Modification command for script: {:?}. Asset Id: {}", + P::LANGUAGE, + metadata, + id + ); + + if let Some(asset) = script_assets.get(*id) { + commands.queue(CreateOrUpdateScript::

::new( + metadata.script_id.clone(), + asset.content.clone(), + Some(script_assets.reserve_handle().clone_weak()), + )); + } + } + AssetEvent::Removed { id } => { + let metadata = match script_metadata.get(*id) { + Some(m) => m, + None => { + error!( + "{}: Script metadata not found for script asset with id: {}. Cannot delete script.", + P::LANGUAGE, + id + ); + return; + } + }; + + info!( + "{}: Dispatching Deletion command for script: {:?}. Asset Id: {}", + P::LANGUAGE, + metadata, + id + ); + commands.queue(DeleteScript::

::new(metadata.script_id.clone())); + } + _ => return, + }; + } +} + +/// Setup all the asset systems for the scripting plugin and the dependencies +pub(crate) fn configure_asset_systems(app: &mut App) -> &mut App { + // these should be in the same set as bevy's asset systems + // currently this is in the PreUpdate set + app.add_systems( + PreUpdate, + ( + insert_script_metadata.in_set(ScriptingSystemSet::ScriptMetadataInsertion), + remove_script_metadata.in_set(ScriptingSystemSet::ScriptMetadataRemoval), + ), + ) + .configure_sets( + PreUpdate, + ( + ScriptingSystemSet::ScriptMetadataInsertion.after(bevy::asset::TrackAssets), + ScriptingSystemSet::ScriptCommandDispatch + .after(ScriptingSystemSet::ScriptMetadataInsertion) + .before(ScriptingSystemSet::ScriptMetadataRemoval), + ), + ) + .init_resource::() + .init_resource::(); + + app +} + +/// Setup all the asset systems for the scripting plugin and the dependencies +pub(crate) fn configure_asset_systems_for_plugin( + app: &mut App, +) -> &mut App { + app.add_systems( + PreUpdate, + sync_script_data::

.in_set(ScriptingSystemSet::ScriptCommandDispatch), + ); + app +} + +#[cfg(test)] +mod tests { + use bevy::{ + app::{App, Update}, + asset::{AssetApp, AssetPlugin, AssetServer, Assets, Handle, LoadState}, + MinimalPlugins, + }; + + use super::*; + + fn init_loader_test(loader: ScriptAssetLoader) -> App { + let mut app = App::new(); + app.add_plugins((MinimalPlugins, AssetPlugin::default())); + app.init_asset::(); + app.register_asset_loader(loader); + app + } + + fn make_test_settings() -> ScriptAssetSettings { + ScriptAssetSettings { + script_id_mapper: AssetPathToScriptIdMapper { + map: |path| path.to_string_lossy().into_owned().into(), + }, + script_language_mappers: vec![ + AssetPathToLanguageMapper { + map: |path| { + if path.extension().unwrap() == "lua" { + Language::Lua + } else { + Language::Unknown + } + }, + }, + AssetPathToLanguageMapper { + map: |path| { + if path.extension().unwrap() == "rhai" { + Language::Rhai + } else { + Language::Unknown + } + }, + }, + ], + } + } + + fn load_asset(app: &mut App, path: &str) -> Handle { + let handle = app.world_mut().resource::().load(path); + + loop { + let state = app + .world() + .resource::() + .get_load_state(&handle) + .unwrap(); + if !matches!(state, LoadState::Loading) { + break; + } + app.update(); + } + + match app + .world() + .resource::() + .get_load_state(&handle) + .unwrap() + { + LoadState::NotLoaded => panic!("Asset not loaded"), + LoadState::Loaded => {} + LoadState::Failed(asset_load_error) => { + panic!("Asset load failed: {:?}", asset_load_error) + } + _ => panic!("Unexpected load state"), + } + + handle + } + + #[test] + fn test_asset_loader_loads() { + let loader = ScriptAssetLoader { + extensions: &["script"], + preprocessor: None, + }; + let mut app = init_loader_test(loader); + + let handle = load_asset(&mut app, "test_assets/test_script.script"); + let asset = app + .world() + .get_resource::>() + .unwrap() + .get(&handle) + .unwrap(); + + assert_eq!( + asset.asset_path, + PathBuf::from("test_assets/test_script.script") + ); + + assert_eq!( + String::from_utf8(asset.content.clone().to_vec()).unwrap(), + "test script".to_string() + ); + } + + #[test] + fn test_asset_loader_applies_preprocessor() { + let loader = ScriptAssetLoader { + extensions: &["script"], + preprocessor: Some(Box::new(|content| { + content[0] = b'p'; + Ok(()) + })), + }; + let mut app = init_loader_test(loader); + + let handle = load_asset(&mut app, "test_assets/test_script.script"); + let asset = app + .world() + .get_resource::>() + .unwrap() + .get(&handle) + .unwrap(); + + assert_eq!( + asset.asset_path, + PathBuf::from("test_assets/test_script.script") + ); + assert_eq!( + String::from_utf8(asset.content.clone().to_vec()).unwrap(), + "pest script".to_string() + ); + } + + #[test] + fn test_metadata_store() { + let mut store = ScriptMetadataStore::default(); + let id = AssetId::invalid(); + let meta = ScriptMetadata { + script_id: "test".into(), + language: Language::Lua, + }; + + store.insert(id, meta.clone()); + assert_eq!(store.get(id), Some(&meta)); + + assert_eq!(store.remove(id), Some(meta)); + } + + #[test] + fn test_script_asset_settings_select_language() { + let settings = make_test_settings(); + + let path = Path::new("test.lua"); + assert_eq!(settings.select_script_language(path), Language::Lua); + assert_eq!( + settings.select_script_language(Path::new("test.rhai")), + Language::Rhai + ); + assert_eq!( + settings.select_script_language(Path::new("test.blob")), + Language::Unknown + ); + } + + fn run_app_untill_asset_event(app: &mut App, event_kind: AssetEvent) { + let checker_system = |mut reader: EventReader>, + mut event_target: ResMut| { + println!("Reading asset events this frame"); + for event in reader.read() { + println!("{:?}", event); + if matches!( + (event_target.event, event), + (AssetEvent::Added { .. }, AssetEvent::Added { .. }) + | (AssetEvent::Modified { .. }, AssetEvent::Modified { .. }) + | (AssetEvent::Removed { .. }, AssetEvent::Removed { .. }) + | (AssetEvent::Unused { .. }, AssetEvent::Unused { .. }) + | ( + AssetEvent::LoadedWithDependencies { .. }, + AssetEvent::LoadedWithDependencies { .. }, + ) + ) { + println!("Event matched"); + event_target.happened = true; + } + } + }; + + if !app.world().contains_resource::() { + // for when we run this multiple times in a test + app.add_systems(Update, checker_system); + } + + #[derive(Resource)] + struct EventTarget { + event: AssetEvent, + happened: bool, + } + app.world_mut().insert_resource(EventTarget { + event: event_kind, + happened: false, + }); + + loop { + println!("Checking if asset event was dispatched"); + if app.world().get_resource::().unwrap().happened { + println!("Stopping loop"); + break; + } + println!("Running app"); + + app.update(); + } + } + + struct DummyPlugin; + + impl IntoScriptPluginParams for DummyPlugin { + type R = (); + type C = (); + const LANGUAGE: Language = Language::Lua; + + fn build_runtime() -> Self::R { + todo!() + } + } + + #[test] + fn test_asset_metadata_insert_remove_systems() { + // test metadata flow + let mut app = init_loader_test(ScriptAssetLoader { + extensions: &[], + preprocessor: None, + }); + app.world_mut().insert_resource(make_test_settings()); + configure_asset_systems(&mut app); + + // update untill the asset event gets dispatched + let asset_server: &AssetServer = app.world().resource::(); + let handle = asset_server.load("test_assets/test_script.lua"); + run_app_untill_asset_event( + &mut app, + AssetEvent::LoadedWithDependencies { + id: AssetId::invalid(), + }, + ); + let asset_id = handle.id(); + + // we expect the metadata to be inserted now, in the same frame as the asset is loaded + let metadata = app + .world() + .get_resource::() + .unwrap() + .get(asset_id) + .expect("Metadata not found"); + + assert_eq!(metadata.script_id, "test_assets/test_script.lua"); + assert_eq!(metadata.language, Language::Lua); + + // ----------------- REMOVING ----------------- + + // we drop the handle and wait untill the first asset event is dispatched + drop(handle); + + run_app_untill_asset_event( + &mut app, + AssetEvent::Removed { + id: AssetId::invalid(), + }, + ); + + // we expect the metadata to be removed now, in the same frame as the asset is removed + let metadata_len = app + .world() + .get_resource::() + .unwrap() + .map + .len(); + + assert_eq!(metadata_len, 0); + } + + #[test] + fn test_syncing_assets() { + todo!() + } +} diff --git a/crates/bevy_mod_scripting_core/src/bindings/allocator.rs b/crates/bevy_mod_scripting_core/src/bindings/allocator.rs index 52b56edda1..d31fcc9090 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/allocator.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/allocator.rs @@ -1,4 +1,4 @@ -use bevy::{ecs::system::Resource, reflect::PartialReflect}; +use bevy::{ecs::system::Resource, prelude::ResMut, reflect::PartialReflect}; use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use std::{ any::TypeId, @@ -205,6 +205,12 @@ impl ReflectAllocator { } } +/// Cleans up dangling script allocations +pub fn garbage_collector(allocator: ResMut) { + let mut allocator = allocator.write(); + allocator.clean_garbage_allocations() +} + #[cfg(test)] mod test { use super::*; diff --git a/crates/bevy_mod_scripting_core/src/commands.rs b/crates/bevy_mod_scripting_core/src/commands.rs index ac548f78ab..713034a748 100644 --- a/crates/bevy_mod_scripting_core/src/commands.rs +++ b/crates/bevy_mod_scripting_core/src/commands.rs @@ -2,10 +2,9 @@ use crate::{ asset::ScriptAsset, context::{ContextLoadingSettings, ScriptContexts}, event::{IntoCallbackLabel, OnScriptLoaded, OnScriptUnloaded}, - handler::CallbackSettings, + handler::{handle_script_errors, CallbackSettings}, runtime::RuntimeContainer, script::{Script, ScriptId, Scripts}, - systems::handle_script_errors, IntoScriptPluginParams, }; use bevy::{asset::Handle, ecs::world::Mut, log::debug, prelude::Command}; @@ -76,12 +75,8 @@ impl Command for DeleteScript

{ } } - let assigner = settings - .assigner - .as_ref() - .expect("Could not find context assigner in settings"); debug!("Removing script with id: {}", self.id); - (assigner.remove)(script.context_id, &script, &mut ctxts) + (settings.assigner.remove)(script.context_id, &script, &mut ctxts) } else { bevy::log::error!( "Attempted to delete script with id: {} but it does not exist, doing nothing!", @@ -122,19 +117,21 @@ impl Command for CreateOrUpdateScript

{ fn apply(self, world: &mut bevy::prelude::World) { let settings = world .get_resource::>() - .unwrap() + .expect( + "Missing ContextLoadingSettings resource. Was the plugin initialized correctly?", + ) .clone(); let mut contexts = world .remove_non_send_resource::>() - .unwrap(); + .expect("No ScriptContexts resource found. Was the plugin initialized correctly?"); let mut runtime = world .remove_non_send_resource::>() - .unwrap(); + .expect("No RuntimeContainer resource found. Was the plugin initialized correctly?"); let runner = world.get_resource::>().unwrap(); // assign context - let assigner = settings.assigner.clone().expect("No context assigner set"); - let builder = settings.loader.clone().expect("No context loader set"); + let assigner = settings.assigner.clone(); + let builder = settings.loader.clone(); let runner = runner.callback_handler.expect("No callback handler set"); world.resource_scope(|world, mut scripts: Mut| { diff --git a/crates/bevy_mod_scripting_core/src/context.rs b/crates/bevy_mod_scripting_core/src/context.rs index 04318645e9..664a2fcc86 100644 --- a/crates/bevy_mod_scripting_core/src/context.rs +++ b/crates/bevy_mod_scripting_core/src/context.rs @@ -11,6 +11,7 @@ impl Context for T {} pub type ContextId = u32; +/// Stores script state for a scripting plugin. Scripts are identified by their `ScriptId`, while contexts are identified by their `ContextId`. #[derive(Resource)] pub struct ScriptContexts { pub contexts: HashMap, @@ -26,12 +27,6 @@ impl Default for ScriptContexts

{ static CONTEXT_ID_COUNTER: AtomicU32 = AtomicU32::new(0); impl ScriptContexts

{ - pub fn new() -> Self { - Self { - contexts: HashMap::new(), - } - } - /// Allocates a new ContextId and inserts the context into the map pub fn insert(&mut self, ctxt: P::C) -> ContextId { let id = CONTEXT_ID_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed); @@ -64,30 +59,20 @@ pub type ContextInitializer

= pub type ContextPreHandlingInitializer

= fn(&str, Entity, &mut

::C) -> Result<(), ScriptError>; +/// Settings concerning the creation and assignment of script contexts as well as their initialization. #[derive(Resource)] pub struct ContextLoadingSettings { /// Defines the strategy used to load and reload contexts - pub loader: Option>, + pub loader: ContextBuilder

, /// Defines the strategy used to assign contexts to scripts - pub assigner: Option>, + pub assigner: ContextAssigner

, /// Initializers run once after creating a context but before executing it for the first time pub context_initializers: Vec>, /// Initializers run every time before executing or loading a script pub context_pre_handling_initializers: Vec>, } -impl Default for ContextLoadingSettings

{ - fn default() -> Self { - Self { - loader: None, - assigner: None, - context_initializers: Default::default(), - context_pre_handling_initializers: Default::default(), - } - } -} - -impl Clone for ContextLoadingSettings

{ +impl Clone for ContextLoadingSettings { fn clone(&self) -> Self { Self { loader: self.loader.clone(), @@ -162,3 +147,54 @@ impl Clone for ContextAssigner

{ } } } + +#[cfg(test)] +mod tests { + use crate::asset::Language; + + use super::*; + + struct DummyParams; + impl IntoScriptPluginParams for DummyParams { + type C = String; + type R = (); + + const LANGUAGE: Language = Language::Lua; + + fn build_runtime() -> Self::R { + todo!() + } + } + + #[test] + fn test_script_contexts_insert_get() { + let mut contexts: ScriptContexts = ScriptContexts::default(); + let id = contexts.insert("context1".to_string()); + assert_eq!(contexts.contexts.get(&id), Some(&"context1".to_string())); + assert_eq!( + contexts.contexts.get_mut(&id), + Some(&mut "context1".to_string()) + ); + } + + #[test] + fn test_script_contexts_allocate_id() { + let contexts: ScriptContexts = ScriptContexts::default(); + let id = contexts.allocate_id(); + let next_id = contexts.allocate_id(); + assert_eq!(next_id, id + 1); + } + + #[test] + fn test_script_contexts_remove() { + let mut contexts: ScriptContexts = ScriptContexts::default(); + let id = contexts.insert("context1".to_string()); + let removed = contexts.remove(id); + assert_eq!(removed, Some("context1".to_string())); + assert!(!contexts.contexts.contains_key(&id)); + + // assert next id is still incremented + let next_id = contexts.allocate_id(); + assert_eq!(next_id, id + 1); + } +} diff --git a/crates/bevy_mod_scripting_core/src/handler.rs b/crates/bevy_mod_scripting_core/src/handler.rs index dff84c585a..61c60d4410 100644 --- a/crates/bevy_mod_scripting_core/src/handler.rs +++ b/crates/bevy_mod_scripting_core/src/handler.rs @@ -1,8 +1,25 @@ +use std::any::type_name; + use crate::{ - bindings::script_value::ScriptValue, context::ContextPreHandlingInitializer, - error::ScriptError, event::CallbackLabel, script::ScriptId, IntoScriptPluginParams, + bindings::{ + pretty_print::DisplayWithWorld, script_value::ScriptValue, WorldAccessGuard, WorldGuard, + }, + context::{ContextLoadingSettings, ContextPreHandlingInitializer, ScriptContexts}, + error::ScriptError, + event::{CallbackLabel, IntoCallbackLabel, ScriptCallbackEvent, ScriptErrorEvent}, + runtime::RuntimeContainer, + script::{ScriptComponent, ScriptId, Scripts}, + IntoScriptPluginParams, +}; +use bevy::{ + ecs::{ + entity::Entity, + system::{Resource, SystemState}, + world::World, + }, + log::{debug, trace}, + prelude::{EventReader, Events, Query, Ref, Res}, }; -use bevy::ecs::{entity::Entity, system::Resource, world::World}; pub trait Args: Clone + Send + Sync + 'static {} impl Args for T {} @@ -31,3 +48,401 @@ impl Default for CallbackSettings

{ } } } + +macro_rules! push_err_and_continue { + ($errors:ident, $expr:expr) => { + match $expr { + Ok(v) => v, + Err(e) => { + $errors.push(e); + continue; + } + } + }; +} + +/// Passes events with the specified label to the script callback with the same name and runs the callback +pub fn event_handler( + world: &mut World, + params: &mut SystemState<( + EventReader, + Res>, + Res>, + Res, + Query<(Entity, Ref)>, + )>, +) { + let mut runtime_container = world + .remove_non_send_resource::>() + .unwrap_or_else(|| { + panic!( + "No runtime container for runtime {} found. Was the scripting plugin initialized correctly?", + type_name::() + ) + }); + let runtime = &mut runtime_container.runtime; + let mut script_contexts = world + .remove_non_send_resource::>() + .unwrap_or_else(|| panic!("No script contexts found for context {}", type_name::

())); + + let (mut script_events, callback_settings, context_settings, scripts, entities) = + params.get_mut(world); + + let handler = *callback_settings + .callback_handler + .as_ref() + .unwrap_or_else(|| { + panic!( + "No handler registered for - Runtime: {}, Context: {}", + type_name::(), + type_name::() + ) + }); + let pre_handling_initializers = context_settings.context_pre_handling_initializers.clone(); + let scripts = scripts.clone(); + let mut errors = Vec::default(); + + let events = script_events.read().cloned().collect::>(); + let entity_scripts = entities + .iter() + .map(|(e, s)| (e, s.0.clone())) + .collect::>(); + + for event in events + .into_iter() + .filter(|e| e.label == L::into_callback_label()) + { + for (entity, entity_scripts) in entity_scripts.iter() { + for script_id in entity_scripts.iter() { + match &event.recipients { + crate::event::Recipients::Script(target_script_id) + if target_script_id != script_id => + { + continue + } + crate::event::Recipients::Entity(target_entity) if target_entity != entity => { + continue + } + _ => (), + } + debug!( + "Handling event for script {} on entity {:?}", + script_id, entity + ); + let script = match scripts.scripts.get(script_id) { + Some(s) => s, + None => { + trace!( + "Script `{}` on entity `{:?}` is either still loading or doesn't exist, ignoring.", + script_id, entity + ); + continue; + } + }; + + let ctxt = match script_contexts.contexts.get_mut(&script.context_id) { + Some(ctxt) => ctxt, + None => { + // if we don't have a context for the script, it's either: + // 1. a script for a different language, in which case we ignore it + // 2. something went wrong. This should not happen though and it's best we ignore this + continue; + } + }; + + let handler_result = (handler)( + event.args.clone(), + *entity, + &script.id, + &L::into_callback_label(), + ctxt, + &pre_handling_initializers, + runtime, + world, + ) + .map_err(|e| { + e.with_script(script.id.clone()) + .with_context(format!("Event handling for: Language: {}", P::LANGUAGE)) + }); + + let _ = push_err_and_continue!(errors, handler_result); + } + } + } + + world.insert_non_send_resource(runtime_container); + world.insert_non_send_resource(script_contexts); + + handle_script_errors(world, errors.into_iter()); +} + +/// Handles errors caused by script execution and sends them to the error event channel +pub(crate) fn handle_script_errors + Clone>( + world: &mut World, + errors: I, +) { + let mut error_events = world + .get_resource_mut::>() + .expect("Missing events resource"); + + for error in errors.clone() { + error_events.send(ScriptErrorEvent { error }); + } + + for error in errors { + let arc_world = WorldGuard::new(WorldAccessGuard::new(world)); + bevy::log::error!("{}", error.display_with_world(arc_world)); + } +} + +#[cfg(test)] +mod test { + use std::{borrow::Cow, collections::HashMap}; + + use bevy::app::{App, Update}; + + use crate::{ + bindings::script_value::ScriptValue, + context::{ContextAssigner, ContextBuilder, ContextLoadingSettings, ScriptContexts}, + event::{CallbackLabel, IntoCallbackLabel, ScriptCallbackEvent, ScriptErrorEvent}, + handler::HandlerFn, + runtime::RuntimeContainer, + script::{Script, ScriptComponent, ScriptId, Scripts}, + }; + + use super::*; + struct OnTestCallback; + + impl IntoCallbackLabel for OnTestCallback { + fn into_callback_label() -> CallbackLabel { + "OnTest".into() + } + } + + struct TestPlugin; + + impl IntoScriptPluginParams for TestPlugin { + type C = TestContext; + type R = TestRuntime; + + const LANGUAGE: crate::asset::Language = crate::asset::Language::Unknown; + + fn build_runtime() -> Self::R { + TestRuntime { + invocations: vec![], + } + } + } + + struct TestRuntime { + pub invocations: Vec<(Entity, ScriptId)>, + } + + struct TestContext { + pub invocations: Vec, + } + + fn setup_app( + handler_fn: HandlerFn

, + runtime: P::R, + contexts: HashMap, + scripts: HashMap, + ) -> App { + let mut app = App::new(); + + app.add_event::(); + app.add_event::(); + app.insert_resource::>(CallbackSettings { + callback_handler: Some(handler_fn), + }); + app.add_systems(Update, event_handler::); + app.insert_resource::(Scripts { scripts }); + app.insert_non_send_resource(RuntimeContainer::

{ runtime }); + app.insert_non_send_resource(ScriptContexts::

{ contexts }); + app.insert_resource(ContextLoadingSettings::

{ + loader: ContextBuilder { + load: |_, _, _, _, _, _| todo!(), + reload: |_, _, _, _, _, _, _| todo!(), + }, + assigner: ContextAssigner { + assign: |_, _, _, _| todo!(), + remove: |_, _, _| todo!(), + }, + context_initializers: vec![], + context_pre_handling_initializers: vec![], + }); + app.finish(); + app.cleanup(); + app + } + + #[test] + fn test_handler_called_with_right_args() { + let test_script_id = Cow::Borrowed("test_script"); + let test_ctxt_id = 0; + let test_script = Script { + id: test_script_id.clone(), + asset: None, + context_id: test_ctxt_id, + }; + let scripts = HashMap::from_iter(vec![(test_script_id.clone(), test_script.clone())]); + let contexts = HashMap::from_iter(vec![( + test_ctxt_id, + TestContext { + invocations: vec![], + }, + )]); + let runtime = TestRuntime { + invocations: vec![], + }; + let mut app = setup_app::( + |args, entity, script, _, ctxt, _, runtime, _| { + ctxt.invocations.extend(args); + runtime.invocations.push((entity, script.clone())); + Ok(ScriptValue::Unit) + }, + runtime, + contexts, + scripts, + ); + let test_entity_id = app + .world_mut() + .spawn(ScriptComponent(vec![test_script_id.clone()])) + .id(); + + app.world_mut().send_event(ScriptCallbackEvent::new_for_all( + OnTestCallback::into_callback_label(), + vec![ScriptValue::String("test_args".into())], + )); + app.update(); + + let test_context = app + .world() + .get_non_send_resource::>() + .unwrap(); + let test_runtime = app + .world() + .get_non_send_resource::>() + .unwrap(); + + assert_eq!( + test_context + .contexts + .get(&test_ctxt_id) + .unwrap() + .invocations, + vec![ScriptValue::String("test_args".into())] + ); + + assert_eq!( + test_runtime + .runtime + .invocations + .iter() + .map(|(e, s)| (*e, s.clone())) + .collect::>(), + vec![(test_entity_id, test_script_id.clone())] + ); + } + + #[test] + fn test_handler_called_on_right_recipients() { + let test_script_id = Cow::Borrowed("test_script"); + let test_ctxt_id = 0; + let test_script = Script { + id: test_script_id.clone(), + asset: None, + context_id: test_ctxt_id, + }; + let scripts = HashMap::from_iter(vec![ + (test_script_id.clone(), test_script.clone()), + ( + "wrong".into(), + Script { + id: "wrong".into(), + asset: None, + context_id: 1, + }, + ), + ]); + let contexts = HashMap::from_iter(vec![ + ( + test_ctxt_id, + TestContext { + invocations: vec![], + }, + ), + ( + 1, + TestContext { + invocations: vec![], + }, + ), + ]); + let runtime = TestRuntime { + invocations: vec![], + }; + let mut app = setup_app::( + |args, entity, script, _, ctxt, _, runtime, _| { + ctxt.invocations.extend(args); + runtime.invocations.push((entity, script.clone())); + Ok(ScriptValue::Unit) + }, + runtime, + contexts, + scripts, + ); + let test_entity_id = app + .world_mut() + .spawn(ScriptComponent(vec![test_script_id.clone()])) + .id(); + + app.world_mut().send_event(ScriptCallbackEvent::new( + OnTestCallback::into_callback_label(), + vec![ScriptValue::String("test_args_script".into())], + crate::event::Recipients::Script(test_script_id.clone()), + )); + + app.world_mut().send_event(ScriptCallbackEvent::new( + OnTestCallback::into_callback_label(), + vec![ScriptValue::String("test_args_entity".into())], + crate::event::Recipients::Entity(test_entity_id), + )); + + app.update(); + + let test_context = app + .world() + .get_non_send_resource::>() + .unwrap(); + let test_runtime = app + .world() + .get_non_send_resource::>() + .unwrap(); + + assert_eq!( + test_context + .contexts + .get(&test_ctxt_id) + .unwrap() + .invocations, + vec![ + ScriptValue::String("test_args_script".into()), + ScriptValue::String("test_args_entity".into()) + ] + ); + + assert_eq!( + test_runtime + .runtime + .invocations + .iter() + .map(|(e, s)| (*e, s.clone())) + .collect::>(), + vec![ + (test_entity_id, test_script_id.clone()), + (test_entity_id, test_script_id.clone()) + ] + ); + } +} diff --git a/crates/bevy_mod_scripting_core/src/lib.rs b/crates/bevy_mod_scripting_core/src/lib.rs index a25e3b6e0a..f8db9c41c8 100644 --- a/crates/bevy_mod_scripting_core/src/lib.rs +++ b/crates/bevy_mod_scripting_core/src/lib.rs @@ -1,13 +1,13 @@ use crate::event::ScriptErrorEvent; use asset::{ - AssetPathToLanguageMapper, Language, ScriptAsset, ScriptAssetLoader, ScriptAssetSettings, - ScriptMetadataStore, + configure_asset_systems, configure_asset_systems_for_plugin, AssetPathToLanguageMapper, + Language, ScriptAsset, ScriptAssetLoader, ScriptAssetSettings, }; use bevy::prelude::*; use bindings::{ - function::script_function::AppScriptFunctionRegistry, script_value::ScriptValue, - AppReflectAllocator, ReflectAllocator, ReflectReference, ScriptTypeRegistration, - WorldCallbackAccess, + function::script_function::AppScriptFunctionRegistry, garbage_collector, + script_value::ScriptValue, AppReflectAllocator, ReflectAllocator, ReflectReference, + ScriptTypeRegistration, WorldCallbackAccess, }; use context::{ Context, ContextAssigner, ContextBuilder, ContextInitializer, ContextLoadingSettings, @@ -16,13 +16,8 @@ use context::{ use docs::{Documentation, DocumentationFragment}; use event::ScriptCallbackEvent; use handler::{CallbackSettings, HandlerFn}; - -use runtime::{Runtime, RuntimeContainer, RuntimeInitializer, RuntimeSettings}; +use runtime::{initialize_runtime, Runtime, RuntimeContainer, RuntimeInitializer, RuntimeSettings}; use script::Scripts; -use systems::{ - garbage_collector, initialize_runtime, insert_script_metadata, remove_script_metadata, - sync_script_data, ScriptingSystemSet, -}; pub mod asset; pub mod bindings; @@ -35,7 +30,20 @@ pub mod handler; pub mod reflection_extensions; pub mod runtime; pub mod script; -pub mod systems; + +#[derive(SystemSet, Hash, Debug, Eq, PartialEq, Clone)] +/// Labels for various BMS systems +pub enum ScriptingSystemSet { + // Post Setup processes + RuntimeInitialization, + + // Post Update processes + GarbageCollection, + + ScriptMetadataInsertion, + ScriptCommandDispatch, + ScriptMetadataRemoval, +} /// 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 @@ -56,9 +64,9 @@ pub struct ScriptingPlugin { /// The handler used for executing callbacks in scripts pub callback_handler: Option>, /// The context builder for loading contexts - pub context_builder: Option>, + pub context_builder: ContextBuilder

, /// The context assigner for assigning contexts to scripts, if not provided default strategy of keeping each script in its own context is used - pub context_assigner: Option>, + pub context_assigner: ContextAssigner

, pub language_mapper: Option, /// initializers for the contexts, run when loading the script @@ -67,23 +75,6 @@ pub struct ScriptingPlugin { pub context_pre_handling_initializers: Vec>, } -impl Default for ScriptingPlugin

-where - P::R: Default, -{ - fn default() -> Self { - Self { - runtime_settings: Default::default(), - callback_handler: Default::default(), - context_builder: Default::default(), - context_assigner: Default::default(), - language_mapper: Default::default(), - context_initializers: Default::default(), - context_pre_handling_initializers: Default::default(), - } - } -} - impl Plugin for ScriptingPlugin

{ fn build(&self, app: &mut bevy::prelude::App) { app.insert_resource(self.runtime_settings.as_ref().cloned().unwrap_or_default()) @@ -96,9 +87,9 @@ impl Plugin for ScriptingPlugin

{ }) .insert_resource::>(ContextLoadingSettings { loader: self.context_builder.clone(), - assigner: Some(self.context_assigner.clone().unwrap_or_default()), - context_initializers: vec![], - context_pre_handling_initializers: vec![], + assigner: self.context_assigner.clone(), + context_initializers: self.context_initializers.clone(), + context_pre_handling_initializers: self.context_pre_handling_initializers.clone(), }); register_script_plugin_systems::

(app); @@ -113,14 +104,6 @@ impl Plugin for ScriptingPlugin

{ } register_types(app); - - for initializer in self.context_initializers.iter() { - app.add_context_initializer::

(*initializer); - } - - for initializer in self.context_pre_handling_initializers.iter() { - app.add_context_pre_handling_initializer::

(*initializer); - } } } @@ -170,9 +153,7 @@ fn once_per_app_init(app: &mut App) { app.add_event::() .add_event::() .init_resource::() - .init_resource::() .init_resource::() - .init_resource::() .init_asset::() .init_resource::() .register_asset_loader(ScriptAssetLoader { @@ -182,21 +163,10 @@ fn once_per_app_init(app: &mut App) { app.add_systems( PostUpdate, - ( - (garbage_collector).in_set(ScriptingSystemSet::GarbageCollection), - (insert_script_metadata).in_set(ScriptingSystemSet::ScriptMetadataInsertion), - (remove_script_metadata).in_set(ScriptingSystemSet::ScriptMetadataRemoval), - ), - ) - .configure_sets( - PostUpdate, - ( - ScriptingSystemSet::ScriptMetadataInsertion.after(bevy::asset::TrackAssets), - ScriptingSystemSet::ScriptCommandDispatch - .after(ScriptingSystemSet::ScriptMetadataInsertion) - .before(ScriptingSystemSet::ScriptMetadataRemoval), - ), + ((garbage_collector).in_set(ScriptingSystemSet::GarbageCollection),), ); + + configure_asset_systems(app); } /// Systems registered per-language @@ -204,11 +174,9 @@ fn register_script_plugin_systems(app: &mut App) { app.add_systems( PostStartup, (initialize_runtime::

).in_set(ScriptingSystemSet::RuntimeInitialization), - ) - .add_systems( - PostUpdate, - ((sync_script_data::

).in_set(ScriptingSystemSet::ScriptCommandDispatch),), ); + + configure_asset_systems_for_plugin::

(app); } /// Register all types that need to be accessed via reflection @@ -243,50 +211,6 @@ impl AddRuntimeInitializer for App { } } -pub trait AddContextInitializer { - fn add_context_initializer( - &mut self, - initializer: ContextInitializer

, - ) -> &mut Self; -} - -impl AddContextInitializer for App { - fn add_context_initializer( - &mut self, - initializer: ContextInitializer

, - ) -> &mut Self { - self.world_mut() - .init_resource::>(); - self.world_mut() - .resource_mut::>() - .as_mut() - .context_initializers - .push(initializer); - self - } -} - -pub trait AddContextPreHandlingInitializer { - fn add_context_pre_handling_initializer( - &mut self, - initializer: ContextPreHandlingInitializer

, - ) -> &mut Self; -} - -impl AddContextPreHandlingInitializer for App { - fn add_context_pre_handling_initializer( - &mut self, - initializer: ContextPreHandlingInitializer

, - ) -> &mut Self { - self.world_mut() - .resource_mut::>() - .as_mut() - .context_pre_handling_initializers - .push(initializer); - self - } -} - pub trait StoreDocumentation { /// Adds a documentation fragment to the documentation store. fn add_documentation_fragment(&mut self, fragment: D) -> &mut Self; @@ -327,47 +251,3 @@ impl StoreDocumentation for App { top_fragment.gen_docs() } } - -#[cfg(test)] -mod test { - use asset::ScriptMetadataStore; - - use super::*; - - #[test] - fn test_default_scripting_plugin_initializes_all_resources_correctly() { - let mut app = App::new(); - - #[derive(Default, Clone)] - struct C; - #[derive(Default, Clone)] - struct R; - - struct Plugin; - - impl IntoScriptPluginParams for Plugin { - type C = C; - type R = R; - const LANGUAGE: Language = Language::Unknown; - - fn build_runtime() -> Self::R { - R - } - } - - app.add_plugins(AssetPlugin::default()); - app.add_plugins(ScriptingPlugin::::default()); - - assert!(app.world().contains_resource::()); - assert!(app.world().contains_resource::()); - assert!(app.world().contains_resource::()); - assert!(app.world().contains_resource::>()); - assert!(app.world().contains_resource::>()); - assert!(app - .world() - .contains_resource::>()); - assert!(app.world().contains_non_send::>()); - assert!(app.world().contains_non_send::>()); - assert!(app.world().contains_resource::()); - } -} diff --git a/crates/bevy_mod_scripting_core/src/runtime.rs b/crates/bevy_mod_scripting_core/src/runtime.rs index 8f0a6aa189..01eb83fc50 100644 --- a/crates/bevy_mod_scripting_core/src/runtime.rs +++ b/crates/bevy_mod_scripting_core/src/runtime.rs @@ -2,7 +2,10 @@ //! The important thing is that there is only one runtime which is used to execute all scripts of a particular type or `context`. use crate::IntoScriptPluginParams; -use bevy::ecs::system::Resource; +use bevy::{ + ecs::system::Resource, + prelude::{NonSendMut, Res}, +}; pub trait Runtime: 'static {} impl Runtime for T {} @@ -35,3 +38,12 @@ impl Clone for RuntimeSettings

{ pub struct RuntimeContainer { pub runtime: P::R, } + +pub fn initialize_runtime( + mut runtime: NonSendMut>, + settings: Res>, +) { + for initializer in settings.initializers.iter() { + (initializer)(&mut runtime.runtime); + } +} diff --git a/crates/bevy_mod_scripting_core/src/systems.rs b/crates/bevy_mod_scripting_core/src/systems.rs deleted file mode 100644 index 57b29c144b..0000000000 --- a/crates/bevy_mod_scripting_core/src/systems.rs +++ /dev/null @@ -1,551 +0,0 @@ -use crate::{ - asset::{ScriptAsset, ScriptAssetSettings, ScriptMetadata, ScriptMetadataStore}, - bindings::{pretty_print::DisplayWithWorld, AppReflectAllocator, WorldAccessGuard, WorldGuard}, - commands::{CreateOrUpdateScript, DeleteScript}, - context::{ContextLoadingSettings, ScriptContexts}, - error::ScriptError, - event::{IntoCallbackLabel, ScriptCallbackEvent, ScriptErrorEvent}, - handler::CallbackSettings, - runtime::{RuntimeContainer, RuntimeSettings}, - script::{ScriptComponent, Scripts}, - IntoScriptPluginParams, -}; -use bevy::{ecs::system::SystemState, prelude::*}; -use std::any::type_name; - -#[derive(SystemSet, Hash, Debug, Eq, PartialEq, Clone)] -/// Labels for various BMS systems -pub enum ScriptingSystemSet { - // Post Setup processes - RuntimeInitialization, - - // Post Update processes - GarbageCollection, - - ScriptMetadataInsertion, - ScriptCommandDispatch, - ScriptMetadataRemoval, -} - -/// Cleans up dangling script allocations -pub fn garbage_collector(allocator: ResMut) { - let mut allocator = allocator.write(); - allocator.clean_garbage_allocations() -} - -pub fn initialize_runtime( - mut runtime: NonSendMut>, - settings: Res>, -) { - for initializer in settings.initializers.iter() { - (initializer)(&mut runtime.runtime); - } -} - -/// Listens to `AssetEvent::Added` events and populates the script metadata store -pub fn insert_script_metadata( - mut events: EventReader>, - script_assets: Res>, - mut asset_path_map: ResMut, - settings: Res, -) { - for event in events.read() { - if let AssetEvent::Added { id } = event { - let asset = script_assets.get(*id); - if let Some(asset) = asset { - let path = &asset.asset_path; - let converter = settings.script_id_mapper.map; - let script_id = converter(path); - - let language = settings.select_script_language(path); - let metadata = ScriptMetadata { - script_id, - language, - }; - info!("Populating script metadata for script: {:?}:", metadata); - asset_path_map.insert(*id, metadata); - } else { - error!("A script was added but it's asset was not found, failed to compute metadata. This script will not be loaded. {}", id); - } - } - } -} - -/// Listens to [`AssetEvent::Removed`] events and removes the corresponding script metadata -pub fn remove_script_metadata( - mut events: EventReader>, - mut asset_path_map: ResMut, -) { - for event in events.read() { - if let AssetEvent::Removed { id } = event { - let previous = asset_path_map.remove(*id); - if let Some(previous) = previous { - info!("Removed script metadata for removed script: {:?}", previous); - } - } - } -} - -/// Listens to [`AssetEvent`] events and dispatches [`CreateOrUpdateScript`] and [`DeleteScript`] commands accordingly. -/// -/// Allows for hot-reloading of scripts. -pub fn sync_script_data( - mut events: EventReader>, - script_assets: Res>, - script_metadata: Res, - mut commands: Commands, -) { - for event in events.read() { - trace!("{}: Received script asset event: {:?}", P::LANGUAGE, event); - match event { - // emitted when a new script asset is loaded for the first time - AssetEvent::Added { id } | AssetEvent::Modified { id } => { - let metadata = match script_metadata.get(*id) { - Some(m) => m, - None => { - error!( - "{}: Script metadata not found for script asset with id: {}. Cannot load script.", - P::LANGUAGE, - id - ); - continue; - } - }; - - if metadata.language != P::LANGUAGE { - trace!( - "{}: Script asset with id: {} is for a different langauge than this sync system. Skipping.", - P::LANGUAGE, - metadata.script_id - ); - continue; - } - - info!( - "{}: Dispatching Creation/Modification command for script: {:?}. Asset Id: {}", - P::LANGUAGE, - metadata, - id - ); - - if let Some(asset) = script_assets.get(*id) { - commands.queue(CreateOrUpdateScript::

::new( - metadata.script_id.clone(), - asset.content.clone(), - Some(script_assets.reserve_handle().clone_weak()), - )); - } - } - AssetEvent::Removed { id } => { - let metadata = match script_metadata.get(*id) { - Some(m) => m, - None => { - error!( - "{}: Script metadata not found for script asset with id: {}. Cannot delete script.", - P::LANGUAGE, - id - ); - return; - } - }; - - info!( - "{}: Dispatching Deletion command for script: {:?}. Asset Id: {}", - P::LANGUAGE, - metadata, - id - ); - commands.queue(DeleteScript::

::new(metadata.script_id.clone())); - } - _ => return, - }; - } -} - -macro_rules! push_err_and_continue { - ($errors:ident, $expr:expr) => { - match $expr { - Ok(v) => v, - Err(e) => { - $errors.push(e); - continue; - } - } - }; -} - -/// Passes events with the specified label to the script callback with the same name and runs the callback -pub fn event_handler( - world: &mut World, - params: &mut SystemState<( - EventReader, - Res>, - Res>, - Res, - Query<(Entity, Ref)>, - )>, -) { - let mut runtime_container = world - .remove_non_send_resource::>() - .unwrap_or_else(|| { - panic!( - "No runtime container for runtime {} found. Was the scripting plugin initialized correctly?", - type_name::() - ) - }); - let runtime = &mut runtime_container.runtime; - let mut script_contexts = world - .remove_non_send_resource::>() - .unwrap_or_else(|| panic!("No script contexts found for context {}", type_name::

())); - - let (mut script_events, callback_settings, context_settings, scripts, entities) = - params.get_mut(world); - - let handler = *callback_settings - .callback_handler - .as_ref() - .unwrap_or_else(|| { - panic!( - "No handler registered for - Runtime: {}, Context: {}", - type_name::(), - type_name::() - ) - }); - let pre_handling_initializers = context_settings.context_pre_handling_initializers.clone(); - let scripts = scripts.clone(); - let mut errors = Vec::default(); - - let events = script_events.read().cloned().collect::>(); - let entity_scripts = entities - .iter() - .map(|(e, s)| (e, s.0.clone())) - .collect::>(); - - for event in events - .into_iter() - .filter(|e| e.label == L::into_callback_label()) - { - for (entity, entity_scripts) in entity_scripts.iter() { - for script_id in entity_scripts.iter() { - match &event.recipients { - crate::event::Recipients::Script(target_script_id) - if target_script_id != script_id => - { - continue - } - crate::event::Recipients::Entity(target_entity) if target_entity != entity => { - continue - } - _ => (), - } - debug!( - "Handling event for script {} on entity {:?}", - script_id, entity - ); - let script = match scripts.scripts.get(script_id) { - Some(s) => s, - None => { - trace!( - "Script `{}` on entity `{:?}` is either still loading or doesn't exist, ignoring.", - script_id, entity - ); - continue; - } - }; - - let ctxt = match script_contexts.contexts.get_mut(&script.context_id) { - Some(ctxt) => ctxt, - None => { - // if we don't have a context for the script, it's either: - // 1. a script for a different language, in which case we ignore it - // 2. something went wrong. This should not happen though and it's best we ignore this - continue; - } - }; - - let handler_result = (handler)( - event.args.clone(), - *entity, - &script.id, - &L::into_callback_label(), - ctxt, - &pre_handling_initializers, - runtime, - world, - ) - .map_err(|e| { - e.with_script(script.id.clone()) - .with_context(format!("Event handling for: Language: {}", P::LANGUAGE)) - }); - - let _ = push_err_and_continue!(errors, handler_result); - } - } - } - - world.insert_non_send_resource(runtime_container); - world.insert_non_send_resource(script_contexts); - - handle_script_errors(world, errors.into_iter()); -} - -/// Handles errors caused by script execution and sends them to the error event channel -pub(crate) fn handle_script_errors + Clone>( - world: &mut World, - errors: I, -) { - let mut error_events = world - .get_resource_mut::>() - .expect("Missing events resource"); - - for error in errors.clone() { - error_events.send(ScriptErrorEvent { error }); - } - - for error in errors { - let arc_world = WorldGuard::new(WorldAccessGuard::new(world)); - bevy::log::error!("{}", error.display_with_world(arc_world)); - } -} - -#[cfg(test)] -mod test { - use std::{borrow::Cow, collections::HashMap}; - - use crate::{ - bindings::script_value::ScriptValue, - event::CallbackLabel, - handler::HandlerFn, - script::{Script, ScriptId}, - }; - - use super::*; - struct OnTestCallback; - - impl IntoCallbackLabel for OnTestCallback { - fn into_callback_label() -> CallbackLabel { - "OnTest".into() - } - } - - struct TestPlugin; - - impl IntoScriptPluginParams for TestPlugin { - type C = TestContext; - type R = TestRuntime; - - const LANGUAGE: crate::asset::Language = crate::asset::Language::Unknown; - - fn build_runtime() -> Self::R { - TestRuntime { - invocations: vec![], - } - } - } - - struct TestRuntime { - pub invocations: Vec<(Entity, ScriptId)>, - } - - struct TestContext { - pub invocations: Vec, - } - - fn setup_app( - handler_fn: HandlerFn

, - runtime: P::R, - contexts: HashMap, - scripts: HashMap, - ) -> App { - let mut app = App::new(); - - app.add_event::(); - app.add_event::(); - app.insert_resource::>(CallbackSettings { - callback_handler: Some(handler_fn), - }); - app.add_systems(Update, event_handler::); - app.insert_resource::(Scripts { scripts }); - app.insert_non_send_resource(RuntimeContainer::

{ runtime }); - app.insert_non_send_resource(ScriptContexts::

{ contexts }); - app.insert_resource(ContextLoadingSettings::

{ - loader: None, - assigner: None, - context_initializers: vec![], - context_pre_handling_initializers: vec![], - }); - app.finish(); - app.cleanup(); - app - } - - #[test] - fn test_handler_called_with_right_args() { - let test_script_id = Cow::Borrowed("test_script"); - let test_ctxt_id = 0; - let test_script = Script { - id: test_script_id.clone(), - asset: None, - context_id: test_ctxt_id, - }; - let scripts = HashMap::from_iter(vec![(test_script_id.clone(), test_script.clone())]); - let contexts = HashMap::from_iter(vec![( - test_ctxt_id, - TestContext { - invocations: vec![], - }, - )]); - let runtime = TestRuntime { - invocations: vec![], - }; - let mut app = setup_app::( - |args, entity, script, _, ctxt, _, runtime, _| { - ctxt.invocations.extend(args); - runtime.invocations.push((entity, script.clone())); - Ok(ScriptValue::Unit) - }, - runtime, - contexts, - scripts, - ); - let test_entity_id = app - .world_mut() - .spawn(ScriptComponent(vec![test_script_id.clone()])) - .id(); - - app.world_mut().send_event(ScriptCallbackEvent::new_for_all( - OnTestCallback::into_callback_label(), - vec![ScriptValue::String("test_args".into())], - )); - app.update(); - - let test_context = app - .world() - .get_non_send_resource::>() - .unwrap(); - let test_runtime = app - .world() - .get_non_send_resource::>() - .unwrap(); - - assert_eq!( - test_context - .contexts - .get(&test_ctxt_id) - .unwrap() - .invocations, - vec![ScriptValue::String("test_args".into())] - ); - - assert_eq!( - test_runtime - .runtime - .invocations - .iter() - .map(|(e, s)| (*e, s.clone())) - .collect::>(), - vec![(test_entity_id, test_script_id.clone())] - ); - } - - #[test] - fn test_handler_called_on_right_recipients() { - let test_script_id = Cow::Borrowed("test_script"); - let test_ctxt_id = 0; - let test_script = Script { - id: test_script_id.clone(), - asset: None, - context_id: test_ctxt_id, - }; - let scripts = HashMap::from_iter(vec![ - (test_script_id.clone(), test_script.clone()), - ( - "wrong".into(), - Script { - id: "wrong".into(), - asset: None, - context_id: 1, - }, - ), - ]); - let contexts = HashMap::from_iter(vec![ - ( - test_ctxt_id, - TestContext { - invocations: vec![], - }, - ), - ( - 1, - TestContext { - invocations: vec![], - }, - ), - ]); - let runtime = TestRuntime { - invocations: vec![], - }; - let mut app = setup_app::( - |args, entity, script, _, ctxt, _, runtime, _| { - ctxt.invocations.extend(args); - runtime.invocations.push((entity, script.clone())); - Ok(ScriptValue::Unit) - }, - runtime, - contexts, - scripts, - ); - let test_entity_id = app - .world_mut() - .spawn(ScriptComponent(vec![test_script_id.clone()])) - .id(); - - app.world_mut().send_event(ScriptCallbackEvent::new( - OnTestCallback::into_callback_label(), - vec![ScriptValue::String("test_args_script".into())], - crate::event::Recipients::Script(test_script_id.clone()), - )); - - app.world_mut().send_event(ScriptCallbackEvent::new( - OnTestCallback::into_callback_label(), - vec![ScriptValue::String("test_args_entity".into())], - crate::event::Recipients::Entity(test_entity_id), - )); - - app.update(); - - let test_context = app - .world() - .get_non_send_resource::>() - .unwrap(); - let test_runtime = app - .world() - .get_non_send_resource::>() - .unwrap(); - - assert_eq!( - test_context - .contexts - .get(&test_ctxt_id) - .unwrap() - .invocations, - vec![ - ScriptValue::String("test_args_script".into()), - ScriptValue::String("test_args_entity".into()) - ] - ); - - assert_eq!( - test_runtime - .runtime - .invocations - .iter() - .map(|(e, s)| (*e, s.clone())) - .collect::>(), - vec![ - (test_entity_id, test_script_id.clone()), - (test_entity_id, test_script_id.clone()) - ] - ); - } -} diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index fe335fb5ad..7ba627e771 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -229,6 +229,10 @@ impl App { cmd.arg("--profile").arg(profile); } + if self.global_args.coverage { + cmd.arg("--coverage"); + } + match self.subcmd { Xtasks::Macros { macro_name } => { cmd.arg("macros").arg(macro_name.as_ref()); @@ -255,11 +259,7 @@ impl App { cmd.arg("--no-rust-docs"); } } - Xtasks::Test { - name, - package, - no_coverage, - } => { + Xtasks::Test { name, package } => { cmd.arg("test"); if let Some(name) = name { cmd.arg("--name").arg(name); @@ -267,9 +267,6 @@ impl App { if let Some(package) = package { cmd.arg("--package").arg(package); } - if no_coverage { - cmd.arg("--no-coverage"); - } } Xtasks::CiCheck => { cmd.arg("ci-check"); @@ -323,6 +320,14 @@ struct GlobalArgs { #[clap(long, short, global = true, value_parser=clap::value_parser!(Features), value_name=Features::to_placeholder(), default_value=Features::default().to_string(),required = false)] features: Features, + #[clap( + long, + global = true, + default_value = "false", + help = "Enable coverage collection for cargo commands" + )] + coverage: bool, + #[clap( long, short, @@ -333,6 +338,23 @@ struct GlobalArgs { profile: Option, } +impl GlobalArgs { + pub fn with_coverage(self) -> Self { + Self { + coverage: false, + ..self + } + } + + pub fn with_features(self, features: Features) -> Self { + Self { features, ..self } + } + + pub fn with_profile(self, profile: Option) -> Self { + Self { profile, ..self } + } +} + #[derive(Debug, Clone, Default, strum::EnumString, strum::VariantNames, strum::AsRefStr)] #[strum(serialize_all = "snake_case")] enum CheckKind { @@ -420,10 +442,6 @@ enum Xtasks { /// Run tests in the given package only #[clap(long)] package: Option, - - /// Run tests without coverage - #[clap(long)] - no_coverage: bool, }, /// Perform a full check as it would be done in CI, except not parallelised CiCheck, @@ -452,22 +470,22 @@ struct CiMatrixRow { impl Xtasks { fn run(self, app_settings: GlobalArgs) -> Result { + if app_settings.coverage { + Self::set_cargo_coverage_settings(); + } + match self { Xtasks::Build => Self::build(app_settings), Xtasks::Check { ide_mode, kind } => Self::check(app_settings, ide_mode, kind), Xtasks::Docs { open, no_rust_docs } => Self::docs(app_settings, open, no_rust_docs), - Xtasks::Test { - name, - package, - no_coverage, - } => Self::test(app_settings, name, package, no_coverage), + Xtasks::Test { name, package } => Self::test(app_settings, name, package), Xtasks::CiCheck => Self::cicd(app_settings), Xtasks::Init => Self::init(app_settings), Xtasks::Macros { macro_name } => match macro_name { Macro::ScriptTests => { let mut settings = app_settings.clone(); settings.features = Features::all_features(); - Self::test(settings, Some("script_test".to_owned()), None, true) + Self::test(settings, Some("script_test".to_owned()), None) } }, Xtasks::CiMatrix => { @@ -781,30 +799,20 @@ impl Xtasks { Ok(()) } - fn test( - app_settings: GlobalArgs, - package: Option, - name: Option, - no_coverage: bool, - ) -> Result<()> { - // run cargo test with instrumentation - - if !no_coverage { - std::env::set_var("CARGO_INCREMENTAL", "0"); - Self::append_rustflags("-Cinstrument-coverage"); - - let target_dir = - std::env::var("CARGO_TARGET_DIR").unwrap_or_else(|_| "target".to_owned()); - let coverage_dir = std::path::PathBuf::from(target_dir).join("coverage"); - let coverage_file = coverage_dir.join("cargo-test-%p-%m.profraw"); + fn set_cargo_coverage_settings() { + // This makes local dev hell + // std::env::set_var("CARGO_INCREMENTAL", "0"); + Self::append_rustflags("-Cinstrument-coverage"); - // clear coverage directory - assert!(coverage_dir != std::path::Path::new("/")); - let _ = std::fs::remove_dir_all(coverage_dir); + let target_dir = std::env::var("CARGO_TARGET_DIR").unwrap_or_else(|_| "target".to_owned()); + let coverage_dir = std::path::PathBuf::from(target_dir).join("coverage"); + let coverage_file = coverage_dir.join("cargo-test-%p-%m.profraw"); - std::env::set_var("LLVM_PROFILE_FILE", coverage_file); - } + std::env::set_var("LLVM_PROFILE_FILE", coverage_file); + } + fn test(app_settings: GlobalArgs, package: Option, name: Option) -> Result<()> { + // run cargo test with instrumentation let mut test_args = vec![]; if let Some(package) = package { test_args.push("--package".to_owned()); @@ -824,7 +832,7 @@ impl Xtasks { )?; // generate coverage report and lcov file - if !no_coverage { + if app_settings.coverage { Self::run_system_command( "grcov", "Generating html coverage report", @@ -903,7 +911,15 @@ impl Xtasks { powersets.reverse(); info!("Powerset: {:?}", powersets); - let profile = app_settings.profile.or(Some("ephemeral-build".to_owned())); + let default_args = app_settings + .clone() + .with_features(Features::all_features()) + .with_profile( + app_settings + .profile + .clone() + .or(Some("ephemeral-build".to_owned())), + ); for feature_set in powersets.iter_mut() { // choose language features @@ -917,10 +933,7 @@ impl Xtasks { } output.push(App { - global_args: GlobalArgs { - features: feature_set.clone(), - profile: profile.clone(), - }, + global_args: default_args.clone().with_features(feature_set.clone()), subcmd: Xtasks::Build, }) } @@ -941,22 +954,14 @@ impl Xtasks { } output.push(App { - global_args: GlobalArgs { - features, - profile: profile.clone(), - }, + global_args: default_args.clone().with_features(features), subcmd: Xtasks::Build, }); } - let global_args = GlobalArgs { - features: Features::all_features(), - profile: profile.clone(), - }; - // next run a full lint check with all features output.push(App { - global_args: global_args.clone(), + global_args: default_args.clone(), subcmd: Xtasks::Check { ide_mode: false, kind: CheckKind::All, @@ -965,8 +970,7 @@ impl Xtasks { // then run docs output.push(App { - global_args: global_args.clone(), - + global_args: default_args.clone(), subcmd: Xtasks::Docs { open: false, no_rust_docs: false, @@ -975,11 +979,10 @@ impl Xtasks { // and finally run tests with coverage output.push(App { - global_args: global_args.clone(), + global_args: default_args.clone().with_coverage(), subcmd: Xtasks::Test { name: None, package: None, - no_coverage: false, }, }); From 00059199b7ffd4a833d1f085fc1ee7c982afcbb7 Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 12:05:07 +0000 Subject: [PATCH 02/34] remove unnecessary imports in bindings --- crates/bevy_api_gen/templates/header.tera | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/bevy_api_gen/templates/header.tera b/crates/bevy_api_gen/templates/header.tera index 435722a94a..a9116009ef 100644 --- a/crates/bevy_api_gen/templates/header.tera +++ b/crates/bevy_api_gen/templates/header.tera @@ -10,8 +10,6 @@ use super::{{crate}}::*; use bevy_mod_scripting_core::{ - AddContextInitializer, - StoreDocumentation, bindings::{ ReflectReference, function::{from::{Ref, Mut, Val}, namespace::{NamespaceBuilder}} From 9702459f5596085d0e42f530b874c6c1971b77a6 Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 12:10:39 +0000 Subject: [PATCH 03/34] update release-plz config --- release-plz.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/release-plz.toml b/release-plz.toml index 8104b48a5e..c6de6120dd 100644 --- a/release-plz.toml +++ b/release-plz.toml @@ -8,6 +8,7 @@ git_tag_enable = false commit_parsers = [ # dont include chore changes in changelog { message = "^chore.*", skip = true }, + { message = "^test.*", skip = true }, { message = "^feat", group = "added" }, { message = "^changed", group = "changed" }, { message = "^deprecated", group = "deprecated" }, From fd2c9c2595700d73dd7ff1e33d84c884905507dc Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 12:12:30 +0000 Subject: [PATCH 04/34] update pr-title filter --- .github/workflows/pr-titles.yml | 4 +++- release-plz.toml | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr-titles.yml b/.github/workflows/pr-titles.yml index a55a70ffdb..9d08f74f92 100644 --- a/.github/workflows/pr-titles.yml +++ b/.github/workflows/pr-titles.yml @@ -23,4 +23,6 @@ jobs: types: | fix feat - chore \ No newline at end of file + chore + test + docs \ No newline at end of file diff --git a/release-plz.toml b/release-plz.toml index c6de6120dd..928bb139f6 100644 --- a/release-plz.toml +++ b/release-plz.toml @@ -9,6 +9,7 @@ commit_parsers = [ # dont include chore changes in changelog { message = "^chore.*", skip = true }, { message = "^test.*", skip = true }, + { message = "^docs.*", skip = true }, { message = "^feat", group = "added" }, { message = "^changed", group = "changed" }, { message = "^deprecated", group = "deprecated" }, From 50502735da361b8df7357d2440e091f19657a3cc Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 12:13:01 +0000 Subject: [PATCH 05/34] trigger bindings ci --- .github/workflows/generate_bindings.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/generate_bindings.yml b/.github/workflows/generate_bindings.yml index 2300ba3748..a9e7b8e163 100644 --- a/.github/workflows/generate_bindings.yml +++ b/.github/workflows/generate_bindings.yml @@ -13,6 +13,7 @@ env: BRANCH_NAME: __update-bevy-bindings-${{ github.head_ref || github.ref_name }} GH_TOKEN: ${{ github.token }} + jobs: generate_bindings: permissions: From 05ffc7e4a85da9cfed07e5ab78ad45f6603133fb Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 15 Jan 2025 12:18:19 +0000 Subject: [PATCH 06/34] chore(codegen): update bevy bindings (#209) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../src/bevy_bindings/bevy_core.rs | 13 +++----- .../src/bevy_bindings/bevy_ecs.rs | 13 +++----- .../src/bevy_bindings/bevy_hierarchy.rs | 13 +++----- .../src/bevy_bindings/bevy_input.rs | 13 +++----- .../src/bevy_bindings/bevy_math.rs | 31 +++++++++---------- .../src/bevy_bindings/bevy_reflect.rs | 13 +++----- .../src/bevy_bindings/bevy_time.rs | 13 +++----- .../src/bevy_bindings/bevy_transform.rs | 13 +++----- 8 files changed, 49 insertions(+), 73 deletions(-) diff --git a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_core.rs b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_core.rs index d63e530405..78dac678a8 100644 --- a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_core.rs +++ b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_core.rs @@ -4,14 +4,11 @@ #![cfg_attr(rustfmt, rustfmt_skip)] use super::bevy_ecs::*; use super::bevy_reflect::*; -use bevy_mod_scripting_core::{ - AddContextInitializer, StoreDocumentation, - bindings::{ - ReflectReference, - function::{ - from::{Ref, Mut, Val}, - namespace::NamespaceBuilder, - }, +use bevy_mod_scripting_core::bindings::{ + ReflectReference, + function::{ + from::{Ref, Mut, Val}, + namespace::NamespaceBuilder, }, }; use crate::*; diff --git a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_ecs.rs b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_ecs.rs index 7a024becf5..2c21229248 100644 --- a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_ecs.rs +++ b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_ecs.rs @@ -3,14 +3,11 @@ #![allow(unused, deprecated, dead_code)] #![cfg_attr(rustfmt, rustfmt_skip)] use super::bevy_reflect::*; -use bevy_mod_scripting_core::{ - AddContextInitializer, StoreDocumentation, - bindings::{ - ReflectReference, - function::{ - from::{Ref, Mut, Val}, - namespace::NamespaceBuilder, - }, +use bevy_mod_scripting_core::bindings::{ + ReflectReference, + function::{ + from::{Ref, Mut, Val}, + namespace::NamespaceBuilder, }, }; use crate::*; diff --git a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_hierarchy.rs b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_hierarchy.rs index d3795e583b..d429479d29 100644 --- a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_hierarchy.rs +++ b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_hierarchy.rs @@ -5,14 +5,11 @@ use super::bevy_ecs::*; use super::bevy_reflect::*; use super::bevy_core::*; -use bevy_mod_scripting_core::{ - AddContextInitializer, StoreDocumentation, - bindings::{ - ReflectReference, - function::{ - from::{Ref, Mut, Val}, - namespace::NamespaceBuilder, - }, +use bevy_mod_scripting_core::bindings::{ + ReflectReference, + function::{ + from::{Ref, Mut, Val}, + namespace::NamespaceBuilder, }, }; use crate::*; diff --git a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_input.rs b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_input.rs index 0cc7e22b21..31a0471f4e 100644 --- a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_input.rs +++ b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_input.rs @@ -6,14 +6,11 @@ use super::bevy_ecs::*; use super::bevy_reflect::*; use super::bevy_core::*; use super::bevy_math::*; -use bevy_mod_scripting_core::{ - AddContextInitializer, StoreDocumentation, - bindings::{ - ReflectReference, - function::{ - from::{Ref, Mut, Val}, - namespace::NamespaceBuilder, - }, +use bevy_mod_scripting_core::bindings::{ + ReflectReference, + function::{ + from::{Ref, Mut, Val}, + namespace::NamespaceBuilder, }, }; use crate::*; diff --git a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_math.rs b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_math.rs index c737c60758..6e5d7c68d2 100644 --- a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_math.rs +++ b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_math.rs @@ -3,14 +3,11 @@ #![allow(unused, deprecated, dead_code)] #![cfg_attr(rustfmt, rustfmt_skip)] use super::bevy_reflect::*; -use bevy_mod_scripting_core::{ - AddContextInitializer, StoreDocumentation, - bindings::{ - ReflectReference, - function::{ - from::{Ref, Mut, Val}, - namespace::NamespaceBuilder, - }, +use bevy_mod_scripting_core::bindings::{ + ReflectReference, + function::{ + from::{Ref, Mut, Val}, + namespace::NamespaceBuilder, }, }; use crate::*; @@ -292,12 +289,9 @@ impl ::bevy::app::Plugin for BevyMathScriptingPlugin { ) .register( "mul", - | - _self: Val, - rhs: Val| - { - let output: Val = , rhs: Val| { + let output: Val = >::mul(_self.into_inner(), rhs.into_inner()) .into(); output @@ -305,9 +299,12 @@ impl ::bevy::app::Plugin for BevyMathScriptingPlugin { ) .register( "mul", - |_self: Val, rhs: Val| { - let output: Val = , + rhs: Val| + { + let output: Val = >::mul(_self.into_inner(), rhs.into_inner()) .into(); output diff --git a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_reflect.rs b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_reflect.rs index 13f1cff050..1307cb3e1a 100644 --- a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_reflect.rs +++ b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_reflect.rs @@ -2,14 +2,11 @@ #![allow(clippy::all)] #![allow(unused, deprecated, dead_code)] #![cfg_attr(rustfmt, rustfmt_skip)] -use bevy_mod_scripting_core::{ - AddContextInitializer, StoreDocumentation, - bindings::{ - ReflectReference, - function::{ - from::{Ref, Mut, Val}, - namespace::NamespaceBuilder, - }, +use bevy_mod_scripting_core::bindings::{ + ReflectReference, + function::{ + from::{Ref, Mut, Val}, + namespace::NamespaceBuilder, }, }; use crate::*; diff --git a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_time.rs b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_time.rs index 3175e34e8d..4e5f4d5a67 100644 --- a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_time.rs +++ b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_time.rs @@ -4,14 +4,11 @@ #![cfg_attr(rustfmt, rustfmt_skip)] use super::bevy_ecs::*; use super::bevy_reflect::*; -use bevy_mod_scripting_core::{ - AddContextInitializer, StoreDocumentation, - bindings::{ - ReflectReference, - function::{ - from::{Ref, Mut, Val}, - namespace::NamespaceBuilder, - }, +use bevy_mod_scripting_core::bindings::{ + ReflectReference, + function::{ + from::{Ref, Mut, Val}, + namespace::NamespaceBuilder, }, }; use crate::*; diff --git a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_transform.rs b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_transform.rs index a9af8d8918..ce8666a335 100644 --- a/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_transform.rs +++ b/crates/bevy_mod_scripting_functions/src/bevy_bindings/bevy_transform.rs @@ -7,14 +7,11 @@ use super::bevy_reflect::*; use super::bevy_core::*; use super::bevy_math::*; use super::bevy_hierarchy::*; -use bevy_mod_scripting_core::{ - AddContextInitializer, StoreDocumentation, - bindings::{ - ReflectReference, - function::{ - from::{Ref, Mut, Val}, - namespace::NamespaceBuilder, - }, +use bevy_mod_scripting_core::bindings::{ + ReflectReference, + function::{ + from::{Ref, Mut, Val}, + namespace::NamespaceBuilder, }, }; use crate::*; From d108cbd9f26f1b2a67927bec4810ce162fab9511 Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 12:45:42 +0000 Subject: [PATCH 07/34] Improve UI for adding intiializers and add docs, correct initialization of lua plugins --- crates/bevy_mod_scripting_core/src/lib.rs | 36 ++++++++++++++++++- .../bevy_mod_scripting_lua/src/lib.rs | 13 +++++-- .../bevy_mod_scripting_lua/tests/lua_tests.rs | 7 ++-- .../src/lib.rs | 2 +- .../Summary/customizing-script-contexts.md | 9 ++--- examples/game_of_life.rs | 2 +- 6 files changed, 53 insertions(+), 16 deletions(-) diff --git a/crates/bevy_mod_scripting_core/src/lib.rs b/crates/bevy_mod_scripting_core/src/lib.rs index f8db9c41c8..901ed4ae44 100644 --- a/crates/bevy_mod_scripting_core/src/lib.rs +++ b/crates/bevy_mod_scripting_core/src/lib.rs @@ -65,7 +65,7 @@ pub struct ScriptingPlugin { pub callback_handler: Option>, /// The context builder for loading contexts pub context_builder: ContextBuilder

, - /// The context assigner for assigning contexts to scripts, if not provided default strategy of keeping each script in its own context is used + /// The context assigner for assigning contexts to scripts. pub context_assigner: ContextAssigner

, pub language_mapper: Option, @@ -139,6 +139,40 @@ impl ScriptingPlugin

{ } } +/// Utility trait for configuring all scripting plugins. +pub trait ConfigureScriptPlugin { + type P: IntoScriptPluginParams; + fn add_context_initializer(self, initializer: ContextInitializer) -> Self; + fn add_context_pre_handling_initializer( + self, + initializer: ContextPreHandlingInitializer, + ) -> Self; + fn add_runtime_initializer(self, initializer: RuntimeInitializer) -> Self; +} + +impl>> ConfigureScriptPlugin for P { + type P = P; + + fn add_context_initializer(mut self, initializer: ContextInitializer) -> Self { + self.as_mut().add_context_initializer(initializer); + self + } + + fn add_context_pre_handling_initializer( + mut self, + initializer: ContextPreHandlingInitializer, + ) -> Self { + self.as_mut() + .add_context_pre_handling_initializer(initializer); + self + } + + fn add_runtime_initializer(mut self, initializer: RuntimeInitializer) -> Self { + self.as_mut().add_runtime_initializer(initializer); + self + } +} + // One of registration of things that need to be done only once per app fn once_per_app_init(app: &mut App) { #[derive(Resource)] diff --git a/crates/languages/bevy_mod_scripting_lua/src/lib.rs b/crates/languages/bevy_mod_scripting_lua/src/lib.rs index a049808864..3861eb7bca 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/lib.rs +++ b/crates/languages/bevy_mod_scripting_lua/src/lib.rs @@ -31,6 +31,13 @@ impl IntoScriptPluginParams for LuaScriptingPlugin { fn build_runtime() -> Self::R {} } +// necessary for automatic config goodies +impl AsMut> for LuaScriptingPlugin { + fn as_mut(&mut self) -> &mut ScriptingPlugin { + &mut self.scripting_plugin + } +} + pub struct LuaScriptingPlugin { pub scripting_plugin: ScriptingPlugin, } @@ -39,13 +46,13 @@ impl Default for LuaScriptingPlugin { fn default() -> Self { LuaScriptingPlugin { scripting_plugin: ScriptingPlugin { - context_assigner: None, + context_assigner: Default::default(), runtime_settings: None, callback_handler: Some(lua_handler), - context_builder: Some(ContextBuilder:: { + context_builder: ContextBuilder:: { load: lua_context_load, reload: lua_context_reload, - }), + }, language_mapper: Some(AssetPathToLanguageMapper { map: lua_language_mapper, }), diff --git a/crates/languages/bevy_mod_scripting_lua/tests/lua_tests.rs b/crates/languages/bevy_mod_scripting_lua/tests/lua_tests.rs index 672a5f576d..cfc38a3859 100644 --- a/crates/languages/bevy_mod_scripting_lua/tests/lua_tests.rs +++ b/crates/languages/bevy_mod_scripting_lua/tests/lua_tests.rs @@ -1,7 +1,7 @@ use bevy_mod_scripting_core::{ bindings::{pretty_print::DisplayWithWorld, ThreadWorldContainer, WorldContainer}, error::ScriptError, - AddContextInitializer, + ConfigureScriptPlugin, }; use bevy_mod_scripting_lua::LuaScriptingPlugin; use libtest_mimic::{Arguments, Failed, Trial}; @@ -26,8 +26,7 @@ impl Test { let _ = type_registry; }, |app| { - app.add_plugins(LuaScriptingPlugin::default()); - app.add_context_initializer::(|_,ctxt: &mut Lua| { + app.add_plugins(LuaScriptingPlugin::default().add_context_initializer(|_,ctxt: &mut Lua| { let globals = ctxt.globals(); globals.set( "assert_throws", @@ -60,7 +59,7 @@ impl Test { })?, )?; Ok(()) - }); + })); }, self.path.as_os_str().to_str().unwrap(), self.code.as_bytes(), diff --git a/crates/script_integration_test_harness/src/lib.rs b/crates/script_integration_test_harness/src/lib.rs index d0c7d0b0e0..02d56125b9 100644 --- a/crates/script_integration_test_harness/src/lib.rs +++ b/crates/script_integration_test_harness/src/lib.rs @@ -66,7 +66,7 @@ pub fn execute_integration_test< }); // load the context as normal - let mut loaded_context = (context_settings.loader.unwrap().load)( + let mut loaded_context = (context_settings.loader.load)( &(script_id.to_owned()).into(), code, &context_settings.context_initializers, diff --git a/docs/src/Summary/customizing-script-contexts.md b/docs/src/Summary/customizing-script-contexts.md index 68cd7c5339..f06b960799 100644 --- a/docs/src/Summary/customizing-script-contexts.md +++ b/docs/src/Summary/customizing-script-contexts.md @@ -11,8 +11,7 @@ For example, let's say you want to set a dynamic amount of globals in your scrip You could do this by customizing the scripting plugin: ```rust,ignore -let plugin = LuaScriptingPlugin::default(); -plugin.add_context_initializer(|script_id: &str, context: &mut Lua| { +let plugin = LuaScriptingPlugin::default().add_context_initializer(|script_id: &str, context: &mut Lua| { let globals = context.globals(); for i in 0..10 { globals.set(i, i); @@ -29,8 +28,7 @@ The above will run every time the script is loaded or re-loaded. If you want to customize your context before every time it's about to handle events, you can use `Context Pre Handling Initializers`: ```rust,ignore -let plugin = LuaScriptingPlugin::default(); -plugin.add_context_pre_handling_initializer(|script_id: &str, entity: Entity, context: &mut Lua| { +let plugin = LuaScriptingPlugin::default().add_context_pre_handling_initializer(|script_id: &str, entity: Entity, context: &mut Lua| { let globals = context.globals(); globals.set("script_name", script_id.to_owned()); Ok(()) @@ -40,8 +38,7 @@ plugin.add_context_pre_handling_initializer(|script_id: &str, entity: Entity, co Some scripting languages, have the concept of a `runtime`. This is a global object which is shared between all contexts. You can customize this object using `Runtime Initializers`: ```rust,ignore -let plugin = SomeScriptingPlugin::default(); -plugin.add_runtime_initializer(|runtime: &mut Runtime| { +let plugin = SomeScriptingPlugin::default().add_runtime_initializer(|runtime: &mut Runtime| { runtime.set_max_stack_size(1000); Ok(()) }); diff --git a/examples/game_of_life.rs b/examples/game_of_life.rs index a7fa3617cb..2827958196 100644 --- a/examples/game_of_life.rs +++ b/examples/game_of_life.rs @@ -18,8 +18,8 @@ use bevy_mod_scripting_core::{ bindings::{function::namespace::NamespaceBuilder, script_value::ScriptValue}, callback_labels, event::ScriptCallbackEvent, + handler::event_handler, script::ScriptComponent, - systems::event_handler, }; use bevy_mod_scripting_lua::LuaScriptingPlugin; // use bevy_mod_scripting_rhai::RhaiScriptingPlugin; From a54e1396aed5cffa676ef42249b11f62d57984aa Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 13:05:20 +0000 Subject: [PATCH 08/34] remove some unnecessary optionals --- .../bevy_mod_scripting_core/src/commands.rs | 5 ++-- crates/bevy_mod_scripting_core/src/handler.rs | 23 ++------------- crates/bevy_mod_scripting_core/src/lib.rs | 28 ++++++++----------- .../bevy_mod_scripting_lua/src/lib.rs | 9 +++--- .../src/lib.rs | 4 +-- 5 files changed, 23 insertions(+), 46 deletions(-) diff --git a/crates/bevy_mod_scripting_core/src/commands.rs b/crates/bevy_mod_scripting_core/src/commands.rs index 713034a748..8e56ab3eab 100644 --- a/crates/bevy_mod_scripting_core/src/commands.rs +++ b/crates/bevy_mod_scripting_core/src/commands.rs @@ -35,8 +35,7 @@ impl Command for DeleteScript

{ let runner = world .get_resource::>() .expect("No CallbackSettings resource found") - .callback_handler - .expect("No callback handler set"); + .callback_handler; let mut ctxts = world .remove_non_send_resource::>() @@ -132,7 +131,7 @@ impl Command for CreateOrUpdateScript

{ // assign context let assigner = settings.assigner.clone(); let builder = settings.loader.clone(); - let runner = runner.callback_handler.expect("No callback handler set"); + let runner = runner.callback_handler; world.resource_scope(|world, mut scripts: Mut| { diff --git a/crates/bevy_mod_scripting_core/src/handler.rs b/crates/bevy_mod_scripting_core/src/handler.rs index 61c60d4410..b395f432e3 100644 --- a/crates/bevy_mod_scripting_core/src/handler.rs +++ b/crates/bevy_mod_scripting_core/src/handler.rs @@ -38,15 +38,7 @@ pub type HandlerFn

= fn( /// A resource that holds the settings for the callback handler for a specific combination of type parameters #[derive(Resource)] pub struct CallbackSettings { - pub callback_handler: Option>, -} - -impl Default for CallbackSettings

{ - fn default() -> Self { - Self { - callback_handler: None, - } - } + pub callback_handler: HandlerFn

, } macro_rules! push_err_and_continue { @@ -88,16 +80,7 @@ pub fn event_handler( let (mut script_events, callback_settings, context_settings, scripts, entities) = params.get_mut(world); - let handler = *callback_settings - .callback_handler - .as_ref() - .unwrap_or_else(|| { - panic!( - "No handler registered for - Runtime: {}, Context: {}", - type_name::(), - type_name::() - ) - }); + let handler = callback_settings.callback_handler; let pre_handling_initializers = context_settings.context_pre_handling_initializers.clone(); let scripts = scripts.clone(); let mut errors = Vec::default(); @@ -253,7 +236,7 @@ mod test { app.add_event::(); app.add_event::(); app.insert_resource::>(CallbackSettings { - callback_handler: Some(handler_fn), + callback_handler: handler_fn, }); app.add_systems(Update, event_handler::); app.insert_resource::(Scripts { scripts }); diff --git a/crates/bevy_mod_scripting_core/src/lib.rs b/crates/bevy_mod_scripting_core/src/lib.rs index 901ed4ae44..fa0c4a50ea 100644 --- a/crates/bevy_mod_scripting_core/src/lib.rs +++ b/crates/bevy_mod_scripting_core/src/lib.rs @@ -53,21 +53,20 @@ pub trait IntoScriptPluginParams: 'static { type R: Runtime; fn build_runtime() -> Self::R; - - // fn supported_language() -> Language; } /// Bevy plugin enabling scripting within the bevy mod scripting framework pub struct ScriptingPlugin { /// Settings for the runtime - pub runtime_settings: Option>, + pub runtime_settings: RuntimeSettings

, /// The handler used for executing callbacks in scripts - pub callback_handler: Option>, + pub callback_handler: HandlerFn

, /// The context builder for loading contexts pub context_builder: ContextBuilder

, /// The context assigner for assigning contexts to scripts. pub context_assigner: ContextAssigner

, - pub language_mapper: Option, + + pub language_mapper: AssetPathToLanguageMapper, /// initializers for the contexts, run when loading the script pub context_initializers: Vec>, @@ -77,7 +76,7 @@ pub struct ScriptingPlugin { impl Plugin for ScriptingPlugin

{ fn build(&self, app: &mut bevy::prelude::App) { - app.insert_resource(self.runtime_settings.as_ref().cloned().unwrap_or_default()) + app.insert_resource(self.runtime_settings.clone()) .insert_non_send_resource::>(RuntimeContainer { runtime: P::build_runtime(), }) @@ -95,13 +94,11 @@ impl Plugin for ScriptingPlugin

{ register_script_plugin_systems::

(app); once_per_app_init(app); - if let Some(language_mapper) = &self.language_mapper { - app.world_mut() - .resource_mut::() - .as_mut() - .script_language_mappers - .push(*language_mapper); - } + app.world_mut() + .resource_mut::() + .as_mut() + .script_language_mappers + .push(self.language_mapper); register_types(app); } @@ -131,10 +128,7 @@ impl ScriptingPlugin

{ /// /// Initializers will be run after the runtime is created, but before any contexts are loaded. pub fn add_runtime_initializer(&mut self, initializer: RuntimeInitializer

) -> &mut Self { - self.runtime_settings - .get_or_insert_with(Default::default) - .initializers - .push(initializer); + self.runtime_settings.initializers.push(initializer); self } } diff --git a/crates/languages/bevy_mod_scripting_lua/src/lib.rs b/crates/languages/bevy_mod_scripting_lua/src/lib.rs index 3861eb7bca..7cd05e2bc8 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/lib.rs +++ b/crates/languages/bevy_mod_scripting_lua/src/lib.rs @@ -12,6 +12,7 @@ use bevy_mod_scripting_core::{ error::ScriptError, event::CallbackLabel, reflection_extensions::PartialReflectExt, + runtime::RuntimeSettings, script::ScriptId, IntoScriptPluginParams, ScriptingPlugin, }; @@ -47,15 +48,15 @@ impl Default for LuaScriptingPlugin { LuaScriptingPlugin { scripting_plugin: ScriptingPlugin { context_assigner: Default::default(), - runtime_settings: None, - callback_handler: Some(lua_handler), + runtime_settings: RuntimeSettings::default(), + callback_handler: lua_handler, context_builder: ContextBuilder:: { load: lua_context_load, reload: lua_context_reload, }, - language_mapper: Some(AssetPathToLanguageMapper { + language_mapper: AssetPathToLanguageMapper { map: lua_language_mapper, - }), + }, context_initializers: vec![ |_script_id, context| { // set the world global diff --git a/crates/script_integration_test_harness/src/lib.rs b/crates/script_integration_test_harness/src/lib.rs index 02d56125b9..97f6c562b2 100644 --- a/crates/script_integration_test_harness/src/lib.rs +++ b/crates/script_integration_test_harness/src/lib.rs @@ -81,7 +81,7 @@ pub fn execute_integration_test< })?; // call on_script_loaded as normal - let val = (callback_settings.callback_handler.unwrap())( + let val = (callback_settings.callback_handler)( vec![], Entity::from_raw(0), &(script_id.to_owned()).into(), @@ -98,7 +98,7 @@ pub fn execute_integration_test< } // call on_test callback - let val = (callback_settings.callback_handler.unwrap())( + let val = (callback_settings.callback_handler)( vec![], Entity::from_raw(0), &(script_id.to_owned()).into(), From 86915f5160ed86f6b4eea1d0a616a6e4ca066c35 Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 13:05:25 +0000 Subject: [PATCH 09/34] report coverage --- .github/workflows/bevy_mod_scripting.yml | 29 +++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index 2cc2180e25..868dbedd50 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -73,4 +73,31 @@ jobs: cargo xtask init - name: Check run: | - ${{ matrix.run_args.command }} \ No newline at end of file + ${{ matrix.run_args.command }} + - name: Report code coverage + uses: dethereum/github-actions-report-lcov@v1.0.0 + - name: Upload existing lcov files if any exist + uses: actions/upload-artifact@v2 + with: + name: lcov-${{ matrix.run_args.os }}-${{ matrix.run_args.name }} + path: | + **/lcov.* + if-no-files-found: ignore + + report-coverage: + permissions: + pull-requests: write + steps: + - name: Download all lcov files + uses: actions/download-artifact@v2 + with: + merge-multiple: true + pattern: lcov-* + download-path: coverage + - name: Report code coverage + uses: dethereum/github-actions-report-lcov@v1.0.0 + with: + coverage-files: coverage/lcov.* + minimum-coverage: 50 + artifact-name: code-coverage-report + github-token: ${{ secrets.GITHUB_TOKEN }} \ No newline at end of file From 7b4746b27ce1cf1bc481770ceab16f575d53a8d0 Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 13:05:39 +0000 Subject: [PATCH 10/34] remove failing todo --- crates/bevy_mod_scripting_core/src/asset.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_mod_scripting_core/src/asset.rs b/crates/bevy_mod_scripting_core/src/asset.rs index 179c404bb9..fb4830602a 100644 --- a/crates/bevy_mod_scripting_core/src/asset.rs +++ b/crates/bevy_mod_scripting_core/src/asset.rs @@ -598,8 +598,8 @@ mod tests { assert_eq!(metadata_len, 0); } - #[test] - fn test_syncing_assets() { - todo!() - } + // #[test] + // fn test_syncing_assets() { + // todo!() + // } } From f4596ba8ac2983c39ea2f88ff902cd415aa72bb0 Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 13:06:20 +0000 Subject: [PATCH 11/34] set error settings --- .github/workflows/bevy_mod_scripting.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index 868dbedd50..72a779c8fb 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -85,6 +85,8 @@ jobs: if-no-files-found: ignore report-coverage: + runs-on: ubuntu-latest + continue-on-error: true permissions: pull-requests: write steps: From 7c908eb0569cbaf7e03bc5f9749865ef1c04d475 Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 13:07:59 +0000 Subject: [PATCH 12/34] fix workflow syntax --- .github/workflows/bevy_mod_scripting.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index 72a779c8fb..2e244f611a 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -74,9 +74,8 @@ jobs: - name: Check run: | ${{ matrix.run_args.command }} - - name: Report code coverage - uses: dethereum/github-actions-report-lcov@v1.0.0 - - name: Upload existing lcov files if any exist + + - name: Upload lcov files if any exist uses: actions/upload-artifact@v2 with: name: lcov-${{ matrix.run_args.os }}-${{ matrix.run_args.name }} From 1e982a8729f593739af40d06150a72b2c17b677b Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 13:08:47 +0000 Subject: [PATCH 13/34] run after check and bump versions --- .github/workflows/bevy_mod_scripting.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index 2e244f611a..d4a7bbdf25 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -76,7 +76,7 @@ jobs: ${{ matrix.run_args.command }} - name: Upload lcov files if any exist - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: lcov-${{ matrix.run_args.os }}-${{ matrix.run_args.name }} path: | @@ -84,13 +84,14 @@ jobs: if-no-files-found: ignore report-coverage: + needs: check runs-on: ubuntu-latest continue-on-error: true permissions: pull-requests: write steps: - name: Download all lcov files - uses: actions/download-artifact@v2 + uses: actions/download-artifact@v4 with: merge-multiple: true pattern: lcov-* From f37f71b7657cf563b079b8116d667aa4e06b2fec Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 13:22:54 +0000 Subject: [PATCH 14/34] actually include coverage in the run --- crates/xtask/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index 7ba627e771..acba7980f8 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -341,7 +341,7 @@ struct GlobalArgs { impl GlobalArgs { pub fn with_coverage(self) -> Self { Self { - coverage: false, + coverage: true, ..self } } From 782d3b944baef12fad306972b6350946bd9fb5ba Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 14:04:54 +0000 Subject: [PATCH 15/34] install genhtml --- .github/workflows/bevy_mod_scripting.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index d4a7bbdf25..bc96f6cb2f 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -96,6 +96,9 @@ jobs: merge-multiple: true pattern: lcov-* download-path: coverage + - name: Install genhtml + run: | + sudo apt-get install lcov -y - name: Report code coverage uses: dethereum/github-actions-report-lcov@v1.0.0 with: From da86e0c7c13fcd1c3db8009035496cf80c212765 Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 18:30:09 +0000 Subject: [PATCH 16/34] actually use the right path and only upload coverage from ubuntu --- .github/workflows/bevy_mod_scripting.yml | 6 +- crates/xtask/src/main.rs | 80 ++++++++++++++---------- 2 files changed, 51 insertions(+), 35 deletions(-) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index bc96f6cb2f..9c160376b4 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -76,6 +76,7 @@ jobs: ${{ matrix.run_args.command }} - name: Upload lcov files if any exist + if: ${{ matrix.run_args.os == 'ubuntu-latest' }} uses: actions/upload-artifact@v4 with: name: lcov-${{ matrix.run_args.os }}-${{ matrix.run_args.name }} @@ -100,9 +101,10 @@ jobs: run: | sudo apt-get install lcov -y - name: Report code coverage - uses: dethereum/github-actions-report-lcov@v1.0.0 + uses: zgosalvez/github-actions-report-lcov@v3 with: - coverage-files: coverage/lcov.* + coverage-files: | + **/lcov.info minimum-coverage: 50 artifact-name: code-coverage-report github-token: ${{ secrets.GITHUB_TOKEN }} \ No newline at end of file diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index acba7980f8..9c80f7951e 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -355,7 +355,18 @@ impl GlobalArgs { } } -#[derive(Debug, Clone, Default, strum::EnumString, strum::VariantNames, strum::AsRefStr)] +#[derive( + Debug, + Clone, + PartialEq, + Eq, + PartialOrd, + Ord, + Default, + strum::EnumString, + strum::VariantNames, + strum::AsRefStr, +)] #[strum(serialize_all = "snake_case")] enum CheckKind { #[default] @@ -370,7 +381,17 @@ impl CheckKind { } } -#[derive(Debug, Clone, strum::EnumString, strum::AsRefStr, strum::VariantNames)] +#[derive( + Debug, + Clone, + PartialEq, + Eq, + PartialOrd, + Ord, + strum::EnumString, + strum::AsRefStr, + strum::VariantNames, +)] #[strum(serialize_all = "snake_case")] enum Macro { /// Integration tests for all script plugins @@ -383,7 +404,7 @@ impl Macro { } } -#[derive(Clone, Debug, clap::Subcommand, strum::AsRefStr)] +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, clap::Subcommand, strum::AsRefStr)] #[clap( name = "xtask", bin_name = "cargo xtask", @@ -489,38 +510,31 @@ impl Xtasks { } }, Xtasks::CiMatrix => { - let output = Self::ci_matrix(app_settings)?; - let mut matrix = output - .into_iter() - .map(|a| a.into_ci_row("ubuntu-latest".to_owned())) - .collect::>(); - - // clone for macos and windows for certain steps - let mut multi_os_steps = matrix.clone(); - - // we don't need to verify all feature flags on all platforms, this is mostly a "does it compile" check - // for finding out missing compile time logic or bad imports - multi_os_steps - .retain(|e| !e.command.contains(" build") && !e.command.contains(" docs")); - - let mut macos_matrix = multi_os_steps.clone(); - let mut windows_matrix = multi_os_steps.clone(); - - for row in macos_matrix.iter_mut() { - row.os = "macos-latest".to_owned(); - row.name = row.name.replace("ubuntu-latest", "macos-latest"); + let mut output = Self::ci_matrix(app_settings)?; + output.sort_by(|e1, e2| e1.subcmd.cmp(&e2.subcmd)); + let mut rows = Vec::default(); + for os in &["ubuntu-latest", "macos-latest", "windows-latest"] { + for row in output.iter_mut() { + let is_main_os = os == &"ubuntu-latest"; + let step_should_run_on_main_os = + matches!(row.subcmd, Xtasks::Build | Xtasks::Docs { .. }); + let is_coverage_step = row.global_args.coverage; + + if !is_main_os && step_should_run_on_main_os { + continue; + } + + // we only need one source of coverage + windows is slow with this setting + if !is_main_os && is_coverage_step { + row.global_args.coverage = false; + } + + let row = row.clone().into_ci_row(os.to_string()); + rows.push(row); + } } - for row in windows_matrix.iter_mut() { - row.os = "windows-latest".to_owned(); - row.name = row.name.replace("ubuntu-latest", "windows-latest"); - } - - matrix.extend(macos_matrix); - matrix.extend(windows_matrix); - - matrix.sort_by_key(|e| e.name.to_owned()); - let json = serde_json::to_string_pretty(&matrix)?; + let json = serde_json::to_string_pretty(&rows)?; return Ok(json); } }?; From b8ead876ce04dbaab0ac4268e84f5587d0d36069 Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 19:05:59 +0000 Subject: [PATCH 17/34] change profile for coverage runs --- Cargo.toml | 4 ++++ crates/xtask/src/main.rs | 44 ++++++++++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a82d20b880..31eb9bcf27 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -105,6 +105,10 @@ codegen-units = 8 incremental = false debug = false +[profile.ephemeral-coverage] +inherits = "ephemeral-build" +debug = true + [profile.release-with-debug] inherits = "release" debug = true diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index 9c80f7951e..e9027ac406 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -346,6 +346,13 @@ impl GlobalArgs { } } + pub fn without_coverage(self) -> Self { + Self { + coverage: false, + ..self + } + } + pub fn with_features(self, features: Features) -> Self { Self { features, ..self } } @@ -514,7 +521,7 @@ impl Xtasks { output.sort_by(|e1, e2| e1.subcmd.cmp(&e2.subcmd)); let mut rows = Vec::default(); for os in &["ubuntu-latest", "macos-latest", "windows-latest"] { - for row in output.iter_mut() { + for row in output.iter() { let is_main_os = os == &"ubuntu-latest"; let step_should_run_on_main_os = matches!(row.subcmd, Xtasks::Build | Xtasks::Docs { .. }); @@ -525,11 +532,17 @@ impl Xtasks { } // we only need one source of coverage + windows is slow with this setting - if !is_main_os && is_coverage_step { - row.global_args.coverage = false; - } + let row = if !is_main_os && is_coverage_step { + let new_args = row.global_args.clone().without_coverage(); + App { + global_args: new_args, + ..row.clone() + } + .into_ci_row(os.to_string()) + } else { + row.clone().into_ci_row(os.to_string()) + }; - let row = row.clone().into_ci_row(os.to_string()); rows.push(row); } } @@ -611,7 +624,12 @@ impl Xtasks { add_args: I, dir: Option<&Path>, ) -> Result<()> { - info!("Running workspace command: {}", command); + let coverage_mode = app_settings + .coverage + .then_some("with coverage") + .unwrap_or_default(); + + info!("Running workspace command {coverage_mode}: {command}"); let mut args = vec![]; args.push(command.to_owned()); @@ -622,8 +640,18 @@ impl Xtasks { args.push("--workspace".to_owned()); if let Some(profile) = app_settings.profile.as_ref() { - args.push("--profile".to_owned()); - args.push(profile.clone()); + let use_profile = if profile == "ephemeral-build" && app_settings.coverage { + // use special profile for coverage as it needs debug information + // but also don't want it too slow + "ephemeral-coverage" + } else { + profile + }; + + if !app_settings.coverage { + args.push("--profile".to_owned()); + args.push(use_profile.to_owned()); + } } args.extend(app_settings.features.to_cargo_args()); From 2809440435e3fef59f350e71611b056d1853967b Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 19:12:11 +0000 Subject: [PATCH 18/34] move coverage gen into the matrix --- .github/workflows/bevy_mod_scripting.yml | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index 9c160376b4..ab60cf7427 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -83,24 +83,12 @@ jobs: path: | **/lcov.* if-no-files-found: ignore - - report-coverage: - needs: check - runs-on: ubuntu-latest - continue-on-error: true - permissions: - pull-requests: write - steps: - - name: Download all lcov files - uses: actions/download-artifact@v4 - with: - merge-multiple: true - pattern: lcov-* - download-path: coverage - - name: Install genhtml + - name: Install lcov/genhtml + if: ${{ matrix.run_args.os == 'ubuntu-latest' }} run: | sudo apt-get install lcov -y - name: Report code coverage + if: ${{ matrix.run_args.os == 'ubuntu-latest' }} uses: zgosalvez/github-actions-report-lcov@v3 with: coverage-files: | From 4d3b82f1df78d6415a6f6c063953074a9ccc5a0a Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 19:31:14 +0000 Subject: [PATCH 19/34] output coverage in the matrix --- .github/workflows/bevy_mod_scripting.yml | 13 ++----- crates/xtask/src/main.rs | 44 +++++++++++++++++++----- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index ab60cf7427..416d9a2c97 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -74,21 +74,12 @@ jobs: - name: Check run: | ${{ matrix.run_args.command }} - - - name: Upload lcov files if any exist - if: ${{ matrix.run_args.os == 'ubuntu-latest' }} - uses: actions/upload-artifact@v4 - with: - name: lcov-${{ matrix.run_args.os }}-${{ matrix.run_args.name }} - path: | - **/lcov.* - if-no-files-found: ignore - name: Install lcov/genhtml - if: ${{ matrix.run_args.os == 'ubuntu-latest' }} + if: ${{ matrix.run_args.generates_coverage }} run: | sudo apt-get install lcov -y - name: Report code coverage - if: ${{ matrix.run_args.os == 'ubuntu-latest' }} + if: ${{ matrix.run_args.generates_coverage }} uses: zgosalvez/github-actions-report-lcov@v3 with: coverage-files: | diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index e9027ac406..e56dfb4142 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -206,6 +206,32 @@ impl From for Features { } } +#[derive( + Debug, + Clone, + Copy, + PartialEq, + Eq, + strum::EnumString, + strum::AsRefStr, + strum::Display, + strum::VariantArray, +)] +enum CiOs { + #[strum(serialize = "windows-latest")] + Windows, + #[strum(serialize = "macos-latest")] + Macos, + #[strum(serialize = "ubuntu-latest")] + Ubuntu, +} + +impl CiOs { + fn is_main_os(&self) -> bool { + matches!(self, CiOs::Ubuntu) + } +} + #[derive(Debug, Clone, Parser)] struct App { #[clap(flatten)] @@ -297,7 +323,7 @@ impl App { os_string } - pub(crate) fn into_ci_row(self, os: String) -> CiMatrixRow { + pub(crate) fn into_ci_row(self, os: CiOs) -> CiMatrixRow { CiMatrixRow { command: self.clone().into_command_string().into_string().unwrap(), name: format!( @@ -310,7 +336,8 @@ impl App { self.global_args.features.to_string() } ), - os, + os: os.to_string(), + generates_coverage: self.global_args.coverage, } } } @@ -494,6 +521,8 @@ struct CiMatrixRow { name: String, /// The os to run this on os: String, + /// If this run produces lcov files + generates_coverage: bool, } impl Xtasks { @@ -520,27 +549,26 @@ impl Xtasks { let mut output = Self::ci_matrix(app_settings)?; output.sort_by(|e1, e2| e1.subcmd.cmp(&e2.subcmd)); let mut rows = Vec::default(); - for os in &["ubuntu-latest", "macos-latest", "windows-latest"] { + for os in ::VARIANTS { for row in output.iter() { - let is_main_os = os == &"ubuntu-latest"; let step_should_run_on_main_os = matches!(row.subcmd, Xtasks::Build | Xtasks::Docs { .. }); let is_coverage_step = row.global_args.coverage; - if !is_main_os && step_should_run_on_main_os { + if !os.is_main_os() && step_should_run_on_main_os { continue; } // we only need one source of coverage + windows is slow with this setting - let row = if !is_main_os && is_coverage_step { + let row = if !os.is_main_os() && is_coverage_step { let new_args = row.global_args.clone().without_coverage(); App { global_args: new_args, ..row.clone() } - .into_ci_row(os.to_string()) + .into_ci_row(*os) } else { - row.clone().into_ci_row(os.to_string()) + row.clone().into_ci_row(*os) }; rows.push(row); From bd7702973baf57356368244a790f6215f2c1cb43 Mon Sep 17 00:00:00 2001 From: makspll Date: Wed, 15 Jan 2025 23:41:35 +0000 Subject: [PATCH 20/34] improve comments slightly, and make global contexts work --- .../bevy_mod_scripting_core/src/commands.rs | 498 +++++++++++++++--- crates/bevy_mod_scripting_core/src/context.rs | 51 +- crates/bevy_mod_scripting_core/src/handler.rs | 2 +- crates/bevy_mod_scripting_core/src/lib.rs | 4 +- .../Summary/customizing-script-contexts.md | 4 +- 5 files changed, 473 insertions(+), 86 deletions(-) diff --git a/crates/bevy_mod_scripting_core/src/commands.rs b/crates/bevy_mod_scripting_core/src/commands.rs index 8e56ab3eab..2ce7dd809d 100644 --- a/crates/bevy_mod_scripting_core/src/commands.rs +++ b/crates/bevy_mod_scripting_core/src/commands.rs @@ -2,7 +2,7 @@ use crate::{ asset::ScriptAsset, context::{ContextLoadingSettings, ScriptContexts}, event::{IntoCallbackLabel, OnScriptLoaded, OnScriptUnloaded}, - handler::{handle_script_errors, CallbackSettings}, + handler::{handle_script_errors, CallbackSettings, HandlerFn}, runtime::RuntimeContainer, script::{Script, ScriptId, Scripts}, IntoScriptPluginParams, @@ -110,6 +110,156 @@ impl CreateOrUpdateScript

{ _ph: std::marker::PhantomData, } } + + fn run_on_load_callback( + &self, + settings: &ContextLoadingSettings

, + runtime: &mut RuntimeContainer

, + runner: HandlerFn

, + world: &mut bevy::prelude::World, + ctxt: &mut

::C, + ) { + match (runner)( + vec![], + bevy::ecs::entity::Entity::from_raw(0), + &self.id, + &OnScriptLoaded::into_callback_label(), + ctxt, + &settings.context_pre_handling_initializers, + &mut runtime.runtime, + world, + ) { + Ok(_) => {} + Err(e) => { + handle_script_errors( + world, + [e.with_context(format!( + "{}: Running initialization hook for script with id: {}", + P::LANGUAGE, + self.id + ))] + .into_iter(), + ); + } + } + } + + fn reload_context( + &self, + world: &mut bevy::prelude::World, + settings: &ContextLoadingSettings

, + runtime: &mut RuntimeContainer

, + builder: &crate::context::ContextBuilder

, + log_context: String, + previous_context: &mut

::C, + ) -> bool { + match (builder.reload)( + &self.id, + &self.content, + previous_context, + &settings.context_initializers, + &settings.context_pre_handling_initializers, + world, + &mut runtime.runtime, + ) { + Ok(_) => {} + Err(e) => { + handle_script_errors(world, [e.with_context(log_context)].into_iter()); + return false; + } + }; + true + } + + #[inline(always)] + fn execute( + self, + world: &mut bevy::prelude::World, + settings: &ContextLoadingSettings

, + contexts: &mut ScriptContexts

, + runtime: &mut RuntimeContainer

, + scripts: &mut Scripts, + assigner: crate::context::ContextAssigner

, + builder: crate::context::ContextBuilder

, + runner: HandlerFn

, + previous_context_id: Option, + ) { + match previous_context_id { + Some(previous_context_id) => { + if let Some(previous_context) = contexts.get_mut(previous_context_id) { + let log_context = format!("{}: Reloading script: {}.", P::LANGUAGE, self.id); + bevy::log::info!("{}", log_context); + if !self.reload_context( + world, + settings, + runtime, + &builder, + log_context, + previous_context, + ) { + return; + } + self.run_on_load_callback(settings, runtime, runner, world, previous_context); + } else { + bevy::log::error!("{}: Could not find previous context with id: {}. Could not reload script: {}. Someone deleted the context.", P::LANGUAGE, previous_context_id, self.id); + } + } + None => { + let log_context = format!("{}: Loading script: {}", P::LANGUAGE, self.id); + + let new_context_id = (assigner.assign)(&self.id, &self.content, contexts) + .unwrap_or_else(|| contexts.allocate_id()); + if let Some(existing_context) = contexts.get_mut(new_context_id) { + // this can happen if we're sharing contexts between scripts + if !self.reload_context( + world, + settings, + runtime, + &builder, + log_context, + existing_context, + ) { + return; + } + + self.run_on_load_callback(settings, runtime, runner, world, existing_context); + } else { + // load new context + bevy::log::info!("{}", log_context); + let ctxt = (builder.load)( + &self.id, + &self.content, + &settings.context_initializers, + &settings.context_pre_handling_initializers, + world, + &mut runtime.runtime, + ); + let mut ctxt = match ctxt { + Ok(ctxt) => ctxt, + Err(e) => { + handle_script_errors(world, [e.with_context(log_context)].into_iter()); + return; + } + }; + + self.run_on_load_callback(settings, runtime, runner, world, &mut ctxt); + + if contexts.insert_with_id(new_context_id, ctxt).is_some() { + bevy::log::warn!("{}: Context with id {} was not expected to exist. Overwriting it with a new context. This might happen if a script is not completely removed.", P::LANGUAGE, new_context_id); + } + } + + scripts.scripts.insert( + self.id.clone(), + Script { + id: self.id, + asset: self.asset, + context_id: new_context_id, + }, + ); + } + } + } } impl Command for CreateOrUpdateScript

{ @@ -126,6 +276,9 @@ impl Command for CreateOrUpdateScript

{ let mut runtime = world .remove_non_send_resource::>() .expect("No RuntimeContainer resource found. Was the plugin initialized correctly?"); + let mut scripts = world + .remove_resource::() + .expect("No Scripts resource found. Was the plugin initialized correctly?"); let runner = world.get_resource::>().unwrap(); // assign context @@ -133,86 +286,289 @@ impl Command for CreateOrUpdateScript

{ let builder = settings.loader.clone(); let runner = runner.callback_handler; - world.resource_scope(|world, mut scripts: Mut| { + let script = scripts.scripts.get(&self.id); + let previous_context_id = script.as_ref().map(|s| s.context_id); + debug!( + "{}: CreateOrUpdateScript command applying (script_id: {}, previous_context_id: {:?})", + P::LANGUAGE, + self.id, + previous_context_id + ); - // check if script already exists + // closure to prevent returns from re-inserting resources + self.execute( + world, + &settings, + &mut contexts, + &mut runtime, + &mut scripts, + assigner, + builder, + runner, + previous_context_id, + ); - let mut script = scripts.scripts.get_mut(&self.id); - let previous_context_id = script.as_ref().map(|s| s.context_id); - debug!( - "{}: CreateOrUpdateScript command applying (script_id: {}, previous_context_id: {:?})", - P::LANGUAGE, - self.id, previous_context_id - ); + world.insert_resource(scripts); + world.insert_resource(settings); + world.insert_non_send_resource(runtime); + world.insert_non_send_resource(contexts); + } +} - // If None assign new context ID, otherwise assign the old one - // If re-loading and different from the previous one, the old one will be removed - let current_context_id = (assigner.assign)(script.as_deref(), &self.id, &self.content, &mut contexts); +#[cfg(test)] +mod test { + use bevy::{ + app::App, + prelude::{Entity, World}, + }; - debug!("{}: New context assigned?: {:?}", P::LANGUAGE, current_context_id.is_none() || current_context_id != previous_context_id); + use crate::{ + asset::Language, + bindings::script_value::ScriptValue, + context::{ContextAssigner, ContextBuilder}, + }; - let current_context_id = if let Some(id) = current_context_id { - // reload existing context - id - } else { - let log_context = format!("{}: Loading script: {}", P::LANGUAGE, self.id); - bevy::log::info!("{}", log_context); - let ctxt = (builder.load)(&self.id, &self.content, &settings.context_initializers, &settings.context_pre_handling_initializers, world, &mut runtime.runtime); - match ctxt { - Ok(ctxt) => contexts.insert(ctxt), - Err(e) => { - handle_script_errors(world, [e.with_context(log_context)].into_iter()); - return; + use super::*; + + fn setup_app() -> App { + // setup all the resources necessary + let mut app = App::new(); + + app.insert_resource(ContextLoadingSettings:: { + loader: ContextBuilder { + load: |name, c, init, pre_run_init, _, _| { + let mut context = String::from_utf8_lossy(c).into(); + for init in init { + init(name, &mut context)?; } - } - }; + for init in pre_run_init { + init(name, Entity::from_raw(0), &mut context)?; + } + Ok(context) + }, + reload: |name, new, existing, init, pre_run_init, _, _| { + *existing = String::from_utf8_lossy(new).into(); + for init in init { + init(name, existing)?; + } + for init in pre_run_init { + init(name, Entity::from_raw(0), existing)?; + } + Ok(()) + }, + }, + assigner: Default::default(), + context_initializers: vec![|_, c| { + c.push_str(" initialized"); + Ok(()) + }], + context_pre_handling_initializers: vec![|_, _, c| { + c.push_str(" pre-handling-initialized"); + Ok(()) + }], + }) + .insert_non_send_resource(ScriptContexts:: { + contexts: Default::default(), + }) + .insert_non_send_resource(RuntimeContainer:: { + runtime: "Runtime".to_string(), + }) + .insert_resource(CallbackSettings:: { + callback_handler: |_, _, _, callback, c, _, _, _| { + c.push_str(format!(" callback-ran-{}", callback).as_str()); + Ok(ScriptValue::Unit) + }, + }) + .insert_resource(Scripts { + scripts: Default::default(), + }); + app + } - if let Some(previous) = previous_context_id { - if let Some(previous_context_id) = contexts.get_mut(previous) { - let log_context = format!("{}: Reloading script: {}.", P::LANGUAGE, self.id); - bevy::log::info!("{}", log_context); - match (builder.reload)(&self.id, &self.content, previous_context_id, &settings.context_initializers, &settings.context_pre_handling_initializers, world, &mut runtime.runtime) { - Ok(_) => {}, - Err(e) => { - handle_script_errors(world, [e.with_context(log_context)].into_iter()); - return; - } - }; - } else { - bevy::log::error!("{}: Could not find previous context with id: {}. Could not reload script: {}", P::LANGUAGE, previous, self.id); - } + struct DummyPlugin; - if previous != current_context_id { - bevy::log::info!("{}: Unloading script with id: {}. As it was assigned to a new context", P::LANGUAGE, self.id); - script.as_deref_mut().unwrap().context_id = current_context_id; - (assigner.remove)(previous, script.unwrap(), &mut contexts); - } - } + impl IntoScriptPluginParams for DummyPlugin { + type R = String; + type C = String; + const LANGUAGE: Language = Language::Unknown; - if let Some(context) = contexts.get_mut(current_context_id) { - match (runner)(vec![], bevy::ecs::entity::Entity::from_raw(0), &self.id, &OnScriptLoaded::into_callback_label(), context, &settings.context_pre_handling_initializers, &mut runtime.runtime, world) { - Ok(_) => {}, - Err(e) => { - handle_script_errors(world, [e.with_context(format!("{}: Running initialization hook for script with id: {}", P::LANGUAGE, self.id))].into_iter()); - }, - } + fn build_runtime() -> Self::R { + "Runtime".to_string() + } + } - // we only want to insert the script if a context is present, otherwise something went wrong - scripts.scripts.insert( - self.id.clone(), - Script { - id: self.id, - asset: self.asset, - context_id: current_context_id, - }, - ); - } else { - bevy::log::error!("{}: Context loading failed for script: {}. Did not run on_script_loaded hook",P::LANGUAGE ,self.id); - } - }); - world.insert_resource(settings); - world.insert_non_send_resource(runtime); - world.insert_non_send_resource(contexts); + fn assert_context_and_script(world: &World, id: &str, context: &str) { + let contexts = world + .get_non_send_resource::>() + .unwrap(); + let scripts = world.get_resource::().unwrap(); + + let script = scripts.scripts.get(id).expect("Script not found"); + + assert_eq!(id, script.id); + let found_context = contexts + .contexts + .get(&script.context_id) + .expect("Context not found"); + + assert_eq!(found_context, context); + } + + #[test] + fn test_commands_with_default_assigner() { + let mut app = setup_app(); + + let world = app.world_mut(); + let content = "content".as_bytes().to_vec().into_boxed_slice(); + let command = CreateOrUpdateScript::::new("script".into(), content, None); + command.apply(world); + + // check script + assert_context_and_script( + world, + "script", + "content initialized pre-handling-initialized callback-ran-on_script_loaded", + ); + + // update the script + let content = "new content".as_bytes().to_vec().into_boxed_slice(); + let command = CreateOrUpdateScript::::new("script".into(), content, None); + command.apply(world); + + // check script + assert_context_and_script( + world, + "script", + "new content initialized pre-handling-initialized callback-ran-on_script_loaded", + ); + + // create second script + let content = "content2".as_bytes().to_vec().into_boxed_slice(); + let command = CreateOrUpdateScript::::new("script2".into(), content, None); + + command.apply(world); + + // check second script + + assert_context_and_script( + world, + "script2", + "content2 initialized pre-handling-initialized callback-ran-on_script_loaded", + ); + + // delete both scripts + let command = DeleteScript::::new("script".into()); + command.apply(world); + let command = DeleteScript::::new("script2".into()); + command.apply(world); + + // check that the scripts are gone + let scripts = world.get_resource::().unwrap(); + assert!(scripts.scripts.is_empty()); + + let contexts = world + .get_non_send_resource::>() + .unwrap(); + assert!(contexts.contexts.is_empty()); + } + + #[test] + fn test_commands_with_global_assigner() { + // setup all the resources necessary + let mut app = setup_app(); + + let mut settings = app + .world_mut() + .get_resource_mut::>() + .unwrap(); + + settings.assigner = ContextAssigner::new_global_context_assigner(); + + // create a script + let content = "content".as_bytes().to_vec().into_boxed_slice(); + let command = CreateOrUpdateScript::::new("script".into(), content, None); + + command.apply(app.world_mut()); + + // check script + assert_context_and_script( + app.world(), + "script", + "content initialized pre-handling-initialized callback-ran-on_script_loaded", + ); + + // update the script + + let content = "new content".as_bytes().to_vec().into_boxed_slice(); + let command = CreateOrUpdateScript::::new("script".into(), content, None); + + command.apply(app.world_mut()); + + // check script + + assert_context_and_script( + app.world(), + "script", + "new content initialized pre-handling-initialized callback-ran-on_script_loaded", + ); + + // create second script + + let content = "content2".as_bytes().to_vec().into_boxed_slice(); + let command = CreateOrUpdateScript::::new("script2".into(), content, None); + + command.apply(app.world_mut()); + + // check both scripts have the new context + + assert_context_and_script( + app.world(), + "script", + "content2 initialized pre-handling-initialized callback-ran-on_script_loaded", + ); + + assert_context_and_script( + app.world(), + "script2", + "content2 initialized pre-handling-initialized callback-ran-on_script_loaded", + ); + + // check one context exists only + let context = app + .world() + .get_non_send_resource::>() + .unwrap(); + assert!(context.contexts.len() == 1); + + // delete first script + let command = DeleteScript::::new("script".into()); + + command.apply(app.world_mut()); + + // check second script still has the context, and on unload was called + assert_context_and_script( + app.world(), + "script2", + "content2 initialized pre-handling-initialized callback-ran-on_script_loaded callback-ran-on_script_unloaded", + ); + + // delete second script + + let command = DeleteScript::::new("script2".into()); + + command.apply(app.world_mut()); + + // check that the scripts are gone, but context is still there + + let scripts = app.world().get_resource::().unwrap(); + assert!(scripts.scripts.is_empty()); + + let contexts = app + .world() + .get_non_send_resource::>() + .unwrap(); + + assert!(contexts.contexts.len() == 1); } } diff --git a/crates/bevy_mod_scripting_core/src/context.rs b/crates/bevy_mod_scripting_core/src/context.rs index 664a2fcc86..8c338859fa 100644 --- a/crates/bevy_mod_scripting_core/src/context.rs +++ b/crates/bevy_mod_scripting_core/src/context.rs @@ -34,6 +34,10 @@ impl ScriptContexts

{ id } + pub fn insert_with_id(&mut self, id: ContextId, ctxt: P::C) -> Option { + self.contexts.insert(id, ctxt) + } + /// Allocate new context id without inserting a context pub fn allocate_id(&self) -> ContextId { CONTEXT_ID_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed) @@ -50,12 +54,17 @@ impl ScriptContexts

{ pub fn get_mut(&mut self, id: ContextId) -> Option<&mut P::C> { self.contexts.get_mut(&id) } + + pub fn contains(&self, id: ContextId) -> bool { + self.contexts.contains_key(&id) + } } -/// Initializer run once after creating a context but before executing it for the first time +/// Initializer run once after creating a context but before executing it for the first time as well as after re-loading the script pub type ContextInitializer

= fn(&str, &mut

::C) -> Result<(), ScriptError>; -/// Initializer run every time before executing or loading a script + +/// Initializer run every time before executing or loading/re-loading a script pub type ContextPreHandlingInitializer

= fn(&str, Entity, &mut

::C) -> Result<(), ScriptError>; @@ -115,30 +124,52 @@ impl Clone for ContextBuilder

{ /// A strategy for assigning contexts to new and existing but re-loaded scripts as well as for managing old contexts pub struct ContextAssigner { - /// Assign a context to the script, if script is `None`, this is a new script, otherwise it is an existing script with a context inside `contexts`. - /// Returning None means the script should be assigned a new context + /// Assign a context to the script. + /// The assigner can either return `Some(id)` or `None`. + /// Returning None will request the processor to assign a new context id to assign to this script. + /// + /// Regardless, whether a script gets a new context id or not, the processor will check if the given context exists. + /// If it does not exist, it will create a new context and assign it to the script. + /// If it does exist, it will NOT create a new context, but assign the existing one to the script, and re-load the context. + /// + /// This function is only called once for each script, when it is loaded for the first time. pub assign: fn( - old_script: Option<&Script>, script_id: &ScriptId, new_content: &[u8], contexts: &ScriptContexts

, ) -> Option, /// Handle the removal of the script, if any clean up in contexts is necessary perform it here. - /// This will also be called, when a script is assigned a contextId on reload different from the previous one - /// the context_id in that case will be the old context_id and the one stored in the script will be the old one + /// + /// If you do not clean up the context here, it will stay in the context map! pub remove: fn(context_id: ContextId, script: &Script, contexts: &mut ScriptContexts

), } -impl Default for ContextAssigner

{ - fn default() -> Self { +impl ContextAssigner

{ + /// Create an assigner which re-uses a single global context for all scripts, only use if you know what you're doing. + /// Will not perform any clean up on removal. + pub fn new_global_context_assigner() -> Self { + Self { + assign: |_, _, _| Some(0), // always use the same id in rotation + remove: |_, _, _| {}, // do nothing + } + } + + /// Create an assigner which assigns a new context to each script. This is the default strategy. + pub fn new_individual_context_assigner() -> Self { Self { - assign: |old, _, _, _| old.map(|s| s.context_id), + assign: |_, _, _| None, remove: |id, _, c| _ = c.remove(id), } } } +impl Default for ContextAssigner

{ + fn default() -> Self { + Self::new_individual_context_assigner() + } +} + impl Clone for ContextAssigner

{ fn clone(&self) -> Self { Self { diff --git a/crates/bevy_mod_scripting_core/src/handler.rs b/crates/bevy_mod_scripting_core/src/handler.rs index b395f432e3..6145d4345b 100644 --- a/crates/bevy_mod_scripting_core/src/handler.rs +++ b/crates/bevy_mod_scripting_core/src/handler.rs @@ -248,7 +248,7 @@ mod test { reload: |_, _, _, _, _, _, _| todo!(), }, assigner: ContextAssigner { - assign: |_, _, _, _| todo!(), + assign: |_, _, _| todo!(), remove: |_, _, _| todo!(), }, context_initializers: vec![], diff --git a/crates/bevy_mod_scripting_core/src/lib.rs b/crates/bevy_mod_scripting_core/src/lib.rs index fa0c4a50ea..21845aa857 100644 --- a/crates/bevy_mod_scripting_core/src/lib.rs +++ b/crates/bevy_mod_scripting_core/src/lib.rs @@ -107,7 +107,7 @@ impl Plugin for ScriptingPlugin

{ impl ScriptingPlugin

{ /// Adds a context initializer to the plugin /// - /// Initializers will be run every time a context is loaded or re-loaded + /// Initializers will be run every time a context is loaded or re-loaded and before any events are handled pub fn add_context_initializer(&mut self, initializer: ContextInitializer

) -> &mut Self { self.context_initializers.push(initializer); self @@ -115,7 +115,7 @@ impl ScriptingPlugin

{ /// Adds a context pre-handling initializer to the plugin. /// - /// Initializers will be run every time before handling events. + /// Initializers will be run every time before handling events and after the context is loaded or re-loaded. pub fn add_context_pre_handling_initializer( &mut self, initializer: ContextPreHandlingInitializer

, diff --git a/docs/src/Summary/customizing-script-contexts.md b/docs/src/Summary/customizing-script-contexts.md index f06b960799..2419989fbb 100644 --- a/docs/src/Summary/customizing-script-contexts.md +++ b/docs/src/Summary/customizing-script-contexts.md @@ -22,11 +22,11 @@ let plugin = LuaScriptingPlugin::default().add_context_initializer(|script_id: & app.add_plugins(plugin) ``` -The above will run every time the script is loaded or re-loaded. +The above will run every time the script is loaded or re-loaded and before it handles any callbacks. ## Context Pre Handling Initializers -If you want to customize your context before every time it's about to handle events, you can use `Context Pre Handling Initializers`: +If you want to customize your context before every time it's about to handle events (and when it's loaded + reloaded), you can use `Context Pre Handling Initializers`: ```rust,ignore let plugin = LuaScriptingPlugin::default().add_context_pre_handling_initializer(|script_id: &str, entity: Entity, context: &mut Lua| { let globals = context.globals(); From 561583701f12b5cdd08c11cc2be9b0b3e00fcac5 Mon Sep 17 00:00:00 2001 From: makspll Date: Thu, 16 Jan 2025 23:34:22 +0000 Subject: [PATCH 21/34] refactor asset systems, make use of internal script asset events --- crates/bevy_mod_scripting_core/src/asset.rs | 160 +++++++++--------- .../bevy_mod_scripting_core/src/commands.rs | 5 +- crates/bevy_mod_scripting_core/src/lib.rs | 15 +- 3 files changed, 95 insertions(+), 85 deletions(-) diff --git a/crates/bevy_mod_scripting_core/src/asset.rs b/crates/bevy_mod_scripting_core/src/asset.rs index fb4830602a..ff0e307d79 100644 --- a/crates/bevy_mod_scripting_core/src/asset.rs +++ b/crates/bevy_mod_scripting_core/src/asset.rs @@ -8,8 +8,11 @@ use bevy::{ app::{App, PreUpdate}, asset::{Asset, AssetEvent, AssetId, AssetLoader, Assets}, ecs::system::Resource, - log::{error, info, trace}, - prelude::{Commands, EventReader, IntoSystemConfigs, IntoSystemSetConfigs, Res, ResMut}, + log::{debug, error, info, trace}, + prelude::{ + Commands, Event, EventReader, EventWriter, IntoSystemConfigs, IntoSystemSetConfigs, Res, + ResMut, + }, reflect::TypePath, utils::HashMap, }; @@ -49,6 +52,13 @@ pub struct ScriptAsset { pub asset_path: PathBuf, } +#[derive(Event, Debug, Clone)] +pub(crate) enum ScriptAssetEvent { + Added(ScriptMetadata), + Removed(ScriptMetadata), + Modified(ScriptMetadata), +} + #[derive(Default)] pub struct ScriptAssetLoader { /// The file extensions this loader should handle @@ -140,6 +150,7 @@ pub struct ScriptMetadataStore { #[derive(Debug, Clone, PartialEq, Eq)] pub struct ScriptMetadata { + pub asset_id: AssetId, pub script_id: ScriptId, pub language: Language, } @@ -157,78 +168,94 @@ impl ScriptMetadataStore { pub fn remove(&mut self, id: AssetId) -> Option { self.map.remove(&id) } + + pub fn contains(&self, id: AssetId) -> bool { + self.map.contains_key(&id) + } } -/// Listens to `AssetEvent::Added` events and populates the script metadata store -pub fn insert_script_metadata( +/// Converts incoming asset events, into internal script asset events, also loads and inserts metadata for newly added scripts +pub(crate) fn dispatch_script_asset_events( mut events: EventReader>, - script_assets: Res>, - mut asset_path_map: ResMut, + mut script_asset_events: EventWriter, + assets: Res>, + mut metadata_store: ResMut, settings: Res, ) { for event in events.read() { - if let AssetEvent::LoadedWithDependencies { id } = event { - let asset = script_assets.get(*id); - if let Some(asset) = asset { - let path = &asset.asset_path; - let converter = settings.script_id_mapper.map; - let script_id = converter(path); - - let language = settings.select_script_language(path); - let metadata = ScriptMetadata { - script_id, - language, - }; - info!("Populating script metadata for script: {:?}:", metadata); - asset_path_map.insert(*id, metadata); - } else { - error!("A script was added but it's asset was not found, failed to compute metadata. This script will not be loaded. {}", id); + match event { + AssetEvent::LoadedWithDependencies { id } | AssetEvent::Added { id } => { + // these can occur multiple times, we only send one added event though + if !metadata_store.contains(*id) { + let asset = assets.get(*id); + if let Some(asset) = asset { + let path = &asset.asset_path; + let converter = settings.script_id_mapper.map; + let script_id = converter(path); + + let language = settings.select_script_language(path); + let metadata = ScriptMetadata { + asset_id: *id, + script_id, + language, + }; + debug!("Script loaded, populating metadata: {:?}:", metadata); + script_asset_events.send(ScriptAssetEvent::Added(metadata.clone())); + metadata_store.insert(*id, metadata); + } else { + error!("A script was added but it's asset was not found, failed to compute metadata. This script will not be loaded. {}", id); + } + } + } + AssetEvent::Removed { id } => { + if let Some(metadata) = metadata_store.get(*id) { + debug!("Script removed: {:?}", metadata); + script_asset_events.send(ScriptAssetEvent::Removed(metadata.clone())); + } else { + error!("Script metadata not found for removed script asset: {}. Cannot properly clean up script", id); + } } + AssetEvent::Modified { id } => { + if let Some(metadata) = metadata_store.get(*id) { + debug!("Script modified: {:?}", metadata); + script_asset_events.send(ScriptAssetEvent::Modified(metadata.clone())); + } else { + error!("Script metadata not found for modified script asset: {}. Cannot properly update script", id); + } + } + _ => {} } } } -/// Listens to [`AssetEvent::Removed`] events and removes the corresponding script metadata -pub fn remove_script_metadata( - mut events: EventReader>, +/// Listens to [`ScriptAssetEvent::Removed`] events and removes the corresponding script metadata +pub(crate) fn remove_script_metadata( + mut events: EventReader, mut asset_path_map: ResMut, ) { for event in events.read() { - if let AssetEvent::Removed { id } = event { - let previous = asset_path_map.remove(*id); + if let ScriptAssetEvent::Removed(metadata) = event { + let previous = asset_path_map.remove(metadata.asset_id); if let Some(previous) = previous { - info!("Removed script metadata for removed script: {:?}", previous); + debug!("Removed script metadata: {:?}", previous); } } } } -/// Listens to [`AssetEvent`] events and dispatches [`CreateOrUpdateScript`] and [`DeleteScript`] commands accordingly. +/// Listens to [`ScriptAssetEvent`] events and dispatches [`CreateOrUpdateScript`] and [`DeleteScript`] commands accordingly. /// /// Allows for hot-reloading of scripts. -pub fn sync_script_data( - mut events: EventReader>, +pub(crate) fn sync_script_data( + mut events: EventReader, script_assets: Res>, - script_metadata: Res, mut commands: Commands, ) { for event in events.read() { trace!("{}: Received script asset event: {:?}", P::LANGUAGE, event); match event { // emitted when a new script asset is loaded for the first time - AssetEvent::LoadedWithDependencies { id } | AssetEvent::Modified { id } => { - let metadata = match script_metadata.get(*id) { - Some(m) => m, - None => { - error!( - "{}: Script metadata not found for script asset with id: {}. Cannot load script.", - P::LANGUAGE, - id - ); - continue; - } - }; - + ScriptAssetEvent::Added(metadata) | ScriptAssetEvent::Modified(metadata) => { if metadata.language != P::LANGUAGE { trace!( "{}: Script asset with id: {} is for a different langauge than this sync system. Skipping.", @@ -238,14 +265,9 @@ pub fn sync_script_data( continue; } - info!( - "{}: Dispatching Creation/Modification command for script: {:?}. Asset Id: {}", - P::LANGUAGE, - metadata, - id - ); + info!("{}: Loading Script: {:?}", P::LANGUAGE, metadata.script_id,); - if let Some(asset) = script_assets.get(*id) { + if let Some(asset) = script_assets.get(metadata.asset_id) { commands.queue(CreateOrUpdateScript::

::new( metadata.script_id.clone(), asset.content.clone(), @@ -253,28 +275,10 @@ pub fn sync_script_data( )); } } - AssetEvent::Removed { id } => { - let metadata = match script_metadata.get(*id) { - Some(m) => m, - None => { - error!( - "{}: Script metadata not found for script asset with id: {}. Cannot delete script.", - P::LANGUAGE, - id - ); - return; - } - }; - - info!( - "{}: Dispatching Deletion command for script: {:?}. Asset Id: {}", - P::LANGUAGE, - metadata, - id - ); + ScriptAssetEvent::Removed(metadata) => { + info!("{}: Deleting Script: {:?}", P::LANGUAGE, metadata.script_id,); commands.queue(DeleteScript::

::new(metadata.script_id.clone())); } - _ => return, }; } } @@ -286,21 +290,22 @@ pub(crate) fn configure_asset_systems(app: &mut App) -> &mut App { app.add_systems( PreUpdate, ( - insert_script_metadata.in_set(ScriptingSystemSet::ScriptMetadataInsertion), + dispatch_script_asset_events.in_set(ScriptingSystemSet::ScriptAssetDispatch), remove_script_metadata.in_set(ScriptingSystemSet::ScriptMetadataRemoval), ), ) .configure_sets( PreUpdate, ( - ScriptingSystemSet::ScriptMetadataInsertion.after(bevy::asset::TrackAssets), + ScriptingSystemSet::ScriptAssetDispatch.after(bevy::asset::TrackAssets), ScriptingSystemSet::ScriptCommandDispatch - .after(ScriptingSystemSet::ScriptMetadataInsertion) + .after(ScriptingSystemSet::ScriptAssetDispatch) .before(ScriptingSystemSet::ScriptMetadataRemoval), ), ) .init_resource::() - .init_resource::(); + .init_resource::() + .add_event::(); app } @@ -455,6 +460,7 @@ mod tests { let mut store = ScriptMetadataStore::default(); let id = AssetId::invalid(); let meta = ScriptMetadata { + asset_id: AssetId::invalid(), script_id: "test".into(), language: Language::Lua, }; @@ -544,7 +550,7 @@ mod tests { } #[test] - fn test_asset_metadata_insert_remove_systems() { + fn test_asset_metadata_systems() { // test metadata flow let mut app = init_loader_test(ScriptAssetLoader { extensions: &[], diff --git a/crates/bevy_mod_scripting_core/src/commands.rs b/crates/bevy_mod_scripting_core/src/commands.rs index 2ce7dd809d..d9357353ca 100644 --- a/crates/bevy_mod_scripting_core/src/commands.rs +++ b/crates/bevy_mod_scripting_core/src/commands.rs @@ -144,6 +144,7 @@ impl CreateOrUpdateScript

{ } } + #[inline(always)] fn reload_context( &self, world: &mut bevy::prelude::World, @@ -188,7 +189,7 @@ impl CreateOrUpdateScript

{ Some(previous_context_id) => { if let Some(previous_context) = contexts.get_mut(previous_context_id) { let log_context = format!("{}: Reloading script: {}.", P::LANGUAGE, self.id); - bevy::log::info!("{}", log_context); + bevy::log::debug!("{}", log_context); if !self.reload_context( world, settings, @@ -225,7 +226,7 @@ impl CreateOrUpdateScript

{ self.run_on_load_callback(settings, runtime, runner, world, existing_context); } else { // load new context - bevy::log::info!("{}", log_context); + bevy::log::debug!("{}", log_context); let ctxt = (builder.load)( &self.id, &self.content, diff --git a/crates/bevy_mod_scripting_core/src/lib.rs b/crates/bevy_mod_scripting_core/src/lib.rs index 21845aa857..d7dcbd73ef 100644 --- a/crates/bevy_mod_scripting_core/src/lib.rs +++ b/crates/bevy_mod_scripting_core/src/lib.rs @@ -34,15 +34,18 @@ pub mod script; #[derive(SystemSet, Hash, Debug, Eq, PartialEq, Clone)] /// Labels for various BMS systems pub enum ScriptingSystemSet { - // Post Setup processes + /// Systems which handle the processing of asset events for script assets, and dispatching internal script asset events + ScriptAssetDispatch, + /// Systems which read incoming internal script asset events and produce script lifecycle commands + ScriptCommandDispatch, + /// Systems which read incoming script asset events and remove metadata for removed assets + ScriptMetadataRemoval, + + /// One time runtime initialization systems RuntimeInitialization, - // Post Update processes + /// Systems which handle the garbage collection of allocated values GarbageCollection, - - ScriptMetadataInsertion, - ScriptCommandDispatch, - ScriptMetadataRemoval, } /// Types which act like scripting plugins, by selecting a context and runtime From c050ba4c7a4b7d08bcb21179b75d7457b3ef0ac0 Mon Sep 17 00:00:00 2001 From: makspll Date: Thu, 16 Jan 2025 23:49:14 +0000 Subject: [PATCH 22/34] add simple error test --- crates/bevy_mod_scripting_core/src/error.rs | 371 ++++++++++++++------ 1 file changed, 254 insertions(+), 117 deletions(-) diff --git a/crates/bevy_mod_scripting_core/src/error.rs b/crates/bevy_mod_scripting_core/src/error.rs index 36776aa393..bb14ac623d 100644 --- a/crates/bevy_mod_scripting_core/src/error.rs +++ b/crates/bevy_mod_scripting_core/src/error.rs @@ -584,78 +584,220 @@ impl PartialEq for InteropErrorInner { } } +macro_rules! missing_function_error { + ($function_name:expr, $on:expr) => { + format!( + "Could not find function: {} for type: {}", + $function_name, $on + ) + }; +} + +macro_rules! unregistered_base { + ($base:expr) => { + format!("Unregistered base type: {}", $base) + }; +} + +macro_rules! cannot_claim_access { + ($base:expr, $location:expr) => { + format!( + "Cannot claim access to base type: {}. The base is already claimed by something else in a way which prevents safe access. Location: {}", + $base, $location + ) + }; +} + +macro_rules! impossible_conversion { + ($into:expr) => { + format!("Cannot convert to type: {}", $into) + }; +} + +macro_rules! type_mismatch { + ($expected:expr, $got:expr) => { + format!("Type mismatch, expected: {}, got: {}", $expected, $got) + }; +} + +macro_rules! string_type_mismatch { + ($expected:expr, $got:expr) => { + format!("Type mismatch, expected: {}, got: {}", $expected, $got) + }; +} + +macro_rules! could_not_downcast { + ($from:expr, $to:expr) => { + format!("Could not downcast from: {} to: {}", $from, $to) + }; +} + +macro_rules! garbage_collected_allocation { + ($reference:expr) => { + format!( + "Allocation was garbage collected. Could not access reference: {} as a result.", + $reference + ) + }; +} + +macro_rules! reflection_path_error { + ($error:expr, $reference:expr) => { + format!( + "Error while reflecting path: {} on reference: {}", + $error, $reference + ) + }; +} + +macro_rules! missing_type_data { + ($type_data:expr, $type_id:expr) => { + format!( + "Missing type data {} for type: {}. Did you register the type correctly?", + $type_data, $type_id + ) + }; +} + +macro_rules! failed_from_reflect { + ($type_id:expr, $reason:expr) => { + format!( + "Failed to convert from reflect for type: {} with reason: {}", + $type_id, $reason + ) + }; +} + +macro_rules! value_mismatch { + ($expected:expr, $got:expr) => { + format!("Value mismatch, expected: {}, got: {}", $expected, $got) + }; +} + +macro_rules! unsupported_operation { + ($operation:expr, $base:expr, $value:expr) => { + format!( + "Unsupported operation: {} on base: {} with value: {:?}", + $operation, $base, $value + ) + }; +} + +macro_rules! invalid_index { + ($value:expr, $reason:expr) => { + format!("Invalid index for value: {}: {}", $value, $reason) + }; +} + +macro_rules! missing_entity { + ($entity:expr) => { + format!("Missing or invalid entity: {}", $entity) + }; +} + +macro_rules! invalid_component { + ($component_id:expr) => { + format!("Invalid component: {:?}", $component_id) + }; +} + +macro_rules! function_interop_error { + ($display_name:expr, $opt_on:expr, $error:expr) => { + format!( + "Error in function {} {}: {}", + $display_name, $opt_on, $error + ) + }; +} + +macro_rules! function_arg_conversion_error { + ($argument:expr, $error:expr) => { + format!("Error converting argument {}: {}", $argument, $error) + }; +} + +macro_rules! function_call_error { + ($inner:expr) => { + format!("Error in function call: {}", $inner) + }; +} + +macro_rules! better_conversion_exists { + ($context:expr) => { + format!("Unfinished conversion in context of: {}. A better conversion exists but caller didn't handle the case.", $context) + }; +} + +macro_rules! length_mismatch { + ($expected:expr, $got:expr) => { + format!( + "Array/List Length mismatch, expected: {}, got: {}", + $expected, $got + ) + }; +} + +macro_rules! invalid_access_count { + ($expected:expr, $count:expr, $context:expr) => { + format!( + "Invalid access count, expected: {}, got: {}. {}", + $expected, $count, $context + ) + }; +} + impl DisplayWithWorld for InteropErrorInner { fn display_with_world(&self, world: crate::bindings::WorldGuard) -> String { match self { InteropErrorInner::MissingFunctionError { on, function_name } => { - format!( - "Could not find function: {} for type: {}", - function_name, - on.display_with_world(world) - ) + missing_function_error!(function_name, on.display_with_world(world)) }, InteropErrorInner::UnregisteredBase { base } => { - format!("Unregistered base type: {}", base.display_with_world(world)) + unregistered_base!(base.display_with_world(world)) } InteropErrorInner::CannotClaimAccess { base, location } => { - format!( - "Cannot claim access to base type: {}. The base is already claimed by something else in a way which prevents safe access. Location: {}", - base.display_with_world(world), - location.display_location() - ) + cannot_claim_access!(base.display_with_world(world), location.display_location()) } InteropErrorInner::ImpossibleConversion { into } => { - format!("Cannot convert to type: {}", into.display_with_world(world)) + impossible_conversion!(into.display_with_world(world)) } InteropErrorInner::TypeMismatch { expected, got } => { - format!( - "Type mismatch, expected: {}, got: {}", + type_mismatch!( expected.display_with_world(world.clone()), got.map(|t| t.display_with_world(world)) .unwrap_or("None".to_owned()) ) } InteropErrorInner::StringTypeMismatch { expected, got } => { - format!( - "Type mismatch, expected: {}, got: {}", + string_type_mismatch!( expected, got.map(|t| t.display_with_world(world)) .unwrap_or("None".to_owned()) ) } InteropErrorInner::CouldNotDowncast { from, to } => { - format!( - "Could not downcast from: {} to: {}", + could_not_downcast!( from.display_with_world(world.clone()), to.display_with_world(world) ) } InteropErrorInner::GarbageCollectedAllocation { reference } => { - format!( - "Allocation was garbage collected. Could not access reference: {} as a result.", - reference.display_with_world(world), - ) + garbage_collected_allocation!(reference.display_with_world(world)) } InteropErrorInner::ReflectionPathError { error, reference } => { - format!( - "Error while reflecting path: {} on reference: {}", + reflection_path_error!( error, reference .as_ref() .map(|r| r.display_with_world(world)) - .unwrap_or("None".to_owned()), + .unwrap_or("None".to_owned()) ) } InteropErrorInner::MissingTypeData { type_id, type_data } => { - format!( - "Missing type data {} for type: {}. Did you register the type correctly?", - type_data, - type_id.display_with_world(world), - ) + missing_type_data!(type_data, type_id.display_with_world(world)) } InteropErrorInner::FailedFromReflect { type_id, reason } => { - format!( - "Failed to convert from reflect for type: {} with reason: {}", + failed_from_reflect!( type_id .map(|t| t.display_with_world(world)) .unwrap_or("None".to_owned()), @@ -663,8 +805,7 @@ impl DisplayWithWorld for InteropErrorInner { ) } InteropErrorInner::ValueMismatch { expected, got } => { - format!( - "Value mismatch, expected: {}, got: {}", + value_mismatch!( expected.display_with_world(world.clone()), got.display_with_world(world) ) @@ -674,8 +815,7 @@ impl DisplayWithWorld for InteropErrorInner { value, operation, } => { - format!( - "Unsupported operation: {} on base: {} with value: {:?}", + unsupported_operation!( operation, base.map(|t| t.display_with_world(world)) .unwrap_or("None".to_owned()), @@ -683,17 +823,13 @@ impl DisplayWithWorld for InteropErrorInner { ) } InteropErrorInner::InvalidIndex { value, reason } => { - format!( - "Invalid index for value: {}: {}", - value.display_with_world(world), - reason - ) + invalid_index!(value.display_with_world(world), reason) } InteropErrorInner::MissingEntity { entity } => { - format!("Missing or invalid entity: {}", entity) + missing_entity!(entity) } InteropErrorInner::InvalidComponent { component_id } => { - format!("Invalid component: {:?}", component_id) + invalid_component!(component_id) } InteropErrorInner::StaleWorldAccess => { "Stale world access. The world has been dropped and a script tried to access it. Do not try to store or copy the world." @@ -712,32 +848,23 @@ impl DisplayWithWorld for InteropErrorInner { } else { function_name.as_str() }; - format!( - "Error in function {} {}: {}", - display_name, - opt_on, - error.display_with_world(world), - ) + function_interop_error!(display_name, opt_on, error.display_with_world(world)) }, InteropErrorInner::FunctionArgConversionError { argument, error } => { - format!( - "Error converting argument {}: {}", - argument, - error.display_with_world(world) - ) + function_arg_conversion_error!(argument, error.display_with_world(world)) }, InteropErrorInner::FunctionCallError { inner } => { - format!("Error in function call: {}", inner) + function_call_error!(inner) }, InteropErrorInner::BetterConversionExists{ context } => { - format!("Unfinished conversion in context of: {}. A better conversion exists but caller didn't handle the case.", context) + better_conversion_exists!(context) }, InteropErrorInner::OtherError { error } => error.to_string(), InteropErrorInner::LengthMismatch { expected, got } => { - format!("Array/List Length mismatch, expected: {}, got: {}", expected, got) + length_mismatch!(expected, got) }, InteropErrorInner::InvalidAccessCount { count, expected, context } => { - format!("Invalid access count, expected: {}, got: {}. {}", expected, count, context) + invalid_access_count!(expected, count, context) }, } } @@ -746,74 +873,54 @@ impl DisplayWithWorld for InteropErrorInner { fn display_without_world(&self) -> String { match self { InteropErrorInner::MissingFunctionError { on, function_name } => { - format!( - "Could not find function: {} for type: {}", - function_name, - on.display_without_world() - ) + missing_function_error!(function_name, on.display_without_world()) }, InteropErrorInner::UnregisteredBase { base } => { - format!("Unregistered base type: {}", base.display_without_world()) + unregistered_base!(base.display_without_world()) } InteropErrorInner::CannotClaimAccess { base, location } => { - format!( - "Cannot claim access to base type: {}. The base is already claimed by something else in a way which prevents safe access. Location: {}", - base.display_without_world(), - location.display_location() - ) + cannot_claim_access!(base.display_without_world(), location.display_location()) } InteropErrorInner::ImpossibleConversion { into } => { - format!("Cannot convert to type: {}", into.display_without_world()) + impossible_conversion!(into.display_without_world()) } InteropErrorInner::TypeMismatch { expected, got } => { - format!( - "Type mismatch, expected: {}, got: {}", + type_mismatch!( expected.display_without_world(), got.map(|t| t.display_without_world()) .unwrap_or("None".to_owned()) ) } InteropErrorInner::StringTypeMismatch { expected, got } => { - format!( - "Type mismatch, expected: {}, got: {}", + string_type_mismatch!( expected, got.map(|t| t.display_without_world()) .unwrap_or("None".to_owned()) ) } InteropErrorInner::CouldNotDowncast { from, to } => { - format!( - "Could not downcast from: {} to: {}", + could_not_downcast!( from.display_without_world(), to.display_without_world() ) } InteropErrorInner::GarbageCollectedAllocation { reference } => { - format!( - "Allocation was garbage collected. Could not access reference: {} as a result.", - reference.display_without_world(), - ) + garbage_collected_allocation!(reference.display_without_world()) } InteropErrorInner::ReflectionPathError { error, reference } => { - format!( - "Error while reflecting path: {} on reference: {}", + reflection_path_error!( error, reference .as_ref() .map(|r| r.display_without_world()) - .unwrap_or("None".to_owned()), + .unwrap_or("None".to_owned()) ) } InteropErrorInner::MissingTypeData { type_id, type_data } => { - format!( - "Missing type data {} for type: {}. Did you register the type correctly?", - type_data, - type_id.display_without_world(), - ) + missing_type_data!(type_data, type_id.display_without_world()) } InteropErrorInner::FailedFromReflect { type_id, reason } => { - format!( - "Failed to convert from reflect for type: {} with reason: {}", + failed_from_reflect!( type_id .map(|t| t.display_without_world()) .unwrap_or("None".to_owned()), @@ -821,8 +928,7 @@ impl DisplayWithWorld for InteropErrorInner { ) } InteropErrorInner::ValueMismatch { expected, got } => { - format!( - "Value mismatch, expected: {}, got: {}", + value_mismatch!( expected.display_without_world(), got.display_without_world() ) @@ -832,8 +938,7 @@ impl DisplayWithWorld for InteropErrorInner { value, operation, } => { - format!( - "Unsupported operation: {} on base: {} with value: {:?}", + unsupported_operation!( operation, base.map(|t| t.display_without_world()) .unwrap_or("None".to_owned()), @@ -841,17 +946,13 @@ impl DisplayWithWorld for InteropErrorInner { ) } InteropErrorInner::InvalidIndex { value, reason } => { - format!( - "Invalid index for value: {}: {}", - value.display_without_world(), - reason - ) + invalid_index!(value.display_without_world(), reason) } InteropErrorInner::MissingEntity { entity } => { - format!("Missing or invalid entity: {}", entity) + missing_entity!(entity) } InteropErrorInner::InvalidComponent { component_id } => { - format!("Invalid component: {:?}", component_id) + invalid_component!(component_id) } InteropErrorInner::StaleWorldAccess => { "Stale world access. The world has been dropped and a script tried to access it. Do not try to store or copy the world." @@ -864,37 +965,29 @@ impl DisplayWithWorld for InteropErrorInner { let opt_on = match on { Namespace::Global => "".to_owned(), Namespace::OnType(type_id) => format!("on type: {}", type_id.display_without_world()), - }; let display_name = if function_name.starts_with("TypeId") { + }; + let display_name = if function_name.starts_with("TypeId") { function_name.split("::").last().unwrap() } else { function_name.as_str() }; - format!( - "Error in function {} {}: {}", - display_name, - opt_on, - error.display_without_world(), - ) + function_interop_error!(display_name, opt_on, error.display_without_world()) }, InteropErrorInner::FunctionArgConversionError { argument, error } => { - format!( - "Error converting argument {}: {}", - argument, - error.display_without_world() - ) + function_arg_conversion_error!(argument, error.display_without_world()) }, InteropErrorInner::FunctionCallError { inner } => { - format!("Error in function call: {}", inner) + function_call_error!(inner) }, InteropErrorInner::BetterConversionExists{ context } => { - format!("Unfinished conversion in context of: {}. A better conversion exists but caller didn't handle the case.", context) + better_conversion_exists!(context) }, InteropErrorInner::OtherError { error } => error.to_string(), InteropErrorInner::LengthMismatch { expected, got } => { - format!("Array/List Length mismatch, expected: {}, got: {}", expected, got) + length_mismatch!(expected, got) }, InteropErrorInner::InvalidAccessCount { count, expected, context } => { - format!("Invalid access count, expected: {}, got: {}. {}", expected, count, context) + invalid_access_count!(expected, count, context) }, } } @@ -906,3 +999,47 @@ impl Default for InteropErrorInner { InteropErrorInner::StaleWorldAccess } } + +#[cfg(test)] +mod test { + use bevy::prelude::{AppTypeRegistry, World}; + + use crate::bindings::{ + function::script_function::AppScriptFunctionRegistry, AppReflectAllocator, + WorldAccessGuard, WorldGuard, + }; + + use super::*; + + #[test] + fn test_error_display() { + let error = + InteropError::failed_from_reflect(Some(TypeId::of::()), "reason".to_owned()); + let mut world = World::default(); + let type_registry = AppTypeRegistry::default(); + world.insert_resource(type_registry); + + let script_allocator = AppReflectAllocator::default(); + world.insert_resource(script_allocator); + + let script_function_registry = AppScriptFunctionRegistry::default(); + world.insert_resource(script_function_registry); + + let world_guard = WorldGuard::new(WorldAccessGuard::new(&mut world)); + assert_eq!( + error.display_with_world(world_guard), + format!( + "Failed to convert from reflect for type: {} with reason: reason", + std::any::type_name::() + ) + ); + + assert_eq!( + error.display_without_world(), + format!( + "Failed to convert from reflect for type: {:?} with reason: reason", + TypeId::of::() + ) + ); + } +} From f065a905efe52197d1caad04c3d77ab678187302 Mon Sep 17 00:00:00 2001 From: makspll Date: Thu, 16 Jan 2025 23:50:24 +0000 Subject: [PATCH 23/34] remove inactive docs code --- crates/bevy_mod_scripting_core/src/docs.rs | 24 ------------ crates/bevy_mod_scripting_core/src/lib.rs | 43 ---------------------- 2 files changed, 67 deletions(-) delete mode 100644 crates/bevy_mod_scripting_core/src/docs.rs diff --git a/crates/bevy_mod_scripting_core/src/docs.rs b/crates/bevy_mod_scripting_core/src/docs.rs deleted file mode 100644 index d186bcfe96..0000000000 --- a/crates/bevy_mod_scripting_core/src/docs.rs +++ /dev/null @@ -1,24 +0,0 @@ -use bevy::ecs::system::Resource; - -/// A documentation piece which can be used to make a piece of documentation, most often a module. -pub trait DocumentationFragment: 'static + Sized { - /// Merges two documentation fragments into one, retaining the title of the first fragment. - fn merge(self, o: Self) -> Self; - fn gen_docs(self) -> Result<(), Box>; - - /// Retrieves the name of the documentation fragment, most likely the name of your game! - fn name(&self) -> &'static str; -} - -#[derive(Resource)] -pub struct Documentation { - pub fragments: Vec, -} - -impl Default for Documentation { - fn default() -> Self { - Self { - fragments: Default::default(), - } - } -} diff --git a/crates/bevy_mod_scripting_core/src/lib.rs b/crates/bevy_mod_scripting_core/src/lib.rs index d7dcbd73ef..64ec4ce5ad 100644 --- a/crates/bevy_mod_scripting_core/src/lib.rs +++ b/crates/bevy_mod_scripting_core/src/lib.rs @@ -13,7 +13,6 @@ use context::{ Context, ContextAssigner, ContextBuilder, ContextInitializer, ContextLoadingSettings, ContextPreHandlingInitializer, ScriptContexts, }; -use docs::{Documentation, DocumentationFragment}; use event::ScriptCallbackEvent; use handler::{CallbackSettings, HandlerFn}; use runtime::{initialize_runtime, Runtime, RuntimeContainer, RuntimeInitializer, RuntimeSettings}; @@ -23,7 +22,6 @@ pub mod asset; pub mod bindings; pub mod commands; pub mod context; -pub mod docs; pub mod error; pub mod event; pub mod handler; @@ -241,44 +239,3 @@ impl AddRuntimeInitializer for App { self } } - -pub trait StoreDocumentation { - /// Adds a documentation fragment to the documentation store. - fn add_documentation_fragment(&mut self, fragment: D) -> &mut Self; - /// Consumes all the stored documentation fragments, and merges them into one, then generates the documentation. - fn generate_docs(&mut self) -> Result<(), Box>; -} - -impl StoreDocumentation for App { - fn add_documentation_fragment(&mut self, fragment: D) -> &mut Self { - self.world_mut() - .init_non_send_resource::>(); - self.world_mut() - .non_send_resource_mut::>() - .as_mut() - .fragments - .push(fragment); - self - } - - fn generate_docs(&mut self) -> Result<(), Box> { - let mut docs = match self - .world_mut() - .remove_non_send_resource::>() - { - Some(docs) => docs, - None => return Ok(()), - }; - - let mut top_fragment = match docs.fragments.pop() { - Some(fragment) => fragment, - None => return Ok(()), - }; - - for fragment in docs.fragments.into_iter() { - top_fragment = top_fragment.merge(fragment); - } - - top_fragment.gen_docs() - } -} From 9a71df2ec942aa6ac26986543a27edd1c11f5adc Mon Sep 17 00:00:00 2001 From: makspll Date: Thu, 16 Jan 2025 23:52:13 +0000 Subject: [PATCH 24/34] improve docs --- docs/src/Summary/running-scripts.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/src/Summary/running-scripts.md b/docs/src/Summary/running-scripts.md index 3d072515d7..13f08e0c61 100644 --- a/docs/src/Summary/running-scripts.md +++ b/docs/src/Summary/running-scripts.md @@ -47,4 +47,7 @@ app.add_systems(Update, event_handler::); Note the system is parameterized by the label we defined earlier, and the scripting plugin we are using. You can add as many of these systems as you like. -The event handler will catch all events with the label `OnEvent` and trigger the `on_event` callback on all targeted scripts which have that callback defined. \ No newline at end of file +The event handler will catch all events with the label `OnEvent` and trigger the `on_event` callback on all targeted scripts which have that callback defined. + +In order to handle events in the same frame and not accidentally have events "spill over" into the next frame, you should make sure to order any systems which produce these events *before* the event handler systems. + From 71720fd8f60c53115b47211be6ec8daa817f0876 Mon Sep 17 00:00:00 2001 From: makspll Date: Fri, 17 Jan 2025 00:15:56 +0000 Subject: [PATCH 25/34] add reflect reference tests --- .../src/bindings/reference.rs | 284 ++++++++++++++++++ 1 file changed, 284 insertions(+) diff --git a/crates/bevy_mod_scripting_core/src/bindings/reference.rs b/crates/bevy_mod_scripting_core/src/bindings/reference.rs index 92948b5ae5..d1709cf8e6 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/reference.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/reference.rs @@ -80,6 +80,11 @@ impl ReflectReference { }) } + /// Create a new reference to a value by allocating it. + /// + /// You can retrieve the allocator from the world using [`WorldGuard::allocator`]. + /// Make sure to drop the allocator write guard before doing anything with the reference to prevent deadlocks. + /// pub fn new_allocated( value: T, allocator: &mut ReflectAllocator, @@ -547,3 +552,282 @@ impl Iterator for ReflectRefIter { Some(result) } } + +#[cfg(test)] +mod test { + use bevy::prelude::{AppTypeRegistry, World}; + + use crate::bindings::{ + function::script_function::AppScriptFunctionRegistry, AppReflectAllocator, WorldAccessGuard, + }; + + use super::*; + + #[derive(Reflect, Component, Debug, Clone, PartialEq)] + struct Component(Vec); + + #[derive(Reflect, Resource, Debug, Clone, PartialEq)] + struct Resource(Vec); + + fn setup_world() -> World { + let mut world = World::default(); + + let type_registry = AppTypeRegistry::default(); + { + let mut guard_type_registry = type_registry.write(); + guard_type_registry.register::(); + guard_type_registry.register::(); + } + + world.insert_resource(type_registry); + + let allocator = AppReflectAllocator::default(); + world.insert_resource(allocator); + + let script_function_registry = AppScriptFunctionRegistry::default(); + world.insert_resource(script_function_registry); + + world + } + + #[test] + fn test_component_ref() { + let mut world = setup_world(); + + let entity = world + .spawn(Component(vec!["hello".to_owned(), "world".to_owned()])) + .id(); + + let world_guard = WorldGuard::new(WorldAccessGuard::new(&mut world)); + + let mut component_ref = + ReflectReference::new_component_ref::(entity, world_guard.clone()) + .expect("could not create component reference"); + + // index into component + assert_eq!( + component_ref + .tail_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + + component_ref + .with_reflect(world_guard.clone(), |s| { + let s = s.try_downcast_ref::().unwrap(); + assert_eq!(s, &Component(vec!["hello".to_owned(), "world".to_owned()])); + }) + .unwrap(); + + // index into vec field + component_ref.index_path(ParsedPath::parse_static(".0").unwrap()); + assert_eq!( + component_ref + .tail_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::>() + ); + + assert_eq!( + component_ref + .element_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + + assert_eq!( + component_ref + .key_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + + component_ref + .with_reflect(world_guard.clone(), |s| { + let s = s.try_downcast_ref::>().unwrap(); + assert_eq!(s, &vec!["hello".to_owned(), "world".to_owned()]); + }) + .unwrap(); + + // index into vec + component_ref.index_path(ParsedPath::parse_static("[0]").unwrap()); + + component_ref + .with_reflect(world_guard.clone(), |s| { + let s = s.try_downcast_ref::().unwrap(); + assert_eq!(s, "hello"); + }) + .unwrap(); + + assert_eq!( + component_ref + .tail_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + } + + #[test] + fn test_resource_ref() { + let mut world = setup_world(); + + world.insert_resource(Resource(vec!["hello".to_owned(), "world".to_owned()])); + + let world_guard = WorldGuard::new(WorldAccessGuard::new(&mut world)); + + let mut resource_ref = ReflectReference::new_resource_ref::(world_guard.clone()) + .expect("could not create resource reference"); + + // index into resource + assert_eq!( + resource_ref + .tail_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + + resource_ref + .with_reflect(world_guard.clone(), |s| { + let s = s.try_downcast_ref::().unwrap(); + assert_eq!(s, &Resource(vec!["hello".to_owned(), "world".to_owned()])); + }) + .unwrap(); + + // index into vec field + resource_ref.index_path(ParsedPath::parse_static(".0").unwrap()); + assert_eq!( + resource_ref + .tail_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::>() + ); + + assert_eq!( + resource_ref + .element_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + + assert_eq!( + resource_ref + .key_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + + resource_ref + .with_reflect(world_guard.clone(), |s| { + let s = s.try_downcast_ref::>().unwrap(); + assert_eq!(s, &vec!["hello".to_owned(), "world".to_owned()]); + }) + .unwrap(); + + // index into vec + resource_ref.index_path(ParsedPath::parse_static("[0]").unwrap()); + + resource_ref + .with_reflect(world_guard.clone(), |s| { + let s = s.try_downcast_ref::().unwrap(); + assert_eq!(s, "hello"); + }) + .unwrap(); + + assert_eq!( + resource_ref + .tail_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + } + + #[test] + fn test_allocation_ref() { + let mut world = setup_world(); + + let value = Component(vec!["hello".to_owned(), "world".to_owned()]); + + let world_guard = WorldGuard::new(WorldAccessGuard::new(&mut world)); + let allocator = world_guard.allocator(); + let mut allocator_write = allocator.write(); + let mut allocation_ref = ReflectReference::new_allocated(value, &mut allocator_write); + drop(allocator_write); + + // index into component + assert_eq!( + allocation_ref + .tail_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + + allocation_ref + .with_reflect(world_guard.clone(), |s| { + let s = s.try_downcast_ref::().unwrap(); + assert_eq!(s, &Component(vec!["hello".to_owned(), "world".to_owned()])); + }) + .unwrap(); + + // index into vec field + allocation_ref.index_path(ParsedPath::parse_static(".0").unwrap()); + assert_eq!( + allocation_ref + .tail_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::>() + ); + + assert_eq!( + allocation_ref + .element_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + + assert_eq!( + allocation_ref + .key_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + + allocation_ref + .with_reflect(world_guard.clone(), |s| { + let s = s.try_downcast_ref::>().unwrap(); + assert_eq!(s, &vec!["hello".to_owned(), "world".to_owned()]); + }) + .unwrap(); + + // index into vec + allocation_ref.index_path(ParsedPath::parse_static("[0]").unwrap()); + + allocation_ref + .with_reflect(world_guard.clone(), |s| { + let s = s.try_downcast_ref::().unwrap(); + assert_eq!(s, "hello"); + }) + .unwrap(); + + assert_eq!( + allocation_ref + .tail_type_id(world_guard.clone()) + .unwrap() + .unwrap(), + TypeId::of::() + ); + } +} From 02d921a0d5902fd42f06e7944dc2bd4903fa8b69 Mon Sep 17 00:00:00 2001 From: makspll Date: Fri, 17 Jan 2025 00:27:28 +0000 Subject: [PATCH 26/34] update report command --- .github/workflows/bevy_mod_scripting.yml | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index 416d9a2c97..affd9caf21 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -74,16 +74,9 @@ jobs: - name: Check run: | ${{ matrix.run_args.command }} - - name: Install lcov/genhtml + - name: Upload coverage artifact if: ${{ matrix.run_args.generates_coverage }} - run: | - sudo apt-get install lcov -y - - name: Report code coverage - if: ${{ matrix.run_args.generates_coverage }} - uses: zgosalvez/github-actions-report-lcov@v3 + uses: actions/upload-artifact@v4 with: - coverage-files: | - **/lcov.info - minimum-coverage: 50 - artifact-name: code-coverage-report - github-token: ${{ secrets.GITHUB_TOKEN }} \ No newline at end of file + name: code-coverage-report + path: target/coverage/html/ \ No newline at end of file From 8b45bec32c773752f95c5ebb676b4e026fb5641d Mon Sep 17 00:00:00 2001 From: makspll Date: Fri, 17 Jan 2025 00:30:25 +0000 Subject: [PATCH 27/34] include coverage badge as test --- badges/coverage.svg | 13 +++++++++++++ readme.md | 1 + 2 files changed, 14 insertions(+) create mode 100644 badges/coverage.svg diff --git a/badges/coverage.svg b/badges/coverage.svg new file mode 100644 index 0000000000..2e25bc0122 --- /dev/null +++ b/badges/coverage.svg @@ -0,0 +1,13 @@ + + COVERAGE: 53% + + + + + + COVERAGE + 53% + + + \ No newline at end of file diff --git a/readme.md b/readme.md index 83a6284773..c78cd84f0d 100644 --- a/readme.md +++ b/readme.md @@ -1,3 +1,4 @@ +

From 84b5553ce5a9217dc5109c9d6214780b18f4f90e Mon Sep 17 00:00:00 2001 From: makspll Date: Fri, 17 Jan 2025 00:36:59 +0000 Subject: [PATCH 28/34] increase badge width and update badge on commit --- .github/workflows/bevy_mod_scripting.yml | 15 ++++++++++++++- readme.md | 5 ++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index affd9caf21..94ef59eed0 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -79,4 +79,17 @@ jobs: uses: actions/upload-artifact@v4 with: name: code-coverage-report - path: target/coverage/html/ \ No newline at end of file + path: target/coverage/html/ + - name: Update coverage badge + if: ${{ matrix.run_args.generates_coverage }} + run: | + cp target/coverage/html/badges/for_the_badge.svg badges/coverage.svg + + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add badges/coverage.svg + + if [[ -n $(git status -s) ]]; then + git commit -m "chore(badge): Update coverage badge" -m "[skip ci]" + git push + fi \ No newline at end of file diff --git a/readme.md b/readme.md index c78cd84f0d..2aff8c8e4b 100644 --- a/readme.md +++ b/readme.md @@ -1,4 +1,7 @@ - + + +--- +

From 0109654dccd82520c0037fe7252d394fed0602c1 Mon Sep 17 00:00:00 2001 From: makspll Date: Fri, 17 Jan 2025 00:38:48 +0000 Subject: [PATCH 29/34] force badge re-creation --- .github/workflows/bevy_mod_scripting.yml | 6 ++++++ badges/coverage.svg | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index 94ef59eed0..d000bd27ba 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -57,6 +57,7 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 + - name: Install alsa and udev if: runner.os == 'linux' run: | @@ -66,20 +67,25 @@ jobs: with: toolchain: stable override: true + - name: Rust Cache uses: Swatinem/rust-cache@v2.7.3 + - name: Setup run: | cargo xtask init + - name: Check run: | ${{ matrix.run_args.command }} + - name: Upload coverage artifact if: ${{ matrix.run_args.generates_coverage }} uses: actions/upload-artifact@v4 with: name: code-coverage-report path: target/coverage/html/ + - name: Update coverage badge if: ${{ matrix.run_args.generates_coverage }} run: | diff --git a/badges/coverage.svg b/badges/coverage.svg index 2e25bc0122..f51bd67218 100644 --- a/badges/coverage.svg +++ b/badges/coverage.svg @@ -1,6 +1,6 @@ - COVERAGE: 53% + COVERAGE: 00% From ff90a0bf72476c3b1604eb101c884ac4a9b460c4 Mon Sep 17 00:00:00 2001 From: makspll Date: Fri, 17 Jan 2025 00:56:08 +0000 Subject: [PATCH 30/34] add more tests to printing and conversions --- .../src/bindings/pretty_print.rs | 112 +++++++++++++++++- .../src/bindings/script_value.rs | 32 +++++ 2 files changed, 143 insertions(+), 1 deletion(-) diff --git a/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs b/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs index c7faba4d64..8720a7d7ce 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs @@ -366,7 +366,7 @@ impl DisplayWithWorld for ReflectBaseType { impl DisplayWithWorld for TypeId { fn display_with_world(&self, world: WorldGuard) -> String { if *self == TypeId::of::() { - return "Dynamic Type".to_owned(); + return "Unknown Type".to_owned(); } else if *self == TypeId::of::() { // does not implement Reflect, so we do this manually return "World".to_owned(); @@ -485,3 +485,113 @@ impl DisplayWithWorld for Vec { string } } + +#[cfg(test)] +mod test { + use bevy::prelude::AppTypeRegistry; + + use crate::bindings::{ + function::script_function::AppScriptFunctionRegistry, AppReflectAllocator, + ReflectAllocationId, WorldAccessGuard, + }; + + use super::*; + + fn setup_world() -> World { + let mut world = World::default(); + + let type_registry = AppTypeRegistry::default(); + world.insert_resource(type_registry); + + let allocator = AppReflectAllocator::default(); + world.insert_resource(allocator); + + let script_function_registry = AppScriptFunctionRegistry::default(); + world.insert_resource(script_function_registry); + + world + } + + #[test] + fn test_type_id() { + let mut world = setup_world(); + let world = WorldGuard::new(WorldAccessGuard::new(&mut world)); + + let type_id = TypeId::of::(); + assert_eq!(type_id.display_with_world(world.clone()), "usize"); + assert_eq!(type_id.display_value_with_world(world.clone()), "usize"); + assert_eq!(type_id.display_without_world(), format!("{:?}", type_id)); + + let type_id = TypeId::of::(); + assert_eq!(type_id.display_with_world(world.clone()), "Unknown Type"); + assert_eq!( + type_id.display_value_with_world(world.clone()), + "Unknown Type" + ); + assert_eq!(type_id.display_without_world(), format!("{:?}", type_id)); + } + + #[test] + fn test_reflect_base_type() { + let mut world = setup_world(); + let world = WorldGuard::new(WorldAccessGuard::new(&mut world)); + + let type_id = TypeId::of::(); + + assert_eq!( + ReflectBaseType { + base_id: ReflectBase::Owned(ReflectAllocationId::new(0)), + type_id, + } + .display_with_world(world.clone()), + "Allocation(0)(usize)" + ); + + assert_eq!( + ReflectBaseType { + base_id: ReflectBase::Owned(ReflectAllocationId::new(0)), + type_id, + } + .display_value_with_world(world.clone()), + "Allocation(0)(usize)" + ); + + assert_eq!( + ReflectBaseType { + base_id: ReflectBase::Owned(ReflectAllocationId::new(0)), + type_id, + } + .display_without_world(), + format!("Allocation(0)({:?})", type_id) + ); + } + + #[test] + fn test_reflect_reference() { + let mut world = setup_world(); + + let world = WorldGuard::new(WorldAccessGuard::new(&mut world)); + + let type_id = TypeId::of::(); + + let allocator = world.allocator(); + let mut allocator_write = allocator.write(); + let reflect_reference = ReflectReference::new_allocated(2usize, &mut allocator_write); + drop(allocator_write); + + assert_eq!( + reflect_reference.display_with_world(world.clone()), + " usize>" + ); + + assert_eq!( + reflect_reference.display_value_with_world(world.clone()), + "Reflect(usize(2))" + ); + + assert_eq!( + reflect_reference.display_without_world(), + format!("", type_id) + ); + } +} diff --git a/crates/bevy_mod_scripting_core/src/bindings/script_value.rs b/crates/bevy_mod_scripting_core/src/bindings/script_value.rs index 9cbf5b7167..993980141b 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/script_value.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/script_value.rs @@ -182,3 +182,35 @@ impl TryFrom for ParsedPath { }) } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_script_value_to_parsed_path() { + let value = ScriptValue::String("test".into()); + let parsed_path = ParsedPath::from(vec![OffsetAccess { + access: bevy::reflect::Access::Field("test".to_owned().into()), + offset: Some(4), + }]); + assert_eq!(parsed_path, ParsedPath::try_from(value).unwrap()); + + let value = ScriptValue::String("_0".into()); + let parsed_path = ParsedPath::from(vec![OffsetAccess { + access: bevy::reflect::Access::TupleIndex(0), + offset: Some(1), + }]); + assert_eq!(parsed_path, ParsedPath::try_from(value).unwrap()); + + let value = ScriptValue::Integer(0); + let parsed_path = ParsedPath::from(vec![OffsetAccess { + access: bevy::reflect::Access::ListIndex(0), + offset: Some(1), + }]); + assert_eq!(parsed_path, ParsedPath::try_from(value).unwrap()); + + let value = ScriptValue::Float(0.0); + assert!(ParsedPath::try_from(value).is_err()); + } +} From f11d09df9da9d20c01477c8bb570eebe347420ad Mon Sep 17 00:00:00 2001 From: makspll Date: Fri, 17 Jan 2025 00:58:45 +0000 Subject: [PATCH 31/34] update workflow permissions --- .github/workflows/bevy_mod_scripting.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index d000bd27ba..e4491259b8 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -46,6 +46,7 @@ jobs: check: permissions: pull-requests: write + contents: write name: Check - ${{ matrix.run_args.name }} runs-on: ${{ matrix.run_args.os }} # container: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest From c4e63483c393c8576e46a1b6a93d1f6a6116de4a Mon Sep 17 00:00:00 2001 From: makspll Date: Fri, 17 Jan 2025 01:00:55 +0000 Subject: [PATCH 32/34] update coverage badge on main only --- .github/workflows/bevy_mod_scripting.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bevy_mod_scripting.yml b/.github/workflows/bevy_mod_scripting.yml index e4491259b8..c422973cb7 100644 --- a/.github/workflows/bevy_mod_scripting.yml +++ b/.github/workflows/bevy_mod_scripting.yml @@ -88,7 +88,7 @@ jobs: path: target/coverage/html/ - name: Update coverage badge - if: ${{ matrix.run_args.generates_coverage }} + if: ${{ matrix.run_args.generates_coverage && github.ref == 'refs/heads/main' }} run: | cp target/coverage/html/badges/for_the_badge.svg badges/coverage.svg From 0f49dcdb3f8d5f6786807a2ffd08761c9cca1588 Mon Sep 17 00:00:00 2001 From: makspll Date: Fri, 17 Jan 2025 01:16:38 +0000 Subject: [PATCH 33/34] fix test and stop ignoring things --- .../bevy_mod_scripting_core/src/bindings/pretty_print.rs | 9 +++++++-- crates/xtask/src/main.rs | 8 -------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs b/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs index 8720a7d7ce..d32c741e3f 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs @@ -577,11 +577,16 @@ mod test { let allocator = world.allocator(); let mut allocator_write = allocator.write(); let reflect_reference = ReflectReference::new_allocated(2usize, &mut allocator_write); + let id = match reflect_reference.base.base_id { + ReflectBase::Owned(ref id) => id.to_string(), + _ => panic!("Expected owned allocation"), + }; + drop(allocator_write); assert_eq!( reflect_reference.display_with_world(world.clone()), - " usize>" + format!(" usize>") ); assert_eq!( @@ -591,7 +596,7 @@ mod test { assert_eq!( reflect_reference.display_without_world(), - format!("", type_id) + format!("", type_id) ); } } diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index e56dfb4142..3076997a59 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -916,10 +916,6 @@ impl Xtasks { "html", "--branch", "--ignore-not-existing", - "--ignore", - "../*", - "--ignore", - "/*", "-o", "target/coverage/html", ], @@ -939,10 +935,6 @@ impl Xtasks { "lcov", "--branch", "--ignore-not-existing", - "--ignore", - "../*", - "--ignore", - "/*", "-o", "target/coverage/lcov.info", ], From db67affdc51d8015d8127640b30670edb53d8ebc Mon Sep 17 00:00:00 2001 From: makspll Date: Fri, 17 Jan 2025 01:38:53 +0000 Subject: [PATCH 34/34] ignore bevy bindings from coverage --- crates/xtask/src/main.rs | 4 ++++ readme.md | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index 3076997a59..7a7c8227bd 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -916,6 +916,8 @@ impl Xtasks { "html", "--branch", "--ignore-not-existing", + "--ignore", + "**/bevy_bindings/**", "-o", "target/coverage/html", ], @@ -935,6 +937,8 @@ impl Xtasks { "lcov", "--branch", "--ignore-not-existing", + "--ignore", + "**/bevy_bindings/**", "-o", "target/coverage/lcov.info", ], diff --git a/readme.md b/readme.md index 2aff8c8e4b..dae13e5b99 100644 --- a/readme.md +++ b/readme.md @@ -46,6 +46,4 @@ The languages currently supported are as follows: For examples, installation and usage instructions see our shiny new [book](https://makspll.github.io/bevy_mod_scripting) -## Footnotes - [^1]: Due to the recent re-write of the crate, documentation generation as well as rhai and rune support are temporarilly on hold. They will likely be re-implemented in the future. `Rhai` in particualar is difficult to re-implement due to a lack of support for first-class-functions. \ No newline at end of file