ref: 7167895612387ce676c4e3606cac07101719ee2f
parent: 805474b0db4a87d230852c7ce6803f5151a321a6
author: Remy Oukaour <remy.oukaour@gmail.com>
date: Thu Dec 28 07:49:32 EST 2017
Document another bug Document fixes for design flaws GetForestTreeFrame is more like a design flaw than a bug/glitch (though it's really just calling out humorously bad code)
--- a/docs/bugs_and_glitches.md
+++ b/docs/bugs_and_glitches.md
@@ -8,6 +8,7 @@
- [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)
@@ -51,7 +52,6 @@
- [`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
@@ -182,6 +182,13 @@
```
+## Confusion damage is affected by type-boosting items and Explosion/Self-Destruct doubling
+
+([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
([Video](https://www.youtube.com/watch?v=OGwKPRJLaaI))
@@ -1412,46 +1419,3 @@
```
**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
-```
--- a/docs/design_flaws.md
+++ b/docs/design_flaws.md
@@ -10,6 +10,7 @@
- [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)
+- [`GetForestTreeFrame` works, but it's still bad](#getforesttreeframe-works-but-its-still-bad)
## Pic banks are offset by `PICS_FIX`
@@ -71,7 +72,9 @@
db BANK(Pics_1) + 23
```
+**Fix:** Use `dba` instead of `dba_pic`, and don't call `FixPicBank` to modify `a`.
+
## `PokemonPicPointers` and `UnownPicPointers` are assumed to start at the same address
In [gfx/pics.asm](/gfx/pics.asm):
@@ -90,6 +93,19 @@
INCLUDE "data/pokemon/unown_pic_pointers.asm"
```
+In [pokecrystal.link](/pokecrystal.link):
+
+```
+ROMX $48
+ org $4000
+ "Pic Pointers"
+ "Pics 1"
+ROMX $49
+ org $4000
+ "Unown Pic Pointers"
+ "Pics 2"
+```
+
Two routines in [gfx/load_pics.asm](/gfx/load_pics.asm) make this assumption; `GetFrontpicPointer`:
```asm
@@ -130,7 +146,51 @@
call AddNTimes
```
+**Fix:**
+Don't enforce `org $4000` in pokecrystal.link.
+
+Modify `GetFrontpicPointer`:
+
+```asm
+ ld a, [CurPartySpecies]
+ cp UNOWN
+ jr z, .unown
+ ld a, [CurPartySpecies]
+ ld d, BANK(PokemonPicPointers)
+ ld hl, PokemonPicPointers
+ jr .ok
+
+.unown
+ ld a, [UnownLetter]
+ ld d, BANK(UnownPicPointers)
+ ld hl, UnownPicPointers
+
+.ok
+ dec a
+ ld bc, 6
+ call AddNTimes
+```
+
+And `GetMonBackpic`:
+
+```asm
+ GLOBAL PokemonPicPointers, UnownPicPointers
+ ld a, b
+ ld hl, PokemonPicPointers
+ ld d, BANK(PokemonPicPointers)
+ cp UNOWN
+ jr nz, .ok
+ ld a, c
+ ld hl, UnownPicPointers
+ 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):
@@ -191,7 +251,32 @@
call Request1bpp
```
+**Fix:**
+Store footprints contiguously:
+
+```asm
+INCBIN "gfx/footprints/bulbasaur.1bpp"
+INCBIN "gfx/footprints/ivysaur.1bpp"
+INCBIN "gfx/footprints/venusaur.1bpp"
+INCBIN "gfx/footprints/charmander.1bpp"
+INCBIN "gfx/footprints/charmeleon.1bpp"
+INCBIN "gfx/footprints/charizard.1bpp"
+INCBIN "gfx/footprints/squirtle.1bpp"
+INCBIN "gfx/footprints/wartortle.1bpp"
+```
+
+Modify `Pokedex_LoadAnyFootprint`:
+
+```asm
+ ld e, l
+ ld d, h
+ ld hl, VTiles2 tile $62
+ lb bc, BANK(Footprints), 4
+ 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.)
@@ -297,7 +382,9 @@
db BANK(PokedexEntries4)
```
+**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
[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.
@@ -354,4 +441,71 @@
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
+```
+
+
+## `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
```