Get examples working at 1.3 #4

diarmidmackenzie · 2022-10-02T09:48:24Z

Fixes as follows:

Updated examples to use A-Frame 1.3.0
Updated to use three-to-cannon v4
Updated examples to use latest environment component & sphere colliders
Updated CANNON-shape2mesh to use BufferGeometry rather than Geometry. This affects all of: convex polyhedron, trimesh & heightfield. Only convex polyhedron has been tested. Trimesh and heightfield are implemented in CANNON.shape2mesh (copied from here, but not actually supported in aframe-physics-system, hence code changes are untested).
Updated invert -> inverse for Quaternions (reflecting THREE.js interface change)
Fixed issues with force-pushable resolved by tightening up raycast targets
Added interactivity to compound & constraints
Removed cannon worker driver from examples. It doesn't work with some features. I'll raise a separate issue to track that.
Added commentary and code links to each example.

This PR also includes the changes submitted under PR#1 (though they aren't actually needed to get 1.3.0 examples working). Can revert those if we want to merge this without merging that.
#1

This breaks Cannon.js examples.

This solves the issue with deprecation of THREE.Geometry in A-Frame 1.2.0

Haven't tested this yet...

diarmidmackenzie · 2022-10-02T09:49:11Z

Live version of the code on this branch can be accessed here:
https://diarmidmackenzie.github.io/aframe-physics-system/examples/

diarmidmackenzie · 2022-10-02T09:52:46Z

Sadly I seem to have completely broken the tests on this branch.

Will look into this...

vincentfretin · 2022-10-02T10:06:06Z

src/components/body/body.js

