Add the option to use a folder of gamecube saves instead of a memcard image [issue 6599] #243

LPFaint99 · 2014-04-03T06:44:04Z

Delroth requested this feature back in sept as issue 6599
This includes the feature to create a virtual memorycard from a folder of gci files.
The saves that have been written to during gameplay will be stored in the savestate.

On first use of the option the users memorycard will be migrated to a folder (leaving the original memcard untouched)
After the memorycard folder exists, the user is required to synchronize the folder with a memcard image if wanted

commit includes the changes from PR 226

delroth · 2014-04-03T08:21:31Z

A quick question before I look at the code itself:

What is the new default setting for memcards? Do we default to raw or directory? I think defaulting to raw is a good idea atm, but making sure we can switch that default easily would be nice for the future (when we get more feedback from testers).

LPFaint99 · 2014-04-03T08:23:26Z

right now it is unchanged (raw memcard images)
switching is just a matter of changing the dropdown/ default ini setting

delroth · 2014-04-03T08:24:00Z

Ok, cool. I'll start reviewing when build passes :)

Source/Core/Core/HW/EXI_Device.cpp

@@ -3,6 +3,7 @@
 // Refer to the license.txt file included.


+


delroth · 2014-04-03T11:23:13Z

Did a quick first pass, haven't really looked into the details yet.

shuffle2 · 2014-05-22T04:00:52Z

Hello, this is not able to be auto-merged anymore and could probably use a rebasing. Please do so, and I'll merge once it passes the linter/buildbot again.

Source/Core/Core/HW/GCMemcard.cpp

@@ -28,7 +28,7 @@ GCMemcard::GCMemcard(const std::string& filename, bool forceCreation, bool sjis)
 		{
 			return;
 		}
-		Format(forceCreation ? sjis : !AskYesNoT("Format as ascii (NTSC\\PAL)?\nChoose no for sjis (NTSC-J)"));
+		Format(forceCreation ? sjis : AskYesNoT("Format as ascii (NTSC\\PAL)?\nChoose no for sjis (NTSC-J)"));


shuffle2 · 2014-05-22T06:32:28Z

There are some strange C-isms and small formatting issues, but it LGTM as-is. let me know if you're going to update it further or not and I'll merge it.

LPFaint99 · 2014-05-22T06:42:44Z

i'll fix the spacings, I'd like to address the C-isms separately.

go ahead and merge, and I'll create a new pull request after fixing the C-isms

shuffle2 · 2014-05-22T06:48:01Z

Ah, there are some more instances of such small spacing "issues", though.
By separately do you mean another commit in this PR, or separate PR?

LPFaint99 · 2014-05-22T06:56:19Z

is there an existing astyle config (or other autoformatter) that will fix the formatting to match the style guide?

I think I would prefer to do it as a separate PR, but whichever is preferred.

shuffle2 · 2014-05-22T07:03:57Z

I'm not aware of any tools like that. You can get pretty close (I think the only missing thing is the weird tab + space style of dolphin) with the built-in formatting options of visual studio (Tools->Options->Text Editor->C/C++).

I just saw http://visualstudioextensions.vlasovstudio.com/2013/11/05/format-c-code-intelligently-in-visual-studio-with-clang-format/ , which looks quite nice...I might try it in the future.
...but:

Notably, the Express editions do not support plugins

LPFaint99 · 2014-05-22T07:12:01Z

I'll have to look into it more. I'd love to have a tool fix my forgetfullness/laziness.

MayImilae · 2014-05-22T07:21:03Z

I have to ask... what happens if someone uses this and builds individual saves as GCI files, then moves over to an older version for say... issue testing? I assume that the saves that are in GCI format would simply not appear. This would be a big problem if someone has to convert their whole memory card to GCIs or back again. They'd essentially have no saves on older versions.

In the end you need to do what's best for the future, not the past, but we testers need to know this stuff. I assume in the worst case scenario, a tester could always load up a GCI file via the memory card manager.

LPFaint99 · 2014-05-22T07:32:34Z

