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

draw knights moves using right-angled arrows #710

Closed
wants to merge 3 commits into from

Conversation

tewhalen
Copy link

Because I am a beginner at chess, it's easier for me to understand what's going on when arrows depicting a knight's move are drawn using right angles. I whipped up a patch that does this (and because I am not very good at math, I off-loaded some of the calculations onto the svg renderer). See attached image.
Screen Shot 2020-12-13 at 4 38 55 PM

@niklasf
Copy link
Owner

niklasf commented Dec 14, 2020

Thanks for the pull request. The rendered result looks pretty good. A couple of comments:

  • Please don't make changes to the coding style of the surrounding lines. It makes it hard to spot the actual changes. Instead, adjust your own additions to the surrounding coding style.
  • I like the right-angled style, but I don't think it should be default. Arrows by (2, 1) are not guaranteed to be knight moves, semantically. Instead, shape or style could be a property of Arrow, similar to color.
  • The transforms appear to be SVG Tiny compatible. We should double check that, though.

@sshivaji
Copy link

sshivaji commented Dec 14, 2020 via email

@tewhalen
Copy link
Author

Wanted to point out that the bishop move is incorrect, it's not diagonal in the diagram. Am sure this is probably minor.

Yeah, that's a D5 -> G3 arrow, not intended to indicate a bishop's move.

* Please don't make changes to the coding style of the surrounding lines. It makes it hard to spot the actual changes. Instead, adjust your own additions to the surrounding coding style.

Sorry about that – the perils of relying on black for formatting help.

* I like the right-angled style, but I don't think it should be default. Arrows by `(2, 1)` are not guaranteed to be knight moves, semantically. Instead, `shape` or `style` could be a property of `Arrow`, similar to `color`.

That makes sense. I'll see if I can revise to make the arrow style selectable. I'm still learning the ins and outs of SVG, so I made the whole arrow (line + head) a polygon, which perhaps is not the right move, since it will be harder to style. I'll see about switching the lines back to lines or polylines and making the arrow head a marker.

* The transforms appear to be SVG Tiny compatible. We should double check that, though.

Hrrrm, I wasn't aware that was the target - I'll see if I can find a validator and check the output.

@niklasf
Copy link
Owner

niklasf commented Dec 14, 2020

Sounds good. I don't mind the line/polygon decision. Whichever is easier to implement.

@tewhalen

This comment has been minimized.

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

Successfully merging this pull request may close these issues.

None yet

3 participants