Player: Implement PlayerStateJump - #1327
Conversation
github-actions
Bot
added
the
status:waiting for review
MonsterDruide1
left a comment
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 reviewed 5 files and all commit messages, and made 3 comments.
Reviewable status: 5 of 7 files reviewed, 3 unresolved discussions (waiting on c4nvm).
src/Player/PlayerStateJump.cpp line 106 at r1 (raw file):
} if ((u32)mJumpMessageRequest->jumpType == 0xf && !mJumpMessageRequest->isEnableStandUp) {
create enum values for all possible jump types, potentially with placeholder names
Suggestion:
mJumpMessageRequest->jumpType == PlayerJumpType::val_0Fsrc/Player/PlayerStateJump.cpp line 121 at r1 (raw file):
rs::scaleVelocityInertiaWallHit(actor, mCollider, 0.25f, 1.0f, getNormalMaxSpeed(mConst, mModelChanger));
TODO: continue below here
src/Player/PlayerStateJump.cpp line 124 at r1 (raw file):
u32 messageJumpBit = (u32)mJumpMessageRequest->jumpType - 1; bool isInMessageJumpRange = messageJumpBit < 0x10; bool isMessageJump = ((0x9001u >> (u16)messageJumpBit) & 1) && isInMessageJumpRange;
try to never cast jumpType to the raw integer.
For checking JumpRange, create an enum entry JumpType::MAX that can be used instead of a magical number.
For the shift-and-AND-ing, this is a list of allowed values: 0, 12, 15 that you need to check against (t == 0 || t == 12 || t == 15, with enum entries for these constants).
Code quote:
u32 messageJumpBit = (u32)mJumpMessageRequest->jumpType - 1;
bool isInMessageJumpRange = messageJumpBit < 0x10;
bool isMessageJump = ((0x9001u >> (u16)messageJumpBit) & 1) && isInMessageJumpRange;
github-actions
Bot
removed
the
status:waiting for review
MonsterDruide1
changed the title
PlayerStateJumpPlayerStateJump
github-actions
Bot
added
the
status:waiting for author
2 participants
status: 43 / 43 functions matching (was 37 / 43)
this closes the remaining six:
appear— the jump-type dispatch (1104 insns),an outer switch over types 12–18 whose cases each
setNerveand return (tail-merged into one call),then a shared gravity /
isMove/ velocity·gravity block,then an inner switch over types 1–17 whose default is the normal-jump and cap-catch path,
calcJumpPowerBorderSpeed{Min,Max},calcJumpPower{Min,Max}andcalcJumpGravityall inline into it to feedPlayerActionFunction::calcJumpSpeedcalcJumpPowerMax,calcJumpGravityandtryCountUpContinuousJump(previouslyWip)~PlayerStateJump(D0) — it only became emittable onceappear, the key function for this vtable, was definedheader changes
PlayerActionAirMoveControl: replacedset_42/44/48withsetMaxJumpSpeed(f32 minSpeed, f32 maxSpeed),the three field writes are a single inline,
and corrected
_44froms32tof32,appear's prologue stores it paired with_48(stp s0, s8, [x8, #0x44]), receiving the force-run speed, or0.0fwhen not force-running,also added
set_33(the "in 2D area" flagappearsets)PlayerActionVelocityControl:getVelocityFront()PlayerTrigger:EActionTrigger_val21, used byappearnotes for review
three constructs here exist for codegen reasons rather than clarity,
each is load-bearing so removing any one breaks the match,
but i'm happy to reshape them if there's a preferred house style:
sead::Vector3f dir = jumpDir;/lastJumpDircopies before each dot product,without them the six component loads interleave with the multiplies,
with them they group into
ldppairs as in the target,.dot()itself is fine —appear's other dot (al::getVelocity(actor).dot(gravity)) matches with the plain formbool isInMessageJumpRangeas its own statement,with the shift on the left of the
&&in the message-jump bitmask test,that ordering is what yields
and+tbnzwithcsetahead oflsrcalcVelocityFrontSpeed, a file-local helper (always inlined so it leaves no out-of-line symbol),used at the two sites whose result is consumed,
alongside a plain
PlayerActionVelocityControllocal in the cap-catch branch,allocas from an inlined function are hoisted to the top of the caller's entry block,
which is what places the merged velocity-control stack slot at
sp+0x38as in the target,using the helper in both branches of
if (isCapCatchJump)instead lets SimplifyCFG hoist the constructor above the branch,the cap-catch object being constructed and never read is also what the target does
verification
Player/PlayerStateJump.oreportOK,and
tools/check -ppasses for the newly matched onesclang-format(18, as pinned by the lint workflow) andtools/check-format.pycleanThis change is
Report for 1.0 (0de3c75 - d099663)
📈 Matched code: 15.47% (+0.11%, +14228 bytes)
✅ 43 new matches
Player/PlayerStateJumpPlayerStateJump::appear()Player/PlayerStateJumpPlayerStateJump::exeJump()Player/PlayerStateJumpPlayerStateJump::exeJumpSpinFlower()Player/PlayerStateJumpPlayerStateJump::exeJumpTurn()Player/PlayerStateJumpPlayerStateJump::exeHoveringJump2D()Player/PlayerStateJumpPlayerStateJump::exeJumpSpinGround()Player/PlayerStateJumpPlayerStateJump::exeJumpBack()Player/PlayerStateJumpPlayerStateJump::updateNerveDownFall(char const*, float, float, float, al::Nerve const*)Player/PlayerStateJumpPlayerStateJump::exeHovering2D()Player/PlayerStateJumpPlayerStateJump::PlayerStateJump(al::LiveActor*, PlayerConst const*, IPlayerModelChanger const*, IUseDimension const*, PlayerInput const*, PlayerJumpMessageRequest const*, IJudge const*, PlayerCounterForceRun const*, PlayerJointControlKeeper const*, IUsePlayerCollision const*, PlayerTrigger*, PlayerContinuousJump*, PlayerAnimator*, PlayerActionDiveInWater*, HackCap*, IJudge*, bool)Player/PlayerStateJumpPlayerStateJump::tryCountUpContinuousJump(PlayerContinuousJump*)Player/PlayerStateJumpPlayerStateJump::calcJumpAnimName() constPlayer/PlayerStateJumpPlayerStateJump::isEndJumpDownFallLand() constPlayer/PlayerStateJumpPlayerStateJump::calcJumpPowerMin() constPlayer/PlayerStateJumpPlayerStateJump::calcJumpPowerMax() constPlayer/PlayerStateJumpPlayerStateJump::exeJumpSpinGroundDownFall()Player/PlayerStateJump(anonymous namespace)::PlayerStateJumpNrvJumpSpinGroundDownFall::execute(al::NerveKeeper*) constPlayer/PlayerStateJumpPlayerStateJump::isFormSquat2D() constPlayer/PlayerStateJumpPlayerStateJump::isEnableHipDropStart() constPlayer/PlayerStateJumpPlayerStateJump::isEnableCancelCarryThrow() constPlayer/PlayerStateJumpPlayerStateJump::exeJumpSpinFlowerDownFall()Player/PlayerStateJump(anonymous namespace)::PlayerStateJumpNrvJumpSpinFlowerDownFall::execute(al::NerveKeeper*) constPlayer/PlayerStateJumpPlayerStateJump::isJumpCapCatch() constPlayer/PlayerStateJumpPlayerStateJump::calcJumpGravity() constPlayer/PlayerStateJumpPlayerStateJump::isEnableTrampleByHipDropAttack() constPlayer/PlayerStateJumpPlayerStateJump::calcJumpPowerBorderSpeedMin() constPlayer/PlayerStateJumpPlayerStateJump::calcJumpPowerBorderSpeedMax() constPlayer/PlayerStateJumpPlayerStateJump::isJumpSpinFlower() constPlayer/PlayerStateJumpPlayerStateJump::isJumpSpinGround() constPlayer/PlayerStateJumpPlayerStateJump::isHovering() const...and 13 more new matches