Always display Find Online Subtitle OSD messages #4992
Conversation
This commit will: - Add an isAlwaysShown property to OSDMessage that is true for OSD messages emitted by the Find Online Subtitles feature - Change PlayerCore.sendOSD to send messages whose isAlwaysShown property is true even when the enableOSD setting is false - Change MainWindowController.displayOSD to display messages whose isAlwaysShown property is true even when the IINA OSD is disabled due to the user enabling mpv's OSD These changes correct a problem where IINA's Find Online Subtitles feature appears to do nothing when the OSD is disabled. This feature uses the OSD to interact with the user. Failing to display the messages breaks the feature.
|
This does not address Thoughts? |
|
|
This commit will: - Add an alwaysEnabled property to OSDMessage that is true for OSD messages emitted by the Find Online Subtitles feature - Change PlayerCore.sendOSD to send messages whose alwaysEnabled property is true even when the enableOSD setting is false - Change MainWindowController.displayOSD to display messages whose alwaysEnabled property is true even when the IINA OSD is disabled due to the user enabling mpv's OSD These changes correct a problem where IINA's Find Online Subtitles feature appears to do nothing when the OSD is disabled. This feature uses the OSD to interact with the user. Failing to display the messages breaks the feature.
|
Makes sense. I was too close to the code to notice that it could be interpreted that way. I renamed it to We still need to figure out how to properly solve the similar issue for plugins. |
| @@ -2158,7 +2158,7 @@ class PlayerCore: NSObject { | |||
|
|
|||
| func sendOSD(_ osd: OSDMessage, autoHide: Bool = true, forcedTimeout: Float? = nil, accessoryView: NSView? = nil, context: Any? = nil, external: Bool = false) { | |||
| // querying `mainWindow.isWindowLoaded` will initialize mainWindow unexpectly | |||
| guard mainWindow.loaded && Preference.bool(for: .enableOSD) else { return } | |||
| guard mainWindow.loaded, Preference.bool(for: .enableOSD) || osd.alwaysEnabled else { return } | |||
There was a problem hiding this comment.
Why using , here instead of &&?
There was a problem hiding this comment.
I tried to find what Swift considers the best practice in this case and only found people debating that. The advice in the Medium post Chaining logical expressions with commas in Swift about expressions using commas being easier to read than complex expressions reflects what I was thinking. Using && would require adding () around the || expression. I was also thinking that the comma was more like swift has you have to use it when using let.
I'm good with whatever style the group wants to use. Let me know if you want me to switch this to &&.
There was a problem hiding this comment.
I was just asking and also found it a little bit confusing. I think it's alright to use , here to avoid the parentheses.
This commit will:
These changes correct a problem where IINA's Find Online Subtitles feature appears to do nothing when the OSD is disabled. This feature uses the OSD to interact with the user. Failing to display the messages breaks the feature.
Description: