Update src/extras/geometries/PlaneGeometry.js #2458

Closed
wants to merge 1 commit into
from

Conversation

6 participants
Contributor

Wilt commented Sep 27, 2012

First proposal for changes from me on Github.
I hope it is all according to the houserules...
I'm not a pro programmer, but I started working on a big WebGL ThreeJS project, so I might come with more additions or comments.

Collaborator

WestLangley commented Sep 28, 2012

Well, I don't like the terminology change at all, but if you are going to do it, it needs to be done consistently across all geometries.

Contributor

alteredq commented Sep 28, 2012

I didn't even realize before this was ambiguous. Maybe just something like this?

THREE.PlaneGeometry = function ( width, height, widthSegments, heightSegments ) 

or more verbose

THREE.PlaneGeometry = function ( width, height, widthSegmentsCount, heightSegmentsCount ) 

or

THREE.PlaneGeometry = function ( width, height, widthTessellation, heightTessellation ) 
Collaborator

WestLangley commented Sep 28, 2012

THREE.PlaneGeometry = function ( width, height, widthSegments, heightSegments )

sounds OK. Of course, that implies radiusSegments, tubeSegments, etc...

Owner

mrdoob commented Sep 28, 2012

THREE.PlaneGeometry = function ( width, height, widthSegments, heightSegments )

sounds OK. Of course, that implies radiusSegments, tubeSegments, etc...

Sounds good to me.

Contributor

gero3 commented Sep 29, 2012

THREE.PlaneGeometry = function ( width, height, widthSegments, heightSegments )

sounds OK. Of course, that implies radiusSegments, tubeSegments, etc...

Sounds good to me.

I also think it would be a better change.

Owner

mrdoob commented Sep 29, 2012

Anyone doing a patch for it?

Contributor

Wilt commented Sep 30, 2012

widthSegments, heightSegments sounds good to me too, but I think widthSegmentsCount, heightSegmentsCount is even more unambiguous. I do realize it is a long name, maybe that's a downside.
@mrdoob I started the whole topic so I'd love to help out, but I think I don't have rights to fix it do I?

mrdoob closed this Sep 30, 2012

mrdoob reopened this Sep 30, 2012

mrdoob closed this in a495334 Oct 15, 2012

Contributor

jpryne commented Oct 21, 2012

Inconsistency remains across primitive types:

For example, CubeGeometry uses "segmentsHeight"
whereas CylinderGeometry uses "heightSegments".

New issue?

Owner

mrdoob commented Oct 21, 2012

I think it's fixed on the dev branch.

Contributor

alteredq commented Oct 21, 2012

Yup, CubeGeometry should be already fixed. If you find some other forgotten ones let us know or you can make a pull request.

@wvl wvl pushed a commit to wvl/three.js that referenced this pull request Nov 28, 2012

@mrdoob mrdoob (threejs src) segments* to *Segments. Closes #2458. 89d7106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment