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

menus.lua: tweaks and fixes the display of the track menu #902

Merged
merged 2 commits into from
May 6, 2024

Conversation

dyphire
Copy link
Contributor

@dyphire dyphire commented May 5, 2024

mpv upstream commit mpv-player/mpv@ab3b174 has introduced support for BCP 47 language tags,
and now track lang has uppercase and lowercase content.

Avoid showing the selection status of secondary subtitle in the subtitle menu at the same time.

current behavior of mian:
image
apply changes:
image

mpv upstream commit mpv-player/mpv@ab3b174 has introduced support for BCP 47 language tags,
and now track lang has uppercase and lowercase content.
@dyphire dyphire changed the title menus.lua: tweaks and fixes to track menu display menus.lua: tweaks and fixes the display of the track menu May 5, 2024
Copy link
Contributor

@christoph-heinrich christoph-heinrich left a comment

Choose a reason for hiding this comment

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

Seems reasonable

@@ -148,10 +148,10 @@ function create_select_tracklist_type_menu_opener(menu_title, track_type, track_
title = (track.title and track.title or t('Track %s', track.id)),
hint = table.concat(hint_values, ', '),
value = track.id,
active = track.selected,
active = track.selected and track.id == tonumber(mp.get_property(track_prop)),
Copy link
Contributor

@christoph-heinrich christoph-heinrich May 5, 2024

Choose a reason for hiding this comment

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

I'm somewhat bothered by tonumber(mp.get_property(track_prop)) being done twice on each iteration of the loop instead of once before the loop.

Youtube videos can add a ton of subtitle tracks if not restricted to specific languages via the ytdl options, and I don't know if the difference would be noticeable (probably not tbh), but since it's easy to avoid we might as well 😄

Copy link
Contributor Author

@dyphire dyphire May 5, 2024

Choose a reason for hiding this comment

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

Change to use local variables.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's still doing tonumber(mp.get_property(track_prop)) on each loop iteration though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean, done.

@dyphire dyphire force-pushed the track branch 2 times, most recently from 366fac3 to 0af7f61 Compare May 5, 2024 20:42
Avoid showing the selection status of secondary subtitle in the subtitle menu at the same time
@tomasklaen tomasklaen merged commit 8936009 into tomasklaen:main May 6, 2024
@dyphire dyphire deleted the track branch May 6, 2024 10:51
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.

3 participants