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

*Extension classes missing virtual methods that accept or return pointers #191

Closed
lilizoey opened this issue Mar 20, 2023 · 2 comments · Fixed by #272
Closed

*Extension classes missing virtual methods that accept or return pointers #191

lilizoey opened this issue Mar 20, 2023 · 2 comments · Fixed by #272
Labels
c: engine Godot classes (nodes, resources, ...) feature Adds functionality to the library

Comments

@lilizoey
Copy link
Member

lilizoey commented Mar 20, 2023

*Extension classes often use pointers to interact with the engine. For instance, all of PhysicsDirectSpaceState2D except for the ones inherited from Object use pointers in various places.

We currently do not generate virtual methods that accept or return pointers. This makes many of these classes unusable, as key behavior is not available.

non-exhaustive list:

@Bromeon Bromeon added bug c: engine Godot classes (nodes, resources, ...) labels Mar 20, 2023
@lilizoey
Copy link
Member Author

lilizoey commented Mar 21, 2023

It seems that many of the Extension classes have virtual methods that either accept or return pointers. Which are currently excluded because they are usually supplementary/not callable from gdscript, however in Extension classes they are generally not supplementary. and Extension classes are usually not meant to be used in gdscript anyway.

@Bromeon
Copy link
Member

Bromeon commented Mar 21, 2023

True, this was actually omitted by design for now, and can be added later:

// Currently excluded:
//
// * Private virtual methods are only included in a virtual
// implementation.
//
// * Methods accepting pointers are often supplementary
// E.g.: TextServer::font_set_data_ptr() -- in addition to TextServer::font_set_data().
// These are anyway not accessible in GDScript since that language has no pointers.
// As such support could be added later (if at all), with possibly safe interfaces (e.g. Vec for void*+size pairs)

Reclassifying as feature request.
Maybe you could update the title to clarify which types of methods are missing?

@Bromeon Bromeon added feature Adds functionality to the library and removed bug labels Mar 21, 2023
@lilizoey lilizoey changed the title Missing virtual methods in PhysicsDirectSpaceState2DExtensionVirtual *Extension classes missing virtual methods that accept or return pointers Mar 21, 2023
Mercerenies added a commit to Mercerenies/rust-gdextension that referenced this issue May 11, 2023
Mercerenies added a commit to Mercerenies/rust-gdextension that referenced this issue May 13, 2023
Mercerenies added a commit to Mercerenies/rust-gdextension that referenced this issue May 13, 2023
Mercerenies added a commit to Mercerenies/rust-gdextension that referenced this issue May 13, 2023
Mercerenies added a commit to Mercerenies/rust-gdextension that referenced this issue May 14, 2023
bors bot added a commit that referenced this issue May 24, 2023
272: Implement virtual methods that accept or return pointers (per #191) r=Bromeon a=Mercerenies

This MR adds all missing virtual functions which were previously omitted due to an argument or return value being a pointer. All pointers are translated into Rust-side raw pointers (`*const T` or `*mut T`, as appropriate). All virtual trait functions which either take or return a pointer are marked as `unsafe`.

All `native_structures` structs in the JSON file have been translated into Rust-side structures (with `#[repr(C)]` to ensure binary compatibility) and placed in the `godot::native` directory. These are all pure data structs (all fields are public and they have no methods). Many of the pointer functions take or return pointers to these native structures, so being able to  construct and access them Rust-side is essential for this functionality.

There is one double pointer in the JSON API. `const uint8_t**` appears in several of the networking functions. I believe the correct translation of this type (preserving `const`) is `*mut *const u8`, which is what the code in this MR uses.

Co-authored-by: Silvio Mayolo <mercerenies@comcast.net>
@bors bors bot closed this as completed in #272 May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: engine Godot classes (nodes, resources, ...) feature Adds functionality to the library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants