Three.js: API Design: Object3D.position is read-only

Created on 20 May 2016  路  11Comments  路  Source: mrdoob/three.js

Description of the problem

The position property of Object3D, and more importantly all it's ancestors, is read-only. The default behavior of Object.defineProperties sets properties that way -- that's what's used in Object3D.js.

Consider the scenario where a user wants to subclass Mesh.js.

var Particle = function (positionVector, mass, radius) {

    THREE.Mesh.call(this);

    // will cause read-only assignment error
    this.position = positionVector; 
   ...
    this.geometry = new THREE.SphereGeometry(this.radius, 10, 10);
    this.material = this.getMaterial({
        color: 0xff0000
    });
}

Particle.prototype = Object.create(THREE.Mesh.prototype);
Particle.prototype.constructor = Particle;

Copying or clone methods of Vector3 are problematic because there's no way to preserve the original reference passed (or 'injected' if you want to be tedious) in the constructor above. This prevents elegant subclassing for child classes of Object3D like Mesh, and I think is a mistake in an otherwise very-well designed library. Please consider altering the relevant Object.defineProperties line to add writable: true.

Three.js version
  • [ ] Dev
  • [x] r76
  • [ ] ...

    Browser
  • [x] All of them

  • [ ] Chrome
  • [ ] Firefox
  • [ ] Internet Explorer

    OS
  • [x] All of them

  • [ ] Windows
  • [ ] Linux
  • [ ] Android
  • [ ] IOS
    Hardware Requirements (graphics card, VR Device, ...)

N/A

Most helpful comment

@mrdoob

Fair enough: protecting the internals from users is a good motivation. And it's not difficult to keep a separate Vector3 to use for operations and then assign its values to position using the Vector3 methods. @aardgoose 's suggestion to document the read-only would be helpful.

My compliments on an extraordinary and elegant library.

All 11 comments

We had to do it this way because the pattern you are talking about confused people. Then they would ask for help and it would be hard for us to try to follow the flow of their code.

I think it would be useful to document the nature of these "read only" properties to avoid confusion. It would probably avoid some queries, although I think the term "read only" isn't very helpful, maybe "un -assignable" since the property value can be changed by its methods.

I found it confusing that I couldn't assign a position simply at first, and it shouldn't be necessary to dig through source code to find out what is happening.

I'm happy to give this a go, if you want.

@mrdoob This isn't a pattern. It's straightforward inheritance in JavaScript; the majority of three.js elegantly relies on it. Granted, many JavaScript developers are confused by basic OO, but I don't see that as a reason to hobble a great API. If JavaScript allowed the prevention of subclassing at the Class level (like final in Java) I could understand it but, as it stands, the read-only properties just make more experienced developers scratch their heads and go digging in the code to explain an odd restriction. That JS developers were confused isn't the least bit surprising, but it doesn't mean ignorance should be accommodated.

@arctwelve

The problem is... You start with position and you quickly end up doing the same with quaternion, matrix, matrixWorld, etc

If you re-assign quaternion we can't keep rotation in sync anymore. If you re-assign matrix, well... lots of horrible things can happen.

@mrdoob

Fair enough: protecting the internals from users is a good motivation. And it's not difficult to keep a separate Vector3 to use for operations and then assign its values to position using the Vector3 methods. @aardgoose 's suggestion to document the read-only would be helpful.

My compliments on an extraordinary and elegant library.

Closed: intentional design of API

@WestLangley @aardgoose The property in the actual Object.defineProperties code is writable so IMHO 'read-only' is straightforward and clear.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperties

writable: true if and only if the value associated with the property may be changed with an assignment operator. Defaults to false

Immutable/Mutable is more complicated. The position property is mutable. It's internal values like x, y and z can be changed. It just can't be directly assigned in Object3D ancestors.

quite, which is where we lack decent terminology: RORTMO (read only reference to mutable object).

Though being fair the defineProperties writable: false and configurable: false defaults is immutable ( you can't remove the reference property or change it to writable, which is equivalent to immutable files in ext2(+) file systems, so maybe IRTMO is what we want.

Change it to get/set, track the new values, and then you'd avoid the problems you mentioned @mrdoob EDIT: I know, maintenance cost.

@trusktr uh?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Horray picture Horray  路  3Comments

ghost picture ghost  路  3Comments

Bandit picture Bandit  路  3Comments

makc picture makc  路  3Comments

filharvey picture filharvey  路  3Comments