Skip to content
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

Pass-by-ref for non-Copy builtins (backend) #906

Merged
merged 3 commits into from
Sep 25, 2024
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
11 changes: 6 additions & 5 deletions godot-core/src/builtin/collections/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::builtin::*;
use crate::meta::error::{ConvertError, FromGodotError, FromVariantError};
use crate::meta::{
ArrayElement, ArrayTypeInfo, FromGodot, GodotConvert, GodotFfiVariant, GodotType,
PropertyHintInfo, ToGodot,
PropertyHintInfo, RefArg, ToGodot,
};
use crate::registry::property::{Export, Var};
use godot_ffi as sys;
Expand Down Expand Up @@ -1047,10 +1047,11 @@ impl<T: ArrayElement> Drop for Array<T> {
impl<T: ArrayElement> GodotType for Array<T> {
type Ffi = Self;

fn to_ffi(&self) -> Self::Ffi {
// SAFETY: we may pass type-transmuted arrays to FFI (e.g. Array<T> as Array<Variant>). This would fail the regular
// type-check in clone(), so we disable it. Type invariants are upheld by the "front-end" APIs.
unsafe { self.clone_unchecked() }
type ToFfi<'f> = RefArg<'f, Array<T>>
where Self: 'f;

fn to_ffi(&self) -> Self::ToFfi<'_> {
RefArg::new(self)
}

fn into_ffi(self) -> Self::Ffi {
Expand Down
69 changes: 47 additions & 22 deletions godot-core/src/builtin/variant/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use super::*;
use crate::builtin::*;
use crate::global;
use crate::meta::error::{ConvertError, FromVariantError};
use crate::meta::{ArrayElement, GodotFfiVariant, GodotType, PropertyHintInfo, PropertyInfo};
use crate::meta::{
ArrayElement, GodotFfiVariant, GodotType, PropertyHintInfo, PropertyInfo, RefArg,
};
use godot_ffi as sys;
// For godot-cpp, see https://github.com/godotengine/godot-cpp/blob/master/include/godot_cpp/core/type_info.hpp.

Expand All @@ -22,7 +24,15 @@ use godot_ffi as sys;
//
// Therefore, we can use `init` to indicate when it must be initialized in 4.0.
macro_rules! impl_ffi_variant {
($T:ty, $from_fn:ident, $to_fn:ident $(; $godot_type_name:ident)?) => {
(ref $T:ty, $from_fn:ident, $to_fn:ident $(; $GodotTy:ident)?) => {
impl_ffi_variant!(@impls by_ref; $T, $from_fn, $to_fn $(; $GodotTy)?);
};
($T:ty, $from_fn:ident, $to_fn:ident $(; $GodotTy:ident)?) => {
impl_ffi_variant!(@impls by_val; $T, $from_fn, $to_fn $(; $GodotTy)?);
};

// Implementations
(@impls $by_ref_or_val:ident; $T:ty, $from_fn:ident, $to_fn:ident $(; $GodotTy:ident)?) => {
impl GodotFfiVariant for $T {
fn ffi_to_variant(&self) -> Variant {
let variant = unsafe {
Expand Down Expand Up @@ -58,10 +68,7 @@ macro_rules! impl_ffi_variant {

impl GodotType for $T {
type Ffi = Self;

fn to_ffi(&self) -> Self::Ffi {
self.clone()
}
impl_ffi_variant!(@assoc_to_ffi $by_ref_or_val);

fn into_ffi(self) -> Self::Ffi {
self
Expand All @@ -71,7 +78,7 @@ macro_rules! impl_ffi_variant {
Ok(ffi)
}

impl_ffi_variant!(@godot_type_name $T $(, $godot_type_name)?);
impl_ffi_variant!(@godot_type_name $T $(, $GodotTy)?);
}

impl ArrayElement for $T {}
Expand All @@ -88,6 +95,22 @@ macro_rules! impl_ffi_variant {
stringify!($godot_type_name).into()
}
};

(@assoc_to_ffi by_ref) => {
type ToFfi<'a> = RefArg<'a, Self>;

fn to_ffi(&self) -> Self::ToFfi<'_> {
RefArg::new(self)
}
};

(@assoc_to_ffi by_val) => {
type ToFfi<'a> = Self;

fn to_ffi(&self) -> Self::ToFfi<'_> {
self.clone()
}
};
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -116,17 +139,17 @@ mod impls {
impl_ffi_variant!(GString, string_to_variant, string_from_variant; String);
impl_ffi_variant!(StringName, string_name_to_variant, string_name_from_variant);
impl_ffi_variant!(NodePath, node_path_to_variant, node_path_from_variant);
impl_ffi_variant!(PackedByteArray, packed_byte_array_to_variant, packed_byte_array_from_variant);
impl_ffi_variant!(PackedInt32Array, packed_int32_array_to_variant, packed_int32_array_from_variant);
impl_ffi_variant!(PackedInt64Array, packed_int64_array_to_variant, packed_int64_array_from_variant);
impl_ffi_variant!(PackedFloat32Array, packed_float32_array_to_variant, packed_float32_array_from_variant);
impl_ffi_variant!(PackedFloat64Array, packed_float64_array_to_variant, packed_float64_array_from_variant);
impl_ffi_variant!(PackedStringArray, packed_string_array_to_variant, packed_string_array_from_variant);
impl_ffi_variant!(PackedVector2Array, packed_vector2_array_to_variant, packed_vector2_array_from_variant);
impl_ffi_variant!(PackedVector3Array, packed_vector3_array_to_variant, packed_vector3_array_from_variant);
impl_ffi_variant!(ref PackedByteArray, packed_byte_array_to_variant, packed_byte_array_from_variant);
impl_ffi_variant!(ref PackedInt32Array, packed_int32_array_to_variant, packed_int32_array_from_variant);
impl_ffi_variant!(ref PackedInt64Array, packed_int64_array_to_variant, packed_int64_array_from_variant);
impl_ffi_variant!(ref PackedFloat32Array, packed_float32_array_to_variant, packed_float32_array_from_variant);
impl_ffi_variant!(ref PackedFloat64Array, packed_float64_array_to_variant, packed_float64_array_from_variant);
impl_ffi_variant!(ref PackedStringArray, packed_string_array_to_variant, packed_string_array_from_variant);
impl_ffi_variant!(ref PackedVector2Array, packed_vector2_array_to_variant, packed_vector2_array_from_variant);
impl_ffi_variant!(ref PackedVector3Array, packed_vector3_array_to_variant, packed_vector3_array_from_variant);
#[cfg(since_api = "4.3")]
impl_ffi_variant!(PackedVector4Array, packed_vector4_array_to_variant, packed_vector4_array_from_variant);
impl_ffi_variant!(PackedColorArray, packed_color_array_to_variant, packed_color_array_from_variant);
impl_ffi_variant!(ref PackedVector4Array, packed_vector4_array_to_variant, packed_vector4_array_from_variant);
impl_ffi_variant!(ref PackedColorArray, packed_color_array_to_variant, packed_color_array_from_variant);
impl_ffi_variant!(Plane, plane_to_variant, plane_from_variant);
impl_ffi_variant!(Projection, projection_to_variant, projection_from_variant);
impl_ffi_variant!(Rid, rid_to_variant, rid_from_variant; RID);
Expand All @@ -135,7 +158,7 @@ mod impls {
impl_ffi_variant!(Signal, signal_to_variant, signal_from_variant);
impl_ffi_variant!(Transform2D, transform_2d_to_variant, transform_2d_from_variant);
impl_ffi_variant!(Transform3D, transform_3d_to_variant, transform_3d_from_variant);
impl_ffi_variant!(Dictionary, dictionary_to_variant, dictionary_from_variant);
impl_ffi_variant!(ref Dictionary, dictionary_to_variant, dictionary_from_variant);

}

Expand All @@ -162,9 +185,10 @@ impl GodotFfiVariant for () {
}

impl GodotType for () {
type Ffi = Self;
type Ffi = ();
type ToFfi<'a> = ();

fn to_ffi(&self) -> Self::Ffi {}
fn to_ffi(&self) -> Self::ToFfi<'_> {}

fn into_ffi(self) -> Self::Ffi {}

Expand All @@ -189,9 +213,10 @@ impl GodotFfiVariant for Variant {

impl GodotType for Variant {
type Ffi = Variant;
type ToFfi<'a> = RefArg<'a, Variant>;

fn to_ffi(&self) -> Self::Ffi {
self.clone()
fn to_ffi(&self) -> Self::ToFfi<'_> {
RefArg::new(self)
}

fn into_ffi(self) -> Self::Ffi {
Expand Down
6 changes: 3 additions & 3 deletions godot-core/src/global/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,19 @@ use crate::obj::Gd;
// Reminder: remove #![allow(deprecated)] in utilities.test along with the below functions.

#[deprecated = "Instance utilities in `godot::global` will be removed. Use methods on `Gd` and `InstanceId` instead.\n\
For detailed reasons, see https://github.com/godot-rust/gdext/pull/892."]
For detailed reasons, see https://github.com/godot-rust/gdext/pull/901."]
pub fn instance_from_id(instance_id: i64) -> Option<Gd<crate::classes::Object>> {
crate::gen::utilities::instance_from_id(instance_id)
}

#[deprecated = "Instance utilities in `godot::global` will be removed. Use methods on `Gd` and `InstanceId` instead.\n\
For detailed reasons, see https://github.com/godot-rust/gdext/pull/892."]
For detailed reasons, see https://github.com/godot-rust/gdext/pull/901."]
pub fn is_instance_valid(instance: Variant) -> bool {
crate::gen::utilities::is_instance_valid(&instance)
}

#[deprecated = "Instance utilities in `godot::global` will be removed. Use methods on `Gd` and `InstanceId` instead.\n\
For detailed reasons, see https://github.com/godot-rust/gdext/pull/892."]
For detailed reasons, see https://github.com/godot-rust/gdext/pull/901."]
pub fn is_instance_id_valid(instance_id: i64) -> bool {
crate::gen::utilities::is_instance_id_valid(instance_id)
}
20 changes: 15 additions & 5 deletions godot-core/src/meta/godot_convert/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@ impl<T> GodotType for Option<T>
where
T: GodotType,
T::Ffi: GodotNullableFfi,
for<'f> T::ToFfi<'f>: GodotNullableFfi,
{
type Ffi = T::Ffi;

fn to_ffi(&self) -> Self::Ffi {
type ToFfi<'f> = T::ToFfi<'f>;

fn to_ffi(&self) -> Self::ToFfi<'_> {
GodotNullableFfi::flatten_option(self.as_ref().map(|t| t.to_ffi()))
}

Expand Down Expand Up @@ -91,7 +94,11 @@ where
impl<T: ToGodot> ToGodot for Option<T>
where
Option<T::Via>: GodotType,
for<'f> T::ToVia<'f>: GodotType<Ffi: GodotNullableFfi>,
for<'v, 'f> T::ToVia<'v>: GodotType<
// Associated types need to be nullable.
Ffi: GodotNullableFfi,
ToFfi<'f>: GodotNullableFfi,
>,
{
type ToVia<'v> = Option<T::ToVia<'v>>
// type ToVia<'v> = Self::Via
Expand Down Expand Up @@ -155,8 +162,9 @@ macro_rules! impl_godot_scalar {
($T:ty as $Via:ty, $err:path, $param_metadata:expr) => {
impl GodotType for $T {
type Ffi = $Via;
type ToFfi<'f> = $Via;

fn to_ffi(&self) -> Self::Ffi {
fn to_ffi(&self) -> Self::ToFfi<'_> {
(*self).into()
}

Expand Down Expand Up @@ -188,8 +196,9 @@ macro_rules! impl_godot_scalar {
($T:ty as $Via:ty, $param_metadata:expr; lossy) => {
impl GodotType for $T {
type Ffi = $Via;
type ToFfi<'f> = $Via;

fn to_ffi(&self) -> Self::Ffi {
fn to_ffi(&self) -> Self::ToFfi<'_> {
*self as $Via
}

Expand Down Expand Up @@ -289,8 +298,9 @@ impl_godot_scalar!(

impl GodotType for u64 {
type Ffi = i64;
type ToFfi<'f> = i64;

fn to_ffi(&self) -> Self::Ffi {
fn to_ffi(&self) -> Self::ToFfi<'_> {
*self as i64
}

Expand Down
13 changes: 4 additions & 9 deletions godot-core/src/meta/godot_convert/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pub trait GodotConvert {
///
/// Please read the [`godot::meta` module docs][crate::meta] for further information about conversions.
pub trait ToGodot: Sized + GodotConvert {
/// Target type of [`to_godot()`](ToGodot::to_godot), which differs from [`Via`](GodotConvert::Via) for pass-by-reference types.
type ToVia<'v>: GodotType
where
Self: 'v;
Expand Down Expand Up @@ -95,16 +96,10 @@ pub trait FromGodot: Sized + GodotConvert {
}
}

// Note: removing the implicit lifetime (by taking value: T instead of &T) causes issues due to allegedly returning a lifetime
// to a local variable, even though the result Ffi is 'static by definition.
#[allow(clippy::needless_lifetimes)] // eliding causes error: missing generics for associated type `godot_convert::ToGodot::ToVia`
pub(crate) fn into_ffi<'v, T: ToGodot>(value: &'v T) -> <T::ToVia<'v> as GodotType>::Ffi {
let by_ref = value.to_godot();
by_ref.to_ffi()
}

pub(crate) fn into_ffi_variant<T: ToGodot>(value: &T) -> Variant {
GodotFfiVariant::ffi_to_variant(&into_ffi(value))
let via = value.to_godot();
let ffi = via.to_ffi();
GodotFfiVariant::ffi_to_variant(&ffi)
}

pub(crate) fn try_from_ffi<T: FromGodot>(
Expand Down
Loading
Loading