fix: wire the metalHUD + vendorSpoof Settings toggles into the Wine env#7
Open
jonx wants to merge 1 commit into
Open
fix: wire the metalHUD + vendorSpoof Settings toggles into the Wine env#7jonx wants to merge 1 commit into
jonx wants to merge 1 commit into
Conversation
Both toggles existed in Settings but `WineProcess.environment` never read them, so they did nothing: - "Show Metal performance HUD" now sets `MTL_HUD_ENABLED=1` (Apple's built-in FPS / frame-time overlay), off by default. - "Spoof GPU as Apple" now gates the `D3DM_VENDOR_ID/DEVICE_ID/DESCRIPTION` block, on by default (matches the previous hardcoded behaviour). Read straight from UserDefaults since WineProcess isn't @mainactor; defaults mirror the @AppStorage defaults. Adds a Performance section footer noting the toggles apply the next time Battle.net launches (they're env vars set at Wine start, not live). The syncStyle picker is intentionally left to the separate sync PR to avoid a conflicting edit to the same function. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@jonx is attempting to deploy a commit to the michaellod's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthrough
ChangesPerformance Settings Toggle & UI
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/D4Mac/BNetLauncher.swift`:
- Around line 45-55: The code only sets environment variables when toggles are
enabled, but inherited variables from the parent environment are not being
cleared when toggles are disabled. Add else clauses to both the vendorSpoof and
metalHUD conditional blocks to explicitly clear the corresponding environment
variables when their toggles are off. For the vendorSpoof block, clear
D3DM_VENDOR_ID, D3DM_DEVICE_ID, and D3DM_DEVICE_DESCRIPTION by setting them to
empty strings. For the metalHUD block, clear MTL_HUD_ENABLED by setting it to an
empty string when the toggle is false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 84b27da6-db6d-47c8-b3b3-b253584dbf8c
📒 Files selected for processing (2)
Sources/D4Mac/BNetLauncher.swiftSources/D4Mac/Settings.swift
Comment on lines
+45
to
+55
| // GPU vendor/device spoof so D4 takes the Apple-GPU path. | ||
| if vendorSpoof { | ||
| env["D3DM_VENDOR_ID"] = "0x106b" | ||
| env["D3DM_DEVICE_ID"] = "0x0209" | ||
| env["D3DM_DEVICE_DESCRIPTION"] = "Apple GPU" | ||
| } | ||
|
|
||
| // Apple's built-in Metal performance HUD (FPS / frame-time overlay). | ||
| if defaults.bool(forKey: "metalHUD") { | ||
| env["MTL_HUD_ENABLED"] = "1" | ||
| } |
There was a problem hiding this comment.
Clear inherited env keys when toggles are off.
Because Line 21 seeds from the parent environment, turning a toggle off does not currently guarantee deactivation if D3DM_* or MTL_HUD_ENABLED are already set upstream.
Suggested fix
// GPU vendor/device spoof so D4 takes the Apple-GPU path.
if vendorSpoof {
env["D3DM_VENDOR_ID"] = "0x106b"
env["D3DM_DEVICE_ID"] = "0x0209"
env["D3DM_DEVICE_DESCRIPTION"] = "Apple GPU"
+ } else {
+ env.removeValue(forKey: "D3DM_VENDOR_ID")
+ env.removeValue(forKey: "D3DM_DEVICE_ID")
+ env.removeValue(forKey: "D3DM_DEVICE_DESCRIPTION")
}
// Apple's built-in Metal performance HUD (FPS / frame-time overlay).
if defaults.bool(forKey: "metalHUD") {
env["MTL_HUD_ENABLED"] = "1"
+ } else {
+ env.removeValue(forKey: "MTL_HUD_ENABLED")
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/D4Mac/BNetLauncher.swift` around lines 45 - 55, The code only sets
environment variables when toggles are enabled, but inherited variables from the
parent environment are not being cleared when toggles are disabled. Add else
clauses to both the vendorSpoof and metalHUD conditional blocks to explicitly
clear the corresponding environment variables when their toggles are off. For
the vendorSpoof block, clear D3DM_VENDOR_ID, D3DM_DEVICE_ID, and
D3DM_DEVICE_DESCRIPTION by setting them to empty strings. For the metalHUD
block, clear MTL_HUD_ENABLED by setting it to an empty string when the toggle is
false.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
The
metalHUDandvendorSpooftoggles already existed in Settings, butWineProcess.environmentnever read them — so they did nothing.Changes
MTL_HUD_ENABLED=1(Apple's built-in FPS / frame-time overlay). Off by default.D3DM_VENDOR_ID/DEVICE_ID/DESCRIPTIONblock. On by default (matches the previous hardcoded behaviour).Read straight from
UserDefaultssinceWineProcessisn't@MainActor; defaults mirror the@AppStoragedefaults.Note: leaves the
syncStylepicker alone to avoid a conflicting edit to the same function — that one's handled by the separate sync-primitive PR; happy to rebase on top of it.Testing
swift buildclean; verified the HUD overlay appears when toggled on, via a local dev build.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style