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

Add hot reload support when building with GCC and CMake #1548

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

ytnuf
Copy link
Contributor

@ytnuf ytnuf commented Aug 13, 2024

This is a small continuation to PR #1330 which adds hot reloading options for CMake

However when compiling with GCC, the option -fno-gnu-unique must also be enabled to allow hot reloading.
This is done in the SCons script:

env.Append(CXXFLAGS=["-fno-gnu-unique"])

@ytnuf ytnuf requested a review from a team as a code owner August 13, 2024 18:49
@Calinou Calinou added the enhancement This is an enhancement on the current functionality label Aug 13, 2024
@jpxaraujo
Copy link

Even with this, it still does not work for me. Maybe i'm missing something?
image

Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks!

Insofar as I understand cmake, this seems good. I only have the one note

CMakeLists.txt Outdated
@@ -126,6 +126,9 @@ endif()

if (GODOT_ENABLE_HOT_RELOAD)
set(GODOT_COMPILE_FLAGS "${GODOT_COMPILE_FLAGS} -D HOT_RELOAD_ENABLED")
if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have the compiler_is_gnu variable defined above. Could we use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! But since it's a generator expression, it requires a slightly different approach (using the target_compile_options function instead).

@dsnopek
Copy link
Collaborator

dsnopek commented Aug 22, 2024

@jpxaraujo:

Even with this, it still does not work for me. Maybe i'm missing something?

That error is usually caused by not having HOT_RELOAD_ENABLED defined, which I think is set via calling cmake with -DGODOT_ENABLE_HOT_RELOAD.

@jpxaraujo
Copy link

jpxaraujo commented Aug 23, 2024

@jpxaraujo:

Even with this, it still does not work for me. Maybe i'm missing something?

That error is usually caused by not having HOT_RELOAD_ENABLED defined, which I think is set via calling cmake with -DGODOT_ENABLE_HOT_RELOAD.

@dsnopek Thanks for the reply!

So, I added the changes in this commit, added "reloadable = true" to .gdextension and added the HOT_RELOAD_ENABLED

image

But still have the same problem ...

@Drahiri
Copy link

Drahiri commented Aug 25, 2024

Finally made it work.
@jpxaraujo for hot reloading to work you also need to include HOT_RELOAD_ENABLED and -fno-gnu-unique flag when building your shared library.
Adding this worked

set_property(TARGET ${LIBRARY_NAME} APPEND_STRING PROPERTY COMPILE_FLAGS "-D HOT_RELOAD_ENABLED -fno-gnu-unique")

Where ${LIBRARY_NAME} is name of target from add_library().

@jpxaraujo
Copy link

jpxaraujo commented Aug 25, 2024

@Drahiri well, I am bit confused I think. It was already working when running a scene. But I thought it would also update the scene in the editor.
It would be a game changer if we could get an automatic update for scenes in the editor.

@Drahiri
Copy link

Drahiri commented Aug 25, 2024

It's doing exactly that. After I updated CMake to match this pull request and updated mine CMake with that line. After rebuilding shared library, editor automatically updated without that message asking if hot reloading is enabled.
No need to reload whole project to see changes.

@jpxaraujo
Copy link

jpxaraujo commented Aug 25, 2024

@Drahiri I followed every step, It does not work for me, don't know why. I tried in two different systems, one with a 12800h and a 3050ti with fedora and another with 5800X3D and a 6950xt with gentoo ...
It only updates if I run the scene, the editor does not change for me ...

@Drahiri
Copy link

Drahiri commented Aug 26, 2024

@jpxaraujo I've added repo with working example. Also sorry to all staff if it's turning into spam (I feel like a little).

@jpxaraujo
Copy link

jpxaraujo commented Aug 26, 2024

@jpxaraujo I've added repo with working example. Also sorry to all staff if it's turning into spam (I feel like a little).

Thank you for taking your time to create an example! Hopefully it can help other people. Will test it today or tomorrow.
Regarding the spam, my bad, I'm sorry for that.

@ytnuf
Copy link
Contributor Author

ytnuf commented Aug 26, 2024

Now using the compiler_is_gnu generator expression as suggested.

Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks!

@dsnopek dsnopek merged commit f4d3817 into godotengine:master Sep 10, 2024
12 checks passed
@dsnopek dsnopek added cmake topic:buildsystem Related to the buildsystem or CI setup labels Sep 18, 2024
@ytnuf ytnuf deleted the hot_reload branch October 12, 2024 15:03
@dsnopek
Copy link
Collaborator

dsnopek commented Oct 28, 2024

Cherry-picked for 4.2 in PR #1631

@dsnopek
Copy link
Collaborator

dsnopek commented Oct 28, 2024

Cherry-picked for 4.3 in PR #1632

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake enhancement This is an enhancement on the current functionality topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants