pret / pokeheartgold

Decompilation of Pokemon HeartGold/SoulSilver
310 stars 97 forks source link

All local variables, function parameters, struct fields should be camelCase. #215

Open luckytyphlosion opened 1 year ago

luckytyphlosion commented 1 year ago

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)

struct HiddenItemData {
    u16 itemId;
    u8 quantity;
    u8 unk_3;
    u16 unk_4;
    u16 index;
};
struct HiddenItemData {
    u16 itemId;
    u8 quantity;
    u8 unk3;
    u16 unk4;
    u16 index;
};
AnonymousRandomPerson commented 1 year ago

I don't see a reason to include underscores in the unk vars if nothing else has them.

red031000 commented 1 year ago

imo no underscore in unknown fields, again this is already in the style guide, I just haven't gone through and fixed it all yet

PikalaxALT commented 1 year ago

Hi, snakehead (Python programmer) here. snake_case is beautiful and any attempts to eliminate it will be met with Protean Arceus.

adrienntindall commented 1 year ago

imo no underscore in unknown fields, again this is already in the style guide, I just haven't gone through and fixed it all yet

I use underscore in BattleContext to differentiate it from unknowns in BattleSystem since it's much easier to control and replace that way without conflicts lol

luckytyphlosion commented 1 year ago

imo no underscore in unknown fields, again this is already in the style guide, I just haven't gone through and fixed it all yet

I use underscore in BattleContext to differentiate it from unknowns in BattleSystem since it's much easier to control and replace that way without conflicts lol

That sounds like you need a better way of distinguishing the fields.

adrienntindall commented 1 year ago

imo no underscore in unknown fields, again this is already in the style guide, I just haven't gone through and fixed it all yet

I use underscore in BattleContext to differentiate it from unknowns in BattleSystem since it's much easier to control and replace that way without conflicts lol

That sounds like you need a better way of distinguishing the fields.

No like I would cntrl + f and replace the unkowns constantly and it was much easier to do so when I wasn't accidently replacing the wrong unkA40 or whatever

luckytyphlosion commented 1 year ago

yes so do something like unk_z_A40 or something

tgsm commented 1 year ago

I personally prefer snake_case, no underscores in unknown variables/members

AsparagusEduardo commented 1 year ago

I prefer camelCase in general, though I feel it would be useful to have unknown fields to stand out with a different style specifically to make people document them.

luckytyphlosion commented 1 year ago

On the record:

Donnel — Today at 10:10 PM I’m for camelCase Lhea — Today at 10:10 PM I'm for camelCase (ignore that I haven't worked on pokeplatinum lately)

github-actions[bot] commented 1 month ago

This issue has had no activity for 60 days and will be marked stale. If there is no further activity, it will be closed in 30 days.