There is the initial one time migration (when the gci dir doesn't exist).
You can use the memcard manager to export all saves (right click menu), but
import is currently 1 at a time.

If wanted/necessary I can implement syncing between folder and memcard,
triggered by changing the setting in the gui, but I think it would be
better to implement a feature in the memcard manager to build a card from a
folder of gcis
On May 22, 2014 12:21 AM, "MaJoRoesch" notifications@github.com wrote:

I have to ask... what happens if someone uses this and builds individual
saves as GCI files, then moves over to an older version for say... issue
testing? I assume that the saves that are in GCI format would simply not
appear. This would be a big problem if someone has to convert their whole
memory card to GCIs or back again. They'd essentially have no saves on
older versions.

In the end you need to do what's best for the future, not the past, but we
testers need to know this stuff. I assume in the worst case scenario, a
tester could always load up a GCI file via the memory card manager.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/243#issuecomment-43855984
.

MayImilae · 2014-05-22T07:38:29Z

It would be great if you could do that. Most frequent testers like myself will probably just stick to the memory card, since it's backwards compatible going way back, but a way to go between them quickly would really make the GCI folder better for users and especially testers.

Source/Core/Core/HW/EXI_DeviceMemoryCard.cpp

+	{
+		if (File::Rename(strDirectoryName, strDirectoryName + ".original"))
+		{
+			PanicAlertT("%s was not a directory, moved to *.original", strDirectoryName.c_str());


Source/Core/Core/HW/EXI_DeviceMemoryCard.cpp

+		}
+		else // we tried but the user wants to crash
+		{
+			// TODO more user friendly abort


LPFaint99 · 2014-05-22T19:44:17Z

I would like to nail down sync rules, I will get some time to get a list
started to get feedback on

edit- if it is unclear this was a response to
#243 (comment)

On May 22, 2014 12:38 AM, "MaJoRoesch" notifications@github.com wrote:

It would be great if you could do that. Most frequent testers like myself
will probably just stick to the memory card, since it's backwards
compatible going way back, but a way to go between them quickly would
really make the GCI folder better for users and especially testers.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/243#issuecomment-43857172
.

neobrain · 2014-06-19T14:28:05Z

I think further features like auto-synchronization between GCI folder and raw memory cards is beyond the scope of this PR, particularly since it's not obvious how to implement such a thing. Personally I'd like to be able to make the GCI folder location configurable (mostly because it's possible to have multiple memory card images, but not multiple GCI folders), but I think that also should simply happen in a followup PR.

Hence, LGTM.

neobrain · 2014-06-19T14:30:07Z

As far as I can see, 94c14af and 57ed39b (and possibly e84e5d4 ?) are the only commits which actually introduce new changes, rather than fixing up previous commits in this same branch. Can you see how hard it is to squash the other commits into the corresponding original commits?

delroth · 2014-06-23T00:43:18Z

This looks mostly ok to me and @neobrain LGTM'd earlier, please rebase and this can be merged.

\o/

delroth · 2014-06-23T00:55:41Z

Source/Core/Core/HW/EXI_DeviceMemoryCardRaw.h: includes are incorrect

Don't use relative paths that depend on the local dir, include "Core/HW/GCMemcard.h" (or wherever it is) explicitly.

Also, trailing whitespaces on Source/Core/Core/HW/EXI_DeviceMemoryCardRaw.cpp:126.

LPFaint99 · 2014-06-23T00:57:23Z

as far as I know those are fixed as of rd9eecd7

delroth · 2014-06-23T00:59:10Z

Last one:

Source/Core/Core/HW/GCMemcardRaw.h: includes are incorrect
Current Should be
"Common/ChunkFile.h" "Common/ChunkFile.h"
"Core/HW/GCMemcard.h" "Common/Thread.h"
"Common/Thread.h" "Core/HW/GCMemcard.h"

Also, please remove all the unneeded things from the commit messages. A commit message should explain what the commit is doing now, not the history of everything that has been squashed into that commit.

Thanks.

… from USERDIR/GC/<REGION>/Card <A/B> Savestates include the entire memorycard, but the only saves that should be modified are the ones that are directly modified by the game the others are preserved merely to avoid changing the memory card header during the game as some games (Zelda) refuse to save Implement DMA r/w for memcard. Skips programming buffer for writes Add a migration feature that auto imports all saves from your default memcard to the new memcard dir if it doesn't exist. Actually "delete" save files by renaming to s/*.gci/*.gci.deleted/

LPFaint99 · 2014-06-23T01:05:41Z

I think I have it all cleaned up

Add the option to use a folder of gamecube saves instead of a memcard image [issue 6599]

delroth reviewed Apr 3, 2014
View reviewed changes

Source/Core/Core/HW/EXI_Device.cpp

@@ -3,6 +3,7 @@

// Refer to the license.txt file included.

This comment was marked as off-topic.

Sign in to view

shuffle2 reviewed May 22, 2014
View reviewed changes

neobrain reviewed May 22, 2014
View reviewed changes

Source/Core/Core/HW/EXI_DeviceMemoryCard.cpp

}

else // we tried but the user wants to crash

{

// TODO more user friendly abort

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

move memorycard handling to its own class

404e9ce

delroth added a commit that referenced this pull request Jun 23, 2014

Merge pull request #243 from LPFaint99/memcard

01fc1a6

Add the option to use a folder of gamecube saves instead of a memcard image [issue 6599]

delroth merged commit 01fc1a6 into dolphin-emu:master Jun 23, 2014

Joern-P pushed a commit to Joern-P/dolphin that referenced this pull request Mar 16, 2023

Shake motions to middle mouse (dolphin-emu#243)

48066c8

Add the option to use a folder of gamecube saves instead of a memcard image [issue 6599] #243

Add the option to use a folder of gamecube saves instead of a memcard image [issue 6599] #243

LPFaint99 commented Apr 3, 2014

delroth commented Apr 3, 2014

LPFaint99 commented Apr 3, 2014

delroth commented Apr 3, 2014

This comment was marked as off-topic.

delroth commented Apr 3, 2014

shuffle2 commented May 22, 2014

This comment was marked as off-topic.

This comment was marked as off-topic.

shuffle2 commented May 22, 2014

LPFaint99 commented May 22, 2014

shuffle2 commented May 22, 2014

LPFaint99 commented May 22, 2014

shuffle2 commented May 22, 2014

LPFaint99 commented May 22, 2014

MayImilae commented May 22, 2014

LPFaint99 commented May 22, 2014

MayImilae commented May 22, 2014

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

LPFaint99 commented May 22, 2014

neobrain commented Jun 19, 2014

neobrain commented Jun 19, 2014

delroth commented Jun 23, 2014

delroth commented Jun 23, 2014

LPFaint99 commented Jun 23, 2014

delroth commented Jun 23, 2014

LPFaint99 commented Jun 23, 2014

Add the option to use a folder of gamecube saves instead of a memcard image [issue 6599] #243

Add the option to use a folder of gamecube saves instead of a memcard image [issue 6599] #243

Conversation

LPFaint99 commented Apr 3, 2014

delroth commented Apr 3, 2014

LPFaint99 commented Apr 3, 2014

delroth commented Apr 3, 2014

This comment was marked as off-topic.

delroth commented Apr 3, 2014

shuffle2 commented May 22, 2014

This comment was marked as off-topic.

This comment was marked as off-topic.

shuffle2 commented May 22, 2014

LPFaint99 commented May 22, 2014

shuffle2 commented May 22, 2014

LPFaint99 commented May 22, 2014

shuffle2 commented May 22, 2014

LPFaint99 commented May 22, 2014

MayImilae commented May 22, 2014

LPFaint99 commented May 22, 2014

MayImilae commented May 22, 2014

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

LPFaint99 commented May 22, 2014

neobrain commented Jun 19, 2014

neobrain commented Jun 19, 2014

delroth commented Jun 23, 2014

delroth commented Jun 23, 2014

LPFaint99 commented Jun 23, 2014

delroth commented Jun 23, 2014

LPFaint99 commented Jun 23, 2014