Miscellaneous cleanup (January 2025)

audio/engine_1.asm

@@ -1186,11 +1186,11 @@ Audio1_InitPitchSlideVars:
 	sub e
 	ld e, a
 
-; Bug. Instead of borrowing from the high byte of the target frequency as it
-; should, it borrows from the high byte of the current frequency instead.
-; This means that the result will be 0x200 greater than it should be if the
-; low byte of the current frequency is greater than the low byte of the
-; target frequency.
+	; BUG: Instead of borrowing from the high byte of the target frequency as it
+	; should, it borrows from the high byte of the current frequency instead.
+	; This means that the result will be 0x200 greater than it should be if the
+	; low byte of the current frequency is greater than the low byte of the
+	; target frequency.
 	ld a, d
 	sbc b
 	ld d, a

audio/engine_2.asm

@@ -1249,11 +1249,11 @@ Audio2_InitPitchSlideVars:
 	sub e
 	ld e, a
 
-; Bug. Instead of borrowing from the high byte of the target frequency as it
-; should, it borrows from the high byte of the current frequency instead.
-; This means that the result will be 0x200 greater than it should be if the
-; low byte of the current frequency is greater than the low byte of the
-; target frequency.
+	; BUG: Instead of borrowing from the high byte of the target frequency as it
+	; should, it borrows from the high byte of the current frequency instead.
+	; This means that the result will be 0x200 greater than it should be if the
+	; low byte of the current frequency is greater than the low byte of the
+	; target frequency.
 	ld a, d
 	sbc b
 	ld d, a

audio/engine_3.asm

@@ -1186,11 +1186,11 @@ Audio3_InitPitchSlideVars:
 	sub e
 	ld e, a
 
-; Bug. Instead of borrowing from the high byte of the target frequency as it
-; should, it borrows from the high byte of the current frequency instead.
-; This means that the result will be 0x200 greater than it should be if the
-; low byte of the current frequency is greater than the low byte of the
-; target frequency.
+	; BUG: Instead of borrowing from the high byte of the target frequency as it
+	; should, it borrows from the high byte of the current frequency instead.
+	; This means that the result will be 0x200 greater than it should be if the
+	; low byte of the current frequency is greater than the low byte of the
+	; target frequency.
 	ld a, d
 	sbc b
 	ld d, a

engine/battle/animations.asm

@@ -1341,7 +1341,8 @@ AdjustOAMBlockYPos2:
 	cp 112
 	jr c, .skipSettingPreviousEntrysAttribute
 	dec hl
-	ld a, 160 ; bug, sets previous OAM entry's attribute
+	; BUG: sets previous OAM entry's attribute
+	ld a, 160
 	ld [hli], a
 .skipSettingPreviousEntrysAttribute
 	ld [hl], a
@@ -1833,9 +1834,9 @@ _AnimationSlideMonOff:
 .PlayerNextTile
 	ld a, [hl]
 	add 7
-; This is a bug. The lower right corner tile of the mon back pic is blanked
-; while the mon is sliding off the screen. It should compare with the max tile
-; plus one instead.
+	; BUG: The lower right corner tile of the mon back pic is blanked
+	; while the mon is sliding off the screen. It should compare with
+	; the max tile plus one instead.
 	cp $61
 	ret c
 	ld a, " "
@@ -1844,8 +1845,9 @@ _AnimationSlideMonOff:
 .EnemyNextTile
 	ld a, [hl]
 	sub 7
-; This has the same problem as above, but it has no visible effect because
-; the lower right tile is in the first column to slide off the screen.
+	; BUG: This has the same problem as above, but it has no visible effect
+	; because the lower right tile is in the first column to slide off
+	; the screen.
 	cp $30
 	ret c
 	ld a, " "
@@ -2187,8 +2189,7 @@ AnimCopyRowRight:
 	jr nz, AnimCopyRowRight
 	ret
 
-; get the sound of the move id in b
-GetMoveSoundB:
+GetMoveSoundB: ; unused
 	ld a, b
 	call GetMoveSound
 	ld b, a

engine/battle/core.asm

@@ -544,7 +544,7 @@ HurtByLeechSeedText:
 	text_end
 
 ; decreases the mon's current HP by 1/16 of the Max HP (multiplied by number of toxic ticks if active)
-; note that the toxic ticks are considered even if the damage is not poison (hence the Leech Seed glitch)
+; note that the toxic ticks are considered even if the damage is not poison (hence the Leech Seed bug)
 ; hl: HP pointer
 ; bc (out): total damage
 HandlePoisonBurnLeechSeed_DecreaseOwnHP:
