Merge pull request #6081 from TrevorDev/removeImpostorDuringStep

remove impostor from world after step to avoid cannon exception

dist/preview release/what's new.md

@@ -211,6 +211,7 @@
 - AssetContainer should not dispose objects it doesn't contain. Support for environmentTexture add/remove ([TrevorDev](https://github.com/TrevorDev))
 - Fix `mesh.visibility` not working properly when certain material properties are set that changes the interpretation of alpha (e.g. refraction, specular over alpha, etc.) ([bghgary](https://github.com/bghgary))
 - Fix material and texture leak when loading/removing GLTF with AssetContainer ([TrevorDev](https://github.com/TrevorDev))
+- Avoid exception when removing impostor during cannon world step ([TrevorDev](https://github.com/TrevorDev))
 
 ### Core Engine
 - Fixed a bug with `mesh.alwaysSelectAsActiveMesh` preventing layerMask to be taken in account ([Deltakosh](https://github.com/deltakosh))

src/Physics/Plugins/cannonJSPlugin.ts

@@ -21,6 +21,7 @@ export class CannonJSPlugin implements IPhysicsEnginePlugin {
     private _fixedTimeStep: number = 1 / 60;
     private _cannonRaycastResult: any;
     private _raycastResult: PhysicsRaycastResult;
+    private _removeAfterStep = new Array<PhysicsImpostor>();
     //See https://github.com/schteppe/CANNON.js/blob/gh-pages/demos/collisionFilter.html
     public BJSCANNON: any;
 
@@ -54,6 +55,12 @@ export class CannonJSPlugin implements IPhysicsEnginePlugin {
 
     public executeStep(delta: number): void {
         this.world.step(this._fixedTimeStep, this._useDeltaForWorldStep ? delta : 0, 3);
+        if (this._removeAfterStep.length > 0) {
+            this._removeAfterStep.forEach((impostor) => {
+                this.world.remove(impostor.physicsBody);
+            });
+            this._removeAfterStep = [];
+        }
     }
 
     public applyImpulse(impostor: PhysicsImpostor, force: Vector3, contactPoint: Vector3) {
@@ -165,7 +172,9 @@ export class CannonJSPlugin implements IPhysicsEnginePlugin {
         impostor.physicsBody.removeEventListener("collide", impostor.onCollide);
         this.world.removeEventListener("preStep", impostor.beforeStep);
         this.world.removeEventListener("postStep", impostor.afterStep);
-        this.world.remove(impostor.physicsBody);
+
+        // Only remove the physics body after the physics step to avoid disrupting cannon's internal state
+        this._removeAfterStep.push(impostor);
     }
 
     public generateJoint(impostorJoint: PhysicsImpostorJoint) {

src/Physics/physicsEngine.ts

@@ -117,8 +117,7 @@ export class PhysicsEngine implements IPhysicsEngine {
             var removed = this._impostors.splice(index, 1);
             //Is it needed?
             if (removed.length) {
-                //this will also remove it from the world.
-                removed[0].physicsBody = null;
+                this.getPhysicsPlugin().removePhysicsBody(impostor);
             }
         }
     }

what's new.md

@@ -193,7 +193,7 @@
 - Fixed issue where VRExperienceHelper.onExitingVR observable was being fired twice ([atulyar](https://github.com/atulyar))
 - Avoid firing onExitingVR observable multiple times when calling exitVR() and add observables to Viewer that can be used instead of the ones in VRExperienceHelper ([atulyar](https://github.com/atulyar))
 - GizmoManager should hide existing gizmos if a non-attachable mesh is selected ([TrevorDev](https://github.com/TrevorDev))
-- Ignore isPickable = false for vr ray casting if the mesh's name matches the specified floorMeshName to maintain backwards compatability ([TrevorDev](https://github.com/TrevorDev))
+- Ignore isPickable = false for vr ray casting if the mesh's name matches the specified floorMeshName to maintain backwards compatibility ([TrevorDev](https://github.com/TrevorDev))
 - Fix File Loading if hosted from `file:`-Protocol ([ltetzlaff](https://github.com/ltetzlaff))
 - Do not throw error when updating a controller with no left stick ([TrevorDev](https://github.com/TrevorDev))
 - Exiting VR can result in messed up view ([TrevorDev](https://github.com/TrevorDev))
@@ -236,7 +236,7 @@
 - Animation navbar now updates correctly when a new model is loaded ([RaananW](https://github.com/RaananW))
 - Non-normalized meshes didn't center and focus correctly ([RaananW](https://github.com/RaananW))
 - Meshes with skeletons could have incorrect animations ([RaananW](https://github.com/RaananW))
-- Removed element IDs from viewer's templates to allow muitiple viewers in a single page ([RaananW](https://github.com/RaananW))
+- Removed element IDs from viewer's templates to allow multiple viewers in a single page ([RaananW](https://github.com/RaananW))
 - Viewer is not using Engine.LastCreatedScene anymore, to support multiple viewers in a single page ([RaananW](https://github.com/RaananW))
 - Template location was ignored if html was defined ([RaananW](https://github.com/RaananW))
 - Drag and Drop only worked if a model was already loaded before ([RaananW](https://github.com/RaananW))