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

Add a "keep" import mode to keep files as-is and export them. #47268

Merged
merged 1 commit into from
Mar 22, 2021

Conversation

reduz
Copy link
Member

@reduz reduz commented Mar 22, 2021

Just choose this mode, and the file will not be imported (but it will be exported as-is).

image

Cherry picking for 3.x should e relatively easy.

Fixes #38957.
Fixes #47061.

@reduz reduz requested review from a team as code owners March 22, 2021 19:46
@KoBeWi KoBeWi added this to the 4.0 milestone Mar 22, 2021
@KoBeWi KoBeWi added cherrypick:3.x Considered for cherry-picking into a future 3.x release topic:import and removed topic:editor labels Mar 22, 2021
@akien-mga akien-mga merged commit a7fb5f8 into godotengine:master Mar 22, 2021
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.3. (Via #47300.)

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Mar 24, 2021
akien-mga added a commit that referenced this pull request Mar 24, 2021
Its only purpose was to prevent importing CSV files as translations, but it
would still import them as *nothing*, leading to workflow issues.

This is now properly fixed with #47268 which allows disabling the import for
specific files.

(cherry picked from commit 7ed2220)
@golddotasksquestions
Copy link

I don't understand the nomenclature on this at all. Who keeps the file if I chose "Keep File"? And why is there a "No Import" setting on the Import tab? I literally have to click "reimport" to apply and reimport any of the Settings here. Selecting a "No Import" only then to reimport the file makes no sense to me at all.
Seeing this option I would even have more questions and be hesitant to use it: If Godot has auto imported a CSV and it shows up in the FileSystem, if I select "Keep File (No Import)" and then reimport the file, will it disappear from the FileSystem panel? Do I have to go to the Editor settings and enable "Show hidden Files" in order to reimport it again if I changed my mind?

To a godot beginner user (like a game designer who works a lot with spread sheets), non of this would be intuitive or self explanatory. I'm definitely not a beginner any more and I still don't understand what the expected process now is starting with 3.3 to use CSVs for general purpose data, and not for translations.

Right now in Godot 3.2.3 stable, I can use CSV files without fiddling with the Import tab. I would really like this to stay the same.

@akien-mga
Copy link
Member

akien-mga commented Mar 26, 2021

@golddotasksquestions Godot imports all files by default for which it has a ResourceImporter. CSV is one such file type as CSV files are imported by default as "CSV Translations" (.translation files) which are compressed and optimized translation resources in binary data used by the TranslationServer.

So as of 3.2.3, this is not accurate:

Right now in Godot 3.2.3 stable, I can use CSV files without fiddling with the Import tab. I would really like this to stay the same.

What actually happens when you add a CSV file to a project to use it, not as translation, but as raw data, is that you get a stream of errors each time you focus the editor:

ERROR: import: Error importing CSV translation: 'a' is not a valid locale.
   At: editor/import/resource_importer_csv_translation.cpp:104.
ERROR: _reimport_file: Error importing 'res://test.csv'.
   At: editor/editor_file_system.cpp:1801.

If that doesn't happen for you, it means that you did fiddle with the Import tab to select the "CSV" importer instead of "CSV Translation". That importer was a hack added to prevent trying to import the CSV as translations (but instead it would import them as nothing - leading to various bugs, such as .csv files being missing once the project is exported, since the import system considers the source files unnecessary given that it will only rely on the imported file).

This PR solves this issue by adding a proper "Keep" mode which tells the importer, on a file by file basis, that this file should not be imported by Godot, i.e. it's meant to be used "manually" from your script with the File API.

I appreciate that it might seem confusing to have to go to the "Import" tab to configure the non-import of a given file, but this dock is where the import behavior is configured, and disabling it for a file is part of this configuration.

With godotengine/godot-proposals#2500 you'd be further able to configure in the Project Settings that you want all CSV files to be left alone un-imported, without ever having to go configure it on a file by file basis in the Import dock. But this PR already fixes a completely broken system by making it at least usable.

@golddotasksquestions
Copy link

golddotasksquestions commented Mar 26, 2021

Thank you for taking the time to explain this in such detail Akien, also thanks to reduz for improving this issue, I really do appreciate the better CSV support a lot (I'm sorry I have not said this in my previous comment)

I am pretty certain I never fiddled with CSV import settings. I clearly remember last year after up grading my experience was that it "just worked". That's why I left this comment here.

I think the reason for this is because I never imported the CSV in the first place. I always loaded it from an externally file:

func parse_csv():
	var file = File.new()
	file.open(str(os_dependent_csv_path,"data.csv"), file.READ)
	while !file.eof_reached():
		var csv_rows = file.get_csv_line("\t")
		csv.append(csv_rows)

	file.close()
	csv.pop_back() #remove last empty array get_csv_line() has created 
	headers = Array(csv[0])
	cardnumber_idx = headers.find("cardnumber")
	name_idx = headers.find("name")
	type_idx = headers.find("type")
	csv_noheaders = csv 
	csv_noheaders.pop_front() #remove first array (headers) from the csv

So I guess this totally is a misunderstanding on my part, I am sorry for the confusion.


Having that cleared up, I still think the naming here does not make any sense.
Personally I would really prefer if CSVs would not be imported as translations by default with Godot 4.0 going forward.
CSVs have a much broader use in game design than just translation files. Translations seem like a specific usecase to me, I would therefore prefer to have to option to "reimport as translation" and maybe "reimport as text data" (with text data being the default), rather than "reimport as No Import" or "reimport as Keep File".

geekrelief pushed a commit to geekrelief/godot that referenced this pull request Apr 5, 2021
Its only purpose was to prevent importing CSV files as translations, but it
would still import them as *nothing*, leading to workflow issues.

This is now properly fixed with godotengine#47268 which allows disabling the import for
specific files.

(cherry picked from commit 7ed2220)
lekoder pushed a commit to KoderaSoftwareUnlimited/godot that referenced this pull request Apr 24, 2021
Its only purpose was to prevent importing CSV files as translations, but it
would still import them as *nothing*, leading to workflow issues.

This is now properly fixed with godotengine#47268 which allows disabling the import for
specific files.

(cherry picked from commit 7ed2220)
@Paulb23 Paulb23 mentioned this pull request Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants