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

Create strings.xml files for project name translations #40182

Merged

Conversation

amanj120
Copy link
Contributor

@amanj120 amanj120 commented Jul 7, 2020

When a Godot game is being exported, one of the processes that takes place is that the resource table is modified to have different translations of the project name. This PR accomplishes the same purpose by writing strings.xml files in the Gradle project directory.

Note: This PR builds off of #39864, the store_string_at_path method will be provided through that PR.

For more details on the final scope of this project, read this comment.

The _create_project_name_strings_files method isn't currently called anywhere in the code, but it will be useful in subsequent PR's.

We are working on this PR with @m4gr3d, as explained here

@amanj120 amanj120 force-pushed the bundle_pr_resources branch from 6b26405 to 0ac0d81 Compare July 7, 2020 18:07
@m4gr3d m4gr3d self-requested a review July 7, 2020 19:36
@m4gr3d m4gr3d added this to the 3.2 milestone Jul 7, 2020
Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

The approach used here has visibility issues.

Unless a contributor is familiar with the added logic here, she/he may inadvertently create a string resource file in the project and be confused when the end result is not as expected since this logic would overwrite that file.

Even when a contributor is familiar with this logic, the addition of new string values now becomes a cumbersome task.

What about the approach of having the string resources already in the gradle project (e.g: moving them from the lib to the app module) and just overwriting the target string value?

@amanj120 amanj120 force-pushed the bundle_pr_resources branch from 0ac0d81 to 9df67bf Compare July 8, 2020 15:33
@amanj120 amanj120 marked this pull request as draft July 8, 2020 16:11
@amanj120 amanj120 force-pushed the bundle_pr_resources branch 3 times, most recently from 1d34bd9 to aa87e3d Compare July 8, 2020 17:15
@amanj120 amanj120 marked this pull request as ready for review July 8, 2020 17:20
@amanj120 amanj120 force-pushed the bundle_pr_resources branch 2 times, most recently from 3cff169 to 0b02158 Compare July 8, 2020 21:14
@amanj120
Copy link
Contributor Author

amanj120 commented Jul 8, 2020

The approach used here has visibility issues.

Unless a contributor is familiar with the added logic here, she/he may inadvertently create a string resource file in the project and be confused when the end result is not as expected since this logic would overwrite that file.

Even when a contributor is familiar with this logic, the addition of new string values now becomes a cumbersome task.

What about the approach of having the string resources already in the gradle project (e.g: moving them from the lib to the app module) and just overwriting the target string value?

I'm replying to this to indicate that we've resolved this issue.

@amanj120 amanj120 force-pushed the bundle_pr_resources branch from b53a0ab to d036543 Compare July 9, 2020 18:20
@amanj120 amanj120 force-pushed the bundle_pr_resources branch from d036543 to 9715110 Compare July 9, 2020 19:27
@akien-mga
Copy link
Member

Changes look good overall, though I have the same nitpicks on some of the xml files as in #40260.

@akien-mga akien-mga changed the base branch from 3.2 to 3.2-android-app-bundle July 15, 2020 14:02
@amanj120 amanj120 force-pushed the bundle_pr_resources branch 3 times, most recently from 19b6e28 to d9e5919 Compare July 15, 2020 20:08
Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

Can you squash the commits for this PR.

@amanj120 amanj120 force-pushed the bundle_pr_resources branch from d9e5919 to 8376abf Compare July 17, 2020 14:08
@akien-mga akien-mga merged commit d88e422 into godotengine:3.2-android-app-bundle Jul 20, 2020
@akien-mga
Copy link
Member

Thanks!

@amanj120 amanj120 deleted the bundle_pr_resources branch July 20, 2020 20:30
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.

4 participants