Skip to content

Commit

Permalink
Address review comments, see #247
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisklus committed Aug 29, 2019
1 parent b6a6c3f commit 5c8e45f
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 35 deletions.
1 change: 0 additions & 1 deletion js/systems/model/EnergySystemElementCarousel.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@ define( require => {
} );
}

//REVIEW #247 document
/**
* whether the current selected element is in its destination spot
* @private
Expand Down
3 changes: 1 addition & 2 deletions js/systems/model/EnergyUser.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ define( require => {
this.incomingEnergyChunks = _.union( this.incomingEnergyChunks, energyChunks );
}

//REVIEW #247 this was @protected in the superclass, is @public accurate?
/**
* @public
* @protected
* @override
*/
clearEnergyChunks() {
Expand Down
65 changes: 33 additions & 32 deletions js/systems/model/Fan.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,10 @@ define( require => {
this.targetVelocity = 0;
}

//REVIEW #247 step appears nowhere in the class hierarchy, why is this an override?
/**
* @param {number} dt - time step, in seconds
* @param {Energy} incomingEnergy
* @public
* @override
*/
step( dt, incomingEnergy ) {
if ( !this.activeProperty.value ) {
Expand Down Expand Up @@ -181,8 +179,9 @@ define( require => {
this.bladePositionProperty.set( newAngle );
}

//REVIEW #247 document
/**
* move electrical energy chunks through the fan's wire
*
* @param {number} dt - time step, in seconds
* @private
*/
Expand Down Expand Up @@ -210,7 +209,7 @@ define( require => {

// release the energy chunk as mechanical to blow away
this.mechanicalEnergyChunkMovers.push( new EnergyChunkPathMover( mover.energyChunk,
this.createBlownEnergyChunkPath( mover.energyChunk.positionProperty.get() ),
createBlownEnergyChunkPath( mover.energyChunk.positionProperty.get() ),
EFACConstants.ENERGY_CHUNK_VELOCITY ) );
}
else {
Expand All @@ -230,8 +229,9 @@ define( require => {
} );
}

//REVIEW #247 document
/**
* move thermal energy chunks up and away from the fan
*
* @param {number} dt - time step, in seconds
* @private
*/
Expand All @@ -250,6 +250,8 @@ define( require => {
}

/**
* move mechanical energy chunks out of the motor and away from the blades as wind
*
* @param {number} dt - time step, in seconds
* @private
*/
Expand All @@ -267,33 +269,6 @@ define( require => {
} );
}

//REVIEW #247 function can be private, it has no dependencies on Fan
/**
* create a path for chunks to follow when blown out of the fan.
* @param {Vector2} startingPoint
* @returns {Vector2[]}
* @private
*/
createBlownEnergyChunkPath( startingPoint ) {
const path = [];
const numberOfDirectionChanges = 20; // empirically determined
const nominalTravelVector = new Vector2( BLOWN_ENERGY_CHUNK_TRAVEL_DISTANCE / numberOfDirectionChanges, 0 );

// The first point is straight right the starting point. This is done because it makes the chunk
// move straight out of the fan center cone.
let currentPosition = startingPoint.plus( new Vector2( INSIDE_FAN_ENERGY_CHUNK_TRAVEL_DISTANCE, 0 ) );
path.push( currentPosition );

// add the remaining points in the path
for ( let i = 0; i < numberOfDirectionChanges - 1; i++ ) {
const movement = nominalTravelVector.rotated( ( phet.joist.random.nextDouble() - 0.5 ) * Math.PI / 4 );
currentPosition = currentPosition.plus( movement );
path.push( currentPosition );
}

return path;
}

/**
* deactivate this energy system element
* @public
Expand Down Expand Up @@ -384,6 +359,32 @@ define( require => {
}
}

/**
* create a path for chunks to follow when blown out of the fan.
* @param {Vector2} startingPoint
* @returns {Vector2[]}
* @private
*/
const createBlownEnergyChunkPath = startingPoint => {
const path = [];
const numberOfDirectionChanges = 20; // empirically determined
const nominalTravelVector = new Vector2( BLOWN_ENERGY_CHUNK_TRAVEL_DISTANCE / numberOfDirectionChanges, 0 );

// The first point is straight right the starting point. This is done because it makes the chunk
// move straight out of the fan center cone.
let currentPosition = startingPoint.plus( new Vector2( INSIDE_FAN_ENERGY_CHUNK_TRAVEL_DISTANCE, 0 ) );
path.push( currentPosition );

// add the remaining points in the path
for ( let i = 0; i < numberOfDirectionChanges - 1; i++ ) {
const movement = nominalTravelVector.rotated( ( phet.joist.random.nextDouble() - 0.5 ) * Math.PI / 4 );
currentPosition = currentPosition.plus( movement );
path.push( currentPosition );
}

return path;
};

return energyFormsAndChanges.register( 'Fan', Fan );
} );

0 comments on commit 5c8e45f

Please sign in to comment.