Some nz1 duplicate functions#3102
Conversation
hohle
left a comment
hohle
left a comment
There was a problem hiding this comment.
Welcome and thanks for the contribution!
There are some minor things with this that could be improved and I've pointed out, but there's some higher level concerns that may be an issue as well and I'll let others chime in.
There are two types of duplicates that we have to work through - exact duplicates that usually just need source and symbol mapping and then the near-duplicates that are almost the same source, but differ slightly for some reason or another.
For the exact duplicates that can already share source, we generally #include the shared file. The near duplicates are a bit more challenging, and some the duplicates that you've chosen don't have shared source files yet. On the one hand duplicates is a great way to learn and become familiar with decompiling and the project structure, on the other hand, the work is likely to be replaced shortly.
I'd probably lean towards new contributors getting to know their way around, but I don't know what direction others will want to go.
There was a problem hiding this comment.
This looks identical to the version in src/st/e_red_door.h.
The "interesting" function in this file is NZ1_EntityRedDoor, which is only a partial match with the other EntityRedoDoor implementations.
There was a problem hiding this comment.
Just on this - if you're interested in seeing how including the existing shared header is done I did this recently when I imported the RCAT overlay
34beb11
(among other deduplication via shared header, see commits)
If you would like some more guidance on this feel free to reach out on Discord!
| extern u16 PLAYER_posX_i_hi; | ||
| extern u16 PLAYER_posY_i_hi; |
| INCLUDE_ASM("st/nz1/nonmatchings/e_skull_lord", EntitySkullLordFlames); | ||
|
|
||
| INCLUDE_ASM("st/nz1/nonmatchings/e_skull_lord", EntitySkullLordPieces); | ||
| extern EInit g_EInitSkullLord; |
| extern int g_Timer; | ||
| extern Entity* g_CurrentEntity; |
There was a problem hiding this comment.
These will come from game.h (or transitively through stage.h)
| INCLUDE_ASM("st/nz1/nonmatchings/e_vandal_sword", VandalSwordDrawAlastor); | ||
|
|
||
| INCLUDE_ASM("st/nz1/nonmatchings/e_vandal_sword", VandalSwordTargetPlayer); | ||
| extern Entity g_Entities_224; |
There was a problem hiding this comment.
This is a shorthand symbol for the array entry g_Entities[224]. We only use it to map a symbol for disassembly. It's not a real symbol in C or in the final compilation unit.
| if (g_Timer & 7) | ||
| goto done; |
There was a problem hiding this comment.
m2c will use gotos because it doesn't know any better. In practice, it's pretty rare that they are required for a match and in most cases they won't match between different versions of the game. For these you can return directly from the block.
| temp_v0 = AllocEntity( | ||
| &g_Entities_224, (Entity*)((char*)&g_Entities_224 + 0x1780)); |
There was a problem hiding this comment.
temp_v0 = AllocEntity(&g_Entities[224], &g_Entities[256]);
| extern EInit g_EInitCommon; | ||
| extern s16 (*g_api_func_800EDB58)(s32 kind, s32 count); |
There was a problem hiding this comment.
Like g_Entities_224 this is a shorthand for g_api.func_800EDB58. The declaration isn't necessary (again, comes from game.h).
Seeing as it is likely to be replaced soon, would it be interesting for me to review the comments and apply the corrections in this branch or go in another direction in order to contribute? |
Hey, sorry for the late review. I verified the work that has been merged in NZ1 shortly after your contribution. The only file that got decompiled is Everything else is still very relevant, and suggested changes need to be applied before this is mergeable. |
4 participants
Inserts following matching nz1 overlay function declarations:
EntityIsNearPlayer
EntitySkullLordPieces
VandalSwordTargetPlayer
VandalSwordTrail
EntityWaterDrop