Merge pull request #500 from godot-rust/qol/useless-impl

Empty `#[godot_api] impl` blocks are no longer needed

Author: Bromeon

godot-core/src/lib.rs

@@ -45,11 +45,6 @@ mod gen;
 
 #[doc(hidden)]
 pub mod private {
-    // If someone forgets #[godot_api], this causes a compile error, rather than virtual functions not being called at runtime.
-    #[allow(non_camel_case_types)]
-    pub trait You_forgot_the_attribute__godot_api {}
-    pub use crate::property::Cannot_export_without_godot_api_impl;
-
     use std::sync::{Arc, Mutex};
 
     pub use crate::gen::classes::class_macros;
@@ -59,6 +54,10 @@ pub mod private {
 
     use crate::{log, sys};
 
+    // If someone forgets #[godot_api], this causes a compile error, rather than virtual functions not being called at runtime.
+    #[allow(non_camel_case_types)]
+    pub trait You_forgot_the_attribute__godot_api {}
+
     sys::plugin_registry!(pub __GODOT_PLUGIN_REGISTRY: ClassPlugin);
 
     pub(crate) fn iterate_plugins(mut visitor: impl FnMut(&ClassPlugin)) {

godot-core/src/obj/traits.rs

@@ -38,7 +38,7 @@ where
     /// During which initialization level this class is available/should be initialized with Godot.
     ///
     /// Is `None` if the class has complicated initialization requirements, and generally cannot be inherited
-    /// from.
+    /// from (currently only for `()`, the "base" of `Object`).
     const INIT_LEVEL: Option<InitLevel>;
 
     /// The name of the class, under which it is registered in Godot.

godot-core/src/property.rs

@@ -106,23 +106,6 @@ impl PropertyHintInfo {
     }
 }
 
-/// To export properties to Godot, you must have an impl-block with the `#[godot_api]` attribute, even if
-/// it is empty.
-///
-/// This trait is automatically implemented when such an impl-block is present. If Rust complains that it is
-/// not implemented, then you can usually fix this by adding:
-///
-/// ```ignore
-/// #[godot_api]
-/// impl MyClass {}
-/// ```
-///
-/// Where you replace `MyClass` with the name of your class.
-#[allow(non_camel_case_types)]
-pub trait Cannot_export_without_godot_api_impl {
-    const EXISTS: () = ();
-}
-
 /// Functions used to translate user-provided arguments into export hints.
 pub mod export_info_functions {
     use crate::builtin::GString;

godot-core/src/registry.rs

@@ -39,6 +39,9 @@ static LOADED_CLASSES: Mutex<Option<HashMap<InitLevel, Vec<ClassName>>>> = Mutex
 pub struct ClassPlugin {
     pub class_name: ClassName,
     pub component: PluginComponent,
+
+    // Init-level is per ClassPlugin and not per PluginComponent, because all components of all classes are mixed together in one
+    // huge linker list. There is no per-class aggregation going on, so this allows to easily filter relevant classes.
     pub init_level: Option<InitLevel>,
 }
 
@@ -60,11 +63,11 @@ impl fmt::Debug for ErasedRegisterFn {
 /// Represents the data part of a [`ClassPlugin`] instance.
 #[derive(Clone, Debug)]
 pub enum PluginComponent {
-    /// Class definition itself, must always be available
+    /// Class definition itself, must always be available.
     ClassDef {
         base_class_name: ClassName,
 
-        /// Godot low-level`create` function, wired up to library-generated `init`
+        /// Godot low-level `create` function, wired up to library-generated `init`.
         generated_create_fn: Option<
             unsafe extern "C" fn(
                 _class_userdata: *mut std::ffi::c_void, //
@@ -78,6 +81,9 @@ pub enum PluginComponent {
             ) -> sys::GDExtensionClassInstancePtr,
         >,
 
+        /// Callback to library-generated function which registers properties in the `struct` definition.
+        register_properties_fn: ErasedRegisterFn,
+
         free_fn: unsafe extern "C" fn(
             _class_user_data: *mut std::ffi::c_void,
             instance: sys::GDExtensionClassInstancePtr,
@@ -86,18 +92,18 @@ pub enum PluginComponent {
 
     /// Collected from `#[godot_api] impl MyClass`
     UserMethodBinds {
-        /// Callback to library-generated function which registers functions in the `impl`
+        /// Callback to library-generated function which registers functions and constants in the `impl` block.
         ///
         /// Always present since that's the entire point of this `impl` block.
-        generated_register_fn: ErasedRegisterFn,
+        register_methods_constants_fn: ErasedRegisterFn,
     },
 
     /// Collected from `#[godot_api] impl GodotExt for MyClass`
     UserVirtuals {
-        /// Callback to user-defined `register_class` function
+        /// Callback to user-defined `register_class` function.
         user_register_fn: Option<ErasedRegisterFn>,
 
-        /// Godot low-level`create` function, wired up to the user's `init`
+        /// Godot low-level `create` function, wired up to the user's `init`.
         user_create_fn: Option<
             unsafe extern "C" fn(
                 _class_userdata: *mut std::ffi::c_void, //
@@ -111,7 +117,7 @@ pub enum PluginComponent {
             ) -> sys::GDExtensionClassInstancePtr,
         >,
 
-        /// User-defined `to_string` function
+        /// User-defined `to_string` function.
         user_to_string_fn: Option<
             unsafe extern "C" fn(
                 p_instance: sys::GDExtensionClassInstancePtr,
@@ -120,7 +126,7 @@ pub enum PluginComponent {
             ),
         >,
 
-        /// User-defined `on_notification` function
+        /// User-defined `on_notification` function.
         #[cfg(before_api = "4.2")]
         user_on_notification_fn: Option<
             unsafe extern "C" fn(
@@ -137,7 +143,7 @@ pub enum PluginComponent {
             ),
         >,
 
-        /// Callback for other virtuals
+        /// Callback for other virtuals.
         get_virtual_fn: unsafe extern "C" fn(
             p_userdata: *mut std::os::raw::c_void,
             p_name: sys::GDExtensionConstStringNamePtr,
@@ -154,8 +160,12 @@ pub enum PluginComponent {
 struct ClassRegistrationInfo {
     class_name: ClassName,
     parent_class_name: Option<ClassName>,
-    generated_register_fn: Option<ErasedRegisterFn>,
+    // Following functions are stored separately, since their order matters.
+    register_methods_constants_fn: Option<ErasedRegisterFn>,
+    register_properties_fn: Option<ErasedRegisterFn>,
     user_register_fn: Option<ErasedRegisterFn>,
+
+    /// Godot low-level class creation parameters.
     #[cfg(before_api = "4.2")]
     godot_params: sys::GDExtensionClassCreationInfo,
     #[cfg(since_api = "4.2")]
@@ -208,7 +218,8 @@ pub fn register_class<
     register_class_raw(ClassRegistrationInfo {
         class_name: T::class_name(),
         parent_class_name: Some(T::Base::class_name()),
-        generated_register_fn: None,
+        register_methods_constants_fn: None,
+        register_properties_fn: None,
         user_register_fn: Some(ErasedRegisterFn {
             raw: callbacks::register_class_by_builder::<T>,
         }),
@@ -232,13 +243,15 @@ pub fn auto_register_classes(init_level: InitLevel) {
 
     crate::private::iterate_plugins(|elem: &ClassPlugin| {
         //out!("* Plugin: {elem:#?}");
+
+        // Filter per ClassPlugin and not PluginComponent, because all components of all classes are mixed together in one huge list.
         match elem.init_level {
             None => {
                 log::godot_error!("Unknown initialization level for class {}", elem.class_name);
                 return;
             }
             Some(elem_init_level) if elem_init_level != init_level => return,
-            _ => (),
+            _ => { /* Nothing */ }
         }
 
         let name = elem.class_name;
@@ -304,6 +317,7 @@ fn fill_class_info(component: PluginComponent, c: &mut ClassRegistrationInfo) {
             base_class_name,
             generated_create_fn,
             generated_recreate_fn,
+            register_properties_fn,
             free_fn,
         } => {
             c.parent_class_name = Some(base_class_name);
@@ -335,12 +349,13 @@ fn fill_class_info(component: PluginComponent, c: &mut ClassRegistrationInfo) {
             assert!(generated_recreate_fn.is_none()); // not used
 
             c.godot_params.free_instance_func = Some(free_fn);
+            c.register_properties_fn = Some(register_properties_fn);
         }
 
         PluginComponent::UserMethodBinds {
-            generated_register_fn,
+            register_methods_constants_fn,
         } => {
-            c.generated_register_fn = Some(generated_register_fn);
+            c.register_methods_constants_fn = Some(register_methods_constants_fn);
         }
 
         PluginComponent::UserVirtuals {
@@ -433,11 +448,18 @@ fn register_class_raw(info: ClassRegistrationInfo) {
     //let mut class_builder = crate::builder::ClassBuilder::<?>::new();
     let mut class_builder = 0; // TODO dummy argument; see callbacks
 
-    // First call generated (proc-macro) registration function, then user-defined one.
-    // This mimics the intuition that proc-macros are running "before" normal runtime code.
-    if let Some(register_fn) = info.generated_register_fn {
+    // Order of the following registrations is crucial:
+    // 1. Methods and constants.
+    // 2. Properties (they may depend on get/set methods).
+    // 3. User-defined registration function (intuitively, user expects their own code to run after proc-macro generated code).
+    if let Some(register_fn) = info.register_methods_constants_fn {
+        (register_fn.raw)(&mut class_builder);
+    }
+
+    if let Some(register_fn) = info.register_properties_fn {
         (register_fn.raw)(&mut class_builder);
     }
+
     if let Some(register_fn) = info.user_register_fn {
         (register_fn.raw)(&mut class_builder);
     }
@@ -639,7 +661,11 @@ pub mod callbacks {
         T::__godot_register_class(&mut class_builder);
     }
 
-    pub fn register_user_binds<T: cap::ImplementsGodotApi + cap::ImplementsGodotExports>(
+    pub fn register_user_properties<T: cap::ImplementsGodotExports>(_class_builder: &mut dyn Any) {
+        T::__register_exports();
+    }
+
+    pub fn register_user_methods_constants<T: cap::ImplementsGodotApi>(
         _class_builder: &mut dyn Any,
     ) {
         // let class_builder = class_builder
@@ -649,7 +675,6 @@ pub mod callbacks {
         //T::register_methods(class_builder);
         T::__register_methods();
         T::__register_constants();
-        T::__register_exports();
     }
 }
 
@@ -662,7 +687,8 @@ fn default_registration_info(class_name: ClassName) -> ClassRegistrationInfo {
     ClassRegistrationInfo {
         class_name,
         parent_class_name: None,
-        generated_register_fn: None,
+        register_methods_constants_fn: None,
+        register_properties_fn: None,
         user_register_fn: None,
         godot_params: default_creation_info(),
         init_level: InitLevel::Scene,

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

@@ -203,18 +203,8 @@ pub fn make_property_impl(class_name: &Ident, fields: &Fields) -> TokenStream {
         });
     }
 
-    let enforce_godot_api_impl = if !export_tokens.is_empty() {
-        quote! {
-            const MUST_HAVE_GODOT_API_IMPL: () = <#class_name as ::godot::private::Cannot_export_without_godot_api_impl>::EXISTS;
-        }
-    } else {
-        TokenStream::new()
-    };
-
     quote! {
         impl #class_name {
-            #enforce_godot_api_impl
-
             #(#getter_setter_impls)*
         }
 

godot-macros/src/class/derive_godot_class.rs

@@ -103,6 +103,9 @@ pub fn derive_godot_class(decl: Declaration) -> ParseResult<TokenStream> {
                 base_class_name: #base_class_name_obj,
                 generated_create_fn: #create_fn,
                 generated_recreate_fn: #recreate_fn,
+                register_properties_fn: #prv::ErasedRegisterFn {
+                    raw: #prv::callbacks::register_user_properties::<#class_name>,
+                },
                 free_fn: #prv::callbacks::free::<#class_name>,
             },
             init_level: <#class_name as ::godot::obj::GodotClass>::INIT_LEVEL,

godot-macros/src/class/godot_api.rs

@@ -77,10 +77,10 @@ struct SignalDefinition {
 }
 
 /// Codegen for `#[godot_api] impl MyType`
-fn transform_inherent_impl(mut decl: Impl) -> Result<TokenStream, Error> {
-    let class_name = util::validate_impl(&decl, None, "godot_api")?;
+fn transform_inherent_impl(mut original_impl: Impl) -> Result<TokenStream, Error> {
+    let class_name = util::validate_impl(&original_impl, None, "godot_api")?;
     let class_name_obj = util::class_name_obj(&class_name);
-    let (funcs, signals) = process_godot_fns(&mut decl)?;
+    let (funcs, signals) = process_godot_fns(&mut original_impl)?;
 
     let mut signal_cfg_attrs: Vec<Vec<&Attribute>> = Vec::new();
     let mut signal_name_strs: Vec<String> = Vec::new();
@@ -135,7 +135,7 @@ fn transform_inherent_impl(mut decl: Impl) -> Result<TokenStream, Error> {
         .into_iter()
         .map(|func_def| make_method_registration(&class_name, func_def));
 
-    let consts = process_godot_constants(&mut decl)?;
+    let consts = process_godot_constants(&mut original_impl)?;
     let mut integer_constant_cfg_attrs = Vec::new();
     let mut integer_constant_names = Vec::new();
     let mut integer_constant_values = Vec::new();
@@ -184,7 +184,7 @@ fn transform_inherent_impl(mut decl: Impl) -> Result<TokenStream, Error> {
     };
 
     let result = quote! {
-        #decl
+        #original_impl
 
         impl ::godot::obj::cap::ImplementsGodotApi for #class_name {
             fn __register_methods() {
@@ -222,13 +222,11 @@ fn transform_inherent_impl(mut decl: Impl) -> Result<TokenStream, Error> {
             }
         }
 
-        impl ::godot::private::Cannot_export_without_godot_api_impl for #class_name {}
-
         ::godot::sys::plugin_add!(__GODOT_PLUGIN_REGISTRY in #prv; #prv::ClassPlugin {
             class_name: #class_name_obj,
             component: #prv::PluginComponent::UserMethodBinds {
-                generated_register_fn: #prv::ErasedRegisterFn {
-                    raw: #prv::callbacks::register_user_binds::<#class_name>,
+                register_methods_constants_fn: #prv::ErasedRegisterFn {
+                    raw: #prv::callbacks::register_user_methods_constants::<#class_name>,
                 },
             },
             init_level: <#class_name as ::godot::obj::GodotClass>::INIT_LEVEL,