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

Cleanup around godot::meta argument conversions #948

Merged
merged 6 commits into from
Nov 10, 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
5 changes: 2 additions & 3 deletions godot-codegen/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ pub fn make_imports() -> TokenStream {
quote! {
use godot_ffi as sys;
use crate::builtin::*;
use crate::meta::{AsArg, ClassName, CowArg, PtrcallSignatureTuple, RefArg, VarcallSignatureTuple};
use crate::meta::{AsArg, AsObjectArg, ClassName, CowArg, ObjectArg, ObjectCow, PtrcallSignatureTuple, RefArg, VarcallSignatureTuple};
use crate::classes::native::*;
use crate::classes::Object;
use crate::obj::{Gd, AsObjectArg};
use crate::obj::object_arg::{ObjectArg, ObjectCow};
use crate::obj::Gd;
use crate::sys::GodotFfi as _;
}
}
Expand Down
9 changes: 5 additions & 4 deletions godot-core/src/builtin/callable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ impl Callable {
pub fn from_object_method<T, S>(object: &Gd<T>, method_name: S) -> Self
where
T: GodotClass, // + Inherits<Object>,
S: Into<StringName>,
S: meta::AsArg<StringName>,
{
// upcast not needed
let method = method_name.into();
meta::arg_into_ref!(method_name);

unsafe {
Self::new_with_uninit(|self_ptr| {
let ctor = sys::builtin_fn!(callable_from_object_method);
let raw = object.to_ffi();
let args = [raw.as_arg_ptr(), method.sys()];
let args = [raw.as_arg_ptr(), method_name.sys()];
ctor(self_ptr, args.as_ptr());
})
}
Expand Down Expand Up @@ -340,6 +340,7 @@ impl fmt::Display for Callable {
#[cfg(since_api = "4.2")]
use custom_callable::*;

use crate::meta;
#[cfg(since_api = "4.2")]
pub use custom_callable::RustCallable;

Expand Down
8 changes: 4 additions & 4 deletions godot-core/src/builtin/collections/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use crate::builtin::*;
use crate::meta;
use crate::meta::error::{ConvertError, FromGodotError, FromVariantError};
use crate::meta::{
ApiParam, ArrayElement, ArrayTypeInfo, AsArg, CowArg, FromGodot, GodotConvert, GodotFfiVariant,
GodotType, PropertyHintInfo, RefArg, ToGodot,
ArrayElement, ArrayTypeInfo, AsArg, CowArg, FromGodot, GodotConvert, GodotFfiVariant,
GodotType, ParamType, PropertyHintInfo, RefArg, ToGodot,
};
use crate::registry::property::{Export, Var};
use godot_ffi as sys;
Expand Down Expand Up @@ -944,7 +944,7 @@ impl<'r, T: ArrayElement> AsArg<Array<T>> for &'r Array<T> {
}
}

impl<T: ArrayElement> ApiParam for Array<T> {
impl<T: ArrayElement> ParamType for Array<T> {
type Arg<'v> = CowArg<'v, Self>;

fn owned_to_arg<'v>(self) -> Self::Arg<'v> {
Expand Down Expand Up @@ -1221,7 +1221,7 @@ impl<T: ArrayElement + ToGodot> Extend<T> for Array<T> {
// A faster implementation using `resize()` and direct pointer writes might still be possible.
// Note that this could technically also use iter(), since no moves need to happen (however Extend requires IntoIterator).
for item in iter.into_iter() {
self.push(ApiParam::owned_to_arg(item));
self.push(ParamType::owned_to_arg(item));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/collections/packed_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ macro_rules! impl_packed_array {
// A faster implementation using `resize()` and direct pointer writes might still be
// possible.
for item in iter.into_iter() {
self.push(meta::ApiParam::owned_to_arg(item));
self.push(meta::ParamType::owned_to_arg(item));
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions godot-core/src/builtin/signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use godot_ffi as sys;
use crate::builtin::{inner, Array, Callable, Dictionary, StringName, Variant};
use crate::classes::Object;
use crate::global::Error;
use crate::meta;
use crate::meta::{FromGodot, GodotType, ToGodot};
use crate::obj::bounds::DynMemory;
use crate::obj::{Bounds, Gd, GodotClass, InstanceId};
Expand Down Expand Up @@ -40,9 +41,10 @@ impl Signal {
pub fn from_object_signal<T, S>(object: &Gd<T>, signal_name: S) -> Self
where
T: GodotClass,
S: Into<StringName>,
S: meta::AsArg<StringName>,
{
let signal_name = signal_name.into();
meta::arg_into_ref!(signal_name);

unsafe {
Self::new_with_uninit(|self_ptr| {
let ctor = sys::builtin_fn!(signal_from_object_signal);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ use std::ffi::CStr;
/// - `&T` for by-ref builtins: `GString`, `Array`, `Dictionary`, `Packed*Array`, `Variant`...
/// - `&str`, `&String` additionally for string types `GString`, `StringName`, `NodePath`.
///
/// See also the [`AsObjectArg`][crate::meta::AsObjectArg] trait which is specialized for object arguments. It may be merged with `AsArg`
/// in the future.
///
/// # Pass by value
/// Implicitly converting from `T` for by-ref builtins is explicitly not supported. This emphasizes that there is no need to consume the object,
/// thus discourages unnecessary cloning.
///
/// If you need to pass owned values in generic code, you can use [`ApiParam::owned_to_arg()`].
/// If you need to pass owned values in generic code, you can use [`ParamType::owned_to_arg()`].
///
/// # Performance for strings
/// Godot has three string types: [`GString`], [`StringName`] and [`NodePath`]. Conversions between those three, as well as between `String` and
Expand All @@ -43,19 +46,19 @@ use std::ffi::CStr;
/// you can only forward it as-is to a Godot API -- there are no stable APIs to access the inner object yet.
///
/// Furthermore, there is currently no benefit in implementing `AsArg` for your own types, as it's only used by Godot APIs which don't accept
/// custom types. Classes are already supported through upcasting and [`AsObjectArg`][crate::obj::AsObjectArg].
/// custom types. Classes are already supported through upcasting and [`AsObjectArg`][crate::meta::AsObjectArg].
#[diagnostic::on_unimplemented(
message = "Argument of type `{Self}` cannot be passed to an `impl AsArg<{T}>` parameter",
note = "If you pass by value, consider borrowing instead.",
note = "GString/StringName/NodePath aren't implicitly convertible for performance reasons; use their `arg()` method.",
note = "See also `AsArg` docs: https://godot-rust.github.io/docs/gdext/master/godot/meta/trait.AsArg.html"
)]
pub trait AsArg<T: ApiParam>
pub trait AsArg<T: ParamType>
where
Self: Sized,
{
#[doc(hidden)]
fn into_arg<'r>(self) -> <T as ApiParam>::Arg<'r>
fn into_arg<'r>(self) -> <T as ParamType>::Arg<'r>
where
Self: 'r;
}
Expand All @@ -77,7 +80,7 @@ macro_rules! arg_into_ref {
};
($arg_variable:ident: $T:ty) => {
let $arg_variable = $arg_variable.into_arg();
let $arg_variable: &$T = $crate::meta::ApiParam::arg_to_ref(&$arg_variable);
let $arg_variable: &$T = $crate::meta::ParamType::arg_to_ref(&$arg_variable);
};
}

Expand All @@ -89,21 +92,21 @@ macro_rules! arg_into_owned {
($arg_variable:ident) => {
let $arg_variable = $arg_variable.into_arg();
let $arg_variable = $arg_variable.cow_into_owned();
// cow_into_owned() is not yet used generically; could be abstracted in ApiParam::arg_to_owned() as well.
// cow_into_owned() is not yet used generically; could be abstracted in ParamType::arg_to_owned() as well.
};
}

#[macro_export]
macro_rules! impl_asarg_by_value {
($T:ty) => {
impl $crate::meta::AsArg<$T> for $T {
fn into_arg<'r>(self) -> <$T as $crate::meta::ApiParam>::Arg<'r> {
fn into_arg<'r>(self) -> <$T as $crate::meta::ParamType>::Arg<'r> {
// Moves value (but typically a Copy type).
self
}
}

impl $crate::meta::ApiParam for $T {
impl $crate::meta::ParamType for $T {
type Arg<'v> = $T;

fn owned_to_arg<'v>(self) -> Self::Arg<'v> {
Expand All @@ -128,15 +131,15 @@ macro_rules! impl_asarg_by_ref {
// Thus, keep `where` on same line.
// type ArgType<'v> = &'v $T where Self: 'v;

fn into_arg<'cow>(self) -> <$T as $crate::meta::ApiParam>::Arg<'cow>
fn into_arg<'cow>(self) -> <$T as $crate::meta::ParamType>::Arg<'cow>
where
'r: 'cow, // Original reference must be valid for at least as long as the returned cow.
{
$crate::meta::CowArg::Borrowed(self)
}
}

impl $crate::meta::ApiParam for $T {
impl $crate::meta::ParamType for $T {
type Arg<'v> = $crate::meta::CowArg<'v, $T>;

fn owned_to_arg<'v>(self) -> Self::Arg<'v> {
Expand All @@ -157,11 +160,11 @@ macro_rules! declare_arg_method {
///
/// # Generic bounds
/// The bounds are implementation-defined and may change at any time. Do not use this function in a generic context requiring `T`
/// -- use the `From` trait or [`ApiParam`][crate::meta::ApiParam] in that case.
/// -- use the `From` trait or [`ParamType`][crate::meta::ParamType] in that case.
pub fn arg<T>(&self) -> impl $crate::meta::AsArg<T>
where
for<'a> T: From<&'a Self>
+ $crate::meta::ApiParam<Arg<'a> = $crate::meta::CowArg<'a, T>>
+ $crate::meta::ParamType<Arg<'a> = $crate::meta::CowArg<'a, T>>
+ 'a,
{
$crate::meta::CowArg::Owned(T::from(self))
Expand All @@ -178,7 +181,7 @@ macro_rules! declare_arg_method {
/// This is necessary for packed array dispatching to different "inner" backend signatures.
impl<'a, T> AsArg<T> for CowArg<'a, T>
where
for<'r> T: ApiParam<Arg<'r> = CowArg<'r, T>> + 'r,
for<'r> T: ParamType<Arg<'r> = CowArg<'r, T>> + 'r,
{
fn into_arg<'r>(self) -> CowArg<'r, T>
where
Expand All @@ -188,7 +191,7 @@ where
}
}

// impl<'a, T> ApiParam for CowArg<'a, T> {
// impl<'a, T> ParamType for CowArg<'a, T> {
// type Type<'v> = CowArg<'v, T>
// where Self: 'v;
// }
Expand Down Expand Up @@ -250,7 +253,7 @@ impl AsArg<NodePath> for &String {
// ----------------------------------------------------------------------------------------------------------------------------------------------

/// Implemented for all parameter types `T` that are allowed to receive [impl `AsArg<T>`][AsArg].
pub trait ApiParam: GodotType
pub trait ParamType: GodotType
// GodotType bound not required right now, but conceptually should always be the case.
{
/// Canonical argument passing type, either `T` or an internally-used CoW type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl<'r, T> CowArg<'r, T> {
}
}

/// Exists for polymorphism in [`crate::meta::ApiParam`].
/// Exists for polymorphism in [`crate::meta::ParamType`].
///
/// Necessary for generics in e.g. `Array<T>`, when accepting `impl AsArg<T>` parameters.
///
Expand Down
37 changes: 37 additions & 0 deletions godot-core/src/meta/args/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (c) godot-rust; Bromeon and contributors.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

mod as_arg;
mod cow_arg;
mod object_arg;
mod ref_arg;

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Public APIs

pub use as_arg::{AsArg, ParamType};
pub use object_arg::AsObjectArg;
pub use ref_arg::RefArg;

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Internal APIs

// Solely public for itest/convert_test.rs.
#[cfg(feature = "trace")]
#[doc(hidden)]
pub use cow_arg::CowArg;
#[cfg(not(feature = "trace"))]
pub(crate) use cow_arg::CowArg;

#[allow(unused_imports)] // ObjectCow is used in generated code.
pub(crate) use object_arg::{ObjectArg, ObjectCow, ObjectNullArg};

// #[doc(hidden)]
// pub use cow_arg::*;
//
// #[doc(hidden)]
// pub use ref_arg::*;
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
use crate::builtin::Variant;
use crate::meta::error::ConvertError;
use crate::meta::{ClassName, FromGodot, GodotConvert, GodotFfiVariant, GodotType, ToGodot};
use crate::obj::{bounds, raw_gd, Bounds, Gd, GodotClass, Inherits, RawGd};
use crate::sys;
use crate::obj::{bounds, Bounds, Gd, GodotClass, Inherits, RawGd};
use crate::{obj, sys};
use godot_ffi::{GodotFfi, GodotNullableFfi, PtrcallType};
use std::ptr;

Expand All @@ -20,6 +20,8 @@ use std::ptr;
/// - [`Option<&Gd<T>>`][Option], to pass optional objects. `None` is mapped to a null argument.
/// - [`Gd::null_arg()`], to pass `null` arguments without using `Option`.
///
/// Note that [`AsObjectArg`] is very similar to the more general [`AsArg`][crate::meta::AsArg] trait. The two may be merged in the future.
///
/// # Nullability
/// <div class="warning">
/// The GDExtension API does not inform about nullability of its function parameters. It is up to you to verify that the arguments you pass
Expand Down Expand Up @@ -281,7 +283,7 @@ where
// https://github.com/godotengine/godot-cpp/issues/954

fn as_arg_ptr(&self) -> sys::GDExtensionConstTypePtr {
raw_gd::object_as_arg_ptr(&self.object_ptr)
obj::object_as_arg_ptr(&self.object_ptr)
}

unsafe fn from_arg_ptr(_ptr: sys::GDExtensionTypePtr, _call_type: PtrcallType) -> Self {
Expand Down Expand Up @@ -341,7 +343,7 @@ impl<T: GodotClass> GodotType for ObjectArg<T> {
impl<T: GodotClass> GodotFfiVariant for ObjectArg<T> {
fn ffi_to_variant(&self) -> Variant {
// Note: currently likely not invoked since there are no known varcall APIs taking Object parameters; however this might change.
raw_gd::object_ffi_to_variant(self)
obj::object_ffi_to_variant(self)
}

fn ffi_from_variant(_variant: &Variant) -> Result<Self, ConvertError> {
Expand Down
File renamed without changes.
21 changes: 12 additions & 9 deletions godot-core/src/meta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

//! Meta-information about variant types, properties and class names.
//! Meta-information about Godot types, their properties and conversions between them.
//!
//! # Conversions between types
//!
Expand Down Expand Up @@ -33,16 +33,23 @@
//! Godot classes exist in a hierarchy. In OOP, it is usually possible to represent pointers to derived objects as pointer to their bases.
//! For conversions between base and derived class objects, you can use `Gd` methods [`cast()`][crate::obj::Gd::cast],
//! [`try_cast()`][crate::obj::Gd::try_cast] and [`upcast()`][crate::obj::Gd::upcast]. Upcasts are infallible.
//!
//! ## Argument conversions
//!
//! Rust does not support implicit conversions, however it has something very close: the `impl Into<T>` idiom, which can be used to convert
//! "T-compatible" arguments into `T`. This library specializes this idea with two traits:
//!
//! - [`AsArg<T>`] allows argument conversions from arguments into `T`. This is most interesting in the context of strings (so you can pass
//! `&str` to a function expecting `GString`), but is generic to support e.g. array insertion.
//! - [`AsObjectArg<T>`] is a more specialized version of `AsArg` that is used for object arguments, i.e. `Gd<T>`.

mod args;
mod array_type_info;
mod as_arg;
mod class_name;
mod godot_convert;
mod method_info;
mod property_info;
mod ref_arg;
// RpcConfig uses MultiplayerPeer::TransferMode and MultiplayerApi::RpcMode, which are only enabled in `codegen-full` feature.
mod cow_arg;
#[cfg(feature = "codegen-full")]
mod rpc_config;
mod sealed;
Expand All @@ -51,7 +58,7 @@ mod traits;

pub mod error;

pub use as_arg::*;
pub use args::*;
pub use class_name::ClassName;
pub use godot_convert::{FromGodot, GodotConvert, ToGodot};
#[cfg(feature = "codegen-full")]
Expand All @@ -68,10 +75,6 @@ pub(crate) use crate::{
impl_godot_as_self,
};

#[doc(hidden)]
pub use cow_arg::*;
#[doc(hidden)]
pub use ref_arg::*;
#[doc(hidden)]
pub use signature::*;

Expand Down
5 changes: 3 additions & 2 deletions godot-core/src/meta/sealed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@

// To ensure the user does not implement `GodotType` for their own types.
use crate::builtin::*;
use crate::meta;
use crate::meta::traits::{ArrayElement, GodotNullableFfi, GodotType};
use crate::obj::*;
use crate::obj::{Gd, GodotClass, RawGd};

pub trait Sealed {}
impl Sealed for Aabb {}
Expand Down Expand Up @@ -63,7 +64,7 @@ impl Sealed for Variant {}
impl<T: ArrayElement> Sealed for Array<T> {}
impl<T: GodotClass> Sealed for Gd<T> {}
impl<T: GodotClass> Sealed for RawGd<T> {}
impl<T: GodotClass> Sealed for object_arg::ObjectArg<T> {}
impl<T: GodotClass> Sealed for meta::ObjectArg<T> {}
impl<T> Sealed for Option<T>
where
T: GodotType,
Expand Down
Loading
Loading