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

Unbind GDExtension methods that can't reasonably be used #88418

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Feb 16, 2024

At the GDExtension meeting on January 26th, 2024 (see notes) we agreed to unbind a number of methods on the GDExtension class that can't reasonably be used by developers.

These methods include:

  • GDExtension::open_library()
  • GDExtension::initialize_library()
  • GDExtension::close_library()

A developer could maybe have partially correctly loaded and initialized a GDExtension using open_library() and then calling initialize_library() with each initialization level. However, a number of other important things are managed by using GDExtensionManager::load_extension(), which this would bypass. Strangely, we never bound deinitialize_library() (the opposite to initialize_library()), so a developer could not have closed a GDExtension properly using these APIs.

Anyway, developers should be using GDExtensionManager::load_extension() and GDExtensionManager::unload_extension() to correctly load and unload a GDExtension.

Since it's basically impossible to use the methods being unbound by this PR in any useful way, we agreed at the meeting that we didn't need to provide compatibility methods for them.

This discussion started from PR #86968 which was attempting to document the methods in question

Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the rationale and the team consensus, I approve despite the compat breakage.

@dsnopek dsnopek requested a review from a team as a code owner February 16, 2024 22:31
@akien-mga akien-mga merged commit 8ff8216 into godotengine:master Feb 16, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@JekSun97
Copy link

Rewrite everyone's extensions again...

@dsnopek
Copy link
Contributor Author

dsnopek commented Feb 16, 2024

Rewrite everyone's extensions again...

Unless an extension called one of the 3 specific method unbound here (which I doubt any extension did because they can't really be used in any reasonable way), then no changes will be needed.

@Bromeon
Copy link
Contributor

Bromeon commented Feb 19, 2024

Unless an extension called one of the 3 specific methods...

or uses load-on-init pattern 😁 but it was quick to adjust.

akien-mga added a commit that referenced this pull request May 30, 2024
Bind compatibility GDExtension methods removed in #88418
@dsnopek dsnopek deleted the gdextension-unbind-methods branch July 22, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants