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

feat(youtube/settings): simplify API #980

Closed
oSumAtrIX opened this issue May 13, 2023 · 3 comments
Closed

feat(youtube/settings): simplify API #980

oSumAtrIX opened this issue May 13, 2023 · 3 comments
Assignees

Comments

@oSumAtrIX
Copy link
Member

Issue

Currently, you have to manually use StringResource and specify a key and value. This is only useful in some cases, but in other cases, such as when specifying a string for the summaries, it may not be.

Solution

Rely on a convention to specify the keys of strings as specified here. Do not require manually using StringResource; instead, handle the strings keys based on the convention in the settings patch.

@LisoUseInAIKyrios
Copy link
Contributor

LisoUseInAIKyrios commented May 15, 2023

This brings up the question of localization to other languages. ReVanced Manager already has translations, and it's only fitting that the most popular app (YouTube ReVanced) also has translations.

If StringResource usage was replaced with a string key, then all the strings could be in localized strings.xml files.

The only concern is how to handle the multiple strings.xml files used by different patches (SponsorBlock, RYD, MicroG, and all other patch strings.xml files).

Would each of these string files have their own localizations? Or is it better to merge all application patch strings into a single file?

Each patch has it's own strings.xml file and multiple localized strings.xml files

upside:

  • when patching, only the strings required are included (will not include strings for excluded patches)
  • keep strings more organized to their patches.

downside:

  • requires many more localized files
  • probably a bigger hassle updating the translations (more files to update, possibly more complicated usage of crowdin)

Put all application patch strings into a single file (one strings.xml for each language of YouTube, one for each language for Twitch, etc)

upside:

  • less strings files
  • easier to update (only 1 file for each language for each app)

downside:

  • less modular
  • includes all application strings (even if a patch is excluded)

@oSumAtrIX
Copy link
Member Author

oSumAtrIX commented May 15, 2023

The idea is that each patch is isolated from each other, which means that each patch has to ship their own strings. Patches can depend on each other though, just like the SettingsPatch. It is possible to outsource strings to a strings.xml resource file for each patch and design the SettingsPatch, to consume it instead of manually using the StringResource API. This would enable localisation. If such a change is considered, it should be discussed throughoughly in a dedicated issue.

@LisoUseInAIKyrios
Copy link
Contributor

String translations could be somewhat discussed here, since it needs to be considered since it's directly related to the changes made here.

Since YouTube has over 50 patches, that means 50 different string files (and most files would contain only 3 strings, since most patches expose a single settings switch). When adding translations this gets really cumbersome, as translating to 10 or 20 languages gives between 500 and 1,000 string files. And when including all other apps that adds strings (YouTube Music, Twitch, TikTok, etc), you then have many thousands of string files. Bonkers.

After thinking more, I see a third 3 option based on your suggestion of consuming strings:

All strings are held in a single language file for each app, so YouTube translated into 10-20 languages means 10-20 YouTube ReVanced string files. Each of these files would contain all strings for all patches of that app. To add a new language for an app, you add exactly 1 new string file. To prevent bundling strings for patches not included, each patch could declare what strings to bundle based on a prefix. ie: copy-video-url would make a resource call to bundle all strings with prefix revanced_share_copy_url. This keeps the resources separated and no extra strings are bundled with the patched app unless they are needed. Yes the patches would hold their strings in a shared file, but the strings are keyed and name spaced, so the actual data is separated and this is vastly cleaner and simpler than creating thousands of files.

Personally, I am against using thousands of string files (way too many files, too much work updating them all when translations change). I much prefer the '1 language = 1 string file' approach for each app. This would also simplify using Crowdin for translations.

@oSumAtrIX oSumAtrIX transferred this issue from ReVanced/revanced-patches-template Dec 14, 2023
@oSumAtrIX oSumAtrIX transferred this issue from another repository Dec 14, 2023
@LisoUseInAIKyrios LisoUseInAIKyrios moved this from 📋 Backlog to ✅ Done in Development of ReVanced Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

2 participants