Merge pull request #445 from roukaour/master

Rename Pokémon animation files; consistent (x, y) coords; document design flaws

constants/pokemon_constants.asm

@@ -13,7 +13,7 @@
 ; - NewPokedexOrder (see data/pokemon/dex_order_new.asm)
 ; - Pokered_MonIndices (see data/time_capsule/mon_order.asm)
 ; - AnimationPointers (see gfx/pokemon/anim_pointers.asm)
-; - AnimationExtraPointers (see gfx/pokemon/extra_pointers.asm)
+; - AnimationIdlePointers (see gfx/pokemon/idle_pointers.asm)
 ; - BitmasksPointers (see gfx/pokemon/bitmask_pointers.asm)
 ; - FramesPointers (see gfx/pokemon/frame_pointers.asm)
 ; - Footprints (see gfx/footprints.asm)

data/maps/spawn_points.asm

@@ -1,5 +1,5 @@
 spawn: MACRO
-; map, y, x
+; map, x, y
 	map \1
 	db \2, \3
 ENDM

docs/bugs_and_glitches.md

@@ -1,11 +1,14 @@
 # Bugs and Glitches
 
+These are known bugs and glitches in the original Pokémon Crystal game: code that clearly does not work as intended, or that only works in limited circumstances but has the possibility to fail or crash.
+
 
 ## Contents
 
 - [Thick Club and Light Ball can decrease damage done with boosted (Special) Attack](#thick-club-and-light-ball-can-decrease-damage-done-with-boosted-special-attack)
 - [Metal Powder can increase damage taken with boosted (Special) Defense](#metal-powder-can-increase-damage-taken-with-boosted-special-defense)
 - [Belly Drum sharply boosts Attack even with under 50% HP](#belly-drum-sharply-boosts-attack-even-with-under-50-hp)
+- [Confusion damage is affected by type-boosting items and Explosion/Self-Destruct doubling](#confusion-damage-is-affected-by-type-boosting-items-and-explosionself-destruct-doubling)
 - [Moves that lower Defense can do so after breaking a Substitute](#moves-that-lower-defense-can-do-so-after-breaking-a-substitute)
 - [Counter and Mirror Coat still work if the opponent uses an item](#counter-and-mirror-coat-still-work-if-the-opponent-uses-an-item)
 - [A Disabled but PP Up–enhanced move may not trigger Struggle](#a-disabled-but-pp-upenhanced-move-may-not-trigger-struggle)
@@ -27,6 +30,7 @@
 - [Dragon Scale, not Dragon Fang, boosts Dragon-type moves](#dragon-scale-not-dragon-fang-boosts-dragon-type-moves)
 - [Daisy's grooming doesn't always increase happiness](#daisys-grooming-doesnt-always-increase-happiness)
 - [Magikarp in Lake of Rage are shorter, not longer](#magikarp-in-lake-of-rage-are-shorter-not-longer)
+- [Magikarp length limits have a unit conversion error](#magikarp-length-limits-have-a-unit-conversion-error)
 - [Magikarp lengths can be miscalculated](#magikarp-lengths-can-be-miscalculated)
 - [Battle transitions fail to account for the enemy's level](#battle-transitions-fail-to-account-for-the-enemys-level)
 - [Slot machine payout sound effects cut each other off](#slot-machine-payout-sound-effects-cut-each-other-off)
@@ -49,11 +53,12 @@
 - [`TryObjectEvent` arbitrary code execution](#tryobjectevent-arbitrary-code-execution)
 - [`Special_CheckBugContestContestantFlag` can read beyond its data table](#special_checkbugcontestcontestantflag-can-read-beyond-its-data-table)
 - [`ClearWRAM` only clears WRAM bank 1](#clearwram-only-clears-wram-bank-1)
-- [`GetForestTreeFrame` works, but it's still bad](#getforesttreeframe-works-but-its-still-bad)
 
 
 ## Thick Club and Light Ball can decrease damage done with boosted (Special) Attack
 
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
 ([Video](https://www.youtube.com/watch?v=rGqu3d3pdok&t=450))
 
 This is a bug with `SpeciesItemBoost` in [engine/battle/effect_commands.asm](/engine/battle/effect_commands.asm):
@@ -88,6 +93,8 @@ This is a bug with `SpeciesItemBoost` in [engine/battle/effect_commands.asm](/en
 
 ## Metal Powder can increase damage taken with boosted (Special) Defense
 
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
 ([Video](https://www.youtube.com/watch?v=rGqu3d3pdok&t=450))
 
 This is a bug with `DittoMetalPowder` in [engine/battle/effect_commands.asm](/engine/battle/effect_commands.asm):
@@ -144,6 +151,8 @@ This is a bug with `DittoMetalPowder` in [engine/battle/effect_commands.asm](/en
 
 ## Belly Drum sharply boosts Attack even with under 50% HP
 
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
 ([Video](https://www.youtube.com/watch?v=zuCLMikWo4Y))
 
 This is a bug with `BattleCommand_BellyDrum` in [engine/battle/effect_commands.asm](/engine/battle/effect_commands.asm):
@@ -180,8 +189,19 @@ BattleCommand_BellyDrum: ; 37c1a
 ```
 
 
+## Confusion damage is affected by type-boosting items and Explosion/Self-Destruct doubling
+
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
+([Video](https://twitter.com/crystal_rby/status/874626362287562752))
+
+*To do:* Identify specific code causing this bug and fix it.
+
+
 ## Moves that lower Defense can do so after breaking a Substitute
 
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
 ([Video](https://www.youtube.com/watch?v=OGwKPRJLaaI))
 
 This bug affects Acid, Iron Tail, and Rock Smash.
@@ -218,6 +238,8 @@ DefenseDownHit:
 
 ## Counter and Mirror Coat still work if the opponent uses an item
 
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
 ([Video](https://www.youtube.com/watch?v=uRYyzKRatFk))
 
 *To do:* Identify specific code causing this bug and fix it.
@@ -225,6 +247,8 @@ DefenseDownHit:
 
 ## A Disabled but PP Up–enhanced move may not trigger Struggle
 
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
 ([Video](https://www.youtube.com/watch?v=1v9x4SgMggs))
 
 This is a bug with `CheckPlayerHasUsableMoves` in [engine/battle/core.asm](/engine/battle/core.asm):
@@ -250,6 +274,8 @@ This is a bug with `CheckPlayerHasUsableMoves` in [engine/battle/core.asm](/engi
 
 ## A Pokémon that fainted from Pursuit will have its old status condition when revived
 
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
 ([Video](https://www.youtube.com/watch?v=tiRvw-Nb2ME))
 
 *To do:* Identify specific code causing this bug and fix it.
@@ -257,6 +283,8 @@ This is a bug with `CheckPlayerHasUsableMoves` in [engine/battle/core.asm](/engi
 
 ## Lock-On and Mind Reader don't always bypass Fly and Dig
 
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
 This bug affects Attract, Curse, Foresight, Mean Look, Mimic, Nightmare, Spider Web, Transform, and stat-lowering effects of moves like String Shot or Bubble during the semi-invulnerable turn of Fly or Dig.
 
 This is a bug with `CheckHiddenOpponent` in [engine/battle/effect_commands.asm](/engine/battle/effect_commands.asm):
@@ -275,6 +303,8 @@ CheckHiddenOpponent: ; 37daa
 
 ## Beat Up can desynchronize link battles
 
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
 ([Video](https://www.youtube.com/watch?v=202-iAsrIa8))
 
 This is a bug with `BattleCommand_BeatUp` in [engine/battle/effect_commands.asm](/engine/battle/effect_commands.asm):
@@ -310,6 +340,8 @@ This is a bug with `BattleCommand_BeatUp` in [engine/battle/effect_commands.asm]
 
 ## Present damage is incorrect in link battles
 
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
 ([Video](https://www.youtube.com/watch?v=XJaQoKtrEuw))
 
 This bug existed for all battles in Gold and Silver, and was only fixed for single-player battles in Crystal to preserve link compatibility.
@@ -661,6 +693,8 @@ FastBallMultiplier:
 
 ## Dragon Scale, not Dragon Fang, boosts Dragon-type moves
 
+*Fixing this bug will break compatibility with standard Pokémon Crystal for link battles.*
+
 This is a bug with `ItemAttributes` in [items/attributes.asm](/items/attributes.asm):
 
 ```asm
@@ -751,6 +785,43 @@ This is a bug with `LoadEnemyMon.CheckMagikarpArea` in [engine/battle/core.asm](
 **Fix:** Change both `jr z, .Happiness` to `jr nz, .Happiness`.
 
 
+## Magikarp length limits have a unit conversion error
+
+This is a bug with `LoadEnemyMon.CheckMagikarpArea` in [engine/battle/core.asm](/engine/battle/core.asm):
+
+```asm
+; Get Magikarp's length
+	ld de, EnemyMonDVs
+	ld bc, PlayerID
+	callfar CalcMagikarpLength
+
+; No reason to keep going if length > 1536 (i.e. if length / 256 != 6)
+	ld a, [wMagikarpLength]
+	cp HIGH(1536) ; this compares to 6'0'', should be cp 5
+	jr nz, .CheckMagikarpArea
+
+; 5% chance of skipping both size checks
+	call Random
+	cp 5 percent
+	jr c, .CheckMagikarpArea
+; Try again if length > 1615
+	ld a, [wMagikarpLength + 1]
+	cp LOW(1616) ; this compares to 6'80'', should be cp 3
+	jr nc, .GenerateDVs
+
+; 20% chance of skipping this check
+	call Random
+	cp 20 percent - 1
+	jr c, .CheckMagikarpArea
+; Try again if length > 1599
+	ld a, [wMagikarpLength + 1]
+	cp LOW(1600) ; this compares to 6'64'', should be cp 2
+	jr nc, .GenerateDVs
+```
+
+**Fix:** Change the three `cp` instructions to use their commented values.
+
+
 ## Magikarp lengths can be miscalculated
 
 This is a bug with `CalcMagikarpLength.BCLessThanDE` in [engine/events/magikarp.asm](/engine/events/magikarp.asm):
@@ -1410,46 +1481,3 @@ ClearWRAM:: ; 25a
 ```
 
 **Fix:** Change `jr nc, .bank_loop` to `jr c, .bank_loop`.
-
-
-## `GetForestTreeFrame` works, but it's still bad
-
-In [tilesets/animations.asm](/tilesets/animations.asm):
-
-```asm
-GetForestTreeFrame: ; fc54c
-; Return 0 if a is even, or 2 if odd.
-	and a
-	jr z, .even
-	cp 1
-	jr z, .odd
-	cp 2
-	jr z, .even
-	cp 3
-	jr z, .odd
-	cp 4
-	jr z, .even
-	cp 5
-	jr z, .odd
-	cp 6
-	jr z, .even
-.odd
-	ld a, 2
-	scf
-	ret
-.even
-	xor a
-	ret
-; fc56d
-```
-
-**Fix:**
-
-```asm
-GetForestTreeFrame: ; fc54c
-; Return 0 if a is even, or 2 if odd.
-	and 1
-	add a
-	ret
-; fc56d
-```

docs/map_scripts.md

@@ -60,17 +60,17 @@ Callback types:
 
 ## `.Warps: db` *N*
 
-- **`warp_def` *y*, *x*, *warp_id*, *map***
+- **`warp_def` *x*, *y*, *warp_id*, *map***
 
 
 ## `.CoordEvents: db` *N*
 
-- **`coord_event` *scene id*, *y*, *x*, *script***
+- **`coord_event` *x*, *y*, *scene id*, *script***
 
 
 ## `.BGEvents: db` *N*
 
-- **`bg_event` *y*, *x*, *type*, *script***
+- **`bg_event` *x*, *y*, *type*, *script***
 
 BG event types:
 
@@ -90,7 +90,7 @@ BG event types:
 
 ## `.ObjectEvents: db` *N*
 
-- **`object_event` *sprite*, *y*, *x*, *movement*, *ry*, *rx*, *h1*, *h2*, *palette*, *type*, *range*, *script*, *event_flag***
+- **`object_event` *x*, *y*, *sprite*, *movement*, *ry*, *rx*, *h1*, *h2*, *palette*, *type*, *range*, *script*, *event_flag***
 
 Movement types:
 

engine/billspc.asm

@@ -1442,9 +1442,9 @@ copy_box_data: MACRO
 	jr .loop\@
 
 .done\@
-IF \1
+if \1
 	call CloseSRAM
-ENDC
+endc
 	ld a, -1
 	ld [de], a
 	ld a, [wd004]

engine/events/battle_tower.asm

@@ -26,13 +26,13 @@ Function_LoadOpponentTrainerAndPokemons: ; 1f8000
 	ld a, [hRandomAdd]
 	add b
 	ld b, a ; b contains the nr of the trainer
-IF DEF(CRYSTAL11)
+if DEF(CRYSTAL11)
 	and (1 << 7) - 1
 	cp 70
-ELSE
+else
 	and (1 << 5) - 1
 	cp 21
-ENDC
+endc
 	jr nc, .resample
 	ld b, a
 

engine/events/crystal_unown.asm

@@ -105,7 +105,7 @@ SpecialKabutoChamber: ; 8ae4e
 
 Special_DisplayUnownWords: ; 8ae68
 	ld a, [ScriptVar]
-	ld hl, .MenuDataHeader_Escape
+	ld hl, .MenuDataHeaders_UnownWalls
 	and a
 	jr z, .load
 
@@ -153,8 +153,6 @@ Special_DisplayUnownWords: ; 8ae68
 	ret
 ; 8aebc
 
-.UnownText: ; 8aebc
-
 unownwall: MACRO
 rept _NARG
 if \1 == "-"
@@ -180,36 +178,35 @@ endr
 	db $ff
 ENDM
 
-.UnownText_Escape:
+.UnownText: ; 8aebc
+;.UnownText_Escape:
 	; db      $08, $44, $04, $00, $2e, $08, $ff
 	unownwall "E", "S", "C", "A", "P", "E"
-.UnownText_Light:
+;.UnownText_Light:
 	; db      $26, $20, $0c, $0e, $46, $ff
 	unownwall "L", "I", "G", "H", "T"
-.UnownText_Water:
+;.UnownText_Water:
 	; db      $4c, $00, $46, $08, $42, $ff
 	unownwall "W", "A", "T", "E", "R"
-.UnownText_Ho_Oh:
+;.UnownText_Ho_Oh:
 	; db      $0e, $2c, $64, $2c, $0e, $ff
 	unownwall "H", "O", "-", "O", "H"
 ; 8aed5
 
-.MenuDataHeader_Escape: ; 0x8aed5
+.MenuDataHeaders_UnownWalls: ; 0x8aed5
+;.MenuDataHeader_Escape:
 	db $40 ; flags
 	db 04, 03 ; start coords
 	db 09, 16 ; end coords
-
+;.MenuDataHeader_Light:
-.MenuDataHeader_Light: ; 0x8aeda
 	db $40 ; flags
 	db 04, 04 ; start coords
 	db 09, 15 ; end coords
-
+;.MenuDataHeader_Water:
-.MenuDataHeader_Water: ; 0x8aedf
 	db $40 ; flags
 	db 04, 04 ; start coords
 	db 09, 15 ; end coords
-
+;.MenuDataHeader_Ho_Oh:
-.MenuDataHeader_Ho_Oh: ; 0x8aee4
 	db $40 ; flags
 	db 04, 04 ; start coords
 	db 09, 15 ; end coords

engine/intro_menu.asm

@@ -199,9 +199,9 @@ _ResetWRAM: ; 5bae
 	ld [Coins], a
 	ld [Coins + 1], a
 
-IF START_MONEY >= $10000
+if START_MONEY >= $10000
 	ld a, HIGH(START_MONEY >> 8)
-ENDC
+endc
 	ld [Money], a
 	ld a, HIGH(START_MONEY) ; mid
 	ld [Money + 1], a
@@ -646,11 +646,11 @@ Continue_DisplayPokedexNumCaught: ; 5f6b
 	ret z
 	push hl
 	ld hl, PokedexCaught
-IF NUM_POKEMON % 8
+if NUM_POKEMON % 8
 	ld b, NUM_POKEMON / 8 + 1
-ELSE
+else
 	ld b, NUM_POKEMON / 8
-ENDC
+endc
 	call CountSetBits
 	pop hl
 	ld de, wd265

engine/items.asm

@@ -455,15 +455,12 @@ CheckTMHM: ; d3fb
 
 GetTMHMNumber:: ; d407
 ; Return the number of a TM/HM by item id c.
-
 	ld a, c
-
 ; Skip any dummy items.
 	cp ITEM_C3 ; TM04-05
 	jr c, .done
 	cp ITEM_DC ; TM28-29
 	jr c, .skip
-
 	dec a
 .skip
 	dec a
@@ -475,15 +472,12 @@ GetTMHMNumber:: ; d407
 
 GetNumberedTMHM: ; d417
 ; Return the item id of a TM/HM by number c.
-
 	ld a, c
-
 ; Skip any gaps.
 	cp ITEM_C3 - (TM01 - 1)
 	jr c, .done
 	cp ITEM_DC - (TM01 - 1) - 1
 	jr c, .skip_one
-
 .skip_two
 	inc a
 .skip_one

engine/pic_animation.asm

@@ -55,7 +55,7 @@ AnimateMon_Unused: ; d003a
 pokeanim: MACRO
 	rept _NARG
 ; Workaround for a bug where macro args can't come after the start of a symbol
-if !def(\1_POKEANIM)
+if !DEF(\1_POKEANIM)
 \1_POKEANIM EQUS "PokeAnim_\1_"
 endc
 	db (\1_POKEANIM - PokeAnim_SetupCommands) / 2
@@ -77,13 +77,13 @@ PokeAnims: ; d0042
 
 .Slow:   pokeanim StereoCry, Setup2, Play
 .Normal: pokeanim StereoCry, Setup, Play
-.Menu:   pokeanim CryNoWait, Setup, Play, SetWait, Wait, Extra, Play
+.Menu:   pokeanim CryNoWait, Setup, Play, SetWait, Wait, Idle, Play
-.Trade:  pokeanim Extra, Play2, Extra, Play, SetWait, Wait, Cry, Setup, Play
+.Trade:  pokeanim Idle, Play2, Idle, Play, SetWait, Wait, Cry, Setup, Play
-.Evolve: pokeanim Extra, Play, SetWait, Wait, CryNoWait, Setup, Play
+.Evolve: pokeanim Idle, Play, SetWait, Wait, CryNoWait, Setup, Play
-.Hatch:  pokeanim Extra, Play, CryNoWait, Setup, Play, SetWait, Wait, Extra, Play
+.Hatch:  pokeanim Idle, Play, CryNoWait, Setup, Play, SetWait, Wait, Idle, Play
-.Unused: pokeanim CryNoWait, Setup, Play, SetWait, Wait, Extra, Play
+.Unused: pokeanim CryNoWait, Setup, Play, SetWait, Wait, Idle, Play
 .Egg1:   pokeanim Setup, Play
-.Egg2:   pokeanim Extra, Play
+.Egg2:   pokeanim Idle, Play
 
 
 AnimateFrontpic: ; d008e
@@ -151,7 +151,7 @@ ENDM
 	setup_command PokeAnim_Wait
 	setup_command PokeAnim_Setup
 	setup_command PokeAnim_Setup2
-	setup_command PokeAnim_Extra
+	setup_command PokeAnim_Idle
 	setup_command PokeAnim_Play
 	setup_command PokeAnim_Play2
 	setup_command PokeAnim_Cry
@@ -198,7 +198,7 @@ PokeAnim_Setup2: ; d011d
 	ret
 ; d012f
 
-PokeAnim_Extra: ; d012f
+PokeAnim_Idle: ; d012f
 	ld c, TRUE
 	ld b, 0
 	call PokeAnim_InitAnim
@@ -360,18 +360,18 @@ PokeAnim_InitPicAttributes: ; d01d6
 PokeAnim_InitAnim: ; d0228
 	ld a, [rSVBK]
 	push af
-	ld a, BANK(wPokeAnimExtraFlag)
+	ld a, BANK(wPokeAnimIdleFlag)
 	ld [rSVBK], a
 	push bc
-	ld hl, wPokeAnimExtraFlag
+	ld hl, wPokeAnimIdleFlag
-	ld bc, wPokeAnimStructEnd - wPokeAnimExtraFlag
+	ld bc, wPokeAnimStructEnd - wPokeAnimIdleFlag
 	xor a
 	call ByteFill
 	pop bc
 	ld a, b
 	ld [wPokeAnimSpeed], a
 	ld a, c
-	ld [wPokeAnimExtraFlag], a
+	ld [wPokeAnimIdleFlag], a
 	call GetMonAnimPointer
 	call GetMonFramesPointer
 	call GetMonBitmaskPointer
@@ -949,20 +949,20 @@ GetMonAnimPointer: ; d055c
 
 	ld c, BANK(UnownAnimations)
 	ld hl, UnownAnimationPointers
-	ld de, UnownAnimationExtraPointers
+	ld de, UnownAnimationIdlePointers
 	call PokeAnim_IsUnown
 	jr z, .unown
 	ld c, BANK(PicAnimations)
 	ld hl, AnimationPointers
-	ld de, AnimationExtraPointers
+	ld de, AnimationIdlePointers
 .unown
 
-	ld a, [wPokeAnimExtraFlag]
+	ld a, [wPokeAnimIdleFlag]
 	and a
-	jr z, .extras
+	jr z, .idles
 	ld h, d
 	ld l, e
-.extras
+.idles
 
 	ld a, [wPokeAnimSpeciesOrUnown]
 	dec a
@@ -982,12 +982,12 @@ GetMonAnimPointer: ; d055c
 .egg
 	ld hl, EggAnimation
 	ld c, BANK(EggAnimation)
-	ld a, [wPokeAnimExtraFlag]
+	ld a, [wPokeAnimIdleFlag]
 	and a
-	jr z, .extras_egg
+	jr z, .idles_egg
-	ld hl, EggAnimationExtra
+	ld hl, EggAnimationIdle
-	ld c, BANK(EggAnimationExtra)
+	ld c, BANK(EggAnimationIdle)
-.extras_egg
+.idles_egg
 
 	ld a, c
 	ld [wPokeAnimPointerBank], a

engine/player_object.asm

@@ -55,7 +55,7 @@ PlayerObjectTemplate: ; 8071
 ; A dummy map object used to initialize the player object.
 ; Shorter than the actual amount copied by two bytes.
 ; Said bytes seem to be unused.
-	object_event SPRITE_CHRIS, -4, -4, SPRITEMOVEDATA_PLAYER, 15, 15, -1, -1, 0, OBJECTTYPE_SCRIPT, 0, 0, -1
+	object_event -4, -4, SPRITE_CHRIS, SPRITEMOVEDATA_PLAYER, 15, 15, -1, -1, 0, OBJECTTYPE_SCRIPT, 0, 0, -1
 
 CopyDECoordsToMapObject:: ; 807e
 	push de

engine/scripting.asm

@@ -147,9 +147,9 @@ ScriptCommandTable:
 	dw Script_loadmenudata               ; 4f
 	dw Script_closewindow                ; 50
 	dw Script_jumptextfaceplayer         ; 51
-IF _CRYSTAL
+if _CRYSTAL
 	dw Script_farjumptext                ; 52
-ENDC
+endc
 	dw Script_jumptext                   ; 53
 	dw Script_waitbutton                 ; 54
 	dw Script_buttonsound                ; 55
@@ -338,7 +338,7 @@ JumpTextScript:
 	end
 
 
-IF _CRYSTAL
+if _CRYSTAL
 
 Script_farjumptext:
 ; script command 0x52
@@ -355,7 +355,7 @@ Script_farjumptext:
 	ld hl, JumpTextScript
 	jp ScriptJump
 
-ENDC
+endc
 
 
 Script_writetext: