PhET-iO: How to support dynamic range for ISLCObject.positionProperty #172
Assignees
Comments
|
From #172 |
|
Is this primarily a problem because the studio controls let you go out of the enabled range? Should the |
Yes studio sets the controls for the initial state, but even by the time startup is complete the range has changed. So the studio control is out of sync with the range. Do you think that we could fix this by adding support for |
|
At the very least we should set |
|
Discussed with @chrisklus @samreid and @zepumph. We decided that we would add support for Property for range in We also decided that in this sim, |
|
I was able to implement everything discussed above, and it seemed like things were going very well. I then proceeded to run into tremendous trouble about validating the dynamic range within the ISLC model. Basically I ran into more than 3 places (and counting) where the dynamic range was being updated such that it excluded the current position, or the position was trying to be set outside of the current range. My best guess is that this has to do with the order in which positionProperty and its range is being set. Included in this patch are a few fixes (or at least I think) to this problem. All in all this has turned out to be a bit of a headache. I was able to do the numberProperty stuff in about an hour, and have been trying to sort out ISLC specific buggy logic for 90+ minutes. Index: phet-io-wrappers/studio/StudioInstanceCreator.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- phet-io-wrappers/studio/StudioInstanceCreator.js (revision 6d48cabead79a1e232f757c0203cc31ae5a229b3)
+++ phet-io-wrappers/studio/StudioInstanceCreator.js (date 1566435721578)
@@ -537,35 +537,62 @@
const input = document.createElement( 'input' );
input.type = 'range';
input.name = 'points';
+
+ // These defaults are overwritten if more range data is specified.
let min = 0;
let max = 1;
- if ( stateObject.range ) {
- min = stateObject.range.min;
- const unitsString = stateObject.units || '';
- div.appendChild( createSpan( 'min: ' + min + ' ' + unitsString, { className: 'displayBlock' } ) );
+ // create these eagerly so they can be referenced in `updateRange`
+ const minSpan = createSpan( '', { className: 'displayBlock' } );
+ const maxSpan = createSpan( '', { className: 'displayBlock' } );
+ const unitsString = stateObject.units || '';
- max = stateObject.range.max;
- div.appendChild( createSpan( 'max: ' + max + ' ' + unitsString, { className: 'displayBlock' } ) );
- }
+ // Update the range pieces of the div, to support dynamic range, see https://github.com/phetsims/gravity-force-lab/issues/172
+ const updateRangeAndStep = () => {
+
+ // set the range text
+ minSpan.innerHTML = 'min: ' + min + ' ' + unitsString;
+ maxSpan.innerHTML = 'max: ' + max + ' ' + unitsString;
- // Edge cases for infinity
- if ( typeof min === 'string' && min.indexOf( 'INFINITY' ) >= 0 ) {
- min = -100;
- }
- if ( typeof max === 'string' && max.indexOf( 'INFINITY' ) >= 0 ) {
- max = 100;
- }
- input.min = min + '';
- input.max = max + '';
+ // Edge cases for infinity
+ if ( typeof min === 'string' && min.indexOf( 'INFINITY' ) >= 0 ) {
+ min = -100;
+ }
+ if ( typeof max === 'string' && max.indexOf( 'INFINITY' ) >= 0 ) {
+ max = 100;
+ }
+
+ input.min = min + '';
+ input.max = max + '';
- // integral sliders should only try to set integer values
- if ( stateObject.numberType === 'Integer' ) {
- input.step = 1;
- }
- else {
- input.step = ( max - min ) / 100 + '';
+ // integral sliders should only try to set integer values. Recompute step each time range is updated
+ if ( stateObject.numberType === 'Integer' ) {
+ input.step = 1;
+ }
+ else {
+ input.step = ( max - min ) / 100 + '';
+ }
+ };
+
+ if ( stateObject.range ) {
+ min = stateObject.range.min;
+ max = stateObject.range.max;
+
+ div.appendChild( minSpan );
+ div.appendChild( maxSpan );
+
+ // The presence of `rangePhetioID means it is a dynamic range Property, so link to it
+ if ( stateObject.rangePhetioID ) {
+ phetioClient.invoke( stateObject.rangePhetioID, 'link', [ newRange => {
+ min = newRange.min;
+ max = newRange.max;
+ updateRangeAndStep();
+ } ] );
+ }
}
+
+ // support for static ranges
+ updateRangeAndStep();
let onlyHasOneValue = false;
if ( max === min ) {
Index: inverse-square-law-common/js/view/ISLCObjectNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- inverse-square-law-common/js/view/ISLCObjectNode.js (revision b0234acfc4b4852e41b38f752ab07208b0e41b8f)
+++ inverse-square-law-common/js/view/ISLCObjectNode.js (date 1566437801771)
@@ -209,10 +209,8 @@
// drag position relative to the pointer pointer start position and convert to model coordinates
let x = modelViewTransform.viewToModelX( this.globalToParentPoint( event.pointer.point ).x - clickOffset );
- // absolute drag bounds based on model
- // see method descriptions for details
- const xMax = model.getObjectMaxPosition( object );
- const xMin = model.getObjectMinPosition( object );
+ const xMax = object.positionProperty.range.value.max;
+ const xMin = object.positionProperty.range.value.min;
// apply limitations and update position
x = Math.max( Math.min( x, xMax ), xMin ); // limited value of x (by boundary) in model coordinates
@@ -282,7 +280,7 @@
// a11y - initialize the accessible slider, which makes this Node act like an accessible range input
this.initializeAccessibleSlider(
object.positionProperty,
- object.enabledRangeProperty,
+ object.positionProperty.range,
new BooleanProperty( true ), // always enabled
accessibleSliderOptions
);
Index: inverse-square-law-common/js/view/describers/PositionDescriber.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- inverse-square-law-common/js/view/describers/PositionDescriber.js (revision b0234acfc4b4852e41b38f752ab07208b0e41b8f)
+++ inverse-square-law-common/js/view/describers/PositionDescriber.js (date 1566432287162)
@@ -393,7 +393,7 @@
*/
objectAtTouchingMin( objectEnum ) {
const object = this.getObjectFromEnum( objectEnum );
- return object.positionProperty.get() === object.enabledRangeProperty.get().min;
+ return object.positionProperty.get() === object.positionProperty.range.min;
}
/**
@@ -405,7 +405,7 @@
*/
objectAtTouchingMax( objectEnum ) {
const object = this.getObjectFromEnum( objectEnum );
- return object.positionProperty.get() === object.enabledRangeProperty.get().max;
+ return object.positionProperty.get() === object.positionProperty.range.max;
}
/**
Index: inverse-square-law-common/js/model/ISLCObject.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- inverse-square-law-common/js/model/ISLCObject.js (revision b0234acfc4b4852e41b38f752ab07208b0e41b8f)
+++ inverse-square-law-common/js/model/ISLCObject.js (date 1566437831284)
@@ -9,6 +9,7 @@
'use strict';
// modules
+ const BooleanProperty = require( 'AXON/BooleanProperty' );
const DerivedProperty = require( 'AXON/DerivedProperty' );
const DerivedPropertyIO = require( 'AXON/DerivedPropertyIO' );
const Emitter = require( 'AXON/Emitter' );
@@ -44,11 +45,20 @@
valueUnits: 'kilograms'
}, options );
+ const enabledRange = new Range( options.leftObjectBoundary, options.rightObjectBoundary );
+
+ const positionPropertyTandem = tandem.createTandem( 'positionProperty' );
+
// @public
this.positionProperty = new NumberProperty( initialPosition, {
- tandem: tandem.createTandem( 'positionProperty' ),
+ tandem: positionPropertyTandem,
units: 'meters',
- range: new Range( options.leftObjectBoundary, options.rightObjectBoundary ),
+ range: new Property( enabledRange, {
+ tandem: positionPropertyTandem.createTandem( 'rangeProperty' ),
+ phetioDocumentation: 'The range for position of this object based on the radius and location of both objects',
+ phetioType: PropertyIO( RangeIO ),
+ phetioReadOnly: true
+ } ),
phetioDocumentation: 'The position of the object along the track, in meters.'
} );
@@ -73,17 +83,6 @@
}
);
- const enabledRange = new Range( options.leftObjectBoundary, options.rightObjectBoundary );
-
- // @public {Property.<Range>}- set by ISLCModel when the force changes, the range for position of the objects
- // based on their radius and location
- this.enabledRangeProperty = new Property( enabledRange, {
- tandem: tandem.createTandem( 'enabledRangeProperty' ),
- phetioDocumentation: 'The range for position of this object based on the radius and location of both objects',
- phetioType: PropertyIO( RangeIO ),
- phetioReadOnly: true
- } );
-
// @public (read-only) - Emitter that fires whenever the position changes as a result of an object's value changing.
// Emits with the objectEnum that caused the position change.
this.positionChangedFromSecondarySourceEmitter = new Emitter( { parameters: [ { valueType: ISLCObjectEnum } ] } );
@@ -99,9 +98,9 @@
// from a value change. See https://github.com/phetsims/gravity-force-lab-basics/issues/168
this.valueChangedSinceLastStep = false;
- // @public (read-only) - flag flipped when the constant radius boolean is toggled. This is used to check if a position change is
+ // @public (read-only) {Property.<boolean>} - flipped when the constant radius boolean is toggled. This is used to check if a position change is
// caused from the constant radius toggling. See https://github.com/phetsims/gravity-force-lab-basics/issues/168
- this.constantRadiusChangedSinceLastStep = false;
+ this.constantRadiusChangedSinceLastStepProperty = new BooleanProperty( false );
// @public
this.valueRange = valueRange;
@@ -120,7 +119,7 @@
// Link these flags to their associatedProperties
this.valueProperty.link( () => { this.valueChangedSinceLastStep = true; } );
- constantRadiusProperty.link( () => { this.constantRadiusChangedSinceLastStep = true; } );
+ constantRadiusProperty.link( () => { this.constantRadiusChangedSinceLastStepProperty.value = true; } );
}
inverseSquareLawCommon.register( 'ISLCObject', ISLCObject );
@@ -133,7 +132,7 @@
*/
onStepEnd: function() {
this.valueChangedSinceLastStep = false;
- this.constantRadiusChangedSinceLastStep = false;
+ this.constantRadiusChangedSinceLastStepProperty.value = false;
},
/**
Index: inverse-square-law-common/js/model/ISLCModel.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- inverse-square-law-common/js/model/ISLCModel.js (revision b0234acfc4b4852e41b38f752ab07208b0e41b8f)
+++ inverse-square-law-common/js/model/ISLCModel.js (date 1566438127061)
@@ -85,19 +85,14 @@
units: 'newtons',
phetioDocumentation: 'The force of one object on the other (in Newtons)'
} );
+ window.model = this;
- const updateRange = object => {
- const maxPosition = this.getObjectMaxPosition( object );
- const minPosition = this.getObjectMinPosition( object );
- object.enabledRangeProperty.set( new Range( minPosition, maxPosition ) );
- };
-
- // a11y - necessary to reset the enabledRangeProperty to prevent object overlap, disposal not necessary
+ // a11y - necessary to reset the positionProperty.range to prevent object overlap, disposal not necessary
// We need to update the available range for each object when the either's radius or position changes.
Property.multilink( [ object1.positionProperty, object2.positionProperty ], () => {
- updateRange( object1 );
- updateRange( object2 );
+ this.updateRange( object1 );
+ this.updateRange( object2 );
} );
// when sim is reset, we only reset the position properties of each object to their initial values
@@ -105,19 +100,11 @@
this.object1.radiusProperty.link( () => {
this.object1.radiusLastChanged = true;
this.object2.radiusLastChanged = false;
-
- // update range if radius changed with "constant radius" setting (which didn't trigger other model updates)
- updateRange( object1 );
- updateRange( object2 );
} );
this.object2.radiusProperty.link( () => {
this.object2.radiusLastChanged = true;
this.object1.radiusLastChanged = false;
-
- // update range if radius changed with "constant radius" setting (which didn't trigger other model updates)
- updateRange( object2 );
- updateRange( object1 );
} );
// wire up logic to update the state of the pushedObjectEnumProperty
@@ -160,6 +147,7 @@
return inherit( Object, ISLCModel, {
+
/**
* Step function makes sure objects don't go out of bounds and don't overlap each other at each time step.
*
@@ -245,11 +233,25 @@
}
}
}
+ // update range if radius changed with "constant radius" setting (which didn't trigger other model updates)
+ if ( this.object1.constantRadiusChangedSinceLastStepProperty.value ||
+ this.object2.constantRadiusChangedSinceLastStepProperty.value ) {
+ this.updateRange( this.object1 );
+ this.updateRange( this.object2 );
+ }
// broadcast a message that we have updated the model
this.stepEmitter.emit();
},
+ updateRange: function( object ) {
+ const maxPosition = this.getObjectMaxPosition( object );
+ const minPosition = this.getObjectMinPosition( object );
+
+ object.positionProperty.range.set( new Range( minPosition, maxPosition ) );
+
+ },
+
/**
* Helper function to for accessing and mapping force ranges in the inheriting sims' views
* @returns {number} - positive number, representing the magnitude of the force vector
Index: axon/js/NumberProperty.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- axon/js/NumberProperty.js (date 1566431411000)
+++ axon/js/NumberProperty.js (date 1566436303575)
@@ -32,12 +32,14 @@
options = _.extend( {
numberType: 'FloatingPoint', // {string} see VALID_NUMBER_TYPES
- // {Range|null} range
+ // {Range|Property.<Range>|null} range
range: null
}, options );
assert && assert( _.includes( VALID_NUMBER_TYPES, options.numberType ), 'invalid numberType: ' + options.numberType );
- assert && options.range && assert( options.range instanceof Range, 'options.range must be of type Range:' + options.range );
+ assert && options.range && assert( options.range instanceof Range ||
+ ( options.range instanceof Property && options.range.value instanceof Range ),
+ 'options.range must be of type Range or Property.<Range>:' + options.range );
// client cannot specify superclass options that are controlled by NumberProperty
assert && assert( !options.valueType, 'NumberProperty sets valueType' );
@@ -54,7 +56,7 @@
// @public (read-only) - used by PhET-iO in NumberPropertyIO as metadata passed to the wrapper.
this.numberType = options.numberType;
- // @public {Range|null} (read-only) - If defined, provides the range of possible values (inclusive)
+ // @public {Range|Property.<Range>|null} (read-only) - if defined, provides the range of possible values (inclusive)
this.range = options.range;
// @private {function|null} value validation that is specific to NumberProperty, null if assertions are disabled
@@ -64,7 +66,11 @@
options.numberType === 'Integer' && validate( value, VALID_INTEGER );
// validate for range
- options.range && validate( value, { isValidValue: v => options.range.contains( v ) } );
+ if ( options.range ) {
+ const currentRange = options.range instanceof Property ? options.range.value : options.range;
+ assert && assert( currentRange instanceof Range, `unexpected Range: ${currentRange}` );
+ validate( value, { isValidValue: v => currentRange.contains( v ) } );
+ }
} );
// verify that validValues meet other NumberProperty-specific validation criteria
@@ -72,10 +78,27 @@
options.validValues.forEach( this.assertNumberPropertyValidateValue );
}
+ // @private - keep track for disposal
+ this.rangeChangeListener = () => {
+ this.assertNumberPropertyValidateValue( this.value );
+ };
+ if ( options.range && options.range instanceof Property ) {
+ options.range.link( this.rangeChangeListener );
+ }
+
// validate initial value
this.assertNumberPropertyValidateValue && this.assertNumberPropertyValidateValue( value );
}
+ /**
+ * @public
+ * @override
+ */
+ dispose() {
+ this.range && this.range instanceof Property && this.range.unlink( this.rangeChangeListener );
+ super.dispose();
+ }
+
/**
* Performs value validation that is specific to NumberProperty.
* Then sets the value and notifies listeners.
Index: axon/js/NumberPropertyIO.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- axon/js/NumberPropertyIO.js (date 1566431411000)
+++ axon/js/NumberPropertyIO.js (date 1566433001346)
@@ -13,10 +13,14 @@
const axon = require( 'AXON/axon' );
const NumberIO = require( 'TANDEM/types/NumberIO' );
const phetioInherit = require( 'TANDEM/phetioInherit' );
+ const Property = require( 'AXON/Property' );
const PropertyIO = require( 'AXON/PropertyIO' );
const RangeIO = require( 'DOT/RangeIO' );
const validate = require( 'AXON/validate' );
+ // ifphetio
+ const phetioEngine = require( 'ifphetio!PHET_IO/phetioEngine' );
+
// constants
const PropertyIOImpl = PropertyIO( NumberIO );
@@ -60,7 +64,11 @@
}
if ( numberProperty.range ) {
- parentStateObject.range = RangeIO.toStateObject( numberProperty.range );
+ const range = numberProperty.range instanceof Property ? numberProperty.range.value : numberProperty.range;
+ parentStateObject.range = RangeIO.toStateObject( range );
+ }
+ if ( numberProperty.range instanceof Property && numberProperty.isPhetioInstrumented() ) {
+ parentStateObject.rangePhetioID = numberProperty.range.tandem.phetioID;
}
return parentStateObject;
},
@@ -76,7 +84,9 @@
fromParentStateObject.numberType = stateObject.numberType;
// Create Range instance if defined, otherwise preserve value of null or undefined.
- fromParentStateObject.range = stateObject.range ? RangeIO.fromStateObject( stateObject.range ) : stateObject.range;
+ fromParentStateObject.range = stateObject.rangePhetioID ? phetioEngine.getPhetioObject( stateObject.rangePhetioID ) :
+ stateObject.range ? RangeIO.fromStateObject( stateObject.range ) :
+ stateObject.range;
return fromParentStateObject;
},
Index: gravity-force-lab-basics/js/phet-io/gravity-force-lab-basics-phet-io-types.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- gravity-force-lab-basics/js/phet-io/gravity-force-lab-basics-phet-io-types.js (revision a261a5d5e5f660759c4f250ca68ed251300a09ae)
+++ gravity-force-lab-basics/js/phet-io/gravity-force-lab-basics-phet-io-types.js (date 1566435833475)
@@ -1065,7 +1065,7 @@
],
"methods": {
"addEventListener": {
- "documentation": "Adds a listener to the PhET-iO dataStream, which can be used to respond to events or for data analysis. Unlike Client.launchSim( {onEvent} ) which is called recursively for each child event, this is only called with top-level events.",
+ "documentation": "Adds a listener to the PhET-iO dataStream, which can be used to respond to events or for data analysis. Unlike Client.launchSim( {onEvent} ) which is called recursively for each parsed child event, this is only called with stringified top-level events.",
"parameterTypes": [
"FunctionIO.(ObjectIO)=>VoidIO"
],
Index: gravity-force-lab-basics/js/phet-io/gravity-force-lab-basics-phet-io-elements-baseline.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- gravity-force-lab-basics/js/phet-io/gravity-force-lab-basics-phet-io-elements-baseline.js (revision a261a5d5e5f660759c4f250ca68ed251300a09ae)
+++ gravity-force-lab-basics/js/phet-io/gravity-force-lab-basics-phet-io-elements-baseline.js (date 1566435833432)
@@ -913,29 +913,29 @@
"phetioStudioControl": true,
"phetioTypeName": "DerivedPropertyIO.<NumberIO>"
},
- "gravityForceLabBasics.gravityForceLabBasicsScreen.model.mass1.enabledRangeProperty": {
- "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
+ "gravityForceLabBasics.gravityForceLabBasicsScreen.model.mass1.positionProperty": {
+ "phetioDocumentation": "The position of the object along the track, in meters.",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": true,
+ "phetioReadOnly": false,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "PropertyIO.<RangeIO>"
+ "phetioTypeName": "NumberPropertyIO"
},
- "gravityForceLabBasics.gravityForceLabBasicsScreen.model.mass1.positionProperty": {
- "phetioDocumentation": "The position of the object along the track, in meters.",
+ "gravityForceLabBasics.gravityForceLabBasicsScreen.model.mass1.positionProperty.rangeProperty": {
+ "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": false,
+ "phetioReadOnly": true,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "NumberPropertyIO"
+ "phetioTypeName": "PropertyIO.<RangeIO>"
},
"gravityForceLabBasics.gravityForceLabBasicsScreen.model.mass1.radiusProperty": {
"phetioDocumentation": "The radius of the object",
@@ -961,29 +961,29 @@
"phetioStudioControl": true,
"phetioTypeName": "NumberPropertyIO"
},
- "gravityForceLabBasics.gravityForceLabBasicsScreen.model.mass2.enabledRangeProperty": {
- "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
+ "gravityForceLabBasics.gravityForceLabBasicsScreen.model.mass2.positionProperty": {
+ "phetioDocumentation": "The position of the object along the track, in meters.",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": true,
+ "phetioReadOnly": false,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "PropertyIO.<RangeIO>"
+ "phetioTypeName": "NumberPropertyIO"
},
- "gravityForceLabBasics.gravityForceLabBasicsScreen.model.mass2.positionProperty": {
- "phetioDocumentation": "The position of the object along the track, in meters.",
+ "gravityForceLabBasics.gravityForceLabBasicsScreen.model.mass2.positionProperty.rangeProperty": {
+ "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": false,
+ "phetioReadOnly": true,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "NumberPropertyIO"
+ "phetioTypeName": "PropertyIO.<RangeIO>"
},
"gravityForceLabBasics.gravityForceLabBasicsScreen.model.mass2.radiusProperty": {
"phetioDocumentation": "The radius of the object",
Index: coulombs-law/js/phet-io/coulombs-law-phet-io-elements-baseline.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- coulombs-law/js/phet-io/coulombs-law-phet-io-elements-baseline.js (revision 84a77e9531078ee6110679557d6544b8bf3163b2)
+++ coulombs-law/js/phet-io/coulombs-law-phet-io-elements-baseline.js (date 1566435824541)
@@ -37,29 +37,29 @@
"phetioStudioControl": true,
"phetioTypeName": "PropertyIO.<BooleanIO>"
},
- "coulombsLaw.atomicScreen.coulombsLawAtomicScreen.model.charge1.enabledRangeProperty": {
- "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
+ "coulombsLaw.atomicScreen.coulombsLawAtomicScreen.model.charge1.positionProperty": {
+ "phetioDocumentation": "The position of the object along the track, in meters.",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": true,
+ "phetioReadOnly": false,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "PropertyIO.<RangeIO>"
+ "phetioTypeName": "NumberPropertyIO"
},
- "coulombsLaw.atomicScreen.coulombsLawAtomicScreen.model.charge1.positionProperty": {
- "phetioDocumentation": "The position of the object along the track, in meters.",
+ "coulombsLaw.atomicScreen.coulombsLawAtomicScreen.model.charge1.positionProperty.rangeProperty": {
+ "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": false,
+ "phetioReadOnly": true,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "NumberPropertyIO"
+ "phetioTypeName": "PropertyIO.<RangeIO>"
},
"coulombsLaw.atomicScreen.coulombsLawAtomicScreen.model.charge1.radiusProperty": {
"phetioDocumentation": "The radius of the object",
@@ -109,29 +109,29 @@
"phetioStudioControl": true,
"phetioTypeName": "PropertyIO.<BooleanIO>"
},
- "coulombsLaw.atomicScreen.coulombsLawAtomicScreen.model.charge2.enabledRangeProperty": {
- "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
+ "coulombsLaw.atomicScreen.coulombsLawAtomicScreen.model.charge2.positionProperty": {
+ "phetioDocumentation": "The position of the object along the track, in meters.",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": true,
+ "phetioReadOnly": false,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "PropertyIO.<RangeIO>"
+ "phetioTypeName": "NumberPropertyIO"
},
- "coulombsLaw.atomicScreen.coulombsLawAtomicScreen.model.charge2.positionProperty": {
- "phetioDocumentation": "The position of the object along the track, in meters.",
+ "coulombsLaw.atomicScreen.coulombsLawAtomicScreen.model.charge2.positionProperty.rangeProperty": {
+ "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": false,
+ "phetioReadOnly": true,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "NumberPropertyIO"
+ "phetioTypeName": "PropertyIO.<RangeIO>"
},
"coulombsLaw.atomicScreen.coulombsLawAtomicScreen.model.charge2.radiusProperty": {
"phetioDocumentation": "The radius of the object",
@@ -5053,29 +5053,29 @@
"phetioStudioControl": true,
"phetioTypeName": "PropertyIO.<BooleanIO>"
},
- "coulombsLaw.macroScreen.coulombsLawMacroScreen.model.charge1.enabledRangeProperty": {
- "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
+ "coulombsLaw.macroScreen.coulombsLawMacroScreen.model.charge1.positionProperty": {
+ "phetioDocumentation": "The position of the object along the track, in meters.",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": true,
+ "phetioReadOnly": false,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "PropertyIO.<RangeIO>"
+ "phetioTypeName": "NumberPropertyIO"
},
- "coulombsLaw.macroScreen.coulombsLawMacroScreen.model.charge1.positionProperty": {
- "phetioDocumentation": "The position of the object along the track, in meters.",
+ "coulombsLaw.macroScreen.coulombsLawMacroScreen.model.charge1.positionProperty.rangeProperty": {
+ "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": false,
+ "phetioReadOnly": true,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "NumberPropertyIO"
+ "phetioTypeName": "PropertyIO.<RangeIO>"
},
"coulombsLaw.macroScreen.coulombsLawMacroScreen.model.charge1.radiusProperty": {
"phetioDocumentation": "The radius of the object",
@@ -5125,29 +5125,29 @@
"phetioStudioControl": true,
"phetioTypeName": "PropertyIO.<BooleanIO>"
},
- "coulombsLaw.macroScreen.coulombsLawMacroScreen.model.charge2.enabledRangeProperty": {
- "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
+ "coulombsLaw.macroScreen.coulombsLawMacroScreen.model.charge2.positionProperty": {
+ "phetioDocumentation": "The position of the object along the track, in meters.",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": true,
+ "phetioReadOnly": false,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "PropertyIO.<RangeIO>"
+ "phetioTypeName": "NumberPropertyIO"
},
- "coulombsLaw.macroScreen.coulombsLawMacroScreen.model.charge2.positionProperty": {
- "phetioDocumentation": "The position of the object along the track, in meters.",
+ "coulombsLaw.macroScreen.coulombsLawMacroScreen.model.charge2.positionProperty.rangeProperty": {
+ "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": false,
+ "phetioReadOnly": true,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "NumberPropertyIO"
+ "phetioTypeName": "PropertyIO.<RangeIO>"
},
"coulombsLaw.macroScreen.coulombsLawMacroScreen.model.charge2.radiusProperty": {
"phetioDocumentation": "The radius of the object",
Index: coulombs-law/js/phet-io/coulombs-law-phet-io-types.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- coulombs-law/js/phet-io/coulombs-law-phet-io-types.js (revision 84a77e9531078ee6110679557d6544b8bf3163b2)
+++ coulombs-law/js/phet-io/coulombs-law-phet-io-types.js (date 1566435824670)
@@ -1275,7 +1275,7 @@
],
"methods": {
"addEventListener": {
- "documentation": "Adds a listener to the PhET-iO dataStream, which can be used to respond to events or for data analysis. Unlike Client.launchSim( {onEvent} ) which is called recursively for each child event, this is only called with top-level events.",
+ "documentation": "Adds a listener to the PhET-iO dataStream, which can be used to respond to events or for data analysis. Unlike Client.launchSim( {onEvent} ) which is called recursively for each parsed child event, this is only called with stringified top-level events.",
"parameterTypes": [
"FunctionIO.(ObjectIO)=>VoidIO"
],
Index: gravity-force-lab/js/phet-io/gravity-force-lab-phet-io-elements-baseline.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- gravity-force-lab/js/phet-io/gravity-force-lab-phet-io-elements-baseline.js (revision e959191e9bc26910900dfe4ccab00b2a18f9e0fc)
+++ gravity-force-lab/js/phet-io/gravity-force-lab-phet-io-elements-baseline.js (date 1566435816378)
@@ -901,29 +901,29 @@
"phetioStudioControl": true,
"phetioTypeName": "DerivedPropertyIO.<NumberIO>"
},
- "gravityForceLab.gravityForceLabScreen.model.mass1.enabledRangeProperty": {
- "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
+ "gravityForceLab.gravityForceLabScreen.model.mass1.positionProperty": {
+ "phetioDocumentation": "The position of the object along the track, in meters.",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": true,
+ "phetioReadOnly": false,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "PropertyIO.<RangeIO>"
+ "phetioTypeName": "NumberPropertyIO"
},
- "gravityForceLab.gravityForceLabScreen.model.mass1.positionProperty": {
- "phetioDocumentation": "The position of the object along the track, in meters.",
+ "gravityForceLab.gravityForceLabScreen.model.mass1.positionProperty.rangeProperty": {
+ "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": false,
+ "phetioReadOnly": true,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "NumberPropertyIO"
+ "phetioTypeName": "PropertyIO.<RangeIO>"
},
"gravityForceLab.gravityForceLabScreen.model.mass1.radiusProperty": {
"phetioDocumentation": "The radius of the object",
@@ -949,29 +949,29 @@
"phetioStudioControl": true,
"phetioTypeName": "NumberPropertyIO"
},
- "gravityForceLab.gravityForceLabScreen.model.mass2.enabledRangeProperty": {
- "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
+ "gravityForceLab.gravityForceLabScreen.model.mass2.positionProperty": {
+ "phetioDocumentation": "The position of the object along the track, in meters.",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": true,
+ "phetioReadOnly": false,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "PropertyIO.<RangeIO>"
+ "phetioTypeName": "NumberPropertyIO"
},
- "gravityForceLab.gravityForceLabScreen.model.mass2.positionProperty": {
- "phetioDocumentation": "The position of the object along the track, in meters.",
+ "gravityForceLab.gravityForceLabScreen.model.mass2.positionProperty.rangeProperty": {
+ "phetioDocumentation": "The range for position of this object based on the radius and location of both objects",
"phetioDynamicElement": false,
"phetioEventType": "MODEL",
"phetioFeatured": false,
"phetioHighFrequency": false,
"phetioPlayback": false,
- "phetioReadOnly": false,
+ "phetioReadOnly": true,
"phetioState": true,
"phetioStudioControl": true,
- "phetioTypeName": "NumberPropertyIO"
+ "phetioTypeName": "PropertyIO.<RangeIO>"
},
"gravityForceLab.gravityForceLabScreen.model.mass2.radiusProperty": {
"phetioDocumentation": "The radius of the object",
Index: gravity-force-lab/js/gravity-force-lab/view/GravityForceLabAlertManager.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- gravity-force-lab/js/gravity-force-lab/view/GravityForceLabAlertManager.js (revision e959191e9bc26910900dfe4ccab00b2a18f9e0fc)
+++ gravity-force-lab/js/gravity-force-lab/view/GravityForceLabAlertManager.js (date 1566436787623)
@@ -94,7 +94,7 @@
const secondaryPositionChangedListener = objectEnum => {
// handle the case where the position changed from the constant radius being toggled
- if ( model.object1.constantRadiusChangedSinceLastStep || model.object2.constantRadiusChangedSinceLastStep ) {
+ if ( model.object1.constantRadiusChangedSinceLastStepProperty.value || model.object2.constantRadiusChangedSinceLastStepProperty.value ) {
this.alertConstantSizeChangedPosition();
}
else { // value specific assumption
Index: gravity-force-lab/js/phet-io/gravity-force-lab-phet-io-types.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- gravity-force-lab/js/phet-io/gravity-force-lab-phet-io-types.js (revision e959191e9bc26910900dfe4ccab00b2a18f9e0fc)
+++ gravity-force-lab/js/phet-io/gravity-force-lab-phet-io-types.js (date 1566435816420)
@@ -1175,7 +1175,7 @@
],
"methods": {
"addEventListener": {
- "documentation": "Adds a listener to the PhET-iO dataStream, which can be used to respond to events or for data analysis. Unlike Client.launchSim( {onEvent} ) which is called recursively for each child event, this is only called with top-level events.",
+ "documentation": "Adds a listener to the PhET-iO dataStream, which can be used to respond to events or for data analysis. Unlike Client.launchSim( {onEvent} ) which is called recursively for each parsed child event, this is only called with stringified top-level events.",
"parameterTypes": [
"FunctionIO.(ObjectIO)=>VoidIO"
], |
zepumph
added a commit
to phetsims/inverse-square-law-common
that referenced
this issue
Aug 23, 2019
zepumph
added a commit
to phetsims/gravity-force-lab-basics
that referenced
this issue
Aug 23, 2019
zepumph
added a commit
that referenced
this issue
Aug 23, 2019
zepumph
added a commit
to phetsims/coulombs-law
that referenced
this issue
Aug 23, 2019
|
Again I tried to fix this, and continued to run into many problems within validating the range and the position as they change. I continued to hit new assertions as I tried to fix prior ones, and I don't feel like I know the model well enough to trust any of the hacky changes I'm making. It would potentially be nice to pair with @jessegreenberg on this, because I think he understands the model operations a bit better. |
|
@jessegreenberg and I worked on this, and we were able to think through the problem a bit more thoroughly. It seemed like good things happened when we decided to update the range of the positionProperty whenever a positionProperty was set that would require changing. (i.e. update object2 range when object1 position changes, or update object2 range when object1 radius changed). This more manual update seemed to work a bit nicer. but we are still getting an assertion where the range is being updated to something that the current position doesn't validate in. |
|
I ended up committing the common-code piece of this work over in phetsims/projectile-motion#192. I'm unsure about how to proceed here, as it is pretty broken, but it seems like that faulty logic is built pretty deep into the way that ISLCModel is stepped. I'm not sure how long it would take to continue on the above patch, or if it is even worth it. I think I would like to ask designers at an upcoming phet-io meeting. |
zepumph
referenced
this issue
in phetsims/axon
Nov 11, 2019
zepumph
referenced
this issue
in phetsims/axon
Nov 11, 2019
|
The design team did not think that trying to add the dynamic range was worth the effort. We did think though that updating the static range to be -4.8 to 4.8 instead of -5 to 5. So that it actually represents the true min/max of the track. We should also add to the |
zepumph
added a commit
to phetsims/inverse-square-law-common
that referenced
this issue
Jan 7, 2020
zepumph
added a commit
to phetsims/gravity-force-lab-basics
that referenced
this issue
Jan 7, 2020
zepumph
added a commit
that referenced
this issue
Jan 7, 2020
zepumph
added a commit
to phetsims/coulombs-law
that referenced
this issue
Jan 7, 2020
This was being handled in BASICS but not yet in REGULAR, it was good that we noticed it. All has now been dealt with here, closing. |
zepumph
added a commit
to phetsims/inverse-square-law-common
that referenced
this issue
Feb 26, 2021
zepumph
added a commit
to phetsims/inverse-square-law-common
that referenced
this issue
Mar 8, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is a problem in Studio because the enabledRange is constantly changing, but that isn't reflected in the positionProperty.range metadata in studio.
The text was updated successfully, but these errors were encountered: