Skip to content

Commit

Permalink
Treat Callable as by-ref in APIs
Browse files Browse the repository at this point in the history
  • Loading branch information
Bromeon committed Nov 5, 2024
1 parent af8c1e1 commit 856842c
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 23 deletions.
2 changes: 1 addition & 1 deletion examples/dodge-the-creeps/rust/src/hud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl Hud {
self.show_message("Game Over".into());

let mut timer = self.base().get_tree().unwrap().create_timer(2.0).unwrap();
timer.connect("timeout", self.base().callable("show_start_button"));
timer.connect("timeout", &self.base().callable("show_start_button"));
}

#[func]
Expand Down
2 changes: 1 addition & 1 deletion examples/dodge-the-creeps/rust/src/main_scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl Main {
mob.set_linear_velocity(Vector2::new(range, 0.0).rotated(real::from_f32(direction)));

let mut hud = self.base().get_node_as::<Hud>("Hud");
hud.connect("start_game", mob.callable("on_start_game"));
hud.connect("start_game", &mob.callable("on_start_game"));
}

fn music(&mut self) -> &mut AudioStreamPlayer {
Expand Down
5 changes: 4 additions & 1 deletion godot-codegen/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,12 @@ impl<'a> Context<'a> {
// Already handled separately.
debug_assert!(!godot_ty.ty.starts_with("Packed"));

// IMPORTANT: Keep this in sync with impl_ffi_variant!() macros taking `ref` or not.

// Arrays are also handled separately, and use ByRef.
match godot_ty.ty.as_str() {
"Variant" | "Array" | "Dictionary" => ArgPassing::ByRef,
// Note: Signal is currently not used in any parameter, but this may change.
"Variant" | "Array" | "Dictionary" | "Callable" | "Signal" => ArgPassing::ByRef,
"String" | "StringName" | "NodePath" => ArgPassing::ImplAsArg,
_ => ArgPassing::ByValue,
}
Expand Down
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 @@ -614,7 +614,7 @@ impl<T: ArrayElement> Array<T> {
///
/// 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 {
pub fn bsearch_custom(&self, value: impl AsArg<T>, func: &Callable) -> usize {
meta::arg_into_ref!(value: T);

to_usize(
Expand Down Expand Up @@ -646,7 +646,7 @@ impl<T: ArrayElement> Array<T> {
/// 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`.
#[doc(alias = "sort_custom")]
pub fn sort_unstable_custom(&mut self, func: Callable) {
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.
unsafe { self.as_inner_mut() }.sort_custom(func);
}
Expand Down
6 changes: 3 additions & 3 deletions godot-core/src/builtin/signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl Signal {
/// returns [`Error::ERR_INVALID_PARAMETER`] and
/// pushes an error message, unless the signal is connected with [`ConnectFlags::REFERENCE_COUNTED`](crate::classes::object::ConnectFlags::REFERENCE_COUNTED).
/// To prevent this, use [`Self::is_connected`] first to check for existing connections.
pub fn connect(&self, callable: Callable, flags: i64) -> Error {
pub fn connect(&self, callable: &Callable, flags: i64) -> Error {
let error = self.as_inner().connect(callable, flags);

Error::from_godot(error as i32)
Expand All @@ -83,7 +83,7 @@ impl Signal {
/// Disconnects this signal from the specified [`Callable`].
///
/// If the connection does not exist, generates an error. Use [`Self::is_connected`] to make sure that the connection exists.
pub fn disconnect(&self, callable: Callable) {
pub fn disconnect(&self, callable: &Callable) {
self.as_inner().disconnect(callable);
}

Expand Down Expand Up @@ -142,7 +142,7 @@ impl Signal {
}

/// Returns `true` if the specified [`Callable`] is connected to this signal.
pub fn is_connected(&self, callable: Callable) -> bool {
pub fn is_connected(&self, callable: &Callable) -> bool {
self.as_inner().is_connected(callable)
}

Expand Down
6 changes: 3 additions & 3 deletions itest/rust/src/builtin_tests/containers/array_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ fn array_should_format_with_display() {
fn array_sort_custom() {
let mut a = array![1, 2, 3, 4];
let func = backwards_sort_callable();
a.sort_unstable_custom(func);
a.sort_unstable_custom(&func);
assert_eq!(a, array![4, 3, 2, 1]);
}

Expand All @@ -488,8 +488,8 @@ fn array_sort_custom() {
fn array_binary_search_custom() {
let a = array![5, 4, 2, 1];
let func = backwards_sort_callable();
assert_eq!(a.bsearch_custom(1, func.clone()), 3);
assert_eq!(a.bsearch_custom(3, func), 2);
assert_eq!(a.bsearch_custom(1, &func), 3);
assert_eq!(a.bsearch_custom(3, &func), 2);
}

#[cfg(since_api = "4.2")]
Expand Down
17 changes: 7 additions & 10 deletions itest/rust/src/builtin_tests/containers/signal_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

use godot::builtin::{Callable, GString, Signal, StringName, Variant};
use godot::builtin::{Callable, GString, Signal, StringName};
use godot::meta::ToGodot;
use godot::register::{godot_api, GodotClass};
use std::cell::Cell;
Expand Down Expand Up @@ -71,18 +71,15 @@ fn signals() {

let args = [
vec![],
vec![Variant::from(987)],
vec![
Variant::from(receiver.clone()),
Variant::from(SIGNAL_ARG_STRING),
],
vec![987.to_variant()],
vec![receiver.to_variant(), SIGNAL_ARG_STRING.to_variant()],
];

for (i, arg) in args.iter().enumerate() {
let signal_name = format!("signal_{i}_arg");
let receiver_name = format!("receive_{i}_arg");

emitter.connect(&signal_name, receiver.callable(receiver_name));
emitter.connect(&signal_name, &receiver.callable(receiver_name));
emitter.emit_signal(&signal_name, arg);

assert!(receiver.bind().used[i].get());
Expand Down Expand Up @@ -117,7 +114,7 @@ fn emit_signal() {

object.connect(
&StringName::from("test_signal"), // explicit StringName
Callable::from_object_method(&receiver, "receive_1_arg"),
&Callable::from_object_method(&receiver, "receive_1_arg"),
);
assert_eq!(signal.connections().len(), 1);

Expand All @@ -136,7 +133,7 @@ fn connect_signal() {
let signal = Signal::from_object_signal(&object, "test_signal");
let receiver = Receiver::new_alloc();

signal.connect(Callable::from_object_method(&receiver, "receive_1_arg"), 0);
signal.connect(&Callable::from_object_method(&receiver, "receive_1_arg"), 0);
assert_eq!(signal.connections().len(), 1);

object.emit_signal("test_signal", &[987i64.to_variant()]);
Expand Down Expand Up @@ -277,7 +274,7 @@ mod custom_callable {

let received = Arc::new(AtomicU32::new(0));
let callable = callable(received.clone());
signal.connect(callable, 0);
signal.connect(&callable, 0);

emit(&mut node);

Expand Down
2 changes: 1 addition & 1 deletion itest/rust/src/object_tests/object_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,7 @@ fn double_use_reference() {
emitter
.clone()
.upcast::<Object>()
.connect("do_use", double_use.callable("use_1"));
.connect("do_use", &double_use.callable("use_1"));

let guard = double_use.bind();

Expand Down
2 changes: 1 addition & 1 deletion itest/rust/src/object_tests/reentrant_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ fn reentrant_emit_succeeds() {
let mut class = ReentrantClass::new_alloc();

let callable = class.callable("second");
class.connect("some_signal", callable);
class.connect("some_signal", &callable);

assert!(!class.bind().first_called_pre);
assert!(!class.bind().first_called_post);
Expand Down

0 comments on commit 856842c

Please sign in to comment.