Skip to content

[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

Merged
merged 9 commits into from
Mar 10, 2025

Conversation

flavioislima
Copy link
Member

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:

Version update:


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@flavioislima flavioislima added the pr:ready-for-review Feature-complete, ready for the grind! :P label Mar 9, 2025
@flavioislima flavioislima added this to the 2.16.1 Hotfix milestone Mar 9, 2025
@@ -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
Copy link
Collaborator

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

@@ -1288,6 +1290,69 @@ async function getPathDiskSize(path: string): Promise<number> {
return statData.size
}

export async function checkRosettaInstall() {
if (!isMac) {
Copy link
Collaborator

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)


// check if on arm64 macOS
const { stdout: archCheck } = await execAsync('arch')
const isArm64 = archCheck.trim() === 'arm64'
Copy link
Collaborator

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

if (isLinux) {
return version.type === 'GE-Proton'
} else if (isMac) {
if (isIntelMac) {
const isMacOSUpToDate = await isMacSonomaOrHigher()
if (isIntelMac && isMacOSUpToDate) {
Copy link
Collaborator

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

Copy link
Member Author

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.


export async function isMacSonomaOrHigher() {
if (!isMac) {
return false
Copy link
Collaborator

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

Copy link
Member Author

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

@@ -168,6 +169,9 @@ async function initializeWindow(): Promise<BrowserWindow> {
Winetricks.download()
if (!availableWine.length) {
downloadDefaultWine()
if (isMac) {
Copy link
Collaborator

@arielj arielj Mar 9, 2025

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point

@flavioislima flavioislima requested a review from arielj March 10, 2025 20:29
@flavioislima flavioislima merged commit 54cd8a0 into main Mar 10, 2025
9 checks passed
@flavioislima flavioislima deleted the feat/check_rosetta branch March 10, 2025 21:25
@Heroic-Games-Launcher Heroic-Games-Launcher locked and limited conversation to collaborators Mar 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants