C#/Mono: Check assembly version when loading

Not sure if we should check revision too, but this is good enough for what we want.
This will be needed to load the correct Microsoft.Build when we switch to the nuget version.

2 files changed, 54 insertions, 40 deletions

diff --git a/modules/mono/editor/bindings_generator.cpp b/modules/mono/editor/bindings_generator.cpp
index bdf9cf965f..258b8ed3ed 100644
--- a/modules/mono/editor/bindings_generator.cpp
+++ b/modules/mono/editor/bindings_generator.cpp
@@ -1664,6 +1664,10 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf
 		}
 
 		if (!p_imethod.is_internal) {
+			// TODO: This alone adds ~0.2 MB of bloat to the core API assembly. It would be
+			// better to generate a table in the C++ glue instead. That way the strings wouldn't
+			// add that much extra bloat as they're already used in engine code. Also, it would
+			// probably be much faster than looking up the attributes when fetching methods.
 			p_output.append(MEMBER_BEGIN "[GodotMethod(\"");
 			p_output.append(p_imethod.name);
 			p_output.append("\")]");
@@ -2139,7 +2143,7 @@ Error BindingsGenerator::_generate_glue_method(const BindingsGenerator::TypeInte
 			if (return_type->ret_as_byref_arg) {
 				p_output.append("\tif (" CS_PARAM_INSTANCE " == nullptr) { *arg_ret = ");
 				p_output.append(fail_ret);
-				p_output.append("; ERR_FAIL_MSG(\"Parameter ' arg_ret ' is null.\"); }\n");
+				p_output.append("; ERR_FAIL_MSG(\"Parameter ' " CS_PARAM_INSTANCE " ' is null.\"); }\n");
 			} else {
 				p_output.append("\tERR_FAIL_NULL_V(" CS_PARAM_INSTANCE ", ");
 				p_output.append(fail_ret);
@@ -2390,6 +2394,11 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
 			if (property.usage & PROPERTY_USAGE_GROUP || property.usage & PROPERTY_USAGE_SUBGROUP || property.usage & PROPERTY_USAGE_CATEGORY)
 				continue;
 
+			if (property.name.find("/") >= 0) {
+				// Ignore properties with '/' (slash) in the name. These are only meant for use in the inspector.
+				continue;
+			}
+
 			PropertyInterface iprop;
 			iprop.cname = property.name;
 			iprop.setter = ClassDB::get_property_setter(type_cname, iprop.cname);
@@ -2402,7 +2411,7 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
 
 			bool valid = false;
 			iprop.index = ClassDB::get_property_index(type_cname, iprop.cname, &valid);
-			ERR_FAIL_COND_V(!valid, false);
+			ERR_FAIL_COND_V_MSG(!valid, false, "Invalid property: '" + itype.name + "." + String(iprop.cname) + "'.");
 
 			iprop.proxy_name = escape_csharp_keyword(snake_to_pascal_case(iprop.cname));
 
@@ -2414,8 +2423,6 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
 				iprop.proxy_name += "_";
 			}
 
-			iprop.proxy_name = iprop.proxy_name.replace("/", "__"); // Some members have a slash...
-
 			iprop.prop_doc = nullptr;
 
 			for (int i = 0; i < itype.class_doc->properties.size(); i++) {
diff --git a/modules/mono/editor/godotsharp_export.cpp b/modules/mono/editor/godotsharp_export.cpp
index 4126da16be..d6a271f1d9 100644
--- a/modules/mono/editor/godotsharp_export.cpp
+++ b/modules/mono/editor/godotsharp_export.cpp
@@ -32,68 +32,70 @@
 
 #include <mono/metadata/image.h>
 
+#include "core/io/file_access_pack.h"
 #include "core/os/os.h"
+#include "core/project_settings.h"
 
 #include "../mono_gd/gd_mono.h"
 #include "../mono_gd/gd_mono_assembly.h"
 #include "../mono_gd/gd_mono_cache.h"
+#include "../utils/macros.h"
 
 namespace GodotSharpExport {
 
-String get_assemblyref_name(MonoImage *p_image, int index) {
+struct AssemblyRefInfo {
+	String name;
+	uint16_t major;
+	uint16_t minor;
+	uint16_t build;
+	uint16_t revision;
+};
+
+AssemblyRefInfo get_assemblyref_name(MonoImage *p_image, int index) {
 	const MonoTableInfo *table_info = mono_image_get_table_info(p_image, MONO_TABLE_ASSEMBLYREF);
 
 	uint32_t cols[MONO_ASSEMBLYREF_SIZE];
 
 	mono_metadata_decode_row(table_info, index, cols, MONO_ASSEMBLYREF_SIZE);
 
-	return String::utf8(mono_metadata_string_heap(p_image, cols[MONO_ASSEMBLYREF_NAME]));
+	return {
+		String::utf8(mono_metadata_string_heap(p_image, cols[MONO_ASSEMBLYREF_NAME])),
+		(uint16_t)cols[MONO_ASSEMBLYREF_MAJOR_VERSION],
+		(uint16_t)cols[MONO_ASSEMBLYREF_MINOR_VERSION],
+		(uint16_t)cols[MONO_ASSEMBLYREF_BUILD_NUMBER],
+		(uint16_t)cols[MONO_ASSEMBLYREF_REV_NUMBER]
+	};
 }
 
 Error get_assembly_dependencies(GDMonoAssembly *p_assembly, const Vector<String> &p_search_dirs, Dictionary &r_assembly_dependencies) {
 	MonoImage *image = p_assembly->get_image();
 
 	for (int i = 0; i < mono_image_get_table_rows(image, MONO_TABLE_ASSEMBLYREF); i++) {
-		String ref_name = get_assemblyref_name(image, i);
+		AssemblyRefInfo ref_info = get_assemblyref_name(image, i);
+
+		const String &ref_name = ref_info.name;
 
 		if (r_assembly_dependencies.has(ref_name))
 			continue;
 
-		GDMonoAssembly *ref_assembly = nullptr;
-		String path;
-		bool has_extension = ref_name.ends_with(".dll") || ref_name.ends_with(".exe");
-
-		for (int j = 0; j < p_search_dirs.size(); j++) {
-			const String &search_dir = p_search_dirs[j];
-
-			if (has_extension) {
-				path = search_dir.plus_file(ref_name);
-				if (FileAccess::exists(path)) {
-					GDMono::get_singleton()->load_assembly_from(ref_name.get_basename(), path, &ref_assembly, true);
-					if (ref_assembly != nullptr)
-						break;
-				}
-			} else {
-				path = search_dir.plus_file(ref_name + ".dll");
-				if (FileAccess::exists(path)) {
-					GDMono::get_singleton()->load_assembly_from(ref_name, path, &ref_assembly, true);
-					if (ref_assembly != nullptr)
-						break;
-				}
-
-				path = search_dir.plus_file(ref_name + ".exe");
-				if (FileAccess::exists(path)) {
-					GDMono::get_singleton()->load_assembly_from(ref_name, path, &ref_assembly, true);
-					if (ref_assembly != nullptr)
-						break;
-				}
-			}
-		}
+		GDMonoAssembly *ref_assembly = NULL;
+
+		{
+			MonoAssemblyName *ref_aname = mono_assembly_name_new("A"); // We can't allocate an empty MonoAssemblyName, hence "A"
+			CRASH_COND(ref_aname == nullptr);
+			SCOPE_EXIT {
+				mono_assembly_name_free(ref_aname);
+				mono_free(ref_aname);
+			};
 
-		ERR_FAIL_COND_V_MSG(!ref_assembly, ERR_CANT_RESOLVE, "Cannot load assembly (refonly): '" + ref_name + "'.");
+			mono_assembly_get_assemblyref(image, i, ref_aname);
 
-		// Use the path we got from the search. Don't try to get the path from the loaded assembly as we can't trust it will be from the selected BCL dir.
-		r_assembly_dependencies[ref_name] = path;
+			if (!GDMono::get_singleton()->load_assembly(ref_name, ref_aname, &ref_assembly, /* refonly: */ true, p_search_dirs)) {
+				ERR_FAIL_V_MSG(ERR_CANT_RESOLVE, "Cannot load assembly (refonly): '" + ref_name + "'.");
+			}
+
+			r_assembly_dependencies[ref_name] = ref_assembly->get_path();
+		}
 
 		Error err = get_assembly_dependencies(ref_assembly, p_search_dirs, r_assembly_dependencies);
 		ERR_FAIL_COND_V_MSG(err != OK, err, "Cannot load one of the dependencies for the assembly: '" + ref_name + "'.");
@@ -113,6 +115,11 @@ Error get_exported_assembly_dependencies(const Dictionary &p_initial_assemblies,
 	Vector<String> search_dirs;
 	GDMonoAssembly::fill_search_dirs(search_dirs, p_build_config, p_custom_bcl_dir);
 
+	if (p_custom_bcl_dir.length()) {
+		// Only one mscorlib can be loaded. We need this workaround to make sure we get it from the right BCL directory.
+		r_assembly_dependencies["mscorlib"] = p_custom_bcl_dir.plus_file("mscorlib.dll").simplify_path();
+	}
+
 	for (const Variant *key = p_initial_assemblies.next(); key; key = p_initial_assemblies.next(key)) {
 		String assembly_name = *key;
 		String assembly_path = p_initial_assemblies[*key];