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

feat: initial support for updating options at runtime #571

Merged

Conversation

christoph-heinrich
Copy link
Contributor

Options can be changed during runtime by changing script-opts.
So far such option changes were simply ignored, but basic support for some options is very easy to achieve.

This is a very half baked thing, so far only time_precision and font_scale have been tested. Options that are as trivial to update as font_scale should also work without a problem, but other options need some additional work (similar to time_precision).
Also changes to e.g. proximity_out don't execute the same sanity checking as it does during initialization. We could move all the initialization stuff into a function that then gets executed in the update callback, which is trivial for the sanity checking, but requires some more work for the config table. Opinions?

@tomasklaen
Copy link
Owner

Well, yeah, there would have to be a lot of tinkering to make everything respond to these updates correctly. And how many people actually need it, or even have a use for it?

I don't think the demand matches the effort, but if you want to work on it, I won't stop you :)

@christoph-heinrich
Copy link
Contributor Author

I'll work on it whenever the motivation hits me, but since I don't actually need it myself, it might take a while 😃

@xzpyth can use it as is to make their time precision toggle work.

If anyone has any specific options they would like to have working, I'll prioritize those over the rest.

@xzpyth
Copy link

xzpyth commented May 31, 2023

I am absolute noob in this so I will just stick with vanilla osc + thumbfast + playlist manager since i need the clickable precision time. But thanks for the effort

@hooke007
Copy link
Contributor

hooke007 commented May 31, 2023

the clickable precision time

But your feature request is not really related to this PR. The timecode is one 'button' in vanilla osc while uosc's timecode is not.

edit: I read the posts in discussion and think christoph-heinrich might misunderstand OP's thoughts.
If we make timecodes clickable in uosc, then there are less room for the covered timeline to be clicked. It may not be a good idea for the current UI design.

@christoph-heinrich
Copy link
Contributor Author

I have no intention of making the time in the timeline clickable.

@xzpyth
Copy link

xzpyth commented Jun 2, 2023

It may not be a good idea for the current UI design.

thats why this ui is broken you just don't put timecode into clickable area of timeline (see youtube or vanilla mpv) this is just ridiculous

@FichteFoll
Copy link

FichteFoll commented Jun 16, 2023

If your goal is to simply copy the current timecode to the clipboard, you could instead also use a keybinding like the following:
https://github.com/FichteFoll/dotfiles/blob/b88d5f1bbcc484dcccc968baf5d0c8417185303f/mpv/.config/mpv/input.conf#L133-L139

# Copy stuff to clipboard
y-y run sh -c "echo -n '${path}' | xsel -b"; show-text "Copied path to clipboard"
y-p run sh -c "echo -n '${path}' | xsel -b"; show-text "Copied path to clipboard"
y-t run sh -c "echo -n '${time-pos}' | xsel -b"; show-text "Copied timestamp to clipboard"
y-T run sh -c "echo -n '${media-title} [${time-pos}/${duration}]' | xsel -b"; show-text "Copied title+timestamp to clipboard"
# time-code with fractions
y-f run sh -c "python -c 'print(r\"${time-pos}\" + \"${=time-pos}\"[-7:-3])' | xsel -b"; show-text "Copied timestamp to clipboard"

Note that these assume being run on Linux as well as having xsel and python installed.

@christoph-heinrich christoph-heinrich force-pushed the runtime_option_change branch 2 times, most recently from 80fcb33 to 396e594 Compare September 15, 2023 19:04
@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Sep 15, 2023

@tomasklaen We could have each element react to option changes in the same way they react to property changes by having a on_options method. That way each element can decide on it's own what it needs to do.

That could even be more fine grained by passing in which option has changed, but imo it's fine if all of them do everything for any option change. Even the element based filtering I currently have is probably unnecessary.

Thoughts?

@tomasklaen
Copy link
Owner

tomasklaen commented Sep 15, 2023

Sure. You can just use Elements:trigger('event_name', [arg1], [argN]) for that. And no need for options diff. Don't spend time and code lines on something we might not need.

Options can be changed during runtime by changing `script-opts`.
So far such option changes were simply ignored.
Now most options work, and the rest can be implemented when the needed.
@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Sep 17, 2023

I forgot to give an update on this yesterday.

The elements themselves should be done now, but the things that get initialized at the beginning of in main.lua aren't updated yet.
This is already useful as is (most options work), we could merge it and any options that don't work yet can be implemented when the need arises.

@christoph-heinrich christoph-heinrich marked this pull request as ready for review September 17, 2023 21:57
@tomasklaen tomasklaen merged commit 8fe748c into tomasklaen:main Sep 18, 2023
tam1m pushed a commit to tam1m/uosc that referenced this pull request Oct 2, 2023
Options can be changed during runtime by changing `script-opts`.
So far such option changes were simply ignored.
Now most options work, and the rest can be implemented when the needed.
@christoph-heinrich christoph-heinrich deleted the runtime_option_change branch November 11, 2023 18:16
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.

5 participants