Library/LiveActor: Split ActorSensorFunction

[LynxDev2]

This PR splits ActorSensorFunction since their functions being in the same TU would cause mismatches do to inlining. As discussed in DMs, the ActorSensorFunction.h file now holds the alActorSensorFunction namespace and everything that was previously in ActorSensorFunction.h is now moved to ActorSensorUtil.h, with a few additions. I've also changed each include of ActorSensorFunction.h to include ActorSensorUtil.h instead

This PR is mainly prep for my future PRs

---

This change is 

[Moddimation]

I think Monsterdruide likes to have one commit per PR, but I'm not sure

Reviewed 25 of 25 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @LynxDev2)

[LynxDev2]

Nope, he squish merges them which converts however many commits into one

[LynxDev2]

ActorSensorMsgFunction needs a new name to fit the file older of the function map, so this PR is blocked by that discussion

[LynxDev2]

Since te discord thread has been inactive even after pinning a few times, I’ll implement the latest suggestion which is merging SensorMsgFunction into SensorMsgUtil soon

[LynxDev2]

As discussed on discord, I've also merged ActorSensorMsgFunction into ActorSensorUtil. This PR should now be ready for another review

[LynxDev2]

Rebased, added PR requirement and fixed submodule

[LynxDev2]

Did another rebase after Nokonoko, it would be nice if this PR was merged before other PRs that implement actors

[LynxDev2]

Although ig actor PRs that don't need any more changes do need to be merged first so that they don't end up being merged with the old wokflow run

[LynxDev2]

Another rebase after KuriboGirl

[MonsterDruide1]

Reviewed 4 of 25 files at r1, 13 of 47 files at r2, 36 of 37 files at r3, 2 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @LynxDev2)

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 16 at r5 (raw file):

    bool isMsg##Type(const al::SensorMsg* msg) {                                                   \
        return SensorMsg##Type::checkDerivedRuntimeTypeInfoStatic(msg->getRuntimeTypeInfo());      \
    }

Should probably not be placed in the header?

Code quote:

// TODO: This defines the class but the sead decomp doesn't have anything inside the RTTI functions,
// causing the functions in the vtable to be exported
#define SENSOR_MSG(Type)                                                                           \
    class SensorMsg##Type : public al::SensorMsg {                                                 \
        SEAD_RTTI_OVERRIDE(SensorMsg##Type, al::SensorMsg)                                         \
    };                                                                                             \
    bool isMsg##Type(const al::SensorMsg* msg) {                                                   \
        return SensorMsg##Type::checkDerivedRuntimeTypeInfoStatic(msg->getRuntimeTypeInfo());      \
    }

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 22 at r5 (raw file):

class SensorMsg {
    SEAD_RTTI_BASE(SensorMsg);
};

Suggestion:

class SensorMsg {
    SEAD_RTTI_BASE(SensorMsg);
public:
    virtual ~SensorMsg() = default;
};

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 97 at r5 (raw file):

