[ Common ] CMakePresets.json does not pick RelWithDebInfo by default

[ChthonVII]

Describe the bug
Some commit later than b6bf503 and not later than f7cb728 has badly broken the progression system in ways that first become very obvious at Wall Market. The gun-equipped vending machine hands out the Premium Heart the first time it's encountered. The weapon shop owner is ready to sell the sneak glove on your first meeting. The street in front of the Honeybee Inn has the wrong NPCs (only one Shinra guard at the door), so progress beyond this point becomes impossible. Also, if you leave that map, then come back, then try to leave again, Aerith will separate from Cloud to interact with NPCs who aren't there, getting stuck.

To Reproduce

1. Start a new game a play through to Wall Market. (Alternatively, the bottom save in the attached file is immediately before entering Wall Market for the first time.)
2. Notice progression is broken.

Expected behavior
Progression is not broken.

Workaround
Revert to older FFNx binary.

save03.ff7.zip

[julianxhokaxhiu]

I don't see how a stack bump would break such in-game functionality since we didn't touch anything related. But I'd appreciate if anyone can replicate this bug.

Again, Stable 1.15 vs canary is enough.

Thank you in advance.

[ChthonVII]

I'm not having the same issue with the most recent canary.

This suggests that it's caused by something in my build environment. I've got a theory, but it will take awhile to check out. I'll report back.

[ChthonVII]

I think I've narrowed this down to mostly a vcpkg issue.

When I went to try bisecting to find this and #578, I noticed that my build profile had been unexpectedly changed to "debug" at some point. After I switched that back to "release," I was immediately unable to build. The linker failed with errors indicating that vcpkg had built the "release" versions of some dependencies against the wrong runtimes. (?!)

So I what I theorize happened was this: The binaries that were acting funny were broken debug builds with runtime mismatches that the linker didn't detect. (So far as I know, it doesn't actually care if the runtimes are mismatched so long as the symbols line up.) That could lead to a "same variable name, but on a different heap" bug. If that happened with progress-related variables, it might conceivably look like the wonky behavior I saw. That's my best guess anyway.

I was able to resolve the issue by:

1. Delete vcpkg and then reinstall the latest version.
2. Delete my FFNx directory and start from a fresh copy.
3. Pre-apply the workarounds for vcpkg problems noted here. (At least the grep problem is finally fixed in the latest vcpkg.)
4. Add my new FFNx directory to Visual Studio and let it rebuild dependencies.
5. Switch build profile to "release." (See below.)
6. Build successfully. Resulting binary does not exhibit this bug, nor [ COMMON ] Massive Performance Degradation #578.

I did however encounter the same issue with the build profile being unexpectedly set to "debug" instead of "release." I believe this might be caused by commit 50e182c. Visual Studio is probably taking the top profile as the default. In the old CMakeSettings.json "release" was the top entry, but in the new CMakePresets.json "debug" is the top entry. This should probably be changed, since it's unexpected and inconsistent with what the read-me says the default is.

How the heck did vcpkg manage to build the dependencies against the wrong runtimes? I have no idea.

[julianxhokaxhiu]

This should probably be changed, since it's unexpected and inconsistent with what the read-me says the default is.

Feel free to do some tests and open a PR if it works :) I'll be happy to merge it. RelWithDebInfo should be the default one which provides a good balance between performance and debugging. So if that's not the case with CMakePresets.json let's fix it.

About the rest, it is expected to have a slower runtime if you build with Debug because it's exactly for debug purposes like in every other software you build. If you want better performance you must use other profiles, like Release, MinSizeRel. RelWithDebInfo sits somewhere in the middle and so far worked fine for us.

[julianxhokaxhiu]

First profile to be built by default now is going to be Release like the one used here on the CI. Second to that is RelWithDebInfo. This should provide at least a match in performance of what you expect by default.