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

Add config setting for base GCI folder path. #11236

Merged
merged 4 commits into from Nov 22, 2022

Conversation

AdmiralCurtiss
Copy link
Contributor

Let's try this again.

I feel like people request this once a week, so we should really implement this in some form. The previous attempt (at #10379) got stalled because a) no one reviews UI code and b) we couldn't even agree on a proper UI for this, so I'll just take a hard stance this time and say this uses the exact same UI that is already in use for the raw memory cards.

This intentionally does not use the existing internal 'GCI folder override' settings (which were designed for Netplay) because they ignore the automatic region handling, which should be kept because mixing Western and Japanese text encodings on the same memory card can cause issues.

I've thought long and hard about how to present to the user what path they are configuring, and I ended up with a similar thing to what we use for raw memory cards: The user configures eg. /path/to/gci/folder/EUR, ending in any supported region identifier, and gets an error stating that the folder must end in a region identifier if it doesn't. This identifier is then replaced at runtime depending on what game is launched. Note that this does not comply with the existing default GCI folders, which end in /EUR/Card A etc., but that seemed really silly to keep for custom configured folders. If the user selects the default Card A/Card B folders this is detected and the path is reset to the default one.

@mbc07
Copy link
Contributor

mbc07 commented Oct 31, 2022

Gave this a try and noticed the "GCI Folder Path" field only appears if I set the GCI Folder to something via the "..." button (e.g. on a clean Dolphin install, the default setting for Slot A is GCI Folder, but the folder path field won't appear). Is that how it's supposed to work? LGTM regardless, just checking if that's intended behavior or a possible bug...

@AdmiralCurtiss
Copy link
Contributor Author

Yes, that's intentional to match the behavior of the raw memory cards (see #10757 (comment)). I'm not really the biggest fan of it, but I figured by matching existing behavior there would be less friction points to this PR.

@AdmiralCurtiss
Copy link
Contributor Author

Does anyone have anything to say here? If not I'll just merge this sometime because again, users do ask for this ever so often.

@MayImilae
Copy link
Contributor

MayImilae commented Nov 22, 2022

LGTM.

Honestly, having rethought it, I will no longer object to making the GCI path always be present below the selection when a GCI folder is selected. Memory cards are doing that anyway so whatever I guess. IMO they shouldn't, but this is more of an overall UX design change that should be part of a proper cleanup. I'll get to it soon enough. So if you want to make that change, by all means. But the PR as is is totally fine too.

Copy link
Member

@delroth delroth left a comment

Choose a reason for hiding this comment

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

LGTM with one tiny nit. Feel free to merge once addressed (or ping me to do so).

Source/Core/Common/CommonPaths.h Show resolved Hide resolved
@AdmiralCurtiss AdmiralCurtiss merged commit 132bf65 into dolphin-emu:master Nov 22, 2022
11 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the gci-path-config branch November 22, 2022 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants