episode playback preferences for remembering source and subtitle selections.#2574
episode playback preferences for remembering source and subtitle selections.#2574onurcvnoglu wants to merge 3 commits intorecloudstream:masterfrom
Conversation
…nd subtitle selections.
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
First review, what I had time for right now.
Generally the code is way too complex for something so simple. It should be easy to read and follow along, and understand how it integrates with the current automatic subtitle selection system (preferredAutoSelectSubtitles) and automatic priority source selection system.
Use comments to explain the intended behavior of complex code.
| internal data class EpisodePlaybackPreference( | ||
| val sourceDisplayName: String? = null, | ||
| val subtitleOriginalName: String? = null, | ||
| val subtitleSuffix: String? = null, |
There was a problem hiding this comment.
The suffixes are not stable, they get assigned by order of arrival.
If you want a more stable subtitle saving you should save the link instead of suffix. Subtitle links are generally relatively stable, but it is not a perfect solution.
app/src/main/java/com/lagradost/cloudstream3/ui/player/GeneratorPlayer.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/lagradost/cloudstream3/ui/player/GeneratorPlayer.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/lagradost/cloudstream3/ui/player/GeneratorPlayer.kt
Show resolved
Hide resolved
| ) { | ||
| val parentId = getEpisodePreferenceParentId(meta) ?: return | ||
| val updatedPreference = update(getEpisodePreference(meta) ?: EpisodePlaybackPreference()) | ||
| setKey( |
There was a problem hiding this comment.
This data is persistent and impossible to remove user-wise. It will cause backup files to grow endlessly (more than they already are).
You should expire and remove data after some time (max a month). It is unlikely that the user even wants to keep it longer than that, especially with sources changing so much.
…Player` to a new `EpisodePreferenceHelper`.
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
This is getting much better!
You have good comments and this is now much more understandable.
Only some smaller issues left to fix, then I will test it myself.
| autoSelectSubtitles() | ||
| } | ||
| } | ||
| observeLoadingLinks() |
There was a problem hiding this comment.
Please refrain from refactoring existing code unless required.
Unnecessary changes makes it more difficult to review and track down bugs, and it creates merge conflicts.
It is much better to keep each pull request scoped to solve one specific issue.
| ) ?: return null | ||
|
|
||
| // Expire preferences older than 30 days | ||
| if (System.currentTimeMillis() - preference.savedAt > PREFERENCE_MAX_AGE_MS) { |
There was a problem hiding this comment.
This does not delete any saved data.
The core issue is that we don't want user preferences to grow forever.
Therefore we must clean unwanted data, not just return null.
I suggest you place a removeKey here and then make the clear cache button in settings also clear all redundant keys. That way we can make it possible for users to clean this themselves, without looking through all keys all the time to check expiration date.
| ) | ||
| } | ||
|
|
||
| // Priority 2: Match by originalName + nameSuffix (cross-episode, preserves "Turkish 3" selection) |
There was a problem hiding this comment.
Do not use nameSuffix for any fallbacks, it could lead to incorrect automatic subtitle selection, which would mislead the user.
Only use url, name and language.
| * | ||
| * If subtitles were explicitly disabled, returns blockFallback=true | ||
| * to prevent auto-selection from overriding the user's choice. | ||
| */ |
| private const val PREFERENCE_MAX_AGE_MS = 30L * 24 * 60 * 60 * 1000 | ||
|
|
||
| internal data class EpisodePlaybackPreference( | ||
| val sourceDisplayName: String? = null, |
There was a problem hiding this comment.
Please use annotations, e.g., @JsonProperty("sourceDisplayName") before each variable in objects you are storing.
This is to signify to the reader that the content is going to be serialized and to ensure that no issues occur if someone renames the variables in the future.
2 participants
Remembers your selected source and subtitle options for each series. Since multiple sources and subtitles may be available and the default selection may not always be accurate, your choice will be automatically applied to subsequent episodes. This way, you won’t need to reselect them every time you switch episodes.
Some parts of this feature were developed with the assistance of AI, but all implementations have been reviewed and thoroughly tested.