# Refactoring Code

**URL:** <https://phaser.discourse.group/t/refactoring-code/14095>\
**Category:** Phaser 3\
**Created:** [March 8, 2024, 3:07am UTC](https://phaser.discourse.group/t/refactoring-code/14095 "2024-03-08T03:07:46Z")\
**Posts on this page:** 2\
**Page:** 1

<div class="post-metadata">

**Author:** ![vegancatter](https://avatars.discourse-cdn.com/v4/letter/v/d6d6ee/32.png) [@vegancatter](https://phaser.discourse.group/u/vegancatter)\
**Post date:** [March 8, 2024, 3:07am UTC](https://phaser.discourse.group/t/refactoring-code/14095/1 "2024-03-08T03:07:46Z")

</div>

Hello guys I am a beginner towards Phaser and JS in general could you help me refactor my code and give me tips as well?

```javascript
var config = {
  type: Phaser.AUTO,
  width: 800,
  height: 600,
  pixelArt: true,
  backgroundColor: '#4488aa',

  scene: {
    preload: preload,
    create: create,
    update: update
  },

  physics: {
    default: 'arcade',
    arcade: {
        debug: true
    }
  }
};

const game = new Phaser.Game(config);

function preload() {
  this.load.spritesheet('player-sheet', 'assets/player-sheet.png', {frameWidth: 16, frameHeight: 16});
  this.load.image('top-pipe', 'assets/top-pipe.png');
  this.load.image('btm-pipe', 'assets/btm-pipe.png');
}     

function create() {
  // GENERAL //

  this.spacebarkey = this.input.keyboard.addKey(Phaser.Input.Keyboard.KeyCodes.SPACE)

  // this.PLAYER //
  this.player = [];
  this.player.x = 400;
  this.player.y = 300;

  const jumpAnim = {
    key: 'jump',
    defaultTextureKey: "player-sheet",
    duration: 1,
    frames: [
      {frame: 0, duration: 1 },
      { frame: 1, duration: 50},
      { frame: 2, duration: 75},
      { frame: 1, duration: 20},
      {frame: 0, duration: 1 }

    ]
  };

  this.anims.create(jumpAnim);

  this.player.sprite = this.physics.add.sprite(this.player.x, this.player.y, 'player-sheet', 0);
  this.player.sprite.setScale(4);
  this.player.sprite.smoothed = false;
  this.player.sprite.setGravityY(950);

  // PIPES //

  this.pipes = this.physics.add.group();

  this.physics.add.overlap(
    this.pipes,
    this.player.sprite,
    () => {
      this.scene.restart();
    }
  );

  var spawnPipeTimer = this.time.addEvent({
    delay: 5000,
    callback: spawnPipe,
    callbackScope: this,
    loop: true
  
  });

  function spawnPipe() {
    var yOffset = ((Math.random() - 0.5) * 100) * 2

    var top_pipe = this.pipes.create(800, -50 + yOffset, 'top-pipe');
    var btm_pipe = this.pipes.create(800, 650 + yOffset, 'btm-pipe');

    top_pipe.setScale(4)
    top_pipe.setPosition(top_pipe.x , top_pipe.y + yOffset)
    this.pipes.add(top_pipe)

    btm_pipe.setScale(4)
    btm_pipe.setPosition(btm_pipe.x , btm_pipe.y + yOffset)
    this.pipes.add(btm_pipe)
  }
}

function update() {
  // PLAYER //
  if (Phaser.Input.Keyboard.JustDown(this.spacebarkey)) {
    this.player.sprite.play("jump", true)
    var maxVel = 50
    var yJump = this.player.sprite.body.velocity.y - 500
    if (yJump > maxVel) {
      yJump = maxVel
    }
    this.player.sprite.setVelocity(0, yJump)
  }

  if (this.player.sprite.y < 0 || this.player.sprite.y > config.height) {
    this.scene.restart();
  }

  // Pipes //

  this.pipes.children.iterate(function(pipe) {
    pipe.setPosition(pipe.x - 2, pipe.y);
  });
}

```

Thanks!

---

<div class="post-metadata">

**Author:** ![samme](https://yyz2.discourse-cdn.com/free1/user_avatar/phaser.discourse.group/samme/32/5275_2.png) [@samme](https://phaser.discourse.group/u/samme)\
**Post date:** [March 8, 2024, 5:22pm UTC](https://phaser.discourse.group/t/refactoring-code/14095/2 "2024-03-08T17:22:34Z")

</div>

Congrats on your first game. 🏆

> [@vegancatter](#):
>
> ```javascript
> this.player = [];
> this.player.x = 400;
> this.player.y = 300;
> 
> ```

This can be

```js
this.player = { x: 400, y: 300 };

```

> [@vegancatter](#):
>
> `{frame: 0, duration: 1 }`

If the duration is so short could you leave this frame out?

> [@vegancatter](#):
>
> `var maxVel = 50`

50 would be slightly down, not up. Not sure if that’s what you want.

> [@vegancatter](#):
>
> ```javascript
> pipe.setPosition(pipe.x - 2, pipe.y);
> 
> ```

It would be better to move the pipes with velocity instead. So remove this loop and use instead in `spawnPipe()`:

```javascript
top_pipe.setVelocityX(-120);

```

etc.

You probably want to remove passed pipes or they will accumulate forever. You could do this during `spawnPipe()` too — loop through all of them and destroy the ones offscreen.