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

UICommon: Add support for portable.txt on macOS #10430

Merged
merged 1 commit into from Jul 8, 2022

Conversation

OatmealDome
Copy link
Member

This PR adds support for portable.txt on macOS. Placing a file named portable.txt next to Dolphin.app will force Dolphin to create and use a user directory in the same folder as the app.

While this is a very un-Mac like behavior (apps aren't supposed to store files next to the app bundle!), it's useful for development.

@OatmealDome OatmealDome force-pushed the mac-portable branch 2 times, most recently from 85bb9fa to bb09b8d Compare February 3, 2022 06:43
@OatmealDome OatmealDome force-pushed the mac-portable branch 2 times, most recently from e934bd8 to cba32c1 Compare February 3, 2022 18:16
@OatmealDome OatmealDome marked this pull request as ready for review February 8, 2022 18:54
@Dentomologist
Copy link
Contributor

Would there be a problem if portable.txt and the resulting user folder were inside the bundle? That would allow people to have portable Dolphin in their Applications folder without polluting it.

@OatmealDome
Copy link
Member Author

I consulted with @ryanmcgrath on this, and this is what he had to say:

That’s actually what Slippi effectively did for awhile, and I suggested it in the IRC chat but was shot down

IIRC the logic against it was that it makes testing portable builds annoying since it needs to be done for each build

Personally, I'm fairly neutral on this. Perhaps both methods can be supported?

@OatmealDome
Copy link
Member Author

This technical note states that storing new data within the bundle after codesigning it is not allowed.

(I tried touching a new file inside a codesigned bundle, and macOS seemed to be OK with it. Not sure why, but I think I'd much rather prefer storing the user directory next to the bundle instead of inside of it to be 100% sure we don't accidentally break any future macOS updates.)

@nolrinale
Copy link
Contributor

nolrinale commented Jul 8, 2022

Because the usual method is to drag and drop the .app into the Applications folder it might pollute the App folder with another folder that might look strange and out of place for mac users specially if they use Launchpad a lot, I honestly think is better to leave the default behavior of keeping the user configs and saves in the Application Support folder like most macOS apps do.

What it can be done is maybe add an "Open Config Folder.." option into the Dolphin app menu bar so the user gets access to the dolphin folder within the library instantly via Finder and can copy/move/replace files if needed for troubleshooting or backup purposes in a more friendly way.

What do you think?

@OatmealDome
Copy link
Member Author

OatmealDome commented Jul 8, 2022

The point of this PR is not to move the User folder. Instead, I'm introducing an option that already exists on Windows and Linux to force the User folder to be placed next to Dolphin.app by creating a portable.txt file. There are various use cases for this, such as running builds for debugging purposes without affecting your actual Dolphin settings used for playing games.

The User folder will continue to remain in ~/Library/Application Support/Dolphin by default.

@nolrinale
Copy link
Contributor

Ah okay since its an optional setting I don't see any problem with it, whoever is gonna deal with this already has the knowledge to troubleshoot any situation that can arise.

Copy link
Member

@Simonx22 Simonx22 left a comment

Choose a reason for hiding this comment

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

I've tested this and can confirm that it works as expected on both macOS 12 and Manjaro Linux. Windows wasn't touched by this PR but I've tested that as well.

Code looks good to me too.

@JMC47 JMC47 merged commit 7853b72 into dolphin-emu:master Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants