EmbarkStudios / physx-rs

🎳 Rust binding for NVIDIA PhysX 🦀
http://embark.rs
Apache License 2.0
656 stars 42 forks source link

physx-sys: Emit default values in FFI bindings for literal values #221

Open slvmnd opened 1 year ago

slvmnd commented 1 year ago

Hi, this is a simple fix for https://github.com/EmbarkStudios/physx-rs/issues/188

It generates comments for literal default values (ints, floats, ..) like this:

/// inflation = 1.01
pub fn PxActor_getWorldBounds(self_: *const PxActor, inflation: f32) -> PxBounds3;

/// inflation = 0
/// doubleSided = false
pub fn PxMeshQuery_sweep(unitDir: *const PxVec3, distance: f32, geom: *const PxGeometry, pose: *const PxTransform, triangleCount: u32, triangles: *const PxTriangle, sweepHit: *mut PxGeomSweepHit, hitFlags: PxHitFlags, cachedIndex: *const u32, inflation: f32, doubleSided: bool, queryFlags: PxGeometryQueryFlags) -> bool;

/// scaleMassProps = true
pub fn phys_PxScaleRigidActor(actor: *mut PxRigidActor, scale: f32, scaleMassProps: bool);

A totally different way of solving this would be to not care about the kind of AST node and use the range begin/end offset to read the source expression from the source code instead. But if almost all default values are simple values then this approach might be good enough.

Not really expecting you to merge this as is without changes, but at least it can serve as a starting point.

I haven't really written much rust before so let me know if there is something un-idiomatic.

slvmnd commented 1 year ago

One thing that needs to be considered is what to do about default values which this pull request does not currently handle, so for example:

const PxReal inflation = 0.0f,
bool doubleSided = false,
PxGeometryQueryFlags queryFlags = PxGeometryQueryFlag::eDEFAULT

Here this code will pick up the default values for inflation and doubleSided, but not for queryFlags.

Generating "PxGeometryQueryFlag::eDEFAULT" or something similar from the AST seems fairly complicated and fragile, so the best alternative for complex values is probably to read the value from the C++ source code, but that would be a fairly large change to how the generator is currently architected.