Skip to content

Commit

Permalink
Documentation and naming improvements. Magic numbers are now constant…
Browse files Browse the repository at this point in the history
…s, and a string is in the strings file. Add parentheses for readability. Add missing return types. See #75.
  • Loading branch information
Luisav1 committed Dec 12, 2021
1 parent 51bd3ee commit 16ad891
Show file tree
Hide file tree
Showing 13 changed files with 43 additions and 27 deletions.
2 changes: 1 addition & 1 deletion js/game/model/CountingGameLevel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class CountingGameLevel extends NumberPlayGameLevel {
super( levelNumber, numberPlayStrings.counting, LEVEL_INPUT_RANGE );

// whether objects should be able to be grouped
this.groupObjects = levelNumber === 2;
this.groupObjects = ( levelNumber === 2 );

this.objectsPlayArea = new OnesPlayArea( this.challengeNumberProperty, new Vector2( 0, 0 ), {
isOnes: false,
Expand Down
5 changes: 4 additions & 1 deletion js/game/model/NumberPlayGameLevel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ abstract class NumberPlayGameLevel {
// guessed the answer
this.isChallengeSolvedProperty = new BooleanProperty( false );

// the random number generated to create a subitized representation for
// the random number generated to create a representation for
this.challengeNumberProperty = new NumberProperty( this.getRandomChallengeNumber(), {
numberType: 'Integer',
range: this.challengeRange
Expand Down Expand Up @@ -100,6 +100,9 @@ abstract class NumberPlayGameLevel {
this.challengeNumberProperty.value = newChallengeNumber;
}

/**
* Generate a random challenge number that is in the defined challenge range.
*/
private getRandomChallengeNumber(): number {
return dotRandom.nextIntBetween( this.challengeRange.min, this.challengeRange.max );
}
Expand Down
4 changes: 2 additions & 2 deletions js/game/model/SubitizeGameLevel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ class SubitizeGameLevel extends NumberPlayGameLevel {
this.subitizer.newChallenge();
}

public reset() {
public reset(): void {
super.reset();
this.subitizer.reset();
}

public step( dt: number ) {
public step( dt: number ): void {
this.subitizer.step( dt );
}
}
Expand Down
10 changes: 7 additions & 3 deletions js/game/model/Subitizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ class Subitizer {
// how long the shape is visible when shown, in seconds. This is a derived Property instead of a constant because
// the time that the shape is shown is increased if the user gets the answer wrong multiple times.
this.timeToShowShapeProperty = new DerivedProperty( [ numberOfAnswerButtonPressesProperty ],
( numberOfAnswerButtonPresses: number ) => {
numberOfAnswerButtonPresses => {
if ( numberOfAnswerButtonPresses > NumberPlayConstants.NUMBER_OF_SUBITIZER_GUESSES_AT_NORMAL_TIME ) {
return this.timeToShowShapeProperty.value + NumberPlayConstants.SHAPE_VISIBLE_TIME_INCREASE_AMOUNT;
}
Expand Down Expand Up @@ -397,13 +397,17 @@ class Subitizer {
// randomly pick between modifying the top row or bottom row
if ( dotRandom.nextBoolean() ) {
const sliceIndex = points.length - challengeNumber;
reducedPoints = points.slice( sliceIndex ); // remove extra points from the top row, left side

// remove extra points from the top row, left side
reducedPoints = points.slice( sliceIndex );

// remove extra points from the top row, right side
reducedShiftedPoints.splice( numberOfColumns - sliceIndex, sliceIndex );
}
else {
reducedPoints = points.slice( 0, challengeNumber ); // remove extra points from the bottom row, right side

// remove extra points from the bottom row, right side
reducedPoints = points.slice( 0, challengeNumber );

// remove extra points from the bottom row, left side
reducedShiftedPoints.splice( points.length - numberOfColumns, points.length - challengeNumber );
Expand Down
6 changes: 4 additions & 2 deletions js/game/view/CountingGameLevelNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ class CountingGameLevelNode extends NumberPlayGameLevelNode<CountingGameLevel> {
layoutBounds: Bounds2,
visibleBoundsProperty: Property<Bounds2> ) {

super( level, levelProperty, layoutBounds, visibleBoundsProperty, { statusBarFill: NumberPlayColors.countingGameColorProperty } );
super( level, levelProperty, layoutBounds, visibleBoundsProperty, {
statusBarFill: NumberPlayColors.countingGameColorProperty
} );

// create and add the answerButtons
this.answerButtons = new NumberPlayGameAnswerButtons( level, this.pointsAwardedNodeVisibleProperty, () => {
Expand Down Expand Up @@ -121,7 +123,7 @@ class CountingGameLevelNode extends NumberPlayGameLevelNode<CountingGameLevel> {
tenFramePanel.visible = !isObjects;
} );

// start a challenge
// start a new challenge
super.newChallenge();
}

Expand Down
6 changes: 3 additions & 3 deletions js/game/view/NumberPlayGameLevelNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ abstract class NumberPlayGameLevelNode<T extends NumberPlayGameLevel> extends No
levelNumber: level.levelNumber
} ), {
font: new PhetFont( 21 ),
maxWidth: 650 // determined empirically
maxWidth: 650
} );

// bar across the top of the screen
Expand All @@ -81,7 +81,7 @@ abstract class NumberPlayGameLevelNode<T extends NumberPlayGameLevel> extends No
levelProperty.value = null; // back to the level-selection UI
}
} );
// color the back button in the statusBar yellow
// color the backButton in the statusBar yellow and increase its touch area
const backButton = this.statusBar.children[ 1 ].children[ 0 ];
// @ts-ignore
backButton.baseColor = Color.YELLOW;
Expand Down Expand Up @@ -115,7 +115,7 @@ abstract class NumberPlayGameLevelNode<T extends NumberPlayGameLevel> extends No

// create and add the pointsAwardedNode which is shown when a correct guess is made on the first answer button press
const starNode = new StarNode( { value: 1, scale: 1.5 } );
const pointsNode = new Text( '+1', { font: new PhetFont( 44 ), fill: Color.BLACK } );
const pointsNode = new Text( numberPlayStrings.plusOne, { font: new PhetFont( 44 ), fill: Color.BLACK } );
const pointsAwardedNode = new HBox( {
children: [ pointsNode, starNode ],
spacing: 10,
Expand Down
9 changes: 4 additions & 5 deletions js/game/view/NumberPlayGameScreenView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const TRANSITION_OPTIONS = {
class NumberPlayGameScreenView extends ScreenView {

private readonly levelNodes: Array<SubitizeGameLevelNode | CountingGameLevelNode>;
private readonly subitizeLevelNodes: SubitizeGameLevelNode[];

constructor( model: NumberPlayGameModel, tandem: Tandem ) {

Expand All @@ -47,20 +46,20 @@ class NumberPlayGameScreenView extends ScreenView {
new CountingGameLevelNode( level, model.levelProperty, this.layoutBounds, this.visibleBoundsProperty ) );

// create the level nodes for the 'Subitize' game
this.subitizeLevelNodes = model.subitizeLevels.map( level =>
const subitizeLevelNodes = model.subitizeLevels.map( level =>
new SubitizeGameLevelNode( level, model.levelProperty, this.layoutBounds, this.visibleBoundsProperty ) );

// store all level nodes in one place for easy iteration
this.levelNodes = [ ...countingLevelNodes, ...this.subitizeLevelNodes ];
this.levelNodes = [ ...countingLevelNodes, ...subitizeLevelNodes ];

// create the transitionNode which handles the animated slide transition between levelSelectionNode and a level
const transitionNode = new TransitionNode( this.visibleBoundsProperty, {
content: levelSelectionNode,
cachedNodes: [ levelSelectionNode, ...this.levelNodes ]
} );

// Transition between levelSelectionNode and the selected level when the model changes.
// A null value for levelProperty indicates that no level is selected, and levelSelectionNode should be shown.
// Transition between the levelSelectionNode and the selected level when the model changes.
// if levelProperty has a null value, then no level is selected and levelSelectionNode will be displayed.
model.levelProperty.lazyLink( level => {
this.interruptSubtreeInput();

Expand Down
4 changes: 3 additions & 1 deletion js/game/view/SubitizeGameLevelNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ class SubitizeGameLevelNode extends NumberPlayGameLevelNode<SubitizeGameLevel> {
layoutBounds: Bounds2,
visibleBoundsProperty: Property<Bounds2> ) {

super( level, levelProperty, layoutBounds, visibleBoundsProperty, { statusBarFill: NumberPlayColors.subitizeGameColorProperty } );
super( level, levelProperty, layoutBounds, visibleBoundsProperty, {
statusBarFill: NumberPlayColors.subitizeGameColorProperty
} );

// create and add the answerButtons
this.answerButtons = new NumberPlayGameAnswerButtons( level, this.pointsAwardedNodeVisibleProperty, () => {
Expand Down
9 changes: 5 additions & 4 deletions js/game/view/SubitizeLoadingBarNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@ const ANIMATION_DELAY = 0.1; // in seconds
class SubitizeLoadingBarNode extends Node {

private readonly isLoadingBarAnimatingProperty: BooleanProperty;
private readonly newChallenge: () => void;
private readonly newChallengeCallback: () => void;
private loadingBarAnimation: Animation | null;
private readonly loadingBarWidthProperty: NumberProperty;

constructor( newChallenge: () => void, isLoadingBarAnimatingProperty: BooleanProperty ) {
constructor( newChallengeCallback: () => void, isLoadingBarAnimatingProperty: BooleanProperty ) {
super();
this.newChallenge = newChallenge;

this.newChallengeCallback = newChallengeCallback;
this.isLoadingBarAnimatingProperty = isLoadingBarAnimatingProperty;

// the rectangle that is a border to the filled rectangle
Expand Down Expand Up @@ -115,7 +116,7 @@ class SubitizeLoadingBarNode extends Node {
*/
private end(): void {
this.loadingBarAnimation = null;
this.newChallenge();
this.newChallengeCallback();
}
}

Expand Down
5 changes: 3 additions & 2 deletions js/game/view/SubitizeRevealButton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import NumberPlayColors from '../../common/NumberPlayColors.js';
// constants
const BUTTON_SIDE_LENGTH = SceneryPhetConstants.DEFAULT_BUTTON_RADIUS * 2;
const BUTTON_CONTENT_MARGIN = 6;
const BUTTON_TOUCH_AREA_DILATION = 10;

class SubitizeRevealButton extends RectangularPushButton {

Expand All @@ -42,8 +43,8 @@ class SubitizeRevealButton extends RectangularPushButton {
content: eyeNode,
xMargin: BUTTON_CONTENT_MARGIN,
yMargin: BUTTON_CONTENT_MARGIN,
touchAreaXDilation: 10,
touchAreaYDilation: 10,
touchAreaXDilation: BUTTON_TOUCH_AREA_DILATION,
touchAreaYDilation: BUTTON_TOUCH_AREA_DILATION,
size: new Dimension2( BUTTON_SIDE_LENGTH, BUTTON_SIDE_LENGTH ),
baseColor: NumberPlayColors.subitizeGameLightColorProperty,
visibleProperty: revealButtonVisibleProperty,
Expand Down
4 changes: 2 additions & 2 deletions js/game/view/SubitizerNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const REVEAL_BUTTON_MARGIN = 12;

class SubitizerNode extends Node {

constructor( subitizer: Subitizer, isChallengeSolvedProperty: BooleanProperty, newChallenge: () => void ) {
constructor( subitizer: Subitizer, isChallengeSolvedProperty: BooleanProperty, newChallengeCallback: () => void ) {
super();

// for scaling the objects
Expand All @@ -43,7 +43,7 @@ class SubitizerNode extends Node {
this.addChild( backgroundNode );

// create and add the subitizeLoadingBarNode
const subitizeLoadingBarNode = new SubitizeLoadingBarNode( newChallenge, subitizer.isLoadingBarAnimatingProperty );
const subitizeLoadingBarNode = new SubitizeLoadingBarNode( newChallengeCallback, subitizer.isLoadingBarAnimatingProperty );
subitizeLoadingBarNode.center = this.center;
this.addChild( subitizeLoadingBarNode );

Expand Down
3 changes: 2 additions & 1 deletion js/numberPlayStrings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ type StringsType = {
'gameLevelPattern': string,
'chooseYourGame': string,
'counting': string,
'subitize': string
'subitize': string,
'plusOne': string
};

const numberPlayStrings = getStringModule( 'NUMBER_PLAY' ) as StringsType;
Expand Down
3 changes: 3 additions & 0 deletions number-play-strings_en.json
Original file line number Diff line number Diff line change
Expand Up @@ -124,5 +124,8 @@
},
"subitize": {
"value": "Subitize"
},
"plusOne": {
"value": "+1"
}
}

0 comments on commit 16ad891

Please sign in to comment.