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

[Qol][Refactor] i18n lazy-loading #4327

Merged
merged 48 commits into from
Oct 1, 2024

Conversation

flx-sta
Copy link
Collaborator

@flx-sta flx-sta commented Sep 19, 2024

Caution

Make sure to consult the @pagefaultgames/translation-team & @pagefaultgames/translation-leads before merging as all locale files moved from /src/locales into /public/locales !!!


What are the changes the user will see?

Only the languages being used will be loaded thus less memory usage from the i18n side

Why am I making these changes?

The memory shouldn't be bloated with all languages at all time. IT should only hold the languages currently being used which at max is 2 (en as a fallback for missing keys).

What are the changes from a developer perspective?

  1. Moved all /src/locales into /public/locales as they will be loaded async (via network request).
  2. Install the i18next-http-backend plugin which is necessary for lazy-loading
  3. Add a camelCaseToKebabCase function to utils
  4. Clean up /src/plugin/i18n.ts file structure
  5. move phaser config into startGame() and lazy-load the scenes to prevent any i18n.t() usages before it is initialized
  6. Remove initI18n() from LoadingScene
  7. Remove all config.ts files inside the locales/
  8. Remove all locales tests. Reason: We don't need to test if i18next is working properly. They have their own test-coverage to ensure that
  9. i18next is now mocked for tests. This was overdue and now necessary due to the backend module

How to test the changes?

  1. Play the game
  2. Switch languages and keep on playing.
  3. Check the network tab for any failed translation requests (locales/**)

Checklist

  • I'm using beta as my base branch
  • There is no overlap with another PR?
  • The PR is self-contained and cannot be split into smaller PRs?
  • Have I provided a clear explanation of the changes?
  • [ ] Have I considered writing automated tests for the issue?
  • [ ] If I have text, did I make it translatable and add a key in the English locale file(s)?
  • Have I tested the changes (manually)?
    • Are all unit tests still passing? (npm run test)
  • [ ] Are the changes visual?
    • [ ] Have I provided screenshots/videos of the changes?

@flx-sta
Copy link
Collaborator Author

flx-sta commented Sep 19, 2024

Review Helper

to mark all **/*.json files as Viewed in the Files tab simply run this script in your browser-console:

 $$("#files .js-reviewed-toggle > input[id*='.json']").forEach((el) => el.click()); 

Your browser will probably lagg for 1-3 minutes as it sends quite a lot of server-requests at the same time then


Or use this link:
https://github.com/pagefaultgames/pokerogue/pull/4327/files?file-filters%5B%5D=.ts&show-deleted-files=false&show-viewed-files=true

It uses the filters to exclude all json file changes

src/test/vitest.setup.ts Outdated Show resolved Hide resolved
src/test/vitest.setup.ts Outdated Show resolved Hide resolved
@DayKev DayKev added Localization Provides or updates translation efforts Miscellaneous Changes that don't fit under any other label Refactor Rewriting existing code related labels Sep 19, 2024
now using "mysteryEncounters/..."
the encounter was checking if the modifier name includes `Berry` which is only true for english. Instead it has to check if the modifier is an instance of BerryModifier
the new i18n pattern requires a different namespacing which has been adopted
@flx-sta flx-sta marked this pull request as ready for review September 20, 2024 00:23
@flx-sta flx-sta requested review from a team as code owners September 20, 2024 00:23
innerthunder
innerthunder previously approved these changes Oct 1, 2024
there was an issue with circular dependencies
@MokaStitcher
Copy link
Collaborator

Found a missing change from . to: in one ME by pure chance when playtesting.

Screenshot 2024-10-01 at 21 50 43

Will try and double check all ME files

CodeTappert
CodeTappert previously approved these changes Oct 1, 2024
Copy link
Collaborator

@MokaStitcher MokaStitcher left a comment

Choose a reason for hiding this comment

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

Found 2 keys not properly updated for MEs:

await showEncounterText(scene, `${namespace}.option.1.bad`);

should be await showEncounterText(scene, ${namespace}:option.1.bad);

and

should be text: ${namespace}:outro_failed,

and remove `nonExplicitSupportedLngs`
@f-fsantos f-fsantos merged commit 9538686 into pagefaultgames:beta Oct 1, 2024
14 checks passed
@flx-sta flx-sta deleted the qol/i18n-lazy-loading branch October 1, 2024 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Localization Provides or updates translation efforts Miscellaneous Changes that don't fit under any other label Refactor Rewriting existing code related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants