Memory Card as folder instead of a big raw file. #359
Conversation
|
Dolphin includes games’ individual saves in savestates, if you’re wondering how that’s handled. You'll also ideally want to handle size/file-per-card limitations in a way that doesn't prevent games from accessing files other than their own (e.g. the Ratchet & Clank series, where having data from previous games makes the guns-from-previous-games weapon shop sell everything much cheaper). Dolphin does this, but I haven't really looked at the code to know how it does so, nor do I know if it's even deterministic (which matters for things like netplay). Anyway, code looks sufficiently clean and well-documented to me, aside from the handful of todos and the sheer length of the commit log. I'm not a skilled programmer, though, so feel free to ignore my code review. v_v |
|
If you want me to merge some of these commits I can totally do that. |
|
That's pretty cool that you coded this into deliverable state even! :p I have a suggestion for handling savestates: Regarding the flushing: |
|
Regarding auto-ejecting on save load, I see that the current implementation checks that by comparing memory card CRC and reinserting if it mismatches. Since it's actually just about seeing if the memory card state of the savestate and the current state match, my GetCRC() now returns a millisecond timestamp of the last time the card was written to. This should work out to the intended result, though I had to use wxGetLocalTimeMillis() instead of wxGetUTCTimeMillis() since the latter isn't available for some reason. Regarding flushing, yes, detecting a finished write would be ideal. Something along the lines of "if no data has been written in x seconds or frames, assume the current operation is done" might work, but I'm unsure if that's reliable. |
|
I've done a quick test of this and it works nicely :) |
|
Could please fix the new warnings in your code. And could you also fix the linux error. Thanks you. By the way, I don't understand what is the goal of this commit but is there any impact of the current memcard file (I mean from a compatibility point of view)? Or is this fully transparent. |
|
@gregory38: All new code is for adding an additional implementation like "FileMemoryCard impl". |
|
@gregory38 Will do. |
|
Are there any guidelines on how to implement timed events in PCSX2? Especially if they should depend on emulation time rather than real clock time. |
|
So I tried implementing delayed post-write flushing using a wxTimer which is reset every time a write occurs, and thus would only actually trigger and call Flush() after a few seconds of no writes, but kinda ran into a brick wall because -- apparently -- wxTimers can only be started from the main thread. Is there a different timer I could use instead? |
|
Hm, if real world timers cause problems, you could base it off of various emulation events. |
|
Is there an easy way to hook myself into emulation events, or do I have to explicitly insert a call into, say, VSyncEnd()? Cause the latter seems rather intrusive, especially since not all memory card implementations require it. |
|
Adding your timing there seems fine to me. If you look around, you should find other events also using VSyncEnd() as a timer. |
|
Alright then. Comment in the #define MEMORYCARD_USE_FOLDER in MemoryCardFile.h to use the FolderMemoryCard, otherwise it uses the default FileMemoryCard. Rudimentary tests (idling ingame with the frame limiter off) suggest no noticeable difference in performance. I'm not sure I'm happy with the additional method in the plugin API, but whatever, it works. I chose 60 frames as the cutoff for assuming the current write has finished, which seems reasonable enough to me. You can change it by modifying the FramesAfterWriteUntilFlush constant in the FolderMemoryCard class. |
|
Pretty coding there, not bad :) Hm, just one "assignee" possible in Github. Ref should've gotten a mail though :p |
ramapcsx2
assigned sudonim1 and refractionpcsx2 and unassigned sudonim1 and refractionpcsx2
|
Yeah i got it, ill give it a test at some point :) |
|
The plugin api has reserved space for these kinds of additions. I guess it's a question of finding out whether that actually works (with savestates for example). |
|
Still no luck on linux. Be aware of Unicode. You probably need to use the WX_STR macro. |
|
Hm that's weird, I made sure it compiled fine on Xubuntu after you pointed out the warnings last time. I'll try to figure it out. |
|
Hum it could be related to wx2.8/wx3.0. IIRC .c_str() doesn't work anymore on latest wx |
|
Yup that seems to have been it. Try now. |
|
Good it compiled fine on linux :) |
|
Compiles fine on Windows vs2013... |
|
Thanks for your comment, uyjulian. |
|
In regards the detection of read/writes, why not simply turn the original function names into a pointers to the core handlers and then let the new handlers override the pointers and call the original function themselves when it's done what it needs to do? |
…also fit into the remaining memory card space. This avoids situations where, for example, it would only add the icon file but not the game data file because the memory card was near-full.
…ethods as const where appropriate, and some other minor things.
We're not actually deleting files though, we just rename them to prepend _pcsx2_deleted_, in case something breaks or whatever, so the user can in an emergency just restore the save by removing that part of the filename.
…perly on first write to any given page. I'm kinda surprised this didn't horribly break things, honestly. I guess it's because memory card data is always written in blocks, but still.
…when the card is Close()d. This mainly means that the superblock is now no longer written every single time the memory card is closed, but only when it's changed (which should be exactly once, when the memory card is formatted). It also means that you can format a memory card and then have the emulator crash later without having to reformat the card next time.
…that have actually changed since the last known memory card state on the host file system.
This means that we are now no longer touching files that haven't technically been written to. Some games use timestamp information to automatically highlight the save that was last written to, so this should fix a small but annoying bug where it would highlight the wrong one.
Do note that while there is a much simpler check that looks like this:
// Remove (== don't flush) all memory card pages that haven't actually changed.
for ( auto oldIt = m_oldDataCache.begin(); oldIt != m_oldDataCache.end(); ++oldIt ) {
auto newIt = m_cache.find( oldIt->first );
assert( newIt != m_cache.end() ); // if this isn't true something broke somewhere, the two maps should always contain the same pages
if ( memcmp( &oldIt->second.raw[0], &newIt->second.raw[0], PageSize ) == 0 ) {
m_cache.erase( newIt );
}
}
m_oldDataCache.clear();
It can fail in edge cases that don't actually seem too unlikely. Imagine a save being deleted, and then a new save from the same game but in a different slot being created quickly afterwards. It seems quite possible that the new save's file data then occupies the exact same pages as the old save's, and since it's from the same game it might be close enough to where a page sized section (juse 0x200 bytes!) matches the data from the old save that previously resided in that location -- which would cause this code to throw away and not flush this data! It's a shame too, since this variant would be a few ms faster as well, but I feel it's better to be safe than sorry here.
AdmiralCurtiss
force-pushed
the
memcard-folder-cache
branch
from
0de78e0
to
6bd578c
Compare
|
Rebased. Windows build here: https://dl.dropboxusercontent.com/u/52687139/pcsx2-foldermemcard-public-test-rev7.7z Loaded and saved in a couple games, seems fine here. |
|
No errors reported, from what I can see? |
…nd 64MB FileMemoryCards.
|
Since it was basically requested, added convert options in the GUI to convert to 16MB/32MB/64MB files. https://dl.dropboxusercontent.com/u/52687139/pcsx2-foldermemcard-public-test-rev8.7z Identical to the previous build (rev7) otherwise. |
|
Anything still holding up the merge of this? |
|
The changes were just being tested with the new WX 3.0 stuff, as soon as i can confirm that is all good, I will merge it :) hopefully some time today |
…on process once before writing the data. This prevents half-converted/corrupted cards by ensuring we crash before any actual writes occur.
AdmiralCurtiss
force-pushed
the
memcard-folder-cache
branch
from
28da4a6
to
d331d59
Compare
Memory Card as folder support by AdmiralCurtiss
|
Is there any ps2 game with more than one disc? If so, they should be able to see saves from the other disc(s). Somehow... |
|
Already implemented: 02ae12c It's quite likely I missed some more obscure games so feel free to report any games that do not find saves they should be able to see. |
|
That was... a lot of work. |
Long forgot this thread, anyways to answer your question yes there are ps2 games with more than one disc, Star Ocean: Till the End of Time used 2 discs for example if I remember rightly (been a while since I booted PCSX2 to play anything) |
|
Well. Not only. Search for 'disc' and you'll see in the right hand side all of the ones with two discs - or the ones that have the possibility to import data from other games for those that don't have 'disc' on the name, which is a surprisingly large list. |
|
Never said it was the only one, merely that there are multi-disk ps2 games, since your question was on that I gave an example of such a game |
|
No idea why you are guys are talking on a merged PR from 6 years ago, trying to find multi-disk games, when there is a meta: #4851 |
|
It's because I saw that no-one actually responded to i30817's question, that bugs me when a question goes ignored so I gave the response |
This adds a class (and some helper structures) that implements memory card reads and writes into the host file system instead of a raw memory card file.
I was mostly doing this because I was curious if it was possible to implement the same sort of thing Dolphin added recently for the PS2 as well, and it kinda is, although the way and order the PS2 writes FAT data make it quite difficult at times. I ended up caching all modified pages of data in memory and only flushing to disk when the emulator is shutting down, which isn't great but at least works consistently. Memory Card FAT is recreated whenever it's opened, so you can freely add and remove files from the folder.
It's probably far from perfect and could use some optimization, especially when it comes to finding a good time to flush the in-memory data, but it works quite well for regular use of creating and overwriting saves. Deleting isn't implemented yet. I imagine that savestates could mess with it quite a bit, but that's probably the case with the raw files as well.
Here's a picture of it running in the PS2 file browser: https://cloud.githubusercontent.com/assets/4522237/5193663/b7cb128e-7503-11e4-8cbe-8fade86e678d.png
To test it out, comment out the FileMemoryCard impl in Component_FileMcd and comment in the FolderMemoryCardAggregator instead. Ideally, there should be an option to allow the user to switch between the two modes. A few other small GUI changes would also be needed so you can select folders instead of files as memory cards.
Tell me what you think and if this could be a good addition to the emulator with a bit more polish.