e.g. (from pokeplatinum)

typedef struct SideConditions {
    u32 reflectBattlerId        :2,
        reflectTurns            :3,
        lightScreenBattlerId    :2,
        lightScreenTurns        :3,
        mistBattlerId           :2,
        mistTurns               :3,
        safeguardBattlerId      :2,
        safeguardTurns          :3,
        followMeActive          :1,
        followMeBattlerId       :2,
        knockedOffItemBattlers  :6,
        padding00_1D            :3;

    u32 spikesLayers        :2,
        toxicSpikesLayers   :2,
        padding04_04        :28;
} SideConditions;

vs.

typedef struct SideConditions {
    u32 reflectBattlerId:2;
    u32 reflectTurns:3;
    u32 lightScreenBattlerId:2;
    u32 lightScreenTurns:3;
    u32 mistBattlerId:2;
    u32 mistTurns:3;
    u32 safeguardBattlerId:2;
    u32 safeguardTurns:3;
    u32 followMeActive:1;
    u32 followMeBattlerId:2;
    u32 knockedOffItemBattlers:6;
    u32 padding00_1D:3;

    u32 spikesLayers:2;
    u32 toxicSpikesLayers:2;
    u32 padding04_04:28;
} SideConditions;

TODO: write up a grep

Style guide for reference: Link

Consensus deciding

• #212
• #213
• #214
• #217
• #218
• #219
• #221
• #222
• #227
• #237
• #243
• #262
• #266

Implementation tasks

• #215
• #216
• #228
• #229
• #235
• #261

There is no consensus whether enums are enums or typedefs.

Case 1: enums are enums, not typedefs

Do not typedef enums

Good:

enum RTC_TimeOfDay {
    RTC_TIMEOFDAY_MORN = 0,
    RTC_TIMEOFDAY_DAY,
    RTC_TIMEOFDAY_EVE,
    RTC_TIMEOFDAY_NITE,
    RTC_TIMEOFDAY_LATE,
};

Bad:

• Using a typedef

typedef enum RTC_TimeOfDay {
    RTC_TIMEOFDAY_MORN = 0,
    RTC_TIMEOFDAY_DAY,
    RTC_TIMEOFDAY_EVE,
    RTC_TIMEOFDAY_NITE,
    RTC_TIMEOFDAY_LATE,
} RTC_TimeOfDay;

There is no consensus of the casing of enums (when enums are enums and not typedefs).

Option 1: PascalCase

enum RTC_TimeOfDay {
    RTC_TIMEOFDAY_MORN = 0,
    RTC_TIMEOFDAY_DAY,
    RTC_TIMEOFDAY_EVE,
    RTC_TIMEOFDAY_NITE,
    RTC_TIMEOFDAY_LATE,
};

Option 2: alllowercase_t (similar to primitive non-unit typedefs)
Note: Primitive unit typedefs are u8, u16, u32, s8, s16, s32 etc....

enum rtc_timeofday_t {
    RTC_TIMEOFDAY_MORN = 0,
    RTC_TIMEOFDAY_DAY,
    RTC_TIMEOFDAY_EVE,
    RTC_TIMEOFDAY_NITE,
    RTC_TIMEOFDAY_LATE,
};

Reference all enums with enum in code (this is forced anyway)

Good:

enum RTC_TimeOfDay GF_RTC_GetTimeOfDay(void) {
    RTCTime time;
    GF_RTC_CopyTime(&time);
    return GF_RTC_GetTimeOfDayByHour(time.hour);
}

enum rtc_timeofday_t GF_RTC_GetTimeOfDay(void) {
    RTCTime time;
    GF_RTC_CopyTime(&time);
    return GF_RTC_GetTimeOfDayByHour(time.hour);
}

Bad (doesn't compile anyway):

RTC_TimeOfDay GF_RTC_GetTimeOfDay(void) {
    RTCTime time;
    GF_RTC_CopyTime(&time);
    return GF_RTC_GetTimeOfDayByHour(time.hour);
}

rtc_timeofday_t GF_RTC_GetTimeOfDay(void) {
    RTCTime time;
    GF_RTC_CopyTime(&time);
    return GF_RTC_GetTimeOfDayByHour(time.hour);
}

Case 2: enums are typedefs

Enum casing should be alllowercase_t

Good:

typedef enum rtc_timeofday_t {
    RTC_TIMEOFDAY_MORN = 0,
    RTC_TIMEOFDAY_DAY,
    RTC_TIMEOFDAY_EVE,
    RTC_TIMEOFDAY_NITE,
    RTC_TIMEOFDAY_LATE,
} rtc_timeofday_t;

Bad:

typedef enum RTC_TimeOfDay {
    RTC_TIMEOFDAY_MORN = 0,
    RTC_TIMEOFDAY_DAY,
    RTC_TIMEOFDAY_EVE,
    RTC_TIMEOFDAY_NITE,
    RTC_TIMEOFDAY_LATE,
} RTC_TimeOfDay;

Follow points 1, 2, and 3 of issue #212 (covers typedef struct naming) for the rest of the info

the entire file is wrongly documented, the key struct should be called LocalFieldData instead of FLYPOINTS_SAVE, as it contains information on a lot more than flypoints (as per discussion in the discord)

as such all funcs in the file need to be reanmed, along with all substructs (and the constant to get from the save array)

Modern canonical names vs gen4 names, e.g.:

#define MOVE_VISE_GRIP
#define MOVE_FEINT_ATTACK
#define MOVE_SMOKESCREEN
#define MOVE_SMELLING_SALTS

vs.

#define MOVE_VICE_GRIP
#define MOVE_FAINT_ATTACK
#define MOVE_SMOKE_SCREEN
#define MOVE_SMELLING_SALT

Canonical list of modified moves.

There are still some style directions relating to structs which have no consensus. Those can be found at issue #212

1. All structs must have a corresponding typedef. The typedef must be defined with the struct.

See issue #212 point 5 for recursive/self-referential/"references each other" structs.

• Good:

typedef struct BattleCursor {
    void *unk0[5];
    SysTask *task;
} BattleCursor;

Opaque struct declaration (only okay if the corresponding file isn't decompiled yet)

typedef struct GameStats GameStats;

• Bad:

Missing corresponding typedef.

struct BattleCursor {
    void *unk0[5];
    SysTask *task;
};

Same as above but for opaque struct declaration.

struct GameStats;

Corresponding typedef is separate from struct definition.

struct BattleCursor {
    void *unk0[5];
    SysTask *task;
};
typedef struct BattleCursor BattleCursor;

struct must be defined as well as the typedef.

typedef struct {
    void *unk0[5];
    SysTask *task;
} BattleCursor;

2. Refer to the typedef in code, not the struct

• Good:

const HiddenItemData *table = sHiddenItemParam;

• Bad:

const struct HiddenItemData *table = sHiddenItemParam;

3. Struct names must match the typedef struct name

• Good:

typedef struct HeapParam {
    u32 size;         // maximum size of the heap
    OSArenaId arena;  // where to allocate the heap from
} HeapParam;

• Bad:

typedef struct HeapParam {
    u32 size;         // maximum size of the heap
    OSArenaId arena;  // where to allocate the heap from
} HeapParam_t; // _t suffix

4. Struct names and typedef struct names must be PascalCase

• Good:

typedef struct WildEncounter {
    int state;
    int effect;
    int bgm;
    int *winFlag;
    BattleSetup *setup;
} WildEncounter;

• Bad:

//  using ALL_CAPS instead of PascalCase
typedef struct WILD_ENCOUNTER {
    int state;
    int effect;
    int bgm;
    int *winFlag;
    BATTLE_SETUP *setup;
} WILD_ENCOUNTER;

there are a few funcs in src that use the sdk naming conventions when they are not part of the SDK. this causes confusion.
stuff like GX_EngineBToggleLayers
as side note this should probably have a different name
smth like GFGX_SubEngineTogglePlanes

Not actually a real function.

To reproduce:

• Create an empty directory (except maybe for .*ignore files and any file pattern matching the .*ignore spec)
• Run knarc in pack mode on that directory

Expectation:
knarc exits or warns with a useful message

Observation:
knarc throws SIGSEGV (11)

Useful grep:
grep -Pwr --include="*.c" --include="*.h" --exclude-dir="tools" --exclude-dir="sub" --exclude-dir="lib" --exclude-dir=".github" --exclude-dir="files" "(\w+[wW]ork|work)"

Rationale:

Kurausukun — 05/12/2023 5:46 PM
Remember that "work" is Japanese code lingo for "temporary/generic variable/memory"
Kurausukun — 05/12/2023 5:49 PM
"work" will not mean anything to anyone who doesn't know JP and I think all instances of it should be removed

lucky made me do this

all files that belong to overlays should be within their own folders, preferably those folders being inside an overlay folder

preferably the format should be

src/overlay/battle/{src,include}/file.c

this can be debated

Alternative is num, index, idx, probably others depending on context.

You need to be very careful when including header files in jinja templates, otherwise you could end up including msg.naix before it is built.

Alternatively, we can just ban including .naix files in headers. Some are less stringent than others, for example the encounter tables (for now).

We already have consensus on pointer-star alignment for variable declarations, e.g.:

Pokemon *mon;
void ZeroMonData(Pokemon *mon);

I don't see any consensus on pointer-star alignment for function return types. Three options:

Pokemon * AllocMonZeroed(u32 heapID);
// vs
Pokemon* AllocMonZeroed(u32 heapID);
// vs
Pokemon *AllocMonZeroed(u32 heapID);

Self-explanatory.

5. There is no consensus for defining recursive structs.

When defining a recursive struct, the struct must be declared and defined. Below are the options for declaring the struct.

• Option A1:

struct UnkSPLStruct6;

• Option A2:

typedef struct UnkSPLStruct6 UnkSPLStruct6;

Below are the options for defining the struct. Option A1 is only compatible with Option B1. Keywords/identifiers in <> are optional.

• Option B1:

typedef struct UnkSPLStruct6 {
    struct UnkSPLStruct6 * unk_00;
    struct UnkSPLStruct6 * unk_04;
  	// rest of struct omitted
} UnkSPLStruct6;

• Option B2:

typedef struct UnkSPLStruct6 {
    struct UnkSPLStruct6 * unk_00;
    struct UnkSPLStruct6 * unk_04;
};

• Option B3:

typedef struct UnkSPLStruct6 {
    UnkSPLStruct6 * unk_00;
    UnkSPLStruct6 * unk_04;
} <UnkSPLStruct6>;

• Option B4:

struct UnkSPLStruct6 {
    <struct> UnkSPLStruct6 * unk_00;
    <struct> UnkSPLStruct6 * unk_04;
};

6. There is no consensus on whether unions should follow the same rule as structs.

• Option 1:

// no union typedef
union StrbufForPrint {
    String *wrapped;
    const u16 *raw;
};

typedef struct TextPrinterTemplate {
	// mention union here
    union StrbufForPrint currentChar;
    Window *window;

	// rest of struct omitted
} TextPrinterTemplate;

• Option 2:

// union typedef
typedef union StrbufForPrint {
    String *wrapped;
    const u16 *raw;
} StrbufForPrint;

typedef struct TextPrinterTemplate {
	// no mentioning union
    StrbufForPrint currentChar;
    Window *window;

	// rest of struct omitted
} TextPrinterTemplate;

See the consensus discussion at #218

TODO write up a grep.

See the previous consensus discussion at #217

TODO write up a grep.

NCMP block can appear after NCLR block
Format:

• u16 numPalettes
• u16 0xBEEF
• u32 0x00000008
• u16 paletteIndices[numPalettes]

im lazy

remove links to pokediamond channels, replace with HG links

link to diamond and plat repos in other decomps

Useful grep command: grep -Pzowr --include="*.c" --include="*.h" --exclude-dir="tools" --exclude-dir="sub" --exclude-dir="lib" --exclude-dir=".github" --exclude-dir="files" 'switch \([^{]+\n?[^{\n]*{(?:\s+//.*?\n)*\s*case (?:(?:0x[0-9a-f]+|[0-9])*\s*[&\*\+-/\|]*)+:.*?\n' | tr -d '\000'

Unfortunately does not catch switch statements starting with default:, so a custom tool would need to be made

I didn't realize this was a contention point, but apparently it is. Don't waste time thinking about this issue, I just wanted to get these thoughts down while they were in my head.

Option 1: Use the SDK names as-is, regardless of inconsistency

• NitroSDK/card/card_pullOut.c
• NitroSDK/card/card_rom_teg.c
• NitroSDK/ctrdg/ctrdg_flash_LE26FV10N1TS-10.c
• NitroSDK/os/os_protectionRegion.c
• NitroSDK/rtc/internal.c
• NitroSDK/spi/mic.c
• NitroDWC/base/dwc_error.c
• NitroDWC/ilobby/dwci_lobbyBase.cpp
• NitroSystem/fnd/heapcommon.c
• NitroSystem/g2d/g2d_CellTransferManager.c
• NitroSystem/mcs/ringBuffer.c
• NitroSystem/snd/sndarc.c
• NitroSystem/snd/sndarc_loader.c
• NitroWiFi/soc/soc.c
• NitroWiFi/soc/socl_listen_accept.c
• NitroWiFi/stubs/md5/dummy_md5.c
• NitroWiFi/stubs/soc/soc_stub.c
• NitroWiFi/stubs/ssl/ssl_stub.c

Option 2: Use the SDK names as-is, except prefix each filename with the "module" folder name if the file does not already have a prefix

• NitroSDK/card/card_pullOut.c
• NitroSDK/card/card_rom_teg.c
• NitroSDK/ctrdg/ctrdg_flash_LE26FV10N1TS-10.c
• NitroSDK/os/os_protectionRegion.c
• NitroSDK/rtc/internal.c -> NitroSDK/rtc/rtc_internal.c
• NitroSDK/spi/mic.c -> NitroSDK/spi/spi_mic.c
• NitroDWC/base/dwc_error.c
• NitroDWC/ilobby/dwci_lobbyBase.cpp
• NitroSystem/fnd/heapcommon.c -> NitroSystem/fnd/fnd_heapcommon.c
• NitroSystem/g2d/g2d_CellTransferManager.c
• NitroSystem/mcs/ringBuffer.c -> NitroSystem/mcs/mcs_ringBuffer.c

TODO: what to do with these weird examples

• NitroSystem/snd/sndarc.c
• NitroSystem/snd/sndarc_loader.c (prefix with snd, or keep sndarc?)
• NitroWiFi/soc/soc.c
• NitroWiFi/soc/socl_listen_accept.c
• NitroWiFi/stubs/md5/dummy_md5.c
• NitroWiFi/stubs/soc/soc_stub.c
• NitroWiFi/stubs/ssl/ssl_stub.c

Option 3: Do our own thing

Currently unknown what possible "our things" we could do.

Option 3A: Replace or add each file prefix with the prefix used in the file functions, capitalization included. Preserve the module descriptor.

This is what @red031000 wants, but there are a few unknowns even from her description of what she wanted.

It is unknown how to handle the case where a prefix ends with i. E.g. NitroSDK/rtc and NitroDWC/base/ use both RTCi? and DWCi? respectively. The i does not imply static; functions with an i suffix in the prefix are still part of the public API. For now, assume don't use i.

Additionally, some modules have some kind of "sub-prefix". E.g. in NitroDWC/ilobby/dwci_lobbyBase.cpp, all functions are prefixed with DWCi_Lobby<rest of function name>, without a space before the <rest of function name>. The same applies to NitroSystem/fnd/heapcommon.c (NNSi?_Fnd) and NitroSystem/g2d/g2d_CellTransferManager.c (NNSi?_G2d). Files which have this "sub-prefix" will be marked as such, and conversions will be given for both the first prefix and sub-prefix.

• NitroSDK/card/card_pullOut.c -> NitroSDK/card/CARD_pullOut.c
• NitroSDK/card/card_rom_teg.c -> NitroSDK/card/CARD_rom_teg.c
• NitroSDK/ctrdg/ctrdg_flash_LE26FV10N1TS-10.c -> NitroSDK/ctrdg/CTRDG_flash_LE26FV10N1TS-10.c
• NitroSDK/os/os_protectionRegion.c -> NitroSDK/os/OS_protectionRegion.c
• NitroSDK/rtc/internal.c -> NitroSDK/rtc/RTC_internal.c (uses i)
• NitroSDK/spi/mic.c:
  • Rule fails: mic.c uses MIC prefix but NitroSDK/spi/pm.c uses PMi? and NitroSDK/spi/tp.c uses TPi?.
• NitroDWC/base/dwc_error.c -> NitroDWC/base/DWC_error.c (uses i)
• NitroDWC/ilobby/dwci_lobbyBase.cpp: (full prefix DWCi_Lobby)
  • Rule warning: module ONLY uses DWCi. Possible conversions: NitroDWC/ilobby/DWCi_lobbyBase.cpp, NitroDWC/ilobby/lobby_lobbyBase.cpp.
• NitroSystem/fnd/heapcommon.c -> NitroSystem/fnd/NNS_heapcommon.c, NitroSystem/fnd/Fnd_heapcommon.c (full prefix NNS_Fnd)
• NitroSystem/g2d/g2d_CellTransferManager.c -> NitroSystem/g2d/NNS_CellTransferManager.c, NitroSystem/g2d/G2d_CellTransferManager.c (full prefix NNS_G2d)
• NitroSystem/mcs/ringBuffer.c -> NitroSystem/mcs/NNS_ringBuffer.c, NitroSystem/mcs/Mcs_ringBuffer.c (full prefix NNS_Mcs)
• NitroSystem/snd/sndarc.c -> NitroSystem/snd/NNS_sndarc.c, NitroSystem/snd/SndArc_sndarc.c (full prefix NNS_SndArc)
• NitroSystem/snd/sndarc_loader.c -> NitroSystem/snd/NNS_sndarc_loader.c, NitroSystem/snd/SndArc_sndarc_loader.c (full prefix NNS_SndArc, but also it is true that full prefix is NNS_SndArcLoad)
• NitroWiFi/soc/soc.c -> NitroWiFi/soc/SOC_soc.c
• NitroWiFi/soc/socl_listen_accept.c -> NitroWiFi/soc/SOCL_listen_accept.c

TODO: what to do with nested directory libraries

• NitroWiFi/stubs/md5/dummy_md5.c
• NitroWiFi/stubs/soc/soc_stub.c
• NitroWiFi/stubs/ssl/ssl_stub.c

Script/Bytecode styling

Probably don't need to explain what scripts are (e.g. trainer AI scripts, battle scripts, event scripts (if they're even called that in Gen 4)).

Shorthand for options are in the format ., e.g. for snake_case local labels, write 3a.2. But it's always good to mention what the option is anyway.

Example format for what styling you want

Command name styling: PascalCase
Entry-point label styling: PascalCase
Use local labels: No
Within-subroutine label styling: _UnderscorePrefixPascalCase

Copypasteable version:

Command name styling: `PascalCase`
Entry-point label styling: `PascalCase`
Use local labels: No
Within-subroutine label styling: `_UnderscorePrefixPascalCase`

1. There is no consensus for command name styling

Option 1: PascalCase

    MoldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP
    PrintAttackMessage
    WaitMessage
    Wait 0x1E
    ChangeVar VAR_OP_SETMASK, VAR_SERVER_STATUS1, 0x2
    ChangeVar VAR_OP_SETMASK, VAR_MOVE_STATUS, 0x80000000
    EndScript

Option 2: alllowercase

    moldbreakerabilitycheck 0x0, BATTLER_ALL, ABILITY_DAMP
    printattackmessage
    waitmessage
    wait 0x1E
    changevar VAR_OP_SETMASK, VAR_SERVER_STATUS1, 0x2
    changevar VAR_OP_SETMASK, VAR_MOVE_STATUS, 0x80000000
    endscript

Option 3: snake_case

    mold_breaker_ability_check 0x0, BATTLER_ALL, ABILITY_DAMP
    print_attack_message
    wait_message
    wait 0x1E
    change_var VAR_OP_SETMASK, VAR_SERVER_STATUS1, 0x2
    change_var VAR_OP_SETMASK, VAR_MOVE_STATUS, 0x80000000
    endscript

Option 4: camelCase

    moldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP
    printAttackMessage
    waitMessage
    wait 0x1E
    changeVar VAR_OP_SETMASK, VAR_SERVER_STATUS1, 0x2
    changeVar VAR_OP_SETMASK, VAR_MOVE_STATUS, 0x80000000
    endScript

2. There is no consensus for styling for "entry-point" or "subroutine" script labels

Option 1: PascalCase

CommonDamageCalc:
    CritCalc
    DamageCalc
    EndScript

CommonDamageCalc:
    critcalc
    damagecalc
    endscript

CommonDamageCalc:
    crit_calc
    damage_calc
    end_script

CommonDamageCalc:
    critCalc
    damageCalc
    endScript

Option 2: snake_case

common_damage_calc:
    CritCalc
    DamageCalc
    EndScript

common_damage_calc:
    critcalc
    damagecalc
    endscript

common_damage_calc:
    crit_calc
    damage_calc
    end_script

common_damage_calc:
    critCalc
    damageCalc
    endScript

Option 3: camelCase

commonDamageCalc:
    CritCalc
    DamageCalc
    EndScript

commonDamageCalc:
    critcalc
    damagecalc
    endscript

commonDamageCalc:
    crit_calc
    damage_calc
    end_script

commonDamageCalc:
    critCalc
    damageCalc
    endScript

