-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
unk_020215A0 #304
unk_020215A0 #304
Conversation
e1acf4d
to
4f2eb1a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
few things
also as a more general point, can we rename ObjCharTransfer to VramTransfer (or CharVramTransfer if it's specific for chars and there are other vram transfers out there)
just cause we already started referring to the process as vram transfer, and it's weird seeing ObjCharTransfer call NNS's vram transfer funcs and not be named the same thing (also it's inconsistent with nitrogfx)
sorry if I was a bit harsh, I tend to do that sometimes, I get annoyed by minor things, nothing personal ofc
src/credits/credits.c
Outdated
@@ -463,15 +463,15 @@ static void LoadBgGraphics(CreditsAppWork *work) { | |||
} | |||
|
|||
static void CreateOamAndObjResMgrs(CreditsAppWork *work) { | |||
UnkStruct_020215A0 temp; | |||
ObjCharTransferTemplate temp; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename from temp?
include/constants/global.h
Outdated
@@ -13,6 +13,15 @@ | |||
|
|||
#define PARTY_SIZE 6 | |||
|
|||
// alias | |||
#ifndef PM_ASM | |||
// NNS_G2D_VRAM_TYPE_3DMAIN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be in global.h? wouldn't it be more appropriate to put it in an nns vram header, or even just a generic vram header?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should absolutely NOT put something in an sdk header that did not originally come from that sdk, this is to make hotswapping the sdk later actually feasible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
understandable, however, it should be in a generic vram header then, and not in global.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should still not be in global.h, needs to be moved out of here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where would you put it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
constants/vram.h, it would be a new file
also any vram transfer file that's used by all of them is also acceptable imo
include/constants/global.h
Outdated
// NNS_G2D_VRAM_TYPE_3DMAIN | ||
#define NNS_G2D_VRAM_TYPE_NEITHER ((NNS_G2D_VRAM_TYPE)0) | ||
// NNS_G2D_VRAM_TYPE_MAX | ||
#define NNS_G2D_VRAM_TYPE_BOTH ((NNS_G2D_VRAM_TYPE)(NNS_G2D_VRAM_TYPE_2DMAIN|NNS_G2D_VRAM_TYPE_2DSUB)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both is a weird one for this, since for me in my brain both means 2d and 3d, but here both is 2dmain and sub
suggestion:
NNS_G2D_VRAM_TYPE_2DMAIN_SUB
or similar (2DBOTH may be also acceptable)
src/intro_movie.c
Outdated
@@ -185,10 +185,10 @@ static void IntroMovie_InitSpriteGraphicsHW(IntroMovieOverlayData *data) { | |||
GX_SetOBJVRamModeChar(GX_OBJVRAMMODE_CHAR_1D_32K); | |||
GXS_SetOBJVRamModeChar(GX_OBJVRAMMODE_CHAR_1D_32K); | |||
|
|||
UnkStruct_020215A0 sp14 = {10, 0, 0, HEAP_ID_INTRO_MOVIE}; | |||
sub_020215A0(&sp14); | |||
ObjCharTransferTemplate sp14 = {10, 0, 0, HEAP_ID_INTRO_MOVIE}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename sp14
src/obj_char_transfer.c
Outdated
} | ||
} | ||
|
||
static BOOL getBlockNumAndFreeSpaceForTransfer(int vram, u32 *blockNumMain, u32 *blockNumSub, u32 size, u32 *freeSpaceMain, u32 *freeSpaceSub) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see a static declaration at the top of the file for this, though I could just be blind
also needs a prefix for this func
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are blind. also, i omitted the prefix and used camelCase because this function is local and operates only on primitive types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ehhhhhh, it's still part of object transfer tbh....
src/obj_char_transfer.c
Outdated
} | ||
} | ||
|
||
static void reserveTransferBlocksByVramOffsetAndSize(NNS_G2D_VRAM_TYPE vram, u32 offsetMain, u32 offsetSub, u32 sizeMain, u32 sizeSub) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefix, declaration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i say leave as-is, since it's a static function that does not act on any of this file's struct types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ehh that is a fair point, however it is associated with ObjCharTransfer so imo it should have that prefix, even if it does not act on the struct itself
break; | ||
case GX_VRAM_OBJ_16_F: | ||
case GX_VRAM_OBJ_16_G: | ||
sObjCharTransferTasksManager->vramCapacityMain = 16 * 1024; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest some constants here as it'd make it cleaner, but I don't actually know what to suggest
src/obj_char_transfer.c
Outdated
*bitIndex = arrayBitIndex & 7; | ||
} | ||
|
||
static void boundsFixOffsetAndSize(u32 baseOffset, u32 curOffset, u32 size, int *correctedOffset, int *correctedSize) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefix, declaration
the "Obj" refers to character data destined for object memory rather than background memory. |
tbh I kinda have to trust your judgement with obj vs vram, this is one area that I lack quite a bit of knowledge on, and well, AFAIK it's not exactly well documented, so... |
build failed |
include/constants/global.h
Outdated
@@ -13,6 +13,15 @@ | |||
|
|||
#define PARTY_SIZE 6 | |||
|
|||
// alias | |||
#ifndef PM_ASM | |||
// NNS_G2D_VRAM_TYPE_3DMAIN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should still not be in global.h, needs to be moved out of here
src/obj_char_transfer.c
Outdated
} *sObjCharTransferTasksManager; | ||
|
||
static BOOL ObjCharTransfer_TaskExistsByID(int resId); | ||
static void resetAllTransferTasks(void); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still leaning towards prefix for this, I can understand why you may not want to have it though
src/obj_char_transfer.c
Outdated
} | ||
} | ||
|
||
static BOOL getBlockNumAndFreeSpaceForTransfer(int vram, u32 *blockNumMain, u32 *blockNumSub, u32 size, u32 *freeSpaceMain, u32 *freeSpaceSub) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ehhhhhh, it's still part of object transfer tbh....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
follows from #302