C#: Move marshaling logic and generated glue to C# #48409
Conversation
|
This is the first step towards godotengine/godot-proposals#2333 The changes are being done in a way that don't require migrating to .NET 5/6, as that may not make it in time for Godot 4.0. This way we make make progress towards the .NET 5/6 goal without switching to it yet. |
You can maybe have a look at how @Faless handled packaging the JavaScript templates in |
This can't be done as part as the same |
neikeq
force-pushed
the
oh-im-die-ty-4ever
branch
8 times, most recently
from
f11d0e8 to
296693b
Compare
|
@neikeq it is unclear what is your time line for this as an interim transition before aligning with .NET5/6, [.NET6 scheduled for Nov 2021] How shall users contribute to the testing?
From .NET6 point of view, Mono embedding API will only be relevant for mobile, consoles and WebAssembly, not desktop for e.g. linux and windows. I assume that the second step is to replace the Mono embedding API for NON (mobile, console and WebAssembly) with that of CoreCLR [for desktop] or even better CoreRT?
|
There's no timeline. Nothing is set on stone right now and we don't know enough about .NET 6 yet. If in one month or two I conclude we won't have .NET 6 support in time for Godot 4.0, then I will put that aside for the time being while I focus on the stable release.
You can try it with different projects to see if anything breaks. Other than that, there's no unit tests set up right now. That's something I would like to change for Godot 4.1 if things go well.
I'm not sure what you mean by the naming being stabilized. I'll continue to move more code to C# this month.
This has nothing to do with platforms. The idea is to limit the use of embedding APIs to only startup tasks in order to make it easy to later support to CoreCLR which has a much more limited embedding API. Let's leave discussion about roadmaps and global vision out of this PR unless it has significant impact on its review. |
|
With this PR, when I try to build the assemblies, it fails, and this is the output (I'm on Ubuntu 20.04 64-bit): |
|
@aaronfranke Did you generate the glue before attempting to build the assemblies? You still need to run |
|
@neikeq Yes, that was it. I read the post wrong and thought this script replaced the glue generation code. So to be clear, the process used to be:
And now the process is:
I suppose this is an improvement, since the Python script runs faster than rebuilding Godot, and now the first build doesn't need to run with different settings on repeated builds, making recompiles faster. Eventually, it would be nice to get to a point where this is a one-step process inside of SCons, and it seems we're not far off from that. Is there any reason we can't just execute these commands from within SCons after the binary has been built? |
| default: | ||
| { | ||
| if (type == typeof(Vector2)) | ||
| return Variant.Type.Vector2; |
There was a problem hiding this comment.
VS Code's C# formatter indents these blocks further (starting with the {). I don't really have an opinion on this, but I'm mentioning it in case we wanted to change this to avoid fighting with tools.
There was a problem hiding this comment.
No, no. We don't want to change it. That indentation style is such a waste of horizontal space. I remember MonoDevelop doing the same thing; very annoying. Perhaps we can solve this with .editorconfig?
There was a problem hiding this comment.
This is the .editorconfig rule you are looking for:
csharp_indent_case_contents_when_block = false
It's not just that. Previously the third-step would do both re-build the engine (only mono module; most time was spent linking) and build the C# assemblies. Now we only do the latter.
We would need to make Godot runnable without a window on all desktop platforms and guarantee it doesn't exit with error code for a reason other than glue generation failing. |
Since 3.3, |
|
In the end I decided to merge this into a branch were .NET 6 support will developed in parallel with the master branch. It's better not to divert too much in the module code from 3.x to master in order to make working with both easier in case .NET 6 ends up happening in 4.1 instead of 4.0. |
neikeq
force-pushed
the
oh-im-die-ty-4ever
branch
from
296693b to
719d759
Compare
|
I made a new project with this PR, it fails to load a minimal script: 48409.zip
|
|
@neikeq Seems like we forgot to merge this, do you want to rebase and merge now, or should further work be done first to fix C# bindings after recent core changes? |
|
I'll fix the bindings first and then rebase this. |
neikeq
force-pushed
the
oh-im-die-ty-4ever
branch
from
719d759 to
6499fe2
Compare
We will be progressively moving most code to C#.
The plan is to only use Mono's embedding APIs to set things at launch.
This will make it much easier to later support CoreCLR too which
doesn't have rich embedding APIs.
Additionally the code in C# is more maintainable and makes it easier
to implement new features, e.g.: runtime codegen which we could use to
avoid using reflection for marshaling everytime a field, property or
method is accessed.
SOME NOTES ON INTEROP
We make the same assumptions as GDNative about the size of the Godot
structures we use. We take it a bit further by also assuming the layout
of fields in some cases, which is riskier but let's us squeeze out some
performance by avoiding unnecessary managed to native calls.
Code that deals with native structs is less safe than before as there's
no RAII and copy constructors in C#. It's like using the GDNative C API
directly. One has to take special care to free values they own.
Perhaps we could use roslyn analyzers to check this, but I don't know
any that uses attributes to determine what's owned or borrowed.
As to why we maily use pointers for native structs instead of ref/out:
- AFAIK (and confirmed with a benchmark) ref/out are pinned
during P/Invoke calls and that has a cost.
- Native struct fields can't be ref/out in the first place.
- A `using` local can't be passed as ref/out, only `in`. Calling a
method or property on an `in` value makes a silent copy, so we want
to avoid `in`.
REGARDING THE BUILD SYSTEM
There's no longer a `mono_glue=yes/no` SCons options. We no longer
need to build with `mono_glue=no`, generate the glue and then build
again with `mono_glue=yes`. We build only once and generate the glue
(which is in C# now).
However, SCons no longer builds the C# projects for us. Instead one
must run `build_assemblies.py`, e.g.:
```sh
%godot_src_root%/modules/mono/build_scripts/build_assemblies.py \
--godot-output-dir=%godot_src_root%/bin \
--godot-target=release_debug`
```
We could turn this into a custom build target, but I don't know how
to do that with SCons (it's possible with Meson).
OTHER NOTES
Most of the moved code doesn't follow the C# naming convention and
still has the word Mono in the names despite no longer dealing with
Mono's embedding APIs. This is just temporary while transitioning,
to make it easier to understand what was moved where.
neikeq
force-pushed
the
oh-im-die-ty-4ever
branch
from
6499fe2 to
4830717
Compare
|
Rebased, this should be good to merge now (into the |
|
Thanks! |
We will be progressively moving most code to C#.
The plan is to only use Mono's embedding APIs to set things at launch.
This will make it much easier to later support CoreCLR too which doesn't have rich embedding APIs.
Additionally the code in C# is more maintainable and makes it easier to implement new features, e.g.: runtime codegen which we could use to avoid using reflection for marshaling everytime a field, property or method is accessed.
Some notes on interop
We make the same assumptions as GDNative about the size of the Godot structures we use. We take it a bit further by also assuming the layout of fields in some cases, which is riskier but let's us squeeze out some performance by avoiding unnecessary managed to native calls.
Code that deals with native structs is less safe than before as there's no RAII and copy constructors in C#. It's like using the GDNative C API directly. One has to take special care to free values they own. Perhaps we could use roslyn analyzers to check this, but I don't know any that uses attributes to determine what's owned or borrowed.
As to why we maily use pointers for native structs instead of ref/out:
usinglocal can't be passed as ref/out, onlyin. Calling a method or property on aninvalue makes a silent copy, so we want to avoidin.Regarding the build system
There's no longer a
mono_glue=yes/noSCons options. We no longer need to build withmono_glue=no, generate the glue and then build again withmono_glue=yes. We build only once and generate the glue (which is in C# now).However, SCons no longer builds the C# projects for us. Instead one must run
build_assemblies.py, e.g.:%godot_src_root%/modules/mono/build_scripts/build_assemblies.py \ --godot-output-dir=%godot_src_root%/bin \ --godot-target=release_debug`We could turn this into a custom build target, but I don't know how to do that with SCons (it's possible with Meson).
Other notes
Most of the moved code doesn't follow the C# naming convention and still has the word Mono in the names despite no longer dealing with Mono's embedding APIs. This is just temporary while transitioning, to make it easier to understand what was moved where.