Merge #811

811: Simplify module structure II r=Bromeon a=Bromeon

This PR is a continuation of #788 and addresses the remaining, somewhat ambitious tasks for module cleanup.

Goals:
* Every symbol appears at most once in prelude
* Every symbol appears at exactly once outside prelude
* 2-3 modules (inside the crate) are the maximum nesting depth
* Modules named according to related functionality from user point of view

Changes:

1. `nativescript` module
    * Rename to `export`. Rationale: "native script" is quite a wide term, while most the module's current functionality is directly related to _exporting_ symbols from Rust. In practice, godot-rust is always (?) used as a native script. Other potential use cases, as a pure Godot API library or with native_calls, seem to be the exception, if at all.
    * Along with renaming, the `nativescript` feature is removed.
    * Nested symbols in `export::{properties, methods}` are moved one level up.
    * As a result, we can avoid very long qualifiers and multiple candidates for `use` statements.
       *  `nativescript::init::property::hint::EnumHint` -> `export::hint::EnumHint`
       * `nativescript::export::method::MethodBuilder` -> `export::MethodBuilder`

1. `api` module
    * Remove inner types like `api::area::Area`, as they are already present in `api`.
    * Remove all modules which would then become empty.
    * Create doc links between class (`Camera2D`) and related module (`camera_2d`).

1. Smaller top-level modules
    * Add `init` (previously part of `nativescript`).
    * Add `profiler` (previously part of `nativescript`).
    * Extend `log` with related macros.
    * Remove `macros` and distribute its symbols to the most fitting API.

1. `prelude` module
    * Remove a few macros (`godot_gdnative_init` etc.) from the prelude, suggesting that `godot_init` should be used.
    * `user_data` symbols are accessible through `prelude::user_data` instead of `prelude` directly. This mirrors common usage in examples.

1. `core_types` module
    No changes in this PR; see PR discussion for reasons and potential alternatives.

Co-authored-by: Jan Haller <[email protected]>

Author: bors[bot]

bindings_generator/src/api.rs

@@ -160,6 +160,11 @@ impl GodotClass {
     pub fn is_getter(&self, name: &str) -> bool {
         self.properties.iter().any(|p| p.getter == name)
     }
+
+    /// Whether there is a snake_case module containing related symbols (nested types in C++)
+    pub fn has_related_module(&self) -> bool {
+        !self.enums.is_empty()
+    }
 }
 
 pub type ConstantName = String;
