Merge pull request #865 from Yarwin/bugfix/fix_vector_sign

Bugfix - Vector3::sign gives incorrect results due to i32 conversion

Author: Bromeon

godot-core/src/builtin/vectors/vector2.rs

@@ -227,6 +227,14 @@ mod test {
         assert_eq_approx!(a.coord_max(b), Vector2::new(1.2, 5.6));
     }
 
+    #[test]
+    fn sign() {
+        let vector = Vector2::new(0.2, -0.5);
+        assert_eq!(vector.sign(), Vector2::new(1., -1.));
+        let vector = Vector2::new(0.1, 0.0);
+        assert_eq!(vector.sign(), Vector2::new(1., 0.));
+    }
+
     #[cfg(feature = "serde")]
     #[test]
     fn serde_roundtrip() {

godot-core/src/builtin/vectors/vector2i.rs

@@ -150,6 +150,14 @@ mod test {
         crate::builtin::test_utils::roundtrip(&vector, expected_json);
     }
 
+    #[test]
+    fn sign() {
+        let vector = Vector2i::new(2, -5);
+        assert_eq!(vector.sign(), Vector2i::new(1, -1));
+        let vector = Vector2i::new(1, 0);
+        assert_eq!(vector.sign(), Vector2i::new(1, 0));
+    }
+
     #[test]
     fn axis_min_max() {
         assert_eq!(Vector2i::new(10, 5).max_axis(), Some(Vector2Axis::X));

godot-core/src/builtin/vectors/vector3.rs

@@ -325,6 +325,12 @@ mod test {
         );
     }
 
+    #[test]
+    fn sign() {
+        let vector = Vector3::new(0.2, -0.5, 0.0);
+        assert_eq!(vector.sign(), Vector3::new(1., -1., 0.));
+    }
+
     #[test]
     fn coord_min_max() {
         let a = Vector3::new(1.2, 3.4, 5.6);

godot-core/src/builtin/vectors/vector3i.rs

@@ -154,6 +154,12 @@ mod test {
         crate::builtin::test_utils::roundtrip(&vector, expected_json);
     }
 
+    #[test]
+    fn sign() {
+        let vector = Vector3i::new(2, -5, 0);
+        assert_eq!(vector.sign(), Vector3i::new(1, -1, 0));
+    }
+
     #[test]
     fn axis_min_max() {
         assert_eq!(Vector3i::new(10, 5, -5).max_axis(), Some(Vector3Axis::X));

godot-core/src/builtin/vectors/vector4.rs

@@ -127,9 +127,8 @@ impl GlamConv for Vector4 {
 
 #[cfg(test)]
 mod test {
-    use crate::assert_eq_approx;
-
     use super::*;
+    use crate::builtin::math::assert_eq_approx;
 
     #[test]
     fn coord_min_max() {
@@ -139,6 +138,12 @@ mod test {
         assert_eq_approx!(a.coord_max(b), Vector4::new(1.2, 5.6, 5.6, 1.2),);
     }
 
+    #[test]
+    fn sign() {
+        let vector = Vector4::new(0.2, -0.5, 0., 999.0);
+        assert_eq!(vector.sign(), Vector4::new(1., -1., 0., 1.));
+    }
+
     #[cfg(feature = "serde")]
     #[test]
     fn serde_roundtrip() {

godot-core/src/builtin/vectors/vector4i.rs

@@ -157,6 +157,12 @@ mod test {
         crate::builtin::test_utils::roundtrip(&vector, expected_json);
     }
 
+    #[test]
+    fn sign() {
+        let vector = Vector4i::new(2, -5, 0, 999);
+        assert_eq!(vector.sign(), Vector4i::new(1, -1, 0, 1));
+    }
+
     #[test]
     fn axis_min_max() {
         assert_eq!(Vector4i::new(10, 5, -5, 0).max_axis(), Some(Vector4Axis::X));

godot-core/src/builtin/vectors/vector_macros.rs

@@ -484,16 +484,17 @@ macro_rules! impl_vector_fns {
             #[inline]
             pub fn sign(self) -> Self {
                 #[inline]
-                fn f(x: i32) -> i32 {
-                    match x.cmp(&0) {
-                        Ordering::Equal => 0,
-                        Ordering::Greater => 1,
-                        Ordering::Less => -1,
+                fn f(c: $Scalar) -> $Scalar {
+                    let r = c.partial_cmp(&(0 as $Scalar)).unwrap_or_else(|| panic!("Vector component {c} isn't signed!"));
+                    match r {
+                        Ordering::Equal => 0 as $Scalar,
+                        Ordering::Greater => 1 as $Scalar,
+                        Ordering::Less => -1 as $Scalar,
                     }
                 }
 
                 Self::new(
-                    $( f(self.$comp as i32) as $Scalar ),*
+                    $( f(self.$comp) ),*
                 )
             }
         }