Fix PathScript caching.

src/jvm_wrapper/bridge/variant_array_bridge.cpp

@@ -23,7 +23,7 @@ uintptr_t VariantArrayBridge::engine_call_constructor_typed(JNIEnv* p_raw_env, j
     StringName base_class_name;
     Variant script;
     if (userTypeIndex != -1) {
-        const Ref<JvmScript>& kotlin_script {JvmScriptManager::get_instance().get_user_script_for_index(userTypeIndex)};
+        const Ref<JvmScript>& kotlin_script {JvmScriptManager::get_instance().get_named_script_for_index(userTypeIndex)};
         base_class_name = kotlin_script->get_instance_base_type();
         script = kotlin_script;
     } else if (engineTypeIndex != -1) {

src/jvm_wrapper/memory/transfer_context.cpp

@@ -133,7 +133,7 @@ void TransferContext::create_native_object(JNIEnv* p_raw_env, jobject p_instance
     int script_index {static_cast<int>(p_script_index)};
     if (script_index != -1) {
         KtObject* kt_object = memnew(KtObject(env, jni::JObject(p_object), ptr->is_ref_counted()));
-        Ref<JvmScript> kotlin_script {JvmScriptManager::get_instance().get_user_script_for_index(script_index)};
+        Ref<JvmScript> kotlin_script {JvmScriptManager::get_instance().get_named_script_for_index(script_index)};
         JvmInstance* script = memnew(JvmInstance(env, ptr, kt_object, kotlin_script.ptr()));
         ptr->set_script_instance(script);
     }

src/jvm_wrapper/registration/kt_class.cpp

@@ -41,7 +41,7 @@ KtObject* KtClass::create_instance(jni::Env& env, const Variant** p_args, int p_
 #endif
 
     KtObject* jvm_instance {constructor->create_instance(env, p_args, p_owner)};
-    LOG_DEV_VERBOSE(vformat("Instantiated an object with resource path %s", registered_class_name));
+    LOG_DEV_VERBOSE(vformat("Instantiated a Jvm script: %s", registered_class_name));
 
     return jvm_instance;
 }

src/language/gdj_language.cpp

@@ -35,12 +35,6 @@ void GdjLanguage::init() {
 #endif
 }
 
-void GdjLanguage::frame() {
-#ifdef TOOLS_ENABLED
-    JvmScriptManager::get_instance().update_all_exports_if_dirty();
-#endif
-}
-
 void GdjLanguage::thread_enter() {
     jni::Jvm::attach();
 }
@@ -85,7 +79,7 @@ String GdjLanguage::get_global_class_name(const String& p_path, String* r_base_t
     if (p_path.begins_with(ENTRY_DIRECTORY) || !p_path.ends_with(GODOT_JVM_REGISTRATION_FILE_EXTENSION)) { return {}; }
 
     String script_name = JvmScript::get_script_file_name(p_path);
-    Ref<NamedScript> named_script = JvmScriptManager::get_instance().get_user_script_from_name(script_name);
+    Ref<NamedScript> named_script = JvmScriptManager::get_instance().get_script_from_name(script_name);
     if (!named_script.is_null() && named_script.is_valid()) {
         if (r_base_type) {
             if (named_script->get_base_script().is_null()) {

src/language/gdj_language.h

@@ -13,7 +13,6 @@ class GdjLanguage : public JvmLanguage {
     static GdjLanguage* get_instance();
 
     void init() override;
-    void frame() override;
 
     void thread_enter() override;
     void thread_exit() override;

src/resource_format/jvm_resource_format_loader.cpp

@@ -57,23 +57,11 @@ Ref<Resource> JvmResourceFormatLoader::load(const String& p_path, const String&
 
     String extension = p_path.get_extension();
     if (extension == GODOT_JVM_REGISTRATION_FILE_EXTENSION) {
-        // We don't import Kotlin scripts so p_path == p_original_path
-        String script_name = JvmScript::get_script_file_name(p_path);
-        ref = JvmScriptManager::get_instance().get_user_script_from_name(script_name);
-        if (ref.is_null()) {
-#ifdef TOOLS_ENABLED
-            // If we reach that location, it means that the script file being loaded hasn't been built into the .jar.
-            // We create a script placeholder instead. When reloading, it will be properly updated with the correct KtClass.
-            ref = JvmScriptManager::get_instance().create_script<GdjScript>(p_path);
-#endif
-        }
-    }
-    // Path scripts are always created from the resource_loader and set in the resource cache afterward.
-    // If we reach that location, it means the script doesn't exist.
-    else if (extension == GODOT_KOTLIN_SCRIPT_EXTENSION) {
-        ref = JvmScriptManager::get_instance().create_script<KotlinScript>(p_path);
+        ref = JvmScriptManager::get_instance().get_or_create_script<GdjScript>(p_path);
+    } else if (extension == GODOT_KOTLIN_SCRIPT_EXTENSION) {
+        ref = JvmScriptManager::get_instance().get_or_create_script<KotlinScript>(p_path);
     } else if (extension == GODOT_JAVA_SCRIPT_EXTENSION) {
-        ref = JvmScriptManager::get_instance().create_script<JavaScript>(p_path);
+        ref = JvmScriptManager::get_instance().get_or_create_script<JavaScript>(p_path);
     }
 
     if (ref.is_valid()) {
@@ -82,8 +70,8 @@ Ref<Resource> JvmResourceFormatLoader::load(const String& p_path, const String&
         Error load_err {read_all_file_utf8(p_path, source_code)};
         if (r_error) { *r_error = load_err; }
         ref->set_source_code(source_code);
+        ref->update_script();
 #endif
-        ref->set_path(p_path, true);
     } else {
         if (r_error) { *r_error = Error::ERR_UNAVAILABLE; }
     }

src/script/jvm_script.cpp

@@ -59,7 +59,7 @@ bool JvmScript::inherits_script(const Ref<Script>& p_script) const {
 Ref<Script> JvmScript::get_base_script() const {
     if (!is_valid() || kotlin_class->registered_supertypes.size() == 0) { return {}; }
     StringName parent_name = kotlin_class->registered_supertypes[0];
-    return JvmScriptManager::get_instance().get_user_script_from_name(parent_name);
+    return JvmScriptManager::get_instance().get_script_from_name(parent_name);
 }
 
 StringName JvmScript::get_instance_base_type() const {
@@ -220,13 +220,7 @@ PlaceHolderScriptInstance* JvmScript::placeholder_instance_create(Object* p_this
     return placeholder;
 }
 
-void JvmScript::update_exports() {
-    // TODO: Remove this when multithreading is fixed.
-    if (!Thread::is_main_thread()) {
-        MessageQueue::get_singleton()->push_callable(callable_mp(this, &JvmScript::update_exports));
-        return;
-    }
-
+void JvmScript::update_script() {
     exported_members_default_value_cache.clear();
     if (!is_valid()) { return; }
 

src/script/jvm_script.h

@@ -63,7 +63,7 @@ class JvmScript : public Script {
 
 public:
     PlaceHolderScriptInstance* placeholder_instance_create(Object* p_this) override;
-    void update_exports() override;
+    void update_script();
     Vector<DocData::ClassDoc> get_documentation() const override;
     PropertyInfo get_class_category() const override;
     String get_class_icon_path() const override;

src/script/jvm_script_manager.cpp

@@ -1,122 +1,148 @@
 
 #include "jvm_script_manager.h"
 
+#include "lifecycle/paths.h"
 #include "script/language/gdj_script.h"
 
 void JvmScriptManager::create_and_update_scripts(Vector<KtClass*>& classes) {
-    LOG_DEV("Loading JVM Scripts...");
-
 #if defined(DEBUG_ENABLED) && !defined(TOOLS_ENABLED)
-    JVM_ERR_FAIL_COND_MSG(named_user_scripts.size() != 0, "JVM scripts are being initialized more than once.");
+    JVM_ERR_FAIL_COND_MSG(named_scripts.size() != 0, "JVM scripts are being initialized more than once.");
 #endif
 
-    Vector<Ref<NamedScript>> scripts;
-    // We first deal with named scripts.
-
 #ifdef TOOLS_ENABLED
     // Clear all containers and keeping a cache for comparison.
-    HashMap<StringName, Ref<NamedScript>> script_cache = named_user_scripts_map;
-    named_user_scripts.clear();
-    named_user_scripts_map.clear();
+    HashMap<StringName, Ref<NamedScript>> named_script_cache = named_scripts_map;
+    named_scripts.clear();
+    named_scripts_map.clear();
+
     filepath_to_name_map.clear();
+
+    Vector<Ref<PathScript>> path_script_cache = path_scripts;
+    path_scripts.clear();
+    path_scripts_map.clear();
 #endif
 
+    LOG_DEV("Loading JVM Scripts...");
+
+    // Named Script
     for (KtClass* kotlin_class : classes) {
         String script_name = kotlin_class->registered_class_name;
-        Ref<GdjScript> script;
+
+        Ref<GdjScript> named_script;
 #ifdef TOOLS_ENABLED
         // First check if the scripts already exist
-        if (script_cache.has(script_name)) {
-            script = script_cache[script_name];
-            delete script->kotlin_class;
-            script->kotlin_class = kotlin_class;
-            script_cache.erase(script_name);
+        if (named_script_cache.has(script_name)) {
+            named_script = named_script_cache[script_name];
+
+            delete named_script->kotlin_class;
+            named_script->kotlin_class = kotlin_class;
+
+            named_script_cache.erase(script_name);
+
             LOG_DEV_VERBOSE(vformat("JVM Script updated: %s", script_name));
         } else {
 #endif
-            script.instantiate();
-            script->kotlin_class = kotlin_class;
+            named_script.instantiate();
+            named_script->kotlin_class = kotlin_class;
+
             LOG_DEV_VERBOSE(vformat("JVM Script created: %s", script_name));
 #ifdef TOOLS_ENABLED
         }
 #endif
-        scripts.push_back(Ref(script));
-    }
-
-    for (const Ref<NamedScript>& script : scripts) {
-        named_user_scripts.push_back(script);
-        named_user_scripts_map[script->get_global_name()] = script;
-
-        String source_path = "res://" + script->kotlin_class->relative_source_path;
+        // Add mapping from path to name for PathScripts.
+        String source_path = RES_DIRECTORY + kotlin_class->relative_source_path;
         if (FileAccess::exists(source_path)) {
-            filepath_to_name_map[source_path] = script->kotlin_class->registered_class_name;
+            filepath_to_name_map[source_path] = kotlin_class->registered_class_name;
         }
+
+        named_scripts.push_back(named_script);
+        named_scripts_map[script_name] = named_script;
     }
 
 #ifdef TOOLS_ENABLED
     // Only scripts left in the cache are the ones that have been removed or placeholders without associated KtClass
-    // We simply delete their kotlin_class if they got one
-    for (const KeyValue<StringName, Ref<NamedScript>>& keyValue : script_cache) {
-        Ref<JvmScript> ref = keyValue.value;
-        if (ref->kotlin_class) {
-            LOG_DEV_VERBOSE(vformat("JVM Script deleted: %s", ref->kotlin_class->registered_class_name));
-            delete ref->kotlin_class;
-            ref->kotlin_class = nullptr;
+    // We simply remove their kotlin_class if they got one.
+    for (const KeyValue<StringName, Ref<NamedScript>>& keyValue : named_script_cache) {
+        Ref<NamedScript> script {keyValue.value};
+        StringName name {keyValue.key};
+        if (script->kotlin_class) {
+            LOG_DEV_VERBOSE(vformat("JVM Script deleted: %s", script->kotlin_class->registered_class_name));
+            delete script->kotlin_class;
+            script->kotlin_class = nullptr;
         }
 
-        // We only add them back if they are in use, otherwise we let the Script die.
-        if (!ref->placeholders.is_empty()) { scripts.push_back(ref); }
+        // We only add them back if placeholders are in use in the editor. That way they can be updated if back in the next reload.
+        // Without that a separate Script instance would be created and nodes not updated.
+        // Otherwise, we let the script die.
+        if (!script->placeholders.is_empty()) {
+            named_scripts.push_back(script);
+            named_scripts_map[name] = script;
+        }
     }
 
     // Now we deal with path script reloading.
-    Vector<Ref<PathScript>> path_script_cache = path_user_scripts;
-    path_user_scripts.clear();
     for (Ref<PathScript>& script : path_script_cache) {
         String path = script->get_path();
-        // No need to delete the KotlinClass, it has already been done with the namedScript that shares it.
+        // No need to delete the KotlinClass, it has already been done with the NamedScript that shares it.
         script->kotlin_class = nullptr;
         if (filepath_to_name_map.has(path)) {
-            script->kotlin_class = named_user_scripts_map[filepath_to_name_map[path]]->kotlin_class;
-            path_user_scripts.push_back(script);
+            script->kotlin_class = named_scripts_map[filepath_to_name_map[path]]->kotlin_class;
+
+        } else if (script->placeholders.is_empty()) {
+            continue;
         }
+        // Only scripts used in placeholder or with a mapping to a Named Script are kept.
+        path_scripts.push_back(script);
+        path_scripts_map[path] = script;
     }
+
+    update_all_scripts();
 #endif
 }
 
-const Ref<NamedScript>& JvmScriptManager::get_user_script_for_index(int p_index) const {
+const Ref<NamedScript>& JvmScriptManager::get_named_script_for_index(int p_index) const {
     // No check. Meant to be a fast operation
-    return named_user_scripts[p_index];
+    return named_scripts[p_index];
 }
 
-Ref<NamedScript> JvmScriptManager::get_user_script_from_name(const StringName& name) const {
-    if (HashMap<StringName, Ref<NamedScript>>::ConstIterator element = named_user_scripts_map.find(name)) {
+Ref<NamedScript> JvmScriptManager::get_script_from_name(const StringName& name) const {
+    if (HashMap<StringName, Ref<NamedScript>>::ConstIterator element = named_scripts_map.find(name)) {
         return element->value;
     }
-    return Ref<JvmScript>();
+    return {};
+}
+
+Ref<PathScript> JvmScriptManager::get_script_from_path(const String& p_path) const {
+    if (HashMap<String, Ref<PathScript>>::ConstIterator element = path_scripts_map.find(p_path)) {
+        return element->value;
+    }
+    return {};
 }
 
 #ifdef TOOLS_ENABLED
-void JvmScriptManager::update_all_exports_if_dirty() {
-    if (!scripts_dirty) return;
-    for (const Ref<NamedScript>& script : named_user_scripts) {
-        script->update_exports();
+void JvmScriptManager::update_all_scripts() {
+    for (const Ref<NamedScript>& script : named_scripts) {
+        // We have to delay the update_export. The engine is not fully initialized and scripts can cause undefined behaviors.
+        JvmScript* ptr = script.ptr();
+        MessageQueue::get_singleton()->push_callable(callable_mp(ptr, &JvmScript::update_script));
     }
-    for (const Ref<PathScript>& script : path_user_scripts) {
-        script->update_exports();
+    for (const Ref<PathScript>& script : path_scripts) {
+        // We have to delay the update_export. The engine is not fully initialized and scripts can cause undefined behaviors.
+        JvmScript* ptr = script.ptr();
+        MessageQueue::get_singleton()->push_callable(callable_mp(ptr, &JvmScript::update_script));
     }
-    scripts_dirty = false;
 }
 #endif
 
 void JvmScriptManager::clear() {
-    named_user_scripts.clear();
-    named_user_scripts_map.clear();
-    path_user_scripts.clear();
+    named_scripts.clear();
+    named_scripts_map.clear();
     filepath_to_name_map.clear();
+    path_scripts.clear();
+    path_scripts_map.clear();
 }
 
 JvmScriptManager& JvmScriptManager::get_instance() {
     static JvmScriptManager instance;
     return instance;
-}
-
+}