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

Examples: Move jsm/nodes to ES6. #21801

Merged
merged 1 commit into from
May 8, 2021
Merged

Examples: Move jsm/nodes to ES6. #21801

merged 1 commit into from
May 8, 2021

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented May 8, 2021

Related issue: -

Description

Since devs still work at this part of the repository, it's actually better to move it to ES6, too.

@mrdoob
Copy link
Owner

mrdoob commented May 8, 2021

Which should be merged first? This one or #21800?

@donmccurdy
Copy link
Collaborator

Let's merge this PR first, I can clean up the other one afterward. Will be glad to have ES6 Class syntax here! 👍

@Mugen87 Mugen87 merged commit b13eccc into mrdoob:dev May 8, 2021
@Mugen87 Mugen87 added this to the r129 milestone May 8, 2021
@Mugen87
Copy link
Collaborator Author

Mugen87 commented May 8, 2021

@sunag I have not converted the following line to ES6:

name = TempNode.prototype.generate.call( this, builder, output, uuid, data.output, ns );
const code = this.generate( builder, type, uuid );

I'm not sure why generate() is called twice in different ways? 🤔

@sunag
Copy link
Collaborator

sunag commented May 8, 2021

A long time ago I made this block of code, it is need of some revisions...
The use prototype is to prevent call the extended abstract function without call the super.generate().

I don`t test it but we could do something different now, like this for example:

..

name = this.generateTempVar( builder, output, uuid, data.output, ns );

const code = this.generate( builder, type, uuid );

..

generateTempVar( builder, output, uuid, type, ns ) {

	if ( ! this.getShared( builder, output ) ) console.error( 'THREE.TempNode is not shared!' );

	uuid = uuid || this.uuid;

	return builder.getTempVar( uuid, type || this.getType( builder ), ns, this.getLabel() ).name;

}

generate( builder, output, uuid, type, ns ) {

	return this.generateTempVar( builder, output, uuid, type, ns );

}

@mrdoob
Copy link
Owner

mrdoob commented May 8, 2021

Thanks!

@joshuaellis joshuaellis mentioned this pull request May 8, 2021
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants