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

.equals in class Spherical #28191

Closed
chenxinhang12345 opened this issue Apr 22, 2024 · 6 comments
Closed

.equals in class Spherical #28191

chenxinhang12345 opened this issue Apr 22, 2024 · 6 comments
Milestone

Comments

@chenxinhang12345
Copy link

Description

We have .equals in Vector classes for all dimensions. Why cant we have .equals in Spherical?

Solution

Add comparison method for class Spherical just like other VectorN classes, should be pretty simple I guess?

Alternatives

Do nothing but we would definitely want to use something like JSON.stringify() or some other workaround to compare two Spherical?

Additional context

No response

@WestLangley
Copy link
Collaborator

Spherical and Cylindrical coordinates are not unique, and neither class has an equals() method.

Can you please explain your use case?

@chenxinhang12345
Copy link
Author

Yeah you are right. Those classes shouldn't have .equals methods.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 23, 2024

Spherical and Cylindrical coordinates are not unique, and neither class has an equals() method.

Do you mind explaining this in more detail? Why can we compare euclidian coordinates but not spherical ones? The following does work as expected:

const v1 = new THREE.Vector3( 1, 0, 0 );
const v2 = new THREE.Vector3( 1, 0, 0 );
console.log( v1.equals( v2 ) ); // true

const s1 = new THREE.Spherical().setFromVector3( v1 );
const s2 = new THREE.Spherical().setFromVector3( v2 );
console.log( equals( s1, s2 ) ); // true
 
function equals( s1, s2 ) {

	return ( ( s1.radius === s2.radius ) && ( s1.phi === s2.phi ) && ( s1.theta === s2.theta ) );

}

Demo: https://jsfiddle.net/8fbkpr60/1/

@WestLangley
Copy link
Collaborator

... because Spherical and Cylindrical coordinates are not unique, just as 2D polar coordinates are not unique.

const s1 = new THREE.Spherical( 1, 0, 0 );
const s2 = new THREE.Spherical( 1, 0, 2 * Math.PI );

Likewise, quaternion.equals( q ) is also incorrect because quaternions are not unique.

//

As a side note, I am not in favor of methods that compare floating point values using equality. I would have preferred

Vector3.equals( vector, tolerance = 0 ).

@chenxinhang12345
Copy link
Author

Instead of adding .equals(), would it be a good idea to add toVector3() so that we could compare the derived Euclidian coordinates?

@WestLangley
Copy link
Collaborator

@chenxinhang12345 Do this:

const v1 = new THREE.Vector3().setFromSpherical( s1 );

@Mugen87 Mugen87 closed this as completed Apr 23, 2024
@Mugen87 Mugen87 added this to the r164 milestone Apr 23, 2024
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

No branches or pull requests

3 participants