Ajatt-Tools / mpvacious

🍜 Adds mpv keybindings to create Anki cards from movies and TV shows.
https://tatsumoto-ren.github.io/blog/mining-from-movies-and-tv-shows.html
GNU General Public License v3.0
602 stars 51 forks source link

Fixed secondary_sub_visibility=never not working #87

Closed Aquafina-water-bottle closed 1 year ago

Aquafina-water-bottle commented 1 year ago

I attempted to use the following option in my config:

secondary_sub_visibility=never

It seems like the secondary subtitles still showed up until I used ctrl+v. This PR fixes that issue.

Notes:

tatsumoto-ren commented 1 year ago

I can't seem to find any uses for the self.visibility ~= 'auto' section (even in change_visibility() function), but I kept it just in case. In other words, everything seems to work perfectly fine even if the condition was removed.

"auto" means that visibility will be controlled by moving the mouse.

Aquafina-water-bottle commented 1 year ago

I can't seem to find any uses for the self.visibility ~= 'auto' section (even in change_visibility() function), but I kept it just in case. In other words, everything seems to work perfectly fine even if the condition was removed.

"auto" means that visibility will be controlled by moving the mouse.

What I mean is that if I changed the code:

   if self.visibility ~= 'auto' then
        mp.set_property_bool('secondary-sub-visibility', self.visibility == 'always')
    end

to the following:

    mp.set_property_bool('secondary-sub-visibility', self.visibility == 'always')

then it appears to behave exactly the same, including the hovering ability with auto. This applies for both if conditions.

tatsumoto-ren commented 1 year ago

I guess you're right about checking the condition on startup, but I don't like that you duplicated the code.

Aquafina-water-bottle commented 1 year ago

I guess you're right about checking the condition on startup, but I don't like that you duplicated the code.

Sounds good, I'll move it to a function in a second.

tatsumoto-ren commented 1 year ago

it appears to behave exactly the same, including the hovering ability with auto. This applies for both if conditions.

Makes sense. If it is set to auto then secondary-sub-visibility will be changed when you move the mouse, so setting it beforehand has no effect.

Aquafina-water-bottle commented 1 year ago

Alright, I removed the if condition and separated it into a update_visibility function. Let me know if there's any other problems!

tatsumoto-ren commented 1 year ago

That's okay. Thank you!