migueldeicaza / SwiftGodot

New Godot bindings for Swift
https://migueldeicaza.github.io/SwiftGodotDocs/tutorials/swiftgodot-tutorials/
MIT License
1.07k stars 71 forks source link

Hot Reloading not working? #273

Closed jsbeckr closed 7 months ago

jsbeckr commented 9 months ago

I tried out hot reloading but it seems it's not working, at least for me.

Project Structure:

➜ tree .
.
├── SimpleRunnerDriver
│   ├── Package.resolved
│   ├── Package.swift
│   ├── Sources
│   │   └── SimpleRunnerDriver
│   │       ├── Dummy.swift
│   │       ├── PlayerController.swift
│   │       └── SimpleRunnerDriver.swift
│   └── Tests
│       └── SimpleRunnerDriverTests
│           └── SimpleRunnerDriverTests.swift
├── SimpleRunnerDriver.gdextension
├── assets
│   ├── swift.png
│   ├── swift.png.import
│   ├── tileset.png
│   └── tileset.png.import
├── icon.svg
├── icon.svg.import
├── main.tscn
├── objects
│   └── player.tscn
└── project.godot

SimpleRunnerDriver.gdextension

   1   │ [configuration]
   2   │ entry_symbol = "swift_entry_point"
   3   │ compatibility_minimum = 4.2
   4   │ reloadable = true
   5   │
   6   │ [libraries]
   7   │ macos.debug = "res://SimpleRunnerDriver/.build/arm64-apple-macosx/debug/libSimpleRunnerDriver.dylib"

Godot Editor log:

SWIFT: extension_initialize
Registering PlayerController : CharacterBody2D
Registering Dummy : Node
SWIFT: extension_initialize
2023-12-04 13:28:16.116 Godot[42742:8159462] WARNING: AVCaptureDeviceTypeExternal is deprecated for Continuity Cameras. Please use AVCaptureDeviceTypeContinuityCamera and add NSCameraUseContinuityCameraDeviceType to your Info.plist.
WARNING: Blend file import is enabled in the project settings, but no Blender path is configured in the editor settings. Blend files will not be imported.
     at: _editor_init (modules/gltf/register_types.cpp:63)
