leopard-js / leopard

Library for making Scratch-like projects with JavaScript.
https://leopardjs.com
MIT License
136 stars 26 forks source link

Investigate Sprite.createClone implementation and investigation #166

Open towerofnix opened 1 year ago

towerofnix commented 1 year ago

Particularly this code (and the rest of this method):

https://github.com/leopard-js/leopard/blob/c7e793ba32de207c9cb92741b7ad91848658be1d/src/Sprite.ts#L508-L518

See my comments:


I get it, but this feels like a distinctly ES5-ism. Is it possible to explicitly provide the necessary configuration options through to a normally constructed new Sprite instance? I feel like this risks sharing actual objects (like effects) if we aren't careful.

As far as I can tell Object.create doesn't even call the class constructor (just creating an object whose internal prototype is the provided prototype). It feels really weird to me to "construct" a new instance in a way which bypasses the stated constructor.

class Foo { field = 32; constructor() { this.value = 123; } }

foo = Object.create(Foo.prototype);
console.log(foo.field, foo.value);

foo2 = new Foo();
console.log(foo2.field, foo2.value);

foo3 = Object.create(Object.getPrototypeOf(foo2), Object.getOwnPropertyDescriptors(foo2));
console.log(foo3.field, foo3.value);

I'll branch this off into its own issue since it probably takes some investigation (and deciding if it's worth it), and isn't really relevant to this PR, just wanted to give you a heads up on it since the odd look of the code itself is a little exacerbated with TypeScript.