Skip to content
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

MacUI: Add stub implementation of UI::IsTestMode() #11664

Merged
merged 1 commit into from
Mar 16, 2023

Conversation

OatmealDome
Copy link
Member

Fixes a regression from #11636.

#11636 never added an implementation of UI::IsTestMode() in MacUI.m, which causes a linker error. Bizarrely, the build never failed despite a linker error popping up in the log. This still needs to be investigated.

For now, just fix the error so that the updater can actually build again on macOS.

@OatmealDome
Copy link
Member Author

OatmealDome commented Mar 16, 2023

Just as a note: the artifacts confirm that this PR was built correctly (if it wasn't, the app icon would show up with a big prohibited symbol over it).

@delroth delroth merged commit d623871 into dolphin-emu:master Mar 16, 2023
@@ -139,6 +139,12 @@ void run_on_main(std::function<void()> fnc)
{
}

// test-updater.py only works on Windows.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it actually break on mac? Only thing I can think of is that it won't modify exe/dll files on mac, but that just results in less test coverage, it should otherwise "work"?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess another thing would be dealing with the bundle-ness of mac, idk what restrictions that would impose on copying the dolphin executables around and running from /tmp or whatever

Copy link
Member Author

Choose a reason for hiding this comment

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

To be honest, I just assumed it didn't work. I'll try it out...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants