Game of Runes: revised version

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, with user interface improvements.

This question is not a duplicate of that question, because this one has a revised and easier to read version of the code in the other one.

```
/*
* 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 cannot attack, can only shoot.
* 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.
*
* Shooting 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).
*
* The objective of this game is to kill the opposing Dragon AND the opposing
* Emperor.
*
* TECHNICAL INFORMATION
* There is an array of 256 elements which describes the position of the
* pieces on the board. Star

This code is improved and I see that you've added the command shoot. I think there are still some things you could do to further improve the code.

Enforce the rules

It's a bit disappointing to be able to win in two moves:

shoot 9 16 8 1
shoot 9 16 9 1

This shouldn't be possible because the dragon should not be able to shoot across the entire board. The code should enforce the rules.

Validate the input

Nothing in the code currently prevents moving a piece completely outside the space allocated for the board. The run_runes_commands routine should validate the coordinates or the underlying shoot or move handlers should do so.

Store the game state in a struct

Right now there are two essential things that represent the game state, which are the board itself and the current_player. Additionally, instead of scanning the entire board each time, it would be possible to keep track of whether both sides still had both dragon and emperor pieces. I'd suggest storing all of that information in a single struct and then passing a pointer to the struct to the relevant functions.

Simplify checking for stronger piece

Rather than passing coordinates, pass piece values into is_stronger_than:

unsigned short is_stronger_than(char oldpiece, char newpiece)
{
    unsigned short s1 = get_strength(oldpiece);
    unsigned short s2 = get_strength(newpiece);
    return s2 > s1 || s1 == 0;
}

Now the calling code is simplified as well:

if (is_stronger_than(oldpiece, newpiece)) {

You can remove the need to check explictly for an empty square by making get_strength return the value of 1 for an archer or emperor and 0 for an empty square.

Simplify checking for an empty square

The first if clause of shoot() is overly complex:

if (get_piece_on_board(sx, sy, board) != 'w' && get_piece_on_board(sx, sy, board) != 'W' && get_piece_on_board(sx, sy, board) != 'a' && get_piece_on_board(sx, sy, board) != 'A' && get_piece_on_board(sx, sy, board) != 'd' && get_piece_on_board(sx, sy, board) != 'D') {

It could be greatly simplified:

if (shooter == EMPTY) {

Be careful in how you arrange if...else tests

In the move_piece routine, the message "The blank is not listening to you" will never be printed because of the first if clause:

if (get_colour(oldpiece) != current_player) {

Instead the check for an empty square should be first.

if (oldpiece == EMPTY) {

Use const where practical

In the get_piece_on_board routine, the underlying board is not altered. Make this explicit by declaring that parameter const:

char get_piece_on_board(unsigned short x, unsigned short y, const char board[])

Print square numbers to aid the user

It's somewhat difficult to enter valid moves because the squares are not labeled. Instead, consider printing the needed square numbering as the board is rendered:

void render_board(const char board[])
{
    unsigned short x;
    unsigned short y;
    const char horz[]="  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+";

    puts(horz);
    for (y = 0; y 1   2   3   4   5   6   7   8   9  10  11  12  13  14  15  16");
}

Note that this also uses a single const char[] for the horizontal line separator and uses puts instead of printf where appropriate.

StackExchange Code Review Q#100363, answer score: 11