Library/MapObj: Implement ConveyerStep

[Fuzzy2319]

This PR adds ConveyerStep functions.

---

This change is 

[MonsterDruide1]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Fuzzy2319)

---

lib/al/Library/MapObj/ConveyerStep.cpp line 56 at r1 (raw file):

__attribute__((noinline)) void ConveyerStep::setTransByCoord(f32 coord, bool isBackwards,
                                                             bool isForceReset) {

Suggestion:

void ConveyerStep::setTransByCoord(f32 coord, bool isBackwards, bool isForceReset) {

---

lib/al/Library/MapObj/ConveyerStep.cpp line 84 at r1 (raw file):

    if ((isBackwards && newCoord < mCurrentCoord) || (!isBackwards && newCoord > mCurrentCoord) ||
        isForceReset)
        resetPosition(this);

I'm not sure if the isBackwards name is a good match for the logic here? Should it always resetPosition when moving in the "right" direction?

Code quote:

    if ((isBackwards && newCoord < mCurrentCoord) || (!isBackwards && newCoord > mCurrentCoord) ||
        isForceReset)
        resetPosition(this);

---

lib/al/Library/MapObj/ConveyerStep.h line 18 at r1 (raw file):

    void setConveyerKeyKeeper(const ConveyerKeyKeeper* conveyerKeyKeeper, f32 coord);
    void setTransByCoord(f32 coord, bool isBackwards);
    __attribute__((noinline)) void setTransByCoord(f32 coord, bool isBackwards, bool isForceReset);

Should not be necessary, because the function is large enough to not be inlined by default

Suggestion:

void setTransByCoord(f32 coord, bool isBackwards, bool isForceReset);

---

lib/al/Library/MapObj/ConveyerStep.h line 29 at r1 (raw file):

    const char* mActionName = nullptr;
    f32 mCurrentCoord = 0.0f;
    f32 mInitialCoord = 0.0f;

Suggestion:

f32 mMaxCoord = 0.0f;

[Fuzzy2319]

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

---

lib/al/Library/MapObj/ConveyerStep.h line 18 at r1 (raw file):

Previously, MonsterDruide1 wrote…

Should not be necessary, because the function is large enough to not be inlined by default

Done.

---

lib/al/Library/MapObj/ConveyerStep.cpp line 84 at r1 (raw file):

Previously, MonsterDruide1 wrote…

I'm not sure if the isBackwards name is a good match for the logic here? Should it always resetPosition when moving in the "right" direction?

Yes, I've inverted the comparison. I read mMoveSpeed <= 0.0f instead of mMoveSpeed >= 0.0f

Done.

---

lib/al/Library/MapObj/ConveyerStep.cpp line 56 at r1 (raw file):

__attribute__((noinline)) void ConveyerStep::setTransByCoord(f32 coord, bool isBackwards,
                                                             bool isForceReset) {

Done.

---

lib/al/Library/MapObj/ConveyerStep.h line 29 at r1 (raw file):

    const char* mActionName = nullptr;
    f32 mCurrentCoord = 0.0f;
    f32 mInitialCoord = 0.0f;

Done.

[MonsterDruide1]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Fuzzy2319)