Browse Source

Merge pull request #38638 from pepegadeveloper123/mono_inherited_scene_bug

Fix inherited C# scene not inheriting parent's fields
Rémi Verschelde 5 years ago
parent
commit
dbd90b9478
2 changed files with 94 additions and 68 deletions
  1. 92 67
      modules/mono/csharp_script.cpp
  2. 2 1
      modules/mono/csharp_script.h

+ 92 - 67
modules/mono/csharp_script.cpp

@@ -2421,58 +2421,68 @@ void CSharpScript::_update_member_info_no_exports() {
 bool CSharpScript::_update_exports() {
 
 #ifdef TOOLS_ENABLED
-	if (!Engine::get_singleton()->is_editor_hint())
-		return false;
-
-	placeholder_fallback_enabled = true; // until proven otherwise
-
+	bool is_editor = Engine::get_singleton()->is_editor_hint();
+	if (is_editor)
+		placeholder_fallback_enabled = true; // until proven otherwise
+#endif
 	if (!valid)
 		return false;
 
 	bool changed = false;
 
-	if (exports_invalidated) {
+#ifdef TOOLS_ENABLED
+	if (exports_invalidated)
+#endif
+	{
 		GD_MONO_SCOPE_THREAD_ATTACH;
 
-		exports_invalidated = false;
-
 		changed = true;
 
 		member_info.clear();
-		exported_members_cache.clear();
-		exported_members_defval_cache.clear();
 
-		// Here we create a temporary managed instance of the class to get the initial values
+#ifdef TOOLS_ENABLED
+		MonoObject *tmp_object = nullptr;
+		Object *tmp_native = nullptr;
+		uint32_t tmp_pinned_gchandle = 0;
 
-		MonoObject *tmp_object = mono_object_new(mono_domain_get(), script_class->get_mono_ptr());
+		if (is_editor) {
+			exports_invalidated = false;
 
-		if (!tmp_object) {
-			ERR_PRINT("Failed to allocate temporary MonoObject.");
-			return false;
-		}
+			exported_members_cache.clear();
+			exported_members_defval_cache.clear();
 
-		uint32_t tmp_pinned_gchandle = GDMonoUtils::new_strong_gchandle_pinned(tmp_object); // pin it (not sure if needed)
+			// Here we create a temporary managed instance of the class to get the initial values
+			tmp_object = mono_object_new(mono_domain_get(), script_class->get_mono_ptr());
 
-		GDMonoMethod *ctor = script_class->get_method(CACHED_STRING_NAME(dotctor), 0);
+			if (!tmp_object) {
+				ERR_PRINT("Failed to allocate temporary MonoObject.");
+				return false;
+			}
 
-		ERR_FAIL_NULL_V_MSG(ctor, false,
-				"Cannot construct temporary MonoObject because the class does not define a parameterless constructor: '" + get_path() + "'.");
+			tmp_pinned_gchandle = GDMonoUtils::new_strong_gchandle_pinned(tmp_object); // pin it (not sure if needed)
 
-		MonoException *ctor_exc = nullptr;
-		ctor->invoke(tmp_object, nullptr, &ctor_exc);
+			GDMonoMethod *ctor = script_class->get_method(CACHED_STRING_NAME(dotctor), 0);
 
-		Object *tmp_native = GDMonoMarshal::unbox<Object *>(CACHED_FIELD(GodotObject, ptr)->get_value(tmp_object));
+			ERR_FAIL_NULL_V_MSG(ctor, false,
+					"Cannot construct temporary MonoObject because the class does not define a parameterless constructor: '" + get_path() + "'.");
 
-		if (ctor_exc) {
-			// TODO: Should we free 'tmp_native' if the exception was thrown after its creation?
+			MonoException *ctor_exc = nullptr;
+			ctor->invoke(tmp_object, nullptr, &ctor_exc);
 
-			GDMonoUtils::free_gchandle(tmp_pinned_gchandle);
-			tmp_object = nullptr;
+			tmp_native = GDMonoMarshal::unbox<Object *>(CACHED_FIELD(GodotObject, ptr)->get_value(tmp_object));
 
-			ERR_PRINT("Exception thrown from constructor of temporary MonoObject:");
-			GDMonoUtils::debug_print_unhandled_exception(ctor_exc);
-			return false;
+			if (ctor_exc) {
+				// TODO: Should we free 'tmp_native' if the exception was thrown after its creation?
+
+				GDMonoUtils::free_gchandle(tmp_pinned_gchandle);
+				tmp_object = nullptr;
+
+				ERR_PRINT("Exception thrown from constructor of temporary MonoObject:");
+				GDMonoUtils::debug_print_unhandled_exception(ctor_exc);
+				return false;
+			}
 		}
+#endif
 
 		GDMonoClass *top = script_class;
 
@@ -2488,16 +2498,16 @@ bool CSharpScript::_update_exports() {
 				if (_get_member_export(field, /* inspect export: */ true, prop_info, exported)) {
 					StringName member_name = field->get_name();
 
-					if (exported) {
-						member_info[member_name] = prop_info;
+					member_info[member_name] = prop_info;
+#ifdef TOOLS_ENABLED
+					if (is_editor && exported) {
 						exported_members_cache.push_front(prop_info);
 
 						if (tmp_object) {
 							exported_members_defval_cache[member_name] = GDMonoMarshal::mono_object_to_variant(field->get_value(tmp_object));
 						}
-					} else {
-						member_info[member_name] = prop_info;
 					}
+#endif
 				}
 			}
 
@@ -2509,10 +2519,10 @@ bool CSharpScript::_update_exports() {
 				if (_get_member_export(property, /* inspect export: */ true, prop_info, exported)) {
 					StringName member_name = property->get_name();
 
-					if (exported) {
-						member_info[member_name] = prop_info;
+					member_info[member_name] = prop_info;
+#ifdef TOOLS_ENABLED
+					if (is_editor && exported) {
 						exported_members_cache.push_front(prop_info);
-
 						if (tmp_object) {
 							MonoException *exc = nullptr;
 							MonoObject *ret = property->get_value(tmp_object, &exc);
@@ -2523,57 +2533,62 @@ bool CSharpScript::_update_exports() {
 								exported_members_defval_cache[member_name] = GDMonoMarshal::mono_object_to_variant(ret);
 							}
 						}
-					} else {
-						member_info[member_name] = prop_info;
 					}
+#endif
 				}
 			}
 
 			top = top->get_parent_class();
 		}
 
-		// Need to check this here, before disposal
-		bool base_ref = Object::cast_to<Reference>(tmp_native) != nullptr;
+#ifdef TOOLS_ENABLED
+		if (is_editor) {
+			// Need to check this here, before disposal
+			bool base_ref = Object::cast_to<Reference>(tmp_native) != nullptr;
 
-		// Dispose the temporary managed instance
+			// Dispose the temporary managed instance
 
-		MonoException *exc = nullptr;
-		GDMonoUtils::dispose(tmp_object, &exc);
+			MonoException *exc = nullptr;
+			GDMonoUtils::dispose(tmp_object, &exc);
 
-		if (exc) {
-			ERR_PRINT("Exception thrown from method Dispose() of temporary MonoObject:");
-			GDMonoUtils::debug_print_unhandled_exception(exc);
-		}
+			if (exc) {
+				ERR_PRINT("Exception thrown from method Dispose() of temporary MonoObject:");
+				GDMonoUtils::debug_print_unhandled_exception(exc);
+			}
 
-		GDMonoUtils::free_gchandle(tmp_pinned_gchandle);
-		tmp_object = nullptr;
+			GDMonoUtils::free_gchandle(tmp_pinned_gchandle);
+			tmp_object = nullptr;
 
-		if (tmp_native && !base_ref) {
-			Node *node = Object::cast_to<Node>(tmp_native);
-			if (node && node->is_inside_tree()) {
-				ERR_PRINT("Temporary instance was added to the scene tree.");
-			} else {
-				memdelete(tmp_native);
+			if (tmp_native && !base_ref) {
+				Node *node = Object::cast_to<Node>(tmp_native);
+				if (node && node->is_inside_tree()) {
+					ERR_PRINT("Temporary instance was added to the scene tree.");
+				} else {
+					memdelete(tmp_native);
+				}
 			}
 		}
+#endif
 	}
 
-	placeholder_fallback_enabled = false;
+#ifdef TOOLS_ENABLED
+	if (is_editor) {
+		placeholder_fallback_enabled = false;
 
-	if (placeholders.size()) {
-		// Update placeholders if any
-		Map<StringName, Variant> values;
-		List<PropertyInfo> propnames;
-		_update_exports_values(values, propnames);
+		if (placeholders.size()) {
+			// Update placeholders if any
+			Map<StringName, Variant> values;
+			List<PropertyInfo> propnames;
+			_update_exports_values(values, propnames);
 
-		for (Set<PlaceHolderScriptInstance *>::Element *E = placeholders.front(); E; E = E->next()) {
-			E->get()->update(propnames, values);
+			for (Set<PlaceHolderScriptInstance *>::Element *E = placeholders.front(); E; E = E->next()) {
+				E->get()->update(propnames, values);
+			}
 		}
 	}
+#endif
 
 	return changed;
-#endif
-	return false;
 }
 
 void CSharpScript::load_script_signals(GDMonoClass *p_class, GDMonoClass *p_native_class) {
@@ -2679,7 +2694,6 @@ bool CSharpScript::_get_signal(GDMonoClass *p_class, GDMonoMethod *p_delegate_in
 	return true;
 }
 
-#ifdef TOOLS_ENABLED
 /**
  * Returns false if there was an error, otherwise true.
  * If there was an error, r_prop_info and r_exported are not assigned any value.
@@ -2693,8 +2707,10 @@ bool CSharpScript::_get_member_export(IMonoClassMember *p_member, bool p_inspect
 	(m_member->get_enclosing_class()->get_full_name() + "." + (String)m_member->get_name())
 
 	if (p_member->is_static()) {
+#ifdef TOOLS_ENABLED
 		if (p_member->has_attribute(CACHED_CLASS(ExportAttribute)))
 			ERR_PRINT("Cannot export member because it is static: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
+#endif
 		return false;
 	}
 
@@ -2716,13 +2732,17 @@ bool CSharpScript::_get_member_export(IMonoClassMember *p_member, bool p_inspect
 	if (p_member->get_member_type() == IMonoClassMember::MEMBER_TYPE_PROPERTY) {
 		GDMonoProperty *property = static_cast<GDMonoProperty *>(p_member);
 		if (!property->has_getter()) {
+#ifdef TOOLS_ENABLED
 			if (exported)
 				ERR_PRINT("Read-only property cannot be exported: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
+#endif
 			return false;
 		}
 		if (!property->has_setter()) {
+#ifdef TOOLS_ENABLED
 			if (exported)
 				ERR_PRINT("Write-only property (without getter) cannot be exported: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
+#endif
 			return false;
 		}
 	}
@@ -2742,10 +2762,13 @@ bool CSharpScript::_get_member_export(IMonoClassMember *p_member, bool p_inspect
 	String hint_string;
 
 	if (variant_type == Variant::NIL && !nil_is_variant) {
+#ifdef TOOLS_ENABLED
 		ERR_PRINT("Unknown exported member type: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
+#endif
 		return false;
 	}
 
+#ifdef TOOLS_ENABLED
 	int hint_res = _try_get_member_export_hint(p_member, type, variant_type, /* allow_generics: */ true, hint, hint_string);
 
 	ERR_FAIL_COND_V_MSG(hint_res == -1, false,
@@ -2756,6 +2779,7 @@ bool CSharpScript::_get_member_export(IMonoClassMember *p_member, bool p_inspect
 		hint = PropertyHint(CACHED_FIELD(ExportAttribute, hint)->get_int_value(attr));
 		hint_string = CACHED_FIELD(ExportAttribute, hintString)->get_string_value(attr);
 	}
+#endif
 
 	uint32_t prop_usage = PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_SCRIPT_VARIABLE;
 
@@ -2772,6 +2796,7 @@ bool CSharpScript::_get_member_export(IMonoClassMember *p_member, bool p_inspect
 #undef MEMBER_FULL_QUALIFIED_NAME
 }
 
+#ifdef TOOLS_ENABLED
 int CSharpScript::_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) {
 
 	if (p_variant_type == Variant::NIL) {

+ 2 - 1
modules/mono/csharp_script.h

@@ -147,8 +147,9 @@ private:
 	bool _get_signal(GDMonoClass *p_class, GDMonoMethod *p_delegate_invoke, Vector<SignalParameter> &params);
 
 	bool _update_exports();
-#ifdef TOOLS_ENABLED
+
 	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