From a9ac2e4dc61c58a7df22f963d699865d627d136b Mon Sep 17 00:00:00 2001 From: Tim Yuen Date: Sat, 12 Aug 2023 03:24:53 -0400 Subject: [PATCH 1/2] Add ability to annotate default values in GDScript. This does not allow for actually passing default values. --- .../derive_godot_class/property/field_var.rs | 9 +---- godot-macros/src/godot_api.rs | 37 ++++++++++++++++--- godot-macros/src/method_registration/mod.rs | 14 +++++++ .../method_registration/register_method.rs | 17 +++++++-- itest/godot/ManualFfiTests.gd | 9 +++++ itest/rust/src/func_test.rs | 26 +++++++++++++ 6 files changed, 96 insertions(+), 16 deletions(-) diff --git a/godot-macros/src/derive_godot_class/property/field_var.rs b/godot-macros/src/derive_godot_class/property/field_var.rs index 2d80a0382..22b10c05c 100644 --- a/godot-macros/src/derive_godot_class/property/field_var.rs +++ b/godot-macros/src/derive_godot_class/property/field_var.rs @@ -189,13 +189,8 @@ impl GetterSetterImpl { }; let signature = util::parse_signature(signature); - let export_token = make_method_registration( - class_name, - FuncDefinition { - func: signature, - rename: None, - }, - ); + let export_token = + make_method_registration(class_name, FuncDefinition::from_signature(signature)); Self { function_name, diff --git a/godot-macros/src/godot_api.rs b/godot-macros/src/godot_api.rs index d6ba802f3..1cff5de18 100644 --- a/godot-macros/src/godot_api.rs +++ b/godot-macros/src/godot_api.rs @@ -49,7 +49,10 @@ pub fn transform(input_decl: Declaration) -> Result { /// Attribute for user-declared function enum BoundAttrType { - Func { rename: Option }, + Func { + rename: Option, + default_params: Option>, + }, Signal(AttributeValue), Const(AttributeValue), } @@ -232,10 +235,17 @@ fn process_godot_fns(decl: &mut Impl) -> Result<(Vec, Vec { + BoundAttrType::Func { + rename, + default_params, + } => { // Signatures are the same thing without body let sig = util::reduce_to_signature(method); - func_definitions.push(FuncDefinition { func: sig, rename }); + func_definitions.push(FuncDefinition { + func: sig, + rename, + default_params, + }); } BoundAttrType::Signal(ref _attr_val) => { if method.return_ty.is_some() { @@ -312,17 +322,32 @@ where let new_found = match attr_name { name if name == "func" => { - // TODO you-win (August 8, 2023): handle default values here as well? - // Safe unwrap since #[func] must be present if we got to this point let mut parser = KvParser::parse(attributes, "func")?.unwrap(); let rename = parser.handle_expr("rename")?.map(|ts| ts.to_string()); + let default_params: Option> = + if let Some(mut list_parser) = parser.handle_array("defaults")? { + let mut defaults = vec![]; + + while let Ok(expr) = list_parser.next_expr() { + defaults.push(expr.to_string()); + } + + list_parser.finish()?; + + Some(defaults) + } else { + None + }; Some(BoundAttr { attr_name: attr_name.clone(), index, - ty: BoundAttrType::Func { rename }, + ty: BoundAttrType::Func { + rename, + default_params, + }, }) } name if name == "signal" => { diff --git a/godot-macros/src/method_registration/mod.rs b/godot-macros/src/method_registration/mod.rs index f07a82e51..a52dcadd4 100644 --- a/godot-macros/src/method_registration/mod.rs +++ b/godot-macros/src/method_registration/mod.rs @@ -33,6 +33,20 @@ pub struct FuncDefinition { pub func: venial::Function, /// The name the function will be exposed as in Godot. If `None`, the Rust function name is used. pub rename: Option, + // TODO you-win August 12, 2023: right now, this is an all-or-nothing operation + // either all params must have default args or no params have default args + /// Default parameters in sequential order. + pub default_params: Option>, +} + +impl FuncDefinition { + pub fn from_signature(signature: venial::Function) -> Self { + FuncDefinition { + func: signature, + rename: None, + default_params: None, + } + } } /// Returns a closure expression that forwards the parameters to the Rust instance. diff --git a/godot-macros/src/method_registration/register_method.rs b/godot-macros/src/method_registration/register_method.rs index f3f83d581..156f658e7 100644 --- a/godot-macros/src/method_registration/register_method.rs +++ b/godot-macros/src/method_registration/register_method.rs @@ -10,7 +10,7 @@ use crate::method_registration::{ }; use crate::util; use proc_macro2::{Ident, TokenStream}; -use quote::quote; +use quote::{quote, ToTokens}; /// Generates code that registers the specified method for the given class. pub fn make_method_registration( @@ -39,10 +39,18 @@ pub fn make_method_registration( method_name.to_string() }; let param_ident_strs = param_idents.iter().map(|ident| ident.to_string()); + // TODO you-win August 12, 2023: Generate *_ex methods from this somehow? + // TODO you-win August 12, 2023: Definitely not working, causes Godot to hang during itests + let default_params = func_definition.default_params.map_or(vec![], |params| { + params + .into_iter() + .map(|param| param.to_token_stream()) + .collect::>() + }); quote! { { - use ::godot::obj::GodotClass; + use ::godot::obj::{Gd, GodotClass}; use ::godot::builtin::meta::registration::method::MethodInfo; use ::godot::builtin::{StringName, Variant}; use ::godot::sys; @@ -54,6 +62,8 @@ pub fn make_method_registration( let varcall_func = #varcall_func; let ptrcall_func = #ptrcall_func; + let default_params = vec![ #( Variant::from( #default_params ) ),* ]; + // SAFETY: // `get_varcall_func` upholds all the requirements for `call_func`. // `get_ptrcall_func` upholds all the requirements for `ptrcall_func` @@ -67,7 +77,8 @@ pub fn make_method_registration( &[ #( #param_ident_strs ),* ], - Vec::new() + // Vec::new() + default_params ) }; diff --git a/itest/godot/ManualFfiTests.gd b/itest/godot/ManualFfiTests.gd index 9563cad44..b1392b01a 100644 --- a/itest/godot/ManualFfiTests.gd +++ b/itest/godot/ManualFfiTests.gd @@ -304,3 +304,12 @@ func test_func_rename(): assert_eq(func_rename.has_method("renamed_static"), false) assert_eq(func_rename.has_method("spell_static"), true) assert_eq(func_rename.spell_static(), "static") + +func test_default_params_primitives(): + var tester := DefaultParamsPrimitives.new() + + assert_eq(tester.add_ints(), 3) + assert_eq(tester.add_ints(5, 6), 11) + + assert_eq(tester.pass_string(), "hello") + assert_eq(tester.pass_string("world"), "world") diff --git a/itest/rust/src/func_test.rs b/itest/rust/src/func_test.rs index b29019cf5..18876f7fb 100644 --- a/itest/rust/src/func_test.rs +++ b/itest/rust/src/func_test.rs @@ -46,3 +46,29 @@ impl RefCountedVirtual for FuncRename { Self } } + +#[derive(GodotClass)] +#[class(base=RefCounted)] +struct DefaultParamsPrimitives; + +#[godot_api] +impl DefaultParamsPrimitives { + #[func(defaults = [1, 2])] + fn add_ints(&self, a: Variant, b: Variant) -> i32 { + println!("{:?}", a.get_type()); + println!("{:?}", b.get_type()); + (if a.is_nil() { 1 } else { a.to::() }) + (if b.is_nil() { 2 } else { b.to::() }) + } + + #[func(defaults = ["hello"])] + fn pass_string(&self, text: GodotString) -> GodotString { + text + } +} + +#[godot_api] +impl RefCountedVirtual for DefaultParamsPrimitives { + fn init(_base: Base) -> Self { + Self + } +} From e0734829f4f5399d80fbed65b12504f26696bcd0 Mon Sep 17 00:00:00 2001 From: Tim Yuen Date: Sun, 13 Aug 2023 02:11:01 -0400 Subject: [PATCH 2/2] pass arg count when using varcall, check arg tuple index against arg count, update default func param tests to test happy path first --- godot-core/src/builtin/meta/signature.rs | 13 ++++-- godot-macros/src/method_registration/mod.rs | 40 +++++++++---------- .../method_registration/register_method.rs | 28 ++++++++++--- itest/godot/ManualFfiTests.gd | 3 +- itest/rust/src/func_test.rs | 8 ++-- 5 files changed, 59 insertions(+), 33 deletions(-) diff --git a/godot-core/src/builtin/meta/signature.rs b/godot-core/src/builtin/meta/signature.rs index 4d856e41f..8c077801b 100644 --- a/godot-core/src/builtin/meta/signature.rs +++ b/godot-core/src/builtin/meta/signature.rs @@ -21,6 +21,7 @@ pub trait VarcallSignatureTuple: PtrcallSignatureTuple { unsafe fn varcall( instance_ptr: sys::GDExtensionClassInstancePtr, args_ptr: *const sys::GDExtensionConstVariantPtr, + arg_count: sys::GDExtensionInt, ret: sys::GDExtensionVariantPtr, err: *mut sys::GDExtensionCallError, func: fn(sys::GDExtensionClassInstancePtr, Self::Params) -> Self::Ret, @@ -75,7 +76,7 @@ macro_rules! impl_varcall_signature_for_tuple { #[allow(unused_variables)] impl<$R, $($Pn,)*> VarcallSignatureTuple for ($R, $($Pn,)*) where $R: VariantMetadata + ToVariant + sys::GodotFuncMarshal + Debug, - $( $Pn: VariantMetadata + FromVariant + sys::GodotFuncMarshal + Debug, )* + $( $Pn: VariantMetadata + FromVariant + sys::GodotFuncMarshal + Debug + Default, )* { const PARAM_COUNT: usize = $PARAM_COUNT; @@ -106,6 +107,7 @@ macro_rules! impl_varcall_signature_for_tuple { unsafe fn varcall( instance_ptr: sys::GDExtensionClassInstancePtr, args_ptr: *const sys::GDExtensionConstVariantPtr, + arg_count: sys::GDExtensionInt, ret: sys::GDExtensionVariantPtr, err: *mut sys::GDExtensionCallError, func: fn(sys::GDExtensionClassInstancePtr, Self::Params) -> Self::Ret, @@ -114,7 +116,7 @@ macro_rules! impl_varcall_signature_for_tuple { $crate::out!("varcall: {}", method_name); let args = ($( - unsafe { varcall_arg::<$Pn, $n>(args_ptr, method_name) }, + unsafe { varcall_arg::<$Pn, $n>(args_ptr, arg_count as isize, method_name) }, )*) ; varcall_return::<$R>(func(instance_ptr, args), ret, err) @@ -163,10 +165,14 @@ macro_rules! impl_ptrcall_signature_for_tuple { /// /// # Safety /// - It must be safe to dereference the pointer at `args_ptr.offset(N)` . -unsafe fn varcall_arg( +unsafe fn varcall_arg( args_ptr: *const sys::GDExtensionConstVariantPtr, + arg_count: isize, method_name: &str, ) -> P { + if arg_count >= N { + return P::default(); + } let variant = &*(*args_ptr.offset(N) as *mut Variant); // TODO from_var_sys P::try_from_variant(variant) .unwrap_or_else(|_| param_error::

(method_name, N as i32, variant)) @@ -199,6 +205,7 @@ unsafe fn ptrcall_arg( method_name: &str, call_type: sys::PtrcallType, ) -> P { + println!("ptrcall - {method_name}"); P::try_from_arg(sys::force_mut_ptr(*args_ptr.offset(N)), call_type) .unwrap_or_else(|e| param_error::

(method_name, N as i32, &e)) } diff --git a/godot-macros/src/method_registration/mod.rs b/godot-macros/src/method_registration/mod.rs index a52dcadd4..8015a5bfc 100644 --- a/godot-macros/src/method_registration/mod.rs +++ b/godot-macros/src/method_registration/mod.rs @@ -33,13 +33,13 @@ pub struct FuncDefinition { pub func: venial::Function, /// The name the function will be exposed as in Godot. If `None`, the Rust function name is used. pub rename: Option, - // TODO you-win August 12, 2023: right now, this is an all-or-nothing operation // either all params must have default args or no params have default args /// Default parameters in sequential order. pub default_params: Option>, } impl FuncDefinition { + /// Convenience function for struct creation with _only_ a signature. pub fn from_signature(signature: venial::Function) -> Self { FuncDefinition { func: signature, @@ -175,22 +175,22 @@ fn make_ptrcall_invocation( } } -/// Generate code for a `varcall()` call expression. -fn make_varcall_invocation( - method_name: &Ident, - sig_tuple: &TokenStream, - wrapped_method: &TokenStream, -) -> TokenStream { - let method_name_str = method_name.to_string(); - - quote! { - <#sig_tuple as ::godot::builtin::meta::VarcallSignatureTuple>::varcall( - instance_ptr, - args, - ret, - err, - #wrapped_method, - #method_name_str, - ) - } -} +//// Generate code for a `varcall()` call expression. +// fn make_varcall_invocation( +// method_name: &Ident, +// sig_tuple: &TokenStream, +// wrapped_method: &TokenStream, +// ) -> TokenStream { +// let method_name_str = method_name.to_string(); + +// quote! { +// <#sig_tuple as ::godot::builtin::meta::VarcallSignatureTuple>::varcall( +// instance_ptr, +// args, +// ret, +// err, +// #wrapped_method, +// #method_name_str, +// ) +// } +// } diff --git a/godot-macros/src/method_registration/register_method.rs b/godot-macros/src/method_registration/register_method.rs index 156f658e7..3b530fc03 100644 --- a/godot-macros/src/method_registration/register_method.rs +++ b/godot-macros/src/method_registration/register_method.rs @@ -5,8 +5,11 @@ */ use crate::method_registration::{ - get_signature_info, make_forwarding_closure, make_method_flags, make_ptrcall_invocation, - make_varcall_invocation, + get_signature_info, + make_forwarding_closure, + make_method_flags, + make_ptrcall_invocation, + // make_varcall_invocation, }; use crate::util; use proc_macro2::{Ident, TokenStream}; @@ -100,7 +103,8 @@ fn make_varcall_func( sig_tuple: &TokenStream, wrapped_method: &TokenStream, ) -> TokenStream { - let invocation = make_varcall_invocation(method_name, sig_tuple, wrapped_method); + // let invocation = make_varcall_invocation(method_name, sig_tuple, wrapped_method); + let method_name_str = method_name.to_string(); quote! { { @@ -108,15 +112,29 @@ fn make_varcall_func( _method_data: *mut std::ffi::c_void, instance_ptr: sys::GDExtensionClassInstancePtr, args: *const sys::GDExtensionConstVariantPtr, - _arg_count: sys::GDExtensionInt, + arg_count: sys::GDExtensionInt, ret: sys::GDExtensionVariantPtr, err: *mut sys::GDExtensionCallError, ) { + // let success = ::godot::private::handle_panic( + // || stringify!(#method_name), + // || #invocation + // ); let success = ::godot::private::handle_panic( || stringify!(#method_name), - || #invocation + || <#sig_tuple as ::godot::builtin::meta::VarcallSignatureTuple>::varcall( + instance_ptr, + args, + arg_count, + ret, + err, + #wrapped_method, + #method_name_str + ) ); + // println!("make_varcall_func arg_count - {_arg_count:?}"); + if success.is_none() { // Signal error and set return type to Nil (*err).error = sys::GDEXTENSION_CALL_ERROR_INVALID_METHOD; // no better fitting enum? diff --git a/itest/godot/ManualFfiTests.gd b/itest/godot/ManualFfiTests.gd index b1392b01a..cd3c3d02d 100644 --- a/itest/godot/ManualFfiTests.gd +++ b/itest/godot/ManualFfiTests.gd @@ -308,8 +308,9 @@ func test_func_rename(): func test_default_params_primitives(): var tester := DefaultParamsPrimitives.new() - assert_eq(tester.add_ints(), 3) assert_eq(tester.add_ints(5, 6), 11) + assert_eq(tester.add_ints(5), 5) + assert_eq(tester.add_ints(), 0) assert_eq(tester.pass_string(), "hello") assert_eq(tester.pass_string("world"), "world") diff --git a/itest/rust/src/func_test.rs b/itest/rust/src/func_test.rs index 18876f7fb..749f7487d 100644 --- a/itest/rust/src/func_test.rs +++ b/itest/rust/src/func_test.rs @@ -47,6 +47,8 @@ impl RefCountedVirtual for FuncRename { } } +// TODO you-win August 12, 2023: impl to/from variant for option? + #[derive(GodotClass)] #[class(base=RefCounted)] struct DefaultParamsPrimitives; @@ -54,10 +56,8 @@ struct DefaultParamsPrimitives; #[godot_api] impl DefaultParamsPrimitives { #[func(defaults = [1, 2])] - fn add_ints(&self, a: Variant, b: Variant) -> i32 { - println!("{:?}", a.get_type()); - println!("{:?}", b.get_type()); - (if a.is_nil() { 1 } else { a.to::() }) + (if b.is_nil() { 2 } else { b.to::() }) + fn add_ints(&self, a: i32, b: i32) -> i32 { + a + b } #[func(defaults = ["hello"])]