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

DOCS/man/input: note that properties can be unavailable on init #15230

Merged
merged 3 commits into from
Nov 2, 2024

Conversation

guidocella
Copy link
Contributor

It comes up often in IRC and issues that users don't understand why the path property is initially unavailable, so document it.

sfan5
sfan5 previously approved these changes Oct 30, 2024
Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

lgtm

@na-na-hi
Copy link
Contributor

I don't think it's documented. I documented it in #15230.

That's not correct. It's documented here: https://mpv.io/manual/master/#details-on-the-script-initialization-and-lifecycle

Note that since the script starts execution concurrently with player initialization, some properties may not be populated with meaningful values until the relevant subsystems have initialized.

So there is no need for the first commit, which says the same thing but in a less general way and inaccurate ("retrieve in an event like
file-loaded instead" is inaccurate since the event doesn't have any field).

Because it is a bit shorter than calling
utils.join_path(working_directory, path), it gives you a canonical path,
and it doesn't concate working-directory before URLs.
If an option is referenced, the property will normally take/return exactly the
same values as the option. In these cases, properties are merely a way to change
an option at runtime.

Note that some properties are unavailable until playback starts. See `Details on
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that some properties are unavailable until playback starts.

I'm not sure I like this wording. It's oddly specific. Some properties won't be available until "something" happens. Example would be that video-params won't be available after audio file playback starts. Playback start doesn't guarantee that all properties are available.

Scripts are not suppose to pull the data and especially not at init time. But init time is not special, because we might go to idle state after playback, and many properties won't be available.

We need to educate users that they should observe properties or expect that any and all of them may or may not be present depending on the current player state.

I think it is the core of scripting in mpv and such vague one-liner won't help really.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you see the rest of the commit? I mentioned observe_property in details on script initialization.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can write it in general terms without mentioning any specific circumstances like "playback starts" or file-loaded event.

This is mentioned again below in Property list and it makes more sense
near the list of properties.
DOCS/man/lua.rst Outdated
@@ -86,7 +86,8 @@ own event handlers which you have registered with ``mp.register_event``, or
timers added with ``mp.add_timeout`` or similar. Note that since the
script starts execution concurrently with player initialization, some properties
may not be populated with meaningful values until the relevant subsystems have
initialized.
initialized. Rather than retrieving these properties are the top of scripts, you
Copy link
Contributor

Choose a reason for hiding this comment

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

Grammar

It comes up often in IRC and issues that users don't understand why the
path property is initially unavailable, so link the section that
mentions it from the Properties section, and expand on how to get these
properties.
@kasper93 kasper93 merged commit 4def28a into mpv-player:master Nov 2, 2024
6 checks passed
@guidocella guidocella deleted the property-docs branch November 2, 2024 05:38
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