ObjectDrivable infrastructure, rDKM bridge, MT net, RR wavy road, BCWii Twisted Way #251
Conversation
| #include "ObjectTwistedWay.hh" | ||
|
|
||
| #include "game/field/CollisionDirector.hh" | ||
| #include "game/system/RaceManager.hh" |
There was a problem hiding this comment.
| #include "game/system/RaceManager.hh" | |
| #include "game/system/RaceManager.hh" |
| const EGG::Vector3f &relativePos, CollisionInfo *pInfo, KCLTypeMask *pFlagsOut, | ||
| bool full, bool push); | ||
|
|
||
| f32 m_ac = 1000.0f; |
There was a problem hiding this comment.
This is my interpretation
| f32 m_ac = 1000.0f; | |
| u32 m_preRaceTimer = 1000; |
| /// @addr{0x80813cfc} | ||
| void ObjectTwistedWay::calc() { | ||
| if (!System::RaceManager::Instance()->isStageReached(System::RaceManager::Stage::Race)) { | ||
| m_ac++; |
There was a problem hiding this comment.
| m_ac++; | |
| ++m_preRaceTimer; |
Prefer prefix incrementor for consistency with other files.
| /// @addr{0x80813c40} | ||
| void ObjectTwistedWay::init() {} | ||
|
|
||
| /// @addr{0x80813cfc} |
There was a problem hiding this comment.
| /// @addr{0x80813cfc} | |
| /// @addr{0x80813CFC} |
| /// @addr{0x80814918} | ||
| ObjectTwistedWay::~ObjectTwistedWay() = default; | ||
|
|
||
| /// @addr{0x80813c40} |
There was a problem hiding this comment.
| /// @addr{0x80813c40} | |
| /// @addr{0x80813C40} |
| } | ||
|
|
||
| return true; | ||
| } else { |
There was a problem hiding this comment.
Personal preference, but you don't need an else indentation if you have a guaranteed return in the if-statement above.
There was a problem hiding this comment.
Completely missed that, mb
| if (!System::RaceManager::Instance()->isStageReached(System::RaceManager::Stage::Race)) { | ||
| frameCount = m_ac; | ||
| } else { | ||
| frameCount = System::RaceManager::Instance()->timer(); | ||
| } |
There was a problem hiding this comment.
Maybe overkill with the ternary. More importantly, we should only get the pointer here once. Writing the boolean out to a variable avoids recomputing it again in your next if-statement
| if (!System::RaceManager::Instance()->isStageReached(System::RaceManager::Stage::Race)) { | |
| frameCount = m_ac; | |
| } else { | |
| frameCount = System::RaceManager::Instance()->timer(); | |
| } | |
| auto *raceMgr = System::RaceManager::Instance(); | |
| bool raceStarted = raceMgr->isStageReached(System::RaceManager::Stage::Race); | |
| m_frameCount = raceStarted ? raceMgr->timer() : m_ac; |
There was a problem hiding this comment.
I've seen other cases in Kinoko where an instance getter is called multiple times in a function, for instance CollisionDirector gets re-fetched multiple times per iteration in a couple for loops in KartMove and KartCollide, and BoxColManager gets re-fetched multiple times in a while loop in ObjectDirector::checkKartObjectCollision. Maybe cleaning those up could be a PR?
| fnrm *= fnrm.dot(EGG::Vector3f(sinfidx, cosfidx, 0.0f)); | ||
| fnrm = relativePos - (fnrm + b8Y); | ||
|
|
||
| f32 dVar2 = radius + 227.0f - fnrm.length(); |
| return false; | ||
| } | ||
|
|
||
| (void)fnrm.normalise(); |
There was a problem hiding this comment.
| (void)fnrm.normalise(); | |
| fnrm.normalise(); |
| static constexpr s32 PHASE_COUNT = 120; | ||
|
|
||
| static constexpr f32 WIDTH = 2000.0f; | ||
| static constexpr f32 _B8 = 2000.0f; |
There was a problem hiding this comment.
Couple uses of _B8 still, but if it's the same as width, does it make sense to just use WIDTH instead? Or were these left over?
There was a problem hiding this comment.
I haven't really figured out what B8 does, but in-game testing doesn't seem to suggest it has anything to do with the width.
No description provided.