Document design flaws

This commit is contained in:
Remy Oukaour 2017-12-28 11:28:23 -05:00
parent b373984ab9
commit fe92e4d848
2 changed files with 355 additions and 6 deletions

355
docs/design_flaws.md Normal file
View File

@ -0,0 +1,355 @@
# Design Flaws
## Contents
- [Pic banks are offset by `PICS_FIX`](#pic-banks-are-offset-by-pics_fix)
- [`PokemonPicPointers` and `UnownPicPointers` are assumed to start at the same address](#pokemonpicpointers-and-unownpicpointers-are-assumed-to-start-at-the-same-address)
- [Footprints are split into top and bottom halves](#footprints-are-split-into-top-and-bottom-halves)
- [Pokédex entry banks are derived from their species IDs](#pokédex-entry-banks-are-derived-from-their-species-ids)
- [`ITEM_C3` and `ITEM_DC` break up the continuous sequence of TM items](#item_c3-and-item_dc-break-up-the-continuous-sequence-of-tm-items)
## Pic banks are offset by `PICS_FIX`
[data/pokemon/pic_pointers.asm](/data/pokemon/pic_pointers.asm), [data/pokemon/unown_pic_pointers.asm](/data/pokemon/unown_pic_pointers.asm), and [data/trainers/pic_pointers.asm](/data/trainers/pic_pointers.asm) all have to use `dba_pic` instead of `dba`. This is a macro in [macros/data.asm](/macros/data.asm) that offsets banks by `PICS_FIX`:
```asm
dba_pic: MACRO ; dbw bank, address
db BANK(\1) - PICS_FIX
dw \1
ENDM
```
The offset is translated into a correct bank by `FixPicBank` in [gfx/load_pics.asm](/gfx/load_pics.asm):
```asm
FixPicBank: ; 511c5
; This is a thing for some reason.
PICS_FIX EQU $36
GLOBAL PICS_FIX
push hl
push bc
sub BANK(Pics_1) - PICS_FIX
ld c, a
ld b, 0
ld hl, .PicsBanks
add hl, bc
ld a, [hl]
pop bc
pop hl
ret
.PicsBanks: ; 511d4
db BANK(Pics_1) + 0
db BANK(Pics_1) + 1
db BANK(Pics_1) + 2
db BANK(Pics_1) + 3
db BANK(Pics_1) + 4
db BANK(Pics_1) + 5
db BANK(Pics_1) + 6
db BANK(Pics_1) + 7
db BANK(Pics_1) + 8
db BANK(Pics_1) + 9
db BANK(Pics_1) + 10
db BANK(Pics_1) + 11
db BANK(Pics_1) + 12
db BANK(Pics_1) + 13
db BANK(Pics_1) + 14
db BANK(Pics_1) + 15
db BANK(Pics_1) + 16
db BANK(Pics_1) + 17
db BANK(Pics_1) + 18
db BANK(Pics_1) + 19
db BANK(Pics_1) + 20
db BANK(Pics_1) + 21
db BANK(Pics_1) + 22
db BANK(Pics_1) + 23
```
## `PokemonPicPointers` and `UnownPicPointers` are assumed to start at the same address
In [gfx/pics.asm](/gfx/pics.asm):
```asm
; PokemonPicPointers and UnownPicPointers are assumed to start at the same
; address, but in different banks. This is enforced in pokecrystal.link.
SECTION "Pic Pointers", ROMX
INCLUDE "data/pokemon/pic_pointers.asm"
SECTION "Unown Pic Pointers", ROMX
INCLUDE "data/pokemon/unown_pic_pointers.asm"
```
Two routines in [gfx/load_pics.asm](/gfx/load_pics.asm) make this assumption; `GetFrontpicPointer`:
```asm
ld a, [CurPartySpecies]
cp UNOWN
jr z, .unown
ld a, [CurPartySpecies]
ld d, BANK(PokemonPicPointers)
jr .ok
.unown
ld a, [UnownLetter]
ld d, BANK(UnownPicPointers)
.ok
ld hl, PokemonPicPointers ; UnownPicPointers
dec a
ld bc, 6
call AddNTimes
```
And `GetMonBackpic`:
```asm
; These are assumed to be at the same
; address in their respective banks.
GLOBAL PokemonPicPointers, UnownPicPointers
ld hl, PokemonPicPointers ; UnownPicPointers
ld a, b
ld d, BANK(PokemonPicPointers)
cp UNOWN
jr nz, .ok
ld a, c
ld d, BANK(UnownPicPointers)
.ok
dec a
ld bc, 6
call AddNTimes
```
## Footprints are split into top and bottom halves
In [gfx/footprints.asm](/gfx/footprints.asm):
```asm
; Footprints are 2x2 tiles each, but are stored as a 16x64-tile image
; (32 rows of 8 footprints per row).
; That means there's a row of the top two tiles for eight footprints,
; then a row of the bottom two tiles for those eight footprints.
; These macros help extract the first and the last two tiles, respectively.
footprint_top EQUS "0, 2 * LEN_1BPP_TILE"
footprint_bottom EQUS "2 * LEN_1BPP_TILE, 2 * LEN_1BPP_TILE"
; Entries correspond to Pokémon species, two apiece, 8 tops then 8 bottoms
; 001-008 top halves
INCBIN "gfx/footprints/bulbasaur.1bpp", footprint_top
INCBIN "gfx/footprints/ivysaur.1bpp", footprint_top
INCBIN "gfx/footprints/venusaur.1bpp", footprint_top
INCBIN "gfx/footprints/charmander.1bpp", footprint_top
INCBIN "gfx/footprints/charmeleon.1bpp", footprint_top
INCBIN "gfx/footprints/charizard.1bpp", footprint_top
INCBIN "gfx/footprints/squirtle.1bpp", footprint_top
INCBIN "gfx/footprints/wartortle.1bpp", footprint_top
; 001-008 bottom halves
INCBIN "gfx/footprints/bulbasaur.1bpp", footprint_bottom
INCBIN "gfx/footprints/ivysaur.1bpp", footprint_bottom
INCBIN "gfx/footprints/venusaur.1bpp", footprint_bottom
INCBIN "gfx/footprints/charmander.1bpp", footprint_bottom
INCBIN "gfx/footprints/charmeleon.1bpp", footprint_bottom
INCBIN "gfx/footprints/charizard.1bpp", footprint_bottom
INCBIN "gfx/footprints/squirtle.1bpp", footprint_bottom
INCBIN "gfx/footprints/wartortle.1bpp", footprint_bottom
...
```
`Pokedex_LoadAnyFootprint` in [engine/pokedex.asm](/engine/pokedex.asm):
```asm
push hl
ld e, l
ld d, h
ld hl, VTiles2 tile $62
lb bc, BANK(Footprints), 2
call Request1bpp
pop hl
; Whoever was editing footprints forgot to fix their
; tile editor. Now each bottom half is 8 tiles off.
ld de, 8 tiles
add hl, de
ld e, l
ld d, h
ld hl, VTiles2 tile $64
lb bc, BANK(Footprints), 2
call Request1bpp
```
## Pokédex entry banks are derived from their species IDs
`PokedexDataPointerTable` in [data/pokemon/dex_entry_pointers.asm](/data/pokemon/dex_entry_pointers.asm) is a table of `dw`, not `dba`, yet there are four banks used for Pokédex entries. The correct bank is derived from the species ID at the beginning of each entry. (This is the only use that species ID has.)
Three separate routines do the same derivation; `GetDexEntryPointer` in [engine/pokedex_2.asm](/engine/pokedex_2.asm):
```asm
GetDexEntryPointer: ; 44333
; return dex entry pointer b:de
push hl
ld hl, PokedexDataPointerTable
ld a, b
dec a
ld d, 0
ld e, a
add hl, de
add hl, de
ld e, [hl]
inc hl
ld d, [hl]
push de
rlca
rlca
and $3
ld hl, .PokedexEntryBanks
ld d, 0
ld e, a
add hl, de
ld b, [hl]
pop de
pop hl
ret
.PokedexEntryBanks: ; 44351
GLOBAL PokedexEntries1
GLOBAL PokedexEntries2
GLOBAL PokedexEntries3
GLOBAL PokedexEntries4
db BANK(PokedexEntries1)
db BANK(PokedexEntries2)
db BANK(PokedexEntries3)
db BANK(PokedexEntries4)
```
`GetPokedexEntryBank` in [engine/item_effects.asm](/engine/item_effects.asm):
```asm
GetPokedexEntryBank:
push hl
push de
ld a, [EnemyMonSpecies]
rlca
rlca
and 3
ld hl, .PokedexEntryBanks
ld d, 0
ld e, a
add hl, de
ld a, [hl]
pop de
pop hl
ret
.PokedexEntryBanks:
GLOBAL PokedexEntries1
GLOBAL PokedexEntries2
GLOBAL PokedexEntries3
GLOBAL PokedexEntries4
db BANK(PokedexEntries1)
db BANK(PokedexEntries2)
db BANK(PokedexEntries3)
db BANK(PokedexEntries4)
```
And `PokedexShow_GetDexEntryBank` in [engine/radio.asm](/engine/radio.asm):
```asm
PokedexShow_GetDexEntryBank:
push hl
push de
ld a, [CurPartySpecies]
dec a
rlca
rlca
and 3
ld hl, .pokedexbanks
ld d, 0
ld e, a
add hl, de
ld a, [hl]
pop de
pop hl
ret
.pokedexbanks
db BANK(PokedexEntries1)
db BANK(PokedexEntries2)
db BANK(PokedexEntries3)
db BANK(PokedexEntries4)
```
## `ITEM_C3` and `ITEM_DC` break up the continuous sequence of TM items
[constants/item_constants.asm](/constants/item_constants.asm) defined the 50 TMs in order with `add_tm`, but `ITEM_C3` and `ITEM_DC` break up that sequence.
```asm
add_tm DYNAMICPUNCH ; $BF
...
add_tm ROLLOUT ; $C2
const ITEM_C3 ; $C3
add_tm ROAR ; $C4
...
add_tm DIG ; $DB
const ITEM_DC ; $DC
add_tm PSYCHIC_M ; $DD
...
add_tm NIGHTMARE ; $F2
NUM_TMS = const_value - TM01 - 2 ; discount ITEM_C3 and ITEM_DC
```
`GetTMHMNumber` and `GetNumberedTMHM` in [engine/items.asm](/engine/items.asm) have to compensate for this:
```asm
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
.done
sub TM01
inc a
ld c, a
ret
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
inc a
.done
add TM01
dec a
ld c, a
ret
```

View File

@ -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