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

PSP2: Simplify the filesystem code #2459

Merged
merged 1 commit into from Sep 15, 2020
Merged

PSP2: Simplify the filesystem code #2459

merged 1 commit into from Sep 15, 2020

Conversation

@bgK
Copy link
Member

bgK commented Sep 13, 2020

The Vita SDK gained support for dirent at the end of 2017. Our copy
is no longer necessary.

DrivesPOSIXFilesystemFactory allows to specify the contents of the
pseudo-root file system node. There is no need to hardcode them in
posix-fs.cpp anymore.

I have tested this on a Vita. Seems to work fine.

The Vita SDK gained support for dirent at the end of 2017. There is no
need to have our own version anymore.

DrivesPOSIXFilesystemFactory allows to specify the contents of the
pseudo-root file system node. There is no need to hardcode them in
posix-fs.cpp anymore.
@bgK bgK requested a review from rsn8887 Sep 13, 2020
@sev-
Copy link
Member

sev- commented Sep 13, 2020

If you want this to be in 2.2.0, then there are only a few hours left before the tagging.

@bgK
Copy link
Member Author

bgK commented Sep 13, 2020

If you want this to be in 2.2.0, then there are only a few hours left before the tagging.

This is just a cleanup, there is no point in including it in the 2.2.0 release.

@rsn8887
Copy link
Contributor

rsn8887 commented Sep 14, 2020

I am wondering if this could cause a bug on Vita similar to a bug that plagued PSP, 3DS and Switch for a while. The bug was due to an fseek in The Dig (and maybe other games) causing crashes:
See
https://bugs.scummvm.org/ticket/11342
and
https://bugs.scummvm.org/ticket/11384

The fix to these bugs was to use the ScummVM internal filesystem implementation, instead of the respective native SDK-provided one.

I think it might be good to carefully check that the changes in this PR don't cause problems similar to the above cited bugs that have been fixed in those other platforms.

@bgK
Copy link
Member Author

bgK commented Sep 14, 2020

This PR does not change the file stream implementation, it only changes the listing of files and folders. I have tested the scene that showed the issue on the other portable console platforms in The Dig. It still works fine.

@rsn8887
Copy link
Contributor

rsn8887 commented Sep 14, 2020

Ok then I am all for it.

@bgK bgK merged commit 1c5f923 into scummvm:master Sep 15, 2020
5 checks passed
5 checks passed
Windows (win32, x86-windows, x86, --enable-faad --enable-mpeg2 --enable-discord --disable-fribidi...
Details
Windows (x64, x64, x64-windows, --enable-faad --enable-mpeg2 --enable-discord --disable-fribidi, ...
Details
Windows (arm64, arm64, arm64-windows, --enable-faad --enable-mpeg2 --enable-discord --disable-fri...
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deepcode-ci-bot Well done, no issues found!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.