Skip to content

Commit

Permalink
Improvements on constants, visibility annotations, and naming of prop…
Browse files Browse the repository at this point in the history
…erties. Add missing assert to getPredeterminedShapes function. See #75.
  • Loading branch information
Luisav1 committed Dec 13, 2021
1 parent 16ad891 commit dfe8c42
Show file tree
Hide file tree
Showing 13 changed files with 66 additions and 50 deletions.
9 changes: 8 additions & 1 deletion js/common/NumberPlayQueryParameters.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,18 @@
* @author Chris Klusendorf (PhET Interactive Simulations)
*/

import StringUtils from '../../../phetcommon/js/util/StringUtils.js';
import numberPlay from '../numberPlay.js';
import numberPlayStrings from '../numberPlayStrings.js';
import NumberPlayConstants from './NumberPlayConstants.js';

// constants
const GAME_LEVELS_DEFAULT_VALUES = [ 'a1', 'a2', 'b1', 'b2' ];
const GAME_LEVELS_DEFAULT_VALUES = [
StringUtils.fillIn( numberPlayStrings.aLevelNumberPattern, { levelNumber: 1 } ),
StringUtils.fillIn( numberPlayStrings.aLevelNumberPattern, { levelNumber: 2 } ),
StringUtils.fillIn( numberPlayStrings.bLevelNumberPattern, { levelNumber: 1 } ),
StringUtils.fillIn( numberPlayStrings.bLevelNumberPattern, { levelNumber: 2 } )
];

const NumberPlayQueryParameters = QueryStringMachine.getAll( {

Expand Down
4 changes: 1 addition & 3 deletions js/game/NumberPlayGameScreen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ import numberPlayStrings from '../numberPlayStrings.js';
import NumberPlayGameModel from './model/NumberPlayGameModel.js';
import NumberPlayGameScreenView from './view/NumberPlayGameScreenView.js';

const screenGameString = numberPlayStrings.screen.game;

class NumberPlayGameScreen extends Screen {

constructor( tandem: Tandem ) {

const options = {
name: screenGameString,
name: numberPlayStrings.screen.game,
backgroundColorProperty: NumberPlayColors.gameScreenBackgroundColorProperty,
homeScreenIcon: new ScreenIcon( new Image( gameScreenIconImage ), {
maxIconWidthProportion: 1,
Expand Down
2 changes: 1 addition & 1 deletion js/game/model/NumberPlayGameLevel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ abstract class NumberPlayGameLevel {
* Sets a new challenge number. Can be the value of the previous challenge number, but there cannot be three of the
* same number in a row.
*/
protected setRandomChallengeNumber(): void {
private setRandomChallengeNumber(): void {
assert && assert( this.challengeRange.min !== this.challengeRange.max,
`challengeRange must contain more than one number: ${this.challengeRange.toString()}` );

Expand Down
10 changes: 8 additions & 2 deletions js/game/model/NumberPlayGameModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import numberPlay from '../../numberPlay.js';
import SubitizeGameLevel from './SubitizeGameLevel.js';
import CountingGameLevel from './CountingGameLevel.js';
import NumberPlayQueryParameters from '../../common/NumberPlayQueryParameters.js';
import numberPlayStrings from '../../numberPlayStrings.js';
import StringUtils from '../../../../phetcommon/js/util/StringUtils.js';

class NumberPlayGameModel {

Expand All @@ -24,8 +26,12 @@ class NumberPlayGameModel {
constructor( tandem: Tandem ) {

// get the level numbers specified by the gameLevels query parameter
const gameALevelNumbers = NumberPlayGameModel.getLevelNumbers( NumberPlayQueryParameters.gameLevels, 'a' );
const gameBLevelNumbers = NumberPlayGameModel.getLevelNumbers( NumberPlayQueryParameters.gameLevels, 'b' );
const gameALevelNumbers = NumberPlayGameModel.getLevelNumbers( NumberPlayQueryParameters.gameLevels,
StringUtils.fillIn( numberPlayStrings.aLevelNumberPattern, { levelNumber: '' } )
);
const gameBLevelNumbers = NumberPlayGameModel.getLevelNumbers( NumberPlayQueryParameters.gameLevels,
StringUtils.fillIn( numberPlayStrings.bLevelNumberPattern, { levelNumber: '' } )
);

// create the levels for each game
this.countingLevels = gameALevelNumbers.map( gameALevelNumber => new CountingGameLevel( gameALevelNumber ) );
Expand Down
11 changes: 8 additions & 3 deletions js/game/model/Subitizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ class Subitizer {
public readonly objectSize: number;
public readonly isInputEnabledProperty: BooleanProperty;
private timeToShowShapeProperty: IReadOnlyProperty<number>;
public objectTypeProperty: Property<SubitizeObjectTypeEnum>;
public isDelayStarted: boolean;
public readonly objectTypeProperty: Property<SubitizeObjectTypeEnum>;
private isDelayStarted: boolean;
private timeSinceDelayStarted: number;
public readonly isLoadingBarAnimatingProperty: BooleanProperty;
public static SUBITIZER_BOUNDS: Bounds2;
Expand Down Expand Up @@ -309,7 +309,7 @@ class Subitizer {
/**
* Sets this.objectTypeProperty with a new object type for the current challenge.
*/
public setRandomPlayObjectType(): void {
private setRandomPlayObjectType(): void {
this.objectTypeProperty.value = dotRandom.sample( SubitizeObjectTypeValues.slice() );
}

Expand All @@ -324,6 +324,11 @@ class Subitizer {
* N = the provided challengeNumber.
*/
private static getPredeterminedShapePoints( challengeNumber: number ): Vector2[] {

assert && assert( PREDETERMINED_SHAPES && PREDETERMINED_SHAPES[ challengeNumber ],
`There exists no predetermined shape for challenge number ${challengeNumber}. ` +
`Predetermined shapes: ${Object.keys( PREDETERMINED_SHAPES ).toString()}` );

const shapeIndex = dotRandom.nextInt( PREDETERMINED_SHAPES[ challengeNumber ].length );
const shape = PREDETERMINED_SHAPES[ challengeNumber ][ shapeIndex ];

Expand Down
16 changes: 8 additions & 8 deletions js/game/view/CountingGameLevelNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import NumberPlayColors from '../../common/NumberPlayColors.js';
// constants
const RECTANGLE_WIDTH = 550;
const RECTANGLE_HEIGHT = 325;
const PANEL_LINE_WIDTH = 2;
const TEN_FRAME_MARGIN = 10;

// TODO: The parts of this file that are used for the play area need to be refactored once the play area is updated.
class CountingGameLevelNode extends NumberPlayGameLevelNode<CountingGameLevel> {
Expand Down Expand Up @@ -82,21 +84,19 @@ class CountingGameLevelNode extends NumberPlayGameLevelNode<CountingGameLevel> {
playAreaNode.addChild( objectsPlayAreaNode );

// create and add the playAreaPanel, a panel for the play area
const panelLineWidth = 2;
const playAreaPanel = new Panel( playAreaNode, {
xMargin: 0,
yMargin: 0,
fill: NumberPlayColors.blueBackgroundColorProperty,
lineWidth: panelLineWidth
lineWidth: PANEL_LINE_WIDTH
} );
playAreaPanel.centerX = layoutBounds.centerX;
playAreaPanel.bottom = this.answerButtons.top - NumberPlayGameLevelNode.GAME_AREA_NODE_BOTTOM_MARGIN_Y;
this.addChild( playAreaPanel );

const margin = 10;
const tenFrameBackgroundNode = new Rectangle( {
rectWidth: RECTANGLE_WIDTH - margin * 2,
rectHeight: RECTANGLE_HEIGHT - margin * 2,
rectWidth: RECTANGLE_WIDTH - TEN_FRAME_MARGIN * 2,
rectHeight: RECTANGLE_HEIGHT - TEN_FRAME_MARGIN * 2,
fill: NumberPlayColors.lightGreenBackgroundColorProperty
} );

Expand All @@ -108,10 +108,10 @@ class CountingGameLevelNode extends NumberPlayGameLevelNode<CountingGameLevel> {

// create and add the tenFramePanel, a panel for the ten frame
const tenFramePanel = new Panel( tenFrameBackgroundNode, {
xMargin: margin,
yMargin: margin,
xMargin: TEN_FRAME_MARGIN,
yMargin: TEN_FRAME_MARGIN,
fill: NumberPlayColors.lightGreenBackgroundColorProperty,
lineWidth: panelLineWidth
lineWidth: PANEL_LINE_WIDTH
} );
tenFramePanel.centerX = layoutBounds.centerX;
tenFramePanel.bottom = this.answerButtons.top - NumberPlayGameLevelNode.GAME_AREA_NODE_BOTTOM_MARGIN_Y;
Expand Down
9 changes: 3 additions & 6 deletions js/game/view/NumberPlayGameAnswerButtons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,9 @@ const BUTTON_OBJECT_CORNER_RADIUS = 10;
class NumberPlayGameAnswerButtons extends Node {

private readonly buttonObjects: ButtonObject[];
private readonly level: SubitizeGameLevel | CountingGameLevel;
private readonly hBox: HBox;
private readonly buttonListener: ( index: number ) => void;
static BUTTON_DIMENSION: Dimension2;
public static BUTTON_DIMENSION: Dimension2;

constructor( level: SubitizeGameLevel | CountingGameLevel,
pointsAwardedNodeVisibleProperty: BooleanProperty,
Expand All @@ -61,8 +60,6 @@ class NumberPlayGameAnswerButtons extends Node {

this.buttonObjects = [];

this.level = level;

/**
* Listener that is added to every answer button. It disabled selected buttons that are wrong, and turns correct
* answer buttons green.
Expand Down Expand Up @@ -153,8 +150,8 @@ class NumberPlayGameAnswerButtons extends Node {
* Returns everything in the HBox to its original button state.
*/
public reset(): void {
this.buttonObjects.forEach( object => object.enabledProperty.reset() );
this.hBox.children = this.buttonObjects.map( object => object.button );
this.buttonObjects.forEach( buttonObject => buttonObject.enabledProperty.reset() );
this.hBox.children = this.buttonObjects.map( buttonObject => buttonObject.button );
}
}

Expand Down
22 changes: 9 additions & 13 deletions js/game/view/NumberPlayGameLevelNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,13 @@ const FACE_DIAMETER = 150;

abstract class NumberPlayGameLevelNode<T extends NumberPlayGameLevel> extends Node {

public readonly statusBar: InfiniteStatusBar;
public readonly level: T;
private readonly layoutBounds: Bounds2;
private readonly frownyFaceNode: FaceNode;
private frownyFaceAnimation: Animation | null;
protected readonly pointsAwardedNodeVisibleProperty: BooleanProperty;
protected abstract answerButtons: NumberPlayGameAnswerButtons;
static ANSWER_BUTTONS_BOTTOM_MARGIN_Y: number;
static GAME_AREA_NODE_BOTTOM_MARGIN_Y: number;
public static ANSWER_BUTTONS_BOTTOM_MARGIN_Y: number;
public static GAME_AREA_NODE_BOTTOM_MARGIN_Y: number;

protected constructor( level: T,
levelProperty: Property<SubitizeGameLevel | CountingGameLevel | null>,
Expand All @@ -62,7 +60,7 @@ abstract class NumberPlayGameLevelNode<T extends NumberPlayGameLevel> extends No
}, providedOptions ) as StatusBarOptions;

// text displayed in the statusBar
const levelDescriptionText = new RichText( StringUtils.fillIn( numberPlayStrings.gameLevelPattern, {
const levelDescriptionText = new RichText( StringUtils.fillIn( numberPlayStrings.gameNameLevelNumberPattern, {
gameName: level.gameName,
levelNumber: level.levelNumber
} ), {
Expand All @@ -72,7 +70,7 @@ abstract class NumberPlayGameLevelNode<T extends NumberPlayGameLevel> extends No

// bar across the top of the screen
// @ts-ignore
this.statusBar = new InfiniteStatusBar( layoutBounds, visibleBoundsProperty, levelDescriptionText, level.scoreProperty, {
const statusBar = new InfiniteStatusBar( layoutBounds, visibleBoundsProperty, levelDescriptionText, level.scoreProperty, {
floatToTop: true,
spacing: 20,
barFill: options.statusBarFill,
Expand All @@ -82,16 +80,14 @@ abstract class NumberPlayGameLevelNode<T extends NumberPlayGameLevel> extends No
}
} );
// color the backButton in the statusBar yellow and increase its touch area
const backButton = this.statusBar.children[ 1 ].children[ 0 ];
const backButton = statusBar.children[ 1 ].children[ 0 ];
// @ts-ignore
backButton.baseColor = Color.YELLOW;
backButton.touchArea = backButton.bounds.dilated( NumberPlayConstants.TOUCH_AREA_DILATION );
this.addChild( this.statusBar );
this.addChild( statusBar );

this.level = level;

this.layoutBounds = layoutBounds;

// create and add the frownyFaceNode which is visible when an incorrect answer button is pressed
this.frownyFaceNode = new FaceNode( FACE_DIAMETER, {
visible: false
Expand Down Expand Up @@ -126,7 +122,7 @@ abstract class NumberPlayGameLevelNode<T extends NumberPlayGameLevel> extends No
this.addChild( pointsAwardedNode );

// create and add the newChallengeButton which is visible when a challenge is solved, meaning a correct answer button was pressed
const arrowShape = new ArrowShape( 0, 0, 42, 0, {
const rightArrowShape = new ArrowShape( 0, 0, 42, 0, {
tailWidth: 12,
headWidth: 25,
headHeight: 23
Expand All @@ -137,12 +133,12 @@ abstract class NumberPlayGameLevelNode<T extends NumberPlayGameLevel> extends No
yMargin: 10.9,
touchAreaXDilation: NumberPlayConstants.TOUCH_AREA_DILATION,
touchAreaYDilation: NumberPlayConstants.TOUCH_AREA_DILATION,
content: new Path( arrowShape, { fill: Color.BLACK } ),
content: new Path( rightArrowShape, { fill: Color.BLACK } ),
visibleProperty: level.isChallengeSolvedProperty,
listener: () => this.newChallenge()
} );
newChallengeButton.centerX = smileyFaceNode.centerX;
newChallengeButton.bottom = this.layoutBounds.maxY -
newChallengeButton.bottom = layoutBounds.maxY -
NumberPlayGameLevelNode.ANSWER_BUTTONS_BOTTOM_MARGIN_Y -
NumberPlayGameAnswerButtons.BUTTON_DIMENSION.height -
NumberPlayGameLevelNode.GAME_AREA_NODE_BOTTOM_MARGIN_Y;
Expand Down
2 changes: 1 addition & 1 deletion js/game/view/NumberPlayGameLevelSelectionNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class NumberPlayGameLevelSelectionNode extends Node {
children: [
new HStrut( 47 ), // empirically determined to keep text the same size, based on the button size
new Text( level.gameName ),
new Text( StringUtils.fillIn( numberPlayStrings.levelPattern, { levelNumber: level.levelNumber } ) )
new Text( StringUtils.fillIn( numberPlayStrings.levelNumberPattern, { levelNumber: level.levelNumber } ) )
]
} ), level.scoreProperty, {
scoreDisplayConstructor: ScoreDisplayNumberAndStar,
Expand Down
6 changes: 2 additions & 4 deletions js/game/view/SubitizeLoadingBarNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ const RECTANGLE_HEIGHT = 30;
const RECTANGLE_LINE_WIDTH = 2;
const RECTANGLE_OUTER_CORNER_RADIUS = 8;
const RECTANGLE_INNER_CORNER_RADIUS = RECTANGLE_OUTER_CORNER_RADIUS - RECTANGLE_LINE_WIDTH / 2;
const ANIMATION_DURATION = 2; // in seconds
const ANIMATION_DELAY = 0.1; // in seconds

class SubitizeLoadingBarNode extends Node {

Expand Down Expand Up @@ -90,8 +88,8 @@ class SubitizeLoadingBarNode extends Node {
this.loadingBarWidthProperty.reset();

this.loadingBarAnimation = new Animation( {
delay: ANIMATION_DELAY,
duration: ANIMATION_DURATION,
delay: 0.1,
duration: 2,
targets: [ {
property: this.loadingBarWidthProperty,
easing: Easing.LINEAR,
Expand Down
7 changes: 4 additions & 3 deletions js/game/view/SubitizerNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import SubitizeRevealButton from './SubitizeRevealButton.js';
import NumberPlayConstants from '../../common/NumberPlayConstants.js';

// constants
const CORNER_RADIUS = 10;
const BACKGROUND_RECTANGLE_CORNER_RADIUS = 10;
const REVEAL_BUTTON_MARGIN = 12;

class SubitizerNode extends Node {
Expand All @@ -34,7 +34,8 @@ class SubitizerNode extends Node {

// create and add the backgroundNode
const backgroundNode = new Rectangle( 0, 0, scaleMVT.modelToViewDeltaX( Subitizer.SUBITIZER_BOUNDS.width ),
scaleMVT.modelToViewDeltaY( Subitizer.SUBITIZER_BOUNDS.height ), CORNER_RADIUS, CORNER_RADIUS, {
scaleMVT.modelToViewDeltaY( Subitizer.SUBITIZER_BOUNDS.height ),
BACKGROUND_RECTANGLE_CORNER_RADIUS, BACKGROUND_RECTANGLE_CORNER_RADIUS, {
fill: Color.WHITE,
stroke: Color.BLACK,
lineWidth: 2
Expand Down Expand Up @@ -92,7 +93,7 @@ class SubitizerNode extends Node {
points.forEach( point => {
let object;
if ( subitizer.objectTypeProperty.value === 'circle' ) {
object = new Circle( scaleMVT.modelToViewDeltaX( subitizer.objectSize * 0.5 ), { fill: Color.BLACK } );
object = new Circle( scaleMVT.modelToViewDeltaX( subitizer.objectSize / 2 ), { fill: Color.BLACK } );
}
else {
object = new Image(
Expand Down
8 changes: 5 additions & 3 deletions js/numberPlayStrings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,14 @@ type StringsType = {
'objects': string,
'english': string,
'spanish': string,
'levelPattern': string,
'gameLevelPattern': string,
'levelNumberPattern': string,
'gameNameLevelNumberPattern': string,
'chooseYourGame': string,
'counting': string,
'subitize': string,
'plusOne': string
'plusOne': string,
'aLevelNumberPattern': string,
'bLevelNumberPattern': string
};

const numberPlayStrings = getStringModule( 'NUMBER_PLAY' ) as StringsType;
Expand Down
10 changes: 8 additions & 2 deletions number-play-strings_en.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@
"spanish": {
"value": "Español"
},
"levelPattern": {
"levelNumberPattern": {
"value": "Level {{levelNumber}}"
},
"gameLevelPattern": {
"gameNameLevelNumberPattern": {
"value": "{{gameName}}: Level {{levelNumber}}"
},
"chooseYourGame": {
Expand All @@ -127,5 +127,11 @@
},
"plusOne": {
"value": "+1"
},
"aLevelNumberPattern": {
"value": "a{{levelNumber}}"
},
"bLevelNumberPattern": {
"value": "b{{levelNumber}}"
}
}

0 comments on commit dfe8c42

Please sign in to comment.