Merge pull request #1064 from godot-rust/feature/callable-convenience-apis

Callables to builtin methods; `Array::bsearch_by, sort_unstable_by`

Author: Bromeon

check.sh

@@ -165,6 +165,8 @@ function cmd_clippy() {
 }
 
 function cmd_klippy() {
+    # TODO(Rust 1.86): remove `-A clippy::precedence`.
+
     run cargo clippy --fix --all-targets "${extraCargoArgs[@]}" -- \
         -D clippy::suspicious \
         -D clippy::style \
@@ -173,7 +175,8 @@ function cmd_klippy() {
         -D clippy::dbg_macro \
         -D clippy::todo \
         -D clippy::unimplemented \
-        -D warnings
+        -D warnings \
+        -A clippy::precedence
 }
 
 function cmd_test() {

godot-core/src/builtin/callable.rs

@@ -64,10 +64,32 @@ impl Callable {
         }
     }
 
+    /// Create a callable for a method on any [`Variant`].
+    ///
+    /// Allows to dynamically call methods on builtin types (e.g. `String.md5_text`). Note that Godot method names are used, not Rust ones.
+    /// If the variant type is `Object`, the behavior will match that of `from_object_method()`.
+    ///
+    /// If the builtin type does not have the method, the returned callable will be invalid.
+    ///
+    /// Static builtin methods (e.g. `String.humanize_size`) are not supported in reflection as of Godot 4.4. For static _class_ functions,
+    /// use [`from_local_static()`][Self::from_local_static] instead.
+    ///
+    /// _Godot equivalent: `Callable.create(Variant variant, StringName method)`_
+    #[cfg(since_api = "4.3")]
+    pub fn from_variant_method<S>(variant: &Variant, method_name: S) -> Self
+    where
+        S: meta::AsArg<StringName>,
+    {
+        meta::arg_into_ref!(method_name);
+        inner::InnerCallable::create(variant, method_name)
+    }
+
     /// Create a callable for the static method `class_name::function` (single-threaded).
     ///
     /// Allows you to call static functions through `Callable`.
     ///
+    /// Does not support built-in types (such as `String`), only classes.
+    ///
     /// # Compatibility
     /// Not available before Godot 4.4. Library versions <0.3 used to provide this, however the polyfill used to emulate it was half-broken
     /// (not supporting signals, bind(), method_name(), is_valid(), etc).
@@ -133,6 +155,24 @@ impl Callable {
         })
     }
 
+    #[cfg(since_api = "4.2")]
+    pub(crate) fn with_scoped_fn<S, F, Fc, R>(name: S, rust_function: F, callable_usage: Fc) -> R
+    where
+        S: meta::AsArg<GString>,
+        F: FnMut(&[&Variant]) -> Result<Variant, ()>,
+        Fc: FnOnce(&Callable) -> R,
+    {
+        meta::arg_into_owned!(name);
+
+        let callable = Self::from_fn_wrapper(FnWrapper {
+            rust_function,
+            name,
+            thread_id: Some(std::thread::current().id()),
+        });
+
+        callable_usage(&callable)
+    }
+
     /// Create callable from **thread-safe** Rust function or closure.
     ///
     /// `name` is used for the string representation of the closure, which helps debugging.

godot-core/src/builtin/collections/array.rs

@@ -5,8 +5,8 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use std::fmt;
 use std::marker::PhantomData;
+use std::{cmp, fmt};
 
 use crate::builtin::*;
 use crate::meta;
@@ -612,37 +612,80 @@ impl<T: ArrayElement> Array<T> {
         }
     }
 
-    /// Finds the index of an existing value in a sorted array using binary search.
-    /// Equivalent of `bsearch` in GDScript.
+    /// Finds the index of a value in a sorted array using binary search.
     ///
-    /// If the value is not present in the array, returns the insertion index that
-    /// would maintain sorting order.
+    /// If the value is not present in the array, returns the insertion index that would maintain sorting order.
     ///
-    /// Calling `bsearch` on an unsorted array results in unspecified behavior.
+    /// Calling `bsearch` on an unsorted array results in unspecified behavior. Consider using `sort()` to ensure the sorting
+    /// order is compatible with your callable's ordering.
     pub fn bsearch(&self, value: impl AsArg<T>) -> usize {
         meta::arg_into_ref!(value: T);
 
         to_usize(self.as_inner().bsearch(&value.to_variant(), true))
     }
 
-    /// Finds the index of an existing value in a sorted array using binary search.
-    /// Equivalent of `bsearch_custom` in GDScript.
+    /// Finds the index of a value in a sorted array using binary search, with type-safe custom predicate.
     ///
-    /// Takes a `Callable` and uses the return value of it to perform binary search.
+    /// The comparator function should return an ordering that indicates whether its argument is `Less`, `Equal` or `Greater` the desired value.
+    /// For example, for an ascending-ordered array, a simple predicate searching for a constant value would be `|elem| elem.cmp(&4)`.
+    /// See also [`slice::binary_search_by()`].
     ///
-    /// If the value is not present in the array, returns the insertion index that
-    /// would maintain sorting order.
+    /// If the value is found, returns `Ok(index)` with its index. Otherwise, returns `Err(index)`, where `index` is the insertion index
+    /// that would maintain sorting order.
     ///
-    /// Calling `bsearch_custom` on an unsorted array results in unspecified behavior.
+    /// Calling `bsearch_by` on an unsorted array results in unspecified behavior. Consider using [`sort_by()`] to ensure
+    /// the sorting order is compatible with your callable's ordering.
+    #[cfg(since_api = "4.2")]
+    pub fn bsearch_by<F>(&self, mut func: F) -> Result<usize, usize>
+    where
+        F: FnMut(&T) -> cmp::Ordering + 'static,
+    {
+        // Early exit; later code relies on index 0 being present.
+        if self.is_empty() {
+            return Err(0);
+        }
+
+        // We need one dummy element of type T, because Godot's bsearch_custom() checks types (so Variant::nil() can't be passed).
+        // Optimization: roundtrip Variant -> T -> Variant could be avoided, but anyone needing speed would use Rust binary search...
+        let ignored_value = self.at(0);
+        let ignored_value = <T as ParamType>::owned_to_arg(ignored_value);
+
+        let godot_comparator = |args: &[&Variant]| {
+            let value = T::from_variant(args[0]);
+            let is_less = matches!(func(&value), cmp::Ordering::Less);
+
+            Ok(is_less.to_variant())
+        };
+
+        let debug_name = std::any::type_name::<F>();
+        let index = Callable::with_scoped_fn(debug_name, godot_comparator, |pred| {
+            self.bsearch_custom(ignored_value, pred)
+        });
+
+        if let Some(value_at_index) = self.get(index) {
+            if func(&value_at_index) == cmp::Ordering::Equal {
+                return Ok(index);
+            }
+        }
+
+        Err(index)
+    }
+
+    /// Finds the index of a value in a sorted array using binary search, with `Callable` custom predicate.
+    ///
+    /// The callable `pred` takes two elements `(a, b)` and should return if `a < b` (strictly less).
+    /// For a type-safe version, check out [`bsearch_by()`][Self::bsearch_by].
     ///
