Skip to content

Improve and cleanup PlayerHelper formatters/localization#12470

Merged
Stypox merged 5 commits intoTeamNewPipe:release-0.28.0from
litetex:cleanup-PlayerHelper-localization
Jul 28, 2025
Merged

Improve and cleanup PlayerHelper formatters/localization#12470
Stypox merged 5 commits intoTeamNewPipe:release-0.28.0from
litetex:cleanup-PlayerHelper-localization

Conversation

@litetex
Copy link
Copy Markdown
Member

@litetex litetex commented Jul 26, 2025 •
edited
Loading

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

Followup of #12446

  1. Contains some cleanup regarding the PlayerHelper localization and related code, removes dead code, etc.
  2. If the language is changed via ContentSettingsFragment, PlayerHelper's formatters will be reset. If formatting is now requested next time it will be reinitialized and the correct format will be applied.
Here's an example
Np12470Showcase1.webm

Fixes the following issue(s)

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.

Due diligence

@github-actions github-actions Bot added the size/medium PRs with less than 250 changed lines label Jul 26, 2025
@litetex litetex added player Issues related to any player (main, popup and background) size/medium PRs with less than 250 changed lines and removed size/medium PRs with less than 250 changed lines labels Jul 26, 2025
@litetex litetex marked this pull request as ready for review July 26, 2025 15:59
@litetex litetex added the code quality Improvements to the codebase to improve the code quality label Jul 26, 2025
Stypox
Stypox approved these changes Jul 28, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Thanks!

@Stypox Stypox changed the base branch from dev to release-0.28.0 July 28, 2025 13:10
litetex added 4 commits July 28, 2025 15:11
@litetex @Stypox
Use regions
859555e
@litetex @Stypox
Cleanup Localization formatting
70841db
@litetex @Stypox
Remove unused method
ebd5e1a
@litetex @Stypox
Encapsulate Formatters in PlayerHelper
893a1cb 
and reset them when the language is changed/changing.
This way they will be re-initialized on the next call.

Also Remove a bunch of outdated/non-thread safe code (STRING_FORMATTER)
@Stypox Stypox force-pushed the cleanup-PlayerHelper-localization branch from 141016a to 893a1cb Compare July 28, 2025 13:12
@Stypox
Copy link
Copy Markdown
Member

Stypox commented Jul 28, 2025

Actually, wait, it does not work while the player is open, because formatters() is called again by the player before the user has a chance to click on the language in the language chooser.

@Stypox
Fix player formatters resetting too early on language change
eea2b74 
formatters() is called again by the player before the user has a chance to click on the language in the language chooser.

So the correct solution would probably be to attach to https://developer.android.com/reference/android/content/Intent#ACTION_LOCALE_CHANGED, but let's keep it simple. I added `PlayerHelper.resetFormat();` in `ContentSettingsFragment.onDestroy()` and it works. It will mean the player formatters will be reset every time the user exits content settings, but whatever.
@Stypox
Copy link
Copy Markdown
Member

Stypox commented Jul 28, 2025

So the correct solution would probably be to attach to https://developer.android.com/reference/android/content/Intent#ACTION_LOCALE_CHANGED, buuuuuuuuut let's keep it simple. I added PlayerHelper.resetFormat(); in ContentSettingsFragment.onDestroy() and it works. It will mean the player formatters will be reset every time the user exits content settings, but whatever.

@Stypox Stypox merged commit e9922fe into TeamNewPipe:release-0.28.0 Jul 28, 2025
5 checks passed
@Stypox Stypox mentioned this pull request Jul 28, 2025
Closed
@TobiGr
Copy link
Copy Markdown
Contributor

TobiGr commented Jul 28, 2025

What happens if the user changes the system language or the per-app pref while the player is open? That is not covered by the hack.

@Stypox
Copy link
Copy Markdown
Member

Stypox commented Jul 28, 2025

The old formatting remains in that case, until the app is closed and reopened, but I'd say it's an edge case and it's not worth implementing a more complex solution at the cost of introducing bugs related to broadcast receivers

@litetex litetex deleted the cleanup-PlayerHelper-localization branch July 28, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Stypox Stypox Stypox approved these changes

Assignees

No one assigned

Labels

code quality Improvements to the codebase to improve the code quality player Issues related to any player (main, popup and background) size/medium PRs with less than 250 changed lines

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Speed/Tempo use system language for formatting and not app language

3 participants

@litetex @Stypox @TobiGr