zepumph
commented
3 years ago I am finding that this is increasing the number of lines in the API file by ~7-10%. I don't think that is a deal breaker, but wanted to note that I specifically removed this comment in Property about units:
// Only supply units if they were specified, to avoid seeing "units: null" in so many properties, see https://github.com/phetsims/phet-io/issues/1315
Looks like that issue has reminded to me to look into studio to see if any of these changes will harm the wrapper side.
Here is my current changeset:
Index: axon/js/NumberProperty.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/axon/js/NumberProperty.js b/axon/js/NumberProperty.js
--- a/axon/js/NumberProperty.js (revision d848101bd6e9f29d49173b5f41e8a5c7baa70516)
+++ b/axon/js/NumberProperty.js (date 1621720087193)
@@ -240,19 +240,14 @@
const parentStateObject = PropertyIOImpl.toStateObject( numberProperty );
// conditionals to avoid keys with value "null" in state objects
- if ( numberProperty.numberType ) {
- parentStateObject.numberType = numberProperty.numberType;
- }
+ parentStateObject.numberType = NullableIO( StringIO ).toStateObject( numberProperty.numberType );
+
+ parentStateObject.range = NullableIO( Range.RangeIO ).toStateObject( numberProperty.rangeProperty.value );
- if ( numberProperty.rangeProperty.value ) {
- parentStateObject.range = Range.RangeIO.toStateObject( numberProperty.rangeProperty.value );
- if ( numberProperty.rangeProperty.isPhetioInstrumented() ) {
- parentStateObject.rangePhetioID = StringIO.toStateObject( numberProperty.rangeProperty.tandem.phetioID );
- }
- }
- if ( numberProperty.step ) {
- parentStateObject.step = NumberIO.toStateObject( numberProperty.step );
- }
+ const hasRangePhetioID = numberProperty.rangeProperty && numberProperty.rangeProperty.isPhetioInstrumented();
+ parentStateObject.rangePhetioID = hasRangePhetioID ? StringIO.toStateObject( numberProperty.rangeProperty.tandem.phetioID ) : null;
+
+ parentStateObject.step = NullableIO( NumberIO ).toStateObject( numberProperty.step );
return parentStateObject;
},
applyState: ( numberProperty, stateObject ) => {
@@ -260,6 +255,7 @@
PropertyIOImpl.applyState( numberProperty, stateObject );
numberProperty.step = stateObject.step;
numberProperty.numberType = stateObject.numberType;
+ // TODO: apply range if rangePhetioID is null?
}
} );
Index: scenery/js/nodes/IndexedNodeIO.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/nodes/IndexedNodeIO.js b/scenery/js/nodes/IndexedNodeIO.js
--- a/scenery/js/nodes/IndexedNodeIO.js (revision 6734e11f7f1178eebeb2250aecd5601a8dc2ffb6)
+++ b/scenery/js/nodes/IndexedNodeIO.js (date 1621720534436)
@@ -30,12 +30,15 @@
assert && assert( node.parents.length === 1, 'IndexedNodeIO only supports nodes with a single parent' );
stateObject.index = node.parents[ 0 ].indexOfChild( node );
}
+ else {
+ stateObject.index = null;
+ }
return stateObject;
},
- applyState: ( node, fromStateObject ) => {
- if ( node.parents[ 0 ] ) {
+ applyState: ( node, stateObject ) => {
+ if ( node.parents[ 0 ] && stateObject.index ) {
assert && assert( node.parents.length === 1, 'IndexedNodeIO only supports nodes with a single parent' );
- node.parents[ 0 ].moveChildToIndex( node, fromStateObject.index );
+ node.parents[ 0 ].moveChildToIndex( node, stateObject.index );
}
},
methods: {
Index: axon/js/Property.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/axon/js/Property.js b/axon/js/Property.js
--- a/axon/js/Property.js (revision d848101bd6e9f29d49173b5f41e8a5c7baa70516)
+++ b/axon/js/Property.js (date 1621720310656)
@@ -13,6 +13,7 @@
import FunctionIO from '../../tandem/js/types/FunctionIO.js';
import IOType from '../../tandem/js/types/IOType.js';
import NullableIO from '../../tandem/js/types/NullableIO.js';
+import StringIO from '../../tandem/js/types/StringIO.js';
import VoidIO from '../../tandem/js/types/VoidIO.js';
import axon from './axon.js';
import Multilink from './Multilink.js';
@@ -579,7 +580,7 @@
events: [ 'changed' ],
parameterTypes: [ parameterType ],
toStateObject: property => {
- assert && assert( parameterType.toStateObject, `toStateObject doesnt exist for ${parameterType.typeName}` );
+ assert && assert( parameterType.toStateObject, `toStateObject doesn't exist for ${parameterType.typeName}` );
const stateObject = {
value: parameterType.toStateObject( property.value )
};
@@ -590,15 +591,15 @@
return parameterType.toStateObject( v );
} );
}
-
- // Only supply units if they were specified, to avoid seeing "units: null" in so many properties, see https://github.com/phetsims/phet-io/issues/1315
- if ( property.units ) {
- stateObject.units = property.units;
+ else {
+ stateObject.validValues = null;
}
+
+ stateObject.units = NullableIO( StringIO ).toStateObject( property.units );
return stateObject;
},
applyState: ( property, stateObject ) => {
- property.units = stateObject.units;
+ property.units = NullableIO( StringIO ).fromStateObject( stateObject.units );
property.set( parameterType.fromStateObject( stateObject.value ) );
if ( stateObject.validValues ) {
Index: scenery/js/input/SceneryEvent.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery/js/input/SceneryEvent.js b/scenery/js/input/SceneryEvent.js
--- a/scenery/js/input/SceneryEvent.js (revision 6734e11f7f1178eebeb2250aecd5601a8dc2ffb6)
+++ b/scenery/js/input/SceneryEvent.js (date 1621720534436)
@@ -133,12 +133,8 @@
type: event.type
};
- if ( event.domEvent ) {
- eventObject.domEventType = event.domEvent.type;
- }
- if ( event.pointer && event.pointer.point ) {
- eventObject.point = Vector2.Vector2IO.toStateObject( event.pointer.point );
- }
+ eventObject.domEventType = event.domEvent ? event.domEvent.type : null;
+ eventObject.point = ( event.pointer && event.pointer.point ) ? Vector2.Vector2IO.toStateObject( event.pointer.point ) : null;
// Note: If changing the contents of this object, please document it in the public documentation string.
return eventObject;
samreid
From https://github.com/phetsims/phet-io/issues/1774. In order to have a stateSchema based on each IOType, we need to have a consistent state for each IOType. This means that we can no longer conditionally add state keys like units, range, etc. Tagging @samreid because I feel like doing this as a side issue, before continuing with https://github.com/phetsims/phet-io/issues/1774, and I have hope that it won't interfere too badly with the patch over in that issue.