Skip to content

Commit

Permalink
C#: Restructure code prior move to .NET Core
Browse files Browse the repository at this point in the history
The main focus here was to remove the majority of code that relied on
Mono's embedding APIs, specially the reflection APIs. The embedding
APIs we still use are the bare minimum we need for things to work.
A lot of code was moved to C#. We no longer deal with any managed
objects (`MonoObject*`, and such) in native code, and all marshaling
is done in C#.

The reason for restructuring the code and move away from embedding APIs
is that once we move to .NET Core, we will be limited by the much more
minimal .NET hosting.

PERFORMANCE REGRESSIONS
-----------------------

Some parts of the code were written with little to no concern about
performance. This includes code that calls into script methods and
accesses script fields, properties and events.
The reason for this is that all of that will be moved to source
generators, so any work prior to that would be a waste of time.

DISABLED FEATURES
-----------------

Some code was removed as it no longer makes sense (or won't make sense
in the future).
Other parts were commented out with `#if 0`s and TODO warnings because
it doesn't make much sense to work on them yet as those parts will
change heavily when we switch to .NET Core but also when we start
introducing source generators.
As such, the following features were disabled temporarily:
- Assembly-reloading (will be done with ALCs in .NET Core).
- Properties/fields exports and script method listing (will be
  handled by source generators in the future).
- Exception logging in the editor and stack info for errors.
- Exporting games.
- Building of C# projects. We no longer copy the Godot API assemblies
  to the project directory, so MSBuild won't be able to find them. The
  idea is to turn them into NuGet packages in the future, which could
  also be obtained from local NuGet sources during development.
  • Loading branch information
neikeq committed Aug 22, 2022
1 parent 5e37d07 commit 513ee85
Show file tree
Hide file tree
Showing 79 changed files with 2,557 additions and 5,218 deletions.
1,311 changes: 509 additions & 802 deletions modules/mono/csharp_script.cpp

Large diffs are not rendered by default.

85 changes: 17 additions & 68 deletions modules/mono/csharp_script.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@

#include "mono_gc_handle.h"
#include "mono_gd/gd_mono.h"
#include "mono_gd/gd_mono_header.h"
#include "mono_gd/gd_mono_internals.h"

