-
Notifications
You must be signed in to change notification settings - Fork 36
iModel work #579
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
iModel work #579
Conversation
🆗 ✅+68 bytes No Regressions 🎉Progress: 5📈 |
|
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.
Good work! Left some review comments
RpLight* sEmptyDirectionalLight; | ||
RpLight* sEmptyAmbientLight; |
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.
These need to be made static per objdiff.
} | ||
if (clump != 0) | ||
{ | ||
RpClumpDestroy(clump); | ||
} | ||
} | ||
|
||
|
||
RpAtomic* NextAtomicCallback(RpAtomic* atomic, void* data) |
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 function should be static as per objdiff.
for (S32 i = 0; i < 4; i++) | ||
{ | ||
frame = (RwFrame*)RpLightCreate(i); | ||
//black = frame; |
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.
Please remember to take out commented code - it adds unnecessary noise when trying to debug functions to increase the match.
} | ||
else | ||
{ | ||
//modelArray[0] = (S32)model; |
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.
Please remember to take out commented code - it adds unnecessary noise when trying to debug functions to increase the match.
// What? | ||
|
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.
Could you clarify this comment? Is there something here that you felt might be particularly confusing for future contributors?
Not proud of the work done here so far. It's a lot of guessing even with the dwarf data
TLDR; Skill issue