Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion godot-codegen/src/generator/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,10 @@ pub fn make_enum_definition_with(

impl crate::meta::FromGodot for #name {
fn try_from_godot(via: Self::Via) -> std::result::Result<Self, crate::meta::error::ConvertError> {
// Pass i32/u64 enum/bitfield as i64 on the FFI layer. Only necessary for bitfields (u64).
// Bitfields are cast to i64 for FFI, then reinterpreted in C++ as uint64_t.
<Self as #engine_trait>::try_from_ord(via)
.ok_or_else(|| crate::meta::error::FromGodotError::InvalidEnum.into_error(via))
.ok_or_else(|| crate::meta::error::FromGodotError::InvalidEnum.into_error(via as i64))
}
}
}
Expand Down
20 changes: 12 additions & 8 deletions godot-core/src/builtin/variant/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use crate::builtin::{
use crate::classes;
use crate::meta::error::{ConvertError, FromVariantError};
use crate::meta::{
arg_into_ref, ffi_variant_type, ArrayElement, AsArg, ExtVariantType, FromGodot, GodotType,
ToGodot,
arg_into_ref, ffi_variant_type, ArrayElement, AsArg, EngineFromGodot, ExtVariantType,
FromGodot, GodotType, ToGodot,
};

mod impls;
Expand Down Expand Up @@ -118,14 +118,18 @@ impl Variant {
try_from_variant_relaxed(self)
}

pub(crate) fn engine_try_to_relaxed<T: EngineFromGodot>(&self) -> Result<T, ConvertError> {
try_from_variant_relaxed(self)
}

/// Helper function for relaxed variant conversion with panic on failure.
/// Similar to [`to()`](Self::to) but uses relaxed conversion rules.
pub(crate) fn to_relaxed_or_panic<T, F>(&self, context: F) -> T
where
T: FromGodot,
T: EngineFromGodot,
F: FnOnce() -> String,
{
self.try_to_relaxed::<T>()
self.engine_try_to_relaxed::<T>()
.unwrap_or_else(|err| panic!("{}: {err}", context()))
}

Expand Down Expand Up @@ -604,17 +608,17 @@ impl fmt::Debug for Variant {
}
}

fn try_from_variant_relaxed<T: FromGodot>(variant: &Variant) -> Result<T, ConvertError> {
fn try_from_variant_relaxed<T: EngineFromGodot>(variant: &Variant) -> Result<T, ConvertError> {
let from_type = variant.get_type();
let to_type = match ffi_variant_type::<T>() {
ExtVariantType::Variant => {
// Converting to Variant always succeeds.
return T::try_from_variant(variant);
return T::engine_try_from_variant(variant);
}
ExtVariantType::Concrete(to_type) if from_type == to_type => {
// If types are the same, use the regular conversion.
// This is both an optimization (avoids more FFI) and ensures consistency between strict and relaxed conversions for identical types.
return T::try_from_variant(variant);
return T::engine_try_from_variant(variant);
}
ExtVariantType::Concrete(to_type) => to_type,
};
Expand Down Expand Up @@ -649,7 +653,7 @@ fn try_from_variant_relaxed<T: FromGodot>(variant: &Variant) -> Result<T, Conver

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

Ok(concrete)
}
Expand Down
42 changes: 21 additions & 21 deletions godot-core/src/meta/args/as_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use crate::builtin::{GString, NodePath, StringName, Variant};
use crate::meta::sealed::Sealed;
use crate::meta::traits::{GodotFfiVariant, GodotNullableFfi};
use crate::meta::{CowArg, FfiArg, GodotType, ObjectArg, ToGodot};
use crate::meta::{CowArg, EngineToGodot, FfiArg, GodotType, ObjectArg, ToGodot};
use crate::obj::{DynGd, Gd, GodotClass, Inherits};

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

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

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

fn ref_to_owned_via<T>(value: &T) -> T::Via
where
T: ToGodot<Pass = Self>,
T: EngineToGodot<Pass = Self>,
T::Via: Clone,
{
value.to_godot()
value.engine_to_godot()
}

fn ref_to_ffi<T>(value: &T) -> Self::FfiOutput<'_, T::Via>
where
T: ToGodot<Pass = Self>,
T: EngineToGodot<Pass = Self>,
T::Via: GodotType,
{
// For ByValue: to_godot() returns owned T::Via, move directly to FFI.
GodotType::into_ffi(value.to_godot())
// For ByValue: engine_to_godot() returns owned T::Via, move directly to FFI.
GodotType::into_ffi(value.engine_to_godot())
}
}

Expand All @@ -662,20 +662,20 @@ impl ArgPassing for ByRef {

fn ref_to_owned_via<T>(value: &T) -> T::Via
where
T: ToGodot<Pass = Self>,
T: EngineToGodot<Pass = Self>,
T::Via: Clone,
{
// For ByRef types, clone the reference to get owned value.
value.to_godot().clone()
value.engine_to_godot().clone()
}

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

Expand All @@ -696,19 +696,19 @@ impl ArgPassing for ByObject {

fn ref_to_owned_via<T>(value: &T) -> T::Via
where
T: ToGodot<Pass = Self>,
T: EngineToGodot<Pass = Self>,
T::Via: Clone,
{
// For ByObject types, do like ByRef: clone the reference to get owned value.
value.to_godot().clone()
value.engine_to_godot().clone()
}

fn ref_to_ffi<T>(value: &T) -> ObjectArg<'_>
where
T: ToGodot<Pass = Self>,
T: EngineToGodot<Pass = Self>,
T::Via: GodotType,
{
let obj_ref: &T::Via = value.to_godot(); // implements GodotType.
let obj_ref: &T::Via = value.engine_to_godot(); // implements GodotType.
obj_ref.as_object_arg()
}
}
Expand Down Expand Up @@ -745,20 +745,20 @@ where
// return: T::Via = Option<U::Via>
fn ref_to_owned_via<T>(value: &T) -> T::Via
where
T: ToGodot<Pass = Self>,
T: EngineToGodot<Pass = Self>,
T::Via: Clone,
{
value.to_godot_owned()
value.engine_to_godot_owned()
}

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

Expand Down
9 changes: 3 additions & 6 deletions godot-core/src/meta/error/convert_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,6 @@ pub(crate) enum FromFfiError {
U16,
I32,
U32,
U64,
}

impl FromFfiError {
Expand All @@ -317,7 +316,6 @@ impl fmt::Display for FromFfiError {
Self::U16 => "u16",
Self::I32 => "i32",
Self::U32 => "u32",
Self::U64 => "u64",
};

write!(f, "`{target}` cannot store the given value")
Expand All @@ -332,15 +330,15 @@ pub(crate) enum FromVariantError {
actual: VariantType,
},

/// Value cannot be represented in target type's domain.
BadValue,

WrongClass {
expected: ClassId,
},

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

impl FromVariantError {
Expand All @@ -359,7 +357,6 @@ impl fmt::Display for FromVariantError {
// Note: wording is the same as in CallError::failed_param_conversion_engine()
write!(f, "cannot convert from {actual:?} to {expected:?}")
}
Self::BadValue => write!(f, "value cannot be represented in target type's domain"),
Self::WrongClass { expected } => {
write!(f, "cannot convert to class {expected}")
}
Expand Down
43 changes: 21 additions & 22 deletions godot-core/src/meta/godot_convert/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use godot_ffi as sys;

use crate::builtin::{Array, Variant};
use crate::meta;
use crate::meta::error::{ConvertError, ErrorKind, FromFfiError, FromVariantError};
use crate::meta::error::{ConvertError, ErrorKind, FromFfiError};
use crate::meta::{
ArrayElement, ClassId, FromGodot, GodotConvert, GodotNullableFfi, GodotType, PropertyHintInfo,
PropertyInfo, ToGodot,
Expand Down Expand Up @@ -331,47 +331,46 @@ impl GodotType for u64 {
}

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

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

/// Test that `u64` does not implement `ToGodot/FromGodot`.
///
/// 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
/// value). So `#[func]` does not support it. However, engine APIs may still need it, so there is codegen/macro support.
///
/// ```compile_fail
/// # use godot::prelude::*;
/// let value: u64 = 42;
/// let variant = value.to_variant(); // Error: u64 does not implement ToGodot
/// ```
impl GodotConvert for u64 {
type Via = u64;
}

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

fn to_godot(&self) -> Self::Via {
fn engine_to_godot(&self) -> meta::ToArg<'_, Self::Via, Self::Pass> {
*self
}

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

impl FromGodot for u64 {
fn try_from_godot(via: Self::Via) -> Result<Self, ConvertError> {
impl meta::EngineFromGodot for u64 {
fn engine_try_from_godot(via: Self::Via) -> Result<Self, ConvertError> {
Ok(via)
}

fn try_from_variant(variant: &Variant) -> Result<Self, ConvertError> {
// Fail for values that are not representable as u64.
let value = variant.try_to::<i64>()?;

u64::try_from(value).map_err(|_rust_err| {
// TODO maybe use better error enumerator
FromVariantError::BadValue.into_error(value)
})
fn engine_try_from_variant(variant: &Variant) -> Result<Self, ConvertError> {
variant.try_to::<i64>().map(|i| i as u64)
}
}

Expand Down
Loading
Loading