diff --git a/modules/SCsub b/modules/SCsub index 69dfadfa311..e987dde0e5c 100644 --- a/modules/SCsub +++ b/modules/SCsub @@ -52,6 +52,7 @@ for name, path in env.module_list.items(): # Generate header to be included in `tests/test_main.cpp` to run module-specific tests. if env["tests"]: + env.Append(CPPDEFINES=["TESTS_ENABLED"]) env.CommandNoCache("modules_tests.gen.h", test_headers, env.Run(modules_builders.modules_tests_builder)) # libmodules.a with only register_module_types. diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp index 9ebcb2b9602..2be563ed04e 100644 --- a/modules/gdscript/gdscript.cpp +++ b/modules/gdscript/gdscript.cpp @@ -1074,6 +1074,12 @@ void GDScript::_bind_methods() { ClassDB::bind_vararg_method(METHOD_FLAGS_DEFAULT, "new", &GDScript::_new, MethodInfo("new")); } +void GDScript::set_path_cache(const String &p_path) { + if (is_root_script()) { + Script::set_path_cache(p_path); + } +} + void GDScript::set_path(const String &p_path, bool p_take_over) { if (is_root_script()) { Script::set_path(p_path, p_take_over); @@ -3041,7 +3047,11 @@ Ref GDScriptLanguage::get_script_by_fully_qualified_name(const String Ref ResourceFormatLoaderGDScript::load(const String &p_path, const String &p_original_path, Error *r_error, bool p_use_sub_threads, float *r_progress, CacheMode p_cache_mode) { Error err; bool ignoring = p_cache_mode == CACHE_MODE_IGNORE || p_cache_mode == CACHE_MODE_IGNORE_DEEP; - Ref scr = GDScriptCache::get_full_script(p_original_path, err, "", ignoring); + Ref scr = GDScriptCache::get_full_script_no_resource_cache(p_original_path, err, "", ignoring); + // Reset `path_cache` so that when resource loader uses `set_path()` later, the script gets added to the cache. + if (scr.is_valid()) { + scr->set_path_cache(String()); + } if (err && scr.is_valid()) { // If !scr.is_valid(), the error was likely from scr->load_source_code(), which already generates an error. diff --git a/modules/gdscript/gdscript.h b/modules/gdscript/gdscript.h index 25496ea72cc..a9d9c867f2f 100644 --- a/modules/gdscript/gdscript.h +++ b/modules/gdscript/gdscript.h @@ -310,6 +310,7 @@ public: virtual Error reload(bool p_keep_state = false) override; + virtual void set_path_cache(const String &p_path) override; virtual void set_path(const String &p_path, bool p_take_over = false) override; String get_script_path() const; Error load_source_code(const String &p_path); diff --git a/modules/gdscript/gdscript_cache.cpp b/modules/gdscript/gdscript_cache.cpp index 3cbd817f993..e57376dce2e 100644 --- a/modules/gdscript/gdscript_cache.cpp +++ b/modules/gdscript/gdscript_cache.cpp @@ -212,7 +212,7 @@ void GDScriptCache::remove_script(const String &p_path) { Ref GDScriptCache::get_parser(const String &p_path, GDScriptParserRef::Status p_status, Error &r_error, const String &p_owner) { MutexLock lock(singleton->mutex); Ref ref; - if (!p_owner.is_empty()) { + if (!p_owner.is_empty() && p_path != p_owner) { singleton->dependencies[p_owner].insert(p_path); singleton->parser_inverse_dependencies[p_path].insert(p_owner); } @@ -298,7 +298,7 @@ Vector GDScriptCache::get_binary_tokens(const String &p_path) { Ref GDScriptCache::get_shallow_script(const String &p_path, Error &r_error, const String &p_owner) { MutexLock lock(singleton->mutex); - if (!p_owner.is_empty()) { + if (!p_owner.is_empty() && p_path != p_owner) { singleton->dependencies[p_owner].insert(p_path); } if (singleton->full_gdscript_cache.has(p_path)) { @@ -312,7 +312,8 @@ Ref GDScriptCache::get_shallow_script(const String &p_path, Error &r_e Ref script; script.instantiate(); - script->set_path(p_path, true); + + script->set_path_cache(p_path); if (remapped_path.has_extension("gdc")) { Vector buffer = get_binary_tokens(remapped_path); if (buffer.is_empty()) { @@ -338,9 +339,20 @@ Ref GDScriptCache::get_shallow_script(const String &p_path, Error &r_e } Ref GDScriptCache::get_full_script(const String &p_path, Error &r_error, const String &p_owner, bool p_update_from_disk) { + Ref uncached_script = get_full_script_no_resource_cache(p_path, r_error, p_owner, p_update_from_disk); + + // Resources don't know whether they are cached, so using `set_path()` after `set_path_cache()` does not add the resource to the cache if the path is the same. + // We reset the cached path from `get_shallow_script()` so that the subsequent call to `set_path()` caches everything correctly. + uncached_script->set_path_cache(String()); + uncached_script->set_path(p_path, true); + + return uncached_script; +} + +Ref GDScriptCache::get_full_script_no_resource_cache(const String &p_path, Error &r_error, const String &p_owner, bool p_update_from_disk) { MutexLock lock(singleton->mutex); - if (!p_owner.is_empty()) { + if (!p_owner.is_empty() && p_path != p_owner) { singleton->dependencies[p_owner].insert(p_path); } diff --git a/modules/gdscript/gdscript_cache.h b/modules/gdscript/gdscript_cache.h index 3930bfebc33..fb757fcb4f2 100644 --- a/modules/gdscript/gdscript_cache.h +++ b/modules/gdscript/gdscript_cache.h @@ -78,6 +78,12 @@ public: ~GDScriptParserRef(); }; +#ifdef TESTS_ENABLED +namespace GDScriptTests { +class TestGDScriptCacheAccessor; +} +#endif // TESTS_ENABLED + class GDScriptCache { // String key is full path. HashMap parser_map; @@ -91,6 +97,9 @@ class GDScriptCache { friend class GDScript; friend class GDScriptParserRef; friend class GDScriptInstance; +#ifdef TESTS_ENABLED + friend class GDScriptTests::TestGDScriptCacheAccessor; +#endif // TESTS_ENABLED static GDScriptCache *singleton; @@ -112,6 +121,19 @@ public: static String get_source_code(const String &p_path); static Vector get_binary_tokens(const String &p_path); static Ref get_shallow_script(const String &p_path, Error &r_error, const String &p_owner = String()); + /** + * Returns a fully loaded GDScript using an already cached script if one exists. + * + * If a new script is created, the resource will only have its patch_cache set, so it won't be present in the ResourceCache. + * Mismatches between GDScriptCache and ResourceCache might trigger complex issues so when using this method ensure + * that the script is added to the resource cache or removed from the GDScript cache. + */ + static Ref get_full_script_no_resource_cache(const String &p_path, Error &r_error, const String &p_owner = String(), bool p_update_from_disk = false); + /** + * Returns a fully loaded GDScript using an already cached script if one exists. + * + * The returned instance is present in GDScriptCache and ResourceCache. + */ static Ref get_full_script(const String &p_path, Error &r_error, const String &p_owner = String(), bool p_update_from_disk = false); static Ref get_cached_script(const String &p_path); static Error finish_compiling(const String &p_owner); diff --git a/modules/gdscript/tests/gdscript_test_runner_suite.h b/modules/gdscript/tests/gdscript_test_runner_suite.h index ed9f24f6c5e..2eba3d9c18d 100644 --- a/modules/gdscript/tests/gdscript_test_runner_suite.h +++ b/modules/gdscript/tests/gdscript_test_runner_suite.h @@ -32,10 +32,23 @@ #include "gdscript_test_runner.h" +#include "modules/gdscript/gdscript_cache.h" #include "tests/test_macros.h" +#include "tests/test_utils.h" namespace GDScriptTests { +class TestGDScriptCacheAccessor { +public: + static bool has_shallow(String p_path) { + return GDScriptCache::singleton->shallow_gdscript_cache.has(p_path); + } + + static bool has_full(String p_path) { + return GDScriptCache::singleton->full_gdscript_cache.has(p_path); + } +}; + // TODO: Handle some cases failing on release builds. See: https://github.com/godotengine/godot/pull/88452 #ifdef TOOLS_ENABLED TEST_SUITE("[Modules][GDScript]") { @@ -72,6 +85,26 @@ func _init(): CHECK_MESSAGE(int(ref_counted->get_meta("result")) == 42, "The script should assign object metadata successfully."); } +TEST_CASE("[Modules][GDScript] Loading keeps ResourceCache and GDScriptCache in sync") { + const String path = TestUtils::get_temp_path("gdscript_load_test.gd"); + + { + Ref fa = FileAccess::open(path, FileAccess::ModeFlags::WRITE); + fa->store_string("extends Node\n"); + fa->close(); + } + + CHECK(!ResourceCache::has(path)); + CHECK(!TestGDScriptCacheAccessor::has_shallow(path)); + CHECK(!TestGDScriptCacheAccessor::has_full(path)); + + Ref loaded = ResourceLoader::load(path); + + CHECK(ResourceCache::has(path)); + CHECK(!TestGDScriptCacheAccessor::has_shallow(path)); + CHECK(TestGDScriptCacheAccessor::has_full(path)); +} + TEST_CASE("[Modules][GDScript] Validate built-in API") { GDScriptLanguage *lang = GDScriptLanguage::get_singleton();