@@ -744,7 +744,7 @@ FaintEnemyPokemon:
 .wild
 	ld hl, wPlayerBattleStatus1
 	res ATTACKING_MULTIPLE_TIMES, [hl]
-; Bug. This only zeroes the high byte of the player's accumulated damage,
+; BUG: This only zeroes the high byte of the player's accumulated damage,
 ; setting the accumulated damage to itself mod 256 instead of 0 as was probably
 ; intended. That alone is problematic, but this mistake has another more severe
 ; effect. This function's counterpart for when the player mon faints,
@@ -794,7 +794,7 @@ FaintEnemyPokemon:
 	ld a, MUSIC_DEFEATED_WILD_MON
 	call PlayBattleVictoryMusic
 .sfxplayed
-; bug: win sfx is played for wild battles before checking for player mon HP
+; BUG: win sfx is played for wild battles before checking for player mon HP
 ; this can lead to odd scenarios where both player and enemy faint, as the win sfx plays yet the player never won the battle
 	ld hl, wBattleMonHP
 	ld a, [hli]
@@ -1177,7 +1177,7 @@ LinkBattleLostText:
 	text_end
 
 ; slides pic of fainted mon downwards until it disappears
-; bug: when this is called, [hAutoBGTransferEnabled] is non-zero, so there is screen tearing
+; BUG: when this is called, [hAutoBGTransferEnabled] is non-zero, so there is screen tearing
 SlideDownFaintedMonPic:
 	ld a, [wStatusFlags5]
 	push af
@@ -1230,7 +1230,7 @@ SevenSpacesText:
 ; slides the player or enemy trainer off screen
 ; a is the number of tiles to slide it horizontally (always 9 for the player trainer or 8 for the enemy trainer)
 ; if a is 8, the slide is to the right, else it is to the left
-; bug: when this is called, [hAutoBGTransferEnabled] is non-zero, so there is screen tearing
+; BUG: when this is called, [hAutoBGTransferEnabled] is non-zero, so there is screen tearing
 SlideTrainerPicOffScreen:
 	ldh [hSlideAmount], a
 	ld c, a
@@ -2013,26 +2013,17 @@ DisplayBattleMenu::
 .menuselected
 	ld [wTextBoxID], a
 	call DisplayTextBoxID
- ; handle menu input if it's not the old man tutorial
 	ld a, [wBattleType]
-	dec a
+	dec a ; BATTLE_TYPE_OLD_MAN
 	jp nz, .handleBattleMenuInput
-; the following happens for the old man tutorial
-	; Temporarily save the player name in wLinkEnemyTrainerName.
-	; Since wLinkEnemyTrainerName == wGrassRate, this affects wild encounters.
-	; The wGrassRate byte and following wGrassMons buffer are supposed
-	; to get overwritten when entering a map with wild Pokémon,
-	; but an oversight prevents this in Cinnabar and Route 21,
-	; so the infamous MissingNo. glitch can show up.
 	ld hl, wPlayerName
-	ld de, wLinkEnemyTrainerName
+	ld de, wLinkEnemyTrainerName ; same as wGrassRate
 	ld bc, NAME_LENGTH
 	call CopyData
 	ld hl, .oldManName
 	ld de, wPlayerName
 	ld bc, NAME_LENGTH
 	call CopyData
-; the following simulates the keystrokes by drawing menus on screen
 	hlcoord 9, 14
 	ld [hl], "▶"
 	ld c, 80
@@ -2043,7 +2034,7 @@ DisplayBattleMenu::
 	ld c, 50
 	call DelayFrames
 	ld [hl], "▷"
-	ld a, $2 ; select the "ITEM" menu
+	ld a, 2 ; ITEM
 	jp .upperLeftMenuItemWasNotSelected
 .oldManName
 	db "OLD MAN@"
@@ -2133,19 +2124,16 @@ DisplayBattleMenu::
 	ld a, [wCurrentMenuItem]
 	ld [wBattleAndStartSavedMenuItem], a
 	jr z, .handleMenuSelection
-; not Safari battle
-; swap the IDs of the item menu and party menu (this is probably because they swapped the positions
-; of these menu items in first generation English versions)
-	cp $1 ; was the item menu selected?
+; Swap the ITEM and <PK><MN> battle menu functions.
+; This change applies to all Western localizations.
+	cp 1 ; ITEM
 	jr nz, .notItemMenu
