From f08c48395b139456557adeead332170be002f016 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 20 Jul 2025 21:15:18 +0200 Subject: [PATCH 1/4] Retain spans in newly generated TokenStreams when possible --- godot-macros/src/class/data_models/func.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/godot-macros/src/class/data_models/func.rs b/godot-macros/src/class/data_models/func.rs index 4c31d799c..9c38484a6 100644 --- a/godot-macros/src/class/data_models/func.rs +++ b/godot-macros/src/class/data_models/func.rs @@ -440,8 +440,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) From fff255c3d35e3cd937cc958c6f0ff760be7bffe1 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 20 Jul 2025 22:25:56 +0200 Subject: [PATCH 2/4] Move virtual definitions: sys::godot_virtual_consts -> private::virtuals --- godot-codegen/src/generator/mod.rs | 10 ++-------- ...ual_definition_consts.rs => virtual_definitions.rs} | 4 ++-- godot-codegen/src/lib.rs | 9 ++++++++- .../src/class/data_models/interface_trait_impl.rs | 6 +++--- godot-macros/src/class/derive_godot_class.rs | 2 +- 5 files changed, 16 insertions(+), 15 deletions(-) rename godot-codegen/src/generator/{virtual_definition_consts.rs => virtual_definitions.rs} (92%) diff --git a/godot-codegen/src/generator/mod.rs b/godot-codegen/src/generator/mod.rs index 68082b711..4aa0c379f 100644 --- a/godot-codegen/src/generator/mod.rs +++ b/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); diff --git a/godot-codegen/src/generator/virtual_definition_consts.rs b/godot-codegen/src/generator/virtual_definitions.rs similarity index 92% rename from godot-codegen/src/generator/virtual_definition_consts.rs rename to godot-codegen/src/generator/virtual_definitions.rs index 9aa25f632..3e6b636f3 100644 --- a/godot-codegen/src/generator/virtual_definition_consts.rs +++ b/godot-codegen/src/generator/virtual_definitions.rs @@ -9,9 +9,9 @@ use proc_macro2::TokenStream; use quote::quote; use crate::context::Context; -use crate::models::domain::{Class, ClassLike, ExtensionApi, FnDirection, Function}; +use crate::models::domain::{Class, ExtensionApi, FnDirection}; -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) } diff --git a/godot-codegen/src/lib.rs b/godot-codegen/src/lib.rs index 955c27e13..eb9e4ea75 100644 --- a/godot-codegen/src/lib.rs +++ b/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( diff --git a/godot-macros/src/class/data_models/interface_trait_impl.rs b/godot-macros/src/class/data_models/interface_trait_impl.rs index e373cdce7..554191ed6 100644 --- a/godot-macros/src/class/data_models/interface_trait_impl.rs +++ b/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 ParseResult ::godot::sys::GDExtensionClassCallVirtual { //println!("virtual_call: {}.{}", std::any::type_name::(), 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::("method") godot_name_hash_constant: quote! { virtuals::#method_name_ident }, signature_info, diff --git a/godot-macros/src/class/derive_godot_class.rs b/godot-macros/src/class/derive_godot_class.rs index 6c6cf49c9..8e2964af6 100644 --- a/godot-macros/src/class/derive_godot_class.rs +++ b/godot-macros/src/class/derive_godot_class.rs @@ -444,7 +444,7 @@ fn make_user_class_impl( if cfg!(since_api = "4.4") { hash_param = quote! { hash: u32, }; matches_ready_hash = quote! { - (name, hash) == ::godot::sys::godot_virtual_consts::Node::ready + (name, hash) == ::godot::private::virtuals::Node::ready }; } else { hash_param = TokenStream::new(); From 854897ba178d51887732c7abf441f25318f14c46 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 20 Jul 2025 22:28:54 +0200 Subject: [PATCH 3/4] Declare virtual signatures in central place --- .../src/generator/virtual_definitions.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/godot-codegen/src/generator/virtual_definitions.rs b/godot-codegen/src/generator/virtual_definitions.rs index 3e6b636f3..31b529589 100644 --- a/godot-codegen/src/generator/virtual_definitions.rs +++ b/godot-codegen/src/generator/virtual_definitions.rs @@ -6,7 +6,7 @@ */ use proc_macro2::TokenStream; -use quote::quote; +use quote::{format_ident, quote}; use crate::context::Context; use crate::models::domain::{Class, ExtensionApi, FnDirection}; @@ -21,7 +21,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 +34,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() @@ -50,14 +56,22 @@ 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(); + let param_types = method.params().iter().map(|p| &p.type_); + + let rust_sig_name = format_ident!("Sig_{rust_name}"); + let sig_decl = quote! { + 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); + #sig_decl }; #[cfg(before_api = "4.4")] let constant = quote! { pub const #rust_name: &'static str = #godot_name_str; + #sig_decl }; Some(constant) From e3f67f6b2d64f82104293671c9316244f91ada18 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Fri, 17 Oct 2025 04:26:20 +0200 Subject: [PATCH 4/4] Store signature of virtual methods Attempts to provide type information that's coming from the GDExtension API, rather than from the user (through the macro). This should help in the future with more detailed error messages, by using the API signatures as source of truth. --- .../src/generator/functions_common.rs | 55 ++++++++++------- .../src/generator/virtual_definitions.rs | 17 ++++-- godot-core/src/private.rs | 1 + godot-ffi/src/lib.rs | 1 - godot-macros/src/class/data_models/func.rs | 61 +++++++++++++------ .../class/data_models/interface_trait_impl.rs | 5 +- godot-macros/src/class/derive_godot_class.rs | 19 ++++-- 7 files changed, 104 insertions(+), 55 deletions(-) diff --git a/godot-codegen/src/generator/functions_common.rs b/godot-codegen/src/generator/functions_common.rs index e1600587c..7381bc9ff 100644 --- a/godot-codegen/src/generator/functions_common.rs +++ b/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>` instead of `Gd` 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>, 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, @@ -614,30 +640,13 @@ pub(crate) fn make_params_exprs_virtual<'a>( let param_name = ¶m.name; let param_ty = ¶m.type_; - match ¶m.type_ { - // Virtual methods accept Option>, 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 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 diff --git a/godot-codegen/src/generator/virtual_definitions.rs b/godot-codegen/src/generator/virtual_definitions.rs index 31b529589..c8143481b 100644 --- a/godot-codegen/src/generator/virtual_definitions.rs +++ b/godot-codegen/src/generator/virtual_definitions.rs @@ -9,7 +9,8 @@ use proc_macro2::TokenStream; use quote::{format_ident, quote}; use crate::context::Context; -use crate::models::domain::{Class, ExtensionApi, FnDirection}; +use crate::generator::functions_common::make_virtual_param_type; +use crate::models::domain::{Class, ClassLike, ExtensionApi, FnDirection, Function}; pub fn make_virtual_definitions_file(api: &ExtensionApi, ctx: &mut Context) -> TokenStream { make_virtual_hashes_for_all_classes(&api.classes, ctx) @@ -56,21 +57,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(); - let param_types = method.params().iter().map(|p| &p.type_); + + // Generate parameter types, wrapping EngineClass in Option<> just like the trait does + let param_types = method + .params() + .iter() + .map(|param| make_virtual_param_type(¶m.type_, ¶m.name, method)); let rust_sig_name = format_ident!("Sig_{rust_name}"); let sig_decl = quote! { - type #rust_sig_name = ( #(#param_types,)* ); + // 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 }; diff --git a/godot-core/src/private.rs b/godot-core/src/private.rs index 9f1ac4009..6e3148a96 100644 --- a/godot-core/src/private.rs +++ b/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; diff --git a/godot-ffi/src/lib.rs b/godot-ffi/src/lib.rs index a1e2e4156..3807b9356 100644 --- a/godot-ffi/src/lib.rs +++ b/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::*; diff --git a/godot-macros/src/class/data_models/func.rs b/godot-macros/src/class/data_models/func.rs index 9c38484a6..ecc83c4b0 100644 --- a/godot-macros/src/class/data_models/func.rs +++ b/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 = diff --git a/godot-macros/src/class/data_models/interface_trait_impl.rs b/godot-macros/src/class/data_models/interface_trait_impl.rs index 554191ed6..ebcd7525d 100644 --- a/godot-macros/src/class/data_models/interface_trait_impl.rs +++ b/godot-macros/src/class/data_models/interface_trait_impl.rs @@ -178,7 +178,7 @@ pub fn transform_trait_impl(mut original_impl: venial::Impl) -> ParseResult { } 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(), diff --git a/godot-macros/src/class/derive_godot_class.rs b/godot-macros/src/class/derive_godot_class.rs index 8e2964af6..e2b6533b4 100644 --- a/godot-macros/src/class/derive_godot_class.rs +++ b/godot-macros/src/class/derive_godot_class.rs @@ -99,8 +99,12 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { TokenStream::new() }; - let (user_class_impl, has_default_virtual) = - make_user_class_impl(class_name, struct_cfg.is_tool, &fields.all_fields); + let (user_class_impl, has_default_virtual) = make_user_class_impl( + class_name, + &struct_cfg.base_ty, + struct_cfg.is_tool, + &fields.all_fields, + ); let mut init_expecter = TokenStream::new(); let mut godot_init_impl = TokenStream::new(); @@ -415,6 +419,7 @@ fn make_oneditor_panic_inits(class_name: &Ident, all_fields: &[Field]) -> TokenS fn make_user_class_impl( class_name: &Ident, + trait_base_class: &Ident, is_tool: bool, all_fields: &[Field], ) -> (TokenStream, bool) { @@ -425,7 +430,6 @@ fn make_user_class_impl( let rpc_registrations = TokenStream::new(); let onready_inits = make_onready_init(all_fields); - let oneditor_panic_inits = make_oneditor_panic_inits(class_name, all_fields); let run_before_ready = !onready_inits.is_empty() || !oneditor_panic_inits.is_empty(); @@ -434,8 +438,13 @@ fn make_user_class_impl( let tool_check = util::make_virtual_tool_check(); let signature_info = SignatureInfo::fn_ready(); - let callback = - make_virtual_callback(class_name, &signature_info, BeforeKind::OnlyBefore, None); + let callback = make_virtual_callback( + class_name, + trait_base_class, + &signature_info, + BeforeKind::OnlyBefore, + None, + ); // See also __virtual_call() codegen. // This doesn't explicitly check if the base class inherits from Node (and thus has `_ready`), but the derive-macro already does