#ifdef TOOLS_ENABLED
Expand Down Expand Up @@ -67,18 +66,6 @@ TScriptInstance *cast_script_instance(ScriptInstance *p_inst) {

#define CAST_CSHARP_INSTANCE(m_inst) (cast_script_instance<CSharpInstance, CSharpLanguage>(m_inst))

struct DotNetScriptLookupInfo {
String class_namespace;
String class_name;
GDMonoClass *script_class = nullptr;

DotNetScriptLookupInfo() {} // Required by HashMap...

DotNetScriptLookupInfo(const String &p_class_namespace, const String &p_class_name, GDMonoClass *p_script_class) :
class_namespace(p_class_namespace), class_name(p_class_name), script_class(p_script_class) {
}
};

class CSharpScript : public Script {
GDCLASS(CSharpScript, Script);

Expand All @@ -89,25 +76,14 @@ class CSharpScript : public Script {
bool nil_is_variant = false;
};

struct EventSignal {
GDMonoField *field = nullptr;
GDMonoMethod *invoke_method = nullptr;
Vector<SignalParameter> parameters;
};

private:
friend class CSharpInstance;
friend class CSharpLanguage;
friend struct CSharpScriptDepSort;

bool tool = false;
bool valid = false;
bool reload_invalidated = false;

GDMonoClass *base = nullptr;
GDMonoClass *native = nullptr;
GDMonoClass *script_class = nullptr;

Ref<CSharpScript> base_cache; // TODO what's this for?

HashSet<Object *> instances;
Expand All @@ -128,14 +104,9 @@ class CSharpScript : public Script {
#endif

String source;
StringName name;

SelfList<CSharpScript> script_list = this;

HashMap<StringName, Vector<SignalParameter>> _signals;
HashMap<StringName, EventSignal> event_signals;
bool signals_invalidated = true;

Dictionary rpc_config;

#ifdef TOOLS_ENABLED
Expand All @@ -158,34 +129,26 @@ class CSharpScript : public Script {

void _clear();

void _update_name();

void load_script_signals(GDMonoClass *p_class, GDMonoClass *p_native_class);
bool _get_signal(GDMonoClass *p_class, GDMonoMethod *p_delegate_invoke, Vector<SignalParameter> &params);

bool _update_exports(PlaceHolderScriptInstance *p_instance_to_update = nullptr);

#warning TODO
#if 0
bool _get_member_export(IMonoClassMember *p_member, bool p_inspect_export, PropertyInfo &r_prop_info, bool &r_exported);
#ifdef TOOLS_ENABLED
static int _try_get_member_export_hint(IMonoClassMember *p_member, ManagedType p_type, Variant::Type p_variant_type, bool p_allow_generics, PropertyHint &r_hint, String &r_hint_string);
#endif
#endif

CSharpInstance *_create_instance(const Variant **p_args, int p_argcount, Object *p_owner, bool p_is_ref_counted, Callable::CallError &r_error);
Variant _new(const Variant **p_args, int p_argcount, Callable::CallError &r_error);

// Do not use unless you know what you are doing
friend void GDMonoInternals::tie_managed_to_unmanaged(MonoObject *, Object *);
static Ref<CSharpScript> create_for_managed_type(GDMonoClass *p_class, GDMonoClass *p_native);
static void update_script_class_info(Ref<CSharpScript> p_script);
static void initialize_for_managed_type(Ref<CSharpScript> p_script, GDMonoClass *p_class, GDMonoClass *p_native);

Variant _member_get_rpc_config(IMonoClassMember *p_member) const;
static void initialize_for_managed_type(Ref<CSharpScript> p_script);

protected:
static void _bind_methods();

Variant callp(const StringName &p_method, const Variant **p_args, int p_argcount, Callable::CallError &r_error) override;
void _resource_path_changed() override;
bool _get(const StringName &p_name, Variant &r_ret) const;
bool _set(const StringName &p_name, const Variant &p_value);
void _get_property_list(List<PropertyInfo> *p_properties) const;
Expand Down Expand Up @@ -270,22 +233,21 @@ class CSharpInstance : public ScriptInstance {
bool _unreference_owner_unsafe();

/*
* If nullptr is returned, the caller must destroy the script instance by removing it from its owner.
* If false is returned, the caller must destroy the script instance by removing it from its owner.
*/
MonoObject *_internal_new_managed();
bool _internal_new_managed();

// Do not use unless you know what you are doing
friend void GDMonoInternals::tie_managed_to_unmanaged(MonoObject *, Object *);
static CSharpInstance *create_for_managed_type(Object *p_owner, CSharpScript *p_script, const MonoGCHandleData &p_gchandle);

void get_properties_state_for_reloading(List<Pair<StringName, Variant>> &r_state);
void get_event_signals_state_for_reloading(List<Pair<StringName, Array>> &r_state);

public:
MonoObject *get_mono_object() const;

_FORCE_INLINE_ bool is_destructing_script_instance() { return destructing_script_instance; }

_FORCE_INLINE_ GCHandleIntPtr get_gchandle_intptr() { return gchandle.get_intptr(); }

Object *get_owner() override;

bool set(const StringName &p_name, const Variant &p_value) override;
Expand All @@ -300,15 +262,15 @@ class CSharpInstance : public ScriptInstance {
bool has_method(const StringName &p_method) const override;
Variant callp(const StringName &p_method, const Variant **p_args, int p_argcount, Callable::CallError &r_error) override;

void mono_object_disposed(MonoObject *p_obj);
void mono_object_disposed();

/*
* If 'r_delete_owner' is set to true, the caller must memdelete the script instance's owner. Otherwise, if
* 'r_remove_script_instance' is set to true, the caller must destroy the script instance by removing it from its owner.
*/
void mono_object_disposed_baseref(MonoObject *p_obj, bool p_is_finalizer, bool &r_delete_owner, bool &r_remove_script_instance);
void mono_object_disposed_baseref(bool p_is_finalizer, bool &r_delete_owner, bool &r_remove_script_instance);

void connect_event_signals();
void connect_event_signal(const StringName &p_event_signal);
void disconnect_event_signals();

void refcount_incremented() override;
Expand All @@ -332,7 +294,6 @@ class CSharpInstance : public ScriptInstance {
struct CSharpScriptBinding {
bool inited = false;
StringName type_name;
GDMonoClass *wrapper_class = nullptr;
MonoGCHandleData gchandle;
Object *owner = nullptr;

Expand Down Expand Up @@ -370,28 +331,17 @@ class CSharpLanguage : public ScriptLanguage {
ManagedCallableMiddleman *managed_callable_middleman = memnew(ManagedCallableMiddleman);

struct StringNameCache {
StringName _signal_callback;
StringName _set;
StringName _get;
StringName _get_property_list;
StringName _property_can_revert;
StringName _property_get_revert;
StringName _notification;
StringName _script_source;
StringName dotctor; // .ctor
StringName on_before_serialize; // OnBeforeSerialize
StringName on_after_deserialize; // OnAfterDeserialize
StringName delegate_invoke_method_name;

StringNameCache();
};

int lang_idx = -1;

HashMap<String, DotNetScriptLookupInfo> dotnet_script_lookup_map;

void lookup_script_for_class(GDMonoClass *p_class);

// For debug_break and debug_break_parse
int _debug_parse_err_line = -1;
String _debug_parse_err_file;
Expand Down Expand Up @@ -421,6 +371,7 @@ class CSharpLanguage : public ScriptLanguage {
StringNameCache string_names;

const Mutex &get_language_bind_mutex() { return language_bind_mutex; }
const Mutex &get_script_instances_mutex() { return script_instances_mutex; }

_FORCE_INLINE_ int get_language_index() { return lang_idx; }
void set_language_index(int p_idx);
Expand All @@ -434,7 +385,7 @@ class CSharpLanguage : public ScriptLanguage {
#endif

static void release_script_gchandle(MonoGCHandleData &p_gchandle);
static void release_script_gchandle(MonoObject *p_expected_obj, MonoGCHandleData &p_gchandle);
static void release_script_gchandle(void *p_expected_mono_obj_unused, MonoGCHandleData &p_gchandle);

bool debug_break(const String &p_error, bool p_allow_continue = true);
bool debug_break_parse(const String &p_file, int p_line, const String &p_error);
Expand All @@ -446,12 +397,6 @@ class CSharpLanguage : public ScriptLanguage {

_FORCE_INLINE_ ManagedCallableMiddleman *get_managed_callable_middleman() const { return managed_callable_middleman; }

void lookup_scripts_in_assembly(GDMonoAssembly *p_assembly);

const DotNetScriptLookupInfo *lookup_dotnet_script(const String &p_script_path) const {
return dotnet_script_lookup_map.getptr(p_script_path);
}

String get_name() const override;

/* LANGUAGE FUNCTIONS */
Expand Down Expand Up @@ -527,6 +472,10 @@ class CSharpLanguage : public ScriptLanguage {
RBMap<Object *, CSharpScriptBinding>::Element *insert_script_binding(Object *p_object, const CSharpScriptBinding &p_script_binding);
bool setup_csharp_script_binding(CSharpScriptBinding &r_script_binding, Object *p_object);

static void tie_native_managed_to_unmanaged(GCHandleIntPtr p_gchandle_intptr, Object *p_unmanaged, const StringName *p_native_name, bool p_ref_counted);
static void tie_user_managed_to_unmanaged(GCHandleIntPtr p_gchandle_intptr, Object *p_unmanaged, CSharpScript *p_script, bool p_ref_counted);
static void tie_managed_to_unmanaged_with_pre_setup(GCHandleIntPtr p_gchandle_intptr, Object *p_unmanaged);

#ifdef DEBUG_ENABLED
Vector<StackInfo> stack_trace_get_info(MonoObject *p_stack_trace);
#endif
Expand Down
11 changes: 0 additions & 11 deletions modules/mono/editor/GodotTools/GodotTools/Build/BuildManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,17 +178,6 @@ private static bool BuildProjectBlocking(BuildInfo buildInfo)
if (!File.Exists(buildInfo.Solution))
return true; // No solution to build

// Make sure the API assemblies are up to date before building the project.
// We may not have had the chance to update the release API assemblies, and the debug ones
// may have been deleted by the user at some point after they were loaded by the Godot editor.
string apiAssembliesUpdateError = Internal.UpdateApiAssembliesFromPrebuilt(buildInfo.Configuration == "ExportRelease" ? "Release" : "Debug");

if (!string.IsNullOrEmpty(apiAssembliesUpdateError))
{
ShowBuildErrorDialog("Failed to update the Godot API assemblies");
return false;
}

using (var pr = new EditorProgress("mono_project_debug_build", "Building project solution...", 1))
{
pr.Step("Building project solution", 0);
Expand Down
63 changes: 44 additions & 19 deletions modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Godot;
using Godot.NativeInterop;
using System;
using System.Collections.Generic;
using System.Diagnostics;
Expand Down Expand Up @@ -100,7 +101,9 @@ public override void _ExportFile(string path, string type, string[] features)
return;

if (Path.GetExtension(path) != Internal.CSharpLanguageExtension)
throw new ArgumentException($"Resource of type {Internal.CSharpLanguageType} has an invalid file extension: {path}", nameof(path));
throw new ArgumentException(
$"Resource of type {Internal.CSharpLanguageType} has an invalid file extension: {path}",
nameof(path));

// TODO What if the source file is not part of the game's C# project

Expand Down Expand Up @@ -194,7 +197,8 @@ private void _ExportBeginImpl(string[] features, bool isDebug, string path, int
{
string thisWasmFrameworkAssemblyPath = Path.Combine(bclDir, thisWasmFrameworkAssemblyName + ".dll");
if (!File.Exists(thisWasmFrameworkAssemblyPath))
throw new FileNotFoundException($"Assembly not found: '{thisWasmFrameworkAssemblyName}'", thisWasmFrameworkAssemblyPath);
throw new FileNotFoundException($"Assembly not found: '{thisWasmFrameworkAssemblyName}'",
thisWasmFrameworkAssemblyPath);
assemblies[thisWasmFrameworkAssemblyName] = thisWasmFrameworkAssemblyPath;
}

Expand All @@ -206,18 +210,21 @@ private void _ExportBeginImpl(string[] features, bool isDebug, string path, int

foreach (var thisWasmFrameworkAssemblyName in wasmFrameworkAssembliesOneOf)
{
string thisWasmFrameworkAssemblyPath = Path.Combine(bclDir, thisWasmFrameworkAssemblyName.newName + ".dll");
string thisWasmFrameworkAssemblyPath =
Path.Combine(bclDir, thisWasmFrameworkAssemblyName.newName + ".dll");
if (File.Exists(thisWasmFrameworkAssemblyPath))
{
assemblies[thisWasmFrameworkAssemblyName.newName] = thisWasmFrameworkAssemblyPath;
}
else
{
thisWasmFrameworkAssemblyPath = Path.Combine(bclDir, thisWasmFrameworkAssemblyName.oldName + ".dll");
thisWasmFrameworkAssemblyPath =
Path.Combine(bclDir, thisWasmFrameworkAssemblyName.oldName + ".dll");
if (!File.Exists(thisWasmFrameworkAssemblyPath))
{
throw new FileNotFoundException("Expected one of the following assemblies but none were found: " +
$"'{thisWasmFrameworkAssemblyName.newName}' / '{thisWasmFrameworkAssemblyName.oldName}'",
throw new FileNotFoundException(
"Expected one of the following assemblies but none were found: " +
$"'{thisWasmFrameworkAssemblyName.newName}' / '{thisWasmFrameworkAssemblyName.oldName}'",
thisWasmFrameworkAssemblyPath);
}

Expand All @@ -227,7 +234,12 @@ private void _ExportBeginImpl(string[] features, bool isDebug, string path, int
}

var initialAssemblies = assemblies.Duplicate();
internal_GetExportedAssemblyDependencies(initialAssemblies, buildConfig, bclDir, assemblies);
godot_dictionary initialAssembliesAux = ((Godot.Collections.Dictionary)initialAssemblies).NativeValue;
using godot_string buildConfigAux = Marshaling.mono_string_to_godot(buildConfig);
using godot_string bclDirAux = Marshaling.mono_string_to_godot(bclDir);
godot_dictionary assembliesAux = ((Godot.Collections.Dictionary)assemblies).NativeValue;
internal_GetExportedAssemblyDependencies(initialAssembliesAux, buildConfigAux, bclDirAux,
ref assembliesAux);

AddI18NAssemblies(assemblies, bclDir);

Expand All @@ -237,9 +249,12 @@ private void _ExportBeginImpl(string[] features, bool isDebug, string path, int
outputDataDir = ExportDataDirectory(features, platform, isDebug, outputDir);

string apiConfig = isDebug ? "Debug" : "Release";
string resAssembliesDir = Path.Combine(GodotSharpDirs.ResAssembliesBaseDir, apiConfig);
// TODO
throw new NotImplementedException();
string resAssembliesDir = null; // Path.Combine(GodotSharpDirs.ResAssembliesBaseDir, apiConfig);

bool assembliesInsidePck = (bool)ProjectSettings.GetSetting("mono/export/export_assemblies_inside_pck") || outputDataDir == null;
bool assembliesInsidePck = (bool)ProjectSettings.GetSetting("mono/export/export_assemblies_inside_pck") ||
outputDataDir == null;

if (!assembliesInsidePck)
{
Expand Down Expand Up @@ -277,7 +292,8 @@ void AddToAssembliesDir(string fileSrcPath)
}

// AOT compilation
bool aotEnabled = platform == OS.Platforms.iOS || (bool)ProjectSettings.GetSetting("mono/export/aot/enabled");
bool aotEnabled = platform == OS.Platforms.iOS ||
(bool)ProjectSettings.GetSetting("mono/export/aot/enabled");

if (aotEnabled)
{
Expand All @@ -296,14 +312,19 @@ void AddToAssembliesDir(string fileSrcPath)
LLVMOnly = false,
LLVMPath = "",
LLVMOutputPath = "",
FullAot = platform == OS.Platforms.iOS || (bool)(ProjectSettings.GetSetting("mono/export/aot/full_aot") ?? false),
FullAot = platform == OS.Platforms.iOS ||
(bool)(ProjectSettings.GetSetting("mono/export/aot/full_aot") ?? false),
UseInterpreter = (bool)ProjectSettings.GetSetting("mono/export/aot/use_interpreter"),
ExtraAotOptions = (string[])ProjectSettings.GetSetting("mono/export/aot/extra_aot_options") ?? Array.Empty<string>(),
ExtraOptimizerOptions = (string[])ProjectSettings.GetSetting("mono/export/aot/extra_optimizer_options") ?? Array.Empty<string>(),
ExtraAotOptions = (string[])ProjectSettings.GetSetting("mono/export/aot/extra_aot_options") ??
Array.Empty<string>(),
ExtraOptimizerOptions =
(string[])ProjectSettings.GetSetting("mono/export/aot/extra_optimizer_options") ??
Array.Empty<string>(),
ToolchainPath = aotToolchainPath
};

AotBuilder.CompileAssemblies(this, aotOpts, features, platform, isDebug, bclDir, outputDir, outputDataDir, assemblies);
AotBuilder.CompileAssemblies(this, aotOpts, features, platform, isDebug, bclDir, outputDir,
outputDataDir, assemblies);
}
}

Expand All @@ -316,8 +337,10 @@ public override void _ExportEnd()
if (Directory.Exists(aotTempDir))
Directory.Delete(aotTempDir, recursive: true);

// TODO: Just a workaround until the export plugins can be made to abort with errors
if (!string.IsNullOrEmpty(_maybeLastExportError)) // Check empty as well, because it's set to empty after hot-reloading
// TODO: The following is just a workaround until the export plugins can be made to abort with errors

// We check for empty as well, because it's set to empty after hot-reloading
if (!string.IsNullOrEmpty(_maybeLastExportError))
{
string lastExportError = _maybeLastExportError;
_maybeLastExportError = null;
Expand Down Expand Up @@ -381,7 +404,8 @@ private static string ExportDataDirectory(string[] features, string platform, bo
private static bool PlatformHasTemplateDir(string platform)
{
// macOS export templates are contained in a zip, so we place our custom template inside it and let Godot do the rest.
return !new[] { OS.Platforms.MacOS, OS.Platforms.Android, OS.Platforms.iOS, OS.Platforms.HTML5 }.Contains(platform);
return !new[] { OS.Platforms.MacOS, OS.Platforms.Android, OS.Platforms.iOS, OS.Platforms.HTML5 }
.Contains(platform);
}

private static bool DeterminePlatformFromFeatures(IEnumerable<string> features, out string platform)
Expand Down Expand Up @@ -476,7 +500,8 @@ private static string DetermineDataDirNameForProject()
}

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern void internal_GetExportedAssemblyDependencies(Godot.Collections.Dictionary<string, string> initialAssemblies,
string buildConfig, string customBclDir, Godot.Collections.Dictionary<string, string> dependencyAssemblies);
private static extern void internal_GetExportedAssemblyDependencies(
in godot_dictionary initialAssemblies, in godot_string buildConfig,
in godot_string customBclDir, ref godot_dictionary dependencyAssemblies);
}
}
Loading

0 comments on commit 513ee85

Please sign in to comment.