Skip to content

Commit

Permalink
Improve script update when (re)loaded.
Browse files Browse the repository at this point in the history
  • Loading branch information
CedNaru committed May 14, 2024
1 parent ea3ef1a commit e1b884c
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 113 deletions.
2 changes: 1 addition & 1 deletion src/jvm_wrapper/bridge/variant_array_bridge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/jvm_wrapper/memory/transfer_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/jvm_wrapper/registration/kt_class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
5 changes: 3 additions & 2 deletions src/language/gdj_language.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,11 @@ void GdjLanguage::init() {

void GdjLanguage::frame() {
#ifdef TOOLS_ENABLED
JvmScriptManager::get_instance().update_all_exports_if_dirty();
JvmScriptManager::get_instance().update_all_scripts();
#endif
}


void GdjLanguage::thread_enter() {
jni::Jvm::attach();
}
Expand Down Expand Up @@ -85,7 +86,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()) {
Expand Down
22 changes: 5 additions & 17 deletions src/resource_format/jvm_resource_format_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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; }
}
Expand Down
10 changes: 2 additions & 8 deletions src/script/jvm_script.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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; }

Expand Down
2 changes: 1 addition & 1 deletion src/script/jvm_script.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
134 changes: 78 additions & 56 deletions src/script/jvm_script_manager.cpp
Original file line number Diff line number Diff line change
@@ -1,122 +1,144 @@

#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.");

Check failure on line 9 in src/script/jvm_script_manager.cpp

View workflow job for this annotation

GitHub Actions / 🪟 Build Windows / Build debug template

'named_user_scripts': undeclared identifier
#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) {
script->update_script();
}
for (const Ref<PathScript>& script : path_user_scripts) {
script->update_exports();
for (const Ref<PathScript>& script : path_scripts) {
script->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;
}

}
Loading

0 comments on commit e1b884c

Please sign in to comment.