Skip to content

Commit

Permalink
scripting: don't observe properties with type nil
Browse files Browse the repository at this point in the history
mp.observe_property('foo', nil, ...) calls the handler at least 2 times
on each playlist change even when the property doesn't change. This is
dangerous because if you haven't read observe_property's documentation
in a long time this is easy to forget, and you can end up using it for
handlers that are computationally expensive or that cause unintended
side effects.

Therefore, this commit discourages its use more explicitly in the
documentation, and replaces its usages in scripts.

For console.lua, observing focused with type nil leads to calling
mp.osd_message('') when changing file while playing in the terminal with
the console disabled. I don't notice issues from this, but it's safer to
avoid it.

For playlist and track-list this doesn't really matter since they
trigger multiple changes on each new file anyway, but changing it can
avoid encouraging people to imitate the code.

One usage of none in stats.lua is kept because according to b9084df
it is a hack to replicate the deprecated tick event.
  • Loading branch information
guidocella authored and Dudemanguy committed Jan 20, 2024
1 parent c209d4f commit de08494
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 8 deletions.
6 changes: 3 additions & 3 deletions DOCS/man/lua.rst
Original file line number Diff line number Diff line change
Expand Up @@ -426,9 +426,9 @@ The ``mp`` module is preloaded, although it can be loaded manually with
This depends on the property, and it's a valid feature request to ask for
better update handling of a specific property.

If the ``type`` is ``none`` or ``nil``, sporadic property change events are
possible. This means the change function ``fn`` can be called even if the
property doesn't actually change.
If the ``type`` is ``none`` or ``nil``, the change function ``fn`` will be
called sporadically even if the property doesn't actually change. You should
therefore avoid using these types.

You always get an initial change notification. This is meant to initialize
the user's state to the current value of the property.
Expand Down
2 changes: 1 addition & 1 deletion TOOLS/lua/skip-logo.lua
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ local function read_frames()
end
end

mp.observe_property(meta_property, "none", function()
mp.observe_property(meta_property, "native", function()
-- Ignore frames that are decoded/filtered during seeking.
if seeking then
return
Expand Down
2 changes: 1 addition & 1 deletion player/lua/console.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1470,7 +1470,7 @@ end)
mp.observe_property('osd-width', 'native', update)
mp.observe_property('osd-height', 'native', update)
mp.observe_property('display-hidpi-scale', 'native', update)
mp.observe_property('focused', nil, update)
mp.observe_property('focused', 'native', update)

-- Enable log messages. In silent mode, mpv will queue log messages in a buffer
-- until enable_messages is called again without the silent: prefix.
Expand Down
6 changes: 3 additions & 3 deletions player/lua/osc.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2734,7 +2734,7 @@ function update_duration_watch()

if want_watch ~= duration_watched then
if want_watch then
mp.observe_property("duration", nil, on_duration)
mp.observe_property("duration", "native", on_duration)
else
mp.unobserve_property(on_duration)
end
Expand All @@ -2747,8 +2747,8 @@ update_duration_watch()

mp.register_event("shutdown", shutdown)
mp.register_event("start-file", request_init)
mp.observe_property("track-list", nil, request_init)
mp.observe_property("playlist", nil, request_init)
mp.observe_property("track-list", "native", request_init)
mp.observe_property("playlist", "native", request_init)
mp.observe_property("chapter-list", "native", function(_, list)
list = list or {} -- safety, shouldn't return nil
table.sort(list, function(a, b) return a.time < b.time end)
Expand Down

0 comments on commit de08494

Please sign in to comment.