search

ShadowOne333 / The-Legend-of-Zelda-Redux

Source code for the hack The Legend of Zelda Redux
GNU General Public License v3.0
110 stars 11 forks source link

Various fixes to broken optional patches #13

Closed auburnbit closed 2 years ago

auburnbit commented 2 years ago

Please see commits for details. (fixes #11, fixes #12)

ShadowOne333 commented 2 years ago

Sorry for the very late reply, I did see your issue reports but haven't been able to respond to them yet due to vacations. So far it all seems good, the only change I have doubts about is the one inside the Original HUD code for MMC5 (MMC5/code/optional/OriginalHUD.asm)

Have you verified and confirmed that commenting out those PPU related opcodes doesn't have any undesired drawbacks in-game? Specifically these ones: Original code from Move maps (MMC5): https://github.com/ShadowOne333/The-Legend-of-Zelda-Redux/blob/master/MMC5/code/gameplay/move_maps.asm#L36-L49

Your change:

//org $9DC5 // 0x19DD5
//  adc.b #$62  // Starting PPU low byte
//org $9E22 // 0x19E32
//  ldy.b #$62  // Starting PPU low byte
//org $9E3D // 0x19E4D
//  cmp.b #$F6  // Finishing PPU low byte

The one change at 9E3D doesn't seem to matter (since it's the same in both instances), but the other 2 have me in doubt as I believe these ones are the ones that set the starting X position for the Automap for MMC5. The original game used 2062 in PPU, while Bogaa's MMC5 implementation of Automap used 2076.

Once this one is confirmed, the rest can be easily be merged. Thanks for taking a look at this!

auburnbit commented 2 years ago

Agreed on the uncertainty on the Original HUD code fix. I can say that it does stop the crash and allows the game to play, so I guess it's better than the alternative. However, I haven't done extensive testing or paid close attention to whether everything about the Automap position is correct. I'll play around with it some more and see what I can figure out.

auburnbit commented 2 years ago

It looks like the one change at 9E3D was the problem, commenting that out alone seems to stop the crash. I'm curious why modifying 9E3D makes it crash when Move Maps seems to make the same change without issue.

auburnbit commented 2 years ago

Wait... I bet $9E3D's supposed to be cmp.b #$E2 It's working as it is with that line commented out but I'll test it with the above change instead.

Edit: Yep that was it. I've updated the commit with the details.

Edit2: Also double checked the three new IPS files to be sure I didn't mess something up and they seem to be correct. Also, it appears that there actually is no need to comment move_maps.asm out of main.asm when using OriginalHUD.asm (as it is now at least), as it overwrites everything move_maps.asm does (in both MMC1 and MMC5). As long as that remains true in future iterations of both files, there shouldn't be a need to comment out move_maps.asm when using Original.HUD.asm. I've removed the warnings I had added to the optional.asm files and updated the commit.

(I apologize for all the edits and force pushes, this is my first time actually using GitHub so I'm still figuring things out.)

ShadowOne333 commented 2 years ago

Gave these a look with the new modifications, and it all seems good! Thank you so much for narrowing down the issues and debugging them to find a proper fix for these. I'll merge this and update the optional patches in Romhacking.net as well with your fixes, I'll also add you in the credits for the fixes.

Just to be sure, the only IPS files modified with Link's Awakening GFX, Original GFX (MMC1) and Original HUD (MMC5), right?

auburnbit commented 2 years ago

Yes, that should be the only IPS files modified. Thanks!