samreid
commented
2 weeks ago From collaboration with @zepumph
```diff
Subject: [PATCH] Require providedOptions in optionize, see https://github.com/phetsims/phet-core/issues/134
---
Index: js/common/view/MaterialControlNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/MaterialControlNode.ts b/js/common/view/MaterialControlNode.ts
--- a/js/common/view/MaterialControlNode.ts (revision 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/common/view/MaterialControlNode.ts (date 1720637112824)
@@ -8,13 +8,10 @@
* @author Jonathan Olson (PhET Interactive Simulations)
*/
-import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
import Property from '../../../../axon/js/Property.js';
-import Utils from '../../../../dot/js/Utils.js';
import optionize from '../../../../phet-core/js/optionize.js';
-import { HBox, Node, Text, VBox, VBoxOptions } from '../../../../scenery/js/imports.js';
+import { Color, ColorProperty, HBox, Node, Text, VBox, VBoxOptions } from '../../../../scenery/js/imports.js';
import ComboBox from '../../../../sun/js/ComboBox.js';
-import StringIO from '../../../../tandem/js/types/StringIO.js';
import densityBuoyancyCommon from '../../densityBuoyancyCommon.js';
import DensityBuoyancyCommonStrings from '../../DensityBuoyancyCommonStrings.js';
import DensityBuoyancyCommonConstants from '../DensityBuoyancyCommonConstants.js';
@@ -82,33 +79,6 @@
const materialNames: MaterialName[] = [ ...materials.map( material => material.identifier ) ];
options.supportCustomMaterial && materialNames.push( 'CUSTOM' );
- // TODO: https://github.com/phetsims/density-buoyancy-common/issues/163 re-evaluate eliminating or moving this code
- // after addressing #163
- const comboBoxMaterialProperty = new DynamicProperty( new Property( materialProperty ), {
- bidirectional: true,
- map: ( material: Material ) => material.identifier,
- inverseMap: ( materialName: MaterialName ): Material => {
- if ( materialName === 'CUSTOM' ) {
- // Handle our minimum volume if we're switched to custom (if needed)
- const volume = Math.max( volumeProperty.value, options.minCustomVolumeLiters / DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER );
- return Material.createCustomSolidMaterial( {
- density: Utils.clamp( materialProperty.value.density, options.minCustomMass / volume, options.maxCustomMass / volume ),
- densityRange: this.customDensityRange
- } );
- }
- else {
- return Material.getMaterial( materialName );
- }
- },
- reentrant: true,
- phetioState: false,
- tandem: options.tandem.createTandem( 'comboBoxMaterialProperty' ),
- phetioFeatured: true,
- phetioDocumentation: 'Current material of the block. Changing the material will result in changes to the mass, but the volume will remain the same.',
- validValues: materialNames,
- phetioValueType: StringIO
- } );
-
const comboMaxWidth = 110;
const regularMaterials = materials.filter( material => !material.hidden );
@@ -116,7 +86,7 @@
const materialToItem = ( material: Material ) => {
return {
- value: material.identifier,
+ value: material,
createNode: () => new Text( material.nameProperty, {
font: DensityBuoyancyCommonConstants.COMBO_BOX_ITEM_FONT,
maxWidth: comboMaxWidth
@@ -125,12 +95,14 @@
a11yName: material.nameProperty
};
};
+ const customMaterial = Material.createCustomMaterial( {
+ customColor: new ColorProperty( new Color( 'green' ) )
+ } );
- // TODO: parametrize to MaterialName, https://github.com/phetsims/density-buoyancy-common/issues/176
- const comboBox = new ComboBox( comboBoxMaterialProperty, [
+ const comboBox = new ComboBox( materialProperty, [
...regularMaterials.map( materialToItem ),
...( options.supportCustomMaterial ? [ {
- value: 'CUSTOM',
+ value: customMaterial,
createNode: () => new Text( DensityBuoyancyCommonStrings.material.customStringProperty, {
font: DensityBuoyancyCommonConstants.COMBO_BOX_ITEM_FONT,
maxWidth: comboMaxWidth
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 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/buoyancy/view/applications/BuoyancyApplicationsScreenView.ts (date 1720649883713)
@@ -105,6 +105,11 @@
this.transformEmitter.addListener( this.positionResetSceneButton );
this.positionResetSceneButton();
+ const customMaterialInside = Material.createCustomSolidMaterial( {
+ density: 1000,
+ densityRange: new Range( 0, 1000 )
+ } );
+
const bottleControlsTandem = tandem.createTandem( 'bottleControls' );
const materialInsideControlsTandem = bottleControlsTandem.createTandem( 'materialInsideControls' );
const materialInsideControls = new MaterialMassVolumeControlNode( model.bottle.materialInsideProperty, model.bottle.interiorMassProperty, model.bottle.materialInsideVolumeProperty, [
@@ -138,23 +143,20 @@
let materialChangeLocked = false;
Multilink.lazyMultilink( [
- model.bottle.customDensityProperty,
- model.bottle.customDensityProperty.rangeProperty,
+ // model.bottle.customDensityProperty,
+ // model.bottle.customDensityProperty.rangeProperty,
model.bottle.interiorMassProperty,
customDensityControlVisibleProperty
], ( density, densityRange ) => {
if ( !materialChangeLocked && model.bottle.materialInsideProperty.value.custom ) {
materialChangeLocked = true;
- model.bottle.materialInsideProperty.value = Material.createCustomSolidMaterial( {
- density: density * DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER,
- densityRange: densityRange.times( DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER )
- } );
+ model.bottle.materialInsideProperty.value.densityProperty.value = density * DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER;
materialChangeLocked = false;
}
} );
const customBottleDensityControlTandem = materialInsideControlsTandem.createTandem( 'customBottleDensityNumberControl' );
- const customBottleDensityControl = new NumberControl( DensityBuoyancyCommonStrings.densityStringProperty, model.bottle.customDensityProperty, model.bottle.customDensityProperty.range, combineOptions( {
+ const customBottleDensityControl = new NumberControl( DensityBuoyancyCommonStrings.densityStringProperty, customMaterialInside.densityProperty, customMaterialInside.densityProperty.range, combineOptions( {
visibleProperty: new GatedVisibleProperty( customDensityControlVisibleProperty, customBottleDensityControlTandem ),
sliderOptions: {
accessibleName: DensityBuoyancyCommonStrings.densityStringProperty,
Index: js/common/view/MassLabelNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/MassLabelNode.ts b/js/common/view/MassLabelNode.ts
--- a/js/common/view/MassLabelNode.ts (revision 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/common/view/MassLabelNode.ts (date 1720650286242)
@@ -35,12 +35,14 @@
this.readoutStringProperty = new DerivedProperty(
[
mass.materialProperty,
+ mass.densityProperty,
mass.volumeProperty,
DensityBuoyancyCommonStrings.kilogramsPatternStringProperty,
DensityBuoyancyCommonStrings.questionMarkStringProperty
],
(
material,
+ density,
volume,
patternStringProperty,
questionMarkString
@@ -49,7 +51,7 @@
questionMarkString :
StringUtils.fillIn( patternStringProperty, {
// Deriving the mass instead of using massProperty to avoid including the contained mass, for the case of the boat
- kilograms: Utils.toFixed( volume * material.density, 2 ),
+ kilograms: Utils.toFixed( volume * density, 2 ),
decimalPlaces: 2
} );
} );
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 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/common/model/Mass.ts (date 1720650148617)
@@ -27,7 +27,7 @@
import IOType from '../../../../tandem/js/types/IOType.js';
import densityBuoyancyCommon from '../../densityBuoyancyCommon.js';
import InterpolatedProperty from './InterpolatedProperty.js';
-import Material, { CreateCustomMaterialOptions, MaterialName } from './Material.js';
+import Material from './Material.js';
import PhysicsEngine, { PhysicsEngineBody } from './PhysicsEngine.js';
import Basin from './Basin.js';
import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
@@ -41,7 +41,7 @@
import BlendedVector2Property from './BlendedVector2Property.js';
import { GuardedNumberProperty, GuardedNumberPropertyOptions } from './GuardedNumberProperty.js';
import DensityBuoyancyCommonConstants from '../DensityBuoyancyCommonConstants.js';
-import StringUnionProperty from '../../../../axon/js/StringUnionProperty.js';
+import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
// For the Buoyancy Shapes screen, but needed here because setRatios is included in each core type
// See https://github.com/phetsims/buoyancy/issues/29
@@ -77,7 +77,7 @@
massPropertyOptions?: NumberPropertyOptions;
// Accepted values for the Material Combo Box
- materialEnumPropertyValidValues?: MaterialName[];
+ materialValidValues?: Material[];
minVolume?: number;
maxVolume?: number;
@@ -119,10 +119,10 @@
public readonly materialProperty: Property;
// for phet-io support (to control the materialProperty)
- public readonly materialEnumProperty?: StringUnionProperty;
+ // public readonly materialEnumProperty?: StringUnionProperty;
// for phet-io support (to control the materialProperty)
- public readonly customDensityProperty?: NumberProperty;
+ // public readonly customDensityProperty?: NumberProperty;
// for phet-io support (to control the materialProperty)
private readonly customColorProperty?: Property;
@@ -195,6 +195,7 @@
public stepTop: number; // maximum y value of the mass
public readonly adjustableMaterial: boolean;
+ public readonly densityProperty: DynamicProperty;
protected constructor( engine: PhysicsEngine, providedOptions: MassOptions ) {
@@ -213,16 +214,22 @@
volumePropertyOptions: {},
massPropertyOptions: {},
- materialEnumPropertyValidValues: [
- 'ALUMINUM',
- 'BRICK',
- 'COPPER',
- 'ICE',
- 'PLATINUM',
- 'STEEL',
- 'STYROFOAM',
- 'WOOD',
- 'CUSTOM'
+ // TODO: Make the default an empty list and move this list to density's mystery screen
+ // TODO: Should all masses be created with their valid materials immediately?
+ // TODO: Move the materialValidValues over to the view, and let each materialProperty be able to take on any material.
+ materialValidValues: [
+ Material.ALUMINUM,
+ Material.BRICK,
+ Material.COPPER,
+ Material.ICE,
+ Material.PLATINUM,
+ Material.STEEL,
+ Material.STYROFOAM,
+ Material.WOOD,
+ Material.createCustomSolidMaterial( {
+ density: 1000,
+ customColor: new ColorProperty( Color.white )
+ } )
],
minVolume: 0,
@@ -276,86 +283,91 @@
phetioFeatured: true
}, options.materialPropertyOptions ) );
+ this.densityProperty = new DynamicProperty( this.materialProperty, {
+ bidirectional: false,
+ derive: material => material.densityProperty
+ } );
+
this.adjustableMaterial = options.adjustableMaterial;
if ( options.adjustableMaterial ) {
- this.materialEnumProperty = new StringUnionProperty( options.material.identifier, {
- tandem: tandem?.createTandem( 'materialEnumProperty' ),
- validValues: options.materialEnumPropertyValidValues,
- phetioFeatured: true,
- phetioDocumentation: 'Current material of the object. Changing the material will result in changes to the mass, but the volume will remain the same.'
- } );
- this.customDensityProperty = new NumberProperty( options.material.density, {
- tandem: tandem?.createTandem( 'customDensityProperty' ),
- phetioFeatured: true,
- phetioDocumentation: 'Density of the object when the material is set to “CUSTOM”.',
- range: new Range( 150, 23000 ),
- units: 'kg/m^3'
- } );
- this.customColorProperty = new ColorProperty( options.material.customColor ? options.material.customColor.value : Color.WHITE, {
- tandem: options.adjustableColor ? tandem?.createTandem( 'customColorProperty' ) : Tandem.OPT_OUT,
- phetioFeatured: true
- } );
-
- this.materialProperty.addPhetioStateDependencies( [ this.materialEnumProperty, this.customDensityProperty, this.customColorProperty ] );
-
- // Hook up phet-io Properties for interoperation with the normal ones
- let enumLock = false;
- let densityLock = false;
- let colorLock = false;
- const colorListener = ( color: Color ) => {
- if ( !colorLock ) {
- colorLock = true;
- this.customColorProperty!.value = color;
- colorLock = false;
- }
- };
- this.materialProperty.link( ( material, oldMaterial ) => {
- if ( !enumLock ) {
- enumLock = true;
- this.materialEnumProperty!.value = material.identifier;
- enumLock = false;
- }
- if ( !densityLock ) {
- densityLock = true;
- this.customDensityProperty!.value = material.density;
- densityLock = false;
- }
- if ( oldMaterial && oldMaterial.customColor ) {
- oldMaterial.customColor.unlink( colorListener );
- }
- if ( material && material.customColor ) {
- material.customColor.link( colorListener );
- }
- } );
-
- Multilink.lazyMultilink( [ this.materialEnumProperty, this.customDensityProperty, this.customColorProperty ], materialEnum => {
- // See if it's an external change
- if ( !enumLock && !densityLock && !colorLock ) {
- enumLock = true;
- densityLock = true;
- colorLock = true;
- if ( materialEnum === 'CUSTOM' ) {
- const createMaterialOptions: CreateCustomMaterialOptions = {
- density: this.customDensityProperty!.value
- };
- if ( options.adjustableColor ) {
- createMaterialOptions.customColor = this.customColorProperty;
- }
- else {
- createMaterialOptions.densityRange = this.customDensityProperty!.range;
- }
-
- this.materialProperty.value = Material.createCustomSolidMaterial( createMaterialOptions );
- }
- else {
- this.materialProperty.value = Material.getMaterial( materialEnum );
- }
- enumLock = false;
- densityLock = false;
- colorLock = false;
- }
- } );
+ // this.materialEnumProperty = new StringUnionProperty( options.material.identifier, {
+ // tandem: tandem?.createTandem( 'materialEnumProperty' ),
+ // validValues: options.materialEnumPropertyValidValues,
+ // phetioFeatured: true,
+ // phetioDocumentation: 'Current material of the object. Changing the material will result in changes to the mass, but the volume will remain the same.'
+ // } );
+ // this.customDensityProperty = new NumberProperty( options.material.density, {
+ // tandem: tandem?.createTandem( 'customDensityProperty' ),
+ // phetioFeatured: true,
+ // phetioDocumentation: 'Density of the object when the material is set to “CUSTOM”.',
+ // range: new Range( 150, 23000 ),
+ // units: 'kg/m^3'
+ // } );
+ // this.customColorProperty = new ColorProperty( options.material.customColor ? options.material.customColor.value : Color.WHITE, {
+ // tandem: options.adjustableColor ? tandem?.createTandem( 'customColorProperty' ) : Tandem.OPT_OUT,
+ // phetioFeatured: true
+ // } );
+ //
+ // this.materialProperty.addPhetioStateDependencies( [ this.materialEnumProperty, this.customDensityProperty, this.customColorProperty ] );
+ //
+ // // Hook up phet-io Properties for interoperation with the normal ones
+ // let enumLock = false;
+ // let densityLock = false;
+ // let colorLock = false;
+ // const colorListener = ( color: Color ) => {
+ // if ( !colorLock ) {
+ // colorLock = true;
+ // this.customColorProperty!.value = color;
+ // colorLock = false;
+ // }
+ // };
+ // this.materialProperty.link( ( material, oldMaterial ) => {
+ // if ( !enumLock ) {
+ // enumLock = true;
+ // this.materialEnumProperty!.value = material.identifier;
+ // enumLock = false;
+ // }
+ // if ( !densityLock ) {
+ // densityLock = true;
+ // this.customDensityProperty!.value = material.density;
+ // densityLock = false;
+ // }
+ // if ( oldMaterial && oldMaterial.customColor ) {
+ // oldMaterial.customColor.unlink( colorListener );
+ // }
+ // if ( material && material.customColor ) {
+ // material.customColor.link( colorListener );
+ // }
+ // } );
+ //
+ // Multilink.lazyMultilink( [ this.materialEnumProperty, this.customDensityProperty, this.customColorProperty ], materialEnum => {
+ // // See if it's an external change
+ // if ( !enumLock && !densityLock && !colorLock ) {
+ // enumLock = true;
+ // densityLock = true;
+ // colorLock = true;
+ // if ( materialEnum === 'CUSTOM' ) {
+ // const createMaterialOptions: CreateCustomMaterialOptions = {
+ // density: this.customDensityProperty!.value
+ // };
+ // if ( options.adjustableColor ) {
+ // createMaterialOptions.customColor = this.customColorProperty;
+ // }
+ // else {
+ // createMaterialOptions.densityRange = this.customDensityProperty!.range;
+ // }
+ //
+ // this.materialProperty.value = Material.createCustomSolidMaterial( createMaterialOptions );
+ // }
+ // else {
+ // this.materialProperty.value = Material.getMaterial( materialEnum );
+ // }
+ // enumLock = false;
+ // densityLock = false;
+ // colorLock = false;
+ // }
+ // } );
}
this.volumeProperty = new NumberProperty( options.volume, combineOptions( {
@@ -404,11 +416,10 @@
}
}, options.massPropertyOptions ) );
- Multilink.multilink( [ this.materialProperty, this.volumeProperty, this.containedMassProperty ], ( material, volume, containedMass ) => {
+ Multilink.multilink( [ this.materialProperty, this.volumeProperty, this.containedMassProperty, this.densityProperty ], ( material, volume, containedMass, density ) => {
- const selfMass = Utils.roundToInterval( material.density * volume, DensityBuoyancyCommonConstants.TOLERANCE );
+ const selfMass = Utils.roundToInterval( density * volume, DensityBuoyancyCommonConstants.TOLERANCE );
this.massProperty.value = selfMass + containedMass;
-
} );
this.bodyOffsetProperty = new Vector2Property( Vector2.ZERO, {
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 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/common/model/CompareBlockSetModel.ts (date 1720649706872)
@@ -20,7 +20,6 @@
import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
import Material from './Material.js';
import Property from '../../../../axon/js/Property.js';
-import Tandem from '../../../../tandem/js/Tandem.js';
import Cube, { CubeOptions } from './Cube.js';
import merge from '../../../../phet-core/js/merge.js';
import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
@@ -159,8 +158,7 @@
// don't need to be removed.
return blockSet === BlockSet.SAME_MASS ?
cubesData.map( cubeData => {
- const cube = Cube.createWithMass( model.engine, cubeData.sameMassMaterialProperty.value, Vector2.ZERO,
- massProperty.value, cubeData.sameMassCubeOptions );
+ const cube = Cube.createWithMass( model.engine, cubeData.sameMassMaterialProperty.value, Vector2.ZERO, massProperty.value, cubeData.sameMassCubeOptions );
cubeData.sameMassMaterialProperty.link( material => cube.materialProperty.set( material ) );
massProperty.lazyLink( massValue => cubeData.sameMassDensityProperty.set( massValue / cube.volumeProperty.value ) );
@@ -243,36 +241,49 @@
*/
private static createMaterialProperty( colorProperty: TReadOnlyProperty, densityProperty: TReadOnlyProperty,
blockSetValueChangedProperty: TReadOnlyProperty, initialMaterials: Material[] ): TReadOnlyProperty {
- return new DerivedProperty( [ colorProperty, densityProperty, blockSetValueChangedProperty ],
- ( color, density, blockSetValueChanged ) => {
+
+ // Create and return a custom material with the modified color and density.
+ const myColorProperty = new DerivedProperty( [ colorProperty, densityProperty ], ( color, density ) => {
+ // myCustomMaterial.densityProperty.value = density;
+
+ // Calculate the lightness of the material based on its density.
+ const lightness = Material.getNormalizedLightness( densityProperty.value, COLOR_DENSITY_RANGE ); // 0-1
+
+ // Modify the color brightness based on the lightness.
+ const modifier = 0.1;
+ const rawValue = ( lightness * 2 - 1 ) * ( 1 - modifier ) + modifier;
+ const power = 0.7;
+
+ return colorProperty.value.colorUtilsBrightness( Math.sign( rawValue ) * Math.pow( Math.abs( rawValue ), power ) );
+ } );
+ const myCustomMaterial = Material.createCustomSolidMaterial( {
+ density: densityProperty.value,
+ customColor: myColorProperty
+ } );
+
+ // TODO: Pass the densityProperty directly into the custom one
+ densityProperty.link( density => {
+ myCustomMaterial.densityProperty.value = density;
+ } );
+ //
+ // // TODO: https://github.com/phetsims/density-buoyancy-common/issues/256 allow passing in the density property directly
+ // Multilink.multilink( [ colorProperty, densityProperty ], ( color, density ) => {
+ //
+ // } );
+
+ return new DerivedProperty( [ densityProperty, blockSetValueChangedProperty ], ( density, blockSetValueChanged ) => {
- // If the block set value has not changed, attempt to use an initial material with the same density.
- if ( !blockSetValueChanged ) {
- for ( let i = 0; i < initialMaterials.length; i++ ) {
- const material = initialMaterials[ i ];
- if ( material.density === density ) {
- return material;
- }
- }
- }
-
- // Calculate the lightness of the material based on its density.
- const lightness = Material.getNormalizedLightness( density, COLOR_DENSITY_RANGE ); // 0-1
-
- // Modify the color brightness based on the lightness.
- const modifier = 0.1;
- const rawValue = ( lightness * 2 - 1 ) * ( 1 - modifier ) + modifier;
- const power = 0.7;
- const modifiedColor = color.colorUtilsBrightness( Math.sign( rawValue ) * Math.pow( Math.abs( rawValue ), power ) );
-
- // Create and return a custom material with the modified color and density.
- return Material.createCustomSolidMaterial( {
- density: density,
- customColor: new Property( modifiedColor, { tandem: Tandem.OPT_OUT } )
- } );
- }, {
- tandem: Tandem.OPT_OUT
- } );
+ // If the block set value has not changed, attempt to use an initial material with the same density.
+ if ( !blockSetValueChanged ) {
+ for ( let i = 0; i < initialMaterials.length; i++ ) {
+ const material = initialMaterials[ i ];
+ if ( material.density === density ) {
+ return material;
+ }
+ }
+ }
+ return myCustomMaterial;
+ } );
}
}
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 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/buoyancy-basics/model/BuoyancyBasicsExploreModel.ts (date 1720637112855)
@@ -12,11 +12,12 @@
import Vector2 from '../../../../dot/js/Vector2.js';
import Cube from '../../common/model/Cube.js';
import DensityBuoyancyModel, { DensityBuoyancyModelOptions } from '../../common/model/DensityBuoyancyModel.js';
-import Material, { MaterialName } from '../../common/model/Material.js';
+import Material from '../../common/model/Material.js';
import Scale, { DisplayType } from '../../common/model/Scale.js';
import TwoBlockMode from '../../common/model/TwoBlockMode.js';
import densityBuoyancyCommon from '../../densityBuoyancyCommon.js';
import MassTag from '../../common/model/MassTag.js';
+import { Color, ColorProperty } from '../../../../scenery/js/imports.js';
type BuoyancyBasicsExploreModelOptions = DensityBuoyancyModelOptions;
@@ -38,12 +39,15 @@
} );
// B:B Explore has a more limited set of available materials.
- const simpleMaterialsIdentifiers: MaterialName[] = Material.SIMPLE_MASS_MATERIALS.map( x => x.identifier ).concat( [ 'CUSTOM' ] );
+ // const simpleMaterialsIdentifiers: MaterialName[] = Material.SIMPLE_MASS_MATERIALS.map( x => x.identifier ).concat( [ 'CUSTOM' ] );
this.massA = Cube.createWithMass( this.engine, Material.WOOD, new Vector2( -0.2, 0.2 ), 2, {
tag: MassTag.OBJECT_A,
adjustableMaterial: true,
- materialEnumPropertyValidValues: simpleMaterialsIdentifiers,
+ materialValidValues: Material.SIMPLE_MASS_MATERIALS.concat( [ Material.createCustomSolidMaterial( {
+ density: 1000,
+ customColor: new ColorProperty( Color.RED )
+ } ) ] ),
adjustableColor: false,
tandem: blocksTandem.createTandem( 'blockA' )
} );
@@ -51,7 +55,10 @@
this.massB = Cube.createWithMass( this.engine, Material.ALUMINUM, new Vector2( 0.05, 0.35 ), 13.5, {
tag: MassTag.OBJECT_B,
adjustableMaterial: true,
- materialEnumPropertyValidValues: simpleMaterialsIdentifiers,
+ materialValidValues: Material.SIMPLE_MASS_MATERIALS.concat( [ Material.createCustomSolidMaterial( {
+ density: 1000,
+ customColor: new ColorProperty( Color.RED )
+ } ) ] ),
adjustableColor: false,
tandem: blocksTandem.createTandem( 'blockB' ),
visible: false
Index: js/buoyancy/view/DensityAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buoyancy/view/DensityAccordionBox.ts b/js/buoyancy/view/DensityAccordionBox.ts
--- a/js/buoyancy/view/DensityAccordionBox.ts (revision 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/buoyancy/view/DensityAccordionBox.ts (date 1720649237948)
@@ -18,13 +18,11 @@
import DerivedStringProperty from '../../../../axon/js/DerivedStringProperty.js';
import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js';
-type DensityReadoutType = TReadOnlyProperty;
-
-type ParentOptions = ReadoutListAccordionBoxOptions;
+type ParentOptions = ReadoutListAccordionBoxOptions>;
type SelfOptions = EmptySelfOptions;
type DensityAccordionBoxOptions = SelfOptions & ParentOptions;
-export default class DensityAccordionBox extends ReadoutListAccordionBox {
+export default class DensityAccordionBox extends ReadoutListAccordionBox> {
public constructor( titleStringProperty: TReadOnlyProperty, providedOptions?: DensityAccordionBoxOptions ) {
@@ -36,7 +34,7 @@
super( titleStringProperty, options );
}
- public override generateReadoutData( materialProperty: DensityReadoutType ): ReadoutData {
+ public override generateReadoutData( materialProperty: TReadOnlyProperty ): ReadoutData {
// Use DynamicProperty so that this name is updated based on the material AND material's name changing.
const nameProperty = new DynamicProperty( materialProperty, {
@@ -48,7 +46,8 @@
[
materialProperty,
DensityBuoyancyCommonConstants.KILOGRAMS_PER_VOLUME_PATTERN_STRING_PROPERTY,
- DensityBuoyancyCommonStrings.questionMarkStringProperty
+ DensityBuoyancyCommonStrings.questionMarkStringProperty,
+ // myCustomProperty.densityProperty
],
( material, patternStringProperty, questionMarkString ) => {
return material.hidden ?
@@ -56,6 +55,9 @@
StringUtils.fillIn( patternStringProperty, {
// convert from kg/m^3 to kg/L
+ // TODO: This needs to update when custom material density changes
+ // Can do with a DynamicProperty that takes the value of density of the selected material
+ // Or with DerivedProperty.deriveAny across all the validValues which are Materials and then take their densityProperties
value: Utils.toFixed( material.density / DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER, 2 ),
decimalPlaces: 2
} );
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 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/common/model/Material.ts (date 1720649992807)
@@ -29,6 +29,7 @@
import Range from '../../../../dot/js/Range.js';
import WithRequired from '../../../../phet-core/js/types/WithRequired.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
+import NumberProperty from '../../../../axon/js/NumberProperty.js';
const NullableColorPropertyReferenceType = NullableIO( ReferenceIO( Property.PropertyIO( Color.ColorIO ) ) );
@@ -117,10 +118,10 @@
hidden?: boolean;
// Uses the color for a solid material's color
- customColor?: Property | null;
+ customColor?: ReadOnlyProperty | null;
// Uses the alpha channel for opacity
- liquidColor?: Property | null;
+ liquidColor?: ReadOnlyProperty | null;
// Used for the color of depth lines added on top of the Material
depthLinesColorProperty?: TReadOnlyProperty;
@@ -133,15 +134,15 @@
public readonly nameProperty: TReadOnlyProperty;
public readonly identifier: MaterialName;
public readonly tandemName: string | null;
- public readonly density: number; // in SI (kg/m^3)
public readonly viscosity: number;
// TODO: Eliminate custom as an orthogonal attribute, it can be determined from identifier. https://github.com/phetsims/density-buoyancy-common/issues/176
public readonly custom: boolean;
public readonly hidden: boolean;
- public readonly customColor: Property | null;
- public readonly liquidColor: Property | null;
+ public readonly customColor: ReadOnlyProperty | null;
+ public readonly liquidColor: ReadOnlyProperty | null;
public readonly depthLinesColorProperty: TReadOnlyProperty;
+ public readonly densityProperty: NumberProperty;
public constructor( providedOptions: MaterialOptions ) {
@@ -162,13 +163,24 @@
this.nameProperty = options.nameProperty;
this.identifier = options.identifier;
this.tandemName = options.tandemName;
- this.density = options.density;
+ this.densityProperty = new NumberProperty( options.density, {
+ // phetioFeatured: true,
+ // phetioDocumentation: 'Density of the object when the material is set to “CUSTOM”.',
+ range: new Range( 0, 23000 ),
+ units: 'kg/m^3'
+ } );
this.viscosity = options.viscosity;
this.custom = options.custom;
this.hidden = options.hidden;
this.customColor = options.customColor;
this.liquidColor = options.liquidColor;
this.depthLinesColorProperty = options.depthLinesColorProperty;
+
+ console.log( 'hello new material' );
+ }
+
+ public get density(): number {
+ return this.densityProperty.value;
}
/**
@@ -188,6 +200,9 @@
*/
public static createCustomLiquidMaterial( options: WithRequired ): Material {
return Material.createCustomMaterial( combineOptions( {
+
+ // TODO: Make sure to change the liquidColor when the density changes
+ // This can be done by moving more things into the Material constructor, so they can use the densityProperty
liquidColor: Material.getCustomLiquidColor( options.density, options.densityRange )
}, options ) );
}
@@ -199,7 +214,10 @@
assert && assert( options.hasOwnProperty( 'customColor' ) || options.hasOwnProperty( 'densityRange' ), 'we need a way to have a material color' );
+ // TODO: Make sure to change the solidColorProperty when the density changes
const solidColorProperty = options.customColor || Material.getCustomSolidColor( options.density, options.densityRange! );
+
+ // TODO: Make sure to change the depthLinesColorPropertyProperty when the density changes
const depthLinesColorPropertyProperty = new MappedProperty( solidColorProperty, {
map: solidColor => {
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 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/common/view/BlockControlNode.ts (date 1720637112852)
@@ -19,6 +19,7 @@
import PrecisionSliderThumb from './PrecisionSliderThumb.js';
import UnitConversionProperty from '../../../../axon/js/UnitConversionProperty.js';
import Range from '../../../../dot/js/Range.js';
+import NumberProperty from '../../../../axon/js/NumberProperty.js';
type SelfOptions = {
mysteryMaterials: Material[]; // Provide empty list to opt out.
@@ -41,19 +42,23 @@
assert && assert( cuboid.adjustableMaterial, 'useDensityControlInsteadOfMassControl should only be used with adjustable materials' );
const densityNumberControlTandem = options.tandem.createTandem( 'densityNumberControl' );
- const customDensityProperty = cuboid.customDensityProperty!;
+ // const customDensityProperty = cuboid.customDensityProperty!;
// TODO: See https://github.com/phetsims/density-buoyancy-common/issues/176
// customDensityProperty.lazyLink( () => {
// cuboid.materialEnumProperty!.value = 'CUSTOM';
// } );
+ const customDensityProperty = new NumberProperty( 1000, {} );
+
const densityAsLitersProperty = new UnitConversionProperty( customDensityProperty, {
factor: 1 / DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER
} );
- const densityNumberControl = new NumberControl( DensityBuoyancyCommonStrings.densityStringProperty,
- densityAsLitersProperty, new Range( customDensityProperty.range.min / DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER, 10 ),
+ const densityNumberControl = new NumberControl(
+ DensityBuoyancyCommonStrings.densityStringProperty,
+ densityAsLitersProperty,
+ new Range( customDensityProperty.range.min / DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER, 10 ),
combineOptions( {
sliderOptions: {
accessibleName: DensityBuoyancyCommonStrings.densityStringProperty,
Index: js/common/view/MaterialView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/MaterialView.ts b/js/common/view/MaterialView.ts
--- a/js/common/view/MaterialView.ts (revision 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/common/view/MaterialView.ts (date 1720646764416)
@@ -52,6 +52,7 @@
import Wood26_rgh_jpg from '../../../images/Wood26_rgh_jpg.js';
import densityBuoyancyCommon from '../../densityBuoyancyCommon.js';
import Material from '../model/Material.js';
+import ReadOnlyProperty from '../../../../axon/js/ReadOnlyProperty.js';
// MaterialView definition
@@ -339,10 +340,10 @@
export class ColoredMaterialView extends MaterialView {
- private readonly colorProperty: Property;
+ private readonly colorProperty: ReadOnlyProperty;
private readonly listener: ( color: Color ) => void;
- public constructor( colorProperty: Property ) {
+ public constructor( colorProperty: ReadOnlyProperty ) {
assert && assert( colorProperty !== null, 'colorProperty should not be null' );
Index: js/common/view/MaterialMassVolumeControlNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/MaterialMassVolumeControlNode.ts b/js/common/view/MaterialMassVolumeControlNode.ts
--- a/js/common/view/MaterialMassVolumeControlNode.ts (revision 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/common/view/MaterialMassVolumeControlNode.ts (date 1720637112850)
@@ -182,10 +182,7 @@
// If we're custom, adjust the density
if ( materialProperty.value.custom && !options.customKeepsConstantDensity ) {
- materialProperty.value = Material.createCustomSolidMaterial( {
- density: massProperty.value / cubicMeters,
- densityRange: this.customDensityRange
- } );
+ materialProperty.value.densityProperty.value = massProperty.value / cubicMeters;
}
setVolume( cubicMeters );
@@ -221,10 +218,11 @@
// It is possible for the volumeProperty to be 0, so avoid the infinite density case, see https://github.com/phetsims/density-buoyancy-common/issues/78
if ( materialProperty.value.custom && volumeProperty.value > 0 ) {
if ( !options.customKeepsConstantDensity ) { // Separate if statement, so we don't setVolume() below
- materialProperty.value = Material.createCustomSolidMaterial( {
- density: mass / volumeProperty.value,
- densityRange: this.customDensityRange
- } );
+ materialProperty.value.densityProperty.value = mass / volumeProperty.value;
+ // materialProperty.value = Material.createCustomSolidMaterial( {
+ // density: mass / volumeProperty.value,
+ // densityRange: this.customDensityRange
+ // } );
}
}
else {
Index: js/common/view/CuboidView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CuboidView.ts b/js/common/view/CuboidView.ts
--- a/js/common/view/CuboidView.ts (revision 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/common/view/CuboidView.ts (date 1720648238174)
@@ -95,6 +95,8 @@
const materialListener = ( material: Material ) => {
this.depthLinesNode.stroke = material.depthLinesColorProperty;
};
+
+ // TODO: Change the depth lines stroke when the
cuboid.materialProperty.link( materialListener );
this.disposeEmitter.addListener( () => {
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 90eda68564375b5ea129e761083ca1ed1d888f92)
+++ b/js/buoyancy/model/applications/Bottle.ts (date 1720637112834)
@@ -191,7 +191,7 @@
// In kg (kilograms)
public interiorMassProperty: ReadOnlyProperty;
- public override readonly customDensityProperty: NumberProperty;
+ // public override readonly customDensityProperty: NumberProperty;
public constructor( engine: PhysicsEngine, providedOptions: BottleOptions ) {
@@ -231,16 +231,15 @@
units: 'm^3'
} );
- // @ts-expect-error
- assert && assert( !this.customDensityProperty, 'There should not be a customDensityProperty on bottle before we create our internal one below' );
-
- this.customDensityProperty = new NumberProperty( 1, {
- range: new Range( 0.05, 20 ),
- tandem: materialInsideTandem.createTandem( 'customDensityProperty' ),
- phetioDocumentation: 'Density of the material inside the bottle when ‘CUSTOM’ is chosen.',
- phetioFeatured: true,
- units: 'kg/L'
- } );
+ // assert && assert( !this.customDensityProperty, 'There should not be a customDensityProperty on bottle before we create our internal one below' );
+ //
+ // this.customDensityProperty = new NumberProperty( 1, {
+ // range: new Range( 0.05, 20 ),
+ // tandem: materialInsideTandem.createTandem( 'customDensityProperty' ),
+ // phetioDocumentation: 'Density of the material inside the bottle when ‘CUSTOM’ is chosen.',
+ // phetioFeatured: true,
+ // units: 'kg/L'
+ // } );
this.interiorMassProperty = new DerivedProperty( [ this.materialInsideProperty, this.materialInsideVolumeProperty ], ( material, volume ) => {
return material.density * volume;
zepumph
AgustinVallejo
The simulation currently has multiple ways to manage the material of a solid or fluid. It is managed by a named enumeration plus a Material instance. In some, but not all cases, there is a 2-way binding. Custom materials have a separate outer Property to manage creating new Materials when the density changes. This has led to problems such as https://github.com/phetsims/density-buoyancy-common/issues/176, as well as https://github.com/phetsims/density-buoyancy-common/issues/250 as well as generally making the simulation very difficult to navigate, understand and maintain. We should attempt to preallocate all Material instances (including the custom ones) and make the density a densityProperty on the Materials in order to solve this family of problems and make the simulation much less complex in general.