-
-
Notifications
You must be signed in to change notification settings - Fork 227
Add OnEditor<T>
, remove impl<T> Export for Gd<T>
and DynGd<T, D>
#1051
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
661c80f
49c27de
cdcdadc
f339323
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,11 +12,12 @@ use crate::builtin::*; | |
use crate::meta; | ||
use crate::meta::error::{ConvertError, FromGodotError, FromVariantError}; | ||
use crate::meta::{ | ||
element_godot_type_name, element_variant_type, ArrayElement, ArrayTypeInfo, AsArg, CowArg, | ||
FromGodot, GodotConvert, GodotFfiVariant, GodotType, ParamType, PropertyHintInfo, RefArg, | ||
ToGodot, | ||
element_godot_type_name, element_variant_type, ArrayElement, ArrayTypeInfo, AsArg, ClassName, | ||
CowArg, FromGodot, GodotConvert, GodotFfiVariant, GodotType, ParamType, PropertyHintInfo, | ||
RefArg, ToGodot, | ||
}; | ||
use crate::registry::property::{Export, Var}; | ||
use crate::obj::{bounds, Bounds, DynGd, Gd, GodotClass}; | ||
use crate::registry::property::{BuiltinExport, Export, Var}; | ||
use godot_ffi as sys; | ||
use sys::{ffi_methods, interface_fn, GodotFfi}; | ||
|
||
|
@@ -1099,6 +1100,40 @@ where | |
} | ||
} | ||
|
||
impl<T: ArrayElement> BuiltinExport for Array<T> {} | ||
|
||
impl<T: GodotClass> Export for Array<Gd<T>> | ||
where | ||
T: Bounds<Exportable = bounds::Yes>, | ||
{ | ||
fn export_hint() -> PropertyHintInfo { | ||
PropertyHintInfo::export_array_element::<Gd<T>>() | ||
} | ||
|
||
#[doc(hidden)] | ||
fn as_node_class() -> Option<ClassName> { | ||
PropertyHintInfo::object_as_node_class::<T>() | ||
} | ||
Comment on lines
+1114
to
+1116
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to be |
||
} | ||
|
||
/// `#[export]` for `Array<DynGd<T, D>>` is available only for `T` being Engine class (such as Node or Resource). | ||
/// | ||
/// Consider exporting `Array<Gd<T>>` instead of `Array<DynGd<T, D>>` for user-declared GDExtension classes. | ||
impl<T: GodotClass, D> Export for Array<DynGd<T, D>> | ||
where | ||
T: GodotClass + Bounds<Exportable = bounds::Yes, Declarer = bounds::DeclEngine>, | ||
D: ?Sized + 'static, | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't mix inline and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I thought in #1056 (diff) we settled on not restricting the Could you keep this also for arrays? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aye aye, additionally I fixed export for |
||
fn export_hint() -> PropertyHintInfo { | ||
PropertyHintInfo::export_array_element::<DynGd<T, D>>() | ||
} | ||
|
||
#[doc(hidden)] | ||
fn as_node_class() -> Option<ClassName> { | ||
PropertyHintInfo::object_as_node_class::<T>() | ||
} | ||
} | ||
|
||
impl<T: ArrayElement> Default for Array<T> { | ||
#[inline] | ||
fn default() -> Self { | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -12,15 +12,14 @@ use godot_ffi as sys; | |||||
use sys::{static_assert_eq_size_align, SysPtr as _}; | ||||||
|
||||||
use crate::builtin::{Callable, NodePath, StringName, Variant}; | ||||||
use crate::global::PropertyHint; | ||||||
use crate::meta::error::{ConvertError, FromFfiError}; | ||||||
use crate::meta::{ | ||||||
ArrayElement, AsArg, CallContext, ClassName, CowArg, FromGodot, GodotConvert, GodotType, | ||||||
ParamType, PropertyHintInfo, RefArg, ToGodot, | ||||||
}; | ||||||
use crate::obj::{ | ||||||
bounds, cap, Bounds, DynGd, GdDerefTarget, GdMut, GdRef, GodotClass, Inherits, InstanceId, | ||||||
RawGd, WithSignals, | ||||||
OnEditor, RawGd, WithSignals, | ||||||
}; | ||||||
use crate::private::callbacks; | ||||||
use crate::registry::property::{object_export_element_type_string, Export, Var}; | ||||||
|
@@ -896,8 +895,6 @@ impl<T: GodotClass> Clone for Gd<T> { | |||||
} | ||||||
} | ||||||
|
||||||
// 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> { | ||||||
fn get_property(&self) -> Self::Via { | ||||||
self.to_godot() | ||||||
|
@@ -908,34 +905,64 @@ impl<T: GodotClass> Var for Gd<T> { | |||||
} | ||||||
} | ||||||
|
||||||
impl<T> Export for Gd<T> | ||||||
impl<T> Export for Option<Gd<T>> | ||||||
where | ||||||
T: GodotClass + Bounds<Exportable = bounds::Yes>, | ||||||
Option<Gd<T>>: Var, | ||||||
{ | ||||||
fn export_hint() -> PropertyHintInfo { | ||||||
let hint = if T::inherits::<classes::Resource>() { | ||||||
PropertyHint::RESOURCE_TYPE | ||||||
} else if T::inherits::<classes::Node>() { | ||||||
PropertyHint::NODE_TYPE | ||||||
} else { | ||||||
unreachable!("classes not inheriting from Resource or Node should not be exportable") | ||||||
}; | ||||||
PropertyHintInfo::export_gd::<T>() | ||||||
} | ||||||
|
||||||
// Godot does this by default too; the hint is needed when the class is a resource/node, | ||||||
// but doesn't seem to make a difference otherwise. | ||||||
let hint_string = T::class_name().to_gstring(); | ||||||
#[doc(hidden)] | ||||||
fn as_node_class() -> Option<ClassName> { | ||||||
PropertyHintInfo::object_as_node_class::<T>() | ||||||
} | ||||||
} | ||||||
|
||||||
PropertyHintInfo { hint, hint_string } | ||||||
impl<T: GodotClass> Default for OnEditor<Gd<T>> { | ||||||
fn default() -> Self { | ||||||
OnEditor::gd_invalid() | ||||||
} | ||||||
} | ||||||
|
||||||
impl<T> GodotConvert for OnEditor<Gd<T>> | ||||||
where | ||||||
T: GodotClass, | ||||||
Option<<Gd<T> as GodotConvert>::Via>: GodotType, | ||||||
{ | ||||||
type Via = Option<<Gd<T> as GodotConvert>::Via>; | ||||||
} | ||||||
|
||||||
impl<T> Var for OnEditor<Gd<T>> | ||||||
where | ||||||
T: GodotClass, | ||||||
OnEditor<Gd<T>>: GodotConvert<Via = Option<<Gd<T> as GodotConvert>::Via>>, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OnEditor<Gd<T>>: GodotConvert<Via = Option<<Gd<T> as GodotConvert>::Via>>, This is a rather complex bound. You could probably start it with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. even better, this whole nonsensical bound is dead code leftover 🙄. Same-y bound as for |
||||||
{ | ||||||
fn get_property(&self) -> Self::Via { | ||||||
OnEditor::<Gd<T>>::get_property_inner(self) | ||||||
} | ||||||
|
||||||
fn set_property(&mut self, value: Self::Via) { | ||||||
OnEditor::<Gd<T>>::set_property_inner(self, value) | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also here, use |
||||||
} | ||||||
|
||||||
impl<T> Export for OnEditor<Gd<T>> | ||||||
where | ||||||
T: GodotClass + Bounds<Exportable = bounds::Yes>, | ||||||
OnEditor<Gd<T>>: Var, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
{ | ||||||
fn export_hint() -> PropertyHintInfo { | ||||||
PropertyHintInfo::export_gd::<T>() | ||||||
} | ||||||
|
||||||
#[doc(hidden)] | ||||||
fn as_node_class() -> Option<ClassName> { | ||||||
T::inherits::<classes::Node>().then(|| T::class_name()) | ||||||
PropertyHintInfo::object_as_node_class::<T>() | ||||||
} | ||||||
} | ||||||
|
||||||
// Trait impls Property, Export and TypeStringHint for Option<Gd<T>> are covered by blanket impl for Option<T> | ||||||
|
||||||
impl<T: GodotClass> PartialEq for Gd<T> { | ||||||
/// ⚠️ Returns whether two `Gd` pointers point to the same object. | ||||||
/// | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here, no mixed bounds