icculus / physfs

A portable, flexible file i/o abstraction.
https://icculus.org/physfs/
zlib License
553 stars 98 forks source link

PhysicsFS 4.0 plans... #41

Open icculus opened 2 years ago

icculus commented 2 years ago

This is preliminary, but here's some initial wishlist things I'd like to do:

Fothsid commented 2 years ago

Built-in UDF .iso support would be neat.

icculus commented 2 years ago
Green-Sky commented 2 years ago
  • Some sort of way to signal that Emscripten should persist some files and not others (MEMFS vs IndexedDB), and possibly a "commit" API (needed by Emscripten and Nintendo Switch).

I worked around that for emscripten by mounting IDB, and setting PhysFS write dir to the same directory. (iirc)

vectorstorm commented 2 years ago

file move/rename support.

(I’m currently using std::filesystem::rename() for this in order to build an “atomic file write” functionality, so that I don’t end up with partially-written files in the event of crashes/kills/power loss in the middle of saving a file. It’d be really handy to have file move/rename functionality provided natively by PhysFS, or even have the whole “atomic file write” functionality itself be directly provided! Maybe that usage would tie into the “commit” API discussed higher up in this thread?)

vectorstorm commented 2 years ago

Multiple write directories; they'll interpolate and mount and such like the search path does

Related to this, one current pain point for me with PhysFS has been how the write path is treated like a separate filesystem which doesn't necessarily match the structure of the main virtual filesystem.

In my game, I want my writeable files to be mounted somewhere outside of the root of my game's main virtual filesystem, so that I can know for certain that files written by the game at runtime cannot accidentally override files which were provided by the game; I mount the whole write directory over into a side "/user/" virtual directory in the search path's virtual filesystem.

This means that my code has to always know and remember that if I write a file out to "/file.txt", that same file would need to be addressed as "/user/file.txt" to read it back (because that's where the write directory is mapped within the seek path's virtual file system). This does work in PhysFS right now as long as I'm diligent about it (I've written a whole wrapper around PhysFS to try to hide this implementation detail from game code), but it's obviously a bit of a cognitive load that feels like maybe PhysFS could simplify.

Proposal: maybe this whole thing could be implemented by having PHYSFS_mount() include some configuration flags that say for each mount point whether that path can be written into; maybe turn its current append?1:0 parameter into a real bitfield with named parameters that can be bitwise-or'd together to denote which mounting points should allow writes or other behaviours. Maybe something like this:

#define PHYSFS_READ (0) // implicit
#define PHYSFS_LOWPRIORITY (0) // implicit if HIGHPRIORITY isn't set.
#define PHYSFS_HIGHPRIORITY (1<<0) // matches "1 == append to search path" current functionality
#define PHYSFS_WRITE (1<<1)
#define PHYSFS_READWRITE (PHYSFS_READ|PHYSFS_WRITE)

PHYSFS_init(argv[0]);
PHYSFS_mount("MyZippedData.zip", "/data", PHYSFS_HIGHPRIORITY );
PHYSFS_mount( SDL_GetPrefPath(companyName, title), "/preferences", PHYSFS_READWRITE | PHYSFS_LOWPRIORITY);
PHYSFS_mount( SDL_GetSaveGamePath(companyName, title), "/saves", PHYSFS_READWRITE | PHYSFS_LOWPRIORITY);
  // SDL doesn't actually currently provide a 'SDL_GetSaveGamePath()' API, but just as a fictional example of a setup with two write directories.

PHYSFS_File *file;
file = PHYSFS_openWrite("myfile.txt"); // fails, not in a writable directory.
file = PHYSFS_openWrite("/preferences/myfile.txt"); // succeeds
// (...)
PHYSFS_close(file);
file = PHYSFS_openRead("/preferences/myfile.txt"); // also succeeds, opens the same file saved above.
file = PHYSFS_openWrite("/data/myfile.txt"); // fails, as there's no 'WRITE' path mapped here. 
file = PHYSFS_openWrite("/preferences/../myfile.txt"); // fails, as this resolves into a filepath which doesn't have a anything mounted.
kleszcz commented 2 years ago
  • Remove CD-ROM support (entry point will remain but never find any discs).

Does it mean that physical media for Xbox Series X and PS5 won't be handled?

icculus commented 2 years ago

Does it mean that physical media for Xbox Series X and PS5 won't be handled?

Mmm...that's a good question. Maybe we'll leave this API after all. Let me think on it.

icculus commented 2 years ago

In my game, I want my writeable files to be mounted somewhere outside of the root of my game's main virtual filesystem, so that I can know for certain that files written by the game at runtime cannot accidentally override files which were provided by the game; I mount the whole write directory over into a side "/user/" virtual directory in the search path's virtual filesystem.

I'm probably misunderstanding the problem, and if so I apologize, but why don't you mount the write dir's path last, to "/", and put it at the end of the search path, so it never overrides anything and you don't have to manage it as a separate /user path when reading?

