Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added hot reload support to CMakeLists.txt #1330

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# GODOT_GDEXTENSION_DIR: Path to the directory containing GDExtension interface header and API JSON file
# GODOT_CPP_SYSTEM_HEADERS Mark the header files as SYSTEM. This may be useful to supress warnings in projects including this one.
# GODOT_CPP_WARNING_AS_ERROR Treat any warnings as errors
# GODOT_ENABLE_HOT_RELOAD Build with hot reload support. Defaults to YES for Debug-builds and NO for Release-builds.
# GODOT_CUSTOM_API_FILE: Path to a custom GDExtension API JSON file (takes precedence over `gdextension_dir`)
# FLOAT_PRECISION: Floating-point precision level ("single", "double")
#
Expand Down Expand Up @@ -57,6 +58,13 @@ if("${CMAKE_BUILD_TYPE}" STREQUAL "")
set(CMAKE_BUILD_TYPE Debug)
endif()

# Hot reload is enabled by default in Debug-builds
if("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine, given that it looks like "Debug" and "Release" are the only options here. With scons, there's also the possibility of making editor builds, which is why the logic there is "not release" as opposed to checking directly for debug. I guess if we ever have editor builds via cmake, though, we'll need to change this.

Copy link

Choose a reason for hiding this comment

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

Please consider also Multiconfig generators like "Visual Studio ..." or NinjaMultiConfig where there is no CMAKE_BUILD_TYPE. Then you can use generator expression $

option(GODOT_ENABLE_HOT_RELOAD "Build with hot reload support" ON)
else()
option(GODOT_ENABLE_HOT_RELOAD "Build with hot reload support" OFF)
endif()

if(NOT DEFINED BITS)
set(BITS 32)
if(CMAKE_SIZEOF_VOID_P EQUAL 8)
Expand Down Expand Up @@ -116,6 +124,10 @@ else()
endif()
endif()

if (GODOT_ENABLE_HOT_RELOAD)
set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} -D HOT_RELOAD_ENABLED")
Copy link
Contributor

Choose a reason for hiding this comment

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

Other defines are added to the command-line without the space after -D, it'd be nice for that to be consistent:

Suggested change
set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} -D HOT_RELOAD_ENABLED")
set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} -DHOT_RELOAD_ENABLED")

endif()

# Generate source from the bindings file
find_package(Python3 3.4 REQUIRED) # pathlib should be present
if(GENERATE_TEMPLATE_GET_NODE)
Expand Down