Sewer56 / FileEmulationFramework

Framework for emulating files using Reloaded-II.
GNU Lesser General Public License v3.0
7 stars 8 forks source link

Pak Emulator Fixes #5

Closed AnimatedSwine37 closed 1 year ago

AnimatedSwine37 commented 1 year ago

Some minor fixes for PAK Emulator needed for PAK Merging to work properly in Persona Essentials. (see #12 over there)

Sewer56 commented 1 year ago

I spent some time looking at the problem, as it could potentially be an indicator of a bigger design flaw; figured it wasn't, this was just the first case where an emulated archive could have children.

Anyway, one thing comes to mind:

How would this handle recursively nested archives?

I know that this wouldn't be necessary for this specific use case; but FEF should technically work recursively.

As this is the first case of nested files in an emulated file (surprisingly, I missed out on this in the spec); this will ultimately end up setting a precedent for future implementations.

Per specification here, FEF should allow specializing the paths, so suppose hypothetically a path like the following existed:

Per spec, the user should be able to drop a folder named

To specifically target the child inside init_free.bin.
The issue here now is, child.bin is no longer the first part of the path; so the current code would fail to meet the spec.


With the code that previously existed, the issue was presumably:

And the match failed because init_free.bin\field\script was longer than the original, and we use Contains for the comparison as part of the path resolution logic. (Surprisingly I never considered subfolders in the spec)

Some thinking needs to be done on this end, for emulated files with nested archive structures.

This will probably require an alternative method to Route.Matches (which seeks backslashes, and tries testing on sub paths) and some accompanying unit tests.

Part of this will be deciding how to handle paths like

The concern there is, it's possible that with a wrong implementation, init_free.bin might end up injecting children\child.bin\file.dds inside itself as a subpath, while file.dds is only supposed to be injected into child.bin.

How do we handle that? Can we detect it and ensure there's no accidental injection into the wrong archive?

Do we disallow paths like that outright? And limit users to just e.g. children\child.bin\file.dds?

What if the user inserts init_free.bin\children\child.bin\file.dds anyway? How do we guard against that?

Some discussion and experimentation is needed there. So it's best to bring more people along if possible. This might be a situation where you just try to write some code, write some tests, see what happens.

AnimatedSwine37 commented 1 year ago

Tbh, I think my current "fix" doesn't really fix the problem since I'm checking if the group contains the route's path which If I understand correctly should really be the other way around to properly work with the spec.


With regards to having a path like children\child.bin\file.dds replacing a file in init_free.bin I'm not sure how it'd be possible with the current way everything works for PAK merging.

Basically, Persona Essentials goes through the routes that PAK Emulator finds and then attempts to get any archives from the CPK using their paths (relevant code). If it's possible to specify a valid route without the actual archive name in it we'd potentially have to extract every single archive from the cpks which sounds like a really bad idea.

I don't know how we could go about that without just breaking the spec (or changing it?) and not allowing routes that don't start with the name of the archive.


For detecting nested files and not accidentally injecting them as subpaths I think it'd be fair to just say that any part of a path with a file extension (like child.bin in your examples) is always a file, not part of a subpath. I don't think detecting that and handling it correctly would be too hard.

Sewer56 commented 1 year ago

It's not a big deal for this specific use case; just something to bear in mind for the future.

Only request:

Can you replace regex in this PR? Regex is slow and not as trimmer friendly. You should be able to replicate the functionality with an IndexOf('\') and detecting a file name from there. I'll merge it after and push an update.

I think it'd be fair to just say that any part of a path with a file extension (like child.bin in your examples) is always a file

Sounds reasonable by the way; though there are some rare scenarios of games without file extensions of any kind. Sonic Riders for example is one of them.

AnimatedSwine37 commented 1 year ago

I've removed the Regex now, it should be all good now.

Sewer56 commented 1 year ago

LGTM