From 9ebeeb41d36c57622225d2b3d075caf0ee4c4fe1 Mon Sep 17 00:00:00 2001 From: Lukas Rieger Date: Mon, 16 Jun 2025 02:15:07 +0200 Subject: [PATCH 1/4] try to add doc comments to functions with a link to the godot documentation --- godot-codegen/src/generator/builtins.rs | 4 +- godot-codegen/src/generator/classes.rs | 3 ++ godot-codegen/src/generator/docs.rs | 44 +++++++++++++++++++ .../src/generator/functions_common.rs | 12 +++++ .../src/generator/utility_functions.rs | 1 + godot-codegen/src/generator/virtual_traits.rs | 1 + 6 files changed, 64 insertions(+), 1 deletion(-) diff --git a/godot-codegen/src/generator/builtins.rs b/godot-codegen/src/generator/builtins.rs index bb609d992..d0fa04654 100644 --- a/godot-codegen/src/generator/builtins.rs +++ b/godot-codegen/src/generator/builtins.rs @@ -8,7 +8,7 @@ use crate::context::Context; use crate::generator::functions_common::{FnCode, FnDefinition, FnDefinitions}; use crate::generator::method_tables::MethodTableKey; -use crate::generator::{enums, functions_common}; +use crate::generator::{docs, enums, functions_common}; use crate::models::domain::{ BuiltinClass, BuiltinMethod, ClassLike, ExtensionApi, FnDirection, Function, ModName, RustTy, TyName, @@ -276,6 +276,7 @@ fn make_builtin_method_definition( ) }; + let doc = docs::make_method_doc(builtin_class.name(), method.name()); let safety_doc = method_safety_doc(builtin_class.name(), method); functions_common::make_function_definition( @@ -287,6 +288,7 @@ fn make_builtin_method_definition( is_virtual_required: false, is_varcall_fallible: false, }, + Some(quote! {#[doc = #doc] }), safety_doc, &TokenStream::new(), ) diff --git a/godot-codegen/src/generator/classes.rs b/godot-codegen/src/generator/classes.rs index e31769b75..1629211c8 100644 --- a/godot-codegen/src/generator/classes.rs +++ b/godot-codegen/src/generator/classes.rs @@ -574,6 +574,8 @@ fn make_class_method_definition( let rust_method_name = method.name(); let godot_method_name = method.godot_name(); + let doc = docs::make_method_doc(&class.name(), &method.name()); + let receiver = functions_common::make_receiver(method.qualifier(), quote! { self.object_ptr }); let table_index = ctx.get_table_index(&MethodTableKey::from_class(class, method)); @@ -634,6 +636,7 @@ fn make_class_method_definition( is_virtual_required: false, is_varcall_fallible: true, }, + Some(quote! {#[doc = #doc] }), None, cfg_attributes, ) diff --git a/godot-codegen/src/generator/docs.rs b/godot-codegen/src/generator/docs.rs index b9a954dc6..3a3e56a99 100644 --- a/godot-codegen/src/generator/docs.rs +++ b/godot-codegen/src/generator/docs.rs @@ -158,3 +158,47 @@ pub fn make_module_doc(class_name: &TyName) -> String { See also [Godot docs for `{godot_ty}` enums]({online_link}).\n\n" ) } + +pub fn make_method_doc(class_name: &TyName, method_name: &str) -> String { + let TyName { rust_ty, godot_ty } = class_name; + + let class_name_lower = godot_ty.to_ascii_lowercase(); + let method_name_slug = method_name.to_ascii_lowercase().replace('_', "-"); + + // note: godot properties are in the method table of the json file + let is_property = method_name.starts_with("get_") || method_name.starts_with("set_"); + let is_actual_method = !is_property; + + if is_actual_method { + let online_link = format!( + "https://docs.godotengine.org/en/stable/classes/class_{class_name_lower}.html#class-{class_name_lower}-method-{method_name_slug}" + ); + + format!( + "Method `{method_name}` of class [`{rust_ty}`][crate::classes::{rust_ty}].\ + \n\n + See also [Godot docs for `{godot_ty}.{method_name}`]({online_link}).\n\n" + ) + } else + /* it's a property */ + { + let prop_name = method_name[4..].to_string(); + let prop_name_slug = prop_name.to_ascii_lowercase().replace('_', "-"); + + let getter_setter = if method_name.starts_with("get_") { + "Getter" + } else { + "Setter" + }; + + let online_link = format!( + "https://docs.godotengine.org/en/stable/classes/class_{class_name_lower}.html#class-{class_name_lower}-property-{prop_name_slug}" + ); + + format!( + "{getter_setter} function for Property `{prop_name}` of class [`{rust_ty}`][crate::classes::{rust_ty}].\ + \n\n + See also [Godot docs for `{godot_ty}.{prop_name}`]({online_link}).\n\n" + ) + } +} diff --git a/godot-codegen/src/generator/functions_common.rs b/godot-codegen/src/generator/functions_common.rs index 961fa15b8..d046a3ece 100644 --- a/godot-codegen/src/generator/functions_common.rs +++ b/godot-codegen/src/generator/functions_common.rs @@ -103,6 +103,7 @@ pub struct FnParamTokens { pub fn make_function_definition( sig: &dyn Function, code: &FnCode, + doc: Option, safety_doc: Option, cfg_attributes: &TokenStream, ) -> FnDefinition { @@ -115,6 +116,12 @@ pub fn make_function_definition( make_vis(sig.is_private()) }; + let maybe_doc = if let Some(doc) = doc { + doc + } else { + TokenStream::new() + }; + // Functions are marked unsafe as soon as raw pointers are involved, irrespectively of whether they appear in parameter or return type // position. In cases of virtual functions called by Godot, a returned pointer must be valid and of the expected type. It might be possible // to only use `unsafe` for pointers in parameters (for outbound calls), and in return values (for virtual calls). Or technically more @@ -196,6 +203,7 @@ pub fn make_function_definition( quote! { #maybe_safety_doc + #maybe_doc #maybe_unsafe fn #primary_fn_name ( #receiver_param #( #params, )* @@ -211,6 +219,7 @@ pub fn make_function_definition( if !code.is_varcall_fallible { quote! { #maybe_safety_doc + #maybe_doc #vis #maybe_unsafe fn #primary_fn_name ( #receiver_param #( #params, )* @@ -243,6 +252,7 @@ pub fn make_function_definition( /// This is a _varcall_ method, meaning parameters and return values are passed as `Variant`. /// It can detect call failures and will panic in such a case. #maybe_safety_doc + #maybe_doc #vis #maybe_unsafe fn #primary_fn_name ( #receiver_param #( #params, )* @@ -256,6 +266,7 @@ pub fn make_function_definition( /// This is a _varcall_ method, meaning parameters and return values are passed as `Variant`. /// It can detect call failures and will return `Err` in such a case. #maybe_safety_doc + #maybe_doc #vis #maybe_unsafe fn #try_fn_name( #receiver_param #( #params, )* @@ -278,6 +289,7 @@ pub fn make_function_definition( quote! { #maybe_safety_doc + #maybe_doc #vis #maybe_unsafe fn #primary_fn_name #fn_lifetime ( #receiver_param #( #params, )* diff --git a/godot-codegen/src/generator/utility_functions.rs b/godot-codegen/src/generator/utility_functions.rs index c6587ff6f..44acd06e8 100644 --- a/godot-codegen/src/generator/utility_functions.rs +++ b/godot-codegen/src/generator/utility_functions.rs @@ -74,6 +74,7 @@ pub(crate) fn make_utility_function_definition(function: &UtilityFunction) -> To is_varcall_fallible: false, }, None, + None, &TokenStream::new(), ); diff --git a/godot-codegen/src/generator/virtual_traits.rs b/godot-codegen/src/generator/virtual_traits.rs index ce87feec7..87ed65e3a 100644 --- a/godot-codegen/src/generator/virtual_traits.rs +++ b/godot-codegen/src/generator/virtual_traits.rs @@ -203,6 +203,7 @@ fn make_virtual_method( is_varcall_fallible: true, }, None, + None, &TokenStream::new(), ); From 07109b24bb12464f7987bc60e7ab07df510247b8 Mon Sep 17 00:00:00 2001 From: Lukas Rieger Date: Mon, 16 Jun 2025 02:33:31 +0200 Subject: [PATCH 2/4] try to unbreak formatting --- godot-codegen/src/generator/docs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/godot-codegen/src/generator/docs.rs b/godot-codegen/src/generator/docs.rs index 3a3e56a99..d037a6cb3 100644 --- a/godot-codegen/src/generator/docs.rs +++ b/godot-codegen/src/generator/docs.rs @@ -176,7 +176,7 @@ pub fn make_method_doc(class_name: &TyName, method_name: &str) -> String { format!( "Method `{method_name}` of class [`{rust_ty}`][crate::classes::{rust_ty}].\ - \n\n + \n\n\ See also [Godot docs for `{godot_ty}.{method_name}`]({online_link}).\n\n" ) } else @@ -197,7 +197,7 @@ pub fn make_method_doc(class_name: &TyName, method_name: &str) -> String { format!( "{getter_setter} function for Property `{prop_name}` of class [`{rust_ty}`][crate::classes::{rust_ty}].\ - \n\n + \n\n\ See also [Godot docs for `{godot_ty}.{prop_name}`]({online_link}).\n\n" ) } From 63664304c04be0bfe50f43ee832bfa4cf681fe60 Mon Sep 17 00:00:00 2001 From: Lukas Rieger Date: Mon, 16 Jun 2025 03:43:25 +0200 Subject: [PATCH 3/4] actually parse 'properties' field from json --- godot-codegen/src/generator/builtins.rs | 5 +- godot-codegen/src/generator/classes.rs | 2 +- godot-codegen/src/generator/docs.rs | 57 +++++++++++++--------- godot-codegen/src/models/domain.rs | 9 ++++ godot-codegen/src/models/domain_mapping.rs | 26 ++++++++-- godot-codegen/src/models/json.rs | 14 +++--- 6 files changed, 73 insertions(+), 40 deletions(-) diff --git a/godot-codegen/src/generator/builtins.rs b/godot-codegen/src/generator/builtins.rs index d0fa04654..b4231640d 100644 --- a/godot-codegen/src/generator/builtins.rs +++ b/godot-codegen/src/generator/builtins.rs @@ -8,7 +8,7 @@ use crate::context::Context; use crate::generator::functions_common::{FnCode, FnDefinition, FnDefinitions}; use crate::generator::method_tables::MethodTableKey; -use crate::generator::{docs, enums, functions_common}; +use crate::generator::{enums, functions_common}; use crate::models::domain::{ BuiltinClass, BuiltinMethod, ClassLike, ExtensionApi, FnDirection, Function, ModName, RustTy, TyName, @@ -276,7 +276,6 @@ fn make_builtin_method_definition( ) }; - let doc = docs::make_method_doc(builtin_class.name(), method.name()); let safety_doc = method_safety_doc(builtin_class.name(), method); functions_common::make_function_definition( @@ -288,7 +287,7 @@ fn make_builtin_method_definition( is_virtual_required: false, is_varcall_fallible: false, }, - Some(quote! {#[doc = #doc] }), + None, safety_doc, &TokenStream::new(), ) diff --git a/godot-codegen/src/generator/classes.rs b/godot-codegen/src/generator/classes.rs index 1629211c8..c0b90d689 100644 --- a/godot-codegen/src/generator/classes.rs +++ b/godot-codegen/src/generator/classes.rs @@ -574,7 +574,7 @@ fn make_class_method_definition( let rust_method_name = method.name(); let godot_method_name = method.godot_name(); - let doc = docs::make_method_doc(&class.name(), &method.name()); + let doc = docs::make_method_doc(&class, &method.name()); let receiver = functions_common::make_receiver(method.qualifier(), quote! { self.object_ptr }); diff --git a/godot-codegen/src/generator/docs.rs b/godot-codegen/src/generator/docs.rs index d037a6cb3..b65640bf9 100644 --- a/godot-codegen/src/generator/docs.rs +++ b/godot-codegen/src/generator/docs.rs @@ -10,7 +10,7 @@ //! Single module for documentation, rather than having it in each symbol-specific file, so it's easier to keep docs consistent. use crate::generator::signals; -use crate::models::domain::{ModName, TyName}; +use crate::models::domain::{Class, ClassLike as _, ModName, TyName}; use crate::special_cases; use proc_macro2::Ident; @@ -159,46 +159,55 @@ pub fn make_module_doc(class_name: &TyName) -> String { ) } -pub fn make_method_doc(class_name: &TyName, method_name: &str) -> String { +pub fn make_method_doc(class: &Class, method_name: &str) -> String { + let class_name = class.name(); let TyName { rust_ty, godot_ty } = class_name; let class_name_lower = godot_ty.to_ascii_lowercase(); let method_name_slug = method_name.to_ascii_lowercase().replace('_', "-"); - // note: godot properties are in the method table of the json file - let is_property = method_name.starts_with("get_") || method_name.starts_with("set_"); - let is_actual_method = !is_property; + let getter_for = class + .properties + .iter() + .find(|prop| prop.getter == Some(method_name.to_string())); + let setter_for = class + .properties + .iter() + .find(|prop| prop.setter == Some(method_name.to_string())); - if is_actual_method { - let online_link = format!( + let property_for = getter_for.or(setter_for); + + match property_for { + None => { + let online_link = format!( "https://docs.godotengine.org/en/stable/classes/class_{class_name_lower}.html#class-{class_name_lower}-method-{method_name_slug}" ); - format!( - "Method `{method_name}` of class [`{rust_ty}`][crate::classes::{rust_ty}].\ + format!( + "Method `{method_name}` of class [`{rust_ty}`][crate::classes::{rust_ty}].\ \n\n\ See also [Godot docs for `{godot_ty}.{method_name}`]({online_link}).\n\n" - ) - } else - /* it's a property */ - { - let prop_name = method_name[4..].to_string(); - let prop_name_slug = prop_name.to_ascii_lowercase().replace('_', "-"); - - let getter_setter = if method_name.starts_with("get_") { - "Getter" - } else { - "Setter" - }; - - let online_link = format!( + ) + } + Some(prop) => { + let prop_name = &prop.name; + let prop_name_slug = prop_name.to_ascii_lowercase().replace('_', "-"); + + let getter_setter = if getter_for.is_some() { + "Getter" + } else { + "Setter" + }; + + let online_link = format!( "https://docs.godotengine.org/en/stable/classes/class_{class_name_lower}.html#class-{class_name_lower}-property-{prop_name_slug}" ); - format!( + format!( "{getter_setter} function for Property `{prop_name}` of class [`{rust_ty}`][crate::classes::{rust_ty}].\ \n\n\ See also [Godot docs for `{godot_ty}.{prop_name}`]({online_link}).\n\n" ) + } } } diff --git a/godot-codegen/src/models/domain.rs b/godot-codegen/src/models/domain.rs index 8304f4ad7..3bcff5e49 100644 --- a/godot-codegen/src/models/domain.rs +++ b/godot-codegen/src/models/domain.rs @@ -168,6 +168,7 @@ pub struct Class { pub enums: Vec, pub methods: Vec, pub signals: Vec, + pub properties: Vec, } impl ClassLike for Class { @@ -453,6 +454,14 @@ pub struct ClassSignal { // ---------------------------------------------------------------------------------------------------------------------------------------------- +pub struct ClassProperty { + pub name: String, + pub getter: Option, + pub setter: Option, +} + +// ---------------------------------------------------------------------------------------------------------------------------------------------- + #[derive(Copy, Clone, Debug)] pub enum FnDirection { /// Godot -> Rust. diff --git a/godot-codegen/src/models/domain_mapping.rs b/godot-codegen/src/models/domain_mapping.rs index cbb692c03..b8d5b1b27 100644 --- a/godot-codegen/src/models/domain_mapping.rs +++ b/godot-codegen/src/models/domain_mapping.rs @@ -8,21 +8,22 @@ use crate::context::Context; use crate::models::domain::{ BuildConfiguration, BuiltinClass, BuiltinMethod, BuiltinSize, BuiltinVariant, Class, - ClassCommons, ClassConstant, ClassConstantValue, ClassMethod, ClassSignal, Constructor, Enum, - Enumerator, EnumeratorValue, ExtensionApi, FnDirection, FnParam, FnQualifier, FnReturn, - FunctionCommon, GodotApiVersion, ModName, NativeStructure, Operator, RustTy, Singleton, TyName, - UtilityFunction, + ClassCommons, ClassConstant, ClassConstantValue, ClassMethod, ClassProperty, ClassSignal, + Constructor, Enum, Enumerator, EnumeratorValue, ExtensionApi, FnDirection, FnParam, + FnQualifier, FnReturn, FunctionCommon, GodotApiVersion, ModName, NativeStructure, Operator, + RustTy, Singleton, TyName, UtilityFunction, }; use crate::models::json::{ JsonBuiltinClass, JsonBuiltinMethod, JsonBuiltinSizes, JsonClass, JsonClassConstant, JsonClassMethod, JsonConstructor, JsonEnum, JsonEnumConstant, JsonExtensionApi, JsonHeader, - JsonMethodReturn, JsonNativeStructure, JsonOperator, JsonSignal, JsonSingleton, + JsonMethodReturn, JsonNativeStructure, JsonOperator, JsonProperty, JsonSignal, JsonSingleton, JsonUtilityFunction, }; use crate::util::{get_api_level, ident, option_as_slice}; use crate::{conv, special_cases}; use proc_macro2::Ident; use std::collections::HashMap; +use std::option; // ---------------------------------------------------------------------------------------------------------------------------------------------- // Top-level @@ -128,6 +129,11 @@ impl Class { }) .collect(); + let properties = option_as_slice(&json.properties) + .iter() + .map(|p| ClassProperty::from_json(p, ctx)) + .collect::>(); + let base_class = json .inherits .as_ref() @@ -148,6 +154,7 @@ impl Class { enums, methods, signals, + properties, }) } } @@ -589,6 +596,15 @@ impl ClassSignal { }) } } +impl ClassProperty { + pub fn from_json(json_property: &JsonProperty, _ctx: &mut Context) -> Self { + Self { + name: json_property.name.clone(), + getter: json_property.getter.clone(), + setter: json_property.setter.clone(), + } + } +} impl UtilityFunction { pub fn from_json(function: &JsonUtilityFunction, ctx: &mut Context) -> Option { diff --git a/godot-codegen/src/models/json.rs b/godot-codegen/src/models/json.rs index 335f29146..50722ed4f 100644 --- a/godot-codegen/src/models/json.rs +++ b/godot-codegen/src/models/json.rs @@ -79,7 +79,7 @@ pub struct JsonClass { pub constants: Option>, pub enums: Option>, pub methods: Option>, - // pub properties: Option>, + pub properties: Option>, pub signals: Option>, } @@ -173,12 +173,12 @@ pub struct JsonMember { #[derive(DeJson)] #[allow(dead_code)] pub struct JsonProperty { - #[nserde(rename = "type")] - type_: String, - name: String, - setter: String, - getter: String, - index: i32, // can be -1 + // #[nserde(rename = "type")] + // type_: String, + pub name: String, + pub setter: Option, + pub getter: Option, + // index: Option, } #[derive(DeJson)] From f008687b7c5807b904fef1340461bc5013d17575 Mon Sep 17 00:00:00 2001 From: Lukas Rieger Date: Mon, 16 Jun 2025 04:15:10 +0200 Subject: [PATCH 4/4] clippy --- godot-codegen/src/generator/classes.rs | 2 +- godot-codegen/src/models/domain_mapping.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/godot-codegen/src/generator/classes.rs b/godot-codegen/src/generator/classes.rs index c0b90d689..341eebcc5 100644 --- a/godot-codegen/src/generator/classes.rs +++ b/godot-codegen/src/generator/classes.rs @@ -574,7 +574,7 @@ fn make_class_method_definition( let rust_method_name = method.name(); let godot_method_name = method.godot_name(); - let doc = docs::make_method_doc(&class, &method.name()); + let doc = docs::make_method_doc(class, method.name()); let receiver = functions_common::make_receiver(method.qualifier(), quote! { self.object_ptr }); diff --git a/godot-codegen/src/models/domain_mapping.rs b/godot-codegen/src/models/domain_mapping.rs index b8d5b1b27..599504bf5 100644 --- a/godot-codegen/src/models/domain_mapping.rs +++ b/godot-codegen/src/models/domain_mapping.rs @@ -23,7 +23,6 @@ use crate::util::{get_api_level, ident, option_as_slice}; use crate::{conv, special_cases}; use proc_macro2::Ident; use std::collections::HashMap; -use std::option; // ---------------------------------------------------------------------------------------------------------------------------------------------- // Top-level