Single- and multi-threading support for Callable::from_fn

[MatrixDev]

Callable::from_fn requires Send + Sync from closures but Gd<T> is not. I saw in few bugs Callable::from_fn is mentioned as an alternative to using function name Base::callable / Callable::from_object_method (which are not type safe because function name can be misspelled) but it doesn't really work.

Basically this limitation limits usefulness of Callable::from_fn quite a bit.

Possibly related to #18 but usecase is different. Maybe Callable can be adjusted instead of Gd<T>.

Code

#[derive(GodotClass)]
#[class(init, base = Node3D)]
pub struct TestNode {
    base: Base<Node3D>,
    #[export]
    button: Option<Gd<Button>>,
}

#[godot_api]
impl TestNode {
    fn on_clicked(&mut self) {}
}

#[godot_api]
impl INode3D for TestNode {
    fn ready(&mut self) {
        if let Some(mut button) = &self.button {
            let mut this = self.to_gd();
            let callback = Callable::from_fn("callback", move |args| {
                this.bind_mut().on_clicked();
                Ok(Variant::nil())
            });
            button.connect("pressed".into(), callback);
        }
    }
}

Error

error[E0277]: `*mut TestNode` cannot be sent between threads safely
   --> src/nodes/screens/ship_edit.rs:101:56
    |
101 |               let callback = Callable::from_fn("sensor", move |args| {
    |                              -----------------           ^----------
    |                              |                           |
    |  ____________________________|___________________________within this `{closure@src/nodes/screens/ship_edit.rs:101:56: 101:67}`
    | |                            |
    | |                            required by a bound introduced by this call
102 | |                 this.bind_mut().on_clicked();
103 | |                 Ok(Variant::nil())
104 | |             });
    | |_____________^ `*mut TestNode` cannot be sent between threads safely
    |
    = help: within `{closure@src/nodes/screens/ship_edit.rs:101:56: 101:67}`, the trait `Send` is not implemented for `*mut TestNode`
note: required because it appears within the type `RawGd<TestNode>`
   --> /Users/matrixdev/.cargo/git/checkouts/gdext-76630c89719e160c/df302ea/godot-core/src/obj/raw.rs:30:12
    |
30  | pub struct RawGd<T: GodotClass> {
    |            ^^^^^
note: required because it appears within the type `Gd<TestNode>`
   --> /Users/matrixdev/.cargo/git/checkouts/gdext-76630c89719e160c/df302ea/godot-core/src/obj/gd.rs:87:12
    |
87  | pub struct Gd<T: GodotClass> {
    |            ^^
note: required because it's used within this closure
   --> src/nodes/screens/ship_edit.rs:101:56
    |
101 |             let callback = Callable::from_fn("sensor", move |args| {
    |                                                        ^^^^^^^^^^^
note: required by a bound in `godot::prelude::Callable::from_fn`
   --> /Users/matrixdev/.cargo/git/checkouts/gdext-76630c89719e160c/df302ea/godot-core/src/builtin/callable.rs:95:22
    |
93  |     pub fn from_fn<F, S>(name: S, rust_function: F) -> Self
    |            ------- required by a bound in this associated function
94  |     where
95  |         F: 'static + Send + Sync + FnMut(&[&Variant]) -> Result<Variant, ()>,
    |                      ^^^^ required by this bound in `Callable::from_fn`

[Bromeon]

Callable::from_fn requires Send + Sync from closures but Gd<T> is not.

This is mostly a direct consequence of #18 not yet being implemented.

Mid-term, I see the option to have two functions:

1. Callable::from_unsync_fn()

   • This would record the thread ID on creation and only allow execution on the same thread.
   • Even if called from GDScript, it could simply skip the user code and print an error if invoked from a different thread.
   • It would not require Send/Sync.

2. Callable::from_sync_fn()

   • Requires Send/Sync and can be called from any thread.
   • To be actually useful, it needs threading support of objects.

[Houtamelo]

@Bromeon I'm working on the Signal support PR, and while testing my design I remembered that callable requiring Send + Sync essentially means you can't interact with Godot from inside them, which extremely limits their usefulness.

In my experience, 95% of the time callables were used to connect to signals, and so I had to make this workaround that bypasses the Send + Sync: https://github.com/Houtamelo/houtamelo_utils_gdext/blob/main/src/connect_with_deferred.rs. I use it every single time I want to connect to a signal because otherwise there isn't much I can do with the Send + Sync bounds in place.

I think it would be reasonable if we at least had a from_fn_unchecked that's marked with unsafe.

[Bromeon]

I think it would be reasonable if we at least had a from_fn_unchecked that's marked with unsafe.

Fair point, maybe it's worth providing this as an interim solution until we have a solid design. We should probably explicitly state in the docs that such an API will be removed once there is a proper solution.