Browse Source

Inherited C# scene not inheriting parent's fields

When a child scene inherits a parent scene with a C# root node, the
parent scene's export variables appear to assume values set in the
parent scene, in the child scene's Inspector. However, when the child
scene is played, the parent scene's export variables assume default
values.
When a node is created, it inherits its parent C# script's fields from
the map CSharpScriptInstance::script->member_info. However this map was
not initialized outside the editor, and this commit ensured it is. This
fixes issues #36480 and #37581.
pepegadeveloper123 5 years ago
parent
commit
4e00d8520d
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() {
 bool CSharpScript::_update_exports() {
 
 
 #ifdef TOOLS_ENABLED
 #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)
 	if (!valid)
 		return false;
 		return false;
 
 
 	bool changed = false;
 	bool changed = false;
 
 
-	if (exports_invalidated) {
+#ifdef TOOLS_ENABLED
+	if (exports_invalidated)
+#endif
+	{
 		GD_MONO_SCOPE_THREAD_ATTACH;
 		GD_MONO_SCOPE_THREAD_ATTACH;
 
 
-		exports_invalidated = false;
-
 		changed = true;
 		changed = true;
 
 
 		member_info.clear();
 		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;
 		GDMonoClass *top = script_class;
 
 
@@ -2488,16 +2498,16 @@ bool CSharpScript::_update_exports() {
 				if (_get_member_export(field, /* inspect export: */ true, prop_info, exported)) {
 				if (_get_member_export(field, /* inspect export: */ true, prop_info, exported)) {
 					StringName member_name = field->get_name();
 					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);
 						exported_members_cache.push_front(prop_info);
 
 
 						if (tmp_object) {
 						if (tmp_object) {
 							exported_members_defval_cache[member_name] = GDMonoMarshal::mono_object_to_variant(field->get_value(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)) {
 				if (_get_member_export(property, /* inspect export: */ true, prop_info, exported)) {
 					StringName member_name = property->get_name();
 					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);
 						exported_members_cache.push_front(prop_info);
-
 						if (tmp_object) {
 						if (tmp_object) {
 							MonoException *exc = nullptr;
 							MonoException *exc = nullptr;
 							MonoObject *ret = property->get_value(tmp_object, &exc);
 							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);
 								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();
 			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;
 	return changed;
-#endif
-	return false;
 }
 }
 
 
 void CSharpScript::load_script_signals(GDMonoClass *p_class, GDMonoClass *p_native_class) {
 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;
 	return true;
 }
 }
 
 
-#ifdef TOOLS_ENABLED
 /**
 /**
  * Returns false if there was an error, otherwise true.
  * 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.
  * 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())
 	(m_member->get_enclosing_class()->get_full_name() + "." + (String)m_member->get_name())
 
 
 	if (p_member->is_static()) {
 	if (p_member->is_static()) {
+#ifdef TOOLS_ENABLED
 		if (p_member->has_attribute(CACHED_CLASS(ExportAttribute)))
 		if (p_member->has_attribute(CACHED_CLASS(ExportAttribute)))
 			ERR_PRINT("Cannot export member because it is static: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
 			ERR_PRINT("Cannot export member because it is static: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
+#endif
 		return false;
 		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) {
 	if (p_member->get_member_type() == IMonoClassMember::MEMBER_TYPE_PROPERTY) {
 		GDMonoProperty *property = static_cast<GDMonoProperty *>(p_member);
 		GDMonoProperty *property = static_cast<GDMonoProperty *>(p_member);
 		if (!property->has_getter()) {
 		if (!property->has_getter()) {
+#ifdef TOOLS_ENABLED
 			if (exported)
 			if (exported)
 				ERR_PRINT("Read-only property cannot be exported: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
 				ERR_PRINT("Read-only property cannot be exported: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
+#endif
 			return false;
 			return false;
 		}
 		}
 		if (!property->has_setter()) {
 		if (!property->has_setter()) {
+#ifdef TOOLS_ENABLED
 			if (exported)
 			if (exported)
 				ERR_PRINT("Write-only property (without getter) cannot be exported: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
 				ERR_PRINT("Write-only property (without getter) cannot be exported: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
+#endif
 			return false;
 			return false;
 		}
 		}
 	}
 	}
@@ -2742,10 +2762,13 @@ bool CSharpScript::_get_member_export(IMonoClassMember *p_member, bool p_inspect
 	String hint_string;
 	String hint_string;
 
 
 	if (variant_type == Variant::NIL && !nil_is_variant) {
 	if (variant_type == Variant::NIL && !nil_is_variant) {
+#ifdef TOOLS_ENABLED
 		ERR_PRINT("Unknown exported member type: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
 		ERR_PRINT("Unknown exported member type: '" + MEMBER_FULL_QUALIFIED_NAME(p_member) + "'.");
+#endif
 		return false;
 		return false;
 	}
 	}
 
 
+#ifdef TOOLS_ENABLED
 	int hint_res = _try_get_member_export_hint(p_member, type, variant_type, /* allow_generics: */ true, hint, hint_string);
 	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,
 	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 = PropertyHint(CACHED_FIELD(ExportAttribute, hint)->get_int_value(attr));
 		hint_string = CACHED_FIELD(ExportAttribute, hintString)->get_string_value(attr);
 		hint_string = CACHED_FIELD(ExportAttribute, hintString)->get_string_value(attr);
 	}
 	}
+#endif
 
 
 	uint32_t prop_usage = PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_SCRIPT_VARIABLE;
 	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
 #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) {
 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) {
 	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 _get_signal(GDMonoClass *p_class, GDMonoMethod *p_delegate_invoke, Vector<SignalParameter> &params);
 
 
 	bool _update_exports();
 	bool _update_exports();
-#ifdef TOOLS_ENABLED
+
 	bool _get_member_export(IMonoClassMember *p_member, bool p_inspect_export, PropertyInfo &r_prop_info, bool &r_exported);
 	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);
 	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