[Agent] feat(default-skin): global controller overlay scale & opacity settings#3523
[Agent] feat(default-skin): global controller overlay scale & opacity settings#3523github-actions[bot] wants to merge 10 commits intodevelopfrom
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
🤖 PR created. AI review starting automatically. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
a7d49e1 to
a9cbc62
Compare
|
@claude this seems so lazy, it just scales! lol, what if it clips out of view or buttons overlap, this is retarded, do better |
✅ Rebase Conflicts ResolvedCompleted
Conflict Summary
|
|
@claude please do a self-review pass on this PR:
Then request a Copilot review. |
There was a problem hiding this comment.
Pull request overview
Adds a new global controllerScale preference and wires it into the SwiftUI “default skin” on-screen controller overlay, with matching UI controls in both SwiftUI Settings and the legacy UIKit settings screen.
Changes:
- Added
Defaults.Keys.controllerScale(0.5–2.0, default 1.0) inPVSettings. - Bound
controllerScale/controllerOpacityintoDefaultControllerSkinViewand applied them to portrait + landscape overlays. - Exposed the scale slider in both SwiftUI Settings and UIKit settings, plus a changelog fragment.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
PVSettings/Sources/PVSettings/Settings/Model/PVSettingsModel.swift |
Adds the new controllerScale Defaults key. |
PVUI/Sources/PVUIBase/SwiftUI/DeltaSkins/Views/Display/EmulatorWithSkinView+DefaultSkin.swift |
Applies user-configurable scale/opacity to the default controller overlay. |
PVUI/Sources/PVSwiftUI/Settings/SettingsSwiftUI.swift |
Adds “Controller Scale” slider to the On-Screen Controller settings section. |
PVUI/Sources/PVUIBase/Settings/PVSettingsViewController.swift |
Adds a “Scale” slider row in the UIKit controller settings section. |
.changelog/3523.md |
Adds release notes for the new setting(s). |
| .scaleEffect(controllerScale) | ||
| .opacity(controllerOpacity) |
There was a problem hiding this comment.
controllerScale is a Double (from Defaults.Key<Double>), but scaleEffect(_:) expects a CGFloat. As written this won’t compile unless SwiftUI provides an overload. Convert to CGFloat(controllerScale) (or change the key/type to CGFloat) before passing to scaleEffect.
| .scaleEffect(controllerScale) | ||
| .opacity(controllerOpacity) |
There was a problem hiding this comment.
Same issue as the landscape path: controllerScale is a Double, but scaleEffect(_:) expects CGFloat. Convert/cast before calling scaleEffect to avoid a compile error.
| - **Controller Overlay Scale** — New global setting (0.5×–2.0×, default 1.0×) to resize the on-screen controller overlay; accessible via Settings → On-Screen Controller. | ||
| - **Controller Overlay Opacity** — Existing opacity setting now also exposed in the UIKit settings view alongside the new scale control for full parity. |
There was a problem hiding this comment.
Changelog fragments are required to be plain English with no trailing period. Both entries end with a period, and the second bullet also claims the opacity setting is “now” exposed in UIKit even though UIKit already had an Opacity slider (so the entry is misleading). Please remove trailing periods and adjust the wording to match what actually changed.
| - **Controller Overlay Scale** — New global setting (0.5×–2.0×, default 1.0×) to resize the on-screen controller overlay; accessible via Settings → On-Screen Controller. | |
| - **Controller Overlay Opacity** — Existing opacity setting now also exposed in the UIKit settings view alongside the new scale control for full parity. | |
| - **Controller Overlay Scale** — New global setting (0.5×–2.0×, default 1.0×) to resize the on-screen controller overlay; accessible via Settings → On-Screen Controller | |
| - **Controller Overlay Opacity** — Updated UIKit controller overlay settings so the existing opacity slider appears alongside the new scale control for full parity |
Replaced naive
|
|
✅ Fixes pushed — addressed all 3 Copilot review comments:
|
| // Portrait layout — allocate height proportional to controllerScale | ||
| // so the game viewport and controller area grow/shrink together, | ||
| // with no clipping and no overlap possible. | ||
| let controllerFraction = min(0.55, max(0.20, 0.35 * CGFloat(controllerScale))) | ||
| VStack(spacing: 0) { | ||
| // Spacer to push controller to bottom area (top ~65% is screen area) | ||
| // Game area — expands when controls shrink, contracts when they grow | ||
| Spacer() | ||
| .frame(maxHeight: geometry.size.height * 0.65) | ||
| .frame(maxHeight: geometry.size.height * (1.0 - controllerFraction)) | ||
|
|
||
| // Controller area - constrained to bottom portion | ||
| // Controller area — height is the layout truth; no scaleEffect needed | ||
| dynamicControllerSkin | ||
| .frame(maxHeight: geometry.size.height * 0.35) | ||
| .clipped() | ||
| .opacity(controllerOpacity) | ||
| .frame(maxHeight: geometry.size.height * controllerFraction) |
There was a problem hiding this comment.
In portrait mode, controllerScale currently only changes the allocated controller-area height (controllerFraction) but does not actually scale the control content. Most controls have fixed sizes (e.g. circleButton is 60×60, joystick is 150×150), so the slider likely won’t make the overlay larger/smaller in portrait—only shrink/expand the game viewport. Consider applying a scale transform to the controller overlay (anchored to bottom) or making button sizing derive from controllerScale, while still preventing clipping/overlap.
| let controllerFraction = min(0.55, max(0.20, 0.35 * CGFloat(controllerScale))) | ||
| VStack(spacing: 0) { | ||
| // Spacer to push controller to bottom area (top ~65% is screen area) | ||
| // Game area — expands when controls shrink, contracts when they grow | ||
| Spacer() | ||
| .frame(maxHeight: geometry.size.height * 0.65) | ||
| .frame(maxHeight: geometry.size.height * (1.0 - controllerFraction)) | ||
|
|
There was a problem hiding this comment.
The portrait-only background layout above still uses the hardcoded 65%/35% split (geometry.size.height * 0.65 / * 0.35). Now that the actual controller area height is driven by controllerFraction, the background will be misaligned at non-1.0 scales (either not filling the controller area or bleeding into the game area). Update the background spacer/frame to use the same controllerFraction calculation.
|
✅ Fixes pushed — addressed all 5 Copilot review comments:
|
7ae882a to
15361dd
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
15361dd to
4793ee6
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
4793ee6 to
c115593
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
c115593 to
1fb9b55
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1fb9b55 to
cf5fbfe
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cf5fbfe to
dcdab61
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dcdab61 to
25209a8
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
25209a8 to
c30bccb
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
c30bccb to
b24ff09
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
🤖 Merge conflict detected — Claude has been dispatched to resolve the conflicts automatically. If conflicts cannot be resolved cleanly, the workflow will post a comment explaining what needs manual attention. |
✅ Rebase Conflicts ResolvedRebased Conflicts resolved
All 10 commits rebased cleanly after resolving this one conflict. |
- Add `controllerScale` key (0.5–2.0, default 1.0) to PVSettings - Apply `.scaleEffect()` and `.opacity()` to DefaultControllerSkinView for both portrait and landscape layouts - Add "Controller Scale" slider to SwiftUI OnScreenControllerSection - Add "Scale" slider to UIKit PVSettingsViewController Part of #2889 Closes #2890 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…verlap Replace naive scaleEffect with proper layout-aware scaling: Portrait: - Remove scaleEffect+clipped; compute controllerFraction = clamp(35% × scale, 20%–55%) - VStack spacer and controller frame both use this fraction so game area and controller area resize together — overlap/clip is structurally impossible Landscape: - Remove whole-view scaleEffect (was scaling from center → buttons flew off-screen) - Left panel: scaleEffect(.leading) so D-pad grows inward from screen left edge - Right panel: scaleEffect(.trailing) so action buttons grow inward from right edge Viewport calc: - Portrait controllerHeight derived from same clamp formula (was hardcoded 35%) - Landscape sideReserve multiplied by controllerScale (was ignoring scale entirely) - Both changes mean the game render rect always matches the actual control footprint Part of #2889 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eading UIKit opacity entry
…prevent misalignment Hoist controllerFraction calculation above the background VStack so the background spacer/frame matches the controller area at any scale, eliminating the bleed-into-game-area bug at non-1.0 scale values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add .scaleEffect(CGFloat(controllerScale), anchor: .bottom) to the portrait dynamicControllerSkin so controls visually resize at non-1.0 scale values. Also add .clipped() to prevent visual overflow and .contentShape(Rectangle()) to constrain hit-testing to the allocated frame area. Addresses Copilot review: portrait scale slider now actually resizes the overlay content, not just the height allocation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Apply controllerOpacity to the portrait RetrowaveBackground ZStack so the entire overlay panel fades uniformly (previously only the controls faded, leaving the background at full opacity). - Add .onChange(of: controllerScale) that re-emits the game viewport immediately when the scale slider changes, so the render area adjusts without requiring rotation or relaunch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ared helper - Clamp was 0.55 so 2× scale couldn't be represented (0.35×2.0=0.70 was cut to 0.55, causing layout/visual mismatch); raise to 0.70 to match the slider max. - Extract duplicated portraitControllerFraction() computation into a single private static helper so the SwiftUI layout and calculateDefaultViewport() always use exactly the same formula. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rWithSkinView+DefaultSkin.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
controllerScalesetting (Double, range 0.5–2.0, default 1.0) toPVSettings/Defaults alongside the existingcontrollerOpacity.scaleEffect(controllerScale)and.opacity(controllerOpacity)to the default controller skin overlay inDefaultControllerSkinView(both portrait and landscape layouts)SettingsSwiftUI.swift)PVSettingsViewController.swift) for consistencyDefaults(UserDefaults-backed)Changed Files
PVSettings/.../PVSettingsModel.swiftcontrollerScalekeyEmulatorWithSkinView+DefaultSkin.swiftDefaults; bind scale/opacity; apply to both portrait & landscape overlaysSettingsSwiftUI.swift@Default(.controllerScale)binding + scale slider inOnScreenControllerSectionPVSettingsViewController.swiftTest plan
Part of #2889
Closes #2890
🤖 Generated with Claude Code