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

GDNative 1.3 version check breaks compilation for Godot 3.5 RC #904

Closed
kylewlacy opened this issue Jul 11, 2022 · 4 comments · Fixed by #909
Closed

GDNative 1.3 version check breaks compilation for Godot 3.5 RC #904

kylewlacy opened this issue Jul 11, 2022 · 4 comments · Fixed by #909
Labels
Milestone

Comments

@kylewlacy
Copy link

I'm evaluating Godot 3.5 RC 6 for a project. I followed along with the "Hello, World!" tutorial in the book, and everything built fine. I then added a new GDNativeLibrary resource, set up the HelloWorld class as a NativeScript, and attached it to a node. When starting my project, I got the following error:

E 0:00:00.515   gdnative_init: Error loading GDNative file res://MyLibrary.dll: GodotEngine v4.* is not yet supported. See https://github.com/godot-rust/godot-rust/issues/396
  <Source>      MyLibrary.dll:0 @ gdnative_init()

I tracked this error down to this section in the code:

https://github.com/godot-rust/godot-rust/blob/bacc380bfeee5aba44dd1a6966adc39b4912c6e0/gdnative-core/src/private.rs#L55-L61

I did some digging and it looks like this check was added at a time where Godot 4 reported a GDNative API version of 1.3 even though the Godot 4 API was not compatible. My understanding of the situation since then is that Godot 4 removed GDNative in favor of the new "GDExtension" mechanism, and Godot 3.5 bumped its own version of GDNative to 1.3 (which I'm guessing is broadly compatible with API version 1.2?)

As a result, it seems like godot-rust v0.10.0 isn't compatible with Godot 3.5, as it's treating Godot 3.5 the same as Godot 4.


I made a fork of godot-rust with this check disabled (https://github.com/kylewlacy/godot-rust/tree/wip/godot-3.5). Compiling my GDNative extension using my godot-rust fork seems to make the extension load properly now, and "Hello, World!" gets printed to the console when loaded. I haven't tested anything beyond the "Hello World" tutorial at this point, so I can't speak about other pitfalls or breakages. Would Godot 3.5 support be as simple as removing/tweaking this version check, or would it be a more involved change?

@kylewlacy kylewlacy added the feature Adds functionality to the library label Jul 11, 2022
@Bromeon Bromeon added bug and removed feature Adds functionality to the library labels Jul 11, 2022
@Bromeon
Copy link
Member

Bromeon commented Jul 11, 2022

Nice observation, thanks for reporting!

I did some digging and it looks like this check was added at a time where Godot 4 reported a GDNative API version of 1.3 even though the Godot 4 API was not compatible. My understanding of the situation since then is that Godot 4 removed GDNative in favor of the new "GDExtension" mechanism, and Godot 3.5 bumped its own version of GDNative to 1.3

This is a great example why version numbers should not be reused. No matter how circumstances changed, 1.3 had a certain meaning some time ago, but now has a different one. This makes it impossible to account for both scenarios: either 1.3 meaning an early Godot 4 or meaning Godot 3.5. Since it has been 2 years, I guess the reasonable choice would be to use the current meaning and assume that 1.3 really means "Godot 3.5" -- especially because GDNative has been completely reworked into GDExtension. I'm thinking about how to check for incompatibilites and try to follow up with a PR this week.

Would Godot 3.5 support be as simple as removing/tweaking this version check, or would it be a more involved change?

I'm not sure if GDNative 1.2 -> 1.3 entails a breaking change, as I don't trust SemVer to be upheld here. So we'd need to see if there are breaking changes in the FFI interface. Even adding a default parameter is a breaking change from Rust perspective, see #814.

In general, once Godot 3.5 stable is released, godot-rust should follow suit and support it out of the box with its default configuration. There is the feature flag custom-godot to support older versions.

@Bromeon Bromeon added this to the v0.10.1 milestone Jul 11, 2022
@bors bors bot closed this as completed in 626648b Jul 17, 2022
@bors bors bot closed this as completed in #909 Jul 17, 2022
@Bromeon Bromeon changed the title Support Godot 3.5 GDNative 1.3 version check breaks compilation for Godot 3.5 RC Jul 17, 2022
@Bromeon
Copy link
Member

Bromeon commented Jul 17, 2022

Further efforts to support Godot 3.5 are tracked in #910.

@akien-mga
Copy link

I'm not sure if GDNative 1.2 -> 1.3 entails a breaking change, as I don't trust SemVer to be upheld here.

To be clear, this is not the GDNative version. It's a new Core API subset that can be used optionally. You don't have to use it and code using the 1.2 Core API will still work. It's just new core methods that were added, and when we do so we create a new Core API version to hold those extensions, specifically to prevent breakage.

I wasn't aware that GDNative in early 4.0 days also declared Core API 1.3, but this was pre-alpha software and shouldn't be assumed to be kept as is (and it indeed wasn't kept).

@Bromeon
Copy link
Member

Bromeon commented Aug 7, 2022

Oh, thanks for the clarification! That was before my days, and I wasn't aware that this was pre-alpha!

I guess there's no good way to depend on pre-alpha software, but probably it's still better to have some kind of versioning (even if that only expresses "I am pre-alpha"), to differentiate it from future, more stable versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants