jrouwe / JoltPhysics.js

Port of JoltPhysics to JavaScript using emscripten
MIT License
246 stars 19 forks source link

Types: WrapPointer arguement is not always a number. #126

Closed DennisSmolek closed 6 months ago

DennisSmolek commented 6 months ago

When setting up listeners I need to wrap pointers in the callbacks. For example from the contactListener example:

body1 = Jolt.wrapPointer(body1, Jolt.Body);
body2 = Jolt.wrapPointer(body2, Jolt.Body);

In this example body1 and body2 are Jolt.BodyID's but the wrapPointer is set as a number : Types.d.ts Line 7

    function wrapPointer<C extends new (...args: any) => any>(ptr: number, Class: C): InstanceType<C>;

does it need to be an unknown like castObject?

    function castObject<C extends new (...args: any) => any>(object: unknown, Class: C): InstanceType<C>;

Typescript is complaining:

Argument of type 'BodyID' is not assignable to parameter of type 'number'.ts(2345)
jrouwe commented 6 months ago

@isaac-mason is this something you can fix in webidl-dts-gen?

isaac-mason commented 6 months ago

Seems the root issue here is JSImplementation method arguments don't have correct types.

As an example, currently the ContactListenerJS type is:

    class ContactListenerJS {
        constructor();
        OnContactValidate(inBody1: Body, inBody2: Body, inBaseOffset: RVec3, inCollisionResult: CollideShapeResult): number;
        OnContactAdded(inBody1: Body, inBody2: Body, inManifold: ContactManifold, ioSettings: ContactSettings): void;
        OnContactPersisted(inBody1: Body, inBody2: Body, inManifold: ContactManifold, ioSettings: ContactSettings): void;
        OnContactRemoved(inSubShapePair: SubShapeIDPair): void;
    }

But the webidl binder calls the methods with pointers, so this needs to change to:

    class ContactListenerJS {
        constructor();
        OnContactValidate(inBody1: number, inBody2: number, inBaseOffset: number, inCollisionResult: number): number;
        OnContactAdded(inBody1: number, inBody2: number, inManifold: number, ioSettings: number): void;
        OnContactPersisted(inBody1: number, inBody2: number, inManifold: number, ioSettings: number): void;
        OnContactRemoved(inSubShapePair: number): void;
    }

I'll cut a fix in webidl-dts-gen shortly.

isaac-mason commented 6 months ago

I've released a fix for webidl-dts-gen under 1.9.0: https://github.com/pmndrs/webidl-dts-gen/releases/tag/webidl-dts-gen%401.9.0

And raised a PR to bump it's version here: https://github.com/jrouwe/JoltPhysics.js/pull/130

jrouwe commented 6 months ago

Thanks for fixing this! I've released a new package 0.20.0 with this fix.

isaac-mason commented 6 months ago

No worries!