(Or maybe, once I build instances, you have a second instance that only has the write path mounted as "/", so if PHYSFS_openRead() fails, you can just try the second instance, so it's extremely isolated against accidental access. Maybe?)

vectorstorm commented 2 years ago

I'm probably misunderstanding the problem, and if so I apologize, but why don't you mount the write dir's path last, to "/", and put it at the end of the search path, so it never overrides anything and you don't have to manage it as a separate /user path when reading?

Yeah, apologies, I really should have given a better description of what I was actually trying to accomplish!

Overall goal: our game submits crash reports when crashes happen, and one piece of information I want to have inside those crash reports is whether the user was running using just the game's shipped data (we're calling this "pristine" data), or whether they had local modifications at the time of the crash. That's been useful knowledge for us during debugging; more than once we've seen crashes which occurred due to particularly buggy mods, so when we're looking at a crash report it's handy to know whether a mod is in play!

We have a canonical way to create and install mods so that we'll be aware that mods are in use, but if our write directory is mapped to "/", then the player can just drop mods directly into the write directory and the game can't spot that they aren't running with pristine data. That's what I was trying to solve by mounting the write directory under "/user"; there's no shipped-with-the-game data over there, so stuff copied into the write directory can't invisibly modify or add to the game's own data; stuff has to go into a 'mod' directory to do that (or modify the game's archives directly, which we can detect)

(Or maybe, once I build instances, you have a second instance that only has the write path mounted as "/", so if PHYSFS_openRead() fails, you can just try the second instance, so it's extremely isolated against accidental access. Maybe?)

I started thinking about this after I wrote my earlier comment, and I think this would probably work for me as well. And the more I think about it, the more I'm thinking that it might actually be more what I want than what I wrote initially; having that kind of hard separation between game data and user data as entirely separate instances just makes the division between them stronger. I can map mods into the "game data" tree and the prefs directory into the "user data" tree and then I don't have to worry about conflicts in either direction.

I think I'd have to expose the idea of having multiple filesystems to game code; I don't think I can hide an abstraction that large entirely inside the engine. So with that approach it's probably more of a "next game" sort of upgrade for us, rather than something that could be upgraded to in the middle of a project.

edubart commented 2 years ago

I wish the issue https://github.com/icculus/physfs/issues/13 would be fixed for PhysFS 4.0. Because I've tried before to use PhysFS to load multiple assets in parallel, and then found out it didn't speed up things (actually it made loading slower), details of why that happens is mentioned in the blog post of the issue.

NebularNerd commented 2 years ago

Is #18 under consideration? As mentioned if you have a file inside a zip then change it (for example a default config file), the change is wrote to the overlay. However the change is not seen and only the original file is picked up, ideally the overlay version should override the .zip original.

Currently the only solution is to see what file is changed then delete it from inside the .zip to then let the new one be seen.

UPDATE/EDIT: #18 appears to be an implementation issue rather than a PhyFS issue.

icculus commented 2 years ago

Does it mean that physical media for Xbox Series X and PS5 won't be handled?

Mmm...that's a good question. Maybe we'll leave this API after all. Let me think on it.

Physical media is not accessible to PS5 games, even if the game was installed from disc (a disc-based game is installed to the internal SSD before it can run, the same as if it were downloaded, and after that as far as the game is concerned, there is no disc ever).

I assume this is not true for Xbox, but I don't actually know yet.

icculus commented 2 years ago

PhysicsFS 4.0 Wishlist item:

I think for the most part it's just str and mem functions. Probably won't slot dlmalloc in here, but make it so you can build PhysicsFS without a reference to malloc() where the app must specify their own allocator at startup.

icculus commented 2 years ago
iryont commented 2 years ago

I wish the issue #13 would be fixed for PhysFS 4.0. Because I've tried before to use PhysFS to load multiple assets in parallel, and then found out it didn't speed up things (actually it made loading slower), details of why that happens is mentioned in the blog post of the issue.

I came here to actually say the same thing and then I read your post.

Anyway, I believe this is the upmost important thing, even for current PhysicsFS 3.0.

icculus commented 2 years ago

Anyway, I believe this is the upmost important thing, even for current PhysicsFS 3.0.

I agree, although the most immediate workaround is to not use the callback enumeration mechanism, so you end up with a complete list of files before starting to process data, and then it won't block on the mutex, and that can be done by the app right now.

I suspect the correct fix inside PhysicsFS, for 3.x if not in general, is to have each archiver promise thread safety directly, which might mean duplicating the archive file handle in the same way we do when opening individual files within it, so it can seek around to enumerate without interfering with other operations that might occur, but I haven't had a moment to untangle the whole mess yet.

iryont commented 2 years ago

Anyway, I believe this is the upmost important thing, even for current PhysicsFS 3.0.

I agree, although the most immediate workaround is to not use the callback enumeration mechanism, so you end up with a complete list of files before starting to process data, and then it won't block on the mutex, and that can be done by the app right now.

I suspect the correct fix inside PhysicsFS, for 3.x if not in general, is to have each archiver promise thread safety directly, which might mean duplicating the archive file handle in the same way we do when opening individual files within it, so it can seek around to enumerate without interfering with other operations that might occur, but I haven't had a moment to untangle the whole mess yet.

I suppose the main problem is reading files from the same archive (handle) in multiple threads (e.g. assets.zip). Just like you said, having duplicated archive file handle could solve this issue once for all.

MikuAuahDark commented 1 year ago

I'd love if PhysFS is able to mount Android content:// URI that's retrieved from ACTION_OPEN_DOCUMENT_TREE intent (for directories) or being able to create PhysFS_IO from an Android content:// URI (for files).

icculus commented 8 months ago

Split out public header: #71