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

Load SD card contents from a folder #10525

Closed
wants to merge 4 commits into from
Closed

Conversation

stblr
Copy link
Contributor

@stblr stblr commented Mar 21, 2022

At the moment, to use the Wii SD card functionality in Dolphin, users have to use the command line or even an external application. This PR adds functions to convert between a FAT32 image and a folder, and exposes them in the Tools menu of the Qt UI.

This has been briefly tested with OHBC (libfat) and mkw-sp (FatFs).

@iwubcode
Copy link
Contributor

Thanks for doing this!

The ability to convert, while nice, seems unnecessary for the end user to hassle with. I think better would be to just have a folder (outside the NAND) that would be used to load the files.

@AdmiralCurtiss
Copy link
Contributor

This is an excellent feature to have, thank you.

I agree that it would be more ideal if the user could just point at a folder and that gets compiled to a sd.raw automatically on boot, but I also don't mind leaving that out for now and putting it in a separate PR if that becomes too complicated. The ability to convert from/to an sd.raw is worthwhile to have by itself.

Since we still use Visual Studio project files for building on Windows (no one wants to actually be the one to force the switchover, it seems) you'll have to add the build steps to that one too -- though I'm sure someone else can do that for you if you're not familiar with the format.

@stblr
Copy link
Contributor Author

stblr commented Mar 22, 2022

I agree that it would be more ideal if the user could just point at a folder and that gets compiled to a sd.raw automatically on boot, but I also don't mind leaving that out for now and putting it in a separate PR if that becomes too complicated. The ability to convert from/to an sd.raw is worthwhile to have by itself.

I made it manual because I was afraid that it could break for some users in certain cases. I don't think the conversion menu would be needed anymore if I change it to automatic, which would make things a little simpler.

Since we still use Visual Studio project files for building on Windows (no one wants to actually be the one to force the switchover, it seems) you'll have to add the build steps to that one too -- though I'm sure someone else can do that for you if you're not familiar with the format.

I'm going to setup a VM to do it and will ask for help if needed.

@stblr
Copy link
Contributor Author

stblr commented Mar 23, 2022

Updated the PR to make the conversion automatic (but it will only happen if an SD folder exists, which means existing user installs won't be broken).

Regarding Windows, I tried to setup something but at least on my VM it was failing to find the FatFs symbols.

@Pokechu22
Copy link
Contributor

The following change seems to fix it:

diff --git a/Externals/ExternalsReferenceAll.props b/Externals/ExternalsReferenceAll.props
index 38ec0bd827..7df4eaacbe 100644
--- a/Externals/ExternalsReferenceAll.props
+++ b/Externals/ExternalsReferenceAll.props
@@ -34,6 +34,9 @@
     <ProjectReference Include="$(ExternalsDir)enet\enet.vcxproj">
       <Project>{cbc76802-c128-4b17-bf6c-23b08c313e5e}</Project>
     </ProjectReference>
+    <ProjectReference Include="$(ExternalsDir)FatFs\FatFs.vcxproj">
+      <Project>{3F17D282-A77D-4931-B844-903AD0809A5E}</Project>
+    </ProjectReference>
     <ProjectReference Include="$(ExternalsDir)FreeSurround\FreeSurround.vcxproj">
       <Project>{8498f2fa-5ca6-4169-9971-de5b1fe6132c}</Project>
     </ProjectReference>

(dolphin-emu.sln also uses ..\Externals\FatFs/FatFs.vcxproj where I think that should be all backslashes, but that doesn't seem to be the source of the issue.)

@stblr
Copy link
Contributor Author

stblr commented Mar 24, 2022

Applied, thanks.

@stblr stblr changed the title Make it possible to convert between sd.raw and a folder from the Dolphin UI Load SD card contents from a folder Mar 24, 2022
@AdmiralCurtiss
Copy link
Contributor

So, wait, am I reading this right? You reused the previous SD card image path as the target path for the directory packing? That seems a bit dangerous for users who already have an sd.raw setup, and also a bit unintuitive. I would have expected a toggle somewhere between using a folder (which then packs to an arbitrary temporary path, ignoring the configured sd.raw path) and using a raw SD card image file.

I also think that the previous feature of un/packing a folder manually would be a nice-to-have in addition to the auto-packing (for, say, migrating an existing sd.raw to a new folder), but I dunno how other people feel about that, it is a bit of a feature bloat at some point.

@JMC47
Copy link
Contributor

JMC47 commented Mar 25, 2022

Something that might be interesting is the ability to synchronize SD card folders/generate new ones for stuff like netplay, especially now that syncing wouldn't require syncing a gigantic single file. That's for after this Pull Request obviously, but we should make sure that two identical folders sync up on netplay still.

@stblr
Copy link
Contributor Author

stblr commented Mar 25, 2022

So, wait, am I reading this right? You reused the previous SD card image path as the target path for the directory packing? That seems a bit dangerous for users who already have an sd.raw setup, and also a bit unintuitive. I would have expected a toggle somewhere between using a folder (which then packs to an arbitrary temporary path, ignoring the configured sd.raw path) and using a raw SD card image file.

How would the UI be for that one? A single path that can be set to either a folder or an image, or two paths plus a toggle?

I also think that the previous feature of un/packing a folder manually would be a nice-to-have in addition to the auto-packing (for, say, migrating an existing sd.raw to a new folder), but I dunno how other people feel about that, it is a bit of a feature bloat at some point.

So the same as the previous version, except that both the folder and the image would be manually specified by the user?

@Pokechu22
Copy link
Contributor

That seems a bit dangerous for users who already have an sd.raw setup, and also a bit unintuitive.

If I'm understanding things correctly, the packing only happens if no sd.raw file currently exists. Which is safe... but not intuitive, so not ideal.

I personally would rather have the image get packed on startup and unpacked (and deleted?) when emulation stops, so that the SD folder is the only source of data if that mode is in use. My use-case is that I have hardware tests that generate images in a folder on the SD card, and currently I take those images out by extracting the raw SD card with 7-zip, but it would be convenient to have them just appear in a folder that I can then move them out of when I'm done with them (rather than having to occasionally delete the entire raw SD card when it starts getting full). Though that's probably an unusual use-case.

@iwubcode
Copy link
Contributor

iwubcode commented Mar 25, 2022

personally would rather have the image get packed on startup and unpacked (and deleted?) when emulation stops, so that the SD folder is the only source of data if that mode is in use

Just to chime in, this is exactly what I expected from such a feature as well.

--

Maybe if on first time load, if the folder is empty but there is a sdcard.raw, the files are first unpacked to the SDcard folder. Then normal operation continues (when the game ends the sdcard.raw is deleted)

@AdmiralCurtiss
Copy link
Contributor

AdmiralCurtiss commented Mar 26, 2022

Yeah, that's pretty much what I'd expect too. User picks Folder, we automatically pack the folder to a raw file during emulation and if modified unpack it again afterwards. The user doesn't even need to be aware that there's a file happening in the meantime -- in fact, we could just pack to memory and the behavior should be identical from the user point of view.

UI-wise, I would put a new panel in the Wii section, move the 'Insert SD card' and 'Allow writes to SD card' checkboxes in there, then have two radio buttons for File and Folder (or alternatively: a dropdown with 'None', 'File', 'Folder', similar to GC memory cards; if you do this remove the 'Insert SD card' checkbox and have it be implicit from the dropdown state) and a Path selector under each radio button (or under the dropdown).

@Leseratte10
Copy link
Contributor

Leseratte10 commented Mar 31, 2022

Won't this cause data loss when Dolphin crashes or is killed while a game is running?
A folder is bundled up into a FAT32 image, that image is mounted into the emulated game. The game writes a file to the SD card. Then Dolphin happens to crash or gets killed so it can no longer unpack that file back to a folder structure.

Now at the next boot / launch, either all changes the application did to the SD card are gone (if Dolphin just packs a new image), or the changes the user made to the SD card folder in the meantime (prior to restarting Dolphin) are gone (if Dolphin regognizes that it's crashed and un-packs the old image).

If possible (no idea if it is), it would be better to "emulate" a FAT32 image on the fly so that A) writes by the emulated software will immediately be cloned to the folder, and B) writes to the folder will immediately be cloned to the emulated image.
If that is not possible, I'd prefer the solution where the user has to manually decide to convert folder to image and vice versa. Which, if I understand it correctly, is how this PR currently implements it.

@JosJuice
Copy link
Member

Perhaps we could do what we do with GameCube memory cards, and periodically flash to disk (to the raw file in this case) after the game has written data and then not written any data for a few seconds? Immediately flushing sounds troublesome.

@AdmiralCurtiss
Copy link
Contributor

Okay, I'm starting to feel like we're bloating this thing here a bit. Maybe we should walk back, get just the manual user-driven conversion merged, and then build on top of that for a more user-friendly 'use folder as SD card' feature in another PR?

@NintendoManiac64
Copy link

It may be worth mentioning that melonDS basically has this feature already as of v0.9.4 - it may or may not be a good idea to take a look at how it does things.

(fun fact: the sd card images that melonDS creates are 100% compatible with Dolphin if you just rename the file to "sd.raw'")

@Leseratte10
Copy link
Contributor

(fun fact: the sd card images that melonDS creates are 100% compatible with Dolphin if you just rename the file to "sd.raw'")

That's probably because it's not really a special format, it's just an image of a FAT32 filesystem. You could flash that file onto an actual SD card using dd or any other image writer and use that on an actual Wii, too. Or make an image from your existing Wii SD card.

@AdmiralCurtiss
Copy link
Contributor

AdmiralCurtiss commented Apr 16, 2022

Alright, I checked out the melonDS implementation, and it makes a lot of sense if you present it like this:

dsi

Enable DSi SD card is a central 'SD inserted' switch. If checked you can then pick a path for the image file, an image size (there's also an Auto option), and whether it's read-only or not -- we already have all these options, but not organized nicely like this. All the Sync SD to folder checkbox (which enables the path box next to it if checked) does is add a Folder -> Image syncing process on emulation start, and then the inverse Image -> Folder syncing on emulation end. Pretty straightforward.

IMO we could more or less copy exactly this set of options, probably in the Wii settings tab. I would also add two extra buttons to trigger the Image <-> Folder syncs immediately (since users may already have an SD Image they want to keep using with the new Folder syncing format), but otherwise just take this as-is.

Is @stblr still around, do you want to tweak this PR to work like that? Otherwise I might take this PR and adapt it.

@AdmiralCurtiss
Copy link
Contributor

FYI, I've started adapting this at https://github.com/AdmiralCurtiss/dolphin/tree/sdcard-folder-sync. I'll open a new PR once it's functional.

@AdmiralCurtiss
Copy link
Contributor

I've rebased and adapted this at #10590. Please add further comments there.

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