Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Joezeo opened this issue Oct 10, 2024 · 1 comment
Open

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

Joezeo opened this issue Oct 10, 2024 · 1 comment
Labels
c: core Core components c: ffi Low-level components and interaction with GDExtension API feature Adds functionality to the library

Comments

@Joezeo
Copy link

Joezeo commented Oct 10, 2024

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
Copy link
Member

Bromeon commented Oct 10, 2024

This is a variant of #338.

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

  • on_input binds the object mutably on the emit side
  • _on_state_transition binds the object mutably in the signal callback

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.

@Bromeon Bromeon added c: core Core components c: ffi Low-level components and interaction with GDExtension API feature Adds functionality to the library labels Oct 10, 2024
@Bromeon Bromeon changed the title Panic already bound when emit signal in virtual function. Re-entrant emitting of signals + script-virtual functions Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: core Core components c: ffi Low-level components and interaction with GDExtension API feature Adds functionality to the library
Projects
None yet
Development

No branches or pull requests

2 participants