nvzqz / Sage

A cross-platform chess library for Swift
Apache License 2.0
375 stars 43 forks source link

Board initializer use of piece.hashValue crashes under Swift 4.2 #21

Open SuperGeroy opened 5 years ago

SuperGeroy commented 5 years ago

Swift 4.2 introduced random seeding for hashing so that hash values are really different each time the program runs. The board initializer function in line 360 of Board.swift uses hash values as identifier for an array with 12 elements which under Swift 4.2/XCode 10 causes an index out of range error:

/// Create a chess board.
///
/// - parameter variant: The variant to populate the board for. Won't populate if `nil`. Default is `Standard`.
public init(variant: Variant? = ._standard) {
    #if swift(>=3)
        _bitboards = Array(repeating: 0, count: 12)
    #else
        _bitboards = Array(count: 12, repeatedValue: 0)
    #endif
    if let variant = variant {
        for piece in Piece.all {
            _bitboards[piece.hashValue] = Bitboard(startFor: piece)
        }
        if variant.isUpsideDown {
            for index in _bitboards.indices {
                _bitboards[index].flipVertically()
            }
        }
    }
}

The critical code is in the for loop: _bitboards[piece.hashValue] = Bitboard(startFor: piece) now crashes the program. I understand why the error is there but don't know why this program ran without error under Swift 4.1 because the hash value should (also under Swift 4.1) be a kind of random Int which may be bigger than the maximum index number 11 of the _bitboards array. @nvzqz seemed to have used the piece.hashValue rather like an identifier or rawValue.

Any help in explaining why the program didn't crash under Swift 4.1 and in rectifying the error would be appreciated!

SuperGeroy commented 5 years ago

I dealt with the issue in my fork of the repository: https://github.com/SuperGeroy/Sage

Indeed the piece.hashValue property was used like a rawValue and behavior of hash values changed under Swift 4.2.

netbe commented 5 years ago

@SuperGeroy are you taking over the project ? I am looking at your fork and it does work on my project (Xcode 10 & Swift 4.2) but I had to put the SWIFT_VERSION of the pod to `3.0

Also I am looking at a way to have the Sage.Game instance reading PGN moves but could not found anything on the project so far.

Here's what I got so far:

...
let position = Sage.Game.Position(fen: fenString)
let game = try Sage.Game(position: position)
let pgnParsed = try PGN(parse: pgnContent)
let pgnMoves = pgnParsed.moves
game.somethingPlay(pgnMoves[49]) // "Bf8"
...

Any help appreciated :)

SuperGeroy commented 5 years ago

Yes, I will further clean up the code to work well with Swift 4.2.

I am not using CocoaPods but rather copy the files directly into a project so I cannot say much about that. Should I correct the description in the readme?

PGN move parsing works for me with pull request #10. Need to include it in my branch though.

netbe commented 5 years ago

Thanks @SuperGeroy, #10 seems to be exactly what I needed.

Should I correct the description in the readme?

Yes.