@@ -329,7 +336,7 @@ var Body = {
      } else {
        q1.copy(body.quaternion);
        parentEl.object3D.getWorldQuaternion(q2);
-        q1.premultiply(q2.invert());
+        q1.premultiply(q2.inverse());


I don't understand why you did this change.
Quaternion inverse() was renamed to invert() since three r125. I think you want to keep invert() here and in other places where you did this change.

Thanks for catching this. I was getting "invert() is not a function" errors. Must have been somehow using an old version of THREE that didn't support invert() yet? I'll investigate further.

There is also "aframe": ">=0.5.0" in peerDependencies, you may have an issue with that because it doesn't match the version in devDependencies, so it's actually running with a really old super-three that is included in aframe 0.5.0?
I'm not sure why we have three in devDependencies here, we don't seem to have imports from three in the code.

I have reverted this. Expected it to break the tests since the tests still run against A-Frame 1.1.0, but it didn't - I guess because there is so little actual test coverage.

vincentfretin · 2022-10-02T10:08:31Z

package.json

@@ -71,7 +71,7 @@
    "replace": "^1.2.0",
    "sinon": "^2.4.1",
    "sinon-chai": "^2.14.0",
-    "three": "^0.123.0",
+    "three": "^0.125.0",


We probably want to test against aframe ^1.3.0 and three ^0.137.0.

Yes, sensible. I was going for the minimum required change here, but if I don't introduce new issues upgrading further I'll do what you suggest. (and if I do, I guess we'll need to fix them anyway).

Yes, and if you can, can you please test the examples with an aframe master build as well? aframe master is currently using three r144, you may catch some warnings to be fixed.

OK, so the test issue that I hit here: #4 (comment) turns out to appear whenever the tests use A-Frame > 1.1.0.

I don't understand this but moving tests to A-Frame 1.2.0+ breaks them as the browserify build step fails.

I'll raise a separate issue for that, but for now it's better to have tests that run cleanly against 1.1.0 than tests that don't work at all against 1.2.0 or 1.3.0.

That said it is a little moot as the tests barely cover anything anyway...!

FYI aframe 1.1.0 is using super-three 0.123.1 so it's Quaternion.inverse.
aframe 1.2.0 is using super-three 0.125.0 so it's Quaternion.invert. so aframe 1.2.0 should be the minimum we support here. But yeah because the tests are not covering much of the code, it probably doesn't fail with aframe 1.1.0. :)
I'm ok with keeping 1.1.0 here for now in this PR, we should probably move from browserify to webpack anyway in a separate PR like I did with aframe and networked-aframe.

Separate issue raised here:
#5

If moving to webpack will make this obsolete that sounds like a good option too.

vincentfretin · 2022-10-02T10:21:13Z

Thank you for doing this work!
We can merge only this PR and close the #1 or merge #1 first and rework this one, I'm ok with both.

…failures.

vincentfretin · 2022-10-02T11:03:38Z

We should replace all https://rawgit.com urls in the examples by jsdelivr or unpkg.

diarmidmackenzie · 2022-10-02T11:18:54Z

We should replace all https://rawgit.com urls in the examples by jsdelivr or unpkg.

Done

src/components/body/ammo-body.js

lib/CANNON-shape2mesh.js

kylebakerio

these look good, just a matter of making those last recommendations from vincent. I did have on question here, but I assume there's a reason.

lib/CANNON-shape2mesh.js

kylebakerio · 2022-10-05T23:45:44Z

So, we're waiting to merge on:

removing unnecesary Vector3's
adding reparenting comment

Then Vincent will rework to webpack in a separate PR.

That right?

vincentfretin · 2022-10-06T05:59:47Z

Yes.

diarmidmackenzie · 2022-10-06T16:25:22Z

removing unnecesary Vector3's
adding reparenting comment
Both done.

Also...

one of the inverse/invert confusions was not a Quaternion and should be inverse. Turns out this was the issue, nothing to do with old versions of THREE.js
some wrinkles with the parentEl merge now sorted out.

Examples are all now clean. I think tis is now ready to merge. Just wwaiting to hear if anyone has any concerns, as it's ended up quite a large PR.

kylebakerio · 2022-10-06T19:54:37Z

Looks good to me. At this point, it's obviously a massive improvement over what was before it, we've all looked over it, and the best way to find any more issues is to merge it and use it.

Thanks for this excellent stuff, there's not many people out there who are able and willing to do this. Very excited to use this in a current project of mine immediately!

kylebakerio · 2022-10-06T20:07:56Z

src/components/ammo-constraint.js

@@ -69,7 +69,7 @@ module.exports = AFRAME.registerComponent("ammo-constraint", {

    const bodyTransform = body
      .getCenterOfMassTransform()
-      .invert()
+      .inverse()


Yeah, that's super sneaky. Was indeed confusing. But inverse() does seem to be what's actually available here: https://github.com/kripken/ammo.js/blob/99d0ec0b1e26d7ccc13e013caba8e8a5c98d953b/bullet/src/BulletDynamics/ConstraintSolver/btGeneric6DofSpringConstraint.cpp#L178-L179

You might add a comment here about this, because this could be confused by someone else in the future.

vincentfretin · 2022-10-07T08:32:23Z

All good, thanks @diarmidmackenzie

diarmidmackenzie added 17 commits October 1, 2022 11:04

Update everything to 1.3.0

1ec9379

This breaks Cannon.js examples.

Migrate to three-to-cannon v4

56e2b52

This solves the issue with deprecation of THREE.Geometry in A-Frame 1.2.0

Move to maintained environment component

6887318

Move Geometry to Buffer Geometry.

d5a11ca

Haven't tested this yet...

Use latest sphere collider

7ab95ae

Updated builds

5ba0ff9

Update components

910da38

Update to latest sphere collider

edf7ea0

invert -> inverse

429f10d

Use correct offset & orientation from three-to-cannon

f243785

Show debug info in example

c175eb3

Focus cursor on pushable entities.

b5df39b

Focus cursor on pushable entities.

04f8206

replace camera userHeight with position

78a18c0

Add interactivity via cursor

4261970

Disabled "driver: worker" in examples as it's not working.

cdf3646

Add commentary and code links to each example.

f295387

diarmidmackenzie requested review from kylebakerio and vincentfretin October 2, 2022 09:49

vincentfretin reviewed Oct 2, 2022

View reviewed changes

diarmidmackenzie added 3 commits October 2, 2022 11:45

Change dev dependencies to fix npm run test error

0eb9c92

Don't allow a-frame past 1.1.0 in test environment as it causes test …

f8d4b75

…failures.

invert() is correct after THREE r125

96137c2

Replace rawgit URLs with jsdelivr

0421e33

diarmidmackenzie mentioned this pull request Oct 2, 2022

Unit Tests don't work with A-Frame 1.2.0+ #5

Open

vincentfretin reviewed Oct 2, 2022

View reviewed changes

src/components/body/ammo-body.js Outdated Show resolved Hide resolved

vincentfretin reviewed Oct 2, 2022

View reviewed changes

lib/CANNON-shape2mesh.js Show resolved Hide resolved

kylebakerio reviewed Oct 5, 2022

View reviewed changes

lib/CANNON-shape2mesh.js Show resolved Hide resolved

diarmidmackenzie added 8 commits October 6, 2022 15:54

Revert change that shouldn't be on this branch

597682a

Revert change that shouldn't have been on this branch

48a1db4

Revert change that shouldn't have been on this branch

33c702f

Merge branch 'master' into examples-to-1.3

338a5a3

FIx merge error

f9dc225

This is not a THREE.Quaternion. inverse() is still correct.

023c85c

Optimization - create fewer THREE.Vector3s.

dc7dcf4

Update builds

7001a5b

kylebakerio reviewed Oct 6, 2022

View reviewed changes

kylebakerio merged commit 6e3fccd into c-frame:master Oct 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get examples working at 1.3 #4

Get examples working at 1.3 #4

diarmidmackenzie commented Oct 2, 2022

diarmidmackenzie commented Oct 2, 2022

diarmidmackenzie commented Oct 2, 2022

vincentfretin Oct 2, 2022

diarmidmackenzie Oct 2, 2022 •

edited

Loading

vincentfretin Oct 2, 2022

diarmidmackenzie Oct 2, 2022

vincentfretin Oct 2, 2022

diarmidmackenzie Oct 2, 2022

vincentfretin Oct 2, 2022

diarmidmackenzie Oct 2, 2022

vincentfretin Oct 2, 2022

diarmidmackenzie Oct 2, 2022

vincentfretin commented Oct 2, 2022

vincentfretin commented Oct 2, 2022

diarmidmackenzie commented Oct 2, 2022

kylebakerio left a comment

kylebakerio commented Oct 5, 2022

vincentfretin commented Oct 6, 2022

diarmidmackenzie commented Oct 6, 2022

kylebakerio commented Oct 6, 2022

kylebakerio Oct 6, 2022 •

edited

Loading

vincentfretin commented Oct 7, 2022

Get examples working at 1.3 #4

Get examples working at 1.3 #4

Conversation

diarmidmackenzie commented Oct 2, 2022

diarmidmackenzie commented Oct 2, 2022

diarmidmackenzie commented Oct 2, 2022

Choose a reason for hiding this comment

diarmidmackenzie Oct 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vincentfretin commented Oct 2, 2022

vincentfretin commented Oct 2, 2022

diarmidmackenzie commented Oct 2, 2022

kylebakerio left a comment

Choose a reason for hiding this comment

kylebakerio commented Oct 5, 2022

vincentfretin commented Oct 6, 2022

diarmidmackenzie commented Oct 6, 2022

kylebakerio commented Oct 6, 2022

kylebakerio Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

vincentfretin commented Oct 7, 2022

diarmidmackenzie Oct 2, 2022 •

edited

Loading

kylebakerio Oct 6, 2022 •

edited

Loading