phetsims / density-buoyancy-common

Common code for the Density and Buoyancy simulations
GNU General Public License v3.0
0 stars 2 forks source link

Review API changes from `staticMaterial` work for regressions. #271

Closed samreid closed 1 month ago

samreid commented 1 month ago

After merging the staticMaterials branch, we want to understand and correct for all of the API changes in

For instance, here are the buoyancy issues we detected with API comparison, we annotated some of them near the top.

``` buoyancy BREAKING PROBLEMS // Changed to materialProperty.customMaterial.densityProperty PhET-iO Element missing: buoyancy.applicationsScreen.model.objects.bottle.materialInside.customDensityProperty // dependencies will change, that's ok. We renamed it and pointed to the density Property PhET-iO Element missing: buoyancy.applicationsScreen.model.objects.bottle.materialInside.massProperty.dependencies.buoyancy-applicationsScreen-model-objects-bottle-materialInside-materialProperty // Uninstrumented PhET-iO Element missing: buoyancy.applicationsScreen.model.pool.scale.materialProperty PhET-iO Element missing: buoyancy.applicationsScreen.model.scale.materialProperty PhET-iO Element missing: buoyancy.applicationsScreen.view.blockControls.comboBox.listBox.customItem buoyancy.applicationsScreen.view.blockControls.comboBox.property._data.initialState differs. Expected: {"elementID":"buoyancy.applicationsScreen.view.blockControls.comboBoxMaterialProperty"} actual: {"elementID":"buoyancy.applicationsScreen.model.objects.block.materialProperty"} PhET-iO Element missing: buoyancy.applicationsScreen.view.blockControls.comboBoxMaterialProperty buoyancy.applicationsScreen.view.blockControls.massNumberControl.highDensityMassNumberControl.visibleProperty._data.initialState differs. Expected: {"units":null,"validValues":null,"value":false} actual: {"value":true,"validValues":null,"units":null} PhET-iO Element missing: buoyancy.applicationsScreen.view.bottleControls.materialInsideControls.comboBox.listBox.customItem buoyancy.applicationsScreen.view.bottleControls.materialInsideControls.comboBox.property._data.initialState differs. Expected: {"elementID":"buoyancy.applicationsScreen.view.bottleControls.materialInsideControls.comboBoxMaterialProperty"} actual: {"elementID":"buoyancy.applicationsScreen.model.objects.bottle.materialInside.materialInsideProperty"} PhET-iO Element missing: buoyancy.applicationsScreen.view.bottleControls.materialInsideControls.comboBoxMaterialProperty PhET-iO Element missing: buoyancy.applicationsScreen.view.bottleControls.materialInsideControls.customBottleDensityNumberControl.numberDisplay.valueText.stringProperty.dependencies PhET-iO Element missing: buoyancy.applicationsScreen.view.bottleControls.materialInsideControls.customBottleDensityNumberControl.slider.valueProperty PhET-iO Element missing: buoyancy.applicationsScreen.view.fluidDensityPanel.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.compareScreen.model.pool.scale.materialProperty PhET-iO Element missing: buoyancy.compareScreen.model.scale.materialProperty PhET-iO Element missing: buoyancy.exploreScreen.model.pool.scale.materialProperty PhET-iO Element missing: buoyancy.exploreScreen.model.scale.materialProperty PhET-iO Element missing: buoyancy.exploreScreen.view.blockAPanel.comboBox.listBox.customItem buoyancy.exploreScreen.view.blockAPanel.comboBox.property._data.initialState differs. Expected: {"elementID":"buoyancy.exploreScreen.view.blockAPanel.comboBoxMaterialProperty"} actual: {"elementID":"buoyancy.exploreScreen.model.blocks.blockA.materialProperty"} PhET-iO Element missing: buoyancy.exploreScreen.view.blockAPanel.comboBoxMaterialProperty PhET-iO Element missing: buoyancy.exploreScreen.view.blockBPanel.comboBox.listBox.customItem buoyancy.exploreScreen.view.blockBPanel.comboBox.property._data.initialState differs. Expected: {"elementID":"buoyancy.exploreScreen.view.blockBPanel.comboBoxMaterialProperty"} actual: {"elementID":"buoyancy.exploreScreen.model.blocks.blockB.materialProperty"} PhET-iO Element missing: buoyancy.exploreScreen.view.blockBPanel.comboBoxMaterialProperty PhET-iO Element missing: buoyancy.exploreScreen.view.fluidDensityPanel.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.labScreen.model.pool.scale.materialProperty PhET-iO Element missing: buoyancy.labScreen.model.scale.materialProperty PhET-iO Element missing: buoyancy.labScreen.view.blockPanel.comboBox.listBox.customItem buoyancy.labScreen.view.blockPanel.comboBox.property._data.initialState differs. Expected: {"elementID":"buoyancy.labScreen.view.blockPanel.comboBoxMaterialProperty"} actual: {"elementID":"buoyancy.labScreen.model.block.materialProperty"} PhET-iO Element missing: buoyancy.labScreen.view.blockPanel.comboBoxMaterialProperty PhET-iO Element missing: buoyancy.labScreen.view.fluidDensityPanel.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.labScreen.view.gravityPanel.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.shapesScreen.model.pool.scale.materialProperty PhET-iO Element missing: buoyancy.shapesScreen.model.scale.materialProperty PhET-iO Element missing: buoyancy.shapesScreen.view.fluidDensityPanel.comboBox.listBox.customItem buoyancy.shapesScreen.view.materialControlNode.comboBox.property._data.initialState differs. Expected: {"elementID":"buoyancy.shapesScreen.view.materialControlNode.comboBoxMaterialProperty"} actual: {"elementID":"buoyancy.shapesScreen.model.objects.materialProperty"} PhET-iO Element missing: buoyancy.shapesScreen.view.materialControlNode.comboBoxMaterialProperty Type missing: ArrayIO Type missing: ArrayIO Type missing: FunctionIO(GravityIO)=>VoidIO Type missing: FunctionIO(GravityIO,NullableIO)=>VoidIO Type missing: FunctionIO(MaterialIO)=>VoidIO Type missing: FunctionIO(MaterialIO,NullableIO)=>VoidIO Type missing: GravityIO Type missing: MaterialIO Type missing: NullableIO> Type missing: NullableIO> Type missing: NullableIO Type missing: NullableIO Type missing: NullableIO>> Type missing: PropertyIO Type missing: PropertyIO Type missing: ReferenceIO> Type missing: ReferenceIO> ```

Here is the sha with the legacy API files from main that we should endeavor to understand: https://github.com/phetsims/phet-io-sim-specific/commit/4049c3101e5ecf0bf8ae225b15c56ac8eb2c3e17

zepumph commented 1 month ago

Tagging https://github.com/phetsims/density-buoyancy-common/issues/256.

zepumph commented 1 month ago

Working on this now.

zepumph commented 1 month ago

Ok. Here is an updated list, organized:

``` // Changed to materialProperty.customMaterial.densityProperty PhET-iO Element missing: buoyancy.applicationsScreen.model.objects.bottle.materialInside.customDensityProperty // dependencies will change, that's ok. We renamed it and pointed to the density Property PhET-iO Element missing: buoyancy.applicationsScreen.model.objects.bottle.materialInside.massProperty.dependencies.buoyancy-applicationsScreen-model-objects-bottle-materialInside-materialProperty // Uninstrumented scale.materialProperty PhET-iO Element missing: buoyancy.applicationsScreen.model.pool.scale.materialProperty PhET-iO Element missing: buoyancy.applicationsScreen.model.scale.materialProperty PhET-iO Element missing: buoyancy.compareScreen.model.pool.scale.materialProperty PhET-iO Element missing: buoyancy.compareScreen.model.scale.materialProperty PhET-iO Element missing: buoyancy.exploreScreen.model.pool.scale.materialProperty PhET-iO Element missing: buoyancy.exploreScreen.model.scale.materialProperty PhET-iO Element missing: buoyancy.labScreen.model.pool.scale.materialProperty PhET-iO Element missing: buoyancy.labScreen.model.scale.materialProperty PhET-iO Element missing: buoyancy.shapesScreen.model.pool.scale.materialProperty PhET-iO Element missing: buoyancy.shapesScreen.model.scale.materialProperty // Renamed back from customMaterialItem PhET-iO Element missing: buoyancy.applicationsScreen.view.blockControls.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.applicationsScreen.view.bottleControls.materialInsideControls.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.applicationsScreen.view.fluidDensityPanel.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.exploreScreen.view.blockAPanel.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.exploreScreen.view.blockBPanel.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.exploreScreen.view.fluidDensityPanel.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.labScreen.view.fluidDensityPanel.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.labScreen.view.gravityPanel.comboBox.listBox.customItem PhET-iO Element missing: buoyancy.shapesScreen.view.fluidDensityPanel.comboBox.listBox.customItem // Yup! That's kinda the point. PhET-iO Element missing: buoyancy.applicationsScreen.view.blockControls.comboBoxMaterialProperty PhET-iO Element missing: buoyancy.applicationsScreen.view.bottleControls.materialInsideControls.comboBoxMaterialProperty PhET-iO Element missing: buoyancy.exploreScreen.view.blockAPanel.comboBoxMaterialProperty PhET-iO Element missing: buoyancy.exploreScreen.view.blockBPanel.comboBoxMaterialProperty PhET-iO Element missing: buoyancy.labScreen.view.blockPanel.comboBoxMaterialProperty PhET-iO Element missing: buoyancy.shapesScreen.view.materialControlNode.comboBoxMaterialProperty PhET-iO Element missing: buoyancy.labScreen.view.blockPanel.comboBox.listBox.customItem // Sounds like a bug to me!! buoyancy.applicationsScreen.view.blockControls.massNumberControl.highDensityMassNumberControl.visibleProperty._data.initialState differs. Expected: {"units":null,"validValues":null,"value":false} actual: {"value":true,"validValues":null,"units":null} // Investigation needed PhET-iO Element missing: buoyancy.applicationsScreen.view.bottleControls.materialInsideControls.customBottleDensityNumberControl.numberDisplay.valueText.stringProperty.dependencies // Added back in a phetioLinkedProperty PhET-iO Element missing: buoyancy.applicationsScreen.view.bottleControls.materialInsideControls.customBottleDensityNumberControl.slider.valueProperty // We do not have GravityIO or MaterialIO anymore. Type missing: ArrayIO Type missing: ArrayIO Type missing: FunctionIO(GravityIO)=>VoidIO Type missing: FunctionIO(GravityIO,NullableIO)=>VoidIO Type missing: FunctionIO(MaterialIO)=>VoidIO Type missing: FunctionIO(MaterialIO,NullableIO)=>VoidIO Type missing: GravityIO Type missing: MaterialIO Type missing: NullableIO> Type missing: NullableIO> Type missing: NullableIO Type missing: NullableIO Type missing: NullableIO>> Type missing: PropertyIO Type missing: PropertyIO Type missing: ReferenceIO> Type missing: ReferenceIO> ```
zepumph commented 1 month ago

I fixed everything, except one that I added a TODO to investigate over in https://github.com/phetsims/density-buoyancy-common/issues/256.

I do still think this would be good to do in B:B and Density, but I don't have time to do this now.

zepumph commented 1 month ago

I believe that the above work fixed ~10 bugs from previously designed phet-io APi.

zepumph commented 1 month ago

We already handled this for density by fixing the migration wrapper over in #274.

zepumph commented 1 month ago

I tool another glance through buoyancy-basics, and all is good.