Merge pull request #719 from godot-rust/qol/type-string-duplication

Small code deduplication + docs

Author: Bromeon

examples/dodge-the-creeps/rust/.cargo/config renamed to examples/dodge-the-creeps/rust/.cargo/config.toml

@@ -8,7 +8,7 @@
 use godot_ffi as sys;
 
 use crate::builtin::*;
-use crate::property::{Export, PropertyHintInfo, TypeStringHint, Var};
+use crate::property::{builtin_type_string, Export, PropertyHintInfo, TypeStringHint, Var};
 use std::fmt;
 use std::marker::PhantomData;
 use sys::{ffi_methods, interface_fn, GodotFfi};
@@ -834,11 +834,7 @@ impl<T: ArrayElement + TypeStringHint> TypeStringHint for Array<T> {
 
 impl TypeStringHint for VariantArray {
     fn type_string() -> String {
-        if sys::GdextBuild::since_api("4.3") {
-            format!("{}:", VariantType::Array as i32)
-        } else {
-            format!("{}:Array", VariantType::Array as i32)
-        }
+        builtin_type_string::<VariantArray>()
     }
 }
 

godot-core/src/builtin/array.rs

@@ -9,12 +9,13 @@ use godot_ffi as sys;
 
 use crate::builtin::meta::{FromGodot, ToGodot};
 use crate::builtin::{inner, Variant, VariantArray};
-use crate::property::{Export, PropertyHintInfo, TypeStringHint, Var};
-use std::marker::PhantomData;
-use std::{fmt, ptr};
+use crate::property::{builtin_type_string, Export, PropertyHintInfo, TypeStringHint, Var};
 use sys::types::OpaqueDictionary;
 use sys::{ffi_methods, interface_fn, GodotFfi};
 
+use std::marker::PhantomData;
+use std::{fmt, ptr};
+
 use super::meta::impl_godot_as_self;
 
 /// Godot's `Dictionary` type.
@@ -330,11 +331,7 @@ impl Var for Dictionary {
 
 impl TypeStringHint for Dictionary {
     fn type_string() -> String {
-        if sys::GdextBuild::since_api("4.3") {
-            format!("{}:", sys::VariantType::Dictionary as i32)
-        } else {
-            format!("{}:Dictionary", sys::VariantType::Dictionary as i32)
-        }
+        builtin_type_string::<Dictionary>()
     }
 }
 

godot-core/src/builtin/dictionary.rs

@@ -481,15 +481,16 @@ macro_rules! impl_packed_array {
 
         impl $crate::property::Export for $PackedArray {
             fn default_export_info() -> $crate::property::PropertyHintInfo {
+                // In 4.3 Godot can (and does) use type hint strings for packed arrays, see https://github.com/godotengine/godot/pull/82952.
                 if sys::GdextBuild::since_api("4.3") {
-                    // In 4.3 Godot can (and does) use type hint strings for packed arrays.
-                    // https://github.com/godotengine/godot/pull/82952
                     $crate::property::PropertyHintInfo {
                         hint: $crate::engine::global::PropertyHint::TYPE_STRING,
                         hint_string: <$Element as $crate::property::TypeStringHint>::type_string().into(),
                     }
                 } else {
-                    $crate::property::PropertyHintInfo::with_hint_none(<$PackedArray as $crate::builtin::meta::GodotType>::godot_type_name())
+                    $crate::property::PropertyHintInfo::with_hint_none(
+                        <$PackedArray as $crate::builtin::meta::GodotType>::godot_type_name()
+                    )
                 }
             }
         }

godot-core/src/builtin/packed_array.rs

@@ -13,7 +13,6 @@ pub mod builtin;
 pub mod init;
 pub mod log;
 pub mod obj;
-pub mod property;
 
 #[doc(hidden)]
 #[path = "deprecated.rs"]

godot-core/src/lib.rs

@@ -64,14 +64,16 @@ pub(super) mod private {
     ///
     /// See [`bounds`](crate::obj::bounds) module for how to use this for bounds checking.
     ///
+    /// # No manual `impl`
+    ///
     /// <div class="warning">
     /// <strong>Never</strong> implement this trait manually.
     /// </div>
     ///
     /// Most of the time, this trait is covered by [`#[derive(GodotClass)]`](../register/derive.GodotClass.html).
     /// If you implement `GodotClass` manually, use the [`implement_godot_bounds!`][crate::implement_godot_bounds] macro.
     ///
-    /// There are two reasons to avoid a hand-written `impl Bounds`:
+    /// There are two reasons to avoid a handwritten `impl Bounds`:
     /// - The trait is `unsafe` and it is very easy to get internal bounds wrong. This will lead to immediate UB.
     /// - Apart from the documented members, the trait may have undocumented items that may be broken at any time and stand under no SemVer
     ///   guarantees.

godot-core/src/obj/bounds.rs

@@ -137,6 +137,7 @@ pub unsafe trait Inherits<Base: GodotClass>: GodotClass {}
 unsafe impl<T: GodotClass> Inherits<T> for T {}
 
 /// Implemented for all user-defined classes, providing extensions on the raw object to interact with `Gd`.
+#[doc(hidden)]
 pub trait UserClass: Bounds<Declarer = bounds::DeclUser> {
     #[doc(hidden)]
     fn __config() -> crate::private::ClassConfig;
@@ -207,15 +208,14 @@ pub trait IndexEnum: EngineEnum {
     }
 }
 
-/// Trait that's implemented for user-defined classes that provide a `Base<T>` field.
+/// Trait that is automatically implemented for user classes containing a `Base<T>` field.
 ///
-/// Gives direct access to the containing `Gd<Self>` from `Self`.
+/// Gives direct access to the containing `Gd<Self>` from `self`.
 ///
-/// # Using WithBaseField as a bound
+/// # Usage as a bound
 ///
-/// In order to call `self.base()` or `self.base_mut()` within a trait or on a type you define, the type of `Self::Base` must be specified via `WithBaseField<Base = T>`
-///
-/// E.g.
+/// In order to call `base()` or `base_mut()` within a function or on a type you define, you need a `WithBaseField<Base = T>` bound,
+/// where `T` is the base class of your type.
 ///
 /// ```no_run
 /// # use godot::prelude::*;
@@ -225,6 +225,7 @@ pub trait IndexEnum: EngineEnum {
 ///     T: WithBaseField<Base = Node3D>,
 /// {
 ///     let base = value.base();
+///     let pos = base.get_position();
 /// }
 /// ```
 ///

godot-core/src/obj/traits.rs

@@ -16,6 +16,7 @@ use std::{fmt, ptr};
 use sys::{interface_fn, Global, GlobalGuard, GlobalLockError};
 
 pub mod callbacks;
+pub mod property;
 
 // Needed for class unregistering. The variable is populated during class registering. There is no actual concurrency here, because Godot
 // calls register/unregister in the main thread. Mutex is just casual way to ensure safety in this non-performance-critical path.

godot-core/src/registry/mod.rs

@@ -9,7 +9,7 @@
 
 use godot_ffi as sys;
 
-use crate::builtin::meta::{FromGodot, GodotConvert, ToGodot};
+use crate::builtin::meta::{FromGodot, GodotConvert, GodotType, ToGodot};
 use crate::builtin::GString;
 use crate::engine::global::PropertyHint;
 
@@ -123,9 +123,9 @@ impl PropertyHintInfo {
     ///
     /// Starting with Godot version 4.3, the hint string will always be the empty string. Before that, the hint string is set to
     /// be `type_name`.
-    pub fn with_hint_none<S: Into<GString>>(type_name: S) -> Self {
+    pub fn with_hint_none(type_name: impl Into<GString>) -> Self {
         let hint_string = if sys::GdextBuild::since_api("4.3") {
-            "".into()
+            GString::new()
         } else {
             type_name.into()
         };
@@ -336,7 +336,6 @@ pub mod export_info_functions {
 mod export_impls {
     use super::*;
     use crate::builtin::*;
-    use godot_ffi as sys;
 
     macro_rules! impl_property_by_godot_convert {
         ($Ty:ty, no_export) => {
@@ -373,15 +372,7 @@ mod export_impls {
         (@type_string_hint $Ty:ty) => {
             impl TypeStringHint for $Ty {
                 fn type_string() -> String {
-                    use sys::GodotFfi as _;
-                    let variant_type = <$Ty as $crate::builtin::meta::GodotType>::Ffi::variant_type();
-
-                    if sys::GdextBuild::since_api("4.3") {
-                        format!("{}:", variant_type as i32)
-                    } else {
-                        let type_name = <$Ty as $crate::builtin::meta::GodotType>::godot_type_name();
-                        format!("{}:{}", variant_type as i32, type_name)
-                    }
+                    builtin_type_string::<$Ty>()
                 }
             }
         }
@@ -467,3 +458,19 @@ mod export_impls {
 
     // impl_property_by_godot_convert!(Signal);
 }
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Crate-local utilities
+
+pub(crate) fn builtin_type_string<T: GodotType>() -> String {
+    use sys::GodotFfi as _;
+
+    let variant_type = T::Ffi::variant_type();
+
+    // Godot 4.3 changed representation for type hints, see https://github.com/godotengine/godot/pull/90716.
+    if sys::GdextBuild::since_api("4.3") {
+        format!("{}:", variant_type.sys())
+    } else {
+        format!("{}:{}", variant_type.sys(), T::godot_type_name())
+    }
+}

godot-core/src/property.rs renamed to godot-core/src/registry/property.rs

@@ -33,10 +33,10 @@ fn property_template_test(ctx: &TestContext) {
     for property in rust_properties.get_property_list().iter_shared() {
         let name = property.get("name").unwrap().to::<String>();
 
-        // The format of array-properties in Godot 4.2 changed. This doesn't seem to cause issues if we
-        // compile against 4.1 and provide the property in the format 4.1 expects but run it with Godot 4.2.
-        // However this test checks that our output matches that of Godot, and so would fail in this
-        // circumstance. So here for now, just ignore array properties when we compile for 4.1 but run in 4.2.
+        // The format of array properties in Godot 4.2 changed. This doesn't seem to cause issues if we
+        // compile against 4.1 and provide the property in the format 4.1 expects, but run it with Godot 4.2.
+        // However, this test checks that our output matches that of Godot, and so would fail in this circumstance.
+        // For now, just ignore array properties when we compile for 4.1 but run in 4.2.
         if GdextBuild::since_api("4.2")
             && cfg!(before_api = "4.2")
             && name.starts_with("property_array_")