Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding a translation CSV results in errors on initial import for many types of resources #93704

Closed
mihe opened this issue Jun 28, 2024 · 5 comments · Fixed by #93919
Closed

Comments

@mihe
Copy link
Contributor

mihe commented Jun 28, 2024

Tested versions

Reproducible in 4.3.beta [811ce36]

System information

Windows 11 (10.0.22631)

Issue description

(I believe this is a duplicate of #92262, but there wasn't much of an MRP provided in that issue.)

When a project includes a "CSV Translation" file, that file will emit .translation files (one for every language/locale) next to the .csv file. The writing of these .translation files will (in the same callstack) seemingly cause every scene in the project to be loaded, which in turn will try to load every resource referenced by those scenes, most of which will (on initial import) fail, since many of these resources won't have been imported at that point.

The type of resource matters to some degree, due to the ordering of the reimports that happens here:

reimport_files.sort();

... as this will sort the imports by the importer name (ResourceImporter::get_importer_name()), where "csv_importer" will often get sorted first, before things like "texture", like in the provided MRP.

Here is the callstack when one of these texture resources fail to load due to not having been imported:

ResourceLoader::_load(const String &p_path, const String &p_original_path, const String &p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error *r_error, bool p_use_sub_threads, float *r_progress) (core/io/resource_loader.cpp:283)
ResourceFormatImporter::load(const String &p_path, const String &p_original_path, Error *r_error, bool p_use_sub_threads, float *r_progress, ResourceFormatLoader::CacheMode p_cache_mode) (core/io/resource_importer.cpp:154)
ResourceLoader::_load(const String &p_path, const String &p_original_path, const String &p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error *r_error, bool p_use_sub_threads, float *r_progress) (core/io/resource_loader.cpp:269)
ResourceLoader::_thread_load_function(void *p_userdata) (core/io/resource_loader.cpp:333)
ResourceLoader::_load_start(const String &p_path, const String &p_type_hint, ResourceLoader::LoadThreadMode p_thread_mode, ResourceFormatLoader::CacheMode p_cache_mode) (core/io/resource_loader.cpp:535)
ResourceLoaderText::load() (scene/resources/resource_format_text.cpp:472)
ResourceFormatLoaderText::load(const String &p_path, const String &p_original_path, Error *r_error, bool p_use_sub_threads, float *r_progress, ResourceFormatLoader::CacheMode p_cache_mode) (scene/resources/resource_format_text.cpp:1392)
ResourceLoader::_load(const String &p_path, const String &p_original_path, const String &p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error *r_error, bool p_use_sub_threads, float *r_progress) (core/io/resource_loader.cpp:269)
ResourceLoader::_thread_load_function(void *p_userdata) (core/io/resource_loader.cpp:333)
ResourceLoader::_load_start(const String &p_path, const String &p_type_hint, ResourceLoader::LoadThreadMode p_thread_mode, ResourceFormatLoader::CacheMode p_cache_mode) (core/io/resource_loader.cpp:535)
ResourceLoader::load(const String &p_path, const String &p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error *r_error) (core/io/resource_loader.cpp:454)
EditorFileSystem::_get_scene_groups(const String &p_path) (editor/editor_file_system.cpp:1939)
EditorFileSystem::_update_scene_groups() (editor/editor_file_system.cpp:1895)
EditorFileSystem::_update_pending_scene_groups() (editor/editor_file_system.cpp:1914)
EditorFileSystem::_process_update_pending() (editor/editor_file_system.cpp:1854)
EditorFileSystem::update_files(const Vector<String> &p_script_paths) (editor/editor_file_system.cpp:2076)
EditorFileSystem::update_file(const String &p_file) (editor/editor_file_system.cpp:1946)
EditorNode::_resource_saved(Ref<Resource> p_resource, const String &p_path) (editor/editor_node.cpp:6261)
ResourceSaver::save(const Ref<Resource> &p_resource, const String &p_path, unsigned int p_flags) (core/io/resource_saver.cpp:149)
ResourceImporterCSVTranslation::import(const String &p_source_file, const String &p_save_path, const HashMap<StringName,Variant,HashMapHasherDefault,HashMapComparatorDefault<StringName>,DefaultTypedAllocator<HashMapElement<StringName,Variant>>> &p_options, List<String,DefaultAllocator> *r_platform_variants, List<String,DefaultAllocator> *r_gen_files, Variant *r_metadata) (editor/import/resource_importer_csv_translation.cpp:146)
EditorFileSystem::_reimport_file(const String &p_file, const HashMap<StringName,Variant,HashMapHasherDefault,HashMapComparatorDefault<StringName>,DefaultTypedAllocator<HashMapElement<StringName,Variant>>> &p_custom_options, const String &p_custom_importer, Variant *p_generator_parameters) (editor/editor_file_system.cpp:2434)
EditorFileSystem::reimport_files(const Vector<String> &p_files) (editor/editor_file_system.cpp:2716)
EditorFileSystem::_update_scan_actions() (editor/editor_file_system.cpp:779)
EditorFileSystem::_notification(int p_what) (editor/editor_file_system.cpp:1465)
EditorFileSystem::_notificationv(int p_notification, bool p_reversed) (editor/editor_file_system.h:140)
Object::notification(int p_notification, bool p_reversed) (core/object/object.cpp:873)
SceneTree::_process_group(SceneTree::ProcessGroup *p_group, bool p_physics) (scene/main/scene_tree.cpp:965)
SceneTree::_process(bool p_physics) (scene/main/scene_tree.cpp:1042)
SceneTree::process(double p_time) (scene/main/scene_tree.cpp:528)
Main::iteration() (main/main.cpp:4099)

(Note that I disabled import/use_multiple_threads in the MRP to make debugging more straight-forward, hence the callstack, but that setting is inconsequential to this issue.)

As far as I can tell (after bisecting) this is a regression in 4.3, caused by #60965, as EditorFileSystem::_update_pending_scene_groups loading all the scenes in EditorFileSystem::_process_update_pending is largely what's causing this problem.

However, it does seem to me like the CSV importer itself is also part of the problem here, as every other import artifact that triggers EditorNode::_resource_saved will simply early out in EditorFileSystem::_find_file here due to those import artifacts being in the .godot folder, as opposed to being generated next to the source file like with .csv files.

Steps to reproduce

  1. Ensure there is no existing .godot folder in the MRP
  2. Open the MRP in the editor
  3. Note errors about failing to open/load logo1.png, logo2.png and their respective import artifacts
  4. Close the project
  5. Open the project again
  6. Note the lack of errors

Minimal reproduction project (MRP)

LocalizationErrors.zip

@akien-mga akien-mga added this to the 4.3 milestone Jun 28, 2024
@akien-mga akien-mga moved this from Unassessed to Release Blocker in 4.x Release Blockers Jun 28, 2024
@KoBeWi
Copy link
Member

KoBeWi commented Jun 28, 2024

Wasn't #92320 supposed to fix this?

@mihe
Copy link
Contributor Author

mihe commented Jun 28, 2024

Wasn't #92320 supposed to fix this?

What #92320 fixes might still be good/relevant, but it does not address this issue, nor did it address the issue that it claimed to fix. The MRP in #83200 is still broken, and was still broken when #92320 was (inadvertently) merged, at 74f9f12.

@mihe
Copy link
Contributor Author

mihe commented Jun 28, 2024

I'm not entirely sure of the consequences of this, but if a quick hack is needed, this might do the trick:

-	if (!f.begins_with("res://")) {
+	if (!f.begins_with("res://") || f.ends_with(".translation")) {
 		return false;
 	}

... here:

if (!f.begins_with("res://")) {
return false;
}

@Hilderin
Copy link
Contributor

I don't think it's the same issue as #92320. In fact, the issue is probably caused became csv are now importing at startup. In #93064 I added a check if a scene was modified and not first_scan in update_files before calling _update_pending_scene_groups to prevent this bug. I don't have access to my computer until next week to check this issue unfortunaly.

@jackcasey
Copy link

👍 I can confirm this is what has been plaguing me for weeks. Deleted the translation files to test and now it's fine again.

I am seeing it in 4.2.1 and 4.2.2 as well though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Release Blocker
5 participants