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

Allow to translate achievements #578

Open
wants to merge 9 commits into
base: beta
Choose a base branch
from
Open

Conversation

alyral
Copy link

@alyral alyral commented Jun 18, 2021

Strings needed:

  • AchUnlocked ("Achievement Unlocked!")
  • MoreAchUnlocked ("New Unlocks!")
  • For all achievements: Achievement[ID]Name and Achievement[ID]Descr

Known issues:
It's needed to write achievement name/description both in English.xml file and in bot code (required for website).

Note:
The fact that the group language is used for achievements sent during or at the end of a game is intentional.

@alyral
Copy link
Author

alyral commented Jun 19, 2021

Now I also added the needed strings.
I know you don't pull langfiles from GitHub, but at least you can copy-paste it more easily.
Can the lack of Achievement0Name/Descr represent a problem?

@alyral alyral changed the base branch from master to beta June 19, 2021 14:08
@OlgabrezelPrivate
Copy link
Collaborator

Hi there, thanks for your pull request!

Translatable achievements have been an open question for years now (see #89), so it's nice that you made those!
A few comments to your approach:

The fact that the group language is used for achievements sent during or at the end of a game is intentional.

This may be a problem: If a group has, for example, superamnesia mode set, players would probably get an empty achievement message and not even know what they achieved. So always using the player language might be better, so that players can decide in which language they would like to get the messages.

The translation will also ruin the "comparability" a bit: If you get an Achievement in Russian, the message won't be understandable to anyone who doesn't speak it, so you can't "show off" the message to other language speakers anymore. A solution could be to translate the achievement's descriptions but not the names.

What I don't really like about your approach is the way you made the string keys: Achievement7Name or Achievement12Desc doesn't in its own explain what the achievements 7 and 12 are about. It also means we can never "reuse" achievement IDs if we ever decided to drop certain achievements, which could happen, since some of them are already not in use.

Why don't you use the name of the AchievementsReworked instead? If it was AchievementWelcomeToHellDesc, we could just add deprecated=true to the string if we ever were to decide to drop the "Welcome to Hell" achievement (just as an example).

Looking forward to your answers! :​)

@alyral
Copy link
Author

alyral commented Jul 6, 2021

This may be a problem: If a group has, for example, superamnesia mode set, players would probably get an empty achievement message and not even know what they achieved. So always using the player language might be better, so that players can decide in which language they would like to get the messages.

I decided to use the group language because I know that many players don't know about the /setlang command, but of course I can change it if you think it's better that way.

The translation will also ruin the "comparability" a bit: If you get an Achievement in Russian, the message won't be understandable to anyone who doesn't speak it, so you can't "show off" the message to other language speakers anymore. A solution could be to translate the achievement's descriptions but not the names.

I hadn't thought of this problem, but another solution could be a command within the bot to display the unlocked achievements instead of the website.
It would use the language set in private, so just temporarily setting the appropriate language to show it to other people.

Why don't you use the name of the AchievementsReworked instead? If it was AchievementWelcomeToHellDesc, we could just add deprecated=true to the string if we ever were to decide to drop the "Welcome to Hell" achievement (just as an example).

Yes, it actually makes more sense

@alyral
Copy link
Author

alyral commented Jul 6, 2021

Sorry for the trouble, I made a terrible mistake copying from the wrong file (from the one I used to quickly try that thing without compiling the entire bot)

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

Successfully merging this pull request may close these issues.

2 participants