From fe92e4d848eacba957fac472322ae16698451987 Mon Sep 17 00:00:00 2001 From: Remy Oukaour Date: Thu, 28 Dec 2017 11:28:23 -0500 Subject: Document design flaws --- docs/design_flaws.md | 355 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 355 insertions(+) create mode 100644 docs/design_flaws.md (limited to 'docs') diff --git a/docs/design_flaws.md b/docs/design_flaws.md new file mode 100644 index 000000000..1ddc92094 --- /dev/null +++ b/docs/design_flaws.md @@ -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 +``` -- cgit v1.2.3 From 0ceee9696a2a9045697d01c89761b6cee9dea861 Mon Sep 17 00:00:00 2001 From: Remy Oukaour Date: Thu, 28 Dec 2017 11:50:02 -0500 Subject: Explain what bugs, glitches, and flaws are --- docs/bugs_and_glitches.md | 2 ++ docs/design_flaws.md | 2 ++ 2 files changed, 4 insertions(+) (limited to 'docs') diff --git a/docs/bugs_and_glitches.md b/docs/bugs_and_glitches.md index 69f902eb0..2382a8e29 100644 --- a/docs/bugs_and_glitches.md +++ b/docs/bugs_and_glitches.md @@ -1,5 +1,7 @@ # 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 diff --git a/docs/design_flaws.md b/docs/design_flaws.md index 1ddc92094..bcd7ea516 100644 --- a/docs/design_flaws.md +++ b/docs/design_flaws.md @@ -1,5 +1,7 @@ # Design Flaws +These are parts of the code that do not work *incorrectly*, like [bugs and glitches](/docs/bugs_and_glitches.md), but that clearly exist just to work around a problem. In other words, with a slightly different design, the code would not need to exist at all. Design flaws may be exceptions to a usual rule, such as "tables of pointers in different banks use `dba`" ([one exception](#pic-banks-are-offset-by-pics_fix), [and another](#pokédex-entry-banks-are-derived-from-their-species-ids)) or "graphics used as a unit as stored and loaded contiguously" ([a notable exception](#footprints-are-split-into-top-and-bottom-halves)). + ## Contents -- cgit v1.2.3 From 805474b0db4a87d230852c7ce6803f5151a321a6 Mon Sep 17 00:00:00 2001 From: Remy Oukaour Date: Thu, 28 Dec 2017 12:14:47 -0500 Subject: =?UTF-8?q?as=20=E2=86=92=20are?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/design_flaws.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'docs') diff --git a/docs/design_flaws.md b/docs/design_flaws.md index bcd7ea516..848b4c584 100644 --- a/docs/design_flaws.md +++ b/docs/design_flaws.md @@ -1,6 +1,6 @@ # Design Flaws -These are parts of the code that do not work *incorrectly*, like [bugs and glitches](/docs/bugs_and_glitches.md), but that clearly exist just to work around a problem. In other words, with a slightly different design, the code would not need to exist at all. Design flaws may be exceptions to a usual rule, such as "tables of pointers in different banks use `dba`" ([one exception](#pic-banks-are-offset-by-pics_fix), [and another](#pokédex-entry-banks-are-derived-from-their-species-ids)) or "graphics used as a unit as stored and loaded contiguously" ([a notable exception](#footprints-are-split-into-top-and-bottom-halves)). +These are parts of the code that do not work *incorrectly*, like [bugs and glitches](/docs/bugs_and_glitches.md), but that clearly exist just to work around a problem. In other words, with a slightly different design, the code would not need to exist at all. Design flaws may be exceptions to a usual rule, such as "tables of pointers in different banks use `dba`" ([one exception](#pic-banks-are-offset-by-pics_fix), [and another](#pokédex-entry-banks-are-derived-from-their-species-ids)) or "graphics used as a unit are stored and loaded contiguously" ([a notable exception](#footprints-are-split-into-top-and-bottom-halves)). ## Contents -- cgit v1.2.3 From 7167895612387ce676c4e3606cac07101719ee2f Mon Sep 17 00:00:00 2001 From: Remy Oukaour Date: Thu, 28 Dec 2017 12:49:32 -0500 Subject: 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) --- docs/bugs_and_glitches.md | 52 +++------------- docs/design_flaws.md | 154 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 44 deletions(-) (limited to 'docs') diff --git a/docs/bugs_and_glitches.md b/docs/bugs_and_glitches.md index 2382a8e29..d5761b5a3 100644 --- a/docs/bugs_and_glitches.md +++ b/docs/bugs_and_glitches.md @@ -8,6 +8,7 @@ These are known bugs and glitches in the original Pokémon Crystal game: code th - [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 @@ These are known bugs and glitches in the original Pokémon Crystal game: code th - [`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 @@ BattleCommand_BellyDrum: ; 37c1a ``` +## 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 @@ 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 -``` diff --git a/docs/design_flaws.md b/docs/design_flaws.md index 848b4c584..6009d0265 100644 --- a/docs/design_flaws.md +++ b/docs/design_flaws.md @@ -10,6 +10,7 @@ These are parts of the code that do not work *incorrectly*, like [bugs and glitc - [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,6 +72,8 @@ GLOBAL PICS_FIX 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 @@ -90,6 +93,19 @@ SECTION "Unown Pic Pointers", ROMX 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,6 +146,50 @@ And `GetMonBackpic`: 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 @@ -191,6 +251,31 @@ INCBIN "gfx/footprints/wartortle.1bpp", footprint_bottom 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 @@ -297,6 +382,8 @@ PokedexShow_GetDexEntryBank: 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 @@ -355,3 +442,70 @@ GetNumberedTMHM: ; d417 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 +``` -- cgit v1.2.3 From b5ffbfe2af79c7176688db60a7a0ae099733f8b5 Mon Sep 17 00:00:00 2001 From: Remy Oukaour Date: Thu, 28 Dec 2017 12:58:18 -0500 Subject: Specify which bugs are needed for backwards compatibility --- docs/bugs_and_glitches.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'docs') diff --git a/docs/bugs_and_glitches.md b/docs/bugs_and_glitches.md index d5761b5a3..bf7c1d71e 100644 --- a/docs/bugs_and_glitches.md +++ b/docs/bugs_and_glitches.md @@ -56,6 +56,8 @@ These are known bugs and glitches in the original Pokémon Crystal game: code th ## 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): @@ -90,6 +92,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): @@ -146,6 +150,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): @@ -184,6 +190,8 @@ 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. @@ -191,6 +199,8 @@ BattleCommand_BellyDrum: ; 37c1a ## 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. @@ -227,6 +237,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. @@ -234,6 +246,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): @@ -259,6 +273,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. @@ -266,6 +282,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): @@ -284,6 +302,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): @@ -319,6 +339,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. @@ -670,6 +692,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 -- cgit v1.2.3 From 97dd40284b885da2dd27099d97d03c221a1cb879 Mon Sep 17 00:00:00 2001 From: Remy Oukaour Date: Thu, 28 Dec 2017 13:17:43 -0500 Subject: Document another Magikarp length bug. (to do: correct formula values) --- docs/bugs_and_glitches.md | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) (limited to 'docs') diff --git a/docs/bugs_and_glitches.md b/docs/bugs_and_glitches.md index bf7c1d71e..d246d3916 100644 --- a/docs/bugs_and_glitches.md +++ b/docs/bugs_and_glitches.md @@ -30,6 +30,7 @@ These are known bugs and glitches in the original Pokémon Crystal game: code th - [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 lengths in Lake of Rage have a unit conversion error](#magikarp-lengths-in-lake-of-rage-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) @@ -784,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 lengths in Lake of Rage 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 + +; We're clear if the length is < 1536 + ld a, [wMagikarpLength] + cp HIGH(1536) + jr nz, .CheckMagikarpArea + +; 5% chance of skipping both size checks + call Random + cp 5 percent + jr c, .CheckMagikarpArea +; Try again if > 1614 + ld a, [wMagikarpLength + 1] + cp LOW(1616) + jr nc, .GenerateDVs + +; 20% chance of skipping this check + call Random + cp 20 percent - 1 + jr c, .CheckMagikarpArea +; Try again if > 1598 + ld a, [wMagikarpLength + 1] + cp LOW(1600) + jr nc, .GenerateDVs +``` + +*To do:* Fix this bug. + + ## Magikarp lengths can be miscalculated This is a bug with `CalcMagikarpLength.BCLessThanDE` in [engine/events/magikarp.asm](/engine/events/magikarp.asm): -- cgit v1.2.3 From fe605b2fa756b7ea18d3f92b95523fa0eb7a2a9e Mon Sep 17 00:00:00 2001 From: Remy Oukaour Date: Thu, 28 Dec 2017 14:08:29 -0500 Subject: Document the Magikarp bug fix from PR #443 --- docs/bugs_and_glitches.md | 18 +++++++++--------- docs/design_flaws.md | 5 +++-- 2 files changed, 12 insertions(+), 11 deletions(-) (limited to 'docs') diff --git a/docs/bugs_and_glitches.md b/docs/bugs_and_glitches.md index d246d3916..52d788987 100644 --- a/docs/bugs_and_glitches.md +++ b/docs/bugs_and_glitches.md @@ -30,7 +30,7 @@ These are known bugs and glitches in the original Pokémon Crystal game: code th - [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 lengths in Lake of Rage have a unit conversion error](#magikarp-lengths-in-lake-of-rage-have-a-unit-conversion-error) +- [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) @@ -785,7 +785,7 @@ This is a bug with `LoadEnemyMon.CheckMagikarpArea` in [engine/battle/core.asm]( **Fix:** Change both `jr z, .Happiness` to `jr nz, .Happiness`. -## Magikarp lengths in Lake of Rage have a unit conversion error +## Magikarp length limits have a unit conversion error This is a bug with `LoadEnemyMon.CheckMagikarpArea` in [engine/battle/core.asm](/engine/battle/core.asm): @@ -795,31 +795,31 @@ This is a bug with `LoadEnemyMon.CheckMagikarpArea` in [engine/battle/core.asm]( ld bc, PlayerID callfar CalcMagikarpLength -; We're clear if the length is < 1536 +; No reason to keep going if length > 1536 (i.e. if length / 256 != 6) ld a, [wMagikarpLength] - cp HIGH(1536) + 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 > 1614 +; Try again if length > 1615 ld a, [wMagikarpLength + 1] - cp LOW(1616) + 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 > 1598 +; Try again if length > 1599 ld a, [wMagikarpLength + 1] - cp LOW(1600) + cp LOW(1600) ; this compares to 6'64'', should be cp 2 jr nc, .GenerateDVs ``` -*To do:* Fix this bug. +**Fix:** Change the three `cp` instructions to use their commented values. ## Magikarp lengths can be miscalculated diff --git a/docs/design_flaws.md b/docs/design_flaws.md index 6009d0265..b82cfb394 100644 --- a/docs/design_flaws.md +++ b/docs/design_flaws.md @@ -157,14 +157,14 @@ Modify `GetFrontpicPointer`: cp UNOWN jr z, .unown ld a, [CurPartySpecies] - ld d, BANK(PokemonPicPointers) ld hl, PokemonPicPointers + ld d, BANK(PokemonPicPointers) jr .ok .unown ld a, [UnownLetter] - ld d, BANK(UnownPicPointers) ld hl, UnownPicPointers + ld d, BANK(UnownPicPointers) .ok dec a @@ -264,6 +264,7 @@ INCBIN "gfx/footprints/charmeleon.1bpp" INCBIN "gfx/footprints/charizard.1bpp" INCBIN "gfx/footprints/squirtle.1bpp" INCBIN "gfx/footprints/wartortle.1bpp" +... ``` Modify `Pokedex_LoadAnyFootprint`: -- cgit v1.2.3 From ce795692d91c36d1db099a26bf7cdf794db400ba Mon Sep 17 00:00:00 2001 From: Remy Oukaour Date: Thu, 28 Dec 2017 14:10:46 -0500 Subject: Correct comment --- docs/design_flaws.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'docs') diff --git a/docs/design_flaws.md b/docs/design_flaws.md index b82cfb394..21a222743 100644 --- a/docs/design_flaws.md +++ b/docs/design_flaws.md @@ -280,7 +280,7 @@ Modify `Pokedex_LoadAnyFootprint`: ## 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.) +`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.) Three separate routines do the same derivation; `GetDexEntryPointer` in [engine/pokedex_2.asm](/engine/pokedex_2.asm): -- cgit v1.2.3 From 84e22eb578bdd2dbe4da320f4c4e17cc3688a8a6 Mon Sep 17 00:00:00 2001 From: Remy Oukaour Date: Thu, 28 Dec 2017 14:31:25 -0500 Subject: Consistent (x, y) coordinate formatting in map scripts --- docs/map_scripts.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'docs') diff --git a/docs/map_scripts.md b/docs/map_scripts.md index fa8fa308b..4206c7a86 100644 --- a/docs/map_scripts.md +++ b/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: -- cgit v1.2.3