Skip to content

Commit 78ca889

Browse files
authored
Merge pull request #1093 from PgBiel/fix-wasm-threading
Wasm threading fixes
2 parents 18d5707 + fd752ba commit 78ca889

File tree

4 files changed

+99
-30
lines changed

4 files changed

+99
-30
lines changed

godot-core/src/init/mod.rs

+42
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,48 @@ pub unsafe trait ExtensionLibrary {
282282
// Nothing by default.
283283
}
284284

285+
/// Whether to override the Wasm binary filename used by your GDExtension which the library should expect at runtime. Return `None`
286+
/// to use the default where gdext expects either `{YourCrate}.wasm` (default binary name emitted by Rust) or
287+
/// `{YourCrate}.threads.wasm` (for builds producing separate single-threaded and multi-threaded binaries).
288+
///
289+
/// Upon exporting a game to the web, the library has to know at runtime the exact name of the `.wasm` binary file being used to load
290+
/// each GDExtension. By default, Rust exports the binary as `cratename.wasm`, so that is the name checked by godot-rust by default.
291+
///
292+
/// However, if you need to rename that binary, you can make the library aware of the new binary name by returning
293+
/// `Some("newname.wasm")` (don't forget to **include the `.wasm` extension**).
294+
///
295+
/// For example, to have two simultaneous versions, one supporting multi-threading and the other not, you could add a suffix to the
296+
/// filename of the Wasm binary of the multi-threaded version in your build process. If you choose the suffix `.threads.wasm`,
297+
/// you're in luck as godot-rust already accepts this suffix by default, but let's say you want to use a different suffix, such as
298+
/// `-with-threads.wasm`. For this, you can have a `"nothreads"` feature which, when absent, should produce a suffixed binary,
299+
/// which can be informed to gdext as follows:
300+
///
301+
/// ```no_run
302+
/// # use godot::init::*;
303+
/// struct MyExtension;
304+
///
305+
/// #[gdextension]
306+
/// unsafe impl ExtensionLibrary for MyExtension {
307+
/// fn override_wasm_binary() -> Option<&'static str> {
308+
/// // Binary name unchanged ("mycrate.wasm") without thread support.
309+
/// #[cfg(feature = "nothreads")]
310+
/// return None;
311+
///
312+
/// // Tell gdext we add a custom suffix to the binary with thread support.
313+
/// // Please note that this is not needed if "mycrate.threads.wasm" is used.
314+
/// // (You could return `None` as well in that particular case.)
315+
/// #[cfg(not(feature = "nothreads"))]
316+
/// Some("mycrate-with-threads.wasm")
317+
/// }
318+
/// }
319+
/// ```
320+
/// Note that simply overriding this method won't change the name of the Wasm binary produced by Rust automatically: you'll still
321+
/// have to rename it by yourself in your build process, as well as specify the updated binary name in your `.gdextension` file.
322+
/// This is just to ensure gdext is aware of the new name given to the binary, avoiding runtime errors.
323+
fn override_wasm_binary() -> Option<&'static str> {
324+
None
325+
}
326+
285327
/// Whether to enable hot reloading of this library. Return `None` to use the default behavior.
286328
///
287329
/// Enabling this will ensure that the library can be hot reloaded. If this is disabled then hot reloading may still work, but there is no

godot-core/src/private.rs

+10-10
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub use sys::out;
1717

1818
#[cfg(feature = "trace")]
1919
pub use crate::meta::trace;
20-
#[cfg(debug_assertions)]
20+
#[cfg(all(debug_assertions, not(wasm_nothreads)))]
2121
use std::cell::RefCell;
2222

