IOS/FS: Reimplement many functions in a more accurate way #8539
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
leoetlino
force-pushed
the
fs-accuracy
branch
from
10853b5
to
0bca98f
Compare
lioncash
reviewed
Dec 30, 2019
|
I still want a NAND image backend. I know there are people that would appreciate it. I know this fixes the biggest issues, but there are some advantages to a NAND image backend like uh... savestates would be way bigger. I mean, there has to be SOME advantage. |
leoetlino
force-pushed
the
fs-accuracy
branch
from
0bca98f
to
32b32f9
Compare
|
@lioncash Fixed. I've added a new commit for IsPrintableCharacter. @JMC47 One advantage is that you'd be able to use it for testing with IOS, since it's fully compatible with Nintendo's driver. And I guess another one is that it becomes impossible to have a filesystem that goes over the size limit. But IMO the drawbacks far outweigh those benefits. |
leoetlino
force-pushed
the
fs-accuracy
branch
from
32b32f9
to
34947a9
Compare
BhaaLseN
reviewed
Dec 30, 2019
leoetlino
force-pushed
the
fs-accuracy
branch
2 times, most recently
from
3e4364b
to
1c28345
Compare
leoetlino
force-pushed
the
fs-accuracy
branch
2 times, most recently
from
a919a40
to
dface2c
Compare
leoetlino
force-pushed
the
fs-accuracy
branch
from
dface2c
to
f37d4fc
Compare
They will be used in more places than just HostBackend/FS.cpp. Also fix the check and make it accurate while we're at it.
Some official titles rely on implementation details of Nintendo's FS sysmodule and will not work properly if those are changed. Notably, some games and older versions of the System Menu appear to be relying on the order of files returned by FS::ReadDirectory and will either fail to find their save data (for Bolt) or outright crash (for the System Menu). Some titles also actually expect filesystem metadata to be correct. One title that has been confirmed to do this is DQX, which generates paths based on the GID of files within its own title directory. While it is easy to make workarounds for these issues -- and in fact we already do have some for the sysmenu and DQX, having hacks is obviously nonideal and adding yet another hack would be required to fix Bolt -- one that would be even uglier. Furthermore, while it is currently unknown whether any official title cares about permissions, the lack of FS metadata means that we are unable to implement them if that turns out to be desirable or necessary. By adding a FST, we can implement things correctly and solve all those problems without hacks. Apart from DQX, the sysmenu and Bolt, this changeset also fixes the Photo Channel complaining about corrupted system files on the initial launch. This first commit adds the basic structures and functions that are necessary to load, save, query and update our version of the FST. For simplicity, a binary format that is inspired from Nintendo's FST structure was chosen for serialization. It is not expected to ever receive an update. PS: an update on the NAND image backend: A long time ago I had planned to add another FS backend which would be using a NAND image/blob as the storage. While I have already written an implementation that has been tested, solves all the aforementioned issues and more, produces images that are fully compatible with IOS's FS driver, I feel like NAND images raise too many issues: savestate sizes, code complexity and maintenance cost. Since many fixes and additions that are part of that implementation (e.g. FS timings, utility structures, FST) have already been merged or will be submitted as part of this changeset, I will likely not submit the branch.
Prevents /tmp from being cleared unnecessarily; clearing /tmp is normally only done once every time IOS is reloaded.
Previously, the FS root directory would get created as a side effect of calling CreateDirectory during boot (since the implementation was sloppy and used File::CreateFullDir). Since CreateDirectory no longer does that, it is necessary to ensure that the FS root directory does exist by creating it explicitly.
CreateDirectory does not create missing parent directories. If that behaviour is desired, CreateFullPath should be used instead. (These small misuses went unnoticed since the previous implementation of CreateDirectory automatically created parent directories.)
With the CreateFile/CreateDirectory fix and this commit, we can finally return correct results in ReadDirectory and the sorting hack -- whose purpose was to prevent certain versions of the System Menu from crashing -- can be removed too.
Now that all FS functions that create new inodes are properly implemented, we can make GetMetadata actually return correct file metadata rather than giving fixed information. The hack for the DQX installer can also be removed now since our ES and FS keep track of caller UID/GIDs now.
Files cannot be given a different file name, only moved across directories. Add a test for that behaviour and fix the existing RenameWithExistingTargetFile test.
* Test recursive directory deletion * Test "in use" check for both files and directories
ES now uses FS to access the filesystem and FS's ReadDirectory now returns file lists that are correctly ordered.
leoetlino
force-pushed
the
fs-accuracy
branch
from
f37d4fc
to
c02e7de
Compare
|
This breaks Mario Kart Wii initial saves for me. Unable to test subsequent saves. |
|
To clarify the above comment, with a fresh NAND I get this message after it attempts to create the initial save file: I can't progress past this message, so I restart the game and am presented with this: Pressing OK then attempts once again to create the initial save and fails, looping back to the first message, thereby rendering Mario Kart Wii unplayable unless you import a good save or have one from a previous build. |
|
If I install the system menu, boot it and do the initial setup, Mario Kart Wii is able to successfully create its save. |
|
inb4 this is just how the game would act without a properly setup NAND. |
leoetlino
added a commit
to leoetlino/dolphin
that referenced
this pull request
Jan 30, 2020
Fixes a regression from dolphin-emu#8539. CreateDirectory was the correct function to use for creating directories since parent directories already exist and are not owned by the system menu.
|
Thanks for the clarification @Techjar |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some official titles rely on implementation details of Nintendo's FS sysmodule and will not work properly if those are changed. Notably, some games and older versions of the System Menu appear to be relying on the order of files returned by FS::ReadDirectory and will either fail to find their save data (for Bolt) or outright crash (for the System Menu).
Some titles also actually expect filesystem metadata to be correct. One title that has been confirmed to do this is DQX, which generates paths based on the GID of files within its own title directory.
While it is easy to make workarounds for these issues -- and in fact we already do have some for the sysmenu and DQX, having hacks is obviously nonideal and adding yet another hack would be required to fix Bolt -- one that would be even uglier.
Furthermore, while it is currently unknown whether any official title cares about permissions, the lack of FS metadata means that we are unable to implement them if that turns out to be desirable or necessary.
By adding a FST, we can implement things correctly and solve all those problems without adding hacks. In fact, this PR even gets rid of two hacks in the FS code.
For simplicity, a binary format that is inspired from Nintendo's FST structure was chosen for serialization. It is not expected to ever receive an update.
Most of the new code that is added by this PR is additional checks or necessary logic which were missing from the previous implementation. I've also seized this opportunity to even implement permission checks, since obscure IOS features have a history of coming into play at unexpected times...
The last few commits are mostly unit test fixes or additions. All tests passed.
Results
This PR fixes Bolt, which is now able to find its save files:
Two hacks were removed.
Apart from DQX, the sysmenu and Bolt, this PR also happens to fix the Photo Channel complaining about corrupted system files on the initial launch.
Before (initial boot):
After (initial boot):
An update on the NAND image backend
A long time ago I had planned to add another FS backend which would be using a NAND image/blob as the storage. While I have already written an implementation that has been tested, solves all the aforementioned issues and more, produces images that are fully compatible with IOS's FS driver, I feel like NAND images raise too many issues: savestate sizes, code complexity and maintenance cost.
Since many fixes and additions that are part of that branch (e.g. FS timings, utility structures, FST) have already been merged or submitted as part of this PR, I will likely not submit it.