Skip to content

Commit dbfef74

Browse files
makspllmakspll
authored
feat: Add optional arguments to script functions (#225)
* implement optional arguments * bump rhai version * fmt
1 parent 361ca78 commit dbfef74

File tree

9 files changed

+351
-33
lines changed

9 files changed

+351
-33
lines changed

‎crates/bevy_mod_scripting_core/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ rhai_impls = ["rhai"]
2626

2727
[dependencies]
2828
mlua = { version = "0.10", default-features = false, optional = true }
29-
rhai = { git = "https://github.com/rhaiscript/rhai", rev = "4ead53eb40f4a18d6f827609041ef1c742f04799", default-features = false, features = [
29+
rhai = { version = "1.21", default-features = false, features = [
3030
"sync",
3131
], optional = true }
3232

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//! Trait implementations to help with function dispatch.
2+
3+
use std::{ffi::OsString, path::PathBuf};
4+
5+
use crate::bindings::{script_value::ScriptValue, ReflectReference};
6+
7+
use super::{
8+
from::{FromScript, Mut, Ref, Val},
9+
into::IntoScript,
10+
script_function::{DynamicScriptFunction, DynamicScriptFunctionMut, GetInnerTypeDependencies},
11+
};
12+
13+
/// Marker trait for types that can be used as arguments to a script function.
14+
pub trait ScriptArgument: ArgInfo + FromScript + GetInnerTypeDependencies {}
15+
impl<T: ArgInfo + FromScript + GetInnerTypeDependencies> ScriptArgument for T {}
16+
17+
/// Marker trait for types that can be used as return values from a script function.
18+
pub trait ScriptReturn: IntoScript + GetInnerTypeDependencies {}
19+
20+
/// Describes an argument to a script function. Provides necessary information for the function to handle dispatch.
21+
pub trait ArgInfo {
22+
fn default_value() -> Option<ScriptValue> {
23+
None
24+
}
25+
}
26+
27+
impl ArgInfo for ScriptValue {}
28+
29+
impl ArgInfo for () {
30+
fn default_value() -> Option<ScriptValue> {
31+
Some(ScriptValue::Unit)
32+
}
33+
}
34+
35+
macro_rules! impl_arg_info {
36+
($($ty:ty),*) => {
37+
$(
38+
impl ArgInfo for $ty {}
39+
)*
40+
};
41+
}
42+
43+
impl_arg_info!(bool, i8, i16, i32, i64, i128, u8, u16, u32, u64, u128, f32, f64, usize, isize);
44+
45+
impl_arg_info!(String, PathBuf, OsString);
46+
47+
impl_arg_info!(char);
48+
49+
impl_arg_info!(ReflectReference);
50+
51+
impl<T> ArgInfo for Val<T> {}
52+
impl<T> ArgInfo for Ref<'_, T> {}
53+
impl<T> ArgInfo for Mut<'_, T> {}
54+
55+
impl<T> ArgInfo for Option<T> {
56+
fn default_value() -> Option<ScriptValue> {
57+
Some(ScriptValue::Unit)
58+
}
59+
}
60+
61+
impl<T> ArgInfo for Vec<T> {}
62+
impl<T, const N: usize> ArgInfo for [T; N] {}
63+
64+
impl_arg_info!(DynamicScriptFunction, DynamicScriptFunctionMut);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
pub mod arg_info;
12
pub mod from;
23
pub mod from_ref;
34
pub mod into;

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

Lines changed: 92 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use super::{from::FromScript, into::IntoScript, namespace::Namespace};
2+
use crate::bindings::function::arg_info::ArgInfo;
23
use crate::{
34
bindings::{
45
function::from::{Mut, Ref, Val},
@@ -9,20 +10,17 @@ use crate::{
910
};
1011
use bevy::{
1112
prelude::{Reflect, Resource},
12-
reflect::{
13-
func::{args::GetOwnership, FunctionError},
14-
FromReflect, GetTypeRegistration, TypePath, TypeRegistry, Typed,
15-
},
13+
reflect::{func::FunctionError, FromReflect, GetTypeRegistration, TypeRegistry, Typed},
1614
};
1715
use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard};
1816
use std::borrow::Cow;
19-
use std::collections::HashMap;
17+
use std::collections::{HashMap, VecDeque};
2018
use std::hash::Hash;
2119
use std::ops::{Deref, DerefMut};
2220
use std::sync::Arc;
2321

2422
#[diagnostic::on_unimplemented(
25-
message = "Only functions with all arguments impplementing FromScript and return values supporting IntoScript are supported. Registering functions also requires they implement GetInnerTypeDependencies",
23+
message = "This function does not fulfil the requirements to be a script callable function. All arguments must implement the ScriptArgument trait and all return values must implement the ScriptReturn trait",
2624
note = "If you're trying to return a non-primitive type, you might need to use Val<T> Ref<T> or Mut<T> wrappers"
2725
)]
2826
pub trait ScriptFunction<'env, Marker> {
@@ -159,7 +157,9 @@ impl FunctionInfo {
159157
pub struct DynamicScriptFunction {
160158
pub info: FunctionInfo,
161159
// TODO: info about the function, this is hard right now because of non 'static lifetimes in wrappers, we can't use TypePath etc
162-
func: Arc<dyn Fn(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static>,
160+
func: Arc<
161+
dyn Fn(FunctionCallContext, VecDeque<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
162+
>,
163163
}
164164

165165
impl PartialEq for DynamicScriptFunction {
@@ -175,7 +175,10 @@ pub struct DynamicScriptFunctionMut {
175175
func: Arc<
176176
RwLock<
177177
// I'd rather consume an option or something instead of having the RWLock but I just wanna get this release out
178-
dyn FnMut(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
178+
dyn FnMut(FunctionCallContext, VecDeque<ScriptValue>) -> ScriptValue
179+
+ Send
180+
+ Sync
181+
+ 'static,
179182
>,
180183
>,
181184
}
@@ -195,7 +198,7 @@ impl DynamicScriptFunction {
195198
args: I,
196199
context: FunctionCallContext,
197200
) -> Result<ScriptValue, InteropError> {
198-
let args = args.into_iter().collect::<Vec<_>>();
201+
let args = args.into_iter().collect::<VecDeque<_>>();
199202
// should we be inlining call errors into the return value?
200203
let return_val = (self.func)(context, args);
201204
match return_val {
@@ -242,7 +245,7 @@ impl DynamicScriptFunctionMut {
242245
args: I,
243246
context: FunctionCallContext,
244247
) -> Result<ScriptValue, InteropError> {
245-
let args = args.into_iter().collect::<Vec<_>>();
248+
let args = args.into_iter().collect::<VecDeque<_>>();
246249
// should we be inlining call errors into the return value?
247250
let mut write = self.func.write();
248251
let return_val = (write)(context, args);
@@ -298,7 +301,7 @@ impl std::fmt::Debug for DynamicScriptFunctionMut {
298301

299302
impl<F> From<F> for DynamicScriptFunction
300303
where
301-
F: Fn(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
304+
F: Fn(FunctionCallContext, VecDeque<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
302305
{
303306
fn from(fn_: F) -> Self {
304307
DynamicScriptFunction {
@@ -311,7 +314,7 @@ where
311314

312315
impl<F> From<F> for DynamicScriptFunctionMut
313316
where
314-
F: FnMut(FunctionCallContext, Vec<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
317+
F: FnMut(FunctionCallContext, VecDeque<ScriptValue>) -> ScriptValue + Send + Sync + 'static,
315318
{
316319
fn from(fn_: F) -> Self {
317320
DynamicScriptFunctionMut {
@@ -562,39 +565,50 @@ macro_rules! impl_script_function {
562565
impl<
563566
'env,
564567
'w : 'static,
565-
$( $param: FromScript, )*
568+
$( $param: FromScript + ArgInfo,)*
566569
O,
567570
F
568571
> $trait_type<'env,
569572
fn( $($contextty,)? $($param ),* ) -> $res
570573
> for F
571574
where
572-
O: IntoScript + TypePath + GetOwnership,
575+
O: IntoScript,
573576
F: $fn_type( $($contextty,)? $($param ),* ) -> $res + Send + Sync + 'static ,
574577
$( $param::This<'w>: Into<$param>,)*
575578
{
576579
#[allow(unused_mut,unused_variables)]
577580
fn $trait_fn_name(mut self) -> $dynamic_type {
578-
let func = (move |caller_context: FunctionCallContext, args: Vec<ScriptValue> | {
581+
let func = (move |caller_context: FunctionCallContext, mut args: VecDeque<ScriptValue> | {
579582
let res: Result<ScriptValue, InteropError> = (|| {
583+
let received_args_len = args.len();
580584
let expected_arg_count = count!($($param )*);
581-
if args.len() < expected_arg_count {
582-
return Err(InteropError::function_call_error(FunctionError::ArgCountMismatch{
583-
expected: expected_arg_count,
584-
received: args.len()
585-
}));
586-
}
585+
587586
$( let $context = caller_context; )?
588587
let world = caller_context.world()?;
589588
world.begin_access_scope()?;
589+
let mut current_arg = 0;
590+
591+
$(
592+
current_arg += 1;
593+
let $param = args.pop_front();
594+
let $param = match $param {
595+
Some($param) => $param,
596+
None => {
597+
if let Some(default) = <$param>::default_value() {
598+
default
599+
} else {
600+
return Err(InteropError::function_call_error(FunctionError::ArgCountMismatch{
601+
expected: expected_arg_count,
602+
received: received_args_len
603+
}));
604+
}
605+
}
606+
};
607+
let $param = <$param>::from_script($param, world.clone())
608+
.map_err(|e| InteropError::function_arg_conversion_error(current_arg.to_string(), e))?;
609+
)*
610+
590611
let ret = {
591-
let mut current_arg = 0;
592-
let mut arg_iter = args.into_iter();
593-
$(
594-
current_arg += 1;
595-
let $param = <$param>::from_script(arg_iter.next().expect("invariant"), world.clone())
596-
.map_err(|e| InteropError::function_arg_conversion_error(current_arg.to_string(), e))?;
597-
)*
598612
let out = self( $( $context,)? $( $param.into(), )* );
599613
$(
600614
let $out = out?;
@@ -638,10 +652,20 @@ bevy::utils::all_tuples!(impl_script_function_type_dependencies, 0, 13, T);
638652
#[cfg(test)]
639653
mod test {
640654
use super::*;
655+
656+
fn with_local_world<F: Fn()>(f: F) {
657+
let mut world = bevy::prelude::World::default();
658+
WorldGuard::with_static_guard(&mut world, |world| {
659+
ThreadWorldContainer.set_world(world).unwrap();
660+
f()
661+
});
662+
}
663+
641664
#[test]
642665
fn test_register_script_function() {
643666
let mut registry = ScriptFunctionRegistry::default();
644667
let fn_ = |a: usize, b: usize| a + b;
668+
645669
let namespace = Namespace::Global;
646670
registry.register(namespace, "test", fn_);
647671
let function = registry
@@ -652,6 +676,46 @@ mod test {
652676
assert_eq!(function.info.namespace(), namespace);
653677
}
654678

679+
#[test]
680+
fn test_optional_argument_not_required() {
681+
let fn_ = |a: usize, b: Option<usize>| a + b.unwrap_or(0);
682+
let script_function = fn_.into_dynamic_script_function();
683+
684+
with_local_world(|| {
685+
let out = script_function
686+
.call(vec![ScriptValue::from(1)], FunctionCallContext::default())
687+
.unwrap();
688+
689+
assert_eq!(out, ScriptValue::from(1));
690+
});
691+
}
692+
693+
#[test]
694+
fn test_invalid_amount_of_args_errors_nicely() {
695+
let fn_ = |a: usize, b: usize| a + b;
696+
let script_function = fn_.into_dynamic_script_function().with_name("my_fn");
697+
698+
with_local_world(|| {
699+
let out =
700+
script_function.call(vec![ScriptValue::from(1)], FunctionCallContext::default());
701+
702+
assert!(out.is_err());
703+
assert_eq!(
704+
out.unwrap_err().into_inner().unwrap(),
705+
InteropError::function_interop_error(
706+
"my_fn",
707+
Namespace::Global,
708+
InteropError::function_call_error(FunctionError::ArgCountMismatch {
709+
expected: 2,
710+
received: 1
711+
})
712+
)
713+
.into_inner()
714+
.unwrap()
715+
);
716+
});
717+
}
718+
655719
#[test]
656720
fn test_overloaded_script_function() {
657721
let mut registry = ScriptFunctionRegistry::default();

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,8 @@ impl<'w> WorldAccessGuard<'w> {
322322
}
323323

324324
/// Safey modify or insert a component by claiming and releasing global access.
325-
pub fn with_or_insert_component_mut<F, T, O>(&self,
325+
pub fn with_or_insert_component_mut<F, T, O>(
326+
&self,
326327
entity: Entity,
327328
f: F,
328329
) -> Result<O, InteropError>

0 commit comments

Comments
 (0)