Merge #911

911: `Ord` implementation now covered by macro r=Bromeon a=Bromeon

Unifies `impl Ord` for various types.

Also upgrades from `PartialOrd` to `Ord`, since Godot defines total order on these types.

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

gdnative-core/src/core_types/rid.rs

@@ -1,6 +1,5 @@
 use crate::private::get_api;
 use crate::sys;
-use std::cmp::Ordering;
 use std::mem::transmute;
 
 // Note: for safety design, consult:
@@ -90,20 +89,8 @@ impl Rid {
 
 impl_basic_traits_as_sys! {
     for Rid as godot_rid {
-        Eq => godot_rid_operator_equal;
         Default => godot_rid_new;
-    }
-}
-
-impl PartialOrd for Rid {
-    #[inline]
-    fn partial_cmp(&self, other: &Rid) -> Option<Ordering> {
-        if PartialEq::eq(self, other) {
-            Some(Ordering::Equal)
-        } else if unsafe { (get_api().godot_rid_operator_less)(self.sys(), other.sys()) } {
-            Some(Ordering::Less)
-        } else {
-            Some(Ordering::Greater)
-        }
+        Eq => godot_rid_operator_equal;
+        Ord => godot_rid_operator_less;
     }
 }

gdnative-core/src/core_types/string.rs

@@ -3,7 +3,6 @@ use crate::object::NewRef;
 use crate::private::get_api;
 use crate::sys;
 
-use std::cmp::Ordering;
 use std::ffi::CStr;
 use std::fmt;
 use std::mem::forget;
@@ -301,6 +300,7 @@ impl_basic_traits_as_sys!(
     for GodotString as godot_string {
         Drop => godot_string_destroy;
         Eq => godot_string_operator_equal;
+        Ord => godot_string_operator_less;
         Default => godot_string_new;
         NewRef => godot_string_new_copy;
     }
@@ -382,26 +382,6 @@ where
     }
 }
 
-impl PartialOrd for GodotString {
-    #[inline]
-    fn partial_cmp(&self, other: &GodotString) -> Option<Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-impl Ord for GodotString {
-    #[inline]
-    fn cmp(&self, other: &GodotString) -> Ordering {
-        if self == other {
-            Ordering::Equal
-        } else if unsafe { (get_api().godot_string_operator_less)(&self.0, &other.0) } {
-            Ordering::Less
-        } else {
-            Ordering::Greater
-        }
-    }
-}
-
 /// Type representing a character in Godot's native encoding. Can be converted to and
 /// from `char`. Depending on the platform, this might not always be able to represent
 /// a full code point.
@@ -612,6 +592,7 @@ impl_basic_traits_as_sys! {
     for StringName as godot_string_name {
         Drop => godot_string_name_destroy;
         Eq => godot_string_name_operator_equal;
+        Ord => godot_string_name_operator_less;
     }
 }
 
@@ -622,21 +603,6 @@ impl fmt::Debug for StringName {
     }
 }
 
-impl PartialOrd for StringName {
-    #[inline]
-    fn partial_cmp(&self, other: &StringName) -> Option<Ordering> {
-        unsafe {
-            let native = (get_api().godot_string_name_operator_less)(&self.0, &other.0);
-
-            if native {
-                Some(Ordering::Less)
-            } else {
-                Some(Ordering::Greater)
-            }
-        }
-    }
-}
-
 impl<S> From<S> for GodotString
 where
     S: AsRef<str>,
@@ -699,6 +665,7 @@ mod serialize {
 
 godot_test!(test_string {
     use crate::core_types::{GodotString, Variant, VariantType, ToVariant, VariantArray, Dictionary};
+    use std::cmp::Ordering;
 
     let foo: GodotString = "foo".into();
     assert_eq!(foo.len(), 3);

gdnative-core/src/core_types/variant.rs

@@ -603,6 +603,7 @@ impl_basic_traits_as_sys!(
         Drop => godot_variant_destroy;
         Clone => godot_variant_new_copy;
         PartialEq => godot_variant_operator_equal;
+        Ord => godot_variant_operator_less;
     }
 );
 

gdnative-core/src/macros.rs

@@ -169,13 +169,32 @@ macro_rules! impl_basic_trait_as_sys {
     (
         Eq for $Type:ty as $GdType:ident : $gd_method:ident
     ) => {
-        impl PartialEq for $Type {
+        impl_basic_trait_as_sys!(PartialEq for $Type as $GdType : $gd_method);
+        impl Eq for $Type {}
+    };
+
+    (
+        Ord for $Type:ty as $GdType:ident : $gd_method:ident
+    ) => {
+        impl PartialOrd for $Type {
             #[inline]
-            fn eq(&self, other: &Self) -> bool {
-                unsafe { (get_api().$gd_method)(self.sys(), other.sys()) }
+            fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+                Some(self.cmp(other))
+            }
+        }
+        impl Ord for $Type {
+            #[inline]
+            fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+                let op_less = get_api().$gd_method;
+                if unsafe { op_less(&self.0, &other.0) } {
+                    std::cmp::Ordering::Less
+                } else if unsafe { op_less(&other.0, &self.0) } {
+                    std::cmp::Ordering::Greater
+                } else {
+                    std::cmp::Ordering::Equal
+                }
             }
         }
-        impl Eq for $Type {}
     };
 
     (