Safely integrate Godot cross-thread calls (e.g. `AudioStreamPlayback::mix`)
- Dominant language
- Rust
- Stars
- 5.2k
- Forks
- 312
- Avg merge
- 11h 10m
- Merged PRs (30d)
- 10
Description
This is a rough proposal for how to safely implement functions that currently require `experimental-threads`. It's not completely fleshed out but I wanted to see if there's any interest before I expand on it further.
If you're not taking proposals on how to solve this problem at the moment, or you don't find this proposal useful, I won't be offended if you close the issue.
Take `AudioStreamPlayback` as an example:
```rust
unsafe fn mix(&mut self, buffer: * mut AudioFrame, rate_scale: f32, frames: i32,) -> i32;
```
Godot calls `mix` from a non-main thread, which means that access to `self` in this function is unsound. (Without `experimental-threads` this condition is checked at runtime by godot-rust, which will panic before `mix` is even called).
My proposal is to remove the `self` parameter from functions that are called from a non-main thread, and replace it with a user-defined `state` parameter, which must impl `Clone + Send + 'static` and can therefore be passed between threads safely. For example:
```rust
trait IAudioStreamPlayback /* : ... */
where
AudioState: Clone + Send + 'static
{
/* ... */
unsafe fn mix(state: AudioState, buffer: * mut AudioFrame, rate_scale: f32, frames: i32) -> i32;
/* ... */
}
```
A sensible choice for `AudioState` might be for example `Arc>`, where `S` is some arbitrary struct. The `AudioState` could then be safely read and mutated from multiple threads. But the idea is that the actual type of `AudioState` should be left up to the programmer as long as it implements `Clone + Send + 'static`.
The state object should be attached to the class struct, for example:
```rust
#[derive(GodotClass)]
#[class(base=AudioStreamPlayback)]
struct Example {
#[audio_state]
state: Arc>,
}
#[godot_api]
impl IAudioStreamPlayback> for Example {
unsafe fn mix(state: Arc>, buffer: * mut AudioFrame, rate_scale: f32, frames: i32) -> i32 {
/ * ... */
frames
}
}
struct ExampleState {
position: usize,
}
```
godot-rust would then have some glue code that takes care of cloning `state` and passing it through to `IAudioStreamPlayback::mix` when `mix` is called from Godot.
It would be illegal to mark a field `#[export]` if it is also marked `#[audio_state]`.
This approach is roughly what I'm using in my own project, although I of course have to implement the glue manually each time I extend `AudioStreamPlayback`.
I *think* this general approach should be logically extensible to other cases where Godot calls functions from non-main threads.
If you're interested in something like this approach I would be interested in having a go at implementing it.
Contributor guide
Research direction
Start with the AudioStreamPlayback::mix example and the existing experimental-threads behavior described in the issue. Trace where Godot invokes non-main-thread methods and where GodotClass and godot_api glue would need to carry state. Done would mean a settled, implementable design for Clone + Send + 'static state, including export restrictions and applicability to other cross-thread calls.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- godot, rust
- Domain
- game-dev
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100