Skip to content

Use VB hook to fix Deku Nut upgrade bug#5047

Merged
Archez merged 6 commits intoHarbourMasters:developfrom
JordanLongstaff:fix-nut-upgrade-vb
Feb 20, 2025
Merged

Use VB hook to fix Deku Nut upgrade bug#5047
Archez merged 6 commits intoHarbourMasters:developfrom
JordanLongstaff:fix-nut-upgrade-vb

Conversation

@JordanLongstaff
Copy link
Contributor

@JordanLongstaff JordanLongstaff commented Feb 12, 2025

Alternative to #4777. Changes the retroactive flag fix from a wasteful frame update hook to a lazy VB hook.

I'd like some feedback on whether there's a difference between this and #4777 in terms of how they affect rando. Just in case, I've kept the IS_RANDO part of the hook.

Build Artifacts

Copy link
Contributor

@Archez Archez left a comment

Choose a reason for hiding this comment

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

Ok sorry for the split reviews 😅 just noticed two more things, one commented below, and the other is the following:

When not in a rando save while skip misc interations is on, we bypass the flow for mask judging which means your VB_DEKU_SCRUBS_REACT_TO_MASK_OF_TRUTH wont be triggered and therefore the "fix" wont take effect (unsetting the flag)

To counteract that, we can "cheat" by placing a GameInteractor_Should(VB_DEKU_SCRUBS_REACT_TO_MASK_OF_TRUTH, true) before the flag set in timesave_hook_handlers.cpp:EnDntDemo_JudgeSkipToReward

@JordanLongstaff
Copy link
Contributor Author

When not in a rando save while skip misc interations is on, we bypass the flow for mask judging which means your VB_DEKU_SCRUBS_REACT_TO_MASK_OF_TRUTH wont be triggered and therefore the "fix" wont take effect (unsetting the flag)

To counteract that, we can "cheat" by placing a GameInteractor_Should(VB_DEKU_SCRUBS_REACT_TO_MASK_OF_TRUTH, true) before the flag set in timesave_hook_handlers.cpp:EnDntDemo_JudgeSkipToReward

I was thinking another alternative would be to change this from a VB hook to an ActorInit hook. But that would leave the edge case of the option being turned on while Link is already in the Forest Stage.

Am I wrong on that? Do ActorInit hooks get called immediately upon registration if the corresponding actor is present?

@Archez
Copy link
Contributor

Archez commented Feb 12, 2025

I was thinking another alternative would be to change this from a VB hook to an ActorInit hook. But that would leave the edge case of the option being turned on while Link is already in the Forest Stage.

Yea using onActorInit would mean toggling after the actor has spawned requires a scene reload. As long as the tooltip entry for the fix calls that out, then thats fine to me. Trying to support live-toggling is not always worth the added complexity, IMO, and in this case might be for the best.

Copy link
Contributor

@Archez Archez left a comment

Choose a reason for hiding this comment

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

LGTM now 👍

@Archez Archez merged commit b6e2a99 into HarbourMasters:develop Feb 20, 2025
5 checks passed
@JordanLongstaff JordanLongstaff deleted the fix-nut-upgrade-vb branch February 21, 2025 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants