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

osc.lua: add idlescreen and osc-idlescreen #10207

Merged
merged 1 commit into from
Jun 4, 2022

Conversation

Dudemanguy
Copy link
Member

This is mainly for other user scripts that may conflict with the osc
logo in some way. While you can use script-message and
shared-script-properties (highly discouraged in the documentation for
some reason) to work around this, there's still some brittle edge cases
and complicated logic that can pop up. Adding an option for these two
things (the logo and the "drop files..." text) is really trivial and
would make things easier for scripts. Discussed in a couple of issues
below:
#10201
CogentRedTester/mpv-file-browser#55

@Dudemanguy Dudemanguy force-pushed the osc-logo branch 2 times, most recently from e814280 to ee30f51 Compare May 24, 2022 23:39
@Dudemanguy Dudemanguy changed the title osc.lua: add showlogo and showidlemsg options osc.lua add idlelogo and osc-idlelogo May 24, 2022
@Dudemanguy
Copy link
Member Author

I've changed it so only one option (idlelogo) controls all of this now. I also added a script-message that can be used to toggle this osc-idlelogo.

@stax76
Copy link
Contributor

stax76 commented May 26, 2022

I tested it by changing mpv.net and the osm script accordingly, it works fine, thank you.

@stax76
Copy link
Contributor

stax76 commented May 26, 2022

I think it would be helpful for scripts writing the osc-idlelogo state to shared-script-properties, much like osc-visibility is written to shared-script-properties. This would allow scripts to temporarily change the state and afterwards restore it. Restoring it, at least for me, is not important, but would still be nice if it could be done.

In my osm script, before I show my menu, I do this:

  1. Save osd-level in variable
  2. Set osd-level to 0
  3. Send script-message osc-idlelogo no
  4. Sleep 100 ms
  5. Restore osd-level

https://github.com/stax76/mpv-scripts/blob/main/osm.lua#L101

@Dudemanguy
Copy link
Member Author

I think it would be helpful for scripts writing the osc-idlelogo state to shared-script-properties, much like osc-visibility is written to shared-script-properties.

It now does this.

@sfan5
Copy link
Member

sfan5 commented May 30, 2022

Code lgtm but the commit message is missing a colon

@Dudemanguy Dudemanguy force-pushed the osc-logo branch 2 times, most recently from 29796b4 to 4a58f75 Compare June 1, 2022 14:06
@stax76
Copy link
Contributor

stax76 commented Jun 3, 2022

It now does this.

Thanks, I'm looking forward to this getting merged.

@Dudemanguy
Copy link
Member Author

I'm thinking about instead making the ASS text in here configurable so you could just pass an empty string (i.e. no logo). It would be more flexible. The implementation is a little trickier though because the logo is technically multiple strings not one so it would not be as simple as a typical string conf option.

@Dudemanguy Dudemanguy changed the title osc.lua add idlelogo and osc-idlelogo osc.lua: add idlescreen and osc-idlescreen Jun 3, 2022
@Dudemanguy
Copy link
Member Author

I'm thinking about instead making the ASS text in here configurable

Decided to ax this idea. I renamed this to idlescreen and osc-idlescreen. Should be OK now. I'll double check it in a bit to make sure I didn't somehow goof up find/replace.

This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlescreen option
disables all idlescreen related things (including the santa hat) if it
is set to "no". A new script message (osc-idlescreen) is also added so
users can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found in the
below github issues.
mpv-player#10201
CogentRedTester/mpv-file-browser#55
@Dudemanguy Dudemanguy merged commit ec236f7 into mpv-player:master Jun 4, 2022
@Dudemanguy Dudemanguy deleted the osc-logo branch June 4, 2022 14:48
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