Skip to content

Commit 88ebf37

Browse files
committed
Split ToGodot/FromGodot -> internal Engine* traits
Allows to treat user-defined #[func] conversions separately from engine-provided APIs (class methods, enums, ...). Removes #[func] support for u64.
1 parent 797b94e commit 88ebf37

File tree

13 files changed

+247
-100
lines changed

13 files changed

+247
-100
lines changed

godot-codegen/src/generator/enums.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,10 @@ pub fn make_enum_definition_with(
127127

128128
impl crate::meta::FromGodot for #name {
129129
fn try_from_godot(via: Self::Via) -> std::result::Result<Self, crate::meta::error::ConvertError> {
130+
// Pass i32/u64 enum/bitfield as i64 on the FFI layer. Only necessary for bitfields (u64).
131+
// Bitfields are cast to i64 for FFI, then reinterpreted in C++ as uint64_t.
130132
<Self as #engine_trait>::try_from_ord(via)
131-
.ok_or_else(|| crate::meta::error::FromGodotError::InvalidEnum.into_error(via))
133+
.ok_or_else(|| crate::meta::error::FromGodotError::InvalidEnum.into_error(via as i64))
132134
}
133135
}
134136
}

godot-core/src/builtin/variant/mod.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ use crate::builtin::{
1616
use crate::classes;
1717
use crate::meta::error::{ConvertError, FromVariantError};
1818
use crate::meta::{
19-
arg_into_ref, ffi_variant_type, ArrayElement, AsArg, ExtVariantType, FromGodot, GodotType,
20-
ToGodot,
19+
arg_into_ref, ffi_variant_type, ArrayElement, AsArg, EngineFromGodot, ExtVariantType,
20+
FromGodot, GodotType, ToGodot,
2121
};
2222

2323
mod impls;
@@ -118,14 +118,18 @@ impl Variant {
118118
try_from_variant_relaxed(self)
119119
}
120120

121+
pub(crate) fn engine_try_to_relaxed<T: EngineFromGodot>(&self) -> Result<T, ConvertError> {
122+
try_from_variant_relaxed(self)
123+
}
124+
121125
/// Helper function for relaxed variant conversion with panic on failure.
122126
/// Similar to [`to()`](Self::to) but uses relaxed conversion rules.
123127
pub(crate) fn to_relaxed_or_panic<T, F>(&self, context: F) -> T
124128
where
125-
T: FromGodot,
129+
T: EngineFromGodot,
126130
F: FnOnce() -> String,
127131
{
128-
self.try_to_relaxed::<T>()
132+
self.engine_try_to_relaxed::<T>()
129133
.unwrap_or_else(|err| panic!("{}: {err}", context()))
130134
}
131135

@@ -604,17 +608,17 @@ impl fmt::Debug for Variant {
604608
}
605609
}
606610

607-
fn try_from_variant_relaxed<T: FromGodot>(variant: &Variant) -> Result<T, ConvertError> {
611+
fn try_from_variant_relaxed<T: EngineFromGodot>(variant: &Variant) -> Result<T, ConvertError> {
608612
let from_type = variant.get_type();
609613
let to_type = match ffi_variant_type::<T>() {
610614
ExtVariantType::Variant => {
611615
// Converting to Variant always succeeds.
612-
return T::try_from_variant(variant);
616+
return T::engine_try_from_variant(variant);
613617
}
614618
ExtVariantType::Concrete(to_type) if from_type == to_type => {
615619
// If types are the same, use the regular conversion.
616620
// This is both an optimization (avoids more FFI) and ensures consistency between strict and relaxed conversions for identical types.
617-
return T::try_from_variant(variant);
621+
return T::engine_try_from_variant(variant);
618622
}
619623
ExtVariantType::Concrete(to_type) => to_type,
620624
};
@@ -649,7 +653,7 @@ fn try_from_variant_relaxed<T: FromGodot>(variant: &Variant) -> Result<T, Conver
649653

650654
// Try to convert the FFI types back to the user type. Can still fail, e.g. i64 -> i8.
651655
let via = <T::Via as GodotType>::try_from_ffi(ffi_result)?;
652-
let concrete = T::try_from_godot(via)?;
656+
let concrete = T::engine_try_from_godot(via)?;
653657

654658
Ok(concrete)
655659
}

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

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
use crate::builtin::{GString, NodePath, StringName, Variant};
99
use crate::meta::sealed::Sealed;
1010
use crate::meta::traits::{GodotFfiVariant, GodotNullableFfi};
11-
use crate::meta::{CowArg, FfiArg, GodotType, ObjectArg, ToGodot};
11+
use crate::meta::{CowArg, EngineToGodot, FfiArg, GodotType, ObjectArg, ToGodot};
1212
use crate::obj::{DynGd, Gd, GodotClass, Inherits};
1313

1414
/// Implicit conversions for arguments passed to Godot APIs.
@@ -595,21 +595,21 @@ pub trait ArgPassing: Sealed {
595595
#[doc(hidden)]
596596
fn ref_to_owned_via<T>(value: &T) -> T::Via
597597
where
598-
T: ToGodot<Pass = Self>,
598+
T: EngineToGodot<Pass = Self>,
599599
T::Via: Clone;
600600

601601
/// Convert to FFI repr in the most efficient way (move or borrow).
602602
#[doc(hidden)]
603603
fn ref_to_ffi<T>(value: &T) -> Self::FfiOutput<'_, T::Via>
604604
where
605-
T: ToGodot<Pass = Self>,
605+
T: EngineToGodot<Pass = Self>,
606606
T::Via: GodotType;
607607

608608
/// Convert to `Variant` in the most efficient way (move or borrow).
609609
#[doc(hidden)]
610610
fn ref_to_variant<T>(value: &T) -> Variant
611611
where
612-
T: ToGodot<Pass = Self>,
612+
T: EngineToGodot<Pass = Self>,
613613
{
614614
let ffi_result = Self::ref_to_ffi(value);
615615
GodotFfiVariant::ffi_to_variant(&ffi_result)
@@ -631,19 +631,19 @@ impl ArgPassing for ByValue {
631631

632632
fn ref_to_owned_via<T>(value: &T) -> T::Via
633633
where
634-
T: ToGodot<Pass = Self>,
634+
T: EngineToGodot<Pass = Self>,
635635
T::Via: Clone,
636636
{
637-
value.to_godot()
637+
value.engine_to_godot()
638638
}
639639

640640
fn ref_to_ffi<T>(value: &T) -> Self::FfiOutput<'_, T::Via>
641641
where
642-
T: ToGodot<Pass = Self>,
642+
T: EngineToGodot<Pass = Self>,
643643
T::Via: GodotType,
644644
{
645-
// For ByValue: to_godot() returns owned T::Via, move directly to FFI.
646-
GodotType::into_ffi(value.to_godot())
645+
// For ByValue: engine_to_godot() returns owned T::Via, move directly to FFI.
646+
GodotType::into_ffi(value.engine_to_godot())
647647
}
648648
}
649649

@@ -662,20 +662,20 @@ impl ArgPassing for ByRef {
662662

663663
fn ref_to_owned_via<T>(value: &T) -> T::Via
664664
where
665-
T: ToGodot<Pass = Self>,
665+
T: EngineToGodot<Pass = Self>,
666666
T::Via: Clone,
667667
{
668668
// For ByRef types, clone the reference to get owned value.
669-
value.to_godot().clone()
669+
value.engine_to_godot().clone()
670670
}
671671

672672
fn ref_to_ffi<T>(value: &T) -> <T::Via as GodotType>::ToFfi<'_>
673673
where
674-
T: ToGodot<Pass = Self>,
674+
T: EngineToGodot<Pass = Self>,
675675
T::Via: GodotType,
676676
{
677677
// Use by-ref conversion if possible, avoiding unnecessary clones when passing to FFI.
678-
GodotType::to_ffi(value.to_godot())
678+
GodotType::to_ffi(value.engine_to_godot())
679679
}
680680
}
681681

@@ -696,19 +696,19 @@ impl ArgPassing for ByObject {
696696

697697
fn ref_to_owned_via<T>(value: &T) -> T::Via
698698
where
699-
T: ToGodot<Pass = Self>,
699+
T: EngineToGodot<Pass = Self>,
700700
T::Via: Clone,
701701
{
702702
// For ByObject types, do like ByRef: clone the reference to get owned value.
703-
value.to_godot().clone()
703+
value.engine_to_godot().clone()
704704
}
705705

706706
fn ref_to_ffi<T>(value: &T) -> ObjectArg<'_>
707707
where
708-
T: ToGodot<Pass = Self>,
708+
T: EngineToGodot<Pass = Self>,
709709
T::Via: GodotType,
710710
{
711-
let obj_ref: &T::Via = value.to_godot(); // implements GodotType.
711+
let obj_ref: &T::Via = value.engine_to_godot(); // implements GodotType.
712712
obj_ref.as_object_arg()
713713
}
714714
}
@@ -745,20 +745,20 @@ where
745745
// return: T::Via = Option<U::Via>
746746
fn ref_to_owned_via<T>(value: &T) -> T::Via
747747
where
748-
T: ToGodot<Pass = Self>,
748+
T: EngineToGodot<Pass = Self>,
749749
T::Via: Clone,
750750
{
751-
value.to_godot_owned()
751+
value.engine_to_godot_owned()
752752
}
753753

754754
fn ref_to_ffi<T>(value: &T) -> Self::FfiOutput<'_, T::Via>
755755
where
756-
T: ToGodot<Pass = Self>,
756+
T: EngineToGodot<Pass = Self>,
757757
T::Via: GodotType,
758758
{
759759
// Reuse pattern from impl GodotType for Option<T>:
760760
// Convert Option<&Via> to Option<Via::ToFfi> and then flatten to Via::ToFfi with null handling.
761-
GodotNullableFfi::flatten_option(value.to_godot().map(|via_ref| via_ref.to_ffi()))
761+
GodotNullableFfi::flatten_option(value.engine_to_godot().map(|via_ref| via_ref.to_ffi()))
762762
}
763763
}
764764

godot-core/src/meta/error/convert_error.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,6 @@ pub(crate) enum FromFfiError {
277277
U16,
278278
I32,
279279
U32,
280-
U64,
281280
}
282281

283282
impl FromFfiError {
@@ -302,7 +301,6 @@ impl fmt::Display for FromFfiError {
302301
Self::U16 => "u16",
303302
Self::I32 => "i32",
304303
Self::U32 => "u32",
305-
Self::U64 => "u64",
306304
};
307305

308306
write!(f, "`{target}` cannot store the given value")
@@ -317,15 +315,15 @@ pub(crate) enum FromVariantError {
317315
actual: VariantType,
318316
},
319317

320-
/// Value cannot be represented in target type's domain.
321-
BadValue,
322-
323318
WrongClass {
324319
expected: ClassId,
325320
},
326321

327322
/// Variant holds an object which is no longer alive.
328323
DeadObject,
324+
//
325+
// BadValue: Value cannot be represented in target type's domain.
326+
// Used in the past for types like u64, with fallible FromVariant.
329327
}
330328

331329
impl FromVariantError {
@@ -344,7 +342,6 @@ impl fmt::Display for FromVariantError {
344342
// Note: wording is the same as in CallError::failed_param_conversion_engine()
345343
write!(f, "cannot convert from {actual:?} to {expected:?}")
346344
}
347-
Self::BadValue => write!(f, "value cannot be represented in target type's domain"),
348345
Self::WrongClass { expected } => {
349346
write!(f, "cannot convert to class {expected}")
350347
}

godot-core/src/meta/godot_convert/impls.rs

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use godot_ffi as sys;
99

1010
use crate::builtin::{Array, Variant};
1111
use crate::meta;
12-
use crate::meta::error::{ConvertError, ErrorKind, FromFfiError, FromVariantError};
12+
use crate::meta::error::{ConvertError, ErrorKind, FromFfiError};
1313
use crate::meta::{
1414
ArrayElement, ClassId, FromGodot, GodotConvert, GodotNullableFfi, GodotType, PropertyHintInfo,
1515
PropertyInfo, ToGodot,
@@ -331,47 +331,46 @@ impl GodotType for u64 {
331331
}
332332

333333
fn try_from_ffi(ffi: Self::Ffi) -> Result<Self, ConvertError> {
334-
// Ok(ffi as u64)
335-
Self::try_from(ffi).map_err(|_rust_err| FromFfiError::U64.into_error(ffi))
334+
Ok(ffi as u64)
336335
}
337336

338337
impl_godot_scalar!(@shared_fns; i64, sys::GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_UINT64);
339338
}
340339

340+
/// Test that `u64` does not implement `ToGodot/FromGodot`.
341+
///
342+
/// The impls are not provided since `u64` is not a natively supported type in GDScript (e.g. cannot be stored in a variant without altering the
343+
/// value). So `#[func]` does not support it. However, engine APIs may still need it, so there is codegen/macro support.
344+
///
345+
/// ```compile_fail
346+
/// # use godot::prelude::*;
347+
/// let value: u64 = 42;
348+
/// let variant = value.to_variant(); // Error: u64 does not implement ToGodot
349+
/// ```
341350
impl GodotConvert for u64 {
342351
type Via = u64;
343352
}
344353

345-
impl ToGodot for u64 {
354+
// u64 implements internal-only conversion traits for use in engine APIs and virtual methods.
355+
impl meta::EngineToGodot for u64 {
346356
type Pass = meta::ByValue;
347357

348-
fn to_godot(&self) -> Self::Via {
358+
fn engine_to_godot(&self) -> meta::ToArg<'_, Self::Via, Self::Pass> {
349359
*self
350360
}
351361

352-
fn to_variant(&self) -> Variant {
353-
// TODO panic doesn't fit the trait's infallibility too well; maybe in the future try_to_godot/try_to_variant() methods are possible.
354-
i64::try_from(*self)
355-
.map(|v| v.to_variant())
356-
.unwrap_or_else(|_| {
357-
panic!("to_variant(): u64 value {self} is not representable inside Variant, which can only store i64 integers")
358-
})
362+
fn engine_to_variant(&self) -> Variant {
363+
Variant::from(*self as i64) // Treat as i64.
359364
}
360365
}
361366

362-
impl FromGodot for u64 {
363-
fn try_from_godot(via: Self::Via) -> Result<Self, ConvertError> {
367+
impl meta::EngineFromGodot for u64 {
368+
fn engine_try_from_godot(via: Self::Via) -> Result<Self, ConvertError> {
364369
Ok(via)
365370
}
366371

367-
fn try_from_variant(variant: &Variant) -> Result<Self, ConvertError> {
368-
// Fail for values that are not representable as u64.
369-
let value = variant.try_to::<i64>()?;
370-
371-
u64::try_from(value).map_err(|_rust_err| {
372-
// TODO maybe use better error enumerator
373-
FromVariantError::BadValue.into_error(value)
374-
})
372+
fn engine_try_from_variant(variant: &Variant) -> Result<Self, ConvertError> {
373+
variant.try_to::<i64>().map(|i| i as u64)
375374
}
376375
}
377376

0 commit comments

Comments
 (0)