10
\$\begingroup\$

I wrote a Pacman game in Phaser, but I think I have far too many lines of code. Here is the main code.

Phaser uses states to store different aspects of a game. The main state is called level state.

var levelState = {

    buildMap: function() {
        map = this.game.add.tilemap('map1');
        map.addTilesetImage('pacman_tiles', 'pacmanTiles');
        map.addTilesetImage('pacman_tiles_2', 'pacmanTiles2');
        wallLayer = map.createLayer('WallLayer');
        dotsLayer = map.createLayer('DotsLayer');
        map.setCollisionBetween(1, 1000, true, 'WallLayer');
    },

    addPacMan: function() {
        pacman = this.game.add.sprite(gameStats.pacmanStartingX, gameStats.pacmanStartingY, 'pacman');
        pacman.anchor.setTo(0.5);
    },

    addGhosts: function() {
        ghosts = this.game.add.group();

        redGhost = ghosts.create(155, 255, 'redghost');
        orangeGhost = ghosts.create(180, 255, 'orangeghost');
        pinkGhost = ghosts.create(205, 255, 'pinkghost');
        turquoiseGhost = ghosts.create(230, 255, 'turquoiseghost');

        ghosts.setAll('anchor.x', 0.5);
        ghosts.setAll('anchor.y', 0.5);
        ghosts.setAll('frame', 0);

        ghosts.forEachExists(ghost => ghost.mobilized = false);

    },

    addAnimations: function() {
        pacman.animations.add('pacmanRight', [0,1], 3, true);
        pacman.animations.add('pacmanLeft', [4,5], 3, true);
        pacman.animations.add('pacmanUp', [6,7], 3, true);
        pacman.animations.add('pacmanDown', [2,3], 3, true);

        ghosts.forEach(function(ghost) {
            ghost.animations.add('move', [0,1,2,3,4,5,6,7], 2, true);
            ghost.animations.add('vulnerable', [8,9], 2, true);
            ghost.animations.add('warning', [10,11], 2, true);
            ghost.animations.add('retreat', [12,13], 2, true);
        });

    },

    animateGhosts: function() {
        ghosts.forEachExists(ghost => ghost.animations.play('move'));
    },

    initGraphics: function() {
        this.buildMap();
        this.addPacMan();
        this.addGhosts();
        this.addAnimations();
        this.animateGhosts();
    },

In Pacman, all ghosts have the same velocity except for the red ghost. This function is used to get a ghost's velocity.

    getGhostVelocity: function(ghost) {
        return ghost == redGhost ? gameStats.redGhostVelocity : gameStats.ghostVelocity;
    },

This function moves ghosts out of their ghost house

    mobilizeGhosts: function(ghost) {

        if (ghost == redGhost) {
            this.game.time.events.add(gameStats.redGhostMoveDelay, function() {
                ghost.body.velocity.y = -gameStats.ghostVelocity;
            });
        }

        else if (ghost == orangeGhost) {
            this.game.time.events.add(gameStats.orangeGhostMoveDelay, function() {
                ghost.body.velocity.y = -gameStats.ghostVelocity;
            });
        }

        else if (ghost == pinkGhost) {
            this.game.time.events.add(gameStats.pinkGhostMoveDelay, function() {
                ghost.body.velocity.y = -gameStats.ghostVelocity;
            });
        }

        else {
            this.game.time.events.add(gameStats.turquoiseGhostMoveDelay, function() {
                ghost.body.velocity.y = -gameStats.ghostVelocity;
            });

        }

        ghost.direction = 'up';
        ghost.body.velocity.x = 0;

    },

    initPhysics: function() {
        //this.game.time.events.add(3000, function() {
            this.game.physics.arcade.enable(pacman);
            this.game.physics.arcade.enable(ghosts);
            pacman.body.velocity.x = gameStats.pacmanVelocity;
            pacman.body.immovable = true;

            ghosts.setAll('body.immovable', true);

            ghosts.forEachExists(ghost => levelState.mobilizeGhosts(ghost));
            gameStats.inPlay = true;
        //});
    },

    initKeyboard: function() {
        cursors = this.game.input.keyboard.createCursorKeys();
    },

    create: function() {
        this.initGraphics();
        this.initPhysics();
        this.initKeyboard();
        initScoreBoard();
    },

These functions I think can be optimized. I am using Tiled, so I have to calculate the characters position in tiles. The next four functions are almost identical.

    updatePacmanPosition: function() {
        pacmanX = Math.round(pacman.x / map.tileWidth);
        pacmanY = Math.round(pacman.y / map.tileHeight);
    },

    pathRightAvailable: function() {
        var index1 = map.getTileRight(0, pacmanX, pacmanY).index;
        var index2 = map.getTileRight(0, pacmanX + 1, pacmanY).index;
        var index3 = map.getTileRight(0, pacmanX + 2, pacmanY).index;
        return [index1, index2, index3].every(index => index == -1);
    },

    pathLeftAvailable: function() {
        var index1 = map.getTileLeft(0, pacmanX, pacmanY).index;
        var index2 = map.getTileLeft(0, pacmanX - 1, pacmanY).index;
        var index3 = map.getTileLeft(0, pacmanX - 2, pacmanY).index;
        return [index1, index2, index3].every(index => index == -1);
    },

    pathUpAvailable: function() {
        var index1 = map.getTileAbove(0, pacmanX, pacmanY).index;
        var index2 = map.getTileAbove(0, pacmanX, pacmanY - 1).index;
        var index3 = map.getTileAbove(0, pacmanX, pacmanY - 2).index;
        return [index1, index2, index3].every(index => index == -1);
    },

    pathDownAvailable: function() {
        var index1 = map.getTileBelow(0, pacmanX, pacmanY).index
        var index2 = map.getTileBelow(0, pacmanX, pacmanY + 1).index;
        var index3 = map.getTileBelow(0, pacmanX, pacmanY + 2).index
        return [index1, index2, index3].every(index => index == -1);
    },

Same with these functions. These control the movement. It looks like there is a little code duplication here and these can be more concise.

    moveLeft: function() {
        if (cursors.left.justPressed()) {
            pacman.direction = 'left';
            pacman.body.velocity.y = 0;
            pacman.body.velocity.x = -(gameStats.pacmanVelocity);
            pacman.frame = 4;
            pacman.animations.stop();
            pacman.animations.play('pacmanLeft');
        }
    },

    moveRight: function() {
        if (cursors.right.justPressed()) {
            pacman.direction = 'right';
            pacman.body.velocity.y = 0;
            pacman.body.velocity.x = gameStats.pacmanVelocity;
            pacman.frame = 0;
            pacman.animations.stop();
            pacman.animations.play('pacmanRight');
        }
    },

    moveUp: function() {
        if (cursors.up.justPressed() && this.pathUpAvailable()) {
            pacman.direction = 'up';
            pacman.body.velocity.x = 0;
            pacman.body.velocity.y = -gameStats.pacmanVelocity;
            pacman.frame = 6;
            pacman.animations.stop();
            pacman.animations.play('pacmanUp');
        }
    },

    moveDown: function() {
        if (cursors.down.justPressed() && this.pathDownAvailable()) {
            pacman.direction = 'down';
            pacman.body.velocity.x = 0;
            pacman.body.velocity.y = gameStats.pacmanVelocity;
            pacman.frame = 2;
            pacman.animations.stop();
            pacman.animations.play('pacmanDown');
        }
    },

This is the main movement function which is broken up into separate functions for each direction. There are also functions for wrapping around the screen, running into a wall and eating the pellets.

    movePacMan: function() {
        this.moveLeft();
        this.moveRight();
        this.moveUp();
        this.moveDown();
        this.wrapAround();
        this.hitWall();
        this.chompDots();
    },

This function determines the ghosts movement. At the start of the level, ghosts can pass through the ghost house wall, but after they pass through, they will no longer be able to hit walls. This is where I set the mobilized flag on the ghosts. I have four different functions that determine the ghosts movement based on its direction

    moveGhosts: function() {
        ghosts.forEachExists(function(ghost) {

            if (ghost.y <= 210) {
                ghost.mobilized = true;
            }

            if (ghost.mobilized) {
                this.game.physics.arcade.collide(ghost, wallLayer, function() {

                    if (ghost.direction == 'up' ) {
                        ghost.y++;

                        var direction = Math.random() > 0.5 ? 'left' : 'right';
                        var ghostVelocity = levelState.getGhostVelocity();
                        ghost.body.velocity.y = 0;
                        ghost.body.velocity.x = direction == 'left' ? -ghostVelocity : ghostVelocity;
                        ghost.direction = direction;
                    }

                    else if (ghost.direction == 'down') {
                        ghost.y--;
                        var direction = Math.random() > 0.5 ? 'left' : 'right';
                        var ghostVelocity = levelState.getGhostVelocity();
                        ghost.body.velocity.y = 0;
                        ghost.body.velocity.x = direction == 'left' ? -ghostVelocity : ghostVelocity;
                        ghost.direction = direction;
                    }

                    else if (ghost.direction == 'left') {
                        ghost.x++;
                        var direction = Math.random() > 0.5 ? 'up': 'down';
                        var ghostVelocity = levelState.getGhostVelocity();

                        ghost.body.velocity.x = 0;
                        ghost.body.velocity.y = direction == 'up' ? -ghostVelocity : ghostVelocity;
                        ghost.direction = direction;
                    }

                    else {
                        ghost.x--;
                        var direction = Math.random() > 0.5 ? 'up' : 'down';
                        var ghostVelocity = levelState.getGhostVelocity();

                        ghost.body.velocity.x = 0;
                        ghost.body.velocity.y = direction == 'up' ? -ghostVelocity : ghostVelocity;
                        ghost.direction = 'down';
                    }


                }, null, this);
            }
        });
    },

In the levelUp function, the number of dots resets as does the fruit flag. The invincibility and warning times granted by eating the power pellets also decreases.

    levelUp: function() {
        gameStats.level++;
        gameStats.inPlay = false;
        gameStats.dotsLeft = 205;
        gameStats.fruitAdded = false;

        if (gameStats.invincibilityTime > 0) {
            gameStats.invincibilityTime -= 1000;
            gameStats.warningTime -= 300;
        }

        else {
            if (gameStats.warningTime > 0) {
                gameStats.warningTime -= 500;
            }
        }

        pacman.animations.stop();
        ghosts.forEachExists(ghost => ghost.animations.stop());
        this.game.state.start('levelup');
    },

This is called after losing a life. Pacman returns to the starting position

    resetPacman: function() {
        pacman.body.velocity.x = gameStats.pacmanVelocity;
        pacman.body.velocity.y = 0;
        pacman.x = gameStats.pacmanStartingX;
        pacman.y = gameStats.pacmanStartingY;
        pacman.frame = 0;
    },

This function returns the ghosts to their ghost house

    resetGhost: function(ghost) {
        switch (ghost) {
            case redGhost: ghost.position.setTo(155,255); break;
            case orangeGhost: ghost.position.setTo(180, 255); break;
            case pinkGhost: ghost.position.setTo(205, 255); break;
            case turquoiseGhost: ghost.position.setTo(230, 255); break;
        }
        ghost.mobilized = false;
    },

    removeLifeIcons: function() {
        switch (gameStats.lives) {
            case 3: lifeIcon3.kill(); break;
            case 2: lifeIcon2.kill(); break;
            case 1: lifeIcon1.kill(); break;
        }
    },

    loseLife: function() {
        levelState.resetPacman();
        ghosts.forEachExists(ghost => levelState.resetGhost(ghost));
        levelState.removeLifeIcons();
        gameStats.lives--;
        gameStats.inPlay = false;
        ghosts.forEach(ghost => levelState.mobilizeGhosts(ghost));

        if (gameStats.lives == 0) {
            levelState.gameOver();
        }

        this.game.time.events.add(500, function() { gameStats.inPlay = true });
    },

    gameOver: function() {
        pacman.animations.stop();
        ghosts.forEachExists(ghost => ghost.animations.stop());
        game.state.start('gameover');
    },

    displayHitScore: function() {
        ghostScore = game.add.text(pacman.x - 50, pacman.y, gameStats.ghostPointValue, { 'fill': 'white' });
        ghostScore.anchor.setTo(0.5);
        ghostScore.lifespan = 500;
    },

Detect collisions between Pacman and a ghost.

    hitGhost: function() {
        this.game.physics.arcade.collide(pacman, ghosts, function(sprite, ghost) {
            if (gameStats.inPlay) {
                if (gameStats.invincible) {
                    ghost.animations.play('retreat');
                    levelState.resetGhost(ghost);

                    game.time.events.add(3000, function() {
                        levelState.mobilizeGhosts(ghost);
                    });

                    levelState.displayHitScore();

                    gameStats.score += gameStats.ghostPointValue;
                    gameStats.ghostPointValue *= 2;

                }
                else {
                    levelState.loseLife();
                }
            }
        });
    },

If a wall is hit, Pacman stops moving.

    hitWall: function() {
        this.game.physics.arcade.collide(pacman, wallLayer, function(sprite, wall) {
            pacman.body.velocity.x = 0;
            pacman.animations.stop();
        }, null, this);
    },

This is kind of hard coded, but this determines the locations of the four power pellets. The locations are given in tiles, not pixels.

    powerPillLocation: function(object) {
        return ((object.x == 2 && (object.y == 64 || object.y == 65 || object.y == 14 || object.y == 15)) || 
                (object.x == 60 && (object.y == 14 || object.y == 15)) ||
                (object.x == 61 && (object.y == 64 || object.y == 65)));
    },

    transitionAnimation: function(sprite, animation1, animation2) {
        sprite.animations.stop(animation1);
        sprite.animations.play(animation2);
    },

    turnGhostsBlue: function() {
        ghosts.forEachExists(ghost => levelState.transitionAnimation(ghost, 'move', 'vulnerable'));
        gameStats.invincible = true;

    },

    revertGhosts: function() {
        ghosts.forEachExists(ghost => levelState.transitionAnimation(ghost, 'vulnerable', 'warning'));

        game.time.events.add(gameStats.warningTime, function() {
            ghosts.forEachExists(ghost => levelState.transitionAnimation(ghost, 'warning', 'move'));
            gameStats.invincible = false;
            gameStats.ghostPointValue = 200;
        });
    },

This function is called to eat the pellets.

    chompDots: function() {
        this.game.physics.arcade.overlap(pacman, dotsLayer, function(sprite, dot) {
            if (map.hasTile(dot.x, dot.y, dotsLayer)) {
                gameStats.dotsLeft--;
                map.removeTile(dot.x, dot.y, dotsLayer);
                gameStats.score += levelState.powerPillLocation(dot) ? 25 : 10;
                if (levelState.powerPillLocation(dot)) {
                    levelState.turnGhostsBlue();
                    ghostTimer = this.game.time.events.add(gameStats.invincibilityTime, levelState.revertGhosts);

                }

            }
        }, null, this);

        if (gameStats.dotsLeft == 0) {
            levelState.levelUp();
        }

    },

    collectFruit: function() {
        if (fruit) {
            this.game.physics.arcade.overlap(pacman, fruit, function() {
                fruit.destroy();
                var currentFruit = gameStats.fruits.find(fruit => fruit.levels.includes(gameStats.level));
                gameStats.score += currentFruit.value;
                fruitScore = game.add.text(190, 305, currentFruit.value, { 'fill': 'white', 'fontSize': 25 });
                fruitScore.anchor.setTo(0.5);
                fruitScore.lifespan = 1000;
            });
        }
    },

    wrapAround: function() {
        if (pacman.right >= game.world.right && pacman.direction == 'right') {
            pacman.right = 0;
        }
        if (pacman.left <= 0 && pacman.direction == 'left') {
            pacman.left = game.world.right;
        }
    },

    updateScore: function() {
        scoreDisplay.setText('Score: ' + gameStats.score);
        highScoreDisplay.setText('High Score: ' + Math.max(gameStats.score, gameStats.highScore));
        if (gameStats.score >= 10000 && !gameStats.livesEarned) {
            extraLife();
        }
    },

These are the functions that are called 60 times every second.

    update: function() {

        if (gameStats.inPlay) {
            this.movePacMan();
            this.updatePacmanPosition();
            this.moveGhosts();
            this.updateScore();
        }

        addFruit();
        this.hitGhost();
        this.collectFruit();
    }

}

As you can see the code is pretty self explanatory. I added descriptive variable names and functions to help the reader. Pacman is also a pretty famous game, so it's easy to understand its functionality.

My main concerns are.

  • It's too verbose. As the code base increases in scope, it becomes harder and harder to maintain. It also looks pretty ugly and hackish.
  • I think this requires too many calculations per second. I want to reduce the calculations so this runs faster on slower devices.

The original Pacman worked on computers in 1980. There's no way they required this many calculations. Where are the bottlenecks here?

For more information on the Phaser framework, please visit

https://phaser.io/

Edit:

Alright, by request I created a GitHub repository for this project. Here is the link

https://github.com/abjcaksd/Pacman

\$\endgroup\$
  • 3
    \$\begingroup\$ Welcome to CR! You know what would be awesome? Including an on-site embedded executable snippet! Hit Ctrl+M when editing, to supply HTML, CSS and JS code, much like a jsfiddle. Not sure about Phaser though, but definitely worth trying to make it work! \$\endgroup\$ – Mathieu Guindon May 26 '17 at 14:20
  • 3
    \$\begingroup\$ Minor disappointment that the ghosts aren't named inky (teal), blinky (red), pinky (pink) and clyde (orange) =) \$\endgroup\$ – Mathieu Guindon May 26 '17 at 14:24
  • \$\begingroup\$ That is not even remotely possible. In addition to the huge filesize (Phaser 1MB), this is only a fraction of my codebase. It also contains multiple assets such as images and tile maps. And because of the Same Origin Policy, it wouldn't work anyways \$\endgroup\$ – Pacman May 26 '17 at 14:51
  • 3
    \$\begingroup\$ I kind of expected that... that's excellent news for my workday though ;-) \$\endgroup\$ – Mathieu Guindon May 26 '17 at 14:52
  • \$\begingroup\$ I wrote a pacman some years ago that I recently posted on SO meta, it's not nearly as long as your code, but then from the overview, it also doesn't seem that I have the same amount of functionalities you have, great work you put it :) \$\endgroup\$ – Icepickle Jul 12 '17 at 22:16
