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

refactor: changed default actions for some commands #266

Merged
merged 1 commit into from
Dec 23, 2024

Conversation

Keith94
Copy link
Contributor

@Keith94 Keith94 commented Dec 22, 2024

Changed the title commands again to match mpv standard. cycle audio and cycle sub show an osd-msg by default, so I removed that bit of unneeded text.

`cycle audio` and `cycle sub` shows an `osd-msg` by default, so I removed that bit of unneeded text.
@Keith94
Copy link
Contributor Author

Keith94 commented Dec 22, 2024

@Samillion These commands were also changed if you want to follow these too.

They moved show-text ${playlist} 3000 after playlist-prev/next and used select-playlist for right mouse clicks.

playlist_prev_mbtn_left_command=playlist-prev; show-text ${playlist} 3000
playlist_prev_mbtn_right_command=script-binding select/select-playlist; script-message-to osc osc-hide
playlist_next_mbtn_left_command=playlist-next; show-text ${playlist} 3000
playlist_next_mbtn_right_command=script-binding select/select-playlist; script-message-to osc osc-hide

Edit:

Audio and sub are also swapped, so select functions happen on right click.

audio_track_mbtn_left_command = "cycle audio",
audio_track_mbtn_right_command = "script-binding select/select-aid; script-message-to osc osc-hide",
sub_track_mbtn_left_command = "cycle sub",
sub_track_mbtn_right_command = "script-binding select/select-sid; script-message-to osc osc-hide",

@Samillion
Copy link
Owner

Awesome, thank you very much.

cycle audio and cycle sub show an osd-msg by default, so I removed that bit of unneeded text.

I added those to help identify what is selected, but I see how it can be an annoyance.

@Samillion These commands were also changed if you want to follow these too.

This was a forced change at mpv, to be honest. Few of us (including console/select maintainer) were against this change, but a couple of users preferred the old way and not the select menu.

Not sure why, but this one is definitely a skip, as in my opinion, the select menu is one of the best additions to mpv.

It is why console/select maintainer liked our idea in compact mode discussion for a "all in one" button then added it to stock osc.
mpv-player/mpv#15499

@Keith94
Copy link
Contributor Author

Keith94 commented Dec 23, 2024

I added those to help identify what is selected, but I see how it can be an annoyance.

For some reason even with osd-msg removed, it still shows an osd-msg, is what I meant. Does that happen for you, too?

Also what do you think about the playlist ones I mentioned? Skip those too?

@Samillion
Copy link
Owner

For some reason even with osd-msg removed, it still shows an osd-msg, is what I meant. Does that happen for you, too?

LOL, now I know why it wasn't showing for me. In my config I limit the level of OSD messages. Thank you.

Also what do you think about the playlist ones I mentioned? Skip those too?

Sorry for the confusion, this was the response to the playlist ones:

This was a forced change at mpv, to be honest. Few of us (including console/select maintainer) were against this change, but a couple of users preferred the old way and not the select menu.

Not sure why, but this one is definitely a skip, as in my opinion, the select menu is one of the best additions to mpv.

It is why console/select maintainer liked our idea in compact mode discussion for a "all in one" button then added it to stock osc.
mpv-player/mpv#15499

@Samillion Samillion merged commit 5b99fdc into Samillion:main Dec 23, 2024
@Samillion
Copy link
Owner

Wait. Sigh, I always read too fast, a horrible habit of mine. You meant to bind select menu to prev/next. I just read "playlist" and my brain decided to fill the rest lol.

@Keith94
Copy link
Contributor Author

Keith94 commented Dec 23, 2024

Yeah, specifically for right-click and probably for shift+mbtn_left_down too?

ModernZ/modernz.lua

Lines 2141 to 2142 in 5b99fdc

ne.eventresponder["mbtn_right_up"] = function () mp.command("show-text ${playlist} 3000") end
ne.eventresponder["shift+mbtn_left_down"] = function () mp.command("show-text ${playlist} 3000") end

mpv ones are set to "select" here:

playlist_prev_mbtn_right_command = "script-binding select/select-playlist; script-message-to osc osc-hide",
playlist_next_mbtn_right_command = "script-binding select/select-playlist; script-message-to osc osc-hide",

https://github.com/mpv-player/mpv/blob/d82701962f99051a18d65c215b70d41ebadd9a22/player/lua/osc.lua#L76

@Samillion
Copy link
Owner

I rarely use right/middle click on those, so if you think it's a useful change, by all means open a PR and I'll merge.

Again, terribly sorry for reading too fast. Old habits die hard.

@Keith94
Copy link
Contributor Author

Keith94 commented Dec 23, 2024

No worries at all and good to hear. I'll get around to it soon.

@Keith94 Keith94 deleted the title-command branch December 23, 2024 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants