#p5.js code for working chess from scratch

1 messages · Page 1 of 1 (latest)

storm field
#

My code is currently using an array of objects I will change this to a board representation 2d array or bitboard when I rewrite the code(because its more effecient for a chess engine).

I am currently replacing magic numbers with constants to make the code more clean for example the squareSize constant.

checkmate stalemate and check will still be implemented aswell as en passant and the promotion code and movesmemory code are close to working but are currently bugged.

Is my code well written? What could be improved?

wintry dock
#
  • you should probably split the code into separate files. For example one for the game logic, and one for the drawing logic. This means the game logic shouldn't have to worry about stuff like the size of the square in the canvas for instance, or how to draw the dot for possible moves.
  • Typescript would help to make sure you are using your variables correctly, and also help other people understand what the variable contains. But using typescript probably means you have to move away of the online editor and use p5.js locally, making so if you are not used to that I wouldn't worry.
  • Representing the pieces in your variables with non ASCII characters like ♙♘♗♖ is kinda odd. You should probably use a plain string like "pawn" or "rook".
  • your function are not named very well. As a general rule, the name of a function should start with a verb. Instead of piecesToggle, it should be togglePieces, and instead of queen, it should be toggleQueen. Also, there is a balance between name being too short and not descriptive enough, and too long and hard to read, and I think you are on the shorter side. For example toggleQueenAndDrawMoves would be better I think.
  • You should probably learn a bit about object oriented programming, and apply it for some of your code. This way instead of having to do a bunch of if (piece.currentPiece === '♜') { rook(...) } else if (...) that get long very quick, you could just do piece.drawMoves() and the correct logic would be invoked.
  • Try to split the color of the piece and the type of the piece in your logic, this will avoid a bunch of duplicated code. Also, try to get not use the index of a piece in the Pieces array for any of the logic. This would avoid those ugly (direction+1)/2*15, Pieces[13+(direction+1)*8], the use of pieceNumber and whiteKingIndex that make it very hard to understand what is going on for example.
#

And also, some remarks on the style of your code :

  • Try to not make the lines longer than 100~120 characters, you can always break a big line into multiple smaller lines
  • You should probably use let and const instead of var, they are more modern
  • You should declare function by doing function name() instead of var name = function()
  • Capitalized names are usually reserve for class, so Pieces should be named pieces, or allPieces.
  • make sure you indent your code correctly
#

Oh and I missed this one : the movesMemory is structured in a very unconventional way.
You are storing each part of the moves in their own sub array, instead of using an array of objects.
It should be structured like this :

const movesMemory = []
movesMemory.push({
  oldX: ...,
  oldY: ...,
  x: ...,
  y: ...,
  piece: ...,
  capturedPiece: ...
})

This way the updatePieceLocationsMemory and all other place where you manipulate the memory becomes shorter and clearer.

Here is an example with all other suggestion applied

function updatePieceLocationsMemory(piece, oldPosition)
{
    if (move < movesMemory.length) 
    {
        // This removes everything from the current move index to the end
        movesMemory.splice(move - 1);
        // Reset the limit of the "future"
        totalMoves = move;
    }

    if (!piece.isPromotedPawn()) {
        move++;
        totalMoves = move;
    }

    const move = {
        oldPosition,
        position: piece.position,
        piece,
        capturedPiece: null,
    }

    iterateOnAllOpponentPieces(piece.color, opponentPiece => {
        if (opponentPiece.position.isEqual(piece.position)) {
            move.capturedPiece = opponentPiece;
        }
    })

    movesMemory.push(move);
    
    piece.firstMove = false;
    piece.toggle = false;
}
#

Feel free to ask for clarification if you feel lost about some of the concepts I talked about.

storm field
# wintry dock * you should probably split the code into separate files. For example one for th...

"you should probably split the code into separate files. For example one for the game logic, and one for the drawing logic. This means the game logic shouldn't have to worry about stuff like the size of the square in the canvas for instance, or how to draw the dot for possible moves. "

  • yeah I guess splitting it would make it better I had it as one code because it used to be code written in khanacadamy I recently transferred it to p5.js.

"Representing the pieces in your variables with non ASCII characters like ♙♘♗♖ is kinda odd. You should probably use a plain string like "pawn" or "rook"."

  • those symbols just make it look nice thats why I use it because I dont understand vertexes and beziercurves etc enough to make my own pieces

