Remove prototype functions

[pjcozzi]

As discussed on the forum.

Let's start with the Cartesian, Quaternion, and Matrix types in Core. Then, we are likely to move beyond them to types like BoundingSphere and BoundingRectangle.

[cmorse]

@pjcozzi So if I am understanding this correctly, to fix add in Cartesian2, I would remove Cartesian2.prototype.add completely, and modify the existing specs so that they use Cartesian2.add directly?

[pjcozzi]

@cmorse correct, but more than the specs will need to change for many functions.

[cmorse]

Are you guys aware that this change would break chaining? For example, in the CatmullRomSpline constructor:

   this._ti = controlPoint1
                          .multiplyByScalar(2.0)
                          .subtract(controlPoint2)
                          .subtract(controlPoint0)
                          .multiplyByScalar(0.5);

would become something like:

   this._ti = Cartesian3.multiplyByScalar(
                       Cartesian3.subtract(
                            Cartesian3.subtract(
                                 Cartesian3.multiplyByScalar(controlPoint1, 2.0),
                            controlPoint2),
                       controlPoint0),
                  0.5);

I don't know anything about the web worker stuff, but if they lose the prototypes, it probably would be better to go this route in the end.

[mramato]

In our own code, I'm pretty sure we don't really want chaining (even if some places currently have it). However, that is a good point in terms of user code convenience. Assuming we are going ahead with the change, I would break these out into multiple lines.

[cmorse]

Sounds good. How would you break them out into multiple lines? With a temporary variable, or like I did above?

[pjcozzi]

@cmorse yes good point, but we are OK with it. I also agree with @mramato that we would want to separate these out - and eventually use result parameters.

[mramato]

I just realized that there are places in the code that we rely on the ability to clone an object without knowing what that object is, for instance, ConstantProperty. In order to remove prototype clone that means that everywhere which creates a ConstantProperty or TimeIntervalProperty must pass in the type of object (like we do with SampledProperty). I'm not sure if there are other places where we rely on prototype functions in this manner. @shunter can you think of any?

Things like this are why I'm hesitant to pull any fixes for this issue into master until the entire conversion is complete. There are too many issues that could crop up and dong it piecemeal might create additional problems if larger areas of the code need to be reworked.

[cmorse]

@mramato That's why I didn't remove the clone prototype function in my pull request. I saw what SampledProperty was doing with it, and decided it was better to leave it in. At least for now.

[mramato]

Awesome, I missed that. Thanks. We'll figure out in the future if we should keep it or not.

[mramato]

In order to cleanly implement some things in DynamicScene, I need to put back prototype equals functionality. @shunter recommends that we keep clone, equals, and equalsEpsilon because there usage often happens in cases where you don't know the type (as is the case in DynamicScene).

[pjcozzi]

OK with me.

[pjcozzi]

We've made excellent progress here. Thanks @cmorse!

Before closing this, here's the other types I think we should address:

• AxisAlignedBoundingBox
• BoundingRectangle
• BoundingSphere
• Plane
• Ray
• Spherical
• Remove 'static ... from spec descriptions where it is no longer true.

These will be much easier than the types we already changed.

[mramato]

JulianDate needs other major changes and (and I'm not sure we can go to a non-prototype version due to it's nature). However I won't know for sure until I look into what exactly the caveats will be. I'll probably handle that refactor myself (in fact I already have some very minor changes in the its-about-time branch.

[pjcozzi]

Alright. Removed JulianDate from the above list.

[pjcozzi]

Added above

• Remove 'static ... from spec descriptions where it is no longer true.

[pjcozzi]

This was a big effort. Thanks @cmorse and @hpinkos!