diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index 775daa6d5..206d0e4ae 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -326,7 +326,8 @@ impl Gd { /// Equivalent to [`upcast::()`][Self::upcast], but without bounds. // Not yet public because it might need _mut/_ref overloads, and 6 upcast methods are a bit much... - pub(crate) fn upcast_object(self) -> Gd { + #[doc(hidden)] // no public API, but used by #[signal]. + pub fn upcast_object(self) -> Gd { self.owned_cast() .expect("Upcast to Object failed. This is a bug; please report it.") } diff --git a/godot-macros/src/class/data_models/inherent_impl.rs b/godot-macros/src/class/data_models/inherent_impl.rs index eba6a3fb7..69f11d887 100644 --- a/godot-macros/src/class/data_models/inherent_impl.rs +++ b/godot-macros/src/class/data_models/inherent_impl.rs @@ -71,14 +71,17 @@ struct SignalAttr { pub no_builder: bool, } -// ---------------------------------------------------------------------------------------------------------------------------------------------- - -pub struct InherentImplAttr { +pub(crate) struct InherentImplAttr { /// For implementation reasons, there can be a single 'primary' impl block and 0 or more 'secondary' impl blocks. /// For now, this is controlled by a key in the 'godot_api' attribute. pub secondary: bool, + + /// When typed signal generation is explicitly disabled by the user. + pub no_typed_signals: bool, } +// ---------------------------------------------------------------------------------------------------------------------------------------------- + /// Codegen for `#[godot_api] impl MyType` pub fn transform_inherent_impl( meta: InherentImplAttr, @@ -107,8 +110,12 @@ pub fn transform_inherent_impl( // For each #[func] in this impl block, create one constant. let func_name_constants = make_funcs_collection_constants(&funcs, &class_name); - let (signal_registrations, signal_symbol_types) = - make_signal_registrations(&signals, &class_name, &class_name_obj)?; + let (signal_registrations, signal_symbol_types) = make_signal_registrations( + &signals, + &class_name, + &class_name_obj, + meta.no_typed_signals, + )?; #[cfg(feature = "codegen-full")] let rpc_registrations = crate::class::make_rpc_registrations_fn(&class_name, &funcs); @@ -206,6 +213,23 @@ pub fn transform_inherent_impl( } } +/* Re-enable if we allow controlling declarative macros for signals (base_field_macro, visibility_macros). +fn extract_hint_attribute(impl_block: &mut venial:: Impl) -> ParseResult { + // #[hint(has_base_field = BOOL)] + let has_base_field; + if let Some(mut hints) = KvParser::parse_remove(&mut impl_block.attributes, "hint")? { + has_base_field = hints.handle_bool("has_base_field")?; + } else { + has_base_field = None; + } + + // #[hint(class_visibility = pub(crate))] + // ... + + Ok(GodotApiHints { has_base_field }) +} +*/ + fn process_godot_fns( class_name: &Ident, impl_block: &mut venial::Impl, diff --git a/godot-macros/src/class/data_models/signal.rs b/godot-macros/src/class/data_models/signal.rs index 6ab368722..d46a6e7ae 100644 --- a/godot-macros/src/class/data_models/signal.rs +++ b/godot-macros/src/class/data_models/signal.rs @@ -182,6 +182,7 @@ pub fn make_signal_registrations( signals: &[SignalDefinition], class_name: &Ident, class_name_obj: &TokenStream, + no_typed_signals: bool, ) -> ParseResult<(Vec, Option)> { let mut signal_registrations = Vec::new(); @@ -210,8 +211,11 @@ pub fn make_signal_registrations( signal_registrations.push(registration); } + // Rewrite the above using #[cfg]. #[cfg(since_api = "4.2")] - let signal_symbols = make_signal_symbols(class_name, collection_api); + let signal_symbols = + (!no_typed_signals).then(|| make_signal_symbols(class_name, collection_api)); + #[cfg(before_api = "4.2")] let signal_symbols = None; @@ -311,7 +315,7 @@ impl SignalCollection { .push(make_signal_individual_struct(details)) } - pub fn is_empty(&self) -> bool { + fn is_empty(&self) -> bool { self.individual_structs.is_empty() } } @@ -380,18 +384,26 @@ fn make_signal_individual_struct(details: &SignalDetails) -> TokenStream { /// Generates symbolic API for signals: /// * collection (unspecified-name struct holding methods to access each signal) +/// * can be absent if the class declares no own #[signal]s. /// * individual signal types /// * trait impls fn make_signal_symbols( class_name: &Ident, collection_api: SignalCollection, // max_visibility: SignalVisibility, -) -> Option { - // Future note: if we add Rust->Rust inheritance, then the WithSignals trait must be unconditionally implemented. - if collection_api.is_empty() { - return None; - } +) -> TokenStream { + // Earlier implementation generated a simplified code when no #[signal] was declared: only WithSignals/WithUserSignals impl, but no own + // collection, instead the associated type pointing to the base class. This has however some problems: + // * Part of the reason for user-defined collection is to store UserSignalObject instead of Gd, which can store &mut self. + // This is necessary for self.signals().some_base_signal().emit(), if such a signal is connected to Self::method_mut; + // Gd would cause a borrow error. + // * Once we add Rust-Rust inheritance, we'd need to differentiate case again, which can be tricky since #[godot_api] has no information + // about the base class. + // + // As compromise, we always generate a collection struct if either at least 1 #[signal] is declared or the struct has a Base field. + // We also provide opt-out via #[godot_api(no_typed_signals)]. + let declares_no_signals = collection_api.is_empty(); let collection_struct_name = format_ident!("__godot_Signals_{}", class_name); let collection_struct_methods = &collection_api.provider_methods; let with_signals_impl = make_with_signals_impl(class_name, &collection_struct_name); @@ -431,7 +443,7 @@ fn make_signal_symbols( let visibility_macro = util::format_class_visibility_macro(class_name); - let code = quote! { + let mut code = quote! { #visibility_macro! { #[allow(non_camel_case_types)] #[doc(hidden)] // Only on struct, not methods, to allow completion in IDEs. @@ -455,9 +467,21 @@ fn make_signal_symbols( #( #individual_structs )* }; - Some(code) + // base_field_macro! is a macro that expands to all input tokens if the class declares a Base field, and to nothing otherwise. + // This makes sure that WithSignals is only implemented for classes with a base field, and avoids compile errors about it. + // Only when no #[signal] is declared -> otherwise the user explicitly requests it, and a compile error is better to guide them. + if declares_no_signals { + let base_field_macro = util::format_class_base_field_macro(class_name); + + code = quote! { + #base_field_macro! { #code } + }; + } + + code } +/// Declare `impl WithSignals` and `impl WithUserSignals` with own signal collection. fn make_with_signals_impl(class_name: &Ident, collection_struct_name: &Ident) -> TokenStream { quote! { impl ::godot::obj::WithSignals for #class_name { @@ -467,10 +491,10 @@ fn make_with_signals_impl(class_name: &Ident, collection_struct_name: &Ident) -> type __SignalObj<'c> = ::godot::private::UserSignalObject<'c, Self>; #[doc(hidden)] - fn __signals_from_external(external: &mut Gd) -> Self::SignalCollection<'_, Self> { + fn __signals_from_external(external: &mut ::godot::obj::Gd) -> Self::SignalCollection<'_, Self> { Self::SignalCollection { __internal_obj: Some(::godot::private::UserSignalObject::External { - gd: external.clone().upcast::() + gd: external.clone().upcast_object() }) } } diff --git a/godot-macros/src/class/derive_godot_class.rs b/godot-macros/src/class/derive_godot_class.rs index 5cd42138f..dd084d3d5 100644 --- a/godot-macros/src/class/derive_godot_class.rs +++ b/godot-macros/src/class/derive_godot_class.rs @@ -142,7 +142,9 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { pub struct #funcs_collection_struct_name {} }; + // Note: one limitation is that macros don't work for `impl nested::MyClass` blocks. let visibility_macro = make_visibility_macro(class_name, class.vis_marker.as_ref()); + let base_field_macro = make_base_field_macro(class_name, fields.base_field.is_some()); Ok(quote! { impl ::godot::obj::GodotClass for #class_name { @@ -174,6 +176,7 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { #user_class_impl #init_expecter #visibility_macro + #base_field_macro #( #deprecations )* #( #errors )* @@ -211,6 +214,25 @@ fn make_visibility_macro( } } +/// Generates code for a decl-macro, which evaluates to nothing if the class has no base field. +fn make_base_field_macro(class_name: &Ident, has_base_field: bool) -> TokenStream { + let macro_name = util::format_class_base_field_macro(class_name); + + if has_base_field { + quote! { + macro_rules! #macro_name { + ( $( $tt:tt )* ) => { $( $tt )* }; + } + } + } else { + quote! { + macro_rules! #macro_name { + ( $( $tt:tt )* ) => {}; + } + } + } +} + /// Checks at compile time that a function with the given name exists on `Self`. #[must_use] pub fn make_existence_check(ident: &Ident) -> TokenStream { diff --git a/godot-macros/src/class/godot_api.rs b/godot-macros/src/class/godot_api.rs index 178331799..f19e2a060 100644 --- a/godot-macros/src/class/godot_api.rs +++ b/godot-macros/src/class/godot_api.rs @@ -17,9 +17,20 @@ fn parse_inherent_impl_attr(meta: TokenStream) -> Result ParseResult> { let mut found_attr: Option = None; @@ -45,7 +47,7 @@ impl KvParser { let path = &attr.path; if path_is_single(path, expected) { if found_attr.is_some() { - return bail!(attr, "only a single #[{expected}] attribute allowed",); + return bail!(attr, "only a single #[{expected}] attribute allowed"); } let attr_name = expected.to_string(); @@ -59,6 +61,40 @@ impl KvParser { Ok(found_attr) } + /// Like `parse()`, but removes the attribute from the list. + /// + /// Useful for `#[proc_macro_attributes]`, where handled attributes must not show up in resulting code. + // Currently unused. + pub fn parse_remove( + attributes: &mut Vec, + expected: &str, + ) -> ParseResult> { + let mut found_attr: Option = None; + let mut found_index: Option = None; + + for (i, attr) in attributes.iter().enumerate() { + let path = &attr.path; + if path_is_single(path, expected) { + if found_attr.is_some() { + return bail!(attr, "only a single #[{expected}] attribute allowed"); + } + + let attr_name = expected.to_string(); + found_index = Some(i); + found_attr = Some(Self { + span: attr.tk_brackets.span, + map: ParserState::parse(attr_name, &attr.value)?, + }); + } + } + + if let Some(index) = found_index { + attributes.remove(index); + } + + Ok(found_attr) + } + pub fn span(&self) -> Span { self.span } diff --git a/godot-macros/src/util/mod.rs b/godot-macros/src/util/mod.rs index d10a51a01..627d5c52f 100644 --- a/godot-macros/src/util/mod.rs +++ b/godot-macros/src/util/mod.rs @@ -409,3 +409,8 @@ pub fn format_funcs_collection_struct(class_name: &Ident) -> Ident { pub fn format_class_visibility_macro(class_name: &Ident) -> Ident { format_ident!("__godot_{class_name}_vis_macro") } + +/// Returns the name of the macro used to communicate whether the `struct` (class) contains a base field. +pub fn format_class_base_field_macro(class_name: &Ident) -> Ident { + format_ident!("__godot_{class_name}_has_base_field_macro") +} diff --git a/itest/rust/src/builtin_tests/containers/signal_test.rs b/itest/rust/src/builtin_tests/containers/signal_test.rs index d18f4d278..02c022ece 100644 --- a/itest/rust/src/builtin_tests/containers/signal_test.rs +++ b/itest/rust/src/builtin_tests/containers/signal_test.rs @@ -327,6 +327,24 @@ fn signal_symbols_engine_inherited_internal() { node.free(); } +// Test that Node signals are accessible from a derived class, when the class itself has no #[signal] declarations. +// Verifies the code path that only generates the traits, no dedicated signal collection. +#[cfg(since_api = "4.2")] +#[itest] +fn signal_symbols_engine_inherited_no_own_signals() { + let mut obj = Receiver::new_alloc(); + + let mut sig = obj.signals().property_list_changed(); + sig.connect_self(|this: &mut Receiver| { + this.receive_int(941); + }); + + obj.notify_property_list_changed(); + assert_eq!(obj.bind().last_received.get(), LastReceived::Int(941)); + + obj.free(); +} + #[itest] fn signal_construction_and_id() { let mut object = RefCounted::new_gd(); @@ -451,6 +469,8 @@ struct Receiver { #[godot_api] impl Receiver { + // Do not declare any #[signal]s here -- explicitly test this implements WithSignal without them. + fn last_received(&self) -> LastReceived { self.last_received.get() } diff --git a/itest/rust/src/object_tests/object_test.rs b/itest/rust/src/object_tests/object_test.rs index b249f6ba6..947641486 100644 --- a/itest/rust/src/object_tests/object_test.rs +++ b/itest/rust/src/object_tests/object_test.rs @@ -997,7 +997,9 @@ pub mod object_test_gd { } use nested::ObjectTest; - #[godot_api] + // Disabling signals allows nested::ObjectTest, which would fail otherwise due to generated decl-macro being out-of-scope. + #[godot_api(no_typed_signals)] + // #[hint(has_base_field = false)] // if we allow more fine-grained control in the future impl nested::ObjectTest { #[func] fn pass_object(&self, object: Gd) -> i64 {