thompsonate / Shifty

☀️ A macOS menu bar app that gives you more control over Night Shift.
http://shifty.natethompson.io
GNU General Public License v3.0
1.24k stars 33 forks source link

Disabling night shift for an application fails for certain applications - Could not obtain bundle identifier of current application #75

Open Sarah-E-Greene opened 5 years ago

Sarah-E-Greene commented 5 years ago

I use Parsec (https://parsecgaming.com/) which, when connected to another machine, spawns a Parsec Game Window. I'm unable to disable night shift on just the Parsec Game Window, and the logs say "Could not obtain bundle identifier of current application" whenever I try to click the option moused over in the screenshot.

Disabling for the regular Parsec window works fine - but that doesn't also disable night shift on the actual game window - and my games look much better without night shift on!

Please let me know if there's any other information you'd like.

Screen Shot 2019-05-10 at 5 50 00 pm

choco commented 5 years ago

I took a quick look at the Parsec app bundle and inside there's an additional executable for the window. We use the bundleIdentifier property on NSRunningApplication for the rules, but that will be null in this instance since that specific executable doesn't have a plist associated. We should probably use a fall back in this situations, either bundleUrl or executableUrl (this one should be always valid, even though it isn't as flexible, since changing the app location creates a new rule)

Sarah-E-Greene commented 5 years ago

@choco Sounds logical. Happy to test if you want to build me a binary.

choco commented 5 years ago

This is a just a quick test binary using executableUrl as fallback. I haven't tested it myself but it should work!

Shifty.app.zip (N.B: this executable obviously has a different code signature, mine) Here's the diff I used if you want to compile the app yourself

diff --git a/Shifty/RuleManager.swift b/Shifty/RuleManager.swift
index ecdf862..222791e 100644
--- a/Shifty/RuleManager.swift
+++ b/Shifty/RuleManager.swift
@@ -27,15 +27,16 @@ enum SubdomainRuleType: String, Codable {

 struct AppRule: CustomStringConvertible, Hashable, Codable {
-    var bundleIdentifier: BundleIdentifier
+    var bundleIdentifier: BundleIdentifier?
+    var executableURL: URL?
     var fullScreenOnly: Bool

     var description: String {
-        return "Rule for \(bundleIdentifier); full screen only: \(fullScreenOnly)"
+        return "Rule for \(bundleIdentifier ?? "unknown") located \(executableURL?.absoluteString ?? "not set"); full screen only: \(fullScreenOnly)"
     }

     static func == (lhs: AppRule, rhs: AppRule) -> Bool {
-        return lhs.bundleIdentifier == rhs.bundleIdentifier
+        return ((lhs.bundleIdentifier == rhs.bundleIdentifier) || (lhs.executableURL == rhs.executableURL))
             && lhs.fullScreenOnly == rhs.fullScreenOnly
     }
 }
@@ -86,19 +87,34 @@ enum RuleManager {

     static var disabledForApp: Bool {
         get {
-            guard let bundleIdentifier = currentApp?.bundleIdentifier else {
+            if let bundleIdentifier = currentApp?.bundleIdentifier {
+                return disabledApps.filter {
+                    $0.bundleIdentifier == bundleIdentifier }.count > 0
+            } else {
                 logw("Could not obtain bundle identifier of current application")
-                return false
+                
+                guard let executableURL = currentApp?.executableURL else {
+                    logw("Could not obtain executable url of current application")
+                    return false
+                }
+                return disabledApps.filter {
+                        $0.executableURL == executableURL }.count > 0
             }
-            return disabledApps.filter {
-                $0.bundleIdentifier == bundleIdentifier }.count > 0
         }
         set(newValue) {
-            guard let bundleIdentifier = currentApp?.bundleIdentifier else {
-                logw("Could not obtain bundle identifier of current application")
-                return
+            var rule : AppRule
+            if let bundleIdentifier = currentApp?.bundleIdentifier {
+                rule = AppRule(bundleIdentifier: bundleIdentifier,
+                                   executableURL: nil, fullScreenOnly: false)
+            } else {
+                guard let executableURL = currentApp?.executableURL else {
+                    logw("Could not obtain executable url of current application")
+                    return
+                }
+                rule = AppRule(bundleIdentifier: nil,
+                               executableURL: executableURL, fullScreenOnly: false)
             }
-            let rule = AppRule(bundleIdentifier: bundleIdentifier, fullScreenOnly: false)
+
             if newValue {
                 disabledApps.insert(rule)
                 NightShiftManager.respond(to: .nightShiftDisableRuleActivated)
Sarah-E-Greene commented 5 years ago

I can confirm, that works perfectly. Disabling for the Parsec Game Window doesn't disable it for the regular Parsec window, as expected.

Thanks!

choco commented 5 years ago

Nice! 🎉 I cleaned up the code and pushed a PR (https://github.com/thompsonate/Shifty/pull/76) would you mind testing this test binary built on that branch? Thank you! Shifty.app.zip

Sarah-E-Greene commented 5 years ago

Certainly. That one also works correctly.

thompsonate commented 5 years ago

I've merged @choco's fix. Thank you both!

thompsonate commented 5 years ago

Actually, this'll take a little more work, since fixing this has some side effects that need more time to deal with (see #76). Reopening for now.

Sarah-E-Greene commented 4 years ago

I've had to stop using shifty because of this, I hope that this doesn't take much to fix again and can be fixed soon.