Quest Cleanup

[Pepe20129]

This PR replaces isMasterQuest, n64ddFlag & isBossRush checks with a new field (questId) and macros.
It also restores authentic n64ddFlag behaviour.
It doesn't change savefiles on disk which still use isMasterQuest & n64ddFlag because I don't think breaking save compatibility for something minor like this is worth it and it could be grouped with future changes to the save format.

Build Artifacts

• soh.otr.zip
• soh-linux-compatibility.zip
• soh-linux-performance.zip
• soh-mac.zip
• soh-switch.zip
• soh-wiiu.zip
• soh-windows.zip

[garrettjoecox]

Bravo. My main points of initial feedback would be:

Why a macro function instead of just assuming gSaveContext is available? Eg IS_RANDO vs IS_RANDO(gSaveContext)

Even though they won’t be used much, you should use macros for the quest IDs

[Pepe20129]

Why a macro function instead of just assuming gSaveContext is available? Eg IS_RANDO vs IS_RANDO(gSaveContext)

I did it because some places like SaveManager::SaveRandomizer use a pointer to the save context so it does IS_RANDO(*saveContext).

Even though they won’t be used much, you should use macros for the quest IDs

I didn't do that because the enum is in file_choose.h and I thought that it may not be the best idea to include it in z64save.h as z64 headers are included almost everywhere. If it's not a big deal I can do it.

[garrettjoecox]

I did it because some places like SaveManager::SaveRandomizer use a pointer to the save context so it does IS_RANDO(*saveContext).

That's fair, though how often do those special cases show up compared to not? Could we just do an inline comparison to the enum for the save manager case?

I didn't do that because the enum is in file_choose.h and I thought that it may not be the best idea to include it in z64save.h as z64 headers are included almost everywhere. If it's not a big deal I can do it.

I'd say just move the enum, no particular reason it needs to stay there right?

[Pepe20129]

I'd say just move the enum, no particular reason it needs to stay there right?

Moved it.

[Pepe20129]

That's fair, though how often do those special cases show up compared to not? Could we just do an inline comparison to the enum for the save manager case?

They're rare so I could just change it to assume gSaveContext and require manual checks when it's not the case

[Pepe20129]

Done, the macros no longer take gSaveContext and in the end there was only one place that didn't use it.

[garrettjoecox]

Great work again. Dropping an approval but I did have one question around the save manager stuff someone might be able to pipe in on.

[garrettjoecox]

I can't recall how we were supposed to treat changes like this, is it the correct behavior to make this change in LoadV1, LoadV2, LoadV3, and LoadV4? I think so?

[Pepe20129]

I think so as if this change wasn't made, older save files would not migrate their rando/master quest status correctly.

[briaguya-ai]

I'm pretty sure that's correct. I think we discussed having it be chained loading so loading V1 would mean V1 -> V2, V2 -> V3, V3 -> V4 but decided against it. Instead we have "this is how you load this old version of a save into the current save format"

[Malkierian]

Honestly, I feel like maybe the chain load would have been better in the end, but the only other option would be to make a new loader version and settings migrator, which would be overkill for a single thing like this. I'm OK with this method of handling it.