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

Provide MaterialProperty validValues during construction #270

Closed
zepumph opened this issue Jul 17, 2024 · 7 comments
Closed

Provide MaterialProperty validValues during construction #270

zepumph opened this issue Jul 17, 2024 · 7 comments

Comments

@zepumph
Copy link
Member

zepumph commented Jul 17, 2024

From #256, we are trying to decide what info belongs in the model, vs in the view. Right now, the list of items supported by the combo box are in the view. This has implications for PhET-iO, and we may want to try to tie the materials directly to the Property in Mass.ts. Let's investigate.

zepumph added a commit that referenced this issue Jul 17, 2024
Signed-off-by: Michael Kauzmann <[email protected]>
samreid added a commit that referenced this issue Jul 18, 2024
@samreid
Copy link
Member

samreid commented Jul 18, 2024

I observed that the materialValidValues was creating extraneous and confusing custom materials in the studio tree, and also that it was currently unused. I do not consider the commit great or complete, but just wanted to try it as a way to clear up the tree. Feel free to revert depending on how we proceed with this issue. But I also feel it is likely we may just want to adjust

// TODO: But can we just use the validValues of the provided MaterialProperty, https://github.com/phetsims/density-buoyancy-common/issues/270
// TODO: But hidden ones!!! https://github.com/phetsims/density-buoyancy-common/issues/270
const comboBox = new ComboBox( materialProperty, [
...regularMaterials.map( materialToItem ),
...( options.supportCustomMaterial ? customMaterials.map( materialToItem ) : [] ),
...mysteryMaterials.map( materialToItem )
], listParent, {
xMargin: 8,
yMargin: 4,
tandem: options.tandem.createTandem( 'comboBox' )
} );
and call it good. One unknown for me is I thought I heard of an issue where a PhET-iO client is supposed to be able to tap into materials that are not in the combo box. Is that accurate? And if so, does that mean they can add more items to the combo box? Or just set materials that are not showable in the combo box?

@zepumph
Copy link
Member Author

zepumph commented Jul 18, 2024

Noting here that our new system does have a bit of a customization overload. Since materialProperties are set as references to materials, it is easy in studio to set a blocks materialProperty to water. This doesn't fail validation, but everything downstream breaks (like the combo box, which doesn't have a water item for the block).

@zepumph
Copy link
Member Author

zepumph commented Jul 22, 2024

  • Compare screens on all three sims, materialProperty should be phetioReadOnly:true for the blocks
  • validValues should be provided to the blocks if there is a combo box controlling the material for the block. (B:lab/shapes/explore B:B:explore)
  • validValues should be provided to the pool's fluidMaterialProperty as well when there is a combo box for the the fluid. Note that density will only have water as the validValue (make phetioReadOnly:true if not too hard).
  • Same for gravity. We know the valid values for all cases.
  • For applications screen, validValues for bottle.materialInside.materialProperty and the block for the boat scene. The materialProperty for the boat and bottle themselves should be validValues.length=1 and phetioReadOnly:true (if not too hard).
  • For the Density Mystery screen, we want to have valid values, to keep the same behavior as adjustableMaterial materialEnumProperty in Density 1.1.

zepumph added a commit that referenced this issue Jul 23, 2024
zepumph added a commit that referenced this issue Jul 23, 2024
zepumph added a commit to phetsims/buoyancy-basics that referenced this issue Jul 23, 2024
zepumph added a commit to phetsims/density that referenced this issue Jul 23, 2024
zepumph added a commit to phetsims/buoyancy that referenced this issue Jul 23, 2024
zepumph added a commit that referenced this issue Jul 23, 2024
@zepumph
Copy link
Member Author

zepumph commented Jul 23, 2024

This is very close to a commit point. We just have a couple more spots that use BlockControlNode to transfer the simple mass materials+custom+mystery over to the blocks in the model.

Subject: [PATCH] provide validValues to all MappedWrappedProperties except Mass.MaterialProperty, 
https://github.com/phetsims/density-buoyancy-common/issues/270
---
Index: js/common/model/CompareBlockSetModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/CompareBlockSetModel.ts b/js/common/model/CompareBlockSetModel.ts
--- a/js/common/model/CompareBlockSetModel.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/common/model/CompareBlockSetModel.ts	(date 1721773878794)
@@ -109,7 +109,8 @@
       sharedCubeOptions: {
         materialPropertyOptions: {
           phetioReadOnly: true // See https://github.com/phetsims/density-buoyancy-common/issues/270#issuecomment-2243371397
-        }
+        },
+        availableMassMaterials: [] // TODO: 'CUSTOM' isn't right in here, instead we need to figure out the "side MaterialProperty" instances first. https://github.com/phetsims/density-buoyancy-common/issues/273
       },
 
       // BlockSetModel options
@@ -168,11 +169,11 @@
       }, cubeData );
     } );
 
-    const getCubeOptions = ( cubeOptions: StrictCubeOptions ) => combineOptions<CubeOptions>( {}, options.sharedCubeOptions, cubeOptions );
+    // TODO: We lost type checking for availableMassMaterials because of the `CubeOptions` type, https://github.com/phetsims/density-buoyancy-common/issues/273
+    const getCubeOptions = ( cubeOptions: StrictOmit<StrictCubeOptions, 'availableMassMaterials'> ) => combineOptions<CubeOptions>( {}, options.sharedCubeOptions, cubeOptions );
 
     // TODO: Helpful documentation please, see https://github.com/phetsims/density-buoyancy-common/issues/273
     const createMasses = ( model: BlockSetModel<BlockSet>, blockSet: BlockSet ) => {
-
       // TODO: Helpful documentation please, see https://github.com/phetsims/density-buoyancy-common/issues/273
       // In the following code, the cube instance persists for the lifetime of the simulation and the listeners
       // don't need to be removed.
@@ -263,6 +264,7 @@
    * Otherwise, it creates a custom material with a modified color based on the density.
    *
    * TODO: Should this return MaterialProperty and create its own customMaterial? see https://github.com/phetsims/density-buoyancy-common/issues/273
+   * TODO: Can we use initialMaterials as the availableMassMaterials for the blocks created for this blockSet? https://github.com/phetsims/density-buoyancy-common/issues/273
    */
   private static createMaterialProperty( tandem: Tandem, colorProperty: TReadOnlyProperty<Color>, densityProperty: TReadOnlyProperty<number>,
                                          blockSetValueChangedProperty: TReadOnlyProperty<boolean>, initialMaterials: Material[] ): TReadOnlyProperty<Material> {
Index: js/buoyancy/view/applications/BuoyancyApplicationsScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buoyancy/view/applications/BuoyancyApplicationsScreenView.ts b/js/buoyancy/view/applications/BuoyancyApplicationsScreenView.ts
--- a/js/buoyancy/view/applications/BuoyancyApplicationsScreenView.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/buoyancy/view/applications/BuoyancyApplicationsScreenView.ts	(date 1721763467757)
@@ -204,23 +204,13 @@
 
     const rightBottleContent = new Panel( bottleBox, DensityBuoyancyCommonConstants.PANEL_OPTIONS );
 
-    const blockControls = new MaterialMassVolumeControlNode( model.block.materialProperty, model.block.massProperty, model.block.volumeProperty, _.sortBy( [
-        Material.PYRITE,
-        Material.STEEL,
-        Material.SILVER,
-        Material.TANTALUM,
-        Material.GOLD,
-        Material.PLATINUM
-      ].concat( Material.SIMPLE_MASS_MATERIALS ),
-      material => material.density ).concat( [
-      // Adding custom/mystery Materials separately, so they aren't sorted by density
-      model.block.materialProperty.customMaterial,
-      Material.MATERIAL_V,
-      Material.MATERIAL_W
-    ] ), cubicMeters => model.block.updateSize( Cube.boundsFromVolume( cubicMeters ) ), this.popupLayer, true, {
-      tandem: tandem.createTandem( 'blockControls' ),
-      highDensityMaxMass: 215
-    } );
+
+
+    const blockControls = new MaterialMassVolumeControlNode( model.block.materialProperty, model.block.massProperty, model.block.volumeProperty,
+      model.block.materialProperty.availableValues, cubicMeters => model.block.updateSize( Cube.boundsFromVolume( cubicMeters ) ), this.popupLayer, true, {
+        tandem: tandem.createTandem( 'blockControls' ),
+        highDensityMaxMass: 215
+      } );
 
     model.block.materialProperty.link( material => {
       if ( material === Material.MATERIAL_V ) {
Index: js/common/view/ABControlsNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/ABControlsNode.ts b/js/common/view/ABControlsNode.ts
--- a/js/common/view/ABControlsNode.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/common/view/ABControlsNode.ts	(date 1721772910326)
@@ -18,7 +18,7 @@
 import Material from '../model/Material.js';
 
 type SelfOptions = {
-  mysteryMaterials?: Material[];
+  mysteryMaterials?: Material[]; // TODO: delete, https://github.com/phetsims/density-buoyancy-common/issues/270
 };
 
 export type ABControlsNodeOptions = SelfOptions & BlockControlNodeOptions & { tandem: Tandem };
Index: js/common/model/Material.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/Material.ts b/js/common/model/Material.ts
--- a/js/common/model/Material.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/common/model/Material.ts	(date 1721774298634)
@@ -473,6 +473,14 @@
     Material.ALUMINUM
   ];
 
+  // TODO: This is the exact list from Density 1.1 (adding in PVC), do we need to support this exactly for some reason? Or can it just be a more general list? https://github.com/phetsims/density-buoyancy-common/issues/270
+  public static readonly DENSITY_MYSTERY_PHET_IO_CUSTOMIZABLE_MATERIAL = [
+    ...Material.SIMPLE_MASS_MATERIALS,
+    Material.STEEL,
+    Material.COPPER,
+    Material.PLATINUM
+  ];
+
   public static readonly BUOYANCY_FLUID_MATERIALS = [
     Material.GASOLINE,
     Material.OIL,
Index: js/common/model/Cube.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/Cube.ts b/js/common/model/Cube.ts
--- a/js/common/model/Cube.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/common/model/Cube.ts	(date 1721773714844)
@@ -13,8 +13,8 @@
 import Cuboid, { CuboidOptions } from './Cuboid.js';
 import PhysicsEngine from './PhysicsEngine.js';
 import optionize, { combineOptions } from '../../../../phet-core/js/optionize.js';
-import Material from './Material.js';
 import Vector2 from '../../../../dot/js/Vector2.js';
+import { MaterialSchema } from './Mass.js';
 
 type SelfOptions = {
 
@@ -79,7 +79,7 @@
   /**
    * Creates a Cube with a defined volume
    */
-  public static createWithVolume( engine: PhysicsEngine, material: Material | 'CUSTOM', position: Vector2, volume: number, options?: StrictCubeOptions ): Cube {
+  public static createWithVolume( engine: PhysicsEngine, material: MaterialSchema, position: Vector2, volume: number, options?: StrictCubeOptions ): Cube {
     return new Cube( engine, volume, combineOptions<CubeOptions>( {
       matrix: Matrix3.translation( position.x, position.y ),
       minVolume: Cuboid.MIN_VOLUME,
@@ -91,7 +91,7 @@
   /**
    * Creates a Cube with a defined volume
    */
-  public static createWithMass( engine: PhysicsEngine, material: Material | 'CUSTOM', position: Vector2, mass: number, options?: StrictCubeOptions ): Cube {
+  public static createWithMass( engine: PhysicsEngine, material: MaterialSchema, position: Vector2, mass: number, options?: StrictCubeOptions ): Cube {
     let density: number;
     if ( material === 'CUSTOM' ) {
       assert && assert( options?.customMaterialOptions?.density, 'density needed to create with mass' );
Index: js/common/model/Scale.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/Scale.ts b/js/common/model/Scale.ts
--- a/js/common/model/Scale.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/common/model/Scale.ts	(date 1721772014420)
@@ -65,7 +65,9 @@
   displayType?: DisplayType;
 };
 
-export type ScaleOptions = SelfOptions & StrictOmit<InstrumentedMassOptions, 'body' | 'shape' | 'volume' | 'material' | 'massShape'> & PickOptional<InstrumentedMassOptions, 'body' | 'shape'>;
+export type ScaleOptions = SelfOptions & StrictOmit<InstrumentedMassOptions,
+  'body' | 'shape' | 'volume' | 'material' | 'massShape' | 'availableMassMaterials'> &
+  PickOptional<InstrumentedMassOptions, 'body' | 'shape'>;
 
 export default class Scale extends Mass {
 
@@ -89,6 +91,7 @@
 
       displayType: DisplayType.NEWTONS,
       material: Material.PLATINUM,
+      availableMassMaterials: [ Material.PLATINUM],
 
       accessibleName: 'Scale',
 
Index: js/common/view/BlockControlNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/BlockControlNode.ts b/js/common/view/BlockControlNode.ts
--- a/js/common/view/BlockControlNode.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/common/view/BlockControlNode.ts	(date 1721772910312)
@@ -21,7 +21,7 @@
 import Range from '../../../../dot/js/Range.js';
 
 type SelfOptions = {
-  mysteryMaterials: Material[]; // Provide empty list to opt out.
+  mysteryMaterials: Material[]; // Provide empty list to opt out. // TODO: delete, https://github.com/phetsims/density-buoyancy-common/issues/270
 };
 
 export type BlockControlNodeOptions = MaterialMassVolumeControlNodeOptions & SelfOptions;
@@ -29,11 +29,7 @@
 export default class BlockControlNode extends MaterialMassVolumeControlNode {
   public constructor( cuboid: Cuboid, listParent: Node, numberControlMassPropertyFeatured: boolean, options: BlockControlNodeOptions ) {
 
-    const materials = [
-      ...Material.SIMPLE_MASS_MATERIALS,
-      cuboid.materialProperty.customMaterial,
-      ...options.mysteryMaterials
-    ];
+    const materials = cuboid.materialProperty.availableValues;
 
     // If we have useDensityControlInsteadOfMassControl, we control the logic completely here, and hence do not want the  one-way synchronization in the super.
     if ( assert && options.useDensityControlInsteadOfMassControl ) {
Index: js/buoyancy/model/applications/BuoyancyApplicationsModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buoyancy/model/applications/BuoyancyApplicationsModel.ts b/js/buoyancy/model/applications/BuoyancyApplicationsModel.ts
--- a/js/buoyancy/model/applications/BuoyancyApplicationsModel.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/buoyancy/model/applications/BuoyancyApplicationsModel.ts	(date 1721771997563)
@@ -67,9 +67,27 @@
     } );
     this.availableMasses.push( this.bottle );
 
+    const sortedByDensity: ( Material | 'CUSTOM' )[] = _.sortBy( [
+        Material.PYRITE,
+        Material.STEEL,
+        Material.SILVER,
+        Material.TANTALUM,
+        Material.GOLD,
+        Material.PLATINUM
+      ].concat( Material.SIMPLE_MASS_MATERIALS ),
+
+      material => material.density );
+    const availableMassMaterials = sortedByDensity.concat( [
+      // Adding custom/mystery Materials separately, so they aren't sorted above by density
+      'CUSTOM',
+      Material.MATERIAL_V,
+      Material.MATERIAL_W
+    ] );
+
     this.block = Cube.createWithVolume( this.engine, Material.BRICK, new Vector2( -0.5, 0.3 ), 0.001, {
       visible: false,
-      tandem: objectsTandem.createTandem( 'block' )
+      tandem: objectsTandem.createTandem( 'block' ),
+      availableMassMaterials: availableMassMaterials
     } );
     this.availableMasses.push( this.block );
 
Index: js/buoyancy-basics/model/BuoyancyBasicsExploreModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buoyancy-basics/model/BuoyancyBasicsExploreModel.ts b/js/buoyancy-basics/model/BuoyancyBasicsExploreModel.ts
--- a/js/buoyancy-basics/model/BuoyancyBasicsExploreModel.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/buoyancy-basics/model/BuoyancyBasicsExploreModel.ts	(date 1721772976812)
@@ -17,6 +17,7 @@
 import TwoBlockMode from '../../common/model/TwoBlockMode.js';
 import densityBuoyancyCommon from '../../densityBuoyancyCommon.js';
 import MassTag from '../../common/model/MassTag.js';
+import { MaterialSchema } from '../../common/model/Mass.js';
 
 type BuoyancyBasicsExploreModelOptions = DensityBuoyancyModelOptions;
 
@@ -37,11 +38,17 @@
       phetioFeatured: true
     } );
 
+    const availableMassMaterials: MaterialSchema[] = [
+      ...Material.SIMPLE_MASS_MATERIALS,
+      'CUSTOM'
+    ];
+
     const blockATandem = blocksTandem.createTandem( 'blockA' );
     this.massA = Cube.createWithMass( this.engine, Material.WOOD, new Vector2( -0.2, 0.2 ), 2, {
       tag: MassTag.OBJECT_A,
       adjustableMaterial: true,
       adjustableColor: false,
+      availableMassMaterials: availableMassMaterials,
       tandem: blockATandem
     } );
     this.availableMasses.push( this.massA );
@@ -51,6 +58,7 @@
       tag: MassTag.OBJECT_B,
       adjustableMaterial: true,
       adjustableColor: false,
+      availableMassMaterials: availableMassMaterials,
       tandem: blockBTandem,
       visible: false
     } );
Index: js/buoyancy/model/applications/Bottle.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buoyancy/model/applications/Bottle.ts b/js/buoyancy/model/applications/Bottle.ts
--- a/js/buoyancy/model/applications/Bottle.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/buoyancy/model/applications/Bottle.ts	(date 1721772051655)
@@ -176,7 +176,8 @@
 // {Material}
 const BOTTLE_INITIAL_INTERIOR_MATERIAL = Material.WATER;
 
-export type BottleOptions = StrictOmit<ApplicationsMassOptions, 'body' | 'shape' | 'volume' | 'material' | 'massShape'>;
+export type BottleOptions = StrictOmit<ApplicationsMassOptions,
+  'body' | 'shape' | 'volume' | 'material' | 'massShape' | 'availableMassMaterials'>;
 
 export default class Bottle extends ApplicationsMass {
 
@@ -202,6 +203,7 @@
       shape: Shape.polygon( vertices ),
       volume: BOTTLE_VOLUME,
       material: 'CUSTOM',
+      availableMassMaterials: [ 'CUSTOM' ],
       materialPropertyOptions: {
         phetioReadOnly: true
       },
Index: js/common/model/Mass.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/Mass.ts b/js/common/model/Mass.ts
--- a/js/common/model/Mass.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/common/model/Mass.ts	(date 1721772976803)
@@ -49,14 +49,16 @@
 export const MASS_MIN_SHAPES_DIMENSION = 0.1; // 10cm => 1L square
 export const MASS_MAX_SHAPES_DIMENSION = Math.pow( 0.01, 1 / 3 ); // 10L square
 
+export type MaterialSchema = Material | 'CUSTOM';
 type SelfOptions = {
 
   // Required
   body: PhysicsEngineBody;
   shape: Shape;
 
-  // Use "CUSTOM" to tell the MaterialProperty to take the initial value from the internal customMaterial it creates.
-  material: Material | 'CUSTOM';
+  // Use closure to provide the customMaterial as the initial value to the MaterialProperty.
+  material: MaterialSchema; // TODO: ??? ( ( customMaterial: Material ) => Material ); https://github.com/phetsims/density-buoyancy-common/issues/270
+  availableMassMaterials: ( MaterialSchema )[]; // TODO: Required? https://github.com/phetsims/density-buoyancy-common/issues/270
   volume: number;
   massShape: MassShape;
 
@@ -67,10 +69,12 @@
 
   // Allow Customization of the material beyond initial value, this includes PhET-iO support for changing the density
   // and color, as well as support for some screens to adjust density instead of mass/volume as is most typical, see https://github.com/phetsims/density/issues/101
+  // TODO: Get outta here, https://github.com/phetsims/density-buoyancy-common/issues/270
   adjustableMaterial?: boolean;
 
   // Only used when adjustableMaterial:true. Set to true to support a PhET-iO instrumented Property to set the color
   // of the block. Set to false in order to calculate the color based on the current density + the density range.
+  // TODO: Get outta here, https://github.com/phetsims/density-buoyancy-common/issues/270
   adjustableColor?: boolean;
   tag?: MassTag;
   accessibleName?: PDOMValueType | null;
@@ -261,9 +265,7 @@
 
     const initialMaterial = options.material === 'CUSTOM' ? customSolidMaterial : options.material;
     this.materialProperty = new MaterialProperty( initialMaterial, customSolidMaterial,
-
-      // TODO: Do this part next, https://github.com/phetsims/density-buoyancy-common/issues/270
-      [],
+      options.availableMassMaterials.map( x => x === 'CUSTOM' ? customSolidMaterial : x ),
       options.materialPropertyOptions as MaterialPropertyOptions );
 
     this.volumeProperty = new NumberProperty( options.volume, combineOptions<NumberPropertyOptions>( {
Index: js/buoyancy/model/shapes/BuoyancyShapesModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buoyancy/model/shapes/BuoyancyShapesModel.ts b/js/buoyancy/model/shapes/BuoyancyShapesModel.ts
--- a/js/buoyancy/model/shapes/BuoyancyShapesModel.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/buoyancy/model/shapes/BuoyancyShapesModel.ts	(date 1721772312046)
@@ -42,6 +42,7 @@
   public readonly objectB: BuoyancyShapeModel;
 
   public readonly materialProperty: MaterialProperty;
+  private readonly availableMaterials: Material[];
 
   public constructor( options: BuoyancyShapesModelOptions ) {
 
@@ -54,11 +55,13 @@
       phetioFeatured: true
     } );
 
+    this.availableMaterials = Material.SIMPLE_MASS_MATERIALS;
+
     this.materialProperty = new MaterialProperty( Material.WOOD,
 
       // This hack is a way of saying, we do not create or support a custom material in this case.
       Material.WOOD,
-      Material.SIMPLE_MASS_MATERIALS, {
+      this.availableMaterials, {
         tandem: objectsTandem.createTandem( 'materialProperty' ),
         phetioValueType: ReferenceIO( IOType.ObjectIO )
       } );
@@ -116,6 +119,10 @@
   private createMass( tandem: Tandem, shape: MassShape, widthRatio: number, heightRatio: number, tag: MassTag ): Mass {
     const massOptions = {
       material: this.materialProperty.value,
+      availableMassMaterials: this.availableMaterials,
+      materialPropertyOptions: {
+        phetioReadOnly: true
+      },
       minVolume: 0.0002, // Cones have a smaller volume at min height/width
       maxVolume: Cuboid.MAX_VOLUME, // Cubes are the highest volume object in this screen
       tandem: tandem,
Index: js/density/model/DensityMysteryModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/density/model/DensityMysteryModel.ts b/js/density/model/DensityMysteryModel.ts
--- a/js/density/model/DensityMysteryModel.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/density/model/DensityMysteryModel.ts	(date 1721774407793)
@@ -30,6 +30,7 @@
 import Emitter from '../../../../axon/js/Emitter.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
 import Tandem from '../../../../tandem/js/Tandem.js';
+import { MaterialSchema } from '../../common/model/Mass.js';
 
 // constants
 const randomColors = [
@@ -77,7 +78,11 @@
 
     const commonCubeOptions = {
       adjustVolumeOnMassChanged: true,
-      adjustableMaterial: true
+      adjustableMaterial: true,
+      availableMassMaterials: [
+        ...Material.DENSITY_MYSTERY_PHET_IO_CUSTOMIZABLE_MATERIAL,
+        'CUSTOM'
+      ] satisfies MaterialSchema[]
     };
 
     let densities: number[];
@@ -382,16 +387,15 @@
               tandem: cubeTandem.createTandem( 'materialProperty' ).createTandem( 'customMaterial' ).createTandem( 'colorProperty' )
             } );
 
-            const cube = Cube.createWithVolume( model.engine, 'CUSTOM', Vector2.ZERO, mysteryVolumes[ i ], {
-              adjustVolumeOnMassChanged: true,
-              adjustableMaterial: true,
-              tag: tags[ i ],
-              tandem: cubeTandem,
-              customMaterialOptions: {
-                colorProperty: colorProperty,
-                density: densities[ i ]
-              }
-            } );
+            const cube = Cube.createWithVolume( model.engine, 'CUSTOM', Vector2.ZERO, mysteryVolumes[ i ],
+              combineOptions<CubeOptions>( {}, commonCubeOptions, {
+                tag: tags[ i ],
+                tandem: cubeTandem,
+                customMaterialOptions: {
+                  colorProperty: colorProperty,
+                  density: densities[ i ]
+                }
+              } ) );
 
             randomizeMaterialsEmitter.addListener( () => {
               cube.materialProperty.customMaterial.densityProperty.value = densities[ i ];
Index: js/buoyancy/model/applications/Boat.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buoyancy/model/applications/Boat.ts b/js/buoyancy/model/applications/Boat.ts
--- a/js/buoyancy/model/applications/Boat.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/buoyancy/model/applications/Boat.ts	(date 1721771997557)
@@ -28,7 +28,8 @@
 import NumberProperty from '../../../../../axon/js/NumberProperty.js';
 import Pool from '../../../common/model/Pool.js';
 
-export type BoatOptions = StrictOmit<ApplicationsMassOptions, 'body' | 'shape' | 'volume' | 'material' | 'massShape'>;
+export type BoatOptions = StrictOmit<ApplicationsMassOptions,
+  'body' | 'shape' | 'volume' | 'material' | 'massShape' | 'availableMassMaterials'>;
 
 export default class Boat extends ApplicationsMass {
 
@@ -65,6 +66,7 @@
       volume: volume,
       massShape: MassShape.BLOCK,
       material: Material.BOAT_BODY,
+      availableMassMaterials: [ Material.BOAT_BODY ],
 
       accessibleName: 'Boat'
     }, providedOptions );
Index: js/common/model/MaterialProperty.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/MaterialProperty.ts b/js/common/model/MaterialProperty.ts
--- a/js/common/model/MaterialProperty.ts	(revision af0f367ddbedb73ac9df1829392fb9ee16e0aa2b)
+++ b/js/common/model/MaterialProperty.ts	(date 1721771728153)
@@ -25,7 +25,7 @@
   public readonly customMaterial: Material;
 
   // Note the material could be the customMaterial
-  public constructor( material: Material, customMaterial: Material, availableMaterials: Material[], providedOptions: MaterialPropertyOptions ) {
+  public constructor( material: Material, customMaterial: Material, availableMaterials: (Material)[], providedOptions: MaterialPropertyOptions ) {
     super( material, customMaterial, availableMaterials, combineOptions<MaterialPropertyOptions>( {
       phetioFeatured: true
     }, providedOptions ) );

@AgustinVallejo AgustinVallejo self-assigned this Jul 24, 2024
zepumph added a commit that referenced this issue Jul 24, 2024
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/buoyancy that referenced this issue Jul 24, 2024
@zepumph
Copy link
Member Author

zepumph commented Jul 24, 2024

@AgustinVallejo and I got to a commit point above. Yay! There are a few more TODOs that @AgustinVallejo is kind enough to take the lead on. Thanks!

@zepumph zepumph removed their assignment Jul 24, 2024
zepumph added a commit that referenced this issue Jul 24, 2024
zepumph added a commit that referenced this issue Jul 24, 2024
Signed-off-by: Michael Kauzmann <[email protected]>
@AgustinVallejo
Copy link
Contributor

@zepumph will spot check and close hopefully :)

@zepumph
Copy link
Member Author

zepumph commented Jul 26, 2024

Looks great. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants