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

Dungeon 7 Map Bug #17

Closed jason-oliveira closed 1 year ago

jason-oliveira commented 1 year ago

When first going into Dungeon 7 on version 3.3.2, the map is already populated. This happens in both Mesen and FCEUmm/X

Mesen: 20230630_184148 - Screenshot

ShadowOne333 commented 1 year ago

What combination of patches did you use?

jason-oliveira commented 1 year ago

Bug was filed after confirming behavior with lone 3.3.2 patch.

ShadowOne333 commented 1 year ago

So standalone 3.3.2 with no additional patches then. I'll see if I can debug this during the week.

ericune commented 1 year ago

I got this issue as well. I'm not sure what factors into the map getting displayed or not, but this is how I played it:

EDIT: I just went and checked dungeons 8 and 9, without beating dungeon 7 (or grabbing its map/compass):

EDIT 2: It's also worth noting that on a fresh new file, I went straight for dungeon 8 (I couldn't go for 7, since I don't have the flute) and it properly did NOT show the entire map on first entry. This leads me to believe that the problem might be related to grabbing maps on previous dungeons (e.g. maybe grabbing maps from dungeons 3 and 4 sets the flag for dungeons 7 and 8, as well? just throwing ideas).

ShadowOne333 commented 1 year ago

Does Dungeon 7 show the full map if entered on a fresh save file? Meaning nothing grabbed and just going straight to 7.

Thanks to ericune's report it seems Dungeon 8 might have issues depending on certain map or compass grabs, but I need to know if Dungeon 7 does it as well.

One more thing that might help could be what maps and compasses were grabbed before entering Dungeons 7 or 8

jason-oliveira commented 1 year ago

Issue is also happening on both level 7 and 8 of Second Quest as well.

ericune commented 1 year ago

Took me a while, but I have some findings. My best attempt at trying to get into Dungeon 7 on a fresh new file (it's not trivial) was to essentially go through dungeons 1 to 4, grabbing items, heart containers, but skipping maps, compasses and triforce pieces. In dungeon 5, I went straight for the flute (had to beat two waves of blue iron knuckles, which is why I needed heart containers and better swords, I'm not very skilled). I then went into Dungeon 7. Keep in mind, I did not explore every room of every dungeon, as I usually do, I went straight for the item and the boss. When I entered dungeon 7 for the first time, the map was showing partially. See image below: Screenshot_20230705-113733-01

I then went and checked dungeon 8. Also showing the map partially. Screenshot_20230705-114117-01

It seems then that the answer to this problem is more complicated than I initially thought. Dungeons 7 and 8 might be reading memory of explored rooms from other dungeons and do not depend on whether maps were grabbed or not.

ShadowOne333 commented 1 year ago

So it seems like the issue goes more in depth than initially thought, it's possible that the code might be reading from other addresses, possibly some overflow of sorts? I'm not sure, but I'll try to debug this, though it does seem a bit hard to narrow down, at least for me.

ShadowOne333 commented 1 year ago

Going through the dungeon_automap.asm file, I think the possible issue might be with one of the many sta.w $30X opcodes found through the code (it's either those or the ones modifying $06FF).

From what I could gather, it's possible that the RAM addresses $0300 up to $0306 (which the Dungeon Automap uses) could be used for other stuff, which is what's causing the maps for certain dungeons to be unveiled partially or completely in some instances.

I have yet to confirm this, and I have also yet to debug this properly, but it's at least a possibility. These two RAM maps lead me to that possibility: https://www.bwass.org/romhack/zelda1/zelda1rammap.txt https://github.com/aldonunez/zelda1-disassembly/blob/master/src/Variables.inc

To debug/test this theory, @ericune would be able to share one of your save files with this issue, please? That would help to test if modifying certain RAM addresses in the code for another would probably have any effect on the partial maps showing up on first entrance.

ghost commented 1 year ago

Honestly this puzzled me for awhile. Then I saw this map. https://i.stack.imgur.com/MmtY8.png

So upper 8 rows for RAM 6ff. Bottom 8 rows for 77f. I don't remember which dungeons are in the bottom half, but we need to add checks for those ones (7 + 8 + 9)?.

ShadowOne333 commented 1 year ago

I can help with identifying the maps from that image. Starting from upper left to right: 4, 1, 5, 6, 3, 2 7, 8, 9

Hopefully this helps a bit

ghost commented 1 year ago

https://github.com/ShadowOne333/The-Legend-of-Zelda-Redux/blob/47a54b75a63bd204fd397ba37b9c38ce2f0da3ac/code/gameplay/dungeon_automap.asm#L265-L278

        ldy.b $10       // Dungeon 7-9
        cpy.b #$07
        bcc .get_map_room_index_exit

This is a quick hack but I think it works from basic testing.

I'm curious how the vanilla code knows which bank to read from for dungeon room data.

ShadowOne333 commented 1 year ago

I think these are the places where the hijack occurs in bank 5, around the routine in 0x16720 and the one at 0x1701D: https://www.bwass.org/romhack/zelda1/zelda1bank5.txt

The code at bank 6 is within free/unused space in the original game, so that's not counted in, so most likely the original code for the bank read for Dungeon room data should be within range of those two routines in bank 5.

Thank you for the quick hack, I have put together a quick IPS for testing. For those having the issue, please try out this patch and let me know if that fixes the bug with Dungeons 7 and 8 showing partial or full maps on entrance. Zelda1_Redux.zip

ghost commented 1 year ago

Dungeons 2+3 overlap 7+8, so only those need to be visited to trigger the bug which I could not anymore.

Load dungeon data ptr + room data
 07:E6CE: AD AF 6B  LDA $6BAF = #$7F
 07:E6D1: 85 00     STA $00 = #$7F
 07:E6D3: AD B0 6B  LDA $6BB0 = #$07
 07:E6D6: 85 01     STA $01 = #$07

 07:E6D8: A4 EB     LDY $EB = #$7E
 07:E6DA: B1 00     LDA ($00),Y @ $0787 = #$00
 07:E6DC: 60        RTS -----------------------------------------

Maybe we could save some rom space, or clean it up a little with the above jsr.

ghost commented 1 year ago

Final solution

    ldy.w $6bb0     // Ptr bank 6ff, 77f
    cpy.b #$07-1
    beq .get_map_room_index_exit
ericune commented 1 year ago

I'll gladly test out the final solution, if you put together a patch for it.

ShadowOne333 commented 1 year ago

Thanks @vailkyte for the solution! Here you go @ericune , latest patch with the latest change suggested by vailkyte, let me know if anything odd comes up:

Zelda1_Redux.zip

ericune commented 1 year ago

I have run through the game again, same way as I did last time, and I can confirm that with that latest patch I found NO issues with dungeons 7, 8, and 9 (or any other dungeons, for that matter). They all had an unexplored map when I entered them for the first time. Great work! And thank you for the fix!

ShadowOne333 commented 1 year ago

Fantastic! Thanks for testing it out, ericune! And of course thanks to vailkyte for the amazing work on bugfixing all the automap issues for Dungeons 7, 8 and 9 as well. I have pushed the newest changes to GitHub already, and I'll bump the main release version to v3.3.3. I'll upload it to Romhacking.net in a bit.

Thank you all!