void calcDirBetweenSensors(sead::Vector3f*, const HitSensor*, const HitSensor*);
void calcDirBetweenSensorsH(sead::Vector3f*, const HitSensor*, const HitSensor*);
void calcDirBetweenSensorsNormal(sead::Vector3f*, const HitSensor*, const HitSensor*,

Return value used for example at 710009BB6C

Suggestion:

bool calcDirBetweenSensors(sead::Vector3f*, const HitSensor*, const HitSensor*);
bool calcDirBetweenSensorsH(sead::Vector3f*, const HitSensor*, const HitSensor*);
bool calcDirBetweenSensorsNormal(sead::Vector3f*, const HitSensor*, const HitSensor*,

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 105 at r5 (raw file):

void calcStrikeArrowCollideWallAndCeilingBetweenAttackSensor(const LiveActor*, const HitSensor*,
                                                             const HitSensor*,
                                                             const sead::Vector3f&, f32);

Suggestion:

s32 calcStrikeArrowCollideWallAndCeilingBetweenAttackSensor(const LiveActor*, const HitSensor*,
                                                             const HitSensor*,
                                                             const sead::Vector3f&, f32);

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 156 at r5 (raw file):

bool isSensorNpc(const HitSensor*);
void validateHitSensorPlayerAll(LiveActor*);
bool isSensorPlayer(const HitSensor*);

Not placed here correctly

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 161 at r5 (raw file):

bool isSensorRide(const HitSensor*);
bool isSensorSimple(const HitSensor*);
bool isSensorLookAt(const HitSensor*);

Not placed here correctly

Code quote:

bool isSensorSimple(const HitSensor*);
bool isSensorLookAt(const HitSensor*);

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 174 at r5 (raw file):

bool isSensorBindableRouteDokan(const HitSensor*);
bool isSensorBindableBubblePadInput(const HitSensor*);
bool isSensorBindable(const HitSensor*);

Not placed here correctly

Code quote:

bool isSensorPlayerEye(const HitSensor*);

bool isSensorBindableGoal(const HitSensor*);
bool isSensorBindableAllPlayer(const HitSensor*);
bool isSensorBindableBubbleOutScreen(const HitSensor*);
bool isSensorBindableKoura(const HitSensor*);
bool isSensorBindableRouteDokan(const HitSensor*);
bool isSensorBindableBubblePadInput(const HitSensor*);
bool isSensorBindable(const HitSensor*);

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 601 at r5 (raw file):

bool tryReceiveMsgPushAndAddVelocityH(LiveActor*, const SensorMsg*, const HitSensor*,
                                      const HitSensor*, f32);
bool isMySensor(const HitSensor* sensor, const LiveActor* actor);

Lots of functions missing here

Code quote:

bool sendMsgPushAndKillVelocityToTarget(LiveActor*, HitSensor*, HitSensor*);
bool tryReceiveMsgPushAndAddVelocity(LiveActor*, const SensorMsg*, const HitSensor*,
                                     const HitSensor*, f32);
bool tryReceiveMsgPushAndAddVelocityH(LiveActor*, const SensorMsg*, const HitSensor*,
                                      const HitSensor*, f32);
bool isMySensor(const HitSensor* sensor, const LiveActor* actor);

---

src/MapObj/TransparentWall.cpp line 7 at r5 (raw file):

#include "Library/LiveActor/ActorCollisionFunction.h"
#include "Library/LiveActor/ActorInitUtil.h"
#include "Library/LiveActor/ActorPoseKeeper.h"

Useless include

---

lib/al/Library/MapObj/SeesawMapParts.cpp line 7 at r5 (raw file):

#include "Library/LiveActor/ActorInitUtil.h"
#include "Library/LiveActor/ActorModelFunction.h"
#include "Library/LiveActor/ActorPoseKeeper.h"

Useless include

[LynxDev2]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @MonsterDruide1)

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 16 at r5 (raw file):

Previously, MonsterDruide1 wrote…

Should probably not be placed in the header?

These macros are moved and changed in #415, I just copied the stuff from the other header for this PR

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 97 at r5 (raw file):

Previously, MonsterDruide1 wrote…

Return value used for example at 710009BB6C

Done.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 156 at r5 (raw file):

Previously, MonsterDruide1 wrote…

Not placed here correctly

This PR isn't meant for completely refactoring this header, it mainly copies the old stuff and I thought we didn't care about function ordering in the same TU yet

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 161 at r5 (raw file):

Previously, MonsterDruide1 wrote…

Not placed here correctly

Same thing

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 174 at r5 (raw file):

Previously, MonsterDruide1 wrote…

Not placed here correctly

Same thing

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 601 at r5 (raw file):

Previously, MonsterDruide1 wrote…

Lots of functions missing here

Same thing, should be added in the PR where they are implemented (one of my future PRs)

---

lib/al/Library/MapObj/SeesawMapParts.cpp line 7 at r5 (raw file):

Previously, MonsterDruide1 wrote…

Useless include

Done.

---

src/MapObj/TransparentWall.cpp line 7 at r5 (raw file):

Previously, MonsterDruide1 wrote…

Useless include

Done.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 22 at r5 (raw file):

class SensorMsg {
    SEAD_RTTI_BASE(SensorMsg);
};

This should probably be placed in the cpp which can be done in the other PR

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 105 at r5 (raw file):

void calcStrikeArrowCollideWallAndCeilingBetweenAttackSensor(const LiveActor*, const HitSensor*,
                                                             const HitSensor*,
                                                             const sead::Vector3f&, f32);

Done.

[LynxDev2]

Reviewable status: 53 of 56 files reviewed, 10 unresolved discussions (waiting on @MonsterDruide1)

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 22 at r5 (raw file):

Previously, LynxDev2 wrote…

This should probably be placed in the cpp which can be done in the other PR

Ig it can be added to the header since it's a virtual function, I'll add that to the other PR since that once places this class in another header

[MonsterDruide1]

Reviewed 3 of 3 files at r6, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @LynxDev2)

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 16 at r5 (raw file):

Previously, LynxDev2 wrote…

These macros are moved and changed in #415, I just copied the stuff from the other header for this PR

If you will change it around in another PR later and it's not needed here, remove it for now, and introduce it "correctly" in the other PR.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 22 at r5 (raw file):

Previously, LynxDev2 wrote…

Ig it can be added to the header since it's a virtual function, I'll add that to the other PR since that once places this class in another header

It must be placed in the header, because it is lazy (we don't check for that yet, but if it's obvious, might as well do it right the first time). Either fix or entirely remove the class definition in this PR, we don't want to introduce wrong information into the repo if it's avoidable.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 156 at r5 (raw file):

Previously, LynxDev2 wrote…

This PR isn't meant for completely refactoring this header, it mainly copies the old stuff and I thought we didn't care about function ordering in the same TU yet

Well, it does show up as all-new-file, so might just be a full refactor then. About function order: There's no check in place, so no hard enforcement on doing it correctly, so I also don't thoroughly check that in reviews. However, for new-header-PRs, I look at the header and function list side-by-side, so things being placed incorrectly jump out very obviously, so I also point them out.

Regarding this and all other open comments below: As you're basically doing a refactor of this header, please just finish it entirely, so it can be seen as "done".

[LynxDev2]

Reviewable status: 50 of 58 files reviewed, 6 unresolved discussions (waiting on @Moddimation and @MonsterDruide1)

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 16 at r5 (raw file):

Previously, MonsterDruide1 wrote…

If you will change it around in another PR later and it's not needed here, remove it for now, and introduce it "correctly" in the other PR.

Done.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 22 at r5 (raw file):

Previously, MonsterDruide1 wrote…

It must be placed in the header, because it is lazy (we don't check for that yet, but if it's obvious, might as well do it right the first time). Either fix or entirely remove the class definition in this PR, we don't want to introduce wrong information into the repo if it's avoidable.

Done.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 156 at r5 (raw file):

Previously, MonsterDruide1 wrote…

Well, it does show up as all-new-file, so might just be a full refactor then. About function order: There's no check in place, so no hard enforcement on doing it correctly, so I also don't thoroughly check that in reviews. However, for new-header-PRs, I look at the header and function list side-by-side, so things being placed incorrectly jump out very obviously, so I also point them out.

Regarding this and all other open comments below: As you're basically doing a refactor of this header, please just finish it entirely, so it can be seen as "done".

Done.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 161 at r5 (raw file):

Previously, LynxDev2 wrote…

Same thing

Done

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 174 at r5 (raw file):

Previously, LynxDev2 wrote…

Same thing

Done

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 601 at r5 (raw file):

Previously, LynxDev2 wrote…

Same thing, should be added in the PR where they are implemented (one of my future PRs)

Done

[MonsterDruide1]

Reviewed 8 of 8 files at r7, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @LynxDev2)

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 22 at r5 (raw file):

Previously, LynxDev2 wrote…

Done.

Unchanged

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 573 at r7 (raw file):

bool isMsgKickStoneTrampleForCrossoverSensor(const SensorMsg* msg, const HitSensor*,
                                             const HitSensor*);
bool isSensorPlayer(const HitSensor*);

Suggestion:

bool isMsgKickStoneTrampleForCrossoverSensor(const SensorMsg* msg, const HitSensor*,
                                             const HitSensor*);
