Eisa01 / mpv-scripts

This repository contain scripts I have made for mpv media player...
BSD 2-Clause "Simplified" License
507 stars 35 forks source link

Lua script file name should not include version number #17

Closed snowman closed 3 years ago

snowman commented 3 years ago

description

When using require("SCRIPT_NAME-VERSION") to load Lua script, it'll first replace version number with "directory separator" in the modname, for example:

-- FILE: MPV_CONFIG_DIR/scripts/loader.lua
local smart_history = require "SmartHistory-1.7.1"
-- the same as
local smart_history = require "SmartHistory-1/7/1"

require (modname) document: https://www.lua.org/manual/5.1/manual.html#5.3

The second searcher looks for a loader as a Lua library, using the path stored at package.path.

A path is a sequence of templates separated by semicolons.

For each template, the searcher will change each interrogation mark in the template by filename, which is the module name with each dot replaced by a "directory separator" (such as "/" in Unix); then it will try to open the resulting file name.

So, for instance, if the Lua path is the string

 "./?.lua;./?.lc;/usr/local/?/init.lua"

the search for a Lua file for module foo will try to open the files

  1. ./foo.lua
  2. ./foo.lc
  3. /usr/local/foo/init.lua

in that order.

What's expected?

Strip version number from the script file name, for example, SmartHistory-1.7.1.lua should be renamed into SmartHistory.lua

snowman commented 3 years ago

Edited:

You can ignore this comment, this shows one limitation of using require(modname) funciton.


When the script seek-to.lua is loaded with loader.lua, seek-to.lua does NOT respond to the input.conf key bindings.

-- FILE: MPV_CONFIG_DIR/scripts/loader.lua
require "occivink/mpv-scripts/scripts/seek-to"
-- FILE: MPV_CONFIG_DIR/scripts/occivink/mpv-scripts/scripts/seek-to.lua
mp.add_key_binding(nil, "toggle-seeker", toggle-seeker)
## FILE: input.conf
## This does not work, because "seek-to.lua" is required by "loader.lua"
##
## So the target name is "loader" instead of "seek_to", you can check
## script name by put "mp.msg.error(mp.get_script_name())" at the top of
## "seek_to.lua"
t script-message-to seek_to toggle-seeker

Edited: https://mpv.io/manual/master/#command-interface-script-message-to

Same as script-message, but send it only to the client named .

Each client (scripts etc.) has a unique name.

For example, Lua scripts can get their name via "mp.get_script_name()".

This command has a variable number of arguments, and cannot be used with named arguments.

## FILE: input.conf
## Either work
t script-message           toggle-seeker
t script-message-to loader toggle-seeker
snowman commented 3 years ago

emmm, anyway, using require(modname) is not the best practice.

What if the script has options script-opts/SCRIPT_NAME.conf file?

so use this instead:

function load_script(script)
   mp.command("load-script " .. "~~/scripts/" .. script)
end

-- mpv-scripts
load_script("vendor/occivink/mpv-scripts/scripts/seek-to.lua")

So the best practice to load a script is to use mp.command("load-script " .. path_to_script) instead of require(modname) which has the limitation not allowing VERSION number like 3.1 to appear in the modename.

Eisa01 commented 3 years ago

I have no clue what you are trying to achieve and what you are doing

snowman commented 3 years ago

Use version number with dot(.) like 3.1 in SimpleUndo-3.1.lua is not best practice, how about strip the version number -3.1 off into SimpleUndo.lua

But now i am using mp.command("load-script " .. "SimpleUndo-3.1.lua"), the problem never bother me, so help yourself :D

Eisa01 commented 3 years ago

You can load the scripts, simply by placing them in the scripts directory. It will automatically use the script. But I believe removing the version number is a better approach, I will be removing it in the future. Feel free to remove it, it won't break anything.

snowman commented 3 years ago

Another example

If oneself wants to remap the keybinding,

## FILE: input.conf

## use "SmartHistory_1_7_1/resume" instead of "resume" to avoid conflict
x script-binding resume
x script-binding SmartHistory_1_7_1/resume

## same as above
x script-message                       resume
x script-message-to SmartHistory_1_7_1 resume

After upgrade SmartHistory from 1.7.1 into 1.7.2, you have to change those input.conf accordingly.

Thanks :heart:

Eisa01 commented 3 years ago

Renamed version number from scripts