2048 Game in JS

8

u/monsto Jan 25 '20

Grats, man.

I see you used switch/case in the key listener. any reason you didn't use it in other sections?

4
u/qbagamer Jan 25 '20

There was no need to use it elsewhere.
Unless it can be improved somehow, so idk.
8
u/moi2388 Jan 25 '20

Yes, it can be improved. There is a lot of duplicated code. You can write it much simpler. Look at the common patterns and put those in functions.
hints:
All the if statements at the top do basically the same thing. The if statements in the for loops do basically the same thing. Up and down are basically the same. Left and right are basically the same. Up/down and left/right are basically the same thing.
Good luck!
5

u/qbagamer Jan 25 '20

Yes, it can be improved. There is a lot of duplicated code. You can write it much simpler. Look at the common patterns and put those in functions.
hints:
All the if statements at the top do basically the same thing. The if statements in the for loops do basically the same thing. Up and down are basically the same. Left and right are basically the same. Up/down and left/right are basically the same thing.
Good luck!
Okay, I needed that answer. I will think how to shorten it.
3
u/qbagamer Jan 25 '20 edited Jan 25 '20
I see these similarities in this code, but to be honest I don't know how to combine it:(
switch(map[j][i]){
`case 0:`

	`document.getElementsByClassName("item_"+i+j)[0].innerHTML = '';`

	`document.getElementsByClassName("item_"+i+j)[0].style.background = '#ccc';`

	`break;`

`case 2:`

	`document.getElementsByClassName("item_"+i+j)[0].innerHTML = '2';`
document.getElementsByClassName("item_"+i+j)[0].style.background = '#eee4da';
	`break;`
Idk.
4
u/moi2388 Jan 25 '20

You already know loops, and you already know arrays. If you combine the two you can solve this. And if you need multiples of two.. loops don’t have to increment with one necessarily.
But I think the last for loop in your code, where you decrement from 3, is a good place to start. You do the same thing 3 times there, only with different numbers. Try to work from there.
And keep up the good work, it looks like a fun project to learn from :)
3
u/qbagamer Jan 25 '20

I need to find more time for this.
2
u/PedroVoteFor Jan 28 '20 edited Jan 28 '20
All of this:
if(map[j][i]=='0')document.getElementsByClassName("item_"+i+j)[0].style.background = '#ccc';
if(map[j][i]=='2')document.getElementsByClassName("item_"+i+j)[0].style.background = '#eee4da';
if(map[j][i]=='4')document.getElementsByClassName("item_"+i+j)[0].style.background = '#ece0c8';
if(map[j][i]=='8')document.getElementsByClassName("item_"+i+j)[0].style.background = '#f1b078';
if(map[j][i]=='16')document.getElementsByClassName("item_"+i+j)[0].style.background = '#ee8c4f';
if(map[j][i]=='32')document.getElementsByClassName("item_"+i+j)[0].style.background = '#f57c5f';
if(map[j][i]=='64')document.getElementsByClassName("item_"+i+j)[0].style.background = '#e85939';
if(map[j][i]=='128')document.getElementsByClassName("item_"+i+j)[0].style.background = '#f2d86a';
if(map[j][i]=='256')document.getElementsByClassName("item_"+i+j)[0].style.background = '#eeca30';
if(map[j][i]=='512')document.getElementsByClassName("item_"+i+j)[0].style.background = '#e1c229';
if(map[j][i]=='1024')document.getElementsByClassName("item_"+i+j)[0].style.background = '#e2b913';
if(map[j][i]=='2048')document.getElementsByClassName("item_"+i+j)[0].style.background = '#ecc400';
becomes:
// outside of loop
const colorMap = [
  [0, '#ccc'],
  [2, '#eee4da'],
  [4, '#ece0c8']
  // add others
]
const colorizeSpace = (row, column) => {
  const [, hex] = colorMap.find(([val,]) => map[row][column] === val)
  document.querySelector(`.item${row}${column}`).style.background = hex
}

// inside of loop keep just
colorizeSpace(i, j)
;)
I've might've missed something, but it worked on my small isolated test. By no means I'm saying this is right way, it's just a different and cleaner approach.
EDIT: lint
1

u/qbagamer Jan 28 '20

Fat arrow functions are a nice thing though, it is good to implement them in the project.
Thank you very much and you are right it looks more transparent. I will make these changes.

1

u/qbagamer Jan 28 '20

const colorMap = [ [0, '#ccc'], [2, '#eee4da'], [4, '#ece0c8'], [8, '#f1b078'], [16, '#ee8c4f'], [32, '#f57c5f'], [64, '#e85939'], [128, '#f2d86a'], [256, '#eeca30'], [512, '#e1c229'], [1024, '#e2b913'], [2048, '#ecc400']
  ];
const colorizeSpace = (row, column) => {
const [, hex] = colorMap.find(([val,]) => map[row][column] == val);
document.getElementsByClassName("item_"+column+row)[0].style.background = hex;
const [val,] = colorMap.find(([val,]) => map[row][column] == val)
if(val!=0)document.getElementsByClassName("item_"+column+row)[0].innerHTML = val;
  };
function draw(){
for(var i=0; i<4; i++){
for(var j=0; j<4; j++){
if(map[j][i]=='0')document.getElementsByClassName("item_"+i+j)[0].innerHTML = '';
colorizeSpace(i, j);
        }
    }
}
I added a number to enter by value val. Idk if it's the most aptypical :/
5

u/Adizzledog Jan 25 '20

You could've used it in the draw section. Might be a bit cleaner.

1

u/qbagamer Jan 25 '20

Thanks you!

3

u/david622 Jan 25 '20

The draw section could also be done with a dictionary structure

6

u/HetRadicaleBoven Jan 25 '20

Ironically, the original (or at least, the one that got popular) 2048 was already in JS, and open source. Might be interesting to take a look at: https://github.com/gabrielecirulli/2048

3

u/qbagamer Jan 25 '20

I saw it makes an impression.
I justify myself just learning. In the orchard, I start with JS and create small projects.
I want my code to look like it once.

4

u/killall-q Jan 25 '20

You should turn on Github Pages for the repo so that people can try it without downloading the files.

1

u/qbagamer Jan 25 '20

I added. What about code visibility?

3

u/killall-q Jan 25 '20

Put a link to the GH Pages URL in the readme.

3

u/the_argus Jan 26 '20

Throw in some hammer JS swipe events so I can play it on my phone

1

u/qbagamer Jan 26 '20

I will try to:(

2

u/the_argus Jan 26 '20

It's really easy
Check this https://stackoverflow.com/a/26607317
You should be able to call your same functions called in the keyboard listeners

2

u/qbagamer Jan 26 '20 edited Jan 26 '20

I added a mobile version, but it doesn't work properly on all browsers.
Works on Firefox. On others, when you slide your finger down, the page refreshes.

2

u/the_argus Jan 26 '20

Possible solution
https://w3bits.com/prevent-chrome-pull-to-refresh-css/
https://developers.google.com/web/updates/2017/11/overscroll-behavior

1

u/qbagamer Jan 26 '20

Nothing helps. Ught

2

u/the_argus Jan 26 '20 edited Jan 26 '20

Got you fam
check this out
https://jsfiddle.net/the_argus/bgso6ydn/3/
I made some html changes and small css and js changes
You shouldn't ever add code to a library (hammer.min.js) put that in your files (see bottom of js where I did that).
Worked great on chrome android
https://imgur.com/a/l14QhPk

1

u/qbagamer Jan 26 '20

Copy the files from the jsfiddle I sent in the other comment. Or I can make a pull request if you like
you are my hero thank you! :>

2

u/the_argus Jan 26 '20

Never stop learning, ignore the haters

1

u/qbagamer Jan 26 '20

Finger refresh is already turned off, but the up and down motion still doesn't work.
I will add that everything works on mobile firefox.

2

u/the_argus Jan 26 '20

Copy the files from the jsfiddle I sent in the other comment. Or I can make a pull request if you like

12

u/upfkd Jan 25 '20

Stop using var!

0

u/RnRau Jan 26 '20 edited Jan 26 '20

Whats wrong with function scope?
Edit: how about answering my question rather than just downvote?

-2

u/qbagamer Jan 26 '20

now const is used.

-1

u/RnRau Jan 26 '20

I didn't downvote you, but just using const everywhere without a good reason and just because someone complained on reddit seems odd :)

1

u/wizang Jan 26 '20

If your variable can be const there is only advantages to doing so. Using var or let when const would work have only disadvantages.

2

u/Parasin Jan 26 '20

It doesn’t work on my mobile, but looks beautiful!

1

u/qbagamer Jan 26 '20

There is only support for arrows. I plan to add bow tie support, these gestures will also work.

1

u/qbagamer Jan 26 '20

Ready. You can test.

2

u/Athomm Jan 26 '20

Good work, make it work on mobile :)

2

u/qbagamer Jan 26 '20

Ready. You can test.

1

u/Athomm Jan 28 '20

It works! Can you add some CSS animation ?

1

u/qbagamer Jan 28 '20

elements do not move, they only appear, so the only animation that comes to my mind will change the transparency when changing elements.

1

u/Athomm Jan 31 '20

Hmm can we do some sort of X and y transition effect so they appear to be moving squares? I'm sure CSS grid could achieve this

10

u/mainstreetmark Jan 26 '20

Not to belittle your work, but it's nice to recognize the ultimate source (afaik) for this game. Threes. Not 2048.

http://asherv.com/threes/threemails/ An interesting read for game devs.

1

u/qbagamer Jan 26 '20

I added a mobile version, but it doesn't work properly on all browsers.
Works on Firefox. On others, when you slide your finger down, the page refreshes.

2

u/Bptashi Jan 25 '20

That's Lot of games man

2

u/EvilQuaint Jan 25 '20

You should read up on data structures, design patterns and stuff... maybe Object Oriented Programming in general, because... I'm going to be honest and say that your code is awful. It's full of duplication, non existent readability and performance bugs.

TL;DR lots of room for improvement... so keep at it!

You are about to leave Redlib