search

HarbourMasters / Shipwright

3.27k stars 489 forks source link

Fix the vanilla Deku Nut upgrade bug #330

Closed vaguerant closed 2 years ago

vaguerant commented 2 years ago

This bug exists in all Ocarina of Time N64 ROMs, including GameCube/Wii VC/etc. and is carried over to Ship of Harkinian. The bug was fixed in the official 3DS port. The flag at bit 0x8000 of gSaveContext.itemGetInf[1] is set in two completely unrelated places. This prevents the player from receiving a Deku Nut capacity upgrade if they wait to do it until after the Biggoron's Sword trading sequence.

Screenshot of the Save Editor with flag set

The first (and obviously intended) is when the player visits the Forest Stage (113: Grotto 12 in the Map Select) as child Link while wearing the Mask of Truth. The Deku Scrubs will reward the player with a Deku Nut capacity upgrade and set the above noted flag. This flag works as expected. Manually setting the flag in the Save Editor window before visiting the Forest Stage prevents the Deku Nut upgrade from being offered, since the game assumes this has already been done. Unsetting the flag allows the upgrade to be obtained.

https://github.com/HarbourMasters/Shipwright/blob/ab2819d9fbe533037996e857530ae0a65a910f75/soh/src/overlays/actors/ovl_En_Dnt_Jiji/z_en_dnt_jiji.c#L277-L287

The second place (more likely to be a bug) is when the player encounters Fado (the blonde Kokiri girl) in the Lost Woods and exchanges the Odd Potion for the Poacher's Saw, as part of the Biggoron's Sword trading sequence. I'm just a lay user who has some idea how to navigate GitHub, so I have no understanding of why this particular flag is set during this sequence or whether anything else checks the flag to confirm that it has been set.

https://github.com/HarbourMasters/Shipwright/blob/ab2819d9fbe533037996e857530ae0a65a910f75/soh/src/code/z_parameter.c#L1797-L1799

The only place I can see where the flag is ever read is in the Forest Stage:

https://github.com/HarbourMasters/Shipwright/blob/ab2819d9fbe533037996e857530ae0a65a910f75/soh/src/overlays/actors/ovl_En_Dnt_Demo/z_en_dnt_demo.c#L166-L167

Manually unsetting this flag in the Save Editor does not seem to prevent continuing the adult trading sequence--I completed two further steps in the trading sequence without issue after this point. Maybe the flag is completely unused in this context, or maybe the flag is just mispositioned and it should be bit 0x8000 of gSaveContext.itemGetInf[2] instead or something similar? The other adult trading sequence items seem to be in the 2 and 3 rows, so the Poacher's Saw being in 1 does seem like it could be a typo.

I am not aware of any beneficial use of this bug, e.g. for speedrunning, so I would suggest any fix for this bug should default to being enabled.

vaguerant commented 2 years ago

Just a warning in case anybody's about to start looking into this, I'm taking a shot at fixing this one myself and will probably have a PR for it in a while if that works.