From e420f5f910443d4b64c2f4856d29ec47d3aa85b7 Mon Sep 17 00:00:00 2001 From: Mivort Date: Tue, 30 Sep 2025 01:23:22 +0100 Subject: [PATCH 01/12] Provide error context for typed array clone check This patch adds output of `ConvertError`'s `Display` to debug-only check's panic message which provides additional context when type mismatch happens. Panic message would include the intended type name and what was given instead of it. The output would look roughly like this: ``` copied array should have same type as original array: expected array of type Builtin(DICTIONARY), got Untyped: [] ``` --- godot-core/src/builtin/collections/array.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs index cdbd5ba5d..111feda91 100644 --- a/godot-core/src/builtin/collections/array.rs +++ b/godot-core/src/builtin/collections/array.rs @@ -1245,8 +1245,9 @@ impl Clone for Array { // Double-check copy's runtime type in Debug mode. if cfg!(debug_assertions) { - copy.with_checked_type() - .expect("copied array should have same type as original array") + copy.with_checked_type().unwrap_or_else(|e| { + panic!("copied array should have same type as original array: {e}") + }) } else { copy } From fe94254f2e0f8028ca0e064528aeef5f6937cb2f Mon Sep 17 00:00:00 2001 From: Houtamelo Date: Fri, 3 Oct 2025 19:21:16 -0300 Subject: [PATCH 02/12] Fix secondary api blocks not registering docs. --- godot-core/src/docs.rs | 88 +++++++++++-------- godot-core/src/private.rs | 16 ++++ godot-core/src/registry/class.rs | 5 +- godot-core/src/registry/plugin.rs | 10 +-- .../src/class/data_models/inherent_impl.rs | 46 +++++++--- godot-macros/src/docs.rs | 28 +++--- 6 files changed, 120 insertions(+), 73 deletions(-) diff --git a/godot-core/src/docs.rs b/godot-core/src/docs.rs index beaba7219..ef597cd1f 100644 --- a/godot-core/src/docs.rs +++ b/godot-core/src/docs.rs @@ -48,16 +48,16 @@ pub struct StructDocs { /// All fields are XML parts, escaped where necessary. #[derive(Default, Copy, Clone, Debug)] pub struct InherentImplDocs { - pub methods: &'static str, - pub signals_block: &'static str, - pub constants_block: &'static str, + pub methods: Vec<&'static str>, + pub signals: Vec<&'static str>, + pub constants: Vec<&'static str>, } #[derive(Default)] struct DocPieces { definition: StructDocs, inherent: InherentImplDocs, - virtual_methods: &'static str, + virtual_methods: Vec<&'static str>, } /// This function scours the registered plugins to find their documentation pieces, @@ -79,18 +79,21 @@ pub fn gather_xml_docs() -> impl Iterator { crate::private::iterate_plugins(|x| { let class_name = x.class_name; - match x.item { + match &x.item { PluginItem::InherentImpl(InherentImpl { docs, .. }) => { - map.entry(class_name).or_default().inherent = docs + map.entry(class_name).or_default().inherent = docs.clone(); } - PluginItem::ITraitImpl(ITraitImpl { virtual_method_docs, .. - }) => map.entry(class_name).or_default().virtual_methods = virtual_method_docs, + }) => map + .entry(class_name) + .or_default() + .virtual_methods + .push(virtual_method_docs), PluginItem::Struct(Struct { docs, .. }) => { - map.entry(class_name).or_default().definition = docs + map.entry(class_name).or_default().definition = *docs; } _ => (), @@ -98,32 +101,43 @@ pub fn gather_xml_docs() -> impl Iterator { }); map.into_iter().map(|(class, pieces)| { - let StructDocs { - base, - description, - experimental, - deprecated, - members, - } = pieces.definition; - - let InherentImplDocs { - methods, - signals_block, - constants_block, - } = pieces.inherent; - - let virtual_methods = pieces.virtual_methods; - let methods_block = (virtual_methods.is_empty() && methods.is_empty()) - .then(String::new) - .unwrap_or_else(|| format!("{methods}{virtual_methods}")); - - let (brief, description) = match description - .split_once("[br]") { - Some((brief, description)) => (brief, description.trim_start_matches("[br]")), - None => (description, ""), - }; - - format!(r#" + let StructDocs { + base, + description, + experimental, + deprecated, + members, + } = pieces.definition; + + let virtual_methods = pieces.virtual_methods; + + let mut method_docs = String::from_iter(pieces.inherent.methods); + let signal_docs = String::from_iter(pieces.inherent.signals); + let constant_docs = String::from_iter(pieces.inherent.constants); + + method_docs.extend(virtual_methods); + let methods_block = if method_docs.is_empty() { + String::new() + } else { + format!("{method_docs}") + }; + let signals_block = if signal_docs.is_empty() { + String::new() + } else { + format!("{signal_docs}") + }; + let constants_block = if constant_docs.is_empty() { + String::new() + } else { + format!("{constant_docs}") + }; + let (brief, description) = match description + .split_once("[br]") { + Some((brief, description)) => (brief, description.trim_start_matches("[br]")), + None => (description, ""), + }; + + format!(r#" {brief} {description} @@ -132,8 +146,8 @@ pub fn gather_xml_docs() -> impl Iterator { {signals_block} {members} "#) - }, - ) + }, + ) } /// # Safety diff --git a/godot-core/src/private.rs b/godot-core/src/private.rs index 74aaed0ef..fbaa54555 100644 --- a/godot-core/src/private.rs +++ b/godot-core/src/private.rs @@ -142,6 +142,22 @@ pub fn next_class_id() -> u16 { NEXT_CLASS_ID.fetch_add(1, atomic::Ordering::Relaxed) } +// Don't touch unless you know what you're doing. +#[doc(hidden)] +pub fn edit_inherent_impl(class_name: crate::meta::ClassName, f: impl FnOnce(&mut InherentImpl)) { + let mut plugins = __GODOT_PLUGIN_REGISTRY.lock().unwrap(); + + for elem in plugins.iter_mut().filter(|p| p.class_name == class_name) { + match &mut elem.item { + PluginItem::InherentImpl(inherent_impl) => { + f(inherent_impl); + return; + } + PluginItem::Struct(_) | PluginItem::ITraitImpl(_) | PluginItem::DynTraitImpl(_) => {} + } + } +} + pub(crate) fn iterate_plugins(mut visitor: impl FnMut(&ClassPlugin)) { sys::plugin_foreach!(__GODOT_PLUGIN_REGISTRY; visitor); } diff --git a/godot-core/src/registry/class.rs b/godot-core/src/registry/class.rs index 4a13622d5..d9335745d 100644 --- a/godot-core/src/registry/class.rs +++ b/godot-core/src/registry/class.rs @@ -115,7 +115,10 @@ impl ClassRegistrationInfo { // Note: when changing this match, make sure the array has sufficient size. let index = match item { PluginItem::Struct { .. } => 0, - PluginItem::InherentImpl(_) => 1, + PluginItem::InherentImpl(_) => { + // Inherent impls don't need to be unique. + return; + } PluginItem::ITraitImpl { .. } => 2, // Multiple dyn traits can be registered, thus don't validate for uniqueness. diff --git a/godot-core/src/registry/plugin.rs b/godot-core/src/registry/plugin.rs index 7d2ad995f..614828f0c 100644 --- a/godot-core/src/registry/plugin.rs +++ b/godot-core/src/registry/plugin.rs @@ -39,10 +39,10 @@ pub struct ClassPlugin { /// Incorrectly setting this value should not cause any UB but will likely cause errors during registration time. // Init-level is per ClassPlugin and not per PluginItem, because all components of all classes are mixed together in one // huge linker list. There is no per-class aggregation going on, so this allows to easily filter relevant classes. - pub(crate) init_level: InitLevel, + pub init_level: InitLevel, /// The actual item being registered. - pub(crate) item: PluginItem, + pub item: PluginItem, } impl ClassPlugin { @@ -300,9 +300,7 @@ pub struct InherentImpl { } impl InherentImpl { - pub fn new( - #[cfg(all(since_api = "4.3", feature = "register-docs"))] docs: InherentImplDocs, - ) -> Self { + pub fn new() -> Self { Self { register_methods_constants_fn: ErasedRegisterFn { raw: callbacks::register_user_methods_constants::, @@ -311,7 +309,7 @@ impl InherentImpl { raw: callbacks::register_user_rpcs::, }), #[cfg(all(since_api = "4.3", feature = "register-docs"))] - docs, + docs: Default::default(), } } } diff --git a/godot-macros/src/class/data_models/inherent_impl.rs b/godot-macros/src/class/data_models/inherent_impl.rs index 63f988ad5..53c2ade94 100644 --- a/godot-macros/src/class/data_models/inherent_impl.rs +++ b/godot-macros/src/class/data_models/inherent_impl.rs @@ -93,8 +93,6 @@ pub fn transform_inherent_impl( #[cfg(all(feature = "register-docs", since_api = "4.3"))] let docs = crate::docs::document_inherent_impl(&funcs, &consts, &signals); - #[cfg(not(all(feature = "register-docs", since_api = "4.3")))] - let docs = quote! {}; // Container struct holding names of all registered #[func]s. // The struct is declared by #[derive(GodotClass)]. @@ -127,17 +125,41 @@ pub fn transform_inherent_impl( let method_storage_name = format_ident!("__registration_methods_{class_name}"); let constants_storage_name = format_ident!("__registration_constants_{class_name}"); - let fill_storage = quote! { - ::godot::sys::plugin_execute_pre_main!({ - #method_storage_name.lock().unwrap().push(|| { - #( #method_registrations )* - #( #signal_registrations )* - }); + let fill_storage = { + #[cfg(all(feature = "register-docs", since_api = "4.3"))] + let push_docs = { + let crate::docs::InherentImplXmlDocs { + method_xml_elems, + constant_xml_elems, + signal_xml_elems, + } = docs; + + quote! { + #prv::edit_inherent_impl(#class_name_obj, |inherent_impl| { + inherent_impl.docs.methods.push(#method_xml_elems); + inherent_impl.docs.constants.push(#constant_xml_elems); + inherent_impl.docs.signals.push(#signal_xml_elems); + }); + } + }; + + #[cfg(not(all(feature = "register-docs", since_api = "4.3")))] + let push_docs = TokenStream::new(); - #constants_storage_name.lock().unwrap().push(|| { - #constant_registration + quote! { + ::godot::sys::plugin_execute_pre_main!({ + #method_storage_name.lock().unwrap().push(|| { + #( #method_registrations )* + #( #signal_registrations )* + }); + + #constants_storage_name.lock().unwrap().push(|| { + #constant_registration + }); + + #push_docs }); - }); + } }; if !meta.secondary { @@ -175,7 +197,7 @@ pub fn transform_inherent_impl( let class_registration = quote! { ::godot::sys::plugin_add!(#prv::__GODOT_PLUGIN_REGISTRY; #prv::ClassPlugin::new::<#class_name>( - #prv::PluginItem::InherentImpl(#prv::InherentImpl::new::<#class_name>(#docs)) + #prv::PluginItem::InherentImpl(#prv::InherentImpl::new::<#class_name>()) )); }; diff --git a/godot-macros/src/docs.rs b/godot-macros/src/docs.rs index de57c209b..cfdc65b0a 100644 --- a/godot-macros/src/docs.rs +++ b/godot-macros/src/docs.rs @@ -24,6 +24,12 @@ struct XmlParagraphs { deprecated_attr: String, } +pub struct InherentImplXmlDocs { + pub method_xml_elems: String, + pub constant_xml_elems: String, + pub signal_xml_elems: String, +} + /// Returns code containing the doc information of a `#[derive(GodotClass)] struct MyClass` declaration iff class or any of its members is documented. pub fn document_struct( base: String, @@ -59,39 +65,27 @@ pub fn document_inherent_impl( functions: &[FuncDefinition], constants: &[ConstDefinition], signals: &[SignalDefinition], -) -> TokenStream { - let group_xml_block = |s: String, tag: &str| -> String { - if s.is_empty() { - s - } else { - format!("<{tag}>{s}") - } - }; - +) -> InherentImplXmlDocs { let signal_xml_elems = signals .iter() .filter_map(format_signal_xml) .collect::(); - let signals_block = group_xml_block(signal_xml_elems, "signals"); let constant_xml_elems = constants .iter() .map(|ConstDefinition { raw_constant }| raw_constant) .filter_map(format_constant_xml) .collect::(); - let constants_block = group_xml_block(constant_xml_elems, "constants"); let method_xml_elems = functions .iter() .filter_map(format_method_xml) .collect::(); - quote! { - ::godot::docs::InherentImplDocs { - methods: #method_xml_elems, - signals_block: #signals_block, - constants_block: #constants_block, - } + InherentImplXmlDocs { + method_xml_elems, + constant_xml_elems, + signal_xml_elems, } } From 663535e7f5568c330f8c704b3621dd12de0db65a Mon Sep 17 00:00:00 2001 From: Luo Zhihao Date: Wed, 8 Oct 2025 12:43:59 +0800 Subject: [PATCH 03/12] Add main loop callbacks to ExtensionLibrary --- godot-core/src/init/mod.rs | 84 ++++++++++++++++--- itest/rust/src/lib.rs | 15 ++++ .../rust/src/object_tests/init_level_test.rs | 46 +++++++++- 3 files changed, 134 insertions(+), 11 deletions(-) diff --git a/godot-core/src/init/mod.rs b/godot-core/src/init/mod.rs index f1ebd978c..8aefef1cb 100644 --- a/godot-core/src/init/mod.rs +++ b/godot-core/src/init/mod.rs @@ -20,6 +20,28 @@ mod reexport_pub { } pub use reexport_pub::*; +#[repr(C)] +struct InitUserData { + library: sys::GDExtensionClassLibraryPtr, + #[cfg(since_api = "4.5")] + main_loop_callbacks: sys::GDExtensionMainLoopCallbacks, +} + +#[cfg(since_api = "4.5")] +unsafe extern "C" fn startup_func() { + E::on_main_loop_startup(); +} + +#[cfg(since_api = "4.5")] +unsafe extern "C" fn frame_func() { + E::on_main_loop_frame(); +} + +#[cfg(since_api = "4.5")] +unsafe extern "C" fn shutdown_func() { + E::on_main_loop_shutdown(); +} + #[doc(hidden)] #[deny(unsafe_op_in_unsafe_fn)] pub unsafe fn __gdext_load_library( @@ -60,10 +82,20 @@ pub unsafe fn __gdext_load_library( // Currently no way to express failure; could be exposed to E if necessary. // No early exit, unclear if Godot still requires output parameters to be set. let success = true; + // Leak the userdata. It will be dropped in core level deinitialization. + let userdata = Box::into_raw(Box::new(InitUserData { + library, + #[cfg(since_api = "4.5")] + main_loop_callbacks: sys::GDExtensionMainLoopCallbacks { + startup_func: Some(startup_func::), + frame_func: Some(frame_func::), + shutdown_func: Some(shutdown_func::), + }, + })); let godot_init_params = sys::GDExtensionInitialization { minimum_initialization_level: E::min_level().to_sys(), - userdata: std::ptr::null_mut(), + userdata: userdata.cast::(), initialize: Some(ffi_initialize_layer::), deinitialize: Some(ffi_deinitialize_layer::), }; @@ -88,13 +120,14 @@ pub unsafe fn __gdext_load_library( static LEVEL_SERVERS_CORE_LOADED: AtomicBool = AtomicBool::new(false); unsafe extern "C" fn ffi_initialize_layer( - _userdata: *mut std::ffi::c_void, + userdata: *mut std::ffi::c_void, init_level: sys::GDExtensionInitializationLevel, ) { + let userdata = userdata.cast::().as_ref().unwrap(); let level = InitLevel::from_sys(init_level); let ctx = || format!("failed to initialize GDExtension level `{level:?}`"); - fn try_load(level: InitLevel) { + fn try_load(level: InitLevel, userdata: &InitUserData) { // Workaround for https://github.com/godot-rust/gdext/issues/629: // When using editor plugins, Godot may unload all levels but only reload from Scene upward. // Manually run initialization of lower levels. @@ -102,8 +135,8 @@ unsafe extern "C" fn ffi_initialize_layer( // TODO: Remove this workaround once after the upstream issue is resolved. if level == InitLevel::Scene { if !LEVEL_SERVERS_CORE_LOADED.load(Ordering::Relaxed) { - try_load::(InitLevel::Core); - try_load::(InitLevel::Servers); + try_load::(InitLevel::Core, userdata); + try_load::(InitLevel::Servers, userdata); } } else if level == InitLevel::Core { // When it's normal initialization, the `Servers` level is normally initialized. @@ -112,18 +145,18 @@ unsafe extern "C" fn ffi_initialize_layer( // SAFETY: Godot will call this from the main thread, after `__gdext_load_library` where the library is initialized, // and only once per level. - unsafe { gdext_on_level_init(level) }; + unsafe { gdext_on_level_init(level, userdata) }; E::on_level_init(level); } // Swallow panics. TODO consider crashing if gdext init fails. let _ = crate::private::handle_panic(ctx, || { - try_load::(level); + try_load::(level, userdata); }); } unsafe extern "C" fn ffi_deinitialize_layer( - _userdata: *mut std::ffi::c_void, + userdata: *mut std::ffi::c_void, init_level: sys::GDExtensionInitializationLevel, ) { let level = InitLevel::from_sys(init_level); @@ -134,6 +167,9 @@ unsafe extern "C" fn ffi_deinitialize_layer( if level == InitLevel::Core { // Once the CORE api is unloaded, reset the flag to initial state. LEVEL_SERVERS_CORE_LOADED.store(false, Ordering::Relaxed); + + // Drop the userdata. + drop(Box::from_raw(userdata.cast::())); } E::on_level_deinit(level); @@ -149,7 +185,7 @@ unsafe extern "C" fn ffi_deinitialize_layer( /// - The interface must have been initialized. /// - Must only be called once per level. #[deny(unsafe_op_in_unsafe_fn)] -unsafe fn gdext_on_level_init(level: InitLevel) { +unsafe fn gdext_on_level_init(level: InitLevel, userdata: &InitUserData) { // TODO: in theory, a user could start a thread in one of the early levels, and run concurrent code that messes with the global state // (e.g. class registration). This would break the assumption that the load_class_method_table() calls are exclusive. // We could maybe protect globals with a mutex until initialization is complete, and then move it to a directly-accessible, read-only static. @@ -158,6 +194,15 @@ unsafe fn gdext_on_level_init(level: InitLevel) { unsafe { sys::load_class_method_table(level) }; match level { + InitLevel::Core => { + #[cfg(since_api = "4.5")] + unsafe { + sys::interface_fn!(register_main_loop_callbacks)( + userdata.library, + &raw const userdata.main_loop_callbacks, + ) + }; + } InitLevel::Servers => { // SAFETY: called from the main thread, sys::initialized has already been called. unsafe { sys::discover_main_thread() }; @@ -173,7 +218,6 @@ unsafe fn gdext_on_level_init(level: InitLevel) { crate::docs::register(); } } - _ => (), } crate::registry::class::auto_register_classes(level); @@ -303,6 +347,26 @@ pub unsafe trait ExtensionLibrary { // Nothing by default. } + /// Callback that is called after all initialization levels when Godot is fully initialized. + #[cfg(since_api = "4.5")] + fn on_main_loop_startup() { + // Nothing by default. + } + + /// Callback that is called for every process frame. + /// + /// This will run after all `_process()` methods on Node, and before `ScriptServer::frame()`. + #[cfg(since_api = "4.5")] + fn on_main_loop_frame() { + // Nothing by default. + } + + /// Callback that is called before Godot is shutdown when it is still fully initialized. + #[cfg(since_api = "4.5")] + fn on_main_loop_shutdown() { + // Nothing by default. + } + /// Whether to override the Wasm binary filename used by your GDExtension which the library should expect at runtime. Return `None` /// to use the default where gdext expects either `{YourCrate}.wasm` (default binary name emitted by Rust) or /// `{YourCrate}.threads.wasm` (for builds producing separate single-threaded and multi-threaded binaries). diff --git a/itest/rust/src/lib.rs b/itest/rust/src/lib.rs index 9ed9c0460..201977657 100644 --- a/itest/rust/src/lib.rs +++ b/itest/rust/src/lib.rs @@ -27,4 +27,19 @@ unsafe impl ExtensionLibrary for framework::IntegrationTests { fn on_level_init(level: InitLevel) { object_tests::on_level_init(level); } + + #[cfg(since_api = "4.5")] + fn on_main_loop_startup() { + object_tests::on_main_loop_startup(); + } + + #[cfg(since_api = "4.5")] + fn on_main_loop_frame() { + object_tests::on_main_loop_frame(); + } + + #[cfg(since_api = "4.5")] + fn on_main_loop_shutdown() { + object_tests::on_main_loop_shutdown(); + } } diff --git a/itest/rust/src/object_tests/init_level_test.rs b/itest/rust/src/object_tests/init_level_test.rs index fecb05308..ad4fe397c 100644 --- a/itest/rust/src/object_tests/init_level_test.rs +++ b/itest/rust/src/object_tests/init_level_test.rs @@ -7,8 +7,10 @@ use std::sync::atomic::{AtomicBool, Ordering}; +use godot::builtin::Rid; +use godot::classes::{Engine, IObject, RenderingServer}; use godot::init::InitLevel; -use godot::obj::{NewAlloc, Singleton}; +use godot::obj::{Base, GodotClass, NewAlloc, Singleton}; use godot::register::{godot_api, GodotClass}; use godot::sys::Global; @@ -120,3 +122,45 @@ fn on_init_scene() { pub fn on_init_editor() { // Nothing yet. } + +#[derive(GodotClass)] +#[class(base=Object)] +struct MainLoopCallbackSingleton { + tex: Rid, +} + +#[godot_api] +impl IObject for MainLoopCallbackSingleton { + fn init(_: Base) -> Self { + Self { + tex: RenderingServer::singleton().texture_2d_placeholder_create(), + } + } +} + +pub fn on_main_loop_startup() { + // RenderingServer should be accessible in MainLoop startup and shutdown. + let singleton = MainLoopCallbackSingleton::new_alloc(); + assert!(singleton.bind().tex.is_valid()); + Engine::singleton().register_singleton( + &MainLoopCallbackSingleton::class_id().to_string_name(), + &singleton, + ); +} + +pub fn on_main_loop_frame() { + // Nothing yet. +} + +pub fn on_main_loop_shutdown() { + let singleton = Engine::singleton() + .get_singleton(&MainLoopCallbackSingleton::class_id().to_string_name()) + .unwrap() + .cast::(); + Engine::singleton() + .unregister_singleton(&MainLoopCallbackSingleton::class_id().to_string_name()); + let tex = singleton.bind().tex; + assert!(tex.is_valid()); + RenderingServer::singleton().free_rid(tex); + singleton.free(); +} From 7187618434224f007b26ead0e73a01bb3c6f6136 Mon Sep 17 00:00:00 2001 From: Yarvin Date: Fri, 10 Oct 2025 07:10:49 +0200 Subject: [PATCH 04/12] Codegen: Support sys types in outgoing Ptrcalls. Adds support for sys types (defined in `gdextension_interface.h`) to codegen. --- godot-codegen/src/context.rs | 18 ++++ godot-codegen/src/conv/type_conversions.rs | 9 ++ godot-codegen/src/generator/central_files.rs | 4 + godot-codegen/src/generator/mod.rs | 1 + godot-codegen/src/generator/sys.rs | 82 +++++++++++++++++++ godot-codegen/src/models/domain.rs | 4 + .../special_cases/codegen_special_cases.rs | 1 + 7 files changed, 119 insertions(+) create mode 100644 godot-codegen/src/generator/sys.rs diff --git a/godot-codegen/src/context.rs b/godot-codegen/src/context.rs index 9a62fbbef..c48785604 100644 --- a/godot-codegen/src/context.rs +++ b/godot-codegen/src/context.rs @@ -12,6 +12,7 @@ use quote::{format_ident, ToTokens}; use crate::generator::method_tables::MethodTableKey; use crate::generator::notifications; +use crate::generator::sys::SYS_PARAMS; use crate::models::domain::{ArgPassing, GodotTy, RustTy, TyName}; use crate::models::json::{ JsonBuiltinClass, JsonBuiltinMethod, JsonClass, JsonClassConstant, JsonClassMethod, @@ -28,6 +29,9 @@ pub struct Context<'a> { /// Which interface traits are generated (`false` for "Godot-abstract"/final classes). classes_final: HashMap, cached_rust_types: HashMap, + /// Various pointers defined in `gdextension_interface`, for example `GDExtensionInitializationFunction`. + /// Used in some APIs that are not exposed to GDScript. + sys_types: HashSet<&'a str>, notifications_by_class: HashMap>, classes_with_signals: HashSet, notification_enum_names_by_class: HashMap, @@ -43,6 +47,8 @@ impl<'a> Context<'a> { ctx.singletons.insert(class.name.as_str()); } + Self::populate_sys_types(&mut ctx); + ctx.builtin_types.insert("Variant"); // not part of builtin_classes for builtin in api.builtin_classes.iter() { let ty_name = builtin.name.as_str(); @@ -152,6 +158,14 @@ impl<'a> Context<'a> { ctx } + /// Adds Godot pointer types to [`Context`]. + /// + /// Godot pointer types, for example `GDExtensionInitializationFunction`, are defined in `gdextension_interface` + /// but aren't described in `extension_api.json` – despite being used as parameters in various APIs. + fn populate_sys_types(ctx: &mut Context) { + ctx.sys_types.extend(SYS_PARAMS.iter().map(|p| p.type_())); + } + fn populate_notification_constants( class_name: &TyName, constants: &[JsonClassConstant], @@ -293,6 +307,10 @@ impl<'a> Context<'a> { self.native_structures_types.contains(ty_name) } + pub fn is_sys(&self, ty_name: &str) -> bool { + self.sys_types.contains(ty_name) + } + pub fn is_singleton(&self, class_name: &TyName) -> bool { self.singletons.contains(class_name.godot_ty.as_str()) } diff --git a/godot-codegen/src/conv/type_conversions.rs b/godot-codegen/src/conv/type_conversions.rs index 5e55d922b..e22c49d59 100644 --- a/godot-codegen/src/conv/type_conversions.rs +++ b/godot-codegen/src/conv/type_conversions.rs @@ -172,6 +172,15 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy { // .trim() is necessary here, as Godot places a space between a type and the stars when representing a double pointer. // Example: "int*" but "int **". + if ctx.is_sys(ty.trim()) { + let ty = rustify_ty(&ty); + return RustTy::RawPointer { + inner: Box::new(RustTy::SysIdent { + tokens: quote! { sys::#ty }, + }), + is_const, + }; + } let inner_type = to_rust_type(ty.trim(), None, ctx); return RustTy::RawPointer { inner: Box::new(inner_type), diff --git a/godot-codegen/src/generator/central_files.rs b/godot-codegen/src/generator/central_files.rs index 15a562e2b..8901a3336 100644 --- a/godot-codegen/src/generator/central_files.rs +++ b/godot-codegen/src/generator/central_files.rs @@ -10,6 +10,7 @@ use quote::{format_ident, quote, ToTokens}; use crate::context::Context; use crate::conv; +use crate::generator::sys::make_godotconvert_for_systypes; use crate::generator::{enums, gdext_build_struct}; use crate::models::domain::ExtensionApi; use crate::util::ident; @@ -60,6 +61,7 @@ pub fn make_core_central_code(api: &ExtensionApi, ctx: &mut Context) -> TokenStr let (global_enum_defs, global_reexported_enum_defs) = make_global_enums(api); let variant_type_traits = make_variant_type_enum(api, false); + let sys_types_godotconvert_impl = make_godotconvert_for_systypes(); // TODO impl Clone, Debug, PartialEq, PartialOrd, Hash for VariantDispatch // TODO could use try_to().unwrap_unchecked(), since type is already verified. Also directly overload from_variant(). @@ -121,6 +123,8 @@ pub fn make_core_central_code(api: &ExtensionApi, ctx: &mut Context) -> TokenStr use crate::sys; #( #global_reexported_enum_defs )* } + + #( #sys_types_godotconvert_impl )* } } diff --git a/godot-codegen/src/generator/mod.rs b/godot-codegen/src/generator/mod.rs index 15b315270..68082b711 100644 --- a/godot-codegen/src/generator/mod.rs +++ b/godot-codegen/src/generator/mod.rs @@ -28,6 +28,7 @@ pub mod method_tables; pub mod native_structures; pub mod notifications; pub mod signals; +pub mod sys; pub mod utility_functions; pub mod virtual_definition_consts; pub mod virtual_traits; diff --git a/godot-codegen/src/generator/sys.rs b/godot-codegen/src/generator/sys.rs new file mode 100644 index 000000000..2c265d1d1 --- /dev/null +++ b/godot-codegen/src/generator/sys.rs @@ -0,0 +1,82 @@ +/* + * Copyright (c) godot-rust; Bromeon and contributors. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +use proc_macro2::{Ident, TokenStream}; +use quote::{quote, ToTokens}; + +use crate::util::ident; + +#[allow(unused)] +pub enum SysTypeParam { + /// `* mut SysType` + Mut(&'static str), + /// `* const SysType` + Const(&'static str), +} + +impl SysTypeParam { + pub fn type_(&self) -> &'static str { + match self { + SysTypeParam::Mut(s) | SysTypeParam::Const(s) => s, + } + } + + fn to_ident(&self) -> Ident { + match self { + SysTypeParam::Mut(s) | SysTypeParam::Const(s) => ident(s), + } + } +} + +impl ToTokens for SysTypeParam { + fn to_tokens(&self, tokens: &mut TokenStream) { + let type_ident = self.to_ident(); + match self { + SysTypeParam::Mut(_) => quote! { * mut crate::sys::#type_ident }, + SysTypeParam::Const(_) => quote! { * const crate::sys::#type_ident }, + } + .to_tokens(tokens); + } +} + +/// SysTypes used as parameters in various APIs defined in `extension_api.json`. +// Currently hardcoded and it probably will stay this way – extracting types from gdextension_interface is way too messy. +// Must be different abstraction to avoid clashes with other types passed as pointers (e.g. *Glyph). +pub static SYS_PARAMS: &[SysTypeParam] = &[ + #[cfg(since_api = "4.6")] + SysTypeParam::Const("GDExtensionInitializationFunction"), +]; + +/// Creates `GodotConvert`, `ToGodot` and `FromGodot` impl +/// for SysTypes – various pointer types declared in `gdextension_interface`. +pub fn make_godotconvert_for_systypes() -> Vec { + let mut tokens = vec![]; + for sys_type_param in SYS_PARAMS { + tokens.push( + quote! { + impl crate::meta::GodotConvert for #sys_type_param { + type Via = i64; + + } + + impl crate::meta::ToGodot for #sys_type_param { + type Pass = crate::meta::ByValue; + fn to_godot(&self) -> Self::Via { + * self as i64 + } + } + + impl crate::meta::FromGodot for #sys_type_param { + fn try_from_godot(via: Self::Via) -> Result < Self, crate::meta::error::ConvertError > { + Ok(via as Self) + } + } + } + ) + } + tokens +} diff --git a/godot-codegen/src/models/domain.rs b/godot-codegen/src/models/domain.rs index 7da3aa270..d331edc7b 100644 --- a/godot-codegen/src/models/domain.rs +++ b/godot-codegen/src/models/domain.rs @@ -699,6 +699,9 @@ pub struct GodotTy { pub enum RustTy { /// `bool`, `Vector3i`, `Array`, `GString` BuiltinIdent { ty: Ident, arg_passing: ArgPassing }, + /// Pointers declared in `gdextension_interface` such as `sys::GDExtensionInitializationFunction` + /// used as parameters in some APIs. + SysIdent { tokens: TokenStream }, /// `Array` /// @@ -789,6 +792,7 @@ impl ToTokens for RustTy { RustTy::EngineEnum { tokens: path, .. } => path.to_tokens(tokens), RustTy::EngineClass { tokens: path, .. } => path.to_tokens(tokens), RustTy::ExtenderReceiver { tokens: path } => path.to_tokens(tokens), + RustTy::SysIdent { tokens: path } => path.to_tokens(tokens), } } } diff --git a/godot-codegen/src/special_cases/codegen_special_cases.rs b/godot-codegen/src/special_cases/codegen_special_cases.rs index c045e4630..816c30f5c 100644 --- a/godot-codegen/src/special_cases/codegen_special_cases.rs +++ b/godot-codegen/src/special_cases/codegen_special_cases.rs @@ -49,6 +49,7 @@ fn is_type_excluded(ty: &str, ctx: &mut Context) -> bool { RustTy::BuiltinIdent { .. } => false, RustTy::BuiltinArray { .. } => false, RustTy::RawPointer { inner, .. } => is_rust_type_excluded(inner), + RustTy::SysIdent { .. } => true, RustTy::EngineArray { elem_class, .. } => is_class_excluded(elem_class.as_str()), RustTy::EngineEnum { surrounding_class, .. From 88de7c82331579e3a5191a1e4680d04eb123d6cb Mon Sep 17 00:00:00 2001 From: Yarvin Date: Mon, 6 Oct 2025 09:36:24 +0200 Subject: [PATCH 05/12] Codegen for `Array` - required to properly initialize and cache Array runtime type. ----- Methods such as `duplicate_...` were initializing and caching typed `VariantArray` as a return value for outgoing calls. We fix it by initializing proper `Array` in the first place. --- godot-codegen/src/generator/builtins.rs | 33 ++++++++---- .../src/generator/default_parameters.rs | 2 +- .../src/generator/functions_common.rs | 13 +++-- godot-codegen/src/models/domain.rs | 53 +++++++++++++++++++ godot-codegen/src/models/domain_mapping.rs | 17 ++++-- .../special_cases/codegen_special_cases.rs | 1 + .../src/special_cases/special_cases.rs | 20 ++++++- godot-core/src/builtin/collections/array.rs | 37 +++++-------- .../builtin_tests/containers/array_test.rs | 19 +++++++ 9 files changed, 150 insertions(+), 45 deletions(-) diff --git a/godot-codegen/src/generator/builtins.rs b/godot-codegen/src/generator/builtins.rs index 321fd0a8b..bfdcc9acf 100644 --- a/godot-codegen/src/generator/builtins.rs +++ b/godot-codegen/src/generator/builtins.rs @@ -197,14 +197,27 @@ fn make_special_builtin_methods(class_name: &TyName, _ctx: &Context) -> TokenStr /// Get the safety docs of an unsafe method, or `None` if it is safe. fn method_safety_doc(class_name: &TyName, method: &BuiltinMethod) -> Option { - if class_name.godot_ty == "Array" - && &method.return_value().type_tokens().to_string() == "VariantArray" - { - return Some(quote! { - /// # Safety - /// - /// You must ensure that the returned array fulfils the safety invariants of [`Array`](crate::builtin::Array). - }); + if class_name.godot_ty == "Array" { + if method.is_generic() { + return Some(quote! { + /// # Safety + /// You must ensure that the returned array fulfils the safety invariants of [`Array`](crate::builtin::Array), this being: + /// - Any values written to the array must match the runtime type of the array. + /// - Any values read from the array must be convertible to the type `T`. + /// + /// If the safety invariant of `Array` is intact, which it must be for any publicly accessible arrays, then `T` must match + /// the runtime type of the array. This then implies that both of the conditions above hold. This means that you only need + /// to keep the above conditions in mind if you are intentionally violating the safety invariant of `Array`. + /// + /// In the current implementation, both cases will produce a panic rather than undefined behavior, but this should not be relied upon. + }); + } else if &method.return_value().type_tokens().to_string() == "VariantArray" { + return Some(quote! { + /// # Safety + /// + /// You must ensure that the returned array fulfils the safety invariants of [`Array`](crate::builtin::Array). + }); + } } None @@ -252,11 +265,13 @@ fn make_builtin_method_definition( let receiver = functions_common::make_receiver(method.qualifier(), ffi_arg_in); let object_ptr = &receiver.ffi_arg; + let maybe_generic_params = method.return_value().generic_params(); + let ptrcall_invocation = quote! { let method_bind = sys::builtin_method_table().#fptr_access; - Signature::::out_builtin_ptrcall( + Signature::::out_builtin_ptrcall( method_bind, #builtin_name_str, #method_name_str, diff --git a/godot-codegen/src/generator/default_parameters.rs b/godot-codegen/src/generator/default_parameters.rs index 7f0ac3d82..520596a60 100644 --- a/godot-codegen/src/generator/default_parameters.rs +++ b/godot-codegen/src/generator/default_parameters.rs @@ -11,7 +11,7 @@ use quote::{format_ident, quote}; use crate::generator::functions_common; use crate::generator::functions_common::{ - make_arg_expr, make_param_or_field_type, FnArgExpr, FnCode, FnKind, FnParamDecl, FnParamTokens, + make_arg_expr, make_param_or_field_type, FnArgExpr, FnCode, FnKind, FnParamDecl, }; use crate::models::domain::{FnParam, FnQualifier, Function, RustTy, TyName}; use crate::util::{ident, safe_ident}; diff --git a/godot-codegen/src/generator/functions_common.rs b/godot-codegen/src/generator/functions_common.rs index aa14d177b..0adcbd3c6 100644 --- a/godot-codegen/src/generator/functions_common.rs +++ b/godot-codegen/src/generator/functions_common.rs @@ -142,7 +142,6 @@ pub fn make_function_definition( callsig_param_types: param_types, callsig_lifetime_args, arg_exprs: arg_names, - func_general_lifetime: fn_lifetime, } = if sig.is_virtual() { make_params_exprs_virtual(sig.params().iter(), sig) } else { @@ -175,11 +174,14 @@ pub fn make_function_definition( default_structs_code = TokenStream::new(); }; + let maybe_func_generic_params = sig.return_value().generic_params(); + let maybe_func_generic_bounds = sig.return_value().where_clause(); + let call_sig_decl = { let return_ty = &sig.return_value().type_tokens(); quote! { - type CallRet = #return_ty; + type CallRet #maybe_func_generic_params = #return_ty; type CallParams #callsig_lifetime_args = (#(#param_types,)*); } }; @@ -279,10 +281,12 @@ pub fn make_function_definition( quote! { #maybe_safety_doc - #vis #maybe_unsafe fn #primary_fn_name #fn_lifetime ( + #vis #maybe_unsafe fn #primary_fn_name #maybe_func_generic_params ( #receiver_param #( #params, )* - ) #return_decl { + ) #return_decl + #maybe_func_generic_bounds + { #call_sig_decl let args = (#( #arg_names, )*); @@ -489,6 +493,7 @@ pub(crate) fn make_param_or_field_type( .. } | RustTy::BuiltinArray { .. } + | RustTy::GenericArray | RustTy::EngineArray { .. } => { let lft = lifetimes.next(); special_ty = Some(quote! { RefArg<#lft, #ty> }); diff --git a/godot-codegen/src/models/domain.rs b/godot-codegen/src/models/domain.rs index d331edc7b..3500cc3ff 100644 --- a/godot-codegen/src/models/domain.rs +++ b/godot-codegen/src/models/domain.rs @@ -300,28 +300,40 @@ pub trait Function: fmt::Display { fn name(&self) -> &str { &self.common().name } + /// Rust name as `Ident`. Might be cached in future. fn name_ident(&self) -> Ident { safe_ident(self.name()) } + fn godot_name(&self) -> &str { &self.common().godot_name } + fn params(&self) -> &[FnParam] { &self.common().parameters } + fn return_value(&self) -> &FnReturn { &self.common().return_value } + fn is_vararg(&self) -> bool { self.common().is_vararg } + fn is_private(&self) -> bool { self.common().is_private } + fn is_virtual(&self) -> bool { matches!(self.direction(), FnDirection::Virtual { .. }) } + + fn is_generic(&self) -> bool { + matches!(self.return_value().type_, Some(RustTy::GenericArray)) + } + fn direction(&self) -> FnDirection { self.common().direction } @@ -621,6 +633,13 @@ impl FnReturn { Self::with_enum_replacements(return_value, &[], ctx) } + pub fn with_generic_builtin(generic_type: RustTy) -> Self { + Self { + decl: generic_type.return_decl(), + type_: Some(generic_type), + } + } + pub fn with_enum_replacements( return_value: &Option, replacements: EnumReplacements, @@ -669,6 +688,14 @@ impl FnReturn { } } + pub fn generic_params(&self) -> Option { + self.type_.as_ref()?.generic_params() + } + + pub fn where_clause(&self) -> Option { + self.type_.as_ref()?.where_clause() + } + pub fn call_result_decl(&self) -> TokenStream { let ret = self.type_tokens(); quote! { -> Result<#ret, crate::meta::error::CallError> } @@ -708,6 +735,11 @@ pub enum RustTy { /// Note that untyped arrays are mapped as `BuiltinIdent("Array")`. BuiltinArray { elem_type: TokenStream }, + /// Will be included as `Array` in the generated source. + /// + /// Set by [`builtin_method_generic_ret`](crate::special_cases::builtin_method_generic_ret) + GenericArray, + /// C-style raw pointer to a `RustTy`. RawPointer { inner: Box, is_const: bool }, @@ -758,10 +790,30 @@ impl RustTy { pub fn return_decl(&self) -> TokenStream { match self { Self::EngineClass { tokens, .. } => quote! { -> Option<#tokens> }, + Self::GenericArray => quote! { -> Array }, other => quote! { -> #other }, } } + pub fn generic_params(&self) -> Option { + if matches!(self, Self::GenericArray) { + Some(quote! { < Ret > }) + } else { + None + } + } + + pub fn where_clause(&self) -> Option { + if matches!(self, Self::GenericArray) { + Some(quote! { + where + Ret: crate::meta::ArrayElement, + }) + } else { + None + } + } + pub fn is_integer(&self) -> bool { let RustTy::BuiltinIdent { ty, .. } = self else { return false; @@ -792,6 +844,7 @@ impl ToTokens for RustTy { RustTy::EngineEnum { tokens: path, .. } => path.to_tokens(tokens), RustTy::EngineClass { tokens: path, .. } => path.to_tokens(tokens), RustTy::ExtenderReceiver { tokens: path } => path.to_tokens(tokens), + RustTy::GenericArray => quote! { Array }.to_tokens(tokens), RustTy::SysIdent { tokens: path } => path.to_tokens(tokens), } } diff --git a/godot-codegen/src/models/domain_mapping.rs b/godot-codegen/src/models/domain_mapping.rs index dbc89d0ee..477075028 100644 --- a/godot-codegen/src/models/domain_mapping.rs +++ b/godot-codegen/src/models/domain_mapping.rs @@ -366,10 +366,17 @@ impl BuiltinMethod { return None; } - let return_value = method - .return_type - .as_deref() - .map(JsonMethodReturn::from_type_no_meta); + let return_value = if let Some(generic) = + special_cases::builtin_method_generic_ret(builtin_name, method) + { + generic + } else { + let return_value = &method + .return_type + .as_deref() + .map(JsonMethodReturn::from_type_no_meta); + FnReturn::new(return_value, ctx) + }; Some(Self { common: FunctionCommon { @@ -381,7 +388,7 @@ impl BuiltinMethod { parameters: FnParam::builder() .no_defaults() .build_many(&method.arguments, ctx), - return_value: FnReturn::new(&return_value, ctx), + return_value, is_vararg: method.is_vararg, is_private: false, // See 'exposed' below. Could be special_cases::is_method_private(builtin_name, &method.name), is_virtual_required: false, diff --git a/godot-codegen/src/special_cases/codegen_special_cases.rs b/godot-codegen/src/special_cases/codegen_special_cases.rs index 816c30f5c..f254ace0f 100644 --- a/godot-codegen/src/special_cases/codegen_special_cases.rs +++ b/godot-codegen/src/special_cases/codegen_special_cases.rs @@ -48,6 +48,7 @@ fn is_type_excluded(ty: &str, ctx: &mut Context) -> bool { match ty { RustTy::BuiltinIdent { .. } => false, RustTy::BuiltinArray { .. } => false, + RustTy::GenericArray => false, RustTy::RawPointer { inner, .. } => is_rust_type_excluded(inner), RustTy::SysIdent { .. } => true, RustTy::EngineArray { elem_class, .. } => is_class_excluded(elem_class.as_str()), diff --git a/godot-codegen/src/special_cases/special_cases.rs b/godot-codegen/src/special_cases/special_cases.rs index 518a5c1af..afa444f21 100644 --- a/godot-codegen/src/special_cases/special_cases.rs +++ b/godot-codegen/src/special_cases/special_cases.rs @@ -33,7 +33,7 @@ use proc_macro2::Ident; use crate::conv::to_enum_type_uncached; use crate::models::domain::{ - ClassCodegenLevel, Enum, EnumReplacements, RustTy, TyName, VirtualMethodPresence, + ClassCodegenLevel, Enum, EnumReplacements, FnReturn, RustTy, TyName, VirtualMethodPresence, }; use crate::models::json::{JsonBuiltinMethod, JsonClassMethod, JsonSignal, JsonUtilityFunction}; use crate::special_cases::codegen_special_cases; @@ -778,6 +778,24 @@ pub fn is_builtin_method_deleted(_class_name: &TyName, method: &JsonBuiltinMetho codegen_special_cases::is_builtin_method_excluded(method) } +/// Returns some generic type – such as `GenericArray` representing `Array` – if method is marked as generic, `None` otherwise. +/// +/// Usually required to initialize the return value and cache its type (see also https://github.com/godot-rust/gdext/pull/1357). +pub fn builtin_method_generic_ret( + class_name: &TyName, + method: &JsonBuiltinMethod, +) -> Option { + match ( + class_name.rust_ty.to_string().as_str(), + method.name.as_str(), + ) { + ("Array", "duplicate") | ("Array", "slice") => { + Some(FnReturn::with_generic_builtin(RustTy::GenericArray)) + } + _ => None, + } +} + /// True if signal is absent from codegen (only when surrounding class is excluded). pub fn is_signal_deleted(_class_name: &TyName, signal: &JsonSignal) -> bool { // If any argument type (a class) is excluded. diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs index cdbd5ba5d..4d263e27b 100644 --- a/godot-core/src/builtin/collections/array.rs +++ b/godot-core/src/builtin/collections/array.rs @@ -533,12 +533,11 @@ impl Array { /// To create a deep copy, use [`duplicate_deep()`][Self::duplicate_deep] instead. /// To create a new reference to the same array data, use [`clone()`][Clone::clone]. pub fn duplicate_shallow(&self) -> Self { - // SAFETY: We never write to the duplicated array, and all values read are read as `Variant`. - let duplicate: VariantArray = unsafe { self.as_inner().duplicate(false) }; + // SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type + let duplicate: Self = unsafe { self.as_inner().duplicate(false) }; - // SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type. - let result = unsafe { duplicate.assume_type() }; - result.with_cache(self) + // Note: cache is being set while initializing the duplicate as a return value for above call. + duplicate } /// Returns a deep copy of the array. All nested arrays and dictionaries are duplicated and @@ -548,12 +547,11 @@ impl Array { /// To create a shallow copy, use [`duplicate_shallow()`][Self::duplicate_shallow] instead. /// To create a new reference to the same array data, use [`clone()`][Clone::clone]. pub fn duplicate_deep(&self) -> Self { - // SAFETY: We never write to the duplicated array, and all values read are read as `Variant`. - let duplicate: VariantArray = unsafe { self.as_inner().duplicate(true) }; + // SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type + let duplicate: Self = unsafe { self.as_inner().duplicate(true) }; - // SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type. - let result = unsafe { duplicate.assume_type() }; - result.with_cache(self) + // Note: cache is being set while initializing the duplicate as a return value for above call. + duplicate } /// Returns a sub-range `begin..end` as a new `Array`. @@ -588,13 +586,10 @@ impl Array { let (begin, end) = range.signed(); let end = end.unwrap_or(i32::MAX as i64); - // SAFETY: The type of the array is `T` and we convert the returned array to an `Array` immediately. - let subarray: VariantArray = - unsafe { self.as_inner().slice(begin, end, step as i64, deep) }; + // SAFETY: slice() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type. + let subarray: Self = unsafe { self.as_inner().slice(begin, end, step as i64, deep) }; - // SAFETY: slice() returns a typed array with the same type as Self. - let result = unsafe { subarray.assume_type() }; - result.with_cache(self) + subarray } /// Returns an iterator over the elements of the `Array`. Note that this takes the array @@ -950,8 +945,7 @@ impl Array { } } - /// Changes the generic type on this array, without changing its contents. Needed for API - /// functions that return a variant array even though we know its type, and for API functions + /// Changes the generic type on this array, without changing its contents. Needed for API functions /// that take a variant array even though we want to pass a typed one. /// /// # Safety @@ -966,13 +960,6 @@ impl Array { /// Note also that any `GodotType` can be written to a `Variant` array. /// /// In the current implementation, both cases will produce a panic rather than undefined behavior, but this should not be relied upon. - unsafe fn assume_type(self) -> Array { - // The memory layout of `Array` does not depend on `T`. - std::mem::transmute::, Array>(self) - } - - /// # Safety - /// See [`assume_type`](Self::assume_type). unsafe fn assume_type_ref(&self) -> &Array { // The memory layout of `Array` does not depend on `T`. std::mem::transmute::<&Array, &Array>(self) diff --git a/itest/rust/src/builtin_tests/containers/array_test.rs b/itest/rust/src/builtin_tests/containers/array_test.rs index ea8e3de98..ea8fc7a5a 100644 --- a/itest/rust/src/builtin_tests/containers/array_test.rs +++ b/itest/rust/src/builtin_tests/containers/array_test.rs @@ -677,6 +677,25 @@ func make_array() -> Array[CustomScriptForArrays]: assert_eq!(script.get_global_name(), "CustomScriptForArrays".into()); } +// Test that proper type has been set&cached while creating new Array. +// https://github.com/godot-rust/gdext/pull/1357 +#[itest] +fn array_inner_type() { + let primary = Array::::new(); + + let secondary = primary.duplicate_shallow(); + assert_eq!(secondary.element_type(), primary.element_type()); + + let secondary = primary.duplicate_deep(); + assert_eq!(secondary.element_type(), primary.element_type()); + + let subarray = primary.subarray_deep(.., None); + assert_eq!(subarray.element_type(), primary.element_type()); + + let subarray = primary.subarray_shallow(.., None); + assert_eq!(subarray.element_type(), primary.element_type()); +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- // Class definitions From 66e74c9d66e1278f6da3abbcd120de472ec47912 Mon Sep 17 00:00:00 2001 From: Yarvin Date: Mon, 6 Oct 2025 19:59:11 +0200 Subject: [PATCH 06/12] =?UTF-8?q?Remove=20`func=5Fgeneral=5Flifetime`=20fr?= =?UTF-8?q?om=20FnParamTokens=20since=20it=20was=20never=20set.=20Remove?= =?UTF-8?q?=20`ExBuilderConstructor`=20=E2=80=93=20it=20was=20doing=20noth?= =?UTF-8?q?ing,=20and=20in=20practice=20it=20was=20supposed=20to=20do=20th?= =?UTF-8?q?e=20same=20thing=20as=20`ExBuilderConstructorLifetimed`.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- godot-codegen/src/generator/default_parameters.rs | 11 +---------- godot-codegen/src/generator/functions_common.rs | 7 +------ 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/godot-codegen/src/generator/default_parameters.rs b/godot-codegen/src/generator/default_parameters.rs index 520596a60..8ec204cb2 100644 --- a/godot-codegen/src/generator/default_parameters.rs +++ b/godot-codegen/src/generator/default_parameters.rs @@ -59,15 +59,6 @@ pub fn make_function_definition_with_defaults( &default_fn_params, ); - // ExBuilder::new() constructor signature. - let FnParamTokens { - func_general_lifetime: simple_fn_lifetime, - .. - } = fns::make_params_exprs( - required_fn_params.iter().cloned(), - FnKind::ExBuilderConstructor, - ); - let return_decl = &sig.return_value().decl; // If either the builder has a lifetime (non-static/global method), or one of its parameters is a reference, @@ -119,7 +110,7 @@ pub fn make_function_definition_with_defaults( // Lifetime is set if any parameter is a reference. #[doc = #default_parameter_usage] #[inline] - #vis fn #simple_fn_name #simple_fn_lifetime ( + #vis fn #simple_fn_name ( #simple_receiver_param #( #class_method_required_params, )* ) #return_decl { diff --git a/godot-codegen/src/generator/functions_common.rs b/godot-codegen/src/generator/functions_common.rs index 0adcbd3c6..e1600587c 100644 --- a/godot-codegen/src/generator/functions_common.rs +++ b/godot-codegen/src/generator/functions_common.rs @@ -98,7 +98,6 @@ pub struct FnParamTokens { /// Generic argument list `<'a0, 'a1, ...>` after `type CallSig`, if available. pub callsig_lifetime_args: Option, pub arg_exprs: Vec, - pub func_general_lifetime: Option, } pub fn make_function_definition( @@ -361,10 +360,7 @@ pub(crate) enum FnKind { /// `call()` forwarding to `try_call()`. DelegateTry, - /// Default extender `new()` associated function -- optional receiver and required parameters. - ExBuilderConstructor, - - /// Same as [`ExBuilderConstructor`], but for a builder with an explicit lifetime. + /// Default extender `new()` associated function -- optional receiver and required parameters. Has an explicit lifetime. ExBuilderConstructorLifetimed, /// Default extender `new()` associated function -- only default parameters. @@ -577,7 +573,6 @@ pub(crate) fn make_params_exprs<'a>( // Methods relevant in the context of default parameters. Flow in this order. // Note that for builder methods of Ex* structs, there's a direct call in default_parameters.rs to the parameter manipulation methods, // bypassing this method. So one case is missing here. - FnKind::ExBuilderConstructor => (FnParamDecl::FnPublic, FnArgExpr::StoreInField), FnKind::ExBuilderConstructorLifetimed => { (FnParamDecl::FnPublicLifetime, FnArgExpr::StoreInField) } From ae07e11bb2c1e35fd574608f30a11bc3043cc966 Mon Sep 17 00:00:00 2001 From: Yarvin Date: Sat, 4 Oct 2025 11:02:33 +0200 Subject: [PATCH 07/12] Doc bugfixes and improvements - Bugfix: Handle docs in `#[godot_api(secondary)]` - Code tweaks: Separate docs modules to not pollute rest of the code with `#[cfg(all(feature = "register-docs", since_api = "4.3"))]`. Simplify registration logic. - Create separate PluginRegistry for Docs and Docs only. --- godot-core/src/docs.rs | 161 +++++++++++------- godot-core/src/private.rs | 25 +-- godot-core/src/registry/class.rs | 11 +- godot-core/src/registry/plugin.rs | 31 +--- .../src/class/data_models/inherent_impl.rs | 27 +-- .../class/data_models/interface_trait_impl.rs | 13 +- godot-macros/src/class/derive_godot_class.rs | 18 +- .../src/{docs.rs => docs/extract_docs.rs} | 4 +- godot-macros/src/docs/mod.rs | 115 +++++++++++++ godot-macros/src/lib.rs | 1 - .../rust/src/register_tests/constant_test.rs | 2 - .../src/register_tests/register_docs_test.rs | 14 ++ .../register_tests/res/registered_docs.xml | 24 ++- 13 files changed, 284 insertions(+), 162 deletions(-) rename godot-macros/src/{docs.rs => docs/extract_docs.rs} (99%) create mode 100644 godot-macros/src/docs/mod.rs diff --git a/godot-core/src/docs.rs b/godot-core/src/docs.rs index ef597cd1f..46d04a8a3 100644 --- a/godot-core/src/docs.rs +++ b/godot-core/src/docs.rs @@ -8,7 +8,41 @@ use std::collections::HashMap; use crate::meta::ClassId; -use crate::registry::plugin::{ITraitImpl, InherentImpl, PluginItem, Struct}; +use crate::obj::GodotClass; + +/// Piece of information that is gathered by the self-registration ("plugin") system. +/// +/// You should not manually construct this struct, but rather use [`DocsPlugin::new()`]. +#[derive(Debug)] +pub struct DocsPlugin { + /// The name of the class to register docs for. + class_name: ClassId, + + /// The actual item being registered. + item: DocsItem, +} + +impl DocsPlugin { + /// Creates a new `DocsPlugin`, automatically setting the `class_name` to the values defined in [`GodotClass`]. + pub fn new(item: DocsItem) -> Self { + Self { + class_name: T::class_id(), + item, + } + } +} + +type ITraitImplDocs = &'static str; + +#[derive(Debug)] +pub enum DocsItem { + /// Docs for `#[derive(GodotClass)] struct MyClass`. + Struct(StructDocs), + /// Docs for `#[godot_api] impl MyClass`. + InherentImpl(InherentImplDocs), + /// Docs for `#[godot_api] impl ITrait for MyClass`. + ITraitImpl(ITraitImplDocs), +} /// Created for documentation on /// ```ignore @@ -29,7 +63,7 @@ pub struct StructDocs { pub members: &'static str, } -/// Keeps documentation for inherent `impl` blocks, such as: +/// Keeps documentation for inherent `impl` blocks (primary and secondary), such as: /// ```ignore /// #[godot_api] /// impl Struct { @@ -46,18 +80,19 @@ pub struct StructDocs { /// } /// ``` /// All fields are XML parts, escaped where necessary. -#[derive(Default, Copy, Clone, Debug)] +#[derive(Default, Clone, Debug)] pub struct InherentImplDocs { - pub methods: Vec<&'static str>, - pub signals: Vec<&'static str>, - pub constants: Vec<&'static str>, + pub methods: &'static str, + pub signals: &'static str, + pub constants: &'static str, } #[derive(Default)] struct DocPieces { definition: StructDocs, - inherent: InherentImplDocs, - virtual_methods: Vec<&'static str>, + methods: Vec<&'static str>, + signals: Vec<&'static str>, + constants: Vec<&'static str>, } /// This function scours the registered plugins to find their documentation pieces, @@ -76,68 +111,66 @@ struct DocPieces { #[doc(hidden)] pub fn gather_xml_docs() -> impl Iterator { let mut map = HashMap::::new(); - crate::private::iterate_plugins(|x| { + crate::private::iterate_docs_plugins(|x| { let class_name = x.class_name; - match &x.item { - PluginItem::InherentImpl(InherentImpl { docs, .. }) => { - map.entry(class_name).or_default().inherent = docs.clone(); + DocsItem::Struct(s) => { + map.entry(class_name).or_default().definition = *s; } - PluginItem::ITraitImpl(ITraitImpl { - virtual_method_docs, - .. - }) => map - .entry(class_name) - .or_default() - .virtual_methods - .push(virtual_method_docs), - - PluginItem::Struct(Struct { docs, .. }) => { - map.entry(class_name).or_default().definition = *docs; + DocsItem::InherentImpl(trait_docs) => { + let InherentImplDocs { + methods, + constants, + signals, + } = trait_docs; + map.entry(class_name).or_default().methods.push(methods); + map.entry(class_name) + .and_modify(|pieces| pieces.constants.push(constants)); + map.entry(class_name) + .and_modify(|pieces| pieces.signals.push(signals)); + } + DocsItem::ITraitImpl(methods) => { + map.entry(class_name).or_default().methods.push(methods); } - - _ => (), } }); map.into_iter().map(|(class, pieces)| { - let StructDocs { - base, - description, - experimental, - deprecated, - members, - } = pieces.definition; - - let virtual_methods = pieces.virtual_methods; - - let mut method_docs = String::from_iter(pieces.inherent.methods); - let signal_docs = String::from_iter(pieces.inherent.signals); - let constant_docs = String::from_iter(pieces.inherent.constants); - - method_docs.extend(virtual_methods); - let methods_block = if method_docs.is_empty() { - String::new() - } else { - format!("{method_docs}") - }; - let signals_block = if signal_docs.is_empty() { - String::new() - } else { - format!("{signal_docs}") - }; - let constants_block = if constant_docs.is_empty() { - String::new() - } else { - format!("{constant_docs}") - }; - let (brief, description) = match description - .split_once("[br]") { - Some((brief, description)) => (brief, description.trim_start_matches("[br]")), - None => (description, ""), - }; - - format!(r#" + let StructDocs { + base, + description, + experimental, + deprecated, + members, + } = pieces.definition; + + + let method_docs = String::from_iter(pieces.methods); + let signal_docs = String::from_iter(pieces.signals); + let constant_docs = String::from_iter(pieces.constants); + + let methods_block = if method_docs.is_empty() { + String::new() + } else { + format!("{method_docs}") + }; + let signals_block = if signal_docs.is_empty() { + String::new() + } else { + format!("{signal_docs}") + }; + let constants_block = if constant_docs.is_empty() { + String::new() + } else { + format!("{constant_docs}") + }; + let (brief, description) = match description + .split_once("[br]") { + Some((brief, description)) => (brief, description.trim_start_matches("[br]")), + None => (description, ""), + }; + + format!(r#" {brief} {description} @@ -146,8 +179,8 @@ pub fn gather_xml_docs() -> impl Iterator { {signals_block} {members} "#) - }, - ) + }, + ) } /// # Safety diff --git a/godot-core/src/private.rs b/godot-core/src/private.rs index fbaa54555..9f1ac4009 100644 --- a/godot-core/src/private.rs +++ b/godot-core/src/private.rs @@ -22,6 +22,8 @@ use crate::{classes, sys}; // Public re-exports 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; #[cfg(feature = "trace")] pub use crate::meta::trace; @@ -52,6 +54,8 @@ static CALL_ERRORS: Global = Global::default(); static ERROR_PRINT_LEVEL: atomic::AtomicU8 = atomic::AtomicU8::new(2); sys::plugin_registry!(pub __GODOT_PLUGIN_REGISTRY: ClassPlugin); +#[cfg(all(since_api = "4.3", feature = "register-docs"))] +sys::plugin_registry!(pub __GODOT_DOCS_REGISTRY: DocsPlugin); // ---------------------------------------------------------------------------------------------------------------------------------------------- // Call error handling @@ -142,26 +146,15 @@ pub fn next_class_id() -> u16 { NEXT_CLASS_ID.fetch_add(1, atomic::Ordering::Relaxed) } -// Don't touch unless you know what you're doing. -#[doc(hidden)] -pub fn edit_inherent_impl(class_name: crate::meta::ClassName, f: impl FnOnce(&mut InherentImpl)) { - let mut plugins = __GODOT_PLUGIN_REGISTRY.lock().unwrap(); - - for elem in plugins.iter_mut().filter(|p| p.class_name == class_name) { - match &mut elem.item { - PluginItem::InherentImpl(inherent_impl) => { - f(inherent_impl); - return; - } - PluginItem::Struct(_) | PluginItem::ITraitImpl(_) | PluginItem::DynTraitImpl(_) => {} - } - } -} - pub(crate) fn iterate_plugins(mut visitor: impl FnMut(&ClassPlugin)) { sys::plugin_foreach!(__GODOT_PLUGIN_REGISTRY; visitor); } +#[cfg(all(since_api = "4.3", feature = "register-docs"))] +pub(crate) fn iterate_docs_plugins(mut visitor: impl FnMut(&DocsPlugin)) { + sys::plugin_foreach!(__GODOT_DOCS_REGISTRY; visitor); +} + #[cfg(feature = "codegen-full")] // Remove if used in other scenarios. pub(crate) fn find_inherent_impl(class_name: crate::meta::ClassId) -> Option { // We do this manually instead of using `iterate_plugins()` because we want to break as soon as we find a match. diff --git a/godot-core/src/registry/class.rs b/godot-core/src/registry/class.rs index d9335745d..1715313ec 100644 --- a/godot-core/src/registry/class.rs +++ b/godot-core/src/registry/class.rs @@ -115,10 +115,7 @@ impl ClassRegistrationInfo { // Note: when changing this match, make sure the array has sufficient size. let index = match item { PluginItem::Struct { .. } => 0, - PluginItem::InherentImpl(_) => { - // Inherent impls don't need to be unique. - return; - } + PluginItem::InherentImpl(_) => 1, PluginItem::ITraitImpl { .. } => 2, // Multiple dyn traits can be registered, thus don't validate for uniqueness. @@ -427,8 +424,6 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) { is_editor_plugin, is_internal, is_instantiable, - #[cfg(all(since_api = "4.3", feature = "register-docs"))] - docs: _, reference_fn, unreference_fn, }) => { @@ -476,8 +471,6 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) { PluginItem::InherentImpl(InherentImpl { register_methods_constants_fn, register_rpcs_fn: _, - #[cfg(all(since_api = "4.3", feature = "register-docs"))] - docs: _, }) => { c.register_methods_constants_fn = Some(register_methods_constants_fn); } @@ -495,8 +488,6 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) { user_free_property_list_fn, user_property_can_revert_fn, user_property_get_revert_fn, - #[cfg(all(since_api = "4.3", feature = "register-docs"))] - virtual_method_docs: _, validate_property_fn, }) => { c.user_register_fn = user_register_fn; diff --git a/godot-core/src/registry/plugin.rs b/godot-core/src/registry/plugin.rs index 614828f0c..f03da5fae 100644 --- a/godot-core/src/registry/plugin.rs +++ b/godot-core/src/registry/plugin.rs @@ -8,8 +8,6 @@ use std::any::Any; use std::{any, fmt}; -#[cfg(all(since_api = "4.3", feature = "register-docs"))] -use crate::docs::*; use crate::init::InitLevel; use crate::meta::ClassId; use crate::obj::{bounds, cap, Bounds, DynGd, Gd, GodotClass, Inherits, UserClass}; @@ -39,10 +37,10 @@ pub struct ClassPlugin { /// Incorrectly setting this value should not cause any UB but will likely cause errors during registration time. // Init-level is per ClassPlugin and not per PluginItem, because all components of all classes are mixed together in one // huge linker list. There is no per-class aggregation going on, so this allows to easily filter relevant classes. - pub init_level: InitLevel, + pub(crate) init_level: InitLevel, /// The actual item being registered. - pub item: PluginItem, + pub(crate) item: PluginItem, } impl ClassPlugin { @@ -197,16 +195,10 @@ pub struct Struct { /// Whether the class has a default constructor. pub(crate) is_instantiable: bool, - - /// Documentation extracted from the struct's RustDoc. - #[cfg(all(since_api = "4.3", feature = "register-docs"))] - pub(crate) docs: StructDocs, } impl Struct { - pub fn new( - #[cfg(all(since_api = "4.3", feature = "register-docs"))] docs: StructDocs, - ) -> Self { + pub fn new() -> Self { let refcounted = ::IS_REF_COUNTED; Self { @@ -222,8 +214,6 @@ impl Struct { is_editor_plugin: false, is_internal: false, is_instantiable: false, - #[cfg(all(since_api = "4.3", feature = "register-docs"))] - docs, // While Godot doesn't do anything with these callbacks for non-RefCounted classes, we can avoid instantiating them in Rust. reference_fn: refcounted.then_some(callbacks::reference::), unreference_fn: refcounted.then_some(callbacks::unreference::), @@ -294,9 +284,6 @@ pub struct InherentImpl { // This field is only used during codegen-full. #[cfg_attr(not(feature = "codegen-full"), expect(dead_code))] pub(crate) register_rpcs_fn: Option, - - #[cfg(all(since_api = "4.3", feature = "register-docs"))] - pub docs: InherentImplDocs, } impl InherentImpl { @@ -308,18 +295,12 @@ impl InherentImpl { register_rpcs_fn: Some(ErasedRegisterRpcsFn { raw: callbacks::register_user_rpcs::, }), - #[cfg(all(since_api = "4.3", feature = "register-docs"))] - docs: Default::default(), } } } #[derive(Default, Clone, Debug)] pub struct ITraitImpl { - #[cfg(all(since_api = "4.3", feature = "register-docs"))] - /// Virtual method documentation. - pub(crate) virtual_method_docs: &'static str, - /// Callback to user-defined `register_class` function. pub(crate) user_register_fn: Option, @@ -434,12 +415,8 @@ pub struct ITraitImpl { } impl ITraitImpl { - pub fn new( - #[cfg(all(since_api = "4.3", feature = "register-docs"))] virtual_method_docs: &'static str, - ) -> Self { + pub fn new() -> Self { Self { - #[cfg(all(since_api = "4.3", feature = "register-docs"))] - virtual_method_docs, get_virtual_fn: Some(callbacks::get_virtual::), ..Default::default() } diff --git a/godot-macros/src/class/data_models/inherent_impl.rs b/godot-macros/src/class/data_models/inherent_impl.rs index 53c2ade94..2bcd8ac74 100644 --- a/godot-macros/src/class/data_models/inherent_impl.rs +++ b/godot-macros/src/class/data_models/inherent_impl.rs @@ -91,8 +91,8 @@ pub fn transform_inherent_impl( let (funcs, signals) = process_godot_fns(&class_name, &mut impl_block, meta.secondary)?; let consts = process_godot_constants(&mut impl_block)?; - #[cfg(all(feature = "register-docs", since_api = "4.3"))] - let docs = crate::docs::document_inherent_impl(&funcs, &consts, &signals); + let inherent_impl_docs = + crate::docs::make_trait_docs_registration(&funcs, &consts, &signals, &class_name, &prv); // Container struct holding names of all registered #[func]s. // The struct is declared by #[derive(GodotClass)]. @@ -126,26 +126,6 @@ pub fn transform_inherent_impl( let constants_storage_name = format_ident!("__registration_constants_{class_name}"); let fill_storage = { - #[cfg(all(feature = "register-docs", since_api = "4.3"))] - let push_docs = { - let crate::docs::InherentImplXmlDocs { - method_xml_elems, - constant_xml_elems, - signal_xml_elems, - } = docs; - - quote! { - #prv::edit_inherent_impl(#class_name_obj, |inherent_impl| { - inherent_impl.docs.methods.push(#method_xml_elems); - inherent_impl.docs.constants.push(#constant_xml_elems); - inherent_impl.docs.signals.push(#signal_xml_elems); - }); - } - }; - - #[cfg(not(all(feature = "register-docs", since_api = "4.3")))] - let push_docs = TokenStream::new(); - quote! { ::godot::sys::plugin_execute_pre_main!({ #method_storage_name.lock().unwrap().push(|| { @@ -157,7 +137,6 @@ pub fn transform_inherent_impl( #constant_registration }); - #push_docs }); } }; @@ -211,6 +190,7 @@ pub fn transform_inherent_impl( #( #func_name_constants )* } #signal_symbol_types + #inherent_impl_docs }; Ok(result) @@ -224,6 +204,7 @@ pub fn transform_inherent_impl( impl #funcs_collection { #( #func_name_constants )* } + #inherent_impl_docs }; Ok(result) 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 5d78ef3ca..e373cdce7 100644 --- a/godot-macros/src/class/data_models/interface_trait_impl.rs +++ b/godot-macros/src/class/data_models/interface_trait_impl.rs @@ -20,10 +20,11 @@ pub fn transform_trait_impl(mut original_impl: venial::Impl) -> ParseResult ParseResult(#docs); + let mut item = #prv::ITraitImpl::new::<#class_name>(); #(#modifications)* item } @@ -202,6 +203,8 @@ pub fn transform_trait_impl(mut original_impl: venial::Impl) -> ParseResult( #prv::PluginItem::ITraitImpl(#item_constructor) )); + + #register_docs }; // #decls still holds a mutable borrow to `original_impl`, so we mutate && append it afterwards. diff --git a/godot-macros/src/class/derive_godot_class.rs b/godot-macros/src/class/derive_godot_class.rs index ea6057997..09d5c5f96 100644 --- a/godot-macros/src/class/derive_godot_class.rs +++ b/godot-macros/src/class/derive_godot_class.rs @@ -69,18 +69,21 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { modifiers.push(quote! { with_internal }) } let base_ty = &struct_cfg.base_ty; - #[cfg(all(feature = "register-docs", since_api = "4.3"))] - let docs = - crate::docs::document_struct(base_ty.to_string(), &class.attributes, &fields.all_fields); - #[cfg(not(all(feature = "register-docs", since_api = "4.3")))] - let docs = quote! {}; + let prv = quote! { ::godot::private }; + + let struct_docs_registration = crate::docs::make_struct_docs_registration( + base_ty.to_string(), + &class.attributes, + &fields.all_fields, + class_name, + &prv, + ); let base_class = quote! { ::godot::classes::#base_ty }; // Use this name because when typing a non-existent class, users will be met with the following error: // could not find `inherit_from_OS__ensure_class_exists` in `class_macros`. let inherits_macro_ident = format_ident!("inherit_from_{}__ensure_class_exists", base_ty); - let prv = quote! { ::godot::private }; let godot_exports_impl = make_property_impl(class_name, &fields); let godot_withbase_impl = if let Some(Field { name, ty, .. }) = &fields.base_field { @@ -196,9 +199,10 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { #( #deprecations )* #( #errors )* + #struct_docs_registration ::godot::sys::plugin_add!(#prv::__GODOT_PLUGIN_REGISTRY; #prv::ClassPlugin::new::<#class_name>( #prv::PluginItem::Struct( - #prv::Struct::new::<#class_name>(#docs)#(.#modifiers())* + #prv::Struct::new::<#class_name>()#(.#modifiers())* ) )); diff --git a/godot-macros/src/docs.rs b/godot-macros/src/docs/extract_docs.rs similarity index 99% rename from godot-macros/src/docs.rs rename to godot-macros/src/docs/extract_docs.rs index cfdc65b0a..44ed4fd62 100644 --- a/godot-macros/src/docs.rs +++ b/godot-macros/src/docs/extract_docs.rs @@ -4,13 +4,11 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ - -mod markdown_converter; - use proc_macro2::{Ident, TokenStream}; use quote::{quote, ToTokens}; use crate::class::{ConstDefinition, Field, FuncDefinition, SignalDefinition}; +use crate::docs::markdown_converter; #[derive(Default)] struct XmlParagraphs { diff --git a/godot-macros/src/docs/mod.rs b/godot-macros/src/docs/mod.rs new file mode 100644 index 000000000..4bb1b80f5 --- /dev/null +++ b/godot-macros/src/docs/mod.rs @@ -0,0 +1,115 @@ +/* + * Copyright (c) godot-rust; Bromeon and contributors. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ +#[cfg(all(feature = "register-docs", since_api = "4.3"))] +mod extract_docs; +#[cfg(all(feature = "register-docs", since_api = "4.3"))] +mod markdown_converter; + +use proc_macro2::{Ident, TokenStream}; + +use crate::class::{ConstDefinition, Field, FuncDefinition, SignalDefinition}; + +#[cfg(all(feature = "register-docs", since_api = "4.3"))] +mod docs_generators { + use quote::quote; + + use super::*; + + pub fn make_struct_docs_registration( + base: String, + description: &[venial::Attribute], + fields: &[Field], + class_name: &Ident, + prv: &TokenStream, + ) -> TokenStream { + let struct_docs = extract_docs::document_struct(base, description, fields); + quote! { + ::godot::sys::plugin_add!(#prv::__GODOT_DOCS_REGISTRY; #prv::DocsPlugin::new::<#class_name>( + #prv::DocsItem::Struct( + #struct_docs + ) + )); + } + } + + pub fn make_trait_docs_registration( + functions: &[FuncDefinition], + constants: &[ConstDefinition], + signals: &[SignalDefinition], + class_name: &Ident, + prv: &TokenStream, + ) -> TokenStream { + let extract_docs::InherentImplXmlDocs { + method_xml_elems, + constant_xml_elems, + signal_xml_elems, + } = extract_docs::document_inherent_impl(functions, constants, signals); + + quote! { + ::godot::sys::plugin_add!(#prv::__GODOT_DOCS_REGISTRY; #prv::DocsPlugin::new::<#class_name>( + #prv::DocsItem::InherentImpl(#prv::InherentImplDocs { + methods: #method_xml_elems, + signals: #signal_xml_elems, + constants: #constant_xml_elems + }) + )); + } + } + + pub fn make_interface_impl_docs_registration( + impl_members: &[venial::ImplMember], + class_name: &Ident, + prv: &TokenStream, + ) -> TokenStream { + let virtual_methods = extract_docs::document_interface_trait_impl(impl_members); + + quote! { + ::godot::sys::plugin_add!(#prv::__GODOT_DOCS_REGISTRY; #prv::DocsPlugin::new::<#class_name>( + #prv::DocsItem::ITraitImpl(#virtual_methods) + )); + } + } +} + +#[cfg(all(feature = "register-docs", since_api = "4.3"))] +pub use docs_generators::*; + +#[cfg(not(all(feature = "register-docs", since_api = "4.3")))] +mod placeholders { + use super::*; + + pub fn make_struct_docs_registration( + _base: String, + _description: &[venial::Attribute], + _fields: &[Field], + _class_name: &Ident, + _prv: &TokenStream, + ) -> TokenStream { + TokenStream::new() + } + + pub fn make_trait_docs_registration( + _functions: &[FuncDefinition], + _constants: &[ConstDefinition], + _signals: &[SignalDefinition], + _class_name: &Ident, + _prv: &proc_macro2::TokenStream, + ) -> TokenStream { + TokenStream::new() + } + + pub fn make_interface_impl_docs_registration( + _impl_members: &[venial::ImplMember], + _class_name: &Ident, + _prv: &TokenStream, + ) -> TokenStream { + TokenStream::new() + } +} + +#[cfg(not(all(feature = "register-docs", since_api = "4.3")))] +pub use placeholders::*; diff --git a/godot-macros/src/lib.rs b/godot-macros/src/lib.rs index 8ba7d92ae..0f35ff4d5 100644 --- a/godot-macros/src/lib.rs +++ b/godot-macros/src/lib.rs @@ -13,7 +13,6 @@ mod bench; mod class; mod derive; -#[cfg(all(feature = "register-docs", since_api = "4.3"))] mod docs; mod ffi_macros; mod gdextension; diff --git a/itest/rust/src/register_tests/constant_test.rs b/itest/rust/src/register_tests/constant_test.rs index ae8806403..9ab6f0066 100644 --- a/itest/rust/src/register_tests/constant_test.rs +++ b/itest/rust/src/register_tests/constant_test.rs @@ -172,8 +172,6 @@ godot::sys::plugin_add!( godot::private::ClassPlugin::new::( godot::private::PluginItem::InherentImpl( godot::private::InherentImpl::new::( - #[cfg(feature = "register-docs")] - Default::default() ) ) ) diff --git a/itest/rust/src/register_tests/register_docs_test.rs b/itest/rust/src/register_tests/register_docs_test.rs index a7951c656..d0c23037e 100644 --- a/itest/rust/src/register_tests/register_docs_test.rs +++ b/itest/rust/src/register_tests/register_docs_test.rs @@ -305,6 +305,20 @@ impl FairlyDocumented { fn other_signal(x: i64); } +#[godot_api(secondary)] +impl FairlyDocumented { + /// Documented method in godot_api secondary block + #[func] + fn secondary_but_documented(&self, _smth: i64) {} +} + +#[godot_api(secondary)] +impl FairlyDocumented { + /// Documented method in other godot_api secondary block + #[func] + fn trinary_but_documented(&self, _smth: i64) {} +} + #[itest] fn test_register_docs() { let xml = find_class_docs("FairlyDocumented"); diff --git a/itest/rust/src/register_tests/res/registered_docs.xml b/itest/rust/src/register_tests/res/registered_docs.xml index a0551a2ba..bf378abdb 100644 --- a/itest/rust/src/register_tests/res/registered_docs.xml +++ b/itest/rust/src/register_tests/res/registered_docs.xml @@ -17,6 +17,14 @@ public class Player : Node2D code&nbsp;block </div>[/codeblock][br][br][url=https://www.google.com/search?q=2+%2B+2+<+5]Google: 2 + 2 < 5[/url][br][br]connect these[br][br]¹ [url=https://example.org]https://example.org[/url][br]² because the glorb doesn't flibble.[br]³ This is the third footnote in order of definition.[br]⁴ Fourth footnote in order of definition.[br]⁵ This is the fifth footnote in order of definition.[br]⁶ sixth footnote in order of definition. + + + + + initialize this + + + @@ -65,11 +73,19 @@ public class Player : Node2D - - - + + + - initialize this + Documented method in godot_api secondary block + + + + + + + + Documented method in other godot_api secondary block From f08c48395b139456557adeead332170be002f016 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 20 Jul 2025 21:15:18 +0200 Subject: [PATCH 08/12] 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 09/12] 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 10/12] 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 11/12] 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 From ee2b1fa01c9fea1b9dec88d8b1166003aa2875ce Mon Sep 17 00:00:00 2001 From: Yarvin Date: Tue, 21 Oct 2025 11:36:41 +0200 Subject: [PATCH 12/12] Add rustfmt to clippy job in CI to format files generated by itest. --- .github/workflows/full-ci.yml | 3 ++- .github/workflows/minimal-ci.yml | 3 ++- itest/rust/build.rs | 1 - 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml index a11adf0a6..2704a98f6 100644 --- a/.github/workflows/full-ci.yml +++ b/.github/workflows/full-ci.yml @@ -101,7 +101,8 @@ jobs: - name: "Install Rust" uses: ./.github/composite/rust with: - components: clippy + # Rustfmt is needed to format generated code in itest. + components: clippy,rustfmt # Note: could use `-- --no-deps` to not lint dependencies, however it doesn't really speed up and also skips deps in workspace. - name: "Check clippy" diff --git a/.github/workflows/minimal-ci.yml b/.github/workflows/minimal-ci.yml index 691f7669f..4f1553445 100644 --- a/.github/workflows/minimal-ci.yml +++ b/.github/workflows/minimal-ci.yml @@ -94,7 +94,8 @@ jobs: - name: "Install Rust" uses: ./.github/composite/rust with: - components: clippy + # Rustfmt is needed to format generated code in itest. + components: clippy,rustfmt - name: "Check clippy" run: | diff --git a/itest/rust/build.rs b/itest/rust/build.rs index fd16833bd..153cbd8cd 100644 --- a/itest/rust/build.rs +++ b/itest/rust/build.rs @@ -361,7 +361,6 @@ fn generate_rust_methods(inputs: &[Input]) -> Vec { .collect::>(); let manual_methods = quote! { - #[allow(clippy::suspicious_else_formatting)] // `quote!` might output whole file as one big line. #[func] fn check_last_notrace(last_method_name: String, expected_callconv: String) -> bool { let last = godot::private::trace::pop();