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

unpredictable errors with methods returning Ref #1300

Closed
vinni-richburgh opened this issue Nov 5, 2023 · 5 comments
Closed

unpredictable errors with methods returning Ref #1300

vinni-richburgh opened this issue Nov 5, 2023 · 5 comments
Labels
bug This has been identified as a bug

Comments

@vinni-richburgh
Copy link

vinni-richburgh commented Nov 5, 2023

Godot version

4.1.2-stable

godot-cpp version

4.1.2

System information

MacOS 13 & 14, Apple Silicon

Issue description

I'm observing Ref-returning methods throwing errors and behaving unexpectedly.
This happens since the switch to 4.1 if I'm not mistaken.

This is the error:

E 0:00:00:0376   get_instance_binding_callbacks: Cannot find instance binding callbacks for class 'Shader'.
  <C++ Error>    Condition "callbacks_it == instance_binding_callbacks.end()" is true. Returning: nullptr
  <C++ Source>   /Users/user0/Documents/issue-project/gdextension/godot-cpp/src/core/class_db.cpp:321 @ get_instance_binding_callbacks()

In the example I provided the error should happen for the classes Shader, Image and ImageTexture.
The following method calls in node0.cpp are to blame.

godot::ResourceLoader::get_singleton()->load("res://gdextension/node0/shader.gdshader")
noise.get_seamless_image(512, 512)
godot::ImageTexture::create_from_image(noise_image)

Over on my main project I'm also observing this error related to AudioStreamPlayer::get_stream_playback.
Refs instantiated with memnew work like a charm, so issue #1057 might be related to this one?

Steps to reproduce

From the project's root directory run ./setup.sh. (nothing scary there, just setting up git and building w/ cmake)

Open the project in Godot and play the scene "sandbox.tscn".

Three errors (see Issue description) should be printed.

Also try renaming gdextension/lindenmayer_tree to gdextension/node1, adjust CMakeLists.txts and update the lib paths in node1.gdextension, recompile and the errors should be gone, which is very odd!

Minimal reproduction project

minimal-reproduction-proj.zip

@dsnopek
Copy link
Collaborator

dsnopek commented Nov 8, 2023

Thanks!

I just tried the MRP on Linux using Godot 4.1.2, and, unfortunately, I can't reproduce the issue. The scene seems to run fine, and I don't get any errors printed to the console.

However, this error:

get_instance_binding_callbacks: Cannot find instance binding callbacks for class 'Shader'.

... will happen when an object of a particular type (in this case Shader) is passed from Godot to a GDExtension, but the wrapper class hasn't been registered in the GDExtension. In PR #1266, we attempted to make it so that only classes whose header files have been included are registered. (BTW, if a class isn't registered, it'll just wrap the object in the closest parent class, so most things should still work, even if we can't use exactly the correct wrapper.)

So, I could see this error happening when using ResourceLoader::get_singleton()->load() on a resource type whose header was not included. If this prevents code from working as expected, this could certainly be bug!

However, since I can't seem to reproduce the issue with the MRP in order to verify that, it could also be something else entirely.

@dsnopek dsnopek added the bug This has been identified as a bug label Nov 8, 2023
@vinni-richburgh
Copy link
Author

Hey, thanks for looking into this!

Just double checked my mrp and sprinkled in some extra-verbose includes over various code files, but the three errors wouldn't go away.

I don't think this recent pull request caused my issue though. Invalid Refs being returned by the methods mentioned above happen since even before August. But only recently did they bother me enough to search for a solution. Much to my surprise it seems no one is having the same issue.

Might as well be a MacOS-specific thing. But I'm certainly not the only one using GDExtension on Apple-Silicon, right?
This is very weird.

What I will try next is downgrade godot and godot-cpp to a point where the errors go away or at least the methods return valid Refs again. Hopefully this way I can date the breakage back to a certain version bump or maybe even a specific commit.

Best regards
Vinni

@vinni-richburgh
Copy link
Author

So, I dug a little deeper and found a few relevant commits in the godot-cpp log.

I checked out the 4.1 tags in copies of godot and godot-cpp and downgraded godot-cpp until I observed the following breakage:

  • Running sandbox.tscn crashes from and including ac98dd2752a49b8e80629a59d7243e589f44358d all the way back to and including 1c18413de00f1a6265b2b6c30175b2f6a434b574 with godot v4.1-stable.
  • The earliest commit on which I could trigger an instance binding error message was 8052f818b47c163d3e0a19b13282d909ebfa55f2 with godot v4.1-stable. No idea though, if for earlier commits the scene just crashes before the errors would otherwise get triggered. Don't know if the crashes are even related to this issue, they might as well not be.
  • sandbox.tscn works flawlessly up until and including 813827c26ae079a9c29302a0a4d4c53dc4d89c19 with godot v4.0.3-stable. No errors, no crashes, the sprite of node0 even shows a texture. (Needed to tweak register_types.cpp files, since this is 4.0-territory.)

All we need now is someone with the right setup to reproduce the issue. In the meantime I'll poke at my MacOS install a bit and see if maybe I can find something of interest there.

My apologies btw for the nasty project name of my mrp. I created this project after a bunch of quite frustrating attempts at fixing my code (that apparently works on @dsnopek's machine) and forgot to rename it before uploading. Didn't mean to dunk on what you've achieved here! I love Godot. <3

Best regards
Vinni

@dsnopek
Copy link
Collaborator

dsnopek commented Nov 10, 2023

  • Running sandbox.tscn crashes from and including ac98dd2752a49b8e80629a59d7243e589f44358d all the way back to and including 1c18413de00f1a6265b2b6c30175b2f6a434b574 with godot v4.1-stable.

Commit 1c18413de00f1a6265b2b6c30175b2f6a434b574 is the PR where we switched from using the Godot 4.0 GDExtension interface to the new 4.1 interface. So, I'd expect it to crash with any earlier commits as well. :-) But I guess this basically means that you're getting a crash with all 4.1 compatible versions.

  • The earliest commit on which I could trigger an instance binding error message was 8052f818b47c163d3e0a19b13282d909ebfa55f2 with godot v4.1-stable.

Well, this is the commit that added the error message, and which attempted to fix the issue where the incorrect wrapper class could be used. All the way back then, though, godot-cpp should have registered all engine classes (assuming that the code generation was working correctly), so there really shouldn't be any reason to expect that error for a class that has existed for as long as Shader.

Since you're using cmake (rather than scons), I wonder if something is going wrong with the code generation?

In any case, I don't know how much more I can investigate personally, since the MRP doesn't crash or error for me. When I run it, it even shows a texture, although I haven't inspected the code to see if the texture looks as expected.

Can you compile with debug symbols and get a stack trace for the crash?

@vinni-richburgh
Copy link
Author

vinni-richburgh commented Nov 12, 2023

Indeed, your suspicion of CMake and code generation not working properly lead me somewhere.
When I built the mrp with SCons everything worked as expected.
I never used SCons much because I'm including libs in my godot projects that only use CMake and I couldn't get such a setup with SCons at the top to work.

Turns out to get the mrp working you need to add set_target_properties(godot-cpp PROPERTIES CXX_VISIBILITY_PRESET hidden) to gdextension/CMakeLists.txt right after add_subdirectory(godot-cpp).
Looks like until 4.1 I got away with this CMake setup, but not anymore after the switch to 4.1 .
I thought you built the mrp with CMake like I did, so I assumed my setup worked correctly, when it did not.

Problem was sitting before the computer all along.
Thank you for taking the time to help out!
Closing this issue.

Best regards
Vinni

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug
Projects
None yet
Development

No branches or pull requests

2 participants