Skip to content

Commit 85e0ae6

Browse files
authored
feat: add global types cache making get_type_by_name redundant (#370)
# Summary using `get_type_by_name` is slightly cumbersome. In order to not repeat yourself AND avoid polluting globals, you need to call: ```lua local Type = world.get_type_by_name("MYType") ``` in EVERY callback, or alternatively do it once at the top of your script, but doing that goes against the advice to not run code in the body of the script outside of any functions. this PR adds a global `types` cache, which lets you do: ```lua types.MyType ``` or for more complex types: ```lua types["MyGenericType<MyOtherType>"] ``` The types populated in the cache are equivalent to the types available as static globals with the addition of generic types. The change makes use of changes in #369 to correctly type this global as `HashMap<String, ScriptComponentRegistration | ScriptResourceRegistration | ScriptTypeRegistration >` meaning once we start generating decleration files for lua we will have the correct types at hand # Migration Guide - Not a breaking change, but you should change instances where you call `get_type_by_name("T")` to `types["T"]` etc. this will increase performance as well as make your scripts easier to read!
1 parent 2dc6558 commit 85e0ae6

File tree

6 files changed

+104
-45
lines changed

6 files changed

+104
-45
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
function on_test()
3+
local my_type = types.TestResource;
4+
assert(my_type ~= nil, "Type TestResource is not available in type cache");
5+
assert(my_type:short_name() == "TestResource", "Type t.TestResource:short_name() is not correct: " .. my_type:short_name());
6+
end
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
fn on_test() {
2+
let my_type = types.TestResource;
3+
assert(type_of(my_type) != "()", "Type TestResource is not available in type cache");
4+
assert(my_type.short_name.call() == "TestResource", "Type t.TestResource:short_name() is not correct: " + my_type.short_name.call());
5+
}

crates/bevy_mod_scripting_core/src/bindings/globals/core.rs

+39-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
//! Core globals exposed by the BMS framework
22
3+
use std::{collections::HashMap, sync::Arc};
4+
35
use bevy::{app::Plugin, ecs::reflect::AppTypeRegistry};
6+
use bevy_mod_scripting_derive::script_globals;
7+
8+
use crate::{bindings::{function::from::{Union, Val}, ScriptComponentRegistration, ScriptResourceRegistration, ScriptTypeRegistration, WorldGuard}, error::InteropError};
49

510
use super::AppScriptGlobalsRegistry;
611

@@ -10,12 +15,16 @@ pub struct CoreScriptGlobalsPlugin;
1015
impl Plugin for CoreScriptGlobalsPlugin {
1116
fn build(&self, _app: &mut bevy::app::App) {}
1217
fn finish(&self, app: &mut bevy::app::App) {
13-
let global_registry = app
14-
.world_mut()
18+
register_static_core_globals(app.world_mut());
19+
register_core_globals(app.world_mut());
20+
}
21+
}
22+
23+
fn register_static_core_globals(world: &mut bevy::ecs::world::World) {
24+
let global_registry = world
1525
.get_resource_or_init::<AppScriptGlobalsRegistry>()
1626
.clone();
17-
let type_registry = app
18-
.world_mut()
27+
let type_registry = world
1928
.get_resource_or_init::<AppTypeRegistry>()
2029
.clone();
2130
let mut global_registry = global_registry.write();
@@ -34,8 +43,32 @@ impl Plugin for CoreScriptGlobalsPlugin {
3443
None,
3544
global_name.into(),
3645
documentation.into(),
37-
)
46+
);
3847
}
3948
}
40-
}
4149
}
50+
51+
#[script_globals(
52+
bms_core_path = "crate",
53+
name = "core_globals",
54+
)]
55+
impl CoreGlobals {
56+
/// A cache of types normally available through the `world.get_type_by_name` function.
57+
///
58+
/// You can use this to avoid having to store type references.
59+
fn types(guard: WorldGuard) -> Result<HashMap<String, Union<Val<ScriptTypeRegistration>, Union<Val<ScriptComponentRegistration>, Val<ScriptResourceRegistration>>>>, InteropError> {
60+
let type_registry = guard.type_registry();
61+
let type_registry = type_registry.read();
62+
let mut type_cache = HashMap::<String, _>::default();
63+
for registration in type_registry.iter(){
64+
if let Some(ident) = registration.type_info().type_path_table().ident() {
65+
let registration = ScriptTypeRegistration::new(Arc::new(registration.clone()));
66+
let registration = guard.clone().get_type_registration(registration)?;
67+
let registration = registration.map_both(Val::from, |u| u.map_both(Val::from, Val::from));
68+
type_cache.insert(ident.to_string(), registration);
69+
}
70+
}
71+
72+
Ok(type_cache)
73+
}
74+
}

