Npc: Implement SessionMusicianManager - #1130
Conversation
451026b to
fd5746b
Compare
SessionMusicianManager
german77
left a comment
There was a problem hiding this comment.
The AI didn't fair well in this one
@german77 made 30 comments.
Reviewable status: 0 of 8 files reviewed, 29 unresolved discussions (waiting on guymakinggames).
src/Npc/SessionMayorNpc.h line 16 at r1 (raw file):
class SessionMusicianNpc; class SessionMayorNpc : public al::LiveActor {
Suggestion:
class SessionMayorNpc : public al::LiveActor, public al::IEventFlowEventReceiver{src/Npc/SessionMayorNpc.h line 38 at r1 (raw file):
void exeDance(); void exeDemo(); };
static_assert(sizeof(SessionMayorNpc ) == 0x170);
src/Npc/SessionMusicianManager.cpp line 62 at r1 (raw file):
if (musician == &mMusicians.data()[mMusicians.size()]) break; }
Cursed code
Suggestion:
for(s32 i=0;i<mMusicians.size();i++){
if (mMusicians[i]->isJoined())
return true;
}src/Npc/SessionMusicianManager.cpp line 79 at r1 (raw file):
if (musician == &mMusicians.data()[mMusicians.size()]) break; }
Suggestion:
for(s32 i=0;i<mMusicians.size();i++){
// Might need SessionMusicianNpc* npc=mMusicians[i] to match
if (mMusicians[i]->isJoined())
return mMusicians[i];
}src/Npc/SessionMusicianManager.cpp line 97 at r1 (raw file):
if (musician == &mMusicians.data()[mMusicians.size()]) break; }
Suggestion:
for(s32 i=0;i<mMusicians.size();i++){
SessionMusicianNpc* musician=mMusicians[i];
if (SessionMusicianLocalFunction::isMusicianType(musician, type) &&
SessionMusicianLocalFunction::isAlreadySessionMember(musician))
return true;
}src/Npc/SessionMusicianManager.cpp line 110 at r1 (raw file):
if (GameDataFunction::getSessionEventProgress(accessor).value() < SessionEventProgress::WaitThePowerPlantWorks) return false;
Suggestion:
if (GameDataFunction::getSessionEventProgress(this).value() <
SessionEventProgress::WaitThePowerPlantWorks)
return false;src/Npc/SessionMusicianManager.cpp line 128 at r1 (raw file):
if (musician == &mMusicians.data()[mMusicians.size()]) break; }
Suggestion:
for(s32 i=0;i<mMusicians.size();i++){
SessionMusicianNpc* musician=mMusicians[i];
if (SessionMusicianLocalFunction::getMusicianType(*musician) == SessionMusicianType::Vocal)
return musician;
}src/Npc/SessionMusicianManager.cpp line 151 at r1 (raw file):
if (musician == &mMusicians.data()[mMusicians.size()]) break; }
Suggestion:
for(s32 i=0;i<mMusicians.size();i++){
SessionMusicianNpc* musician=mMusicians[i];
SessionMusicianWarpAgent* warpAgent = musician->getWarpAgent();
if (warpAgent->tryGetWarpTargetInfo(placementInfo) && warpAgent->tryStartWarp()){
musician->doneWarp();
al::invalidateClipping(mMayor);
return true;
}
}src/Npc/SessionMusicianManager.cpp line 166 at r1 (raw file):
if (musician == &mMusicians.data()[mMusicians.size()]) break; }
Suggestion:
for(s32 i=0;i<mMusicians.size();i++){
rs::addDemoActor(musician[i], true);
}src/Npc/SessionMusicianManager.cpp line 173 at r1 (raw file):
mBgmController->updateNerve(); if (!(SessionMusicianLocalFunction::getMemberMusicianNum(this) < 5))
invert this condition
src/Npc/SessionMusicianManager.cpp line 183 at r1 (raw file):
powerPlant->appear(); } return;
Suggestion:
tryAppearPowerPlant();src/Npc/SessionMusicianManager.cpp line 194 at r1 (raw file):
void tryCreateSessionMusicianManager(const al::IUseSceneObjHolder* holder) { if (al::isExistSceneObj(holder, SceneObjID_SessionMusicianManager))
Suggestion:
if (isExistSessionMusicianManager(holder)src/Npc/SessionMusicianManager.cpp line 210 at r1 (raw file):
bool tryStartWarpToSessionMayor(const al::IUseSceneObjHolder* holder, al::PlacementInfo* placementInfo) { if (!al::isExistSceneObj(holder, SceneObjID_SessionMusicianManager))
Suggestion:
if (isExistSessionMusicianManager(holder)src/Npc/SessionMusicianManager.cpp line 214 at r1 (raw file):
SessionMusicianManager* manager = al::tryGetSceneObj<SessionMusicianManager>(holder); return manager->tryStartWarp(placementInfo);
Suggestion:
return getSessionMusicianManager(holder)->tryStartWarp(placementInfo);src/Npc/SessionMusicianManager.cpp line 219 at r1 (raw file):
void entrySessionMayorToManager(SessionMayorNpc* mayor) { SessionMusicianManager* manager = al::tryGetSceneObj<SessionMusicianManager>(mayor); manager->setSessionMayor(mayor);
Suggestion:
return getSessionMusicianManager(holder)->setSessionMayor(mayor);src/Npc/SessionMusicianManager.cpp line 240 at r1 (raw file):
} return false;
Suggestion:
SessionMusicianManager* manager = getSessionMusicianManager(holder);
if (manager == nullptr)
return false;
return manager->isJoinedMusician();src/Npc/SessionMusicianManager.cpp line 262 at r1 (raw file):
} return nullptr;
Suggestion:
SessionMusicianManager* manager = getSessionMusicianManager(holder);
if (manager == nullptr)
return false;
return manager->getJoinedMusician();src/Npc/SessionMusicianManager.cpp line 283 at r1 (raw file):
if (musicianPtr == &musicians.data()[musicians.size()]) return false; }
Suggestion:
SessionMusicianManager* manager = getSessionMusicianManager(holder);
if (manager == nullptr)
return false;
if(!manager->isJoinedMusician())
return false;src/Npc/SessionMusicianManager.cpp line 285 at r1 (raw file):
} SessionMusicianNpc* musician = *musicianPtr;
Suggestion:
SessionMusicianNpc* musician = manager->getMusicians();src/Npc/SessionMusicianManager.cpp line 301 at r1 (raw file):
} while (demoActorIndex < musician->getDemoActorNum()); return result;
Suggestion:
for(s32 i=0;i< musician->getDemoActorNum();i++) {
rs::addDemoActor(musician->tryGetDemoActor(i), true);
}
return true;src/Npc/SessionMusicianManager.cpp line 321 at r1 (raw file):
if (musicianPtr == &musicians.data()[musicians.size()]) return false; }
Suggestion:
SessionMusicianManager* manager = getSessionMusicianManager(holder);
if (manager == nullptr)
return false;
if(!manager->isJoinedMusician())
return false;src/Npc/SessionMusicianManager.cpp line 327 at r1 (raw file):
return false; new (out) sead::Vector3f(musician->getMoonGetDemoPlayerPos());
Suggestion:
SessionMusicianNpc* musician = manager->getMusicians();
if (musician == nullptr)
return false;
out->set(musician->getMoonGetDemoPlayerPos());src/Npc/SessionMusicianManager.cpp line 348 at r1 (raw file):
if (musicianPtr == &musicians.data()[musicians.size()]) return false; }
Suggestion:
SessionMusicianManager* manager = getSessionMusicianManager(holder);
if (manager == nullptr)
return false;
if(!manager->isJoinedMusician())
return false;src/Npc/SessionMusicianManager.cpp line 354 at r1 (raw file):
return false; new (out) sead::Quatf(musician->getMoonGetDemoPlayerPose());
Suggestion:
SessionMusicianNpc* musician = manager->getMusicians();
if (musician == nullptr)
return false;
out->set(musician->getMoonGetDemoPlayerPose());src/Npc/SessionMusicianManager.cpp line 375 at r1 (raw file):
if (musicianPtr == &musicians.data()[musicians.size()]) return false; }
Suggestion:
SessionMusicianManager* manager = getSessionMusicianManager(holder);
if (manager == nullptr)
return false;
if(!manager->isJoinedMusician())
return false;src/Npc/SessionMusicianManager.cpp line 383 at r1 (raw file):
sead::Vector3f pos = musician->getMoonGetDemoPlayerPos(); al::faceToTarget(musician, pos); return true;
Suggestion:
SessionMusicianNpc* musician = manager->getMusicians();
if (musician == nullptr)
return false;
al::faceToTarget(musician, musician->getMoonGetDemoPlayerPos());
return true;src/Npc/SessionMusicianManager.cpp line 388 at r1 (raw file):
void addDemoAllMusicians(const al::IUseSceneObjHolder* holder) { SessionMusicianManager* manager = al::tryGetSceneObj<SessionMusicianManager>(holder); manager->addDemoAllMusicians();
Suggestion:
getSessionMusicianManager(holder)->addDemoAllMusicians();src/Npc/SessionMusicianNpc.h line 19 at r1 (raw file):
class SessionMusicianWarpAgent; class SessionMusicianNpc : public al::LiveActor {
Suggestion:
class SessionMusicianNpc : public al::LiveActor, public al::IEventFlowEventReceiver, public al::IEventFlowQueryJudgesrc/Npc/SessionMusicianNpc.h line 57 at r1 (raw file):
return nullptr; return mDemoActors.data()[index];
Suggestion:
return mDemoActors[index];fd5746b to
350ebe3
Compare
guymakinggames
left a comment
There was a problem hiding this comment.
It's a tricky one to uncurse!
@guymakinggames made 30 comments.
Reviewable status: 0 of 8 files reviewed, 29 unresolved discussions (waiting on german77).
src/Npc/SessionMayorNpc.h line 38 at r1 (raw file):
Previously, german77 (Narr the Reg) wrote…
static_assert(sizeof(SessionMayorNpc ) == 0x170);
Done.
src/Npc/SessionMusicianManager.cpp line 62 at r1 (raw file):
Previously, german77 (Narr the Reg) wrote…
Cursed code
Did not match. Also tried with
Code snippet:
bool SessionMusicianManager::isJoinedMusician() const {
for (s32 i = 0; i < mMusicians.size(); i++) {
SessionMusicianNpc* npc = mMusicians[i];
if (npc->isJoined())
return true;
}
return false;
}src/Npc/SessionMusicianManager.cpp line 173 at r1 (raw file):
Previously, german77 (Narr the Reg) wrote…
invert this condition
Mismatch when doing:
Code snippet:
if (SessionMusicianLocalFunction::getMemberMusicianNum(this) >= 5)src/Npc/SessionMayorNpc.h line 16 at r1 (raw file):
class SessionMusicianNpc; class SessionMayorNpc : public al::LiveActor {
Done.
src/Npc/SessionMusicianManager.cpp line 79 at r1 (raw file):
if (musician == &mMusicians.data()[mMusicians.size()]) break; }
Neither version matches
src/Npc/SessionMusicianManager.cpp line 97 at r1 (raw file):
if (musician == &mMusicians.data()[mMusicians.size()]) break; }
Also does not match
src/Npc/SessionMusicianManager.cpp line 110 at r1 (raw file):
if (GameDataFunction::getSessionEventProgress(accessor).value() < SessionEventProgress::WaitThePowerPlantWorks) return false;
Done.
src/Npc/SessionMusicianManager.cpp line 128 at r1 (raw file):
if (musician == &mMusicians.data()[mMusicians.size()]) break; }
Does not match
src/Npc/SessionMusicianManager.cpp line 151 at r1 (raw file):
if (musician == &mMusicians.data()[mMusicians.size()]) break; }
Not a match
src/Npc/SessionMusicianManager.cpp line 166 at r1 (raw file):
if (musician == &mMusicians.data()[mMusicians.size()]) break; }
Mismatch
src/Npc/SessionMusicianManager.cpp line 183 at r1 (raw file):
powerPlant->appear(); } return;
Done.
src/Npc/SessionMusicianManager.cpp line 194 at r1 (raw file):
void tryCreateSessionMusicianManager(const al::IUseSceneObjHolder* holder) { if (al::isExistSceneObj(holder, SceneObjID_SessionMusicianManager))
Done.
src/Npc/SessionMusicianManager.cpp line 210 at r1 (raw file):
bool tryStartWarpToSessionMayor(const al::IUseSceneObjHolder* holder, al::PlacementInfo* placementInfo) { if (!al::isExistSceneObj(holder, SceneObjID_SessionMusicianManager))
Done.
src/Npc/SessionMusicianManager.cpp line 214 at r1 (raw file):
SessionMusicianManager* manager = al::tryGetSceneObj<SessionMusicianManager>(holder); return manager->tryStartWarp(placementInfo);
Done.
src/Npc/SessionMusicianManager.cpp line 219 at r1 (raw file):
void entrySessionMayorToManager(SessionMayorNpc* mayor) { SessionMusicianManager* manager = al::tryGetSceneObj<SessionMusicianManager>(mayor); manager->setSessionMayor(mayor);
Done.
src/Npc/SessionMusicianManager.cpp line 240 at r1 (raw file):
} return false;
Done.
src/Npc/SessionMusicianManager.cpp line 262 at r1 (raw file):
} return nullptr;
Done.
src/Npc/SessionMusicianManager.cpp line 283 at r1 (raw file):
if (musicianPtr == &musicians.data()[musicians.size()]) return false; }
See below
src/Npc/SessionMusicianManager.cpp line 285 at r1 (raw file):
} SessionMusicianNpc* musician = *musicianPtr;
getMusicians returns PtrArray so the code after doesn't work unless I'm confusing myself here
src/Npc/SessionMusicianManager.cpp line 301 at r1 (raw file):
} while (demoActorIndex < musician->getDemoActorNum()); return result;
See above
src/Npc/SessionMusicianManager.cpp line 321 at r1 (raw file):
if (musicianPtr == &musicians.data()[musicians.size()]) return false; }
Also clashing with the change below
src/Npc/SessionMusicianManager.cpp line 327 at r1 (raw file):
return false; new (out) sead::Vector3f(musician->getMoonGetDemoPlayerPos());
Same issue
src/Npc/SessionMusicianManager.cpp line 348 at r1 (raw file):
if (musicianPtr == &musicians.data()[musicians.size()]) return false; }
See below
src/Npc/SessionMusicianManager.cpp line 354 at r1 (raw file):
return false; new (out) sead::Quatf(musician->getMoonGetDemoPlayerPose());
For all of these, unless I'm reading this wrong, the original code was trying to find the first joined musician, but getMusicians will return all of them
src/Npc/SessionMusicianManager.cpp line 375 at r1 (raw file):
if (musicianPtr == &musicians.data()[musicians.size()]) return false; }
Same
src/Npc/SessionMusicianManager.cpp line 383 at r1 (raw file):
sead::Vector3f pos = musician->getMoonGetDemoPlayerPos(); al::faceToTarget(musician, pos); return true;
Same
src/Npc/SessionMusicianManager.cpp line 388 at r1 (raw file):
void addDemoAllMusicians(const al::IUseSceneObjHolder* holder) { SessionMusicianManager* manager = al::tryGetSceneObj<SessionMusicianManager>(holder); manager->addDemoAllMusicians();
Done.
src/Npc/SessionMusicianNpc.h line 19 at r1 (raw file):
class SessionMusicianWarpAgent; class SessionMusicianNpc : public al::LiveActor {
Also causes a series of mismatches. What is doing on?
src/Npc/SessionMusicianNpc.h line 57 at r1 (raw file):
return nullptr; return mDemoActors.data()[index];
Causes mismatches when used
|
Previously, guymakinggames wrote…
For this one you got to see what's going on AI is usually useless on ctors. Most likely you need to change the members. See the 0x28 gap? those probably correspond to the event flow parents. |
|
Given the bast amount of mismatches from review I need to verify myself the issue. |
german77
left a comment
There was a problem hiding this comment.
@german77 made 1 comment and resolved 9 discussions.
Reviewable status: 0 of 8 files reviewed, 20 unresolved discussions (waiting on guymakinggames).
src/Npc/SessionMusicianManager.cpp line 62 at r1 (raw file):
Previously, guymakinggames wrote…
Did not match. Also tried with
This is an iterator on second look. Still not matching we need to see how to solve the loading issue
Code snippet:
for(auto iter=mMusicians.begin(); iter!=mMusicians.end();++iter){
if(iter->isJoined())
return true;
}
return false;|
Blocked by open-ead/sead#257 |
|
Iterators have been fixed. Requires rebase and converting all loops to iterators |
|
Superseded by #1330 |
This change is
Report for 1.0 (d6735da - 350ebe3)
📈 Matched code: 14.64% (+0.03%, +3160 bytes)
✅ 30 new matches
Npc/SessionMusicianManagerSessionMusicianLocalFunction::tryCreateSessionMusicianManager(al::IUseSceneObjHolder 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/SessionMusicianManagerSessionMusicianLocalFunction::tryGetSessionMoonGetDemoPlayerPos(sead::Vector3<float>*, al::IUseSceneObjHolder const*)Npc/SessionMusicianManagerSessionMusicianManager::tryStartWarp(al::PlacementInfo*)Npc/SessionMusicianManagerSessionMusicianLocalFunction::trySetJoinedSessionMusicianTransformForMoonGetDemo(al::IUseSceneObjHolder 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/SessionMusicianManagerSessionMusicianManager::addDemoAllMusicians()Npc/SessionMusicianManagerSessionMusicianLocalFunction::entrySessionMayorToManager(SessionMayorNpc*)Npc/SessionMusicianManagerSessionMusicianManager::entryMusician(SessionMusicianNpc*)Npc/SessionMusicianManagerSessionMusicianLocalFunction::getSessionMusicianManager(al::IUseSceneObjHolder const*)Npc/SessionMusicianManagerSessionMusicianManager::exeComplete()Npc/SessionMusicianManagernon-virtual thunk to SessionMusicianManager::initAfterPlacementSceneObj(al::ActorInitInfo const&)Npc/SessionMusicianManagerSessionMusicianLocalFunction::isExistSessionMusicianManager(al::IUseSceneObjHolder const*)Npc/SessionMusicianManagernon-virtual thunk to SessionMusicianManager::~SessionMusicianManager()Npc/SessionMusicianManagerSessionMusicianManager::~SessionMusicianManager()Npc/SessionMusicianManagerSessionMusicianManager::~SessionMusicianManager()Npc/SessionMusicianManagernon-virtual thunk to SessionMusicianManager::~SessionMusicianManager()