pokered
pokered copied to clipboard
pret
Document cycling road flag oversight
https://github.com/pret/pokered/blob/610ec0fd2271defcc6ebda89b379455155fdb3ca/engine/movie/oak_speech/oak_speech.asm#L6
The following interesting oversight can be documented in the dissambly. Maybe, the address wd732 also can be given a proper name like wWarpCyclingRoadAndDebugFlags or similar.
Address D732 is intentionally carried over to the next save file. It contained debug flags in early development, (see here)
In the final version of the games that byte stores the cycling road flag and warp related flags (dig/teleport/fly/escape rope), which is actually very clever, because you can fly out of the cycling road and that byte needs to be cleared after warping anyway.
Starting a new game while being on the cycling road, preserves this bit, making it impossible to surf or ride the bike unless you enter and leave the cycling road or use one of the warp techniques. Not unsetting that flag when blacking out was present in the Japanese but has been fixed in the US version.
From a cursory look at the address. I don't believe wd732 to be specific to Cycling Road. It appears to handle setting warp flags for all warps. It just also happens to contain a flag for Debug and Cycling Road Bike Riding.
It looks like the following addresses all contain various flags similar to pokecrystals' engine_flags... just without dedicated functions to process them like pokecrystal:
wd728:: db
wBeatGymFlags:: db
wd72c:: db
wd72d:: db
wd72e:: db
wd730:: db
wd732:: db
wFlags_D733:: db
wBeatLorelei:: db
wd736:: db
wCompletedInGameTradeFlags:: dw <- possibly.
My recommendation would be to rename these to something along the lines of wFlags#; and create constants for each of the bits referenced in their wram comments.
As far as the documentation piece goes, this can be something added when #258 is done. An easy fix for this could be something like (Assuming the end user wants to keep those debug features for the debug rom).
In engine/movie/oak_speech/oak_speech.asm
SetDefaultNames:
ld a, [wLetterPrintingDelayFlags]
push af
ld a, [wOptions]
push af
+IF DEF(_DEBUG)
ld a, [wd732]
push af
+ENDC
ld hl, wPlayerName
...
call FillMemory
+IF DEF(_DEBUG)
pop af
ld [wd732], a
+ENDC
pop af
ld [wOptions], a
pop af
ld [wLetterPrintingDelayFlags], a
....
As far as the documentation piece goes, this can be something added when #258 is done. An easy fix for this could be something like (Assuming the end user wants to keep those debug features for the debug rom).
The code which preserves that byte is in fact in the production build and not only in the debug build of the games.
The code which preserves that byte is in fact in the production build and not only in the debug build of the games.
Hence, why my suggested bug fix action (to go in the future bug docs) forces the code to only build if you are building the debug rom.
It looks like the start of the Bug Documentation for this has been created ~~here.~~ This will eventually be included in pokered's future docs/bugs_and_glitches.md (https://github.com/pret/pokered/issues/258). This wram address and the remaining unnamed wram addresses still require proper names, but this would be a seperate issue. I believe we can close this issue for now.
Edit: I guess it's been moved here now...
I added a temporary comment in https://github.com/pret/pokered/pull/410 that documents this:
https://github.com/pret/pokered/blob/a38c7922dda3b6650a1dfe0fe544175ded259b19/engine/movie/oak_speech/oak_speech.asm#L6-L12
The simplest fix is to add res 5, [hl] here: https://github.com/pret/pokered/blob/a38c7922dda3b6650a1dfe0fe544175ded259b19/engine/menus/main_menu.asm#L309-L321
Does this help, @spixi?
Well done @SatoMew, but I still think that wd732 needs a proper constant name.
I still think that wd732 needs a proper constant name.
That will be taken care of eventually.