Merge pull request #497 from godot-rust/qol/docs-and-utils

Bromeon · web-flow · commit d3745f8bfa84 · 2023-11-26T10:47:33.000Z
`to_gd()`, `Gd::try_cast()` result, formatting
diff --git a/.github/bors.toml b/.github/bors.toml
diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml
@@ -8,10 +8,7 @@ name: Full CI
 
 on:
   merge_group:
-  push:
-    branches:
-      - staging
-      - trying
+#  push:
 
 env:
   GDEXT_FEATURES: ''
@@ -328,9 +325,9 @@ jobs:
   # CI status report
 
   # Job to notify merge queue about success/failure
-  # 'push' is for workflows still triggered by bors
   ci-status:
-    if: always() && (github.event_name == 'merge_group' || github.event_name == 'push')
+    # Check for 'merge_group' not strictly necessary, but helpful when adding add-hoc `push:` trigger to `on:` for testing branch.
+    if: always() && github.event_name == 'merge_group'
     needs:
       - rustfmt
       - doc-lints
diff --git a/check.sh b/check.sh
@@ -125,7 +125,7 @@ function cmd_fmt() {
 }
 
 function cmd_clippy() {
-    run cargo clippy "${extraCargoArgs[@]}" -- \
+    run cargo clippy --all-targets "${extraCargoArgs[@]}" -- \
         -D clippy::suspicious \
         -D clippy::style \
         -D clippy::complexity \
diff --git a/examples/dodge-the-creeps/rust/src/main_scene.rs b/examples/dodge-the-creeps/rust/src/main_scene.rs
@@ -105,10 +105,7 @@ impl Main {
         mob.set_linear_velocity(lin_vel);
 
         let mut hud = self.base.get_node_as::<Hud>("Hud");
-        hud.connect(
-            "start_game".into(),
-            Callable::from_object_method(mob, "on_start_game"),
-        );
+        hud.connect("start_game".into(), mob.callable("on_start_game"));
     }
 
     fn music(&mut self) -> &mut AudioStreamPlayer {
diff --git a/godot-bindings/src/lib.rs b/godot-bindings/src/lib.rs
@@ -19,7 +19,7 @@ compile_error!(
 
 // This is outside of `godot_version` to allow us to use it even when we don't have the `custom-godot`
 // feature enabled.
-#[derive(Debug, Eq, PartialEq)]
+#[derive(Eq, PartialEq, Debug)]
 pub struct GodotVersion {
     /// the original string (trimmed, stripped of text around)
     pub full_string: String,
diff --git a/godot-codegen/src/util.rs b/godot-codegen/src/util.rs
@@ -15,7 +15,7 @@ use quote::{format_ident, quote, ToTokens};
 
 use std::fmt;
 
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, Eq, PartialEq, Debug)]
 pub struct NativeStructuresField {
     pub field_type: String,
     pub field_name: String,
diff --git a/godot-core/Cargo.toml b/godot-core/Cargo.toml
@@ -31,7 +31,6 @@ serde = { version = "1", features = ["derive"], optional = true }
 # Reverse dev dependencies so doctests can use `godot::` prefix
 [dev-dependencies]
 godot = { path = "../godot" }
-serde_json = "1.0"
 
 [build-dependencies]
 godot-bindings = { path = "../godot-bindings" }
diff --git a/godot-core/src/builtin/array.rs b/godot-core/src/builtin/array.rs
@@ -1022,7 +1022,7 @@ macro_rules! varray {
 /// [`set_typed`](https://docs.godotengine.org/en/latest/classes/class_array.html#class-array-method-set-typed).
 ///
 /// We ignore the `script` parameter because it has no impact on typing in Godot.
-#[derive(PartialEq, Eq)]
+#[derive(Eq, PartialEq)]
 pub(crate) struct TypeInfo {
     variant_type: VariantType,
 
diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs
@@ -35,8 +35,10 @@ impl Callable {
 
     /// Create a callable for the method `object::method_name`.
     ///
+    /// See also [`Gd::callable()`].
+    ///
     /// _Godot equivalent: `Callable(Object object, StringName method)`_
-    pub fn from_object_method<T, S>(object: Gd<T>, method_name: S) -> Self
+    pub fn from_object_method<T, S>(object: &Gd<T>, method_name: S) -> Self
     where
         T: GodotClass, // + Inherits<Object>,
         S: Into<StringName>,
diff --git a/godot-core/src/builtin/meta/godot_convert/convert_error.rs b/godot-core/src/builtin/meta/godot_convert/convert_error.rs
@@ -108,7 +108,7 @@ impl Error for ConvertError {
     }
 }
 
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Eq, PartialEq, Debug)]
 pub(crate) enum ErrorKind {
     FromGodot(FromGodotError),
     FromFfi(FromFfiError),
diff --git a/godot-core/src/builtin/vectors/vector_axis.rs b/godot-core/src/builtin/vectors/vector_axis.rs
@@ -73,7 +73,7 @@ pub trait ToVector: Sized {
 }
 
 /// Enumerates the axes in a [`Vector2`].
-#[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
+#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug)]
 #[repr(i32)]
 pub enum Vector2Axis {
     /// The X axis.
@@ -117,7 +117,7 @@ impl FromGodot for Vector2Axis {
 
 /// Enumerates the axes in a [`Vector3`].
 // TODO auto-generate this, alongside all the other builtin type's enums
-#[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
+#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug)]
 #[repr(i32)]
 pub enum Vector3Axis {
     /// The X axis.
@@ -164,7 +164,7 @@ impl FromGodot for Vector3Axis {
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 
 /// Enumerates the axes in a [`Vector4`].
-#[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
+#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug)]
 #[repr(i32)]
 pub enum Vector4Axis {
     /// The X axis.
diff --git a/godot-core/src/engine/mod.rs b/godot-core/src/engine/mod.rs
@@ -59,7 +59,7 @@ impl PackedSceneExt for PackedScene {
     where
         T: Inherits<Node>,
     {
-        self.instantiate().and_then(|gd| gd.try_cast::<T>())
+        self.instantiate().and_then(|gd| gd.try_cast::<T>().ok())
     }
 }
 
@@ -102,7 +102,7 @@ impl NodeExt for Node {
 
         // TODO differentiate errors (not found, bad type) with Result
         self.get_node_or_null(path)
-            .and_then(|node| node.try_cast::<T>())
+            .and_then(|node| node.try_cast::<T>().ok())
     }
 }
 
@@ -252,5 +252,5 @@ where
         .load_ex(path.clone())
         .type_hint(T::class_name().to_godot_string())
         .done() // TODO unclone
-        .and_then(|res| res.try_cast::<T>())
+        .and_then(|res| res.try_cast::<T>().ok())
 }
diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs
@@ -308,12 +308,12 @@ impl<T: GodotClass> Gd<T> {
     /// If `T`'s dynamic type is not `Derived` or one of its subclasses, `None` is returned
     /// and the reference is dropped. Otherwise, `Some` is returned and the ownership is moved
     /// to the returned value.
-    // TODO consider Result<Gd<Derived>, Self> so that user can still use original object (e.g. to free if manual)
-    pub fn try_cast<Derived>(self) -> Option<Gd<Derived>>
+    pub fn try_cast<Derived>(self) -> Result<Gd<Derived>, Self>
     where
         Derived: GodotClass + Inherits<T>,
     {
-        self.owned_cast().ok()
+        // Separate method due to more restrictive bounds.
+        self.owned_cast()
     }
 
     /// ⚠️ **Downcast:** convert into a smart pointer to a derived class. Panics on error.
@@ -385,9 +385,12 @@ impl<T: GodotClass> Gd<T> {
     pub(crate) fn obj_sys(&self) -> sys::GDExtensionObjectPtr {
         self.raw.obj_sys()
     }
+
     /// Returns a callable referencing a method from this object named `method_name`.
+    ///
+    /// This is shorter syntax for [`Callable::from_object_method(self, method_name)`][Callable::from_object_method].
     pub fn callable<S: Into<StringName>>(&self, method_name: S) -> Callable {
-        Callable::from_object_method(self.clone(), method_name)
+        Callable::from_object_method(self, method_name)
     }
 }
 
diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs
@@ -206,6 +206,18 @@ pub trait IndexEnum: EngineEnum {
     }
 }
 
+/// Trait that's implemented for user-defined classes that provide a `#[base]` field.
+///
+/// Gives direct access to the containing `Gd<Self>` from `Self`.
+// Possible alternative for builder APIs, although even less ergonomic: Base<T> could be Base<T, Self> and return Gd<Self>.
+pub trait WithBaseField: GodotClass {
+    /// Returns the `Gd` pointer containing this object.
+    ///
+    /// This is intended to be stored or passed to engine methods. You cannot call `bind()` or `bind_mut()` on it, while the method
+    /// calling `to_gd()` is still running; that would lead to a double borrow panic.
+    fn to_gd(&self) -> Gd<Self>;
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 
 /// Capability traits, providing dedicated functionalities for Godot classes
@@ -257,17 +269,6 @@ pub mod cap {
         }
     }
 
-    /// Trait that's implemented for user-defined classes that provide a `#[base]` field.
-    ///
-    /// Gives direct access to the base pointer without going through upcast FFI.
-    pub trait WithBaseField: GodotClass {
-        #[doc(hidden)]
-        fn __godot_base(&self) -> &Gd<Self::Base>;
-
-        #[doc(hidden)]
-        fn __godot_base_mut(&mut self) -> &mut Gd<Self::Base>;
-    }
-
     // TODO Evaluate whether we want this public or not
     #[doc(hidden)]
     pub trait GodotToString: GodotClass {
diff --git a/godot-fmt/Cargo.toml b/godot-fmt/Cargo.toml
@@ -8,9 +8,13 @@ edition = "2021"
 [dependencies]
 proc-macro2 = "1"
 
-[dev-dependencies]
-criterion = "0.3"
 
-[[bench]]
-name = "gdext_bench"
-harness = false
+# Disabled below, since it pulls in too many dependencies during `cargo test` but is not really used.
+# Dev-dependencies cannot be optional and feature-gated. Enable manually when needed.
+
+#[[bench]]
+#name = "gdext_bench"
+#harness = false
+#
+#[dev-dependencies]
+#criterion = "0.5"
diff --git a/godot-fmt/benches/gdext_bench.rs b/godot-fmt/benches/gdext_bench.rs
@@ -4,6 +4,10 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
+// IMPORTANT: to enable this benchmark, uncomment the corresponding lines in Cargo.toml.
+// First tried with #![allow(clippy::all)], but clippy still tries to compile the code and fails on imports.
+#![cfg(FALSE)]
+
 use std::{path::PathBuf, str::FromStr};
 
 use criterion::{black_box, criterion_group, criterion_main, Criterion};
diff --git a/godot-fmt/src/lib.rs b/godot-fmt/src/lib.rs
@@ -88,7 +88,7 @@ fn indent(n: usize) -> &'static str {
 /// State that is kept between processing `TokenTree`s, used to decide
 /// how to insert whitespace.
 
-#[derive(Clone, Copy, PartialEq, Eq)]
+#[derive(Copy, Clone, Eq, PartialEq)]
 enum FormatState {
     /// Starting state, meaning no whitespace is needed
     Start,
diff --git a/godot-macros/src/class/derive_godot_class.rs b/godot-macros/src/class/derive_godot_class.rs
@@ -50,6 +50,18 @@ pub fn derive_godot_class(decl: Declaration) -> ParseResult<TokenStream> {
         quote! {}
     };
 
+    let godot_withbase_impl = if let Some(Field { name, .. }) = &fields.base_field {
+        quote! {
+            impl ::godot::obj::WithBaseField for #class_name {
+                fn to_gd(&self) -> Gd<Self> {
+                    ::godot::obj::Gd::clone(&*self.#name).cast()
+                }
+            }
+        }
+    } else {
+        quote! {}
+    };
+
     let (godot_init_impl, create_fn, recreate_fn);
     if struct_cfg.has_generated_init {
         godot_init_impl = make_godot_init_impl(class_name, fields);
@@ -78,11 +90,10 @@ pub fn derive_godot_class(decl: Declaration) -> ParseResult<TokenStream> {
                 ::godot::builtin::meta::ClassName::from_ascii_cstr(#class_name_cstr)
             }
         }
-        impl ::godot::obj::UserClass for #class_name {
-
-        }
+        impl ::godot::obj::UserClass for #class_name {}
 
         #godot_init_impl
+        #godot_withbase_impl
         #godot_exports_impl
         #config_impl
 
diff --git a/godot-macros/src/derive/derive_from_variant.rs b/godot-macros/src/derive/derive_from_variant.rs
@@ -261,8 +261,8 @@ fn make_enum_tuple(
         } else {
             quote! {
                 let #ident = variant.pop_front()
-                                    .ok_or(ConvertError::with_cause_value("missing expected value", variant.clone()))?
-                                    .try_to::<#field_type>()?;
+                    .ok_or(ConvertError::with_cause_value("missing expected value", variant.clone()))?
+                    .try_to::<#field_type>()?;
             }
         };
         (ident.to_token_stream(), set_ident)
diff --git a/godot-macros/src/lib.rs b/godot-macros/src/lib.rs
@@ -100,7 +100,7 @@ use crate::util::ident;
 /// #[class(base = Node2D)]
 /// struct MyStruct {
 ///     #[base]
-///     base: Gd<Node2D>,
+///     base: Base<Node2D>,
 /// }
 /// ```
 ///
@@ -529,7 +529,7 @@ pub fn derive_from_godot(input: TokenStream) -> TokenStream {
 /// # use godot::prelude::*;
 /// #[repr(i32)]
 /// #[derive(Property)]
-/// # #[derive(PartialEq, Eq, Debug)]
+/// # #[derive(Eq, PartialEq, Debug)]
 /// enum TestEnum {
 ///     A = 0,
 ///     B = 1,
diff --git a/godot/src/lib.rs b/godot/src/lib.rs
@@ -106,11 +106,6 @@
 //!
 //! The following features can be enabled for this crate. All off them are off by default.
 //!
-//! * **`formatted`**
-//!
-//!   Format the generated bindings with `rustfmt`. This significantly increases initial compile times and is
-//!   mostly useful when you actively contribute to the library and/or want to inspect generated files.<br><br>
-//!
 //! * **`double-precision`**
 //!
 //!   Use `f64` instead of `f32` for the floating-point type [`real`][type@builtin::real]. Requires Godot to be compiled with the
@@ -131,32 +126,38 @@
 //!
 //! * **`serde`**
 //!
-//!   Implement the [serde](https://docs.rs/serde) traits `Serialize` and `Deserialize` traits for certain built-in types.
+//!   Implement the [serde](https://serde.rs/) traits `Serialize` and `Deserialize` traits for certain built-in types.
 //!   The serialized representation underlies **no stability guarantees** and may change at any time, even without a SemVer-breaking change.
 //!   <br><br>
 //!
+//! * **`lazy-function-tables`**
+//!
+//!   Instead of loading all engine function pointers at startup, load them lazily on first use. This reduces startup time and RAM usage, but
+//!   incurs additional overhead in each FFI call. Also, you lose the guarantee that once the library has booted, all function pointers are
+//!   truly available. Function calls may thus panic only at runtime, possibly in deeply nested code paths.
+//!   This feature is not yet thread-safe and can thus not be combined with `experimental-threads`.<br><br>
+//!
+//! * **`formatted`**
+//!
+//!   Format the generated binding code with a custom-built formatter, which aims to strike a balance between runtime and human readability.
+//!   rustfmt generates nice output, but it is unfortunately excessively slow across hundreds of Godot classes.<br><br>
+//!
 //! * **`experimental-threads`**
 //!
 //!   Experimental threading support. This enables `Send`/`Sync` traits for `Gd<T>` and makes the guard types `Gd`/`GdMut` aware of
 //!   multi-threaded references. There safety aspects are not ironed out yet; there is a high risk of unsoundness at the moment.
-//!   As this evolves, it is very likely that the API becomes more strict.
+//!   As this evolves, it is very likely that the API becomes more strict.<br><br>
 //!
 //! * **`experimental-godot-api`**
 //!
 //!   Access to `godot::engine` APIs that Godot marks "experimental". These are under heavy development and may change at any time.
-//!   If you opt in to this feature, expect breaking changes at compile and runtime.
+//!   If you opt in to this feature, expect breaking changes at compile and runtime.<br><br>
 //!
 //! * **`experimental-wasm`**
 //!
 //!   Support for WebAssembly exports is still a work-in-progress and is not yet well tested. This feature is in place for users
-//!   to explicitly opt-in to any instabilities or rough edges that may result.
-//!
-//! * **`lazy-function-tables`**
-//!
-//!   Instead of loading all engine function pointers at startup, load them lazily on first use. This reduces startup time and RAM usage, but
-//!   incurs additional overhead in each FFI call. Also, you lose the guarantee that once the library has booted, all function pointers are
-//!   truly available. Function calls may thus panic only at runtime, possibly in deeply nested code paths.
-//!   This feature is not yet thread-safe and can thus not be combined with `experimental-threads`.
+//!   to explicitly opt-in to any instabilities or rough edges that may result. Due to a limitation in Godot, it might currently not
+//!   work Firefox browser.<br><br>
 //!
 //! # Public API
 //!
@@ -234,5 +235,6 @@ pub mod prelude {
     // Make trait methods available
     pub use super::engine::NodeExt as _;
     pub use super::obj::EngineEnum as _;
-    pub use super::obj::UserClass as _; // new_gd(), self_gd()
+    pub use super::obj::UserClass as _; // new_gd(), alloc_gd()
+    pub use super::obj::WithBaseField as _; // to_gd()
 }
diff --git a/itest/rust/build.rs b/itest/rust/build.rs
diff --git a/itest/rust/src/builtin_tests/containers/callable_test.rs b/itest/rust/src/builtin_tests/containers/callable_test.rs
diff --git a/itest/rust/src/builtin_tests/convert_test.rs b/itest/rust/src/builtin_tests/convert_test.rs
diff --git a/itest/rust/src/object_tests/base_test.rs b/itest/rust/src/object_tests/base_test.rs
diff --git a/itest/rust/src/object_tests/object_test.rs b/itest/rust/src/object_tests/object_test.rs
diff --git a/itest/rust/src/object_tests/property_test.rs b/itest/rust/src/object_tests/property_test.rs
diff --git a/itest/rust/src/object_tests/virtual_methods_test.rs b/itest/rust/src/object_tests/virtual_methods_test.rs
diff --git a/itest/rust/src/register_tests/derive_variant_test.rs b/itest/rust/src/register_tests/derive_variant_test.rs

Original file line number	Diff line number	Diff line change
`@@ -125,7 +125,7 @@ function cmd_fmt() {`
`125`	`125`	`}`
`126`	`126`
`127`	`127`	`function cmd_clippy() {`
`128`		`- run cargo clippy "${extraCargoArgs[@]}" -- \`
	`128`	`+ run cargo clippy --all-targets "${extraCargoArgs[@]}" -- \`
`129`	`129`	`-D clippy::suspicious \`
`130`	`130`	`-D clippy::style \`
`131`	`131`	`-D clippy::complexity \`
Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,7 @@ impl Error for ConvertError {`
`108`	`108`	`}`
`109`	`109`	`}`
`110`	`110`
`111`		`-#[derive(Debug, PartialEq, Eq)]`
	`111`	`+#[derive(Eq, PartialEq, Debug)]`
`112`	`112`	`pub(crate) enum ErrorKind {`
`113`	`113`	`FromGodot(FromGodotError),`
`114`	`114`	`FromFfi(FromFfiError),`