Skip to content

Commit

Permalink
Fix: Allow to export Array<DynGd<T,D>>
Browse files Browse the repository at this point in the history
- Add base inherited class to DynTraitImpl - it is necessary due to lack of guaranteed order of registering classes (meaning that we are unable to check if given GDExtension class inherits the other – since it might not be loaded yet) .
- Implement `element_type_string` for `DynGd<T, D>` ArrayElement.
- Generate proper hint string while exporting Resource-based `DynGd<T, D>`. Don't include classes that don't inherit the base class T (for example Objects/Nodes for `DynGd<Resource, D>`).
- Use base class while exporting Node-based `DynGd<T, D>`. In other words – `#[export] DynGd<T, D>` and `#[export] Gd<T>` works identically editor-wise.
- 2.0 compatibility – allow to use `DynGd<MyRustClass, D>` and `Array<DynGd<MyRustClass, D>>` even if that doesn't make sense semantically (just use Gd<MyRustClass> instead).
- Extract `object_export_element_type_string` from `Gd<T>`'s ArrayElement` to share implementation between both.
  • Loading branch information
Yarwin committed Feb 24, 2025
1 parent 40dcc56 commit e67d86f
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 37 deletions.
2 changes: 1 addition & 1 deletion godot-core/src/classes/class_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub(crate) fn ensure_object_inherits(derived: ClassName, base: ClassName, instan

/// Checks if `derived` inherits from `base`, using a cache for _successful_ queries.
#[cfg(debug_assertions)]
fn is_derived_base_cached(derived: ClassName, base: ClassName) -> bool {
pub(crate) fn is_derived_base_cached(derived: ClassName, base: ClassName) -> bool {
use std::collections::HashSet;
use sys::Global;
static CACHE: Global<HashSet<(ClassName, ClassName)>> = Global::default();
Expand Down
10 changes: 7 additions & 3 deletions godot-core/src/obj/dyn_gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

use crate::builtin::Variant;
use crate::builtin::{GString, Variant};
use crate::meta::error::ConvertError;
use crate::meta::{ClassName, FromGodot, GodotConvert, PropertyHintInfo, ToGodot};
use crate::obj::guards::DynGdRef;
use crate::obj::{bounds, AsDyn, Bounds, DynGdMut, Gd, GodotClass, Inherits};
use crate::registry::class::{get_dyn_property_hint_string, try_dynify_object};
use crate::registry::property::{Export, Var};
use crate::registry::property::{object_export_element_type_string, Export, Var};
use crate::{meta, sys};
use std::{fmt, ops};

Expand Down Expand Up @@ -478,6 +478,10 @@ where
T: GodotClass,
D: ?Sized + 'static,
{
fn element_type_string() -> String {
let hint_string = get_dyn_property_hint_string::<T, D>();
object_export_element_type_string::<T>(hint_string)
}
}

impl<T, D> Var for DynGd<T, D>
Expand All @@ -502,7 +506,7 @@ where
{
fn export_hint() -> PropertyHintInfo {
PropertyHintInfo {
hint_string: get_dyn_property_hint_string::<D>(),
hint_string: GString::from(get_dyn_property_hint_string::<T, D>()),
..<Gd<T> as Export>::export_hint()
}
}
Expand Down
30 changes: 5 additions & 25 deletions godot-core/src/obj/gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
use std::ops::{Deref, DerefMut};

use godot_ffi as sys;
use sys::{static_assert_eq_size_align, SysPtr as _, VariantType};
use sys::{static_assert_eq_size_align, SysPtr as _};

use crate::builtin::{Callable, NodePath, StringName, Variant};
use crate::global::PropertyHint;
Expand All @@ -19,11 +19,11 @@ use crate::meta::{
ParamType, PropertyHintInfo, RefArg, ToGodot,
};
use crate::obj::{
bounds, cap, Bounds, DynGd, EngineEnum, GdDerefTarget, GdMut, GdRef, GodotClass, Inherits,
InstanceId, RawGd,
bounds, cap, Bounds, DynGd, GdDerefTarget, GdMut, GdRef, GodotClass, Inherits, InstanceId,
RawGd,
};
use crate::private::callbacks;
use crate::registry::property::{Export, Var};
use crate::registry::property::{object_export_element_type_string, Export, Var};
use crate::{classes, out};

/// Smart pointer to objects owned by the Godot engine.
Expand Down Expand Up @@ -795,27 +795,7 @@ impl<T: GodotClass> GodotType for Gd<T> {
impl<T: GodotClass> ArrayElement for Gd<T> {
fn element_type_string() -> String {
// See also impl Export for Gd<T>.

let hint = if T::inherits::<classes::Resource>() {
Some(PropertyHint::RESOURCE_TYPE)
} else if T::inherits::<classes::Node>() {
Some(PropertyHint::NODE_TYPE)
} else {
None
};

// Exportable classes (Resource/Node based) include the {RESOURCE|NODE}_TYPE hint + the class name.
if let Some(export_hint) = hint {
format!(
"{variant}/{hint}:{class}",
variant = VariantType::OBJECT.ord(),
hint = export_hint.ord(),
class = T::class_name()
)
} else {
// Previous impl: format!("{variant}:", variant = VariantType::OBJECT.ord())
unreachable!("element_type_string() should only be invoked for exportable classes")
}
object_export_element_type_string::<T>(T::class_name())
}
}

Expand Down
42 changes: 34 additions & 8 deletions godot-core/src/registry/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ use godot_ffi::join_with;
use std::collections::HashMap;
use std::{any, ptr};

use crate::builtin::GString;
use crate::classes::ClassDb;
use crate::init::InitLevel;
use crate::meta::error::{ConvertError, FromGodotError};
use crate::meta::ClassName;
use crate::obj::{cap, DynGd, Gd, GodotClass};
use crate::private::{ClassPlugin, PluginItem};
use crate::registry::callbacks;
use crate::registry::plugin::{DynTraitImpl, ErasedRegisterFn, ITraitImpl, InherentImpl, Struct};
use crate::{godot_error, godot_warn, sys};
use crate::{classes, godot_error, godot_warn, sys};
use sys::{interface_fn, out, Global, GlobalGuard, GlobalLockError};

/// Returns a lock to a global map of loaded classes, by initialization level.
Expand Down Expand Up @@ -244,7 +244,10 @@ fn register_classes_and_dyn_traits(
let metadata = ClassMetadata {};

// Transpose Class->Trait relations to Trait->Class relations.
for (trait_type_id, dyn_trait_impl) in info.dynify_fns_by_trait.drain() {
for (trait_type_id, mut dyn_trait_impl) in info.dynify_fns_by_trait.drain() {
// Note: Must be done after filling out the class info since plugins are being iterated in unspecified order.
dyn_trait_impl.parent_class_name = info.parent_class_name;

dyn_traits_by_typeid
.entry(trait_type_id)
.or_default()
Expand Down Expand Up @@ -331,21 +334,29 @@ pub(crate) fn try_dynify_object<T: GodotClass, D: ?Sized + 'static>(
/// Responsible for creating hint_string for [`DynGd<T, D>`][crate::obj::DynGd] properties which works with [`PropertyHint::NODE_TYPE`][crate::global::PropertyHint::NODE_TYPE] or [`PropertyHint::RESOURCE_TYPE`][crate::global::PropertyHint::RESOURCE_TYPE].
///
/// Godot offers very limited capabilities when it comes to validating properties in the editor if given class isn't a tool.
/// Proper hint string combined with `PropertyHint::NODE_TYPE` or `PropertyHint::RESOURCE_TYPE` allows to limit selection only to valid classes - those registered as implementors of given `DynGd<T, D>`'s `D` trait.
/// Proper hint string combined with `PropertyHint::RESOURCE_TYPE` allows to limit selection only to valid classes - those registered as implementors of given `DynGd<T, D>`'s `D` trait.
/// Godot editor allows to export only one node type with `PropertyHint::NODE_TYPE` – therefore we are returning only the base class.
///
/// See also [Godot docs for PropertyHint](https://docs.godotengine.org/en/stable/classes/class_@globalscope.html#enum-globalscope-propertyhint).
pub(crate) fn get_dyn_property_hint_string<D>() -> GString
pub(crate) fn get_dyn_property_hint_string<T, D>() -> String
where
T: GodotClass,
D: ?Sized + 'static,
{
// Exporting multiple node types is not supported.
if T::inherits::<classes::Node>() {
return T::class_name().to_string();
}

let typeid = any::TypeId::of::<D>();
let dyn_traits_by_typeid = global_dyn_traits_by_typeid();

let Some(relations) = dyn_traits_by_typeid.get(&typeid) else {
let trait_name = sys::short_type_name::<D>();
godot_warn!(
"godot-rust: No class has been linked to trait {trait_name} with #[godot_dyn]."
);
return GString::default();
return String::new();
};
assert!(
!relations.is_empty(),
Expand All @@ -354,10 +365,25 @@ where
**this is a bug, please report it**",
trait_name = sys::short_type_name::<D>()
);
let relations_iter = relations.iter();

// Include only implementors inheriting given T.
// For example – don't include Nodes or Objects while creating hint_string for Resource.
let relations_iter = relations_iter.filter_map(|implementor| {
// TODO – check if caching it (using is_derived_base_cached) yields any benefits.
if ClassDb::singleton().is_parent_class(
&implementor.parent_class_name?.to_string_name(),
&T::class_name().to_string_name(),
) {
Some(implementor)
} else {
None
}
});

GString::from(join_with(relations.iter(), ", ", |dyn_trait| {
join_with(relations_iter, ", ", |dyn_trait| {
dyn_trait.class_name().to_cow_str()
}))
})
}

/// Populate `c` with all the relevant data from `component` (depending on component type).
Expand Down
11 changes: 11 additions & 0 deletions godot-core/src/registry/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,16 @@ pub struct DynTraitImpl {
/// The class that this `dyn Trait` implementation corresponds to.
class_name: ClassName,

/// Base inherited class required for `DynGd<T, D>` exports (i.e. one specified in `#[class(base = ...)]`).
///
/// Godot doesn't guarantee availability of all the GDExtension classes through the ClassDb while generating `PropertyHintInfo` for our exports.
/// Therefore, we rely on the built-in inherited base class in such cases.
/// Only [`class_name`][DynTraitImpl::class_name] is available at the time of adding given `DynTraitImpl` to plugin registry with `#[godot_dyn]`;
/// It is important to fill this information before registration.
///
/// See also [`get_dyn_property_hint_string`][crate::registry::class::get_dyn_property_hint_string].
pub(crate) parent_class_name: Option<ClassName>,

/// TypeId of the `dyn Trait` object.
dyn_trait_typeid: any::TypeId,

Expand All @@ -533,6 +543,7 @@ impl DynTraitImpl {
{
Self {
class_name: T::class_name(),
parent_class_name: None,
dyn_trait_typeid: std::any::TypeId::of::<D>(),
erased_dynify_fn: callbacks::dynify_fn::<T, D>,
}
Expand Down
32 changes: 32 additions & 0 deletions godot-core/src/registry/property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,14 @@

//! Registration support for property types.
use crate::classes;
use crate::global::PropertyHint;
use godot_ffi as sys;
use godot_ffi::VariantType;
use std::fmt::Display;

use crate::meta::{ClassName, FromGodot, GodotConvert, GodotType, PropertyHintInfo, ToGodot};
use crate::obj::{EngineEnum, GodotClass};

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Trait definitions
Expand Down Expand Up @@ -549,3 +554,30 @@ pub(crate) fn builtin_type_string<T: GodotType>() -> String {
format!("{}:{}", variant_type.sys(), T::godot_type_name())
}
}

/// Creates `hint_string` to be used for given `GodotClass` when used as an `ArrayElement`.
pub(crate) fn object_export_element_type_string<T>(class_hint: impl Display) -> String
where
T: GodotClass,
{
let hint = if T::inherits::<classes::Resource>() {
Some(PropertyHint::RESOURCE_TYPE)
} else if T::inherits::<classes::Node>() {
Some(PropertyHint::NODE_TYPE)
} else {
None
};

// Exportable classes (Resource/Node based) include the {RESOURCE|NODE}_TYPE hint + the class name.
if let Some(export_hint) = hint {
format!(
"{variant}/{hint}:{class}",
variant = VariantType::OBJECT.ord(),
hint = export_hint.ord(),
class = class_hint
)
} else {
// Previous impl: format!("{variant}:", variant = VariantType::OBJECT.ord())
unreachable!("element_type_string() should only be invoked for exportable classes")
}
}
1 change: 1 addition & 0 deletions itest/rust/src/object_tests/dyn_gd_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ impl InstanceIdProvider for foreign::NodeHealth {
struct RefcDynGdExporter {
#[var]
first: Option<DynGd<Object, dyn Health>>,
// Using DynGd with concrete type foreign::NodeHealth doesn't give benefits over Gd, but is allowed in godot-rust 0.2.x.
#[export]
second: Option<DynGd<foreign::NodeHealth, dyn InstanceIdProvider<Id = InstanceId>>>,
}
Expand Down

0 comments on commit e67d86f

Please sign in to comment.