Real time Tic Tac Toe game in Javascript using Socket.IO

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

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:

• Keep the canonical game state

• Only respect a single playTurn message from the player who's turn it is

• 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 $('')
.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
button.tile {
display: block;
float: left;
border: 1px solid black;
width: 30px;
height: 30px;
}

button.tile:nth-of-type(3n) {
float: none;
}




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);
  }
  // ...

StackExchange Code Review Q#155011, answer score: 7