migueldeicaza / SwiftGodot

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

@BindNode fails on SteamDeck (Arch Linux) #486

Closed lorenalexm closed 1 month ago

lorenalexm commented 1 month ago

Describe the bug After building and running a SwiftGodot project on the SteamDeck, the application exits with a error of ERROR: Node not found: "<computed 0x00007fc9dc8b23e0 (CPUParticles3D)»" (relative to "/root/Main/Player"). get_node (scene/main/node.cpp:1651). This crash only appears when using @BindNode not when using direct getNode(path:) calls.

After replacing all @BindNode property wrappers with optional properties, calling getNode(path:) within the _ready function will prevent the crash from happening and the build runs as expected.

To Reproduce

Expected behavior The Godot build should run without issue.

Screenshots BindNodeError

Desktop (please complete the following information):

lorenalexm commented 1 month ago

I have done a little more digging on Linux, and it seems that the issue lies with pulling the Node's path from wrappedKeyPath.debugDescription.

On macOS this works as expected and provides results such as \Player.Character which works its way down to just Character by the NodePath(from:) call.

When running on Linux (Steam Deck) the debug description provides vastly different results: \Player.<computed 0x00007f5fb851d0f0 (Node3D)>. Making complete sense why Godot wouldn't be able to find a Node with that name 😅 At this time I have not done any testing on Windows.

I am not sure if there is an easy way to fix this in the property wrapper itself, since different systems are providing vastly different debug descriptions, or if a breaking change is needed and force the use of @BindNode(path:) as an example?

Screenshot 2024-06-02 at 6 26 09 PM
lorenalexm commented 1 month ago

Small continuation: When running under Windows, the same error presents itself (albeit with a different computed identifier).

migueldeicaza commented 1 month ago

Mhm, I think we might need a different approach, Kurt provided an alternative macro that does not rely on these key paths, but the issue it that it comes with some smarts, that I am not thrilled about.

Will th

migueldeicaza commented 1 month ago

For now, could you try using BindNode(withPath:...) to force the path?

Or use @SceneTree which should default to something nicer.

I am afraid by looking at the source code for Swift, that relying on the debugDescription to compute the name of the property is just not going to be resilient.

migueldeicaza commented 1 month ago

I will update the docs to point users to the more reliable SceneTree.

migueldeicaza commented 1 month ago

Updated docs.

lorenalexm commented 1 month ago

Sorry for the delayed reply! @BindNode(withPath:) has worked, and has been primarily what I have used since I submitted the PR for it. In testing on other platforms besides macOS I ran into the original issue when forgetting to update a property from the @BindNode with no parameters.

In addition to the docs being updated to use the @SceneTree, should a #warning be put into @BindNode to further advise of failings on platforms beside macOS and direct to either @SceneTree or @BindNode(withPath:)?

migueldeicaza commented 1 month ago

It looks like #warning would only produce that warning when you build SwiftGodot, but would not show up when the user consumes it, do you know of another way of doing that?

If not, we can fall back to adding a runtime log.

lorenalexm commented 1 month ago

Off the top of my head, I do not have a solution. A runtime message could work, but as the property wrapper doesn't return an optional it could result in a crash prior to it being shown if the node isn't verified prior to use (which may be overlooked given it's not optional).

My thought for the #warning would be directing the use of @SceneTree or @BindNode(withPath:) if compiled on a platform aside from macOS; as technically @BindNode isn't broken per se.

lorenalexm commented 1 month ago

It could also be that @BindNode(withPath:) becomes the required way to use the property wrapper for full platform support. That would be a breaking change, and I am not sure if that is the proper solution.