Decompiled search_level_by_id and fixed a symbol metadata issue that was preventing a matching rebuild. - #267
Conversation
Correct g_note's size from 0x280 to 0x278 to match the actual symbol boundary. This restores a checksum-clean build by ensuring the symbol metadata matches the original binary.
Replace the INCLUDE_ASM stub with a byte-matching C implementation reverse engineered from the original assembly.
Add the search_level_by_id declaration to include/game.h while keeping LevelLoadData forward-declared.
TheOnlyZac
left a comment
TheOnlyZac
left a comment
There was a problem hiding this comment.
Thank you for the PR! I would just ask you to make a few tweaks, please let me know if you run into any issues.
| #ifdef __cplusplus | ||
| extern "C" { | ||
| #endif | ||
|
|
||
| LevelLoadData *search_level_by_id(int search_id); | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif |
There was a problem hiding this comment.
This function should be C++ so you can delete everything here except the function signature.
There was a problem hiding this comment.
The function name appears to mangle when I try to run it without stating to use C linkage. Condensed the code to take up less space.
| { | ||
| // ... | ||
| }; | ||
| struct LevelLoadData; |
There was a problem hiding this comment.
Please re-add the braces, even though the struct is empty, to indicate that the struct eventually needs to be defined here.
There was a problem hiding this comment.
I had originally defined this struct in the src/P2/game.c file. I moved the definition to the header file.
| uint unk_18; | ||
| uint unk_1C; | ||
| int level_id; | ||
| const char *friendly; |
There was a problem hiding this comment.
Rename to achzFriendly to adhere to style guide.
There was a problem hiding this comment.
Renamed to pchzFriendly. Used 'p' instead of 'a' to indicate pointer instead of array.
| loop: | ||
| if (search_id != level->level_id) | ||
| { | ||
| level++; | ||
|
|
||
| if (level < end) | ||
| { | ||
| goto loop; | ||
| } | ||
|
|
||
| return NULL; | ||
| } |
There was a problem hiding this comment.
Maybe this could be done with a while loop? It's unlikely that they used goto in the original code.
There was a problem hiding this comment.
I originally tried, and retested today, rewriting the function using more common loop constructs, such as a while loop, but was only able to produce a matching build using goto. Yes, I agree - odd...
There was a problem hiding this comment.
I managed to get a full match with a for loop. Do a for loop for each LevelLoadData then in the body get a pointer to the current data. Return that pointer if the id matches. If the end of the function is reached return NULL.
LevelLoadData *search_level_by_id(int search_id)
{
for (uint i = 0; i < sizeof(D_00247AF0) / sizeof(LevelLoadData); i++)
{
LevelLoadData *level = &D_00247AF0[i];
if (search_id == level->level_id)
{
return level;
}
}
return NULL;
}Move the full LevelLoadData struct definition from game.c into game.h so the type and its fields are defined in the header. Rename the friendly string field to pchzFriendly for project naming consistency. Replace the multi-line extern "C" wrapper for search_level_by_id with a single-line declaration to reduce header space while preserving C linkage and avoiding C++ name mangling. Keep the goto-based implementation after verifying equivalent while-loop rewrites do not produce a matching build. Move the search_level_by_id documentation to game.h alongside its declaration and remove the duplicate comment from game.c.
| * @param search_id Level ID to search for. | ||
| * @return Pointer to the matching LevelLoadData, or NULL if no match is found. | ||
| */ | ||
| extern "C" LevelLoadData *search_level_by_id(int search_id); |
There was a problem hiding this comment.
You need to mangle the name instead of treating it as C code. Take a look at the config/symbol_addrs.txt file and find the entry for search_level_by_id. Change it to search_level_by_id__Fi and then remove the extern "C" from the declaration.
3 participants
src/P2/game.c: Replaced the INCLUDE_ASM stub for search_level_by_id with a byte-matching C implementation reverse engineered from the original assembly.
include/game.h: Add the search_level_by_id declaration to include/game.h while keeping LevelLoadData forward-declared.
config/symbol_addrs.txt: Corrected g_note's size from 0x280 to 0x278 to match the actual symbol boundary and restore a checksum-clean build.