Closed pixelzoom closed 3 years ago
While I'm waiting to be able to patch the 1.3 branch, I ran the Diff Wrapper to compare 1.2 and master (which should be the same as 1.3). Below is the set of "Breaking Changes". I'm very confused by what I see here, and it's going to take me hours to go through this.
@kathy-phet @amanda-phet please advise: Do you want the API changes reviewed and verified, to identify any unintended changes that may have been introduced?
... I'm very confused by what I see here
Please DO NOT review the diff above. @samreid and I just discovered that Diff wrapper has its comparison inverted, see https://github.com/phetsims/phet-io-wrappers/issues/408. So the above diff results are meaningless.
This issue is on hold until #271 (Diff wrapper is broken) is resolved.
@zepumph and I used the diff wrapper and our knowledge of recent changes to write this condensed migration guide. This guide or something like it would be useful to supply to clients that we know are using 1.2. In the future, we may wish to include migration guides like this into builds, as part of or adjacent to the diff wrapper.
Natural Selection 1.2 -> 1.3 Migration guide
naturalSelection.general.controller.input.mouseDownAction.phetioTypeName changed from ActionIO<Vector2IO, EventIO> to ActionIO<NullableIO<NumberIO>, Vector2IO, EventIO>
PhET-iO Element missing: naturalSelection.introScreen.view.environmentNode
PhET-iO Element missing: naturalSelection.introScreen.view.environmentRadioButtonGroup
PhET-iO Element missing: naturalSelection.introScreen.view.generationClockNode
naturalSelection.introScreen.view.graphs.populationNode.populationGraphNode.dataProbeNode.visibleProperty.phetioTypeName changed from PropertyIO<BooleanIO> to LinkedElementIO
PhET-iO Element missing: naturalSelection.introScreen.view.playButtonGroup
naturalSelection.introScreen.view.timeControlNode.fastForwardButton.phetioTypeName changed from RoundMomentaryButtonIO to NodeIO
PhET-iO Element missing: naturalSelection.labScreen.view.environmentNode
PhET-iO Element missing: naturalSelection.labScreen.view.environmentRadioButtonGroup
PhET-iO Element missing: naturalSelection.labScreen.view.generationClockNode
naturalSelection.labScreen.view.graphs.populationNode.populationGraphNode.dataProbeNode.visibleProperty.phetioTypeName changed from PropertyIO<BooleanIO> to LinkedElementIO
PhET-iO Element missing: naturalSelection.labScreen.view.playButtonGroup
naturalSelection.labScreen.view.timeControlNode.fastForwardButton.phetioTypeName changed from RoundMomentaryButtonIO to NodeIO
AlleleIO parameter types changed from ObjectIO to . This may or may not be a breaking change, but we are reporting it just in case.
GeneIO parameter types changed from ObjectIO to . This may or may not be a breaking change, but we are reporting it just in case.
Pickable property have been replaced with inputEnabledProperty
Opacity property has been removed
pressAction and releaseAction became phetioReadOnly
ObservableArrayIO is missing event: itemAdded, please see the emitted event from the elementAddedEmitter
ObservableArrayIO is missing event: itemRemoved, please see the emitted event from the elementRemovedEmitter
Method missing, type=ObservableArrayIO, method=addItemAddedListener, please use elementAddedEmitter
Method missing, type=ObservableArrayIO, method=addItemRemovedListener, please use elementRemovedEmitter
Method missing, type=ObservableArrayIO, method=getLength, please use lengthProperty
Look for corresponding emitters for the following:
Method missing, type=PhetioEngineIO, method=addPhetioElementAddedListener
Method missing, type=PhetioEngineIO, method=addPhetioElementRemovedListener
Method missing, type=PhetioEngineIO, method=addPhetioElementsBaselineListener
Method missing, type=PhetioEngineIO, method=addTypesAddedListener
Method missing, type=PhetioEngineIO, method=endEvent
Method missing, type=PhetioEngineIO, method=startEvent
Feedback on "condensed migration guide"....
naturalSelection.general.controller.input.mouseDownAction.phetioTypeName changed from ActionIO<Vector2IO, EventIO> to ActionIO<NullableIO
, Vector2IO, EventIO> PhET-iO Element missing: naturalSelection.introScreen.view.environmentNode PhET-iO Element missing: naturalSelection.introScreen.view.environmentRadioButtonGroup PhET-iO Element missing: naturalSelection.introScreen.view.generationClockNode PhET-iO Element missing: naturalSelection.introScreen.view.playButtonGroup PhET-iO Element missing: naturalSelection.labScreen.view.environmentNode PhET-iO Element missing: naturalSelection.labScreen.view.environmentRadioButtonGroup PhET-iO Element missing: naturalSelection.labScreen.view.generationClockNode PhET-iO Element missing: naturalSelection.labScreen.view.playButtonGroup
None of the above elements are missing - they have been relocated. A new environmentPanel
element was introduced as an ancestor. And that's probably what's useful to tell clients in a migraton guide.
naturalSelection.introScreen.view.graphs.populationNode.populationGraphNode.dataProbeNode.visibleProperty.phetioTypeName changed from PropertyIO
to LinkedElementIO naturalSelection.introScreen.view.timeControlNode.fastForwardButton.phetioTypeName changed from RoundMomentaryButtonIO to NodeIO naturalSelection.labScreen.view.graphs.populationNode.populationGraphNode.dataProbeNode.visibleProperty.phetioTypeName changed from PropertyIO to LinkedElementIO
I have no idea what the above is telling me, or how I'd use it.
AlleleIO parameter types changed from ObjectIO to . This may or may not be a breaking change, but we are reporting it just in case. GeneIO parameter types changed from ObjectIO to . This may or may not be a breaking change, but we are reporting it just in case.
"from ObjectIO to ." ?? What does that mean?
Pickable property have been replaced with inputEnabledProperty Opacity property has been removed
Uppercase "property" please. And why not pickableProperty
and opacityProperty
, which is what the element names were? And you're referring to inputEnabledProperty
...
Method missing, type=ObservableArrayIO, method=addItemAddedListener, please use elementAddedEmitter Method missing, type=ObservableArrayIO, method=addItemRemovedListener, please use elementRemovedEmitter Method missing, type=ObservableArrayIO, method=getLength, please use lengthProperty
Look for corresponding emitters for the following: Method missing, type=PhetioEngineIO, method=addPhetioElementAddedListener Method missing, type=PhetioEngineIO, method=addPhetioElementRemovedListener Method missing, type=PhetioEngineIO, method=addPhetioElementsBaselineListener Method missing, type=PhetioEngineIO, method=addTypesAddedListener Method missing, type=PhetioEngineIO, method=endEvent Method missing, type=PhetioEngineIO, method=startEvent
I have no idea what the above is telling me, or how I'd use this info.
So I think this is a good idea in concept. But if PhET decides to produce a migration guide, it needs to be a more instructive and informative.
I agree it needs to be more instructive and informative, and knowing the sim specific details will help in that regard.
"from ObjectIO to ." ?? What does that mean?
It changed from having a parameter type of ObjectIO
to not having a parameter type at all. I'm sure we can phrase that better.
Uppercase "property" please.
Great idea!
All of @pixelzoom comments are good ideas, but it unclear how much time we should spend refining this document, or producing similar documents when this comes up in the future. For instance, should this be a standard document supplied with each new minor version?
@pixelzoom, sorry we weren't clear, we did not try to understand any of the natural-selection specific changes, but were thinking that you would instead apply the same strategy-esque writing that we did for common code.
Got it, thanks for clarifying.
AlleleIO parameter types changed from ObjectIO to . This may or may not be a breaking change, but we are reporting it just in case. GeneIO parameter types changed from ObjectIO to . This may or may not be a breaking change, but we are reporting it just in case.
After the below commit, these now look like:
AlleleIO parameter types changed from [ObjectIO] to []. This may or may not be a breaking change, but we are reporting it just in case. GeneIO parameter types changed from [ObjectIO] to []. This may or may not be a breaking change, but we are reporting it just in case.
Most likely this change was because the supertype is parametric now (ReferenceIO
@pixelzoom, @samreid and I took the above in https://github.com/phetsims/natural-selection/issues/275#issuecomment-827973296 and annotated the rest of it. Here is the updated guide:
Natural Selection 1.2 -> 1.3 Migration Guide
// model visibleProperty is now passed to view.
// See https://github.com/phetsims/natural-selection/issues/262
naturalSelection.introScreen.view.graphs.populationNode.populationGraphNode.dataProbeNode.visibleProperty.phetioTypeName changed from PropertyIO<BooleanIO> to LinkedElementIO
naturalSelection.labScreen.view.graphs.populationNode.populationGraphNode.dataProbeNode.visibleProperty.phetioTypeName changed from PropertyIO<BooleanIO> to LinkedElementIO
// Removed useless IOType, use parent instead.
// See https://github.com/phetsims/sun/issues/558
naturalSelection.introScreen.view.timeControlNode.fastForwardButton.phetioTypeName changed from RoundMomentaryButtonIO to NodeIO
naturalSelection.labScreen.view.timeControlNode.fastForwardButton.phetioTypeName changed from RoundMomentaryButtonIO to NodeIO
// Used to extend ReferenceIO but now uses IOType.supertype.
// See https://github.com/phetsims/tandem/issues/211
AlleleIO parameter types changed from ObjectIO to . This may or may not be a breaking change, but we are reporting it just in case.
GeneIO parameter types changed from ObjectIO to . This may or may not be a breaking change, but we are reporting it just in case.
// environmentPanel was added as a parent element.
// See https://github.com/phetsims/natural-selection/issues/263
naturalSelection.introScreen.view.environmentNode moved to naturalSelection.introScreen.view.environmentPanel.environmentNode
naturalSelection.introScreen.view.environmentRadioButtonGroup moved to naturalSelection.introScreen.view.environmentPanel.environmentRadioButtonGroup
naturalSelection.introScreen.view.generationClockNode moved to naturalSelection.introScreen.view.environmentPanel.generationClockNode
naturalSelection.introScreen.view.playButtonGroup moved to naturalSelection.introScreen.view.environmentPanel.playButtonGroup
naturalSelection.labScreen.view.environmentNode moved to naturalSelection.labScreen.view.environmentPanel.environmentNode
naturalSelection.labScreen.view.environmentRadioButtonGroup moved to naturalSelection.labScreen.view.environmentPanel.environmentRadioButtonGroup
naturalSelection.labScreen.view.generationClockNode moved to naturalSelection.labScreen.view.environmentPanel.generationClockNode
naturalSelection.labScreen.view.playButtonGroup moved to naturalSelection.labScreen.view.environmentPanel.playButtonGroup
// pickableProperty have been replaced with inputEnabledProperty.
// See https://github.com/phetsims/scenery/issues/1158
. . .
// opacityProperty has been uninstrumented.
// See https://github.com/phetsims/scenery/issues/1158
. . .
// PressListener pressAction and releaseAction became phetioReadOnly: true.
// See https://github.com/phetsims/scenery/commit/3bf7cb79e5aeb72ef30055c28759d151c62c66b5
. . .
// ObservableArray events and methods were replaced with Emitters.
// See https://github.com/phetsims/axon/issues/330
ObservableArrayIO is missing event: itemAdded, please use elementAddedEmitter
ObservableArrayIO is missing event: itemRemoved, please use elementRemovedEmitter
Method missing, type=ObservableArrayIO, method=addItemAddedListener, please use elementAddedEmitter
Method missing, type=ObservableArrayIO, method=addItemRemovedListener, please use elementRemovedEmitter
Method missing, type=ObservableArrayIO, method=getLength, please use lengthProperty
// phetioEngine methods were converted to Emitters.
// See https://github.com/phetsims/phet-io/issues/1655
Method missing, type=PhetioEngineIO, method=addPhetioElementAddedListener
Method missing, type=PhetioEngineIO, method=addPhetioElementRemovedListener
Method missing, type=PhetioEngineIO, method=addPhetioElementsBaselineListener
Method missing, type=PhetioEngineIO, method=addTypesAddedListener
Method missing, type=PhetioEngineIO, method=endEvent
Method missing, type=PhetioEngineIO, method=startEvent
// Input.js mouseDownAction parameter signature was changed to support iframe fix.
// See https://github.com/phetsims/natural-selection/issues/264.
naturalSelection.general.controller.input.mouseDownAction.phetioTypeName changed from ActionIO<Vector2IO, EventIO> to ActionIO<NullableIO<NumberIO>, Vector2IO, EventIO> changed a parameter
I added some missing issue numbers to the migration guide in the previous commit, so that all changes are documented with a GitHub issue.
For posterity, see below for the full "Breaking Changes" diff, created using by comparing 1.3 to 1.2.
All of the diffs reported appear to be intended and accounted for, so I don't see any reason to have @amanda-phet and @kathy-phet review this. I'll close this issue and proceed with the next RC.
When the Diff wrapper is fixed (see https://github.com/phetsims/natural-selection/issues/271), we'll presumably want to use the Diff wrapper to determine if there were unintended changes between 1.2 and 1.3. This issue is to review and verify those changes.