diff --git a/benches/benchmarks.rs b/benches/benchmarks.rs index 4440f67f81..edab8b98c4 100644 --- a/benches/benchmarks.rs +++ b/benches/benchmarks.rs @@ -266,22 +266,13 @@ fn conversion_benchmarks(criterion: &mut Criterion) { fn script_load_benchmarks(criterion: &mut Criterion) { let mut group = criterion.benchmark_group("loading"); - let reload_probability = 0.5; // lua - let plugin = make_test_lua_plugin(); let content = include_str!("../assets/macro_benchmarks/loading/empty.lua"); - run_plugin_script_load_benchmark(plugin, "empty Lua", content, &mut group, reload_probability); + run_plugin_script_load_benchmark(make_test_lua_plugin, "empty Lua", content, &mut group); // rhai - let plugin = make_test_rhai_plugin(); let content = include_str!("../assets/macro_benchmarks/loading/empty.rhai"); - run_plugin_script_load_benchmark( - plugin, - "empty Rhai", - content, - &mut group, - reload_probability, - ); + run_plugin_script_load_benchmark(make_test_rhai_plugin, "empty Rhai", content, &mut group); } pub fn benches() { diff --git a/crates/bevy_mod_scripting_bindings/src/globals/core.rs b/crates/bevy_mod_scripting_bindings/src/globals/core.rs index 769d2b53bd..828dbe4b90 100644 --- a/crates/bevy_mod_scripting_bindings/src/globals/core.rs +++ b/crates/bevy_mod_scripting_bindings/src/globals/core.rs @@ -7,6 +7,7 @@ use ::{ bevy_reflect::TypeRegistration, }; use bevy_app::App; +use bevy_log::warn; use bevy_mod_scripting_asset::ScriptAsset; use bevy_mod_scripting_derive::script_globals; use bevy_platform::collections::HashMap; @@ -63,6 +64,7 @@ impl Plugin for CoreScriptGlobalsPlugin { register_core_globals(app.world_mut()); } } +const MSG_DUPLICATE_GLOBAL: &str = "This can cause confusing issues for script users, use `CoreScriptGlobalsPlugin.filter` to filter out uneeded duplicate types."; #[profiling::function] fn register_static_core_globals(world: &mut World, filter: fn(&TypeRegistration) -> bool) { @@ -82,20 +84,41 @@ fn register_static_core_globals(world: &mut World, filter: fn(&TypeRegistration) if let Some(global_name) = registration.type_info().type_path_table().ident() { let documentation = "A reference to the type, allowing you to call static methods."; let type_info = registration.type_info(); - global_registry.register_static_documented_dynamic( - registration.type_id(), - into_through_type_info(type_info), - global_name.into(), - documentation.into(), - ); + if global_registry + .register_static_documented_dynamic( + registration.type_id(), + into_through_type_info(type_info), + global_name.into(), + documentation.into(), + ) + .is_some() + { + warn!( + "Duplicate global registration for type: '{}'. {MSG_DUPLICATE_GLOBAL}", + type_info.type_path_table().short_path() + ) + }; } } // register basic globals - global_registry.register_dummy::("world", "The current ECS world."); - global_registry - .register_dummy::("entity", "The entity this script is attached to if any."); - global_registry.register_dummy_typed::>>("script_asset", "the asset handle for this script. If the asset is ever unloaded, the handle will be less useful."); + if global_registry + .register_dummy::("world", "The current ECS world.") + .is_some() + { + warn!("existing `world` global was replaced by the core `world` dummy type.") + }; + + if global_registry + .register_dummy::("entity", "The entity this script is attached to if any.") + .is_some() + { + warn!("existing `entity` global was replaced by the core `entity` dummy type.") + } + + if global_registry.register_dummy_typed::>>("script_asset", "the asset handle for this script. If the asset is ever unloaded, the handle will be less useful.").is_some() { + warn!("existing `script_asset` global was replaced by the core `script_asset` dummy type.") + }; } #[script_globals(bms_bindings_path = "crate", name = "core_globals")] @@ -128,7 +151,15 @@ impl CoreGlobals { let registration = guard.clone().get_type_registration(registration)?; let registration = registration.map_both(Val::from, |u| u.map_both(Val::from, Val::from)); - type_cache.insert(type_path.to_owned(), registration); + if type_cache + .insert(type_path.to_owned(), registration) + .is_some() + { + warn!( + "duplicate entry inside `types` global for type: {}. {MSG_DUPLICATE_GLOBAL}", + type_path + ) + }; } Ok(type_cache) diff --git a/crates/bevy_mod_scripting_bindings/src/globals/mod.rs b/crates/bevy_mod_scripting_bindings/src/globals/mod.rs index b93ccda3ea..bdabd4dcec 100644 --- a/crates/bevy_mod_scripting_bindings/src/globals/mod.rs +++ b/crates/bevy_mod_scripting_bindings/src/globals/mod.rs @@ -116,6 +116,7 @@ impl ScriptGlobalsRegistry { } /// Inserts a global into the registry, returns the previous value if it existed + #[must_use] pub fn register< T: ScriptReturn + 'static + Typed, F: Fn(WorldGuard) -> Result + 'static + Send + Sync, @@ -140,11 +141,12 @@ impl ScriptGlobalsRegistry { /// This can be useful for globals which you cannot expose normally. /// /// Dummy globals are stored as non-static instances, i.e. they're expected to be values not type references. + #[must_use] pub fn register_dummy( &mut self, name: impl Into>, documentation: impl Into>, - ) { + ) -> Option { self.dummies.insert( name.into(), ScriptGlobalDummy { @@ -152,15 +154,16 @@ impl ScriptGlobalsRegistry { type_id: TypeId::of::(), type_information: None, }, - ); + ) } /// Typed equivalent to [`Self::register_dummy`]. + #[must_use] pub fn register_dummy_typed( &mut self, name: impl Into>, documentation: impl Into>, - ) { + ) -> Option { self.dummies.insert( name.into(), ScriptGlobalDummy { @@ -168,12 +171,13 @@ impl ScriptGlobalsRegistry { type_id: TypeId::of::(), type_information: Some(T::through_type_info()), }, - ); + ) } /// Inserts a global into the registry, returns the previous value if it existed. /// /// This is a version of [`Self::register`] which stores type information regarding the global. + #[must_use] pub fn register_documented< T: TypedScriptReturn + 'static, F: Fn(WorldGuard) -> Result + 'static + Send + Sync, @@ -195,7 +199,11 @@ impl ScriptGlobalsRegistry { } /// Registers a static global into the registry. - pub fn register_static(&mut self, name: Cow<'static, str>) { + #[must_use] + pub fn register_static( + &mut self, + name: Cow<'static, str>, + ) -> Option { self.globals.insert( name, ScriptGlobal { @@ -204,17 +212,18 @@ impl ScriptGlobalsRegistry { type_id: TypeId::of::(), type_information: into_through_type_info(T::type_info()), }, - ); + ) } /// Registers a static global into the registry. /// /// This is a version of [`Self::register_static`] which stores rich type information regarding the global. + #[must_use] pub fn register_static_documented( &mut self, name: Cow<'static, str>, documentation: Cow<'static, str>, - ) { + ) -> Option { self.globals.insert( name, ScriptGlobal { @@ -223,19 +232,20 @@ impl ScriptGlobalsRegistry { type_id: TypeId::of::(), type_information: T::through_type_info(), }, - ); + ) } /// Registers a static global into the registry. /// /// This is a version of [`Self::register_static_documented`] which does not require compile time type knowledge. + #[must_use] pub fn register_static_documented_dynamic( &mut self, type_id: TypeId, type_information: ThroughTypeInfo, name: Cow<'static, str>, documentation: Cow<'static, str>, - ) { + ) -> Option { self.globals.insert( name, ScriptGlobal { @@ -244,7 +254,7 @@ impl ScriptGlobalsRegistry { type_id, type_information, }, - ); + ) } } @@ -307,14 +317,14 @@ mod test { fn test_static_globals() { let mut registry = ScriptGlobalsRegistry::default(); - registry.register_static::(Cow::Borrowed("foo")); + _ = registry.register_static::(Cow::Borrowed("foo")); let global = registry.get("foo").unwrap(); assert!(global.maker.is_none()); assert_eq!(global.type_id, TypeId::of::()); // the same but documented - registry.register_static_documented::( + _ = registry.register_static_documented::( Cow::Borrowed("bar"), Cow::Borrowed("This is a test"), ); diff --git a/crates/bevy_mod_scripting_derive/src/derive/script_globals.rs b/crates/bevy_mod_scripting_derive/src/derive/script_globals.rs index c3877a616d..ed2931751f 100644 --- a/crates/bevy_mod_scripting_derive/src/derive/script_globals.rs +++ b/crates/bevy_mod_scripting_derive/src/derive/script_globals.rs @@ -42,8 +42,11 @@ pub fn script_globals( let registry = world.get_resource_or_init::<#bms_bindings_path::globals::AppScriptGlobalsRegistry>(); let mut registry = registry.write(); - registry - #(#function_registrations)*; + #( + if (registry #function_registrations).is_some() { + warn!("conflicting global registration under name: {}. This might cause confusing problems, use `CoreScriptGlobalsPlugin.filter` to filter out uneeded duplicate types.", stringify!(#function_name)) + } + )*; } }; diff --git a/crates/testing_crates/script_integration_test_harness/src/lib.rs b/crates/testing_crates/script_integration_test_harness/src/lib.rs index ff3edb9571..90596f3ca7 100644 --- a/crates/testing_crates/script_integration_test_harness/src/lib.rs +++ b/crates/testing_crates/script_integration_test_harness/src/lib.rs @@ -4,7 +4,6 @@ pub mod test_functions; use std::{ path::PathBuf, - sync::Mutex, time::{Duration, Instant}, }; @@ -22,7 +21,6 @@ use ::{ bevy_reflect::Reflect, }; use bevy_asset::Assets; -use bevy_ecs::world::World; use bevy_mod_scripting_asset::ScriptAsset; use bevy_mod_scripting_bindings::{ CoreScriptGlobalsPlugin, ReflectAccessId, ThreadWorldContainer, WorldAccessGuard, @@ -340,61 +338,40 @@ where pub fn run_plugin_script_load_benchmark< P: IntoScriptPluginParams + Plugin + FromWorld, + F: Fn() -> P, M: Measurement, >( - plugin: P, + plugin_maker: F, benchmark_id: &str, content: &str, criterion: &mut criterion::BenchmarkGroup, - reload_probability: f32, ) { - let mut app = setup_integration_test(|_, _| {}); - install_test_plugin(&mut app, false); - app.add_plugins(plugin); let content_boxed = content.to_string().into_bytes().into_boxed_slice(); - let mut rng_guard = RNG.lock().unwrap(); - *rng_guard = rand_chacha::ChaCha12Rng::from_seed([42u8; 32]); - drop(rng_guard); - - let world_ptr = app.world_mut() as *mut World; - let world_guard = Mutex::<()>::new(()); criterion.bench_function(benchmark_id, |c| { c.iter_batched( || { - let mut rng = RNG.lock().unwrap(); - let is_reload = rng.random_range(0f32..=1f32) < reload_probability; + let mut app = setup_integration_test(|_, _| {}); + install_test_plugin(&mut app, false); + app.add_plugins(plugin_maker()); - let guard = world_guard.lock().unwrap(); // Safety: we claimed a unique guard, only code accessing this will need to do the same - let world = unsafe { &mut *world_ptr }; + let world = app.world_mut(); let mut assets = world.get_resource_or_init::>(); - let id = is_reload - .then(|| assets.ids().next()) - .flatten() - .and_then(|id| assets.get_strong_handle(id)) - .unwrap_or_else(|| { - assets.add(ScriptAsset { - content: content_boxed.clone(), - language: P::LANGUAGE, - }) - }); - drop(guard); + let id = assets.add(ScriptAsset { + content: content_boxed.clone(), + language: P::LANGUAGE, + }); // We manually load the script inside a command. ( + app, AttachScript::

::new(ScriptAttachment::StaticScript(id)), - is_reload, ) }, - |(command, _is_reload)| { - tracing::event!( - Level::TRACE, - "profiling_iter {} is reload?: {}", - benchmark_id, - _is_reload - ); + |(mut app, command)| { + tracing::event!(Level::TRACE, "profiling_iter {}", benchmark_id); command.apply(app.world_mut()); }, BatchSize::LargeInput,