marcrobledo / RomPatcher.js

An IPS/UPS/APS/BPS/RUP/PPF/xdelta ROM patcher made in HTML5.
https://www.marcrobledo.com/RomPatcher.js/
Other
723 stars 141 forks source link

Fix: UPS patch's cut glitch #40

Closed laqieer closed 2 years ago

laqieer commented 2 years ago

Fix the glitch that cut the end of the file if it's larger than the changed file patch was originally created with like NUPS does, because many patch creators are using it to create UPS patches, and they won't apply correctly without this fix. It also fixes mismatching results with NUPS in the same filesize.

marcrobledo commented 2 years ago

Sounds good! Can you provide an example patch that currently fails without your fix?

laqieer commented 2 years ago

Sounds good! Can you provide an example patch that currently fails without your fix?

marcrobledo commented 2 years ago

I see... But I am not sure which one is doing wrong implementation: Rom Patcher JS or NUPS?

  1. The first Burdened Crown v2.2.0.ups creates a valid 22485048 bytes ROM file.
  2. However, when trying to validate the source ROM for the second Burdened Crown v2.2.0 Blue (Apply to Patched ROM).ups, it fails. That's because the patch was probably not created with the patched ROM. It is looking for a 22450244 bytes ROM, which is a few kB smaller than the real thing.
  3. Despite not validating, if I try to patch it, your fix really works and adds the missing data. However, the root of the problem is not the patcher, but how the patch was created beforehand.
laqieer commented 2 years ago

NUPS does it intentionally. This comes from its README:

V 1.2
-Fixed the glitch that cut the end of the file if it's larger
 than the changed file patch was originally created with.

It is an implementation of UPS format's undefined behaviour when validation is ignored by intent, so there is no right/wrong implementation here.

The principle in such cases is:

3. However, the root of the problem is not the patcher, but how the patch was created beforehand.

That is "the root of the problem" in technique, and let's seek "the root of the problem" in users' needs.

  1. Why to create patch A to patch B?

This comes from Sacred Contention's README:

In order to use these you must apply it onto an already patched game. That means first you patch the normal version, and THEN you can patch any of these ONTO it. So you have to do TWO PATCHINGS, one for the normal, and then another one for the mode you want. It'll add the desired effect.

This also means that you can combine the patches to allow for many combinations--I calculated 64 myself (8 different options including on/off x 8 = 64) but idk if that's actually true. Just patch them over and over (but if you patch a patch twice, it undoes itself).

Assume that patch B is the base project, and patch A is a mode/option/feature/switch/configuration. The creator hopes it can be applied by user preference and doesn't want to create many patches, so patch A to patch B is created.

  1. Why the actual baserom may be bigger?

Still assume that patch B is the base project and it is still in active development. More content is added and it becomes patch C. C is bigger than B, and if you apply patch A to patch C, the patch C will be cut to the size of patch B. In fact, patch A is only to switch a small feature of the whole project, but it causes severe bug to the whole project, which is unexpected behaviour. Of course you can make patch A' for patch C besides patch A for patch B, but it is too troublesome to create patches for all hack versions and it is hard to manage a large amount of patches, which is confusing.

Therefore, I think NUPS's implementation is better considering the real user needs.

We just talked about the design philosophy. Let's talk about realistic reasons next.

Frankly speaking, UPS format is too old, and there are better alternatives to it now. However, many ROM hacks are ancient and no one maintains them now. Moreover, some hacking community are still using UPS format and NUPS as patcher. It is hard to push them to update those old patches or switch to another patcher to create new patches. NUPS has been widely used and it is not in active maintain. It is hard to change its implementation and replace old versions on the Internet. Rom Patcher JS is still in active development and it is a online patcher, so it is easy to fix its implementation.

Users only want to apply the patch and play the hack. They don't care about the technique details, so they will choose the one which works for them.

Therefore, I think it is good to be compatible with NUPS's implementation.

At last, if you are not sure yet, it is always good to add a setting or ask for it to let the user choose which implementation.

marcrobledo commented 2 years ago

This is the kind of explanations I love to read!

No need to discuss further :-)