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

[3.2] Mono: Make Godot provide its own WASM m2n trampolines #44373

Merged
merged 2 commits into from
Dec 17, 2020

Conversation

neikeq
Copy link
Contributor

@neikeq neikeq commented Dec 14, 2020

Back-port of #44105 to the 3.2 branch.

This depends on a custom Mono patch from this commit: godotengine/godot-mono-builds@0e31293

Also fixes #36858

`-rdynamic` was causing the emsdk linker to silently fail to
generate the output `.wasm` file (even though exit code was 0).
@neikeq neikeq added this to the 3.2 milestone Dec 14, 2020
@neikeq
Copy link
Contributor Author

neikeq commented Dec 14, 2020

This PR against the 3.2 branch is tested and ready to be merged. Only pending review.

@@ -95,7 +95,7 @@
#define C_METHOD_MONOARRAY_TO(m_type) C_NS_MONOMARSHAL "::mono_array_to_" #m_type
#define C_METHOD_MONOARRAY_FROM(m_type) C_NS_MONOMARSHAL "::" #m_type "_to_mono_array"

#define BINDINGS_GENERATOR_VERSION UINT32_C(11)
#define BINDINGS_GENERATOR_VERSION UINT32_C(13)
Copy link
Member

Choose a reason for hiding this comment

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

Is the bump from 11 to 13 expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was back-ported from master so probably version 12 was some other change that wasn't back-ported. It doesn't matter too much as this is used together with the API hash.

@@ -151,6 +154,21 @@ struct ScopeThreadAttach {
MonoThread *mono_thread;
};

template <typename... P>
Copy link
Member

Choose a reason for hiding this comment

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

Is this C++11/14 compatible in the end or does this form of parameter pack require C++17? https://en.cppreference.com/w/cpp/language/parameter_pack

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's C++11. I only had to look for an alternative for the if constexprs when backporting this.

@akien-mga akien-mga merged commit 80c9f2d into godotengine:3.2 Dec 17, 2020
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants