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

WiiSave: Use new filesystem interface #7059

Merged
merged 3 commits into from Jun 3, 2018

Conversation

leoetlino
Copy link
Member

Migrates WiiSave to the new filesystem interface.

The first commit is straightforward: it just replaces all of the FileUtil/File calls with IOS/FS in NandStorage.

The second commit makes WiiSave use the file permission information that is returned by FS or stored in the save header. Two functions, GetFsMode and GetBinMode, were added to convert the permission values between the two formats. Files will now be saved and restored with non-hardcoded, correct values.

Copy link
Member

@BhaaLseN BhaaLseN left a comment

Choose a reason for hiding this comment

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

I've considered commenting on that tuple to become a struct with named members (and perhaps be shared with FS::Metadata), but it might not be worth the extra code (that can also be saved by using structured bindings; once we can at least). Also, since it seems to be constexpr right now (does that really work in practise?) a struct might get in the way.

@@ -204,8 +205,9 @@ class NandStorage final : public Storage
return false;

const std::string banner_file_path = m_data_dir + "/banner.bin";
const auto file = m_fs->CreateAndOpenFile(*m_uid, *m_gid, banner_file_path, FS::Mode::ReadWrite,
FS::Mode::ReadWrite, FS::Mode::ReadWrite);
const std::tuple<FS::Mode, FS::Mode, FS::Mode> modes = GetFsMode(header.permissions);

This comment was marked as off-topic.

@leoetlino
Copy link
Member Author

Hmm, adding a Modes struct sounds like a good idea, as this is not the only place where we take a triplet of modes. Changing functions to take just a Modes could cut down on argument repetition. Thanks for the suggestion, I'll do this soon

@BhaaLseN
Copy link
Member

BhaaLseN commented Jun 3, 2018

If it plays nice with the constexpr there, sure, go for it!

As suggested here: https://dolp.in/pr7059#pullrequestreview-125401778

More descriptive than having a std::tuple of FS::Mode, and lets us
give names to known triplets of modes (like in ES). Functions that
only forward mode arguments are slightly less verbose now too.
@leoetlino
Copy link
Member Author

@BhaaLseN: pushed another commit with the Modes struct change

Copy link
Member

@BhaaLseN BhaaLseN left a comment

Choose a reason for hiding this comment

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

👍

@leoetlino leoetlino merged commit 8fce18e into dolphin-emu:master Jun 3, 2018
leoetlino added a commit to leoetlino/dolphin that referenced this pull request Jun 4, 2018
All paths in SaveFile are relative to the title data directory, not
absolute. Fixes an accidental regression from 5.0-7988 (PR dolphin-emu#7059).
Felk pushed a commit to Felk/dolphin that referenced this pull request Jul 18, 2018
All paths in SaveFile are relative to the title data directory, not
absolute. Fixes an accidental regression from 5.0-7988 (PR dolphin-emu#7059).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants