Player: Implement PlayerPowerGlove and PlayerSword

[GRAnimated]

These are unused classes with very similar code. Neither of them seem particularly interesting tbh

---

This change is 

[MonsterDruide1]

Reviewed 2 of 6 files at r1, all commit messages.
Reviewable status: 2 of 6 files reviewed, 9 unresolved discussions (waiting on @GRAnimated)

---

src/Player/PlayerSword.h line 20 at r1 (raw file):

    al::HitSensor* mBodySensor = nullptr;
    const sead::Matrix34f* mMtx = nullptr;
    bool field_120 = false;

Suggestion:

    al::LiveActor* mPlayer = nullptr;
    al::HitSensor* mPlayerBodySensor = nullptr;
    const sead::Matrix34f* mPlayerBaseMtx = nullptr;
    bool mIsInvisible = false;

---

src/Player/PlayerPowerGlove.cpp line 36 at r1 (raw file):

}

void PlayerPowerGlove::updatePose() {

Given
https://decomp.me/scratch/NESlb
and

OdysseyDecomp/src/MapObj/WorldMapParts.cpp

Line 27 in 1846eb7

void WorldMapParts::updatePose() {

... you should be able to come up with something better here.

---

src/Player/PlayerPowerGlove.cpp line 53 at r1 (raw file):

void PlayerPowerGlove::control() {
    if (al::updateSyncHostVisible(&field_120, this, mOther, 0)) {

Suggestion:

if (al::updateSyncHostVisible(&field_120, this, mOther, false)) {

---

src/Player/PlayerSword.cpp line 44 at r1 (raw file):

}

void PlayerSword::updatePose() {

Same, that's probably improvable

---

src/Player/PlayerSword.cpp line 61 at r1 (raw file):

void PlayerSword::control() {
    if (al::updateSyncHostVisible(&field_120, this, mOther, 0)) {

Suggestion:

if (al::updateSyncHostVisible(&field_120, this, mOther, false)) {

---

src/Player/PlayerPowerGlove.h line 18 at r1 (raw file):

private:
    al::LiveActor* mOther = nullptr;

We have almost no information about it, only that the visibility of the power glove is copied from the "other one" - so I would assume that it was intended to be the player itself.

Suggestion:

al::LiveActor* mPlayer = nullptr;

---

src/Player/PlayerPowerGlove.h line 19 at r1 (raw file):

private:
    al::LiveActor* mOther = nullptr;
    al::HitSensor* mBodySensor = nullptr;

Suggestion:

al::HitSensor* mPlayerBodySensor = nullptr;

---

src/Player/PlayerPowerGlove.h line 20 at r1 (raw file):

    al::LiveActor* mOther = nullptr;
    al::HitSensor* mBodySensor = nullptr;
    const sead::Matrix34f* mMtx = nullptr;

Suggestion:

const sead::Matrix34f* mPlayerBaseMtx = nullptr;

---

src/Player/PlayerPowerGlove.h line 21 at r1 (raw file):

    al::HitSensor* mBodySensor = nullptr;
    const sead::Matrix34f* mMtx = nullptr;
    bool field_120 = false;

updateSyncHostVisible always updates it to the opposite of the return type, so it's true when both should be invisible

Suggestion:

bool mIsInvisible = false;

[GRAnimated]

I also renamed weaponTypes to sWeaponTypes for consistency since its static

Reviewable status: 2 of 6 files reviewed, 9 unresolved discussions (waiting on @MonsterDruide1)

---

src/Player/PlayerPowerGlove.h line 18 at r1 (raw file):

Previously, MonsterDruide1 wrote…

We have almost no information about it, only that the visibility of the power glove is copied from the "other one" - so I would assume that it was intended to be the player itself.

Done.

---

src/Player/PlayerPowerGlove.h line 21 at r1 (raw file):

Previously, MonsterDruide1 wrote…

updateSyncHostVisible always updates it to the opposite of the return type, so it's true when both should be invisible

Done.

---

src/Player/PlayerPowerGlove.cpp line 36 at r1 (raw file):

Previously, MonsterDruide1 wrote…

Given
https://decomp.me/scratch/NESlb
and

OdysseyDecomp/src/MapObj/WorldMapParts.cpp

Line 27 in 1846eb7

void WorldMapParts::updatePose() {

... you should be able to come up with something better here.

I don't think it's actually an inlined function here. Making a local function in both PlayerPowerGlove and PlayerSword with the same name causes a linker error from a duplicate symbol. I also don't think there's enough evidence that the two matrices get multiplied together.

---

src/Player/PlayerSword.cpp line 44 at r1 (raw file):

Previously, MonsterDruide1 wrote…

Same, that's probably improvable

Same answer as in PlayerPowerGlove

---

src/Player/PlayerSword.h line 20 at r1 (raw file):

    al::HitSensor* mBodySensor = nullptr;
    const sead::Matrix34f* mMtx = nullptr;
    bool field_120 = false;

Done.

---

src/Player/PlayerSword.cpp line 61 at r1 (raw file):

void PlayerSword::control() {
    if (al::updateSyncHostVisible(&field_120, this, mOther, 0)) {

Done.

---

src/Player/PlayerPowerGlove.h line 19 at r1 (raw file):

private:
    al::LiveActor* mOther = nullptr;
    al::HitSensor* mBodySensor = nullptr;

Done.

---

src/Player/PlayerPowerGlove.h line 20 at r1 (raw file):

    al::LiveActor* mOther = nullptr;
    al::HitSensor* mBodySensor = nullptr;
    const sead::Matrix34f* mMtx = nullptr;

Done.

---

src/Player/PlayerPowerGlove.cpp line 53 at r1 (raw file):

void PlayerPowerGlove::control() {
    if (al::updateSyncHostVisible(&field_120, this, mOther, 0)) {

Done.

[MonsterDruide1]

Good catch, thanks!

Reviewed all commit messages.
Reviewable status: 2 of 6 files reviewed, 2 unresolved discussions (waiting on @GRAnimated)

---

src/Player/PlayerPowerGlove.cpp line 36 at r1 (raw file):

Previously, GRAnimated wrote…

I don't think it's actually an inlined function here. Making a local function in both PlayerPowerGlove and PlayerSword with the same name causes a linker error from a duplicate symbol. I also don't think there's enough evidence that the two matrices get multiplied together.

If that removes the "magic matrix" given here, that's a good improvement. If an inline function is not necessary, you should avoid it, do the calculations in the function itself then.

[GRAnimated]

Reviewable status: 2 of 6 files reviewed, 2 unresolved discussions (waiting on @MonsterDruide1)

---

src/Player/PlayerPowerGlove.cpp line 36 at r1 (raw file):

Previously, MonsterDruide1 wrote…

If that removes the "magic matrix" given here, that's a good improvement. If an inline function is not necessary, you should avoid it, do the calculations in the function itself then.

Done.

[MonsterDruide1]

Reviewed all commit messages.
Reviewable status: 2 of 6 files reviewed, 2 unresolved discussions (waiting on @GRAnimated)

---

src/Player/PlayerPowerGlove.cpp line 36 at r1 (raw file):

Previously, GRAnimated wrote…

Done.

No, there's still the magic matrix around here.

[MonsterDruide1]

Requires rebase