Saving issues with hacks that include patched code

[ghost]

Greetings! I'm a ROM hacker and I've recently released a patch that integrates mirror mode into Super Mario Kart ROM hacks. However, after releasing the patch, I've noticed an oddity that Epic Edit does while saving ROM hacks. The tool seemingly saves the pointer to the vanilla decoding algorithm at 0x1E765 in ROM rather than leaving it be. This results in the game loading the track incorrectly. I haven't tested if Epic Edit saves more redundant information in more places, but this could be grounds for software plagiarism within the tool's source, or at least confusion from its users.

Pointer change at 0x1E765.

Mirror patch before saving with Epic Edit.

Mirror patch after saving with Epic Edit.

Thank you for reading through this issue!

[thomasgoldstein]

Hey. I had a quick look at the sources. It is data that Epic Edit resets in order to fix ROMs that have been modified with MAKE (another SMK track editor).

From Offsets.cs:

                    this[Offset.MakeDataReset1] = 0x1E765;
                    this[Offset.MakeDataReset2] = 0x1FBCD;

From Game.cs:

        private void ResetTrackLoadingLogic()
        {
            // HACK: MAKE modifies these bytes in order to relocate track-related data.
            // I'm not sure what they are or reference, but they're probably related to track map and AI data.
            // In order to keep MAKE ROMs working when resaving them, we need to reset them to their original values,
            // because we do not save data the way MAKE does.

            var offset1 = _offsets[Offset.MakeDataReset1];
            var offset2 = _offsets[Offset.MakeDataReset2];

            _romBuffer[offset1] = _region == Region.US ? (byte)0x9E : (byte)0x41;
            _romBuffer[offset1 + 1] = _region == Region.US ? (byte)0xE0 : (byte)0xDF;
            _romBuffer[offset1 + 2] = _region == Region.US ? (byte)0x84 : (byte)0x84;

            _romBuffer[offset2] = 0xBD;
            _romBuffer[offset2 + 1] = _region == Region.Jap ? (byte)0x8C : _region == Region.Euro ? (byte)0x6D : (byte)0x9B;
            _romBuffer[offset2 + 2] = 0xFF;
            _romBuffer[offset2 + 3] = 0x85;
            _romBuffer[offset2 + 4] = 0x08;
        }

I'm not sure how to fix this nicely. Let me know if you've got ideas.
I guess I could reset that data only if it contains exactly the values that MAKE normally writes there.

[ghost]

I suppose there could be a deeper check for the decoding algorithm. Here is the source code for 0xD000EE relative to system bus. It immediately jumps to the decoding algorithm before checking the mirror flag. If the data that the decode pointer jumps to reads 22 9E E0 84 (jsl 0x84E09E), then there wouldn't be a need to change the pointer, otherwise it could be a MAKE-generated ROM.

Of course, you'd need to modulo this address check by 0x400000 due to ROM mirroring.

[thomasgoldstein]

If I understand correctly, you'd check for your specific case, and otherwise assume it is a MAKE ROM. Wouldn't it be safer to swap that logic around, ie: specifically check for what MAKE does (whatever it really does, I don't know), and otherwise leave things untouched? That way, we'd avoid rewriting those bytes in most cases.

Also, would you be able to submit a pull request? I am both lazy and rusty. 😅