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

Implement NativeExtension pointer arguments #52036

Merged
merged 1 commit into from
Aug 23, 2021

Conversation

reduz
Copy link
Member

@reduz reduz commented Aug 23, 2021

  • Allows calling into native extensions directly with a pointer
  • Makes it easier to implement some APIs more efficiently
  • Appears with a "*" in the documentation for the argument.
  • Implementing the pointer handling is entirely up to the implementation, although the extension API provides some hint.
  • AudioStream has been implemented as an example, allowing to create NativeExtension based AudioStreams.

Example of how it looks:

GDVIRTUAL3(_mix, GDNativePtr<AudioFrame>, float, int)

@reduz reduz requested review from a team as code owners August 23, 2021 18:12
@reduz reduz force-pushed the native-extension-argument-pointers branch 3 times, most recently from 35897ff to e66d25d Compare August 23, 2021 18:30
@Calinou Calinou added this to the 4.0 milestone Aug 23, 2021
@CedNaru
Copy link
Contributor

CedNaru commented Aug 23, 2021

So if I understand we can now directly send pointers to Godot by casting them to regular int64 by using this new hint.
Will that solve this proposal: godotengine/godot-proposals#3037 ?

When it comes to usability, not sure I really get the AudioStream example.
So in 3.x, this class could only be used/implemented in a module because it was making use of a pointer to an internal structure AudioFrame. But in 4.0, AudioStream is exposed in the API because of the new virtual bind and the _mix method "sees" an int in place of the AudioFrame pointer?

@reduz
Copy link
Member Author

reduz commented Aug 23, 2021

@CedNaru This is not a way to send generic pointers to Godot, it's a way for Godot to send generic pointer to native extensions. It's only meant for implementations using compiled languages to receive low level data required in some situations (like mixing audio, writing network data buffers, etc).

@reduz
Copy link
Member Author

reduz commented Aug 23, 2021

I also have no idea if it solves the proposal, the author did not make clear what the intention was with it.

@CedNaru
Copy link
Contributor

CedNaru commented Aug 23, 2021

Hum ok, so this PR won't solve it.
The intent was to send third-party library structs/classes pointers to Godot and store them in a variant for future use.
In other words, it's a way to replicate RID but for structs external to Godot.
But this PR is kinda the reverse of that.

@reduz
Copy link
Member Author

reduz commented Aug 23, 2021

@CedNaru as written in that proposal, you can cast to integer and back, its not very difficult and makes little difference.

@reduz reduz force-pushed the native-extension-argument-pointers branch 3 times, most recently from 1d17c0a to a1c1f60 Compare August 23, 2021 22:33
* Allows calling into native extensions directly with a pointer
* Makes it easier to implement some APIs more efficiently
* Appears with a "*" in the documentation for the argument.
* Implementing the pointer handling is entirely up to the implementation, although the extension API provides some hint.
* AudioStream has been implemented as an example, allowing to create NativeExtension based AudioStreams.
@reduz reduz force-pushed the native-extension-argument-pointers branch from a1c1f60 to 44d62a9 Compare August 23, 2021 22:58
@reduz reduz merged commit aa3c3a9 into godotengine:master Aug 23, 2021
@neikeq
Copy link
Contributor

neikeq commented Aug 29, 2021

Are the AudioFrame members meant to be accessed by native extensions? If so how can they do this if they don't have the struct declaration?

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.

4 participants