Clean up signal handling code + user-facing symbols

Also disallow visibility markers in #[signal].

Author: Bromeon

godot-core/src/private.rs

@@ -120,16 +120,6 @@ pub(crate) fn call_error_remove(in_error: &sys::GDExtensionCallError) -> Option<
     call_error
 }
 
-// ----------------------------------------------------------------------------------------------------------------------------------------------
-// Functional and signal APIs
-
-// pub fn emit_signal<T>(obj: &mut BaseMut<T>, varargs: &[Variant])
-// where
-//     T: GodotClass<Declarer = bounds::DeclEngine> + Inherits<crate::classes::Object>,
-// {
-//     obj.upcast_mut().emit_signal(varargs);
-// }
-
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Plugin and global state handling
 

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

@@ -310,7 +310,7 @@ fn process_godot_fns(
                 if function.return_ty.is_some() {
                     return bail!(
                         &function.return_ty,
-                        "return types in #[signal] are not supported"
+                        "#[signal] does not support return types"
                     );
                 }
                 if function.body.is_some() {
@@ -319,6 +319,12 @@ fn process_godot_fns(
                         "#[signal] must not have a body; declare the function with a semicolon"
                     );
                 }
+                if function.vis_marker.is_some() {
+                    return bail!(
+                        &function.vis_marker,
+                        "#[signal] must not have a visibility specifier; signals are always public"
+                    );
+                }
 
                 let external_attributes = function.attributes.clone();
                 let sig = util::reduce_to_signature(function);
@@ -576,7 +582,7 @@ where
                 // Safe unwrap, since #[signal] must be present if we got to this point.
                 let mut parser = KvParser::parse(attributes, "signal")?.unwrap();
 
-                // Private #[__signal(no_builder)]
+                // Private #[signal(__no_builder)]
                 let no_builder = parser.handle_alone("__no_builder")?;
 
                 parser.finish()?;

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

@@ -77,7 +77,7 @@ impl<'a> SignalDetails<'a> {
 
         let param_tuple = quote! { ( #( #param_types, )* ) };
         let signal_name = &original_decl.name;
-        let individual_struct_name = format_ident!("{}_{}", class_name, signal_name);
+        let individual_struct_name = format_ident!("__godot_Signal_{}_{}", class_name, signal_name);
 
         Ok(Self {
             original_decl,
@@ -200,10 +200,11 @@ impl SignalCollection {
         } = details;
 
         self.collection_methods.push(quote! {
+            // Deliberately not #[doc(hidden)] for IDE completion.
             #(#signal_cfg_attrs)*
             fn #signal_name(self) -> #individual_struct_name<'a> {
                 #individual_struct_name {
-                    typed: ::godot::register::TypedSignal::new(self.object, #signal_name_str)
+                    typed: ::godot::register::TypedSignal::new(self.__internal_obj, #signal_name_str)
                 }
             }
         });
@@ -224,23 +225,29 @@ fn make_signal_individual_struct(details: &SignalDetails) -> TokenStream {
         class_name,
         param_names,
         param_tuple,
-        // signal_name,
         signal_cfg_attrs,
         individual_struct_name,
         ..
     } = details;
 
-    // let module_name = format_ident!("__godot_signal_{class_name}_{signal_name}");
-
-    // Module + re-export.
-    // Could also keep contained in module to reduce namespace pollution, but might make docs a bit more nested.
+    // Define the individual types + trait impls. The idea was originally to use a module to reduce namespace pollution:
+    //   let module_name = format_ident!("__godot_signal_{class_name}_{signal_name}");
+    //   #(#signal_cfg_attrs)* pub mod #module_name { use super::*; ... }
+    //   #(#signal_cfg_attrs)* pub(crate) use #module_name::#individual_struct_name;
+    // However, there are some challenges:
+    // - Visibility becomes a pain to handle (rustc doesn't like re-exporting private symbols as pub, and we can't know the visibility of the
+    //   surrounding class struct). Having signals always-public is much less of a headache, requires less choice on the user side
+    //   (pub/pub(crate)/nothing on #[signal]), and likely good enough for the moment.
+    // - Not yet clear if we should have each signal + related types in separate module. If #[signal] is supported in #[godot_api(secondary)]
+    //   impl blocks, then we would have to group them by the impl block. Rust doesn't allow partial modules, so they'd need to have individual
+    //   names as well, possibly explicitly chosen by the user.
+    //
+    // For now, #[doc(hidden)] is used in some places to limit namespace pollution at least in IDEs + docs. This also means that the generated
+    // code is less observable by the user. If someone comes up with a good idea to handle all this, let us know :)
     quote! {
-        // #(#signal_cfg_attrs)*
-        // mod #module_name {
-
-        // TODO make pub without running into "private type `MySignal` in public interface" errors.
         #(#signal_cfg_attrs)*
         #[allow(non_camel_case_types)]
+        #[doc(hidden)] // Signal struct is hidden, but the method returning it is not (IDE completion).
         struct #individual_struct_name<'a> {
             #[doc(hidden)]
             typed: ::godot::register::TypedSignal<'a, #class_name, #param_tuple>,
@@ -269,27 +276,26 @@ fn make_signal_individual_struct(details: &SignalDetails) -> TokenStream {
                 &mut self.typed
             }
         }
-
-        // #(#signal_cfg_attrs)*
-        // pub(crate) use #module_name::#individual_struct_name;
     }
 }
 
-// See also make_func_collection().
+/// Generates a unspecified-name struct holding methods to access each signal.
 fn make_signal_collection(class_name: &Ident, collection: SignalCollection) -> Option<TokenStream> {
     if collection.is_empty() {
         return None;
     }
 
-    let collection_struct_name = format_ident!("{}Signals", class_name);
+    let collection_struct_name = format_ident!("__godot_Signals_{}", class_name);
     let collection_struct_methods = &collection.collection_methods;
     let individual_structs = collection.individual_structs;
 
     let code = quote! {
         #[allow(non_camel_case_types)]
+        #[doc(hidden)] // Only on struct, not methods, to allow completion in IDEs.
         pub struct #collection_struct_name<'a> {
             // To allow external call in the future (given Gd<T>, not self), this could be an enum with either BaseMut or &mut Gd<T>/&mut T.
-            object: ::godot::register::ObjectRef<'a, #class_name>
+            #[doc(hidden)] // Necessary because it's in the same scope as the user-defined class, so appearing in IDE completion.
+            __internal_obj: ::godot::register::ObjectRef<'a, #class_name>
         }
 
         impl<'a> #collection_struct_name<'a> {
@@ -301,14 +307,14 @@ fn make_signal_collection(class_name: &Ident, collection: SignalCollection) -> O
 
             fn signals(&mut self) -> Self::SignalCollection<'_> {
                 Self::SignalCollection {
-                    object: ::godot::register::ObjectRef::Internal { obj_mut: self }
+                    __internal_obj: ::godot::register::ObjectRef::Internal { obj_mut: self }
                 }
             }
 
             #[doc(hidden)]
             fn __signals_from_external(external: &Gd<Self>) -> Self::SignalCollection<'_> {
                 Self::SignalCollection {
-                    object: ::godot::register::ObjectRef::External { gd: external.clone() }
+                    __internal_obj: ::godot::register::ObjectRef::External { gd: external.clone() }
                 }
             }
         }

godot-macros/src/class/derive_godot_class.rs

@@ -329,12 +329,14 @@ fn make_user_class_impl(
 
     let user_class_impl = quote! {
         impl ::godot::obj::UserClass for #class_name {
+            #[doc(hidden)]
             fn __config() -> ::godot::private::ClassConfig {
                 ::godot::private::ClassConfig {
                     is_tool: #is_tool,
                 }
             }
 
+            #[doc(hidden)]
             fn __before_ready(&mut self) {
                 #rpc_registrations
                 #onready_inits

godot-macros/src/util/mod.rs

@@ -360,12 +360,8 @@ pub fn make_funcs_collection_constant(
         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;
@@ -409,5 +405,5 @@ pub fn format_funcs_collection_constant(_class_name: &Ident, func_name: &Ident)
 
 /// 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")
+    format_ident!("__godot_{class_name}_Funcs")
 }