summaryrefslogtreecommitdiff
path: root/docs/design_flaws.md
diff options
context:
space:
mode:
Diffstat (limited to 'docs/design_flaws.md')
-rw-r--r--docs/design_flaws.md428
1 files changed, 336 insertions, 92 deletions
diff --git a/docs/design_flaws.md b/docs/design_flaws.md
index 7dc4d13db..613f90b0c 100644
--- a/docs/design_flaws.md
+++ b/docs/design_flaws.md
@@ -8,8 +8,9 @@ These are parts of the code that do not work *incorrectly*, like [bugs and glitc
- [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)
+- [Pokédex entry banks are derived from their species IDs](#pokédex-entry-banks-are-derived-from-their-species-ids)
+- [Identical sine wave code and data is repeated five times](#identical-sine-wave-code-and-data-is-repeated-five-times)
- [`GetForestTreeFrame` works, but it's still bad](#getforesttreeframe-works-but-its-still-bad)
@@ -109,15 +110,15 @@ ROMX $49
Two routines in [engine/load_pics.asm](/engine/load_pics.asm) make this assumption; `GetFrontpicPointer`:
```asm
- ld a, [CurPartySpecies]
+ ld a, [wCurPartySpecies]
cp UNOWN
jr z, .unown
- ld a, [CurPartySpecies]
+ ld a, [wCurPartySpecies]
ld d, BANK(PokemonPicPointers)
jr .ok
.unown
- ld a, [UnownLetter]
+ ld a, [wUnownLetter]
ld d, BANK(UnownPicPointers)
.ok
@@ -130,9 +131,7 @@ Two routines in [engine/load_pics.asm](/engine/load_pics.asm) make this assumpti
And `GetMonBackpic`:
```asm
- ; These are assumed to be at the same
- ; address in their respective banks.
- GLOBAL PokemonPicPointers, UnownPicPointers
+ ; These are assumed to be at the same address in their respective banks.
ld hl, PokemonPicPointers ; UnownPicPointers
ld a, b
ld d, BANK(PokemonPicPointers)
@@ -153,16 +152,16 @@ Don't enforce `org $4000` in pokecrystal.link.
Modify `GetFrontpicPointer`:
```asm
- ld a, [CurPartySpecies]
+ ld a, [wCurPartySpecies]
cp UNOWN
jr z, .unown
- ld a, [CurPartySpecies]
+ ld a, [wCurPartySpecies]
ld hl, PokemonPicPointers
ld d, BANK(PokemonPicPointers)
jr .ok
.unown
- ld a, [UnownLetter]
+ ld a, [wUnownLetter]
ld hl, UnownPicPointers
ld d, BANK(UnownPicPointers)
@@ -175,7 +174,6 @@ Modify `GetFrontpicPointer`:
And `GetMonBackpic`:
```asm
- GLOBAL PokemonPicPointers, UnownPicPointers
ld a, b
ld hl, PokemonPicPointers
ld d, BANK(PokemonPicPointers)
@@ -234,7 +232,7 @@ INCBIN "gfx/footprints/wartortle.1bpp", footprint_bottom
push hl
ld e, l
ld d, h
- ld hl, VTiles2 tile $62
+ ld hl, vTiles2 tile $62
lb bc, BANK(Footprints), 2
call Request1bpp
pop hl
@@ -246,7 +244,7 @@ INCBIN "gfx/footprints/wartortle.1bpp", footprint_bottom
ld e, l
ld d, h
- ld hl, VTiles2 tile $64
+ ld hl, vTiles2 tile $64
lb bc, BANK(Footprints), 2
call Request1bpp
```
@@ -272,12 +270,95 @@ Modify `Pokedex_LoadAnyFootprint`:
```asm
ld e, l
ld d, h
- ld hl, VTiles2 tile $62
+ ld hl, vTiles2 tile $62
lb bc, BANK(Footprints), 4
call Request1bpp
```
+## `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
+```
+
+**Fix:**
+
+Move `ITEM_C3` and `ITEM_DC` above all the TMs in every table of item data.
+
+Modify engine/items.asm:
+
+```asm
+GetTMHMNumber:: ; d407
+; Return the number of a TM/HM by item id c.
+ ld a, c
+ sub TM01
+ inc a
+ ld c, a
+ ret
+
+GetNumberedTMHM: ; d417
+; Return the item id of a TM/HM by number c.
+ ld a, c
+ add TM01
+ dec a
+ ld c, a
+ ret
+```
+
+
## 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 Pokémon's base stats. (This is the only use the base stat species ID has.)
@@ -301,7 +382,7 @@ GetDexEntryPointer: ; 44333
push de
rlca
rlca
- and $3
+ maskbits NUM_DEX_ENTRY_BANKS
ld hl, .PokedexEntryBanks
ld d, 0
ld e, a
@@ -311,13 +392,7 @@ GetDexEntryPointer: ; 44333
pop hl
ret
-.PokedexEntryBanks: ; 44351
-
-GLOBAL PokedexEntries1
-GLOBAL PokedexEntries2
-GLOBAL PokedexEntries3
-GLOBAL PokedexEntries4
-
+.PokedexEntryBanks:
db BANK(PokedexEntries1)
db BANK(PokedexEntries2)
db BANK(PokedexEntries3)
@@ -330,10 +405,10 @@ GLOBAL PokedexEntries4
GetPokedexEntryBank:
push hl
push de
- ld a, [EnemyMonSpecies]
+ ld a, [wEnemyMonSpecies]
rlca
rlca
- and 3
+ maskbits NUM_DEX_ENTRY_BANKS
ld hl, .PokedexEntryBanks
ld d, 0
ld e, a
@@ -344,12 +419,6 @@ GetPokedexEntryBank:
ret
.PokedexEntryBanks:
-
-GLOBAL PokedexEntries1
-GLOBAL PokedexEntries2
-GLOBAL PokedexEntries3
-GLOBAL PokedexEntries4
-
db BANK(PokedexEntries1)
db BANK(PokedexEntries2)
db BANK(PokedexEntries3)
@@ -362,12 +431,12 @@ And `PokedexShow_GetDexEntryBank` in [engine/radio.asm](/engine/radio.asm):
PokedexShow_GetDexEntryBank:
push hl
push de
- ld a, [CurPartySpecies]
+ ld a, [wCurPartySpecies]
dec a
rlca
rlca
- and 3
- ld hl, .pokedexbanks
+ maskbits NUM_DEX_ENTRY_BANKS
+ ld hl, .PokedexEntryBanks
ld d, 0
ld e, a
add hl, de
@@ -376,7 +445,7 @@ PokedexShow_GetDexEntryBank:
pop hl
ret
-.pokedexbanks
+.PokedexEntryBanks:
db BANK(PokedexEntries1)
db BANK(PokedexEntries2)
db BANK(PokedexEntries3)
@@ -386,88 +455,263 @@ PokedexShow_GetDexEntryBank:
**Fix:** Use `dba` instead of `dw` in `PokedexDataPointerTable`, and modify the code that accesses it to match.
-## `ITEM_C3` and `ITEM_DC` break up the continuous sequence of TM items
+## Identical sine wave code and data is repeated five times
-[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.
+`_Sine` in [engine/routines/sine.asm](/engine/routines/sine.asm):
```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
+_Sine:: ; 84d9
+; A simple sine function.
+; Return d * sin(e) in hl.
+
+; e is a signed 6-bit value.
+ ld a, e
+ and %111111
+ cp %100000
+ jr nc, .negative
+ call .ApplySineWave
+ ld a, h
+ ret
+
+.negative
+ and %011111
+ call .ApplySineWave
+ ld a, h
+ xor $ff
+ inc a
+ ret
+
+.ApplySineWave: ; 84ef
+ ld e, a
+ ld a, d
+ ld d, 0
+ ld hl, .sinewave
+ add hl, de
+ add hl, de
+ ld e, [hl]
+ inc hl
+ ld d, [hl]
+ ld hl, 0
+; Factor amplitude
+.multiply
+ srl a
+ jr nc, .even
+ add hl, de
+.even
+ sla e
+ rl d
+ and a
+ jr nz, .multiply
+ ret
+
+.sinewave ; 850b
+; A 32-word table representing a sine wave.
+; sin(90 degrees) is index $10 with an amplitude of $100.
+ sine_wave 32
```
-`GetTMHMNumber` and `GetNumberedTMHM` in [engine/items.asm](/engine/items.asm) have to compensate for this:
+`Sprites_Cosine` and `Sprites_Sine` in [engine/sprites.asm](/engine/sprites.asm):
```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
+Sprites_Cosine: ; 8e72a
+; a = d * cos(a * pi/32)
+ add %010000
+Sprites_Sine: ; 8e72c
+; a = d * sin(a * pi/32)
+ and %111111
+ cp %100000
+ jr nc, .negative
+ call .ApplySineWave
+ ld a, h
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
+.negative
+ and %011111
+ call .ApplySineWave
+ ld a, h
+ xor $ff
inc a
-.skip_one
+ ret
+; 8e741
+
+.ApplySineWave: ; 8e741
+ ld e, a
+ ld a, d
+ ld d, 0
+ ld hl, .sinewave
+ add hl, de
+ add hl, de
+ ld e, [hl]
+ inc hl
+ ld d, [hl]
+ ld hl, 0
+.multiply
+ srl a
+ jr nc, .even
+ add hl, de
+.even
+ sla e
+ rl d
+ and a
+ jr nz, .multiply
+ ret
+; 8e75d
+
+.sinewave ; 8e75d
+ sine_wave 32
+```
+
+`BattleAnim_Cosine` and `BattleAnim_Sine` in [engine/battle_anims/functions.asm](/engine/battle_anims/functions.asm):
+
+```asm
+BattleAnim_Cosine: ; ce732 (33:6732)
+; a = d * cos(a * pi/32)
+ add %010000
+BattleAnim_Sine: ; ce734 (33:6734)
+; a = d * sin(a * pi/32)
+ and %111111
+ cp %100000
+ jr nc, .negative
+ call .ApplySineWave
+ ld a, h
+ ret
+
+.negative
+ and %011111
+ call .ApplySineWave
+ ld a, h
+ xor $ff
inc a
-.done
- add TM01
- dec a
- ld c, a
ret
+
+.ApplySineWave:
+ ld e, a
+ ld a, d
+ ld d, 0
+ ld hl, BattleAnimSineWave
+ add hl, de
+ add hl, de
+ ld e, [hl]
+ inc hl
+ ld d, [hl]
+ ld hl, 0
+.multiply
+ srl a
+ jr nc, .even
+ add hl, de
+.even
+ sla e
+ rl d
+ and a
+ jr nz, .multiply
+ ret
+
+...
+
+BattleAnimSineWave: ; ce77f
+ sine_wave 32
+; ce7bf
```
-**Fix:**
+`StartTrainerBattle_DrawSineWave` in [engine/battle/battle_transition.asm](/engine/battle/battle_transition.asm):
-Move `ITEM_C3` and `ITEM_DC` above all the TMs in every table of item data.
+```asm
+StartTrainerBattle_DrawSineWave: ; 8c6f7 (23:46f7)
+; a = d * sin(a * pi/32)
+ and %111111
+ cp %100000
+ jr nc, .negative
+ call .ApplySineWave
+ ld a, h
+ ret
-Modify engine/items.asm:
+.negative
+ and %011111
+ call .ApplySineWave
+ ld a, h
+ xor $ff
+ inc a
+ ret
+
+.ApplySineWave: ; 8c70c (23:470c)
+ ld e, a
+ ld a, d
+ ld d, 0
+ ld hl, .sinewave
+ add hl, de
+ add hl, de
+ ld e, [hl]
+ inc hl
+ ld d, [hl]
+ ld hl, 0
+.multiply
+ srl a
+ jr nc, .even
+ add hl, de
+.even
+ sla e
+ rl d
+ and a
+ jr nz, .multiply
+ ret
+; 8c728 (23:4728)
+
+.sinewave ; 8c728
+ sine_wave 32
+; 8c768
+```
+
+And `CelebiEvent_Cosine` in [engine/events/celebi.asm](/engine/events/celebi.asm):
```asm
-GetTMHMNumber:: ; d407
-; Return the number of a TM/HM by item id c.
- ld a, c
- sub TM01
+CelebiEvent_Cosine: ; 49b3b (12:5b3b)
+; a = d * cos(a * pi/32)
+ add %010000
+ and %111111
+ cp %100000
+ jr nc, .negative
+ call .ApplySineWave
+ ld a, h
+ ret
+
+.negative
+ and %011111
+ call .ApplySineWave
+ ld a, h
+ xor $ff
inc a
- ld c, a
ret
-GetNumberedTMHM: ; d417
-; Return the item id of a TM/HM by number c.
- ld a, c
- add TM01
- dec a
- ld c, a
+.ApplySineWave: ; 49b52 (12:5b52)
+ ld e, a
+ ld a, d
+ ld d, 0
+ ld hl, .sinewave
+ add hl, de
+ add hl, de
+ ld e, [hl]
+ inc hl
+ ld d, [hl]
+ ld hl, 0
+.multiply
+ srl a
+ jr nc, .even
+ add hl, de
+.even
+ sla e
+ rl d
+ and a
+ jr nz, .multiply
ret
+; 49b6e (12:5b6e)
+
+.sinewave ; 49b6e
+ sine_wave 32
+; 49bae
```
+**Fix:** Call a single shared copy of the (co)sine code in bank 0.
+
## `GetForestTreeFrame` works, but it's still bad
generated by cgit v1.2.3 (git 2.25.1) at 2025-06-04 18:59:15 +0000