profi200 / open_agb_firm

open_agb_firm is a bare metal app for running GBA homebrew/games using the 3DS builtin GBA hardware.
GNU General Public License v3.0
850 stars 42 forks source link

Automatic ROM Patching #75

Closed spitzeqc closed 1 year ago

spitzeqc commented 2 years ago

Added support for automatic ROM patching for IPS and UPS patches

ROMs can now have patches applied if ".ips" or ".ups" is present. Currently this is achieved by running "patchRom()" within the fixRomPadding function. This is done to allow for patches to be applied before mirroring. Currently autodetecting of saves does not work if the rom gets patched as detectSaveType is run after loadGbaRom, and by then the hash has changed. This could be fixed by either adjusting the logic of oafInitAndRun and loadGbaRom to load rom into memory, detect save type, patch, and (if needed) mirror, or by adding patched rom data to the save database (I believe the latter is a better option as it would also work for pre-patched roms, and it requires less code to be rewritten).

Future revisions/improvements could add patch options in game-specific settings, and possibly optimizing the patch function as it is a bit on the slow side currently (~15 seconds to apply the mother3 translation patch)

profi200 commented 2 years ago

Honestly in the current state i can't merge this. We need to bring down the patch time significantly. 15 seconds is a lot. I see you are using calloc() for very small buffers in many places. This can be changed to stack buffers. And i see you do fRead() calls with very small size. I would cache the patch data for fast access (let's say 512 bytes of cache).

And the save type detection is also a problem. We could assume by default the patched ROM uses the same save type as the original but not sure how reliable this is.

spitzeqc commented 2 years ago

I have gotten ups patch time down to ~2 seconds for mother3 translation, and smaller ups patches seem to load almost instantly.

As for the save detection issue, I'm not sure what the best course of action should be. Moving the patch function to after the save type is detected works (current method of latest push), but I fear any patches applied to classic nes roms (and other roms that get mirrored) may not patch properly. To be fair, my quick search for classic nes patches only brings up emulation/anti-piracy patches so this may not be a real issue in practice. But if there are patches for mirrored roms, the logic would need to be changed (assuming the mirroring messes up patch offsets) unless an alternative solution is found.

EDIT: further testing has reveled that the latest push is not functioning properly ATM, so dont merge

EDIT 2: it appears the rom does patch properly. Perhaps ram did not fully clear before patching, causing issues? (rom does not take all 32MB)

profi200 commented 2 years ago

What is not functioning properly?

spitzeqc commented 2 years ago

Patching Tomato Adventure with a ups translation patch (converted from bps) created distortion on the title screen. However patching was functioning properly before, and after powering off and waiting a minute or so it worked properly again which is why i believe it is ram related. I have not been able to reproduce the issue. Further investigation may be needed, but at present patching appears to be functioning properly

spitzeqc commented 1 year ago

I have begun testing with classic nes series crack patches (these were the only ones I could find). So far, the Dr. Mario, Metroid, and Icd Climbers patches match their prepatched counterparts without adjusting fixRomPadding()'s logic (though neither autopatched or prepatched versions run properly, I'm getting "Game Pak Error" for both). More testing is needed to confirm however, as these patches may only need to patch data that does not get moved around.

It may be a good idea to distribute a prerelease version (with debug output) with the latest commits to see if patching is truly working for classic nes games, along with gathering a larger patch sample size

profi200 commented 1 year ago

The reason it's showing this error is because there is something wrong with the save type. These specific games are working using the save type database. Possibly the AP patches patch these to SRAM saving or some other nonsense.

profi200 commented 1 year ago

There are still conflicts. It looks like you are trying to PR commits which already got merged (the backlight stuff). No idea how this happened.

spitzeqc commented 1 year ago

Ok, I think I have fixed the issue with the backlight merge stuff