@@ -406,9 +411,8 @@ impl Ty {
                 }
             }
             ty => {
-                let module = format_ident!("{}", module_name_from_class_name(ty));
                 let ty = format_ident!("{}", ty);
-                Ty::Object(syn::parse_quote! { crate::generated::#module::#ty })
+                Ty::Object(syn::parse_quote! { crate::generated::#ty })
             }
         }
     }

bindings_generator/src/classes.rs

@@ -9,17 +9,22 @@ use quote::{format_ident, quote};
 
 use std::collections::HashMap;
 
-pub(crate) fn generate_class_struct(class: &GodotClass) -> TokenStream {
+pub(crate) fn generate_class_struct(class: &GodotClass, class_doc: TokenStream) -> TokenStream {
     let class_name = format_ident!("{}", &class.name);
 
     // dead_code: 'this' might not be read
+    // mod private: hide the type in the #module_name module, export it only in gdnative::api
     quote! {
-        #[allow(non_camel_case_types)]
-        #[derive(Debug)]
-        pub struct #class_name {
-            #[allow(dead_code)]
-            this: RawObject<Self>,
+        pub(crate) mod private {
+            #class_doc
+            #[allow(non_camel_case_types)]
+            #[derive(Debug)]
+            pub struct #class_name {
+                #[allow(dead_code)]
+                pub(crate) this: super::RawObject<Self>,
+            }
         }
+        use private::#class_name;
     }
 }
 

bindings_generator/src/documentation.rs

@@ -12,43 +12,56 @@ pub fn class_doc_link(class: &GodotClass) -> String {
 
 pub fn official_doc_url(class: &GodotClass) -> String {
     format!(
-        "https://godot.readthedocs.io/en/latest/classes/class_{lower_case}.html",
+        "https://godot.readthedocs.io/en/stable/classes/class_{lower_case}.html",
         lower_case = class.name.to_lowercase(),
     )
 }
 
+pub fn generate_module_doc(class: &GodotClass) -> TokenStream {
+    let module_doc = format!(
+        "This module contains types related to the API class [`{m}`][super::{m}].",
+        m = class.name
+    );
+
+    quote! {
+        #![doc=#module_doc]
+    }
+}
+
 pub fn generate_class_documentation(api: &Api, class: &GodotClass) -> TokenStream {
     let has_parent = !class.base_class.is_empty();
     let singleton_str = if class.singleton { "singleton " } else { "" };
-    let ownership_type = if class.is_refcounted() {
-        "reference counted"
+    let memory_type = if class.is_refcounted() {
+        "reference-counted"
     } else {
-        "unsafe"
+        "manually managed"
     };
 
-    let summary_doc = if &class.name == "Reference" {
+    let mut summary_doc = if &class.name == "Reference" {
         "Base class of all reference-counted types. Inherits `Object`.".into()
     } else if &class.name == "Object" {
-        "The base class of most Godot classes.".into()
+        "The base class of all classes in the Godot hierarchy.".into()
     } else if has_parent {
         format!(
-            "`{api_type} {singleton}class {name}` inherits `{base_class}` ({ownership_type}).",
+            "`{api_type} {singleton}class {name}` inherits `{base_class}` ({memory_type}).",
             api_type = class.api_type,
             name = class.name,
             base_class = class.base_class,
-            ownership_type = ownership_type,
-            singleton = singleton_str
+            memory_type = memory_type,
+            singleton = singleton_str,
         )
     } else {
         format!(
-            "`{api_type} {singleton}class {name}` ({ownership_type}).",
+            "`{api_type} {singleton}class {name}` ({memory_type})",
             api_type = class.api_type,
             name = class.name,
-            ownership_type = ownership_type,
+            memory_type = memory_type,
             singleton = singleton_str,
         )
     };
 
+    append_related_module(&mut summary_doc, class);
+
     let official_docs = format!(
         r#"## Official documentation
 
@@ -66,7 +79,7 @@ The lifetime of this object is automatically managed through reference counting.
         format!(
             r#"## Memory management
 
-Non reference counted objects such as the ones of this type are usually owned by the engine.
+Non-reference-counted objects, such as the ones of this type, are usually owned by the engine.
 
 `{name}` is a reference-only type. Persistent references can
 only exist in the unsafe `Ref<{name}>` form.
@@ -113,7 +126,7 @@ This class is used to interact with Godot's editor."#
     let safety_doc = r#"
 ## Safety
 
-All types in the Godot API have "interior mutability" in Rust parlance.
+All types in the Godot API have _interior mutability_ in Rust parlance.
 To enforce that the official [thread-safety guidelines][thread-safety] are
 followed, the typestate pattern is used in the `Ref` and `TRef` smart pointers,
 and the `Instance` API. The typestate `Access` in these types tracks whether the
@@ -145,3 +158,16 @@ fn list_base_classes(output: &mut impl Write, api: &Api, parent_name: &str) -> G
 
     Ok(())
 }
+
+// If present, links to the module with related (C++: nested) types.
+fn append_related_module(string: &mut String, class: &GodotClass) {
+    use std::fmt::Write;
+    if class.has_related_module() {
+        write!(
+            string,
+            "\n\nThis class has related types in the [`{m}`][super::{m}] module.",
+            m = class.module()
+        )
+        .expect("append to string via write!");
+    }
+}

bindings_generator/src/lib.rs

@@ -38,20 +38,20 @@ use std::io;
 
 pub type GeneratorResult<T = ()> = Result<T, io::Error>;
 
-pub struct BindingResult {
-    pub class_bindings: HashMap<String, TokenStream>,
+pub struct BindingResult<'a> {
+    pub class_bindings: Vec<(&'a GodotClass, TokenStream)>,
     pub icalls: TokenStream,
 }
 
-pub fn generate_bindings(api: &Api, docs: Option<&GodotXmlDocs>) -> BindingResult {
+pub fn generate_bindings<'a>(api: &'a Api, docs: Option<&GodotXmlDocs>) -> BindingResult<'a> {
     let mut icalls = HashMap::new();
 
     let class_bindings = api
         .classes
         .iter()
         .map(|class| {
             (
-                class.name.clone(),
+                class,
                 generate_class_bindings(api, class, &mut icalls, docs),
             )
         })
@@ -84,9 +84,9 @@ fn generate_class_bindings(
 ) -> TokenStream {
     // types and methods
     let types_and_methods = {
-        let documentation = generate_class_documentation(api, class);
-
-        let class_struct = generate_class_struct(class);
+        let module_doc = generate_module_doc(class);
+        let class_doc = generate_class_documentation(api, class);
+        let class_struct = generate_class_struct(class, class_doc);
 
         let enums = generate_enums(class);
 
@@ -99,7 +99,7 @@ fn generate_class_bindings(
         let class_impl = generate_class_impl(class, icalls, docs);
 
         quote! {
-            #documentation
+            #module_doc
             #class_struct
             #enums
             #constants
@@ -196,17 +196,23 @@ pub(crate) mod test_prelude {
 
     #[test]
     fn sanity_test_generated_code() {
+        // Tests whether each generated snippet individually constitutes a valid AST representation of Rust code
+
         let api = Api::new(include_str!("../../gdnative-bindings/api.json"));
         let mut buffer = BufWriter::new(Vec::with_capacity(16384));
         for class in &api.classes {
             let mut icalls = HashMap::new();
 
-            let code = generate_class_documentation(&api, &class);
+            let code = generate_module_doc(&class);
+            write!(&mut buffer, "{}", code).unwrap();
+            validate_and_clear_buffer!(buffer);
+
+            let class_doc = generate_class_documentation(&api, &class);
             write!(&mut buffer, "{}", code).unwrap();
-            write!(&mut buffer, "{}", quote! { struct Docs {} }).unwrap();
+            write!(&mut buffer, "{}", quote! { struct StructWithDocs {} }).unwrap();
             validate_and_clear_buffer!(buffer);
 
-            let code = generate_class_struct(&class);
+            let code = generate_class_struct(&class, class_doc);
             write!(&mut buffer, "{}", code).unwrap();
             validate_and_clear_buffer!(buffer);
 

bindings_generator/src/special_methods.rs

@@ -154,11 +154,10 @@ pub fn generate_deref_impl(class: &GodotClass) -> TokenStream {
     );
 
     let class_name = format_ident!("{}", class.name);
-    let base_class_module = format_ident!("{}", class.base_class_module());
     let base_class = format_ident!("{}", class.base_class);
 
     let qualified_base_class = quote! {
-        crate::generated::#base_class_module::#base_class
+        crate::generated::#base_class
     };
 
     quote! {
@@ -190,11 +189,10 @@ pub fn generate_sub_class_impls<'a>(api: &'a Api, mut class: &'a GodotClass) ->
     let mut tokens = TokenStream::new();
 
     while let Some(base_class) = class.base_class(api) {
-        let base_class_module = format_ident!("{}", base_class.module());
         let base_class_ident = format_ident!("{}", base_class.name);
 
         tokens.extend(quote! {
-            unsafe impl SubClass<crate::generated::#base_class_module::#base_class_ident> for #class_name {}
+            unsafe impl SubClass<crate::generated::#base_class_ident> for #class_name {}
         });
 
         class = base_class;

examples/array_export/src/lib.rs

@@ -1,4 +1,4 @@
-use gdnative::nativescript::export::property::hint::{ArrayHint, IntHint, RangeHint};
+use gdnative::export::hint::{ArrayHint, IntHint, RangeHint};
 use gdnative::prelude::*;
 
 #[derive(NativeClass)]

examples/spinning_cube/src/lib.rs

@@ -1,7 +1,7 @@
 use gdnative::api::MeshInstance;
 use gdnative::prelude::*;
 
-use gdnative::nativescript::export::property::{EnumHint, IntHint, StringHint};
+use gdnative::export::hint::{EnumHint, IntHint, StringHint};
 
 #[derive(gdnative::derive::NativeClass)]
 #[inherit(MeshInstance)]

gdnative-async/src/method.rs

@@ -5,10 +5,10 @@ use std::sync::Arc;
 use futures_task::{LocalFutureObj, LocalSpawn, SpawnError};
 
 use gdnative_core::core_types::{ToVariant, Variant};
+use gdnative_core::export::{Method, NativeClass, Varargs};
 use gdnative_core::log::{self, Site};
-use gdnative_core::nativescript::export::{Method, Varargs};
-use gdnative_core::nativescript::{NativeClass, RefInstance};
 use gdnative_core::object::ownership::Shared;
+use gdnative_core::object::RefInstance;
 
 use crate::rt::Context;
 

gdnative-async/src/rt.rs

@@ -1,21 +1,17 @@
 use std::marker::PhantomData;
 
+use func_state::FuncState;
 use gdnative_bindings::Object;
-use gdnative_core::object::SubClass;
-
 use gdnative_core::core_types::{GodotError, Variant};
-use gdnative_core::nativescript::export::InitHandle;
-use gdnative_core::nativescript::{Instance, RefInstance};
+use gdnative_core::init::InitHandle;
 use gdnative_core::object::ownership::Shared;
-use gdnative_core::object::TRef;
+use gdnative_core::object::{Instance, RefInstance, SubClass, TRef};
 
 use crate::future;
 
 mod bridge;
 mod func_state;
 
-use func_state::FuncState;
-
 /// Context for creating `yield`-like futures in async methods.
 pub struct Context {
     func_state: Instance<FuncState, Shared>,

gdnative-async/src/rt/bridge.rs

@@ -5,12 +5,11 @@ use parking_lot::Mutex;
 
 use gdnative_bindings::{Object, Reference};
 use gdnative_core::core_types::{GodotError, Variant, VariantArray};
+use gdnative_core::export::user_data::{ArcData, Map};
+use gdnative_core::export::{ClassBuilder, Method, NativeClass, NativeClassMethods, Varargs};
 use gdnative_core::godot_site;
-use gdnative_core::nativescript::export::method::{Method, Varargs};
-use gdnative_core::nativescript::export::ClassBuilder;
-use gdnative_core::nativescript::user_data::{ArcData, Map};
-use gdnative_core::nativescript::{Instance, NativeClass, NativeClassMethods, RefInstance};
-use gdnative_core::object::{ownership::Shared, TRef};
+use gdnative_core::object::ownership::Shared;
+use gdnative_core::object::{Instance, RefInstance, TRef};
 
 use crate::future::Resume;
 

gdnative-async/src/rt/func_state.rs

@@ -1,15 +1,13 @@
 use gdnative_bindings::Reference;
 use gdnative_core::core_types::{ToVariant, Variant, VariantType};
-use gdnative_core::godot_site;
-use gdnative_core::nativescript::export::method::StaticArgs;
-use gdnative_core::nativescript::export::method::StaticArgsMethod;
-use gdnative_core::nativescript::export::{
-    ClassBuilder, ExportInfo, PropertyUsage, Signal, SignalArgument,
+use gdnative_core::export::user_data::{LocalCellData, Map, MapMut};
+use gdnative_core::export::{
+    ClassBuilder, ExportInfo, NativeClass, NativeClassMethods, PropertyUsage, Signal,
+    SignalArgument, StaticArgs, StaticArgsMethod,
 };
-use gdnative_core::nativescript::user_data::LocalCellData;
-use gdnative_core::nativescript::user_data::{Map, MapMut};
-use gdnative_core::nativescript::{Instance, NativeClass, NativeClassMethods, RefInstance};
+use gdnative_core::godot_site;
 use gdnative_core::object::ownership::{Shared, Unique};
+use gdnative_core::object::{Instance, RefInstance};
 use gdnative_derive::FromVarargs;
 
 use crate::future::Resume;