godotengine / godot

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

CSV importer works only with comma delimiter #82002

Open warkenji opened 11 months ago

warkenji commented 11 months ago

Godot version

4.1.1.stable

System information

Godot v4.1.1.stable.mono - Pop!_OS 22.04 LTS - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3080 Laptop GPU () - AMD Ryzen 9 5900HX with Radeon Graphics (16 Threads)

Issue description

When a csv translation file doesn't use the comma as a delimiter, the file is not imported.

Steps to reproduce

Minimal reproduction project

Translation Example.zip

adeneve commented 11 months ago

Hi @warkenji it looks like if you provide an import file (file_with_semicolon.csv.import) with the following information: [params] compress=true delimiter=1

it will tell godot to use semi-colons as the delimiter for that file. I tested it and it seems to work, but it does require you to manually add that file yourself. Could this be a possible work-around? It looks like the default delimiter is a comma unless specified otherwise.

Aizakkuz commented 11 months ago

Hey, so I have not used Godot dealing with translations. However, I have dealt with writing things responsible for importing CSV files onto databases at scale professionally, and from what is outlined in your issue it seems you unfortunately did not choose the correct delimiter with Godot.

https://docs.godotengine.org/en/stable/tutorials/assets_pipeline/importing_translations.html#:~:text=%E8%A8%80%E3%81%84%E3%81%BE%E3%81%97%E3%81%9F-,CSV%20importer,(or%20the%20project%20settings).

image image

Let me know if this helps, and hopefully I am not misunderstanding!

warkenji commented 11 months ago

When I tried to import with:

akien-mga commented 11 months ago

Indeed selection the file_with_semicolon.csv does not show the Import details for it, so it the delimiter can't be changed.

NolanDC commented 11 months ago

I looked into the CSV import code to see if I could figure out what was going on. The problem is that files with recognized extensions are automatically imported, which the CSV importer attempts to load using the default comma delimiter. For CSVs without commas, the first line will only have a single entry, thus failing the condition in resource_importer_csv_translation.cpp.

Some possible solutions for this:

  1. Attempt to auto-detect the delimiter. This has a ton of edge-cases, and could lead to even more problems in cases where it doesn't work. On second thought, since the first line of a valid translation CSV has such strict formatting, it might be straightforward to auto-detect the delimiter. However, this should still be combined with fix 4 below so that non-translation CSV's can be successfully imported. Otherwise, users won't be able to select the "keep" option.

  2. Default to “keep” mode for certain filetypes such as CSV, so that import options can be set before the import logic runs. This is not easily done and would be a bit messy.

  3. Proper handling of “raw” imported files using a new importer. I think that the “keep” logic could be generalized into a ResourceImporterRaw that would essentially import files but keep them as-is. With a little fiddling, this could be set as the default importer for some file types such as CSV so that it’d be imported as raw, and then the import options could be set before the logic runs. This is more “correct” than option 1 IMO, but is also a bit complex. I found a few PRs/Issues that discuss the handling of raw files (mostly CSVs), but I think more discussion is necessary since it’d require extending the ResourceImporter class, and refactoring the logic that selects available/default importers.

  4. Relax the parsing rules around CSVs to warn of improper formatting rather than failing the import. I have a branch up for this: fix_csv_import_for_non_comma_separated_files and can make a PR, but wanted to make sure this seems like a reasonable solution first. I think this is fairly clean, and also gives better feedback to devs dealing with malformed translation files. This solution allows the import to succeed even if the content is malformed by skipping files with no translation keys, and skipping lines where the number of provided translations does not match the number of expected locales. Since the import succeeds, the user can then update the delimiter and re-import, at which point the translation files will be created.

    One downside to this solution is that it does require that devs watch the errors a little more closely, because a successful import no longer means that the file was properly formed. I think this is acceptable, since it’s reasonable to expect devs to test the translations in their game anyway.

Calinou commented 7 months ago
golddotasksquestions commented 7 months ago

Default to “keep” mode for certain filetypes such as CSV, so that import options can be set before the import logic runs.

Proper handling of “raw” imported files using a new importer. I think that the “keep” logic could be generalized into a ResourceImporterRaw that would essentially import files but keep them as-is.

This should have been the default for a long time. CSVs are not used for translation only in game dev. We use it for all kinds of game mechanic balancing and data management. In every game company I worked, designers used CSV for these purposes (yes, also translation).

Add to that often games that use CSV with translation also use Dialog. Dialog means you'll have comma in your data. Which means you have to use a different delimiter.

40-4 commented 4 months ago

Unfortunately this still is the issue. There is a workaround of simply changing the delimeter for one import, and then reimporting with correct delimeter, however it is pretty janky. Hope it will be resolved soon.