n5ro / aframe-physics-system

Physics system for A-Frame VR, built on CANNON.js.
https://n5ro.github.io/aframe-physics-system/
MIT License
505 stars 136 forks source link

Our version of Cannon calls depreciated THREE.Geometry 14 times #189

Open n5ro opened 3 years ago

n5ro commented 3 years ago

Our version of Cannon calls depreciated THREE.Geometry 14 times. Originally when I searched the Aframe Physics System I found almost no instance of THREE.Geometry inside this repo. It turns out that searching github does not find everything that is in your code. Do not rely on github searches. If you visit this CDN file and search for "THREE.Geometry" it comes up 14 times in areas that refer to Cannon. This means that until we get this fixed Cannon is not going to work in the Aframe Physics System. If you were thinking about going with Ammo anyways today would be a good day to go that direction. I am going to try to fix our implementation of Cannon by using BufferGeometry, but I am not an expert, and fixing this may take a while, so if you folks can look at this code below and help me identify all the things that need to be changed that would help, and if you can help me figure how to make the right changes that is even better, and if you wish also you may skip the dialog with me and just submit a PR yourself to try to fix some of these things that need fixing. Go for it in the last case. https://cdn.jsdelivr.net/gh/n5ro/aframe-physics-system@v4.0.1/dist/aframe-physics-system.js

donmccurdy commented 3 years ago

Cannon.js itself does not know about any part of three.js — it can be used with both three.js and babylon.js, it's up to the user to create CANNON.Shape objects from whatever graphics library you happen to be using. In the case of three.js, that conversion is handled by the three-to-cannon project. Inspecting the compiled source code of A-Frame Physics System...

https://unpkg.com/aframe-physics-system@4.0.1/dist/aframe-physics-system.js

... it looks like nearly all (and perhaps all?) of the references to THREE.Geometry are coming from the three-to-cannon library. This might explain why you don't see these references in GitHub — GitHub searches the local source code, but not the dependencies that eventually get bundled into the published build. I'm working on updating three-to-cannon so that it does not rely on THREE.Geometry, you can track that progress in https://github.com/donmccurdy/three-to-cannon/issues/36. Once v4 of three-to-cannon is published, I'd recommend upgrading the version used here and confirming that things work as expected.

On the other hand, conversion of three.js objects to Ammo.js bodies is handled by the three-to-ammo package. I don't maintain that package, but it appears to work fine with both Geometry and BufferGeometry types already, and so it might not need any changes.

donmccurdy commented 3 years ago

v4 of three-to-cannon is published now — updating here should remove all of its references to THREE.Geometry, but note there are a few API changes shown in the readme:

https://github.com/donmccurdy/three-to-cannon#api

arpu commented 3 years ago

nice i testing this now, but hit the problem the result shape does not include the body?

arpu commented 3 years ago

ok looks like result.shape works fine but not const {shape, offset, quaternion} = result; shape.body is null

arpu commented 3 years ago

found something different in Box generation first is the old second the new Bildschirmfoto von 2021-05-13 21-17-58

arpu commented 3 years ago

@donmccurdy should i create a bugreport on three-to-cannon repo?

donmccurdy commented 3 years ago

Can't guess from the screenshot here sorry, but if you can share code to reproduce the issue and file a bug on three-to-cannon i'll take a look. 👍