Skip to content

Commit

Permalink
Fix useDensityControlInsteadOfMassControl, see #256
Browse files Browse the repository at this point in the history
  • Loading branch information
samreid committed Jul 12, 2024
1 parent 92a2e6f commit 7d5e5f6
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 28 additions & 4 deletions js/common/view/BlockControlNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 );

Expand All @@ -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
Expand Down
25 changes: 15 additions & 10 deletions js/common/view/MaterialControlNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -51,6 +53,7 @@ export default class MaterialControlNode extends VBox {


const options = optionize<MaterialControlNodeOptions, SelfMaterialControlNodeOptions, VBoxOptions>()( {
syncCustomMaterialDensity: true,
labelNode: null,
supportCustomMaterial: true,
supportHiddenMaterial: false,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 7d5e5f6

Please sign in to comment.