2323
use crate::global::godot_error;
@@ -321,12 +321,12 @@ pub(crate) fn has_error_print_level(level: u8) -> bool {
321321
/// Internal type used to store context information for debug purposes. Debug context is stored on the thread-local
322322
/// ERROR_CONTEXT_STACK, which can later be used to retrieve the current context in the event of a panic. This value
323323
/// probably shouldn't be used directly; use ['get_gdext_panic_context()'](get_gdext_panic_context) instead.
324-
#[cfg(debug_assertions)]
324+
#[cfg(all(debug_assertions, not(wasm_nothreads)))]
325325
struct ScopedFunctionStack {
326326
functions: Vec<*const dyn Fn() -> String>,
327327
}
328328

329-
#[cfg(debug_assertions)]
329+
#[cfg(all(debug_assertions, not(wasm_nothreads)))]
330330
impl ScopedFunctionStack {
331331
/// # Safety
332332
/// Function must be removed (using [`pop_function()`](Self::pop_function)) before lifetime is invalidated.
@@ -351,7 +351,7 @@ impl ScopedFunctionStack {
351351
}
352352
}
353353

354-
#[cfg(debug_assertions)]
354+
#[cfg(all(debug_assertions, not(wasm_nothreads)))]
355355
thread_local! {
356356
static ERROR_CONTEXT_STACK: RefCell<ScopedFunctionStack> = const {
357357
RefCell::new(ScopedFunctionStack { functions: Vec::new() })
@@ -360,10 +360,10 @@ thread_local! {
360360

361361
// Value may return `None`, even from panic hook, if called from a non-Godot thread.
362362
pub fn get_gdext_panic_context() -> Option<String> {
363-
#[cfg(debug_assertions)]
363+
#[cfg(all(debug_assertions, not(wasm_nothreads)))]
364364
return ERROR_CONTEXT_STACK.with(|cell| cell.borrow().get_last());
365365

366-
#[cfg(not(debug_assertions))]
366+
#[cfg(not(all(debug_assertions, not(wasm_nothreads))))]
367367
None
368368
}
369369

@@ -378,10 +378,10 @@ where
378378
E: Fn() -> String,
379379
F: FnOnce() -> R + std::panic::UnwindSafe,
380380
{
381-
#[cfg(not(debug_assertions))]
382-
let _ = error_context; // Unused in Release.
381+
#[cfg(not(all(debug_assertions, not(wasm_nothreads))))]
382+
let _ = error_context; // Unused in Release or `wasm_nothreads` builds.
383383

384-
#[cfg(debug_assertions)]
384+
#[cfg(all(debug_assertions, not(wasm_nothreads)))]
385385
ERROR_CONTEXT_STACK.with(|cell| unsafe {
386386
// SAFETY: &error_context is valid for lifetime of function, and is removed from LAST_ERROR_CONTEXT before end of function.
387387
cell.borrow_mut().push_function(&error_context)
@@ -390,7 +390,7 @@ where
390390
let result =
391391
std::panic::catch_unwind(code).map_err(|payload| extract_panic_message(payload.as_ref()));
392392

393-
#[cfg(debug_assertions)]
393+
#[cfg(all(debug_assertions, not(wasm_nothreads)))]
394394
ERROR_CONTEXT_STACK.with(|cell| cell.borrow_mut().pop_function());
395395
result
396396
}

godot-core/src/task/async_runtime.rs

+1
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ pub fn spawn(future: impl Future<Output = ()> + 'static) -> TaskHandle {
5555
// By limiting async tasks to the main thread we can redirect all signal callbacks back to the main thread via `call_deferred`.
5656
//
5757
// Once thread-safe futures are possible the restriction can be lifted.
58+
#[cfg(not(wasm_nothreads))]
5859
assert!(
5960
crate::init::is_main_thread(),
6061
"godot_task() can only be used on the main thread"

godot-macros/src/gdextension.rs

+46-20
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,13 @@ pub fn attribute_gdextension(item: venial::Item) -> ParseResult<TokenStream> {
5858
// host. See: https://github.com/rust-lang/rust/issues/42587
5959
#[cfg(target_os = "emscripten")]
6060
fn emscripten_preregistration() {
61+
let pkg_name = env!("CARGO_PKG_NAME");
62+
let wasm_binary = <#impl_ty as ::godot::init::ExtensionLibrary>::override_wasm_binary()
63+
.map_or_else(
64+
|| std::string::String::from("null"),
65+
|bin| format!("'{}'", bin.replace("\\", "\\\\").replace("'", "\\'"))
66+
);
67+
6168
// Module is documented here[1] by emscripten, so perhaps we can consider it a part
6269
// of its public API? In any case for now we mutate global state directly in order
6370
// to get things working.
@@ -69,34 +76,53 @@ pub fn attribute_gdextension(item: venial::Item) -> ParseResult<TokenStream> {
6976
// involved, but I don't know what guarantees we have here.
7077
//
7178
// We should keep an eye out for these sorts of failures!
72-
let script = std::ffi::CString::new(concat!(
73-
"var pkgName = '", env!("CARGO_PKG_NAME"), "';", r#"
74-
var libName = pkgName.replaceAll('-', '_') + '.wasm';
75-
if (!(libName in LDSO.loadedLibsByName)) {
76-
// Always print to console, even if the error is suppressed.
77-
console.error(`godot-rust could not find the Wasm module '${libName}', needed to load the '${pkgName}' crate. Please ensure a file named '${libName}' exists in the game's web export files. This may require updating Wasm paths in the crate's corresponding '.gdextension' file, or just renaming the Wasm file to the correct name otherwise.`);
78-
throw new Error(`Wasm module '${libName}' not found. Check the console for more information.`);
79-
}
79+
let script = format!(
80+
r#"var pkgName = '{pkg_name}';
81+
var wasmBinary = {wasm_binary};
82+
if (wasmBinary === null) {{
83+
var snakePkgName = pkgName.replaceAll('-', '_');
84+
var normalLibName = snakePkgName + '.wasm';
85+
var threadedLibName = snakePkgName + '.threads.wasm';
86+
if (normalLibName in LDSO.loadedLibsByName) {{
87+
var libName = normalLibName;
88+
}} else if (threadedLibName in LDSO.loadedLibsByName) {{
89+
var libName = threadedLibName;
90+
}} else {{
91+
// Always print to console, even if the error is suppressed.
92+
console.error(`godot-rust could not find the Wasm module '${{normalLibName}}' nor '${{threadedLibName}}', one of which is needed by default to load the '${{pkgName}}' crate. This indicates its '.wasm' binary file was renamed to an unexpected name.\n\nPlease ensure its Wasm binary file has one of those names in the game's web export files. This may require updating Wasm paths in the crate's corresponding '.gdextension' file, or just renaming the Wasm file to one of the expected names otherwise.\n\nIf that GDExtension uses a different Wasm filename, please ensure it informs this new name to godot-rust by returning 'Some("newname.wasm")' from 'ExtensionLibrary::override_wasm_binary'.`);
93+
throw new Error(`Wasm module '${{normalLibName}}' not found. Check the console for more information.`);
94+
}}
95+
}} else if (!wasmBinary.endsWith(".wasm")) {{
96+
console.error(`godot-rust received an invalid Wasm binary name ('${{wasmBinary}}') from crate '${{pkgName}}', as the '.wasm' extension was missing.\n\nPlease ensure the 'ExtensionLibrary::override_wasm_binary' function for that GDExtension always returns a filename with the '.wasm' extension and try again.`);
97+
throw new Error(`Invalid Wasm module '${{wasmBinary}}' (missing '.wasm' extension). Check the console for more information.`);
98+
}} else if (wasmBinary in LDSO.loadedLibsByName) {{
99+
var libName = wasmBinary;
100+
}} else {{
101+
console.error(`godot-rust could not find the Wasm module '${{wasmBinary}}', needed to load the '${{pkgName}}' crate. This indicates its '.wasm' binary file was renamed to an unexpected name.\n\nPlease ensure its Wasm binary file is named '${{wasmBinary}}' in the game's web export files. This may require updating Wasm paths in the crate's corresponding '.gdextension' file, or just renaming the Wasm file to the expected name otherwise.`);
102+
throw new Error(`Wasm module '${{wasmBinary}}' not found. Check the console for more information.`);
103+
}}
80104
var dso = LDSO.loadedLibsByName[libName];
81105
// This property was renamed as of emscripten 3.1.34
82106
var dso_exports = "module" in dso ? dso["module"] : dso["exports"];
83107
var registrants = [];
84-
for (sym in dso_exports) {
85-
if (sym.startsWith("dynCall_")) {
86-
if (!(sym in Module)) {
87-
console.log(`Patching Module with ${sym}`);
108+
for (sym in dso_exports) {{
109+
if (sym.startsWith("dynCall_")) {{
110+
if (!(sym in Module)) {{
111+
console.log(`Patching Module with ${{sym}}`);
88112
Module[sym] = dso_exports[sym];
89-
}
90-
} else if (sym.startsWith("__godot_rust_registrant_")) {
113+
}}
114+
}} else if (sym.startsWith("__godot_rust_registrant_")) {{
91115
registrants.push(sym);
92-
}
93-
}
94-
for (sym of registrants) {
95-
console.log(`Running registrant ${sym}`);
116+
}}
117+
}}
118+
for (sym of registrants) {{
119+
console.log(`Running registrant ${{sym}}`);
96120
dso_exports[sym]();
97-
}
121+
}}
98122
console.log("Added", registrants.length, "plugins to registry!");
99-
"#)).expect("Unable to create CString from script");
123+
"#);
124+
125+
let script = std::ffi::CString::new(script).expect("Unable to create CString from script");
100126

101127
extern "C" { fn emscripten_run_script(script: *const std::ffi::c_char); }
102128
unsafe { emscripten_run_script(script.as_ptr()); }

0 commit comments

Comments
 (0)