Library/Camera: Implement CameraPoserFunction - #1303
Conversation
a4daade to
96db104
Compare
96db104 to
57b99c6
Compare
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 reviewed 21 files and all commit messages, and made 9 comments.
Reviewable status: 21 of 25 files reviewed, 9 unresolved discussions (waiting on german77).
lib/al/Library/Camera/CameraArrowCollider.h line 37 at r1 (raw file):
char mBuffer[0x474]; bool _48c; char mBuffer2[0x3];
offset-based names for unknown variables
Suggestion:
char _XY[0x474];
bool _48c;
char _XY[0x3];lib/al/Library/Camera/CameraPoserFunction.cpp line 451 at r1 (raw file):
f32* tryGetEquipmentDistanceCurve(const al::CameraPoser* cameraPoser) { return cameraPoser->getSceneInfo()->requestParamHolder->get_58(); }
Suggestion:
al::CameraDistanceCurve* tryGetBossDistanceCurve(const al::CameraPoser* cameraPoser) {
return cameraPoser->getSceneInfo()->requestParamHolder->getBossDistanceCurve();
}
al::CameraDistanceCurve* tryGetEquipmentDistanceCurve(const al::CameraPoser* cameraPoser) {
return cameraPoser->getSceneInfo()->requestParamHolder->getEquipmentDistanceCurve();
}lib/al/Library/Camera/CameraRequestParamHolder.h line 40 at r1 (raw file):
f32* get_60() const { return _60; } f32* get_58() const { return _58; }
change accordingly
Code quote:
f32* get_60() const { return _60; }
f32* get_58() const { return _58; }lib/al/Library/Camera/CameraRequestParamHolder.h line 63 at r1 (raw file):
const IUseCamera* mRideObjCamera = nullptr; f32* _58 = nullptr; f32* _60 = nullptr;
change accordingly
Code quote:
f32* _58 = nullptr;
f32* _60 = nullptr;lib/al/Library/Camera/CameraTargetCollideInfoHolder.h at r1 (raw file):
offset-based names for unknown variables
lib/al/Library/Camera/CameraTargetHolder.h line 20 at r1 (raw file):
struct ViewSubTargetInfo { CameraSubTargetBase* target = nullptr; // Note: s8 is used instead of bool to match isChangeSubTarget
Suggestion:
// NOTE: s8 is used instead of bool to match isChangeSubTargetlib/al/Library/Camera/SnapShotCameraCtrl.h at r1 (raw file):
Remove ; after definition of setters
lib/al/Library/Camera/SnapShotCameraCtrl.h line 35 at r1 (raw file):
f32 getFovyDegree() const { return mFovyDegree; }; AudioKeeper* getAudioKeeper() const override;
Suggestion:
void exeReset();
AudioKeeper* getAudioKeeper() const override;
f32 getFovyDegree() const { return mFovyDegree; };lib/al/Library/Obj/CameraRailHolder.h line 15 at r1 (raw file):
s32 getRailCount() const { return mCameraRailCount; } CameraLimitRailKeeper* getRail(s32 index) { return mCameraRails[index]; }
Suggestion:
CameraLimitRailKeeper* getRail(s32 index) const { return mCameraRails[index]; }c2afc02 to
7e736b0
Compare
german77
left a comment
There was a problem hiding this comment.
@german77 made 9 comments.
Reviewable status: 18 of 25 files reviewed, 9 unresolved discussions (waiting on MonsterDruide1).
lib/al/Library/Camera/CameraArrowCollider.h line 37 at r1 (raw file):
Previously, MonsterDruide1 wrote…
offset-based names for unknown variables
Done.
lib/al/Library/Camera/CameraRequestParamHolder.h line 40 at r1 (raw file):
Previously, MonsterDruide1 wrote…
change accordingly
Done.
lib/al/Library/Camera/CameraRequestParamHolder.h line 63 at r1 (raw file):
Previously, MonsterDruide1 wrote…
change accordingly
Done.
lib/al/Library/Camera/CameraTargetCollideInfoHolder.h at r1 (raw file):
Previously, MonsterDruide1 wrote…
offset-based names for unknown variables
Done.
lib/al/Library/Camera/SnapShotCameraCtrl.h at r1 (raw file):
Previously, MonsterDruide1 wrote…
Remove
;after definition of setters
Done.
lib/al/Library/Camera/CameraPoserFunction.cpp line 451 at r1 (raw file):
f32* tryGetEquipmentDistanceCurve(const al::CameraPoser* cameraPoser) { return cameraPoser->getSceneInfo()->requestParamHolder->get_58(); }
Done.
lib/al/Library/Camera/CameraTargetHolder.h line 20 at r1 (raw file):
struct ViewSubTargetInfo { CameraSubTargetBase* target = nullptr; // Note: s8 is used instead of bool to match isChangeSubTarget
Done.
lib/al/Library/Camera/SnapShotCameraCtrl.h line 35 at r1 (raw file):
f32 getFovyDegree() const { return mFovyDegree; }; AudioKeeper* getAudioKeeper() const override;
Done.
lib/al/Library/Obj/CameraRailHolder.h line 15 at r1 (raw file):
s32 getRailCount() const { return mCameraRailCount; } CameraLimitRailKeeper* getRail(s32 index) { return mCameraRails[index]; }
Done.
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 reviewed 6 files and all commit messages, made 3 comments, and resolved 6 discussions.
Reviewable status: 23 of 25 files reviewed, 3 unresolved discussions (waiting on german77).
lib/al/Library/Camera/CameraPoserFunction.cpp line 451 at r1 (raw file):
Previously, german77 (Narr the Reg) wrote…
Done.
Also change return type
lib/al/Library/Camera/CameraRequestParamHolder.h line 40 at r1 (raw file):
Previously, german77 (Narr the Reg) wrote…
Done.
also change return type
lib/al/Library/Camera/CameraRequestParamHolder.h line 63 at r1 (raw file):
Previously, german77 (Narr the Reg) wrote…
Done.
also change member type
german77
left a comment
There was a problem hiding this comment.
@german77 made 3 comments.
Reviewable status: 22 of 25 files reviewed, 3 unresolved discussions (waiting on MonsterDruide1).
lib/al/Library/Camera/CameraPoserFunction.cpp line 451 at r1 (raw file):
Previously, MonsterDruide1 wrote…
Also change return type
Done.
lib/al/Library/Camera/CameraRequestParamHolder.h line 40 at r1 (raw file):
Previously, MonsterDruide1 wrote…
also change return type
Done.
lib/al/Library/Camera/CameraRequestParamHolder.h line 63 at r1 (raw file):
Previously, MonsterDruide1 wrote…
also change member type
Done.
259b995 to
d07faee
Compare
MonsterDruide1
left a comment
There was a problem hiding this comment.
I'm impressed that you pushed through and implemented so many of these boring small functions. I'll need at least one, probably two more passes to work through them.
@MonsterDruide1 reviewed 2 files and all commit messages, made 11 comments, and resolved 3 discussions.
Reviewable status: 24 of 25 files reviewed, 10 unresolved discussions (waiting on german77).
lib/al/Library/Camera/CameraPoser.h line 224 at r3 (raw file):
CameraViewInfo* getViewInfo() const { return mViewInfo; } CameraViewInfo* getCameraViewInfo() const { return mViewInfo; }
Suggestion:
CameraViewInfo* getViewInfo() const { return mViewInfo; }lib/al/Library/Camera/CameraPoserFunction.cpp line 48 at r3 (raw file):
static sead::Vector3f sMtxX = {-1.0f, 0.0f, 0.0f}; static sead::Vector3f sMtxY = {0.0f, 1.0f, 0.0f}; static sead::Vector3f sMtxZ = {0.0f, 0.0f, -1.0f};
TODO: check these
Code quote:
static al::CameraCollisionPartsFilter sPartsFiler;
static al::CameraTriangleFilter sTriangleFilter;
static al::CameraTriangleFilterOnlyCeiling sCeilFilter;
static sead::Vector3f sMtxX = {-1.0f, 0.0f, 0.0f};
static sead::Vector3f sMtxY = {0.0f, 1.0f, 0.0f};
static sead::Vector3f sMtxZ = {0.0f, 0.0f, -1.0f};lib/al/Library/Camera/CameraPoserFunction.cpp line 52 at r3 (raw file):
static inline s32 getViewInfoIndex(const al::CameraPoser* cameraPoser) { return cameraPoser->getViewInfo()->getIndex(); }
exists already (getViewIndex)
lib/al/Library/Camera/CameraPoserFunction.cpp line 164 at r3 (raw file):
if (cameraStartInfo.preCameraName) return al::isEqualString(cameraName, cameraStartInfo.preCameraName); return false;
Suggestion:
if (!cameraStartInfo.preCameraName)
return false;
return al::isEqualString(cameraName, cameraStartInfo.preCameraName);lib/al/Library/Camera/CameraPoserFunction.cpp line 266 at r3 (raw file):
sead::Vector3f facingDir = cameraPoser->getEye() - cameraPoser->getAt(); al::normalize(&facingDir); outDir->set(facingDir.cross(cameraPoser->getUp()));
Suggestion:
outDir->setCross(facingDir, cameraPoser->getUp());lib/al/Library/Camera/CameraPoserFunction.cpp line 371 at r3 (raw file):
return speedV; else return -speedV;
probably mismatches, but worth a try
Suggestion:
return sead::Mathf::sign(direction) * speedV;lib/al/Library/Camera/CameraPoserFunction.cpp line 491 at r3 (raw file):
} bool checkValidTurnToSubTarget(const al::CameraPoser* cameraPoser) {
TODO: check this function
lib/al/Library/Camera/CameraPoserFunction.cpp line 560 at r3 (raw file):
} bool tryCalcSubTargetTurnBrakeDistanceRate(f32* outDistanceRate,
TODO: check this
lib/al/Library/Camera/CameraPoserFunction.cpp line 613 at r3 (raw file):
void initCameraVerticalAbsorberNoCameraPosAbsorb(al::CameraPoser* cameraPoser) { cameraPoser->setVerticalAbsorber(new al::CameraVerticalAbsorber(cameraPoser, true)); }
Please rename the parameter in CameraVerticalAbsorber accordingly (isNoCameraPosAbsorb)
Code quote:
void initCameraVerticalAbsorber(al::CameraPoser* cameraPoser) {
cameraPoser->setVerticalAbsorber(new al::CameraVerticalAbsorber(cameraPoser, false));
}
void initCameraVerticalAbsorberNoCameraPosAbsorb(al::CameraPoser* cameraPoser) {
cameraPoser->setVerticalAbsorber(new al::CameraVerticalAbsorber(cameraPoser, true));
}lib/al/Library/Camera/CameraPoserFunction.cpp line 808 at r3 (raw file):
return cameraPoser->getFlagCtrl()->isSnapShotModeRunning; }
TODO: continue below
d07faee to
448f2e5
Compare
german77
left a comment
There was a problem hiding this comment.
Credit to muz he did most of them. I mostly cleaned up his work and finished the remaining ones.
@german77 made 7 comments.
Reviewable status: 24 of 25 files reviewed, 10 unresolved discussions (waiting on MonsterDruide1).
lib/al/Library/Camera/CameraPoserFunction.cpp line 52 at r3 (raw file):
Previously, MonsterDruide1 wrote…
exists already (
getViewIndex)
Done.
lib/al/Library/Camera/CameraPoserFunction.cpp line 371 at r3 (raw file):
Previously, MonsterDruide1 wrote…
probably mismatches, but worth a try
Doesn't match. Loading order changes.
lib/al/Library/Camera/CameraPoserFunction.cpp line 613 at r3 (raw file):
Previously, MonsterDruide1 wrote…
Please rename the parameter in
CameraVerticalAbsorberaccordingly (isNoCameraPosAbsorb)
Done.
lib/al/Library/Camera/CameraPoser.h line 224 at r3 (raw file):
CameraViewInfo* getViewInfo() const { return mViewInfo; } CameraViewInfo* getCameraViewInfo() const { return mViewInfo; }
Done.
lib/al/Library/Camera/CameraPoserFunction.cpp line 164 at r3 (raw file):
if (cameraStartInfo.preCameraName) return al::isEqualString(cameraName, cameraStartInfo.preCameraName); return false;
Done.
lib/al/Library/Camera/CameraPoserFunction.cpp line 266 at r3 (raw file):
sead::Vector3f facingDir = cameraPoser->getEye() - cameraPoser->getAt(); al::normalize(&facingDir); outDir->set(facingDir.cross(cameraPoser->getUp()));
Done.
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 reviewed 4 files and all commit messages, made 6 comments, and resolved 7 discussions.
Reviewable status: 24 of 26 files reviewed, 8 unresolved discussions (waiting on german77).
lib/al/Library/Camera/CameraPoser.h line 238 at r4 (raw file):
GyroCameraCtrl* getGyroCtrl() const { return mGyroCtrl; } CameraPoserFlag* getPoserFag() const { return mPoserFlag; }
No swear words, please!
Suggestion:
CameraPoserFlag* getPoserFlag() const { return mPoserFlag; }lib/al/Library/Camera/CameraPoserFunction.cpp line 43 at r4 (raw file):
namespace alCameraPoserFunction { static al::CameraCollisionPartsFilter sPartsFiler;
Suggestion:
static al::CameraCollisionPartsFilter sPartsFilter;lib/al/Library/Camera/CameraPoserFunction.cpp line 969 at r4 (raw file):
bool isPause(const al::CameraPoser* cameraPoser) { return false; }
Oh wow, useful.
Code quote:
bool isPause(const al::CameraPoser* cameraPoser) {
return false;
}lib/al/Library/Camera/CameraPoserFunction.cpp line 1005 at r4 (raw file):
type = 1; outResult->type = type;
Turn this into an enum? I'm not sure if there might be one with these values already...
Code quote:
s32 type = 2;
if (hitInfo->hitInfo->isCollisionAtFace())
type = 0;
else if (hitInfo->hitInfo->isCollisionAtEdge())
type = 1;
outResult->type = type;lib/al/Library/Camera/CameraPoserFunction.cpp line 1080 at r4 (raw file):
return true; } return false;
Suggestion:
if (!calcOffsetCameraKeepInFrameV(&offset, camera, vec, cameraPoser, a, b)) {
return false;
}
camera->setAt(camera->getAt() + offset);
camera->setPos(camera->getPos() + offset);
return true;lib/al/Library/Camera/CameraPoserFunction.cpp line 1106 at r4 (raw file):
} al::CameraLimitRailKeeper* tryFindNearestLimitRailKeeper(const al::CameraPoser* cameraPoser,
TODO: check this function
448f2e5 to
3c6ec48
Compare
german77
left a comment
There was a problem hiding this comment.
@german77 made 4 comments.
Reviewable status: 22 of 26 files reviewed, 8 unresolved discussions (waiting on MonsterDruide1).
lib/al/Library/Camera/CameraPoser.h line 238 at r4 (raw file):
Previously, MonsterDruide1 wrote…
No swear words, please!
Done. Whoops my evil plan has failed.
lib/al/Library/Camera/CameraPoserFunction.cpp line 1005 at r4 (raw file):
Previously, MonsterDruide1 wrote…
Turn this into an enum? I'm not sure if there might be one with these values already...
Done. Seems similar to CollisionLocation but the enum doesn't line up
lib/al/Library/Camera/CameraPoserFunction.cpp line 43 at r4 (raw file):
namespace alCameraPoserFunction { static al::CameraCollisionPartsFilter sPartsFiler;
Done.
lib/al/Library/Camera/CameraPoserFunction.cpp line 1080 at r4 (raw file):
return true; } return false;
Done.
Initially developed by muz I finished the class with the exception of two functions
checkCameraCollisionMoveSphereandcalcOffsetCameraKeepInFrameVthose had very severe mismatches.The main motivation for this one is that the header return types were incorrect. Leading to a few issues when implementing other camera related classes. This also corrected multiple issues in other headers as well resulting in a quite large PR overall.
This change is
Report for 1.0 (441aec9 - 3c6ec48)
📈 Matched code: 15.59% (+0.09%, +11580 bytes)
✅ 213 new matches
Library/Camera/CameraPoserFunctionalCameraPoserFunction::checkValidTurnToSubTarget(al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::tryFindNearestLimitRailKeeper(al::CameraPoser const*, sead::Vector3<float> const&)Library/Camera/CameraPoserFunctionalCameraPoserFunction::tryCalcSubTargetTurnBrakeDistanceRate(float*, al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcTargetJumpSpeed(al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcTargetFallSpeed(al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcTargetSpeedV(al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::checkFirstCameraCollisionArrow(sead::Vector3<float>*, sead::Vector3<float>*, al::IUseCollision const*, sead::Vector3<float> const&, sead::Vector3<float> const&)Library/Camera/CameraPoserFunctionalCameraPoserFunction::multVecInvZone(sead::Vector3<float>*, sead::Vector3<float> const&, al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::checkFirstCameraCollisionArrowOnlyCeiling(sead::Vector3<float>*, sead::Vector3<float>*, al::IUseCollision const*, sead::Vector3<float> const&, sead::Vector3<float> const&)Library/Camera/CameraPoserFunctionalCameraPoserFunction::updateSnapShotCameraCtrl(al::CameraPoser*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcTargetSpeedH(al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcPreCameraAngleV(al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::checkFirstCameraCollisionArrow(alCameraPoserFunction::CameraCollisionHitResult*, al::IUseCollision const*, sead::Vector3<float> const&, sead::Vector3<float> const&)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcZoneInvRotateAngleH(float, sead::Matrix34<float> const&)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcCameraRotateStickPower(al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcSideDir(sead::Vector3<float>*, al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcTargetVelocityH(sead::Vector3<float>*, al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcCameraPose(sead::Quat<float>*, al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcZoneRotateAngleH(float, al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcZoneRotateAngleH(float, sead::Matrix34<float> const&)Library/Camera/CameraPoserFunctionalCameraPoserFunction::makeCameraKeepInFrameV(sead::LookAtCamera*, sead::Vector3<float> const&, al::CameraPoser const*, float, float)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcTargetPose(sead::Quat<float>*, al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcPreCameraAngleH(al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcTargetTransWithOffset(sead::Vector3<float>*, al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcLookDirH(sead::Vector3<float>*, al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcCameraRotateStick(sead::Vector2<float>*, al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::calcCameraRotateStickH(al::CameraPoser const*)Library/Camera/CameraPoserFunctionalCameraPoserFunction::setLookAtPosToTargetAddOffset(al::CameraPoser*, sead::Vector3<float> const&)Library/Camera/CameraPoserFunctionalCameraPoserFunction::setCameraPosToTargetAddOffset(al::CameraPoser*, sead::Vector3<float> const&)Library/Camera/CameraPoserFunctionalCameraPoserFunction::multVecZone(sead::Vector3<float>*, sead::Vector3<float> const&, al::CameraPoser const*)...and 183 more new matches