// missing `sendMsgEnemyAttackForCrossoverX` (2 functions)
bool isSensorPlayer(const HitSensor*);

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 594 at r7 (raw file):

bool isSensorHitAnyPlane(const HitSensor*, const HitSensor*, const sead::Vector3f&);
bool isSensorHitRingShape(const HitSensor*, const HitSensor*, f32);
void tryGetEnemyAttackFireMaterialCode(const char**, const SensorMsg*);

Suggestion:

bool tryGetEnemyAttackFireMaterialCode(const char**, const SensorMsg*);

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 599 at r7 (raw file):

void pushAndAddVelocity(LiveActor*, const HitSensor*, const HitSensor*, f32);
void pushAndAddVelocityH(LiveActor*, const HitSensor*, const HitSensor*, f32);
void pushAndAddVelocityV(LiveActor*, const HitSensor*, const HitSensor*, f32);

Suggestion:

bool sendMsgPushAndKillVelocityToTarget(LiveActor*, HitSensor*, HitSensor*);
bool sendMsgPushAndKillVelocityToTargetH(LiveActor*, HitSensor*, HitSensor*);
bool pushAndAddVelocity(LiveActor*, const HitSensor*, const HitSensor*, f32);
bool pushAndAddVelocityH(LiveActor*, const HitSensor*, const HitSensor*, f32);
bool pushAndAddVelocityV(LiveActor*, const HitSensor*, const HitSensor*, f32);

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 607 at r7 (raw file):

                                       const HitSensor*, const HitSensor*, f32);
void sendMsgCollidePush(HitSensor*, HitSensor*, const sead::Vector3f&);
void tryReceiveMsgCollidePush(sead::Vector3f*, const SensorMsg*);

Suggestion:

bool tryReceiveMsgPushAndAddVelocity(LiveActor*, const SensorMsg*, const HitSensor*,
                                     const HitSensor*, f32);
bool tryReceiveMsgPushAndAddVelocityH(LiveActor*, const SensorMsg*, const HitSensor*,
                                      const HitSensor*, f32);
bool tryReceiveMsgPushAndCalcPushTrans(sead::Vector3f*, const SensorMsg*, const LiveActor*,
                                       const HitSensor*, const HitSensor*, f32);
bool sendMsgCollidePush(HitSensor*, HitSensor*, const sead::Vector3f&);
bool tryReceiveMsgCollidePush(sead::Vector3f*, const SensorMsg*);

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 614 at r7 (raw file):

void getMsgStringV4fSensorPtr(const SensorMsg*, sead::Vector4f, HitSensor**);
void getMsgStringVoidPtr(const SensorMsg*, void**);
void getPlayerReleaseEquipmentGoalType(const SensorMsg*)

(hopefully fixes the pipeline)

Suggestion:

void getPlayerReleaseEquipmentGoalType(const SensorMsg*);

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 614 at r7 (raw file):

void getMsgStringV4fSensorPtr(const SensorMsg*, sead::Vector4f, HitSensor**);
void getMsgStringVoidPtr(const SensorMsg*, void**);
void getPlayerReleaseEquipmentGoalType(const SensorMsg*)

Suggestion:

float getChangeAlphaValue(const SensorMsg*);
u32 getBindInitType(const SensorMsg*);
const char* getMsgString(const SensorMsg*);
const char* getMsgStringV4fPtr(const SensorMsg*, sead::Vector4f**);
const char* getMsgStringV4fSensorPtr(const SensorMsg*, sead::Vector4f, HitSensor**);
const char* getMsgStringVoidPtr(const SensorMsg*, void**);
u32 getPlayerReleaseEquipmentGoalType(const SensorMsg*)

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 623 at r7 (raw file):

    static getAttackSensor(const HitSensor*, s32);
    static findNearestAttackSensor(const HitSensor*);
};

... or should it be friend with HitSensor? If yes, then ignore the namespace part of this suggestion.

Suggestion:

namespace AttackSensorFunction {
    u16 getAttackSensorNum(const HitSensor*);
    u16 getAttackSensorNumMax(const HitSensor*);
    al::HitSensor* getAttackSensor(const HitSensor*, s32);
    al::HitSensor* findNearestAttackSensor(const HitSensor*);
};

