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

feat: Adds support for displaying NAGs in the chess.svg.board AND other optimizations chess.svg.board function #1062

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

Conversation

Vikasg7
Copy link

@Vikasg7 Vikasg7 commented Jan 23, 2024

  • Adds support for NAGs in the svg.
    • five NAGs has been added ie. !, !!, ?, ?? and ?!
      svg
  • Attempted to remove unnecessary if statements from chess.svg.board and separated the generation of X squares from the rendering of pieces.

@niklasf
Copy link
Owner

niklasf commented Feb 7, 2024

Nice addition, thanks!

Maybe a nags: Iterable[int] parameter would be more future-proof? We can currently only display one, but it could at least pick the most relevant one and show it. It would also be easier to just pass node.nags then, without having to filter out all the irrelevant ones (e.g., Ke2 $70 $6 $77).

@Vikasg7
Copy link
Author

Vikasg7 commented Feb 7, 2024

Hi,
I just covered 5 out of 6 here (6th being the speculative which I couldn't find the SVG for). Also, I am not sure how we are gonna decide the relevant ones in case of multiple nags. Also, I forgot to mention that we are covering only 5 in the function docs.

So if we decide on displaying multiple nags, it would be difficult to decide where to show them in the square as I am already moving the nag left or right based on where the arrow is coming from so that it doesn't overlap the arrow.

Glyphs show up at the end of the attached video which I am trying to produce with python.

output.mp4

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.

2 participants