From 0edf6e360656b2f5374741dce7e931aa2c75cb76 Mon Sep 17 00:00:00 2001 From: Lukas Rieger Date: Mon, 20 Jan 2025 03:22:45 +0100 Subject: [PATCH 1/2] #[var(get=f, set=f)]: properly handle #[func]s that have been renamed with (rename=godot_name) --- .../src/class/data_models/field_var.rs | 6 + .../src/class/data_models/inherent_impl.rs | 25 +++- .../src/class/data_models/property.rs | 40 +++++-- godot-macros/src/class/derive_godot_class.rs | 13 ++- godot-macros/src/util/mod.rs | 107 +++++++++++++++++- itest/godot/ManualFfiTests.gd | 60 ++++++++++ itest/rust/src/object_tests/object_test.rs | 21 ++++ itest/rust/src/object_tests/property_test.rs | 55 +++++++++ itest/rust/src/register_tests/func_test.rs | 14 +++ 9 files changed, 329 insertions(+), 12 deletions(-) diff --git a/godot-macros/src/class/data_models/field_var.rs b/godot-macros/src/class/data_models/field_var.rs index 1e95984e7..c0a543e0c 100644 --- a/godot-macros/src/class/data_models/field_var.rs +++ b/godot-macros/src/class/data_models/field_var.rs @@ -12,6 +12,7 @@ use crate::class::{ into_signature_info, make_existence_check, make_method_registration, Field, FieldHint, FuncDefinition, }; +use crate::util::make_func_name_constant; use crate::util::KvParser; use crate::{util, ParseResult}; @@ -166,6 +167,7 @@ pub struct GetterSetterImpl { pub function_name: Ident, pub function_impl: TokenStream, pub export_token: TokenStream, + pub func_name_const: TokenStream, } impl GetterSetterImpl { @@ -206,6 +208,8 @@ impl GetterSetterImpl { } }; + let func_name_const = make_func_name_constant(class_name, &function_name, None, &[]); + let signature = util::parse_signature(signature); let export_token = make_method_registration( class_name, @@ -230,6 +234,7 @@ impl GetterSetterImpl { function_name, function_impl, export_token, + func_name_const, } } @@ -238,6 +243,7 @@ impl GetterSetterImpl { function_name: function_name.clone(), function_impl: TokenStream::new(), export_token: make_existence_check(function_name), + func_name_const: TokenStream::new(), } } } diff --git a/godot-macros/src/class/data_models/inherent_impl.rs b/godot-macros/src/class/data_models/inherent_impl.rs index 42701e788..bab0bdf3a 100644 --- a/godot-macros/src/class/data_models/inherent_impl.rs +++ b/godot-macros/src/class/data_models/inherent_impl.rs @@ -10,12 +10,16 @@ use crate::class::{ make_signal_registrations, ConstDefinition, FuncDefinition, RpcAttr, RpcMode, SignalDefinition, SignatureInfo, TransferMode, }; -use crate::util::{bail, c_str, ident, require_api_version, KvParser}; +use crate::util::{ + bail, c_str, error_fn, format_func_name_struct_name, ident, make_func_name_constants, + replace_class_in_path, require_api_version, KvParser, +}; use crate::{handle_mutually_exclusive_keys, util, ParseResult}; use proc_macro2::{Delimiter, Group, Ident, TokenStream}; use quote::spanned::Spanned; use quote::{format_ident, quote}; +use venial::Path; /// Attribute for user-declared function. enum ItemAttrType { @@ -89,6 +93,19 @@ pub fn transform_inherent_impl( #[cfg(not(all(feature = "register-docs", since_api = "4.3")))] let docs = quote! {}; + // This is the container struct that holds the names of all registered #[func]s. + // (The struct is declared by the macro derive_godot_class.) + let class_functions_name = format_func_name_struct_name(&class_name); + // As the impl block could be of the form "path::class", and we add a second impl block below, we need the full path, not just the class name. + let this_class_full_path = impl_block.self_ty.as_path().ok_or(error_fn( + "unexpected: the function already checked 'as_path' above in validate_impl", + &impl_block, + ))?; + let class_functions_path: Path = + replace_class_in_path(this_class_full_path, class_functions_name); + // For each #[func] in this impl block, we create one constant. + let func_name_constants = make_func_name_constants(&funcs, &class_name); + let signal_registrations = make_signal_registrations(signals, &class_name_obj); #[cfg(feature = "codegen-full")] @@ -164,6 +181,9 @@ pub fn transform_inherent_impl( #trait_impl #fill_storage #class_registration + impl #class_functions_path { + #( #func_name_constants )* + } }; Ok(result) @@ -174,6 +194,9 @@ pub fn transform_inherent_impl( let result = quote! { #impl_block #fill_storage + impl #class_functions_path { + #( #func_name_constants )* + } }; Ok(result) diff --git a/godot-macros/src/class/data_models/property.rs b/godot-macros/src/class/data_models/property.rs index be4077964..0a04aac0f 100644 --- a/godot-macros/src/class/data_models/property.rs +++ b/godot-macros/src/class/data_models/property.rs @@ -8,6 +8,7 @@ //! Parsing the `var` and `export` attributes on fields. use crate::class::{Field, FieldVar, Fields, GetSet, GetterSetterImpl, UsageFlags}; +use crate::util::{format_func_name_constant_name, format_func_name_struct_name, ident}; use proc_macro2::{Ident, TokenStream}; use quote::quote; @@ -38,6 +39,7 @@ impl FieldHint { pub fn make_property_impl(class_name: &Ident, fields: &Fields) -> TokenStream { let mut getter_setter_impls = Vec::new(); + let mut func_name_consts = Vec::new(); let mut export_tokens = Vec::new(); for field in &fields.all_fields { @@ -134,33 +136,47 @@ pub fn make_property_impl(class_name: &Ident, fields: &Fields) -> TokenStream { }, }; - let getter_name = make_getter_setter( + // Note: (getter/setter)_tokens can be either a path ``Class_Functions::constant_name`` or an empty string ``""``. + + let getter_tokens = make_getter_setter( getter.to_impl(class_name, GetSet::Get, field), &mut getter_setter_impls, + &mut func_name_consts, &mut export_tokens, + class_name, ); - let setter_name = make_getter_setter( + let setter_tokens = make_getter_setter( setter.to_impl(class_name, GetSet::Set, field), &mut getter_setter_impls, + &mut func_name_consts, &mut export_tokens, + class_name, ); export_tokens.push(quote! { ::godot::register::private::#registration_fn::<#class_name, #field_type>( #field_name, - #getter_name, - #setter_name, + #getter_tokens, + #setter_tokens, #hint, #usage_flags, ); }); } + // For each generated #[func], add a const. + // This is the name of the container struct, which is declared by the derive macro GodotClass. + let class_functions_name = format_func_name_struct_name(class_name); + quote! { impl #class_name { #(#getter_setter_impls)* } + impl #class_functions_name { + #(#func_name_consts)* + } + impl ::godot::obj::cap::ImplementsGodotExports for #class_name { fn __register_exports() { #( @@ -176,20 +192,30 @@ pub fn make_property_impl(class_name: &Ident, fields: &Fields) -> TokenStream { fn make_getter_setter( getter_setter_impl: Option, getter_setter_impls: &mut Vec, + func_name_consts: &mut Vec, export_tokens: &mut Vec, -) -> String { + class_name: &Ident, +) -> TokenStream { if let Some(getter_impl) = getter_setter_impl { let GetterSetterImpl { function_name, function_impl, export_token, + func_name_const, } = getter_impl; getter_setter_impls.push(function_impl); + func_name_consts.push(func_name_const); export_tokens.push(export_token); - function_name.to_string() + let getter_setter_name = function_name.to_string(); + + let class_functions_name = format_func_name_struct_name(class_name); + + let getter_setter_fn_const = + format_func_name_constant_name(class_name, &ident(&getter_setter_name)); + quote! { #class_functions_name::#getter_setter_fn_const } } else { - String::new() + quote! { "" } } } diff --git a/godot-macros/src/class/derive_godot_class.rs b/godot-macros/src/class/derive_godot_class.rs index 15f7ec1d2..58e8338bd 100644 --- a/godot-macros/src/class/derive_godot_class.rs +++ b/godot-macros/src/class/derive_godot_class.rs @@ -12,7 +12,10 @@ use crate::class::{ make_property_impl, make_virtual_callback, BeforeKind, Field, FieldDefault, FieldExport, FieldVar, Fields, SignatureInfo, }; -use crate::util::{bail, error, ident, path_ends_with_complex, require_api_version, KvParser}; +use crate::util::{ + bail, error, format_func_name_struct_name, ident, path_ends_with_complex, require_api_version, + KvParser, +}; use crate::{handle_mutually_exclusive_keys, util, ParseResult}; pub fn derive_godot_class(item: venial::Item) -> ParseResult { @@ -134,6 +137,13 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { modifiers.push(quote! { with_tool }) } + // Declares a dummy struct that, for each #[func], holds a constant that maps the rust name to the name under which it is registered in godot. + let class_functions_name = format_func_name_struct_name(class_name); + let class_functions_struct = quote! { + #[doc(hidden)] + pub struct #class_functions_name { } + }; + Ok(quote! { impl ::godot::obj::GodotClass for #class_name { type Base = #base_class; @@ -157,6 +167,7 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { type Exportable = <::Base as ::godot::obj::Bounds>::Exportable; } + #class_functions_struct #godot_init_impl #godot_withbase_impl #godot_exports_impl diff --git a/godot-macros/src/util/mod.rs b/godot-macros/src/util/mod.rs index f88525733..66578db00 100644 --- a/godot-macros/src/util/mod.rs +++ b/godot-macros/src/util/mod.rs @@ -7,10 +7,12 @@ // Note: some code duplication with godot-codegen crate. +use crate::class::FuncDefinition; use crate::ParseResult; -use proc_macro2::{Delimiter, Group, Ident, Literal, TokenStream, TokenTree}; +use proc_macro2::{Delimiter, Group, Ident, Literal, Punct, Spacing, TokenStream, TokenTree}; use quote::spanned::Spanned; use quote::{format_ident, quote, ToTokens, TokenStreamExt}; +use venial::{Attribute, Path, PathSegment}; mod kv_parser; mod list_parser; @@ -243,8 +245,18 @@ pub(crate) fn extract_cfg_attrs( attrs: &[venial::Attribute], ) -> impl IntoIterator { attrs.iter().filter(|attr| { - attr.get_single_path_segment() - .is_some_and(|name| name == "cfg") + let Some(attr_name) = attr.get_single_path_segment() else { + return false; + }; + // #[cfg(condition)] + if attr_name == "cfg" { + return true; + } + // #[cfg_attr(condition, attributes...)], note that there can be multiple attributes seperated by comma. + if attr_name == "cfg_attr" && attr.value.to_token_stream().to_string().contains("cfg(") { + return true; + } + false }) } @@ -303,3 +315,92 @@ pub fn venial_parse_meta( venial::parse_item(input) } + +// ---------------------------------------------------------------------------------------------------------------------------------------------- + +// util functions for handling #[func]s and #[var(get=f, set=f)] + +pub fn make_func_name_constants(funcs: &[FuncDefinition], class_name: &Ident) -> Vec { + funcs + .iter() + .map(|func| { + // The constant needs the same #[cfg] attribute(s) as the function, so that it is only active if the function is also active. + let cfg_attributes = extract_cfg_attrs(&func.external_attributes) + .into_iter() + .collect::>(); + + make_func_name_constant( + class_name, + &func.signature_info.method_name, + func.registered_name.as_ref(), + &cfg_attributes, + ) + }) + .collect() +} + +/// Funcs can be renamed with `#[func(rename=new_name) fn f();`. +/// To be able to access the renamed function name at a later point, it is saved in a string constant. +pub fn make_func_name_constant( + class_name: &Ident, + func_name: &Ident, + registered_name: Option<&String>, + attributes: &[&Attribute], +) -> TokenStream { + let const_name = format_func_name_constant_name(class_name, func_name); + let const_value = match ®istered_name { + Some(renamed) => renamed.to_string(), + None => func_name.to_string(), + }; + + let doc_comment = + format!("The Rust function `{func_name}` is registered with Godot as `{const_value}`."); + + quote! { + #(#attributes)* + #[doc = #doc_comment] + #[doc(hidden)] + #[allow(non_upper_case_globals)] + pub const #const_name: &str = #const_value; + } +} + +/// Converts "path::class" to "path::new_class". +pub fn replace_class_in_path(path: Path, new_class: Ident) -> Path { + match path.segments.as_slice() { + // Can't happen, you have at least one segment (the class name). + [] => panic!("unexpected: empty path"), + + [_single] => Path { + segments: vec![PathSegment { + ident: new_class, + generic_args: None, + tk_separator_colons: None, + }], + }, + + [path @ .., _last] => { + let mut segments = vec![]; + segments.extend(path.iter().cloned()); + segments.push(PathSegment { + ident: new_class, + generic_args: None, + tk_separator_colons: Some([ + Punct::new(':', Spacing::Joint), + Punct::new(':', Spacing::Alone), + ]), + }); + Path { segments } + } + } +} + +/// Returns the name of the constant that will be autogenerated. +pub fn format_func_name_constant_name(_class_name: &Ident, func_name: &Ident) -> Ident { + format_ident!("{func_name}") +} + +/// Returns the name of the dummy struct that's used as container for all function name constants. +pub fn format_func_name_struct_name(class_name: &Ident) -> Ident { + format_ident!("__gdext_{class_name}_Functions") +} diff --git a/itest/godot/ManualFfiTests.gd b/itest/godot/ManualFfiTests.gd index 88023ad07..cfa85b45e 100644 --- a/itest/godot/ManualFfiTests.gd +++ b/itest/godot/ManualFfiTests.gd @@ -390,3 +390,63 @@ func test_get_set(): assert_eq(obj.set_get, 1000) assert(obj.is_set_called()) assert(obj.is_get_called()) + + +# Validates the shape of the Class defined in Rust: +# In Rust we declared a single property (int_val) and two functions (f1 and f2). +# In addition, Godot defines a property with the name of the Class, which acts as the top-level category in the inspector. +func test_RenamedFunc_shape(): + # Note: RenamedFunc is located in property_test.rs. + var obj: RenamedFunc = RenamedFunc.new() + + # Get baseline Node properties and methods + var base_node = Node.new() + var node_props = base_node.get_property_list().map(func(p): return p.name) + var node_methods = base_node.get_method_list().map(func(m): return m.name) + base_node.free() + + # Get our object's properties and methods + var obj_props = obj.get_property_list().map(func(p): return p.name) + var obj_methods = obj.get_method_list().map(func(m): return m.name) + + # Get only the new properties and methods (not in Node) + var gdext_props = obj_props.filter(func(name): return not node_props.has(name)) + var gdext_methods = obj_methods.filter(func(name): return not node_methods.has(name)) + + # Debug print to see what we found + #print("GDExt properties: ", gdext_props) + #print("GDExt methods: ", gdext_methods) + + # Assert counts + assert_eq(gdext_props.size(), 2, "number of properties should be 2") + assert_eq(gdext_methods.size(), 2, "number of methods should be 2") + + # Assert specific names + assert(gdext_props.has("int_val"), "should have a property named 'int_val'") + assert(gdext_props.has("RenamedFunc"), "should have a property named 'RenamedFunc'") # godot automatically adds a property of the class name + assert(gdext_methods.has("f1"), "should have a method named 'f1'") + assert(gdext_methods.has("f2"), "should have a method named 'f2'") + + obj.free() + + +# Validates that the property has been linked to the correct rust get/set functions. +func test_RenamedFunc_get_set(): + # Note: RenamedFunc is located in property_test.rs. + var obj: RenamedFunc = RenamedFunc.new() + + assert_eq(obj.int_val, 0) + assert_eq(obj.f1(), 0) + + obj.int_val = 42; + + assert_eq(obj.int_val, 42) + assert_eq(obj.f1(), 42) + + obj.f2(84) + + assert_eq(obj.int_val, 84) + assert_eq(obj.f1(), 84) + + obj.free() + diff --git a/itest/rust/src/object_tests/object_test.rs b/itest/rust/src/object_tests/object_test.rs index 63c21e326..8cf5384b5 100644 --- a/itest/rust/src/object_tests/object_test.rs +++ b/itest/rust/src/object_tests/object_test.rs @@ -5,6 +5,9 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +// Needed for Clippy to accept #[cfg(all())] +#![allow(clippy::non_minimal_cfg)] + use std::cell::{Cell, RefCell}; use std::rc::Rc; @@ -1090,3 +1093,21 @@ fn double_use_reference() { double_use.free(); emitter.free(); } + +// ---------------------------------------------------------------------------------------------------------------------------------------------- + +// Test that one class can be declared multiple times (using #[cfg]) without conflicts + +#[derive(GodotClass)] +#[class(init, base=Object)] +struct MultipleStructsCfg {} + +#[derive(GodotClass)] +#[class(init, base=Object)] +#[cfg(any())] +struct MultipleStructsCfg {} + +#[cfg(any())] +#[derive(GodotClass)] +#[class(init, base=Object)] +struct MultipleStructsCfg {} diff --git a/itest/rust/src/object_tests/property_test.rs b/itest/rust/src/object_tests/property_test.rs index 49b3b4f1d..1b878607b 100644 --- a/itest/rust/src/object_tests/property_test.rs +++ b/itest/rust/src/object_tests/property_test.rs @@ -482,3 +482,58 @@ fn override_export() { fn check_property(property: &Dictionary, key: &str, expected: impl ToGodot) { assert_eq!(property.get_or_nil(key), expected.to_variant()); } + +// ---------------------------------------------------------------------------------------------------------------------------------------------- + +#[derive(GodotClass)] +#[class(base=Node, init)] +struct RenamedFunc { + #[var(get = get_int_val, set = set_int_val)] + int_val: i32, +} + +#[godot_api] +impl RenamedFunc { + #[func(rename=f1)] + pub fn get_int_val(&self) -> i32 { + self.int_val + } + + #[func(rename=f2)] + pub fn set_int_val(&mut self, val: i32) { + self.int_val = val; + } +} + +#[itest] +fn test_var_with_renamed_funcs() { + let mut obj = RenamedFunc::new_alloc(); + + assert_eq!(obj.bind().int_val, 0); + assert_eq!(obj.bind().get_int_val(), 0); + assert_eq!(obj.call("f1", &[]).to::(), 0); + assert_eq!(obj.get("int_val").to::(), 0); + + obj.bind_mut().int_val = 42; + + assert_eq!(obj.bind().int_val, 42); + assert_eq!(obj.bind().get_int_val(), 42); + assert_eq!(obj.call("f1", &[]).to::(), 42); + assert_eq!(obj.get("int_val").to::(), 42); + + obj.call("f2", &[84.to_variant()]); + + assert_eq!(obj.bind().int_val, 84); + assert_eq!(obj.bind().get_int_val(), 84); + assert_eq!(obj.call("f1", &[]).to::(), 84); + assert_eq!(obj.get("int_val").to::(), 84); + + obj.set("int_val", &128.to_variant()); + + assert_eq!(obj.bind().int_val, 128); + assert_eq!(obj.bind().get_int_val(), 128); + assert_eq!(obj.call("f1", &[]).to::(), 128); + assert_eq!(obj.get("int_val").to::(), 128); + + obj.free(); +} diff --git a/itest/rust/src/register_tests/func_test.rs b/itest/rust/src/register_tests/func_test.rs index 94fd08581..0ceee9c95 100644 --- a/itest/rust/src/register_tests/func_test.rs +++ b/itest/rust/src/register_tests/func_test.rs @@ -130,6 +130,20 @@ impl GdSelfObj { compile_error!("Removed by #[cfg]") } + // note about the panic: We just need a condition that always evaluates to true, and #[cfg_attr(true)] is still experimental. (https://github.com/rust-lang/rust/issues/131204) + #[cfg_attr(any(panic = "abort", panic = "unwind"), cfg(any()))] + #[func] + fn cfg_removes_duplicate_function_impl() -> bool { + compile_error!("Removed by #[cfg]") + } + + #[func] + // note about the panic: We just need a condition that always evaluates to true, and #[cfg_attr(true)] is still experimental. (https://github.com/rust-lang/rust/issues/131204) + #[cfg_attr(any(panic = "abort", panic = "unwind"), cfg(any()))] + fn cfg_removes_duplicate_function_impl() -> bool { + compile_error!("Removed by #[cfg]") + } + #[cfg(any())] #[func] fn cfg_removes_function() -> bool { From 22127f511f179c1be3400917349b4cbbdfcdc31d Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 1 Feb 2025 18:56:46 +0100 Subject: [PATCH 2/2] Follow-up changes to "func collections" --- .../src/class/data_models/field_var.rs | 11 +++-- .../src/class/data_models/inherent_impl.rs | 32 ++++++------ .../src/class/data_models/property.rs | 48 ++++++++---------- godot-macros/src/class/derive_godot_class.rs | 15 +++--- godot-macros/src/class/godot_api.rs | 4 +- godot-macros/src/util/mod.rs | 49 +++++++++++-------- itest/godot/ManualFfiTests.gd | 19 +++---- itest/rust/src/register_tests/func_test.rs | 6 ++- 8 files changed, 90 insertions(+), 94 deletions(-) diff --git a/godot-macros/src/class/data_models/field_var.rs b/godot-macros/src/class/data_models/field_var.rs index c0a543e0c..4b3c25be0 100644 --- a/godot-macros/src/class/data_models/field_var.rs +++ b/godot-macros/src/class/data_models/field_var.rs @@ -12,7 +12,7 @@ use crate::class::{ into_signature_info, make_existence_check, make_method_registration, Field, FieldHint, FuncDefinition, }; -use crate::util::make_func_name_constant; +use crate::util::make_funcs_collection_constant; use crate::util::KvParser; use crate::{util, ParseResult}; @@ -167,7 +167,7 @@ pub struct GetterSetterImpl { pub function_name: Ident, pub function_impl: TokenStream, pub export_token: TokenStream, - pub func_name_const: TokenStream, + pub funcs_collection_constant: TokenStream, } impl GetterSetterImpl { @@ -208,7 +208,8 @@ impl GetterSetterImpl { } }; - let func_name_const = make_func_name_constant(class_name, &function_name, None, &[]); + let funcs_collection_constant = + make_funcs_collection_constant(class_name, &function_name, None, &[]); let signature = util::parse_signature(signature); let export_token = make_method_registration( @@ -234,7 +235,7 @@ impl GetterSetterImpl { function_name, function_impl, export_token, - func_name_const, + funcs_collection_constant, } } @@ -243,7 +244,7 @@ impl GetterSetterImpl { function_name: function_name.clone(), function_impl: TokenStream::new(), export_token: make_existence_check(function_name), - func_name_const: TokenStream::new(), + funcs_collection_constant: TokenStream::new(), } } } diff --git a/godot-macros/src/class/data_models/inherent_impl.rs b/godot-macros/src/class/data_models/inherent_impl.rs index bab0bdf3a..89759089b 100644 --- a/godot-macros/src/class/data_models/inherent_impl.rs +++ b/godot-macros/src/class/data_models/inherent_impl.rs @@ -11,7 +11,7 @@ use crate::class::{ SignatureInfo, TransferMode, }; use crate::util::{ - bail, c_str, error_fn, format_func_name_struct_name, ident, make_func_name_constants, + bail, c_str, format_funcs_collection_struct, ident, make_funcs_collection_constants, replace_class_in_path, require_api_version, KvParser, }; use crate::{handle_mutually_exclusive_keys, util, ParseResult}; @@ -19,7 +19,6 @@ use crate::{handle_mutually_exclusive_keys, util, ParseResult}; use proc_macro2::{Delimiter, Group, Ident, TokenStream}; use quote::spanned::Spanned; use quote::{format_ident, quote}; -use venial::Path; /// Attribute for user-declared function. enum ItemAttrType { @@ -79,6 +78,7 @@ pub struct InherentImplAttr { pub fn transform_inherent_impl( meta: InherentImplAttr, mut impl_block: venial::Impl, + self_path: venial::Path, ) -> ParseResult { let class_name = util::validate_impl(&impl_block, None, "godot_api")?; let class_name_obj = util::class_name_obj(&class_name); @@ -93,19 +93,15 @@ pub fn transform_inherent_impl( #[cfg(not(all(feature = "register-docs", since_api = "4.3")))] let docs = quote! {}; - // This is the container struct that holds the names of all registered #[func]s. - // (The struct is declared by the macro derive_godot_class.) - let class_functions_name = format_func_name_struct_name(&class_name); - // As the impl block could be of the form "path::class", and we add a second impl block below, we need the full path, not just the class name. - let this_class_full_path = impl_block.self_ty.as_path().ok_or(error_fn( - "unexpected: the function already checked 'as_path' above in validate_impl", - &impl_block, - ))?; - let class_functions_path: Path = - replace_class_in_path(this_class_full_path, class_functions_name); - // For each #[func] in this impl block, we create one constant. - let func_name_constants = make_func_name_constants(&funcs, &class_name); + // Container struct holding names of all registered #[func]s. + // The struct is declared by #[derive(GodotClass)]. + let funcs_collection = { + let struct_name = format_funcs_collection_struct(&class_name); + replace_class_in_path(self_path, struct_name) + }; + // For each #[func] in this impl block, create one constant. + let func_name_constants = make_funcs_collection_constants(&funcs, &class_name); let signal_registrations = make_signal_registrations(signals, &class_name_obj); #[cfg(feature = "codegen-full")] @@ -181,8 +177,8 @@ pub fn transform_inherent_impl( #trait_impl #fill_storage #class_registration - impl #class_functions_path { - #( #func_name_constants )* + impl #funcs_collection { + #( #func_name_constants )* } }; @@ -194,8 +190,8 @@ pub fn transform_inherent_impl( let result = quote! { #impl_block #fill_storage - impl #class_functions_path { - #( #func_name_constants )* + impl #funcs_collection { + #( #func_name_constants )* } }; diff --git a/godot-macros/src/class/data_models/property.rs b/godot-macros/src/class/data_models/property.rs index 0a04aac0f..719c61a9e 100644 --- a/godot-macros/src/class/data_models/property.rs +++ b/godot-macros/src/class/data_models/property.rs @@ -5,10 +5,10 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -//! Parsing the `var` and `export` attributes on fields. +//! Parses the `#[var]` and `#[export]` attributes on fields. use crate::class::{Field, FieldVar, Fields, GetSet, GetterSetterImpl, UsageFlags}; -use crate::util::{format_func_name_constant_name, format_func_name_struct_name, ident}; +use crate::util::{format_funcs_collection_constant, format_funcs_collection_struct}; use proc_macro2::{Ident, TokenStream}; use quote::quote; @@ -136,7 +136,7 @@ pub fn make_property_impl(class_name: &Ident, fields: &Fields) -> TokenStream { }, }; - // Note: (getter/setter)_tokens can be either a path ``Class_Functions::constant_name`` or an empty string ``""``. + // Note: {getter,setter}_tokens can be either a path `Class_Functions::constant_name` or an empty string `""`. let getter_tokens = make_getter_setter( getter.to_impl(class_name, GetSet::Get, field), @@ -164,9 +164,9 @@ pub fn make_property_impl(class_name: &Ident, fields: &Fields) -> TokenStream { }); } - // For each generated #[func], add a const. - // This is the name of the container struct, which is declared by the derive macro GodotClass. - let class_functions_name = format_func_name_struct_name(class_name); + // For each generated #[func], add a const declaration. + // This is the name of the container struct, which is declared by #[derive(GodotClass)]. + let class_functions_name = format_funcs_collection_struct(class_name); quote! { impl #class_name { @@ -196,26 +196,18 @@ fn make_getter_setter( export_tokens: &mut Vec, class_name: &Ident, ) -> TokenStream { - if let Some(getter_impl) = getter_setter_impl { - let GetterSetterImpl { - function_name, - function_impl, - export_token, - func_name_const, - } = getter_impl; - - getter_setter_impls.push(function_impl); - func_name_consts.push(func_name_const); - export_tokens.push(export_token); - - let getter_setter_name = function_name.to_string(); - - let class_functions_name = format_func_name_struct_name(class_name); - - let getter_setter_fn_const = - format_func_name_constant_name(class_name, &ident(&getter_setter_name)); - quote! { #class_functions_name::#getter_setter_fn_const } - } else { - quote! { "" } - } + let Some(gs) = getter_setter_impl else { + return quote! { "" }; + }; + + getter_setter_impls.push(gs.function_impl); + func_name_consts.push(gs.funcs_collection_constant); + export_tokens.push(gs.export_token); + + // Getters/setters are, like #[func]s, subject to additional code generation: a constant inside a "funcs collection" struct + // stores their Godot name and can be used as an indirection to refer to their true name from other procedural macros. + let funcs_collection = format_funcs_collection_struct(class_name); + let constant = format_funcs_collection_constant(class_name, &gs.function_name); + + quote! { #funcs_collection::#constant } } diff --git a/godot-macros/src/class/derive_godot_class.rs b/godot-macros/src/class/derive_godot_class.rs index 58e8338bd..756b48a50 100644 --- a/godot-macros/src/class/derive_godot_class.rs +++ b/godot-macros/src/class/derive_godot_class.rs @@ -13,8 +13,8 @@ use crate::class::{ FieldVar, Fields, SignatureInfo, }; use crate::util::{ - bail, error, format_func_name_struct_name, ident, path_ends_with_complex, require_api_version, - KvParser, + bail, error, format_funcs_collection_struct, ident, path_ends_with_complex, + require_api_version, KvParser, }; use crate::{handle_mutually_exclusive_keys, util, ParseResult}; @@ -137,11 +137,12 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { modifiers.push(quote! { with_tool }) } - // Declares a dummy struct that, for each #[func], holds a constant that maps the rust name to the name under which it is registered in godot. - let class_functions_name = format_func_name_struct_name(class_name); - let class_functions_struct = quote! { + // Declares a "funcs collection" struct that, for holds a constant for each #[func]. + // That constant maps the Rust name (constant ident) to the Godot registered name (string value). + let funcs_collection_struct_name = format_funcs_collection_struct(class_name); + let funcs_collection_struct = quote! { #[doc(hidden)] - pub struct #class_functions_name { } + pub struct #funcs_collection_struct_name {} }; Ok(quote! { @@ -167,7 +168,7 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { type Exportable = <::Base as ::godot::obj::Bounds>::Exportable; } - #class_functions_struct + #funcs_collection_struct #godot_init_impl #godot_withbase_impl #godot_exports_impl diff --git a/godot-macros/src/class/godot_api.rs b/godot-macros/src/class/godot_api.rs index 0c233b058..178331799 100644 --- a/godot-macros/src/class/godot_api.rs +++ b/godot-macros/src/class/godot_api.rs @@ -41,7 +41,7 @@ pub fn attribute_godot_api( )?; } - if decl.self_ty.as_path().is_none() { + let Some(self_path) = decl.self_ty.as_path() else { return bail!(decl, "invalid Self type for #[godot_api] impl"); }; @@ -57,7 +57,7 @@ pub fn attribute_godot_api( transform_trait_impl(decl) } else { match parse_inherent_impl_attr(meta) { - Ok(meta) => transform_inherent_impl(meta, decl), + Ok(meta) => transform_inherent_impl(meta, decl, self_path), Err(err) => Err(err), } } diff --git a/godot-macros/src/util/mod.rs b/godot-macros/src/util/mod.rs index 66578db00..46fbcaba0 100644 --- a/godot-macros/src/util/mod.rs +++ b/godot-macros/src/util/mod.rs @@ -12,7 +12,6 @@ use crate::ParseResult; use proc_macro2::{Delimiter, Group, Ident, Literal, Punct, Spacing, TokenStream, TokenTree}; use quote::spanned::Spanned; use quote::{format_ident, quote, ToTokens, TokenStreamExt}; -use venial::{Attribute, Path, PathSegment}; mod kv_parser; mod list_parser; @@ -248,14 +247,17 @@ pub(crate) fn extract_cfg_attrs( let Some(attr_name) = attr.get_single_path_segment() else { return false; }; + // #[cfg(condition)] if attr_name == "cfg" { return true; } - // #[cfg_attr(condition, attributes...)], note that there can be multiple attributes seperated by comma. + + // #[cfg_attr(condition, attributes...)]. Multiple attributes can be seperated by comma. if attr_name == "cfg_attr" && attr.value.to_token_stream().to_string().contains("cfg(") { return true; } + false }) } @@ -320,7 +322,10 @@ pub fn venial_parse_meta( // util functions for handling #[func]s and #[var(get=f, set=f)] -pub fn make_func_name_constants(funcs: &[FuncDefinition], class_name: &Ident) -> Vec { +pub fn make_funcs_collection_constants( + funcs: &[FuncDefinition], + class_name: &Ident, +) -> Vec { funcs .iter() .map(|func| { @@ -329,7 +334,7 @@ pub fn make_func_name_constants(funcs: &[FuncDefinition], class_name: &Ident) -> .into_iter() .collect::>(); - make_func_name_constant( + make_funcs_collection_constant( class_name, &func.signature_info.method_name, func.registered_name.as_ref(), @@ -339,15 +344,17 @@ pub fn make_func_name_constants(funcs: &[FuncDefinition], class_name: &Ident) -> .collect() } -/// Funcs can be renamed with `#[func(rename=new_name) fn f();`. -/// To be able to access the renamed function name at a later point, it is saved in a string constant. -pub fn make_func_name_constant( +/// Returns a `const` declaration for the funcs collection struct. +/// +/// User-defined functions can be renamed with `#[func(rename=new_name)]`. To be able to access the renamed function name from another macro, +/// a constant is used as indirection. +pub fn make_funcs_collection_constant( class_name: &Ident, func_name: &Ident, registered_name: Option<&String>, - attributes: &[&Attribute], + attributes: &[&venial::Attribute], ) -> TokenStream { - let const_name = format_func_name_constant_name(class_name, func_name); + let const_name = format_funcs_collection_constant(class_name, func_name); let const_value = match ®istered_name { Some(renamed) => renamed.to_string(), None => func_name.to_string(), @@ -365,14 +372,14 @@ pub fn make_func_name_constant( } } -/// Converts "path::class" to "path::new_class". -pub fn replace_class_in_path(path: Path, new_class: Ident) -> Path { +/// Converts `path::class` to `path::new_class`. +pub fn replace_class_in_path(path: venial::Path, new_class: Ident) -> venial::Path { match path.segments.as_slice() { // Can't happen, you have at least one segment (the class name). - [] => panic!("unexpected: empty path"), + [] => unreachable!("empty path"), - [_single] => Path { - segments: vec![PathSegment { + [_single] => venial::Path { + segments: vec![venial::PathSegment { ident: new_class, generic_args: None, tk_separator_colons: None, @@ -382,7 +389,7 @@ pub fn replace_class_in_path(path: Path, new_class: Ident) -> Path { [path @ .., _last] => { let mut segments = vec![]; segments.extend(path.iter().cloned()); - segments.push(PathSegment { + segments.push(venial::PathSegment { ident: new_class, generic_args: None, tk_separator_colons: Some([ @@ -390,17 +397,17 @@ pub fn replace_class_in_path(path: Path, new_class: Ident) -> Path { Punct::new(':', Spacing::Alone), ]), }); - Path { segments } + venial::Path { segments } } } } -/// Returns the name of the constant that will be autogenerated. -pub fn format_func_name_constant_name(_class_name: &Ident, func_name: &Ident) -> Ident { +/// Returns the name of the constant inside the func "collection" struct. +pub fn format_funcs_collection_constant(_class_name: &Ident, func_name: &Ident) -> Ident { format_ident!("{func_name}") } -/// Returns the name of the dummy struct that's used as container for all function name constants. -pub fn format_func_name_struct_name(class_name: &Ident) -> Ident { - format_ident!("__gdext_{class_name}_Functions") +/// Returns the name of the struct used as collection for all function name constants. +pub fn format_funcs_collection_struct(class_name: &Ident) -> Ident { + format_ident!("__gdext_{class_name}_Funcs") } diff --git a/itest/godot/ManualFfiTests.gd b/itest/godot/ManualFfiTests.gd index cfa85b45e..92e5a7044 100644 --- a/itest/godot/ManualFfiTests.gd +++ b/itest/godot/ManualFfiTests.gd @@ -392,10 +392,10 @@ func test_get_set(): assert(obj.is_get_called()) -# Validates the shape of the Class defined in Rust: -# In Rust we declared a single property (int_val) and two functions (f1 and f2). -# In addition, Godot defines a property with the name of the Class, which acts as the top-level category in the inspector. -func test_RenamedFunc_shape(): +# Validates the shape of the class defined in Rust: +# - Rust declares a single property (int_val) and two functions (f1 and f2). +# - In addition, Godot defines a property with the name of the class, which acts as the top-level category in the inspector UI. +func test_renamed_func_shape(): # Note: RenamedFunc is located in property_test.rs. var obj: RenamedFunc = RenamedFunc.new() @@ -412,18 +412,15 @@ func test_RenamedFunc_shape(): # Get only the new properties and methods (not in Node) var gdext_props = obj_props.filter(func(name): return not node_props.has(name)) var gdext_methods = obj_methods.filter(func(name): return not node_methods.has(name)) - - # Debug print to see what we found - #print("GDExt properties: ", gdext_props) - #print("GDExt methods: ", gdext_methods) - + # Assert counts assert_eq(gdext_props.size(), 2, "number of properties should be 2") assert_eq(gdext_methods.size(), 2, "number of methods should be 2") # Assert specific names assert(gdext_props.has("int_val"), "should have a property named 'int_val'") - assert(gdext_props.has("RenamedFunc"), "should have a property named 'RenamedFunc'") # godot automatically adds a property of the class name + # Godot automatically adds a property of the class name (acts as the top-level category in the inspector UI). + assert(gdext_props.has("RenamedFunc"), "should have a property named 'RenamedFunc'") assert(gdext_methods.has("f1"), "should have a method named 'f1'") assert(gdext_methods.has("f2"), "should have a method named 'f2'") @@ -431,7 +428,7 @@ func test_RenamedFunc_shape(): # Validates that the property has been linked to the correct rust get/set functions. -func test_RenamedFunc_get_set(): +func test_renamed_func_get_set(): # Note: RenamedFunc is located in property_test.rs. var obj: RenamedFunc = RenamedFunc.new() diff --git a/itest/rust/src/register_tests/func_test.rs b/itest/rust/src/register_tests/func_test.rs index 0ceee9c95..43aab3efa 100644 --- a/itest/rust/src/register_tests/func_test.rs +++ b/itest/rust/src/register_tests/func_test.rs @@ -130,7 +130,8 @@ impl GdSelfObj { compile_error!("Removed by #[cfg]") } - // note about the panic: We just need a condition that always evaluates to true, and #[cfg_attr(true)] is still experimental. (https://github.com/rust-lang/rust/issues/131204) + // Why `panic = "abort"`: we need a condition that always evaluates to true, and #[cfg_attr(true)] is still experimental. + // (https://github.com/rust-lang/rust/issues/131204) #[cfg_attr(any(panic = "abort", panic = "unwind"), cfg(any()))] #[func] fn cfg_removes_duplicate_function_impl() -> bool { @@ -138,7 +139,8 @@ impl GdSelfObj { } #[func] - // note about the panic: We just need a condition that always evaluates to true, and #[cfg_attr(true)] is still experimental. (https://github.com/rust-lang/rust/issues/131204) + // Why `panic = "abort"`: we need a condition that always evaluates to true, and #[cfg_attr(true)] is still experimental. + // (https://github.com/rust-lang/rust/issues/131204) #[cfg_attr(any(panic = "abort", panic = "unwind"), cfg(any()))] fn cfg_removes_duplicate_function_impl() -> bool { compile_error!("Removed by #[cfg]")