Fix _removeObjectsWithPrefix For Duplicate Object Names

[dhakim87]

Resolves #748

The underlying issue is that axes were created with duplicate names in the scene when multiple axes were set to the same axis index. This would cause objects to be leftover when switching states. This was addressed by making _removeObjectsWithPrefix remove -all- objects with a given prefix, rather than 1 with each name. This does however require one additional full search of the tree.

[dhakim87]

Either should work, as long as you can guarantee we generate separate remove calls for each object with a duplicate name. If you’re worried about the cost of traversing the tree, we should double check that three.js doesn’t look up names with a full scan also.

Sent from my iPhone

On Jan 15, 2020, at 3:44 PM, Yoshiki Vázquez Baeza notifications@github.com wrote:

 @ElDeveloper requested changes on this pull request.

Thanks for finding the issue @dhakim87. Based on our conversation earlier today, I've suggested a different approach that wouldn't require a recursive traversal of objects in the scene. I tested the new approach and it worked, but of course do let me know if this doesn't work in some of your tests.

In emperor/support_files/js/sceneplotview3d.js:

*

*/ ScenePlotView3D.prototype._removeObjectsWithPrefix = function(prefix) { var scope = this;

• .each(.range(this.decViews.scatter.decomp.dimensions), function(i) { After looking into this, I think the old function would still work, as long as we update the objects we iterate over. Instead of iterating over decomp.dimensions (which would go from 1 - N), we need to iterate over scenePlotView.visibleDimensions (which tracks the index of the visible dimensions). Something like this should work:

ScenePlotView3D.prototype.removeObjectsWithPrefix = function(prefix) { var scope = this; .each(this.visibleDimensions, function(i) { var objectWithPrefix = scope.scene.getObjectByName(prefix + i); scope.scene.remove(objectWithPrefix); }); }; — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[ElDeveloper]

If you’re worried about the cost of traversing the tree, we should double check that three.js doesn’t look up names with a full scan also.

This is a good point, as far as I recall they both follow a similar approach i.e. traversing the whole tree: https://github.com/mrdoob/three.js/blob/dev/src/core/Object3D.js#L352

Do you think we could do the removal of objects in a smarter way with this in mind?

Also, I'm looking into solving the issues with the build failing in #752.

[ElDeveloper]

Closing in favor of #755