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: use script message to control the osc logo display in idle state #70

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

dyphire
Copy link
Contributor

@dyphire dyphire commented Oct 4, 2022

ref: #55, mpv-player/mpv@ec236f7
This will allow automatic control the logo display of osc scripts with have the osc-idlesereen feature.
However, since this feature is a recently added function, let's keep utils.shared_script_property_set.

@dyphire
Copy link
Contributor Author

dyphire commented Oct 4, 2022

The implementation of some osc scripts does not have the visibility function, because there are useful functions in the idle state. Ref: https://github.com/tomasklaen/uosc.

This pr can work better for such these osc scripts: tomasklaen/uosc#293, sosc.

@dyphire dyphire changed the title feat: use script message to control the osc logo display feat: use script message to control the osc logo display in idle state Oct 4, 2022
@CogentRedTester
Copy link
Owner

I not sure that only sending the message while idle is a good idea. What if the browser is opened while idle but only closed after something has started playing? The idlescreen will be left disabled even though we don't want it to be. It doesn't seem like enabling/disabling the idlescreen is a very intensive operation for the OSC, so it is probably fine to just always send the script messages.

However I do think there should be a script-opt in file-browser to control whether or not we send these messages. A simple toggle_idlescreen option will suffice.

@CogentRedTester CogentRedTester added the enhancement New feature or request label Oct 5, 2022
@dyphire
Copy link
Contributor Author

dyphire commented Oct 5, 2022

I not sure that only sending the message while idle is a good idea. What if the browser is opened while idle but only closed after something has started playing? The idlescreen will be left disabled even though we don't want it to be.

Ah, that's true. I missed it.

@dyphire
Copy link
Contributor Author

dyphire commented Oct 5, 2022

However I do think there should be a script-opt in file-browser to control whether or not we send these messages. A simple toggle_idlescreen option will suffice.

Done.

file-browser.lua Outdated
@@ -89,6 +89,9 @@ local o = {
--directory to load external modules - currently just user-input-module
module_directory = "~~/script-modules",

--enable script-message to control the idlescreen
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
--enable script-message to control the idlescreen
--turn the OSC idle screen off and on when opening and closing the browser

@@ -84,6 +84,9 @@ addon_directory=~~/script-modules/file-browser-addons
#directory to load external modules - currently just user-input-module
module_directory=~~/script-modules

#enable script-message to control the idlescreen
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
#enable script-message to control the idlescreen
#turn the OSC idle screen off and on when opening and closing the browser

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

ref: mpv-player/mpv@ec236f7, #55
This will allow automatic control the logo display of osc scripts with have the `osc-idlesereen` feature.
However, since this feature is a recently added function, let's keep `utils.shared_script_property_set`.
@CogentRedTester CogentRedTester merged commit 6763594 into CogentRedTester:master Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants