search

Alcaro / Flips

Floating IPS is a patcher for IPS and BPS files.
Other
337 stars 46 forks source link

"appears scrambled or malformed" warning if not in ascending order? #13

Closed bbbradsmith closed 6 years ago

bbbradsmith commented 6 years ago

I was getting the error "The patch was applied, but appears scrambled or malformed." for an IPS patch that was otherwise being correctly applied and could not understand why.

After building and debugging flips, I discovered that this was caused by the the chunks in the file not appearing in ascending order of address.

Is it possible to make this error more descriptive of which "scrambled or malformed" condition it's giving a warning about? There appear to be at least 3 different conditions that can give this same message.

.

I can see how it's a useful warning if you're expecting your own tool to always generate it in that order, but it doesn't appear to be part of any definition of the file format I could find, so I was very confused about why an IPS I was generating that appeared to obey the format was giving an error in your tool. Was even trying to eliminate chunks one by one to see which one was the cause, but that just led to more confusion since this error isn't due to any specific chunk, but their collective arrangement.

(Otherwise thanks for making a good replacement for LunarIPS. It's very handy to have.)

Alcaro commented 6 years ago

I don't think the format is defined anywhere at all, I mostly made things up and matched Lunar IPS.

I couldn't think of any sensible creation algorithm that yields unsorted chunks, so I deemed 'corrupt file' the most likely cause and added that warning. If you've found something I couldn't think of, explain your algorithm and I'll drop it.

bbbradsmith commented 6 years ago

It wasn't an algorithmic generation, it was made by hand to express a series of changes to a file. The order was organized by meaning/category of what they're changing, not by their destination in the target file.

There are a bunch of reasons a person might have for wanting to make an IPS manually rather than with a tool, and once you impose a sorting rule like that it becomes very hard to do it with anything but a tool.

I've made patches in the past that are able to apply to multiple versions of a game despite them having different data. If I could only make a patch with a tool, I would have had to distribute separate versions of the patch instead of being able to prepare a universal one.

Or right now, I'm working on a patch and wanted to use flips as a command line tool as part of the build and test process while I add/remove/reorganize stuff in the patch as I iterate on it. Having to sort the chunks at the same time kinda kills the utility of that.

In other cases it might be reasonable to group things in the file just semantically for anyone that wants to follow the series of changes. Or if you were concatenating a few independent patches, you have to re-sort them with a tool and obliterate their pre-existing natural separation.

I do think checking if they're sorted is a good optional validation step for ones that were made in an automated way, but if it's a mandatory warning I think it will make some patches that weren't made automatically look like there's something wrong with it.

Alcaro commented 6 years ago

That counts as a valid reason for having unsorted data. Personally I'd use another format for manually generated patches (preferably one that supports comments), but I have little if any right to demand others do that. Or if you edit them in a commentable format, you could add a sorting step to the converter, but figuring out that Flips wants that is indeed quite obscure.

Corrupt patches probably either corrupt a length and desync the EOF, or corrupt data that can't be detected, anyways. Corrupting an address only is rare. And file corruption itself isn't a relevant concept anymore, either.

Obvious follow up question: How about old Flips versions, like this one? The obvious solution would be submitting an update, I left Windows a while ago, so that's kinda tricky.

bbbradsmith commented 6 years ago

This is a Windows build I made with the latest changes. (link removed)

I'm noticing with this version, though, if I try to use the same file as input and output ROM, I get "Couldn't write any ROMs. Are you on a read-only medium?" (I'll figure this out and send a pull request.)

Update: https://github.com/Alcaro/Flips/pull/14