Code cleanup - pret/pokecrystal GitHub Wiki

These are all general issues with the pokecrystal code. Keep an eye out for opportunities to refactor them.

Use the style guide

Reformat code to adhere to STYLE.md. (Converting labels to .snake_case is one that stands out.)

Take advantage of up-to-date rgbds features

As of 0.3.7, we have these features that are little-used:

  • HIGH() and LOW() work for constants
  • BANK(@) gets the current bank; BANK("Section Name") is also valid (some labels can thus be removed)
  • \ continues lines, like in C or Python, which could help clean up long macros (trainer, npctrade) and implement new ones (like for data/trainers/attributes.asm or data/pokemon/base_stats/*.asm)
  • assert verifies implicit assumptions that are necessary for the code to work

Meaningless temporary labels

Look for UnknownText_*, UnknownScript_*, Unknown_*, Function_* and MovementData_*.

Hard-coded "magic numbers"

Most of the time, raw numbers should not exist because there are more meaningful replacements. Examples:

  • BANK(Label): Mostly finished for WRAM labels, but still a problem with mobile code.
  • SomeLabelEnd - SomeLabel: Some structs and sub-structs still have hard-coded sizes like this.
  • (SomeGFXLabel.End - SomeGFXLabel) / LEN_2BPP_TILE: pokered and pokegold-spaceworld already do this.
  • Bit flags: Use SOME_BIT_F constants instead of hard-coding bits 0-7 for the bit/res/set instructions. Find opportunities with:
grep -rE --include='*.asm' --exclude-dir=mobile '^\s(bit|res|set) [0-7],'
  • Bit masks: Use 1 << SOME_BIT_F masks and the maskbits macro.
  • Jumptable indexes: Look for raw values being loaded into [wJumptableIndex].

Text label styles in data/text/

Currently a mixture of Text_*, *Text, and BattleText_*. Likewise for the engine/ labels that just text_far to one of them.

The convention that should be used is as follows:

  • Use suffixes, i.e. not Text_Something, but SomethingText. Same for Movement.
  • Labels that are referenced through text_far should be prefixed with an underscore, e.g. SomethingText: text_far _SomethingText.

WRAM label style

  • wMapObjects and friends to be re-formatted to wObjectEvents
  • wObjectStructs and friends to be re-formatted to wMapObjects
  • Removal of struct macros which include only a partial label definition (e.g. wNorthMapConnection:: map_connection_struct wNorth)

Project structure

  • Split big files into meaningful parts
  • Give files meaningful names
  • Remove redundancy in names such as thing/thing_subthing.asm → thing/subthing.asm
  • Move files into their proper directories:
    • Palettes in gfx/
    • Pointer tables in data/
  • Group related things in meaningful directories.

Future of SECTIONs

Namely determining the purpose of them and when they should be defined.

Unused/Unreferenced/Stubs

Currently, a mixture of UnusedFunction, Unreferenced_Function and StubbedFunction is being used. This should be changed, where appropriate names can be given, to comments right after the label in question, meaning the following:

  • unused for code or data that is referenced but never used, through branches that are never taken during normal gameplay
  • unreferenced for code or data that's never referenced, thus can never be used, even with glitches, short of arbitrary code execution
  • stub for functions that were supposed to do something but don't, because their functionality was removed

Refer to docs/bugs_and_glitches.md instead of explaining a bug inline

A lot of bugs aren't always objective in nature, and others require an in-depth explanation of what's going wrong in order to get the point across. In order to reduce clutter in having huge explanatory comments, it's better to refer to the bugs_and_glitches document using a ; BUG: marker, like so:

.Cry:
; BUG: Playing Entei's Pokédex cry can distort Raikou's and Suicune's (see docs/bugs_and_glitches.md)
	call Pokedex_GetSelectedMon