Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate Sprite.createClone implementation and investigation #166

Open
towerofnix opened this issue Mar 7, 2023 · 0 comments
Open

Investigate Sprite.createClone implementation and investigation #166

towerofnix opened this issue Mar 7, 2023 · 0 comments
Labels
discussion Looking for feedback and input

Comments

@towerofnix
Copy link
Member

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

leopard/src/Sprite.ts

Lines 508 to 518 in c7e793b

public createClone(): void {
const clone = Object.assign(
Object.create(Object.getPrototypeOf(this) as object) as Sprite,
this
);
clone._project = this._project;
clone.triggers = this.triggers.map((trigger) => trigger.clone());
clone.costumes = this.costumes;
clone.sounds = this.sounds;
clone._vars = Object.assign({}, this._vars);

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Looking for feedback and input
Projects
None yet
Development

No branches or pull requests

1 participant