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

Emulate GameCube memory card speeds #581

Merged
merged 3 commits into from Jul 19, 2014
Merged

Emulate GameCube memory card speeds #581

merged 3 commits into from Jul 19, 2014

Conversation

ghost
Copy link

@ghost ghost commented Jul 8, 2014

Fixes http://code.google.com/p/dolphin-emu/issues/detail?id=2284

Attempts to emulate GameCube memory card transfer speeds.

@delroth
Copy link
Member

delroth commented Jul 8, 2014

@dolphin-emu-bot rebuild

@@ -227,26 +234,47 @@ void CEXIMemoryCard::SetCS(int cs)

//???

CmdDoneLater(5000);
if (address >= MC_DATA_FILES)

This comment was marked as off-topic.

This comment was marked as off-topic.

@JMC47
Copy link
Contributor

JMC47 commented Jul 8, 2014

Did some rough and dirty testing with this build before PR went up and this is actually pretty nifty so far. I haven't tested enough games yet, but it fixes the mentioned issue in Wind Waker (J) and makes the saving/loading save times much, much, much more accurate in the games I tested.

MC_DATA_DIRECTORY = 0x2000,
MC_DATA_DIRECTORYBACKUP = 0x4000,
MC_DATA_BLOCKALLOCMAP = 0x6000,
MC_DATA_BLOACKALLOCMAPBACKUP = 0x8000,

This comment was marked as off-topic.

@delroth
Copy link
Member

delroth commented Jul 8, 2014

@LPFaint99 could you find some time to review this? AFAIK you're the local expert on everything memcards related :)

@shuffle2
Copy link
Contributor

shuffle2 commented Jul 8, 2014

I don't really know if the method of the fix is correct (hwtest?), but great job fixing that old bug @TotalNerd !

@ghost
Copy link
Author

ghost commented Jul 8, 2014

I'm hoping for somebody to make it more accurate. I originally did it cosmetically to satisfy my GameCube emulation authenticity but JMC47 pointed out it actually fixes an issue.

@JMC47
Copy link
Contributor

JMC47 commented Jul 8, 2014

Tested a few more games to try and see if this breaks anything, as well as compared save/loading times to console. The memcard save and load times were consistent with console in Wrestlemania X8, Wind Waker, Luigi's Mansion, Worms 3D, and Zelda: Four Swords Adventures. I will test more games later.

// These are rough estimates based on GameCube video evidence of memory card "speeds"
// This will need to be refined, and perhaps the idea behind the speed thing is wrong
// But it works with (near?) perfect speed as far as saving goes
static const float MC_TRANSFER_RATE_WRITE = 22.0f * 1024.0f;

This comment was marked as off-topic.

This comment was marked as off-topic.

@ghost
Copy link
Author

ghost commented Jul 13, 2014

Just wanted to point out with the latest commit, I am working on making this a little different and clean... was kinda a rushed result to see if it would work, but I'll see what I can do. Other than that, the read/write speed is not correct by any means.

@JMC47
Copy link
Contributor

JMC47 commented Jul 13, 2014

Everything appears to be working with the new system for Reading/Writing... except GCI folder doesn't seem to have proper timings. As such, Wind Waker (J) only saves properly when you select memcard, not GCI folder. Weird, huh?

Other than that, it's all about tightening up the timings. First thing I noticed is that on console, different memory cards gave me different speeds (Metroid Prime took almost a quarter of a second longer to read my third party memcard vs a first party one.)

@JMC47
Copy link
Contributor

JMC47 commented Jul 13, 2014

Did some serious testing on the latest revision (changed to 96.125 kb/s write, which will be added to the pull request soon, I'm sure)

I used a timer program plus my own reflexes for timing, which results in some of the variation I'm sure. At least it should be fairly consistent.

Console The Legend of Zelda: The Wind Waker - 1.41 - 1.55 seconds (2.33 - 2.5 unofficial card)
Pull Request The Legend of Zelda: The Wind Waker - 1.43 - 1.58 seconds
Master The Legend of Zelda: The Wind Waker - .3 seconds (or less, it's near instant)

Console Metroid Prime: .99 - 1.03 seconds (0.89 - 2 seconds with third party memory cards)
Pull Request Metroid Prime: .97 - 1.03 seconds
Master Metroid Prime: Less than .2 seconds; screen doesn't even stay up

For Metroid Prime, there was no good cue for timing, so I ended up doing a side by side test (wavebird hooked up to both console and emulator) and it came out identical (except the slight delay on T.V. tuner)

Console MVP 2004 Dynasty mode: 32.4-32.8 seconds (third party was over a minute)
Pull Request MVP 2004 Dynasty Mode: 32.8 - 33 seconds (the variation was pretty much none.)
Master MVP 2004 Dynasty Mode: 4.2 - 4.38 seconds

I will test more games, but it's a bit time consuming.

@JMC47
Copy link
Contributor

JMC47 commented Jul 13, 2014

@LPFaint99 After testing this again, and seeing the new method of timing the memory card syncing, it should be ready for review.

@shuffle2
Copy link
Contributor

@dolphin-emu-bot rebuild

@@ -102,7 +103,8 @@ IEXIDevice* EXIDevice_Create(TEXIDevices device_type, const int channel_num)
case EXIDEVICE_MEMORYCARDFOLDER:
{
bool gci_folder = (device_type == EXIDEVICE_MEMORYCARDFOLDER);
result = new CEXIMemoryCard(channel_num, gci_folder);
device_type = EXIDEVICE_MEMORYCARD;

This comment was marked as off-topic.

This comment was marked as off-topic.

@phire
Copy link
Member

phire commented Jul 15, 2014

@dolphin-emu-bot rebuild

1 similar comment
@Parlane
Copy link
Member

Parlane commented Jul 15, 2014

@dolphin-emu-bot rebuild

@JMC47
Copy link
Contributor

JMC47 commented Jul 15, 2014

Looks good to me. Retested GCI Folders and regular memory cards to make sure they were still working after the latest change. I can't do much code review, but functionally it's working as intended. Fixes issues with Netplay save timings in Gauntlet Dark Legacy, Wind Waker (NTSC-J) softlock when saving and, the bigger thing, is that it finally makes save and load times accurate,which was greatly lacking in Dolphin.

@JMC47
Copy link
Contributor

JMC47 commented Jul 15, 2014

@LPFaint99 A few details emerged on issue 7481 about how memcards communicate. It's new to me, at least.

"The pull request isn't exactly correct since not all memory cards are created equal. For reading there's a latency amount (hardcoded as part of the card's ID) which necessitates a number of dummy reads to clock the EXI bus/nand chip before real data starts being returned. For writing there are certain cards (using samsung NANDs) that support "fast mode" which require no separate erase commands and can write 512 bytes at a time instead of 128."

My question is, since you're the memcard expert, is this more accurate implementation good enough to be merged (after code review of course,) and better than the current implementation? Or would it be better to stick with what's in Dolphin now?

@LPFaint99
Copy link
Contributor

As long as we are using timings from a nintendo brand card, it seems fine to me. deciding exactly which card/speed to use can be decided at any time.

3rd party card timing should be ignored.

as this solves several issues and is more accurate I would say merge is ok

Code LGTM, haven't done any functional testing, trusting JMC47's testing

@Parlane
Copy link
Member

Parlane commented Jul 16, 2014

@dolphin-emu-bot rebuild

2 similar comments
@phire
Copy link
Member

phire commented Jul 16, 2014

@dolphin-emu-bot rebuild

@Parlane
Copy link
Member

Parlane commented Jul 16, 2014

@dolphin-emu-bot rebuild

@JMC47
Copy link
Contributor

JMC47 commented Jul 16, 2014

Retested the very latest build, everything's still working, I've never noticed any instability issues. Still fixes the issue mentioned throughout the PR.

LGTM (again :))

@@ -97,6 +97,11 @@ void ChangeDevice(const u8 channel, const TEXIDevices device_type, const u8 devi
CoreTiming::ScheduleEvent_Threadsafe(500000000, changeDevice, ((u64)channel << 32) | ((u64)device_type << 16) | device_num);
}

CEXIChannel* GetChannel(const u32 index)

This comment was marked as off-topic.

@JMC47
Copy link
Contributor

JMC47 commented Jul 17, 2014

Tested the latest version of the PR, everything still works.

delroth added a commit that referenced this pull request Jul 19, 2014
Emulate GameCube memory card speeds
@delroth delroth merged commit 963e1a6 into dolphin-emu:master Jul 19, 2014
@ghost ghost deleted the memcard-emu branch September 12, 2014 08:08
@PatrickFerry
Copy link
Contributor

This PR introduced a bug. In "XIII" the main menu runs at 3 FPS

Edit: or very slowly, as long as you have ever saved that is

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