godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.75k stars 580 forks source link

Hot reloading does not reload nodes in editor viewport #1502

Open Zacrain opened 5 months ago

Zacrain commented 5 months ago

Godot version

v4.2.2.stable.official [15073afe3]

godot-cpp version

4.2 [9da6ecd14]

System information

Windows 10, Intel Core i5-9300H

Issue description

Testing out the new hot-reloading feature, I have observed that changes do not appear in the editor view after recompiling a GDExtension. Or let's say, they only appear partly.

Two examples where I encountered this:

In order to visualize what I mean (I'll also provide a minimal reproducible example project), I'll detail those two examples now. Let's say we start with this in a button class:

void StartStopButton::_ready() {
    this->set_text("Start Moving");
}

void StartStopButton::_pressed() {
    if (!flipMode)
        this->set_text("Stop Moving");
    else
        this->set_text("Start Moving");

    flipMode = !flipMode;   
}

This is reflected in the editor view: image

However, now change the text in the code, compile and see that the change is not reflected within the editor view. But, if the scene / game is run, the changes are correctly shown. image

Now an example for renamed method bindings. Let's say we have a method like:

void GDExample::_on_startstop_button_pressed() {
    move = !move;
}

in the Sprite2D child. (And correctly bound in the _bind_methods() method.) This method is connected to the pressed() signal of the button. Rename it to something else, for example on_start_button_pressed, recompile and then we have this:

The old name of the connected method is still visible: image

If I now double click on the signal in order to pick another method, and I'm looking for the renamed one, it is correctly listed: image

Yielding two methods, if the connection to the old one is not removed: image

Running the scene like that is totally fine. Which is good but also problematic as I would expect Godot to throw an error or display an error message if the outdated signal handler can not be called. Which it really can't as it does not exist anymore in the code and resulting library.

If the whole project is reloaded after recompiling the code, those issues disappear. But that's not what I would expect from hot reloading.

Steps to reproduce

  1. Create a C++ GDExtension project.
  2. Create some node which should have visible changes in the editor, like a labeled button or a method name for signal handling.
  3. Compile.
  4. Change something about it in the code, e.g., the text label or the method name.
  5. Compile again.
  6. See that the changes are not or only partly reflected in the editor, but running the game or scene is fine.

Minimal reproduction project

The minimal example, which I provided, should be fine to use for that. Note, that you need to change the env path in the SConstruct file as I placed the godot-cpp compiled C++ API in a different directory and left the path in a probably unusable state now. And of course, don't forget to compile otherwise Godot will not find the libraries as I have not bundled them with the zip archive.

GDExtension CPP Example V2.zip

wakeofluna commented 3 months ago

I had a similar, although not sure if it's the same, problem.

I have a C++ gdextension where I create a few Resource classes, and then a scene where I have several of those classes active. I observed the following issues after making changes followed by a hotreload:

So I was doing some good ol' printf-debugging and found out the following with regard to the hot reloading sequence:

So far so good. Especially that last step shows that the hot reload has succesfully tracked the new objects being created. Also when I compare values such as get_instance_id() it shows that my instances are correctly tied to the already existing Godot Object _owner. But whenever I create a Callable(this, ...) on my new instances then these fail complaining about objects being null pointers.

Long story, it turns out that godot-cpp calls into Godot with a call gdextension_interface_object_get_instance_binding() to lookup my instance pointer that is tied to the _owner, but this binding has been cleared by the unloading of library.

Possible solution

There is a small kludge in the Wrapped constructor that upon hot reload restores the link to the original _owner. I think at this time, we also need to restore the instance_binding to the new this-pointer. This is also required to call the correct destructor (free_callback) for the new instances.

I made a small change to wrapped.cpp to accomplish this:

diff --git a/src/classes/wrapped.cpp b/src/classes/wrapped.cpp
index ffca4f9..3728d25 100644
--- a/src/classes/wrapped.cpp
+++ b/src/classes/wrapped.cpp
@@ -61,6 +61,10 @@ Wrapped::Wrapped(const StringName p_godot_class) {
                while (recreate_data) {
                        if (recreate_data->wrapper == this) {
                                _owner = recreate_data->owner;
+                               if (likely(_constructing_class_binding_callbacks)) {
+                                       godot::internal::gdextension_interface_object_set_instance_binding(_owner, godot::internal::token, this, _constructing_class_binding_callbacks);
+                                       _constructing_class_binding_callbacks = nullptr;
+                               }
                                if (previous) {
                                        previous->next = recreate_data->next;
                                } else {

I don't know for sure if this fix is complete, but it at least fixes my problem.

EDIT: my version information:

wakeofluna commented 2 months ago

Also see #1589

dsnopek commented 2 months ago

I just posted PR https://github.com/godotengine/godot-cpp/pull/1590 which should fix this! If anyone has time to test it with their project, I'd appreciate it :-)

Zacrain commented 2 months ago

I just posted PR #1590 which should fix this! If anyone has time to test it with their project, I'd appreciate it :-)

Nope, sadly did not resolve the issue. :( Tested with a minimal example similar to the one provided in the issue here

dsnopek commented 2 months ago

Ok, thanks for testing!

zero334 commented 2 weeks ago

I'm not entirely sure if this issue is related or a separate bug. If it turns out to be something different, I'll open a separate issue for it. While following the GDExtension C++ example on an M1 Mac running macOS 13.6.7, I noticed that properties set in C++ are lost when the Godot window loses and then regains focus. The only way to recover these properties is by restarting the editor or reloading the current project. I'm using Godot 4.3 stable.

https://github.com/user-attachments/assets/4dd5560b-7bb2-4dbe-95f2-336173e5c751

dsnopek commented 2 weeks ago

@zero334 Could this be related to the issue on MacOS where reload doesn't work if using relative (rather than absolute) paths in the .gdextension file? See https://github.com/godotengine/godot/issues/90108#issuecomment-2400353562

zero334 commented 1 week ago

@dsnopek I'm using absolute paths, exactly as described in the documentation like:

macos.debug = "res://bin/libgdexample.macos.template_debug.framework"
macos.release = "res://bin/libgdexample.macos.template_release.framework"