Allow for early returns in ForEach functions #390
Conversation
lib/ForEach.js
Outdated
| return ForEach.mesh(gltf, function (mesh) { | ||
| return ForEach.meshPrimitive(mesh, function (primitive) { | ||
| var value; | ||
| value = ForEach.meshPrimitiveAttribute(primitive, function (accessorId, attributeSemantic) { |
There was a problem hiding this comment.
For all the value variables in here it should be fine to initialize them as var value = ... since they are all function scoped.
Same note for accessorContainingVertexAttributeData
lib/ForEach.js
Outdated
| @@ -20,12 +20,16 @@ function ForEach() { | |||
| * Fallback for glTF 1.0 | |||
| * @private | |||
| */ | |||
| ForEach.objectLegacy = function(objects, handler) { | |||
| ForEach.objectLegacy = function (objects, handler) { | |||
There was a problem hiding this comment.
The style that Cesium uses (for the most part) is ForEach.objectLegacy = function(objects, handler) {
| var value = handler(object, objectId); | ||
|
|
||
| if (defined(value)) { | ||
| return value; |
There was a problem hiding this comment.
ForEachSpec should be updated to test early returns.
| var value = handler(object, objectId); | ||
|
|
||
| if (defined(value)) { | ||
| return value; |
There was a problem hiding this comment.
Are there any area in gltf-pipeline that can take advantage of early returns now? https://github.com/AnalyticalGraphicsInc/gltf-pipeline/blob/2.0/lib/writeResources.js#L141-L152 comes to mind. If anything else comes up we can just fix as we notice them.
|
Thanks @lilleyse, updated! |
|
Looks good, thanks! |
Restructure
ForEachfunction to allow for early returns when defined values are returned from a handler function. This is often used in Cesium and I think the restructure is easier here than jumping through hoops there.