From de0849404b65876f0f8fe934c4b74f497bf4e29b Mon Sep 17 00:00:00 2001 From: Guido Cella Date: Sun, 14 Jan 2024 08:00:08 +0100 Subject: [PATCH] scripting: don't observe properties with type nil 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 b9084dfd47 it is a hack to replicate the deprecated tick event. --- DOCS/man/lua.rst | 6 +++--- TOOLS/lua/skip-logo.lua | 2 +- player/lua/console.lua | 2 +- player/lua/osc.lua | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/DOCS/man/lua.rst b/DOCS/man/lua.rst index 1ea612a0be..6a18e60e1d 100644 --- a/DOCS/man/lua.rst +++ b/DOCS/man/lua.rst @@ -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. diff --git a/TOOLS/lua/skip-logo.lua b/TOOLS/lua/skip-logo.lua index 8e1f9da489..ae66b22250 100644 --- a/TOOLS/lua/skip-logo.lua +++ b/TOOLS/lua/skip-logo.lua @@ -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 diff --git a/player/lua/console.lua b/player/lua/console.lua index 941f1a114e..4625c4ce3c 100644 --- a/player/lua/console.lua +++ b/player/lua/console.lua @@ -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. diff --git a/player/lua/osc.lua b/player/lua/osc.lua index 414066313d..7221de9867 100644 --- a/player/lua/osc.lua +++ b/player/lua/osc.lua @@ -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 @@ -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)