Path: csiph.com!usenet.pasdenom.info!weretis.net!feeder4.news.weretis.net!ecngs!feeder2.ecngs.de!newsfeed.freenet.ag!news2.euro.net!newsgate.cistron.nl!newsgate.news.xs4all.nl!post.news.xs4all.nl!not-for-mail
MIME-Version: 1.0
In-Reply-To: <2013021323250974803-chrishinsley@gmailcom>
References: <2013021323250974803-chrishinsley@gmailcom>
From: Oscar Benjamin <oscar.j.benjamin@gmail.com>
Date: Fri, 15 Feb 2013 11:22:14 +0000
Subject: Re: First attempt at a Python prog (Chess)
To: Chris Hinsley <chris.hinsley@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Cc: Python List <python-list@python.org>
Precedence: list
Newsgroups: comp.lang.python
Message-ID: <mailman.1802.1360927363.2939.python-list@python.org>
Lines: 298
NNTP-Posting-Host: 2001:888:2000:d::a6
Xref: csiph.com comp.lang.python:38914

On 13 February 2013 23:25, Chris Hinsley <chris.hinsley@gmail.com> wrote:
> New to Python, which I really like BTW.
>
> First serious prog. Hope you like it. I know it needs a 'can't move if your
> King would be put into check' test. But the weighted value of the King piece
> does a surprising emergent job.
>
> #!/usr/bin/python -tt
> # -*- coding: utf-8 -*-
> # Copyright (C) 2013 Chris Hinsley, GPL V3 License
>
> import sys
> import random
> import os
>
> PLY = 3
>
> EMPTY = 0
> BLACK = 1
> WHITE = 2
> NO_CAPTURE = 3
> MAY_CAPTURE = 4
> MUST_CAPTURE = 5
>
> def piece_type(piece):
>    return EMPTY if piece == 32 else BLACK if chr(piece) in 'KQRBNP' else
> WHITE

You call chr(piece) many times in this program. It would be better to
just have piece be a string rather than always converting it to one
every time you want to do something. Also comparing it with a numeric
code is fairly cryptic. I guess that ascii 32 is a space character but
I'd have to look that up to be sure.

>
> def display_board(board):
>    print '  a   b   c   d   e   f   g   h'
>    print '+---+---+---+---+---+---+---+---+'
>    for row in range(8):
>        for col in range(8):

Why not make board a list of lists. Then you can do:

for row in board:
    for piece in row:

rather than using range().

Or perhaps you could have a dict that maps position tuples to pieces,
e.g.: {(1, 2): 'k', ...}

>            sys.stdout.write('| ')
>            sys.stdout.write(chr(board[row * 8 + col]))
>            sys.stdout.write(' ')
>        sys.stdout.write('|')
>        print 8 - row
>        print '+---+---+---+---+---+---+---+---+'

You seem to be using sys.stdout.write as a way of printing without a
trailing newline. In Python 2 you can get this effect by using:
    print 'foo',

(note the trailing comma). In Python 3 you would do

   print('foo', end=' ')

You can use the Python 3 syntax in your Python 2 script if you do
"from __future__ import print_function" so that your script works on
Python 2 and 3.

Also I would probably separate the function that generates the text
representing the board from the code that actually sends that
information to stdout.


>
> def piece_moves(board, index, dx, dy, capture_flag, distance):
>    piece = board[index]
>    type = piece_type(piece)
>    cx = index % 8
>    cy = index / 8

You can use divmod for this:
cx, cy = divmod(index, 8)

Also in Python 3 index / 8 will return a float. Use // for floor
division in both versions ("from __future__ import division").

>    for step in range(distance):
>        nx = cx + (dx * (step + 1))
>        ny = cy + (dy * (step + 1))

Why not make a function that yields these values and loop over that?

def continued_moves(x, y, dx, dy):
    while 0 <= x < 8 and 0 <= y < 8:
        x += dx
        y += dy
        yield x, y

>        if nx in range(8) and ny in range(8):

Use chained comparisons 0 <= x < 8 rather than testing for membership
in a range object. "x in range(N, M)" creates (in Python 2) a list
integers and then (in 2 or 3) iterates over that list to find an
object equal to x. This is inefficient and not as clear.

>            newindex = ny * 8 + nx
>            newpiece = board[newindex]

With a list of lists you could access the board with board[ny][nx]
which is clearer. Or with the dict: board[(nx, ny)].

The code below is overly indented. consider factoring it into functions.

>            newtype = piece_type(newpiece)
>            if capture_flag == MUST_CAPTURE:
>                if newtype != EMPTY and newtype != type:
>                    board[index] = ' '
>                    if (ny == 0 or ny == 7) and chr(piece) in 'Pp':
>                        for promote in 'QRBN' if type == BLACK else 'qrbn':
>                            board[newindex] = promote
>                            yield board
>                    else:
>                        board[newindex] = piece
>                        yield board
>                    board[index], board[newindex] = piece, newpiece
>            elif capture_flag == MAY_CAPTURE:
>                if newtype == EMPTY or newtype != type:
>                    board[index], board[newindex] = ' ', piece
>                    yield board
>                    board[index], board[newindex] = piece, newpiece

Rather than modifying and unmodifying the board in place (which is
fragile), could you not just have the compute_score function compute
the score as if the move had taken place? Then you could just yield
the move and the score.

>                break
>            elif newtype == EMPTY:
>                board[index] = ' '
>                if (ny == 0 or ny == 7) and chr(piece) in 'Pp':
>                    for promote in 'QRBN' if type == BLACK else 'qrbn':
>                        board[newindex] = promote
>                        yield board
>                else:
>                    board[newindex] = piece
>                    yield board
>                board[index], board[newindex] = piece, newpiece
>        else:
>            break
>
> def pawn_moves(board, index, options):
>    for x, y, flag, distance in options:
>        for new_board in piece_moves(board, index, x, y, flag, distance):
>            yield new_board
>
> def other_moves(board, index, options, distance):
>    for x, y in options:
>        for new_board in piece_moves(board, index, x, y, MAY_CAPTURE,
> distance):
>            yield new_board
>
> def black_pawn_moves(board, index):
>    distance = 2 if index in range(8, 16) else 1
>    for new_board in pawn_moves(board, index, [(0, 1, NO_CAPTURE, distance),
> (-1, 1, MUST_CAPTURE, 1), (1, 1, MUST_CAPTURE, 1)]):
>        yield new_board
>
> def white_pawn_moves(board, index):
>    distance = 2 if index in range(48, 56) else 1
>    for new_board in pawn_moves(board, index, [(0, -1, NO_CAPTURE, distance),
> (-1, -1, MUST_CAPTURE, 1), (1, -1, MUST_CAPTURE, 1)]):
>        yield new_board

Do you really need separate functions for black and white pawns?

>
> def rook_moves(board, index):
>    for new_board in other_moves(board, index, [(0, -1), (-1, 0), (0, 1), (1,
> 0)], 7):
>        yield new_board
>
> def bishop_moves(board, index):
>    for new_board in other_moves(board, index, [(-1, -1), (-1, 1), (1, 1),
> (1, -1)], 7):
>        yield new_board
>
> def knight_moves(board, index):
>    for new_board in other_moves(board, index, [(-2, 1), (2, -1), (2, 1),
> (-1, -2), (-1, 2), (1, -2), (1, 2)], 1):
>        yield new_board
>
> def queen_moves(board, index):
>    for new_board in bishop_moves(board, index):
>        yield new_board
>    for new_board in rook_moves(board, index):
>        yield new_board
>
> def king_moves(board, index):
>    for new_board in other_moves(board, index, [(0, -1), (-1, 0), (0, 1), (1,
> 0), (-1, -1), (-1, 1), (1, 1), (1, -1)], 1):
>        yield new_board
>
> moves = {'P' : black_pawn_moves, 'p' : white_pawn_moves, \
>        'R' : rook_moves, 'r' : rook_moves, \
>        'B' : bishop_moves, 'b' : bishop_moves, \
>        'N' : knight_moves, 'n' : knight_moves, \
>        'Q' : queen_moves, 'q' : queen_moves, \
>        'K' : king_moves, 'k' : king_moves}
>
> def all_moves(board, turn):
>    for index, piece in enumerate(board):
>        if piece_type(piece) == turn:
>            for new_board in moves[chr(piece)](board, index):
>                yield new_board
>
> piece_values = {'K' : (1000000, 0), 'k' : (0, 1000000), \
>                'P' : (1, 0), 'p' : (0, 1), \
>                'N' : (3, 0), 'n' : (0, 3), \
>                'B' : (3, 0), 'b' : (0, 3), \
>                'R' : (5, 0), 'r' : (0, 5), \
>                'Q' : (9, 0), 'q' : (0, 9)}
>
> position_values = [0, 0, 0, 0, 0, 0, 0, 0, \
>                    0, 1, 1, 1, 1, 1, 1, 0, \
>                    0, 1, 2, 2, 2, 2, 1, 0, \
>                    0, 1, 2, 3, 3, 2, 1, 0, \
>                    0, 1, 2, 3, 3, 2, 1, 0, \
>                    0, 1, 2, 2, 2, 2, 1, 0, \
>                    0, 1, 1, 1, 1, 1, 1, 0, \
>                    0, 0, 0, 0, 0, 0, 0, 0]
>
> def score_board(board):
>    black_score, white_score = 0, 0
>    for index, piece in enumerate(board):

Iterating over the whole board is wasteful when it is usually empty
the dict approach would be better in this case.

>        type = piece_type(piece)
>        if type != EMPTY:
>            position_value = position_values[index]
>            if type == BLACK:
>                black_score += position_value
>            else:
>                white_score += position_value
>            black_value, white_value = piece_values[chr(piece)]
>            black_score += black_value
>            white_score += white_value
>    return (black_score, white_score)
>
> def turn_score(board, turn):
>    black_score, white_score = score_board(board)
>    return (white_score - black_score) if turn == WHITE else (black_score -
> white_score)
>
> def best_move(board, turn, ply):
>    best_score = -10000000

best_score = None

>    best_boards = []
>    for new_board in all_moves(board, turn):
>        if ply:
>            next_turn = BLACK if turn == WHITE else WHITE
>            score = turn_score(best_move(new_board, next_turn, ply - 1),
> turn)
>        else:
>            score = turn_score(new_board, turn)
>        if score > best_score or not best_boards:

if score > best_score or best_score is None:

>            best_score = score
>            best_boards = [new_board[:]]
>        elif score == best_score:
>            best_boards.append(new_board[:])
>    if best_boards:
>        return random.choice(best_boards)
>    return board[:]
>
> def main():
>    board = bytearray('RNBQKBNRPPPPPPPP
> pppppppprnbqkbnr')
>    turn = WHITE
>    while True:
>        board = best_move(board, turn, PLY)
>        turn = BLACK if turn == WHITE else WHITE
>        os.system('clear')
>        display_board(board)
>        #raw_input()
>
> if __name__ == '__main__':
>    main()


Oscar