CogentRedTester / mpv-sub-select

An advanced conditional subtitle track selector for mpv player
MIT License
89 stars 8 forks source link

Feature: further simplify config files, optimize priority logic #21

Closed dyphire closed 1 year ago

dyphire commented 1 year ago

At present, the following config are required to achieve accurate priority:

[
    {
        "alang": "j[ap]n?",
        "slang": ["chi", "und"],
        "whitelist": ["chs&j[ap]n?"]
    },
    {
        "inherit": "^",
        "whitelist": ["sc&j[ap]n?"]
    },
    {
        "inherit": "^",
        "whitelist": ["cht&j[ap]n?"]
    },
    {
        "inherit": "^",
        "whitelist": ["ch&j[ap]n?"]
    },
    {
        "inherit": "^",
        "whitelist": ["tc&j[ap]n?"]
    },
    {
        "inherit": "^",
        "whitelist": ["zh&j[ap]n?"]
    }
]

I hope it can be optimized to the following concise content:

[
    {
        "alang": "j[ap]n?",
        "slang": ["chi", "und"],
        "whitelist": ["chs&j[ap]n?", "sc&j[ap]n?", "cht&j[ap]n?", "ch&j[ap]n?", "tc&j[ap]n?", "zh&j[ap]n?"]
    }
]
CogentRedTester commented 1 year ago

But if we did this you would lose the ability to match the first subtitle track that fits any of the whitelisted strings. Instead it will always prefer a track that matches the first string. I think it might be misleading that the order of the whitelist and blacklist matter. I also think it makes the config file more ambiguous and difficult to understand. I added inheritance specifically to avoid further complicating the options with things like this.

It also goes against my initial design concept of the preferences which is that the first sub track to match all the conditions will be chosen. Adding preferences inside of those preferences makes the code more complicated and obscures the mechanics behind how the subtitles are chosen. I was already unsure about the PR that added preference to the slang option.

So basically unless you have some very good arguments against what I've talked about I won't be doing this.

dyphire commented 1 year ago

What you said is reasonable. I just saw https://github.com/CogentRedTester/mpv-sub-select/pull/20 and thought that the config file may be can further simplified. In fact, I don't strongly need this function.

I was already unsure about the PR that added preference to the slang option.

Some new functions added to this script recently are somewhat bloated in my view, especially the audio selection.

CogentRedTester commented 1 year ago

Some new functions added to this script recently are somewhat bloated in my view, especially the audio selection.

Yea, part of the reason that I added inheritance and the condition field were to reduce the amount of feature creep by providing generic ways to perform more complex functions.