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

[FIX] [WASM-Export] Unable to use multiple rust GDExtensions at the same time #973

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Guusggg
Copy link

@Guusggg Guusggg commented Dec 16, 2024

It runs...!

When I was running it past engine initialization, I encountered an error that registered the two seperate plugins as though they were the same. So there was a Node TestNodeFromClient and a Node TestNodeFromShared, but both element's ClassPlugin was referring to the name TestNodeFromClient. I thought this was a strange issue and investigated. After changing some code to let the runtime print me a list of registered class names, I noticed that the error was gone. So for some strange reason iterating through all the class names made them "correct", as the runtime previously complained that it's trying to register classes with the same name. I suspect this is due to the line

let string_name = entry
                .godot_str
                .get_or_init(|| entry.rust_str.to_string_name());

which might have corrected the previous wrongly set class name? I'm not really sure. (Edit: Could #834 have to do something with it?)

For now, it runs with a ton of errors about double registering methods; this is probably due to the runtime registering the methods twice. It does run however, and it seems to give the correct output.

Please do not actually merge this, as it isn't pretty. I'm looking for ways to improve this.

I suspect there needs to be a bigger refactor to make this production worthy code. It seems as though there's only one runtime in WASM, when on Linux I have no issues with the same code. This means that even though the runtime in WASM shares it's memory (as I could use a static to see if the iniitalization code already ran), however it runs auto_register_classes for every plugin. I'm not sure how to mitigate this really...

For some reason iterating through all the class names made them "correct", as the runtime previously complained that it's trying to register classes with the same name.
It runs with a ton of errors about double registering methods
Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this pull request!

I'm not sure if we should just add a flag "already loaded" to circumvent the current checks that ensure it's not loaded twice. This defeats their presence, and we could as well have only a global flag. With this approach we risk that accidental double initializations are no longer visible. This is e.g. what happened with Linux hot reload (fixed in #656), and the current logic would hide such bugs.

So I'm wondering if being a dependency is not something that should be explicitly declared by the user, and the library would verify that the assumptions really hold. In #968 (comment), I suggested a macro attribute:

#[gdextension(dependency)]
unsafe impl ExtensionLibrary for ClientExtension {}

Did you have a chance to experiment with that? (Totally fine if not 🙂 )


The behavior you found with ClassName is interesting. In the quoted code

let string_name = entry
    .godot_str
    .get_or_init(|| entry.rust_str.to_string_name());

the StringName is lazy-initialized. This is necessary because the builtin type StringName itself isn't yet loaded at the time the classes are collected by the "plugin" system. Which is also why ClassName::new_cached() takes Rust standard String.

You mention:

So there was a Node TestNodeFromClient and a Node TestNodeFromShared, but both element's ClassPlugin was referring to the name TestNodeFromClient

How and when (at which initialization stage) did you print this? Is this behavior also there if you use ClassName::to_cow_str(), which doesn't rely on Godot functionality?

We have two global variables here. Do you know if they only exist once in memory and hold all the classes?

// First element (index 0) is always the empty string name, which is used for "no class".
static CLASS_NAMES: Global<Vec<ClassNameEntry>> = Global::new(|| vec![ClassNameEntry::none()]);
static DYNAMIC_INDEX_BY_CLASS_TYPE: Global<HashMap<TypeId, u16>> = Global::default();

(Sorry for the interrogation, I'm trying to get to the root of where this duplication/overlapping happens 😀 )

@Guusggg
Copy link
Author

Guusggg commented Dec 19, 2024

Hi @Bromeon,

I'm repeating myself again and again, but thank you for the great response! Very in-depth and insightful.


Did you have a chance to experiment with that? (Totally fine if not 🙂 )

I did experiment with this (not with an attribute macro but just some random hacking with a boolean flag) however what I noticed is that the loading of plugins seemed not always consistent, so stopped trying in that direction. While experimenting, I noticed that sometimes the extensions in res://.godot/extensions.cfg would change order (as I placed them in and out of the project) and if it's not consistent, there's no telling which plugin is the actual "dependency", if you get what I mean.

Sometimes the order was: 1. Shared, 2. Client ("hacked" as dependency), and other times it was 1. Client (dependency), 2. Shared. If we could find a way to make sure the "dependency" is in fact always loaded after the "non-dependents", this would could be fruitful!


How and when (at which initialization stage) did you print this? Is this behavior also there if you use ClassName::to_cow_str(), which doesn't rely on Godot functionality?

I think I have a saved log somewhere.

The log
Multiple debug symbols for script were found. Using EmbeddedDWARF
Multiple debug symbols for script were found. Using EmbeddedDWARF
Multiple debug symbols for script were found. Using EmbeddedDWARF
Multiple debug symbols for script were found. Using EmbeddedDWARF
Multiple debug symbols for script were found. Using EmbeddedDWARF
Multiple debug symbols for script were found. Using EmbeddedDWARF
Multiple debug symbols for script were found. Using EmbeddedDWARF
Multiple debug symbols for script were found. Using EmbeddedDWARF
Multiple debug symbols for script were found. Using EmbeddedDWARF
Multiple debug symbols for script were found. Using EmbeddedDWARF
Multiple debug symbols for script were found. Using EmbeddedDWARF
Multiple debug symbols for script were found. Using EmbeddedDWARF
VM831:23 Running registrant rust_gdext_registrant___gensym_9bbaab5e60af4b658cb8036e10c0a039
Multiple debug symbols for script were found. Using EmbeddedDWARF
VM831:23 Running registrant rust_gdext_registrant___gensym_61f73841f1e147709ab6bc7b6ce41675
VM831:26 Added 2 plugins to registry!
Multiple debug symbols for script were found. Using EmbeddedDWARF
tmp_js_export.js:477 Is engine initialized? nope

tmp_js_export.js:477 Initialize gdext...

tmp_js_export.js:477 Godot version against which gdext was compiled: v4.3.stable.official

tmp_js_export.js:477 Godot version of GDExtension API at runtime: GDExtensionGodotVersion { major: 4, minor: 3, patch: 0, string: 0x5b8014 }

tmp_js_export.js:477 Loaded interface.

tmp_js_export.js:477 Loaded global method table.

tmp_js_export.js:477 Loaded utility function table.

tmp_js_export.js:477 Loaded builtin method table.

tmp_js_export.js:477 Assigned binding.

tmp_js_export.js:462 Initialize godot-rust (API v4.3.stable.official, runtime v4.3.stable.official)

VM832:23 Running registrant rust_gdext_registrant___gensym_aabda5236b724596878729c480ee3c06
VM832:23 Running registrant rust_gdext_registrant___gensym_fbbe74decd14451ca1cc7116b4a7fe49
VM832:26 Added 2 plugins to registry!
tmp_js_export.js:477 Is engine initialized? yes

tmp_js_export.js:477 Load class method table for level 'Core'...

tmp_js_export.js:477 Core level: loaded 0 classes and 0 methods in 0.000064942s.

tmp_js_export.js:477 Auto-register classes at level `Core`...

tmp_js_export.js:477 All classes for level `Core` auto-registered.

tmp_js_export.js:477 Load class method table for level 'Core'...

tmp_js_export.js:477 Core level: loaded 0 classes and 0 methods in 0.001895019s.

tmp_js_export.js:477 Auto-register classes at level `Core`...

tmp_js_export.js:477 All classes for level `Core` auto-registered.

tmp_js_export.js:462 Godot Engine v4.3.stable.official.77dcf97d8 - https://godotengine.org
tmp_js_export.js:477 Load class method table for level 'Servers'...

tmp_js_export.js:477 Servers level: loaded 19 classes and 1039 methods in 0.064179931s.

tmp_js_export.js:477 Auto-register classes at level `Servers`...

tmp_js_export.js:477 All classes for level `Servers` auto-registered.

tmp_js_export.js:477 Load class method table for level 'Servers'...

tmp_js_export.js:477 Table for classes (Server level) is already initialized!

tmp_js_export.js:477 Servers level: loaded 19 classes and 1039 methods in 0.075514893s.

tmp_js_export.js:477 Auto-register classes at level `Servers`...

tmp_js_export.js:477 All classes for level `Servers` auto-registered.

tmp_js_export.js:462 OpenGL API OpenGL ES 3.0 (WebGL 2.0 (OpenGL ES 3.0 Chromium)) - Compatibility - Using Device: WebKit - WebKit WebGL
tmp_js_export.js:9 The AudioContext was not allowed to start. It must be resumed (or created) after a user gesture on the page. https://goo.gl.qjz9zk/7K7WLu

tmp_js_export.js:477 Scene level: loaded 619 classes and 10489 methods in 0.360505127s.

tmp_js_export.js:477 Check Godot precision setting...

tmp_js_export.js:477 Is double precision: Godot=single, gdext=single

tmp_js_export.js:477 Auto-register classes at level `Scene`...

tmp_js_export.js:477 Aborted(undefined)
onPrintError @ tmp_js_export.js:477
abort @ tmp_js_export.js:9
___cxa_throw @ tmp_js_export.js:9
$panic_unwind::imp::panic::h7a89a72211144e04 @ client.wasm-12421a4e:0x400f36
$__rust_start_panic @ client.wasm-12421a4e:0x400d99
$rust_panic @ client.wasm-12421a4e:0x3f2d15
$std::panicking::rust_panic_with_hook::hc3a8cd21f77927d3 @ client.wasm-12421a4e:0x3f2c0b
$std::panicking::begin_panic_handler::_$u7b$$u7b$closure$u7d$$u7d$::hb19be6fa9857ee61 @ client.wasm-12421a4e:0x3f00fe
$std::sys::backtrace::__rust_end_short_backtrace::h585b6ab617e4600a @ client.wasm-12421a4e:0x3efeef
$rust_begin_unwind @ client.wasm-12421a4e:0x3f1cd3
$core::panicking::panic_fmt::he773db1f33281f18 @ client.wasm-12421a4e:0x46fc3a

$godot_core::registry::class::ClassRegistrationInfo::validate_unique::hca8fdee7c3ddd669 @ client.wasm-12421a4e:0x149617

$godot_core::registry::class::fill_class_info::h9096c2b57e481a10 @ client.wasm-12421a4e:0x14d1ca
$godot_core::registry::class::auto_register_classes::_$u7b$$u7b$closure$u7d$$u7d$::h4e8f71d29f9e4b38 @ client.wasm-12421a4e:0xc2bb0
$godot_core::private::iterate_plugins::h4891a60bfbcbfcb5 @ client.wasm-12421a4e:0xb9183
$godot_core::registry::class::auto_register_classes::hb466f03c53a7f5f9 @ client.wasm-12421a4e:0x14986a
$godot_core::init::gdext_on_level_init::he07d03a4f9f1c7d6 @ client.wasm-12421a4e:0xc1e33
$godot_core::init::ffi_initialize_layer::try_load::h3f76e631d13323ff @ client.wasm-12421a4e:0x6aba6
$godot_core::init::ffi_initialize_layer::_$u7b$$u7b$closure$u7d$$u7d$::h9633f77db84f27d5 @ client.wasm-12421a4e:0x6aa20
$std::panicking::try::do_call::h00c16f90d40b73af @ client.wasm-12421a4e:0x6fc87
$__rust_try @ client.wasm-12421a4e:0x6fa7d
$std::panicking::try::h254b8137f22d1573 @ client.wasm-12421a4e:0x6fb25
$std::panic::catch_unwind::h1dc11fa29f7714fc @ client.wasm-12421a4e:0x7738d
$godot_core::private::handle_panic_with_print::h4dca4c0b3c60c941 @ client.wasm-12421a4e:0x50b13
$godot_core::private::handle_panic::h014b24c220f81148 @ client.wasm-12421a4e:0x5086a
$godot_core::init::ffi_initialize_layer::h3456bbac7755dccf @ client.wasm-12421a4e:0x6a8c4
$func58013 @ 0aaeb74a:0x2599d56
$func1213 @ 0aaeb74a:0x431d7a
$func1264 @ 0aaeb74a:0x489fb7
$_Z14godot_web_mainiPPc @ 0aaeb74a:0x2e41ef
__Z14godot_web_mainiPPc @ tmp_js_export.js:9
$__main_argc_argv @ 006438ee:0xa5975
callMain @ tmp_js_export.js:9
(anonymous) @ tmp_js_export.js:804
(anonymous) @ tmp_js_export.js:799
Promise.then
start @ tmp_js_export.js:778
(anonymous) @ tmp_js_export.js:837
Promise.then
startGame @ tmp_js_export.js:836
(anonymous) @ tmp_js_export.html:181
(anonymous) @ tmp_js_export.html:195
tmp_js_export.html:139 RuntimeError: Aborted(undefined). Build with -sASSERTIONS for more info.
    at abort (tmp_js_export.js:9:8604)
    at ___cxa_throw (tmp_js_export.js:9:425328)
    at client.wasm.panic_unwind::imp::panic::h7a89a72211144e04 (client.wasm-12421a4e:0x400f36)
    at client.wasm.__rust_start_panic (client.wasm-12421a4e:0x400d99)
    at client.wasm.rust_panic (client.wasm-12421a4e:0x3f2d15)
    at client.wasm.std::panicking::rust_panic_with_hook::hc3a8cd21f77927d3 (client.wasm-12421a4e:0x3f2c0b)
    at client.wasm.std::panicking::begin_panic_handler::_$u7b$$u7b$closure$u7d$$u7d$::hb19be6fa9857ee61 (client.wasm-12421a4e:0x3f00fe)
    at client.wasm.std::sys::backtrace::__rust_end_short_backtrace::h585b6ab617e4600a (client.wasm-12421a4e:0x3efeef)
    at client.wasm.rust_begin_unwind (client.wasm-12421a4e:0x3f1cd3)
    at client.wasm.core::panicking::panic_fmt::he773db1f33281f18 (client.wasm-12421a4e:0x46fc3a)
displayFailureNotice @ tmp_js_export.html:139
Promise.then
(anonymous) @ tmp_js_export.html:191
(anonymous) @ tmp_js_export.html:195

In the above, the line $godot_core::registry::class::ClassRegistrationInfo::validate_unique::hca8fdee7c3ddd669 @ client.wasm-12421a4e:0x149617 is what causes the panic, which made sense to me considering the source. Whenever I added the code above (quoting for completeness)

let string_name = entry
    .godot_str
    .get_or_init(|| entry.rust_str.to_string_name());

The error did not happen, so validation happened correctly. Please keep in mind I know a bit more about the library, but still nothing at all.

I just happened to want to print the string names on start up, to verify that indeed StringName with index 1 was always "TestNodeFromClient" (or "TestNodeFromShared" depending on which initialized first). I called the above code in the startup at gdext_on_level_init


I'm not sure how to continue at the moment, however I'm hellbent on getting it to work now. I've crawled so deep into this library at the moment, it'd be a waste not to fix it properly.

Do you have any recommendations for the consistency of loading the plugins? I'm convinced that the #[gdextensions(dependency)] could be a great fix for right now, as it might be a stepping stone for #951.

@Guusggg Guusggg changed the title [FIX] [WASM-Export] Unable to use multiple rust GDExtensions at the same time [DRAFT] [FIX] [WASM-Export] Unable to use multiple rust GDExtensions at the same time Dec 19, 2024
@Guusggg Guusggg marked this pull request as draft December 22, 2024 14:34
@Guusggg Guusggg changed the title [DRAFT] [FIX] [WASM-Export] Unable to use multiple rust GDExtensions at the same time [FIX] [WASM-Export] Unable to use multiple rust GDExtensions at the same time Dec 22, 2024
@Bromeon
Copy link
Member

Bromeon commented Dec 22, 2024

While experimenting, I noticed that sometimes the extensions in res://.godot/extensions.cfg would change order (as I placed them in and out of the project) and if it's not consistent, there's no telling which plugin is the actual "dependency", if you get what I mean.

Sometimes the order was: 1. Shared, 2. Client ("hacked" as dependency), and other times it was 1. Client (dependency), 2. Shared. If we could find a way to make sure the "dependency" is in fact always loaded after the "non-dependents", this would could be fruitful!

I'm not sure how exactly Godot determines the order of GDExtensions, or if there is a deterministic way to do so. This .cfg file is in the .godot folder and as such considered "private"...

But in your case, that means you need to register the 3 parts as individual extensions? If the client->shared dependency would be "hidden" from Godot (i.e. it just sees client including the shared classes), that wouldn't work?

Thanks for the debugging insights! I'll need to analyze that a bit more in-depth once I get some time.

@Guusggg
Copy link
Author

Guusggg commented Dec 23, 2024

Yeah, I agree the .cfg is private so I don't think we need to rely on it.

My case is that I'd like to have the client project along with the shared project, but I'd also like a third, namely the server project. I don't want to distribute the server along with the client and shared project, because I'd like to keep the multiplayer part private, which is a big reason I chose Rust instead of GDScript. Rust has excellent low-level support, and it's not easy for script kiddo's to disassemble WebAssembly to get into the nitty gritty parts of the network code. I realize this is security-by-obscurity, and it's not the only measure I take.

Just throwing some ideas around, but maybe I should go a step back and research why the three extensions work fine on my computer, a Linux box, and not on the web. Because Godot is already succesfully loading all projects (I can use them fine in the editor, and running the game is also working fine.)

I have a bit of experience using emscripten, would you say it's worthwhile to attempt to change the WASM part of gdext, so that it behaves more like the Linux part? Maybe some WASM specific flags to only run the gdext initialization once, and decouple the extensions a little more?

I know I'm asking a lot of questions, please excuse me for that. I'm willing to take it on, but I'd need your support so that it's actually an idea you'd like to see merged :)

@Bromeon
Copy link
Member

Bromeon commented Dec 23, 2024

I have a bit of experience using emscripten, would you say it's worthwhile to attempt to change the WASM part of gdext, so that it behaves more like the Linux part? Maybe some WASM specific flags to only run the gdext initialization once, and decouple the extensions a little more?

Yeah, I think making the behavior more consistent and intuitive would be great! Myself, I'm not a Wasm expert, and it's one of the areas where I can't devote as much time as I'd like to (several contributors like @PgBiel have done a tremendous job here).

That said, one thing that's somewhat important to me is that we try to understand why something happens. It's often easy to patch something to get the expected behavior right now, but without understanding the underlying problem, another issue will likely come up again in the future. And if the original author of that patch is no longer actively contributing, such code is now even harder to understand. Of course, there are situations where other tools (emscripten, Godot, browsers, ...) have bugs that just need to be worked around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants