updates to TODOs, #256 #270

Signed-off-by: Michael Kauzmann <[email protected]>

js/buoyancy/model/applications/Bottle.ts

@@ -183,9 +183,9 @@ export default class Bottle extends ApplicationsMass {
   // model-coordinate bounds in x,y
   private readonly bottleBounds: Bounds2;
 
-  // TODO: Sometimes when switching materials with the combo box, the volume maxes out to 10L, see https://github.com/phetsims/density-buoyancy-common/issues/256
   public readonly materialInsideProperty: MaterialProperty;
   public readonly materialInsideVolumeRange = new Range( 0, 10 );
+  // TODO: BUG: When switching to a mystery material for the bottle, why does the volume sometimes change to 10L?, see https://github.com/phetsims/density-buoyancy-common/issues/256
   public readonly materialInsideVolumeProperty: Property<number>; // m^3
 
   public readonly maxVolumeDisplacedProperty = new NumberProperty( ApplicationsMass.DEFAULT_DISPLACEMENT_VOLUME );

js/common/model/Gravity.ts

@@ -53,8 +53,7 @@ export default class Gravity extends PhetioObject implements HasValueProperty {
 
     // TODO: Make sure gravityValueProperty is reset, see https://github.com/phetsims/density-buoyancy-common/issues/267
     this.gravityValueProperty = new NumberProperty( options.value, {
-      // TODO: Instrumentation, see https://github.com/phetsims/density-buoyancy-common/issues/256
-      // tandem: options.tandem
+      tandem: options.tandem.createTandem( 'gravityValueProperty' )
     } );
     this.custom = options.custom;
     this.hidden = options.hidden;
@@ -92,8 +91,6 @@ export default class Gravity extends PhetioObject implements HasValueProperty {
     value: 19.6,
     hidden: true
   } );
-
-  // TODO: Search for other new IOType that can be deleted, see https://github.com/phetsims/density-buoyancy-common/issues/256
 }
 
 densityBuoyancyCommon.register( 'Gravity', Gravity );

js/common/model/MappedWrappedProperty.ts

@@ -30,13 +30,11 @@ export default abstract class MappedWrappedProperty<T extends HasValueProperty>
   protected constructor( initialValue: T, customValue: T, providedOptions?: PropertyOptions<T> ) {
     super( initialValue, providedOptions );
 
-    // TODO: phet-io editable density? https://github.com/phetsims/density-buoyancy-common/issues/256
     this.dynamicValueProperty = new DynamicProperty<number, number, T>( this, {
       bidirectional: false,
       derive: value => value.valueProperty
     } );
 
-    //  TODO: Why here? https://github.com/phetsims/density-buoyancy-common/issues/256
     this.customValue = customValue;
   }
 }

js/common/model/Mass.ts

@@ -206,10 +206,12 @@ export default abstract class Mass extends PhetioObject {
       volumePropertyOptions: {},
       massPropertyOptions: {},
 
-      // TODO: Make the default an empty list and move this list to density's mystery screen
+      /*
+        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.
-      // TODO: please note `materialPropertyOptions`. https://github.com/phetsims/density-buoyancy-common/issues/256
+      // TODO: please note `materialPropertyOptions`. https://github.com/phetsims/density-buoyancy-common/issues/270
+       */
       materialValidValues: [
         Material.ALUMINUM,
         Material.BRICK,
@@ -220,9 +222,10 @@ export default abstract class Mass extends PhetioObject {
         Material.STYROFOAM,
         Material.WOOD,
 
-        // TODO: Visit this tandem after moving elsewhere, see https://github.com/phetsims/density-buoyancy-common/issues/256
-        // TODO: MaterialProperty really wants to create its own custom instances, see https://github.com/phetsims/density-buoyancy-common/issues/256
-        new CustomSolidMaterial( providedOptions.tandem ? providedOptions.tandem.createTandem( 'customSolidMaterial' ) : Tandem.OPT_OUT, {
+        // TODO: Visit this tandem after moving elsewhere, see https://github.com/phetsims/density-buoyancy-common/issues/270
+        new CustomSolidMaterial( providedOptions.tandem ?
+                                 providedOptions.tandem.createTandem( 'customSolidMaterial' ) :
+                                 Tandem.OPT_OUT, {
           density: 1000,
           colorProperty: new ColorProperty( Color.white )
         } )
@@ -280,14 +283,12 @@ export default abstract class Mass extends PhetioObject {
       valueType: Material,
       reentrant: true,
 
-      // TODO: Is this correct? See // TODO: Is the aggregate custom material supposed to be solid or liquid? Or maybe it doesn't matter. It mainly affects the color. See https://github.com/phetsims/density-buoyancy-common/issues/256
-      tandem: ( tandem || Tandem.OPT_OUT ).createTandem( 'materialProperty' ),
+      // TODO: Is this correct? See https://github.com/phetsims/density-buoyancy-common/issues/256
+      tandem: tandem?.createTandem( 'materialProperty' ),
       phetioValueType: ReferenceIO( IOType.ObjectIO ),
       phetioFeatured: true
     }, options.materialPropertyOptions ) );
 
-    // TODO: In some cases, when the materialProperty changes to custom, we should set the custom density to be the value of the previous material, where/how should that happen? https://github.com/phetsims/density-buoyancy-common/issues/256
-
     this.volumeProperty = new NumberProperty( options.volume, combineOptions<NumberPropertyOptions>( {
       tandem: tandem?.createTandem( 'volumeProperty' ),
       phetioFeatured: true,
@@ -334,9 +335,11 @@ export default abstract class Mass extends PhetioObject {
       }
     }, options.massPropertyOptions ) );
 
-    // TODO: Think one more time about how all density changes must come back to change the mass (as mass is the only thing that p2 cares about). https://github.com/phetsims/density-buoyancy-common/issues/256
-    Multilink.multilink( [ this.materialProperty.densityProperty, this.volumeProperty, this.containedMassProperty ], ( density, volume, containedMass ) => {
-
+    Multilink.multilink( [
+      this.materialProperty.densityProperty,
+      this.volumeProperty,
+      this.containedMassProperty
+    ], ( density, volume, containedMass ) => {
       const selfMass = Utils.roundToInterval( density * volume, DensityBuoyancyCommonConstants.TOLERANCE );
       this.massProperty.value = selfMass + containedMass;
     } );

js/common/view/FluidDensityControlNode.ts

@@ -3,7 +3,7 @@
 /**
  * Shows a combined NumberControl/ComboBox for controlling fluid density.
  *
- * TODO: Changing the fluid density slider on Buoyancy/Explore does not change the actual fluid density. See https://github.com/phetsims/density-buoyancy-common/issues/256
+ * TODO: BUG: Changing the fluid density slider on Buoyancy/Explore does not change the actual fluid density (block doesn't move). See https://github.com/phetsims/density-buoyancy-common/issues/256
  *
  * @author Jonathan Olson (PhET Interactive Simulations)
  */

js/common/view/MaterialControlNode.ts

@@ -112,8 +112,8 @@ export default class MaterialControlNode extends VBox {
       } );
     }
 
-    // TODO: But can we just use the validValues of the provided MaterialProperty, https://github.com/phetsims/density-buoyancy-common/issues/256
-    // TODO: But hidden ones!!! https://github.com/phetsims/density-buoyancy-common/issues/256
+    // 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 ) : [] ),

js/density/view/DensityNumberLineNode.ts

@@ -135,7 +135,7 @@ export default class DensityNumberLineNode extends Node {
     background.localBounds = new Bounds2( 0, 0, options.width, options.height ).dilatedX( options.maxLabelWidth / 2 );
 
     const lineOptions = { stroke: 'black' };
-    // TODO: what if the density changes? https://github.com/phetsims/density-buoyancy-common/issues/256
+    // TODO: BUG: what if the density changes? https://github.com/phetsims/density-buoyancy-common/issues/256
     options.materials.forEach( ( material, index ) => {
       const x = this.modelViewTransform( material.density );
       const label = new Text( material.nameProperty, {