Skip to content

Commit

Permalink
Move TODOs, see #319
Browse files Browse the repository at this point in the history
  • Loading branch information
samreid committed Nov 16, 2024
1 parent 4bd25b8 commit 2338184
Show file tree
Hide file tree
Showing 11 changed files with 29 additions and 29 deletions.
2 changes: 1 addition & 1 deletion js/motion/MotionScreen.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2015-2023, University of Colorado Boulder

/**
* TODO https://github.com/phetsims/tasks/issues/1129
* TODO https://github.com/phetsims/forces-and-motion-basics/issues/319
* @author Sam Reid (PhET Interactive Simulations)
*/

Expand Down
4 changes: 2 additions & 2 deletions js/motion/model/Item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ export default class Item extends PhetioObject {
// the position of the item
public readonly positionProperty: Vector2Property;

// TODO: does this need to be instrumented for phet-io? https://github.com/phetsims/tasks/issues/1129
// TODO: does this need to be instrumented for phet-io? https://github.com/phetsims/forces-and-motion-basics/issues/319
public readonly pusherInsetProperty: Property<number>;

// whether the item is being dragged
public readonly draggingProperty: BooleanProperty;

// direction of the item, 'left'|'right'
// TODO: Why not an enum? https://github.com/phetsims/tasks/issues/1129
// TODO: Why not an enum? https://github.com/phetsims/forces-and-motion-basics/issues/319
public readonly directionProperty: StringProperty;

// tracks the animation state of the item
Expand Down
18 changes: 9 additions & 9 deletions js/motion/model/MotionModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,12 @@ export default class MotionModel {
private readonly movingRightProperty: BooleanProperty;

// 'right'|'left'|none, direction of movement of the stack of items
// TODO: Why not an enum? https://github.com/phetsims/tasks/issues/1129
// TODO: Why not an enum? https://github.com/phetsims/forces-and-motion-basics/issues/319
private readonly directionProperty: StringProperty;

// time since pusher has fallen over, in seconds
// TODO: Should we this have a tandem? It spams the data stream. https://github.com/phetsims/tasks/issues/1129
// TODO: Why is default value 10? https://github.com/phetsims/tasks/issues/1129
// TODO: Should we this have a tandem? It spams the data stream. https://github.com/phetsims/forces-and-motion-basics/issues/319
// TODO: Why is default value 10? https://github.com/phetsims/forces-and-motion-basics/issues/319
private readonly timeSinceFallenProperty: NumberProperty;

// whether the pusher has fallen over
Expand All @@ -110,11 +110,11 @@ export default class MotionModel {
public readonly fallenDirectionProperty: StringProperty;

// how long the simulation has been running
// TODO: Should we this have a tandem? It spams the data stream. https://github.com/phetsims/tasks/issues/1129
// TODO: Should we this have a tandem? It spams the data stream. https://github.com/phetsims/forces-and-motion-basics/issues/319
public readonly timeProperty: NumberProperty;

//stack.length is already a property, but mirror it here to easily multilink with it, see usage in MotionScreenView.js
//TODO: Perhaps a DerivedProperty would be more suitable instead of duplicating/synchronizing this value https://github.com/phetsims/tasks/issues/1129
//TODO: Perhaps a DerivedProperty would be more suitable instead of duplicating/synchronizing this value https://github.com/phetsims/forces-and-motion-basics/issues/319
public readonly stackSizeProperty: NumberProperty;

// is the sim running or paused?
Expand Down Expand Up @@ -235,13 +235,13 @@ export default class MotionModel {
// Keep track of whether the speed is classified as:
// 'RIGHT_SPEED_EXCEEDED', 'LEFT_SPEED_EXCEEDED' or 'WITHIN_ALLOWED_RANGE'
// so that the Applied Force can be stopped if the speed goes out of range.
// TODO: Why not an enum? https://github.com/phetsims/tasks/issues/1129
// TODO: Why not an enum? https://github.com/phetsims/forces-and-motion-basics/issues/319
this.speedClassificationProperty = new StringProperty( 'WITHIN_ALLOWED_RANGE', {
tandem: tandem.createTandem( 'speedClassificationProperty' )
} );

// See speedClassification
// TODO: Why not an enum? https://github.com/phetsims/tasks/issues/1129
// TODO: Why not an enum? https://github.com/phetsims/forces-and-motion-basics/issues/319
this.previousSpeedClassificationProperty = new StringProperty( 'WITHIN_ALLOWED_RANGE', {
tandem: tandem.createTandem( 'previousSpeedClassificationProperty' )
} );
Expand Down Expand Up @@ -285,7 +285,7 @@ export default class MotionModel {
//Zero out the applied force when the last object is removed. Necessary to remove the force applied with the slider tweaker buttons. See #37
this.stackObservableArray.lengthProperty.link( length => { if ( length === 0 ) { this.appliedForceProperty.set( 0 ); } } );

// TODO: Should stacksize Property be removed? https://github.com/phetsims/tasks/issues/1129
// TODO: Should stacksize Property be removed? https://github.com/phetsims/forces-and-motion-basics/issues/319
this.stackObservableArray.lengthProperty.link( length => {
this.stackSizeProperty.set( length );
} );
Expand Down Expand Up @@ -396,7 +396,7 @@ export default class MotionModel {
for ( let i = 0; i < this.stackObservableArray.length; i++ ) {
const size = this.view.getSize( this.stackObservableArray.get( i ) );
sumHeight += size.height;
this.stackObservableArray.get( i ).animateTo( this.view.layoutBounds.width / 2 - size.width / 2, ( this.skateboard ? 334 : 360 ) - sumHeight, 'stack' );//TODO: factor out this code for layout, which is duplicated in MotionTab.topOfStack https://github.com/phetsims/tasks/issues/1129
this.stackObservableArray.get( i ).animateTo( this.view.layoutBounds.width / 2 - size.width / 2, ( this.skateboard ? 334 : 360 ) - sumHeight, 'stack' );//TODO: factor out this code for layout, which is duplicated in MotionTab.topOfStack https://github.com/phetsims/forces-and-motion-basics/issues/319
}
}

Expand Down
2 changes: 1 addition & 1 deletion js/motion/view/ItemNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export default class ItemNode extends Node {
};

// Make sure the arms are updated (even if nothing else changed)
// TODO: It is possible that this can be removed once these issues are closed, see https://github.com/phetsims/tasks/issues/1129
// TODO: It is possible that this can be removed once these issues are closed, see https://github.com/phetsims/forces-and-motion-basics/issues/319
// https://github.com/phetsims/forces-and-motion-basics/issues/240
// https://github.com/phetsims/axon/issues/135
phetioStateSetEmitter.addListener( updateImage );
Expand Down
2 changes: 1 addition & 1 deletion js/motion/view/MotionScreenView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export default class MotionScreenView extends ScreenView {
this.addChild( playPauseStepHBox );

//Reset all button goes beneath the control panel. Not a closure variable since API access is required.
//TODO: Is that OK? or should we invest dynamic search/lookups to keep as closure var? https://github.com/phetsims/tasks/issues/1129
//TODO: Is that OK? or should we invest dynamic search/lookups to keep as closure var? https://github.com/phetsims/forces-and-motion-basics/issues/319
this.resetAllButton = new ResetAllButton( {
listener: () => {
model.reset();
Expand Down
12 changes: 6 additions & 6 deletions js/motion/view/MovingBackgroundNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export default class MovingBackgroundNode extends Node {

const mountainY = 311;

//TODO: It would be good to use cssTransforms here but they are a bit buggy https://github.com/phetsims/tasks/issues/1129
//TODO: It would be good to use cssTransforms here but they are a bit buggy https://github.com/phetsims/forces-and-motion-basics/issues/319
const mountainAndCloudLayer = new Node( {
tandem: tandem.createTandem( 'mountainAndCloudLayer' ),
x: layoutCenterX,
Expand All @@ -79,7 +79,7 @@ export default class MovingBackgroundNode extends Node {
this.addChild( mountainAndCloudLayer );

//Move the background objects
//TODO: support background objects with scale !== 1 https://github.com/phetsims/tasks/issues/1129
//TODO: support background objects with scale !== 1 https://github.com/phetsims/forces-and-motion-basics/issues/319

const getLayerUpdater = ( layer: Node, motionScale: number ) => {
let netDelta = 0;
Expand All @@ -98,7 +98,7 @@ export default class MovingBackgroundNode extends Node {
if ( sign === 1 ) {
// console.log( child.offsetX + netDelta, -800 );

//TODO: use modulus instead of while loop https://github.com/phetsims/tasks/issues/1129
//TODO: use modulus instead of while loop https://github.com/phetsims/forces-and-motion-basics/issues/319
// @ts-expect-error
while ( child.offsetX + netDelta < -L ) {
// console.log( 'jump 1' );
Expand All @@ -113,7 +113,7 @@ export default class MovingBackgroundNode extends Node {
else {
// console.log( child.offsetX + netDelta, L );

//TODO: use modulus instead of while loop https://github.com/phetsims/tasks/issues/1129
//TODO: use modulus instead of while loop https://github.com/phetsims/forces-and-motion-basics/issues/319
// @ts-expect-error
while ( child.offsetX + netDelta > L ) {
// console.log( 'jump 2' );
Expand Down Expand Up @@ -179,7 +179,7 @@ export default class MovingBackgroundNode extends Node {
model.frictionZeroProperty.linkAttribute( iceLayer, 'visible' );
this.addChild( iceLayer );

//TODO: could prevent updater from firing if ice is not visible https://github.com/phetsims/tasks/issues/1129
//TODO: could prevent updater from firing if ice is not visible https://github.com/phetsims/forces-and-motion-basics/issues/319
// @ts-expect-error
model.positionProperty.link( getLayerUpdater( iceLayer, 1 ) );

Expand Down Expand Up @@ -264,7 +264,7 @@ export default class MovingBackgroundNode extends Node {
numWhite--;
}

//TODO: get rid of pattern here, possibly by converting it too to an image? https://github.com/phetsims/tasks/issues/1129
//TODO: get rid of pattern here, possibly by converting it too to an image? https://github.com/phetsims/forces-and-motion-basics/issues/319
gravelSource.toImage( image => { gravel.fill = new Pattern( image ); }, 0, 0, tileWidth, height );
} );
}
Expand Down
2 changes: 1 addition & 1 deletion js/netforce/model/Knot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default class Knot extends PhetioObject {
* Constructor for the 8 knots that appear along the rope.
*
* @param x - the horizontal position (in meters) of the knot
* // TODO: Fix JSDoc https://github.com/phetsims/tasks/issues/1129
* // TODO: Fix JSDoc https://github.com/phetsims/forces-and-motion-basics/issues/319
* @param type - whether the knot is for red or blue pullers
* @param [options]
*/
Expand Down
8 changes: 4 additions & 4 deletions js/netforce/model/NetForceModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,14 @@ export default class NetForceModel extends PhetioObject {
range: new Range( 0, 8 )
} );

// TODO what are the valid values? https://github.com/phetsims/tasks/issues/1129
// TODO: Why not an enum? https://github.com/phetsims/tasks/issues/1129
// TODO what are the valid values? https://github.com/phetsims/forces-and-motion-basics/issues/319
// TODO: Why not an enum? https://github.com/phetsims/forces-and-motion-basics/issues/319
this.stateProperty = new StringProperty( 'experimenting', {
tandem: tandem.createTandem( 'stateProperty' )
} );

this.timeProperty = new Property( 0, {
// TODO: Removed this property for phet-io spam https://github.com/phetsims/tasks/issues/1129
// TODO: Removed this property for phet-io spam https://github.com/phetsims/forces-and-motion-basics/issues/319
// tandem: tandem.createTandem( 'timeProperty' )
// phetioValueType: NumberIO,
// units: 'seconds'
Expand Down Expand Up @@ -539,7 +539,7 @@ export default class NetForceModel extends PhetioObject {
*/
private getKnotDescription(): IntentionalAny {
return this.pullers.map( puller => ( {
id: puller.pullerTandem.phetioID, // TODO: addInstance for Puller https://github.com/phetsims/tasks/issues/1129
id: puller.pullerTandem.phetioID, // TODO: addInstance for Puller https://github.com/phetsims/forces-and-motion-basics/issues/319
knot: puller.knotProperty.get() && puller.knotProperty.get()!.phetioID
} ) );
}
Expand Down
4 changes: 2 additions & 2 deletions js/netforce/model/Puller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ export default class Puller {
public readonly positionProperty: Vector2Property;

// a classified position in the play area
// TODO: What are the valid values for this Property? https://github.com/phetsims/tasks/issues/1129
// TODO: Why not an enum? https://github.com/phetsims/tasks/issues/1129
// TODO: What are the valid values for this Property? https://github.com/phetsims/forces-and-motion-basics/issues/319
// TODO: Why not an enum? https://github.com/phetsims/forces-and-motion-basics/issues/319
public readonly lastPlacementProperty: StringProperty;

// emits an event when the puller is dropped
Expand Down
2 changes: 1 addition & 1 deletion js/netforce/view/GoPauseButton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
//Given nodes that have possibly different sizes, wrap the specified node in a parent empty Rectangle node so the bounds will match up
//If the node is already the largest, don't wrap it.
//Centers all the nodes in the parent wrappers
//TODO: Would be good to factor this out or provide better library support https://github.com/phetsims/tasks/issues/1129
//TODO: Would be good to factor this out or provide better library support https://github.com/phetsims/forces-and-motion-basics/issues/319
/**
* Given nodes that have possibly different sizes, wrap the specified node in a parent empty Rectangle node so the
* bounds will match up. If the node is already the largest, don't wrap it.
Expand Down
2 changes: 1 addition & 1 deletion js/netforce/view/ReturnButton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default class ReturnButton extends TextPushButton {

public constructor( model: NetForceModel, tandem: Tandem, options: TextPushButtonOptions ) {

// TODO: this method bound the model. Why? https://github.com/phetsims/tasks/issues/1129
// TODO: this method bound the model. Why? https://github.com/phetsims/forces-and-motion-basics/issues/319
const returnCart = () => {
model.returnCart();
};
Expand Down

0 comments on commit 2338184

Please sign in to comment.