From 61020957912cd213a65b4ae98729fd457ed5106b Mon Sep 17 00:00:00 2001 From: zepumph Date: Wed, 17 Mar 2021 15:54:50 -0800 Subject: [PATCH] convert instrumented pickableProperty to inputEnabledProperty, https://github.com/phetsims/scenery/issues/1158 --- js/nodes/Node.js | 101 ++++++++++++------------------------------ js/nodes/NodeAPI.js | 11 +++-- js/nodes/NodeTests.js | 12 +++-- 3 files changed, 39 insertions(+), 85 deletions(-) diff --git a/js/nodes/Node.js b/js/nodes/Node.js index 5c31367b5..bb1dd2355 100644 --- a/js/nodes/Node.js +++ b/js/nodes/Node.js @@ -170,7 +170,6 @@ import PhetioObject from '../../../tandem/js/PhetioObject.js'; import Tandem from '../../../tandem/js/Tandem.js'; import BooleanIO from '../../../tandem/js/types/BooleanIO.js'; import IOType from '../../../tandem/js/types/IOType.js'; -import NullableIO from '../../../tandem/js/types/NullableIO.js'; import ParallelDOM from '../accessibility/pdom/ParallelDOM.js'; import Instance from '../display/Instance.js'; import Renderer from '../display/Renderer.js'; @@ -188,7 +187,6 @@ let globalIdCounter = 1; const scratchBounds2 = Bounds2.NOTHING.copy(); // mutable {Bounds2} used temporarily in methods const scratchMatrix3 = new Matrix3(); -const PICKABLE_PROPERTY_TANDEM_NAME = 'pickableProperty'; const ENABLED_PROPERTY_TANDEM_NAME = EnabledProperty.TANDEM_NAME; const VISIBLE_PROPERTY_TANDEM_NAME = 'visibleProperty'; const INPUT_ENABLED_PROPERTY_TANDEM_NAME = 'inputEnabledProperty'; @@ -201,7 +199,6 @@ const NODE_OPTION_KEYS = [ 'visibleProperty', // {Property.|null} - Sets forwarding of the visibleProperty, see setVisibleProperty() for more documentation 'visible', // {boolean} - Whether the Node is visible, see setVisible() for more documentation - 'pickablePropertyPhetioInstrumented', // {boolean} - When true, create an instrumented pickableProperty when this Node is instrumented, see setPickablePropertyPhetioInstrumented() for more documentation 'pickableProperty', // {Property.|null} - Sets forwarding of the pickableProperty, see setPickableProperty() for more documentation 'pickable', // {boolean|null} - Whether the Node is pickable, see setPickable() for more documentation @@ -258,7 +255,6 @@ const DEFAULT_OPTIONS = { visible: true, opacity: 1, pickable: null, - pickablePropertyPhetioInstrumented: false, enabled: true, enabledPropertyPhetioInstrumented: false, inputEnabled: true, @@ -348,7 +344,7 @@ class Node extends PhetioObject { // @private {TinyForwardingProperty.} - See setPickable() and setPickableProperty() // NOTE: This is fired synchronously when the pickability of the Node is toggled this._pickableProperty = new TinyForwardingProperty( DEFAULT_OPTIONS.pickable, - DEFAULT_OPTIONS.pickablePropertyPhetioInstrumented, this.onPickablePropertyChange.bind( this ) ); + false, this.onPickablePropertyChange.bind( this ) ); // @public {TinyForwardingProperty.} - See setEnabled() and setEnabledProperty() this._enabledProperty = new TinyForwardingProperty( DEFAULT_OPTIONS.enabled, @@ -4003,7 +3999,7 @@ class Node extends PhetioObject { * @returns {Node} for chaining */ setPickableProperty( newTarget ) { - return this._pickableProperty.setTargetProperty( this, PICKABLE_PROPERTY_TANDEM_NAME, newTarget ); + return this._pickableProperty.setTargetProperty( this, null, newTarget ); } /** @@ -4016,29 +4012,6 @@ class Node extends PhetioObject { this.setPickableProperty( property ); } - /** - * Handles linking and checking child PhET-iO Properties such as visibleProperty and pickableProperty. - * @public - * - * @param {string} tandemName - the name for the child tandem - * @param {Property.|undefined|null} oldProperty - same typedef as TinyForwardingProperty.targetProperty - * @param {Property.|undefined|null} newProperty - same typedef as TinyForwardingProperty.targetProperty - */ - updateLinkedElementForProperty( tandemName, oldProperty, newProperty ) { - assert && assert( oldProperty !== newProperty, 'should not be called on same values' ); - - // Only update linked elements if this Node is instrumented for PhET-iO - if ( this.isPhetioInstrumented() ) { - - oldProperty && oldProperty.isPhetioInstrumented() && this.removeLinkedElements( oldProperty ); - - const tandem = this.tandem.createTandem( tandemName ); - if ( newProperty && newProperty.isPhetioInstrumented() && tandem !== newProperty.tandem ) { - this.addLinkedElement( newProperty, { tandem: tandem } ); - } - } - } - /** * Get this Node's pickableProperty. Note! This is not the reciprocal of setPickableProperty. Node.prototype._pickableProperty * is a TinyForwardingProperty, and is set up to listen to changes from the pickableProperty provided by @@ -4067,28 +4040,6 @@ class Node extends PhetioObject { return this.getPickableProperty(); } - /** - * Use this to automatically create a forwarded, PhET-iO instrumented pickableProperty internal to Node. This is different - * from visible because pickable by default doesn't not create this forwarded Property. - * @public - * - * @param {boolean} pickablePropertyPhetioInstrumented - * @returns {Node} - for chaining - */ - setPickablePropertyPhetioInstrumented( pickablePropertyPhetioInstrumented ) { - return this._pickableProperty.setTargetPropertyInstrumented( pickablePropertyPhetioInstrumented, this ); - } - - /** - * See setPickablePropertyPhetioInstrumented() for more information - * @public - * - * @param {boolean} value - */ - set pickablePropertyPhetioInstrumented( value ) { - this.setPickablePropertyPhetioInstrumented( value ); - } - /** * Sets whether this Node (and its subtree) will allow hit-testing (and thus user interaction), controlling what * Trail is returned from node.trailUnderPoint(). @@ -4173,6 +4124,30 @@ class Node extends PhetioObject { // TODO: invalidate the cursor somehow? #150 } + + /** + * Handles linking and checking child PhET-iO Properties such as visibleProperty and enabledProperty. + * @public + * + * @param {string} tandemName - the name for the child tandem + * @param {Property.|undefined|null} oldProperty - same typedef as TinyForwardingProperty.targetProperty + * @param {Property.|undefined|null} newProperty - same typedef as TinyForwardingProperty.targetProperty + */ + updateLinkedElementForProperty( tandemName, oldProperty, newProperty ) { + assert && assert( oldProperty !== newProperty, 'should not be called on same values' ); + + // Only update linked elements if this Node is instrumented for PhET-iO + if ( this.isPhetioInstrumented() ) { + + oldProperty && oldProperty.isPhetioInstrumented() && this.removeLinkedElements( oldProperty ); + + const tandem = this.tandem.createTandem( tandemName ); + if ( newProperty && newProperty.isPhetioInstrumented() && tandem !== newProperty.tandem ) { + this.addLinkedElement( newProperty, { tandem: tandem } ); + } + } + } + /** * Sets what Property our enabledProperty is backed by, so that changes to this provided Property will change this * Node's enabled, and vice versa. This does not change this._enabledProperty. See TinyForwardingProperty.setTargetProperty() @@ -6618,11 +6593,12 @@ class Node extends PhetioObject { // the default, instrumented visibleProperty is conditionally created. We don't want to store these on the Node, // and thus they aren't support through `mutate()`. visiblePropertyOptions: null, - pickablePropertyOptions: null, enabledPropertyOptions: null, inputEnabledPropertyOptions: null }, config ); + assert && assert( !config.hasOwnProperty( 'pickablePropertyOptions' ), 'DEBUG, remove me!!!!' ); + // Track this, so we only override our visibleProperty once. const wasInstrumented = this.isPhetioInstrumented(); @@ -6643,24 +6619,6 @@ class Node extends PhetioObject { }, config.visiblePropertyOptions ) ) ); - this._pickableProperty.initializePhetio( this, PICKABLE_PROPERTY_TANDEM_NAME, () => new Property( this.pickable, merge( { - - // by default, use the value from the Node - phetioReadOnly: this.phetioReadOnly, - tandem: this.tandem.createTandem( PICKABLE_PROPERTY_TANDEM_NAME ), - phetioType: Property.PropertyIO( NullableIO( BooleanIO ) ), - phetioFeatured: true, // Since this property is opt-in, we typically only opt-in when it should be featured - phetioDocumentation: 'Sets whether the node will be pickable (and hence interactive). Pickable can take one ' + - 'of three values:
' + - '
    ' + - '
  • null: pass-through behavior. Nodes with input listeners are pickable, but nodes without input listeners won\'t block events for nodes behind it.
    • ' + - '
  • false: The node cannot be interacted with, and it blocks events for nodes behind it.
    • ' + - '
  • true: The node can be interacted with (if it has an input listener).
    • ' + - '
' + - 'For more about Scenery node pickability, please see http://phetsims.github.io/scenery/doc/implementation-notes#pickability' - }, config.pickablePropertyOptions ) ) - ); - this._enabledProperty.initializePhetio( this, ENABLED_PROPERTY_TANDEM_NAME, () => new EnabledProperty( this.enabled, merge( { // by default, use the value from the Node @@ -6681,9 +6639,8 @@ class Node extends PhetioObject { phetioDocumentation: 'Sets whether the node will have input enabled for (and hence interactive). This is a ' + 'subset of enabledProperty, which controls if input is enabled as well as changing style ' + 'etc.' - }, config.pickablePropertyOptions ) ) + }, config.inputEnabledPropertyOptions ) ) ); - } } diff --git a/js/nodes/NodeAPI.js b/js/nodes/NodeAPI.js index 64311a77a..37abcdf56 100644 --- a/js/nodes/NodeAPI.js +++ b/js/nodes/NodeAPI.js @@ -12,7 +12,6 @@ import PropertyAPI from '../../../axon/js/PropertyAPI.js'; import merge from '../../../phet-core/js/merge.js'; import PhetioObjectAPI from '../../../tandem/js/PhetioObjectAPI.js'; import BooleanIO from '../../../tandem/js/types/BooleanIO.js'; -import NullableIO from '../../../tandem/js/types/NullableIO.js'; import scenery from '../scenery.js'; import Node from './Node.js'; @@ -30,9 +29,9 @@ class NodeAPI extends PhetioObjectAPI { phetioType: Property.PropertyIO( BooleanIO ) }, - pickablePropertyPhetioInstrumented: false, - pickablePropertyOptions: { - phetioType: Property.PropertyIO( NullableIO( BooleanIO ) ), + inputEnabledPropertyPhetioInstrumented: false, + inputEnabledPropertyOptions: { + phetioType: Property.PropertyIO( BooleanIO ), phetioFeatured: true }, @@ -47,10 +46,10 @@ class NodeAPI extends PhetioObjectAPI { // @public (read-only) this.visibleProperty = new PropertyAPI( options.visiblePropertyOptions ); - if ( options.pickablePropertyPhetioInstrumented ) { + if ( options.inputEnabledPropertyPhetioInstrumented ) { // @public (read-only) - this.pickableProperty = new PropertyAPI( options.pickablePropertyOptions ); + this.inputEnabledProperty = new PropertyAPI( options.inputEnabledPropertyOptions ); } if ( options.enabledPropertyPhetioInstrumented ) { diff --git a/js/nodes/NodeTests.js b/js/nodes/NodeTests.js index a2c1de62f..cca7a3f72 100644 --- a/js/nodes/NodeTests.js +++ b/js/nodes/NodeTests.js @@ -162,9 +162,7 @@ QUnit.test( 'Trail and Node transform equivalence', assert => { if ( Tandem.PHET_IO_ENABLED ) { QUnit.test( 'Node instrumented visibleProperty', assert => testInstrumentedNodeProperty( assert, 'visible', 'visibleProperty', 'setVisibleProperty', true ) ); - - QUnit.test( 'Node instrumented pickableProperty', assert => testInstrumentedNodeProperty( assert, 'pickable', 'pickableProperty', 'setPickableProperty', Node.DEFAULT_OPTIONS.pickablePropertyPhetioInstrumented ) ); - + QUnit.test( 'Node instrumented enabledProperty', assert => testInstrumentedNodeProperty( assert, 'enabled', 'enabledProperty', 'setEnabledProperty', Node.DEFAULT_OPTIONS.enabledPropertyPhetioInstrumented ) ); QUnit.test( 'Node instrumented inputEnabledProperty', assert => testInstrumentedNodeProperty( assert, 'inputEnabled', 'inputEnabledProperty', 'setInputEnabledProperty', Node.DEFAULT_OPTIONS.inputEnabledPropertyPhetioInstrumented ) ); @@ -421,17 +419,17 @@ if ( Tandem.PHET_IO_ENABLED ) { QUnit.test( 'PhET-iO API Validation', assert => { phetioAPITest( assert, new NodeAPI( { enabledPropertyPhetioInstrumented: true, - pickablePropertyPhetioInstrumented: true + inputEnabledPropertyPhetioInstrumented: true } ), 'node', tandem => new Node( { enabledPropertyPhetioInstrumented: true, - pickablePropertyPhetioInstrumented: true, + inputEnabledPropertyPhetioInstrumented: true, tandem: tandem } ) ); phetioAPITest( assert, new NodeAPI( { - pickablePropertyPhetioInstrumented: true + inputEnabledPropertyPhetioInstrumented: true } ), 'node', tandem => new Node( { - pickablePropertyPhetioInstrumented: true, + inputEnabledPropertyPhetioInstrumented: true, tandem: tandem } ) );