Vanilla bugfix, ladder cutscene softlock

[djevangelia]

Fix of vanilla softlock on ladder dismount during cutscene or using cutscene items (with restricted items glitch).

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.

I have also written in more detail here https://github.com/djevangelia/ootmm-bugfix/blob/main/oot%20ladder%20cutscene%20softlock.c, with addition that this also concerns unk_6AD = 4 (cutscene items).

This fix does not affect normal or glitched gameplay, this bug is purely a softlock.

Demonstration of softlock and fix in SoH: https://www.youtube.com/watch?v=9RazVt8pKeQ

I've also submitted naming the Player_Action_DismountLadder function etc to decomp zeldaret/oot#2771 but reviews there are generally going slow.

Build Artifacts

• soh-linux.zip
• soh-mac.zip
• soh-windows.zip

[Pepper0ni]

While this is a purely positive fix, it should probably still be behind a setting, as unmodified Ship is supposed to be a totally vanilla experience, warts and all.

There are exceptions to this rule, notably hookshot as child crash, but generally those a combination of difficult to trigger by accident, hard to fix/re-break in that particular case, and purely destructive. This case only covers the latter, so I do not image it would get an exception, but if a more senior dev wants to overrule I would not mind.

[serprex]

thinking we might group this & other soft lock behind shared setting. maybe can review existing settings in case there's room for more consolidation

[djevangelia]

Thanks! My interest is primarily working with decomp/vanilla and porting vanilla fixes, so if there should be a setting, once it's been decided upon just let me know how to put it in SoH and I'll change it.

I could see the case to make cutscene items (unk_6AD 4) a special case in this bugfix, as it allows use of Farore's Wind with ladder restricted items glitch (all CS items of course softlock, but FW is not usable after dismount). Depends on one's view on bugfixes and glitches - one could argue that FW should work, because the softlock is a bug; or, that like in vanilla FW should not be possible, even if the softlock is fixed. I can look into ways of prohibiting FW if needed.

[djevangelia]

Did some testing and this doesn't affect the ability to use Farore's Wind, it still works when dismounting down, it now just doesn't softlock when dismounting up but doesn't use the item. So to my knowledge now no gameplay impact.

[garrettjoecox]

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)) {

[djevangelia]

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

[garrettjoecox]

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)) {

[djevangelia]

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.)

[djevangelia]

I tried as a less invasive fix, appending after GameInteractor_ExecuteOnPlayerUpdate():

if (this->unk_6AD > 2 && this->actionFunc == Player_Action_DismountLadder) {
    this->unk_6AD = this->csAction = 0;
}

This works, but then you can move during the cutscene because you aren't in a cutscene state anymore, and to fix that you need to add something else. (Without the action function condition, this did softlock at least scene intro cutscenes and cutscene talking - so it's kind of invasive)
Also, trying to cast FW when dismounting can give a weird teleporting animation (from the place on the ladder where player gets softlocked).

Only setting unk_6AD is not enough, because it will get re-set to 3 every player update because csAction is not 0.
One must either remove the cutscene state variables and fix those consequences, or make DismountLadder action work regardless of it.
So, I don't think tinkering in a more global scope would be efficient or clean.