HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

b10k minigame problems

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
b10kproblemsminigame

Problem

I tried to recreate a mini game (not sure about its name, I call it b10k) with 2048 looks, therefore it looked better if I use html elements, and I really am not good at working with them, but I tried: https://github.com/Cicada3301/b10k

I could also find out that there is a bug that sometimes doesn't update the player's position when going up or down. I suggest using this for testing it on non-local machine.

Please insult me if it's constructive, and don't forget about code readability!
Could you also find out the reason of the glitch?

here is the JavaScript code for who don't want to click the link:

```
function b10k() {
var player,
boardSide = 40,
boardMargin = 2.5;
function Level(map) {
this.map = map;
this.size = this.map.length;
}
function Player(x, y, map, container) {
this.x =x;
this.y =y;
this.map = map;
this.container = container;
this.el = document.getElementById('player');
this.render();
}
Player.prototype.move = function (dir) {
var nextBlock = this.map[this.y + dir.y]? this.map[this.y+dir.y][this.x + dir.x]:undefined;
if (nextBlock !== undefined&&nextBlock!==1) {
this.x += dir.x;
this.y += dir.y
this.move(dir);
} else {
this.render();
}
}
Player.prototype.render = function () {
this.el.style.left = '' + ((this.x * (boardSide / this.map.length) + boardMargin) + 'vw');
this.el.style.top = '' + ((this.y * (boardSide / this.map.length) + boardMargin) + 'vw');
}
Level.prototype.play = function () {
this.render(document.getElementById('map'))
this.adjustElements();
}
Level.prototype.adjustElements = function () {
var player = document.getElementById('player');
player.style.setProperty('width', (boardSide/2 / this.size) + 'vw');
player.style.setProperty('height', (boardSide/2 / this.size) + 'vw')

Solution

I'll primarily do the JS:

  • Spacing and code style: Everything is perfect except for one line.



  • if(this.map[this.y-1]) if (this.map[this.y-1][this.x] ===0){ - if if? Why not use &&? See short circuit evaluation



  • Your move* methods are repetitive. Perhaps some abstraction is in order?



  • Avoid magical constants - What's 40? What's 2.5?



  • Lack of Documentation - Your construct method could really use it.



Also, if you're trying to move to the top or bottom layers, your render() method will never reach. You should add a check against this.map[expected Y value] to not be undefined.

Context

StackExchange Code Review Q#54687, answer score: 2

Revisions (0)

No revisions yet.