Skip to content

Commit fabf5a7

Browse files
makspllmakspll
committed
feat!: Remove WorldCallbackAccess & unify dynamic function context arguments
Reduces indirection and simplifies the entire interface, speeds up compile time for functions
1 parent 6c92a68 commit fabf5a7

File tree

16 files changed

+692
-612
lines changed

16 files changed

+692
-612
lines changed

‎crates/bevy_mod_scripting_core/src/bindings/function/script_function.rs

Lines changed: 41 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ use super::{from::FromScript, into::IntoScript, namespace::Namespace};
22
use crate::{
33
bindings::{
44
function::from::{Mut, Ref, Val},
5-
ReflectReference, WorldGuard,
5+
ReflectReference, ThreadWorldContainer, WorldContainer, WorldGuard,
66
},
77
error::InteropError,
8-
ScriptValue, WorldCallbackAccess,
8+
ScriptValue,
99
};
1010
use bevy::{
1111
prelude::{Reflect, Resource},
@@ -96,7 +96,8 @@ macro_rules! register_tuple_dependencies {
9696
}
9797

9898
no_type_dependencies!(InteropError);
99-
self_type_dependency_only!(WorldCallbackAccess, CallerContext, ReflectReference);
99+
no_type_dependencies!(WorldGuard<'static>);
100+
self_type_dependency_only!(FunctionCallContext, ReflectReference);
100101

101102
recursive_type_dependencies!(
102103
(Val<T> where T: GetTypeRegistration),
@@ -118,9 +119,21 @@ pub trait GetFunctionTypeDependencies<Marker> {
118119
/// Functions can choose to react to caller preferences such as converting 1-indexed numbers to 0-indexed numbers
119120
#[derive(Clone, Copy, Debug, Reflect, Default)]
120121
#[reflect(opaque)]
121-
pub struct CallerContext {
122+
pub struct FunctionCallContext {
122123
pub convert_to_0_indexed: bool,
123124
}
125+
impl FunctionCallContext {
126+
pub fn new(convert_to_0_indexed: bool) -> Self {
127+
Self {
128+
convert_to_0_indexed,
129+
}
130+
}
131+
132+
/// Tries to access the world, returning an error if the world is not available
133+
pub fn world(&self) -> Result<WorldGuard<'static>, InteropError> {
134+
ThreadWorldContainer.try_get_world()
135+
}
136+
}
124137

125138
#[derive(Clone, Debug, PartialEq, Default)]
126139
pub struct FunctionInfo {
@@ -146,12 +159,7 @@ impl FunctionInfo {
146159
pub struct DynamicScriptFunction {
147160
pub info: FunctionInfo,
148161
// TODO: info about the function, this is hard right now because of non 'static lifetimes in wrappers, we can't use TypePath etc
149-
func: Arc<
150-
dyn Fn(CallerContext, WorldCallbackAccess, Vec<ScriptValue>) -> ScriptValue
151-
+ Send
152-
+ Sync
153-
+ 'static,
154-
>,
162+
func: Arc<dyn Fn(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static>,
155163
}
156164

157165
impl PartialEq for DynamicScriptFunction {
@@ -167,10 +175,7 @@ pub struct DynamicScriptFunctionMut {
167175
func: Arc<
168176
RwLock<
169177
// I'd rather consume an option or something instead of having the RWLock but I just wanna get this release out
170-
dyn FnMut(CallerContext, WorldCallbackAccess, Vec<ScriptValue>) -> ScriptValue
171-
+ Send
172-
+ Sync
173-
+ 'static,
178+
dyn FnMut(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
174179
>,
175180
>,
176181
}
@@ -188,13 +193,11 @@ impl DynamicScriptFunction {
188193
pub fn call<I: IntoIterator<Item = ScriptValue>>(
189194
&self,
190195
args: I,
191-
world: WorldGuard,
192-
context: CallerContext,
196+
context: FunctionCallContext,
193197
) -> Result<ScriptValue, InteropError> {
194198
let args = args.into_iter().collect::<Vec<_>>();
195-
let world_callback_access = WorldCallbackAccess::from_guard(world.clone());
196199
// should we be inlining call errors into the return value?
197-
let return_val = (self.func)(context, world_callback_access, args);
200+
let return_val = (self.func)(context, args);
198201
match return_val {
199202
ScriptValue::Error(e) => Err(InteropError::function_interop_error(
200203
self.name(),
@@ -237,14 +240,12 @@ impl DynamicScriptFunctionMut {
237240
pub fn call<I: IntoIterator<Item = ScriptValue>>(
238241
&self,
239242
args: I,
240-
world: WorldGuard,
241-
context: CallerContext,
243+
context: FunctionCallContext,
242244
) -> Result<ScriptValue, InteropError> {
243245
let args = args.into_iter().collect::<Vec<_>>();
244-
let world_callback_access = WorldCallbackAccess::from_guard(world.clone());
245246
// should we be inlining call errors into the return value?
246247
let mut write = self.func.write();
247-
let return_val = (write)(context, world_callback_access, args);
248+
let return_val = (write)(context, args);
248249
match return_val {
249250
ScriptValue::Error(e) => Err(InteropError::function_interop_error(
250251
self.name(),
@@ -297,10 +298,7 @@ impl std::fmt::Debug for DynamicScriptFunctionMut {
297298

298299
impl<F> From<F> for DynamicScriptFunction
299300
where
300-
F: Fn(CallerContext, WorldCallbackAccess, Vec<ScriptValue>) -> ScriptValue
301-
+ Send
302-
+ Sync
303-
+ 'static,
301+
F: Fn(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
304302
{
305303
fn from(fn_: F) -> Self {
306304
DynamicScriptFunction {
@@ -313,10 +311,7 @@ where
313311

314312
impl<F> From<F> for DynamicScriptFunctionMut
315313
where
316-
F: FnMut(CallerContext, WorldCallbackAccess, Vec<ScriptValue>) -> ScriptValue
317-
+ Send
318-
+ Sync
319-
+ 'static,
314+
F: FnMut(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
320315
{
321316
fn from(fn_: F) -> Self {
322317
DynamicScriptFunctionMut {
@@ -527,52 +522,43 @@ macro_rules! impl_script_function {
527522
// FnMut(T1...Tn) -> O
528523
impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : -> O => O );
529524

530-
// Fn(WorldCallbackAccess, T1...Tn) -> O
531-
impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : ,(callback: WorldCallbackAccess) -> O => O);
532-
// FnMut(WorldCallbackAccess, T1...Tn) -> O
533-
impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : ,(callback: WorldCallbackAccess) -> O => O);
534-
535-
// Fn(CallerContext, WorldCallbackAccess, T1...Tn) -> O
536-
impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : (context: CallerContext),(callback: WorldCallbackAccess) -> O => O);
537-
// FnMut(CallerContext, WorldCallbackAccess, T1...Tn) -> O
538-
impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : (context: CallerContext),(callback: WorldCallbackAccess) -> O => O);
525+
// Fn(CallerContext, T1...Tn) -> O
526+
impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : (context: FunctionCallContext) -> O => O);
527+
// FnMut(FunctionCallContext, T1...Tn) -> O
528+
impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : (context: FunctionCallContext) -> O => O);
539529

540530
// Fn(T1...Tn) -> Result<O, InteropError>
541531
impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : -> O => Result<O, InteropError> where s);
542532
// FnMut(T1...Tn) -> Result<O, InteropError>
543533
impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : -> O => Result<O, InteropError> where s);
544534

545-
// Fn(WorldCallbackAccess, T1...Tn) -> Result<O, InteropError>
546-
impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : ,(callback: WorldCallbackAccess) -> O => Result<O, InteropError> where s);
547-
// FnMut(WorldCallbackAccess, T1...Tn) -> Result<O, InteropError>
548-
impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : ,(callback: WorldCallbackAccess) -> O => Result<O, InteropError> where s);
549-
550-
// Fn(CallerContext, WorldCallbackAccess, T1...Tn) -> Result<O, InteropError>
551-
impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : (context: CallerContext),(callback: WorldCallbackAccess) -> O => Result<O, InteropError> where s);
552-
// FnMut(CallerContext, WorldCallbackAccess, T1...Tn) -> Result<O, InteropError>
553-
impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : (context: CallerContext),(callback: WorldCallbackAccess) -> O => Result<O, InteropError> where s);
535+
// Fn(FunctionCallContext, WorldGuard<'w>, T1...Tn) -> Result<O, InteropError>
536+
impl_script_function!(@ ScriptFunction Fn DynamicScriptFunction into_dynamic_script_function $( $param ),* : (context: FunctionCallContext)-> O => Result<O, InteropError> where s);
537+
// FnMut(FunctionCallContext, WorldGuard<'w>, T1...Tn) -> Result<O, InteropError>
538+
impl_script_function!(@ ScriptFunctionMut FnMut DynamicScriptFunctionMut into_dynamic_script_function_mut $( $param ),* : (context: FunctionCallContext) -> O => Result<O, InteropError> where s);
554539

555540

556541
};
557542

558-
(@ $trait_type:ident $fn_type:ident $dynamic_type:ident $trait_fn_name:ident $( $param:ident ),* : $(($context:ident: $contextty:ty))? $(,($callback:ident: $callbackty:ty))? -> O => $res:ty $(where $out:ident)?) => {
543+
(@ $trait_type:ident $fn_type:ident $dynamic_type:ident $trait_fn_name:ident $( $param:ident ),* : $(($context:ident: $contextty:ty))? -> O => $res:ty $(where $out:ident)?) => {
559544
#[allow(non_snake_case)]
560545
impl<
561546
'env,
547+
'w : 'static,
562548
$( $param: FromScript, )*
563549
O,
564550
F
565551
> $trait_type<'env,
566-
fn( $($contextty,)? $( $callbackty, )? $($param ),* ) -> $res
552+
fn( $($contextty,)? $($param ),* ) -> $res
567553
> for F
568554
where
569555
O: IntoScript + TypePath + GetOwnership,
570-
F: $fn_type( $($contextty,)? $( $callbackty, )? $($param ),* ) -> $res + Send + Sync + 'static,
571-
$( $param::This<'env>: Into<$param>,)*
556+
F: $fn_type( $($contextty,)? $($param ),* ) -> $res + Send + Sync + 'static ,
557+
$( $param::This<'w>: Into<$param>,)*
572558
{
573559
#[allow(unused_mut,unused_variables)]
574560
fn $trait_fn_name(mut self) -> $dynamic_type {
575-
let func = (move |caller_context: CallerContext, world: WorldCallbackAccess, args: Vec<ScriptValue> | {
561+
let func = (move |caller_context: FunctionCallContext, args: Vec<ScriptValue> | {
576562
let res: Result<ScriptValue, InteropError> = (|| {
577563
let expected_arg_count = count!($($param )*);
578564
if args.len() < expected_arg_count {
@@ -582,8 +568,7 @@ macro_rules! impl_script_function {
582568
}));
583569
}
584570
$( let $context = caller_context; )?
585-
$( let $callback = world.clone(); )?
586-
let world = world.try_read()?;
571+
let world = caller_context.world()?;
587572
world.begin_access_scope()?;
588573
let ret = {
589574
let mut current_arg = 0;
@@ -593,7 +578,7 @@ macro_rules! impl_script_function {
593578
let $param = <$param>::from_script(arg_iter.next().expect("invariant"), world.clone())
594579
.map_err(|e| InteropError::function_arg_conversion_error(current_arg.to_string(), e))?;
595580
)*
596-
let out = self( $( $context,)? $( $callback, )? $( $param.into(), )* );
581+
let out = self( $( $context,)? $( $param.into(), )* );
597582
$(
598583
let $out = out?;
599584
let out = $out;

‎crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,8 @@ impl DisplayWithWorld for ComponentId {
373373
fn display_with_world(&self, world: WorldGuard) -> String {
374374
let component_name = world
375375
.as_unsafe_world_cell()
376-
.components()
377-
.get_info(*self)
376+
.ok()
377+
.and_then(|c| c.components().get_info(*self))
378378
.map(|info| info.name());
379379

380380
match component_name {
@@ -543,7 +543,7 @@ mod test {
543543

544544
use crate::bindings::{
545545
function::script_function::AppScriptFunctionRegistry, AppReflectAllocator,
546-
ReflectAllocationId, WorldAccessGuard,
546+
ReflectAllocationId,
547547
};
548548

549549
use super::*;
@@ -566,7 +566,7 @@ mod test {
566566
#[test]
567567
fn test_type_id() {
568568
let mut world = setup_world();
569-
let world = WorldGuard::new(WorldAccessGuard::new(&mut world));
569+
let world = WorldGuard::new(&mut world);
570570

571571
let type_id = TypeId::of::<usize>();
572572
assert_eq!(type_id.display_with_world(world.clone()), "usize");
@@ -585,7 +585,7 @@ mod test {
585585
#[test]
586586
fn test_reflect_base_type() {
587587
let mut world = setup_world();
588-
let world = WorldGuard::new(WorldAccessGuard::new(&mut world));
588+
let world = WorldGuard::new(&mut world);
589589

590590
let type_id = TypeId::of::<usize>();
591591

@@ -621,7 +621,7 @@ mod test {
621621
fn test_reflect_reference() {
622622
let mut world = setup_world();
623623

624-
let world = WorldGuard::new(WorldAccessGuard::new(&mut world));
624+
let world = WorldGuard::new(&mut world);
625625

626626
let type_id = TypeId::of::<usize>();
627627

‎crates/bevy_mod_scripting_core/src/bindings/query.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::{ReflectReference, WorldAccessGuard, WorldCallbackAccess};
1+
use super::{ReflectReference, WorldAccessGuard};
22
use crate::{error::InteropError, with_global_access};
33
use bevy::{
44
ecs::{component::ComponentId, entity::Entity},
@@ -152,23 +152,23 @@ pub struct ScriptQueryResult {
152152
pub components: Vec<ReflectReference>,
153153
}
154154

155-
impl WorldCallbackAccess {
156-
pub fn query(
157-
&self,
158-
query: ScriptQueryBuilder,
159-
) -> Result<VecDeque<ScriptQueryResult>, InteropError> {
160-
// find the set of components
161-
self.try_read().and_then(|world| world.query(query))
162-
}
163-
}
155+
// impl WorldCallbackAccess {
156+
// pub fn query(
157+
// &self,
158+
// query: ScriptQueryBuilder,
159+
// ) -> Result<VecDeque<ScriptQueryResult>, InteropError> {
160+
// // find the set of components
161+
// self.try_read().and_then(|world| world.query(query))
162+
// }
163+
// }
164164

165165
impl WorldAccessGuard<'_> {
166166
pub fn query(
167167
&self,
168168
query: ScriptQueryBuilder,
169169
) -> Result<VecDeque<ScriptQueryResult>, InteropError> {
170170
with_global_access!(self.0.accesses, "Could not query", {
171-
let world = unsafe { self.as_unsafe_world_cell().world_mut() };
171+
let world = unsafe { self.as_unsafe_world_cell()?.world_mut() };
172172
let mut dynamic_query = QueryBuilder::<EntityRef>::new(world);
173173

174174
// we don't actually want to fetch the data for components now, only figure out

‎crates/bevy_mod_scripting_core/src/bindings/reference.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ impl ReflectReference {
139139
}
140140

141141
pub fn new_resource_ref<T: Resource>(world: WorldGuard) -> Result<Self, InteropError> {
142-
let reflect_id = ReflectAccessId::for_resource::<T>(&world.as_unsafe_world_cell())?;
142+
let reflect_id = ReflectAccessId::for_resource::<T>(&world.as_unsafe_world_cell()?)?;
143143
Ok(Self {
144144
base: ReflectBaseType {
145145
type_id: TypeId::of::<T>(),
@@ -153,7 +153,7 @@ impl ReflectReference {
153153
entity: Entity,
154154
world: WorldGuard,
155155
) -> Result<Self, InteropError> {
156-
let reflect_id = ReflectAccessId::for_component::<T>(&world.as_unsafe_world_cell())?;
156+
let reflect_id = ReflectAccessId::for_component::<T>(&world.as_unsafe_world_cell()?)?;
157157
Ok(Self {
158158
base: ReflectBaseType {
159159
type_id: TypeId::of::<T>(),
@@ -323,7 +323,7 @@ impl ReflectReference {
323323
.base
324324
.base_id
325325
.clone()
326-
.into_ptr(world.as_unsafe_world_cell())
326+
.into_ptr(world.as_unsafe_world_cell()?)
327327
.ok_or_else(|| InteropError::unregistered_base(self.base.clone()))?;
328328

329329
// (Ptr) Safety: we use the same type_id to both
@@ -377,7 +377,7 @@ impl ReflectReference {
377377
.base
378378
.base_id
379379
.clone()
380-
.into_ptr_mut(world.as_unsafe_world_cell())
380+
.into_ptr_mut(world.as_unsafe_world_cell()?)
381381
.ok_or_else(|| InteropError::unregistered_base(self.base.clone()))?;
382382

383383
// (Ptr) Safety: we use the same type_id to both
@@ -569,7 +569,7 @@ mod test {
569569
use bevy::prelude::{AppTypeRegistry, World};
570570

571571
use crate::bindings::{
572-
function::script_function::AppScriptFunctionRegistry, AppReflectAllocator, WorldAccessGuard,
572+
function::script_function::AppScriptFunctionRegistry, AppReflectAllocator,
573573
};
574574

575575
use super::*;
@@ -609,7 +609,7 @@ mod test {
609609
.spawn(Component(vec!["hello".to_owned(), "world".to_owned()]))
610610
.id();
611611

612-
let world_guard = WorldGuard::new(WorldAccessGuard::new(&mut world));
612+
let world_guard = WorldGuard::new(&mut world);
613613

614614
let mut component_ref =
615615
ReflectReference::new_component_ref::<Component>(entity, world_guard.clone())
@@ -689,7 +689,7 @@ mod test {
689689

690690
world.insert_resource(Resource(vec!["hello".to_owned(), "world".to_owned()]));
691691

692-
let world_guard = WorldGuard::new(WorldAccessGuard::new(&mut world));
692+
let world_guard = WorldGuard::new(&mut world);
693693

694694
let mut resource_ref = ReflectReference::new_resource_ref::<Resource>(world_guard.clone())
695695
.expect("could not create resource reference");
@@ -768,7 +768,7 @@ mod test {
768768

769769
let value = Component(vec!["hello".to_owned(), "world".to_owned()]);
770770

771-
let world_guard = WorldGuard::new(WorldAccessGuard::new(&mut world));
771+
let world_guard = WorldGuard::new(&mut world);
772772
let allocator = world_guard.allocator();
773773
let mut allocator_write = allocator.write();
774774
let mut allocation_ref = ReflectReference::new_allocated(value, &mut allocator_write);

0 commit comments

Comments
 (0)