Skip to content

Commit

Permalink
Remove TypeStringHint trait -> ArrayElement::element_type_string()
Browse files Browse the repository at this point in the history 
The type_string() function was previously overridden for many types,
even specialized for Array<T>, however it was only ever used for
elements of arrays and packed arrays.
  • Loading branch information
@Bromeon
Bromeon committed Aug 4, 2024
1 parent b4ffe6f commit be32f1d
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 100 deletions.
44 changes: 16 additions & 28 deletions godot-core/src/builtin/collections/array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@ use crate::meta::error::{ConvertError, FromGodotError, FromVariantError};
use crate::meta::{
ArrayElement, ArrayTypeInfo, FromGodot, GodotConvert, GodotFfiVariant, GodotType, ToGodot,
};
use crate::obj::EngineEnum;
use crate::registry::property::{
builtin_type_string, Export, PropertyHintInfo, TypeStringHint, Var,
};
use crate::registry::property::{Export, PropertyHintInfo, Var};
use godot_ffi as sys;
use sys::{ffi_methods, interface_fn, GodotFfi};

Expand Down Expand Up @@ -778,6 +775,14 @@ impl<T: ArrayElement> Array<T> {
}
}
}

/// Whether this array is untyped and holds `Variant` elements (compile-time check).
///
/// 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
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -886,43 +891,26 @@ impl<T: ArrayElement> Clone for Array<T> {
}
}

impl<T: ArrayElement + TypeStringHint> TypeStringHint for Array<T> {
fn type_string() -> String {
format!("{}:{}", VariantType::ARRAY.ord(), T::type_string())
}
}

impl TypeStringHint for VariantArray {
fn type_string() -> String {
builtin_type_string::<VariantArray>()
}
}

impl<T: ArrayElement> Var for Array<T> {
fn get_property(&self) -> Self::Via {
self.to_godot()
}

fn set_property(&mut self, value: Self::Via) {
*self = FromGodot::from_godot(value)
}
}

impl<T: ArrayElement + TypeStringHint> Export for Array<T> {
impl<T: ArrayElement> Export for Array<T> {
fn default_export_info() -> PropertyHintInfo {
PropertyHintInfo {
hint: crate::global::PropertyHint::TYPE_STRING,
hint_string: T::type_string().into(),
// If T == Variant, then we return "Array" builtin type hint.
if Self::has_variant_t() {
PropertyHintInfo::with_type_name::<VariantArray>()
} else {
PropertyHintInfo::with_array_element::<T>()
}
}
}

impl Export for Array<Variant> {
fn default_export_info() -> PropertyHintInfo {
PropertyHintInfo::with_hint_none("Array")
}
}

impl<T: ArrayElement> Default for Array<T> {
#[inline]
fn default() -> Self {
Expand Down Expand Up @@ -975,7 +963,7 @@ impl<T: ArrayElement> GodotType for Array<T> {
#[cfg(since_api = "4.2")]
fn property_hint_info() -> PropertyHintInfo {
// Array<Variant>, aka untyped array, has no hints.
if T::Ffi::variant_type() == VariantType::NIL {
if Self::has_variant_t() {
return PropertyHintInfo::none();
}

Expand Down
10 changes: 1 addition & 9 deletions godot-core/src/builtin/collections/dictionary.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use godot_ffi as sys;

use crate::builtin::{inner, Variant, VariantArray};
use crate::meta::{FromGodot, ToGodot};
use crate::registry::property::{
builtin_type_string, Export, PropertyHintInfo, TypeStringHint, Var,
};
use crate::registry::property::{Export, PropertyHintInfo, Var};
use sys::types::OpaqueDictionary;
use sys::{ffi_methods, interface_fn, GodotFfi};

Expand Down Expand Up @@ -412,12 +410,6 @@ impl Var for Dictionary {
}
}

impl TypeStringHint for Dictionary {
fn type_string() -> String {
builtin_type_string::<Dictionary>()
}
}

impl Export for Dictionary {
fn default_export_info() -> PropertyHintInfo {
PropertyHintInfo::with_hint_none("Dictionary")
Expand Down
9 changes: 2 additions & 7 deletions godot-core/src/builtin/collections/packed_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,14 +548,9 @@ macro_rules! impl_packed_array {
fn default_export_info() -> $crate::registry::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") {
$crate::registry::property::PropertyHintInfo {
hint: $crate::global::PropertyHint::TYPE_STRING,
hint_string: <$Element as $crate::registry::property::TypeStringHint>::type_string().into(),
}
$crate::registry::property::PropertyHintInfo::with_array_element::<$Element>()
} else {
$crate::registry::property::PropertyHintInfo::with_hint_none(
<$PackedArray as $crate::meta::GodotType>::godot_type_name()
)
$crate::registry::property::PropertyHintInfo::with_type_name::<$PackedArray>()
}
}
}
Expand Down
17 changes: 15 additions & 2 deletions godot-core/src/meta/traits.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::meta::{sealed, ClassName, FromGodot, GodotConvert, PropertyInfo, ToGo
use crate::registry::method::MethodParamOrReturnInfo;

// Re-export sys traits in this module, so all are in one place.
use crate::registry::property::PropertyHintInfo;
use crate::registry::property::{builtin_type_string, PropertyHintInfo};
pub use sys::{GodotFfi, GodotNullableFfi};

/// Conversion of [`GodotFfi`] types to/from [`Variant`].
Expand Down Expand Up @@ -126,4 +126,17 @@ pub trait GodotType:
label = "does not implement `Var`",
note = "see also: https://godot-rust.github.io/docs/gdext/master/godot/builtin/meta/trait.ArrayElement.html"
)]
pub trait ArrayElement: GodotType {}
pub trait ArrayElement: GodotType + sealed::Sealed {
/// Returns the representation of this type as a type string.
///
/// Used for elements in arrays and packed arrays (the latter despite `ArrayElement` not having a direct relation).
///
/// See [`PropertyHint::TYPE_STRING`] and [upstream docs].
///
/// [upstream docs]: https://docs.godotengine.org/en/stable/classes/class_%40globalscope.html#enum-globalscope-propertyhint
#[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>()
}
}
43 changes: 22 additions & 21 deletions godot-core/src/obj/gd.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::obj::{
RawGd,
};
use crate::private::callbacks;
use crate::registry::property::{Export, PropertyHintInfo, TypeStringHint, Var};
use crate::registry::property::{Export, PropertyHintInfo, Var};
use crate::{classes, out};

/// Smart pointer to objects owned by the Godot engine.
Expand Down Expand Up @@ -734,8 +734,27 @@ impl<T: GodotClass> GodotType for Gd<T> {
}
}

impl<T: GodotClass> ArrayElement for Gd<T> {}
impl<T: GodotClass> ArrayElement for Option<Gd<T>> {}
impl<T: GodotClass> ArrayElement for Gd<T> {
fn element_type_string() -> String {
match Self::default_export_info().hint {
hint @ (PropertyHint::RESOURCE_TYPE | PropertyHint::NODE_TYPE) => {
format!(
"{}/{}:{}",
VariantType::OBJECT.ord(),
hint.ord(),
T::class_name()
)
}
_ => format!("{}:", VariantType::OBJECT.ord()),
}
}
}

impl<T: GodotClass> ArrayElement for Option<Gd<T>> {
fn element_type_string() -> String {
Gd::<T>::element_type_string()
}
}

impl<T> Default for Gd<T>
where
Expand All @@ -760,24 +779,6 @@ impl<T: GodotClass> Clone for Gd<T> {
}
}

impl<T: GodotClass> TypeStringHint for Gd<T> {
fn type_string() -> String {
use crate::global::PropertyHint;

match Self::default_export_info().hint {
hint @ (PropertyHint::RESOURCE_TYPE | PropertyHint::NODE_TYPE) => {
format!(
"{}/{}:{}",
VariantType::OBJECT.ord(),
hint.ord(),
T::class_name()
)
}
_ => format!("{}:", VariantType::OBJECT.ord()),
}
}
}

// TODO: Do we even want to implement `Var` and `Export` for `Gd<T>`? You basically always want to use `Option<Gd<T>>` because the editor
// may otherwise try to set the object to a null value.
impl<T: GodotClass> Var for Gd<T> {
Expand Down
45 changes: 13 additions & 32 deletions godot-core/src/registry/property.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use godot_ffi as sys;

use crate::builtin::GString;
use crate::global::PropertyHint;
use crate::meta::{FromGodot, GodotConvert, GodotType, ToGodot};
use crate::meta::{ArrayElement, FromGodot, GodotConvert, GodotType, ToGodot};

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Trait definitions
Expand Down Expand Up @@ -53,29 +53,9 @@ pub trait Export: Var {
fn default_export_info() -> PropertyHintInfo;
}

/// Marks types that are registered via "type string hint" in Godot.
///
/// See [`PropertyHint::TYPE_STRING`] and [upstream docs].
///
/// [upstream docs]: https://docs.godotengine.org/en/stable/classes/class_%40globalscope.html#enum-globalscope-propertyhint
pub trait TypeStringHint {
/// Returns the representation of this type as a type string.
///
/// See [`PropertyHint.PROPERTY_HINT_TYPE_STRING`](
/// https://docs.godotengine.org/en/stable/classes/class_%40globalscope.html#enum-globalscope-propertyhint
/// ).
fn type_string() -> String;
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Blanket impls for Option<T>

impl<T: TypeStringHint> TypeStringHint for Option<T> {
fn type_string() -> String {
T::type_string()
}
}

impl<T> Var for Option<T>
where
T: Var + FromGodot,
Expand Down Expand Up @@ -144,6 +124,17 @@ impl PropertyHintInfo {
hint_string,
}
}

pub fn with_array_element<T: ArrayElement>() -> Self {
Self {
hint: PropertyHint::TYPE_STRING,
hint_string: GString::from(T::element_type_string()),
}
}

pub fn with_type_name<T: GodotType>() -> Self {
Self::with_hint_none(T::godot_type_name())
}
}

/// Functions used to translate user-provided arguments into export hints.
Expand Down Expand Up @@ -408,13 +399,11 @@ mod export_impls {
macro_rules! impl_property_by_godot_convert {
($Ty:ty, no_export) => {
impl_property_by_godot_convert!(@property $Ty);
impl_property_by_godot_convert!(@type_string_hint $Ty);
};

($Ty:ty) => {
impl_property_by_godot_convert!(@property $Ty);
impl_property_by_godot_convert!(@export $Ty);
impl_property_by_godot_convert!(@type_string_hint $Ty);
};

(@property $Ty:ty) => {
Expand All @@ -432,18 +421,10 @@ mod export_impls {
(@export $Ty:ty) => {
impl Export for $Ty {
fn default_export_info() -> PropertyHintInfo {
PropertyHintInfo::with_hint_none(<$Ty as $crate::meta::GodotType>::godot_type_name())
PropertyHintInfo::with_type_name::<$Ty>()
}
}
};

(@type_string_hint $Ty:ty) => {
impl TypeStringHint for $Ty {
fn type_string() -> String {
builtin_type_string::<$Ty>()
}
}
}
}

// Bounding Boxes
Expand Down
2 changes: 1 addition & 1 deletion godot/src/prelude.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

pub use super::register::property::{Export, TypeStringHint, Var};
pub use super::register::property::{Export, Var};

// Re-export macros.
pub use super::register::{godot_api, Export, GodotClass, GodotConvert, Var};
Expand Down

0 comments on commit be32f1d

Please sign in to comment.