Skip to content

Commit

Permalink
Merge pull request #943 from godot-rust/qol/arg-and-3d
Browse files Browse the repository at this point in the history
Remove `to_2d()` + `to_3d()`; clean up `ApiParam`
  • Loading branch information
Bromeon authored Nov 5, 2024
2 parents 3722014 + 3aefb03 commit 9e4f380
Show file tree
Hide file tree
Showing 9 changed files with 20 additions and 34 deletions.
4 changes: 2 additions & 2 deletions godot-core/src/builtin/collections/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -947,7 +947,7 @@ impl<'r, T: ArrayElement> AsArg<Array<T>> for &'r Array<T> {
impl<T: ArrayElement> ApiParam for Array<T> {
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)
}

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::value_to_arg(item));
self.push(ApiParam::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::value_to_arg(item));
self.push(meta::ApiParam::owned_to_arg(item));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions godot-core/src/builtin/vectors/vector2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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] |
///
/// <br>You can convert to 3D vectors using [`to_3d(z)`][Self::to_3d], and to `Vector2i` using [`cast_int()`][Self::cast_int].
/// <br>You can convert to `Vector2i` using [`cast_int()`][Self::cast_int].
///
/// # Godot docs
///
Expand Down
4 changes: 2 additions & 2 deletions godot-core/src/builtin/vectors/vector2i.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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] |
///
/// <br>You can convert to 3D vectors using [`to_3d(z)`][Self::to_3d], and to `Vector2` using [`cast_float()`][Self::cast_float].
/// <br>You can convert to `Vector2` using [`cast_float()`][Self::cast_float].
///
/// # Godot docs
///
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/vectors/vector3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use std::fmt;
/// | 3D | **`Vector3`** | [`Vector3i`][crate::builtin::Vector3i] |
/// | 4D | [`Vector4`][crate::builtin::Vector4] | [`Vector4i`][crate::builtin::Vector4i] |
///
/// <br>You can convert to 2D vectors using [`to_2d()`][Self::to_2d], and to `Vector3i` using [`cast_int()`][Self::cast_int].
/// <br>You can convert to `Vector3i` using [`cast_int()`][Self::cast_int].
///
/// # Godot docs
///
Expand Down
4 changes: 2 additions & 2 deletions godot-core/src/builtin/vectors/vector3i.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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] |
///
/// <br>You can convert to 2D vectors using [`to_2d()`][Self::to_2d], and to `Vector3` using [`cast_float()`][Self::cast_float].
/// <br>You can convert to `Vector3` using [`cast_float()`][Self::cast_float].
///
/// # Godot docs
///
Expand Down
15 changes: 0 additions & 15 deletions godot-core/src/builtin/vectors/vector_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)`.
Expand Down
15 changes: 8 additions & 7 deletions godot-core/src/meta/as_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -252,22 +252,23 @@ impl AsArg<NodePath> for &String {
/// Implemented for all parameter types `T` that are allowed to receive [impl `AsArg<T>`][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<Self>
where
Self: 'v;

/// Converts an owned value to the canonical argument type, which can be passed to [`impl AsArg<T>`][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<Self>`.
fn owned_to_arg<'v>(self) -> Self::Arg<'v>;

/// Converts an argument to a shared reference.
///
Expand Down
4 changes: 2 additions & 2 deletions godot-core/src/obj/gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ impl<'r, T: GodotClass> AsArg<Gd<T>> for &'r Gd<T> {
impl<T: GodotClass> ApiParam for Gd<T> {
type Arg<'v> = CowArg<'v, Gd<T>>;

fn value_to_arg<'v>(self) -> Self::Arg<'v> {
fn owned_to_arg<'v>(self) -> Self::Arg<'v> {
CowArg::Owned(self)
}

Expand All @@ -806,7 +806,7 @@ impl<'r, T: GodotClass> AsArg<Option<Gd<T>>> for Option<&'r Gd<T>> {
impl<T: GodotClass> ApiParam for Option<Gd<T>> {
type Arg<'v> = CowArg<'v, Option<Gd<T>>>;

fn value_to_arg<'v>(self) -> Self::Arg<'v> {
fn owned_to_arg<'v>(self) -> Self::Arg<'v> {
CowArg::Owned(self)
}

Expand Down

0 comments on commit 9e4f380

Please sign in to comment.