1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
|
These are all general issues with the pokecrystal code. Keep an eye out for opportunities to refactor them.
### Use the [style guide](../tree/master/STYLE.md)
Reformat code to adhere to [STYLE.md](../tree/master/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:
```bash
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/](../tree/master/data/text/)
Currently a mixture of `Text_*`, `*Text`, and `BattleText_*`. Likewise for the [engine/](../tree/master/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 `SECTION`s
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
|
