11
\$\begingroup\$

This is my first attempt at building a full project. I've recently learned using prototypical inheritance in JS. Please tell me how I can improve the quality of this code.

The game is available here: http://tic-tac-toe-realtime.herokuapp.com

index.js file for the server:

var express = require('express');
var bodyParser = require('body-parser')
var app = express();
var server = require('http').Server(app);
var io = require('socket.io')(server);

var rooms = 0;

app.use(bodyParser.urlencoded({
    extended: true
}));
app.use(express.static('.'));

app.get('/', function (req, res) {
    res.sendFile(__dirname + '/game.html');
});

io.on('connection', function(socket){
    socket.on('createGame', function(data){
        socket.join('room-' + ++rooms);
        socket.emit('newGame', {name: data.name, room: 'room-'+rooms});
    });

    socket.on('joinGame', function(data){
        var room = io.nsps['/'].adapter.rooms[data.room];
        if( room && room.length == 1){
            socket.join(data.room);
            console.log(io.nsps['/'].adapter.rooms[data.room]);
            socket.broadcast.to(data.room).emit('player1', {});
            socket.emit('player2', {name: data.name, room: data.room })
        }
        else {
            socket.emit('err', {message: 'Sorry, The room is full!'});
        }
    });

    socket.on('playTurn', function(data){
        socket.broadcast.to(data.room).emit('turnPlayed', {
            tile: data.tile,
            room: data.room
        });
    });
    socket.on('gameEnded', function(data){
        socket.broadcast.to(data.room).emit('gameEnd', data);
    })
})

server.listen(process.env.PORT || 5000);

main.js file for the client side:

'use strict';
(function() {

    var P1 = 'X', P2 = 'O';
    //Connect to Socket.IO
    var socket = io.connect('http://tic-tac-toe-realtime.herokuapp.com'),
    // var socket = io.connect('http://localhost:5000'),
        player,
        game;

    //Game Class Definition

    var Game = function(roomId){
        this.roomId = roomId;
        this.board = [];
        this.moves = 0;
    }

    Game.prototype.getRoomId = function(){
        return this.roomId;
    }

    Game.prototype.displayBoard = function(message){
        $('.menu').css('display', 'none');
        $('.gameBoard').css('display', 'block');
        $('#userHello').html(message);
        this.createGameBoard();
    }

    Game.prototype.createGameBoard = function(){
        for(var i=0; i<3; i++) {
            this.board.push(['','','']);
            for(var j=0; j<3; j++) {
                $('#button_' + i + '' + j).on('click', function(){

                    //Check for turn
                    if(!player.getCurrentTurn()){
                        alert('Its not your turn!');
                        return;
                    }

                    //Error on playing same button again.
                    if($(this).prop('disabled')){
                        alert('This tile has already been played on!');
                    }

                    //Update board after your turn.
                    var row = parseInt(this.id.split('_')[1][0]);
                    var col = parseInt(this.id.split('_')[1][1]);
                    game.playTurn(this);
                    game.updateBoard(player.getPlayerType(), row, col, this.id);

                    player.setCurrentTurn(false);
                    player.updatePlaysArr(1 << (row * 3 + col));

                    game.checkWinner();

                    return false;
                });
            }
        }
    }

    Game.prototype.updateBoard = function(type, row, col, tile){
        $('#'+tile).text(type);
        $('#'+tile).prop('disabled', true);
        this.board[row][col] = type;
        this.moves ++;
    }

    Game.prototype.playTurn = function(tile){
        var clickedTile = $(tile).attr('id');
        var turnObj = {
            tile: clickedTile,
            room: this.getRoomId()
        };
        // Emit an event to update other player that you've played your turn.
        socket.emit('playTurn', turnObj);
    }

    Game.prototype.endGame = function(message){
        alert(message);
        location.reload();
    }

    /*
    * To determine a win condition, each square is "tagged" from left
    * to right, top to bottom, with successive powers of 2.  Each cell
    * thus represents an individual bit in a 9-bit string, and a
    * player's squares at any given time can be represented as a
    * unique 9-bit value. A winner can thus be easily determined by
    * checking whether the player's current 9 bits have covered any
    * of the eight "three-in-a-row" combinations.
    *
    *     273                 84
    *        \               /
    *          1 |   2 |   4  = 7
    *       -----+-----+-----
    *          8 |  16 |  32  = 56
    *       -----+-----+-----
    *         64 | 128 | 256  = 448
    *       =================
    *         73   146   292
    *
    *  We have these numbers in the Player.wins array and for the current 
    *  player, we've stored this information in playsArr.
    */
    Game.prototype.checkWinner = function(){        
        var currentPlayerPositions = player.getPlaysArr();
        Player.wins.forEach(function(winningPosition){
            if(winningPosition & currentPlayerPositions == winningPosition){
                game.announceWinner();
            }
        });

        var tied = this.checkTie();
        if(tied){
            socket.emit('gameEnded', {room: this.getRoomId(), message: 'Game Tied :('});
            alert('Game Tied :(');
            location.reload();  
        }
    }

    Game.prototype.checkTie = function(){
        return this.moves >= 9;
    }

    Game.prototype.announceWinner = function(){
        var message = player.getPlayerName() + ' wins!';
        socket.emit('gameEnded', {room: this.getRoomId(), message: message});
        alert(message);
        location.reload();
    }

    var Player = function(name, type){
        this.name = name;
        this.type = type;
        this.currentTurn = true;
        this.playsArr = 0;
    }

    Player.wins = [7, 56, 448, 73, 146, 292, 273, 84];

    Player.prototype.updatePlaysArr = function(tileValue){
        this.playsArr += tileValue;
    }

    Player.prototype.getPlaysArr = function(tileValue){
        return this.playsArr;
    }

    Player.prototype.setCurrentTurn = function(turn){
        this.currentTurn = turn;
        if(turn)
            $('#turn').text('Your turn.');
        else
            $('#turn').text('Waiting for Opponent');
    }

    Player.prototype.getPlayerName = function(){
        return this.name;
    }

    Player.prototype.getPlayerType = function(){
        return this.type;
    }

    Player.prototype.getCurrentTurn = function(){
        return this.currentTurn;
    }

    //Create a new game.
    $('#new').on('click', function(){
        var name = $('#nameNew').val();
        if(!name){
            alert('Please enter your name.');
            return;
        }
        socket.emit('createGame', {name: name});
        player = new Player(name, P1);
    });

    //Join an existing game
    $('#join').on('click', function(){
        var name = $('#nameJoin').val();
        roomID = $('#room').val();
        if(!name || !roomID){
            alert('Please enter your name and game ID.');
            return;
        }
        socket.emit('joinGame', {name: name, room: roomID});
        player = new Player(name, P2);
    });

    //New Game created. Update UI.
    socket.on('newGame', function(data){

        var message = 'Hello, ' + data.name + 
            '. Please ask your friend to enter Game ID: ' +
            data.room + '. Waiting for player 2...';

        // Create game for player 1
        game = new Game(data.room);
        game.displayBoard(message);     
    });

    //If player creates the game, He is the the host
    socket.on('player1', function(data){        
        var message = 'Hello, ' + player.getPlayerName();

        // Reset the message for the player
        $('#userHello').html(message);

        // Set the current player's turn
        player.setCurrentTurn(true);
    });

    //Joined the game, so player is player 2
    socket.on('player2', function(data){
        var message = 'Hello, ' + data.name;

        //Create game for player 2
        game = new Game(data.room);
        game.displayBoard(message);

        // First turn is of player 1, so set to false
        player.setCurrentTurn(false);   
    }); 

    //Opponent played his turn. Update UI.
    socket.on('turnPlayed', function(data){
        var row = data.tile.split('_')[1][0];
        var col = data.tile.split('_')[1][1];
        var opponentType = player.getPlayerType() == P1 ? P2 : P1;
        game.updateBoard(opponentType, row, col, data.tile);
        player.setCurrentTurn(true);
    });

    socket.on('gameEnd', function(data){
        game.endGame(data.message);
    })

    socket.on('err', function(data){
        game.endGame(data.message);
    });
})();

index.html for the UI:

<!DOCTYPE html>
<html>
    <head>
        <title>Tic Tac Toe</title>
        <link rel="stylesheet" href="main.css">
        <link rel="stylesheet" href="node_modules/skeleton-css/css/skeleton.css">
    </head>
    <body>
        <div class="container">
            <div class="menu">
                <h1>Tic - Tac - Toe</h1>
                <h3>How To Play</h3>
                <ol>
                    <li>Player 1 Create a new game by entering the username</li>
                    <li>Player 2 Enter another username and the room id that is displayed on first window.</li>
                    <li>Click on join game. </li>
                </ol>
                <h4>Create a new Game</h4>
                <input type="text" name="name" id="nameNew" placeholder="Enter your name" required>
                <button id="new">New Game</button>
                <br><br>
                <h4>Join an existing game</h4>
                <input type="text" name="name" id="nameJoin" placeholder="Enter your name" required>
                <input type="text" name="room" id="room" placeholder="Enter Game ID" required>
                <button id="join">Join Game</button>
            </div>
            <div class="gameBoard">
                <h2 id="userHello"></h2>
                <h3 id="turn"></h3>
                <table class="center">
                    <tr>
                        <td><button class="tile" id="button_00"></button></td>
                        <td><button class="tile" id="button_01"></button></td>
                        <td><button class="tile" id="button_02"></button></td>
                    </tr>
                    <tr>
                        <td><button class="tile" id="button_10"></button></td>
                        <td><button class="tile" id="button_11"></button></td>
                        <td><button class="tile" id="button_12"></button></td>
                    </tr>
                    <tr>
                        <td><button class="tile" id="button_20"></button></td>
                        <td><button class="tile" id="button_21"></button></td>
                        <td><button class="tile" id="button_22"></button></td>
                    </tr>
                </table>
            </div>
        </div>
        <script src="node_modules/jquery/dist/jquery.min.js"></script>
        <script src="/socket.io/socket.io.js"></script>
        <script src="main.js"></script>
    </body>
</html>
\$\endgroup\$
  • \$\begingroup\$ So, the checks for "Tile alredy used" and "Not your turn" are on the client side? What if I change the code on my side (I'm the client after all) so that I set a tile ignoring those checks? \$\endgroup\$ – ChatterOne Feb 10 '17 at 14:58
  • \$\begingroup\$ @ChatterOne I didn't think of that! Yes then the client can cheat. I'll try to check it on the server as well for every room. \$\endgroup\$ – ayushgp Feb 10 '17 at 15:26
  • \$\begingroup\$ Definitely try checking/recording the game state on the server. Also, you should maybe ignore playTurn messages unless they're from the "active" player. Otherwise it could be very easy to just send 3 playTurns in a row, and win the game immediately \$\endgroup\$ – Flambino Feb 12 '17 at 22:45
5
+50
\$\begingroup\$

Server-side code

Looks pretty good!

However, as discussed in the comments, your server should probably be checking the game state to curtail cheating. As it is, it's pretty easy to "win" by fiddling with the client side code.

The server should:

  1. Keep the canonical game state
  2. Only respect a single playTurn message from the player who's turn it is
  3. Determine the winner (or that the game's a tie)

I'd also recommend using something like a UUID to label rooms and players. With UUIDs, it's practically impossible for someone to guess a room or player ID. Of course, a regular v4 UUID isn't terribly user-friendly for the Player 2 who has to join, so you might consider something shorter, like this.

The bitmask trick used on the client-side for determining the board state is neat. Using it for two players on the server means tracking two bitmasks, but it's fairly easy to check if a tile is "taken" already (pseudo-code):

var newMark = 32; // sent from client either as-is or calculated from row/col or index
if (newMark & (player1.marks | player2.marks)) {
  // tile is already marked
}

And checking for a full board would be (player1.marks | player2.marks) === 511.

Client-side

Also looks pretty nice, though of course most should be moved to the server. Ideally, the client-side code is "thin" and the server-side code is "fat".

In all there's just too much going on in the client-side code. Parsing button IDs, checking game state, and so on.

I also don't like the io.connect line(s), since it's error prone. In fact, at the time of writing, your Heroku site doesn't work because it's trying to connect to localhost:5000 instead of your server. You've got the wrong line commented out in the deployed code, so I haven't actually been able to play the game...

Instead use something like:

io.connect(window.location.protocol + "//" + window.location.host);

and it'll work both on the server and on your local machine, since it's just taking the page's address (and location.host includes the port number).

The client-side could also build the board instead of having it in the HTML. Of course it's not like the board changes, so it can be in static HTML, but building the board in code can make it easier to add event listeners etc..

One niggling thing: I wouldn't use a table element for the board. I'd use plain buttons and CSS to lay them out correctly. Semantically, a tic-tac-toe board isn't quite a table (though it's darn close and an argument could be made either way).

E.g. for building and laying out the board:

function createButton(index) {
  return $('<button></button>')
    .addClass('tile')
    .on('click', function (event) {
      // Add code to send the relevant index/code/row-and-column to the server
      // This is just a demo
    
      var row = index / 3 | 0,
        col = index % 3,
        code = Math.pow(2, index);
    
      alert("Clicked button " + index + " at (" + row + ", " + col + ") with code " + code);
    
      return false;
    });
}

for(var i = 0; i < 9; i++) {
  createButton(i).appendTo(document.body);
}
button.tile {
  display: block;
  float: left;
  border: 1px solid black;
  width: 30px;
  height: 30px;
}

button.tile:nth-of-type(3n) {
  float: none;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

And you can always get all the tiles with $('button.tile') when you want to iterate them, e.g. when setting the board to match what the server sent.

Speaking of, the server can just send the two numbers that indicate the two players's marks. Say, 17 for player 1, and 260 for player 2, which you can then use to set the appropriate button states:

// received from server
var playerMask = 260;
var playerMark = "O";

$('button.tile').each(function () {
  if (playerMask & 1) {
    $(this).text(playerMark).prop('disable', true);
  }
  playerMask = playerMask >> 1; // shift a bit off the end
});

The above will mark button 3 and 9 with an O and disable them. The above modifies the playerMask value, but you could avoid that by using the element index:

$('button.tile').each(function (i) {
  if ((playerMask >> i) & 1) {
    $(this).text(playerMark).prop('disable', true);
  }
  // ...
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.