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

Adding localization support to cheevos.c #15739

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

toadkarter
Copy link

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 to msg_hash and msg_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

@@ -13668,6 +13668,50 @@ MSG_HASH(
MSG_CHEEVOS_HARDCORE_MODE_ENABLE,
"Achievements Hardcore Mode Enabled, save state & rewind were disabled."
)
MSG_HASH(
MSG_CHEEVOS_HARDCORE_MODE_PAUSED,
"Hardcore paused. You cannot earn hardcore achievements for %s using %s"
Copy link
Contributor

Choose a reason for hiding this comment

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

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));
Copy link
Contributor

Choose a reason for hiding this comment

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

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));
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about rc_error_str.

MSG_HASH(
MSG_CHEEVOS_RICH_PRESENCE_PLAYING,
"Playing "
)
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

)
MSG_HASH(
MSG_CHEEVOS_NUMBER_ACHIEVEMENTS_UNLOCKED_UNSUPPORTED,
"You have %d of %d achievements unlocked (%d unsupported)."
Copy link
Contributor

Choose a reason for hiding this comment

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

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);

Copy link
Author

Choose a reason for hiding this comment

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

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)

@toadkarter
Copy link
Author

@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

@toadkarter toadkarter marked this pull request as draft September 26, 2023 07:23
@toadkarter toadkarter marked this pull request as ready for review September 26, 2023 09:15
@LibretroAdmin
Copy link
Contributor

Hi, just a couple of code style nits, single line code blocks don't need a bracket.

cheevos/cheevos.c Outdated Show resolved Hide resolved
cheevos/cheevos.c Outdated Show resolved Hide resolved
cheevos/cheevos.c Outdated Show resolved Hide resolved
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.

3 participants