-
-
Notifications
You must be signed in to change notification settings - Fork 481
[Feat] macOS: Check if Rosetta is available on Startup #4402
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
Conversation
…cGamesLauncher into feat/check_rosetta
…r repository selection
src/backend/constants.ts
Outdated
@@ -28,7 +28,7 @@ const fontsStore = new TypeCheckedStoreBackend('fontsStore', { | |||
}) | |||
|
|||
const isMac = process.platform === 'darwin' | |||
const isIntelMac = isMac && cpus()[0].model.includes('Intel') // so we can have different behavior for Intel Mac | |||
const isIntelMac: boolean = isMac && cpus()[0].model.includes('Intel') // so we can have different behavior for Intel Mac |
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.
weird, shouldn't this be inferred as boolean? isMac
is boolean and includes
returns a boolean
src/backend/utils.ts
Outdated
@@ -1288,6 +1290,69 @@ async function getPathDiskSize(path: string): Promise<number> { | |||
return statData.size | |||
} | |||
|
|||
export async function checkRosettaInstall() { | |||
if (!isMac) { |
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.
this is only called inside an if (isMac)
check, so we don't need to check again (or we don't need the other check)
src/backend/utils.ts
Outdated
|
||
// check if on arm64 macOS | ||
const { stdout: archCheck } = await execAsync('arch') | ||
const isArm64 = archCheck.trim() === 'arm64' |
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 the isIntelMac
constant here? then we don't need to execute a process
if it's not intel mac then it's arm
src/backend/utils.ts
Outdated
if (isLinux) { | ||
return version.type === 'GE-Proton' | ||
} else if (isMac) { | ||
if (isIntelMac) { | ||
const isMacOSUpToDate = await isMacSonomaOrHigher() | ||
if (isIntelMac && isMacOSUpToDate) { |
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.
is this check correct? because this means that we'd set wine-crossover only if intel mac and new OS version, when I think we want to only set game porting toolkit if NOT intel mac and up-to-date os
we actually want wine-crossover always for intel macs
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.
TRue, should be !isMacOsUpToDate then. good catch.
src/backend/utils.ts
Outdated
|
||
export async function isMacSonomaOrHigher() { | ||
if (!isMac) { | ||
return false |
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.
I think it's better to return null
here because it's not even a macos at this point, it's not that it's not sonoma or higher, and then we can be more explicit on how we handle this
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.
We dont need this though, same case as you pointed above, it is only called inside isMac checks
src/backend/main.ts
Outdated
@@ -168,6 +169,9 @@ async function initializeWindow(): Promise<BrowserWindow> { | |||
Winetricks.download() | |||
if (!availableWine.length) { | |||
downloadDefaultWine() | |||
if (isMac) { |
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.
I don't know if this is the right place to check this though, cause the first time a user opens heroic, it will download a wine and run this check, but the second time it won't run this cause availableWine.length
will be more than 0
I think this should be done somewhere else so we can show it if the user doesn't install rosetta the first time
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.
Good point
Co-authored-by: Mathis Dröge <[email protected]>
Co-authored-by: Mathis Dröge <[email protected]>
This will check if Rosetta is available on Macs with Apple Silicon and show a warning with a message on how to install it.
Also checks if macOS is update to Sonoma or Higher for better GPTK support..
AI Summary
This pull request includes several important changes to improve compatibility and functionality on macOS systems, particularly for Apple Silicon chips. The changes involve adding checks for macOS version and Rosetta installation, as well as updating the logic for selecting the appropriate Wine version based on the system architecture.
Improvements for macOS compatibility:
src/backend/constants.ts
: Added type annotation forisIntelMac
to ensure type safety.src/backend/launcher.ts
: Added checks to ensure compatibility with macOS Sonoma or higher and to verify if Rosetta is installed on Apple Silicon Macs. [1] [2]src/backend/main.ts
: Integrated Rosetta installation check during the initialization process for macOS systems. [1] [2]src/backend/utils.ts
: Added functionscheckRosettaInstall
andisMacSonomaOrHigher
to handle Rosetta installation verification and macOS version checks. [1] [2]src/frontend/screens/WineManager/index.tsx
: Refactored logic for setting the default Wine repository based on the system's architecture, simplifying the code and improving readability.Version update:
package.json
: Updated the version from2.16.0
to2.16.1
.Use the following Checklist if you have changed something on the Backend or Frontend: