Chess Game with GUI in C++ using SDL2

Question

Introduction

So, I recently completed a chess game with a GUI in Python, using Pygame. Upon research, I learned that Pygame is built on SDL, and since I wanted to practice C++ more, I decided to code chess again in C++.

• I have 5 source files and 6 header files. Since this is a large project, feel free only to review the code design/style or just one file.

Questions

• Is my use of namespaces correct?
• Is my header/source splitting (organization) correct? I come from Python so don't have too much experience
• Is SDL2 used properly? (eg. is there a better way to handle events, draw the board...)
• Is my game logic correct? (eg. a better way to check bishop moves, pawn moves...)
• Is my piece/board representation good?
• Are there missed optimizations/shortcuts?
  
  
• Generally, is this idiomatic C++?

Project Info

This project was compiled with g++ on a Late 2012 MacBookPro.

Compiler flags

• -I $(INCLUDE_DIR)
• -Wall
• -Wextra
• -std=c++11

Linker flags

• -D_THREAD_SAFE
• -lSDL2
• -lSDL2_image
• -lSDL2_ttf (not used but planning to use)

Code

board.cpp

#include "board.hpp"

#include <cctype>
#include <iostream>
#include <string>

#include "defines.hpp"
#include "game.hpp"
#include "pieces.hpp"

// TODO: add other FEN fields
void Board::readFen(std::string fen) {
    int row = 0;
    int col = 0;

    for (char c : fen) {
        if (c == '/') {
            row++;
            col = 0;
            continue;
        } else {
            if (isdigit(c)) {
                for (int _ = 0; _ < static_cast<int>(c - '0'); _++) {
                    this->board[row][col] = 0x00;
                    col++;
                }
            } else {
                PieceColor color = isupper(c) ? WHITE : BLACK;
                Piece p;

                c = tolower(c);

                switch (c) {
                    case 'p':
                        p = Pieces::makePiece(PAWN, color);
                        break;
                    case 'r':
                        p = Pieces::makePiece(ROOK, color);
                        break;
                    case 'n':
                        p = Pieces::makePiece(KNIGHT, color);
                        break;
                    case 'b':
                        p = Pieces::makePiece(BISHOP, color);
                        break;
                    case 'q':
                        p = Pieces::makePiece(QUEEN, color);
                        break;
                    case 'k':
                        p = Pieces::makePiece(KING, color);
                        break;
                    default:
                        p = Pieces::makePiece(PAWN, color);
                }

                this->board[row][col] = p;
                col++;
            }
        }
    }
}
// TODO: add other info to board printing
void Board::printBoard() {
    std::cout << "CHESSBOARD INFO" << std::endl;
    std::cout << "Turn: " << (this->getActiveColor() == WHITE ? "white" : "black") << std::endl;
    std::cout << "Move: " << this->getMoveNumber() << std::endl;

    for (int i = 0; i < 8; i++) {
        for (int j = 0; j < 8; j++) {
            std::cout << this->board[i][j] << std::string((board[i][j] >= 10 ? 1 : 2), ' ');
        }
        std::cout << std::endl;
    }
    std::cout << std::endl;
}

PieceColor Board::getActiveColor() {
    return this->active_color;
}

float Board::getMoveNumber() {
    return this->move_number;
}

void Board::incrementMoveNumber() {
    this->move_number += 0.5f;
}

void Board::toggleActiveColor() {
    this->active_color = this->getActiveColor() == WHITE ? BLACK : WHITE;
}

void Board::reverse() {
    int board[8][8];

    for (int i = 7; i >= 0; i--) {
        for (int j = 0; j < 8; j++) {
            board[7 - i][j] = this->board[i][j];
        }
    }

    for (int i = 0; i < 8; i++) {
        for (int j = 0; j < 8; j++) {
            this->board[i][j] = board[i][j];
        }
    }

    this->is_reversed = !this->is_reversed;
}

bool Board::isReversed() {
    return this->is_reversed;
}

void Board::tryMove(Location starting, Location ending) {
    if (this->getActiveColor() != Pieces::getPieceColor(this->getPieceAt(starting))) { // out of turn
        return;
    }

    if (Game::isValidMove(this, starting, ending, true)) {
        this->makeMove(starting, ending);
        this->clearSelectedPiece();

        this->incrementMoveNumber();
        this->toggleActiveColor();
    }
}

void Board::makeMove(Location starting, Location ending) {
    int r1, c1, r2, c2;

    r1 = starting.first;
    c1 = starting.second;

    r2 = ending.first;
    c2 = ending.second;

    this->board[r2][c2] = this->board[r1][c1];
    this->board[r1][c1] = 0x00;
}

int Board::getPieceAt(Location location) {
    return this->board[location.first][location.second];
}


Location Board::getSelectedPiece() {
    return this->selected_piece;
}

void Board::setSelectedPiece(int i, int j) {
    this->selected_piece = makeLocation(i, j);
}

void Board::clearSelectedPiece() {
    this->selected_piece = makeLocation(-1, -1);
}

bool Board::hasSelectedPiece() {
    return (this->selected_piece.first != -1 && this->selected_piece.second != -1);
}

game.cpp

#include "game.hpp"

#include <iostream>

#include "board.hpp"
#include "defines.hpp"
#include "pieces.hpp"


bool Game::isValidMove(Board* board, Location starting, Location ending, bool check_king) {
    Piece starting_piece = board->getPieceAt(starting);
    Piece ending_piece = board->getPieceAt(ending);

    PieceColor starting_color = Pieces::getPieceColor(starting_piece);

    if (ending_piece != 0x00 && starting_color == Pieces::getPieceColor(ending_piece)) {
        return false;  // can't capture your own piece
    }

    bool result = true;
    switch (Pieces::getPieceClass(starting_piece)) {
        case PAWN:
            result = isValidPawnMove(board, starting, ending, starting_color);
            break;
        case KNIGHT:
            result = isValidKnightMove(starting, ending);
            break;
        case BISHOP:
            result = isValidBishopMove(board, starting, ending, starting_color);
            break;
        case ROOK:
            result = isValidRookMove(board, starting, ending, starting_color);
            break;
        case QUEEN:
            result = isValidQueenMove(board, starting, ending, starting_color);
            break;
        case KING:
            result = isValidKingMove(starting, ending);
            break;
    }

    if (!check_king) {
        return result;
    } else if (result == false) {
        return result;
    }

    board->makeMove(starting, ending);

    if (isInCheck(board, starting_color)) {
        board->makeMove(ending, starting);
        board->board[ending.first][ending.second] = ending_piece;
        return false;
    }

    board->makeMove(ending, starting);
    board->board[ending.first][ending.second] = ending_piece;
    return true;
}

bool Game::isValidPawnMove(Board* board, Location starting, Location ending, PieceColor starting_color) {
    Piece ending_piece = board->getPieceAt(ending);

    PieceColor opposite_color = starting_color == WHITE ? BLACK : WHITE;

    if (!board->isReversed()) {
        opposite_color = starting_color;
    }
    if (ending_piece == 0) {  // open piece
        if (opposite_color == WHITE) {
            if (
                starting.first == 6 && starting.first - ending.first == 2 && starting.second == ending.second) {
                return true;
            }
            if (
                starting.first - ending.first == 1 && starting.second == ending.second) {
                return true;
            }
        } else if (opposite_color == BLACK) {
            if (
                starting.first == 1 && ending.first - starting.first == 2 && starting.second == ending.second) {
                return true;
            }
            if (
                ending.first - starting.first == 1 && starting.second == ending.second) {
                return true;
            }
        }
    } else {  // piece is occupied
        if (starting_color == WHITE) {
            if (ending.first == starting.first - 1) {
                if (abs(ending.second - starting.second) == 1) {
                    return true;
                }
            }
        } else if (starting_color == BLACK) {
            if (ending.first == starting.first + 1) {
                if (abs(ending.second - starting.second) == 1) {
                    return true;
                }
            }
        }

        return false;
    }
    return false;
}

bool Game::isValidKnightMove(Location starting, Location ending) {
    int x_diff = abs(ending.first - starting.first);
    int y_diff = abs(ending.second - starting.second);

    if (x_diff == 1 && y_diff == 2) {
        return true;
    }
    if (x_diff == 2 && y_diff == 1) {
        return true;
    }

    return false;
}

bool Game::isValidBishopMove(Board* board, Location starting, Location ending, PieceColor starting_color) {
    if (starting.first + starting.second != ending.first + ending.second && starting.first - starting.second != ending.first - ending.second) {
        return false;
    }

    int x_change = ending.second < starting.second ? -1 : 1;
    int y_change = ending.first < starting.first ? -1 : 1;

    Location indices = starting;

    while (0 <= indices.first <= 7 && 0 <= indices.second <= 7 && indices != ending) {
        indices.second += x_change;
        indices.first += y_change;

        if (board->getPieceAt(indices)) {
            if (Pieces::getPieceColor(board->getPieceAt(indices)) == starting_color) {
                indices.second -= x_change;
                indices.first -= y_change;
            }
            break;
        }
    }

    if (indices == ending) {
        return true;
    }

    return false;
}

bool Game::isValidRookMove(Board* board, Location starting, Location ending, PieceColor starting_color) {
    if (
        starting.first != ending.first && starting.second != ending.second) {
        return false;
    }

    int x_change, y_change;

    x_change = ending.second < starting.second ? -1 : 1;
    y_change = ending.first < starting.first ? -1 : 1;

    x_change = ending.second == starting.second ? 0 : x_change;
    y_change = ending.first == starting.first ? 0 : y_change;

    Location indices = (starting);

    while (0 <= indices.first <= 7 && 0 <= indices.second <= 7 && indices != ending) {
        indices.second += x_change;
        indices.first += y_change;

        if (board->getPieceAt(indices)) {
            if (Pieces::getPieceColor(board->getPieceAt(indices)) == starting_color) {
                indices.second -= x_change;
                indices.first -= y_change;
            }

            break;
        }
    }
    if (indices == ending) {
        return true;
    }

    return false;
}

bool Game::isValidQueenMove(Board* board, Location starting, Location ending, PieceColor starting_color) {
    return isValidBishopMove(
               board, starting, ending, starting_color) ||
           isValidRookMove(board, starting, ending, starting_color);
}

bool Game::isValidKingMove(Location starting, Location ending) {
    int x_diff = abs(starting.first - ending.first);
    int y_diff = abs(starting.second - ending.second);

    if (x_diff == 1 and y_diff == 1) {
        return true;
    }
    if (x_diff == 1 and y_diff == 0) {
        return true;
    }
    if (x_diff == 0 and y_diff == 1) {
        return true;
    }

    return false;
}

bool Game::isInCheck(Board* board, PieceColor color) {
    Location king_location;
    Piece piece;

    for (int i = 0; i < 8; i++) {
        for (int j = 0; j < 8; j++) {
            piece = board->getPieceAt(makeLocation(i, j));
            if (Pieces::getPieceClass(piece) == KING && Pieces::getPieceColor(piece) == color) {
                king_location = makeLocation(i, j);
                break;
            }
        }
    }

    for (int i = 0; i < 8; i++) {
        for (int j = 0; j < 8; j++) {
            piece = board->getPieceAt(makeLocation(i, j));
            if (piece == 0x00) {
                continue;
            }

            if (Pieces::getPieceColor(piece) != color) {
                if (Game::isValidMove(board, makeLocation(i, j), king_location, false)) {
                    return true;
                }
            }
        }
    }
    return false;
}

bool Game::isInCheckMate(Board* board, PieceColor color) {
    Location king_location;
    Piece piece;

    if (!isInCheck(board, color)) {
        return false;  // can't be in mate if ur not checked
    }

    for (int i = 0; i < 8; i++) {
        for (int j = 0; j < 8; j++) {
            piece = board->getPieceAt(makeLocation(i, j));
            if (Pieces::getPieceClass(piece) == KING && Pieces::getPieceColor(piece) == color) {
                king_location = makeLocation(i, j);
                break;
            }
        }
    }

    for (int i = 0; i < 8; i++) {
        for (int j = 0; j < 8; j++) {
            piece = board->getPieceAt(makeLocation(i, j));
            if (piece == 0x00) {
                continue;
            }

            if (Pieces::getPieceColor(piece) == color) {
                Location starting_location = makeLocation(i, j);
                Location ending_location;

                for (int x = 0; x < 8; x++) {
                    for (int y = 0; y < 8; y++) {
                        ending_location = makeLocation(x, y);

                        if (Game::isValidMove(board, starting_location, ending_location, true)) {
                            return false;
                        }
                    }
                }
            }
        }
    }
    return true;
}

gui.cpp

#include "gui.hpp"

#include <SDL2/SDL.h>

#include <fstream>
#include <iostream>
#include <string>

#include "board.hpp"
#include "defines.hpp"
#include "game.hpp"
#include "pieces.hpp"

// TODO: add special moves like castle, en passant
// TODO: add showing checkmate

void GUI::initSDL() {
    SDL_Init(SDL_INIT_EVERYTHING);
}

void GUI::cleanupSDL(SDL_Renderer* renderer, SDL_Window* window) {
    SDL_DestroyRenderer(renderer);
    SDL_DestroyWindow(window);
    SDL_Quit();
}

SDL_Window* GUI::createSDLWindow() {
    return SDL_CreateWindow("C++ Chess", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED,
                            WINDOW_WIDTH, WINDOW_HEIGHT, SDL_WINDOW_SHOWN);
}

SDL_Renderer* GUI::createSDLRenderer(SDL_Window* window) {
    SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "2");
    return SDL_CreateRenderer(window, -1, SDL_RENDERER_PRESENTVSYNC);
}

void GUI::drawChessboard(SDL_Renderer* renderer, Board* board) {
    Piece p;
    Location selected_piece = board->getSelectedPiece();

    for (int i = 0; i < 8; i++) {
        for (int j = 0; j < 8; j++) {
            SDL_Rect tile;

            tile.x = j * TILE_SIZE;
            tile.y = i * TILE_SIZE;
            tile.w = TILE_SIZE;
            tile.h = TILE_SIZE;

            if ((i + j) % 2 == (board->isReversed() ? 1 : 0)) {
                SDL_SetRenderDrawColor(renderer, 0xEE, 0xEE, 0xEE, 0xFF);
            } else {
                SDL_SetRenderDrawColor(renderer, 0x99, 0x99, 0x99, 0xFF);
            }

            if (selected_piece.first == i && selected_piece.second == j) {
                SDL_SetRenderDrawColor(renderer, 0xFF, 0x00, 0x00, 0xFF);

                SDL_Rect highlight;
                highlight.x = tile.x + 5;
                highlight.y = tile.y + 3;
                highlight.w = tile.w - 10;
                highlight.h = tile.h - 10;

                SDL_SetRenderDrawColor(renderer, 0xFF, 0x00, 0x00, 0xFF);
                SDL_RenderFillRect(renderer, &tile);

                if ((i + j) % 2 == (board->isReversed() ? 1 : 0)) {
                    SDL_SetRenderDrawColor(renderer, 0xEE, 0xEE, 0xEE, 0xFF);
                } else {
                    SDL_SetRenderDrawColor(renderer, 0x99, 0x99, 0x99, 0xFF);
                }

                SDL_RenderFillRect(renderer, &highlight);
            } else {
                SDL_RenderFillRect(renderer, &tile);
            }

            p = board->board[i][j];

            if (p != 0) {
                std::string filepath = "../data/bmp/" + Pieces::getPieceFilename(p);

                SDL_Surface* image = SDL_LoadBMP(filepath.c_str());
                if (image == NULL) {
                    std::cerr << "image=" << image << " Reason: " << SDL_GetError() << " " << SDL_GetBasePath() << std::endl;
                }
                SDL_Texture* texture = SDL_CreateTextureFromSurface(renderer, image);

                SDL_RenderCopy(renderer, texture, NULL, &tile);

                SDL_FreeSurface(image);
                SDL_DestroyTexture(texture);
            }

            if (board->hasSelectedPiece() && Game::isValidMove(board, board->getSelectedPiece(), makeLocation(i, j), true)) {
                SDL_SetRenderDrawColor(renderer, 0xFF, 0x00, 0x00, 0xFF);

                int centerX = tile.x + tile.w / 2;
                int centerY = tile.y + tile.h / 2;
                int radius = 15;
                SDL_RenderFillCircle(renderer, centerX, centerY, radius);
            }
        }
    }
}

