Cannot animate Node opacityProperty

[pixelzoom]

In Fourier's Wave Game screen, I am trying to do something that is quite common in PhET games, and I would suspect is quite common in many animation situations. I'm trying to animate a Node's opacity. When a challenge is correctly answered, I want to show a smily face (FaceNode), then fade it out by changing its opacity. I'm using Animation to do the fade.

In previous sims, I've created my own faceOpacityProperty and used it to animate the opacity of a FaceNode instance, and it was worked nicely. Something like this:

const faceNode = new FaceNode(...);
const faceOpacityProperty = new NumberProperty( ... );
faceOpacityProperty.link( opacity => {
  faceNode.opacity = opacity;
} );
const faceAnimation = new Animation( {
  targets: [ {
    property: faceOpacityProperty,
    ..
  } ]
  ...
} );

But now that opacityProperty is standard equipment for every Node, I thought I should be able to skip the intermedite Property, and simply do this:

const faceNode = new FaceNode(...);
const faceAnimation = new Animation( {
  targets: [ {
    property: faceNode.opacityProperty,
    ..
  } ]
  ...
} );

This fails with this assertion:

assert.js:25 Uncaught Error: Assertion failed at window.assertions.assertFunction (assert.js:25) at new AnimationTarget (AnimationTarget.js:145) at Animation.js:138 at arrayMap (lodash-4.17.4.js:660) at Function.map (lodash-4.17.4.js:9571) at new Animation (Animation.js:137) at WaveGameLevelNode.js:267 at TinyEmitter.emit (TinyEmitter.js:86) at NumberProperty._notifyListeners (Property.js:294) at NumberProperty.set (Property.js:191)

First, an Error message would be nice here.

Second, here's the assertion that's failing in AnimationTarget:

assert && assert( config.property === null || config.property instanceof Property );

Node opacityProperty is an instance of TinyProperty, which is not a subclass of Property. (Yet another weirdness in the Property class hierarchy.)

Until this is addressed, I'll continue to an intermediate {Property} faceOpacityProperty.

[jonathanolson]

@pixelzoom thoughts on explicitly including TinyProperty as allowed until we get the hierarchy set up nicely?

[pixelzoom]

I'm fine with explicitly including TinyProperty, as long as it supports everything needed by AnimationTarget. I'm not familair with what parts of the Property API are required.

[jonathanolson]

I checked over, it's just get/set and TinyProperty should be fully compatible. Implemented above, feel free to take it for a spin.

[pixelzoom]

This assertion in AnimationTarget is not sufficient:

assert && assert( config.property === null || config.property instanceof Property || config.property instanceof TinyProperty );

You also need to verify that config.property.isSettable(). If I pass in a DerivedProperty, the assertion will pass, and it's going to fail in PROPERTY_SET.

[jonathanolson]

Done above.

[pixelzoom]

Thanks, I'll take it for a spin in Fourier.

[pixelzoom]

Works great in Fourier, thanks, closing.