From f3c7527225c73812a7de10dfc273ba0e82e8d7ec Mon Sep 17 00:00:00 2001
From: Ignacio Etcheverry <ignalfonsore@gmail.com>
Date: Sun, 29 Jul 2018 22:40:09 +0200
Subject: Fix case where exported properties value is lost

Fixes exported property modified values lost when creating a placeholder script instance with a failed script compilation

- Object set/get will call PlaceHolderScriptInstance's new fallback set/get methods as a last resort. This way, placeholder script instances can keep the values for storage or until the script is compiled successfuly.
- Script::can_instance() will only return true if a real script instance can be created. Otherwise, in the case of placeholder script instances, it will return false.
- Object::set_script(script) is now in charge of requesting the creation of placeholder script instances. It's no longer Script::instance_create(owner)'s duty.
- PlaceHolderScriptInstance has a new method set_build_failed(bool) to determine whether it should call into its script methods or not.
- Fixed a few problems during reloading of C# scripts.
---
 core/script_language.cpp | 94 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 85 insertions(+), 9 deletions(-)

(limited to 'core/script_language.cpp')

diff --git a/core/script_language.cpp b/core/script_language.cpp
index 37ba3cfc62..e146fb773c 100644
--- a/core/script_language.cpp
+++ b/core/script_language.cpp
@@ -255,6 +255,17 @@ void ScriptInstance::call_multilevel_reversed(const StringName &p_method, const
 	call(p_method, p_args, p_argcount, ce); // script may not support multilevel calls
 }
 
+void ScriptInstance::property_set_fallback(const StringName &, const Variant &, bool *r_valid) {
+	if (r_valid)
+		*r_valid = false;
+}
+
+Variant ScriptInstance::property_get_fallback(const StringName &, bool *r_valid) {
+	if (r_valid)
+		*r_valid = false;
+	return Variant();
+}
+
 void ScriptInstance::call_multilevel(const StringName &p_method, VARIANT_ARG_DECLARE) {
 
 	VARIANT_ARGPTRS;
@@ -364,6 +375,9 @@ ScriptDebugger::ScriptDebugger() {
 
 bool PlaceHolderScriptInstance::set(const StringName &p_name, const Variant &p_value) {
 
+	if (build_failed)
+		return false;
+
 	if (values.has(p_name)) {
 		Variant defval;
 		if (script->get_property_default_value(p_name, defval)) {
@@ -392,22 +406,31 @@ bool PlaceHolderScriptInstance::get(const StringName &p_name, Variant &r_ret) co
 		return true;
 	}
 
-	Variant defval;
-	if (script->get_property_default_value(p_name, defval)) {
-		r_ret = defval;
-		return true;
+	if (!build_failed) {
+		Variant defval;
+		if (script->get_property_default_value(p_name, defval)) {
+			r_ret = defval;
+			return true;
+		}
 	}
+
 	return false;
 }
 
 void PlaceHolderScriptInstance::get_property_list(List<PropertyInfo> *p_properties) const {
 
-	for (const List<PropertyInfo>::Element *E = properties.front(); E; E = E->next()) {
-		PropertyInfo pinfo = E->get();
-		if (!values.has(pinfo.name)) {
-			pinfo.usage |= PROPERTY_USAGE_SCRIPT_DEFAULT_VALUE;
+	if (build_failed) {
+		for (const List<PropertyInfo>::Element *E = properties.front(); E; E = E->next()) {
+			p_properties->push_back(E->get());
+		}
+	} else {
+		for (const List<PropertyInfo>::Element *E = properties.front(); E; E = E->next()) {
+			PropertyInfo pinfo = E->get();
+			if (!values.has(pinfo.name)) {
+				pinfo.usage |= PROPERTY_USAGE_SCRIPT_DEFAULT_VALUE;
+			}
+			p_properties->push_back(E->get());
 		}
-		p_properties->push_back(E->get());
 	}
 }
 
@@ -426,12 +449,18 @@ Variant::Type PlaceHolderScriptInstance::get_property_type(const StringName &p_n
 
 void PlaceHolderScriptInstance::get_method_list(List<MethodInfo> *p_list) const {
 
+	if (build_failed)
+		return;
+
 	if (script.is_valid()) {
 		script->get_script_method_list(p_list);
 	}
 }
 bool PlaceHolderScriptInstance::has_method(const StringName &p_method) const {
 
+	if (build_failed)
+		return false;
+
 	if (script.is_valid()) {
 		return script->has_method(p_method);
 	}
@@ -440,6 +469,8 @@ bool PlaceHolderScriptInstance::has_method(const StringName &p_method) const {
 
 void PlaceHolderScriptInstance::update(const List<PropertyInfo> &p_properties, const Map<StringName, Variant> &p_values) {
 
+	build_failed = false;
+
 	Set<StringName> new_values;
 	for (const List<PropertyInfo>::Element *E = p_properties.front(); E; E = E->next()) {
 
@@ -483,6 +514,51 @@ void PlaceHolderScriptInstance::update(const List<PropertyInfo> &p_properties, c
 	//change notify
 }
 
+void PlaceHolderScriptInstance::property_set_fallback(const StringName &p_name, const Variant &p_value, bool *r_valid) {
+
+	if (build_failed) {
+		Map<StringName, Variant>::Element *E = values.find(p_name);
+
+		if (E) {
+			E->value() = p_value;
+		} else {
+			values.insert(p_name, p_value);
+		}
+
+		bool found = false;
+		for (const List<PropertyInfo>::Element *E = properties.front(); E; E = E->next()) {
+			if (E->get().name == p_name) {
+				found = true;
+				break;
+			}
+		}
+		if (!found) {
+			properties.push_back(PropertyInfo(p_value.get_type(), p_name, PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NOEDITOR | PROPERTY_USAGE_SCRIPT_VARIABLE));
+		}
+	}
+
+	if (r_valid)
+		*r_valid = false; // Cannot change the value in either case
+}
+
+Variant PlaceHolderScriptInstance::property_get_fallback(const StringName &p_name, bool *r_valid) {
+
+	if (build_failed) {
+		const Map<StringName, Variant>::Element *E = values.find(p_name);
+
+		if (E) {
+			if (r_valid)
+				*r_valid = true;
+			return E->value();
+		}
+	}
+
+	if (r_valid)
+		*r_valid = false;
+
+	return Variant();
+}
+
 PlaceHolderScriptInstance::PlaceHolderScriptInstance(ScriptLanguage *p_language, Ref<Script> p_script, Object *p_owner) :
 		owner(p_owner),
 		language(p_language),
-- 
cgit v1.2.3