From 7d5e5f679917995d6ff6d2d0b146e48b0be7aeda Mon Sep 17 00:00:00 2001
From: Sam Reid <reids@colorado.edu>
Date: Fri, 12 Jul 2024 15:52:01 -0600
Subject: [PATCH] Fix useDensityControlInsteadOfMassControl, see
 https://github.com/phetsims/density-buoyancy-common/issues/256

---
 .../BuoyancyApplicationsScreenView.ts         |  6 +++-
 js/common/view/BlockControlNode.ts            | 32 ++++++++++++++++---
 js/common/view/MaterialControlNode.ts         | 25 +++++++++------
 3 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/js/buoyancy/view/applications/BuoyancyApplicationsScreenView.ts b/js/buoyancy/view/applications/BuoyancyApplicationsScreenView.ts
index 02a24bed..58581bfc 100644
--- a/js/buoyancy/view/applications/BuoyancyApplicationsScreenView.ts
+++ b/js/buoyancy/view/applications/BuoyancyApplicationsScreenView.ts
@@ -137,7 +137,11 @@ export default class BuoyancyApplicationsScreenView extends BuoyancyScreenView<B
       showMassAsReadout: true,
       supportHiddenMaterial: true,
       customKeepsConstantDensity: true,
-      tandem: materialInsideControlsTandem
+      tandem: materialInsideControlsTandem,
+
+      // When controlling the material inside, the custom density is an independent variable and should not automatically
+      // sync with the previously selected material's density.
+      syncCustomMaterialDensity: false
     } );
 
     // This DerivedProperty doesn't need disposal, since everything here lives for the lifetime of the simulation
diff --git a/js/common/view/BlockControlNode.ts b/js/common/view/BlockControlNode.ts
index 90a45650..6fec5f1b 100644
--- a/js/common/view/BlockControlNode.ts
+++ b/js/common/view/BlockControlNode.ts
@@ -19,7 +19,6 @@ import { combineOptions } from '../../../../phet-core/js/optionize.js';
 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.
@@ -36,6 +35,13 @@ export default class BlockControlNode extends MaterialMassVolumeControlNode {
       ...options.mysteryMaterials
     ];
 
+    // If we have useDensityControlInsteadOfMassControl, we control the logic completely here, and hence will shut off
+    // that one-way synchronization in the super.
+    // TODO: Use optionize, see https://github.com/phetsims/density-buoyancy-common/issues/256
+    if ( options.useDensityControlInsteadOfMassControl ) {
+      options.syncCustomMaterialDensity = false;
+    }
+
     super( cuboid.materialProperty, cuboid.massProperty, cuboid.volumeProperty, materials,
       cubicMeters => cuboid.updateSize( Cube.boundsFromVolume( cubicMeters ) ), listParent, numberControlMassPropertyFeatured, options );
 
@@ -45,9 +51,27 @@ export default class BlockControlNode extends MaterialMassVolumeControlNode {
 
       const densityNumberControlTandem = options.tandem.createTandem( 'densityNumberControl' );
 
-      // TODO: Lots of work needed here, https://github.com/phetsims/density-buoyancy-common/issues/256
-      // TODO: Manually set material to custom when this property changes? https://github.com/phetsims/density-buoyancy-common/issues/256
-      const customDensityProperty = new NumberProperty( 1000, {} );
+      const customDensityProperty = cuboid.materialProperty.customMaterial.densityProperty;
+
+      let isChangingToPredefinedMaterialLock = false;
+
+      // When the user changes the density by dragging the slider, automatically switch from the predefined material to
+      // the custom material
+      customDensityProperty.lazyLink( () => {
+        if ( !isChangingToPredefinedMaterialLock ) {
+          cuboid.materialProperty.value = cuboid.materialProperty.customMaterial;
+        }
+      } );
+
+      // In the explore screen, when switching from custom to wood, change the density back to the wood density
+      cuboid.materialProperty.lazyLink( material => {
+        if ( !material.custom ) {
+
+          isChangingToPredefinedMaterialLock = true;
+          customDensityProperty.value = material.density;
+          isChangingToPredefinedMaterialLock = false;
+        }
+      } );
 
       const densityAsLitersProperty = new UnitConversionProperty( customDensityProperty, {
         factor: 1 / DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER
diff --git a/js/common/view/MaterialControlNode.ts b/js/common/view/MaterialControlNode.ts
index c236eeed..f9509876 100644
--- a/js/common/view/MaterialControlNode.ts
+++ b/js/common/view/MaterialControlNode.ts
@@ -16,12 +16,14 @@ import DensityBuoyancyCommonConstants from '../DensityBuoyancyCommonConstants.js
 import Material from '../model/Material.js';
 import { PhetioObjectOptions } from '../../../../tandem/js/PhetioObject.js';
 import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
-import Utils from '../../../../dot/js/Utils.js';
 import densityBuoyancyCommon from '../../densityBuoyancyCommon.js';
 import MaterialProperty from '../model/MaterialProperty.js';
+import Utils from '../../../../dot/js/Utils.js';
 
 type SelfMaterialControlNodeOptions = {
 
+  syncCustomMaterialDensity?: boolean;
+
   // A label, if provided to be placed to the right of the ComboBox
   labelNode?: Node | null;
 
@@ -51,6 +53,7 @@ export default class MaterialControlNode extends VBox {
 
 
     const options = optionize<MaterialControlNodeOptions, SelfMaterialControlNodeOptions, VBoxOptions>()( {
+      syncCustomMaterialDensity: true,
       labelNode: null,
       supportCustomMaterial: true,
       supportHiddenMaterial: false,
@@ -95,15 +98,17 @@ export default class MaterialControlNode extends VBox {
     };
 
     // When switching to custom, set the custom density to the previous material's density (clamped just in case)
-    materialProperty.lazyLink( ( material, oldMaterial ) => {
-      if ( material.custom ) {
-        assert && assert( materialProperty.customMaterial === customMaterials[ 0 ], 'I would really rather know what customMaterial we are dealing with' );
-
-        // Handle our minimum volume if we're switched to custom (if needed)
-        const maxVolume = Math.max( volumeProperty.value, options.minCustomVolumeLiters / DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER );
-        materialProperty.customMaterial.densityProperty.value = Utils.clamp( oldMaterial.density, options.minCustomMass / maxVolume, options.maxCustomMass / maxVolume );
-      }
-    } );
+    if ( options.syncCustomMaterialDensity ) {
+      materialProperty.lazyLink( ( material, oldMaterial ) => {
+        if ( material.custom ) {
+          assert && assert( materialProperty.customMaterial === customMaterials[ 0 ], 'I would really rather know what customMaterial we are dealing with' );
+
+          // Handle our minimum volume if we're switched to custom (if needed)
+          const maxVolume = Math.max( volumeProperty.value, options.minCustomVolumeLiters / DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER );
+          materialProperty.customMaterial.densityProperty.value = Utils.clamp( oldMaterial.density, options.minCustomMass / maxVolume, options.maxCustomMass / maxVolume );
+        }
+      } );
+    }
 
     // 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