Three.js: Obj.set should default to constructor defaults

[drcmda]

Description of the problem

Consider this:

const a = new Vector3(4,5) // -> 4,5,0
a.set(1,2)
console.log(a) // -> 1,2,undefined

This is how the constructor is defined:

function Vector3(x, y, z) {
  this.x = x || 0;
  this.y = y || 0;
  this.z = z || 0;
}
````

And this is the set function:

```jsx
function set(x, y, z) {
  this.x = x;
  this.y = y;
  this.z = z;
  return this;
}

It would be helpful and safer to assume constructor defaults always, for all objects that carry a set. Otherwise it's throwing undefined into the renderer.

Here's an example for instance that causes undefined behaviour where it works on some platforms but crashes on others: https://discourse.threejs.org/t/need-some-help-with-a-texture-that-doesnt-work-on-mobile/10553/2

[Mugen87]

So you mean set() should look like so:

set: function ( x, y, z ) {

   this.x = x || 0;
   this.y = y || 0;
   this.z = z || 0;

   return this;

},

TBH, I don't think this is a good suggestion. We basically would allow this:

vector.set();

which would set all vector components to zero.

Having default parameters in the constructor does not mean we also need them elsewhere. In general, constructors should allow to create objects in an initial state. The purpose of a set() method is to assign the given arguments to the respective properties. So the semantics are slightly different.

When using Vector3.set(), all parameters are mandatory. If you don't provide all of them, the behavior is undefined.

[drcmda]

Yes, i think it should be the same behaviour as the constructor. If i can do new Vector3(), why shouldn't i be able to do set()? I see no reason why it should put an "undefined" into the vector. undefined seems to work as "zero" in some systems, while on others it fails. You could still document it as mandatory, and force typescript to adhere to it, but overall it would be safer.

another suggestion would be to fallback to this, you're doing this with euler for instance: https://github.com/mrdoob/three.js/blob/master/src/math/Euler.js#L109

so:

set: function ( x, y, z ) {

   this.x = x === void 0 ? x : this.x;
   this.y = y === void 0 ? y : this.y;
   this.z = z === void 0 ? z : this.z;

   return this;

},

would prefer 0 though, it doesn't cause much fuss and just maintains system stability as a safeguard.

[Mugen87]

We recently discussed set() methods here https://github.com/mrdoob/three.js/pull/15043#issuecomment-539759212 and here https://github.com/mrdoob/three.js/pull/17792#issuecomment-545683497 and the consensus was to not change the current behavior.

[Mugen87]

I think your problem at the forum can also be fixed at react-three-fiber level.

[drcmda]

it doesn't know what a vector3 is unfortunately. it just knows there's a set method. but either way, i didn't know there was already a discussion about this, nothing immediate came up in the github search for it.

[Mugen87]

No problem, this discussion is really hard to find^^.

@mrdoob To recap: The idea is to make the parameters of all .set() methods optional. If no arguments are provided, the current property value does not change (rather than assigning undefined or set a ctor default value). One could argue that we already do this in Euler.set() but we all know that Euler._order is somewhat special.

[mrdoob]

Hmm, not sure...

If someone wants to set the x and the z, they'll have to do...

vector.set( x, undefined, z );

I'm undecided.

[drcmda]

wouldn't they write vector.set(x, 0, z)? the undefined in general is what's causing problems, it behaves differently in windows, linux and mac. i was proposing constructor assumption just as a fallback, so that it never ends up with an undefined value, since later it will try to make calculations based on that. so in essence, if you do new Vector3(x, undefined, z) it'll be [x, 0, z] effectively b/c the constructor watches out for faulty input.

[Mugen87]

Like I said before, this proposal might be helpful in your case but will cause confusion or bad code style like mentioned here: https://github.com/mrdoob/three.js/issues/17845#issuecomment-548725270

It seems there are pro and cons for all approaches but no one is good enough to change the status quo.

[drcmda]

how does safeguarding against faulty values create confusion or bad code though, could you explain what you mean by this? typescript expects three numbers, documentation remains as it is, yet when someone does set(x,y) anyway, what is the benefit if putting a value into threejs that destroys the renderer on some platforms while it works on others?

[Mugen87]

_Like I said before:_ You could do this and it would be valid code.

vector.set();

[drcmda]

well, not in typescript (i would assume). but either way, you can do this today as well. and it's valid code (but sometimes kills the renderer due to undefined being treated as a number without checks).