[LynxDev2]

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @MonsterDruide1)

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 623 at r7 (raw file):

Previously, MonsterDruide1 wrote…

... or should it be friend with HitSensor? If yes, then ignore the namespace part of this suggestion.

Might be. The reason I made it a class is that I'm pretty confident that's what the devs did: normally al namespaces are all prefixed with "al", I think what happened here is that they meant to put a class inside namespace al, but accidentally put it in the global namespace

[MonsterDruide1]

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @LynxDev2)

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 623 at r7 (raw file):

Previously, LynxDev2 wrote…

Might be. The reason I made it a class is that I'm pretty confident that's what the devs did: normally al namespaces are all prefixed with "al", I think what happened here is that they meant to put a class inside namespace al, but accidentally put it in the global namespace

But why should this be a function? I would rather argue that they just forgot the al prefix on their naming, because almost no [...]Function exist within al, so this should've been namespace alAttackSensorFunction instead.

[LynxDev2]

Reviewable status: 41 of 59 files reviewed, 8 unresolved discussions (waiting on @Moddimation and @MonsterDruide1)

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 22 at r5 (raw file):

Previously, MonsterDruide1 wrote…

Unchanged

Done.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 614 at r7 (raw file):

Previously, MonsterDruide1 wrote…

(hopefully fixes the pipeline)

Done.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 623 at r7 (raw file):

Previously, MonsterDruide1 wrote…

But why should this be a function? I would rather argue that they just forgot the al prefix on their naming, because almost no [...]Function exist within al, so this should've been namespace alAttackSensorFunction instead.

I think accidentally putting it outside a specific scope is easier than forgetting a prefix. Regardless, I don't think we need to follow what we think the original code had too closely so the deicing factor is if it should be a friend class. It doesn't set any HitSensor fields, but would require at least four new getters in HitSensor, so not sure.

I decided to make it a namespace for now, but if you think it should be a friend class based on what I said above, let me know in this discussion and I'll change it in the upcoming PR that implements this namespace/class

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 573 at r7 (raw file):

bool isMsgKickStoneTrampleForCrossoverSensor(const SensorMsg* msg, const HitSensor*,
                                             const HitSensor*);
bool isSensorPlayer(const HitSensor*);

Done.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 594 at r7 (raw file):

bool isSensorHitAnyPlane(const HitSensor*, const HitSensor*, const sead::Vector3f&);
bool isSensorHitRingShape(const HitSensor*, const HitSensor*, f32);
void tryGetEnemyAttackFireMaterialCode(const char**, const SensorMsg*);

Done.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 599 at r7 (raw file):

void pushAndAddVelocity(LiveActor*, const HitSensor*, const HitSensor*, f32);
void pushAndAddVelocityH(LiveActor*, const HitSensor*, const HitSensor*, f32);
void pushAndAddVelocityV(LiveActor*, const HitSensor*, const HitSensor*, f32);

Done.

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 607 at r7 (raw file):

                                       const HitSensor*, const HitSensor*, f32);
void sendMsgCollidePush(HitSensor*, HitSensor*, const sead::Vector3f&);
void tryReceiveMsgCollidePush(sead::Vector3f*, const SensorMsg*);

Done.

[MonsterDruide1]

Reviewed 18 of 18 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @LynxDev2)

---

lib/al/Library/LiveActor/ActorSensorUtil.h line 623 at r7 (raw file):

Previously, LynxDev2 wrote…

I think accidentally putting it outside a specific scope is easier than forgetting a prefix. Regardless, I don't think we need to follow what we think the original code had too closely so the deicing factor is if it should be a friend class. It doesn't set any HitSensor fields, but would require at least four new getters in HitSensor, so not sure.

I decided to make it a namespace for now, but if you think it should be a friend class based on what I said above, let me know in this discussion and I'll change it in the upcoming PR that implements this namespace/class

We'll see once those functions are implemented.

[LynxDev2]

Rebased and made more fixes for new actors

[MonsterDruide1]

Reviewed 5 of 5 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @LynxDev2)