Merge pull request #1396 from godot-rust/feature/func-default-params

Bromeon · web-flow · commit 309881cab732 · 2025-11-04T12:45:54.000Z
Default parameters via `#[opt(default = ...)]`  syntax
diff --git a/godot-core/src/meta/error/call_error.rs b/godot-core/src/meta/error/call_error.rs
@@ -121,16 +121,18 @@ impl CallError {
     /// Checks whether number of arguments matches the number of parameters.
     pub(crate) fn check_arg_count(
         call_ctx: &CallContext,
-        arg_count: usize,
-        param_count: usize,
+        arg_count: usize,           // Arguments passed by the caller.
+        default_value_count: usize, // Fallback/default values, *not* arguments.
+        param_count: usize,         // Parameters declared by the function.
     ) -> Result<(), Self> {
-        // This will need to be adjusted once optional parameters are supported in #[func].
-        if arg_count == param_count {
+        // Valid if both:
+        // - Provided args + available defaults (fallbacks) are enough to fill all parameters.
+        // - Provided args don't exceed parameter count.
+        if arg_count + default_value_count >= param_count && arg_count <= param_count {
             return Ok(());
         }
 
         let call_error = Self::failed_param_count(call_ctx, arg_count, param_count);
-
         Err(call_error)
     }
 
@@ -180,7 +182,7 @@ impl CallError {
     /// Returns an error for a failed parameter conversion.
     pub(crate) fn failed_param_conversion<P>(
         call_ctx: &CallContext,
-        param_index: isize,
+        param_index: usize,
         convert_error: ConvertError,
     ) -> Self {
         let param_ty = std::any::type_name::<P>();
diff --git a/godot-core/src/meta/method_info.rs b/godot-core/src/meta/method_info.rs
@@ -24,6 +24,8 @@ pub struct MethodInfo {
     pub class_name: ClassId,
     pub return_type: PropertyInfo,
     pub arguments: Vec<PropertyInfo>,
+    /// Whether default arguments are real "arguments" is controversial. From the function PoV they are, but for the caller,
+    /// they are just pre-set values to fill in for missing arguments.
     pub default_arguments: Vec<Variant>,
     pub flags: MethodFlags,
 }
diff --git a/godot-core/src/meta/param_tuple.rs b/godot-core/src/meta/param_tuple.rs
@@ -42,15 +42,18 @@ pub trait ParamTuple: Sized {
 /// As an example, this would be used for user-defined functions that will be called from Godot, however this is _not_ used when
 /// calling a Godot function from Rust code.
 pub trait InParamTuple: ParamTuple {
-    /// Converts `args_ptr` to `Self` by first going through [`Variant`].
+    /// Converts `args_ptr` to `Self`, merging with default values if needed.
     ///
     /// # Safety
     ///
-    /// - `args_ptr` must be a pointer to an array of length [`Self::LEN`](ParamTuple::LEN)
+    /// - `args_ptr` must be a pointer to an array of length `arg_count`
     /// - Each element of `args_ptr` must be reborrowable as a `&Variant` with a lifetime that lasts for the duration of the call.
-    #[doc(hidden)] // Hidden since v0.3.2.
+    /// - `arg_count + default_values.len()` must equal `Self::LEN`
+    #[doc(hidden)]
     unsafe fn from_varcall_args(
         args_ptr: *const sys::GDExtensionConstVariantPtr,
+        arg_count: usize,
+        default_values: &[Variant],
         call_ctx: &CallContext,
     ) -> CallResult<Self>;
 
diff --git a/godot-core/src/meta/param_tuple/impls.rs b/godot-core/src/meta/param_tuple/impls.rs
@@ -56,20 +56,40 @@ macro_rules! unsafe_impl_param_tuple {
         impl<$($P),*> InParamTuple for ($($P,)*) where $($P: FromGodot + fmt::Debug),* {
             unsafe fn from_varcall_args(
                 args_ptr: *const sys::GDExtensionConstVariantPtr,
+                arg_count: usize,
+                default_values: &[Variant],
                 call_ctx: &crate::meta::CallContext,
             ) -> CallResult<Self> {
-                let args = (
-                    $(
-                        // SAFETY: `args_ptr` is an array with length `Self::LEN` and each element is a valid pointer, since they
-                        // are all reborrowable as references.
-                        unsafe { *args_ptr.offset($n) },
-                    )*
-                );
+                // Fast path: all args provided, no defaults needed (zero allocations).
+                if arg_count == Self::LEN {
+                    let param_tuple = (
+                        $(
+                            unsafe { varcall_arg::<$P>(*args_ptr.add($n), call_ctx, $n)? },
+                        )*
+                    );
+                    return Ok(param_tuple);
+                }
+
+                // Slow path: merge provided args with defaults (requires allocation).
+                let mut all_args = Vec::with_capacity(Self::LEN);
+
+                // Copy all provided args.
+                for i in 0..arg_count {
+                    all_args.push(unsafe { *args_ptr.add(i) });
+                }
+
+                // Fill remaining parameters with default values.
+                let required_param_count = Self::LEN - default_values.len();
+                let first_missing_index = arg_count - required_param_count;
+                for i in first_missing_index..default_values.len() {
+                    all_args.push(default_values[i].var_sys());
+                }
 
+                // Convert all args to the tuple.
                 let param_tuple = (
                     $(
-                        // SAFETY: Each pointer in `args_ptr` is reborrowable as a `&Variant` for the duration of this call.
-                        unsafe { varcall_arg::<$P>(args.$n, call_ctx, $n)? },
+                        // SAFETY: Each pointer in `args_ptr` is borrowable as a &Variant for the duration of this call.
+                        unsafe { varcall_arg::<$P>(all_args[$n], call_ctx, $n)? },
                     )*
                 );
 
@@ -191,16 +211,16 @@ unsafe_impl_param_tuple!((p0, 0): P0, (p1, 1): P1, (p2, 2): P2, (p3, 3): P3, (p4
 /// Convert the `N`th argument of `args_ptr` into a value of type `P`.
 ///
 /// # Safety
-/// - It must be safe to dereference the address at `args_ptr.offset(N)`.
-/// - The pointer at `args_ptr.offset(N)` must follow the safety requirements as laid out in
+/// - It must be safe to dereference the address at `args_ptr.add(N)`.
+/// - The pointer at `args_ptr.add(N)` must follow the safety requirements as laid out in
 ///   [`GodotFfi::from_arg_ptr`].
-pub(super) unsafe fn ptrcall_arg<P: FromGodot, const N: isize>(
+pub(super) unsafe fn ptrcall_arg<P: FromGodot, const N: usize>(
     args_ptr: *const sys::GDExtensionConstTypePtr,
     call_ctx: &CallContext,
     call_type: sys::PtrcallType,
 ) -> CallResult<P> {
     // SAFETY: It is safe to dereference `args_ptr` at `N`.
-    let offset_ptr = unsafe { *args_ptr.offset(N) };
+    let offset_ptr = unsafe { *args_ptr.add(N) };
 
     // SAFETY: The pointer follows the safety requirements from `GodotFfi::from_arg_ptr`.
     let ffi = unsafe {
@@ -220,7 +240,7 @@ pub(super) unsafe fn ptrcall_arg<P: FromGodot, const N: isize>(
 pub(super) unsafe fn varcall_arg<P: FromGodot>(
     arg: sys::GDExtensionConstVariantPtr,
     call_ctx: &CallContext,
-    param_index: isize,
+    param_index: usize,
 ) -> CallResult<P> {
     // SAFETY: It is safe to dereference `args_ptr` at `N` as a `Variant`.
     let variant_ref = unsafe { Variant::borrow_var_sys(arg) };
diff --git a/godot-core/src/meta/signature.rs b/godot-core/src/meta/signature.rs
@@ -63,23 +63,27 @@ where
     /// # Safety
     /// A call to this function must be caused by Godot making a varcall with parameters `Params` and return type `Ret`.
     #[inline]
+    #[allow(clippy::too_many_arguments)]
     pub unsafe fn in_varcall(
         instance_ptr: sys::GDExtensionClassInstancePtr,
         call_ctx: &CallContext,
         args_ptr: *const sys::GDExtensionConstVariantPtr,
         arg_count: i64,
+        default_values: &[Variant],
         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)?;
+        let arg_count = arg_count as usize;
+        CallError::check_arg_count(call_ctx, arg_count, default_values.len(), Params::LEN)?;
 
         #[cfg(feature = "trace")]
         trace::push(true, false, call_ctx);
 
         // SAFETY: TODO.
-        let args = unsafe { Params::from_varcall_args(args_ptr, call_ctx)? };
+        let args =
+            unsafe { Params::from_varcall_args(args_ptr, arg_count, default_values, call_ctx)? };
 
         let rust_result = unsafe { func(instance_ptr, args) };
         // SAFETY: TODO.
diff --git a/godot-core/src/registry/method.rs b/godot-core/src/registry/method.rs
@@ -34,6 +34,8 @@ pub struct ClassMethodInfo {
     method_flags: MethodFlags,
     return_value: Option<MethodParamOrReturnInfo>,
     arguments: Vec<MethodParamOrReturnInfo>,
+    /// Whether default arguments are real "arguments" is controversial. From the function PoV they are, but for the caller,
+    /// they are just pre-set values to fill in for missing arguments.
     default_arguments: Vec<Variant>,
 }
 
@@ -59,12 +61,11 @@ impl ClassMethodInfo {
         ptrcall_func: sys::GDExtensionClassMethodPtrCall,
         method_flags: MethodFlags,
         param_names: &[&str],
-        // default_arguments: Vec<Variant>, - not yet implemented
+        default_arguments: Vec<Variant>,
     ) -> Self {
         let return_value = Ret::Via::return_info();
         let arguments = Signature::<Params, Ret>::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
@@ -120,12 +120,24 @@ pub fn make_method_registration(
         interface_trait,
     );
 
+    let default_parameters = make_default_argument_vec(
+        &signature_info.optional_param_default_exprs,
+        &signature_info.param_types,
+    )?;
+
     // 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);
+
+    // Both varcall and ptrcall functions are always generated and registered, even when default parameters are present via #[opt].
+    // Key differences are:
+    // - varcall: handles default parameters, applying them when caller provides fewer arguments.
+    // - ptrcall: optimized path without default handling, can be used when caller provides all arguments.
+    //
+    // Godot decides at call-time which calling convention to use based on available type information.
+    let varcall_fn_decl = make_varcall_fn(&call_ctx, &forwarding_closure, &default_parameters);
     let ptrcall_fn_decl = make_ptrcall_fn(&call_ctx, &forwarding_closure);
 
     // String literals II
@@ -166,6 +178,7 @@ pub fn make_method_registration(
                     &[
                         #( #param_ident_strs ),*
                     ],
+                    #default_parameters,
                 )
             };
 
@@ -183,6 +196,48 @@ pub fn make_method_registration(
     Ok(registration)
 }
 
+/// Generates code to create a `Vec<Variant>` containing default argument values for varcall. Allocates on every call.
+fn make_default_argument_vec(
+    optional_param_default_exprs: &[TokenStream],
+    all_params: &[venial::TypeExpr],
+) -> ParseResult<TokenStream> {
+    // Optional params appearing at the end has already been validated in validate_default_exprs().
+
+    // Early exit: all parameters are required, not optional. This check is not necessary for correctness.
+    if optional_param_default_exprs.is_empty() {
+        return Ok(quote! { vec![] });
+    }
+
+    let optional_param_types = all_params
+        .iter()
+        .skip(all_params.len() - optional_param_default_exprs.len());
+
+    let default_parameters = optional_param_default_exprs
+        .iter()
+        .zip(optional_param_types)
+        .map(|(value, param_type)| {
+            quote! {
+                ::godot::builtin::Variant::from(
+                    ::godot::meta::AsArg::<#param_type>::into_arg(#value)
+                )
+            }
+        });
+
+    // Performance: This generates `vec![...]` in the varcall FFI function, which allocates on *every* call when default parameters
+    // are present. This is a performance cost we accept for now.
+    //
+    // If no #[opt] attributes are used, this generates `vec![]` which does *not* allocate, so most #[func] functions are unaffected.
+    //
+    // Potential future improvements:
+    // - Use `Global<Vec<Variant>>` (or LazyLock/thread_local) to allocate once per function instead of per call.
+    // - Store defaults in MethodInfo during registration and retrieve via method_data pointer.
+    //
+    // Note also that there may be a semantic difference on reusing the same object vs. recreating it, see Python's default-param issue.
+    Ok(quote! {
+        vec![ #(#default_parameters),* ]
+    })
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Implementation
 
@@ -208,6 +263,9 @@ 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)>,
+
+    /// Default value expressions `EXPR` from `#[opt(default = EXPR)]`, for all optional parameters.
+    pub optional_param_default_exprs: Vec<TokenStream>,
 }
 
 impl SignatureInfo {
@@ -220,6 +278,7 @@ impl SignatureInfo {
             param_types: vec![],
             return_type: quote! { () },
             modified_param_types: vec![],
+            optional_param_default_exprs: vec![],
         }
     }
 
@@ -413,7 +472,7 @@ pub(crate) fn into_signature_info(
     let params_span = signature.span();
     let mut param_idents = Vec::with_capacity(num_params);
     let mut param_types = Vec::with_capacity(num_params);
-    let ret_type = match signature.return_ty {
+    let return_type = match signature.return_ty {
         None => quote! { () },
         Some(ty) => map_self_to_class_name(ty.tokens, class_name),
     };
@@ -469,8 +528,9 @@ pub(crate) fn into_signature_info(
         params_span,
         param_idents,
         param_types,
-        return_type: ret_type,
+        return_type,
         modified_param_types,
+        optional_param_default_exprs: vec![], // Assigned outside, if relevant.
     }
 }
 
@@ -552,8 +612,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: &TokenStream,
+) -> TokenStream {
+    let invocation = make_varcall_invocation(wrapped_method, default_parameters);
 
     // TODO reduce amount of code generated, by delegating work to a library function. Could even be one that produces this function pointer.
     quote! {
@@ -616,17 +680,24 @@ 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: &TokenStream,
+) -> TokenStream {
     quote! {
-        ::godot::meta::Signature::<CallParams, CallRet>::in_varcall(
-            instance_ptr,
-            &call_ctx,
-            args_ptr,
-            arg_count,
-            ret,
-            err,
-            #wrapped_method,
-        )
+        {
+            let defaults = #default_parameters;
+            ::godot::meta::Signature::<CallParams, CallRet>::in_varcall(
+                instance_ptr,
+                &call_ctx,
+                args_ptr,
+                arg_count,
+                &defaults,
+                ret,
+                err,
+                #wrapped_method,
+            )
+        }
     }
 }
 
diff --git a/godot-macros/src/class/data_models/inherent_impl.rs b/godot-macros/src/class/data_models/inherent_impl.rs
diff --git a/godot-macros/src/lib.rs b/godot-macros/src/lib.rs
diff --git a/itest/rust/src/register_tests/func_test.rs b/itest/rust/src/register_tests/func_test.rs

Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,8 @@ pub struct MethodInfo {`
`24`	`24`	`pub class_name: ClassId,`
`25`	`25`	`pub return_type: PropertyInfo,`
`26`	`26`	`pub arguments: Vec<PropertyInfo>,`
	`27`	`+ /// Whether default arguments are real "arguments" is controversial. From the function PoV they are, but for the caller,`
	`28`	`+ /// they are just pre-set values to fill in for missing arguments.`
`27`	`29`	`pub default_arguments: Vec<Variant>,`
`28`	`30`	`pub flags: MethodFlags,`
`29`	`31`	`}`