Double runtime borrow when signal callback mutably change attribute
- 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
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