TASEmulators
usable movie sram
Users should be able to extract the sram from a movie file and use it, and be able to replace the sram in a movie.
Check if completed:
- [x] I have run any relevant test suites
- [x] I, the commit author, have read the licensing terms for contributors (last updated 2024-06-22) and am compliant
I'm personally indifferent on whether or not sram should be compressed, but won't this require a version bump?
And if we do format changes, we should ideally bundle as many of them into one version change as possible, like #3734 or #2090 and perhaps renaming the text files that are actually json to .json already.
Yeah, I prefer this just being a version bump instead of just creating a new file.
I made it use a different file name, so that it can see which is there. That means old files can still be loaded, which I think it valuable. If there will be other changes that cannot be backward-compatible then yeah this could be bundled with that and then not bother with making this backward-compatible.
Was the filename changed when Zstd compression was added?
And keep in mind that migrating movies from older versions isn't something we claim to support.
Was the filename changed when Zstd compression was added?
No.
And keep in mind that migrating movies from older versions isn't something we claim to support.
But when it's easy to make something backward-compatible, I will.
I changed my approach for making movie SRAM usable/editable. to also address #3734. All zstd compressed files are now given an additional .zst extension. So MovieSaveRam.bin is now MovieSaveRam.bin.zst. These can be easily decompressed with either Windows' File Explorer or 7-Zip File Manager, which makes the uncompressed content readily available. When loading, if a .zst version is not found it will look for a non-zst version and use that if found. This makes replacing the MovieSaveRam file easy.
This does not address #2090 or update JSON files to actually have .json extension. Do we want to do both of those along with this version bump?
IMO the
.binshould take precedence over.bin.zst
I just made the most likely to exist one be the first it checked, but I think your suggestion makes sense. Although it might also make sense to fail if they both exist and tell the user they should delete one, so that there's no ambiguity.
I realized that the ZipStateLoader in master doesn't care at all what the file extensions are, which means we could change the extension of JSON files to .json with no real consequences. So I modified the commit to keep that behavior, and then changed file extensions to .json.
It also now throws if there are two files with the same name minus extension, except in the case that one is .zst in which case uncompressed takes precedence.
And why not go ahead and zstd compress the text/json files too? Since users can still access them, edit them, and replace them.
Can this be merged? Tagging @CasualPokePlayer since he's been requested as a reviewer. (I did fix the thing you already commented on.)
Which two issues would that be?
Version bump: This does make a version bump. .bin take precedence over .bin.zst: Yes, it does that now.
Mentioned GitHub issue "framebuffer isn't easily accessible": Resolved Mentioned GitHub issue "crashes when loading savestate with different sync settings": I think this is unrelated and doesn't need a version bump, but I can look into it if desired.
re: #2090, the state just needs to include SyncSettings.json, which is then checked before loading a state. Whether that's done by comparing sync settings objects or comparing JSON strings doesn't really matter for UX. It can be in a separate PR.