Add chess960 support. This is a roll-up of all changes from the 'chess960support' branch.

[deverac]

This contains all changes up to and including the commit 'Refactor to remove PreMove and _preprocessMove().' f1b9523

This is a 'roll-up' of all changes made in PR #493.

[neofight78]

It's coming together nicely now.

Looking at load() it seems to me that the castling logic can be simplified quite a bit, and consequently elsewhere in the code. My observations are:

• _kings gives us the location of the kings already.
• Rightly or wrongly, the existing load() method does not validate then FEN. (There is a separate function for that). So there is no need to validate the castling field in load()
• For standard chess the rook squares are standard. For shredder fen the columns give us the location. Only for chess960 & k/q symbols is a search for the rook required.
• The use of rk as an intermediate structure can be eliminated.
• The __getKingAndRookInfo() method can be eliminated here and then elsewhere as after load() the king and rook placements are stored on the chess object as well as the castling rights. Maybe some simplified helper(s) will still be needed in some cases i.e. _updateCastlingRights()

Hopefully that gives you enough to go on?

[neofight78]

This looks like it might be a bug as it won't filter out any moves:

// return all non-castling moves
(mv) => mv.flags & (~BITS.KSIDE_CASTLE | ~BITS.QSIDE_CASTLE),

I think the correct logic is to OR the flags and then NOT the result.

Either this doesn't matter in which case there is no need for the filter, or there should be a test that fails on it (before fixing).

[deverac]

Great catch! You are exactly right about the OR/NOT logic. I've added a bug fix (with tests) for it.

I think I understand the other suggestions you mentioned, which all seem sensible. When I initially submitted the Chess960 changes, I was trying to touch the existing code as little as possible, but it's nice to have some knowledgeable eyes to help integrate the necessary changes into the existing code base. I think all of your suggestions have been good steps in the right direction.

[neofight78]

In addition to the suggested changes, one further optimisation could be made.

Instead of filtering all the moves with _filterMoves() and then looping through them, you could put all the logic together inside a find() allowing you to stop as soon as a match is made. Best to benchmark it before and after to check, but it should improve the speed I think.

[neofight78]

Worth taking note of this recent change / fix BTW: #554

Let me know if you are still happy to make the suggested changes / fixes. I assume you are just busy with other things right now?

[sowson]

Hi @deverac can you check this one. Is that by propuse? on my PR at last it works :-).

test('X-FEN 960 44', () => {
  const chess = new Chess(
    'n1rqb1kr/p1pppp1p/1pn4b/3P2p1/P2Q4/1P6/2P1PPPP/NNR1BBKR b HChc - 1 9', { chess960: true }
  )
  const moves = chess.move({ from: 'g8', to: 'f8' })
  expect(chess.fen()).toEqual(
    'n1rqbk1r/p1pppp1p/1pn4b/3P2p1/P2Q4/1P6/2P1PPPP/NNR1BBKR w HC - 2 10'
  )
})

[neofight78]

Hi @deverac, please let me know if you are wanting to get back to the branch and complete the changes. If not, we'll have to look at how to proceed.

[deverac]

@neofight78 @sowson I'm sorry about my absence. I'm back 'online' now and, if you are agreeable with it, will continue from were I left off.

[neofight78]

Yes, that would be great! This PR is not too far off being ready.

[deverac]

Hi @deverac can you check this one. Is that by propuse? on my PR at last it works :-).

test('X-FEN 960 44', () => {
  const chess = new Chess(
    'n1rqb1kr/p1pppp1p/1pn4b/3P2p1/P2Q4/1P6/2P1PPPP/NNR1BBKR b HChc - 1 9', { chess960: true }
  )
  const moves = chess.move({ from: 'g8', to: 'f8' })
  expect(chess.fen()).toEqual(
    'n1rqbk1r/p1pppp1p/1pn4b/3P2p1/P2Q4/1P6/2P1PPPP/NNR1BBKR w HC - 2 10'
  )
})

I am not quite sure what you are asking. This test above does not appear in the chess.js code base, so I assume that it is a test that you are running that passes on your PR, but fails on my PR.

In the test above, the expect() clause fails because the test expects w HC - to be returned by chess.fen(), but gets w KQ - instead. The use of 'HC' or 'KQ' in the FEN to record which rooks may castle is entirely implementation dependent. Neither is wrong, because both can be used to unambiguously recreate the castling information of the game. In Chess960, sometimes specifying 'KQ' cannot unambiguously recreate the castling information of the game. In such cases, chess.js will output the columns of the rooks (e.g. 'HC') that are allowed to castle, instead of using 'KQ'.

I am not sure if this has answered your question. If not, please rephrase your question.

[deverac]

In addition to the suggested changes, one further optimisation could be made.

Instead of filtering all the moves with _filterMoves() and then looping through them, you could put all the logic together inside a find() allowing you to stop as soon as a match is made. Best to benchmark it before and after to check, but it should improve the speed I think.

I wrote two versions of the code: one version is the current code in this PR (it calls the _filter() function and then runs a for loop afterwards); the other version removed the _filter() function but ran a for loop that implemented the logic of the _filter() function, but did not use moves.filter(). Both versions of the code passed all the tests.

I then wrote a test which performs a Chess960 castling move and makes the move 100,000 times. I ran the test against both version of the code. The second version of the code might be a tiny bit faster, but if it is, the difference is negligible.

When running the test, which uses Google/Node's V8 engine, I suspect the Javascript engine (or JIT compiler?) may be doing optimizations which causes both versions of the code to be essentially the same. When chess.js is run in a user's browser, the browser's Javascript engine may not be as optimized as V8 and there might be a significant difference (or not) between the running time of the two versions of code.

In any case, I think the version that removed the _filter() function is nicer because all the logic is contained in one place, within a single for loop.

[GuillaumeSD]

Hi guys, I'm just wondering what is the status of this PR ? Can we assume chess960 will be supported by this lib at some point ?