Instrument ObservableArray's lengthProperty

[jessegreenberg]

lengthProperty of ObservableArray needs to be instrumented.

[jessegreenberg]

I instrumented lengthProperty and the Property seems to behave well, but in the state wrapper (phetsims/forces-and-motion-basics#240) I noticed that in the state simulation myObservableArray.length !== myObservableArray.lengthProperty.get().

Im not sure why this is, @zepumph or @samreid would you be able to help take a look? It might be due to my instrumentation of lengthProperty or I was wondering if this was due to implementation of TObservableArray?

[samreid]

Having the lengthProperty save and restore its state independently of ObservableArray means there can now be points in the code where the lengths mismatch. I think lengthProperty should be treated conceptually as a derived property (derived from the ObservableArray object) and not something that saves and restores itself independently. @jessegreenberg can you talk more about why you instrumented lengthProperty and what problems it solved? Then we can get a greater understanding of how to proceed.

[jessegreenberg]

Sure, I instrumented lengthProperty for phetsims/forces-and-motion-basics#240.

forces-and-motion-basics adds a listener to lengthProperty to update images. Since lengthProperty was not instrumented, this listener was never being called in the state wrapper and the state sim was getting out of sync.

[samreid]

I noticed this code in TObservableArray:

        setValue: function( instance, value){
          // TODO: is this is a no no? Does PhET-iO have this sort of power, see https://github.com/phetsims/phet-io/issues/1054
          instance._array = value;
        },

@jessegreenberg can you please change this to:

        setValue: function( instance, value ) {
          instance.clear();
          instance.addAll( value );
        },

It should update the lengthProperty correctly. After making this change, please uninstrument lengthProperty and check whether phetsims/forces-and-motion-basics#240 will work properly. The idea is that this new setValue will update the lengthProperty itself at the appropriate time instead of at separate times like was happening when lengthProperty was instrumented separately.

[jessegreenberg]

It's working great @samreid, thanks! I replaced implementation in setValaue and removed instrumentation in lengthProperty. I think this issue can be closed.