Game of Runes: a similar concept to chess

I have created a game called 'runes' in C. It is similar to Chess, but has a bigger board and different pieces. I would like to know how I can improve the code, for example, by adding proper error checking and by fixing any bugs. (This is probably not called 'broken code' because the main functions work fine.) Also, there is a large amount of spaghetti code in the middle and I don't know how to fix it.

```
/*
* runes.c The Game of Runes, similar to chess.
*
* RULES
* Pieces: Wizard, Peasant, Knight, Emperor, Archer, Dragon.
*
* Attack Abilities:
* Wizard can attack wizard, peasant, knight, emperor, archer.
* Peasant can attack peasant, knight.
* Knight can attack peasant, knight, emperor.
* Emperor cannot attack.
* Archer can attack anything with up to 2 squares between it and the target.
* Dragon can attack anything.
*
* Movement Abilities:
* All pieces can move 1 square in any direction except for the following:
* Knight. It moves in the L shape, as in chess. It can jump over pieces.
* Dragon. Each turn, it can move 3 squares in any direction. It can jump
* over pieces. However, it cannot hover above a piece.
*
* Misc Abilities:
* The Wizard can protect all friendly pieces right next to him.
*
* Capturing Pieces:
* All pieces can kill enemy pieces if they can walk over them, with two
* exceptions:
* Archer. It can kill enemy pieces with 2 squares between them, 1 if
* diagonal. 1 piece cannot block the arrow, but 2 pieces can.
* Wizard. Same as Archer.
* Dragon. It can kill enemy pieces with up to 1 square between them, 1 if
* diagonal
*
* The Wizard, Dragon and Emperor are major pieces.
* The Peasant, Knight, and Archer are minor pieces.
* In general, major pieces are more "valuable" than minor pieces.
*
* Each turn, you can move up to 9 pieces, but you must move at least one.
* This is so you can move either of your four "battalions" at once (see
* diagram below).
*
* TECHNICAL INFORMATION
* There

First, let me say that the quality of this code is higher than the usual C posted here, so well done! It compiles without errors or warnings, is formatted nicely and has useful and informative comments. There are still some things I noticed that may help you improve your code.

Don't leak memory

Both malloc and getline allocate memory but this code does not contain any calls to free. This is easily addressed by modifying main:

for (i = 0; i  ", current_player);
    line = read_command_line();
    args = split_line(line);
    if (run_runes_commands(args)) {
        i--;
    }
    free(line);
    free(args);
}

Avoid global variables

The current_player and board variables would be better as variables within the scope of main instead of global variables. Then pass them to the functions that need them. This would make the linkage among the functions explicit rather than implicit and leads to more robust, more maintainable code. In fact, I'd probably wrap both of these in a structure, neatly encapsulating the game state.

Create a strength table

Based on the text description of the game, some pieces are stronger than others, and we can create a chart:

D A W P K E
D 1 1 1 1 1 1 = 6
A 1 1 1 1 1 1 = 6
W   1 1 1 1 1 = 5
K     1 1 1 1 = 4 
P       1 1 1 = 3
E             = 0

On the left side of the chart is the attacking piece and the columns represent the piece that is attacked. We can now simplify the code by using a function that returns the strength of a piece:

unsigned short get_strength(char p)
{
    switch (toupper(p)) {
        case 'D':
        case 'A':
            return 6;
            break;
        case 'W':
            return 5;
            break;
        case 'K':
            return 4;
            break;
        case 'P':
            return 3;
            break;
        default:
            return 0;
    }
}

Now your "spaghetti" becomes this very neat function:

unsigned short int is_stronger_than(unsigned short int p1x, unsigned short int p1y, unsigned short int p2x, unsigned short int p2y)
{
    unsigned short s1 = get_strength(get_piece_on_board(p1x, p1y));
    unsigned short s2 = get_strength(get_piece_on_board(p2x, p2y));
    return s1 > 0 && s1 >= s2;
}

Reduce duplication of effort in the code

If we look at a function such as move_piece, it takes the old and new coordinates and calls a number of functions to check the legality of the move. This is fine, but the index calculation and the lookup of the piece is done repetitively. Instead, I think it would make more sense to look up the pieces at each coordinate pair once and then make decisions based on the value of the pieces. That is, if the first two lines of that function were these:

char oldpiece = get_piece_on_board(oldx, oldy);
char newpiece = get_piece_on_board(newx, newy);

I think it would be easy to see how the rest of the code could be redone.

Use functions to track attributes

In the piece_belongs_to_current_player there are a number of things I'd suggest changing. First, pass in the value of the piece rather than the coordinates. Second, rather than comparing to the global variable current_player, why not simply make the function return the corresponding player? If we do that, it can be rewritten much more clearly and simply like this:

char color(char piece)
{
    if (piece == EMPTY)
        return EMPTY;
    return islower(piece) ? 'b' : 'w';
}

Reduce the scope of variables

Within main the variable i only has meaning with the inner for loop, so it should instead be declared within the for loop:

for (unsigned short i = 0; i <= 9; i++) {

Eliminate "magic numbers"

As with the function above, there are a number of places in the code where 'b' signifies the black player and 'w' signifies the white player. Unfortunately, other places in the code 'w' signifies a black wizard. Since you already have a #define for the pieces, and since the letters 'b' and 'w' are only shown to the player in the context of main, I'd recommend eliminating these and using this instead:

#define BLACK 0
#define WHITE 1

Now main could look like this:

int main(void)
{
    char *line;
    char **args;
    const char players[] = "bw";

    print_header();
    print_menu();

    /* Main loop */
    for (char current_player = WHITE; ; current_player = WHITE - current_player) {
        for (unsigned short i = 0; i  ", players[(int)current_player]);
            line = read_command_line();
            args = split_line(line);
            if (run_runes_commands(args, current_player)) {
                i--;
            }
            free(line);
            free(args);
        }
    }
}

Obviously, I have altered current_player (but not yet board) to be local, and have changed the corresponding functions to use BLACK and WHITE instead of the letters.

Eliminate return 0 at the end of main

Since C99, the compiler automa

StackExchange Code Review Q#99330, answer score: 6