Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion soh/include/z64player.h
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,7 @@ typedef struct Player {
/* 0x06A4 */ f32 closestSecretDistSq;
/* 0x06A8 */ Actor* unk_6A8;
/* 0x06AC */ s8 idleType;
/* 0x06AD */ u8 unk_6AD;
/* 0x06AD */ u8 unk_6AD; // Camera related. 0 = normal, 1 = first person without weapon, 2 = first person with weapon, 3 = cutscene action, 4 = cutscene items
/* 0x06AE */ u16 unk_6AE_rotFlags; // See `UNK6AE_ROT_` macros. If its flag isn't set, a rot steps to 0.
/* 0x06B0 */ s16 upperLimbYawSecondary;
/* 0x06B2 */ char unk_6B4[0x004];
Expand Down
17 changes: 16 additions & 1 deletion soh/soh/Enhancements/AlwaysOnFixes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,19 @@ void RegisterFixOutsideTotCrash() {
});
}

static RegisterShipInitFunc initFunc(RegisterFixOutsideTotCrash, { "" });
// `Player_Action_DismountLadder`: Dismounting a ladder in a cutscene or when using
// a cutscene item (possible using restricted items glitch), i.e. `player->unk_6AD` is 3 or 4,
// causes the animation in `Player_Action_DismountLadder` to never finish because
// `Player_TryActionInterrupt` always returns true and player cannot exit the action.
// Fix: Disregard result of `Player_TryActionInterrupt` if `unk_6AD` >= 3.
void RegisterFixLadderCutsceneSoftlock() {
COND_VB_SHOULD(VB_LADDER_CUTSCENE_FLAG, true, {
u8* unk_6AD = va_arg(args, u8*);
if (*unk_6AD >= 3) {
*should = false;
}
});
}

static RegisterShipInitFunc initFuncFixOutsideTotCrash(RegisterFixOutsideTotCrash, { "" });
static RegisterShipInitFunc initFuncFixLadderCutsceneSoftlock(RegisterFixLadderCutsceneSoftlock, { "" });
Original file line number Diff line number Diff line change
Expand Up @@ -3056,6 +3056,14 @@ typedef enum {
// - `*EnItem00`
VB_ITEM00_KILL,

// #### `result`
// ```c
// false if `player->unk_6AD` >= 3
Comment thread
djevangelia marked this conversation as resolved.
Outdated
// ```
// #### `args`
// - `*u8 (&player->unk_6AD)`
VB_LADDER_CUTSCENE_FLAG,

// true
// ```
// #### `args`
Expand Down
61 changes: 34 additions & 27 deletions soh/src/overlays/actors/ovl_player_actor/z_player.c
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ void Player_Action_8084B9E4(Player* this, PlayState* play);
void Player_Action_8084BBE4(Player* this, PlayState* play);
void Player_Action_8084BDFC(Player* this, PlayState* play);
void Player_Action_8084BF1C(Player* this, PlayState* play);
void Player_Action_8084C5F8(Player* this, PlayState* play);
void Player_Action_DismountLadder(Player* this, PlayState* play);
void Player_Action_8084C760(Player* this, PlayState* play);
void Player_Action_8084C81C(Player* this, PlayState* play);
void Player_Action_8084CC98(Player* this, PlayState* play);
Expand Down Expand Up @@ -7600,8 +7600,8 @@ s32 func_8083EC18(Player* this, PlayState* play, u32 wallFlags) {
return false;
}

void func_8083F070(Player* this, LinkAnimationHeader* anim, PlayState* play) {
Player_SetupActionPreserveAnimMovement(play, this, Player_Action_8084C5F8, 0);
void Player_SetupDismountLadder(Player* this, LinkAnimationHeader* anim, PlayState* play) {
Player_SetupActionPreserveAnimMovement(play, this, Player_Action_DismountLadder, 0);
LinkAnimation_PlayOnceSetSpeed(play, &this->skelAnime, anim, (4.0f / 3.0f));
}

Expand Down Expand Up @@ -13322,7 +13322,7 @@ void Player_Action_8084BF1C(Player* this, PlayState* play) {
func_8083A9B8(this, &gPlayerAnim_link_normal_jump_climb_up_free, play);
this->stateFlags1 |= PLAYER_STATE1_CLIMBING_LEDGE;
} else {
func_8083F070(this, this->ageProperties->unk_CC[this->av2.actionVar2], play);
Player_SetupDismountLadder(this, this->ageProperties->unk_CC[this->av2.actionVar2], play);
}
} else {
this->skelAnime.prevTransl = this->ageProperties->unk_4A[sp68];
Expand All @@ -13336,7 +13336,7 @@ void Player_Action_8084BF1C(Player* this, PlayState* play) {
if (this->av2.actionVar2 != 0) {
this->skelAnime.prevTransl = this->ageProperties->unk_44;
}
func_8083F070(this, this->ageProperties->unk_C4[this->av2.actionVar2], play);
Player_SetupDismountLadder(this, this->ageProperties->unk_C4[this->av2.actionVar2], play);
this->av2.actionVar2 = 1;
}
} else {
Expand Down Expand Up @@ -13385,50 +13385,57 @@ void Player_Action_8084BF1C(Player* this, PlayState* play) {
}
}

static f32 D_80854898[] = { 10.0f, 20.0f };
static f32 D_808548A0[] = { 40.0f, 50.0f };
static f32 sUpDismountLadderFrames[] = { 10.0f, 20.0f };
static f32 sDownDismountLadderFrames[] = { 40.0f, 50.0f };

static AnimSfxEntry D_808548A8[] = {
static AnimSfxEntry sDownDismountLadderAnimSfx[] = {
{ NA_SE_PL_WALK_LADDER, ANIMSFX_DATA(ANIMSFX_TYPE_GENERAL, 10) },
{ NA_SE_PL_WALK_LADDER, ANIMSFX_DATA(ANIMSFX_TYPE_GENERAL, 20) },
{ NA_SE_PL_WALK_LADDER, -ANIMSFX_DATA(ANIMSFX_TYPE_GENERAL, 30) },
};

void Player_Action_8084C5F8(Player* this, PlayState* play) {
s32 temp;
f32* sp38;
CollisionPoly* sp34;
s32 sp30;
Vec3f sp24;
/**
* Dismounting ladders, both upwards and downwards.
* `actionVar2` true if dismounting down
*/
void Player_Action_DismountLadder(Player* this, PlayState* play) {
s32 interruptResult;
f32* frame;
CollisionPoly* groundPoly;
s32 bgId;
Vec3f raycastPos;

this->stateFlags2 |= PLAYER_STATE2_DISABLE_ROTATION_ALWAYS;

temp = Player_TryActionInterrupt(play, this, &this->skelAnime, 4.0f);
interruptResult = Player_TryActionInterrupt(play, this, &this->skelAnime, 4.0f);

if (temp == 0) {
// SoH: Check for `this->unk_6AD` >= 3 to fix ladder dismount softlock with cutscenes and restricted items.
// See `RegisterFixLadderCutsceneSoftlock()`
if (interruptResult == PLAYER_INTERRUPT_NEW_ACTION &&
GameInteractor_Should(VB_LADDER_CUTSCENE_FLAG, true, &this->unk_6AD)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The true here is where you would put the vanilla condition we are overriding. Additionally "Should ladder cutscene flag" does not read like it makes sense, I'm not sure this has anything to do with cutscenes. Maybe "Should remove climbing flag"?

Suggested change
if (interruptResult == PLAYER_INTERRUPT_NEW_ACTION &&
GameInteractor_Should(VB_LADDER_CUTSCENE_FLAG, true, &this->unk_6AD)) {
if (GameInteractor_Should(VB_REMOVE_CLIMBING_FLAG, interruptResult == PLAYER_INTERRUPT_NEW_ACTION, &this->unk_6AD)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it was my first PR after asking for some guidance which resulted in yolo committing... so the name is really bad. I'm changing it now.

(The climbing flag is not the issue, the problem is the early return. The function expects that a new action function has been set through the action handlers, which is not the case if cutscene or cutscene items. This results in player permanently stuck in Player_Action_DismountLadder because it needs the animation to finish to setup new action, but LinkAnimation_Update() runs after the early return.)

this->stateFlags1 &= ~PLAYER_STATE1_CLIMBING_LADDER;
return;
}

if ((temp > 0) || LinkAnimation_Update(play, &this->skelAnime)) {
if ((interruptResult >= PLAYER_INTERRUPT_MOVE) || LinkAnimation_Update(play, &this->skelAnime)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a change from upstream? If not lets restore the > even though it's functionally the same

Suggested change
if ((interruptResult >= PLAYER_INTERRUPT_MOVE) || LinkAnimation_Update(play, &this->skelAnime)) {
if ((interruptResult > PLAYER_INTERRUPT_NEW_ACTION) || LinkAnimation_Update(play, &this->skelAnime)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's upstream. But I tested now and interruptResult > PLAYER_INTERRUPT_NEW_ACTION matches (gc-mq-eu-debug) so changing it.

func_8083C0E8(this, play);
this->stateFlags1 &= ~PLAYER_STATE1_CLIMBING_LADDER;
return;
}

sp38 = D_80854898;
frame = sUpDismountLadderFrames;

if (this->av2.actionVar2 != 0) {
Player_ProcessAnimSfxList(this, D_808548A8);
sp38 = D_808548A0;
if (this->av2.actionVar2 != false) {
Player_ProcessAnimSfxList(this, sDownDismountLadderAnimSfx);
frame = sDownDismountLadderFrames;
}

if (LinkAnimation_OnFrame(&this->skelAnime, sp38[0]) || LinkAnimation_OnFrame(&this->skelAnime, sp38[1])) {
sp24.x = this->actor.world.pos.x;
sp24.y = this->actor.world.pos.y + 20.0f;
sp24.z = this->actor.world.pos.z;
if (BgCheck_EntityRaycastFloor3(&play->colCtx, &sp34, &sp30, &sp24) != 0.0f) {
this->floorSfxOffset = func_80041F10(&play->colCtx, sp34, sp30);
if (LinkAnimation_OnFrame(&this->skelAnime, frame[0]) || LinkAnimation_OnFrame(&this->skelAnime, frame[1])) {
raycastPos.x = this->actor.world.pos.x;
raycastPos.y = this->actor.world.pos.y + 20.0f;
raycastPos.z = this->actor.world.pos.z;
if (BgCheck_EntityRaycastFloor3(&play->colCtx, &groundPoly, &bgId, &raycastPos) != 0.0f) {
this->floorSfxOffset = func_80041F10(&play->colCtx, groundPoly, bgId);
Player_PlayLandingSfx(this);
}
}
Expand Down
Loading