1
\$\begingroup\$

First of all, I think that making this game is a great idea :-)

Adapting to the given framework

When you must work with the elements of a given framework, it may seem difficult to build your own data structures in a natural way. But it should be possible, with translation between your model and that of the framework where necessary.

For example I noticed that the movement functions do checks like cursors.left.justPressed(), for each direction. Instead of that, it would be great to get notified on arrow key press events, and record the last direction in lastDirection or something.

But let's assume the framework doesn't provide a way to do that. You could emulate it, by creating a helper function updateLastDirection:

function updateLastDirection() {
    if (cursors.down.justPressed() && this.pathDownAvailable()) {
        lastDirection = directions.down;
    } else if (cursors.up.justPressed() && this.pathUpAvailable()) {
        lastDirection = directions.up;
    } // ... and so on
}

You could call this right before you need lastDirection to be up to date. It's not quite as good as having lastDirection updated automatically, but it would work. The only cost compared to the chains of conditions on cursors.somedirection.justPressed() is one extra function call. I think that's a small price to pay for a clearer design that's more natural to read, and with less duplication.

As another similar example is this kind of code in mobilizeGhosts:

if (ghost == redGhost) {
    this.game.time.events.add(gameStats.redGhostMoveDelay, function() {
        ghost.body.velocity.y = -gameStats.ghostVelocity;
    });
}

else if (ghost == orangeGhost) {
    this.game.time.events.add(gameStats.orangeGhostMoveDelay, function() {
        ghost.body.velocity.y = -gameStats.ghostVelocity;
    });
}

This piece of code should look more like this, without any switches or code specific to ghost color:

this.game.time.events.add(ghost.moveDelay(), function() {
    ghost.updateVelocity(-gameStats.ghostVelocity);
});

But let's assume the framework doesn't provide a way to do that. You could emulate it, by creating a ghost factory, to get a ghost by color, implemented in your own naturally designed model, with the methods natural for its behavior, no more switches.

Observations about elements of the design

In the update function the overall flow of the game is easy to see and looks natural, except this bit:

this.movePacMan();
this.updatePacmanPosition();

This makes me wonder why is it that movePacMan doesn't update the position of pacman, and why two functions exist with confusingly similar names.


Moving on to movePacMan, this sequence of calls looks strange:

this.moveLeft();
this.moveRight();
this.moveUp();
this.moveDown();
this.wrapAround();
this.hitWall();
this.chompDots();

For one thing, I would (like to) assume that pacman's state is encapsulated in an object, with values such as current position and movement direction. It would have expected to see something more like this:

if (this.move()) {
  this.chompDots();
} else {
  this.hitWall();
}

Where move() would move in the current direction if possible, implying correct wrapping around when necessary.

The current implementation involves evaluating many conditions unnecessarily:

  • It's not possible to move in multiple directions simultaneously, yet the code checks movement in all possible directions.
  • It's not possible to wrap around at multiple edges, yet the code checks all possible edges.
  • It's not possible to chomp dots and hit a wall at the same time, yet the code checks both.

With the reworked code, such unnecessary checks would go away.

Don't repeat yourself

As you yourself have noticed, there are many similar chunks of code, only differing in a few values. You probably know what to do.

For example the move functions should use one common function:

pacman.direction = direction;
pacman.body.velocity.x = vx;
pacman.body.velocity.y = vy;
pacman.frame = frame;
pacman.animations.stop();
pacman.animations.play(animation);
\$\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.