godot-rust / godot-rust/gdext

Double runtime borrow when signal callback mutably change attribute

Open
#1,697 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
5.2k
Forks
312
Avg merge
11h 10m
Merged PRs (30d)
10

Description

This issue might be related to #1692 , #916, and #713.

Based on [Firebelley's video about Composition Pattern ](https://youtu.be/rCu8vQrdDDI?si=bFNTC4H2ZntMDPt0), I'm trying to tear down each of functionality to the lowest composition possible.

My case are something like this:
- I have `Player`, which is a `GodotClass` and has `Health` & `State`
- `Health` is a `GodotClass` that only handle health/hitpoint, like `take_damage()` and `take_heal()`, and will emit `dead` signal when hitpoint reaches 0. This class doesn't need to know about its parent or where it is being used.
- `State` is a simple Rust enum that holds `State::Alive` and `State::Dead`
- Inside `Player`'s ready hook, I want to connect `Health.dead` signal to a closure (or function, doesn't really matter), that change `Player.state` from `State::Alive` to `State::Dead`

`Player` code look like this:

```rust
#[derive(GodotClass)]
#[class(init, base=CharacterBody2D)]
pub struct Player {
#[export]
health: OnEditor>,
state: State,
base: Base,
}
```

and `Health` code look like this:

```rust
#[derive(GodotClass)]
#[class(init, base=Node)]
pub struct Health {
#[export]
#[init(val = 100)]
current: u32,
base: Base,
}

#[godot_api]
impl Health {
#[func]
pub fn take_damage(&mut self, value: u32) {
// ...
// if dead, emit dead signal
// ...
self.signals().dead().emit()
}

#[signal]
pub fn dead();
}
```

and finally the `Player` implementation that I have trouble with:

```rust
#[godot_api]
impl ICharacterBody2D for Player {
fn ready(&mut self) {
// on ready: connect to Health.dead signal to closure/callback
// that change Player state

let health_gd: Gd = self.health.clone();
let mut player_gd = self.to_gd();
health_gd.signals().dead().connect(move || {

// these 2 lines will trigger panic at runtime
let mut player_gd = player_gd.bind_mut();
player_gd.state = PlayerState::Dead;

});
}
}
```

I have tried several approach: using closure like above, or connecting to local method like so:

```rust
#[godot_api]
impl ICharacterBody2D for Player {
fn ready(&mut self) {
self.health
.signals()
.dead()
.connect_other(&self.to_gd(), Self::on_health_dead);
}
}

#[godot_api]
impl Player {
fn on_health_dead(&mut self) {
// entering this function alone seems to already trigger the multiple borrow panic
// even when the line below is being commented
self.state = PlayerState::Dead;
}
}

```

My understanding so far is that such code trigger double borrow at run time:

```
Player has &mut self


Health.dead is emitted and trigger a Godot callback (closure or function)


callback tries to access Player again


Gd::bind_mut()

X
already borrowed
```

I'd be grateful for any direction on the recommended approach of implementing Composition Pattern. Thankyou!

---

**Update**

I was able to get this working (I think), by defer the signal call like so:

```rust
#[godot_api]
impl ICharacterBody2D for Player {
fn ready(&mut self) {
let health_dead_signal = self.health.signals().dead();
health_dead_signal
.builder()
.flags(ConnectFlags::DEFERRED)
.connect_other_mut(&self.to_gd(), Self::on_health_dead);
}
}
```

I will try this approach with more case, but I'm a bit concerned about how scalable this pattern is. If I end up deferring multiple dependent signals across systems, does Godot guarantee they all process within the current frame? I worry some callbacks might get delayed to the next frame and cause race conditions.

Take this scenario: `Health` emits dead, `Player` receives it via a deferred signal and then propagates a new `dead` signal to `Level`—which is also deferred. Since both are deferred, could `Level` end up reading the player's state before `Player` actually finishes processing its own dead signal? I'm concerned about signal ordering issues when chaining deferred connections.

Contributor guide

Open the contributing guide

Research direction

Start with the Player ready hook, Health::take_damage, and Player::on_health_dead shown in the issue, reproducing the immediate and DEFERRED signal connections. Determine the safe composition approach and callback ordering for chained signals, including what guarantees apply within a frame; done means the runtime borrow panic and ordering concerns have a documented, supported resolution.

Written by the indexing model from the issue text.

Assessment

Tech stack
godot, rust
Domain
game-dev
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.