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

NANDImporter: Various improvements and cleanup #10322

Merged
merged 6 commits into from Mar 2, 2022

Conversation

Starsam80
Copy link
Contributor

A collection of fixes of things I've noticed over the years. There are still a few things I would like to fix, but I figured this would be a good starting point before I make any bigger changes. Future changes include keeping the FST metadata intact (fst.bin) and some integration with DolphinTool.

I've tried to keep each commit separate and reviewable, but I can squash it into a single one if that's preferred for small changes like this.

Source/Core/DiscIO/NANDImporter.cpp Outdated Show resolved Hide resolved
Source/Core/DiscIO/NANDImporter.cpp Outdated Show resolved Hide resolved
Source/Core/DiscIO/NANDImporter.cpp Outdated Show resolved Hide resolved
@Starsam80
Copy link
Contributor Author

Rebased and fixed what would have caused a build error on newer versions of fmt.

Source/Core/DiscIO/NANDImporter.cpp Outdated Show resolved Hide resolved
Source/Core/DiscIO/NANDImporter.cpp Outdated Show resolved Hide resolved
Source/Core/DiscIO/NANDImporter.cpp Outdated Show resolved Hide resolved
Source/Core/DiscIO/NANDImporter.cpp Outdated Show resolved Hide resolved
Source/Core/DiscIO/NANDImporter.cpp Outdated Show resolved Hide resolved
return false;
}

INFO_LOG_FMT(DISCIO, "Using superblock version {:#x}", m_superblock->version);
Copy link
Contributor

Choose a reason for hiding this comment

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

It might still be useful to log the position/index for the superblock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. I think anybody that needs that information could easily find it with just the version that is printed and comparing it to the previous logs. Feel free to correct me if you think otherwise

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, true, each one is logged beforehand, and that is definitely enough information as long as the version is unique (and if it's not unique, it's still enough information based on the code)

`uid` is a u32, not a u16. Also, everything is big endian, so we
can simplify the code a little bit.
It also simplifies the code flow, as it no longer goes backwards
through the filesystem chain.
Create a struct describing the superblock layout and use it directly
without needing to specify offsets and such.
There is no need to constantly reset the key for every file entry.
Copy link
Contributor

@Pokechu22 Pokechu22 left a comment

Choose a reason for hiding this comment

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

All changes look reasonable. I haven't done any testing myself.

@leoetlino leoetlino merged commit 666db19 into dolphin-emu:master Mar 2, 2022
@Starsam80 Starsam80 deleted the nand branch March 2, 2022 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants