2
\$\begingroup\$

Below is the working code but it can surely be simplified with some sort of changeCol() and changeRow() functions, or perhaps a single function?

$(document).ready(function(e){
 let keys = {};
 let selRow = 0;
 let selCol = 0;
 let rowCount = 5;
 let colCount = 5;
 //set initial highlits
 $('[data-row="0"]').addClass('hl');
 $('[data-col="0"]').addClass('hl');
 $(document).keydown(function(event){
 keys[event.which] = true;
 }).keyup(function(event){
 delete keys[event.which];
 });
 
 for (let row = 0; row < rowCount; row++) {
 for (let col = 0; col < colCount; col++) {
 let elPiece = $("<span>", {
 "class": 'piece',
 "data-col": col,
 "data-row": row,
 text: "O"
 });
 $('#board').append(elPiece)
 }
 }
 function gameLoop() {
 // w for north
 if (keys[87]) {
 shiftCol(selCol, -1);
 }
 //press s for south
 if (keys[83]) {
 shiftCol(selCol, 1);
 }
 //up arrow
 if (keys[38]) {
 $('[data-row="' + selRow + '"]').removeClass('hl');
 $('[data-col="' + selCol + '"]').addClass('hl');
 if (selRow === 0) {
 selRow = rowCount - 1;
 } else {
 selRow--;
 }
 $('[data-row="' + selRow + '"]').addClass('hl');
 }
 //down arrow
 if (keys[40]) {
 $('[data-row="' + selRow + '"]').removeClass('hl');
 $('[data-col="' + selCol + '"]').addClass('hl');
 if (selRow === rowCount - 1) {
 selRow = 0;
 } else {
 selRow++;
 }
 $('[data-row="' + selRow + '"]').addClass('hl');
 }
 //left arrow
 if (keys[37]) {
 $('[data-col="' + selCol + '"]').removeClass('hl');
 $('[data-row="' + selRow + '"]').addClass('hl');
 if (selCol === 0) {
 selCol = colCount - 1;
 } else {
 selCol--;
 }
 $('[data-col="' + selCol + '"]').addClass('hl');
 }
 //right arrow
 if (keys[39]) {
 $('[data-col="' + selCol + '"]').removeClass('hl');
 $('[data-row="' + selRow + '"]').addClass('hl');
 if (selCol === colCount - 1) {
 selCol = 0;
 } else {
 selCol++;
 }
 $('[data-col="' + selCol + '"]').addClass('hl');
 }
 // code to move objects and repaint canvas goes here
 setTimeout(gameLoop, 100);
 }
 gameLoop();
});
#board {
 width: 150px;
 height: 150px;
}
.piece {
 width: 30px;
 height: 30px;
 display: inline-block;
 background-color: black;
 color: white;
 text-align: center;
 line-height: 30px;
}
.hl {
 background-color: gray;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id="board">
</div>

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Sep 12, 2017 at 2:15
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Do you really need a loop? Can't you use a simple event listener? Of course using a loop has the benefit that you can keep pressing and don't have to wait for the delay... \$\endgroup\$ Commented Sep 12, 2017 at 2:40
  • \$\begingroup\$ "you can keep pressing and don't have to wait for the delay"... that's exactly why I'm using a loop. The part I'm really looking to reduce is all the if (keys...) statements. Lots of duplicated code. \$\endgroup\$ Commented Sep 12, 2017 at 2:42
  • \$\begingroup\$ Ok, what it should do is move like if it was a "crane" being able to move the "horizontal" line of divs and the "vertical" line? Of course using the .hl class? If this is the case I can create a pen improving the code. Also, you can store all the divs in an array or object after you generate them so then you don't have to parse the DOM on each iteration of the loop, improving speed significantly (even more if you have a bigger board). \$\endgroup\$ Commented Sep 12, 2017 at 2:48

1 Answer 1

2
\$\begingroup\$

Ok, after redoing it from scratch, here's a few points to consider:

  • Instead of checking if the key "exists" check if it's true/false
  • You can make it more flexible by making the size of the tile a variable (or even better, a variable for height and one for width)
  • It's better to store the tiles into an array, with their respective x and y variable. This way, you don't have to parse the DOM with jQuery each iteration of the loop, you only loop through the array and select the tile element.

Here's the new code:

HTML - Really simple, just the container for the tiles

<div class="board"></div>

CSS - We don't set the size of anything, we'll calculate it with JS

*{
 box-sizing: border-box;
 margin: 0;
 padding: 0;
}
body{
 background: #f1f1f1;
}
html,
body{
 width: 100%;
 height: 100%;
 display: flex;
 align-items: center;
 justify-content: center;
}
.board{
 display: flex;
 flex-wrap: wrap;
}
.tile{
 background-color: #000;
 color: #fff;
 display: flex;
 align-items: center;
 justify-content: center;
 font-size: .75rem;
}
.active{
 background: #aaa;
}

JS

//amount of tiles horinzontally and vertically
var tileAmountX = 15;
var tileAmountY = 15;
//actual column/row
var actualX = 0;
var actualY = 0;
//size in pixels of the tiles
var tileWidth = 40;
var tileHeight = 40;
//size in pixels of the board, calculated with tile size and tile amount
var boardWidth = tileWidth * tileAmountX;
var boardHeight = tileHeight * tileAmountY;
//array for storing the tiles
var tiles = [];
//keys object, default state are all off (false)
var keys = {left: false, top: false, right: false, down: false};
//set board size style
$('.board').css({
 width: boardWidth + 'px',
 height: boardHeight + 'px'
});
//generate tiles
for(var i = 0; i < tileAmountX; i++){
 for(var j = 0; j < tileAmountY; j++){ 
 //create tile and store it in temporary variable
 var el = $('<div class="tile" style="width: '+tileWidth+'px; height: '+tileHeight+'px;">O</div>').appendTo('.board');
 //store that variable in the `tiles` array, with the respective `x` and `y` coordinate
 tiles.push({
 el: el,
 x: i,
 y: j
 }); 
 }
}
//move to right function, just changes the actualY value and check for overflow
function moveRight(){ 
 actualY = actualY < tileAmountY - 1 ? actualY + 1 : 0;
}
//move to left function, just changes the actualY value and check for overflow
function moveLeft(){
 actualY = actualY > 0 ? actualY - 1 : tileAmountY - 1;
}
//move to top function, just changes the actualX value and check for overflow
function moveTop(){
 actualX = actualX > 0 ? actualX - 1 : tileAmountX - 1;
}
//move to down function, just changes the actualX value and check for overflow
function moveDown(){
 actualX = actualX < tileAmountX - 1 ? actualX + 1 : 0;
}
//generic move function
function move(){
 //get the tiles that have the X or Y coordinate from actualX or actualY
 var starting = tiles.filter(function(val){
 return (val.x == actualX || val.y == actualY);
 });
 //remove the active class for all tiles
 for(var i = 0; i < tiles.length; i++){
 var tile = tiles[i];
 tile.el.removeClass('active');
 }
 //add the active class for the tiles that have the X or Y coordinate from actualX and actualY
 for(var i = 0; i < starting.length; i++){
 var tile = starting[i];
 tile.el.addClass('active');
 }
}
function loop(){
 //this is pretty self explanatory
 if(keys.right){
 moveRight();
 }else if(keys.left){
 moveLeft();
 }
 if(keys.top){
 moveTop();
 }else if(keys.down){
 moveDown();
 }
 //this function runs regardless of the user input
 move();
 //loop with 30fps
 setTimeout(loop, 1000/30);
}
//initiate loop
loop();
//event listener for keydown and keyup
$(window).on('keydown', function(e){
 if(e.keyCode==37){keys.left = true}else if(e.keyCode==39){keys.right = true};
 if(e.keyCode==38){keys.top = true}else if(e.keyCode==40){keys.down = true};
});
$(window).on('keyup', function(e){
 if(e.keyCode==37){keys.left = false}else if(e.keyCode==39){keys.right = false};
 if(e.keyCode==38){keys.top = false}else if(e.keyCode==40){keys.down = false};
});

If you want to see a live example, check this link:

https://codepen.io/tyrellrummage/full/bobxPp/

As you can see, even with a big board (15x15) the motion is very fluid and responsive.

There may be a better way to achieve this, but this is the best I can come up with!

answered Sep 12, 2017 at 3:22
\$\endgroup\$
2
  • \$\begingroup\$ OP uses let, so you are fully entitled to use const for nearly all of these vars \$\endgroup\$ Commented Sep 12, 2017 at 7:13
  • \$\begingroup\$ @BenC yeah I should, I didn't fully made the jump to ES6 yet but I should. \$\endgroup\$ Commented Sep 12, 2017 at 17:28

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.