From f3c7527225c73812a7de10dfc273ba0e82e8d7ec Mon Sep 17 00:00:00 2001 From: Ignacio Etcheverry 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. --- modules/mono/csharp_script.cpp | 75 +++++++++++++++++++++++++++++++++--------- 1 file changed, 59 insertions(+), 16 deletions(-) (limited to 'modules/mono/csharp_script.cpp') diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp index 7d7028a7a6..cd1a8266ed 100644 --- a/modules/mono/csharp_script.cpp +++ b/modules/mono/csharp_script.cpp @@ -736,6 +736,9 @@ void CSharpLanguage::reload_assemblies_if_needed(bool p_soft_reload) { obj->get_script_instance()->get_property_state(state); map[obj->get_instance_id()] = state; obj->set_script(RefPtr()); + } else { + // no instance found. Let's remove it so we don't loop forever + E->get()->placeholders.erase(E->get()->placeholders.front()->get()); } } @@ -747,8 +750,24 @@ void CSharpLanguage::reload_assemblies_if_needed(bool p_soft_reload) { } } - if (gdmono->reload_scripts_domain() != OK) + if (gdmono->reload_scripts_domain() != OK) { + // Failed to reload the scripts domain + // Make sure to add the scripts back to their owners before returning + for (Map, Map > > >::Element *E = to_reload.front(); E; E = E->next()) { + Ref scr = E->key(); + for (Map > >::Element *F = E->get().front(); F; F = F->next()) { + Object *obj = ObjectDB::get_instance(F->key()); + if (!obj) + continue; + obj->set_script(scr.get_ref_ptr()); + // Save reload state for next time if not saved + if (!scr->pending_reload_state.has(obj->get_instance_id())) { + scr->pending_reload_state[obj->get_instance_id()] = F->get(); + } + } + } return; + } for (Map, Map > > >::Element *E = to_reload.front(); E; E = E->next()) { @@ -778,6 +797,14 @@ void CSharpLanguage::reload_assemblies_if_needed(bool p_soft_reload) { continue; } + if (scr->valid && scr->is_tool() && obj->get_script_instance()->is_placeholder()) { + // Script instance was a placeholder, but now the script was built successfully and is a tool script. + // We have to replace the placeholder with an actual C# script instance. + scr->placeholders.erase(static_cast(obj->get_script_instance())); + ScriptInstance *script_instance = scr->instance_create(obj); + obj->set_script_instance(script_instance); // Not necessary as it's already done in instance_create, but just in case... + } + for (List >::Element *G = F->get().front(); G; G = G->next()) { obj->get_script_instance()->set(G->get().first, G->get().second); } @@ -1474,8 +1501,12 @@ void CSharpScript::_update_exports_values(Map &values, List bool CSharpScript::_update_exports() { #ifdef TOOLS_ENABLED - if (!valid) + if (!valid) { + for (Set::Element *E = placeholders.front(); E; E = E->next()) { + E->get()->set_build_failed(true); + } return false; + } bool changed = false; @@ -1577,6 +1608,7 @@ bool CSharpScript::_update_exports() { _update_exports_values(values, propnames); for (Set::Element *E = placeholders.front(); E; E = E->next()) { + E->get()->set_build_failed(false); E->get()->update(propnames, values); } } @@ -1687,7 +1719,7 @@ bool CSharpScript::_get_member_export(GDMonoClass *p_class, GDMonoClassMember *p MonoObject *attr = p_member->get_attribute(CACHED_CLASS(ExportAttribute)); - PropertyHint hint; + PropertyHint hint = PROPERTY_HINT_NONE; String hint_string; if (variant_type == Variant::NIL) { @@ -1873,7 +1905,11 @@ bool CSharpScript::can_instance() const { } #endif - return valid || (!tool && !ScriptServer::is_scripting_enabled()); +#ifdef TOOLS_ENABLED + return valid && (tool || ScriptServer::is_scripting_enabled()); +#else + return valid; +#endif } StringName CSharpScript::get_instance_base_type() const { @@ -1971,16 +2007,9 @@ Variant CSharpScript::_new(const Variant **p_args, int p_argcount, Variant::Call ScriptInstance *CSharpScript::instance_create(Object *p_this) { - if (!tool && !ScriptServer::is_scripting_enabled()) { -#ifdef TOOLS_ENABLED - PlaceHolderScriptInstance *si = memnew(PlaceHolderScriptInstance(CSharpLanguage::get_singleton(), Ref