-; item menu was selected
-	inc a ; increment a to 2
+	inc a
 	jr .handleMenuSelection
 .notItemMenu
-	cp $2 ; was the party menu selected?
+	cp 2 ; <PK><MN>
 	jr nz, .handleMenuSelection
-; party menu selected
-	dec a ; decrement a to 1
+	dec a
 .handleMenuSelection
 	and a
 	jr nz, .upperLeftMenuItemWasNotSelected
@@ -2489,8 +2477,7 @@ MoveSelectionMenu:
 	hlcoord 4, 12
 	ld b, 4
 	ld c, 14
-	di ; out of pure coincidence, it is possible for vblank to occur between the di and ei
-	   ; so it is necessary to put the di ei block to not cause tearing
+	di
 	call TextBoxBorder
 	hlcoord 4, 12
 	ld [hl], "─"
@@ -4630,8 +4617,9 @@ CriticalHitTest:
 	ld c, [hl]                   ; read move id
 	ld a, [de]
 	bit GETTING_PUMPED, a        ; test for focus energy
-	jr nz, .focusEnergyUsed      ; bug: using focus energy causes a shift to the right instead of left,
-	                             ; resulting in 1/4 the usual crit chance
+	; BUG: Using Focus Energy causes a shift to the right
+	; instead of left, resulting in 1/4 the usual crit chance.
+	jr nz, .focusEnergyUsed
 	sla b                        ; (effective (base speed/2)*2)
 	jr nc, .noFocusEnergyUsed
 	ld b, $ff                    ; cap at 255/256
@@ -4675,7 +4663,7 @@ HandleCounterMove:
 ; This is irrelevant for the opponent's side outside of link battles, since the move selection is controlled by the AI.
 ; However, in the scenario where the player switches out and the opponent uses Counter,
 ; the outcome may be affected by the player's actions in the move selection menu prior to switching the Pokemon.
-; This might also lead to desync glitches in link battles.
+; This might also lead to desync bugs in link battles.
 
 	ldh a, [hWhoseTurn] ; whose turn
 	and a
@@ -4977,7 +4965,7 @@ AttackSubstitute:
 ; Self-confusion damage as well as Hi-Jump Kick and Jump Kick recoil cause a momentary turn swap before being applied.
 ; If the user has a Substitute up and would take damage because of that,
 ; damage will be applied to the other player's Substitute.
-; Normal recoil such as from Double-Edge isn't affected by this glitch,
+; Normal recoil such as from Double-Edge isn't affected by this bug,
 ; because this function is never called in that case.
 
 	ld hl, SubstituteTookDamageText
@@ -5324,7 +5312,8 @@ AIGetTypeEffectiveness:
 	inc hl
 	ld c, [hl]                 ; c = type 2 of player's pokemon
 	; initialize to neutral effectiveness
-	ld a, $10 ; bug: should be EFFECTIVE (10)
+	; BUG: should be EFFECTIVE (10)
+	ld a, $10
 	ld [wTypeEffectiveness], a
 	ld hl, TypeEffects
 .loop
@@ -6385,8 +6374,7 @@ LoadPlayerBackPic:
 	hlcoord 1, 5
 	predef_jump CopyUncompressedPicToTilemap
 
-; does nothing since no stats are ever selected (barring glitches)
-DoubleOrHalveSelectedStats:
+DoubleOrHalveSelectedStats: ; unused
 	callfar DoubleSelectedStats
 	jpfar HalveSelectedStats
 

engine/battle/move_effects/substitute.asm

@@ -36,8 +36,9 @@ SubstituteEffect_:
 	ld a, [hl]
 	sbc 0
 	pop bc
+	; BUG: since it only branches on carry,
+	; it will possibly leave user with 0 HP.
 	jr c, .notEnoughHP ; underflow means user would be left with negative health
-                           ; bug: since it only branches on carry, it will possibly leave user with 0 HP
 .userHasZeroOrMoreHP
 	ldi [hl], a ; save resulting HP after subtraction into current HP
 	ld [hl], d

engine/battle/unused_stats_functions.asm

@@ -1,5 +1,4 @@
-; does nothing since no stats are ever selected (barring glitches)
-DoubleSelectedStats:
+DoubleSelectedStats: ; unused
 	ldh a, [hWhoseTurn]
 	and a
 	ld a, [wPlayerStatsToDouble]
@@ -28,8 +27,7 @@ DoubleSelectedStats:
 	ld [hli], a
 	ret
 
