Npc: Implement SessionMusicianLocalFunction and SessionMusicianManager - #1330
Npc: Implement SessionMusicianLocalFunction and SessionMusicianManager#1330Spletz47 wants to merge 1 commit into
SessionMusicianLocalFunction and SessionMusicianManager#1330Conversation
d3ac1f9 to
57226f1
Compare
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 made 3 comments.
Reviewable status: 0 of 7 files reviewed, 3 unresolved discussions (waiting on Spletz47).
a discussion (no related file):
Try rewriting the loop stuff with iterators.
Example: https://github.com/MonsterDruide1/OdysseyDecomp/blob/master/src/Player/PlayerModelHolder.cpp#L16
a discussion (no related file):
Make sure all nerves are labelled properly. Insert placeholder names if required (precede them with # to mark them as comments in the yaml). All HostNrv instances are currently weird, take a look at how other examples avoid workflow failures, and decide to go with one way - only relevant thing is that it should be somehow obvious that this is not their final form, and we'll eventually take another look at it.
src/Npc/SessionMusicianManager.cpp line 160 at r1 (raw file):
do { rs::addDemoActor(*it++, 1); } while (it != &mMusicianPtrArray.data()[mMusicianPtrArray.size()]);
Example of iterators:
Suggestion:
for (auto it = mMusicianPtrArray.begin(); it != mMusicianPtrArray.end(); it++)
rs::addDemoActor(*it, 1);
Spletz47
left a comment
There was a problem hiding this comment.
@Spletz47 made 3 comments.
Reviewable status: 0 of 7 files reviewed, 3 unresolved discussions (waiting on MonsterDruide1).
a discussion (no related file):
Previously, MonsterDruide1 wrote…
Try rewriting the loop stuff with iterators.
Example: https://github.com/MonsterDruide1/OdysseyDecomp/blob/master/src/Player/PlayerModelHolder.cpp#L16
Done.
a discussion (no related file):
Previously, MonsterDruide1 wrote…
Make sure all nerves are labelled properly. Insert placeholder names if required (precede them with
#to mark them as comments in the yaml). All HostNrv instances are currently weird, take a look at how other examples avoid workflow failures, and decide to go with one way - only relevant thing is that it should be somehow obvious that this is not their final form, and we'll eventually take another look at it.
I think I've done what you've asked? Let me know if I've messed it up.
src/Npc/SessionMusicianManager.cpp line 160 at r1 (raw file):
Previously, MonsterDruide1 wrote…
Example of iterators:
Done.
german77
left a comment
There was a problem hiding this comment.
test
@german77 made 23 comments.
Reviewable status: 0 of 7 files reviewed, 25 unresolved discussions (waiting on MonsterDruide1 and Spletz47).
src/Npc/SessionMusicianLocalFunction.cpp line 31 at r2 (raw file):
bool isMusicianType(const al::LiveActor* actor, SessionMusicianType type) { return (u32)getMusicianType(actor) == type;
Suggestion:
return (u32)getMusicianType(actor) == (u32)type;src/Npc/SessionMusicianLocalFunction.cpp line 45 at r2 (raw file):
s32 index = musician->getLinkedShineIndex(); return GameDataFunction::isGotShine(accessor, index);
Suggestion:
return GameDataFunction::isGotShine(musician, musician->getLinkedShineIndex());src/Npc/SessionMusicianLocalFunction.cpp line 50 at r2 (raw file):
void entryMusicianToManager(SessionMusicianNpc* musician) { getSessionMusicianManager(musician)->entryMusician(musician); return;
src/Npc/SessionMusicianLocalFunction.cpp line 56 at r2 (raw file):
GameDataHolderAccessor accessor(actor); return (SessionEventProgress::Wait4thMusician < GameDataFunction::getSessionEventProgress(accessor));
Suggestion:
return (SessionEventProgress::Wait4thMusician <
GameDataFunction::getSessionEventProgress(actor));src/Npc/SessionMusicianLocalFunction.cpp line 62 at r2 (raw file):
SessionMusicianManager* manager = getSessionMusicianManager(actor); if (manager->isSubscribed(SessionMusicianType::Vocal))
Suggestion:
if (isSubscribed(actor,SessionMusicianType::Vocal))src/Npc/SessionMusicianLocalFunction.cpp line 68 at r2 (raw file):
for (s32 i = 0; i < SessionMusicianType::size(); ++i) if (getSessionMusicianManager(actor)->isSubscribed(i))
Suggestion:
if (isSubscribed(actor,i))src/Npc/SessionMusicianManager.h line 26 at r2 (raw file):
SessionMusicianManager(const char* name); ~SessionMusicianManager() {};
Check if this auto generated
src/Npc/SessionMusicianManager.cpp line 55 at r2 (raw file):
bool SessionMusicianManager::isJoinedMusician() const { if (mMusicianPtrArray.size() == 0) return false;
Autogenerated by the compiler
src/Npc/SessionMusicianManager.cpp line 66 at r2 (raw file):
SessionMusicianNpc* SessionMusicianManager::getJoinedMusician() const { if (mMusicianPtrArray.size() == 0) return nullptr;
src/Npc/SessionMusicianManager.cpp line 77 at r2 (raw file):
bool SessionMusicianManager::isSubscribed(SessionMusicianType type) const { if (mMusicianPtrArray.size() == 0) return false;
src/Npc/SessionMusicianManager.cpp line 105 at r2 (raw file):
SessionMusicianNpc* SessionMusicianManager::findPowerPlant() const { if (mMusicianPtrArray.size() == 0) return nullptr;
src/Npc/SessionMusicianManager.cpp line 118 at r2 (raw file):
bool SessionMusicianManager::tryStartWarp(al::PlacementInfo* info) { if (mMusicianPtrArray.size() == 0) return false;
src/Npc/SessionMusicianManager.cpp line 124 at r2 (raw file):
SessionMusicianWarpAgent* warpAgent = it->getWarpAgent(); if (warpAgent->tryGetWarpTargetInfo(info)) { if (warpAgent->tryStartWarp()) {
Suggestion:
if (warpAgent->tryGetWarpTargetInfo(info) && warpAgent->tryStartWarp()) {src/Npc/SessionMusicianManager.cpp line 138 at r2 (raw file):
void SessionMusicianManager::addDemoAllMusicians() { if (mMusicianPtrArray.size() == 0) return;
src/Npc/SessionMusicianManager.cpp line 183 at r2 (raw file):
SessionMusicianManager* manager = al::tryGetSceneObj<SessionMusicianManager>(holder); return manager->tryStartWarp(info);
Suggestion:
return getSessionMusicianManager(holder)->tryStartWarp(info);src/Npc/SessionMusicianManager.cpp line 188 at r2 (raw file):
void entrySessionMayorToManager(SessionMayorNpc* mayor) { al::tryGetSceneObj<SessionMusicianManager>(mayor)->entryMayor(mayor); return;
Suggestion:
return getSessionMusicianManager(mayor)->entryMayor(mayor);src/Npc/SessionMusicianManager.cpp line 199 at r2 (raw file):
return true; return false;
Suggestion:
SessionMusicianManager* manager = getSessionMusicianManager(holder);
return manager && manager->isJoinedMusician();src/Npc/SessionMusicianManager.cpp line 203 at r2 (raw file):
SessionMusicianNpc* tryGetJoinedSessionMusicanActor(const al::IUseSceneObjHolder* holder) { SessionMusicianManager* manager = al::tryGetSceneObj<SessionMusicianManager>(holder);
Suggestion:
SessionMusicianManager* manager = getSessionMusicianManager(holder);src/Npc/SessionMusicianManager.cpp line 211 at r2 (raw file):
bool tryAddJoinedSessionMusicianDemoActor(const al::IUseSceneObjHolder* holder) { SessionMusicianNpc* musician = tryGetJoinedSessionMusicanActor(holder);
Ditto for the other instances
Suggestion:
SessionMusicianManager* manager = getSessionMusicianManager(holder);src/Npc/SessionMusicianManager.cpp line 225 at r2 (raw file):
return false; SessionMusicianNpc* musician = manager->getJoinedMusician();
Suggestion:
SessionMusicianNpc* musician = tryGetJoinedSessionMusicanActor(holder);src/Npc/SessionMusicianManager.cpp line 238 at r2 (raw file):
return false; SessionMusicianNpc* musician = manager->getJoinedMusician();
Suggestion:
SessionMusicianNpc* musician = tryGetJoinedSessionMusicanActor(holder);src/Npc/SessionMusicianManager.cpp line 251 at r2 (raw file):
return false; SessionMusicianNpc* musician = manager->getJoinedMusician();
Suggestion:
SessionMusicianNpc* musician = tryGetJoinedSessionMusicanActor(holder);
Spletz47
left a comment
There was a problem hiding this comment.
@Spletz47 made 22 comments.
Reviewable status: 0 of 7 files reviewed, 25 unresolved discussions (waiting on german77 and MonsterDruide1).
src/Npc/SessionMusicianManager.h line 26 at r2 (raw file):
Previously, german77 (Narr the Reg) wrote…
Check if this auto generated
I remember having to add it at some point for something, but it seems to compile just fine without it. I'll remove it for now.
src/Npc/SessionMusicianManager.cpp line 55 at r2 (raw file):
Previously, german77 (Narr the Reg) wrote…
Autogenerated by the compiler
Done.
src/Npc/SessionMusicianManager.cpp line 211 at r2 (raw file):
Previously, german77 (Narr the Reg) wrote…
Ditto for the other instances
Done.
src/Npc/SessionMusicianLocalFunction.cpp line 31 at r2 (raw file):
bool isMusicianType(const al::LiveActor* actor, SessionMusicianType type) { return (u32)getMusicianType(actor) == type;
Done.
src/Npc/SessionMusicianLocalFunction.cpp line 45 at r2 (raw file):
s32 index = musician->getLinkedShineIndex(); return GameDataFunction::isGotShine(accessor, index);
Does not match.
src/Npc/SessionMusicianLocalFunction.cpp line 50 at r2 (raw file):
void entryMusicianToManager(SessionMusicianNpc* musician) { getSessionMusicianManager(musician)->entryMusician(musician); return;
Done.
src/Npc/SessionMusicianLocalFunction.cpp line 56 at r2 (raw file):
GameDataHolderAccessor accessor(actor); return (SessionEventProgress::Wait4thMusician < GameDataFunction::getSessionEventProgress(accessor));
Does not match.
src/Npc/SessionMusicianLocalFunction.cpp line 62 at r2 (raw file):
SessionMusicianManager* manager = getSessionMusicianManager(actor); if (manager->isSubscribed(SessionMusicianType::Vocal))
Done.
src/Npc/SessionMusicianLocalFunction.cpp line 68 at r2 (raw file):
for (s32 i = 0; i < SessionMusicianType::size(); ++i) if (getSessionMusicianManager(actor)->isSubscribed(i))
Done.
src/Npc/SessionMusicianManager.cpp line 66 at r2 (raw file):
SessionMusicianNpc* SessionMusicianManager::getJoinedMusician() const { if (mMusicianPtrArray.size() == 0) return nullptr;
Done.
src/Npc/SessionMusicianManager.cpp line 77 at r2 (raw file):
bool SessionMusicianManager::isSubscribed(SessionMusicianType type) const { if (mMusicianPtrArray.size() == 0) return false;
Done.
src/Npc/SessionMusicianManager.cpp line 105 at r2 (raw file):
SessionMusicianNpc* SessionMusicianManager::findPowerPlant() const { if (mMusicianPtrArray.size() == 0) return nullptr;
Done.
src/Npc/SessionMusicianManager.cpp line 118 at r2 (raw file):
bool SessionMusicianManager::tryStartWarp(al::PlacementInfo* info) { if (mMusicianPtrArray.size() == 0) return false;
Done.
src/Npc/SessionMusicianManager.cpp line 124 at r2 (raw file):
SessionMusicianWarpAgent* warpAgent = it->getWarpAgent(); if (warpAgent->tryGetWarpTargetInfo(info)) { if (warpAgent->tryStartWarp()) {
Done.
src/Npc/SessionMusicianManager.cpp line 138 at r2 (raw file):
void SessionMusicianManager::addDemoAllMusicians() { if (mMusicianPtrArray.size() == 0) return;
Done.
src/Npc/SessionMusicianManager.cpp line 183 at r2 (raw file):
SessionMusicianManager* manager = al::tryGetSceneObj<SessionMusicianManager>(holder); return manager->tryStartWarp(info);
Done.
src/Npc/SessionMusicianManager.cpp line 188 at r2 (raw file):
void entrySessionMayorToManager(SessionMayorNpc* mayor) { al::tryGetSceneObj<SessionMusicianManager>(mayor)->entryMayor(mayor); return;
Done.
src/Npc/SessionMusicianManager.cpp line 199 at r2 (raw file):
return true; return false;
Done.
src/Npc/SessionMusicianManager.cpp line 203 at r2 (raw file):
SessionMusicianNpc* tryGetJoinedSessionMusicanActor(const al::IUseSceneObjHolder* holder) { SessionMusicianManager* manager = al::tryGetSceneObj<SessionMusicianManager>(holder);
Done.
src/Npc/SessionMusicianManager.cpp line 225 at r2 (raw file):
return false; SessionMusicianNpc* musician = manager->getJoinedMusician();
Done.
src/Npc/SessionMusicianManager.cpp line 238 at r2 (raw file):
return false; SessionMusicianNpc* musician = manager->getJoinedMusician();
Done.
src/Npc/SessionMusicianManager.cpp line 251 at r2 (raw file):
return false; SessionMusicianNpc* musician = manager->getJoinedMusician();
Done.
german77
left a comment
There was a problem hiding this comment.
@german77 made 2 comments and resolved 19 discussions.
Reviewable status: 0 of 7 files reviewed, 6 unresolved discussions (waiting on MonsterDruide1 and Spletz47).
src/Npc/SessionMusicianLocalFunction.cpp line 45 at r2 (raw file):
Previously, Spletz47 (Spletz) wrote…
Does not match.
It should. I tested all the suggestions. Literally copy pasted the code, but let me double check
src/Npc/SessionMusicianManager.cpp line 188 at r2 (raw file):
Previously, Spletz47 (Spletz) wrote…
Done.
Uh this is void. It shouldn't return anything
Spletz47
left a comment
There was a problem hiding this comment.
@Spletz47 made 2 comments.
Reviewable status: 0 of 7 files reviewed, 6 unresolved discussions (waiting on german77 and MonsterDruide1).
src/Npc/SessionMusicianLocalFunction.cpp line 45 at r2 (raw file):
Previously, german77 (Narr the Reg) wrote…
It should. I tested all the suggestions. Literally copy pasted the code, but let me double check
Sorry, you were right. I made a small error in trying your implementation and just assumed that if I had done it the way it was it was probably for a reason. Implemented correctly now.
src/Npc/SessionMusicianManager.cpp line 188 at r2 (raw file):
Previously, german77 (Narr the Reg) wrote…
Uh this is void. It shouldn't return anything
Sorry, fixed now.
german77
left a comment
There was a problem hiding this comment.
@german77 reviewed 7 files and resolved 3 discussions.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on MonsterDruide1).
Spletz47
left a comment
There was a problem hiding this comment.
poke
@Spletz47 made 1 comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on MonsterDruide1).
Implements the
SessionMusicianLocalFunctionnamespace and theSessionMusicianManagerclass. IncludesSessionMayorNpc.handSessionMusicianNpc(this will be the biggest single commit I do of these musician-related classes, then its smooth sailing from here in terms of pull-request reviewing! Exciting stuff!)Thanks to @german77 for helping with
SessionMusicianLocalFunction::tryAddJoinedSessionMusicianDemoActorand similar functions.Also thanks to all those who helped with the
SessionMusicianNpcclass members.SessionMusicianManager.cppincludes some of theSessionMusicianLocalFunctionnamespace implementation, which is why they are both included in this pull request.This change is
Report for 1.0 (b64cbfb - f7958ae)
📈 Matched code: 15.53% (+0.03%, +4184 bytes)
✅ 46 new matches
Npc/SessionMusicianManagerSessionMusicianLocalFunction::tryCreateSessionMusicianManager(al::IUseSceneObjHolder const*)Npc/SessionMusicianLocalFunctionSessionMusicianLocalFunction::getMemberMusicianNum(al::LiveActor const*)Npc/SessionMusicianManagerSessionMusicianLocalFunction::tryAddJoinedSessionMusicianDemoActor(al::IUseSceneObjHolder const*)Npc/SessionMusicianManagerSessionMusicianManager::exeWait()Npc/SessionMusicianManagerSessionMusicianManager::initAfterPlacementSceneObj(al::ActorInitInfo const&)Npc/SessionMusicianManagerSessionMusicianLocalFunction::tryStartWarpToSessionMayor(al::IUseSceneObjHolder const*, al::PlacementInfo*)Npc/SessionMusicianManagerSessionMusicianManager::tryAppearPowerPlant()Npc/SessionMusicianManagerSessionMusicianManager::SessionMusicianManager(char const*)Npc/SessionMusicianManagerSessionMusicianLocalFunction::tryGetSessionMoonGetDemoPlayerPose(sead::Quat<float>*, al::IUseSceneObjHolder const*)Npc/SessionMusicianManagerSessionMusicianManager::SessionMusicianManager(char const*)Npc/SessionMusicianLocalFunctionSessionMusicianLocalFunction::isMusicianType(al::LiveActor const*, SessionMusicianType)Npc/SessionMusicianManagerSessionMusicianLocalFunction::tryGetSessionMoonGetDemoPlayerPos(sead::Vector3<float>*, al::IUseSceneObjHolder const*)Npc/SessionMusicianManagerSessionMusicianManager::tryStartWarp(al::PlacementInfo*)Npc/SessionMusicianManagerSessionMusicianLocalFunction::trySetJoinedSessionMusicianTransformForMoonGetDemo(al::IUseSceneObjHolder const*)Npc/SessionMusicianLocalFunctionSessionMusicianLocalFunction::getMusicianType(al::LiveActor const*)Npc/SessionMusicianManagerSessionMusicianManager::isSubscribed(SessionMusicianType) constNpc/SessionMusicianManagerSessionMusicianLocalFunction::tryGetJoinedSessionMusicanActor(al::IUseSceneObjHolder const*)Npc/SessionMusicianManagerSessionMusicianLocalFunction::isJoinedSessionMusician(al::IUseSceneObjHolder const*)Npc/SessionMusicianManagerSessionMusicianManager::findPowerPlant() constNpc/SessionMusicianManagerSessionMusicianManager::getJoinedMusician() constNpc/SessionMusicianManagerSessionMusicianLocalFunction::addDemoAllMusicians(al::IUseSceneObjHolder const*)Npc/SessionMusicianManagerSessionMusicianManager::isJoinedMusician() constNpc/SessionMusicianLocalFunctionSessionMusicianLocalFunction::isAlreadySessionMember(SessionMusicianNpc const*)Npc/SessionMusicianManagerSessionMusicianManager::addDemoAllMusicians()Npc/SessionMusicianLocalFunctionSessionMusicianLocalFunction::isSessionFullMember(al::LiveActor const*)Npc/SessionMusicianManagerSessionMusicianLocalFunction::entrySessionMayorToManager(SessionMayorNpc*)Npc/SessionMusicianLocalFunctionSessionMusicianLocalFunction::isSubscribed(al::LiveActor const*, SessionMusicianType)Npc/SessionMusicianLocalFunctionSessionMusicianLocalFunction::entryMusicianToManager(SessionMusicianNpc*)Npc/SessionMusicianManagerSessionMusicianManager::entryMusician(SessionMusicianNpc*)Npc/SessionMusicianManagerSessionMusicianLocalFunction::getSessionMusicianManager(al::IUseSceneObjHolder const*)...and 16 more new matches