Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#319: add null move functionality #451

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ppeloton
Copy link

Implemented null move logic for SAN '--' and added a simple test case. Please review, I am open for discussions!

@jhlywa
Copy link
Owner

jhlywa commented Mar 20, 2024

Thanks for the PR @ppeloton.

This is a good start on the null move logic. A couple of random questions that I haven't really thought through yet:

  • Are null moves supported in the PGN logic (both loading and exporting)?
  • Should null moves be entered with a dedicated function instead of the .move() function? I think there's a lot to think about here. A dedicated function would keep .move() consistent with the results of .moves(). It also side-steps the issue of not being able to enter a null move when using the verbose move format. A lot of people have been asking for setSideToMove function. Could this be considered a null move? Could we combine the two?
  • Should a null move entered at move 1 (this._history.length === 0) be omitted from history? Omitting it would require modifying the starting FEN and updating the Setup header. This relates to the previous question about setSideToMove.
  • Can null moves be made in succession?
  • Can null moves be made when the side to move is in check?

I'd like to let this PR soak for a bit to let us (and others) ponder some of these questions. Thanks again for submitting!

src/chess.ts Outdated
@@ -101,6 +101,7 @@ const FLAGS: Record<string, string> = {
PROMOTION: 'p',
KSIDE_CASTLE: 'k',
QSIDE_CASTLE: 'q',
NULL_MOVE: 'nm',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FLAGS.NULL_MOVE value should be a single character. As written, the n in nm collides with FLAGS.NORMAL. Maybe use '-'?

The whole flag system is an artifact of some C code I wrote years ago, and is kinda kludgy.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed this with the suggested value '-'

src/chess.ts Outdated
@@ -1961,6 +1981,8 @@ export class Chess {
output = 'O-O'
} else if (move.flags & BITS.QSIDE_CASTLE) {
output = 'O-O-O'
} else if (move.flags & BITS.NULL_MOVE){
output = SAN_NULLMOVE
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you just return SAN_NULLMOVE here to skip the rest of this function?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I fixed this.

@ppeloton
Copy link
Author

@jhlywa : Happy to contribute and thank you for your good feedback and questions. Generally, I think that null moves do not have one common standard, so if we want to implement this in the package, the developers have to make choices and document how null moves are handled in chess.js

About your questions:

  • Exporting/Importing in pgn: Just the SAN for a null move has no standard (see e.g. https://chess.stackexchange.com/questions/14072/san-for-nullmove), so exporting / importing of null moves is probably differently supported. I personally use Scid, which supports importing/exporting of null moves in pgn files (using SAN '--').

  • a seperate method for null moves sounds good (e.g. makeNullMove()) sounds like a good way to implement it. Even more if there is a wish for setSideToMove functionality (if I understand it correctly, it basically corresponds to making a null move and giving the turn to the otherside).

  • Rules relating null moves: I have not found a standard for this. But using Scid as a reference, the program allows multiple null moves in succession but no null moves when one is in check. I could try to find out if there are more rules in Scid related to null moves and we good take that as a starting point for this package.

I am happy to continue the discussion!

@ppeloton
Copy link
Author

Sorry, I forgot to run the prettify command. I hope now everything is fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants