Single property schema array

[diarmidmackenzie]

Description:

Small change to fix #5242 and allow an array to be used as the property in a single property schema.
Unit tests for this.

Changes proposed:

I don't have much confidence in this fix, so needs review by someone who understands the code better. It may be that an altogether different approach is needed.

Following up on my previous analysis here, I experimented with various changes.

What seemed to work best was skipping the object pool altogether for this array property, by updating this test:

if ((value instanceof Object) && !(value instanceof window.HTMLElement))

to this:

if ((value instanceof Object) &&
        !(value instanceof Array) &&
        !(value instanceof window.HTMLElement))

I tried this simple option:

if (this.isObjectBased && !(value instanceof window.HTMLElement))

but that broke some cursor UTs, presumably because of they use Vector3s, for which it's also the case that instanceof Object is true, but isObject is false... I guess we don't want to make any changes for Vector3s, just arrays...?

A build including this fix does resolve the problem.
https://glitch.com/edit/#!/single-property-schema-array-fix?path=index.html%3A8%3A23

There's still lots I don't understand about the code I have modified, exactly when it is invoked, and what it should do.

Things I don't understand:

• Why does the problem we have with Arrays not also happen for Vector3s? Components like position and rotation use a single property schema, with a Vector3, and they clearly work fine.
• Why did we only have a problem on entity creation, and not on subsequent setAttribute() calls?

Regarding the new UTs...

I have reasonable confidence that the 2 x new UTs are valid, but not sure they belong in the component suite: although they are testing code in src/core/component they are more like end-to-end tests, rather than unit tests of this module.

Maybe there's a better place for these?

[dmarcos]

This needs rebase. I haven't stumbled much in the wild on arrays for single property schemas.

@vincentfretin @mrxz thoughts?

[vincentfretin]

It seems you can close that PR. The issue was fixed in #5500 that included those tests.

[dmarcos]

Fixed by #5500