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

Move generated translation files into res://.godot/translations #42392

Closed

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Sep 28, 2020

Follow up to #38607. Implements and closes godotengine/godot-proposals#1812.

This moves the generated .translation files into a subfolder of res://.godot called res://.godot/translations (defined as a constant: TranslationServer::TRANSLATION_FILES_PATH). These files are generated and don't need to be committed, which is the goal of the res://.godot folder.

Note that projects need to be manually updated, since project.godot explicitly references the translation files. For example, the translation demo contains res://text.en.translation in project.godot.

The simplest solution was to make the contents of this folder replicate the folder structure of the project, so a file res://subfolder/text.csv gets imported as res://.godot/translations/subfolder/text.en.translation etc. Because the generated translation files have to be referenced explicitly, I think this is actually really nice.

Another question is what to call this folder. I think "translation" is nice because it contains .translation files, but one could make the argument that this folder should be called "translations" instead.

@Calinou

This comment has been minimized.

@aaronfranke aaronfranke changed the title Move generated translation files into res://.godot/translation Move generated translation files into res://.godot/translations Nov 12, 2020
@aaronfranke aaronfranke force-pushed the translation-dotgodot branch from c999894 to 79592a6 Compare January 9, 2021 03:29
@aaronfranke aaronfranke requested review from a team as code owners February 25, 2021 02:42
@aaronfranke aaronfranke force-pushed the translation-dotgodot branch from 8fbc33b to 5b1331c Compare March 19, 2021 08:57
@aaronfranke aaronfranke force-pushed the translation-dotgodot branch from 5b1331c to f7b6fb3 Compare April 7, 2021 03:17
@aaronfranke aaronfranke force-pushed the translation-dotgodot branch 2 times, most recently from f7345af to 005a99b Compare May 8, 2021 08:25
@aaronfranke aaronfranke force-pushed the translation-dotgodot branch from 005a99b to 1666709 Compare May 24, 2021 07:57
@aaronfranke aaronfranke changed the title Move generated translation files into res://.godot/translations Move generated translation files into res://.godot/translations May 24, 2021
@aaronfranke aaronfranke force-pushed the translation-dotgodot branch from 1666709 to 4bd264b Compare June 3, 2021 11:49
Comment on lines 135 to 136
// `.right(6)` strips away the leading "res://".
String stripped_path = TranslationServer::TRANSLATION_FILES_PATH.plus_file(p_source_file.get_basename().right(6));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not work anymore, right() was changed in #36180. This should use the clearer substr().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR to use substr. I also tried testing it again, and I wasn't able to get the translation demo to work either with or without this PR. For now I will assume that this specific code still works because it worked when I made the PR.

@akien-mga
Copy link
Member

akien-mga commented Jun 3, 2021

For the record, the reason I haven't merged this yet is that I'm not sure that translation files can be considered as files which shouldn't be version controlled. I need to double check that.

They do get generated from CSVs, but I'm not sure that they get automatically generated or updated, and they depend on enabling specific languages in Project Settings, etc. It's not as clear cut as textures or samples.

@aaronfranke aaronfranke force-pushed the translation-dotgodot branch from 4bd264b to 679c28d Compare June 3, 2021 12:43
@aaronfranke aaronfranke force-pushed the translation-dotgodot branch from 679c28d to b635532 Compare June 15, 2021 00:53
@aaronfranke aaronfranke force-pushed the translation-dotgodot branch from b635532 to 6175d10 Compare July 3, 2021 21:07
@aaronfranke aaronfranke force-pushed the translation-dotgodot branch 2 times, most recently from 6757074 to 970867d Compare July 26, 2021 04:28
@akien-mga
Copy link
Member

Finally got to test this, and indeed as I feared it doesn't seem to work with the actual translation workflow. Did you try to port the gui/translation demo using it?

The CSV gets imported as .translation files, and then the translation files need to be added to the Localization tab in the Project Settings:
Screenshot_20210727_114402

With this change users would need to go find them in .godot/translations/ which is hidden.
Screenshot_20210727_114435

@aaronfranke
Copy link
Member Author

@akien-mga Is there a way that we can have users select the original CSV and have Godot automatically get the translation file?

@aaronfranke aaronfranke marked this pull request as draft August 9, 2021 16:32
@mhilbrunner
Copy link
Member

@akien-mga See above :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move generated translation files to res://.godot/translations
4 participants