Skip to content

Commit

Permalink
Change REVIEW to TODO, see #123 and #227
Browse files Browse the repository at this point in the history
  • Loading branch information
samreid committed Jul 31, 2024
1 parent 57995fc commit 485458d
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 10 deletions.
2 changes: 1 addition & 1 deletion js/buoyancy/model/BuoyancyExploreModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export default class BuoyancyExploreModel extends DensityBuoyancyModel {
this.massB = Cube.createWithMass( this.engine, Material.ALUMINUM, new Vector2( 0.05, 0.35 ), 13.5, {
tag: MassTag.OBJECT_B,

// REVIEW: Tandem doesn't match variable name
// TODO: Tandem doesn't match variable name, see https://github.com/phetsims/density-buoyancy-common/issues/123
tandem: blocksTandem.createTandem( 'blockB' ),
availableMassMaterials: availableMassMaterials,
visible: false
Expand Down
4 changes: 2 additions & 2 deletions js/buoyancy/model/applications/BoatDesign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* compute a shape (for a given block size) such that in 2D it "acts" as the proper 3d shape. This is mainly due to
* the block corners pressing against the inside of the boat's hull.
*
* REVIEW: https://github.com/phetsims/density-buoyancy-common/issues/123 This file is > 800 lines of complicated code
* TODO: https://github.com/phetsims/density-buoyancy-common/issues/123 This file is > 800 lines of complicated code
* with many magic numbers. We should make an effort to remove unused parts, and clean up where possible.
*
* @author Jonathan Olson (PhET Interactive Simulations)
Expand Down Expand Up @@ -696,7 +696,7 @@ BoatDesign.ONE_LITER_HULL_VOLUME = BoatDesign.DESIGN_HULL_VOLUME * BoatDesign.ON

/**
* Replaces the main page with a debug view of the bottle, for debugging various curves and properties.
* REVIEW: https://github.com/phetsims/density-buoyancy-common/issues/123 this is unused and also says "bottle"
* TODO: https://github.com/phetsims/density-buoyancy-common/issues/123 this is unused and also says "bottle"
*
* phet.densityBuoyancyCommon.BoatDesign.getDebugCanvas()
*/
Expand Down
2 changes: 1 addition & 1 deletion js/buoyancy/model/applications/Bottle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
* the tip to the saddle point (second curve).
* - Saddle point: The true mathematical saddle where the saddle/tip curves meet back up, at r=0
*
* REVIEW: https://github.com/phetsims/density-buoyancy-common/issues/123 This file is > 1200 lines of complicated code
* TODO: https://github.com/phetsims/density-buoyancy-common/issues/123 This file is > 1200 lines of complicated code
* with many magic numbers. We should make an effort to remove unused parts, and clean up where possible.
*
* Diagram in x,r coordinates, showing the "profile" (exactly half of the bottle, and the curves that the
Expand Down
8 changes: 4 additions & 4 deletions js/buoyancy/model/applications/BuoyancyApplicationsModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,15 @@ export default class BuoyancyApplicationsModel extends DensityBuoyancyModel {
canMove: false,
inputEnabledPropertyOptions: {

// REVIEW: Why can the input enabled be turned on? Is this adding a phet-io feature that makes the scale movable?
// TODO: Why can the input enabled be turned on? Is this adding a phet-io feature that makes the scale movable? see , see https://github.com/phetsims/density-buoyancy-common/issues/123
// If so, is changing the inputEnabledProperty to true the only thing that has to happen to make it work?
phetioReadOnly: false
}
} );
this.availableMasses.push( this.scale );

// Adjust pool volume so that it's at the desired value WITH the pool scales inside.
// REVIEW: How does this relate to https://github.com/phetsims/density-buoyancy-common/blob/4038cb05c2b5c2b8b1f600bfbcf0a7eaac4617a2/js/common/model/DensityBuoyancyModel.ts#L435-L437
// TODO: How does this relate to https://github.com/phetsims/density-buoyancy-common/blob/4038cb05c2b5c2b8b1f600bfbcf0a7eaac4617a2/js/common/model/DensityBuoyancyModel.ts#L435-L437, see , see https://github.com/phetsims/density-buoyancy-common/issues/123
this.pool.fluidVolumeProperty.setInitialValue( this.pool.fluidVolumeProperty.value );

// This instance lives for the lifetime of the simulation, so we don't need to remove this listener
Expand Down Expand Up @@ -165,7 +165,7 @@ export default class BuoyancyApplicationsModel extends DensityBuoyancyModel {
this.boat.resetPosition();
this.block.resetPosition();

// REVIEW: Can we call boat.reset() here?
// TODO: Can we call boat.reset() here? See , see https://github.com/phetsims/density-buoyancy-common/issues/123
this.boat.verticalAcceleration = 0;
this.boat.verticalVelocity = 0;

Expand All @@ -177,7 +177,7 @@ export default class BuoyancyApplicationsModel extends DensityBuoyancyModel {
*/
public override reset(): void {

// REVIEW: Can we call resetBoatScene from reset?
// TODO: Can we call resetBoatScene from reset? See , see https://github.com/phetsims/density-buoyancy-common/issues/123

this.bottle.reset();
this.block.reset();
Expand Down
2 changes: 1 addition & 1 deletion js/common/model/BlendedVector2Property.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export default class BlendedVector2Property extends Property<Vector2> {
// This adds a hysteresis effect to the readout, which reduces flickering inherent to the model.
const MIN_BLEND = 0.1; // When close, blend with the old value more

// REVIEW: Does this MAX_BLEND differ from BlendedNumberProperty max on purpose? If so, document.
// TODO: Does this MAX_BLEND differ from BlendedNumberProperty max on purpose? If so, document. See , see https://github.com/phetsims/density-buoyancy-common/issues/123
const MAX_BLEND = 0.7; // When far apart, take the new value completely
const blendAmount = Utils.clamp(
Utils.linear( 0, 1, MIN_BLEND, MAX_BLEND, newValue.minus( oldValue ).magnitude ),
Expand Down
2 changes: 1 addition & 1 deletion js/common/model/Mass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ export default abstract class Mass extends PhetioObject {

/**
* Given a list of values and a ratio from 0 (the start) to 1 (the end), return an interpolated value.
* REVIEW: See if this and other occurrences should use dot piecewise linear functions.
* TODO: See if this and other occurrences should use dot piecewise linear functions, see https://github.com/phetsims/density-buoyancy-common/issues/123
*/
protected static evaluatePiecewiseLinear( values: number[], ratio: number ): number {
const logicalIndex = ratio * ( values.length - 1 );
Expand Down

0 comments on commit 485458d

Please sign in to comment.