Add Settings checkbox to disable EOF restart (addresses #3785) #4568
Conversation
|
I too am struggling with a name for this setting. As there are multiple ways to restart playback I would go generic and avoid using controls names such as "play" and "resume". Also, possibly we can rely upon the context of being a sub-option of On this:
The correct solution is to disable the button once EOF is reached if the user has turned off the setting to restart from the beginning. This also means the With this PR not addressing the issues with the spacebar, which is what issue #3785 is reporting we can't say this PR fixes that issue. I'm thinking this needs to be put in draft mode until the key binding problems are sorted out. IINA is too far behind on reviewing and merging. I will see if I can get the attention of the other developers. |
Yeah, definitely a rabbit hole of its own... So I actually attempted this, and it's harder than it seems. We can't generalize that the play button should be disabled at EOF, because it may still be legitimate to play the next item in the playlist. Or loop back from the beginning of the playlist. Or something else. It will require careful consideration of all the possibilities so that we don't risk introducing a new bug where it worked fine before. Also, what about the Considering the effort that would go into that, let's not have perfect be the enemy of the good? It's a pretty minor issue to worry about, considering it's blocking a fix which multiple users have expressed interest in getting resolved.
That could work. Will continue to ponder...
Looks like that should happen soon, but I'll put in draft for now as per request. |
|
Thanks for putting this into draft mode. I hope the devs will find some time for reviewing and merging soon.
I mentioned that in my comment. Yes, that menu item also needs to be disabled. I don't think the next and prev operations are tied to this. Disabling is definitely the correct way to handle this. It is not a case of disabling at EOF. It is disabling only when pressing space does not start playback. Exactly the same conditions the code already has to check. |
svobs
force-pushed
the
pr-restart-from-eof
branch
from
f31a4ed to
a6092ff
Compare
svobs
force-pushed
the
pr-restart-from-eof
branch
from
a6092ff to
ea6b161
Compare
|
Rebased and fixed conflicts, and made a further refinement. I stumbled onto a way to fix the play button behavior which allows this PR to appear complete. See the updated description. |
|
When The new text needs to be added to the English |
|
Description:
NOTE: If testing this, do not use key bindings! Use either thePlaybackmenu or the Play button in the OSC. There are currently a couple of open bugs which are causing this key binding to be unreliable.Trying to push this along. Not in love with the text, but would love suggestions.
Also encountered an issue with the Play button. Because it's implemented as a Cocoa toggle button with 2 states, clicking on it will always toggle the icon. Couldn't find an easy way to tell it not to toggle, which presented a problem when at the end and the Play button does nothing. Added a quick & dirty fix by adding code to thesyncUImechanism to bring it back in sync with the actual play state, but there may be numerous other ways to do it and seemed like that could be a whole issue by itself.This also changes
playButtoninto a push button instead of a toggle button. This means that its image does not automatically change in response to being clicked; it can only be changed via code. (I checked mpv's UI and it appears to handle its play button similarly. The image does not toggle between play & pause at mouseDown.)Doing this was necessary to prevent the button image from changing from
pausetoplaywhen clicked even when it is not possible to play (e.g. at EOF when this PR's new setting is unchecked). It was also necessary to make some minor logic changes to ensure that state always flows down frominfo.state == .pausedtoplayButton.image.