Option 4: _UnderscorePrefixPascalCase
Note: Technically shouldn't be done because _ prefix means a symbol is reserved. This should only matter for Trainer AI scripts which are linked with C files.

_CommonDamageCalc:
    CritCalc
    DamageCalc
    EndScript

_CommonDamageCalc:
    critcalc
    damagecalc
    endscript

_CommonDamageCalc:
    crit_calc
    damage_calc
    end_script

_CommonDamageCalc:
    critCalc
    damageCalc
    endScript

Suggest more options in the comments.

3. There is no consensus for "within-subroutine" labels

This issue is a bit more complicated because it depends on whether you use mwasmarm or not. mwasmarm has true local label support, where a label goes out of scope after a non-local label, for example:

DealDamage:
  if DAMAGE, 0, @done
  dealdamage
@done: // . at the start means it's a local label
  end

AccuracyCheck:
  if ACCURACY, 100, @done
  accuracycheck
@done: // this will not throw an error, because AccuracyCheck was defined
       // undefining the previous .done
  end

mwasmarm pros:

• Unified toolset under the metrowerks label (probably higher compatibility, something about how metrowerks handles c preprocessor in asm files)
  • Note: Trainer AI is included as an object, not a dumped binary, so it HAS to be a mwasmarm object.
• True local label support (as opposed to GNU Assembler)

mwasmarm cons:

• Ancient binary (slight performance hit) + more shit to run through wine
• Prevents hypothetical pure GNU-based build from existing (with no Nintendo/Metrowerks tooling at all)

GNU assembler pros are the reverse of mwasmarm pros

GNU assembler cons (not mentioned above):

• Enums cannot be handled by the C Preprocessor on assembly files, they have to be #defines

3a. If using local labels (mwasmarm assumed/forced)

Option 1: @camelCase

    MoldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, @anyBattlerHasDamp
    AbilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @attackerHasCorrosion
    EndScript
@anyBattlerHasDamp:
    CritCalc
    EndScript
@attackerHasCorrosion:
    DamageCalc
    EndScript

    moldbreakerabilitycheck 0x0, BATTLER_ALL, ABILITY_DAMP, @anyBattlerHasDamp
    abilitycheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @attackerHasCorrosion
    endscript
@anyBattlerHasDamp:
    critcalc
    endscript
@attackerHasCorrosion:
    damagecalc
    endscript

    mold_breaker_ability_check 0x0, BATTLER_ALL, ABILITY_DAMP, @anyBattlerHasDamp
    ability_check 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @attackerHasCorrosion
    end_script
@anyBattlerHasDamp:
    crit_calc
    end_script
@attackerHasCorrosion:
    damage_calc
    end_script

    moldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, @anyBattlerHasDamp
    abilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @attackerHasCorrosion
    endEcript
@anyBattlerHasDamp:
    critCalc
    endScript
@attackerHasCorrosion:
    damagecalc
    endScript

Option 2: @snake_case

    MoldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, @any_battler_has_damp
    AbilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @attacker_has_corrosion
    EndScript
@any_battler_has_damp:
    CritCalc
    EndScript
@attacker_has_corrosion:
    DamageCalc
    EndScript

    moldbreakerabilitycheck 0x0, BATTLER_ALL, ABILITY_DAMP, @any_battler_has_damp
    abilitycheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @attacker_has_corrosion
    endscript
@any_battler_has_damp:
    critcalc
    endscript
@attacker_has_corrosion:
    damagecalc
    endscript

    mold_breaker_ability_check 0x0, BATTLER_ALL, ABILITY_DAMP, @any_battler_has_damp
    ability_check 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @attacker_has_corrosion
    end_script
@any_battler_has_damp:
    crit_calc
    end_script
@attacker_has_corrosion:
    damage_calc
    end_script

    moldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, @any_battler_has_damp
    abilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @attacker_has_corrosion
    endEcript
@any_battler_has_damp:
    critCalc
    endScript
@attacker_has_corrosion:
    damagecalc
    endScript

Option 3: @PascalCase

    MoldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, @AnyBattlerHasDamp
    AbilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @AttackerHasCorrosion
    EndScript
@AnyBattlerHasDamp:
    CritCalc
    EndScript
@AttackerHasCorrosion:
    DamageCalc
    EndScript

    moldbreakerabilitycheck 0x0, BATTLER_ALL, ABILITY_DAMP, @AnyBattlerHasDamp
    abilitycheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @AttackerHasCorrosion
    endscript
@AnyBattlerHasDamp:
    critcalc
    endscript
@AttackerHasCorrosion:
    damagecalc
    endscript

    mold_breaker_ability_check 0x0, BATTLER_ALL, ABILITY_DAMP, @AnyBattlerHasDamp
    ability_check 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @AttackerHasCorrosion
    end_script
@AnyBattlerHasDamp:
    crit_calc
    end_script
@AttackerHasCorrosion:
    damage_calc
    end_script

    moldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, @AnyBattlerHasDamp
    abilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, @AttackerHasCorrosion
    endEcript
@AnyBattlerHasDamp:
    critCalc
    endScript
@AttackerHasCorrosion:
    damagecalc
    endScript

3b. If not using local labels

Option 1: PascalCase_WithPascalCaseSubLabel (implies PascalCase for entry-point label)

SpecialMoveAbilityCheck:
    MoldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_AnyBattlerHasDamp
    AbilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_AttackerHasCorrosion
    EndScript
SpecialMoveAbilityCheck_AnyBattlerHasDamp:
    CritCalc
    EndScript
SpecialMoveAbilityCheck_AttackerHasCorrosion:
    DamageCalc
    EndScript

SpecialMoveAbilityCheck:
    moldbreakerabilitycheck 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_AnyBattlerHasDamp
    abilitycheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_AttackerHasCorrosion
    endscript
SpecialMoveAbilityCheck_AnyBattlerHasDamp:
    critcalc
    endscript
SpecialMoveAbilityCheck_AttackerHasCorrosion:
    damagecalc
    endscript

SpecialMoveAbilityCheck:
    mold_breaker_ability_check 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_AnyBattlerHasDamp
    ability_check 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_AttackerHasCorrosion
    end_script
SpecialMoveAbilityCheck_AnyBattlerHasDamp:
    crit_calc
    end_script
SpecialMoveAbilityCheck_AttackerHasCorrosion:
    damage_calc
    end_script

SpecialMoveAbilityCheck:
    moldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_AnyBattlerHasDamp
    abilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_AttackerHasCorrosion
    endEcript
SpecialMoveAbilityCheck_AnyBattlerHasDamp:
    critCalc
    endScript
SpecialMoveAbilityCheck_AttackerHasCorrosion:
    damagecalc
    endScript

Option 2: PascalCase_withCamelCaseSubLabel (implies PascalCase for entry-point label)

SpecialMoveAbilityCheck:
    MoldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_anyBattlerHasDamp
    AbilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_attackerHasCorrosion
    EndScript
SpecialMoveAbilityCheck_anyBattlerHasDamp:
    CritCalc
    EndScript
SpecialMoveAbilityCheck_attackerHasCorrosion:
    DamageCalc
    EndScript

SpecialMoveAbilityCheck:
    moldbreakerabilitycheck 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_anyBattlerHasDamp
    abilitycheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_attackerHasCorrosion
    endscript
SpecialMoveAbilityCheck_anyBattlerHasDamp:
    critcalc
    endscript
SpecialMoveAbilityCheck_attackerHasCorrosion:
    damagecalc
    endscript

SpecialMoveAbilityCheck:
    mold_breaker_ability_check 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_anyBattlerHasDamp
    ability_check 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_attackerHasCorrosion
    end_script
SpecialMoveAbilityCheck_anyBattlerHasDamp:
    crit_calc
    end_script
SpecialMoveAbilityCheck_attackerHasCorrosion:
    damage_calc
    end_script

SpecialMoveAbilityCheck:
    moldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_anyBattlerHasDamp
    abilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_attackerHasCorrosion
    endEcript
SpecialMoveAbilityCheck_anyBattlerHasDamp:
    critCalc
    endScript
SpecialMoveAbilityCheck_attackerHasCorrosion:
    damagecalc
    endScript

Option 3: PascalCase_with_snake_case_sub_label (implies PascalCase for entry-point label)

SpecialMoveAbilityCheck:
    MoldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_any_battler_has_damp
    AbilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_attacker_has_corrosion
    EndScript
SpecialMoveAbilityCheck_any_battler_has_damp:
    CritCalc
    EndScript
SpecialMoveAbilityCheck_attacker_has_corrosion:
    DamageCalc
    EndScript

