1
\$\begingroup\$

So I implemented a simple console-based, offline player vs. player chess game in C++17. I am aware that there are multiple chess-specific features I have not yet added, but the core gameplay works perfectly fine. This is quite a big project for a review, but I would be very grateful for any kind of advice. The code is structured in 3 classes:

  1. Cchess_game is responsible for the main game loop.
  2. Cchess_board is by far the biggest class, as it is responsible for anything related to the game board, so basically the whole game logic.
  3. Cplayer honestly just exists for possible extensions in the future, like player specific timers. At the moment it only manages the user input.

Additionally, there is a header game_constants.h, which includes the global constants used in the game. This is the code:

source.cpp

#include "Cchess_game.h"

int main()
{
    chess::Cchess_game chess;
    int32_t winner = chess.play();

    std::cout << "\nPlayer " << winner << " won!\n";

    return 0;
}

Cchess_game.h

#ifndef CCHESS_GAME_H
#define CCHESS_GAME_H

#include "Cchess_board.h"
#include "Cplayer.h"

namespace chess
{
    class Cchess_game
    {
    public:

        Cchess_game() : m_player1{ Cplayer() }, m_player2{ Cplayer() }, m_board{ Cchess_board() }, m_player_switch{ true }{};
        game_result play(); // play single game of chess and return 1,2 for winner and 0 for draw

    private:

        Cplayer m_player1;
        Cplayer m_player2;
        Cchess_board m_board;
        bool m_player_switch;

    };
#endif
}

Cchess_game.cpp

#include "Cchess_game.h"

namespace chess
{
    game_result Cchess_game::play()
    {
        game_result result;
        std::pair<int32_t, int32_t> from_coords;
        std::pair<int32_t, int32_t> to_coords;
        Cplayer* player = nullptr;

        do
        {
            // get pointer to current player to avoid repetition
            if (m_player_switch)
            {
                player = &m_player1;
            }
            else
            {
                player = &m_player2;
            }

            // display current game state
            m_board.display();
            std::cout << "\n\n";

            std::cout << "Player " << !m_player_switch + 1 << "'s turn: \n\n";

            // get move info until move is valid
            do
            {
                std::cout << "Please enter the position of the figure you want to move: ";
                from_coords = player->get_pos();
                std::cout << "Please enter the position you want your figure to move to: ";
                to_coords = player->get_pos();

            } while (!m_board.is_valid(from_coords, to_coords, m_player_switch));

            // execute move and update board
            m_board.move(from_coords, to_coords);

            // switch player
            m_player_switch = !m_player_switch;

            // clear console
            std::cout << std::flush;
            system("CLS");

        } while ((result = m_board.game_state()) == still_playing);

        return result;
    }
}

Cchess_board.h

#ifndef CCHESS_BOARD_H
#define CCHESS_BOARD_H

#include "game_constants.h"
#include <array>
#include <iostream>
#include <algorithm>
#include <unordered_map>
#include <stdlib.h>

namespace chess
{
    class Cchess_board
    {
    public:

        Cchess_board(); // initialise default chess board
        game_result game_state() const; // return current game state
        void display() const; // display current board in console
        bool is_valid(std::pair<int32_t, int32_t> from, std::pair<int32_t, int32_t> to, bool player_switch) const; // return true if move is valid
        void move(std::pair<int32_t, int32_t> from, std::pair<int32_t, int32_t> to); // execute move

    private:

        bool find_figure(chess_board_state figure) const; // return true if figure is in board
        bool is_relative_field(std::pair<int32_t, int32_t> from, std::pair<int32_t, int32_t> to, int32_t x_shift, int32_t y_shift) const; // return true if position "to" is relative to "from" with shifts
        bool is_mate(std::pair<int32_t, int32_t> field, std::pair<chess_board_state, chess_board_state> figure_range) const; // return true if field is mate
        bool check_linear_move(std::pair<int32_t, int32_t> from, std::pair<int32_t, int32_t> to) const; // return true if move is linear and valid
        bool check_diagonal_move(std::pair<int32_t, int32_t> from, std::pair<int32_t, int32_t> to) const; // return true if move is diagonal and valid

        std::array<std::array<chess_board_state, chess_board_width>, chess_board_height> m_board;
        int32_t m_invert_y; // help value to distract from to invert index

    };
#endif
}

Cchess_board.cpp

#include "Cchess_board.h"

namespace chess
{
    Cchess_board::Cchess_board()  
    {
        m_board = std::array<std::array<chess_board_state, chess_board_width>, chess_board_height>{ {
            { p2_rook, p2_knight, p2_bishop, p2_queen, p2_king, p2_bishop, p2_knight, p2_rook },
            { p2_pawn, p2_pawn,   p2_pawn,   p2_pawn,  p2_pawn, p2_pawn,   p2_pawn,   p2_pawn },
            { empty,   empty,     empty,     empty,    empty,   empty,     empty,     empty },
            { empty,   empty,     empty,     empty,    empty,   empty,     empty,     empty },
            { empty,   empty,     empty,     empty,    empty,   empty,     empty,     empty },
            { empty,   empty,     empty,     empty,    empty,   empty,     empty,     empty },
            { p1_pawn, p1_pawn,   p1_pawn,   p1_pawn,  p1_pawn, p1_pawn,   p1_pawn,   p1_pawn },
            { p1_rook, p1_knight, p1_bishop, p1_queen, p1_king, p1_bishop, p1_knight, p1_rook }
            }};

        m_invert_y = chess_board_height - 1;
    }

    game_result Cchess_board::game_state() const
    {
        // check for p1 win
        if (!find_figure(p2_king))
        {
            return p1_win;
        }

        // check for p2 win
        if (!find_figure(p1_king))
        {
            return p2_win;
        }

        return still_playing;
    }

    void Cchess_board::display() const
    {
        std::unordered_map<chess_board_state, std::string> figure_to_string = {
        {empty,"empty       "}, {p1_pawn,"white_pawn  "}, {p1_knight,"white_knight"}, {p1_bishop,"white_bishop"}, {p1_rook,"white_rook  "}, {p1_queen,"white_queen "}, {p1_king,"white_king  "},
                                {p2_pawn,"black_pawn  "}, {p2_knight,"black_knight"}, {p2_bishop,"black_bishop"}, {p2_rook,"black_rook  "}, {p2_queen,"black_queen "}, {p2_king,"black_king  "}
        };

        std::array<int32_t, chess_board_height> y_axis_descr = { 8, 7, 6, 5, 4, 3, 2, 1 };

        // print x-axis description
        std::cout << "  a             b             c             d             e             f             g             h\n";

        // print line by line
        for (int32_t i = 0; i < chess_board_height; i++)
        {
            std::cout << static_cast<int>(y_axis_descr[i]) << " ";
            for (chess_board_state state : m_board[i])
            {
                std::cout << figure_to_string[state].c_str() << "  ";
            }
            std::cout << "\n";
        }

    }

    bool Cchess_board::is_valid(std::pair<int32_t, int32_t> from, std::pair<int32_t, int32_t> to, bool player_switch) const
    {
        int32_t from_x = from.first;
        int32_t from_y = m_invert_y - from.second;
        int32_t to_x = to.first;
        int32_t to_y = m_invert_y - to.second;

        std::pair<chess_board_state, chess_board_state> figure_range;

        // change range of own figures depending on player switch
        if (player_switch)
        {
            figure_range.first = p1_pawn;
            figure_range.second = p1_king;
        }
        else
        {
            figure_range.first = p2_pawn;
            figure_range.second = p2_king;
        }
        
        // check if move is valid based on figure
        // switch statement was not possible, because it requires compile-time constants

        // pawn
        if (m_board[from_y][from_x] == figure_range.first)
        {      
            if ((is_relative_field(from, to,  0, -1) && m_board[to_y][to_x] == empty && player_switch)                                  || // 1 forward - player 1
                (is_relative_field(from, to,  0, -2) && m_board[to_y][to_x] == empty && from_y == 6                  && player_switch)  || // 2 forward from start - player 1
                (is_relative_field(from, to, -1, -1) && !is_mate(to, figure_range)   && m_board[to_y][to_x] != empty && player_switch)  || // attack diagonal - player 1
                (is_relative_field(from, to,  1, -1) && !is_mate(to, figure_range)   && m_board[to_y][to_x] != empty && player_switch)  || // attack diagonal - player 1

                (is_relative_field(from, to,  0,  1) && m_board[to_y][to_x] == empty && !player_switch)                                 || // 1 forward - player 2
                (is_relative_field(from, to,  0,  2) && m_board[to_y][to_x] == empty && from_y == 1                  && !player_switch) || // 2 forward from start - player 2
                (is_relative_field(from, to, -1,  1) && !is_mate(to, figure_range)   && m_board[to_y][to_x] != empty && !player_switch) || // attack diagonal - player 2
                (is_relative_field(from, to,  1,  1) && !is_mate(to, figure_range)   && m_board[to_y][to_x] != empty && !player_switch))   // attack diagonal - player 2 
            {
                return true;
            }
        }
        // knight
        else if (m_board[from_y][from_x] == figure_range.first + 1)
        {
            if ((is_relative_field(from, to, -1, -2) && !is_mate(to, figure_range)) || // 2 up, 1 left
                (is_relative_field(from, to,  1, -2) && !is_mate(to, figure_range)) || // 2 up, 1 right
                (is_relative_field(from, to, -1,  2) && !is_mate(to, figure_range)) || // 2 down, 1 left
                (is_relative_field(from, to,  1,  2) && !is_mate(to, figure_range)) || // 2 down, 1 right
                (is_relative_field(from, to,  2, -1) && !is_mate(to, figure_range)) || // 2 right, 1 up
                (is_relative_field(from, to,  2,  1) && !is_mate(to, figure_range)) || // 2 right, 1 down
                (is_relative_field(from, to, -2, -1) && !is_mate(to, figure_range)) || // 2 left, 1 up
                (is_relative_field(from, to, -2,  1) && !is_mate(to, figure_range)))   // 2 left, 1 down
            {
                return true;
            }
        }
        // bishop
        else if (m_board[from_y][from_x] == figure_range.first + 2)
        {
            if (check_diagonal_move(from, to) && !is_mate(to, figure_range))
            {
                return true;
            }
        }
        // rook
        else if (m_board[from_y][from_x] == figure_range.first + 3)
        {
            if (check_linear_move(from, to) && !is_mate(to, figure_range))
            {
                return true;
            }
        }
        // queen
        else if (m_board[from_y][from_x] == figure_range.first + 4)
        {
            if ((check_diagonal_move(from, to) || check_linear_move(from, to)) && !is_mate(to, figure_range))
            {
                return true;
            }
        }
        // king
        else if (m_board[from_y][from_x] == figure_range.first + 5)
        {

            if ((is_relative_field(from, to,  0, -1) && !is_mate(to, figure_range)) || // 1 up
                (is_relative_field(from, to,  1, -1) && !is_mate(to, figure_range)) || // 1 up, 1 right
                (is_relative_field(from, to, -1, -1) && !is_mate(to, figure_range)) || // 1 up, 1 left
                (is_relative_field(from, to,  1,  0) && !is_mate(to, figure_range)) || // 1 right
                (is_relative_field(from, to, -1,  0) && !is_mate(to, figure_range)) || // 1 left
                (is_relative_field(from, to,  0,  1) && !is_mate(to, figure_range)) || // 1 down
                (is_relative_field(from, to,  1,  1) && !is_mate(to, figure_range)) || // 1 down, 1 right
                (is_relative_field(from, to, -1,  1) && !is_mate(to, figure_range)))   // 1 down, 1 left
            {
                return true;
            }
        }
        else
        {
            std::cout << "\nThis is not one of your figures.\n\n";
            return false;
        }

        std::cout << "\nNot a valid move.\n\n";

        return false;
    }