crates/bevy_mod_scripting_core/src/bindings/globals/mod.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,11 @@ impl ScriptGlobalsRegistry {
100100
F: Fn(WorldGuard) -> Result<T, InteropError> + 'static + Send + Sync,
101101
>(
102102
&mut self,
103-
name: Cow<'static, str>,
103+
name: impl Into<Cow<'static, str>>,
104104
maker: F,
105105
) -> Option<ScriptGlobal> {
106106
self.globals.insert(
107-
name,
107+
name.into(),
108108
ScriptGlobal {
109109
maker: Some(Self::type_erase_maker(maker)),
110110
documentation: None,
@@ -122,15 +122,15 @@ impl ScriptGlobalsRegistry {
122122
F: Fn(WorldGuard) -> Result<T, InteropError> + 'static + Send + Sync,
123123
>(
124124
&mut self,
125-
name: Cow<'static, str>,
125+
name: impl Into<Cow<'static, str>>,
126126
maker: F,
127-
documentation: Cow<'static, str>,
127+
documentation: impl Into<Cow<'static, str>>,
128128
) -> Option<ScriptGlobal> {
129129
self.globals.insert(
130-
name,
130+
name.into(),
131131
ScriptGlobal {
132132
maker: Some(Self::type_erase_maker(maker)),
133-
documentation: Some(documentation),
133+
documentation: Some(documentation.into()),
134134
type_id: TypeId::of::<T>(),
135135
type_information: Some(T::through_type_info()),
136136
},

crates/bevy_mod_scripting_core/src/bindings/world.rs

+35-2
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use super::{
1212
}, function::{
1313
namespace::Namespace,
1414
script_function::{AppScriptFunctionRegistry, DynamicScriptFunction, FunctionCallContext},
15-
}, pretty_print::DisplayWithWorld, schedule::AppScheduleRegistry, script_value::ScriptValue, with_global_access, AppReflectAllocator, ReflectBase, ReflectBaseType, ReflectReference, ScriptComponentRegistration, ScriptResourceRegistration, ScriptTypeRegistration
15+
}, pretty_print::DisplayWithWorld, schedule::AppScheduleRegistry, script_value::ScriptValue, with_global_access, AppReflectAllocator, ReflectBase, ReflectBaseType, ReflectReference, ScriptComponentRegistration, ScriptResourceRegistration, ScriptTypeRegistration, Union
1616
};
1717
use crate::{
1818
bindings::{function::{from::FromScript, from_ref::FromScriptRef}, with_access_read, with_access_write},
@@ -542,6 +542,7 @@ impl<'w> WorldAccessGuard<'w> {
542542
/// Impl block for higher level world methods
543543
#[profiling::all_functions]
544544
impl WorldAccessGuard<'_> {
545+
545546
fn construct_from_script_value(
546547
&self,
547548
descriptor: impl Into<Cow<'static, str>>,
@@ -800,7 +801,7 @@ impl WorldAccessGuard<'_> {
800801
})
801802
}
802803

803-
/// get a type registration for the type
804+
/// get a type registration for the type, without checking if it's a component or resource
804805
pub fn get_type_by_name(&self, type_name: String) -> Option<ScriptTypeRegistration> {
805806
let type_registry = self.type_registry();
806807
let type_registry = type_registry.read();
@@ -810,6 +811,38 @@ impl WorldAccessGuard<'_> {
810811
.map(|registration| ScriptTypeRegistration::new(Arc::new(registration.clone())))
811812
}
812813

814+
/// get a type erased type registration for the type including information about whether it's a component or resource
815+
pub(crate) fn get_type_registration(&self, registration: ScriptTypeRegistration) -> Result<Union<ScriptTypeRegistration, Union<ScriptComponentRegistration, ScriptResourceRegistration>>, InteropError> {
816+
817+
let registration = match self.get_resource_type(registration)? {
818+
Ok(res) => {
819+
return Ok(Union::new_right(Union::new_right(res)));
820+
}
821+
Err(registration) => registration,
822+
};
823+
824+
let registration = match self.get_component_type(registration)? {
825+
Ok(comp) => {
826+
return Ok(Union::new_right(Union::new_left(comp)));
827+
}
828+
Err(registration) => registration,
829+
};
830+
831+
Ok(Union::new_left(registration))
832+
}
833+
834+
/// Similar to [`Self::get_type_by_name`] but returns a type erased [`ScriptTypeRegistration`], [`ScriptComponentRegistration`] or [`ScriptResourceRegistration`]
835+
/// depending on the underlying type and state of the world.
836+
pub fn get_type_registration_by_name(&self, type_name: String) -> Result<Option<Union<ScriptTypeRegistration, Union<ScriptComponentRegistration, ScriptResourceRegistration>>>, InteropError> {
837+
let val = self.get_type_by_name(type_name);
838+
Ok(match val {
839+
Some(registration) => {
840+
Some(self.get_type_registration(registration)?)
841+
}
842+
None => None,
843+
})
844+
}
845+
813846
/// get a schedule by name
814847
pub fn get_schedule_by_name(&self, schedule_name: String) -> Option<ReflectSchedule> {
815848
let schedule_registry = self.schedule_registry();

crates/bevy_mod_scripting_functions/src/core.rs

+13-31
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ use bindings::{
3030
};
3131
use error::InteropError;
3232
use reflection_extensions::{PartialReflectExt, TypeIdExtensions};
33+
3334
pub fn register_bevy_bindings(app: &mut App) {
3435
#[cfg(feature = "bevy_bindings")]
3536
app.add_plugins(crate::bevy_bindings::LuaBevyScriptingPlugin);
@@ -45,39 +46,20 @@ impl World {
4546
fn get_type_by_name(
4647
ctxt: FunctionCallContext,
4748
type_name: String,
48-
) -> Result<Option<ReflectReference>, InteropError> {
49+
) -> Result<
50+
Option<
51+
Union<
52+
Val<ScriptTypeRegistration>,
53+
Union<Val<ScriptComponentRegistration>, Val<ScriptResourceRegistration>>,
54+
>,
55+
>,
56+
InteropError,
57+
> {
4958
profiling::function_scope!("get_type_by_name");
5059
let world = ctxt.world()?;
51-
let val = world.get_type_by_name(type_name);
52-
53-
Ok(match val {
54-
Some(registration) => {
55-
let allocator = world.allocator();
56-
57-
let registration = match world.get_resource_type(registration)? {
58-
Ok(res) => {
59-
let mut allocator = allocator.write();
60-
return Ok(Some(ReflectReference::new_allocated(res, &mut allocator)));
61-
}
62-
Err(registration) => registration,
63-
};
64-
65-
let registration = match world.get_component_type(registration)? {
66-
Ok(comp) => {
67-
let mut allocator = allocator.write();
68-
return Ok(Some(ReflectReference::new_allocated(comp, &mut allocator)));
69-
}
70-
Err(registration) => registration,
71-
};
72-
73-
let mut allocator = allocator.write();
74-
Some(ReflectReference::new_allocated(
75-
registration,
76-
&mut allocator,
77-
))
78-
}
79-
None => None,
80-
})
60+
world
61+
.get_type_registration_by_name(type_name)
62+
.map(|v| v.map(|v| v.map_both(Val::from, |u| u.map_both(Val::from, Val::from))))
8163
}
8264

8365
/// Retrieves the schedule with the given name, Also ensures the schedule is initialized before returning it.

0 commit comments

Comments
 (0)