Allow #[hint(typed_signals = true|false)] to influence signals being used

Author: Bromeon

godot-macros/src/class/data_models/inherent_impl.rs

@@ -19,6 +19,7 @@ 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::Impl;
 
 /// Attribute for user-declared function.
 enum ItemAttrType {
@@ -71,6 +72,11 @@ struct SignalAttr {
     pub no_builder: bool,
 }
 
+#[derive(Default)]
+pub(crate) struct GodotApiHints {
+    pub has_typed_signals: Option<bool>,
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 
 pub struct InherentImplAttr {
@@ -89,6 +95,8 @@ pub fn transform_inherent_impl(
     let class_name_obj = util::class_name_obj(&class_name);
     let prv = quote! { ::godot::private };
 
+    let hints = extract_hint_attribute(&mut impl_block)?;
+
     // Can add extra functions to the end of the impl block.
     let (funcs, signals) = process_godot_fns(&class_name, &mut impl_block, meta.secondary)?;
     let consts = process_godot_constants(&mut impl_block)?;
@@ -108,7 +116,7 @@ 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)?;
+        make_signal_registrations(&signals, &class_name, &class_name_obj, hints)?;
 
     #[cfg(feature = "codegen-full")]
     let rpc_registrations = crate::class::make_rpc_registrations_fn(&class_name, &funcs);
@@ -206,6 +214,20 @@ pub fn transform_inherent_impl(
     }
 }
 
+fn extract_hint_attribute(impl_block: &mut Impl) -> ParseResult<GodotApiHints> {
+    // Could possibly be extended with #[hint(signal_vis = pub)] or so.
+
+    // #[hint(typed_signals)]
+    let has_typed_signals;
+    if let Some(mut hints) = KvParser::parse_remove(&mut impl_block.attributes, "hint")? {
+        has_typed_signals = hints.handle_bool("typed_signals")?;
+    } else {
+        has_typed_signals = None;
+    }
+
+    Ok(GodotApiHints { has_typed_signals })
+}
+
 fn process_godot_fns(
     class_name: &Ident,
     impl_block: &mut venial::Impl,

godot-macros/src/class/data_models/signal.rs

@@ -7,6 +7,7 @@
 
 // Some duplication with godot-codegen/signals.rs; see comments there.
 
+use crate::class::GodotApiHints;
 use crate::util::bail;
 use crate::{util, ParseResult};
 use proc_macro2::{Delimiter, Ident, TokenStream, TokenTree};
@@ -182,6 +183,7 @@ pub fn make_signal_registrations(
     signals: &[SignalDefinition],
     class_name: &Ident,
     class_name_obj: &TokenStream,
+    hints: GodotApiHints,
 ) -> ParseResult<(Vec<TokenStream>, Option<TokenStream>)> {
     let mut signal_registrations = Vec::new();
 
@@ -211,7 +213,7 @@ pub fn make_signal_registrations(
     }
 
     #[cfg(since_api = "4.2")]
-    let signal_symbols = Some(make_signal_symbols(class_name, collection_api));
+    let signal_symbols = Some(make_signal_symbols(class_name, collection_api, hints));
     #[cfg(before_api = "4.2")]
     let signal_symbols = None;
 
@@ -387,20 +389,30 @@ fn make_signal_symbols(
     class_name: &Ident,
     collection_api: SignalCollection,
     // max_visibility: SignalVisibility,
+    hints: GodotApiHints,
 ) -> TokenStream {
     // If class declares no own signals, just implement the traits, no collection APIs.
-    if collection_api.is_empty() {
+    if hints.has_typed_signals == Some(false) || collection_api.is_empty() {
         let with_signals_impl = make_with_signals_impl_delegated_to_base(class_name);
-        let base_field_macro = util::format_class_base_field_macro(class_name);
 
         // base_field_macro! is a macro that expands to all input tokens if the class declares a Base<T> 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.
-        // Note: this doesn't work with `impl nested::MyClass` style remote impls, which can be enabled via #[hint(signals=true/false)].
 
-        return quote! {
-            #base_field_macro! {
-                #with_signals_impl
+        return match hints.has_typed_signals {
+            // The default case: try to infer from #[derive(GodotClass)] declaration whether a Base<T> field is present.
+            None => {
+                let base_field_macro = util::format_class_base_field_macro(class_name);
+                quote! {
+                    #base_field_macro! {
+                        #with_signals_impl
+                    }
+                }
             }
+
+            // Hints are useful in cases like `impl nested::MyClass` style remote impls. Here, the decl-macro can't work due to being invisible
+            // in other scopes (and crate-global #[macro_export] has its own problems). Thus, generate code directly without decl-macro.
+            Some(true) => quote! { #with_signals_impl },
+            Some(false) => quote! {},
         };
     }
 

godot-macros/src/util/kv_parser.rs

@@ -38,14 +38,16 @@ impl KvParser {
     }
 
     /// Create a new parser which checks for presence of an `#[expected]` attribute.
+    ///
+    /// Returns `Ok(None)` if the attribute is not present.
     pub fn parse(attributes: &[venial::Attribute], expected: &str) -> ParseResult<Option<Self>> {
         let mut found_attr: Option<Self> = None;
 
         for attr in attributes.iter() {
             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,39 @@ 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.
+    pub fn parse_remove(
+        attributes: &mut Vec<venial::Attribute>,
+        expected: &str,
+    ) -> ParseResult<Option<Self>> {
+        let mut found_attr: Option<Self> = None;
+        let mut found_index: Option<usize> = 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
     }

itest/rust/src/builtin_tests/containers/signal_test.rs

@@ -331,28 +331,26 @@ fn signal_symbols_engine_inherited_internal() {
 // 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(ctx: &crate::framework::TestContext) {
-    let mut node = Receiver::new_alloc();
+fn signal_symbols_engine_inherited_no_own_signals() {
+    let mut obj = Receiver::new_alloc();
 
-    // Add to tree, so signals are propagated.
-    ctx.scene_tree.clone().add_child(&node);
-
-    let mut sig = node.signals().renamed();
-    sig.connect_self(|this: &mut Emitter| {
-        this.last_received_int = 887;
+    let mut sig = obj.signals().property_list_changed();
+    sig.connect_self(|this: &mut Receiver| {
+        this.receive_int_mut(941);
     });
 
-    node.set_name("new name");
-
-    assert_eq!(node.bind().last_received_int, 887);
+    obj.notify_property_list_changed();
+    assert_eq!(obj.bind().last_received.get(), LastReceived::Int(941));
 
-    // Remove from tree for other tests.
-    node.free();
+    obj.free();
 }
 
 // Test that trait is implemented even without own #[signal] declarations (to access base signals).
 #[cfg(since_api = "4.2")]
 const fn __type_check<'c>() {
+    use godot::classes::Object;
+    use godot::obj::WithSignals;
+
     // Needs WithUserSignals, not just WithSignals, to allow self.signals().
     const fn has_with_user_signals<T: godot::obj::WithUserSignals>() {}
 
@@ -369,8 +367,8 @@ const fn __type_check<'c>() {
     // Checks whether there is no new collection defined, but instead the base collection is reused.
     // This reduces the amount of proc-macro generated code.
     is_same_type::<
-        Receiver::SignalCollection<'c, Receiver>,
-        Object::SignalCollection<'c, Receiver>, //.
+        <Receiver as WithSignals>::SignalCollection<'c, Receiver>,
+        <Object as WithSignals>::SignalCollection<'c, Receiver>, //.
     >();
 }
 

itest/rust/src/object_tests/object_test.rs

@@ -998,6 +998,7 @@ pub mod object_test_gd {
     use nested::ObjectTest;
 
     #[godot_api]
+    #[hint(typed_signals = false)] // Allows nested::ObjectTest, which would fail otherwise due to generated decl-macro being out-of-scope.
     impl nested::ObjectTest {
         #[func]
         fn pass_object(&self, object: Gd<Object>) -> i64 {