C Language, I am having with my bishop

newb
#1 / 7

I am having with my bishop

Hello:
Recently, I have finished writing a chess program. Unfortunately, my
bishop does not move. Can someone please tell me what is wrong with my
code and what things i should check for bishop? Below is my bishop
function. (NULL = .) (. = empty space)
int validBishop(int *from, int *to, char board[][10])
{
int validMove = 0;
int row1 = from[0], col1 = from[1], row2 = to[0], col2 = to[1];
int diffRow = abs(row1 - row2), diffCol = abs(row1 - row2);

if(diffRow == diffCol) {
if(row1 > row2) {
if(col1 < col2) {
for(row1++; row1 != row2; row1++)
for(col1++; col1 != col2; col1++)
if(board[row1][col1] != NULL) return(validMove);
}

if(col1 > col2) {
for(row1++; row1 != row2; row1++)
for(col1--; col1 != col2; col1--)
if(board[row1][col1] != NULL) return(validMove);
}

return(validMove = 1);
}
if(row1 < row2) {
if(col1 < col2) {
for(row1++; row1 != row2; row1++)
for(col1++; col1 != col2; col1++)
if(board[row1][col1] != NULL) return(validMove);
}

if(col1 > col2) {
for(row1++; row1 != row2; row1++)
for(col1--; col1 != col2; col1--)
if(board[row1][col1] != NULL) return(validMove);
}

return(validMove = 1);
}

Tue, 24 Aug 2004 15:34:49 GMT

Martin Ambuh
#2 / 7

I am having with my bishop

Quote:

> Hello:
> Recently, I have finished writing a chess program. Unfortunately, my
> bishop does not move. Can someone please tell me what is wrong with my
> code and what things i should check for bishop? Below is my bishop
> function. (NULL = .) (. = empty space)
> int validBishop(int *from, int *to, char board[][10])

Among other things, an element of board is a char, but NULL is a pointer
constant. If you mean to store '.' into an empty space and use a
symbolic name, choose something like EMPTY instead of a name that
conflicts with a defined term in C.

--
It is better that the grammarians should chide us
than that the people should not understand us.
- St. Augustine

Tue, 24 Aug 2004 17:29:05 GMT

Tim Wooda
#3 / 7

I am having with my bishop

On 7 Mar 2002 23:34:49 -0800,

Quote:

> Hello:
> Recently, I have finished writing a chess program. Unfortunately, my
> bishop does not move. Can someone please tell me what is wrong with my
> code and what things i should check for bishop? Below is my bishop
> function. (NULL = .) (. = empty space)

Strange use of NULL! NULL is a pointer, you are comparing a char.

Quote:

> int validBishop(int *from, int *to, char board[][10])
> {
> int validMove = 0;
> int row1 = from[0], col1 = from[1], row2 = to[0], col2 = to[1];
> int diffRow = abs(row1 - row2), diffCol = abs(row1 - row2);

> if(diffRow == diffCol) {
> if(row1 > row2) {
^^^^
> if(col1 < col2) {
> for(row1++; row1 != row2; row1++)

^^^ ^^^
How can this work.

Quote:

> for(col1++; col1 != col2; col1++)
> if(board[row1][col1] != NULL) return(validMove);
> }

The whole function look strange anyway: shouldn't it be something like:

if(diffRow != diffCol)
return 0;

if(row1 > row2 && col1 > col2)
for(i=0;i<diffRow;i++)
if(board[row1-i][col1-i] != 0) return 0;
if(row1 > row2 && col1 < col2)
for(i=0;i<diffRow;i++)
if(board[row1-i][col1+i+1] != 0) return 0;
if(row1 < row2 && col1 > col2)
for(i=0;i<diffRow;i++)
if(board[row1+i+1][col1-i] != 0) return 0;
if(row1 < row2 && col1 < col2)
for(i=0;i<diffRow;i++)
if(board[row1+i+1][col1+i+1] != 0) return 0;

return 1;

But this doesn't seem to be handling the case of a capture!
Makes chess rather "interesting" if you aren't allowed to take
anything :-(

Tim.

--

and there was light.

http://tjw.hn.org/ http://www.locofungus.btinternet.co.uk/

Tue, 24 Aug 2004 21:10:12 GMT

Richard B
#4 / 7

I am having with my bishop

Quote:

> Hello:
> Recently, I have finished writing a chess program. Unfortunately, my
> bishop does not move. Can someone please tell me what is wrong with my
> code and what things i should check for bishop? Below is my bishop
> function. (NULL = .) (. = empty space)
> int validBishop(int *from, int *to, char board[][10])
> {
> int validMove = 0;
> int row1 = from[0], col1 = from[1], row2 = to[0], col2 = to[1];
> int diffRow = abs(row1 - row2), diffCol = abs(row1 - row2);

> if(diffRow == diffCol) {
> if(row1 > row2) {
> if(col1 < col2) {
> for(row1++; row1 != row2; row1++)
> for(col1++; col1 != col2; col1++)
> if(board[row1][col1] != NULL) return(validMove);
> }

Here, and in the other blocks, you're not checking the diagonal between
the from position and the to position for blocking pieces, you're
checking the entire square of which that line is the diagonal.

Richard

Tue, 24 Aug 2004 20:07:54 GMT

Dann Corbi
#5 / 7

I am having with my bishop

Suggestions:
Computer chess newsgroup:

There is a lot of noise there, but if you only have a newsreader and limited
web access, it might be your only option.

Computer Chess Club (most of the best chess programmers in the world hang
out there):
http://www.talkchess.com/forums/1/index.html?from=0&to=168&view_by=th...
You have to sign up first here (but it is free and anyone can join):
http://www.talkchess.com/signup.html

Winboard Forum:
http://f11.parsimony.net/forum16635/
--
C-FAQ: http://www.eskimo.com/~scs/C-faq/top.html
"The C-FAQ Book" ISBN 0-201-84519-9
C.A.P. FAQ: ftp://cap.connx.com/pub/Chess%20Analysis%20Project%20FAQ.htm

Wed, 25 Aug 2004 01:50:11 GMT

Dave Vandervi
#6 / 7

I am having with my bishop

Quote:

>Among other things, an element of board is a char, but NULL is a pointer
>constant. If you mean to store '.' into an empty space and use a
>symbolic name, choose something like EMPTY instead of a name that
>conflicts with a defined term in C.

Doesn't EMPTY also invade the implementation's namespace? Or is that
only for macros when appropriate headers (<errno.h> comes to mind)
have been included?
(In any case, I can't be the only one who finds it easier to avoid names
that are reserved in any context everywhere. There are more than enough
left to go around.)

dave
(Hmm... EMPTY... Error, mangled pseudo-TTY device detected?)

--

I never thought of this combination before, but what if _pointers_ were
floating point? There may be a DS9000.001 design here ... we could support
as a nonstandard extension &struct.bitfield ... --David Thompson in CLC

Tue, 31 Aug 2004 04:25:19 GMT

David Thompso
#7 / 7

I am having with my bishop

...

Quote:

> int validBishop(int *from, int *to, char board[][10])
> {
> int validMove = 0;
> int row1 = from[0], col1 = from[1], row2 = to[0], col2 = to[1];
> int diffRow = abs(row1 - row2), diffCol = abs(row1 - row2);

Personally I might use for position a struct containing (named)
row,col members instead of an array of 2 int and have to remember
everyplace which is which. And I might well pass the positions
by value here and just reuse the parameters as locals; the normal
reasons to pass an argument as (by) pointer is that you need to
modify it, it is so large copying is costly, or its identity matters
as well as (or instead of) its value; none of those apply here.

Why [10]? Are you leaving a border? I would probably write
[10][10] as documentation even though the leading dimension
_on a function parameter_ is effectively discarded.

Quote:

> if(diffRow == diffCol) {
> if(row1 > row2) {
> if(col1 < col2) {
> for(row1++; row1 != row2; row1++)
> for(col1++; col1 != col2; col1++)
> if(board[row1][col1] != NULL) return(validMove);
> }

This case, and the following one, are backwards: you step
row1 up for the case where it is already > row2. This runs
off the end of your array, probably accessing garbage (that
happens to appear as non-empty squares) until/unless
it wraps around, but much worse results are possible.

Others have already commented about NULL for int.
For that matter, char is (portably) more than big enough
for a reasonable piece code, and saves space on nearly(?)
all implementations, which might matter if you want to store
large numbers of (e.g. candidate) boards/sequences.

And it is unnecessary and a bit silly to use a variable
that is in fact constant to contain the return value;
either just return(0) (and you can omit the parens if you like)
or for adherents (or dogmatists?) of structured code
initialize validMove to 1, _assign_ 0 when a conflict is found,
and _fall through_ to a common return of the variable.

Quote:

> if(col1 > col2) {

You don't really need to test col1 > col2; if you instead
make this an 'else' to the 'if' above it will only cover
col1>=col2, and since you've already checked diagonality
col1 == col2 only if also row1 == row2 which means an
attempt to move to the same square, which the caller(s)
should be able to avoid easily, and if not, a single check
above for diffRow (or diffCol) == 0 will handle it.

...

Quote:

> return(validMove = 1);

As above, it's silly to assign a variable here.

Remaining cases have the correct direction for row1
but other comments above apply.

Finally, I would probably consider using instead a 1D array
and do the i*10+j myself; or even better i*8U+j, which is
almost always implemented as a shift instead of a multiply,
and more importantly the 2 axes can be separated back out
by masking and shifting rather than full division/remainder.
Then I can use a single composite stride: +7 +9 -9 -7 for
bishop; +8 -1 +1 -8 for rook; +8 or -8 for normal pawn; etc.
Although a good compiler might manage to do this optimization
itself, this is one case where I would be reluctant to rely on it.

--
- David.Thompson 1 now at worldnet.att.net

Tue, 07 Sep 2004 12:08:31 GMT