Merge pull request #957 from godot-rust/qol/traits-and-tools

`sys::short_type_name`, conversions and relaxed `GodotType`

Author: Bromeon

godot-core/src/builtin/collections/array.rs

@@ -12,8 +12,9 @@ use crate::builtin::*;
 use crate::meta;
 use crate::meta::error::{ConvertError, FromGodotError, FromVariantError};
 use crate::meta::{
-    ArrayElement, ArrayTypeInfo, AsArg, CowArg, FromGodot, GodotConvert, GodotFfiVariant,
-    GodotType, ParamType, PropertyHintInfo, RefArg, ToGodot,
+    element_godot_type_name, element_variant_type, ArrayElement, ArrayTypeInfo, AsArg, CowArg,
+    FromGodot, GodotConvert, GodotFfiVariant, GodotType, ParamType, PropertyHintInfo, RefArg,
+    ToGodot,
 };
 use crate::registry::property::{Export, Var};
 use godot_ffi as sys;
@@ -886,10 +887,16 @@ impl<T: ArrayElement> Array<T> {
     /// Used as `if` statement in trait impls. Avoids defining yet another trait or non-local overridden function just for this case;
     /// `Variant` is the only Godot type that has variant type NIL and can be used as an array element.
     fn has_variant_t() -> bool {
-        T::Ffi::variant_type() == VariantType::NIL
+        element_variant_type::<T>() == VariantType::NIL
     }
 }
 
+#[test]
+fn correct_variant_t() {
+    assert!(Array::<Variant>::has_variant_t());
+    assert!(!Array::<i64>::has_variant_t());
+}
+
 impl VariantArray {
     /// # Safety
     /// - Variant must have type `VariantType::ARRAY`.
@@ -1133,7 +1140,7 @@ impl<T: ArrayElement> GodotType for Array<T> {
         // Typed arrays use type hint.
         PropertyHintInfo {
             hint: crate::global::PropertyHint::ARRAY_TYPE,
-            hint_string: T::godot_type_name().into(),
+            hint_string: GString::from(element_godot_type_name::<T>()),
         }
     }
 }

godot-core/src/meta/args/as_arg.rs

@@ -6,7 +6,7 @@
  */
 
 use crate::builtin::{GString, NodePath, StringName};
-use crate::meta::{CowArg, GodotType};
+use crate::meta::{sealed, CowArg};
 use std::ffi::CStr;
 
 /// Implicit conversions for arguments passed to Godot APIs.
@@ -253,7 +253,9 @@ impl AsArg<NodePath> for &String {
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 
 /// Implemented for all parameter types `T` that are allowed to receive [impl `AsArg<T>`][AsArg].
-pub trait ParamType: GodotType
+// ParamType used to be a subtrait of GodotType, but this can be too restrictive. For example, DynGd is not a "Godot canonical type"
+// (GodotType), however it's still useful to store it in arrays -- which requires AsArg and subsequently ParamType.
+pub trait ParamType: sealed::Sealed + Sized + 'static
 // GodotType bound not required right now, but conceptually should always be the case.
 {
     /// Canonical argument passing type, either `T` or an internally-used CoW type.

godot-core/src/meta/array_type_info.rs

@@ -6,8 +6,8 @@
  */
 
 use crate::builtin::{StringName, VariantType};
-use crate::meta::traits::GodotFfi;
-use crate::meta::GodotType;
+use crate::meta::traits::element_variant_type;
+use crate::meta::{ArrayElement, GodotType};
 use std::fmt;
 
 /// Represents the type information of a Godot array. See
@@ -26,8 +26,8 @@ pub(crate) struct ArrayTypeInfo {
 }
 
 impl ArrayTypeInfo {
-    pub fn of<T: GodotType>() -> Self {
-        let variant_type = <T::Via as GodotType>::Ffi::variant_type();
+    pub fn of<T: ArrayElement>() -> Self {
+        let variant_type = element_variant_type::<T>();
         let class_name = if variant_type == VariantType::OBJECT {
             Some(T::Via::class_name().to_string_name())
         } else {

godot-core/src/meta/mod.rs

@@ -61,7 +61,9 @@ pub use godot_convert::{FromGodot, GodotConvert, ToGodot};
 pub use traits::{ArrayElement, GodotType, PackedArrayElement};
 
 pub(crate) use array_type_info::ArrayTypeInfo;
-pub(crate) use traits::{GodotFfiVariant, GodotNullableFfi};
+pub(crate) use traits::{
+    element_godot_type_name, element_variant_type, GodotFfiVariant, GodotNullableFfi,
+};
 
 use crate::registry::method::MethodParamOrReturnInfo;
 

godot-core/src/meta/property_info.rs

@@ -7,7 +7,9 @@
 
 use crate::builtin::{GString, StringName};
 use crate::global::{PropertyHint, PropertyUsageFlags};
-use crate::meta::{ArrayElement, ClassName, GodotType, PackedArrayElement};
+use crate::meta::{
+    element_godot_type_name, ArrayElement, ClassName, GodotType, PackedArrayElement,
+};
 use crate::registry::property::{Export, Var};
 use crate::sys;
 use godot_ffi::VariantType;
@@ -234,7 +236,7 @@ impl PropertyHintInfo {
     pub fn var_array_element<T: ArrayElement>() -> Self {
         Self {
             hint: PropertyHint::ARRAY_TYPE,
-            hint_string: GString::from(T::godot_type_name()),
+            hint_string: GString::from(element_godot_type_name::<T>()),
         }
     }
 

godot-core/src/meta/traits.rs

@@ -5,15 +5,14 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use godot_ffi as sys;
-
-use crate::builtin::Variant;
+use crate::builtin::{Variant, VariantType};
 use crate::global::PropertyUsageFlags;
 use crate::meta::error::ConvertError;
 use crate::meta::{
     sealed, ClassName, FromGodot, GodotConvert, PropertyHintInfo, PropertyInfo, ToGodot,
 };
 use crate::registry::method::MethodParamOrReturnInfo;
+use godot_ffi as sys;
 
 // Re-export sys traits in this module, so all are in one place.
 use crate::registry::property::builtin_type_string;
@@ -162,7 +161,10 @@ pub trait GodotType: GodotConvert<Via = Self> + sealed::Sealed + Sized + 'static
     message = "`Array<T>` can only store element types supported in Godot arrays (no nesting).",
     label = "has invalid element type"
 )]
-pub trait ArrayElement: GodotType + ToGodot + FromGodot + sealed::Sealed + meta::ParamType {
+pub trait ArrayElement: ToGodot + FromGodot + sealed::Sealed + meta::ParamType {
+    // Note: several indirections in ArrayElement and the global `element_*` functions go through `GodotConvert::Via`,
+    // to not require Self: GodotType. What matters is how array elements map to Godot on the FFI level (GodotType trait).
+
     /// Returns the representation of this type as a type string.
     ///
     /// Used for elements in arrays (the latter despite `ArrayElement` not having a direct relation).
@@ -172,7 +174,7 @@ pub trait ArrayElement: GodotType + ToGodot + FromGodot + sealed::Sealed + meta:
     #[doc(hidden)]
     fn element_type_string() -> String {
         // Most array elements and all packed array elements are builtin types, so this is a good default.
-        builtin_type_string::<Self>()
+        builtin_type_string::<Self::Via>()
     }
 
     #[doc(hidden)]
@@ -182,6 +184,23 @@ pub trait ArrayElement: GodotType + ToGodot + FromGodot + sealed::Sealed + meta:
     }
 }
 
+// Non-polymorphic helper functions, to avoid constant `<T::Via as GodotType>::` in the code.
+
+#[doc(hidden)]
+pub(crate) fn element_variant_type<T: ArrayElement>() -> VariantType {
+    <T::Via as GodotType>::Ffi::variant_type()
+}
+
+#[doc(hidden)]
+pub(crate) fn element_godot_type_name<T: ArrayElement>() -> String {
+    <T::Via as GodotType>::godot_type_name()
+}
+
+// #[doc(hidden)]
+// pub(crate)  fn element_godot_type_name<T: ArrayElement>() -> String {
+//     <T::Via as GodotType>::godot_type_name()
+// }
+
 /// Marker trait to identify types that can be stored in `Packed*Array` types.
 #[diagnostic::on_unimplemented(
     message = "`Packed*Array` can only store element types supported in Godot packed arrays.",

godot-core/src/obj/gd.rs

@@ -692,6 +692,7 @@ impl<T: GodotClass> GodotConvert for Gd<T> {
 }
 
 impl<T: GodotClass> ToGodot for Gd<T> {
+    // TODO return RefArg here?
     type ToVia<'v> = Gd<T>;
 
     fn to_godot(&self) -> Self::ToVia<'_> {

godot-ffi/src/conv.rs

@@ -25,11 +25,23 @@ pub fn u32_to_usize(i: u32) -> usize {
     unsafe { i.try_into().unwrap_unchecked() }
 }
 
-/// Converts a rust-bool into a sys-bool.
+/// Converts a Rust-bool into a sys-bool.
 pub const fn bool_to_sys(value: bool) -> sys::GDExtensionBool {
     value as sys::GDExtensionBool
 }
 
+/// Converts a sys-bool to Rust-bool.
+///
+/// # Panics
+/// If the value is not a valid sys-bool (0 or 1).
+pub fn bool_from_sys(value: sys::GDExtensionBool) -> bool {
+    match value {
+        SYS_TRUE => true,
+        SYS_FALSE => false,
+        _ => panic!("Invalid GDExtensionBool value: {}", value),
+    }
+}
+
 /// Convert a list into a pointer + length pair. Should be used together with [`ptr_list_from_sys`].
 ///
 /// If `list_from_sys` is not called on this list then that will cause a memory leak.

godot-ffi/src/toolbox.rs

@@ -187,6 +187,67 @@ pub fn unqualified_type_name<T>() -> &'static str {
 }
 */
 
+/// Like [`std::any::type_name`], but returns a short type name without module paths.
+pub fn short_type_name<T>() -> String {
+    let full_name = std::any::type_name::<T>();
+    strip_module_paths(full_name)
+}
+
+/// Like [`std::any::type_name_of_val`], but returns a short type name without module paths.
+pub fn short_type_name_of_val<T>(val: &T) -> String {
+    let full_name = std::any::type_name_of_val(val);
+    strip_module_paths(full_name)
+}
+
+/// Helper function to strip module paths from a fully qualified type name.
+fn strip_module_paths(full_name: &str) -> String {
+    let mut result = String::new();
+    let mut identifier = String::new();
+
+    let mut chars = full_name.chars().peekable();
+
+    while let Some(c) = chars.next() {
+        match c {
+            '<' | '>' | ',' | ' ' | '&' | '(' | ')' | '[' | ']' => {
+                // Process the current identifier.
+                if !identifier.is_empty() {
+                    let short_name = identifier.split("::").last().unwrap_or(&identifier);
+                    result.push_str(short_name);
+                    identifier.clear();
+                }
+                result.push(c);
+
+                // Handle spaces after commas for readability.
+                if c == ',' && chars.peek().map_or(false, |&next_c| next_c != ' ') {
+                    result.push(' ');
+                }
+            }
+            ':' => {
+                // Check for '::' indicating module path separator.
+                if chars.peek() == Some(&':') {
+                    // Skip the second ':'
+                    chars.next();
+                    identifier.push_str("::");
+                } else {
+                    identifier.push(c);
+                }
+            }
+            _ => {
+                // Part of an identifier.
+                identifier.push(c);
+            }
+        }
+    }
+
+    // Process any remaining identifier.
+    if !identifier.is_empty() {
+        let short_name = identifier.split("::").last().unwrap_or(&identifier);
+        result.push_str(short_name);
+    }
+
+    result
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Private helpers
 
@@ -457,3 +518,53 @@ mod manual_init_cell {
 }
 
 pub(crate) use manual_init_cell::ManualInitCell;
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Unit tests
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_short_type_name() {
+        assert_eq!(short_type_name::<i32>(), "i32");
+        assert_eq!(short_type_name::<Option<i32>>(), "Option<i32>");
+        assert_eq!(
+            short_type_name::<Result<Option<i32>, String>>(),
+            "Result<Option<i32>, String>"
+        );
+        assert_eq!(
+            short_type_name::<Vec<Result<Option<i32>, String>>>(),
+            "Vec<Result<Option<i32>, String>>"
+        );
+        assert_eq!(
+            short_type_name::<std::collections::HashMap<String, Vec<i32>>>(),
+            "HashMap<String, Vec<i32>>"
+        );
+        assert_eq!(
+            short_type_name::<Result<Option<i32>, String>>(),
+            "Result<Option<i32>, String>"
+        );
+        assert_eq!(short_type_name::<i32>(), "i32");
+        assert_eq!(short_type_name::<Vec<String>>(), "Vec<String>");
+    }
+
+    #[test]
+    fn test_short_type_name_of_val() {
+        let value = Some(42);
+        assert_eq!(short_type_name_of_val(&value), "Option<i32>");
+
+        let result: Result<_, String> = Ok(Some(42));
+        assert_eq!(
+            short_type_name_of_val(&result),
+            "Result<Option<i32>, String>"
+        );
+
+        let vec = vec![result];
+        assert_eq!(
+            short_type_name_of_val(&vec),
+            "Vec<Result<Option<i32>, String>>"
+        );
+    }
+}