-
Notifications
You must be signed in to change notification settings - Fork 149
Used some of my claude time to update some things. #64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
xocialize
wants to merge
13
commits into
Stengo:main
Choose a base branch
from
xocialize-code:claude/modernize-phase2-4QYgc
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Used some of my claude time to update some things. #64
xocialize
wants to merge
13
commits into
Stengo:main
from
xocialize-code:claude/modernize-phase2-4QYgc
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Replace Timer.scheduledTimer with Combine Timer.publish - Replace NotificationCenter observer with Combine publisher - Add @mainactor annotations for thread safety - Add Sendable conformance to state types for thread safety - Use @main attribute instead of main.swift entry point - Encapsulate side effects in proper classes instead of global functions - Replace force unwrapping with safer optional binding - Extract magic numbers to named constants (DisplayConstants) - Use assertionFailure instead of fatalError for abstract methods - Improve code organization with smaller, focused methods https://claude.ai/code/session_01LsaxVzrkjDJMK8PYpthKvX
The Task { @mainactor } wrapper was causing side effects to initialize
asynchronously, which meant the notification observer wasn't set up
before the screen parameters notification fired from the virtual display.
Changes:
- Remove @mainactor class wrappers from side effects, use module-level
state with Combine publishers that handle main thread scheduling
- Remove @mainactor from SubscriberViewController, use DispatchQueue.main.async
- Remove @mainactor from ScreenViewController
- Simplify windowWillResize delegate method
The Combine Timer.publish and NotificationCenter.publisher still ensure
callbacks run on the main thread, but the observer setup is now synchronous.
https://claude.ai/code/session_01LsaxVzrkjDJMK8PYpthKvX
ENABLE_USER_SCRIPT_SANDBOXING was enabled by Xcode's recommended settings, but this prevents the SwiftFormat build script from accessing project source files. https://claude.ai/code/session_01LsaxVzrkjDJMK8PYpthKvX
The didChangeScreenParametersNotification may fire when CGVirtualDisplay is created, before the observer is set up. This caused the initial screen configuration to be missed. Now when setDisplayID is dispatched, we also query the screen configuration after a short async delay to allow the virtual display to fully initialize. This ensures the window gets its proper size on launch. https://claude.ai/code/session_01LsaxVzrkjDJMK8PYpthKvX
The Combine-based side effects had subtle timing differences that prevented the virtual display from initializing properly. Reverted to the original Timer.scheduledTimer and NotificationCenter.addObserver patterns which work correctly. Also fixed the SwiftFormat build phase warning by setting alwaysOutOfDate to tell Xcode the script should run on every build (equivalent to unchecking "Based on dependency analysis" in the build phase settings). https://claude.ai/code/session_01LsaxVzrkjDJMK8PYpthKvX
Complete rewrite of the application with a cleaner, more modern architecture: New Core module: - VirtualDisplayManager: Manages virtual display lifecycle with Combine - @published properties for reactive state updates - Automatic retry mechanism for display detection - Clean initialization and cleanup - DisplayStreamRenderer: Metal-backed display streaming - CAMetalLayer for GPU-efficient rendering - Direct IOSurface assignment (zero-copy) - Coordinate conversion for click-to-move - MouseTracker: Mouse position tracking with Combine - @published isWithinVirtualDisplay state - Timer-based polling with clean lifecycle - Cursor movement via CGDisplayMoveCursorToPoint - DeskPadViewController: Main view controller - Combine bindings for reactive updates - Dependency injection for testability - Clean separation of concerns Key improvements: - No more ReSwift dependency for core functionality - Combine for reactive state management - Metal layer for GPU-friendly rendering - Proper retry mechanism for display initialization - Clear ownership and lifecycle management https://claude.ai/code/session_01LsaxVzrkjDJMK8PYpthKvX
Added detailed print statements throughout the initialization flow: - VirtualDisplayManager: display creation, settings, screen detection retry - DisplayStreamRenderer: Metal setup, stream creation, frame rendering - DeskPadViewController: ready state handling, resolution changes - AppDelegate: applicationSupportsSecureRestorableState to fix warning This logging will help identify where the display stream initialization is failing. https://claude.ai/code/session_01LsaxVzrkjDJMK8PYpthKvX
- Removed Metal complexity from DisplayStreamRenderer - Use simple layer.contents approach like original code - Added logging to AppDelegate to trace initialization flow The simplified approach matches the working original implementation. https://claude.ai/code/session_01LsaxVzrkjDJMK8PYpthKvX
The original app used main.swift to manually set up NSApplication and its delegate. The @main attribute approach wasn't working correctly. This restores the original initialization pattern. https://claude.ai/code/session_01LsaxVzrkjDJMK8PYpthKvX
Changes: - Add @mainactor to VirtualDisplayManager, MouseTracker, DeskPadViewController - Replace Timer.scheduledTimer with Combine Timer.publish - Replace NotificationCenter.addObserver with Combine publisher - Remove all debug logging - Extract magic numbers to named Constants enums - Add applicationSupportsSecureRestorableState to fix warning - Clean up code structure with proper MARK sections The app now uses modern Swift patterns while maintaining the working main.swift entry point approach. https://claude.ai/code/session_01LsaxVzrkjDJMK8PYpthKvX
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Modernized some of the underlying technologies.