[Feature Request] Patch Picker

[brianblakely]

Background

Softpatching represents a significant advancement in RetroArch, shifting the burden of managing content patches away from the file system. However, patch releases have become so popular that you might have dozens of mutually exclusive patches for a single piece of content. This becomes cumbersome to manage, as you must rename or move files around to choose which patch is active.

Requested Feature

Enable users to place patches in the file system one time and then, within RetroArch, select which patches to apply to content before running it.

Proposed Usage

1. Adopt an extension format content.patch-name.ips[N], where .patch-name. is a user-defined key, distinguishing each patch or group of patches. Ex: My Content.Patch Name.ips, My Content.Patch Name.ips1, My Content.Another Patch.ips
2. Within the content view—the interface for setting core associations, adding favorites, etc.—introduce a "Set Patch" menu item. This feature would enable users to choose a patch/patch group from a list based on the patch-name key. Perhaps in the future, a patch hash database could pretty-print patch names automatically.
3. The menu item would also allow a "No Patch" option, making it possible to run the unpatched content, which would be valuable for both users and patch developers in testing.

[hizzlekizzle]

When we've discussed this in the past, the preferred workflow was: name a directory after the ROM and fill it with patches, then add a 'patch' (or 'patch-and-run') entry to the existing 'run' menu where you can choose which to apply.

So, if you have Foo.sfc and Bar.sfc in a directory, you would have a structure like this:

Roms
-|Foo.sfc
-|Foo
--|patch1.ips
--|patch2.ips
--|etc.
-|Bar.sfc
-|Bar
--|patch1.bps
--|patch2.bps
--|etc.

[brianblakely]

That's terrific, and I think it makes sense for saves/states to integrate with this system as well, as patches can often lead to confusing or harmful results for save management.

[i30817]

As long as multiple patches automatic application doesn't get broken (very common in romhacking.net addendum patches)...

If you don't know about it, ra invented a extension to the usual 'name the patch as the rom with a different extension' to apply multiple patches one after the other by naming them the same as the rom but add a number to the patch extension. It's very useful when you have a way to check for updates for patches on romhacking,net (I made one such program for my own use), and you can just replace a patch out of multiple, instead of creating a combined one.

Now. If softpatching worked for xdelta cd patches...

Anyway, since I have multiple such patches and I hate submenus, what I do instead is making hardlinks for the rom, with different names (for the main patch usually, ie "Super Metroid Redesign.sfc" and corresponding patch in another dir for example). A Linux only solution clearly.

But my point with this comment is it would be nice if the solution\workflow hizzlekizzle mentioned above still worked with a 'group' of patches with the same name that are numbered, so are applied one after the other, but only shows the main one (the first), in order not to confuse

Ie, since the subdir solution was obviously chosen in order to not sabotage the current name shadow solution for automatic softpatch and automatic multiple softpatch, it would be nice if the manual choice solution still allowed multiple softpatch. Shouldn't be a problem, just remembering the implementer that wants to build that solution that this also exists.

[brianblakely]

@i30817 The existing "workflow" shouldn't change at all, and directory workflow allows for it as well. Example of multiple patch groups living alongside one another, as well as alongside individual patches:

Roms
-|Foo.sfc
-|Foo
--|My Mod.ips
--|My Mod.ips1
--|My Mod.ips2
--|Another Mod.ips
--|Another Mod.ips1
--|Yet Another Mod.ips
--|And So On.ips

[hizzlekizzle]

Yeah, whether and/or how a hypothetical patch-picker and the existing multi-patch system interact would be up to the implementer to sort out. Sometimes that stuff gets complicated and it's better to just punt and not have them interact at all at first. No way to know until someone starts working on it.

[brianblakely]

@hizzlekizzle I am going to take a look into it over the weekend and see if I can pick this up, unless someone else hits it first.

[i30817]

Don't forget that unfortunately some rom formats cue\bin for instance, already have multiple formats with the same name minus extension, so the patch directory might have to not be exactly the same name as the rom minus extension because of target confusion. This is currently not relevant for cue\bin (RA doesnt support softpatch of it) but might be in the future and there are always other formats using the idea so likely something like this is already happening such as audio extensions for sfc roms or bnes xml formats or something...

The same way, it can't be exactly the same name because filesystems won't allow it.

tl;Dr: you might need a suffix in the directory name and that dir include the extension of the target.

[hizzlekizzle]

yeah, fair points. [gamename]-patches or somesuch might be better.

[i30817]

I really hated typing that comment because something like 'Super Metroid (USA).sfc-patches' is aesthetically displeasing to say the least, but RetroArch is so wide because of the number of emulator cores under its umbrella that its the only thing I could think of that wouldn't conflict or cause some target confusion.

Maybe someone else has a better idea, like another text file format pointing to the patch files or something like that.

On second and third thought....

I guess target confusion is not such a big deal actually since the emulator should know what rom file it's opening and that's the target (except for index files like cue\toc\m3u or compressed files like zip\chd). May not be easy depending on RA frontend code though. I'm a bit worried about certain cases (like a .fig and a .sfc of the same name in the same directory), but filesystems unique names probably take care of that, since it's impossible a track name to be the same as another except if they're the same file, for instance, or in the cases where the extension is not the same, like the .fig and .sfc it's a clear user error to expect the same patches to work.

