Skip to content

Commit

Permalink
fix some TODOs, #270
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Kauzmann <[email protected]>
  • Loading branch information
zepumph committed Jul 24, 2024
1 parent 1804cf8 commit 8db5e2c
Showing 1 changed file with 10 additions and 9 deletions.
19 changes: 10 additions & 9 deletions js/common/model/CompareBlockSetModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ export type CompareBlockSetModelOptions = SelfOptions & StrictOmit<ParentOptions
// Filled in later, and so not needed by the optionize call
type OptionizeParent = StrictOmit<ParentOptions, 'createMassesCallback'>;

type StrictCubeOptionsNoAvailableMaterials = StrictOmit<StrictCubeOptions, 'availableMassMaterials'>;

export default class CompareBlockSetModel extends BlockSetModel<BlockSet> {

// TODO: Helpful documentation please, see https://github.com/phetsims/density-buoyancy-common/issues/273
// Properties that control all blocks within the block set named for it massProperty -> "sameMass".
public readonly massProperty: NumberProperty;
public readonly volumeProperty: NumberProperty;

// TODO: When are these properties used? How do they relate to the custom or non-custom material values? see https://github.com/phetsims/density-buoyancy-common/issues/273
public readonly densityProperty: NumberProperty;

public constructor( providedOptions: CompareBlockSetModelOptions ) {
Expand Down Expand Up @@ -132,12 +132,11 @@ export default class CompareBlockSetModel extends BlockSetModel<BlockSet> {
units: 'kg/m^3'
} );

// 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 );
const getCubeOptions = ( cubeOptions: StrictCubeOptionsNoAvailableMaterials ) => combineOptions<StrictCubeOptionsNoAvailableMaterials>( {}, options.sharedCubeOptions, cubeOptions );

// TODO: Helpful documentation please, see https://github.com/phetsims/density-buoyancy-common/issues/273
// Create one mass for each cubeData/blockSet combo, based on the provided blockSet
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.
return blockSet === BlockSet.SAME_MASS ?
Expand Down Expand Up @@ -188,7 +187,9 @@ export default class CompareBlockSetModel extends BlockSetModel<BlockSet> {
startingMass, options.sameDensityValue,
cubeData.colorProperty, densityProperty.hasChangedProperty, options.initialMaterials,
combineOptions<StrictCubeOptions>( {
customMaterialOptions: { densityRange: options.sameDensityRange } // TODO: totally unsure, https://github.com/phetsims/density-buoyancy-common/issues/273
customMaterialOptions: {
densityRange: options.sameDensityRange
}
}, getCubeOptions( cubeData.sameDensityCubeOptions ) ) );

// Keep this block's density in sync with the controlling densityProperty when it changes.
Expand Down Expand Up @@ -250,7 +251,7 @@ export default class CompareBlockSetModel extends BlockSetModel<BlockSet> {
baseColorProperty: ColorProperty,
blockSetValueChangedProperty: TReadOnlyProperty<boolean>,
initialMaterials: Material[],
providedOptions?: StrictCubeOptions ): Cube {
providedOptions?: StrictCubeOptionsNoAvailableMaterials ): Cube {

const densityAdjustedColorProperty = new ColorProperty( baseColorProperty.value );

Expand Down

0 comments on commit 8db5e2c

Please sign in to comment.