From 4c283d42e9a90ed187928c584a91d0e114ba851b Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Tue, 5 Nov 2024 18:25:45 +0100 Subject: [PATCH 1/2] Rename ApiParam::value_to_arg() -> owned_to_arg(), hide Arg type --- godot-core/src/builtin/collections/array.rs | 4 ++-- .../src/builtin/collections/packed_array.rs | 2 +- godot-core/src/meta/as_arg.rs | 15 ++++++++------- godot-core/src/obj/gd.rs | 4 ++-- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs index ffe1fe812..427e517b4 100644 --- a/godot-core/src/builtin/collections/array.rs +++ b/godot-core/src/builtin/collections/array.rs @@ -947,7 +947,7 @@ impl<'r, T: ArrayElement> AsArg> for &'r Array { impl ApiParam for Array { type Arg<'v> = CowArg<'v, Self>; - fn value_to_arg<'v>(self) -> Self::Arg<'v> { + fn owned_to_arg<'v>(self) -> Self::Arg<'v> { CowArg::Owned(self) } @@ -1221,7 +1221,7 @@ impl Extend for Array { // 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::value_to_arg(item)); + self.push(ApiParam::owned_to_arg(item)); } } } diff --git a/godot-core/src/builtin/collections/packed_array.rs b/godot-core/src/builtin/collections/packed_array.rs index 4347b6e65..e6f79efc7 100644 --- a/godot-core/src/builtin/collections/packed_array.rs +++ b/godot-core/src/builtin/collections/packed_array.rs @@ -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::value_to_arg(item)); + self.push(meta::ApiParam::owned_to_arg(item)); } } } diff --git a/godot-core/src/meta/as_arg.rs b/godot-core/src/meta/as_arg.rs index c96192b82..f4939fcfe 100644 --- a/godot-core/src/meta/as_arg.rs +++ b/godot-core/src/meta/as_arg.rs @@ -23,7 +23,7 @@ use std::ffi::CStr; /// 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::value_to_arg()`]. +/// If you need to pass owned values in generic code, you can use [`ApiParam::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 @@ -106,7 +106,7 @@ macro_rules! impl_asarg_by_value { impl $crate::meta::ApiParam for $T { type Arg<'v> = $T; - fn value_to_arg<'v>(self) -> Self::Arg<'v> { + fn owned_to_arg<'v>(self) -> Self::Arg<'v> { self } @@ -139,7 +139,7 @@ macro_rules! impl_asarg_by_ref { impl $crate::meta::ApiParam for $T { type Arg<'v> = $crate::meta::CowArg<'v, $T>; - fn value_to_arg<'v>(self) -> Self::Arg<'v> { + fn owned_to_arg<'v>(self) -> Self::Arg<'v> { $crate::meta::CowArg::Owned(self) } @@ -252,14 +252,13 @@ impl AsArg for &String { /// Implemented for all parameter types `T` that are allowed to receive [impl `AsArg`][AsArg]. pub trait ApiParam: GodotType // GodotType bound not required right now, but conceptually should always be the case. -where - Self: Sized, { /// Canonical argument passing type, either `T` or an internally-used CoW type. /// /// The general rule is that `Copy` types are passed by value, while the rest is passed by reference. /// - /// This associated type is closely related to [`ToGodot::ToVia<'v>`][crate::meta::ToGodot::ToVia] and may be reorganized. + /// This associated type is closely related to [`ToGodot::ToVia<'v>`][crate::meta::ToGodot::ToVia] and may be reorganized in the future. + #[doc(hidden)] type Arg<'v>: AsArg where Self: 'v; @@ -267,7 +266,9 @@ where /// Converts an owned value to the canonical argument type, which can be passed to [`impl AsArg`][AsArg]. /// /// Useful in generic contexts where only a value is available, and one doesn't want to dispatch between value/reference. - fn value_to_arg<'v>(self) -> Self::Arg<'v>; + /// + /// You should not rely on the exact return type, as it may change in future versions; treat it like `impl AsArg`. + fn owned_to_arg<'v>(self) -> Self::Arg<'v>; /// Converts an argument to a shared reference. /// diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index a862c70a5..cf4621719 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -784,7 +784,7 @@ impl<'r, T: GodotClass> AsArg> for &'r Gd { impl ApiParam for Gd { type Arg<'v> = CowArg<'v, Gd>; - fn value_to_arg<'v>(self) -> Self::Arg<'v> { + fn owned_to_arg<'v>(self) -> Self::Arg<'v> { CowArg::Owned(self) } @@ -806,7 +806,7 @@ impl<'r, T: GodotClass> AsArg>> for Option<&'r Gd> { impl ApiParam for Option> { type Arg<'v> = CowArg<'v, Option>>; - fn value_to_arg<'v>(self) -> Self::Arg<'v> { + fn owned_to_arg<'v>(self) -> Self::Arg<'v> { CowArg::Owned(self) } From 3aefb03441bd715886ef562228297948c0a9b23a Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Tue, 5 Nov 2024 19:10:21 +0100 Subject: [PATCH 2/2] Remove vector to_2d() + to_3d() methods Semantics aren't obvious -- while Z component is the mathematically natural extension, in Godot Y would be more typical (being the up/down axis in 3D). There's also the topic of +/- signs... Maybe it can be re-added once there are clearer use cases. Alternatively, a more flexible swizzle! macro supporting 0 when extending is possible. --- godot-core/src/builtin/vectors/vector2.rs | 4 ++-- godot-core/src/builtin/vectors/vector2i.rs | 4 ++-- godot-core/src/builtin/vectors/vector3.rs | 2 +- godot-core/src/builtin/vectors/vector3i.rs | 4 ++-- godot-core/src/builtin/vectors/vector_macros.rs | 15 --------------- 5 files changed, 7 insertions(+), 22 deletions(-) diff --git a/godot-core/src/builtin/vectors/vector2.rs b/godot-core/src/builtin/vectors/vector2.rs index d64b552b5..72f0da818 100644 --- a/godot-core/src/builtin/vectors/vector2.rs +++ b/godot-core/src/builtin/vectors/vector2.rs @@ -11,7 +11,7 @@ use sys::{ffi_methods, GodotFfi}; use crate::builtin::math::{FloatExt, GlamConv, GlamType}; use crate::builtin::vectors::Vector2Axis; -use crate::builtin::{inner, real, RAffine2, RVec2, Vector2i, Vector3}; +use crate::builtin::{inner, real, RAffine2, RVec2, Vector2i}; use std::fmt; @@ -44,7 +44,7 @@ use std::fmt; /// | 3D | [`Vector3`][crate::builtin::Vector3] | [`Vector3i`][crate::builtin::Vector3i] | /// | 4D | [`Vector4`][crate::builtin::Vector4] | [`Vector4i`][crate::builtin::Vector4i] | /// -///
You can convert to 3D vectors using [`to_3d(z)`][Self::to_3d], and to `Vector2i` using [`cast_int()`][Self::cast_int]. +///
You can convert to `Vector2i` using [`cast_int()`][Self::cast_int]. /// /// # Godot docs /// diff --git a/godot-core/src/builtin/vectors/vector2i.rs b/godot-core/src/builtin/vectors/vector2i.rs index 8ea03434a..6948e30b4 100644 --- a/godot-core/src/builtin/vectors/vector2i.rs +++ b/godot-core/src/builtin/vectors/vector2i.rs @@ -10,7 +10,7 @@ use std::cmp::Ordering; use sys::{ffi_methods, GodotFfi}; use crate::builtin::math::{GlamConv, GlamType}; -use crate::builtin::{inner, real, RVec2, Vector2, Vector2Axis, Vector3i}; +use crate::builtin::{inner, real, RVec2, Vector2, Vector2Axis}; use std::fmt; @@ -42,7 +42,7 @@ use std::fmt; /// | 3D | [`Vector3`][crate::builtin::Vector3] | [`Vector3i`][crate::builtin::Vector3i] | /// | 4D | [`Vector4`][crate::builtin::Vector4] | [`Vector4i`][crate::builtin::Vector4i] | /// -///
You can convert to 3D vectors using [`to_3d(z)`][Self::to_3d], and to `Vector2` using [`cast_float()`][Self::cast_float]. +///
You can convert to `Vector2` using [`cast_float()`][Self::cast_float]. /// /// # Godot docs /// diff --git a/godot-core/src/builtin/vectors/vector3.rs b/godot-core/src/builtin/vectors/vector3.rs index 7f70f7ff3..dace9a05a 100644 --- a/godot-core/src/builtin/vectors/vector3.rs +++ b/godot-core/src/builtin/vectors/vector3.rs @@ -45,7 +45,7 @@ use std::fmt; /// | 3D | **`Vector3`** | [`Vector3i`][crate::builtin::Vector3i] | /// | 4D | [`Vector4`][crate::builtin::Vector4] | [`Vector4i`][crate::builtin::Vector4i] | /// -///
You can convert to 2D vectors using [`to_2d()`][Self::to_2d], and to `Vector3i` using [`cast_int()`][Self::cast_int]. +///
You can convert to `Vector3i` using [`cast_int()`][Self::cast_int]. /// /// # Godot docs /// diff --git a/godot-core/src/builtin/vectors/vector3i.rs b/godot-core/src/builtin/vectors/vector3i.rs index a93d40bb7..3a49124a9 100644 --- a/godot-core/src/builtin/vectors/vector3i.rs +++ b/godot-core/src/builtin/vectors/vector3i.rs @@ -12,7 +12,7 @@ use godot_ffi as sys; use sys::{ffi_methods, GodotFfi}; use crate::builtin::math::{GlamConv, GlamType}; -use crate::builtin::{inner, real, RVec3, Vector2i, Vector3, Vector3Axis}; +use crate::builtin::{inner, real, RVec3, Vector3, Vector3Axis}; /// Vector used for 3D math using integer coordinates. /// @@ -42,7 +42,7 @@ use crate::builtin::{inner, real, RVec3, Vector2i, Vector3, Vector3Axis}; /// | 3D | [`Vector3`][crate::builtin::Vector3] | **`Vector3i`** | /// | 4D | [`Vector4`][crate::builtin::Vector4] | [`Vector4i`][crate::builtin::Vector4i] | /// -///
You can convert to 2D vectors using [`to_2d()`][Self::to_2d], and to `Vector3` using [`cast_float()`][Self::cast_float]. +///
You can convert to `Vector3` using [`cast_float()`][Self::cast_float]. /// /// # Godot docs /// diff --git a/godot-core/src/builtin/vectors/vector_macros.rs b/godot-core/src/builtin/vectors/vector_macros.rs index 1a7568004..f9a181b38 100644 --- a/godot-core/src/builtin/vectors/vector_macros.rs +++ b/godot-core/src/builtin/vectors/vector_macros.rs @@ -791,12 +791,6 @@ macro_rules! impl_vector2x_fns { /// # 2D functions /// The following methods are only available on 2D vectors (for both float and int). impl $Vector { - /// Increases dimension to 3D, accepting a new value for the Z component. - #[inline] - pub fn to_3d(self, z: $Scalar) -> $Vector3D { - <$Vector3D>::new(self.x, self.y, z) - } - /// Returns the aspect ratio of this vector, the ratio of [`Self::x`] to [`Self::y`]. #[inline] pub fn aspect(self) -> real { @@ -853,15 +847,6 @@ macro_rules! impl_vector3x_fns { /// # 3D functions /// The following methods are only available on 3D vectors (for both float and int). impl $Vector { - /// Reduces dimension to 2D, discarding the Z component. - /// - /// See also [`swizzle!`][crate::builtin::swizzle] for a more general way to extract components. - /// `self.to_2d()` is equivalent to `swizzle!(self => x, y)`. - #[inline] - pub fn to_2d(self) -> $Vector2D { - <$Vector2D>::new(self.x, self.y) - } - /// Returns the axis of the vector's highest value. See [`Vector3Axis`] enum. If all components are equal, this method returns [`None`]. /// /// To mimic Godot's behavior, unwrap this function's result with `unwrap_or(Vector3Axis::X)`.