item: Implement CoinRotateCalculator

[german77]

This branch has been sitting for a few weeks. I can't match the update function.

---

This change is 

[MonsterDruide1]

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

---

src/Item/CoinRotateCalculator.h line 20 at r1 (raw file):

private:
    f32 getObjAngle(bool isWater, s32 index) const;

That results in the function showing up with symbol in the executable, see tools/listsym | grep CoinRotateCalculator. If you declare a local helper function that does not show up in the executable, you need to keep it in the cpp file only, and mark it as inline (inline f32 getObjAngle(CoinRotateCalculator* self, bool isWater, s32 index) {...})

---

src/Item/CoinRotateCalculator.h line 25 at r1 (raw file):

    f32 mRotate = 0.0f;
    f32 mLastObjAngle = 0.0f;
    s32 mCount = 0;

Suggestion:

s32 mForceFrames = 0;

---

src/Item/CoinRotateCalculator.cpp line 17 at r1 (raw file):

inline f32 convergeDegree(f32 value) {
    return al::modf(value + 360.0f, 360.0f) + 0.0f;
}

It's not converging (approaching a constant value), so that name is not quite representative here

Suggestion:

inline f32 modDegree(f32 value) {
    return al::modf(value + 360.0f, 360.0f) + 0.0f;
}

---

src/Item/CoinRotateCalculator.cpp line 37 at r1 (raw file):

}

// NON_MATCHING: mismatch in storing mLastObjAngle = objAngle; (two strs instead of stp)

If you leave a function as non-matching, please also attach a decomp.me link if possible, for example this:
https://decomp.me/scratch/aNkgJ

Code quote:

// NON_MATCHING: mismatch in storing mLastObjAngle = objAngle; (two strs instead of stp)

---

src/Item/CoinRotateCalculator.cpp line 53 at r1 (raw file):

    } else {
        mForceOffset += mCount * 0.3f;
    }

Order of branches just like they should be executed: First, the force contains something, then the counter needs to tick back down, and finally the standard case without force.

Suggestion:

    } else if (--mCount > 0) {
        mForceOffset += mCount * 0.3f;
    } else {
        mForceOffset = convergeDegree(mForceOffset) - 0.8f;
        if (mForceOffset < 0.0f)
            mForceOffset = 0.0f;
    }

---

src/Item/CoinRotateCalculator.cpp line 75 at r1 (raw file):

    if (!al::isInWater(mActor))
        return 3.0f;
    return 2.0f;

Suggestion:

    return al::isInWater(mActor) ? 2.0f : 3.0f;

[german77]

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

---

src/Item/CoinRotateCalculator.h line 20 at r1 (raw file):

Previously, MonsterDruide1 wrote…

That results in the function showing up with symbol in the executable, see tools/listsym | grep CoinRotateCalculator. If you declare a local helper function that does not show up in the executable, you need to keep it in the cpp file only, and mark it as inline (inline f32 getObjAngle(CoinRotateCalculator* self, bool isWater, s32 index) {...})

Done.

---

src/Item/CoinRotateCalculator.cpp line 17 at r1 (raw file):

Previously, MonsterDruide1 wrote…

It's not converging (approaching a constant value), so that name is not quite representative here

Done.

---

src/Item/CoinRotateCalculator.cpp line 37 at r1 (raw file):

Previously, MonsterDruide1 wrote…

If you leave a function as non-matching, please also attach a decomp.me link if possible, for example this:
https://decomp.me/scratch/aNkgJ

Done.

---

src/Item/CoinRotateCalculator.cpp line 53 at r1 (raw file):

Previously, MonsterDruide1 wrote…

Order of branches just like they should be executed: First, the force contains something, then the counter needs to tick back down, and finally the standard case without force.

Not equivalent. Produces different code.

---

src/Item/CoinRotateCalculator.h line 25 at r1 (raw file):

    f32 mRotate = 0.0f;
    f32 mLastObjAngle = 0.0f;
    s32 mCount = 0;

Done.

---

src/Item/CoinRotateCalculator.cpp line 75 at r1 (raw file):

    if (!al::isInWater(mActor))
        return 3.0f;
    return 2.0f;

Done.

[MonsterDruide1]

Reviewed 2 of 3 files at r2, all commit messages.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @german77)

---

src/Item/CoinRotateCalculator.cpp line 53 at r1 (raw file):

Previously, german77 (Narr the Reg) wrote…

Not equivalent. Produces different code.

In the current diff, you flipped the condition from <= to >, but not the two branches... something about this is wrong.