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

Make ptrcalls opt-in #973

Merged
merged 1 commit into from
Nov 30, 2022
Merged

Conversation

chitoyuu
Copy link
Contributor

@chitoyuu chitoyuu commented Nov 4, 2022

This implements cuddlefishie's suggestion in #814, but with the semantics reversed to better conform with Cargo's additive combination of features.

  • Added the new feature flag ptrcall, which enables performant API calls at the cost of forward binary compatibility with the engine.
  • Added tests with and without the feature to the full CI suite.

I believe the current semantics to be better, since by making the safer option the default, it helps reduce the surprise factor when someone tries to use the crate with a supposedly compatible version of Godot. Different opinions are welcome.

Note that this only addresses binary compatibility -- #814 is also in a large part an API problem.

@Bromeon Bromeon added feature Adds functionality to the library c: bindings Component: GDNative bindings (mod api) labels Nov 17, 2022
@chitoyuu
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Nov 24, 2022
@bors
Copy link
Contributor

bors bot commented Nov 24, 2022

try

Build succeeded:

@chitoyuu
Copy link
Contributor Author

CI was broken: matrix.include didn't work the way I expected, and prevented integration tests against Godot 3.5.1 from running.

bors try

bors bot added a commit that referenced this pull request Nov 26, 2022
@bors
Copy link
Contributor

bors bot commented Nov 26, 2022

try

Build succeeded:

This implements cuddlefishie's suggestion in godot-rust#814, but with the
semantics reversed to better conform with Cargo's additive combination
of features.

- Added the new feature flag `ptrcall`, which enables performant API
  calls at the cost of forward binary compatibility with the engine.
- Added tests with and without the feature to the full CI suite.
@chitoyuu chitoyuu marked this pull request as ready for review November 26, 2022 16:14
@chitoyuu
Copy link
Contributor Author

CI fixed. Marking as ready-for-review.

@chitoyuu chitoyuu changed the title Draft: Make ptrcalls opt-in Make ptrcalls opt-in Nov 26, 2022
@chitoyuu
Copy link
Contributor Author

Merging this for now.

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 30, 2022

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: bindings Component: GDNative bindings (mod api) feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants