Decompile sub_11C14 #64
Conversation
There was a problem hiding this comment.
What is the status of sub_11C14? Based on src/rom_11B9C.c, it appears to be fully decompiled. If that is the case, then I'm confused as to this file's purpose - it should be deleted unless there is some extenuating circumstance. If sub_11C14 is not fully decompiled, then I will need more information in order to suggest what should be done.
| @@ -72,6 +72,22 @@ | |||
| #define JOY_NEW_EXACT(button) TEST_BUTTON_EXACT(gMain.newKeys, button) | |||
| #define JOY_HELD_EXACT(button) TEST_BUTTON_EXACT(gMain.heldKeys, button) | |||
|
|
|||
| struct unk_struct{ | |||
There was a problem hiding this comment.
A few style corrections:
- The brace should be on its own line
- Struct names should generally be UpperCamelCase, with underscores as needed.
- Unknown structs should generally be named UnkStruct_<address>. Obviously an address would be difficult in this situation, but I would probably rename it to
UnkStruct_PinballGame_132Cor perhaps evenUnkSubstruct_PinballGame_132C. I will leave it up to you to come up with the exact name.
My general advice would be that if you are in doubt about the style guidelines, look at other files for guidance. At least, that's what I've been doing this entire time. Sometimes other files won't be helpful (I still don't know the proper style for switch-case indentation), but it will eventually become second nature once you get more familiar with the codebase. (Although, I will admit, I still catching myself putting braces on the same line as the function/if/for/etc.)
There was a problem hiding this comment.
Another option for 3 that I encountered while reviewing #67 is PinballGameUnk132C. Really, as long as it mentions PinballGame, 132C, and Unk; any name you can come up with will do.
| /*0x8*/ s16 unk8; | ||
| /*0x00*/ u8 filler0[0x8]; | ||
| /*0x08*/ s16 unk8; | ||
| /*0x0A*/ u8 unk10[0x1C]; |
There was a problem hiding this comment.
I believe this should be filler10[0x1C]. If you are working on another function and know that unk10 is an array of exactly 0x1C bytes then this is fine to leave as is, otherwise it should be changed.
| @@ -46,6 +46,18 @@ struct Coord32 | |||
| u32 y; | |||
| }; | |||
|
|
|||
| struct sCoord16 | |||
There was a problem hiding this comment.
This is one of the few times I would say it is appropriate to start a struct name with a lower case letter. In fact, if you feel comfortable doing so, I would rename the original Coord16/32 to uCoord16/32 to help disambiguate them. If not, I can do so later in another PR.
There was a problem hiding this comment.
I would actually recommend we follow pokeemerald's naming scheme, which is:
struct Coords16
{
s16 x;
s16 y;
};
struct UCoords16
{
u16 x;
u16 y;
};
Description
Implemented sub_11C14
Added signed versions of the coordinate structures
Added struct unk_struct (I expect to get a feedback saying to rename this)
Modified struct PinballGame
Modified struct Unk02031520
Discord contact info
adewotta#1337