Change 'each' function scope to first argument

[matthijsgroen]

https://github.com/craftyjs/Crafty/blob/f3982baa51af24d4e8a14a3194fd3f0e92a2dcba/src/core/core.js#L949

Currently, the this scope is set for a passed function. this creates scope juggling.

Crafty("2d").each(function() { 
  // work with a magic 'this', and  loosing the outer scope.
});

Code I wish I had:

Crafty("2d").each(entity => { 
  // no magic 'this', and keeping the outer scope.
});

It's essentially changing the code above to:

  func(entities[this[i]], i); 

It makes it more in line how forEach, map etc work in JS.

Problem is, it is API breaking.

Any thoughts?

[matthijsgroen]

ps. I could create a forEach function for it, so that it does not break the API of the old function. But it would create confusion.

[starwed]

I think this is one of the many core Crafty methods that were conceptually borrowed from jQuery -- jQuery's each has the same behavior.

I'll do some more research into our options here.

[matthijsgroen]

The function of jQuery does provide the element as second argument to the function, so that you can dodge the 'this' scope changing when using lambda functions.

Personally I would prefer vanillaJS way of working above jQuery 😅

[matthijsgroen]

I've fixed it by adding this after loading crafty:

Crafty.fn.forEach = function(iterator) {
  this.each(function(index) {
    iterator(this, index);
  });
};

So now I can use:

Crafty("Enemy").forEach(enemy => enemy.destroy())