Fix Browserify Integration #5
Conversation
2 changes were necessary:
- make the PhysijsWorker for Browserify a function/ES5 constructor
- unlike worker-loader, webworkify already instantiates the Worker,
so the `new PhysijsWorker()` instantiantion would error out
- wrapping `work`'s output in a function/constructor resolves this
- wrap `physijs_worker.js` into a module that exports a single
argumentless function
- this is required by the webworkify spec actually, but is quite
different from how worker-loader works
- all code outside this function runs on the main thread, so without
the stub module, this breaks a lot of things due to worker code
running in main thread
Browserify integration is quite a bit more complex than I realized 😅
- this was auto-fixed when I ran some commands - this artifact is leftover from the fork
- ignore some Node stuff and build/ folder where browserify and webpack bundles will go
- add test utilities for browserify to handle exporting and importing
from a bundle
- we need to test the actual output of the bundler to make sure the
integration works
- add some initial scripts for testing
- haven't set up the runner or a _real_ assertion yet
- that's for next commit!
(deps): add browserify, three, and webworkify as devDeps for testing
(deps): add browser-env, jsdom-worker, and node-fetch
- browser-env to handle the browser necessities for testing in Node
- basically just a global export of jsdom
- jsdom-worker because jsdom does not currently implement workers on
its own
- jsdom-worker runs it in the same thread actually, so it's not a
full implementation either
- Node now supports worker_threads so maybe jsdom will soon too
- using a fork due to some bugs in the original jsdom-worker
- node-fetch is a peerDep of jsdom-worker
(deps): add a package-lock.json bc we got deps now
- for now, create the Scene like before (which instantiates a Worker) and then just check that a property of the Scene is correct - use ES6+ here because this isn't being packaged and therefore doesn't have a need to be similar to PhysiJS (deps): add ava as the test runner - rename some files to start with underscore to follow ava's convention for helpers - move browser-env/worker stuff to a separate file that's pre-loaded before tests per ava's browser-env recipe
- README.md and package.json are always included as well - tested with `npm pack`!
- patch fix for browserify integration
|
@mpaccione if you could test this PR/branch in your project ( I've added a test here, but it's quite small. I think it should be good enough but not really sure what surface area the tests should actually capture for an accurate baseline (don't want to be testing PhysiJS itself). If you could confirm that this test is enough and that the browserify integration is now working, that would add some more certainty. |
|
@agilgur5 Okay this runs! Physijs.Scene() executes without error. |
@mpaccione have you used any other PhysiJS objects in your project yet? Or just creating the scene so far? Because correct creation of |
|
@agilgur5 Haha, yes I have used Box Meshes and Materials with Friction values and such with no logged error :) I emailed chandler, the creator of Physijs about the continued maintenance of the library. He currently has no intention of maintaining it :( He was open to someone else doing that though ha! Right now, my next big challenge is to investigate the possibility of integrating BufferGeometry support into the library. I have large (massive) planes for the landscape and standard plane geometries require too much memory / can crash the browser. |
Great! Will merge this in in a bit :) Thanks for the help!
I'm kinda surprised he even responded, been a while since it got an update and v2 which uses GoblinPhysics never got merged in. He upvoted my comment when I got this library to work so I guess he occasionally has an eye on it. PhysiJS isn't altogether that huge at 3k LoC, but the possible surface area for features is much larger and it's pretty complex because physics. The Three integration is also painful, though hopefully Three might become more modularized, pluggable, and SemVer compatible at some point -- all
😅 yea that sounds tough. LTLMoPWeb3D had some pretty simple Three & Physijs integrations, but it spikes CPU and memory hard (it also runs its own simulation loop and can have even more workers for ROS integration, but I believe it was spiking even before both of those existed). I'm curious if Unity's in-browser performance differs |
|
Released as v0.1.2 |
This PR resolves #2 and replaces #3 .
It also adds a test for certainty, resolving #4 as well (at least on the Browserify side). I don't have a project I could even test it out in (hence the issue in the first place), so a test is very helpful. Will also pave the way forward for more tests, CI, etc.
A lot of work went into bugfixing/trial-and-erroring the
webworkifyintegration as well as setting up the test harness correctly to support al the browser nuances necessary to get the code to even run.