Merge #314

314: `ApproxEq` trait + refactorings r=Bromeon a=Bromeon

Changes:

### Add `ApproxEq` trait and implement it for many builtin types.
  * This standardizes checking for approximate equality across types.
  * Gets rid of the boilerplate 3rd argument `assert_eq_approx!(a, b, is_equal_approx)`.
    * That argument can still be specified if desired, using `assert_eq_approx!(a, b, fn = is_equal_approx)`. 
    * In 95% of cases, it's not needed though.

### Refactor modules
  * `godot::builtin::math` is now a public module, which contains:
    * Godot ported functions such as `lerp`, `sign`
    * `ApproxEq` trait + `assert_eq_approx!`, `assert_ne_approx!` macros
    * glam utilities: `IVec4`, `GlamConv`, etc (internal)
  * The above symbols are no longer in `godot::builtin`. If this turns out to be an issue, we can also revert this in the future; but it might help some discoverability. Some symbols could also be considered to be part of the prelude.
  * Move `godot::native_structure` -> `godot::engine::native`
    * Added in #272
    * Native structures are now less prominent because rarely used.
  * Move `real`-related symbols into their own `real.rs` file (internal; API unaffected).

### Refactor import statements (just around geometric types, not whole codebase)
  * Remove wildcard imports
  * Remove nested import statements
  * Consistent order

Co-authored-by: Jan Haller <[email protected]>

Author: bors[bot]

.github/workflows/full-ci.yml

@@ -12,6 +12,7 @@ on:
     branches:
       - staging
       - trying
+
 env:
   GDEXT_FEATURES: ''
 

godot-codegen/src/class_generator.rs

@@ -333,7 +333,7 @@ fn make_class(class: &Class, class_name: &TyName, ctx: &mut Context) -> Generate
         use godot_ffi as sys;
         use crate::engine::notify::*;
         use crate::builtin::*;
-        use crate::native_structure::*;
+        use crate::engine::native::*;
         use crate::obj::{AsArg, Gd};
         use sys::GodotFfi as _;
         use std::ffi::c_void;
@@ -564,7 +564,7 @@ fn make_builtin_class(
     let tokens = quote! {
         use godot_ffi as sys;
         use crate::builtin::*;
-        use crate::native_structure::*;
+        use crate::engine::native::*;
         use crate::obj::{AsArg, Gd};
         use crate::sys::GodotFfi as _;
         use crate::engine::Object;
@@ -605,7 +605,7 @@ fn make_native_structure(
     let tokens = quote! {
         use godot_ffi as sys;
         use crate::builtin::*;
-        use crate::native_structure::*;
+        use crate::engine::native::*;
         use crate::obj::{AsArg, Gd};
         use crate::sys::GodotFfi as _;
         use crate::engine::Object;

godot-core/src/builtin/aabb.rs

@@ -7,7 +7,8 @@
 use godot_ffi as sys;
 use sys::{ffi_methods, GodotFfi};
 
-use super::{real, Plane, Vector3, Vector3Axis};
+use crate::builtin::math::ApproxEq;
+use crate::builtin::{real, Plane, Vector3, Vector3Axis};
 
 /// Axis-aligned bounding box in 3D space.
 ///
@@ -185,13 +186,6 @@ impl Aabb {
         self.size = end - self.position
     }
 
-    /// Returns `true` if the two `Aabb`s are approximately equal, by calling `is_equal_approx` on
-    /// `position` and `size`.
-    #[inline]
-    pub fn is_equal_approx(&self, other: &Self) -> bool {
-        self.position.is_equal_approx(other.position) && self.size.is_equal_approx(other.size)
-    }
-
     /// Returns the normalized longest axis of the AABB.
     pub fn longest_axis(&self) -> Vector3 {
         match self.longest_axis_index() {
@@ -388,6 +382,16 @@ unsafe impl GodotFfi for Aabb {
     ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
 }
 
+impl ApproxEq for Aabb {
+    /// Returns `true` if the two `Aabb`s are approximately equal, by calling `is_equal_approx` on
+    /// `position` and `size`.
+    #[inline]
+    fn approx_eq(&self, other: &Self) -> bool {
+        Vector3::approx_eq(&self.position, &other.position)
+            && Vector3::approx_eq(&self.size, &other.size)
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;

godot-core/src/builtin/basis.rs

@@ -1,18 +1,19 @@
-use std::cmp::Ordering;
 /*
  * 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/.
  */
-use std::{fmt::Display, ops::*};
 
 use godot_ffi as sys;
 use sys::{ffi_methods, GodotFfi};
 
-use super::glam_helpers::{GlamConv, GlamType};
-use super::real_consts::FRAC_PI_2;
-use super::{math::*, Quaternion, Vector3};
-use super::{real, RMat3, RQuat, RVec2, RVec3};
+use crate::builtin::math::{is_equal_approx, lerp, ApproxEq, GlamConv, GlamType, CMP_EPSILON};
+use crate::builtin::real_consts::FRAC_PI_2;
+use crate::builtin::{real, Quaternion, RMat3, RQuat, RVec2, RVec3, Vector3};
+
+use std::cmp::Ordering;
+use std::fmt::Display;
+use std::ops::{Mul, MulAssign};
 
 /// A 3x3 matrix, typically used as an orthogonal basis for [`Transform3D`](crate::builtin::Transform3D).
 ///
@@ -448,16 +449,6 @@ impl Basis {
         self.rows[0].is_finite() && self.rows[1].is_finite() && self.rows[2].is_finite()
     }
 
-    /// Returns `true` if this basis and `other` are approximately equal,
-    /// by calling `is_equal_approx` on each row.
-    ///
-    /// _Godot equivalent: `Basis.is_equal_approx(Basis b)`_
-    pub fn is_equal_approx(&self, other: &Self) -> bool {
-        self.rows[0].is_equal_approx(other.rows[0])
-            && self.rows[1].is_equal_approx(other.rows[1])
-            && self.rows[2].is_equal_approx(other.rows[2])
-    }
-
     /// Returns the first column of the matrix,
     ///
     /// _Godot equivalent: `Basis.x`_
@@ -518,6 +509,15 @@ impl Display for Basis {
     }
 }
 
+impl ApproxEq for Basis {
+    /// Returns if this basis and `other` are approximately equal, by calling `is_equal_approx` on each row.
+    fn approx_eq(&self, other: &Self) -> bool {
+        Vector3::approx_eq(&self.rows[0], &other.rows[0])
+            && Vector3::approx_eq(&self.rows[1], &other.rows[1])
+            && Vector3::approx_eq(&self.rows[2], &other.rows[2])
+    }
+}
+
 impl GlamConv for Basis {
     type Glam = RMat3;
 }
@@ -693,45 +693,29 @@ mod test {
     fn consts_behavior_correct() {
         let v = Vector3::new(1.0, 2.0, 3.0);
 
-        assert_eq_approx!(Basis::IDENTITY * v, v, Vector3::is_equal_approx);
-        assert_eq_approx!(
-            Basis::FLIP_X * v,
-            Vector3::new(-v.x, v.y, v.z),
-            Vector3::is_equal_approx
-        );
-        assert_eq_approx!(
-            Basis::FLIP_Y * v,
-            Vector3::new(v.x, -v.y, v.z),
-            Vector3::is_equal_approx
-        );
-        assert_eq_approx!(
-            Basis::FLIP_Z * v,
-            Vector3::new(v.x, v.y, -v.z),
-            Vector3::is_equal_approx
-        );
+        assert_eq_approx!(Basis::IDENTITY * v, v);
+        assert_eq_approx!(Basis::FLIP_X * v, Vector3::new(-v.x, v.y, v.z),);
+        assert_eq_approx!(Basis::FLIP_Y * v, Vector3::new(v.x, -v.y, v.z),);
+        assert_eq_approx!(Basis::FLIP_Z * v, Vector3::new(v.x, v.y, -v.z),);
     }
 
     #[test]
     fn basic_rotation_correct() {
         assert_eq_approx!(
             Basis::from_axis_angle(Vector3::FORWARD, 0.0) * Vector3::RIGHT,
             Vector3::RIGHT,
-            Vector3::is_equal_approx,
         );
         assert_eq_approx!(
             Basis::from_axis_angle(Vector3::FORWARD, FRAC_PI_2) * Vector3::RIGHT,
             Vector3::DOWN,
-            Vector3::is_equal_approx,
         );
         assert_eq_approx!(
             Basis::from_axis_angle(Vector3::FORWARD, PI) * Vector3::RIGHT,
             Vector3::LEFT,
-            Vector3::is_equal_approx,
         );
         assert_eq_approx!(
             Basis::from_axis_angle(Vector3::FORWARD, PI + FRAC_PI_2) * Vector3::RIGHT,
             Vector3::UP,
-            Vector3::is_equal_approx,
         );
     }
 

godot-core/src/builtin/color.rs

@@ -5,11 +5,14 @@
  */
 
 use crate::builtin::inner::InnerColor;
+use crate::builtin::math::ApproxEq;
 use crate::builtin::GodotString;
+
 use godot_ffi as sys;
-use std::ops;
 use sys::{ffi_methods, GodotFfi};
 
+use std::ops;
+
 /// Color built-in type, in floating-point RGBA format.
 ///
 /// Channel values are _typically_ in the range of 0 to 1, but this is not a requirement, and
@@ -287,12 +290,6 @@ impl Color {
         self.as_inner().to_html(false)
     }
 
-    /// Returns true if `self` and `to` are approximately equal, within the tolerance used by
-    /// the global `is_equal_approx` function in GDScript.
-    pub fn is_equal_approx(self, to: Color) -> bool {
-        self.as_inner().is_equal_approx(to)
-    }
-
     /// Returns the color converted to a 32-bit integer (each component is 8 bits) with the given
     /// `order` of channels (from most to least significant byte).
     pub fn to_u32(self, order: ColorChannelOrder) -> u32 {
@@ -321,6 +318,13 @@ unsafe impl GodotFfi for Color {
     ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
 }
 
+impl ApproxEq for Color {
+    fn approx_eq(&self, other: &Self) -> bool {
+        // TODO(bromeon): re-implement in Rust
+        self.as_inner().is_equal_approx(*other)
+    }
+}
+
 #[derive(Copy, Clone, Eq, PartialEq, Debug)]
 pub enum ColorChannelOrder {
     /// RGBA channel order. Godot's default.

godot-core/src/builtin/math/approx_eq.rs

@@ -0,0 +1,74 @@
+/*
+ * 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/.
+ */
+
+use crate::builtin::real;
+
+// TODO(bromeon): test this against Godot's own is_equal_approx() implementation for equality-comparable built-in types (excl Callable/Rid/...)
+
+/// Approximate equality-comparison of geometric types.
+///
+/// The implementation is specific to the type. It's mostly used for gdext-internal tests, but you may use it for your own code.
+/// Note that we give no guarantees about precision, and implementation can change at any time.
+///
+/// We currently also do not guarantee that this gives the same results as Godot's own `is_equal_approx()` function; although this may
+/// be the goal in the future.
+pub trait ApproxEq: PartialEq {
+    fn approx_eq(&self, other: &Self) -> bool;
+}
+
+impl ApproxEq for real {
+    fn approx_eq(&self, other: &Self) -> bool {
+        crate::builtin::math::is_equal_approx(*self, *other)
+    }
+}
+
+/// Asserts that two values are approximately equal
+///
+/// For comparison, this uses `ApproxEq::approx_eq` by default, or the provided `fn = ...` function.
+#[macro_export]
+macro_rules! assert_eq_approx {
+    ($actual:expr, $expected:expr, fn = $func:expr $(,)?) => {
+        match ($actual, $expected) {
+            (a, b) => assert!(($func)(&a, &b), "\n  left: {:?},\n right: {:?}", $actual, $expected)
+        }
+    };
+    ($actual:expr, $expected:expr, fn = $func:expr, $($t:tt)+) => {
+        match ($actual, $expected) {
+            (a, b) => assert!(($func)(&a, &b), "\n  left: {:?},\n right: {:?}{}", $actual, $expected, format_args!($($t)+) )
+        }
+    };
+    ($actual:expr, $expected:expr $(,)?) => {
+        match ($actual, $expected) {
+             (a, b) => assert!($crate::builtin::math::ApproxEq::approx_eq(&a, &b), "\n  left: {:?},\n right: {:?}", $actual, $expected),
+            // (a, b) => $crate::assert_eq_approx!($actual, $expected, fn = $crate::builtin::ApproxEq::approx_eq),
+        }
+    };
+    ($actual:expr, $expected:expr, $($t:tt)+) => {
+        match ($actual, $expected) {
+            (a, b) => assert!($crate::builtin::math::ApproxEq::approx_eq(&a, &b), "\n  left: {:?},\n right: {:?},\n{}", $actual, $expected, format_args!($($t)+)),
+            // (a, b) => $crate::assert_eq_approx!($actual, $expected, fn = $crate::builtin::ApproxEq::approx_eq, $($t)+),
+        }
+    };
+}
+
+/// Asserts that two values are not approximately equal, using the provided
+/// `func` for equality checking.
+#[macro_export]
+macro_rules! assert_ne_approx {
+    ($actual:expr, $expected:expr, fn = $func:expr $(, $($t:tt)* )?) => {
+        #[allow(clippy::redundant_closure_call)]
+        {
+            $crate::assert_eq_approx!($actual, $expected, fn = |a,b| !($func)(a, b) $(, $($t)* )?)
+        }
+    };
+
+    ($actual:expr, $expected:expr $(, $($t:tt)* )?) => {
+        #[allow(clippy::redundant_closure_call)]
+        {
+            $crate::assert_eq_approx!($actual, $expected, fn = |a, b| !$crate::builtin::math::ApproxEq::approx_eq(a, b) $(, $($t)* )?)
+        }
+    };
+}

godot-core/src/builtin/glam_helpers.rs renamed to godot-core/src/builtin/math/glam_helpers.rs

@@ -15,7 +15,7 @@
 //   self.glam().dot(b.glam())
 //   GlamType::dot(self.glam(), b.glam())
 
-use super::real;
+use crate::builtin::real;
 
 pub(crate) trait GlamConv {
     type Glam: GlamType<Mapped = Self>;