    bool Cchess_board::is_relative_field(std::pair<int32_t, int32_t> from, std::pair<int32_t, int32_t> to, int32_t x_shift, int32_t y_shift) const
    {
        return (m_invert_y - from.second + y_shift == m_invert_y - to.second &&
                from.first               + x_shift == to.first              );
    }

    bool Cchess_board::is_mate(std::pair<int32_t, int32_t> field, std::pair<chess_board_state, chess_board_state> figure_range) const
    {
        return !(m_board[m_invert_y - field.second][field.first] < figure_range.first ||
                 m_board[m_invert_y - field.second][field.first] > figure_range.second);
    }

    bool Cchess_board::check_linear_move(std::pair<int32_t, int32_t> from, std::pair<int32_t, int32_t> to) const
    {
        // is linear?
        if (from.first == to.first || m_invert_y - from.second == m_invert_y - to.second)
        {
            int32_t temp_x = from.first;
            int32_t temp_y = m_invert_y - from.second;

            // is valid?
            // x-axis case
            while (temp_x != to.first)
            {
                if (temp_x > to.first)
                {
                    temp_x--;
                }
                else
                {
                    temp_x++;
                }

                if (m_board[m_invert_y - from.second][temp_x] != empty && temp_x != to.first)
                {
                    return false;
                }
            }
            

            // y-axis case
            while (temp_y != m_invert_y - to.second)
            {
                if (temp_y > m_invert_y - to.second)
                {
                    temp_y--;
                }
                else
                {
                    temp_y++;
                }

                if (m_board[temp_y][from.first] != empty && temp_y != m_invert_y - to.second)
                {
                    return false;
                }
            }

            return true;
        }

        return false;
    }

    bool Cchess_board::check_diagonal_move(std::pair<int32_t, int32_t> from, std::pair<int32_t, int32_t> to) const
    {
        int32_t temp_x = from.first;
        int32_t temp_y = m_invert_y - from.second;

        // is diagonal?
        if (abs((m_invert_y - from.second) - (m_invert_y - to.second)) == abs(from.first - to.first))
        {
            // is valid?
            while (temp_x != to.first)
            {
                if (temp_x > to.first)
                {
                    temp_x--;
                }
                else
                {
                    temp_x++;
                }

                if (temp_y > m_invert_y - to.second)
                {
                    temp_y--;
                }
                else
                {
                    temp_y++;
                }

                if (m_board[temp_y][temp_x] != empty && temp_x != to.first)
                {
                    return false;
                }
            }

            return true;
        }

        return false;
    }

    void Cchess_board::move(std::pair<int32_t, int32_t> from, std::pair<int32_t, int32_t> to)
    {
        // assign figure to new position
        m_board[m_invert_y - to.second][to.first] = m_board[m_invert_y - from.second][from.first];

        // clear old position
        m_board[m_invert_y - from.second][from.first] = empty;
    }

    bool Cchess_board::find_figure(chess_board_state figure) const
    {
        for (auto& row : m_board)
        {
            if (std::find(row.begin(), row.end(), figure) != row.end())
            {
                return true;
            }
        }
        return false;
    }
}

Cplayer.h

#ifndef CPLAYER_H
#define CPLAYER_H

#include "game_constants.h"
#include <utility>
#include <string>
#include <iostream>
#include <ctype.h>
#include <unordered_map>

namespace chess
{
    class Cplayer
    {
    public:

        std::pair<int32_t, int32_t> get_pos() const;

    };
#endif
}

Cplayer.cpp

#include "Cplayer.h"

namespace chess
{
    std::pair<int32_t, int32_t> Cplayer::get_pos() const
    {
        // dictionary to convert x-axis char to integer index
        std::unordered_map<char, int32_t> convert_to_int({ {'a',0}, {'b',1}, {'c',2}, {'d',3}, {'e',4}, {'f',5}, {'g',6}, {'h',7} });

        std::string pos;

        // get valid user input
        for (;;)
        {
            std::cin >> pos;
            // check for
            //  size               a-h                                                    <= 8               >= 1               
            if (pos.size() == 2 && convert_to_int.find(pos[0]) != convert_to_int.end() && pos[1] - '0' <= 8 && pos[1] - '0' >= 1)
            {
                return { convert_to_int[pos[0]], pos[1] - '0' - 1 };
            }
            std::cout << "\nInvalid input format.\n\n";
        }
    }
}

game_constants.h

#ifndef GAME_CONSTANTS_H
#define GAME_CONSTANTS_H

#include <stdint.h>

constexpr int32_t chess_board_height = 8;
constexpr int32_t chess_board_width = 8;

enum chess_board_state
{
    empty = 0,

    p1_pawn = 1, 
    p1_knight,
    p1_bishop,
    p1_rook,
    p1_queen,
    p1_king,

    p2_pawn = 11,
    p2_knight,
    p2_bishop,
    p2_rook,
    p2_queen,
    p2_king,
};

enum game_result
{
    draw = 0, p1_win, p2_win, still_playing
};

#endif

I would especially appreciate feedback regarding Cchess_board::is_valid as I am really unsatisfied with my solution.

Improve this question
\$\endgroup\$
6
  • \$\begingroup\$ Do you have reasons for not using inheritance to represent the chess pieces? \$\endgroup\$
    – theProgrammer
    Jan 2, 2021 at 18:47
  • \$\begingroup\$ @theProgrammer Why would I use classes at all to represent the pieces? Enum's work just fine imo \$\endgroup\$
    – Tom Gebel
    Jan 2, 2021 at 20:57
  • \$\begingroup\$ @TomGebel Because it makes checking valid moves trivial. You simply ask the piece if it made a valid move rather than having a big switch statement. This is the classic teaching example on how to use OOD. \$\endgroup\$
    – Martin York
    Jan 2, 2021 at 21:02
  • \$\begingroup\$ @MartinYork But it does not really reduce code length, does it? Couldn't one argue that it is not worth the effort just for a little bit more readability? \$\endgroup\$
    – Tom Gebel
    Jan 2, 2021 at 21:06
  • \$\begingroup\$ One could argue that the whole point of writting in a human readable language is to make it as readable as possible. The largest cost of any application is maintaining it. Thus reducing this cost is more important than any other (unless explicitly required) thus readability is the MOST important thing for a professional programmer. \$\endgroup\$
    – Martin York
    Jan 2, 2021 at 21:09

1 Answer 1

Reset to default
2
\$\begingroup\$

Design

You have two much responsibility in a single class (the board).

  1. Why does the board need to know how to display itself.
    Would be a good idea to inject the display class into the board at construction.
    This will allow for future enhancements into the interface without having to modify the board.
  • Separating the board and display allows for better separation of logic and thus testing.
  1. Player input does not need to be part of the board.
    Would be a good idea to inject player objects into the board thus allowing you to specialize later.
  • Separating the board and player allows for better separation of logic and thus testing.
  • This will make adding the AI trivial (once you have written the AI player).

Why not use OO design for the pieces. You could have just as easily written this implementation in C.

Overall I find the coupling in the code to tight.

Code Review

Namespace chess implies you then don't need to keep adding `chess to everything in that namespace (we now know that everything is chess related).

    chess::Cchess_game chess;
    ^^^^^   ^^^^^            OK. Its chesss we get it.

Why is the C at the front here. Are you doing reverse polish type names? C=>class. That was sort of good style 30 years ago. Its now considered rather redundant. Not going to say bad explicitly but I think its horrible.

You are changing the type of winner here:

    int32_t winner = chess.play();

    // The interface to `plat()` is
    game_result play();

Why are you not getting a game_result here?

Would be nice with a more human readable result "White" or "Black" or player name?

    std::cout << "\nPlayer " << winner << " won!\n";

Redundant:

    return 0;

Not required in C++. Most people only return a value at the end here if there is an option for failure (and thus there can be another result). If I see return 0 at the end I start looking for the failure cases in main().

None of that is requred.

        Cchess_game() : m_player1{ Cplayer() }, m_player2{ Cplayer() }, m_board{ Cchess_board() }, m_player_switch{ true }{};

One variable definition per line please.

        Cchess_game() 
            : m_player1{ Cplayer() }     // Default Create temporary object to initialize member!!
            , m_player2{ Cplayer() }
            , m_board{ Cchess_board() }
            , m_player_switch{ true }
        {};  // Now I can see the un-needed ';' at the end.

There is no prize for using the least number of lines. The whole point is to make this as readable and understandable to the next person that comes along to fix it (which is probably going to be you). Save yourself time in the future by taking some time now.

The default constructors for each member would have taken care of this. Personally I would change this so that game takes references to players and board and then passed these references to the constructor. This way my main would have looked like:

 int main()
 {
     Chess::PlayerHuman   player1;
     Chess::PlayerAI      player2;
     Chess::Board         board;
     Chess::Display       display(board);

     Chess::Game          game(player1, player2, board, display);

     game.play();
}

Comments contradict the code:

        game_result play(); // play single game of chess and return 1,2 for winner and 0 for draw

Does the maintainer fix the code to match the comment or fix the comment. Extra waisted time. This is why bad comments are worse than zero comments. Explain why you are doing something in comments not how (the code does how).

Though an enum is convertible to int that is not the same as returning an int.

This looks like a BUG:

    };
#endif
}

Dangerous to have uninitialized variables.

        game_result result;

These variables are not used until the next scope block.

        std::pair<int32_t, int32_t> from_coords;
        std::pair<int32_t, int32_t> to_coords;
        Cplayer* player = nullptr;

Declare variables are close to the point of usage as possible.

That's a horrible named variable (m_player_switch).

            // get pointer to current player to avoid repetition
            if (m_player_switch)
            {
                player = &m_player1;
            }
            else
            {
                player = &m_player2;
            }

Its not about switching player. If its true then we use player one. If it is false we use player two. Why not call it player ID?

I would simplify this: bool isWhiteTurn = true;

           .... 
           Player& currentPlayer = isWhiteTurn ? m_player1 : m_player2;
           ....
           isWhiteTurn = !isWhiteTurn;

This loop should be part of the Player Code.

            {
                std::cout << "Please enter the position of the figure you want to move: ";
                from_coords = player->get_pos();
                std::cout << "Please enter the position you want your figure to move to: ";
                to_coords = player->get_pos();

            } while (!m_board.is_valid(from_coords, to_coords, m_player_switch));

If you swap out Humans for AI then writing to the output is confusing for the player. The type of output is dependent on the type of player. So move this to the Player code.

Also you don't ask a player for a position. You ask a player for a move. Ask the player for a position twice with no context is bad API choice.

Clearing the screen is not part of the game. This should be part of the display code. Sure you should tell the dislay that the current move is over. But what the display does is not part of the game.

            // clear console
            std::cout << std::flush;
            system("CLS");

Seems like another BUG

    };
#endif
}

When possible us an initializer list:

    Cchess_board::Cchess_board()  
    {
        m_board = std::array<std::array<chess_board_state, chess_board_width>, chess_board_height>{ {
            { p2_rook, p2_knight, p2_bishop, p2_queen, p2_king, p2_bishop, p2_knight, p2_rook },
            { p2_pawn, p2_pawn,   p2_pawn,   p2_pawn,  p2_pawn, p2_pawn,   p2_pawn,   p2_pawn },
            { empty,   empty,     empty,     empty,    empty,   empty,     empty,     empty },
            { empty,   empty,     empty,     empty,    empty,   empty,     empty,     empty },
            { empty,   empty,     empty,     empty,    empty,   empty,     empty,     empty },
            { empty,   empty,     empty,     empty,    empty,   empty,     empty,     empty },
            { p1_pawn, p1_pawn,   p1_pawn,   p1_pawn,  p1_pawn, p1_pawn,   p1_pawn,   p1_pawn },
            { p1_rook, p1_knight, p1_bishop, p1_queen, p1_king, p1_bishop, p1_knight, p1_rook }
            }};

        m_invert_y = chess_board_height - 1;
    }

    void Cchess_board::display() const
    {

This structure is initialized every time this function is called.

        std::unordered_map<chess_board_state, std::string> figure_to_string = {
        {empty,"empty       "}, {p1_pawn,"white_pawn  "}, {p1_knight,"white_knight"}, {p1_bishop,"white_bishop"}, {p1_rook,"white_rook  "}, {p1_queen,"white_queen "}, {p1_king,"white_king  "},
                                {p2_pawn,"black_pawn  "}, {p2_knight,"black_knight"}, {p2_bishop,"black_bishop"}, {p2_rook,"black_rook  "}, {p2_queen,"black_queen "}, {p2_king,"black_king  "}
        };

It should be a static member (inside the function (though if you made it a static member of the class so it could be used in other functions that would not bother me)).

Not sure this is requied (but if it was another static member):

        std::array<int32_t, chess_board_height> y_axis_descr = { 8, 7, 6, 5, 4, 3, 2, 1 };

You use this below to print out the row numbers.

 std::cout << static_cast<int>(y_axis_descr[i]) << " ";

 // Could we not just do:
 std::cout << (8-i) << " ";

This while section could have been simplified by using OO.

       m_board[from_y][from_x]->isGoodMove(to_x, to_y);

But even using enum you can simplify by using s switch:

       switch(m_board[from_y][from_x]) {
           case pawn:     checkMovePawn(from_y, form_x, to_y, to_x);  break;
           case castle:   checkMoveCastle(from_y, form_x, to_y, to_x);break;
           ....
       }

You check for mate on every combination of moves (pawn/knight/king). I would rather check for mate once, after I have figured out the move is valid (bishop/rook/queen).

Your test for "White" Vs Black version of pieces means the types need to be ordered. But its not very logical organized. See below. when we look at the piece enum.

Linear and diagnoal checks are the same thing and can be much simplified.

    xDist = to_x - from_x;
    yDist = to_y - from_y;


    if (xDist == 0 || yDist == 0 || abs(xDist) == abs(yDist)) {
        // Valid move
        xInc = xDist / abs(xDist); // 1 or -1
        yInc = yDist / abs(yDist); // 1 or -1
        
        int xStart = from_x + xInc;
        int yStart = from_y + yInc;
        for(; xStart != to_x && yStart != to_y;xStart += xInc, yStart += yInc) {
            // check for empty square.
        }
    }

This will simplify all your scanning checks.

Especially if your parameterize the call with if they are allowed that type of move.

 queen:  checkScanMove(true, true, 8, from, to); break;
 rook:   checkScanMove(true, false,8, from, to); break;
 bishop: checkScanMove(false,true, 8, from, to); break;
 king:   checkScanMove(true, true, 1, from ,to); break;

Its a square board?

constexpr int32_t chess_board_height = 8;
constexpr int32_t chess_board_width = 8;

These could have been done better.

enum chess_board_state
{
    empty = 0,

    p1_pawn = 1, 
    p1_knight,
    p1_bishop,
    p1_rook,
    p1_queen,
    p1_king,

    p2_pawn = 11,  // Why 11? 9 would have been better
    p2_knight,
    p2_bishop,
    p2_rook,
    p2_queen,
    p2_king,
};

If it was nine: Then you simply check if bit 3 (value 8) to see if it is a black or white you just test bit 3.

Also rather than p1_ and p2_ why not White_ or Black_

Good idea

enum game_result
{
    draw = 0, p1_win, p2_win, still_playing
};

But you never use it.

Improve this answer
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.