[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.