Skip to content

shape_collide and body_collide_shape from IPhysicsServer2DExtension gives compilation error #677

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

Closed
Ughuuu opened this issue Apr 26, 2024 · 3 comments
Labels
bug c: register Register classes, functions and other symbols to GDScript

Comments

@Ughuuu
Copy link
Contributor

Ughuuu commented Apr 26, 2024

The following 2 functions:

#[godot_api]
impl IPhysicsServer2DExtension for RapierPhysicsServer2D {
...
    unsafe fn shape_collide(&mut self, shape_A: Rid, xform_A: Transform2D, motion_A: Vector2, shape_B: Rid, xform_B: Transform2D, motion_B: Vector2, results: * mut c_void, result_max: i32, result_count: * mut i32,) -> bool {
        false
    }

    unsafe fn body_collide_shape(&mut self, body: Rid, body_shape: i32, shape: Rid, shape_xform: Transform2D, motion: Vector2, results: * mut c_void, result_max: i32, result_count: * mut i32,) -> bool {
    }
...
}

When implemented to a IPhysicsServer2DExtension gives the following error:

17 | #[godot_api]
   | ^^^^^^^^^^^^ the trait `ToGodot` is not implemented for `*mut i32`, which is required by `(bool, godot::prelude::Rid, godot::prelude::Transform2D, godot::prelude::Vector2, godot::prelude::Rid, godot::prelude::Transform2D, godot::prelude::Vector2, *mut c_void, i32, *mut i32): PtrcallSignatureTuple`
   |
   = help: the trait `ToGodot` is implemented for `i32`
   = note: required for `(bool, Rid, Transform2D, Vector2, Rid, Transform2D, Vector2, *mut c_void, i32, *mut i32)` to implement `PtrcallSignatureTuple`
@lilizoey lilizoey added bug c: register Register classes, functions and other symbols to GDScript labels Apr 26, 2024
@lilizoey
Copy link
Member

This happens because we only implement ToGodot/FromGodot for native class pointers, but not primitive pointers even if they show up in parameters of functions. as a simple fix we could just manually implement this, but a more proper long term fix would be to have the codegen find all used pointer types and automatically implement the traits for them.

@lilizoey
Copy link
Member

from what i can tell, these are the pointers used which i think dont have the trait impls:

*mut *const u8
*mut i32
*mut f64
*mut u8

@Bromeon
Copy link
Member

Bromeon commented Apr 21, 2025

This happens because we only implement ToGodot/FromGodot for native class pointers, but not primitive pointers even if they show up in parameters of functions. as a simple fix we could just manually implement this, but a more proper long term fix would be to have the codegen find all used pointer types and automatically implement the traits for them.

Here's a ripgrep command to find pointers in the API spec, here's for version 4.4:

rg '"type": ".+\*"' extension_api.json -N | sort | uniq

yields:

        "type": "AudioFrame*"
        "type": "CaretInfo*"
"type": "const Glyph*"
        "type": "const uint8_t **"
        "type": "const uint8_t*"
        "type": "const void*"
        "type": "float*"
        "type": "int32_t*"
        "type": "PhysicsServer2DExtensionMotionResult*"
        "type": "PhysicsServer2DExtensionRayResult*"
        "type": "PhysicsServer2DExtensionShapeRestInfo*"
        "type": "PhysicsServer2DExtensionShapeResult*"
        "type": "PhysicsServer3DExtensionMotionResult*"
        "type": "PhysicsServer3DExtensionRayResult*"
        "type": "PhysicsServer3DExtensionShapeRestInfo*"
        "type": "PhysicsServer3DExtensionShapeResult*"
        "type": "ScriptLanguageExtensionProfilingInfo*"
        "type": "uint8_t*"
        "type": "void*"
"type": "void*"

I checked that FromGodot is implemented for the above. It's also implemented for PhysicsServer3DExtensionMotionCollision, even though that is never used as a pointer parameter, but rather a field of another native struct.


It seems like the manual approach has worked well enough for a year, and this changes rarely enough that it's probably not worth automating. Especially since there's quite a bit of overlap between native structs and other pointer parameters, which may complicate an implementation slightly.

I'd suggest:

  • we keep adding these impls manually.
  • when new APIs pop up, it's a good reminder to check if we can create safe versions of them, see #1130.
  • closing this issue, as the original bug reported has been solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: register Register classes, functions and other symbols to GDScript
Projects
None yet
Development

No branches or pull requests

3 participants