MSVC: Enable Edit & Continue in debug builds

[obligaron]

PR enables Edit & Continue support for MSVC, see https://learn.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio#edit-and-continue-for-cmake-projects.

This avoids the need to restart a debug session. This is especially useful when debugging multiplayer sessions.

I wasn't sure if we should add a CMake option for this, and if so, whether or not to enable it by default.

[glebm]

I don't think CMAKE_BUILD_TYPE works properly in multi-config IDE such as Visual Studio, so you need to use a $<CONFIG:Debug> generator expression (like in the example in CMake documentation).

[obligaron]

I changed it to

  if(${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.25")
    # This enables Edit & Continue support, see https://learn.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio#edit-and-continue-for-cmake-projects
    set(CMAKE_MSVC_DEBUG_INFORMATION_FORMAT "$<$<CONFIG:Debug>:EditAndContinue>") # Sets /ZI compiler option, see https://cmake.org/cmake/help/latest/variable/CMAKE_MSVC_DEBUG_INFORMATION_FORMAT.html
    add_link_options("$<$<CONFIG:Debug>:/INCREMENTAL>")
  endif()

and removed the CMAKE_BUILD_TYPE check.
Do I need an additional protection for msvc? (as in your example above)

Thanks for the help glebm. ❤️

[glebm]

Do I need an additional protection for msvc? (as in your example above)

Not according to cmake documentation:

The value is ignored on compilers not targeting the MSVC ABI

However, it does require setting CMP0141 to NEW

This variable has effect only when policy CMP0141 is set to NEW

[obligaron]

I added the CMP0141 policy in the beginning of the CMake file, where we also set other policies to NEW.

Another question:
We also use CMAKE_BUILD_TYPE STREQUAL "Debug" in

devilutionX/CMakeLists.txt

Lines 197 to 202 in 5ed7ca9

	if(MSVC AND NOT CMAKE_BUILD_TYPE STREQUAL "Debug" AND NOT DISABLE_LTO)
	# Work around MSVC + CMake bug when LTO is enabled.
	# See https://github.com/diasurgical/devilutionX/issues/3778
	# and https://gitlab.kitware.com/cmake/cmake/-/issues/23035
	set(BUILD_TESTING OFF)
	endif()

should this also be changed (in another PR)?

[glebm]

The add_link_options call is already inside if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") so it should be fine.

[glebm]

We also use CMAKE_BUILD_TYPE STREQUAL "Debug"

I'm not sure how to handle that one or whether it even works as expected.
It's a bit tricky because BUILD_TESTING is used at configure-time to decide whether to define the test targets.

[obligaron]

I tried it, but it didn't work for me. 😢
But that is not directly related to this PR.

I did some reconfiguring and rebuilding with this PR and it looks like it works. Do you think the PR is ok this way? 🙂

[glebm]

I think it's fine but while I know a bit about CMake I don't use Visual Studio at all, so let's get @StephenCWills or someone else who uses VS to also have a look.

[StephenCWills]

Note that developers will get the warning that both /Zi and /ZI are set unless they use Delete Cache and Reconfigure because /Zi has already been added to CMAKE_C(XX)_FLAGS_DEBUG in CMakeCache.txt. There's not really anything we can do about this, but we can explain it for anyone who asks.

I've also read that the enc_temp_folder may actually be somewhat annoying when using Find in Files. I suppose if anyone has any issues with this, we can revisit making this into an option.

I don't think we need to add /INCREMENTAL via add_link_options() because CMake already adds it to Debug and RelWithDebInfo build types. But it doesn't hurt anything, and Microsoft docs suggested it so maybe it's necessary in older CMake versions. 🤷