Wasm registration fn names now based on crate name + index

[0x53A]

fixes #1202

[GodotRust]

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1205

[Bromeon]

Thanks a lot, great simplification! 👍

Is the libc dependency still necessary after this?

[PgBiel]

In principle this should be fine, though we may want to go back to the hash idea if we hit some name length limit. Let's wait and see...

[0x53A]

It works in my simple case

---

if we hit some name length limit

I don't think there is a limit for identifier names, and if there is, it should be large enough

[PgBiel]

Yeah, it's just that emscripten is now a bit picky about identifiers: #438 (comment)

But it's causing Rust to not work altogether, so this is probably not a concern for us, but for them. :p

[0x53A]

This is ready from my side

[PgBiel]

On my limited testing on hello-gdext-wasm, seems to work fine too.

I'll just note that there are some concerns noted here: https://users.rust-lang.org/t/simple-state-in-procedural-macro/68204/2

In particular, proc macro caching and other details could potentially lead to problems.
But, I suppose, the previous approach wasn't any better than the current one, if the seed was stored in the process memory anyway... So I'd vote to ship this as an incremental improvement and consider changing further in the future should problems arise.

That thread also suggested using UUIDs, but I'm fairly sure they are generated using the current time?
Though I suppose that's not a problem for reproducible builds since they can control the current time in the sandbox... So we could eventually consider something like that, but let's go one step at a time.

[Bromeon]

I'll just note that there are some concerns noted here: https://users.rust-lang.org/t/simple-state-in-procedural-macro/68204/2

Very good point 👍

@0x53A looks good for now. Could you apply my suggestion on the static comment (verbatim), so we're not oblivious to this issue, should it cause trouble at some point?

[0x53A]

if it comes to that we can pass through an identifier so it would become "__godot_rust_registrant__{crate_name}__v{crate_version}__{user_type_name}"

[Bromeon]

Minor comment, otherwise looks ready to merge!

[PgBiel]

if it comes to that we can pass through an identifier so it would become "__godot_rust_registrant__{crate_name}__v{crate_version}__{user_type_name}"

I hadn't thought of this, it's pretty smart. Though note that you'd also have to add the module path... I don't think we have access to that.

Though the user type name might have to be unique in Godot... But either way you might still have more than one function per type, or something that isn't bound to a type...

Let's think about this further later. But this could be a good start for something.

[Bromeon]

Thanks! Can you squash the commits to 1?

Generally, you can keep squashing right away, GitHub also shows diffs on force-pushes 🙂

[0x53A]

I’ll do that tonight, wasn’t on the computer

[0x53A]

rebased and force-pushed

[Bromeon]

Thank you! 🙂