Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review API changes from staticMaterial work for regressions. #271

Closed
3 tasks done
samreid opened this issue Jul 18, 2024 · 7 comments
Closed
3 tasks done

Review API changes from staticMaterial work for regressions. #271

samreid opened this issue Jul 18, 2024 · 7 comments
Assignees

Comments

@samreid
Copy link
Member

samreid commented Jul 18, 2024

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

  • density
  • buoyancy
  • buoyancy-basics

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<GravityIO>
Type missing: ArrayIO<MaterialIO>
Type missing: FunctionIO(GravityIO)=>VoidIO
Type missing: FunctionIO(GravityIO,NullableIO<GravityIO>)=>VoidIO
Type missing: FunctionIO(MaterialIO)=>VoidIO
Type missing: FunctionIO(MaterialIO,NullableIO<MaterialIO>)=>VoidIO
Type missing: GravityIO
Type missing: MaterialIO
Type missing: NullableIO<ArrayIO<GravityIO>>
Type missing: NullableIO<ArrayIO<MaterialIO>>
Type missing: NullableIO<GravityIO>
Type missing: NullableIO<MaterialIO>
Type missing: NullableIO<ReferenceIO<PropertyIO<ColorIO>>>
Type missing: PropertyIO<GravityIO>
Type missing: PropertyIO<MaterialIO>
Type missing: ReferenceIO<PropertyIO<ColorIO>>
Type missing: ReferenceIO<PropertyIO<StringIO>>


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
Copy link
Member

zepumph commented Jul 18, 2024

Tagging #256.

@zepumph
Copy link
Member

zepumph commented Jul 18, 2024

Working on this now.

@zepumph
Copy link
Member

zepumph commented Jul 18, 2024

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<GravityIO>
Type missing: ArrayIO<MaterialIO>
Type missing: FunctionIO(GravityIO)=>VoidIO
Type missing: FunctionIO(GravityIO,NullableIO<GravityIO>)=>VoidIO
Type missing: FunctionIO(MaterialIO)=>VoidIO
Type missing: FunctionIO(MaterialIO,NullableIO<MaterialIO>)=>VoidIO
Type missing: GravityIO
Type missing: MaterialIO
Type missing: NullableIO<ArrayIO<GravityIO>>
Type missing: NullableIO<ArrayIO<MaterialIO>>
Type missing: NullableIO<GravityIO>
Type missing: NullableIO<MaterialIO>
Type missing: NullableIO<ReferenceIO<PropertyIO<ColorIO>>>
Type missing: PropertyIO<GravityIO>
Type missing: PropertyIO<MaterialIO>
Type missing: ReferenceIO<PropertyIO<ColorIO>>
Type missing: ReferenceIO<PropertyIO<StringIO>>


zepumph added a commit that referenced this issue Jul 18, 2024
@zepumph zepumph changed the title Review the API file changes Review API changes from staticMaterial work for regressions. Jul 18, 2024
@zepumph
Copy link
Member

zepumph commented Jul 18, 2024

I fixed everything, except one that I added a TODO to investigate over in #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
Copy link
Member

zepumph commented Jul 18, 2024

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

@zepumph
Copy link
Member

zepumph commented Jul 23, 2024

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

@zepumph
Copy link
Member

zepumph commented Jul 24, 2024

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

@zepumph zepumph closed this as completed Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants