8
\$\begingroup\$

I am fairly new to programming using the site openprocessing.org.

My program, available to play online here, is a simple puzzle game in which you slide the circles so that you end up with 4 circles in the top row whose colors are the same as the colors of the circles in the corresponding positions in the bottom row.

Any comments on my program would be appreciated.

// This program lets the user play an electronic version of the CHANGE CHANGE game.
// The CHANGE CHANGE game was published by Sid Sackson in his book, A GAMUT OF GAMES.
//
// In this version, the user repositions the 11 colored circles of a 3 by 4 grid ...
// ... with the goal of making the colors of the circles in the top row the same as ...
// ... the colors of the circles in the corresponding positions in the bottom row.
//
// The user can slide circles up, down, left or right into an adjacent empty spot ...
// ... by tapping/clicking on the circle they want to move.
//
// I developed this program on my iPad and it works fine if the user interacts with finger taps.
// I think the program also works fine with mouse clicks but I haven't verified this personally.
// For each puzzle, 4 of the 6 colors below will be used to draw the circles
let ColorList = ['red', 'yellow', 'lime', 'aqua', 'fuchsia', 'white'];
let Circles = [ [3,2,0,2], [0,1,1,2], [0,2,0,-1] ]; // 3x4 playing grid
let EmptySpace = -1; // Circles can be slid into the empty space
let EmptyRow, EmptyCol; // Position of empty space in grid
let SquareSize; // Height & width of the 3x4 invisible squares all but one containing a circle
let SquareHalfSize; // Used to compute circle's center
let SlideSpeed=15; // Speed of sliding circle measured in pixels/frame
let CircleSize; // Diameter of circles
let TapTolerance; // Maximum allowable tap distance from circle's center for circle selection
let TopBorder; // Gap above playing grid
let LeftBorder; // Gap to the left of playing grid
///////////////////////////////////////////////////////////////////////////////////////////////
let GameState; // Progress of game: Displaying introduction? or Game started? or ...
let IntroductionState = 50; // State: Introduction is displayed for the user
let StartedState = 100; // State: Game has started
let CircleSelectedState = 101; // State: User successfully selected a circle to be slid
let CircleSlidingState = 102; // State: Currently sliding the user selected circle
let SolvedState = 103; // State: User has won the game (top row = bottom row)
///////////////////////////////////////////////////////////////////////////////////////////////
let SelectedRow = -99, SelectedCol = -99; // Row, column of user selected circle in 3x4 grid
let InitialframeCount; // Drawing frame count just prior to sliding a circle
function setup() {
 
 // Game initialization
 
 createCanvas(windowWidth, windowHeight);
 noStroke();
 
 Create3x4GridSizes();
 FindEmptySpace();
 
 GameState = IntroductionState;
 
} // end setup
function Create3x4GridSizes() {
 
 // Create all size information for the playing grid
 
 SquareSize = min( windowHeight/3, windowWidth/4 );
 SquareSize = floor( 0.99 * SquareSize ); // Ensure a bit of a border
 
 // Ensure SquareSize is a multiple of SlideSpeed
 SquareSize = SlideSpeed * floor(SquareSize/SlideSpeed);
 
 SquareHalfSize = floor( SquareSize/2 );
 
 CircleSize = floor( 0.75 * SquareSize );
 TapTolerance = floor( 0.85 * (CircleSize/2) ); // 85% of radius
 
 TopBorder = floor( (windowHeight - 3*SquareSize) / 2 );
 LeftBorder = floor( (windowWidth - 4*SquareSize) / 2 );
 
} // end Create3x4GridSizes
function touchStarted() {
 
 // Handle screen taps
 
 // Assuming game is in progress, has user selected a circle that can be slid?
 //
 // ***** OR *****
 //
 // Assuming game has been solved, has user selected the happy smiley face to start a new game?
 
 if( GameState === StartedState || GameState === SolvedState ) {
 if( mouseX >= LeftBorder && mouseX < LeftBorder+4*SquareSize ) { // In 3x4 grid?
 if( mouseY >= TopBorder && mouseY < TopBorder+3*SquareSize ) {
 SelectedRow = floor( (mouseY-TopBorder) / SquareSize );
 SelectedCol = floor( (mouseX-LeftBorder) / SquareSize );
 Selected_X = LeftBorder + SelectedCol*SquareSize + SquareHalfSize;
 Selected_Y = TopBorder + SelectedRow*SquareSize + SquareHalfSize;
 
 DistanceFromCenter = sqrt( (mouseX-Selected_X)**2 + (mouseY-Selected_Y)**2 );
 if ( DistanceFromCenter <= TapTolerance ) { // Tap near circle (or face) center?
 
 if( GameState === StartedState ) {
 
 // Is selected circle adjacent to the empty space?
 
 if( abs(SelectedRow-EmptyRow) + abs(SelectedCol-EmptyCol) === 1 ) {
 GameState = CircleSelectedState;
 }
 
 } else if( GameState === SolvedState ) {
 
 // Has user tapped the happy smiley face?
 
 if( SelectedRow === EmptyRow && SelectedCol === EmptyCol ) {
 GameState = StartedState;
 NewGame();
 }
 
 }
 
 }
 }
 }
 }
 
 // Start game if user taps screen while introductory screen is displayed
 
 if( GameState === IntroductionState) {
 GameState = StartedState;
 }
 
} // end touchStarted
function draw() {
 
 // Handle screen output (rules, game play, ...)
 
 if( GameState === IntroductionState ) {
 DisplayIntroduction();
 }
 
 if( GameState >= StartedState ) {
 DisplayCircles();
 }
 
} // end draw
function DisplayCircles() {
 
 // Displays the 3x4 playing grid of circles that the user manipulates
 
 background('black');
 for (row = 0; row <= 2; row += 1) {
 for (col = 0; col <= 3; col += 1) {
 if ( Circles[row][col] !== EmptySpace ) {
 fill( ColorList[Circles[row][col]] );
 CircleCenter_X = LeftBorder + col*SquareSize + SquareHalfSize;
 CircleCenter_Y = TopBorder + row*SquareSize + SquareHalfSize;
 if( GameState !== CircleSlidingState ||
 row !== SelectedRow ||
 col !== SelectedCol ) {
 circle( CircleCenter_X, CircleCenter_Y, CircleSize );
 } else {
 
 // Slide selected circle to empty space
 
 SlideProgress = frameCount - InitialframeCount;
 SlideProgress = SlideSpeed * SlideProgress;
 
 if( SelectedRow === EmptyRow ) {
 
 // Slide is horizontal
 
 if( SelectedCol < EmptyCol ) {
 CircleCenter_X += SlideProgress;
 } else {
 CircleCenter_X -= SlideProgress;
 }
 
 } else {
 
 // Slide is vertical
 
 if( SelectedRow < EmptyRow ) {
 CircleCenter_Y += SlideProgress;
 } else {
 CircleCenter_Y -= SlideProgress;
 }
 
 }
 circle( CircleCenter_X, CircleCenter_Y, CircleSize );
 
 // Has selected circle completed its slide to the empty space?
 
 if (SlideProgress === SquareSize ) {
 Circles[EmptyRow][EmptyCol] = Circles[SelectedRow][SelectedCol];
 Circles[SelectedRow][SelectedCol] = EmptySpace;
 EmptyRow = SelectedRow;
 EmptyCol = SelectedCol;
 GameState = StartedState;
 }
 }
 }
 }
 }
 
 if( GameState === CircleSelectedState ) {
 GameState = CircleSlidingState; // Slide will start in next drawing frame
 InitialframeCount = frameCount;
 }
 
 // Does top row match the bottom row colorwise? In other words, has user solved the game?
 
 if( 
 Circles[0][0] === Circles[2][0] &&
 Circles[0][1] === Circles[2][1] &&
 Circles[0][2] === Circles[2][2] &&
 Circles[0][3] === Circles[2][3] ) {
 
 GameState = SolvedState;
 DrawHappyFace();
 }
 
} // end DisplayCircles
function DrawHappyFace() {
 
 // Draw happy face to indicate successful solve
 
 FaceCenter_X = LeftBorder + EmptyCol*SquareSize + SquareHalfSize;
 FaceCenter_Y = TopBorder + EmptyRow*SquareSize + SquareHalfSize;
 FaceRadius = floor( 0.50 * CircleSize );
 
 EyeOffset_X = floor( 0.30 * FaceRadius );
 EyeOffset_Y = floor( 0.30 * FaceRadius );
 EyeWidth = floor( 0.22 * FaceRadius );
 EyeHeight = floor( 0.40 * FaceRadius );
 
 BottomSmileOffset_Y = floor( 0.15 * FaceRadius );
 BottomSmileWidth = floor( 1.20 * FaceRadius );
 BottomSmileHeight = floor( 0.95 * FaceRadius );
 
 TopSmileOffset_Y = BottomSmileOffset_Y;
 TopSmileWidth = floor( 1.30 * FaceRadius );
 TopSmileHeight = floor( 0.55 * FaceRadius );
 
 fill('yellow');
 circle( FaceCenter_X, FaceCenter_Y, CircleSize ); // Round face
 
 fill('black');
 ellipse( FaceCenter_X-EyeOffset_X, FaceCenter_Y-EyeOffset_Y, EyeWidth, EyeHeight ); // Left
 ellipse( FaceCenter_X+EyeOffset_X, FaceCenter_Y-EyeOffset_Y, EyeWidth, EyeHeight ); // Right
 
 arc( FaceCenter_X, FaceCenter_Y+BottomSmileOffset_Y, // Bottom of smile
 BottomSmileWidth, BottomSmileHeight, 0, PI);
 
 fill('yellow');
 arc( FaceCenter_X, FaceCenter_Y+TopSmileOffset_Y, // Top of smile
 TopSmileWidth, TopSmileHeight, 0, PI);
} // end DrawHappyFace
function NewGame() {
 
 // Randomly shuffle the circles for a new game.
 // Using the variable <RowMatches>, ensure that the new game is not already completely solved.
 
 RowMatches = 999; // Dummy value to ensure at least one reshuffling
 while( RowMatches !== 0 ) {
 for (row = 0; row <= 2; row += 1) {
 for (col = 0; col <= 3; col += 1) {
 NewRow = floor(random(3));
 NewCol = floor(random(4));
 
 SaveColor = Circles[row][col];
 Circles[row][col] = Circles[NewRow][NewCol];
 Circles[NewRow][NewCol] = SaveColor;
 }
 }
 
 // Compute the number of positions in the top row where the circle's color ...
 // ... matches the circle's color in the corresponding position in the bottom row.
 
 RowMatches = 0;
 for (col = 0; col <= 3; col += 1) {
 if( Circles[0][col] === Circles[2][col] ) {
 RowMatches++;
 }
 }
 
 }
 
 FindEmptySpace();
 
 // Randomize the colors
 
 for (i = 0; i < ColorList.length; i += 1) {
 j = floor(random(ColorList.length));
 SaveColor = ColorList[i];
 ColorList[i] = ColorList[j];
 ColorList[j] = SaveColor;
 }
 
} // end NewGame
function FindEmptySpace() {
 
 // Determine the position of the Empty Space in the game grid
 
 for (row = 0; row <= 2; row += 1) {
 for (col = 0; col <= 3; col += 1) {
 if( Circles[row][col] === EmptySpace ) {
 EmptyRow = row;
 EmptyCol = col;
 }
 }
 }
} // end FindEmptySpace
function preload() {
 
 // Preload introduction to the game
 
 UserHelp = loadImage('UserHelp.png');
 
} // end preload
function DisplayIntroduction() {
 
 // Displays an introduction of the game for the user's benefit
 
 background('#005678');
 
 image(UserHelp, 0, 0, width, height, 0, 0, UserHelp.width, UserHelp.height, CONTAIN);
 
} // end DisplayIntroduction
asked Aug 11 at 22:04
\$\endgroup\$
2
  • 1
    \$\begingroup\$ The puzzle works, for those who don't want to test it. \$\endgroup\$ Commented Aug 11 at 23:46
  • 1
    \$\begingroup\$ (it's a simple but fun game and it runs well though, so your loss) \$\endgroup\$ Commented Aug 12 at 0:46

1 Answer 1

8
\$\begingroup\$

Things that are good:

  • Extensive commenting, sometimes slightly too much, but helpful none-the-less.
  • The UI and game board variables have been grouped together.
  • The game has clear states, which makes reading through the code that much easier.
  • While the variable names are not correctly formatted they do convey their use well.

Coding issues

There are too few functions defined.

  • In touchStarted for instance there is code to see what is being clicked. That could be in a separate function, decoupled from the state handling.
  • In DisplayCircles it almost by definition makes sense to have it call a DisplayCircle function, just like it has a DrawHappyFace. As it also contains game logic as well as a call to DrawHappyFace the function clearly tries to do too much, especially given its name.

The way to handle variables is unclear, and there are too many global variables. It is often unclear where those are changed and set. This generally makes the code much harder to maintain. Maybe this is because the user isn't yet familiar with constants and classes. Constants can be defined globally and are set directly. Classes can be used to group functionality. For instance, you could calculate the right sizes of the elements once and then pass that around as some kind of graphics context structure.

This use of global variables also appears in functions that do not seem to have side effects, but actually do. A side effect occurs when a function changes some part of the application's state. For example, function FindEmptySpace() modifies two global variables. Any side effects a function has should be documented. However, we generally try to avoid them whenever possible. In this case FindEmptySpace could simply return a position, allowing the calling code to set an empty variable.

In general:

  • We try and split up the problem in as many functions that make sense.
  • We try and minimize the mutable state size, e.g. by using constants instead of global variables.
  • Try and minimize the amount of locations in the code where state changes. This can be done by grouping variables together (as fields in classes for instance) and by having functions that return results rather than changing fields or global variables.

Another way of having less state - and in your case represented by global variables - is to only calculate variables where they are required. E.g. (re-)calculating SquareHalfSize will be easy enough for a processor, so it can be calculated locally in a function rather than globally.

Comments on specific parts

let ColorList = ['red', 'yellow', 'lime', 'aqua', 'fuchsia', 'white'];

Generally variable names start with a lowercase, so lower camelCase. Constants - variables that do not change are written like COLOR_LIST, so

const COLOR_LIST = ['red', 'yellow', 'lime', 'aqua', 'fuchsia', 'white'];

let EmptyRow, EmptyCol; // Position of empty space in grid

In the end it generally pays off to have an class / structure, whatever for a position. I would not call this EmptyRow if there isn't an empty row. EmptyX would already be an improvement, but a Postition reading empty.x seems even better to me.


let StartedState = 100; // State: Game has started

You'd use something like enums. Now JavaScript doesn't have enums, but we can simulate this using constants:

const GameState = Object.freeze({
 INTRODUCTION: 50, // Introduction is displayed for the user
 STARTED: 100, // Game has started
 CIRCLE_SELECTED: 101,// User successfully selected a circle to be slid
 CIRCLE_SLIDING: 102, // Currently sliding the user selected circle
 SOLVED: 103 // User has won the game (top row = bottom row)
});

Style issues

Sometimes the spacing is just slightly off.

let TopBorder;

Uses an extra space between let and TopBorder.


SquareSize = min( windowHeight/3, windowWidth/4 );

... should probably be written like ...

SquareSize = min(windowHeight / 3, windowWidth / 4);

There are generally good guidelines about how to write expressions, and I'd keep as much to the defaults for the platform as possible; you'll quickly get used to it and it makes all code easier to read.

Some IDE's will allow you to select a part of an expression (or put the cursor in front or within it) and then use e.g. alt-up-arrow to select larger and larger scopes. That way you can determine which operators etc. take precedent.


Sometimes your functions start with upper and sometimes with lowercase characters. Functions and methods should start with a lowercase. Class constructors are the odd one out, as they have the same name as the class itself.

For more information on this look at a style guide such as the Google style guide (link to the naming part). You'll also find that most JavaScript API's will use the same style.


let Circles = [ [3,2,0,2], [0,1,1,2], [0,2,0,-1] ]; // 3x4 playing grid

I personally don't like comments that are at the end of lines, because it either looks messy or you have to spend time to indent them properly, which is then messed up when you e.g. change a variable name. Putting a comment in front of the lines should be prefered.


// end Create3x4GridSizes

This is really not needed, and it is kinda dangerous when the function is renamed for some reason. Most IDE's and text editors will automatically indicate the end of a function, which should be good enough.

answered Aug 12 at 0:39
\$\endgroup\$
10
  • \$\begingroup\$ "Methods except for constructors should start with a lowercase" per idiomatic JavaScript? style guide(s)? \$\endgroup\$ Commented Aug 12 at 0:55
  • 1
    \$\begingroup\$ @SᴀᴍOnᴇᴌᴀ In this JS is very much like C or Java from what I see. Most API's clearly use MyClass.myMethod and of course myVariable and MY_CONST. I guess you'll find this in most style guides such as the one published by Google. \$\endgroup\$ Commented Aug 12 at 1:24
  • \$\begingroup\$ @MaartenBodewes +1 Thanks for the thorough review. I am glad you found the game fun. \$\endgroup\$ Commented Aug 12 at 4:53
  • 1
    \$\begingroup\$ You could argue that constants and configuration options also are part of the state of an application (or runtime module of any kind). So the state is basically everything but the code itself during runtime. The mutable state is everything that can be changed (or assigned-to) during runtime. Generally however, we exclude the inner state of the functions, so in your case the state consists of all the global variables. \$\endgroup\$ Commented Aug 12 at 21:02
  • 1
    \$\begingroup\$ If you'd use OO or structures then a global var could e.g. contain a set of objects with their own fields. Those would all be part of the state. However, if you'd have a set of immutable objects rather than a set of mutable object then you'd still have fewer possible states in your application. In general we strive for applications with a small state and of course we try to avoid invalid state, as in that case your application will become a mess. \$\endgroup\$ Commented Aug 12 at 21:06

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.