If any dumping groups are idiotic enough to name tracks the same but using different extensions... Well first it's probably tosec, but even tosec Dreamcast wasn't that stupid (they were stupid enough to name every track the same fo different games though and thus make nearly every cue file have the same checksum and putting games in the same dir overwrite tracks).

Audio extensions for sfc roms are, I think, done by having a directory with the same name minus extension in snes9x, so it would have to be the same directory. But you know, that might not be a problem if the menu to choose only lists certain extensions (it would have to to support multiple patches for the same rom with the number suffix anyway).

Softpatching indirect files will need RetroArch to understand that certain files given to emulators need further processing to check if they have patches (that VFS idea) and this further interaction would have to slot there as a alternate manual location in the allowed files that if it exists asks for user interaction, even if all the current cases are direct. So in a hypothetical future where there is a transparent VFS for patches ideally, the VFS would ask the user for a choice when the emulator tries to load a rom file and only a rom file with a multipatch directory present, instead of the automatic patching it would do if it found a shadow patch file.

Tl;Dr: I'm now convinced that directory name just being the filename minus extension is not a problem IF the file that is going to be patched is provided to the frontend patching mechanism by the core\vfs (hopefully in the future, for indirect files patching) and not scanned for , and in case of a future VFS solution, only patch certain allowed files (for instance, patching subchannel data in a psx emulator when it tries to load it because the filename minus extension happens to match a patch intended for a track is a mistake)....

There is a particular case I found myself hitting 2 times over hundreds of patches where I had to turn a cue\bin with mode 2\2352 to mode1\2048 so both the bin and the cue had to be changed so the multiple patches could get confused about which file to apply, but that can be worked around by just having two cues instead of one + a patch if RA ever gets a VFS for patches working. More generally, it seems it's just a bad idea to patch index files.

[brianblakely]

@i30817 Here is the RFC right now:

• Playlist content will have a Set Patch field. Selecting that field will look for patches/patch groups in the directory [content name]_patches. In your hypothetical example, the directory name would be Super Metroid (USA) _patches. This token is case-insensitive, so there is some flexibility in presentation.

• A patch field will be added to a playlist entry, so you aren’t forced to mess with directories at all. You could make manual playlist entries for your patches, referencing them in whatever location you like, all using soft patching.

[i30817]

Are you saying you plan to add the field to the playlist JSON file entries for persistent memory?

Thats seems like its going to interfere with the rescan playlist functionality. You might have to fix bugs there to prevent a rescan\recreation of a playlist not to nuke a saved patch.

Granted, only the rescan functionality would have that expectation. And because of the weird way the manual scanner is done: #16031

There is already a lot of needless confusion there (id love if you solved that issue in the logical expected way)

[brianblakely]

@i30817

Thats seems like its going to interfere with the rescan playlist functionality. You might have to fix bugs there to prevent a rescan\recreation of a playlist not to nuke a saved patch.

Would you kindly provide some steps to test this?

[i30817]

Let's see. The way the manual scanner does this is a bit weird.

A playlist created by the manual scanner has various extra fields compared to a normal scan one.

The relevant ones being scan_content_dir (activates the ability to refresh a playlist in the playlist management menu or if you press start\long android press while focused in a playlist name in ozone iirc) and the scan_* fields. Which are properties/options of the scan. One relevant one is scan_overwrite_playlist or not.

The difference is if it is false, the refresh option just adds new accepted rom entries, and if it is true, the playlist is first deleted.

You usually WANT to overwrite the playlist to not have to click the other option 'clean playlist' which deletes nonexistent entries from a playlist. Anyway, the overwrite playlist option will probably nuke those extra properties.

Curiously, RetroArch also has a 'validate existing content' option that disappears when you enable scan_overwrite_playlist. I don't get it either... assuming RetroArch UI is the only supported way to edit these files, only one of these would be needed... and you could fix losing extra playlist JSON properties by removing scan_overwrite_playlist and let users use validate existing content if they want to remove deleted things on a refresh. I'm PROBABLY missing something about this, jdgleaver usually thought very exhaustively about options. Maybe it's just premature optimization for not having to test existence of files.

Anyway this is yet another way manual playlists can get needlessly confusing. I'd say 99% of people don't want nonexistent rom files to stay on a rescan\refresh and if they did the default option should be to remove (the exact opposite of what's happening). And even if they did want it two strategies for it are 1 too much.

The point of these options is to make rom management easy, besides all the advantages of manual playlists (there are many, even with how confusing it is see the bitching I did on that other bug report I linked). Just add or delete roms, or index files if your original scan added index files, or to a single directory if you have a core where you're categorizing games by the directory you put them in and scanned different playlists for different dirs, or refresh the .dat file if you're using one for new custom names... Then start RetroArch and long press a playlist and click refresh playlist.

[i30817]

There is also the fact that for some inexplicable and probably dumb reason RetroArch still supports the old style playlists that don't support custom properties (it's a awful line format with a truly atrocious way to tell when the entries stop and playlist properties start... It involves % 6 enough said).

And not just reading, there is a actual option to write it too.

[brianblakely]

@i30817 Thank you very much for that description, I fully understand now. It may be out of scope for this issue to resolve UX gaps in the Manual Scan feature. The idea is to follow the same UX pattern as custom labels (i.e. "Rename" content option). Custom labels are indeed wiped out when a Manual Scan playlist is overwritten. However, if/when Manual Scan behavior changes, it should be possible for patches to be preserved just as well as custom labels.

[i30817]

Changing this would cause that bug, and fixing that would be a opportunity to fix other things. Although fixing the worst problem of playlists (renaming one\creating with a custom name sabotaging thumbnails, manual playlist or not) would be a radical departure, that would finally kill off the older playlist format for good, probably. Good fucking riddance.

Renaming playlists is a kind of important feature because many cores have hundreds of games that don't fit in the same list. For instance scummvm supports both graphics adventures and text adventures, and their images are all in the scummvm thumbnail directory, but it's pretty handy to keep seperate playlists.

I figured out all of these things because I did a fuzzy RA thumbnail downloader, in which I can download thumbnails of whatever server directory I want into whatever playlist name I want, but just renaming in RA or creating a playlist with a custom name in the manual scanner shouldn't sabotage thumbnails. But it does.

Anyway, the refresh manual playlist functionality when that setting is true is the only option when RA deletes a playlist and the user doesn't expect it, so it's the only problem like that you'd have. Well... I'm not sure how JSON compliant RA is when it processes playlists, might be that there may be trouble because it's not using a 'real' JSON parser and instead of preserving unknown JSON properties it deletes them when turning the playlist in memory representation into JSON, so it would have to be done manually whenever a entry is written, I haven't read that code.

[i30817]

Btw your solution wouldn't work for a hypothetical future where RetroArch allows cd softpatching (because tracks are not part of the playlist, only 'index files' (cue, toc, m3u, iso would work because it's a single file format, although isos patches are rare).

Fortunately, if the emulator supports chd, and do the bare minimum work to support parent-child discovery in the same dir (scan the current dir to find a parent chd if one is required), there is already a alternative to softpatch cds in cue\bin format by changing the format...

Won't work for rvz though. And it will however, show all versions of the game in the playlist. The annoying part is converting a xdelta into a 2 parent child chds manually then permitting upgrades. A script should be able to do it, but it's annoying to be forced to create a script and most users will just flay around and the inferior solution will become standard...

[brianblakely]

This feature involves patch discovery, so any routine that applies patches to multi-track disc images should be able to leverage it as well.

[i30817]

Mmm? The problem is that when RetroArch loads a file to send to a core, then tries to patch it before, it would only get a cue file (sometimes a m3u too for one more level of indirection). It's the cores that then load tracks, as far as I know.

So trying to patch a track would do nothing unless you want to scan tracks for playlists and that would only 'work' (badly because even single track dumps sometimes have audio glued as bytes and without the cue that would fail to play music, and because 99% of xdelta patches will only apply to the track containing the executable, not joined tracks).

To build a patch aware cue\bin loader you'd have to change all cores using CDs to use the RetroArch patch service when try to load tracks (or a VFS where the cores aren't aware theyre loading a patched file when they ask for the original and the VFS notices the corresponding patch by whatever mechanism), instead of the current situation where as far I know, the files are patched and sent to cores before loading the core.

Besides that, as I mentioned, the playlist (usually, unless you use the manual playlist, or just iso dumps with all the problems they have), will only point to a cue\m3u\toc\chd\zip, which aren't the actual target of 99% of patches. You can't even standardize on 'first track for cds' because some sega platform first track is not actually the game track.

Some especially irritating game patches even change more than one track (music changes) or require a mode change to iso mode1\2048. Sega console patches are especially annoying because they're done with a custom tool and format that until a few months back didn't work outside windows even in wine.

I'm pretty much tired of waiting for RA to support softpatching of cds. Instead I do a 'reverse hardpatch' to recover the original later for patch upgrades. I'm hoping that cores start supporting parent child chd discovery, but even that is pretty annoying because you have to uncompress the original to create the patched child when a patch updates and even then you need to divide the tracks and find the original cue because chdman doesn't do that for you.

Ideally you'd have a cmd line tool that given a chd, a argument to which track the xdelta applies (all\joined\0, 1, 2,...), a list of xdelta or other formats to apply, would spit out a patched child chd.... but such a thing does not exist. And it's kind of pointless if the cores do not support parent child relationships for chd too (if I was asked how to do it, id suggest looking in the parent dir for a chd with the right sha1 for the parent, because in the same dir, files with the same name - readme, autogenerated program files etc - would get overwritten).

The chd automatic method does mean you can't choose a patch on the RetroArch UI like this method. But it's pretty similar to the current 'copy\hardlink rom and rename rom and patch' method and doesn't depend on RetroArch implementing anything complex, which is like waiting for Godot (just the discovery method in individual cores assuming libchd finally has parent child). It does populate the playlist more, but since I started using m3u for cds Im not as irritated by multiple CDs in playlists.