int GUI::SDL_RenderFillCircle(SDL_Renderer* renderer, int x, int y, int radius) {
    int offsetx, offsety, d;
    int status;

    offsetx = 0;
    offsety = radius;
    d = radius - 1;
    status = 0;

    while (offsety >= offsetx) {
        status += SDL_RenderDrawLine(renderer, x - offsety, y + offsetx,
                                     x + offsety, y + offsetx);
        status += SDL_RenderDrawLine(renderer, x - offsetx, y + offsety,
                                     x + offsetx, y + offsety);
        status += SDL_RenderDrawLine(renderer, x - offsetx, y - offsety,
                                     x + offsetx, y - offsety);
        status += SDL_RenderDrawLine(renderer, x - offsety, y - offsetx,
                                     x + offsety, y - offsetx);

        if (status < 0) {
            status = -1;
            break;
        }

        if (d >= 2 * offsetx) {
            d -= 2 * offsetx + 1;
            offsetx += 1;
        } else if (d < 2 * (radius - offsety)) {
            d += 2 * offsety - 1;
            offsety -= 1;
        } else {
            d += 2 * (offsety - offsetx - 1);
            offsety -= 1;
            offsetx += 1;
        }
    }

    return status;
}

void GUI::handleMouseClicked(SDL_MouseButtonEvent event, Board* board) {
    if (event.button == SDL_BUTTON_RIGHT) {
        return;
    }

    Location board_indices = getBoardIndices(event.x, event.y);

    if (board->getSelectedPiece() == board_indices) {  // de-select a piece
        board->clearSelectedPiece();
    } else if (!board->hasSelectedPiece()) {  // select a piece
        if (board->getPieceAt(board_indices) != 0 && Pieces::getPieceColor(board->getPieceAt(board_indices)) == board->getActiveColor()) {
            board->setSelectedPiece(board_indices.first, board_indices.second);
        }
    } else if (board->hasSelectedPiece()) {  // move a piece
        if (Pieces::getPieceColor(board->getPieceAt(board_indices)) == Pieces::getPieceColor(board->getPieceAt(board->getSelectedPiece()))) {
            board->setSelectedPiece(board_indices.first, board_indices.second);
        } else {
            board->tryMove(board->getSelectedPiece(), board_indices);
        }
    }
}

void GUI::handleKeyPressed(SDL_KeyboardEvent event, Board* board) {
    if (event.keysym.sym == SDL_GetKeyFromName("r") || event.keysym.sym == SDL_GetKeyFromName("f")) {
        std::cout << "r/f pressed, reversing board" << std::endl;
        board->reverse();
    }
}

Location GUI::getBoardIndices(int x, int y) {
    /**
     * Gets board indices from a x, y coordinate of a mouse click.
     *
     * @param x: integer value of x coordiate of mouse click
     * @param y: integer value of y coordiate of mouse click
     *
     * @returns std::pair with integer values from 0-7 in first and second
     */

    Location out;

    out.second = x / TILE_SIZE;
    out.first = y / TILE_SIZE;

    return out;
}

main.cpp

#include "main.hpp"

#include <SDL2/SDL.h>

#include <iostream>

#include "board.hpp"
#include "game.hpp"
#include "gui.hpp"

int main() {
    SDL_Window* window = GUI::createSDLWindow();
    SDL_Renderer* renderer = GUI::createSDLRenderer(window);

    SDL_Event event;

    Board board;

    board.readFen(STARTING_FEN);
    board.printBoard();

    bool running = true;
    bool gameover = false;

    while (running) {
        while (SDL_PollEvent(&event)) {
            if (event.type == SDL_QUIT) {
                running = false;
            } else if (event.type == SDL_MOUSEBUTTONUP && !gameover) {
                GUI::handleMouseClicked(event.button, &board);
            } else if (event.type == SDL_KEYUP && !gameover) {
                GUI::handleKeyPressed(event.key, &board);
            }
        }

        if (gameover) {
            continue;
        }

        if (Game::isInCheckMate(&board, WHITE)) {
            std::cout << "WHITE IS MATED" << std::endl;
            gameover = true;
        }
        if (Game::isInCheckMate(&board, BLACK)) {
            std::cout << "BLACK IS MATED" << std::endl;
            gameover = true;
        }

        SDL_RenderClear(renderer);
        GUI::drawChessboard(renderer, &board);

        SDL_RenderPresent(renderer);
    }

    GUI::cleanupSDL(renderer, window);

    return 0;
}

pieces.cpp

#include "pieces.hpp"

#include <iostream>
#include <string>

PieceColor Pieces::getPieceColor(Piece p) {
    int color = p >> 3;

    if (color == 1) {
        return BLACK;
    } else if (color == 2) {
        return WHITE;
    } else {
        return NOCOLOR;
    }
}

PieceClass Pieces::getPieceClass(Piece p) {
    int pc = p & 7;

    return static_cast<PieceEnum>(pc);
}

std::string Pieces::getPieceFilename(Piece p) {
    PieceColor color = getPieceColor(p);
    PieceClass pc = getPieceClass(p);

    std::string filename;

    switch (color) {
        case WHITE:
            filename += "white_";
            break;
        case BLACK:
            filename += "black_";
            break;
    }

    switch (pc) {
        case PAWN:
            filename += "pawn";
            break;
        case KNIGHT:
            filename += "knight";
            break;
        case BISHOP:
            filename += "bishop";
            break;
        case ROOK:
            filename += "rook";
            break;
        case QUEEN:
            filename += "queen";
            break;
        case KING:
            filename += "king";
            break;
    }

    filename += ".bmp";

    return filename;
}

Piece Pieces::makePiece(PieceClass c, PieceColor pc) {
    return c | pc;
}

Headers

board.hpp

#ifndef BOARD_H_INCLUDED
#define BOARD_H_INCLUDED

#include <string>
#include "defines.hpp"
#include "pieces.hpp"

const std::string STARTING_FEN = "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR";

class Board {
   private:
    PieceColor active_color = WHITE;
    float move_number = 1.0f;

    bool is_reversed = false;

    Location selected_piece = {-1, -1};

   public:
    int board[8][8];

    float getMoveNumber();
    void incrementMoveNumber();

    PieceColor getActiveColor();
    void toggleActiveColor();

    Location getSelectedPiece();
    int getPieceAt(Location location);

    void setSelectedPiece(int i, int j);
    bool hasSelectedPiece();
    void clearSelectedPiece();


    void tryMove(Location starting, Location ending);
    void makeMove(Location starting, Location ending);


    void readFen(std::string fen);
    void printBoard();

    void reverse();
    bool isReversed();
};

#endif  // !BOARD_H_INCLUDED

defines.hpp

#ifndef DEFINES_HPP_INCLUDED
#define DEFINES_HPP_INCLUDED

typedef std::pair<int, int> Location;

inline std::pair<int, int> makeLocation(int i, int j) {
    return std::make_pair(i, j);
}

#endif // !DEFINES_HPP_INCLUDED

game.hpp

#ifndef GAME_HPP_INCLUDED
#define GAME_HPP_INCLUDED

#include "board.hpp"
#include "pieces.hpp"

namespace Game {
bool isValidMove(Board* board, Location starting, Location ending, bool check_king);

bool isValidPawnMove(Board* board, Location starting, Location ending, PieceColor starting_color);
bool isValidKnightMove(Location starting, Location ending);
bool isValidBishopMove(Board* board, Location starting, Location ending, PieceColor starting_color);
bool isValidRookMove(Board* board, Location starting, Location ending, PieceColor starting_color);
bool isValidQueenMove(Board* board, Location starting, Location ending, PieceColor starting_color);
bool isValidKingMove(Location starting, Location ending);

bool isInCheck(Board* board, PieceColor color);
bool isInCheckMate(Board* board, PieceColor color);
}  // namespace Game

#endif  // !GAME_HPP_INCLUDED

gui.hpp

#ifndef GUI_H_INCLUDED
#define GUI_H_INCLUDED

#include <SDL2/SDL.h>

#include "board.hpp"

const int WINDOW_WIDTH = 1200;
const int WINDOW_HEIGHT = 1200;

const int TILE_SIZE = 150;

namespace GUI {
    void initSDL();
    void cleanupSDL(SDL_Renderer* renderer, SDL_Window* window);

    SDL_Window* createSDLWindow();
    SDL_Renderer* createSDLRenderer(SDL_Window* window);

    void drawChessboard(SDL_Renderer* renderer, Board* board);
    int SDL_RenderFillCircle(SDL_Renderer* renderer, int x, int y, int radius);

    void handleMouseClicked(SDL_MouseButtonEvent event, Board* board);
    void handleKeyPressed(SDL_KeyboardEvent event, Board* board);

    std::pair<int, int> getBoardIndices(int x, int y);

}  // namespace GUI

#endif  // !GUI_H_INCLUDED

main.hpp

#ifndef MAIN_HPP_INCLUDED
#define MAIN_HPP_INCLUDED

// TODO: add stuff or remove if unnessecary

#endif // !MAIN_HPP_INCLUDED

pieces.hpp

#ifndef PIECES_H_INCLUDED
#define PIECES_H_INCLUDED

#include <string>

typedef unsigned int Piece;
typedef unsigned int PieceColor;
typedef unsigned int PieceClass;

enum PieceColors {
    WHITE = 16,
    BLACK = 8,
    NOCOLOR = 0
};

enum PieceEnum {
    PAWN,
    KNIGHT,
    BISHOP,
    ROOK,
    QUEEN,
    KING
};

namespace Pieces {
    PieceColor getPieceColor(Piece p);
    PieceClass getPieceClass(Piece p);
    Piece makePiece(PieceClass c, PieceColor pc);
    std::string getPieceFilename(Piece p);
}  // namespace Pieces

#endif  // !PIECES_H_INCLUDED

Answer 1

Since you're already on C++11, it shouldn't be too much effort to move to C++17 or 20, which you should pursue.

Since you have a project with multiple translation units (object files), strongly consider:

• enable -flto, link-time optimization
• enable -fvisibility=hidden

readFen should not accept a std::string, but instead a const std::string &.

Don't use _ as an iteration variable (don't do this in Python either; it isn't unused).

This is a matter of personal taste, but I prefer to omit this-> when there is an unambiguous member name.

printBoard should be printBoard() const. The same for every one of your get methods and isValidMove. When possible, these should also be given the constexpr prefix.

This is also (mostly) a matter of personal preference, but for simple one-line accessors like getActiveColor, just define them in the header and not separately in the .cpp.

isValidBishopMove and the like should accept a const Board &, not a Board*.

std::string filepath can be replaced with the new STL filesystem::path library.

Board::board must not be public.

Almost all isValidMove and family can just be methods on Board, since they need a Board reference anyway.

Replace your enums with enum class.

There's more, but that's a start.

Answer 2

Answers to your questions

• Is my use of namespaces correct?

It's definitely better than not using namespaces. However, you can improve it:

• Move more things into the namespaces. For example, why not move WINDOW_WIDTH, WINDOW_HEIGHT and TILE_SIZE into namespace GUI? Similarly for PieceColors and PieceEnum, these could be moved into Pieces.
• A class is effectively also a namespace. Instead of having a namespace Game which just contains functions for checking valid moves, many of which take a Board* as the first parameter, why not move these functions into class Board?

• Is my header/source splitting (organization) correct? I come from Python so don't have too much experience

It is correct. Be aware when defining variables inside header files though, for example STARTING_FEN in board.hpp. If multiple source files include that, they would each cause a new variable to be defined, but since they have the same name in the global namespace, this can lead to a conflict at link time. Here it is fine because STARTING_FEN is const and since this is C++, but without const this wouldn't compile. If you are aware of this, it is fine.

Note that if you have non-const variables you want to define in header files, you can make the inline since C++17.

• Is SDL2 used properly? (eg. is there a better way to handle events, draw the board...)

Using SDL_Renderer is great for 2D programs. You enabled VSYNC which is also great. However, if most of the time nothing changes on screen, you can avoid even that. Use SDL_WaitEvent() to wait for something to happen.

You also have fixed the size of the window. Consider that someone might want to resize it, for example if they have a much larger or smaller screen than what you assume. Set the SDL_WINDOW_RESIZABLE flag when creating the SDL_Window. The nice thing about SDL_Renderer is that the rendering surface can have a different size than the window has, and SDL will take care of scaling if necessary.

