three-types / three-ts-types

TypeScript types for the popular 3D library three.js
237 stars 155 forks source link

Euler type does not match ThreeJs declaration #355

Closed arcasoy closed 1 year ago

arcasoy commented 1 year ago

Problem description:

Relevant code:

Example of where the error occurs:

function dampEuler(v: Euler, t: Euler, lambda: number, delta: number) {
  v.x = MathUtils.damp(v._x, t._x, lambda, delta);
  v.y = MathUtils.damp(v._y, t._y, lambda, delta);
  v.z = MathUtils.damp(v._z, t._z, lambda, delta);
}

Suggested solution:

Redefine type with ._x, ._y, and ._z instead of x, y, z

arcasoy commented 1 year ago

@rafaelsc will you be handling the code change for this considering you referenced that larger PR or is that only documentation focused?

Methuselah96 commented 1 year ago

I think they referenced this issue by mistake.

Methuselah96 commented 1 year ago

I'm not quite sure I understand the issue. It seems like the three.js source code you linked to defines x, y, and z with getters/setters as the proper way of accessing those variables. The _x, _y, and _z variables are private variables that are not meant to be accessed directly by the user.

arcasoy commented 1 year ago

My issues lie in directly accessing the attributes. In my app, the Euler's are saved and upon reload get assigned to a variable of type Euler. When this occurs, the methods of the object are no longer part of the Euler, and instead only the data exists. Because of this, the getters and setters and no longer defined, and only the public variables _x, _y, and _z are accessible.

I'm providing a screenshot of two Eulers, one of which is from the camera in the application so all the methods are there, while the other was saved to state before printed. To do a comparison between these two, it seems like I must access the _ attributes, but the types argue that those attributes don't exist. image

Specifically my goal is to run those two through the code in the original post to damp the Euler between two values.

Methuselah96 commented 1 year ago

In my app, the Euler's are saved and upon reload get assigned to a variable of type Euler.

It doesn't sound like it's actually an Euler then, it's just an object that contains (private and public) data of an Euler and not an instance of the Euler class. Is there a way for you to actually construct an Euler instead of only having an object with the underlying data?

If you have code demonstrating the issue, that would be helpful as well.

arcasoy commented 1 year ago

Unfortunately when the application saves, all data is stringifies and parsed when returned. The states, defined with TS as thisIsAnEulerVar:Euler are set to the values, but not instantiated on reload.

Perhaps it's a misunderstanding on my part, but I'm a little confused why you say that _x, _y, and _z are private in your comment above? Here MDN lists private fields as those denoted with a #. Additionally, if these attributes were private, I don't see why I can clearly access them. My function above works with the _'s included, but TypeScript argues that the syntax is improper.

Additionally, the Three link I posted above only has this._x, this._y, and this._z, the x, y, and z are only methods, not class members.

I have provided some code above where the issue occurs, but let me know what in specific you need in addition and I can try to provide it. Thanks for all of the help

Methuselah96 commented 1 year ago

Unfortunately when the application saves, all data is stringifies and parsed when returned. The states, defined with TS as thisIsAnEulerVar:Euler are set to the values, but not instantiated on reload.

Unfortunately thisIsAnEulerVar is incorrectly typed then, because it's not actually an Euler, it's just an object that contains the data that an Euler contains.

Perhaps it's a misunderstanding on my part, but I'm a little confused why you say that _x, _y, and _z are private in your comment above? Here MDN lists private fields as those denoted with a #. Additionally, if these attributes were private, I don't see why I can clearly access them. My function above works with the _'s included, but TypeScript argues that the syntax is improper.

The reason I keep calling them private is because the _ dangle at the front of these class variables are an old convention for trying to signify that something is private and shouldn't be used by external users of the class. The MDN article you linked to describes a feature that was added in ES2022, and three.js is sticking to ES2018 for now (see https://github.com/mrdoob/three.js/pull/24237, https://github.com/mrdoob/three.js/pull/25168, and https://github.com/mrdoob/three.js/pull/25172), so it can't use that feature and has to rely on the _ convention.

Additionally, the Three link I posted above only has this._x, this._y, and this._z, the x, y, and z are only methods, not class members.

To be clear, the "methods" are getters and setters and they are accessed as if it's just a normal class member (i.e., euler.x, etc.). See the MDN articles I linked to if you're not clear how that works.

I have provided some code above where the issue occurs, but let me know what in specific you need in addition and I can try to provide it. Thanks for all of the help

Do you have the code where the Euler is being stringified/parsed? It seems like maybe you have some unsound types that are letting you assign a stringified/parsed Euler to the Euler type when doing so is actually incorrect.

Is there a way for you to define your own Euler interface that is just a simple object that has x/y/z properties that you could then instantiate an Euler object from?

rafaelsc commented 1 year ago

I think they referenced this issue by mistake.

Yes. Sorry about that. Was my mistake.

Methuselah96 commented 1 year ago

Closing for now since I don't think this is an issue with the types, feel free to comment if you would like it to be reopened.