Skip to content

[Feat] macOS: Check if Rosetta is available on Startup#4402

Merged
flavioislima merged 9 commits into
mainfrom
feat/check_rosetta
Mar 10, 2025
Merged

[Feat] macOS: Check if Rosetta is available on Startup#4402
flavioislima merged 9 commits into
mainfrom
feat/check_rosetta

Conversation

@flavioislima

Copy link
Copy Markdown
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
Comment thread src/backend/utils.ts Outdated
Comment thread src/backend/utils.ts Outdated
Comment thread src/backend/constants.ts Outdated

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
Copy Markdown
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

Comment thread src/backend/utils.ts Outdated
}

export async function checkRosettaInstall() {
if (!isMac) {

Copy link
Copy Markdown
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)

Comment thread src/backend/utils.ts Outdated

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

Copy link
Copy Markdown
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

Comment thread src/backend/utils.ts Outdated
} else if (isMac) {
if (isIntelMac) {
const isMacOSUpToDate = await isMacSonomaOrHigher()
if (isIntelMac && isMacOSUpToDate) {

Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread src/backend/utils.ts Outdated

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

Copy link
Copy Markdown
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
Copy Markdown
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

Comment thread src/backend/main.ts Outdated
Winetricks.download()
if (!availableWine.length) {
downloadDefaultWine()
if (isMac) {

@arielj arielj Mar 9, 2025

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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 and others added 4 commits March 10, 2025 12:41
Co-authored-by: Mathis Dröge <mathis.droege@ewe.net>
Co-authored-by: Mathis Dröge <mathis.droege@ewe.net>
@flavioislima flavioislima requested a review from arielj March 10, 2025 20:29
@flavioislima flavioislima merged commit 54cd8a0 into main Mar 10, 2025
@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