Fix Broken Giant's Knife flag not resetting

[JordanLongstaff]

Fixes #4601

Added an enhancement hook for custom checks on the Biggoron Sword/Giant's Knife.
The vanilla code mistakenly checks that you already have all of the previous swords (Kokiri, Master etc.) as well as the Broken Giant's Knife; if any one of them is missing, it won't touch the flag.
This shouldn't affect a vanilla playthrough since you get both of them first anyway (unless you use glitches or hack the save file), but the enhancement has been added to allow checking ONLY the Giant's Knife and not caring about the other two swords.

giantsknife.mp4

Build Artifacts

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

[Pepper0ni]

I would like it if it were possible to use the enhancement to buy a new knife even if you are already in the bugged state where your knife is broken but appears whole in the icon, so that broken files can be repaired without the save editor.

[garrettjoecox]

I'm not really familiar with the bug at hand, but this doesn't sound like something that should be an enhancement, but should just be the default behavior while in rando. It also probably makes more sense more integrated in the rando item giving process rather than hooking in afterwards from mods.cpp, or just wrapping the condition you want to override in a VB_ should.

[Pepper0ni]

While I think the sword check doesn't really need to be a fix enhancment on it's own, the sword icon issue that makes it impossible to buy a new knife after the second breaks does sound like something worth patching, and having that enhancment also cover the erronous sword check makes sense.

[garrettjoecox]

If we can reliably detect that a rando save is in this state, can we can just fix it in a save migration?

[Pepper0ni]

While it takes glitching to get there, and this is undoubtedly super niche I'm not a fan of rando only fixes unless it's a direct consequence of rando mechanics (which this is not, because you can glitch to skip ksword and it will trigger), but I understand the settings bloat argument.

In the mid-long term I can minor settings and fixes like this being consolidated similar to how the cut-scene skips have "Skip Misc Interactions" as a better solution. If we say something like that is planned I will withdrawal objections to making this rando only until we have a few more fixes to make this setting with.

[JordanLongstaff]

Can we sum up the requests for this PR?
I chose to fix it because it's an issue that someone logged and it clearly had a detrimental impact on their experience. But I also needed context because someone playing it the vanilla way with no glitches would supposedly never be affected.
So if this doesn't belong with the other fixes, that's fine. But it's not rando-only (if it was, I'd have put it with the rando hook handlers) because it also applies with skip glitches.

I don't agree with just patching a save or allowing the bugged state to still exist. I wanted to address the root cause of the bugged state's existence, which is the flag not resetting.

One question I have is, why does the rando part of the fix only bypass the Kokiri Sword check? The Master Sword can also be shuffled and I imagine it's very possible to not have it either.

[garrettjoecox]

I don't agree with just patching a save

I didn’t mean to imply only the save patching, I meant that as a better way to “fix existing people’s saves that are broken” rather than add two different changes, one to fix the bug and one to handle existing broken saves, which would probably just get stuck in our codebase forever (hence me thinking a save migration is probably a better spot for this)

Knowing it’s plausible to get into this state in vanilla, but that we’d basically always want it to be fixed in rando, let’s keep the enhancement and force it on for rando.

One question I have is, why does the rando part of the fix only bypass the Kokiri Sword check? The Master Sword can also be shuffled and I imagine it's very possible to not have it either.

I agree. it should just bypass both.

[Archez]

“fix existing people’s saves that are broken”

Considering the only way to have a broken vanilla save is through intentional glitching, is it even worth doing a save migration? Especially considering that it would mean migration behavior is tied to a user toggle.
And for rando saves I don't even think its worth it either since rando saves are generally short lived.

[JordanLongstaff]

“fix existing people’s saves that are broken”

There's one part of this I agree with: if someone breaks their Giant's Knife, buys a new one (and the flag doesn't get reset), then enables this enhancement, it'll be too late.
I don't have the brain power to find the fix for that right now, but it should be included in this PR.

[Malkierian]

Have you been able to revisit this for the save restoration functionality?