Label and constant cleanup (#528)

* Label cleanup, add some constants instead of raw numbers

* Rename load/save game functions to be more explicit

1 files changed, 109 insertions, 92 deletions

diff --git a/engine/menus/save.asm b/engine/menus/save.asm
index fba1dc53..7112207f 100644
--- a/engine/menus/save.asm
+++ b/engine/menus/save.asm
@@ -1,16 +1,15 @@
-LoadSAV:
-; if carry, write "the file data is destroyed"
+TryLoadSaveFile:
 	call ClearScreen
 	call LoadFontTilePatterns
 	call LoadTextBoxTilePatterns
-	call LoadSAV0
+	call LoadMainData
 	jr c, .badsum
-	call LoadSAV1
+	call LoadCurrentBoxData
 	jr c, .badsum
-	call LoadSAV2
+	call LoadPartyAndDexData
 	jr c, .badsum
 	ld a, $2 ; good checksum
-	jr .goodsum
+	jr .done
 .badsum
 	ld hl, wStatusFlags5
 	push hl
@@ -22,7 +21,7 @@ LoadSAV:
 	pop hl
 	res BIT_NO_TEXT_DELAY, [hl]
 	ld a, $1 ; bad checksum
-.goodsum
+.done
 	ld [wSaveFileStatus], a
 	ret
 
@@ -30,33 +29,34 @@ FileDataDestroyedText:
 	text_far _FileDataDestroyedText
 	text_end
 
-LoadSAV0:
+LoadMainData:
 	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $1
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
+	ASSERT BANK("Save Data") == BMODE_ADVANCED
 	ld [rRAMB], a
 ; This vc_hook does not have to be in any particular location.
 ; It is defined here because it refers to the same labels as the two lines below.
 	vc_hook Unknown_save_limit
 	ld hl, sGameData
 	ld bc, sGameDataEnd - sGameData
-	call SAVCheckSum
+	call CalcCheckSum
 	ld c, a
 	ld a, [sMainDataCheckSum]
 	cp c
-	jp z, .checkSumsMatched
+	jp z, .checkSumMatched
 
 ; If the computed checksum didn't match the saved on, try again.
 	ld hl, sGameData
 	ld bc, sGameDataEnd - sGameData
-	call SAVCheckSum
+	call CalcCheckSum
 	ld c, a
 	ld a, [sMainDataCheckSum]
 	cp c
-	jp nz, SAVBadCheckSum
+	jp nz, CheckSumFailed
 
-.checkSumsMatched
+.checkSumMatched
 	ld hl, sPlayerName
 	ld de, wPlayerName
 	ld bc, NAME_LENGTH
@@ -73,46 +73,51 @@ LoadSAV0:
 	call CopyData
 	ld a, [sTileAnimations]
 	ldh [hTileAnimations], a
+
+; this part is redundant, LoadCurrentBoxData is always called next
 	ld hl, sCurBoxData
 	ld de, wBoxDataStart
 	ld bc, wBoxDataEnd - wBoxDataStart
 	call CopyData
+
 	and a
-	jp SAVGoodChecksum
+	jp GoodCheckSum
 
-LoadSAV1:
+LoadCurrentBoxData:
 	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $1
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
+	ASSERT BANK("Save Data") == BMODE_ADVANCED
 	ld [rRAMB], a
 	ld hl, sGameData
 	ld bc, sGameDataEnd - sGameData
-	call SAVCheckSum
+	call CalcCheckSum
 	ld c, a
 	ld a, [sMainDataCheckSum]
 	cp c
-	jr nz, SAVBadCheckSum
+	jr nz, CheckSumFailed
 	ld hl, sCurBoxData
 	ld de, wBoxDataStart
 	ld bc, wBoxDataEnd - wBoxDataStart
 	call CopyData
 	and a
-	jp SAVGoodChecksum
+	jp GoodCheckSum
 
-LoadSAV2:
+LoadPartyAndDexData:
 	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $1
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
+	ASSERT BANK("Save Data") == BMODE_ADVANCED
 	ld [rRAMB], a
 	ld hl, sGameData
 	ld bc, sGameDataEnd - sGameData
-	call SAVCheckSum
+	call CalcCheckSum
 	ld c, a
 	ld a, [sMainDataCheckSum]
 	cp c
-	jp nz, SAVBadCheckSum
+	jp nz, CheckSumFailed
 	ld hl, sPartyData
 	ld de, wPartyDataStart
 	ld bc, wPartyDataEnd - wPartyDataStart
@@ -122,40 +127,43 @@ LoadSAV2:
 	ld bc, wPokedexSeenEnd - wPokedexOwned
 	call CopyData
 	and a
-	jp SAVGoodChecksum
+	jp GoodCheckSum
 
-SAVBadCheckSum:
+CheckSumFailed:
 	scf
+	; fallthrough
 
-SAVGoodChecksum:
-	ld a, $0
+GoodCheckSum:
+	ld a, BMODE_SIMPLE ; preserve flags
 	ld [rBMODE], a
+	ASSERT RAMG_SRAM_DISABLE == BMODE_SIMPLE
 	ld [rRAMG], a
 	ret
 
-LoadSAVIgnoreBadCheckSum:
-; unused function that loads save data and ignores bad checksums
-	call LoadSAV0
-	call LoadSAV1
-	jp LoadSAV2
+TryLoadSaveFileIgnoreChecksum: ; unreferenced
+; don't update wSaveFileStatus upon success or failure
+; don't display warning in case of failed checksum
+	call LoadMainData
+	call LoadCurrentBoxData
+	jp LoadPartyAndDexData
 
-SaveSAV:
+SaveMenu:
 	farcall PrintSaveScreenText
 	ld hl, WouldYouLikeToSaveText
-	call SaveSAVConfirm
+	call SaveTheGame_YesOrNo
 	and a   ;|0 = Yes|1 = No|
 	ret nz
 	ld a, [wSaveFileStatus]
 	dec a
 	jr z, .save
-	call SAVCheckRandomID
+	call CheckPreviousSaveFile
 	jr z, .save
 	ld hl, OlderFileWillBeErasedText
-	call SaveSAVConfirm
+	call SaveTheGame_YesOrNo
 	and a
 	ret nz
 .save
-	call SaveSAVtoSRAM
+	call SaveGameData
 	hlcoord 1, 13
 	lb bc, 4, 18
 	call ClearScreenArea
@@ -175,13 +183,13 @@ SaveSAV:
 NowSavingString:
 	db "Now saving...@"
 
-SaveSAVConfirm:
+SaveTheGame_YesOrNo:
 	call PrintText
 	hlcoord 0, 7
 	lb bc, 8, 1
 	ld a, TWO_OPTION_MENU
 	ld [wTextBoxID], a
-	call DisplayTextBoxID ; yes/no menu
+	call DisplayTextBoxID
 	ld a, [wCurrentMenuItem]
 	ret
 
@@ -197,12 +205,14 @@ OlderFileWillBeErasedText:
 	text_far _OlderFileWillBeErasedText
 	text_end
 
-SaveSAVtoSRAM0:
+SaveMainData:
 	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $1
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
+	ASSERT BANK("Save Data") == BMODE_ADVANCED
 	ld [rRAMB], a
+
 	ld hl, wPlayerName
 	ld de, sPlayerName
 	ld bc, NAME_LENGTH
@@ -216,26 +226,29 @@ SaveSAVtoSRAM0:
 	ld bc, wSpriteDataEnd - wSpriteDataStart
 	call CopyData
 	ld hl, wBoxDataStart
+
+; this part is redundant, SaveCurrentBoxData is always called next
 	ld de, sCurBoxData
 	ld bc, wBoxDataEnd - wBoxDataStart
 	call CopyData
 	ldh a, [hTileAnimations]
 	ld [sTileAnimations], a
+
 	ld hl, sGameData
 	ld bc, sGameDataEnd - sGameData
-	call SAVCheckSum
+	call CalcCheckSum
 	ld [sMainDataCheckSum], a
 	xor a
 	ld [rBMODE], a
 	ld [rRAMG], a
 	ret
 
-SaveSAVtoSRAM1:
-; stored pokémon
+SaveCurrentBoxData:
 	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $1
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
+	ASSERT BANK("Save Data") == BMODE_ADVANCED
 	ld [rRAMB], a
 	ld hl, wBoxDataStart
 	ld de, sCurBoxData
@@ -243,18 +256,19 @@ SaveSAVtoSRAM1:
 	call CopyData
 	ld hl, sGameData
 	ld bc, sGameDataEnd - sGameData
-	call SAVCheckSum
+	call CalcCheckSum
 	ld [sMainDataCheckSum], a
 	xor a
 	ld [rBMODE], a
 	ld [rRAMG], a
 	ret
 
-SaveSAVtoSRAM2:
+SavePartyAndDexData:
 	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $1
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
+	ASSERT BANK("Save Data") == BMODE_ADVANCED
 	ld [rRAMB], a
 	ld hl, wPartyDataStart
 	ld de, sPartyData
@@ -266,21 +280,21 @@ SaveSAVtoSRAM2:
 	call CopyData
 	ld hl, sGameData
 	ld bc, sGameDataEnd - sGameData
-	call SAVCheckSum
+	call CalcCheckSum
 	ld [sMainDataCheckSum], a
 	xor a
 	ld [rBMODE], a
 	ld [rRAMG], a
 	ret
 
-SaveSAVtoSRAM::
+SaveGameData::
 	ld a, $2
 	ld [wSaveFileStatus], a
-	call SaveSAVtoSRAM0
-	call SaveSAVtoSRAM1
-	jp SaveSAVtoSRAM2
+	call SaveMainData
+	call SaveCurrentBoxData
+	jp SavePartyAndDexData
 
-SAVCheckSum:
+CalcCheckSum:
 ;Check Sum (result[1 byte] is complemented)
 	ld d, 0
 .loop
@@ -303,7 +317,7 @@ CalcIndividualBoxCheckSums:
 	push bc
 	push de
 	ld bc, wBoxDataEnd - wBoxDataStart
-	call SAVCheckSum
+	call CalcCheckSum
 	pop de
 	ld [de], a
 	inc de
@@ -317,7 +331,7 @@ GetBoxSRAMLocation:
 ; out: b = box SRAM bank, hl = pointer to start of box
 	ld hl, BoxSRAMPointerTable
 	ld a, [wCurrentBoxNum]
-	and $7f
+	and BOX_NUM_MASK
 	cp NUM_BOXES / 2
 	ld b, 2
 	jr c, .next
@@ -379,7 +393,7 @@ ChangeBox::
 	ld a, [hl]
 	ld [de], a
 	call RestoreMapTextPointer
-	call SaveSAVtoSRAM
+	call SaveGameData
 	ld hl, wChangeBoxSavedMapTextPointer
 	call SetMapTextPointer
 	ld a, SFX_SAVE
@@ -396,7 +410,7 @@ CopyBoxToOrFromSRAM:
 	push hl
 	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $1
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
 	ld a, b
 	ld [rRAMB], a
@@ -404,7 +418,7 @@ CopyBoxToOrFromSRAM:
 	call CopyData
 	pop hl
 
-; mark the memory that the box was copied from as am empty box
+; mark the source box as an empty box
 	xor a
 	ld [hli], a
 	dec a
@@ -412,7 +426,7 @@ CopyBoxToOrFromSRAM:
 
 	ld hl, sBox1 ; sBox7
 	ld bc, sBank2AllBoxesChecksum - sBox1
-	call SAVCheckSum
+	call CalcCheckSum
 	ld [sBank2AllBoxesChecksum], a ; sBank3AllBoxesChecksum
 	call CalcIndividualBoxCheckSums
 	xor a
@@ -434,7 +448,7 @@ DisplayChangeBoxMenu:
 	xor a
 	ld [wMenuWatchMovingOutOfBounds], a
 	ld a, [wCurrentBoxNum]
-	and $7f
+	and BOX_NUM_MASK
 	ld [wCurrentMenuItem], a
 	ld [wLastMenuItem], a
 	hlcoord 0, 0
@@ -455,7 +469,7 @@ DisplayChangeBoxMenu:
 	ld hl, hUILayoutFlags
 	res BIT_SINGLE_SPACED_LINES, [hl]
 	ld a, [wCurrentBoxNum]
-	and $7f
+	and BOX_NUM_MASK
 	cp 9
 	jr c, .singleDigitBoxNum
 	sub 9
@@ -474,7 +488,7 @@ DisplayChangeBoxMenu:
 	hlcoord 18, 1
 	ld de, wBoxMonCounts
 	ld bc, SCREEN_WIDTH
-	ld a, $c
+	ld a, NUM_BOXES
 .loop
 	push af
 	ld a, [de]
@@ -517,12 +531,12 @@ EmptyAllSRAMBoxes:
 ; player changes the box)
 	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $1
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
-	ld a, 2
+	ld a, BANK("Saved Boxes 1")
 	ld [rRAMB], a
 	call EmptySRAMBoxesInBank
-	ld a, 3
+	ld a, BANK("Saved Boxes 2")
 	ld [rRAMB], a
 	call EmptySRAMBoxesInBank
 	xor a
@@ -546,7 +560,7 @@ EmptySRAMBoxesInBank:
 	call EmptySRAMBox
 	ld hl, sBox1 ; sBox7
 	ld bc, sBank2AllBoxesChecksum - sBox1
-	call SAVCheckSum
+	call CalcCheckSum
 	ld [sBank2AllBoxesChecksum], a ; sBank3AllBoxesChecksum
 	call CalcIndividualBoxCheckSums
 	ret
@@ -563,12 +577,12 @@ GetMonCountsForAllBoxes:
 	push hl
 	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $1
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
-	ld a, $2
+	ld a, BANK("Saved Boxes 1")
 	ld [rRAMB], a
 	call GetMonCountsForBoxesInBank
-	ld a, $3
+	ld a, BANK("Saved Boxes 2")
 	ld [rRAMB], a
 	call GetMonCountsForBoxesInBank
 	xor a
@@ -578,7 +592,7 @@ GetMonCountsForAllBoxes:
 
 ; copy the count for the current box from WRAM
 	ld a, [wCurrentBoxNum]
-	and $7f
+	and BOX_NUM_MASK
 	ld c, a
 	ld b, 0
 	add hl, bc
@@ -602,25 +616,26 @@ GetMonCountsForBoxesInBank:
 	ld [hli], a
 	ret
 
-SAVCheckRandomID:
-; checks if Sav file is the same by checking player's name 1st letter
-; and the two random numbers generated at game beginning
-; (which are stored at wPlayerID)s
-	ld a, $0a
+; check if we're trying to save over a different playthrough
+; return result in zero flag
+; z set: a valid save file was found, and saved PlayerID is different from wPlayerID
+CheckPreviousSaveFile:
+	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $01
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
+	ASSERT BANK("Save Data") == BMODE_ADVANCED
 	ld [rRAMB], a
 	ld a, [sPlayerName]
 	and a
 	jr z, .next
 	ld hl, sGameData
 	ld bc, sGameDataEnd - sGameData
-	call SAVCheckSum
+	call CalcCheckSum
 	ld c, a
 	ld a, [sMainDataCheckSum]
 	cp c
-	jr nz, .next
+	jr nz, .next ; return z set if save data is corrupted
 	ld hl, sMainData + (wPlayerID - wMainDataStart) ; player ID
 	ld a, [hli]
 	ld h, [hl]
@@ -631,8 +646,9 @@ SAVCheckRandomID:
 	ld a, [wPlayerID + 1]
 	cp h
 .next
-	ld a, $00
+	ld a, BMODE_SIMPLE
 	ld [rBMODE], a
+	ASSERT RAMG_SRAM_DISABLE == BMODE_SIMPLE
 	ld [rRAMG], a
 	ret
 
@@ -674,7 +690,7 @@ LoadHallOfFameTeams:
 HallOfFame_Copy:
 	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $1
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
 	xor a
 	ld [rRAMB], a
@@ -684,25 +700,26 @@ HallOfFame_Copy:
 	ld [rRAMG], a
 	ret
 
-ClearSAV:
+; Fill SRAM with $ff, erasing save data. Used by DoClearSaveDialogue
+ClearAllSRAMBanks:
 	ld a, RAMG_SRAM_ENABLE
 	ld [rRAMG], a
-	ld a, $1
+	ld a, BMODE_ADVANCED
 	ld [rBMODE], a
 	xor a
-	call PadSRAM_FF
-	ld a, $1
-	call PadSRAM_FF
-	ld a, $2
-	call PadSRAM_FF
-	ld a, $3
-	call PadSRAM_FF
+	call .PadSRAM_FF
+	ld a, 1
+	call .PadSRAM_FF
+	ld a, 2
+	call .PadSRAM_FF
+	ld a, 3
+	call .PadSRAM_FF
 	xor a
 	ld [rBMODE], a
 	ld [rRAMG], a
 	ret
 
-PadSRAM_FF:
+.PadSRAM_FF
 	ld [rRAMB], a
 	ld hl, STARTOF(SRAM)
 	ld bc, SIZEOF(SRAM)