SWIFT: Removed object from our live SubType list (type was: <Dummy#1551305338405>
SWIFT: instanceBindingFree token=Optional(0x0000000150031950) instance=Optional(0x000000014f374810) binding=Optional(0x0000600003967860)
SWIFT: Removed object from our live SubType list (type was: <PlayerController#1551338892837>
SWIFT: instanceBindingFree token=Optional(0x0000000150031950) instance=Optional(0x000000014f4d0210) binding=Optional(0x0000600006c54840)
SWIFT: extension_deinitialize
SWIFT: extension_deinitialize
SWIFT: extension_deinitialize
SWIFT: extension_deinitialize
ERROR: Condition "p_level > int32_t(level_initialized)" is true.
   at: deinitialize_library (core/extension/gdextension.cpp:794)
ERROR: Condition "p_level > int32_t(level_initialized)" is true.
   at: deinitialize_library (core/extension/gdextension.cpp:794)
ERROR: Condition "p_level > int32_t(level_initialized)" is true.
   at: deinitialize_library (core/extension/gdextension.cpp:794)
ERROR: Condition "p_level > int32_t(level_initialized)" is true.
   at: deinitialize_library (core/extension/gdextension.cpp:794)

I am not sure if reloadable = true is necessary in the .gdextension file (I think I got that from the Rust bindings). Nevertheless hot reloading is not working for me. Maybe I am doing something wrong?

Thank you for the awesome work and the cool talk at GodotCon ;)

kevinw commented 9 months ago

I tried this too, and hit the same problems. Digging into the rust gdext implementation, in which hot reloading does work for me, I noticed some differences:

They pass GDEXTENSION_INITIALIZATION_SCENE for minimum_initiailzation_level

     initialization.pointee.deinitialize = extension_deinitialize
     initialization.pointee.initialize = extension_initialize
-    initialization.pointee.minimum_initialization_level = GDEXTENSION_INITIALIZATION_CORE
+    initialization.pointee.minimum_initialization_level = GDEXTENSION_INITIALIZATION_SCENE
     extensionInitCallbacks.append(initHook)
     extensionDeInitCallbacks.append (deInitHook)

They also unregister extension classes during deinit, so in struct GodotInterface we need

     let classdb_register_extension_class_property_subgroup:
     GDExtensionInterfaceClassdbRegisterExtensionClassPropertySubgroup

+    let classdb_unregister_extension_class: GDExtensionInterfaceClassdbUnregisterExtensionClass
+    
     let object_set_instance: GDExtensionInterfaceObjectSetInstance
     let object_set_instance_binding: GDExtensionInterfaceObjectSetInstanceBinding

and in initializeSwiftModule we need

         classdb_register_extension_class_property_subgroup: load ("classdb_register_extension_class_property_subgroup"),

+        classdb_unregister_extension_class: load("classdb_unregister_extension_class"),
+        
         object_set_instance: load ("object_set_instance"),
         object_set_instance_binding: load ("object_set_instance_binding"),

I also added an unregister func and called it in my extension's deinit hook:

diff --git a/Sources/SwiftGodot/Core/Wrapped.swift b/Sources/SwiftGodot/Core/Wrapped.swift
index 726f37b..5656f50 100644
--- a/Sources/SwiftGodot/Core/Wrapped.swift
+++ b/Sources/SwiftGodot/Core/Wrapped.swift
@@ -221,6 +221,15 @@ func register<T:Wrapped> (type name: StringName, parent: StringName, type: T.Typ
     }
 }

+public func unregister<T:Wrapped> (type: T.Type) {
+    let typeStr = String(describing: type)
+    let name = StringName(typeStr)
+    print("Unregistering \(typeStr)")
+    withUnsafePointer(to: &name.content) { namePtr in
+        gi.classdb_unregister_extension_class(library, namePtr)
+    }
+}
+

But after these changes, and calling unregister I hit a new snag:

objc[70182]: Class _TtC9SwiftSync12SpinningCube is implemented in both /Users/kev/src/mjgodot/SwiftSync/.build/arm64-apple-macosx/debug/libSwiftSync.dylib (0x1202a4138) and /Users/kev/src/mjgodot/SwiftSync/.build/arm64-apple-macosx/debug/libSwiftSync.dylib (0x12b6b4138). One of the two will be used. Which one is undefined.

This to me is implying that the old Swift class hasn't been unloaded by Godot yet (but it should have already unloaded the .dylib by this point??), but I'm not an expert in this area, so I leave it here for now...

migueldeicaza commented 9 months ago

If you undo the GDEXTENSION_INITIALIZATION_CORE change, is there any effect?

kevinw commented 9 months ago

If you look at Godot's core/extension/gdextension_manager.cpp, you can see that init levels beneath SCENE cause Godot to inform the user they need to restart the editor to see changes:

GDExtensionManager::LoadStatus GDExtensionManager::_load_extension_internal(const Ref<GDExtension> &p_extension) {
    if (level >= 0) { // Already initialized up to some level.
        int32_t minimum_level = p_extension->get_minimum_library_initialization_level();
        if (minimum_level < MIN(level, GDExtension::INITIALIZATION_LEVEL_SCENE)) {
            return LOAD_STATUS_NEEDS_RESTART;
        }
        // Initialize up to current level.
        for (int32_t i = minimum_level; i <= level; i++) {
            p_extension->initialize_library(GDExtension::InitializationLevel(i));
        }
    }

    for (const KeyValue<String, String> &kv : p_extension->class_icon_paths) {
        gdextension_class_icon_paths[kv.key] = kv.value;
    }

    return LOAD_STATUS_OK;
}