Repository navigation
Trying Auto Skip - #2618
Trying Auto Skip#2618phisher98 wants to merge 16 commits into
Conversation
|
Please check out the new API at 1d03b05 |
|
Also, if this is done, I think it would make more sense for consistency with current behavior to be disabled by default. Just a suggestion though but not to adamant about it. |
|
Auto Skip Disabled by Default |
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
While I have no doubt this works fine, the logic is hard to follow given the indirect recursion and global variables. I think it would be much cleaner if this was a simple for loop inside a suspended context. Then you only need a single "Job" variable that can be canceled.
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
Good changes, but I think you can simplify this even more! ❤️
|
Done |
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
Good improvements, however while testing it I found that it is impossible to cancel it manually. Normally you either have a 5s countdown with a cancel button, or auto skip instantly. TV devices often cancel it immediately on any key-press.
So my question is: Is this the intended experience?
Yes I guess do we need that ? |
|
What I mean is that autoskip after 5s is a bit weird from a UX perspective. Here is an image of what I think autoskip should act like on TV, given my experience with the feature from other services. 
Where as phone has a similar cancel when you press the screen. Otherwise what is the point of a 5s timer? |
fire-light42
added
AI Assisted
# Conflicts: # app/src/main/res/values/strings.xml
- Any tap, swipe or key press now cancels the countdown, giving the user manual control (per review feedback) - Cancel the countdown when playback is paused so the player never seeks away on its own while paused - Restore the plain skip label whenever the countdown ends or is cancelled instead of leaving a stale '(3)' on the button - Run the countdown in viewLifecycleOwner.lifecycleScope so it is cancelled with the view; removes the onDestroyView override and the redundant try/catch/finally - Revert the ineffective skip button width change (the show/hide animator always sets its own width); keep the ellipsize fix - Reword the setting title/description to match what it does
…p margin - Show a 'Skipped <type> — Undo' snackbar after an automatic skip so inaccurate timestamps are recoverable; undo seeks back and suppresses auto-skipping that same timestamp again - New 'Auto-skip countdown (Seconds)' setting (0-10, default 5); 0 skips instantly, covering both behaviours requested in review - New 'Auto-skip types' setting with a multi-select dialog so users can e.g. auto-skip openings but never endings; defaults to all types - Both new preferences depend on the Auto skip switch and are disabled while it is off - Seek 500ms past the timestamp end when skipping a chapter so the last frames of the skipped section are not shown
The player re-invokes the current timestamp on buffering transitions, seeks and stamp loading, which reset the button label and restarted the countdown from scratch each time. On streams that buffer at the start of a video this meant the countdown never completed and the button only ever showed the plain 'Skip Opening' label. - Ignore re-invokes of a timestamp that is already showing - Remember a timestamp the user declined by interacting or pausing, so a later re-show does not start a new countdown and skip anyway
… undo snackbar The legacy XML preferences screen (settings_player.xml) is no longer reachable; the live Player settings are the Compose SettingsPlayerScreen, so the auto-skip settings were invisible. Add them there instead: - 'Auto skip' switch under 'Skip OP' - 'Auto-skip countdown (Seconds)' slider (0-10), disabled while off - 'Auto-skip types' multi-select, disabled while off, defaults to all Also remove the 'Skipped ... Undo' snackbar per feedback; the countdown can be cancelled by interacting with the player, and a wrong skip can be undone by seeking back manually
Trying to implement auto-skip; used AI for guidance.
Added a setting to disable it as well.