More REVIEW bits and cleanup, see #88

phetsims · May 25, 2017 · 37aaa4a · 37aaa4a
1 parent b3bc8d4
commit 37aaa4a
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 5 deletions.
diff --git a/js/game/model/EECollectionArea.js b/js/game/model/EECollectionArea.js
@@ -98,7 +98,7 @@ define( function( require ) {
     /**
      * Test the provided coin term and, if it matches the spec, capture it by moving it into the center of this
      * collection area and, if it doesn't match, push it away.
-     * @param {Expression} coinTerm
+     * @param {CoinTerm} coinTerm
      * @public
      */
     collectOrRejectCoinTerm: function( coinTerm ) {
@@ -113,6 +113,7 @@ define( function( require ) {
       if ( this.isEmpty() && this.expressionDescriptionProperty.get().coinTermMatches( coinTerm ) ) {
 
         // collect this coin term
+        //REVIEW: use this.bounds.center instead?
         coinTerm.travelToDestination( new Vector2( this.bounds.getCenterX(), this.bounds.getCenterY() ) );
         coinTerm.collectedProperty.set( true );
         this.collectedItemProperty.set( coinTerm );
@@ -150,6 +151,7 @@ define( function( require ) {
       // figure out the destination, which is slightly different for coin terms versus expressions
       if ( collectedItem instanceof Expression ) {
         collectedItemBounds = collectedItem.getBounds();
+        //REVIEW: xDestination is the same for both branches of the if. Consolidate?
         xDestination = this.bounds.minX - collectedItemBounds.width - REJECTED_ITEM_DISTANCE;
         yDestination = this.bounds.getCenterY() - collectedItemBounds.height / 2;
       }

diff --git a/js/game/model/EEGameLevelModel.js b/js/game/model/EEGameLevelModel.js
@@ -22,7 +22,7 @@ define( function( require ) {
   var ViewMode = require( 'EXPRESSION_EXCHANGE/common/enum/ViewMode' );
 
   // constants
-  var EXPRESSION_COLLECTION_AREA_X_OFFSET = 750;
+  var EXPRESSION_COLLECTION_AREA_X_OFFSET = 750; //REVIEW: THe only use of this is in EECollectionArea. Move to there?
   var EXPRESSION_COLLECTION_AREA_INITIAL_Y_OFFSET = 50;
   var EXPRESSION_COLLECTION_AREA_Y_SPACING = 60;
   var NUM_EXPRESSION_COLLECTION_AREAS = 3;
@@ -49,18 +49,20 @@ define( function( require ) {
     var self = this;
 
     this.level = level; // {number} @public, read only
-    this.soundEnabledProperty = soundEnabledProperty; // @public (listen-only), used by view to enable/disable sounds
-    this.currentChallengeNumber = 0;
+    this.soundEnabledProperty = soundEnabledProperty; // @public {Property.<boolean>} (listen-only), used by view to enable/disable sounds
+    this.currentChallengeNumber = 0; //REVIEW: docs/visibility?
 
     // @public - property that refers to the current challenge
+    //REVIEW: type doc?
     this.currentChallengeProperty = new Property(
       EEChallengeDescriptors.getChallengeDescriptor( level, this.currentChallengeNumber )
     );
 
-    // @public (read only) - current score for this level
+    // @public {Property.<number>} (read only) - current score for this level
     this.scoreProperty = new Property( 0 );
 
     // helper function to update the score when items are collected or un-collected
+    //REVIEW: Lots of self references, maybe easier to specify as a method?
     function updateScore() {
       var score = 0;
       self.collectionAreas.forEach( function( collectionArea ) {
@@ -118,6 +120,7 @@ define( function( require ) {
       this.loadNextChallenge();
     },
 
+    //REVIEW: visibility/other doc?
     loadNextChallenge: function() {
       this.currentChallengeNumber = ( this.currentChallengeNumber + 1 ) % EEChallengeDescriptors.CHALLENGES_PER_LEVEL;
       this.currentChallengeProperty.set( EEChallengeDescriptors.getChallengeDescriptor(