Adding localization support to cheevos.c#15739
Adding localization support to cheevos.c#15739toadkarter wants to merge 2 commits intolibretro:masterfrom
Conversation
| ) | ||
| MSG_HASH( | ||
| MSG_CHEEVOS_HARDCORE_MODE_PAUSED, | ||
| "Hardcore paused. You cannot earn hardcore achievements for %s using %s" |
There was a problem hiding this comment.
The enum for this should be more specific. Maybe MSG_CHEEVOS_HARDCORE_PAUSED_INVALID_CORE?
There are several other reasons hardcore could be disabled in rcheevos_validate_config_settings.
| snprintf(buffer, sizeof(buffer), | ||
| "Could not activate achievement %u \"%s\": %s", | ||
| msg_hash_to_str(MSG_CHEEVOS_COULD_NOT_ACTIVATE_ACHIEVEMENT), | ||
| achievement->id, achievement->title, rc_error_str(result)); |
There was a problem hiding this comment.
I'm not sure it's reasonable to localize this as rc_error_str will still return an English error message.
| snprintf(buffer, sizeof(buffer), | ||
| "Could not activate leaderboard %u \"%s\": %s", | ||
| msg_hash_to_str(MSG_CHEEVOS_COULD_NOT_ACTIVATE_LEADERBOARD), | ||
| leaderboard->id, leaderboard->title, rc_error_str(result)); |
There was a problem hiding this comment.
Same comment about rc_error_str.
| MSG_HASH( | ||
| MSG_CHEEVOS_RICH_PRESENCE_PLAYING, | ||
| "Playing " | ||
| ) |
There was a problem hiding this comment.
It's probably better to include the placeholder ("Playing %s") and change the string building to use snprintf. String concatenation is generally frowned upon for localization.
intl/msg_hash_us.h
Outdated
| ) | ||
| MSG_HASH( | ||
| MSG_CHEEVOS_NUMBER_ACHIEVEMENTS_UNLOCKED_UNSUPPORTED, | ||
| "You have %d of %d achievements unlocked (%d unsupported)." |
There was a problem hiding this comment.
Recommend just having a single string for "%d unsupported", then use the above messages to generate the output.
Untested code:
char unsupported[64];
snprintf(unsupported, sizeof(unsupported),
msg_hash_to_str(MSG_CHEEVOS_UNSUPPORTED_COUNT), number_of_unsupported);
snprintf(msg, sizeof(msg), msg_hash_to_str(MSG_CHEEVOS_ALL_ACHIEVEMENTS_ACTIVATED), number_of_core);
snprintf(msg, sizeof(msg), "%s (%s)", msg, unsupported);
There was a problem hiding this comment.
For this one, I made the following additions on top of the code that you suggested:
- I have added a new buffer for the initial main message, as we get a compiler warning otherwise for writing to the same buffer in a circular way;
- I am checking the return value of the last snprintf and returning early if it's -1, as there could be a hypothetical scenario where the result is greater than the msg buffer (this was also a compiler warning)
|
@Jamiras Thank you for your helpful comments - I will make changes in a new commit just so that they are easier to review, and then if these are ok from your perspective I will squash these commits for readability in the main build |
|
Hi, just a couple of code style nits, single line code blocks don't need a bracket. |
toadkarter
force-pushed
the
localization-cheevos
branch
from
ba33252 to
2f96322
Compare
toadkarter
force-pushed
the
localization-cheevos
branch
from
c2111b6 to
fd9960f
Compare
3 participants
Description
There were some TODOs surrounding unlocalized strings in
cheevos.c, so I have added localization support to these in the same style as the rest of the project (i.e., added the relevant info tomsg_hashandmsg_hash_us.Hopefully this is in line with what is expected, but if not please do let me know and I would be happy to make any changes.
Related Issues
N/A
Related Pull Requests
N/A
Reviewers
N/A