Tough promotion bug(s)

[Henk]

In my chess program a chess piece is not represented by a simple scalar.

By the way it is easier to remove a redundant class than failure to introduce one. So in my opinion if in doubt always make it a class.

[Sven]

In your implementation a piece appears to be something like an immutable object. The white pawn that is on d2 in the initial position is always that particular pawn. Promoting a pawn to a queen means to take that pawn object from the board and to put a new queen object on the promotion square instead. Therefore for unmaking a promotion you need to remember that particular pawn somehow in order to restore it after removing the queen. As you see this can cause trouble.

But you could also think about a slightly different modelling of pieces, by making their piece type a property that can be changed. You would have two operations on a piece object for that purpose:

Code: Select all

void promote(PieceType promotionPieceType); // change piece type into given type
void unpromote(); // change piece type into "Pawn"

That could make your life a lot easier. At least it would avoid the need to store anything about promotions on a stack. Only the move data structure needs to include the type of the promotion piece, if any.

In many chess engines the representation of pieces and their types is even simpler.

Modeling type as a property is some form of the bridge design pattern. A disadvantage is that it is takes more time to access the PieceType (piece.PieceType instead of piece (where piece is PieceType))

Think simple. You do not need to think of fairly complex design patterns like "Bridge" in this case. Just think of the piece type as an attribute that is usually never modified after creation but can be modified by promote() and unpromote(). In terms of software design, let all pieces be of the same class "Piece", i.e. no "Pawn", "Knight" etc. derived classes.

In the implementation I would even drop the "Piece" class (since it is overkill in practice) and simply replace it by a scalar that represents the piece type, and possibly also the color (depending on your design). Then the promote() and unpromote() operations become operations of the Board class, with an additional parameter to identify the location of the piece (i.e. the square number). And still you will not need a stack-like data structure to store some history of promotions.

Sven

[Henk]

In your implementation a piece appears to be something like an immutable object. The white pawn that is on d2 in the initial position is always that particular pawn. Promoting a pawn to a queen means to take that pawn object from the board and to put a new queen object on the promotion square instead. Therefore for unmaking a promotion you need to remember that particular pawn somehow in order to restore it after removing the queen. As you see this can cause trouble.

But you could also think about a slightly different modelling of pieces, by making their piece type a property that can be changed. You would have two operations on a piece object for that purpose:

Code: Select all

void promote(PieceType promotionPieceType); // change piece type into given type
void unpromote(); // change piece type into "Pawn"

That could make your life a lot easier. At least it would avoid the need to store anything about promotions on a stack. Only the move data structure needs to include the type of the promotion piece, if any.

In many chess engines the representation of pieces and their types is even simpler.

Modeling type as a property is some form of the bridge design pattern. A disadvantage is that it is takes more time to access the PieceType (piece.PieceType instead of piece (where piece is PieceType))

Think simple. You do not need to think of fairly complex design patterns like "Bridge" in this case. Just think of the piece type as an attribute that is usually never modified after creation but can be modified by promote() and unpromote(). In terms of software design, let all pieces be of the same class "Piece", i.e. no "Pawn", "Knight" etc. derived classes.

In the implementation I would even drop the "Piece" class (since it is overkill in practice) and simply replace it by a scalar that represents the piece type, and possibly also the color (depending on your design). Then the promote() and unpromote() operations become operations of the Board class, with an additional parameter to identify the location of the piece (i.e. the square number). And still you will not need a stack-like data structure to store some history of promotions.

Sven

I have different methods for Pawn and King. I also think if I would remove these classes I get a lot of switch statements (or if statements). I don't think the implementation will be more clear or stable or extendable.

About thinking simple. I think using a switch statement is a design error.

To be honest I also have a BlackKing, WhiteKing, WhiteBishop etc class.

I'm a class freak

[gladius]

In your implementation a piece appears to be something like an immutable object. The white pawn that is on d2 in the initial position is always that particular pawn. Promoting a pawn to a queen means to take that pawn object from the board and to put a new queen object on the promotion square instead. Therefore for unmaking a promotion you need to remember that particular pawn somehow in order to restore it after removing the queen. As you see this can cause trouble.

But you could also think about a slightly different modelling of pieces, by making their piece type a property that can be changed. You would have two operations on a piece object for that purpose:

Code: Select all

void promote(PieceType promotionPieceType); // change piece type into given type
void unpromote(); // change piece type into "Pawn"

That could make your life a lot easier. At least it would avoid the need to store anything about promotions on a stack. Only the move data structure needs to include the type of the promotion piece, if any.

In many chess engines the representation of pieces and their types is even simpler.

Modeling type as a property is some form of the bridge design pattern. A disadvantage is that it is takes more time to access the PieceType (piece.PieceType instead of piece (where piece is PieceType))

Think simple. You do not need to think of fairly complex design patterns like "Bridge" in this case. Just think of the piece type as an attribute that is usually never modified after creation but can be modified by promote() and unpromote(). In terms of software design, let all pieces be of the same class "Piece", i.e. no "Pawn", "Knight" etc. derived classes.

In the implementation I would even drop the "Piece" class (since it is overkill in practice) and simply replace it by a scalar that represents the piece type, and possibly also the color (depending on your design). Then the promote() and unpromote() operations become operations of the Board class, with an additional parameter to identify the location of the piece (i.e. the square number). And still you will not need a stack-like data structure to store some history of promotions.

Sven

I have different methods for Pawn and King. I also think if I would remove these classes I get a lot of switch statements (or if statements). I don't think the implementation will be more clear or stable or extendable.

About thinking simple. I think using a switch statement is a design error.

To be honest I also have a BlackKing, WhiteKing, WhiteBishop etc class.

I'm a class freak

I'm not quite sure if you are being serious. Can you post a snippet of code from your engine?

Generally, favoring composition over inheritance is a good thing. ie. Your Piece class could have pieceType and color members. http://en.wikipedia.org/wiki/Compositio ... nheritance

[Henk]

In your implementation a piece appears to be something like an immutable object. The white pawn that is on d2 in the initial position is always that particular pawn. Promoting a pawn to a queen means to take that pawn object from the board and to put a new queen object on the promotion square instead. Therefore for unmaking a promotion you need to remember that particular pawn somehow in order to restore it after removing the queen. As you see this can cause trouble.

But you could also think about a slightly different modelling of pieces, by making their piece type a property that can be changed. You would have two operations on a piece object for that purpose:

Code: Select all

void promote(PieceType promotionPieceType); // change piece type into given type
void unpromote(); // change piece type into "Pawn"

That could make your life a lot easier. At least it would avoid the need to store anything about promotions on a stack. Only the move data structure needs to include the type of the promotion piece, if any.

In many chess engines the representation of pieces and their types is even simpler.

Modeling type as a property is some form of the bridge design pattern. A disadvantage is that it is takes more time to access the PieceType (piece.PieceType instead of piece (where piece is PieceType))

Think simple. You do not need to think of fairly complex design patterns like "Bridge" in this case. Just think of the piece type as an attribute that is usually never modified after creation but can be modified by promote() and unpromote(). In terms of software design, let all pieces be of the same class "Piece", i.e. no "Pawn", "Knight" etc. derived classes.

In the implementation I would even drop the "Piece" class (since it is overkill in practice) and simply replace it by a scalar that represents the piece type, and possibly also the color (depending on your design). Then the promote() and unpromote() operations become operations of the Board class, with an additional parameter to identify the location of the piece (i.e. the square number). And still you will not need a stack-like data structure to store some history of promotions.

Sven

I have different methods for Pawn and King. I also think if I would remove these classes I get a lot of switch statements (or if statements). I don't think the implementation will be more clear or stable or extendable.

About thinking simple. I think using a switch statement is a design error.

To be honest I also have a BlackKing, WhiteKing, WhiteBishop etc class.

I'm a class freak

I'm not quite sure if you are being serious. Can you post a snippet of code from your engine?

Generally, favoring composition over inheritance is a good thing. ie. Your Piece class could have pieceType and color members. http://en.wikipedia.org/wiki/Compositio ... nheritance

Yes I am serious. I only know a few design patterns. That one you mention composition over inheritance, I did not know about. Using PieceType doesn't look efficient, means an extra indirection.

I'm not posting code snippets, it's not written for other people. I'm also doing some bitboard stuff in these classes. Virtual methods are very handy.

class BlackKing : Skipper.Models.King
{
public BlackKing(CPosition.Coord startPos, CPosition position)
: base(ChessPiece.Color.Black, startPos, position)
{

}

public BlackKing()
: base(ChessPiece.Color.Black)
{
}

public override Skipper.Models.ChessPiece.Color PieceColor
{
get
{
return Color.Black;
}
}

public override int Value
{
get
{
return 300 * 1024;
}

}

/// <summary>
/// Bitset which contains one flag.
/// Flag is true when this piece has moved.
/// </summary>
public override ulong HasMovedBit
{
get
{
return BitSet64.UBit(BitBoard.BLACK_KING_HASMOVED);
}
}

//public override void RemoveFromBoard(CPosition pos)
//{

// Location.Occupier = null;
// pos.Board.RemoveBlackPieceFromBoard(this, Location.NegatedBitCoord);
// Removed = true;

//}

public override void RemoveFromBoard(CPosition pos)
{

Location.Occupier = null;

BitBoard board = pos.Board;
ulong negCoord = Location.NegatedBitCoord;

board.BlackPieces &= negCoord;
board.Kings &= negCoord;
Removed = true;


}



public override void PutOnBoard(CPosition pos)
{

Location.Occupier = this;

ulong coord = Location.BitCoord;
BitBoard board = pos.Board;

board.BlackPieces |= coord;
board.Kings |= coord;
Removed = false;
}


}
}

[Henk]

forget it

[sje]

Your project is DOOMED.

[Henk]

Your project is DOOMED.

Of course everybody knows that.

[Codesquid]

Code: Select all

 class BlackKing : Skipper.Models.King
    {
        public BlackKing(CPosition.Coord startPos, CPosition position)
            : base(ChessPiece.Color.Black, startPos, position)
        {

        }

        public BlackKing()
            : base(ChessPiece.Color.Black)
        {
        }

        public override Skipper.Models.ChessPiece.Color PieceColor
        {
            get
            {
                return Color.Black;
            }
        }

        public override int Value
        {
            get
            {
                return 300 * 1024;
            }

        }

        /// <summary>
        /// Bitset which contains one flag.
        /// Flag is true when this piece has moved.
        /// </summary>
        public override ulong HasMovedBit
        &#123;
            get
            &#123;
                return BitSet64.UBit&#40;BitBoard.BLACK_KING_HASMOVED&#41;;
            &#125;
        &#125;

        //public override void RemoveFromBoard&#40;CPosition pos&#41;
        //&#123;

        //    Location.Occupier = null;
        //    pos.Board.RemoveBlackPieceFromBoard&#40;this, Location.NegatedBitCoord&#41;;
        //    Removed = true;

        //&#125;

        public override void RemoveFromBoard&#40;CPosition pos&#41;
        &#123;

            Location.Occupier = null;

            BitBoard board = pos.Board;
            ulong negCoord = Location.NegatedBitCoord;

            board.BlackPieces &= negCoord;
            board.Kings &= negCoord;
            Removed = true;


        &#125;



        public override void PutOnBoard&#40;CPosition pos&#41;
        &#123;

            Location.Occupier = this;

            ulong coord = Location.BitCoord;
            BitBoard board = pos.Board;

            board.BlackPieces |= coord;
            board.Kings |= coord;
            Removed = false;
        &#125;


    &#125;
&#125;

http://www.youtube.com/watch?v=6nSKkwzwdW4

[Henk]

Code: Select all

 class BlackKing : Skipper.Models.King
    {
        public BlackKing(CPosition.Coord startPos, CPosition position)
            : base(ChessPiece.Color.Black, startPos, position)
        {

        }

        public BlackKing()
            : base(ChessPiece.Color.Black)
        {
        }

        public override Skipper.Models.ChessPiece.Color PieceColor
        {
            get
            {
                return Color.Black;
            }
        }

        public override int Value
        {
            get
            {
                return 300 * 1024;
            }

        }

        /// <summary>
        /// Bitset which contains one flag.
        /// Flag is true when this piece has moved.
        /// </summary>
        public override ulong HasMovedBit
        &#123;
            get
            &#123;
                return BitSet64.UBit&#40;BitBoard.BLACK_KING_HASMOVED&#41;;
            &#125;
        &#125;

        //public override void RemoveFromBoard&#40;CPosition pos&#41;
        //&#123;

        //    Location.Occupier = null;
        //    pos.Board.RemoveBlackPieceFromBoard&#40;this, Location.NegatedBitCoord&#41;;
        //    Removed = true;

        //&#125;

        public override void RemoveFromBoard&#40;CPosition pos&#41;
        &#123;

            Location.Occupier = null;

            BitBoard board = pos.Board;
            ulong negCoord = Location.NegatedBitCoord;

            board.BlackPieces &= negCoord;
            board.Kings &= negCoord;
            Removed = true;


        &#125;



        public override void PutOnBoard&#40;CPosition pos&#41;
        &#123;

            Location.Occupier = this;

            ulong coord = Location.BitCoord;
            BitBoard board = pos.Board;

            board.BlackPieces |= coord;
            board.Kings |= coord;
            Removed = false;
        &#125;


    &#125;
&#125;

http://www.youtube.com/watch?v=6nSKkwzwdW4

Yes as I said the code is not written for others. By the way If I look at code from other people I'm shocked too.

Comment above HasMovedBit property is wrong. Forget to update it or remove it. People interested:

public static UInt64 UBit(int pos)
{
return ((UInt64)1) << (int)pos;
}