Bind compatibility GDExtension methods removed in #88418

[lyuma]

• Reproducible in 4.3dev6
• System Information: Godot v4.3.dev (6118592) - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3090 (NVIDIA; 31.0.15.3713) - Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz (16 Threads)

These functions were likely not used, but we must ensure they are still bound to ensure API stability.

Without this PR, rust addons give this error:

ERROR: Panic msg:
  Failed to load class method GDExtension::open_library (hash 852856452).
  Make sure gdext and Godot are compatible: https://godot-rust.github.io/book/gdext/advanced/compatibility.html

GDExtension::open_library() with hash 852856452 needs to be re-added as a bind_compatibility_method.

[dsnopek]

We removed these methods without providing compatibility in PR #88418, because there is no way to use them correctly.

If we do need to add them back for compatibility, then we should make them do nothing (and in the case of open_library() return an error). These methods never should have been bound, and using them will lead to problems.

[Bromeon]

In the godot-rust bindings, I explicitly removed these 3 functions. That way, even when using it with older API levels (4.1 or 4.2), it's not possible to accidentally use them.

So it's likely that the initial problem is solved by updating to the latest version.

[lyuma]

Right, but I have gdextension dlls that are probably not using the latest version of the bindings and work fine on Godot 4.2. Even if we remove them from the latest rust bindings, this is still technically a compat break since dlls have to be rebuilt.

[Bromeon]

Right, but I have gdextension dlls that are probably not using the latest version of the bindings and work fine on Godot 4.2. Even if we remove them from the latest rust bindings, this is still technically a compat break since dlls have to be rebuilt.

As I wrote on RocketChat, I think there are some misunderstandings.

First, what we try to achieve with compatibility on godot-rust side is that you can develop against API level 4.1 and still use the extension in Godot 4.2 and 4.3. This is the case on latest master.

If we encounter bugs, it can unfortunately happen that binary compatibility is affected. Not just the example here with 3 GDExtension methods, but in some cases we fixed soundness issues regarding how data was passed over FFI. So there are benefits to remaining up-to-date either way. Especially since we spend a lot of effort in maintaining compatibility with older Godot versions -- you can even still use 4.0 if you like -- so you're not missing out on compatibility when updating.

The second and much more important point: godot-rust has supported lazy loading of function pointers for more than half a year. So it explicitly offers different trade-offs depending on the extension's needs. This is documented on the main API docs page.

TLDR: like all software, godot-rust is improved over time and it is advised to remain reasonably up-to-date. We can't be responsible for bugs fixed in the past because recompiling DLL is a problem. Furthermore, you can enable lazy loading if that fits your workflow better.

[Bromeon]

Note that my above comment merely clarifies the situation around godot-rust.

The core discussion here -- breaking API changes in the presence of severe bugs -- can be quite nuanced.
Some thoughts:

• A "no breaking changes ever" policy would need alternative ways from discouraging the usage of certain functions -- documentation, error messages, etc.
• Such a policy can actively hide incorrect usage of the GDExtension API. If the usage no longer compiles, it forces the user to correct things, instead of possibly worse alternatives (logic errors or even UB).
• Obviously, removal should never be done lightly. Certainly not in "we now have a better API for this" cases. Imo only in "there's no way to use the previous correctly, at all".
• We didn't talk about backporting Unbind GDExtension methods that can't reasonably be used #88418 to previous Godot versions. Arguably it might create even greater confusion, on the other hand all latest patch versions would be on the same page and there's no breakage from 4.2.x -> 4.3 anymore.
• A "no breaking changes ever" policy would ideally come with a way to advertise "discouraged" (one level beyond "deprecated") in extension_api.json. In that case, it's up to the binding to retain or remove the function. But it's certainly better than the current hardcoded list 🙂

[dsnopek]

Discussed at the GDExtension meeting, and even though we decided not add compatibility methods for these, this PR is good and there would be no harm in merging this. However, this isn't a policy change in favor of never breaking compatibility ever.

[akien-mga]

Thanks!