Library/Camera: Implement CameraUtil - #1122
Conversation
cf5ef9e to
577c5f0
Compare
CameraUtil
MonsterDruide1
left a comment
There was a problem hiding this comment.
reviewed most stuff alphabetically before CameraUtil.
blocked by #1073 for the second half.
@MonsterDruide1 reviewed 12 files and all commit messages, and made 12 comments.
Reviewable status: 12 of 36 files reviewed, 11 unresolved discussions (waiting on guymakinggames).
lib/al/Library/Camera/CameraDistanceCurve.h line 21 at r1 (raw file):
f32 calcDistance(f32 rate) const; };
Suggestion:
private:
const char* mName;
const CameraDistanceAtPoint* mPoints;
s32 mPointNum;
};lib/al/Library/Camera/CameraFlagCtrl.h line 14 at r1 (raw file):
bool isValidCameraAreaKids = false; bool isSeparatePlayMode = false; bool _6 = false;
seems good enough to add a name
Suggestion:
bool isInvalidEndEntraceCamera = false;lib/al/Library/Camera/CameraFlagCtrl.h line 18 at r1 (raw file):
void invalidateEndEntranceCamera() { _6 = true; } void validateEndEntranceCamera() { _6 = false; }
modify _6 directly at the usages
lib/al/Library/Camera/CameraPoserAnim.h line 39 at r1 (raw file):
void setRotateBaseUp() { _1ad = true; } void setBaseMtxPtr(const sead::Matrix34f* baseMtxPtr) { _1a0 = baseMtxPtr; }
Suggestion:
void setRotateBaseUp() { mIsRotateBaseUp = true; }
void setBaseMtxPtr(const sead::Matrix34f* baseMtxPtr) { mBaseMtxPtr = baseMtxPtr; }lib/al/Library/Camera/CameraPoserFixActor.h line 17 at r1 (raw file):
void update() override; void setTargetActor(const LiveActor* actor) { mActor = actor; }
Suggestion:
void setTargetActor(const LiveActor* actor) { mTargetActor = actor; }lib/al/Library/Camera/CameraPoserFixActor.h line 29 at r1 (raw file):
void setAutoAroundFront() { mIsAutoAroundFront = true; } void setCalcNearestAtFromPreAt() { mIsCalcNearestAtFromPreAt = true; }
I'm okay with all others, but don't think that these two are explored enough to assign names yet. Please replace them with placeholders for now.
Code quote:
void setAutoAroundFront() { mIsAutoAroundFront = true; }
void setCalcNearestAtFromPreAt() { mIsCalcNearestAtFromPreAt = true; }lib/al/Library/Camera/CameraPoserFixActor.h line 49 at r1 (raw file):
void start(const CameraStartInfo& startInfo) override; void setTalkDistance(f32 distance) { mTalkDistance = distance; }
No, as far as I can tell that's connected to angleH in some way - which conflicts the current naming. Placeholder please.
Code quote:
void setTalkDistance(f32 distance) { mTalkDistance = distance; }lib/al/Library/Camera/CameraPoserFlag.h line 15 at r1 (raw file):
bool isValidKeepPreSelfPoseNextCamera() const; void validateResetPoseNextCamera() { _3 = true; }
Suggestion:
void validateResetPoseNextCamera() { isResetPoseNextCamera = true; }lib/al/Library/Camera/CameraPoserFlag.h line 19 at r1 (raw file):
void validateKeepPreSelfPoseNextCamera() { isOverWriteProgram = true; } void onForceCollideAtStartInterpole() { _c = true; }
Suggestion:
void onForceCollideAtStartInterpole() { isForceCollideAtStartInterpole = true; }lib/al/Library/Camera/CameraPoseUpdater.h line 0 at r1 (raw file):
Please order functions by address in the binary
lib/al/Library/Camera/CameraTargetHolder.h line 40 at r1 (raw file):
void removePlacementSubTarget(CameraSubTargetBase* subTarget); private:
Suggestion:
bool isChangeViewTarget(s32 index) const;
CameraSubTargetBase* getTopSubTarget() const;
void addSubTarget(CameraSubTargetBase* subTarget);
void removeSubTarget(CameraSubTargetBase* subTarget);
void addPlacementSubTarget(CameraSubTargetBase* subTarget);
void removePlacementSubTarget(CameraSubTargetBase* subTarget);
void setViewTarget(CameraTargetBase* target, s32 index) { mViewTargetArray[index] = target; }
private:
german77
left a comment
There was a problem hiding this comment.
@german77 made 13 comments.
Reviewable status: 12 of 36 files reviewed, 24 unresolved discussions (waiting on guymakinggames and MonsterDruide1).
lib/al/Library/Camera/CameraUtil.cpp line 215 at r1 (raw file):
sead::Vector3f diff = getCameraPos(user, viewIdx) - getCameraAt(user, viewIdx); return diff.length();
I know this isn't part of your changes but we might as well apply the same format to the full file
Suggestion:
return (getCameraPos(user, viewIdx) - getCameraAt(user, viewIdx)).length();lib/al/Library/Camera/CameraUtil.cpp line 221 at r1 (raw file):
f32 aspect = getSceneCameraInfo(user)->getViewAt(viewIdx)->getAspect(); return aspect * getFovyDegree(user, viewIdx);
Suggestion:
return getSceneCameraInfo(user)->getViewAt(viewIdx)->getAspect() * getFovyDegree(user, viewIdx);lib/al/Library/Camera/CameraUtil.cpp line 238 at r1 (raw file):
const sead::Vector3f& pos = getCameraPos(user, viewIdx); front->set(at - pos); normalize(front);
Suggestion:
front->set(getCameraAt(user, viewIdx) - getCameraPos(user, viewIdx));
normalize(front);lib/al/Library/Camera/CameraUtil.cpp line 257 at r1 (raw file):
const sead::Vector3f& cameraUp = getCameraUp(user, 0); poseInfo->cameraUp.set(cameraUp);
Suggestion:
poseInfo->cameraPos.set(getCameraPos(user, 0));
poseInfo->lookAtPos.set(getCameraAt(user, 0));
poseInfo->cameraUp.set(getCameraUp(user, 0));lib/al/Library/Camera/CameraUtil.cpp line 261 at r1 (raw file):
void calcCameraDir(sead::Vector3f* dir, const IUseCamera* user, s32 viewIdx) { const sead::Matrix34f& cameraMtx = getLookAtCamera(user, viewIdx).getMatrix();
Same for all instances of "getLookAtCamera(user, viewIdx).getMatrix();"
Suggestion:
const sead::Matrix34f& cameraMtx = getViewMtx(user, viewIdx);lib/al/Library/Camera/CameraUtil.cpp line 268 at r1 (raw file):
const sead::Matrix34f& cameraMtx = getLookAtCamera(user, viewIdx).getMatrix(); lookDir->set(-cameraMtx.m[2][0], -cameraMtx.m[2][1], cameraMtx.m[2][2]); lookDir->z = -lookDir->z;
Suggestion:
calcCameraDir(lookDir,user,viewIdx);
lookDir->negate();lib/al/Library/Camera/CameraUtil.cpp line 303 at r1 (raw file):
lookDirH->negate(); return true;
Suggestion:
if (!tryCalcCameraDirH(lookDirH,info,upDir,viewIdx))
return false;
lookDirH->negate();
return true;lib/al/Library/Camera/CameraUtil.cpp line 317 at r1 (raw file):
s32 updaterIdx) { ticket->getAnimPoser()->setAnim(animName, -1, -1, -1); getCameraSwitchRequester(user, 0)->requestStart(ticket, updaterIdx);
Suggestion:
startAnimCameraAnim(ticket, animName, -1, -1, -1);
startCamera(user, ticket, updaterIdx);lib/al/Library/Camera/CameraUtil.cpp line 329 at r1 (raw file):
s32 endStep, s32 playStep, s32 updaterIdx) { ticket->getAnimPoser()->setAnim(animName, startStep, endStep, playStep); getCameraSwitchRequester(user, 0)->requestStart(ticket, updaterIdx);
Suggestion:
startAnimCameraAnim(ticket, animName, startStep, endStep, playStep);
startCamera(user, ticket, updaterIdx);lib/al/Library/Camera/CameraUtil.cpp line 625 at r1 (raw file):
void startCameraShakeByAction(const LiveActor* actor, const char* shakeName, const char* actionName, s32 step, s32 viewIdx) { const IUseCamera* user = actor;
actor can be used directly
lib/al/Library/Camera/CameraUtil.cpp line 688 at r1 (raw file):
return getCameraDirector(user)->createObjectCamera(createPlacementId(placementInfo), cameraName, suffix, CameraTicket::Priority_Object, zoneMtx);
Suggestion:
return initObjectCamera(user, getPlacementInfo(actorInitInfo), cameraName, suffix);elib/al/Library/Camera/CameraUtil.cpp line 1151 at r1 (raw file):
extern "C" f32 _ZN2al30getSubjectiveCameraOffsetFrontEv(CameraPoserSubjective* poser) { return poser->getCameraOffsetFront(); }
This probably means getCameraOffsetFront is static.
Suggestion:
f32 getSubjectiveCameraOffsetFront() {
return CameraPoserSubjective::getCameraOffsetFront();
}lib/al/Library/Camera/CameraUtil.cpp line 1327 at r1 (raw file):
const IUseCamera* user = actor; actor->getName(); getCameraDirector(user)->getFlagCtrl()->invalidateEndEntranceCamera();
Suggestion:
invalidateEndEntranceCameraWithName(actor,actor->getName());577c5f0 to
c2747bb
Compare
guymakinggames
left a comment
There was a problem hiding this comment.
Rebased
@guymakinggames made 25 comments.
Reviewable status: 4 of 37 files reviewed, 24 unresolved discussions (waiting on german77 and MonsterDruide1).
lib/al/Library/Camera/CameraFlagCtrl.h line 14 at r1 (raw file):
Previously, MonsterDruide1 wrote…
seems good enough to add a name
Done.
lib/al/Library/Camera/CameraFlagCtrl.h line 18 at r1 (raw file):
Previously, MonsterDruide1 wrote…
modify
_6directly at the usages
Done.
lib/al/Library/Camera/CameraPoserFixActor.h line 29 at r1 (raw file):
Previously, MonsterDruide1 wrote…
I'm okay with all others, but don't think that these two are explored enough to assign names yet. Please replace them with placeholders for now.
Done.
lib/al/Library/Camera/CameraPoserFixActor.h line 49 at r1 (raw file):
Previously, MonsterDruide1 wrote…
No, as far as I can tell that's connected to
angleHin some way - which conflicts the current naming. Placeholder please.
Done.
lib/al/Library/Camera/CameraPoseUpdater.h line at r1 (raw file):
Previously, MonsterDruide1 wrote…
Please order functions by address in the binary
Done.
lib/al/Library/Camera/CameraUtil.cpp line 215 at r1 (raw file):
Previously, german77 (Narr the Reg) wrote…
I know this isn't part of your changes but we might as well apply the same format to the full file
Done.
lib/al/Library/Camera/CameraUtil.cpp line 261 at r1 (raw file):
Previously, german77 (Narr the Reg) wrote…
Same for all instances of "getLookAtCamera(user, viewIdx).getMatrix();"
Done.
lib/al/Library/Camera/CameraUtil.cpp line 625 at r1 (raw file):
Previously, german77 (Narr the Reg) wrote…
actor can be used directly
Done.
lib/al/Library/Camera/CameraUtil.cpp line 1151 at r1 (raw file):
Previously, german77 (Narr the Reg) wrote…
This probably means getCameraOffsetFront is static.
Done.
lib/al/Library/Camera/CameraDistanceCurve.h line 21 at r1 (raw file):
f32 calcDistance(f32 rate) const; };
Done.
lib/al/Library/Camera/CameraPoserAnim.h line 39 at r1 (raw file):
void setRotateBaseUp() { _1ad = true; } void setBaseMtxPtr(const sead::Matrix34f* baseMtxPtr) { _1a0 = baseMtxPtr; }
Done.
lib/al/Library/Camera/CameraPoserFixActor.h line 17 at r1 (raw file):
void update() override; void setTargetActor(const LiveActor* actor) { mActor = actor; }
Done.
lib/al/Library/Camera/CameraPoserFlag.h line 15 at r1 (raw file):
bool isValidKeepPreSelfPoseNextCamera() const; void validateResetPoseNextCamera() { _3 = true; }
Done.
lib/al/Library/Camera/CameraPoserFlag.h line 19 at r1 (raw file):
void validateKeepPreSelfPoseNextCamera() { isOverWriteProgram = true; } void onForceCollideAtStartInterpole() { _c = true; }
Done.
lib/al/Library/Camera/CameraTargetHolder.h line 40 at r1 (raw file):
void removePlacementSubTarget(CameraSubTargetBase* subTarget); private:
Done.
lib/al/Library/Camera/CameraUtil.cpp line 221 at r1 (raw file):
f32 aspect = getSceneCameraInfo(user)->getViewAt(viewIdx)->getAspect(); return aspect * getFovyDegree(user, viewIdx);
Done.
lib/al/Library/Camera/CameraUtil.cpp line 238 at r1 (raw file):
const sead::Vector3f& pos = getCameraPos(user, viewIdx); front->set(at - pos); normalize(front);
Done.
lib/al/Library/Camera/CameraUtil.cpp line 257 at r1 (raw file):
const sead::Vector3f& cameraUp = getCameraUp(user, 0); poseInfo->cameraUp.set(cameraUp);
Done.
lib/al/Library/Camera/CameraUtil.cpp line 268 at r1 (raw file):
const sead::Matrix34f& cameraMtx = getLookAtCamera(user, viewIdx).getMatrix(); lookDir->set(-cameraMtx.m[2][0], -cameraMtx.m[2][1], cameraMtx.m[2][2]); lookDir->z = -lookDir->z;
Done.
lib/al/Library/Camera/CameraUtil.cpp line 303 at r1 (raw file):
lookDirH->negate(); return true;
Done.
lib/al/Library/Camera/CameraUtil.cpp line 317 at r1 (raw file):
s32 updaterIdx) { ticket->getAnimPoser()->setAnim(animName, -1, -1, -1); getCameraSwitchRequester(user, 0)->requestStart(ticket, updaterIdx);
Done.
lib/al/Library/Camera/CameraUtil.cpp line 329 at r1 (raw file):
s32 endStep, s32 playStep, s32 updaterIdx) { ticket->getAnimPoser()->setAnim(animName, startStep, endStep, playStep); getCameraSwitchRequester(user, 0)->requestStart(ticket, updaterIdx);
Done.
lib/al/Library/Camera/CameraUtil.cpp line 688 at r1 (raw file):
return getCameraDirector(user)->createObjectCamera(createPlacementId(placementInfo), cameraName, suffix, CameraTicket::Priority_Object, zoneMtx);
Done.
lib/al/Library/Camera/CameraUtil.cpp line 1327 at r1 (raw file):
const IUseCamera* user = actor; actor->getName(); getCameraDirector(user)->getFlagCtrl()->invalidateEndEntranceCamera();
Done.
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 reviewed 16 files and all commit messages, made 4 comments, and resolved 9 discussions.
Reviewable status: 20 of 37 files reviewed, 17 unresolved discussions (waiting on german77 and guymakinggames).
lib/al/Library/Camera/CameraFlagCtrl.h line 18 at r1 (raw file):
Previously, guymakinggames wrote…
Done.
No, my point is "delete these two functions", as you can just do flags.isInvalidEndEntranceCamera = true; whenever required.
lib/al/Library/Camera/CameraPoserFixActor.h line 29 at r1 (raw file):
Previously, guymakinggames wrote…
Done.
also rename the setAutoAroundFront and setCalcNearestAtFromPreAt to set_160 and similar
lib/al/Library/Camera/CameraPoserFixActor.h line 49 at r2 (raw file):
void start(const CameraStartInfo& startInfo) override; void setTalkDistance(f32 distance) { _173 = distance; }
Suggestion:
void set_173(f32 distance) { _173 = distance; }lib/al/Library/Camera/CameraPoseUpdater.h line 40 at r2 (raw file):
void requestCancelInterpole(); bool calcCameraPoseWithoutInterpole(sead::LookAtCamera*) const; void startSnapshotMode(bool) const;
Suggestion:
void startSnapshotMode(bool);
GRAnimated
left a comment
There was a problem hiding this comment.
I'm taking over this branch, also rebased.
@GRAnimated made 5 comments.
Reviewable status: 20 of 37 files reviewed, 17 unresolved discussions (waiting on german77 and MonsterDruide1).
lib/al/Library/Camera/CameraFlagCtrl.h line 18 at r1 (raw file):
Previously, MonsterDruide1 wrote…
No, my point is "delete these two functions", as you can just do
flags.isInvalidEndEntranceCamera = true;whenever required.
Done.
lib/al/Library/Camera/CameraPoserFixActor.h line 29 at r1 (raw file):
Previously, MonsterDruide1 wrote…
also rename the
setAutoAroundFrontandsetCalcNearestAtFromPreAttoset_160and similar
Done.
lib/al/Library/Camera/CameraPoserFixActor.h line 49 at r2 (raw file):
void start(const CameraStartInfo& startInfo) override; void setTalkDistance(f32 distance) { _173 = distance; }
Done.
lib/al/Library/Camera/CameraPoseUpdater.h line 40 at r2 (raw file):
void requestCancelInterpole(); bool calcCameraPoseWithoutInterpole(sead::LookAtCamera*) const; void startSnapshotMode(bool) const;
Done.
|
Remaining reviews fixed and rebased. |
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 reviewed 3 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: 19 of 37 files reviewed, 16 unresolved discussions (waiting on german77, GRAnimated, and guymakinggames).
a discussion (no related file):
blocked by #1303, they share a bunch of headers.
This change is
Report for 1.0 (befc651 - 3661e38)
📈 Matched code: 15.53% (+0.12%, +14448 bytes)
✅ 184 new matches
Library/Camera/CameraUtilalCameraFunction::initAreaCamera(al::IUseCamera const*, al::PlacementInfo const&, char const*)Library/Camera/CameraUtilal::initFixFishingCamera(al::LiveActor const*, al::ActorInitInfo const&, char const*, sead::Vector3<float> const&, sead::Vector3<float> const&, float, float, float, bool)Library/Camera/CameraUtilal::initFixActorCamera(al::LiveActor const*, al::ActorInitInfo const&, char const*, sead::Vector3<float> const&, float, float, float, bool)Library/Camera/CameraUtilal::initFixTalkCamera(al::LiveActor const*, al::ActorInitInfo const&, char const*, sead::Vector3<float> const&, float, float, float, bool)Library/Camera/CameraUtilal::startCameraShakeByAction(al::LiveActor const*, char const*, char const*, int, int)Library/Camera/CameraUtilal::initProgramableCameraWithCollider(al::IUseCamera const*, al::ActorInitInfo const&, char const*, sead::Vector3<float> const*, sead::Vector3<float> const*, sead::Vector3<float> const*)Library/Camera/CameraUtilal::initProgramableCamera(al::IUseCamera const*, al::ActorInitInfo const&, char const*, sead::Vector3<float> const*, sead::Vector3<float> const*, sead::Vector3<float> const*)Library/Camera/CameraUtilal::initProgramableCameraKeepColliderPreCamera(al::IUseCamera const*, al::ActorInitInfo const&, char const*, sead::Vector3<float> const*, sead::Vector3<float> const*, sead::Vector3<float> const*)Library/Camera/CameraUtilal::initAnimCamera(al::IUseCamera const*, al::ActorInitInfo const&, al::Resource const*, sead::Matrix34<float> const*, char const*)Library/Camera/CameraUtilal::initProgramableAngleCamera(al::IUseCamera const*, al::PlacementInfo const&, char const*, sead::Vector3<float> const*, float const*, float const*, float const*)Library/Camera/CameraUtilal::initFixPointCamera(al::IUseCamera const*, al::ActorInitInfo const&, char const*, bool)Library/Camera/CameraUtilal::initFixLookCamera(al::LiveActor*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilal::initActorRailParallelCamera(al::LiveActor const*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilal::initLookDownCamera(al::LiveActor const*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilal::initTowerCameraWithSave(al::IUseCamera const*, sead::Vector3<float> const*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilal::initTowerCamera(al::IUseCamera const*, sead::Vector3<float> const*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilal::initBossBattleCamera(al::IUseCamera const*, sead::Vector3<float> const*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilal::startCameraShakeByHitReaction(al::IUseCamera const*, char const*, char const*, char const*, int, int)Library/Camera/CameraUtilal::initFollowCameraSimple(al::IUseCamera const*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilal::initSubjectiveCamera(al::IUseCamera const*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilal::initParallelCamera(al::IUseCamera const*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilal::initRaceCamera(al::IUseCamera const*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilal::initCartCamera(al::IUseCamera const*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilal::initKinopioBrigadeCamera(al::IUseCamera const*, al::ActorInitInfo const&, char const*)Library/Camera/CameraUtilalCameraFunction::initCameraNoSave(al::CameraPoser*, al::IUseCamera const*, al::ActorInitInfo const&, char const*, int)Library/Camera/CameraUtilalCameraFunction::initCamera(al::CameraPoser*, al::IUseCamera const*, al::ActorInitInfo const&, char const*, int)Library/Camera/CameraUtilalCameraFunction::initCameraNoSave(al::CameraPoser*, al::IUseCamera const*, al::PlacementInfo const&, char const*, int)Library/Camera/CameraUtilalCameraFunction::initCamera(al::CameraPoser*, al::IUseCamera const*, al::PlacementInfo const&, char const*, int)Library/Camera/CameraUtilal::setCurrentCameraPose(al::CameraPoseInfo*, al::IUseCamera const*)Library/Camera/CameraUtilal::initFixDoorwayCamera(al::IUseCamera const*, char const*, sead::Vector3<float> const&, sead::Vector3<float> const&)...and 154 more new matches