-    /// Consider using `sort_custom()` to ensure the sorting order is compatible with
-    /// your callable's ordering
-    pub fn bsearch_custom(&self, value: impl AsArg<T>, func: &Callable) -> usize {
+    /// If the value is not present in the array, returns the insertion index that would maintain sorting order.
+    ///
+    /// Calling `bsearch_custom` on an unsorted array results in unspecified behavior. Consider using `sort_custom()` to ensure
+    /// the sorting order is compatible with your callable's ordering.
+    pub fn bsearch_custom(&self, value: impl AsArg<T>, pred: &Callable) -> usize {
         meta::arg_into_ref!(value: T);
 
         to_usize(
             self.as_inner()
-                .bsearch_custom(&value.to_variant(), func, true),
+                .bsearch_custom(&value.to_variant(), pred, true),
         )
     }
 
@@ -654,20 +697,55 @@ impl<T: ArrayElement> Array<T> {
 
     /// Sorts the array.
     ///
-    /// Note: The sorting algorithm used is not [stable](https://en.wikipedia.org/wiki/Sorting_algorithm#Stability).
-    /// This means that values considered equal may have their order changed when using `sort_unstable`.
+    /// The sorting algorithm used is not [stable](https://en.wikipedia.org/wiki/Sorting_algorithm#Stability).
+    /// This means that values considered equal may have their order changed when using `sort_unstable`. For most variant types,
+    /// this distinction should not matter though.
+    ///
+    /// _Godot equivalent: `Array.sort()`_
     #[doc(alias = "sort")]
     pub fn sort_unstable(&mut self) {
         // SAFETY: We do not write any values that don't already exist in the array, so all values have the correct type.
         unsafe { self.as_inner_mut() }.sort();
     }
 
-    /// Sorts the array.
+    /// Sorts the array, using a type-safe comparator.
+    ///
+    /// The predicate expects two parameters `(a, b)` and should return an ordering relation. For example, simple ascending ordering of the
+    /// elements themselves would be achieved with `|a, b| a.cmp(b)`.
+    ///
+    /// The sorting algorithm used is not [stable](https://en.wikipedia.org/wiki/Sorting_algorithm#Stability).
+    /// This means that values considered equal may have their order changed when using `sort_unstable_by`. For most variant types,
+    /// this distinction should not matter though.
+    #[cfg(since_api = "4.2")]
+    pub fn sort_unstable_by<F>(&mut self, mut func: F)
+    where
+        F: FnMut(&T, &T) -> cmp::Ordering,
+    {
+        let godot_comparator = |args: &[&Variant]| {
+            let lhs = T::from_variant(args[0]);
+            let rhs = T::from_variant(args[1]);
+            let is_less = matches!(func(&lhs, &rhs), cmp::Ordering::Less);
+
+            Ok(is_less.to_variant())
+        };
+
+        let debug_name = std::any::type_name::<F>();
+        Callable::with_scoped_fn(debug_name, godot_comparator, |pred| {
+            self.sort_unstable_custom(pred)
+        });
+    }
+
+    /// Sorts the array, using type-unsafe `Callable` comparator.
+    ///
+    /// For a type-safe variant of this method, use [`sort_unstable_by()`][Self::sort_unstable_by].
+    ///
+    /// The callable expects two parameters `(lhs, rhs)` and should return a bool `lhs < rhs`.
     ///
-    /// Uses the provided `Callable` to determine ordering.
+    /// The sorting algorithm used is not [stable](https://en.wikipedia.org/wiki/Sorting_algorithm#Stability).
+    /// This means that values considered equal may have their order changed when using `sort_unstable_custom`.For most variant types,
+    /// this distinction should not matter though.
     ///
-    /// Note: The sorting algorithm used is not [stable](https://en.wikipedia.org/wiki/Sorting_algorithm#Stability).
-    /// This means that values considered equal may have their order changed when using `sort_unstable_custom`.
+    /// _Godot equivalent: `Array.sort_custom()`_
     #[doc(alias = "sort_custom")]
     pub fn sort_unstable_custom(&mut self, func: &Callable) {
         // SAFETY: We do not write any values that don't already exist in the array, so all values have the correct type.

itest/rust/src/builtin_tests/containers/array_test.rs

@@ -197,17 +197,6 @@ fn array_first_last() {
     assert_eq!(empty_array.back(), None);
 }
 
-#[itest]
-fn array_binary_search() {
-    let array = array![1, 3];
-
-    assert_eq!(array.bsearch(0), 0);
-    assert_eq!(array.bsearch(1), 0);
-    assert_eq!(array.bsearch(2), 1);
-    assert_eq!(array.bsearch(3), 1);
-    assert_eq!(array.bsearch(4), 2);
-}
-
 #[itest]
 fn array_find() {
     let array = array![1, 2, 1];
@@ -298,13 +287,6 @@ fn array_extend() {
     assert_eq!(array, array![1, 2, 3, 4]);
 }
 
-#[itest]
-fn array_sort() {
-    let mut array = array![2, 1];
-    array.sort_unstable();
-    assert_eq!(array, array![1, 2]);
-}
-
 #[itest]
 fn array_reverse() {
     let mut array = array![1, 2];
@@ -474,18 +456,57 @@ fn array_should_format_with_display() {
     assert_eq!(format!("{a}"), "[]");
 }
 
+#[itest]
+fn array_sort_unstable() {
+    let mut array = array![2, 1];
+    array.sort_unstable();
+    assert_eq!(array, array![1, 2]);
+}
+
 #[itest]
 #[cfg(since_api = "4.2")]
-fn array_sort_custom() {
+fn array_sort_unstable_by() {
+    let mut array: Array<i32> = array![2, 1, 4, 3];
+    array.sort_unstable_by(|a, b| a.cmp(b));
+    assert_eq!(array, array![1, 2, 3, 4]);
+}
+
+#[itest]
+#[cfg(since_api = "4.2")]
+fn array_sort_unstable_custom() {
     let mut a = array![1, 2, 3, 4];
     let func = backwards_sort_callable();
     a.sort_unstable_custom(&func);
     assert_eq!(a, array![4, 3, 2, 1]);
 }
 
+#[itest]
+fn array_bsearch() {
+    let array = array![1, 3];
+
+    assert_eq!(array.bsearch(0), 0);
+    assert_eq!(array.bsearch(1), 0);
+    assert_eq!(array.bsearch(2), 1);
+    assert_eq!(array.bsearch(3), 1);
+    assert_eq!(array.bsearch(4), 2);
+}
+
+#[itest]
+#[cfg(since_api = "4.2")]
+fn array_bsearch_by() {
+    let a: Array<i32> = array![1, 2, 4, 5];
+
+    assert_eq!(a.bsearch_by(|e| e.cmp(&2)), Ok(1));
+    assert_eq!(a.bsearch_by(|e| e.cmp(&4)), Ok(2));
+
+    assert_eq!(a.bsearch_by(|e| e.cmp(&0)), Err(0));
+    assert_eq!(a.bsearch_by(|e| e.cmp(&3)), Err(2));
+    assert_eq!(a.bsearch_by(|e| e.cmp(&9)), Err(4));
+}
+
 #[itest]
 #[cfg(since_api = "4.2")]
-fn array_binary_search_custom() {
+fn array_bsearch_custom() {
     let a = array![5, 4, 2, 1];
     let func = backwards_sort_callable();
     assert_eq!(a.bsearch_custom(1, &func), 3);

itest/rust/src/builtin_tests/containers/callable_test.rs

@@ -7,7 +7,8 @@
 
 use crate::framework::itest;
 use godot::builtin::{
-    array, varray, Array, Callable, GString, NodePath, StringName, Variant, VariantArray, Vector2,
+    array, dict, varray, Array, Callable, Color, GString, NodePath, StringName, Variant,
+    VariantArray, Vector2,
 };
 use godot::classes::{Node2D, Object, RefCounted};
 use godot::init::GdextBuild;
@@ -97,6 +98,43 @@ fn callable_object_method() {
     assert_eq!(callable.object(), None);
 }
 
+#[itest]
+#[cfg(since_api = "4.3")]
+fn callable_variant_method() {
+    // Dictionary
+    let dict = dict! { "one": 1, "value": 2 };
+    let dict_get = Callable::from_variant_method(&dict.to_variant(), "get");
+    assert_eq!(dict_get.call(&["one".to_variant()]), 1.to_variant());
+
+    // GString
+    let string = GString::from("some string").to_variant();
+    let string_md5 = Callable::from_variant_method(&string, "md5_text");
+    assert_eq!(
+        string_md5.call(&[]),
+        "5ac749fbeec93607fc28d666be85e73a".to_variant()
+    );
+
+    // Object
+    let obj = CallableTestObj::new_gd().to_variant();
+    let obj_stringify = Callable::from_variant_method(&obj, "stringify_int");
+    assert_eq!(obj_stringify.call(&[10.to_variant()]), "10".to_variant());
+
+    // Vector3
+    let vector = Vector2::new(-1.2, 2.5).to_variant();
+    let vector_round = Callable::from_variant_method(&vector, "round");
+    assert_eq!(vector_round.call(&[]), Vector2::new(-1.0, 3.0).to_variant());
+
+    // Color
+    let color = Color::from_rgba8(255, 0, 127, 255).to_variant();
+    let color_to_html = Callable::from_variant_method(&color, "to_html");
+    assert_eq!(color_to_html.call(&[]), "ff007fff".to_variant());
+
+    // Color - invalid method.
+    let color = Color::from_rgba8(255, 0, 127, 255).to_variant();
+    let color_to_html = Callable::from_variant_method(&color, "to_htmI");
+    assert!(!color_to_html.is_valid());
+}
+
 #[itest]
 #[cfg(since_api = "4.4")]
 fn callable_static() {