SpecialMoveAbilityCheck:
    moldbreakerabilitycheck 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_any_battler_has_damp
    abilitycheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_attacker_has_corrosion
    endscript
SpecialMoveAbilityCheck_any_battler_has_damp:
    critcalc
    endscript
SpecialMoveAbilityCheck_attacker_has_corrosion:
    damagecalc
    endscript

SpecialMoveAbilityCheck:
    mold_breaker_ability_check 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_any_battler_has_damp
    ability_check 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_attacker_has_corrosion
    end_script
SpecialMoveAbilityCheck_any_battler_has_damp:
    crit_calc
    end_script
SpecialMoveAbilityCheck_attacker_has_corrosion:
    damage_calc
    end_script

SpecialMoveAbilityCheck:
    moldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, SpecialMoveAbilityCheck_any_battler_has_damp
    abilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, SpecialMoveAbilityCheck_attacker_has_corrosion
    endEcript
SpecialMoveAbilityCheck_any_battler_has_damp:
    critCalc
    endScript
SpecialMoveAbilityCheck_attacker_has_corrosion:
    damagecalc
    endScript

Option 4: _underscorePrefixCamelCase, just try to avoid label conflicts

SpecialMoveAbilityCheck:
    MoldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, _anyBattlerHasDamp
    AbilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _attackerHasCorrosion
    EndScript
_anyBattlerHasDamp:
    CritCalc
    EndScript
_attackerHasCorrosion:
    DamageCalc
    EndScript

SpecialMoveAbilityCheck:
    moldbreakerabilitycheck 0x0, BATTLER_ALL, ABILITY_DAMP, _anyBattlerHasDamp
    abilitycheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _attackerHasCorrosion
    endscript
_anyBattlerHasDamp:
    critcalc
    endscript
_attackerHasCorrosion:
    damagecalc
    endscript

SpecialMoveAbilityCheck:
    mold_breaker_ability_check 0x0, BATTLER_ALL, ABILITY_DAMP, _anyBattlerHasDamp
    ability_check 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _attackerHasCorrosion
    end_script
_anyBattlerHasDamp:
    crit_calc
    end_script
_attackerHasCorrosion:
    damage_calc
    end_script

SpecialMoveAbilityCheck:
    moldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, _anyBattlerHasDamp
    abilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _attackerHasCorrosion
    endEcript
_anyBattlerHasDamp:
    critCalc
    endScript
_attackerHasCorrosion:
    damagecalc
    endScript

Option 5: _UnderscorePrefixPascalCase, just try to avoid label conflicts

SpecialMoveAbilityCheck:
    MoldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, _AnyBattlerHasDamp
    AbilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _AttackerHasCorrosion
    EndScript
_AnyBattlerHasDamp:
    CritCalc
    EndScript
_AttackerHasCorrosion:
    DamageCalc
    EndScript

SpecialMoveAbilityCheck:
    moldbreakerabilitycheck 0x0, BATTLER_ALL, ABILITY_DAMP, _AnyBattlerHasDamp
    abilitycheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _AttackerHasCorrosion
    endscript
_AnyBattlerHasDamp:
    critcalc
    endscript
_AttackerHasCorrosion:
    damagecalc
    endscript

SpecialMoveAbilityCheck:
    mold_breaker_ability_check 0x0, BATTLER_ALL, ABILITY_DAMP, _AnyBattlerHasDamp
    ability_check 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _AttackerHasCorrosion
    end_script
_AnyBattlerHasDamp:
    crit_calc
    end_script
_AttackerHasCorrosion:
    damage_calc
    end_script

SpecialMoveAbilityCheck:
    moldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, _AnyBattlerHasDamp
    abilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _AttackerHasCorrosion
    endEcript
_AnyBattlerHasDamp:
    critCalc
    endScript
_AttackerHasCorrosion:
    damagecalc
    endScript

Option 6: _underscore_prefix_snake_case, just try to avoid label conflicts

SpecialMoveAbilityCheck:
    MoldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, _any_battler_has_damp
    AbilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _attacker_has_corrosion
    EndScript
_any_battler_has_damp:
    CritCalc
    EndScript
_attacker_has_corrosion:
    DamageCalc
    EndScript

SpecialMoveAbilityCheck:
    moldbreakerabilitycheck 0x0, BATTLER_ALL, ABILITY_DAMP, _any_battler_has_damp
    abilitycheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _attacker_has_corrosion
    endscript
_any_battler_has_damp:
    critcalc
    endscript
_attacker_has_corrosion:
    damagecalc
    endscript

SpecialMoveAbilityCheck:
    mold_breaker_ability_check 0x0, BATTLER_ALL, ABILITY_DAMP, _any_battler_has_damp
    ability_check 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _attacker_has_corrosion
    end_script
_any_battler_has_damp:
    crit_calc
    end_script
_attacker_has_corrosion:
    damage_calc
    end_script

SpecialMoveAbilityCheck:
    moldBreakerAbilityCheck 0x0, BATTLER_ALL, ABILITY_DAMP, _any_battler_has_damp
    abilityCheck 0x0, BATTLER_ATTACKER, ABILITY_CORROSION, _attacker_has_corrosion
    endEcript
_any_battler_has_damp:
    critCalc
    endScript
_attacker_has_corrosion:
    damagecalc
    endScript

Examples to kick off discussion.

Example 1

Example 2

Example 3

Example 4

Example 5

Useful grep to find instances:
grep -Pwr --include="*.c" --include="*.h" "[a-z_]+_[a-z_]+[,;\)]" --exclude-dir="tools" --exclude-dir="sub" --exclude-dir="lib" --exclude-dir=".github" --exclude-dir="files"

Do not apply this logic to Nintendo Libraries.

Local variable example

Good:

void String_RadioAddStatic(String *string, u8 level) {
    u32 width3Dots = FontID_GetGlyphWidth(0, CHAR_ELLIPSIS);
    u32 width1Dot = FontID_GetGlyphWidth(0, CHAR_ONE_DOT);
    u32 width2Dots = FontID_GetGlyphWidth(0, CHAR_TWO_DOTS);
    u32 curWidth;
    int strLen;
    int i;

	// rest of function omitted
}

Bad:

void String_RadioAddStatic(String *string, u8 level) {
    u32 width_3dots = FontID_GetGlyphWidth(0, CHAR_ELLIPSIS);
    u32 width_1dot = FontID_GetGlyphWidth(0, CHAR_ONE_DOT);
    u32 width_2dots = FontID_GetGlyphWidth(0, CHAR_TWO_DOTS);
    u32 cur_width;
    int str_len;
    int i;

	// rest of function omitted
}

Function parameter example

Good:

int TrainerData_GetAttr(u32 trIdx, TrainerAttr attrNo);

Bad:

int TrainerData_GetAttr(u32 tr_idx, TrainerAttr attr_no);

Struct field example

Good:

typedef struct MailMessage {
    u16 msgBank; // camelCase
    u16 msgNo;
    u16 fields[MAILMSG_FIELDS_MAX];
} MailMessage;

Bad:

typedef struct MailMessage {
    u16 msg_bank; // snake_case
    u16 msg_no;
    u16 fields[MAILMSG_FIELDS_MAX];
} MailMessage;

Exception: unknown fields. There is no consensus whether unk fields should have an underscore between the unk and the offset value (and probably other stuff)

• Case 1: underscore

struct HiddenItemData {
    u16 itemId;
    u8 quantity;
    u8 unk_3;
    u16 unk_4;
    u16 index;
};

• Case 2: no underscore

struct HiddenItemData {
    u16 itemId;
    u8 quantity;
    u8 unk3;
    u16 unk4;
    u16 index;
};

Hello. Could you please add Overlay77 from (J) version that is the slot machines code? Maybe in the future could you be able to adapt it to works with western releases.

It makes no sense. There are four structs: SaveArrayHeader, SaveArrayFooter, SaveChunkHeader, SaveChunkFooter. SaveChunkHeader isn't even defined in the same place as SaveChunkFooter.

static void CreateChunkFooter(SaveData *saveData, void *data, int idx, u32 size) {
    struct SaveArrayFooter *footer;

    footer = (struct SaveArrayFooter *)((u8 *)data + size);

    footer->magic = SAVE_CHUNK_MAGIC;
    footer->saveno = saveData->lastGoodSaveNo + 1;
    footer->size = size;
    footer->idx = idx;
    footer->crc = GF_CalcCRC16(data, size + offsetof(struct SaveArrayFooter, crc));
}

The function is named CreateChunkFooter yet it doesn't even use SaveChunkFooter.

Probably a bunch of other stuff, I could spend 15 minutes and find more inconsistencies but I don't think it's necessary to point out more stuff.

Cardinality is just number of elements for a set. So it could refer to the total length of an array, the number of values an enum could have, and other stuff.

For example:

u16 GetTotalMulchQuantity(Bag *bag, HeapID heapId) {
    s32 i;
    u16 total;
    for (total = 0, i = 0; i < NUM_MULCHES; i++) {
        total += Bag_GetQuantity(bag, ITEM_GROWTH_MULCH + i, heapId);
    }
    return total;
}

NUM_ to refer to the number of elements in a set is weird, because you could also have numMulches which stores the current amount of mulches. TODO investigate what naming we want.

We could say TOTAL_MULCHES, but in the above example, total is used to determine a dynamic cardinality.

It's probably good to determine what terminology should be used.

ARM9 overlays still need to be dumped

Please please please let's not decide on _021D4178 as it is ambiguous with auto-disassembled "anonymous" labels.

Possible candidates are:

• g021D4178 (shinny notation)
• gUnk_021D4178 (what was used in the gen 3 decomps. Can omit the underscore)

particularly msl routines, the calls to which are automatically inserted by the compiler so it is of utmost importance that they are correct

Variant A: Case labels flush with the switch statement. This is the standard in pret/pokeruby etc.

switch (state) {
case 0:
    // ...
case 1:
    // ...
}

Variant B: Case labels indented by 1.

switch (state) {
    case 0:
        // ...
    case 1:
        // ...
}