Page MenuHomeWildfire Games

Improvements to the translators credits script
ClosedPublic

Authored by Itms on Apr 2 2019, 11:12 PM.

Details

Reviewers
bb
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22499: Improvements to the translators credits script.
Summary

Tiber7 reported that their name had disappeared from the credits. Upon investigation, this is due to a bug in the regex parsing the translator names.

I used the opportunity to also:

  • Port the script to Python 3
  • Remove a broken feature of the script that has been unused since the introduction of the translation credits
  • Remove trailing whitespace in the credits file (not included in the current diff, for clarity)
Test Plan

Test the script and check the changes performed on the credits list are consistent with the result I included in the patch

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Itms created this revision.Apr 2 2019, 11:12 PM
Itms added a comment.Apr 2 2019, 11:19 PM

Some inline details.

binaries/data/mods/public/gui/credits/texts/translators.json
2 ↗(On Diff #7647)

The whitespace fix was excluded from the diff for clarity (it changes one third of all lines in that file).

source/tools/i18n/creditTranslators.py
78 ↗(On Diff #7647)

Removed broken feature

83 ↗(On Diff #7647)

This now matches names containing non alphanumerical chars (typically, dashes and dots). I excluded < and , which respectively mark the email address boundaries and the years of activity.

83 ↗(On Diff #7647)

Unicode is the Python 3 default

108 ↗(On Diff #7647)
113 ↗(On Diff #7647)

Ditto

118 ↗(On Diff #7647)

Python 3 removes some whitespace by default. In order to keep the same behavior as previously and obtain the current diff, use the option separator=(', ', ': ').

Vulcan added a subscriber: Vulcan.Apr 2 2019, 11:22 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1152/display/redirect

bb added a subscriber: bb.Apr 5 2019, 12:23 PM
bb added inline comments.
binaries/data/mods/public/gui/credits/texts/translators.json
2 ↗(On Diff #7647)

sure, but as the added lines have trailing whitespaces aren't we adding them with the script, thus the next time we run it, we need to remove them again?

186 ↗(On Diff #7647)

Where did he went?

310 ↗(On Diff #7647)

names of ppl nowadays, thought twice the same character was weird enough...

497 ↗(On Diff #7647)

could be name changes too, but not clear seeing from this file...

566 ↗(On Diff #7647)

same

917 ↗(On Diff #7647)

same

1676 ↗(On Diff #7647)

Actually I don't believe this: much more logical would be that this guy actually is S\u00e1ndor Uszkai who is removed below

2096 ↗(On Diff #7647)

Never seen him

2472 ↗(On Diff #7647)

k, name change

3024 ↗(On Diff #7647)

don't see him back either

3318 ↗(On Diff #7647)

same

source/tools/i18n/creditTranslators.py
106 ↗(On Diff #7647)

Is that still valid as we don't pre-add existing ones anymore?

Itms marked 2 inline comments as done.Apr 7 2019, 7:50 PM

@bb Thanks for the remarks! Regarding the name disappearing or changing too stupidly:

  • If users want to use very strange nicknames on Transifex, we can't do a lot.... It's worth noting that in Transifex the username is used by default, but you can choose to supply a different name to appear in the po files ("Nicolas Auvray" instead of "Itms" typically). So some of the changes might be from people deleting this extra information.
  • If all translations of a user have been overwritten over time, they will disappear from the po file. If that is the case for all resources, we will loose trace of them. I agree that in theory we should keep them in the credits (for code or art, even if all changes from someone are superseded later, we thank them for contributing). However, we have no possibility to automatically detect if a name removal is a name change, or all translations from a user obsolete. So we would have, for instance, both Nicolas Auvray and Itms in the credits (stupid). There is also a privacy issue: if someone wants to remove their real name from the credits, they will do so on Transifex, but they won't be warned that we keep old names in the credits, and they won't know they should contact us.

So I think it's better to "forget" about people who contributed obsolete translations. If they want to be in the credits, we can easily see in the SVN log that they indeed contributed, and add them manually. Doing the converse and removing manually all old names of translators would be too much work.

binaries/data/mods/public/gui/credits/texts/translators.json
2 ↗(On Diff #7647)

No, when I commit the script, I'll commit the updated json file with all the whitespace removed (including in lines that were not changed here).

2096 ↗(On Diff #7647)

Very fishy: he seems to be in several language credits 🤔

source/tools/i18n/creditTranslators.py
106 ↗(On Diff #7647)

Yes, as there will be names in each translation resource file for each language, thus always creating duplicates.

bb added a comment.Apr 7 2019, 10:12 PM

I get that ppl can choose whatever name they want, but who is named 4c905de7e2c9950b7d83273a8070b072 (second one in the german list)? I tried finding him/her in transifex but no luck.

Regarding the removing: I guess that would work, but it needs to be stated in a transifex privacy policy I suppose

binaries/data/mods/public/gui/credits/texts/translators.json
2 ↗(On Diff #7647)

wasn't my question: is the script adding whitespaces?

source/tools/i18n/creditTranslators.py
106 ↗(On Diff #7647)

k

Itms updated this revision to Diff 8951.Jul 17 2019, 2:36 PM
Itms marked 2 inline comments as done.
Itms added a reviewer: bb.

You were right, there are a few bogus names here! I tracked them down, they are users who deleted their Transifex account. I added a regex to remove them from the credits.

4c905de7e2c9950b7d83273a8070b072 was, for instance, the translator of I will accept your offer to become allies, %(_player_)s. We will both benefit from this partnership., among others.

Itms marked an inline comment as done.Jul 17 2019, 2:37 PM
Itms added inline comments.
binaries/data/mods/public/gui/credits/texts/translators.json
2 ↗(On Diff #7647)

No, it doesn't! Moreover, switching from python2 to python3 will delete all existing whitespace.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/112/display/redirect

bb accepted this revision.Jul 17 2019, 10:33 PM
bb added inline comments.
source/tools/i18n/creditTranslators.py
84 ↗(On Diff #8951)

I now want the nick bbitmsbbitmsbbitmsbbitmsbbitmsbb. I don't think there is a way to such a case however

This revision is now accepted and ready to land.Jul 17 2019, 10:33 PM
bb added inline comments.Jul 17 2019, 10:38 PM
source/tools/i18n/creditTranslators.py
84 ↗(On Diff #8951)

*I don't think there is a way to avoid such a case however

Itms marked 3 inline comments as done.Jul 17 2019, 11:12 PM

Thanks for the review 🙂

source/tools/i18n/creditTranslators.py
84 ↗(On Diff #8951)

This is covered! i, t, m and s are not possible in hexadecimal and are not matching the regex 😛

This revision was landed with ongoing or failed builds.Jul 17 2019, 11:32 PM
This revision was automatically updated to reflect the committed changes.
Itms marked an inline comment as done.
Owners added a subscriber: Restricted Owners Package.Jul 17 2019, 11:32 PM