search

fabmax / physx-js-webidl

Javascript WASM bindings for Nvidia PhysX
MIT License
119 stars 28 forks source link

Typescript Support #1

Closed lucas-jones closed 2 years ago

lucas-jones commented 3 years ago

Hey, awesome work in converting the PhysX to use WebIDL, it looks great!

Would be ace to get some Typescript support with a .d.ts somehow auto generated

Could use something like https://github.com/giniedp/webidl2ts

What are your thoughts, would be best approach for this?

fabmax commented 3 years ago

I never used Typescript so I'm probably not able to integrate that into the build, however PR's are welcome so you are welcome to do that 😄

lucas-jones commented 3 years ago

Alrighty. I'll give it a go!

fabmax commented 3 years ago

Awesome! Merged...

edit: I also incremented the version to 0.4.7 and published the npm package

fabmax commented 3 years ago

Just on a site note: When testing the docker build I noticed a weird behavior. With recent versions of emscripten box-box collisions sometimes result in a very unexpected explosion-like behavior. I added a bit more description to the README and a hint to use emscripten 2.0.7 (which does not show that behavior). I also changed the docker build to use that particular version.

lucas-jones commented 3 years ago

Could we get an updated build?

fabmax commented 3 years ago

The old build was not affected by this bug since I built it with my local emscirpten version which was pre 2.0.7. However I did a full rebuild now with the docker version and also updated the npm package to 0.4.8 (which now also contains your updated typescript bindings)

lucas-jones commented 3 years ago

Thank you very much :)

fabmax commented 3 years ago

I just tried to update the typescript bindings by running 

npx milkshake-inc/webidl2ts -e -d -n PhysX -i PhysX/physx/source/physxwebbindings/src/PhysXJs.idl -o dist/physx-js-webidl.wasm.d.ts

But it fails with the following error:

npx: installed 80 in 5.47s
(node:28583) UnhandledPromiseRejectionWarning: WebIDLParseError: Syntax error at line 1208, since `interface PxSimulationStatistics`:
    attribute unsigned FrozenArray<long> nbShapes
                       ^ Failed to parse integer type
    at Tokeniser.error (/home/max/.npm/_npx/28583/lib/node_modules/webidl2ts/node_modules/webidl2/dist/webidl2.js:1:22658)
    at /home/max/.npm/_npx/28583/lib/node_modules/webidl2ts/node_modules/webidl2/dist/webidl2.js:1:18484
    at b (/home/max/.npm/_npx/28583/lib/node_modules/webidl2ts/node_modules/webidl2/dist/webidl2.js:1:18522)
    at d (/home/max/.npm/_npx/28583/lib/node_modules/webidl2ts/node_modules/webidl2/dist/webidl2.js:1:8616)
    at Function.parse (/home/max/.npm/_npx/28583/lib/node_modules/webidl2ts/node_modules/webidl2/dist/webidl2.js:1:9042)
    at v (/home/max/.npm/_npx/28583/lib/node_modules/webidl2ts/node_modules/webidl2/dist/webidl2.js:1:18970)
    at parse (/home/max/.npm/_npx/28583/lib/node_modules/webidl2ts/node_modules/webidl2/dist/webidl2.js:1:16189)
    at Function.parse (/home/max/.npm/_npx/28583/lib/node_modules/webidl2ts/node_modules/webidl2/dist/webidl2.js:1:27951)
    at Function.parse (/home/max/.npm/_npx/28583/lib/node_modules/webidl2ts/node_modules/webidl2/dist/webidl2.js:1:32844)
    at i (/home/max/.npm/_npx/28583/lib/node_modules/webidl2ts/node_modules/webidl2/dist/webidl2.js:1:38785)
(node:28583) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
(node:28583) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@lucas-jones Apparently the WebIDL parser doesn't like array members. Am I doing something wrong?

lucas-jones commented 3 years ago

My mistake, I've actually commented out a few long[] in the WebIDL and it's just been in my local changes since. I'll look into adding support for them

fabmax commented 3 years ago

Ah ok. The easiest thing would probably be to change the parser to ignore these lines, since arrays don't work well in emscripten anyway.

Arrays work a bit better in the Java/JNI code I generate from the WebIDL file so I would prefer the arrays to stay in the WebIDL file, otherwise the easiest option would be to remove them completely.

lucas-jones commented 3 years ago

Just updated the webidl2ts. Should be able to handle them arrays now

https://github.com/Milkshake-Inc/webidl2ts/commit/c77519550942572547b14b6be4541dbf3fce601e

fabmax commented 3 years ago

I thought it would work now but apparently it does not. I think it's caused by my npm version or something like that. It installs a few packages and then simply quits without any further message, so no idea what's actually the problem (My npm version is 7.19.0).

andrewmunro commented 2 years ago

@fabmax Fixed this, there was a missing dependency in the npm package.json. I've had to push it to a new space on npm, as I didn't have permissions for the old one. New command is:

npx @milkshakeio/webidl2ts
fabmax commented 2 years ago

Awesome! Works for me now