-; does nothing since no stats are ever selected (barring glitches)
-HalveSelectedStats:
+HalveSelectedStats: ; unused
 	ldh a, [hWhoseTurn]
 	and a
 	ld a, [wPlayerStatsToHalve]

engine/battle/wild_encounters.asm

@@ -1,11 +1,9 @@
-; try to initiate a wild pokemon encounter
-; returns success in Z
 TryDoWildEncounter:
 	ld a, [wNPCMovementScriptPointerTableNum]
 	and a
 	ret nz
 	ld a, [wMovementFlags]
-	and a ; is player exiting a door, jumping over a ledge, or fishing?
+	and a
 	ret nz
 	callfar IsPlayerStandingOnDoorTileOrWarpTile
 	jr nc, .notStandingOnDoorOrWarpTile
@@ -23,30 +21,37 @@ TryDoWildEncounter:
 	jr z, .lastRepelStep
 	ld [wRepelRemainingSteps], a
 .next
-; determine if wild pokemon can appear in the half-block we're standing in
-; is the bottom right tile (9,9) of the half-block we're standing in a grass/water tile?
-	hlcoord 9, 9
+	; BUG: This coordinates set is inconsistent with
+	; the one in the .gotEncounterSlot subroutine below.
+	; It was changed from the Japanese versions and
+	; reverted for the Western localizations of Yellow.
+	; A (9, 9) coordinates set affects wild encounters as
+	; they do not occur in tile $34 of the FOREST tileset.
+	; The inconsistency causes wGrassRate, which is also
+	; wLinkEnemyTrainerName, in both NPC trades and the
+	; old man battle type to have random wild encounter data.
+	; This localization issue is infamously responsible for
+	; the discovery of MISSINGNO. in English Red and Blue
+	; while surfing on the eastern shore of CINNABAR_ISLAND,
+	; which is actually part of the ROUTE_20 map.
+	hlcoord 9, 9 ; should be hlcoord 8, 9
 	ld c, [hl]
 	ld a, [wGrassTile]
 	cp c
-	ld a, [wGrassRate]
+	ld a, [wGrassRate] ; same as [wLinkEnemyTrainerName]
 	jr z, .CanEncounter
-	ld a, $14 ; in all tilesets with a water tile, this is its id
+	ld a, $14 ; water tile
 	cp c
 	ld a, [wWaterRate]
 	jr z, .CanEncounter
-; even if not in grass/water, standing anywhere we can encounter pokemon
-; so long as the map is "indoor" and has wild pokemon defined.
-; ...as long as it's not Viridian Forest or Safari Zone.
 	ld a, [wCurMap]
-	cp FIRST_INDOOR_MAP ; is this an indoor map?
+	cp FIRST_INDOOR_MAP
 	jr c, .CantEncounter2
 	ld a, [wCurMapTileset]
-	cp FOREST ; Viridian Forest/Safari Zone
+	cp FOREST ; VIRIDIAN_FOREST, SAFARI_ZONE
 	jr z, .CantEncounter2
-	ld a, [wGrassRate]
+	ld a, [wGrassRate] ; same as [wLinkEnemyTrainerName]
 .CanEncounter
-; compare encounter chance with a random number to determine if there will be an encounter
 	ld b, a
 	ldh a, [hRandomAdd]
 	cp b
@@ -61,15 +66,12 @@ TryDoWildEncounter:
 	inc hl
 	jr .determineEncounterSlot
 .gotEncounterSlot
-; determine which wild pokemon (grass or water) can appear in the half-block we're standing in
 	ld c, [hl]
 	ld hl, wGrassMons
 	lda_coord 8, 9
-	cp $14 ; is the bottom left tile (8,9) of the half-block we're standing in a water tile?
-	jr nz, .gotWildEncounterType ; else, it's treated as a grass tile by default
+	cp $14 ; water tile
+	jr nz, .gotWildEncounterType
 	ld hl, wWaterMons
-; since the bottom right tile of a "left shore" half-block is $14 but the bottom left tile is not,
-; "left shore" half-blocks (such as the one in the east coast of Cinnabar) load grass encounters.
 .gotWildEncounterType
 	ld b, 0
 	add hl, bc
@@ -85,7 +87,7 @@ TryDoWildEncounter:
 	ld b, a
 	ld a, [wCurEnemyLevel]
 	cp b
-	jr c, .CantEncounter2 ; repel prevents encounters if the leading party mon's level is higher than the wild mon
+	jr c, .CantEncounter2
 	jr .willEncounter
 .lastRepelStep
 	ld [wRepelRemainingSteps], a