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

LatheGeometry: Make all parameters optional. #21540

Closed
wants to merge 1 commit into from
Closed

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Mar 29, 2021

Related issue: see #21536 (comment)

Description

Ensures users can do new LatheGeometry() like with other generators. Right now a runtime error occurs.

The default shape is a simple lamp shade.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented May 2, 2021

Seems the PR did not get enough support. Closing.

@Mugen87 Mugen87 closed this May 2, 2021
@mrdoob
Copy link
Owner

mrdoob commented Sep 7, 2021

Sorry, this got lost in between all the ES6 class craziness.

Is this the only geometry left to solve the polymorphism issue?

If so, how about this default shape instead?

[ new Vector2( 0, 0.5 ), new Vector2( 0.5, 0 ), new Vector2( 0, - 0.5 ) ]

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Sep 7, 2021

Is this the only geometry left to solve the polymorphism issue?

Unfortunately, no. ShapeGeometry, ExtrudeGeometry, TextGeometry and ParametricGeometry also require a change.

EdgesGeometry and WireframeGeometry, too. To me however, these are helper classes and should be better placed in the helpers directory imo. But that is a different matter.

If so, how about this default shape instead?

That looks good, too!

@mrdoob
Copy link
Owner

mrdoob commented Sep 7, 2021

Unfortunately, no. ShapeGeometry, ExtrudeGeometry, TextGeometry and ParametricGeometry also require a change.

EdgesGeometry and WireframeGeometry, too. To me however, these are helper classes and should be better placed in the helpers directory imo. But that is a different matter.

I think I agree with that. But definitely not in a hurry to organize all that.

If so, how about this default shape instead?

That looks good, too!

Okay, yeah. Lets do this 👍
That way we don't need this code in the editor:

var points = [
new THREE.Vector2( 0, 0 ),
new THREE.Vector2( 0.4, 0 ),
new THREE.Vector2( 0.35, 0.05 ),
new THREE.Vector2( 0.1, 0.075 ),
new THREE.Vector2( 0.08, 0.1 ),
new THREE.Vector2( 0.08, 0.4 ),
new THREE.Vector2( 0.1, 0.42 ),
new THREE.Vector2( 0.14, 0.48 ),
new THREE.Vector2( 0.2, 0.5 ),
new THREE.Vector2( 0.25, 0.54 ),
new THREE.Vector2( 0.3, 1.2 )
];

@mrdoob
Copy link
Owner

mrdoob commented Sep 7, 2021

Ops, can't reopen this.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Sep 7, 2021

Sorry, I have forse-pushed the branch. I'll create a new PR 👍 .

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.

2 participants