Improve spans; use tuple type for virtual signatures

godot-codegen/src/generator/functions_common.rs

@@ -603,6 +603,32 @@ pub(crate) fn make_params_exprs<'a>(
     ret
 }
 
+/// Returns the type for a virtual method parameter.
+///
+/// Generates `Option<Gd<T>>` instead of `Gd<T>` for object parameters (which are currently all nullable).
+///
+/// Used for consistency between virtual trait definitions and `type Sig = ...` type-safety declarations
+/// (which are used to improve compile-time errors on mismatch).
+pub(crate) fn make_virtual_param_type(
+    param_ty: &RustTy,
+    param_name: &Ident,
+    function_sig: &dyn Function,
+) -> TokenStream {
+    match param_ty {
+        // Virtual methods accept Option<Gd<T>>, since we don't know whether objects are nullable or required.
+        RustTy::EngineClass { .. }
+            if !special_cases::is_class_method_param_required(
+                function_sig.surrounding_class().unwrap(),
+                function_sig.godot_name(),
+                param_name,
+            ) =>
+        {
+            quote! { Option<#param_ty> }
+        }
+        _ => quote! { #param_ty },
+    }
+}
+
 /// For virtual functions, returns the parameter declarations, type tokens, and names.
 pub(crate) fn make_params_exprs_virtual<'a>(
     method_args: impl Iterator<Item = &'a FnParam>,
@@ -614,30 +640,13 @@ pub(crate) fn make_params_exprs_virtual<'a>(
         let param_name = &param.name;
         let param_ty = &param.type_;
 
-        match &param.type_ {
-            // Virtual methods accept Option<Gd<T>>, since we don't know whether objects are nullable or required.
-            RustTy::EngineClass { .. }
-                if !special_cases::is_class_method_param_required(
-                    function_sig.surrounding_class().unwrap(),
-                    function_sig.godot_name(),
-                    param_name,
-                ) =>
-            {
-                ret.param_decls
-                    .push(quote! { #param_name: Option<#param_ty> });
-                ret.arg_exprs.push(quote! { #param_name });
-                ret.callsig_param_types.push(quote! { #param_ty });
-            }
+        // Map parameter types (e.g. virtual functions need Option<Gd> instead of Gd).
+        let param_ty_tokens = make_virtual_param_type(param_ty, param_name, function_sig);
 
-            // All other methods and parameter types: standard handling.
-            // For now, virtual methods always receive their parameter by value.
-            //_ => ret.push_regular(param_name, param_ty, true, false, false),
-            _ => {
-                ret.param_decls.push(quote! { #param_name: #param_ty });
-                ret.arg_exprs.push(quote! { #param_name });
-                ret.callsig_param_types.push(quote! { #param_ty });
-            }
-        }
+        ret.param_decls
+            .push(quote! { #param_name: #param_ty_tokens });
+        ret.arg_exprs.push(quote! { #param_name });
+        ret.callsig_param_types.push(quote! { #param_ty });
     }
 
     ret

godot-codegen/src/generator/mod.rs

@@ -30,7 +30,7 @@ pub mod notifications;
 pub mod signals;
 pub mod sys;
 pub mod utility_functions;
-pub mod virtual_definition_consts;
+pub mod virtual_definitions;
 pub mod virtual_traits;
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
@@ -57,7 +57,6 @@ pub fn generate_sys_module_file(sys_gen_path: &Path, submit_fn: &mut SubmitFn) {
         pub mod central;
         pub mod gdextension_interface;
         pub mod interface;
-        pub mod virtual_consts;
     };
 
     submit_fn(sys_gen_path.join("mod.rs"), code);
@@ -87,12 +86,6 @@ pub fn generate_sys_classes_file(
         submit_fn(sys_gen_path.join(filename), code);
         watch.record(format!("generate_classes_{}_file", api_level.lower()));
     }
-
-    // From 4.4 onward, generate table that maps all virtual methods to their known hashes.
-    // This allows Godot to fall back to an older compatibility function if one is not supported.
-    let code = virtual_definition_consts::make_virtual_consts_file(api, ctx);
-    submit_fn(sys_gen_path.join("virtual_consts.rs"), code);
-    watch.record("generate_virtual_consts_file");
 }
 
 pub fn generate_sys_utilities_file(
@@ -132,6 +125,7 @@ pub fn generate_core_mod_file(gen_path: &Path, submit_fn: &mut SubmitFn) {
         pub mod builtin_classes;
         pub mod utilities;
         pub mod native;
+        pub mod virtuals;
     };
 
     submit_fn(gen_path.join("mod.rs"), code);

godot-codegen/src/generator/virtual_definition_consts.rs → godot-codegen/src/generator/virtual_definitions.rs

@@ -6,12 +6,13 @@
  */
 
 use proc_macro2::TokenStream;
-use quote::quote;
+use quote::{format_ident, quote};
 
 use crate::context::Context;
+use crate::generator::functions_common::make_virtual_param_type;
 use crate::models::domain::{Class, ClassLike, ExtensionApi, FnDirection, Function};
 
-pub fn make_virtual_consts_file(api: &ExtensionApi, ctx: &mut Context) -> TokenStream {
+pub fn make_virtual_definitions_file(api: &ExtensionApi, ctx: &mut Context) -> TokenStream {
     make_virtual_hashes_for_all_classes(&api.classes, ctx)
 }
 
@@ -21,7 +22,7 @@ fn make_virtual_hashes_for_all_classes(all_classes: &[Class], ctx: &mut Context)
         .map(|class| make_virtual_hashes_for_class(class, ctx));
 
     quote! {
-        #![allow(non_snake_case, non_upper_case_globals, unused_imports)]
+        #![allow(non_snake_case, non_camel_case_types, non_upper_case_globals, unused_imports)]
 
         #( #modules )*
     }
@@ -34,6 +35,12 @@ fn make_virtual_hashes_for_class(class: &Class, ctx: &mut Context) -> TokenStrea
     let use_base_class = if let Some(base_class) = ctx.inheritance_tree().direct_base(class_name) {
         quote! {
             pub use super::#base_class::*;
+
+            // For type references in `Sig_*` signature tuples:
+            pub use crate::builtin::*;
+            pub use crate::classes::native::*;
+            pub use crate::obj::Gd;
+            pub use std::ffi::c_void;
         }
     } else {
         TokenStream::new()
@@ -51,13 +58,27 @@ fn make_virtual_hashes_for_class(class: &Class, ctx: &mut Context) -> TokenStrea
         let rust_name = method.name_ident();
         let godot_name_str = method.godot_name();
 
+        // Generate parameter types, wrapping EngineClass in Option<> just like the trait does
+        let param_types = method
+            .params()
+            .iter()
+            .map(|param| make_virtual_param_type(&param.type_, &param.name, method));
+
+        let rust_sig_name = format_ident!("Sig_{rust_name}");
+        let sig_decl = quote! {
+            // Pub to allow "inheritance" from other modules.
+            pub type #rust_sig_name = ( #(#param_types,)* );
+        };
+
         #[cfg(since_api = "4.4")]
         let constant = quote! {
-            pub const #rust_name: (&'static str, u32) = (#godot_name_str, #hash);
+            pub const #rust_name: (&str, u32) = (#godot_name_str, #hash);
+            #sig_decl
         };
         #[cfg(before_api = "4.4")]
         let constant = quote! {
-            pub const #rust_name: &'static str = #godot_name_str;
+            pub const #rust_name: &str = #godot_name_str;
+            #sig_decl
         };
 
         Some(constant)

godot-codegen/src/lib.rs

@@ -37,7 +37,7 @@ use crate::generator::utility_functions::generate_utilities_file;
 use crate::generator::{
     generate_core_central_file, generate_core_mod_file, generate_sys_builtin_lifecycle_file,
     generate_sys_builtin_methods_file, generate_sys_central_file, generate_sys_classes_file,
-    generate_sys_module_file, generate_sys_utilities_file,
+    generate_sys_module_file, generate_sys_utilities_file, virtual_definitions,
 };
 use crate::models::domain::{ApiView, ExtensionApi};
 use crate::models::json::{load_extension_api, JsonExtensionApi};
@@ -173,6 +173,13 @@ pub fn generate_core_files(core_gen_path: &Path) {
     generate_utilities_file(&api, core_gen_path, &mut submit_fn);
     watch.record("generate_utilities_file");
 
+    // From 4.4 onward, generate table that maps all virtual methods to their known hashes.
+    // This allows Godot to fall back to an older compatibility function if one is not supported.
+    // Also expose tuple signatures of virtual methods.
+    let code = virtual_definitions::make_virtual_definitions_file(&api, &mut ctx);
+    submit_fn(core_gen_path.join("virtuals.rs"), code);
+    watch.record("generate_virtual_definitions");
+
     // Class files -- currently output in godot-core; could maybe be separated cleaner
     // Note: deletes entire generated directory!
     generate_class_files(

godot-core/src/private.rs

@@ -25,6 +25,7 @@ mod reexport_pub {
     #[cfg(all(since_api = "4.3", feature = "register-docs"))]
     pub use crate::docs::{DocsItem, DocsPlugin, InherentImplDocs, StructDocs};
     pub use crate::gen::classes::class_macros;
+    pub use crate::gen::virtuals; // virtual fn names, hashes, signatures
     #[cfg(feature = "trace")]
     pub use crate::meta::trace;
     pub use crate::obj::rtti::ObjectRtti;

godot-ffi/src/lib.rs

@@ -86,7 +86,6 @@ pub use gen::table_editor_classes::*;
 pub use gen::table_scene_classes::*;
 pub use gen::table_servers_classes::*;
 pub use gen::table_utilities::*;
-pub use gen::virtual_consts as godot_virtual_consts;
 pub use global::*;
 pub use string_cache::StringCache;
 pub use toolbox::*;

godot-macros/src/class/data_models/func.rs

@@ -51,14 +51,20 @@ impl FuncDefinition {
 // Virtual methods are non-static by their nature; so there's no support for static ones.
 pub fn make_virtual_callback(
     class_name: &Ident,
+    trait_base_class: &Ident,
     signature_info: &SignatureInfo,
     before_kind: BeforeKind,
     interface_trait: Option<&venial::TypeExpr>,
 ) -> TokenStream {
     let method_name = &signature_info.method_name;
 
-    let wrapped_method =
-        make_forwarding_closure(class_name, signature_info, before_kind, interface_trait);
+    let wrapped_method = make_forwarding_closure(
+        class_name,
+        trait_base_class,
+        signature_info,
+        before_kind,
+        interface_trait,
+    );
     let sig_params = signature_info.params_type();
     let sig_ret = &signature_info.return_type;
 
@@ -108,6 +114,7 @@ pub fn make_method_registration(
 
     let forwarding_closure = make_forwarding_closure(
         class_name,
+        class_name, // Not used in this case.
         signature_info,
         BeforeKind::Without,
         interface_trait,
@@ -235,6 +242,7 @@ pub enum BeforeKind {
 /// Returns a closure expression that forwards the parameters to the Rust instance.
 fn make_forwarding_closure(
     class_name: &Ident,
+    trait_base_class: &Ident,
     signature_info: &SignatureInfo,
     before_kind: BeforeKind,
     interface_trait: Option<&venial::TypeExpr>,
@@ -269,29 +277,43 @@ fn make_forwarding_closure(
         ReceiverType::Ref | ReceiverType::Mut => {
             // Generated default virtual methods (e.g. for ready) may not have an actual implementation (user code), so
             // all they need to do is call the __before_ready() method. This means the actual method call may be optional.
-            let method_call = if matches!(before_kind, BeforeKind::OnlyBefore) {
-                TokenStream::new()
+            let method_call;
+            let sig_tuple_annotation;
+
+            if matches!(before_kind, BeforeKind::OnlyBefore) {
+                sig_tuple_annotation = TokenStream::new();
+                method_call = TokenStream::new()
+            } else if let Some(interface_trait) = interface_trait {
+                // impl ITrait for Class {...}
+                // Virtual methods.
+
+                let instance_ref = match signature_info.receiver_type {
+                    ReceiverType::Ref => quote! { &instance },
+                    ReceiverType::Mut => quote! { &mut instance },
+                    _ => unreachable!("unexpected receiver type"), // checked above.
+                };
+
+                let rust_sig_name = format_ident!("Sig_{method_name}");
+
+                sig_tuple_annotation = quote! {
+                    : ::godot::private::virtuals::#trait_base_class::#rust_sig_name
+                };
+                method_call = quote! {
+                    <#class_name as #interface_trait>::#method_name( #instance_ref, #(#params),* )
+                };
             } else {
-                match interface_trait {
-                    // impl ITrait for Class {...}
-                    Some(interface_trait) => {
-                        let instance_ref = match signature_info.receiver_type {
-                            ReceiverType::Ref => quote! { &instance },
-                            ReceiverType::Mut => quote! { &mut instance },
-                            _ => unreachable!("unexpected receiver type"), // checked above.
-                        };
-
-                        quote! { <#class_name as #interface_trait>::#method_name( #instance_ref, #(#params),* ) }
-                    }
+                // impl Class {...}
+                // Methods are non-virtual.
 
-                    // impl Class {...}
-                    None => quote! { instance.#method_name( #(#params),* ) },
-                }
+                sig_tuple_annotation = TokenStream::new();
+                method_call = quote! {
+                    instance.#method_name( #(#params),* )
+                };
             };
 
             quote! {
                 |instance_ptr, params| {
-                    let ( #(#params,)* ) = params;
+                    let ( #(#params,)* ) #sig_tuple_annotation = params;
 
                     let storage =
                         unsafe { ::godot::private::as_storage::<#class_name>(instance_ptr) };
@@ -307,6 +329,7 @@ fn make_forwarding_closure(
             // (Absent method is only used in the case of a generated default virtual method, e.g. for ready()).
             quote! {
                 |instance_ptr, params| {
+                    // Not using `virtual_sig`, because right now, #[func(gd_self)] is only possible for non-virtual methods.
                     let ( #(#params,)* ) = params;
 
                     let storage =
@@ -440,8 +463,12 @@ fn maybe_change_parameter_type(
         && param_ty.tokens.len() == 1
         && param_ty.tokens[0].to_string() == "f32"
     {
+        // Retain span of input parameter -> for error messages, IDE support, etc.
+        let mut f64_ident = ident("f64");
+        f64_ident.set_span(param_ty.span());
+
         Ok(venial::TypeExpr {
-            tokens: vec![TokenTree::Ident(ident("f64"))],
+            tokens: vec![TokenTree::Ident(f64_ident)],
         })
     } else {
         Err(param_ty)

godot-macros/src/class/data_models/interface_trait_impl.rs

@@ -139,7 +139,7 @@ pub fn transform_trait_impl(mut original_impl: venial::Impl) -> ParseResult<Toke
             cfg_attrs: vec![],
             rust_method_name: "_ready".to_string(),
             // Can't use `virtuals::ready` here, as the base class might not be `Node` (see above why such a branch is still added).
-            godot_name_hash_constant: quote! { ::godot::sys::godot_virtual_consts::Node::ready },
+            godot_name_hash_constant: quote! { ::godot::private::virtuals::Node::ready },
             signature_info: SignatureInfo::fn_ready(),
             before_kind: BeforeKind::OnlyBefore,
             interface_trait: None,
@@ -178,7 +178,7 @@ pub fn transform_trait_impl(mut original_impl: venial::Impl) -> ParseResult<Toke
     let virtual_match_arms = decls
         .overridden_virtuals
         .iter()
-        .map(|v| v.make_match_arm(&class_name));
+        .map(|v| v.make_match_arm(&class_name, &trait_base_class));
 
     let mut result = quote! {
         // #original_impl and gd_self_impls are inserted below.
@@ -190,7 +190,7 @@ pub fn transform_trait_impl(mut original_impl: venial::Impl) -> ParseResult<Toke
             fn __virtual_call(name: &str, #hash_param) -> ::godot::sys::GDExtensionClassCallVirtual {
                 //println!("virtual_call: {}.{}", std::any::type_name::<Self>(), name);
                 use ::godot::obj::UserClass as _;
-                use ::godot::sys::godot_virtual_consts::#trait_base_class as virtuals;
+                use ::godot::private::virtuals::#trait_base_class as virtuals;
                 #tool_check
 
                 match #match_expr {
@@ -610,7 +610,7 @@ fn handle_regular_virtual_fn<'a>(
         cfg_attrs,
         rust_method_name: virtual_method_name,
         // If ever the `I*` verbatim validation is relaxed (it won't work with use-renames or other weird edge cases), the approach
-        // with godot_virtual_consts module could be changed to something like the following (GodotBase = nearest Godot base class):
+        // with godot::private::virtuals module could be changed to something like the following (GodotBase = nearest Godot base class):
         // __get_virtual_hash::<Self::GodotBase>("method")
         godot_name_hash_constant: quote! { virtuals::#method_name_ident },
         signature_info,
@@ -719,13 +719,14 @@ struct OverriddenVirtualFn<'a> {
 }
 
 impl OverriddenVirtualFn<'_> {
-    fn make_match_arm(&self, class_name: &Ident) -> TokenStream {
+    fn make_match_arm(&self, class_name: &Ident, trait_base_class: &Ident) -> TokenStream {
         let cfg_attrs = self.cfg_attrs.iter();
         let godot_name_hash_constant = &self.godot_name_hash_constant;
 
         // Lazily generate code for the actual work (calling user function).
         let method_callback = make_virtual_callback(
             class_name,
+            trait_base_class,
             &self.signature_info,
             self.before_kind,
             self.interface_trait.as_ref(),