move item in scope of loader, remove all utility mcData vars with 1

[u9g]

No description provided.

[extremeheat]

I believe the reason behind not putting it in the loader was to not recreate the function every time a new item gets created. Looks like there’s variables in the current scope that get passed into the function so it’s likely the function gets recreated every time now. Not sure how significant the performance impact would be here, but it may be ignorable.

[u9g]

I believe the reason behind not putting it in the loader was to not recreate the function every time a new item gets created. Looks like there’s variables in the current scope that get passed into the function so it’s likely the function gets recreated every time now. Not sure how significant impact would be here, but it may be ignorable.

The problem with not having mcData in scope is that then you can't use many versions of Item since the version changes every time you call the loader.

[extremeheat]

Ok, there’s ways to fix that, I don’t think changing Item versions is a common use case. But look at how prismarine-block handles this

[u9g]

Ok, there’s ways to fix that, I don’t think changing Item versions is a common use case. But look at how prismarine-block handles this

if you would like to you can implement that here too, but we will need mcData.enchantments too in the future, so include that please.

[extremeheat]

I did a benchmark to fill about 4 chests with items:

var g = {}
function loader(salt) {
	g.salt = salt
	return MyClass
}

class MyClass {
	addTwoNumbers(a, b) {
		return a + b + g.salt
	}
}

for(var i = 0; i < 256; i++) {
	var clas = loader(20)
	new clas().addTwoNumbers(40,40)	
}

function loader(salt) {
	return (class MyClass {
		addTwoNumbers(a, b) {
			return a + b + salt
	}})
}

for(var i = 0; i < 256; i++) {
	var clas = loader(20)
	new clas().addTwoNumbers(40,40)	
}

like above

Static x 40,590 ops/sec ±5.06% (77 runs sampled)
Runtime x 479 ops/sec ±4.83% (67 runs sampled)
Fastest is Static

with static classes:

Static x 140,122 ops/sec ±3.89% (77 runs sampled)
Runtime x 1,356 ops/sec ±6.50% (61 runs sampled)
Fastest is Static

So it's 98% slower instantiating a new object

[u9g]

if you feel like its needed, you can implement that here too, but we will need mcData.enchantments too in the future, so I'm not sure how we can pass that in

[extremeheat]

All you have to do is return a reference to the class with a non-static method like above. Users can be responsible for instantiating that class, or not, to use it as static. What you're doing here is basically runtime code generation when the function gets called vs the file parse time.

class A {
  constructor(a, b) { this.a = a; this.b = b; }
  static addStatic(a, b) { return a + b }
  add() { return this.a + this.b }
}

var a = new A(4,4)
a.add() => 8
A.add(4,4) => 8

[u9g]

All you have to do is return a reference to the class with a non-static method like above. Users can be responsible for instantiating that class, or not, to use it as static.

class A {
  constructor(a, b) { this.a = a; this.b = b; }
  static addStatic(a, b) { return a + b }
  add() { return this.a + this.b }
}

var a = new A(4,4)
a.add() => 8
A.add(4,4) => 8

standard has problems with the static keyword. & we always need mc version

[extremeheat]

No, that's only for static and class variables (a ES7 feature), static functions (ES6) should work fine.
standardjs doesn't support newer node features, so things from newer versions do break. See standard/standard#1165