godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.73k stars 20.84k forks source link

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

Closed mihe closed 3 months ago

mihe commented 3 months ago

Tested versions

Reproducible in 4.3.beta [811ce36c6]

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:

https://github.com/godotengine/godot/blob/811ce36c6090013f1a48676c0388892bf1621288/editor/editor_file_system.cpp#L2660

... 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

KoBeWi commented 3 months ago

Wasn't #92320 supposed to fix this?

mihe commented 3 months ago

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 74f9f12c71c0b1cd66481cdc8b3b3432b7c541a6.

mihe commented 3 months ago

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:

https://github.com/godotengine/godot/blob/811ce36c6090013f1a48676c0388892bf1621288/editor/editor_file_system.cpp#L1545-L1547

Hilderin commented 3 months ago

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 commented 3 months ago

:+1: 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.