From f24ec87bfe91b903aeacf699c4db8ae28f196dd3 Mon Sep 17 00:00:00 2001 From: astrale Date: Tue, 24 Jun 2025 15:50:50 +0200 Subject: [PATCH 1/3] adds default parameters parsing --- godot-core/src/registry/method.rs | 3 +- godot-macros/src/class/data_models/func.rs | 38 ++++++++++++++++++- .../src/class/data_models/inherent_impl.rs | 21 +++++++++- .../register_tests/default_parameters_test.rs | 27 +++++++++++++ itest/rust/src/register_tests/mod.rs | 1 + 5 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 itest/rust/src/register_tests/default_parameters_test.rs diff --git a/godot-core/src/registry/method.rs b/godot-core/src/registry/method.rs index c2393bc7d..49e8a6edc 100644 --- a/godot-core/src/registry/method.rs +++ b/godot-core/src/registry/method.rs @@ -59,12 +59,11 @@ impl ClassMethodInfo { ptrcall_func: sys::GDExtensionClassMethodPtrCall, method_flags: MethodFlags, param_names: &[&str], - // default_arguments: Vec, - not yet implemented + default_arguments: Vec, ) -> Self { let return_value = Ret::Via::return_info(); let arguments = Signature::::param_names(param_names); - let default_arguments = vec![]; // not yet implemented. assert!( default_arguments.len() <= arguments.len(), "cannot have more default arguments than arguments" diff --git a/godot-macros/src/class/data_models/func.rs b/godot-macros/src/class/data_models/func.rs index 0a91f7e50..45230d2d3 100644 --- a/godot-macros/src/class/data_models/func.rs +++ b/godot-macros/src/class/data_models/func.rs @@ -7,7 +7,7 @@ use crate::class::RpcAttr; use crate::util::{bail_fn, ident, safe_ident}; -use crate::{util, ParseResult}; +use crate::{bail, util, ParseResult}; use proc_macro2::{Group, Ident, TokenStream, TokenTree}; use quote::{format_ident, quote}; @@ -126,6 +126,9 @@ pub fn make_method_registration( .iter() .map(|ident| ident.to_string()); + let default_parameters = + validate_default_parameters(&func_definition.signature_info.default_parameters)?; + // Transport #[cfg] attrs to the FFI glue to ensure functions which were conditionally // removed from compilation don't cause errors. let cfg_attrs = util::extract_cfg_attrs(&func_definition.external_attributes) @@ -158,6 +161,9 @@ pub fn make_method_registration( &[ #( #param_ident_strs ),* ], + vec![ + #(::godot::builtin::Variant::from(#default_parameters)),* + ] ) }; @@ -175,6 +181,32 @@ pub fn make_method_registration( Ok(registration) } +fn validate_default_parameters( + default_parameters: &[Option], +) -> ParseResult> { + let mut res = vec![]; + let mut allowed = true; + for param in default_parameters.iter().rev() { + match (param, allowed) { + (Some(tk), true) => { + res.push(tk.clone()); // toreview: if we really care about it, we can use &mut sig_info and mem::take() as we don't use this later + } + (None, true) => { + allowed = false; + } + (None, false) => {} + (Some(tk), false) => { + return bail!( + tk, + "opt arguments are only allowed at the end of the argument list." + ); + } + } + } + res.reverse(); + Ok(res) +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- // Implementation @@ -199,6 +231,8 @@ pub struct SignatureInfo { /// /// Index points into original venial tokens (i.e. takes into account potential receiver params). pub modified_param_types: Vec<(usize, venial::TypeExpr)>, + /// Contains expressions of the default values of parameters. + pub default_parameters: Vec>, } impl SignatureInfo { @@ -210,6 +244,7 @@ impl SignatureInfo { param_types: vec![], return_type: quote! { () }, modified_param_types: vec![], + default_parameters: vec![], } } @@ -412,6 +447,7 @@ pub(crate) fn into_signature_info( param_types, return_type: ret_type, modified_param_types, + default_parameters: vec![], } } diff --git a/godot-macros/src/class/data_models/inherent_impl.rs b/godot-macros/src/class/data_models/inherent_impl.rs index 6f09e3f4f..32864e22c 100644 --- a/godot-macros/src/class/data_models/inherent_impl.rs +++ b/godot-macros/src/class/data_models/inherent_impl.rs @@ -304,9 +304,10 @@ fn process_godot_fns( }; // Clone might not strictly be necessary, but the 2 other callers of into_signature_info() are better off with pass-by-value. - let signature_info = + let mut signature_info = into_signature_info(signature.clone(), class_name, gd_self_parameter.is_some()); + signature_info.default_parameters = parse_default_parameters(&mut function.params)?; // For virtual methods, rename/mangle existing user method and create a new method with the original name, // which performs a dynamic dispatch. let registered_name = if func.is_virtual { @@ -688,6 +689,24 @@ fn parse_constant_attr( Ok(AttrParseResult::Constant(attr.value.clone())) } +fn parse_default_parameters( + params: &mut venial::Punctuated, +) -> ParseResult>> { + let mut res = vec![]; + for param in params.iter_mut() { + let typed_param = match &mut param.0 { + venial::FnParam::Receiver(_) => continue, + venial::FnParam::Typed(fn_typed_param) => fn_typed_param, + }; + let default = match KvParser::parse_remove(&mut typed_param.attributes, "opt")? { + None => None, + Some(mut parser) => Some(parser.handle_expr_required("default")?), + }; + res.push(default); + } + Ok(res) +} + fn bail_attr(attr_name: &Ident, msg: &str, method_name: &Ident) -> ParseResult { bail!(method_name, "#[{attr_name}]: {msg}") } diff --git a/itest/rust/src/register_tests/default_parameters_test.rs b/itest/rust/src/register_tests/default_parameters_test.rs new file mode 100644 index 000000000..7f269a645 --- /dev/null +++ b/itest/rust/src/register_tests/default_parameters_test.rs @@ -0,0 +1,27 @@ +use crate::framework::itest; +use godot::prelude::*; + +#[derive(GodotClass)] +#[class(init)] +struct HasDefaultParameters {} + +#[godot_api] +impl HasDefaultParameters { + #[func] + fn function_with_default_params( + required: i32, + #[opt(default = "test")] string: GString, + #[opt(default = 123)] integer: i32, + // #[opt(default=None)] object: Option>, + ) -> VariantArray { + varray![required, string, integer,] + } +} + +#[itest] +fn tests_default_parameters() { + let mut obj = HasDefaultParameters::new_gd(); + let r = obj.call("function_with_default_params", &[0.to_variant()]); + let r = r.to::(); + assert_eq!(r, varray![0, "test", 123]); +} diff --git a/itest/rust/src/register_tests/mod.rs b/itest/rust/src/register_tests/mod.rs index 3af883be2..9352e96a9 100644 --- a/itest/rust/src/register_tests/mod.rs +++ b/itest/rust/src/register_tests/mod.rs @@ -7,6 +7,7 @@ mod constant_test; mod conversion_test; +mod default_parameters_test; mod derive_godotconvert_test; mod func_test; mod gdscript_ffi_test; From ff9ec4c739bbaccec07667c45a94a1c3b465fe33 Mon Sep 17 00:00:00 2001 From: astrale Date: Thu, 26 Jun 2025 11:59:55 +0200 Subject: [PATCH 2/3] changes varcall_fn signature to take into account default arguments --- godot-core/src/meta/error/call_error.rs | 4 +- godot-core/src/meta/signature.rs | 3 +- godot-macros/src/class/data_models/func.rs | 48 ++++++++++++++++++---- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/godot-core/src/meta/error/call_error.rs b/godot-core/src/meta/error/call_error.rs index ff82c16e5..ef06cf5d6 100644 --- a/godot-core/src/meta/error/call_error.rs +++ b/godot-core/src/meta/error/call_error.rs @@ -117,10 +117,11 @@ impl CallError { pub(crate) fn check_arg_count( call_ctx: &CallContext, arg_count: usize, + default_args_count: usize, param_count: usize, ) -> Result<(), Self> { // This will need to be adjusted once optional parameters are supported in #[func]. - if arg_count == param_count { + if arg_count + default_args_count >= param_count { return Ok(()); } @@ -221,6 +222,7 @@ impl CallError { ) } + // should this be adjusted with default param counts? fn failed_param_count( call_ctx: &CallContext, arg_count: usize, diff --git a/godot-core/src/meta/signature.rs b/godot-core/src/meta/signature.rs index 5e8174f17..9dd8b1fa7 100644 --- a/godot-core/src/meta/signature.rs +++ b/godot-core/src/meta/signature.rs @@ -66,12 +66,13 @@ impl Signature { call_ctx: &CallContext, args_ptr: *const sys::GDExtensionConstVariantPtr, arg_count: i64, + default_arg_count: usize, ret: sys::GDExtensionVariantPtr, err: *mut sys::GDExtensionCallError, func: unsafe fn(sys::GDExtensionClassInstancePtr, Params) -> Ret, ) -> CallResult<()> { //$crate::out!("in_varcall: {call_ctx}"); - CallError::check_arg_count(call_ctx, arg_count as usize, Params::LEN)?; + CallError::check_arg_count(call_ctx, arg_count as usize, default_arg_count, Params::LEN)?; #[cfg(feature = "trace")] trace::push(true, false, call_ctx); diff --git a/godot-macros/src/class/data_models/func.rs b/godot-macros/src/class/data_models/func.rs index 45230d2d3..8177b6ef3 100644 --- a/godot-macros/src/class/data_models/func.rs +++ b/godot-macros/src/class/data_models/func.rs @@ -112,12 +112,15 @@ pub fn make_method_registration( interface_trait, ); + let (default_parameters, default_parameters_count) = + make_default_parameters(&func_definition, signature_info)?; + // String literals let class_name_str = class_name.to_string(); let method_name_str = func_definition.godot_name(); let call_ctx = make_call_context(&class_name_str, &method_name_str); - let varcall_fn_decl = make_varcall_fn(&call_ctx, &forwarding_closure); + let varcall_fn_decl = make_varcall_fn(&call_ctx, &forwarding_closure, default_parameters_count); let ptrcall_fn_decl = make_ptrcall_fn(&call_ctx, &forwarding_closure); // String literals II @@ -126,8 +129,7 @@ pub fn make_method_registration( .iter() .map(|ident| ident.to_string()); - let default_parameters = - validate_default_parameters(&func_definition.signature_info.default_parameters)?; + // #(::godot::builtin::Variant::from(#default_parameters)),* // Transport #[cfg] attrs to the FFI glue to ensure functions which were conditionally // removed from compilation don't cause errors. @@ -161,9 +163,7 @@ pub fn make_method_registration( &[ #( #param_ident_strs ),* ], - vec![ - #(::godot::builtin::Variant::from(#default_parameters)),* - ] + #default_parameters, ) }; @@ -181,6 +181,27 @@ pub fn make_method_registration( Ok(registration) } +fn make_default_parameters( + func_definition: &FuncDefinition, + signature_info: &SignatureInfo, +) -> Result<(TokenStream, usize), venial::Error> { + let default_parameters = + validate_default_parameters(&func_definition.signature_info.default_parameters)?; + let len = default_parameters.len(); + let default_parameters_type = signature_info + .param_types + .iter() + .rev() + .take(default_parameters.len()) + .rev(); + let default_parameters = default_parameters + .iter() + .zip(default_parameters_type) + .map(|(value, ty)| quote!(::godot::meta::arg_into_ref!(#value: #ty))); + let default_parameters = quote! {vec![#(#default_parameters),*]}; + Ok((default_parameters, len)) +} + fn validate_default_parameters( default_parameters: &[Option], ) -> ParseResult> { @@ -525,8 +546,12 @@ fn make_method_flags( } /// Generate code for a C FFI function that performs a varcall. -fn make_varcall_fn(call_ctx: &TokenStream, wrapped_method: &TokenStream) -> TokenStream { - let invocation = make_varcall_invocation(wrapped_method); +fn make_varcall_fn( + call_ctx: &TokenStream, + wrapped_method: &TokenStream, + default_parameters_count: usize, +) -> TokenStream { + let invocation = make_varcall_invocation(wrapped_method, default_parameters_count); // TODO reduce amount of code generated, by delegating work to a library function. Could even be one that produces this function pointer. quote! { @@ -593,13 +618,18 @@ fn make_ptrcall_invocation(wrapped_method: &TokenStream, is_virtual: bool) -> To } /// Generate code for a `varcall()` call expression. -fn make_varcall_invocation(wrapped_method: &TokenStream) -> TokenStream { +fn make_varcall_invocation( + wrapped_method: &TokenStream, + default_parameters_count: usize, +) -> TokenStream { + // to adjust with use of default parameters quote! { ::godot::meta::Signature::::in_varcall( instance_ptr, &call_ctx, args_ptr, arg_count, + #default_parameters_count, ret, err, #wrapped_method, From b9ed1f6795c30f5a7c6f37b575a9af37925a552f Mon Sep 17 00:00:00 2001 From: astrale Date: Thu, 26 Jun 2025 12:10:22 +0200 Subject: [PATCH 3/3] quick test with Variant::from --- godot-macros/src/class/data_models/func.rs | 3 ++- itest/rust/src/register_tests/default_parameters_test.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/godot-macros/src/class/data_models/func.rs b/godot-macros/src/class/data_models/func.rs index 8177b6ef3..863f6ad2b 100644 --- a/godot-macros/src/class/data_models/func.rs +++ b/godot-macros/src/class/data_models/func.rs @@ -197,7 +197,8 @@ fn make_default_parameters( let default_parameters = default_parameters .iter() .zip(default_parameters_type) - .map(|(value, ty)| quote!(::godot::meta::arg_into_ref!(#value: #ty))); + .map(|(value, ty)| quote!(::godot::builtin::Variant::from(#value))); + // .map(|(value, ty)| quote!(::godot::meta::arg_into_ref!(#value: #ty))); let default_parameters = quote! {vec![#(#default_parameters),*]}; Ok((default_parameters, len)) } diff --git a/itest/rust/src/register_tests/default_parameters_test.rs b/itest/rust/src/register_tests/default_parameters_test.rs index 7f269a645..4cd17e3d6 100644 --- a/itest/rust/src/register_tests/default_parameters_test.rs +++ b/itest/rust/src/register_tests/default_parameters_test.rs @@ -18,7 +18,7 @@ impl HasDefaultParameters { } } -#[itest] +#[itest(focus)] fn tests_default_parameters() { let mut obj = HasDefaultParameters::new_gd(); let r = obj.call("function_with_default_params", &[0.to_variant()]);