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

Add node native bindings and prebuild #83

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

itsame-luigi
Copy link
Contributor

@itsame-luigi itsame-luigi commented Jul 2, 2021

This adds native bindings for NodeJS, instead of depending on a WASM build. The WASM build is still used for the browser. I also added a script to run on npm install that will fetch a specific release build (configured in package.json as "ffi-version").

Also, it is no longer necessary to invoke buttplugInit() in NodeJS (though it is still necessary in the browser).

There are a few things that still need to be worked out and tested:

  • Testing prebuilds of the bindings.
  • Testing on other OSes (only tested in Windows so far)
  • I could not get rust to build x64 dll on Windows for some reason, so the script currently fetches a release build. I'm not familiar enough with Rust to resolve this currently.
    • I was able to work out the issue with x64 builds, but I'm still fetching latest from the github releases since that happens on install anyways.

@itsame-luigi
Copy link
Contributor Author

I've made some progress on this, and done some additional cleanup on the js side. I've removed the parts of modularize.cjs that dealt with WASM functionality in NodeJS that I added in #80, since WASM is no longer used for NodeJS. I've also changed the browser-specific build outputs back to ./dist/web/... to keep the URL consistent for anyone loading via CDN.

I've also reorganized the scripts in package.json to break things down by build task, and added some clean tasks to make it easier to get back to a clean JS environment.

With these changes, the web output should be mostly unchanged from what it was prior to #80, except for a minor source path change as I moved src/buttplug-rs-ffi to src/web/buttplug-rs-ffi to keep the "node"- and "web"-specific components isolated from each other.

I need to run a few more tests in NodeJS by installing the package on various OSes (probably by packing it into a tarball to avoid needing to publish to npm), but the overall design should be fairly solid now.

@itsame-luigi
Copy link
Contributor Author

Also, it seems the "Core Library + Nuget" builds are failing though I haven't made any changes that should be causing those errors. I'm not sure if its just because its a PR from a forked repository, or if there's some other reason for the failure.

It does look like the CI setup needs to include a scenario for building/testing JS.

@itsame-luigi
Copy link
Contributor Author

@qdot, I think this is as ready as I can make it, if you're able to review.

I can't test the new Nodejs bindings on MacOS, but I've tested it on Windows. I can try and see if it works on Linux via WSL, but I don't have any physical hardware with Linux+Bluetooth aside from WSL.

@qdot
Copy link
Member

qdot commented Aug 15, 2021

Great, thanks for letting me know! Right now I'm neck deep in trying to release buttplug v5, but once that's done I'm hoping to take a look at this. I've got all 3 platforms running on dedicated hardware so I can test those too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants