fenomas / noa

Experimental voxel game engine.
MIT License
616 stars 91 forks source link

migrated index.js to es6 #88

Closed Tubaleviao closed 5 years ago

Tubaleviao commented 5 years ago

Part of the issue #85, please consider it :grinning:

fenomas commented 5 years ago

Hi, Thanks for the contribution but as I mentioned in #85 I'm not currently planning to adopt const/let, since I'm not a big fan of them stylistically, and I don't think there's any performance benefit.

Also, I could be missing something but aren't some of the arrow functions in this PR broken? E.g. this one:

- Engine.prototype.tick = function () {
+ Engine.prototype.tick = () => {

Since changing to arrow functions changes the value of this inside the function.

Tubaleviao commented 5 years ago

It's not all about performance... It's just because when you try to create another variable with the same name, the program will give you an error if you has declared it with "let". And it also help you to let the variable inside the scope. If you declare an "var i=0" in a for loop, you can access this i value from outside the for loop. So if you have two for loops executing at the same time using i, they can affect each other.

I tested here tho check if value of this change from one function to another and it seems to remain the same value. I just don't know how to run this app to test it out with the new arrow functions, should I try it?

fenomas commented 5 years ago

I'm familiar with let and const semantics, thanks! I just don't think the stylistic benefits are worth migrating the codebase to use them - const is somewhat unintuitive, and the current linting rules should mean that there is no variable reuse anywhere, such that let would improve.

About arrow functions, they change the behavior of this.

var obj = {
    value: 5,
    a: function() { return this.value },
    b: () => this.value,
}
obj.a() // 5
obj.b() // undefined

Please test code to make sure it works before submitting PRs, thanks!

Tubaleviao commented 5 years ago

Ok, I guess we shall disconsider this changes.