diff options
Diffstat (limited to 'docs/design_flaws.md')
| -rw-r--r-- | docs/design_flaws.md | 62 |
1 files changed, 31 insertions, 31 deletions
diff --git a/docs/design_flaws.md b/docs/design_flaws.md index 2227d2c3f..8669bc7a6 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 are 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](https://github.com/pret/pokecrystal/blob/master/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 @@ -17,7 +17,7 @@ These are parts of the code that do not work *incorrectly*, like [bugs and glitc ## 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`: +[data/pokemon/pic_pointers.asm](https://github.com/pret/pokecrystal/blob/master/data/pokemon/pic_pointers.asm), [data/pokemon/unown_pic_pointers.asm](https://github.com/pret/pokecrystal/blob/master/data/pokemon/unown_pic_pointers.asm), and [data/trainers/pic_pointers.asm](https://github.com/pret/pokecrystal/blob/master/data/trainers/pic_pointers.asm) all have to use `dba_pic` instead of `dba`. This is a macro in [macros/data.asm](https://github.com/pret/pokecrystal/blob/master/macros/data.asm) that offsets banks by `PICS_FIX`: ```asm dba_pic: MACRO ; dbw bank, address @@ -26,7 +26,7 @@ dba_pic: MACRO ; dbw bank, address ENDM ``` -The offset is translated into a correct bank by `FixPicBank` in [engine/gfx/load_pics.asm](/engine/gfx/load_pics.asm): +The offset is translated into a correct bank by `FixPicBank` in [engine/gfx/load_pics.asm](https://github.com/pret/pokecrystal/blob/master/engine/gfx/load_pics.asm): ```asm FixPicBank: @@ -74,14 +74,14 @@ GLOBAL PICS_FIX db BANK("Pics 24") ; BANK("Pics 1") + 23 ``` -**Fix:** Delete `FixPicBank` and remove all four calls to `FixPicBank` in [engine/gfx/load_pics.asm](/engine/gfx/load_pics.asm). Then use `dba` instead of `dba_pic` everywhere. +**Fix:** Delete `FixPicBank` and remove all four calls to `FixPicBank` in [engine/gfx/load_pics.asm](https://github.com/pret/pokecrystal/blob/master/engine/gfx/load_pics.asm). Then use `dba` instead of `dba_pic` everywhere. ## `PokemonPicPointers` and `UnownPicPointers` are assumed to start at the same address -`GetFrontpicPointer` and `GetMonBackpic` in [engine/gfx/load_pics.asm](/engine/gfx/load_pics.asm) make this assumption, which has to be accounted for in the data files. +`GetFrontpicPointer` and `GetMonBackpic` in [engine/gfx/load_pics.asm](https://github.com/pret/pokecrystal/blob/master/engine/gfx/load_pics.asm) make this assumption, which has to be accounted for in the data files. -In [gfx/pics.asm](/gfx/pics.asm): +In [gfx/pics.asm](https://github.com/pret/pokecrystal/blob/master/gfx/pics.asm): ```asm ; PokemonPicPointers and UnownPicPointers are assumed to start at the same @@ -98,7 +98,7 @@ SECTION "Unown Pic Pointers", ROMX INCLUDE "data/pokemon/unown_pic_pointers.asm" ``` -In [pokecrystal.link](/pokecrystal.link): +In [pokecrystal.link](https://github.com/pret/pokecrystal/blob/master/pokecrystal.link): ``` ROMX $48 @@ -113,7 +113,7 @@ ROMX $49 **Fix:** -Don't enforce `org $4000` in [pokecrystal.link](/pokecrystal.link). +Don't enforce `org $4000` in [pokecrystal.link](https://github.com/pret/pokecrystal/blob/master/pokecrystal.link). Edit `GetFrontpicPointer`: @@ -160,7 +160,7 @@ And `GetMonBackpic`: ## Footprints are split into top and bottom halves -In [gfx/footprints.asm](/gfx/footprints.asm): +In [gfx/footprints.asm](https://github.com/pret/pokecrystal/blob/master/gfx/footprints.asm): ```asm ; Footprints are 2x2 tiles each, but are stored as a 16x64-tile image @@ -195,7 +195,7 @@ INCBIN "gfx/footprints/wartortle.1bpp", footprint_bottom ... ``` -`Pokedex_LoadAnyFootprint` in [engine/pokedex/pokedex.asm](/engine/pokedex/pokedex.asm) has to load the halves separately. +`Pokedex_LoadAnyFootprint` in [engine/pokedex/pokedex.asm](https://github.com/pret/pokecrystal/blob/master/engine/pokedex/pokedex.asm) has to load the halves separately. **Fix:** @@ -240,13 +240,13 @@ Edit `Pokedex_LoadAnyFootprint`: ## Music IDs $64 and $80 or above have special behavior -If a map's music ID in [data/maps/maps.asm](/master/data/maps/maps.asm) is $64 (the value of `MUSIC_MAHOGANY_MART` or `MUSIC_SUICUNE_BATTLE`) it will play either `MUSIC_ROCKET_HIDEOUT` or `MUSIC_CHERRYGROVE_CITY`. Moreover, if a map's music ID is $80 or above (the value of `RADIO_TOWER_MUSIC`) it might play `MUSIC_ROCKET_OVERTURE` or something else. This is caused by `GetMapMusic` in [home/map.asm](/master/home/map.asm). +If a map's music ID in [data/maps/maps.asm](https://github.com/pret/pokecrystal/blob/master/master/data/maps/maps.asm) is $64 (the value of `MUSIC_MAHOGANY_MART` or `MUSIC_SUICUNE_BATTLE`) it will play either `MUSIC_ROCKET_HIDEOUT` or `MUSIC_CHERRYGROVE_CITY`. Moreover, if a map's music ID is $80 or above (the value of `RADIO_TOWER_MUSIC`) it might play `MUSIC_ROCKET_OVERTURE` or something else. This is caused by `GetMapMusic` in [home/map.asm](https://github.com/pret/pokecrystal/blob/master/master/home/map.asm). **Fix:** -Replace `RADIO_TOWER_MUSIC | MUSIC_GOLDENROD_CITY` with `MUSIC_RADIO_TOWER` in [data/maps/maps.asm](/master/data/maps/maps.asm). +Replace `RADIO_TOWER_MUSIC | MUSIC_GOLDENROD_CITY` with `MUSIC_RADIO_TOWER` in [data/maps/maps.asm](https://github.com/pret/pokecrystal/blob/master/master/data/maps/maps.asm). -Redefine the special music constants in [constants/music_constants.asm](/master/constants/music_constants.asm): +Redefine the special music constants in [constants/music_constants.asm](https://github.com/pret/pokecrystal/blob/master/master/constants/music_constants.asm): ```diff -; GetMapMusic picks music for this value (see home/map.asm) @@ -318,7 +318,7 @@ Edit `GetMapMusic`: ## `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. +[constants/item_constants.asm](https://github.com/pret/pokecrystal/blob/master/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 @@ -335,7 +335,7 @@ Edit `GetMapMusic`: NUM_TMS = const_value - TM01 - 2 ; discount ITEM_C3 and ITEM_DC ``` -`GetTMHMNumber` and `GetNumberedTMHM` in [engine/items/items.asm](/engine/items/items.asm) have to compensate for this. +`GetTMHMNumber` and `GetNumberedTMHM` in [engine/items/items.asm](https://github.com/pret/pokecrystal/blob/master/engine/items/items.asm) have to compensate for this. > There was originally a good reason for these two gaps! > @@ -357,7 +357,7 @@ NUM_TMS = const_value - TM01 - 2 ; discount ITEM_C3 and ITEM_DC > - Wild Dragonair's catch rate became 27 = $1B for `PROTEIN` > - Wild Dragonite's catch rate became 9 = $09 for `ANTIDOTE` > -> Most catch rates were left as gaps in the item list, and transformed into held items via the `TimeCapsule_CatchRateItems` table in [data/items/catch_rate_items.asm](/data/items/catch_rate_items.asm). For example, the 52 Pokémon with catch rate 45 would hold the gap `ITEM_2D`, except that gets transformed into `BITTER_BERRY`. +> Most catch rates were left as gaps in the item list, and transformed into held items via the `TimeCapsule_CatchRateItems` table in [data/items/catch_rate_items.asm](https://github.com/pret/pokecrystal/blob/master/data/items/catch_rate_items.asm). For example, the 52 Pokémon with catch rate 45 would hold the gap `ITEM_2D`, except that gets transformed into `BITTER_BERRY`. > > But a few Pokémon end up with weird items. Abra has a catch rate of 200, or $C8; and Krabby, Horsea, Goldeen, and Staryu have a catch rate of 225, or $E1. Those indexes correspond to the items `TM_PSYCH_UP` and `TM_ICE_PUNCH`, which seem like random choices—because they are. > @@ -369,7 +369,7 @@ NUM_TMS = const_value - TM01 - 2 ; discount ITEM_C3 and ITEM_DC Move `ITEM_C3` and `ITEM_DC` above all the TMs in every table of item data. -Edit [engine/items/items.asm](/engine/items/items.asm): +Edit [engine/items/items.asm](https://github.com/pret/pokecrystal/blob/master/engine/items/items.asm): ```diff GetTMHMNumber:: @@ -411,9 +411,9 @@ Edit [engine/items/items.asm](/engine/items/items.asm): ## 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.) +`PokedexDataPointerTable` in [data/pokemon/dex_entry_pointers.asm](https://github.com/pret/pokecrystal/blob/master/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/pokedex_2.asm](/engine/pokedex/pokedex_2.asm): +Three separate routines do the same derivation; `GetDexEntryPointer` in [engine/pokedex/pokedex_2.asm](https://github.com/pret/pokecrystal/blob/master/engine/pokedex/pokedex_2.asm): ```asm GetDexEntryPointer: @@ -449,7 +449,7 @@ GetDexEntryPointer: db BANK("Pokedex Entries 193-251") ``` -`GetPokedexEntryBank` in [engine/items/item_effects.asm](/engine/items/item_effects.asm): +`GetPokedexEntryBank` in [engine/items/item_effects.asm](https://github.com/pret/pokecrystal/blob/master/engine/items/item_effects.asm): ```asm GetPokedexEntryBank: @@ -475,7 +475,7 @@ GetPokedexEntryBank: db BANK("Pokedex Entries 193-251") ``` -And `PokedexShow_GetDexEntryBank` in [engine/pokegear/radio.asm](/engine/pokegear/radio.asm): +And `PokedexShow_GetDexEntryBank` in [engine/pokegear/radio.asm](https://github.com/pret/pokecrystal/blob/master/engine/pokegear/radio.asm): ```asm PokedexShow_GetDexEntryBank: @@ -502,12 +502,12 @@ PokedexShow_GetDexEntryBank: db BANK("Pokedex Entries 193-251") ``` -**Fix:** Use `dba` instead of `dw` in `PokedexDataPointerTable`. Then edit [home.asm](/home.asm) to contain a single copy of the `PokedexDataPointerTable` lookup code, updated to work with 3-byte `dba` entries and get the bank from the first entry byte. Delete the three separate lookup routines and use the new one (placed in [home.asm](/home.asm) so it can be called from any bank.) +**Fix:** Use `dba` instead of `dw` in `PokedexDataPointerTable`. Then edit [home.asm](https://github.com/pret/pokecrystal/blob/master/home.asm) to contain a single copy of the `PokedexDataPointerTable` lookup code, updated to work with 3-byte `dba` entries and get the bank from the first entry byte. Delete the three separate lookup routines and use the new one (placed in [home.asm](https://github.com/pret/pokecrystal/blob/master/home.asm) so it can be called from any bank.) ## Identical sine wave code and data is repeated five times -`_Sine` in [engine/math/sine.asm](/engine/math/sine.asm): +`_Sine` in [engine/math/sine.asm](https://github.com/pret/pokecrystal/blob/master/engine/math/sine.asm): ```asm _Sine:: @@ -516,7 +516,7 @@ _Sine:: calc_sine_wave ``` -`Sprites_Cosine` and `Sprites_Sine` in [engine/gfx/sprites.asm](/engine/gfx/sprites.asm): +`Sprites_Cosine` and `Sprites_Sine` in [engine/gfx/sprites.asm](https://github.com/pret/pokecrystal/blob/master/engine/gfx/sprites.asm): ```asm Sprites_Cosine: @@ -528,7 +528,7 @@ Sprites_Sine: calc_sine_wave ``` -`BattleAnim_Cosine` and `BattleAnim_Sine` in [engine/battle_anims/functions.asm](/engine/battle_anims/functions.asm): +`BattleAnim_Cosine` and `BattleAnim_Sine` in [engine/battle_anims/functions.asm](https://github.com/pret/pokecrystal/blob/master/engine/battle_anims/functions.asm): ```asm BattleAnim_Cosine: @@ -545,14 +545,14 @@ BattleAnimSineWave: sine_table 32 ``` -`StartTrainerBattle_DrawSineWave` in [engine/battle/battle_transition.asm](/engine/battle/battle_transition.asm): +`StartTrainerBattle_DrawSineWave` in [engine/battle/battle_transition.asm](https://github.com/pret/pokecrystal/blob/master/engine/battle/battle_transition.asm): ```asm StartTrainerBattle_DrawSineWave: calc_sine_wave ``` -And `CelebiEvent_Cosine` in [engine/events/celebi.asm](/engine/events/celebi.asm): +And `CelebiEvent_Cosine` in [engine/events/celebi.asm](https://github.com/pret/pokecrystal/blob/master/engine/events/celebi.asm): ```asm CelebiEvent_Cosine: @@ -561,7 +561,7 @@ CelebiEvent_Cosine: calc_sine_wave ``` -They all rely on `calc_sine_wave` in [macros/code.asm](/macros/code.asm): +They all rely on `calc_sine_wave` in [macros/code.asm](https://github.com/pret/pokecrystal/blob/master/macros/code.asm): ```asm calc_sine_wave: MACRO @@ -612,7 +612,7 @@ endc ENDM ``` -And on `sine_table` in [macros/data.asm](/macros/data.asm): +And on `sine_table` in [macros/data.asm](https://github.com/pret/pokecrystal/blob/master/macros/data.asm): ```asm sine_table: MACRO @@ -625,12 +625,12 @@ endr ENDM ``` -**Fix:** Edit [home/sine.asm](/home/sine.asm) to contain a single copy of the (co)sine code in bank 0, and call it from those five sites. +**Fix:** Edit [home/sine.asm](https://github.com/pret/pokecrystal/blob/master/home/sine.asm) to contain a single copy of the (co)sine code in bank 0, and call it from those five sites. ## `GetForestTreeFrame` works, but it's still bad -The routine `GetForestTreeFrame` in [engine/tilesets/tileset_anims.asm](/engine/tilesets/tileset_anims.asm) is hilariously inefficient. +The routine `GetForestTreeFrame` in [engine/tilesets/tileset_anims.asm](https://github.com/pret/pokecrystal/blob/master/engine/tilesets/tileset_anims.asm) is hilariously inefficient. **Fix:** |