"your function are not named very well. As a general rule, the name of a function should start with a verb. Instead of piecesToggle, it should be togglePieces, and instead of queen, it should be toggleQueen. Also, there is a balance between name being too short and not descriptive enough, and too long and hard to read, and I think you are on the shorter side. For example toggleQueenAndDrawMoves would be better I think."

  • yeah I guess its probably better to just use 2 words minimum for the name most of the time unless its obvious I just thought queen and king etc were understandable because it makes it look like a black box kinda u just call king for all the king logic and queen for all the queen logic

"You should probably learn a bit about object oriented programming, and apply it for some of your code. This way instead of having to do a bunch of if (piece.currentPiece === '♜') { rook(...) } else if (...) that get long very quick, you could just do

#

piece.drawMoves() and the correct logic would be invoked."

  • yeah the object oriented I find hard to apply I guess Ill try another big project using it because Im used to not doing it because I have mostly done small projects.

"Try to split the color of the piece and the type of the piece in your logic, this will avoid a bunch of duplicated code. Also, try to get not use the index of a piece in the Pieces array for any of the logic. This would avoid those ugly (direction+1)/2*15, Pieces[13+(direction+1)*8], the use of pieceNumber and whiteKingIndex that make it very hard to understand what is going on for example."

  • if I put a seperate variable called team and used the ternary operator to assign white or black and the starting and ending index based off the (direction+1)/2 would u say thats ok or do u think I should just pass the team directly and pass more arguments in the functions to seperate the logic for the teams like that. If (direction+1)/2*15 is defined in a for loop its gonna recompute it every iteration which is less effecient and also less easy to understand so if its put in a seperate variable I get why its better.

"You should probably use let and const instead of var, they are more modern"

  • yeah I looked it up and var can have weird behaviour so ill start doing that and const also makes sense for constants

It should be structured like this :
js const movesMemory = [] movesMemory.push({ oldX: ..., oldY: ..., x: ..., y: ..., piece: ..., capturedPiece: ... }) ​
ok ill update that it does make it more obvious yeah

also I was wondering what you would rate the code out of 10 for code effeciency(how many lines of code compared to performance), functionality, and how clean the code is

wintry dock
# storm field "you should probably split the code into separate files. For example one for the...

those symbols just make it look nice
I'm not talking about the result, here it's completely fine to use those characters.
I'm saying don't use it in the code. Stuff likePieces[i].currentPiece === '♙' feels very hard to read.

I guess its probably better to just use 2 words minimum
Yeah, but the most important part is the verb at the beggining.

yeah the object oriented I find hard to apply
Here the different pieces which all have their special logic fit very well into the inheritance model.

variable called team and
That' probably better if it is a property of the Pieces array don't you think ?

what you would rate the code out of 10 for ...
I can't speak about performance, that's something you will have to test for yourself. And stuff like changing (direction+1)/2*15 into a team lookup won't change the performance much (but you should still change it to make the code more readable).

Going after performance isn't something you should focus on at first. And if you end up doing it, the golden rule is to always measure the before and after of every change you do in the name of performance. Because computer are so complex nowadays that it's difficult to assert with certainty if any given change will have the impact you think it has.

For example, the number of line of codes has no relation to performance really.

Functionnality : you tell me. Does it do what you want it to do ? Maximizing functionnality is not a sane goal, you'll just end up with a massive project that does everything badly. Better to focus on one thing and do it well, especially if you don't have much experience.

How clean the code is : could be better. just go over what I've said previously. I mean this in the best possible way, but this looks like the kind of code I did when I was just starting out. Lots of complex ideas that are not managed very well and make it overall difficult to read.

storm field
#

yeah I agree about the performance thing I put performance instead of functionality by accident. I meant more like a combination of the amount of lines compared to functionality while also keeping it clean because its possible to have good functionality but with very little code(ive seen alternatives of my code from AI and other people), however then it will probably be less clean to read or you will need to have more knowledge to understand it. I will only really focus on performance if I rewrite the code for a chess engine.

Also I used AI before to ask about the code quality but it gives high ratings too easily

wintry dock
#

For me the two main things that's make it not that cleans are :

  • not organized into different files.
  • repeated code. (it's not an absolute rule, but usually we say to DRY : Don't Repeat Yourself)