GeezerVison is showing github.com — Virtual displays now use serial number as preferences ID appendix. by waydabber · Pull Request #793 · MonitorControl/MonitorControl
Conversation
There was an issue regarding virtual displays (DisplayLink, Sidecar etc) - the app uses CGDirectDisplayID as a preference ID suffix alongside the display name and vendor/model number. Since on every connect this id is changing for these displays, MonitorControl could not track preferences associated to these displays (+ the preferences store was polluted with dead items as well). This is now fixed, we now use the system generated, permanent serial number as suffix for virtual displays. For real displays we continue to use CGDirectDisplayID as the serial number is often the same for identical displays which causes issues in multi-display configs with similar physical displays.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…tead of CGDirectDisplayID for preferences storage appendix.
|
An other day, an other fix. Approve this one please. :) This is an addition to the bugfix release 4.0.2. Thanks!
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
|
self.serialNumber = serialNumber
|
|
|
self.isVirtual = DEBUG_VIRTUAL ? true : isVirtual
|
|
|
self.isDummy = isDummy
|
|
|
self.prefsId = "(" + String(name.filter { !$0.isWhitespace }) + String(vendorNumber ?? 0) + String(modelNumber ?? 0) + "@" + (self.isVirtual ? String(self.serialNumber ?? 9999) : String(identifier)) + ")"
|
JoniVR
Nov 16, 2021
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use string concatenation here instead? I think that might simplify this line quite a bit.
I haven't really tested it myself, because I don't really know how the virtual display feature is supposed to work, but will this handle me changing the name of the display without issues? Or is this name property a different one from the one the user can configure?
I think something like:
self.prefsId = "(\(name.filter { !$0.isWhitespace })\(vendorNumber ?? 0)\(modelNumber ?? 0)@\(self.isVirtual ? (self.serialNumber ?? 9999) : identifier))"might do the trick, is imo a bit more concise.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks nicer.
Yes, this name is a permanent one and different from friendlyName which was differentiated a while back.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed that line to look nice. Approve please! Thanks! :)
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
JoniVR
JoniVR approved these changes
Uh oh!
There was an error while loading. Please reload this page.
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|