godot-rust / gdext

Rust bindings for Godot 4
https://mastodon.gamedev.place/@GodotRust
Mozilla Public License 2.0
3.1k stars 198 forks source link

Re-entrant emitting of signals + script-virtual functions #916

Open Joezeo opened 1 month ago

Joezeo commented 1 month ago

Here is the sample code

#[derive(GodotClass)]
#[class(init, base = Node)]
pub struct State {
    base: Base<Node>,
}

#[godot_api]
impl State {
    #[signal]
    pub fn state_transition(to: i32);

    ...

    #[func(gd_self, virtual)]
    pub fn on_input(gd: Gd<Self>, event: Gd<InputEvent>) {}

    ...
}

#[derive(GodotClass)]
#[class(init, base = Node)]
pub struct FsmMgr {
    #[export]
    initial_state: Option<Gd<State>>,
    current_state: Option<Gd<State>>,

    states: HashMap<i32, Gd<State>>,
    base: Base<Node>,
}

#[godot_api]
impl INode for FsmMgr {
    fn ready(&mut self) {
        ...

        let children = self.base_mut().get_children();
        for child in children.iter_shared() {
            if let Ok(mut state) = child.try_cast::<State>() {
                state.connect(
                    "state_transition".into(),
                    Callable::from_object_method(&self.base_mut(), "_on_state_transition"),
                );

        ...
            }
        }
    }

    fn input(&mut self, event: Gd<InputEvent>) {
        if let Some(cur_state) = self.current_state.as_mut() {
            State::on_input(cur_state.to_godot(), event);
        }
    }

    ...
}

#[godot_api]
impl FsmMgr {
    #[func]
    pub fn _on_state_transition(&mut self, to: i32) {
        ...
    }
}

State's on_input function was overridden in GDScript, and emitted the signal state_transition there, causing the panic.

E 0:00:02:0079   idle_state.gd:20 @ _on_input(): godot-rust function call failed: FsmMgr::_on_state_transition()
    Reason: [panic]
  Gd<T>::bind_mut() failed, already bound; T = game_builtin::fsm::fsm_mgr::FsmMgr.
    Make sure to use `self.base_mut()` instead of `self.to_gd()` when possible.
    Details: cannot borrow while accessible mutable borrow exists.
  <C++ Source>   C:\Users\user\.cargo\git\checkouts\gdext-76630c89719e160c\8ad9cb0\godot-core\src\private.rs:332 @ godot_core::private::report_call_error()
  <Stack Trace>  idle_state.gd:20 @ _on_input()

I tried to mark the _on_state_transition function with gd_self. There was no panic if it was an empty function, but it still panicked when I tried to call some functions on Gd\<Self>.

Bromeon commented 1 month ago

This is a variant of https://github.com/godot-rust/gdext/issues/338.

The TLDR is that Rust cannot borrow-check through FFI calls, so:

This causes a double runtime borrow.

We should see if #[func(virtual)] and/or signals can use "re-borrowing", i.e. the base_mut() pattern, to work around it. This needs some design though.