• Is my game logic correct? (eg. a better way to check bishop moves, pawn moves...)

Be aware that C++ handles expressions like a < b < c differently than Python does. While Python handles this the way you'd expect this from mathematics, in C++ it evaluates the first comparison first, which yields a boolean true or false value, and then that boolean is compared to the third value. So 0 <= indices.first <= 7 will always be true, because 0 <= indices.first will be true or false, but true (1) and false (0) will always be considered less than 7. You have to write it in the form a < b && b < c:

while (0 <= indices.first && indices.first <= 7 &&
       0 <= indices.second && indices.second <= 7 &&
       indices != ending) {
    …
}

I also recommend you make more helper functions to simplify the code. For example, in isValidBishopMove(), the very first if-statement should be replaced with something that looks like:

if (!isDiagonalMove(starting, ending)) {
    return false;
}

• Is my piece/board representation good?

Using a two-dimensional array makes sense. However, avoid using int as the type for things that are not actually integer numbers. Since a board is made up of 8 by 8 squares, create a struct named Square, so you can write:

Square board[8][8];

And Square should describe what is on a given square, so for example:

struct Square {
    enum Color: std::uint8_t {
        NONE,
        WHITE,
        BLACK,
    } color;

    enum Piece: std::uint8_t {
        PAWN,
        KNIGHT,
        …,
    } piece;
};

In the above, I gave each enum an explicit type, so the struct only takes 2 bytes, as opposed to int being 4 bytes on most platforms. However, there are many other ways to represent chess boards, with various tradeoffs in complexity and performance.

Use enums where possible, and don't fall back to int. Statements like if (piece == 0x00) should not appear in your code.

• Are there missed optimizations/shortcuts?

For a simple GUI for playing chess, without a computer AI, this looks fine. I don't think you should worry about performance here. Focus on making the code more readable.

• Generally, is this idiomatic C++?

It doesn't look too bad. Some things that can be improved are:

• Avoid writing this->, it's almost never necessary in C++. If you tend to give local variables in member functions the same name as class member variables, consider renaming either the local variable, or add a prefix like m_ to member variables. The latter is a common code style.
• Don't use std::endl, write '\n'.
• Pass by (const) reference instead of by pointer where possible.
• Use constexpr for constants where possible.

Use std:: also for C functions

If you include the C header files using #include <csomething> instead of #include <something.h>, which you indeed should, make sure you prepend std:: to the functions and variables you use from those header files. While often it will work without, it is not guaranteed by the C++ standard that the C functions will be available in the global namespace.

For math functions, using the std:: version has the additional benefit of automatically selecting the right version depending on the type of the argument; for example if you would pass a long to abs() it would be silently truncated to an int, but std::abs() will do the right thing.

Carefully read the documentation of the <cctype> functions

The functions from the <cctype> header do not work on chars, but on ints, and they handle negative values differently than expected. For 7-bit ASCII characters it's not a problem, but if you have high ASCII characters or UTF-8 characters, chars might have negative values, and this causes issues. Make sure you cast to unsigned char before you pass characters to functions like std::isdigit().

Prefer a struct over a std::pair

There are certainly cases where you want to use a std::pair or std::tuple, but if you can define a struct instead, prefer that. You can give struct members useful names, whereas std::pair forces you to use .first and .second, and then you have to remember the order of things. So instead of making Location an alias for a std::pair<int, int>, write:

struct Location {
    int row;
    int col;
};

You also don't need a makeLocation() function, you can just write:

piece = getPieceAt({i, j});

Make more use of the types you define

It's very good to create a type like Location instead of passing two ints everywhere. However, you could use Location in more places. For example, you can define a member function that takes a Location as an argument, and returns a reference to an element of board[][]:

Square& Board::squareAt(Location postion) {
    return board[position.row][position.col];
};

That will simplify other functions, for example:

void Board::makeMove(Location starting, Location ending) {
    squareAt(ending) = squareAt(starting);
    squareAt(starting) = {};
}

You could also overload math operators and write your own abs() function that takes Location. That way you could write:

bool Game::isValidKingMove(Location starting, Location ending) {
    Location abs_diff = abs(starting - ending);
    return (abs_diff.row | abs_diff.col) == 1;
}