Decomp/pokegear configure #405
Conversation
| @@ -0,0 +1,57 @@ | |||
| #ifndef GUARD_POKEHEARTGOLD_APPLICATION_POKEGEAR_CONFIGURE_POKEGEAR_CONFIGURE_INTERNAL_H | |||
There was a problem hiding this comment.
can we have a more sane name pls (and one that follows the convention of the other ones)
There was a problem hiding this comment.
the name is a representation of the path relative to the includedir, as has been standard for this project
There was a problem hiding this comment.
sure, it's just this one does get kinda long but I guess that's fair, though I would prefer it to not have GUARD in front as the others don't, but it's not siginificant
| #include "touchscreen.h" | ||
|
|
||
| enum TouchscreenListMenuTextAlignment { | ||
| TSMENU_ALN_LEFT, |
There was a problem hiding this comment.
I'd prefer a longer and more descriptive value
(at least expand ALN to ALIGN)
There was a problem hiding this comment.
ok... is there any particular reason? the expanded name would be more descriptive
There was a problem hiding this comment.
it would be unnecessarily verbose, and "aln" is a common-enough shortening of "alignment" to be well understood
There was a problem hiding this comment.
I don't consider this to be unnecessarily verbose by any means, we use ALIGN elsewhere in the project so it's a bit inconsistent to just switch to ALN
There was a problem hiding this comment.
this is pointless bikeshedding
| @@ -0,0 +1,387 @@ | |||
| #include "application/pokegear/configure/pokegear_configure_internal.h" | |||
| int PokegearConfigure_ContextMenu(PokegearConfigureAppData *configureApp) { | ||
| int input; | ||
|
|
||
| input = TouchscreenListMenu_HandleInput(configureApp->contextMenu); |
There was a problem hiding this comment.
merge declaration and assignment?
There was a problem hiding this comment.
the style for this component is declare first, assign later
There was a problem hiding this comment.
I don't believe we ever actually agreed on this style, so we probably should actually agree on this
(if we did I'd appreciate a link to where we discussed this because I can't remember this immediately)
There was a problem hiding this comment.
Enough functions within the pokegear scope require forward-declaration of locals to match, that it is likely the style used by the original dev for all functions therein. I may go back at some point and retro the rest of pokegear to fit this style.
| int i; | ||
| GX_SetGraphicsMode(GX_DISPMODE_GRAPHICS, GX_BGMODE_0, GX_BG0_AS_2D); | ||
|
|
||
| { |
There was a problem hiding this comment.
is there a reason for this empty scope?
There was a problem hiding this comment.
it's standard for these types of routines
| @@ -0,0 +1,149 @@ | |||
| #include "application/pokegear/configure/pokegear_configure_internal.h" | |||
| .wrapAround = TRUE, | ||
| .centered = TRUE, | ||
| .xOffset = 0, | ||
| .bgId = 1, |
There was a problem hiding this comment.
okay this one's fair
| void PokegearConfigure_LoadGraphics_Internal(PokegearConfigureAppData *configureApp) { | ||
| NARC *narc; | ||
|
|
||
| narc = NARC_New(NARC_a_1_4_5, configureApp->heapId); |
There was a problem hiding this comment.
merge declaration and assignment
|
|
||
| BOOL PokegearConfigure_Exit(OverlayManager *man, int *state) { | ||
| PokegearConfigureAppData *configureApp; | ||
| enum HeapID heapID; |
There was a problem hiding this comment.
does this one needs to be a separate variable?
does it match without?
There was a problem hiding this comment.
it absolutely does need to be a separate variable, otherwise it's a read-after-free
| configureApp->pokegear->reselectAppCB = PokegearConfigure_OnReselectApp; | ||
| configureApp->skin = Pokegear_GetBackgroundStyle(configureApp->pokegear->savePokegear); | ||
| configureApp->unlockedSkins = Pokegear_GetUnlockedSkins(configureApp->pokegear->savePokegear); | ||
| configureApp->unlockedSkins = 0xFF; // nani the fuck? |
There was a problem hiding this comment.
i don't think it's a bug, i think it's a feature that was removed later in development
red031000
dismissed
their stale review
dismissing so as not to block any approvals
2 participants
No description provided.