daMagLift_c work #2275
daMagLift_c work #2275
Conversation
include/d/actor/d_a_obj_magLift.h
Outdated
| /* 0x5ae */ u8 field_0x5ae; | ||
| /* 0x5af */ s8 field_0x5af; | ||
| /* 0x5b0 */ s16 mPoint; | ||
| // two bytes of padding. |
There was a problem hiding this comment.
comments about padding aren't that necessary
include/d/actor/d_a_obj_magLift.h
Outdated
| // two bytes of padding. | ||
| /* 0x5b4 */ cXyz field_0x5b4; | ||
| /* 0x5c0 */ cXyz field_0x5c0; | ||
| /* 0x5cc */ float mMoveSpeed; |
There was a problem hiding this comment.
f32 is preferred for float types in game code
include/d/actor/d_a_obj_magLift.h
Outdated
| /* 0x5cc */ float mMoveSpeed; | ||
| /* 0x5d0 */ u8 mMode; | ||
| /* 0x5d1 */ u8 is_switch; | ||
| // The rest is word padding. |
There was a problem hiding this comment.
same thing about comment being unnecessary
src/d/actor/d_a_obj_magLift.cpp
Outdated
| PSMTXTrans(mDoMtx_stack_c::get(), current.pos.x, current.pos.y, current.pos.z); | ||
| mDoMtx_ZXYrotM(mDoMtx_stack_c::get(), current.angle.x, current.angle.y, current.angle.z); | ||
| mpModel->mBaseScale = scale; | ||
| PSMTXCopy(mDoMtx_stack_c::get(), mpModel->mBaseTransformMtx); |
There was a problem hiding this comment.
this can probably be cleaned up with multiple inlines. I'd recommend for all functions in this TU and all future work referencing the debug rom for what inlines are used to help simplify the code
There was a problem hiding this comment.
Also drop the PS prefix -- we have a macro for most of these functions for better debug rom compatibility.
There was a problem hiding this comment.
- I accomplished hatal's comment in a local update.
- I changed mBaseScale to "setBaseScale()".
- When I try to change mpModel->mBaseTransformMtx to mpModel->getModelData(), that breaks code matching.
src/d/actor/d_a_obj_magLift.cpp
Outdated
| /* 80C8DE98-80C8DF20 000558 0088+00 1/0 0/0 0/0 .text modeAcc__11daMagLift_cFv */ | ||
| void daMagLift_c::modeAcc() { | ||
| // NONMATCHING | ||
| if (1 == cLib_chaseF(&speedF, mMoveSpeed, mMoveSpeed / 30.0f) || current.pos == field_0x5b4) { |
There was a problem hiding this comment.
this would look cleaner as cLib_chaseF() == TRUE or something
| MTXTrans(mDoMtx_stack_c::get(), current.pos.x, current.pos.y, current.pos.z); | ||
| mDoMtx_ZXYrotM(mDoMtx_stack_c::get(), current.angle.x, current.angle.y, current.angle.z); | ||
| mpModel->setBaseScale(scale); | ||
| MTXCopy(mDoMtx_stack_c::get(), mpModel->mBaseTransformMtx); |
There was a problem hiding this comment.
this should probably be
mDoMtx_stack_c::transS(current.pos.x, current.pos.y, current.pos.z);
mDoMtx_stack_c::ZXYrotM(current.angle.x, current.angle.y, current.angle.z);
mpModel->setBaseScale(scale);
mpModel->setBaseTRMtx(mDoMtx_stack_c::get());| } | ||
| field_0x5ae = fopAcM_GetParam(this); | ||
| dPath* path = dPath_GetRoomPath(field_0x5ae, fopAcM_GetRoomNo(this)); | ||
| if (!path) { |
There was a problem hiding this comment.
this isnt a major blocker or anything, personally i like explicit NULL pointer checks (path == NULL) more since its less ambiguous when reading.
| init_modeMoveWait(); | ||
| } | ||
| } | ||
| cullMtx = mpModel->getBaseTRMtx(); |
There was a problem hiding this comment.
fopAcM_setCullSizeBox2here
| } | ||
|
|
||
| /* 80C8E2C4-80C8E318 000984 0054+00 1/0 0/0 0/0 .text modeMoveWait__11daMagLift_cFv */ | ||
| void daMagLift_c::modeMoveWait() { | ||
| // NONMATCHING | ||
| if (fopAcM_isSwitch(this, fopAcM_GetParamBit(this, 0xc, 8)) & 0xff) { |
There was a problem hiding this comment.
pretty sure this is supposed to be fopAcM_GetParam rather than GetParamBit
The file is matching, however I still have some cleanup to do.
Most notably, I haven't named the cXyz fields, though those names should be guessable.
This is my first stab at decompilation, so please be gentle XD
Also, I forgot to remove "NONMATCHING" from many functions as of my 2nd commit.