Merge pull request #4624 from TrevorDev/obervableHandleRemoveDuringNotify

Observable handle remove during notify

dist/preview release/what's new.md

@@ -96,8 +96,9 @@
 - VR experience helper's controllers will not fire pointer events when laser's are disabled, instead the camera ray pointer event will be used ([TrevorDev](https://github.com/TrevorDev))
 - Node's setParent(node.parent) will no longer throw an exception when parent is undefined and will behave the same as setParent(null) ([TrevorDev](https://github.com/TrevorDev))
 - Mesh.MergeMeshes flips triangles on meshes with negative scaling ([SvenFrankson](http://svenfrankson.com))
-- Avoid firing button events multiple times when calling vrController.attachMesh() ([TrevorDev]
+- Avoid firing button events multiple times when calling vrController.attachMesh() ([TrevorDev](https://github.com/TrevorDev))
 - Parse geometry when load binary mesh ([SinhNQ](https://github.com/quocsinh))
+- Removing observers during observable notify should not skip over valid observers ([TrevorDev](https://github.com/TrevorDev))
 
 ### Core Engine
 

src/Tools/babylon.observable.ts

@@ -217,8 +217,7 @@
             var index = this._observers.indexOf(observer);
 
             if (index !== -1) {
-
-                this._observers.splice(index, 1);
+                this._deferUnregister(observer);
                 return true;
             }
 
@@ -236,7 +235,7 @@
 
             for (var index = 0; index < this._observers.length; index++) {
                 if (this._observers[index].callback === callback && (!scope || scope === this._observers[index].scope)) {
-                    this._observers.splice(index, 1);
+                    this._deferUnregister(this._observers[index]);
                     return true;
                 }
             }
@@ -248,10 +247,27 @@
             observer.unregisterOnNextCall = false;
             observer._willBeUnregistered = true;
             Tools.SetImmediate(() => {
-                this.remove(observer);
+                this._remove(observer);
             })
         }
 
+        // This should only be called when not iterating over _observers to avoid callback skipping.
+        // Removes an observer from the _observer Array.
+        private _remove(observer: Nullable<Observer<T>>): boolean {
+            if (!observer) {
+                return false;
+            }
+
+            var index = this._observers.indexOf(observer);
+
+            if (index !== -1) {
+                this._observers.splice(index, 1);
+                return true;
+            }
+
+            return false;
+        }
+
         /**
          * Notify all Observers by calling their respective callback with the given data
          * Will return true if all observers were executed, false if an observer set skipNextObservers to true, then prevent the subsequent ones to execute